fix(serve): Route session continue, language, and artifacts by owner - #6833
Conversation
E2E test reportTested on macOS with Node.js v22.22.3, using a primary workspace plus a trusted secondary workspace and bearer authentication.
Automated results: build passed, typecheck passed, multi-workspace session tests passed 75/75, and serve regression tests passed 692/692. |
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with evidence. The PR documents that continue, language, artifact add, and artifact delete against trusted secondary sessions returned Direction: aligned. This is the P0 owner-routing portion of #6378, following the same incremental pattern as #6798. The approach — switching route wrappers to owner-aware variants — is consistent with previously merged work. Size: no core paths touched ( Approach: the scope is mostly minimal — the route handler changes are surgical wrapper swaps ( Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测到的 bug,有复现证据。PR 记录了在基线版本(Qwen Code 0.19.9)中,针对可信 secondary session 的 continue、language、artifact add 和 artifact delete 返回 方向:对齐。这是 #6378 的 P0 owner-routing 部分,遵循与 #6798 相同的增量模式。方案——将路由 wrapper 切换为 owner-aware 变体——与已合入的工作一致。 规模:未触及核心路径( 方案:范围基本精简——路由 handler 的改动是精确的 wrapper 替换( 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
1886969 to
7f1d780
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)为单个提交。 |
Code ReviewIndependent proposal (before reading the diff): introduce owner-aware route wrappers for the four primary-only session mutations, using the same Comparison with the diff: the PR's approach matches this proposal. The route handler changes are minimal wrapper swaps, the error handling (403/404/500) is consistent with other owner-routed routes, and the SDK No critical blockers found. The implementation is correct and well-structured:
One minor concern: the AGENTS.md edit (clarifying TestingBuild: passed ✓
Test coverage pins owner dispatch, zero calls to non-owning bridges, trust/ambiguity failures, Real-scenario tmux testing is not feasible in this environment — the scenario requires a daemon with primary + trusted secondary workspace and live session state, which cannot be set up in this CI runner. The author's own E2E report (baseline vs local bundle) covers the before/after. 中文说明代码审查独立方案(阅读 diff 前):为四个 primary-only 的 session mutation 引入 owner-aware 路由 wrapper,使用 stacking 依赖(#6826)中建立的 与 diff 的对比: PR 的方案与独立方案一致。路由 handler 改动是最小化的 wrapper 替换,错误处理(403/404/500)与其他 owner-routed 路由一致,SDK 未发现关键阻塞问题。 实现正确且结构良好:
一个小问题: AGENTS.md 的修改(澄清 测试Build: 通过 ✓
测试覆盖固定了 owner dispatch、非 owning bridge 零调用、trust/ambiguity 失败、 真实场景 tmux 测试在当前环境不可行——该场景需要带有 primary + 可信 secondary workspace 的 daemon 和 live session 状态,无法在此 CI runner 中设置。作者自己的 E2E 报告(基线 vs 本地构建)覆盖了 before/after。 — Qwen Code · qwen3.7-max |
|
This is a solid, well-tested piece of incremental multi-workspace routing work. The problem is real (secondary sessions were completely blocked from continue, language, and artifact operations), the before/after reproduction is documented, and the approach is the minimal change needed — wrapper swaps in session.ts plus a clean The implementation matches what I'd have proposed independently. The test coverage is thorough: 75 multi-workspace session tests pinning owner dispatch, trust boundaries, ambiguity handling, and validation ordering; 692 serve regression tests confirming no breakage; plus SDK tests verifying the REST fallback for rewind with ACP transport. Two minor hygiene items (not blocking):
Approving. ✅ 中文说明这是一个扎实、测试充分的增量多 workspace 路由改动。问题是真实的(secondary session 完全无法使用 continue、language 和 artifact 操作),有 before/after 复现记录,方案也是最小必要改动——session.ts 中的 wrapper 替换加上 SDK transport 接口中简洁的 实现与我独立提出的方案一致。测试覆盖全面:75 个多 workspace session 测试固定了 owner dispatch、trust boundary、ambiguity 处理和校验顺序;692 个 serve 回归测试确认无破坏;SDK 测试验证了 ACP transport 下 rewind 的 REST 回退。 两个非阻塞的小建议:
批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
yiliang114
left a comment
There was a problem hiding this comment.
LGTM — routing through withOwnerMutableSession is the right pattern, auth gates are preserved, single-workspace deployments are unaffected.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
4a67ee8
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
✅ Maintainer local verification — merge referenceI built and ran this PR from a real local build in an isolated worktree at the PR head ( What the change doesFour legacy REST mutations — Results
The five new tests assert exactly the contract in the PR's reviewer plan: owner dispatch to the secondary bridge (never the primary); Load-bearing controlled experimentTo prove the tests actually exercise the fix (and aren't vacuous), I reverted only the 4-route source change back to its pre-PR state while keeping the new tests, then re-ran them:
This is the ideal signature of a tightly-scoped change: the new-behavior assertions are load-bearing, the invariants are untouched. Restoring the fix returns the tree to clean and all five back to green. Notes for merge
Environment: macOS (darwin 24.6.0) · Node v22.23.1 · npm 10.9.8 · LGTM from a verification standpoint — routing behavior, trust boundaries, and non-regression are all confirmed. ✅ 🇨🇳 中文版本(点击展开)✅ 维护者本地验证 —— 合并参考我在一个隔离的 worktree 中,从 PR head( 该改动做了什么四个 legacy REST mutation —— 验证结果
这 5 个新测试正好验证了 PR reviewer 计划中的约束:owner dispatch 到 secondary bridge(绝不落到 primary);每次 载荷性对照实验(load-bearing)为证明这些测试确实覆盖了本次修复(而非空测试),我只把这 4 个路由的源码改动回退到 PR 之前的状态、保留新测试,然后重新运行:
这正是一个范围收敛良好的改动应有的特征:新行为断言是载荷性的,不变量则未被触碰。恢复修复后,工作树变干净,5 个测试全部恢复为绿色。 合并注意事项
环境: macOS(darwin 24.6.0)· Node v22.23.1 · npm 10.9.8 · 从验证角度 LGTM —— 路由行为、信任边界、非回归均已确认。✅ Verification run in an isolated worktree; evidence images hosted on branch |



What this PR does
This PR routes the legacy REST mutations for continuing a session, changing its language, adding an artifact, and deleting an artifact through the runtime that owns the live session. It preserves the existing archive lease, request validation, authentication gates, bridge error mapping, request and response shapes, and the requirement that artifact mutations target a live session. The protocol documentation now lists these routes as owner-routed, while the historical Phase 2a design notes that its original primary-only limitation has been superseded.
Why it's needed
Trusted secondary-workspace sessions can already be resolved to their owning runtime, but these four mutations still used the primary-only guard and returned
non_primary_session_route_not_supported. That prevented clients from continuing interrupted turns, changing output language, or managing artifacts for an otherwise valid secondary session. This is the P0 owner-routing portion of #6378 and follows the same incremental routing pattern as #6798.Reviewer Test Plan
How to verify
Start a daemon with a primary workspace and one trusted secondary workspace, create a live session in the secondary workspace, and complete at least one turn before testing continue. Confirm that language changes, artifact add/delete, and continue all succeed against the secondary session and invoke only the secondary runtime. Confirm that continue preserves the optional client identity and produces a non-empty unique prompt identifier; language preserves the requested language, output-language synchronization flag, and optional client identity; artifact mutations preserve the artifact payload or artifact identifier and required client identity. Also confirm that primary and single-workspace sessions retain their existing behavior, untrusted owners return
403 untrusted_workspace, missing owners return404 session_not_foundwithout falling back to primary, ambiguous ownership returns500 ambiguous_session_owner, continue and artifact mutations still require authentication, and language keeps its existing non-strict authentication behavior.Automated verification passed on the latest
main: the multi-workspace session suite passed 76/76, the serve regression suite passed 693/693, and the repository build and typecheck completed successfully. Formatting and lint checks for the changed files also passed.Evidence (Before & After)
Before: with global Qwen Code 0.19.9, all four requests against a trusted secondary session returned
400 non_primary_session_route_not_supported.After: with the local bundle, language change and artifact add/delete returned
200; continue returned200withaccepted: trueand a new prompt identifier after an interrupted turn. Requests were handled by the secondary runtime rather than the primary runtime.Tested on
Environment (optional)
macOS, Node.js v22.22.3, npm 10.9.8, local build and bundle, sandbox disabled.
Risk & Scope
cdownership, export, runtime removal, and new capabilities remain out of scope. The branch is rebased ontomainafter feat(serve): support multi-workspace rewind and shell #6826 merged.Linked Issues
Refs #6378
Related: #6826
中文说明
本 PR 做了什么
本 PR 将继续会话、修改会话语言、添加 artifact 和删除 artifact 这四个 legacy REST mutation 路由到拥有该 live session 的 runtime。现有的 archive lease、请求校验、鉴权 gate、bridge 错误映射、请求与响应结构以及 artifact mutation 必须针对 live session 的约束均保持不变。协议文档现已将这些路由列为 owner-routed,历史 Phase 2a 设计文档也注明其最初的 primary-only 限制已被后续实现取代。
为什么需要它
可信 secondary workspace 中的 session 已经能够解析到其 owning runtime,但这四个 mutation 仍使用 primary-only guard,并返回
non_primary_session_route_not_supported。这导致客户端无法为一个原本有效的 secondary session 继续被中断的 turn、修改输出语言或管理 artifact。本 PR 是 #6378 中 P0 owner-routing 的部分,并遵循与 #6798 相同的增量路由模式。Reviewer 测试计划
如何验证
启动一个包含 primary workspace 和一个可信 secondary workspace 的 daemon,在 secondary workspace 中创建 live session,并在测试 continue 前至少完成一个 turn。确认 language change、artifact add/delete 和 continue 均能针对 secondary session 成功,并且只调用 secondary runtime。确认 continue 保留可选 client identity 并生成非空且唯一的 prompt identifier;language 保留请求的 language、output-language synchronization flag 和可选 client identity;artifact mutation 保留 artifact payload 或 artifact identifier 以及必需的 client identity。还需确认 primary 和 single-workspace session 保持现有行为,untrusted owner 返回
403 untrusted_workspace,missing owner 返回404 session_not_found且不 fallback 到 primary,ambiguous ownership 返回500 ambiguous_session_owner,continue 和 artifact mutation 仍要求鉴权,而 language 保持现有的非 strict 鉴权行为。在最新
main上自动化验证已通过:multi-workspace session 测试 76/76 通过,serve 回归测试 693/693 通过,仓库 build 和 typecheck 成功完成。变更文件的 formatting 和 lint 检查也已通过。证据(修改前与修改后)
修改前:使用全局 Qwen Code 0.19.9 时,针对可信 secondary session 的四个请求均返回
400 non_primary_session_route_not_supported。修改后:使用本地 bundle 时,language change 和 artifact add/delete 返回
200;在 turn 被中断后,continue 返回200、accepted: true和新的 prompt identifier。请求由 secondary runtime 而不是 primary runtime 处理。测试平台
环境(可选)
macOS、Node.js v22.22.3、npm 10.9.8、本地 build 和 bundle、禁用 sandbox。
风险与范围
cdownership、export、runtime removal 和新增 capability 仍不在范围内。feat(serve): support multi-workspace rewind and shell #6826 合入后,本分支现已 rebase 到main。关联 Issue
Refs #6378
Related: #6826