fix(review): default reviewEvasionProtection to close, not off (#4011)

src/db/repositories.ts

@@ -564,7 +564,7 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise
       moderationRules: undefined,
       moderationWarningLabel: undefined,
       moderationBannedLabel: undefined,
-      reviewEvasionProtection: "off",
+      reviewEvasionProtection: "close", // #4011: default-ON -- see normalizeReviewEvasionProtection's doc comment
       reviewEvasionLabel: DEFAULT_REVIEW_EVASION_LABEL,
       reviewEvasionComment: true,
       screenshotTableGate: { ...DEFAULT_SCREENSHOT_TABLE_GATE, whenLabels: [], whenPaths: [] },
@@ -7055,8 +7055,19 @@ function normalizeReviewNagPolicy(value: string | null | undefined): "off" | "ho
 // Review-evasion protection (#review-evasion-protection): binary off|close, mirroring reviewNagPolicy's
 // shape minus the "hold" tier (an evasion attempt is always re-closed as the App when enabled, never merely
 // held -- there is no partial-enforcement mode).
+//
+// #4011: default-ON, the deliberate exception to every other field in this file defaulting conservatively
+// (off/false/advisory). A repo that hasn't discovered and explicitly set this field got ZERO self-close/
+// draft-dodge/repeated-cycling protection under the old "off" default -- a real, already-exploited gaming
+// vector (see gittensory-ai-review-repeat-spend-and-draft-gaming-fix). Any value other than the explicit
+// opt-out "off" (including undefined/garbage) now resolves to "close": protected unless a repo deliberately
+// turns it off, not unprotected unless a repo discovers and turns it on. This is the ONLY reachable default
+// for this field -- the raw schema.ts column-level DEFAULT and the SQLite DDL default are never reached by
+// any live write path (upsertRepositorySettings always resolves and supplies an explicit value through this
+// exact function; see migration 0102's doc comment for the same lesson learned on a sibling field), so
+// changing them would have zero effect and was deliberately left alone.
 function normalizeReviewEvasionProtection(value: string | null | undefined): "off" | "close" {
-  return value === "close" ? "close" : "off";
+  return value === "off" ? "off" : "close";
 }
 
 // Config-driven before/after screenshot-table gate (#2006): the row stores whenLabels/whenPaths as JSON string

src/db/schema.ts

@@ -151,9 +151,17 @@ export const repositorySettings = sqliteTable("repository_settings", {
   moderationRulesJson: text("moderation_rules_json"),
   moderationWarningLabel: text("moderation_warning_label"),
   moderationBannedLabel: text("moderation_banned_label"),
-  // Review-evasion protection (#review-evasion-protection): off by default. reviewEvasionLabel mirrors
-  // blacklistLabel/reviewNagLabel's shape -- NOT NULL with a string default; "no label" is a
-  // `.gittensory.yml`-only override, never persisted here.
+  // Review-evasion protection (#review-evasion-protection). reviewEvasionLabel mirrors blacklistLabel/
+  // reviewNagLabel's shape -- NOT NULL with a string default; "no label" is a `.gittensory.yml`-only
+  // override, never persisted here.
+  //
+  // #4011: this raw column-level DEFAULT ('off') is intentionally left unchanged even though the actual
+  // resolved default is now 'close' (protection ON) -- upsertRepositorySettings is the ONLY writer and
+  // always resolves an explicit value through normalizeReviewEvasionProtection (its own doc comment has
+  // the full reasoning), so this raw default never fires through any live write path. Rebuilding it would
+  // need a full create-copy-drop-rename table migration for zero behavioral effect (SQLite has no ALTER
+  // COLUMN SET DEFAULT) -- see migration 0102's doc comment for the identical lesson already learned on
+  // linked_issue_gate_mode. Don't "fix" this value without re-reading that reasoning first.
   reviewEvasionProtection: text("review_evasion_protection").notNull().default("off"),
   reviewEvasionLabel: text("review_evasion_label").notNull().default("review-evasion"),
   reviewEvasionComment: integer("review_evasion_comment", { mode: "boolean" }).notNull().default(true),

src/queue/processors.ts

@@ -5854,7 +5854,7 @@ async function processGitHubWebhook(
       // Review-evasion protection (#review-evasion-protection): a contributor closing their OWN PR while
       // gittensory has an ACTIVE review pass running is dodging the one-shot review, not making an ordinary
       // close. Runs regardless of the general draft-dodge/reopen-reclose gates above -- it is its own
-      // independent enforcement, config-gated on settings.reviewEvasionProtection (off by default).
+      // independent enforcement, config-gated on settings.reviewEvasionProtection (close by default, #4011).
       if (payload.action === "closed" && installationId) {
         await maybeCloseReviewEvasionSelfClose(
           env,
@@ -11770,7 +11770,7 @@ async function closeReviewEvasionSelfCloseIfActive(
   payload: GitHubWebhookPayload,
   settings: RepositorySettings,
 ): Promise<void> {
-  if ((settings.reviewEvasionProtection ?? "off") !== "close") return;
+  if (settings.reviewEvasionProtection === "off") return; // #4011: default-ON -- only the explicit opt-out bails
   const closer = (payload.sender?.login ?? "").toLowerCase();
   const authorLogin = (pr.authorLogin ?? "").toLowerCase();
   // Only the PR's OWN author closing their OWN PR is a self-close-evasion candidate -- a third party (e.g. a
@@ -12016,7 +12016,7 @@ async function closeReviewEvasionDraftConversionIfActive(
   payload: GitHubWebhookPayload,
   settings: RepositorySettings,
 ): Promise<void> {
-  if ((settings.reviewEvasionProtection ?? "off") !== "close") return;
+  if (settings.reviewEvasionProtection === "off") return; // #4011: default-ON -- only the explicit opt-out bails
   const converter = (payload.sender?.login ?? "").toLowerCase();
   const authorLogin = (pr.authorLogin ?? "").toLowerCase();
   // Only the PR's OWN author converting their OWN PR to draft is a draft-conversion-evasion candidate -- a
@@ -12205,7 +12205,7 @@ async function maybeCloseRepeatedDraftCycling(
   // fires on the >=2nd author-driven conversion of a `reviewEvasionProtection: close` repo, which is rare -- an
   // unconditional lock claim on every converted_to_draft webhook would add avoidable contention with the two
   // siblings above on every repo that never enabled this feature, or on every first-time conversion.
-  if ((settings.reviewEvasionProtection ?? "off") !== "close") return;
+  if (settings.reviewEvasionProtection === "off") return; // #4011: default-ON -- only the explicit opt-out bails
   if (draftConversionCount < 2) return;
   const actuationLock = await claimPrActuationLock(env, repoFullName, pr.number);
   if (!actuationLock.acquired) {

test/unit/moderation-config-db.test.ts

@@ -237,9 +237,9 @@ describe("per-repo moderation settings DB round-trip (#selfhost-mod-engine)", ()
 });
 
 describe("per-repo review-evasion protection settings DB round-trip (#review-evasion-protection)", () => {
-  it("defaults to off/review-evasion/true for an unconfigured repo", async () => {
+  it("defaults to close/review-evasion/true for an unconfigured repo (#4011: default-ON)", async () => {
     const settings = await getRepositorySettings(createTestEnv(), "owner/none");
-    expect(settings.reviewEvasionProtection).toBe("off");
+    expect(settings.reviewEvasionProtection).toBe("close");
     expect(settings.reviewEvasionLabel).toBe("review-evasion");
     expect(settings.reviewEvasionComment).toBe(true);
   });
@@ -274,11 +274,19 @@ describe("per-repo review-evasion protection settings DB round-trip (#review-eva
     expect(settings.reviewEvasionComment).toBe(false);
   });
 
-  it("a malformed raw DB value for review_evasion_protection normalizes to 'off' on read (defensive against a direct SQL write bypassing app-level validation)", async () => {
+  it("a malformed raw DB value for review_evasion_protection normalizes to 'close' on read (#4011: only the explicit opt-out 'off' is honored; anything else, including a bypassed-validation bogus value, fails safe to protected)", async () => {
     const env = createTestEnv();
     await upsertRepositorySettings(env, { repoFullName: "owner/repo" });
     await env.DB.prepare("UPDATE repository_settings SET review_evasion_protection = 'bogus' WHERE repo_full_name = ?").bind("owner/repo").run();
     const settings = await getRepositorySettings(env, "owner/repo");
+    expect(settings.reviewEvasionProtection).toBe("close");
+  });
+
+  it("the explicit opt-out 'off' is honored on read even after a direct SQL write (#4011)", async () => {
+    const env = createTestEnv();
+    await upsertRepositorySettings(env, { repoFullName: "owner/repo" });
+    await env.DB.prepare("UPDATE repository_settings SET review_evasion_protection = 'off' WHERE repo_full_name = ?").bind("owner/repo").run();
+    const settings = await getRepositorySettings(env, "owner/repo");
     expect(settings.reviewEvasionProtection).toBe("off");
   });
 });

test/unit/queue.test.ts

@@ -25616,7 +25616,7 @@ describe("review-evasion protection (#review-evasion-protection)", () => {
       expect(audit?.n).toBe(0); // no decision recorded either way -- the queue retry owns the deferred decision
     });
 
-    it("does nothing when reviewEvasionProtection is off (the default)", async () => {
+    it("does nothing when reviewEvasionProtection is explicitly off (#4011: the only respected opt-out)", async () => {
       const calls: Array<{ url: string; method: string }> = [];
       stubEvasionFetch(calls);
       const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem(), GITHUB_APP_SLUG: "gittensory" });
@@ -25969,10 +25969,12 @@ describe("review-evasion protection (#review-evasion-protection)", () => {
       expect(reopenAudit?.detail).toContain("one-shot");
     });
 
-    it("does nothing when reviewEvasionProtection is unset (undefined, not just 'off')", async () => {
-      // upsertRepositorySettings coalesces undefined -> "off" at write time (mirrors reviewEvasionLabel/
-      // reviewEvasionComment's own write-time defaulting below), so the only way to get `undefined` past that
-      // coalescing and into the handler is to mock the resolved-settings layer directly.
+    it("STILL protects when reviewEvasionProtection is unset (undefined, not an explicit 'off') (#4011: default-ON)", async () => {
+      // upsertRepositorySettings coalesces undefined -> "close" at write time (mirrors reviewEvasionLabel/
+      // reviewEvasionComment's own write-time defaulting below), and the consuming handler's own fallback
+      // (settings.reviewEvasionProtection === "off") treats anything but an explicit "off" as protected too --
+      // so the only way to get `undefined` past BOTH layers and into the handler is to mock the resolved-
+      // settings layer directly, confirming neither layer silently reintroduces the old off-by-default gap.
       const calls: Array<{ url: string; method: string }> = [];
       stubEvasionFetch(calls);
       const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem(), GITHUB_APP_SLUG: "gittensory" });
@@ -25983,7 +25985,8 @@ describe("review-evasion protection (#review-evasion-protection)", () => {
 
       await processJob(env, { type: "github-webhook", deliveryId: "self-close-protection-unset", eventName: "pull_request", payload: closedPayload("contributor") });
 
-      expect(calls.some((c) => c.method === "PATCH" && c.url.endsWith("/pulls/42"))).toBe(false);
+      const patches = calls.filter((c) => c.method === "PATCH" && c.url.endsWith("/pulls/42"));
+      expect(patches.length).toBeGreaterThanOrEqual(2); // reopen then re-close, same as an explicit "close"
     });
 
     it("does nothing when the webhook payload has no sender", async () => {
@@ -26363,7 +26366,7 @@ describe("review-evasion protection (#review-evasion-protection)", () => {
       expect(strike?.n).toBe(0);
     });
 
-    it("does nothing when reviewEvasionProtection is unset (undefined, not just 'off')", async () => {
+    it("STILL protects when reviewEvasionProtection is unset (undefined, not an explicit 'off') (#4011: default-ON)", async () => {
       const calls: Array<{ url: string; method: string }> = [];
       stubEvasionFetch(calls);
       const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem(), GITHUB_APP_SLUG: "gittensory" });
@@ -26374,7 +26377,7 @@ describe("review-evasion protection (#review-evasion-protection)", () => {
 
       await processJob(env, { type: "github-webhook", deliveryId: "draft-evasion-protection-unset", eventName: "pull_request", payload: draftEvasionPayload("contributor") });
 
-      expect(calls.some((c) => c.method === "PATCH" && c.url.endsWith("/pulls/42"))).toBe(false);
+      expect(calls.some((c) => c.method === "PATCH" && c.url.endsWith("/pulls/42"))).toBe(true);
     });
 
     it("does nothing when the webhook payload has no sender", async () => {
@@ -26567,7 +26570,7 @@ describe("review-evasion protection (#review-evasion-protection)", () => {
       expect(calls.some((c) => c.method === "PATCH" && c.url.endsWith("/pulls/42"))).toBe(false);
     });
 
-    it("does nothing when reviewEvasionProtection is unset (undefined, not just 'off'), even after a repeated cycle", async () => {
+    it("STILL enforces the repeated-cycle close when reviewEvasionProtection is unset (undefined, not an explicit 'off') (#4011: default-ON)", async () => {
       const calls: Array<{ url: string; method: string }> = [];
       stubEvasionFetch(calls);
       const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem(), GITHUB_APP_SLUG: "gittensory" });
@@ -26576,9 +26579,12 @@ describe("review-evasion protection (#review-evasion-protection)", () => {
       vi.spyOn(repositorySettingsModule, "resolveRepositorySettings").mockResolvedValue({ ...baseSettings, reviewEvasionProtection: undefined });
 
       await processJob(env, { type: "github-webhook", deliveryId: "draft-cycle-unset-1", eventName: "pull_request", payload: draftEvasionPayload("contributor") });
+      expect(calls.some((c) => c.method === "PATCH" && c.url.endsWith("/pulls/42"))).toBe(false); // first conversion never closes
+
       await processJob(env, { type: "github-webhook", deliveryId: "draft-cycle-unset-2", eventName: "pull_request", payload: draftEvasionPayload("contributor") });
 
-      expect(calls.some((c) => c.method === "PATCH" && c.url.endsWith("/pulls/42"))).toBe(false);
+      const patches = calls.filter((c) => c.method === "PATCH" && c.url.endsWith("/pulls/42"));
+      expect(patches).toHaveLength(1); // second conversion closes, same as an explicit "close"
     });
 
     it("REGRESSION (gate-flagged): does not enforce against a THIRD PARTY repeatedly converting someone else's PR to draft", async () => {