Skip to content

perf(core): cheapen the authz transport scan so a slow test stops aborting the Test Core shard - #13656

Merged
zhuangjianguo merged 1 commit into
mainfrom
claude/issue-13645-authz-transport-scan-cost
Aug 31, 2026
Merged

perf(core): cheapen the authz transport scan so a slow test stops aborting the Test Core shard#13656
zhuangjianguo merged 1 commit into
mainfrom
claude/issue-13645-authz-transport-scan-cost

Conversation

@claude

@claudeclaudeBot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Fixes#13645

packages/core/src/security/authz-store-unavailable.test.ts rebuilt its transport ledger from source twice per run, and each rebuild walked all of packages/, statSync'd every directory entry, and read every .ts file into a UTF-8 string — then discarded 59% of them for their path. Under vitest's inherited 5000 ms default that timed out on CI, and a timeout aborts the shard, so one slow test cost eleven other packages their entire run on PRs that never touched authorization.

Verified at 9ae177c47c — the commit this PR ships.

What actually costs what

The card blamed the full-tree read and offered memoisation. Both readings were measured, and the picture is more specific than either.

half of the scanmedian, warmshare
walk()readdirSync + 5,926 statSync18.6 ms9.9%
readFileSync(f, 'utf8') over 5,076 files170.0 ms90.1%

The directory walk was never the problem. Roughly half of the read half was the UTF-8 decode itself — 147.5 MB of file bytes turned into transient JS strings per run, to answer a yes/no question about an ASCII substring.

The repair — four changes, no assertion touched

  1. The scaffolding filter runs before the read, not after it. A path belongs to the result iff it both contains the call and is not scaffolding; set intersection does not care which half is tested first, so this is semantically free. It removes 2,979 of 5,076 files (56% of the bytes) that were read in full only to be thrown away for their name.
  2. The needle is matched against bytes.readFileSync(f) returns bytes with no decode, and Buffer.prototype.includes searches them. The two spellings cannot disagree: the needle is pure ASCII, and an ASCII byte never occurs inside a multi-byte UTF-8 sequence (continuation bytes are all at or above 0x80), so a byte hit and a decoded-string hit are the same hit.
  3. readdirSync(dir, { withFileTypes: true }) answers "is this a directory?" from the readdir result, replacing 5,926 statSync calls. The symlink limb keeps the old follow-the-link semantics exactlyDirent.isDirectory() describes the link, not its target, so without it a symlinked directory would stop being descended and a transport behind one would drop out of the ledger's reach.
  4. The enumeration is computed once per process. Both tests need the whole thing; deriving it twice read the tree twice for one answer.

Measured

Wall time of the scan, and of the two tests that use it.

beforeafter
CONTROL callback (in vitest, warm)295 ms101 ms2.9x
SET EQUALITY callback (in vitest, warm)218 ms0 msmemoised
one scan, standalone, cold page cache632 ms255 ms2.5x
whole run, standalone, cold page cache885 ms255 ms3.5x

Cold figures drop the page cache before every run, median of 3. Work performed, which is environment-independent and therefore the number that travels:

per runbeforeafter
file opens10,1522,0974.84x fewer
bytes read147.5 MB32.5 MB4.54x fewer
bytes UTF-8 decoded into JS strings147.5 MB0eliminated
statSync calls per walk5,9260one per symlink; none in this tree

Headroom, stated as a multiple

vitest's timeout is per test callback, so the worst single callback is the unit that matters.

5000 ms budgetheadroom
before, warm295 ms16.9x
before, cold632 ms7.9x
after, warm101 ms49.5x
after, cold255 ms19.6x

⚠️What this does not establish. CI's actual cost is censored — vitest aborts at the timeout, so the only fact CI gives us is "greater than 5000 ms". The CI-to-local factor is therefore bounded below at 7.9x (against local-cold) and not bounded above. NOT MEASURED, and no repair measured on a developer box can close that gap by itself. Cost is linear in file count here (0.124 ms per file before, 0.122 ms after), so the repair divides whatever that factor produces by 2.42x on the worst callback — which is a real improvement and not, on its own, a comfortable margin against an unbounded number.

That is why the two scanning tests now carry a stated budget of 30,000 ms. The 5000 ms they had was vitest's default for a test that does no I/O; it was inherited, never chosen, and was measurably the wrong budget for a filesystem scan. It is a budget, not a timing assertion — deliberately not expect(elapsed).toBeLessThan(n), which on a shared runner is flaky by construction and would only re-file this card's successor. The asymmetry justifies generosity: a budget set close to the observed cost buys nothing, while the failure it guards against takes eleven other packages down with it.

The guarantee is intact, and that is checked rather than claimed

The ledger is still rebuilt from source on every run — memoisation means compute once per process, never a checked-in list, a snapshot fixture, or a cache keyed on anything that outlives a commit. Demonstrated rather than asserted, by adding a new production file containing resolveAuthzContext({ and re-running:

× SET EQUALITY: the ledger names exactly the transports source contains
- packages/core/src/security/scan-probe-tmp.ts == the new transport, found

Removing it returns the suite to green (37/37). A transport added later still cannot inherit the old silence.

Unchanged, verbatim: TRANSPORT_LEDGER and every disposition in it · toBeGreaterThanOrEqual(8) · toContain('packages/rest/src/rest-server.ts') · toEqual(Object.keys(TRANSPORT_LEDGER).sort()) · every per-transport check. git diff shows no expect( line changed anywhere in the file. Nothing is skipped, disabled, quarantined, allow-listed, deleted or re-baselined. The #13279 semantics are untouched — resolve-authz-context.ts is not in this diff.

Verification

Full packages/core suite at 9ae177c47c: Test Files 46 passed (46) · Tests 1135 passed (1135) — the same totals CI reported when it was 1 failed | 45 passed. Both discoverTransports() call sites exercised: CONTROL pays the scan (101 ms), SET EQUALITY reads the memoised result (0 ms).

Gate families derived from the actual diff with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, all green at 9ae177c47c: check:cross-package-test-inputs · check:test-source-alias · check:engine-double-contract · check:where-matcher · check:query-options-erasure · check:nul-bytes · check:type-check-coverage · check:doc-authoring · check-comment-mask-adoption · check-ci-filter-parity · check-keyed-text-bounds · check-plugin-teardown-shape · check-undeclared-dep-imports. eslint --no-inline-config on the changed file: 0 errors, 0 warnings.

packages/core declares no typecheck script — its type coverage is carried by the ledger, and check:type-check-coverage passes.

No changeset — skip-changeset

The diff is one .test.ts file. It publishes nothing from any package, which is the label's own case (AGENTS.md: a changeset is for feature work; pure fixes do not require one). The label is applied on this PR.


Generated by Claude Code

… shard
`authz-store-unavailable.test.ts` rebuilt its transport ledger from source
TWICE per run — once in the CONTROL test, once in the SET EQUALITY test — and
each rebuild walked all of `packages/`, `statSync`'d every directory entry, and
read every `.ts` file into a UTF-8 string before discarding 59% of them for
their path. Measured on this tree: 10,152 file opens and 147.5 MB decoded into
transient JS strings per run, against vitest's inherited 5000 ms default. On CI
that timed out, and a timeout ABORTS THE SHARD — so one slow test cost eleven
other packages their entire run, on PRs that never touched authorization.
Four changes, none of which touch what the suite asserts:
- the scaffolding path filter runs BEFORE the read instead of after it. A
path belongs to the result iff it both contains the call and is not
scaffolding, and set intersection does not care which half is tested
first, so the reordering is semantically free: 2,979 of 5,076 files are no
longer read in full only to be thrown away.
- the needle is matched against BYTES. The needle is pure ASCII and an ASCII
byte never occurs inside a multi-byte UTF-8 sequence, so a byte hit and a
decoded-string hit are the same hit — with no 147 MB decode in between.
- `readdirSync(dir, { withFileTypes: true })` answers "is this a directory?"
from the readdir result, replacing 5,926 `statSync` calls. The symlink limb
keeps the old follow-the-link semantics exactly, so a transport behind a
symlinked directory still cannot escape the ledger.
- the enumeration is computed once per PROCESS. It is still rebuilt FROM
SOURCE on every run, which is the guarantee the #13279 ruling requires; it
is simply not rebuilt twice for one answer.
The 5000 ms budget was inherited, never chosen, and was measurably the wrong
budget for a filesystem scan. The two scanning tests now state one explicitly.
It is a budget, not a timing assertion — deliberately not
`expect(elapsed).toBeLessThan(n)`, which on a shared runner is flaky by
construction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 31, 2026
@github-actions

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 eaba72e48cc294038e74ece5bf7677568d55b038packageMentionDocs.

@zhuangjianguoClaude

Copy link
Copy Markdown
Collaborator

PM verdict: ACCEPT — releasing

Fully green at 9ae177c47c: all 35 checks success or skipped, zero failures. The two I deliberately refused to assume from a check-suite event both came back green — Lint & Repo Gates (05:36:43Z) and Governed Surface Queue Guard. mergeable_state: clean.

⭐ The proof this PR exists for

Test Core (6/6) — SUCCESS at 05:30:23Z. That is the exact shard that had been aborting, on the real CI environment whose slowness could not be measured from a developer box. It is the one result no local measurement could substitute for.

Reviewed independently, not taken on the PR body's word

  • walk() is semantically equivalent, checked across all four cases rather than the happy path: regular directory, symlinked directory, symlink-to-.ts-file, and broken symlink (still throws, as statSync did). The entry.isSymbolicLink() && statSync(full).isDirectory() limb is load-bearing — Dirent.isDirectory() is false for a symlink-to-directory, so without it a transport behind one would silently drop out of the ledger.
  • Byte-matching is sound. UTF-8 is self-synchronising — every byte of a multi-byte sequence is ≥ 0x80 — so a pure-ASCII needle matches in raw bytes iff it matches in the decoded string. Invalid-UTF-8 files don't change that either: U+FFFD replacement only touches bytes ≥ 0x80.
  • Zone 1.1 holds, and was PROVEN rather than asserted.SCANNED ??= is per-process state in a fresh process per run — not a fixture, not a checked-in list, not a commit-spanning cache. The seat validated it with a discriminating probe: added a file containing resolveAuthzContext({, showed SET EQUALITY go red naming the probe, removed it, 37/37 green. That is the probe discipline the order asked for, executed properly.
  • No assertion moved. No expect( line changed anywhere in the file; TRANSPORT_LEDGER and all eight dispositions byte-identical; resolve-authz-context.ts not in the diff. Nothing skipped, disabled, quarantined, allow-listed, deleted or re-baselined.

⭐ The strongest argument, which the PR body undersells

Moving the scaffolding filter ahead of the read doesn't only cut the constant — it changes what the scan grows with. Cost now scales with production file count rather than total file count, and test files are the faster-growing population in this repo (2,979 of 5,076 walked files were scaffolding). That flattens the growth curve, which is the real answer to "is this a postponement rather than a fix?"

Zone 2: three falsifications, one of them mine

Four of five assumptions returned with the measurement that decided them, and the seat falsified rather than confirmed where the numbers said so:

⭐ Also recorded: concurrent reads at concurrency 48 were measured (285 ms cold vs 255 ms serial) and rejected rather than shipped. Declining complexity on a measurement is the same discipline as adopting it.

The 30,000 ms stated budget is ratified per option A, ruled at #13645 comment 5474133014.

Releasing

Marked ready and enqueued. ⛔ Not merged by this seat, and no approving review submitted by it — the queue and a human reviewer own that.

This unblocks #13630 (dequeued 04:50:50Z on this exact timeout) and #13635 (Test Core (6/6) red on it), neither of which needs a code change.


Generated by Claude Code

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review August 31, 2026 05:44
@zhuangjianguo
zhuangjianguo added this pull request to the merge queueAug 31, 2026
Merged via the queue into main with commit 575ce83Aug 31, 2026
37 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-13645-authz-transport-scan-cost branch August 31, 2026 06:04
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@zhuangjianguo@claude