Repository navigation
fix(chat): finish atomic stop and queued Turn delivery - #7505
Conversation
interrupt_turn reads the Turn, appends the durable interrupt-intent event, and only then compare-and-sets the status. When the Turn completed in between, the CAS refuses the change but the intent event remains in the completed Turn's ledger, because #7191 moved the intent ahead of the status write without keeping the terminal-state check that write used to provide. That breaks the terminal event history pinned by test_interrupt_does_not_overwrite_a_completed_turn and makes the completed Turn report stop_observed, which suppresses the unfinished-work continuation in the external-conversation planner. Make the status check, the intent event and the status write one store transition under the Turn write lock. The intent still lands before the status change, so a crash cannot turn a requested stop into an automatic review, and a Turn that already reached a terminal state can no longer gain a stop event after its terminal outcome. Fixes #7476 Signed-off-by: JasonBuildAI <jasonbuildai@gmail.com>
The stop-intent transition adds 33 lines to chat_store.py and one net line to chat_runtime.py. Refresh the reviewed module metric ceilings in the same diff so the maintainability ratchet settles the accepted growth. Signed-off-by: JasonBuildAI <jasonbuildai@gmail.com>
`claim_next_queued_turn` wrote the Turn from the snapshot the queue scan had read, under the Session lock only, while `begin_turn_interrupt` read and wrote the same Turn under the Turn write lock. The two writers therefore did not share a serialization boundary: an interrupt that read `queued` could resume after the claim had committed `running` with a `host_claim_id` and had published the Session active pointer, then write its older snapshot back as `interrupting` with no claim id. Take the Turn write lock for the claim too, and re-read the Turn inside it. An interrupt that already committed now wins the transition, so the claim returns `None` instead of starting a Turn the stop request owns, and a claim that commits first is never rewritten from a snapshot that predates it. The queue scan stays a candidate list; the authoritative status check moves under the lock that the interrupt also takes. Cover the interleaving deterministically: the interrupt is paused after its read, the claim is attempted while the interrupt still holds the lock, and the test asserts the persisted status and claim id, the Session active pointer and the event ledger agree. Signed-off-by: JasonBuildAI <jasonbuildai@gmail.com>
Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com>
…to codex/review-chat-interrupt-7477-20261027 Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com>
Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com>
Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com>
Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com>
…lease220-chat-stop-integration-20261028 Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com>
loopx-agent
left a comment
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent; gpt-6.1-sol; OpenAI; runtime_reported; reasoning_effort=max
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
APPROVE，精确 head c184163f8cf9534c24f8e5be2a13f153f98e59b5。本轮独立复审并修复了旧评审指出的claim竞争，以及复审新发现的expiry竞争。
动机 / Motivation
在Chat、桌面或Lark会话停止一个正在结束或排队的Turn的用户与宿主。 停止请求输给已完成Turn时仍追加stop intent，使未取消的工作被当作用户取消；早期修复又可能用旧快照抹掉成功领取的host_claim_id。 同一Turn锁内复查状态、先持久化停止意图再变更状态，终态败者不追加事件；领取、排队超时和acceptance结清共享该锁并重新读取当前状态。 不授予新的执行或自动继续权限，不改变宿主取消API、队列顺序、模型、配额或用户界面。
改动思路 / Approach
本次是维护者接管后的完整发布提案，保留 @JasonBuildAI 原始commit/DCO、#7477讨论与两个评审finding。原fork PR因最后pusher同reviewer，GitHub仍REVIEW_REQUIRED、native ready=false；该事实未被改写。这里按维护者明确授权的作者自评审路径，重新验证当前整合head的265项、premerge和静态/边界检查，声明为自评审，不冒充独立二审，也不修改GitHub规则。原PR只能在本提案实际合入后标为superseded。
复用ChatSessionStore原有Turn文件锁、事件账本及状态集合，将同一Turn的竞争写入纳入现有owner，不另建事务框架或Python/TS决策副本。 本PR修复既有Chat停止路径及同一store中直接竞争的queued writer，保留原宿主与恢复责任。 不是把事件移回状态写之后，而是把最新状态检查、durable intent和状态写入收在同一Turn锁内；这样终态败者无新增事件，崩溃时已接受的停止意图仍先于状态存在。相邻queued claim、expiry和acceptance结清也必须复用此边界，否则只锁interrupt仍会读旧快照覆盖执行归属。锁顺序保持Session后Turn，interrupt持Turn期间不反向取得Session。
具体改动 / Changes
逐项按 #7476 的 2026-10-27T09:11:31Z 版本核对 I7476-TERMINAL-LOSER、I7476-INTENT-ORDER、I7476-ATOMIC-TURN-TRANSITION：ChatRuntimeController.interrupt_turn调用store helper；begin_turn_interrupt在原Turn文件锁内重新读取，终态直接返回，允许状态才append intent并写interrupting。queued host claim锁内重新读取，只有仍queued才能提交claim并发布Session active pointer；claim先提交时完整claim字段保留，interrupt先赢时不领取其他queued头。排队expiry也锁内重读，不能在已接受停止后写timed_out/turn.failed；prepared acceptance结清仅在同锁内去掉capsule，保留当前Turn字段。
旧 CHANGES_REQUESTED #5666309659 的claim blocker已逐项核对：原来成功claim后的host_claim_id可能被旧queued快照抹掉，现在两个writer共享Turn锁，控制线程测试同时断言persisted status、claim、Session pointer和event ledger。新评审 #5670407863 的expiry blocker也逐项核对：我方另以7fdf3cade6dd独立反例实际复现expiry额外追加turn.failed，再修复并保留该例；另经真实claim_next_queued_turn让expiry先提交，再从Runtime stop，完整timed_out/session_queue_expired、claim/Session和ledger保持不变。claim先赢与intent后状态写前崩溃两例核验claim identity和原intent顺序。最终head七个完整模块265通过、0跳过；终态败者、interrupt先赢、claim先赢、intent后状态写前崩溃及排队超时均覆盖。先前7fdf3cade6dd的expiry反例1失败1通过，最终修复保留该反例。最终premerge、规定Ruff、mypy和四文件隔离边界通过。 模块大小在相同base/head测得chat_store 2003→2066、chat_runtime 2319→2320；只结清实测增长，不删锁、字段或安全语义来满足旧上限。
对主干的风险 / Risks
主要风险是锁序反转或把旧候选当作最新authority。逐条检查了Session→Turn顺序、lock内重新读取与终态返回；没有Turn→Session的新增持锁路径，事件仍走原账本owner。265例覆盖实际本地JSON、文件锁、队列、进程恢复及原UI队列后端，适配器仍为合成Host，不能把这些结果称为真实模型取消或Windows资格。本机真实JSON存储、文件锁和事件账本已验证；适配器使用合成Host，未执行Windows或真实远端模型取消。发布版完整安装与UI旅程由统一候选完成。 CI依Goal策略未查询，先前失败保持历史失败，只有最终source的通过支持当前结论。
我的整体评价 / Verdict
复用ChatSessionStore原有Turn文件锁、事件账本及状态集合，将同一Turn的竞争写入纳入现有owner，不另建事务框架或Python/TS决策副本。 本PR修复既有Chat停止路径及同一store中直接竞争的queued writer，保留原宿主与恢复责任。 三项issue验收和旧claim blocker在当前head都有代码与独立执行证据；same-owner companion修复关闭了expiry的实际反例，acceptance结清也不再越过Turn锁。没有新增通用事务框架、影子状态或外部权限。当前改动可以独立回滚，无状态迁移；仍需在正式发布候选完成完整安装和渠道核验。发布APPROVE后只按原生closeout核对已解决旧review，未核验的异议不会被删除或忽略。
English verdict: APPROVE c184163f8cf9534c24f8e5be2a13f153f98e59b5. The terminal loser adds no stop intent; admitted intent stays durable before status; claim,expiry andacceptance settlement serialize onthe sameTurnlock.265cases andrequiredlocalgates passed. Priorclaim blocker is resolved;realremoteHost/Windows qualification isnotclaimed.
Maintainer delivery and provenance
This maintainer-owned release proposal completes the work from #7477. Original commits and DCO authorship by @JasonBuildAI are retained, together with the claim and expiry findings and their public exact-head resolution review. Maintainer follow-through adds expiry/acceptance serialization and independent recovery/order tests.
The original fork PR still has a truthful
ready=false: its final repair pusher and approver are the same account, while GitHub requires last-push approval by another account. Its review, discussion and native readiness observation are preserved. This proposal explicitly uses the maintainer-authorized author-owned self-review/admin-delivery path, with a new complete exact-head review, qualification and readiness decision; it does not claim an independent second reviewer or change repository approval rules. #7477 will be marked superseded only after this proposal actually merges.Problem and resulting behavior
A stop request racing completion could append stop intent to a Turn that had already completed, suppressing legitimate unfinished-work recovery. The store now checks the current status, persists admitted intent, and enters interrupting under the existing Turn lock. A terminal loser leaves the ledger unchanged; intent still precedes status for crash safety.
Attached-host queued claims, queue expiry and prepared acceptance settlement share the same Turn serialization boundary and re-read current state. They preserve successful claim identity and cannot overwrite a stop from an older queue snapshot. This resolves the claim-authority blocker in reviews #5666309659 and #5670407863, independently reproduced during maintainer review. No new execution permission, state schema, model policy or UI is added.
Closes #7476.
Validation
Exact head:
c184163f8cf9534c24f8e5be2a13f153f98e59b5.7fdf3cade6dd51900bcfa6f43cdb60d76bc73447failed and remains in the final regression suite.chat_store.py2,003 -> 2,066 lines andchat_runtime.py2,319 -> 2,320. The ceilings settle the reviewed atomicity work in this diff; no safety clause, authority field or unique regression assertion is removed to fit an earlier size.Frontend / Visual Evidence
UI impact: none. Existing stop and queue interfaces retain their caller contract; installed release/UI qualification is separate.