From c011d5a9455922e14430426492588fd1759759aa Mon Sep 17 00:00:00 2001 From: Saffron <263493777+itsmiso-ai@users.noreply.github.com> Date: Fri, 4 Sep 2026 08:11:54 +0000 Subject: [PATCH] fix(lesson-feed): use Dispatch LLM configuration with legacy fallbacks Prefer DISPATCH_LLM_API_KEY, DISPATCH_LLM_BASE_URL, and DISPATCH_LESSON_FEED_MODEL/DISPATCH_GROOMER_MODEL while retaining the documented OpenAI fallbacks and warning when legacy settings are active. Cover precedence and missing-key behavior with tests. Fixes #913 Signed-off-by: Saffron <263493777+itsmiso-ai@users.noreply.github.com> --- .env.example | 23 ++-- README.md | 9 +- src/lib/lesson-feed.test.ts | 239 +++++++++++++++++++++++++++++++++++- src/lib/lesson-feed.ts | 57 ++++++++- 4 files changed, 310 insertions(+), 18 deletions(-) diff --git a/.env.example b/.env.example index da358abe..29a10d26 100644 --- a/.env.example +++ b/.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) --- diff --git a/README.md b/README.md index 2fc6f6fd..08f4831a 100644 --- a/README.md +++ b/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`. | diff --git a/src/lib/lesson-feed.test.ts b/src/lib/lesson-feed.test.ts index 20004e64..5a50276f 100644 --- a/src/lib/lesson-feed.test.ts +++ b/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, + fn: () => void | Promise, +): Promise { + const saved: Record = {}; + 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); diff --git a/src/lib/lesson-feed.ts b/src/lib/lesson-feed.ts index 5c8285cb..8241ffd9 100644 --- a/src/lib/lesson-feed.ts +++ b/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 }; } /**