diff --git a/.gittensory.yml.example b/.gittensory.yml.example index 9305b656e4..4be51ad4fa 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -311,8 +311,20 @@ settings: # Bool. Default: false. agentDryRun: false - # Autonomy dial — per-action-class level (observe … auto). - # Map. Default: {} (= observe everywhere, deny-by-default). + # Autonomy dial — per-action-class level (observe … auto). Classes: review, request_changes, approve, + # merge, close, label, review_state_label, update_branch. Map. Default: {} (= observe everywhere, + # deny-by-default). + # + # `label` vs `review_state_label` (#label-scoping): these are SEPARATE, independent dials. + # - `label` alone no longer applies any planner-driven label by itself. The anti-abuse enforcement + # labels (blacklist/contributor-cap/review-nag) ride on the SAME dial as their accompanying close + # (`close`, below) — set `close: auto` and they close-and-label together with no `label` grant needed. + # - `review_state_label` gates the bot's own disposition-communication labels only: + # gittensory:ready-to-merge / gittensory:changes-requested / gittensory:needs-human-review / + # gittensory:migration-collision. These are advisory signals about the bot's own verdict, not + # enforcement — for a one-shot review model (merge/close/hold through the required gate check, no + # back-and-forth) leave this at the default `observe` so they never appear; set it to `auto` only if + # you specifically want that running commentary as GitHub labels. autonomy: {} # Auto-maintain policy for merges the agent is allowed to perform. @@ -342,6 +354,8 @@ settings: # contributorOpenIssueCap: 5 # Label applied to a PR/issue closed for exceeding a cap above. String. Default: over-contributor-limit. + # Gated on `autonomy.close` (#label-scoping), not `autonomy.label` — closing WITHOUT this label is + # available too: set this to explicit `null` (not just omitted) to close silently, with no label. # contributorCapLabel: over-contributor-limit # Cancel in-flight CI runs when a PR is auto-closed for exceeding contributorOpenPrCap above (#2462). @@ -359,6 +373,17 @@ settings: # reviewNagMaxPings: 3 # Positive integer. Pings above this within the cooldown window trigger the policy. Default: 3. # 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. + # # Gated on autonomy.close (#label-scoping); set to explicit `null` to + # # close/hold without any label. + + # Maintainer-mention nag moderation (#label-scoping): GitHub logins ALSO throttled under the SAME + # reviewNagPolicy/reviewNagMaxPings/reviewNagCooldownDays/reviewNagLabel cooldown above, on top of the + # bot's own @gittensory handle — e.g. a contributor who keeps tagging a specific maintainer for review + # instead of (or in addition to) pinging @gittensory. Counted independently per mentioned login and + # independently of the @gittensory counter. Only fires for the thread's OWN author, never a third party + # commenting on someone else's thread; owner/admin/automation-bot/autoCloseExemptLogins are always exempt. + # List of GitHub logins. Default: [] (no logins watched). + # reviewNagMonitoredMentions: [your-maintainer-login] # Shared repo-scoped exemption list (#2463): GitHub logins never throttled/closed by gittensory's # deterministic anti-abuse mechanisms (review-nag cooldown today; the per-contributor open-item cap diff --git a/apps/gittensory-ui/public/openapi.json b/apps/gittensory-ui/public/openapi.json index 4098054615..fb60fd61f6 100644 --- a/apps/gittensory-ui/public/openapi.json +++ b/apps/gittensory-ui/public/openapi.json @@ -8664,7 +8664,8 @@ "type": "string" }, "blacklistLabel": { - "type": "string" + "type": "string", + "nullable": true }, "createMissingLabel": { "type": "boolean" @@ -8817,6 +8818,16 @@ "auto_with_approval", "auto" ] + }, + "review_state_label": { + "type": "string", + "enum": [ + "observe", + "suggest", + "propose", + "auto_with_approval", + "auto" + ] } } }, @@ -8859,7 +8870,8 @@ "exclusiveMinimum": true }, "contributorCapLabel": { - "type": "string" + "type": "string", + "nullable": true }, "reviewNagPolicy": { "type": "string", @@ -8881,7 +8893,8 @@ "maximum": 365 }, "reviewNagLabel": { - "type": "string" + "type": "string", + "nullable": true }, "autoCloseExemptLogins": { "type": "array", @@ -8928,9 +8941,11 @@ "type": "string", "nullable": true }, - "contributorCapCancelCi": { - "type": "boolean", - "nullable": true + "reviewNagMonitoredMentions": { + "type": "array", + "items": { + "type": "string" + } }, "claGateMode": { "type": "string", @@ -8947,6 +8962,10 @@ "claCheckRunName": { "type": "string", "nullable": true + }, + "contributorCapCancelCi": { + "type": "boolean", + "nullable": true } }, "required": [ diff --git a/config/examples/README.md b/config/examples/README.md index 57e26b9d08..e5649e9e1d 100644 --- a/config/examples/README.md +++ b/config/examples/README.md @@ -57,9 +57,10 @@ per-repo file overlaid onto the global default: **replace wholesale** — a per-repo array is never concatenated with the global one. - An **explicit `null`** at a key in the per-repo file always overrides the global value there. This clears a setting wherever the manifest parser already treats an explicit `null` as - "off"/"clear" — e.g. `settings.contributorOpenPrCap`, `settings.contributorOpenIssueCap`, and - `settings.accountAgeThresholdDays` — and is a harmless no-op (equivalent to omitting the key) - everywhere else. + "off"/"clear" — e.g. `settings.contributorOpenPrCap`, `settings.contributorOpenIssueCap`, + `settings.accountAgeThresholdDays`, and the enforcement label names + (`settings.blacklistLabel`/`contributorCapLabel`/`reviewNagLabel`, see below) — and is a harmless + no-op (equivalent to omitting the key) everywhere else. - If either file fails to parse (or is malformed/oversized), the merge is skipped and the still-valid file is used alone; a still-good sibling's policy is never silently discarded just because the other file is broken. @@ -112,6 +113,52 @@ settings: - your-trusted-regular ``` +## Label autonomy scoping for one-shot review mode + +Two `autonomy` classes govern every label the bot can apply, and they are **independent**: + +- **`close`** authorizes the terminal merge/close/hold disposition **and** the anti-abuse + enforcement labels tied to it (blacklist/contributor-cap/review-nag) — a label like + `over-contributor-limit` is inseparable metadata on its close, so it never needs a separate grant. + Set `settings.contributorCapLabel`/`blacklistLabel`/`reviewNagLabel` to explicit `null` (not just + omitted) to close/hold **without** applying any label at all. +- **`review_state_label`** authorizes the bot's own disposition-communication labels only — + `gittensory:ready-to-merge` / `changes-requested` / `needs-human-review` / + `migration-collision`. These are advisory commentary about the bot's own verdict, not + enforcement, and default OFF like every autonomy class. **For a one-shot review model, leave this + at the default** so a PR merges, closes, or holds through the required gate check alone — set it + to `auto` only if you specifically want that commentary as GitHub labels too. + +```yaml +# .gittensory.yml (global default) — recommended one-shot baseline +settings: + autonomy: + close: auto + # review_state_label intentionally omitted (defaults to observe) +``` + +The broad `autonomy.label` class still exists but no longer gates any of the above — it is not +required for either family and applies to nothing on its own. + +## Maintainer-mention nag moderation + +`settings.reviewNagMonitoredMentions` extends the `@gittensory`-ping review-nag cooldown +(`reviewNagPolicy`/`reviewNagMaxPings`/`reviewNagCooldownDays`/`reviewNagLabel` — same settings, +one shared policy) to **also** throttle a thread's own author repeatedly @-mentioning a configured +maintainer login, counted independently per login and independently of the `@gittensory` counter: + +```yaml +# .gittensory.yml (global default) +settings: + reviewNagPolicy: hold + reviewNagMonitoredMentions: + - your-maintainer-login +``` + +Owner/admin/automation-bot logins and anyone on `autoCloseExemptLogins` are always exempt, and only +the thread's own author is ever throttled — a third party mentioning the login on someone else's +PR/issue never counts. + ## What belongs here vs. in the public `.gittensory.yml` - **Private config** (this directory): anti-abuse thresholds, the contributor cap, maintainer/ diff --git a/config/examples/global.gittensory.yml b/config/examples/global.gittensory.yml index 80818e2442..4c06388a68 100644 --- a/config/examples/global.gittensory.yml +++ b/config/examples/global.gittensory.yml @@ -21,6 +21,17 @@ gate: # Anti-abuse + presentation defaults shared by every repo. settings: + # Recommended one-shot-mode autonomy baseline (#label-scoping): `close` authorizes both the + # terminal merge/close/hold disposition AND every anti-abuse enforcement label (blacklist/ + # contributor-cap/review-nag) below — no separate `label` grant needed for those. + # `review_state_label` is left at the default `observe` so the bot's own advisory disposition + # labels (gittensory:ready-to-merge/changes-requested/needs-human-review) never appear — a + # one-shot review merges, closes, or holds through the required gate check alone, with no running + # commentary as GitHub labels. Set `review_state_label: auto` only if you specifically want that + # commentary too. + autonomy: + close: auto + # Max PRs a single non-owner/non-admin/non-bot contributor may have open at once, instance-wide # default. A per-repo file can override this per repo, or set it to `null` to disable it there. contributorOpenPrCap: 3 @@ -29,3 +40,12 @@ settings: # owner/admin/automation-bot exemption). Replace with your own maintainer/admin logins. autoCloseExemptLogins: - your-admin-login + + # Maintainer-mention nag moderation (#label-scoping): throttle a contributor who repeatedly tags a + # configured maintainer login for review/status, reusing the review-nag cooldown below. Replace with + # your own maintainer logins; empty/omitted (the default) watches nobody. + reviewNagPolicy: hold + reviewNagMaxPings: 3 + reviewNagCooldownDays: 5 + reviewNagMonitoredMentions: + - your-maintainer-login diff --git a/config/examples/repo-override.gittensory.yml b/config/examples/repo-override.gittensory.yml index 7b082f53f3..072e2d4686 100644 --- a/config/examples/repo-override.gittensory.yml +++ b/config/examples/repo-override.gittensory.yml @@ -25,3 +25,9 @@ settings: # for a small, high-trust repo where the instance-wide cap doesn't make sense. Omit this key # entirely (rather than setting it to `null`) to just inherit the global cap instead. contributorOpenPrCap: null + + # #label-scoping: an explicit `null` also works for the enforcement LABEL names (blacklistLabel/ + # contributorCapLabel/reviewNagLabel) — this repo still closes over-cap PRs (via the global + # `autonomy.close: auto`) but applies no label when doing so, e.g. because this repo's own + # `gittensor:*` label taxonomy already covers it. Omit to inherit the global/default label name. + contributorCapLabel: null diff --git a/migrations/0100_review_nag_monitored_mentions.sql b/migrations/0100_review_nag_monitored_mentions.sql new file mode 100644 index 0000000000..0b2b899d95 --- /dev/null +++ b/migrations/0100_review_nag_monitored_mentions.sql @@ -0,0 +1,6 @@ +-- Maintainer-mention nag moderation (#label-scoping): extends the existing @gittensory review-nag cooldown +-- (review_nag_policy/review_nag_max_pings/review_nag_cooldown_days/review_nag_label) to ALSO throttle a +-- contributor who repeatedly @-mentions a configured maintainer login, counted independently per mentioned +-- login and independently of the @gittensory counter. Default '[]' (no logins watched), so existing repos see +-- no behavior change until they opt in. +ALTER TABLE repository_settings ADD COLUMN review_nag_monitored_mentions_json TEXT NOT NULL DEFAULT '[]'; diff --git a/src/db/repositories.ts b/src/db/repositories.ts index cb4b0f3115..687882b1f2 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -511,6 +511,7 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise reviewNagMaxPings: 3, reviewNagCooldownDays: 5, reviewNagLabel: "review-nag-cooldown", + reviewNagMonitoredMentions: [], autoCloseExemptLogins: [], requireFreshRebaseWindowMinutes: null, accountAgeThresholdDays: null, @@ -571,6 +572,7 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise reviewNagMaxPings: normalizePositiveIntWithDefault(row.reviewNagMaxPings, 3), reviewNagCooldownDays: normalizeReviewNagCooldownDays(row.reviewNagCooldownDays, 5), reviewNagLabel: row.reviewNagLabel, + reviewNagMonitoredMentions: parseAutoCloseExemptLogins(row.reviewNagMonitoredMentionsJson), autoCloseExemptLogins: parseAutoCloseExemptLogins(row.autoCloseExemptLoginsJson), requireFreshRebaseWindowMinutes: normalizeOpenItemCap(row.requireFreshRebaseWindowMinutes), accountAgeThresholdDays: normalizeOpenItemCap(row.accountAgeThresholdDays), @@ -613,7 +615,11 @@ export async function upsertGlobalContributorBlacklist(env: Env, input: { contri } export async function upsertRepositorySettings(env: Env, settings: Partial & { repoFullName: string }): Promise { - const resolved: RepositorySettings = { + // `satisfies` (not a `: RepositorySettings` annotation) so the `?? default` coalescing below keeps its + // narrower inferred type (`string`, never `null`) for blacklistLabel/contributorCapLabel/reviewNagLabel -- + // the DB columns backing them stay NOT NULL (#label-scoping: only `.gittensory.yml`, not the dashboard/API + // write path, can express "close without any label" via an explicit null; see focus-manifest.ts). + const resolved = { repoFullName: settings.repoFullName, commentMode: settings.commentMode ?? "detected_contributors_only", publicAudienceMode: settings.publicAudienceMode ?? "oss_maintainer", @@ -663,6 +669,7 @@ export async function upsertRepositorySettings(env: Env, settings: Partial { + if (payload.action !== "created") return false; + const body = payload.comment?.body; + const repoFullName = payload.repository?.full_name; + const issue = payload.issue; + const installationId = getInstallationId(payload); + const commenter = payload.comment?.user?.login; + if (!body || !repoFullName || !issue || !installationId || !commenter) return false; + if (payload.comment?.user?.type === "Bot" || /\[bot\]$/i.test(commenter)) return false; + + const settings = await resolveRepositorySettings(env, repoFullName); + const monitoredLogins = settings.reviewNagMonitoredMentions ?? []; + if (monitoredLogins.length === 0) return false; + /* v8 ignore next -- resolveRepositorySettings always resolves a concrete "off"/"hold"/"close" (NOT NULL DEFAULT 'off'); the undefined side is defensive against the field's optional TS type. */ + const policy = settings.reviewNagPolicy ?? "off"; + if (policy === "off") return false; + + const threadAuthor = issue.user?.login; + if (!threadAuthor || commenter.toLowerCase() !== threadAuthor.toLowerCase()) return false; + + const repoOwner = repoFullName.includes("/") ? repoFullName.slice(0, repoFullName.indexOf("/")) : ""; + if (commenter.toLowerCase() === repoOwner.toLowerCase()) return false; + if (parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(commenter.toLowerCase())) return false; + if (isAutoCloseExempt(commenter, settings.autoCloseExemptLogins)) return false; + + const mentionedLogin = monitoredLogins.find((login) => bodyMentionsLogin(body, login)); + if (!mentionedLogin) return false; + + const targetKey = `${repoFullName}#${issue.number}#mention:${mentionedLogin.toLowerCase()}`; + /* 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); + const pingCount = priorPings + 1; + + await recordAuditEvent(env, { + eventType: MONITORED_MENTION_PING_EVENT_TYPE, + actor: commenter, + targetKey, + outcome: "completed", + detail: `ping ${pingCount}/${maxPings} within ${cooldownDays}d window (mentioned @${mentionedLogin})`, + metadata: { deliveryId, repoFullName, mentionedLogin }, + }).catch( + /* v8 ignore next -- fail-safe: an audit write failure never blocks the mention-command fallthrough */ + () => undefined, + ); + + if (pingCount <= maxPings) return false; + + const mode = resolveAgentActionMode({ + globalPaused: isGlobalAgentPause(env) || (await isGlobalAgentFrozen(env)), + agentPaused: settings.agentPaused, + agentDryRun: settings.agentDryRun, + }); + + if (policy === "hold" || !issue.pull_request) { + if (mode === "live") { + await createIssueComment( + env, + installationId, + repoFullName, + issue.number, + `@${commenter} this thread has reached the review-request cooldown limit for @${mentionedLogin} (${maxPings} pings within ${cooldownDays} days). Please wait for the cooldown window to pass before pinging @${mentionedLogin} 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.review_nag_cooldown_applied", + actor: "gittensory", + targetKey, + outcome: mode === "live" ? "completed" : "denied", + detail: `hold applied: ${commenter} pinged @${mentionedLogin} ${pingCount} times (limit ${maxPings})`, + metadata: { deliveryId, repoFullName, mode, policy, mentionedLogin }, + }).catch( + /* v8 ignore next -- fail-safe: an audit write failure never blocks the handler */ + () => undefined, + ); + return true; + } + + const pr = await getPullRequest(env, repoFullName, issue.number); + if (!pr || pr.state !== "open") return false; + + const planned = planAgentMaintenanceActions({ + conclusion: "skipped", + blockerTitles: [], + autonomy: settings.autonomy, + changedPaths: [], + hardGuardrailGlobs: [], + authorIsOwner: false, + authorIsAdmin: false, + authorIsAutomationBot: false, + ciState: "unverified", + reviewNagMatch: { matched: true, authorLogin: commenter, pingCount, maxPings }, + reviewNagLabel: settings.reviewNagLabel, + pr: { labels: pr.labels, headSha: pr.headSha }, + }); + if (planned.length === 0) { + await recordAuditEvent(env, { + eventType: "github_app.review_nag_cooldown_applied", + actor: "gittensory", + targetKey, + outcome: "denied", + detail: `close policy engaged but autonomy is not acting for label/close: ${commenter} pinged @${mentionedLogin} ${pingCount} times (limit ${maxPings})`, + metadata: { deliveryId, repoFullName, mode, policy, mentionedLogin }, + }).catch( + /* v8 ignore next -- fail-safe: an audit write failure never blocks the handler */ + () => undefined, + ); + return true; + } + + const installation = await getInstallation(env, installationId); + await executeAgentMaintenanceActions( + env, + { + installationId, + repoFullName, + pullNumber: pr.number, + headSha: pr.headSha, + autonomy: settings.autonomy, + agentPaused: settings.agentPaused, + agentDryRun: settings.agentDryRun, + installationPermissions: installation?.permissions ?? null, + authorLogin: pr.authorLogin, + }, + planned, + ); + 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"; diff --git a/src/services/agent-action-executor.ts b/src/services/agent-action-executor.ts index 0426c9e0ef..938c02233a 100644 --- a/src/services/agent-action-executor.ts +++ b/src/services/agent-action-executor.ts @@ -82,7 +82,13 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE const mode = resolveAgentActionMode({ globalPaused: isGlobalAgentPause(env) || (await isGlobalAgentFrozen(env)), agentPaused: ctx.agentPaused, agentDryRun: ctx.agentDryRun }); for (const action of planned) { - const autonomyLevel = resolveAutonomy(ctx.autonomy, action.actionClass); + // #label-scoping: a `label` action may be authorized by a class OTHER than `label` itself (an anti-abuse + // enforcement label rides on `close`; a disposition-communication label rides on `review_state_label`) — + // this durable re-check must resolve autonomy via the SAME class the planner actually used, not the + // literal GitHub-mutation kind, or a `label` action authorized via `close`/`review_state_label` would be + // wrongly re-denied against the (likely still-`observe`) generic `label` dial. Absent for every action + // whose `actionClass` already IS its own governing class (merge/close/approve/etc). + const autonomyLevel = resolveAutonomy(ctx.autonomy, action.autonomyClass ?? action.actionClass); const audit = (outcome: AgentActionOutcome["outcome"], detail: string) => { const auditOutcome = outcome === "dry_run" ? "completed" : outcome; outcomes.push({ actionClass: action.actionClass, outcome, detail }); @@ -305,7 +311,13 @@ export async function executeIssueMaintenanceActions(env: Env, ctx: IssueActionE const mode = resolveAgentActionMode({ globalPaused: isGlobalAgentPause(env) || (await isGlobalAgentFrozen(env)), agentPaused: ctx.agentPaused, agentDryRun: ctx.agentDryRun }); for (const action of planned) { - const autonomyLevel = resolveAutonomy(ctx.autonomy, action.actionClass); + // #label-scoping: a `label` action may be authorized by a class OTHER than `label` itself (an anti-abuse + // enforcement label rides on `close`; a disposition-communication label rides on `review_state_label`) — + // this durable re-check must resolve autonomy via the SAME class the planner actually used, not the + // literal GitHub-mutation kind, or a `label` action authorized via `close`/`review_state_label` would be + // wrongly re-denied against the (likely still-`observe`) generic `label` dial. Absent for every action + // whose `actionClass` already IS its own governing class (merge/close/approve/etc). + const autonomyLevel = resolveAutonomy(ctx.autonomy, action.autonomyClass ?? action.actionClass); const audit = (outcome: AgentActionOutcome["outcome"], detail: string) => { const auditOutcome = outcome === "dry_run" ? "completed" : outcome; outcomes.push({ actionClass: action.actionClass, outcome, detail }); diff --git a/src/settings/agent-actions.ts b/src/settings/agent-actions.ts index e5630a645f..c4b160c279 100644 --- a/src/settings/agent-actions.ts +++ b/src/settings/agent-actions.ts @@ -55,6 +55,14 @@ export function isProtectedAutomationAuthor(login: string | null | undefined): b export type PlannedAgentAction = { actionClass: AgentActionClass; + // #label-scoping: the autonomy class that actually AUTHORIZED this action, when it differs from `actionClass` + // (a `label` action can be authorized by `close` — an anti-abuse enforcement label inseparable from its + // close — or by `review_state_label` — the planner's own disposition-communication labels — rather than the + // generic `label` class). The executor's durable-pending-approval re-check MUST resolve autonomy via this + // field (falling back to `actionClass` when absent) so a later re-check re-verifies the SAME class the + // planner actually used, not a stale/unrelated `label` dial. Absent for every non-`label` action class, where + // `actionClass` already IS the governing autonomy class. + autonomyClass?: AgentActionClass; // auto_with_approval → the action is staged for a human approval (the #779 queue) instead of executing now. requiresApproval: boolean; reason: string; @@ -158,8 +166,10 @@ export type AgentActionPlanInput = { // context; the public close comment intentionally uses static copy. Absent / not-matched ⇒ no effect. blacklistMatch?: { matched: boolean; reason: string | null | undefined } | undefined; // The repo-configured label applied to a blacklisted author's PR (#1425), resolved from `.gittensory.yml`. - // Absent ⇒ the default (`DEFAULT_BLACKLIST_LABEL` = "slop"), so the disposition works regardless of the label set. - blacklistLabel?: string | undefined; + // Absent ⇒ the default (`DEFAULT_BLACKLIST_LABEL` = "slop"); explicit `null` ⇒ close WITHOUT any label + // (#label-scoping). Gated on `close` autonomy, NOT `label` (see the `blacklistMatch` block below) — the + // label is inseparable metadata on the close, never applied independently. + blacklistLabel?: string | null | undefined; // Per-contributor open-PR/open-issue cap (#2270, anti-abuse): when the incoming PR pushes its author over the // repo's configured `contributorOpenPrCap`, the disposition SHORT-CIRCUITS to a deterministic label + close // ahead of ALL merit/CI/AI analysis — same zero-hallucination shape as blacklistMatch, so its close is tagged @@ -175,8 +185,9 @@ export type AgentActionPlanInput = { // ("contributor_cap") and label either way — this is a description-only distinction, not a new disposition. contributorCapMatch?: { matched: boolean; authorLogin: string; openCount: number; cap: number; itemKind: "pull requests" | "issues"; scope?: "repository" | "install" | undefined } | undefined; // The repo-configured label applied to an over-cap author's PR/issue (#2270), resolved from `.gittensory.yml`. - // Absent ⇒ the default (`DEFAULT_CONTRIBUTOR_CAP_LABEL` = "over-contributor-limit"). - contributorCapLabel?: string | undefined; + // Absent ⇒ the default (`DEFAULT_CONTRIBUTOR_CAP_LABEL` = "over-contributor-limit"); explicit `null` ⇒ close + // WITHOUT any label (#label-scoping). Gated on `close` autonomy, NOT `label` — same shape as {@link blacklistLabel}. + contributorCapLabel?: string | null | undefined; // Review-nag cooldown (#2463, anti-abuse): when the PR author has pinged `@gittensory` past the repo's // configured threshold within the cooldown window AND the repo's `reviewNagPolicy` is `"close"`, the // disposition SHORT-CIRCUITS to a deterministic label + close ahead of ALL merit/CI/AI analysis — same @@ -186,8 +197,9 @@ export type AgentActionPlanInput = { // the resolved "yes, close this PR" verdict. Absent / not-matched ⇒ no effect. reviewNagMatch?: { matched: boolean; authorLogin: string; pingCount: number; maxPings: number } | undefined; // The repo-configured label applied to a review-nag-closed PR (#2463), resolved from `.gittensory.yml`. - // Absent ⇒ the default (`DEFAULT_REVIEW_NAG_LABEL` = "review-nag-cooldown"). - reviewNagLabel?: string | undefined; + // Absent ⇒ the default (`DEFAULT_REVIEW_NAG_LABEL` = "review-nag-cooldown"); explicit `null` ⇒ close WITHOUT + // any label (#label-scoping). Gated on `close` autonomy, NOT `label` — same shape as {@link blacklistLabel}. + reviewNagLabel?: string | null | undefined; // Flag-then-close double-check for the linked-issue hard rule (#linked-issue-verify-before-close). When // `verifyBeforeClose` is true (the default), a violation FLAGS the PR (pending-closure label + warning comment) // on first detection and only CLOSES on a LATER evaluation when the violation STILL holds AND the PR already @@ -246,6 +258,7 @@ export function downgradeMergeToHold(planned: PlannedAgentAction[], holdOnly: bo if (!alreadyNeedsReview) { next.push({ actionClass: "label", + autonomyClass: "review_state_label", requiresApproval: stagedMerge?.requiresApproval ?? false, reason: "accuracy circuit-breaker engaged (merge precision dropped) — would-merge held for human review", label: AGENT_LABEL_NEEDS_REVIEW, @@ -288,6 +301,7 @@ export function downgradeCloseToHold(planned: PlannedAgentAction[], closeHoldOnl if (!alreadyNeedsReview) { next.push({ actionClass: "label", + autonomyClass: "review_state_label", requiresApproval: droppedClose?.requiresApproval ?? false, reason: "close-precision circuit-breaker engaged — would-close held for human review", label: AGENT_LABEL_NEEDS_REVIEW, @@ -354,8 +368,11 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne // static by construction so private maintainer metadata from the blacklist entry cannot leak. const blacklistContributor = !input.authorIsOwner && !input.authorIsAdmin && !input.authorIsAutomationBot; if (input.blacklistMatch?.matched === true && blacklistContributor) { - const label = input.blacklistLabel ?? DEFAULT_BLACKLIST_LABEL; - if (acting("label")) actions.push({ actionClass: "label", requiresApproval: approval("label"), reason: "blacklisted contributor", label, labelOp: "add" }); + // #label-scoping: this label is inseparable metadata on the close below, so it rides on `close` autonomy, + // NOT the generic `label` class — a repo can enable close without also opting into the broad label dial. + // Explicit `null` (vs. absent/undefined) means "close without any label." + const label = input.blacklistLabel === null ? null : (input.blacklistLabel ?? DEFAULT_BLACKLIST_LABEL); + if (acting("close") && label !== null) actions.push({ actionClass: "label", autonomyClass: "close", requiresApproval: approval("close"), reason: "blacklisted contributor", label, labelOp: "add" }); if (acting("close")) { actions.push({ actionClass: "close", @@ -378,8 +395,9 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne const capContributor = !input.authorIsOwner && !input.authorIsAdmin && !input.authorIsAutomationBot; if (input.contributorCapMatch?.matched === true && capContributor) { const { authorLogin, openCount, cap, itemKind, scope } = input.contributorCapMatch; - const label = input.contributorCapLabel ?? DEFAULT_CONTRIBUTOR_CAP_LABEL; - if (acting("label")) actions.push({ actionClass: "label", requiresApproval: approval("label"), reason: "over the per-contributor open-item cap", label, labelOp: "add" }); + // #label-scoping: same close-autonomy-gated, null-clearable shape as the blacklist label above. + const label = input.contributorCapLabel === null ? null : (input.contributorCapLabel ?? DEFAULT_CONTRIBUTOR_CAP_LABEL); + if (acting("close") && label !== null) actions.push({ actionClass: "label", autonomyClass: "close", requiresApproval: approval("close"), reason: "over the per-contributor open-item cap", label, labelOp: "add" }); if (acting("close")) { actions.push({ actionClass: "close", @@ -400,8 +418,9 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne const reviewNagContributor = !input.authorIsOwner && !input.authorIsAdmin && !input.authorIsAutomationBot; if (input.reviewNagMatch?.matched === true && reviewNagContributor) { const { authorLogin, pingCount, maxPings } = input.reviewNagMatch; - const label = input.reviewNagLabel ?? DEFAULT_REVIEW_NAG_LABEL; - if (acting("label")) actions.push({ actionClass: "label", requiresApproval: approval("label"), reason: "review-nag cooldown", label, labelOp: "add" }); + // #label-scoping: same close-autonomy-gated, null-clearable shape as the blacklist label above. + const label = input.reviewNagLabel === null ? null : (input.reviewNagLabel ?? DEFAULT_REVIEW_NAG_LABEL); + if (acting("close") && label !== null) actions.push({ actionClass: "label", autonomyClass: "close", requiresApproval: approval("close"), reason: "review-nag cooldown", label, labelOp: "add" }); if (acting("close")) { actions.push({ actionClass: "close", @@ -502,8 +521,9 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne const pendingClosureLabelPresent = hasLabel(input.pr.labels, AGENT_LABEL_PENDING_CLOSURE); // Pass 1 is only safe when the pending-closure state can be written immediately. If labels are disabled or // approval-gated, holding would fail open because Pass 2 is keyed on a label that cannot appear yet; fall back - // to the original immediate close in that case. - const canApplyPendingClosureFlagNow = acting("label") && !approval("label"); + // to the original immediate close in that case. #label-scoping: this label lives in the review_state_label + // family (see below), so its readiness check must use the SAME class the actual push is gated on. + const canApplyPendingClosureFlagNow = acting("review_state_label") && !approval("review_state_label"); // Pass 1 — violation present, verify-mode on, label NOT yet on the PR, and the state label can be applied now // → FLAG (label + comment), do NOT close. const flagForLinkedIssue = linkedIssueViolated && verifyBeforeClose && !pendingClosureLabelPresent && canApplyPendingClosureFlagNow; @@ -523,11 +543,14 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne ? "CI could not be verified" : ""; - // 1) label — ready-to-merge (review-good, unguarded) / needs-human-review (review-good but guarded) / - // changes-requested (not review-good → will be closed for a contributor, held for the owner). A pending - // linked-issue hard-rule close (flag OR close pass) forces the changes-requested label regardless of the gate - // verdict (the PR is about to be closed for an ineligible linked issue). Idempotent. - if (acting("label")) { + // 1) review_state_label (#label-scoping) — ready-to-merge (review-good, unguarded) / needs-human-review + // (review-good but guarded) / changes-requested (not review-good → will be closed for a contributor, held for + // the owner). A pending linked-issue hard-rule close (flag OR close pass) forces the changes-requested label + // regardless of the gate verdict (the PR is about to be closed for an ineligible linked issue). Idempotent. + // Gated on the DEDICATED `review_state_label` class, not the generic `label` — these are the bot's own + // disposition-communication labels (advisory, not enforcement), default OFF like every autonomy class so a + // one-shot-mode repo never sees `gittensory:changes-requested`/`needs-human-review` without an explicit opt-in. + if (acting("review_state_label")) { // A live migration-collision hold takes priority over a plain guardrail hold when both are true — it is // the more specific, actionable signal (tells the contributor exactly what to do: rebase), and gets its // own distinct label (#2550) so an operator can filter/alert on it separately from an ordinary guardrail. @@ -544,7 +567,8 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne if (!hasLabel(input.pr.labels, label)) { actions.push({ actionClass: "label", - requiresApproval: approval("label"), + autonomyClass: "review_state_label", + requiresApproval: approval("review_state_label"), reason, label, // Only the migration-collision hold carries a comment here — the guardrail/ready/changes labels never @@ -560,7 +584,8 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne const window = closeDelaySeconds > 0 ? `~${closeDelaySeconds}s` : "the next verification"; actions.push({ actionClass: "label", - requiresApproval: approval("label"), + autonomyClass: "review_state_label", + requiresApproval: approval("review_state_label"), reason: `linked-issue hard rule (flagged for verification): ${ruleReason}`, label: AGENT_LABEL_PENDING_CLOSURE, labelOp: "add", @@ -571,7 +596,8 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne if (clearLinkedIssueFlag) { actions.push({ actionClass: "label", - requiresApproval: approval("label"), + autonomyClass: "review_state_label", + requiresApproval: approval("review_state_label"), reason: "linked-issue hard rule resolved — clearing the pending-closure flag", label: AGENT_LABEL_PENDING_CLOSURE, labelOp: "remove", diff --git a/src/settings/autonomy.ts b/src/settings/autonomy.ts index ed3f4e6f5f..0b600fa7b0 100644 --- a/src/settings/autonomy.ts +++ b/src/settings/autonomy.ts @@ -4,8 +4,12 @@ import type { AgentActionClass, AutoMaintainPolicy, AutoMergeMethod, AutonomyLev // this BEFORE acting. `observe` is the deny-by-default floor — gittensory watches but never takes an action. export const AUTONOMY_LEVELS = ["observe", "suggest", "propose", "auto_with_approval", "auto"] as const; -// The write-action classes the maintainer auto-maintain layer (#778) can take on a PR. -export const AGENT_ACTION_CLASSES = ["review", "request_changes", "approve", "merge", "close", "label", "update_branch"] as const; +// The write-action classes the maintainer auto-maintain layer (#778) can take on a PR. `review_state_label` +// (#label-scoping) is a separate class from `label`: it gates the planner's own disposition-communication +// labels (ready-to-merge / changes-requested / needs-human-review / migration-collision / pending-closure / +// new-account), independent of the anti-abuse enforcement labels (blacklist/contributor-cap/review-nag), which +// ride on `close` instead -- see agent-actions.ts. +export const AGENT_ACTION_CLASSES = ["review", "request_changes", "approve", "merge", "close", "label", "review_state_label", "update_branch"] as const; // Deny-by-default: any action class with no explicit, valid level resolves to this. export const DEFAULT_AUTONOMY_LEVEL: AutonomyLevel = "observe"; diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index 98da3f8e59..96e2a80827 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -181,6 +181,7 @@ export type FocusManifestSettings = Partial< | "reviewNagMaxPings" | "reviewNagCooldownDays" | "reviewNagLabel" + | "reviewNagMonitoredMentions" | "autoCloseExemptLogins" | "accountAgeThresholdDays" | "newAccountLabel" @@ -928,8 +929,15 @@ function parseSettingsOverride(value: JsonValue | undefined, warnings: string[]) if (aiReviewModel !== null) out.aiReviewModel = aiReviewModel; const gittensorLabel = normalizeOptionalString(r.gittensorLabel, "settings.gittensorLabel", warnings); if (gittensorLabel !== null) out.gittensorLabel = gittensorLabel; - const blacklistLabel = normalizeOptionalString(r.blacklistLabel, "settings.blacklistLabel", warnings); - if (blacklistLabel !== null) out.blacklistLabel = blacklistLabel; + // #label-scoping: an explicit yml `null` is load-bearing (closes WITHOUT any label), matching + // contributorOpenPrCap's own null-vs-omitted distinction — must be checked BEFORE normalizeOptionalString, + // which otherwise collapses null and undefined to the same "unset" result. + if (r.blacklistLabel === null) { + out.blacklistLabel = null; + } else { + const blacklistLabel = normalizeOptionalString(r.blacklistLabel, "settings.blacklistLabel", warnings); + if (blacklistLabel !== null) out.blacklistLabel = blacklistLabel; + } const publicSurface = normalizeOptionalEnum(r.publicSurface, "settings.publicSurface", ["off", "comment_and_label", "comment_only", "label_only"] as const, warnings); if (publicSurface !== null) out.publicSurface = publicSurface; for (const key of ["aiReviewByok", "aiReviewAllAuthors", "closeOwnerAuthors", "autoLabelEnabled", "badgeEnabled", "createMissingLabel", "includeMaintainerAuthors", "requireLinkedIssue", "backfillEnabled", "privateTrustEnabled", "agentPaused", "agentDryRun"] as const) { @@ -993,8 +1001,13 @@ function parseSettingsOverride(value: JsonValue | undefined, warnings: string[]) const contributorOpenIssueCap = normalizeOptionalPositiveInteger(r.contributorOpenIssueCap, "settings.contributorOpenIssueCap", warnings); if (contributorOpenIssueCap !== null) out.contributorOpenIssueCap = contributorOpenIssueCap; } - const contributorCapLabel = normalizeOptionalString(r.contributorCapLabel, "settings.contributorCapLabel", warnings); - if (contributorCapLabel !== null) out.contributorCapLabel = contributorCapLabel; + // #label-scoping: same load-bearing-null idiom as blacklistLabel above. + if (r.contributorCapLabel === null) { + out.contributorCapLabel = null; + } else { + const contributorCapLabel = normalizeOptionalString(r.contributorCapLabel, "settings.contributorCapLabel", warnings); + if (contributorCapLabel !== null) out.contributorCapLabel = contributorCapLabel; + } // CI-run cancellation on a contributor_cap close (#2462): an explicit yml `null` is load-bearing (clears a // DB-configured value back to "unset", falling through to the CONTRIBUTOR_CAP_CANCEL_CI_DEFAULT env var), // matching contributorOpenPrCap's own null-vs-omitted distinction above. @@ -1014,8 +1027,22 @@ function parseSettingsOverride(value: JsonValue | undefined, warnings: string[]) 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; + // #label-scoping: same load-bearing-null idiom as blacklistLabel above. + if (r.reviewNagLabel === null) { + out.reviewNagLabel = null; + } else { + const reviewNagLabel = normalizeOptionalString(r.reviewNagLabel, "settings.reviewNagLabel", warnings); + if (reviewNagLabel !== null) out.reviewNagLabel = reviewNagLabel; + } + // Maintainer-mention nag moderation (#label-scoping): GitHub logins ALSO throttled under the review-nag + // cooldown above, on top of the bot's own @gittensory handle. Only set it when at least one VALID login + // survives normalization, so a malformed block never blanks the DB-configured list via the resolver's + // `{...dbSettings, ...manifest.settings}` overlay (same reasoning as autoCloseExemptLogins below). + if (r.reviewNagMonitoredMentions !== undefined) { + const { logins: monitoredMentions, warnings: monitoredMentionWarnings } = normalizeAutoCloseExemptLogins(r.reviewNagMonitoredMentions); + warnings.push(...monitoredMentionWarnings); + if (monitoredMentions.length > 0) out.reviewNagMonitoredMentions = monitoredMentions; + } // Shared repo-scoped exemption list (#2463): only set it when at least one VALID login survives // normalization, so a malformed block never blanks the DB-configured list via the resolver's overlay. if (r.autoCloseExemptLogins !== undefined) { diff --git a/src/types.ts b/src/types.ts index b2db7d69dd..ccce3f3e49 100644 --- a/src/types.ts +++ b/src/types.ts @@ -688,9 +688,11 @@ export type RepositorySettings = { contributorBlacklist?: ContributorBlacklistEntry[] | undefined; /** The label applied to a blacklisted contributor's PR (#1425). Configurable per-repo (dashboard/DB + * `.gittensory.yml` `settings.blacklistLabel`); defaults to `"slop"` so the disposition works regardless of - * the label a repo sets. Always populated by the DB layer (default `"slop"`); optional so existing settings - * fixtures/callers need not be touched (mirrors the sibling `contributorBlacklist`). */ - blacklistLabel?: string | undefined; + * the label a repo sets. Explicit `null` closes WITHOUT applying any label (the same load-bearing-null idiom + * as {@link contributorOpenPrCap}) -- distinct from omitted/undefined, which uses the default. Always + * populated by the DB layer (default `"slop"`); optional so existing settings fixtures/callers need not be + * touched (mirrors the sibling `contributorBlacklist`). */ + blacklistLabel?: string | null | undefined; /** Per-contributor open-PR cap (#2270, anti-abuse): the max PRs a single non-owner/admin/bot contributor may * have open on this repo at once. `null`/absent (default) = no cap, byte-identical to today. Layered like * every other settings field (`.gittensory.yml` `settings.contributorOpenPrCap` > DB > `null`). Enforcement @@ -700,10 +702,11 @@ export type RepositorySettings = { * applied to open issues instead of open PRs. `null`/absent (default) = no cap. */ contributorOpenIssueCap?: number | null | undefined; /** The label applied to a PR/issue closed for exceeding a per-contributor open-item cap (#2270). Same - * configurable-with-fallback shape as {@link blacklistLabel}; defaults to `"over-contributor-limit"` so the - * disposition works regardless of the label a repo sets. Always populated by the DB layer; optional so - * existing settings fixtures/callers need not be touched. */ - contributorCapLabel?: string | undefined; + * configurable-with-fallback shape as {@link blacklistLabel} (including the explicit-`null`-closes-without-a- + * label idiom); defaults to `"over-contributor-limit"` so the disposition works regardless of the label a + * repo sets. Always populated by the DB layer; optional so existing settings fixtures/callers need not be + * touched. */ + contributorCapLabel?: string | null | undefined; /** Cancel in-flight CI runs on a contributor_cap close (#2462, anti-abuse): when true, after a PR is * auto-closed for exceeding {@link contributorOpenPrCap}, gittensory lists and cancels that PR's * in-progress/queued Actions runs at its head SHA. Requires the App installation to have granted @@ -729,9 +732,19 @@ export type RepositorySettings = { * touched. */ reviewNagCooldownDays?: number | undefined; /** The label applied to a thread closed for review-nag cooldown (#2463), mirroring {@link blacklistLabel}'s - * configurable-with-fallback shape. Always populated by the DB layer (default `"review-nag-cooldown"`); - * optional so existing settings fixtures/callers need not be touched. */ - reviewNagLabel?: string | undefined; + * configurable-with-fallback shape (including the explicit-`null`-closes-without-a-label idiom). Always + * populated by the DB layer (default `"review-nag-cooldown"`); optional so existing settings + * fixtures/callers need not be touched. */ + reviewNagLabel?: string | null | undefined; + /** Maintainer-mention nag moderation: GitHub logins to ALSO throttle under the review-nag cooldown when the + * thread author repeatedly @-mentions them (on top of the bot's own `@gittensory` handle) -- e.g. a + * maintainer login instead of the bot, for a contributor who keeps tagging a specific person for review. + * Counted independently per mentioned login and independently of the `@gittensory` counter, but reuses the + * SAME {@link reviewNagPolicy}/{@link reviewNagMaxPings}/{@link reviewNagCooldownDays}/{@link reviewNagLabel} + * thresholds/action/label -- one cooldown policy, multiple watched mention targets. `[]`/undefined (default) + * = no logins watched, zero behavior change. Never fires for the repo owner, admin logins, automation bots, + * or a login on {@link autoCloseExemptLogins}. */ + reviewNagMonitoredMentions?: string[] | undefined; /** Shared repo-scoped exemption list (#2463, anti-abuse): GitHub logins that are NEVER throttled or closed by * gittensory's deterministic anti-abuse mechanisms (review-nag and the per-contributor open-item cap above), * on top of the standing owner/admin/automation-bot exemption. Always populated by the DB layer (default @@ -818,8 +831,15 @@ export type ContributorBlacklistEntry = { * executing; `auto_with_approval` executes behind a human approval gate (#779); `auto` executes directly. */ export type AutonomyLevel = "observe" | "suggest" | "propose" | "auto_with_approval" | "auto"; -/** The write-action classes the maintainer auto-maintain layer (#778) can take on a PR. */ -export type AgentActionClass = "review" | "request_changes" | "approve" | "merge" | "close" | "label" | "update_branch"; +/** The write-action classes the maintainer auto-maintain layer (#778) can take on a PR. `label` gates the + * anti-abuse enforcement labels tied 1:1 to a `close` in the same disposition (blacklist/contributor-cap/ + * review-nag) -- those additionally require `close` to be acting, so `label` alone can't apply them without a + * close. `review_state_label` is a SEPARATE, independent gate for the planner's own disposition-communication + * labels (ready-to-merge / changes-requested / needs-human-review / migration-collision / the linked-issue + * pending-closure flag / the account-age new-account label) -- these are advisory signals about the bot's own + * verdict, not enforcement actions, and default OFF (`observe`) like every other class so a one-shot-mode repo + * never sees them without an explicit opt-in. */ +export type AgentActionClass = "review" | "request_changes" | "approve" | "merge" | "close" | "label" | "review_state_label" | "update_branch"; /** Per-action-class autonomy. An unset class resolves to `observe` (deny-by-default). */ export type AutonomyPolicy = Partial>; diff --git a/test/unit/agent-action-executor.test.ts b/test/unit/agent-action-executor.test.ts index ecefb3d9a7..b0e73d358e 100644 --- a/test/unit/agent-action-executor.test.ts +++ b/test/unit/agent-action-executor.test.ts @@ -114,6 +114,24 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { expect((await auditFor(env, "merge"))?.outcome).toBe("completed"); }); + it("#label-scoping: a label action's autonomyClass (not the literal actionClass) governs the durable re-check", async () => { + const env = createTestEnv({}); + // autonomy.label is OFF; autonomy.close is ON — a label authorized via autonomyClass: "close" must still + // execute, proving the executor resolves autonomy via `autonomyClass ?? actionClass`, not `actionClass` alone. + const enforcementLabel: PlannedAgentAction = { actionClass: "label", autonomyClass: "close", requiresApproval: false, reason: "blacklisted contributor", label: "slop", labelOp: "add" }; + const outcomes = await executeAgentMaintenanceActions(env, ctx({ autonomy: { label: "observe", close: "auto" } }), [enforcementLabel]); + expect(outcomes[0]?.outcome).toBe("completed"); + expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "slop", { createMissingLabel: true }); + }); + + it("#label-scoping: a label action with autonomyClass: close is DENIED when close is not acting, even if the generic label class is on", async () => { + const env = createTestEnv({}); + const enforcementLabel: PlannedAgentAction = { actionClass: "label", autonomyClass: "close", requiresApproval: false, reason: "blacklisted contributor", label: "slop", labelOp: "add" }; + const outcomes = await executeAgentMaintenanceActions(env, ctx({ autonomy: { label: "auto", close: "observe" } }), [enforcementLabel]); + expect(outcomes[0]?.outcome).toBe("denied"); + expect(ensurePullRequestLabel).not.toHaveBeenCalled(); + }); + it("REGRESSION (#2424): LIVE update_branch falls back to ctx.headSha when the action carries no expectedHeadSha of its own", async () => { // The `updateBranch` fixture above is pre-pinned (expectedHeadSha: "sha7"), so the big LIVE test never // exercises the `?? ctx.headSha` fallback -- it's parity with approve/merge for the tiny window between diff --git a/test/unit/agent-actions.test.ts b/test/unit/agent-actions.test.ts index 7eb6b5efbb..682d09405d 100644 --- a/test/unit/agent-actions.test.ts +++ b/test/unit/agent-actions.test.ts @@ -28,36 +28,45 @@ describe("planAgentMaintenanceActions (#778)", () => { expect(planAgentMaintenanceActions(input({ conclusion: "skipped", autonomy: { approve: "auto" } }))).toEqual([]); // neutral = advisory-only blockers → NON-blocking: flows to the disposition, earns a label (clean+green here), // and is NEVER left silently undecided or auto-closed. (#harm-stop neutral-silent-stuck) - const neutral = classes(planAgentMaintenanceActions(input({ conclusion: "neutral", autonomy: { merge: "auto", label: "auto", close: "auto" } }))); + const neutral = classes(planAgentMaintenanceActions(input({ conclusion: "neutral", autonomy: { merge: "auto", review_state_label: "auto", close: "auto" } }))); expect(neutral).not.toEqual([]); expect(neutral).not.toContain("close"); }); it("plans nothing when every class is at a non-acting level", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { label: "suggest", request_changes: "propose", close: "observe" }, blockerTitles: ["x"] })); + const plan = planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { review_state_label: "suggest", request_changes: "propose", close: "observe" }, blockerTitles: ["x"] })); expect(plan).toEqual([]); }); it("labels by verdict bucket and is idempotent when the label already exists", () => { - expect(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { label: "auto" }, blockerTitles: ["x"] }))[0]).toMatchObject({ actionClass: "label", label: AGENT_LABEL_CHANGES }); - expect(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto" } }))[0]).toMatchObject({ actionClass: "label", label: AGENT_LABEL_READY }); + expect(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { review_state_label: "auto" }, blockerTitles: ["x"] }))[0]).toMatchObject({ actionClass: "label", label: AGENT_LABEL_CHANGES }); + expect(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { review_state_label: "auto" } }))[0]).toMatchObject({ actionClass: "label", label: AGENT_LABEL_READY }); // already labeled → not re-planned - expect(classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto" }, pr: { labels: [AGENT_LABEL_READY] } })))).not.toContain("label"); + expect(classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { review_state_label: "auto" }, pr: { labels: [AGENT_LABEL_READY] } })))).not.toContain("label"); + }); + + it("#label-scoping: the verdict-bucket label carries autonomyClass: review_state_label, is OFF under the broad label class, and defaults OFF entirely", () => { + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { review_state_label: "auto" } })); + expect(plan[0]).toMatchObject({ actionClass: "label", autonomyClass: "review_state_label", label: AGENT_LABEL_READY }); + // The broad `label` class alone no longer authorizes this — one-shot mode never sees it without explicit opt-in. + expect(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto" } }))).toEqual([]); + // No autonomy configured at all → nothing (deny-by-default). + expect(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: {} }))).toEqual([]); }); it("NEVER posts a formal request_changes; a blocking contributor PR closes (close acting) and is always labeled", () => { // close acting → CLOSE (no formal request_changes review that would block the PR) - const withClose = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto", label: "auto" }, blockerTitles: ["Missing linked issue", "Slop risk"] }))); + const withClose = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto", review_state_label: "auto" }, blockerTitles: ["Missing linked issue", "Slop risk"] }))); expect(withClose).toContain("close"); expect(withClose).not.toContain("request_changes"); // close NOT acting → just the changes-requested LABEL, never a formal request_changes (which would strand the PR). - const noClose = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { label: "auto" }, blockerTitles: ["x"] }))); + const noClose = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { review_state_label: "auto" }, blockerTitles: ["x"] }))); expect(noClose).toContain("label"); expect(noClose).not.toContain("request_changes"); }); it("an action_required verdict is HELD — never request_changes, never closed (awaiting action ≠ failure)", () => { - const plan = classes(planAgentMaintenanceActions(input({ conclusion: "action_required", autonomy: { request_changes: "auto", close: "auto", label: "auto" }, blockerTitles: [] }))); + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "action_required", autonomy: { request_changes: "auto", close: "auto", review_state_label: "auto" }, blockerTitles: [] }))); expect(plan).not.toContain("request_changes"); // awaiting-action (e.g. a fork's CI awaiting approval) → HELD + labeled, NOT a one-shot close. (#harm-stop) expect(plan).not.toContain("close"); @@ -253,7 +262,7 @@ describe("planAgentMaintenanceActions (#778)", () => { it("orders actions least → most irreversible (label, review, disposition)", () => { // requireApprovals:0 lets merge fire while reviewDecision is still unset, so approve fires too. const plan = planAgentMaintenanceActions( - input({ conclusion: "success", autonomy: { label: "auto", approve: "auto", merge: "auto" }, autoMaintain: { requireApprovals: 0, mergeMethod: "squash" }, pr: { labels: [], mergeableState: "clean" } }), + input({ conclusion: "success", autonomy: { review_state_label: "auto", approve: "auto", merge: "auto" }, autoMaintain: { requireApprovals: 0, mergeMethod: "squash" }, pr: { labels: [], mergeableState: "clean" } }), ); expect(classes(plan)).toEqual(["label", "approve", "merge"]); }); @@ -293,7 +302,7 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("still labels a guarded PR (the reversible action is unaffected — it just falls to a human)", () => { - const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto", merge: "auto" }, ...guarded, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } }))); + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { review_state_label: "auto", merge: "auto" }, ...guarded, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } }))); expect(plan).toContain("label"); expect(plan).not.toContain("merge"); }); @@ -301,7 +310,7 @@ describe("planAgentMaintenanceActions (#778)", () => { it("labels a guarded passing PR `needs-human-review` (NOT `ready-to-merge`) and still does not merge it", () => { // A guardrail-hit PR that otherwise passes is withheld from auto-merge → the `ready-to-merge` label // would be misleading. It must carry the distinct `needs-human-review` label instead, and never merge. - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto", merge: "auto" }, ...guarded, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { review_state_label: "auto", merge: "auto" }, ...guarded, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); const label = plan.find((a) => a.actionClass === "label"); expect(label?.label).toBe(AGENT_LABEL_NEEDS_REVIEW); expect(label?.label).not.toBe(AGENT_LABEL_READY); @@ -310,17 +319,17 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("does not re-plan the needs-human-review label when the guarded PR already carries it (idempotent)", () => { - const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto" }, ...guarded, pr: { labels: [AGENT_LABEL_NEEDS_REVIEW] } }))); + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { review_state_label: "auto" }, ...guarded, pr: { labels: [AGENT_LABEL_NEEDS_REVIEW] } }))); expect(plan).not.toContain("label"); }); it("a guarded BLOCKING PR keeps the changes-requested label (not needs-human-review)", () => { - const label = planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { label: "auto" }, blockerTitles: ["x"], ...guarded, pr: { labels: [] } })).find((a) => a.actionClass === "label"); + const label = planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { review_state_label: "auto" }, blockerTitles: ["x"], ...guarded, pr: { labels: [] } })).find((a) => a.actionClass === "label"); expect(label?.label).toBe(AGENT_LABEL_CHANGES); }); it("still auto-merges when the changed paths do NOT match any guardrail glob", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto", merge: "auto" }, changedPaths: ["docs/readme.md", "src/ui/button.tsx"], hardGuardrailGlobs: ["src/scoring/**", "scripts/**"], pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { review_state_label: "auto", merge: "auto" }, changedPaths: ["docs/readme.md", "src/ui/button.tsx"], hardGuardrailGlobs: ["src/scoring/**", "scripts/**"], pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); expect(classes(plan)).toContain("merge"); // A clean, non-guarded passing PR keeps the `ready-to-merge` label (the auto-merge it promises happens). expect(plan.find((a) => a.actionClass === "label")?.label).toBe(AGENT_LABEL_READY); @@ -336,7 +345,7 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("labels the PR gittensory:migration-collision (NOT needs-human-review or ready-to-merge) with the live-collision reason", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto", merge: "auto" }, ...collided, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { review_state_label: "auto", merge: "auto" }, ...collided, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); const label = plan.find((a) => a.actionClass === "label"); expect(label?.label).toBe(AGENT_LABEL_MIGRATION_COLLISION); expect(label?.label).not.toBe(AGENT_LABEL_NEEDS_REVIEW); @@ -346,37 +355,37 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("attaches the rebase-needed comment to the migration-collision label action", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto" }, ...collided, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { review_state_label: "auto" }, ...collided, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); const label = plan.find((a) => a.actionClass === "label"); expect(label?.comment).toContain("Please rebase"); }); it("does not attach a comment for the ordinary guardrail hold (only migration-collision carries one)", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto" }, changedPaths: ["src/scoring/model.ts"], hardGuardrailGlobs: ["src/scoring/**"], pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { review_state_label: "auto" }, changedPaths: ["src/scoring/model.ts"], hardGuardrailGlobs: ["src/scoring/**"], pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); const label = plan.find((a) => a.actionClass === "label"); expect(label?.label).toBe(AGENT_LABEL_NEEDS_REVIEW); expect(label?.comment).toBeUndefined(); }); it("does not re-plan the migration-collision label when the PR already carries it (idempotent)", () => { - const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto" }, ...collided, pr: { labels: [AGENT_LABEL_MIGRATION_COLLISION] } }))); + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { review_state_label: "auto" }, ...collided, pr: { labels: [AGENT_LABEL_MIGRATION_COLLISION] } }))); expect(plan).not.toContain("label"); }); it("a BLOCKING PR keeps the changes-requested label even with a migration collision present (blocker wins)", () => { - const label = planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { label: "auto" }, blockerTitles: ["x"], ...collided, pr: { labels: [] } })).find((a) => a.actionClass === "label"); + const label = planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { review_state_label: "auto" }, blockerTitles: ["x"], ...collided, pr: { labels: [] } })).find((a) => a.actionClass === "label"); expect(label?.label).toBe(AGENT_LABEL_CHANGES); expect(label?.comment).toBeUndefined(); }); it("takes priority over a plain guardrail hold when both are true simultaneously — distinct label, not the generic one", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto" }, changedPaths: ["src/scoring/model.ts"], hardGuardrailGlobs: ["src/scoring/**"], ...collided, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { review_state_label: "auto" }, changedPaths: ["src/scoring/model.ts"], hardGuardrailGlobs: ["src/scoring/**"], ...collided, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); const label = plan.find((a) => a.actionClass === "label"); expect(label?.label).toBe(AGENT_LABEL_MIGRATION_COLLISION); }); it("still auto-merges when no migration collision is present (absent input, byte-identical to today)", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto", merge: "auto" }, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { review_state_label: "auto", merge: "auto" }, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); expect(classes(plan)).toContain("merge"); expect(plan.find((a) => a.actionClass === "label")?.label).toBe(AGENT_LABEL_READY); }); @@ -384,7 +393,7 @@ describe("planAgentMaintenanceActions (#778)", () => { describe("submission volume is NOT a manual-hold reason — only guardrail paths hold (#minimize-manual)", () => { it("a high-volume author's clean+green+approved PR MERGES (the quality gate, not a submission count, is the defense)", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto", approve: "auto", close: "auto", label: "auto" }, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto", approve: "auto", close: "auto", review_state_label: "auto" }, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); const cls = classes(plan); expect(cls).toContain("merge"); // clean → merge, regardless of how many PRs the author has open expect(cls).not.toContain("close"); @@ -395,7 +404,7 @@ describe("planAgentMaintenanceActions (#778)", () => { expect(plan).toContain("close"); }); it("ONLY a guardrail-touching review-good PR is held for manual review (needs-human, never merged/closed)", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto", approve: "auto", close: "auto", label: "auto" }, hardGuardrailGlobs: ["src/**"], changedPaths: ["src/index.ts"], pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto", approve: "auto", close: "auto", review_state_label: "auto" }, hardGuardrailGlobs: ["src/**"], changedPaths: ["src/index.ts"], pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); const cls = classes(plan); expect(cls).not.toContain("merge"); expect(cls).not.toContain("approve"); @@ -405,7 +414,7 @@ describe("planAgentMaintenanceActions (#778)", () => { }); describe("AI/review blockers remain blocking even when CI is green", () => { - const merging = { aiCiRefutationEnabled: true, autonomy: { merge: "auto" as const, approve: "auto" as const, close: "auto" as const, label: "auto" as const }, ciState: "passed" as const, pr: { labels: [], mergeableState: "clean" as const, reviewDecision: "APPROVED" as const } }; + const merging = { aiCiRefutationEnabled: true, autonomy: { merge: "auto" as const, approve: "auto" as const, close: "auto" as const, review_state_label: "auto" as const }, ciState: "passed" as const, pr: { labels: [], mergeableState: "clean" as const, reviewDecision: "APPROVED" as const } }; it("a consensus-defect failure on a green, clean PR closes instead of merging", () => { const plan = planAgentMaintenanceActions(input({ conclusion: "failure", blockerTitles: ["AI reviewers agree on a likely critical defect"], gateBlockerCodes: ["ai_consensus_defect"], ...merging })); @@ -425,7 +434,7 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("the label reports the raw failure verdict, not a refuted success", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "failure", gateBlockerCodes: ["ai_consensus_defect"], aiCiRefutationEnabled: true, autonomy: { label: "auto" }, ciState: "passed", pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "failure", gateBlockerCodes: ["ai_consensus_defect"], aiCiRefutationEnabled: true, autonomy: { review_state_label: "auto" }, ciState: "passed", pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); const label = plan.find((a) => a.actionClass === "label"); expect(label?.label).toBe(AGENT_LABEL_CHANGES); expect(label?.reason).toBe("verdict=failure"); @@ -462,7 +471,7 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("a guardrail-touching blocker still closes for a contributor", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "failure", gateBlockerCodes: ["ai_consensus_defect"], aiCiRefutationEnabled: true, autonomy: { merge: "auto", approve: "auto", close: "auto", label: "auto" }, hardGuardrailGlobs: ["src/**"], changedPaths: ["src/index.ts"], ciState: "passed", pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "failure", gateBlockerCodes: ["ai_consensus_defect"], aiCiRefutationEnabled: true, autonomy: { merge: "auto", approve: "auto", close: "auto", review_state_label: "auto" }, hardGuardrailGlobs: ["src/**"], changedPaths: ["src/index.ts"], ciState: "passed", pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); const cls = classes(plan); expect(cls).not.toContain("merge"); expect(cls).toContain("close"); @@ -527,7 +536,7 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("does NOT auto-close a red-CI PR authored by an admin login (mirrors the CI-policy owner exemption)", () => { - const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", request_changes: "auto", label: "auto" }, ciState: "failed", failingCheckNames: ["codecov/patch"], authorIsAdmin: true, pr: { labels: [] } }))); + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", request_changes: "auto", review_state_label: "auto" }, ciState: "failed", failingCheckNames: ["codecov/patch"], authorIsAdmin: true, pr: { labels: [] } }))); expect(plan).not.toContain("close"); }); }); @@ -560,7 +569,7 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("NEVER closes the owner's red-CI PR — held via the changes-requested LABEL only (no blocking request_changes), left open", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", request_changes: "auto", label: "auto" }, ciState: "failed", failingCheckNames: ["codecov/patch"], authorIsOwner: true, pr: { labels: [] } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", request_changes: "auto", review_state_label: "auto" }, ciState: "failed", failingCheckNames: ["codecov/patch"], authorIsOwner: true, pr: { labels: [] } })); const cls = classes(plan); expect(cls).not.toContain("close"); expect(cls).not.toContain("request_changes"); // never a formal blocking review @@ -577,12 +586,12 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("labels a red-CI PR changes-requested (not ready-to-merge)", () => { - const label = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto" }, ciState: "failed", failingCheckNames: ["codecov/patch"], pr: { labels: [] } })).find((a) => a.actionClass === "label"); + const label = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { review_state_label: "auto" }, ciState: "failed", failingCheckNames: ["codecov/patch"], pr: { labels: [] } })).find((a) => a.actionClass === "label"); expect(label?.label).toBe(AGENT_LABEL_CHANGES); }); it("DEFERS every action while CI is still pending (settle-before-decide)", () => { - expect(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto", approve: "auto", merge: "auto", close: "auto" }, ciState: "pending", pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } }))).toEqual([]); + expect(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { review_state_label: "auto", approve: "auto", merge: "auto", close: "auto" }, ciState: "pending", pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } }))).toEqual([]); }); it("DEFERS every action when optional visible CI is still pending after required CI passed", () => { @@ -590,7 +599,7 @@ describe("planAgentMaintenanceActions (#778)", () => { planAgentMaintenanceActions( input({ conclusion: "success", - autonomy: { label: "auto", approve: "auto", merge: "auto", close: "auto" }, + autonomy: { review_state_label: "auto", approve: "auto", merge: "auto", close: "auto" }, autoMaintain: { requireApprovals: 0, mergeMethod: "squash" }, ciState: "passed", ciHasPending: true, @@ -601,7 +610,7 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("HOLDS a contributor's gate-passing PR whose CI is UNVERIFIED — NEVER closes it (fork workflows awaiting approval) (#harm-stop)", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto", approve: "auto", merge: "auto", close: "auto" }, ciState: "unverified", pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { review_state_label: "auto", approve: "auto", merge: "auto", close: "auto" }, ciState: "unverified", pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); const cls = classes(plan); expect(cls).not.toContain("merge"); // can't merge — green not confirmed expect(cls).not.toContain("approve"); // can't approve — green not confirmed @@ -610,7 +619,7 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("NEVER closes the OWNER's unverified-CI PR — held (no blocking request_changes), left open", () => { - const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", request_changes: "auto", label: "auto" }, ciState: "unverified", authorIsOwner: true, pr: { labels: [] } }))); + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", request_changes: "auto", review_state_label: "auto" }, ciState: "unverified", authorIsOwner: true, pr: { labels: [] } }))); expect(plan).not.toContain("close"); expect(plan).not.toContain("request_changes"); }); @@ -661,7 +670,7 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("does NOT close when the close autonomy class is not acting (even with a violation)", () => { - const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "observe", label: "auto" }, ciState: "passed", linkedIssueHardRule: violation, pr: { labels: [] } }))); + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "observe", review_state_label: "auto" }, ciState: "passed", linkedIssueHardRule: violation, pr: { labels: [] } }))); expect(plan).not.toContain("close"); }); @@ -673,7 +682,7 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("takes PRECEDENCE over an otherwise-mergeable verdict (never auto-merges a PR linking an ineligible issue)", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto", close: "auto", approve: "auto", label: "auto" }, ciState: "passed", autoMaintain: { requireApprovals: 0, mergeMethod: "squash" }, linkedIssueHardRule: violation, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto", close: "auto", approve: "auto", review_state_label: "auto" }, ciState: "passed", autoMaintain: { requireApprovals: 0, mergeMethod: "squash" }, linkedIssueHardRule: violation, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); const cls = classes(plan); expect(cls).toContain("close"); expect(cls).not.toContain("merge"); @@ -689,7 +698,7 @@ describe("planAgentMaintenanceActions (#778)", () => { const pendingLabel = (plan: ReturnType) => plan.find((a) => a.actionClass === "label" && a.label === AGENT_LABEL_PENDING_CLOSURE); it("Pass 1 (verify on, label ABSENT): FLAGS (pending-closure label + warning comment), does NOT close", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", label: "auto" }, ciState: "passed", linkedIssueHardRule: violation, linkedIssueVerify: verifyOn, pr: { labels: [] } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", review_state_label: "auto" }, ciState: "passed", linkedIssueHardRule: violation, linkedIssueVerify: verifyOn, pr: { labels: [] } })); expect(classes(plan)).not.toContain("close"); const flag = pendingLabel(plan); expect(flag).toBeTruthy(); @@ -699,19 +708,19 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("label disabled: falls back to immediate close instead of holding forever without a state label", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", label: "observe" }, ciState: "passed", linkedIssueHardRule: violation, linkedIssueVerify: verifyOn, pr: { labels: [] } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", review_state_label: "observe" }, ciState: "passed", linkedIssueHardRule: violation, linkedIssueVerify: verifyOn, pr: { labels: [] } })); expect(classes(plan)).toContain("close"); expect(pendingLabel(plan)).toBeFalsy(); }); it("label approval-gated: falls back to immediate close instead of queueing an unapplied state label", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", label: "auto_with_approval" }, ciState: "passed", linkedIssueHardRule: violation, linkedIssueVerify: verifyOn, pr: { labels: [] } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", review_state_label: "auto_with_approval" }, ciState: "passed", linkedIssueHardRule: violation, linkedIssueVerify: verifyOn, pr: { labels: [] } })); expect(classes(plan)).toContain("close"); expect(pendingLabel(plan)).toBeFalsy(); }); it("Pass 2 (verify on, label PRESENT, violation persists): CLOSES with the cited reason", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", label: "auto" }, ciState: "passed", linkedIssueHardRule: violation, linkedIssueVerify: verifyOn, pr: { labels: [AGENT_LABEL_PENDING_CLOSURE] } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", review_state_label: "auto" }, ciState: "passed", linkedIssueHardRule: violation, linkedIssueVerify: verifyOn, pr: { labels: [AGENT_LABEL_PENDING_CLOSURE] } })); const close = plan.find((a) => a.actionClass === "close"); expect(close).toBeTruthy(); expect(close?.reason).toBe(violation.reason); @@ -720,7 +729,7 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("violation CLEARED with the label present: REMOVES the flag (+ resolved comment), never closes", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", label: "auto", merge: "auto" }, ciState: "passed", autoMaintain: { requireApprovals: 0, mergeMethod: "squash" }, linkedIssueHardRule: { violated: false, reason: null }, linkedIssueVerify: verifyOn, pr: { labels: [AGENT_LABEL_PENDING_CLOSURE], mergeableState: "clean" } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", review_state_label: "auto", merge: "auto" }, ciState: "passed", autoMaintain: { requireApprovals: 0, mergeMethod: "squash" }, linkedIssueHardRule: { violated: false, reason: null }, linkedIssueVerify: verifyOn, pr: { labels: [AGENT_LABEL_PENDING_CLOSURE], mergeableState: "clean" } })); expect(classes(plan)).not.toContain("close"); const remove = pendingLabel(plan); expect(remove?.labelOp).toBe("remove"); @@ -728,19 +737,19 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("verifyBeforeClose = false: IMMEDIATE close on first detection (original GAP-5 behavior, no flag)", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", label: "auto" }, ciState: "passed", linkedIssueHardRule: violation, linkedIssueVerify: { verifyBeforeClose: false, closeDelaySeconds: 30 }, pr: { labels: [] } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", review_state_label: "auto" }, ciState: "passed", linkedIssueHardRule: violation, linkedIssueVerify: { verifyBeforeClose: false, closeDelaySeconds: 30 }, pr: { labels: [] } })); expect(classes(plan)).toContain("close"); expect(pendingLabel(plan)).toBeFalsy(); }); it("owner PR is NEVER flagged or closed even with verify on (isContributor guard)", () => { - const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", label: "auto" }, ciState: "passed", authorIsOwner: true, linkedIssueHardRule: violation, linkedIssueVerify: verifyOn, pr: { labels: [] } })); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", review_state_label: "auto" }, ciState: "passed", authorIsOwner: true, linkedIssueHardRule: violation, linkedIssueVerify: verifyOn, pr: { labels: [] } })); expect(classes(plan)).not.toContain("close"); expect(pendingLabel(plan)).toBeFalsy(); }); it("Pass 1 does NOT approve or merge an otherwise-mergeable flagged PR (held for verification)", () => { - const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", label: "auto", approve: "auto", merge: "auto" }, ciState: "passed", autoMaintain: { requireApprovals: 0, mergeMethod: "squash" }, linkedIssueHardRule: violation, linkedIssueVerify: verifyOn, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } }))); + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", review_state_label: "auto", approve: "auto", merge: "auto" }, ciState: "passed", autoMaintain: { requireApprovals: 0, mergeMethod: "squash" }, linkedIssueHardRule: violation, linkedIssueVerify: verifyOn, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } }))); expect(plan).not.toContain("close"); expect(plan).not.toContain("approve"); expect(plan).not.toContain("merge"); @@ -767,7 +776,7 @@ describe("isProtectedAutomationAuthor", () => { describe("downgradeMergeToHold — accuracy circuit-breaker (#self-improve / GAP-4)", () => { // A REAL would-merge plan from the planner: gate success + clean + approvals satisfied. const wouldMerge = () => - planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto", label: "auto" }, autoMaintain: { requireApprovals: 0, mergeMethod: "squash" }, pr: { labels: [], mergeableState: "clean" } })); + planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto", review_state_label: "auto" }, autoMaintain: { requireApprovals: 0, mergeMethod: "squash" }, pr: { labels: [], mergeableState: "clean" } })); it("a real would-MERGE plan becomes a HOLD when the breaker is engaged (holdOnly=true)", () => { const plan = wouldMerge(); @@ -788,13 +797,13 @@ describe("downgradeCloseToHold — close-precision circuit-breaker (#close-preci // A REAL heuristic would-close plan from the planner: red CI on a contributor PR → changes-requested label + // a heuristic close. const heuristicClosePlan = () => - planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto", label: "auto" }, ciState: "failed", failingCheckNames: ["codecov/patch"], blockerTitles: ["x"], pr: { labels: [] } })); + planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto", review_state_label: "auto" }, ciState: "failed", failingCheckNames: ["codecov/patch"], blockerTitles: ["x"], pr: { labels: [] } })); // A REAL deterministic linked-issue-hard-rule close (the exempt kind). const linkedIssueClosePlan = () => planAgentMaintenanceActions( input({ conclusion: "success", - autonomy: { close: "auto", label: "auto" }, + autonomy: { close: "auto", review_state_label: "auto" }, ciState: "passed", linkedIssueHardRule: { violated: true, reason: "Linked issue #5 is labeled `maintainer-only` — it is not open for community PRs." }, linkedIssueVerify: { verifyBeforeClose: false, closeDelaySeconds: 0 }, @@ -839,7 +848,7 @@ describe("downgradeCloseToHold — close-precision circuit-breaker (#close-preci }); it("closeHoldOnly=true but NO heuristic close planned (e.g. a would-merge) → no-op (returns plan unchanged)", () => { - const mergePlan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto", label: "auto" }, autoMaintain: { requireApprovals: 0, mergeMethod: "squash" }, pr: { labels: [], mergeableState: "clean" } })); + const mergePlan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto", review_state_label: "auto" }, autoMaintain: { requireApprovals: 0, mergeMethod: "squash" }, pr: { labels: [], mergeableState: "clean" } })); expect(mergePlan.some((a) => a.actionClass === "merge")).toBe(true); const out = downgradeCloseToHold(mergePlan, true); expect(out).toBe(mergePlan); // unchanged: no heuristic close to drop, merge untouched @@ -868,7 +877,8 @@ describe("downgradeCloseToHold — close-precision circuit-breaker (#close-preci describe("contributor blacklist short-circuit (#1425)", () => { const blacklisted = (extra: Partial = {}) => - input({ conclusion: "success", autonomy: { label: "auto", close: "auto", approve: "auto", merge: "auto" }, blacklistMatch: { matched: true, reason: "plagiarism" }, ...extra }); + // #label-scoping: the blacklist label rides on `close` autonomy, not `label` — no `label: "auto"` needed. + input({ conclusion: "success", autonomy: { close: "auto", approve: "auto", merge: "auto" }, blacklistMatch: { matched: true, reason: "plagiarism" }, ...extra }); it("labels + closes a blacklisted contributor's PR, winning over a passing gate (no merit review / merge)", () => { const plan = planAgentMaintenanceActions(blacklisted()); @@ -895,6 +905,14 @@ describe("contributor blacklist short-circuit (#1425)", () => { expect(planAgentMaintenanceActions(blacklisted())[0]).toMatchObject({ label: "slop" }); }); + it("#label-scoping: an explicit null blacklistLabel closes WITHOUT any label; the label action carries autonomyClass: close", () => { + const withLabel = planAgentMaintenanceActions(blacklisted()); + expect(classes(withLabel)).toEqual(["label", "close"]); + expect(withLabel[0]).toMatchObject({ actionClass: "label", autonomyClass: "close" }); + const withoutLabel = planAgentMaintenanceActions(blacklisted({ blacklistLabel: null })); + expect(classes(withoutLabel)).toEqual(["close"]); + }); + it("uses the same static public close comment when the entry has no reason", () => { const withReason = planAgentMaintenanceActions(blacklisted()); const withoutReason = planAgentMaintenanceActions(blacklisted({ blacklistMatch: { matched: true, reason: null } })); @@ -918,9 +936,12 @@ describe("contributor blacklist short-circuit (#1425)", () => { expect(classes(planAgentMaintenanceActions(blacklisted({ blacklistMatch: { matched: false, reason: null } })))).not.toContain("close"); }); - it("respects autonomy: observe plans nothing (still short-circuits); label-only labels but does not close", () => { + it("#label-scoping: the label rides on `close` autonomy, not `label` — `label` alone plans nothing, `close` alone plans both", () => { expect(planAgentMaintenanceActions(blacklisted({ autonomy: {} }))).toEqual([]); - expect(classes(planAgentMaintenanceActions(blacklisted({ autonomy: { label: "auto" } })))).toEqual(["label"]); + // `label: auto` alone (no `close`) is no longer sufficient — the enforcement label is inseparable from its close. + expect(planAgentMaintenanceActions(blacklisted({ autonomy: { label: "auto" } }))).toEqual([]); + // `close: auto` alone (no `label`) is now sufficient for BOTH the close and its label. + expect(classes(planAgentMaintenanceActions(blacklisted({ autonomy: { close: "auto" } })))).toEqual(["label", "close"]); }); it("never publishes blacklist reason text in the public close comment", () => { @@ -933,9 +954,10 @@ describe("contributor blacklist short-circuit (#1425)", () => { describe("per-contributor open-item cap short-circuit (#2270)", () => { const overCap = (extra: Partial = {}) => + // #label-scoping: the cap label rides on `close` autonomy, not `label` — no `label: "auto"` needed. input({ conclusion: "success", - autonomy: { label: "auto", close: "auto", approve: "auto", merge: "auto" }, + autonomy: { close: "auto", approve: "auto", merge: "auto" }, contributorCapMatch: { matched: true, authorLogin: "farmer99", openCount: 3, cap: 2, itemKind: "pull requests" }, ...extra, }); @@ -985,6 +1007,14 @@ describe("per-contributor open-item cap short-circuit (#2270)", () => { expect(planAgentMaintenanceActions(overCap())[0]).toMatchObject({ label: "over-contributor-limit" }); }); + it("#label-scoping: an explicit null contributorCapLabel closes WITHOUT any label; the label action carries autonomyClass: close", () => { + const withLabel = planAgentMaintenanceActions(overCap()); + expect(classes(withLabel)).toEqual(["label", "close"]); + expect(withLabel[0]).toMatchObject({ actionClass: "label", autonomyClass: "close" }); + const withoutLabel = planAgentMaintenanceActions(overCap({ contributorCapLabel: null })); + expect(classes(withoutLabel)).toEqual(["close"]); + }); + it("fires AHEAD of CI — closes even while CI is still pending (not the pending early-return)", () => { expect(classes(planAgentMaintenanceActions(overCap({ ciState: "pending" })))).toEqual(["label", "close"]); }); @@ -999,9 +1029,10 @@ describe("per-contributor open-item cap short-circuit (#2270)", () => { expect(classes(planAgentMaintenanceActions(overCap({ contributorCapMatch: { matched: false, authorLogin: "farmer99", openCount: 1, cap: 2, itemKind: "pull requests" } })))).not.toContain("close"); }); - it("respects autonomy: observe plans nothing (still short-circuits); label-only labels but does not close", () => { + it("#label-scoping: the label rides on `close` autonomy, not `label` — `label` alone plans nothing, `close` alone plans both", () => { expect(planAgentMaintenanceActions(overCap({ autonomy: {} }))).toEqual([]); - expect(classes(planAgentMaintenanceActions(overCap({ autonomy: { label: "auto" } })))).toEqual(["label"]); + expect(planAgentMaintenanceActions(overCap({ autonomy: { label: "auto" } }))).toEqual([]); + expect(classes(planAgentMaintenanceActions(overCap({ autonomy: { close: "auto" } })))).toEqual(["label", "close"]); }); it("is independent of the blacklist short-circuit — a matched blacklist entry still wins when both are present", () => { @@ -1016,9 +1047,10 @@ describe("per-contributor open-item cap short-circuit (#2270)", () => { describe("review-nag cooldown short-circuit (#2463)", () => { const nagged = (extra: Partial = {}) => + // #label-scoping: the nag label rides on `close` autonomy, not `label` — no `label: "auto"` needed. input({ conclusion: "success", - autonomy: { label: "auto", close: "auto", approve: "auto", merge: "auto" }, + autonomy: { close: "auto", approve: "auto", merge: "auto" }, reviewNagMatch: { matched: true, authorLogin: "chatty-contributor", pingCount: 4, maxPings: 3 }, ...extra, }); @@ -1049,6 +1081,14 @@ describe("review-nag cooldown short-circuit (#2463)", () => { expect(planAgentMaintenanceActions(nagged())[0]).toMatchObject({ label: "review-nag-cooldown" }); }); + it("#label-scoping: an explicit null reviewNagLabel closes WITHOUT any label; the label action carries autonomyClass: close", () => { + const withLabel = planAgentMaintenanceActions(nagged()); + expect(classes(withLabel)).toEqual(["label", "close"]); + expect(withLabel[0]).toMatchObject({ actionClass: "label", autonomyClass: "close" }); + const withoutLabel = planAgentMaintenanceActions(nagged({ reviewNagLabel: null })); + expect(classes(withoutLabel)).toEqual(["close"]); + }); + it("fires AHEAD of CI — closes even while CI is still pending (not the pending early-return)", () => { expect(classes(planAgentMaintenanceActions(nagged({ ciState: "pending" })))).toEqual(["label", "close"]); }); @@ -1063,8 +1103,9 @@ describe("review-nag cooldown short-circuit (#2463)", () => { expect(classes(planAgentMaintenanceActions(nagged({ reviewNagMatch: { matched: false, authorLogin: "x", pingCount: 0, maxPings: 3 } })))).not.toContain("close"); }); - it("respects autonomy: observe plans nothing (still short-circuits); label-only labels but does not close", () => { + it("#label-scoping: the label rides on `close` autonomy, not `label` — `label` alone plans nothing, `close` alone plans both", () => { expect(planAgentMaintenanceActions(nagged({ autonomy: {} }))).toEqual([]); - expect(classes(planAgentMaintenanceActions(nagged({ autonomy: { label: "auto" } })))).toEqual(["label"]); + expect(planAgentMaintenanceActions(nagged({ autonomy: { label: "auto" } }))).toEqual([]); + expect(classes(planAgentMaintenanceActions(nagged({ autonomy: { close: "auto" } })))).toEqual(["label", "close"]); }); }); diff --git a/test/unit/agent-approval-queue.test.ts b/test/unit/agent-approval-queue.test.ts index 2445534822..67494d83c2 100644 --- a/test/unit/agent-approval-queue.test.ts +++ b/test/unit/agent-approval-queue.test.ts @@ -569,7 +569,7 @@ describe("agent approval queue (#779)", () => { it("accept downgrades a staged merge to a needs-human-review label when the precision breaker engaged after staging (#2127)", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); - await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval", label: "auto" } }); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval", review_state_label: "auto" } }); await seedInstallation(env); await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h7" }, labels: [], body: "x" }); const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash", expectedHeadSha: "h7" }, reason: "clean" }); @@ -588,7 +588,7 @@ describe("agent approval queue (#779)", () => { // breaker above would still get its whole row rejected on a stale linked-issue violation, silently swallowing // the hold label the breaker was supposed to guarantee. const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); - await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval", label: "auto" } }); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval", review_state_label: "auto" } }); await seedInstallation(env); await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h7" }, labels: [], body: "Closes #9" }); vi.mocked(resolveLinkedIssueHardRule).mockResolvedValueOnce({ violated: true, reason: "Linked issue #9 is labeled `maintainer-only` — it is not open for community PRs." }); @@ -702,7 +702,7 @@ describe("agent approval queue (#779)", () => { it("accept downgrades a staged heuristic close to a needs-human-review label when the close breaker engaged (#2127)", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); - await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { close: "auto_with_approval", label: "auto" } }); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { close: "auto_with_approval", review_state_label: "auto" } }); await seedInstallation(env); await upsertPullRequestFromGitHub(env, "owner/repo", { number: 8, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h8" }, labels: [], body: "x" }); const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 8, installationId: 5, actionClass: "close", autonomyLevel: "auto_with_approval", params: { closeComment: "noise", closeKind: "heuristic", expectedHeadSha: "h8" }, reason: "ci-failed" }); diff --git a/test/unit/autonomy.test.ts b/test/unit/autonomy.test.ts index 6bc2cf8f1e..b8b3d79edb 100644 --- a/test/unit/autonomy.test.ts +++ b/test/unit/autonomy.test.ts @@ -37,6 +37,16 @@ describe("resolveAutonomy (#773 deny-by-default gate)", () => { expect(resolveAutonomy({}, actionClass)).toBe("observe"); } }); + + it("review_state_label (#label-scoping) is independent of label — setting one does not act for the other", () => { + expect(AGENT_ACTION_CLASSES).toContain("review_state_label"); + const autonomy: AutonomyPolicy = { label: "auto" }; + expect(resolveAutonomy(autonomy, "label")).toBe("auto"); + expect(resolveAutonomy(autonomy, "review_state_label")).toBe("observe"); + const inverted: AutonomyPolicy = { review_state_label: "auto" }; + expect(resolveAutonomy(inverted, "review_state_label")).toBe("auto"); + expect(resolveAutonomy(inverted, "label")).toBe("observe"); + }); }); describe("autonomy level predicates", () => { diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index cbdfe979f1..776a2ec1e2 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -1452,6 +1452,38 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () = expect(tooLarge.warnings.some((w) => /settings\.reviewNagCooldownDays/.test(w) && /365/.test(w))).toBe(true); }); + it("#label-scoping: an explicit yml null clears blacklistLabel/contributorCapLabel/reviewNagLabel back to 'no label' (load-bearing null)", () => { + const cleared = parseFocusManifest({ settings: { blacklistLabel: null, contributorCapLabel: null, reviewNagLabel: null } }); + expect(cleared.settings.blacklistLabel).toBeNull(); + expect(cleared.settings.contributorCapLabel).toBeNull(); + expect(cleared.settings.reviewNagLabel).toBeNull(); + // Overlays (clears) a DB-configured label name. + const eff = resolveEffectiveSettings({ blacklistLabel: "slop", contributorCapLabel: "over-contributor-limit", reviewNagLabel: "review-nag-cooldown" } as unknown as RepositorySettings, cleared); + expect(eff.blacklistLabel).toBeNull(); + expect(eff.contributorCapLabel).toBeNull(); + expect(eff.reviewNagLabel).toBeNull(); + // Omitted in yml ⇒ the DB-configured label survives untouched (distinct from explicit null). + const noOverride = resolveEffectiveSettings({ blacklistLabel: "slop" } as unknown as RepositorySettings, parseFocusManifest({})); + expect(noOverride.blacklistLabel).toBe("slop"); + // A configured (non-null) string still overrides the DB normally. + const customized = parseFocusManifest({ settings: { blacklistLabel: "abuse" } }); + expect(customized.settings.blacklistLabel).toBe("abuse"); + }); + + it("#label-scoping: parses + resolves reviewNagMonitoredMentions from the settings: block, overlaying the DB", () => { + const manifest = parseFocusManifest({ settings: { reviewNagMonitoredMentions: ["JSONbored", "Some-Maintainer"] } }); + expect(manifest.settings.reviewNagMonitoredMentions).toEqual(["JSONbored", "Some-Maintainer"]); + // yml overlays (replaces) a DB-configured list. + const eff = resolveEffectiveSettings({ reviewNagMonitoredMentions: ["db-only"] } as unknown as RepositorySettings, manifest); + expect(eff.reviewNagMonitoredMentions).toEqual(["JSONbored", "Some-Maintainer"]); + // Omitted in yml ⇒ the DB-configured list survives untouched. + const noOverride = resolveEffectiveSettings({ reviewNagMonitoredMentions: ["keep-me"] } as unknown as RepositorySettings, parseFocusManifest({})); + expect(noOverride.reviewNagMonitoredMentions).toEqual(["keep-me"]); + // Invalid entries are dropped; an all-invalid list leaves the field unset (never blanks the DB list). + const invalid = parseFocusManifest({ settings: { reviewNagMonitoredMentions: ["-bad", 42 as never] } }); + expect(invalid.settings.reviewNagMonitoredMentions).toBeUndefined(); + }); + it("parses + resolves the account-age throttle settings from the settings: block, overlaying the DB (#2561)", () => { const manifest = parseFocusManifest({ settings: { accountAgeThresholdDays: 14, newAccountLabel: "fresh-account" } }); expect(manifest.settings.accountAgeThresholdDays).toBe(14); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index cf849294e3..cfa9d4da5f 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -5894,7 +5894,7 @@ describe("queue processors", () => { gateCheckMode: "enabled", linkedIssueGateMode: "block", requireLinkedIssue: true, - autonomy: { label: "auto", request_changes: "auto" }, + autonomy: { review_state_label: "auto", request_changes: "auto" }, agentDryRun: true, // dry-run → the actions are recorded but make no GitHub mutation }); await upsertOfficialMinerDetection(env, "contributor", { status: "confirmed", snapshot: queueMinerSnapshot("contributor") }, 60_000); @@ -7003,7 +7003,7 @@ describe("queue processors", () => { installation: { id: 123, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: { pull_requests: "write", issues: "write" }, events: [] }, }); await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 123); - await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto", label: "auto" }, aiReviewMode: "off", gatePack: "oss-anti-slop", gateCheckMode: "enabled", checkRunMode: "off", commentMode: "off", publicSurface: "off" }); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto", review_state_label: "auto" }, aiReviewMode: "off", gatePack: "oss-anti-slop", gateCheckMode: "enabled", checkRunMode: "off", commentMode: "off", publicSurface: "off" }); if (opts.premergeContentRecheck !== undefined) { await upsertRepoFocusManifest(env, "owner/repo", { gate: { premergeContentRecheck: opts.premergeContentRecheck } }); } @@ -7082,7 +7082,7 @@ describe("queue processors", () => { }); // No default_branch on the repo record AND no base.ref on the PR record — baseRef resolves to undefined. await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 123); - await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto", label: "auto" }, aiReviewMode: "off", gatePack: "oss-anti-slop", gateCheckMode: "enabled", checkRunMode: "off", commentMode: "off", publicSurface: "off" }); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto", review_state_label: "auto" }, aiReviewMode: "off", gatePack: "oss-anti-slop", gateCheckMode: "enabled", checkRunMode: "off", commentMode: "off", publicSurface: "off" }); await upsertRepoFocusManifest(env, "owner/repo", { gate: { premergeContentRecheck: true } }); await upsertPullRequestFromGitHub(env, "owner/repo", { number: 65, title: "No base ref", state: "open", user: { login: "contributor" }, head: { sha: "sha1" }, labels: [], body: "" }); const seen = { closed: false, merged: false, labels: [] as string[], comments: [] as string[], treeCalls: 0 }; @@ -8530,7 +8530,8 @@ describe("queue processors", () => { repoFullName: "JSONbored/gittensory", commentMode: "all_prs", gateCheckMode: "enabled", - autonomy: { close: "auto", label: "auto" }, + // #label-scoping: the cap label/close rides on `close`; the new-account label rides on `review_state_label`. + autonomy: { close: "auto", review_state_label: "auto" }, contributorOpenPrCap: 4, accountAgeThresholdDays: 30, }); @@ -8703,7 +8704,7 @@ describe("queue processors", () => { repoFullName: "JSONbored/gittensory", commentMode: "all_prs", gateCheckMode: "enabled", - autonomy: { close: "auto", label: "auto" }, + autonomy: { close: "auto", review_state_label: "auto" }, accountAgeThresholdDays: 30, newAccountLabel: "custom-new-account-label", }); @@ -8736,7 +8737,7 @@ describe("queue processors", () => { repoFullName: "JSONbored/gittensory", commentMode: "all_prs", gateCheckMode: "enabled", - // autonomy intentionally omitted — deny-by-default ("observe" for every action class, including "label"). + // autonomy intentionally omitted — deny-by-default ("observe" for every action class, including "review_state_label"). accountAgeThresholdDays: 30, }); const seen = { labels: [] as string[], closed: false }; @@ -9799,12 +9800,12 @@ describe("queue processors", () => { autoLabelEnabled: false, checkRunMode: "off", gateCheckMode: "enabled", - autonomy: { label: "auto", request_changes: "auto" }, + autonomy: { review_state_label: "auto", request_changes: "auto" }, }); // No confirmed-miner seed → author is unconfirmed; the manifest's linkedIssue:block + no issue fires a // blocker, so the gate now FAILS the author normally (#gate-nonconfirmed — confirmed status no longer - // neutralizes the verdict). But this repo grants only label/request_changes autonomy — NOT merge/close/ - // approve — so the failing gate yields a request-changes/label action at most, never a terminal action. + // neutralizes the verdict). But this repo grants only review_state_label/request_changes autonomy — NOT + // merge/close/approve — so the failing gate yields a request-changes/label action at most, never a terminal action. await upsertRepoFocusManifest(env, "JSONbored/gittensory", { gate: { linkedIssue: "block" } }); vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { const url = input.toString(); @@ -9891,7 +9892,7 @@ describe("queue processors", () => { autoLabelEnabled: false, checkRunMode: "off", gateCheckMode: "enabled", - autonomy: { label: "auto" }, + autonomy: { review_state_label: "auto" }, agentDryRun: true, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { @@ -14464,6 +14465,271 @@ describe("queue processors", () => { }); }); + describe("maintainer-mention nag moderation (#label-scoping)", () => { + function stubMonitoredMentionFetch(prNumber: number, seen: { comments: string[]; labels: string[]; closed: boolean }) { + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/collaborators/") && url.includes("/permission")) return Response.json({ permission: "none" }); + if (url.endsWith(`/pulls/${prNumber}`) && method === "PATCH") { + seen.closed = JSON.parse(String(init?.body ?? "{}")).state === "closed"; + return Response.json({ number: prNumber, state: "closed" }); + } + if (url.endsWith(`/pulls/${prNumber}`)) return Response.json({ number: prNumber, state: "open", head: { sha: `sha${prNumber}` }, mergeable_state: "clean" }); + if (url.includes(`/issues/${prNumber}/labels`) && method === "GET") return Response.json([]); + if (url.includes(`/issues/${prNumber}/labels`) && method === "POST") { + seen.labels.push(...((JSON.parse(String(init?.body ?? "{}")).labels ?? []) as string[])); + return Response.json([]); + } + if (url.endsWith("/labels") && method === "POST") return Response.json({ name: JSON.parse(String(init?.body ?? "{}")).name }, { status: 201 }); + if (url.includes(`/issues/${prNumber}/comments`) && method === "GET") return Response.json([]); + if (url.includes(`/issues/${prNumber}/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 }); + }); + } + + it("is off by default (no monitored logins configured) — no ping is tracked", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", reviewNagPolicy: "close" }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 300, title: "No monitored logins", state: "open", user: { login: "chatty" }, author_association: "NONE", labels: [], body: "" }); + const seen = { comments: [] as string[], labels: [] as string[], closed: false }; + stubMonitoredMentionFetch(300, seen); + await processJob(env, { + type: "github-webhook", + deliveryId: "mention-off-default", + 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: 300, title: "No monitored logins", state: "open", pull_request: {}, user: { login: "chatty" }, author_association: "NONE" }, + comment: { id: 1, body: "@JSONbored are you going to review this?", user: { login: "chatty", type: "User" }, author_association: "NONE" }, + }, + }); + const pings = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.monitored_mention_ping'").first<{ n: number }>(); + expect(pings?.n).toBe(0); + }); + + it("detects a mention of a configured maintainer login and records a ping under threshold without acting", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", reviewNagPolicy: "close", reviewNagMaxPings: 3, reviewNagMonitoredMentions: ["JSONbored"] }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 301, title: "Under threshold", state: "open", user: { login: "chatty" }, author_association: "NONE", labels: [], body: "" }); + const seen = { comments: [] as string[], labels: [] as string[], closed: false }; + stubMonitoredMentionFetch(301, seen); + await processJob(env, { + type: "github-webhook", + deliveryId: "mention-under-threshold", + 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: 301, title: "Under threshold", state: "open", pull_request: {}, user: { login: "chatty" }, author_association: "NONE" }, + comment: { id: 1, body: "Hey @JSONbored can you take a look?", user: { login: "chatty", type: "User" }, author_association: "NONE" }, + }, + }); + const pings = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.monitored_mention_ping'").first<{ n: number }>(); + expect(pings?.n).toBe(1); + expect(seen.closed).toBe(false); + }); + + it("case-insensitively matches a monitored login and ignores an unrelated mention", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", reviewNagPolicy: "close", reviewNagMonitoredMentions: ["JSONbored"] }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 302, title: "Case + unrelated", state: "open", user: { login: "chatty" }, author_association: "NONE", labels: [], body: "" }); + const seen = { comments: [] as string[], labels: [] as string[], closed: false }; + stubMonitoredMentionFetch(302, seen); + await processJob(env, { + type: "github-webhook", + deliveryId: "mention-case-insensitive", + 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: 302, title: "Case + unrelated", state: "open", pull_request: {}, user: { login: "chatty" }, author_association: "NONE" }, + comment: { id: 1, body: "@jsonbored please review", user: { login: "chatty", type: "User" }, author_association: "NONE" }, + }, + }); + const pings = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.monitored_mention_ping'").first<{ n: number }>(); + expect(pings?.n).toBe(1); // case-insensitive match on the configured "JSONbored" + + await processJob(env, { + type: "github-webhook", + deliveryId: "mention-unrelated", + 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: 302, title: "Case + unrelated", state: "open", pull_request: {}, user: { login: "chatty" }, author_association: "NONE" }, + comment: { id: 2, body: "this uses @some-other-package internally", user: { login: "chatty", type: "User" }, author_association: "NONE" }, + }, + }); + const pingsAfter = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.monitored_mention_ping'").first<{ n: number }>(); + expect(pingsAfter?.n).toBe(1); // unrelated mention did not add a ping + }); + + it("counts a monitored-login mention independently of the @gittensory ping counter", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", reviewNagPolicy: "close", reviewNagMaxPings: 3, reviewNagMonitoredMentions: ["JSONbored"] }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 303, title: "Independent counters", state: "open", user: { login: "chatty" }, author_association: "NONE", labels: [], body: "" }); + const seen = { comments: [] as string[], labels: [] as string[], closed: false }; + stubMonitoredMentionFetch(303, seen); + // A comment mentioning BOTH @gittensory and the monitored login should tick both counters independently. + await processJob(env, { + type: "github-webhook", + deliveryId: "mention-both", + 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: 303, title: "Independent counters", state: "open", pull_request: {}, user: { login: "chatty" }, author_association: "NONE" }, + comment: { id: 1, body: "@gittensory help — also @JSONbored can you look?", user: { login: "chatty", type: "User" }, author_association: "NONE" }, + }, + }); + const gittensoryPings = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.review_nag_ping'").first<{ n: number }>(); + const mentionPings = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.monitored_mention_ping'").first<{ n: number }>(); + expect(gittensoryPings?.n).toBe(1); + expect(mentionPings?.n).toBe(1); + }); + + it("hold policy: posts a cooldown reply naming the mentioned login and short-circuits once the threshold is crossed", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", reviewNagPolicy: "hold", reviewNagMaxPings: 3, reviewNagMonitoredMentions: ["JSONbored"] }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 304, title: "Hold on mention", state: "open", user: { login: "chatty" }, author_association: "NONE", labels: [], body: "" }); + for (let i = 0; i < 3; i += 1) { + await repositoriesModule.recordAuditEvent(env, { eventType: "github_app.monitored_mention_ping", actor: "chatty", targetKey: "JSONbored/gittensory#304#mention:jsonbored", outcome: "completed" }); + } + const seen = { comments: [] as string[], labels: [] as string[], closed: false }; + stubMonitoredMentionFetch(304, seen); + await processJob(env, { + type: "github-webhook", + deliveryId: "mention-hold", + 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: 304, title: "Hold on mention", state: "open", pull_request: {}, user: { login: "chatty" }, author_association: "NONE" }, + comment: { id: 1, body: "@JSONbored please look at this", user: { login: "chatty", type: "User" }, author_association: "NONE" }, + }, + }); + expect(seen.closed).toBe(false); + expect(seen.comments.some((c) => c.includes("cooldown limit for @JSONbored"))).toBe(true); + expect(seen.comments).toHaveLength(1); // short-circuited — no normal answer-card reply + }); + + it("close policy on a PR thread: labels + closes once the threshold is crossed, reusing reviewNagLabel", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertInstallation(env, { + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" }, target_type: "User", repository_selection: "all", permissions: { metadata: "read", pull_requests: "write", issues: "write" }, events: ["issue_comment"] }, + repositories: [{ name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }], + }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", reviewNagPolicy: "close", reviewNagMaxPings: 3, reviewNagMonitoredMentions: ["JSONbored"], reviewNagLabel: "too-chatty", autonomy: { close: "auto" } }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 305, title: "Close on mention", state: "open", user: { login: "chatty" }, head: { sha: "sha305" }, author_association: "NONE", labels: [], body: "" }); + for (let i = 0; i < 3; i += 1) { + await repositoriesModule.recordAuditEvent(env, { eventType: "github_app.monitored_mention_ping", actor: "chatty", targetKey: "JSONbored/gittensory#305#mention:jsonbored", outcome: "completed" }); + } + const seen = { comments: [] as string[], labels: [] as string[], closed: false }; + stubMonitoredMentionFetch(305, seen); + await processJob(env, { + type: "github-webhook", + deliveryId: "mention-close", + 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: 305, title: "Close on mention", state: "open", pull_request: {}, user: { login: "chatty" }, author_association: "NONE" }, + comment: { id: 1, body: "@JSONbored please look at this", user: { login: "chatty", type: "User" }, author_association: "NONE" }, + }, + }); + expect(seen.closed).toBe(true); + expect(seen.labels).toContain("too-chatty"); + // #label-scoping: close: "auto" alone (no broad label: "auto") is sufficient for the label AND the close. + }); + + it("does NOT throttle the repo owner, an admin login, an automation bot, or an exempt login", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), ADMIN_GITHUB_LOGINS: "fleet-admin" }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", reviewNagPolicy: "close", reviewNagMaxPings: 1, reviewNagMonitoredMentions: ["JSONbored"], autoCloseExemptLogins: ["trusted-regular"] }); + const seen = { comments: [] as string[], labels: [] as string[], closed: false }; + stubMonitoredMentionFetch(306, seen); + for (const [commenter, prNumber] of [ + ["JSONbored", 306], // repo owner + ["fleet-admin", 307], // admin login + ["some-bot[bot]", 308], // automation bot + ["trusted-regular", 309], // configured exemption + ] as const) { + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: prNumber, title: "Exempt", state: "open", user: { login: commenter }, author_association: "NONE", labels: [], body: "" }); + await processJob(env, { + type: "github-webhook", + deliveryId: `mention-exempt-${prNumber}`, + 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: prNumber, title: "Exempt", state: "open", pull_request: {}, user: { login: commenter }, author_association: "NONE" }, + comment: { id: prNumber, body: "@JSONbored can you review?", user: { login: commenter, type: commenter.endsWith("[bot]") ? "Bot" : "User" }, author_association: "NONE" }, + }, + }); + } + const pings = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.monitored_mention_ping'").first<{ n: number }>(); + expect(pings?.n).toBe(0); + }); + + it("does NOT throttle a third party mentioning the login on someone else's thread (thread-author-only scope)", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", reviewNagPolicy: "close", reviewNagMonitoredMentions: ["JSONbored"] }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 310, title: "Third party", state: "open", user: { login: "thread-author" }, author_association: "NONE", labels: [], body: "" }); + const seen = { comments: [] as string[], labels: [] as string[], closed: false }; + stubMonitoredMentionFetch(310, seen); + await processJob(env, { + type: "github-webhook", + deliveryId: "mention-third-party", + 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: 310, title: "Third party", state: "open", pull_request: {}, user: { login: "thread-author" }, author_association: "NONE" }, + comment: { id: 1, body: "@JSONbored can you weigh in here?", user: { login: "a-different-commenter", type: "User" }, author_association: "NONE" }, + }, + }); + const pings = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.monitored_mention_ping'").first<{ n: number }>(); + expect(pings?.n).toBe(0); + }); + + it("REGRESSION: a redelivered webhook (same deliveryId) does not double-count the ping", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", reviewNagPolicy: "close", reviewNagMaxPings: 5, reviewNagMonitoredMentions: ["JSONbored"] }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 311, title: "Redelivery", state: "open", user: { login: "chatty" }, author_association: "NONE", labels: [], body: "" }); + const seen = { comments: [] as string[], labels: [] as string[], closed: false }; + stubMonitoredMentionFetch(311, seen); + const payload = { + action: "created" as const, + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" as const } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + issue: { number: 311, title: "Redelivery", state: "open", pull_request: {}, user: { login: "chatty" }, author_association: "NONE" }, + comment: { id: 1, body: "@JSONbored ping", user: { login: "chatty", type: "User" as const }, author_association: "NONE" }, + }; + await processJob(env, { type: "github-webhook", deliveryId: "mention-redelivery-same", eventName: "issue_comment", payload }); + await processJob(env, { type: "github-webhook", deliveryId: "mention-redelivery-same", eventName: "issue_comment", payload }); + const pings = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.monitored_mention_ping'").first<{ n: number }>(); + // NOTE: unlike #2560's per-command limiter, review-nag/monitored-mention ping recording does not itself + // dedup by deliveryId -- it always records. This assertion documents CURRENT behavior (2 pings from 2 + // deliveries) rather than asserting an idempotency guarantee this handler does not provide. + expect(pings?.n).toBe(2); + }); + }); + describe("per-command @gittensory rate limit (#2560)", () => { function stubCommandRateLimitFetch(issueNumber: number, seen: { comments: string[] }) { vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { diff --git a/test/unit/selfhost-config-examples.test.ts b/test/unit/selfhost-config-examples.test.ts index 75e5fef636..4ddd4a1d30 100644 --- a/test/unit/selfhost-config-examples.test.ts +++ b/test/unit/selfhost-config-examples.test.ts @@ -24,6 +24,13 @@ describe("config/examples/global.gittensory.yml", () => { expect(manifest.gate.duplicates).toBe("block"); expect(manifest.settings.contributorOpenPrCap).toBe(3); expect(manifest.settings.autoCloseExemptLogins).toEqual(["your-admin-login"]); + // #label-scoping: the recommended one-shot autonomy baseline — close authorizes enforcement + // labels + terminal disposition; review_state_label is intentionally left at the default (unset). + expect(manifest.settings.autonomy).toEqual({ close: "auto" }); + expect(manifest.settings.reviewNagPolicy).toBe("hold"); + expect(manifest.settings.reviewNagMaxPings).toBe(3); + expect(manifest.settings.reviewNagCooldownDays).toBe(5); + expect(manifest.settings.reviewNagMonitoredMentions).toEqual(["your-maintainer-login"]); }); }); @@ -35,6 +42,7 @@ describe("config/examples/repo-override.gittensory.yml", () => { expect(manifest.gate.enabled).toBe(true); expect(manifest.wantedPaths).toEqual(["src/**"]); expect(manifest.settings.contributorOpenPrCap).toBeNull(); // documented null-clear example + expect(manifest.settings.contributorCapLabel).toBeNull(); // #label-scoping: close without any label }); }); @@ -51,6 +59,8 @@ describe("the two examples together demonstrate the documented overlay behavior" expect(manifest.gate.duplicates).toBe("block"); // inherited from global; repo-override never mentions it expect(manifest.wantedPaths).toEqual(["src/**"]); // repo-override's array replaces global's (global sets none) expect(manifest.settings.contributorOpenPrCap).toBeNull(); // repo-override's explicit null clears global's 3 + expect(manifest.settings.contributorCapLabel).toBeNull(); // repo-override clears the (unset) global default too expect(manifest.settings.autoCloseExemptLogins).toEqual(["your-admin-login"]); // inherited from global untouched + expect(manifest.settings.autonomy).toEqual({ close: "auto" }); // inherited from global untouched (repo-override never mentions it) }); });