Skip to content

fix(scripts): close the three conditional import leaks, shrinking KNOWN_IMPORT_UNSAFE 5 -> 2 - #11951

Merged
os-steve merged 3 commits into
mainfrom
claude/issue-10667-entry-guard-burndown-3
Aug 25, 2026
Merged

fix(scripts): close the three conditional import leaks, shrinking KNOWN_IMPORT_UNSAFE 5 -> 2#11951
os-steve merged 3 commits into
mainfrom
claude/issue-10667-entry-guard-burndown-3

Conversation

@claude

@claudeclaudeBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Part of #10667 — sitting 3 of the KNOWN_IMPORT_UNSAFE burn-down.

Sittings 1 and 2 burned files that leaked unconditionally, so a clean-argv
import probe saw them. These three leak on the importer's own argv or env,
which the earlier probe is structurally blind to. So the probe was the work
here, not the guard insert.

The probe, and why exit status alone is not enough

Sitting 2's exhibit was a gate that re-exec'd and process.exit()'d on import,
returning exit 0 with no sentinel — a silent success indistinguishable from
a clean import. This probe therefore reads three signals per run:

  1. exit status;
  2. a sentinel printed after the dynamic import — absent means the module
    ended or replaced the importer, even at status 0;
  3. foreign bytes on stdout/stderr — a leak that prints without exiting.

All three were load-bearing: one target needs signal 2, one needs signal 3, and
neither alone covers both. Every target is probed twice, once with clean
argv/env and once with the trigger planted.

Before — the probe seeing the leaks

CLEAN scripts/invoked-as.mjs control-inert
LEAK scripts/check-changeset-no-major.mjs control-fenced NO-SENTINEL(status=0) FOREIGN-BYTES=98
LEAK scripts/check-empty-changeset.mjs control-fenced FOREIGN-BYTES=149
CLEAN scripts/objectui-range.mjs clean
LEAK scripts/objectui-range.mjs argv --help NO-SENTINEL(status=0) FOREIGN-BYTES=4666
LEAK scripts/objectui-range.mjs argv -h NO-SENTINEL(status=0) FOREIGN-BYTES=4666
CLEAN scripts/qa/qa-rollup.mjs clean
LEAK scripts/qa/qa-rollup.mjs argv --self-test FOREIGN-BYTES=82
CLEAN scripts/ts-parse.mjs clean
LEAK scripts/ts-parse.mjs env CENSUS=1 FOREIGN-BYTES=102
10 probe run(s): 6 LEAK, 4 CLEAN

The four clean rows are the point: with clean argv/env all three targets
read CLEAN
, which is exactly why the earlier sitting's probe reported nothing
here.

After

10 probe run(s): 2 LEAK, 8 CLEAN

The two remaining LEAKs are the two fenced files, untouched on purpose (see
below). They are the positive control: they hold the LEAK reading open in the
same run that reports the six flips, so no zero here is vacuous.
scripts/invoked-as.mjs is the matching control for the CLEAN reading.

The three mechanisms, each verified rather than inherited

filetriggerwhat actually happenedshape of fix
scripts/qa/qa-rollup.mjsimporter's argv has --self-testran qa-rollup's whole self-test inside the importer; survives, status 0, 82 bytes of foreign stdoutreorder — guard first, mode second
scripts/objectui-range.mjsimporter's argv has -h/--helpdumped a 4666-byte header, then process.exit(0) — importer ends mid-import carrying successmove the block into a function called under the guard
scripts/ts-parse.mjsimporter's env has OS_TOOLING_PARSE_CENSUSregistered an exit hook that writes to the importer's stderrcondition moved, not guarded — see below

All three match what the card described. Nothing was inherited.

ts-parse.mjs: the guard is the wrong fix, and that is the interesting half

ts-parse.mjs is a library — eleven gates in scripts/ import it for its
exports — and as an entrypoint it parses nothing at all. Wrapping the census in
if (isEntrypoint(...)) would arm it on the one run that has nothing to count
and leave it silent on every run that does: import-safe and permanently empty.

So the condition moved instead: the report is armed, once, by the first
parse through any of the three doors. The trigger becomes "this module was
USED" rather than "this module was LOADED", which is the event the number was
always about.

One measurable behaviour change, stated rather than left to be discovered:
a process that imports this module and never parses now prints nothing where it
printed 0 parse(s). OS_TOOLING_PARSE_CENSUS appears in no other file in
the tree, so nothing read that line, and a census whose numerator is zero is
the case with nothing to report.

The self-test pins both directions so neither the leak nor the
over-correction returns unnoticed, and both pins were ablated:

ablation legpredictionobserved
reinstate the top-level registrationthe import-safety case reds1 of 30 case(s) failed…a run that only IMPORTED this module writes no census line at all, detail carrying the leaked [ts-parse census] 0 parse(s)…
delete the three arming call sitesthe feature case reds1 of 30 case(s) failedwith the census env set, a run that PARSED still reports at exit

Both legs ran with a trap … EXIT INT TERM restore and anchored on-disk counts
before and after. The first attempt at leg A was a no-op and is reported as
one
: it reinstated the block with an [ablation] marker while the assertion
keys on the literal [ts-parse census], so it exercised nothing and read green.
Re-run with the exact original bytes, it reds. No build is involved anywhere —
node executes these .mjs sources directly, there is no dist/ for a mutation
to fail to reach.

Behaviour preservation

These are behaviour-preserving edits for the first two files, so identical CLI
output is the proof — cmp, both streams:

surfacebytesresult
qa-rollup.mjs --self-test85 / 0IDENTICAL
objectui-range.mjs --help4693 / 0IDENTICAL
objectui-range.mjs -h4693 / 0IDENTICAL
objectui-range.mjs (no args, status 1)0 / 93IDENTICAL
objectui-range.mjs --self-test1578 / 0IDENTICAL
ts-parse.mjs (no args)45 / 0IDENTICAL
ts-parse.mjs --self-testDIFFERS, by design: 30 cases pass where it read 28. The case count is the only difference in the line.

One trap worth recording: objectui-range.mjs's help text is read back out of
its own column-0 // lines (all 79 of them, not just the header), so any
// comment added at column 0 would silently rewrite --help. The new
rationale is a /** */ block for that reason, and the column-0 // count is
still 79.

The ledger shrank 5 → 2, one line per commit

check:entry-guard named each file STALE before its line was deleted, and
each fix carries its own ledger line in the same commit so a revert stays
self-contained. The gate's own count moved
107 of them inert on import (5 known-unsafe)110 of them inert on import (2 known-unsafe).

The remaining two — scripts/check-changeset-no-major.mjs and
scripts/check-empty-changeset.mjs — are fenced: both sit in the declared
file territory of the changesets-v3 epic (#9465), which is open. Not touched.
Verified against that epic's territory list before any edit.

Gates

Derived live with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (4 paths, three-dot against merge base ce744bcdf),
then all nine families run. Union run at e253ab930, the final commit:

[0] pnpm check:agent-test-spelling ✓ 0 violations — 366 file(s) · 3676 bare `--` token(s) …
[0] pnpm check:cross-package-test-inputs OK: 16 package(s) read outside themselves, all declared …
[0] pnpm check:entry-guard ✓ 156 scripts/ file(s) … 110 of them inert on import (2 known-unsafe)
[0] pnpm check:objectui-changeset ✓ objectui-range --self-test: all checks passed
[0] pnpm check:parse-guard ✓ 155 scripts/ file(s) — every TypeScript parse goes through ts-parse.mjs.
[0] pnpm check:pnpm-filter-targets ✓ 135/168 `--filter` occurrence(s) across 26 file(s) resolve …
[0] node scripts/check-ci-filter-parity.mjs OK: all 96 declared cross-package glob(s) …
[0] node scripts/check-cross-package-test-inputs.mjs OK: 16 package(s) read outside themselves …
[0] node scripts/ts-parse.mjs --self-test ✓ 30 cases pass …

Also run, though not in the derived family: pnpm lint (repo-wide,
eslint . --no-inline-config) exit 0 in 53s, and check-nul-bytesOK (scanned 6654 text file(s) … no raw ASCII control bytes).

skip-changeset applied: this PR publishes nothing. It changes only the
import-time behaviour of three scripts/ tools and shrinks a gate's internal
debt ledger; no package's behaviour or public surface moves.

Generated by Claude Code


Generated by Claude Code

`qa-rollup.mjs` exports bindings and tested `--self-test` BEFORE the entry
guard, so the branch read the IMPORTER's argv: any tool that imported this
module for its exports while carrying `--self-test` in its own argv ran
qa-rollup's entire self-test inside itself.
The leak is invisible to the two signals a caller usually has. The self-test
does not exit on success, so the importer survives with status 0 and finishes
its own work; the only trace is 82 bytes of foreign output on the importer's
stdout. Measured with a probe that prints a sentinel AFTER the dynamic import
and counts bytes that are not the sentinel:
before LEAK scripts/qa/qa-rollup.mjs argv --self-test FOREIGN-BYTES=82
after CLEAN scripts/qa/qa-rollup.mjs argv --self-test
Guard first, mode second. CLI behaviour is unchanged — `--self-test` output is
byte-identical before and after (85 bytes, `cmp` clean), because the guard is
true for every direct invocation.
`check:entry-guard` names the file STALE once it is inert, so its
KNOWN_IMPORT_UNSAFE line goes in this same commit: the ledger shrinks 5 -> 4
and the gate's own count moves to `108 of them inert on import (4
known-unsafe)`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ahemw8RcTgqtxrj15PEZx
`objectui-range.mjs` exports bindings and answered `-h`/`--help` from a bare
top-level `if`, so the test read the IMPORTER's argv. An importer carrying
either flag got this file's 4666-byte `//` header written to its stdout and
then `process.exit(0)`.
That is the worst-reading shape in this class: the importer's process ends
mid-import with a SUCCESS status, so a caller holding only `result.status`
cannot tell it from a clean import. Measured with a probe that prints a
sentinel AFTER the dynamic import — the sentinel is what makes the two
distinguishable, exit status alone is not:
before LEAK argv --help NO-SENTINEL(status=0) FOREIGN-BYTES=4666
before LEAK argv -h NO-SENTINEL(status=0) FOREIGN-BYTES=4666
after CLEAN argv --help
after CLEAN argv -h
The help text is unchanged, and deliberately so: it is read back out of this
file's own `//` lines, so a comment added at column 0 would rewrite it. The
new rationale is a `/** */` block for that reason and the column-0 `//` count
is still 79. All four CLI surfaces are byte-identical before and after —
`--help` 4693 bytes, `-h` 4693, no-args 93 bytes on stderr with status 1, and
`--self-test` 1578 bytes, every one `cmp`-clean on both streams.
`check:entry-guard` names the file STALE once it is inert, so its
KNOWN_IMPORT_UNSAFE line goes in this same commit: the ledger shrinks 4 -> 3.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ahemw8RcTgqtxrj15PEZx
`ts-parse.mjs` registered its `OS_TOOLING_PARSE_CENSUS` exit report from a
top-level `if (process.env...)`. This module is a LIBRARY — eleven gates in
`scripts/` import it for its exports — so with the env set, that registration
ran inside every one of those importers and wrote a line to a process whose
only involvement was having loaded it.
The entry-point guard is NOT the fix here, and that is the interesting half.
As an entrypoint this module parses nothing, so `if (isEntrypoint(...))` would
arm the census on the one run that has nothing to count and leave it silent on
every run that does — import-safe and permanently empty. The condition had to
MOVE, not acquire a guard: it is now armed, once, by the first parse through
any of the three doors.
One measurable consequence, stated rather than left to be discovered: a process
that imports this module and never parses now prints nothing where it used to
print `0 parse(s)`. Nothing read that line — `OS_TOOLING_PARSE_CENSUS` appears
in no other file in the tree — and a census whose numerator is zero is the case
with nothing to report.
The self-test pins BOTH directions, so neither the leak nor the over-correction
returns unnoticed: a child that parses still gets `[ts-parse census] 1 parse(s)`
on stderr, and a child that only imports gets no census line. The child harness
grew an optional env argument for it.
Measured with a sentinel-after-import probe:
before LEAK scripts/ts-parse.mjs env CENSUS=1 FOREIGN-BYTES=102
after CLEAN scripts/ts-parse.mjs env CENSUS=1
CLI: `node scripts/ts-parse.mjs` is byte-identical (45 bytes). `--self-test`
is NOT, by design — it reports `30 cases pass` where it reported `28`, and the
case count is the only difference in the line.
`check:entry-guard` names the file STALE once it is inert, so its
KNOWN_IMPORT_UNSAFE line goes in this same commit: the ledger shrinks 3 -> 2
and now holds only the two files fenced by the changesets-v3 epic.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ahemw8RcTgqtxrj15PEZx
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 25, 2026
@os-steveClaude

Copy link
Copy Markdown
Collaborator

ACCEPT, and option A on the open question — confirmed on my own measurement, not on yours. Flipping out of draft (scripts/** is not governed); arming once green.

Verified against the branch

claimcheckresult
ledger 5 → 2the diff✅ exactly 3 lines removed, zero added
the 2 left are the fenced pairthe diffcheck-changeset-no-major.mjs, check-empty-changeset.mjs — nothing else
scopegit diff --stat✅ 4 files, all in scope

⭐ The ruling rests on a fact I re-measured, because it is the whole argument

Option A drops the [ts-parse census] 0 parse(s) line. That is only safe if nothing reads it:

OS_TOOLING_PARSE_CENSUS → scripts/ts-parse.mjs : 2 (its own file, and only its own)
CONTROL isEntrypoint → many files, 2 hits each (the grep discriminates)

One file, itself. So the reading you removed had no consumer, and A costs nothing real. B would reinstate the exact defect this card exists to remove; C buys back an unconsumed reading at eleven call sites that can rot individually. Shipping A and flagging it was right, and the revert path being one commit is what makes flagging it sufficient rather than requiring a pre-ruling.

⭐ The ts-parse.mjs call is the finding of this sitting

The other two took a guard. ts-parse.mjs did not, and your reason is the kind of thing a mechanical burn-down gets wrong:

it is a library imported by eleven gates and parses nothing as an entrypoint, so isEntrypoint() would have armed its census on the one run with nothing to count and silenced it on every run that has something

A guard there would have been actively inverted — green ledger, dead diagnostic. "Move the condition to first-parse" is a different fix, and noticing that the obvious one is backwards is worth more than the three lines removed. The dispatch asked you not to assume a guard was right for all three; this is what that question was for.

The probe, and the two-sided proof

A clean-argv probe reads all three CLEAN — which is exactly why sittings 1 and 2 could not see them, and why the probe was the work. Three signals (exit status, a sentinel printed after the dynamic import, foreign bytes) is the right instrument, and the readings show why each arm is load-bearing:

  • objectui-range --help / -h: NO-SENTINEL, status 0, 4666 foreign bytes — the silent-success shape, status 0 with the importer already dead. Exit code alone would have called it clean.
  • qa-rollup --self-test: sentinel PRESENT, caught only by the foreign-byte arm.

Two different failure shapes, each invisible to the arm that caught the other. And BEFORE 6 leak / 4 clean → AFTER 2 leak / 8 clean, with positive controls in the same run holding both readings open in both directions (invoked-as.mjs for CLEAN, the two fenced files for LEAK).

⭐ Reporting the vacuous ablation leg

the FIRST attempt at leg A read green and was vacuous — I reinstated the block with an [ablation] marker while the assertion keys on the literal [ts-parse census], so the mutation landed on disk but exercised nothing

A green ablation leg that proves nothing is indistinguishable from a passing one, and you caught it by noticing the marker did not match what the assertion reads. That is the fourth time tonight a dev has discarded their own reading rather than bank it, and it is the single habit that has made this shift's measurements worth trusting.

ts-parse --self-test output differing by exactly the case count (28 → 30) is the right kind of difference to declare rather than paper over.

Out-of-scope handling, both correct

#11952 filed — objectui-range --help builds usage from all 79 column-0 // lines, so six mid-file implementation comments print as usage and any future column-0 comment silently rewrites the CLI's help. That your own rationale had to be a /** */ block precisely so --help stayed byte-identical is the sharpest possible demonstration of the defect, and pinning the count at 79 stops the next person tripping it.

Checked-and-not-filed on check-entry-guard.mjs skipping DECLARATION_HEAD — recording that you probed it and found it deliberate and already pinned is exactly right. A future sitting would otherwise have re-opened it as a hole and spent a cycle.

⚠️ After this lands, this card is BLOCKED, not queued

The 2 remaining entries are both inside #9465's territory and #9465 is still open, so there is no sitting 4 until that epic lands or its seat hands the files over. When #11951 merges I will strip pm:dispatched by hand (it is a Part of card) and set pm:blocked with Blocked-by: #9465 rather than returning it to the pool, so no seat takes a card whose entire remaining scope is fenced.


Generated by Claude Code

@os-steve
os-steve marked this pull request as ready for review August 25, 2026 02:34
@os-steve
os-steve added this pull request to the merge queueAug 25, 2026
Merged via the queue into main with commit 8e39bbcAug 25, 2026
32 checks passed
@os-steve
os-steve deleted the claude/issue-10667-entry-guard-burndown-3 branch August 25, 2026 03:12
yinlianghui pushed a commit that referenced this pull request Aug 25, 2026
…refix
--help built its usage text by keeping every column-0 `//` line in the
whole file, not just the header. Six mid-file implementation comments
(the pinAt() helper's rationale, and this file's own self-test section
banner) leaked into the usage text a reader pastes.
The header IS a contiguous prefix of the file (right after the
shebang), so printHelp() now walks lines in order and stops at the
first non-`//` line once the block has started -- a takeWhile, not a
filter -- so a column-0 comment added anywhere later in the file can
never reach --help again.
Pinned as a self-test assertion (the file's own #4843 idiom): --help
still carries the real header, and none of the six measured stray
lines. Reverse-verified: mutating printHelp() back to the old
unbounded filter flips the new pin red; rebuilding the fix restores
green.
column-0 `//` count is unchanged at 79 -- the new rationale uses a
`/** */` block, per PR #11951's byte-identity concern for this file.
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-steve@claude