diff --git a/migrations/0191_linked_issue_claims.sql b/migrations/0191_linked_issue_claims.sql new file mode 100644 index 000000000..84bfc4bab --- /dev/null +++ b/migrations/0191_linked_issue_claims.sql @@ -0,0 +1,33 @@ +-- #9160: durable per-(PR, issue) claim ledger. pull_requests.linked_issue_claimed_at is a single PR-level +-- timestamp that BLENDS every currently-linked issue into one value -- resolveLinkedIssueClaimedAt preserves +-- it whenever the new linked-issue set overlaps the old one, so a PR that claimed issue #1 on day 1 and later +-- adds "Fixes #7" keeps the day-1 timestamp for #7 too. That lets an attacker keep a long-lived PR linking a +-- throwaway issue, then edit its body to add a victim's valuable issue and inherit a backdated claim time, +-- stealing the duplicate-cluster winner slot (queue/duplicate-detection.ts, packages/loopover-engine's +-- duplicate-winner election). +-- +-- Each row here is written ONCE, immutably, the first time loopover observes THIS SPECIFIC (repo, PR, issue) +-- triple (INSERT OR IGNORE) -- so a newly-added issue always gets its own fresh claim time regardless of what +-- the blended pull_requests column says. The duplicate-winner election reads this table, scoped to only the +-- issue(s) actually contested with a sibling, instead of the blended PR-level value. +CREATE TABLE IF NOT EXISTS linked_issue_claims ( + repo_full_name TEXT NOT NULL, + pull_number INTEGER NOT NULL, + issue_number INTEGER NOT NULL, + claimed_at TEXT NOT NULL, + PRIMARY KEY (repo_full_name, pull_number, issue_number) +); + +-- Backfill: seed one row per currently-linked issue from the existing blended pull_requests column (same +-- COALESCE chain 0084_pull_request_linked_issue_claimed_at.sql used), so an issue already linked before this +-- migration keeps its historical relative ordering intact instead of silently resetting to "whenever this PR +-- next happens to re-sync" the first time recordLinkedIssueClaims runs post-deploy. This is exactly as +-- backdated as today's pre-fix behavior for these PRE-EXISTING rows (no worse) -- the fix's actual benefit (a +-- NEWLY-added issue getting its own fresh, un-backdatable clock) applies going forward, from each PR's next +-- sync onward. +INSERT OR IGNORE INTO linked_issue_claims (repo_full_name, pull_number, issue_number, claimed_at) +SELECT pr.repo_full_name, pr.number, CAST(je.value AS INTEGER), COALESCE(pr.linked_issue_claimed_at, pr.updated_at, pr.created_at) +FROM pull_requests pr, json_each(pr.linked_issues_json) je +WHERE pr.linked_issues_json IS NOT NULL + AND pr.linked_issues_json != '[]' + AND pr.linked_issues_json != ''; diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 12666adfa..df3290b54 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -33,6 +33,7 @@ import { installations, issues, githubRateLimitObservations, + linkedIssueClaims, notificationDeliveries, issueWatchSubscriptions, notificationSubscriptions, @@ -453,6 +454,11 @@ export async function upsertPullRequestFromGitHub( existingClaimRow, observedLinkedIssueClaimedAt, ); + // #9160: alongside the blended PR-level column above, durably record each CURRENTLY-linked issue's own + // first-observed claim time (immutable, per-issue) -- a brand-new issue gets `syncedAt` here regardless of + // what the blended column resolves to, so the duplicate-winner election can read a claim time that a + // different, already-linked issue can never backdate. Best-effort: never fails the PR upsert itself. + await recordLinkedIssueClaims(env, repoFullName, pr.number, linkedIssues, syncedAt).catch(() => undefined); // A genuinely observed body (even an explicitly empty one) proves this PR's linked-issue extraction is // trustworthy going forward; a sparse payload (pr.body === undefined) proves nothing either way and must not // start the clock. Set once, keep forever (#linked-issue-sparse-first-upsert). @@ -586,6 +592,39 @@ function linkedIssueSetsOverlap(left: number[], right: number[]): boolean { return right.some((value) => leftSet.has(value)); } +/** #9160: record an IMMUTABLE first-observed claim time for each of `issueNumbers` on (repoFullName, + * pullNumber) -- INSERT ... ON CONFLICT DO NOTHING, so an issue already claimed here keeps its ORIGINAL + * claimedAt forever, while a genuinely NEW issue (never linked to this PR before) gets `observedAt` as its + * own fresh clock -- regardless of what pull_requests.linked_issue_claimed_at's blended value says (see that + * column's own #linked-issue-claim-overlap-preserve doc comment for the backdating bug this ledger exists to + * close). A no-op for an empty `issueNumbers`. Called from upsertPullRequestFromGitHub on every sync; safe to + * call repeatedly with the same issue numbers (idempotent). */ +export async function recordLinkedIssueClaims(env: Env, repoFullName: string, pullNumber: number, issueNumbers: readonly number[], observedAt: string): Promise { + if (issueNumbers.length === 0) return; + const bounded = boundedString(repoFullName, 200); + await getDb(env.DB) + .insert(linkedIssueClaims) + .values(issueNumbers.map((issueNumber) => ({ repoFullName: bounded, pullNumber, issueNumber, claimedAt: observedAt }))) + .onConflictDoNothing({ target: [linkedIssueClaims.repoFullName, linkedIssueClaims.pullNumber, linkedIssueClaims.issueNumber] }); +} + +/** #9160: the EARLIEST recorded per-issue claim time among `issueNumbers` for (repoFullName, pullNumber), or + * `null` when none of them has ever been recorded (a pre-migration PR, or a caller passing an issue number + * this PR never actually linked) -- fails closed the same way resolveDuplicateClusterWinnerNumber's own + * sparse-row handling does, rather than guessing. Callers scope `issueNumbers` to the issue(s) ACTUALLY + * contested with a specific sibling (see queue/duplicate-detection.ts) so an unrelated issue also linked to + * this PR can never backdate (or postdate) the comparison. */ +export async function getEarliestLinkedIssueClaim(env: Env, repoFullName: string, pullNumber: number, issueNumbers: readonly number[]): Promise { + if (issueNumbers.length === 0) return null; + const [row] = await getDb(env.DB) + .select({ claimedAt: linkedIssueClaims.claimedAt }) + .from(linkedIssueClaims) + .where(and(eq(linkedIssueClaims.repoFullName, boundedString(repoFullName, 200)), eq(linkedIssueClaims.pullNumber, pullNumber), inArray(linkedIssueClaims.issueNumber, [...issueNumbers]))) + .orderBy(asc(linkedIssueClaims.claimedAt)) + .limit(1); + return row?.claimedAt ?? null; +} + export async function upsertIssueFromGitHub(env: Env, repoFullName: string, issue: GitHubIssuePayload, options: { seenOpenAt?: string } = {}): Promise { const record = toIssueRecord(repoFullName, issue); const db = getDb(env.DB); diff --git a/src/db/schema.ts b/src/db/schema.ts index 82df0178d..cd6489f14 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -482,6 +482,29 @@ export const pullRequests = sqliteTable( }), ); +// #9160: durable per-(PR, issue) claim ledger. `pullRequests.linkedIssueClaimedAt` above is a single PR-level +// timestamp that BLENDS every currently-linked issue into one value -- resolveLinkedIssueClaimedAt preserves it +// whenever the new linked-issue set overlaps the old one, so a PR that claimed issue #1 on day 1 and later adds +// "Fixes #7" keeps the day-1 timestamp for #7 too, letting an old placeholder PR backdate a claim on a +// newly-added, more valuable issue and steal the duplicate-cluster winner slot from whoever actually claimed it +// first (queue/duplicate-detection.ts). Each row here is written ONCE, immutably, the first time loopover +// observes THIS SPECIFIC (repo, PR, issue) triple (INSERT ... ON CONFLICT DO NOTHING) -- so a newly-added issue +// always gets its own fresh claim time here regardless of what the blended column says, and the duplicate- +// winner election can be scoped to only the issue(s) actually contested with a sibling instead of the blended +// PR-level value. +export const linkedIssueClaims = sqliteTable( + "linked_issue_claims", + { + repoFullName: text("repo_full_name").notNull(), + pullNumber: integer("pull_number").notNull(), + issueNumber: integer("issue_number").notNull(), + claimedAt: text("claimed_at").notNull(), + }, + (table) => ({ + primary: primaryKey({ columns: [table.repoFullName, table.pullNumber, table.issueNumber] }), + }), +); + export const pullRequestFiles = sqliteTable( "pull_request_files", { diff --git a/src/queue/duplicate-detection.ts b/src/queue/duplicate-detection.ts index 907bca7e6..3ff3d1f59 100644 --- a/src/queue/duplicate-detection.ts +++ b/src/queue/duplicate-detection.ts @@ -12,6 +12,7 @@ import { githubRateLimitAdmissionKeyForToken } from "../github/client"; import { resolveCopycatDuplicateSibling } from "../signals/copycat-duplicate"; import { isDuplicateClusterWinnerByClaim, resolveDuplicateClusterWinnerNumber } from "../signals/duplicate-winner"; import { isDuplicateWinnerEnabledGlobally, resolveDuplicateWinnerEnabled } from "../settings/duplicate-winner-mode"; +import { getEarliestLinkedIssueClaim } from "../db/repositories"; import type { CopycatGateMode, PullRequestRecord, RepositorySettings } from "../types"; import { mapWithConcurrency } from "./map-with-concurrency"; @@ -134,6 +135,36 @@ export async function reconcileLiveDuplicateSiblings( ); } +/** + * #9160: scope `pr`'s claim time to ONLY the issue(s) actually contested with at least one of `openSiblings`, + * reading each issue's own IMMUTABLE first-observed claim from the durable per-(PR, issue) ledger + * (`db/repositories.ts`'s `linked_issue_claims`, see its own doc comment) instead of `pr`'s blended + * `linkedIssueClaimedAt` column. This is what closes the #linked-issue-claim-overlap-preserve-style backdating + * attack at the winner-election boundary specifically: an attacker's OTHER, unrelated linked issue (kept + * around purely so the blended column's overlap rule preserves an old timestamp) never enters this + * computation, because it was never shared with any sibling in `openSiblings`. + * + * `contestedIssues` empty (no sibling actually overlaps -- defensive; every real caller already filtered + * `openSiblings` down to PRs sharing an issue with `pr`) returns `pr.linkedIssueClaimedAt` unchanged, since + * there is no backdating angle without a contested issue. Otherwise returns the per-issue ledger's earliest + * claim among the CONTESTED issues only, which fails closed to `null` (mirrors this module's existing + * sparse-row-fails-closed posture) rather than falling back to the blended value when the ledger has no row + * yet for any of them -- a `null` here makes {@link isDuplicateClusterWinnerByClaim} correctly refuse to + * elect `pr` the winner from missing data, rather than risk resurrecting the very blended value being scoped + * away from. + */ +export async function resolveScopedLinkedIssueClaimedAt( + env: Env, + repoFullName: string, + pr: Pick, + openSiblings: readonly Pick[], +): Promise { + const siblingIssues = new Set(openSiblings.flatMap((sibling) => sibling.linkedIssues)); + const contestedIssues = pr.linkedIssues.filter((issue) => siblingIssues.has(issue)); + if (contestedIssues.length === 0) return pr.linkedIssueClaimedAt; + return getEarliestLinkedIssueClaim(env, repoFullName, pr.number, contestedIssues); +} + export function linkedIssueDuplicatePullRequestsForGate( pr: PullRequestRecord, pullRequests: PullRequestRecord[], diff --git a/src/queue/processors.ts b/src/queue/processors.ts index a53b1b8c7..6397e18db 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -380,6 +380,7 @@ import { linkedIssueDuplicatePullRequestRecordsForGate, linkedIssueDuplicatePullRequestsForGate, reconcileLiveDuplicateSiblings, + resolveScopedLinkedIssueClaimedAt, } from "./duplicate-detection"; export { dupWinnerLinkedDuplicateCount, @@ -387,6 +388,7 @@ export { linkedIssueDuplicatePullRequestRecordsForGate, linkedIssueDuplicatePullRequestsForGate, reconcileLiveDuplicateSiblings, + resolveScopedLinkedIssueClaimedAt, } from "./duplicate-detection"; import { mapWithConcurrency } from "./map-with-concurrency"; export { mapWithConcurrency } from "./map-with-concurrency"; @@ -1601,6 +1603,10 @@ export async function sweepRepoRegate( pr.linkedIssues, settings, ); + // #9160: see resolveScopedLinkedIssueClaimedAt's own doc comment -- scopes pr's claim time to only the + // issue(s) actually contested with an open sibling instead of pr's blended linkedIssueClaimedAt column. + const scopedLinkedIssueClaimedAt = + duplicateWinnerEnabled && others.length > 0 ? await resolveScopedLinkedIssueClaimedAt(env, repoFullName, pr, others) : pr.linkedIssueClaimedAt; const advisory = buildPullRequestAdvisory(repo, pr, { otherOpenPullRequests: others, requireLinkedIssue, @@ -1609,6 +1615,7 @@ export async function sweepRepoRegate( confirmedNoOpenLinkedIssue, copycatGateMode: settings.copycatGateMode, copycatGateMinScore: settings.copycatGateMinScore, + scopedLinkedIssueClaimedAt, }); const gate = evaluateGateCheck( advisory, @@ -2721,6 +2728,11 @@ function buildAgentMaintenancePlanInput(args: { pr: PullRequestRecord; openDuplicateSiblings: ReturnType; duplicateWinnerEnabled: boolean; + // #9160: pr's claim time, ALREADY SCOPED by the caller (resolveScopedLinkedIssueClaimedAt) to only the + // issue(s) actually contested with an open sibling -- resolved outside this function since it needs a DB + // read and this function is otherwise pure. Falls back to pr.linkedIssueClaimedAt when undefined so every + // existing test/caller that hasn't been updated to pass it keeps today's (pre-#9160) behavior exactly. + scopedLinkedIssueClaimedAt?: string | null | undefined; manualReviewLockContentionResolved: AgentActionPlanInput["manualReviewLockContentionResolved"]; }): AgentActionPlanInput { const { @@ -2747,8 +2759,10 @@ function buildAgentMaintenancePlanInput(args: { pr, openDuplicateSiblings, duplicateWinnerEnabled, + scopedLinkedIssueClaimedAt, manualReviewLockContentionResolved, } = args; + const linkedIssueClaimedAtForElection = scopedLinkedIssueClaimedAt !== undefined ? scopedLinkedIssueClaimedAt : pr.linkedIssueClaimedAt; return { conclusion: gate.conclusion, blockerTitles: gate.blockers.map((blocker) => blocker.title), @@ -2828,7 +2842,7 @@ function buildAgentMaintenancePlanInput(args: { linkedDuplicateCount: dupWinnerLinkedDuplicateCount( openDuplicateSiblings, pr.number, - pr.linkedIssueClaimedAt, + linkedIssueClaimedAtForElection, duplicateWinnerEnabled, pr.createdAt, ), @@ -2838,7 +2852,7 @@ function buildAgentMaintenancePlanInput(args: { linkedDuplicateWinnerNumber: dupWinnerLinkedDuplicateWinnerNumber( openDuplicateSiblings, pr.number, - pr.linkedIssueClaimedAt, + linkedIssueClaimedAtForElection, duplicateWinnerEnabled, pr.createdAt, ), @@ -2866,7 +2880,10 @@ function buildAgentMaintenancePlanInput(args: { * EVERY PR-open across the fleet, not just the rare over-cap ones — this function alone is cheap enough (one * DB query + a bounded live-GitHub confirm) to run unconditionally on open. */ -async function resolvePerRepoContributorCapMatch( +// #9159: exported so the approval-queue accept path (src/services/agent-approval-queue.ts) can build the SAME +// pre-merge contributor-cap re-check the live webhook path builds below (contributorCapMergeRecheck) -- that +// path previously never threaded one at all, so an accepted staged merge skipped the #7284 re-check entirely. +export async function resolvePerRepoContributorCapMatch( env: Env, deliveryId: string, installationId: number, @@ -3502,6 +3519,17 @@ async function runAgentMaintenancePlanAndExecute( // not inherit this transient provenance on some future pass. await deleteTransientKey(env, manualReviewLockContentionMarkerKey); } + // #9160: scope pr's claim time to only the issue(s) actually contested with an open duplicate sibling, + // reading the durable per-(PR, issue) claim ledger instead of pr's own blended linkedIssueClaimedAt column + // -- see resolveScopedLinkedIssueClaimedAt's own doc comment for why the blended column is unsafe to feed + // directly into the winner election (an unrelated OTHER linked issue on this PR could otherwise "vouch" a + // backdated claim time for the one actually being contested). Only worth the extra DB read when the flag is + // on AND at least one sibling actually overlaps; otherwise this is `pr.linkedIssueClaimedAt` unchanged, + // byte-identical to before this existed. + const scopedLinkedIssueClaimedAt = + duplicateWinnerEnabled && openDuplicateSiblings.length > 0 + ? await resolveScopedLinkedIssueClaimedAt(env, repoFullName, pr, openDuplicateSiblings) + : pr.linkedIssueClaimedAt; const planned = planAgentMaintenanceActions( buildAgentMaintenancePlanInput({ gate, @@ -3527,6 +3555,7 @@ async function runAgentMaintenancePlanAndExecute( pr, openDuplicateSiblings, duplicateWinnerEnabled, + scopedLinkedIssueClaimedAt, manualReviewLockContentionResolved: hadPriorLockContentionHold && !aiReviewLockContentionThisPass, }), ); @@ -4085,14 +4114,22 @@ export async function reReviewStoredPullRequest( cachedOtherOpenPullRequests, settings, ); + const duplicateWinnerEnabledForPr = resolveDuplicateWinnerEnabled(isDuplicateWinnerEnabledGlobally(env), settings.duplicateWinnerMode); + // #9160: see resolveScopedLinkedIssueClaimedAt's own doc comment -- scopes pr's claim time to only the + // issue(s) actually contested with an open sibling instead of pr's blended linkedIssueClaimedAt column. + const scopedLinkedIssueClaimedAt = + duplicateWinnerEnabledForPr && otherOpenPullRequests.length > 0 + ? await resolveScopedLinkedIssueClaimedAt(env, repoFullName, pr, otherOpenPullRequests) + : pr.linkedIssueClaimedAt; const advisory = buildPullRequestAdvisory(repo, pr, { otherOpenPullRequests, requireLinkedIssue: shouldCollectLinkedIssueEvidence(settings), - duplicateWinnerEnabled: resolveDuplicateWinnerEnabled(isDuplicateWinnerEnabledGlobally(env), settings.duplicateWinnerMode), + duplicateWinnerEnabled: duplicateWinnerEnabledForPr, confirmedNoOpenLinkedIssue, linkedIssueAuthorLogins, copycatGateMode: settings.copycatGateMode, copycatGateMinScore: settings.copycatGateMinScore, + scopedLinkedIssueClaimedAt, }); await persistAdvisory(env, advisory); // #2537 follow-up (gate-flagged): the durable review cache's only invalidation path is markPullRequestReviewsInvalidated @@ -6942,14 +6979,22 @@ async function handlePullRequestWebhookEvent( cachedOtherOpenPullRequests, settings, ); + const duplicateWinnerEnabledForPr = resolveDuplicateWinnerEnabled(isDuplicateWinnerEnabledGlobally(env), settings.duplicateWinnerMode); + // #9160: see resolveScopedLinkedIssueClaimedAt's own doc comment -- scopes pr's claim time to only the + // issue(s) actually contested with an open sibling instead of pr's blended linkedIssueClaimedAt column. + const scopedLinkedIssueClaimedAt = + duplicateWinnerEnabledForPr && otherOpenPullRequests.length > 0 + ? await resolveScopedLinkedIssueClaimedAt(env, repoFullName, pr, otherOpenPullRequests) + : pr.linkedIssueClaimedAt; const advisory = buildPullRequestAdvisory(repo, pr, { otherOpenPullRequests, requireLinkedIssue: shouldCollectLinkedIssueEvidence(settings), - duplicateWinnerEnabled: resolveDuplicateWinnerEnabled(isDuplicateWinnerEnabledGlobally(env), settings.duplicateWinnerMode), + duplicateWinnerEnabled: duplicateWinnerEnabledForPr, confirmedNoOpenLinkedIssue, linkedIssueAuthorLogins, copycatGateMode: settings.copycatGateMode, copycatGateMinScore: settings.copycatGateMinScore, + scopedLinkedIssueClaimedAt, }); await persistAdvisory(env, advisory); // Auto-project/milestone matching (#3183): independent of the gate/disposition entirely -- a missed or @@ -14296,14 +14341,22 @@ export async function buildAuthorizedPrActionAdvisory( pr.linkedIssues, settings, ); + const duplicateWinnerEnabledForPr = resolveDuplicateWinnerEnabled(isDuplicateWinnerEnabledGlobally(env), settings.duplicateWinnerMode); + // #9160: see resolveScopedLinkedIssueClaimedAt's own doc comment -- scopes pr's claim time to only the + // issue(s) actually contested with an open sibling instead of pr's blended linkedIssueClaimedAt column. + const scopedLinkedIssueClaimedAt = + duplicateWinnerEnabledForPr && otherOpenPullRequests.length > 0 + ? await resolveScopedLinkedIssueClaimedAt(env, repoFullName, pr, otherOpenPullRequests) + : pr.linkedIssueClaimedAt; const advisory = buildPullRequestAdvisory(repo, pr, { otherOpenPullRequests, requireLinkedIssue: shouldCollectLinkedIssueEvidence(settings), - duplicateWinnerEnabled: resolveDuplicateWinnerEnabled(isDuplicateWinnerEnabledGlobally(env), settings.duplicateWinnerMode), + duplicateWinnerEnabled: duplicateWinnerEnabledForPr, confirmedNoOpenLinkedIssue, linkedIssueAuthorLogins, copycatGateMode: settings.copycatGateMode, copycatGateMinScore: settings.copycatGateMinScore, + scopedLinkedIssueClaimedAt, }); return { repo, advisory, otherOpenPullRequests }; } diff --git a/src/review/linked-issue-label-propagation-fetch.ts b/src/review/linked-issue-label-propagation-fetch.ts index 70551cee3..aa5cb6f23 100644 --- a/src/review/linked-issue-label-propagation-fetch.ts +++ b/src/review/linked-issue-label-propagation-fetch.ts @@ -136,6 +136,12 @@ async function resolveIssueLabelsForPropagation( prAuthorLogin: string | undefined, relaxableLabels: ReadonlySet, prMergedAt: string | null, + // #9161: issueLabels named by at least one ADDITIVE mapping (`removeOtherTypeLabels !== true` -- see + // pr-type-label.ts's exclusive/additive split, e.g. `gittensor:priority` composing alongside bug/feature). + // An additive mapping carries reward weight by construction (it never replaces the PR's type label, it adds + // a bonus on top) -- author/assignee identity must NOT unlock one of these unconditionally the way it does + // for a plain type label; see the author/assignee branch below for why. + rewardLabels: ReadonlySet, ): Promise { if (result.status === "fetch_error") { console.log( @@ -204,7 +210,21 @@ async function resolveIssueLabelsForPropagation( const allLabels = result.facts.labels; const issueAuthorLogin = result.facts.authorLogin?.toLowerCase(); const assignees = result.facts.assignees.map((login) => login.toLowerCase()); - if (issueAuthorLogin === prAuthorLogin || assignees.includes(prAuthorLogin)) return { labels: allLabels, inconclusive: false }; + const isDirectOwnershipMatch = issueAuthorLogin === prAuthorLogin || assignees.includes(prAuthorLogin); + // #9161: a direct author/assignee match unlocks a TYPE label (no reward-semantic mapping targets it) + // unconditionally -- the original, unchanged behavior. It must NOT also unlock a REWARD label + // (`rewardLabels`) the same way: that bypasses the reward-label trust gate (the `...ForReward` opt-in + + // maintainer-authorship check every OTHER path already requires) entirely, letting a contributor who simply + // authored (or was assigned) the linked issue self-award a reward multiplier with no opt-in and no + // maintainer check. A reward label ALWAYS falls through to the maintainer-check gate below, regardless of + // this match. When there is no direct match at all, every label is a "reward candidate" here -- identical to + // the pre-#9161 behavior for that branch (nothing changes for a non-owning contributor). + const directTypeLabels = isDirectOwnershipMatch ? allLabels.filter((label) => !rewardLabels.has(label.toLowerCase())) : []; + const rewardCandidateLabels = isDirectOwnershipMatch ? allLabels.filter((label) => rewardLabels.has(label.toLowerCase())) : allLabels; + // Zero added cost when this issue carries no reward-semantic label at all (the overwhelmingly common case + // for a repo that hasn't configured an additive mapping) -- skips the maintainer-permission GitHub API call + // entirely, mirroring relaxableLabels' own "byte-identical when no mapping opted in" contract. + if (isDirectOwnershipMatch && rewardCandidateLabels.length === 0) return { labels: directTypeLabels, inconclusive: false }; const maintainerCheck: MaintainerCheckResult = relaxableLabels.size > 0 && !!issueAuthorLogin @@ -222,7 +242,8 @@ async function resolveIssueLabelsForPropagation( return { labels: [], inconclusive: true }; } const maintainerAuthored = maintainerCheck === "maintainer"; - const kept = maintainerAuthored ? allLabels.filter((label) => relaxableLabels.has(label.toLowerCase())) : []; + const relaxedRewardLabels = maintainerAuthored ? rewardCandidateLabels.filter((label) => relaxableLabels.has(label.toLowerCase())) : []; + const kept = [...directTypeLabels, ...relaxedRewardLabels]; if (kept.length < allLabels.length && allLabels.length > 0) { console.log( @@ -230,7 +251,11 @@ async function resolveIssueLabelsForPropagation( event: "linked_issue_label_propagation_filtered", repoFullName: args.repoFullName, issueNumber: result.facts.number, - reason: maintainerAuthored ? "strict_label_requires_direct_ownership" : "no_direct_ownership_match", + reason: isDirectOwnershipMatch + ? "reward_label_requires_maintainer_authored_issue_and_opt_in" + : maintainerAuthored + ? "strict_label_requires_direct_ownership" + : "no_direct_ownership_match", droppedCount: allLabels.length - kept.length, }), ); @@ -314,6 +339,14 @@ export async function fetchLinkedIssueLabelsForPropagation(args: { ) .map(([issueLabel]) => issueLabel), ); + // #9161: issueLabels named by at least one ADDITIVE mapping (`removeOtherTypeLabels !== true`) -- see + // resolveIssueLabelsForPropagation's `rewardLabels` parameter doc comment for why this is the reward- + // semantics signal. `.some` (not `.every`, unlike relaxableLabels above): even ONE additive mapping + // targeting this issueLabel is enough to treat it as reward-bearing everywhere, so a repo cannot dilute the + // reward gate by also declaring an unrelated exclusive mapping for the same issueLabel name. + const rewardLabels = new Set( + [...mappingsByIssueLabel.entries()].filter(([, mappings]) => mappings.some((mapping) => mapping.removeOtherTypeLabels !== true)).map(([issueLabel]) => issueLabel), + ); const results = await Promise.all( linkedIssues.map((issueNumber) => fetchLinkedIssueFacts( @@ -333,6 +366,7 @@ export async function fetchLinkedIssueLabelsForPropagation(args: { prAuthorLogin, relaxableLabels, prMergedAt, + rewardLabels, ), ), ); diff --git a/src/rules/advisory.ts b/src/rules/advisory.ts index 8ad4a5958..1c3f26711 100644 --- a/src/rules/advisory.ts +++ b/src/rules/advisory.ts @@ -386,6 +386,11 @@ export function buildPullRequestAdvisory( * identical to before this field existed — the duplicate finding stays linked-issue-overlap only. */ copycatGateMode?: CopycatGateMode | null | undefined; copycatGateMinScore?: number | null | undefined; + /** #9160: see addPullRequestFindings's own parameter doc comment. Resolved by the caller (this function + * stays synchronous/pure) via queue/duplicate-detection.ts's resolveScopedLinkedIssueClaimedAt, from the + * SAME otherOpenPullRequests already being passed above. Absent ⇒ falls back to pr.linkedIssueClaimedAt, + * byte-identical to before this field existed. */ + scopedLinkedIssueClaimedAt?: string | null | undefined; } = {}, ): Advisory { const repoFullName = pr?.repoFullName ?? repo?.fullName ?? "unknown/unknown"; @@ -422,6 +427,7 @@ export function buildPullRequestAdvisory( Boolean(context.confirmedNoOpenLinkedIssue), context.copycatGateMode, context.copycatGateMinScore, + context.scopedLinkedIssueClaimedAt, ); } return advisory("pull_request", targetKey, repoFullName, findings, "Pull request advisory generated.", pr?.number, undefined, pr?.headSha ?? undefined); @@ -990,6 +996,13 @@ function addPullRequestFindings( confirmedNoOpenLinkedIssue: boolean, copycatGateMode: CopycatGateMode | null | undefined, copycatGateMinScore: number | null | undefined, + // #9160: pr's claim time, ALREADY SCOPED by the caller (queue/duplicate-detection.ts's + // resolveScopedLinkedIssueClaimedAt) to only the issue(s) actually contested with an open sibling, instead of + // pr.linkedIssueClaimedAt's blended-across-every-linked-issue value -- see that function's own doc comment + // for why the blended column lets an unrelated, already-linked issue backdate a newly-added one's claim. + // `undefined` (every caller that hasn't been updated, and every non-DB caller like decision-replay.ts) falls + // back to pr.linkedIssueClaimedAt, byte-identical to before this existed. + scopedLinkedIssueClaimedAt?: string | null | undefined, ): void { if (pr.state !== "open") { findings.push({ @@ -1036,7 +1049,11 @@ function addPullRequestFindings( // winner survives while later claimants keep the finding. Sparse legacy rows fail closed instead of // suppressing duplicate evidence with arbitrary PR-number ordering. // Flag-OFF (default) short-circuits ⇒ the finding is pushed exactly as before (byte-identical). - if (overlappingPrs.length > 0 && !(duplicateWinnerEnabled && isDuplicateClusterWinnerByClaim(pr, overlappingPrs))) { + // #9160: compare using the SCOPED claim time (falls back to pr.linkedIssueClaimedAt when the caller didn't + // resolve one) rather than pr's raw, blended column directly -- see scopedLinkedIssueClaimedAt's own + // parameter doc comment above. + const claimMemberForElection = { number: pr.number, linkedIssueClaimedAt: scopedLinkedIssueClaimedAt !== undefined ? scopedLinkedIssueClaimedAt : pr.linkedIssueClaimedAt, createdAt: pr.createdAt }; + if (overlappingPrs.length > 0 && !(duplicateWinnerEnabled && isDuplicateClusterWinnerByClaim(claimMemberForElection, overlappingPrs))) { // #9129: split by corroboration (hasDuplicateOverlapCorroboration) — see its own doc comment. A CONCRETE // finding code (`duplicate_pr_risk`) is reserved for a sibling with real corroborating evidence beyond body // text; it stays configurable via duplicatePrGateMode and evaluateGateCheckCore now HOLDS (never closes) a diff --git a/src/services/agent-action-executor.ts b/src/services/agent-action-executor.ts index 2abc8b7d8..1b237826e 100644 --- a/src/services/agent-action-executor.ts +++ b/src/services/agent-action-executor.ts @@ -729,13 +729,104 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE }).catch(() => undefined); } } - // 8c) pre-merge contributor-cap re-check (#7284-fix, TOCTOU race): confirmed live -- a PR whose OWN earlier - // cap check passed can still merge after a sibling's later cap-close made the author over cap, because each - // PR's cap check runs independently with no shared lock. Re-verify right before the irreversible write, - // under the SAME per-author mutex a concurrent sibling's cap-close/wake also acquires, so the two can never - // both act on a stale view of the author's open-PR count. `ctx.contributorCapMergeRecheck` is only ever set - // by the caller when a cap is actually configured for this repo (the common case leaves it undefined) — - // zero added cost, byte-identical to before this field existed. + // 9) live — perform the real mutation, recording success or the error. Wrapped in a closure so 8c below can + // hold the contributor-cap lock across this call for a merge (see 8c's own doc comment for why). + const performMutation = async (): Promise => { + try { + const detailOverride = await performAction(env, ctx, action); + await audit("completed", detailOverride ?? action.reason); + // #9134: every completed merge/close emits a decision record + chained ledger row, by construction -- + // see DecisionRecordContext's doc comment on why this lives HERE instead of at each call site. Never + // throws: recording legibility must never retroactively fail a mutation that already succeeded. + if (action.actionClass === "merge" || action.actionClass === "close") { + await recordCompletedDecision(env, ctx, action).catch(() => undefined); + } + // CI-run cancellation on an anti-abuse close (#2462 contributor_cap; extended to blacklist #6659): stop + // burning CI minutes on a PR that was just closed for exceeding the contributor cap, or for a banned + // login. contributor_cap stays opt-in (contributorCapCancelCi) since a repo may want the cap to bite + // without touching CI; blacklist is unconditional -- there is no scenario where a maintainer wants a + // permanently-banned login's CI to keep running after the close. Best-effort, AFTER the close already + // succeeded -- cancelInFlightWorkflowRunsForHeadSha never throws, so a missing actions:write grant (or + // any other failure here) can never retroactively turn this already-successful close into a recorded + // "error" by escaping into the catch block below. + if (action.actionClass === "close" && ctx.headSha) { + if (action.closeKind === "contributor_cap" && ctx.contributorCapCancelCi) { + await recordCiCancelOutcome(env, "contributor_cap", ctx, ctx.headSha); + } else if (action.closeKind === "blacklist") { + await recordCiCancelOutcome(env, "blacklist", ctx, ctx.headSha); + } + } + // Re-approval idempotency: record the head SHA we just approved so the planner skips re-approving this + // exact commit on the next sweep (a GitHub App's own approval does not reliably flip reviewDecision to + // APPROVED, so reviewDecision alone can't dedup). A new commit clears the match → the bot approves it. + // Best-effort: a failed persist only risks one redundant re-approval, never a wrong disposition. + if (action.actionClass === "approve" && !action.dismissStaleApproval && ctx.headSha) { + await markPullRequestApproved(env, ctx.repoFullName, ctx.pullNumber, ctx.headSha).catch(() => undefined); + } + // Per-repo Discord notification on a terminal/visible action (reviewbot parity): merge→merged, + // close→closed, request_changes→manual review. Best-effort; never affects the action. RC1 dedups at the + // action level, so this fires once per outcome per PR (no spam). + const notifyOutcome: NotifyOutcome | null = + action.actionClass === "merge" ? "merged" : action.actionClass === "close" ? "closed" : action.actionClass === "request_changes" ? "manual" : null; + if (notifyOutcome) { + // #6636: enrich the notification with the AI's actual gate-verdict reasoning (the latest recorded + // gate_decision summary for this PR) instead of only the plain disposition reason — resolveDispositionReason + // falls back to `action.reason` when no verdict is on record or the read fails, so this is byte-identical + // when there's nothing to enrich with. review_audit keys gate_decision rows by `${repoFullName}#${number}`. + const summary = await resolveDispositionReason(env, `${ctx.repoFullName}#${ctx.pullNumber}`, action.reason); + const notifyParams = { repoFullName: ctx.repoFullName, pullNumber: ctx.pullNumber, outcome: notifyOutcome, summary, submitter: ctx.authorLogin }; + await notifyActionToDiscord(env, notifyParams).catch(() => undefined); + await notifyActionToSlack(env, notifyParams).catch(() => undefined); + } + } catch (error) { + await audit("error", errorMessage(error)); + // RC3 terminal-fail merges: immediate terminal failures (401/405/409/conflict) are marked once; generic + // GitHub 403s are retryable first because branch-protection/check/conversation state can converge shortly + // after the gate publishes. A possibly-transient failure is retried up to MERGE_RETRY_CAP, then held. + if (action.actionClass === "merge" && ctx.headSha) { + await handleMergeFailure(env, ctx, error); + } else if (action.actionClass === "update_branch" && isMergeConflictMessage(errorMessage(error))) { + // LOOPOVER-24: update_branch performs a real merge internally, so it fails with the same "merge + // conflict" shape a MERGE action does -- but unlike a merge's terminal hold (a PR permanently blocked + // until a human intervenes), this is NOT a stuck state: forceUpdateBranch's caller (prReadyForReview) + // already falls through to reviewing the PR on its current, non-rebased head when this returns false + // (see forceUpdateBranch's own doc comment), exactly like every other "couldn't rebase, review anyway" + // path. The branch owner, not the bot, needs to resolve the conflict -- paging on every naturally- + // diverged PR this happens to hit isn't warranted. Still recorded by the audit() call above. + } else if (action.actionClass === "update_branch" && isNoNewBaseCommitsMessage(errorMessage(error))) { + // LOOPOVER-24 (regressed shape): a 422 "There are no new commits on the base branch." means the head + // was already up to date when update-branch fired -- the readiness check acted on a stale/cached + // mergeable_state read. Nothing went wrong and nothing is stuck: the caller falls through to reviewing + // the current head exactly as in the conflict case above. Audit-only; never a PostHog page. + } else { + // Non-merge action classes have no retry loop -- a single failure here is already this pass's terminal + // outcome (the planner may re-attempt on the next sweep if the underlying condition clears itself), so + // it is captured immediately rather than only on eventual exhaustion. Mirrors handleMergeFailure's own + // terminal-hold capture below and the "a real failure the maintainer must see" convention already used + // for review-pass failures (selfhost/posthog.ts's capturePostHogReviewFailure, queue/processors.ts). + // Previously this class of failure was audit-log-only, invisible without a manual audit_events query. + capturePostHogError(error, { kind: "agent_action_execution_failed", repo: ctx.repoFullName, pr: ctx.pullNumber, installationId: ctx.installationId, actionClass: action.actionClass }, "agent_action_execution_failed"); + } + // #2265: a permission-looking 403 on a PR-write mutation can mean the LOCAL installations.permissions + // snapshot is stale after a maintainer-initiated downgrade (GitHub sends no downgrade webhook). Rate-limit + // 403s and operation-specific forbidden states are not permission evidence, and this refresh scans broad + // installation state, so keep the hot error path narrowly filtered and per-installation cooled down. + if (PR_WRITE_CLASSES.has(action.actionClass) && shouldRefreshInstallationHealthAfterPrWriteFailure(ctx.installationId, error)) { + await refreshInstallationHealthForInstallation(env, ctx.installationId).catch(() => undefined); + } + } + }; + // 8c) pre-merge contributor-cap re-check (#7284-fix / #9159, TOCTOU race): confirmed live -- a PR whose OWN + // earlier cap check passed can still merge after a sibling's later cap-close made the author over cap, + // because each PR's cap check runs independently with no shared lock. Re-verify right before the + // irreversible write, under the SAME per-author mutex a concurrent sibling's cap-close/wake also acquires, + // so the two can never both act on a stale view of the author's open-PR count. #9159: the mutex must span + // the ACTUAL merge mutation (step 9) too, not just this re-check read -- releasing it beforehand (as this + // used to) reopens the exact #7284 window the lock exists to close: a concurrent cap-close claims the + // just-released lock, live-verifies this PR as still open, and wrong-closes it for a cap this merge was + // about to relieve milliseconds later. `ctx.contributorCapMergeRecheck` is only ever set by the caller when + // a cap is actually configured for this repo (the common case leaves it undefined) — zero added cost, + // byte-identical to before this field existed. if (action.actionClass === "merge" && ctx.contributorCapMergeRecheck) { const authorLogin = ctx.authorLogin ?? ""; const { acquired, ownerToken } = await claimContributorCapLock(env, ctx.repoFullName, authorLogin); @@ -751,94 +842,16 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE await audit("denied", `contributor cap re-check confirmed ${authorLogin} is now over cap on ${ctx.repoFullName} — action not executed`); continue; } + // #9159: the lock stays held across the merge mutation itself -- only released in the `finally` below, + // AFTER performMutation has run to completion (success or failure) -- see this block's header comment. + await performMutation(); } finally { await releaseContributorCapLock(env, ctx.repoFullName, authorLogin, ownerToken); } + continue; } // 9) live — perform the real mutation, recording success or the error. - try { - const detailOverride = await performAction(env, ctx, action); - await audit("completed", detailOverride ?? action.reason); - // #9134: every completed merge/close emits a decision record + chained ledger row, by construction -- - // see DecisionRecordContext's doc comment on why this lives HERE instead of at each call site. Never - // throws: recording legibility must never retroactively fail a mutation that already succeeded. - if (action.actionClass === "merge" || action.actionClass === "close") { - await recordCompletedDecision(env, ctx, action).catch(() => undefined); - } - // CI-run cancellation on an anti-abuse close (#2462 contributor_cap; extended to blacklist #6659): stop - // burning CI minutes on a PR that was just closed for exceeding the contributor cap, or for a banned - // login. contributor_cap stays opt-in (contributorCapCancelCi) since a repo may want the cap to bite - // without touching CI; blacklist is unconditional -- there is no scenario where a maintainer wants a - // permanently-banned login's CI to keep running after the close. Best-effort, AFTER the close already - // succeeded -- cancelInFlightWorkflowRunsForHeadSha never throws, so a missing actions:write grant (or - // any other failure here) can never retroactively turn this already-successful close into a recorded - // "error" by escaping into the catch block below. - if (action.actionClass === "close" && ctx.headSha) { - if (action.closeKind === "contributor_cap" && ctx.contributorCapCancelCi) { - await recordCiCancelOutcome(env, "contributor_cap", ctx, ctx.headSha); - } else if (action.closeKind === "blacklist") { - await recordCiCancelOutcome(env, "blacklist", ctx, ctx.headSha); - } - } - // Re-approval idempotency: record the head SHA we just approved so the planner skips re-approving this - // exact commit on the next sweep (a GitHub App's own approval does not reliably flip reviewDecision to - // APPROVED, so reviewDecision alone can't dedup). A new commit clears the match → the bot approves it. - // Best-effort: a failed persist only risks one redundant re-approval, never a wrong disposition. - if (action.actionClass === "approve" && !action.dismissStaleApproval && ctx.headSha) { - await markPullRequestApproved(env, ctx.repoFullName, ctx.pullNumber, ctx.headSha).catch(() => undefined); - } - // Per-repo Discord notification on a terminal/visible action (reviewbot parity): merge→merged, - // close→closed, request_changes→manual review. Best-effort; never affects the action. RC1 dedups at the - // action level, so this fires once per outcome per PR (no spam). - const notifyOutcome: NotifyOutcome | null = - action.actionClass === "merge" ? "merged" : action.actionClass === "close" ? "closed" : action.actionClass === "request_changes" ? "manual" : null; - if (notifyOutcome) { - // #6636: enrich the notification with the AI's actual gate-verdict reasoning (the latest recorded - // gate_decision summary for this PR) instead of only the plain disposition reason — resolveDispositionReason - // falls back to `action.reason` when no verdict is on record or the read fails, so this is byte-identical - // when there's nothing to enrich with. review_audit keys gate_decision rows by `${repoFullName}#${number}`. - const summary = await resolveDispositionReason(env, `${ctx.repoFullName}#${ctx.pullNumber}`, action.reason); - const notifyParams = { repoFullName: ctx.repoFullName, pullNumber: ctx.pullNumber, outcome: notifyOutcome, summary, submitter: ctx.authorLogin }; - await notifyActionToDiscord(env, notifyParams).catch(() => undefined); - await notifyActionToSlack(env, notifyParams).catch(() => undefined); - } - } catch (error) { - await audit("error", errorMessage(error)); - // RC3 terminal-fail merges: immediate terminal failures (401/405/409/conflict) are marked once; generic - // GitHub 403s are retryable first because branch-protection/check/conversation state can converge shortly - // after the gate publishes. A possibly-transient failure is retried up to MERGE_RETRY_CAP, then held. - if (action.actionClass === "merge" && ctx.headSha) { - await handleMergeFailure(env, ctx, error); - } else if (action.actionClass === "update_branch" && isMergeConflictMessage(errorMessage(error))) { - // LOOPOVER-24: update_branch performs a real merge internally, so it fails with the same "merge - // conflict" shape a MERGE action does -- but unlike a merge's terminal hold (a PR permanently blocked - // until a human intervenes), this is NOT a stuck state: forceUpdateBranch's caller (prReadyForReview) - // already falls through to reviewing the PR on its current, non-rebased head when this returns false - // (see forceUpdateBranch's own doc comment), exactly like every other "couldn't rebase, review anyway" - // path. The branch owner, not the bot, needs to resolve the conflict -- paging on every naturally- - // diverged PR this happens to hit isn't warranted. Still recorded by the audit() call above. - } else if (action.actionClass === "update_branch" && isNoNewBaseCommitsMessage(errorMessage(error))) { - // LOOPOVER-24 (regressed shape): a 422 "There are no new commits on the base branch." means the head - // was already up to date when update-branch fired -- the readiness check acted on a stale/cached - // mergeable_state read. Nothing went wrong and nothing is stuck: the caller falls through to reviewing - // the current head exactly as in the conflict case above. Audit-only; never a PostHog page. - } else { - // Non-merge action classes have no retry loop -- a single failure here is already this pass's terminal - // outcome (the planner may re-attempt on the next sweep if the underlying condition clears itself), so - // it is captured immediately rather than only on eventual exhaustion. Mirrors handleMergeFailure's own - // terminal-hold capture below and the "a real failure the maintainer must see" convention already used - // for review-pass failures (selfhost/posthog.ts's capturePostHogReviewFailure, queue/processors.ts). - // Previously this class of failure was audit-log-only, invisible without a manual audit_events query. - capturePostHogError(error, { kind: "agent_action_execution_failed", repo: ctx.repoFullName, pr: ctx.pullNumber, installationId: ctx.installationId, actionClass: action.actionClass }, "agent_action_execution_failed"); - } - // #2265: a permission-looking 403 on a PR-write mutation can mean the LOCAL installations.permissions - // snapshot is stale after a maintainer-initiated downgrade (GitHub sends no downgrade webhook). Rate-limit - // 403s and operation-specific forbidden states are not permission evidence, and this refresh scans broad - // installation state, so keep the hot error path narrowly filtered and per-installation cooled down. - if (PR_WRITE_CLASSES.has(action.actionClass) && shouldRefreshInstallationHealthAfterPrWriteFailure(ctx.installationId, error)) { - await refreshInstallationHealthForInstallation(env, ctx.installationId).catch(() => undefined); - } - } + await performMutation(); } await maybeEscalateModeration(env, { installationId: ctx.installationId, repoFullName: ctx.repoFullName, number: ctx.pullNumber, authorLogin: ctx.authorLogin, mode, moderationSettings: ctx.moderationSettings }, planned, outcomes); diff --git a/src/services/agent-approval-queue.ts b/src/services/agent-approval-queue.ts index f787409ff..6208f085b 100644 --- a/src/services/agent-approval-queue.ts +++ b/src/services/agent-approval-queue.ts @@ -4,6 +4,7 @@ import { getPullRequest, getPendingAgentAction, insertNotificationDeliveryIfAbsent, + latestCompletedAuditEventDetail, listPendingAgentActions, recordAuditEvent, setPendingAgentActionStatus, @@ -19,6 +20,10 @@ import { findBlacklistEntry } from "../settings/contributor-blacklist"; import { isCloseHoldOnly, isHoldOnly, readUntrustworthyRuleCodes } from "../review/outcomes-wire"; import { fetchLiveCiAggregate, fetchLivePullRequestMergeState, fetchLivePullRequestReviewDecision, fetchLivePullRequestState, fetchLiveReviewThreadBlockers, fetchRequiredStatusContexts, mergeRequiredCiContexts, REVIEW_DECISION_UNREADABLE } from "../github/backfill"; import { githubRateLimitAdmissionKeyForToken } from "../github/client"; +import { resolvePerRepoContributorCapMatch } from "../queue/processors"; +import { isBelowAccountAgeThreshold } from "../queue/account-age-throttle"; +import { isAutoCloseExempt } from "../settings/auto-close-exempt"; +import { isPerTenantAdmin } from "../auth/security"; import type { AgentPendingActionParams, AgentPendingActionRecord } from "../types"; export type ApprovalDecision = "accept" | "reject"; @@ -63,6 +68,34 @@ export async function decidePendingAgentAction(env: Env, input: { id: string; de return { status: "already_decided", action: current ?? pending }; } + // #9158 (label-close-split-brain, approval-queue half): a label COUPLED to a close (autonomyClass === + // "close" AND closeKind set -- see planContributorCapClose's/the blacklist/review-nag/copycat closes' own + // doc comments: the close is always pushed BEFORE its label, tagged with the SAME closeKind and + // autonomyClass "close") must never post unless its paired close has ACTUALLY completed. The executor's own + // same-batch correlation guard (agent-action-executor.ts's coupledCloseOutcome) only ever sees a close + // staged in the SAME plan -- but this queue stages one action per row (createPendingAgentActionIfAbsent + // conflicts on actionClass), so an independently rejected/never-accepted close has nothing correlating it to + // this label here. Re-derive it from the audit trail -- the ONE record every execution entry point (live + // pass, breaker downgrade, this same accept path) writes through, regardless of which path the close took. + // `autonomyClass` is the disambiguator that keeps this from misfiring on a label-only enforcement tier (e.g. + // maybePlanCopycatLabel's copycat "label" gate mode, which sets closeKind with NO autonomyClass at all, + // since it rides its own generic `label` autonomy and never has -- or needs -- a paired close). + if (pending.actionClass === "label" && pending.params.autonomyClass === "close" && pending.params.closeKind !== undefined) { + const pairedCloseCompleted = (await latestCompletedAuditEventDetail(env, "loopover", "agent.action.close", targetKey)) !== null; + if (!pairedCloseCompleted) { + await setPendingAgentActionStatus(env, pending.id, { status: "rejected", decidedBy: input.decidedBy }); + await recordAuditEvent(env, { + eventType: "agent.pending_action.superseded", + actor: input.decidedBy, + targetKey, + outcome: "denied", + detail: `superseded ${pending.params.closeKind} label: its paired close has not completed — applying it would brand a PR that remains open`, + metadata: baseMetadata, + }); + return { status: "rejected", action: { ...pending, status: "rejected", decidedBy: input.decidedBy }, executionOutcome: "paired_close_not_completed" }; + } + } + // accept → execute the staged action live, then record the result. const [settings, pr, installation] = await Promise.all([ resolveRepositorySettings(env, pending.repoFullName), @@ -422,6 +455,38 @@ export async function decidePendingAgentAction(env: Env, input: { id: string; de .then((branchProtectionContexts) => mergeRequiredCiContexts(branchProtectionContexts, settings.expectedCiContexts)) .catch(() => mergeRequiredCiContexts(null, settings.expectedCiContexts)); + // #9159: pre-merge contributor-cap re-check, threaded into the accept-replay path for the first time -- the + // executor's own step-8c re-check (agent-action-executor.ts) only ever runs when THIS field is set, and this + // accept path previously never set it at all, so a staged merge accepted here skipped the #7284 re-check + // entirely. Constructed the SAME way runAgentMaintenancePlanAndExecute (src/queue/processors.ts) gates its + // own live-path recheck -- only when the POST-downgrade plan still contains a merge, a per-repo cap is + // configured, and the author isn't owner/admin/automation-bot/exempt -- so "who gets rechecked" never drifts + // between the live webhook path and this replay path. + let contributorCapMergeRecheck: (() => Promise) | undefined; + if (plan.some((action) => action.actionClass === "merge") && pr && typeof settings.contributorOpenPrCap === "number" && pr.authorLogin && !isAutoCloseExempt(pr.authorLogin, settings.autoCloseExemptLogins)) { + const repoOwner = pending.repoFullName.includes("/") ? pending.repoFullName.slice(0, pending.repoFullName.indexOf("/")) : ""; + const authorIsOwner = pr.authorLogin.toLowerCase() === repoOwner.toLowerCase(); + const authorIsAutomationBot = isProtectedAutomationAuthor(pr.authorLogin, env); + const authorIsAdmin = await isPerTenantAdmin(env, pending.installationId, pending.repoFullName, pr.authorLogin); + if (!authorIsOwner && !authorIsAdmin && !authorIsAutomationBot) { + const isNewAccount = await isBelowAccountAgeThreshold(env, pending.installationId, pr.authorLogin, settings.accountAgeThresholdDays); + contributorCapMergeRecheck = async () => { + const recheckMatch = await resolvePerRepoContributorCapMatch( + env, + `approval-accept:${pending.repoFullName}#${pending.pullNumber}`, + pending.installationId, + pending.repoFullName, + pr, + settings, + executionCiToken, + executionAdmissionKey, + isNewAccount, + ); + return recheckMatch?.matched !== true; + }; + } + } + const outcomes = await executeAgentMaintenanceActions( env, { @@ -441,6 +506,10 @@ export async function decidePendingAgentAction(env: Env, input: { id: string; de // approval (close autonomy = auto_with_approval), so the accept-replay path needs this resolved the // same way the live webhook path does (src/queue/processors.ts) for the cancel hook to fire here too. contributorCapCancelCi: settings.contributorCapCancelCi ?? env.CONTRIBUTOR_CAP_CANCEL_CI_DEFAULT === "true", + // #9159: see contributorCapMergeRecheck's own construction above -- absent (undefined) exactly when the + // live path would also leave it absent (no cap configured, no merge in the post-downgrade plan, or an + // exempt author), zero added cost in the common case. + contributorCapMergeRecheck, // #selfhost-ci-verification: the executor's OWN final pre-mutation live-CI re-check (step 8 of // executeAgentMaintenanceActions) needs the same effective required contexts this accept-time re-check // (above) evaluated against. Re-fetch so branch-protection changes remain authoritative at accept time. diff --git a/src/settings/agent-actions.ts b/src/settings/agent-actions.ts index bef03d220..aecf2875e 100644 --- a/src/settings/agent-actions.ts +++ b/src/settings/agent-actions.ts @@ -652,8 +652,23 @@ export function downgradeCloseToHold( (noConcreteEvidenceUnderProjectBreaker(action) || (action.closeConcreteEvidence === true && everyJustifyingCodeUntrustworthy(action))); if (!planned.some(isDowngradableClose)) return planned; const labels = resolveAgentDispositionLabels(labelSettings); - // Drop ONLY the downgradable close(s); a deterministic linked-issue-hard-rule close (if any) is left intact. - const next = planned.filter((action) => !isDowngradableClose(action)); + // #9158 (label-close-split-brain, breaker-downgrade half): dropping a close here must ALSO drop any label + // COUPLED to it -- the anti-abuse label pushed alongside a blacklist/contributor_cap/review_nag/copycat + // close carries the SAME closeKind and is inseparable metadata on that close (see planContributorCapClose's/ + // the blacklist/review-nag/copycat closes' own doc comments: close is always pushed BEFORE its label, same + // batch). The executor's own #label-close-split-brain correlation guard (agent-action-executor.ts's + // coupledCloseOutcome) only fires when the close is STILL in the plan and denied/errored this pass -- once + // the close is removed from the plan entirely (exactly what downgrading does), there is nothing left for + // that guard to correlate against, so it silently lets the orphaned label through. Filtering it out HERE, + // at the one place the close is actually dropped, closes that gap without needing the executor to guess + // whether an absent close means "never coupled" (e.g. maybePlanCopycatLabel's label-only copycat tier, which + // legitimately has no close to drop) or "was coupled but just got downgraded". + const droppedCloseKinds = new Set(planned.filter(isDowngradableClose).map((action) => action.closeKind).filter((kind): kind is NonNullable => kind !== undefined)); + const isOrphanedCoupledLabel = (action: PlannedAgentAction): boolean => action.actionClass === "label" && action.closeKind !== undefined && droppedCloseKinds.has(action.closeKind); + // Drop the downgradable close(s) AND any label coupled to one of them; a deterministic linked-issue-hard-rule + // close/label pair (if any) is left intact -- isDowngradableClose never matches that closeKind (see this + // function's own doc comment above), so it can never end up in droppedCloseKinds. + const next = planned.filter((action) => !isDowngradableClose(action) && !isOrphanedCoupledLabel(action)); // The dropped close means the PR is held for a person — surface the manual-review label. Idempotent: only add when // absent (e.g. a guarded-but-passing plan may already carry it). NEVER adds a merge/approve. const alreadyNeedsReview = labels.manualReview !== null && next.some((action) => action.actionClass === "label" && action.label === labels.manualReview && action.labelOp !== "remove"); diff --git a/test/unit/agent-action-executor.test.ts b/test/unit/agent-action-executor.test.ts index f7a0d0f05..061df6747 100644 --- a/test/unit/agent-action-executor.test.ts +++ b/test/unit/agent-action-executor.test.ts @@ -2507,6 +2507,38 @@ describe("pre-merge contributor-cap re-check (#7284-fix, TOCTOU race)", () => { expect(outcomes[0]?.outcome).toBe("completed"); expect(mergePullRequest).toHaveBeenCalled(); }); + + it("REGRESSION (#9159): the lock stays held across the ACTUAL merge mutation, not just the re-check -- a concurrent claim attempt made WHILE the merge is in flight is denied", async () => { + const env = createTestEnv({}); + const recheck = vi.fn(async () => true); + let claimWhileMerging: boolean | undefined; + vi.mocked(mergePullRequest).mockImplementationOnce(async () => { + // Simulate a concurrent sibling's cap-close/wake trying to claim the SAME per-author lock while this + // merge mutation is still in flight -- before #9159's fix, the lock was already released at this point + // (releaseContributorCapLock ran in a `finally` wrapping only the re-check, not the merge below it), so + // this claim would have wrongly succeeded and let a concurrent cap-close act on a stale view. + claimWhileMerging = await env.SELFHOST_TRANSIENT_CACHE?.claim?.("contributor-cap-lock:owner/repo:farmer99", "concurrent-cap-close-token", 30); + return { merged: true, sha: "merged-sha", suppressed: false }; + }); + const outcomes = await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", contributorCapMergeRecheck: recheck }), [merge]); + expect(outcomes[0]?.outcome).toBe("completed"); + expect(claimWhileMerging).toBe(false); // the concurrent claim attempt was denied -- the lock was still held + // Released only AFTER the merge completed: a fresh claim attempt now succeeds. + const claimAfterMerge = await env.SELFHOST_TRANSIENT_CACHE?.claim?.("contributor-cap-lock:owner/repo:farmer99", "probe-token", 30); + expect(claimAfterMerge).toBe(true); + }); + + it("REGRESSION (#9159): the lock is released even when the merge mutation itself throws (finally spans success AND failure)", async () => { + const env = createTestEnv({}); + const recheck = vi.fn(async () => true); + vi.mocked(mergePullRequest).mockImplementationOnce(async () => { + throw new Error("merge conflict"); + }); + const outcomes = await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", contributorCapMergeRecheck: recheck }), [merge]); + expect(outcomes[0]?.outcome).toBe("error"); + const claimAfterFailure = await env.SELFHOST_TRANSIENT_CACHE?.claim?.("contributor-cap-lock:owner/repo:farmer99", "probe-token", 30); + expect(claimAfterFailure).toBe(true); + }); }); // #9134: six of seven executeAgentMaintenanceActions call sites wrote no decision record and no ledger row at diff --git a/test/unit/agent-actions.test.ts b/test/unit/agent-actions.test.ts index 3b9fe75b3..513112329 100644 --- a/test/unit/agent-actions.test.ts +++ b/test/unit/agent-actions.test.ts @@ -1822,6 +1822,54 @@ describe("downgradeCloseToHold — close-precision circuit-breaker (#close-preci const label = held.find((a) => a.actionClass === "label" && a.label === AGENT_LABEL_NEEDS_REVIEW && a.labelOp === "add"); expect(label?.autonomyClass).toBe("close"); }); + + describe("#9158 (label-close-split-brain, breaker-downgrade half): dropping a downgradable close also drops its coupled label", () => { + it("REGRESSION: downgrading a review_nag close also drops the review_nag label coupled to it via closeKind", () => { + const reviewNagClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "review nag", closeKind: "review_nag" }; + const reviewNagLabel: PlannedAgentAction = { actionClass: "label", requiresApproval: false, reason: "review nag", closeKind: "review_nag", label: DEFAULT_REVIEW_NAG_LABEL, labelOp: "add", autonomyClass: "close" }; + const held = downgradeCloseToHold([reviewNagClose, reviewNagLabel], true); + expect(held.some((a) => a.actionClass === "close" && a.closeKind === "review_nag")).toBe(false); + // The orphaned coupled label must NOT survive the downgrade -- applying it would brand a PR that stays open. + expect(held.some((a) => a.actionClass === "label" && a.closeKind === "review_nag")).toBe(false); + // Manual-review hold still substitutes in for the dropped close, same as any other downgrade. + expect(held.some((a) => a.actionClass === "label" && a.label === AGENT_LABEL_NEEDS_REVIEW && a.labelOp === "add")).toBe(true); + }); + + it("REGRESSION: downgrading a copycat close also drops the copycat label coupled to it, leaving an UNCOUPLED label untouched", () => { + const copycatClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "copycat match", closeKind: "copycat" }; + const copycatLabel: PlannedAgentAction = { actionClass: "label", requiresApproval: false, reason: "copycat match", closeKind: "copycat", label: DEFAULT_COPYCAT_LABEL, labelOp: "add", autonomyClass: "close" }; + // An unrelated label with NO closeKind at all (e.g. a plain disposition-communication label) must survive + // untouched -- only a label whose closeKind matches one of the DROPPED closes is orphaned. + const unrelatedLabel: PlannedAgentAction = { actionClass: "label", requiresApproval: false, reason: "ready", label: AGENT_LABEL_READY, labelOp: "add" }; + const held = downgradeCloseToHold([copycatClose, copycatLabel, unrelatedLabel], true); + expect(held.some((a) => a.actionClass === "close" && a.closeKind === "copycat")).toBe(false); + expect(held.some((a) => a.actionClass === "label" && a.closeKind === "copycat")).toBe(false); + expect(held.some((a) => a.actionClass === "label" && a.label === AGENT_LABEL_READY)).toBe(true); + }); + + it("a label-only copycat gate tier (closeKind set but NO matching close in the plan at all) is left untouched -- it was never coupled to a close", () => { + // maybePlanCopycatLabel's label-only tier: a copycat label with closeKind set but no close of that kind + // anywhere in the plan. droppedCloseKinds is empty (no downgradable close present), so isOrphanedCoupledLabel + // can never match this label, and the plan is returned as-is (no heuristic/downgradable close present). + const copycatLabelOnly: PlannedAgentAction = { actionClass: "label", requiresApproval: false, reason: "copycat match (label tier)", closeKind: "copycat", label: DEFAULT_COPYCAT_LABEL, labelOp: "add" }; + const plan = [copycatLabelOnly]; + const held = downgradeCloseToHold(plan, true); + expect(held).toBe(plan); // no downgradable close anywhere → whole plan returned unchanged + expect(held.some((a) => a.actionClass === "label" && a.closeKind === "copycat")).toBe(true); + }); + + it("a deterministic linked-issue-hard-rule close/label pair is left intact even while a sibling review_nag close/label pair is downgraded", () => { + const linkedIssueClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "ineligible issue", closeKind: "linked-issue-hard-rule" }; + const linkedIssueLabel: PlannedAgentAction = { actionClass: "label", requiresApproval: false, reason: "ineligible issue", closeKind: "linked-issue-hard-rule", label: "maintainer-only", labelOp: "add" }; + const reviewNagClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "review nag", closeKind: "review_nag" }; + const reviewNagLabel: PlannedAgentAction = { actionClass: "label", requiresApproval: false, reason: "review nag", closeKind: "review_nag", label: DEFAULT_REVIEW_NAG_LABEL, labelOp: "add", autonomyClass: "close" }; + const held = downgradeCloseToHold([linkedIssueClose, linkedIssueLabel, reviewNagClose, reviewNagLabel], true); + expect(held.some((a) => a.actionClass === "close" && a.closeKind === "linked-issue-hard-rule")).toBe(true); + expect(held.some((a) => a.actionClass === "label" && a.closeKind === "linked-issue-hard-rule")).toBe(true); + expect(held.some((a) => a.actionClass === "close" && a.closeKind === "review_nag")).toBe(false); + expect(held.some((a) => a.actionClass === "label" && a.closeKind === "review_nag")).toBe(false); + }); + }); }); describe("closeConcreteEvidence — concrete-evidence exemption from the close-precision breaker (#hard-blockers-not-ai-judgment)", () => { diff --git a/test/unit/agent-approval-queue.test.ts b/test/unit/agent-approval-queue.test.ts index 7f2b4b06a..c92d51dee 100644 --- a/test/unit/agent-approval-queue.test.ts +++ b/test/unit/agent-approval-queue.test.ts @@ -74,6 +74,7 @@ import { getPendingAgentAction, listNotificationDeliveriesForRecipient, listPendingAgentActions, + recordAuditEvent, setPendingAgentActionStatus, upsertGlobalContributorBlacklist, upsertInstallation, @@ -1703,3 +1704,180 @@ describe("agent approval queue (#779)", () => { expect(await countPendingAgentActions(env, { repoFullName: "owner/repo", status: "pending" })).toBe(201); }); }); + +describe("#9158 (label-close-split-brain, approval-queue half): a coupled enforcement label may not post unless its paired close completed", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("REGRESSION: accept REJECTS a staged label coupled to a close (autonomyClass close + closeKind) when no paired close has EVER completed for this PR", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { label: "auto_with_approval" } }); + await seedInstallation(env); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "plagiarist" }, head: { sha: "h7" }, labels: [], body: "x" }); + const { action } = await createPendingAgentActionIfAbsent(env, { + repoFullName: "owner/repo", + pullNumber: 7, + installationId: 5, + actionClass: "label", + autonomyLevel: "auto_with_approval", + params: { autonomyClass: "close", closeKind: "blacklist", label: "blacklisted-contributor", labelOp: "add" }, + reason: "blacklisted contributor", + }); + + const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" }); + + expect(result.status).toBe("rejected"); + expect(result.executionOutcome).toBe("paired_close_not_completed"); + expect((await getPendingAgentAction(env, action.id))?.status).toBe("rejected"); + const { ensurePullRequestLabel: labelNotApplied } = await import("../../src/github/labels"); + expect(labelNotApplied).not.toHaveBeenCalled(); + const superseded = await env.DB.prepare("select outcome, detail from audit_events where event_type = 'agent.pending_action.superseded' and target_key = ?") + .bind("owner/repo#7") + .first<{ outcome: string; detail: string }>(); + expect(superseded).toMatchObject({ outcome: "denied" }); + expect(superseded?.detail).toContain("paired close has not completed"); + }); + + it("accept EXECUTES a staged coupled label once its paired close has actually completed (real audit trail)", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); + // #label-scoping: this label's autonomyClass is "close" (it rides the close autonomy dial, not the + // generic label one), so accept-time re-verification checks ctx.autonomy.close, not ctx.autonomy.label. + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { close: "auto_with_approval" } }); + await seedInstallation(env); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "plagiarist" }, head: { sha: "h7" }, labels: [], body: "x" }); + // The paired close's own completion is recorded through the SAME audit trail every execution entry point + // writes through (agent-action-executor.ts's `audit()`), regardless of which path the close took. + await recordAuditEvent(env, { eventType: "agent.action.close", actor: "loopover", targetKey: "owner/repo#7", outcome: "completed", detail: "closed for blacklist" }); + const { action } = await createPendingAgentActionIfAbsent(env, { + repoFullName: "owner/repo", + pullNumber: 7, + installationId: 5, + actionClass: "label", + autonomyLevel: "auto_with_approval", + params: { autonomyClass: "close", closeKind: "blacklist", label: "blacklisted-contributor", labelOp: "add" }, + reason: "blacklisted contributor", + }); + + const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" }); + + expect(result.status).toBe("accepted"); + const { ensurePullRequestLabel: labelApplied } = await import("../../src/github/labels"); + expect(labelApplied).toHaveBeenCalled(); + }); + + it("a label NOT coupled to a close (no autonomyClass: 'close', or no closeKind) is unaffected by the pairing check -- e.g. a plain disposition-communication label", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { label: "auto_with_approval" } }); + 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: "label", + autonomyLevel: "auto_with_approval", + params: { label: "ready-for-review", labelOp: "add" }, + reason: "gate passed", + }); + + const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" }); + + expect(result.status).toBe("accepted"); + const { ensurePullRequestLabel: labelApplied } = await import("../../src/github/labels"); + expect(labelApplied).toHaveBeenCalled(); + }); + + it("a label with closeKind set but autonomyClass NOT 'close' (a label-only copycat gate tier riding its own generic label autonomy) is unaffected by the pairing check", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { label: "auto_with_approval" } }); + 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: "label", + autonomyLevel: "auto_with_approval", + params: { closeKind: "copycat", label: "copycat-match", labelOp: "add" }, + reason: "copycat match (label tier)", + }); + + const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" }); + + expect(result.status).toBe("accepted"); + const { ensurePullRequestLabel: labelApplied } = await import("../../src/github/labels"); + expect(labelApplied).toHaveBeenCalled(); + }); +}); + +describe("#9159: the accept-replay path threads a contributor-cap merge re-check, closing the TOCTOU the executor's own recheck exists for", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("REGRESSION: accept DENIES a staged merge when the author is now over the per-repo contributor-open-PR cap at accept time", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval" } }); + // contributorOpenPrCap is config-as-code only (.loopover.yml), resolved from the focus manifest, not a + // repository_settings DB column (mirrors expectedCiContexts elsewhere in this file). + await upsertRepoFocusManifest(env, "owner/repo", { settings: { contributorOpenPrCap: 1 } }); + await seedInstallation(env); + // The author has ANOTHER open PR in the same repo (#3) -- fetchLivePullRequestState defaults to "open" in + // this suite's mocks, so it live-confirms as still open too. With cap=1, the higher-numbered PR (#7, the + // one about to merge) is the one over cap. + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 3, title: "Sibling PR", state: "open", user: { login: "farmer99" }, head: { sha: "s3" }, labels: [], body: "x" }); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "farmer99" }, 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" }); + + const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" }); + + expect(result.status).toBe("accepted"); // the maintainer's decision itself always records as accepted... + expect(mergePullRequest).not.toHaveBeenCalled(); // ...but the executor's own re-check denied the mutation + const audit = await env.DB.prepare("select outcome, detail from audit_events where event_type = 'agent.action.merge' order by created_at desc limit 1").first<{ outcome: string; detail: string }>(); + expect(audit?.outcome).toBe("denied"); + expect(audit?.detail).toMatch(/now over cap/); + }); + + it("accept EXECUTES a staged merge when the author remains under the per-repo contributor-open-PR cap at accept time", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval" } }); + await upsertRepoFocusManifest(env, "owner/repo", { settings: { contributorOpenPrCap: 5 } }); + await seedInstallation(env); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "farmer99" }, 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" }); + + const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" }); + + expect(result.status).toBe("accepted"); + expect(mergePullRequest).toHaveBeenCalled(); + }); + + it("does not build a recheck at all (zero added cost) when no per-repo cap is configured", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval" } }); + await seedInstallation(env); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "farmer99" }, 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" }); + + const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" }); + + expect(result.status).toBe("accepted"); + expect(mergePullRequest).toHaveBeenCalled(); + }); + + it("does not build a recheck for an author exempt from the auto-close list, even with a cap configured", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval" } }); + await upsertRepoFocusManifest(env, "owner/repo", { settings: { contributorOpenPrCap: 1, autoCloseExemptLogins: ["farmer99"] } }); + await seedInstallation(env); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 3, title: "Sibling PR", state: "open", user: { login: "farmer99" }, head: { sha: "s3" }, labels: [], body: "x" }); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "farmer99" }, 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" }); + + const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" }); + + expect(result.status).toBe("accepted"); + expect(mergePullRequest).toHaveBeenCalled(); + }); +}); diff --git a/test/unit/db-parsers.test.ts b/test/unit/db-parsers.test.ts index e19d997bd..5f8ce79fc 100644 --- a/test/unit/db-parsers.test.ts +++ b/test/unit/db-parsers.test.ts @@ -38,6 +38,8 @@ import { extractLinkedIssueNumbers, extractLinkedIssueNumbersWithOverflow, MAX_LINKED_ISSUE_NUMBERS, + recordLinkedIssueClaims, + getEarliestLinkedIssueClaim, } from "../../src/db/repositories"; import { getDb } from "../../src/db/client"; import { webhookEvents } from "../../src/db/schema"; @@ -265,6 +267,81 @@ describe("database row parser hardening", () => { }); }); + it("REGRESSION (#9160): the per-(PR, issue) claim ledger gives a newly-added issue its OWN fresh claim time, immune to the blended column's overlap-preserve backdating", async () => { + const env = createTestEnv(); + + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-06-29T10:00:00.000Z")); + await upsertPullRequestFromGitHub(env, "owner/repo", { + number: 21, + title: "Placeholder claim", + state: "open", + user: { login: "attacker" }, + labels: [], + body: "Fixes #1", + }); + // #1's ledger row is stamped at the moment it was first observed. + await expect(getEarliestLinkedIssueClaim(env, "owner/repo", 21, [1])).resolves.toBe("2026-06-29T10:00:00.000Z"); + + // Weeks later, the attacker edits the body to ALSO claim a victim's valuable issue #7. The blended + // pull_requests.linked_issue_claimed_at column preserves the day-1 timestamp for the WHOLE PR (the + // #linked-issue-claim-overlap-preserve behavior, intentional for #1) -- but the per-issue ledger must give + // #7 its own fresh clock rather than inheriting #1's, since #7 was never linked to this PR before now. + vi.setSystemTime(new Date("2026-07-20T09:00:00.000Z")); + await upsertPullRequestFromGitHub(env, "owner/repo", { + number: 21, + title: "Backdating attempt", + state: "open", + user: { login: "attacker" }, + labels: [], + body: "Fixes #1\nFixes #7", + }); + const blended = (await listPullRequests(env, "owner/repo")).find((p) => p.number === 21); + // The blended column is unchanged (still day-1) -- this is the pre-existing, intentional behavior for #1. + expect(blended?.linkedIssueClaimedAt).toBe("2026-06-29T10:00:00.000Z"); + // #1's OWN ledger row is untouched (immutable, ON CONFLICT DO NOTHING). + await expect(getEarliestLinkedIssueClaim(env, "owner/repo", 21, [1])).resolves.toBe("2026-06-29T10:00:00.000Z"); + // #7 gets its OWN fresh claim time -- NOT the backdated day-1 value the blended column would suggest. + await expect(getEarliestLinkedIssueClaim(env, "owner/repo", 21, [7])).resolves.toBe("2026-07-20T09:00:00.000Z"); + + // A victim's genuine PR claiming #7 on day 1 (before the attacker's edit) keeps the earlier claim when the + // two are compared on issue #7 specifically -- exactly the comparison resolveScopedLinkedIssueClaimedAt + // (src/queue/duplicate-detection.ts) performs for the duplicate-winner election. + vi.setSystemTime(new Date("2026-07-01T00:00:00.000Z")); + await upsertPullRequestFromGitHub(env, "owner/repo", { + number: 22, + title: "Victim's genuine claim", + state: "open", + user: { login: "victim" }, + labels: [], + body: "Fixes #7", + }); + await expect(getEarliestLinkedIssueClaim(env, "owner/repo", 22, [7])).resolves.toBe("2026-07-01T00:00:00.000Z"); + // Scoped to issue #7 only: the victim's 07-01 claim predates the attacker's 07-20 claim, so the victim + // would correctly win the duplicate-cluster election -- the blended column's day-1 value for PR #21 would + // have wrongly beaten the victim's genuine claim had it been used instead. + const attackerClaim7 = await getEarliestLinkedIssueClaim(env, "owner/repo", 21, [7]); + const victimClaim7 = await getEarliestLinkedIssueClaim(env, "owner/repo", 22, [7]); + expect(victimClaim7! < attackerClaim7!).toBe(true); + }); + + it("recordLinkedIssueClaims is a no-op for an empty issue list, and getEarliestLinkedIssueClaim returns null when nothing has ever been recorded", async () => { + const env = createTestEnv(); + await expect(recordLinkedIssueClaims(env, "owner/repo", 30, [], "2026-01-01T00:00:00.000Z")).resolves.toBeUndefined(); + await expect(getEarliestLinkedIssueClaim(env, "owner/repo", 30, [])).resolves.toBeNull(); + await expect(getEarliestLinkedIssueClaim(env, "owner/repo", 30, [99])).resolves.toBeNull(); + }); + + it("getEarliestLinkedIssueClaim returns the EARLIEST among several contested issue numbers, not merely the first row", async () => { + const env = createTestEnv(); + await recordLinkedIssueClaims(env, "owner/repo", 40, [5, 6], "2026-02-02T00:00:00.000Z"); + // A second call with an overlapping+new set never overwrites #5/#6's existing rows (ON CONFLICT DO + // NOTHING) but DOES seed a fresh row for the genuinely new #8. + await recordLinkedIssueClaims(env, "owner/repo", 40, [5, 6, 8], "2026-03-03T00:00:00.000Z"); + await expect(getEarliestLinkedIssueClaim(env, "owner/repo", 40, [6, 8])).resolves.toBe("2026-02-02T00:00:00.000Z"); + await expect(getEarliestLinkedIssueClaim(env, "owner/repo", 40, [8])).resolves.toBe("2026-03-03T00:00:00.000Z"); + }); + it("falls back to the observed linked-issue claim time when the existing same-claim timestamp is missing", async () => { const env = createTestEnv(); diff --git a/test/unit/linked-issue-label-propagation-fetch.test.ts b/test/unit/linked-issue-label-propagation-fetch.test.ts index 885990bb1..51235128f 100644 --- a/test/unit/linked-issue-label-propagation-fetch.test.ts +++ b/test/unit/linked-issue-label-propagation-fetch.test.ts @@ -922,4 +922,97 @@ describe("fetchLinkedIssueLabelsForPropagation (#priority-linked-issue-gate)", ( expectPropagation(result, []); }); }); + + describe("reward-label gate on direct author/assignee match (#9161)", () => { + // An ADDITIVE mapping (removeOtherTypeLabels: false) composes a bonus label alongside the PR's type label + // rather than replacing it -- this is the reward-semantics signal src/review/linked-issue-label-propagation-fetch.ts's + // fetchLinkedIssueLabelsForPropagation derives `rewardLabels` from (see its own #9161 doc comment). + const REWARD_MAPPING = { issueLabel: "gittensor:priority", prLabel: "gittensor:priority", removeOtherTypeLabels: false, trustMaintainerAuthoredIssueForReward: true }; + const TYPE_MAPPING = { issueLabel: "gittensor:bug", prLabel: "gittensor:bug", removeOtherTypeLabels: true }; + + it("REGRESSION (#9161): withholds the reward label from an issue-authoring contributor who is NOT a repo maintainer, keeping only the type label", async () => { + stubFetch((url) => { + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.endsWith("/issues/21")) + return Response.json({ number: 21, state: "open", user: { login: "contrib" }, assignees: [], labels: ["gittensor:bug", "gittensor:priority"] }); + if (url.includes("/collaborators/contrib/permission")) return Response.json({ permission: "read" }); + return new Response("not found", { status: 404 }); + }); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + const result = await fetchLinkedIssueLabelsForPropagation({ + env, + repoFullName: "owner/repo", + linkedIssues: [21], + installationId: 123, + prAuthorLogin: "contrib", + mappings: [TYPE_MAPPING, REWARD_MAPPING], + }); + // "read" permission is not_maintainer -> the reward label (gittensor:priority) is dropped even though the + // mapping opted into trustMaintainerAuthoredIssueForReward; the type label survives via the unconditional + // author-match unlock, since it carries no reward semantics. + expectPropagation(result, ["gittensor:bug"]); + }); + + it("REGRESSION (#9161): withholds the reward label for a PR author who is only ASSIGNED to (not the author of) the linked issue, same as the author-match case", async () => { + stubFetch((url) => { + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.endsWith("/issues/22")) + return Response.json({ number: 22, state: "open", user: { login: "someone-else" }, assignees: [{ login: "contrib" }], labels: ["gittensor:priority"] }); + if (url.includes("/collaborators/someone-else/permission")) return Response.json({ permission: "read" }); + return new Response("not found", { status: 404 }); + }); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + const result = await fetchLinkedIssueLabelsForPropagation({ + env, + repoFullName: "owner/repo", + linkedIssues: [22], + installationId: 123, + prAuthorLogin: "contrib", + mappings: [REWARD_MAPPING], + }); + expectPropagation(result, []); + }); + + it("propagates the reward label to its own issue's author when that author genuinely IS a repo maintainer (opt-in + maintainer check both satisfied)", async () => { + stubFetch((url) => { + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.endsWith("/issues/23")) + // "owner" is the literal repo owner -- isRepoMaintainerLogin short-circuits to "maintainer" without a + // live collaborator-permission fetch, so no signable key is required for this case. + return Response.json({ number: 23, state: "open", user: { login: "owner" }, assignees: [], labels: ["gittensor:bug", "gittensor:priority"] }); + return new Response("not found", { status: 404 }); + }); + const env = createTestEnv({}); + const result = await fetchLinkedIssueLabelsForPropagation({ + env, + repoFullName: "owner/repo", + linkedIssues: [23], + installationId: 123, + prAuthorLogin: "owner", + mappings: [TYPE_MAPPING, REWARD_MAPPING], + }); + expectPropagation(result, ["gittensor:bug", "gittensor:priority"]); + }); + + it("skips the maintainer-permission check entirely (zero added cost) when the direct-match issue carries no reward-semantic label at all", async () => { + const fetchSpy = vi.fn(async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.endsWith("/issues/24")) return Response.json({ number: 24, state: "open", user: { login: "contrib" }, assignees: [], labels: ["gittensor:bug"] }); + return new Response("not found", { status: 404 }); + }); + vi.stubGlobal("fetch", fetchSpy); + const env = createTestEnv({}); + const result = await fetchLinkedIssueLabelsForPropagation({ + env, + repoFullName: "owner/repo", + linkedIssues: [24], + installationId: 123, + prAuthorLogin: "contrib", + mappings: [TYPE_MAPPING, REWARD_MAPPING], + }); + expectPropagation(result, ["gittensor:bug"]); + expect(fetchSpy.mock.calls.some(([input]) => input.toString().includes("/collaborators/"))).toBe(false); + }); + }); }); diff --git a/test/unit/resolve-scoped-linked-issue-claimed-at.test.ts b/test/unit/resolve-scoped-linked-issue-claimed-at.test.ts new file mode 100644 index 000000000..93c5039c5 --- /dev/null +++ b/test/unit/resolve-scoped-linked-issue-claimed-at.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from "vitest"; +import { resolveScopedLinkedIssueClaimedAt } from "../../src/queue/duplicate-detection"; +import { recordLinkedIssueClaims } from "../../src/db/repositories"; +import type { PullRequestRecord } from "../../src/types"; +import { createTestEnv } from "../helpers/d1"; + +// #9160: resolveScopedLinkedIssueClaimedAt scopes a PR's claim time to only the issue(s) actually contested +// with an open sibling, reading the durable per-(PR, issue) ledger (db/repositories.ts's linked_issue_claims) +// instead of the PR's blended linkedIssueClaimedAt column -- see that function's own doc comment for the +// backdating attack this closes (an attacker's unrelated, already-linked issue "vouching" a stale claim time +// for a newly-added, contested one). +function makePr(number: number, linkedIssues: number[], linkedIssueClaimedAt: string | null): Pick { + return { number, linkedIssues, linkedIssueClaimedAt }; +} + +describe("resolveScopedLinkedIssueClaimedAt (#9160)", () => { + it("returns pr.linkedIssueClaimedAt unchanged when no sibling actually overlaps (no contested issue)", async () => { + const env = createTestEnv(); + const pr = makePr(21, [7], "2026-06-29T10:00:00.000Z"); + const siblings = [makePr(22, [99], "2026-07-01T00:00:00.000Z")]; + await expect(resolveScopedLinkedIssueClaimedAt(env, "owner/repo", pr, siblings)).resolves.toBe("2026-06-29T10:00:00.000Z"); + }); + + it("returns pr.linkedIssueClaimedAt unchanged when openSiblings is empty", async () => { + const env = createTestEnv(); + const pr = makePr(21, [7], "2026-06-29T10:00:00.000Z"); + await expect(resolveScopedLinkedIssueClaimedAt(env, "owner/repo", pr, [])).resolves.toBe("2026-06-29T10:00:00.000Z"); + }); + + it("REGRESSION (#9160): reads the per-issue ledger's own claim time for a CONTESTED issue, ignoring the blended column's backdated value", async () => { + const env = createTestEnv(); + // Ledger: issue #1 claimed day-1 (long-lived placeholder), issue #7 claimed later (the backdating target). + await recordLinkedIssueClaims(env, "owner/repo", 21, [1], "2026-06-29T10:00:00.000Z"); + await recordLinkedIssueClaims(env, "owner/repo", 21, [7], "2026-07-20T09:00:00.000Z"); + // pr's blended column still reads day-1 (the #linked-issue-claim-overlap-preserve behavior) even though + // #7 was only just added -- resolveScopedLinkedIssueClaimedAt must NOT trust this for the contested issue. + const pr = makePr(21, [1, 7], "2026-06-29T10:00:00.000Z"); + const victimSibling = [makePr(22, [7], "2026-07-01T00:00:00.000Z")]; + await expect(resolveScopedLinkedIssueClaimedAt(env, "owner/repo", pr, victimSibling)).resolves.toBe("2026-07-20T09:00:00.000Z"); + }); + + it("fails closed to null when the contested issue has no ledger row yet (never falls back to the blended value)", async () => { + const env = createTestEnv(); + const pr = makePr(21, [7], "2026-06-29T10:00:00.000Z"); // blended column set, but no ledger row for #7 + const siblings = [makePr(22, [7], "2026-07-01T00:00:00.000Z")]; + await expect(resolveScopedLinkedIssueClaimedAt(env, "owner/repo", pr, siblings)).resolves.toBeNull(); + }); + + it("scopes to only the CONTESTED issue among several linked issues, ignoring an uncontested one entirely", async () => { + const env = createTestEnv(); + await recordLinkedIssueClaims(env, "owner/repo", 21, [1], "2026-01-01T00:00:00.000Z"); + await recordLinkedIssueClaims(env, "owner/repo", 21, [7], "2026-07-20T09:00:00.000Z"); + const pr = makePr(21, [1, 7], "2026-01-01T00:00:00.000Z"); + // Sibling only overlaps on #7, not #1 -- the (much earlier) #1 ledger row must not leak into the result. + const siblings = [makePr(22, [7], "2026-07-01T00:00:00.000Z")]; + await expect(resolveScopedLinkedIssueClaimedAt(env, "owner/repo", pr, siblings)).resolves.toBe("2026-07-20T09:00:00.000Z"); + }); + + it("takes the EARLIEST ledger row when multiple linked issues are each contested by (possibly different) siblings", async () => { + const env = createTestEnv(); + await recordLinkedIssueClaims(env, "owner/repo", 21, [1], "2026-05-01T00:00:00.000Z"); + await recordLinkedIssueClaims(env, "owner/repo", 21, [7], "2026-07-20T09:00:00.000Z"); + const pr = makePr(21, [1, 7], "2026-05-01T00:00:00.000Z"); + const siblings = [makePr(22, [1], "2026-06-01T00:00:00.000Z"), makePr(23, [7], "2026-07-01T00:00:00.000Z")]; + await expect(resolveScopedLinkedIssueClaimedAt(env, "owner/repo", pr, siblings)).resolves.toBe("2026-05-01T00:00:00.000Z"); + }); +}); diff --git a/test/unit/rules.test.ts b/test/unit/rules.test.ts index e1a015797..3cd0bb8a3 100644 --- a/test/unit/rules.test.ts +++ b/test/unit/rules.test.ts @@ -450,6 +450,60 @@ describe("advisory rules", () => { expect(advisory.findings.map((finding) => finding.code)).not.toContain("duplicate_pr_risk_unconfirmed"); }); + it("#9160: scopedLinkedIssueClaimedAt overrides pr's blended linkedIssueClaimedAt for the winner election, closing the backdating attack", () => { + // The attacker's PR carries a backdated BLENDED linkedIssueClaimedAt (from an old, unrelated linked issue + // whose overlap preserved it) that would make it win against the victim if the raw column were used + // directly -- see db-parsers.test.ts's REGRESSION (#9160) test for how the per-(PR, issue) ledger produces + // the correctly-scoped value the caller (queue/duplicate-detection.ts's resolveScopedLinkedIssueClaimedAt) + // passes in as scopedLinkedIssueClaimedAt. + const attacker: PullRequestRecord = { + repoFullName: repo.fullName, + number: 21, + title: "Backdated claim", + state: "open", + authorLogin: "attacker", + authorAssociation: "NONE", + headSha: "abc123", + labels: [], + linkedIssues: [7], + linkedIssueClaimedAt: "2026-06-29T10:00:00.000Z", // blended column: backdated via an unrelated issue #1 + }; + const victim: PullRequestRecord = { ...attacker, number: 22, title: "Victim's genuine claim", authorLogin: "victim", linkedIssueClaimedAt: "2026-07-01T00:00:00.000Z" }; + + // Without scoping, the attacker's blended (backdated) timestamp wins -- confirms the vulnerable baseline. + const unscoped = buildPullRequestAdvisory(repo, attacker, { otherOpenPullRequests: [victim], duplicateWinnerEnabled: true }); + expect(unscoped.findings.map((finding) => finding.code)).not.toContain("duplicate_pr_risk"); + + // With the per-issue-scoped claim time (the attacker's OWN ledger row for issue #7, correctly dated AFTER + // the victim's genuine claim), the attacker is correctly demoted to loser. + const scoped = buildPullRequestAdvisory(repo, attacker, { + otherOpenPullRequests: [victim], + duplicateWinnerEnabled: true, + scopedLinkedIssueClaimedAt: "2026-07-20T09:00:00.000Z", + }); + expect(scoped.findings.map((finding) => finding.code)).toContain("duplicate_pr_risk_unconfirmed"); + }); + + it("#9160: scopedLinkedIssueClaimedAt undefined falls back to pr.linkedIssueClaimedAt, byte-identical to before this field existed", () => { + const winner: PullRequestRecord = { + repoFullName: repo.fullName, + number: 12, + title: "Add registry sync", + state: "open", + authorLogin: "oktofeesh1", + authorAssociation: "NONE", + headSha: "abc123", + labels: [], + linkedIssues: [4], + linkedIssueClaimedAt: "2026-06-29T10:00:00.000Z", + }; + const higherSibling: PullRequestRecord = { ...winner, number: 13, title: "Alternative registry sync", linkedIssueClaimedAt: "2026-06-29T10:01:00.000Z" }; + + const advisory = buildPullRequestAdvisory(repo, winner, { otherOpenPullRequests: [higherSibling], duplicateWinnerEnabled: true, scopedLinkedIssueClaimedAt: undefined }); + + expect(advisory.findings.map((finding) => finding.code)).not.toContain("duplicate_pr_risk"); + }); + it("keeps weak queue warnings advisory-only for the opt-in gate", () => { const pr: PullRequestRecord = { repoFullName: repo.fullName, diff --git a/worker-configuration.d.ts b/worker-configuration.d.ts index 94c3666d0..486ab2b33 100644 --- a/worker-configuration.d.ts +++ b/worker-configuration.d.ts @@ -1,6 +1,6 @@ /* eslint-disable */ // Generated by Wrangler by running `wrangler types` (hash: 12ab7fcab3ef47d0e7c428770fc3c36a) -// Runtime types generated with workerd@1.20260701.1 2026-05-28 nodejs_compat +// Runtime types generated with workerd@1.20260722.1 2026-05-28 nodejs_compat interface __BaseEnv_Env { REVIEW_AUDIT: R2Bucket; VISUAL_CAPTURE_PUBLIC: R2Bucket; @@ -636,7 +636,7 @@ declare abstract class DurableObjectNamespace; jurisdiction(jurisdiction: DurableObjectJurisdiction): DurableObjectNamespace; } -type DurableObjectJurisdiction = "eu" | "fedramp" | "fedramp-high"; +type DurableObjectJurisdiction = "eu" | "fedramp" | "fedramp-high" | "us"; interface DurableObjectNamespaceNewUniqueIdOptions { jurisdiction?: DurableObjectJurisdiction; } @@ -12331,6 +12331,13 @@ interface ForwardableEmailMessage extends EmailMessage { * @returns A promise that resolves when the email message is replied. */ reply(message: EmailMessage): Promise; + /** + * Reply to the sender of this email message with a message built from the given + * fields. Threading headers (In-Reply-To/References) are set automatically. + * @param builder The reply message contents. + * @returns A promise that resolves when the email message is replied. + */ + reply(builder: EmailReplyMessageBuilder): Promise; } /** A file attachment for an email message */ type EmailAttachment = { @@ -12351,23 +12358,46 @@ interface EmailAddress { name: string; email: string; } +/** + * Recipient fields for `SendEmail.send()`. At least one of `to`, `cc`, or + * `bcc` must be provided. + */ +type EmailDestinations = { + to?: string | EmailAddress | (string | EmailAddress)[]; + cc?: string | EmailAddress | (string | EmailAddress)[]; + bcc?: string | EmailAddress | (string | EmailAddress)[]; +} & ({ + to: string | EmailAddress | (string | EmailAddress)[]; +} | { + cc: string | EmailAddress | (string | EmailAddress)[]; +} | { + bcc: string | EmailAddress | (string | EmailAddress)[]; +}); +/** + * Fields shared by all composed emails (no recipients). Used directly by + * `ForwardableEmailMessage.reply()`, which always replies to the original + * sender, and extended by `EmailMessageBuilder` for `SendEmail.send()`. + */ +interface EmailReplyMessageBuilder { + from: string | EmailAddress; + subject: string; + replyTo?: string | EmailAddress; + headers?: Record; + text?: string; + html?: string; + attachments?: EmailAttachment[]; +} +/** + * Fields for composing an email without constructing raw MIME, for + * `SendEmail.send()`. Requires at least one of `to`, `cc`, or `bcc`. + */ +type EmailMessageBuilder = EmailReplyMessageBuilder & EmailDestinations; /** * A binding that allows a Worker to send email messages. */ interface SendEmail { send(message: EmailMessage): Promise; - send(builder: { - from: string | EmailAddress; - to: string | EmailAddress | (string | EmailAddress)[]; - subject: string; - replyTo?: string | EmailAddress; - cc?: string | EmailAddress | (string | EmailAddress)[]; - bcc?: string | EmailAddress | (string | EmailAddress)[]; - headers?: Record; - text?: string; - html?: string; - attachments?: EmailAttachment[]; - }): Promise; + send(builder: EmailMessageBuilder): Promise; } declare abstract class EmailEvent extends ExtendableEvent { readonly message: ForwardableEmailMessage; @@ -13189,6 +13219,11 @@ declare namespace CloudflareWorkersModule { export type WorkflowDurationLabel = 'second' | 'minute' | 'hour' | 'day' | 'week' | 'month' | 'year'; export type WorkflowSleepDuration = `${number} ${WorkflowDurationLabel}${'s' | ''}` | number; export type WorkflowDelayDuration = WorkflowSleepDuration; + export type WorkflowDynamicDelayContext = { + ctx: WorkflowStepContext; + error: Error; + }; + export type WorkflowDelayFunction = (input: WorkflowDynamicDelayContext) => WorkflowDelayDuration | Promise; export type WorkflowTimeoutDuration = WorkflowSleepDuration; export type WorkflowRetentionDuration = WorkflowSleepDuration; export type WorkflowBackoff = 'constant' | 'linear' | 'exponential'; @@ -13196,7 +13231,7 @@ declare namespace CloudflareWorkersModule { export type WorkflowStepConfig = { retries?: { limit: number; - delay: WorkflowDelayDuration | number; + delay: WorkflowDelayDuration | number | WorkflowDelayFunction; backoff?: WorkflowBackoff; }; timeout?: WorkflowTimeoutDuration | number; @@ -13222,13 +13257,22 @@ declare namespace CloudflareWorkersModule { type: string; sensitive?: WorkflowStepSensitivity; }; - export type WorkflowStepContext = { + export type WorkflowStepContext = { step: { name: string; count: number; }; attempt: number; - config: WorkflowStepConfig; + config: { + retries?: { + limit: number; + backoff?: WorkflowBackoff; + } & (Delay extends WorkflowDelayFunction ? {} : { + delay: WorkflowDelayDuration | number; + }); + timeout?: WorkflowTimeoutDuration | number; + sensitive?: WorkflowStepSensitivity; + }; }; export type WorkflowRollbackContext = { ctx: WorkflowStepContext; @@ -13244,7 +13288,9 @@ declare namespace CloudflareWorkersModule { }; export abstract class WorkflowStep { do>(name: string, callback: (ctx: WorkflowStepContext) => Promise, rollbackOptions?: WorkflowStepRollbackOptions): Promise; - do>(name: string, config: WorkflowStepConfig, callback: (ctx: WorkflowStepContext) => Promise, rollbackOptions?: WorkflowStepRollbackOptions): Promise; + do, const C extends WorkflowStepConfig>(name: string, config: C, callback: (ctx: WorkflowStepContext) => Promise, rollbackOptions?: WorkflowStepRollbackOptions): Promise; sleep: (name: string, duration: WorkflowSleepDuration) => Promise; sleepUntil: (name: string, timestamp: Date | number) => Promise; waitForEvent>(name: string, options: { @@ -14154,6 +14200,7 @@ declare namespace TailStream { readonly dispatchNamespace?: string; readonly entrypoint?: string; readonly executionModel: string; + readonly durableObjectId?: string; readonly scriptName?: string; readonly scriptTags?: string[]; readonly scriptVersion?: ScriptVersion; @@ -14708,6 +14755,13 @@ interface WorkflowError { code?: number; message: string; } +interface WorkflowInstanceTerminateOptions { + /** + * If true, run registered rollback handlers before terminating the instance. + * Only steps that registered rollback handlers are rolled back. + */ + rollback?: boolean; +} interface WorkflowInstanceRestartOptions { /** * Restart from a specific step. If omitted, the instance restarts from the beginning. @@ -14741,8 +14795,9 @@ declare abstract class WorkflowInstance { public resume(): Promise; /** * Terminate the instance. If it is errored, terminated or complete, an error will be thrown. + * @param options Options for termination, including whether registered rollback handlers should run. */ - public terminate(): Promise; + public terminate(options?: WorkflowInstanceTerminateOptions): Promise; /** * Restart the instance. Optionally restart from a specific step, preserving * cached results for all steps before it.