Fail closed on a missing provider key instead of sending an unauthenticated request

.changeset/fail-closed-missing-provider-key.md

@@ -0,0 +1,19 @@
+---
+"@agent-native/core": patch
+---
+
+Fail closed when an `ai-sdk:*` engine has no provider key, instead of sending an
+unauthenticated request. The provider factory was previously built with no
+`apiKey`, so the SDK omitted the Authorization header and the gateway's 401 came
+back as `http_401` "Missing Authentication header" — a transport error naming the
+wrong cause, which a scheduled job then retried on every tick forever. It now
+reports `missing_credentials` and names the env var it wants, matching what
+`builder-engine` and `anthropic-engine` already did.
+
+Also stop reaping in-process background automations (scheduler and trigger runs)
+at the tight 45s post-claim stale window. That window exists to reach a durable
+successor sooner, but these runs carry no `dispatch_payload` and have no
+successor to reach, so an early reap killed still-working jobs that nothing could
+recover. They now get the 90s background window, and the recovery path reports
+`not_redispatchable` rather than `payload_missing`, which read as data loss for
+the one case where nothing was ever lost.

packages/core/src/agent/engine/ai-sdk-engine.spec.ts

@@ -472,17 +472,26 @@ describe("AISDKEngine OpenAI model selection", () => {
     );
   });
 
-  it("passes an empty apiKey when env fallback is disabled", async () => {
+  it("never reaches the deploy key when env fallback is disabled", async () => {
     vi.stubEnv("OPENAI_API_KEY", "sk-deploy");
-    mockAiSdk();
+    const { streamText } = mockAiSdk();
     const { createOpenAI } = mockOpenAIProvider();
 
     const { createAISDKEngine } = await import("./ai-sdk-engine.js");
     const engine = createAISDKEngine("openai", { allowEnvFallback: false });
 
-    await drain(engine.stream(BASE_STREAM_OPTIONS));
+    const events: any[] = [];
+    for await (const e of engine.stream(BASE_STREAM_OPTIONS)) events.push(e);
 
-    expect(createOpenAI).toHaveBeenCalledWith({ apiKey: "" });
+    // Previously this constructed the provider with `apiKey: ""` so the AI SDK
+    // could not read the ambient deploy key itself. That kept the deploy key
+    // out, but shipped a guaranteed-401 unauthenticated request. Failing closed
+    // keeps the deploy key out just as firmly and reports the real cause.
+    expect(createOpenAI).not.toHaveBeenCalled();
+    expect(streamText).not.toHaveBeenCalled();
+    expect(events.find((e) => e.type === "stop")?.errorCode).toBe(
+      "missing_credentials",
+    );
   });
 
   it("keeps Chat Completions for custom OpenAI-compatible base URLs", async () => {
@@ -772,3 +781,73 @@ describe("AISDKEngine streamed tool-input reconciliation", () => {
     expect(events.some((e) => e.type === "tool-call-error")).toBe(false);
   });
 });
+
+describe("AISDKEngine missing-credential fail-closed", () => {
+  beforeEach(() => {
+    vi.resetModules();
+    vi.unstubAllEnvs();
+  });
+
+  afterEach(() => {
+    vi.unstubAllEnvs();
+  });
+
+  // Production: the clips app ran `ai-sdk:openrouter` with no OPENROUTER_API_KEY.
+  // The provider factory was built with no apiKey, the SDK sent no Authorization
+  // header, and the gateway's 401 "Missing Authentication header" was classified
+  // `http_401` — a transport error naming the wrong cause, retried every 30
+  // minutes forever. 18/18 scheduled runs failed this way.
+  it("fails closed with missing_credentials instead of sending an unauthenticated request", async () => {
+    const { streamText } = mockAiSdk();
+    const createOpenRouter = vi.fn();
+    vi.doMock("@openrouter/ai-sdk-provider", () => ({ createOpenRouter }));
+
+    const { createAISDKEngine } = await import("./ai-sdk-engine.js");
+    const engine = createAISDKEngine("openrouter", { allowEnvFallback: false });
+
+    const events: any[] = [];
+    for await (const e of engine.stream(BASE_STREAM_OPTIONS as any)) {
+      events.push(e);
+    }
+
+    const stop = events.find((e) => e.type === "stop");
+    expect(stop?.reason).toBe("error");
+    expect(stop?.errorCode).toBe("missing_credentials");
+    expect(stop?.error).toContain("OPENROUTER_API_KEY");
+    // The whole point: no request was ever built or sent.
+    expect(createOpenRouter).not.toHaveBeenCalled();
+    expect(streamText).not.toHaveBeenCalled();
+  });
+
+  it("still runs when a key is present", async () => {
+    const { streamText } = mockAiSdk();
+    const provider = vi.fn().mockReturnValue({ id: "m" });
+    vi.doMock("@openrouter/ai-sdk-provider", () => ({
+      createOpenRouter: vi.fn().mockReturnValue(provider),
+    }));
+
+    const { createAISDKEngine } = await import("./ai-sdk-engine.js");
+    const engine = createAISDKEngine("openrouter", { apiKey: "sk-or-test" });
+    await drain(engine.stream(BASE_STREAM_OPTIONS as any));
+
+    expect(streamText).toHaveBeenCalled();
+  });
+
+  // A self-hosted or local gateway may legitimately accept no credential.
+  it("allows a keyless provider when a baseUrl is configured", async () => {
+    const { streamText } = mockAiSdk();
+    const provider = vi.fn().mockReturnValue({ id: "m" });
+    vi.doMock("@openrouter/ai-sdk-provider", () => ({
+      createOpenRouter: vi.fn().mockReturnValue(provider),
+    }));
+
+    const { createAISDKEngine } = await import("./ai-sdk-engine.js");
+    const engine = createAISDKEngine("openrouter", {
+      allowEnvFallback: false,
+      baseUrl: "http://localhost:4000/v1",
+    });
+    await drain(engine.stream(BASE_STREAM_OPTIONS as any));
+
+    expect(streamText).toHaveBeenCalled();
+  });
+});

packages/core/src/agent/engine/ai-sdk-engine.ts

@@ -23,6 +23,10 @@ import {
   supportsClaudeAdaptiveThinking,
 } from "../../shared/reasoning-effort.js";
 import { AI_SDK_MODEL_CONFIG, type AISDKProvider } from "../model-config.js";
+import {
+  LLM_MISSING_CREDENTIALS_ERROR_CODE,
+  LLM_MISSING_CREDENTIALS_MESSAGE,
+} from "./credential-errors.js";
 import {
   classifyProviderError,
   describeErrorWithCauses,
@@ -228,6 +232,8 @@ class AISDKEngine implements AgentEngine {
   private readonly provider: AISDKProvider;
   private readonly apiKey?: string;
   private readonly baseUrl?: string;
+  /** Empty for providers that need no key (ollama). */
+  private readonly requiredEnvVars: readonly string[];
   private readonly appName?: string;
   private readonly appUrl?: string;
 
@@ -243,12 +249,34 @@ class AISDKEngine implements AgentEngine {
     this.apiKey =
       config.apiKey ??
       (config.allowEnvFallback === false ? "" : getProviderApiKey(provider));
+    this.requiredEnvVars = PROVIDER_ENV_VARS[provider];
     this.baseUrl = config.baseUrl;
     this.appName = config.appName;
     this.appUrl = config.appUrl;
   }
 
   async *stream(opts: EngineStreamOptions): AsyncIterable<EngineEvent> {
+    // An absent key is not an anonymous request. Without this the provider
+    // factory is constructed with no `apiKey`, the SDK omits the Authorization
+    // header entirely, and the gateway's 401 comes back as
+    // "Missing Authentication header" — which `classifyProviderError` codes
+    // `http_401`, a transport failure naming the wrong cause. A scheduled job
+    // then repeats that doomed unauthenticated request on every tick forever.
+    // `builder-engine` and `anthropic-engine` already fail closed here; this
+    // engine was the only one that did not.
+    //
+    // A configured `baseUrl` is exempt: a self-hosted or local gateway may
+    // legitimately accept unauthenticated requests.
+    if (!this.apiKey && !this.baseUrl && this.requiredEnvVars.length > 0) {
+      yield {
+        type: "stop",
+        reason: "error",
+        error: `${LLM_MISSING_CREDENTIALS_MESSAGE} (engine "${this.name}" has no ${this.requiredEnvVars.join(" or ")})`,
+        errorCode: LLM_MISSING_CREDENTIALS_ERROR_CODE,
+      };
+      return;
+    }
+
     let aiModule: any;
     try {
       aiModule = await import("ai");

packages/core/src/agent/run-store.stale-run-recovery.spec.ts

@@ -85,6 +85,7 @@ const {
   markTurnAborted,
   reapIfStale,
   reapAllStaleRuns,
+  BACKGROUND_PROCESSING_RUN_STALE_MS,
   __resetNoRunningRunsProbeForTests,
 } = await import("./run-store.js");
 
@@ -310,19 +311,53 @@ describe("FIX 3 — stale-run reaper server-owned recovery (reapIfStale)", () =>
   it("does NOT create a successor when the dying run has no dispatch_payload to carry over", async () => {
     currentClient = makeRawClient(true);
     const { runId, thread, turn } = ids();
-    // No dispatchPayload — e.g. a row whose payload was already cleared, or
-    // one that predates this fix.
+    // No dispatchPayload — an in-process background automation, which was never
+    // HTTP-dispatched and so has no request body to rehydrate.
     await insertRun(runId, thread, turn, { dispatchMode: "background" });
     await claimBackgroundRun(runId);
     setStaleLiveness(runId, Date.now() - STALE_PAST_MS);
 
     const reaped = await reapIfStale(runId);
     expect(reaped).toBe(true);
     expect(readRow(runId)?.status).toBe("errored");
-    expect(readRow(runId)?.diag_stage).toContain("payload_missing");
+    // Named for what it is. "payload_missing" read as data loss in production
+    // forensics for the one case where nothing was ever lost.
+    expect(readRow(runId)?.diag_stage).toContain("not_redispatchable");
     expect(rowsForTurn(turn)).toHaveLength(1); // no successor inserted
   });
 
+  it("gives a claimed run with no redispatch path the wider background stale window", async () => {
+    currentClient = makeRawClient(true);
+    // A payload-less claimed run is an in-process automation: reaping it at the
+    // 45s post-claim window kills a job that is still working and that nothing
+    // can recover. It must survive to the 90s background window instead.
+    const noPayload = ids();
+    await insertRun(noPayload.runId, noPayload.thread, noPayload.turn, {
+      dispatchMode: "background",
+    });
+    await claimBackgroundRun(noPayload.runId);
+    setStaleLiveness(
+      noPayload.runId,
+      Date.now() - (BACKGROUND_PROCESSING_RUN_STALE_MS + 5_000),
+    );
+    expect(await reapIfStale(noPayload.runId)).toBe(false);
+    expect(readRow(noPayload.runId)?.status).toBe("running");
+
+    // A genuine HTTP worker (payload present) keeps the tight window, because
+    // for it an early reap does buy a durable successor.
+    const withPayload = ids();
+    await insertRun(withPayload.runId, withPayload.thread, withPayload.turn, {
+      dispatchMode: "background",
+      dispatchPayload: JSON.stringify({ threadId: withPayload.thread }),
+    });
+    await claimBackgroundRun(withPayload.runId);
+    setStaleLiveness(
+      withPayload.runId,
+      Date.now() - (BACKGROUND_PROCESSING_RUN_STALE_MS + 5_000),
+    );
+    expect(await reapIfStale(withPayload.runId)).toBe(true);
+  });
+
   it("does NOT create a successor when a newer run already exists for the same turn", async () => {
     currentClient = makeRawClient(true);
     const { runId, thread, turn } = ids();

packages/core/src/agent/run-store.ts

@@ -736,7 +736,13 @@ function backgroundAwareStaleCutoffSql(): string {
   // `CAST(? AS BIGINT)` is required: without it Postgres infers the param as
   // int4 from the int4 window literals, so the bound `Date.now()` ms epoch
   // overflows int4. The cast keeps the subtraction 64-bit; a no-op on SQLite.
-  return `(CAST(? AS BIGINT) - CASE WHEN dispatch_mode = 'background-processing' THEN ${BACKGROUND_PROCESSING_RUN_STALE_MS} WHEN dispatch_mode LIKE 'background%' THEN ${BACKGROUND_RUN_STALE_MS} ELSE ${RUN_STALE_MS} END)`;
+  // The tight post-claim window buys ONE thing: reaching the durable successor
+  // sooner (see BACKGROUND_PROCESSING_RUN_STALE_MS). A row with no
+  // `dispatch_payload` has no successor to reach — `attemptStaleRunRecovery`
+  // declines it as `not_redispatchable` — so reaping it early is pure loss: it
+  // kills an in-process background automation (scheduler/trigger) that is still
+  // working, and nothing recovers it. Those get the wider background window.
+  return `(CAST(? AS BIGINT) - CASE WHEN dispatch_mode = 'background-processing' AND dispatch_payload IS NOT NULL THEN ${BACKGROUND_PROCESSING_RUN_STALE_MS} WHEN dispatch_mode LIKE 'background%' THEN ${BACKGROUND_RUN_STALE_MS} ELSE ${RUN_STALE_MS} END)`;
 }
 
 function terminalRunEventExclusionSql(runIdColumn = "id"): string {
@@ -1546,7 +1552,7 @@ interface StaleRunRecoverySuccessor {
 type StaleRunRecoveryOutcome =
   | ({ outcome: "recovered" } & StaleRunRecoverySuccessor)
   | { outcome: "not_background" }
-  | { outcome: "payload_missing" }
+  | { outcome: "not_redispatchable" }
   | { outcome: "newer_run_exists" }
   | { outcome: "budget_exhausted" }
   | { outcome: "repeated_no_progress" };
@@ -1596,7 +1602,9 @@ function staleRecoveryDispatchPayload(payload: string): string {
  *   - the row is a background chat-turn dispatch (`dispatch_mode` starting
  *     with "background") — a foreground/foreground-self-chain run has a
  *     connected client to recover it via its own `auto_continue` re-POST.
- *   - its `dispatch_payload` is still present — without it there is nothing
+ *   - its `dispatch_payload` is still present (an in-process automation run
+ *     never had one and is declined as `not_redispatchable`, not as a loss)
+ *     — without it there is nothing
  *     to rehydrate the successor's request body from. Read HERE, before the
  *     caller's terminal write (which NULLs `dispatch_payload` on every other
  *     path), so it survives long enough to carry over.
@@ -1633,7 +1641,14 @@ async function attemptStaleRunRecovery(
   }
   const payload = row.dispatch_payload;
   if (typeof payload !== "string" || payload.length === 0) {
-    return { outcome: "payload_missing" };
+    // Not an anomaly, and specifically NOT a lost payload: every HTTP-dispatched
+    // background run writes `dispatch_payload` in the same INSERT that creates
+    // the row, so a payload-less row is one that was never HTTP-dispatched at
+    // all — an in-process automation from `runBackgroundAutomation`. It has no
+    // request body to rehydrate because there was never a request. Naming this
+    // `payload_missing` read as "we are losing payloads" in production
+    // forensics and sent a reader hunting a bug that does not exist.
+    return { outcome: "not_redispatchable" };
   }
   const threadId = row.thread_id;
   const turnId = row.turn_id ?? runId;