fix(settings): cap review-nag cooldown (#2634)

* fix(settings): cap review nag cooldown

* fix(review): avoid a UI typecheck regression from a new agent-actions import

focus-manifest.ts is part of the UI package's typechecked closure (its
transitive deps get walked by apps/gittensory-ui's own tsc run). Importing
MAX_REVIEW_NAG_COOLDOWN_DAYS from settings/agent-actions.ts pulled that
module's own import of github/commands.ts -> utils/crypto.ts into the UI
build for the first time, exposing a pre-existing latent
Uint8Array<ArrayBufferLike>/BufferSource type mismatch in crypto.ts that
the UI's tsc had never previously reached.

Duplicates the small constant locally in focus-manifest.ts instead of
importing it, keeping db/repositories.ts's own import from agent-actions.ts
(never part of the UI closure) unaffected. Also rebases the branch's merge
commit into a linear rebase onto current main.

* fix(review): make the review-nag cooldown-cap regression test actually exercise the guard

Gate review: the original regression test seeded an oversized
reviewNagCooldownDays through upsertRepositorySettings, but both
upsertRepositorySettings and getRepositorySettings already clamp that field
on write AND read -- so the value read back inside resolveRepositorySettings
was never actually oversized by the time maybeThrottleReviewNagPing saw it.
The test could pass even with processors.ts's own
Math.min(reviewNagCooldownDays, MAX_REVIEW_NAG_COOLDOWN_DAYS) guard removed
entirely.

Mocks resolveRepositorySettings directly (bypassing the DB/yml clamp layers
entirely, not just the write-time one) to actually deliver an oversized
value to the function under test, then proves the guard's effect
behaviorally: three prior pings 400 days old fall outside the CORRECTLY
capped 365-day window (no cooldown applied), whereas an uncapped
"1-billion-day" window would count them and trip the threshold. Confirmed
by mutation-testing: removing the Math.min throws a real RangeError
(Invalid time value) building the Date.

Author: JSONbored

.gittensory.yml.example

@@ -325,7 +325,7 @@ settings:
   # a dedicated closeIssue primitive lands) with a clear reason. Off by default.
   # reviewNagPolicy: off      # off | hold | close. Default: off.
   # reviewNagMaxPings: 3      # Positive integer. Pings above this within the cooldown window trigger the policy. Default: 3.
-  # reviewNagCooldownDays: 5  # Positive integer. Window the ping count is measured over. Default: 5.
+  # reviewNagCooldownDays: 5  # Positive integer up to 365. Window the ping count is measured over. Default: 5.
   # reviewNagLabel: review-nag-cooldown  # Label applied alongside the hold/close action. Default: review-nag-cooldown.
 
   # Shared repo-scoped exemption list (#2463): GitHub logins never throttled/closed by gittensory's

apps/gittensory-ui/public/openapi.json

@@ -8618,7 +8618,8 @@
           "reviewNagCooldownDays": {
             "type": "integer",
             "minimum": 0,
-            "exclusiveMinimum": true
+            "exclusiveMinimum": true,
+            "maximum": 365
           },
           "reviewNagLabel": {
             "type": "string"

src/db/repositories.ts

@@ -57,6 +57,7 @@ import {
   upstreamSourceSnapshots,
   webhookEvents,
 } from "./schema";
+import { MAX_REVIEW_NAG_COOLDOWN_DAYS } from "../settings/agent-actions";
 import type {
   Advisory,
   AdvisoryFinding,
@@ -562,7 +563,7 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise
     contributorCapLabel: row.contributorCapLabel,
     reviewNagPolicy: normalizeReviewNagPolicy(row.reviewNagPolicy),
     reviewNagMaxPings: normalizePositiveIntWithDefault(row.reviewNagMaxPings, 3),
-    reviewNagCooldownDays: normalizePositiveIntWithDefault(row.reviewNagCooldownDays, 5),
+    reviewNagCooldownDays: normalizeReviewNagCooldownDays(row.reviewNagCooldownDays, 5),
     reviewNagLabel: row.reviewNagLabel,
     autoCloseExemptLogins: parseAutoCloseExemptLogins(row.autoCloseExemptLoginsJson),
     requireFreshRebaseWindowMinutes: normalizeOpenItemCap(row.requireFreshRebaseWindowMinutes),
@@ -649,7 +650,7 @@ export async function upsertRepositorySettings(env: Env, settings: Partial<Repos
     contributorCapLabel: settings.contributorCapLabel ?? "over-contributor-limit",
     reviewNagPolicy: normalizeReviewNagPolicy(settings.reviewNagPolicy),
     reviewNagMaxPings: normalizePositiveIntWithDefault(settings.reviewNagMaxPings, 3),
-    reviewNagCooldownDays: normalizePositiveIntWithDefault(settings.reviewNagCooldownDays, 5),
+    reviewNagCooldownDays: normalizeReviewNagCooldownDays(settings.reviewNagCooldownDays, 5),
     reviewNagLabel: settings.reviewNagLabel ?? "review-nag-cooldown",
     autoCloseExemptLogins: normalizeAutoCloseExemptLogins(settings.autoCloseExemptLogins).logins,
     requireFreshRebaseWindowMinutes: normalizeOpenItemCap(settings.requireFreshRebaseWindowMinutes),
@@ -5798,6 +5799,11 @@ function normalizePositiveIntWithDefault(value: number | null | undefined, fallb
   return value;
 }
 
+function normalizeReviewNagCooldownDays(value: number | null | undefined, fallback: number): number {
+  const normalized = normalizePositiveIntWithDefault(value, fallback);
+  return Math.min(normalized, MAX_REVIEW_NAG_COOLDOWN_DAYS);
+}
+
 function parseAutonomyPolicy(value: string): AutonomyPolicy {
   return normalizeAutonomyPolicy(parseJson<unknown>(value, null));
 }

src/openapi/schemas.ts

@@ -1,4 +1,5 @@
 import { z } from "zod";
+import { MAX_REVIEW_NAG_COOLDOWN_DAYS } from "../settings/agent-actions";
 import { extendZodWithOpenApi } from "@asteasolutions/zod-to-openapi";
 
 extendZodWithOpenApi(z);
@@ -647,7 +648,7 @@ export const RepositorySettingsSchema = z
     contributorCapLabel: z.string().optional(),
     reviewNagPolicy: z.enum(["off", "hold", "close"]).optional(),
     reviewNagMaxPings: z.number().int().positive().optional(),
-    reviewNagCooldownDays: z.number().int().positive().optional(),
+    reviewNagCooldownDays: z.number().int().positive().max(MAX_REVIEW_NAG_COOLDOWN_DAYS).optional(),
     reviewNagLabel: z.string().optional(),
     autoCloseExemptLogins: z.array(z.string()).optional(),
     accountAgeThresholdDays: z.number().int().positive().nullable().optional(),

src/queue/processors.ts

@@ -237,6 +237,7 @@ import { aiReviewCacheInputFingerprint } from "../review/ai-review-cache-input";
 import {
   downgradeCloseToHold,
   downgradeMergeToHold,
+  MAX_REVIEW_NAG_COOLDOWN_DAYS,
   isProtectedAutomationAuthor,
   planAgentMaintenanceActions,
   type PlannedAgentAction,
@@ -8858,7 +8859,7 @@ async function maybeThrottleReviewNagPing(
   /* v8 ignore next -- resolveRepositorySettings always resolves a concrete positive integer (NOT NULL DEFAULT 3); the undefined side is defensive against the field's optional TS type. */
   const maxPings = settings.reviewNagMaxPings ?? 3;
   /* v8 ignore next -- resolveRepositorySettings always resolves a concrete positive integer (NOT NULL DEFAULT 5); the undefined side is defensive against the field's optional TS type. */
-  const cooldownDays = settings.reviewNagCooldownDays ?? 5;
+  const cooldownDays = Math.min(settings.reviewNagCooldownDays ?? 5, MAX_REVIEW_NAG_COOLDOWN_DAYS);
   const sinceIso = new Date(Date.now() - cooldownDays * 24 * 60 * 60 * 1000).toISOString();
   const priorPings = await countRecentAuditEventsForActorAndTarget(env, commenter, REVIEW_NAG_PING_EVENT_TYPE, targetKey, sinceIso);
   const pingCount = priorPings + 1; // this ping counts too

src/settings/agent-actions.ts

@@ -31,6 +31,8 @@ export const DEFAULT_CONTRIBUTOR_CAP_LABEL = "over-contributor-limit";
 // configurable per-repo via `.gittensory.yml` (`settings.reviewNagLabel`); the planner uses the resolved label
 // and falls back to this default, mirroring DEFAULT_BLACKLIST_LABEL's shape.
 export const DEFAULT_REVIEW_NAG_LABEL = "review-nag-cooldown";
+// Keep the review-nag lookback operationally bounded so repo-controlled config cannot overflow Date arithmetic.
+export const MAX_REVIEW_NAG_COOLDOWN_DAYS = 365;
 // A PR that PASSES the gate but touches a hard-guardrail path is NOT ready to auto-merge — it is withheld
 // for a human (the merge/approve/close dispositions are suppressed below). Labeling it `ready-to-merge`
 // would be misleading (the label promises an auto-merge that never happens), so a guarded passing PR gets

src/signals/focus-manifest.ts

@@ -741,6 +741,13 @@ function normalizeOptionalString(value: JsonValue | undefined, field: string, wa
   return null;
 }
 
+// Keep the review-nag lookback operationally bounded so repo-controlled config cannot overflow Date
+// arithmetic. Duplicated from settings/agent-actions.ts's own MAX_REVIEW_NAG_COOLDOWN_DAYS (same value,
+// same rationale) rather than imported: this module is part of the UI package's typechecked closure, and
+// agent-actions.ts transitively imports github/commands.ts -> utils/crypto.ts, pulling a heavier
+// GitHub-App-specific dependency chain into the UI build for one small constant.
+const MAX_REVIEW_NAG_COOLDOWN_DAYS = 365;
+
 /**
  * Parse the optional `settings:` mapping — a partial repository-settings override. Only recognized
  * fields are kept; unknown/invalid values are dropped with a warning and never throw.
@@ -859,7 +866,10 @@ function parseSettingsOverride(value: JsonValue | undefined, warnings: string[])
   const reviewNagMaxPings = normalizeOptionalPositiveInteger(r.reviewNagMaxPings, "settings.reviewNagMaxPings", warnings);
   if (reviewNagMaxPings !== null) out.reviewNagMaxPings = reviewNagMaxPings;
   const reviewNagCooldownDays = normalizeOptionalPositiveInteger(r.reviewNagCooldownDays, "settings.reviewNagCooldownDays", warnings);
-  if (reviewNagCooldownDays !== null) out.reviewNagCooldownDays = reviewNagCooldownDays;
+  if (reviewNagCooldownDays !== null && reviewNagCooldownDays <= MAX_REVIEW_NAG_COOLDOWN_DAYS) out.reviewNagCooldownDays = reviewNagCooldownDays;
+  if (reviewNagCooldownDays !== null && reviewNagCooldownDays > MAX_REVIEW_NAG_COOLDOWN_DAYS) {
+    warnings.push(`Manifest field "settings.reviewNagCooldownDays" must be at most ${MAX_REVIEW_NAG_COOLDOWN_DAYS}; ignoring it.`);
+  }
   const reviewNagLabel = normalizeOptionalString(r.reviewNagLabel, "settings.reviewNagLabel", warnings);
   if (reviewNagLabel !== null) out.reviewNagLabel = reviewNagLabel;
   // Shared repo-scoped exemption list (#2463): only set it when at least one VALID login survives

test/unit/data-spine.test.ts

@@ -366,6 +366,10 @@ describe("data spine repositories", () => {
     // back to its default rather than being silently coerced.
     await upsertRepositorySettings(env, { repoFullName: "owner/badnagrepo", reviewNagPolicy: "delete-everything" as never, reviewNagMaxPings: -1, reviewNagCooldownDays: 2.5 as never });
     expect(await getRepositorySettings(env, "owner/badnagrepo")).toMatchObject({ reviewNagPolicy: "off", reviewNagMaxPings: 3, reviewNagCooldownDays: 5 });
+    await upsertRepositorySettings(env, { repoFullName: "owner/bigwindowrepo", reviewNagMaxPings: 1_000, reviewNagCooldownDays: 1_000_000_000 });
+    expect(await getRepositorySettings(env, "owner/bigwindowrepo")).toMatchObject({ reviewNagMaxPings: 1_000, reviewNagCooldownDays: 365 });
+    await env.DB.prepare("update repository_settings set review_nag_cooldown_days = ? where repo_full_name = ?").bind(1_000_000_000, "owner/bigwindowrepo").run();
+    expect(await getRepositorySettings(env, "owner/bigwindowrepo")).toMatchObject({ reviewNagMaxPings: 1_000, reviewNagCooldownDays: 365 });
     expect(updated.slopAiAdvisory).toBe(false);
     expect(await getRepoSyncState(env, "missing/repo")).toBeNull();
     expect(await getPullRequest(env, "owner/repo", 404)).toBeNull();

test/unit/focus-manifest.test.ts

@@ -1382,6 +1382,9 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () =
     expect(invalid.warnings.some((w) => /settings\.reviewNagPolicy/.test(w))).toBe(true);
     expect(invalid.warnings.some((w) => /settings\.reviewNagMaxPings/.test(w))).toBe(true);
     expect(invalid.warnings.some((w) => /settings\.reviewNagCooldownDays/.test(w))).toBe(true);
+    const tooLarge = parseFocusManifest({ settings: { reviewNagCooldownDays: 366 } });
+    expect(tooLarge.settings.reviewNagCooldownDays).toBeUndefined();
+    expect(tooLarge.warnings.some((w) => /settings\.reviewNagCooldownDays/.test(w) && /365/.test(w))).toBe(true);
   });
 
   it("parses + resolves the account-age throttle settings from the settings: block, overlaying the DB (#2561)", () => {

test/unit/queue.test.ts

@@ -12243,6 +12243,54 @@ describe("queue processors", () => {
       expect(seen.closed).toBe(false);
     });
 
+    it("REGRESSION (gate-flagged): caps an oversized review-nag cooldown at MAX_REVIEW_NAG_COOLDOWN_DAYS before Date arithmetic, even when the resolved settings object itself carries an oversized value", async () => {
+      // upsertRepositorySettings/getRepositorySettings both clamp reviewNagCooldownDays on write AND read, so
+      // seeding an oversized value through the normal repository layer (even via a raw DB update bypassing the
+      // write-time clamp) can never actually reach maybeThrottleReviewNagPing uncapped -- the read-time clamp in
+      // getRepositorySettings neutralizes it first. Mock resolveRepositorySettings directly so this test proves
+      // processors.ts's OWN Math.min(reviewNagCooldownDays, MAX_REVIEW_NAG_COOLDOWN_DAYS) guard, not the DB layer.
+      const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
+      await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", reviewNagPolicy: "hold", reviewNagMaxPings: 3 });
+      await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 206, title: "Huge cooldown", state: "open", user: { login: "chatty" }, author_association: "NONE", labels: [], body: "" });
+      // Three prior pings, all 400 DAYS ago -- outside the 365-day cap, but well within an uncapped
+      // "1,000,000,000-day" window. If the guard clamps correctly, these fall outside the window and don't
+      // count; if the guard were removed, the uncapped window would count all three, crossing maxPings=3.
+      vi.setSystemTime(new Date("2025-04-24T00:00:00.000Z"));
+      for (let i = 0; i < 3; i += 1) {
+        await repositoriesModule.recordAuditEvent(env, { eventType: "github_app.review_nag_ping", actor: "chatty", targetKey: "JSONbored/gittensory#206", outcome: "completed" });
+      }
+      vi.setSystemTime(new Date("2026-05-29T00:00:00.000Z")); // ~400 days later
+      const baseSettings = await repositorySettingsModule.resolveRepositorySettings(env, "JSONbored/gittensory");
+      const resolveSettingsSpy = vi
+        .spyOn(repositorySettingsModule, "resolveRepositorySettings")
+        .mockResolvedValueOnce({ ...baseSettings, reviewNagCooldownDays: 1_000_000_000 });
+      const seen = { comments: [] as string[], labels: [] as string[], closed: false };
+      stubReviewNagFetch(206, seen);
+
+      await processJob(env, {
+        type: "github-webhook",
+        deliveryId: "nag-huge-cooldown",
+        eventName: "issue_comment",
+        payload: {
+          action: "created",
+          installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } },
+          repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } },
+          issue: { number: 206, title: "Huge cooldown", state: "open", pull_request: {}, user: { login: "chatty" }, author_association: "NONE" },
+          comment: { id: 1, body: "@gittensory help", user: { login: "chatty", type: "User" }, author_association: "NONE" },
+        },
+      });
+
+      // The 400-day-old pings fell outside the CAPPED 365-day window, so this is only the 1st ping this
+      // window — under maxPings=3, never throttled. An uncapped window would have counted all 3 prior pings
+      // (pingCount=4 > maxPings=3) and applied the cooldown instead.
+      const applied = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.review_nag_cooldown_applied'").first<{ n: number }>();
+      expect(applied?.n).toBe(0);
+      expect(seen.closed).toBe(false);
+      expect(seen.comments.some((c) => c.includes("cooldown limit"))).toBe(false);
+      expect(resolveSettingsSpy).toHaveBeenCalled();
+      resolveSettingsSpy.mockRestore();
+    });
+
     it("records pings under the configured threshold without acting; the normal @gittensory reply still proceeds", async () => {
       const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
       await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", reviewNagPolicy: "close", reviewNagMaxPings: 3 });