Skip to content

[finding] plugin-auth: 5 more first-sibling-pays dynamic imports charged to a case's own testTimeout — census attached, same shape as the durability-swallow-repair flake #15916

Closed

Description

@os-warren

Filed by the os-dev seat while implementing #14998, session session_01XpTx2tbq3pZRYAdoGt6E6Y. Filed bare — type, priority and lane are triage's. Recording only, no severity asserted.

#14998 explicitly asks the implementer to check "whether the other two describe blocks in the same file have the same first-sibling-pays shape, and whether other plugin-auth test files do", and to file the rest as one card rather than widening that PR. This is that card. PR #15914 fixes the sites in durability-swallow-repair.test.ts only.

The shape

An await import('...') written inside an it() body charges that module graph's cold transform-and-import to the case's own testTimeout. vitest wraps test bodies in withTimeout(...) and offers exactly three timeout knobs (testTimeout, hookTimeout, teardownTimeout), none of which covers module loading — so of N structurally identical siblings only the FIRST pays it and the rest hit the warm module cache. Only the first can flake, and it flakes on load rather than on behaviour, which is why re-running turns it green with no code change.

Measured cost of one such load in this package (PR #15914, on a shared box under the repo verify lock): ./admin-import-users.js cold 3299.6 ms versus 0.014 ms warm, against a 10 000 ms testTimeout. On CI shards whose vitest import phase reads 338–500 s, that single load has exceeded 10 s on its own at least six times across three separate cards (#14998, #15603, #15852).

The convention this violates is already stated repo-wide in AGENTS.md, section Build and Test: "Clocked windows measure behaviour, never loading — a test that boots a real plugin chain pays its first load at module top." scripts/check-test-source-alias.mjs enforces it, but only for specifiers that resolve through dist/; every row below is either a relative in-package specifier or a node_modules package, so all of them sit just outside that gate's population.

Census

Anchor: an await import('...') whose nearest enclosing construct is an it()/test() body, brace-tracked, over all 97 plugin-auth test files. Measured at 6b3c135fe after PR #15914's fix, so durability-swallow-repair.test.ts no longer appears (it was 7 of the 22 sites; 15 remain).

Control that fires — the scanner detects an explicit per-case timeout when there is one, so a NONE reading is a measurement rather than a blind spot:

file specifier first payer explicit timeout
credential-at-rest-posture.test.ts @better-auth/scim it@336 60_000

Three further sites are correctly classified as already compliant (module-scope loads, not in a clocked window): auth-email-locale.test.ts:89, verification-email-failure-propagation.test.ts:54, and rate-limit-storage-isolation.test.ts:128.

The finding set — first payer is an it() with no explicit per-case timeout:

file specifier first payer import line siblings sharing the specifier
admin-user-endpoints.test.ts ./admin-user-endpoints.js it@74 :75 3
auth-manager.test.ts better-auth/api it@2209 :2215 4
auth-manager.test.ts ./rate-limit-storage.js it@2358 :2359 2
admin-impersonate-endpoint.test.ts better-auth it@358 :363 1
managed-extension-fields.test.ts better-auth/plugins it@1046 :1059 1

The two auth-manager.test.ts rows are the ones worth grading first: 4 and 2 siblings respectively, and better-auth/api is a node_modules graph comparable in weight to the one that has already flaked six times.

Costs are not measured per row — only the shape is. A row with one site has no warm sibling to contrast against, so it is the shape without the observed signature; whether it is worth touching is triage's call.

Disclosed scanner limitation: the scanner does not mask comment text, so a literal await import('x') appearing in prose is picked up. Exactly one such artifact exists here — specifier x at rate-limit-storage-isolation.test.ts:128, inside a comment documenting a regex. It lands in the module-scope bucket and is not in the finding set above. Nothing else in the 15 is a comment.

Suggested direction, not prescribed

PR #15914 hoisted its loads to module top rather than to a beforeAll, because beforeAll is itself clocked by hookTimeout (also 10 000 ms here) — it stops the load being shared with assertions but keeps it inside a clock. Module top has no clock at all: collectTests() awaits runner.importFile(filepath, 'collect') bare and only records the duration for reporters. Whether each row above wants that, an explicit per-case timeout like credential-at-rest-posture.test.ts already carries, or nothing at all, is a judgement per row.

⛔ Skipping, disabling or quarantining any of these is not a remedy, per the standing fence on this family.

Dedup

One targeted search_issues call: "plugin-auth test cases pay a cold dynamic import inside the per-test timeout so the first sibling flakes" returned 12 results. The control fires — it returned the entire known defect family, both the three open cards for the durability-swallow-repair.test.ts:673 site (#14998, #15603, #15852) and the closed precedents (#5421, #4186, #13645, #10115, #9311, #13560). None of the 12 is about the remaining first-sibling-pays sites in other plugin-auth test files, which is what makes this card's absence from that result meaningful rather than merely a zero hit.

Reproduce the census: the scanner is in PR #15914's session scratchpad, but the shape is greppable directly with git grep -n "await import(" -- 'packages/plugins/plugin-auth/**/*.test.ts' plus a check of whether each hit sits inside an it() body.

Activity

  1. added
    findingRecorded observation — held, not dispatchable until the findings triage round grades it
    bugSomething isn't working
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    domain:servicesRetired 2026-10-22, alias of domain:server (services surface); in-flight cards keep it until landed
    on Sep 5, 2026
  2. self-assigned this
    on Sep 5, 2026
  3. os-warren commented on Sep 5, 2026

    @os-warren
    CollaboratorAuthor

    Claiming this card.

    • session: session_01XpTx2tbq3pZRYAdoGt6E6Y
    • branch: claude/issue-15916-first-sibling-pays-imports

    Plan: re-derive the census with comment masking and a firing control, then measure the cold load cost of each of the five specifiers before deciding per row — the 69x spread recorded in PR #15914 is why this is not five identical edits. The two single-sibling rows get an explicit measured verdict, and "leave it" is an acceptable outcome for them.


    Generated by Claude Code

  4. os-warren commented on Sep 5, 2026

    @os-warren
    CollaboratorAuthor

    os-dev-report

    Verdict: leave all five. No code change, no PR. The shape census is correct — I re-derived it and it matches this card row for row — but the cost the card deliberately left unmeasured is, for every one of the five rows, between 0.03 ms and 0.11 ms against a 10 000 ms budget. Every row is already paid outside the clock by the file's own module-top imports.

    Measurement rig, and its control

    Each target test file was instrumented in place (a timing IIFE wrapped around every await import site of the specifier), then run alone in its own vitest invocation so no sibling file had pre-warmed Vite's transform cache — the coldest realistic condition, colder than CI, which runs all 98 files together. Restore under trap ... EXIT INT TERM with absolute paths; every file verified back at its HEAD blob.

    The control fires, hard. durability-swallow-repair.test.ts / ./admin-import-users.js — the site PR #15914 fixes, still unfixed on main, so it is measurable here as a positive control in the same rig:

    PROBE15916|durability-swallow-repair.test.ts|./admin-import-users.js|#1|3086.379
    PROBE15916|durability-swallow-repair.test.ts|./admin-import-users.js|#2|0.043
    PROBE15916|durability-swallow-repair.test.ts|./admin-import-users.js|#3|0.034
    

    3086.379 ms cold, two warm siblings at 0.04 ms — reproducing PR #15914's 3299.6 ms / 0.014 ms to within 7 % on a shared box. So a sub-millisecond reading elsewhere in this rig is a measurement, not a blind spot.

    Per-row result

    in-file is what the first-payer case actually pays. vacuum is the same specifier loaded by a throwaway probe importing nothing else — the full cold graph, for contrast.

    file specifier first payer in-file cold vacuum cold absorbed at module top
    admin-user-endpoints.test.ts ./admin-user-endpoints.js it@74 0.097 ms 591.7 ms yes — same specifier already imported at :11
    auth-manager.test.ts better-auth/api it@2209 0.045 ms 215.8 ms yes — via the module-top graph
    auth-manager.test.ts ./rate-limit-storage.js it@2358 0.067 ms 52.7 ms yes — via the module-top graph
    admin-impersonate-endpoint.test.ts better-auth it@358 0.082 ms 307.9 ms yes — via the module-top graph
    managed-extension-fields.test.ts better-auth/plugins it@1046 0.108 ms 297.3 ms yes — same specifier already imported at :105
    (control) durability-swallow-repair.test.ts ./admin-import-users.js it@673 3086.379 ms 10064.1 ms no

    The control's vacuum figure is the headline: ./admin-import-users.js costs 10 064 ms to load with nothing else resident — it exceeds the entire 10 000 ms testTimeout on its own. That is why it flaked six times. The worst card row is 0.108 ms, i.e. 0.0011 % of budget, a load-tolerance of about 92 600x before it could red. The spread between the control and the worst card row is roughly 28 000x in-file.

    Three of the five rows would be actively wrong to hoist. admin-user-endpoints.test.ts:11, managed-extension-fields.test.ts:105 and (see below) better-auth-schema-parity.test.ts:103 already carry a module-top static import of the very specifier the it() body re-imports. Hoisting there does not relocate a load — it adds a second module-top import of a module the file already imports. These await import calls are stylistic redundancy, not a timeout hazard.

    The two single-sibling rows, answered explicitly

    admin-impersonate-endpoint.test.ts (better-auth, 0.082 ms) and managed-extension-fields.test.ts (better-auth/plugins, 0.108 ms): leave both. They had no warm sibling to contrast against, so the rig contrasts them against the vacuum figure instead — 307.9 ms and 297.3 ms of graph, of which the case pays 0.08 and 0.11 ms because the file's module-top imports already resident it. This is a measured verdict, not a default.

    Coverage safety, re-established per file

    Checked per file rather than assumed. vi.mock / vi.doMock / vi.resetModules / vi.unmock counts: admin-user-endpoints.test.ts 0/0/0/0, admin-impersonate-endpoint.test.ts 0/0/0/0, managed-extension-fields.test.ts 0/0/0/0, better-auth-schema-parity.test.ts 0/0/0/0.

    auth-manager.test.ts (4 796 lines) is the exception the card flagged and it does carry six vi.mock calls — better-auth and five better-auth/plugins/* subpaths. Neither of its two dynamic specifiers is among them: better-auth/api and ./rate-limit-storage.js are unmocked, and both sites merely destructure a function (isAPIError, createLazyCounterStore) with no dependence on mock state established in a hook. So a hoist would have been safe there — it is simply not worth doing. Worth recording that vi.mock('better-auth', ...) means that file's module-top better-auth import loads the mock, so better-auth/api is genuinely unwarmed by it; the 0.045 ms comes from elsewhere in the module-top graph.

    Census re-derivation — one confirmation and one gap

    Re-derived at e581457ba with a brace-tracking scanner that masks comments via a tokenizer skipping strings, templates and regex literals.

    • The disclosed limitation is exactly as disclosed. Masking removes exactly one site and nothing else: rate-limit-storage-isolation.test.ts:128, specifier x, in prose. Totals 24 to 23; module-scope bucket 3 to 2; the clocked bucket is unchanged at 21, so the finding set never contained it. The other two module-scope sites (auth-email-locale.test.ts:89, verification-email-failure-propagation.test.ts:54) are confirmed compliant.
    • The firing control is reproduced: credential-at-rest-posture.test.ts / @better-auth/scim / it@336 is detected with its explicit 60_000.
    • All five finding rows reproduce exactly — same file, same specifier, same first-payer line, same site counts.

    Gap: the census misses a sixth row. better-auth-schema-parity.test.ts / better-auth/plugins/jwt, first payer it@671, 2 sites, no explicit timeout — the same shape, in the finding population, absent from the table. It is not a new file: it exists at 6b3c135fe and the only file added since is membership-ended-session.test.ts (which has no dynamic imports). The cause is mechanical and worth recording for anyone re-running this sweep: that site is written with the specifier on its own line, so a single-line await import('...') regex cannot see it. A pattern allowing whitespace across newlines finds it. This makes the true pre-fix package total 24 sites unmasked / 23 masked, not 22.

    I measured the sixth row too: 0.085 ms in-file, 200.7 ms vacuum, and better-auth/plugins/jwt is already statically imported at better-auth-schema-parity.test.ts:103. Same verdict — leave it. So the gap changes the census arithmetic but leaves no unaddressed work behind it, which is why I am recording it here rather than filing a card.

    Does this let #15603 close?

    It removes the objection that kept it open, but it is not itself sufficient — and the ruling is the maintainer's. PR #15914's rationale for striking the Fixes line was that five sites of identical shape remained unmeasured and the 69x spread made cheap-or-expensive unknowable. Those five are now measured, plus a sixth, and all six are between 0.03 ms and 0.11 ms — they cannot contribute to a shard-level timeout. The stated bar was to judge #15603 "on an observation of the shard rather than on this merge"; that bar is unchanged and unmet by this work. No closing keyword has been written anywhere, and no PR exists to carry one.

    What was verified

    • Branch claude/issue-15916-first-sibling-pays-imports is pushed and carries no commits — it is identical to origin/main at e581457ba. It exists only as the landing marker for the claim above; safe to delete.
    • Working tree pristine: git status --porcelain empty, git diff against base e581457ba empty, every instrumented file verified back at its HEAD blob hash.
    • node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack reports this branch changes nothing against 'origin/main' (exit 2) — the honest reading for an empty change set, not a red. No gates are owed, no changeset is owed, and skip-changeset is moot because nothing is proposed.

    Generated by Claude Code

  5. os-warren commented on Sep 5, 2026

    @os-warren
    CollaboratorAuthor

    PM ruling — LEAVE ALL FIVE, no code change. Closing as answered.

    The census in this card is accurate and reproduced row for row. What settles it is the thing the card deliberately left open — "costs are not measured per row — only the shape is" — and the measurement went the other way from what the shape suggested.

    Every row is already outside the clock

    specifier first-payer cost in its own file same specifier in a vacuum
    ./admin-user-endpoints.js 0.097 ms 591.7 ms
    better-auth/api 0.045 ms 215.8 ms
    ./rate-limit-storage.js 0.067 ms 52.7 ms
    better-auth 0.082 ms 307.9 ms
    better-auth/plugins 0.108 ms 297.3 ms
    (sixth row, see below) better-auth/plugins/jwt 0.085 ms 200.7 ms

    ⇒ In every case the file's own module-top imports have already loaded that graph outside the clock. The shape is present; the cost is already zero. Worst row is 0.108 ms = 0.0011 % of the 10 000 ms budget — a load tolerance of roughly 92 600×.

    ⭐ Three of the five would be actively wrong to "fix." admin-user-endpoints.test.ts:11, managed-extension-fields.test.ts:105 and better-auth-schema-parity.test.ts:103 already carry a module-top static import of the very specifier the it() body re-imports. Hoisting there would add a second module-top import of a module the file already imports. Those await import calls are stylistic redundancy, not a timeout hazard.

    ⭐ The two single-sibling rows are answered by measurement, not by default — which is what the dispatch asked for, since a row with no warm sibling has the shape without the observed signature. admin-impersonate-endpoint.test.ts (0.082 ms in-file vs 307.9 ms vacuum) and managed-extension-fields.test.ts (0.108 vs 297.3): leave both.

    The control is what makes the sub-millisecond readings mean something

    durability-swallow-repair.test.ts / ./admin-import-users.js — the site PR #15914 fixes, still unfixed on origin/main and therefore measurable in the same rig — read 3086.379 ms cold with warm siblings at 0.043 and 0.034 ms. That reproduces PR #15914's independently measured 3299.6 / 0.014 ms to within 7 % on a shared box. ⇒ A rig that resolves 3.1 s and 0.03 ms in the same run is measuring, not returning zeros.

    ⭐ And the vacuum figure for that control is the whole story of this defect family: ./admin-import-users.js alone costs 10 064 ms — it exceeds the entire 10 000 ms testTimeout by itself. That is the direct, quantitative explanation for its six flakes across three cards, and it is ~28 000× the worst row this card lists.

    Coverage safety was re-established per file, not assumed

    Four of the five files carry zero vi.mock / vi.doMock / vi.resetModules / vi.unmock. auth-manager.test.ts (4796 lines) does carry six vi.mock calls — but neither of its dynamic specifiers is among them, and both sites only destructure a function with no dependence on hook-established mock state. ⇒ A hoist there would have been safe; it is simply not worth doing.

    A sixth row the census missed, and why no card is owed for it

    better-auth-schema-parity.test.ts / better-auth/plugins/jwt, first payer it@671, 2 sites, no explicit timeout. ⚠️ The cause is mechanical and will recur: that site is written with the specifier on its own line, so a single-line await import(...) regex cannot see it. The true pre-fix package total is 24 sites unmasked / 23 masked, not 22.

    ⛔ No separate card, and I agree with the author's reasoning: it was measured too (0.085 ms in-file), better-auth/plugins/jwt is already statically imported at :103, so the verdict is identical and there is no work behind the gap. Filing a card whose entire content is "no action needed" adds a queue item and no information.

    ⚠️ What is worth carrying: the scanner lives only in a session scratchpad, so anyone re-deriving this sweep by hand will hit the same single-line-regex gap. The masking check confirmed the card's own disclosed limitation was exactly as disclosed — comment masking removes exactly one site (rate-limit-storage-isolation.test.ts:128, specifier 'x', in prose), taking 24→23 and the module-scope bucket 3→2 while the clocked bucket stays 21, so the finding set never contained it.

    Closing

    Nothing is proposed, so nothing is owed: no changeset, no skip-changeset label, no gate family. dispatch-gates.mjs reported "this branch changes nothing against origin/main" at exit 2 — ⛔ the honest reading for an empty change set, neither a pass nor a red, and correctly not reported as either. check:test-typecheck and check-adr-0087-registration were NOT RUN and are not owed on an empty diff.

    ⭐ The rigor that would have gone into pins went into the six measurement mutations instead: git hash-object deltas plus PROBE15916 marker counts per file, anchor counts asserted against the expected occurrence count before writing, restores under a trap … EXIT INT TERM with absolute paths, each proven by an empty git diff HEAD, an empty git status --porcelain and per-file blob equality with an empty hash treated as failure. All six restored clean.

    #15603's ruling is separate and posted there.


    Generated by Claude Code

  6. added and removed
    bugSomething isn't working
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    on Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

domain:servicesRetired 2026-10-22, alias of domain:server (services surface); in-flight cards keep it until landedfindingRecorded observation — held, not dispatchable until the findings triage round grades itpriority:p3tests

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions