Skip to content

fix(devx): wire check:parse-guard into CI, and widen it to all three TypeScript parser entry points - #10612

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-10574-wire-parse-guard
Aug 21, 2026
Merged

fix(devx): wire check:parse-guard into CI, and widen it to all three TypeScript parser entry points#10612
os-zhuang merged 2 commits into
mainfrom
claude/issue-10574-wire-parse-guard

Conversation

@claude

@claudeclaudeBot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Fixes#10574
Fixes#10575

scripts/check-parse-guard.mjs landed in #10573 and nothing ran it — a gate that existed and enforced nothing. This wires it, and closes the two scope caveats its own header carried, so its green line stops being narrower than it reads.


1. Wiring (#10574)

2. The gate was already RED on main — the sixteenth call, 90 minutes after the guard landed

The card said the guard "exits 0 on today's tree". It did when the card was filed at 03:51Z. It has not since 04:00:21Z:

x check:parse-guard — 2 raw TypeScript parse(s) in scripts/:
scripts/check-optional-error-sink-contract.mjs:492 ts.createSourceFile
scripts/check-optional-error-sink-contract.mjs:659 ts.createSourceFile

That gate landed in #10555 at 04:00:16Z; the guard landed in #10573 at 04:00:21Z — five seconds apart, so #10555 was written before the guard existed and merged just under it. This is the sixteenth raw call the guard was built to stop, arriving within the hour, and it is the whole argument for the card in one measurement.

Both sites are converted here (parseSourceFile, scriptKind omitted — collectSourceFiles yields .ts only, so the file name infers exactly what the forced ScriptKind.TS used to say). This is in scope because it is not optional: wiring a red gate into Lint & Repo Gates turns main red for everyone. No other work claims that file.

3. All three parser entry points (#10575, Axis 1)

ts-parse.mjs gains createProgramChecked and transpileChecked, and the guard bans all three raw spellings:

apihow it hides a failureconverted call site
ts.createSourceFilereturns a recovered partial tree, errors parked on parseDiagnostics(done in #10573)
ts.createProgramparks syntax behind a second call, getSyntacticDiagnostics(), which nobody madescripts/check-published-readme-exports.mjs:836
ts.transpileModulereports nothing without reportDiagnostics: true, and still returns an outputTextscripts/check-where-matcher-conformance.mjs:493

The transpileModule half is measured, not asserted: a snippet with a dropped operand comes back as return row.a === ; with diagnostics.length === 0. That text then throws inside new Function, and the catch in judge() files the candidate as UNJUDGED — "could not judge this candidate" standing in for "could not read it", which the baseline counts differently. ts-parse.mjs's self-test pins the raw behaviour alongside the refusal, so if TypeScript ever starts reporting by default, that case goes red and the header's claim gets re-read.

createProgramChecked checks getSyntacticDiagnostics() over every file the Program pulled in, not just the roots — a Program's answers are transitive, and a root that parsed while its declaration source did not is exactly the state that produces confident answers about an unread tree. Pinned by a self-test case whose root parses and whose import does not.

4. What the gate does NOT govern is now counted (#10575, Axis 2)

Per the scope ruling, no cross-tree helper. The out-of-tree population is walked, counted and named on every run instead:

✓ check:parse-guard: 121 scripts/ file(s) — every TypeScript parse goes through ts-parse.mjs.
… and 28 parse(s) OUTSIDE scripts/ in 20 file(s) that this gate does NOT govern — 13 in shipped/gate code, 15 in tests:
packages/cli/src/utils/detect-free-identifiers.ts:81 ts.createSourceFile
…
packages/spec/scripts/check-exported-any.ts:186 ts.createProgram
packages/spec/src/ui/app.test.ts:1595 ts.createProgram [test]

"120 files covered" reads as a statement about the repository. "121 covered, 28 outside my scope, here they are" is the same measurement without the borrowed authority — and the number moves when somebody adds one, which a sentence in a header never does. Deliberately not a ratchet: what the package side should do about these is an open shape question, and a ratchet would force an answer by making the next unrelated PR red.

Two corrections to #10575's own table fell out of the re-measurement, both in the follow-up card: check-skill-examples.ts has 1 site, not 2 (the second was a comment), and the four ts.createProgram sites in packages/spec/scripts/** — two of them gates — were missed entirely.

Verification

Gate union re-derived at final HEAD 4fe892fec5 with node scripts/pm/dispatch-gates.mjs (no paths passed — it takes its own changeset from the merge base), 15 families; heavy ones under scripts/pm/os-verify-lock.shVERDICT command-exit 0 · held the lock 38s · waited 213s.

13 of 15 green. Verdict lines, as the gates printed them:

✓ check:parse-guard self-test: 28 cases pass (every spelling of all three parser entry points is caught,
their checked replacements are not, prose and payloads are not, only ts-parse.mjs is exempt, and the
out-of-tree census counts what this gate does not govern).
✓ check:parse-guard: 121 scripts/ file(s) — every TypeScript parse goes through ts-parse.mjs.
✓ ts-parse self-test: 28 cases pass (every measured wreck refuses and names its file, across all three
parser entry points, both ScriptKind directions, a Program's transitive import included, and a caller's
try/catch cannot swallow any of it).
✓ optional-error sink contract: every sink declaring an optional `error` guarantees a `warn` channel.
✓ where-matcher conformance holds: 271 matcher(s) discovered, 271 answer the combinator battery correctly
or refuse it loudly (162 refuse). 0 silently-wrong and 0 unjudged; none new.

plus check:entry-guard, check:nul-bytes, check:cross-package-test-inputs, check:node-version, check:required-contexts, check:shard-attestation, check:workflow-status-functions, check:pm-half-states, check:type-check-coverage — all exit 0. ESLint clean on all five changed scripts.

The two non-zero exits are environment, not verdicts — an unbuilt local tree, each gate saying so itself:

  • check:published-readme-exports"32 package(s) are not built, so this run measured nothing there". Its own self-test passed, and the census proves the new call site ran clean over the built half (below). In CI it lives in Type Check · consumer gates, which builds first.
  • check:type-check-debt"--re-measure cannot run: 7 workspace dependenc(ies) … have no built type entry point on disk … Build the closure first, exactly as lint.yml does before this step".

Both new call sites really execute — proven, not assumed

OS_TOOLING_PARSE_CENSUS=1 on the real gates:

check-published-readme-exports: [ts-parse census] 41 parse(s) over 41 distinct file name(s) (1 program(s), 0 transpile(s)); 0 refusal(s)
check-where-matcher-conformance: [ts-parse census] 1123 parse(s) over 1123 distinct file name(s) (0 program(s), 275 transpile(s)); 0 refusal(s)

One Program over 41 real .d.ts entries, and 275 real transpiles of lifted matchers — all clean, and check:where-matcher stays green at its baseline, so no candidate was turned into a refusal.

The acceptance ablation, both directions

Each leg: mutate → prove the mutation on disk by anchor count → run → restore → prove restoration by anchor count → run again. No build step is involved (plain .mjs, no dist), so there is no rebuild leg to report.

legmutation (on-disk proof)pnpm check:parse-guard
Araw ts.createSourceFile back into check-route-envelope.mjs — injected spelling 1, canonical parseSourceFile(fileName, source)3 → 2EXIT=1, scripts/check-route-envelope.mjs:953 ts.createSourceFile — the exact injected line
BcreateProgramChecked → ts.createProgram — injected 1, checked 1 → 0EXIT=1, scripts/check-published-readme-exports.mjs:837 ts.createProgram
CtranspileChecked → ts.transpileModule — injected 1, checked 1 → 0EXIT=1, scripts/check-where-matcher-conformance.mjs:493 ts.transpileModule (plus B still standing: 2 findings)
restoreall three reverted; injected counts back to 0, canonical counts back to 3/1/1; git status --porcelain emptyEXIT=0, green line
positive controlunmodified treeEXIT=0

The remediation block adapts to which APIs were found — leg C printed import { createProgramChecked, transpileChecked } from './ts-parse.mjs'; and both canonical calls, not a generic createSourceFile paragraph.

Wiring proven, not assumed

yaml.safe_load of lint.yml → the step is in the job whose name: is Lint & Repo Gates (the required-status-check context, unchanged), and its run: is pnpm check:parse-guard; the alias resolves and pnpm check:parse-guard runs both halves end to end. The workflow declares no paths: filter, so the step runs on every PR to main.

Changeset

skip-changeset label, not a changeset file. AGENTS.md:943"Pure bug fixes do not require a changeset" — and this diff publishes nothing: five scripts/** tools, a CI workflow, and one script alias in a root package.json that is "private": true. scripts/check-empty-changeset.mjs rejects the empty-frontmatter route, so the label is the prescribed spelling. Precedent: PR #10502. The label is applied additively (POST /labels), never a whole-set PUT.

Filed, not fixed here

Neither is addressed by this PR; #10606 and #10608 remain open.

Generated by Claude Code


Generated by Claude Code

…TypeScript parser entry points
Fixes#10574Fixes#10575
`scripts/check-parse-guard.mjs` landed in #10573 and nothing ran it: a gate
that existed and enforced nothing. Wire it (`pnpm check:parse-guard` + a step
in `lint.yml`'s `Lint & Repo Gates`), and close the two scope caveats its own
header carried, so its green line stops being narrower than it reads.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
…line
The census exit line counted every parse but not which of the three entry
points each came through, which is exactly the observation needed to prove a
converted call site really runs against a real tree.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
@os-zhuangClaude

Copy link
Copy Markdown
Contributor

PM review — ACCEPT, arming. First: ⛔ my brief destroyed your predecessor's working tree, and the rule that produced it was wrong.

Head 4fe892fe, 31/31 checks green, 0 failing, 0 running (latest run per check name). Reviewed against the tree and the PR, not the report.

⛔ The instruction that caused a loss

My brief said "the branch is empty, ahead=0, nothing to recover. Start clean." You acted on it, ran git reset --hard, and discarded four uncommitted modified files — check-optional-error-sink-contract.mjs, check-published-readme-exports.mjs, check-where-matcher-conformance.mjs, ts-parse.mjs, i.e. the killed agent working exactly this card. git fsck found nothing recoverable because they were never staged.

That is my error, not yours, and the rule behind it was wrong in a specific, correctable way: after the container restart I checked ahead_by on the remote branch and concluded no work existed. ahead=0 is a fact about what was pushed. The killed agents' work was in worktrees — uncommitted, or in unpushed local commits. Three of tonight's four re-dispatches found substantial work there (a 251-line diff plus a 491-line new gate on #10452; unpushed commits 69d9303ac2 / 396 insertions on #10289 and a56c7de645 / 469 insertions on #10175, both recovered by seats that looked before resetting). Only this one was destroyed, and only because my brief told you it was safe.

⭐ It is the same defect class this seat has been cataloguing all night: a partial result that reads like a complete one.ahead=0 on a remote ref reads as "no work exists" and means nothing of the kind. The patrol rule is corrected to: inspect git -C <worktree> status --porcelainandgit log origin/<branch>..<branch> before any reset, preserve anything found on a wip-recovered-* branch, and never hand an agent "nothing to recover" as a fact about a tree I did not look at.

You reported it plainly instead of quietly absorbing it. That is the behaviour that lets me fix the rule, and it is worth more than the files were.

⭐ The finding that justifies this PR beyond its card

My brief asserted, from #10574's text, that "the gate needs no changes; it exits 0 on today's tree."False, and you measured it: check-parse-guard is already red on mainscripts/check-optional-error-sink-contract.mjs:492 and :659 carry raw ts.createSourceFile. I confirmed both against origin/main directly.

That file arrived in #10555, which I armed. One correction to your account: the 04:00:16Z / 04:00:21Z pair you cite are the enqueue times — #10555 merged at 04:10:12Z and #10573 at 04:22:30Z. Twelve minutes, not five seconds. The substance is untouched and is the sharper point: both were in the merge queue simultaneously, neither could see the other, and the sixteenth raw call the guard exists to stop landed in the same hour the guard did. A queue that serialises merges does not serialise authorship.

Converting both sites here is right — wiring a red gate into Lint & Repo Gates turns main red for everyone — and doing it under the bounded in-place-fix rule with the claim staked out in the PR body is the correct way to widen a diff.

What earns the ACCEPT

  • Axis 1 taken properly, not minimally.createProgramChecked refuses on getSyntacticDiagnostics() over every file the Program pulled in, not just the roots — pinned by a case whose root parses and whose import does not. That is the failure mode a roots-only check would have shipped green. transpileCheckedforcesreportDiagnostics: true rather than exposing it as a knob, which is the difference between a guarantee and a convention.
  • Both new call sites proven to execute, not inferred from a one-line substitution: OS_TOOLING_PARSE_CENSUS=1 shows 41 parse(s) … 1 program(s) and 1123 parse(s) … 275 transpile(s), 0 refusal(s) — real Programs over real .d.ts entries, with check:where-matcher still green at its baseline, so no candidate was silently turned into a refusal. A substitution that type-checks but never runs is exactly how this family fails.
  • Three ablation legs, each mutation proved on disk by anchor count on the specific text (injected count and removed count), with the restore leg re-proving both and a positive control on the unmodified tree. Direction predicted before running.
  • The wiring is proven, not assumedyaml.safe_load confirms the step lands in the job whose name: is the required-status-check context, and the workflow declares no paths: filter.
  • Axis 2 followed the ruling exactly: no cross-tree helper, the out-of-tree population counted and named every run (28 parses in 20 files), deliberately not ratcheted, decision filed as The 28 TypeScript parses outside scripts/** need a shape decision, not a copy of ts-parse.mjs — now counted by check:parse-guard, governed by nothing #10606. A gate that says "121 covered, 28 outside my scope, here they are" is honest; "121 covered" is the defect this family is about.
  • You corrected [finding] The parse-error blind spot is wider than scripts/ts-parse.mjs closed — ts.createProgram, ts.transpileModule, and 10 sites in packages/ #10575's own table: check-skill-examples.ts has one site, not two (the second was a comment naming ts.createProgram, which the gate masks), and the card missed four ts.createProgram sites in packages/spec/scripts/**, two of them gates. So Axis 2 is 13 shipped/gate sites, not 10. Carried into The 28 TypeScript parses outside scripts/** need a shape decision, not a copy of ts-parse.mjs — now counted by check:parse-guard, governed by nothing #10606 rather than left in a report.
  • The two non-zero exits are environment, not verdicts, and you said so with each gate's own words rather than reporting a red. That distinction cost me a false alarm on a different gate earlier tonight; naming it is right.

The fence line — announced, as promised

The root package.json line is +1 / −0, next to its sibling check:entry-guard. Verified against the PR's file list. Announced on #9465 at 9465#issuecomment-5365936055 in the established heads-up-with-revert-offer form, before arming — which is the ordering I got wrong on #10555 and have now fixed in procedure.

Your open question — A, ruled. Do not spend a second fence line.

ts-parse.mjs's own 28-case --self-test running in no workflow is a real hole, and your framing of it is exactly right: after this PR, CI enforces "everybody routes through parseSourceFile" and never checks that parseSourceFile still refuses. But an allowance granted for one line is not an allowance for two, and spending it silently would be the worst version of this — your own words, and I agree. It stays with #10608, and I have said on #9465 that if the epic wants it sooner, option C (a lint.yml step invoking the script directly, matching the three sibling steps already in that form, whose comment gives this very fence as the reason) is the shape to take. ⛔ Not B.

#10606 and #10608 filed and triaged.#10608 carries the sharpest item of the three: check:optional-error-sink has an alias in root package.json and no workflow invokes it — so the gate that landed in #10555 has never run in CI at all. That is this card's defect live a second time, and it is a direct consequence of a PR this seat armed.


Generated by Claude Code

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

Labels

ci/cddependenciesPull requests that update a dependency filesize/lskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants

@os-zhuang@claude