From fcb802ef48cadee6fe98695b25d9d324e9745cf7 Mon Sep 17 00:00:00 2001 From: Mao Nakamoto <41178744+maonakamoto@users.noreply.github.com> Date: Wed, 2 Sep 2026 13:22:02 +0200 Subject: [PATCH] fix(health): stop gating HTTP status on AI vendor health in strict mode ?strict=1 turned an LLM outage into an HTTP 503 for the health endpoint, which a monitoring/orchestration system reading that status would treat as a reason to restart the process -- but a dead API key or exhausted free-tier budget can't be fixed by a restart, so that's a pointless restart loop for a problem restarting can't solve. Liveness now always answers 200 once the database is reachable; LLM state stays in the body as an informational field, unconditionally. The strict param existed only to drive this gating, so it's removed rather than left as dead branching. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01HVwg8DKHQktxJuHeLM3xpG --- app/api/health/route.ts | 31 +++++++++----------------- tests/__tests__/lib/llm-health.test.ts | 14 +++++++----- 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/app/api/health/route.ts b/app/api/health/route.ts index 8d7fac15..a8114edc 100644 --- a/app/api/health/route.ts +++ b/app/api/health/route.ts @@ -1,26 +1,22 @@ -import { type NextRequest } from 'next/server'; import { supabase, isSupabaseConfigured } from '@/lib/supabase'; import { jsonSuccess, jsonServiceUnavailable } from '@/lib/api'; import { logger } from '@/lib/logger'; import { getLLMHealth } from '@/lib/llm-health'; /** - * Health check, in two flavours. + * Health check (liveness): is this process serving and can it reach its + * database? Always 200 once the database answers, even when the LLM chain is + * down, because restarting the app does not fix an expired API key -- + * failing liveness on it would just get a healthy process killed, and would + * fail deploy gates on a problem no deploy caused. * - * Default (liveness): is this process serving and can it reach its database? - * Answers 200 even when the LLM chain is down, because restarting the app does - * not fix an expired API key -- failing liveness on it would just get a healthy - * process killed, and would fail deploy gates on a problem no deploy caused. - * - * ?strict=1 (readiness): is the PRODUCT working? 503 once the LLM chain is - * consistently failing. Point alerting here. - * - * Either way the body carries the real state. This endpoint used to report - * only the database, so on 2026-08-28 it said "healthy" while every AI feature - * on the site was failing on an invalid Groq key. + * The body still carries the real LLM state as an informational field, so + * alerting/dashboards can see it without the process being torn down for it. + * This endpoint used to report only the database, so on 2026-08-28 it said + * "healthy" while every AI feature on the site was failing on an invalid Groq + * key. */ -export async function GET(request: NextRequest) { - const strict = request.nextUrl.searchParams.get('strict') === '1'; +export async function GET() { const llm = getLLMHealth(); try { @@ -39,11 +35,6 @@ export async function GET(request: NextRequest) { const status = llm.status === 'down' ? 'down' : llm.status === 'degraded' ? 'degraded' : 'healthy'; - if (strict && llm.status === 'down') { - logger.error('Health check (strict): LLM chain is down', { lastError: llm.lastError }); - return jsonServiceUnavailable('AI provider unavailable'); - } - return jsonSuccess({ status, database: 'connected', llm }, { cache: 'PUBLIC_SHORT' }); } catch (error) { logger.error('Health check failed:', error); diff --git a/tests/__tests__/lib/llm-health.test.ts b/tests/__tests__/lib/llm-health.test.ts index 5a32ad71..544b11d2 100644 --- a/tests/__tests__/lib/llm-health.test.ts +++ b/tests/__tests__/lib/llm-health.test.ts @@ -82,13 +82,15 @@ describe('chat routes do not dress a failure as success', () => { expect(source).toMatch(/llm/); }); - // Liveness must not fail on a dependency a restart cannot fix -- otherwise a + // Health must not fail on a dependency a restart cannot fix -- otherwise a // stale API key gets a perfectly healthy process killed, and fails deploy - // gates on a problem no deploy caused. Readiness is where that belongs. - it('health separates liveness from readiness', () => { + // gates on a problem no deploy caused. That holds for every caller, so + // there must be no query param that flips LLM status into an HTTP failure. + it('health never gates its HTTP status on LLM state', () => { const source = readFileSync(join(process.cwd(), 'app/api/health/route.ts'), 'utf-8'); - expect(source).toContain('strict'); - // the 503-on-LLM path must be gated behind strict, never unconditional - expect(source).toMatch(/if \(strict && llm\.status === 'down'\)/); + expect(source).not.toMatch( + /llm\.status === 'down'\)\s*{?\s*[\s\S]{0,80}jsonServiceUnavailable/, + ); + expect(source).not.toContain('strict'); }); });