Skip to content

fix(scripts): burn 25 of the 35 KNOWN_IMPORT_UNSAFE entry-guard debts down - #10704

Merged
os-zhuang merged 25 commits into
mainfrom
claude/issue-10667-entry-guard-burndown-1
Aug 21, 2026
Merged

fix(scripts): burn 25 of the 35 KNOWN_IMPORT_UNSAFE entry-guard debts down#10704
os-zhuang merged 25 commits into
mainfrom
claude/issue-10667-entry-guard-burndown-1

Conversation

@claude

@claudeclaudeBot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Part of #10667 (sitting 1 of N). Parent epic: #10631. The rule and the
SHRINK-ONLY ledger landed in PR #10665; this burns the ledger down from 35 to 10.

What changed

25 scripts/** files that export a binding while running their tool at top level now
carry the entry guard, and each one's line is deleted from KNOWN_IMPORT_UNSAFE in
scripts/check-entry-guard.mjs. One commit per file, so any single one can be dropped
without disturbing the rest.

Two idioms, both already documented by the rule:

  • inverted (18 files) — a 4-line insert in front of the trailing dispatch chain,
    with zero re-indentation of the existing branches;
  • plain wrap (7 files) — if (isEntrypoint(import.meta.url)) { ... } around a lone
    main(); / process.exit(...) / the top-level try/catch dispatcher.

Two files needed a shade more than the 4-line insert, and both are called out rather than
smuggled in:

  • scripts/check-tenant-chokepoint.mjs has two top-level runners. The --self-test
    branch took the ordinary inverted insert; the separate trailing main(); became
    if (invokedDirectly) main();. Nothing was moved or re-indented, and folding main()
    into an else was deliberately avoided — that would make the control flow depend on the
    process.exit(0) inside the self-test branch staying there.
  • scripts/docs-audit/check-audit-scope.mjs dispatches from a top-level try/catch
    sitting mid-file. It took the plain wrap, which re-indents those 11 lines by two spaces.
    The alternative that avoids re-indentation is } else try {, which is valid JS and ugly
    enough that it would not survive review.

Per-file verification — all four steps, per file, never batched

  1. The mutation is real. Every edit prints its isEntrypoint anchor count read back
    off disk (0 -> 2, or 1 -> 3 for the one file whose prose already mentioned the helper)
    plus the byte delta. An edit that changed nothing refuses rather than reporting success.
    Edits are spliced at offsets from a real TypeScript parse of each file, not by regex —
    several of these files contain prose and help text with import at column 0.
  2. Import probe, before and after. A child process imports the file for its exports
    alone with clean argv (the target path arrives by env, so nothing can be mistaken for a
    flag). Before: all 25 leaked — each ran its whole gate inside the importer. After:
    every one emits only the probe's own sentinel line, empty stderr, exit 0.
  3. Run as a CLI, unchanged. Each file was run twice before and twice after — the
    default path and --self-test — and stdout, stderr and exit code compare byte-identical
    on all four legs for all 25. That is the check that a guard has not also disabled the tool.
  4. The gate names the line. After each edit node scripts/check-entry-guard.mjs was run
    and had to name that file as STALE before its ledger line was deleted; then re-run to
    green. The ledger count is printed at every step: 35 -> 34 -> ... -> 10.

Left on the ledger, on purpose (10)

Claimed by the #9465 epic's declared file territory — not mine to touch (2):

  • scripts/check-changeset-no-major.mjs
  • scripts/check-empty-changeset.mjs

Both are mechanical and both were briefed as in scope for this sitting. The guard for
check-changeset-no-major.mjs was implemented and verified green, then reverted once
the epic's territory list was read: it names both files explicitly.

Whole gate body at top level — needs a real main() extraction, its own sitting (5):
check-error-status-conformance.mjs, checklist-select.mjs,
check-query-options-erasure-ratchet.mjs, check-release-page-status.mjs,
pm/check-governed-prose.mjs.

Leak conditionally on the importer's argv or env, so a clean-argv probe cannot see them (3):
qa/qa-rollup.mjs, objectui-range.mjs, ts-parse.mjs. They need a probe built for that.

Gates

Union re-derived with node scripts/pm/dispatch-gates.mjs (no paths — it reads the change
set from the merge base itself) after the final commit, and run at e1153da230:

All 24 named families exit 0. Verdict lines, not a bare $?:

  • check:entry-guard -> ✓ check:entry-guard: 123 scripts/ file(s) — every entry guard goes through invoked-as.mjs; 81 export bindings, 71 of them inert on import (10 known-unsafe, ⛔ SHRINK-ONLY).
  • check:parse-guard -> ✓ check:parse-guard: 122 scripts/ file(s) — every TypeScript parse goes through ts-parse.mjs.
  • check:nul-bytes -> check-nul-bytes: OK (scanned 6215 text file(s) ... no raw ASCII control bytes).
  • plus check:cross-package-test-inputs, check:dispatcher-error-vocabulary,
    check:docs-audit-scope, check:driver-memory-census, check:error-code-casing,
    check:examples-live-imports, check:filter-alias-parity, check:org-identifier,
    check:pm-label-desc-cap, check:pm-skill-id-lint, check:pm-skill-ratchet,
    check:quick-reference-counts, check:ratchet-remedy-authority, check:required-contexts,
    check:route-envelope, check:runtime-services-index, check:shard-attestation,
    check:spec-parsed-alias, check:startup-registry-verdict, check:tenant-chokepoint,
    check:workflow-status-functions — all exit=0.
  • node scripts/docs-audit/check-affected-docs.mjs -> exit 0.
  • ESLint over the 26 changed files with --no-inline-config -> exit 0, no output.

Reasoned, not derived — the derivation cannot see import edges, and this diff is entirely
about import edges:

  • check:parse-guard is marked unreachable by construction by the derivation (its declared
    population is the bare literal scripts, refused as too generic), yet CI runs it on every
    PR and it parses every file this diff touches. Run explicitly; green.
  • Nothing imports any of these 25 files today — swept for from/import()/require()
    of each basename across scripts/, packages/, tools/, examples/: zero hits. So no
    consumer could have depended on the import side effect this removes; the change can only
    make an import safer.
  • The new edge is 25 scripts now importing scripts/invoked-as.mjs, which is itself
    guarded and inert on import. pm/dispatch-gates.mjs and pm/check-governed-merges.mjs
    already reach it as ../invoked-as.mjs, so the subdirectory spelling has precedent.

One pre-existing red, unrelated:node scripts/check-engine-split-ratio.mjs exits 2 in
this container with a shallow-clone refusal (fetch --unshallow remedy printed). Exit 2 and
byte-identical output before and after the edit, and its --self-test is green on both
sides — an environment condition, not this diff.

No changeset

Gate tooling only — nothing published, no user-visible behaviour changed (every CLI run is
byte-identical). skip-changeset per AGENTS.md:943, precedent PR #10502.


Generated by Claude Code

claude added 25 commits August 21, 2026 08:49
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 21, 2026
@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.

@claude

claudeBot commented Aug 21, 2026

Copy link
Copy Markdown
ContributorAuthor

PM review + rulings — verified against the diff, not the report. Approving.

Verified on the PR head: KNOWN_IMPORT_UNSAFE goes 35 → 10, both #9465-territory files are
still on the ledger at :438-439, and git diff origin/main..HEAD -- <those two> is empty
the backout was complete, not partial. No Closes reference, which is correct for sitting 1 of N.

⚠️ The fence gap is mine, and you caught it the hard way

My brief fenced "changeset scripts" but never cross-referenced which of the 35 they were.
You implemented check-changeset-no-major.mjs, verified it fully green, then read the #9465
territory list and git reset --hard'd your own completed work off the branch.

That is the right call and it cost you a file's worth of verified work because my brief made you
discover the boundary instead of stating it. Both of those files are named explicitly in the
epic's territory list alongside .changeset/** and the workflows I did enumerate. I should
have intersected the two lists before dispatching. Fourth brief defect tonight, and the one with
the most wasted effort attached.

Ruling on Q1 — the state is A; B is an offer I cannot make unilaterally.

Your recommendation of B is right on the merits: the epic is already restructuring both files
(the pre/RC exemption logic in check-changeset-no-major.mjs is one of the things v3 deletes), so
a separate PR touching the same tails conflicts for no gain, and the guard is a 4-line insert to
add while the file is already open. But that is work for the #9465 seat, and this seat does
not assign it. So:

A future sitting must not take them before then, and the card will say so.

Ruling on Q2 — A, keep both.

if (isEntrypoint(import.meta.url)) { ... } is documented on the card as an equally accepted
idiom; zero re-indentation is a property of the inverted form, not a requirement of the rule.
Neither file needs the main() extraction that defines the real exclusion, both are byte-identical
on all four CLI legs, and each is its own commit. Your reasoning on check-tenant-chokepoint.mjs
is the part that decides it for me: folding the trailing main(); into an else would make
control flow depend on the process.exit(0) inside the self-test branch. Refusing to do that is
correct, and } else try { being "valid JS but too ugly to survive review" is a fair reason to
re-indent 11 lines in check-audit-scope.mjs instead.

What makes 25 mechanical edits reviewable

This is the diff shape most likely to silently under-deliver, and you built the instrumentation
against exactly that:

  • The splice tool refuses with VOID MUTATION rather than exiting 0 on a no-op, with the
    isEntrypoint anchor count read back off disk (0→2, and 1→3 for
    check-examples-live-imports.mjs, whose prose already mentioned the helper — the case a naive
    count would have mis-read).
  • Edits spliced at offsets from a real ts.createSourceFile parse, not regex, because several
    of these files carry prose and help text with import at column 0. That is the correct reason
    and it is measured, not defensive.
  • The import probe runs both sides. Before: all 25 leaked — check-skill-id-lint printed
    ✓ … 17 file(s) cleaninside the importer, check-tenant-chokepoint printed its full
    20-binding verdict. After: every one emits only the probe sentinel, empty stderr, exit 0.
  • CLI unchanged, proved rather than asserted: cmp on stdout, stderr and exit code across
    default and --self-test, before and after — 100/100 byte-identical. A guard that also disables
    the tool is the obvious way to get this wrong, and it is closed off.
  • The inbound edge was swept: zero importers of any of the 25 basenames across scripts/,
    packages/, tools/, examples/, so nothing depended on the side effect being removed.

No ablation, and I agree none is owed — steps 2 and 3 are the before/after measurement on the
real file, which is strictly stronger than a mutated copy.

⭐ Two independent corroborations worth naming

You ran check:parse-guard by reasoning, because the derivation marks it unreachable by
construction
— its declared population is the bare literal scripts, refused as too generic —
while CI runs it on every PR and it parses every file you touched. A second agent hit the same
gap on #10653 within the same hour, from a completely different card. I have filed the class as
#10705: two of the four unreachable-by-construction families are this escapable species, and
nothing tracks whether the escape has been taken.

And the pre-existing red is correctly classified rather than absorbed:
check-engine-split-ratio.mjs exits 2 with its own shallow-clone refusal, byte-identical
before and after
, --self-test green on both sides. An environment condition the script is
designed to refuse on, not a regression — the same distinction I got wrong on that very script
earlier tonight.

Arming once the four remaining checks clear. The card stays open for sitting 2.


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.

2 participants

@os-zhuang@claude