Uh oh!
There was an error while loading. Please reload this page.
fix(cli): anchor serve's optional-package resolution at the app, not the CWD (#11185) - #11349
Conversation
…the CWD (#11185) `serve` takes its config as an argument, so the app being served need not be the process CWD — but every host-anchored optional load used `process.cwd()` as its resolution base. Booting an app by config path therefore read the wrong `package.json`: an app-declared optional service (`@objectstack/service-cluster` and its driver, the enterprise organizations runtime, anything a customer installs) came back `undeclared`, fell through to the framework-side fallback, and boot died while the app's own `node_modules` carried the package. `run()` now resolves the config path and the app root in one call (`anchorServedApp`), and every host-anchored load defaults to that root. The config's directory is adopted only when it holds a `package.json`, so no layout that resolves today resolves differently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
📓 Docs Drift CheckThis PR changes 1 package(s): 23 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 60ffe301b42afd95fbdc6e1b43e62aef6909a0a8 && git checkout 60ffe301b42afd95fbdc6e1b43e62aef6909a0a8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin d39569239b46f6ef68e8af438a6e78ccb62d6c92 111f038ecf9f5295a01810722fd273ec5daf26a6 && git checkout -B drift-repro d39569239b46f6ef68e8af438a6e78ccb62d6c92 && git merge --no-ff 111f038ecf9f5295a01810722fd273ec5daf26a6
node scripts/docs-audit/affected-docs.mjs --json d39569239b46f6ef68e8af438a6e78ccb62d6c92
|
…11185) The comment claimed "No layout that resolves today resolves differently after this". Measured, that is false: there is a fifth row. When the CWD's manifest DECLARES the optional package and the served app's does not, the load moves off the declared leg (hostRequire.resolve under the CWD) onto createHostImporter's fallback leg — which, because importFromHost passes no fallbackImport (#11157's residue), is a bare import() inside @objectstack/types. It still succeeds wherever Node's node_modules walk from there reaches the package (a hoisted monorepo whose root manifest declares it), and refuses where it cannot (a global/npx CLI serving an app elsewhere, package installed only beside the operator). Remedy named in place: declare the package in the SERVED app's own package.json, which is what #4719 asks for regardless. Comment text only — no behaviour change, no code touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
Uh oh!
There was an error while loading. Please reload this page.
Fixes#11185
os servetakes its config as an argument, soobjectstack serve /srv/app/objectstack.config.tsis a supported invocation and the app being served need not be the directory the operator stood
in. Every host-anchored optional load nevertheless used
process.cwd()as its resolution base,so with that invocation the CLI read the wrong
package.json: a package the app really doesdeclare, and really does carry in its own
node_modules, came backundeclared, fell through tothe framework-side fallback, and boot died.
Measured on the released EE 4.1.0 image as
OS_CLUSTER_DRIVER=redis⇒ migrate exits 1 ⇒ the wholestack cannot start.
What was still broken, measured at
origin/mainThe card's measured trace (
Cannot find package '@objectstack/service-cluster' imported from …/packages/cli/dist/commands/serve.js) is the bareimport()leg, and that leg is alreadyclosed — #10645 routed these loads through
importFromHostand #10769 hoisted the helper tomodule scope. Re-measured here on a real
serveprocess against a fixture app whose clusterpackages exist only in its own
node_modules:origin/mainserve objectstack.config.tsserve /srv/app/objectstack.config.tsThe second row is the residue, and it is exactly the route the card names as preferred
("
createRequireanchored at the app config directory"). Onorigin/main:The fix
run()now resolves the config path and the app root in one call,anchorServedApp(), andevery host-anchored load in the file defaults to that root:
importFromHost(specifier, hostRoot = servedAppRootOrCwd())Serve.importConfigPlugin'shostRoot ?? servedAppRootOrCwd()const hostRoot = servedAppRootOrCwd()One call, because the anchor must not be a statement a future author can write too late or leave
out: the absolute config path every later line needs is produced by the same call that sets the
root.
servedAppRootOrCwd()is read as a function at each use — a module-scopeconstwouldfreeze the pre-boot answer (
process.cwd()) into every call site, which is the defect wearing adifferent hat.
What is unchanged, and the one row that DOES move
The config's directory is adopted only when it holds a
package.json, and the CWD is keptotherwise.
readHostDeclarationreads a manifest — reachability is deliberately not the contract(#4719) — so a directory with no manifest declares nothing, and anchoring there could only turn a
working boot into an
undeclaredrefusal.dist/…)declared, found under the CWD'snode_modulesundeclared, so the load moves to the fallback leg: it still succeeds wherever that fallback reaches the package, and refuses where it cannotAn earlier revision of this body — and of
anchorServedApp's docblock — asserted "no layout thatresolves today resolves differently after this". That absolute is false, and the fifth row is
the counter-case. Both texts now carry the fifth row; the source comment was the half that mattered
most, because an absolute asserted in the code reads to the next author as a licence to skip the
check.
How far the fifth row actually bites, measured.
importFromHostbuilds its importer ascreateHostImporter(hostRoot)with nofallbackImport(#11157's residue), so the undeclaredleg is a bare
import()physically inside@objectstack/types, and Node ESM walksnode_modulesupward from there:
package,
serve apps/foo/objectstack.config.ts,apps/foo/package.jsonsilent: the walk reachesthe same hoisted store and the boot is what it always was. Only the leg changed.
npxCLIserving an app elsewhere, with the optional service installed only beside the operator.
Checked against
packages/types/src/node.tsand with a two-case probe of Node's own bare-specifierwalk-up: an importer sitting inside a hoisted store resolves the package regardless of the CWD,
while an importer in an unrelated install with the package only beside the CWD raises
ERR_MODULE_NOT_FOUND— whichcreateHostImporterre-throws asundeclared.Remedy for anyone who lands in the refusing case: declare the package in the SERVED app's own
package.json— which is what #4719 asks for regardless.So this narrows one row toward the declaration the contract already requires, and widens nothing.
The #4719 declaration gate itself is untouched: a package present in the app's
node_modulesbutabsent from its
package.jsonis still refused. Its remedy now names the app being served insteadof an unrelated directory the operator happened to be in.
Route chosen, and the one rejected
Chosen: anchor the resolution base at the app's config directory. It generalises — the next
app-declared optional service, and any third-party cluster driver, is covered with nobody
remembering this file exists.
Rejected: declare
@objectstack/service-cluster*inpackages/cli's own manifest. It wouldmake the open-core CLI take a published dependency on packages it never imports (the coupling
the non-literal specifier in
serve.tsexists to avoid), it fixes one instance and leaves theclass open — the same shape #10769 and #11157 both record — and, decisively, it does not fix
this defect at all: with the wrong base the app's declaration is never read, so a CLI-side
declaration changes nothing for an app addressed by config path. It is also a published-manifest
change, which this card was dispatched not to make.
The pin, and both directions
packages/cli/test/serve-app-anchored-optional-import.e2e.test.tsspawns the realserveprocess against a fixture app whose
@objectstack/service-clusterand@objectstack/service-cluster-redisare written into that app'snode_modulesalone — nothing inthis workspace can supply them to
packages/cli, which is what makes a pass unfakeable. It writesits own spawn rather than using
test/helpers/serve-process.ts, because that helper always runsthe child with
cwdset to the app, which is the one shape this file must not use.The in-process pin that already exists
(
src/commands/serve-cluster-host-resolution.test.ts) is green on both trees by construction: ithands
createHostImportera root the test itself chooses, and the root is precisely what waswrong.
Reverse-verification —
packages/cli/src/commands/serve.tsrestored toorigin/main(confirmed ondisk:
anchorServedApp0 occurrences,const hostRoot = process.cwd();1), the pin re-run, thenrestored by an
EXITtrap:The single pass is the control, which is what isolates the failures to the base rather than to
a broken fixture. With the fix:
Test Files 163 passed (163) · Tests 1839 passed (1839)(
@objectstack/cli, whole suite).#11157 is not made redundant by this
Measured on this branch, undeclared app, CWD elsewhere:
This PR corrects the base of the declared branch — which manifest is read and whose
node_modulesis searched. #11157 corrects the fallback base of the undeclared branch, andthat leg is still live and unchanged here. Two different bases; #11157 still has work to do.
Verification
Everything in this first list is at
0c93b6c1, the last commit that changed code.111f038eiscomment text only — no code, no test, no changeset — and is re-verified in its own list underneath.
pnpm --filter @objectstack/cli exec vitest run— 163 files, 1839 tests, all passingpnpm --filter @objectstack/cli typecheck—tsc --noEmit, exit 0pnpm lint(eslint . --no-inline-config, whole repo, not narrowed) — exit 0node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, plus the convention-triggered set for a new test file — all exit 0pnpm check:type-check-debt --re-measure— "33 ledger entr(ies) re-measured in 377.5s, 1897 raw tsc error(s) total, none above its recorded number" (it first refused on an unbuilt@objectstack/service-knowledge; the closure was built and it was re-run)Patch round
111f038e— comment text onlyThe only diff in this round is
anchorServedApp's docblock. Re-run at111f038e, clean tree:pnpm --filter @objectstack/cli typecheck—> tsc --noEmit,VERDICT command-exit 0pnpm --filter @objectstack/cli exec vitest run test/serve-app-anchored-optional-import.e2e.test.ts—
Test Files 1 passed (1) · Tests 6 passed (6), so the comment edit did not disturb the pinpnpm exec eslint --no-inline-config --format json packages/cli/src/commands/serve.ts— exit 0,filesLinted=1 errors=0 warnings=0. Narrowed on purpose, and the narrowing is a measurement:the file count is read from eslint's own JSON output, and this repo's single
eslint.config.mjsnever enables type-aware linting for any file (no
parserOptions.project, no typed@typescript-eslintrules — declared, and positively controlled, in that config's own header), soa comment-only edit inside one file cannot move any untouched file's verdict. CI runs the whole
repo regardless.
serve.ts-derived gate families, re-run at111f038e, all exit 0:check:nul-bytes,check:slot-lookup,check:test-source-alias,check:type-source-resolution,check:published-files,check:route-envelope,check:cross-package-test-inputscheck:type-check-debt --re-measureand the full 1839-test suite. A docblock cannotmove a tsc error count or a test outcome, and both are green at
0c93b6c1above; CI runs them onthis head anyway.
Generated by Claude Code
Generated by Claude Code