Skip to content

[finding] serve.ts's host-anchored importer is a mid-function const, so any app-declared optional package loaded ABOVE it silently falls back to CLI-anchored resolution — this has now shipped twice #10769

Description

@os-elon

Filed unassigned from the domain:cli lane execution seat (session session_019bmVFqoQPq63zhKrxdYG1r), recording only — surfaced while reviewing #10645 / PR #10765. The dev found the mechanism; I verified it independently by content and am filing it because the card it came out of does not cover the class.

The mechanism

packages/cli/src/commands/serve.ts loads app-declared optional packages through createHostImporter (@objectstack/types/node), which anchors resolution at the app. That is the correct instrument — the CLI's own node_modules does not contain packages the app declares, and bare import() resolves from the CLI.

But the helper is bound as a const inside one very long boot function:

constimportFromHost=createHostImporter(hostRoot);

So it exists only below its binding. A load written above that point cannot use it — and the failure mode is the problem: it is not a compile error. The author simply writes a bare import() instead, which resolves from the CLI and works fine in a dev checkout where everything is hoisted to one node_modules. It breaks only in a real distribution layout, at boot, in production.

Measured on origin/main, before PR #10765

importFromHost defined :2128
cluster loads :1425, :1456 ← ~700 lines ABOVE the definition

Both cluster loads were bare import(). They could never have used the helper.

This is the second instance, not the first

Two independent instances of one shape is the thing worth acting on. PR #10765 hoists the binding above the cluster block, which fixes this instance — it does not stop a third. The next load added anywhere above the current binding, by an author who has no reason to know the binding's line number, reproduces it exactly.

⚠️ Note the existing comment at serve.ts:2759 reads "importFromHost (declared above, before the auth block)" — true for that reader, and precisely the kind of local reassurance that makes the hazard invisible to someone working 700 lines earlier.

Suggested shape (grading input, not a prescription)

PR #10765 already ships the right instrument, scoped to one block — packages/cli/src/commands/serve-cluster-host-resolution.test.ts carries five source-scan assertions: loads go through importFromHost, never a bare import(), the helper is defined above the block, and there is exactly one definition.

The generalisation is to make that scan cover every app-declarable optional package serve.ts loads, rather than the cluster pair only. Two candidate routes, both worth measuring before choosing:

  • widen the source scan to the whole file — enumerate every import() with a non-literal or @objectstack/* optional specifier and assert each is host-anchored and below the binding;
  • or remove the ordering hazard at the root by making the importer reachable from anywhere in the function (hoisted binding, or a small module-level factory), so "above the definition" stops being a state a file can be in.

The second closes the class; the first only detects it. A grader may reasonably want both.

Not a duplicate

Metadata

Metadata

Assignees

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions