Skip to content

fix(cli): serve's host importer states its own resolution base - #11405

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-11157-serve-fallback-import-base
Aug 23, 2026
Merged

fix(cli): serve's host importer states its own resolution base#11405
os-zhuang merged 2 commits into
mainfrom
claude/issue-11157-serve-fallback-import-base

Conversation

@claude

@claudeclaudeBot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Fixes#11157

packages/cli/src/commands/serve.ts's importFromHost built its importer without stating its own resolution base. createHostImporter's undeclared leg falls back to "the importing package's own resolution", and which package that is depends entirely on where the import() is physically written — Node ESM resolves a bare specifier against the module containing the call. #10943 turned that into an explicit parameter, options.fallbackImport. @objectstack/verify (bootStack) and the packages/qa/dogfood enterprise probe both pass theirs; packages/cli was the one caller that declined, so its fallback resolved from @objectstack/types, which under a pnpm-isolated layout sees only @objectstack/spec.

importer=createHostImporter(hostRoot,{fallbackImport: (fallbackSpecifier)=>import(/* webpackIgnore: true */fallbackSpecifier),});

The call was re-derived, not quoted: it sat at serve.ts:389 on the merged ref, one line among three moves today.

The measured accept-set delta

Exactly "packages packages/cli itself declares" — Clause-②: no. Re-measured in this checkout with import.meta.resolve from a probe placed inside each package, not taken from the card:

specifierfrom packages/clifrom @objectstack/types
chalkOKMISS
@objectstack/plugin-authOKMISS
@objectstack/plugin-auditOKMISS
@objectstack/specOKOK — types' one dependency
@objectstack/service-cluster (+ -redis, -postgres)MISSMISS
@objectstack/service-i18n, @objectstack/organizations, @objectstack/service-ai, @objectstack/service-ai-studioMISSMISS

All three widened names are in packages/cli's own dependencies, so the delta is a declaration surface and not a hoisting accident (pinned). All seven specifiers serve itself routes through importFromHost MISS from both bases — the card's "measured harmless today", re-confirmed. The #4719 declaration gate on the declared leg is untouched: reachable-but-undeclared is still refused, pinned in the e2e.

Anti-vacuity: the pin, and why half of it could not live in a unit test

⚠️Measured, and it changes where the pin has to go.@objectstack/types is a linked workspace package, so Vite processes it as source rather than externalising it and rewrites the import() inside packages/types/dist/node.mjs to its own resolver — which resolves from the vitest root, packages/cli. An in-process createHostImporter(appRoot)('chalk') with no caller base at all RESOLVES under vitest and THROWS under Node. Under vitest the two bases are the same base, so every in-process assertion about which one is in use — including the anti-vacuity control beside it — is green either way, silently.

So the resolution pins spawn a real Node process (test/serve-host-fallback-base.e2e.test.ts), and the unit file (src/commands/serve-host-fallback-base.test.ts) keeps only what is honest in-process: the undeclaredMessage branch, which is pure logic no resolver touches. Both files say so in their headers. This also means the existing chalk assertion in serve-config-plugin-host-resolution.test.ts is a behaviour statement, not a measurement of this card — it stays green under the ablation below.

Ablation — removing ONLY the fallbackImport option, everything else identical. Mutation confirmed on disk by grep counts either side (fallbackImport: (fallbackSpecifier) 1 → 0, createHostImporter(hostRoot); 0 → 1) plus git diff --stat; no build is involved because both vitest and the spawned tsx child read serve.ts from source; the script carries a trap … EXIT INT TERM restore.

Pre-fix (ablated), the load-bearing pin:

× loads a package the served app does NOT declare but packages/cli DOES
Failed to import plugin 'chalk': Cannot find package 'chalk': the host app does not declare it.
host app: /tmp/os-11157-app-O4WtvH
(fallback resolution also failed: Cannot find package 'chalk'
imported from /home/user/…/packages/types/dist/node.mjs)
(the caller did not pass `fallbackImport`, so that fallback resolved from
@objectstack/types, which can see only its own dependencies …)
expected 'Failed to import plugin…' to be 'RESOLVED'
Test Files 2 failed | 1 passed (3)
Tests 4 failed | 14 passed (18)

Post-fix, same six files:

Test Files 6 passed (6)
Tests 42 passed (42)

The four that move are: the resolution pin above; the e2e diagnostic pin (imported from …/packages/cli/…, not …/packages/types/…); the unit message-branch pin; and the source pin. The standing anti-vacuity floor is the e2e's CONTROL case, which builds createHostImporter(root) with no base and asserts it fails on the same specifier and names packages/types as the origin it failed from.

The diagnostic text — measured against the post-#11185 message

undeclaredMessage branches on fallbackImport !== undefined, confirmed by reading packages/types/src/node.ts (not modified). This change moves serve onto the other branch, so the note that exists to report this exact gap is no longer printed — pinned as an absence, alongside host app: <the app> (#11185's text, never the CWD). No assertion anywhere is written against the CWD-named text older trees emitted.

Second half — importConfigPlugin re-read: COLLAPSED, three branches to two

Re-read with this change and against #10944's landed option B (a relative entry is refused at config load, before any base is chosen), as the card asked. Measured redundant, and behaviour-preserving case by case:

casebeforeafter
app declares it, resolvable thereapp's copyapp's copy
app declares it, not installeddeclared-unresolvable, the INSTALL remedysame
app does NOT declare it, resolvable from this CLIlocal import() in serve.tsfallbackImport — the sameimport(), in the same module
app does NOT declare it, present but throwingcrash propagatescrash propagates — both legs gate on the one shared isModuleNotFoundError (Serve.isModuleNotFoundError delegates to it)
app does NOT declare it, resolvable nowherefail locally, then RE-ENTER the helper for the wordingone call does both; attempted once instead of twice

isDeclaredByHost(spec, root) is literally readHostDeclaration(spec, root).declared, which is what the helper already calls — so the pre-check was a second reading of the same manifest. The non-package branch (absolute path, file://, node:) stays in serve.ts: the helper's pass-through import() lives in @objectstack/types, and keeping it local is what stops a future relative spelling the refusal does not recognise from being silently re-based there.

serve-config-plugin-host-resolution.test.ts keeps every behavioural assertion unchanged — that is what made the collapse safe to take. One structural assertion moved: it required isDeclaredByHost(pluginSpecifier, root)in this method, and requiring the second copy would have pinned the fork Prime Directive #12 exists to prevent. It now asserts the delegation and the absence of a local declaration read; the single owner is pinned in packages/types/src/node.test.ts.

Verification

Run at 2656cafb, the final commit.

  • pnpm --filter @objectstack/cli typechecktsc --noEmit, exit 0.
  • Serve subset, 29 files: Test Files 29 passed (29) · Tests 405 passed (405).
  • Post-fix union, 6 files incl. both e2es: Test Files 6 passed (6) · Tests 42 passed (42).
  • pnpm lint (repo-wide eslint . --no-inline-config) → exit 0, no narrowing claimed.
  • Gate families derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (from the merge base, not a hand-built diff) — all green: check:nul-bytes (OK … no raw ASCII control bytes), check:cross-package-test-inputs (OK: 14 package(s) read outside themselves, all declared), check:test-source-alias, check:published-files, check:route-envelope, check:slot-lookup, check:type-source-resolution, check:changeset-gate-self-tests, check:objectui-changeset, check:type-check-coverage, check-adr-0087-registration, check-changeset-no-major, check-ci-filter-parity, check-empty-changeset, check-plugin-teardown-shape, check-affected-docs.
  • check:cross-package-test-inputs went red first on resolve(HERE, '../../types/dist/node.mjs') in the new e2e and was fixed by respelling, not by widening a glob: the child now resolves @objectstack/types/nodeby specifier from serve.ts's own location, which is what serve.ts itself does and what the gate asks for.
  • ⚠️check:type-check-debt NOT RUN — declared. It refuses to measure without the whole workspace built (Error: --re-measure cannot run: 1 workspace dependenc(ies) … have no built type entry point on disk — @objectstack/service-knowledge), which CI does before this step. Substituted, because packages/cli/tsconfig.json is include: ["src"] and the new e2e lands in the hidden test/ layer that ledger freezes at 146: a tsc -p over the package's own config extended to that one file exits 0, so the frozen count is unmoved. check:type-check-coverage, the sibling that reads the hidden-file population, is green with the file present.

content/docs/releases/** untouched; packages/types/src/node.ts read, not edited; packages/cli/test/helpers/serve-process.ts untouched. Changeset: .changeset/cli-serve-host-importer-caller-base.md.

Generated by Claude Code


Generated by Claude Code

zhuangjianguoand others added 2 commits August 23, 2026 15:37
`createHostImporter`'s undeclared leg falls back to "the importing
package's own resolution", and which package that is depends on where
the `import()` is physically written. #10943 made it an explicit
parameter; `@objectstack/verify` and the qa/dogfood probe pass theirs.
`serve`'s `importFromHost` did not, so its fallback resolved from
`@objectstack/types` — which under a pnpm-isolated layout sees only
`@objectstack/spec`.
Thread the base, and collapse `Serve.importConfigPlugin`'s three-branch
shape to two: the local `import()` its undeclared branch kept, and the
re-entry it made for the failure text, are now the same call.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
…climb
`check:cross-package-test-inputs` flagged `resolve(HERE, '../../types/dist/
node.mjs')` in the new e2e: a filesystem climb names a repo SOURCE input no
turbo glob covers. The child now resolves the helper from serve.ts's own
location by specifier, which is what serve.ts itself does and what the gate
asks for — a bare specifier is an installed dependency, and @objectstack/cli
already declares @objectstack/types, so turbo's task graph carries that edge.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 3 documentable anchor(s).

16 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 8d237b40c7a731f522f1979926a49acd9e430762.

4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

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 8d237b40c7a731f522f1979926a49acd9e430762packageMentionDocs.

Which tree this was computed on

This run read content/docs from 1b4995ae57dd3e64f12b09638c8d4817b241bb1b — the merge of head 2656cafb295773734c49b26a1549c2052c08ec9b into base 8d237b40c7a731f522f1979926a49acd9e430762, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 1b4995ae57dd3e64f12b09638c8d4817b241bb1b && git checkout 1b4995ae57dd3e64f12b09638c8d4817b241bb1b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8d237b40c7a731f522f1979926a49acd9e430762 2656cafb295773734c49b26a1549c2052c08ec9b && git checkout -B drift-repro 8d237b40c7a731f522f1979926a49acd9e430762 && git merge --no-ff 2656cafb295773734c49b26a1549c2052c08ec9b
node scripts/docs-audit/affected-docs.mjs --json 8d237b40c7a731f522f1979926a49acd9e430762

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 8d237b40c7a731f522f1979926a49acd9e430762 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

serve.ts's importFromHost does not pass its own resolution base to createHostImporter — measured harmless today, open by construction

2 participants

@os-zhuang@zhuangjianguo