fix(serve): route session actions to the owning workspace - #6798
Merged
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
Author
E2E report: secondary live-session owner actionsThe global CLI baseline (0.19.9) was run first with two isolated trusted workspaces, bearer authentication, and a disposable runtime. All six authenticated secondary requests returned HTTP 400 with The post-fix local bundle was started on a different port and runtime directory with two trusted disposable workspaces. The local daemon was stopped and the isolated runtime/port were released. Real WebShell interaction and before/after screenshots were not run in this headless environment; active-turn/model-backed behaviors, running-task stopping, and active-goal clearing remain covered by the existing ACP/bridge tests and the deterministic route tests. |
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
|
Thanks for the PR! Template looks good ✓ Problem: Observed bug with concrete evidence. The global CLI 0.19.9 baseline returns HTTP 400 Direction: Aligned. This is a targeted fix within the multi-workspace daemon RFC scope. Secondary workspace sessions resolve correctly but then hit the Phase 2a primary-only guard — a clear gap that this PR closes by routing through the owning runtime. Size: Not applicable — no core module paths touched. All changes are in Approach: The scope is minimal and correct. The PR reuses the existing Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题： 已观测到的 bug，有具体证据。global CLI 0.19.9 基线对全部六个带认证的 secondary session 路由返回 HTTP 400 方向： 对齐。这是 multi-workspace daemon RFC 范围内的针对性修复。Secondary workspace session 能正确解析但在 Phase 2a primary-only guard 处被拦截——本 PR 通过路由到 owning runtime 修复了这个缺口。 规模： 不适用——未触及核心模块路径。所有改动在 方案： 范围最小化且正确。PR 复用了已有的 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewThe production change is mechanical and correct: six route handlers swap The No critical bugs, no security issues, no AGENTS.md violations found. The test infrastructure adds tracking arrays to the fake bridges and covers all six routes with secondary routing, primary preservation, auth enforcement, input validation, and untrusted workspace rejection — comprehensive coverage. Test ResultsUnit Tests (worktree)All 753 tests pass, including the 5 new multi-workspace owner-local action tests and 4 new BTW-specific server tests. CIDaemon Smoke Test (single-workspace, PR branch built and running)All six routes are registered and routing through the owner-resolution path (returning session-not-found 404 rather than the pre-fix 中文说明代码审查生产代码改动是机械性的且正确：六个路由处理函数从
未发现关键 bug、安全问题或 AGENTS.md 违规。测试基础设施为 fake bridge 添加了跟踪数组，覆盖了全部六个路由的 secondary 路由、primary 保持、鉴权强制、输入校验和不可信 workspace 拒绝——覆盖全面。 测试结果单元测试（worktree）全部 753 个测试通过，包括 5 个新的 multi-workspace owner-local action 测试和 4 个新的 BTW server 测试。 CIubuntu 测试和 WebShell E2E Smoke 均通过。 Daemon Smoke Test单 workspace daemon 确认全部六个路由已注册并通过 owner-resolution 路径响应（返回 session-not-found 404，而非修复前的 — Qwen Code · qwen3.7-max |
Collaborator
ReflectionThis is a clean, focused fix. The problem is real (confirmed by baseline CLI evidence, unit tests, and daemon smoke test) and the solution is minimal — six lines changed from My independent proposal before reading the diff was identical to what the PR delivers: use the existing owner-routing helper for these six routes, add test coverage for secondary/primary/auth/validation/untrusted scenarios. The PR implements exactly that, plus adds docs for the new routing rule in the protocol specification. After seeing it run: the unit tests (753/753 pass), CI (all green), and daemon smoke test (all six routes routing through the owner-resolution path) confirm the PR delivers on its promise. If I had to maintain this in six months, I'd thank the author — the pattern is consistent with the existing multi-workspace routing infrastructure, the test coverage is comprehensive, and the code change is small enough to reason about quickly. Verdict: approve. ✅ 中文说明干净的针对性修复。问题是真实的（基线 CLI 证据、单元测试和 daemon smoke test 均确认），方案是最小化的——六个路由从 独立方案与 PR 完全一致：使用已有的 owner-routing helper，添加 secondary/primary/auth/validation/untrusted 场景的测试覆盖。 运行结果确认：单元测试 753/753 通过，CI 全绿，daemon smoke test 确认全部六个路由通过 owner-resolution 路径响应。 六个月后维护这段代码：会感谢作者——模式与已有的 multi-workspace 路由基础设施一致，测试覆盖全面，代码改动小且易理解。 结论：批准。 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
previously approved these changes
Jul 13, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Collaborator
SummarySwitches six workspace-less live-session REST actions — session Confidence: 3/5 — promising and low-risk, but hold: it is a draft with no maintainer review yet. The fix reuses the same owner-resolution helper sequenceDiagram
participant Client as Client
participant Daemon as Daemon
participant Reg as Registry
participant Arch as ArchiveCoordinator
participant Bridge as OwnerBridge
Client->>Daemon: POST session action with bearer token
Daemon->>Daemon: auth check, 401 if missing
Daemon->>Reg: withOwnerMutableSession resolves the session owner
Reg->>Daemon: owning runtime, or 404 or 403 untrusted or 500 ambiguous
Daemon->>Arch: acquire shared archive lease
Daemon->>Bridge: runtime.bridge action, update metadata or cancel task
Bridge->>Daemon: action result
Daemon->>Client: 200 with result
Files changed (4 files)
Gate
ReviewNo blocking issues. The routing correctness is inherited from the same already-tested VerdictClean incremental fix — but it is a draft with the author flagging real-WebShell / active-turn paths as not yet exercised, and there is no maintainer review yet. Hold for the author to mark it ready; the one thing to confirm is that the REST-vs-ACP transport divergence is intentional for now. — Qwen Code · qwen3.7-max Reviewed at 中文说明摘要:把六个无 workspace 参数的 live-session REST 动作( 置信度:3/5 —— 有前景、低风险,但暂缓:它是 draft 且尚无维护者评审。 复用了 门禁:模板 ✓;问题真实(作者 before/after E2E 显示六个次级路由从 400 变 200,Refs #6378);方向对齐;范围最小且复用优先。 评审:无阻塞项。路由正确性继承自 结论:干净的增量修复——但是 draft,作者自述真机 WebShell / active-turn 路径尚未验证,且无维护者评审。等作者标记 ready;唯一要确认的是 REST 与 ACP 传输暂时的分歧是否有意。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
approved these changes
Jul 13, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
wenshao
approved these changes
Jul 13, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
LGTM — clean, mechanical migration of 6 route handlers from withMutableSession to withOwnerMutableSession. All handler bodies preserved verbatim, bridge calls correctly routed through runtime.bridge, auth and trust enforcement unchanged. Tests thoroughly cover happy path, primary passthrough, auth (401), validation (400), and untrusted workspace (403).
— qwen3.7-max via Qwen Code /review
wenshao
reviewed
Jul 13, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
Comment on lines
2698
to
+2700
| '/session/:id/recap', | ||
| mutate(), | ||
| withMutableSession( | ||
| withOwnerMutableSession( |
Collaborator
There was a problem hiding this comment.
[Suggestion] recap, btw, and mid-turn-message use non-strict mutate() — no bearer auth required. Previously these were blocked for non-primary sessions by withMutableSession's primary-only guard. Now that withOwnerMutableSession dispatches to secondary workspace bridges, the blast radius of unauthenticated access widens: /recap and /btw trigger LLM model calls (compute cost), and /mid-turn-message injects a message into a running agent turn on a trusted secondary workspace. The trust check correctly blocks untrusted workspaces, so this is not a privilege escalation — but any local process on a token-less daemon can now invoke model work against any trusted workspace's sessions.
Consider promoting these three routes to mutate({ strict: true }) to match metadata, tasks/:taskId/cancel, and goal/clear, or document the deliberate design choice.
— qwen3.7-max via Qwen Code /review
Comment on lines
+2172
to
+2174
| withOwnerMutableSession( | ||
| 'PATCH /session/:id/metadata', | ||
| (req, res, sessionId, runtime) => { |
Collaborator
There was a problem hiding this comment.
[Suggestion] withOwnerMutableSession handlers MUST use runtime.bridge (the resolved owner's bridge), not the closure-scoped bridge (always primary). Nothing in the type system prevents accidentally capturing bridge — a handler that does so would silently operate on the primary workspace's bridge for a secondary session, and the bug would only surface in multi-workspace mode. A future maintainer adding a 7th handler is most likely to copy an existing one; copying from a withMutableSession handler and mechanically switching the wrapper without updating bridge → runtime.bridge would produce a silent misroute.
A one-line comment on the withOwnerMutableSession definition would defuse this landmine:
// IMPORTANT: handler must use `runtime.bridge`, not the captured `bridge` — the latter is always primary.— qwen3.7-max via Qwen Code /review
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
Routes six existing workspace-less live-session REST actions to the trusted workspace runtime that owns the session identified by
sessionId: metadata update, recap, BTW, mid-turn message, task cancellation, and goal clear. The existing URLs, request and response contracts, authentication, capability advertisement, archive lease, validation, client identity forwarding, and error mapping remain unchanged.Why it's needed
In a multi-workspace daemon, these actions resolved a secondary live session correctly but then stopped at the Phase 2a primary-only guard, so normal WebShell and SDK actions such as rename and recap failed with
non_primary_session_route_not_supported. Dispatching through the owning runtime makes trusted secondary sessions behave like primary sessions without adding a workspace selector or changing clients.Reviewer Test Plan
How to verify
Run the multi-workspace tests and confirm that all six authenticated secondary requests reach the secondary fake bridge with the exact session id, trimmed values, client context, and BTW abort signal, while primary call arrays stay empty. Confirm primary routing, strict unauthenticated 401, invalid-input validation, and untrusted-secondary 403 behavior. Run the BTW server tests and the existing server regression suite; the expected bridge-error log in the BTW mapping test is intentional. A local bundled daemon smoke test with two trusted disposable workspaces should return metadata 200, recap 200 with nullable recap, idle mid-turn
{ accepted: false }, nonexistent task cancellation{ cancelled: false }, absent goal{ cleared: false }, and BTW validation 400 rather than the Phase 2a guard.Evidence (Before & After)
Before: the global 0.19.9 daemon returned HTTP 400
non_primary_session_route_not_supportedfor all six authenticated secondary routes. After: the local bundle returned metadata 200, recap 200, mid-turn 200, task-cancel 200, goal-clear 200, and BTW invalid-input 400 from route validation; the secondary workspace was shown in capabilities and the created session response. WebShell screenshot capture was not run in the headless environment; deterministic SSE/UI coverage remains in the existing tests.Tested on
Environment (optional)
Node.js v24.12.0 on macOS; local
npm run buildandnpm run bundle; isolated trusted workspaces, runtime directory, port, and bearer token for daemon smoke testing; global baseline CLI 0.19.9.Risk & Scope
Linked Issues
Refs #6378
中文说明
What this PR does
将六个现有的无 workspace 参数 live-session REST 操作路由到拥有该
sessionId的可信 workspace runtime：metadata 更新、recap、BTW、mid-turn 消息、任务取消和 goal 清除。现有 URL、请求和响应协议、鉴权、capability 广告、archive lease、参数校验、client identity 传递及错误映射均保持不变。Why it's needed
在 multi-workspace daemon 中，这些操作能够正确解析 secondary live session，却随后被 Phase 2a primary-only guard 拦截，因此重命名、recap 等 WebShell 和 SDK 操作会失败并返回
non_primary_session_route_not_supported。通过 owning runtime 分发后，可信 secondary session 可与 primary session 一致工作，不增加 workspace selector，也不改变客户端。Reviewer Test Plan
How to verify
运行 multi-workspace 测试，确认六个带认证的 secondary 请求都到达 secondary fake bridge，并收到准确的 session id、trim 后值、client context 和 BTW abort signal，同时 primary 调用数组为空。确认 primary 路由、严格鉴权的未认证 401、无效输入校验和不可信 secondary 的 403 行为。运行 BTW server 测试及现有 server 回归套件；BTW 错误映射测试中的 bridge-error 日志是有意触发的。使用两个可信临时 workspace 启动本地 bundle daemon 做 smoke test，应得到 metadata 200、recap 200 且 recap 可为空、idle mid-turn
{ accepted: false }、不存在任务的取消{ cancelled: false }、无 goal 的{ cleared: false }，以及来自路由校验的 BTW 400，而不是 Phase 2a guard。Evidence (Before & After)
修复前：global 0.19.9 daemon 的六个带认证 secondary 路由全部返回 HTTP 400
non_primary_session_route_not_supported。修复后：local bundle 返回 metadata 200、recap 200、mid-turn 200、task-cancel 200、goal-clear 200，以及由路由校验返回的 BTW 无效输入 400；capabilities 显示了 secondary workspace，创建 session 的响应也显示了该 workspace。当前无头环境未执行 WebShell 截图；确定性的 SSE/UI 覆盖仍由现有测试提供。Tested on
Environment (optional)
macOS 上的 Node.js v24.12.0；本地执行
npm run build和npm run bundle；daemon smoke test 使用隔离的可信 workspace、runtime 目录、端口和 bearer token；global baseline CLI 为 0.19.9。Risk & Scope
Linked Issues
Refs #6378