fix(review): scope review-nag ping cooldown to the actor, not one PR

src/db/repositories.ts

@@ -2588,6 +2588,77 @@ export async function countRecentAuditEventsForActorAndTarget(env: Env, actor: s
   return row.count;
 }
 
+/** Shared by every `targetKey` literal-prefix `LIKE` scan below ({@link countRecentAuditEventsForActorInRepo},
+ *  {@link findHottestReviewTargetForRepo}) so a repo name containing a SQL `LIKE` wildcard (`%`/`_`) is always
+ *  matched literally, never as a pattern -- e.g. `owner/foo_bar` must never spuriously match `owner/fooXbar#...`'s
+ *  targets. */
+function escapeSqlLikePattern(value: string): string {
+  return value.replace(/[\\%_]/g, "\\$&");
+}
+
+/**
+ * Repo-scoped sibling of {@link countRecentAuditEventsForActorAndTarget} (#review-nag-cross-pr-carryover): counts
+ * one actor's matching events across EVERY target within `repoFullName` (their current PR/issue plus every other
+ * one they've touched), not just the single `targetKey` the caller happens to be evaluating. The per-target count
+ * lets a contributor who exhausts a cooldown on PR A reset to a clean slate simply by opening a fresh PR B (a new
+ * `issue.number` is a new `targetKey`) -- this is the fix: the running count now follows the ACTOR through the
+ * repo, mirroring how the contributor blacklist and moderation-rules ban tally already persist by login rather
+ * than by thread. Reuses the same literal-prefix `LIKE ... ESCAPE` scoping as {@link findHottestReviewTargetForRepo}
+ * so `owner/foo_bar` can never spuriously match `owner/fooXbar#...`'s targets.
+ */
+export async function countRecentAuditEventsForActorInRepo(env: Env, actor: string, eventType: string, repoFullName: string, sinceIso: string): Promise<number> {
+  const db = getDb(env.DB);
+  const targetPrefixPattern = `${escapeSqlLikePattern(repoFullName)}#%`;
+  const [row] = await db
+    .select({ count: sql<number>`count(*)` })
+    .from(auditEvents)
+    .where(
+      and(
+        eq(auditEvents.actor, actor),
+        eq(auditEvents.eventType, eventType),
+        sql`${auditEvents.targetKey} LIKE ${targetPrefixPattern} ESCAPE '\\'`,
+        gte(auditEvents.createdAt, sinceIso),
+      ),
+    );
+  /* v8 ignore next -- count(*) always returns exactly one row; the empty-array guard only satisfies the destructure type. */
+  if (!row) return 0;
+  return row.count;
+}
+
+/**
+ * Variant of {@link countRecentAuditEventsForActorInRepo} for a `targetKey` shape that carries a THIRD segment
+ * after `repo#issueNumber` (e.g. maybeThrottleMonitoredMentions's `owner/repo#123#mention:someLogin`): scopes
+ * across every PR/issue NUMBER in the repo (the same repo-wide carryover fix) while still pinning to one EXACT
+ * `targetKeySuffix`, so independently-budgeted sub-targets (one per monitored login) never bleed into each
+ * other's count. Pass the suffix literally, e.g. `mention:someLogin` -- both `repoFullName` and `targetKeySuffix`
+ * are escaped before embedding, so neither can smuggle in a stray SQL `LIKE` wildcard.
+ */
+export async function countRecentAuditEventsForActorInRepoWithTargetSuffix(
+  env: Env,
+  actor: string,
+  eventType: string,
+  repoFullName: string,
+  targetKeySuffix: string,
+  sinceIso: string,
+): Promise<number> {
+  const db = getDb(env.DB);
+  const targetPattern = `${escapeSqlLikePattern(repoFullName)}#%#${escapeSqlLikePattern(targetKeySuffix)}`;
+  const [row] = await db
+    .select({ count: sql<number>`count(*)` })
+    .from(auditEvents)
+    .where(
+      and(
+        eq(auditEvents.actor, actor),
+        eq(auditEvents.eventType, eventType),
+        sql`${auditEvents.targetKey} LIKE ${targetPattern} ESCAPE '\\'`,
+        gte(auditEvents.createdAt, sinceIso),
+      ),
+    );
+  /* v8 ignore next -- count(*) always returns exactly one row; the empty-array guard only satisfies the destructure type. */
+  if (!row) return 0;
+  return row.count;
+}
+
 /** #orb-ci-stuck-repeat / #orb-retry-storm ops-alerts signal: the single PR within `repoFullName` that published
  *  the most review surfaces in the last `sinceIso`-bounded window, and how many. `github_app.pr_public_surface_
  *  published` is a genuine INSERT-only event (never upserted) recorded once per successful publish pass
@@ -2596,10 +2667,6 @@ export async function countRecentAuditEventsForActorAndTarget(env: Env, actor: s
  *  overwrites rather than accumulates), this correctly counts repeat publishes even when the head SHA never
  *  changes -- exactly the shape of a stuck-CI or sweep retry-storm bleed. Returns null when the repo published
  *  no surfaces in the window at all. */
-function escapeSqlLikePattern(value: string): string {
-  return value.replace(/[\\%_]/g, "\\$&");
-}
-
 export async function findHottestReviewTargetForRepo(
   env: Env,
   repoFullName: string,

src/queue/processors.ts

@@ -57,6 +57,8 @@ import {
   recordAgentCommandFeedback,
   recordAuditEvent,
   countRecentAuditEventsForActorAndTarget,
+  countRecentAuditEventsForActorInRepo,
+  countRecentAuditEventsForActorInRepoWithTargetSuffix,
   hasAuditEventForDelivery,
   recordGateBlockOutcome,
   getGateBlockOutcome,
@@ -12236,10 +12238,18 @@ const REVIEW_NAG_PING_EVENT_TYPE = "github_app.review_nag_ping";
 
 /**
  * Review-request nagging cooldown (#2463, anti-abuse): throttle a thread's OWN author repeatedly pinging
- * @gittensory for review on the SAME PR/issue. Runs BEFORE maybeProcessGittensoryMentionCommand below so a
- * throttled ping short-circuits ahead of the normal answer-card dispatch — under the threshold this just
- * records the ping (via the shared audit-events ledger, scoped by targetKey so the count never mixes threads)
- * and falls through unchanged; only crossing the threshold applies the repo's configured policy.
+ * @gittensory for review. Runs BEFORE maybeProcessGittensoryMentionCommand below so a throttled ping
+ * short-circuits ahead of the normal answer-card dispatch — under the threshold this just records the ping
+ * (still tagged with the THIS thread's own targetKey, so a per-thread audit trail is preserved) and falls
+ * through unchanged; only crossing the threshold applies the repo's configured policy.
+ *
+ * The running count is scoped to the ACTOR across the WHOLE repo (#review-nag-cross-pr-carryover), not to one
+ * `targetKey` — a contributor who exhausts their pings on PR A and opens a fresh PR B carries the count over
+ * instead of resetting to a clean 0/maxPings slate, mirroring how the contributor blacklist and moderation-rules
+ * ban tally already persist by login rather than by thread. This also makes enforcement immediate rather than
+ * merely cumulative: because the count already reflects every prior target, the very FIRST ping on PR B can
+ * already cross `maxPings` on its own — there's no need for a separate "still on cooldown" table, since the
+ * audit-events ledger read at the new repo scope already IS that persistent per-actor state.
  *
  * Deliberately scoped to the THREAD'S OWN author (`issue.user.login === commenter`): a third party pinging on
  * someone else's PR/issue must never throttle or close the AUTHOR's unrelated work — this mirrors the standing
@@ -12289,7 +12299,10 @@ async function maybeThrottleReviewNagPing(
   /* 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 = 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);
+  // Repo-wide, not per-target (#review-nag-cross-pr-carryover): counts every @gittensory ping this actor has
+  // sent anywhere in this repo within the window, so exhausting the budget on PR A already shows up on PR B's
+  // very first ping instead of restarting at 0/maxPings just because the targetKey (issue.number) is new.
+  const priorPings = await countRecentAuditEventsForActorInRepo(env, commenter, REVIEW_NAG_PING_EVENT_TYPE, repoFullName, sinceIso);
   const pingCount = priorPings + 1; // this ping counts too
 
   // Always record the ping first so the running count reflects reality even when the rest of this handler
@@ -12424,6 +12437,14 @@ function bodyMentionsLogin(body: string, login: string): boolean {
  * don't share one budget. Runs regardless of whether the comment also contains an `@gittensory` mention/command
  * — mentioning a maintainer is never a bot command, so this must not gate or interact with command dispatch.
  * Off (`reviewNagMonitoredMentions` empty/absent, the default) is a complete no-op — no extra reads at all.
+ *
+ * Like {@link maybeThrottleReviewNagPing}, the running count is scoped to the ACTOR across the WHOLE repo
+ * (#review-nag-cross-pr-carryover) rather than to one `targetKey`, so exhausting the budget mentioning @maintainer
+ * on PR A carries over to PR B instead of resetting. Because a mentioned login's own budget must stay independent
+ * of every OTHER monitored login's budget (the "don't share one budget" design above), the repo-wide count is
+ * additionally pinned to this one login's `mention:<login>` targetKey suffix via
+ * {@link countRecentAuditEventsForActorInRepoWithTargetSuffix} — carryover happens across PRs, never across
+ * different mentioned logins.
  */
 async function maybeThrottleMonitoredMentions(
   env: Env,
@@ -12457,13 +12478,25 @@ async function maybeThrottleMonitoredMentions(
   const mentionedLogin = monitoredLogins.find((login) => bodyMentionsLogin(body, login));
   if (!mentionedLogin) return false;
 
-  const targetKey = `${repoFullName}#${issue.number}#mention:${mentionedLogin.toLowerCase()}`;
+  // The per-login suffix is shared between the full targetKey (below, for the recordAuditEvent audit trail) and
+  // the repo-wide count's suffix filter, so a naming drift between the two can never silently under/over-count.
+  const mentionTargetSuffix = `mention:${mentionedLogin.toLowerCase()}`;
+  const targetKey = `${repoFullName}#${issue.number}#${mentionTargetSuffix}`;
   /* 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 = 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, MONITORED_MENTION_PING_EVENT_TYPE, targetKey, sinceIso);
+  // Repo-wide, not per-target (#review-nag-cross-pr-carryover), but still pinned to THIS mentioned login's own
+  // suffix so independently-budgeted mentioned logins never bleed into each other's count.
+  const priorPings = await countRecentAuditEventsForActorInRepoWithTargetSuffix(
+    env,
+    commenter,
+    MONITORED_MENTION_PING_EVENT_TYPE,
+    repoFullName,
+    mentionTargetSuffix,
+    sinceIso,
+  );
   const pingCount = priorPings + 1;
 
   await recordAuditEvent(env, {

test/unit/db-parsers.test.ts

@@ -4,6 +4,8 @@ import {
   countRecentDeadLetters,
   countRecentDeadLettersByType,
   countRecentAuditEventsForActorAndTarget,
+  countRecentAuditEventsForActorInRepo,
+  countRecentAuditEventsForActorInRepoWithTargetSuffix,
   findHottestReviewTargetForRepo,
   hasAuditEventForDelivery,
   getLatestScorePreview,
@@ -581,6 +583,67 @@ describe("database row parser hardening", () => {
     expect(await countRecentAuditEventsForActorAndTarget(env, "chatty", "github_app.review_nag_ping", "owner/repo#1", "2026-06-24T13:00:00.000Z")).toBe(0); // none after the cutoff → count(*) returns 0
   });
 
+  it("countRecentAuditEventsForActorInRepo counts one actor's events across EVERY target within a repo, not just one targetKey (#review-nag-cross-pr-carryover)", async () => {
+    const env = createTestEnv();
+    // Same actor, TWO different targets (PR #1 and PR #2) within the same repo -- both must count toward the total.
+    await recordAuditEvent(env, { eventType: "github_app.review_nag_ping", actor: "chatty", targetKey: "owner/repo#1", outcome: "completed", createdAt: "2026-06-24T10:00:00.000Z" });
+    await recordAuditEvent(env, { eventType: "github_app.review_nag_ping", actor: "chatty", targetKey: "owner/repo#1", outcome: "completed", createdAt: "2026-06-24T10:05:00.000Z" });
+    await recordAuditEvent(env, { eventType: "github_app.review_nag_ping", actor: "chatty", targetKey: "owner/repo#2", outcome: "completed", createdAt: "2026-06-24T12:00:00.000Z" });
+    // A different actor in the SAME repo must not be counted (the actor filter).
+    await recordAuditEvent(env, { eventType: "github_app.review_nag_ping", actor: "someone-else", targetKey: "owner/repo#3", outcome: "completed", createdAt: "2026-06-24T12:00:00.000Z" });
+    // The SAME actor pinging a DIFFERENT repo must not be counted (the repo-prefix scope).
+    await recordAuditEvent(env, { eventType: "github_app.review_nag_ping", actor: "chatty", targetKey: "owner/other-repo#1", outcome: "completed", createdAt: "2026-06-24T12:00:00.000Z" });
+    // An unrelated event type on the same actor+repo must not be counted (the eventType filter).
+    await recordAuditEvent(env, { eventType: "github_app.agent_command_replied", actor: "chatty", targetKey: "owner/repo#1", outcome: "completed", createdAt: "2026-06-24T12:00:00.000Z" });
+
+    expect(await countRecentAuditEventsForActorInRepo(env, "chatty", "github_app.review_nag_ping", "owner/repo", "2026-06-24T09:00:00.000Z")).toBe(3);
+    expect(await countRecentAuditEventsForActorInRepo(env, "chatty", "github_app.review_nag_ping", "owner/repo", "2026-06-24T11:00:00.000Z")).toBe(1); // only the 12:00 owner/repo#2 ping
+    expect(await countRecentAuditEventsForActorInRepo(env, "chatty", "github_app.review_nag_ping", "owner/repo", "2026-06-24T13:00:00.000Z")).toBe(0); // none after the cutoff → count(*) returns 0
+  });
+
+  it("countRecentAuditEventsForActorInRepo treats repo names as literal LIKE prefixes (regression mirroring findHottestReviewTargetForRepo's #review-burst-scope-pollution fix)", async () => {
+    const env = createTestEnv();
+    await recordAuditEvent(env, { eventType: "github_app.review_nag_ping", actor: "chatty", targetKey: "owner/foo_bar#1", outcome: "completed", createdAt: "2026-06-24T10:00:00.000Z" });
+    await recordAuditEvent(env, { eventType: "github_app.review_nag_ping", actor: "chatty", targetKey: "owner/foo_bar#2", outcome: "completed", createdAt: "2026-06-24T10:05:00.000Z" });
+    // owner/fooXbar is a DIFFERENT repo that would spuriously match "owner/foo_bar#%" if `_` were left as a SQL
+    // wildcard instead of being escaped -- must not leak into owner/foo_bar's count.
+    await recordAuditEvent(env, { eventType: "github_app.review_nag_ping", actor: "chatty", targetKey: "owner/fooXbar#99", outcome: "completed", createdAt: "2026-06-24T10:00:00.000Z" });
+    await recordAuditEvent(env, { eventType: "github_app.review_nag_ping", actor: "chatty", targetKey: "owner/fooXbar#99", outcome: "completed", createdAt: "2026-06-24T10:05:00.000Z" });
+    await recordAuditEvent(env, { eventType: "github_app.review_nag_ping", actor: "chatty", targetKey: "owner/fooXbar#99", outcome: "completed", createdAt: "2026-06-24T10:10:00.000Z" });
+
+    expect(await countRecentAuditEventsForActorInRepo(env, "chatty", "github_app.review_nag_ping", "owner/foo_bar", "2026-06-24T09:00:00.000Z")).toBe(2);
+  });
+
+  it("countRecentAuditEventsForActorInRepoWithTargetSuffix counts across every PR/issue number in a repo while still pinning to ONE exact targetKey suffix (#review-nag-cross-pr-carryover)", async () => {
+    const env = createTestEnv();
+    // Two different PR numbers within the SAME repo, both suffixed "#mention:jsonbored" -- both count.
+    await recordAuditEvent(env, { eventType: "github_app.monitored_mention_ping", actor: "chatty", targetKey: "owner/repo#1#mention:jsonbored", outcome: "completed", createdAt: "2026-06-24T10:00:00.000Z" });
+    await recordAuditEvent(env, { eventType: "github_app.monitored_mention_ping", actor: "chatty", targetKey: "owner/repo#2#mention:jsonbored", outcome: "completed", createdAt: "2026-06-24T10:05:00.000Z" });
+    // A DIFFERENT mentioned login's suffix on the SAME repo+actor must NOT bleed into the "jsonbored" count --
+    // this is the independent-budget guarantee the plain repo-prefix countRecentAuditEventsForActorInRepo can't
+    // provide on its own.
+    await recordAuditEvent(env, { eventType: "github_app.monitored_mention_ping", actor: "chatty", targetKey: "owner/repo#3#mention:other-maintainer", outcome: "completed", createdAt: "2026-06-24T10:07:00.000Z" });
+    // A different actor with the SAME suffix must not be counted (the actor filter).
+    await recordAuditEvent(env, { eventType: "github_app.monitored_mention_ping", actor: "someone-else", targetKey: "owner/repo#4#mention:jsonbored", outcome: "completed", createdAt: "2026-06-24T10:08:00.000Z" });
+    // A different repo with the SAME suffix must not be counted (the repo-prefix scope).
+    await recordAuditEvent(env, { eventType: "github_app.monitored_mention_ping", actor: "chatty", targetKey: "owner/other-repo#1#mention:jsonbored", outcome: "completed", createdAt: "2026-06-24T10:09:00.000Z" });
+
+    expect(await countRecentAuditEventsForActorInRepoWithTargetSuffix(env, "chatty", "github_app.monitored_mention_ping", "owner/repo", "mention:jsonbored", "2026-06-24T09:00:00.000Z")).toBe(2);
+    expect(await countRecentAuditEventsForActorInRepoWithTargetSuffix(env, "chatty", "github_app.monitored_mention_ping", "owner/repo", "mention:other-maintainer", "2026-06-24T09:00:00.000Z")).toBe(1);
+    expect(await countRecentAuditEventsForActorInRepoWithTargetSuffix(env, "chatty", "github_app.monitored_mention_ping", "owner/repo", "mention:jsonbored", "2026-06-24T10:06:00.000Z")).toBe(0); // cutoff after both matching pings
+  });
+
+  it("countRecentAuditEventsForActorInRepoWithTargetSuffix escapes BOTH the repo prefix and the suffix before embedding them in the LIKE pattern (regression mirroring countRecentAuditEventsForActorInRepo's escaping fix)", async () => {
+    const env = createTestEnv();
+    await recordAuditEvent(env, { eventType: "github_app.monitored_mention_ping", actor: "chatty", targetKey: "owner/foo_bar#1#mention:some_login", outcome: "completed", createdAt: "2026-06-24T10:00:00.000Z" });
+    // Neither a repo-name collision (fooXbar vs foo_bar) NOR a suffix collision (someXlogin vs some_login) may
+    // leak in if `_` were left as an unescaped SQL wildcard in either segment.
+    await recordAuditEvent(env, { eventType: "github_app.monitored_mention_ping", actor: "chatty", targetKey: "owner/fooXbar#2#mention:some_login", outcome: "completed", createdAt: "2026-06-24T10:01:00.000Z" });
+    await recordAuditEvent(env, { eventType: "github_app.monitored_mention_ping", actor: "chatty", targetKey: "owner/foo_bar#3#mention:someXlogin", outcome: "completed", createdAt: "2026-06-24T10:02:00.000Z" });
+
+    expect(await countRecentAuditEventsForActorInRepoWithTargetSuffix(env, "chatty", "github_app.monitored_mention_ping", "owner/foo_bar", "mention:some_login", "2026-06-24T09:00:00.000Z")).toBe(1);
+  });
+
   it("findHottestReviewTargetForRepo returns the PR with the most published surfaces in the window, scoped to ONE repo (#orb-ci-stuck-repeat)", async () => {
     const env = createTestEnv();
     const publish = (targetKey: string, createdAt: string) =>