fix(review): scope review-nag ping cooldown to the actor, not one PR by JSONbored · Pull Request #4021 · JSONbored/loopover

JSONbored · 2026-07-07T10:40:34Z

Summary

maybeThrottleReviewNagPing counted @gittensory pings via a targetKey scoped to the current PR/issue only (countRecentAuditEventsForActorAndTarget(env, commenter, REVIEW_NAG_PING_EVENT_TYPE, targetKey, sinceIso)). Enforcement holds/closes the PR once the count reaches reviewNagMaxPings, but because the count was scoped per-target, a contributor who exhausted their pings on one PR got a clean 0/maxPings slate on any fresh PR — a new issue.number is a new targetKey, with zero memory of the prior PR's ping history. This is inconsistent with the contributor blacklist and moderation-rules ban tally, which correctly persist by actor login across every PR.

Adds countRecentAuditEventsForActorInRepo (src/db/repositories.ts), which counts one actor's matching audit events across every target within a repo (reusing the same literal-prefix LIKE ... ESCAPE scoping already used by findHottestReviewTargetForRepo), and wires it into maybeThrottleReviewNagPing in place of the per-target count. Because the count now spans every target the actor has pinged on within the repo, the very first ping on a brand-new PR already reflects a prior PR's exhausted count — no separate "still on cooldown" table is needed; the existing audit_events ledger read at the new repo scope already is the persistent per-actor cooldown state.

The sibling maybeThrottleMonitoredMentions (the per-maintainer @-mention nag counter) had the identical gap, via a targetKey suffixed #mention:<login>. Fixed the same way with a second new function, countRecentAuditEventsForActorInRepoWithTargetSuffix, which keeps the repo-wide carryover while still pinning to one mentioned login's own suffix, so independently-budgeted mentioned logins never bleed into each other's count (mentioning maintainer X vs. maintainer Y, and pinging @gittensory vs. mentioning a maintainer, all stay on separate budgets, per the existing design).

Closes #4020

Scope

The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

Every changed line/branch in src/db/repositories.ts and src/queue/processors.ts is covered, confirmed via the v8 lcov report (DA/BRDA entries) for the exact diff ranges. Two regression tests added: one for each throttle function, seeding an exhausted PR-A ping count then confirming a single PR-B ping already crosses the threshold; a third confirms a different mentioned login on PR B keeps its own independent budget.

Note: npm run test:coverage (part of the full test:ci chain) currently has a small number of pre-existing failures on origin/main unrelated to this change (verified by running them against a clean, unmodified origin/main checkout before and after this diff, with identical results): a stale hardcoded-date assertion in test/unit/predicted-gate-engine-coverage.test.ts, a Prometheus-metrics-ordering assertion in 4 tests under test/unit/queue.test.ts's "auto-action convergence" suite, and a doc/example drift in test/unit/config-templates.test.ts (likely from the recently-merged review.shared_config / #2046). None of these touch review-nag, audit-events, or any file this PR changes.

Safety

No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
API/OpenAPI/MCP behavior is updated and tested where needed.
UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.
Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

No UI/frontend files touched — this is a backend-only (src/db/repositories.ts, src/queue/processors.ts) change, so no UI Evidence section applies.

Notes

Separately (and unrelated to this PR): while rebasing onto fresh origin/main, I found src/queue/processors.ts currently fails npm run typecheck on origin/main HEAD due to a splicing error introduced by the just-merged fix(queue): prevent regate repair exhaustion from premature attempt counting #3998 (regate-repair-attempts fix), around the isPriorityRepair/repairHeadSha job-dispatch code — confirmed via git show origin/main:src/queue/processors.ts directly, unrelated to this diff (my changed lines don't overlap that region at all). This appears to already be tracked/being worked separately.

maybeThrottleReviewNagPing counted @gittensory pings via a targetKey scoped to the current PR/issue only, so a contributor who exhausted their pings on one PR got a clean 0/maxPings slate on any fresh PR (a new issue.number is a new targetKey). Add countRecentAuditEventsForActorInRepo, which counts one actor's matching audit events across every target in a repo, and wire it into the throttle so the running count follows the contributor across PRs, mirroring how the contributor blacklist and moderation-rules ban tally already persist by login rather than by thread. maybeThrottleMonitoredMentions had the identical gap for its per-maintainer mention counter. Fixed the same way via a new countRecentAuditEventsForActorInRepoWithTargetSuffix, which keeps the repo-wide carryover while still pinning to one mentioned login's own targetKey suffix, so independently-budgeted mentioned logins never bleed into each other's count.

codecov · 2026-07-07T10:41:35Z

❌ 51 Tests Failed:

Tests completed	Failed	Passed	Skipped
10424	51	10373	7

View the top 3 failed test(s) by shortest run time

test/unit/mcp-server-telemetry.test.ts > MCP server telemetry > records session-scoped MCP request errors without a tool name

Stack Traces | 0.0154s run time

Error: Failed to resolve entry for package "@jsonbored/gittensory-engine". The package may have incorrect main/module/exports specified in its package.json.
 ❯ .../gittensory-miner/lib/opportunity-fanout.js:2:1
 ❯ src/mcp/find-opportunities.ts:11:1

test/unit/mcp-server-telemetry.test.ts > MCP server telemetry > falls back when Hono does not expose an execution context

Stack Traces | 0.0306s run time

Error: Failed to resolve entry for package "@jsonbored/gittensory-engine". The package may have incorrect main/module/exports specified in its package.json.
 ❯ .../gittensory-miner/lib/opportunity-fanout.js:2:1
 ❯ src/mcp/find-opportunities.ts:11:1

test/unit/mcp-server-telemetry.test.ts > MCP server telemetry > records sanitized error telemetry when the MCP transport handler throws

Stack Traces | 0.268s run time

Error: Failed to resolve entry for package "@jsonbored/gittensory-engine". The package may have incorrect main/module/exports specified in its package.json.
 ❯ .../gittensory-miner/lib/opportunity-fanout.js:2:1
 ❯ src/mcp/find-opportunities.ts:11:1

View the full list of 48 ❄️ flaky test(s)

test/integration/api.test.ts > test/integration/api.test.ts