From dc807e09ef350aaaad739efabf63fbdb2c240a50 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Sun, 23 Aug 2026 21:09:23 -0700 Subject: [PATCH] [eric] agents: a drilled fault must be the one the real classifier catches, and fire once (ENG-382) Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018foyDoK19jjbYdudfzQVkZ --- backend/apps/agents/core/fault_injection.py | 20 ++++++ backend/apps/agents/manager/run/TurnRunner.py | 11 +-- .../streaming/handle_assistant_message.py | 9 +++ backend/tests/test_fault_injection.py | 68 ++++++++++++++++--- 4 files changed, 95 insertions(+), 13 deletions(-) diff --git a/backend/apps/agents/core/fault_injection.py b/backend/apps/agents/core/fault_injection.py index dfde3e50..89abec12 100644 --- a/backend/apps/agents/core/fault_injection.py +++ b/backend/apps/agents/core/fault_injection.py @@ -34,6 +34,26 @@ def armed(name: str) -> bool: return name in (wanted & KNOWN_FAULTS) +_FIRED: Set[str] = set() + + +def armed_once(name: str) -> bool: + """Fire a recoverable fault exactly ONCE per process. + + A fault that fires on every turn cannot drill a recovery: the retry hits the same wall and the + drill only ever proves the failure, never the heal. Recoverable classes (a dead pipe, a rotated + token) want one hit and then a clear road.""" + if not armed(name) or name in _FIRED: + return False + _FIRED.add(name) + return True + + +def reset_fired() -> None: + """Test-only: forget what has fired so a case can arm the same one-shot again.""" + _FIRED.clear() + + def unknown_faults() -> Set[str]: """Names asked for that this build does not know: surfaced loudly so a typo cannot read as a pass.""" raw = os.environ.get("OSW_FAULT", "") diff --git a/backend/apps/agents/manager/run/TurnRunner.py b/backend/apps/agents/manager/run/TurnRunner.py index b913fff0..35def361 100644 --- a/backend/apps/agents/manager/run/TurnRunner.py +++ b/backend/apps/agents/manager/run/TurnRunner.py @@ -45,7 +45,7 @@ class TurnRunner(AgentManagerProtocol): global_settings: AppSettings, force_respawn: bool = False) -> None: from claude_agent_sdk import query, ClaudeAgentOptions, AssistantMessage, ResultMessage from claude_agent_sdk.types import StreamEvent, SystemMessage - from backend.apps.agents.core.fault_injection import armed as p_fault_armed + from backend.apps.agents.core.fault_injection import armed as p_fault_armed, armed_once as p_fault_once # Deliberate faults, so the guards below get drilled instead of waited for. Inert unless # OSW_FAULT names them; a shipped build never sets it. Raised HERE because this is the same @@ -59,9 +59,12 @@ class TurnRunner(AgentManagerProtocol): ) if p_fault_armed("auth_401"): raise RuntimeError("API Error: 401 {\"error\":{\"type\":\"authentication_error\",\"message\":\"invalid x-api-key\"}}") - if p_fault_armed("transport_death"): - import anyio - raise anyio.BrokenResourceError() + # One-shot: a dead pipe is recoverable, so the drill needs the retry to find a clear road. + # The type must be one the REAL classifier calls a lost connection, or the drill quietly + # measures the unclassified fall-through instead of the transport heal (caught doing exactly + # that: anyio.BrokenResourceError is not in the transient set and read as a poisoned session). + if p_fault_once("transport_death"): + raise ConnectionError("injected transport death (OSW_FAULT)") async def prompt_stream(): yield { diff --git a/backend/apps/agents/manager/streaming/handle_assistant_message.py b/backend/apps/agents/manager/streaming/handle_assistant_message.py index 2d671b4f..c319b1db 100644 --- a/backend/apps/agents/manager/streaming/handle_assistant_message.py +++ b/backend/apps/agents/manager/streaming/handle_assistant_message.py @@ -261,6 +261,15 @@ async def handle_assistant_message( "message": p_card.model_dump(mode="json"), }) else: + # Drill seam: swallow the model's answer so the turn genuinely ends mute after tool work, + # which is the real shape of a silent quit. Forcing the DECISION further downstream was a + # proxy: it ran the guard while an answer still sat in the transcript, so it could never + # show whether the user actually gets their result back. + from backend.apps.agents.core.fault_injection import armed as p_fault_armed + if p_fault_armed("empty_finish"): + turn.stream_text_accum = "" + live_partial.pop(session_id, None) + return asst_msg = Message( id=turn.stream_text_msg_id or uuid4().hex, role="assistant", diff --git a/backend/tests/test_fault_injection.py b/backend/tests/test_fault_injection.py index ebd0126e..b61f84a4 100644 --- a/backend/tests/test_fault_injection.py +++ b/backend/tests/test_fault_injection.py @@ -9,10 +9,33 @@ classified the same way a REAL failure is, and that the harness is inert unless import re import pytest -from backend.apps.agents.core.fault_injection import KNOWN_FAULTS, armed, unknown_faults -from backend.apps.agents.core.error_classify import has_auth_status, is_content_policy_block +import builtins + +from backend.apps.agents.core.fault_injection import ( + KNOWN_FAULTS, armed, armed_once, reset_fired, unknown_faults, +) +from backend.apps.agents.core.error_classify import ( + has_auth_status, is_connection_lost, is_content_policy_block, +) TURN_RUNNER = "backend/apps/agents/manager/run/TurnRunner.py" +# Where each fault is raised. A fault this build knows but wires nowhere is the exact shape of a +# guard that never fires, so the map is the test, not a convenience. +WIRED_IN = { + "policy_block": TURN_RUNNER, + "auth_401": TURN_RUNNER, + "transport_death": TURN_RUNNER, + "empty_finish": "backend/apps/agents/manager/streaming/handle_assistant_message.py", +} + + +def p_block(kind: str) -> str: + """The source of the branch that fires one fault, whichever helper name arms it.""" + src = open(WIRED_IN[kind]).read() + for call in (f'p_fault_armed("{kind}")', f'p_fault_once("{kind}")'): + if call in src: + return src.split(call)[1].split("if p_fault_")[0] + raise AssertionError(f"{kind} is armed nowhere in {WIRED_IN[kind]}") @pytest.fixture(autouse=True) @@ -22,9 +45,17 @@ def p_clean(monkeypatch): def p_injected(kind: str) -> str: """The literal the harness raises, read out of the source so the test cannot drift from it.""" - src = open(TURN_RUNNER).read() - block = src.split(f'p_fault_armed("{kind}")')[1].split("if p_fault_armed")[0] - return "".join(re.findall(r'"((?:[^"\\]|\\.)*)"', block)).replace('\\"', '"') + return "".join(re.findall(r'"((?:[^"\\]|\\.)*)"', p_block(kind))).replace('\\"', '"') + + +def p_raised_type(kind: str) -> type: + """The exception CLASS the harness raises, resolved for real so a rename cannot pass.""" + name = re.search(r"raise\s+([A-Za-z_][\w.]*)\(", p_block(kind)).group(1) + if "." in name: + import importlib + mod, _, attr = name.rpartition(".") + return getattr(importlib.import_module(mod), attr) + return getattr(builtins, name) def test_inert_unless_armed(): @@ -53,7 +84,26 @@ def test_the_auth_fault_is_what_the_real_classifier_catches(): assert not has_auth_status("File runner.py, line 401, in execute") -def test_every_known_fault_is_wired_or_declared(): - src = open(TURN_RUNNER).read() - wired = {m for m in KNOWN_FAULTS if f'p_fault_armed("{m}")' in src} - assert {"policy_block", "auth_401", "transport_death"} <= wired +def test_the_transport_fault_is_what_the_real_classifier_catches(): + # Cost a wrong verdict once: anyio.BrokenResourceError is NOT in the transient set, so the drill + # measured the unclassified poisoned-session fall-through and read as "the transport heal is broken". + assert is_connection_lost(p_raised_type("transport_death")("injected")), \ + "the injected type must be one the real classifier calls a lost connection" + import anyio + assert not is_connection_lost(anyio.BrokenResourceError()), \ + "control: the type that actually fooled this drill must still read as NOT a lost connection" + + +def test_a_recoverable_fault_fires_once_so_the_retry_finds_a_clear_road(monkeypatch): + monkeypatch.setenv("OSW_FAULT", "transport_death") + reset_fired() + assert armed_once("transport_death") is True + assert armed_once("transport_death") is False, \ + "a fault that fires on every attempt proves the failure and never the heal" + + +def test_every_known_fault_is_wired_somewhere(): + assert set(WIRED_IN) == KNOWN_FAULTS - {"sidecar_wedge"}, \ + "a fault this build knows but wires nowhere is a guard that can never be drilled" + for kind in WIRED_IN: + assert p_block(kind).strip(), f"{kind} has an empty branch"