Problem
_execute_agent_session's finally now contains two independent executor-side finalize guards, and the older one does not know what the newer one knows.
Two consequences, both verified by reading the code at main:
1. The cancel carve-out is void for synthetic dev-* lanes. On a cancelled exit in a dev-{aid8} lane, _finalize_if_still_running is skipped as designed, but ~70 lines later in the same finally the #3176 guard finalizes the row anyway. The chain: agent/session_health.py handle.task.cancel() -> await asyncio.wait_for(handle.task, ...) (the executor finally runs inside this await) -> transition_status(entry, "pending", reason="health check: recovered session ...") -> except StatusConflictError, INFO-logged and swallowed. The AGENT_SESSION_HEALTH_MIN_RUNNING gate sits upstream of the kill and does not protect this ordering. So the exact hazard the carve-out exists to prevent still occurs, scoped to synthetic lanes.
This is long-standing behavior, not a regression — the guard is byte-identical to its pre-#3209 form. #3209 narrowed the blast radius (an uncarved hoist would have extended requeue pre-emption from dev-* lanes to every session on the machine) but did not close it.
2. The two guards can write divergent terminal statuses. _finalize_if_still_running wraps its body in an except Exception that logs a warning and returns. If finalize_session fails for any non-CAS reason on a raise path in a synthetic lane, the row is left running and the #3176 guard then writes _runner_final_status(_task.error, _agent_session) — which returns "completed" for a bound task with falsy task.error. That is precisely the false-success that #3209's raised=True carve-out exists to prevent, written by a sibling guard with no knowledge of _raised_exit.
Desired Outcome
One finalize authority in that finally, not two that disagree. Either:
Whichever shape is chosen, the cancel-vs-requeue tradeoff for synthetic lanes must be decided explicitly rather than inherited: either the lane leak on a cancelled dev-* exit is accepted in exchange for preserving the requeue, or the requeue pre-emption is accepted and documented as intentional. Today docs/features/session-isolation.md and _finalize_if_still_running's docstring assert opposite things about the same write.
Context
Surfaced by the round-5 two-judge review of #3248 (issue #3209); both judges reached this independently and both correctly classified it as inherited rather than introduced, so it did not block that merge.
Triage 2026-09-15
Status: confirmed by code read on main 205344717.
Single-line hotfix: Yes, roughly. The #3176 guard (agent/session_executor.py:3004-3048) already has _raised_exit in scope; its _guard_status calc just needs "failed" if _raised_exit else (...) prepended, mirroring _finalize_if_still_running's raised param. That closes the disagreement for the raise case. It does NOT need a cancel carve-out of its own — cancel is precisely the case this guard exists to cover (the top guard skips it), so no exclusion should be added there.
Reduce-complexity option: Yes, and it's the better fix. Two finalize call sites in one finally, each re-implementing get_authoritative_session + CAS re-read + StatusConflictError handling, disagreeing on raised, is a design smell, not two separate bugs. Collapse to one guard called once near the end of the finally, keyed on an explicit tri-state exit kind (normal / raised / cancelled) already available via _cancelled_exit/_raised_exit, with the worktree-cleanup code just re-reading the row status it left behind. This removes the possibility of the two guards ever diverging again, rather than patching today's known divergence.
Relationship: Same regression surface as #3306 (both introduced by #3209's finalize-in-finally change to the executor); recommend fixing both as one change — a single unified finalize guard whose checkpoint call is also made non-blocking.
Problem
_execute_agent_session'sfinallynow contains two independent executor-side finalize guards, and the older one does not know what the newer one knows._finalize_if_still_running(agent/session_executor.py, added by defer_reaction gates the completion-exit finalize guard, so a deferred session reaches its own worktree cleanup while still running #3209) runs at the TOP of thefinally, covers normal return and raise, and is deliberately skipped on cancellation — because_agent_session_health_checkcancels the task and then decides the row's fate inside the same await, commonly requeueing it topending. A finalize landing first turns that requeue into a swallowedStatusConflictErrorand silently retires the retry loop.Two consequences, both verified by reading the code at
main:1. The cancel carve-out is void for synthetic
dev-*lanes. On a cancelled exit in adev-{aid8}lane,_finalize_if_still_runningis skipped as designed, but ~70 lines later in the samefinallythe #3176 guard finalizes the row anyway. The chain:agent/session_health.pyhandle.task.cancel()->await asyncio.wait_for(handle.task, ...)(the executorfinallyruns inside this await) ->transition_status(entry, "pending", reason="health check: recovered session ...")->except StatusConflictError, INFO-logged and swallowed. TheAGENT_SESSION_HEALTH_MIN_RUNNINGgate sits upstream of the kill and does not protect this ordering. So the exact hazard the carve-out exists to prevent still occurs, scoped to synthetic lanes.This is long-standing behavior, not a regression — the guard is byte-identical to its pre-#3209 form. #3209 narrowed the blast radius (an uncarved hoist would have extended requeue pre-emption from
dev-*lanes to every session on the machine) but did not close it.2. The two guards can write divergent terminal statuses.
_finalize_if_still_runningwraps its body in anexcept Exceptionthat logs a warning and returns. Iffinalize_sessionfails for any non-CAS reason on a raise path in a synthetic lane, the row is leftrunningand the #3176 guard then writes_runner_final_status(_task.error, _agent_session)— which returns"completed"for a bound task with falsytask.error. That is precisely the false-success that #3209'sraised=Truecarve-out exists to prevent, written by a sibling guard with no knowledge of_raised_exit.Desired Outcome
One finalize authority in that
finally, not two that disagree. Either:_finalize_if_still_runningso a single call site owns the cancellation andraisedsemantics, or_cancelled_exit,_raised_exit) into the Synthetic-slug worktrees are invisible to both busy-guard predicates: the executor never persists the slug it synthesizes or the worktree it resolves #3176 guard so it makes the same decisions.Whichever shape is chosen, the cancel-vs-requeue tradeoff for synthetic lanes must be decided explicitly rather than inherited: either the lane leak on a cancelled
dev-*exit is accepted in exchange for preserving the requeue, or the requeue pre-emption is accepted and documented as intentional. Todaydocs/features/session-isolation.mdand_finalize_if_still_running's docstring assert opposite things about the same write.Context
Surfaced by the round-5 two-judge review of #3248 (issue #3209); both judges reached this independently and both correctly classified it as inherited rather than introduced, so it did not block that merge.
Triage 2026-09-15
Status: confirmed by code read on main
205344717.Single-line hotfix: Yes, roughly. The #3176 guard (
agent/session_executor.py:3004-3048) already has_raised_exitin scope; its_guard_statuscalc just needs"failed" if _raised_exit else (...)prepended, mirroring_finalize_if_still_running'sraisedparam. That closes the disagreement for the raise case. It does NOT need a cancel carve-out of its own — cancel is precisely the case this guard exists to cover (the top guard skips it), so no exclusion should be added there.Reduce-complexity option: Yes, and it's the better fix. Two finalize call sites in one
finally, each re-implementingget_authoritative_session+ CAS re-read +StatusConflictErrorhandling, disagreeing onraised, is a design smell, not two separate bugs. Collapse to one guard called once near the end of thefinally, keyed on an explicit tri-state exit kind (normal / raised / cancelled) already available via_cancelled_exit/_raised_exit, with the worktree-cleanup code just re-reading the row status it left behind. This removes the possibility of the two guards ever diverging again, rather than patching today's known divergence.Relationship: Same regression surface as #3306 (both introduced by #3209's finalize-in-
finallychange to the executor); recommend fixing both as one change — a single unified finalize guard whose checkpoint call is also made non-blocking.