From d2f152300439ca7b37efc5d98e1618bf9418d7f9 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Tue, 1 Sep 2026 20:38:56 -0700 Subject: [PATCH] [eric] agents: a SIGKILLed CLI (-9) is an external kill too, and the shutdown line is stamped before the shutdown stops the turn Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_012G8kyALnPjsA7aJFmMBq3R --- backend/apps/agents/agents.py | 2 ++ backend/apps/agents/core/error_classify.py | 3 ++- .../manager/session/SessionPersistence.py | 21 +++++++++++++++---- backend/tests/test_external_kill_respawn.py | 7 ++++++- backend/tests/test_shutdown_stop_note.py | 18 ++++++++++++++++ 5 files changed, 45 insertions(+), 6 deletions(-) diff --git a/backend/apps/agents/agents.py b/backend/apps/agents/agents.py index de1e4e1b..55a88c4c 100644 --- a/backend/apps/agents/agents.py +++ b/backend/apps/agents/agents.py @@ -34,6 +34,8 @@ async def agents_lifespan(): pool_sweeper = start_pool_sweeper(agent_manager.client_pool) yield logger.info("Agents sub-app shutting down") + # Stamp before stopping: once stop_agent has run, a live chat is indistinguishable from one the user stopped. + agent_manager.note_shutdown_stops() for session_id in list(agent_manager.tasks.keys()): await agent_manager.stop_agent(session_id) await agent_manager.persist_all_sessions() diff --git a/backend/apps/agents/core/error_classify.py b/backend/apps/agents/core/error_classify.py index 641e7383..18d1e1c0 100644 --- a/backend/apps/agents/core/error_classify.py +++ b/backend/apps/agents/core/error_classify.py @@ -464,7 +464,8 @@ def is_stale_tool_schema_error(exc: BaseException, extra_text: str = "") -> bool # narrow. 401 stays out (a rotating token really does heal, which is why the reset-hint rule exists), # and so do 408/429. Matched only in status POSITION, so a "400" in a line number or a byte count # cannot promote itself into a verdict (ENG-365 learned that the hard way with "line 401,"). -P_KILLED_EXIT = re.compile(r"Command failed with exit code (143|137)\b") +# 143/137 when the CLI re-raises the signal it caught; -15/-9 when it could not (SIGKILL is uncatchable, so a SIGKILL always arrives as -9). +P_KILLED_EXIT = re.compile(r"Command failed with exit code (143|137|-15|-9)\b") @typechecked diff --git a/backend/apps/agents/manager/session/SessionPersistence.py b/backend/apps/agents/manager/session/SessionPersistence.py index ad6f7373..9fc15f9d 100644 --- a/backend/apps/agents/manager/session/SessionPersistence.py +++ b/backend/apps/agents/manager/session/SessionPersistence.py @@ -134,16 +134,29 @@ class SessionPersistence(AgentManagerProtocol): logger.warning(f"crash-resume: session {sid} failed to auto-resume; amber chip remains", exc_info=True) self.crash_resume_queue = [] + @typechecked + def note_shutdown_stops(self) -> int: + """Stamp every chat with a live turn BEFORE the shutdown stops it. The lifespan stops the tasks + first and flushes second, so by flush time a running chat already reads "stopped" and the + note below never fired (dev kill matrix A9a, 2026-09-01). Returns how many were stamped.""" + stamped = 0 + for session_id in list(self.tasks.keys()): + session = self.sessions.get(session_id) + if session is None or session.status not in ("running", "waiting_approval"): + continue + session.messages.append(Message(role="system", content=SHUTDOWN_STOP_NOTE, branch_id=session.active_branch_id)) + stamped += 1 + return stamped + @typechecked async def persist_all_sessions(self) -> None: """Flush every in-memory session to JSON files (for graceful shutdown).""" for session_id, session in list(self.sessions.items()): if session.status in ("running", "waiting_approval"): session.status = "stopped" - # Say who stopped it. A chat flushed as plain "stopped" reads exactly like the user's own - # Stop, and when something else killed the backend (an agent's pkill, 2026-09-01) the - # user's running work vanished with nothing saying why: silent loss, row 1. - session.messages.append(Message(role="system", content=SHUTDOWN_STOP_NOTE, branch_id=session.active_branch_id)) + # A chat that was never a task (restored mid-turn, never resumed) still gets the note here. + if not session.messages or str(session.messages[-1].content) != SHUTDOWN_STOP_NOTE: + session.messages.append(Message(role="system", content=SHUTDOWN_STOP_NOTE, branch_id=session.active_branch_id)) session.closed_at = None for req in list(session.pending_approvals): ws_manager.resolve_approval(req.id, {"behavior": "deny", "message": "Server shutting down"}) diff --git a/backend/tests/test_external_kill_respawn.py b/backend/tests/test_external_kill_respawn.py index d4f06900..b2f03f68 100644 --- a/backend/tests/test_external_kill_respawn.py +++ b/backend/tests/test_external_kill_respawn.py @@ -16,9 +16,12 @@ def p_session() -> AgentSession: return AgentSession(name="t", model="sonnet") -def test_the_real_143_and_a_sigkill_137_are_recognised(): +def test_the_real_143_and_every_signal_spelling_are_recognised(): assert is_external_kill_error(RuntimeError(REAL)) assert is_external_kill_error(RuntimeError("Command failed with exit code 137 (exit code: 137)")) + # A SIGKILL cannot be caught, so the SDK reports it as a negative signal number, never 137 (live, dev kill matrix A3). + assert is_external_kill_error(RuntimeError("Command failed with exit code -9 (exit code: -9)\nError output: Check stderr output for details")) + assert is_external_kill_error(RuntimeError("Command failed with exit code -15 (exit code: -15)")) @pytest.mark.parametrize("innocent", [ @@ -26,6 +29,8 @@ def test_the_real_143_and_a_sigkill_137_are_recognised(): ("Command failed with exit code 143 (exit code: 143)", "API Error: 401 authentication_error"), ("Command failed with exit code 143 (exit code: 143)", "Error: request blocked by Usage Policy"), ("Command failed with exit code 1430", ""), + ("Command failed with exit code -1 (exit code: -1)", ""), + ("Command failed with exit code -90", ""), ("Error code: 429 - rate limit", ""), ]) def test_other_failures_never_claim_it(innocent): diff --git a/backend/tests/test_shutdown_stop_note.py b/backend/tests/test_shutdown_stop_note.py index af542800..bc263e76 100644 --- a/backend/tests/test_shutdown_stop_note.py +++ b/backend/tests/test_shutdown_stop_note.py @@ -36,3 +36,21 @@ def test_a_settled_chat_is_flushed_untouched(monkeypatch) -> None: doc = saved[s.id] assert doc["status"] == "completed" assert doc["messages"][-1]["role"] == "user", "no note on a chat that was not running" + + +def test_the_lifespan_stamps_live_turns_before_it_stops_them(monkeypatch) -> None: + """Placement, not just behaviour: stop_agent flips running -> stopped, so a note keyed on + "running" at flush time never fires for a chat that was a task (dev kill matrix A9a).""" + import inspect + from backend.apps.agents import agents as agents_mod + src = inspect.getsource(agents_mod.agents_lifespan) + assert src.index("note_shutdown_stops()") < src.index("stop_agent(session_id)") + s = p_session("running") + agent_manager.sessions.clear(); agent_manager.sessions[s.id] = s + monkeypatch.setitem(agent_manager.tasks, s.id, object()) + try: + assert agent_manager.note_shutdown_stops() == 1 + finally: + agent_manager.tasks.pop(s.id, None) + assert s.messages[-1].role == "system" and "not your Stop" in str(s.messages[-1].content) + assert agent_manager.note_shutdown_stops() == 0, "a settled or already-stamped chat is not stamped twice"