Skip to content

Commit f1e21c4

Browse files
test(plugin-auth): the SCIM deactivation and last-admin-guard suites run under 10 s (two #23184 rows) (#23697)
Refs #23184 (plugin-auth rows: scim-deactivation-reconcile-user, last-admin-guard) Clause-②: no ## Rows answered Two rows of the card's `@objectstack/plugin-auth` list: - `src/scim-deactivation-reconcile-user.test.ts`, listed at 35.3 s (run `38267594064`, 14 of 14 window runs over budget); - `src/last-admin-guard.test.ts`, listed at 10.9 s (run `38222448090`). The pin is the next refresh PR's list: it should name neither file. The card stays open for its other rows: the other three `plugin-auth` rows (org-create-posture-gate, implicit-account-linking, dev-admin-seed-credential-gate) and the `plugin-security`, `service-datasource` and `organizations` rows. The diff is tests only: two test files and one new test-only helper. No production file, option, export or published byte changes. The helper's symbol has 0 hits in `packages/plugins/plugin-auth/dist` (the positive control `registerLastAdminGuard` has 4), and the package ships only `dist`, `README.md` and `CHANGELOG.md`. Every `it(`, `describe(` and `expect(` line in both files is byte-identical to the base and in the same order. ## Where the time went, and what changed ### `scim-deactivation-reconcile-user.test.ts`: 18.7 s to 2.2 s local A probe at the base (temporary, deleted) broke the file's wall down. Two real-time 5.5 s ban-expiry waits took 11.0 s. Production-parameter scrypt took 3.3 s over 29 derivations (10 hashes and 19 sign-in verifies). The schema sync took about 0.9 s (12 boots of 32 objects, about 80 ms each). Vendor module evaluation inside the first case took about 0.8 s. Requests took the rest. 1. **The two timed-ban faces move the clock instead of sleeping.** The expiry is judged by better-auth's admin plugin in its `session.create` hook as `banExpires` before `Date.now()`, which is the global clock. So `passTime(ms)` fakes `Date` alone (`vi.useFakeTimers({ toFake: ['Date'], shouldAdvanceTime: true })`) and sets it `TIMED_BAN_WAIT_MS` ahead. Timers stay real, the clock keeps running from there, and an `afterEach` restores the real clock. `TIMED_BAN_MS` and `TIMED_BAN_WAIT_MS` are unchanged. The pre-PATCH sign-in still runs on the real clock, so its race margin is the same as before. No production constant changes. 2. **The scrypt memo from the previous slice** (`src/__tests__/scrypt-memo.testkit.ts`) answers each sign-in verify: same password, same salt, same parameters. 3. **One password hash per file.** The same stored value is written into each case's own database. Every manager in the file is built with the same options and the vendor's default hasher, so only the random salt is shared, and no case reads it. 4. **Vendor modules load at module top** (the list the SCIM, organization and admin managers boot through), following the AGENTS.md rule that clocked windows measure behaviour, never loading. 5. **The engine comes from a per-file pool** (below). The `AuthManager` and the SCIM credential are still built fresh in every case. ### `last-admin-guard.test.ts`: 2.4 s to 1.6 s local This file has no scrypt and no waits. It ran 115 engine boots per run (init plus a schema sync), about 1.05 s of its wall. 90 of them are `boot()` with no options, and those now come from the pool. The 25 boots with options (`readThrough`, `maxScan`, `withIdentityWriteGuard`, `unguarded`) still boot fresh per call, because their hooks differ. ### The pool: `src/__tests__/sqlite-engine-pool.testkit.ts` (new, test-only) The pool keeps one booted sqlite `ObjectQL` engine per file. Before handing an engine out again, it deletes every row of every table in one transaction, then runs the suite's own `onReset` (`last-admin-guard` empties its catalog set there). `take()` hands out an engine no case holds, or boots another, so a case that takes two engines gets two. `release()` runs in `afterEach` and `destroy()` in `afterAll`. A raw row reset cannot undo some engine state, and the pool refuses those cases loudly: - an object declaring an `autonumber` field is refused at the first boot, because the engine keeps the counter in memory; - a database holding a `sys_organization` row is refused, because the engine memoizes the install's organization set until an engine write to that object invalidates it; - a schema a case created or dropped a table in is refused. ## Measurements Each figure is vitest's per-file duration (JSON reporter `endTime - startTime`): `pnpm exec vitest run --maxWorkers=2 FILE --reporter=verbose --reporter=json`, in `packages/plugins/plugin-auth`, under `os-verify-lock`. Runs were on a shared 4-vCPU box: another dev's `check:*` gates ran unlocked beside them, and the load average is given for each set. The before and after runs at the final head were **interleaved**, with the before leg being the base file copied to a temporary name (deleted afterwards), so both sides saw the same load. | file | before, base `c24598658d` alone (load 2.4) | before, interleaved at `3545b9c012` (load 2.5 to 2.9) | after, interleaved at `3545b9c012` | median after | |:--|:--|:--|:--|--:| | scim-deactivation-reconcile-user | 18884 / 18449 / 17830 ms | 18442 / 18689 / 19511 ms | 2179 / 2816 / 2118 ms | **2.18 s** | | last-admin-guard | 2540 / 2635 / 2542 ms | 2351 / 2150 / 2432 ms | 1453 / 1573 / 1660 ms | **1.57 s** | **CI projection.** The per-file CI captures of recent hourly runs are NOT MEASURED: `test-core-timings-N-of-6`, the timing-table artifact and the job logs are all served from `productionresultssa16.blob.core.windows.net`, which this container's egress proxy refuses (CONNECT 403). The check-run annotations carry no per-file line. The ratios below therefore come from the card's own readings: - scim: the 35.34 s worst run includes the two waits, 11.0 s of wall that does not scale with load. Its compute was 24.3 s in CI against 7.7 s locally, a ratio of 3.2. At that ratio the file projects to **7.0 s**. At 4.0, the worst post-landing ratio the previous slice read for a concurrent shape, it projects to 8.7 s. - last-admin-guard: 10.9 s against 2.35 s locally is a ratio of 4.6. At that ratio the file projects to **7.3 s**. ## Proof that every case still runs and still asserts the same - **Case lists:** the sorted fullName and status of every case match the base in every after run: 13 of 13 passed, and 110 of 110 passed. - **Static lines:** the `it(` / `describe(` / `expect(` lines are byte-identical to the base and in order. The counts are 13 / 6 / 83 for scim and 110 / 23 / 277 for last-admin-guard. - **Runtime assertions:** a temporary copy of each file recorded `expect.getState().assertionCalls` per case, at the base and at the head. The per-case counts are identical: 146 assertions over 13 cases, and 283 over 110 cases. No case asserts nothing. - **The moved clock reaches the vendor (mutation, restored):** `scripts/ablation-replace.mjs` removed the clock move (and its `Date.now()` line) from the positive control only. The control turned red at `expectSignInAccepted` with `403 BANNED_USER`, and the other 12 cases stayed green. The restore was verified (blob `0fbcf875db` equals HEAD, and `git diff HEAD` is empty). So the unmutated control is admitted only because the hook read the moved clock, which is the same evidence that makes the main face's refusal mean something. - **Pool equivalence (temporary probe, deleted):** the probe fingerprinted every engine `take()` handed out, covering per-table row counts, hook counts per event, middlewares, registered objects, the organization memo, the autonumber counters, the deviation, attestation and violation sets, both migration-flag memos, unavailable datasources and the catalog size. It compared each against a freshly booted engine. last-admin-guard made 90 takes (2 boots, 88 resets) and scim made 12 (1 boot, 11 resets). Every take matched the fresh fingerprint. - **Order independence:** both files ran with `--sequence.shuffle.tests` under seeds 11, 2026 and 23184. Every run passed 123 of 123 tests. - **The pool's refusals fire (temporary probe, deleted):** an organization row, a table created by a case and an autonumber field were each refused with the stated message. Reuse, two takes in one case, and a full emptying each behaved as described (5 of 5). ## Tests and gates (final head `3545b9c012`) - `pnpm --filter @objectstack/plugin-auth run typecheck`: VERDICT command-exit 0. `check:test-typecheck` is OK with the debt ledger unchanged (10 files / 88 errors / 23 signatures). `tsc --listFilesOnly`: the 3 changed files are in `tsconfig.test.json`. - The whole `@objectstack/plugin-auth` suite, all 146 files in 6 chunks with `--maxWorkers=2`: 146 of 146 files passed, with 2861 tests passed and 10 skipped (the skips predate this PR). Every chunk exited 0. - ESLint on the 3 changed files with `--no-inline-config`: exit 0. `check-nul-bytes`: exit 0, and a control-byte self-scan of the 3 files found 0 hits. - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 53 commands, the same 53 as the seat's lead. All 53 exit 0, as do the 4 roster families that sit under these paths (`check:auth-mount-ledger`, `check:authz-resolver`, `check:error-code-casing`, `check:filter-alias-parity`). `check:dual-build-cjs-loads` first answered PREREQUISITE NOT MET (exit 3) and exited 0 after a full `turbo run build` (75 of 75 tasks). `--ran` reports 53 derived, 53 run, 0 NOT-MEASURED and 0 UNRUN. - `check:engine-double-contract` printed its repo-wide census line verbatim: `UNRECOGNISED [engine-double-contract]: 22 construct(s) in packages, examples declare a scanned verb (delete, update, findOne) alongside engine siblings, and this gate could not read the implementation -- so they are in NEITHER the pinned population nor the ledger.` None of the 22 is in a file this PR touches. - The full lint farm, Test Core, the dogfood jobs and the type-check lanes belong to CI. ## Acceptance notes - Each of these files also spends about 15 to 20 s in vitest import and transform on this box, before and after this change. That time falls outside the per-file reading the budget grades. This is an observation, not a defect. - The per-file CI captures (`test-core-timings-N-of-6`) cannot be read from a cloud container (blob-storage egress is refused), so a seat measuring a test-budget row there reads only the card's numbers and the next refresh. This is an environment fact, not a repo defect. - The pool is opt-in per file and pools only one boot shape. Other suites in this package that boot a real sqlite engine per case could adopt it, each with its own equivalence check. This PR does not touch them. --- _Generated by [Claude Code](https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f77ba41 commit f1e21c4

3 files changed

Lines changed: 318 additions & 48 deletions

File tree

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Test scaffolding [#23184]: one booted sqlite `ObjectQL` engine per test file,
5+
* emptied before each case that takes it, instead of one boot per case.
6+
*
7+
* A suite in this package that builds a real engine over better-sqlite3
8+
* `:memory:` pays `init()` plus `syncSchemas()` (the DDL for every object it
9+
* registers) once per case: about 80 ms for the 32 objects a SCIM deployment
10+
* registers, and 6 to 10 ms for a small fixture set, so a file of 110 cases
11+
* spends most of a second booting. The engine a case receives from this pool
12+
* was built by the same `boot` function, so its registry, its hooks and its
13+
* driver are the ones a fresh boot gives; what the previous case left behind
14+
* is its ROWS, and those are deleted before the engine is handed out again.
15+
*
16+
* What is NOT reset, and why that is safe or refused:
17+
*
18+
* - the engine's in-memory autonumber counters: a pool whose objects declare
19+
* an `autonumber` field is refused when it first boots, since a counter
20+
* seeded by an earlier case would not restart where a fresh engine does;
21+
* - the engine's memo of the install's organizations, which only an engine
22+
* write to `sys_organization` invalidates: the rows are deleted below the
23+
* engine, so a database that holds an organization row is refused rather
24+
* than emptied;
25+
* - the schema itself: a case that created or dropped a table leaves an
26+
* engine a fresh boot would not give, and the next `take()` refuses it;
27+
* - the write epoch and the log-once sets: counters and logging only, read by
28+
* nothing these suites assert on.
29+
*
30+
* A case may hold several engines at once: `take()` hands out a pooled engine
31+
* no case holds, or boots another. Engines that were taken come back with
32+
* `release()` in the file's `afterEach`, and `destroy()` closes them all in
33+
* its `afterAll`. A suite that needs a differently-built engine (other hooks,
34+
* other objects) boots it itself, as before; only the one `boot` shape is
35+
* pooled.
36+
*
37+
* ```ts
38+
* const pool = sqliteEnginePool(async () => ({ engine, driver }));
39+
* afterEach(pool.release);
40+
* afterAll(pool.destroy);
41+
* const engine = await pool.take();
42+
* ```
43+
*/
44+
45+
import type { ObjectQL } from '@objectstack/objectql';
46+
import type { SqlDriver } from '@objectstack/driver-sql';
47+
48+
/** What the pool's `boot` returns: the engine, and the driver it registered as the default. */
49+
export interface BootedSqliteEngine<E extends ObjectQL = ObjectQL> {
50+
engine: E;
51+
driver: SqlDriver;
52+
}
53+
54+
export interface SqliteEnginePoolOptions<E extends ObjectQL> {
55+
/** Per-engine fixture state the suite keeps beside the database (a catalog set, say), reset with it. */
56+
onReset?: (engine: E) => void | Promise<void>;
57+
}
58+
59+
export interface SqliteEnginePool<E extends ObjectQL> {
60+
/** An emptied pooled engine no case holds, or a fresh boot when every pooled engine is held. */
61+
take(): Promise<E>;
62+
/** Hand back every engine taken since the last release; the file's `afterEach`. */
63+
release(): void;
64+
/** Destroy every engine the pool booted; the file's `afterAll`. */
65+
destroy(): Promise<void>;
66+
/** Boots run, and resets performed, since the pool was made. */
67+
counts(): { boots: number; resets: number };
68+
}
69+
70+
interface Entry<E extends ObjectQL> {
71+
engine: E;
72+
driver: SqlDriver;
73+
/** The tables the boot created, sorted. */
74+
tables: readonly string[];
75+
held: boolean;
76+
}
77+
78+
/** The engine's in-memory organization memo is keyed on this object's rows (see the header). */
79+
const ORGANIZATION_TABLE = 'sys_organization';
80+
81+
type Knex = ReturnType<SqlDriver['getKnex']>;
82+
83+
async function tableNames(knex: Knex): Promise<string[]> {
84+
const rows = (await knex('sqlite_master').where({ type: 'table' }).select('name')) as Array<{ name: string }>;
85+
return rows
86+
.map((row) => row.name)
87+
.filter((name) => !name.startsWith('sqlite_'))
88+
.sort();
89+
}
90+
91+
function autonumberFields(engine: ObjectQL): string[] {
92+
const objects = engine.registry.getAllObjects() as Array<{ name: string; fields?: Record<string, { type?: string }> }>;
93+
return objects.flatMap((object) =>
94+
Object.entries(object.fields ?? {})
95+
.filter(([, field]) => field?.type === 'autonumber')
96+
.map(([name]) => `${object.name}.${name}`),
97+
);
98+
}
99+
100+
/** Make a pool over `boot`; see the module header for what a reused engine does and does not carry. */
101+
export function sqliteEnginePool<E extends ObjectQL>(
102+
boot: () => Promise<BootedSqliteEngine<E>>,
103+
options: SqliteEnginePoolOptions<E> = {},
104+
): SqliteEnginePool<E> {
105+
const entries: Array<Entry<E>> = [];
106+
let boots = 0;
107+
let resets = 0;
108+
109+
async function bootEntry(): Promise<Entry<E>> {
110+
const { engine, driver } = await boot();
111+
boots += 1;
112+
const counted = autonumberFields(engine);
113+
if (counted.length > 0) {
114+
try {
115+
await engine.destroy();
116+
} catch {
117+
/* noop */
118+
}
119+
throw new Error(
120+
`sqliteEnginePool: ${counted.join(', ')} declare an autonumber field, whose counter the engine keeps in ` +
121+
'memory across a reset; boot that engine per case instead of pooling it.',
122+
);
123+
}
124+
const entry: Entry<E> = { engine, driver, tables: await tableNames(driver.getKnex()), held: true };
125+
entries.push(entry);
126+
return entry;
127+
}
128+
129+
async function empty(entry: Entry<E>): Promise<void> {
130+
const knex = entry.driver.getKnex();
131+
const tables = await tableNames(knex);
132+
if (tables.join('\n') !== entry.tables.join('\n')) {
133+
const added = tables.filter((t) => !entry.tables.includes(t));
134+
const dropped = entry.tables.filter((t) => !tables.includes(t));
135+
throw new Error(
136+
`sqliteEnginePool: a case changed this engine's schema (created: ${added.join(', ') || 'none'}; ` +
137+
`dropped: ${dropped.join(', ') || 'none'}), so it is no longer the engine a fresh boot gives. ` +
138+
'Boot that case its own engine.',
139+
);
140+
}
141+
if (tables.includes(ORGANIZATION_TABLE)) {
142+
const [row] = (await knex(ORGANIZATION_TABLE).count({ n: '*' })) as Array<{ n: number | string }>;
143+
if (Number(row?.n ?? 0) > 0) {
144+
throw new Error(
145+
`sqliteEnginePool: a case left ${row?.n} ${ORGANIZATION_TABLE} row(s), and the engine memoizes the ` +
146+
'organization set until an engine write to that object invalidates it; deleting them below the ' +
147+
'engine would hand the next case a stale memo. Boot that case its own engine.',
148+
);
149+
}
150+
}
151+
await knex.transaction(async (trx) => {
152+
// Checked at commit, when every table is empty: the delete order does not matter.
153+
await trx.raw('PRAGMA defer_foreign_keys = ON');
154+
for (const table of tables) await trx(table).del();
155+
const [sequence] = (await trx('sqlite_master').where({ type: 'table', name: 'sqlite_sequence' })) as unknown[];
156+
if (sequence) await trx('sqlite_sequence').del();
157+
});
158+
await options.onReset?.(entry.engine);
159+
resets += 1;
160+
}
161+
162+
return {
163+
async take() {
164+
const free = entries.find((entry) => !entry.held);
165+
if (!free) return (await bootEntry()).engine;
166+
free.held = true;
167+
await empty(free);
168+
return free.engine;
169+
},
170+
release() {
171+
for (const entry of entries) entry.held = false;
172+
},
173+
async destroy() {
174+
while (entries.length) {
175+
const entry = entries.pop()!;
176+
try {
177+
await entry.engine.destroy();
178+
} catch {
179+
/* noop */
180+
}
181+
}
182+
},
183+
counts: () => ({ boots, resets }),
184+
};
185+
}

‎packages/plugins/plugin-auth/src/last-admin-guard.test.ts‎

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
* file re-run it on the same fixtures rather than describing it.
5252
*/
5353

54-
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
54+
import { describe, it, expect, beforeEach, afterEach, afterAll } from 'vitest';
5555
import { ObjectQL } from '@objectstack/objectql';
5656
import { SqlDriver } from '@objectstack/driver-sql';
5757
import { isAPIError } from 'better-auth/api';
@@ -63,6 +63,7 @@ import { SYS_USER_PROFILE_EDIT_FIELDS } from './sys-user-writable-fields.js';
6363
import { createObjectQLAdapterFactory } from './objectql-adapter.js';
6464
import { buildAdminPluginSchema, buildOrganizationPluginSchema } from './auth-schema-config.js';
6565
import { bindTestSecurityCatalog } from './__tests__/security-catalog.testkit.js';
66+
import { sqliteEnginePool, type BootedSqliteEngine } from './__tests__/sqlite-engine-pool.testkit.js';
6667
import { admin } from 'better-auth/plugins/admin';
6768
import { organization } from 'better-auth/plugins/organization';
6869

@@ -246,6 +247,7 @@ afterEach(async () => {
246247
else process.env[POSTURE_ENV] = ambientPosture;
247248
if (ambientMultiOrg === undefined) delete process.env[MULTI_ORG_ENV];
248249
else process.env[MULTI_ORG_ENV] = ambientMultiOrg;
250+
defaultEngines.release();
249251
// `:memory:` dies with its connection; closing keeps one live database per
250252
// test from piling up in a file this size.
251253
const open = engines;
@@ -255,17 +257,34 @@ afterEach(async () => {
255257
}
256258
});
257259

260+
/**
261+
* [#23184] Every `boot()` with no options — the engine most cases take, often
262+
* twice in one case — comes from a per-file pool instead of a fresh boot: the
263+
* same six objects, the same guard registration, its rows deleted and its
264+
* catalog emptied before a case receives it (`sqlite-engine-pool.testkit.ts`
265+
* says what a reused engine does and does not carry). 110 cases spent about a
266+
* second booting engines they never told apart. Any other option set boots a
267+
* fresh engine per call, as before, because its hooks differ.
268+
*/
269+
const defaultEngines = sqliteEnginePool(() => bootEngine({}), {
270+
onReset: (engine) => catalogOf(engine).clear(),
271+
});
272+
afterAll(defaultEngines.destroy);
273+
258274
async function boot(opts: BootOptions = {}): Promise<ObjectQL> {
275+
if (Object.keys(opts).length === 0) return defaultEngines.take();
276+
return (await bootEngine(opts, engines)).engine;
277+
}
278+
279+
async function bootEngine(opts: BootOptions, track?: ObjectQL[]): Promise<BootedSqliteEngine> {
259280
const engine = new ObjectQL();
260-
engines.push(engine);
261-
engine.registerDriver(
262-
new SqlDriver({
263-
client: 'better-sqlite3',
264-
connection: { filename: ':memory:' },
265-
useNullAsDefault: true,
266-
}),
267-
true,
268-
);
281+
track?.push(engine);
282+
const driver = new SqlDriver({
283+
client: 'better-sqlite3',
284+
connection: { filename: ':memory:' },
285+
useNullAsDefault: true,
286+
});
287+
engine.registerDriver(driver, true);
269288
await engine.init();
270289
for (const o of [sysUser, sysMember, sysPermissionSet, sysUserPermissionSet, sysAccount, sysMetadataActivation]) {
271290
engine.registry.registerObject(o as never);
@@ -288,7 +307,7 @@ async function boot(opts: BootOptions = {}): Promise<ObjectQL> {
288307
...(opts.maxScan !== undefined ? { maxScan: opts.maxScan } : {}),
289308
});
290309
}
291-
return engine;
310+
return { engine, driver };
292311
}
293312

294313
/** A user row, plus whatever standing the case needs. */

0 commit comments

Comments
 (0)