chore: Lower CI test times

.github/workflows/tests.yml

@@ -65,7 +65,6 @@ jobs:
     name: Backend tests
     needs: 
       - check-changes
-      - check-ts-drift
     runs-on: ubuntu-latest
     timeout-minutes: 20
     steps:
@@ -94,7 +93,6 @@ jobs:
     name: Postgres tests
     needs: 
       - check-changes
-      - check-ts-drift
     runs-on: ubuntu-latest
     timeout-minutes: 45
     services:
@@ -138,7 +136,6 @@ jobs:
     name: UI unit tests
     needs: 
       - check-changes
-      - check-ts-drift
     runs-on: ubuntu-latest
     steps:
       - name: Checkout
@@ -159,7 +156,6 @@ jobs:
     name: UI OS tests
     needs: 
       - check-changes
-      - check-ts-drift
     runs-on: ubuntu-latest
     services:
       email-service:
@@ -205,7 +201,6 @@ jobs:
     name: UI EE tests
     needs: 
       - check-changes
-      - check-ts-drift
     runs-on: ubuntu-latest
     services:
       email-service:

forge/db/controllers/AccessToken.js

@@ -1,4 +1,4 @@
-const { Op } = require('sequelize')
+const { Op, UniqueConstraintError } = require('sequelize')
 
 const mcpToolPermissions = require('../../lib/mcpToolPermissions')
 const { generateToken, generateNumericToken, sha256, randomPhrase, DEFAULT_TOKEN_SESSION_EXPIRY, DEFAULT_REFRESH_TOKEN_EXPIRY } = require('../utils')
@@ -495,26 +495,39 @@ module.exports = {
             const token = generateToken(32, prefix)
             const newRefreshToken = generateToken(32, prefix)
             const expiresAt = capToGrant(Date.now() + DEFAULT_TOKEN_SESSION_EXPIRY, grantExpiresAtMs)
-            // Compare-and-swap: matches only while the token is current, so of two
-            // simultaneous refreshes exactly one rotates.
-            const [rotatedCount] = await app.db.models.AccessToken.update(
-                {
-                    token,
-                    expiresAt,
-                    refreshToken: newRefreshToken,
-                    refreshTokenExpiresAt: capToGrant(Date.now() + DEFAULT_REFRESH_TOKEN_EXPIRY, grantExpiresAtMs)
-                },
-                { where: { refreshToken: existingToken.refreshToken } }
-            )
-            if (rotatedCount > 0) {
-                await app.db.models.AccessTokenRefreshRotation.create({
+            // Claim the rotation by recording it before rotating the row: the unique tokenHash
+            // index lets exactly one of two simultaneous refreshes insert it, and the record
+            // already exists when the other one looks it up below.
+            let rotation = null
+            try {
+                rotation = await app.db.models.AccessTokenRefreshRotation.create({
                     tokenHash: existingToken.refreshToken,
                     rotatedAt: new Date(),
                     AccessTokenId: existingToken.id
                 })
+            } catch (err) {
+                if (!(err instanceof UniqueConstraintError)) {
+                    throw err
+                }
+            }
+            if (rotation) {
+                const [rotatedCount] = await app.db.models.AccessToken.update(
+                    {
+                        token,
+                        expiresAt,
+                        refreshToken: newRefreshToken,
+                        refreshTokenExpiresAt: capToGrant(Date.now() + DEFAULT_REFRESH_TOKEN_EXPIRY, grantExpiresAtMs)
+                    },
+                    { where: { refreshToken: existingToken.refreshToken } }
+                )
+                if (rotatedCount === 0) {
+                    // The grant was revoked since it was read.
+                    await rotation.destroy()
+                    return null
+                }
                 return { token, expiresAt, refreshToken: newRefreshToken }
             }
-            // Lost the swap: the presented token is now rotated out, resolve it below.
+            // Lost the claim: the presented token is now rotated out, resolve it below.
         }
 
         const retired = await app.db.models.AccessToken.byRotatedRefreshToken(refreshToken)

package.json

@@ -35,8 +35,8 @@
         "lint:colors": "node scripts/lint-colors.js",
         "test": "npm-run-all --sequential lint test:unit test:system",
         "test:unit": "npm-run-all --sequential test:unit:forge test:unit:frontend",
-        "test:unit:forge": "mocha \"test/unit/forge/**/*_spec.js\" --timeout 10000 --node-option=unhandled-rejections=strict",
-        "test:unit:forge:ci": "mocha  \"test/unit/forge/**/*_spec.js\" --bail --timeout 10000 --node-option=unhandled-rejections=strict",
+        "test:unit:forge": "mocha \"test/unit/forge/**/*_spec.js\" --require test/lib/fast-bcrypt.js --require test/lib/fastify-schema-cache.js --timeout 10000 --node-option=unhandled-rejections=strict",
+        "test:unit:forge:ci": "mocha  \"test/unit/forge/**/*_spec.js\" --parallel --jobs 4 --require test/lib/fast-bcrypt.js --require test/lib/fastify-schema-cache.js --bail --timeout 10000 --node-option=unhandled-rejections=strict",
         "test:unit:frontend": "vitest run --config ./config/vitest.config.ts",
         "test:docs": "node test/e2e/docs/valid-links.js ./docs",
         "test:system": "mocha \"test/system/**/*_spec.js\" --timeout 10000 --node-option=unhandled-rejections=strict",

test/lib/fast-bcrypt.js

@@ -0,0 +1,6 @@
+// Hashing at the production cost of 10 rounds dominates test CPU time.
+// Tests only need valid hashes, so use the minimum cost.
+const bcrypt = require('bcrypt')
+
+const hashSync = bcrypt.hashSync
+bcrypt.hashSync = (value) => hashSync(value, 4)

test/lib/fastify-schema-cache.js

@@ -0,0 +1,47 @@
+// Every test boots many app instances, and each one compiles the same route
+// schemas from scratch. Share compiled validators and serializers between the
+// instances in a process, keyed on the schema and compiler options.
+const { createHash } = require('crypto')
+const path = require('path')
+
+// Use the compilers fastify itself depends on, so behaviour matches the default
+const fastifyPath = require.resolve('fastify')
+const fromFastify = name => require(require.resolve(name, { paths: [path.dirname(fastifyPath)] }))
+const AjvCompiler = fromFastify('@fastify/ajv-compiler')
+const SerializerCompiler = fromFastify('@fastify/fast-json-stringify-compiler')
+
+const hash = value => createHash('sha1').update(JSON.stringify(value) ?? 'undefined').digest('hex')
+
+function cachedFactory (baseFactory, kind) {
+    const cache = new Map()
+    return function (externalSchemas, options) {
+        const compile = baseFactory(externalSchemas, options)
+        const prefix = kind + hash(externalSchemas) + hash(options)
+        return function (routeOptions) {
+            const key = prefix + hash(routeOptions.schema) + (routeOptions.httpPart || '') + (routeOptions.httpStatus || '') + (routeOptions.contentType || '')
+            let fn = cache.get(key)
+            if (!fn) {
+                fn = compile(routeOptions)
+                cache.set(key, fn)
+            }
+            return fn
+        }
+    }
+}
+
+const compilersFactory = {
+    buildValidator: cachedFactory(AjvCompiler(), 'v'),
+    buildSerializer: cachedFactory(SerializerCompiler(), 's')
+}
+
+const fastify = require(fastifyPath)
+function fastifyWithSchemaCache (options = {}) {
+    return fastify({
+        ...options,
+        schemaController: { ...options.schemaController, compilersFactory }
+    })
+}
+Object.assign(fastifyWithSchemaCache, fastify)
+fastifyWithSchemaCache.default = fastifyWithSchemaCache
+fastifyWithSchemaCache.fastify = fastifyWithSchemaCache
+require.cache[fastifyPath].exports = fastifyWithSchemaCache

test/unit/forge/db/controllers/AccessToken_spec.js

@@ -1,4 +1,6 @@
-const should = require('should') // eslint-disable-line
+const should = require('should')
+const sinon = require('sinon')
+
 const { sha256 } = require('../../../../../forge/db/utils')
 const mcpToolPermissions = require('../../../../../forge/lib/mcpToolPermissions')
 const setup = require('../setup')
@@ -637,6 +639,47 @@ describe('AccessToken controller', function () {
             live.length.should.be.aboveOrEqual(1)
         })
 
+        it('re-mints an access token when another refresh already claimed the rotation', async function () {
+            const original = await createToken()
+            await setRowExpiry(original.refreshToken, { accessMs: -5000 })
+            const row = await app.db.models.AccessToken.byRefreshToken(original.refreshToken)
+            // Record the claim as a concurrent refresh would, between the lookup and the rotation.
+            await app.db.models.AccessTokenRefreshRotation.create({
+                tokenHash: sha256(original.refreshToken),
+                rotatedAt: new Date(),
+                AccessTokenId: row.id
+            })
+
+            const result = await app.db.controllers.AccessToken.refreshToken(original.refreshToken)
+            should.exist(result)
+            result.should.not.have.property('replay')
+            should.not.exist(result.refreshToken)
+            should.exist(await app.db.controllers.AccessToken.getOrExpire(result.token))
+        })
+
+        it('returns null and releases the claim when the grant is revoked mid-refresh', async function () {
+            const original = await createToken()
+            await setRowExpiry(original.refreshToken, { accessMs: -5000 })
+            sinon.stub(app.db.models.AccessToken, 'update').resolves([0])
+            try {
+                should.not.exist(await app.db.controllers.AccessToken.refreshToken(original.refreshToken))
+            } finally {
+                app.db.models.AccessToken.update.restore()
+            }
+            ;(await app.db.models.AccessTokenRefreshRotation.count({ where: { tokenHash: sha256(original.refreshToken) } })).should.equal(0)
+        })
+
+        it('rethrows errors other than a lost claim', async function () {
+            const original = await createToken()
+            await setRowExpiry(original.refreshToken, { accessMs: -5000 })
+            sinon.stub(app.db.models.AccessTokenRefreshRotation, 'create').rejects(new Error('db down'))
+            try {
+                await app.db.controllers.AccessToken.refreshToken(original.refreshToken).should.be.rejectedWith('db down')
+            } finally {
+                app.db.models.AccessTokenRefreshRotation.create.restore()
+            }
+        })
+
         describe('with a grant expiry', function () {
             const DAY = 24 * 60 * 60 * 1000
 

test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js

@@ -1077,8 +1077,8 @@ describe('Application Device Groups API', function () {
             // get the group from DB and verify it contains 2 devices (device1, device3)
             const updatedDeviceGroup2 = await app.db.models.DeviceGroup.byId(deviceGroup.hashid)
             updatedDeviceGroup2.should.have.property('Devices').and.have.length(2)
-            updatedDeviceGroup2.Devices[0].should.have.property('id', device1.id)
-            updatedDeviceGroup2.Devices[1].should.have.property('id', device3.id)
+            // DeviceGroup.byId does not order its Devices
+            updatedDeviceGroup2.Devices.map(d => d.id).sort((a, b) => a - b).should.eql([device1.id, device3.id])
         })
 
         it('Owner can set the list of devices in existing group', async function () {

test/unit/forge/routes/auth/index_spec.js

@@ -1374,7 +1374,7 @@ describe('Accounts API', async function () {
 
         async function getLatestAuditEntry () {
             return app.db.models.AuditLog.findOne({
-                order: [['createdAt', 'DESC']]
+                order: [['id', 'DESC']]
             })
         }