Uh oh!
There was an error while loading. Please reload this page.
docs(cli): correct capability-loop comments — bare import, CLI copy wins (#10909) - #10956
Conversation
Two `serve.ts` comment sites claimed the capability loop resolves `EmailServicePlugin`/`SmsServicePlugin` "host copy first". Measured at head, the loop does a bare `await import(spec.pkg)` / `await import(ex.pkg)` — no `importFromHost` — which Node ESM resolves against this CLI's own realpath, so the CLI's bundled copy always wins and the host's copy is never consulted. Corrected both comments to state that, and named the contrast with `Serve.importConfigPlugin` (the served app's own `plugins: [...]`, which IS host-anchored) so the file's now-real split stays legible. Comment-only: no runtime path, resolution order, or accepted specifier changes. Fixes#10909 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
📓 Docs Drift CheckThis PR changes 1 package(s): 16 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 seeCoarse 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 7377273ec8bb9a2b06e471b9961405a18a737894 && git checkout 7377273ec8bb9a2b06e471b9961405a18a737894
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9cc6777d3e228952bc554c5f1c4f34b063fcf105 6780fab0e3ea306d37d23bf3aa4d7b72054098d6 && git checkout -B drift-repro 9cc6777d3e228952bc554c5f1c4f34b063fcf105 && git merge --no-ff 6780fab0e3ea306d37d23bf3aa4d7b72054098d6
node scripts/docs-audit/affected-docs.mjs --json 9cc6777d3e228952bc554c5f1c4f34b063fcf105
|
Uh oh!
There was an error while loading. Please reload this page.
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 32542946606 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
… 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
Fixes#10909
Measurement at head (re-derived, not inherited)
The card's line numbers (
:37/:44for the comments,:2987/:3052for the loadsites) were measured on the #10769 branch and are stale after #10919 (moved
importFromHostto module scope) and #10948 (added ~104 lines). Re-derived bycontent on
9cc6777d3e(branch base, currentorigin/maintip at dispatch time):packages/cli/src/commands/serve.ts:36-37(email) and:43-44(sms) — both said "host copy first".Serve.CAPABILITY_PROVIDERSiteration,now around
:3058-3154—const mod: any = await import(/* webpackIgnore: true */ spec.pkg);and, for extras,const exMod: any = await import(/* webpackIgnore: true */ ex.pkg);. Both are bareimport(), noimportFromHostanywhere in this loop.Node ESM resolves a bare
import()against the importing module's ownrealpath — i.e. this CLI's — so the CLI's bundled copy of
EmailServicePlugin/SmsServicePluginalways wins; the host app's copy isnever consulted. "Host copy first" described a behaviour the code does not
have, confirmed unchanged at head. (The class stays latent, not live: all 21
Serve.CAPABILITY_PROVIDERSpackages are declared bypackages/cliitself,so bare resolution finds every one of them today — the same fact the card
recorded.)
Fix (option 1, as triage ruled)
Corrected both comments to state the measured behaviour, and made the file's
now-real split legible rather than just negating the old claim: after #10948
serve.tscontains both a host-anchored path (Serve.importConfigPlugin,for the served app's own
plugins: [...], where the app's declaration decideswhich copy wins) and a bare-import path (the capability loop, where the CLI's
copy always wins). The corrected comments name
importConfigPluginas thecontrasting case so the next reader does not assume one rule governs the whole
file. Each comment also cites
#10909as its source, matching this file's ownconvention of leaving a trail for corrections (
#5307,#5448,#10943, …all do the same nearby).
Option 2 (routing the loop through
importFromHost) was not implemented,per triage's ruling (comment
5373477292) and the harder constraint layered ontop of it: #10943 measured that
createHostImporter's undeclared-fallback pathresolves from
@objectstack/types, not the calling package — contradicting itsown docblock. Every
CAPABILITY_PROVIDERSpackage is CLI-declared and none isdeclared by the served app, so routing this loop through
importFromHosttodaywould hand all 21 to that broken fallback and, on a pnpm-isolated install, turn
a working capability into a missing one. No code path changed in this PR — only
the two comments.
#10514 interaction — checked, not fixed (per brief)
#10514 records that two
packages/cliscans readserve.tsraw (nocomment/code separator), so prose counts as code:
serve-email-config-parity.contract.test.ts—keysReadFromConfigEmail()matches
/cfgEmail\.[A-Za-z_$][\w$]*/gover the raw source.serve-multi-node-cap-advisory.pin.test.ts— three assertions matchcheckMultiNodeAllowed(...)shapes over the raw source.Neither corrected comment contains
cfgEmail.orcheckMultiNodeAllowed, soneither token vocabulary is touched. Verified both directions, not just
reasoned about: ran both test files against the pre-edit tree (
git checkout HEAD -- serve.ts, discarding the uncommitted comment edit) and again againstthe corrected tree (re-applying it from a saved patch) — 10/10 pass in both
states, byte-identical result. This edit trips neither scan and un-trips
neither scan.
Pinning the corrected claim
Considered whether the corrected statement could be pinned the way #10514's
scans pin other
serve.tsclaims. It could only be pinned by asserting, fromsource, that the capability-loop's two
import()calls carry noimportFromHostwrapper — a regex/AST assertion built solely to prove acomment's prose true, with no behavioural difference for it to guard (the
"host wins" vs "CLI wins" distinction is not independently observable from
outside the process without faking two installed copies of a provider
package). That is disproportionate machinery for a comment fix, so none was
built here; if this class of claim recurs, the multi-node-cap-advisory pin is
the pattern to follow, keyed on the real production line
(
await import(/* webpackIgnore: true */ spec.pkg)) rather than on the prose.Verification (head
6780fab0e3)pnpm --filter '@objectstack/cli^...' build—os-verify-lock: VERDICT command-exit 0 · held the lock 418s.pnpm --filter @objectstack/cli typecheck(tsc --noEmit) —os-verify-lock: VERDICT command-exit 0.pnpm --filter @objectstack/cli test -- --maxWorkers=2— full suite:Test Files 150 passed (150)/Tests 1663 passed (1663),os-verify-lock: VERDICT command-exit 0.ablate); the Two
packages/cliscans readserve.tswith no comment/code separator at all, so prose counts as code — the directionjs-comment-mask's header calls the worse one #10514 scan re-run (pre-edit vs post-edit tree, bothTest Files 2 passed (2)/Tests 10 passed (10)) is the closestbehavioural check available here, per the brief.
node scripts/pm/dispatch-gates.mjs(no paths — derives from the merge base
9cc6777d3vs HEAD) named 14 localgates; all 14 run and green:
check:changeset-gate-self-tests,check:cross-package-test-inputs,check:objectui-changeset,check:route-envelope,check:slot-lookup(
✓ slot-lookup ratchet holds: 107 unswept site(s) … none new),check:test-source-alias,check:type-source-resolution,check-adr-0087-registration.mjs,check-changeset-no-major.mjs,check-ci-filter-parity.mjs,check-cross-package-test-inputs.mjs,check-empty-changeset.mjs,check-plugin-teardown-shape.mjs,docs-audit/check-affected-docs.mjs..changeset/capability-loop-host-copy-first.mdadded (@objectstack/clipatch) — comment-only fix, still user-visible in the published source, so a
real changeset rather than
skip-changeset.⛔ Draft — PM seat reviews and enqueues. No behaviour changed; no CI wait
performed past this report per the standing lane rule.
Generated by Claude Code
Generated by Claude Code