fix(llm): provider-availability failures no longer block fallover

src/agent/step-executor.ts

@@ -1967,7 +1967,24 @@ function toLlmFailure(err: unknown, ctx: StepContext): LlmFailure {
     );
   }
   if (err instanceof LlamaServerError) {
-    if (err.status === null || err.status >= 500) {
+    // Delegate the status split to `classifyFailure` rather than
+    // restating it. This arm used to carry its own hardcoded copy
+    // (`status === null || >= 500` ⇒ transport, everything else ⇒
+    // grammar), and because `executeStep` rethrows *this* wrapper — and
+    // `classifyFailure`'s first line short-circuits on `LlmFailure` —
+    // the copy, not the classifier, decided the category the user reads.
+    // The two diverged the moment the taxonomy moved: a 404 from a wrong
+    // `localModels.url` still surfaced as `Turn failed [grammar]` with no
+    // unreachable hint. One taxonomy, one place.
+    //
+    // `classifyFailure` cannot return `cancelled`/`model`/`tool` for a
+    // `LlamaServerError` (its own arm returns only `transport` or
+    // `grammar`, and it is reached before the abort/network branches),
+    // and an aborted step has already been claimed by the
+    // `ctx.signal.aborted` check above — so nothing is laundered here.
+    // Only `transport` becomes a `TransportError`; every other answer
+    // keeps the historical `GrammarError`.
+    if (classifyFailure(err) === "transport") {
       return new TransportError(err.message, err.status, err.url, { cause: err });
     }
     return new GrammarError(err.message, "", { cause: err });

src/llm/fallback/should-advance.test.ts

@@ -9,6 +9,10 @@ import {
   ToolExecutionError,
   CancelledError,
 } from "../reliability/llm-failures.js";
+import {
+  SubscriptionCliAuthError,
+  SubscriptionCliNotInstalledError,
+} from "../provider/subscription-cli/subscription-cli-errors.js";
 
 describe("shouldAdvance", () => {
   it("advances immediately on a cloud 429", () => {
@@ -75,14 +79,72 @@ describe("shouldAdvance", () => {
     });
   });
 
-  it("does NOT advance on a local llama 4xx (grammar category)", () => {
-    // LlamaServerError with a 4xx status classifies as grammar.
+  it("does NOT advance on a local llama 400 (request-shape, grammar category)", () => {
+    // A 400 is the server rejecting THIS request; the next link rejects
+    // it the same way, so falling over buys nothing.
     expect(shouldAdvance(new LlamaServerError("x", 400, "http://local"))).toEqual({
       advance: false,
       immediate: false,
     });
   });
 
+  it("advances on a local llama 404 without arming the breaker — the URL serves no completions", () => {
+    // The endpoint is permanently wrong (bad `localModels.url`, or a
+    // server that is not a llama-server), so the chain must move off this
+    // link — and it moves on THIS failure: `advanceFrom` returns the next
+    // provider whenever `advance` is true, immediate or not.
+    // `immediate: false` is about the breaker, not the switch: 404 is not
+    // in the 429/408/5xx provider-down set, so the cooldown that keeps the
+    // link quarantined across later turns is armed only once the
+    // consecutive-failure threshold trips.
+    expect(shouldAdvance(new LlamaServerError("x", 404, "http://local"))).toEqual({
+      advance: true,
+      immediate: false,
+    });
+  });
+
+  it("advances on a local llama 405 without arming the breaker", () => {
+    expect(shouldAdvance(new LlamaServerError("x", 405, "http://local"))).toEqual({
+      advance: true,
+      immediate: false,
+    });
+  });
+
+  it("advances on a local llama 429 and arms the breaker on the first failure", () => {
+    // Transport category plus an unambiguous provider-down status — the
+    // `isImmediateSignal` status read already handled 429; it was simply
+    // unreachable while 4xx classified as grammar. The extra `immediate`
+    // buys the cooldown straight away, not an earlier switch.
+    expect(shouldAdvance(new LlamaServerError("x", 429, "http://local"))).toEqual({
+      advance: true,
+      immediate: true,
+    });
+  });
+
+  it("advances on a local llama 408 and arms the breaker on the first failure", () => {
+    expect(shouldAdvance(new LlamaServerError("x", 408, "http://local"))).toEqual({
+      advance: true,
+      immediate: true,
+    });
+  });
+
+  it("advances on a subscription-CLI binary that is not installed", () => {
+    // It must advance: a missing `claude` binary otherwise pins the chain
+    // to a provider that can never serve a turn — and the switch happens
+    // on this first failure. There is no HTTP status to read, so
+    // `immediate` is false and the breaker cooldown waits for the
+    // consecutive-failure threshold.
+    expect(
+      shouldAdvance(new SubscriptionCliNotInstalledError("claude", "Install it.")),
+    ).toEqual({ advance: true, immediate: false });
+  });
+
+  it("advances on a subscription-CLI provider that is signed out", () => {
+    expect(
+      shouldAdvance(new SubscriptionCliAuthError("codex", "Run /login.")),
+    ).toEqual({ advance: true, immediate: false });
+  });
+
   it("advances via threshold on a TransportError carrying null status", () => {
     expect(shouldAdvance(new TransportError("x", null, "u"))).toEqual({
       advance: true,

src/llm/fallback/should-advance.ts

@@ -11,8 +11,14 @@ import { TransportError } from "../reliability/llm-failures.js";
  *    `false` means the error is deterministic (same request fails the
  *    same way everywhere) or is a cancellation — propagate it untouched.
  *  - `immediate`: an unambiguous provider-down signal (429 / 408 / 5xx /
- *    network-null) that should switch on the FIRST occurrence, bypassing
- *    the consecutive-failure threshold.
+ *    network-null). This does NOT control whether the chain switches —
+ *    `ProviderFallbackChain.advanceFrom` returns the next link on the
+ *    FIRST fallover-worthy failure whenever `advance` is true, immediate
+ *    or not. What it controls is the breaker: `registerFailure` arms the
+ *    cooldown right away on an immediate signal, instead of waiting for
+ *    `failureThreshold` consecutive failures. So `immediate` decides how
+ *    long the failed link stays quarantined across later turns, not the
+ *    in-turn switch.
  */
 export interface AdvanceDecision {
   advance: boolean;
@@ -29,15 +35,21 @@ const NO: AdvanceDecision = { advance: false, immediate: false };
  *  - `transport` → advance (provider unreachable). Note every cloud
  *    `OpenAiHttpError` classifies as `transport` regardless of status, so
  *    a 404 model-not-found or a 401 dead key advances too — a different
- *    link may have the model or a working key. Untyped socket failures
+ *    link may have the model or a working key. The local path now agrees:
+ *    a llama-server 404/405 (the configured URL does not serve
+ *    completions) or 401/403/429 advances for the same reason. A
+ *    subscription-CLI provider whose binary is missing or is signed out
+ *    is the same story with no HTTP in it. Untyped socket failures
  *    (undici's `TypeError: fetch failed` and friends, from surfaces that
  *    do not wrap their own errors) land here too — see `isNetworkError`.
  *  - `model` → advance. This is a *defective completion* from a reachable
  *    provider (truncated / empty / no_stop), not "model not found"; the
  *    same prompt would reproduce it here, so another link is worth a try.
- *  - `grammar` / `tool` / `cancelled` → do not advance. A grammar/4xx
- *    failure is request-shape and repeats identically on every provider;
- *    a tool failure is our own bug; a cancellation is user intent.
+ *  - `grammar` / `tool` / `cancelled` → do not advance. A grammar failure
+ *    is request-shape — an unparseable completion, or the narrow band of
+ *    llama-server 4xx that rejects the request itself (400/413/422) —
+ *    and repeats identically on every provider; a tool failure is our own
+ *    bug; a cancellation is user intent.
  *
  * Immediate signals are read off the typed status carried by
  * `OpenAiHttpError` / `LlamaServerError` / `TransportError`: an explicit

src/llm/reliability/classify-failure.test.ts

@@ -8,6 +8,11 @@ import {
   ToolExecutionError,
   TransportError,
 } from "./llm-failures.js";
+import {
+  SubscriptionCliAuthError,
+  SubscriptionCliInvocationError,
+  SubscriptionCliNotInstalledError,
+} from "../provider/subscription-cli/subscription-cli-errors.js";
 import { classifyFailure } from "./classify-failure.js";
 
 describe("classifyFailure", () => {
@@ -29,7 +34,7 @@ describe("classifyFailure", () => {
     expect(classifyFailure(err)).toBe("transport");
   });
 
-  it("maps LlamaServerError 4xx to grammar", () => {
+  it("maps LlamaServerError request-shape 4xx to grammar", () => {
     const err = new LlamaServerError("bad grammar", 400, "http://x");
     expect(classifyFailure(err)).toBe("grammar");
   });
@@ -87,3 +92,62 @@ describe("classifyFailure — raw network failures", () => {
     expect(classifyFailure(err)).toBe("cancelled");
   });
 });
+
+describe("classifyFailure — llama-server HTTP statuses", () => {
+  // A 4xx that describes the *endpoint* must not be filed as `grammar`:
+  // that category blocks fallover (`shouldAdvance`) and tells the user
+  // their grammar is broken when the real answer is "that URL is not a
+  // llama-server". A 4xx that rejects the request itself stays grammar.
+  const cases: Array<[number | null, string]> = [
+    [null, "transport"],
+    [400, "grammar"],
+    [401, "transport"],
+    [402, "transport"],
+    [403, "transport"],
+    [404, "transport"],
+    [405, "transport"],
+    [408, "transport"],
+    [409, "transport"],
+    [413, "grammar"],
+    [422, "grammar"],
+    [429, "transport"],
+    [500, "transport"],
+    [503, "transport"],
+  ];
+
+  for (const [status, expected] of cases) {
+    it(`maps status ${status ?? "null"} to ${expected}`, () => {
+      const err = new LlamaServerError("boom", status, "http://x");
+      expect(classifyFailure(err)).toBe(expected);
+    });
+  }
+
+  it("leaves an unlisted 4xx on the request-shape side", () => {
+    // Conservative default: only the statuses we can name as
+    // endpoint/auth/availability earn a fallover.
+    expect(classifyFailure(new LlamaServerError("x", 418, "http://x"))).toBe(
+      "grammar",
+    );
+  });
+});
+
+describe("classifyFailure — subscription-CLI providers", () => {
+  it("maps a missing CLI binary to transport, not tool", () => {
+    // "claude is not on PATH" is a dead provider link, not a bug in our
+    // tool layer — the chain must be free to try the next provider.
+    const err = new SubscriptionCliNotInstalledError("claude", "Install it.");
+    expect(classifyFailure(err)).toBe("transport");
+  });
+
+  it("maps a signed-out CLI to transport, not tool", () => {
+    const err = new SubscriptionCliAuthError("codex", "Run /login.");
+    expect(classifyFailure(err)).toBe("transport");
+  });
+
+  it("still treats a failed CLI invocation as a tool failure", () => {
+    // The binary ran and came back unhappy: that is not evidence the
+    // link is unusable, so it keeps the non-advancing category.
+    const err = new SubscriptionCliInvocationError("claude exited 1", 1);
+    expect(classifyFailure(err)).toBe("tool");
+  });
+});

src/llm/reliability/classify-failure.ts

@@ -1,10 +1,31 @@
 import { LlamaServerError } from "../llama-server-client.js";
 import { OpenAiHttpError } from "../provider/openai/openai-http.js";
+import {
+  SubscriptionCliAuthError,
+  SubscriptionCliNotInstalledError,
+} from "../provider/subscription-cli/subscription-cli-errors.js";
 import { ToolCallParseError } from "../grammar/tool-call-grammar.js";
 import type { LlmFailureCategory } from "./failure-category.js";
 import { LlmFailure } from "./llm-failures.js";
 import { isNetworkError } from "./network-error.js";
 
+/**
+ * llama-server 4xx statuses that describe the *endpoint*, not the request
+ * we sent it: the URL does not serve completions (404 — a wrong
+ * `localModels.url`, or a server that is not a llama-server at all), the
+ * method is not allowed (405), a proxy in front of it wants credentials
+ * (401/402/403), or the server is busy / timing us out / conflicting
+ * (408/409/429). None of these repeat identically on a different provider,
+ * so they are `transport`: the link is unusable, try the next one.
+ *
+ * Everything else in the 4xx range stays `grammar` — 400/413/422 are the
+ * server telling us THIS request was malformed or too large, which the
+ * next link would reject the same way.
+ */
+const LLAMA_ENDPOINT_UNAVAILABLE_STATUSES = new Set([
+  401, 402, 403, 404, 405, 408, 409, 429,
+]);
+
 /**
  * Classify any thrown value into the canonical failure taxonomy.
  *
@@ -15,6 +36,13 @@ import { isNetworkError } from "./network-error.js";
  * grammar parser errors, abort signals, and anything else treated as
  * a tool-layer problem by default).
  *
+ * The governing rule across every branch: a failure that means "this
+ * provider is unusable" must not land in a category that blocks
+ * fallover. `shouldAdvance` only advances on `transport` / `model`, so
+ * filing an unusable link under `grammar` or `tool` pins the chain to a
+ * permanently broken provider and hands the user a diagnosis for a
+ * problem they do not have.
+ *
  * The `isNetworkError` branch sits between the abort check and that
  * default: an untyped socket failure (MCP streamable-http, embeddings,
  * a vendor SDK with its own `fetch`) is a `transport` problem even
@@ -29,13 +57,25 @@ export function classifyFailure(err: unknown): LlmFailureCategory {
   if (err instanceof LlamaServerError) {
     if (err.status === null) return "transport";
     if (err.status >= 500) return "transport";
+    if (LLAMA_ENDPOINT_UNAVAILABLE_STATUSES.has(err.status)) return "transport";
     return "grammar";
   }
   // Cloud provider failures are provider-boundary problems whatever the
   // status — a 429 or 401 is no more "our tool broke" than a 503. The
   // retry budget was already spent inside the HTTP client, matching the
   // TransportError contract.
   if (err instanceof OpenAiHttpError) return "transport";
+  // A CLI-backed provider whose binary is missing or signed out is the
+  // same shape of problem as an unreachable HTTP endpoint: this link
+  // cannot serve the turn, and no other link is implicated. The default
+  // `tool` arm below would both mislabel it ("Turn failed [tool]" for a
+  // binary the user never installed) and stop the chain dead.
+  if (
+    err instanceof SubscriptionCliNotInstalledError ||
+    err instanceof SubscriptionCliAuthError
+  ) {
+    return "transport";
+  }
   if (isAbortError(err)) return "cancelled";
   // Checked after the abort branch on purpose: an aborted request can
   // surface as ECONNRESET, and a user pressing Esc is not a fallover.

src/llm/reliability/failure-category.ts

@@ -1,11 +1,18 @@
 /**
  * Canonical taxonomy of failures surfaced by the agent runtime.
  *
- *  - `transport`: llama-server unreachable, network error, HTTP 5xx.
+ *  - `transport`: the provider link is unusable — llama-server
+ *                 unreachable, network error, HTTP 5xx, a llama-server
+ *                 4xx that describes the endpoint rather than the request
+ *                 (401/402/403/404/405/408/409/429), any cloud HTTP
+ *                 failure, or a CLI-backed provider whose binary is
+ *                 missing or signed out. Everything here is worth
+ *                 retrying on the next link in the fallback chain.
  *  - `grammar`:   the completion payload could not be parsed into a valid
- *                 tool call; also covers HTTP 4xx from llama-server which
- *                 generally means the server rejected the grammar or
- *                 request shape.
+ *                 tool call; also covers the llama-server 4xx statuses
+ *                 that reject THIS request as malformed or oversized
+ *                 (400/413/422 and any other unlisted 4xx), which the
+ *                 next provider would reject identically.
  *  - `model`:     the completion itself is defective (truncated, empty,
  *                 or generated without a stop token). Retrying the same
  *                 prompt is unlikely to help, so the runtime does not.