Skip to content

fix(devx): read escaping IMPORT specifiers in the cross-package input gate, and gate the hand-written .d.mts mirrors - #10607

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-10452-cross-package-import-specifier
Aug 21, 2026
Merged

fix(devx): read escaping IMPORT specifiers in the cross-package input gate, and gate the hand-written .d.mts mirrors#10607
os-zhuang merged 1 commit into
mainfrom
claude/issue-10452-cross-package-import-specifier

Conversation

@claude

@claudeclaudeBot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Fixes#10452
Fixes#10549

Two halves of the same undeclared coupling: the gate that decides what a test's
real inputs are could not see an escaping import, and the file at the centre
of that import had a hand-written type mirror with nothing keeping it honest.

All evidence below was measured on this branch at 505e9dd905.


#10452 — the collector now reads escaping import specifiers

check-cross-package-test-inputs recognised only path-shaped file reads seeded
from import.meta.url / __dirname. An ES module specifier is none of those:
it is a bare string in import position that the module resolver, not
node:path, turns into a file. The file said so as its stated boundary —
"Reads that reach another package through Node's RESOLVER rather than through
fs are outside this gate entirely."
That sentence is what this PR removes.

Specifiers are now walked by the same walkLiteral, in the same two
coordinates, judged on the same shallowest point reached. The recognised
spellings are published as RECOGNISED_IMPORT_SPELLINGS and printed in the
failure text, beside the path list, under the same rule: a spelling that is not
there yields no flag.

The boundary that makes it safe. Only specifiers starting ./ or ../ are
collected. A bare specifier (@objectstack/verify, node:fs, vitest) is an
installed dependency resolved through node_modules — the same exclusion
walkLiteral's vendored flag already makes, for the same reason: no turbo
glob can name it, and collecting them would put every package's suite on every
workspace sibling. Four self-test cases pin that direction rather than one.

Reproduction — the pre-change gate is blind, positively controlled

Zero-hit results need a positive control, so there are two, both run with the
origin/main gate placed inside this tree so its REPO_ROOT resolves here.

Control A — a constructed escaping import in a package with no declaration at
all
(a temporary test in packages/services/service-i18n importing
../../../../scripts/js-comment-mask.mjs):

gateexitverdict
origin/main0OK: 12 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.
this branch1@objectstack/service-i18n has test(s) that read outside the package but declares no input radius.

Control B — the real specimen from #10452. With only the hand-added
scripts/js-comment-mask.mjs glob removed from @objectstack/cli's roster:

gateexitverdict
origin/main0OK: 12 package(s) read outside themselves, all declared, …
this branch1scripts/js-comment-mask.mjs (named in packages/cli/src/commands/serve-audit-registration.contract.test.ts)

The hand declarations became ENFORCED, not redundant

The card asked whether PR #10450's hand-added declarations become redundant.
They do not, and could not: the roster is what the gate checks a declaration
against, so the gate never writes a declaration for you. What changed is that
they stopped being voluntary. Before, cli's scripts/js-comment-mask.mjs
glob was a courtesy nothing demanded; now removing it turns the gate red (table
above). They are kept, and both are now derived rather than trusted.

One nuance worth recording, since it partly qualifies the card's own
measurement: on today's tree, removing both hand declarations reddens the
origin/main gate too — but on scripts/js-comment-mask.d.mts, which a second
cli test names in prose and the flat literal collector has always seen. The
.mjs import itself — the actual subject of #10452 — is invisible to the
pre-change gate either way, which is what Control B isolates.

What the new half found

Six couplings nothing had ever declared, all real inputs:

Each is declared per-file rather than by subtree, and mirrored into turbo.json
so Layer B hashes them.

A defect found while verifying, and fixed here

resolveImportTarget judged "extensionless" with !/\.[A-Za-z0-9]+$/, i.e.
does the last segment contain a dot. This repo's authored metadata is
contact.view.ts / semantic-zoo.object.ts / task-triage.page.ts, imported
as …/views/contact.view — read as already having a .view extension, so no
candidate was tried and the specifier resolved to nothing. Measured: that is
exactly the three packages/cli i18n-coverage imports, whose globs were on the
roster by hand. It now tests against the known module extensions.

Ablation, both resolvers on a tree with that one glob dropped:

resolverpackages named
dot-segment (as recovered)@objectstack/lint only
known-extension (this PR)@objectstack/cliand@objectstack/lint

Self-test cases

The gate's own instruction is a case per spelling, and the issue made it part of
acceptance. 60 → 79 cases: one per entry in RECOGNISED_IMPORT_SPELLINGS
(static, import type, export … from, export * from, side-effect, dynamic
await import, require), four boundary cases pinning bare specifiers not
flagged, and naming cases pinning each extension rule against a real file on
disk.

Reverse-verified: with the import half disabled (if (true) continue in place of
the relative-specifier test, injected and confirmed on disk by anchor count),
12 of 79 cases fail and the live gate goes red on the now-stale
@objectstack/client declaration. The 7 new cases that stay green under that
ablation are exactly the "does NOT flag" boundary ones, which are correctly still
true — that asymmetry is the expected direction, not a gap. Restore confirmed on
disk (git diff --stat empty, anchor counts inverted).


#10549scripts/check-declaration-mirrors.mjs

⚠️Not framed as an oversight.js-comment-mask.d.mts was landed
deliberately by #10398 and the hand-maintenance is its stated design. Nothing
here contradicts that trade — the modules stay .mjs because pre-commit and
the gates invoke them with bare node. What this adds is the check that the
trade always assumed and never had.

The sweep the card deliberately left undone (git ls-files 'scripts/**/*.d.mts')
returns two files: check-regen-pending.d.mts and js-comment-mask.d.mts.
The gate does not list them — it discovers every scripts/**/*.d.mts, so a
third mirror added tomorrow is covered by existing.

Per declared export it asserts NAME (a declared export the module does not
export — the fail-green direction), KIND (export function must be a
function at runtime) and required ARITY against Function.length.

Deliberately not asserted: parameter and return types. maskComments
returning string[] where the declaration says string is invisible to any
runtime check, and a gate overstating its coverage is worse than one stating its
limit. Likewise a module export the declaration omits is not fatal:
check-regen-pending.mjs exports seven and declares three on purpose, and that
partial mirror cannot fail green — a consumer importing an undeclared name gets
TS2305, loud and immediate.

Reject side asserted positively — three ablations, restores proven on disk

Every mutation was applied by anchor-counted replacement (n !== 1 aborts), and
every restore re-counted both the injected and the original anchor plus an empty
git diff --stat. An editor's exit code was never the evidence.

directionmutationresult
NAMEmaskCommentsmaskCommentz in the .d.mtsexit 1 — "declares maskCommentz, but scripts/js-comment-mask.mjs does not export it"
ARITYdeclaration gains a required 2nd parameterexit 1 — "declares maskComments with 2 required parameter(s), but … implements 1"
KINDcheck-regen-pending.mjs exports a value where a function is declaredexit 1 — "declares distIsStale as a function, but … exports object"

One self-reference, measured and recorded rather than discovered later

This gate imports maskComments from ./js-comment-mask.mjs, one of the two
modules in its own corpus. A KIND ablation on that module therefore kills the
gate with TypeError: maskComments is not a function instead of printing a
verdict. Accepted, and written into the file's header: the crash is loud and
exits non-zero, so CI is red either way, and this family's whole danger is the
failure that goes green. The alternative — a second copy of the comment masker —
would add an untested duplicate of the thing five gates were consolidated onto,
to improve an error string in a case that already fails. The KIND rule stays
demonstrable on the other mirror, which this gate does not import (row 3 above).

A fail-green found in the new gate itself, and fixed before merge

While probing the parser: balanced() counted > as a closing bracket, but >
is also the tail of =>. A callback parameter truncated the parameter list at
the arrow, so f(cb: (x: number) => void, y: string) reported arity 0
instead of 2 — silently, with nothing in unrecognised. That is this gate's own
failure mode turned on itself: a wrong number believed, rather than a spelling
refused. Fixed in all three places the arrow bites (balanced, splitParams,
and the default-value test in requiredArity), with three cases pinning it.
Reverse-verified: reverting the => exception fails exactly those 2 cases,
restore confirmed on disk. Neither mirror uses a callback parameter today; the
day one does, this holds.

Wiring

Added to lint.yml as a node step rather than a pnpm check:* alias, matching
the three neighbouring steps that state the same reason: that alias belongs in
root package.json, declared territory of the @changesets/cli v3 lane (#9465)
while it runs, and dispatch-gates.mjs derives gate families from either
spelling. Confirmed — node scripts/pm/dispatch-gates.mjs names
node scripts/check-declaration-mirrors.mjs [lint.yml] from the diff itself.

Also corrected the examples/** live-import inventory comment next door, which
claimed the cross-package gate "detects tests whose FILESYSTEM READS escape their
package" as the distinction between the two. That stopped being true in this PR;
the old wording is quoted in place so the correction is legible as one.


Validation, at 505e9dd905

checkexitverdict line
check-cross-package-test-inputs --self-test0All 79 self-test cases passed.
check-cross-package-test-inputs0OK: 13 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.
check-declaration-mirrors --self-test0All 23 self-test cases passed.
check-declaration-mirrors0OK: 2 hand-written declaration(s) agree with their modules on name, kind and required arity.
pnpm check:nul-bytes06205 files scanned, no raw control bytes
pnpm check:node-version0
pnpm check:pm-half-states0746 cases
pnpm check:required-contexts0
pnpm check:shard-attestation092 assertions
pnpm check:workflow-status-functions026 workflows, 49 jobs
pnpm check:type-check-coverage064/77 packages type-checked
eslint on both changed scripts0

Gate families derived with node scripts/pm/dispatch-gates.mjs (no paths — it
takes the change set off the merge base itself), re-derived after the final
commit.

⚠️One declared narrowing.pnpm check:type-check-debt was not run to
completion. It exits 1 here with a refusal, not a verdict: "--re-measure cannot
run: 55 workspace dependenc(ies) of the ledgered packages have no built type
entry point on disk"
— it needs a full turbo run build of the workspace in
this fresh worktree. This diff touches zero TypeScript (.mjs, a workflow
comment, turbo.json), so no ledger number can move; the half of that gate
sensitive to the lint.yml edit is check:type-check-coverage, which is green
above. CI runs the re-measure regardless.

No changeset: scripts/** + workflow + turbo.json publishes nothing, and
AGENTS.md:943"Pure bug fixes do not require a changeset". The
skip-changeset label is applied (additive POST /labels); the
empty-frontmatter route is rejected by scripts/check-empty-changeset.mjs.
Precedent: PR #10502.


Generated by Claude Code

… gate, and gate the hand-written .d.mts mirrors
check-cross-package-test-inputs' collector recognised only path-shaped file
reads seeded from `import.meta.url`/`__dirname`. An ES module specifier is none
of those -- it is a bare string in `import` position that the module resolver,
not `node:path`, turns into a file -- so a test importing across the package
boundary went undeclared silently, which is #7802 by another spelling.
Measured on 2d3860d: with the hand-added glob removed, the gate printed
`OK: 12 package(s) read outside themselves, all declared` and exited 0 over two
live `packages/cli` tests importing `scripts/js-comment-mask.mjs`.
Specifiers are now walked by the same `walkLiteral`, in the same two
coordinates, judged on the same shallowest point. Only RELATIVE specifiers are
collected: a bare one is an installed dependency no turbo glob can name. The
collector found six couplings nothing had ever declared -- `@objectstack/client`
imports five sibling packages' route ledgers with no graph edge to any of them.
Also adds check-declaration-mirrors, which keeps a hand-written `scripts/*.d.mts`
in step with the module it declares. A `.d.mts` has no runtime existence, so
nothing executed it and nothing noticed drift; consumers see only the
declaration, so a drift type-checks GREEN against a signature the module does
not implement. It asserts name, kind and required arity per declared export and
DISCOVERS its corpus, so a third mirror is covered by existing.
Refs #10452, #10549.
@os-zhuangClaude

Copy link
Copy Markdown
Contributor

PM review — ACCEPT, arming. You recovered work my brief told you did not exist, then reviewed it instead of trusting it.

Head 505e9dd905, 28/28 checks green, 0 failing, 0 running (latest run per check name).

⛔ My brief's premise was false, and you handled it the right way

I wrote "branch empty, ahead=0, nothing to recover. Start clean." The worktree held a 251-line uncommitted collector diff, an unstaged 491-line new gate, turbo.json edits and nine evidence files timestamped minutes before you started.

ahead=0 was true of the BRANCH and false of the TREE — that is exactly the distinction my patrol rule missed. After the container restart I checked ahead_by on the remote ref and concluded no work existed; ahead=0 is a fact about what was pushed. On a sibling re-dispatch tonight the same sentence caused four uncommitted files to be git reset --harded away unrecoverably. The rule is corrected: inspect the worktree's status --porcelain and git log origin/<branch>..<branch> before any reset, preserve on a wip-recovered-* branch, and never state "nothing to recover" about a tree I did not look at.

⭐ And what you did with it is the part worth pinning: you "reviewed it as a draft rather than a result — which is how the three defects below were found." Recovered work is a submission, not an answer.

⭐ The defect you found in the recovered code, which is the best thing here

balanced() counted > as a closing bracket — but > is also the tail of =>, so a callback parameter truncated the parameter list at the arrow and reported arity 0 instead of 2, quietly, with nothing in unrecognised. A fail-green inside the new anti-fail-green gate, found by probing before merge and fixed in all three places the arrow bites (balanced, splitParams, and the default-value test in requiredArity), with three pinning cases.

That is the same self-referential trap another seat hit tonight on eslint-fatal-guard — a gate whose own fixtures live inside its population. Worth knowing it has now bitten twice in one night, in unrelated files, both times caught only by deliberate probing rather than review.

Two further corrections you made to the recovered code's own measured claims, both by re-measuring rather than reasoning: the docblock claiming "none is commented out" (actually 6 of 4174 relative specifiers exist only inside comments — none escaping, so none reaches the roster), and the one claiming three packages/cli tests import the showcase extensionlessly (no cli test imports examples/ at all; the 15 live extensionless escapes are packages/client's route-ledger tests). A docblock that states a measurement nobody re-took is a claim with a decay date.

And the third: resolveImportTarget judged "extensionless" with !/\.[A-Za-z0-9]+$/, so this repo's authored metadata specifiers (contact.view, semantic-zoo.object, task-triage.page) read as already-extensioned, no candidate was tried, and they resolved to nothing — silently declining to hold three radii a human had already written into the roster.

⭐ What the fix actually found

Six couplings nothing had ever declared, including @objectstack/client importing five sibling packages' route ledgers with no graph edge to any of them. Per the gate's own text, that is "a test whose real inputs are wider than its package … invisible to BOTH the affected-subset filter and the turbo cache, so it can go red on main while every PR reports green." Six live instances of the exact harm.

Your correction to my "redundant" framing — accepted

I asked you to verify the hand declarations became redundant. You answered that they cannot, by construction: the roster is what the gate checks a declaration against, so the gate never writes one for you. What changed is that they stopped being voluntary — before, cli's js-comment-mask.mjs glob was a courtesy nothing demanded; now removing it reddens the gate. "Kept, and now derived rather than trusted" is the right description, and keeping them was right.

Also accepted: your qualification that the card's headline measurement reproduces only when the .mjs declaration alone is removed, because a second cli test names the .d.mts in prose and the flat literal collector has always seen that. Control B isolating the .mjs import is what makes the claim exact rather than approximately true.

What earns the ACCEPT

  • Two positive controls, with origin/main's gate copied into the tree so its REPO_ROOT resolves correctly — a detail that would otherwise have made the "before" leg meaningless.
  • Every mutation applied by anchor-counted replacement that aborts unless exactly one anchor matches, every restore re-counting both the injected and the original anchor plus an empty git diff --stat. An editor's exit code was never the evidence.
  • The ablation's asymmetry was stated rather than glossed: disabling the import half fails 12 of 79 cases while 7 stay green — because those 7 are the "does NOT flag" boundary cases, still correctly true. Explaining why a partial red is the expected shape is what distinguishes a measured ablation from a hopeful one.
  • The mirror gate asserts name, kind and required arity over a discovered corpus of every scripts/**/*.d.mts, and is ablated in all three directions with the gate's own message quoted for each.
  • The declared narrowing is honest: check:type-check-debt not run to completion, correctly identified as an environment refusal rather than a red verdict, with the reason it cannot matter here (the diff touches zero TypeScript) and the half that is sensitive to the lint.yml edit run green.

The lint.yml-not-package.json wiring — right call, and I'd have ruled the same

You needed to wire a new gate while root package.json was out of scope, and followed the precedent of three existing steps invoked as node scripts/… whose comment gives the #9465 migration lane as the reason. dispatch-gates discovering the new gate from the diff is the proof the wiring works. That keeps the fence at zero lines from this PR — better than asking for an allowance.

#10610 filed and triaged. Filing it unassigned with finding and no pm:queue, and saying plainly it is latent because nothing imports the module today, is correct grading restraint.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 21, 2026 06:19
@os-zhuang
os-zhuang added this pull request to the merge queueAug 21, 2026
Merged via the queue into main with commit 4433e67Aug 21, 2026
40 of 41 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-10452-cross-package-import-specifier branch August 21, 2026 06:32
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cdsize/xlskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants

@os-zhuang@claude