fix(lesson-feed): read DISPATCH_LLM_* env vars with OPENAI_* legacy fallback

.env.example

@@ -156,20 +156,27 @@ DISPATCH_AGENT_TOKEN="your_agent_token_here"
 
 # --- LLM configuration (groomer + lesson feed) ---
 
-# OpenAI-compatible API key. The hosted groomer reads DISPATCH_LLM_API_KEY
-# first, then falls back to OPENAI_API_KEY.
+# Both the hosted groomer and the lesson feed (src/lib/lesson-feed.ts) read the
+# same DISPATCH_LLM_* vars, so a deployment that only sets these works for both.
+# Set these first; the OPENAI_* vars below are a legacy fallback only.
+
+# OpenAI-compatible API key. Read by the groomer and the lesson feed.
 # DISPATCH_LLM_API_KEY="sk-..."
-# Legacy / lesson-feed fallback. Used only when DISPATCH_LLM_API_KEY is unset.
+# Legacy fallback. Used only when DISPATCH_LLM_API_KEY is unset (a one-time
+# lesson-feed warning fires when an OPENAI_* fallback is used).
 # OPENAI_API_KEY="sk-..."
 
-# OpenAI-compatible base URL. The hosted groomer reads DISPATCH_LLM_BASE_URL
-# first, then falls back to OPENAI_BASE_URL.
+# OpenAI-compatible base URL. Read by the groomer and the lesson feed.
 # DISPATCH_LLM_BASE_URL="https://api.openai.com/v1"
-# Legacy / lesson-feed fallback.
+# Legacy fallback. Used only when DISPATCH_LLM_BASE_URL is unset.
 # OPENAI_BASE_URL="https://api.openai.com/v1"
 
-# Default model for the lesson feed. The groomer uses DISPATCH_GROOMER_MODEL
-# instead when set.
+# Model for the lesson feed. Precedence: DISPATCH_LESSON_FEED_MODEL >
+# DISPATCH_GROOMER_MODEL > OPENAI_MODEL (legacy) > gpt-4o-mini.
+# DISPATCH_LESSON_FEED_MODEL="gpt-4o-mini"
+# DISPATCH_GROOMER_MODEL="gpt-4o-mini"
+# Legacy fallback. Used only when neither DISPATCH_LESSON_FEED_MODEL nor
+# DISPATCH_GROOMER_MODEL is set.
 # OPENAI_MODEL="gpt-4o-mini"
 
 # --- PR Followup (inbound webhooks) ---

README.md

@@ -160,6 +160,8 @@ These variables tune the OIDC callback, the operator-UI triage surface, the queu
 view, the inbound webhook (PR followup) handler, and the lesson feed. Most have
 safe defaults and can be omitted in small deployments.
 
+The lesson feed shares `DISPATCH_LLM_API_KEY` and `DISPATCH_LLM_BASE_URL` with the hosted groomer; `OPENAI_API_KEY` and `OPENAI_BASE_URL` are retained only as legacy fallbacks.
+
 | Variable | Required | Description |
 |----------|----------|-------------|
 | `AUTH_URL` | No | Public base URL used to construct the OIDC callback (`/api/auth/callback/...`). Optional; falls back to `NEXTAUTH_URL`. Set this when Dispatch sits behind a reverse proxy that rewrites the public origin. |
@@ -173,9 +175,10 @@ safe defaults and can be omitted in small deployments.
 | `WEBHOOK_GATEWAY_MODE` | No | When set to `true`, disables the built-in signature check because an upstream API gateway has already verified the request. Must be the literal string `"true"` or `"false"` (parsed as a string, not a boolean). |
 | `PR_FOLLOWUP_BOT_IDENTITIES` | No | Comma-separated bot logins whose PR events are ingested (default `github-actions[bot]`). Example: `github-actions[bot],dependabot[bot]`. |
 | `PR_FOLLOWUP_BRANCH_OWNERS` | No | Comma-separated GitHub logins considered the canonical owner of a followup branch. Used to suppress "needs author" nudges. |
-| `OPENAI_API_KEY` | Conditional | OpenAI API key used by the lesson feed. Ignored when `DISPATCH_LLM_API_KEY` is set. |
-| `OPENAI_BASE_URL` | No | OpenAI-compatible base URL for the lesson feed. Ignored when `DISPATCH_LLM_BASE_URL` is set. |
-| `OPENAI_MODEL` | No | Default model for the lesson feed. The groomer uses `DISPATCH_GROOMER_MODEL` instead when set. |
+| `OPENAI_API_KEY` | Conditional | Legacy OpenAI API key fallback for the lesson feed. Ignored when `DISPATCH_LLM_API_KEY` is set. |
+| `OPENAI_BASE_URL` | No | Legacy OpenAI-compatible base URL fallback for the lesson feed. Ignored when `DISPATCH_LLM_BASE_URL` is set. |
+| `DISPATCH_LESSON_FEED_MODEL` | No | Model for the lesson feed. Takes precedence over `DISPATCH_GROOMER_MODEL` and `OPENAI_MODEL`. |
+| `OPENAI_MODEL` | No | Legacy model fallback for the lesson feed. Ignored when `DISPATCH_LESSON_FEED_MODEL` or `DISPATCH_GROOMER_MODEL` is set. |
 | `DISPATCH_AGENT_NAME` | No | Display name used by the agent when posting heartbeats. Defaults to the host's `HOSTNAME` env var. |
 | `DISPATCH_CLOSED_ISSUE_RETENTION_DAYS` | No | Days that a closed issue is kept before `/api/issues/prune-closed` is allowed to remove it. Defaults to `30`. |
 | `DISPATCH_DONE_RETENTION_DAYS` | No | Days that a done issue is kept before the issue list endpoint filters it out. Defaults to `7`. |

src/lib/lesson-feed.test.ts

@@ -1,11 +1,37 @@
-import { describe, expect, it } from "vitest";
+import { describe, expect, it, vi } from "vitest";
 import {
   extractLessonFromFixOutcome,
   lessonAlreadyCovered,
+  readConfig,
   type ExtractLessonInput,
   type LessonOutcome,
 } from "./lesson-feed";
 
+/**
+ * Run `fn` with the given env vars set (or deleted when the value is
+ * undefined), restoring the previous values afterwards. Keeps the
+ * readConfig precedence tests hermetic regardless of the CI environment.
+ */
+async function withEnv(
+  vars: Record<string, string | undefined>,
+  fn: () => void | Promise<unknown>,
+): Promise<unknown> {
+  const saved: Record<string, string | undefined> = {};
+  for (const key of Object.keys(vars)) {
+    saved[key] = process.env[key];
+    if (vars[key] === undefined) delete process.env[key];
+    else process.env[key] = vars[key];
+  }
+  try {
+    return await fn();
+  } finally {
+    for (const key of Object.keys(vars)) {
+      if (saved[key] === undefined) delete process.env[key];
+      else process.env[key] = saved[key];
+    }
+  }
+}
+
 type PlannedResponse = { verdict: "no_lesson" | "lesson"; text?: string };
 
 interface CallRecord {
@@ -130,6 +156,217 @@ describe("extractLessonFromFixOutcome", () => {
   });
 });
 
+describe("readConfig (env precedence, issue #913)", () => {
+  it("prefers DISPATCH_LLM_* over OPENAI_* for apiKey and baseUrl", async () => {
+    await withEnv(
+      {
+        DISPATCH_LLM_API_KEY: "dispatch-key",
+        DISPATCH_LLM_BASE_URL: "https://dispatch.example/v1",
+        OPENAI_API_KEY: "openai-key",
+        OPENAI_BASE_URL: "https://openai.example/v1",
+      },
+      () => {
+        const cfg = readConfig();
+        expect(cfg.apiKey).toBe("dispatch-key");
+        expect(cfg.baseUrl).toBe("https://dispatch.example/v1");
+      },
+    );
+  });
+
+  it("falls back to OPENAI_* when DISPATCH_LLM_* is unset", async () => {
+    await withEnv(
+      {
+        DISPATCH_LLM_API_KEY: undefined,
+        DISPATCH_LLM_BASE_URL: undefined,
+        OPENAI_API_KEY: "openai-key",
+        OPENAI_BASE_URL: "https://openai.example/v1",
+      },
+      () => {
+        const cfg = readConfig();
+        expect(cfg.apiKey).toBe("openai-key");
+        expect(cfg.baseUrl).toBe("https://openai.example/v1");
+      },
+    );
+  });
+
+  it("treats whitespace-only env values as unset", async () => {
+    await withEnv(
+      {
+        DISPATCH_LLM_API_KEY: "  ",
+        DISPATCH_LLM_BASE_URL: "\t",
+        DISPATCH_LESSON_FEED_MODEL: " ",
+        DISPATCH_GROOMER_MODEL: "\n",
+        OPENAI_API_KEY: " ",
+        OPENAI_BASE_URL: "\t",
+        OPENAI_MODEL: "\n",
+      },
+      () => {
+        expect(readConfig()).toEqual({
+          apiKey: "",
+          baseUrl: "https://api.openai.com/v1",
+          model: "gpt-4o-mini",
+        });
+      },
+    );
+  });
+
+  it("resolves model as DISPATCH_LESSON_FEED_MODEL > DISPATCH_GROOMER_MODEL > OPENAI_MODEL > gpt-4o-mini", async () => {
+    await withEnv(
+      {
+        DISPATCH_LESSON_FEED_MODEL: "lesson-model",
+        DISPATCH_GROOMER_MODEL: "groomer-model",
+        OPENAI_MODEL: "openai-model",
+      },
+      () => {
+        expect(readConfig().model).toBe("lesson-model");
+      },
+    );
+    await withEnv(
+      {
+        DISPATCH_LESSON_FEED_MODEL: undefined,
+        DISPATCH_GROOMER_MODEL: "groomer-model",
+        OPENAI_MODEL: "openai-model",
+      },
+      () => {
+        expect(readConfig().model).toBe("groomer-model");
+      },
+    );
+    await withEnv(
+      {
+        DISPATCH_LESSON_FEED_MODEL: undefined,
+        DISPATCH_GROOMER_MODEL: undefined,
+        OPENAI_MODEL: "openai-model",
+      },
+      () => {
+        expect(readConfig().model).toBe("openai-model");
+      },
+    );
+    await withEnv(
+      {
+        DISPATCH_LESSON_FEED_MODEL: undefined,
+        DISPATCH_GROOMER_MODEL: undefined,
+        OPENAI_MODEL: undefined,
+      },
+      () => {
+        expect(readConfig().model).toBe("gpt-4o-mini");
+      },
+    );
+  });
+
+  it("defaults baseUrl to the OpenAI endpoint when neither var is set", async () => {
+    await withEnv(
+      {
+        DISPATCH_LLM_BASE_URL: undefined,
+        OPENAI_BASE_URL: undefined,
+      },
+      () => {
+        expect(readConfig().baseUrl).toBe("https://api.openai.com/v1");
+      },
+    );
+  });
+
+  it("returns an empty apiKey when no key is configured anywhere", async () => {
+    await withEnv(
+      {
+        DISPATCH_LLM_API_KEY: undefined,
+        OPENAI_API_KEY: undefined,
+      },
+      () => {
+        expect(readConfig().apiKey).toBe("");
+      },
+    );
+  });
+
+  it("fires a one-time warning when the legacy OPENAI_* path is in use", async () => {
+    // Fresh module instance so the one-shot `legacyFallbackWarned` guard is
+    // reset, independent of any earlier test in this file.
+    vi.resetModules();
+    const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
+    const mod = await import("./lesson-feed");
+    await withEnv(
+      {
+        DISPATCH_LLM_API_KEY: undefined,
+        OPENAI_API_KEY: "openai-key",
+      },
+      () => {
+        mod.readConfig();
+        mod.readConfig();
+      },
+    );
+    expect(warn).toHaveBeenCalledTimes(1);
+    expect(warn.mock.calls[0][0]).toMatch(/OPENAI_.*DISPATCH_LLM_API_KEY/);
+    warn.mockRestore();
+  });
+
+  it("warns when a DISPATCH key falls back to legacy base or model settings", async () => {
+    vi.resetModules();
+    const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
+    const mod = await import("./lesson-feed");
+    await withEnv(
+      {
+        DISPATCH_LLM_API_KEY: "dispatch-key",
+        OPENAI_BASE_URL: "https://legacy.example/v1",
+        OPENAI_MODEL: "legacy-model",
+      },
+      () => {
+        mod.readConfig();
+      },
+    );
+    expect(warn).toHaveBeenCalledTimes(1);
+    expect(warn.mock.calls[0][0]).toMatch(/OPENAI_BASE_URL.*OPENAI_MODEL/);
+    warn.mockRestore();
+  });
+
+  it("does not warn when DISPATCH_LLM_API_KEY is the active key", async () => {
+    vi.resetModules();
+    const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
+    const mod = await import("./lesson-feed");
+    await withEnv(
+      {
+        DISPATCH_LLM_API_KEY: "dispatch-key",
+        OPENAI_API_KEY: "openai-key",
+      },
+      () => {
+        mod.readConfig();
+      },
+    );
+    expect(warn).not.toHaveBeenCalled();
+    warn.mockRestore();
+  });
+});
+
+describe("extractLessonFromFixOutcome (missing-key early return, issue #913)", () => {
+  it("returns no_lesson and makes no fetch call when no key is configured", async () => {
+    const { fetcher, calls } = makeFetcher({ verdict: "lesson", text: "should not be used" });
+    const out = await withEnv(
+      {
+        DISPATCH_LLM_API_KEY: undefined,
+        OPENAI_API_KEY: undefined,
+      },
+      async () => {
+        return extractLessonFromFixOutcome(baseInput, { fetcher });
+      },
+    );
+    expect(out).toEqual({ kind: "no_lesson" });
+    expect(calls).toHaveLength(0);
+  });
+
+  it("uses the DISPATCH_LLM_API_KEY from the environment when no apiKey option is passed", async () => {
+    const { fetcher, calls } = makeFetcher({ verdict: "no_lesson" });
+    const out = await withEnv(
+      {
+        DISPATCH_LLM_API_KEY: "dispatch-key",
+        OPENAI_API_KEY: undefined,
+      },
+      async () => {
+        return extractLessonFromFixOutcome(baseInput, { fetcher });
+      },
+    );
+    expect(out).toEqual({ kind: "no_lesson" });
+    expect(calls).toHaveLength(1);
+  });
+});
+
 describe("lessonAlreadyCovered", () => {
   it("returns false when no existing AGENTS.md is provided", () => {
     expect(lessonAlreadyCovered(undefined, "anything")).toBe(false);

src/lib/lesson-feed.ts

@@ -73,12 +73,57 @@ export interface LessonFeedOptions {
   timeoutMs?: number;
 }
 
-function readConfig() {
-  return {
-    apiKey: process.env.OPENAI_API_KEY ?? "",
-    baseUrl: process.env.OPENAI_BASE_URL ?? "https://api.openai.com/v1",
-    model: process.env.OPENAI_MODEL ?? "gpt-4o-mini",
-  };
+// One-shot guard so the legacy-fallback warning below fires at most once per
+// process (the lesson feed runs on every pr-fix/tombstone outcome).
+let legacyFallbackWarned = false;
+
+/**
+ * Resolve the LLM credentials + model for the lesson feed.
+ *
+ * Precedence (mirrors src/lib/groomer/config.ts so a deployment that only
+ * configures DISPATCH_LLM_* works for both the groomer and the lesson feed):
+ *   apiKey:  DISPATCH_LLM_API_KEY  > OPENAI_API_KEY (legacy fallback)
+ *   baseUrl: DISPATCH_LLM_BASE_URL > OPENAI_BASE_URL (legacy fallback)
+ *   model:   DISPATCH_LESSON_FEED_MODEL > DISPATCH_GROOMER_MODEL
+ *            > OPENAI_MODEL (legacy fallback) > "gpt-4o-mini"
+ *
+ * When a legacy OPENAI_* value is used, a one-time console.warn fires so the
+ * fallback path is observable (issue #913).
+ */
+export function readConfig() {
+  const dispatchApiKey = process.env.DISPATCH_LLM_API_KEY?.trim() || undefined;
+  const dispatchBaseUrl = process.env.DISPATCH_LLM_BASE_URL?.trim() || undefined;
+  const dispatchLessonFeedModel = process.env.DISPATCH_LESSON_FEED_MODEL?.trim() || undefined;
+  const dispatchGroomerModel = process.env.DISPATCH_GROOMER_MODEL?.trim() || undefined;
+  const openAiApiKey = process.env.OPENAI_API_KEY?.trim() || undefined;
+  const openAiBaseUrl = process.env.OPENAI_BASE_URL?.trim() || undefined;
+  const openAiModel = process.env.OPENAI_MODEL?.trim() || undefined;
+  const apiKey = dispatchApiKey || openAiApiKey || "";
+  const baseUrl = dispatchBaseUrl || openAiBaseUrl || "https://api.openai.com/v1";
+  const model = dispatchLessonFeedModel || dispatchGroomerModel || openAiModel || "gpt-4o-mini";
+
+  // Keep the legacy path observable without logging credentials or repeating the
+  // warning for every feed trigger. This also covers individual fallback values:
+  // for example, a DISPATCH key can still use the legacy model or base URL.
+  const usingLegacyApiKey = !dispatchApiKey && !!openAiApiKey;
+  const usingLegacyBaseUrl = !dispatchBaseUrl && !!openAiBaseUrl;
+  const usingLegacyModel = !dispatchLessonFeedModel && !dispatchGroomerModel && !!openAiModel;
+  const usingLegacyConfig = usingLegacyApiKey || usingLegacyBaseUrl || usingLegacyModel;
+  if (!legacyFallbackWarned && apiKey && usingLegacyConfig) {
+    legacyFallbackWarned = true;
+    const fallbackVars = [
+      usingLegacyApiKey ? "OPENAI_API_KEY" : undefined,
+      usingLegacyBaseUrl ? "OPENAI_BASE_URL" : undefined,
+      usingLegacyModel ? "OPENAI_MODEL" : undefined,
+    ].filter((name): name is string => !!name);
+    console.warn(
+      `[lesson-feed] using legacy ${fallbackVars.join(" / ")} env var(s); ` +
+        "set DISPATCH_LLM_API_KEY / DISPATCH_LLM_BASE_URL and " +
+        "DISPATCH_LESSON_FEED_MODEL (or DISPATCH_GROOMER_MODEL) instead",
+    );
+  }
+
+  return { apiKey, baseUrl, model };
 }
 
 /**