fix(routing): classify primary provider transport failures explicitly

CHANGELOG.md

@@ -90,6 +90,24 @@ and this project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html)
 
 ### Fixed
 
+- `TaskOrchestrator._invoke`'s route/Conduct primary chat call now classifies
+  a `ProviderUpstreamError` (5xx, 429, network) directly from its own
+  already-computed `retryable` flag (`tool_fallback.classify_provider_transport_failure`)
+  instead of `classify_tool_failure`'s tool-execution-oriented message-text
+  heuristics. A plain, side-effect-free completion request can always be
+  safely retried or handed to the next ranked candidate, so this classifier
+  never returns `fail_closed`; previously an upstream error body that
+  happened to also contain a tool-fallback keyword (e.g. a 500 whose message
+  said "invalid arguments") could be misclassified into an
+  `invalid_arguments`/permission/policy fail-closed row and stop
+  `orchestrator/free`/`orchestrator/auto` failover on a request that never
+  touched a tool. `orchestrator/free` still never advances into a priced
+  agent, and exhausting every free/auto candidate still fails closed with the
+  last classified provider error; `classify_tool_failure` itself, and the
+  provider's own `tool_execution_stopped` signal, are unchanged. See
+  [ADR 0001's amendment](docs/adr/0001-tool-execution-fallback-policy.md#amendment-2026-08-30-explicit-provider-transport-classification).
+  Motivated by the `orchestrator/free` review-sidecar reliability gap in
+  `ContextualWisdomLab/.github` PR #1433.
 - Discover chat models from metadata-free OpenAI-compatible gateways. A
   configured gateway whose `/v1/models` rows carry no modality/capability
   metadata previously produced empty-capability chat rows that runtime

contextual_orchestrator/model_discovery.py

@@ -19,6 +19,7 @@
 import math
 import re
 import ssl
+import time
 import urllib.error
 import urllib.request
 import certifi
@@ -41,6 +42,15 @@
 _HTTP_USER_AGENT = "contextual-orchestrator/0.2.0 (+https://github.com/ContextualWisdomLab/contextual-orchestrator)"
 _CAPABILITY_NAMES = {"embeddings": "embedding"}
 _MODELS_DEV_URL = "https://models.dev/api.json"
+# Small bounded retry budget for the one shared, unauthenticated, third-party
+# Models.dev fetch that every ``models_dev_provider_id``-joined source's
+# free-tier classification depends on (ADR 0041/0032). It has already been
+# observed live to reject urllib's default user agent as a bot signature (see
+# ``_HTTP_USER_AGENT`` above); a lone transient failure of that kind must not
+# silently erase every dependent provider's ``orchestrator/free`` coverage for
+# the whole discovery run the way a single un-retried attempt would.
+_MODELS_DEV_FETCH_ATTEMPTS = 3
+_MODELS_DEV_FETCH_RETRY_DELAY_SECONDS = 0.05
 _OPENROUTER_ZDR_ENDPOINTS_URL = "https://openrouter.ai/api/v1/endpoints/zdr"
 _OPENROUTER_PROVIDER_POLICIES_URL = "https://openrouter.ai/api/frontend/v1/all-providers"
 CONFIGURED_GATEWAY_CREDENTIAL_NAME = "LLM_GATEWAY_API_KEY"
@@ -283,6 +293,33 @@ def _fetch_json(url: str, *, api_key: str = "", auth_scheme: str = "Bearer", tim
         return json.loads(response.read().decode("utf-8"))
 
 
+def _fetch_models_dev_metadata(*, timeout: float) -> Any | None:
+    """Fetch the shared Models.dev catalog with a small bounded retry.
+
+    Every ``models_dev_provider_id``-joined source (``opencode_zen``,
+    ``nvidia_nim``, ``nvidia_nim_sub``, ``openai``) shares this one
+    unauthenticated, best-effort, third-party fetch for its free-cost
+    evidence; none of those providers report their own pricing, so a lone
+    transient failure here (a timeout, a reset connection, or the
+    bot-signature rejection ``_HTTP_USER_AGENT`` already guards against) used
+    to silently degrade every one of them to ``is_free = False`` for the rest
+    of the discovery run, collapsing ``orchestrator/free`` coverage over a
+    blip in a service this gateway does not control.
+
+    Returns ``None`` -- the existing "no evidence" fail-closed signal
+    :func:`_merge_models_dev_metadata` already handles -- only once every
+    bounded attempt has failed; a successful attempt returns immediately
+    without spending the rest of the retry budget.
+    """
+    for attempt in range(_MODELS_DEV_FETCH_ATTEMPTS):
+        try:
+            return _fetch_json(_MODELS_DEV_URL, timeout=timeout)
+        except (urllib.error.URLError, TimeoutError, ValueError, OSError):
+            if attempt < _MODELS_DEV_FETCH_ATTEMPTS - 1:
+                time.sleep(_MODELS_DEV_FETCH_RETRY_DELAY_SECONDS)
+    return None
+
+
 def _fetch_configured_gateway_json(
     url: str,
     *,
@@ -1092,10 +1129,7 @@ def discover_provider_models(
         raise ProviderDiscoveryError(source.provider_name, _provider_discovery_error_code(exc)) from None
     if source.models_dev_provider_id:
         if models_dev_metadata is _NOT_FETCHED:
-            try:
-                metadata = _fetch_json(_MODELS_DEV_URL, timeout=timeout)
-            except (urllib.error.URLError, TimeoutError, ValueError, OSError):
-                metadata = None
+            metadata = _fetch_models_dev_metadata(timeout=timeout)
         else:
             metadata = models_dev_metadata
         payload = _merge_models_dev_metadata(payload, metadata, source.models_dev_provider_id)
@@ -1152,7 +1186,8 @@ def discover_all_models(
 
     Up to four sources (``opencode_zen``, ``nvidia_nim``, ``nvidia_nim_sub``,
     ``openai``) each want the same Models.dev catalog. When any registered
-    source declares ``models_dev_provider_id``, fetch it here exactly once and
+    source declares ``models_dev_provider_id``, fetch it here exactly once
+    (:func:`_fetch_models_dev_metadata`, with its own small bounded retry) and
     hand every source the identical parsed payload, instead of each source
     independently repeating the fetch inside :func:`discover_provider_models`.
     """
@@ -1163,10 +1198,7 @@ def discover_all_models(
         source.models_dev_provider_id and get_credential(source.credential_name)
         for source in sources
     ):
-        try:
-            models_dev_metadata = _fetch_json(_MODELS_DEV_URL, timeout=timeout)
-        except (urllib.error.URLError, TimeoutError, ValueError, OSError):
-            models_dev_metadata = None
+        models_dev_metadata = _fetch_models_dev_metadata(timeout=timeout)
     for source in sources:
         try:
             discovered.extend(

contextual_orchestrator/orchestrator.py

@@ -72,6 +72,7 @@
     ToolFailureKind,
     ToolFallbackAction,
     ToolFallbackStoppedError,
+    classify_provider_transport_failure,
     classify_tool_failure,
     downgrade_to_failover,
 )
@@ -6472,10 +6473,37 @@ def call(agent: ModelAgent) -> tuple[str, str, dict[str, Any] | None]:
                     if agent.group_name or allowed_agent_ids is not None:
                         self._group_router.observe_failure(agent.id)
                     if isinstance(exc, ToolFallbackStoppedError):
+                        # Deliberately terminal, even inside a free/auto virtual
+                        # pool with untried candidates remaining: every path that
+                        # raises this (the provider's own explicit terminal
+                        # tool-execution-state signal via
+                        # _provider_tool_execution_stopped, or a FAIL_CLOSED
+                        # verdict from classify_tool_failure below) resolves to
+                        # ambiguous_outcome, permission_denied, policy_blocked, or
+                        # invalid_arguments -- the exact ADR 0001 safety invariants
+                        # ("permission and policy failures never fall through to
+                        # another agent"; "non-idempotent timeout or transport
+                        # uncertainty never replays automatically") that a
+                        # different candidate cannot make safer: an ambiguous
+                        # server-side outcome is ambiguous regardless of which
+                        # agent asks next, and authorization/policy denial must
+                        # not be worked around by trying a different one. Do not
+                        # convert this to failover without an explicit product
+                        # decision distinguishing which failure kinds that would
+                        # actually be safe for.
                         raise
                     if isinstance(exc, ProviderUpstreamError):
                         last_upstream_error = exc
-                    if isinstance(exc, ProviderResponseError):
+                        # The primary chat call is a bounded, side-effect-free
+                        # model request, not a tool invocation: classify from
+                        # the provider's own already-computed retryability
+                        # instead of classify_tool_failure's message-text
+                        # heuristics, so free/auto virtual-model failover can
+                        # never be accidentally downgraded to fail-closed by
+                        # incidental wording in an upstream error body (e.g. a
+                        # 400 that happens to mention "invalid arguments").
+                        decision = classify_provider_transport_failure(exc.retryable)
+                    elif isinstance(exc, ProviderResponseError):
                         if allowed_agent_ids is None:
                             raise
                         bounded_provider_response_failures += 1
@@ -6484,15 +6512,9 @@ def call(agent: ModelAgent) -> tuple[str, str, dict[str, Any] | None]:
                         self._record_tool_fallback(agent.id, decision, retry_attempt)
                         self._record_failure(agent.id)
                         break
-                    decision = classify_tool_failure(exc)
+                    else:
+                        decision = classify_tool_failure(exc)
                     action = decision.action
-                    if (
-                        isinstance(exc, ProviderUpstreamError)
-                        and not exc.retryable
-                        and action is ToolFallbackAction.RETRY_SAME_AGENT
-                    ):
-                        decision = downgrade_to_failover(decision)
-                        action = decision.action
                     # A failed attempt is one Bernoulli stability observation
                     # for measured group routing regardless of what happens next.
                     if (

contextual_orchestrator/tool_fallback.py

@@ -121,6 +121,44 @@ def downgrade_to_failover(decision: ToolFailureDecision) -> ToolFailureDecision:
     )
 
 
+def classify_provider_transport_failure(retryable: bool) -> ToolFailureDecision:
+    """Map one plain (non-tool) provider transport failure to a fallback decision.
+
+    This is the classifier for a bounded, side-effect-free model completion
+    request -- the primary call a virtual ``orchestrator/free``/``orchestrator/auto``
+    route makes, not an in-flight tool invocation. Because the request has no
+    external side effect, replaying it (same agent) or moving to the next
+    ranked candidate can never create the ambiguous-outcome risk
+    :func:`classify_tool_failure` exists to guard against, so this classifier
+    intentionally never returns :attr:`ToolFallbackAction.FAIL_CLOSED`.
+
+    ``retryable`` is the upstream classification already computed by
+    :func:`contextual_orchestrator.provider_errors.classify_provider_failure`
+    (true for a transient 429/500/502/503/504/408/network failure, false for a
+    non-transient 4xx such as 401/403/404). The decision is keyed on that
+    boolean alone -- never on the failure's message text -- so an upstream
+    error body that happens to mention "tool", "command", "invalid arguments",
+    or another :func:`classify_tool_failure` keyword can never accidentally
+    reclassify a plain provider outage as fail-closed (CWE-705: incorrect
+    control flow scoping between the tool-execution and provider-transport
+    failure domains).
+    """
+    if not isinstance(retryable, bool):
+        raise TypeError("retryable must be a boolean")
+    if retryable:
+        return _decision(
+            ToolFailureKind.TRANSPORT_ERROR,
+            ToolFallbackAction.RETRY_SAME_AGENT,
+            retry_safe=True,
+            circuit_failure=True,
+        )
+    return _decision(
+        ToolFailureKind.TRANSPORT_ERROR,
+        ToolFallbackAction.FAILOVER_AGENT,
+        circuit_failure=True,
+    )
+
+
 _HTTP_FAILURE_KIND = {
     400: ToolFailureKind.INVALID_ARGUMENTS,
     401: ToolFailureKind.PERMISSION_DENIED,

docs/adr/0001-tool-execution-fallback-policy.md

@@ -40,6 +40,12 @@ Structured adapters should raise `ToolExecutionError` with a stable failure kind
 
 The route and Conduct-stage invocation path performs at most `tool_retry_attempts` same-agent retries, with a shared hard ceiling of four attempts. Exhausted safe retries become sequential agent failover. Every decision emits a secret-free audit record containing only the agent id, failure kind, action, reason code, and retry count.
 
+## Amendment (2026-08-30): explicit provider-transport classification
+
+The primary model call `TaskOrchestrator._invoke` makes on every route/Conduct step (`ModelClient.chat`) is a bounded, side-effect-free read: it is a model completion request, not a tool invocation, so it can never produce the ambiguous-outcome risk this ADR's `fail_closed` rows exist to guard against. Before this amendment, that call's already-typed `ProviderUpstreamError` (see `contextual_orchestrator.provider_errors`) was still routed through the same message-text classifier this ADR defines for tool runtimes. In practice a generic transport error rarely mentions a tool-runtime keyword, so it fell through to the `unknown` row and correctly kept failing over — but only incidentally: an upstream error body that happened to also say, for example, "invalid arguments" (a phrase also used by ordinary 400s unrelated to any tool) would have been misclassified into this ADR's `invalid_arguments`/`fail_closed` row and stopped free/auto virtual-model failover on a request that had never touched a tool.
+
+`contextual_orchestrator.tool_fallback.classify_provider_transport_failure(retryable: bool)` now classifies this specific call directly from the provider taxonomy's own already-computed `retryable` flag — never from message text — and never returns `fail_closed`: retryable failures (429/500/502/503/504/408/network) get one bounded same-agent retry then sequential failover; non-retryable failures (401/403/404/413 handled earlier/422/...) fail over immediately. This is the same "generic provider transport failures keep the previous agent-failover behavior" intent this ADR already stated; it is now an explicit, provider-status-driven contract instead of an implicit one that depended on a failure message never mentioning a tool-fallback keyword. `classify_tool_failure` itself is unchanged and still governs genuine `ToolExecutionError` adapters and the provider's own explicit `tool_execution_stopped` signal (`_provider_tool_execution_stopped`), both of which keep failing closed exactly as this ADR specifies. Motivated by the `orchestrator/free` review-sidecar reliability gap tracked in `ContextualWisdomLab/.github` PR #1433.
+
 ## Safety invariants
 
 1. Missing-tool handling changes agents; it never guesses an alias for the missing tool.

docs/architecture.md

@@ -65,6 +65,17 @@ bounded, authenticated recursion protocol; it is not administratively disabled.
 - `contextual_orchestrator.orchestrator.ModelAgent`: one configured worker model.
 - `TaskOrchestrator.route_once`: the low-latency routing path.
 - `TaskOrchestrator.conduct`: the workflow path with planner, worker, verifier, and synthesizer steps.
+- `TaskOrchestrator._invoke`: the shared route/Conduct invocation path. A
+  request-time failure of the primary provider call — 5xx, 429, network, a
+  413 request-size rejection, or a non-retryable 4xx such as 401/403/404
+  (this list is illustrative, not exhaustive: any failure the provider
+  taxonomy classifies via `classify_provider_transport_failure` falls into
+  either bucket) — advances to the next ranked candidate within the same
+  cost tier — `orchestrator/free` never fails over into a priced agent, and
+  `orchestrator/auto` only fails over inside the primary's own declared
+  model group — instead of surfacing an opaque error; exhausting every
+  eligible candidate still fails closed with the last classified provider
+  error. See [ADR 0001's amendment](adr/0001-tool-execution-fallback-policy.md#amendment-2026-08-30-explicit-provider-transport-classification).
 - `WorkflowStep.access`: Conductor-style visibility control.
 - `ModelClient`: OpenAI-compatible HTTP client, with `mock://` for local checks.
 - `contextual_orchestrator.server`: small `/v1/chat/completions` HTTP server.

docs/planning/adrs/0041-generalize-models-dev-cost-classification.md

@@ -158,6 +158,25 @@ touches can turn a paid model free.
   `opencode_zen` and removes up to three duplicate fetches when multiple
   supported sources are registered.
 
+## Amendment (2026-08-30): bounded retry on the shared fetch
+
+Restoring real `orchestrator/free` coverage for `nvidia_nim`/`nvidia_nim_sub`
+onto this one shared, unauthenticated, third-party fetch also made it a
+single point of failure for their entire free-tier classification: this
+module already documents that `models.dev` has been observed live to reject
+urllib's default user agent as a bot signature (`_HTTP_USER_AGENT`), and the
+fetch had exactly one attempt. `_fetch_models_dev_metadata` now makes up to
+`_MODELS_DEV_FETCH_ATTEMPTS` (3) total attempts — the initial attempt plus
+2 retries — with a short fixed delay between them before degrading to
+`None`, so one transient blip in a third-party
+service this gateway does not control no longer has to erase `nvidia_nim`'s
+and `nvidia_nim_sub`'s free-tier evidence for an entire discovery run. The
+cost-safety argument above is unchanged: every failure mode this ADR lists
+still leaves `is_free = False` once the retry budget is genuinely exhausted;
+nothing about the retry can turn a paid model free. Motivated by the
+`orchestrator/free` review-sidecar reliability gap in
+`ContextualWisdomLab/.github` PR #1433.
+
 ## References
 
 Models.dev. (2026). *Models.dev API*. https://models.dev/api.json