feat(cli): add extension operation polling - #5753
Conversation
|
Thanks for the PR! Template looks good ✓ On direction: This is a natural fit. Extension mutations are already async from the client's perspective — accepted immediately, completed in the background. Without a polling mechanism, web clients have no way to correlate a specific install request with its outcome. Adding an On approach: Scope feels right and minimal. The in-memory capped operation history (100 entries) is pragmatic — persistent storage would be overkill for a polling cache. The variable narrowing in the install handler ( Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 方向:这个功能很合理。扩展变更本身就是异步的——请求被接受后在后台完成。没有轮询机制的话,web 客户端无法把某次安装请求和最终结果对应起来。返回 方案:范围合理且最小化。内存中有上限的 operation 历史(100 条)是务实的选择——对轮询缓存来说持久化存储过度了。install handler 里的变量重命名( 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependently, I would have designed this almost identically: UUID-keyed in-memory Map for operation tracking, lifecycle states (queued → running → terminal), a GET endpoint for status queries, and SDK/webui wiring. The PR's approach matches this baseline and doesn't miss anything obvious. No critical issues found:
Test ResultsCLI server extension tests (41 passed)Covers: succeeded, failed, succeeded_with_refresh_error, 404 for unknown IDs, queued/running states, eviction of oldest terminal operations (101 iterations), credential redaction in error paths. SDK extension operations test (1 passed)Verifies URL encoding of operation IDs with Typechecks
Real-scenario noteThis PR adds a daemon HTTP API endpoint, not a TUI-visible feature. Real-scenario testing would require starting the daemon in a sandbox environment with API keys. The unit tests comprehensively cover all lifecycle states, error paths, and edge cases (eviction, unknown IDs, credential redaction), providing equivalent confidence. 中文说明代码审查独立来看,我会用几乎相同的方式设计:UUID 索引的内存 Map 做 operation 追踪,生命周期状态(queued → running → 终态),GET 端点查询状态,以及 SDK/webui 的接入。PR 的方案符合这个预期,没有明显遗漏。 没有发现关键问题:
测试结果CLI server 扩展测试:41 通过。覆盖所有生命周期状态、错误路径、淘汰逻辑、凭证脱敏。 本 PR 添加的是 daemon HTTP API 端点,非 TUI 可见功能。真实场景测试需要在沙箱环境中启动 daemon 并配置 API key。单元测试已全面覆盖所有生命周期状态、错误路径和边界情况。 — Qwen Code · qwen3.7-max |
|
This is a clean, well-scoped feature PR. The implementation is exactly what I would have proposed independently — UUID-keyed in-memory operation tracking with capped history, proper lifecycle states including the important Tests are comprehensive — 41 CLI tests covering all five operation states, eviction, 404 handling, and credential redaction in error paths. The SDK test verifies URL encoding. Both typechecks pass. The follow-up commit ( The in-memory-only storage with 100-entry cap is a reasonable tradeoff, clearly documented as intentional. No concerns. Approving. ✅ 中文说明这是一个干净、范围合理的功能 PR。实现和我独立设想的完全一致——UUID 索引的内存 operation 追踪加上有上限的历史记录,完整的生命周期状态(包括重要的 测试全面——41 个 CLI 测试覆盖全部五种 operation 状态、淘汰机制、404 处理和错误路径中的凭证脱敏。SDK 测试验证了 URL 编码。两个 typecheck 都通过。 后续提交( 纯内存存储加上 100 条上限是合理的取舍,在 PR 中明确标注为有意设计。 没有顾虑。通过。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
✅ Local build + real-daemon verification (maintainer merge reference)Verified PR head 1. EnvironmentClean 2. Build & static checks
3. Tests (full files, fresh build)
The PR's new/changed tests were confirmed by name: Stderr during the run also showed the redaction working: 4. Live daemon E2E (real
|
| 检查 | 范围 | 结果 |
|---|---|---|
npm ci + prepare 构建 |
整个 monorepo | ✅ exit 0 |
tsc --noEmit |
sdk-typescript |
✅ 通过 |
tsc --noEmit |
webui |
✅ 通过 |
eslint |
全部 6 个改动源文件 | ✅ 通过 |
3. 测试(完整文件,全新构建)
| 套件 | 结果 |
|---|---|
packages/cli → src/serve/server.test.ts |
✅ 514 / 514 通过 |
packages/sdk-typescript → test/unit/DaemonClient.test.ts |
✅ 166 / 166 通过 |
逐条确认了本 PR 新增/修改的测试:
✓ queues extension install and refreshes active sessions
✓ returns 404 for unknown extension operation ids
✓ broadcasts a failed extension install with redacted error details
✓ does not report a successful extension install as failed when session refresh fails (→ succeeded_with_refresh_error)
✓ DaemonClient > extension operations > GETs an extension operation status by id (对 id 做 URL 编码)
运行过程中 stderr 也显示脱敏生效:... background task failed: https://***REDACTED***@example.com/private-ext failed。
4. 真实 daemon 端到端(真实 qwen serve 二进制,非 fake bridge)
启动 qwen serve --port 4173 --token … --workspace … --no-web,直接打新路由:
GET /workspace/extensions/operations/does-not-exist-uuid (Bearer …)
→ 404 {"error":"Extension operation \"does-not-exist-uuid\" not found","code":"extension_operation_not_found"}
GET /workspace/extensions/operations/x (无 Authorization)
→ 401 {"error":"Unauthorized"}
POST /workspace/extensions/install (Bearer …, X-Qwen-Client-Id: bogus-client, consent:true)
→ 400 {"error":"Client id \"bogus-client\" is not registered …","code":"invalid_client_id"} (不会记录 operation)也就是说在真实二进制上:新路由的路由 + 鉴权 + workspace context 都串通了,返回的正是 SDK 可消费的 code: extension_operation_not_found,被拒绝的 mutation 也不会创建 operation。四个生命周期终态(succeeded / succeeded_with_refresh_error / failed / 404)已由上面的集成套件确定性覆盖——我没有再用真实网络重跑这些,因为那需要已注册的 ACP client + 模型鉴权 + 一次真实失败的网络安装,而单测已经把这些穷尽覆盖了。
5. 观察(均不阻塞合并,当前状态可直接合)
-
result.source未脱敏(轻微一致性 / 低风险信息暴露)。 operation 顶层记录的source经过redactUrlCredentials脱敏,但 success / refresh-error 路径里result: { ...event },而 install 的event.source是原始的sourceValue(server.ts:2324)。如果用带凭据的 URL 安装(https://<token>@host/repo),原始 token 可通过轮询接口的result.source读回。实际风险很低——operationId是不可猜测的 UUIDv4、只返回给发起方、走鉴权后的 loopback/bearer 通道,而该 client 本来就是凭据的提供方。但这和其它地方的脱敏不一致;建议把result.source(或整个result)也脱敏以保持一致,或确认这是有意为之。 -
内存历史上限是安全的,但旧 id 在重启 / 100 个更新的 operation 之后会 404(PR 描述已说明)。我核了驱逐逻辑的相互作用:
MAX_EXTENSION_INSTALL_QUEUE_DEPTH = 10≪MAX_EXTENSION_OPERATION_HISTORY = 100,且深度检查在记录 operation 之前执行,所以最多只有 10 个非终态 operation,FIFO 驱逐只可能删掉已经是终态的条目——正在进行的queued/runningoperation 永远不会被驱逐。👍 设计是稳的;只是把这个已记录的"旧 id → 404"权衡提示给 web 客户端。 -
mutation 超时 2 分钟 → 10 分钟,且队列是串行的。 扩展 mutation 走单一串行队列,所以一个慢安装现在最多会占住队列(以及它后面最多 10 个排队的 mutation)10 分钟,而不是原来的 2 分钟。按 PR 描述这是为慢网络安装有意调大的——这里只提示队头阻塞的权衡。
结论: 本地构建、typecheck、lint、测试全绿,新接口在真实 daemon 二进制上行为正确。无阻塞项。✅
✅ Local runtime verification (real daemon, not just unit tests)I built the real esbuild bundle from this PR's head ( Environment
Results
Sample — success path (live): Sample — failure path (live): Credential redaction is confirmed by the unit test (daemon log shows Notes (non‑blocking)
VerdictWorks as designed across all documented states (queued/running/succeeded/failed/succeeded_with_refresh_error + 404), on a real daemon and in the unit suites. Operation ids are returned and pollable, credentials are redacted, and the SDK helper round‑trips correctly. No blocking issues found — LGTM. ✅ 中文版(合并参考)✅ 本地真实运行时验证(真实 daemon,而非仅单元测试)我基于本 PR 的最新 head( 环境
结果
成功路径示例(真实): 失败路径示例(真实): 凭据脱敏由单元测试确认(daemon 日志中带凭据的 source 显示为 说明(非阻塞)
结论在真实 daemon 与单元测试中,所有文档化状态(queued/running/succeeded/failed/succeeded_with_refresh_error + 404)均按设计工作。operation id 会返回并可轮询、凭据已脱敏、SDK helper 往返正常。未发现阻塞性问题——LGTM。✅ |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues found. Two non-blocking suggestions below for consideration.
— qwen3.7-max via Qwen Code /review
| ? refreshErr.message | ||
| : String(refreshErr), | ||
| ); | ||
| updateExtensionOperation(operationId, { |
There was a problem hiding this comment.
[Suggestion] The catch (refreshErr) block lacks a try/catch isolation guard. The new updateExtensionOperation call here is safe today (just a Map.set), but bridge.broadcastExtensionsChanged and writeStderrLine below are not wrapped. If either throws, the error cascades to the outer .catch on enqueueExtensionInstall, which overwrites the operation record from succeeded_with_refresh_error to failed and broadcasts a spurious failed event — creating a status inversion where the extension is installed but clients see failed.
Wrapping this block in a try/catch (matching the defensive pattern already used in the outer catch) would prevent the cascade:
| updateExtensionOperation(operationId, { | |
| try { | |
| updateExtensionOperation(operationId, { | |
| status: 'succeeded_with_refresh_error', | |
| result: { | |
| ...redactExtensionOperationResult(event), | |
| refreshed: 0, | |
| failed: 1, | |
| error: message.slice(0, 500), | |
| }, | |
| }); | |
| bridge.broadcastExtensionsChanged({ | |
| ...event, | |
| error: message, | |
| }); | |
| writeStderrLine( | |
| `qwen serve: extensions ${operation}: mutation succeeded but refresh failed: ${message}`, | |
| ); | |
| } catch (innerErr) { | |
| try { writeStderrLine(`qwen serve: extensions ${operation}: refresh-error handler threw: ${innerErr instanceof Error ? innerErr.message : String(innerErr)}`); } catch { /* guard */ } | |
| } |
— qwen3.7-max via Qwen Code /review
| @@ -1255,7 +1255,7 @@ export function createServeApp( | |||
| extensionInstallQueue = next.catch(() => undefined); | |||
There was a problem hiding this comment.
[Suggestion] The 10-minute timeout now applies uniformly to all five mutation types (install, enable, disable, update, uninstall), but only install involves network I/O that benefits from the longer window. Enable/disable/update/uninstall are local filesystem operations that typically complete in under a second. If one of these hangs (e.g., a filesystem lock), the caller waits 10 minutes and the queue slot is occupied the entire time.
Consider splitting into two constants — EXTENSION_INSTALL_TIMEOUT_MS = 10 * 60_000 for the install path and keeping EXTENSION_MUTATION_TIMEOUT_MS = 120_000 for the others — to limit the blast radius of a hung local operation.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
What this PR does
Adds an extension operation polling API for daemon-driven extension mutations. Extension install, enable, disable, update, and uninstall requests now return an operation id when accepted, and clients can query that id to observe whether the queued background operation is queued, running, succeeded, failed, or succeeded while session refresh failed. The SDK and daemon workspace action layer expose the new status query so web clients can build polling behavior without relying only on the asynchronous workspace event stream. The extension mutation timeout is also raised to ten minutes so slow network installs have more time to complete.
Why it's needed
Extension installation is asynchronous from the web client perspective: the request is accepted quickly, while the actual install and session refresh happen in the background. Without a polling API, a client can only infer progress from later events, and it cannot reliably inspect the final status for a specific install request. Returning an operation id gives consumers a deterministic way to connect the accepted request with its eventual outcome, including the important partial-success case where installation finished but session refresh failed.
Reviewer Test Plan
How to verify
Run the targeted daemon server tests and confirm extension install responses include an operation id, polling the operation returns succeeded after a successful install, failed after an install failure, succeeded_with_refresh_error when session refresh fails, and 404 for unknown operation ids. Run the SDK client test and confirm the operation id is URL-encoded when querying the status endpoint. Confirm SDK and webui typechecks pass so the new response and action types are consumable.
Evidence (Before & After)
Before: extension mutation requests returned only accepted: true, so a caller could not poll the daemon for the outcome of the specific background operation. After: accepted mutation requests return accepted: true plus operationId, and GET /workspace/extensions/operations/:operationId returns the operation status and result details.
Local verification: cd packages/cli && npx vitest run src/serve/server.test.ts -t "queues extension install and refreshes active sessions|broadcasts a failed extension install|does not report a successful extension install as failed when session refresh fails|returns 404 for unknown extension operation ids" passed with 4 tests. cd packages/sdk-typescript && npx vitest run test/unit/DaemonClient.test.ts -t "extension operations" passed with 1 test. Targeted ESLint and Prettier checks passed for the changed files. cd packages/sdk-typescript && npm run typecheck passed. cd packages/webui && npm run typecheck passed.
Tested on
Environment (optional)
Local macOS development checkout using targeted vitest, ESLint, Prettier, and package typecheck commands.
Risk & Scope
Linked Issues
N/A
中文说明
What this PR does
为 daemon 的扩展变更流程增加 operation 轮询接口。扩展安装、启用、禁用、更新、卸载请求被接受后会返回 operation id,客户端可以用这个 id 查询后台任务当前是 queued、running、succeeded、failed,还是安装成功但 session refresh 失败。SDK 和 daemon workspace action 层也暴露了新的状态查询能力,web 客户端可以基于这个接口实现轮询,而不是只依赖异步 workspace event。扩展 mutation 的总超时时间也延长到了十分钟,以便慢网络安装有更多完成时间。
Why it's needed
从 web 客户端视角看,扩展安装是异步的:请求会快速 accepted,真正的安装和 session refresh 在后台完成。没有轮询接口时,客户端只能从后续事件里推断进度,也无法稳定查询某一次 install 请求最终是否成功。返回 operation id 后,消费侧可以把 accepted 请求和最终结果对应起来,也能明确识别“安装完成但 session refresh 失败”这种部分成功状态。
Reviewer Test Plan
How to verify
运行目标 daemon server 测试,确认扩展安装响应包含 operation id,成功安装后轮询返回 succeeded,安装失败后返回 failed,session refresh 失败后返回 succeeded_with_refresh_error,未知 operation id 返回 404。运行 SDK client 测试,确认查询状态接口会正确 URL encode operation id。确认 SDK 和 webui typecheck 通过,说明新的响应类型和 action 类型可被消费。
Evidence (Before & After)
Before:扩展 mutation 请求只返回 accepted: true,调用方无法用 daemon 查询这次后台任务的最终结果。After:被接受的 mutation 请求会返回 accepted: true 和 operationId,并且 GET /workspace/extensions/operations/:operationId 会返回 operation 状态和结果详情。
本地验证:cd packages/cli && npx vitest run src/serve/server.test.ts -t "queues extension install and refreshes active sessions|broadcasts a failed extension install|does not report a successful extension install as failed when session refresh fails|returns 404 for unknown extension operation ids" 通过 4 个测试。cd packages/sdk-typescript && npx vitest run test/unit/DaemonClient.test.ts -t "extension operations" 通过 1 个测试。变更文件的目标 ESLint 和 Prettier 检查通过。cd packages/sdk-typescript && npm run typecheck 通过。cd packages/webui && npm run typecheck 通过。
Tested on
Environment (optional)
本地 macOS 开发环境,执行了目标 vitest、ESLint、Prettier 和 package typecheck 命令。
Risk & Scope
Linked Issues
N/A