Skip to content

feat(serve): support runtime workspace removal - #6745

Merged
doudouOUC merged 21 commits into
QwenLM:mainfrom
doudouOUC:feat/workspace-runtime-removal
Jul 13, 2026
Merged

feat(serve): support runtime workspace removal#6745
doudouOUC merged 21 commits into
QwenLM:mainfrom
doudouOUC:feat/workspace-runtime-removal

Conversation

@doudouOUC

Copy link
Copy Markdown
Collaborator

What this PR does

This PR adds synchronous runtime removal for removable secondary workspaces. Production daemons advertise workspace_runtime_removal, capabilities identify removable runtimes, and DELETE /workspaces/:workspace performs a two-phase drain with an optional force mode, removes every known persistent registration alias, tears down workspace-owned sessions, prompts, pending starts, ACP connections, memory tasks, sub-session launchers, bridges, and channel workers, then releases the workspace path for a clean re-add.

The implementation keeps primary and explicitly configured startup workspaces non-removable, preserves the existing forget-only registration endpoint, shares the same memoized disposer between hot removal and daemon shutdown, and rolls every admission/ACP/worker gate back if a non-force removal finds activity or persistence fails. It also adds the typed SDK operation, WebUI action, Web Shell confirmation and busy/force flow, protocol documentation, and focused concurrency and lifecycle coverage.

The latest main branch moved composer tag helpers but left one stale import in the scheduled-task dialog; this PR includes the one-line import correction required for the rebased branch to build.

Why it's needed

Dynamic workspace registration and persistent restoration can add secondary runtimes without restarting the daemon, but those runtimes could not previously be unloaded until process restart. Operators therefore could not immediately release sessions, ACP mounts, memory lanes, bridge children, or channel-worker routing, and deleting a persistent registration only affected the next start. Runtime drain completes that lifecycle while retaining project files, settings, transcripts, archives, and other workspace data.

Reviewer Test Plan

How to verify

  1. Start a daemon with a primary workspace, dynamically add and persist a secondary workspace, and verify capabilities advertise workspace_runtime_removal with removable: true only on the secondary runtime.
  2. Remove an idle secondary workspace and verify it disappears from capabilities and persistent registrations, does not return after daemon restart, and its files and session history remain on disk.
  3. Create sessions, prompts, ACP connections, memory tasks, or a channel worker in the secondary workspace; verify non-force removal returns 409 workspace_busy with accurate activity counts, then force removal terminates only the target workspace resources while the primary daemon and sessions remain usable.
  4. Verify a persistence write failure restores the runtime gates and leaves the workspace usable, while a post-commit cleanup failure still converges to logical removal.
  5. Re-add the same path after removal and verify a fresh bridge, ACP mount, dispatcher, registry, and memory lane are created.
  6. In Web Shell, verify an untrusted removable workspace still exposes its independent action menu, the first confirmation performs non-force removal, the busy dialog shows all activity counts, force is disabled for the current session workspace, and successful removal falls back to primary.
  7. Run the focused daemon, bridge, SDK, and Web Shell tests plus npm run build, npm run typecheck, npm run lint, and npm run bundle; all should pass.

Evidence (Before & After)

Before: dynamically added or restored workspaces had no runtime removal action and could only be unloaded by restarting the daemon; forgetting persistence left the active runtime mounted.

After: removable workspace rows expose a separate Remove action. Idle removal completes immediately; busy removal presents the frozen activity counts and an explicit Force Remove confirmation. Component coverage verifies the untrusted-workspace action, busy details, current-session force guard, concurrent-removal state, mismatch reconciliation, and primary fallback.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

macOS, Node.js v22.22.3, local HTTP daemon, sandbox disabled for daemon lifecycle tests.

Risk & Scope

  • Main risk or tradeoff: The main risk is concurrent teardown across session admission, ACP transports, bridge children, memory queues, channel workers, persistence, and daemon shutdown. The design keeps persistence as the commit point, memoizes cleanup, retains the cwd reservation until disposal converges, and has targeted rollback, force-removal, shutdown, and same-path re-add coverage.
  • Not validated / out of scope: Manual Windows/Linux verification, browser screenshots, live external-model integration, and the full sandbox-none integration suites were not run locally. The Qwen review plugin was invoked but did not return within 60 minutes; two open-ended manual audits and the repository quality gates completed successfully.
  • Breaking changes / migration notes: None. The capability feature, removable field, DELETE route, result types, and bridge shutdown option are additive; existing clients and the forget-only registration endpoint retain their behavior.

Linked Issues

Related to #6378. Builds on #6716 and #6635.

中文说明

本 PR 做了什么

本 PR 为可移除的 secondary workspace 增加同步运行时移除能力。生产 daemon 会发布 workspace_runtime_removal,capabilities 会标识可移除 runtime,DELETE /workspaces/:workspace 通过可选 force 模式执行两阶段 drain,删除所有已知的持久化注册别名,清理该 workspace 所有的 session、prompt、待启动 reservation、ACP 连接、memory task、sub-session launcher、bridge 与 channel worker,最后释放 workspace 路径以支持干净地重新添加。

实现保持 primary 与通过启动参数显式配置的 workspace 不可移除,保留现有 registration forget-only 接口,并让热移除与 daemon shutdown 复用同一个 memoized disposer。如果非 force 删除发现活动资源或持久化写入失败,session admission、ACP 与 worker 的所有 gate 都会完整回滚。此外还新增了类型化 SDK 操作、WebUI action、Web Shell 确认与 busy/force 交互、协议文档,以及针对并发和生命周期的定向测试。

最新 main 已将 composer tag helper 迁移到新位置,但 scheduled-task dialog 仍保留一个旧 import;本 PR 包含让 rebase 后分支恢复构建所必需的一行 import 修正。

为什么需要

动态 workspace 注册与持久化恢复已经可以在不重启 daemon 的情况下添加 secondary runtime,但这些 runtime 以前只能等进程重启才能卸载。因此操作者无法立即释放 session、ACP mount、memory lane、bridge 子进程或 channel-worker 路由,而删除持久化注册也只会影响下一次启动。Runtime drain 补齐了这一生命周期,同时保留项目文件、设置、transcript、归档及其他 workspace 数据。

Reviewer 测试计划

如何验证

  1. 使用 primary workspace 启动 daemon,动态添加并持久化一个 secondary workspace,确认 capabilities 发布 workspace_runtime_removal,并且只有 secondary runtime 的 removabletrue
  2. 移除空闲 secondary workspace,确认它从 capabilities 和持久化注册中消失,daemon 重启后不再恢复,同时磁盘上的文件与 session 历史保持不变。
  3. 在 secondary workspace 中创建 session、prompt、ACP 连接、memory task 或 channel worker;确认普通删除返回带准确 activity 数量的 409 workspace_busy,随后 force 删除只终止目标 workspace 资源,primary daemon 与 session 继续可用。
  4. 确认持久化写入失败会恢复 runtime gate 并保持 workspace 可用,而 commit 后的 cleanup 失败仍会收敛为逻辑删除。
  5. 删除后重新添加相同路径,确认创建全新的 bridge、ACP mount、dispatcher、registry 与 memory lane。
  6. 在 Web Shell 中确认 untrusted 且 removable 的 workspace 仍显示独立操作菜单;首次确认执行非 force 删除;busy 弹窗显示全部 activity 数量;当前 session 所属 workspace 禁用 force;成功后回退到 primary。
  7. 运行 daemon、bridge、SDK 与 Web Shell 定向测试,以及 npm run buildnpm run typechecknpm run lintnpm run bundle;所有命令均应通过。

证据(修改前与修改后)

修改前:动态添加或恢复的 workspace 没有运行时移除操作,只能通过重启 daemon 卸载;forget persistence 后活动 runtime 仍保持挂载。

修改后:可移除 workspace 行提供独立 Remove 操作。空闲删除立即完成;繁忙删除显示冻结的 activity 数量并提供明确的 Force Remove 二次确认。组件测试覆盖 untrusted workspace 操作、busy 详情、当前 session force 保护、并发删除状态、mismatch 收敛与 primary fallback。

测试平台

OS 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境(可选)

macOS,Node.js v22.22.3,本地 HTTP daemon,daemon 生命周期测试禁用 sandbox。

风险与范围

  • 主要风险或权衡:主要风险是 session admission、ACP transport、bridge 子进程、memory queue、channel worker、持久化与 daemon shutdown 之间的并发清理。设计将持久化作为 commit point,memoize cleanup,在 disposer 收敛前保留 cwd reservation,并通过定向测试覆盖回滚、force 删除、shutdown 与同路径重新添加。
  • 未验证 / 范围外:未进行 Windows/Linux 手工验证、浏览器截图、真实外部模型集成以及完整 sandbox-none 集成套件。本地已调用 Qwen review 插件,但 60 分钟内未返回;两轮开放式人工审计与仓库质量门禁均已成功完成。
  • 破坏性变更 / 迁移说明:无。capability feature、removable 字段、DELETE route、结果类型与 bridge shutdown option 均为增量变更;现有客户端与 forget-only registration 接口保持原有行为。

关联 Issue

关联 #6378。基于 #6716#6635

@doudouOUC

Copy link
Copy Markdown
Collaborator Author

Local verification report

Validated on macOS with Node.js v22.22.3 after rebasing onto 01d406f1c.

  • npm run build — passed.
  • npm run typecheck — passed.
  • npm run lint — passed.
  • npm run bundle — passed.
  • CLI focused Vitest (workspace-management, channel-worker-group, run-qwen-serve) — 224 passed.
  • ACP bridge shutdown-focused Vitest — 10 passed, 378 skipped by the test-name filter.
  • TypeScript SDK DaemonClient Vitest — 230 passed.
  • Web Shell sidebar + scheduled-task dialog Vitest — 96 passed.

The focused coverage includes capability/removable metadata, primary/static protection, busy and force removal, persistence alias deletion and rollback, post-commit cleanup convergence, add/remove serialization, channel-worker routing and pidfile convergence, bridge shutdown reason/memoization/re-entry, ACP/memory drain behavior, SDK DELETE serialization, untrusted Web Shell removal, busy activity rendering, force guard, mismatch reconciliation, and same-path runtime recreation.

Not run locally: the full sandbox-none CLI/interactive integration suites, manual Windows/Linux verification, live external-model integration, and browser screenshot E2E. The Qwen review plugin was started but produced no result within 60 minutes and was terminated; two open-ended manual audits completed with no remaining actionable findings.

@doudouOUC
doudouOUC marked this pull request as ready for review July 11, 2026 23:26
@doudouOUC
doudouOUC enabled auto-merge July 11, 2026 23:26
@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

Re-run triage after 20+ review feedback commits.

Template ✓ — all required sections present with thorough bilingual reviewer test plan.

Problem: real operational gap. Dynamically registered workspaces had no runtime removal path — only restart could unload them. This is a natural lifecycle completion for the dynamic registration feature shipped in earlier PRs. Not theoretical.

Direction: aligned. Workspace runtime removal is a necessary companion to dynamic workspace addition (#6716, #6635). The capability advertisement, optional removable field, and DELETE /workspaces/:workspace endpoint are all additive — no breaking changes to existing clients.

Size: ~2,496 production additions, ~396 deletions (2,892 total). ~3,192 test lines. ~75 docs lines. No core paths touched — all changes are in packages/cli/src/serve/, packages/acp-bridge/, packages/sdk-typescript/, packages/web-shell/, and packages/webui/.

Approach: the scope is driven by the number of subsystems participating in workspace teardown (sessions, prompts, ACP, memory, channel workers, bridge, persistence). Each subsystem needs its own drain/rollback/cleanup adapter. The scope feels proportional — not scope creep. The one-line import fix for the rebased branch is called out transparently in the PR body.

Moving on to code review. 🔍

中文说明

在 20 多条 review 反馈提交后重新进行 triage。

模板 ✓ — 所有必需部分齐全,包含详尽的双语 reviewer 测试计划。

问题:真实的运维缺口。动态注册的 workspace 没有运行时移除路径——只能重启 daemon 才能卸载。这是早期 PR 中已交付的动态注册功能的自然生命周期补全,非理论性问题。

方向:对齐。workspace 运行时移除是动态添加(#6716#6635)的必要配套。capability 发布、可选 removable 字段和 DELETE /workspaces/:workspace 端点均为增量变更——不破坏现有客户端。

规模:约 2,496 行生产代码新增,约 396 行删除(共 2,892 行)。约 3,192 行测试。约 75 行文档。未触及核心路径——所有变更位于 packages/cli/src/serve/packages/acp-bridge/packages/sdk-typescript/packages/web-shell/packages/webui/

方案:范围由参与 workspace 拆卸的子系统数量决定(session、prompt、ACP、memory、channel worker、bridge、persistence),每个子系统需要自己的 drain/rollback/cleanup 适配器。范围与目标成比例,非 scope creep。rebase 后分支所需的一行 import 修复已在 PR 正文中透明说明。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal: I would have added a removable flag to WorkspaceRuntime, a DELETE /workspaces/:workspace endpoint with two-phase drain (observe → persistence commit → cleanup), a registry state machine (activedrainingremoved), bridge shutdown with a reason parameter, per-workspace session admission drain, and a UI confirmation dialog.

Comparison with the diff: The PR's approach matches and exceeds this baseline. Key design choices are sound:

  • Serialization by canonical cwd (inFlight map with operation types) prevents concurrent add/remove/promote races.
  • Two-phase drain with rollback — registry beginDrain → controller beginDrain → ACP beginDrain → observe activity → rollback if busy, or commit persistence and converge cleanup. Rollback order reverses drain order (ACP → controller → registry).
  • Persistence as commit point — after the store write succeeds, cleanup is best-effort and never rolls back. WorkspaceRegistrationStoreCommittedError correctly treats committed-but-lock-release-failed as committed.
  • Memoized cleanup via WeakMap — prevents double-disposal when shutdown and removal race.
  • Bridge shutdown reason (daemon_shutdown vs workspace_removed) propagates to session close events.
  • Channel worker group adapter — clean drain/cancel/remove integration with existing reconcile loop.

No critical correctness issues found.

Reuse check: reuses existing WorkspaceRegistry drain/cancel/complete, TotalSessionAdmission drain, bridge shutdown(), and WorkspaceRegistrationStore.removeByIds(). No duplicate utilities.

Build & Test Results

✓ npm run build — passed
✓ npm run typecheck — passed
✓ npm run lint — passed
✓ workspace-management.test.ts — 53 passed
✓ workspace-registry.test.ts — 13 passed
✓ workspace-registration-store.test.ts — 15 passed, 1 skipped
✓ run-qwen-serve.test.ts — 178 passed
✓ channel-worker-group.test.ts — 34 passed
✓ channel-worker-manager.test.ts — 27 passed
✓ workspace-remember.test.ts — 27 passed
✓ bridge.test.ts — 392 passed
✓ DaemonClient.test.ts — 236 passed
✓ WebShellSidebar.workspace-removal.test.tsx — 4 passed
Total: 979 tests passed

Real-Scenario Testing

Started daemon from PR bundle, exercised removal flow end-to-end:

qwen serve listening on http://127.0.0.1:18932 (mode=http-bridge)
qwen serve: startup timing: processToListenMs=424 runQwenServeToListenMs=309
2026-07-13T15:38:24.283Z [INFO] [DAEMON] daemon workspace roots initialized
qwen serve: session reaper started (interval 60000ms, idle threshold 1800000ms)
qwen serve: /acp WebSocket transport enabled on /acp
2026-07-13T15:38:35.905Z [INFO] [DAEMON] route=GET /capabilities status=200
2026-07-13T15:38:48.908Z [INFO] [DAEMON] route=POST /workspaces status=201
2026-07-13T15:38:48.931Z [INFO] [DAEMON] route=DELETE /workspaces/3845a120f7f08b71 status=200
  • GET /capabilities advertises workspace_runtime_removal
  • POST /workspaces → 201 (secondary added) ✓
  • DELETE /workspaces/:id on idle secondary → 200 with activity snapshot (all zeros) ✓

Removal response: {"removed":true,"workspaceId":"3845a120f7f08b71","forced":false,"persistedRegistrationRemoved":false,"activity":{"pendingSessionStarts":0,"sessions":0,"activePrompts":0,"acpConnections":0,"memoryTasks":0,"channelWorkers":0}}

中文说明

代码审查

PR 方案匹配并超越了独立基线。关键设计合理:按 cwd 序列化、两阶段 drain 带回滚、持久化作为提交点、WeakMap memoize 防止双重清理、bridge shutdown reason 传播。未发现关键正确性问题。

构建与测试

全部通过:build、typecheck、lint 以及 979 个单元测试。

真实场景测试

从 PR bundle 启动 daemon,端到端验证:capabilities 发布 workspace_runtime_removal;POST 添加 secondary → 201;DELETE 空闲 secondary → 200。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

Re-run after the author addressed 20+ rounds of review feedback. Everything checks out.

The PR fills a real operational gap — dynamic workspace addition without runtime removal leaves operators stuck restarting the daemon. The implementation handles the hard parts correctly: concurrent mutation serialization, two-phase drain with rollback, persistence-as-commit-point, memoized cleanup, and clean subsystem-specific drain adapters.

Scope is large (~2,900 production lines) but proportional — each subsystem owning workspace-scoped state needs drain/cleanup integration. Test coverage (979 tests, 13 files, all passing) matches the complexity. Real-scenario testing confirms the end-to-end flow works.

Previous CHANGES_REQUESTED reviews have been addressed. No unresolved concerns.

Approval guardrail: isCrossRepository=true, title is feat(serve): → guard is ok. No core paths touched → no Stage 0 escalation. Approving. ✅

中文说明

在作者处理了 20 多轮反馈后重新 triage。一切通过。

PR 填补了真实运维缺口。实现正确处理了并发变更序列化、两阶段 drain、持久化提交点、memoize 清理和子系统级适配器。规模较大但与目标成比例。979 个测试全部通过。真实场景测试确认端到端流程正常。之前的 CHANGES_REQUESTED 已解决。批准 ✅

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

⏸️ Deferring to maintainer for final review.

The triage pipeline completed all three stages with no blocking findings — code review clean, 907 tests pass, build/typecheck/lint all green, direction aligned with the workspace lifecycle roadmap. The implementation is sound and the recommendation is to approve.

However, the PR touches ~2,300 production lines across 5 packages in serve infrastructure, which the Stage 0 gate flags for maintainer awareness. A human should verify the architectural choices around the two-phase drain, registry state machine (active → draining → removed), and the cross-package contract before merge.

@doudouOUC — thanks for the thorough PR and design doc. This is queued for maintainer review.

中文说明

⏸️ 转交 maintainer 进行最终审查。

Triage 管线三个阶段全部完成,未发现阻塞问题——代码审查无问题,907 个测试通过,build/typecheck/lint 全部通过,方向与 workspace 生命周期路线图一致。实现合理,建议批准。

但 PR 涉及跨 5 个包的 serve 基础设施约 2,300 生产行,Stage 0 门禁标记需 maintainer 关注。建议人工验证两阶段 drain、registry 状态机(active → draining → removed)和跨包契约的架构选择。

Qwen Code · qwen3.7-max

Comment thread packages/cli/src/serve/channel-worker-group.ts Outdated
Comment thread packages/cli/src/serve/routes/workspace-management.ts
Comment thread packages/cli/src/serve/total-session-admission.ts Outdated
Comment thread packages/web-shell/client/components/sidebar/WebShellSidebar.tsx Outdated
Comment thread packages/cli/src/serve/run-qwen-serve.test.ts
Comment thread packages/cli/src/serve/routes/workspace-management.ts
Comment thread packages/web-shell/client/components/sidebar/WebShellSidebar.tsx
Comment thread packages/cli/src/serve/run-qwen-serve.ts
Comment thread packages/web-shell/client/components/sidebar/WorkspaceSection.tsx Outdated
Comment thread packages/web-shell/client/components/sidebar/WebShellSidebar.test.tsx Outdated
Comment thread packages/cli/src/serve/routes/workspace-management.ts
Comment thread packages/cli/src/serve/routes/workspace-management.ts Outdated
Comment thread packages/webui/src/daemon/workspace/actions.ts

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — no blockers. Suggestions are inline.

Comment thread packages/cli/src/serve/channel-worker-group.ts
Comment thread packages/cli/src/serve/channel-worker-group.ts
Comment thread packages/web-shell/client/components/sidebar/WebShellSidebar.tsx Outdated
Comment thread packages/cli/src/serve/routes/workspace-management.ts Outdated
Comment thread packages/web-shell/client/components/sidebar/WebShellSidebar.test.tsx Outdated
Comment thread packages/cli/src/serve/acp-http/index.ts
Comment thread packages/web-shell/client/components/sidebar/WorkspaceSection.tsx Outdated
@wenshao

wenshao commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /resolve

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

Qwen Code resolved the merge conflicts and pushed the branch update.

Merge conflict resolution summary — PR #6745

Conflicted files (3)

1. packages/cli/src/serve/run-qwen-serve.ts

What conflicted: The close-handler shutdown sequence. The PR branch added workspace-management sealing logic (initiallyMountedApp, workspaceManagementHandle, sealAndWait, initialManagementWait). Main added three shutdown stoppers (stopScheduledTaskKeepalive, stopWorkspaceGitState, subSessionStoppers).

Resolution: Kept both sides. The PR's initiallyMountedApp = runtimeApp ?? runtimeAppForCleanup is the correct app reference (the actual runtime app, not the delegating wrapper), so the main-side stoppers were adapted to use initiallyMountedApp?.locals with optional chaining instead of app.locals. This is more correct than main's original app reference, since the locals are set on the runtime app inside buildRuntime().

2. packages/sdk-typescript/src/daemon/index.ts

What conflicted: Independent type exports added after DaemonWorkspaceCapability. PR added DaemonWorkspaceRemovalActivity and DaemonWorkspaceRemovalResult; main added DaemonWorkspaceGitStatus.

Resolution: Kept all three types in alphabetical order.

3. packages/sdk-typescript/src/index.ts

What conflicted: Same pattern as #2 — independent type re-exports from ./types.js. PR added type DaemonWorkspaceRemovalActivity and type DaemonWorkspaceRemovalResult; main added type DaemonWorkspaceGitStatus.

Resolution: Kept all three types in alphabetical order.

Verification

  • No remaining conflict markers in any file.
  • All three new types (DaemonWorkspaceGitStatus, DaemonWorkspaceRemovalActivity, DaemonWorkspaceRemovalResult) are confirmed to exist in packages/sdk-typescript/src/daemon/types.ts.
  • The merge commit only touches the three conflicted files — no out-of-scope edits.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — no blockers. Suggestions are inline.

Comment thread packages/cli/src/serve/total-session-admission.ts
Comment thread packages/cli/src/serve/workspace-route-runtime.ts
Comment thread packages/cli/src/serve/channel-worker-group.ts Outdated
Comment thread packages/cli/src/serve/routes/capabilities.ts
Comment thread packages/cli/src/serve/routes/workspace-management.ts Outdated
Comment thread packages/cli/src/serve/workspace-remember.ts
Comment thread packages/web-shell/client/components/sidebar/WebShellSidebar.tsx Outdated
Comment thread packages/cli/src/serve/run-qwen-serve.test.ts Outdated
Comment thread packages/cli/src/serve/run-qwen-serve.ts
Comment thread packages/cli/src/serve/acp-http/index.ts
Comment thread packages/cli/src/serve/workspace-registry.ts
Comment thread packages/cli/src/serve/workspace-remember.ts Outdated
Comment thread packages/cli/src/serve/channel-worker-group.ts
Comment thread packages/cli/src/serve/workspace-registry.ts Outdated
@wenshao

wenshao commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /resolve

@doudouOUC
doudouOUC force-pushed the feat/workspace-runtime-removal branch from a0b518d to 8c6489a Compare July 12, 2026 11:24
@github-actions

Copy link
Copy Markdown
Contributor

Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration.

中文

请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

Qwen Code resolved the merge conflicts, but the head branch changed while resolving, so the update was not pushed. Re-run /resolve. The resolved diff is attached as the qwen-resolve-pr-6745 artifact on the workflow run.

Merge Conflict Resolution Summary

PR: #6745 — feat(serve): support runtime workspace removal
Commit: chore: merge origin/main into feat/workspace-runtime-removal

Conflicted File

packages/cli/src/serve/server/error-response.ts

Conflict

Both branches added new error handlers at the top of sendBridgeError(), in the same location:

  • HEAD (PR branch): Added a WorkspaceDrainingError handler — returns 503 with code: 'workspace_draining' and workspaceCwd. This is part of the PR's runtime workspace removal feature.
  • origin/main: Added three transcript-related error handlers:
    • InvalidSessionTranscriptCursorError → 400
    • SessionTranscriptSnapshotUnavailableError → 409
    • SessionTranscriptTooLargeError → 413

Resolution

Kept both sides — all four error handlers are independent and non-overlapping. The PR's WorkspaceDrainingError handler is placed first (matching its position in the PR branch), followed by the three transcript error handlers from main. The imports for all error classes were already auto-merged successfully by git.

doudouOUC added a commit to doudouOUC/qwen-code that referenced this pull request Jul 12, 2026
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC doudouOUC self-assigned this Jul 12, 2026

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

One new Critical finding plus four Suggestions. Build and typecheck pass cleanly. Full test coverage verified (55 behaviors).

Critical (1)

  • workspace-remember.ts:261failRunningTaskAfterRemoval overwrites completed task state (data loss)

Suggestions (4)

  • workspace-registration-store.ts:393 — Error handling masks root cause on dual failure
  • WebShellSidebar.tsx:1490 — Removal polling dialog cannot be dismissed
  • WorkspaceSection.tsx:119 — Focus not restored on pointer-outside menu close
  • run-qwen-serve.test.ts:5252 — Test assertion does not verify removeByIds arguments

Comment thread packages/cli/src/serve/workspace-remember.ts Outdated
Comment thread packages/cli/src/serve/workspace-registration-store.ts
Comment thread packages/web-shell/client/components/sidebar/WebShellSidebar.tsx Outdated
Comment thread packages/web-shell/client/components/sidebar/WorkspaceSection.tsx Outdated
Comment thread packages/cli/src/serve/run-qwen-serve.test.ts
doudouOUC and others added 14 commits July 13, 2026 20:06
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC
doudouOUC force-pushed the feat/workspace-runtime-removal branch from 6802b5a to d509ff7 Compare July 13, 2026 12:11
@doudouOUC
doudouOUC requested a review from wenshao July 13, 2026 12:31

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — no blockers. Suggestions are inline.

— qwen3.7-max via Qwen Code /review

Comment thread packages/cli/src/serve/routes/workspace-management.ts Outdated
@wenshao

wenshao commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

✅ Local verification report — round 3 (maintainer merge reference)

Re-verification of the current head d509ff713. My round-2 report verified head 2da7fcefd; since then the branch was rebased onto newer main — which landed the #6804 Web Shell sidebar modernization (deleting the old 3,238-line WebShellSidebar.test.tsx) — and this PR re-integrated its removal UI on top of the reworked sidebar (WebShellSidebar.tsx, WorkspaceSection.tsx, i18n.tsx, sidebar CSS), swapped the deleted giant suite for a focused WebShellSidebar.workspace-removal.test.tsx, and picked up backend touch-ups in routes/workspace-management.ts. The tip commit is test(web-shell): cover workspace removal after sidebar rebase. Because the frontend layer materially changed, I rebuilt from source in a clean isolated worktree and re-ran everything: the repository quality gates, every changed test file, a fresh live end-to-end run against a real qwen serve daemon driving the actual DELETE /workspaces/:workspace route, and — new this round — browser screenshots of the real removal dialogs. Everything passes.

Environment: macOS (Darwin 24.6.0) · Node.js v22.23.1 · built from source (npm cibuild / bundle) in an isolated worktree · daemon pointed at a fake OpenAI endpoint so no external model is contacted.

1. Repository quality gates

Gate Result Notes
npm run build ✅ pass all workspace packages compiled (tsc --build)
npm run typecheck ✅ pass cli · core · sdk · web-shell · webui (tsc --noEmit)
npm run lint ✅ pass eslint . + integration-tests · 0 problems
npm run bundle ✅ pass esbuild bundle + all assets copied to dist/

2. Focused Vitest — every changed test file in the PR

Package Suite(s) Result
cli serve/* — workspace-management, run-qwen-serve, channel-worker-group, channel-worker-manager, workspace-qualified-acp, workspace-registration-store, workspace-registry, workspace-remember, total-session-admission, workspace-git-state, routes/workspace-git, server (12 files) ✅ 1089 passed · 1 skipped
acp-bridge bridge.test.ts (shutdown reason + memoization) ✅ 392 passed
sdk-typescript daemon/DaemonClient.test.ts ✅ 236 passed
web-shell sidebar/WebShellSidebar.workspace-removal.test.tsx ✅ 4 passed
webui DaemonSessionProvider · workspace/actions · session/actions ✅ 175 passed

Total focused: 1896 passed · 1 skipped · 0 failed.

Note on the web-shell count vs. round 2 (95 → 4): main's #6804 deleted the monolithic WebShellSidebar.test.tsx; this PR now carries only the focused removal suite. Those 4 tests still cover the behaviors that matter here — feature-gating, the untrusted-but-removable action, primary fallback after removal, and the current-session force guard.

quality gates + focused tests

3. Live end-to-end — real daemon, real DELETE /workspaces/:workspace

I booted the bundled dist/index.js serve with a primary + a startup-configured workspace and drove the real HTTP surface with fetch, asserting every response (26 assertions). This exercises the full lifecycle the PR describes — capability advertisement, the two-phase drain, the busy 409 / force path, the persistence commit-point, and same-path re-add:

  • A — capability & flags: GET /capabilities advertises workspace_runtime_removal; primary and the startup-configured workspace report removable:false.
  • B — dynamic add + persist: POST /workspaces {persist:true}201, the secondary now shows removable:true, and the persistent registration is written to disk.
  • C — guards: DELETE on primary → 409 primary_workspace_removal_forbidden; on the startup workspace → 409 static_workspace_removal_forbidden.
  • D — busy → force: with a live ACP connection open, DELETE (no force) → 409 workspace_busy {acpConnections:1}; DELETE {force:true}200 removed:true forced:true and the persistent registration is removed.
  • E — clean release: the secondary is gone from capabilities and erased from disk, while the primary daemon keeps serving 200.
  • F — re-add + idle removal: the same path re-adds as a fresh removable runtime; an idle DELETE returns 200 removed:true forced:false with all activity counts at zero. SIGTERM drains and exits 0.

Result: 26 passed · 0 failed, stable across 3 / 3 consecutive runs — no flakiness in the concurrent-teardown path the PR flags as its main risk.

live daemon E2E — DELETE /workspaces/:workspace

4. Web Shell removal UX — real component screenshots (new this round)

Because the sidebar was reworked this round, I captured the actual removal dialogs from the real WebShellSidebar component (driven via Playwright against a mock daemon), rather than relying on component tests alone. The first confirmation performs a non-force removal and reassures that files/settings/history are retained; a 409 workspace_busy renders the frozen activity snapshot (all six counters) and the explicit Force remove action. Rendered correctly in both light and dark themes.

Web Shell removal UX

Scope not re-covered locally

Same out-of-scope set the author noted: manual Windows/Linux runs, live external-model integration, and the full sandbox-none integration suites. (The Web Shell dialogs are now visually verified above; the current-session force guard remains covered by the focused component test.)

Recommendation

Verification is clean and reproducible on the current head d509ff713. Build / test / runtime behavior and the Web Shell removal UX all check out; from a technical standpoint this remains a solid merge candidate. The remaining gate is process — a formal non-author approval and the final main-integration /resolve.

中文说明

✅ 本地验证报告 — 第 3 轮(维护者合并参考)

针对当前 head d509ff713 的复验。我的第 2 轮报告验证的是 head 2da7fcefd;此后分支被 rebase 到更新的 main——其中合入了 #6804 的 Web Shell sidebar 现代化改造(删除了旧的 3,238 行 WebShellSidebar.test.tsx)——本 PR 随后在重构后的 sidebar 之上重新集成了移除 UIWebShellSidebar.tsxWorkspaceSection.tsxi18n.tsx、sidebar CSS),用一个聚焦的 WebShellSidebar.workspace-removal.test.tsx 替换了被删除的大测试文件,并吸收了 routes/workspace-management.ts 的后端改动。tip 提交为 test(web-shell): cover workspace removal after sidebar rebase。由于前端层有实质变化,我在干净的隔离 worktree 中从源码重新构建并重跑了全部内容:仓库质量门禁、每一个被改动的测试文件、一次全新的针对真实 qwen serve daemon、驱动真实 DELETE /workspaces/:workspace 路由的端到端运行,以及——本轮新增——真实移除弹窗的浏览器截图。全部通过。

环境: macOS (Darwin 24.6.0) · Node.js v22.23.1 · 隔离 worktree 中从源码构建(npm cibuild / bundle)· daemon 指向 fake OpenAI 端点,因此不联系任何外部模型

1. 仓库质量门禁

门禁 结果 说明
npm run build ✅ 通过 所有 workspace 包编译成功(tsc --build
npm run typecheck ✅ 通过 cli · core · sdk · web-shell · webui(tsc --noEmit
npm run lint ✅ 通过 eslint . + integration-tests · 0 问题
npm run bundle ✅ 通过 esbuild 打包 + 全部资源复制到 dist/

2. 定向 Vitest — PR 中每一个改动的测试文件

测试套件 结果
cli serve/* — workspace-management、run-qwen-serve、channel-worker-group、channel-worker-manager、workspace-qualified-acp、workspace-registration-store、workspace-registry、workspace-remember、total-session-admission、workspace-git-state、routes/workspace-git、server(12 个文件 ✅ 1089 通过 · 1 跳过
acp-bridge bridge.test.ts(shutdown reason + memoization) ✅ 392 通过
sdk-typescript daemon/DaemonClient.test.ts ✅ 236 通过
web-shell sidebar/WebShellSidebar.workspace-removal.test.tsx ✅ 4 通过
webui DaemonSessionProvider · workspace/actions · session/actions ✅ 175 通过

定向测试合计:1896 通过 · 1 跳过 · 0 失败。

关于 web-shell 数量相较第 2 轮的变化(95 → 4):main 的 #6804 删除了原来的巨型 WebShellSidebar.test.tsx;本 PR 现在只保留聚焦的移除测试。这 4 个测试仍覆盖了此处关键的行为——feature 门控、untrusted 但可移除的操作、移除后回退到 primary,以及当前 session 所属 workspace 的 force 保护。

3. 端到端 — 真实 daemon、真实 DELETE /workspaces/:workspace

我用一个 primary + 一个启动参数配置的 workspace 启动了打包后的 dist/index.js serve,用 fetch 驱动真实 HTTP 接口并对每个响应做断言(共 26 条断言),覆盖 PR 描述的完整生命周期——capability 发布、两阶段 drain、busy 409 / force 路径、持久化 commit-point,以及同路径重新添加:

  • A — capability 与标志位: GET /capabilities 发布 workspace_runtime_removal;primary 与启动配置的 workspace 均为 removable:false
  • B — 动态添加 + 持久化: POST /workspaces {persist:true}201,secondary 显示 removable:true,并将持久化注册写入磁盘。
  • C — 保护: 对 primary 执行 DELETE409 primary_workspace_removal_forbidden;对启动 workspace → 409 static_workspace_removal_forbidden
  • D — busy → force: 在保持一个真实 ACP 连接时,DELETE(非 force)→ 409 workspace_busy {acpConnections:1}DELETE {force:true}200 removed:true forced:true,同时移除持久化注册。
  • E — 干净释放: secondary 从 capabilities 消失并从磁盘擦除,而 primary daemon 继续返回 200
  • F — 重新添加 + 空闲删除: 相同路径作为全新 removable runtime 重新添加;空闲 DELETE 返回 200 removed:true forced:false,所有 activity 计数为零。SIGTERM drain 后退出码 0

结果:26 通过 · 0 失败,连续 3 / 3 次运行稳定——PR 标注的主要风险(并发清理)路径未见抖动。

4. Web Shell 移除交互 — 真实组件截图(本轮新增)

由于 sidebar 本轮被重构,我从真实的 WebShellSidebar 组件(通过 Playwright 驱动 mock daemon)捕获了实际的移除弹窗,而非仅依赖组件测试。首次确认执行非 force 移除,并提示文件/设置/历史会被保留;409 workspace_busy 会渲染被冻结的 activity 快照(全部 6 个计数)以及明确的 Force remove 操作。在浅色与深色主题下均渲染正确。

本地未重复覆盖的范围

与作者注明的范围外一致:手工 Windows/Linux 运行、真实外部模型集成,以及完整的 sandbox-none 集成套件。(Web Shell 弹窗已在上方做了可视化验证;当前 session 的 force 保护仍由聚焦组件测试覆盖。)

建议

验证在当前 head d509ff713干净且可复现。构建 / 测试 / 运行时行为以及 Web Shell 移除交互均通过;从技术角度看,这仍是一个可靠的合并候选。剩余的门禁属于流程层面——一次正式的非作者审批,以及最终的 main 集成 /resolve

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC
doudouOUC requested a review from wenshao July 13, 2026 14:13
@wenshao

wenshao commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — no blockers. Suggestions are inline.

}
registryDraining = false;
controllerDraining = false;
acpDraining = false;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The 'forget' operation type falls into the "registration in progress" branch of this ternary, producing the misleading message "Workspace registration is in progress" when another forget is in flight for the same cwd. Consider adding a 'forget' branch or using a generic message:

Suggested change
acpDraining = false;
error:
operation === 'removal'
? 'Workspace removal is in progress'
: operation === 'forget'
? 'Workspace forget is in progress'
: 'Workspace registration is in progress',

— qwen3.7-max via Qwen Code /review

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@doudouOUC
doudouOUC added this pull request to the merge queue Jul 13, 2026
Merged via the queue into QwenLM:main with commit 13c224f Jul 13, 2026
72 of 73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants