diff --git a/.gittensory.yml.example b/.gittensory.yml.example index 85b82fdbf8..44bdf316ae 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -339,3 +339,12 @@ settings: # Never fires for the repo owner, admin logins, or automation bots. PR-path only. Off by default. # accountAgeThresholdDays: null # Positive integer, or null/omitted to disable. Default: null (off). # newAccountLabel: new-account # Label applied to a below-threshold-age account's PR. Default: new-account. + + # Per-command @gittensory rate limit (#2560, anti-abuse): generalizes review-nag's cooldown pattern to + # EVERY @gittensory command (help/ask/preflight/blockers/... and the maintainer queue-digest commands), + # not just review-request pings — independent of review-nag's own thread-author-only scope. "hold" + # replies with a cooldown notice and skips the command's own dispatch. Off by default. + # commandRateLimitPolicy: off # off | hold. Default: off. + # commandRateLimitMaxPerWindow: 20 # Positive integer. Per-command invocation limit for a CHEAP command (cache-only, no AI call) within the window. Default: 20. + # commandRateLimitAiMaxPerWindow: 5 # Positive integer. Tighter limit for an AI-cost-bearing command (ask/blockers/preflight/reviewability/packet/duplicate-check/next-action/repo-fit). Default: 5. + # commandRateLimitWindowHours: 24 # Positive integer. Rolling window (hours) both limits above count against. Default: 24. diff --git a/apps/gittensory-ui/public/openapi.json b/apps/gittensory-ui/public/openapi.json index 5cdb50d945..9ad3e1f5b6 100644 --- a/apps/gittensory-ui/public/openapi.json +++ b/apps/gittensory-ui/public/openapi.json @@ -8697,6 +8697,28 @@ }, "newAccountLabel": { "type": "string" + }, + "commandRateLimitPolicy": { + "type": "string", + "enum": [ + "off", + "hold" + ] + }, + "commandRateLimitMaxPerWindow": { + "type": "integer", + "minimum": 0, + "exclusiveMinimum": true + }, + "commandRateLimitAiMaxPerWindow": { + "type": "integer", + "minimum": 0, + "exclusiveMinimum": true + }, + "commandRateLimitWindowHours": { + "type": "integer", + "minimum": 0, + "exclusiveMinimum": true } }, "required": [ diff --git a/migrations/0097_command_rate_limit.sql b/migrations/0097_command_rate_limit.sql new file mode 100644 index 0000000000..51ea2340dc --- /dev/null +++ b/migrations/0097_command_rate_limit.sql @@ -0,0 +1,11 @@ +-- Per-command @gittensory rate limit (#2560, anti-abuse): generalizes the review-nag cooldown's audit-ledger +-- counting pattern to EVERY @gittensory command, not just review-request pings. Independent of and complementary +-- to review-nag (that stays scoped to the thread's own author; this covers any actor invoking any command). +-- Defaults are byte-identical to today: command_rate_limit_policy defaults to 'off' (disabled), so existing +-- repos see no behavior change until they opt in. The AI-cost-bearing commands (ask/blockers/preflight/ +-- reviewability/packet/duplicate-check/next-action/repo-fit) get their own tighter default limit than the +-- cheap, cache-only commands (help/miner-context/the maintainer queue-digest commands). +ALTER TABLE repository_settings ADD COLUMN command_rate_limit_policy TEXT NOT NULL DEFAULT 'off'; +ALTER TABLE repository_settings ADD COLUMN command_rate_limit_max_per_window INTEGER NOT NULL DEFAULT 20; +ALTER TABLE repository_settings ADD COLUMN command_rate_limit_ai_max_per_window INTEGER NOT NULL DEFAULT 5; +ALTER TABLE repository_settings ADD COLUMN command_rate_limit_window_hours INTEGER NOT NULL DEFAULT 24; diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 3589785a29..733ef57818 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -513,6 +513,10 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise requireFreshRebaseWindowMinutes: null, accountAgeThresholdDays: null, newAccountLabel: "new-account", + commandRateLimitPolicy: "off", + commandRateLimitMaxPerWindow: 20, + commandRateLimitAiMaxPerWindow: 5, + commandRateLimitWindowHours: 24, }; } return { @@ -568,6 +572,10 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise requireFreshRebaseWindowMinutes: normalizeOpenItemCap(row.requireFreshRebaseWindowMinutes), accountAgeThresholdDays: normalizeOpenItemCap(row.accountAgeThresholdDays), newAccountLabel: row.newAccountLabel, + commandRateLimitPolicy: normalizeCommandRateLimitPolicy(row.commandRateLimitPolicy), + commandRateLimitMaxPerWindow: normalizePositiveIntWithDefault(row.commandRateLimitMaxPerWindow, 20), + commandRateLimitAiMaxPerWindow: normalizePositiveIntWithDefault(row.commandRateLimitAiMaxPerWindow, 5), + commandRateLimitWindowHours: normalizePositiveIntWithDefault(row.commandRateLimitWindowHours, 24), createdAt: row.createdAt, updatedAt: row.updatedAt, }; @@ -655,6 +663,10 @@ export async function upsertRepositorySettings(env: Env, settings: Partial { + const db = getDb(env.DB); + const [row] = await db + .select({ count: sql`count(*)` }) + .from(auditEvents) + .where( + and( + eq(auditEvents.actor, actor), + eq(auditEvents.eventType, eventType), + eq(auditEvents.targetKey, targetKey), + gte(auditEvents.createdAt, sinceIso), + sql`json_extract(${auditEvents.metadataJson}, '$.deliveryId') = ${deliveryId}`, + ), + ); + /* v8 ignore next -- count(*) always returns exactly one row; the empty-array guard only satisfies the destructure type. */ + return (row?.count ?? 0) > 0; +} + /** Observability for the queue dead-letter rate (#1276): how many jobs (across BOTH the maintenance and webhook * lanes) were dead-lettered since `sinceIso`. Reads the `github_app.dlq_dead_lettered` audit events written by * processDlqBatch — NOT gated behind any review-ops flag, so the infra drop rate is always visible. */ @@ -5790,6 +5840,10 @@ function normalizeReviewNagPolicy(value: string | null | undefined): "off" | "ho return value === "hold" || value === "close" ? value : "off"; } +function normalizeCommandRateLimitPolicy(value: string | null | undefined): "off" | "hold" { + return value === "hold" ? value : "off"; +} + // A review-nag threshold/window is a discrete positive count, not a score — reuses the same non-clamping, // non-rounding shape as contributorOpenPrCap's normalizeOpenItemCap (#2270): an invalid value (fractional, // non-positive, non-finite) falls back to the given default rather than being silently coerced. diff --git a/src/db/schema.ts b/src/db/schema.ts index 5dadf36c1a..ddc534eb1c 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -106,6 +106,12 @@ export const repositorySettings = sqliteTable("repository_settings", { // runAgentMaintenancePlanAndExecute, not here. accountAgeThresholdDays: integer("account_age_threshold_days"), newAccountLabel: text("new_account_label").notNull().default("new-account"), + // Per-command @gittensory rate limit (#2560, anti-abuse): generalizes review-nag's cooldown pattern to every + // command, keyed by (actor, command, targetKey) independent of review-nag's own thread-author-only scope. + commandRateLimitPolicy: text("command_rate_limit_policy").notNull().default("off"), + commandRateLimitMaxPerWindow: integer("command_rate_limit_max_per_window").notNull().default(20), + commandRateLimitAiMaxPerWindow: integer("command_rate_limit_ai_max_per_window").notNull().default(5), + commandRateLimitWindowHours: integer("command_rate_limit_window_hours").notNull().default(24), createdAt: text("created_at").notNull().$defaultFn(() => nowIso()), updatedAt: text("updated_at").notNull().$defaultFn(() => nowIso()), }); diff --git a/src/github/commands.ts b/src/github/commands.ts index 2734c3d938..cf164e5470 100644 --- a/src/github/commands.ts +++ b/src/github/commands.ts @@ -201,6 +201,26 @@ export function isMaintainerOnlyCommand(command: GittensoryMentionCommandName): return isMaintainerQueueDigestCommand(command); } +// Commands that dispatch to a real AI orchestrator call (planNextWork / explainBlockersWithAgent / +// preflightBranchWithAgent / preparePrPacketWithAgent in buildMentionCommandBundle), as opposed to `help`, +// `miner-context` (both no-op), and every maintainer queue-digest command (cache-only DB reads via +// buildMaintainerQueueDigestForCommand, no AI call at all) (#2560). Used to apply a tighter per-command rate +// limit to the AI-cost-bearing surface than the cheap one. +const AI_COST_BEARING_COMMANDS = new Set([ + "ask", + "blockers", + "preflight", + "reviewability", + "packet", + "duplicate-check", + "next-action", + "repo-fit", +]); + +export function isAiCostBearingCommand(command: GittensoryMentionCommandName): boolean { + return AI_COST_BEARING_COMMANDS.has(command); +} + export function isAuthorizedCommandActor(args: { commandName?: GittensoryMentionCommandName | undefined; commenterLogin?: string | null | undefined; diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index 81f3ea0bd7..734106d052 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -652,6 +652,10 @@ export const RepositorySettingsSchema = z autoCloseExemptLogins: z.array(z.string()).optional(), accountAgeThresholdDays: z.number().int().positive().nullable().optional(), newAccountLabel: z.string().optional(), + commandRateLimitPolicy: z.enum(["off", "hold"]).optional(), + commandRateLimitMaxPerWindow: z.number().int().positive().optional(), + commandRateLimitAiMaxPerWindow: z.number().int().positive().optional(), + commandRateLimitWindowHours: z.number().int().positive().optional(), createdAt: z.string().nullable().optional(), updatedAt: z.string().nullable().optional(), }) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 6d7642568d..8853c545b7 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -51,6 +51,7 @@ import { recordAgentCommandFeedback, recordAuditEvent, countRecentAuditEventsForActorAndTarget, + hasAuditEventForDelivery, recordGateBlockOutcome, getGateBlockOutcome, isGlobalAgentFrozen, @@ -138,6 +139,7 @@ import { buildMaintainerQueueDigest, buildPublicAgentCommandComment, type GittensoryMentionCommandName, + isAiCostBearingCommand, isAuthorizedCommandActor, isMaintainerQueueDigestCommand, parseAgentCommandFeedbackContext, @@ -8936,6 +8938,119 @@ async function maybeThrottleReviewNagPing( return true; } +// Audit eventType for one recorded @gittensory command invocation (#2560). Shared between the recorder below +// and the cooldown-window count query so a naming drift can't silently under/over-count. +const COMMAND_RATE_LIMIT_EVENT_TYPE = "github_app.command_invocation"; +// How far back to look for a redelivered webhook's OWN prior invocation record. Deliberately much shorter +// than the rate-limit window itself (hours) -- a genuine GitHub redelivery lands within seconds/minutes. +const COMMAND_RATE_LIMIT_REDELIVERY_WINDOW_MS = 10 * 60_000; + +/** + * Per-command @gittensory rate limit (#2560, anti-abuse): generalizes review-nag's audit-ledger counting + * pattern (`countRecentAuditEventsForActorAndTarget`) to EVERY `@gittensory` Q&A command, not just + * review-request pings. Keyed by `(actor, command, targetKey)` — the command name is folded into targetKey so + * repeatedly invoking ONE command never counts against a DIFFERENT command's own limit. Independent of, and + * complementary to, `maybeThrottleReviewNagPing` above: that one stays scoped to the thread's OWN author and + * can close a PR; this covers ANY authorized actor invoking ANY command and only ever holds (declines with a + * notice), never closes. Off (`commandRateLimitPolicy: "off"`, the default) is a complete no-op. + */ +async function maybeThrottleGittensoryCommand( + env: Env, + args: { + deliveryId: string; + repoFullName: string; + issueNumber: number; + installationId: number; + commenter: string; + command: GittensoryMentionCommandName; + settings: RepositorySettings; + mode: ReturnType; + }, +): Promise { + /* v8 ignore next -- resolveRepositorySettings always resolves a concrete "off"/"hold"; the undefined side is defensive against the field's optional TS type. */ + const policy = args.settings.commandRateLimitPolicy ?? "off"; + if (policy === "off") return false; + + const targetKey = `${args.repoFullName}#${args.issueNumber}#${args.command}`; + + // Webhook redelivery guard: GitHub can and does redeliver the same issue_comment event (timeout/retry) -- + // without this, a redelivered event would increment the counter a SECOND time for one real invocation and + // could incorrectly rate-limit it. Scoped to a short recent window (not the full rate-limit window) — a + // genuine redelivery lands within seconds/minutes, not hours later. + const redeliverySinceIso = new Date(Date.now() - COMMAND_RATE_LIMIT_REDELIVERY_WINDOW_MS).toISOString(); + const alreadySeen = await hasAuditEventForDelivery(env, args.commenter, COMMAND_RATE_LIMIT_EVENT_TYPE, targetKey, args.deliveryId, redeliverySinceIso); + // Gate review finding: returning `false` here let a redelivered webhook fall through to normal dispatch — a + // SECOND run of the (possibly cost-bearing) command for one real invocation, uncounted and unheld. The + // original delivery already ran the command and posted its own answer, so short-circuit the replay entirely + // (no dispatch, no comment) rather than treating it as an under-threshold pass-through. + if (alreadySeen) { + await recordAuditEvent(env, { + eventType: "github_app.command_redelivery_suppressed", + actor: args.commenter, + targetKey, + outcome: "completed", + detail: `redelivered ${args.command} invocation suppressed (deliveryId ${args.deliveryId})`, + metadata: { deliveryId: args.deliveryId, repoFullName: args.repoFullName, command: args.command }, + }).catch( + /* v8 ignore next -- fail-safe: an audit write failure never blocks the redelivery suppression itself */ + () => undefined, + ); + return true; + } + + const aiCostBearing = isAiCostBearingCommand(args.command); + /* v8 ignore next -- resolveRepositorySettings always resolves a concrete positive integer; the undefined side is defensive against the field's optional TS type. */ + const maxPerWindow = aiCostBearing + ? (args.settings.commandRateLimitAiMaxPerWindow ?? 5) + : (args.settings.commandRateLimitMaxPerWindow ?? 20); + /* v8 ignore next -- resolveRepositorySettings always resolves a concrete positive integer; the undefined side is defensive against the field's optional TS type. */ + const windowHours = args.settings.commandRateLimitWindowHours ?? 24; + const sinceIso = new Date(Date.now() - windowHours * 60 * 60 * 1000).toISOString(); + const priorInvocations = await countRecentAuditEventsForActorAndTarget(env, args.commenter, COMMAND_RATE_LIMIT_EVENT_TYPE, targetKey, sinceIso); + const invocationCount = priorInvocations + 1; // this invocation counts too + + // Always record the invocation first so the running count reflects reality even when the rest of this + // handler short-circuits below (a failed recordAuditEvent must never block command dispatch). + await recordAuditEvent(env, { + eventType: COMMAND_RATE_LIMIT_EVENT_TYPE, + actor: args.commenter, + targetKey, + outcome: "completed", + detail: `invocation ${invocationCount}/${maxPerWindow} within ${windowHours}h window`, + metadata: { deliveryId: args.deliveryId, repoFullName: args.repoFullName, command: args.command, aiCostBearing }, + }).catch( + /* v8 ignore next -- fail-safe: an audit write failure never blocks command dispatch */ + () => undefined, + ); + + if (invocationCount <= maxPerWindow) return false; // under threshold — normal dispatch proceeds unchanged + + if (args.mode === "live") { + await createIssueComment( + env, + args.installationId, + args.repoFullName, + args.issueNumber, + `@${args.commenter} the \`${args.command}\` command has reached its rate limit (${maxPerWindow} within ${windowHours}h). Please wait for the window to pass before trying again. This is an automated maintenance action.`, + ).catch( + /* v8 ignore next -- fail-safe: a comment-post failure must not crash the throttle decision itself */ + () => undefined, + ); + } + await recordAuditEvent(env, { + eventType: "github_app.command_rate_limit_applied", + actor: "gittensory", + targetKey, + outcome: args.mode === "live" ? "completed" : "denied", + detail: `hold applied: ${args.commenter} invoked ${args.command} ${invocationCount} times (limit ${maxPerWindow})`, + metadata: { deliveryId: args.deliveryId, repoFullName: args.repoFullName, mode: args.mode, command: args.command }, + }).catch( + /* v8 ignore next -- fail-safe: an audit write failure never blocks the handler */ + () => undefined, + ); + return true; +} + async function maybeProcessGittensoryMentionCommand( env: Env, deliveryId: string, @@ -9133,6 +9248,21 @@ async function maybeProcessGittensoryMentionCommand( return true; } + if ( + await maybeThrottleGittensoryCommand(env, { + deliveryId, + repoFullName, + issueNumber: issue.number, + installationId, + commenter, + command: command.name, + settings, + mode: mentionMode, + }) + ) { + return true; + } + const answerId = crypto.randomUUID(); const login = pullRequestAuthor ?? commenter; const maintainerDigest = isMaintainerQueueDigestCommand(command.name) diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index 85d49f17b8..96d270e33e 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -152,6 +152,10 @@ export type FocusManifestSettings = Partial< | "autoCloseExemptLogins" | "accountAgeThresholdDays" | "newAccountLabel" + | "commandRateLimitPolicy" + | "commandRateLimitMaxPerWindow" + | "commandRateLimitAiMaxPerWindow" + | "commandRateLimitWindowHours" > >; @@ -879,6 +883,15 @@ function parseSettingsOverride(value: JsonValue | undefined, warnings: string[]) } const newAccountLabel = normalizeOptionalString(r.newAccountLabel, "settings.newAccountLabel", warnings); if (newAccountLabel !== null) out.newAccountLabel = newAccountLabel; + // Per-command @gittensory rate limit (#2560): generalizes review-nag's cooldown pattern to every command. + const commandRateLimitPolicy = normalizeOptionalEnum(r.commandRateLimitPolicy, "settings.commandRateLimitPolicy", ["off", "hold"] as const, warnings); + if (commandRateLimitPolicy !== null) out.commandRateLimitPolicy = commandRateLimitPolicy; + const commandRateLimitMaxPerWindow = normalizeOptionalPositiveInteger(r.commandRateLimitMaxPerWindow, "settings.commandRateLimitMaxPerWindow", warnings); + if (commandRateLimitMaxPerWindow !== null) out.commandRateLimitMaxPerWindow = commandRateLimitMaxPerWindow; + const commandRateLimitAiMaxPerWindow = normalizeOptionalPositiveInteger(r.commandRateLimitAiMaxPerWindow, "settings.commandRateLimitAiMaxPerWindow", warnings); + if (commandRateLimitAiMaxPerWindow !== null) out.commandRateLimitAiMaxPerWindow = commandRateLimitAiMaxPerWindow; + const commandRateLimitWindowHours = normalizeOptionalPositiveInteger(r.commandRateLimitWindowHours, "settings.commandRateLimitWindowHours", warnings); + if (commandRateLimitWindowHours !== null) out.commandRateLimitWindowHours = commandRateLimitWindowHours; return out; } diff --git a/src/types.ts b/src/types.ts index a4b5c887b3..0571fb0a49 100644 --- a/src/types.ts +++ b/src/types.ts @@ -676,6 +676,28 @@ export type RepositorySettings = { * configurable-with-fallback shape. Always populated by the DB layer (default `"new-account"`); optional so * existing settings fixtures/callers need not be touched. */ newAccountLabel?: string | undefined; + /** Per-command @gittensory rate limit (#2560, anti-abuse): generalizes the review-nag cooldown's counting + * pattern (the audit-events ledger) to EVERY `@gittensory` command, keyed by `(actor, command, targetKey)` -- + * independent of, and complementary to, review-nag's own narrower thread-author-only scope. `"off"` (default) + * is a no-op; `"hold"` posts a deterministic cooldown reply and skips the command's own dispatch. Always + * populated by the DB layer (default `"off"`); optional so existing settings fixtures/callers need not be + * touched. */ + commandRateLimitPolicy?: "off" | "hold" | undefined; + /** Per-command rate limit (#2560): how many invocations of a single command an actor may make within + * {@link commandRateLimitWindowHours} before the (N+1)th is throttled -- for a CHEAP command (cache-only, + * no AI orchestrator call). Always populated by the DB layer (default `20`); optional so existing settings + * fixtures/callers need not be touched. Only meaningful when {@link commandRateLimitPolicy} is not `"off"`. */ + commandRateLimitMaxPerWindow?: number | undefined; + /** Per-command rate limit (#2560): the same threshold as {@link commandRateLimitMaxPerWindow}, but for an + * AI-cost-bearing command (dispatches to a real orchestrator call: `ask`, `blockers`, `preflight`, + * `reviewability`, `packet`, `duplicate-check`, `next-action`, `repo-fit`). Deliberately tighter than the + * cheap-command default. Always populated by the DB layer (default `5`); optional so existing settings + * fixtures/callers need not be touched. */ + commandRateLimitAiMaxPerWindow?: number | undefined; + /** Per-command rate limit (#2560): the rolling window (in hours) both {@link commandRateLimitMaxPerWindow} + * and {@link commandRateLimitAiMaxPerWindow} count against. Always populated by the DB layer (default `24`); + * optional so existing settings fixtures/callers need not be touched. */ + commandRateLimitWindowHours?: number | undefined; /** Agent-layer autonomy dial (#773): per-action-class level. Always populated by the DB layer (default * `{}` = deny-by-default = "observe" for every class); optional so existing settings fixtures/callers * need not be touched. The single source the action layer (#778) reads via `resolveAutonomy`. */ diff --git a/test/unit/db-parsers.test.ts b/test/unit/db-parsers.test.ts index 4ff5bb26e3..533cd9a9ee 100644 --- a/test/unit/db-parsers.test.ts +++ b/test/unit/db-parsers.test.ts @@ -4,6 +4,7 @@ import { countRecentDeadLetters, countRecentDeadLettersByType, countRecentAuditEventsForActorAndTarget, + hasAuditEventForDelivery, getLatestScorePreview, getRepoAuthorPullRequestHistory, getLatestScoringModelSnapshot, @@ -493,6 +494,56 @@ 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("hasAuditEventForDelivery finds a matching deliveryId inside metadata_json, scoped to actor+eventType+targetKey (#2560)", async () => { + const env = createTestEnv(); + await recordAuditEvent(env, { + eventType: "github_app.command_invocation", + actor: "maintainer", + targetKey: "owner/repo#1#help", + outcome: "completed", + createdAt: "2026-06-24T10:00:00.000Z", + metadata: { deliveryId: "delivery-a" }, + }); + + expect(await hasAuditEventForDelivery(env, "maintainer", "github_app.command_invocation", "owner/repo#1#help", "delivery-a", "2026-06-24T09:00:00.000Z")).toBe(true); + // A different deliveryId on the SAME actor+eventType+targetKey must not match. + expect(await hasAuditEventForDelivery(env, "maintainer", "github_app.command_invocation", "owner/repo#1#help", "delivery-b", "2026-06-24T09:00:00.000Z")).toBe(false); + // The SAME deliveryId but for a DIFFERENT command's targetKey must not match (each command's own counter). + expect(await hasAuditEventForDelivery(env, "maintainer", "github_app.command_invocation", "owner/repo#1#ask", "delivery-a", "2026-06-24T09:00:00.000Z")).toBe(false); + // A cutoff AFTER the recorded event must not match (outside the recent window). + expect(await hasAuditEventForDelivery(env, "maintainer", "github_app.command_invocation", "owner/repo#1#help", "delivery-a", "2026-06-24T11:00:00.000Z")).toBe(false); + }); + + it("REGRESSION (gate-flagged): hasAuditEventForDelivery still finds the matching deliveryId when MORE than 50 other rows exist in the window (burst/spam scenario)", async () => { + // A prior version matched deliveryId IN MEMORY over a `.limit(50)` slice with no ORDER BY, so once an + // actor had more than 50 matching rows in the window, the row carrying the target deliveryId could be + // excluded from that arbitrary slice -- a false negative right when a burst/spam scenario (the abuse + // case this feature exists to handle) makes it most likely. The fix pushes the deliveryId match into the + // SQL predicate itself, so it must still be found regardless of how many OTHER rows exist. + const env = createTestEnv(); + for (let i = 0; i < 60; i += 1) { + await recordAuditEvent(env, { + eventType: "github_app.command_invocation", + actor: "maintainer", + targetKey: "owner/repo#1#help", + outcome: "completed", + createdAt: "2026-06-24T10:00:00.000Z", + metadata: { deliveryId: `delivery-noise-${i}` }, + }); + } + await recordAuditEvent(env, { + eventType: "github_app.command_invocation", + actor: "maintainer", + targetKey: "owner/repo#1#help", + outcome: "completed", + createdAt: "2026-06-24T10:00:00.000Z", + metadata: { deliveryId: "delivery-target" }, + }); + + expect(await hasAuditEventForDelivery(env, "maintainer", "github_app.command_invocation", "owner/repo#1#help", "delivery-target", "2026-06-24T09:00:00.000Z")).toBe(true); + expect(await hasAuditEventForDelivery(env, "maintainer", "github_app.command_invocation", "owner/repo#1#help", "delivery-never-recorded", "2026-06-24T09:00:00.000Z")).toBe(false); + }); + it("computes complete case-insensitive repo author PR history for gate grace", async () => { const env = createTestEnv(); diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 6846e63ada..21f7f59169 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -1402,6 +1402,32 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () = expect(invalid.warnings.some((w) => /settings\.accountAgeThresholdDays/.test(w))).toBe(true); }); + it("parses + resolves the per-command rate limit settings from the settings: block, overlaying the DB (#2560)", () => { + const manifest = parseFocusManifest({ settings: { commandRateLimitPolicy: "hold", commandRateLimitMaxPerWindow: 10, commandRateLimitAiMaxPerWindow: 2, commandRateLimitWindowHours: 12 } }); + expect(manifest.settings.commandRateLimitPolicy).toBe("hold"); + expect(manifest.settings.commandRateLimitMaxPerWindow).toBe(10); + expect(manifest.settings.commandRateLimitAiMaxPerWindow).toBe(2); + expect(manifest.settings.commandRateLimitWindowHours).toBe(12); + // yml overlays a DB-configured policy. + const eff = resolveEffectiveSettings({ commandRateLimitPolicy: "off", commandRateLimitMaxPerWindow: 20, commandRateLimitAiMaxPerWindow: 5, commandRateLimitWindowHours: 24 } as unknown as RepositorySettings, manifest); + expect(eff.commandRateLimitPolicy).toBe("hold"); + expect(eff.commandRateLimitMaxPerWindow).toBe(10); + // Omitted in yml ⇒ the DB-configured policy survives untouched. + const noOverride = resolveEffectiveSettings({ commandRateLimitPolicy: "hold", commandRateLimitAiMaxPerWindow: 3 } as unknown as RepositorySettings, parseFocusManifest({})); + expect(noOverride.commandRateLimitPolicy).toBe("hold"); + expect(noOverride.commandRateLimitAiMaxPerWindow).toBe(3); + // An invalid policy enum / non-positive window value is dropped with a warning rather than silently coerced. + const invalid = parseFocusManifest({ settings: { commandRateLimitPolicy: "close" as never, commandRateLimitMaxPerWindow: 0, commandRateLimitAiMaxPerWindow: -1, commandRateLimitWindowHours: -5 } }); + expect(invalid.settings.commandRateLimitPolicy).toBeUndefined(); + expect(invalid.settings.commandRateLimitMaxPerWindow).toBeUndefined(); + expect(invalid.settings.commandRateLimitAiMaxPerWindow).toBeUndefined(); + expect(invalid.settings.commandRateLimitWindowHours).toBeUndefined(); + expect(invalid.warnings.some((w) => /settings\.commandRateLimitPolicy/.test(w))).toBe(true); + expect(invalid.warnings.some((w) => /settings\.commandRateLimitMaxPerWindow/.test(w))).toBe(true); + expect(invalid.warnings.some((w) => /settings\.commandRateLimitAiMaxPerWindow/.test(w))).toBe(true); + expect(invalid.warnings.some((w) => /settings\.commandRateLimitWindowHours/.test(w))).toBe(true); + }); + it("parses + resolves autoCloseExemptLogins from the settings: block, overlaying the DB (#2463)", () => { const manifest = parseFocusManifest({ settings: { autoCloseExemptLogins: ["Trusted-Regular", "another-one", "-bad", 42 as never] } }); expect(manifest.settings.autoCloseExemptLogins).toEqual(["Trusted-Regular", "another-one"]); // invalid entries dropped diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 5a59c90de7..44ae3415b9 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -12460,6 +12460,145 @@ describe("queue processors", () => { }); }); + describe("per-command @gittensory rate limit (#2560)", () => { + function stubCommandRateLimitFetch(issueNumber: number, seen: { comments: string[] }) { + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" }); + if (url.includes("/collaborators/") && url.includes("/permission")) return Response.json({ permission: "maintain" }); + if (url.includes(`/issues/${issueNumber}/comments`) && method === "GET") return Response.json([]); + if (url.includes(`/issues/${issueNumber}/comments`) && method === "POST") { + seen.comments.push(String(JSON.parse(String(init?.body ?? "{}")).body ?? "")); + return Response.json({ id: seen.comments.length }, { status: 201 }); + } + return new Response("not found", { status: 404 }); + }); + } + + function mentionPayload(issueNumber: number, body: string) { + return { + action: "created" as const, + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + issue: { number: issueNumber, title: "Rate limit target", state: "open", pull_request: {}, user: { login: "oktofeesh1" }, author_association: "NONE" }, + comment: { id: 1, body, user: { login: "maintainer", type: "User" }, author_association: "OWNER" }, + }; + } + + it("is off by default — no invocation is tracked and every command dispatches normally", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 300, title: "Rate limit target", state: "open", user: { login: "oktofeesh1" }, author_association: "NONE", labels: [], body: "" }); + const seen = { comments: [] as string[] }; + stubCommandRateLimitFetch(300, seen); + for (let i = 0; i < 25; i += 1) { + await processJob(env, { type: "github-webhook", deliveryId: `rl-off-${i}`, eventName: "issue_comment", payload: mentionPayload(300, "@gittensory help") }); + } + const invocations = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.command_invocation'").first<{ n: number }>(); + expect(invocations?.n).toBe(0); + expect(seen.comments).toHaveLength(25); // every one of the 25 invocations dispatched normally + }); + + it("records invocations under the configured threshold without holding — the normal reply still proceeds", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", commandRateLimitPolicy: "hold", commandRateLimitMaxPerWindow: 5, commandRateLimitWindowHours: 24 }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 301, title: "Rate limit target", state: "open", user: { login: "oktofeesh1" }, author_association: "NONE", labels: [], body: "" }); + const seen = { comments: [] as string[] }; + stubCommandRateLimitFetch(301, seen); + await processJob(env, { type: "github-webhook", deliveryId: "rl-under", eventName: "issue_comment", payload: mentionPayload(301, "@gittensory help") }); + const invocations = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.command_invocation'").first<{ n: number }>(); + expect(invocations?.n).toBe(1); // 1st of 5 allowed + const applied = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.command_rate_limit_applied'").first<{ n: number }>(); + expect(applied?.n).toBe(0); // under threshold — no hold + expect(seen.comments).toHaveLength(1); // the normal answer card still posted + }); + + it("hold policy: posts a cooldown reply and short-circuits once a CHEAP command crosses its threshold", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", commandRateLimitPolicy: "hold", commandRateLimitMaxPerWindow: 3, commandRateLimitWindowHours: 24 }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 302, title: "Rate limit target", state: "open", user: { login: "oktofeesh1" }, author_association: "NONE", labels: [], body: "" }); + for (let i = 0; i < 3; i += 1) { + await repositoriesModule.recordAuditEvent(env, { eventType: "github_app.command_invocation", actor: "maintainer", targetKey: "JSONbored/gittensory#302#help", outcome: "completed" }); + } + const seen = { comments: [] as string[] }; + stubCommandRateLimitFetch(302, seen); + await processJob(env, { type: "github-webhook", deliveryId: "rl-cheap-over", eventName: "issue_comment", payload: mentionPayload(302, "@gittensory help") }); + // Only ONE comment posted — the short-circuit skipped the normal answer-card dispatch. + expect(seen.comments).toHaveLength(1); + expect(seen.comments[0]).toContain("rate limit"); + const applied = await env.DB.prepare("select outcome, detail from audit_events where event_type = 'github_app.command_rate_limit_applied'").first<{ outcome: string; detail: string }>(); + expect(applied?.outcome).toBe("completed"); + expect(applied?.detail).toContain("hold applied"); + }); + + it("an AI-cost-bearing command uses the TIGHTER commandRateLimitAiMaxPerWindow default, not the cheap-command limit", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + // Cheap-command limit left generous (20, the default); only the AI limit is tight enough to trip here. + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", commandRateLimitPolicy: "hold", commandRateLimitAiMaxPerWindow: 2, commandRateLimitWindowHours: 24 }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 303, title: "Rate limit target", state: "open", user: { login: "oktofeesh1" }, author_association: "NONE", labels: [], body: "" }); + for (let i = 0; i < 2; i += 1) { + await repositoriesModule.recordAuditEvent(env, { eventType: "github_app.command_invocation", actor: "maintainer", targetKey: "JSONbored/gittensory#303#next-action", outcome: "completed" }); + } + const seen = { comments: [] as string[] }; + stubCommandRateLimitFetch(303, seen); + await processJob(env, { type: "github-webhook", deliveryId: "rl-ai-over", eventName: "issue_comment", payload: mentionPayload(303, "@gittensory next-action") }); + expect(seen.comments).toHaveLength(1); + expect(seen.comments[0]).toContain("rate limit"); + const applied = await env.DB.prepare("select detail from audit_events where event_type = 'github_app.command_rate_limit_applied'").first<{ detail: string }>(); + expect(applied?.detail).toContain("limit 2"); // the AI limit (2), not the cheap default (20) + }); + + it("commands have INDEPENDENT counters — repeatedly invoking one command never throttles a DIFFERENT command", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", commandRateLimitPolicy: "hold", commandRateLimitMaxPerWindow: 1, commandRateLimitWindowHours: 24 }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 304, title: "Rate limit target", state: "open", user: { login: "oktofeesh1" }, author_association: "NONE", labels: [], body: "" }); + // Already at the "help" limit (1) — a further "help" invocation would be held. + await repositoriesModule.recordAuditEvent(env, { eventType: "github_app.command_invocation", actor: "maintainer", targetKey: "JSONbored/gittensory#304#help", outcome: "completed" }); + const seen = { comments: [] as string[] }; + stubCommandRateLimitFetch(304, seen); + // A DIFFERENT command ("miner-context") on the same thread by the same actor must not be affected. + await processJob(env, { type: "github-webhook", deliveryId: "rl-independent", eventName: "issue_comment", payload: mentionPayload(304, "@gittensory miner-context") }); + expect(seen.comments).toHaveLength(1); + expect(seen.comments[0]).not.toContain("rate limit"); + const applied = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.command_rate_limit_applied'").first<{ n: number }>(); + expect(applied?.n).toBe(0); + }); + + it("dry-run mode: holds the command but never posts a live cooldown comment", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", commandRateLimitPolicy: "hold", commandRateLimitMaxPerWindow: 1, commandRateLimitWindowHours: 24, agentDryRun: true }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 305, title: "Rate limit target", state: "open", user: { login: "oktofeesh1" }, author_association: "NONE", labels: [], body: "" }); + await repositoriesModule.recordAuditEvent(env, { eventType: "github_app.command_invocation", actor: "maintainer", targetKey: "JSONbored/gittensory#305#help", outcome: "completed" }); + const seen = { comments: [] as string[] }; + stubCommandRateLimitFetch(305, seen); + await processJob(env, { type: "github-webhook", deliveryId: "rl-dry-run", eventName: "issue_comment", payload: mentionPayload(305, "@gittensory help") }); + expect(seen.comments).toHaveLength(0); // held, but dry-run posts nothing live + const applied = await env.DB.prepare("select outcome from audit_events where event_type = 'github_app.command_rate_limit_applied'").first<{ outcome: string }>(); + expect(applied?.outcome).toBe("denied"); + }); + + it("REGRESSION: a redelivered webhook (same deliveryId) does not double-count — the replay is a no-op, not a second invocation", async () => { + // GitHub can and does redeliver the same issue_comment event (timeout/retry). Before the fix, the + // second delivery would increment the counter again for what is really ONE real invocation, and could + // incorrectly cross the rate-limit threshold on a redelivery alone. + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", commandRateLimitPolicy: "hold", commandRateLimitMaxPerWindow: 1, commandRateLimitWindowHours: 24 }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 306, title: "Rate limit target", state: "open", user: { login: "oktofeesh1" }, author_association: "NONE", labels: [], body: "" }); + const seen = { comments: [] as string[] }; + stubCommandRateLimitFetch(306, seen); + // The SAME deliveryId, redelivered — GitHub's own retry behavior on a timeout/5xx. + await processJob(env, { type: "github-webhook", deliveryId: "rl-redelivered", eventName: "issue_comment", payload: mentionPayload(306, "@gittensory help") }); + await processJob(env, { type: "github-webhook", deliveryId: "rl-redelivered", eventName: "issue_comment", payload: mentionPayload(306, "@gittensory help") }); + + const invocations = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.command_invocation'").first<{ n: number }>(); + expect(invocations?.n).toBe(1); // only ONE invocation recorded despite two processing passes + expect(seen.comments).toHaveLength(1); // the replay is suppressed entirely — no second answer card + expect(seen.comments.every((c) => !c.includes("rate limit"))).toBe(true); + const suppressed = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.command_redelivery_suppressed'").first<{ n: number }>(); + expect(suppressed?.n).toBe(1); + }); + }); + it("denies a maintainer Q&A command from an org member without real repo permission (#788)", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", {