fix(lesson-feed): read DISPATCH_LLM_* env vars with OPENAI_* legacy fallback - #937
Conversation
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>
6c8deee to
c011d5a
Compare
Reason: Failing check: npm audit (failure) Total attempts: 6 Attempts by lane:
Final failure: foreman fix Workload prfix-misospace-dispatch-937 succeeded but PR still has a failing check after 3/3 attempts Failing run(s):
Last attempt: CI check "npm audit" failed (failure) on this PR. Error from the job log:
Loaded Prisma config from prisma.config.ts. Prisma schema loaded from prisma/schema.prisma. ✔ Generated Prisma Client (v7.10.0) to ./node_modules/@prisma/client in 333ms Start by importing your Prisma Client (See: https://pris.ly/d/importing-client) added 663 packages, and audited 664 packages in 1m 202 packages are looking for funding found 0 vulnerabilities
npm warn audit 503 Service Unavailable - POST https://registry.npmjs.org/-/npm/v1/security/advisories/bulk - Service Unavailable Full log: https://github.com/misospace/dispatch/actions/runs/33852283434/job/100957564288 Fix the root cause the log shows. Latest note: foreman fix Workload prfix-misospace-dispatch-937 succeeded but PR still has a failing check after 3/3 attempts Posted automatically by Dispatch on 2026-09-04T08:30:07.645Z |
Superseded by a newer automated review for this pull request.
Superseded by a newer automated review for this pull request.
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M3@https://litellm.jory.dev/v1 (anthropic) — routed smart (risk match: auth_changes)
Recommendation: approve
The PR directly satisfies issue PR 913's acceptance criteria: lesson-feed now reads DISPATCH_LLM_* first with documented OPENAI_* legacy fallbacks, model precedence is DISPATCH_LESSON_FEED_MODEL > DISPATCH_GROOMER_MODEL > OPENAI_MODEL > gpt-4o-mini, .env.example and README.md are updated to reflect this, tests pin precedence + missing-key early-return, and a one-time console.warn makes the legacy fallback observable.
Change-by-change findings
src/lib/lesson-feed.ts—readConfig()is exported and rewritten with explicit precedence. Whitespace-only env values are treated as unset via.trim() || undefined, which is a nice robustness improvement. The one-shotlegacyFallbackWarnedguard prevents log spam when the feed runs on every pr-fix/tombstone outcome. The warning composes a list of which specific legacy vars are in use (apiKey / baseUrl / model) without ever logging the credential values themselves.src/lib/lesson-feed.test.ts— Newdescribe("readConfig (env precedence, issue #913)")block pins: (a)DISPATCH_*wins overOPENAI_*for apiKey and baseUrl; (b) whitespace-only values are treated as unset; (c) full model precedence chain including thegpt-4o-minidefault; (d) baseUrl defaults tohttps://api.openai.com/v1; (e) empty apiKey when nothing is set; (f) one-time warning fires when legacy is used; (g) warning covers mixed cases (DISPATCH key + legacy base/model); (h) no warning when DISPATCH key is active. A second block verifies the missing-key early-return still fires with zero fetch calls, and thatDISPATCH_LLM_API_KEYflows through to the fetcher.vi.resetModules()is used to reset the one-shot guard between tests — correct, but worth noting it relies onvi.resetModuleshaving the expected effect in this Vitest version; CI's green Tests run is the empirical confirmation..env.example— Section header rewritten to make clear both groomer and lesson feed readDISPATCH_LLM_*; legacy vars are relabeled "Legacy fallback." with explicit note about the one-time warning.DISPATCH_LESSON_FEED_MODELandDISPATCH_GROOMER_MODELare now both documented in the file. Matches the acceptance criteria.README.md— Adds a one-line note that the lesson feed sharesDISPATCH_LLM_*with the groomer, and updates the env-var table to markOPENAI_API_KEY,OPENAI_BASE_URL,OPENAI_MODELas "Legacy … fallback" and to addDISPATCH_LESSON_FEED_MODEL.
Must-check responses
- review auth flow for regression — This PR does not touch
src/lib/auth.ts,lib/client-auth.ts, the NextAuth setup, OIDC, Basic auth, bearer-token verification, or any HTTP middleware. Theauth_changesrisk flag is a classifier artifact of.env.examplebeing in the diff; the change to that file is purely documentation of existing or new lesson-feed LLM env vars and does not alter the auth surface. No auth-flow regression is possible from this delta. The diff was scanned forBearer,Authorization,secret,token,OIDC,NextAuth,sessionreferences and none of the changed lines introduce or alter any of these. - verify session token handling is correct — Verified:
DISPATCH_AGENT_TOKEN,NEXTAUTH_SECRET,WEBHOOK_SECRET, andAUTH_*/ OIDC vars are untouched in the diff.readConfig()only readsDISPATCH_LLM_*,DISPATCH_LESSON_FEED_MODEL,DISPATCH_GROOMER_MODEL, andOPENAI_*; it never reads, writes, or logs any session, bearer, or webhook credential. The newconsole.warnexplicitly avoids printing values — only var names — so it cannot leak a secret to logs.
Standards Compliance
- AGENTS.md env-var section already documents
DISPATCH_LLM_API_KEY/DISPATCH_LLM_BASE_URLas canonical (groomer-side). This PR extends that contract to the lesson feed and labelsOPENAI_*as legacy, which is the natural extension of the documented "preferred (v0.2.1+)" pattern. - Conventions observed: no secrets committed (
.env.exampleonly, all values are illustrativesk-...placeholders), validation done before DB use (no DB access added here, but env parsing uses safe.trim() || undefined), error handling unchanged, no agent-specific names added to generic docs, JSON-shaped config consistent withDISPATCH_LANE_CONFIG_JSONstyle of "canonical var, document legacy fallback." - The legacy
OPENAI_*path remains supported as a documented fallback, not removed — matching AGENTS.md's "Resolution order" pattern (DISPATCH_* > alias) used elsewhere. - Prisma untouched, no migrations, no Docker changes — consistent with the change scope.
Linked Issue Fit
Issue PR 913 acceptance criteria, mapped to the diff:
- "
src/lib/lesson-feed.tsreadsDISPATCH_LLM_API_KEY(withOPENAI_API_KEYas documented fallback)" — ✅ lines 92–105 of the new file. - "
DISPATCH_LLM_BASE_URL(withOPENAI_BASE_URLas documented fallback)" — ✅ same range. - "either
DISPATCH_LESSON_FEED_MODELorDISPATCH_GROOMER_MODEL(withOPENAI_MODELas documented fallback)" — ✅ model precedence implemented exactly as specified. - ".env.example is updated to mark
OPENAI_*as the lesson-feed legacy fallback and to point operators atDISPATCH_LLM_*first" — ✅ section rewritten, comment on the OPENAI_API_KEY line now says "Legacy fallback. Used only when DISPATCH_LLM_API_KEY is unset (a one-time lesson-feed warning fires when an OPENAI_* fallback is used)." - "A test in
src/lib/lesson-feed.test.tspins the precedence (DISPATCH_* > OPENAI_*) and the missing-key early-return" — ✅ both describe blocks added. - "A startup log line or one-time warning fires when
OPENAI_*is used whileDISPATCH_*is set, so the legacy fallback path is observable" — ✅legacyFallbackWarnedguard +console.warninreadConfig.
Expected files clause ("src/lib/lesson-feed.ts, .env.example, docs/*.md") — the PR touches src/lib/lesson-feed.ts and .env.example. It updates README.md (not docs/*.md) for the env-var table; README.md is the existing canonical place for that table per AGENTS.md's reference, so this is an acceptable substitute. No docs/*.md file referenced OPENAI_* for the lesson feed, so no docs file strictly required updating beyond README.
Unknowns or Needs Verification
- The Vitest
vi.resetModules()behavior betweendescribeblocks is not independently verified against the repo's Vitest version here, but the CI Tests run is green on the exact head SHA (703c791), which is the strongest available evidence the reset actually works as the test author intends. - Whether the
legacyFallbackWarnedflag survives a hot-reload innext devis irrelevant for production (process-scope is fine) and irrelevant for the issue's acceptance criteria.
Updated the lesson feed to use DISPATCH_LLM_* and lesson-feed model settings, retained OpenAI fallbacks with observable warnings, documented the configuration, and added precedence/missing-key tests; lint, typecheck, and all 2,540 tests pass.
Fixes #913