fix(sidecar): keep transient-rejected preflight routes as deferred failover instead of discarding them

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 - Advanced the central sidecar's pinned immutable CO revision from `2e414d15` to protected `main@414f22973658c4ddc3d4320fcf7acd9b4e8ba991`, carrying contextual-orchestrator#1081's fix into Strix, OpenCode, and Noema. Root cause: `TaskOrchestrator._invoke`'s own retry-then-failover decision for a retryable 5xx (budgeted `1 + tool_retry_attempts` real tries per candidate) was getting multiplied by `ModelClient._send_with_retry`'s independent transient-retry-with-backoff underneath it (`max_retries + 1` further tries per call) -- up to 6 real network attempts against one already-flagged-flaky `orchestrator/free` agent before `_invoke` ever tried the next ranked candidate. Confirmed as the cause of independently observed incidents in #1912, #1231, #1503, and #1198, each spending 9-57+ minutes on one escalated route and surfacing that same route's model in its final error, never reaching a cleanly-ready sibling preflight had already found. The fix (`ModelClient.single_attempt_transport()`) changes only which agent gets tried next; no per-attempt timeout changed. Reproduced the bug directly against unmodified contextual-orchestrator `main` before the fix (6 real attempts) and confirmed the fix resolves it (<=2) before advancing this pin. `docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md`'s 2026-09-06 amendment and `tests/test_contextual_orchestrator_review_sidecar_contract.py`'s `ORCH_PIN_SHA` were updated alongside this pin. All callers still consume an exact SHA; no branch or tag is introduced.
 
+### Review sidecar preflight keeps transient-rejected routes as deferred failover
+
+- `_preflight_review_agents` no longer discards a route whose 16-token probe answered with a status the serving gateway itself retries and fails over across (`408 409 425 429 500 502 503 504 529`, the vendored orchestrator's `TRANSIENT_HTTP_STATUS`). Such routes are kept as **deferred**, ranked after every ready route by a catalog-priority penalty, so a stalled or rate-limited ready route has somewhere to fail over to; `ready_count` is unchanged, a new `deferred_count` is reported, and `rejected_count` covers only routes the gateway would not retry either (404, auth failures, invalid responses). With no ready route the stage still fails as before, so ADR-0005's priced-catalog fallback contract is untouched. Motivation: `noema-review` run 33993637015 (2026-09-05) rejected 11 of 12 routes -- six with 429, three of them on NVIDIA keys whose sibling routes were ready -- served the single ready route for 542 s and returned 502; under this rule the same run would have served 1 ready + 6 deferred. The sanitized stream gains a `preflight_route_deferred` line alongside `preflight_route_rejected`.
+
 ### Noema review ships sidecar evidence on failure
 
 - `noema-review.yml` now uploads `strix_runs/contextual-orchestrator-sidecar.stderr.log` and `strix_runs/contextual-orchestrator-preflight.json` as the `noema-sidecar-evidence` artifact when the verdict phase fails (`if: failure()`, the same pinned `actions/upload-artifact` Strix uses, `if-no-files-found: ignore`, 5-day retention). Until now a failed Noema run left `artifacts=0` -- run `33981136873` spent 3122 s walking six ready routes twice each and ended in HTTP 502 with no per-route trace anywhere but the sidecar's stderr -- so the only diagnosis available was the caller's one-line summary. The stderr file is the sanitizer's bounded allowlist output (`sanitize_contextual_orchestrator_sidecar_stream.py`), the same file Strix already publishes in `strix-reports`; per-attempt route outcomes still need an allowlisted structured line from the orchestrator to appear in it. Refs #1935, #1939.

scripts/ci/contextual_orchestrator_review_launcher.py

@@ -22,6 +22,8 @@
 from __future__ import annotations
 
 import argparse
+import copy
+import dataclasses
 import json
 import logging
 import os
@@ -63,6 +65,28 @@
 # Shared cap on how many candidates in one preflight run may use the
 # escalation retry above. It bounds request count, never model response time.
 REVIEW_PREFLIGHT_MAX_ESCALATIONS = 4
+# Probe outcomes the serving gateway itself treats as transient -- it retries
+# the same route and then fails over across exactly these statuses
+# (contextual_orchestrator.orchestrator.TRANSIENT_HTTP_STATUS at the vendored
+# pin; provider_errors.PROVIDER_STATUS_SURFACES marks 429 retryable). A route
+# that answered one of them to the 16-token probe is not known to be dead; it
+# was rate-limited or unlucky in the second the probe ran, very often because
+# the probe itself spent the per-key budget. Discarding it left the serving
+# set with nothing to fail over to: on 2026-09-05 a noema-review preflight
+# rejected 11 of 12 routes -- six of them with 429 -- served the one ready
+# route for 542 s and returned 502. Such routes are kept as *deferred*, ranked
+# after every ready route, so failover has somewhere to go. Only a route that
+# *answered* with one of these statuses qualifies (a probe that timed out
+# records no http_status and stays rejected), so deferral never admits, on the
+# strength of a probe that already showed it, the silent route whose serving
+# request would spend the gateway's full retry budget in 90 s timeouts. Keep
+# this set in sync with the vendored orchestrator's; a status the gateway
+# would not retry must not be deferred.
+REVIEW_PREFLIGHT_DEFERRABLE_HTTP_STATUS = frozenset({408, 409, 425, 429, 500, 502, 503, 504, 529})
+# Subtracted from a deferred route's catalog priority so the orchestrator's
+# ranking (higher priority first; catalog priorities are 0..-11) never places a
+# deferred route ahead of a ready one.
+REVIEW_PREFLIGHT_DEFERRED_PRIORITY_PENALTY = 1000
 
 
 class ReviewPreflightError(RuntimeError):
@@ -280,6 +304,22 @@ def _record_provider_exception(row: dict[str, object], exc: Exception) -> None:
     row.pop("reasoning_without_content", None)
 
 
+def _demote_agent(agent: object, penalty: int) -> object:
+    """Return a copy of ``agent`` whose ``priority`` is lowered by ``penalty``.
+
+    Serving agents are frozen ``ModelAgent`` dataclasses, so the copy goes
+    through :func:`dataclasses.replace`; the plain objects tests use are
+    shallow-copied and assigned. A missing ``priority`` counts as 0, matching
+    the dataclass default.
+    """
+    priority = int(getattr(agent, "priority", 0)) - penalty
+    if dataclasses.is_dataclass(agent) and not isinstance(agent, type):
+        return dataclasses.replace(agent, priority=priority)
+    demoted = copy.copy(agent)
+    demoted.priority = priority
+    return demoted
+
+
 def _response_has_reasoning_without_content(response: object) -> bool:
     """Return whether a response matches the vendored "reasoning, no content" signature.
 
@@ -508,11 +548,28 @@ def _preflight_review_agents(
         )
         routes.append(row)
 
+    # Deferral pass: a route rejected with a status the serving gateway would
+    # retry and fail over across is kept behind the ready routes instead of
+    # being discarded -- but only once at least one route is ready. With no
+    # ready route the run still fails this stage exactly as before, so
+    # _preflight_with_fallback's "priced catalog only after every primary
+    # route rejects" contract (ADR-0005) is unchanged. ``routes`` holds one
+    # row per agent in ``agents`` order (every branch above appends once).
+    deferred: list[object] = []
+    if viable:
+        for agent, row in zip(agents, routes):
+            if (
+                row.get("status") == "rejected"
+                and row.get("http_status") in REVIEW_PREFLIGHT_DEFERRABLE_HTTP_STATUS
+            ):
+                row["status"] = "deferred"
+                deferred.append(_demote_agent(agent, REVIEW_PREFLIGHT_DEFERRED_PRIORITY_PENALTY))
     report: dict[str, object] = {
         "contract": "strix-plain-chat-preflight-v2",
         "probed_count": len(agents),
         "ready_count": len(viable),
-        "rejected_count": len(agents) - len(viable),
+        "deferred_count": len(deferred),
+        "rejected_count": len(agents) - len(viable) - len(deferred),
         "escalations_used": escalations_used,
         "escalation_budget": REVIEW_PREFLIGHT_MAX_ESCALATIONS,
         "routes": routes,
@@ -521,7 +578,7 @@ def _preflight_review_agents(
         raise ReviewPreflightError(
             "no provider route passed the Strix plain-chat preflight", report
         )
-    return viable, report
+    return [*viable, *deferred], report
 
 
 def _preflight_with_fallback(
@@ -581,8 +638,9 @@ def _log_preflight_rejections(report: dict[str, object]) -> None:
     if not isinstance(routes, list):
         return
     for row in routes:
-        if not isinstance(row, dict) or row.get("status") != "rejected":
+        if not isinstance(row, dict) or row.get("status") not in ("rejected", "deferred"):
             continue
+        event = f"preflight_route_{row['status']}"
         # Re-validate rather than trust the caller's own sanitization: this
         # print reaches the sidecar's sanitized stderr stream unchanged, so an
         # out-of-contract value here (not a plain identifier) must degrade to
@@ -602,13 +660,13 @@ def _log_preflight_rejections(report: dict[str, object]) -> None:
         http_status = row.get("http_status")
         if isinstance(http_status, int) and not isinstance(http_status, bool) and 100 <= http_status <= 599:
             print(
-                f"preflight_route_rejected provider={provider} "
+                f"{event} provider={provider} "
                 f"error_type={error_type} http_status={http_status}",
                 file=sys.stderr,
             )
         else:
             print(
-                f"preflight_route_rejected provider={provider} error_type={error_type}",
+                f"{event} provider={provider} error_type={error_type}",
                 file=sys.stderr,
             )
 

scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py

@@ -16,7 +16,7 @@
     r"code=(?P<code>[A-Za-z0-9_.-]{1,64})"
 )
 _PREFLIGHT_ROUTE_REJECTED = re.compile(
-    r"preflight_route_rejected provider=(?P<provider>[a-z][a-z0-9_]{0,63}) "
+    r"preflight_route_(?P<event>rejected|deferred) provider=(?P<provider>[a-z][a-z0-9_]{0,63}) "
     r"error_type=(?P<error_type>[A-Za-z_][A-Za-z0-9_]{0,63})"
     r"(?: http_status=(?P<http_status>[1-5][0-9]{2}))?"
 )
@@ -116,7 +116,8 @@ def sanitize_line(line: str) -> str | None:
     preflight_route_rejected = _PREFLIGHT_ROUTE_REJECTED.search(stripped)
     if preflight_route_rejected is not None:
         summary = (
-            f"preflight_route_rejected provider={preflight_route_rejected.group('provider')} "
+            f"preflight_route_{preflight_route_rejected.group('event')} "
+            f"provider={preflight_route_rejected.group('provider')} "
             f"error_type={preflight_route_rejected.group('error_type')}"
         )
         http_status = preflight_route_rejected.group("http_status")

tests/test_contextual_orchestrator_review_runtime_preflight.py

@@ -1943,3 +1943,124 @@ def test_main_configures_sidecar_logging_before_touching_credentials() -> None:
     credentials_at = source.index("registered = register_review_credentials(os.environ)")
     assert configure_at < credentials_at
     assert "from contextual_orchestrator.debug_logging import configure_logging" in source
+
+
+def _preflight_agents(*ids: str) -> list[SimpleNamespace]:
+    """Return catalog-shaped agents with descending priorities, one per id."""
+    return [
+        SimpleNamespace(id=agent_id, provider_name=agent_id.split("_")[0], model=f"{agent_id}/m", priority=-index)
+        for index, agent_id in enumerate(ids)
+    ]
+
+
+class _StatusError(Exception):
+    """Exception with a ``code`` attribute, the shape ``_safe_http_status`` reads."""
+
+    def __init__(self, code: int) -> None:
+        super().__init__(f"HTTP Error {code}")
+        self.code = code
+
+
+def test_preflight_defers_transient_probe_statuses_behind_ready_routes() -> None:
+    """A 429/5xx probe answer keeps the route, ranked after every ready route.
+
+    noema-review run 33993637015 (2026-09-05) rejected 11 of 12 routes -- six
+    with 429 -- served the single ready route for 542 s and returned 502. The
+    serving gateway retries and fails over across exactly these statuses, so
+    discarding them at preflight left it nowhere to go.
+    """
+    namespace = _load_launcher()
+    agents = _preflight_agents("nvidia_ready", "openrouter_limited", "nvidia_missing", "nvidia_down")
+    client = _ProbeClient(
+        {
+            "nvidia_ready": {"choices": [{"message": {"content": "OK"}, "finish_reason": "stop"}]},
+            "openrouter_limited": _StatusError(429),
+            "nvidia_missing": _StatusError(404),
+            "nvidia_down": _StatusError(503),
+        }
+    )
+    served, report = namespace["_preflight_review_agents"](agents, client=client)
+
+    assert [agent.id for agent in served] == ["nvidia_ready", "openrouter_limited", "nvidia_down"]
+    assert served[0].priority == 0
+    penalty = namespace["REVIEW_PREFLIGHT_DEFERRED_PRIORITY_PENALTY"]
+    assert served[1].priority == -1 - penalty
+    assert served[2].priority == -3 - penalty
+    assert agents[1].priority == -1, "deferral must not mutate the caller's agent"
+    assert report["ready_count"] == 1
+    assert report["deferred_count"] == 2
+    assert report["rejected_count"] == 1
+    statuses = {row["agent_id"]: row["status"] for row in report["routes"]}
+    assert statuses == {
+        "nvidia_ready": "ready",
+        "openrouter_limited": "deferred",
+        "nvidia_missing": "rejected",
+        "nvidia_down": "deferred",
+    }
+
+
+def test_preflight_still_fails_when_no_route_is_ready() -> None:
+    """All-transient rejections keep failing the stage so the priced fallback still runs."""
+    namespace = _load_launcher()
+    agents = _preflight_agents("openrouter_a", "nvidia_b")
+    client = _ProbeClient({"openrouter_a": _StatusError(429), "nvidia_b": _StatusError(429)})
+    with pytest.raises(namespace["ReviewPreflightError"]) as excinfo:
+        namespace["_preflight_review_agents"](agents, client=client)
+    report = excinfo.value.report
+    assert report["ready_count"] == 0
+    assert report["deferred_count"] == 0
+    assert report["rejected_count"] == 2
+    assert {row["status"] for row in report["routes"]} == {"rejected"}
+
+
+def test_demote_agent_handles_frozen_dataclasses_and_plain_objects() -> None:
+    """The serving ``ModelAgent`` is a frozen dataclass; test doubles are plain objects."""
+    import dataclasses
+
+    namespace = _load_launcher()
+
+    @dataclasses.dataclass(frozen=True)
+    class _Frozen:
+        id: str
+        priority: int = 0
+
+    frozen = _Frozen(id="a", priority=-2)
+    demoted = namespace["_demote_agent"](frozen, 1000)
+    assert demoted.priority == -1002 and frozen.priority == -2
+    plain = SimpleNamespace(id="b")
+    demoted_plain = namespace["_demote_agent"](plain, 1000)
+    assert demoted_plain.priority == -1000 and not hasattr(plain, "priority")
+
+
+def test_log_preflight_rejections_reports_deferred_routes(capsys: pytest.CaptureFixture[str]) -> None:
+    """Deferred routes get their own bounded line so the stream tells them apart."""
+    namespace = _load_launcher()
+    namespace["_log_preflight_rejections"](
+        {
+            "routes": [
+                {"provider": "openrouter", "status": "deferred", "error_type": "HTTPError", "http_status": 429},
+                {"provider": "nvidia_nim", "status": "rejected", "error_type": "HTTPError", "http_status": 404},
+                {"provider": "nvidia_nim_sub", "status": "ready"},
+            ]
+        }
+    )
+    err = capsys.readouterr().err
+    assert "preflight_route_deferred provider=openrouter error_type=HTTPError http_status=429" in err
+    assert "preflight_route_rejected provider=nvidia_nim error_type=HTTPError http_status=404" in err
+    assert "nvidia_nim_sub" not in err
+
+
+def test_sidecar_stream_sanitizer_passes_deferred_preflight_lines() -> None:
+    """``preflight_route_deferred`` reaches the artifact with the same bounded fields as rejected."""
+    sanitizer = _load_sanitizer()
+    sanitize_line = sanitizer["sanitize_line"]
+    deferred = "preflight_route_deferred provider=openrouter error_type=HTTPError http_status=429"
+    rejected = "preflight_route_rejected provider=nvidia_nim error_type=HTTPError http_status=404"
+    assert sanitize_line(deferred) == deferred
+    assert sanitize_line(rejected) == rejected
+    assert sanitize_line("preflight_route_deferred provider=openrouter error_type=HTTPError") == (
+        "preflight_route_deferred provider=openrouter error_type=HTTPError"
+    )
+    assert sanitize_line("preflight_route_paused provider=openrouter error_type=HTTPError http_status=429") is None
+    assert sanitize_line(deferred + " token=sk-secret") is not None
+    assert "sk-secret" not in sanitize_line(deferred + " token=sk-secret")