Skip to content

fix(chat): preserve attached session ownership - #4133

Merged
huangruiteng merged 23 commits into
loopx-project:mainfrom
Duang777:codex/fix-attached-session-lifecycle
Sep 13, 2026
Merged

huangruiteng merged 23 commits into
loopx-project:mainfrom
Duang777:codex/fix-attached-session-lifecycle

Conversation

@Duang777

@Duang777 Duang777 commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • reuse attached Sessions without requiring a local managed endpoint
  • reject interruption of host-owned active Turns until an interrupt bridge exists
  • prevent a failed old worker from removing a replacement adapter or marking its Session stale

Validation

  • python -m pytest -q tests/test_attached_session_broker.py tests/test_chat_session_active_turn.py — 50 passed
  • related Chat suite — 108 passed
  • ruff check — passed
  • loopx canary premerge --from-git-diff --git-diff-base origin/main --tier standard — passed
  • full non-Stage2C suite — 6903 passed, 33 existing failures around ~/.codex/loopx/authority-transition; representative failure reproduced unchanged on origin/main

@Duang777
Duang777 force-pushed the codex/fix-attached-session-lifecycle branch 2 times, most recently from dc26a10 to 805d7ad Compare September 9, 2026 08:09
@Duang777

Duang777 commented Sep 9, 2026 •

Copy link
Copy Markdown
Collaborator Author

@huangruiteng 当前 head e918856 已同步 main,并带入同一已验证的 Node 26 fence-error 稳定化修复。验证:业务回归 50/50、fence parity 22/22、Ruff 与 diff check 通过;新 CI 已触发,请按此 head 进行独立技术复审。

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
@Duang777
Duang777 force-pushed the codex/fix-attached-session-lifecycle branch from c9037c1 to 70e1ee7 Compare September 10, 2026 06:10
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

评审绑定 exact head:e91885654df8e6be3e96d26f3c3222f19e8c8e80。

动机

这个 PR 修复三类同属 Session/adapter ownership 的问题:已有 attached Session 不应因为本机没有对应 managed endpoint 而无法 resume;LoopX 在没有 host interrupt bridge 时不能把 host-owned active Turn 伪装成本地已中断;旧 adapter 的迟到失败也不能删除新 replacement adapter、再把正在工作的 Session 标成 stale。这三个问题都会破坏同一 Session 的所有权连续性。

改动思路

改动复用现有 session_mode=attached_host、active_turn_id、file-backed ChatSessionStore 与 adapter map。open_session 先查 exact route 的 attached latest Session,只有新建或 managed resume 才校验/启动本地 endpoint;interrupt_turn 在任何本地 transition 前拒绝 host-owned active Turn;_run_turn 的 unhealthy cleanup 增加 adapter object identity 条件。这些 decision boundary 都放在现有 owner,方向正确。

具体改动

关键代码讲解

  1. ChatRuntimeController.open_session 将 attached resume 前移:同一 Goal/Agent/channel 的已授权 attached Session 直接复用,不创建 adapter。真实 file store counterfactual 中,base 返回 unknown Agent endpoint,本 head 返回相同 Session id 且 adapters == {}。
  2. _run_turn 只在 self.adapters[session_id] is adapter 时执行 pop+stale,阻止旧 worker 在 _fail_turn 后覆盖 replacement 的 busy/active ownership。
  3. interrupt_turn 对 attached_host + matching active_turn_id 抛出 attached_session_interrupt_unavailable 与 host_tool_gate,并让 Turn 维持 running、Session 维持 busy;queued 与 terminal 分支仍沿用原逻辑。
  4. 两组测试覆盖 attached broker 与 active-turn race,exact head 为 50 passed。writer-fence normalization 已经随 #4095 进入 current main,rebase 时应自然消失。

不过新的 typed failure 没有走完公共路径:ChatRequestHandler._interrupt_turn 捕获 CodexChatAgentError 时只传了 status=424 和 gate,没有把 exc.error_code 传给 _send_error。我直接执行 handler,观察到 response 参数确实缺少 attached_session_interrupt_unavailable;客户端只能解析自然语言或 gate,无法稳定区分这个新 contract。

对主干的风险

有两个 P1 阻塞项。第一,补齐上述 HTTP error_code 传播,并增加 endpoint-level 回归,验证 424、exact code、gate,以及 store 中 Turn/Session ownership 不变。第二,分支已明显落后:GitHub 为 CONFLICTING/DIRTY,本地 merge-tree 与当前 origin/main 在 loopx/chat_runtime.py 冲突。main 已在相同 open/run 路径加入 manager context、ACP execution mode 与 handoff 语义,不能机械挑一边;需要 rebase 后重新做完整行为判断。

当前 head 的 50 个 focused tests、Ruff、diff check 和历史远端 checks 都通过,说明三项局部修复是有价值的;但这些证据没有覆盖公共 adapter 的机器语义,也不能证明与当前 main 的组合。最强反例正是“controller 测试全绿,同时 HTTP 丢 code 且 merge 不成立”。

我的整体评价

结论是 REQUEST_CHANGES。核心 ownership 判断基本正确且范围合适,但公共错误 contract 还差一跳,当前 exact head 又无法与主干组合。请先 rebase,按当前 manager/ACP 生命周期语义解决冲突;随后补 error_code=exc.error_code 与 endpoint regression,并重跑 focused/related Chat suite 和 clean merge-tree。完成后应重新绑定新 head 全量复审。

English verdict: REQUEST_CHANGES — exact head e91885654df8e6be3e96d26f3c3222f19e8c8e80 has sound attached-session ownership fixes and 50 focused tests pass, but the public interrupt endpoint drops attached_session_interrupt_unavailable, and the branch conflicts with current main in chat_runtime.py; rebase, propagate/test the code, and revalidate the composed Chat lifecycle.

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
…ssion-lifecycle

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>

# Conflicts:
#	loopx/chat_runtime.py
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
@Duang777

Copy link
Copy Markdown
Collaborator Author

Addressed the two blockers from the exact-head review in 4246bc1294f3995f37f026661bb04153e28c6b83:

  • merged current main (f41beefaeb930b61a29c2081781538a9df2d27a4) and resolved chat_runtime.py by preserving the manager workspace / ACP execution-mode / refreshed-session behavior while retaining attached-session resume before local endpoint validation;
  • propagated CodexChatAgentError.error_code through ChatRequestHandler._interrupt_turn;
  • added an endpoint-level regression asserting HTTP 424, attached_session_interrupt_unavailable, the host_tool_gate, and unchanged running/busy ownership state;
  • the obsolete writer-fence delta disappeared because main already contains the normalized behavior.

Validation: 97 related Chat/attached/manager tests passed, Ruff and diff checks passed, the clean merge-tree check passed, and the quick premerge canary passed. New GitHub CI is running; ready for exact-head re-review after it is green.

…ssion-lifecycle

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
@Duang777

Duang777 commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator Author

Follow-up: main advanced twice while CI was running, so both updates were merged and revalidated rather than inheriting stale green checks. The current exact head is df950ead8292296e1004074197125fedbd5ed5f4 on current main 60b77406f.

The reviewed attached-session fixes remain intact. The latest manager-context change in #4218 auto-merged cleanly; 125 related attached/Chat/manager-inspection/goal-portfolio tests passed, along with Ruff, diff check, and a clean merge-tree. Fresh CI is running—please bind re-review to this head.

…ssion-lifecycle

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

动机

本次复审绑定 exact head df950ead8453d61c0f3f8763bd2f276e91d7fc81。这组改动要解决 attached-host Session 与本地 adapter 生命周期不一致时的三个真实问题:重新打开已附着 Session 不应要求本机 endpoint;晚到的旧 adapter 清理不应删除后来接管的 adapter;活跃 Turn 仍归 host 所有时,本地 interrupt 不应抢占其状态机。

上次在旧 head 上提出的两个 blocker 也已经闭环:HTTP 层现在保留 attached_session_interrupt_unavailable 的 typed error code,而当前分支已经吸收主干并可与最新 origin/main 干净合成。

改动思路

实现继续沿用既有所有权边界,没有新增第二套协调器:Session/Turn 决策留在 ChatRuntimeController,HTTP handler 只把 typed failure 适配成公开错误响应。这样既修正 attached-host 路径,也保持 managed/new Session 的 endpoint 校验、adapter 启动和原有中断语义不变。

具体改动

  • open_session 先识别并返回已有的 attached_host Session,只有创建或打开本地托管 Session 时才要求 endpoint。
  • _run_turn 的晚到 cleanup 仅在映射仍指向当前 adapter 实例时才删除/标记,避免旧任务清掉后来接管的实例。
  • interrupt_turn 在本地状态迁移前识别 host-owned active Turn,返回明确的 attached_session_interrupt_unavailable 与 host_tool_gate。
  • HTTP adapter 将该错误码原样传入错误响应,不再退化为通用错误。
  • 回归覆盖包含 attached Session 重开、adapter 竞态、host-owned interrupt、HTTP 424/error-code/gate,以及失败后 running/busy 所有权保持不变。

关键代码讲解

最关键的是“先判所有权,再产生副作用”:interrupt_turn 在任何本地 transition 之前拒绝 host-owned active Turn;cleanup 又通过 adapter identity 比较保护新 owner。这两处共同避免了本地运行时在不知道远端执行进度时篡改权威状态。HTTP 层只传递 controller 的 typed 结论,未重新实现业务判断。

对主干的风险

我完整检查了 4 个变更文件(生产代码与测试共 +214/-10),并在该 exact head 上运行两个相关测试文件:51 passed in 6.32s;Ruff 与 git diff --check 通过。最新主干在这 4 个文件上没有额外改动,git merge-tree --write-tree origin/main <head> 成功;远端 DCO、依赖检查、构建、Python shards、平台测试、pytest、Sonar 与 merge gate 均为成功或预期跳过。

剩余风险较低:HTTP 回归走真实 handler 方法与文件存储,但没有启动网络 socket;真实外部 host interrupt 也刻意不在本 PR 支持范围内。相关 future-facing pass 已做:现有 controller、store 与 HTTP adapter 是足够清晰的 owner,现在引入新的通用协调抽象反而会扩大范围。

我的整体评价

这次 exact head 已准确修复旧 review 指出的错误码丢失和主干集成问题,也用所有权与竞态回归覆盖了最容易产生细微语义漂移的路径。实现边界合理、验证充分,我批准合入。

English verdict: APPROVE — exact head df950ea closes the prior typed-error and main-integration blockers while preserving Session, Turn, and adapter ownership boundaries.

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

动机

这个 PR 修复 attached-host Session 与本地 adapter 生命周期不一致时的三类 ownership 问题:重新打开已附着 Session 不应依赖本机 endpoint;没有 host interrupt bridge 时,LoopX 不能把 host-owned active Turn 当成本地可中断;旧 worker 的迟到失败也不能删除后来接管的 adapter 并把正在工作的 Session 标成 stale。三者都会让同一个 Session 的权威状态发生漂移。

改动思路

实现继续沿用现有 owner:ChatSessionStore 持久化 session_mode / active_turn_id,ChatRuntimeController 在产生副作用前判断所有权,HTTP handler 只运输 typed error。attached resume 只在 exact route 找到已绑定 Session 时绕过本地 endpoint;managed/new Session 仍保留 capability 校验和 adapter 启动。interrupt bridge 未实现时 fail closed,而不是推断远端控制权。

具体改动

  • open_session 在 resume_latest 时先返回已有 attached_host Session;缺少本地 endpoint 也不会启动 adapter。managed/new 分支行为不变。
  • _run_turn 的 unhealthy cleanup 只有在 self.adapters[session_id] is adapter 时才 pop 并标记 stale,避免旧 worker 清掉 replacement owner。
  • interrupt_turn 在任何本地状态 transition 前识别 attached active Turn,抛出 attached_session_interrupt_unavailable 与 host_tool_gate,Turn 保持 running、Session 保持 busy。
  • ChatRequestHandler._interrupt_turn 把 controller 的 error_code 原样放入 HTTP 424,不再让客户端依赖自然语言判断。

关键代码讲解

核心原则是“先判所有权,再产生副作用”。attached resume 的 bypass 由 typed session_mode 和 exact route 决定,不由 agent 名称或 endpoint 不可用推断;interrupt guard 同时要求 attached mode 与 matching active_turn_id;adapter cleanup 则用对象 identity 证明失败 worker 仍是当前 owner。三处都复用既有状态,没有引入第二套协调协议。

对主干的风险

运行时语义未发现新 blocker:在受支持 Python 3.11 环境中,两个 focused test 文件为 51 passed in 5.77s;Ruff 与 git diff --check 通过。最新 origin/main 已到 97c786a0f,也修改了 chat_runtime.py / chat_server.py 的其他 hunks,但 git merge-tree --write-tree origin/main HEAD 成功,生成 clean tree 0adcdc1945669a220d4fa35b945ab86c76c22400。真实外部 host process / network socket 未在本地复现,这是剩余验证边界,不影响当前 typed ownership 结论。

当前仍有一个 P1 合并阻塞:final merge commit c2d990a247c2b131b7bd0cff697d1f41a67ad1a9 没有 Signed-off-by,本地 PR-range 扫描只报告这一条 unsigned commit,GitHub Sign-off 也明确失败。最小修复是只重建/修订该 integration merge,保留双亲和贡献者 ancestry,补合法 trailer 后以 force-with-lease 更新,并重跑 DCO 与 exact-head checks。

我的整体评价

结论是 REQUEST CHANGES。attached Session、Turn、adapter 的 ownership 边界已经闭环,旧 review 指出的 HTTP error-code 丢失也已修好;214/10 行的范围以回归测试为主,future-facing pass 不需要再造通用协调框架。但仓库要求每个 PR commit(包括 merge commit)都通过 DCO,当前 exact head 尚不满足。修复签名并更新 head 后,需要重新绑定 head 校验,运行时判断本身预计可以批准。

Exact head: c2d990a247c2b131b7bd0cff697d1f41a67ad1a9

English verdict: REQUEST_CHANGES — exact head c2d990a247c2b131b7bd0cff697d1f41a67ad1a9 preserves attached Session/Turn/adapter ownership correctly and passes 51 focused tests, but its final integration merge commit is unsigned and the required DCO Sign-off check fails.

…ssion-lifecycle

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
@Duang777
Duang777 force-pushed the codex/fix-attached-session-lifecycle branch from c2d990a to 6d9f8be Compare September 11, 2026 13:54
…ssion-lifecycle

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
…ssion-lifecycle

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
@Duang777
Duang777 force-pushed the codex/fix-attached-session-lifecycle branch 2 times, most recently from 0c2ea7d to 5ef21ae Compare September 11, 2026 14:41
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
@Duang777
Duang777 force-pushed the codex/fix-attached-session-lifecycle branch from 5ef21ae to 5eb8d46 Compare September 11, 2026 14:50
…ssion-lifecycle

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
(cherry picked from commit 3120939)
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
…ssion-lifecycle

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
…ssion-lifecycle

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>

# Conflicts:
#	examples/personal-workspace-browser-smoke.mjs
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
(cherry picked from commit 8e2262a)
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
(cherry picked from commit f4f2b9b)
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
…ssion-lifecycle

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

评审绑定 exact head:f3302b38c60b6b6e0e87ab07de7fc4fb2d69871d。

动机

这个 PR 的核心目标仍然正确:attached-host Session 不应因本机没有 managed endpoint 而无法 resume;本地 runtime 在没有 host interrupt bridge 时不能抢占 host-owned active Turn;旧 worker 的迟到失败也不能删除后来接管的 adapter 并把新 owner 的 Session 标成 stale。此前复审确认过这组 ownership 修复与 typed HTTP error propagation,现在需要按最新 exact head 重新审查完整 base-to-head 范围。

改动思路

Chat 部分继续复用正确边界:ChatSessionStore 的 session_mode / active_turn_id 是权威状态,ChatRuntimeController 在任何副作用前判断 attached ownership 和 adapter object identity,ChatRequestHandler 只把 controller 的 typed failure 适配成 HTTP 424。这一部分没有创建第二套状态机,managed/new Session 分支也仍走原有 endpoint 校验和本地 adapter 生命周期。

但当前 head 的问题不在核心算法,而在 branch composition:最新 base-to-head 范围额外携带了 Dashboard pending-text locator 和 Todo decision_scope.py cast 修复。它们分别已经由开放的 #4251 和 #4204 负责,既不是 attached Session ownership 的依赖,也没有在本 PR Summary / Validation 中披露。

具体改动

关键代码讲解

  1. ChatRuntimeController.open_session 在 exact route 找到 attached_host Session 时先复用,再决定是否需要本地 endpoint;这保持了 host ownership。
  2. _run_turn cleanup 以 self.adapters.get(session_id) is adapter 证明失败 worker 仍是当前 owner,避免删除 replacement。
  3. interrupt_turn 在本地 transition 前拒绝 matching attached active Turn,返回 attached_session_interrupt_unavailable 与 host_tool_gate;handler 原样转发 error code。
  4. 上述 Chat 语义在当前 head 的两个 focused test 文件中为 51 passed,Ruff、git diff --check 和与 current main 的 clean merge-tree 都通过。
  5. 不相关的 commit 9908571a6 修改 examples/personal-workspace-browser-smoke.mjs,与 #4251 的 exact locator 修复重叠;commit 596716c8d 修改 loopx/control_plane/todos/decision_scope.py,与 #4204 的 typing 修复重叠。当前精确 diff 已从此前的 4 文件 +214/-10 扩张为 6 文件 +235/-21。

对主干的风险

这是一个 P1 合并阻塞。触发条件很直接:如果 #4133 以“attached Session ownership fix”合入,而 #4251/#4204 继续独立演进或合并,同一 Dashboard/Todo 变更就出现两个未披露的交付 owner。即使 git 能自动消解相同 patch,review evidence、变更归属、回滚边界和后续修改点都会与 PR 声明不一致;如果两边继续变化,还会引入不必要的集成冲突。

最小修复不是继续补测试,而是基于 current main 重建分支,只保留 attached Session/Turn/adapter 和 HTTP error 的 commits,丢弃 Dashboard locator 与 Todo typing commits。之后重跑 51 个 focused tests、Ruff、diff check、clean merge-tree 和 hosted checks,并用新 exact head 复审。托管全量 CI 当前尚未结束,但这个 scope/reuse blocker 与 CI 是否转绿无关。

我的整体评价

结论是 REQUEST_CHANGES。核心 ownership 实现依然合理,typed state、authority boundary、default-path isolation 与 domain-neutral error 均未发现新问题;真正阻塞的是最新 exact head 出现了跨域、跨 PR 的未披露 scope drift。future-facing pass 的高价值动作也是减法:恢复四文件的 cohesive Chat change,让 #4251 和 #4204 保持各自唯一 owner,而不是在 #4133 内抽象或重复这些修复。

English verdict: REQUEST_CHANGES — exact head f3302b38c60b6b6e0e87ab07de7fc4fb2d69871d still has sound attached-session ownership semantics and passes 51 focused tests, but it now carries undisclosed Dashboard and Todo commits already owned by open PRs #4251 and #4204; rebuild it on current main with only the Chat changes, then rerun exact-head validation.

@Duang777

Duang777 commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator Author

@huangruiteng Current exact head f3302b3 is synchronized with main 3aa05cc. The prior DCO/integration blockers remain fixed. Local focused validation: 51 attached-session/runtime tests passed; full mypy, Ruff, and diff checks passed. Current GitHub checks are all green, including merge-gate, both Stage2C E2E jobs, mutants, four pytest shards, Windows, and Node compatibility. Please re-review this exact head and clear the stale CHANGES_REQUESTED state if no new finding remains.

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
@Duang777

Duang777 commented Sep 12, 2026 •

Copy link
Copy Markdown
Collaborator Author

@huangruiteng Updated exact head 9af4ac3 onto current main 787e8dd. The unrelated Dashboard and Todo carryover was removed by retaining the main versions; the final base-to-head diff is exactly the four intended Chat/ownership files. Local validation: 51 focused tests passed, scoped Ruff passed, full mypy passed, and git diff --check passed. Fresh exact-head CI is running; please re-review this head after it completes.

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
@Duang777

Copy link
Copy Markdown
Collaborator Author

@huangruiteng Updated exact head 71114ae onto current main bb0ba5b. The merge was clean; the final diff remains exactly the four intended Chat/ownership files with no shared carryover. Local validation: 51 focused tests, scoped Ruff, full mypy, and git diff --check passed. Main now contains the UTC-without-tzdata fix, so fresh CI should clear the prior Windows baseline failure. Please re-review this exact head after CI completes.

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
@Duang777

Copy link
Copy Markdown
Collaborator Author

@huangruiteng Main advanced by docs-only #4265 while the previous exact-head CI completed. Current exact head b3f8d58 now contains latest main 481497e; the merge was clean and the 4-file business diff is unchanged. The prior exact head completed 22/22 checks successfully, including Windows and merge-gate; fresh CI is running.

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
@Duang777

Copy link
Copy Markdown
Collaborator Author

@huangruiteng Current exact head c929e44 now includes latest main 0cbb847, including merged #4204. The merge was clean; the final PR diff remains exactly the four intended Chat/ownership files at +214/-10. Local validation: 51 focused tests, scoped Ruff, full mypy, and git diff --check passed. Fresh exact-head CI is running; please re-review this head when complete.

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval conclusion

动机

本 PR 修复 attached Chat Session 的 ownership:恢复时不应强制启动本地 endpoint;host-owned active Turn 不应被 LoopX 当作本地可中断;旧 worker 失败时也不能清理已经替换的新 adapter。HTTP 层还必须保留 typed error_code 与 gate,让客户端知道下一步由 attached host 处理。

改动思路

沿用现有 ChatSessionStore、ChatRuntimeController 和 ChatRequestHandler 作为唯一 owner。controller 在状态转换前判断 session_mode/active_turn_id,adapter cleanup 用对象 identity 防止 stale worker 越权;handler 只负责把 CodexChatAgentError 的结构化字段传输到 HTTP 424,不新增第二套状态机或远程控制 authority。

具体改动

  • open_session 对 attached_host 直接复用最新 session,managed/new session 仍执行原有 capability validation 与 adapter 启动。
  • interrupt_turn 对 matching host-owned active Turn 在任何本地 transition 前抛出 attached_session_interrupt_unavailable,并返回 host_tool_gate;running/busy 状态保持不变。
  • _run_turn 仅在当前 adapter identity 仍等于失败 worker 的 adapter 时才执行 stale cleanup,避免删除 replacement adapter 或错误标记 Session。
  • ChatRequestHandler._interrupt_turn 保留 error_code、gate 和 424 status;四个文件内的回归测试覆盖 resume、interrupt、replacement cleanup 和 endpoint transport。

对主干的风险

当前精确 head c929e4460f36d1966c4d98d4020d8998b785fbcf 的 diff 为 4 个文件、+214/-10。attached broker/active-turn focused suite 51 个测试通过(5.68s),Ruff、git diff --check 通过,git merge-tree --write-tree origin/main HEAD clean;GitHub 当前检查全部成功、0 failed、0 pending。此前 typed error 丢失、旧分支夹带 Dashboard/Todo 变更和 DCO 问题均已清除。该 PR 有意不实现外部 host interrupt bridge,LoopX 只 fail-closed 并把控制交还 attached host;未发现阻塞问题。

我的整体评价

变更紧贴既有 ownership 边界,修复了 resume、interrupt、late failure cleanup 和 HTTP error transport 的一致性,同时没有改变 managed Chat 的默认路径。我确认当前精确 head 的代码、测试、合并树和远端检查一致,结论为 APPROVE,并清除此前已解决的 CHANGES_REQUESTED 项。

English verdict: APPROVE — exact head c929e44 clears the prior typed-error and branch-composition blockers; 51 focused tests, clean merge-tree, and all remote checks pass.

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval conclusion

Reviewed current exact head: 60facc5c1c6a0ab7a2778e8c81c1eefb5953628b (base main, net diff remains the four intended Chat/ownership files, +214/-10). This is an independent whole-diff review; the earlier approval for c929e4460f36d1966c4d98d4020d8998b785fbcf was not inherited.

动机

这个 PR 修复 attached-host Chat Session 的 ownership 连续性,而不是增加一套新的会话协调器。旧路径在 resume_latest 时会把已有 attached Session 送入本地 adapter 恢复,因此本机没有对应 managed endpoint 时无法重开;本地 interrupt 也可能在不知道 host 控制权的情况下推进 host-owned active Turn;旧 worker 的迟到失败还可能删除后来接管的 replacement adapter 并把 Session 标成 stale。HTTP interrupt handler 同时丢失 CodexChatAgentError.error_code,客户端只能依赖自然语言。结果是 Session/Turn 的权威状态、host 队列和客户端观察之间发生漂移。

改动思路

实现复用现有 ChatSessionStore 的 session_mode、active_turn_id 和状态迁移,以及 ChatRuntimeController 的 adapter map;ChatRequestHandler 只负责 HTTP 结构化错误运输,attached host broker 继续负责 host identity、claim 和 completion。open_session 在 exact Goal/Agent/channel 找到 attached_host latest Session 时先直接返回,只有 managed/new 或非 attached latest 才走既有 capability 校验与 _ensure_adapter。interrupt_turn 在任何本地状态写入前检查 attached mode 与 matching active Turn,明确把控制权交回 host;queued/terminal 和 managed active Turn 仍走原逻辑。_run_turn 的 unhealthy cleanup 通过 adapter object identity 证明失败 worker 仍是当前 owner,handler 则把 typed error code 原样传出。这样正向路径保持 host Session,不引入本地 adapter 或远程 interrupt 假设;负向路径 fail closed,且没有第二套状态机。

具体改动

  • loopx/chat_runtime.py:448-470:open_session 先读取 exact route 的 latest Session;已有 attached_host 直接复用,避免启动/恢复本地 endpoint。managed/new 及非 attached resume 仍要求 capability 可用并复用现有 adapter 生命周期。
  • loopx/chat_runtime.py:1117-1127:异常清理仅在 self.adapters[session_id] is adapter 时 pop 并标记 stale,迟到的旧 worker 不会覆盖 replacement owner。
  • loopx/chat_runtime.py:1178-1195:对 attached_host + active_turn_id == turn_id 抛出 attached_session_interrupt_unavailable 和 host_tool_gate,在 transition 前保持 Turn running、Session busy;queued/terminal 分支不变。
  • loopx/chat_server.py:833:_interrupt_turn 将 error_code 与 424、gate 一起放入 _send_error,不重新解释 controller 的 ownership 判断。
  • tests/test_attached_session_broker.py 新增 attached resume、host-owned interrupt、HTTP typed failure 回归;tests/test_chat_session_active_turn.py 新增旧 adapter 失败与 replacement adapter 竞态回归。

关键代码讲解

  1. open_session 的 attached-first 分支只允许已有持久化 Session 命中 exact route,缺少本地 endpoint 不能单独“推断”出 attached 行为;因此 bypass 是 session_mode 驱动的,而不是 agent 名称或错误文本驱动的。
  2. interrupt_turn 在 update_turn(..., status="interrupting") 之前判定 host ownership;没有 host interrupt bridge 时不会写状态、调用 adapter、取消 Turn 或产生 interrupted event。
  3. _run_turn 的 identity fence 和 _interrupt_turn 的 typed transport 分别保护运行时 owner 与公共机器语义,底层 Session/Turn/store 仍是唯一事实来源。

对主干的风险

主干风险集中在 Session/Turn ownership 组合,而不是新数据格式。attached resume 的副作用是纯读取,且不会写入 runtime.adapters;matching host-owned active interrupt 直接抛出 typed host gate,故不会把远端执行伪装成本地已中断。旧 worker 仍会记录自己的失败 Turn,但只有当前 adapter 仍是它时才清理 map/Session,replacement 的 busy/active 状态保持。HTTP 变化仅增加 error_code 字段,其他 424/gate、404 和成功响应保持兼容。测试使用真实临时 ChatSessionStore 和 controller/handler 方法,覆盖 running/busy/active_turn_id、queued/terminal、adapter replacement 竞态及错误序列化;没有启动外部 host process 或网络 socket,因为本 PR 明确不实现 host interrupt bridge。当前 exact head 的远端检查为 15 个成功、3 个预期跳过、0 个失败,4 个 pytest shard 仍在排队/运行;本地 focused 51/51、相关 Chat/API/store 46/46、Ruff、git diff --check、compileall 和与 origin/main 的 clean merge-tree 均通过。剩余验证边界是远端 shard 完成以及真实外部 host 的进程级对接,二者不会改变当前四文件 ownership 判断。

我的整体评价

这是一个范围紧凑、沿现有 owner 修复的 PR。它把 attached Session resume、host-owned active Turn fail-closed、late-worker cleanup 和 typed HTTP error 四个相互关联的 ownership 缺口闭合,且 managed/new 默认路径、queued/terminal interrupt、host broker claim/completion 和本地 adapter 生命周期均保留。当前基于 main 的净 diff 仍只有四个目标文件,先前 review 里提出的 typed error、DCO、主干组合和跨 PR carryover 问题在本 head 已清除;本次结论来自对当前完整 diff 和独立验证,而非复用旧 APPROVE。结论为 APPROVE;不授予 merge 或远程 host interrupt 权限,后者应在单独的 authority contract/bridge 变更中处理。

English verdict: APPROVE — exact head 60facc5c1c6a0ab7a2778e8c81c1eefb5953628b preserves attached Session/Turn/adapter ownership, propagates the typed interrupt error, and keeps managed paths unchanged. Focused 51-test and related 46-test suites pass, with clean diff/merge-tree; remote pytest shards are still pending but no check has failed.

@huangruiteng
huangruiteng merged commit 42672da into loopx-project:main Sep 13, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants