Skip to content

fix(ci): reconstruct the paths cross-package tests really read, instead of trusting quoted prose - #9826

Open
os-steve wants to merge 2 commits into
mainfrom
claude/issue-9763-literal-collector-spellings
Open

fix(ci): reconstruct the paths cross-package tests really read, instead of trusting quoted prose#9826
os-steve wants to merge 2 commits into
mainfrom
claude/issue-9763-literal-collector-spellings

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes#9763. Sub-issue of #9747 (the meta-card), in its fails toward FALSE GREEN half.

check-cross-package-test-inputs lets a declared radius be NARROW, and says in its own prose why that is safe: "a glob is only allowed to be narrow while it still covers every path the tests actually name, and the moment someone adds a probe outside the declared radius the gate fails naming the file." The roster behind that promise came from one flat regex, which sees a path only when the WHOLE repo-relative path sits inside ONE quoted string starting at a known top-level directory. Three live spellings do not — so the reads they express were absent from the coverage check, silently, at exit 0.

H1 — the three-step ladder, before and after

The card's acceptance test. Ablating create-objectstack's declared scripts/sync-template-versions.mjs glob and progressively unquoting the two header-comment mentions of that path:

stepbefore (06f9848f9)after (c74b2b9aa)
glob dropped, both mentions quotedRED — names template-consistency.test.ts (a mention)RED — names template-consistency.test.ts
+ one mention unquotedRED — names template-version-stamps.test.ts (the other mention)RED — names template-version-stamps.test.ts
+ BOTH mentions unquotedOK: 12 package(s) ... , exit 0RED — names template-version-stamps.test.ts, the file that actually READS

And the other half the card demanded: with the glob kept and both mentions unquoted, the gate is green — the glob is now justified by the read, not by prose. Both mentions are ordinary prose again, free to reword.

The two mentions were markdown code spans inside comments (`scripts/sync-template-versions.mjs`), which the flat regex cannot tell from code. Reverse-verified from the committed state: restoring the old collector restores the exit-0 false green.

H3 — AST or regex? Neither. The resolver was already there

The checker is not a flat regex over source. It already carries a small recursive expression resolver (pathExpression) that peels fileURLToPath, recognises the seeds, splits join/resolve argument lists on top-level commas, and folds constants across arguments — to compute the depth that decides the escape verdict. It then threw the segment names away.

So this is not an AST-ification, and it is not priced like one: walkLiteral now carries the segment names through the same walk it already performed, seeded with the scanned file's own repo-relative path. A path split across join('scripts', 'x.mjs') arguments and an ascent-relative new URL('../../../scripts/x.mjs', import.meta.url) both come out as the repo-relative string an author would have quoted. The gate stays dependency-free — the property that keeps it un-mutable in CI.

Per the card's ruling 2, the three named spellings are not one change:

  1. split-segment join/resolve — reconstruction (mechanism)
  2. ascent-relative literals — reconstruction (same mechanism; it is the same walk starting at ..)
  3. skills/ prefix — a one-line data fix, correctly diagnosed in the card. skills/ was simply missing from the flat alternation.

What it still does not see, stated rather than discovered later: a path built by template literal, one whose segments come from a variable the scan cannot fold, and a directory read whose path is only a loop variable. Each yields no name — never a wrong one. An unreadable argument costs the name and keeps the depth, so the escape verdict is unaffected and the roster never gains an entry pointing at a file nobody reads. Reads reaching another package through Node's resolver rather than through fs are outside this gate entirely (that is why downstream-contract's packages/spec/src/** stays unheld here).

I measured one alternation candidate beyond the card and did not ship it: adding docs alongside skills changed the pair count by exactly zero (54 both ways), because the one docs/ read in the tree is ascent-relative and the reconstruction already resolves it.

No false leads (ruling 3)

Matching #9700's bar, against #9639's +139,084:

pairslostfamilies gaining
#9639 (hint-extractor widening)+139,084
#9700 (declaration-based)+4,75101
this change+806

Every one of the eight, hand-checked:

packagepath gainedwhy it is real
@objectstack/clipackages/connectors/connector-mcp/src/connector-mcp-plugin.tsascent-relative; the entry's own comment already documented this hole
@objectstack/downstream-contractpackages/spec/package.jsonbuilt from a directory prefix, then read
@objectstack/formulaskills/objectstack-formula/SKILL.mdthe skills/ case
@objectstack/metadata-protocolscripts/check-durability-degradation-log-level.mjsascent-relative new URL
@objectstack/plugin-securitypackages/spec/src/security/high-privilege.tssplit-segment
@objectstack/specpackages/lint/srcsplit-segment directory listing
@objectstack/speccontent/docs/api/error-catalog.mdxnever declared — see below
@objectstack/specdocs/audits/2026-07-unknown-key-strictness-ledger.mdnever declared — see below

Nothing outside the intended radii moved: zero pairs lost, and six of the eight fall inside globs the declarations already carried.

Two radii that were never declared at all

The reconstruction found these on its first run — they are not radii @objectstack/spec grew here:

  • packages/spec/src/api/error-catalog-docs.test.ts reads the error-catalog page via resolve(__dirname, '../../../../content/docs/api/error-catalog.mdx') and asserts it documents every StandardErrorCode. spec declared content/docs/references/**, not this page.
  • packages/spec/scripts/strictness-ledger.test.ts reads the audit ledger via resolve(SPEC, '../../docs/audits/...') and ratchets it against the schema files it inventories.

Both are readFileSync on real files. Their globs and the matching turbo.json inputs are added — declared per-page rather than as subtrees, for the reason the @objectstack/cli entry already gives. Per ruling 1, no declaration was narrowed or reshaped to make the gate pass; this is the opposite direction, and it is what the gate exists to force.

H2 — the full sweep, all 12 entries / 53 declared globs

The card's "four of twelve" was found while investigating one entry. Sweeping every glob, and splitting the roster by which half produced it:

roster halfbeforeafter
held by a reconstructed read0 (the mechanism did not exist)19
held by a flat quoted literal only3523
held by nothing machine-visible1611

Seven declared globs moved into read-held, including all four the card named:

packageglobbeforeafter
create-objectstackscripts/sync-template-versions.mjsprose mentionREAD
@objectstack/specpackages/lint/src/**prose mention (in an unrelated test)READ
@objectstack/metadata-protocolscripts/check-durability-degradation-log-level.mjsnothingREAD
@objectstack/formulaskills/objectstack-formula/**nothingREAD
@objectstack/clipackages/connectors/.../connector-mcp-plugin.tsnothingREAD
@objectstack/plugin-securitypackages/spec/src/security/**nothingREAD
@objectstack/downstream-contractpackages/spec/package.jsonnothingREAD

An important correction to the card's framing. "Flat-literal-held" is not the same as "prose-held". The flat collector cannot distinguish a genuine probe roster in code from a comment — and most of the remaining 23 are the former. @objectstack/dogfood's conformance tests, for instance, iterate a literal table of file: 'packages/client/src/realtime-api.ts' entries and read each one; that IS the designed spelling, not prose. Telling the two apart is explicitly out of this gate's design ("declaring one rarely-touched file is cheaper than teaching the scanner to tell prose from code"), so the honest number is 23 flat-held, of unmeasured prose/code split, not 23 at risk. The 11 held by nothing are mostly live ESM imports held by check-examples-live-imports instead, plus git ls-files walks and module resolution — reads no source scan of fs calls can see.

Directories

Two of the card's three split-segment examples resolve to directories, so the file-only roster filter would have dropped them. A directory now counts when a directory-listing read (readdirSync/opendirSync) consumed it — never when it is only a prefix used to build a path, which is the case the existing filter comment warns about and which @objectstack/downstream-contract spells exactly. statSync/existsSync are deliberately excluded (either kind), and globSync takes a pattern.

Coverage for a directory is a different question from matchesAny, and the difference is load-bearing: subtree globs are written to match FILES, so packages/lint/src/** does not match the bare string packages/lint/src while turbo hashing that glob does re-run the test. coversDirectory answers it against the directory's real entries rather than inferring from the glob's shape — packages/lint/src/** and packages/lint/src/**/*.object.ts are the same shape and only one of them re-runs when an ordinary .ts file appears.

Tests

--self-test grows 33 to 52 cases, one per added spelling as #8995 requires. Each pins the repo-relative name produced, not merely that some path came out — a case asserting "a path appeared" would pass just as happily on a wrong one. Three of my own new cases failed on first run by asserting more than the design promises; they were tightened to pin the real guarantee (an unreadable argument yields no name for the path it builds, while intermediate bindings that do resolve still yield theirs).

H4 — gate union, re-derived from the real diff

dispatch-gates names only 2 families for this file, both the same gate — the shortfall #9651's dev hit. Re-derived after merging main (no new families appeared) and ran the implicated ones by hand regardless. All green on c74b2b9aa:

check:cross-package-test-inputs PASS OK: 12 package(s) read outside themselves, all declared
check:examples-live-imports PASS 0 invisible (declared), 6 inputs-declared, 69 graph-visible
check:ratchet-remedy-authority PASS 97 scripts swept
check:nul-bytes PASS 6271 text files, no raw control bytes
dispatch-gates --self-test PASS 285 cases
eslint (changed file) PASS

check-ratchet-remedy-authority classifies this file by name and check-examples-live-imports mirrors its globToRegExp (unchanged here) and reads its globs; dispatch-gates' own self-test reads this file's watch hints and asserts its scripts/** and content/** declarations still reach specific paths.

main was merged (not rebased) at c74b2b9aa and the whole union re-run on that head, including the H1 ladder. Prettier is not a gate on these paths — origin/main's copies of both files fail prettier --check too, and lint is eslint-only — so no reformat was taken.

No changeset: a CI gate script and turbo.json publish nothing.


Generated by Claude Code

…ad of trusting quoted prose
`check-cross-package-test-inputs` lets a declared radius be NARROW, and says in
its own prose why that is safe: a glob is only allowed to be narrow while it
still covers every path the tests actually name. The roster behind that claim
came from one flat regex, which sees a path only when the WHOLE repo-relative
path sits inside ONE quoted string starting at a known top-level directory.
Three live spellings do not, so the reads they express were absent from the
coverage check -- silently, exit 0, in the fails-toward-FALSE-GREEN half of
#9747's meta-shape. Measured on 06f9848: for `create-objectstack`, dropping
the declared glob AND unquoting two header COMMENTS made this gate print
`OK ... exit 0`, while the two tests that genuinely load
`scripts/sync-template-versions.mjs` went on loading it. Prose held the radius.
The fix is a reconstruction, not a wider regex. `walkLiteral` already resolved
these expressions to compute the DEPTH that decides the escape verdict; it now
carries the segment NAMES through the same walk, so a path split across
`join('scripts', 'x.mjs')` arguments and an ascent-relative
`new URL('../../../scripts/x.mjs', import.meta.url)` both come out as the
repo-relative string an author would have quoted. No parser, no dependency --
the gate stays un-mutable in CI. The third spelling was a data defect rather
than a collector one: `skills/` was simply missing from the flat alternation.
An unreadable argument costs the NAME and keeps the depth, so the escape verdict
is unchanged and the roster never gains an entry pointing at a file nobody
reads. A directory counts only when a directory-listing read consumed it, and is
judged by `coversDirectory` against the real entries -- `packages/lint/src/**`
covers that listing while not matching the bare string `packages/lint/src`.
Measured: +8 (package, path) pairs, 0 lost, 6 packages gaining. Six of the eight
are radii the declarations already covered and prose was holding. Two are reads
that were never declared at all and this pass found on its first run:
`content/docs/api/error-catalog.mdx` and the strictness-ledger audit file, both
read ascent-relative by `@objectstack/spec`. Their globs and the matching
turbo.json inputs are added here; no declaration was narrowed.
`--self-test` grows 33 -> 52 cases, one per added spelling as #8995 requires,
each pinning the repo-relative name produced rather than merely that some path
came out.
Refs #9763
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
@os-steveos-steve added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 19, 2026 — with Claude
@claude

claudeBot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

✅ PM ACCEPT — #9763 / PR #9826

Verified independently: 2 files +416/-62, zero governed-surface hits, no non-green gates (two still running).


⭐ H3's price does not apply, and the reason is the best part

I asked whether the collector could reconstruct a split-segment path at all, warned that AST-ifying a regex-based checker would be a much bigger change than the card implied, and said a partial fix was acceptable if you named what it still missed. The answer made the question moot:

pathExpressionalready resolved these expressions to compute the DEPTH that decides the escape verdict, so walkLiteral now carries the segment NAMES through the same walk, seeded with the scanned file's own repo-relative path — no AST, gate stays dependency-free

The machinery was already there. It was computing the path and throwing the names away. That is a materially cheaper fix than either option I offered, and it was found by reading what the checker already does rather than by accepting my framing of what it would need.

Ruling 2 answered too: split-segment and ascent-relative are one mechanism, skills/ is the one-line data fix — 2 + 1, not 3.

⭐ The directory subtlety is one I did not anticipate

two of the card's three split-segment examples resolve to DIRECTORIES, so a directory now rosters when a readdirSync/opendirSync consumed it — never as a mere path prefix — judged by a new coversDirectory against real entries

The obvious implementation would roster any path that looks like a directory prefix, which would have manufactured false coverage everywhere. Gating on the call that actually consumed it, and judging the glob against real entries so packages/lint/src/** covers the listing while not matching the bare string, is the precise version.

H1 — both halves of the ladder, before and after

stepBEFORE (06f9848f9)AFTER
glob removedRED — names a mention
+ one mention unquotedRED — names the other mention
+ both mentions unquotedOK, exit 0 — the false greenRED naming template-version-stamps.test.ts, the file that actually READS
glob kept + both mentions unquotedGREEN exit 0 — the glob is held by the read

That last row is the half I said the fix must produce or it has not closed the gap. Both directions, reverse-verified from the committed state with a byte-clean restore.

⭐ No-false-leads, and a change you measured and DECLINED

46 → 54 pairs: +8, 0 lost, 6 packages gaining — against #9639's rejected +139,084 and #9700's +4,751 / 0 lost / 1 gaining. All 8 hand-checked as real reads, 6 already inside declared globs. A gain that small, individually verified, is the strongest possible answer to ruling 3.

And:

Measured and NOT shipped: adding 'docs' beside 'skills' in the alternation changed the count by exactly 0 (54 both ways)

Declining a change that measures as a no-op — rather than shipping it because it looks symmetrical — is exactly right. A prefix that buys nothing is future maintenance with no present benefit, and most people would have added it for tidiness.

⭐ And you found two radii that were never declared at all

spec reads content/docs/api/error-catalog.mdx and docs/audits/2026-07-unknown-key-strictness-ledger.md, both ascent-relative — invisible to the old collector, and therefore never declared. Their globs plus matching turbo.json inputs are added.

That is the fix finding real gaps on its first run, which is the best evidence it works. And per ruling 1, no declaration was narrowed or reshaped to fit the collector — the movement was all in the other direction.

The prose is updated, and the distinction is correct

I asked you to update #9765's rationale, since your fix makes it obsolete. You did:

- // So do not reword those mentions into unquoted prose on the theory that…
+ // …really reads. The mentions are ordinary prose again — free to reword.

And you correctly left the realtime-protocol.mdx entry at :257 saying "named in a comment rather than read" — because for that one it is still true. #9765's dev removed only the stale cross-reference clause. Two adjacent comments, one now false and one still true, and you changed exactly the false one.

On the mechanics

Verdict: ACCEPT. Arming once the two running gates converge.


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/mskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

check-cross-package-test-inputs' literal collector cannot see split-segment or ascent-relative paths, so four declared radii are held by prose alone

2 participants

@os-steve@claude