fix(gate): close the qualityGateMode: block loophole in settings/dashboard/API

normalizeReadinessGateMode downgrades gate.readiness.mode: block to advisory
at parse time, but that only covered the YAML gate: mapping. The identically
named qualityGateMode field reachable through .gittensory.yml's settings:
override, the maintainer dashboard save, and the internal settings write
endpoint all bypassed it, so a value supplied through any of those paths
still persisted and resolved as "block" — preserving the exact false belief
that a real quality floor is enforced.

Close it at every point the value can enter or be read:
- parseSettingsOverride now normalizes settings.qualityGateMode the same
  way as gate.readiness.mode (exported for reuse).
- Both settings-write API routes (maintainer PATCH-style save and the
  internal full-settings write) downgrade a written "block" before it
  reaches the DB.
- resolveEffectiveSettings downgrades "block" as a final step regardless
  of source, so a repo whose DB row already has quality_gate_mode = "block"
  from before these guards existed is also corrected wherever effective
  settings are read (the review/gate pipeline and the settings-preview
  dashboard both go through this same resolver).

Author: JSONbored

src/api/routes.ts

@@ -238,7 +238,7 @@ import { isRagEnabled } from "../review/rag-wire";
 import { getPublicStats, isPublicStatsEnabled } from "../review/public-stats";
 import { buildMaintainerQualityDashboard, isMaintainerQualityDataStale } from "../services/maintainer-quality-dashboard";
 import { MAX_LOCAL_SCORER_WARNING_CHARS, MAX_LOCAL_SCORER_WARNING_COUNT } from "../signals/local-scorer-diagnostics";
-import { compileFocusManifestPolicy, MAX_FOCUS_MANIFEST_BYTES } from "../signals/focus-manifest";
+import { compileFocusManifestPolicy, MAX_FOCUS_MANIFEST_BYTES, normalizeReadinessGateMode } from "../signals/focus-manifest";
 import { resolveRepositorySettings } from "../settings/repository-settings";
 import { loadPublicRepoFocusManifest, loadRepoFocusManifest, upsertRepoFocusManifest } from "../signals/focus-manifest-loader";
 import { buildRepoOnboardingPackPreviewForRepo } from "../services/repo-onboarding-pack";
@@ -265,6 +265,7 @@ import type {
   PullRequestRecord,
   RepoSyncSegmentRecord,
   RepositoryRecord,
+  RepositorySettings,
 } from "../types";
 import { errorMessage, nowIso } from "../utils/json";
 
@@ -686,6 +687,21 @@ const maintainerSettingsSchema = z
   })
   .partial();
 
+/** Readiness/quality can never hard-block a PR (buildQualityGateWarning is always advisory-severity;
+ *  isConfiguredGateBlocker has no branch for it) — downgrade a settings-write's `qualityGateMode: "block"` to
+ *  `"advisory"` here too, mirroring the same downgrade `.gittensory.yml`'s `gate.readiness.mode` /
+ *  `settings.qualityGateMode` already get in normalizeReadinessGateMode, so the dashboard/API save path can
+ *  never persist a value that implies enforcement it doesn't have (#2267). Callers check for `undefined`
+ *  (a PATCH-style save that didn't touch this field) before calling — this only handles the defined case, so
+ *  the return type never needs `undefined` under `exactOptionalPropertyTypes`. The warnings array is scratch;
+ *  these routes have no warnings-response protocol. */
+function downgradeQualityGateMode(mode: "off" | "advisory" | "block"): "off" | "advisory" {
+  /* v8 ignore next -- never null for a Zod-validated "off"|"advisory"|"block" input (normalizeReadinessGateMode's
+     "must be one of" branch only fires for a value outside that set); the fallback only satisfies the shared
+     parser's broader return type. */
+  return (normalizeReadinessGateMode(mode, "qualityGateMode", []) as "off" | "advisory" | null) ?? "advisory";
+}
+
 // Maintainer BYOK provider key. Write-only: the key is encrypted at rest and never returned. A loose
 // prefix check catches the common provider/key mismatch (e.g. pasting an OpenAI key under Anthropic)
 // without coupling to exact provider key formats: Anthropic keys start with `sk-ant-`; OpenAI keys
@@ -2105,7 +2121,8 @@ export function createApp() {
     const parsed = maintainerSettingsSchema.safeParse(body);
     if (!parsed.success) return c.json({ error: "invalid_repository_settings", issues: parsed.error.issues }, 400);
     const current = await getRepositorySettings(c.env, fullName);
-    const changes = Object.fromEntries(Object.entries(parsed.data).filter(([, value]) => value !== undefined));
+    const changes = Object.fromEntries(Object.entries(parsed.data).filter(([, value]) => value !== undefined)) as Partial<RepositorySettings>;
+    if (changes.qualityGateMode !== undefined) changes.qualityGateMode = downgradeQualityGateMode(changes.qualityGateMode);
     const updated = await upsertRepositorySettings(c.env, { ...current, ...changes, repoFullName: fullName });
     await recordAuditEvent(c.env, {
       eventType: "repo.settings_updated",
@@ -3424,7 +3441,7 @@ export function createApp() {
         gatePack: parsed.data.gatePack,
         linkedIssueGateMode: parsed.data.linkedIssueGateMode,
         duplicatePrGateMode: parsed.data.duplicatePrGateMode,
-        qualityGateMode: parsed.data.qualityGateMode,
+        qualityGateMode: downgradeQualityGateMode(parsed.data.qualityGateMode),
         qualityGateMinScore: parsed.data.qualityGateMinScore,
         aiReviewMode: parsed.data.aiReviewMode,
         aiReviewByok: parsed.data.aiReviewByok,

src/signals/focus-manifest.ts

@@ -357,14 +357,15 @@ function normalizeOptionalGateMode(value: JsonValue | undefined, field: string,
   return null;
 }
 
-/** `gate.readiness.mode` is documented and parsed as the shared off/advisory/block tri-state, but
- *  buildQualityGateWarning (src/rules/advisory.ts) always produces a warning-severity finding — never a
- *  blocker — and isConfiguredGateBlocker has no branch for it: readiness/quality is intentionally
- *  informational-only and can never hard-block a PR. Without this, a maintainer who sets `mode: block`
- *  believes a real quality floor is enforced when the effective behavior is silently advisory-only (#2267).
- *  Downgrade "block" to "advisory" here, with a clear deprecation warning, so the parsed config always
- *  matches what the gate actually does. */
-function normalizeReadinessGateMode(value: JsonValue | undefined, field: string, warnings: string[]): GateRuleMode | null {
+/** `gate.readiness.mode` (and its `settings.qualityGateMode` alias below) is documented and parsed as the shared
+ *  off/advisory/block tri-state, but buildQualityGateWarning (src/rules/advisory.ts) always produces a
+ *  warning-severity finding — never a blocker — and isConfiguredGateBlocker has no branch for it: readiness/
+ *  quality is intentionally informational-only and can never hard-block a PR. Without this, a maintainer who
+ *  sets `mode: block` believes a real quality floor is enforced when the effective behavior is silently
+ *  advisory-only (#2267). Downgrade "block" to "advisory" here, with a clear deprecation warning, so the parsed
+ *  config always matches what the gate actually does. Exported so the settings-write API routes (the
+ *  dashboard/API path for the SAME `qualityGateMode` field) can apply the identical downgrade before persisting. */
+export function normalizeReadinessGateMode(value: JsonValue | undefined, field: string, warnings: string[]): GateRuleMode | null {
   const mode = normalizeOptionalGateMode(value, field, warnings);
   if (mode !== "block") return mode;
   warnings.push(`Manifest gate field "${field}" no longer accepts "block" — readiness/quality is informational-only and can never hard-block a PR; downgrading to "advisory". Use gate.manifestPolicy or another enforceable gate for a real quality floor.`);
@@ -598,7 +599,10 @@ function parseSettingsOverride(value: JsonValue | undefined, warnings: string[])
   if (duplicatePrGateMode !== null) out.duplicatePrGateMode = duplicatePrGateMode;
   const selfAuthoredLinkedIssueGateMode = normalizeOptionalGateMode(r.selfAuthoredLinkedIssueGateMode, "settings.selfAuthoredLinkedIssueGateMode", warnings);
   if (selfAuthoredLinkedIssueGateMode !== null) out.selfAuthoredLinkedIssueGateMode = selfAuthoredLinkedIssueGateMode;
-  const qualityGateMode = normalizeOptionalGateMode(r.qualityGateMode, "settings.qualityGateMode", warnings);
+  // Same tri-state field as gate.readiness.mode above (the friendly gate alias overlays onto it in
+  // resolveEffectiveSettings) — apply the identical "block" → "advisory" downgrade here too, so a maintainer
+  // setting `settings.qualityGateMode: block` directly hits the same deprecation warning (#2267).
+  const qualityGateMode = normalizeReadinessGateMode(r.qualityGateMode, "settings.qualityGateMode", warnings);
   if (qualityGateMode !== null) out.qualityGateMode = qualityGateMode;
   const qualityGateMinScore = normalizeOptionalScore(r.qualityGateMinScore, "settings.qualityGateMinScore", warnings);
   if (qualityGateMinScore !== null) out.qualityGateMinScore = qualityGateMinScore;
@@ -997,6 +1001,14 @@ export function resolveEffectiveSettings(
   if (effective.requireLinkedIssue && effective.linkedIssueGateMode === "off") {
     effective.linkedIssueGateMode = "block";
   }
+  // Readiness/quality can never hard-block a PR (buildQualityGateWarning is always advisory-severity;
+  // isConfiguredGateBlocker has no branch for it). The write-time guards (the settings.qualityGateMode /
+  // gate.readiness.mode parsers above, and the settings-write API routes) stop a NEW "block" value from being
+  // introduced, but a repo whose DB row already has quality_gate_mode = "block" from before those guards
+  // existed would still resolve to it here. Downgrade it at this single resolver too, so the EFFECTIVE settings
+  // the gate/review pipeline AND the settings-preview dashboard read (both call this function) can never carry
+  // a value that implies enforcement it doesn't have, regardless of when or where it was written (#2267).
+  if (effective.qualityGateMode === "block") effective.qualityGateMode = "advisory";
   effective.contributorBlacklist = mergeContributorBlacklists(effective.contributorBlacklist ?? [], sharedContributorBlacklist);
   return effective;
 }

test/integration/api.test.ts

@@ -269,6 +269,20 @@ describe("api routes", () => {
     await expect(response.json()).resolves.toMatchObject({ repoFullName: "acme/badged", badgeEnabled: true });
   });
 
+  it("downgrades qualityGateMode: block to advisory through the internal settings write endpoint too (#2267)", async () => {
+    // Readiness/quality can never hard-block a PR — the internal full-settings write path (used by tooling,
+    // not just the maintainer dashboard) gets the identical downgrade so it can't persist "block" either.
+    const app = createApp();
+    const env = createTestEnv();
+    const response = await app.request(
+      "/v1/internal/repos/acme/readiness-block/settings",
+      { method: "POST", headers: { authorization: `Bearer ${env.INTERNAL_JOB_TOKEN}`, "content-type": "application/json" }, body: JSON.stringify({ qualityGateMode: "block" }) },
+      env,
+    );
+    expect(response.status).toBe(200);
+    await expect(response.json()).resolves.toMatchObject({ repoFullName: "acme/readiness-block", qualityGateMode: "advisory" });
+  });
+
   it("rejects invalid public GitHub repo stats paths before calling GitHub", async () => {
     const app = createApp();
     const env = createTestEnv();
@@ -2274,7 +2288,10 @@ describe("api routes", () => {
         method: "PUT",
         headers: ownerHeaders,
         // #773/#774/#776: the agent-layer config is settable here; the DB layer drops an unknown action class.
-        body: JSON.stringify({ gateCheckMode: "enabled", slopGateMode: "block", slopGateMinScore: 55, autonomy: { merge: "auto_with_approval", deploy: "auto" }, autoMaintain: { requireApprovals: 2, mergeMethod: "rebase" }, agentPaused: true, agentDryRun: true }),
+        // #2267: qualityGateMode: "block" is downgraded to "advisory" on write — readiness/quality can never
+        // hard-block a PR, so the dashboard/API save path can't persist a value implying enforcement it doesn't
+        // have. slopGateMode: "block" is a DIFFERENT, legitimately-blockable dimension and is left untouched.
+        body: JSON.stringify({ gateCheckMode: "enabled", slopGateMode: "block", slopGateMinScore: 55, qualityGateMode: "block", autonomy: { merge: "auto_with_approval", deploy: "auto" }, autoMaintain: { requireApprovals: 2, mergeMethod: "rebase" }, agentPaused: true, agentDryRun: true }),
       },
       ownerEnv,
     );
@@ -2283,6 +2300,7 @@ describe("api routes", () => {
       gateCheckMode: "enabled",
       slopGateMode: "block",
       slopGateMinScore: 55,
+      qualityGateMode: "advisory", // #2267: downgraded, not persisted as "block"
       autonomy: { merge: "auto_with_approval" }, // unknown action class dropped by the DB normalizer
       autoMaintain: { requireApprovals: 2, mergeMethod: "rebase" },
       agentPaused: true, // #776 kill-switch

test/unit/focus-manifest.test.ts

@@ -1089,6 +1089,20 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () =
     expect(parseFocusManifest({ settings: { commentMode: "off" } }).present).toBe(true);
   });
 
+  it("downgrades settings.qualityGateMode: block to advisory with a deprecation warning, same as gate.readiness.mode (#2267)", () => {
+    // The generic settings: override is the SAME dashboard/API-facing qualityGateMode field, read through a
+    // different manifest path than gate.readiness.mode — it must get the identical downgrade, not just a
+    // "must be one of" pass-through, or a maintainer using this path keeps the false-enforcement belief.
+    const m = parseFocusManifest({ settings: { qualityGateMode: "block" } });
+    expect(m.settings.qualityGateMode).toBe("advisory");
+    expect(m.warnings.some((w) => /settings\.qualityGateMode.*no longer accepts "block"/.test(w))).toBe(true);
+    // Genuinely invalid values still take the ORIGINAL "must be one of" warning path, unchanged.
+    const bad = parseFocusManifest({ settings: { qualityGateMode: "sometimes" } });
+    expect(bad.settings.qualityGateMode).toBeUndefined();
+    expect(bad.warnings.some((w) => /settings\.qualityGateMode.*must be one of/.test(w))).toBe(true);
+    expect(bad.warnings.some((w) => /no longer accepts "block"/.test(w))).toBe(false);
+  });
+
   it("round-trips settings through settingsOverrideToJson and serializes empty as null", () => {
     const original = parseFocusManifest({ settings: { commentMode: "all_prs", qualityGateMinScore: 40 } });
     const reparsed = parseFocusManifest({ settings: settingsOverrideToJson(original.settings) });
@@ -1169,6 +1183,17 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () =
     );
     expect(eff.linkedIssueGateMode).toBe("block");
   });
+
+  it("REGRESSION: downgrades a pre-existing DB qualityGateMode: block to advisory, even with no gate.readiness.mode override (#2267)", () => {
+    // Simulates a repo whose DB row already has quality_gate_mode = "block" from before the write-time guards
+    // (the settings.qualityGateMode parser, the settings-write API routes) existed — the dashboard/API path's
+    // "still survives" loophole this resolver-level guard closes for good, regardless of source or vintage.
+    const db = { qualityGateMode: "block" } as unknown as RepositorySettings;
+    expect(resolveEffectiveSettings(db, parseFocusManifest(null)).qualityGateMode).toBe("advisory");
+    // A non-"block" value is untouched — the downgrade only ever fires for "block".
+    const dbAdvisory = { qualityGateMode: "advisory" } as unknown as RepositorySettings;
+    expect(resolveEffectiveSettings(dbAdvisory, parseFocusManifest(null)).qualityGateMode).toBe("advisory");
+  });
 });
 
 describe("parseFocusManifest review config", () => {