Skip to content

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

Merged
joryirving merged 2 commits into
mainfrom
foreman/wl-misospace-dispatch-913/issue-913
Sep 4, 2026
Merged

fix(lesson-feed): read DISPATCH_LLM_* env vars with OPENAI_* legacy fallback#937
joryirving merged 2 commits into
mainfrom
foreman/wl-misospace-dispatch-913/issue-913

Conversation

@itsmiso-ai

@itsmiso-ai itsmiso-ai commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

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

its-saffron[bot]

This comment was marked as outdated.

its-saffron[bot]

This comment was marked as outdated.

its-saffron[bot]

This comment was marked as outdated.

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>
@itsmiso-ai
itsmiso-ai force-pushed the foreman/wl-misospace-dispatch-913/issue-913 branch from 6c8deee to c011d5a Compare September 4, 2026 08:11
@itsmiso-ai itsmiso-ai added the needs-human Human input or decision is required. label Sep 4, 2026
@itsmiso-ai

itsmiso-ai commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

⚠️ This PR fix item has been marked as BLOCKED and needs human attention.

Reason: Failing check: npm audit (failure)

Total attempts: 6

Attempts by lane:

  • NORMAL: 6 attempts

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:
npm warn node_modules/eslint-config-next

dispatch@0.5.52 postinstall
test -f prisma/schema.prisma && prisma generate || true

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
run npm fund for details

found 0 vulnerabilities
npm warn allow-scripts 4 packages have install scripts not yet covered by allowScripts:
npm warn allow-scripts @prisma/engines@7.10.0 (postinstall: node scripts/postinstall.js)
npm warn allow-scripts esbuild@0.28.2 (postinstall: node install.js)
npm warn allow-scripts prisma@7.10.0 (preinstall: node scripts/preinstall-entry.js)
npm warn allow-scripts unrs-resolver@1.12.2 (postinstall: node postinstall.js)
npm warn allow-scripts
npm warn allow-scripts Run npm approve-scripts --allow-scripts-pending to review, or npm approve-scripts <pkg> to allow.
##[group]Run npm run audit
�[36;1mnpm run audit�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]

dispatch@0.5.52 audit
npm audit --omit=dev --audit-level=high

npm warn audit 503 Service Unavailable - POST https://registry.npmjs.org/-/npm/v1/security/advisories/bulk - Service Unavailable
{ error: 'Service Unavailable' }
npm error audit endpoint returned an error
npm error A complete log of this run can be found in: /home/runner/.npm/_logs/2026-09-04T08_14_12_944Z-debug-0.log
##[error]Process completed with exit code 1.
Post job cleanup.
##[start-action display=Setup Node;id=__ae51d9db-5252-4458-8bfa-363965f0d7aa.__actions_setup-node]
##[end-action id=__ae51d9db-5252-4458-8bfa-363965f0d7aa.__actions_setup-node;outcome=skipped;conclusion=skipped;duration_ms=0]
Post job cleanup.
[command]/usr/bin/git version
git version 2.55.0
Temporarily overriding HOME='/home/runner/work/_temp/55bfedbc-0758-4edc-bcea-0a0e5acd1c45' before making global git config changes
Adding repository directory to the temporary git global config as a safe directory
[command]/usr/bin/git config --global --add safe.directory /home/runner/work/dispatch/dispatch
Removing SSH command configuration
[command]/usr/bin/git config --local --name-only --get-regexp core.sshCommand
[command]/usr/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
Removing HTTP extra header
[command]/usr/bin/git config --local --name-only --get-regexp http.https://github.com/.extraheader

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

@its-saffron
its-saffron Bot dismissed their stale review September 4, 2026 14:15

Superseded by a newer automated review for this pull request.

its-saffron[bot]

This comment was marked as outdated.

@joryirving joryirving added the ai-review Request an AI pull request review. label Sep 4, 2026
@its-saffron
its-saffron Bot dismissed their stale review September 4, 2026 14:17

Superseded by a newer automated review for this pull request.

@its-saffron its-saffron Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.tsreadConfig() 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-shot legacyFallbackWarned guard 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 — New describe("readConfig (env precedence, issue #913)") block pins: (a) DISPATCH_* wins over OPENAI_* for apiKey and baseUrl; (b) whitespace-only values are treated as unset; (c) full model precedence chain including the gpt-4o-mini default; (d) baseUrl defaults to https://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 that DISPATCH_LLM_API_KEY flows through to the fetcher. vi.resetModules() is used to reset the one-shot guard between tests — correct, but worth noting it relies on vi.resetModules having 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 read DISPATCH_LLM_*; legacy vars are relabeled "Legacy fallback." with explicit note about the one-time warning. DISPATCH_LESSON_FEED_MODEL and DISPATCH_GROOMER_MODEL are now both documented in the file. Matches the acceptance criteria.
  • README.md — Adds a one-line note that the lesson feed shares DISPATCH_LLM_* with the groomer, and updates the env-var table to mark OPENAI_API_KEY, OPENAI_BASE_URL, OPENAI_MODEL as "Legacy … fallback" and to add DISPATCH_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. The auth_changes risk flag is a classifier artifact of .env.example being 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 for Bearer, Authorization, secret, token, OIDC, NextAuth, session references 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, and AUTH_* / OIDC vars are untouched in the diff. readConfig() only reads DISPATCH_LLM_*, DISPATCH_LESSON_FEED_MODEL, DISPATCH_GROOMER_MODEL, and OPENAI_*; it never reads, writes, or logs any session, bearer, or webhook credential. The new console.warn explicitly 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_URL as canonical (groomer-side). This PR extends that contract to the lesson feed and labels OPENAI_* as legacy, which is the natural extension of the documented "preferred (v0.2.1+)" pattern.
  • Conventions observed: no secrets committed (.env.example only, all values are illustrative sk-... 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 with DISPATCH_LANE_CONFIG_JSON style 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:

  1. "src/lib/lesson-feed.ts reads DISPATCH_LLM_API_KEY (with OPENAI_API_KEY as documented fallback)" — ✅ lines 92–105 of the new file.
  2. "DISPATCH_LLM_BASE_URL (with OPENAI_BASE_URL as documented fallback)" — ✅ same range.
  3. "either DISPATCH_LESSON_FEED_MODEL or DISPATCH_GROOMER_MODEL (with OPENAI_MODEL as documented fallback)" — ✅ model precedence implemented exactly as specified.
  4. ".env.example is updated to mark OPENAI_* as the lesson-feed legacy fallback and to point operators at DISPATCH_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)."
  5. "A test in src/lib/lesson-feed.test.ts pins the precedence (DISPATCH_* > OPENAI_*) and the missing-key early-return" — ✅ both describe blocks added.
  6. "A startup log line or one-time warning fires when OPENAI_* is used while DISPATCH_* is set, so the legacy fallback path is observable" — ✅ legacyFallbackWarned guard + console.warn in readConfig.

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 between describe blocks 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 legacyFallbackWarned flag survives a hot-reload in next dev is irrelevant for production (process-scope is fine) and irrelevant for the issue's acceptance criteria.

@its-saffron its-saffron Bot removed the ai-review Request an AI pull request review. label Sep 4, 2026
@joryirving
joryirving merged commit 5caef62 into main Sep 4, 2026
13 checks passed
@joryirving
joryirving deleted the foreman/wl-misospace-dispatch-913/issue-913 branch September 4, 2026 14:18
@its-miso its-miso Bot mentioned this pull request Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-human Human input or decision is required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[P2] lesson-feed reads OPENAI_API_KEY / OPENAI_BASE_URL / OPENAI_MODEL, not the DISPATCH_LLM_* vars the groomer uses

2 participants