feat(cli): Add Phase 2a workspace foundation - #6410
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Thanks for the PR! Template looks good ✓ — all required headings are present with detailed content. Problem: This PR implements Phase 2a of the multi-workspace daemon RFC (#6378), which has been through extensive maintainer review. The problem is real and well-documented — the current daemon assumes 1:1 workspace binding, and Phase 2a lays the internal foundations for future multi-workspace routing. This is planned roadmap work, not theoretical hardening. Direction: Aligned. Multi-workspace daemon support is tracked in the RFC with multiple related issues (#3803, #4514, #5976). Phase 2a is explicitly scoped to foundation contracts — parser, input validation, registry metadata, and test injection — without exposing any public surface changes. The scope discipline is good. Size: 349 production lines (serve.ts: 2, fast-path.ts: 6, run-qwen-serve.ts: 17, server.ts: 116, workspace-inputs.ts: 104, workspace-registry.ts: 100) + 529 test lines + 116 design doc lines. All changes are in Approach: The scope feels right for a foundation PR. The design doc clearly enumerates what's in and out of scope, and the implementation matches. The four changes (parser repeatability, input validation module, registry generalization, createServeApp injection) are each minimal and necessary for the stated contract. The Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ — 所有必需标题都有,内容详细。 问题: 这个 PR 实现 multi-workspace daemon RFC (#6378) 的 Phase 2a,已经过 maintainer 充分讨论。问题是真实的且有文档支持——当前 daemon 假设 1:1 workspace 绑定,Phase 2a 为未来 multi-workspace 路由铺设内部地基。这是路线图规划内的工作,不是理论性加固。 方向: 对齐。Multi-workspace daemon 支持在 RFC 中跟踪,关联多个 issue(#3803, #4514, #5976)。Phase 2a 明确限定在地基合约范围——parser、输入验证、registry 元数据、测试注入——不暴露任何公开表面变更。范围纪律很好。 规模: 349 行生产代码(serve.ts: 2, fast-path.ts: 6, run-qwen-serve.ts: 17, server.ts: 116, workspace-inputs.ts: 104, workspace-registry.ts: 100)+ 529 行测试 + 116 行设计文档。所有变更在 方案: 范围对地基 PR 来说合理。设计文档清楚列出了范围内和范围外的内容,实现也匹配。四个变更(parser 可重复、输入验证模块、registry 泛化、createServeApp 注入)都是最小且必要的。 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Code ReviewThe implementation matches what I'd propose independently for a Phase 2a foundation: make What's clean:
No blockers found. No correctness bugs, security issues, or regressions. No AGENTS.md violations — the code is minimal, focused, and follows project conventions (ESM, strict TS, collocated tests). TestingInternal infrastructure PR with no TUI surface change. Validation covers unit tests, build, typecheck, and lint. Unit Tests (tmux capture)Files tested: Build & Typecheck (tmux capture)All green — build, typecheck, and lint pass clean. 中文说明代码审查实现与我独立设想的 Phase 2a 地基方案一致:在 parser 层让 做得好的部分:
无阻塞问题。 无正确性 bug、安全问题或回归。无 AGENTS.md 违反。 测试内部地基 PR,无 TUI 表面变更。验证覆盖单元测试、构建、类型检查和 lint。 全部通过:878 个测试,7 个测试文件,构建/类型检查/lint 均 clean。 — Qwen Code · qwen3.7-max |
|
This is a clean foundation PR. The problem is real (RFC #6378, multi-phase daemon workspace support), the scope is disciplined (parser, input validation, registry metadata, test injection — nothing more), and the implementation matches the design doc's Phase 2a contract. The code does what a foundation PR should: establish internal contracts without exposing public surface changes. The The latest commits tightened guardrails and added coverage for registry injection paths — good responsiveness to review feedback. 878 tests pass, build/typecheck/lint all clean. The independent proposal I wrote before reading the diff matches this implementation closely — I don't see a materially simpler path that would cover the same contract. Approving. ✅ 中文说明这是一个干净的地基 PR。问题是真实的(RFC #6378,多阶段 daemon workspace 支持),范围有纪律(parser、输入验证、registry 元数据、测试注入——仅此而已),实现匹配设计文档的 Phase 2a 合约。 代码做了地基 PR 该做的事:建立内部合约而不暴露公开表面变更。 最新提交收紧了 guardrails 并增加了 registry 注入路径的覆盖——对 review 反馈的良好响应。 878 个测试通过,构建/类型检查/lint 均 clean。我在阅读 diff 之前写的独立方案与此实现高度一致——没有看到更简单的路径能覆盖同样的合约。 批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
There was a problem hiding this comment.
Pull request overview
This PR lays Phase 2a groundwork for future multi-workspace qwen serve sessions while keeping multi-workspace serve behavior gated. It makes --workspace repeatable at the CLI parsing boundary, ensures repeated values fall back to the full parser, rejects unsupported multi-workspace inputs before boot, and extends the internal workspace registry/runtime contract with stable metadata and lookup helpers.
Changes:
- Make
--workspacerepeatable in yargs, and ensure the fast-path parser falls back when--workspaceis repeated. - Introduce
resolveSingleWorkspaceInput()with explicit guardrails for duplicate/nested/multiple workspace inputs while multi-workspace serve is gated. - Extend
WorkspaceRegistry/WorkspaceRuntimeto support stable metadata, id/cwd lookups, omitted-cwd primary resolution, and live session owner resolution; allowcreateServeAppto be assembled from an injected registry (for tests/future internals).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/src/serve/workspace-registry.ts | Expands registry/runtime contract (metadata, lookups, live owner resolution) and introduces multi-runtime registry constructor. |
| packages/cli/src/serve/workspace-registry.test.ts | Adds coverage for new registry behaviors, validation, and owner resolution outcomes. |
| packages/cli/src/serve/workspace-inputs.ts | New helper and error types to normalize/reject unsupported multi-workspace inputs pre-boot. |
| packages/cli/src/serve/workspace-inputs.test.ts | Tests for duplicate/nested/multiple-workspace gating behavior and cwd fallback. |
| packages/cli/src/serve/server.ts | Adds optional workspace registry injection, threads runtime metadata into locals, and enforces anti “split-brain” dependency wiring. |
| packages/cli/src/serve/server.test.ts | Verifies new runtime metadata, trust threading, registry injection behavior, and conflict rejection. |
| packages/cli/src/serve/run-qwen-serve.ts | Routes workspace parsing through resolveSingleWorkspaceInput and threads primary trust metadata into createServeApp. |
| packages/cli/src/serve/fast-path.ts | Forces fallback to the full parser when --workspace is repeated. |
| packages/cli/src/serve/fast-path.test.ts | Adds a test asserting repeated --workspace triggers fallback. |
| packages/cli/src/commands/serve.ts | Marks --workspace as an array option so yargs preserves repeated values. |
| packages/cli/src/commands/serve.test.ts | Updates expectations for array-shaped workspace parsing and ensures repeated values are preserved in command mode. |
| .qwen/design/daemon-multi-workspace-phase2a-sessions.md | Documents the Phase 2a foundation boundary and route classification for later dispatch work. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } catch (err) { | ||
| if (err instanceof SessionNotFoundError) continue; | ||
| throw err; | ||
| } |
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
packages/cli/src/serve/server.ts:322-365 |
Conflict-check error messages name the conflicting pair but omit the actual values. Boot-time fatal errors should include both sides of the mismatch for faster diagnosis. | Include both values in each error message, e.g. registry (cwd=/work/a) conflicts with deps.boundWorkspace (/work/b) |
packages/cli/src/serve/workspace-inputs.ts:88-91 |
Reversed nested workspace check ([child, parent] input order) is untested. Only [parent, child] order is covered. |
Add a test: expect(() => resolveSingleWorkspaceInput([child, parent])).toThrow(NestedWorkspaceInputError) |
— qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
— GPT-5 via Qwen Code /review
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
@qwen-code /triage |
wenshao
left a comment
There was a problem hiding this comment.
Phase 2a workspace-foundation review. The core wiring is sound: the parser array:true migration is consistently coalesced (primaryWorkspaceArg in serve.ts, resolveSingleWorkspaceInput in run-qwen-serve.ts), the registry conflict-detection compares against the injected primary correctly, and resolveLiveSessionOwner matches the real getSessionSummary contract (it throws SessionNotFoundError, so the scan is fail-closed). Three follow-ups below — the first seeds a latent nesting hole for the later ungate.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
✅ Local verification — PR #6410 (Phase 2a workspace foundation)I built this PR from source and ran an end-to-end real-behavior verification on Linux. The PR's Tested on table lists only macOS (Windows/Linux "not tested"), so this adds the Linux surface. Everything passes and I found no regressions. Environment: Linux x64 · Node 22.x · built from source ( 1. Real compiled
|
| Input | Result |
|---|---|
--workspace <dir> (single) |
✅ boots |
no --workspace (primary = cwd) |
✅ boots |
--workspace A --workspace C (distinct) |
⛔ exit 1 — "Multiple --workspace values are not supported yet" |
--workspace A --workspace A (duplicate) |
⛔ exit 1 — "Duplicate --workspace value resolves to …" |
--workspace A --workspace A/b (nested) |
⛔ exit 1 — "Nested --workspace values are not supported yet: …" |
--workspace (valueless) |
⛔ exit 1 — "Not enough arguments following: workspace" |
2. Standalone harnesses against the compiled dist — 47 assertions
Imported the compiled modules directly and exercised the new contracts (no mocking of the code under test):
- Harness A —
resolveSingleWorkspaceInput(17): single & array-of-1 passthrough,undefined→cwd,[]→MissingWorkspaceInputError, duplicate→DuplicateWorkspaceInputError, nested (both orders)→NestedWorkspaceInputError, distinct→MultipleWorkspaceInputError, precedence (duplicate before nested), error.names + gate messages. - Harness B —
createWorkspaceRegistry(25): primary /list()/ frozen order,getByWorkspaceCwd&getByWorkspaceIdexact-match (trailing slash →undefined),resolveWorkspaceCwd(undefined)→primary, construction validation (≥1 runtime, exactly-one-primary, duplicate cwd, duplicate id), andresolveLiveSessionOwner→ found / not_found / ambiguous (fail closed) / re-throws non-SessionNotFoundError(no primary fallback, no error swallowing). - Harness C —
createServeAppregistry injection (5): every split-brain conflict (boundWorkspace/bridge/workspace/fsFactory/clientMcpSenderRegistry) throws a descriptive error naming both sides.
3. PR's own Reviewer Test Plan + full suite + gates
- Reviewer Test Plan:
242(parser / fast-path / inputs / run / registry — 5 files) +8(createServeAppfsFactory-wiring, including the new injection & conflict tests) +31(workspace-file-read) — all green. - Full
server.test.ts(superset, sinceserver.tschanged): 603 / 603. tsc --noEmit: clean ·eslint --max-warnings 0on all 6 changed files: clean.
4. RED/GREEN — the gate, not the harness, enforces the behavior
Disabled the reject gate (resolveSingleWorkspaceInput skipping rejectUnsupportedMultiWorkspaceInputs) and rebuilt: Harness A flipped to 8 passed / 9 failed — exactly the duplicate/nested/distinct assertions — and the real daemon then booted the gated multi-workspace inputs (silently taking the first) instead of rejecting. Restored → 17/17 green again.
Verdict
Functionally verified on Linux, end-to-end: the gate fails closed before boot with precise, class-specific errors; single/omitted inputs stay compatible; the WorkspaceRegistry and createServeApp injection contracts behave as documented (fail-closed ambiguity, split-brain rejection); no regressions in the daemon suite. Matches the CI ubuntu Test job, which is already green. LGTM from a verification standpoint. ✅
中文说明
✅ 本地验证 — PR #6410(Phase 2a workspace foundation)
我从源码构建了这个 PR,并在 Linux 上做了端到端的真实行为验证。PR 的 Tested on 表只标了 macOS(Windows/Linux 都是 "not tested"),所以这次补上了 Linux 的覆盖。全部通过,没有发现回归。
环境: Linux x64 · Node 22.x · 从源码构建(@qwen-code/qwen-code-core + @qwen-code/qwen-code,均为 0.19.6)· 分支 codex/phase2a-foundation @ 7d8238163。
1. 真实编译产物 qwen serve 启动矩阵(端到端)
用真实构建出的 daemon(node packages/cli/dist/index.js serve --port 0 …)跑了完整的 --workspace 输入矩阵。resolveSingleWorkspaceInput 在 run-qwen-serve.ts:1390 被调用 —— 早于 workspace 规范化(:1680)和监听(:2797)—— 因此被 gate 的输入会在绑定端口之前就 fail closed:
(见上方第一张截图)
| 输入 | 结果 |
|---|---|
--workspace <dir>(单个) |
✅ 正常启动 |
不带 --workspace(primary = cwd) |
✅ 正常启动 |
--workspace A --workspace C(不同目录) |
⛔ exit 1 —— "Multiple --workspace values are not supported yet" |
--workspace A --workspace A(重复) |
⛔ exit 1 —— "Duplicate --workspace value resolves to …" |
--workspace A --workspace A/b(嵌套) |
⛔ exit 1 —— "Nested --workspace values are not supported yet: …" |
--workspace(无值) |
⛔ exit 1 —— "Not enough arguments following: workspace" |
2. 针对编译产物的独立 harness —— 47 条断言
直接 import 编译后的 模块,驱动这些新契约(不 mock 被测代码):
(见上方第二张截图)
- Harness A —
resolveSingleWorkspaceInput(17): 单个 / 单元素数组 直通、undefined→cwd、[]→MissingWorkspaceInputError、重复→DuplicateWorkspaceInputError、嵌套(两种顺序)→NestedWorkspaceInputError、不同目录→MultipleWorkspaceInputError、优先级(重复先于嵌套)、错误.name及 gate 提示文案。 - Harness B —
createWorkspaceRegistry(25): primary /list()/ 冻结顺序、getByWorkspaceCwd与getByWorkspaceId精确匹配(结尾斜杠 →undefined)、resolveWorkspaceCwd(undefined)→primary、构造校验(至少一个 runtime、恰好一个 primary、cwd 重复、id 重复)、以及resolveLiveSessionOwner→ found / not_found / ambiguous(fail closed) / 对非SessionNotFoundError重新抛出(不回退到 primary、不吞异常)。 - Harness C —
createServeAppregistry 注入(5): 每一种 split-brain 冲突(boundWorkspace/bridge/workspace/fsFactory/clientMcpSenderRegistry)都会抛出同时指明两边的、带描述的错误。
3. PR 自带的 Reviewer Test Plan + 完整套件 + 门禁
- Reviewer Test Plan:
242(parser / fast-path / inputs / run / registry —— 5 个文件)+8(createServeAppfsFactory 接线,含新增的注入与冲突用例)+31(workspace-file-read),全绿。 - 完整
server.test.ts(因为server.ts有改动,这是超集):603 / 603。 tsc --noEmit:干净 · 对 6 个改动文件跑eslint --max-warnings 0:干净。
4. RED/GREEN —— 拦截是靠 gate 本身,而不是 harness
把 reject gate 关掉(让 resolveSingleWorkspaceInput 跳过 rejectUnsupportedMultiWorkspaceInputs)后重新构建:Harness A 变成 8 通过 / 9 失败 —— 恰好是 重复/嵌套/不同目录 那几条断言 —— 并且真实 daemon 此时会正常启动这些本该被 gate 的多 workspace 输入(静默取第一个)而不再拒绝。还原后 → 又回到 17/17 全绿。
结论
在 Linux 上端到端验证通过:gate 会在启动前 fail closed 并给出精确的、按类型区分的错误;单个/省略输入保持兼容;WorkspaceRegistry 与 createServeApp 注入契约的行为与文档一致(歧义 fail-closed、split-brain 拒绝);daemon 套件无回归。与已经变绿的 CI ubuntu Test job 一致。从验证角度 LGTM。✅
🤖 Generated with Claude Code — Claude Opus 4.8 (1M context)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Low-confidence suggestions noted in terminal review output for human consideration. Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review


What this PR does
This PR adds the Phase 2a multi-workspace sessions foundation while keeping multi-workspace serve gated. It makes
--workspacerepeatable at the CLI parser boundary, routes repeated values through the full parser, rejects duplicate, nested, and distinct multiple workspace inputs before runtime boot, and keeps single-workspace inputs compatible.It also extends the internal workspace runtime and registry contract with stable workspace metadata, exact cwd/id lookup, omitted-cwd primary resolution, and fail-closed live session owner resolution.
createServeAppcan now be assembled from an injected registry for tests and future internals, while all existing routes still use the primary runtime and legacy locals.The design record documents the Phase 2a foundation boundary and the route classification for later session dispatch work. This PR does not create non-primary runtimes, expose
workspaces[], publishmulti_workspace_sessions, add plural routes, change SDK types, or move ACP/voice/channel/file/memory/MCP/settings behavior.Why it's needed
Issue #6378 needs a safe internal boundary before session traffic can be routed across multiple workspaces. The previous single-workspace registry work established the primary runtime; this PR fills the remaining foundation gaps so later PRs can add env isolation, total admission, dispatch, and capabilities ungating without changing parser behavior or route ownership at the same time.
The repeated
--workspaceguardrail also prevents parser-dependent behavior from accidentally treating multiple explicit workspaces as one workspace before the full multi-workspace session loop is ready.Reviewer Test Plan
How to verify
Run the focused CLI and serve tests and confirm the parser, fast-path, workspace input resolver, registry, server assembly, and file-route compatibility assertions pass:
cd packages/cli && npx vitest run --coverage.enabled=false --fileParallelism=false --reporter=dot src/commands/serve.test.ts src/serve/workspace-inputs.test.ts src/serve/fast-path.test.ts src/serve/run-qwen-serve.test.ts src/serve/workspace-registry.test.ts,cd packages/cli && npx vitest run --coverage.enabled=false --fileParallelism=false --reporter=dot src/serve/server.test.ts -t "createServeApp ServeAppDeps.fsFactory wiring", andcd packages/cli && npx vitest run --coverage.enabled=false --fileParallelism=false --reporter=dot src/serve/routes/workspace-file-read.test.ts.Run repository validation and confirm no build, type, or lint errors:
npm run build && npm run typecheckandnpm run lint.Evidence (Before & After)
N/A. This is an internal daemon foundation, validation, and documentation change with no TUI or UI surface change.
Tested on
Environment (optional)
Node.js v22.22.3 and npm 10.9.8 in a local macOS worktree.
Risk & Scope
workspaces[], SDK type changes, plural workspace routes, ACP multi-workspace behavior, voice/channel/file/memory/MCP/settings migration, and Windows/Linux local validation are out of scope for this foundation PR.--workspacevalues now fail with a deliberate boot error instead of parser-dependent behavior; a single--workspaceand omitted workspace remain compatible.Linked Issues
Refs #6378
中文说明
What this PR does
这个 PR 增加 Phase 2a multi-workspace sessions 的内部地基,同时继续保持 multi-workspace serve gated。它在 CLI parser 边界让
--workspace可重复,重复值会走完整 parser,duplicate、nested 和 distinct multiple workspace inputs 会在 runtime boot 前失败,并保持单 workspace 输入兼容。它也扩展了内部 workspace runtime 和 registry contract,增加稳定 workspace metadata、精确 cwd/id lookup、cwd 省略时解析 primary,以及 fail-closed 的 live session owner resolution。
createServeApp现在可以从注入的 registry 装配,供测试和后续内部演进使用,但所有现有 routes 仍然只使用 primary runtime 和 legacy locals。设计记录文档化了 Phase 2a foundation 边界,以及后续 session dispatch 工作的 route classification。这个 PR 不创建 non-primary runtimes,不暴露
workspaces[],不发布multi_workspace_sessions,不增加 plural routes,不修改 SDK types,也不迁移 ACP/voice/channel/file/memory/MCP/settings 行为。Why it's needed
Issue #6378 需要先建立安全的内部边界,之后才能把 session traffic 路由到多个 workspace。之前的 single-workspace registry 工作建立了 primary runtime;这个 PR 补齐剩余 foundation gaps,让后续 PR 可以分别加入 env isolation、total admission、dispatch 和 capabilities ungate,而不需要同时改变 parser 行为和 route ownership。
重复
--workspaceguardrail 也能避免在完整 multi-workspace session 闭环准备好之前,因为 parser-dependent 行为把多个显式 workspace 意外当作一个 workspace 处理。Reviewer Test Plan
How to verify
运行聚焦的 CLI 和 serve tests,并确认 parser、fast-path、workspace input resolver、registry、server assembly 和 file-route compatibility 断言通过:
cd packages/cli && npx vitest run --coverage.enabled=false --fileParallelism=false --reporter=dot src/commands/serve.test.ts src/serve/workspace-inputs.test.ts src/serve/fast-path.test.ts src/serve/run-qwen-serve.test.ts src/serve/workspace-registry.test.ts,cd packages/cli && npx vitest run --coverage.enabled=false --fileParallelism=false --reporter=dot src/serve/server.test.ts -t "createServeApp ServeAppDeps.fsFactory wiring",以及cd packages/cli && npx vitest run --coverage.enabled=false --fileParallelism=false --reporter=dot src/serve/routes/workspace-file-read.test.ts。运行仓库级验证,并确认没有 build、type 或 lint 错误:
npm run build && npm run typecheck和npm run lint。Evidence (Before & After)
N/A。这个 PR 是内部 daemon foundation、validation 和 documentation 变更,没有 TUI 或 UI 表面变化。
Tested on
Environment (optional)
本地 macOS worktree,Node.js v22.22.3,npm 10.9.8。
Risk & Scope
workspaces[]、SDK type changes、plural workspace routes、ACP multi-workspace behavior、voice/channel/file/memory/MCP/settings migration,以及 Windows/Linux 本地验证都不在这个 foundation PR 范围内。--workspace现在会以明确 boot error 失败,不再保留 parser-dependent 行为;单个--workspace和省略 workspace 仍保持兼容。Linked Issues
Refs #6378