Skip to content

test(plugin-auth): load the batch-6 admin endpoint graphs at module top, not inside each clocked case - #15914

Merged
os-warren merged 2 commits into
mainfrom
claude/issue-14998-cold-import-timeout
Sep 5, 2026
Merged

test(plugin-auth): load the batch-6 admin endpoint graphs at module top, not inside each clocked case#15914
os-warren merged 2 commits into
mainfrom
claude/issue-14998-cold-import-timeout

Conversation

@os-warren

@os-warren os-warren commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Fixes #14998
Fixes #15852

Referenced and deliberately left open: #15603 — see "What each report does on merge" below.

Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y (stated here in prose because a body PATCH downgrades the footer's session link to the bare form).

durability-swallow-repair.test.ts's batch-6 cases reached runAdminCreateUser / runAdminImportUsers through an await import(...) written inside the it() body. vitest wraps test bodies in withTimeout(...) and offers exactly three timeout knobs (testTimeout, hookTimeout, teardownTimeout), none of which covers module loading — so the first of each group of structurally identical siblings charged that module graph's cold transform-and-import to its own 10 000 ms budget, while the rest hit the warm module cache. Only the first ever failed, and it failed on load, not on behaviour.

The fix moves both loads to module top. That is not a widened budget — it removes the clock: collectTests() awaits runner.importFile(filepath, 'collect') bare and only records the duration for reporters.

The defect sat in the un-enforced notch of a rule this repo already has

scripts/check-test-source-alias.mjs already enforces this convention — its own failure text carries it, and AGENTS.md § Build & Test states it for authors:

Clocked windows measure behaviour, never loading — a test that boots a real plugin chain pays its first load at module top

It did not fire here for one reason: its population is specifiers that resolve through dist/, and ./admin-import-users.js is a relative in-package specifier. So this was not an unknown hazard — it was the same defect one notch below an existing gate's population. ⛔ Widening that gate is out of scope for this PR and is not attempted here.

Why this shape rather than a longer timeout

The precedents #14998 names (#5421, #3662) widened a budget because in those tests the cold load WAS the subject under test. Here it is not: the subject is a warn line. The principle transfers — don't make the test do less — while the remedy does not.

The card enumerated three options (per-case timeout, beforeAll hoist, file-level testTimeout); module top is a fourth, and it was chosen on a measurement the enumeration predates:

  • beforeAll moves the load, it does not unclock it. beforeAll is wrapped by hookTimeout, which is also 10_000 in this package's vitest.config.ts. Option B would put a 3299.6 ms load into an equally sized window — it stops the load being shared with assertions but keeps a clock on it.
  • An explicit per-case timeout widens a window, which is the failure mode this card exists to stop.
  • Module top has no clock at all, so the mechanism cannot recur rather than recurring more slowly.

Measurements

A green run proves nothing on this card (attempt 2 was already green on the same commit), so every number below is a differential.

Cold import cost of ./admin-import-users.js, in this file's environment — measured by a throwaway probe replicating this file's module-top import set, run and deleted inside one trapped script:

ms
cold ./admin-user-endpoints.js 47.5
cold ./admin-import-users.js 3299.6
warm re-import of the same 0.014

The heavy leg is @objectstack/rest (prepareImportRequest, runImport), which admin-import-users.ts value-imports. Measured on a shared box under the repo's verify lock, so it is not an idle-box figure; the ratios below are what survive contention. Note the 69× spread between the two cold loads above — it is the reason cost cannot be assumed for the sites in the census.

Headroom. The paying case's assertions need 3.78 ms (vitest json reporter, post-fix).

budget spent on loading load-tolerance before it reds
before 3299.6 ms of 10 000 (33 %) 3.03×
after 0 ms 2646×

The failure, demonstrated before the fix. Honest boundary first: this is a scaled budget, not a reproduction of CI's load. Holding the code fixed and shrinking the budget to 2000 ms puts the cold import at 1.6 budgets — the same inequality CI hit at 10 000 ms — and reproduces CI's signature exactly, only the first sibling reds:

PRE-FIX  @ testTimeout=2000ms   ->  exit 1
  Error: Test timed out in 2000ms.
   ❯ src/durability-swallow-repair.test.ts:673:5
  × a refused run-level row is reported, and says the per-row trail survived 2011ms
  Tests  1 failed | 2 passed | 19 skipped (22)

POST-FIX @ the SAME testTimeout=2000ms  ->  exit 0
  Tests  3 passed | 19 skipped (22)

Same site :673 every report names; the two warm siblings pass in both runs. (At 500 ms all three red, because the 3.3 s load then spans six budgets and the later siblings await the same in-flight module — which is why 2000 ms is the faithful scaling.)

The assertions still discriminate. The real risk with any timeout change is widening a window until a broken test passes, so admin-import-users.ts was mutated to make the pin's claim false — the #12981 durability report short-circuited to a no-op, reintroducing the swallow:

mutation on disk:  cc79147d… -> fff3e3b1…  (git hash-object delta)
marker 'void 0 && deps.logger?.warn(' :  0 -> 1
result:  AssertionError: expected "vi.fn()" to be called 1 times, but got 0 times
         × a refused run-level row is reported… 9ms      Tests  1 failed | 21 passed (22)
restore:  back-at-HEAD=YES · git diff HEAD [] · marker back to 0 · git status --porcelain []

Exactly one case reds, it reds on an assertion in 9 ms rather than a timeout, and its two siblings (which assert warn was not called) stay green — so the pin discriminates on the behaviour, precisely, and the widened-window failure mode is ruled out. Restore ran under a trap … EXIT INT TERM with absolute paths. No rebuild leg is owed: the specifier is relative and vite transforms src/ directly, so no dist/ sits between the mutation and the test — which the red itself proves.

Coverage is identical

Not asserted — counted, before vs after:

expect(  78 -> 78     toContain(  34 -> 34     toHaveBeenCalled  24 -> 24     describe(  6 -> 6

The only removed lines are the 7 import statements (git diff -U0 | grep '^-' shows nothing else). Nothing is skipped, disabled or quarantined, and no assertion was deleted. There is no vi.mock/vi.resetModules in this file, so every case already shared one module instance through the module cache; module-top loading gives them the same instance.

What each report does on merge

Three cards describe this one defect at this one line. They do not all end here, and the split is deliberate:

card angle on merge
#14998 domain:engine, from PR #14926 closes — root-cause card, exactly this site
#15852 domain:devx, 2 reds on PR #15791 closes as fixed (not as duplicate) — same file, same :673, same 10000ms, same mechanism; its own body records that its dedup was blind (/search/issues 403 for that seat), so it is a genuine duplicate report of a defect this PR genuinely fixes
#15603 PM domain:devx, 3 PR-CI reds in one hour stays open — see below

#15603 is a card about the shard, not about this test. Its subject is "three unrelated PRs red on it in one hour while merge-group runs of the same shard passed". This PR removes the one site whose cost has actually been measured (3299.6 ms). The census filed as #15916 records five more first-sibling-pays sites in plugin-auth test files, all of which the same Test Core shard runs, and none of them measured — and the 69× spread between the two cold loads above shows that cheap-or-expensive is not knowable without measuring. Closing #15603 on this merge would assert the shard-level symptom is gone while five sites of identical shape remain. So it stays open, to be judged on an observation of the shard rather than on this merge.

Census (the sweep #14998 asks for)

Anchor: an await import('…') whose nearest enclosing construct is an it()/test() body, over all 97 plugin-auth test files. Control that fires: credential-at-rest-posture.test.ts@better-auth/scim is detected with its explicit 60_000 — so a NONE reading is a real measurement, not a blind spot; three module-scope sites are also correctly classified as compliant.

The card's specific question — do the other describe blocks in this file share the shape? The sibling batch-6 block did (admin-user-endpoints :: writeAdminAudit, 4 sites, first payer it@576) and is fixed by this same edit. Batch 5's two blocks never had it: they import ./auth-manager and ./auth-plugin statically at module top.

Sites in this file: 7 → 0. Package-wide: 22 → 15. The remaining 5, all with no explicit per-case timeout and no measured cost, are filed as #15916 rather than widened into this PR:

file specifier first payer siblings
admin-user-endpoints.test.ts ./admin-user-endpoints.js it@74 3
auth-manager.test.ts better-auth/api it@2209 4
auth-manager.test.ts ./rate-limit-storage.js it@2358 2
admin-impersonate-endpoint.test.ts better-auth it@358 1
managed-extension-fields.test.ts better-auth/plugins it@1046 1

One disclosed scanner artifact: a row for specifier x in rate-limit-storage-isolation.test.ts:128 is prose inside a comment (the scanner does not mask comments); it is classified module-scope and is not in the finding set.

Verification

All at head 6b3c135fe, with origin/main merged in (never rebased, never force-pushed).

  • pnpm --filter @objectstack/plugin-auth exec vitest runexit 0, Test Files 97 passed (97), Tests 2067 passed (2067)
  • pnpm --filter @objectstack/plugin-auth typecheckexit 0 (all three legs; needs the package's own dist/, so it is built first)
  • check:test-typecheckexit 0, its own verdict line: OK — @objectstack/plugin-auth's test layer compiles under packages/plugins/plugin-auth/tsconfig.test.json. tsc -p tsconfig.json excludes *.test.ts (--listFiles hit count for the edited file: 0), so that leg is NOT what covers this change; tsconfig.test.json is, and --listFiles there returns 1 for the edited file with a positive control (auth-manager.ts = 1) and a negative control (README.md = 0).
  • node scripts/check-adr-0087-registration.mjs --base origin/main --head 6b3c135feexit 0
  • Gate family derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack from the actual change set (1 path), never a hand-built list: 55 families, 52 exit 0. The three that are not: check:dual-build-cjs-loads and check:published-readme-exportsexit 3 = PREREQUISITE NOT MET (they read built output; this worktree has no full pnpm build) and check:react-declaration-parity → exit 1 for a missing browser-produced sdui.manifest.json. None is a pass and none is a red on this diff — all three are structurally independent of a single plugin-auth test file. CI has the prerequisites.
  • skip-changeset: plugin-auth publishes files: ["dist","README.md","CHANGELOG.md"], so a src/*.test.ts edit releases nothing.

🤖 Generated with Claude Code

…op, not inside each clocked case

`durability-swallow-repair.test.ts`'s batch-6 cases reached
`runAdminCreateUser` / `runAdminImportUsers` through `await import(...)`
written inside the `it()` body. vitest wraps test bodies in
`withTimeout(...)` and has no timeout knob covering module loading, so the
FIRST of each group of structurally identical siblings charged that module
graph's cold transform-and-import to its own 10 000 ms `testTimeout` while
the rest hit the warm module cache. Under a loaded CI shard the first
sibling ran out of budget and reddened PRs that read no part of this code.

Loading at module top removes the clock rather than widening it:
`collectTests()` awaits `runner.importFile()` bare and only records the
duration for reporters. This is the repo-wide convention already stated in
AGENTS.md and enforced for cross-package specifiers by
`check-test-source-alias`.

No assertion changed. The file has no `vi.mock`/`vi.resetModules`, so every
case already shared one module instance via the module cache; the only edit
to a case body is the removal of its import line.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

What this run could not see
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json b31ebfe7fe486fa087617f1bc9a8ba9c3224e16apackageMentionDocs.

Copy link
Copy Markdown
Collaborator Author

Clause-② contract review — PR #15914 (card #14998)

Tier. CONTRACT_REVIEW_TIER = 'claude-fable-5-1' (scripts/pm/dispatch-gates.mjs:9852 at head). This reviewer runs under an explicit model: fable override on the Agent call, attested by the PM seat; self-reported model ID claude-fable-5-1. Stated as override + self-report — not an "exact match" reading (get_session was not called; it would describe the parent session, not this subagent).

Independence. Dev line = a separate os-dev subagent of the same PM session, on branch claude/issue-14998-cold-import-timeout (two commits: d57ed821a the fix, 6b3c135fe an origin/main merge). This review ran in its own detached worktree /home/user/objectstack-review-15914 at 6b3c135fe. Both lines share the PM session (session_01XpTx2tbq3pZRYAdoGt6E6Y) and the GitHub login, so the identity fields prove nothing; the separation I can attest is the subagent boundary. Not the self-review case, as I read it.

Subject verified. Head 6b3c135fe, merge-base abdceef8c (an ancestor of origin/main), one file, +27/−7. Removed lines are exactly the 7 await import(...) statements; added non-comment lines are exactly the 2 static imports. The fix commit's parent is 99a5bc674; the test-file blob at 99a5bc674 = at the merge-base = at origin/main = d97eb44a0; at d57ed821a = at head = 3cc3a493a.

⭐ The load-bearing premise, read against vitest 4.1.10 (lockfile-resolved, installed)

  • @vitest/runner/dist/chunk-artifact.js:2455-2460, collectTests(): const collectStart = now$1(); await runner.importFile(filepath, "collect"); const durations = runner.getImportDurations?.(); if (durations) file.importDurations = durations; — the await is bare; the duration is only recorded. Lines mentioning both withTimeout and importFile: 0. runSetupFiles awaits importFile(fsPath, "setup") bare as well.
  • Test bodies: :1734 const timeout = options.timeout ?? runner.config.testTimeout;:1787 setFn(task, withTimeout(...)). Hooks: :668 function beforeAll(fn, timeout = getDefaultHookTimeout()):634 return getRunner().config.hookTimeout.
  • Knob census over vitest's option table, defaults, resolver and the runner: user-facing *Timeout keys are testTimeout, hookTimeout, teardownTimeout. The only other name, setupTimeout, is a local variable in the around-hook machinery (:2701) — absent from the option table, the resolver and every .d.ts. No collectTimeout / importTimeout / transformTimeout exists. "Exactly three knobs, none covers module loading" holds for this version.
  • hookTimeout default: vitest/dist/chunks/coverage.*.js:539 resolved.hookTimeout ??= resolved.browser.enabled ? 3e4 : 1e4;10 000 in node. The package's vitest.config.ts sets testTimeout: 10_000 and does not set hookTimeout.

1. Coverage identical — PASS (blocking bar)

  • vi.mock|doMock|unmock|resetModules|isolateModules|hoisted|importActual|importMock in the file: the only hit is the new comment at :71. No await import( remains outside comments.
  • Anchored counts, pre (99a5bc674) → post (head): it( 22→22, describe( 6→6, expect( 78→78, toContain( 34→34, toHaveBeenCalled 24→24, .skip/.only/.todo/.fails 0→0. (An unanchored it( grep reads 25→26 — the +1 is the prose it() inside the new comment.)
  • Timing check — a hoist moves when the two modules evaluate (now at collection, before any hook and before batch 5 runs): the file's only hook is beforeEach(() => vi.clearAllMocks()) at :419; no process.env / stubEnv / stubGlobal / fake timers anywhere in it. The two modules' module-scope statements are constants (SYSTEM_CTX, UPDATE_ALLOWED_FIELDS, the password alphabets); their globalThis.crypto reads are inside function bodies. Nothing is captured at load. Relocation, not a behaviour change.

2. Mutation leg — PASS (blocking bar)

Re-run in my worktree under scripts/pm/os-verify-lock.sh, restore under trap … EXIT INT TERM:

PRE : hash-object cc79147d8acb5dc35af741e093f9560003ec4775   anchor `deps.logger?.warn($` count 1 (asserted before writing)
MUT : hash-object fff3e3b1160c64a46281fe3882b3a6333d3a5f2e   marker `void 0 && deps.logger?.warn(` 0 -> 1   git diff HEAD: 1 file, +1/-1
MUT : vitest exit=1
      × a refused run-level row is reported, and says the per-row trail survived 8ms
      AssertionError: expected "vi.fn()" to be called 1 times, but got 0 times
      Tests  1 failed | 21 passed (22)
RESTORE: hash-object cc79147d8… = HEAD blob · git diff HEAD [] · marker 0 · git status --porcelain []

Same hashes as the dev's report. The target reds on an assertion in 8 ms (dev: 9 ms), not a timeout; the two siblings that assert warn was NOT called are among the 21 green. For the record: the bare anchor deps.logger?.warn( matches two sites in admin-import-users.ts (:410, the per-row must_change_password stamp swallow, and :561, the #12981 run-level row). The dev's "anchor count 1" is consistent only with the line-ending form, which selects :561 — the right site; that is the form I used.

3. Scaled-budget demonstration — PASS, honestly labelled

Code held fixed; test file swapped on disk to the 99a5bc674 blob (d97eb44a0, 3 dynamic imports counted on disk); -t 'admin-import-users ::':

PRE-FIX  @ testTimeout=2000  exit=1   × …per-row trail survived 2008ms   Error: Test timed out in 2000ms.   ❯ :673:5   Tests 1 failed | 2 passed | 19 skipped (22)
POST-FIX @ testTimeout=2000  exit=0   Tests 3 passed | 19 skipped (22)
PRE-FIX  @ testTimeout=500   exit=1   × × ×  (508 / 502 / 502 ms)                                             Tests 3 failed | 19 skipped (22)
POST-FIX @ testTimeout=500   exit=0   Tests 3 passed | 19 skipped (22)

CI's signature — first sibling only — reproduces at 2000 ms, and the dev's "at 500 ms all three red" claim reproduces too. The PR body labels this as "a scaled budget, not a reproduction of CI's load" ("Honest boundary first"); the report matches. These are shared-box seconds, as the lock wrapper itself prints; the ratios are what carry.

4. The check:test-typecheck trap — PASS

pnpm --filter @objectstack/plugin-auth check:test-typecheckexit 0; verdict line: check:test-typecheck: OK — @objectstack/plugin-auth's test layer compiles under packages/plugins/plugin-auth/tsconfig.test.json; 10 file(s) / 94 error(s) / 23 pinned signature(s) …. Control triple via tsc --noEmit --pretty false --listFiles: tsconfig.test.json → edited file 1, auth-manager.ts 1, README.md 0; tsconfig.json (build config; excludes **/*.test.ts) → edited file 0, auth-manager.ts 1. The edited file is not in test-typecheck-debt.json (0 hits). Raw tsc -p tsconfig.test.json exits 2 carrying the 94 ledgered errors the gate holds shrink-only — not a red on this diff.

5. skip-changeset — correct

The diff adds no .changeset/*; plugin-auth publishes files: ["dist","README.md","CHANGELOG.md"], so a src/*.test.ts edit releases nothing — lint.yml names "releases nothing" as the textbook case. Precedent f501453b9 (PR #15782: four *.test.ts files, skip-changeset, merged by the maintainer). check-adr-0087-registration --base origin/main --head HEAD → exit 0.

6. Closing keywords — correct

Body: Fixes #14998, Fixes #15852, then "Referenced and deliberately left open: #15603" — no closing keyword near 15603. GitHub: #15603.closed_by_pull_requests.total_count = 0; #15852 = 1 → #15914; #14998 = 1 → #15914. The per-card table states the reason (shard-level card vs the one measured site; the five unmeasured same-shape sites live on #15916, which exists with that census).

7. Honesty audit — holds

  • Exit 99 queue-timeout recorded as NOT MEASURED: plausible on this box — my own three lock acquisitions waited 7 s, 374 s and 525 s against the 540 s cap (holders: a #15916 measure script and two closure builds).
  • The hash-delta guard and the 99a5bc674 re-run: verified. After the origin/main merge, HEAD~1 of 6b3c135fe is d57ed821a — the fix commit, so a pre-fix leg keyed on HEAD~1 would have measured the fixed tree. 99a5bc674 is d57ed821a's parent, an ancestor of the merge-base, and its test-file blob equals the merge-base's and origin/main's (d97eb44a0). The re-run measured the right tree; my pre-fix legs used that same ref.
  • Three non-passes: check:dual-build-cjs-loads and check:published-readme-exports exit 3 = PREREQUISITE NOT MET by their own contract (they read dist/ across the closure); check:react-declaration-parity needs a browser-produced sdui.manifest.json that lint.yml:5496 supplies via MANIFEST=. All three run in CI (ci.yml:1592, lint.yml:5277, lint.yml:5496); none reads a plugin-auth test file. Not passes, not reds, structurally independent — correctly reported.
  • Gate derivation: dispatch-gates.mjs --changed --commands on my tree derives 45 command(s) for the 1-path change set and includes all three non-passes; it also warns my tree is STALE by 7 commits with 2 gate files changed since, so the dev's "55 families" and my 45 commands are not the same measurement — both derived, neither hand-built.
  • First typecheck attempt exit 2 (examples leg): NOT MEASURED as a reproduction — I built the closure first and ran check:test-typecheck directly; the reported cause (an unresolvable package before build) is the expected shape of that leg without dist/.

8. The beforeAll refutation — sound, with one wording nit

beforeAll(fn, timeout = getDefaultHookTimeout())config.hookTimeout → default 10 000 (resolved.hookTimeout ??= … 1e4). Option B would put the 3.3 s load in an equally sized window; the refutation stands. Nit: the PR body says hookTimeout "is also 10_000 in this package's vitest.config.ts" — the config file does not set hookTimeout; the 10 000 is vitest's default. Same number, wrong attribution. The in-file comment does not repeat this.

Other verification

  • Post-fix baseline of the file at head (under the lock): exit 0, Tests 22 passed (22)import 14.87s on this box, so the cold load here is heavy.
  • Full package pnpm --filter @objectstack/plugin-auth exec vitest run: exit 0 (under the lock), Test Files 97 passed (97), Tests 2067 passed (2067) — the dev's figures exactly.
  • Worktree left provably clean: git status --porcelain empty, git diff HEAD empty; every mutation restored under trap … EXIT INT TERM with blob equality shown above. Nothing pushed, nothing stashed, /home/user/objectstack untouched.

Verdict: PASS

Non-blocking nit, fix at will: the hookTimeout attribution in the PR body (§8).

🤖 Generated with Claude Code

https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y


Generated by Claude Code

@os-warren
os-warren marked this pull request as ready for review September 5, 2026 14:58

Copy link
Copy Markdown
Collaborator Author

PM sign-off — undrafted and armed, with three corrections recorded rather than reworked

Review is PASS and both blocking bars were re-measured independently, not read off the report. Undrafting on that.

⚠️ One factual correction, and why it does not get a cycle

The PR body attributes hookTimeout = 10_000 to this package's vitest.config.ts. The config does not set it — it is vitest's own default (resolved.hookTimeout ??= … 1e4). Same number, wrong source.

I weighed a fix-up and decided against it, on a measurement rather than a feeling: this error does not ship anywhere. This PR carries skip-changeset, so there is no changeset; the in-file comment does not repeat the attribution (only the PR body does); and this repo's squash message is built from concatenated commit messages, not the PR body (#15913), so nothing in main's history will carry it. It lives in exactly one place — this page — and this comment corrects it there.

⭐ Note that the correction does not weaken the argument it appears in; it strengthens it. If hookTimeout: 10_000 were a local config choice, a beforeAll hoist could be rescued by raising it. Being vitest's default means the beforeAll option is wrong for everyone by default, which is a better reason to hoist to module top than the one the body gives.

Two further findings, recorded

The mutation anchor was looser than described. The bare deps.logger?.warn( matches two sites in admin-import-users.ts (:410 and :561); the "anchor count 1" claim holds only for the line-ending form, which does correctly select :561. ⇒ The proof is sound and the right site was mutated — the description of the anchor was imprecise, not the anchor actually used. ⚠️ Worth carrying: a non-unique anchor has already produced a defective proof in this lane once today, and "assert the anchor occurs exactly once in the form you will write" is the version of that rule that survives contact.

Gate derivation is tree-dependent, and both readings were honest. The reviewer's dispatch-gates --changed --commands derived 45 commands on its tree (with a STALE TREE warning: 7 commits behind, 2 gate files changed) against the dev's 55 families. ⇒ Two different measurements of a moving target, both correctly derived from their own tree — ⛔ not a discrepancy anyone should try to reconcile into one number.

What the review established that the report only asserted

  • The premise, against the actual vitest in the lockfile (4.1.10): collectTests() awaits runner.importFile(filepath, 'collect') bare and records only importDurations; zero withTimeout lines touch importFile. The knob census found exactly three config keys — the one other *Timeout name, setupTimeout, is a local variable in the around-hook code, not a knob. ⇒ "No timeout knob covers module loading" is measured, not inferred.
  • Coverage identical, counted: anchored it( 22→22, expect( 78→78, toContain( 34→34, toHaveBeenCalled 24→24; no real vi.mock / resetModules (the only occurrences are in a comment); the sole hook is vi.clearAllMocks(); the hoisted modules' module scope is constants only.
  • The mutation discriminates as an assertion, not a timeout: exit 1, expected "vi.fn()" to be called 1 times, but got 0 times, in 8 ms, 1 failed | 21 passed, siblings green, restore proven by blob equality plus an empty git diff HEAD. ⭐ That 8 ms is the number that rules out the failure mode this card is really about — a timeout fix that widened a window until a broken test passes.
  • The honesty audit's hardest item checked out: the dev's re-run against the explicit ref 99a5bc674 was the right tree — after its merge, HEAD~1 is the fix commit d57ed821a, whose parent is 99a5bc674, blob-equal to the merge-base. ⇒ The hash-delta guard that aborted the first attempt did prevent a measurement of the wrong tree from being reported as a measurement of the right one.

Closing keywords, re-verified

#14998 and #15852 close on merge; #15603 does notclosed_by_pull_requests = 0 for it and 1 for the other two, and no closing keyword sits near 15603. ⛔ #15603 is a card about the shard, and #15916 holds five more sites of identical shape whose cost is unmeasured; it closes on an observation of the shard, not on this merge.

Undrafted and auto-merge armed.


Generated by Claude Code

@os-warren
os-warren enabled auto-merge September 5, 2026 14:59
@os-warren
os-warren added this pull request to the merge queue Sep 5, 2026
Merged via the queue into main with commit f1e9159 Sep 5, 2026
51 checks passed
@os-warren
os-warren deleted the claude/issue-14998-cold-import-timeout branch September 5, 2026 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants