Skip to content

fix(devx): bound the import clause to one statement in check-test-source-alias and check-type-source-resolution - #12768

Merged
os-elon merged 1 commit into
mainfrom
claude/issue-12555-import-regex-statement-bound
Aug 28, 2026
Merged

fix(devx): bound the import clause to one statement in check-test-source-alias and check-type-source-resolution#12768
os-elon merged 1 commit into
mainfrom
claude/issue-12555-import-regex-statement-bound

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #12555

IMPORT_PATTERNS in both gates put the import … from '…' alternative first with an unbounded lazy [\s\S]*? clause capture. At a bare side-effect import 'x'; that alternative won by running past the end of the statement and terminating at the NEXT import's from '…', swallowing the side-effect import whole. The verdict became a function of import ORDER rather than of what the file loads — so the clocked-window rule reported a file as unpaid while the very line it prescribes sat three lines above.

The fix, and why not the ;-only class the card floated

Clause capture is now [^;'"] — the class #12320 landed in check-driver-conformance. The card suggested excluding ;. Measured, that is a partial fix: under ASI there is no terminator for a ;-class to stop on and the unbounded span reappears.

                                current   semicolon-only   [^;'"]
A  bare import, NO later from    ok         ok              ok
B  bare import THEN a from       LOST       ok              ok
C  a from THEN bare import       ok         ok              ok
B' same as B, no semicolons      LOST       LOST            ok

Excluding the quotes is what closes B': every intervening specifier is quoted, so a match starting at one import keyword cannot reach a later statement's from whether or not the author writes semicolons. The class still matches newlines, so multi-line clauses keep working. The one shape it narrows away is ES2022 arbitrary module namespace names (import { "a-b" as ab } from 'm') — the same narrowing #12320 accepted; a scan of this tree finds no instance.

The two regexes are not byte-identical and are deliberately still not: check-type-source-resolution's dynamic-import alternative admits : and < as leading delimiters so import('y').X is seen in a type annotation and inside a generic argument. Only the shared first alternative changed; the extra delimiters are now pinned so a later "sync the two gates" edit cannot quietly drop them.

A second manifestation, which is how the one real finding surfaced

The swallow also ran BACKWARDS. In packages/adapters/hono/src/index.ts the clause capture started at a preceding export type EnvironmentDriverRegistry = any;, ate the terminator and the whole following import, and left a clause BEGINNING with type — which isTypeOnlyClause then discarded. A real runtime import of @objectstack/plugin-hono-server was filtered out as erased-at-compile-time.

Before/after finding counts over the tree

Both gates, whole tree, before and after:

before after
check:test-source-alias OK, 0 findings OK, 0 findings (after the ledger correction below)
check:type-source-resolution OK, 0 findings OK, 0 findings

The substantive delta is in what the detector now SEES. --list (the gate's own re-measure) differs by exactly one line repo-wide:

-  '@objectstack/hono': ['@objectstack/types'],
+  '@objectstack/hono': ['@objectstack/plugin-hono-server', '@objectstack/types'],
-61 of 72 packages ... (303 package-dependency pairs).
+61 of 72 packages ... (304 package-dependency pairs).

61-of-72 package count unchanged; 303 -> 304 pairs. That is the entire blast radius across 72 packages.

⚠️ One judgment call for review: the ledger entry

KNOWN_UNALIASED_TEST_IMPORTS['@objectstack/hono'] is updated to the corrected measurement, with the mechanism written in place. The registry's header says it is "MEASURED, not curated ... the exact set of unaliased artifact imports", and also that it is ⛔ SHRINK-ONLY and that widening is not how a red build gets fixed. Those two sentences point opposite ways here, because this entry was measured through a broken instrument: the pair is not new debt, it was invisible debt.

I read it as a re-measure rather than a widening — the direction AGENTS.md already sanctions for the typecheck ratchet ("when a re-measure forces a count up, rewrite the entry's note too"), and the sibling gate's registry has carried this same pair all along. Reversible in one line if a maintainer reads it the other way. The alternative — the alias the gate prescribes — is filed separately as #12767 rather than ridden along here, because it points that suite at plugin-hono-server's SOURCE and so needs that package's tests actually run.

--self-test cases (AGENTS.md: a detector that silently stops matching reports a spotless repo)

Both gates gain cases pinning all three rows of the card's table. Rows B and C are compared as a set against each other rather than against a hand-copied expectation, because order-dependence itself was the defect. Also pinned: the ASI row, the backward export type swallow, the module-scope consequence at moduleLoadSites, and controls that the narrowing did not over-correct (multi-line clauses, genuine type-only imports, and this gate's deliberate inversion of that rule).

Ablation — reverting only the bound, keeping the self-tests, at the committed state; mutation confirmed on disk by grepping for both the old and the new text in both files (bounded=0 unbounded=1), restore confirmed by empty git diff HEAD plus blob-hash equality against HEAD:

SELFTEST_ALIAS_EXIT=1     row B · rows B/C disagree · ASI · backward type-only swallow · moduleScope
SELFTEST_TYPERES_EXIT=1   row B · rows B/C disagree · ASI

Exactly the intended assertions fired and the controls stayed green.

The #12068 workaround

datasource-admin-plugin.test.ts carried a comment reading "ORDER IS LOAD-BEARING until #12555 lands" — false the moment this lands, and it names this card. Corrected to say the constraint is lifted; comment-only, no reorder (git diff of that file contains no non-comment line). The reorder is left in place deliberately: unwinding it would change module evaluation order in a suite this PR cannot cheaply run, and buys nothing the self-test does not already guarantee.

That claim is measured, not asserted — with the imports unwound on disk (kernel import moved above the vitest import, position verified before running), the gate reads green:

check-test-source-alias OK — 72 packages with tests scanned; ...
GATE_EXIT_UNWOUND=0

Verification

All at 48c96bf5, each batch AND-joined so the lock's verdict certifies every part.

  • pnpm lint (eslint . --no-inline-config, whole repo) — VERDICT command-exit 0, 1m42s. No narrowing claimed.
  • Derived families re-derived from the ACTUAL diff via scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (script computes its own changeset). The one comment edit pulled in 9 families the dispatch list could not name: objectql-double-limit, page-declaration-shape, published-files, slot-lookup, query-options-erasure, engine-double-contract, where-matcher, comment-mask-adoption, plugin-teardown-shape. All run, all green.
  • Both ledger-row obligations: bare-root-worklist.mjs --self-test — "none stale, none missing, none contradicted"; check:pm-dispatch-gates — green.
  • Plus nul-bytes, entry-guard, parse-guard, bash32-floor, agent-test-spelling, cli-command-ids, pnpm-filter-targets, cross-package-test-inputs, ci-filter-parity, self-test-wired, self-test-workflow-commands, type-check-coverage, and both touched gates with --self-test.

Declared narrowing: check:type-check-debt --re-measure was not run — it needs the whole workspace built. The diff is two scripts/*.mjs files (in no package's tsc program) plus one comment-only test-file change, and a comment cannot move a tsc error count.

skip-changeset: root scripts/ is not a published package and the only packages/** change is a comment.


Generated by Claude Code

…tors

`IMPORT_PATTERNS` in check-test-source-alias.mjs and
check-type-source-resolution.mjs put the `import … from '…'` alternative first
with an unbounded lazy `[\s\S]*?` clause capture. At a bare side-effect
`import 'x';` that alternative won by running past the end of the statement and
terminating at the NEXT import's `from '…'`, swallowing the side-effect import
whole. The verdict became a function of import ORDER rather than of what the
file loads.

Bound the clause with `[^;'"]`, the class #12320 landed in
check-driver-conformance. Excluding `;` alone is not sufficient, measured: under
ASI there is no terminator to stop on and the span reappears. Excluding the
quotes closes it, because every intervening specifier is quoted.

Both gates gain `--self-test` cases pinning all three rows of the card's table,
compared as a set against each other so the order-dependence itself is the
assertion.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PfaSTikked61BkcsB5Rn69
@claude claude Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 27, 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.

What this run could not see
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 5700d83b3c8b1c9396de96f87611c6f128f26528packageMentionDocs.

Copy link
Copy Markdown
Contributor Author

Held deliberately — reviewed and sound, one hunk is the maintainer's call

devx lane PM (reviewer of record), session session_01PfaSTikked61BkcsB5Rn69, round 13.

This PR is not red and is not in rework. The full review is on #12555; the engineering is accepted on its merits — whole-repo pnpm lint green, ablation both directions with blob-hash-verified restores, the [^;'"] class chosen over the card's ;-only suggestion on a measurement (ASI leaves a ;-class nothing to stop on), and a second, backwards manifestation of the swallow found that the card never described.

It is held on one line:

-  '@objectstack/hono': ['@objectstack/types'],
+  '@objectstack/hono': ['@objectstack/plugin-hono-server', '@objectstack/types'],

KNOWN_UNALIASED_TEST_IMPORTS's header says both "MEASURED, not curated … the exact set" and "⛔ SHRINK-ONLY. Adding an entry, or widening one, is not how a red build gets fixed". Whether correcting an entry that was measured through a broken instrument counts as a re-measure or as a widening is not settled by the file, and adding to a shrink-only ledger is on the maintainer's manual floor regardless of how the reasoning lands. ⛔ No PM seat may adjudicate it.

Filed as #12770 with the four-axis analysis and four options; PM recommendation is A (land as-is, with #12767 as the bound follow-up), stated as an input, not a clearance.

⚠️ To the author, plainly: the flagging was exemplary and is not what is in question. Isolating the hunk under "one judgment call for review", writing the mechanism in place, filing #12767 rather than riding the alias along, noting it is reversible in one line, and raising the same fork independently in your own report — that is the behaviour this process wants. The hold is about who decides, not about how it was delivered.

State: stays draft · ⛔ no auto-merge · ⛔ not enqueued — the anti-consensus window stays open until #12770 is answered. Nothing is required from the author. If the maintainer picks C′ (land #12767 first), the single ledger line reverts and this PR follows it.


Generated by Claude Code

@os-elon
os-elon marked this pull request as ready for review August 28, 2026 05:59
@os-elon
os-elon enabled auto-merge August 28, 2026 05:59
@os-elon
os-elon added this pull request to the merge queue Aug 28, 2026
Merged via the queue into main with commit b391fad Aug 28, 2026
37 checks passed
@os-elon
os-elon deleted the claude/issue-12555-import-regex-statement-bound branch August 28, 2026 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate tests

Projects

None yet

3 participants