From d1879a76bb65f3d2143b7e3bd458cbee18af210c Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Sun, 5 Jul 2026 13:05:16 -0700 Subject: [PATCH] fix(selfhost): preserve legacy dual reviewer configs --- .../src/lib/selfhost-env-reference.ts | 12 ++++++------ src/selfhost/ai.ts | 10 ++++++---- test/unit/selfhost-ai.test.ts | 15 ++++++++++++++- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/apps/gittensory-ui/src/lib/selfhost-env-reference.ts b/apps/gittensory-ui/src/lib/selfhost-env-reference.ts index 843892c5d2..422ff91115 100644 --- a/apps/gittensory-ui/src/lib/selfhost-env-reference.ts +++ b/apps/gittensory-ui/src/lib/selfhost-env-reference.ts @@ -7,11 +7,11 @@ export type SelfHostEnvReferenceRow = { export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ { name: "AI_COMBINE", - firstReference: "src/selfhost/ai.ts:1173", + firstReference: "src/selfhost/ai.ts:1149", }, { name: "AI_DUAL_REVIEW", - firstReference: "src/selfhost/ai.ts:1148", + firstReference: "src/selfhost/ai.ts:1151", }, { name: "AI_EMBED_API_KEY", @@ -27,7 +27,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "AI_ON_MERGE", - firstReference: "src/selfhost/ai.ts:1175", + firstReference: "src/selfhost/ai.ts:1149", }, { name: "AI_PROVIDER", @@ -394,12 +394,12 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [ "| Name | First reference |", "| --- | --- |", - "| `AI_COMBINE` | `src/selfhost/ai.ts:1173` |", - "| `AI_DUAL_REVIEW` | `src/selfhost/ai.ts:1148` |", + "| `AI_COMBINE` | `src/selfhost/ai.ts:1149` |", + "| `AI_DUAL_REVIEW` | `src/selfhost/ai.ts:1151` |", "| `AI_EMBED_API_KEY` | `src/server.ts:443` |", "| `AI_EMBED_BASE_URL` | `src/server.ts:440` |", "| `AI_EMBED_MODEL` | `src/selfhost/ai.ts:1045` |", - "| `AI_ON_MERGE` | `src/selfhost/ai.ts:1175` |", + "| `AI_ON_MERGE` | `src/selfhost/ai.ts:1149` |", "| `AI_PROVIDER` | `src/selfhost/ai-config.ts:43` |", "| `ANTHROPIC_AI_BASE_URL` | `src/selfhost/ai.ts:1049` |", "| `ANTHROPIC_AI_MODEL` | `src/selfhost/ai.ts:109` |", diff --git a/src/selfhost/ai.ts b/src/selfhost/ai.ts index 25e824d71f..56a774dc3c 100644 --- a/src/selfhost/ai.ts +++ b/src/selfhost/ai.ts @@ -1138,14 +1138,17 @@ function enabledEnvFlag(value: string | undefined): boolean { /** 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. */ + * for two simultaneous reviewers. Existing multi-provider configs that explicitly set `AI_COMBINE` or + * `AI_ON_MERGE` keep their two-reviewer behavior; `AI_DUAL_REVIEW=1` is the explicit opt-in for new dual + * review configs. */ export function resolveAiReviewerPlan( env: Record, ): { 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 hasLegacyDualReviewConfig = (env.AI_COMBINE ?? "").trim() !== "" || (env.AI_ON_MERGE ?? "").trim() !== ""; + if (names.length === 1) return { reviewers: [{ model: names[0] as string }], combine: "single", onMerge: undefined }; + if (!enabledEnvFlag(env.AI_DUAL_REVIEW) && !hasLegacyDualReviewConfig) { const primary = names[0] as string; const fallback = names.find((name) => name !== primary); return { @@ -1159,7 +1162,6 @@ export function resolveAiReviewerPlan( 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 // "dual review" would silently become "one provider called twice" -- no independent second opinion, and diff --git a/test/unit/selfhost-ai.test.ts b/test/unit/selfhost-ai.test.ts index bf69264803..7cacc20550 100644 --- a/test/unit/selfhost-ai.test.ts +++ b/test/unit/selfhost-ai.test.ts @@ -675,13 +675,26 @@ describe("resolveProviderNames + resolveAiReviewerPlan (#dual-ai-combiner)", () combine: "single", onMerge: undefined, }); - expect(resolveAiReviewerPlan({ AI_PROVIDER: "codex,claude-code,ollama", AI_COMBINE: "consensus", AI_ON_MERGE: "both" })).toEqual({ + expect(resolveAiReviewerPlan({ AI_PROVIDER: "codex,claude-code,ollama" })).toEqual({ reviewers: [{ model: "codex", fallback: "claude-code" }], combine: "single", onMerge: undefined, }); }); + it("resolveAiReviewerPlan: legacy multi-provider combine/on-merge configs remain dual-review", () => { + expect(resolveAiReviewerPlan({ AI_PROVIDER: "codex,claude-code,ollama", AI_COMBINE: "consensus", AI_ON_MERGE: "both" })).toEqual({ + reviewers: [{ model: "codex" }, { model: "claude-code" }], + combine: "consensus", + onMerge: "both", + }); + expect(resolveAiReviewerPlan({ AI_PROVIDER: "codex,claude-code", AI_ON_MERGE: "either" })).toMatchObject({ + reviewers: [{ model: "codex" }, { model: "claude-code" }], + combine: "synthesis", + onMerge: "either", + }); + }); + 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" }],