fix(selfhost): distinguish a stale codex-reviewer flag name from unconfigured and cool down its breaker (#7473)

The GITTENSORY_→LOOPOVER_ env rebrand (#5652) retired dual-read of
GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER and hardcoded a strict
LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER === "1" check. An operator whose .env
still uses the legacy name silently reverts to fully-disabled and gets the
exact same generic codex_credential_isolation_required error as someone who
never opted in at all — no signal telling them to rename the var.

Recognize the retired flag name in assertCodexCredentialIsolation (without
honoring it) and throw an actionable "rename it to
LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER" message, but only when the current flag
isn't already correctly set so a mounted CODEX_HOME isn't mislabeled a rename
problem. The message keeps the codex_credential_isolation_required prefix so
the structural circuit breaker still recognizes it. Read via a constant rather
than env.GITTENSORY_... so the retired name stays out of the generated
self-host env reference.

Also widen isStructuralProviderConfigError's regex to match
codex_credential_isolation_required (bare or with a ": rename …" suffix). It's
a deterministic config failure, so it now earns the hour-long structural
cooldown instead of retrying every 60s — the flood pattern that breaker exists
to prevent.

Closes #7466

Co-authored-by: bitfathers94 <237535319+bitfathers94@users.noreply.github.com>

Author: bitfathers94

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,

src/selfhost/ai.ts

@@ -473,12 +473,29 @@ export function subscriptionCliEnv(
   return child;
 }
 
+// The pre-rebrand name of the unsafe-reviewer opt-in flag. #5652 retired dual-read of GITTENSORY_-prefixed vars
+// repo-wide, so an operator whose .env still uses this name silently reverts to fully-disabled. We deliberately do
+// NOT honor it (the rebrand is intentional), but we still recognize it here purely to emit an actionable "rename it"
+// error instead of the same generic message an operator who never configured anything at all would get. Accessed
+// via this constant (not `env.GITTENSORY_...`) so the retired name stays out of the generated self-host env
+// reference — it is not a var operators should configure, only one we detect to redirect them.
+const LEGACY_UNSAFE_CODEX_REVIEWER_FLAG = "GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER";
+
 function assertCodexCredentialIsolation(env: Record<string, string | undefined>): void {
   // `codex exec` receives attacker-controlled PR title/body/diff text. Its read-only sandbox prevents writes, but not
   // reads, so a self-hosted OAuth home mounted into the same filesystem can be prompt-injected into public output.
   // 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") {
+    // An operator still on the retired flag name gets a specific, actionable signal to rename it — but only when the
+    // current flag isn't already correctly set (a mounted CODEX_HOME with a valid opt-in is a different failure and
+    // must not be mislabeled a rename problem). Keep the `codex_credential_isolation_required` prefix so the
+    // structural circuit breaker in ai-review.ts still recognizes this deterministic failure and backs off.
+    if (env.LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER !== "1" && env[LEGACY_UNSAFE_CODEX_REVIEWER_FLAG] === "1") {
+      throw new Error(
+        `codex_credential_isolation_required: ${LEGACY_UNSAFE_CODEX_REVIEWER_FLAG} is set but was retired in #5652; rename it to LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER`,
+      );
+    }
     throw new Error("codex_credential_isolation_required");
   }
 }

src/services/ai-review.ts

@@ -1022,15 +1022,19 @@ 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
- *  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
- *  {@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. */
+ *  `codex_auth_not_configured` / `codex_no_auth` — a missing or expired credential file — or
+ *  `codex_credential_isolation_required` — the fail-closed opt-in guard, thrown either bare or with a
+ *  `: rename …` detail suffix). Unlike a transient timeout or rate limit, these fail identically on every future
+ *  attempt until an operator re-runs `codex auth` / fixes the opt-in flag -- 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 {@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

test/unit/ai-review.test.ts

@@ -3077,6 +3077,18 @@ describe("pure helpers", () => {
   it("isStructuralProviderConfigError matches only codex's own structural-config error messages, not other Errors or non-Error throws (GITTENSORY-K/8)", () => {
     expect(isStructuralProviderConfigError(new Error("codex_auth_not_configured: ~/.codex/auth.json not found"))).toBe(true);
     expect(isStructuralProviderConfigError(new Error("codex_no_auth: auth.json missing or expired"))).toBe(true);
+    // The fail-closed credential-isolation guard is equally deterministic, thrown either bare (never opted in) or
+    // with a `: rename …` detail suffix (legacy flag name still set) -- both must earn the structural cooldown (#7466).
+    expect(isStructuralProviderConfigError(new Error("codex_credential_isolation_required"))).toBe(true);
+    expect(
+      isStructuralProviderConfigError(
+        new Error(
+          "codex_credential_isolation_required: GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER is set but was retired in #5652; rename it to LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER",
+        ),
+      ),
+    ).toBe(true);
+    // Prefix-anchored, but must not match a longer look-alike token that merely starts with the same characters.
+    expect(isStructuralProviderConfigError(new Error("codex_no_auth_pending"))).toBe(false);
     expect(isStructuralProviderConfigError(new Error("connection reset"))).toBe(false);
     // Anchored ("^codex_...") -- a wrapped/rethrown message doesn't match, only the exact provider-level throw does.
     expect(isStructuralProviderConfigError(new Error("wrapped: codex_auth_not_configured: nested"))).toBe(false);

test/unit/selfhost-ai.test.ts

@@ -1945,6 +1945,52 @@ describe("subscription CLI helpers + fail-safe", () => {
     ).rejects.toThrow(/codex_credential_isolation_required/);
   });
 
+  it("credential isolation: an operator still on the retired GITTENSORY_ flag name gets an actionable rename error, not the generic one (#7466)", async () => {
+    const shouldNotSpawn: StubSpawn = async () => {
+      throw new Error("spawned");
+    };
+    // Legacy flag set (and the current LOOPOVER_ one absent) — the operator opted in under the pre-rebrand name and
+    // silently reverted to disabled. The error must name both the retired var and its replacement so it is fixable
+    // without reading the source, and must still carry the codex_credential_isolation_required prefix.
+    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 was retired in #5652; rename it to LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER/,
+    );
+  });
+
+  it("credential isolation: never-configured still throws the plain generic error, distinct from the legacy-rename one (#7466)", async () => {
+    const shouldNotSpawn: StubSpawn = async () => {
+      throw new Error("spawned");
+    };
+    // Neither the current nor the retired flag is set — this must stay the bare generic message so it is
+    // distinguishable from the legacy-rename case above.
+    await expect(createCodexAi({}, shouldNotSpawn).run("gpt-5", { prompt: "x" })).rejects.toThrow(
+      /^codex_credential_isolation_required$/,
+    );
+  });
+
+  it("credential isolation: a set CODEX_HOME with a valid opt-in is not mislabeled a legacy-rename problem even when the legacy flag is also present (#7466)", async () => {
+    const shouldNotSpawn: StubSpawn = async () => {
+      throw new Error("spawned");
+    };
+    // CODEX_HOME is mounted (a distinct failure) while the current opt-in is correctly "1" — the rename branch must
+    // not fire; the operator sees the generic isolation error, not a spurious "rename your flag" instruction.
+    await expect(
+      createCodexAi(
+        {
+          CODEX_HOME: "/home/node/.codex",
+          LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER: "1",
+          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",