Report a self-requested Codex stop as interrupted, not a provider error

[hamzamerzic]

Problem

When a turn is stopped — the owner pressing Stop, the stall watchdog, or a newer turn superseding this one — Möbius interrupts the turn and, if that interrupt times out, escalates to SIGTERM against the turn's private process group. The Codex transport then dies mid-stream instead of delivering turn/completed, and the runner's generic except path reported that death as a provider error string (e.g. "Codex process closed stdout. stderr_tail=").

That string is not merely wrong attribution — it is destructive. chat.py publishes a non-null error as an error block, and events.process_event coalesces error blocks latest-wins: the newer block pops every ERROR_PASSTHROUGH_FIELDS key it does not itself carry. The stop/stall pause note published moments earlier is an error block too, so it is overwritten and its one-tap Resume disappears. The owner is left with an unexplained provider error and no way back into the conversation.

Cause

run_codex_sdk_turn's except Exception path had no way to tell "the provider broke" from "we killed this turn ourselves". Both arrive as an exception out of the stream, and both were returned as {"error": str(exc)}.

Telling them apart needs two facts the runner already had but never combined:

• whether the exception is the transport dying, as opposed to a provider fault — the runner reraises every non-retryable provider ErrorNotification as a plain RuntimeError(message), and those messages routinely contain closed-ish phrasing such as "MCP server 'x' is not running";
• whether a stop was in flight — either active_turn.interrupt_requested, or the superseded-generation abort, which can be true before an ActiveCodexTurn exists at all.

The existing _is_closed_turn_error is deliberately loose about the first: it accepts bare RuntimeError text because its only caller is the steer path, where a false positive costs a refused steer. It is far too loose to decide whether an error reaches the owner.

Fix

backend/app/codex_sdk_runner.py

• Split _is_transport_death() out of _is_closed_turn_error(). The new predicate matches only the SDK's own TransportClosedError (by isinstance against the real symbol from _sdk_imports(), so genuine subclasses match and same-named impostors do not) or an RPC error about a closed/dead channel. _is_closed_turn_error stays wide and keeps its steer-path caller.
• Add stop_requested() — one definition of "we did this to ourselves", shared by the terminal-validation path (which sees a clean TurnStatus.interrupted) and the except path (which sees the transport die because force_stop killed the process group). It includes the superseded-generation abort, which can be true before active_turn exists: a teardown during startup is no more the provider's fault than one mid-stream.
• Add with_usage() and route the error returns through it, so a turn that spent tokens and then ended reports them however it ended.
• In the except path, a transport death while a stop is in flight returns a clean interrupted terminal status with error: None instead of a raw provider string. Logged at WARNING: the owner still receives a clean interrupted outcome, while a coincident real transport crash remains visible to operators because the transport's dying words are the only forensics left after error is intentionally cleared downstream.
• _sdk_imports() now exposes TransportClosedError, and the guard around it catches ImportError rather than ModuleNotFoundError: an SDK that renames or drops the symbol fails the from … import the same way a missing package does, and this predicate runs inside the turn's except handler — raising there would mask the very exception it was asked to classify.

Verification

The containerised runner was unavailable here, so pytest ran directly with an isolated tmp DATA_DIR/SQLite DB as conftest.py provisions.

• test_codex_sdk_runner.py + test_codex_sdk_contract.py + test_runner_registry.py + test_runner_registry_integration.py → 112 passed
• the focused runner suite, including the warning-level forensic assertion, passed with 96 tests in the final review
• test_chat*.py → 318 passed
• test_events.py + test_runner_registry.py → 90 passed

Every new test is mutation-verified — each mutation applied, suite re-run, then reverted:

Mutation	Result
Except-path guard widened to _is_closed_turn_error	1 fail (provider fault during stop loses its error)
isinstance replaced by class-name comparison	3 fail
with_usage dropped from the sibling error return	1 fail
and stop_requested() dropped	1 fail (unrequested transport death must stay an error)
stop_requested() narrowed to the interrupt flag only	1 fail (superseded-generation kill)
Transport check dropped entirely	2 fail

A new contract test pins that the SDK still exposes TransportClosedError, so a future SDK that renames it fails loudly rather than silently degrading.

Residual notes

• The SDK is an optional install, so the runner tests keep a local stand-in class for environments without it; there the isinstance binding is exercised against a real subclass of the stand-in. The contract test (skipped without the SDK) is what pins the real symbol.
• Reclassification still turns on stop_requested(), so any future exception that is genuinely a transport death but is neither a TransportClosedError subclass nor a closed/not-running/broken-pipe RPC error keeps being reported as a provider error — the safe direction.
• The Claude runner has the same shape of bug and is untouched here; it deserves its own change.