diff --git a/scripts/branding-drift-baseline.json b/scripts/branding-drift-baseline.json index ea0f5f2284..14dabb7fcb 100644 --- a/scripts/branding-drift-baseline.json +++ b/scripts/branding-drift-baseline.json @@ -25,7 +25,7 @@ "src/review/repo-doc-render.ts": 2, "src/review/repo-skill-render.ts": 2, "src/review/selftune-wire.ts": 1, - "src/selfhost/ai.ts": 6, + "src/selfhost/ai.ts": 9, "src/selfhost/health.ts": 3, "src/selfhost/monitored-work.ts": 1, "src/selfhost/orb-collector.ts": 1, diff --git a/src/selfhost/ai.ts b/src/selfhost/ai.ts index 4549c38d84..1b971a8bda 100644 --- a/src/selfhost/ai.ts +++ b/src/selfhost/ai.ts @@ -479,6 +479,17 @@ function assertCodexCredentialIsolation(env: Record) // Fail closed until Codex exposes a brokered credential mode that does not put auth.json in the review sandbox. // Strict "1"-only, matching health.ts's codexAuthReadinessProbe and this flag's narrow opt-in convention. if (env.CODEX_HOME || env.LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER !== "1") { + // #7466: GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER was the only name for this opt-in until the LOOPOVER_ + // rebrand (#5652) retired dual-read support repo-wide. An operator whose .env still uses the old name + // silently reverts to fully-disabled with no distinguishing signal -- surface that specific, actionable + // case instead of the generic message so an upgrading self-hoster knows to rename the var rather than + // assuming Codex was never configured at all. CODEX_HOME being the actual cause takes priority: renaming + // the flag can't fix that one. + if (!env.CODEX_HOME && env.GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER === "1") { + throw new Error( + "codex_credential_isolation_required: GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER is set but no longer read -- rename it to LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER", + ); + } throw new Error("codex_credential_isolation_required"); } } diff --git a/src/services/ai-review.ts b/src/services/ai-review.ts index 093a7335d1..e1d054c538 100644 --- a/src/services/ai-review.ts +++ b/src/services/ai-review.ts @@ -1022,15 +1022,21 @@ export function isRateLimitError(error: unknown): boolean { } /** True for a provider's own STRUCTURAL misconfiguration signal (`src/selfhost/ai.ts`'s - * `codex_auth_not_configured` / `codex_no_auth` — a missing or expired credential file). Unlike a transient + * `codex_auth_not_configured` / `codex_no_auth` / `codex_credential_isolation_required` — a missing or + * expired credential file, or the credential-isolation guard refusing to run at all). Unlike a transient * timeout or rate limit, this will fail identically on every future attempt until an operator re-runs - * `codex auth` -- confirmed live (GITTENSORY-K/8: 2094 + 544 events over 16 days from one unfixed - * misconfiguration, the credential file was never present the whole time). Mirrors + * `codex auth` (or fixes their env var) -- confirmed live (GITTENSORY-K/8: 2094 + 544 events over 16 days + * from one unfixed misconfiguration, the credential file was never present the whole time; #7466: + * `codex_credential_isolation_required` repeating on the default 60s cooldown instead of the hour-long + * structural one, because this regex didn't recognize it yet). Mirrors * {@link isSubscriptionCliTimeout}/{@link isRateLimitError}'s identical non-transient-error short-circuit. * Exported so `src/selfhost/ai.ts`'s circuit breaker can give this failure class a much longer cooldown * than a genuinely transient one. */ export function isStructuralProviderConfigError(error: unknown): boolean { - return error instanceof Error && /^codex_(?:auth_not_configured|no_auth):/.test(error.message); + return ( + error instanceof Error && + /^codex_(?:auth_not_configured|no_auth|credential_isolation_required)(?::|$)/.test(error.message) + ); } /** Cap on the diagnostic prefix logged for an unparseable model response (#observability-unparseable) -- long diff --git a/test/unit/ai-review.test.ts b/test/unit/ai-review.test.ts index 6531f0c16b..5589fc561b 100644 --- a/test/unit/ai-review.test.ts +++ b/test/unit/ai-review.test.ts @@ -3084,6 +3084,21 @@ describe("pure helpers", () => { expect(isStructuralProviderConfigError(undefined)).toBe(false); }); + it("isStructuralProviderConfigError matches codex_credential_isolation_required, bare or with a detail suffix (#7466)", () => { + // The generic throw (assertCodexCredentialIsolation's no-CODEX_HOME, non-legacy-var case): no colon, no suffix. + expect(isStructuralProviderConfigError(new Error("codex_credential_isolation_required"))).toBe(true); + // The specific legacy-env-var rename hint: same prefix, colon-separated detail. + expect( + isStructuralProviderConfigError( + new Error( + "codex_credential_isolation_required: GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER is set but no longer read -- rename it to LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER", + ), + ), + ).toBe(true); + // Not a bare match or a colon-suffixed one -- some other, unrelated code string sharing only the prefix. + expect(isStructuralProviderConfigError(new Error("codex_credential_isolation_requiredXYZ"))).toBe(false); + }); + it("runWorkersOpinion still retries a genuinely transient (non-timeout, non-429) error up to the full budget", async () => { let attempts = 0; const run = vi.fn(async () => { diff --git a/test/unit/selfhost-ai.test.ts b/test/unit/selfhost-ai.test.ts index 6008e445bc..3b5b875deb 100644 --- a/test/unit/selfhost-ai.test.ts +++ b/test/unit/selfhost-ai.test.ts @@ -1945,6 +1945,32 @@ describe("subscription CLI helpers + fail-safe", () => { ).rejects.toThrow(/codex_credential_isolation_required/); }); + it("credential isolation surfaces a specific rename hint when only the legacy GITTENSORY_ var is set (#7466)", async () => { + const shouldNotSpawn: StubSpawn = async () => { + throw new Error("spawned"); + }; + await expect( + createCodexAi( + { GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER: "1" }, + shouldNotSpawn, + ).run("gpt-5", { prompt: "x" }), + ).rejects.toThrow( + /^codex_credential_isolation_required: GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER is set but no longer read -- rename it to LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER$/, + ); + }); + + it("credential isolation: CODEX_HOME being the actual blocker still wins over the legacy-var rename hint", async () => { + const shouldNotSpawn: StubSpawn = async () => { + throw new Error("spawned"); + }; + await expect( + createCodexAi( + { CODEX_HOME: "/home/node/.codex", GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER: "1" }, + shouldNotSpawn, + ).run("gpt-5", { prompt: "x" }), + ).rejects.toThrow(/^codex_credential_isolation_required$/); + }); + it("resolveCodexAuthPath: CODEX_HOME wins, else HOME/.codex, else ~/.codex", () => { expect(resolveCodexAuthPath({ CODEX_HOME: "/data/codex", HOME: "/home/node" })).toBe( "/data/codex/auth.json",