feat(serve): add workspace runtime coordination - #7288
Closed
ytahdn wants to merge 2 commits into
Closed
Conversation
ytahdn
marked this pull request as ready for review
July 20, 2026 06:49
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: This is an architectural feature, not a bug fix — the problem is real and well-established. The CHANGELOG shows active serve/daemon development (workspace session-info endpoint #7077, workspace telemetry #7003, skill management pages #7018, etc.), and the gap is clear: management pages need authoritative live state even when no chat Session exists. Session-owned ACP lifecycle causes MCP auth, extension activation, and Skill discovery to vanish when the last Session detaches. Direction: Aligned. Workspace-scoped runtime ownership is the natural next step for the daemon architecture. The CHANGELOG references above confirm this is an active area of investment. One note: this PR touches trust boundaries (workspace-scoped trust checks, filesystem access isolation) — flagging for maintainer awareness on that front. Size: Cross-package change (acp-bridge + cli). Production logic: ~6,240 lines (additions + deletions). Test code: ~5,749 lines. Docs: ~1,119 lines. This is well above the 500-line maintainer-awareness threshold and the 1,000-line large-PR advisory. @wenshao @yiliang114 — flagging this for your awareness given the scale and the trust-boundary surface area. Approach: The architecture is sound — moving runtime ownership from Session to a workspace-scoped coordinator is the right structural call. The design doc (905 lines) is thorough. That said, the scope is very broad for a single PR: coordinator core, MCP operations, workspace-qualified routes for extensions/skills/settings/MCP-config, plus the acp-bridge status module and significant bridge.ts rework. If any of the route adaptations (extensions, skills, settings) could land as follow-ups against the coordinator core, that would make this easier to review and revert. Not a blocker — just a genuine question about whether splitting was considered. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题： 这是一个架构级特性，不是 bug 修复——问题是真实且明确的。CHANGELOG 显示 serve/daemon 方向持续投入（workspace session-info 端点 #7077、workspace 遥测 #7003、技能管理页面 #7018 等），缺口清晰：管理页面在没有聊天 Session 时也需要权威的实时状态。Session 持有 ACP 生命周期导致最后一个 Session 断开后 MCP 认证、扩展启用和 Skill 发现消失。 方向： 对齐。工作区级运行时所有权是 daemon 架构的自然下一步。上述 CHANGELOG 引用确认这是活跃投资方向。注意：本 PR 触及信任边界（工作区限定信任检查、文件系统访问隔离）——提请维护者关注。 规模： 跨包改动（acp-bridge + cli）。生产逻辑：约 6,240 行（增+删）。测试代码：约 5,749 行。文档：约 1,119 行。远超 500 行维护者关注阈值和 1,000 行大 PR 建议线。@wenshao @yiliang114——鉴于规模和信任边界面，提请关注。 方案： 架构合理——将运行时所有权从 Session 迁移到工作区级协调器是正确的结构性决策。设计文档（905 行）详尽。但单个 PR 范围非常广：协调器核心、MCP 操作、扩展/技能/设置/MCP 配置的工作区限定路由，加上 acp-bridge status 模块和 bridge.ts 的大幅重构。如果部分路由适配（扩展、技能、设置）可以作为后续 PR 基于协调器核心落地，会更容易审查和回滚。不是阻塞——只是真诚地询问是否考虑过拆分。 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
ytahdn
requested review from
doudouOUC,
qwen-code-ci-bot and
wenshao
and removed request for
qwen-code-ci-bot and
wenshao
July 20, 2026 06:56
Collaborator
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Collaborator
Code ReviewIndependent proposal: Given the problem (management pages need sessionless live state), I would have built a workspace-scoped coordinator that owns the ACP bridge lifecycle, exposes capability status without a Session, and adds workspace-qualified HTTP routes. The coordinator would track capability readiness (extensions, MCP, skills, tools) with epoch-based staleness detection, and drain cleanly on workspace removal. Comparison with the diff: The PR's approach matches this closely. The Findings: No critical blockers found. The code is well-written with proper error handling, epoch validation, and drain semantics. A few observations:
sequenceDiagram
participant C as Client
participant R as Runtime Routes
participant Coord as Coordinator
participant Bridge as ACP Bridge
participant ACP as ACP Child
C->>R: POST /workspace/runtime/ensure
R->>Coord: ensure(timeoutMs)
Coord->>Coord: assertAcceptingWork()
Coord->>Bridge: isChannelLive()?
alt not live
Coord->>Bridge: preheatAcpChild()
Bridge->>ACP: spawn channel
ACP-->>Bridge: channel ready
end
Coord->>Bridge: refreshWorkspaceExtensions()
Bridge->>ACP: extMethod call
ACP-->>Bridge: extensions status
Coord->>Bridge: getWorkspaceMcpStatus()
Bridge->>ACP: extMethod call
ACP-->>Bridge: MCP status (discovery completed)
Coord->>Bridge: getWorkspaceSkillsStatus()
Bridge->>ACP: extMethod call
ACP-->>Bridge: skills status
Coord->>Bridge: getWorkspaceToolsStatus()
Bridge->>ACP: extMethod call
ACP-->>Bridge: tools status
Coord-->>R: ServeWorkspaceRuntimeStatus (all ready)
R-->>C: 200 JSON
Files changed (30 of 53 shown)
Real-Scenario TestingStarted GET /workspace/runtime/status (cold start, no Session): {
"v": 1,
"workspaceCwd": "/home/github-runner/actions-runner-3/_work/qwen-code/qwen-code",
"state": "cold",
"runtimeLive": false,
"runtimeEpoch": 0,
"capabilities": {
"extensions": { "state": "not_started" },
"mcp": { "state": "not_started" },
"skills": { "state": "not_started" },
"tools": { "state": "not_started" }
}
}POST /workspace/runtime/ensure (starts ACP child, prepares all capabilities): {
"v": 1,
"workspaceCwd": "/home/github-runner/actions-runner-3/_work/qwen-code/qwen-code",
"state": "idle",
"runtimeLive": true,
"runtimeEpoch": 1,
"capabilities": {
"extensions": { "state": "ready", "desiredGeneration": 0, "appliedGeneration": 0, "runtimeEpoch": 1, "appliedEpoch": 1 },
"mcp": { "state": "ready", "runtimeEpoch": 1 },
"skills": { "state": "ready", "runtimeEpoch": 1 },
"tools": { "state": "ready", "runtimeEpoch": 1 }
}
}GET /workspace/runtime/mcp (live MCP status without a Session): {
"v": 1,
"workspaceCwd": "...",
"initialized": true,
"discoveryState": "completed",
"servers": [],
"clientCount": 0,
"budgetMode": "off",
"budgets": [],
"runtimeEpoch": 1,
"source": "live"
}GET /workspace/runtime/skills — returned 30+ skills with live data. GET /workspace/runtime/tools — returned full tool registry with live data. GET /workspace/runtime/extensions — returned initialized extensions status. Validation checks:
Daemon log confirms all requests handled: Unit tests: 506 tests pass across the three key files (coordinator: 64, runtime routes: 12, bridge: 430). 中文说明代码审查独立方案： 给定问题（管理页面需要无 Session 的实时状态），我会构建一个工作区级协调器来持有 ACP bridge 生命周期，在没有 Session 时暴露能力状态，并添加工作区限定的 HTTP 路由。协调器会跟踪能力就绪状态（扩展、MCP、技能、工具），使用 epoch 过期检测，并在工作区移除时安全排空。 与 diff 对比： PR 的方案与此高度一致。 发现： 未发现关键阻塞问题。代码编写良好，具有正确的错误处理、epoch 验证和排空语义。几点观察：
真实场景测试从 PR 分支启动
— Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 3/5 — clean review across every stage, but the Stage 0 maintainer-awareness escalation (6,240 production lines, cross-package, trust-boundary surface) needs a human sign-off. This is a well-executed architectural change. The coordinator pattern is the right call — my independent proposal arrived at the same structure, and the implementation matches or exceeds it. The code is thoroughly tested (506 unit tests across the key files), the real-scenario testing confirms the endpoints work end-to-end (cold → ensure → all capabilities ready), and the validation edge cases are handled correctly. What I'd want a maintainer to weigh in on:
None of these are blockers from the code review — they're "a human should second this" items. The code itself is clean, well-tested, and works as advertised. 中文说明置信度：3/5——每个阶段审查均干净，但 Stage 0 维护者关注升级（6,240 生产行、跨包、信任边界面）需要人工签核。 这是一个执行良好的架构变更。协调器模式是正确的选择——我的独立方案得出了相同的结构，实现与之一致甚至更优。代码测试充分（关键文件 506 个单测），真实场景测试确认端点端到端工作（冷启动 → ensure → 所有能力就绪），验证边界情况处理正确。 希望维护者关注的：
这些都不是代码审查的阻塞项——而是"需要人工二次确认"的事项。代码本身干净、测试充分、按预期工作。 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
⏸️ Deferring to @wenshao @yiliang114 — this PR is clean across all review stages (code, tests, real-scenario), but the Stage 0 escalation applies: 6,240 production logic lines across two packages (acp-bridge + cli), touching trust-boundary and channel-lifetime invariants. Needs a maintainer's architectural sign-off before merge. Specifically: (1) whether the scope should be split, (2) trust-boundary surface on workspace-qualified routes, (3) channel lifetime decoupling from session count. See the Stage 3 comment above for details. |
Collaborator
Author
|
Superseded by #7307, which reorganizes the workspace-runtime migration into four vertical commits covering the runtime foundation and the MCP, Extensions, and Skills flows end to end. Closing this horizontal-stack PR in favor of the consolidated replacement. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Moves qwen serve runtime ownership from individual Sessions to a workspace-scoped coordinator. Each registered workspace can keep one ACP-backed runtime alive, expose sessionless Extension, MCP, Skill, and built-in Tool state, serialize capability reconciliation, and drain cleanly when the workspace or daemon is removed. It also adds workspace-qualified runtime/configuration routes and updates the serve architecture and lifecycle documentation alongside the implementation.
Why it's needed
Management pages need authoritative live state even when no chat Session exists. Session-owned ACP lifecycle made MCP authentication, extension activation, and Skill discovery disappear or restart whenever the last Session detached. A Workspace Runtime gives those features a stable owner while Sessions remain lightweight consumers.
Reviewer Test Plan
How to verify
Start qwen serve without creating a Session, request a workspace runtime status, ensure the runtime, and confirm Extension, MCP, Skill, and Tool capabilities become ready. Exercise two workspaces and confirm routes, configuration, trust checks, ACP state, and filesystem access remain scoped to the selected workspace. Remove a workspace or stop the daemon and confirm the coordinator drains its runtime. Verify an unset channel idle timeout keeps the workspace runtime alive, a positive timeout opts into delayed compatibility cleanup, and zero is rejected.
Evidence (Before & After)
N/A — daemon architecture and APIs; no standalone UI in this PR.
Tested on
Environment (optional)
Node.js 22; local workspace build. End-to-end validation was intentionally not run.
Risk & Scope
--channel-idle-timeout-ms 0is rejected. Leaving the option unset keeps the workspace runtime resident; positive values enable delayed cleanup.Linked Issues
N/A
中文说明
本 PR 做了什么
将 qwen serve 的运行时所有权从单个 Session 迁移到工作区级协调器。每个已注册工作区可以常驻一个由 ACP 支撑的运行时，在没有 Session 时仍提供扩展、MCP、Skill 和内置工具的真实状态，并统一串行化能力同步，在工作区移除或 daemon 关闭时安全释放。同时新增工作区限定的运行时与配置接口，并让 serve 架构、生命周期文档和实现一起提交。
为什么需要
管理页面即使没有聊天 Session，也需要权威的实时状态。过去 ACP 生命周期依赖 Session，最后一个 Session 断开后，MCP 认证、扩展启用状态和 Skill 发现可能消失或反复重启。Workspace Runtime 为这些能力提供稳定所有者，而 Session 只作为轻量消费者。
Reviewer Test Plan
如何验证
启动 qwen serve 但不创建 Session，读取工作区运行时状态，确保运行时启动，并确认扩展、MCP、Skill 和 Tool 能力进入 ready。使用两个工作区验证接口、配置、信任检查、ACP 状态和文件系统访问始终限定在所选工作区。移除工作区或关闭 daemon，确认协调器会排空并释放运行时。确认未设置 channel idle timeout 时运行时保持常驻，正数开启延迟兼容回收，零会被拒绝。
证据（前后对比）
N/A——本 PR 是 daemon 架构与 API，不包含独立 UI。
测试平台
macOS 已完成 build、typecheck 和定向单测；Windows、Linux 未测试。
环境
Node.js 22，本地工作区构建。按当前约定未执行端到端验证。
风险与范围
--channel-idle-timeout-ms 0会被拒绝。不设置时工作区运行时常驻，设置正数时启用延迟回收。关联 Issue
N/A