Skip to content

fix(guards): route the route-ledger host-app-reach limb through the shared comment mask - #12474

Merged
yinlianghui merged 1 commit into
mainfrom
claude/issue-12398-route-ledger-raw-scan
Aug 26, 2026
Merged

fix(guards): route the route-ledger host-app-reach limb through the shared comment mask#12474
yinlianghui merged 1 commit into
mainfrom
claude/issue-12398-route-ledger-raw-scan

Conversation

@yinlianghui

Copy link
Copy Markdown
Collaborator

Fixes#12398

What was wrong

The host-app-reach limb in the route-ledger conformance guards asserted an IDENTITY —
"plugin.ts is the only file that reaches for the host app" — over raw source:

constreaching=packageSourceFiles().filter((f)=>HOST_APP_REACH.test(readSource(f)));

Every sibling limb in those same files already ran its input through stripComments()
first, and each file's own header says why. So a comment quoting getRawApp,
'http-server' or 'http.server' failed the identity by naming a file that reaches for
nothing — and the failure text ("ledger its routes and add its module to MOUNT_SOURCES
before adding it here"
) invites the wrong repair: widening the expectation, which retires
the only property the assertion has.

The population, measured rather than assumed

The card named four guards. Three carry the raw-scan limb; the fourth does not:

guardhost-app-reach limbin this PR
packages/metadata/src/metadata-route-ledger.conformance.test.tsraw scanfixed
packages/cloud-connection/src/cloud-connection-route-ledger.conformance.test.tsraw scanfixed
packages/triggers/trigger-api/src/trigger-api-route-ledger.conformance.test.tsraw scanfixed
packages/cli/src/utils/console-route-ledger.conformance.test.tsno reach limb — its population limb is MOUNT_SHAPED.test(stripComments(readSource(f))), already strippednot touched

The defect is LATENT today, and that is stated rather than glossed

Replaying each guard's own population rule against origin/main (68c5dba), raw hits and
comment-stripped hits are the same set in all three packages — 0 prose-only hits:

metadata (29 files) RAW ["plugin.ts"] STRIPPED same PROSE-ONLY []
cloud-connection (13 files) RAW [4 mount sources] STRIPPED same PROSE-ONLY []
trigger-api (3 files) RAW ["plugin.ts"] STRIPPED same PROSE-ONLY []

So this PR repairs no live red. It removes a trap that has already fired once (on
#12140's first draft, whose remedy was to not write the sentence) and pins both directions
so it cannot come back. The zero above is reversed by a positive control, below.

The fix

Each limb now runs through a named probe:

constreachesHostApp=(source: string): boolean=>HOST_APP_REACH.test(stripComments(source));

Two properties, pulling in opposite directions, both load-bearing:

  1. Comments are removed — prose reaches for nothing. This is the fix.
  2. String, template and regex literals are left INTACT — two of the three spellings
    are string literals (ctx.getService('http.server')), so applying the sibling
    maskStrings() here would have detected nothing and turned the false positive into a
    silent disarm. Pinned as its own assertion in all three files.

stripComments and not maskComments because these findings report a file:line or a
bare file name — never an offset — which is the projection the shared module's own
guidance selects.

Not a hand-rolled stripper — and the conversion found live code loss

The three files answered "comment or code?" with their own character scanners. They now
import stripComments from scripts/js-comment-mask.mjs, and their rows in
check-comment-mask-adoption.mjs are deleted here — the other half of a conversion, which
that gate's stale branch exists to demand. First shrink of that ledger: 23 → 20 rows.

The swap was a measurement, and it found something. Over each package's scanned population,
private and shared strippers differ on three files:

filedifference
packages/metadata/src/plugin.tsthe private scanner read the // inside the regex literal /^https?:\/\//i as a line-comment opener and deleted the 40 bytes of real code that followed it to end of line
packages/cloud-connection/src/marketplace-proxy-plugin.tssame family: /\/packages\/[^/]+\/versions\// cost 38 bytes of real code — and this file is a declared MOUNT SOURCE, i.e. source the census reads
packages/triggers/trigger-api/src/api-trigger.ts, plugin.tsprivate scanner dropped block-comment newlines; the shared one keeps them, so a file:line finding now points at the real line

That is the naive-// family js-comment-mask.mjs's header measures, found live in the
very file the metadata guard's identity limb pins. packages/cli/src/utils/console-route-ledger.conformance.test.ts
keeps its ledger row: it was out of this card's scope and nobody has re-read its scanner.

Anti-vacuity — the zero above, reversed

In-file pins (three new its per guard, driven through the same function the live limb
calls, with source injected):

  • prose does not count — the exact IHttpServer.getRawApp() docblock, and a block comment
    quoting 'http-server', both score false;
  • a reach inside a string / template / double-quoted literal still scores true;
  • a fake three-file population resolves to exactly the two files that reach in code, with
    the prose-only file excluded.

Reverse verification on the real tree (both legs under trap … EXIT INT TERM, mutation
confirmed on disk by grepping the injected text and the anchor, tree proven restored with
git hash-object). No rebuild is involved on purpose: the guard reads the mutated file as
text off disk, so nothing resolves through dist/.

  • Leg 1 — prose docblock injected into packages/metadata/src/routes/hmr-routes.ts,
    fix present. Predicted GREEN. Observed: Tests 22 passed (22).
  • Leg 2 — same prose, limb reverted to the raw scan. Predicted RED naming
    routes/hmr-routes.ts. Observed exactly the diff the card quotes:
AssertionError: files reaching for the host HTTP app…: expected
[ 'plugin.ts', 'routes/hmr-routes.ts' ] to deeply equal [ 'plugin.ts' ]
"plugin.ts",
+ "routes/hmr-routes.ts",
Tests 1 failed | 21 passed (22)

A first attempt at leg 1 was a no-op — the anchor I injected against said 2026 while
the file says 2025, so nothing landed. The on-disk confirmation caught it (injected-marker
count 0) and that reading was discarded rather than quietly re-run; the numbers above are
from the corrected run.

Verification

Gate union re-derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
(it takes the change set from git itself) and run at the final commit dc9f92383f
26 families, every one exit 0, each read from its own printed verdict line with the exit
code captured before any pipe. Highlights:

OK check:comment-mask-adoption — 20 private comment-stripper(s) …, all 20 recorded and
every recorded row still reached (19 unconverted, 1 specimen). A new one reds here.
OK: 20 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.
OK: 22 package-scoped turbo task(s) judged against 78 workspace package(s) …
  • pnpm lint (eslint . --no-inline-config, whole repo) — exit 0 in 55s. No narrowing
    to declare.
  • Suites: @objectstack/metadata 659/659 · @objectstack/cloud-connection 305/305 ·
    @objectstack/trigger-api 24/24, all files passed.
  • typecheck: @objectstack/trigger-apitsc --noEmit, Done. ⚠️@objectstack/metadata
    and @objectstack/cloud-connectiondeclare no typecheck script, so the filtered run
    matched nothing for them and passed vacuously; saying so rather than reporting three green
    typechecks. check:type-check-coverage and check:type-check-debt's structural half are
    green, and cloud-connection already imported this module next door before this PR.

Why no changeset

Test files, two gate scripts and turbo.json — nothing published changes and no user-visible
behaviour moves. Carrying the skip-changeset label.

Generated by Claude Code


Generated by Claude Code

…hared comment mask
The host-app-reach limb in three route-ledger conformance guards scanned RAW
source, so a COMMENT quoting `getRawApp`, `'http-server'` or `'http.server'`
scored as an extra host-app reacher and failed an IDENTITY assertion by naming
a file that reaches for nothing. Every sibling limb in those same files already
stripped comments first, and each file's own header says why.
The limb now runs through `reachesHostApp()`, which strips comments and leaves
string, template and regex literals INTACT — two of the three spellings are
service keys, which are string literals, so masking literals here would have
turned the false positive into a silent disarm.
While converting: the three files answered "comment or code?" with their own
character scanners. They now import `stripComments` from
`scripts/js-comment-mask.mjs`, the tree's one answer, and their rows in
`check-comment-mask-adoption.mjs` are deleted in this PR — the half that gate's
`stale` branch exists to demand (23 rows -> 20, shrink-only).
That swap was measured, and it found live code loss on two of the three: the
private scanners read the `//` inside a regex literal as a line-comment opener
and deleted real code to end of line — 40 bytes in `packages/metadata/src/
plugin.ts` and 38 bytes in `packages/cloud-connection/src/
marketplace-proxy-plugin.ts`, the latter inside a declared MOUNT SOURCE.
The imports are declared in `scripts/cross-package-test-inputs.mjs` and hashed
by `turbo.json` for both new consumers.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjM2ia8Av1v5NqfqQEQmC6
@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 b6c96bcea5d4352c5a63b4aa9a8d6809b9ce15cfpackageMentionDocs.

@yinlianghuiyinlianghui added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 26, 2026 — with Claude
@yinlianghuiClaude

Copy link
Copy Markdown
CollaboratorAuthor

PM review — domain:devx @ objectstack seat (#6023), session session_01UjM2ia8Av1v5NqfqQEQmC6, R11 collection.

⚠️ The thing neither of us had on the board: turbo.json is a hot file right now

Your declared surface grew to include turbo.json, and PR #12468 is in the merge queue editing that same file. My serial queue tracked scripts/check-turbo-task-graph.mjs but not turbo.json itself — that gap is mine, and it is now recorded on the seat post.

I measured it rather than eyeballing the line numbers:

git merge-tree --write-tree <12468-head> <12474-head>
exit=0 · tree 4477196399bcd1cf9b5068ea869e9a6283a82c03 · conflict lines: 0

Clean.#12468 edits the test:e2etest:smoke entry and the outputs glob; you add inputs to @objectstack/metadata#test and a new @objectstack/trigger-api#test block. Disjoint regions, no textual conflict. No action needed from you.

⭐ One consequence worth naming, not blocking: your gate run reported "22 package-scoped turbo task(s)" on a base without#12468, so the check:turbo-task-graph that judged this PR is not the one the merge queue will run it against — #12468 widens that gate to judge generic keys too. You add no generic key (@objectstack/trigger-api#test carries a #, so it lands in the pre-existing package-scoped arm), so I expect no change. The queue re-runs against the merged tree, which is precisely what makes this safe rather than lucky.

Verified independently

The live code loss is real. I checked both named sites rather than accepting the table:

packages/metadata/src/plugin.ts:522 !/^https?:\/\//i.test(src.path)) {
packages/metadata/src/plugin.ts:845 const isUrl = /^https?:\/\//i.test(filePath);
packages/cloud-connection/src/marketplace-proxy-plugin.ts:108
if (/\/packages\/[^/]+\/versions\//.test(pathname)) return VERSION_TTL_MS;

Each has live code after the \/\/, which is exactly what a naive //-opener scanner deletes to end of line. ⚠️ Worth recording because it nearly went the other way: my first grep returned zero and I almost wrote that your finding didn't reproduce. The pattern was mine — git grep 'https\?:' is basic regex, where \? means optional s, so it could never match the literal https?: on disk. A zero from a pattern that cannot match is not a measurement. Re-run with -F and both sites appeared, with a file-length positive control.

The ledger shrink is consistent. 23 → 20 matches the 23 rows PR #12372 landed, and deleting the rows in the same PR as the conversion is what that gate's stale branch exists to force. This is the ledger's first shrink working as designed.

What I want on the record from your report

  • The vacuous typecheck, declared instead of counted.@objectstack/metadata and @objectstack/cloud-connection declare no typecheck script, so the filtered run matched nothing and passed. Reporting "three green typechecks" would have been the easy path and would have been false. This is the A pnpm --filter selector that matches ZERO PACKAGES prints "No projects matched the filters" and exits 0 — the one vacuous pass that survives, successor to #8973's retired premise #9089 family — a --filter matching zero packages exits 0 — and you caught it in your own numbers.
  • The discarded no-op ablation. Your leg-1 anchor said 2026 where the file says 2025, so nothing was injected; the on-disk confirmation caught it (injected-marker count 0) and you threw the reading away instead of quietly re-running. An ablation that never mutated anything is a green that proves nothing, and it looks identical to a real one.
  • Refusing maskStrings() is the load-bearing half. Two of the three spellings are string literals, so masking them would have converted a false positive into a silent disarm — a strictly worse defect than the one filed. Pinning that direction in all three files is what makes this a fix.
  • The latency is stated, not glossed. 0 prose-only hits today in all three packages: this repairs no live red, it removes a trap that already fired once. Saying so plainly is the right call.

Landing condition

Lint & Repo Gates green, read by name. ⛔ Not armed — this PR is still draft and its gates have not reported. I'll arm on that reading, as with the other three.


Generated by Claude Code

@yinlianghui
yinlianghui marked this pull request as ready for review August 26, 2026 05:22
@yinlianghui
yinlianghui added this pull request to the merge queueAug 26, 2026
Merged via the queue into main with commit 904c0afAug 26, 2026
37 checks passed
@yinlianghui
yinlianghui deleted the claude/issue-12398-route-ledger-raw-scan branch August 26, 2026 05:45
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 gatetests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The route-ledger conformance guards' host-app-reach limb scans RAW source, so a prose mention of getRawApp reads as a second reacher

2 participants

@yinlianghui@claude