From 648f45f2fca9b827d202fe1742ffac83f2991027 Mon Sep 17 00:00:00 2001 From: Renn F Date: Sun, 28 Jun 2026 17:16:37 +0200 Subject: [PATCH] [F068][F069] mcp servers: classify all rejection shapes + envelope 404s MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F068: the do/flow-server circuit breaker only counted rejections whose `error` field was a STRING in _CIRCUIT_REJECTION_KINDS. A 422 validation failure (no `error` field, a `detail` list) and a 500/HTTPException (dict-shaped `error` from the exception handlers) both bypassed the breaker → unbounded retries on a storm of either. Added _classify_rejection(payload) (shared, applied to both servers) mapping all three shapes to a counted kind: string error (existing), dict error → substring-mapped code (*DENIED*/*AUTHORIZED*/*FORBIDDEN*/*PERMISSION*→not_authorized, INVALID_INPUT/*VALIDATION*→incomplete_input, *NOT_FOUND*→None parity, else →invalid_state), 422 detail→incomplete_input. The dict TypeError defence lives in the classifier (isinstance, never dict-in-frozenset). F069: a manifest-registered verb whose HTTP route is missing got FastAPI's raw `{"detail":"Not Found"}` 404 body — a non-envelope payload the breaker couldn't classify, so a storm bypassed it. _post now synthesizes an invalid_state Envelope rejection (with a remediate hint → i_am_blocked/i_am_idle) for a 404 status, routed through _record_and_check_circuit so the breaker counts it. A 404 that carries a real Envelope (error field present) is surfaced as-is, preserving test_flow_post_returns_envelope_on_404. TDD: 422/dict/404 tests in both server test files; updated test_dict_shaped_error_does_not_crash to assert the SDK is now called with not_authorized (replacing the pass-through assertion that encoded the bug). --- roboco/mcp/do_server.py | 128 +++++++++++- roboco/mcp/flow_server.py | 128 +++++++++++- .../test_do_server_circuit_breaker.py | 182 +++++++++++++++++- .../test_flow_server_circuit_breaker.py | 160 +++++++++++++++ 4 files changed, 579 insertions(+), 19 deletions(-) diff --git a/roboco/mcp/do_server.py b/roboco/mcp/do_server.py index ce9ce1ae..29ee20c7 100644 --- a/roboco/mcp/do_server.py +++ b/roboco/mcp/do_server.py @@ -35,6 +35,11 @@ AGENT_ROLE = os.environ["ROBOCO_AGENT_ROLE"] _TIMEOUT = 30 # Tight timeout for SDK loopback — local sidecar; gateway path must not stall. _SDK_TIMEOUT = 2.0 +# FastAPI's default missing-route status. Every /api/v1/do/* route returns +# 200 with an Envelope (including not_found rejections), so a 404 from the +# orchestrator is always a manifest-registered tool whose HTTP route is +# missing — F069 synthesizes an invalid_state Envelope for it. +_MISSING_ROUTE_STATUS = 404 # Envelope error kinds that count toward the per-verb circuit breaker. # Mirrors flow_server._CIRCUIT_REJECTION_KINDS — agent_sdk.server is the @@ -46,6 +51,73 @@ _CIRCUIT_REJECTION_KINDS: frozenset[str] = frozenset( {"tracing_gap", "invalid_state", "not_authorized", "incomplete_input"} ) + +# Dict-shaped `error.code` values (from FastAPI's exception handlers — +# `roboco_exception_handler` / `http_exception_handler` / `generic_exception_handler`) +# mapped to the counted breaker kind they are semantically equivalent to. F068: +# a 422 / 500 / 4xx-exception storm is retry-storm-worthy but the response body +# carries `error` as a DICT (not a string kind), so the breaker's string-only +# check skipped it — unbounded retries. We classify by `error.code` so the SDK +# actually records the attempt. Kinds not in `_CIRCUIT_REJECTION_KINDS` are +# never forwarded (the SDK ignores unknown kinds anyway). +# +# Classification is substring-based so the many custom RobocoError codes +# (A2A_ACCESS_DENIED, NO_WRITE_ACCESS, TASK_NOT_OWNED, …) land on the right +# counted kind without an exhaustive literal map. The NOT_FOUND family returns +# None — parity with the string-error contract that a `not_found` rejection +# does NOT count (retrying a missing resource won't help until state changes). +def _classify_dict_error_code(code: str) -> str | None: + upper = code.upper() + if "NOT_FOUND" in upper: + return None + if ( + "DENIED" in upper + or "AUTHORIZED" in upper + or "FORBIDDEN" in upper + or "PERMISSION" in upper + ): + return "not_authorized" + if upper == "INVALID_INPUT" or "VALIDATION" in upper: + return "incomplete_input" + return "invalid_state" + + +def _classify_rejection(payload: dict[str, Any]) -> str | None: + """Return the breaker kind to forward for this payload, or None. + + The breaker only counts rejections whose kind is in + ``_CIRCUIT_REJECTION_KINDS`` (the SDK's authoritative catalog). Three + reachable rejection shapes must all map to a counted kind so a storm of + any of them trips the breaker (F068): + + 1. Envelope rejection: ``error`` is a STRING kind. Forward it if in + the counted set (existing behaviour). Uncounted string kinds (e.g. + ``not_found``, ``transport_error``, ``circuit_open``) return None — + preserves the prior contract that those don't touch the SDK. + 2. Exception-handler dict: ``error`` is a DICT + (``{code, message, details?}`` from ``roboco_exception_handler`` / + ``http_exception_handler`` / ``generic_exception_handler``). Map its + ``code`` to a counted kind — auth/permission/denied → ``not_authorized``, + ``INVALID_INPUT`` / validation → ``incomplete_input``, anything else + (INTERNAL_ERROR, API_ERROR, TASK_WRONG_STATUS, …) → ``invalid_state``. + NOT_FOUND-family codes return None (parity with string ``not_found``). + 3. 422 validation failure: no ``error`` field, a ``detail`` list + (``request_validation_handler``). → ``incomplete_input``. + + Successful envelopes (``status`` set, ``error`` None) and uncounted + string kinds return None — the SDK is not touched. + """ + error = payload.get("error") + if isinstance(error, str): + return error if error in _CIRCUIT_REJECTION_KINDS else None + if isinstance(error, dict): + return _classify_dict_error_code(str(error.get("code") or "")) + if "detail" in payload: + # 422 request-validation body ({"detail": [...], "body": ...}). + return "incomplete_input" + return None + + mcp = FastMCP("roboco-do") log = structlog.get_logger() @@ -84,6 +156,45 @@ def _post(path: str, body: dict[str, Any]) -> dict[str, Any]: headers=_build_headers(), json=body, ) + # F069: a 404 here means a manifest-registered content tool has no + # matching route on the orchestrator (every /api/v1/do/* route + # returns 200 with an Envelope — including not_found rejections — so + # a 404 status with FastAPI's default body (``{"detail": "Not + # Found"}``, no ``error`` field) is always a missing route, never a + # legit Envelope). That body is a non-envelope payload the breaker + # can't classify, so a storm of these bypassed the circuit breaker → + # unbounded retries on a tool that can never succeed. Synthesize an + # ``invalid_state`` Envelope rejection so the breaker counts it (via + # ``_classify_rejection``) and the agent gets a remediation hint + # instead of a raw ``detail`` body. A 404 that DOES carry a real + # Envelope (an ``error`` field) is surfaced as-is. Mirrors + # flow_server._post. + if response.status_code == _MISSING_ROUTE_STATUS: + try: + body_404 = response.json() + except (ValueError, json.JSONDecodeError): + body_404 = None + if isinstance(body_404, dict) and "error" in body_404: + payload_404: dict[str, Any] = body_404 + else: + verb = _verb_from_path(path) + payload_404 = { + "error": "invalid_state", + "message": ( + f"content tool '{verb}' has no route on the" + f" orchestrator (path {path})" + ), + "remediate": ( + f"the {verb} tool is advertised in your manifest but" + f" its HTTP route is missing — this is a server-side" + f" wiring gap. Call" + f" i_am_blocked(reason='tool {verb} 404s: no route')" + f" or i_am_idle() so the operator can fix the route;" + f" do not retry." + ), + "missing": [], + } + return _record_and_check_circuit(path, body, payload_404) try: payload: dict[str, Any] = response.json() except (ValueError, json.JSONDecodeError): @@ -131,14 +242,15 @@ def _record_and_check_circuit( break the gateway path. """ # Gateway envelopes use a string `error` (kind); RobocoError-derived - # exceptions surface a dict-shaped error via FastAPI's middleware - # (a TypeError on `dict in frozenset`). Defend against the - # dict shape — only string kinds count toward the breaker, dicts pass - # straight through. - rejection_kind = payload.get("error") - if not isinstance(rejection_kind, str): - return payload - if rejection_kind not in _CIRCUIT_REJECTION_KINDS: + # exceptions surface a dict-shaped error via FastAPI's middleware, and + # 422 validation failures carry a `detail` list with no `error` field + # at all. F068: classify all three rejection shapes so a storm of 500s + # or 422s counts toward the breaker (previously bypassed → unbounded + # retries). The dict-shape defence against `TypeError: unhashable type: + # 'dict'` lives in `_classify_rejection` (isinstance checks, never a + # `dict in frozenset` membership test). + rejection_kind = _classify_rejection(payload) + if rejection_kind is None: return payload verb = _verb_from_path(path) diff --git a/roboco/mcp/flow_server.py b/roboco/mcp/flow_server.py index 27c23df4..943e3f02 100644 --- a/roboco/mcp/flow_server.py +++ b/roboco/mcp/flow_server.py @@ -53,6 +53,11 @@ _TIMEOUT = 30 # Tight timeout for SDK loopback — the SDK is a local sidecar; anything # slower than 2s is unhealthy and the gateway path must not stall on it. _SDK_TIMEOUT = 2.0 +# FastAPI's default missing-route status. Every gateway route returns 200 +# with an Envelope (including not_found rejections), so a 404 from the +# orchestrator is always a manifest-registered verb whose HTTP route is +# missing — F069 synthesizes an invalid_state Envelope for it. +_MISSING_ROUTE_STATUS = 404 # Envelope error kinds that count toward the per-verb circuit breaker. # Mirrors agent_sdk.server._CIRCUIT_REJECTION_KINDS; the SDK is the @@ -62,6 +67,73 @@ _CIRCUIT_REJECTION_KINDS: frozenset[str] = frozenset( {"tracing_gap", "invalid_state", "not_authorized", "incomplete_input"} ) + +# Dict-shaped `error.code` values (from FastAPI's exception handlers — +# `roboco_exception_handler` / `http_exception_handler` / `generic_exception_handler`) +# mapped to the counted breaker kind they are semantically equivalent to. F068: +# a 422 / 500 / 4xx-exception storm is retry-storm-worthy but the response body +# carries `error` as a DICT (not a string kind), so the breaker's string-only +# check skipped it — unbounded retries. We classify by `error.code` so the SDK +# actually records the attempt. Kinds not in `_CIRCUIT_REJECTION_KINDS` are +# never forwarded (the SDK ignores unknown kinds anyway). +# +# Classification is substring-based so the many custom RobocoError codes +# (A2A_ACCESS_DENIED, NO_WRITE_ACCESS, TASK_NOT_OWNED, …) land on the right +# counted kind without an exhaustive literal map. The NOT_FOUND family returns +# None — parity with the string-error contract that a `not_found` rejection +# does NOT count (retrying a missing resource won't help until state changes). +def _classify_dict_error_code(code: str) -> str | None: + upper = code.upper() + if "NOT_FOUND" in upper: + return None + if ( + "DENIED" in upper + or "AUTHORIZED" in upper + or "FORBIDDEN" in upper + or "PERMISSION" in upper + ): + return "not_authorized" + if upper == "INVALID_INPUT" or "VALIDATION" in upper: + return "incomplete_input" + return "invalid_state" + + +def _classify_rejection(payload: dict[str, Any]) -> str | None: + """Return the breaker kind to forward for this payload, or None. + + The breaker only counts rejections whose kind is in + ``_CIRCUIT_REJECTION_KINDS`` (the SDK's authoritative catalog). Three + reachable rejection shapes must all map to a counted kind so a storm of + any of them trips the breaker (F068): + + 1. Envelope rejection: ``error`` is a STRING kind. Forward it if in + the counted set (existing behaviour). Uncounted string kinds (e.g. + ``not_found``, ``transport_error``, ``circuit_open``) return None — + preserves the prior contract that those don't touch the SDK. + 2. Exception-handler dict: ``error`` is a DICT + (``{code, message, details?}`` from ``roboco_exception_handler`` / + ``http_exception_handler`` / ``generic_exception_handler``). Map its + ``code`` to a counted kind — auth/permission/denied → ``not_authorized``, + ``INVALID_INPUT`` / validation → ``incomplete_input``, anything else + (INTERNAL_ERROR, API_ERROR, TASK_WRONG_STATUS, …) → ``invalid_state``. + NOT_FOUND-family codes return None (parity with string ``not_found``). + 3. 422 validation failure: no ``error`` field, a ``detail`` list + (``request_validation_handler``). → ``incomplete_input``. + + Successful envelopes (``status`` set, ``error`` None) and uncounted + string kinds return None — the SDK is not touched. + """ + error = payload.get("error") + if isinstance(error, str): + return error if error in _CIRCUIT_REJECTION_KINDS else None + if isinstance(error, dict): + return _classify_dict_error_code(str(error.get("code") or "")) + if "detail" in payload: + # 422 request-validation body ({"detail": [...], "body": ...}). + return "incomplete_input" + return None + + mcp = FastMCP("roboco-flow") log = structlog.get_logger() @@ -104,6 +176,47 @@ def _post(path: str, body: dict[str, Any]) -> dict[str, Any]: headers=_build_headers(), json=body, ) + # F069: a 404 here means a manifest-registered verb has no matching + # route on the orchestrator (every gateway route returns 200 with an + # Envelope — including not_found rejections — so a 404 status with + # FastAPI's default body (``{"detail": "Not Found"}``, no ``error`` + # field) is always a missing route, never a legit Envelope). That + # body is a non-envelope payload the breaker can't classify, so a + # storm of these bypassed the circuit breaker → unbounded retries on + # a verb that can never succeed. Synthesize an ``invalid_state`` + # Envelope rejection so the breaker counts it (via + # ``_classify_rejection``) and the agent gets a remediation hint + # instead of a raw ``detail`` body. A 404 that DOES carry a real + # Envelope (an ``error`` field — e.g. a proxy re-status a 200 + # rejection to 404) is surfaced as-is. + if response.status_code == _MISSING_ROUTE_STATUS: + try: + body_404 = response.json() + except (ValueError, json.JSONDecodeError): + body_404 = None + if isinstance(body_404, dict) and "error" in body_404: + # Real Envelope rejection surfaced under a 404 status — + # surface it as-is so the agent sees the real kind/remediate. + payload_404: dict[str, Any] = body_404 + else: + verb = _verb_from_path(path) + payload_404 = { + "error": "invalid_state", + "message": ( + f"verb '{verb}' has no route on the orchestrator for" + f" role {AGENT_ROLE!r} (path {path})" + ), + "remediate": ( + f"the {verb} verb is advertised in your manifest but" + f" its HTTP route is missing — this is a server-side" + f" wiring gap. Call" + f" i_am_blocked(reason='verb {verb} 404s: no route')" + f" or i_am_idle() so the operator can fix the route;" + f" do not retry." + ), + "missing": [], + } + return _record_and_check_circuit(path, body, payload_404) try: payload: dict[str, Any] = response.json() except (ValueError, json.JSONDecodeError): @@ -155,12 +268,15 @@ def _record_and_check_circuit( break the gateway path. """ # Gateway envelopes use a string `error` (kind); RobocoError-derived - # exceptions surface a dict-shaped error via FastAPI's middleware. Only - # string kinds count toward the breaker; dicts pass straight through. - rejection_kind = payload.get("error") - if not isinstance(rejection_kind, str): - return payload - if rejection_kind not in _CIRCUIT_REJECTION_KINDS: + # exceptions surface a dict-shaped error via FastAPI's middleware, and + # 422 validation failures carry a `detail` list with no `error` field + # at all. F068: classify all three rejection shapes so a storm of 500s + # or 422s counts toward the breaker (previously bypassed → unbounded + # retries). The dict-shape defence against `TypeError: unhashable type: + # 'dict'` lives in `_classify_rejection` (isinstance checks, never a + # `dict in frozenset` membership test). + rejection_kind = _classify_rejection(payload) + if rejection_kind is None: return payload verb = _verb_from_path(path) diff --git a/tests/unit/mcp_servers/test_do_server_circuit_breaker.py b/tests/unit/mcp_servers/test_do_server_circuit_breaker.py index 03af60e3..d1521ed3 100644 --- a/tests/unit/mcp_servers/test_do_server_circuit_breaker.py +++ b/tests/unit/mcp_servers/test_do_server_circuit_breaker.py @@ -263,12 +263,19 @@ def test_verb_extracted_from_path(do_module: types.ModuleType) -> None: def test_dict_shaped_error_does_not_crash(do_module: types.ModuleType) -> None: - """A RobocoError.to_dict()-shaped response must pass through without TypeError. + """A RobocoError.to_dict()-shaped response must not TypeError the breaker. Smoke-7: A2AAccessDeniedError escaped to middleware and was rendered as {'error': {'code': ..., 'message': ..., 'details': ...}}. The circuit breaker's `error in frozenset` check then crashed with `TypeError: unhashable type: 'dict'`. + + F068: a dict-shaped `error` is a retry-storm-worthy rejection (the + orchestrator's exception handlers all surface this shape on 4xx/5xx), + so the breaker must COUNT it — mapped to a counted kind by the + classifier — rather than passing it through silently. The original + dict payload still reaches the agent (the breaker only substitutes + when open). No TypeError may be raised either way. """ factory, captured = _make_client( orchestrator_response={ @@ -278,12 +285,177 @@ def test_dict_shaped_error_does_not_crash(do_module: types.ModuleType) -> None: "details": {}, } }, - sdk_response=None, # SDK must not be touched + sdk_response={ + "verb": "dm", + "task_id": None, + "attempts": 1, + "limit": 3, + "window_seconds": 60, + "open": False, + "circuit_envelope": None, + }, ) - # No TypeError; payload passes through untouched. + # No TypeError; the dict-shaped rejection is forwarded to the SDK. with patch("httpx.Client", side_effect=factory): result = do_module.dm(recipient="qa-all", text="x") + # Original dict payload still reaches the agent (breaker not open). assert isinstance(result["error"], dict) assert result["error"]["code"] == "A2A_ACCESS_DENIED" - # SDK breaker MUST NOT have been called for a non-string error. - assert all("test-sdk" not in url for url, _ in captured) + # SDK breaker MUST now be called so a storm of these counts. + sdk_calls = [(url, body) for url, body in captured if "test-sdk" in url] + assert len(sdk_calls) == 1 + # ACCESS_DENIED maps to the not_authorized counted kind. + assert sdk_calls[0][1]["rejection_kind"] == "not_authorized" + + +def test_422_validation_failure_counts_as_incomplete_input( + do_module: types.ModuleType, +) -> None: + """F068: a 422 validation-failure body (`{"detail": [...], "body": ...}`, + no `error` field) must count toward the breaker — a storm of 422s is + retry-storm-worthy (the agent keeps re-submitting malformed input). + Mapped to `incomplete_input` (the agent's input was incomplete/invalid). + """ + factory, captured = _make_client( + orchestrator_response={ + "detail": [ + { + "loc": ["body", "text"], + "msg": "field required", + "type": "value_error.missing", + } + ], + "body": None, + }, + sdk_response={ + "verb": "note", + "task_id": None, + "attempts": 1, + "limit": 3, + "window_seconds": 60, + "open": False, + "circuit_envelope": None, + }, + ) + with patch("httpx.Client", side_effect=factory): + do_module.note(text="") + sdk_calls = [(url, body) for url, body in captured if "test-sdk" in url] + assert len(sdk_calls) == 1 + assert sdk_calls[0][1]["rejection_kind"] == "incomplete_input" + + +def test_dict_shaped_internal_error_counts_as_invalid_state( + do_module: types.ModuleType, +) -> None: + """F068: a 500 INTERNAL_ERROR dict-shaped response (generic_exception_handler) + must count toward the breaker as `invalid_state` — a storm of 500s is + retry-storm-worthy and previously bypassed the breaker entirely. + """ + factory, captured = _make_client( + orchestrator_response={ + "error": { + "code": "INTERNAL_ERROR", + "message": "An internal error occurred", + "details": {"correlation_id": "abc"}, + } + }, + sdk_response={ + "verb": "commit", + "task_id": None, + "attempts": 1, + "limit": 3, + "window_seconds": 60, + "open": False, + "circuit_envelope": None, + }, + ) + with patch("httpx.Client", side_effect=factory): + do_module.commit(message="[abc12345] a valid commit message here") + sdk_calls = [(url, body) for url, body in captured if "test-sdk" in url] + assert len(sdk_calls) == 1 + assert sdk_calls[0][1]["rejection_kind"] == "invalid_state" + + +def test_dict_shaped_invalid_input_counts_as_incomplete_input( + do_module: types.ModuleType, +) -> None: + """F068: a dict-shaped INVALID_INPUT (mapped from 422 by http_exception_handler) + counts as `incomplete_input` — semantically the agent's input was invalid. + """ + factory, captured = _make_client( + orchestrator_response={ + "error": {"code": "INVALID_INPUT", "message": "bad payload"} + }, + sdk_response={ + "verb": "say", + "task_id": None, + "attempts": 1, + "limit": 3, + "window_seconds": 60, + "open": False, + "circuit_envelope": None, + }, + ) + with patch("httpx.Client", side_effect=factory): + do_module.say(channel="backend-cell", text="x") + sdk_calls = [(url, body) for url, body in captured if "test-sdk" in url] + assert len(sdk_calls) == 1 + assert sdk_calls[0][1]["rejection_kind"] == "incomplete_input" + + +# --------------------------------------------------------------------------- +# F069 — a manifest-registered content tool whose route is missing must +# return an envelope rejection (not a raw 404 body) so the breaker counts it. +# --------------------------------------------------------------------------- + + +def test_missing_route_404_returns_envelope_and_counts( + do_module: types.ModuleType, +) -> None: + """F069: a 404 from the orchestrator (manifest-registered tool with no + route) must surface as a proper `invalid_state` Envelope rejection — not + FastAPI's raw ``{"detail": "Not Found"}`` body — and the breaker must + count it. Without this, the agent retries the missing tool forever and + the breaker never trips. Mirrors flow_server's 404 handling. + """ + captured: list[tuple[str, dict[str, Any] | None]] = [] + + def _client_factory(*_args: Any, **_kwargs: Any) -> MagicMock: + client = MagicMock() + client.__enter__ = MagicMock(return_value=client) + client.__exit__ = MagicMock(return_value=False) + + def _post(url: str, **kwargs: Any) -> MagicMock: + captured.append((url, kwargs.get("json"))) + resp = MagicMock() + if "test-sdk" in url: + resp.json.return_value = { + "verb": "evidence", + "task_id": None, + "attempts": 1, + "limit": 3, + "window_seconds": 60, + "open": False, + "circuit_envelope": None, + } + else: + # FastAPI's default 404 for a missing route. + resp.status_code = 404 + resp.json.return_value = {"detail": "Not Found"} + return resp + + client.post.side_effect = _post + return client + + with patch("httpx.Client", side_effect=_client_factory): + result = do_module.evidence(task_id="some-task") + # Envelope rejection, not the raw 404 body. + assert result["error"] == "invalid_state" + assert "remediate" in result + assert "detail" not in result # the raw 404 body was not passed through + # Breaker was notified so a storm of these trips it. + sdk_calls = [(url, body) for url, body in captured if "test-sdk" in url] + assert len(sdk_calls) == 1 + _, sdk_body = sdk_calls[0] + assert sdk_body is not None + assert sdk_body["rejection_kind"] == "invalid_state" diff --git a/tests/unit/mcp_servers/test_flow_server_circuit_breaker.py b/tests/unit/mcp_servers/test_flow_server_circuit_breaker.py index 2eaa6a91..1143321d 100644 --- a/tests/unit/mcp_servers/test_flow_server_circuit_breaker.py +++ b/tests/unit/mcp_servers/test_flow_server_circuit_breaker.py @@ -429,3 +429,163 @@ def test_task_id_none_is_forwarded_as_null(flow_module: types.ModuleType) -> Non _, sdk_body = sdk_calls[0] assert sdk_body["task_id"] is None assert sdk_body["verb"] == "give_me_work" + + +# --------------------------------------------------------------------------- +# F068 — non-string-error / no-error-field rejection shapes are counted +# --------------------------------------------------------------------------- + + +def test_422_validation_failure_counts_as_incomplete_input( + flow_module: types.ModuleType, +) -> None: + """F068: a 422 validation-failure body (`{"detail": [...]}`, no `error`) + must count toward the breaker as `incomplete_input` — a storm of 422s is + retry-storm-worthy. Mirrors do_server's classifier (the two servers share + the same breaker logic and must stay in parity). + """ + factory, captured = _make_client( + orchestrator_response={ + "detail": [ + { + "loc": ["body", "task_id"], + "msg": "field required", + "type": "value_error.missing", + } + ], + "body": None, + }, + sdk_response={ + "verb": "i_am_done", + "task_id": "task-A", + "attempts": 1, + "limit": 3, + "window_seconds": 60, + "open": False, + "circuit_envelope": None, + }, + ) + with patch("httpx.Client", side_effect=factory): + flow_module.i_am_done("task-A") + sdk_calls = [(url, body) for url, body in captured if "test-sdk" in url] + assert len(sdk_calls) == 1 + assert sdk_calls[0][1]["rejection_kind"] == "incomplete_input" + + +def test_dict_shaped_internal_error_counts_as_invalid_state( + flow_module: types.ModuleType, +) -> None: + """F068: a 500 INTERNAL_ERROR dict-shaped response (generic_exception_handler) + counts as `invalid_state` — a storm of 500s previously bypassed the breaker. + """ + factory, captured = _make_client( + orchestrator_response={ + "error": { + "code": "INTERNAL_ERROR", + "message": "An internal error occurred", + "details": {"correlation_id": "abc"}, + } + }, + sdk_response={ + "verb": "i_am_done", + "task_id": "task-A", + "attempts": 1, + "limit": 3, + "window_seconds": 60, + "open": False, + "circuit_envelope": None, + }, + ) + with patch("httpx.Client", side_effect=factory): + flow_module.i_am_done("task-A") + sdk_calls = [(url, body) for url, body in captured if "test-sdk" in url] + assert len(sdk_calls) == 1 + assert sdk_calls[0][1]["rejection_kind"] == "invalid_state" + + +def test_dict_shaped_not_found_does_not_count( + flow_module: types.ModuleType, +) -> None: + """F068: a dict-shaped NOT_FOUND (404 family) does NOT count — parity with + the string-error contract that a `not_found` rejection isn't counted + (retrying a missing resource won't help until state changes). + """ + factory, captured = _make_client( + orchestrator_response={ + "error": {"code": "TASK_NOT_FOUND", "message": "no such task"} + }, + sdk_response=None, # SDK must not be touched + ) + with patch("httpx.Client", side_effect=factory): + result = flow_module.i_am_done("task-A") + assert isinstance(result["error"], dict) + assert result["error"]["code"] == "TASK_NOT_FOUND" + assert all("test-sdk" not in url for url, _ in captured) + + +# --------------------------------------------------------------------------- +# F069 — a manifest-registered verb whose route is missing must return an +# envelope rejection (not a raw 404 body) so the breaker counts it. +# --------------------------------------------------------------------------- + + +def _make_404_client() -> tuple[Any, list[tuple[str, dict[str, Any] | None]]]: + """Build an httpx.Client mock whose orchestrator call returns FastAPI's + default 404 body (``{"detail": "Not Found"}``, status 404) — the shape a + manifest-registered verb sees when its route is missing. The SDK call + returns a not-yet-open breaker so the original envelope is preserved. + """ + captured: list[tuple[str, dict[str, Any] | None]] = [] + + def _client_factory(*_args: Any, **_kwargs: Any) -> MagicMock: + client = MagicMock() + client.__enter__ = MagicMock(return_value=client) + client.__exit__ = MagicMock(return_value=False) + + def _post(url: str, **kwargs: Any) -> MagicMock: + captured.append((url, kwargs.get("json"))) + resp = MagicMock() + if "test-sdk" in url: + resp.json.return_value = { + "verb": "triage", + "task_id": None, + "attempts": 1, + "limit": 3, + "window_seconds": 60, + "open": False, + "circuit_envelope": None, + } + else: + # FastAPI's default 404 for a missing route. + resp.status_code = 404 + resp.json.return_value = {"detail": "Not Found"} + return resp + + client.post.side_effect = _post + return client + + return _client_factory, captured + + +def test_missing_route_404_returns_envelope_and_counts( + flow_module: types.ModuleType, +) -> None: + """F069: a 404 from the orchestrator (manifest-registered verb with no + route) must surface as a proper `invalid_state` Envelope rejection — not + FastAPI's raw ``{"detail": "Not Found"}`` body — and the breaker must + count it. Without this, the agent retries the missing route forever and + the breaker never trips. + """ + factory, captured = _make_404_client() + with patch("httpx.Client", side_effect=factory): + result = flow_module.triage() + # Envelope rejection, not the raw 404 body. + assert result["error"] == "invalid_state" + assert "remediate" in result + assert "detail" not in result # the raw 404 body was not passed through + # Breaker was notified so a storm of these trips it. + sdk_calls = [(url, body) for url, body in captured if "test-sdk" in url] + assert len(sdk_calls) == 1 + _, sdk_body = sdk_calls[0] + assert sdk_body is not None + assert sdk_body["rejection_kind"] == "invalid_state"