feat(cli): Add workspace-qualified core REST routes - #6567
Conversation
|
Re-triage (updated 2026-07-09) — head Thanks for the continued work on this! Template looks good ✓ — all required sections present with bilingual body. Problem: observed and tracked. This is Phase 3 of the multi-workspace daemon initiative (RFC #6378), building on Phase 1 (#6394), Phase 2a (#6410), and Phase 2b (#6511, #6558 — both merged). The dependency PR #6558 landed today. Clients need workspace-qualified REST routes to operate on secondary workspaces directly instead of tunneling through primary-only daemon state. Direction: clearly aligned. The CHANGELOG shows a systematic phase-by-phase rollout of multi-workspace support. This PR continues the established pattern — each legacy route gets a plural counterpart under Size: ~3,028 production lines + 135 deletions = ~3,163 production logic lines across 23 production files. ~1,479 test lines across 6 test files. Cross-package (cli + sdk-typescript). The 1000+ advisory applies — this is a large PR, but the scope is inherent to the surface area being covered (file, status, settings, permissions, trust, lifecycle, MCP, tools, memory, agents, sessions). Each route module is focused and under 250 production lines. Maintainer awareness has already been demonstrated through multiple runtime verification reports on this PR. Approach: the shared Moving on to code review and testing. 🔍 中文说明感谢持续的工作！ 模板完整 ✓ — 所有章节齐全，含中文对照。 问题： 已观测且有跟踪。这是多 workspace daemon 计划的 Phase 3（RFC #6378），建立在 Phase 1（#6394）、Phase 2a（#6410）和 Phase 2b（#6511、#6558——均已合并）之上。依赖 PR #6558 今天已合并。客户端需要 workspace-qualified REST 路由直接操作 secondary workspace，而不是通过 primary-only daemon 状态绕行。 方向： 明确对齐。CHANGELOG 展示了系统性的分阶段多 workspace 支持推进。本 PR 延续既有模式——每个 legacy 路由在 规模： ~3,028 行生产代码 + 135 行删除 = ~3,163 行生产逻辑，跨 23 个生产文件。~1,479 行测试代码，跨 6 个测试文件。跨包（cli + sdk-typescript）。1000+ 大 PR 建议适用——但范围是所覆盖面（文件、状态、设置、权限、信任、生命周期、MCP、工具、memory、agents、sessions）的固有需求。每个路由模块专注且低于 250 行生产代码。维护者已通过多次运行时验证报告表明知晓。 方案： 共享 进入代码审查和测试 🔍 — Qwen Code · qwen3.7-max |
|
Re-triage (updated 2026-07-09) — fresh verification after main merge into branch. Code ReviewIndependent proposal: before reading the diff, I'd approach this by (1) creating a shared workspace selector middleware that resolves Comparison: the PR's approach matches my independent proposal. The No critical blockers found. The implementation is consistent:
One observation (not a blocker): Real-Scenario TestingBuild: ✅ Typecheck: ✅ Lint: ✅ Unit tests: ✅ 312 tests pass:
Daemon runtime verification (started from worktree build, port 4179): Capability advertised: Plural routes verified: Daemon telemetry confirms correct routing: All plural routes resolve the workspace selector correctly and return expected responses. The workspace_mismatch error fires for invalid selectors. The daemon handles the single-workspace case (no 中文说明代码审查独立方案： 在阅读 diff 之前，我会这样做：(1) 创建共享的 workspace selector middleware，将 对比： PR 方案与我的独立方案一致。 未发现关键阻塞问题。 实现一致：选择器解析处理 id、POSIX 绝对路径、Windows 盘符和 UNC 路径；plural 路由上的信任门控比 legacy 更严格；plural memory/agents/permissions 拒绝 global/user scope；文件系统在 plural 路由上从 WorkspaceRouteContext 解析。 一个观察（非阻塞）： 实际场景测试构建： ✅ 全部编译通过 Daemon 运行时验证： daemon 启动正常， — Qwen Code · qwen3.7-max |
|
Re-triage (updated 2026-07-09) — final assessment after all verification. Stepping back: this is the third phase of a carefully sequenced multi-workspace daemon initiative. The RFC (#6378) laid out the architecture, Phases 1–2 built the foundation (registry, sessions, persistence), and this PR adds the REST surface that clients actually need to operate on secondary workspaces. The implementation matches what I'd have designed independently — shared selector resolution + trust middleware, mirrored plural route handlers, SDK wrapper with URL encoding. Each route module is focused (under 250 lines), the shared runtime helper is 148 lines, and the test coverage is thorough (92 CLI + 220 SDK tests, plus the maintainer's own multi-round runtime verification). The code is straightforward — no clever tricks, no premature abstractions, no speculative features beyond what the phase plan calls for. The 312 tests pass, build/typecheck/lint all clean, daemon starts and serves all plural routes correctly. The maintainer has independently verified this multiple times with full runtime suites. This is a well-scoped feature addition in a tracked initiative. LGTM. 中文说明退一步看：这是多 workspace daemon 计划的第三阶段。RFC（#6378）设计了架构，Phase 1–2 构建了基础（注册表、sessions、持久化），本 PR 添加了客户端实际需要的 secondary workspace REST 操作面。 实现与我的独立设计一致——共享选择器解析 + 信任中间件，镜像 plural 路由处理器，SDK 包装器带 URL 编码。每个路由模块专注（低于 250 行），共享运行时 helper 148 行，测试覆盖充分（CLI 92 + SDK 220 测试，加上维护者自己多轮运行时验证）。 代码直截了当——没有花哨技巧，没有过早抽象，没有超出阶段计划的投机功能。 312 个测试通过，build/typecheck/lint 全部干净，daemon 启动并正确服务所有 plural 路由。维护者已独立多次验证完整运行时套件。 这是一个跟踪计划中范围明确的功能添加。LGTM。 — Qwen Code · qwen3.7-max |
There was a problem hiding this comment.
Pull request overview
This PR adds a Phase 3 REST surface for selecting a non-primary workspace via /workspaces/:workspace/... (by workspace id or URL-encoded absolute cwd), and wires that selection through core “workspace” features (file/status/settings/permissions/trust/lifecycle/MCP/tools/memory/agents/sessions). It also extends the TypeScript SDK with WorkspaceDaemonClient helpers and updates telemetry normalization and protocol documentation to reflect the new capability.
Changes:
- Add workspace selector + trust-gating helpers and mount new workspace-qualified REST routes across the CLI serve layer.
- Extend the TypeScript SDK with
workspaceById()/workspaceByCwd()and a newWorkspaceDaemonClientfacade, plus unit tests. - Normalize telemetry for plural workspace routes, advertise the
workspace_qualified_rest_corecapability, and document the new contract.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/sdk-typescript/test/unit/DaemonClient.test.ts | Adds unit coverage for SDK workspace-qualified agents route URL construction + headers. |
| packages/sdk-typescript/src/index.ts | Re-exports WorkspaceDaemonClient from the package entrypoint. |
| packages/sdk-typescript/src/daemon/index.ts | Re-exports WorkspaceDaemonClient from the daemon barrel. |
| packages/sdk-typescript/src/daemon/DaemonClient.ts | Introduces workspace-qualified request helpers and the WorkspaceDaemonClient API surface. |
| packages/sdk-typescript/scripts/build.js | Raises the daemon browser bundle size budget for the new SDK surface area. |
| packages/cli/src/serve/workspace-route-runtime.ts | Adds workspace selector resolution + trust helper utilities used by plural routes. |
| packages/cli/src/serve/workspace-qualified-rest.test.ts | Adds integration-style tests for selector behavior, trust gating, scope rejection, and capability advertisement. |
| packages/cli/src/serve/workspace-memory.ts | Mounts workspace-qualified memory status/mutation routes with trust + scope enforcement. |
| packages/cli/src/serve/workspace-agents.ts | Mounts workspace-qualified agents CRUD routes with trust + scope enforcement. |
| packages/cli/src/serve/server/telemetry.ts | Normalizes telemetry route labels for plural /workspaces/:workspace/... routes. |
| packages/cli/src/serve/server/telemetry.test.ts | Adds coverage ensuring plural workspace agent routes map to stable telemetry labels. |
| packages/cli/src/serve/server.ts | Mounts new plural routes and updates telemetry workspace attribution to resolve selected workspace when possible. |
| packages/cli/src/serve/routes/workspace-trust.ts | Adds workspace-qualified trust status + trust request routes (including untrusted workspaces). |
| packages/cli/src/serve/routes/workspace-tools.ts | Adds workspace-qualified tool enable/disable route with trusted-runtime gating. |
| packages/cli/src/serve/routes/workspace-status.ts | Adds workspace-qualified status/diagnostic status routes with trusted-runtime gating. |
| packages/cli/src/serve/routes/workspace-settings.ts | Adds workspace-qualified settings read/write route with trusted-runtime gating. |
| packages/cli/src/serve/routes/workspace-permissions.ts | Adds workspace-qualified permissions read/write routes with trusted-runtime + scope enforcement. |
| packages/cli/src/serve/routes/workspace-mcp-control.ts | Adds workspace-qualified MCP mutation routes with trusted-runtime gating + client id validation. |
| packages/cli/src/serve/routes/workspace-lifecycle.ts | Adds workspace-qualified init/reload lifecycle routes with trusted-runtime gating + client id validation. |
| packages/cli/src/serve/routes/workspace-file-write.ts | Adds workspace-qualified file write/edit routes by injecting selected workspace context into existing handlers. |
| packages/cli/src/serve/routes/workspace-file-read.ts | Adds workspace-qualified file read/stat/list/glob routes and adjusts relative-path projection for selected workspaces. |
| packages/cli/src/serve/routes/session.ts | Adds workspace-qualified session batch operations and session group CRUD. |
| packages/cli/src/serve/capabilities.ts | Advertises the new workspace_qualified_rest_core capability. |
| docs/developers/qwen-serve-protocol.md | Documents selector semantics, trust policy, and the new plural route surfaces. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
workspace-route-runtime.ts:137 |
Shared sendWorkspaceMismatch omits boundWorkspace/requestedWorkspace (session.ts local version now includes both) |
Expand shared helper to match session.ts pattern |
workspace-route-runtime.ts:105 |
requireTrustedWorkspaceRuntime doesn't pass workspaceCwd/workspaceId to sendUntrustedWorkspaceResponse |
Pass runtime.workspaceCwd and runtime.workspaceId |
workspace-qualified-rest.test.ts |
file/edit, MCP disable/authenticate/clear-auth, file write mode:replace lack test coverage |
Add tests for untested route categories |
workspace-route-runtime.ts:53 |
Bare catch {} around canonicalizeWorkspace() silently swallows EACCES/EIO/ELOOP |
Narrow to ENOENT or log swallowed errors |
workspace-file-read.ts |
File read routes lack trust gate (unlike all other workspace-qualified routes) | Add requireTrustedWorkspaceRuntime or document the exemption |
— qwen3.7-max via Qwen Code /review
a348596 to
b1e473d
Compare
|
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）为单个提交。 |
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>
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
— 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>
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>
| }); | ||
| } | ||
|
|
||
| function resolveTrustedRuntime( |
There was a problem hiding this comment.
[Suggestion] This resolveTrustedRuntime function is identical to resolveTrustedMcpRuntime in workspace-mcp-control.ts:231 — same 6-line composition of resolveWorkspaceRuntimeFromParam + requireTrustedWorkspaceRuntime. If the trust-gate semantics change, both copies must update in lockstep.
Consider exporting a single shared resolveTrustedWorkspaceRuntime from workspace-route-runtime.ts.
— qwen3.7-max via Qwen Code /review
| const selector = decodeURIComponent(rawSelector); | ||
| const byId = workspaceRegistry.getByWorkspaceId(selector); | ||
| if (byId) return byId.workspaceCwd; | ||
| if (isPortableAbsolutePath(selector)) { |
There was a problem hiding this comment.
[Suggestion] Double workspace resolution per request: the telemetry middleware calls resolveRegisteredWorkspaceRuntimeByPathSelector here to extract workspaceCwd for hashing, then the matched route handler calls resolveWorkspaceRuntimeFromParam again (which invokes the same resolution internally). For path-based selectors, this doubles the canonicalizeWorkspace filesystem I/O (realpathSync).
Consider caching the resolved runtime on the Express req object during this middleware pass (via setWorkspaceRouteContext), or moving telemetry to run after Express route matching.
— qwen3.7-max via Qwen Code /review
| if (req.method === 'GET' && /^\/workspaces\/[^/]+\/sessions$/.test(path)) { | ||
| return { route: 'GET /workspace/:id/sessions' }; | ||
| } | ||
| const pluralWorkspacePrefix = /^\/workspaces\/[^/]+/; |
There was a problem hiding this comment.
[Suggestion] Telemetry route mapping maintenance trap: ~100 lines of hardcoded suffix-to-route mappings for workspace-qualified URLs. Every new route must be manually added to this list with no compile-time or runtime enforcement — a missing entry silently degrades telemetry.
Consider deriving the telemetry route name from Express's req.route.path (available after route matching), or adding a test that asserts every registered workspace-qualified route has a corresponding telemetry mapping.
— qwen3.7-max via Qwen Code /review
| @@ -290,3 +321,39 @@ export function registerWorkspaceFileWriteRoutes( | |||
| handlePostFileEdit(req, res, deps), | |||
There was a problem hiding this comment.
[Suggestion] Test coverage gaps across new workspace-qualified routes:
POST /workspaces/:workspace/file/editis registered here but has no test (onlyfile/writeis tested)- MCP actions
disable,authenticate,clear-authinworkspace-mcp-control.tsare untested (onlyenablehas a test) - File-read sub-routes
/file/bytes,/stat,/list,/globhave no workspace-qualified test - ~15+
WorkspaceDaemonClientSDK methods have no test coverage - Telemetry route patterns are tested only for the agent route family (~5 of ~25 patterns)
A miswired route (e.g., wrong handler mapping in the MCP action loop) or a URL-encoding bug in the SDK would ship undetected.
— qwen3.7-max via Qwen Code /review
Re-verification report — head
|
| Request | before | now |
|---|---|---|
GET /workspaces/<symlink>/file |
400 |
200 |
GET /workspaces/<symlink>/sessions |
400 |
200 |
GET /workspaces/<symlink>/settings / /mcp |
400 |
200 |
POST /session {cwd:<symlink>} (control) |
200 |
200 |
Telemetry attribution followed the fix — the symlinked selector now resolves to the secondary workspace instead of falling back to primary:
[TELEMETRY] GET /workspaces/%2F…%2Fws%2Fsecondary-link/file wsHash=20c04f1684ce94f2 ← was 5692ffac (primary)
Single-decode, proven positively. The error body no longer echoes the selector, so I registered a fourth workspace whose cwd literally contains %20: /var/tmp/…/ws/pct%20name. A double-decoding resolver would turn the encoded selector into …/pct name and miss it.
GET /workspaces/29ed285b908d6e1d/file → 200 (by id)
GET /workspaces/%2F…%2Fws%2Fpct%2520name/file → 200 (by encoded cwd, %25 survives)
GET /workspaces/%2F…%2Fws%2Fpct%20name/file → 400 (what a double decode would produce)
Finding 2 — untrusted-primary asymmetry. Behavior is unchanged (still 10/12 read-only routes diverge on a DO_NOT_TRUST primary), but it is now an explicit, documented contract: "This plural trust gate is intentionally stricter than some legacy primary-workspace read routes, which keep their existing compatibility behavior and are not drop-in replacements." That's exactly what I asked for. Accepted.
Finding 6 — doc drift. All three items corrected: the selector really does canonicalize now; the file-write trust policy text matches the 403 untrusted_workspace short-circuit; and the single-workspace discoverability gap is documented (clients use capabilities.workspaceCwd as the cwd selector). Verified live: on a single-workspace daemon capabilities.workspaces[] is absent, workspaceCwd is present, and the cwd selector returns 200.
Finding 5 — mutation testing now has teeth. This is the important one. Re-running my mutations against the grown suites (workspace-qualified-rest.test.ts + multi-workspace-sessions.test.ts, 50 tests baseline):
| Mutation | last round | now |
|---|---|---|
remove the canonicalizeWorkspace fallback (regress the symlink fix) |
n/a | 1 test fails ✅ |
| drop the plural memory scope gate | 0 fail ❌ | 1 test fails ✅ |
point plural sessions/archive + unarchive at boundWorkspace |
0 fail ❌ | 1 test fails ✅ |
point plural sessions/delete at boundWorkspace |
0 fail ❌ | 1 test fails ✅ |
point plural session-groups CRUD at boundWorkspace |
0 fail ❌ | 1 test fails ✅ |
requireTrustedWorkspaceRuntime → always true |
7 fail ✅ | still covered ✅ |
The destructive persisted-session routes are now regression-protected against silently targeting the wrong workspace. That was my main merge blocker and it's resolved.
Unchanged and still correct: trust gate matrix (all 40 plural routes against the untrusted workspace — 0 ungated), FS confinement (../primary/p.txt, /etc/passwd → 400 path_outside_workspace on trusted and untrusted runtimes), per-workspace session isolation, route/SDK parity (out-of-scope surfaces still 404).
🔴 New regression: CI is red, and the cause is a breaking change to pre-existing routes
Test (ubuntu-latest, Node 22.x) fails. I reproduced it locally — 3 failures in src/serve/server.test.ts, all the same cause:
× createServeApp > POST /session/:id/load and /resume > 400s unknown explicit cwd before touching the bridge
× createServeApp > POST /session/:id/load and /resume > 400 workspace_mismatch before touching the bridge for non-primary cwd
× createServeApp > GET /workspace/:id/sessions > 400 workspace_mismatch when querying a cross-workspace path (#3803 §02)
expect(res.body.boundWorkspace).toBe(WS_BOUND) → received: undefined
This round rewrote sendWorkspaceMismatch and the untrusted-403 body to strip boundWorkspace, requestedWorkspace, workspaceCwd, and workspaceId — but it applied that to the shared helper and to session.ts's local closure, which serve pre-existing routes. Measured on a live daemon:
Route (exists on main today) |
main response |
this PR |
|---|---|---|
POST /session (unregistered cwd) |
{error, code, boundWorkspace, requestedWorkspace} |
{error, code, workspaceCount} |
POST /session (registered, untrusted cwd) |
{error, code, workspaceCwd} |
{error, code} |
GET /workspace/<unknown>/sessions |
{…, boundWorkspace, requestedWorkspace} |
{error, code, workspaceCount} |
Three consequences:
-
CI is red. Those 3 tests are unmodified by this PR — they come straight from
main. The only change the PR makes toserver.test.tsis addingworkspace_qualified_rest_coreto the capability baseline. -
A documented contract is now wrong.
docs/developers/qwen-serve-protocol.mdstill specifies theWorkspaceMismatchErrorbody forPOST /sessionas containingboundWorkspaceandrequestedWorkspace, and tells clients to "route the request to a daemon bound torequestedWorkspace." That section wasn't updated. -
It contradicts "no intended breaking changes" in the PR description.
POST /sessionandGET /workspace/:id/sessionsare outside this PR's stated scope.
On the motivation: I asked for this strip in an inline comment, framing it as an anti-enumeration bypass. Having now measured it, I don't think that framing holds, and I'd rather correct myself than have you carry a broken contract for it. /capabilities sits behind the same bearer token as the mismatch route:
no token: GET /capabilities → 401
no token: GET /workspaces/%2Fno%2Fsuch/file → 401
token: GET /capabilities → workspaceCwd: /…/ws/primary
workspaces[].cwd: primary | secondary | untrusted | pct%20name
Any caller who can trigger a workspace_mismatch can already read every workspace path from /capabilities. So stripping the fields closes nothing, while costing three tests and a documented, useful field. requestedWorkspace in particular is just the caller's own input echoed back.
Suggested resolution — pick one:
- (a) Preferred: keep the minimal bodies on the new plural routes only, and restore
boundWorkspace/requestedWorkspace/workspaceCwdonPOST /session,POST /session/:id/load|resume, andGET /workspace/:id/sessions. Zero behavior change outside this PR's scope, CI green, docs stay true. - (b) Keep the strip everywhere, but then update the 3 tests, rewrite the
WorkspaceMismatchErrorsection ofqwen-serve-protocol.md, and move the change out of a "no breaking changes" PR into its own so clients see it in the changelog.
🟡 Still open from last round (unchanged code)
Finding 3 — telemetry labels name routes that don't exist. server/telemetry.ts is byte-identical to last round. Re-traced from the running daemon:
POST /workspaces/:ws/file/write → label "POST /workspace/file/write" (no such route)
POST /workspaces/:ws/sessions/archive → label "POST /workspace/sessions/archive" (real: POST /sessions/archive)
GET /workspaces/:ws/file → label "GET /file" (legacy GET /file emits nothing)
The GET /file metric series still counts only workspace-qualified traffic.
Finding 4 — scope-rejection vocabulary. The memory scope gate is now tested, but the vocabulary is still inconsistent. Re-measured:
scope |
plural /memory |
plural /agents |
plural /permissions |
|---|---|---|---|
project |
400 global_scope_not_supported… |
accepted | 400 global_scope_not_supported… |
wrokspace (typo) |
400 global_scope_not_supported… |
400 invalid_scope |
400 global_scope_not_supported… |
workspace-memory.ts:346 still reports a typo'd or omitted scope as "global scope not supported."
Both remain follow-up material, not blockers.
Note on workspace_qualified_rest_core gating
This round removed the persistSettingAvailable condition and instead documented that workspace-qualified settings also require the workspace_settings tag. That's a fine resolution — verified that qwen serve always sets persistSetting, so there's no observable regression; it only matters for embedders calling createServeApp directly, and the capability descriptor now says so explicitly.
Verdict
The substance of this PR is in good shape. Selector resolution, trust gating, per-workspace routing, and now the test coverage of the destructive session routes are all correct under real traffic — I mutated each one and the suite caught it.
The only thing standing between this and merge is the error-body regression. Fix it per (a) or (b), get CI green, and I'm happy to approve. Findings 3 and 4 can land as a follow-up.
中文版
复验报告 —— head 1c9ee5731
我重新从源码构建，并针对更新后的 head 重跑了上一份报告里的完整运行时套件，同时针对本轮新引入的改动补了新的探测。
我提出的四个实质性问题中，三个已修复，而且修复都有真实测试兜底。 但本轮也引入了一个回归，它让 CI 变红，且影响的是本 PR 范围之外的既有路由。
✅ 已修复并验证
发现 1 —— 符号链接 cwd selector。 resolveRegisteredWorkspaceRuntimeByPathSelector 现在会回退到 canonicalizeWorkspace()。在真实的 4-workspace daemon 上重跑（ws/secondary-link → ws/secondary）：
| 请求 | 之前 | 现在 |
|---|---|---|
GET /workspaces/<symlink>/file |
400 |
200 |
GET /workspaces/<symlink>/sessions |
400 |
200 |
GET /workspaces/<symlink>/settings / /mcp |
400 |
200 |
POST /session {cwd:<symlink>}（对照组） |
200 |
200 |
telemetry 归属也随之修正——符号链接 selector 现在归属到 secondary，而不是回退到 primary：
[TELEMETRY] GET /workspaces/%2F…%2Fws%2Fsecondary-link/file wsHash=20c04f1684ce94f2 ← 之前是 5692ffac（primary）
单次解码，这次是正面证明。 错误响应体不再回显 selector，所以我注册了第四个 workspace，其 cwd 路径中真的含有 %20：/var/tmp/…/ws/pct%20name。如果解析器解码两次，编码后的 selector 会变成 …/pct name，从而匹配不到它。
GET /workspaces/29ed285b908d6e1d/file → 200 （按 id）
GET /workspaces/%2F…%2Fws%2Fpct%2520name/file → 200 （按编码 cwd，%25 被保留）
GET /workspaces/%2F…%2Fws%2Fpct%20name/file → 400 （这正是双重解码会产生的结果）
发现 2 —— 未受信 primary 的不对称。 行为没有变（在 DO_NOT_TRUST 的 primary 上仍有 12 条只读路由中的 10 条不一致），但现在它是一个明确的、写进文档的契约："This plural trust gate is intentionally stricter than some legacy primary-workspace read routes, which keep their existing compatibility behavior and are not drop-in replacements." 这正是我要求的。接受。
发现 6 —— 文档偏差。 三处全部修正：selector 现在确实做 canonicalization；file-write 的信任策略描述与 403 untrusted_workspace 短路一致；单 workspace 下的可发现性缺口也写进了文档（clients use capabilities.workspaceCwd as the cwd selector）。实测确认：单 workspace daemon 上 capabilities.workspaces[] 缺席，workspaceCwd 存在，cwd selector 返回 200。
发现 5 —— 变异测试现在有牙齿了。 这是最重要的一点。针对扩充后的测试套件（workspace-qualified-rest.test.ts + multi-workspace-sessions.test.ts，基线 50 个测试）重跑我的变异：
| 变异 | 上一轮 | 现在 |
|---|---|---|
删掉 canonicalizeWorkspace 回退（回退符号链接修复） |
无 | 1 个测试失败 ✅ |
| 删掉 plural memory 的 scope 门禁 | 0 失败 ❌ | 1 个测试失败 ✅ |
把 plural sessions/archive + unarchive 指向 boundWorkspace |
0 失败 ❌ | 1 个测试失败 ✅ |
把 plural sessions/delete 指向 boundWorkspace |
0 失败 ❌ | 1 个测试失败 ✅ |
把 plural session-groups CRUD 指向 boundWorkspace |
0 失败 ❌ | 1 个测试失败 ✅ |
requireTrustedWorkspaceRuntime → 恒 true |
7 失败 ✅ | 仍被覆盖 ✅ |
破坏性的持久化 session 路由现在对"静默打到错误 workspace"有了回归保护。这是我上次的主要合并阻塞项，已经解决。
保持不变且依然正确的部分： 信任门禁矩阵（全部 40 条 plural 路由对未受信 workspace —— 零条未受门禁）、文件系统边界（../primary/p.txt、/etc/passwd → 在受信与未受信 runtime 上都是 400 path_outside_workspace）、按 workspace 的 session 隔离、路由与 SDK 的对齐（范围外的面仍然 404）。
🔴 新回归：CI 变红，根因是对既有路由的破坏性改动
Test (ubuntu-latest, Node 22.x) 失败。我在本地复现了——src/serve/server.test.ts 中 3 个失败，根因相同：
× createServeApp > POST /session/:id/load and /resume > 400s unknown explicit cwd before touching the bridge
× createServeApp > POST /session/:id/load and /resume > 400 workspace_mismatch before touching the bridge for non-primary cwd
× createServeApp > GET /workspace/:id/sessions > 400 workspace_mismatch when querying a cross-workspace path (#3803 §02)
expect(res.body.boundWorkspace).toBe(WS_BOUND) → received: undefined
本轮重写了 sendWorkspaceMismatch 和未受信 403 的响应体，剥离了 boundWorkspace、requestedWorkspace、workspaceCwd、workspaceId——但改动同时落在了共享 helper 和 session.ts 里的局部闭包上，而后者服务的是既有路由。在真实 daemon 上实测：
路由（main 上今天就有） |
main 的响应 |
本 PR |
|---|---|---|
POST /session（未注册 cwd） |
{error, code, boundWorkspace, requestedWorkspace} |
{error, code, workspaceCount} |
POST /session（已注册但未受信 cwd） |
{error, code, workspaceCwd} |
{error, code} |
GET /workspace/<unknown>/sessions |
{…, boundWorkspace, requestedWorkspace} |
{error, code, workspaceCount} |
三个后果：
-
CI 红了。 那 3 个测试并未被本 PR 修改——它们直接来自
main。本 PR 对server.test.ts的唯一改动是把workspace_qualified_rest_core加进 capability 基线。 -
一份写进文档的契约现在是错的。
docs/developers/qwen-serve-protocol.md仍然把POST /session的WorkspaceMismatchError响应体规定为包含boundWorkspace和requestedWorkspace，并告诉客户端 "route the request to a daemon bound torequestedWorkspace." 这一节没有被更新。 -
它与 PR 描述里的 "no intended breaking changes" 相矛盾。
POST /session和GET /workspace/:id/sessions都在本 PR 声明的范围之外。
关于动机： 这个剥离是我在一条 inline comment 里要求的，当时我把它定性为"绕过 403 反枚举"。现在实测之后，我认为那个定性站不住脚，与其让你为它背上一个被破坏的契约，不如我先纠正自己。/capabilities 和 mismatch 路由处在同一道 bearer token 之后：
无 token: GET /capabilities → 401
无 token: GET /workspaces/%2Fno%2Fsuch/file → 401
有 token: GET /capabilities → workspaceCwd: /…/ws/primary
workspaces[].cwd: primary | secondary | untrusted | pct%20name
任何能触发 workspace_mismatch 的调用方，本来就能从 /capabilities 读到全部 workspace 路径。所以剥离这些字段并没有关掉任何缺口，代价却是三个测试和一个有文档、有用处的字段。尤其 requestedWorkspace 只是把调用方自己的输入回显一遍。
建议的处理方式——二选一：
- (a) 推荐： 只在新的 plural 路由上保留精简响应体，并在
POST /session、POST /session/:id/load|resume、GET /workspace/:id/sessions上恢复boundWorkspace/requestedWorkspace/workspaceCwd。本 PR 范围之外零行为变更，CI 变绿，文档保持正确。 - (b) 全面保留剥离，但相应地更新那 3 个测试、重写
qwen-serve-protocol.md的WorkspaceMismatchError一节，并把这个改动从"无破坏性变更"的 PR 里拆出去单独提，好让客户端能在 changelog 里看到。
🟡 上一轮遗留、代码未变
发现 3 —— telemetry 标签指向不存在的路由。 server/telemetry.ts 与上一轮逐字节相同。从运行中的 daemon 重新抓取：
POST /workspaces/:ws/file/write → 标签 "POST /workspace/file/write" （不存在此路由）
POST /workspaces/:ws/sessions/archive → 标签 "POST /workspace/sessions/archive" （真实的是 POST /sessions/archive）
GET /workspaces/:ws/file → 标签 "GET /file" （legacy GET /file 不发 telemetry）
名为 GET /file 的指标序列仍然只统计 workspace-qualified 的流量。
发现 4 —— scope 拒绝措辞。 memory 的 scope 门禁现在有测试了，但措辞依旧不一致。重新实测：
scope |
plural /memory |
plural /agents |
plural /permissions |
|---|---|---|---|
project |
400 global_scope_not_supported… |
接受 | 400 global_scope_not_supported… |
wrokspace（拼写错误） |
400 global_scope_not_supported… |
400 invalid_scope |
400 global_scope_not_supported… |
workspace-memory.ts:346 仍然把拼错或省略的 scope 报成 "global scope not supported"。
这两项都属于后续改进，不是阻塞项。
关于 workspace_qualified_rest_core 的 gating
本轮移除了 persistSettingAvailable 条件，改为在文档中说明 workspace-qualified settings 还需要 workspace_settings tag。这是一个合理的处理方式——我确认了 qwen serve 总会设置 persistSetting，所以没有可观察到的回归；它只影响直接调用 createServeApp 的嵌入方，而 capability 描述里现在已经明确写了这一点。
结论
这个 PR 的实质内容状态良好。selector 解析、信任门禁、按 workspace 路由，以及现在破坏性 session 路由的测试覆盖，在真实流量下都是正确的——我对每一条都做了变异，测试套件都抓到了。
距离合并只差那个错误响应体的回归。 按 (a) 或 (b) 处理，把 CI 弄绿，我就可以 approve。发现 3 和 4 可以作为后续 PR。
| const canonicalMatch = registry.getByWorkspaceCwd(canonicalSelector); | ||
| if (canonicalMatch) return canonicalMatch; | ||
| for (const runtime of registry.list()) { | ||
| if (canonicalizeWorkspace(runtime.workspaceCwd) === canonicalSelector) { |
There was a problem hiding this comment.
[Suggestion] Redundant canonicalizeWorkspace() on already-canonical registry entries. The workspace registry stores CWDs that were canonicalized at boot time (via canonicalizeWorkspace() before insertion into byCwd), so calling canonicalizeWorkspace(runtime.workspaceCwd) here re-runs realpathSync.native — a synchronous filesystem syscall — on paths that are already canonical.
For N registered workspaces, this performs N unnecessary syscalls on every path-selector miss, blocking the event loop each time.
Suggested fix: Compare canonicalSelector directly against runtime.workspaceCwd:
for (const runtime of registry.list()) {
if (runtime.workspaceCwd === canonicalSelector) {
return runtime;
}
}— incremental review since 718e80a
| if (!isPortableAbsolutePath(selector)) { | ||
| res.status(400).json({ | ||
| error: `\`:${paramName}\` must decode to a workspace id or absolute path`, | ||
| code: 'workspace_mismatch', |
There was a problem hiding this comment.
[Suggestion] Differentiated error responses enable workspace enumeration. The resolver returns three distinguishable response classes:
| Selector input | Status | Body |
|---|---|---|
| Not a valid format | 400 | {code: "workspace_mismatch"} (no workspaceCount) |
| Valid absolute path, unregistered | 400 | {code: "workspace_mismatch", workspaceCount: N} |
| Registered, untrusted | 403 | {code: "untrusted_workspace"} |
An attacker probing selectors can distinguish all three states to enumerate registered workspace paths, learn trust status, and learn the total workspace count.
Consider returning the same opaque 400 response for both invalid-format and unregistered selectors, and removing workspaceCount from the error body.
— incremental review since 718e80a
| }); | ||
| res.status(200).json({ | ||
| key, | ||
| scope, |
There was a problem hiding this comment.
[Suggestion] Route-parity maintenance risk: this plural settings POST inlines deps.invalidateServeFeaturesCache() + runtime.bridge.publishWorkspaceEvent(...) directly, while the legacy route delegates to the shared broadcastSettingsChanged helper (defined in server.ts:973).
The inlining is intentional here (different bridge target: runtime.bridge vs primaryBridge), but if broadcastSettingsChanged ever gains additional side effects (logging, metrics, etc.), this plural route will not pick them up automatically.
Consider extracting a shared helper that accepts the bridge as a parameter:
broadcastSettingsChanged(key, value, scope, clientId, bridge);— incremental review since 718e80a
| ); | ||
| } | ||
|
|
||
| fileStat(filePath: string): Promise<unknown> { |
There was a problem hiding this comment.
[Suggestion] fileStat, dirList, and glob are the only WorkspaceDaemonClient methods that return Promise<unknown>. Every other method uses typed interfaces (DaemonWorkspaceFile, DaemonToolToggleResult, etc.).
Consumers get no type safety and must cast locally. Consider defining DaemonFileStat, DaemonDirList, and DaemonGlobResult interfaces matching the server response shapes.
— incremental review since 718e80a
|
| # | File | Severity | Issue |
|---|---|---|---|
| 1 | workspace-route-runtime.ts:52-54 |
Suggestion | Redundant canonicalizeWorkspace() on already-canonical registry entries in the fallback loop — unnecessary realpathSync syscalls |
| 2 | DaemonClient.ts:3615-3631 |
Suggestion | fileStat/dirList/glob return Promise<unknown> — only untyped methods on the class |
| 3 | workspace-route-runtime.ts:82-98 |
Suggestion | Differentiated error responses (400 without workspaceCount for invalid format vs 400 with workspaceCount for unregistered path vs 403 for untrusted) enable workspace enumeration |
| 4 | workspace-settings.ts:449-453 |
Suggestion | Plural settings POST inlines cache invalidation + bridge event instead of shared helper — route-parity maintenance risk |
| 5 | Test coverage | Suggestion | Agents routes missing untrusted-workspace rejection test; settings validation error branches, permissions invalid_rule_type, and trust error branches untested |
Verdict
Comment. The PR is in significantly better shape than the previous revision. The core architecture (shared resolver, trust helpers, SDK client) is well-designed. The remaining findings are Suggestion-level improvements that can be addressed in follow-up commits.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
@qwen-code /resolve |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
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 Merge Conflict Resolution Summary — PR #6567Base branch
Conflicts1.
|
Re-verification report — head
|
| Route | body now |
|---|---|
POST /session (unregistered cwd) |
{error, code, boundWorkspace, workspaceCount, requestedWorkspace} ✅ |
POST /session (registered, untrusted) |
{error, code, workspaceCwd, workspaceId} ✅ |
POST /session/:id/load / /resume (bad cwd) |
{…, boundWorkspace, requestedWorkspace} ✅ |
GET /workspace/<unknown>/sessions |
{…, boundWorkspace, requestedWorkspace} ✅ |
GET /workspaces/<unknown>/settings (new) |
{error, code, workspaceCount} — stays minimal ✅ |
GET /workspaces/<untrusted>/settings (new) |
{error, code} — stays minimal ✅ |
On a single-workspace daemon the POST /session mismatch body matches the WorkspaceMismatchError example in qwen-serve-protocol.md verbatim, including the boundWorkspace / requestedWorkspace fields the doc tells clients to use. No doc change needed.
The 3 tests that were red are green: npx vitest run src/serve/server.test.ts -t "workspace_mismatch" → 3 passed. All five directly-affected suites → 728 passed, 0 failed.
And the restored fields have teeth. I checked that the intermediate commit d6c619b26 ("fix CI failure") had made CI pass by weakening the assertions to expect(res.body.boundWorkspace).toBeUndefined(); bb0eba482 reverts that and asserts the real values. So I re-applied the strip as a mutation:
| Mutation | tests failed |
|---|---|
re-strip boundWorkspace / requestedWorkspace from the legacy sendWorkspaceMismatch |
6 ✅ |
re-strip workspaceCwd / workspaceId from the legacy untrusted 403 |
2 ✅ |
A future regression here can't slip through silently again.
✅ Everything from the earlier rounds survived the main merge
The merge pulled in a lot (session-artifact persistence #6557, channel worker reload #6598, MCP prompt fix #6571, …), so I re-ran the whole matrix rather than assuming.
Runtime, on real daemons:
- Selector matrix: id / encoded cwd / trailing slash /
..segments / symlink alias → all200; unknown id, unknown path, relative, Windows drive, UNC → clean400 workspace_mismatch, no crash. - Single-decode still proven positively against the workspace whose cwd literally contains
%20: by-id200, by-encoded-cwd200, and the path a double-decode would produce →400. - Trust gate across all 40 plural routes on the untrusted workspace: 0 ungated.
- FS confinement per runtime:
../primary/p.txtand/etc/passwd→400 path_outside_workspacefrom both trusted and untrusted runtimes. - Session isolation:
GET /workspaces/<primary>/sessionsexcludes the secondary's session;POST /workspaces/<primary>/sessions/archivewith the secondary's id returnsnotFound. - Untrusted-primary asymmetry unchanged (10/12 read-only routes diverge) — now an explicitly documented, intentional contract. Accepted.
Mutation, against the grown suites (50 tests baseline):
| Mutation | tests failed |
|---|---|
remove the canonicalizeWorkspace fallback (regress the symlink fix) |
1 ✅ |
point plural sessions/archive + unarchive at boundWorkspace |
1 ✅ |
point plural sessions/delete at boundWorkspace |
1 ✅ |
On the 12 failures you may see locally
A full src/serve run in my sandbox shows 12 failures across fast-path.test.ts, workspace-memory.test.ts, workspace-agents.test.ts, and fs/workspace-file-system.test.ts. These are not yours. I swapped in origin/main's src/serve as a control and got the identical 12 failures — they're environment artifacts (root user, no real npm install), not a regression.
🟡 Two follow-ups, unchanged and non-blocking
Both files are byte-identical to the previous head, so my earlier evidence stands.
Telemetry route labels (server/telemetry.ts) still name routes that don't exist:
POST /workspaces/:ws/file/write → label "POST /workspace/file/write" (no such route)
POST /workspaces/:ws/sessions/archive → label "POST /workspace/sessions/archive" (real: POST /sessions/archive)
GET /workspaces/:ws/file → label "GET /file" (legacy GET /file emits nothing,
so that series counts only plural traffic)
Scope-rejection vocabulary (workspace-memory.ts:346) still reports a typo'd or omitted scope as "global scope not supported", and scope: "project" is accepted by plural /agents but rejected as a "global scope" by plural /memory and /permissions.
Nit: the untrusted 403 message is now the generic "Workspace is not trusted." on pre-existing routes too, where main embedded the path (Workspace "<cwd>" is not trusted.). The structured workspaceCwd / workspaceId fields carry that information, so only a client string-matching the message would notice. Fine as-is; mentioning it for the changelog.
Verdict
I'm satisfied. Selector resolution, trust gating, per-workspace routing, FS confinement, session isolation, and the error contract are all correct under real traffic, and I mutated each load-bearing behavior to confirm the tests would catch a regression. The main merge didn't erode anything.
LGTM once Test (ubuntu-latest) reports green — it was still running when I posted this, but the three tests it failed on now pass locally and the only other failures reproduce on main. The two follow-ups above are worth a separate PR.
中文版
复验报告 —— head 0c3cbe229
重新从源码构建，并针对合并了当前 main 的新 head，重跑了完整运行时套件，以及我前两份报告（#1、#2）中的全部变异测试。
阻塞项已解决，而且是以正确的方式解决的。 前两轮的每一个修复都在这次 merge 后存活了下来。就我这边而言，只要 CI 变绿即可合并——剩下两个低严重度的后续项，都不构成阻塞。
✅ 错误响应体的回归已修复（方案 (a)）
bb0eba482 在既有路由上恢复了字段，同时让新的 plural 路由面保持精简——正是我建议的切分方式。在真实的 4-workspace daemon 上实测：
| 路由 | 现在的响应体 |
|---|---|
POST /session（未注册 cwd） |
{error, code, boundWorkspace, workspaceCount, requestedWorkspace} ✅ |
POST /session（已注册但未受信） |
{error, code, workspaceCwd, workspaceId} ✅ |
POST /session/:id/load / /resume（错误 cwd） |
{…, boundWorkspace, requestedWorkspace} ✅ |
GET /workspace/<unknown>/sessions |
{…, boundWorkspace, requestedWorkspace} ✅ |
GET /workspaces/<unknown>/settings（新） |
{error, code, workspaceCount} —— 保持精简 ✅ |
GET /workspaces/<untrusted>/settings（新） |
{error, code} —— 保持精简 ✅ |
在单 workspace daemon 上，POST /session 的 mismatch 响应体与 qwen-serve-protocol.md 中 WorkspaceMismatchError 的示例逐字一致，包括文档让客户端使用的 boundWorkspace / requestedWorkspace 字段。文档无需改动。
之前红掉的 3 个测试已经变绿：npx vitest run src/serve/server.test.ts -t "workspace_mismatch" → 3 passed。五个直接受影响的测试文件 → 728 passed，0 failed。
而且恢复的字段是有牙齿的。 我查了一下，中间那个 commit d6c619b26（"fix CI failure"）是通过弱化断言（改成 expect(res.body.boundWorkspace).toBeUndefined()）让 CI 通过的；bb0eba482 把它改了回来，并断言了真实值。于是我把剥离作为变异重新施加了一次：
| 变异 | 失败测试数 |
|---|---|
在 legacy sendWorkspaceMismatch 上重新剥离 boundWorkspace / requestedWorkspace |
6 ✅ |
在 legacy 未受信 403 上重新剥离 workspaceCwd / workspaceId |
2 ✅ |
将来这里的回归不会再被静默放过。
✅ 前几轮的修复全部在 main merge 后存活
这次 merge 带进来的东西不少（session artifact 持久化 #6557、channel worker reload #6598、MCP prompt 修复 #6571 等），所以我没有想当然，而是把整个矩阵重跑了一遍。
运行时，真实 daemon 上：
- selector 矩阵：id / 编码 cwd / 结尾斜杠 /
..段 / 符号链接别名 → 全部200；未知 id、未知路径、相对路径、Windows 盘符、UNC → 干净的400 workspace_mismatch，不崩溃。 - 单次解码仍以那个 cwd 中真的含有
%20的 workspace 正面证明：按 id200，按编码 cwd200，而双重解码会产生的那个路径 →400。 - 未受信 workspace 上全部 40 条 plural 路由的信任门禁：零条未受门禁。
- 按 runtime 的文件系统边界：
../primary/p.txt和/etc/passwd在受信与未受信 runtime 上都返回400 path_outside_workspace。 - session 隔离：
GET /workspaces/<primary>/sessions不含 secondary 的 session；POST /workspaces/<primary>/sessions/archive传 secondary 的 id 返回notFound。 - 未受信 primary 的不对称保持不变（12 条只读路由中 10 条不一致）—— 现在是明确写进文档的、有意的契约。接受。
变异，针对扩充后的套件（基线 50 个测试）：
| 变异 | 失败测试数 |
|---|---|
移除 canonicalizeWorkspace 回退（回退符号链接修复） |
1 ✅ |
把 plural sessions/archive + unarchive 指向 boundWorkspace |
1 ✅ |
把 plural sessions/delete 指向 boundWorkspace |
1 ✅ |
关于你本地可能看到的 12 个失败
在我的沙箱里跑完整的 src/serve 会看到 12 个失败，分布在 fast-path.test.ts、workspace-memory.test.ts、workspace-agents.test.ts 和 fs/workspace-file-system.test.ts。这些不是你造成的。 我把 origin/main 的 src/serve 换进来作为对照组，得到完全相同的 12 个失败——它们是环境导致的（root 用户、没有真正的 npm install），不是回归。
🟡 两个后续项，未变更，不阻塞
这两个文件与上一个 head 逐字节相同，所以我之前的证据依然成立。
telemetry 路由标签（server/telemetry.ts）仍然指向不存在的路由：
POST /workspaces/:ws/file/write → 标签 "POST /workspace/file/write" （不存在此路由）
POST /workspaces/:ws/sessions/archive → 标签 "POST /workspace/sessions/archive" （真实的是 POST /sessions/archive）
GET /workspaces/:ws/file → 标签 "GET /file" （legacy GET /file 不发 telemetry，
所以该序列只统计 plural 流量）
scope 拒绝措辞（workspace-memory.ts:346）仍把拼错或省略的 scope 报成 "global scope not supported"；并且 scope: "project" 被 plural /agents 接受，却被 plural /memory 和 /permissions 当作 "global scope" 拒绝。
小点： 未受信 403 的消息在既有路由上现在也是通用的 "Workspace is not trusted."，而 main 会把路径嵌进去（Workspace "<cwd>" is not trusted.）。结构化的 workspaceCwd / workspaceId 字段已经承载了这个信息，所以只有对消息做字符串匹配的客户端才会察觉。保持现状即可；提一句是为了 changelog。
结论
我满意了。selector 解析、信任门禁、按 workspace 路由、文件系统边界、session 隔离，以及错误契约，在真实流量下都是正确的；我对每一个承重行为都做了变异，确认测试能抓到回归。main 的 merge 没有侵蚀任何东西。
只要 Test (ubuntu-latest) 变绿就 LGTM —— 我发这条时它还在跑，但它之前失败的那三个测试在本地已经通过，而其余失败在 main 上同样复现。上面两个后续项值得单独开一个 PR。
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Incremental review since commit 1c9ee5731. The author's fixes in bb0eba482 correctly address the session-route portion of the previous Critical findings — sendWorkspaceMismatch now includes boundWorkspace and requestedWorkspace, and sendUntrustedWorkspaceResponse now passes workspaceCwd/workspaceId in session routes. Tests are updated to match.
The shared helper sendWorkspaceMismatch in workspace-route-runtime.ts and requireTrustedWorkspaceRuntime still omit these fields for workspace-qualified routes (12+ callers). This inconsistency was discussed in the prior review and approved — no new inline comments needed.
Build passes. 230/230 collectable tests pass. The workspace-qualified-rest.test.ts collection failure is a pre-existing @xterm/headless environment issue, not PR-specific.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
What this PR does
This PR adds the Phase 3 workspace-qualified core REST contract under
/workspaces/:workspace/.... The selector resolves workspace id first and then a URL-encoded absolute cwd, including POSIX, Windows drive, and UNC-style absolute paths. The implementation keeps all legacy/workspace/...and workspace-less session routes on their existing primary-workspace semantics while adding plural routes for core file access, status surfaces, settings, permissions, trust, lifecycle, MCP control, tool toggles, memory, project-agent CRUD, and persisted session organization/batch operations.It also advertises the new
workspace_qualified_rest_corecapability, makes plural route telemetry resolve the selected workspace when possible, adds SDKworkspaceById()andworkspaceByCwd()helpers throughWorkspaceDaemonClient, and documents the selector, trust, scope, and out-of-scope boundaries.Why it's needed
This is the Phase 3 follow-up for issue #6378 on top of #6558. After persisted non-primary workspace session data exists, clients need a stable REST surface for operating on a registered secondary workspace directly instead of tunneling through primary-only daemon state or special-casing individual routes.
The trust policy is explicit: trust status/request and file reads remain available according to their existing policies, file writes rely on the selected workspace filesystem write guard, and other plural core routes require a trusted selected runtime and return
403 untrusted_workspacewhen the workspace is registered but untrusted.Reviewer Test Plan
How to verify
Confirm
/capabilitiesincludesworkspace_qualified_rest_core, then use a registered secondary workspace id and encoded cwd against representative plural routes such as/workspaces/:workspace/file,/workspaces/:workspace/mcp,/workspaces/:workspace/agents, and/workspaces/:workspace/sessions. Expected behavior is that id and cwd selectors route to the selected runtime, unknown selectors return400 workspace_mismatch, untrusted file reads follow the filesystem read policy, and trusted-only plural routes return403 untrusted_workspacefor untrusted workspaces.Confirm plural memory, agents, and permissions mutation routes reject
globaloruserscope with400 global_scope_not_supported_for_workspace_route, while legacy primary/workspace/...routes keep their existing global/user semantics.Local verification run:
cd packages/cli && npx vitest run src/serve/workspace-qualified-rest.test.ts src/serve/server/telemetry.test.ts;cd packages/sdk-typescript && npx vitest run test/unit/DaemonClient.test.ts;npm run build && npm run typecheck && npm run lint.Evidence (Before & After)
N/A. This is REST, SDK, telemetry, and documentation work with no TUI-visible change.
Tested on
Environment (optional)
Node v22.22.3, npm 10.9.8. Local verification used targeted Vitest files plus root
npm run build && npm run typecheck && npm run lint.Risk & Scope
Linked Issues
Refs #6378. Depends on #6558.
中文说明
What this PR does
本 PR 添加 Phase 3 的 workspace-qualified core REST 契约，路径为
/workspaces/:workspace/...。选择器先按 workspace id 解析，再按 URL 编码后的绝对 cwd 解析，并覆盖 POSIX、Windows drive、UNC 风格绝对路径。实现保持所有旧的/workspace/...和不带 workspace 的 session 路由沿用 primary workspace 语义，同时为核心文件访问、状态面、设置、权限、信任、生命周期、MCP 控制、工具开关、memory、project-agent CRUD、持久化 session 组织和批量操作添加 plural 路由。它还新增
workspace_qualified_rest_corecapability，让 plural 路由 telemetry 在可解析时归因到选中的 workspace，添加 SDKworkspaceById()和workspaceByCwd()helper 并通过WorkspaceDaemonClient暴露，同时补充 selector、trust、scope 和排除范围的协议文档。Why it's needed
这是 issue #6378 基于 #6558 的 Phase 3 后续工作。非 primary workspace 的持久化 session 数据可用之后，客户端需要一个稳定的 REST 面直接操作已注册的 secondary workspace，而不是通过 primary-only daemon 状态绕行或为单个路由做特殊处理。
信任策略是显式的：trust status/request 和文件读取沿用现有策略，文件写入依赖选中 workspace 的 filesystem write guard，其他 plural core 路由要求选中的 runtime 已受信任，并在 workspace 已注册但未受信任时返回
403 untrusted_workspace。Reviewer Test Plan
How to verify
确认
/capabilities包含workspace_qualified_rest_core，然后使用已注册 secondary workspace 的 id 和编码 cwd 请求代表性 plural 路由，例如/workspaces/:workspace/file、/workspaces/:workspace/mcp、/workspaces/:workspace/agents和/workspaces/:workspace/sessions。预期行为是 id 和 cwd selector 都会路由到选中的 runtime，未知 selector 返回400 workspace_mismatch，未受信任 workspace 的文件读取遵循 filesystem read policy，而 trust-only plural 路由对未受信任 workspace 返回403 untrusted_workspace。确认 plural memory、agents 和 permissions mutation 路由在请求
global或userscope 时返回400 global_scope_not_supported_for_workspace_route，同时 legacy primary/workspace/...路由保留现有 global/user 语义。本地验证命令：
cd packages/cli && npx vitest run src/serve/workspace-qualified-rest.test.ts src/serve/server/telemetry.test.ts；cd packages/sdk-typescript && npx vitest run test/unit/DaemonClient.test.ts；npm run build && npm run typecheck && npm run lint。Evidence (Before & After)
N/A。本 PR 是 REST、SDK、telemetry 和文档工作，没有 TUI 可见变化。
Tested on
Environment (optional)
Node v22.22.3，npm 10.9.8。本地验证使用了目标 Vitest 文件以及根目录
npm run build && npm run typecheck && npm run lint。Risk & Scope
Linked Issues
Refs #6378. Depends on #6558.