feat(cli): Add Phase 1 workspace runtime registry - #6394
Conversation
Introduce the internal single-workspace runtime registry for qwen serve and wire the primary runtime through the existing server assembly without changing route schemas. Also migrate daemon log and telemetry identity to daemon-scoped values, keep workspace hash as metadata, and reject repeated explicit --workspace inputs until multi-workspace serve is enabled. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Thanks for the PR! Template looks good ✓ Problem: This is a structural preparation PR — Phase 1 of the daemon multi-workspace plan described in issue #6378. No user-facing bug; the goal is to introduce an internal registry boundary so future phases can add workspace routing without restructuring the serve assembly layer. Motivation is clearly stated and linked to the tracking issue. Direction: Aligned with the phased approach in #6378. The scope is intentionally narrow — no new routes, no SDK changes, no multi-workspace CLI behavior. This is the right shape for a Phase 1: establish the boundary, wire it through, preserve existing behavior. CHANGELOG has no direct reference to multi-workspace daemon plans, but the area is clearly within serve/daemon infrastructure evolution. Approach: The registry pattern is minimal (~36 lines of new code in Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题: 这是结构性准备 PR——daemon multi-workspace 方案(issue #6378)的 Phase 1。没有用户可见的 bug;目标是引入内部 registry 边界,使后续 phase 可以添加 workspace routing 而无需重构 serve 装配层。动机清晰,关联了跟踪 issue。 方向: 与 #6378 的分阶段方案一致。范围刻意收窄——无新路由、无 SDK 变更、无 multi-workspace CLI 行为。这是 Phase 1 的正确形态:建立边界、贯穿接线、保留现有行为。 方案: Registry 模式极简( 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
There was a problem hiding this comment.
Pull request overview
Implements Phase 1 of the daemon multi-workspace plan for qwen serve by introducing an internal single-runtime WorkspaceRegistry boundary, while keeping the current single-workspace route behavior and adding guardrails from #6378 (daemon-scoped identity and clear rejection of repeated --workspace inputs).
Changes:
- Introduces a
WorkspaceRegistry/WorkspaceRuntimeabstraction and wires it intocreateServeAppviaapp.locals.workspaceRegistry. - Makes daemon identity daemon-scoped (log filename
serve-<pid>.log, telemetryservice.instance.id=daemon:<pid>), preserving workspace hash as metadata. - Updates
--workspaceparsing/boot validation to handle yargs’ repeatable option shape (string vs string[]) and reject multiple explicit values.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/src/serve/workspace-registry.ts | Adds Phase 1 single-runtime registry interfaces + factory. |
| packages/cli/src/serve/workspace-registry.test.ts | Unit tests for registry primary/list/lookup behavior. |
| packages/cli/src/serve/server/telemetry.ts | Resolves workspace hash per request (future multi-workspace readiness). |
| packages/cli/src/serve/server/telemetry.test.ts | Updates telemetry middleware unit tests for request-time resolution. |
| packages/cli/src/serve/server.ts | Builds registry for primary runtime and threads primary runtime values through server assembly. |
| packages/cli/src/serve/server.test.ts | Asserts registry is parked on app.locals and wiring preserves injected fsFactory. |
| packages/cli/src/serve/run-qwen-serve.ts | Adds boot-time resolution for workspace when provided as array; updates telemetry instance id and startup profiling id. |
| packages/cli/src/serve/run-qwen-serve.test.ts | Adds coverage for rejecting multiple --workspace, accepting single array input, and daemon-scoped telemetry id/log file name. |
| packages/cli/src/serve/daemon-logger.ts | Switches daemon id and log naming to daemon-scoped; moves workspace identity into structured ctx metadata. |
| packages/cli/src/serve/daemon-logger.test.ts | Updates expectations for daemon id/log path/log line metadata ordering. |
| packages/cli/src/commands/serve.ts | Updates CLI arg typing to allow repeatable --workspace inputs and adjusts settings load to use primary workspace. |
| packages/cli/src/commands/serve.test.ts | Adds parsing tests for single vs repeated --workspace. |
| .qwen/design/daemon-multi-workspace-phase1-registry.md | Design note documenting Phase 1 registry scope/bounds and verification. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code ReviewClean implementation. The registry module ( The Telemetry middleware ( Daemon identity ( Workspace arg handling ( Reuse check: no new utility functions that duplicate existing infrastructure. The registry is genuinely new surface. No critical blockers found. No AGENTS.md violations. Real-Scenario TestingBuilt the PR branch ( Daemon startup (tmux)/capabilities endpointDaemon status (new daemon-scoped identity)Daemon log file (new naming:
|
Final Decision: ✅ ApproveGate Summary
Decision RationaleThis PR is a clean Phase 1 foundation change: introduce a No behavioral change for end users in Phase 1 — the daemon still binds to a single workspace. The changes are purely structural preparation for Phase 2. Observations (non-blocking)
Cross-repo Guardrail Check
Approval中文说明最终决定:✅ 批准门禁总结
决策理由这是 Phase 1 的基础变更:引入 Phase 1 对终端用户无行为变化,纯粹是 Phase 2 的结构性准备。 观察项(非阻塞)
跨仓库门禁检查
— Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Memoize daemon telemetry workspace hashes and let runQwenServe honestly accept yargs workspace array inputs while keeping internal ServeOptions single-workspace. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
samuelhsin
left a comment
There was a problem hiding this comment.
Reviewed against the Phase 1 scope agreed in #6378 — the PR matches it: single-runtime WorkspaceRegistry wrapper, daemon-scoped log/telemetry identity, request-time telemetry workspace resolution, and repeatable --workspace input handling with a clear pre-boot error for multiple values. The primaryBridge/primaryWorkspace/primaryRouteFileSystemFactory switches in server.ts are pure aliases of the same objects, so route/session behavior is unchanged as intended.
Things I specifically verified, in case they save others the same pass:
- Early
opts.workspaceresolution breaks no explicitness checks. AfterresolveSingleWorkspaceInputnormalizes the value at options construction, the only remaining consumer is thecreateServeAppfallbackcanonicalizeWorkspace(opts.workspace ?? process.cwd())(server.ts:315), which now receives the same resolved value — no production path branches onworkspace === undefinedanymore. - The
daemon:<pid>colon is safe. RemaininggetDaemonId()consumers are JSON payloads (daemon-status.ts:319-320,run-qwen-serve.ts:1137-1138); the one filename consumer,finalizeStartupProfile, was switched toserve-${process.pid}in this PR, which avoids the Windows-invalid colon. - Registry lookup semantics match the RFC rule.
getByWorkspaceCwdis exact-match with trailing-slash/nested-path cases tested returningundefined; canonicalization stays with the future resolver, as specified. - The multi-value boot guard fires before any runtime side effects (token/bridge/listener), and a single-element array degrades to the existing single-workspace path — both covered by tests.
One non-blocking note for the release-notes callout this PR already plans: with the workspace hash removed from the log filename, a recycled pid can append a different workspace's daemon log to the same serve-<pid>.log. Each line carries workspaceHash so entries stay distinguishable, and same-workspace pid-reuse append was already possible today — fine to leave as-is, just worth a sentence alongside the identity-migration note.
LGTM.
中文说明
按 #6378 约定的 Phase 1 范围逐项核对,本 PR 完全一致:单 runtime 的 registry 包装(server.ts 里 primary* 替换是同对象纯别名,路由/会话行为不变)、daemon-scoped 身份迁移、telemetry 按请求解析 workspace、--workspace 多值在 runtime boot 前明确报错。
已专门验证的点:opts.workspace 提前解析后没有破坏任何"是否显式传参"判断(仅剩 server.ts:315 兜底且值相同);daemon:<pid> 的冒号安全(其余消费者都是 JSON,唯一文件名用途 finalizeStartupProfile 本 PR 已同步改为 serve-<pid>,避开 Windows 非法字符);registry 精确匹配语义符合 RFC(canonicalize 留给后续 resolver);多值 boot 守卫在任何 runtime 副作用之前触发,单元素数组退化为现有单 workspace 路径,均有测试。
一个不阻塞的备注:日志文件名去掉 workspace hash 后,pid 复用时不同 workspace 的日志可能追加进同一个 serve-<pid>.log;每行带 workspaceHash 可区分,且同 workspace 的 pid 复用追加是既有行为——保持现状即可,建议在 identity 迁移的 release note 里带一句。
LGTM.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings at high confidence. All 790 tests pass, deterministic analysis (tsc + eslint) is clean. Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Approved via triage pipeline. Clean Phase 1 foundation: WorkspaceRegistry abstraction, daemon identity migration, boot guard for repeated --workspace. All 821 tests pass, real-scenario verified. One non-blocking observation: add array:true to yargs --workspace option before Phase 2.
Make repeated --workspace values reach the serve runtime guard in both the yargs and fast-path CLI entrypoints. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
What this PR does
This PR implements Phase 1 of the daemon multi-workspace plan by introducing an internal single-runtime registry for
qwen serve. The existing primary workspace runtime now groups the bridge, workspace service, REST-route filesystem factory, and client-MCP sender registry behind a registry boundary while preserving the current single-workspace route behavior and legacy route locals.It also applies the Phase 1 guardrails from issue #6378: daemon log files and telemetry service instance ids are now daemon-scoped, with workspace hash kept as metadata, and repeated explicit
--workspaceinputs now fail clearly at boot instead of being mistaken for a single workspace path.Why it's needed
Phase 1 gives later multi-workspace work a narrow internal assembly boundary without changing public route schemas, SDK types,
/capabilities, or enabling multi-workspace CLI behavior. It also aligns daemon identity and repeatable workspace parsing with the latest issue guidance before Phase 2 adds workspace routing.Reviewer Test Plan
How to verify
Run
cd packages/cli && npx vitest run --fileParallelism=false src/commands/serve.test.ts src/serve/daemon-logger.test.ts src/serve/run-qwen-serve.test.ts src/serve/server/telemetry.test.ts src/serve/workspace-registry.test.ts src/serve/server.test.ts src/serve/routes/workspace-file-read.test.ts; expect all targeted serve tests to pass, including registry lookup, legacy file-route locals, daemon-scoped identity, per-request workspace hashing, yargs single/repeated workspace parsing, and repeated workspace boot rejection.Run
npm run build && npm run typecheck; expect the repository to build and typecheck successfully. Existing Browserslist and chunk-size warnings may still appear during frontend builds.Evidence (Before & After)
N/A. This is an internal daemon assembly change plus daemon identity and boot-guard behavior; there is no TUI or visual surface to compare.
Tested on
Environment (optional)
Local macOS worktree using the repository npm scripts, Node.js/npm from the configured development environment, and
gh2.92.0 for GitHub operations.Risk & Scope
service.instance.idintentionally migrate from workspace-scoped values to daemon-scoped values, so operators that key off the old identity format need to account for the newserve-<pid>.loganddaemon:<pid>forms.workspaces[]capability data, and does not implement session ownership indexing, env overlays, total-session capacity, or workspace-qualified ACP/voice/channel-worker behavior.--workspacevalues now produce a clear boot error until multi-workspace support is enabled; single--workspaceand omitted workspace behavior remain unchanged.Linked Issues
Related to #6378.
中文说明
What this PR does
这个 PR 实现 daemon multi-workspace 方案的 Phase 1,为
qwen serve引入内部 single-runtime registry。现有 primary workspace runtime 现在通过 registry 边界聚合 bridge、workspace service、REST route filesystem factory 和 client-MCP sender registry,同时保留当前单 workspace route 行为和 legacy route locals。它也落实 issue #6378 中 Phase 1 的两个 guardrail:daemon log 文件和 telemetry service instance id 改为 daemon-scoped,workspace hash 保留为元数据;重复显式传入的
--workspace现在会在 boot 阶段清晰失败,而不是被误当成单 workspace 路径。Why it's needed
Phase 1 为后续 multi-workspace 工作建立一个窄的内部装配边界,同时不改变 public route schema、SDK types、
/capabilities,也不启用 multi-workspace CLI 行为。它还先把 daemon identity 和 repeatable workspace parsing 与 issue 最新要求对齐,为 Phase 2 的 workspace routing 做准备。Reviewer Test Plan
How to verify
运行
cd packages/cli && npx vitest run --fileParallelism=false src/commands/serve.test.ts src/serve/daemon-logger.test.ts src/serve/run-qwen-serve.test.ts src/serve/server/telemetry.test.ts src/serve/workspace-registry.test.ts src/serve/server.test.ts src/serve/routes/workspace-file-read.test.ts;预期所有 targeted serve tests 通过,包括 registry lookup、legacy file-route locals、daemon-scoped identity、per-request workspace hashing、yargs single/repeated workspace parsing,以及 repeated workspace boot rejection。运行
npm run build && npm run typecheck;预期仓库构建和 typecheck 成功。前端构建期间仍可能出现已有的 Browserslist 和 chunk-size warnings。Evidence (Before & After)
N/A。这是内部 daemon 装配变更,以及 daemon identity 和 boot-guard 行为变更;没有 TUI 或视觉界面可对比。
Tested on
Environment (optional)
本地 macOS worktree,使用仓库 npm scripts、当前开发环境配置的 Node.js/npm,以及用于 GitHub 操作的
gh2.92.0。Risk & Scope
service.instance.id有意从 workspace-scoped 迁移为 daemon-scoped;如果运维侧依赖旧 identity 格式,需要适配新的serve-<pid>.log和daemon:<pid>形式。workspaces[]capability data,也不实现 session ownership indexing、env overlays、total-session capacity 或 workspace-qualified ACP/voice/channel-worker 行为。--workspace现在会在 multi-workspace 支持启用前产生清晰 boot error;单个--workspace和省略 workspace 的行为保持不变。Linked Issues
关联 #6378。