Uh oh!
There was an error while loading. Please reload this page.
fix(tests): make plugin-auth's rate-limit isolation radius visible to the gate and hashed by turbo - #10161
Merged
Conversation
… the gate and hashed by turbo `rate-limit-storage-isolation.test.ts` reads `packages/runtime/src` and `packages/services/service-sms/src` to check that neither consumer reaches the fixed-window counter through the package ROOT — the #6040 invariant whose breach silently reinstates the whole better-auth load. It derived its roots with a `findUp` walk from `process.cwd()`, which `check:cross-package-test-inputs` does not resolve (`process.cwd` appears nowhere in that detector), so the read produced no flag, no declaration, and no turbo input. Measured on the parent commit: `--list-escapes` named only `managed-extension-fields.test.ts` for plugin-auth, and `@objectstack/plugin-auth#test` hashed to `1bf3935543ab055b` both before and after a change under `packages/runtime/src`. Reinstating the root import in `packages/runtime/src/security/inbound-rate-limit.ts` — the exact regression this test guards — replayed `cache hit, replaying logs` / `>>> FULL TURBO` in 135ms, exit 0, while a direct vitest run on the same tree was RED. That is #7802's shape, on the gate that exists to prevent it. Three changes: - Reseed the test from `__dirname` (TS1470-free under this CJS-typed package's `module: NodeNext`, and a spelling the detector resolves), following the rationale `managed-extension-fields.test.ts` and `platform-objects/src/managed-api-method-affordance-sweep.test.ts` already state for their sibling walks. - Bind each consumer root by NAME and hand it to `readdirSync` by that name. The gate rosters a DIRECTORY only when a directory-read consumes an expression it can resolve; `join(REPO, root)` over an array element yields no name, so the globs would have been declared but UNHELD. - Declare the radius in CROSS_PACKAGE_TEST_INPUTS and mirror it into `turbo.json`'s `@objectstack/plugin-auth#test` inputs. `managed-extension-fields.test.ts` is deliberately NOT touched: it is the only other escaping read the gate sees in this package and it alone holds the `packages/**/*.object.ts` radius. Part of #10029 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
Contributor
📓 Docs Drift CheckNothing 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. |
os-warren
marked this pull request as ready for review
August 20, 2026 12:26
os-warren
enabled auto-merge
August 20, 2026 12:26
Uh oh!
There was an error while loading. Please reload this page.
os-zhuang pushed a commit
that referenced
this pull request
Aug 21, 2026
…-test-inputs recognises The gate resolves four directory-naming seed spellings plus the walked-file form, not "exactly two". `import.meta.dirname` and `dirname(import.meta.filename)` arrived with #9763 and the walked `import.meta.url` seed with #8995; both extensions updated RECOGNISED_PATH_SPELLINGS, which the gate prints, and neither swept for the count restated in prose elsewhere. Corrected, pointing at RECOGNISED_PATH_SPELLINGS as the single source instead of restating a number that drifts: - managed-extension-fields.test.ts: the seed count, plus two more stale claims measured on this tree — it is no longer the ONLY escaping read the gate sees in plugin-auth (#10161 made rate-limit-storage-isolation.test.ts visible), and that file no longer walks up from process.cwd(). The note also promised a stale-declaration failure that no longer fires: reseeding this file from process.cwd() now leaves the gate GREEN, with packages/**/*.object.ts declared and held by nothing. - managed-api-method-affordance-sweep.test.ts: the same seed count. Its stale-declaration consequence IS still real and is now cited from the measurement (that file is platform-objects' only escaping test; the ablation exits 1 naming the package). - check-type-check-coverage.mjs: the same claim in a TEST_DEBT note, the worst-placed instance since an author reads it while choosing a seed. - serve-multi-node-cap-advisory.pin.test.ts (not named in the card): stale in the opposite direction — it claimed the gate cannot follow a new URL() seed or a resolve() nested into the read. It follows both. The instruction to keep the whole path in one literal stands, for the reason that is actually true: only the flat literal collector needs that spelling. Comments and one note string only — no test code, no type surface. The gate's own repoRelativeLiterals roster for all three test files is unchanged from origin/main, so no declaration and no turbo.json input moves. Part of #10163 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
os-zhuang added a commit
that referenced
this pull request
Aug 21, 2026
…eeds `check:cross-package-test-inputs` finds escaping tests by scanning source text, so a seed spelling it does not know yields no flag -- which means no declaration, silently. Every seed it knew answers "where am I?" off the module itself. A CJS-typed package cannot ask: plugin-auth publishes `dist/index.js` as CommonJS, so under `module: NodeNext` `import.meta` is a TS1470 there, and four of its tests walk up from `process.cwd()` to an anchor instead. That walk resolved to nothing, so every path built on it resolved to nothing too. Measured on `19f98fa1f^`: `rate-limit-storage-isolation.test.ts` read `packages/runtime/src` and `packages/services/service-sms/src` through such a seed, appeared in no roster, and turbo replayed a cached green over the scan it never re-ran -- #7802 exactly, by a fourth spelling. #10161 reseeded that one file; this closes the class it was an instance of. `findUpSeeds()` resolves the two predicates that are knowable without executing anything: a manifest `name` matching THIS package (-> the package root, named) and a WORKSPACE_ROOT_MARKERS file (-> the repo root, which escapes on its own exactly as `resolve(HERE, '../../..')` already does). A manifest naming some OTHER package keeps the escape verdict and loses the name, the trade `walkLiteral` already makes for an unreadable argument. Today's findUp population is clean, so no gate turns red and none turns newly green -- the proof is 14 `--self-test` cases, 8 of which fail without the detector change. Part of #10029 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #10029— the finding also lists directions B (teach the detector thefindUpshape, repo-wide) and C (leave it recorded). This PR implements the graded direction A only, so the card should stay open for a decision on B.The defect, measured on the parent commit (
2d3860df9)rate-limit-storage-isolation.test.tsreadspackages/runtime/srcandpackages/services/service-sms/srcto check that neither consumer reaches the fixed-window counter through the package root — the #6040 invariant whose breach silently reinstates the whole better-auth load forservice-sms. It derived its roots with afindUpwalk fromprocess.cwd().check:cross-package-test-inputsresolves seed expressions statically.process.cwdappears nowhere in that detector (grep -c 'process\.cwd'→0; counter-checked against__dirname→8andimport.meta.dirname→7, so the zero is a measurement, not a broken search). The read therefore produced no flag, no declaration, and no turbo input:Turbo's
@objectstack/plugin-auth#testinput set (224 entries, package-relative) contained 0 paths under../../runtime/srcand 0 under../../services/service-sms/src, against counter-checks of 35 for../../core/src/securityand 11 for../../services/service*— the declared globs, present as expected.The cache replay, demonstrated rather than asserted
Reinstating the root import in
packages/runtime/src/security/inbound-rate-limit.ts— the exact regression this test guards — and re-running the task:The same tree, run directly, is red:
A green replayed over a live-broken invariant. That is #7802's shape, on the gate that exists to prevent it.
What changed
Reseed from
__dirname. TS1470-free under this CJS-typed package'smodule: NodeNext, and a spelling the detector resolves — the rationalemanaged-extension-fields.test.tsandplatform-objects/src/managed-api-method-affordance-sweep.test.tsalready state for their sibling walks.Bind each consumer root by name. The gate rosters a directory only when a directory-read consumes an expression it can resolve;
join(REPO, root)over an array element yields no name (its own header lists "a directory read whose path is only a loop variable" among the shapes that yield nothing). Written as a loop, the globs would have been declared but unheld — nothing would fail if a later edit deleted them, which is check-cross-package-test-inputs' literal collector cannot see split-segment or ascent-relative paths, so four declared radii are held by prose alone #9763's failure mode one level up. NowreaddirSync(RUNTIME_SRC, …)/readdirSync(SERVICE_SMS_SRC, …)take named bindings.Declare the radius in
CROSS_PACKAGE_TEST_INPUTSand mirror it intoturbo.json's@objectstack/plugin-auth#testinputs.Three of the declared entries (
scripts/check-published-files.mjs,scripts/check-cross-package-test-inputs.mjs,packages/types/src/node-isolation.test.ts) are named in the test's prose rather than read by it. Declared rather than reworded, following the settled treatment ofserve.tson the@objectstack/specentry andrealtime-protocol.mdxon@objectstack/dogfood: the literal collector takes quoted paths without parsing, so a mention forces a declaration, and declaring is cheaper than rewording prose to dodge the scanner. All three are low-churn.Proof that the gate now SEES the radius (not merely exits 0)
The gate names both directories in its roster — the failure text before the globs were declared:
It is demanding these globs, so they are held: delete them and the gate goes red. After declaring:
Proof that turbo now HASHES the two packages
#testhash1bf3935543ab055b844fad10458a8a2bAblation — prediction stated first, then observed
Predicted: with the fix in place, the same runtime mutation must (a) move the hash off
844fad10458a8a2b, (b) miss the cache and actually execute, and (c) fail with the same offender string.Observed — all three:
Restore proven byte-identical, both sides via
git hash-object:No rebuild was needed for the mutation to be observable, and that is a property of the files involved, not an assumption. The assertion reads
packages/runtime/src/**/*.tsas raw source text throughfs.readFileSync; nothing on its path resolves through@objectstack/runtime'sexportsordist/. The test's own header states this deliberately ("a SOURCE-level scan rather than a probe ofdist/", because a gate reading build output passes or fails by local build state). Confirmed against the run: all 25 dependencybuildtasks stayed cached — no rebuild occurred — and the test still turned red. A staledist/therefore cannot mask this ablation in either leg.Verification
Gate union derived with
node scripts/pm/dispatch-gates.mjs(no paths passed) after the final commit, on a clean worktree, at0a0019c93. Exit codes captured before any pipe.check:cross-package-test-inputs(incl. 57 self-test cases)check:slot-lookupcheck:test-source-aliascheck:type-source-resolutionscripts/docs-audit/check-affected-docs.mjscheck:query-options-erasurecheck:type-check-coveragecheck:type-check-debt(--re-measure, 33 entries, built closure)check:engine-double-contractcheck:where-matchercheck:nul-bytespnpm --filter @objectstack/plugin-auth typecheckturbo run test --filter=@objectstack/plugin-authcheck:type-check-debtreported "33 ledger entr(ies) re-measured … none above its recorded number". No ledger entry was raised:scripts/check-type-check-coverage.mjsis untouched (empty diff), so@objectstack/plugin-authstays at 109 and the self-test fixture at:2766is unmodified.Merge-queue cost, stated deliberately
This is direction A's known price and it is not small:
@objectstack/plugin-auth#testnow re-runs on any diff underpackages/runtime/src(250 files) orpackages/services/service-sms/src(12). That suite is ~50s / 1335 tests, and the queue runs the full suite under heavier load than the PR path. Buying that is the point — the gate's own header argues the radius is what the list buys over "just always run those packages" — but reviewers should expect runtime-touching PRs to newly schedule this suite wheremainhas been serving a cached green.Not done, deliberately
managed-extension-fields.test.tsis untouched. It is the only other escaping read the gate sees in this package and it alone holds thepackages/**/*.object.tsradius; its__dirnameseed is intact (itsfindUpoccurrence is a prose mention, verified).findUpshape, so the class remains open repo-wide — that decision is still the card's.Release notes input
No changeset: this PR touches one test file, one CI gate script and
turbo.json, and publishes nothing. Requesting theskip-changesetlabel per that mechanism rather than shipping an empty changeset. Following the gate, not habit —content/docs/releases/is untouched.Generated by Claude Code