diff --git a/.env.example b/.env.example index 1274495d16..8fddd6e459 100644 --- a/.env.example +++ b/.env.example @@ -450,13 +450,14 @@ REDIS_URL=redis://redis:6379 # REQUIRED for the self-host review # Anthropic settings cannot be mixed up. # # AI_PROVIDER=ollama # ollama | openai-compatible | openai | anthropic | claude-code | -# # codex. A COMMA-LIST of TWO providers is a DUAL reviewer pair -# # (e.g. "claude-code,codex") combined per AI_COMBINE below; for a -# # single provider it's just that one (extra entries are fallbacks). -# AI_COMBINE=synthesis # how two reviewers decide (#dual-ai-combiner): single | consensus | -# # synthesis. consensus = block only when BOTH flag a defect (lone -# # flag → hold). synthesis (default for two) = both review, then ONE -# # merged decision. single = one reviewer's verdict (auto when 1). +# # codex. A comma-list is a FALLBACK chain by default: +# # AI_PROVIDER=codex,claude-code runs Codex first and only spends +# # Claude Code tokens if Codex fails/exhausts. +# AI_DUAL_REVIEW=0 # opt-in only: set to 1/true/yes/on when the first two providers +# # should run as independent reviewers instead of fallback. +# AI_COMBINE=synthesis # dual-review mode only: single | consensus | synthesis. +# # consensus = block only when BOTH flag a defect (lone flag → hold). +# # synthesis = both review, then ONE merged decision. # AI_ON_MERGE=either # synthesis merge rule: either (block if EITHER reviewer flags) | # # both (block only when both do). Ignored unless AI_COMBINE=synthesis. # AI_DAILY_NEURON_BUDGET=10000000 # daily spend cap (Cloudflare Workers AI "neurons") shared by AI @@ -492,7 +493,7 @@ REDIS_URL=redis://redis:6379 # REQUIRED for the self-host review # Claude Code subscription reviewer (AI_PROVIDER=claude-code). # CLAUDE_CODE_OAUTH_TOKEN= # from `claude setup-token` # CLAUDE_AI_MODEL=claude-sonnet-4-6 # any `claude` CLI model id/alias, e.g. sonnet | opus | claude-opus-4-8 -# CLAUDE_AI_EFFORT=high # low | medium | high | xhigh | max +# CLAUDE_AI_EFFORT=medium # low | medium | high | xhigh | max # CLAUDE_AI_TIMEOUT_MS= # override CLI timeout in ms; unset scales by effort (low/medium 120s, high 240s, xhigh 360s, max 600s) # # Codex (ChatGPT subscription) reviewer is fail-closed by default for self-host PR review: `codex exec` stores its @@ -501,7 +502,7 @@ REDIS_URL=redis://redis:6379 # REQUIRED for the self-host review # GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER=1 # Do NOT set CODEX_HOME for the app container; the provider rejects it so credentials are not advertised in env. # CODEX_AI_MODEL=gpt-5.5 # omit to let the Codex account default choose; set explicitly for repeatable reviews -# CODEX_AI_EFFORT=high # low | medium | high | xhigh. `max` is accepted and maps to xhigh. +# CODEX_AI_EFFORT=medium # low | medium | high | xhigh. `max` is accepted and maps to xhigh. # CODEX_AI_TIMEOUT_MS= # override CLI timeout in ms; unset scales by effort (low/medium 120s, high 240s, xhigh 360s) # # Codex service speed is standard by default. No fast/priority tier is requested by this stack. # AI_EMBED_MODEL=nomic-embed-text:latest # embedding model for RAG (openai-compatible /embeddings). Its output diff --git a/apps/gittensory-ui/src/lib/selfhost-env-reference.ts b/apps/gittensory-ui/src/lib/selfhost-env-reference.ts index 4a08b8e3b9..559a9e3434 100644 --- a/apps/gittensory-ui/src/lib/selfhost-env-reference.ts +++ b/apps/gittensory-ui/src/lib/selfhost-env-reference.ts @@ -7,7 +7,11 @@ export type SelfHostEnvReferenceRow = { export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ { name: "AI_COMBINE", - firstReference: "src/selfhost/ai.ts:982", + firstReference: "src/selfhost/ai.ts:1000", + }, + { + name: "AI_DUAL_REVIEW", + firstReference: "src/selfhost/ai.ts:975", }, { name: "AI_EMBED_API_KEY", @@ -23,7 +27,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "AI_ON_MERGE", - firstReference: "src/selfhost/ai.ts:984", + firstReference: "src/selfhost/ai.ts:1002", }, { name: "AI_PROVIDER", @@ -386,11 +390,12 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [ "| Name | First reference |", "| --- | --- |", - "| `AI_COMBINE` | `src/selfhost/ai.ts:982` |", + "| `AI_COMBINE` | `src/selfhost/ai.ts:1000` |", + "| `AI_DUAL_REVIEW` | `src/selfhost/ai.ts:975` |", "| `AI_EMBED_API_KEY` | `src/server.ts:440` |", "| `AI_EMBED_BASE_URL` | `src/server.ts:437` |", "| `AI_EMBED_MODEL` | `src/selfhost/ai.ts:872` |", - "| `AI_ON_MERGE` | `src/selfhost/ai.ts:984` |", + "| `AI_ON_MERGE` | `src/selfhost/ai.ts:1002` |", "| `AI_PROVIDER` | `src/selfhost/ai-config.ts:43` |", "| `ANTHROPIC_AI_BASE_URL` | `src/selfhost/ai.ts:876` |", "| `ANTHROPIC_AI_MODEL` | `src/selfhost/ai.ts:85` |", diff --git a/apps/gittensory-ui/src/routes/docs.self-hosting-ai-providers.tsx b/apps/gittensory-ui/src/routes/docs.self-hosting-ai-providers.tsx index d5bee3c0f3..f9af1045a8 100644 --- a/apps/gittensory-ui/src/routes/docs.self-hosting-ai-providers.tsx +++ b/apps/gittensory-ui/src/routes/docs.self-hosting-ai-providers.tsx @@ -81,12 +81,26 @@ OPENAI_COMPATIBLE_AI_MODEL=llama3.1`}

Fallback and dual review

- A comma-list can be a fallback chain or a two-reviewer plan. With two available providers, - AI_COMBINE controls how decisions are combined. + A comma-list is a fallback chain by default. Use this for subscription CLIs when you want + Codex first and Claude Code only when Codex is unavailable or out of tokens.

+

+ Set AI_DUAL_REVIEW=1 only when you want the first two providers to run as + independent reviewers on every PR. In dual-review mode, AI_COMBINE controls how + decisions are combined. +

+ @@ -95,7 +109,7 @@ AI_ON_MERGE=either`} { title: "single", description: - "One reviewer verdict. This is the automatic mode when one provider is configured.", + "One reviewer verdict. This is the automatic mode for one provider or a fallback chain.", }, { title: "consensus", diff --git a/apps/gittensory-ui/src/routes/docs.self-hosting-quickstart.tsx b/apps/gittensory-ui/src/routes/docs.self-hosting-quickstart.tsx index 96991ed242..8c72690829 100644 --- a/apps/gittensory-ui/src/routes/docs.self-hosting-quickstart.tsx +++ b/apps/gittensory-ui/src/routes/docs.self-hosting-quickstart.tsx @@ -64,28 +64,34 @@ function SelfHostingQuickstart() {

2. Choose your AI provider (optional)

- Skip this step for a fully deterministic review (no AI). Otherwise uncomment ONE of the - three blocks below in .env.selfhost.example — they're mutually exclusive, each - sets its own AI_PROVIDER. The self-host image bundles both CLIs by default; - credentials and provider choice are runtime-only. + Skip this step for a fully deterministic review (no AI). Otherwise set{" "} + AI_PROVIDER to one provider or a fallback chain. The self-host image bundles + both CLIs by default; credentials and provider choice are runtime-only.

+

+ Set AI_DUAL_REVIEW=1 only when you deliberately want the first two providers to + run as independent reviewers instead of a fallback chain. +

Codex stores its OAuth credential in auth.json on the same filesystem that prompt-influenced reviews can read, so it requires explicit opt-in ( diff --git a/apps/gittensory-ui/src/routes/docs.self-hosting-release-checklist.tsx b/apps/gittensory-ui/src/routes/docs.self-hosting-release-checklist.tsx index 83fb90a3ac..041e704f5f 100644 --- a/apps/gittensory-ui/src/routes/docs.self-hosting-release-checklist.tsx +++ b/apps/gittensory-ui/src/routes/docs.self-hosting-release-checklist.tsx @@ -121,9 +121,10 @@ SELFHOST_SMOKE_EXPECT_EVENTS="selfhost_ai_provider" \\ SELFHOST_SMOKE_FORBID_EVENTS="selfhost_ai_cli_missing" \\ ./scripts/smoke-selfhost.sh gittensory:rc-candidate -# Both, synthesized -SELFHOST_SMOKE_EXTRA_ENV="AI_PROVIDER=claude-code,codex -AI_COMBINE=synthesis +# Codex primary, Claude Code fallback +SELFHOST_SMOKE_EXTRA_ENV="AI_PROVIDER=codex,claude-code +CODEX_AI_EFFORT=medium +CLAUDE_AI_EFFORT=medium CLAUDE_CODE_OAUTH_TOKEN=\${TEST_CLAUDE_TOKEN} GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER=1" \\ SELFHOST_SMOKE_EXPECT_EVENTS="selfhost_ai_provider" \\ diff --git a/grafana/dashboards/gittensory.json b/grafana/dashboards/gittensory.json index f61569350e..e1a9dcba93 100644 --- a/grafana/dashboards/gittensory.json +++ b/grafana/dashboards/gittensory.json @@ -1260,7 +1260,7 @@ }, { "type": "timeseries", - "title": "AI requests by model + effort (last 1h)", + "title": "AI requests + fallbacks (last 1h)", "datasource": { "type": "prometheus", "uid": "${DS_PROMETHEUS}" @@ -1280,6 +1280,14 @@ }, "expr": "sum by (model, effort) (increase(gittensory_ai_requests_total[1h]))", "legendFormat": "{{model}} \u00b7 {{effort}}" + }, + { + "datasource": { + "type": "prometheus", + "uid": "${DS_PROMETHEUS}" + }, + "expr": "sum by (primary, fallback) (increase(gittensory_ai_review_model_fallback_total[1h]))", + "legendFormat": "fallback {{primary}}\u2192{{fallback}}" } ], "fieldConfig": { diff --git a/src/env.d.ts b/src/env.d.ts index 3917c05f95..a37cf36915 100644 --- a/src/env.d.ts +++ b/src/env.d.ts @@ -55,12 +55,15 @@ declare global { /** Optional Cloudflare AI Gateway id for legacy env.AI-compatible adapters. Self-host review execution should * prefer provider-specific AI_* configuration instead. */ AI_GATEWAY_ID?: string; - /** Self-host AI provider selection + dual-review config (#dual-ai-combiner). `AI_PROVIDER` is a comma list of - * providers (claude-code, codex, anthropic, ollama, …); `AI_COMBINE` picks single|consensus|synthesis (default - * synthesis for two); `AI_ON_MERGE` is the synthesis rule either|both. Provider-specific model/effort/timeout - * vars keep Claude/Codex/OpenAI/Ollama/Anthropic config explicit. `AI_REVIEW_PLAN` is the resolved plan - * (computed from these at boot in server.ts and read at the review call site); undefined on cloud. */ + /** Self-host AI provider selection + reviewer config (#dual-ai-combiner). `AI_PROVIDER` is a comma list of + * providers (claude-code, codex, anthropic, ollama, ...). By default, the first provider is the reviewer and + * the first distinct later provider is its fallback; `AI_DUAL_REVIEW=1` makes the first two providers run as + * independent reviewers. In dual mode, `AI_COMBINE` picks single|consensus|synthesis and `AI_ON_MERGE` is the + * synthesis rule either|both. Provider-specific model/effort/timeout vars keep Claude/Codex/OpenAI/Ollama/ + * Anthropic config explicit. `AI_REVIEW_PLAN` is the resolved plan (computed from these at boot in server.ts + * and read at the review call site); undefined on cloud. */ AI_PROVIDER?: string; + AI_DUAL_REVIEW?: string; AI_COMBINE?: string; AI_ON_MERGE?: string; CLAUDE_AI_MODEL?: string; @@ -80,7 +83,7 @@ declare global { ANTHROPIC_AI_BASE_URL?: string; ANTHROPIC_AI_MODEL?: string; AI_REVIEW_PLAN?: { - reviewers: Array<{ model: string }>; + reviewers: Array<{ model: string; fallback?: string | null | undefined }>; combine: import("./services/ai-review").CombineStrategy; onMerge?: import("./services/ai-review").OnMerge | undefined; }; diff --git a/src/review/ai-review-cache-input.ts b/src/review/ai-review-cache-input.ts index c4c61805ff..1a3da0516c 100644 --- a/src/review/ai-review-cache-input.ts +++ b/src/review/ai-review-cache-input.ts @@ -34,12 +34,12 @@ export type AiReviewCacheInput = { reviewerPlan: | { combine?: string | null | undefined; - reviewers?: readonly { model?: string | null | undefined }[] | undefined; + reviewers?: readonly { model?: string | null | undefined; fallback?: string | null | undefined }[] | undefined; } | null | undefined; - // reviewerPlan only names WHICH self-host provider(s) are active (e.g. "claude-code") -- it does not carry that - // provider's own model/effort/timeout/base-url, which are resolved separately at review-call time (see + // reviewerPlan only names WHICH self-host provider(s) are active (e.g. "codex" with fallback "claude-code") -- + // it does not carry that provider's own model/effort/timeout/base-url, which are resolved separately at review-call time (see // src/selfhost/ai.ts's buildProvider). Fingerprint those too so switching a provider's underlying model or // endpoint (while the provider name/plan stays the same) forces a cache miss instead of reusing a review // produced against a different configuration. Deliberately excludes API keys (secrets, and irrelevant to output). @@ -119,7 +119,10 @@ export async function aiReviewCacheInputFingerprint(input: AiReviewCacheInput): reviewerPlan: input.reviewerPlan ? { combine: input.reviewerPlan.combine ?? null, - reviewers: (input.reviewerPlan.reviewers ?? []).map((reviewer) => reviewer.model ?? null), + reviewers: (input.reviewerPlan.reviewers ?? []).map((reviewer) => ({ + model: reviewer.model ?? null, + fallback: reviewer.fallback ?? null, + })), } : null, selfHostProviderConfig: input.selfHostProviderConfig diff --git a/src/selfhost/ai-config.ts b/src/selfhost/ai-config.ts index 6281e82b9d..a6aef7a741 100644 --- a/src/selfhost/ai-config.ts +++ b/src/selfhost/ai-config.ts @@ -91,11 +91,17 @@ export function labelSelfHostReviewerModel( } export function labelSelfHostReviewerModels( - reviewers: ReadonlyArray<{ model: string }>, + reviewers: ReadonlyArray<{ model: string; fallback?: string | null | undefined }>, env: Record, ): string { return reviewers - .map((reviewer) => labelSelfHostReviewerModel(reviewer.model, env)) + .map((reviewer) => { + const primary = labelSelfHostReviewerModel(reviewer.model, env); + const fallback = reviewer.fallback?.trim() + ? labelSelfHostReviewerModel(reviewer.fallback, env) + : ""; + return fallback ? `${primary}->${fallback}` : primary; + }) .join("+"); } diff --git a/src/selfhost/ai.ts b/src/selfhost/ai.ts index 364ad42c25..81d47190fd 100644 --- a/src/selfhost/ai.ts +++ b/src/selfhost/ai.ts @@ -103,12 +103,12 @@ function defaultOpenAiCompatibleModel(name: string): string { const VALID_CLAUDE_EFFORTS = new Set(["low", "medium", "high", "xhigh", "max"]); const VALID_CODEX_EFFORTS = new Set(["low", "medium", "high", "xhigh"]); -/** Map `CLAUDE_AI_EFFORT` to a `claude --effort` level. Defaults to "high" — the engine wants a substantive - * review, not a fast shallow one — and falls back to "high" for any unset or unrecognized value so a typo can't - * silently downgrade reviews. The CLI clamps a level above the model's own ceiling (e.g. xhigh on Sonnet) down. */ +/** Map `CLAUDE_AI_EFFORT` to a `claude --effort` level. Defaults to "medium" so subscription fallback preserves + * enough reasoning depth for reviews without burning high-effort tokens on every PR. A typo falls back to medium + * instead of silently disabling the reviewer; operators can still raise important repos to high/xhigh/max. */ export function resolveEffort(configured: string | undefined): string { const level = (configured ?? "").trim().toLowerCase(); - return VALID_CLAUDE_EFFORTS.has(level) ? level : "high"; + return VALID_CLAUDE_EFFORTS.has(level) ? level : "medium"; } /** Map `CODEX_AI_EFFORT` to Codex reasoning effort. Codex currently supports xhigh as its top level, so a @@ -117,7 +117,7 @@ export function resolveCodexEffort(configured: string | undefined): string { const level = (configured ?? "").trim().toLowerCase(); if (VALID_CODEX_EFFORTS.has(level)) return level; if (level === "max") return "xhigh"; - return "high"; + return "medium"; } // Per-effort subprocess timeout (ms) for the subscription CLIs. A higher effort legitimately runs longer, so the @@ -943,8 +943,8 @@ export function resolveRequiredCliProviders(env: Record): SelfH const COMBINE_STRATEGIES = new Set(["single", "consensus", "synthesis"]); const ON_MERGE_RULES = new Set(["either", "both"]); +const TRUE_ENV_VALUES = new Set(["1", "true", "yes", "on"]); -/** Resolve the self-host dual-review plan from env: the credentialed providers become the reviewer(s), `AI_COMBINE` - * the strategy (default `synthesis` for two — "both review, one synthesized decision"), `AI_ON_MERGE` the - * synthesis rule. Returns undefined when no provider is configured (cloud, or AI off) so ai-review keeps its - * byte-identical Workers-AI consensus default; one provider ⇒ `single`; two+ ⇒ the configured strategy over the - * first two. The result is attached to the self-host env at boot and passed to runGittensoryAiReview. */ +function enabledEnvFlag(value: string | undefined): boolean { + return TRUE_ENV_VALUES.has((value ?? "").trim().toLowerCase()); +} + +/** Resolve the self-host review plan from env. By default, `AI_PROVIDER=a,b` means one reviewer using `a` with + * `b` as the per-review fallback, so a Codex quota/auth outage can fall through to Claude Code without paying + * for two simultaneous reviewers. `AI_DUAL_REVIEW=1` opts back into the explicit two-reviewer mode where the + * first two providers run independently and `AI_COMBINE` / `AI_ON_MERGE` decide how to merge them. */ export function resolveAiReviewerPlan( env: Record, -): { reviewers: Array<{ model: string }>; combine: CombineStrategy; onMerge: OnMerge | undefined } | undefined { +): { reviewers: Array<{ model: string; fallback?: string | null | undefined }>; combine: CombineStrategy; onMerge: OnMerge | undefined } | undefined { const names = resolveProviderNames(env); if (names.length === 0) return undefined; + if (!enabledEnvFlag(env.AI_DUAL_REVIEW)) { + const primary = names[0] as string; + const fallback = names.find((name) => name !== primary); + return { + reviewers: [ + { + model: primary, + ...(fallback ? { fallback } : {}), + }, + ], + combine: "single", + onMerge: undefined, + }; + } if (names.length === 1) return { reviewers: [{ model: names[0] as string }], combine: "single", onMerge: undefined }; // Fail loud when the two SLOTS the dual-review plan actually uses (the first two names) are the same // provider: routeProviders' `byName` map collapses duplicate provider names to one runtime instance, so diff --git a/src/selfhost/metrics.ts b/src/selfhost/metrics.ts index 69ccae5ed0..858519966f 100644 --- a/src/selfhost/metrics.ts +++ b/src/selfhost/metrics.ts @@ -111,6 +111,7 @@ const DEFAULT_METRIC_META: readonly (readonly [string, MetricMeta])[] = [ ["gittensory_ai_review_force_bypass_total", { help: "AI review cache force-bypass events.", type: "counter" }], ["gittensory_ai_review_inconclusive_total", { help: "AI review inconclusive outcomes.", type: "counter" }], ["gittensory_ai_review_onmerge_clamped_total", { help: "AI review on-merge mode clamp events.", type: "counter" }], + ["gittensory_ai_review_model_fallback_total", { help: "AI review model fallback attempts by primary and fallback model.", type: "counter" }], ["gittensory_regate_ai_skipped_current_total", { help: "Regate requests skipped because AI state is current.", type: "counter" }], ["gittensory_public_surface_publish_skipped_current_total", { help: "Public surface publishes skipped because state is current.", type: "counter" }], ["gittensory_gate_decisions_total", { help: "Gate decisions by conclusion.", type: "counter" }], diff --git a/src/services/ai-review.ts b/src/services/ai-review.ts index 9e1d1a9c8c..5d64751ac0 100644 --- a/src/services/ai-review.ts +++ b/src/services/ai-review.ts @@ -185,9 +185,9 @@ export type GittensoryAiReviewInput = { onMerge?: OnMerge | null | undefined; /** * The reviewer(s) to run (#dual-ai-combiner). Absent/empty ⇒ the free Workers-AI pair with per-slot fallbacks - * (byte-identical to today). A self-host plan supplies named providers instead — `{ model: "claude-code" }`, - * `{ model: "codex" }` — addressed by the self-host AI router; `fallback` is Workers-AI-only (a self-host - * provider has none). `single` (or a single entry) runs reviewer[0]; consensus/synthesis run [0] and [1]. + * (byte-identical to today). A self-host plan supplies named providers instead — `{ model: "codex", + * fallback: "claude-code" }` — addressed by the self-host AI router. `single` (or a single entry) runs + * reviewer[0]; consensus/synthesis run [0] and [1]. */ reviewers?: | ReadonlyArray<{ model: string; fallback?: string | null | undefined }> @@ -694,9 +694,11 @@ async function runWorkersOpinion( let lastUnparseable: | { model: string; attempt: number; responseChars: number; hasJsonObject: boolean } | undefined; - for (const model of fallback && fallback !== primary - ? [primary, fallback] - : [primary]) { + const models = fallback && fallback !== primary ? [primary, fallback] : [primary]; + for (const [modelIndex, model] of models.entries()) { + if (modelIndex > 0) { + incr("gittensory_ai_review_model_fallback_total", { primary, fallback: model }); + } for (let attempt = 0; attempt < 3; attempt += 1) { try { const cliSystemAppend = selfHostCliSystemAppend(model, systemAppend); diff --git a/test/unit/ai-review-advisory.test.ts b/test/unit/ai-review-advisory.test.ts index bde0996690..8f8ed7fe12 100644 --- a/test/unit/ai-review-advisory.test.ts +++ b/test/unit/ai-review-advisory.test.ts @@ -189,7 +189,9 @@ describe("runAiReviewForAdvisory", () => { const env = aiEnv(async () => { throw new Error("ollama unavailable"); }); Object.assign(env as unknown as Record, { AI_PROVIDER: "anthropic,ollama", - AI_REVIEW_PLAN: { reviewers: [{ model: "ollama" }], combine: "single" }, + ANTHROPIC_AI_MODEL: "claude-sonnet-4-6", + OLLAMA_AI_MODEL: "llama3.1", + AI_REVIEW_PLAN: { reviewers: [{ model: "anthropic", fallback: "ollama" }], combine: "single" }, }); const result = await runAiReviewForAdvisory(env, { settings: { aiReviewMode: "advisory" } as RepositorySettings, advisory: advisory(), repoFullName: "acme/widgets", pr, author: "alice", confirmedContributor: true }); expect(result).toMatchObject({ @@ -197,7 +199,7 @@ describe("runAiReviewForAdvisory", () => { findings: [expect.objectContaining({ code: "ai_review_inconclusive" })], }); 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("ollama"); + expect(usage?.model).toBe("anthropic:claude-sonnet-4-6->ollama:llama3.1"); }); it("no-ops for a non-confirmed contributor under the gittensor pack and when there is no head SHA", async () => { diff --git a/test/unit/ai-review-cache-input.test.ts b/test/unit/ai-review-cache-input.test.ts index 94488616e6..35c55a434c 100644 --- a/test/unit/ai-review-cache-input.test.ts +++ b/test/unit/ai-review-cache-input.test.ts @@ -74,6 +74,29 @@ describe("aiReviewCacheInputFingerprint", () => { expect(updated).not.toBe(original); }); + it("changes when the self-host reviewer fallback changes", async () => { + const original = await aiReviewCacheInputFingerprint({ + ...baseInput(), + reviewerPlan: { combine: "single", reviewers: [{ model: "codex", fallback: "claude-code" }] }, + }); + const fallbackChanged = await aiReviewCacheInputFingerprint({ + ...baseInput(), + reviewerPlan: { combine: "single", reviewers: [{ model: "codex", fallback: "anthropic" }] }, + }); + const omittedFallback = await aiReviewCacheInputFingerprint({ + ...baseInput(), + reviewerPlan: { combine: "single", reviewers: [{ model: "codex" }] }, + }); + const repeated = await aiReviewCacheInputFingerprint({ + ...baseInput(), + reviewerPlan: { combine: "single", reviewers: [{ model: "codex", fallback: "claude-code" }] }, + }); + + expect(fallbackChanged).not.toBe(original); + expect(omittedFallback).not.toBe(original); + expect(repeated).toBe(original); + }); + it("normalizes sparse reviewer plan fields deterministically", async () => { const omittedReviewers = await aiReviewCacheInputFingerprint({ ...baseInput(), @@ -89,7 +112,7 @@ describe("aiReviewCacheInputFingerprint", () => { }); const explicit = await aiReviewCacheInputFingerprint({ ...baseInput(), - reviewerPlan: { combine: null, reviewers: [{ model: null }] }, + reviewerPlan: { combine: null, reviewers: [{ model: null, fallback: null }] }, }); expect(omittedReviewers).toBe(explicitEmpty); diff --git a/test/unit/ai-review.test.ts b/test/unit/ai-review.test.ts index 628ac3d476..0a0261ea5a 100644 --- a/test/unit/ai-review.test.ts +++ b/test/unit/ai-review.test.ts @@ -852,7 +852,7 @@ describe("Workers AI fallback + degraded output", () => { describe("runGittensoryAiReview self-host dual-AI plan (#dual-ai-combiner)", () => { const planEnv = ( plan: { - reviewers: Array<{ model: string }>; + reviewers: Array<{ model: string; fallback?: string | null | undefined }>; combine: string; onMerge?: string; }, @@ -890,6 +890,33 @@ describe("runGittensoryAiReview self-host dual-AI plan (#dual-ai-combiner)", () expect(seen).toEqual(["claude-code"]); // exactly one reviewer, addressed by name }); + it("single provider fallback: tries Claude Code when Codex fails and records the fallback attempt", async () => { + const seen: string[] = []; + const env = planEnv( + { reviewers: [{ model: "codex", fallback: "claude-code" }], combine: "single" }, + async (model) => { + seen.push(model); + if (model === "codex") throw new Error("codex quota exhausted"); + return { + response: reviewJson({ + present: true, + title: "Race condition in src/x.ts", + }), + }; + }, + ); + const result = await runGittensoryAiReview(env, { + ...baseInput, + mode: "block", + }); + if (result.status !== "ok") throw new Error("expected ok"); + expect(result.consensusDefect?.title).toContain("Race condition"); + expect(seen).toEqual(["codex", "codex", "codex", "claude-code"]); + expect(await renderMetrics()).toContain( + 'gittensory_ai_review_model_fallback_total{fallback="claude-code",primary="codex"} 1', + ); + }); + it("dual synthesis (either): runs claude-code AND codex; EITHER blocker decides, never a split", async () => { const seen: string[] = []; const env = planEnv( diff --git a/test/unit/selfhost-ai.test.ts b/test/unit/selfhost-ai.test.ts index e9ecf95b6a..2ac130ae0c 100644 --- a/test/unit/selfhost-ai.test.ts +++ b/test/unit/selfhost-ai.test.ts @@ -3,7 +3,7 @@ import { tmpdir } from "node:os"; import { delimiter, join } from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; import { assertNoLegacySharedAiEnv, buildProvider, claudeErrorStatus, codexErrorFromStdout, createAnthropicAi, createChainAi, createClaudeCodeAi, createCodexAi, createOpenAiCompatibleAi, createSelfHostAi, extractCliText, extractCliUsage, isAiProviderHealthy, markAiProviderUnhealthyAtBoot, resetAiProviderCircuitBreakerForTest, resetAiProviderHealthForTest, resolveAiReviewerPlan, resolveClaudeCliTimeoutMs, resolveCodexAuthPath, resolveCodexCliTimeoutMs, resolveCodexEffort, resolveEffort, resolveModel, resolveProviderNames, resolveRequiredCliProviders, resolveSubscriptionCliPath, redactSecrets, routeProviders, shouldMarkAiProviderUnhealthyAtBoot, subscriptionCliEnv } from "../../src/selfhost/ai"; -import { labelSelfHostReviewerModel } from "../../src/selfhost/ai-config"; +import { labelSelfHostReviewerModel, labelSelfHostReviewerModels } from "../../src/selfhost/ai-config"; import { renderMetrics, resetMetrics } from "../../src/selfhost/metrics"; describe("resolveModel (#979 — never leak the Workers-AI default to a self-host backend)", () => { @@ -19,16 +19,16 @@ describe("resolveModel (#979 — never leak the Workers-AI default to a self-hos }); }); -describe("resolveEffort (#selfhost-effort — Claude Code intelligence dial, default high)", () => { +describe("resolveEffort (#selfhost-effort — Claude Code intelligence dial, default medium)", () => { it("passes a valid level through, trimmed + lowercased", () => { expect(resolveEffort("low")).toBe("low"); expect(resolveEffort(" Medium ")).toBe("medium"); expect(resolveEffort("MAX")).toBe("max"); }); - it("defaults to high when unset or unrecognized so a typo can't downgrade reviews", () => { - expect(resolveEffort(undefined)).toBe("high"); // ?? right side - expect(resolveEffort("")).toBe("high"); // present but not in the valid set - expect(resolveEffort("ultra")).toBe("high"); // unrecognized → safe default + it("defaults to medium when unset or unrecognized to conserve fallback tokens", () => { + expect(resolveEffort(undefined)).toBe("medium"); // ?? right side + expect(resolveEffort("")).toBe("medium"); // present but not in the valid set + expect(resolveEffort("ultra")).toBe("medium"); // unrecognized → conservative default }); }); @@ -38,7 +38,7 @@ describe("resolveCodexEffort (#selfhost-effort — Codex reasoning effort, expli expect(resolveCodexEffort(" Medium ")).toBe("medium"); expect(resolveCodexEffort("xhigh")).toBe("xhigh"); expect(resolveCodexEffort("max")).toBe("xhigh"); - expect(resolveCodexEffort("ultra")).toBe("high"); + expect(resolveCodexEffort("ultra")).toBe("medium"); }); }); @@ -49,7 +49,7 @@ describe("provider-specific CLI timeouts (#selfhost — no shared timeout ambigu expect(resolveClaudeCliTimeoutMs({ CLAUDE_AI_EFFORT: "high" })).toBe(240_000); expect(resolveClaudeCliTimeoutMs({ CLAUDE_AI_EFFORT: "xhigh" })).toBe(360_000); expect(resolveClaudeCliTimeoutMs({ CLAUDE_AI_EFFORT: "max" })).toBe(600_000); - expect(resolveClaudeCliTimeoutMs({})).toBe(240_000); + expect(resolveClaudeCliTimeoutMs({})).toBe(120_000); expect(resolveClaudeCliTimeoutMs({ CLAUDE_AI_TIMEOUT_MS: "300000", CLAUDE_AI_EFFORT: "low" })).toBe(300_000); }); it("scales Codex timeout from CODEX_AI_EFFORT and honors CODEX_AI_TIMEOUT_MS", () => { @@ -58,6 +58,7 @@ describe("provider-specific CLI timeouts (#selfhost — no shared timeout ambigu expect(resolveCodexCliTimeoutMs({ CODEX_AI_EFFORT: "high" })).toBe(240_000); expect(resolveCodexCliTimeoutMs({ CODEX_AI_EFFORT: "xhigh" })).toBe(360_000); expect(resolveCodexCliTimeoutMs({ CODEX_AI_EFFORT: "max" })).toBe(360_000); + expect(resolveCodexCliTimeoutMs({})).toBe(120_000); expect(resolveCodexCliTimeoutMs({ CODEX_AI_TIMEOUT_MS: "1000" })).toBe(30_000); expect(resolveCodexCliTimeoutMs({ CODEX_AI_TIMEOUT_MS: "9999999" })).toBe(1_800_000); }); @@ -555,16 +556,38 @@ describe("resolveProviderNames + resolveAiReviewerPlan (#dual-ai-combiner)", () ]); }); - it("resolveAiReviewerPlan: undefined with no provider; single ⇒ single; two ⇒ default synthesis", () => { + it("resolveAiReviewerPlan: undefined with no provider; single provider stays single", () => { expect(resolveAiReviewerPlan({})).toBeUndefined(); // cloud / AI off expect(resolveAiReviewerPlan({ AI_PROVIDER: "claude-code" })).toEqual({ reviewers: [{ model: "claude-code" }], combine: "single", onMerge: undefined }); - expect(resolveAiReviewerPlan({ AI_PROVIDER: "claude-code,codex" })).toEqual({ reviewers: [{ model: "claude-code" }, { model: "codex" }], combine: "synthesis", onMerge: undefined }); }); - it("resolveAiReviewerPlan: honors AI_COMBINE / AI_ON_MERGE, defaults invalid values, caps at two reviewers", () => { - expect(resolveAiReviewerPlan({ AI_PROVIDER: "claude-code,codex", AI_COMBINE: "consensus", AI_ON_MERGE: "both" })).toMatchObject({ combine: "consensus", onMerge: "both" }); - expect(resolveAiReviewerPlan({ AI_PROVIDER: "claude-code,codex", AI_COMBINE: "garbage", AI_ON_MERGE: "nonsense" })).toMatchObject({ combine: "synthesis", onMerge: undefined }); // invalid → defaults - expect(resolveAiReviewerPlan({ AI_PROVIDER: "claude-code,codex,ollama" })?.reviewers).toEqual([{ model: "claude-code" }, { model: "codex" }]); // first two + it("resolveAiReviewerPlan: comma-list is a single-reviewer fallback chain by default", () => { + expect(resolveAiReviewerPlan({ AI_PROVIDER: "codex,claude-code" })).toEqual({ + reviewers: [{ model: "codex", fallback: "claude-code" }], + combine: "single", + onMerge: undefined, + }); + expect(resolveAiReviewerPlan({ AI_PROVIDER: "codex,codex,claude-code" })).toEqual({ + reviewers: [{ model: "codex", fallback: "claude-code" }], + combine: "single", + onMerge: undefined, + }); + expect(resolveAiReviewerPlan({ AI_PROVIDER: "codex,claude-code,ollama", AI_COMBINE: "consensus", AI_ON_MERGE: "both" })).toEqual({ + reviewers: [{ model: "codex", fallback: "claude-code" }], + combine: "single", + onMerge: undefined, + }); + }); + + it("resolveAiReviewerPlan: AI_DUAL_REVIEW=1 restores two independent reviewers and combine controls", () => { + expect(resolveAiReviewerPlan({ AI_PROVIDER: "claude-code,codex", AI_DUAL_REVIEW: "1" })).toEqual({ + reviewers: [{ model: "claude-code" }, { model: "codex" }], + combine: "synthesis", + onMerge: undefined, + }); + expect(resolveAiReviewerPlan({ AI_PROVIDER: "claude-code,codex", AI_DUAL_REVIEW: "true", AI_COMBINE: "consensus", AI_ON_MERGE: "both" })).toMatchObject({ combine: "consensus", onMerge: "both" }); + expect(resolveAiReviewerPlan({ AI_PROVIDER: "claude-code,codex", AI_DUAL_REVIEW: "yes", AI_COMBINE: "garbage", AI_ON_MERGE: "nonsense" })).toMatchObject({ combine: "synthesis", onMerge: undefined }); // invalid → defaults + expect(resolveAiReviewerPlan({ AI_PROVIDER: "claude-code,codex,ollama", AI_DUAL_REVIEW: "on" })?.reviewers).toEqual([{ model: "claude-code" }, { model: "codex" }]); // first two }); it("resolveAiReviewerPlan: throws when the two dual-review slots resolve to the SAME provider (#2540)", () => { @@ -572,14 +595,14 @@ describe("resolveProviderNames + resolveAiReviewerPlan (#dual-ai-combiner)", () // collapses duplicate names to one runtime instance, so this would silently degrade "dual review" into // "one provider called twice" with no independent second opinion. Fail loud at plan-resolution time // instead of degrading silently. - expect(() => resolveAiReviewerPlan({ AI_PROVIDER: "codex,codex" })).toThrow(/ai_reviewer_providers_not_distinct/); - expect(() => resolveAiReviewerPlan({ AI_PROVIDER: "codex,codex" })).toThrow(/"codex"/); + expect(() => resolveAiReviewerPlan({ AI_PROVIDER: "codex,codex", AI_DUAL_REVIEW: "1" })).toThrow(/ai_reviewer_providers_not_distinct/); + expect(() => resolveAiReviewerPlan({ AI_PROVIDER: "codex,codex", AI_DUAL_REVIEW: "1" })).toThrow(/"codex"/); }); it("resolveAiReviewerPlan: a THIRD-slot duplicate does not throw (only the first two slots are actually used)", () => { // "codex,ollama,codex" — the first two names (codex, ollama) are distinct, so the plan resolves normally; // the trailing repeat of codex is never addressed because reviewers are capped at the first two. - expect(resolveAiReviewerPlan({ AI_PROVIDER: "codex,ollama,codex" })).toMatchObject({ + expect(resolveAiReviewerPlan({ AI_PROVIDER: "codex,ollama,codex", AI_DUAL_REVIEW: "1" })).toMatchObject({ reviewers: [{ model: "codex" }, { model: "ollama" }], combine: "synthesis", }); @@ -588,6 +611,10 @@ describe("resolveProviderNames + resolveAiReviewerPlan (#dual-ai-combiner)", () it("labels explicit provider:model reviewer ids without consulting env defaults", () => { expect(labelSelfHostReviewerModel(" CODEX:gpt-5.5 ", { CODEX_AI_MODEL: "ignored" })).toBe("codex:gpt-5.5"); }); + + it("labels primary→fallback reviewer chains with provider-specific configured models", () => { + expect(labelSelfHostReviewerModels([{ model: "codex", fallback: "claude-code" }], { CODEX_AI_MODEL: "gpt-5.5", CLAUDE_AI_MODEL: "claude-sonnet-4-6" })).toBe("codex:gpt-5.5->claude-code:claude-sonnet-4-6"); + }); }); describe("branch coverage — defaults + edge inputs", () => { @@ -747,7 +774,7 @@ describe("subscription CLI helpers + fail-safe", () => { expect(capturedEnv.CLAUDE_CODE_OAUTH_TOKEN).toBe("t"); }); - it("Claude Code pins the default model (claude-sonnet-4-6) + --effort high; CLAUDE_AI_* overrides explicitly", async () => { + it("Claude Code pins the default model (claude-sonnet-4-6) + --effort medium; CLAUDE_AI_* overrides explicitly", async () => { let seen: string[] = []; let timeout = 0; const cap: StubSpawn = async (_c, a, o) => { @@ -755,12 +782,12 @@ describe("subscription CLI helpers + fail-safe", () => { timeout = o.timeoutMs; return { stdout: JSON.stringify({ type: "result", result: "ok" }), code: 0 }; }; - // Empty model id (the dual-router default) + no CLAUDE_AI_MODEL → pinned claude-sonnet-4-6; unset effort → high. + // Empty model id (the router default) + no CLAUDE_AI_MODEL → pinned claude-sonnet-4-6; unset effort → medium. await createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t" }, cap).run("", { prompt: "x" }); expect(seen[seen.indexOf("--model") + 1]).toBe("claude-sonnet-4-6"); - expect(seen[seen.indexOf("--effort") + 1]).toBe("high"); + expect(seen[seen.indexOf("--effort") + 1]).toBe("medium"); expect(seen).not.toContain("--append-system-prompt"); - expect(timeout).toBe(240_000); // high → 240s (not the old fixed 120s) + expect(timeout).toBe(120_000); // medium → 120s by default to conserve fallback subscription tokens // Provider-specific overrides flow through to the argv + timeout scale. await createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t", CLAUDE_AI_MODEL: "claude-opus-4-8", CLAUDE_AI_EFFORT: "max" }, cap).run("", { prompt: "x" }); expect(seen[seen.indexOf("--model") + 1]).toBe("claude-opus-4-8"); @@ -832,7 +859,7 @@ describe("subscription CLI helpers + fail-safe", () => { prompt: "x", })).response, ).toBe("codex review"); - expect(seen).toEqual(["exec", "--json", "--skip-git-repo-check", "--sandbox", "read-only", "-c", 'model_reasoning_effort="high"']); + expect(seen).toEqual(["exec", "--json", "--skip-git-repo-check", "--sandbox", "read-only", "-c", 'model_reasoning_effort="medium"']); expect(seen).not.toContain("--ask-for-approval"); expect(seen).not.toContain("x"); expect(capturedInput).toBe("x"); @@ -915,7 +942,7 @@ describe("subscription CLI helpers + fail-safe", () => { const empty: StubSpawn = async () => ({ stdout: "", code: 0 }); await expect(createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t" }, empty).run("m", { prompt: "x" })).rejects.toThrow(/claude_code_empty_output/); const metrics = await renderMetrics(); - expect(metrics).toContain('gittensory_ai_requests_total{effort="high",model="m",provider="claude-code"} 2'); + expect(metrics).toContain('gittensory_ai_requests_total{effort="medium",model="m",provider="claude-code"} 2'); }); it("Codex throws on empty output", async () => { @@ -924,7 +951,7 @@ describe("subscription CLI helpers + fail-safe", () => { createCodexAi({ GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER: "1" }, empty, noAuthCheck).run("gpt-5", { prompt: "x" }), ).rejects.toThrow(/codex_empty_output/); const metrics = await renderMetrics(); - expect(metrics).toContain('gittensory_ai_requests_total{effort="high",model="gpt-5",provider="codex"} 1'); + expect(metrics).toContain('gittensory_ai_requests_total{effort="medium",model="gpt-5",provider="codex"} 1'); }); it("Claude Code throws subscription_cli_timeout when the CLI is killed for exceeding its deadline", async () => { @@ -1080,7 +1107,7 @@ describe("subscription CLI helpers + fail-safe", () => { /codex_exit_1: stream error: rate limit reached/, ); const metrics = await renderMetrics(); - expect(metrics).toContain('gittensory_ai_requests_total{effort="high",model="m",provider="codex"} 1'); + expect(metrics).toContain('gittensory_ai_requests_total{effort="medium",model="m",provider="codex"} 1'); }); it("redacts the OAuth token and key-shaped tokens from claude stderr before they reach the error (#1605 sec)", async () => { diff --git a/test/unit/selfhost-grafana-dashboard.test.ts b/test/unit/selfhost-grafana-dashboard.test.ts index e7c7415ea1..2fde90d002 100644 --- a/test/unit/selfhost-grafana-dashboard.test.ts +++ b/test/unit/selfhost-grafana-dashboard.test.ts @@ -89,6 +89,8 @@ describe("Gittensory Self-Host Grafana dashboard", () => { expect(targets.some((target) => target.expr === "sum by (kind, key_scope, job_type) (rate(gittensory_jobs_rate_limit_admission_deferred_total[5m])) or vector(0)")).toBe(true); expect(targets.some((target) => target.expr === "sum by (kind, key_scope, job_type) (rate(gittensory_jobs_rate_limit_budget_deferred_total[5m])) or vector(0)")).toBe(true); expect(targets.some((target) => target.expr === "sum by (kind, key_scope, job_type) (rate(gittensory_jobs_rate_limited_by_type_total[5m])) or vector(0)")).toBe(true); + expect(targets.some((target) => target.expr === "sum by (primary, fallback) (increase(gittensory_ai_review_model_fallback_total[1h]))")).toBe(true); + expect(targets.some((target) => target.legendFormat === "fallback {{primary}}→{{fallback}}")).toBe(true); }); it("keeps Orb dashboard panels zero-safe when telemetry counters are absent", () => {