Skip to content

fix(cli): refuse a relative plugins: [...] entry, naming the two spellings that work - #11154

Merged
os-elon merged 1 commit into
mainfrom
claude/issue-10944-refuse-relative-plugin-specifier
Aug 22, 2026
Merged

fix(cli): refuse a relative plugins: [...] entry, naming the two spellings that work#11154
os-elon merged 1 commit into
mainfrom
claude/issue-10944-refuse-relative-plugin-specifier

Conversation

@os-elon

@os-elonos-elon commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator

Fixes#10944

Ruled at triage: option B — refuse a relative plugins: [...] entry at load,
naming the two spellings that do work. Option A (resolving relative entries
against the served app's root) is deliberately not implemented: it is a
capability addition with zero measured pull, it stays a maintainer decision, and
this refusal is the collection point for such a request. The message does not
hint that it is coming.

(Note on this body: GitHub's sanitizer silently deletes short angle-bracket
fragments, so placeholders below are written [like-this] rather than with
angle brackets — the first revision of this body lost three of them.)

The defect, measured before the fix

packages/cli/src/commands/serve.tsServe.importConfigPlugin. A string entry
that is not a bare package name was handed straight to import(), which ESM
resolves against the file containing the call — the installed CLI's
@objectstack/cli/dist/commands/. The served app's root never entered the
resolution.

Measured on acb4dbc09 (the merge base), through Serve.importConfigPlugin,
against a fixture app that really did carry local-plugin.js beside its
package.json ([cli] = the CLI package root):

'./local-plugin.js' -> THREW Cannot find module '[cli]/src/commands/local-plugin.js'
imported from '[cli]/src/commands/serve.ts'
'../local-plugin.js' -> THREW Cannot find module '[cli]/src/local-plugin.js'
'..' -> LOADED the CLI's own command barrel
(CompileCommand, ValidateCommand, ServeCommand, ...)
'/abs/.../local-plugin.js' -> LOADED { name: 'app-local-plugin' }
'file:///abs/.../local-plugin.js' -> LOADED { name: 'app-local-plugin' }

The app's own file was never seen by any relative spelling, and the '..' row is
the same fact as a positive: a relative entry can load a module — it can only
ever load one belonging to the CLI.

On "silently loads nothing", precisely. It is not literally silent at
stderr: the boot loop catches the throw and prints one red
✗ Failed to load plugin: … line (console.error is not among the streams
serve mutes during boot — only process.stdout.write, console.log and
console.debug are). What was silent is everything that matters afterwards: the
boot continues, the app is served without the plugin, and the only
diagnostic names a path inside the CLI's install directory — which tells an
author nothing about the spelling being the problem. A config could therefore
carry a dead plugins: entry indefinitely while the deployment looked healthy.

The change

Refused before the try, and deliberately not wrapped in the existing
Failed to import plugin '[specifier]': … text — nothing was imported, and
calling a refusal an import failure sends the author hunting for a missing file.
The message is owned by one function (Serve.relativePluginSpecifierRefusal) so
the string a user reads is chosen and pinned rather than assembled at a call
site. Actual text:

Refused the plugin entry './local-plugin.js' in `plugins: [...]`: a RELATIVE path there is
resolved against the CLI's own installation directory, never against your app — so
it can never load a file from your project. This spelling has never worked; it used
to fail with a "Cannot find module" naming a path inside the CLI's install directory.
Use one of the two spellings that resolve from your app:
1. a package name your app DECLARES in its own package.json:
plugins: ['@mycompany/crm'] (then: pnpm add @mycompany/crm)
2. an absolute path, or a file:// URL the config computes for itself:
plugins: [new URL('./local-plugin.js', import.meta.url).href]

Bullet 2 echoes the author's own specifier, so the suggested line is
copy-pasteable, and new URL(spec, import.meta.url).href is the app-local plugin
file written the way an author actually wants it — the config computes the
absolute URL from its own location, using only resolution that already works.
The quoting switches to JSON escaping when a specifier would break single quotes
(the '.\x.js' spelling), so the suggestion never silently drops an escape.

Every specifier shape I tested, and the conclusion for each

Each was run through Serve.importConfigPlugin on the merge base first; the
matrix is pinned in serve-config-plugin-relative-refusal.test.ts.

ShapeVerdictWhy
./local-plugin.js, ./nested/plugin.jsrefusedthe filed shape
../local-plugin.jsrefusedclimbs out of the CLI's commands/ dir
.refusedmeasured: resolves to the CLI's own commands directory
..refusedmeasured: loaded the CLI's own command barrel
./refusedrelative
.\local-plugin.js, ..\local-plugin.jsrefusedNode's URL resolution normalises \ to /, so these are the same relative specifier (measured: identical resolved path to ./local-plugin.js)
/abs/local-plugin.jsuntouchedabsolute POSIX path — base-independent
file:///abs/local-plugin.jsuntouchedbase-independent
C:\app\plugin.js, C:/app/plugin.jsuntouchedWindows drive path is absolute; base-independent. (packageNameFromSpecifier already returns undefined for it via its protocol regex, so it reaches the same import() and is unaffected)
node:pathuntouchedbuiltin; measured: loads
data:text/javascript,…untouchedmeasured: loads
@mycompany/crm, chalkuntouchedbare package names — the #4719 declaration branches
local-plugin.jsuntouchednot relative under ESM: a bare specifier, even one that looks like a filename. Measured: it takes the declaration branch and gets the existing "declare it in that app's package.json" answer, which is the right one for a bare name. Deliberately not folded into this refusal — doing so would mint a rule about package names containing dots
.hidden-not-relativeuntouched. not followed by a separator; not a relative specifier

So the predicate is /^\.\.?(?:[\\/]|$)/ — deliberately narrower than
packageNameFromSpecifier(s) === undefined, which also answers "not a package"
for every base-independent spelling above.

Reverse verification — prediction before each run, then the observation

All legs run against packages/cli/src/commands/serve-config-plugin-relative-refusal.test.ts
and serve-config-plugin-host-resolution.test.ts (15 tests). The fix was
committed first, so each leg was restored with git checkout HEAD -- [path]
(never git stash).

LegPredictionObserved
0 — unmutatedGREENGREEN, 15/15
1 — remove the refusal from importConfigPluginREDRED, 4 failed / 11 passed
2 — isRelativePluginSpecifierreturn true (the "refuse everything" bad implementation)RED on the loads, not on the refusalRED, 9 failed / 6 passed — the headline test fails on the declared-package and absolute/file:// loads, and every load pin in the #10908 file fails with it
3 — narrow the predicate to /^\.\//RED on the shape matrix onlyRED, exactly 1 failed (the matrix, on .., ., .\, ..\, ../)
4 — drop spelling (b) from the messageRED on the spelling-(b) pin onlyRED, exactly 1 failed (names spelling (b))
5 — restoredGREENGREEN, 15/15; git diff HEAD empty

Leg 2 is the preservation leg the third acceptance criterion asks for: a
suite that only asserted "the relative entry is refused" would have passed on
that implementation. It does not, because the headline test asserts the refusal
and both working spellings in one body.

No rebuild is involved in these legs and none is claimed: the subject
(Serve) is imported relatively inside its own package (./serve.js), which
vitest resolves to packages/cli/src/commands/serve.ts — source, not dist/.
Leg 1 going red is the inline proof that the mutation reached the running code,
and leg 5 restores it (git diff HEAD empty, byte-identical).

Fixture triage — the pin this supersedes

serve-config-plugin-host-resolution.test.ts carried
keeps a RELATIVE specifier anchored to serve.ts, not to @objectstack/types,
which pinned the exact branch this card removes: its assertions would have kept
passing only because the resolution it described no longer runs. It is
replaced, not reworded — the slot now asserts that the refusal fires and
that an absolute path still loads, which is what makes it a narrowing rather than
a ban. The full refusal coverage lives in the new file.

I scanned by the rule's consumer radius, not by the edited package:
Serve.importConfigPlugin has exactly one non-test call site (the boot loop in
serve.ts), and no fixture, doc, example app or test anywhere in the repo writes
a relative plugins: spelling — which is also the "zero measured pull" the
triage rests on.

What this deliberately does not do

  • No app-root resolution (option A). Out of authorised scope.
  • The boot loop's behaviour is unchanged. A refused entry is still caught by
    the loop's existing catch, printed as one red line, and the boot continues —
    the same as every other failed plugin load. Making a bad plugins: entry fail
    the boot is a separate policy question about the whole loop, not about this
    spelling.
  • loadConfig() is untouched.os validate / os doctor / os lint do not
    read plugins: entries today; adding a config-time scan there is a wider
    surface than this card and was not authorised.

Verification

Union derived with node scripts/pm/dispatch-gates.mjs (no path arguments
the script takes its own changeset from the merge base). All at
2ff2d33130, the head commit of this PR, on a clean tree.

Merge base acb4dbc09703d5c6145efb376c50ea12dfe9f41c; three-dot changeset
(git diff --name-only $(git merge-base origin/main HEAD) HEAD):

.changeset/refuse-relative-config-plugin-specifier.md
packages/cli/src/commands/serve-config-plugin-host-resolution.test.ts
packages/cli/src/commands/serve-config-plugin-relative-refusal.test.ts
packages/cli/src/commands/serve.ts

Every gate the derivation named, plus the convention-triggered family for a new
test file, run locally and green (each quoted from the gate's own verdict line):

  • check:nul-bytesOK (scanned 6457 text file(s) … no raw ASCII control bytes)
  • check:changeset-gate-self-tests — 118 + 212 + 116 assertions, all
  • check:cross-package-test-inputsOK: 13 package(s) read outside themselves, all declared
  • check:objectui-changeset✓ objectui-range --self-test: all checks passed
  • check:route-envelope — exit 0, ratchet holds (its two ⚠ ratchet #9559 rows are pre-existing packages/rest entries, untouched here)
  • check:slot-lookup✓ slot-lookup ratchet holds: 107 unswept site(s) … none new
  • check:test-source-aliasOK — 72 packages with tests scanned
  • check:type-source-resolutionOK — 77 packages with a tsconfig.json scanned
  • check-adr-0087-registration✓ this PR adds no declared-breaking changeset
  • check-changeset-no-major✓ This diff introduces no 'major' bump.
  • check-ci-filter-parityOK: all 86 declared cross-package glob(s) … covered
  • check-empty-changeset✓ No empty-frontmatter changeset introduced by this diff
  • check-plugin-teardown-shape✓ 63 Plugin implementation(s) across 4479 source(s)
  • docs-audit/check-affected-docs — exit 0
  • check:query-options-erasure✓ ratchet holds: 67 unswept non-test site(s) … none new
  • check:engine-double-contractOK — 381 pinned, 133 in the DEBT ledger, 2 exempt
  • check:where-matcher✓ 280 matcher(s) discovered … 0 silently-wrong … none new
  • check:type-check-coverageOK — 65/78 workspace packages type-checked
  • check:type-check-debt --re-measure — first run refused (@objectstack/service-knowledge had no built type entry point). Treated as NOT MEASURED, not as a pass: built the closure exactly as lint.yml does (turbo run build --filter='./packages/*' --filter='./packages/*/*', 70/70 successful) and re-ran → OK — 33 ledger entr(ies) re-measured in 217.5s, 1895 raw tsc error(s) total, none above its recorded number

Package suites:

  • pnpm --filter @objectstack/cli testTest Files 157 passed (157) / Tests 1730 passed (1730) (full suite, not just the changed files)
  • pnpm --filter @objectstack/cli typechecktsc --noEmit, exit 0

All heavy runs went through scripts/pm/os-verify-lock.sh. CI is still in flight:
the lint farm, the cross-package test shards and the packages outside
@objectstack/cli have not been run locally and are the half CI owns.

Changeset

.changeset/refuse-relative-config-plugin-specifier.md, @objectstack/cli: minor.
minor rather than patch because one shape genuinely changes what it loads:
plugins: ['.'] / plugins: ['..'] resolved into the CLI's own package and could
register its command barrel as a plugin; those are now refused. The changeset
states plainly that the spelling never worked but failed quietly enough to be
missed
, so an author who has been carrying a dead entry now gets an answer
instead of a path into the CLI's install directory. Not declared breaking and not
major, so no ADR-0087 disposition marker is required (the gate confirms:
this PR adds no declared-breaking changeset).

Out-of-scope findings

None. No unrelated defect was encountered.


Generated by Claude Code

…ellings that work (#10944)
A string entry that is not a bare package name was handed straight to
`import()`, which ESM resolves against `serve.ts` itself — the installed CLI's
directory. The served app's root never entered the resolution, so a relative
path could never address anything the app owns; measured, `'..'` even loaded
this package's own command barrel. The boot loop caught the failure, printed
one red line naming a path inside the CLI, and served the app without the
plugin.
Ruled at triage on #10944: refuse the spelling at load, naming the two that do
resolve from the app (a declared bare package name, an absolute/`file://`
path). No accepted set widens — the spelling has never loaded an app's file.
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 17bad125cca89674cfd2dca59025b0d31da68968.

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 17bad125cca89674cfd2dca59025b0d31da68968packageMentionDocs.

Which tree this was computed on

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

⚠️ 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 17bad125cca89674cfd2dca59025b0d31da68968 → 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 22, 2026
@os-elonClaude

Copy link
Copy Markdown
CollaboratorAuthor

复核记录 —— open question 答 A(即本 PR 现状),另确认一处我简报里的前提被你纠正对了

⛔ 尚未入队:Dogfood Regression Gatein_progress,未完成的检查不是通过。绿了我再逐名复核并入队。以下与 CI 无关,先答清楚。

open question:A,就放在 config-plugin load,⛔ 不扩到 loadConfig()

你的论证成立,而且比 triage 措辞更准确:triage 写的是「refuse at config load」,但 serve.ts 自己做 bundleRequire 而不走 loadConfig() —— 所以一个只落在 loadConfig() 的拒绝会完全错过 os serve,而 os serve 正是这个缺陷所在的命令。放在唯一真正读这些条目的地方是对的。

⭐ 更重要的是你没有擅自把 B 立成卡,理由也对:那是本裁定的刻意范围边界,不是附带缺陷。附带发现该立卡(你今天的同僚立了 #11130#11094),范围边界不该 —— 未经请求就立卡等于 mint 一个维护者没见过的范围。这个区分我认可,B 若要做就走它自己的 triage。

⚠️ 你纠正了我简报里的一个前提,纠正得对

我在派发里写「今天是静默什么都不加载(不是报错)」。你实测出来的是:stderr 上并不静默 —— boot loop 既有的 catch 会打印红色的 Failed to load plugin,因为 console.error不在 serve 启动时静音的那组流里(被静音的只有 process.stdout.write / console.log / console.debug)。

真正静默的是之后的一切:boot 继续、app 在没有那个插件的情况下被服务,而唯一那行诊断指向 CLI 安装目录里的路径,所以拼写本身永远读不出是问题。

⭐ 这个区别不是措辞问题,它解释了缺陷为什么能活这么久:不是没有输出,是输出指向了错误的地方。 卡自己的前提(解析基址是 CLI 目录,app 相对路径永不可能工作)完全成立 —— 是我转述时把「静默」说宽了。记在这里,免得它再被当成"无声失败"复述下去。

⭐ 保存腿正是这张卡最容易漏掉的一半,而它真的抓到了

验收第 3 条我在派发里特意压过,你的腿 2(把 isRelativePluginSpecifier 改成 return true,即"全拒"坏实现)观察到 9 failed / 6 passed,而且你自己点出了关键的一句:

A suite that only asserted 'relative is refused' would have stayed green here.

这正是它存在的理由。serve-config-plugin-host-resolution.test.ts 里每一条 load pin 都红了(已声明裸包、app 副本优先、INSTALL 补救、DECLARE 诊断、未声明的 chalk、绝对 + file://),说明"没有误伤正确用法"是被真正守住的,不是被假定的。

腿 3 与腿 4 各自恰好 1 个失败,也说明矩阵与消息 pin 是分别敏感的,没有互相掩盖。

说明符形状:几处判断值得记下来

  • '..' 实测 LOADED 了 CLI 自己的命令 barrelCompileCommandValidateCommand…)—— 这是全篇最有说服力的一条证据:相对条目加载一个模块,但只会是 CLI 自己的。比任何"找不到模块"都更能说明基址错在哪。
  • local-plugin.js 在 ESM 下不是相对说明符,是裸说明符(尽管长得像文件名),你让它继续走既有的声明分支,并故意不折进来 —— 折进来会 mint 一条关于"含点的包名"的规则。这个自我限制是对的。
  • Windows 盘符路径是绝对的、基址无关,实测只因路径在本 OS 不存在而失败、从未被拒绝 —— 边界没有误伤。
  • 谓词故意窄于packageNameFromSpecifier(s) === undefined,因为后者对每一种基址无关拼写也答"不是包"。这个取舍写进 PR 是对的。

拒绝消息

达标:说明了为什么(相对路径对着 CLI 安装目录解析)、承认了历史("这个拼写从来没工作过,它过去以一个指向 CLI 安装目录的 Cannot find module 失败")、给了两种可用拼写的可直接照抄的例子,并且 ⛔ 没有暗示 option A 即将到来 —— 那条 fence 守住了。

其它

主动核了与 #11143 的文件重叠 = 零,正确。changeset 讲清了用户可见的行为变更(那拼写从未生效,但此前静默;带着它的 config 升级后会在 config load 期直接失败)。


Generated by Claude Code

@os-elon
os-elon marked this pull request as ready for review August 22, 2026 22:15
@os-elon
os-elon added this pull request to the merge queueAug 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    ✗ Build failed in 6.00s
    

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

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

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

历史信号:

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

分诊清单:

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

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

Merged via the queue into main with commit e598b1cAug 22, 2026
35 checks passed
@os-elon
os-elon deleted the claude/issue-10944-refuse-relative-plugin-specifier branch August 22, 2026 22:26
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A relative plugins: ['./local-plugin.js'] entry resolves against the CLI's own directory, so an app-relative plugin path can never work

1 participant

@os-elon