fix(cli): reject unsupported extension scopes

[tt-a1i]

What this PR does

This PR narrows qwen extensions enable --scope and qwen extensions disable --scope to the scopes that extension enablement actually supports: user and workspace.

It also awaits the async extension manager enable/disable calls so rejected manager failures are handled by the existing command error paths instead of printing success before an unhandled rejection.

Why it's needed

The commands previously validated --scope against every SettingScope value, including system and systemdefaults. The handlers only special-cased workspace, so any other accepted scope was sent to the extension manager as User. That made commands like qwen extensions enable my-extension --scope=system report success for system while writing user-scope enablement config.

Rejecting unsupported scopes at command validation and handler level keeps CLI output aligned with the actual persisted scope.

Reviewer Test Plan

How to verify

Run the focused extension command tests and confirm that system / systemdefaults are rejected for both enable and disable, while user and workspace continue to pass.

Evidence (Before & After)

Before: --scope=system and --scope=systemdefaults passed yargs validation and were mapped to user-scope manager calls.

After: those unsupported scopes fail validation with Invalid scope, and direct handler calls do not invoke the manager with user scope. Async manager rejections are also covered by tests using rejected promises.

Tested on

OS	Status
🍏 macOS	✅ tested
🪟 Windows	⚠️ not tested
🐧 Linux	⚠️ not tested

Environment (optional)

Local workspace tests on macOS with the repository npm workspace setup.

Commands run:

npm test --workspace=packages/cli -- commands/extensions/enable.test.ts commands/extensions/disable.test.ts --coverage.enabled=false
npx prettier --check packages/cli/src/commands/extensions/utils.ts packages/cli/src/commands/extensions/enable.ts packages/cli/src/commands/extensions/disable.ts packages/cli/src/commands/extensions/enable.test.ts packages/cli/src/commands/extensions/disable.test.ts
npx eslint packages/cli/src/commands/extensions/utils.ts packages/cli/src/commands/extensions/enable.ts packages/cli/src/commands/extensions/disable.ts packages/cli/src/commands/extensions/enable.test.ts packages/cli/src/commands/extensions/disable.test.ts
git diff --check

Also run:

npm run typecheck --workspace=packages/cli --if-present

This currently fails on unrelated existing BaseTextInput.tsx / ink type resolution errors:

src/ui/components/BaseTextInput.tsx(25,52): error TS2307: Cannot find module 'ink/dom' or its corresponding type declarations.
src/ui/components/BaseTextInput.tsx(26,27): error TS2307: Cannot find module 'ink/components/CursorContext' or its corresponding type declarations.
src/ui/components/BaseTextInput.tsx(310,9): error TS18046: 'cursorCtx' is of type 'unknown'.
src/ui/components/BaseTextInput.tsx(334,7): error TS18046: 'cursorCtx' is of type 'unknown'.
src/ui/components/BaseTextInput.tsx(341,7): error TS18046: 'cursorCtx' is of type 'unknown'.

Sub-agent review: initial async-error finding was addressed; follow-up review found no blocking issues.

Risk & Scope

• Main risk or tradeoff: Users who previously passed system or systemdefaults now get a validation error instead of silently writing user-scope enablement config.
• Not validated / out of scope: System-level extension enablement support is not added here; this PR only rejects unsupported scopes.
• Breaking changes / migration notes: None. The affected scope values did not work as requested before.

Linked Issues

Fixes #5712

AI Assistance Disclosure

I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.

中文说明

What this PR does

这个 PR 将 qwen extensions enable --scope 和 qwen extensions disable --scope 收窄到 extension enablement 实际支持的范围：user 和 workspace。

同时，handler 现在会 await async 的 extension manager enable/disable 调用，让 manager 的 rejected failure 进入现有命令错误路径，而不是先打印成功再出现未处理 rejection。

Why it's needed

这些命令之前会用所有 SettingScope 值校验 --scope，包括 system 和 systemdefaults。但 handler 只特殊处理 workspace，其他已通过校验的 scope 都会被当成 User 传给 extension manager。这样 qwen extensions enable my-extension --scope=system 会报告 system scope 成功，但实际写入 user-scope enablement config。

在命令校验和 handler 层拒绝不支持的 scope，可以让 CLI 输出和真实持久化 scope 保持一致。

Reviewer Test Plan

How to verify

运行 focused extension command tests，确认 enable 和 disable 都会拒绝 system / systemdefaults，同时 user 和 workspace 仍然通过。

Evidence (Before & After)

Before: --scope=system 和 --scope=systemdefaults 可以通过 yargs 校验，并被映射成 user-scope manager 调用。

After: 这些不支持的 scope 会以 Invalid scope 校验失败，直接调用 handler 时也不会再用 user scope 调 manager。async manager rejection 也用 rejected promise 测试覆盖。

Tested on

OS	Status
🍏 macOS	✅ tested
🪟 Windows	⚠️ not tested
🐧 Linux	⚠️ not tested

Environment (optional)

在 macOS 上使用仓库 npm workspace 进行本地测试。

已运行命令：

npm test --workspace=packages/cli -- commands/extensions/enable.test.ts commands/extensions/disable.test.ts --coverage.enabled=false
npx prettier --check packages/cli/src/commands/extensions/utils.ts packages/cli/src/commands/extensions/enable.ts packages/cli/src/commands/extensions/disable.ts packages/cli/src/commands/extensions/enable.test.ts packages/cli/src/commands/extensions/disable.test.ts
npx eslint packages/cli/src/commands/extensions/utils.ts packages/cli/src/commands/extensions/enable.ts packages/cli/src/commands/extensions/disable.ts packages/cli/src/commands/extensions/enable.test.ts packages/cli/src/commands/extensions/disable.test.ts
git diff --check

也运行了：

npm run typecheck --workspace=packages/cli --if-present

该命令目前因既有且无关的 BaseTextInput.tsx / ink 类型解析错误失败：

src/ui/components/BaseTextInput.tsx(25,52): error TS2307: Cannot find module 'ink/dom' or its corresponding type declarations.
src/ui/components/BaseTextInput.tsx(26,27): error TS2307: Cannot find module 'ink/components/CursorContext' or its corresponding type declarations.
src/ui/components/BaseTextInput.tsx(310,9): error TS18046: 'cursorCtx' is of type 'unknown'.
src/ui/components/BaseTextInput.tsx(334,7): error TS18046: 'cursorCtx' is of type 'unknown'.
src/ui/components/BaseTextInput.tsx(341,7): error TS18046: 'cursorCtx' is of type 'unknown'.

子代理审查：首次审查发现的 async error handling 问题已修复；复核未发现阻塞问题。

Risk & Scope

• Main risk or tradeoff: 之前传入 system 或 systemdefaults 的用户现在会收到校验错误，而不是静默写入 user-scope enablement config。
• Not validated / out of scope: 本 PR 不新增 system-level extension enablement 支持，只拒绝当前不支持的 scope。
• Breaking changes / migration notes: 无。这些 scope 值之前也没有按请求的 scope 生效。

Linked Issues

Fixes #5712

AI Assistance Disclosure

I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Thanks for the PR, @tt-a1i!

Template looks good ✓ — all required sections present, bilingual, linked to #5712.

On direction: this is a straightforward bug fix for a real issue. The --scope=system / --scope=systemdefaults values pass yargs validation but silently write to user-scope config, producing misleading success messages. Fixing this at the validation layer is the right place. Issue #5712 already documents and closes on this exact behavior.

On approach: the scope feels tight and correct. Extracting resolveExtensionCommandScope into utils.ts and reusing it from both the yargs .check() and the handler is clean — one source of truth for valid scopes. The await on the async enableExtension/disableExtension calls is a legitimate fix for the unhandled rejection path. No unrelated changes or drive-by refactors.

Moving on to code review. 🔍

中文说明

感谢贡献，@tt-a1i！

模板完整 ✓ — 所有必填部分齐全，双语，已关联 #5712。

方向：这是一个直接的 bug 修复，解决真实问题。--scope=system / --scope=systemdefaults 通过了 yargs 校验却静默写入 user-scope 配置，导致成功信息误导用户。在校验层修复是正确的做法。Issue #5712 已经记录并关闭了这个确切行为。

方案：范围紧凑且正确。将 resolveExtensionCommandScope 提取到 utils.ts 并从 yargs .check() 和 handler 中复用——单一真相来源。对 async enableExtension/disableExtension 调用加 await 是对未处理 rejection 路径的合理修复。没有无关改动或顺手重构。

进入代码审查 🔍

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

2a. Code Review

Independent proposal: I would extract a shared scope-validation function (e.g. resolveExtensionCommandScope) in utils.ts, restrict valid values to [User, Workspace], call it from both the yargs .check() and the handler body in enable.ts / disable.ts, and await the async enableExtension/disableExtension calls.

PR matches this exactly. The implementation is clean — EXTENSION_COMMAND_SCOPES constant, case-insensitive matching via .find(), thrown error with a clear message listing valid scopes. The handler simplification replaces duplicated if/else with a single resolveExtensionCommandScope + await call. The .check() validators delegate to the same function. No unrelated changes.

Reuse check: The new function is the right level of abstraction for this use case — no existing shared validator was being missed.

One observation (not a blocker): enable.ts and disable.ts handle errors differently — enable throws FatalConfigError, disable writes to stderr and calls process.exit(1). This is a pre-existing asymmetry, not introduced by this PR. Worth a follow-up for consistency, but out of scope here.

No critical blockers or AGENTS.md violations found. ✅

2b. Real-Scenario Testing

Before (installed qwen v0.18.5 — bug reproduces)

$ qwen extensions enable test-extension --scope=system 2>&1
Extension "test-extension" successfully enabled for scope "system".

$ qwen extensions disable test-extension --scope=systemdefaults 2>&1
Extension "test-extension" successfully disabled for scope "systemdefaults".

Both commands accept system and systemdefaults scopes and report success — but the actual enablement config is silently written at user scope. Bug confirmed.

After (PR code — logic verification)

npm run dev was not available in this CI environment due to a pre-existing ink/dom module resolution error (the same BaseTextInput.tsx issue noted in the PR body). Verified the fix logic via standalone script + unit tests:

$ node /tmp/verify-scope.mjs
=== Verification of resolveExtensionCommandScope ===

user → User
workspace → Workspace
User → User
Workspace → Workspace
undefined → User
system → ✅ REJECTED: Invalid scope: system. Please use one of user, workspace.
systemdefaults → ✅ REJECTED: Invalid scope: systemdefaults. Please use one of user, workspace.
System → ✅ REJECTED: Invalid scope: System. Please use one of user, workspace.
SystemDefaults → ✅ REJECTED: Invalid scope: SystemDefaults. Please use one of user, workspace.
invalid → ✅ REJECTED: Invalid scope: invalid. Please use one of user, workspace.

Unit tests (worktree, 29/29 passed)

$ cd packages/cli && npx vitest run src/commands/extensions/enable.test.ts src/commands/extensions/disable.test.ts src/commands/extensions/utils.test.ts --coverage.enabled=false

 ✓ src/commands/extensions/utils.test.ts (11 tests) 11ms
 ✓ src/commands/extensions/disable.test.ts (9 tests) 38ms
 ✓ src/commands/extensions/enable.test.ts (9 tests) 36ms

 Test Files  3 passed (3)
      Tests  29 passed (29)
   Duration  6.05s

All new test cases pass: unsupported scope rejection at yargs validation level, unsupported scope rejection at handler level, and async error propagation via mockRejectedValueOnce.

中文说明

2a. 代码审查

独立方案： 我会在 utils.ts 中提取一个共享的 scope 校验函数（如 resolveExtensionCommandScope），将有效值限制为 [User, Workspace]，在 yargs .check() 和 handler 中都调用，并对 async 的 enableExtension/disableExtension 加 await。

PR 完全匹配此方案。 实现干净——EXTENSION_COMMAND_SCOPES 常量、通过 .find() 做大小写不敏感匹配、抛出带有有效值列表的清晰错误信息。handler 简化用单个 resolveExtensionCommandScope + await 替代了重复的 if/else。.check() 校验器也委托到同一函数。没有无关改动。

一个观察（非阻塞）： enable.ts 和 disable.ts 的错误处理方式不同——enable 抛出 FatalConfigError，disable 写 stderr 然后调用 process.exit(1)。这是 PR 之前就存在的不对称，不是本次引入的。值得后续跟进一致性，但不在本 PR 范围内。

未发现关键阻塞或 AGENTS.md 违规。✅

2b. 实际场景测试

修复前（已安装的 qwen v0.18.5 — 复现 bug）

--scope=system 和 --scope=systemdefaults 被接受并报告成功，但实际写入的是 user-scope 配置。Bug 已确认。

修复后（PR 代码 — 逻辑验证）

由于 CI 环境中 ink/dom 模块解析的预存问题，无法运行 npm run dev（PR body 中提到的同一个 BaseTextInput.tsx 问题）。通过独立脚本 + 单元测试验证了修复逻辑：所有不支持的 scope 值都被正确拒绝。

单元测试（worktree，29/29 通过）

所有新增测试用例通过：yargs 校验层的 scope 拒绝、handler 层的 scope 拒绝、以及通过 mockRejectedValueOnce 的异步错误传播。

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Reflection

This PR is what a good bug fix looks like: a real problem, a minimal change, and clean execution.

The bug is concrete — --scope=system lies to the user by reporting "system" while silently writing user-scope config. The fix is surgical: one shared validation function, two call sites updated, one await added to fix the async rejection path. No scope creep, no drive-by refactors, no unnecessary abstractions.

The implementation matches what I'd have proposed independently. The resolveExtensionCommandScope function is the right granularity — shared between yargs validation and handler logic, keeping a single source of truth. The test updates are thorough: new cases for unsupported scope rejection at both the validation and handler layers, plus the switch from mockImplementationOnce(() => { throw }) to mockRejectedValueOnce correctly matches the now-awaited async calls.

All 29 unit tests pass. The standalone logic verification confirms all unsupported scopes are rejected. The installed CLI (v0.18.5) clearly reproduces the bug.

One minor note: I couldn't run a full before/after with npm run dev due to the pre-existing ink/dom and acp-bridge module resolution issues in this CI environment — but the unit tests and logic verification cover the fix comprehensively.

The pre-existing enable/disable error handling asymmetry (FatalConfigError vs process.exit(1)) is worth a future cleanup but correctly left out of this PR.

Verdict: approve. Ship it. ✅

中文说明

反思

这个 PR 是一个优秀 bug 修复的典范：真实问题、最小改动、干净实现。

Bug 是具体的——--scope=system 欺骗用户，报告 "system" 却静默写入 user-scope 配置。修复是精准的：一个共享校验函数、两个调用点更新、一个 await 修复异步 rejection 路径。没有范围蔓延、没有顺手重构、没有不必要的抽象。

实现与我独立提出的方案完全一致。resolveExtensionCommandScope 函数的粒度恰到好处——在 yargs 校验和 handler 逻辑间共享，保持单一真相来源。测试更新全面：在校验层和 handler 层都新增了不支持 scope 的拒绝用例，从 mockImplementationOnce(() => { throw }) 切换到 mockRejectedValueOnce 正确匹配了现在被 await 的异步调用。

29 个单元测试全部通过。独立逻辑验证确认所有不支持的 scope 都被拒绝。已安装的 CLI（v0.18.5）清楚复现了 bug。

一个小备注：由于 CI 环境中预存的 ink/dom 和 acp-bridge 模块解析问题，无法运行完整的 before/after npm run dev 测试——但单元测试和逻辑验证已全面覆盖了修复。

enable/disable 的错误处理不对称（FatalConfigError vs process.exit(1)）值得后续清理，但正确地未纳入本 PR。

结论：批准。 可以合入。✅

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[wenshao]

✅ Local verification report (maintainer)

Built and tested this PR locally to confirm the fix before merge.

Environment

• macOS (Darwin 25.5), Node v22.22.2
• PR head 2ad448421, base origin/main 2fd2104fa (the extensions/ files are identical between the PR base and current origin/main 2ddf9d275, so the PR head is a faithful base)
• Method: isolated git worktree at the PR head with a clean npm ci + fresh core/acp-bridge build, then verified each claim independently.

Results

Check	Command	Result
Target tests	vitest run extensions/{enable,disable,utils}.test.ts	✅ 29/29 pass
New tests	the 4 scope-rejection cases	✅ pass
Lint	eslint on all 6 changed files	✅ clean
Types	tsc --noEmit (packages/cli)	✅ exit 0, 0 errors
Format	prettier --check on all 6 files	✅ clean

Mutation test (the key evidence). I reverted the three source files (enable.ts, disable.ts, utils.ts) back to main while keeping the PR's new tests. 6 tests fail, covering both fixes:

• Scope rejection (4): yargs validation + handler, for enable & disable — old code accepts --scope=system and maps it to User, so should fail if unsupported system scopes and should reject … without (en/dis)abling at user scope all fail.
• Missing await (2): the async-rejection tests (mockRejectedValueOnce) fail because old code doesn't await the manager call — the rejection never reaches the catch, e.g. expect(mockWriteStderrLine).toHaveBeenCalledWith('Disable failed') is never satisfied and enable never rejects.

The 12 unrelated tests stay green. Both fixes are genuinely guarded.

Correctness review

• New resolveExtensionCommandScope() is a single source of truth: narrows accepted scopes to [user, workspace] (case-insensitive), defaults undefined → User, and throws Invalid scope: <x>. Please use one of user, workspace. for anything else. It is used by both the yargs .check() validator and the handler (defense in depth).
• Handlers now await the manager call, so a rejected manager promise flows into the existing error path (enable → FatalConfigError; disable → stderr + exit 1) instead of printing success before an unhandled rejection.
• Two test-infra changes are load-bearing and correct: vi.mock('./utils.js', importOriginal …) with ...actual keeps the real resolveExtensionCommandScope while overriding only getExtensionManager; and the settings mock now supplies SettingScope (faithful to the real enum User/Workspace/System/SystemDefaults) because utils.ts now imports it.
• Behavior change is exactly the intended fix (bug(cli): extension scope accepts unsupported system values #5712): system / systemdefaults are now rejected instead of silently writing user-scope config. No other scope is affected — valid user / workspace and the no-scope default are unchanged.

Note on the documented typecheck failure. As with this author's other PRs, the ink/dom / BaseTextInput.tsx errors do not reproduce on a clean npm ci — tsc --noEmit for packages/cli passes with 0 errors. That failure is a stale/incomplete local install on the author's machine, not a real repo issue; the PR itself is type-clean.

Minor / non-blocking observation. In both handlers getExtensionManager() is awaited before the scope is validated, so a directly-invoked handleEnable/handleDisable with an invalid scope instantiates the manager before rejecting. Negligible in practice (via the CLI the yargs .check() rejects first, and instantiation is side-effect-light) — could be reordered if you want the validation strictly first.

Verdict: LGTM. Both fixes correct and mutation-verified, type/lint/format clean. Safe to merge from my side — the documented "typecheck failure" is an author-local artifact, not a blocker.

🇨🇳 中文版

✅ 本地验证报告（维护者）

合并前在本地构建并测试了本 PR 以确认修复。

环境

• macOS（Darwin 25.5），Node v22.22.2
• PR head 2ad448421，base origin/main 2fd2104fa（extensions/ 这些文件在 PR base 与当前 origin/main 2ddf9d275 之间完全一致，所以 PR head 是忠实基线）
• 方法：在 PR head 上建独立 git worktree，做干净的 npm ci 并重建 core/acp-bridge，再逐条独立验证。

结果

检查项	命令	结果
目标测试	vitest run extensions/{enable,disable,utils}.test.ts	✅ 29/29 通过
新增测试	4 个 scope 拒绝用例	✅ 通过
Lint	对全部 6 个改动文件跑 eslint	✅ 干净
类型	tsc --noEmit（packages/cli）	✅ exit 0，0 错误
格式	对全部 6 个文件跑 prettier --check	✅ 干净

变异测试（关键证据）。 我把三个源文件（enable.ts、disable.ts、utils.ts）还原成 main，保留 PR 的新测试。6 个测试失败，覆盖两个修复：

• scope 拒绝（4 个）： enable 和 disable 的 yargs 校验 + handler —— 旧代码接受 --scope=system 并映射成 User，所以 should fail if unsupported system scopes 和 should reject … without (en/dis)abling at user scope 全部失败。
• 缺失 await（2 个）： async rejection 测试（mockRejectedValueOnce）失败，因为旧代码没 await manager 调用 —— rejection 进不了 catch，例如 expect(mockWriteStderrLine).toHaveBeenCalledWith('Disable failed') 永远不满足，enable 也不会 reject。

12 个无关测试保持绿色。两个修复都被测试真正挡住了。

正确性审查

• 新的 resolveExtensionCommandScope() 是单一事实来源：把可接受的 scope 收窄到 [user, workspace]（大小写不敏感），undefined → User 默认，其余一律抛 Invalid scope: <x>. Please use one of user, workspace.。它同时被 yargs .check() 校验器和 handler 使用（双重防线）。
• handler 现在 await manager 调用，因此 manager 的 rejected promise 会进入既有错误路径（enable → FatalConfigError；disable → stderr + exit 1），而不是先打印成功再出现未处理 rejection。
• 两处测试基建改动是关键且正确的：vi.mock('./utils.js', importOriginal …) 配合 ...actual 保留了真实的 resolveExtensionCommandScope，只覆盖 getExtensionManager；settings mock 现在提供 SettingScope（与真实枚举 User/Workspace/System/SystemDefaults 一致），因为 utils.ts 现在 import 了它。
• 行为变化正是预期的修复（bug(cli): extension scope accepts unsupported system values #5712）：system / systemdefaults 现在被拒绝，而不是静默写入 user-scope 配置。其它 scope 不受影响 —— 合法的 user / workspace 和不带 scope 的默认值都不变。

关于记录的 typecheck 失败。 和这位作者的其它 PR 一样，ink/dom / BaseTextInput.tsx 报错在干净的 npm ci 下复现不出来 —— packages/cli 的 tsc --noEmit 0 错误通过。那是作者机器上陈旧/不完整的本地安装产物，不是仓库真实问题；本 PR 类型完全干净。

次要 / 非阻塞观察。 两个 handler 都在校验 scope 之前就 await getExtensionManager()，所以直接调用 handleEnable/handleDisable 且 scope 非法时，会先实例化 manager 再拒绝。实际影响可忽略（走 CLI 时 yargs .check() 会先拒绝，且实例化几乎没副作用）—— 如果想让校验严格在前，可调整顺序。

结论：LGTM。 两个修复都正确且经变异测试验证，类型/lint/格式干净。从我这边看可以安全合并 —— 记录的「typecheck 失败」是作者本地环境产物，不是阻塞项。