fix(safety): kill switches become ratchets; stop a dry-run GitHub write; breaker covers content-inspection closes

src/queue/processors.ts

@@ -11573,6 +11573,8 @@ async function maybePublishPrPublicSurface(
             // Pins the actions_fallback dispatch (#4112) to a trusted ref -- see buildCapture. Absent (no
             // stored default branch yet) ⇒ that dispatch just never fires, same as leaving it unconfigured.
             ...(repo?.defaultBranch ? { defaultBranchRef: repo.defaultBranch } : {}),
+            // #9067: the actions_fallback dispatch is a real GitHub write and must obey dry_run/paused.
+            mode,
           };
           // review.visual.enabled (#4083): a config-as-code override layered on top of the screenshotsAllowed
           // env-var gate above, not a replacement for it. Unset/true ⇒ defer to that gate's decision (buildCapture

src/review/visual/actions-fallback.ts

@@ -1,3 +1,4 @@
+import type { AgentActionMode } from "../../settings/agent-execution";
 // GitHub-Actions build-and-serve FALLBACK for a repo with no CI-produced preview deploy (#4112, part of the
 // #3607 visual-capture convergence epic).
 //
@@ -91,7 +92,17 @@ export async function dispatchVisualCaptureFallback(params: {
   headSha: string;
   routes: readonly string[];
   rateLimitAdmissionKey?: GitHubRateLimitAdmissionKey | undefined;
+  /** #9067: the pass's resolved AgentActionMode. See the refusal below. */
+  mode?: AgentActionMode | undefined;
 }): Promise<boolean> {
+  // #9067: this is a real GitHub WRITE -- it starts an Actions run and burns CI minutes in the target repo --
+  // issued through raw timeoutFetch, so it never passes makeInstallationOctokit's request hook, the structural
+  // check that suppresses every OTHER installation mutation when mode !== "live". runVisualCapture only
+  // short-circuited on `paused`, so under `dry_run` this fired for real, contradicting the documented dry-run
+  // contract ("suppresses the terminal GitHub-side write only"). Refusing HERE rather than at the call site
+  // makes the guarantee structural: a future caller cannot reintroduce the bypass by forgetting a check,
+  // because an absent mode is treated as non-live.
+  if (params.mode !== "live") return false;
   const base = `https://api.github.com/repos/${params.repo.owner}/${params.repo.repo}`;
   try {
     const headers = new Headers();

src/review/visual/capture.ts

@@ -1,3 +1,4 @@
+import type { AgentActionMode } from "../../settings/agent-execution";
 // Realtime visual capture (reviewbot→loopover convergence — visual port). taopedia-style before/after.
 //
 // before = production (review.visual.production_url, falling back to the global PUBLIC_SITE_ORIGIN env var);
@@ -236,6 +237,13 @@ export interface CaptureTarget {
    *  trusted ref rather than the PR's own branch. Absent ⇒ the fallback is never dispatched (fail-safe: no
    *  ref to pin to means no dispatch, not a guess at "main"). */
   defaultBranchRef?: string | undefined;
+  /** #9067: the pass's resolved AgentActionMode. REQUIRED to dispatch the actions_fallback workflow -- that
+   *  dispatch is a real GitHub WRITE (it starts an Actions run and burns CI minutes in the target repo), but
+   *  it goes out via raw timeoutFetch and so bypasses makeInstallationOctokit's mutation-suppression hook,
+   *  which is the structural invariant every other installation write relies on. runVisualCapture only ever
+   *  short-circuited on `paused`, so under `dry_run` this fired for real -- directly contradicting the
+   *  documented dry-run contract. Absent ⇒ treated as non-live and the dispatch never fires (fail-safe). */
+  mode?: AgentActionMode | undefined;
 }
 
 function joinUrl(base: string, path: string): string {
@@ -754,6 +762,8 @@ export async function buildCapture(
         headSha: target.headSha,
         routes,
         rateLimitAdmissionKey,
+        // #9067: threaded so the writer itself can refuse under dry_run/paused (see its own doc comment).
+        mode: target.mode,
       });
       if (dispatched) await markFallbackDispatched(env, target.headSha);
     }

src/settings/agent-actions.ts

@@ -612,10 +612,24 @@ export function downgradeCloseToHold(
   labelSettings: AgentDispositionLabelSettings = {},
   untrustworthyRuleCodes: ReadonlySet<string> = new Set(),
 ): PlannedAgentAction[] {
+  // #9086: the breaker exemption used to key on `closeKind === "heuristic"` alone, so all SIX non-heuristic
+  // close paths (blacklist, contributor_cap, review_nag, copycat, screenshot_table, and the linked-issue hard
+  // rule) were returned untouched by BOTH Reason A and Reason B — and none of them gets the executor-side live
+  // recheck either, which is likewise gated on "heuristic".
+  //
+  // The stated justification ("zero-hallucination, deterministic") holds for the IDENTITY-based closes:
+  // blacklist and contributor_cap read facts about an account, so a precision breaker has nothing to add. It
+  // does NOT hold for the CONTENT-INSPECTION closes, whose "determinism" is a regex heuristic over free-text
+  // markdown — screenshot_table is our single top close reason all-time. A deterministic rule can still be
+  // systematically wrong, which is precisely what the breaker exists to catch.
+  const CONTENT_INSPECTION_CLOSE_KINDS: ReadonlySet<string> = new Set(["screenshot_table", "review_nag", "copycat"]);
+  const isBreakerEligibleClose = (action: PlannedAgentAction): boolean =>
+    action.closeKind === "heuristic" || (action.closeKind !== undefined && CONTENT_INSPECTION_CLOSE_KINDS.has(action.closeKind));
+
   // Reason A (project-level, unchanged from before #7986): no concrete evidence at all, AND the project's
   // close-precision breaker has engaged.
   const noConcreteEvidenceUnderProjectBreaker = (action: PlannedAgentAction): boolean =>
-    action.actionClass === "close" && action.closeKind === "heuristic" && action.closeConcreteEvidence !== true && closeHoldOnly;
+    action.actionClass === "close" && isBreakerEligibleClose(action) && action.closeConcreteEvidence !== true && closeHoldOnly;
   // Reason B (#7986, per-rule, independent of closeHoldOnly): HAS concrete evidence, but every code that
   // justified it has its own bad track record. `.every` (not `.some`) so a close backed by a MIX of a
   // trustworthy code and an untrustworthy one keeps its exemption via the trustworthy code -- only a close
@@ -626,7 +640,7 @@ export function downgradeCloseToHold(
   };
   const isDowngradableClose = (action: PlannedAgentAction): boolean =>
     action.actionClass === "close" &&
-    action.closeKind === "heuristic" &&
+    isBreakerEligibleClose(action) &&
     (noConcreteEvidenceUnderProjectBreaker(action) || (action.closeConcreteEvidence === true && everyJustifyingCodeUntrustworthy(action)));
   if (!planned.some(isDowngradableClose)) return planned;
   const labels = resolveAgentDispositionLabels(labelSettings);

src/signals/focus-manifest.ts

@@ -568,9 +568,25 @@ export function resolveEffectiveSettings(
     unlinkedIssueGuardrail: unlinkedIssueGuardrailOverride,
     screenshotTableGate: screenshotTableGateOverride,
     advisoryAiRouting: advisoryAiRoutingOverride,
+    // #9049: the two KILL-SWITCH flags are pulled out of the wholesale spread and merged as RATCHETS below --
+    // config-as-code may TIGHTEN safety but must never LOOSEN it.
+    agentPaused: agentPausedOverride,
+    agentDryRun: agentDryRunOverride,
     ...restManifestSettings
   } = manifest.settings;
   const effective: RepositorySettings = { ...dbSettings, ...restManifestSettings };
+  // #9049: verified live -- the global default private-config layer carried `agentDryRun: false`, and because
+  // the manifest wins a plain spread, that silently discarded `agent_dry_run=1` on EVERY read for EVERY repo.
+  // An operator flipping dry-run from the dashboard, `PUT /settings`, `loopover-mcp maintain`, or the
+  // `loopover_set_agent_dry_run` MCP tool got a success message while ORB kept making real GitHub writes --
+  // the tool writes the raw row back and never re-resolves effective settings, so nothing surfaced the
+  // discard. A kill switch that reports success and does nothing is worse than one that does not exist.
+  //
+  // Both flags are therefore RATCHETS: `true` from EITHER layer wins. A repo's `.loopover.yml` can still pause
+  // or dry-run itself (tightening), which is the whole point of config-as-code here; it simply cannot un-pause
+  // or un-dry-run what an operator set in the DB. Every other setting keeps manifest-wins semantics unchanged.
+  effective.agentPaused = dbSettings.agentPaused === true || agentPausedOverride === true;
+  effective.agentDryRun = dbSettings.agentDryRun === true || agentDryRunOverride === true;
   if (typeLabelsOverride !== undefined) {
     // `null` is parseFocusManifest's distinct signal for a literal `typeLabels: {}` -- a deliberate
     // "zero configured categories for this repo" that REPLACES the DB value wholesale, rather than a

test/integration/ams-ingest.test.ts

@@ -124,39 +124,45 @@ describe("handleAmsIngest()", () => {
 
 describe("POST /v1/ams/ingest route", () => {
   const app = createApp();
+  // #9046: the collector fails CLOSED on an unset token, so every request-shape test below must present a
+  // valid credential — otherwise it asserts 200/400/413 against an endpoint that now correctly answers 401.
+  const AUTH = { authorization: "Bearer fleet-secret" };
+  const authedEnv = (extra: Partial<Env> = {}) => createTestEnv({ AMS_INGEST_TOKEN: "fleet-secret", ...extra });
 
   it("returns 200 + accepted count for a valid batch", async () => {
-    const env = createTestEnv();
+    const env = authedEnv();
     const body = JSON.stringify({ instanceId: "abc0", events: [{ repoHash: "rhash", prHash: "phash", decision: "merged" }] });
-    const res = await app.request("/v1/ams/ingest", { method: "POST", headers: { "content-type": "application/json" }, body }, env);
+    const res = await app.request("/v1/ams/ingest", { method: "POST", headers: { "content-type": "application/json", ...AUTH }, body }, env);
     expect(res.status).toBe(200);
     expect(((await res.json()) as { accepted: number }).accepted).toBe(1);
   });
 
   it("returns 400 for invalid JSON", async () => {
-    const res = await app.request("/v1/ams/ingest", { method: "POST", headers: { "content-type": "application/json" }, body: "{bad" }, createTestEnv());
+    const res = await app.request("/v1/ams/ingest", { method: "POST", headers: { "content-type": "application/json", ...AUTH }, body: "{bad" }, authedEnv());
     expect(res.status).toBe(400);
     expect(((await res.json()) as { error: string }).error).toBe("invalid_json");
   });
 
   it("returns 400 for an empty body", async () => {
-    const res = await app.request("/v1/ams/ingest", { method: "POST", body: "" }, createTestEnv());
+    const res = await app.request("/v1/ams/ingest", { method: "POST", headers: AUTH, body: "" }, authedEnv());
     expect(res.status).toBe(400);
   });
 
   it("returns 413 when the body exceeds the shared ingest byte ceiling", async () => {
     const huge = "x".repeat(MAX_ORB_INGEST_BODY_BYTES + 16);
-    const res = await app.request("/v1/ams/ingest", { method: "POST", body: huge }, createTestEnv());
+    const res = await app.request("/v1/ams/ingest", { method: "POST", headers: AUTH, body: huge }, authedEnv());
     expect(res.status).toBe(413);
     expect(((await res.json()) as { error: string }).error).toBe("payload_too_large");
   });
 
-  it("optional collector token: open when unset; enforced once AMS_INGEST_TOKEN is set", async () => {
+  // #9046: this used to assert the collector was OPEN when the token was unset — the shipped default — which
+  // let anyone with network access POST batches. It now fails closed.
+  it("collector token: FAILS CLOSED when unset; enforced exactly once AMS_INGEST_TOKEN is set", async () => {
     const body = JSON.stringify({ instanceId: "abc0", events: [{ repoHash: "rhash", prHash: "phash", decision: "merged" }] });
     const post = (env: Env, authorization?: string) =>
       app.request("/v1/ams/ingest", { method: "POST", headers: { "content-type": "application/json", ...(authorization ? { authorization } : {}) }, body }, env);
 
-    expect((await post(createTestEnv())).status).toBe(200);
+    expect((await post(createTestEnv())).status).toBe(401); // unset ⇒ closed, not open
     const env = createTestEnv({ AMS_INGEST_TOKEN: "fleet-secret" });
     expect((await post(env)).status).toBe(401);
     expect((await post(env, "Bearer wrong")).status).toBe(401);

test/integration/orb-ingest.test.ts

@@ -304,29 +304,33 @@ describe("readOrbIngestBody()", () => {
 
 describe("POST /v1/orb/ingest route", () => {
   const app = createApp();
+  // #9046: the collector fails CLOSED on an unset token, so every request-shape test below must present a
+  // valid credential — otherwise it asserts 200/400/413 against an endpoint that now correctly answers 401.
+  const AUTH = { authorization: "Bearer fleet-secret" };
+  const authedEnv = (extra: Partial<Env> = {}) => createTestEnv({ ORB_INGEST_TOKEN: "fleet-secret", ...extra });
 
   it("returns 200 + accepted count for a valid batch", async () => {
-    const env = createTestEnv();
+    const env = authedEnv();
     const body = JSON.stringify({ instance_id: "abc0", events: [{ repo_hash: "rhash", pr_hash: "phash", outcome: "merged", reversal_flag: "none" }] });
-    const res = await app.request("/v1/orb/ingest", { method: "POST", headers: { "content-type": "application/json" }, body }, env);
+    const res = await app.request("/v1/orb/ingest", { method: "POST", headers: { "content-type": "application/json", ...AUTH }, body }, env);
     expect(res.status).toBe(200);
     expect(((await res.json()) as { accepted: number }).accepted).toBe(1);
   });
 
   it("returns 400 for invalid JSON", async () => {
-    const res = await app.request("/v1/orb/ingest", { method: "POST", headers: { "content-type": "application/json" }, body: "{bad" }, createTestEnv());
+    const res = await app.request("/v1/orb/ingest", { method: "POST", headers: { "content-type": "application/json", ...AUTH }, body: "{bad" }, authedEnv());
     expect(res.status).toBe(400);
     expect(((await res.json()) as { error: string }).error).toBe("invalid_json");
   });
 
   it("returns 400 for an empty body", async () => {
-    const res = await app.request("/v1/orb/ingest", { method: "POST", body: "" }, createTestEnv());
+    const res = await app.request("/v1/orb/ingest", { method: "POST", headers: AUTH, body: "" }, authedEnv());
     expect(res.status).toBe(400);
   });
 
   it("returns 413 when the body exceeds the ingest byte ceiling", async () => {
     const huge = "x".repeat(MAX_ORB_INGEST_BODY_BYTES + 16);
-    const res = await app.request("/v1/orb/ingest", { method: "POST", body: huge }, createTestEnv());
+    const res = await app.request("/v1/orb/ingest", { method: "POST", headers: AUTH, body: huge }, authedEnv());
     expect(res.status).toBe(413);
     expect(((await res.json()) as { error: string }).error).toBe("payload_too_large");
   });
@@ -340,20 +344,22 @@ describe("POST /v1/orb/ingest route", () => {
     });
     const res = await app.request(
       "/v1/orb/ingest",
-      { method: "POST", body: stream, ...({ duplex: "half" } as object) },
-      createTestEnv(),
+      { method: "POST", headers: AUTH, body: stream, ...({ duplex: "half" } as object) },
+      authedEnv(),
     );
     expect(res.status).toBe(413);
     expect(((await res.json()) as { error: string }).error).toBe("payload_too_large");
   });
 
-  it("optional collector token (#1285): open when unset; enforced once ORB_INGEST_TOKEN is set", async () => {
+  // #9046: this used to assert the collector was OPEN when the token was unset — the shipped default — so
+  // anyone with network access could POST batches that feed the PUBLISHED accuracy numbers. Now fails closed.
+  it("collector token (#1285/#9046): FAILS CLOSED when unset; enforced exactly once ORB_INGEST_TOKEN is set", async () => {
     const body = JSON.stringify({ instance_id: "abc0", events: [{ repo_hash: "rhash", pr_hash: "phash", outcome: "merged" }] });
     const post = (env: Env, authorization?: string) =>
       app.request("/v1/orb/ingest", { method: "POST", headers: { "content-type": "application/json", ...(authorization ? { authorization } : {}) }, body }, env);
 
-    // Token UNSET → open ingress (the live fleet keeps working with no auth header).
-    expect((await post(createTestEnv())).status).toBe(200);
+    // Token UNSET → CLOSED. An unconfigured collector rejects rather than accepting anonymous writes.
+    expect((await post(createTestEnv())).status).toBe(401);
     // Token SET → a missing or wrong bearer is rejected before the body is parsed.
     const env = createTestEnv({ ORB_INGEST_TOKEN: "fleet-secret" });
     expect((await post(env)).status).toBe(401);
@@ -366,11 +372,17 @@ describe("POST /v1/orb/ingest route", () => {
 describe("Orb instance registry routes (/v1/internal/orb/instances)", () => {
   const app = createApp();
   const auth = { authorization: "Bearer dev-internal-token" };
+  // #9046: ingest now requires a credential, so any env used with ingestOne must carry ORB_INGEST_TOKEN.
+  const ingestEnv = (extra: Partial<Env> = {}) => createTestEnv({ ORB_INGEST_TOKEN: "fleet-secret", ...extra });
   const ingestOne = (env: Env, instance: string) =>
-    app.request("/v1/orb/ingest", { method: "POST", body: JSON.stringify({ instance_id: instance, events: [{ repo_hash: "r", pr_hash: `${instance}-p`, outcome: "merged" }] }) }, env);
+    app.request(
+      "/v1/orb/ingest",
+      { method: "POST", headers: { authorization: "Bearer fleet-secret" }, body: JSON.stringify({ instance_id: instance, events: [{ repo_hash: "r", pr_hash: `${instance}-p`, outcome: "merged" }] }) },
+      env,
+    );
 
   it("lists ingested instances as unregistered with their stored-signal count", async () => {
-    const env = createTestEnv();
+    const env = ingestEnv();
     await ingestOne(env, "inst-a");
     const res = await app.request("/v1/internal/orb/instances", { headers: auth }, env);
     expect(res.status).toBe(200);
@@ -383,7 +395,7 @@ describe("Orb instance registry routes (/v1/internal/orb/instances)", () => {
   });
 
   it("registers an instance (and can unregister it)", async () => {
-    const env = createTestEnv();
+    const env = ingestEnv();
     await ingestOne(env, "inst-b");
     const reg = await app.request("/v1/internal/orb/instances/register", { method: "POST", headers: auth, body: JSON.stringify({ instanceId: "inst-b" }) }, env);
     expect(((await reg.json()) as { registered: boolean }).registered).toBe(true);

test/integration/orb-oauth.test.ts

@@ -32,7 +32,13 @@ describe("GET /v1/orb/oauth/callback (post-install landing)", () => {
   it("the new exemption + rate class are path-specific (a later orb path still routes)", async () => {
     // /v1/orb/ingest falls through PAST the new callback checks, exercising their FALSE side in both
     // requiresApiToken + routeClassForPath (the webhook path short-circuits earlier and wouldn't reach them).
-    const res = await app.request("/v1/orb/ingest", { method: "POST" }, createTestEnv());
+    // #9046: ingest now fails closed on an unset token, so present one — otherwise this would stop at 401 and
+    // no longer demonstrate that the request reaches the ingest handler's own BODY handling.
+    const res = await app.request(
+      "/v1/orb/ingest",
+      { method: "POST", headers: { authorization: "Bearer fleet-secret" } },
+      createTestEnv({ ORB_INGEST_TOKEN: "fleet-secret" }),
+    );
     expect([400, 413]).toContain(res.status); // reached the (exempt) ingest handler, failed only on the empty body
   });