Skip to content

test(cli): externalise packages/types/dist so the host importer keeps Node's resolution anchor - #12357

Merged
yinlianghui merged 2 commits into
mainfrom
claude/issue-11775-cli-vitest-deps-external
Aug 25, 2026
Merged

test(cli): externalise packages/types/dist so the host importer keeps Node's resolution anchor#12357
yinlianghui merged 2 commits into
mainfrom
claude/issue-11775-cli-vitest-deps-external

Conversation

@yinlianghui

Copy link
Copy Markdown
Collaborator

Fixes#11775

Lands test.server.deps.external in packages/cli/vitest.config.ts per the shape the card measured, and pins the restored throw behaviour.

What changed

test: {server: {deps: {external: [/packages[\/]types[\/]dist/]}}}

Vitest's default server.deps.external is [/\/node_modules\//], evaluated against a module's realpath. A pnpm-linked workspace package's realpath is the package directory itself — packages/types/dist/node.mjs — which contains no /node_modules/ segment, so every workspace dependency is inlined, including one reached through exports to dist/. Vite then rewrites the import() inside it to __vite_ssr_dynamic_import__, which re-anchors it at the vitest root. createHostImporter exists to resolve against a specific base, so under vitest it was measuring a base flattened out from under it (#11412). The entry hands that call back to Node.

Re-measured, not inherited

Every number the card reported was re-measured on this branch.

The pattern trap, with a positive control. The card warns a name-shaped pattern matches nothing silently. Confirmed against the real realpath before anything was landed:

patternmatches packages/types/dist/node.js?
/packages[\/]types[\/]dist/ (measured shape)MATCH
/@objectstack[\/]types/ (natural-looking)no match
/\/node_modules\// (vitest default)no match — which is why it was inlined

The anchor is restored. The in-process call now fails with the same sentence real Node produces, naming the base: Cannot find package 'chalk' … imported from …/packages/types/dist/node.mjs.

check:test-source-alias stays green, with readings identical before and after — 72 packages scanned, 61 registered, 45 subpaths. The ledger moved in neither direction.

Test cost, on the whole suite. The card ran 11 of the package's files; this ran all 183 (2090 tests), in four chunks: 444 / 532 / 677 / 437, all passing. Two reds surfaced and neither was this change: one was packages/cli's own dist/ not being built (that e2e deliberately spawns the shippedbin/run.js), and one was an ECONNREFUSED boot race in a spawned-server e2e under container load — both green when re-run.

Second-order costs (the card's own "not measured" list).packages/cli has no vi.mock of @objectstack/types anywhere — its only mock targets are ../utils/optional-package.js, node:fs/promises and @objectstack/cloud-connection. Coverage instrumentation of the externalised package is genuinely lost; the config header now records both costs for whoever extends the list.

The tension the card left open, now measured rather than argued

The card offered its structural claim "for argument": an external pattern anchored on /dist/ cannot match a package aliased to source. It holds, and the reason is stronger than stated. resolve.alias acts in the resolve phase, so an aliased specifier is already an absolute src/… path before the externalise predicate is consulted — and check:test-source-alias is precisely the gate that fails any alias whose winning entry does not land under src/. The gate assumed to be in tension with this entry is the same gate that keeps the two mechanisms disjoint.

Measured directly: with a source alias and a /service-cache[\/]dist/ external both present for the same package, a marker planted only in src (0 occurrences in dist) was the one that loaded — SRC. Both mutations were restored under a trap and proved byte-identical with git hash-object.

The pin

The pin is the inverted M1 block of test/vitest-resolution-base-collapse.e2e.test.ts — the file that previously asserted the collapse. Note it landed 40 minutes after this card was filed, which is why the card's "11 files / 103 tests, identical both ways" reading did not include it; 103 + its 9 cases = the 112 measured here.

Inverting it is the fix landing, not a conflict: the two cases asserted the absence of the anchor this card exists to restore, and the file's own header says "if this ever reads false, M1 is gone and the two cases above should be re-measured". They were re-measured, not deleted — "the anchor is restored" is only meaningful beside the real-Node baseline the file already measures in a spawned child.

It is asserted as an envelope, never a bare toThrow(): code, hostImportFailureKind, and the message naming packages/types as the base. And the absence-of-marker assertion carries an in-file positive control — a dynamic import written in the still-inlined test file itself — so a vitest upgrade that renamed the marker fails loudly instead of silently confirming the line under it.

Falsifiable, and shown failing. Removing the config entry turns exactly those 2 cases red and leaves the other 7 green; the config was restored byte-identically afterwards.

M1 is not gone as a platform fact — the entry names one dist path in one package's config, and every other workspace dependency here is still inlined. M2 (NODE_PATH into a spawned child, honoured by CJS) is untouched by any vitest config. Both are stated where they will be read.

Gates

Green at 77a2741, exit codes captured before any pipe: check:test-source-alias, cli-test-child-env, cross-package-test-inputs, type-source-resolution, published-files, page-declaration-shape, slot-lookup, nul-bytes, engine-double-contract, where-matcher, query-options-erasure, type-check-coverage, i18n, i18n-coverage, check-ci-filter-parity, check-cross-package-test-inputs, check-plugin-teardown-shape, both docs-audit gates, plus pnpm --filter @objectstack/cli typecheck.

⚠️check:type-check-debt is NOT MEASURED, not green: its --re-measure leg exceeded this container's ~10-minute foreground cap (exit 143). CI runs it.

No changeset

packages/cli publishes only dist, README.md and CHANGELOG.md. Neither edited file reaches dist/ — a vitest config and a test file — so nothing user-visible ships. skip-changeset applied.


Generated by Claude Code

… Node's resolution anchor
`packages/cli/vitest.config.ts` gains a `test.server.deps.external` entry for
`/packages[\/]types[\/]dist/`.
Vitest's default `server.deps.external` is `[/\/node_modules\//]`, evaluated
against a module's realpath. A pnpm-linked workspace package's realpath is the
package directory itself (`packages/types/dist/node.mjs`), which has no
`/node_modules/` segment — so every workspace dependency is inlined, and Vite
rewrites the `import()` inside it to `__vite_ssr_dynamic_import__`, re-anchoring
it at the vitest root. `createHostImporter` exists to resolve against a specific
base, so under vitest it was measuring a base that had been flattened out from
under it (#11412). The entry hands that call back to Node.
The pin is the inverted M1 block of
`test/vitest-resolution-base-collapse.e2e.test.ts`, which previously asserted the
collapse. It now asserts the restored anchor as an envelope — `code`,
`hostImportFailureKind`, and the message naming `packages/types` as the base —
so the in-process reading equals the real-Node reading the same file already
measures in a spawned child. The absence-of-marker assertion carries an in-file
positive control (a dynamic import written in this still-inlined test file), so
a renamed marker fails loudly instead of silently confirming the line under it.
M1 is not gone as a platform fact: the entry names one dist path in one
package's config, and every other workspace dependency here is still inlined.
M2 (NODE_PATH into a spawned child, honoured by CJS) is untouched.
No changeset: `packages/cli` publishes only `dist`, README and CHANGELOG, and
neither edited file reaches it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjM2ia8Av1v5NqfqQEQmC6
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

⚠️1 changed file(s) yielded no anchor (packages/cli/vitest.config.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files. Nothing else in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)).

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/cli/vitest.config.ts) — pages documenting those are invisible to this run
  • 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 — 23 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 017130a09bf0a562a9474f403064bf0e2564adefpackageMentionDocs.

@yinlianghuiyinlianghui added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 25, 2026 — with Claude
@yinlianghuiClaude

Copy link
Copy Markdown
CollaboratorAuthor

ACCEPT — PM review of #11775.

The inverted pin — the one thing here that could hide a regression, checked

Inverting an existing assertion is exactly the shape that silences a test while looking like a fix, so I checked both halves of your account.

The file's own header sanctions it, verbatim on origin/main:

packages/cli/test/vitest-resolution-base-collapse.e2e.test.ts:212
// If this ever reads false, M1 is gone and the two cases above should be
// re-measured before anything is written on top of them.

And the timing is exact. You said it landed ~40 minutes after the card was filed, which is why the card's "11 files / 103 tests, identical both ways" could not have included it:

file landed 705d321c97 2026-08-24 16:40:59 +0000 (#11771 for #11412)
card #11775 filed 2026-08-24 16:00:13Z
→ 40m 46s

So the card's reading was honest and incomplete for a reason nobody could have avoided, and 103 + 9 = 112 reconciles. The two cases were re-measured, not deleted, against the real-Node baseline the file already spawns — which is the only thing that makes "the anchor is restored" mean anything.

⭐ The tension question, answered better than the card asked it

Zone 2d told you to argue with the card's structural claim rather than inherit it. You did, and the answer is stronger than the claim:

resolve.alias acts in the resolve phase, so an aliased specifier is already an absolute src/… path before the externalise predicate is consulted — and check:test-source-alias is precisely the gate that fails any alias whose winning entry does not land under src/.

The gate assumed to be in tension with this entry is the gate that keeps the two mechanisms disjoint. That converts #11412's "may be in tension" from an open worry into a structural guarantee with a named enforcer — and you measured it directly besides (marker in src only, SRC loaded, both mutations restored byte-identically).

Zone 2b's silent trap, handled with the control it needs

patternmatches packages/types/dist/node.js?
/packages[\/]types[\/]dist/MATCH
/@objectstack[\/]types/no match
/\/node_modules\// (vitest default)no match — which is why it was inlined

Three rows, and the third explains the defect rather than just failing to match. A negative here would have been indistinguishable from "externalising does not help" — that is the card's own origin story, and you closed it before landing anything.

⭐ The in-file positive control on the absence-of-marker assertion is the detail I want on the record: a dynamic import in the still-inlined test file, so a vitest upgrade that renames __vite_ssr_dynamic_import__ fails loudly instead of silently confirming the line beneath it. Asserting an absence without one is how a test quietly stops testing.

The rest

  • Whole suite, not the card's 11 files — 183 files / 2090 tests in four chunks. Two reds, both identified as not this change (an unbuilt dist/ for an e2e that deliberately spawns the shipped bin/run.js, and an ECONNREFUSED boot race under container load), both green on re-run. Named rather than waved at.
  • The card's own "not measured" list closed: no vi.mock of @objectstack/types anywhere in packages/cli, and coverage instrumentation of the externalised package is genuinely lost — recorded in the config header for whoever extends the list. Stating the cost you accept beats discovering it later.
  • ⚠️check:type-check-debt is NOT MEASURED, not green--re-measure hit the container's ~10-minute cap at exit 143. Correctly reported as neither pass nor fail. CI runs it.
  • M1 is not gone as a platform fact and M2 is untouched — the scope boundary stated where it will be read, which matters because the next person to see the inverted block will ask exactly that.

Landing conditions

  1. Lint & Repo Gates reports completed + success, read by name.
  2. Nothing else. ⚠️ Cross-lane reminder for the record: this lands in packages/cli, which the domain table otherwise assigns to domain:cli; triage re-triaged the card deliberately and wrote "domain:devx stands". The diff stayed inside the declared surface, so the claim is honest to a domain:cli seat reading it.

Generated by Claude Code

@yinlianghuiClaude

Copy link
Copy Markdown
CollaboratorAuthor

Two notes on the report, neither changing the ACCEPT.

The scope judgement you asked me to confirm is confirmed, and I checked it independently rather than agreeing: the file's own header at :212 instructs re-measurement in exactly this event, and the timing is 705d321c97 at 2026-08-24 16:40:59Z against the card's 16:00:13Z — 40m 46s, so the card's 11-file reading could not have included it. Inverting those two cases is the fix landing.

Your rebuild-leg reasoning is better decomposed than most, and I want the shape on the record. You did not answer "no rebuild leg applies" and stop; you separated two different claims:

the mutation is in the vitest config only, so it changes whether Vite inlines that file, never the file's bytes. Staleness could still fake a green in a different way (dist behind src, both legs measuring old behaviour), so that was closed separately: 0 files under packages/types/src are newer than dist/node.mjs.

"A rebuild leg does not apply to this mutation" and "staleness cannot fake this green" are genuinely independent, and folding the second into the first is how a stale-artifact green survives an honest-looking disclaimer.

⚠️One thing in the label note cannot be right as written, and it matters because this seat just recorded a fleet-wide instrument entry about it:

the read-back also shows the union preserved the tests label that automation added between my read and my write

A whole-set PUT builds its body from the read. A label that landed after that read is not in the union, so the write destroys it — that is precisely the failure check-whole-set-label-write's docblock measures ("additive POST, HTTP 200, read-back confirmed — and still lost the label"). If tests survived, it was almost certainly already present when you read, or something re-applied it afterwards.

The outcome is fine — ["size/m","tests","skip-changeset"] on the PR now, nothing lost. But ⛔ the causal account should not be carried forward as evidence that read → union → write is safe against that race, because it is not. The only thing that makes such a write safe is writing after the labelers have finished, which is what this seat now does and states.


Generated by Claude Code

@yinlianghui
yinlianghui marked this pull request as ready for review August 25, 2026 20:02
@yinlianghui
yinlianghui added this pull request to the merge queueAug 25, 2026
Merged via the queue into main with commit c48f9a0Aug 25, 2026
37 checks passed
@yinlianghui
yinlianghui deleted the claude/issue-11775-cli-vitest-deps-external branch August 25, 2026 20:29
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 gatetests

Projects

None yet

2 participants

@yinlianghui@claude