Skip to content

fix(tooling): make an unparseable source REFUSE instead of scoring clean - #10573

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-10133-ts-createsourcefile-parse-errors
Aug 21, 2026
Merged

fix(tooling): make an unparseable source REFUSE instead of scoring clean#10573
os-zhuang merged 1 commit into
mainfrom
claude/issue-10133-ts-createsourcefile-parse-errors

Conversation

@claude

@claudeclaudeBot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Fixes#10133

ts.createSourceFile never throws. A syntax error — or the wrong ScriptKind — returns a SourceFile built by error recovery, with the errors parked on parseDiagnostics, a property that nothing under scripts/ read. So a gate walks the wreckage, finds none of the shapes it is looking for, and scores the file clean: a file the gate could not read, reported as a file with nothing to report.

The card said "nothing is red today". That was wrong, and the measurement is the point

check-engine-double-contract.mjs walks 2504 *.{test,spec}.{ts,tsx,mts} files under packages/ and examples/ while forcing ts.ScriptKind.TSX on every one of them. Under TSX an angle bracket opens a JSX element, so an ordinary generic type argument — new Map with its two type parameters, a generic arrow — made the rest of the file wreckage. 32 of those 2504 files parsed with errors, up to 633 diagnostics in one file, and the gate printed:

check-engine-double-contract: OK — 342 pinned, 133 in the DEBT ledger, 2 exempt.

Read under the ScriptKind their own file names imply, the same 2504 files give 6 problem(s). Nothing about the tree changed; only whether the gate could read it:

forced TSX (main today)ScriptKind inferred
delete doubles236 in 215 files239 in 218
update doubles272 in 246 files275 in 249
pinned (delete / update)170 / 172173 / 175
verdictOK (exit 0)6 problem(s) (exit 1)

The six are RETAINED findings on three files — meta-object-primary-designation-roundtrip, meta-object-search-companion-roundtrip, meta-object-tenant-index-roundtrip — each pinning engine doubles the ledger had never recorded, because the scan had never been able to see them. --write records them: the pinned ledger goes 329 rows to 335, 6 added or grown, 0 lost, and the seam ledger is unchanged (0 added, 0 lost).

What landed

  • scripts/ts-parse.mjs — the one sanctioned parse. Reads parseDiagnostics and refuses, naming the file, the line:column and TypeScript's own message. It exits (code 3) rather than throwing: try { createSourceFile } catch { continue } is already written twice in packages/lint/src, dead code against a throw that cannot happen and a silent skip the moment one arrived. Code 3 is deliberately not 1 — "could not read the tree" is a different verdict from "found violations".
  • scripts/check-parse-guard.mjs — the half that makes it hold. A raw ts.createSourceFile anywhere in scripts/** outside the parser home is a failure. Comments and string / template / regex literals are masked first (js-comment-mask.mjs), so the helper's own documentation is not read as a call site.
  • All 32 call sites across the 15 gates converted. The three engine-double-contract sites that forced TSX over real files now let the file name decide; every other call site keeps the ScriptKind it named.
  • engine-double-contract.pinned.json grows by the 6 rows that became visible.

Shared helper vs per-gate — the ruling, and why

Shared helper, plus a gate that removes the second spelling. The real objection to a helper is that it becomes a second source of truth while the first one is still reachable — so the answer is to remove the first source, not to argue about drift. That is not a theory about this tree; it is the tree's own result twice over, both in scripts/:

  • invoked-as.mjs replaced eleven hand-typed spellings of "was I run?" across 33 files, nine of them wrong — and check-entry-guard.mjs exists because the one-time sweep did not stop a twelfth.
  • js-comment-mask.mjs replaced two private stripComments families, each silently wrong in a different direction.

Fifteen independent copies of "and check the diagnostics" drift the same way: one reads .length on a field it forgot can be undefined, one warns instead of failing, one is simply never typed into the sixteenth gate — and a missing copy is invisible, because its symptom is a green line.

How the entry-guard constraint shaped it.check-entry-guard.mjs bans process.argv[1] in scripts/** outside invoked-as.mjs, verified here (node scripts/check-entry-guard.mjs121 scripts/ file(s), exit 0). Three consequences, designed in from the start rather than discovered:

  1. ts-parse.mjs has a --self-test CLI mode, so it needs an entry guard, and it may not type its own. It imports isEntrypoint from invoked-as.mjs.
  2. check-entry-guard.mjs is the template for the enforcing gate — including the literal-masking, which is load-bearing here for a reason it is not there: this helper's header discusses the banned call at length, so an unmasked scan would fabricate findings out of its own documentation.
  3. It settled the scope boundary. invoked-as.mjs explains why packages/cli/src/utils/invocation.ts is a deliberate duplicate rather than an import: scripts/ runs as plain .mjs against a possibly unbuilt tree. The same argument applies unchanged, so this covers scripts/** and the package-side parses are filed rather than swept in.

Is any gate hiding a real unparseable file today?

Yes — one was, and it is fixed here. After the fix, no. The sweep was run, not assumed:

  • A repo-wide parse of all 4591.ts/.tsx/.mts/.cts files: 0 have parse errors under the ScriptKind their extension implies. 40 are wreckage under forced TSX and 15 under forced TS — which is why the ScriptKind a call site pins is part of this defect and not a separate one.
  • With OS_TOOLING_PARSE_CENSUS set, the 15 converted gates perform 5157 parses and 0 refusals — per-gate distinct file names summing to 3251 (a name shared between two gates counts once in each, so that is a per-gate denominator, not a repo-wide set). Every gate now prints its own numerator and denominator.

Positive control for that zero, because a zero-hit with no control is not a measurement: restoring the ScriptKind.TSX that main ships today makes the refusal fire immediately on the unmodified tree —

x ts-parse — REFUSING to scan a source that does not parse.
file packages/mcp/src/__tests__/plugin.test.ts
parsed as TSX
errors 147 parse diagnostic(s) from TypeScript 6.0.3

— a file main's gate says nothing about while printing OK.

Reproduced harm, and the ablations

Harm, on a real gate over a real population. A genuine syntax error injected into packages/qa/dogfood/test/read-coercion-conformance.test.ts (in check-verify-stand-in-erasure's scan set):

gateexitoutput
origin/main0OK … 10 call site(s) reached — zero mentions of the file
this branch3refuses, names the file, line:column, TypeScript's message

The refusal is the load-bearing part, isolated. Same converted gate, same poisoned tree, with only if (diagnostics.length > 0) neutered in ts-parse.mjs: exit 0 and the identical green OK … 10 call site(s) reached.

A sub-finding worth recording: the count does not reliably move. TypeScript's recovery kept all 10 call sites through 27 parse diagnostics. Controlled fixtures show it can go either way — an unterminated template literal took a three-site file to one site, while a broken statement left all three. That is exactly why the silent green is the dangerous half: you cannot tell from the number whether anything was lost.

Every assertion added was ablated:

ablationexpectedobserved
re-introduce a raw call in check-kernel-hook-pairs.mjsguard red, names the fileexit 1, scripts/check-kernel-hook-pairs.mjs:108
break the guard's detector, same violation presentguard green (assertion is load-bearing)exit 0, identical green line
neuter the refusal in ts-parse.mjspoisoned tree scores cleanexit 0, OK … 10 call site(s)
restore ScriptKind.TSX in engine-double-contractrefuses on the real treeexit 3, names plugin.test.ts

Each mutation was confirmed on disk by anchor count before the measurement was read, and each restore by git hash-object plus an empty git status --porcelain.

Gates

19 families from node scripts/pm/dispatch-gates.mjs (no path arguments), run at d7b687604b, plus check:nul-bytes, check:entry-guard and both new self-tests: all green, one environment refusal — node scripts/pm/check-half-states.mjs exits 3 with PREREQUISITE NOT MET — the token in the environment is not a valid GitHub token, unrelated to this diff; its pnpm check:pm-half-states form passes 715 cases.

check:parse-guard: 120 scripts/ file(s) — every TypeScript parse goes through ts-parse.mjs.
ts-parse self-test: 17 cases pass
check:parse-guard self-test: 13 cases pass
check-engine-double-contract: OK — 348 pinned, 133 in the DEBT ledger, 2 exempt.

Left undone, deliberately — needs a decision

  1. check:parse-guard is not wired into CI. That needs a check:* entry in root package.json and a step in .github/workflows/lint.yml. Root package.json is fenced (Migrate the release toolchain to @changesets/cli v3 — one atomic PR carrying the bump, the pre-mode restructure, and the gates that model v2's semantics #9465) and lint.yml is claimed by in-flight work (lint.yml's "Build the ledgered packages' dependencies" step duplicates the closure refresh once #8330 lands #8331), so this PR does not touch either. Until it is wired, the gate is dormant and a sixteenth raw call would land unnoticed — the conversions and the refusal are fully live regardless.
  2. The parses outside scripts/** are not covered and are filed: packages/lint/src (3 sites, two of them behind the dead try/catch cited above), packages/lint/scripts/check-doc-formula-expressions.mjs, packages/spec/scripts/check-skill-examples.ts, packages/spec/scripts/lib/strictness-ledger.ts, packages/cli/src/utils/detect-free-identifiers.ts.
  3. ts.createProgram and ts.transpileModule carry the same class through different APIs and are named in the gate header as deliberately not covered, so its green line does not over-claim.

Changeset

None. The whole diff is root scripts/**; the root package is private: true and every published package ships only dist, README.md and CHANGELOG.md. Nothing here reaches a publish surface — measured at diff time, not defaulted. skip-changeset.

Merge-order note — one live overlap, reported rather than absorbed

#10562 (claude/route-envelope-decision-y3itw6) is open against scripts/check-route-envelope.mjs, +312/-24 — the same file this PR converts (3 call sites plus an import). Whichever lands second should re-run node scripts/check-parse-guard.mjs: if #10562's 312 added lines introduce a raw ts.createSourceFile, the guard will name it, and the fix is the one-line conversion this PR already applies three times in that file. Nothing here was changed to accommodate it.

Follow-ups filed: #10574 (wire check:parse-guard into CI — blocked on the two claimed surfaces above) and #10575 (the same class outside scripts/ts-parse.mjs's reach: ts.createProgram, ts.transpileModule, and 10 sites under packages/).


Generated by Claude Code


Generated by Claude Code

`ts.createSourceFile` never throws. A syntax error — or the wrong
`ScriptKind` — returns a `SourceFile` built by error recovery with the
errors parked on `parseDiagnostics`, a property nothing under `scripts/`
read. Fifteen gates walked TypeScript that way, so any of them could
report a confident zero about a file it never managed to read.
This was not latent. `check-engine-double-contract.mjs` forced
`ts.ScriptKind.TSX` on all 2504 `*.{test,spec}.{ts,tsx,mts}` files under
`packages/` and `examples/`; in TSX a `<` opens a JSX element, so an
ordinary `new Map<string, X>()` made the rest of the file wreckage. 32 of
those 2504 parsed with errors (up to 633 diagnostics in one file) and the
gate printed `OK — 342 pinned, 133 in the DEBT ledger, 2 exempt`. Read
under the ScriptKind their own names imply, three of those files turn out
to pin six engine doubles the ledger had never recorded.
- `scripts/ts-parse.mjs` — the one sanctioned parse. Reads the
diagnostics and exits 3 (distinct from 1: "could not read the tree" is
not "found violations"), naming the file, the line:column and
TypeScript's own message. It exits rather than throws because
`try { createSourceFile } catch { continue }` is already written in
`packages/lint/src`, against a throw that cannot happen.
- `scripts/check-parse-guard.mjs` — the half that makes it hold. A raw
`ts.createSourceFile` anywhere in `scripts/**` outside the parser home
is a failure, so there is no second spelling left to drift from. Same
shape as `invoked-as.mjs` + `check-entry-guard.mjs`, and for the same
measured reason.
- All 32 call sites across the 15 gates converted; the three
engine-double-contract sites that forced TSX over real files now let
the file name decide.
- `engine-double-contract.pinned.json` grows by the 6 rows that became
visible: 342 -> 348 pinned, 0 lost.
Fixes: #10133
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 — ⭐⭐ accepted. You falsified the card's central claim, and the thing it was hiding is the biggest find of the night.

⛔ "Nothing is red today" was wrong, and the gate was hiding real pins

check-engine-double-contract.mjs forced ts.ScriptKind.TSX over all 2504 of its test files, and 32 of them were wreckage under it (up to 633 diagnostics) while the gate printed OK — 342 pinned.

⇒ Letting the file name pick the ScriptKind turns that green into 6 problems, and three files pin six engine doubles the ledger had never recorded. engine-double-contract.pinned.json goes 329 → 335 rows, 6 added or grown, 0 lost; pinned 342 → 348.

⭐ That is not a latent gap. A gate that exists to pin engine doubles was mangling its own inputs into wreckage and reporting OK over them, and six real pins were invisible for as long as that has been true. The card said the class had no live instance; measurement found one inside the very gate the card listed first.

⚠️ And the in-place ScriptKind correction was forced, not opportunistic — you say so plainly: "the refusal cannot land in that gate while it mis-parses 32 real files." Declared in the PR body with its evidence, and the ledger regeneration shows 0 lost, which is the number that matters when a pinned set moves.

⭐ The sub-finding that reframes the whole class

The COUNT does not reliably move. TypeScript recovery kept all 10 call sites through 27 parse diagnostics; controlled fixtures show both directions. The guaranteed harm is the silent green, not a smaller number — which is exactly why the number cannot be trusted.

⭐ That is the sharpest statement anyone made tonight about this defect family, and it retroactively justifies every "refuse rather than under-report" ruling this repo has landed this week. A shrinking count is a symptom that may not appear; the silent green is the invariant. ⇒ Anyone who tries to detect this class by watching totals will find nothing and conclude there is nothing.

What I checked hardest

  • Population re-derived and it was under-counted: 15 files, but 32 raw call sites — the card gave only the file count. Positive control: all 15 named files appear. Negative controls pinned in the self-test: a line comment, block comment, string payload and template payload naming the call are not findings.
  • ⭐⭐ Ablation 2 is the load-bearing one: breaking the guard's detector regex with the violation still present → EXIT=0 and the identical green line. So the assertion is doing work rather than decorating. Ablation 3 isolates the refusal itself the same way.
  • You caught your own no-op ablation and said so: reverting only the censusRecognizer call site read EXIT=0, "that path runs only under --census, so the reading was a no-op, not a refutation" — re-run against the scanSource site it gave EXIT=3. An ablation that cannot fire is indistinguishable from a passing one; catching that is the difference between evidence and coincidence. That is the fourth dev tonight to catch its own instrument before reporting.
  • Exit, not throw"because packages/lint/src already carries two try/catch wrappers that would turn a throw into a silent skip." Choosing the failure mode by reading what would swallow it is exactly right for this class, and exit 3 distinct from 1 keeps "could not read" separable from "read and found problems".
  • check-parse-guard.mjs answers the helper-vs-per-gate question properly: a helper plus a guard that prevents drifting back to raw calls — "removing the first source a helper could drift from." A shared helper with nothing stopping the next author bypassing it is a helper with a half-life.
  • Post-fix census with its own positive control: 5157 parses, 0 refusals across all 15 gates — and restoring ScriptKind.TSX makes the refusal fire immediately on the unmodified tree. A zero that can be made non-zero on demand.
  • ✅ Entry-guard constraint verified rather than inherited, and it shaped three concrete decisions.

Follow-ups — all three correctly placed

#10574check:parse-guard unwired because it needs root package.json (#9465 fenced) and lint.yml (claimed by #8331). ⚠️ Both blockers clear shortly: #10568 (#8331) is green and about to be armed, and I have announced this lane's root-package.json edits on #9465 in the established heads-up form. ⇒ Worth re-checking #10574 within the hour rather than treating it as a long hold. ⛔ The conversions and the refusal are live regardless; only the anti-regression guard is dormant.

#10575 — the same class outside ts-parse.mjs's reach: ts.createProgram, ts.transpileModule, plus 10 sites under packages/ — ⭐ including two behind a try/catch that would become a silent skip, which is the identical trap you designed around here.

⚠️PR #10562 overlap reported, not absorbed — it rewrites check-route-envelope.mjs, a file this PR converts. Whoever lands second re-runs check-parse-guard. Correct call.

⚠️ Platform note taken: two attribution footers in the PR body, yours plus one the platform appended. ⛔ Leaving it rather than "correcting" it is right — editing a body to fix a duplicate footer risks the sanitizer eating something load-bearing, which has already happened twice tonight.

CI is finishing. ⚠️ GraphQL quota is exhausted until 03:59Z, so the arm will lag the green by a couple of minutes; five other PRs are queued behind the same wait.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 21, 2026 04:00
@os-zhuang
os-zhuang added this pull request to the merge queueAug 21, 2026
Merged via the queue into main with commit 0eca1c8Aug 21, 2026
35 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-10133-ts-createsourcefile-parse-errors branch August 21, 2026 04:22
os-zhuang pushed a commit that referenced this pull request Aug 21, 2026
…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
os-zhuang pushed a commit that referenced this pull request Aug 21, 2026
…ER.md, with a parser that refuses rather than fails open
`check-platform-checklist` enforced a closed vocabulary for `status`,
`priority`, `surface`, `oracle` and `blocked.by` — and never read `traps`
at all. So an item could carry any trap name and the validator stayed
green, which is how eight undocumented values drifted in (#10416), and
how a TYPO in a documented one (`hydration-races` for `hydration-race`,
which is on 79 of the 205 items) lands as simply a twentieth trap that
no runner rules out.
The vocabulary is READ from RUNNER.md's `### Trap vocabulary` table, not
copied into the script: a sixth hardcoded `Set` would only move the
drift one level up, between the script and RUNNER.md, with nothing
watching that seam. The load-bearing part is therefore not the parser
but its positive control — `extractTrapVocabulary` refuses on a table it
cannot recognise (heading renamed, table moved, zero rows, a row that
lost its backticks) instead of returning an empty allow-list, and a
22-assertion fixture battery proves the refusal still fires. The battery
runs inline on every invocation, not only behind `--self-test`, because
this gate is not CI-wired by maintainer decision and its `pnpm` alias
lives in the fenced root package.json (#9465) — a self-test nothing runs
is the #10574/#10573 defect. The OK line now states what the parse read:
`traps: 19 documented, 19 in use (extractor control: 22 assertions)`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

2 participants

@os-zhuang@claude