fix(recovery): do not reattach cancelling runs

[allenliang2022]

Summary

ACTIVE_RUNS tracks worker lifecycle, which is deliberately broader than "a turn a browser may attach to". cancel_stream() keeps the row as phase="cancelling" while the worker unwinds, so a successor turn cannot start on top of it. But the client has already reached a terminal state for that stream — its run journal ends in a terminal event.

The recovery lookups treated every same-session ACTIVE_RUNS row as attachable. An idle session holding a cancelling row therefore received a recovered server_turn_started on every /api/session/stream subscription:

1. client subscribes → server replays server_turn_started for the cancelled stream
2. client attaches its renderer → immediately consumes the terminal event
3. renderer tears down → client resubscribes
4. server replays the same frame again → loop

The user-visible symptom is a transcript that rebuilds and jumps repeatedly on an idle session, with a steady trickle of replay requests. Because the row never leaves ACTIVE_RUNS on its own, the loop persists indefinitely.

The fix

The two meanings are separated at a shared chokepoint rather than by narrowing one call site:

• api/config.py gains active_run_is_attachable() and active_run_cancel_is_stale() as the shared predicates.
• active_stream_id_for_session() (browser recovery) returns attachable rows only.
• _session_has_active_turn() (busy check) still counts a fresh cancellation, so a successor cannot overlap the unwinding worker. This is the direction that must not change.
• _live_active_stream_id() applies the same rule to the hidden-tab status poller, on both the STREAMS and ACTIVE_RUNS paths.
• routes._cancelled_run_is_stale() now delegates to the shared predicate instead of keeping a parallel copy of the staleness rule.
• A cancelling row past a bounded unwind window with no live STREAMS channel is reclaimed from ACTIVE_RUNS and its stream owner released, so a wedged worker cannot suppress background wakeups forever. Age alone does not reclaim a row that still owns a live channel.

Staleness anchors on cancelled_at, falling back to started_at, so a long-running turn cancelled moments ago is never mistaken for an orphan.

State layer being mutated

ACTIVE_RUNS (worker-lifecycle registry) and its companion stream-owner map. The invariant being restored: a row is lifecycle-busy and client-attachable independently; cancellation ends the second while the first is still unwinding.

Siblings found and fixed

The same "any ACTIVE_RUNS row for this session is live UI work" assumption appeared in four places — session SSE recovery, the busy check, the hidden-tab status poller, and a duplicated staleness rule in routes.py. All four now route through the shared predicates. The busy check is intentionally the one caller that keeps counting cancelling rows.

Testing

tests/test_cancelling_run_not_attachable.py — 8 tests covering both directions of each rule, including a running-turn control, a live-channel reverse control, the cancelled_at vs started_at anchor, and predicate edge cases (blank/whitespace phase, non-dict entries stay attachable).

Test fails before, passes after:

# unpatched tree + new tests
6 failed, 2 passed

# with the fix
8 passed

Mutation checks (each turns the suite red, so the tests genuinely bite):

Mutation	Result
Force active_run_is_attachable() to always return True	5 failed, 3 passed
Disable the staleness reaper (stale_cancel never true)	1 failed, 7 passed

Neighboring suites — cancel / recovery / wakeup / hidden-tab / restore families:

117 passed in 18.48s

covering test_cancel_interrupt, test_cancel_stream_owner_guard, test_cancelled_turn_status, test_issue6623_cancel_owner_race, test_evict_session_agent_active_run_guard, test_background_process_restart_recovery, test_background_process_wakeup_format, test_bg_task_complete_wakeup, test_bg_task_complete_loadsession_stream_restart, test_5428_stale_client_recovery, test_hidden_tab_server_initiated_turn, test_hard_refresh_session_restore.

Lint: scripts/ruff_lint.py --diff origin/master → no new violations on added/modified lines. OK.

Relationship to #6623 / #6636

#6636 fixed the cancel owner race — stuck sessions, cancel write-back clobbering a successor, and cancel-worker timeout reclamation. This is a different layer: the row remains legitimately present in the lifecycle registry, and the defect is that the recovery/attach path reads that registry as "renderable live work." Both are needed; neither subsumes the other.

Not verified

• No UI change, so no before/after images apply.
• Verified against a reproduction of the loop and the neighboring automated suites; I have not exercised the multi-worker gateway/ACP surfaces beyond their existing tests.

[greptile-apps]

Greptile Summary

The PR separates worker-lifecycle occupancy from browser attachability so cancelling runs remain busy while no longer being exposed as recoverable UI work.

• Adds shared attachability and stale-cancellation predicates.
• Applies them to session recovery, hidden-tab status polling, and background wakeup admission.
• Reclaims stale cancellation bookkeeping only when no live stream channel remains.
• Documents and tests the cancellation lifecycle contract.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
api/background_process.py	Centralizes session run lookup, excludes cancelling runs from recovery, and retains fresh cancellations for lifecycle-busy checks.
api/config.py	Adds shared predicates for run attachability and cancellation staleness.
api/routes.py	Reuses the shared stale-cancellation predicate for persisted stream-state cleanup.
api/session_ops.py	Prevents hidden-tab status polling from reporting cancelling streams as live UI work.
docs/rfcs/webui-run-state-consistency-contract.md	Documents the distinction between worker lifecycle, browser attachability, and bounded stale cancellation reclamation.
tests/test_cancelling_run_not_attachable.py	Covers attachability, lifecycle-busy behavior, staleness anchors, live-channel protection, and stale-row cleanup.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[ACTIVE_RUNS entry] --> B{phase is cancelling?}
  B -- No --> C[Attachable and lifecycle-busy]
  B -- Yes --> D[Not browser-attachable]
  D --> E{Within unwind window or live STREAMS channel?}
  E -- Yes --> F[Remain lifecycle-busy]
  E -- No --> G[Remove ACTIVE_RUNS row and stream owner]

Loading

_{Reviews (2): Last reviewed commit: "docs(rfc): document cancellation attach/..." | Re-trigger Greptile}

[greptile-apps]

Document cancellation lifecycle semantics

This introduces a bounded cancellation-unwind window and stale-run reclamation alongside new attachability semantics, but the project documentation is not updated to describe the changed runtime contract, increasing the maintenance cost of future recovery and cancellation changes.

Context Used: AGENTS.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[allenliang2022]

Good catch — documented.

I added the contract to docs/rfcs/webui-run-state-consistency-contract.md, which is where the run-state invariants for streaming/recovery already live, rather than describing it only in code comments:

• State Layers table now lists ACTIVE_RUNS as the worker-lifecycle registry, with an explicit "must not do": be read directly as the set of runs a browser may attach to.
• New invariant 9 — "Lifecycle-busy is not client-attachable." Spells out that cancellation splits the two meanings, that recovery paths (session SSE recovery, hidden-tab status polling) must exclude cancelling rows while admission checks must keep counting them, and what goes wrong when a single meaning is used: the attach → terminal event → teardown → resubscribe loop.
• The same invariant documents the bounded unwind window: reclamation requires both age past the window and no live STREAMS channel, and staleness is anchored on the cancellation timestamp (falling back to run start) so an in-flight cancellation is never evicted early.
• Review checklist gained two questions: whether a change reads ACTIVE_RUNS for admission ("may a turn start?") or attachment ("may a browser render this?") and uses the matching predicate, and what evidence proves a reclamation-window change neither evicts early nor leaks a wedged row.

Also rebased onto the current master since the branch had fallen behind. Re-ran the affected and neighboring suites on the new base: 97 passed.

[nesquena-hermes]

Shipped in experimental release exp-v0.52.235. Recovery paths now exclude runs whose phase is cancelling, so a cancelling background run is never reattached/resurrected; genuinely-live runs still reattach; stale-cancelling reclamation is bounded (age ≥ 180s, absent from active streams) and idempotent. Codex + Opus both SAFE, full suite green. Thanks @allenliang2022!