Skip to content

Convert both canonical-envelope page gates onto the shared comment mask - #12317

Merged
yinlianghui merged 2 commits into
mainfrom
claude/issue-12267-third-stripcomments-residue
Aug 25, 2026
Merged

Convert both canonical-envelope page gates onto the shared comment mask#12317
yinlianghui merged 2 commits into
mainfrom
claude/issue-12267-third-stripcomments-residue

Conversation

@yinlianghui

Copy link
Copy Markdown
Collaborator

Fixes#12267

Both canonical-envelope page gates carried a private two-regex comment stripper, byte-identical to each other and to the family #9367 retired from six gates and #10453 found surviving in two packages/cli tests:

functionstripComments(source: string): string{returnsource.replace(/\/\*[\s\S]*?\*\//g,'').replace(/^[\t]*\/\/.*$/gm,'');}

Each fed the export const X: Page = scan that decides which pages its package's gate audits. Both now import maskComments from scripts/js-comment-mask.mjs, and the scan is split into pageDeclarationsIn(source) so the new pins drive the real code path.

The card's premise, verified — and one half of it is wrong

The card names both sites at :94. Confirmed by content, not by line: platform-objects at 94, cloud-connection at 96 (drifted). The regex is byte-identical in both.

The card also says: "No current page declaration in either package trips the phantom-comment shape, so both gates are green today and correctly so."

The conclusion is right; the reason is not. Measured on the base commit by diffing the retired regex against scanSource's comment array, offset-preserving, over both packages' src/:

packagesrc filespages via naive strippages via real maskpopulations agree
platform-objects8133yes
cloud-connection1422yes

So both gates were green. But the phantom-comment shape is already shipped in cloud-connection/src, in the file that declares an audited page:

  • src/cloud-connection-ui.ts — the line comment // … /api/v1/cloud-connection/* routes this plugin mounts. opens a phantom that the next docblock's*/ closes. The retired regex deletes 122 bytes of live page literal between them, type: 'cloud-connection:panel' and properties: {} included, along with the closing }; of CloudConnectionSettingsPage.
  • src/marketplace-proxy-plugin.ts — two more spans, 277 bytes.

The gate stayed green only because that opener sits below the export const … : Page = the scan anchors on. A page declared thirty lines further down that same file would have vanished from the population, and every audit below would have reported green over it. This was one edit away, not a shape only a fixture writes.

The option choice was made by measurement, and it selected A — then B turned out to be moot

The card offered A (convert both), B (widen check:parse-guard's population beyond scripts/), C (leave it), with "no recommendation recorded" and a note that a second hand-filed residue "is itself the argument for B".

The census, on the base commit, over git ls-files '*.{ts,tsx,mts,cts,js,mjs,cjs}' excluding scripts/, by two probes — the regex shape and the identifier, because the family is a shape and not a name:

  • 14 files carry a regex-based comment stripper
  • 7 files carry a hand-rolled char-scanner named stripComments
  • disjoint sets → 21 files, of which exactly 2 are the pair this PR repairs (19 remain after it lands)

A first probe for the block-comment shape returned zero; a positive control against the two named files showed the probe itself was broken (escaping), not the tree. The reading above is from the repaired probe, whose positive control resolves. What these probes could not have found: a stripper built from a variable or template literal, and one that removes comments without either shape.

21 ≫ 2, so by the dispatch's own rule the count points at B. B is nonetheless moot, for a stronger reason than "non-trivial cost": check:parse-guard is the wrong instrument. It governs the three TypeScript parser entry points (ts.createSourceFile, ts.createProgram, ts.transpileModule) and bans them outside scripts/ts-parse.mjs. The stripComments sentence the card quotes appears in its header only as a cited precedent for why one-time sweeps do not hold — not as parse-guard's subject. Widening its population would extend a ban on raw parser entry points to package sources and would not catch a single comment-stripper. Its own header also argues against the widening twice over: parses outside scripts/** are deliberately unbanned (a published package must not depend on repo tooling), and declaring a broader root is refused as the "22 leads is the same as none" failure.

So scripts/check-parse-guard.mjs is untouched, and neither undiscoverable ledger (bare-root-worklist's TRIAGE map, check:pm-dispatch-gates' escapable-literal species) is reached. The class-closing gate — a new adoption gate, not a widening of this one — is filed with the full census as #12307.

The pins

Three cases per file, driving the real pageDeclarationsIn:

  • a page straddled by an opener inside a line comment (the shape cloud-connection-ui.ts ships)
  • a page straddled by an opener inside a string literal (the shape the card names)
  • a : Page = written inside genuine prose, still correctly refused — so the conversion did not simply stop masking

openerIsNotAComment guards each fixture's shape so a future edit cannot leave a test that passes without asserting anything.

Ablation (packages/platform-objects): pageDeclarationsIn reverted to the retired regex, mutation confirmed on disk both ways (injected const naive = source.replace present ×1, removed maskComments(source).matchAll ×0), the fixture pins go red, everything else stays green:

× keeps a page straddled by an opener inside a LINE COMMENT
× keeps a page straddled by an opener inside a STRING LITERAL
Tests 2 failed | 18 passed (20)

No rebuild is involved: the ablated function lives in the test file itself, which vitest transforms from source, and maskComments is a plain .mjs imported at runtime — no dist/ sits between the mutation and the measurement. Restoration was by an EXIT INT TERM trap and verified byte-identical.

The third pin stayed green under the ablation, which is correct and is the point of including it: the retired regex handles ordinary prose fine. Its defect is only the phantom opener.

Landing obligations this created

The maskComments import escapes both packages, so check:cross-package-test-inputs went red — as the card anticipated. Declared rather than worked around:

  • scripts/js-comment-mask.mjs — the real import, for both packages
  • scripts/js-comment-mask.d.mts — the declaration mirror that types maskComments, so each package's typecheck verdict is a function of it (the reason the @objectstack/cli entry declares the pair)
  • packages/lint/src/page-envelope-audit.test.ts and packages/cloud-connection/src/cloud-connection-ui.ts — named in platform-objects' prose and read by nothing. Declared, not reworded: that gate's own roster settles the trade at the check-nul-bytes.mjs entry — "declaring the file beats rewording a comment to dodge a scanner", and over-collection can only widen a radius, never narrow one. The page-envelope-audit.test.ts mention pre-dates this PR; the diff pulled platform-objects into the gate's scan and surfaced it.

Plus the matching turbo.json inputs, including a new @objectstack/cloud-connection#test task (the package had none, so its test cache was keyed on package-local files only).

scripts/cross-package-test-inputs.mjs and turbo.json are therefore two files beyond the claim's declared surface. Both are inseparable from closure A; neither is check-parse-guard.mjs.

Verification

Gate union derived at the final commit (87a8a94957) with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack — 16 path-matched families plus the convention-triggered ones for "adds or edits a test file" and "edits a file in a package that owns an i18n-extract.config.ts". The first derivation printed STALE TREE; origin/main was merged in and the union re-derived clean before any number here was trusted. Every exit code captured before any pipe.

Both packages' own suites, on 87a8a94957:

@objectstack/platform-objects Test Files 31 passed (31) Tests 493 passed (493)
@objectstack/cloud-connection Test Files 27 passed (27) Tests 302 passed (302)

Gate verdict lines, each quoted from the gate's own output:

OK: 17 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.
✓ check:parse-guard: 166 scripts/ file(s) — every TypeScript parse goes through ts-parse.mjs.
check-page-declaration-shape --self-test: all passed
✓ where-matcher conformance holds: 299 matcher(s) discovered, 299 answer the combinator battery correctly or refuse it loudly (186 refuse).
✓ query-options-erasure ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new.
check-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys).

check:agent-test-spelling, check:cli-command-ids, check:entry-guard, check:pnpm-filter-targets, check:published-files, check:slot-lookup, check:test-source-alias, check:type-source-resolution, check:type-check-coverage, check:engine-double-contract, check:i18n-stale-fill, check-ci-filter-parity.mjs, check-plugin-teardown-shape.mjs, check-affected-docs.mjs and check-drift-comment.mjs all exited 0.

Two results that are not plain greens, stated rather than rounded off:

  • check:i18n first reported EXIT=1 and measured nothingPREREQUISITE NOT MET — the workspace CLI is not built … Nothing was checked. That is a refusal, not a verdict. @objectstack/cli was built and the gate re-run; it then exited 0 with the line quoted above. Reported here because an unbuilt refusal reads exactly like a failure and neither one is a measurement.
  • @objectstack/cloud-connection has no typecheck script, so pnpm --filter … typecheck would match zero scripts and exit 0 having run nothing. tsc --noEmit was driven directly instead: 13 errors, exactly the count its DEBT ledger entry records (11 × TS2493 + 2 × TS2550), none of them in the converted file. The ratchet is unmoved.

check:type-check-debt --re-measure was not run locally — it re-runs tsc across every ledger entry in the repo. Declared narrowing: the two packages this diff touches were measured directly and both are unchanged (platform-objects' own typecheck script exits 0; cloud-connection sits at its recorded 13). CI runs the full ratchet regardless.

No changeset: the diff is two test sources, one root gate roster and turbo.json — no published package source, so nothing is released. skip-changeset applies.


Generated by Claude Code

Both gates carried a private two-regex comment stripper, byte-identical to
each other and to the family #9367 retired from six gates and #10453 found
surviving in two packages/cli tests:
source.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^[ \t]*\/\/.*$/gm, '')
Each fed the `export const X: Page =` scan that decides which pages its
package's gate audits, so a block-comment opener that is not a comment --
inside a string literal, or inside a line comment -- opens a phantom comment
running to the next real terminator, deletes the declarations in between, and
the gate reports GREEN over a page it never read.
That shape is already shipped. In packages/cloud-connection/src, the retired
regex deletes 122 bytes of live page literal from cloud-connection-ui.ts (the
file declaring CloudConnectionSettingsPage) and 277 more from
marketplace-proxy-plugin.ts. Both gates were green only because the opener
sits BELOW the declaration the scan anchors on.
Both now import `maskComments` from scripts/js-comment-mask.mjs, and the scan
is split into `pageDeclarationsIn(source)` so the new pins drive the real code
path over fixtures rather than over today's tree.
Cross-package input radius declared for both packages (the import escapes the
package, and the .d.mts mirror types it), with the matching turbo.json inputs.
Part of #12267
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 783111d25227b7f907f2a0059797056e0fb78e8apackageMentionDocs.

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.

[finding] A third residue of #9367's naive stripComments: both canonical-envelope page gates carry the byte-identical regex

2 participants

@yinlianghui@claude