fix(serve): restore build by routing side-task rollback through the workspace session service - #8144
Merged
Merged
Conversation
…orkspace session service The side-task creation route still constructed a session service directly on two rollback paths after the surrounding module moved to the workspace runtime helper, leaving the identifier unbound and breaking the CLI build on main. Route both rollbacks through the workspace runtime helper so they honour the per-workspace runtime base dir like every other session-service use in this module. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the quick follow-up fix! Template looks good ✓ Problem: observed and clear — #7975 removed the Direction: straightforward build restoration, fully aligned. The replacement ( Size: not applicable — single file, 4 lines changed, no core paths. Approach: minimal and correct. Two call sites updated to the established pattern, nothing extra in the diff. Risk: no elevated risk signals. Moving on to code review. 🔍 中文说明感谢快速的后续修复！ 模板完整 ✓ 问题：已明确观测到——#7975 从 session 路由模块删除了 方向：直接的构建修复，完全对齐。替换方案（ 规模：不适用——单文件，4 行改动，未触及核心路径。 方案：最小且正确。两处调用点更新为既有模式，diff 中无多余改动。 风险：无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
Code ReviewIndependent proposal: given that #7975 migrated all session-service construction to Findings: no issues. The two rollback paths (generation-guard-closed and client-disconnected) now use CI Test EvidenceCI is still running on the head commit. Java daemon E2E checks are mostly green; the unit test suite and Serve A/B are in progress. No failures so far. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Not verified: unit test suite and Serve A/B results — still in progress at review time. 中文说明代码审查独立方案： 既然 #7975 已将所有 session service 构造迁移到 发现： 无问题。两处回滚路径（generation-guard 已关闭、客户端断连）现在使用 CI 测试证据CI 仍在运行。Java daemon E2E 检查大部分通过；单元测试和 Serve A/B 正在进行中。目前无失败。 未验证：单元测试和 Serve A/B 结果——审查时仍在运行。 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Confidence: 5/5 — textbook follow-up fix: two missed call sites, one-line-each replacement with the established helper, nothing else in the diff. This is exactly the kind of PR that should be frictionless. #7975 migrated session-service construction but left two rollback paths behind — the build broke, and this restores it with the minimal possible change. The replacement also delivers the workspace-scoped No reservations. Approval deferred only because CI is still running on the head commit. 中文说明置信度：5/5 —— 教科书式的后续修复：两处遗漏的调用点，各一行替换为既有辅助函数，diff 中无其他改动。 这正是应该零摩擦的 PR。#7975 迁移了 session service 构造但遗漏了两处回滚路径——构建失败，本 PR 以最小改动恢复。替换同时也提供了这些路径原本缺少的 workspace 级别 无保留意见。仅因 CI 仍在运行而延迟批准。 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
🩺 serve daemon A/BBuilt the PR base vs this PR head — Qwen Code · serve A/B |
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. |
yiliang114
approved these changes
Jul 30, 2026
qwen-code-ci-bot
approved these changes
Jul 30, 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 — CI landed green after the review. ✅
Collaborator
|
Released in v0.21.2. |
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 the two rollback paths in the side-task creation endpoint through the workspace runtime session service, instead of constructing a session service directly against the workspace directory.
Why it's needed
The daemon session maintenance writer isolation change moved every session-service construction in the session route module over to the workspace runtime helper and dropped the direct import, but two rollback paths in the side-task creation endpoint were left behind. That leaves an unbound identifier, so the CLI package no longer compiles and
mainis red — every job that installs dependencies fails during the build, including the Java daemon E2E job.Beyond restoring the build, this is the behavior the isolation change intended: both rollbacks delete an orphaned session after a failed side-task creation, and they were still writing through the default runtime base dir rather than the workspace's own, which is exactly the cross-workspace writer overlap that change set out to remove.
Reviewer Test Plan
How to verify
Check out
mainand run a build — it fails with two unresolved-name errors in the session route module and the CLI package build aborts. With this change the build and the full typecheck pass.The affected code runs when side-task creation succeeds but the request cannot be completed: either the daemon's generation guard has closed, or the client disconnected before the response could be written. In both cases the daemon kills the freshly created session and deletes its leftover session record. A reviewer can confirm the deletion now targets the workspace's own runtime directory rather than the process-default one.
Build and typecheck output:
Unit tests covering multi-workspace session routing, including the side-task endpoint:
Evidence (Before & After)
N/A — no user-visible or TUI change.
Tested on
Environment (optional)
Local build, typecheck, and unit tests only.
Risk & Scope
Linked Issues
Follow-up to #7975.
中文说明
这个 PR 做了什么
把 side-task 创建接口里两处回滚路径改为通过 workspace runtime 的 session service 获取实例，而不是直接针对工作区目录构造 session service。
为什么需要
守护进程 session 维护写入隔离的改动，把该路由模块里所有 session service 的构造都换成了 workspace runtime 辅助函数，并删掉了原来的直接导入，但 side-task 创建接口里的两处回滚路径被漏掉了。结果是留下了未绑定的标识符，CLI 包无法编译，
main变红——所有会安装依赖的 job 都在构建阶段失败，包括 Java daemon E2E job。除了修复构建，这也是隔离改动本来想要的行为：这两处回滚都是在 side-task 创建失败后删除遗留的 session，而它们仍然写到默认的 runtime 目录而非该工作区自己的目录，正是那次改动要消除的跨工作区写入重叠。
Reviewer Test Plan
如何验证
在
main上执行构建会失败，session 路由模块报两处标识符无法解析，CLI 包构建中止。应用本改动后，构建和完整 typecheck 均通过。受影响的代码在 side-task 创建成功但请求无法完成时执行：要么守护进程的 generation guard 已关闭，要么客户端在响应写出前断开。两种情况下守护进程都会杀掉刚创建的 session 并删除其遗留记录。审阅者可以确认删除操作现在指向该工作区自己的 runtime 目录，而不是进程默认目录。
构建与 typecheck 输出：
覆盖多工作区 session 路由（含 side-task 接口）的单元测试：
Evidence (Before & After)
N/A —— 无用户可见或 TUI 变更。
Tested on
Environment (optional)
仅本地构建、typecheck 与单元测试。
Risk & Scope
Linked Issues
#7975 的后续修复。