From 3a5a3cc6542e3f3d3260fab50f3b51003ada1d25 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 26 Jun 2026 18:33:05 -0700 Subject: [PATCH] fix(selfhost): make AI-provider failures loud + attribute the actual reviewer + boot CLI preflight MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1566. The self-host engine silently produced ZERO AI output for 202 reviews when the claude CLI was absent: spawn('claude') ENOENT'd, runWorkersOpinion's bare catch swallowed it, and record() logged the Workers-AI model ids — so the outage was invisible (no Review-summary/blockers/nits on any comment). Now: (1) runWorkersOpinion logs every failed provider attempt; (2) record() attributes the actual configured reviewer (AI_PROVIDER:AI_MODEL) not BEST_REVIEW_MODELS; (3) a boot preflight shouts selfhost_ai_cli_missing when AI_PROVIDER=claude-code|codex but the CLI is not on PATH. --- src/server.ts | 31 ++++++++++++++++++++++++++-- src/services/ai-review.ts | 27 +++++++++++++++++++++--- test/unit/ai-review-advisory.test.ts | 13 ++++++++++++ 3 files changed, 66 insertions(+), 5 deletions(-) diff --git a/src/server.ts b/src/server.ts index 190ad3236..7855f6d89 100644 --- a/src/server.ts +++ b/src/server.ts @@ -6,7 +6,8 @@ // Serves the Hono app via @hono/node-server, drives the queue with the same processJob, ticks the same // scheduled handler on a timer, exposes /health /ready /metrics, and shuts down gracefully. The Cloudflare // Worker (src/index.ts) is untouched — this is a parallel entry the self-host esbuild build bundles. -import { readFileSync, writeFileSync } from "node:fs"; +import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import { delimiter, join } from "node:path"; import { randomUUID } from "node:crypto"; import { DatabaseSync } from "node:sqlite"; import { serve } from "@hono/node-server"; @@ -313,6 +314,30 @@ async function main(): Promise { provider: process.env.AI_PROVIDER, }), ); + // Fail-LOUD preflight (#1566): a CLI-subscription provider (claude-code/codex) reviews by spawning the CLI as a + // subprocess; if the binary is absent (image built without INSTALL_AI_CLIS=true) the spawn ENOENTs and EVERY AI + // review silently degrades to "no usable output". Shout at boot so the misconfig is obvious, never invisible. + const requiredCli = + process.env.AI_PROVIDER === "claude-code" + ? "claude" + : process.env.AI_PROVIDER === "codex" + ? "codex" + : null; + if ( + requiredCli && + !(process.env.PATH ?? "") + .split(delimiter) + .some((d) => d && existsSync(join(d, requiredCli))) + ) + console.error( + JSON.stringify({ + level: "error", + event: "selfhost_ai_cli_missing", + provider: process.env.AI_PROVIDER, + cli: requiredCli, + message: `AI_PROVIDER=${process.env.AI_PROVIDER} but '${requiredCli}' is not on PATH — every AI review will produce NO output. Rebuild the image with --build-arg INSTALL_AI_CLIS=true (or use the published image) and authenticate the CLI.`, + }), + ); // Dedicated RAG embed provider (keeps the review chain frontier-only): when AI_EMBED_BASE_URL is set, embeddings // route to a SEPARATE openai-compatible endpoint (e.g. ollama at http://ollama:11434/v1, model bge-m3) instead of // the review chain — so a Claude/Codex outage never falls reviews back to a weak local model. Unset ⇒ absent ⇒ @@ -407,7 +432,9 @@ async function main(): Promise { name: "qdrant", check: () => withTimeout( - fetch(qdrantReadyzUrl(qdrantUrl), { signal: AbortSignal.timeout(1500) }) + fetch(qdrantReadyzUrl(qdrantUrl), { + signal: AbortSignal.timeout(1500), + }) .then((r) => r.ok) .catch(() => false), ), diff --git a/src/services/ai-review.ts b/src/services/ai-review.ts index 25d482fc4..c4492df38 100644 --- a/src/services/ai-review.ts +++ b/src/services/ai-review.ts @@ -24,6 +24,7 @@ import { import { sanitizePublicComment } from "../queue-intelligence"; import { defangReviewInput } from "../review/safety"; import { convergedFeatureActive } from "../review/feature-activation"; +import { errorMessage } from "../utils/json"; import type { ReviewProfile } from "../signals/focus-manifest"; /** @@ -493,8 +494,19 @@ async function runWorkersOpinion( ); const parsed = parseModelReview(coerceAiText(result)); if (parsed) return parsed; - } catch { - /* retry / fall through to fallback */ + } catch (error) { + // Fail-LOUD (#1566): a provider/CLI failure (e.g. the claude-code CLI absent → spawn ENOENT, or an auth/API + // error) must be VISIBLE, not silently swallowed into a "no usable output" review. Log every failed attempt; + // the loop still falls through to the fallback model so a transient error doesn't abort the whole review. + console.warn( + JSON.stringify({ + level: "warn", + event: "ai_review_provider_attempt_failed", + model, + attempt, + error: errorMessage(error), + }), + ); } } } @@ -1017,6 +1029,15 @@ export async function runGittensoryAiReview( }; } +/** The actual configured reviewer label for usage attribution (#1566): the self-host `AI_PROVIDER:AI_MODEL` when set, + * else the Worker dual-AI models. Without this, self-host claude-code reviews were mis-logged as the Workers-AI model + * ids (`@cf/openai/gpt-oss-120b+…`), which hid the silent claude-CLI-missing outage. */ +function reviewerModelLabel(env: Env): string { + const e = env as unknown as { AI_PROVIDER?: string; AI_MODEL?: string }; + if (!e.AI_PROVIDER) return BEST_REVIEW_MODELS.join("+"); + return [e.AI_PROVIDER, e.AI_MODEL].filter(Boolean).join(":"); +} + async function record( env: Env, input: GittensoryAiReviewInput, @@ -1032,7 +1053,7 @@ async function record( route: "github_app.ai_review", model: input.providerKey ? `byok:${input.providerKey.provider}` - : BEST_REVIEW_MODELS.join("+"), + : reviewerModelLabel(env), status, estimatedNeurons, detail, diff --git a/test/unit/ai-review-advisory.test.ts b/test/unit/ai-review-advisory.test.ts index 65e7cd7b7..c3dccfcc0 100644 --- a/test/unit/ai-review-advisory.test.ts +++ b/test/unit/ai-review-advisory.test.ts @@ -135,6 +135,19 @@ describe("runAiReviewForAdvisory", () => { } }); + it("degrades when the provider throws and records the CONFIGURED reviewer, not the Workers-AI models (#1566)", async () => { + // The CLI/provider throwing (e.g. claude-code binary absent → ENOENT) must degrade to no-usable-output, and the + // usage event must attribute the ACTUAL configured reviewer — not the hardcoded Workers-AI ids that hid the + // silent outage. Exercises runWorkersOpinion's now-logging catch + reviewerModelLabel's provider arm. + const env = aiEnv(async () => { throw new Error("claude CLI not found"); }); + (env as unknown as { AI_PROVIDER: string; AI_MODEL: string }).AI_PROVIDER = "claude-code"; + (env as unknown as { AI_PROVIDER: string; AI_MODEL: string }).AI_MODEL = "claude-sonnet-4-6"; + const result = await runAiReviewForAdvisory(env, { settings: { aiReviewMode: "advisory" } as RepositorySettings, advisory: advisory(), repoFullName: "acme/widgets", pr, author: "alice", confirmedContributor: true }); + expect(result).toBeUndefined(); // provider threw → no usable output, degraded not crashed + const usage = await env.DB.prepare("SELECT model FROM ai_usage_events WHERE feature = 'ai_review_pr' ORDER BY created_at DESC LIMIT 1").first<{ model: string }>(); + expect(usage?.model).toBe("claude-code:claude-sonnet-4-6"); + }); + it("no-ops for a non-confirmed contributor under the gittensor pack and when there is no head SHA", async () => { const env = aiEnv(async () => ({ response: defectJson() })); const base = { settings: { aiReviewMode: "block", gatePack: "gittensor" } as RepositorySettings, repoFullName: "acme/widgets", pr, author: "alice" };