Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .gittensory.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -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.
22 changes: 22 additions & 0 deletions apps/gittensory-ui/public/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down
11 changes: 11 additions & 0 deletions migrations/0097_command_rate_limit.sql
Original file line number Diff line number Diff line change
@@ -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;
54 changes: 54 additions & 0 deletions src/db/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
};
Expand Down Expand Up @@ -655,6 +663,10 @@ export async function upsertRepositorySettings(env: Env, settings: Partial<Repos
requireFreshRebaseWindowMinutes: normalizeOpenItemCap(settings.requireFreshRebaseWindowMinutes),
accountAgeThresholdDays: normalizeOpenItemCap(settings.accountAgeThresholdDays),
newAccountLabel: settings.newAccountLabel ?? "new-account",
commandRateLimitPolicy: normalizeCommandRateLimitPolicy(settings.commandRateLimitPolicy),
commandRateLimitMaxPerWindow: normalizePositiveIntWithDefault(settings.commandRateLimitMaxPerWindow, 20),
commandRateLimitAiMaxPerWindow: normalizePositiveIntWithDefault(settings.commandRateLimitAiMaxPerWindow, 5),
commandRateLimitWindowHours: normalizePositiveIntWithDefault(settings.commandRateLimitWindowHours, 24),
};
const db = getDb(env.DB);
await db
Expand Down Expand Up @@ -712,6 +724,10 @@ export async function upsertRepositorySettings(env: Env, settings: Partial<Repos
requireFreshRebaseWindowMinutes: resolved.requireFreshRebaseWindowMinutes,
accountAgeThresholdDays: resolved.accountAgeThresholdDays,
newAccountLabel: resolved.newAccountLabel,
commandRateLimitPolicy: resolved.commandRateLimitPolicy,
commandRateLimitMaxPerWindow: resolved.commandRateLimitMaxPerWindow,
commandRateLimitAiMaxPerWindow: resolved.commandRateLimitAiMaxPerWindow,
commandRateLimitWindowHours: resolved.commandRateLimitWindowHours,
updatedAt: nowIso(),
})
.onConflictDoUpdate({
Expand Down Expand Up @@ -770,6 +786,10 @@ export async function upsertRepositorySettings(env: Env, settings: Partial<Repos
requireFreshRebaseWindowMinutes: resolved.requireFreshRebaseWindowMinutes,
accountAgeThresholdDays: resolved.accountAgeThresholdDays,
newAccountLabel: resolved.newAccountLabel,
commandRateLimitPolicy: resolved.commandRateLimitPolicy,
commandRateLimitMaxPerWindow: resolved.commandRateLimitMaxPerWindow,
commandRateLimitAiMaxPerWindow: resolved.commandRateLimitAiMaxPerWindow,
commandRateLimitWindowHours: resolved.commandRateLimitWindowHours,
updatedAt: nowIso(),
},
});
Expand Down Expand Up @@ -2293,6 +2313,36 @@ export async function countRecentAuditEventsForActorAndTarget(env: Env, actor: s
return row.count;
}

/** Whether `deliveryId` has ALREADY been recorded for this (actor, eventType, targetKey) within `sinceIso` --
* makes a counting/rate-limit check idempotent against a REDELIVERED or retried webhook event (GitHub can
* and does redeliver the same issue_comment event), which would otherwise increment the counter twice for
* one real invocation and can incorrectly rate-limit it (#2560). Scoped to a short recent window, not the
* full rate-limit window -- a genuine redelivery lands within seconds, not hours later.
* Gate review finding: an earlier version matched deliveryId IN MEMORY over a `.limit(50)` slice with no
* ORDER BY -- once an actor accumulated more than 50 matching rows within the window (a burst/spam scenario,
* exactly what this feature exists to handle), the row carrying the original deliveryId could be excluded
* from that arbitrary slice, producing a false negative right when it matters most. The deliveryId match is
* now pushed into the SQL predicate itself (json_extract on metadataJson, mirroring
* countRecentDeadLettersByType's own json_extract usage below), so it's an exact match against every row in
* the window regardless of how many other rows exist for this actor/event/target. */
export async function hasAuditEventForDelivery(env: Env, actor: string, eventType: string, targetKey: string, deliveryId: string, sinceIso: string): Promise<boolean> {
const db = getDb(env.DB);
const [row] = await db
.select({ count: sql<number>`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. */
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 6 additions & 0 deletions src/db/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
});
Expand Down
20 changes: 20 additions & 0 deletions src/github/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<GittensoryMentionCommandName>([
"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;
Expand Down
4 changes: 4 additions & 0 deletions src/openapi/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
})
Expand Down
130 changes: 130 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import {
recordAgentCommandFeedback,
recordAuditEvent,
countRecentAuditEventsForActorAndTarget,
hasAuditEventForDelivery,
recordGateBlockOutcome,
getGateBlockOutcome,
isGlobalAgentFrozen,
Expand Down Expand Up @@ -138,6 +139,7 @@ import {
buildMaintainerQueueDigest,
buildPublicAgentCommandComment,
type GittensoryMentionCommandName,
isAiCostBearingCommand,
isAuthorizedCommandActor,
isMaintainerQueueDigestCommand,
parseAgentCommandFeedbackContext,
Expand Down Expand Up @@ -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<typeof resolveAgentActionMode>;
},
): Promise<boolean> {
/* 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,
Expand Down Expand Up @@ -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)
Expand Down
Loading
Loading