fix(review): make review.selftune: false opt-out absolute for the breaker too (#6995)

The accuracy circuit-breaker (runSelfTuneBreaker) had no per-repo
opt-out at all, unlike its sibling selfTuneRepos() (the routine
tuning pass), which already correctly excludes a repo whose
.loopover.yml sets review.selftune: false. A repo could opt out of
routine tuning yet still have its gate mode forced into
holdonly/closehold by the breaker.

Per that flag's own documented intent ("excludes this repo from the
tuning pass"), the opt-out is now absolute: an opted-out repo is
excluded from every part of self-tune, both the merge and close
breakers, in both the plain and miner-scoped passes, and in both
directions -- the breaker can neither newly engage a hold for an
opted-out repo nor auto-clear an already-engaged one. A manifest-load
error fails open (repo stays included), matching selfTuneRepos()'s
same fail-safe precedent.

Closes #6803

Author: JSONbored

src/review/outcomes-wire.ts

@@ -26,6 +26,7 @@
 import { recordAuditEvent } from "../db/repositories";
 import { tryEnqueueDecisionPackRebuild } from "../services/decision-pack";
 import { incr } from "../selfhost/metrics";
+import { loadRepoFocusManifest } from "../signals/focus-manifest-loader";
 import type { GitHubWebhookPayload } from "../types";
 import { errorMessage, nowIso } from "../utils/json";
 import {
@@ -537,6 +538,52 @@ function minerBreakerScope(project: string): string {
   return `${project}${MINER_BREAKER_SCOPE_SUFFIX}`;
 }
 
+/** Strip the `:miner` scope suffix a project key MAY carry, so both `listEngagedProjectScopes`'s scoped keys
+ *  and `GateEvalReport.rows[].project` resolve to the same real repo full name the opt-out check needs. */
+function baseProjectName(project: string): string {
+  return project.endsWith(MINER_BREAKER_SCOPE_SUFFIX) ? project.slice(0, -MINER_BREAKER_SCOPE_SUFFIX.length) : project;
+}
+
+/** #6803: the accuracy circuit-breaker (this whole pass) previously had no per-repo opt-out at all, unlike its
+ *  sibling `selfTuneRepos()` (selftune-wire.ts), which already correctly excludes a repo whose `.loopover.yml`
+ *  sets `review.selftune: false` from the routine tuning pass. Per that flag's own documented intent
+ *  ("excludes this repo from the tuning pass"), the opt-out is ABSOLUTE: it excludes a repo from every part of
+ *  self-tune, not just the routine pass, so the breaker must never engage OR auto-clear holdonly/closehold for
+ *  an opted-out repo either -- a manifest-load error fails OPEN (repo stays included), matching
+ *  `selfTuneRepos()`'s own same fail-safe precedent, since a settings-read blip must never silently widen what
+ *  the breaker acts on. */
+async function isSelfTuneOptedOut(env: Env, repoFullName: string): Promise<boolean> {
+  const manifest = await loadRepoFocusManifest(env, repoFullName).catch(() => null);
+  return manifest?.review.selftune === false;
+}
+
+/** Filter a {@link GateEvalReport}'s rows AND a scope's already-engaged flag list down to the projects that are
+ *  NOT self-tune-opted-out, resolving each project key's real repo name first (`:miner`-suffixed keys included)
+ *  -- the two lists this module's every downstream computation (engage candidates, clear candidates) derives
+ *  from, so filtering both here is sufficient for the opt-out to be absolute. */
+async function excludeSelfTuneOptedOut(
+  env: Env,
+  report: GateEvalReport,
+  engagedHoldonly: readonly string[],
+  engagedClosehold: readonly string[],
+): Promise<{ report: GateEvalReport; engagedHoldonly: string[]; engagedClosehold: string[] }> {
+  const candidateProjects = new Set([
+    ...report.rows.map((row) => baseProjectName(row.project)),
+    ...engagedHoldonly.map(baseProjectName),
+    ...engagedClosehold.map(baseProjectName),
+  ]);
+  const optedOut = new Set<string>();
+  for (const repoFullName of candidateProjects) {
+    if (await isSelfTuneOptedOut(env, repoFullName)) optedOut.add(repoFullName);
+  }
+  if (optedOut.size === 0) return { report, engagedHoldonly: [...engagedHoldonly], engagedClosehold: [...engagedClosehold] };
+  return {
+    report: { ...report, rows: report.rows.filter((row) => !optedOut.has(baseProjectName(row.project))) },
+    engagedHoldonly: engagedHoldonly.filter((project) => !optedOut.has(baseProjectName(project))),
+    engagedClosehold: engagedClosehold.filter((project) => !optedOut.has(baseProjectName(project))),
+  };
+}
+
 /** Run the full engage + auto-clear sequence for one {@link GateEvalReport} (either the plain project-keyed
  *  report or a miner-rescoped one). `eventPrefix` namespaces the emitted log events (`""` for the existing
  *  human/mixed pass, `"miner_"` for the #2352 miner-scoped pass) so an operator can tell which population
@@ -638,22 +685,23 @@ export async function runSelfTuneBreaker(env: Env): Promise<void> {
     const engagedScopes = await listEngagedProjectScopes(env);
     const isMinerScope = (project: string): boolean => project.endsWith(MINER_BREAKER_SCOPE_SUFFIX);
 
-    await runBreakerPassForReport(
-      flags,
+    // #6803: exclude every self-tune-opted-out repo from both passes -- see excludeSelfTuneOptedOut's own doc
+    // comment for why this must happen before engage/clear candidates are computed, not filtered after.
+    const plainPass = await excludeSelfTuneOptedOut(
+      env,
       report,
       engagedScopes.holdonly.filter((project) => !isMinerScope(project)),
       engagedScopes.closehold.filter((project) => !isMinerScope(project)),
-      nowMs,
-      "",
     );
-    await runBreakerPassForReport(
-      flags,
+    const minerPass = await excludeSelfTuneOptedOut(
+      env,
       minerReport,
       engagedScopes.holdonly.filter(isMinerScope),
       engagedScopes.closehold.filter(isMinerScope),
-      nowMs,
-      "miner_",
     );
+
+    await runBreakerPassForReport(flags, plainPass.report, plainPass.engagedHoldonly, plainPass.engagedClosehold, nowMs, "");
+    await runBreakerPassForReport(flags, minerPass.report, minerPass.engagedHoldonly, minerPass.engagedClosehold, nowMs, "miner_");
   } catch (error) {
     console.warn(
       JSON.stringify({

test/unit/outcomes-wire.test.ts

@@ -18,6 +18,7 @@ import {
   type PlannedAgentAction,
 } from "../../src/settings/agent-actions";
 import { recordAuditEvent } from "../../src/db/repositories";
+import { upsertRepoFocusManifest } from "../../src/signals/focus-manifest-loader";
 import type { GitHubPullRequestPayload } from "../../src/types";
 import { createTestEnv } from "../helpers/d1";
 
@@ -774,6 +775,91 @@ describe("runSelfTuneBreaker — reads recorded pr_outcome ground truth + engage
     expect(await isCloseHoldOnly(env, "owner/repo")).toBe(false); // close breaker auto-cleared
   });
 
+  describe("#6803: review.selftune: false opt-out is absolute for the breaker too, not just the routine tuning pass", () => {
+    it("does NOT engage the merge or close breaker for an opted-out repo, even with data that would otherwise trip both", async () => {
+      const env = createTestEnv();
+      await upsertRepoFocusManifest(env, "owner/opted-out", { review: { selftune: false } });
+      // Same shape as the plain ENGAGES tests above -- would trip both breakers if this repo weren't opted out.
+      for (let i = 0; i < 4; i += 1) await seedDecisionAndOutcome(env, "owner/opted-out", i, "merge", "merged");
+      for (let i = 4; i < 12; i += 1) await seedDecisionAndOutcome(env, "owner/opted-out", i, "merge", "closed");
+      for (let i = 12; i < 16; i += 1) await seedDecisionAndOutcome(env, "owner/opted-out", i, "close", "closed");
+      for (let i = 16; i < 24; i += 1) await seedDecisionAndOutcome(env, "owner/opted-out", i, "close", "merged");
+
+      await runSelfTuneBreaker(env);
+
+      expect(await isHoldOnly(env, "owner/opted-out")).toBe(false);
+      expect(await isCloseHoldOnly(env, "owner/opted-out")).toBe(false);
+    });
+
+    it("does NOT auto-clear an already-engaged flag for an opted-out repo, even with fully recovered precision -- the opt-out is absolute, not one-directional", async () => {
+      const env = createTestEnv();
+      const flags = createFlagStore(env);
+      await flags.setFlag("holdonly:owner/opted-out", true);
+      await flags.setFlag("closehold:owner/opted-out", true);
+      await env.DB.prepare(
+        "UPDATE system_flags SET updated_at = datetime('now', '-2 days') WHERE key IN ('holdonly:owner/opted-out', 'closehold:owner/opted-out')",
+      ).run();
+      await upsertRepoFocusManifest(env, "owner/opted-out", { review: { selftune: false } });
+      // Fully recovered precision -- would auto-clear both breakers (per the AUTO-CLEARS test above) if this
+      // repo weren't opted out.
+      for (let i = 0; i < 12; i += 1) await seedDecisionAndOutcome(env, "owner/opted-out", i, "merge", "merged");
+      for (let i = 12; i < 24; i += 1) await seedDecisionAndOutcome(env, "owner/opted-out", i, "close", "closed");
+
+      const log = vi.spyOn(console, "log").mockImplementation(() => {});
+      await runSelfTuneBreaker(env);
+      log.mockRestore();
+
+      expect(await isHoldOnly(env, "owner/opted-out")).toBe(true);
+      expect(await isCloseHoldOnly(env, "owner/opted-out")).toBe(true);
+    });
+
+    it("also excludes an opted-out repo from the miner-scoped pass (#2352)", async () => {
+      const env = createTestEnv();
+      await upsertRepoFocusManifest(env, "owner/opted-out", { review: { selftune: false } });
+      // Miner-authored rows (miner_authored=1), same shape as the #2352 ENGAGES test: 33% precision, would
+      // otherwise trip the holdonly:owner/opted-out:miner flag.
+      for (let i = 0; i < 4; i += 1) {
+        await env.DB.prepare(
+          "INSERT INTO review_audit (id, project, target_id, event_type, decision, source, head_sha, summary, miner_authored, created_at) VALUES (?, ?, ?, 'gate_decision', 'merge', 'gittensory-native', ?, NULL, 1, CURRENT_TIMESTAMP)",
+        )
+          .bind(`gd:m:owner/opted-out#${i}`, "owner/opted-out", `owner/opted-out#${i}`, `sha${i}`)
+          .run();
+        await env.DB.prepare(
+          "INSERT INTO review_audit (id, project, target_id, event_type, decision, source, head_sha, summary, created_at) VALUES (?, ?, ?, 'pr_outcome', 'merged', 'gittensory-native', NULL, NULL, CURRENT_TIMESTAMP)",
+        )
+          .bind(`po:m:owner/opted-out#${i}`, "owner/opted-out", `owner/opted-out#${i}`)
+          .run();
+      }
+      for (let i = 4; i < 12; i += 1) {
+        await env.DB.prepare(
+          "INSERT INTO review_audit (id, project, target_id, event_type, decision, source, head_sha, summary, miner_authored, created_at) VALUES (?, ?, ?, 'gate_decision', 'merge', 'gittensory-native', ?, NULL, 1, CURRENT_TIMESTAMP)",
+        )
+          .bind(`gd:m:owner/opted-out#${i}`, "owner/opted-out", `owner/opted-out#${i}`, `sha${i}`)
+          .run();
+        await env.DB.prepare(
+          "INSERT INTO review_audit (id, project, target_id, event_type, decision, source, head_sha, summary, created_at) VALUES (?, ?, ?, 'pr_outcome', 'closed', 'gittensory-native', NULL, NULL, CURRENT_TIMESTAMP)",
+        )
+          .bind(`po:m:owner/opted-out#${i}`, "owner/opted-out", `owner/opted-out#${i}`)
+          .run();
+      }
+
+      await runSelfTuneBreaker(env);
+
+      expect(await isHoldOnly(env, "owner/opted-out:miner")).toBe(false);
+    });
+
+    it("does not opt out an unrelated repo (the exclusion is per-repo, not global)", async () => {
+      const env = createTestEnv();
+      await upsertRepoFocusManifest(env, "owner/opted-out", { review: { selftune: false } });
+      for (let i = 0; i < 4; i += 1) await seedDecisionAndOutcome(env, "owner/still-tuned", i, "merge", "merged");
+      for (let i = 4; i < 12; i += 1) await seedDecisionAndOutcome(env, "owner/still-tuned", i, "merge", "closed");
+
+      await runSelfTuneBreaker(env);
+
+      expect(await isHoldOnly(env, "owner/still-tuned")).toBe(true);
+    });
+  });
+
   it("never throws (fails safe) even when review_audit reads blow up", async () => {
     const env = createTestEnv();
     const realPrepare = env.DB.prepare.bind(env.DB);