Skip to content

fix(cli): make serve's host importer reachable from anywhere, and sweep for the class - #10919

Merged
os-elon merged 3 commits into
mainfrom
claude/issue-10769-host-importer-unreachable-above-binding
Aug 21, 2026
Merged

fix(cli): make serve's host importer reachable from anywhere, and sweep for the class#10919
os-elon merged 3 commits into
mainfrom
claude/issue-10769-host-importer-unreachable-above-binding

Conversation

@os-elon

Copy link
Copy Markdown
Collaborator

Fixes#10769

serve.ts reaches app-declared optional packages through createHostImporter,
which anchors resolution at the app being served. The helper was bound as a
const partway down one very long boot method, so it existed only below its
own binding — and a load written above that point is not a compile error.
The author simply writes a bare import(), which resolves against the CLI's own
realpath and works fine in a dev checkout where everything is hoisted into one
node_modules. It breaks only in a real distribution layout, at boot, in
production. That is how it reached a published EE image, twice:

PR #10765 hoisted the binding above the cluster block, which addressed that
instance and left the class open: the next load added above the new line
reproduces it exactly, and no author has any reason to know where that line is.

1. Structural — the hazard is now unrepresentable

importFromHost is a module-scope function declaration. A function
declaration is hoisted over the entire module, so every line of serve.ts
reaches it in any order: "above the definition" is no longer a state this file
can be in, and there is no line number for a future author to have to know.

The underlying importer is memoised per host root, so one boot still shares a
single host require, exactly as the one mid-function const did. hostRoot
stays where it was for its other readers; the helper defaults to the same
process.cwd(), and there is no process.chdir anywhere in packages/cli/src.

2. Detection — the scan is widened to the whole class

serve-cluster-host-resolution.test.ts scanned the cluster pair. It now sweeps
every app-declarable optional load, classified mechanically rather than
from a hand-kept list: a package is app-declarable exactly when
packages/cli's own package.json does not declare it — which is precisely the
set a bare import() from the CLI cannot resolve except by accident of workspace
hoisting. A newly added optional package is therefore covered the moment it is
added, with nobody having to remember this test exists.

The sweep reads code only. serve.ts discusses import() in prose throughout its
comments (including the note describing this very defect), and a naive scan
matches that prose — 4 of 46 raw matches were comments. A comment stripper that
preserves byte offsets removes them, and is itself pinned in both directions.

Vacuity guards (the pattern from PR #10882 — a sweep asserting "nothing is
wrong" passes trivially when it matches nothing):

  • floors on sites found, packages resolved, and manifest entries read;
  • the four known app-declarable loads asserted by name, which proves the
    resolver still handles all three spellings serve.ts uses (a const binding, a
    template prefix, and the manifest cross-check);
  • a stripper guard pinning that prose is removed and that real code survives.

3. One live instance the sweep found

@objectstack/service-i18n was loaded with a bare import(). packages/cli does
not declare it, so an app carrying its own copy could only ever be found by
accident of hoisting — green in a dev checkout, absent on a real install layout.
It is the same defect class as the card, mechanically fixed with the instrument
already established in this file, in a file no other claim holds. Named here
because a fix nobody points at is unreviewable.

createHostImporter is a strict superset of a bare import() for a bare package
name — undeclared falls back to the CLI's own resolution — so the quiet
"i18n not installed, use the kernel fallback" path is unchanged.

Fences

Two assertions deliberately replaced — please read this as the diff's one judgement call

The scan carried two assertions that pinned the mid-function const shape
itself
: that the definition's index is below the cluster block's, and that
const importFromHost = occurs exactly once. The structural half removes that
shape by design, so neither can survive verbatim. They were replaced by
strictly stronger checks, not dropped:

wasis now
definition index is above the cluster usedefinition is at module scope (column 0), so it is above every line; a nested or indented declaration fails
exactly one const importFromHost =exactly one module-scope declaration and exactly one createHostImporter( call; anyconst/let/var importFromHost fails

The old check could only notice a load that had already been placed above the
binding. The new one fails the moment the helper stops being reachable from
everywhere — the cause rather than the symptom. No threshold moved, no baseline
raised, nothing skipped or quarantined. Flagging it explicitly because "the
assertion turned red so I rewrote it" is the shape this repo does not accept, and
a reviewer should confirm the replacement is the stronger reading.

Verification

Union of the derived gate families run on the final commit bb14311793.
node scripts/pm/dispatch-gates.mjs derived the set from the merge-base change
set itself (3 paths) rather than from a hand-written list; all 19 reachable
families exit 0, e.g.

  • check-engine-double-contract: OK — 376 pinned, 133 in the DEBT ledger, 2 exempt.
  • check-nul-bytes: OK (scanned 6297 text file(s) ... no raw ASCII control bytes).
  • where-matcher conformance holds: 275 matcher(s) discovered ... none new.
  • OK: 13 package(s) read outside themselves, all declared (cross-package-test-inputs)

The ratchet family needs a built closure, so it was run the way lint.yml does
(turbo build over ./packages/*, then --re-measure), on bb14311793:

  • check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 340.5s, 1908 raw tsc error(s) total, none above its recorded number.

This one matters here rather than being boilerplate: @objectstack/cli is in
TEST_DEBT (146 raw errors across 65 files), so its hidden test tree is measured
by this gate and not by pnpm --filter @objectstack/cli typecheck — the new
scanner code in the test file is real TypeScript that this ratchet compiles.

pnpm --filter @objectstack/cli test146 files, 1617 tests, all passed.
pnpm --filter @objectstack/cli typecheck — clean. The widened file on its own:
12 passed.

Reverse verification (both legs; the scan's subject is serve.ts read from
source with readFileSync, not through dist/, so no rebuild can stale these
— the only built dependency, @objectstack/types/node, is used by the fixture
tests, which were not ablated):

  • reverting the i18n load to a bare import()2 red, naming
    serve.ts:1833 import(i18nPkg) → @objectstack/service-i18n;
  • turning the module-scope declaration back into a const arrow → 2 red on the
    module-scope and single-importer pins.

Each restore leg was git checkout HEAD -- serve.ts, confirmed byte-identical.

The first run of the second leg came back 3 red, not the 2 predicted — the
extra failure was the stripper guard, which had used function importFromHost(
as its "real code survived" marker and so doubled as a second shape check. That
coupling is fixed in its own commit; the re-run then matched the prediction
exactly.


Generated by Claude Code

…ep for the class (#10769)
`importFromHost` was a `const` bound partway down one very long boot method, so
it existed only below its own binding. A load written above it was not a compile
error — the author wrote a bare `import()`, which resolves from the CLI and is
green in any dev checkout where everything is hoisted into one `node_modules`,
and dead at boot on a real distribution layout. That shipped twice (cloud#1013,
#10645); hoisting the binding fixed each instance and left the class open.
It is now a module-scope function declaration, hoisted over the entire module, so
"above the definition" is not a state this file can be in.
Sweeping for the class found one live instance: `@objectstack/service-i18n` was
loaded bare although `packages/cli` does not declare it. Now host-anchored; the
undeclared fallback keeps its quiet-skip path unchanged.
The source scan is widened from the cluster pair to every app-declarable optional
load, classified mechanically against the CLI's own manifest, with a vacuity
guard so it cannot pass by matching nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
…ts pin
Reverse-verifying the structural change turned the stripper guard red as a third
failure: it used `function importFromHost(` as its "real code survived comment
stripping" marker, so mutating that declaration reddened it for a reason that has
nothing to do with the stripper. Its markers are now unrelated to the shape under
test, so it reports on the stripper alone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
The `plugin` entry pointed at "filed separately"; it now names #10908 so the next
reader can find the decision instead of re-deriving it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 4 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 58563be069a70d09c4d9cc5f522bb27c5f8a78b2.

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 58563be069a70d09c4d9cc5f522bb27c5f8a78b2packageMentionDocs.

Which tree this was computed on

This run read content/docs from 62cca2462a65c53c57283b4022ca53f1e23a82ae — the merge of head bb14311793891c201111620f45e8a910e39effbc into base 58563be069a70d09c4d9cc5f522bb27c5f8a78b2, 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 62cca2462a65c53c57283b4022ca53f1e23a82ae && git checkout 62cca2462a65c53c57283b4022ca53f1e23a82ae
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 58563be069a70d09c4d9cc5f522bb27c5f8a78b2 bb14311793891c201111620f45e8a910e39effbc && git checkout -B drift-repro 58563be069a70d09c4d9cc5f522bb27c5f8a78b2 && git merge --no-ff bb14311793891c201111620f45e8a910e39effbc
node scripts/docs-audit/affected-docs.mjs --json 58563be069a70d09c4d9cc5f522bb27c5f8a78b2

⚠️ 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 58563be069a70d09c4d9cc5f522bb27c5f8a78b2 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 21, 2026
@os-elon
os-elon marked this pull request as ready for review August 21, 2026 17:47
@os-elon
os-elon added this pull request to the merge queueAug 21, 2026
Merged via the queue into main with commit 3d7deb7Aug 21, 2026
32 checks passed
@os-elon
os-elon deleted the claude/issue-10769-host-importer-unreachable-above-binding branch August 21, 2026 18:05
@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 32510007801 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

    ✗ Build failed in 6.22s
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • ⚠️本次没有可用的聚合签名(日志里没有能解析出测试文件名的 FAIL 行)—— 这不是「没有同签名的其他 PR」,是这一轮没测到。跨 PR 聚合本次不可用,请手工比对其他 PR 的同类评论。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 43 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

os-elon pushed a commit that referenced this pull request Aug 22, 2026
… scans (#10514)
Two `packages/cli` scans read `serve.ts` raw, with no comment/code separator
at all, so prose counted as code -- the FABRICATES direction
`scripts/js-comment-mask.mjs`'s own header calls the worse one. Neither scan
was wrong today, but both were one ordinary comment away from being wrong:
- `serve-email-config-parity.contract.test.ts`'s `keysReadFromConfigEmail()`
matched `cfgEmail\.\w+` over raw source, so a comment naming an undeclared
key would have fabricated a false "undeclared key" red, and -- the worse
direction -- a comment naming a declared-but-unread key would have
silently restored the exact `DECLARED_BUT_UNREAD` exemption this file's
own docblock says was deleted for good after #5447/#5470.
- `serve-multi-node-cap-advisory.pin.test.ts`'s four shape assertions
matched regexes over raw source, so a reverted call could hide behind a
trailing comment quoting the shape it replaced.
Both now scan `maskComments(SERVE_SOURCE)` instead. `interfaceFields()`'s own
narrower, brace-matched strip is untouched -- out of scope per the issue.
Re-verified per the card's release condition: on origin/main at branch point
(047ac86), the raw / naive-stripped / masked key sets for subject 1 and
the raw / masked match verdicts for subject 2's four assertions are all
identical -- no verdict moved by #10453/#10919/#10948/#10956's changes to
serve.ts since the card was written. `CROSS_PACKAGE_TEST_INPUTS['@objectstack/cli']`
and the turbo.json `@objectstack/cli#test` input already carry
`scripts/js-comment-mask.mjs`/`.d.mts` (landed with #10453/PR #10513), so no
registration work was needed -- just the two-site routing.
Adds vacuity-proof tests demonstrating the FABRICATES shapes on synthetic
sources: a comment containing the exact pattern each scan keys on is shown
producing the wrong verdict when read raw, and the right one once masked.
Fresh answer to the open question #10909's dev never returned: yes, editing
comment prose in serve.ts could move both scans' results pre-fix (demonstrated
above); post-fix it cannot.
Side effect caught by `check:cross-package-test-inputs`: adding the
`maskComments` import gave `serve-email-config-parity.contract.test.ts` its
first real escaping read, which newly exposed a PRE-EXISTING prose mention of
`packages/spec/src/system/email-config.test.ts` (a doc cross-reference, never
actually read) to the gate's literal collector. Reworded to name it without a
single quoted repo-relative literal, following the precedent already set in
this directory's `serve-multi-node-cap-advisory.pin.test.ts` for the identical
gate.
Fixes#10514
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

2 participants

@os-elon@claude