diff --git a/.env.example b/.env.example index 6b38f34b..ee7fef7f 100644 --- a/.env.example +++ b/.env.example @@ -328,6 +328,12 @@ DISPATCH_AGENT_TOKEN="your_agent_token_here" # worker-side readiness check. Bounded to 60 000–604 800 000; default 21 600 000 (6h). # PR_FIX_EXTERNAL_WAIT_COOLDOWN_MS=21600000 +# Bounded cooldown (ms) applied to an explicit capacity-exhaustion handoff +# (#1256). Scoped to the exhausted capacity on the handed-off item only — the +# work stays dispatchable to unrelated capacities. Bounded to 60 000–21 600 000; +# default 900 000 (15 min). See docs/capacity-handoff.md. +# DISPATCH_CAPACITY_COOLDOWN_MS=900000 + # --- Legacy aliases (do not set) --- # MISSION_CONTROL_URL and MISSION_CONTROL_AGENT_TOKEN were renamed to diff --git a/AGENTS.md b/AGENTS.md index d817fe68..0010def3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -49,6 +49,7 @@ npm run db:deploy # Deploy migrations (prod) | `DISPATCH_GROOMER_INTERVAL_MS` | No | Groomer run interval (default 600000 = 10m; `0` disables) | | `DISPATCH_PR_FOLLOWUP_INTERVAL_MS` | No | PR-followup sync interval (default 900000 = 15m; `0` disables) | | `DISPATCH_PRUNE_CLOSED_INTERVAL_MS` | No | Closed-issue prune interval (default 86400000 = 24h; `0` disables) | +| `DISPATCH_CAPACITY_COOLDOWN_MS` | No | Bounded cooldown for explicit capacity-exhaustion handoffs (default 900000 = 15m, clamped 1m–6h). Scoped to the exhausted capacity on the handed item only; unrelated capacities keep dispatch. See [docs/capacity-handoff.md](./docs/capacity-handoff.md) | | `DISPATCH_QUEUE_ADMISSION_MODE` | No | Grooming freshness admission for worker pickup: `off` (default, unchanged), `audit` (annotate queue items, filter nothing) or `enforce` (withhold ready issues without a current applied grooming decision or operator override). See [docs/hosted-groomer.md](./docs/hosted-groomer.md#worker-admission). | Resolution order: `DATABASE_URL` > `DISPATCH_DATABASE_URL`. `DISPATCH_AGENT_TOKEN` for agent API bearer auth. @@ -211,6 +212,7 @@ Cross-cutting concerns and where they live. Read these first when working in tha | Agent claim / unclaim (lease + status) | `app/api/issues/claim/route.ts`, `app/api/issues/unclaim/route.ts`, `lib/lease.ts` | | Assign / unassign (label-only, no lease) | `app/api/issues/actions/route.ts`, `app/api/issues/unassign/route.ts`, `lib/assignment-conflicts.ts` | | AgentWork lifecycle | `app/api/agent-work/route.ts`, `app/api/agent-work/{start,checkpoint,finish}/route.ts`, `lib/agent-work.ts` | +| Fenced capacity handoff (#1256) | `app/api/issues/capacity-handoff/route.ts`, `lib/issue-capacity-handoff.ts`, `lib/capacity-handoff.ts`, `lib/pr-fix-queue.ts` (report resolver), `lib/lease.ts` (claim token), [docs/capacity-handoff.md](./docs/capacity-handoff.md) | | Auth & audit actor identity | `lib/auth.ts` (`authorizeRequest`, `getAuthorizedActor`) | | Lifecycle event stream (claim/status evidence, #1202) | `lib/issue-lifecycle.ts`, `docs/issue-lifecycle-events.md` | | Board UI component tree | `app/board/page.tsx` → `kanban-board-client.tsx` → `components/kanban-board.tsx` → `components/issue-card.tsx` | @@ -317,9 +319,14 @@ The `tasks/report` endpoint accepts these outcomes: | `no_changes_needed` | No action was required | | `already_addressed` | A PR-fix attempt whose feedback was already handled — settles the queue item with an optional evidence string (commit SHAs/paths) and no new push (#1121). Still gated on PR merge state; a repeated same-evidence disagreement with an unchanged head reopens via #940 and bounds to BLOCKED at `PR_FIX_MAX_ATTEMPTS`. | | `handed-off` | The worker accepted the task but handed it to another process; records a neutral terminal run status and preserves supplied issue/PR identity without settling the PR-fix queue. Independent pr-followup sync owns ingestion. | +| `capacity-exhausted` | Explicit recognition that the worker's execution capacity cannot run the handed PR-fix attempt (#1256). Requires the issued `prFixItem` token (including its per-hand-out `handoutToken` receipt) and a `capacity` identifier; releases exactly the issued hand-out without spending repair budget and records a capacity-scoped cooldown. For claimed issue work use `POST /api/issues/capacity-handoff` instead. | Queue-backed `followup-pr` reports must echo the `prFixItem.{id, generation}` token issued by `next-task` on the task. Reports without it still record the AgentRun, but never settle PR-fix queue state — the item stays queued and the report is a no-op against the queue (#1074). +Workers may report `outcome: "capacity-exhausted"` only when a recognized execution capacity is genuinely unavailable, with a non-empty `capacity` identifier (an opaque worker-chosen name, for example a provider/model or runner pool) and the issued `prFixItem: { id, generation, handoutToken }` token. Dispatch fences the settlement to the **exact issued hand-out**: the `handoutToken` is a per-hand-out receipt next-task mints (rotated on every issuance), and the release removes only the reporting agent's receipt/hand-out/ack — other agents' live hand-outs at that generation are preserved, and the hand-out stamps clear only when the last live hand-out is consumed. While the item is still `QUEUED` the attempt token is consumed but the item **stays `QUEUED` at the same generation with the same `fixAttempts` count, branch, and PR identity** — no repair budget is spent and there is no global wait. The suppression is **capacity-scoped**: Dispatch records a bounded cooldown (`DISPATCH_CAPACITY_COOLDOWN_MS`, default 15m, clamped to 1m–6h) for that capacity on that item only. `next-task` requests may declare `?capacity=`; a request whose capacity matches the exhausted one skips the item until the cooldown expires, an unrelated capacity is never suppressed, and a request that declares no capacity conservatively skips cooled items. The capacity is caller-declared trusted *scoping*, never identity: settlement authority comes only from the issued receipt token, the cooldown deliberately does not filter by agent (the same worker with a different, unrelated capacity is legitimately allowed), and lying about a capacity grants nothing beyond re-acquiring work for the capacity one claims to have exhausted. A `capacity_handoff` history row and a `CapacityHandoff` row record the transition. A changed generation, an already-settled status, or a human hold (BLOCKED/PARKED/NEEDS_HUMAN) is never overwritten — the report settles as a no-op. Do not use this outcome for worker-local failures, ordinary errors, or an unrecognized capacity. + +For **claimed issue work** the fenced counterpart is `POST /api/issues/capacity-handoff` (bearer-auth; bound workers only for their own agent). The worker presents the `claimToken` that `POST /api/issues/claim` issued on the claim (also surfaced by `active-work` resume context and the MCP `claim_issue` result). A persistent per-issue `IssueMutationGuard` serializes cooperating claim, `upsertLease`, and release mutations under a PostgreSQL advisory lock. Dispatch fences release to that exact claim (agent + lease row + token, verified against live GitHub labels); if ownership changed during verification, cleanup removes only this worker's agent label and never changes status. Human holds (closed/done, any live `status/blocked`, a cached block carrying a `blockedReason`) and changed/missing ownership are refused. The lease remains until remote label cleanup succeeds; then it is consumed, active AgentWork rows are retired with history, an audit row is written, and the same capacity-scoped cooldown applies to issue pickup. Failed cleanup retains the lease and guard so an exact-token retry can resume. Guards are persistent and have no automatic timeout: after a process crash, an operator must confirm the process stopped and reconcile live GitHub state before removing one. Branch/PR identity on the lease and work history is retained; nothing is re-pushed and no repair budget exists for issue work. See [docs/capacity-handoff.md](./docs/capacity-handoff.md) for the lease/label effects and the GitHub-vs-database consistency boundary. + The report body also accepts an optional `startedAt` — an extended ISO 8601 timestamp with timezone (`Z` or `±HH:MM`), for example `2026-10-03T04:20:58Z` or `2026-10-03T04:20:58.123456+00:00`, marking when the worker began the task. When it parses, is not in the future, and is no more than 24 hours before the report, it is stored as the run's start time so the run records a real duration; missing, malformed, future, or too-old values silently fall back to the report time (never a 400). The raw body value participates in the idempotency payload identity, so an identical-body retry always replays as a duplicate. An accepted `startedAt` is normalized to UTC (`Z`) form when stored and echoed back in the response. The echo reflects re-validation of the current request, so on an idempotent duplicate replay the echoed `startedAt` may be absent even though the original accepted run stored it — for example when an identical-body retry lands more than 24 hours after the reported start (the stored AgentRun is unaffected). #### Idempotent reporting (optional) diff --git a/docs/capacity-handoff.md b/docs/capacity-handoff.md new file mode 100644 index 00000000..708737a4 --- /dev/null +++ b/docs/capacity-handoff.md @@ -0,0 +1,228 @@ +# Fenced Capacity-Exhaustion Handoff (#1256) + +Dispatch-native recovery for the case "the worker holding a handed attempt +cannot actually run it because its execution capacity is exhausted". The work +must return to circulation **for other compatible workers** without letting a +stale worker stomp a newer claim, a human hold, or anyone's retry budget. + +This is a generic Dispatch capability. The `capacity` identifier is an opaque, +worker-chosen string (for example `provider/model-a` or `runner-pool-gpu`); +Dispatch never interprets it and carries no coupling to any particular +executor, provider, or source product. + +## The two surfaces + +| Work form | Operation | Fencing token (issued by) | +|---|---|---| +| PR-fix queue attempt | `POST /api/agents/{agent}/tasks/report` with `outcome: "capacity-exhausted"`, `capacity`, and the issued `prFixItem: { id, generation, handoutToken }` | `next-task` hand-out (#1074 token + #1256 per-hand-out receipt token) | +| Claimed issue | `POST /api/issues/capacity-handoff` with `issueId`, `repoFullName`, `issueNumber`, `agentName`, `claimToken`, `capacity` | `POST /api/issues/claim` response `claimToken` (also in the `active-work` resume context) | + +Both require bearer auth; a bound worker credential (`DISPATCH_WORKER_TOKENS`) +may only hand off its **own** agent's work, and the unbound legacy worker token +is refused (#1129). A handoff must be an *explicit* recognition: a non-empty, +bounded (≤200 chars) `capacity` and a valid issued token are hard 400 gates. + +## Attempt tokens and fencing + +* **PR-fix**: settlement is a compare-and-set on `(item id, status QUEUED, + generation)` — a stale generation (a fresh attempt was re-issued meanwhile), + an already-settled status, or a human hold (BLOCKED / PARKED / NEEDS_HUMAN) + settles as a no-op; the report never overwrites newer state. For + `capacity-exhausted` the CAS is bound further, to the **exact source + attempt**: next-task mints a fresh opaque `handoutToken` per hand-out and + persists it as a receipt (`PrFixQueueItem.handoutReceipts`, one entry per + live hand-out, format `token@agent@generation`). The report must present + that token, and the write — pinned on the full receipt/hand-out/ack lists — + removes **only the reporting agent's** receipt, hand-out record, and own + ack. An unrelated agent that merely knows `(id, generation)` has no + authority, and neither does the *same* agent presenting a token from a + **previous** hand-out of the same generation (release keeps the generation, + so generation alone cannot distinguish hand-outs). A receipt exists exactly + while its `agentHandouts` entry exists — issuance pushes both in one + conditional write and every clear site clears both — so a released, + reclaimed, or generation-superseded hand-out can never be settled with its + old token. Pre-receipt (legacy) hand-outs cannot settle this outcome; every + other outcome ignores the token and settles unchanged. +* **Issue**: the claim token lives on the `Lease` row (`Lease.claimToken`, + unique, minted at lease creation). Renewals deliberately keep the token (a + heartbeating worker keeps its receipt), but a released or expired lease that + is re-claimed is a **new row with a new token** — an old worker's token is + permanently inert against a newer claim. A durable `IssueMutationGuard` + serializes cooperating claim, lease-upsert, and capacity-release mutations + per issue. The release retains the lease while it performs remote cleanup; + only after GitHub cleanup succeeds does it CAS-delete + `(lease id, agentName, issueId, expiredAt > now)`. A failed CAS is an + invariant/bypass race, not the normal inter-writer coordination mechanism. + +## Human-hold and ownership safeguards (issue surface) + +Checked **before** any write; every refusal is 409/400 with zero mutation: + +* closed issue, cached or **live** `status/done`; +* deliberate block: cached `status/blocked` carrying a `blockedReason`, or + **any live `status/blocked`** — fail-closed: a block applied on GitHub that + has not reached the cache is still a hold, and releasing it is an explicit + operator action, never a side effect of a capacity handoff; +* ownership changed: the **live GitHub** label set (not the cache — #1037 + rule) carries a *different* `agent/*` label, or *no* agent label at all + (already released or human-cleared). +* GitHub unreachable → 503 fail-closed: no lease delete, no label write. + +**Post-read ownership re-verification**: a force-claim (which does not delete +the previous agent's lease row) or a human change can land during label +verification. If ownership moved, the release narrows to +**ownership-only** — our own `agent/*` label is removed idempotently and the +cache mirrors the freshest known set minus that label — and no status label +is ever touched, so a newer owner's transition survives. A re-read failure +returns 503 without remote writes, retaining the lease and guard for retry. +A blocked/done hold found on either read is refused without label or lease +mutation; ownership-only cleanup is reserved for genuine changed owners. + +## What success does + +**Issue handoff** (`issue_capacity_handoff` audit action): + +1. The operation creates a committed per-issue durable mutation guard with + `executing: true`. Cooperating claim and `upsertLease` writers cannot enter + during release. Only an exact agent+token retry can atomically resume a + retained `capacity-release` guard with `executing: false`; unrelated + operations and retries while executing are refused as busy. +2. Claim cleanup uses the shared `releaseIssueClaim` policy, based on live + labels: `agent/` removed; `status/in-progress` → `status/ready` + (immediately discoverable to another compatible worker); `status/in-review` + is retained with a status note. If ownership changed during verification, + cleanup removes only our own agent label and never changes status. +3. Only after remote label cleanup succeeds is the exact lease CAS-deleted. + Active `AgentWork` rows are then retired with history, followed by the + `CapacityHandoff` cooldown (`kind: "issue"`) and audit record. + +**PR-fix handoff** (`capacity_handoff` PrFixHistory action): + +1. Exact-owner CAS (see “Attempt tokens and fencing”): removes the reporting + agent's receipt, hand-out record, and own ack — and nothing else. Other + agents' live receipts, hand-out records, and acknowledgements at the same + generation are preserved, and the dispatch stamps + (`dispatchedGeneration/At`) plus `postDispatchEvidenceKeys` survive while + any *other* hand-out at that generation is still live (clearing them would + reclassify the surviving workers' mid-run evidence as pre-dispatch, #1119); + only the report that consumes the last live hand-out resets them, making + the generation freshly dispatchable. The item **stays `QUEUED`** at the + same `generation` and `fixAttempts`, with `branch`, `url`, and PR state + untouched — the attempt is returned, not burned, and no repair budget is + spent. It is explicitly *not* a `WAITING_EXTERNAL` park: nothing hides the + item from workers whose capacity is unrelated. +2. A `CapacityHandoff` row is written (`kind: "pr_fix"`, recording the + settled `generation`). +3. Idempotency is layered: a repeat report of the **same capacity at the + same generation** while the cooldown is active replays as `deferred` + without minting a second window; concurrent duplicates are fenced by the + consumed receipt (only one write can remove the exact token) and by the + CAS pinning the issued hand-out stamp (`dispatchedGeneration = + generation`) and the full receipt/hand-out/ack lists, so exactly one + duplicate can win and losers settle as no-ops with zero mutation. A + *different* capacity exhausting the same live generation records its own + scoped cooldown (it was legitimately dispatched alongside or after the + first hand-off, with its own receipt); an older generation's window never + absorbs a fresh attempt. + +## Capacity-scoped cooldown + +**Trust boundary.** The `capacity` identifier is **caller-declared and +trusted for scoping only** — it is a shaping hint, not an identity claim. +Settlement authority comes exclusively from the issued receipt/claim token, +never from the capacity string, and the cooldown deliberately does **not** +filter by agent: the same worker legitimately declares different capacities +(a different model or runner pool behind one agent name), and an unrelated +capacity must keep receiving the work. A worker that lies about its capacity +can only re-acquire work for a capacity it claims to have exhausted — it +consumes no extra repair budget, and its own hand-out is fenced from anyone +else. Given the active +(`cooldownUntil > now`) handoff rows: + +* requester's capacity **equals** the exhausted capacity (matched + case-insensitively — the identifier is opaque and `Model-A` re-declared is + the same capacity) → the cooled item is skipped until the deadline; +* requester declares an **unrelated** capacity → the item dispatches normally + (this is the entire point of scoping the cooldown); +* requester declares **no capacity** → conservatively skips *any* cooled item + (Dispatch cannot prove it is not the exhausted capacity). Legacy workers are + thus protected until expiry without globalizing the wait for declared, + unrelated capacities. + +Cooldown bound: `DISPATCH_CAPACITY_COOLDOWN_MS` (default 15 minutes, clamped +to [1 minute, 6 hours]). Expired rows are pruned lazily on read, throttled to +once per minute per process so the prune never becomes a per-poll write on the +dispatch hot path. When every remaining candidate is cooled, `next-task` says +so in the idle reason instead of a bare "No work available". A cooldown read +failure **fails open** (dispatch proceeds unfiltered): handing cooled work to +the exhausted capacity is a bounded nuisance the worker reports again; +blocking the whole lane is not. Symmetrically, a cooldown **write** failure +after the release already committed does not fail the handoff — the response +returns `cooldownUntil: null` (audited), because the ownership release is the +operation the worker asked for and a false 500 would invite a retry that can +only 409. + +The exhausted capacity's own re-report is also fenced: the PR-fix attempt +token is consumed and the issue lease is gone, so the same worker cannot +double-settle; at worst it re-receives the item after the cooldown, which is +the intended bounded retry for that capacity. + +## Label / lease effects and consistency boundaries + +**GitHub is the source of truth for the issue's labels and status. The +database (Issue cache, Lease, AgentWork, CapacityHandoff, PrFixQueueItem) is +Dispatch's coordination layer — never write it as if GitHub were behind.** + +* Ownership is *verified* against live GitHub labels before any mutation; the + Prisma cache is used only for non-label facts (`blockedReason`, + `linkedPrNumber`, closed state) and the post-write mirror. +* Label writes are targeted (`removeIssueLabel` for the agent label, + `transitionIssueStatus` for the status swap), never a full label-set + replacement, so concurrent human label changes on GitHub are not reverted. +* A durable `IssueMutationGuard` serializes cooperating claim, + `upsertLease`, and capacity-release writers for an issue. It is a persistent + singleton row per issue, not a lease: there is no automatic timeout or + stale-guard clearing. The `executing` flag is a durable, nonexpiring + execution fence: callback work runs outside database transactions, and only + the matching owner can reopen a completed-but-incomplete attempt. A crash + leaves `executing: true`; an operator must confirm process termination + before resetting it to false. Routine local-only lease failures remove + their own guard safely. A failed claim can resume under the same agent's + nonexecuting guard: it re-verifies live ownership and human holds, repairs + the cache plus lifecycle event transaction and lease, and does not steal a + changed claim. Other agents cannot enter until reconciliation finishes. +* Capacity release retains its lease until GitHub label cleanup succeeds; the + exact lease CAS delete follows remote cleanup. A cleanup failure retains both + lease and guard for an exact-token retry. Failed GitHub reads/removals must + not be represented as a successful cache mirror or lease release. After a + crash, an operator must confirm the prior process stopped and reconcile live + GitHub state before removing the guard. There is no timeout-based automatic + recovery that could overlap an in-flight remote mutation. +* `CapacityHandoff` rows are Dispatch-local dispatch shaping. They are not + mirrored to GitHub, carry no label, and self-expire; deleting them is always + safe. +* The PR-fix cooldown deliberately does not touch PR state on GitHub at all: + no label, no comment, no push. Queue-row changes are cache-side work + bookkeeping; the PR/branch identity is retained exactly. +* Lease expiry alone never clears labels — expiry is a *staleness* signal + owned by stale-claim recovery; capacity handoff is an *intentional* release + owned by the worker holding the token. + +## Failure-mode summary + +| Situation | Result | +|---|---| +| Missing/blank capacity, attempt token, or receipt token (PR-fix) | 400 | +| PR-fix receipt token not owned by the reporting agent (unknown, rotated, or another agent's) | report `skipped`, queue untouched | +| Wrong agent's token (bound worker) | 403 | +| GitHub labels unreadable | 503, zero writes | +| Stale claim token (re-claimed lease) | 409 `stale_claim_token` | +| Expired lease | 409 `claim_expired` (stale recovery owns release) | +| Different/missing live `agent/*` label | 409 `ownership_changed` | +| Deliberate block / done / closed | 409/400 human hold | +| Another cooperating issue mutation is active / guard is not the exact resumable release | 409 busy response; no remote mutation | +| Cleanup fails after guard acquisition | Lease and durable guard retained; exact-token retry can resume | +| Lease CAS unexpectedly misses after remote cleanup (bypassing writer) | 409 `handoff_race_lost`; no cooldown is recorded; reconcile GitHub and lease state | +| Guard remains after process crash | Operator confirms prior process stopped and reconciles GitHub before removal; no automatic timeout | +| PR-fix generation moved / status settled | report `skipped`, queue untouched | +| Repeat PR-fix report | `deferred`, one cooldown window only | diff --git a/docs/worker-execution-contract.md b/docs/worker-execution-contract.md index bd75adfa..4549b7d1 100644 --- a/docs/worker-execution-contract.md +++ b/docs/worker-execution-contract.md @@ -316,3 +316,26 @@ Behavioral notes: `status: "FIXED"` + `alreadyAddressed: true` + optional `evidence` (bearer marks still require a generation; the QUEUED/IGNORED maintainer-only gate is unchanged). + +## Fenced capacity-exhaustion handoff (#1256) + +A worker whose execution capacity cannot run a handed attempt returns it with +an explicit, token-fenced handoff instead of failing or silently stalling: + +- PR-fix attempt: `tasks/report` `outcome: "capacity-exhausted"` + issued + `prFixItem` token (including its per-hand-out `handoutToken` receipt) + + `capacity`. The release is fenced to the exact issued hand-out: only that + hand-out is removed, other agents' live hand-outs at the generation survive, + and the item stays QUEUED at the same generation and fixAttempts; the + cooldown is scoped to the exhausted capacity only (trusted caller-declared + scoping — never identity, never agent-filtered). +- Claimed issue: `POST /api/issues/capacity-handoff` with the `claimToken` + issued by the claim. Release is fenced to the exact claim and refuses human + holds and changed ownership. + +Full contract, lease/label effects, and the GitHub-vs-database consistency +boundary: [capacity-handoff.md](./capacity-handoff.md). + +## History +- **2026-10-14** — Review follow-up (#1256): per-hand-out `handoutToken` receipts bind capacity-exhausted settlement to the exact issued source attempt (unrelated agents and rotated same-agent tokens are refused); the release preserves other agents' live receipts/acks and the dispatch stamps while any hand-out is live; a failed post-CAS live-label re-read now takes the ownership-only path (never a status transition). +- **2026-10-14** — Added fenced capacity-exhaustion handoff (#1256): capacity-scoped cooldowns for PR-fix attempts and claim-token-fenced issue recovery. diff --git a/prisma/migrations/20261014000000_capacity_handoff_recovery/migration.sql b/prisma/migrations/20261014000000_capacity_handoff_recovery/migration.sql new file mode 100644 index 00000000..1d4ba8e7 --- /dev/null +++ b/prisma/migrations/20261014000000_capacity_handoff_recovery/migration.sql @@ -0,0 +1,32 @@ +-- Fenced capacity-exhaustion handoff (#1256) + +-- Claim/attempt fencing token on the issue lease. +ALTER TABLE "Lease" ADD COLUMN "claimToken" TEXT; + +CREATE UNIQUE INDEX "Lease_claimToken_key" ON "Lease"("claimToken"); + +-- Capacity-scoped bounded cooldowns for explicit capacity exhaustion. +CREATE TABLE "CapacityHandoff" ( + "id" TEXT NOT NULL, + "capacity" TEXT NOT NULL, + "kind" TEXT NOT NULL, + "repoFullName" TEXT NOT NULL, + "issueNumber" INTEGER, + "prNumber" INTEGER, + "agentName" TEXT NOT NULL, + "issueId" TEXT, + "generation" INTEGER, + "cooldownUntil" TIMESTAMP(3) NOT NULL, + "note" TEXT, + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + + CONSTRAINT "CapacityHandoff_pkey" PRIMARY KEY ("id") +); + +CREATE INDEX "CapacityHandoff_capacity_cooldownUntil_idx" ON "CapacityHandoff"("capacity", "cooldownUntil"); +CREATE INDEX "CapacityHandoff_kind_repoFullName_issueNumber_cooldownUntil_idx" ON "CapacityHandoff"("kind", "repoFullName", "issueNumber", "cooldownUntil"); +CREATE INDEX "CapacityHandoff_kind_repoFullName_prNumber_cooldownUntil_idx" ON "CapacityHandoff"("kind", "repoFullName", "prNumber", "cooldownUntil"); +CREATE INDEX "CapacityHandoff_cooldownUntil_idx" ON "CapacityHandoff"("cooldownUntil"); + +-- Per-hand-out receipts for exact source-attempt fencing (#1256 review). +ALTER TABLE "PrFixQueueItem" ADD COLUMN "handoutReceipts" TEXT[] NOT NULL DEFAULT ARRAY[]::TEXT[]; diff --git a/prisma/migrations/20261015000000_issue_mutation_guard/migration.sql b/prisma/migrations/20261015000000_issue_mutation_guard/migration.sql new file mode 100644 index 00000000..d7414f8c --- /dev/null +++ b/prisma/migrations/20261015000000_issue_mutation_guard/migration.sql @@ -0,0 +1,13 @@ +CREATE TABLE "IssueMutationGuard" ( + "issueId" TEXT NOT NULL, + "operation" TEXT NOT NULL, + "ownerToken" TEXT NOT NULL, + "agentName" TEXT, + "claimToken" TEXT, + "executing" BOOLEAN NOT NULL DEFAULT true, + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + CONSTRAINT "IssueMutationGuard_pkey" PRIMARY KEY ("issueId"), + CONSTRAINT "IssueMutationGuard_ownerToken_key" UNIQUE ("ownerToken"), + CONSTRAINT "IssueMutationGuard_issueId_fkey" FOREIGN KEY ("issueId") REFERENCES "Issue"("id") ON DELETE CASCADE ON UPDATE CASCADE +); +CREATE INDEX "IssueMutationGuard_agentName_claimToken_idx" ON "IssueMutationGuard"("agentName", "claimToken"); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 5034c516..bb38ea86 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -40,6 +40,7 @@ model Issue { laneHistory IssueLane[] agentWork AgentWork[] leases Lease[] + mutationGuard IssueMutationGuard? groomingRuns GroomingRun[] groomingApplications GroomingApplication[] groomingChildClaims GroomingChildClaim[] @@ -122,6 +123,19 @@ model Issue { @@index([groomingRetryAfter]) } +model IssueMutationGuard { + issueId String @id + issue Issue @relation(fields: [issueId], references: [id], onDelete: Cascade) + operation String + ownerToken String @unique + agentName String? + claimToken String? + executing Boolean @default(true) + createdAt DateTime @default(now()) + + @@index([agentName, claimToken]) +} + enum PrFixLane { NORMAL ESCALATED @@ -187,6 +201,19 @@ model PrFixQueueItem { // wall-clock of that hand-out. dispatchedGeneration Int? dispatchedAt DateTime? + // #1256: authoritative per-hand-out receipts. One entry per live hand-out + // issued by next-task, format "@@" (token is + // an opaque random value containing no "@"; the agent name may contain "@", + // so the token boundary is the FIRST "@" and the generation the LAST). + // Settlement of a capacity-exhausted report requires the reporting agent to + // present the exact token it was issued, and the write releases only that + // agent's receipt — other agents' live hand-outs and acknowledgements at + // the same generation survive. Invariant: an entry exists exactly while the + // matching `agentHandouts` entry exists (issuance pushes both atomically; + // every clear site clears both), so a hand-out that was released, reclaimed, + // or superseded by a generation bump can never be settled with its old + // token, and each new hand-out rotates in a fresh token. + handoutReceipts String[] @default([]) // New (previously-unrecorded) evidence arrived AFTER the current generation // was handed out (#1119): the in-flight worker cannot see it, so settlement // must reopen a fresh attempt instead of absorbing it. Records the evidence @@ -869,6 +896,11 @@ model Lease { prUrl String? // associated PR URL if one exists expiredAt DateTime // absolute expiry; default TTL is set at creation time renewedAt DateTime? + // #1256: opaque attempt/claim token issued to the worker on claim. It fences + // a capacity-handoff release to the EXACT claim: renewals keep the token, + // but a released/expired lease that is re-claimed gets a fresh row and a + // fresh token, so a stale worker's token can never release a newer claim. + claimToken String? @unique createdAt DateTime @default(now()) @@unique([agentName, issueId]) @@ -876,3 +908,41 @@ model Lease { @@index([expiredAt]) @@index([issueId]) } + +// --------------------------------------------------------------------------- +// Capacity handoffs (#1256) — bounded, capacity-SCOPED cooldowns. +// +// When a worker explicitly recognizes that its execution capacity (an opaque +// provider/model/runner identifier chosen by the worker) cannot run a handed +// attempt, it settles the attempt with a fenced handoff. Dispatch keeps the +// work item itself discoverable to other compatible workers; this table only +// records "capacity C should not be re-handed until T". Workers +// that declare no capacity conservatively skip cooled items (fail-closed). +// Expired rows are pruned lazily on read; they carry no retry budget. +// --------------------------------------------------------------------------- + +model CapacityHandoff { + id String @id @default(cuid()) + capacity String // opaque worker-declared capacity identifier + // Which subsystem owns the handed-off work: "issue" (claim/lease release) + // or "pr_fix" (PR-fix queue attempt). + kind String + repoFullName String + issueNumber Int? + prNumber Int? + // The worker that handed the work off, for audit/history. + agentName String + issueId String? // issue-form handoffs: the local Issue row + // The attempt generation the handoff settles (pr_fix rows). The replay + // check is keyed on it so a fresh attempt is never absorbed by an older + // generation's cooldown window. + generation Int? + cooldownUntil DateTime // handoff suppresses re-hand to `capacity` before this time + note String? @db.Text + createdAt DateTime @default(now()) + + @@index([capacity, cooldownUntil]) + @@index([kind, repoFullName, issueNumber, cooldownUntil]) + @@index([kind, repoFullName, prNumber, cooldownUntil]) + @@index([cooldownUntil]) +} diff --git a/src/app/api/agents/[agentName]/next-task/route.test.ts b/src/app/api/agents/[agentName]/next-task/route.test.ts index 5939c9ff..0a21cf00 100644 --- a/src/app/api/agents/[agentName]/next-task/route.test.ts +++ b/src/app/api/agents/[agentName]/next-task/route.test.ts @@ -18,6 +18,8 @@ const { mocks } = vi.hoisted(() => ({ linkedRows: [] as any[], fetchPullRequestLabels: vi.fn().mockResolvedValue([]), fetchPullRequestHeadSha: vi.fn().mockResolvedValue(null), + capacityHandoffFindMany: vi.fn().mockResolvedValue([]), + capacityHandoffDeleteMany: vi.fn().mockResolvedValue({ count: 0 }), }, })); @@ -38,6 +40,10 @@ vi.mock("@/lib/prisma", () => ({ updateMany: mocks.prFixUpdateMany, }, prFixHistory: { create: mocks.prFixHistoryCreate }, + capacityHandoff: { + findMany: mocks.capacityHandoffFindMany, + deleteMany: mocks.capacityHandoffDeleteMany, + }, $transaction: async (fn: any) => fn({ prFixQueueItem: { create: mocks.prFixCreate, @@ -426,7 +432,7 @@ describe("GET /api/agents/[agentName]/next-task", () => { const body = await res.json(); expect(body.type).toBe("followup-pr"); - expect(body.prFixItem).toEqual({ id: "prfix-1", generation: 3 }); + expect(body.prFixItem).toMatchObject({ id: "prfix-1", generation: 3 }); expect(body.prFixExclusions).toEqual({ version: 1, skipped: [] }); }); @@ -597,7 +603,7 @@ describe("GET /api/agents/[agentName]/next-task", () => { expect(body.pullRequest.repoFullName).toBe("org/repo"); expect(body.pullRequest.number).toBe(12); expect(body.pullRequest.url).toBe("https://github.com/org/repo/pull/12"); - expect(body.prFixItem).toEqual({ + expect(body.prFixItem).toMatchObject({ id: "prfix-1", generation: 1, }); @@ -632,7 +638,7 @@ describe("GET /api/agents/[agentName]/next-task", () => { { params: Promise.resolve({ agentName: "example-agent" }) }, ); const firstBody = await first.json(); - expect(firstBody.prFixItem).toEqual({ id: "prfix-1", generation: 1 }); + expect(firstBody.prFixItem).toMatchObject({ id: "prfix-1", generation: 1 }); // Same queue row after an explicit requeue — same id, bumped generation. item.generation = 2; @@ -642,7 +648,7 @@ describe("GET /api/agents/[agentName]/next-task", () => { ); const secondBody = await second.json(); expect(secondBody.type).toBe("followup-pr"); - expect(secondBody.prFixItem).toEqual({ id: "prfix-1", generation: 2 }); + expect(secondBody.prFixItem).toMatchObject({ id: "prfix-1", generation: 2 }); expect(secondBody.prFixItem.id).toBe(firstBody.prFixItem.id); }); @@ -701,7 +707,7 @@ describe("GET /api/agents/[agentName]/next-task", () => { const due = await poll(); const body = await due.json(); expect(body.type).toBe("followup-pr"); - expect(body.prFixItem).toEqual({ id: waiting.id, generation: issuedGeneration + 1 }); + expect(body.prFixItem).toMatchObject({ id: waiting.id, generation: issuedGeneration + 1 }); expect(waiting.status).toBe("QUEUED"); expect(waiting.generation).toBeGreaterThan(issuedGeneration); expect(waiting.fixAttempts).toBeUndefined(); @@ -748,7 +754,7 @@ describe("GET /api/agents/[agentName]/next-task", () => { ); const secondBody = await second.json(); expect(secondBody.type).toBe("followup-pr"); - expect(secondBody.prFixItem).toEqual({ id: itemId, generation: 2 }); + expect(secondBody.prFixItem).toMatchObject({ id: itemId, generation: 2 }); }); it("stamps the hand-out with a generation-conditional update (#1119)", async () => { @@ -848,6 +854,10 @@ describe("GET /api/agents/[agentName]/next-task", () => { expect(row.dispatchedAt).toBeInstanceOf(Date); expect(row.postDispatchEvidenceKeys).toEqual([]); expect(row.agentHandouts).toEqual(["example-agent@2"]); + // #1256 review: the hand-out pushed a per-hand-out receipt alongside the + // record, and the task carries its token back to the worker. + const firstIssued = /^(hnd_[0-9a-f]+)@example-agent@2$/.exec(row.handoutReceipts[0] ?? ""); + expect(firstIssued).not.toBeNull(); const stampedAt = row.dispatchedAt; // (b) New evidence enqueues mid-run, after the dispatch. @@ -862,7 +872,12 @@ describe("GET /api/agents/[agentName]/next-task", () => { ); const secondBody = await second.json(); expect(secondBody.type).toBe("followup-pr"); - expect(secondBody.prFixItem).toEqual({ id: "prfix-1", generation: 2 }); + expect(secondBody.prFixItem).toMatchObject({ id: "prfix-1", generation: 2 }); + expect(typeof secondBody.prFixItem.handoutToken).toBe("string"); + // The token rotates per hand-out: B's receipt is a distinct entry from A's. + expect(row.handoutReceipts).toHaveLength(2); + expect(row.handoutReceipts[1]).toBe(`${secondBody.prFixItem.handoutToken}@other-agent@2`); + expect(row.handoutReceipts[0]).not.toBe(row.handoutReceipts[1]); expect(row.postDispatchEvidenceKeys).toEqual(["review:o/r#7:r2@H1"]); expect(row.dispatchedGeneration).toBe(2); expect(row.dispatchedAt).toBe(stampedAt); @@ -962,7 +977,7 @@ describe("GET /api/agents/[agentName]/next-task", () => { const body = await res.json(); // The already-handed item is skipped; the NEXT pr-fix item ships. expect(body.type).toBe("followup-pr"); - expect(body.prFixItem).toEqual({ id: "prfix-2", generation: 1 }); + expect(body.prFixItem).toMatchObject({ id: "prfix-2", generation: 1 }); expect(handed.agentHandouts).toEqual(["example-agent@2"]); expect(fresh.agentHandouts).toEqual(["example-agent@1"]); }); @@ -1118,7 +1133,7 @@ describe("GET /api/agents/[agentName]/next-task", () => { request("/api/agents/example-agent/next-task?lane=local"), { params: Promise.resolve({ agentName: "example-agent" }) }, ); - expect((await first.json()).prFixItem).toEqual({ id: "prfix-1", generation: 2 }); + expect((await first.json()).prFixItem).toMatchObject({ id: "prfix-1", generation: 2 }); expect(row.agentHandouts).toEqual(["example-agent@2"]); // The item is re-issued as a fresh attempt (requeue/reopen): generation @@ -1133,7 +1148,7 @@ describe("GET /api/agents/[agentName]/next-task", () => { ); const secondBody = await second.json(); expect(secondBody.type).toBe("followup-pr"); - expect(secondBody.prFixItem).toEqual({ id: "prfix-1", generation: 3 }); + expect(secondBody.prFixItem).toMatchObject({ id: "prfix-1", generation: 3 }); expect(row.agentHandouts).toEqual(["example-agent@3"]); }); @@ -1182,7 +1197,7 @@ describe("GET /api/agents/[agentName]/next-task", () => { const body = await res.json(); expect(body.type).toBe("followup-pr"); - expect(body.prFixItem).toEqual({ id: "prfix-1", generation: 2 }); + expect(body.prFixItem).toMatchObject({ id: "prfix-1", generation: 2 }); expect(row.agentHandouts).toEqual(["some-other-agent@2", "example-agent@2"]); }); @@ -1301,7 +1316,7 @@ describe("GET /api/agents/[agentName]/next-task", () => { expect(stamps[1].generation).toBe(3); // The handed-out token is the NEW generation, so the worker's settle // report matches the live row instead of being rejected as a mismatch. - expect(body.prFixItem).toEqual({ id: "prfix-1", generation: 3 }); + expect(body.prFixItem).toMatchObject({ id: "prfix-1", generation: 3 }); // The payload reasons come from the latest read, not the queue snapshot. expect(body.reasons).toContain("CI failure on main (re-issued)"); expect(body.reasons).toContain("fix lint"); @@ -1490,7 +1505,7 @@ describe("GET /api/agents/[agentName]/next-task", () => { expect(body.type).toBe("followup-pr"); // Best-effort: the task is served with the pre-stamp snapshot's // generation and reasons. - expect(body.prFixItem).toEqual({ id: "prfix-1", generation: 2 }); + expect(body.prFixItem).toMatchObject({ id: "prfix-1", generation: 2 }); expect(body.reasons).toContain("CI failure on main"); expect(body.reasons).toContain("update tests"); }); @@ -1530,7 +1545,7 @@ describe("GET /api/agents/[agentName]/next-task", () => { expect(res.status).toBe(200); const body = await res.json(); expect(body.type).toBe("followup-pr"); - expect(body.prFixItem).toEqual({ id: "prfix-1", generation: 2 }); + expect(body.prFixItem).toMatchObject({ id: "prfix-1", generation: 2 }); expect(body.reasons).toContain("CI failure on main"); expect(body.reasons).toContain("update tests"); // Only present in the re-read row — proves the payload reasons come from @@ -1785,7 +1800,7 @@ describe("GET /api/agents/[agentName]/next-task", () => { const body = await res.json(); expect(body.type).toBe("followup-pr"); expect(body.shouldRun).toBe(true); - expect(body.prFixItem).toEqual({ id: "linked-prfix", generation: 1 }); + expect(body.prFixItem).toMatchObject({ id: "linked-prfix", generation: 1 }); expect(mocks.prFixCreate).toHaveBeenCalledTimes(1); expect(mocks.prFixHistoryCreate).toHaveBeenCalledTimes(1); }); @@ -1820,7 +1835,7 @@ describe("GET /api/agents/[agentName]/next-task", () => { const body = await res.json(); expect(body.type).toBe("followup-pr"); - expect(body.prFixItem).toEqual({ id: "existing-prfix", generation: 3 }); + expect(body.prFixItem).toMatchObject({ id: "existing-prfix", generation: 3 }); expect(mocks.prFixCreate).not.toHaveBeenCalled(); expect(mocks.fetchPullRequestLabels).not.toHaveBeenCalled(); }); @@ -1834,7 +1849,7 @@ describe("GET /api/agents/[agentName]/next-task", () => { ); const firstBody = await first.json(); expect(firstBody.type).toBe("followup-pr"); - expect(firstBody.prFixItem).toEqual({ id: "linked-prfix", generation: 1 }); + expect(firstBody.prFixItem).toMatchObject({ id: "linked-prfix", generation: 1 }); expect(mocks.prFixHistoryCreate).toHaveBeenCalledTimes(1); const second = await GET( @@ -1851,7 +1866,12 @@ describe("GET /api/agents/[agentName]/next-task", () => { id: "linked-prfix", generation: 1, status: "QUEUED", NOT: { agentHandouts: { has: "example-agent@1" } }, }), - data: { agentHandouts: { push: ["example-agent@1"] } }, + // #1256 review: the hand-out record and its per-hand-out receipt are + // pushed in the same conditional write (token rotates per issuance). + data: expect.objectContaining({ + agentHandouts: { push: ["example-agent@1"] }, + handoutReceipts: { push: [expect.stringMatching(/^hnd_[0-9a-f]+@example-agent@1$/)] }, + }), })); }); @@ -1990,7 +2010,7 @@ describe("GET /api/agents/[agentName]/next-task", () => { const body = await res.json(); expect(body.type).toBe("followup-pr"); expect(body.pullRequest.number).toBe(16); - expect(body.prFixItem).toEqual({ id: "linked-prfix-2", generation: 1 }); + expect(body.prFixItem).toMatchObject({ id: "linked-prfix-2", generation: 1 }); }); it("materializes linked-only work for a lane without a PR-fix equivalent, without handing it to that lane", async () => { @@ -2663,3 +2683,142 @@ describe("GET /api/agents/[agentName]/next-task — bound worker scope (#1129)", expect(res.status).toBe(403); }); }); + +describe("GET /api/agents/[agentName]/next-task — capacity-scoped handoff cooldowns (#1256)", () => { + beforeEach(() => { + delete process.env.DISPATCH_AUTH_MODE; + resetAuthCaches(); + vi.clearAllMocks(); + mocks.prFixFindMany.mockResolvedValue([]); + mocks.issueFindMany.mockResolvedValue([]); + mocks.findLeasedIssueIds.mockResolvedValue([]); + mocks.prFixUpdateMany.mockResolvedValue({ count: 1 }); + mocks.prFixFindUnique.mockResolvedValue(null); + mocks.prFixFindUniqueByRepo.mockResolvedValue(null); + mocks.capacityHandoffFindMany.mockResolvedValue([]); + mocks.capacityHandoffDeleteMany.mockResolvedValue({ count: 0 }); + }); + + function cooledPrRow(): any { + return { + id: "prfix-cooled", + repo: "org/repo", + pr: 70, + issue: null, + branch: "work/70", + url: "https://github.com/org/repo/pull/70", + lane: "NORMAL", + status: "QUEUED", + reason: "review changes requested", + feedback: [], + generation: 1, + queuedAt: new Date("2026-01-01T00:00:00Z"), + updatedAt: new Date("2026-01-01T00:00:00Z"), + dispatchedAt: null, + dispatchedGeneration: null, + postDispatchEvidenceKeys: [], + agentHandouts: [], + }; + } + + it("rejects a malformed capacity query", async () => { + const res = await GET( + request(`/api/agents/example-agent/next-task?capacity=${"x".repeat(250)}`), + { params: Promise.resolve({ agentName: "example-agent" }) }, + ); + expect(res.status).toBe(400); + }); + + it("does not re-hand a cooled PR-fix attempt to the exhausted capacity, but hands it to an unrelated capacity", async () => { + const row = cooledPrRow(); + mocks.prFixFindMany.mockResolvedValue([row]); + mocks.prFixUpdateMany.mockImplementation(async ({ where, data }: any) => { + if (row.id !== where.id) return { count: 0 }; + if (where.generation !== undefined && row.generation !== where.generation) return { count: 0 }; + applyUpdate(row, data); + return { count: 1 }; + }); + mocks.prFixFindUnique.mockImplementation(async () => ({ + reason: row.reason, feedback: row.feedback, generation: row.generation, + status: row.status, lane: row.lane, agentHandouts: row.agentHandouts, + })); + mocks.capacityHandoffFindMany.mockResolvedValue([ + { kind: "pr_fix", capacity: "model-a", repoFullName: "org/repo", issueNumber: null, prNumber: 70 }, + ]); + + // The exhausted capacity is suppressed; with no other work it goes idle. + const cooled = await GET( + request("/api/agents/example-agent/next-task?lane=local&capacity=model-a"), + { params: Promise.resolve({ agentName: "example-agent" }) }, + ); + expect((await cooled.json()).type).toBe("idle"); + + // An unrelated capacity still gets the work — the cooldown is scoped. + const unrelated = await GET( + request("/api/agents/example-agent/next-task?lane=local&capacity=model-b"), + { params: Promise.resolve({ agentName: "example-agent" }) }, + ); + const body = await unrelated.json(); + expect(body.type).toBe("followup-pr"); + expect(body.pullRequest.number).toBe(70); + expect(body.prFixItem).toMatchObject({ id: row.id, generation: 1 }); + }); + + it("suppresses a cooled issue only for the exhausted capacity; undeclared workers skip conservatively", async () => { + mocks.issueFindMany.mockResolvedValue([independentIssue(101)]); + mocks.capacityHandoffFindMany.mockResolvedValue([ + { kind: "issue", capacity: "model-a", repoFullName: "org/repo", issueNumber: 101, prNumber: null }, + ]); + + const exhausted = await GET( + request("/api/agents/example-agent/next-task?lane=local&capacity=model-a"), + { params: Promise.resolve({ agentName: "example-agent" }) }, + ); + expect((await exhausted.json()).type).toBe("idle"); + + // Fail-closed for workers that declare no capacity: Dispatch cannot prove + // they are not the exhausted capacity. + const undeclared = await GET( + request("/api/agents/example-agent/next-task?lane=local"), + { params: Promise.resolve({ agentName: "example-agent" }) }, + ); + expect((await undeclared.json()).type).toBe("idle"); + + const unrelated = await GET( + request("/api/agents/example-agent/next-task?lane=local&capacity=model-b"), + { params: Promise.resolve({ agentName: "example-agent" }) }, + ); + const body = await unrelated.json(); + expect(body.type).toBe("implement"); + expect(body.issue.number).toBe(101); + }); + + it("resumes normal dispatch once no active cooldown exists (expiry is lazy)", async () => { + mocks.issueFindMany.mockResolvedValue([independentIssue(101)]); + // The route filters by `cooldownUntil > now`; expiry simply means the row + // no longer comes back from the active read. + mocks.capacityHandoffFindMany.mockResolvedValue([]); + + const res = await GET( + request("/api/agents/example-agent/next-task?lane=local&capacity=model-a"), + { params: Promise.resolve({ agentName: "example-agent" }) }, + ); + const body = await res.json(); + expect(body.type).toBe("implement"); + expect(body.issue.number).toBe(101); + }); + + it("dispatches without capacity suppression when the cooldown read fails (availability over strictness)", async () => { + mocks.issueFindMany.mockResolvedValue([independentIssue(101)]); + mocks.capacityHandoffFindMany.mockRejectedValue(new Error("db down")); + + const res = await GET( + request("/api/agents/example-agent/next-task?lane=local&capacity=model-a"), + { params: Promise.resolve({ agentName: "example-agent" }) }, + ); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.type).toBe("implement"); + expect(body.issue.number).toBe(101); + }); +}); diff --git a/src/app/api/agents/[agentName]/next-task/route.ts b/src/app/api/agents/[agentName]/next-task/route.ts index 16fb8759..f69a4889 100644 --- a/src/app/api/agents/[agentName]/next-task/route.ts +++ b/src/app/api/agents/[agentName]/next-task/route.ts @@ -10,9 +10,10 @@ import { } from "@/lib/agent-task"; import { isBacklogLane, prFixLaneForRequest } from "@/lib/lane-config"; import { fetchAgentQueueData } from "@/lib/agent-queue-fetch"; -import { agentAlreadyHanded, agentHandoutToken, createLinkedPrFixItem, normalizeQueueRepo } from "@/lib/pr-fix-queue"; +import { agentAlreadyHanded, agentHandoutToken, createLinkedPrFixItem, handoutReceipt, newHandoutToken, normalizeQueueRepo } from "@/lib/pr-fix-queue"; import { fetchPullRequestLabels, fetchPullRequestHeadSha } from "@/lib/github"; import { NEEDS_HUMAN_LABEL } from "@/lib/pr-fix-surfacing"; +import { buildCapacitySkip, loadActiveCapacityHandoffs, normalizeCapacity } from "@/lib/capacity-handoff"; /** A queued PR-fix item as the route consumes it (see toAgentQueuePrFixItem). */ type PrFixCandidate = { @@ -148,6 +149,17 @@ export async function GET( const { searchParams } = new URL(request.url); const lane = searchParams.get("lane"); + // #1256: optional worker-declared execution capacity. When set, active + // capacity handoffs only suppress re-hand of the matching capacity's work; + // when absent, a worker conservatively skips ANY cooled item (Dispatch + // cannot prove it is not the exhausted capacity). + const capacityParam = searchParams.get("capacity"); + const requestedCapacity = capacityParam === null || capacityParam.trim() === "" + ? undefined + : normalizeCapacity(capacityParam) ?? undefined; + if (capacityParam !== null && capacityParam.trim() !== "" && requestedCapacity === undefined) { + return errorResponse("capacity must be a non-empty string of at most 200 characters", 400); + } const excludeDecomposed = searchParams.get("exclude_decomposed"); const includeClaimed = searchParams.get("includeClaimed") === "true"; const includeRenovate = searchParams.get("includeRenovate") === "true"; @@ -205,6 +217,20 @@ export async function GET( if (scopeError) return scopeError; try { + // Active capacity-scoped cooldowns (#1256). Best-effort: a read failure + // must not 500 the poll (dispatching cooled work to the exhausted capacity + // is a bounded nuisance; blocking the whole lane is not), so fail open + // with no skips and log. + let skipIssueForCapacity = (_repo: string | null | undefined, _issueNumber: number) => false; + let skipPrForCapacity = (_repo: string, _prNumber: number) => false; + try { + const handoffs = await loadActiveCapacityHandoffs(prisma as any); + const skip = buildCapacitySkip(handoffs, requestedCapacity); + skipIssueForCapacity = skip.skipIssue; + skipPrForCapacity = skip.skipPr; + } catch (error) { + console.error("next-task capacity-handoff read failed; dispatching without capacity suppression:", error); + } const { laneValid, resolvedLane, rankedQueue, fullQueue, withheldQueue, admissionMode, prFixItems, availableLanes } = await fetchAgentQueueData({ agentName, @@ -233,16 +259,31 @@ export async function GET( if (excluded(candidate.repo, candidate.pr, candidate.generation)) { return null; } + // #1256: the exhausted capacity must not be re-handed the attempt it + // gave up (and an undeclared capacity is conservatively treated the + // same); unrelated capacities dispatch it normally. + if (skipPrForCapacity(candidate.repo, candidate.pr)) return null; if (agentAlreadyHanded(candidate, agentName)) return null; const confirmed = await confirmPrFixHandOut(candidate, agentName, excluded); if (!confirmed) return null; if (excluded(candidate.repo, candidate.pr, confirmed.generation)) return null; + // Mint the per-hand-out receipt token (#1256 review): every issuance + // rotates in a fresh opaque token, pushed atomically with the hand-out + // record (one conditional write — the hand-out/receipt bijection never + // drifts). The task ships it in `prFixItem.handoutToken`; only a + // capacity-exhausted report must present it back, and settlement then + // releases exactly THIS hand-out — not a newer one for the same agent + // at the same generation, and never another agent's. + const handoutToken = newHandoutToken(); try { // Conditional append fences concurrent polls for the same agent/generation. const claimed = await prisma.prFixQueueItem.updateMany({ where: { id: candidate.id, generation: confirmed.generation, status: "QUEUED", NOT: { agentHandouts: { has: agentHandoutToken(agentName, confirmed.generation) } } }, - data: { agentHandouts: { push: [agentHandoutToken(agentName, confirmed.generation)] } }, + data: { + agentHandouts: { push: [agentHandoutToken(agentName, confirmed.generation)] }, + handoutReceipts: { push: [handoutReceipt(handoutToken, agentName, confirmed.generation)] }, + }, }); if (claimed.count !== 1) return null; } catch (error) { @@ -262,7 +303,7 @@ export async function GET( issue: candidate.issue ? { repoFullName: candidate.repo, number: candidate.issue } : undefined, - prFixItem: { id: candidate.id, generation: confirmed.generation }, + prFixItem: { id: candidate.id, generation: confirmed.generation, handoutToken }, reasons, }); }; @@ -409,7 +450,7 @@ export async function GET( // instead of re-serving the deferred one. const isDeferred = (item: { repoFullName?: string | null; number: number }) => deferredIssues.has(issueKey(item.repoFullName, item.number)); - const first = rankedQueue.find((item) => !isDeferred(item)); + const first = rankedQueue.find((item) => !isDeferred(item) && !skipIssueForCapacity(item.repoFullName, item.number)); if (first) { if (admissionMode === "audit" && first.admission && !first.admission.admitted) { console.warn( @@ -440,15 +481,25 @@ export async function GET( const excludedNote = skipped.size > 0 ? `${skipped.size} PR-fix generations skipped by client exclusion` : null; + // #1256: surface capacity suppression separately from queue ownership, + // so an operator can tell "everything left is cooled for this capacity" + // from an empty lane. + const capacityDeferredCount = + prFixItems.filter((candidate) => skipPrForCapacity(candidate.repo, candidate.pr)).length + + rankedQueue.filter((item) => !isDeferred(item) && skipIssueForCapacity(item.repoFullName, item.number)).length; + const capacityNote = capacityDeferredCount > 0 + ? `${capacityDeferredCount} item${capacityDeferredCount === 1 ? "" : "s"} cooled for ${requestedCapacity ?? "any capacity (undeclared request)"} by a capacity handoff` + : null; + if (withheldQueue.length > 0) { const withheldNote = `${withheldQueue.length} ready issue${withheldQueue.length === 1 ? "" : "s"} withheld by grooming admission`; return respond( - createIdleTask(`No work available (${[withheldNote, deferredNote, excludedNote].filter(Boolean).join("; ")})`), + createIdleTask(`No work available (${[withheldNote, deferredNote, excludedNote, capacityNote].filter(Boolean).join("; ")})`), ); } - if (deferredNote || excludedNote) { - return respond(createIdleTask(`No work available (${[deferredNote, excludedNote].filter(Boolean).join("; ")})`)); + if (deferredNote || excludedNote || capacityNote) { + return respond(createIdleTask(`No work available (${[deferredNote, excludedNote, capacityNote].filter(Boolean).join("; ")})`)); } return respond(createIdleTask("No work available")); diff --git a/src/app/api/agents/[agentName]/queue-admission.test.ts b/src/app/api/agents/[agentName]/queue-admission.test.ts index e9d8fedc..eda491a1 100644 --- a/src/app/api/agents/[agentName]/queue-admission.test.ts +++ b/src/app/api/agents/[agentName]/queue-admission.test.ts @@ -378,7 +378,9 @@ describe("admission enforce", () => { mocks.prFixFindMany.mockResolvedValue([PR_FIX]); const body = await getNextTask(); expect(body.type).toBe("followup-pr"); - expect(body.prFixItem).toEqual({ id: "prfix-1", generation: 1 }); + expect(body.prFixItem).toMatchObject({ id: "prfix-1", generation: 1 }); + expect(body.prFixItem.handoutToken).toEqual(expect.any(String)); + expect(body.prFixItem.handoutToken).not.toBe(""); const listed = await getQueue(); expect(listed[0]).toMatchObject({ pr: 77, repo: "org/repo" }); diff --git a/src/app/api/agents/[agentName]/tasks/report/route.test.ts b/src/app/api/agents/[agentName]/tasks/report/route.test.ts index eabfcd6c..350c0d9c 100644 --- a/src/app/api/agents/[agentName]/tasks/report/route.test.ts +++ b/src/app/api/agents/[agentName]/tasks/report/route.test.ts @@ -1597,6 +1597,34 @@ describe("POST /api/agents/[agentName]/tasks/report — idempotencyKey", () => { }); }); +describe("POST /api/agents/[agentName]/tasks/report — capacity exhaustion handoff", () => { + const validReport = { taskType: "followup-pr", outcome: "pr_updated", repoFullName: "org/repo", pullRequestNumber: 10 }; + + it("requires a recognized capacity identifier and issued PR-fix attempt token", async () => { + const missingCapacity = await postRequest({ ...validReport, taskType: "followup-pr", outcome: "capacity-exhausted", prFixItem: { id: "item-1", generation: 1, handoutToken: "hnd_deadbeef00" } }); + expect(missingCapacity.status).toBe(400); + const missingAttempt = await postRequest({ ...validReport, taskType: "followup-pr", outcome: "capacity-exhausted", capacity: "provider/model" }); + expect(missingAttempt.status).toBe(400); + // #1256 review: the per-hand-out receipt token is a hard gate for this + // outcome — (id, generation) alone cannot identify the exact hand-out. + const missingReceipt = await postRequest({ ...validReport, taskType: "followup-pr", outcome: "capacity-exhausted", capacity: "provider/model", prFixItem: { id: "item-1", generation: 1 } }); + expect(missingReceipt.status).toBe(400); + expect((await missingReceipt.json()).error).toMatch(/handoutToken/); + }); + + it("passes the normalized capacity and issued attempt token to the resolver", async () => { + const response = await postRequest({ + ...validReport, taskType: "followup-pr", outcome: "capacity-exhausted", + capacity: " provider/model ", prFixItem: { id: "item-1", generation: 2, handoutToken: " hnd_deadbeef00 " }, + }); + expect(response.status).toBe(200); + expect(prFixResolveMock).toHaveBeenCalledWith(expect.objectContaining({ + outcome: "capacity-exhausted", capacity: "provider/model", + attempt: { itemId: "item-1", generation: 2, handoutToken: "hnd_deadbeef00" }, + })); + }); +}); + describe("POST /api/agents/[agentName]/tasks/report — prFixItem attempt token (#1074)", () => { beforeEach(() => { delete process.env.DISPATCH_AUTH_MODE; diff --git a/src/app/api/agents/[agentName]/tasks/report/route.ts b/src/app/api/agents/[agentName]/tasks/report/route.ts index 6cc2d65d..3b85cb2e 100644 --- a/src/app/api/agents/[agentName]/tasks/report/route.ts +++ b/src/app/api/agents/[agentName]/tasks/report/route.ts @@ -33,6 +33,7 @@ const VALID_OUTCOMES = [ "already_done", "handed-off", "waiting-external", + "capacity-exhausted", ] as const; type ValidOutcome = (typeof VALID_OUTCOMES)[number]; @@ -68,7 +69,8 @@ export interface TaskReportBody { // The (id, generation) attempt token next-task issued on this followup-pr // task, echoed back by the worker (#1074). Required for PR-fix queue // settlement; part of the report's payload identity (idempotency hash). - prFixItem?: { id: string; generation: number } | null; + prFixItem?: { id: string; generation: number; handoutToken?: string } | null; + capacity?: string; } function deriveStatus(outcome: ValidOutcome): string { @@ -76,6 +78,9 @@ function deriveStatus(outcome: ValidOutcome): string { if (outcome === "failed") return "failed"; if (outcome === "blocked") return "blocked"; if (outcome === "waiting-external") return "waiting-external"; + // #1256: neutral terminal status — the worker's capacity gave the attempt + // back; nothing succeeded or failed. + if (outcome === "capacity-exhausted") return "capacity-exhausted"; return "completed"; } @@ -277,6 +282,28 @@ export async function POST( return errorResponse(`Invalid outcome. Must be one of: ${VALID_OUTCOMES.join(", ")}`, 400); } + if (outcome === "capacity-exhausted") { + if (typeof raw.capacity !== "string" || raw.capacity.trim().length === 0 || raw.capacity.trim().length > 200) { + return errorResponse("capacity must be a non-empty recognized capacity identifier of at most 200 characters", 400); + } + if (!raw.prFixItem || typeof raw.prFixItem !== "object" || Array.isArray(raw.prFixItem)) { + return errorResponse("capacity-exhausted requires the issued prFixItem { id, generation } token", 400); + } + // #1256 review: the release is fenced to the EXACT issued hand-out, so + // the per-hand-out receipt token is a hard gate for this outcome (other + // outcomes never required it and are unchanged). + const rawCapacityAttempt = raw.prFixItem as { handoutToken?: unknown }; + if (typeof rawCapacityAttempt.handoutToken !== "string" || rawCapacityAttempt.handoutToken.trim().length === 0) { + return errorResponse("capacity-exhausted requires the issued prFixItem.handoutToken receipt token", 400); + } + // The outcome's entire effect is the queue settlement; a report without + // the PR coordinates would record an AgentRun and silently settle nothing. + // Claimed-issue exhaustion uses POST /api/issues/capacity-handoff instead. + if (typeof raw.repoFullName !== "string" || raw.repoFullName.trim() === "" || typeof raw.pullRequestNumber !== "number") { + return errorResponse("capacity-exhausted requires repoFullName and pullRequestNumber (issue work uses POST /api/issues/capacity-handoff)", 400); + } + } + if (raw.issueNumber !== undefined && (typeof raw.issueNumber !== "number" || !Number.isInteger(raw.issueNumber))) { return errorResponse("issueNumber must be an integer", 400); } @@ -285,7 +312,7 @@ export async function POST( return errorResponse("pullRequestNumber must be an integer", 400); } - const stringFields: readonly string[] = ["repoFullName", "pullRequestUrl", "summary", "error", "evidence"]; + const stringFields: readonly string[] = ["repoFullName", "pullRequestUrl", "summary", "error", "evidence", "capacity"]; for (const field of stringFields) { if (raw[field] !== undefined && typeof raw[field] !== "string") { return errorResponse(`${field} must be a string`, 400); @@ -424,6 +451,14 @@ export async function POST( if (typeof prFixItem.generation !== "number" || !Number.isInteger(prFixItem.generation) || prFixItem.generation < 1) { return errorResponse("prFixItem.generation must be an integer >= 1", 400); } + // #1256: optional per-hand-out receipt token. Optional here so legacy + // (pre-receipt) reports keep settling every other outcome; the resolver + // enforces its presence for `capacity-exhausted`. + if (prFixItem.handoutToken !== undefined && prFixItem.handoutToken !== null) { + if (typeof prFixItem.handoutToken !== "string" || prFixItem.handoutToken.trim().length === 0 || prFixItem.handoutToken.trim().length > 100) { + return errorResponse("prFixItem.handoutToken must be a non-empty string of at most 100 characters", 400); + } + } } // Optional worker-chosen opaque key. Present → the report becomes @@ -469,6 +504,10 @@ export async function POST( ? { id: (raw.prFixItem as { id: string }).id.trim(), generation: (raw.prFixItem as { generation: number }).generation, + handoutToken: + typeof (raw.prFixItem as { handoutToken?: unknown }).handoutToken === "string" + ? (raw.prFixItem as { handoutToken: string }).handoutToken.trim() + : undefined, } : undefined; @@ -494,6 +533,7 @@ export async function POST( // #1120: normalized worker start time; undefined → report time (now). startedAt: resolveReportedStartedAt(raw.startedAt, now), prFixItem, + capacity: typeof raw.capacity === "string" ? raw.capacity.trim() : undefined, }; try { @@ -599,9 +639,13 @@ export async function POST( // the resolver ignores both for any other outcome. blockReason: report.blockReason, decision: report.decision, + capacity: report.capacity, + agentName, // #1074: settlement is authorized by the attempt token the worker was // issued; a report without one never mutates the queue. - attempt: prFixItem ? { itemId: prFixItem.id, generation: prFixItem.generation } : null, + attempt: prFixItem + ? { itemId: prFixItem.id, generation: prFixItem.generation, handoutToken: prFixItem.handoutToken } + : null, }); if (typeof idempotencyKey === "string") { // Persist the resolution before responding so a retry replays the real diff --git a/src/app/api/issues/capacity-handoff/route.test.ts b/src/app/api/issues/capacity-handoff/route.test.ts new file mode 100644 index 00000000..e24db157 --- /dev/null +++ b/src/app/api/issues/capacity-handoff/route.test.ts @@ -0,0 +1,461 @@ +vi.mock("@/lib/issue-mutation-guard", () => ({ + withIssueMutationGuard: async (_request: unknown, callback: any) => { + const result = await callback(undefined, { resuming: false }); + return { status: result.complete ? "completed" : "retained", value: result.value }; + }, +})); +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + authedRequest, + makeDispatchEnvMock, + TEST_AGENT_TOKEN as mockToken, +} from "@/test/route-helpers"; + +process.env.DISPATCH_AGENT_TOKEN = mockToken; + +// dispatch-env: maintainer token (full rights), a worker token bound to +// "alpha", and an unbound legacy worker token (#1129 refusals). +vi.mock("@/lib/dispatch-env", () => + makeDispatchEnvMock(mockToken, { "unbound-worker-token": "worker" }, { "bound-alpha-token": "alpha" }), +); + +const { mocks, db } = vi.hoisted(() => { + const mocks = { + getLiveIssueLabels: vi.fn<(repo: string, num: number) => Promise>(async () => []), + addIssueLabel: vi.fn(async () => {}), + removeIssueLabel: vi.fn(async () => {}), + fetchPullRequestState: vi.fn(async () => ({ state: null as string | null, mergedAt: null as string | null })), + }; + // Mutable in-memory database, reset per test via makeState(). + const db: Record = { + issue: null, + lease: null, + agentWork: [] as any[], + agentWorkUpdates: [] as any[], + agentWorkHistory: [] as any[], + handoffs: [] as any[], + audit: [] as any[], + leaseDeletes: 0, + }; + return { mocks, db }; +}); + +vi.mock("@/lib/prisma", () => ({ + prisma: { + issue: { + findUnique: async ({ where }: any) => + db.issue && db.issue.id === where.id ? db.issue : null, + update: async ({ where, data }: any) => { + if (db.issue && db.issue.id === where.id) db.issue = { ...db.issue, ...data }; + return db.issue; + }, + }, + lease: { + findUnique: async ({ where }: any) => { + const key = where.agentName_issueId; + return db.lease && db.lease.agentName === key.agentName && db.lease.issueId === key.issueId + ? db.lease + : null; + }, + deleteMany: async ({ where }: any) => { + if ( + db.lease && + db.lease.id === where.id && + db.lease.agentName === where.agentName && + db.lease.issueId === where.issueId && + db.lease.claimToken === where.claimToken + ) { + db.lease = null; + db.leaseDeletes += 1; + return { count: 1 }; + } + return { count: 0 }; + }, + }, + agentWork: { + findMany: async () => db.agentWork, + update: async ({ where, data }: any) => { + db.agentWorkUpdates.push({ where, data }); + return null; + }, + }, + agentWorkHistory: { + create: async ({ data }: any) => { + db.agentWorkHistory.push(data); + return data; + }, + }, + capacityHandoff: { + create: async ({ data }: any) => { + if (db.failHandoffCreate) throw new Error("cooldown write failed"); + db.handoffs.push(data); + return data; + }, + findMany: async () => db.handoffs, + deleteMany: async () => ({ count: 0 }), + }, + auditLog: { + create: async ({ data }: any) => { + db.audit.push(data); + return data; + }, + }, + }, +})); + +vi.mock("@/lib/claim-gate", () => ({ + getLiveIssueLabels: mocks.getLiveIssueLabels, +})); + +vi.mock("@/lib/github", () => ({ + addIssueLabel: mocks.addIssueLabel, + removeIssueLabel: mocks.removeIssueLabel, +})); + +vi.mock("@/lib/github-prs", () => ({ + fetchPullRequestState: mocks.fetchPullRequestState, +})); + +import { POST } from "./route"; +import { resetAuthCaches } from "@/lib/auth"; + +const CLAIM_TOKEN = "claim_test-token-1"; + +function makeState(overrides: Record = {}) { + db.issue = { + id: "issue-1", + number: 42, + state: "open", + blockedReason: null, + linkedPrNumber: null, + labels: ["agent/alpha", "status/in-progress"], + repository: { fullName: "org/repo" }, + ...overrides, + }; + db.lease = { + id: "lease-1", + agentName: "alpha", + issueId: "issue-1", + claimToken: CLAIM_TOKEN, + expiredAt: new Date(Date.now() + 10 * 60 * 1000), + ...(overrides.lease as object | undefined), + }; + if (overrides.lease === null) db.lease = null; + db.agentWork = [{ id: "work-1" }]; + db.agentWorkUpdates = []; + db.agentWorkHistory = []; + db.handoffs = []; + db.audit = []; + db.leaseDeletes = 0; + db.failHandoffCreate = false; + mocks.getLiveIssueLabels.mockResolvedValue( + (overrides.liveLabels as string[] | undefined) ?? ["agent/alpha", "status/in-progress"], + ); + mocks.addIssueLabel.mockResolvedValue(undefined); + mocks.removeIssueLabel.mockResolvedValue(undefined); +} + +function postRequest(body: Record = {}, token?: string) { + return POST( + authedRequest("http://localhost/api/issues/capacity-handoff", { + method: "POST", + token, + body: { + issueId: "issue-1", + repoFullName: "org/repo", + issueNumber: 42, + agentName: "alpha", + claimToken: CLAIM_TOKEN, + capacity: "provider/model-a", + ...body, + }, + }), + ); +} + +describe("POST /api/issues/capacity-handoff (#1256)", () => { + beforeEach(() => { + vi.clearAllMocks(); + resetAuthCaches(); + delete process.env.DISPATCH_AUTH_MODE; + delete process.env.DISPATCH_CAPACITY_COOLDOWN_MS; + makeState(); + }); + + afterEach(() => { + delete process.env.DISPATCH_CAPACITY_COOLDOWN_MS; + }); + + // ── auth ────────────────────────────────────────────────────────────────── + + it("requires bearer authentication", async () => { + const response = await POST( + new Request("http://localhost/api/issues/capacity-handoff", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({}), + }), + ); + expect(response.status).toBe(401); + }); + + it("refuses a bound worker token handing off ANOTHER agent's claim", async () => { + const response = await postRequest({ agentName: "bravo" }, "bound-alpha-token"); + expect(response.status).toBe(403); + expect(db.leaseDeletes).toBe(0); + }); + + it("refuses the unbound legacy worker token (#1129)", async () => { + const response = await postRequest({}, "unbound-worker-token"); + expect(response.status).toBe(403); + expect(db.leaseDeletes).toBe(0); + }); + + // ── validation ───────────────────────────────────────────────────────────── + + it("rejects a missing or blank capacity (explicit recognition required)", async () => { + expect((await postRequest({ capacity: undefined })).status).toBe(400); + expect((await postRequest({ capacity: " " })).status).toBe(400); + expect((await postRequest({ capacity: "x".repeat(201) })).status).toBe(400); + expect(db.leaseDeletes).toBe(0); + }); + + it("rejects a missing or oversized claim token", async () => { + expect((await postRequest({ claimToken: undefined })).status).toBe(400); + expect((await postRequest({ claimToken: "" })).status).toBe(400); + expect((await postRequest({ claimToken: "t".repeat(201) })).status).toBe(400); + }); + + // ── happy path ──────────────────────────────────────────────────────────── + + it("releases the exact claim, returns the issue to ready, and records a scoped cooldown", async () => { + const response = await postRequest(); + expect(response.status).toBe(200); + const json = await response.json(); + expect(json.success).toBe(true); + expect(json.labels).not.toContain("agent/alpha"); + expect(json.labels).toContain("status/ready"); + expect(json.status).toBe("status/ready"); + expect(new Date(json.cooldownUntil).getTime()).toBeGreaterThan(Date.now()); + + // Ownership token consumed, claim label removed, in-progress -> ready. + expect(db.lease).toBeNull(); + expect(mocks.removeIssueLabel).toHaveBeenCalledWith("org/repo", 42, "agent/alpha"); + expect(mocks.removeIssueLabel).toHaveBeenCalledWith("org/repo", 42, "status/in-progress"); + expect(mocks.addIssueLabel).toHaveBeenCalledWith("org/repo", 42, "status/ready"); + expect(db.issue.labels).toContain("status/ready"); + expect(db.issue.labels).not.toContain("agent/alpha"); + + // Capacity-scoped cooldown row. + expect(db.handoffs).toHaveLength(1); + expect(db.handoffs[0]).toMatchObject({ + kind: "issue", + capacity: "provider/model-a", + repoFullName: "org/repo", + issueNumber: 42, + agentName: "alpha", + issueId: "issue-1", + }); + + // AgentWork retired with history. + expect(db.agentWorkUpdates[0]).toMatchObject({ + where: { id: "work-1" }, + data: { state: "RELEASED" }, + }); + expect(db.agentWorkHistory[0]).toMatchObject({ + workId: "work-1", + action: "capacity_handoff", + state: "RELEASED", + }); + + // Audit trail. + expect(db.audit).toHaveLength(1); + expect(db.audit[0]).toMatchObject({ + action: "issue_capacity_handoff", + success: true, + issueNumber: 42, + }); + expect(db.audit[0].notes).toContain("capacity=provider/model-a"); + }); + + it("honors DISPATCH_CAPACITY_COOLDOWN_MS for the bounded cooldown", async () => { + process.env.DISPATCH_CAPACITY_COOLDOWN_MS = "120000"; + const response = await postRequest(); + const json = await response.json(); + const delta = new Date(json.cooldownUntil).getTime() - Date.now(); + expect(delta).toBeGreaterThan(100_000); + expect(delta).toBeLessThan(130_000); + }); + + // ── fencing: stale owners, races, changed ownership ─────────────────────── + + it("refuses a stale claim token after the claim was re-issued (exact-claim fence)", async () => { + // The lease was released and re-claimed: new row, new token. + db.lease = { ...db.lease, id: "lease-2", claimToken: "claim_newer-token" }; + const response = await postRequest(); // old CLAIM_TOKEN + expect(response.status).toBe(409); + expect((await response.json()).error).toMatch(/does not match the current claim/i); + // The NEW claim is untouched. + expect(db.lease).not.toBeNull(); + expect(db.lease.claimToken).toBe("claim_newer-token"); + expect(mocks.removeIssueLabel).not.toHaveBeenCalled(); + expect(db.handoffs).toHaveLength(0); + }); + + it("refuses an expired claim (stale recovery owns release)", async () => { + db.lease = { ...db.lease, expiredAt: new Date(Date.now() - 1000) }; + const response = await postRequest(); + expect(response.status).toBe(409); + expect((await response.json()).error).toMatch(/expired/i); + expect(mocks.removeIssueLabel).not.toHaveBeenCalled(); + }); + + it("refuses when no claim exists", async () => { + db.lease = null; + const response = await postRequest(); + expect(response.status).toBe(409); + expect((await response.json()).error).toMatch(/no active tokened claim/i); + }); + + it("refuses when GitHub shows a DIFFERENT agent owns the claim (concurrent new owner)", async () => { + mocks.getLiveIssueLabels.mockResolvedValue(["agent/bravo", "status/in-progress"]); + const response = await postRequest(); + expect(response.status).toBe(409); + expect((await response.json()).error).toMatch(/belongs to agent\/bravo/i); + // No writes at all — bravo's claim is untouched. + expect(db.lease).not.toBeNull(); + expect(mocks.removeIssueLabel).not.toHaveBeenCalled(); + expect(mocks.addIssueLabel).not.toHaveBeenCalled(); + expect(db.handoffs).toHaveLength(0); + }); + + it("refuses when the agent label is gone from GitHub (ownership changed / already released)", async () => { + mocks.getLiveIssueLabels.mockResolvedValue(["status/ready"]); + const response = await postRequest(); + expect(response.status).toBe(409); + expect((await response.json()).error).toMatch(/no agent claim/i); + expect(db.lease).not.toBeNull(); + }); + + it("relies on the issue guard for cooperating writers and retains the lease if a bypassed CAS fails", async () => { + // A cooperating writer cannot enter while this operation owns the durable + // guard. Simulate an out-of-band/bypassing writer winning the lease CAS: + // remote cleanup may already have happened, but no cooldown is recorded. + const { prisma } = await import("@/lib/prisma"); + const leaseModel = (prisma as any).lease; + const original = leaseModel.deleteMany; + leaseModel.deleteMany = async () => ({ count: 0 }); + try { + const response = await postRequest(); + expect(response.status).toBe(409); + expect((await response.json()).error).toMatch(/changed concurrently/i); + expect(mocks.removeIssueLabel).toHaveBeenCalled(); + expect(db.handoffs).toHaveLength(0); + expect(db.lease).not.toBeNull(); + } finally { + leaseModel.deleteMany = original; + } + }); + + // ── human holds ──────────────────────────────────────────────────────────── + + it("refuses a deliberate block (human hold with blockedReason)", async () => { + db.issue = { ...db.issue, labels: ["agent/alpha", "status/blocked"], blockedReason: "waiting on vendor API" }; + mocks.getLiveIssueLabels.mockResolvedValue(["agent/alpha", "status/blocked"]); + const response = await postRequest(); + expect(response.status).toBe(409); + expect((await response.json()).error).toMatch(/human hold/i); + expect(db.lease).not.toBeNull(); + expect(mocks.removeIssueLabel).not.toHaveBeenCalled(); + }); + + it("refuses a LIVE status/blocked even when the cache has not caught up (fail-closed hold)", async () => { + // Cache still says in-progress with no blockedReason; GitHub was blocked + // out-of-band. The live set wins — the block is never reversed. + mocks.getLiveIssueLabels.mockResolvedValue(["agent/alpha", "status/blocked"]); + const response = await postRequest(); + expect(response.status).toBe(409); + expect((await response.json()).error).toMatch(/status\/blocked/i); + expect(db.lease).not.toBeNull(); + expect(db.leaseDeletes).toBe(0); + expect(mocks.removeIssueLabel).not.toHaveBeenCalled(); + }); + + it("narrows to an ownership-only release when ownership changed after the CAS", async () => { + // First (pre-CAS) read shows our claim; the post-CAS re-read shows a + // force-claim moved the issue to bravo. Only OUR label may be removed — + // bravo's status transition must survive. + mocks.getLiveIssueLabels + .mockResolvedValueOnce(["agent/alpha", "status/in-progress"]) + .mockResolvedValueOnce(["agent/bravo", "status/in-progress"]); + const response = await postRequest(); + expect(response.status).toBe(200); + const json = await response.json(); + // Ownership-only: our agent label removed, no status churn. + expect(mocks.removeIssueLabel).toHaveBeenCalledWith("org/repo", 42, "agent/alpha"); + expect(mocks.removeIssueLabel).not.toHaveBeenCalledWith("org/repo", 42, "status/in-progress"); + expect(mocks.addIssueLabel).not.toHaveBeenCalledWith("org/repo", 42, "status/ready"); + expect(json.labels).toContain("status/in-progress"); + expect(json.labels).not.toContain("agent/alpha"); + expect(db.audit[0].notes).toMatch(/ownership changed before cleanup/i); + }); + + it("still succeeds when the cooldown write fails after the release committed", async () => { + db.failHandoffCreate = true; + const response = await postRequest(); + expect(response.status).toBe(200); + const json = await response.json(); + expect(json.success).toBe(true); + expect(json.cooldownUntil).toBeNull(); + expect(db.lease).toBeNull(); + expect(mocks.removeIssueLabel).toHaveBeenCalledWith("org/repo", 42, "agent/alpha"); + expect(db.audit[0].notes).toMatch(/none \(cooldown write failed\)/i); + }); + + it("refuses a done issue (live status/done wins over the cache)", async () => { + mocks.getLiveIssueLabels.mockResolvedValue(["agent/alpha", "status/done"]); + const response = await postRequest(); + expect(response.status).toBe(409); + expect((await response.json()).error).toMatch(/done/i); + expect(db.lease).not.toBeNull(); + }); + + it("refuses a closed issue before touching the lease", async () => { + db.issue = { ...db.issue, state: "closed" }; + const response = await postRequest(); + expect(response.status).toBe(400); + expect(db.leaseDeletes).toBe(0); + expect(mocks.getLiveIssueLabels).not.toHaveBeenCalled(); + }); + + // ── fail-closed GitHub behavior ─────────────────────────────────────────── + + it("fails closed when live GitHub labels cannot be verified", async () => { + mocks.getLiveIssueLabels.mockRejectedValue(new Error("GitHub 503")); + const response = await postRequest(); + expect(response.status).toBe(503); + expect((await response.json()).error).toMatch(/could not verify live github labels/i); + expect(db.lease).not.toBeNull(); + expect(db.handoffs).toHaveLength(0); + }); + + it("rejects a cross-referenced issue identity before any write (#1129)", async () => { + const response = await postRequest({ issueNumber: 43 }); + expect(response.status).toBe(400); + expect(db.leaseDeletes).toBe(0); + }); + + it("retains the lease and guard after remote label cleanup fails", async () => { + mocks.removeIssueLabel.mockRejectedValueOnce(new Error("GitHub write failed")); + const response = await postRequest(); + expect(response.status).toBe(500); + // Remote cleanup is incomplete: retain ownership state and permit only an + // exact-token retry to resume the durable guard. + expect(db.lease).not.toBeNull(); + expect(db.leaseDeletes).toBe(0); + expect(db.audit).toHaveLength(1); + expect(db.audit[0]).toMatchObject({ action: "issue_capacity_handoff", success: false }); + expect(db.audit[0].errorMessage).toMatch(/lease retained/i); + expect(db.handoffs).toHaveLength(0); + }); +}); diff --git a/src/app/api/issues/capacity-handoff/route.ts b/src/app/api/issues/capacity-handoff/route.ts new file mode 100644 index 00000000..6862ddf0 --- /dev/null +++ b/src/app/api/issues/capacity-handoff/route.ts @@ -0,0 +1,114 @@ +// POST /api/issues/capacity-handoff (#1256) +// +// Worker-facing fenced recovery for explicit capacity exhaustion on a claimed +// issue. The caller presents the claim token Dispatch issued at claim time; +// the release is fenced to that exact claim and refuses human holds and +// changed ownership (see src/lib/issue-capacity-handoff.ts). On success the +// issue returns to circulation for other compatible workers and the exhausted +// capacity gets a bounded, capacity-SCOPED cooldown — unrelated capacities are +// never suppressed. +// +// Auth follows the claim/unclaim contract: bearer-authenticated; a bound +// worker credential may only hand off its own agent's claim; the unbound +// legacy worker token is refused (#1129). +import { NextResponse } from "next/server"; +import { errorResponse, handleApiError } from "@/lib/api-errors"; +import { prisma } from "@/lib/prisma"; +import { authorizeRequest, authErrorResponse, getAuthorizedActor } from "@/lib/auth"; +import { enforceWorkerAgentScope } from "@/lib/worker-identity"; +import { recoverIssueCapacityHandoff } from "@/lib/issue-capacity-handoff"; +import { normalizeCapacity } from "@/lib/capacity-handoff"; +import { enforceRateLimit } from "@/lib/rate-limit"; + +const RATE_LIMIT = { limit: 30, windowMs: 10_000 }; +const MAX_NOTE_LENGTH = 500; +const MAX_TOKEN_LENGTH = 200; + +export async function POST(request: Request) { + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); + } + + const limited = enforceRateLimit(`capacity-handoff:${auth.actor}`, RATE_LIMIT); + if (limited) return limited; + + try { + let body: unknown; + try { + body = await request.json(); + } catch { + return errorResponse("Invalid JSON body", 400); + } + if (typeof body !== "object" || body === null) { + return errorResponse("Invalid JSON body", 400); + } + + const { + issueId, + repoFullName, + issueNumber, + agentName, + claimToken, + capacity, + note, + } = body as Record; + + if ( + typeof issueId !== "string" || issueId.trim() === "" || + typeof repoFullName !== "string" || repoFullName.trim() === "" || + typeof issueNumber !== "number" || !Number.isInteger(issueNumber) || + typeof agentName !== "string" || agentName.trim() === "" + ) { + return errorResponse("Missing required fields: issueId, repoFullName, issueNumber, agentName", 400); + } + if (typeof claimToken !== "string" || claimToken.trim() === "" || claimToken.length > MAX_TOKEN_LENGTH) { + return errorResponse(`claimToken must be a non-empty string of at most ${MAX_TOKEN_LENGTH} characters (issued with the claim)`, 400); + } + // Explicit recognition of capacity exhaustion: a non-empty, bounded, + // opaque capacity identifier. Silently exhausting nothing is not allowed. + const normalizedCapacity = normalizeCapacity(capacity); + if (normalizedCapacity === null) { + return errorResponse("capacity must be a non-empty recognized capacity identifier of at most 200 characters", 400); + } + if (note !== undefined && (typeof note !== "string" || note.length > MAX_NOTE_LENGTH)) { + return errorResponse(`note must be a string of at most ${MAX_NOTE_LENGTH} characters`, 400); + } + + // A bound worker credential may only hand off its own agent's claim; an + // unbound legacy worker token is refused here (#1129). + const scopeError = await enforceWorkerAgentScope(auth, agentName); + if (scopeError) return scopeError; + + const actor = getAuthorizedActor(auth, request, agentName); + + const result = await recoverIssueCapacityHandoff({ + client: prisma as any, + issueId: issueId.trim(), + repoFullName: repoFullName.trim(), + issueNumber, + agentName: agentName.trim(), + claimToken: claimToken.trim(), + capacity: normalizedCapacity, + note: typeof note === "string" && note.trim() !== "" ? note.trim() : null, + actor, + }); + + if (!result.ok) { + return errorResponse(result.message, result.httpStatus); + } + + return NextResponse.json({ + success: true, + labels: result.labels, + status: result.status, + statusNote: result.statusNote, + capacity: normalizedCapacity, + // null only when the (post-release, best-effort) cooldown write failed; + // the ownership release itself succeeded. + cooldownUntil: result.cooldownUntil ? result.cooldownUntil.toISOString() : null, + }); + } catch (error) { + return handleApiError("capacity handoff", error); + } +} diff --git a/src/app/api/issues/claim/route.test.ts b/src/app/api/issues/claim/route.test.ts index e51c2231..8c973e5c 100644 --- a/src/app/api/issues/claim/route.test.ts +++ b/src/app/api/issues/claim/route.test.ts @@ -1,3 +1,10 @@ +const { guardState } = vi.hoisted(() => ({ guardState: { resuming: false } })); +vi.mock("@/lib/issue-mutation-guard", () => ({ + withIssueMutationGuard: async (_request: unknown, callback: any) => { + const result = await callback(undefined, { resuming: guardState.resuming }); + return { status: result.complete ? "completed" : "retained", value: result.value }; + }, +})); import { describe, expect, it, vi, beforeEach } from "vitest"; import { TEST_AGENT_TOKEN as mockToken, authedRequest } from "@/test/route-helpers"; @@ -67,6 +74,7 @@ vi.mock("@/lib/prisma", () => ({ })); vi.mock("@/lib/github", () => ({ addIssueLabel: mocks.addIssueLabel, removeIssueLabel: mocks.removeIssueLabel })); +vi.mock("@/lib/issue-status", () => ({ transitionIssueStatus: vi.fn(async (_repo: string, _number: number, labels: string[], status: string) => { for (const label of labels.filter((value) => value.startsWith("status/") && value !== status)) await mocks.removeIssueLabel(_repo, _number, label); await mocks.addIssueLabel(_repo, _number, status); }) })); vi.mock("@/lib/claim-gate", () => ({ getLiveIssueLabels: mocks.getLiveIssueLabels })); @@ -77,6 +85,7 @@ import { POST } from "./route"; // shared TEST_AGENT_TOKEN buckets cannot bleed across describes (same pattern // as move/webhook suites). beforeEach(() => { + guardState.resuming = false; resetRateLimits(); }); @@ -150,8 +159,15 @@ describe("POST /api/issues/claim — business logic", () => { mocks.leaseFindMany.mockResolvedValue([]); mocks.leaseDeleteMany.mockResolvedValue({ count: 0 }); mocks.removeIssueLabel.mockResolvedValue(undefined); - mocks.leaseFindMany.mockResolvedValue([]); - mocks.leaseDeleteMany.mockResolvedValue({ count: 0 }); + mocks.transaction.mockImplementation((arg: any) => { + if (typeof arg === "function") return Promise.resolve(arg({ + issue: { update: mocks.updateIssue }, + issueLifecycleEvent: { create: mocks.createLifecycleEvent, upsert: mocks.upsertLifecycleEvent }, + agentWork: { update: mocks.agentWorkUpdate }, + agentWorkHistory: { create: vi.fn().mockResolvedValue({}) }, + })); + return Promise.all(arg.map((op: any) => op)); + }); }); it("adds agent label and moves a fresh claim to in-progress", async () => { @@ -165,6 +181,19 @@ describe("POST /api/issues/claim — business logic", () => { expect(mocks.createAuditLog).toHaveBeenCalledWith({ data: expect.objectContaining({ action: "claim_issue", success: true, actor: "test-agent" }) }); }); + it("echoes the issued claim token that fences a later capacity handoff (#1256)", async () => { + // Mirror the real client: create persists what the code minted. + mocks.leaseCreate.mockImplementation(async ({ data }: any) => ({ id: "l-1", ...data })); + const res = await POST(makeRequest()); + expect(res.status).toBe(200); + const body = await res.json(); + expect(typeof body.claimToken).toBe("string"); + expect(body.claimToken).toMatch(/^claim_/); + // The minted token is persisted on the lease row, not just returned. + const created = (mocks.leaseCreate.mock.calls[0] as any)[0].data; + expect(created.claimToken).toBe(body.claimToken); + }); + it("refuses closed issues", async () => { mocks.findUnique.mockResolvedValue({ id: "issue-1", state: "closed", number: 42, repository: { fullName: "org/repo" }, labels: [] as string[] }); const res = await POST(makeRequest()); @@ -172,12 +201,52 @@ describe("POST /api/issues/claim — business logic", () => { expect((await res.json()).error).toBe("Cannot claim a closed issue"); }); - it("refuses done issues", async () => { - mocks.findUnique.mockResolvedValue({ id: "issue-1", state: "open", number: 42, repository: { fullName: "org/repo" }, labels: ["status/done"] }); - mocks.getLiveIssueLabels.mockResolvedValueOnce(["status/done"]); + it.each(["status/blocked", "status/done"])("refuses %s while resuming without remote or lease writes", async (status) => { + guardState.resuming = true; + mocks.findUnique.mockResolvedValue({ id: "issue-1", state: "open", number: 42, repository: { fullName: "org/repo" }, labels: [status] }); + mocks.getLiveIssueLabels.mockResolvedValueOnce([status]); const res = await POST(makeRequest()); expect(res.status).toBe(400); - expect((await res.json()).error).toBe("Cannot claim a done issue"); + expect(mocks.addIssueLabel).not.toHaveBeenCalled(); + expect(mocks.removeIssueLabel).not.toHaveBeenCalled(); + expect(mocks.leaseCreate).not.toHaveBeenCalled(); + expect(mocks.leaseUpdate).not.toHaveBeenCalled(); + }); + + it("repairs a failed claim transaction on exact-agent retry without repeating remote labels", async () => { + guardState.resuming = false; + mocks.getLiveIssueLabels.mockResolvedValueOnce(["status/ready"]).mockResolvedValue(["agent/test-agent", "status/in-progress"]); + mocks.transaction.mockRejectedValueOnce(new Error("db transaction failed")); + const first = await POST(makeRequest()); + expect(first.status).toBe(500); + guardState.resuming = true; + const retry = await POST(makeRequest()); + expect(retry.status).toBe(200); + expect(mocks.addIssueLabel).toHaveBeenCalledTimes(2); + expect(mocks.removeIssueLabel).toHaveBeenCalledTimes(1); + expect(mocks.createLifecycleEvent).toHaveBeenCalledTimes(1); + expect(mocks.leaseCreate).toHaveBeenCalledTimes(1); + }); + + it("refuses a resumed claim after live ownership changes without writes", async () => { + mocks.getLiveIssueLabels.mockResolvedValueOnce(["status/ready"]); + mocks.transaction.mockRejectedValueOnce(new Error("db transaction failed")); + const first = await POST(makeRequest()); + expect(first.status).toBe(500); + expect(mocks.addIssueLabel).toHaveBeenCalledTimes(2); + const remoteAddCount = mocks.addIssueLabel.mock.calls.length; + const remoteRemoveCount = mocks.removeIssueLabel.mock.calls.length; + const issueUpdateCount = mocks.updateIssue.mock.calls.length; + const leaseCreateCount = mocks.leaseCreate.mock.calls.length; + + guardState.resuming = true; + mocks.getLiveIssueLabels.mockResolvedValueOnce(["agent/other-agent", "status/in-progress"]); + const res = await POST(makeRequest()); + expect(res.status).toBe(409); + expect(mocks.addIssueLabel).toHaveBeenCalledTimes(remoteAddCount); + expect(mocks.removeIssueLabel).toHaveBeenCalledTimes(remoteRemoveCount); + expect(mocks.updateIssue).toHaveBeenCalledTimes(issueUpdateCount); + expect(mocks.leaseCreate).toHaveBeenCalledTimes(leaseCreateCount); }); // The 20-day stall: an issue left at status/ready while still carrying its own @@ -486,6 +555,8 @@ describe("POST /api/issues/claim — #1037 live label gate", () => { describe("POST /api/issues/claim — worker tier (#1111)", () => { beforeEach(() => { vi.clearAllMocks(); + guardState.resuming = false; + mocks.getLiveIssueLabels.mockResolvedValue([]); mocks.findUnique.mockResolvedValue({ id: "issue-1", state: "open", number: 42, repository: { fullName: "org/repo" }, labels: [] as string[] }); mocks.updateIssue.mockResolvedValue(undefined); mocks.createAuditLog.mockResolvedValue({ id: "log-1" }); @@ -641,6 +712,7 @@ describe("POST /api/issues/claim — cross-issue identity invariant (#1129 revie describe("POST /api/issues/claim — lifecycle evidence (#1202)", () => { beforeEach(() => { vi.clearAllMocks(); + guardState.resuming = false; mocks.findUnique.mockResolvedValue({ id: "issue-1", state: "open", number: 42, repository: { fullName: "org/repo" }, labels: [] as string[] }); mocks.updateIssue.mockResolvedValue(undefined); mocks.createAuditLog.mockResolvedValue({ id: "log-1" }); @@ -706,6 +778,7 @@ describe("POST /api/issues/claim — lifecycle evidence (#1202)", () => { expect(first.status).toBe(500); expect(mocks.createLifecycleEvent).toHaveBeenCalledTimes(1); + guardState.resuming = true; const retry = await POST(makeRequest()); expect(retry.status).toBe(200); diff --git a/src/app/api/issues/claim/route.ts b/src/app/api/issues/claim/route.ts index 7ab78391..b8365809 100644 --- a/src/app/api/issues/claim/route.ts +++ b/src/app/api/issues/claim/route.ts @@ -12,6 +12,7 @@ import { findAndReleaseStaleAgentWorkForIssue } from "@/lib/agent-work"; import { transitionIssueStatus } from "@/lib/issue-status"; import { recordClaimEvent } from "@/lib/issue-lifecycle"; import { enforceRateLimit } from "@/lib/rate-limit"; +import { withIssueMutationGuard } from "@/lib/issue-mutation-guard"; const IN_PROGRESS_STATUS = "status/in-progress"; const RATE_LIMIT = { limit: 30, windowMs: 10_000 }; @@ -76,6 +77,9 @@ export async function POST(request: Request) { return errorResponse("Force claim requires a maintainer token", 403); } + const guarded = await withIssueMutationGuard({ issueId: issueId as string, operation: "claim", agentName: agentName as string }, async (_tx, { resuming }) => { + let remoteStarted = false; + const value = await (async (): Promise => { // Fetch the issue from the local database to check its state and current labels const issue = await prisma.issue.findUnique({ where: { id: issueId as string }, @@ -143,12 +147,21 @@ export async function POST(request: Request) { return errorResponse(`Could not verify live GitHub labels: ${errorMessage}`, 503); } - // Refuse done issues (live labels are the authoritative base — #1037). - // Any live status/done makes the issue done, even alongside other status labels. + // Refuse human holds (live labels are authoritative), including on a retry + // after GitHub succeeded but the local mirror failed. + if (liveLabels.includes("status/blocked")) { + return errorResponse("Cannot claim a blocked issue", 400); + } if (liveLabels.includes("status/done")) { return errorResponse("Cannot claim a done issue", 400); } + const alreadyOwned = liveLabels.includes(`${AGENT_PREFIX}${agentName}`); + const hasOtherOwner = liveLabels.some((label) => label.startsWith(AGENT_PREFIX) && label !== `${AGENT_PREFIX}${agentName}`); + if (resuming && (!alreadyOwned || hasOtherOwner)) { + return errorResponse("Cannot resume claim because live issue ownership changed", 409); + } + // Check for active leases from OTHER agents — refuse unless force=true const activeLeases = await findActiveLeasesForIssue(issueId as string); const otherAgentLeases = activeLeases.filter((l) => l.agentName !== agentName); @@ -182,6 +195,7 @@ export async function POST(request: Request) { // buildNewLabels below drops every conflicting agent label from the // cache, so GitHub must lose all of them too — leaving extras behind // is the same GitHub/cache divergence class #1037 fights. + remoteStarted = true; for (const staleAgentLabel of analysis.existingAgents) { try { await removeIssueLabel(repoFullName, issueNumber, staleAgentLabel); @@ -202,14 +216,18 @@ export async function POST(request: Request) { const updatedLabels = [...labelsWithAgent.filter((l) => !l.startsWith("status/")), IN_PROGRESS_STATUS]; try { - // Add agent label on GitHub - await addIssueLabel(repoFullName, issueNumber, agentLabel); + remoteStarted = true; + // A resumed claim already landed remotely. Repair only the local mirror, + // lifecycle event and lease; do not repeat GitHub label mutations. + if (!resuming) await addIssueLabel(repoFullName, issueNumber, agentLabel); // Remove ALL existing status labels (not just the first) before adding // status/in-progress — keeps GitHub and the Prisma cache from // diverging when an issue carries more than one status label. // Base is the live GitHub label set (authoritative — #1037). - await transitionIssueStatus(repoFullName, issueNumber, liveLabels, IN_PROGRESS_STATUS); + if (!resuming || !liveLabels.includes(IN_PROGRESS_STATUS)) { + await transitionIssueStatus(repoFullName, issueNumber, liveLabels, IN_PROGRESS_STATUS); + } // Update local cache and record the lifecycle event in one transaction: // a claim whose event failed to commit would leave the stream missing an @@ -244,11 +262,14 @@ export async function POST(request: Request) { }); }); - // Create or renew a lease for this agent (issue #166) - await upsertLease({ + // Create or renew a lease for this agent (issue #166). + // #1256: the lease carries the opaque claim token that fences a later + // capacity-handoff release to THIS exact claim; echo it to the worker. + const lease = await upsertLease({ agentName: agentName as string, issueId: issueId as string, checkpoint: "issue_claimed", + mutationGuardHeld: true, }); // Write audit log with conflict analysis details @@ -282,7 +303,11 @@ export async function POST(request: Request) { }, }); - return NextResponse.json({ success: true, labels: updatedLabels }); + return NextResponse.json({ + success: true, + labels: updatedLabels, + claimToken: lease.lease.claimToken ?? null, + }); } catch (error) { const errorMessage = error instanceof Error ? error.message : "Unknown error"; @@ -303,6 +328,11 @@ export async function POST(request: Request) { return errorResponse(errorMessage, 500); } + })(); + return { complete: !remoteStarted || value.status < 500, value }; + }); + if (guarded.status === "busy") return errorResponse("Issue mutation is in progress", 409); + return guarded.value; } catch (error) { console.error("Claim issue failed:", error); return errorResponse("Failed to claim issue", 500); diff --git a/src/lib/agent-task.ts b/src/lib/agent-task.ts index 88bf5867..65d971c9 100644 --- a/src/lib/agent-task.ts +++ b/src/lib/agent-task.ts @@ -29,6 +29,14 @@ export interface PullRequestRef { export interface PrFixItemIdentity { id: string; generation: number; + /** + * #1256: per-hand-out receipt token minted for THIS hand-out (rotates on + * every issuance). Workers echo it back on reports; a `capacity-exhausted` + * settlement is only authorized with it, releasing exactly the hand-out it + * was issued with. Other outcomes ignore it, so pre-token workers are + * unaffected. + */ + handoutToken?: string; } export interface IdleTask { diff --git a/src/lib/capacity-handoff.test.ts b/src/lib/capacity-handoff.test.ts new file mode 100644 index 00000000..09577b11 --- /dev/null +++ b/src/lib/capacity-handoff.test.ts @@ -0,0 +1,173 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { + buildCapacitySkip, + capacityCooldownMs, + loadActiveCapacityHandoffs, + newClaimToken, + normalizeCapacity, + recordCapacityHandoff, + MAX_CAPACITY_LENGTH, +} from "./capacity-handoff"; + +describe("capacityCooldownMs (#1256)", () => { + const prev = process.env.DISPATCH_CAPACITY_COOLDOWN_MS; + afterEach(() => { + if (prev === undefined) delete process.env.DISPATCH_CAPACITY_COOLDOWN_MS; + else process.env.DISPATCH_CAPACITY_COOLDOWN_MS = prev; + }); + + it("defaults to 15 minutes", () => { + delete process.env.DISPATCH_CAPACITY_COOLDOWN_MS; + expect(capacityCooldownMs()).toBe(15 * 60 * 1000); + }); + + it("honors a configured override", () => { + process.env.DISPATCH_CAPACITY_COOLDOWN_MS = "120000"; + expect(capacityCooldownMs()).toBe(120_000); + }); + + it("clamps the bound: never below 1 minute, never above 6 hours", () => { + process.env.DISPATCH_CAPACITY_COOLDOWN_MS = "1"; + expect(capacityCooldownMs()).toBe(60_000); + process.env.DISPATCH_CAPACITY_COOLDOWN_MS = String(99 * 60 * 60 * 1000); + expect(capacityCooldownMs()).toBe(6 * 60 * 60 * 1000); + }); + + it("falls back to the default on garbage values", () => { + process.env.DISPATCH_CAPACITY_COOLDOWN_MS = "not-a-number"; + expect(capacityCooldownMs()).toBe(15 * 60 * 1000); + process.env.DISPATCH_CAPACITY_COOLDOWN_MS = "-5"; + expect(capacityCooldownMs()).toBe(15 * 60 * 1000); + process.env.DISPATCH_CAPACITY_COOLDOWN_MS = ""; + expect(capacityCooldownMs()).toBe(15 * 60 * 1000); + }); +}); + +describe("normalizeCapacity (#1256)", () => { + it("trims and accepts a bounded non-empty identifier", () => { + expect(normalizeCapacity(" provider/model ")).toBe("provider/model"); + expect(normalizeCapacity("x".repeat(MAX_CAPACITY_LENGTH))).toBe("x".repeat(MAX_CAPACITY_LENGTH)); + }); + + it("rejects blank, oversized, and non-string values", () => { + expect(normalizeCapacity("")).toBeNull(); + expect(normalizeCapacity(" ")).toBeNull(); + expect(normalizeCapacity("x".repeat(MAX_CAPACITY_LENGTH + 1))).toBeNull(); + expect(normalizeCapacity(undefined)).toBeNull(); + expect(normalizeCapacity(42)).toBeNull(); + }); +}); + +describe("newClaimToken (#1256)", () => { + it("mints distinct opaque tokens", () => { + const a = newClaimToken(); + const b = newClaimToken(); + expect(a).toMatch(/^claim_/); + expect(a).not.toBe(b); + }); +}); + +function makeHandoffClient() { + const rows: any[] = []; + return { + rows, + capacityHandoff: { + create: vi.fn(async ({ data }: any) => { + const row = { id: `h-${rows.length + 1}`, createdAt: new Date(), ...data }; + rows.push(row); + return row; + }), + findMany: vi.fn(async ({ where }: any) => + rows.filter( + (r) => + where?.cooldownUntil?.gt == null || + new Date(r.cooldownUntil).getTime() > new Date(where.cooldownUntil.gt).getTime(), + ), + ), + deleteMany: vi.fn(async ({ where }: any) => { + let count = 0; + for (let i = rows.length - 1; i >= 0; i--) { + if ( + where?.cooldownUntil?.lte != null && + new Date(rows[i].cooldownUntil).getTime() <= new Date(where.cooldownUntil.lte).getTime() + ) { + rows.splice(i, 1); + count++; + } + } + return { count }; + }), + }, + }; +} + +describe("recordCapacityHandoff / loadActiveCapacityHandoffs (#1256)", () => { + beforeEach(() => { + delete process.env.DISPATCH_CAPACITY_COOLDOWN_MS; + }); + + it("persists a bounded cooldown and only loads active rows", async () => { + const client = makeHandoffClient(); + const { cooldownUntil } = await recordCapacityHandoff(client, { + kind: "issue", + capacity: "provider/model-a", + repoFullName: "Org/Repo", + issueNumber: 7, + agentName: "alpha", + issueId: "issue-7", + }); + expect(cooldownUntil.getTime()).toBeGreaterThan(Date.now()); + expect(client.rows[0]).toMatchObject({ + kind: "issue", + capacity: "provider/model-a", + repoFullName: "Org/Repo", + issueNumber: 7, + agentName: "alpha", + }); + + // Expire it manually; the active load must prune and skip it. + client.rows[0].cooldownUntil = new Date(Date.now() - 1000); + const active = await loadActiveCapacityHandoffs(client); + expect(active).toHaveLength(0); + expect(client.rows).toHaveLength(0); // lazily pruned + }); +}); + +const HANDOFFS = [ + { kind: "issue", capacity: "provider/model-a", repoFullName: "org/repo", issueNumber: 10, prNumber: null }, + { kind: "pr_fix", capacity: "provider/model-a", repoFullName: "Org/Repo", issueNumber: null, prNumber: 55 }, +]; + +describe("buildCapacitySkip (#1256) — cooldown is scoped to the exhausted capacity", () => { + it("suppresses ONLY the exhausted capacity on the handed item", () => { + const { skipIssue, skipPr } = buildCapacitySkip(HANDOFFS, "provider/model-a"); + expect(skipIssue("org/repo", 10)).toBe(true); + expect(skipPr("org/repo", 55)).toBe(true); + // Other items are untouched even for the exhausted capacity. + expect(skipIssue("org/repo", 11)).toBe(false); + expect(skipPr("org/repo", 56)).toBe(false); + }); + + it("never suppresses an unrelated capacity — the work stays discoverable", () => { + const { skipIssue, skipPr } = buildCapacitySkip(HANDOFFS, "provider/model-b"); + expect(skipIssue("org/repo", 10)).toBe(false); + expect(skipPr("org/repo", 55)).toBe(false); + }); + + it("conservatively suppresses ANY cooled item for a worker that declares no capacity", () => { + const { skipIssue, skipPr } = buildCapacitySkip(HANDOFFS, undefined); + expect(skipIssue("org/repo", 10)).toBe(true); + expect(skipPr("org/repo", 55)).toBe(true); + expect(skipIssue("org/repo", 12)).toBe(false); + }); + + it("folds repo casing and never leaks a handoff into the wrong kind", () => { + const { skipIssue, skipPr } = buildCapacitySkip(HANDOFFS, "provider/model-a"); + expect(skipIssue("ORG/repo", 10)).toBe(true); + expect(skipPr("ORG/REPO", 55)).toBe(true); + // The issue handoff must not suppress a PR-fix item with the same number. + expect(skipPr("org/repo", 10)).toBe(false); + expect(skipIssue("org/repo", 55)).toBe(false); + }); +}); diff --git a/src/lib/capacity-handoff.ts b/src/lib/capacity-handoff.ts new file mode 100644 index 00000000..4df8ff1b --- /dev/null +++ b/src/lib/capacity-handoff.ts @@ -0,0 +1,189 @@ +// ─── Capacity-scoped handoff cooldowns (#1256) ─────────────────────────────── +// +// When a worker explicitly recognizes that its execution capacity cannot run a +// handed attempt, it settles the attempt with a fenced capacity handoff. The +// work item itself stays dispatchable to other compatible workers; this module +// only persists the bounded, capacity-SCOPED suppression: +// +// "capacity C should not be re-handed until T". +// +// A handoff is NOT a global wait: it never parks the item for every worker, +// never consumes task-repair budget (fixAttempts / generation are untouched), +// and never changes branch/PR identity. Workers whose declared capacity is +// unrelated keep receiving the work immediately. +// +// The capacity identifier is an opaque, worker-chosen string (for example a +// provider/model or runner-pool name). Dispatch is the authoritative owner of +// the handoff record; it does not interpret the capacity value and carries no +// coupling to any particular executor or source product. +// +// Consistency boundary: handoff rows are Dispatch-local dispatch state (like +// the lease and the queue cache), NOT mirrored to GitHub. GitHub remains the +// source of truth for labels/status; cooldown rows only shape WHO a Dispatch +// poll hands work to, and expire on their own. + +import { randomUUID } from "node:crypto"; +import { prisma } from "@/lib/prisma"; + +export type CapacityHandoffKind = "issue" | "pr_fix"; + +/** Maximum accepted length for a capacity identifier. */ +export const MAX_CAPACITY_LENGTH = 200; + +const DEFAULT_COOLDOWN_MS = 15 * 60 * 1000; // 15 minutes +const MIN_COOLDOWN_MS = 60 * 1000; // 1 minute +const MAX_COOLDOWN_MS = 6 * 60 * 60 * 1000; // 6 hours + +/** + * Bounded cooldown applied to a capacity handoff. `DISPATCH_CAPACITY_COOLDOWN_MS` + * overrides the default; the result is always clamped into [1m, 6h] so a + * misconfigured value can never park work indefinitely or churn instantly. + */ +export function capacityCooldownMs(now = Date.now()): number { + const raw = process.env.DISPATCH_CAPACITY_COOLDOWN_MS; + const parsed = raw !== undefined && raw.trim() !== "" ? Number(raw) : NaN; + const value = Number.isFinite(parsed) && parsed > 0 ? parsed : DEFAULT_COOLDOWN_MS; + return Math.min(Math.max(value, MIN_COOLDOWN_MS), MAX_COOLDOWN_MS); +} + +/** + * Validate + normalize a capacity identifier from a request. Returns the + * trimmed value, or null when absent/malformed. Routes validate before use; + * libs trust the result. + */ +export function normalizeCapacity(raw: unknown): string | null { + if (typeof raw !== "string") return null; + const trimmed = raw.trim(); + if (trimmed.length === 0 || trimmed.length > MAX_CAPACITY_LENGTH) return null; + return trimmed; +} + +/** Fresh opaque token fencing one claim (issued on lease creation, #1256). */ +export function newClaimToken(): string { + return `claim_${randomUUID()}`; +} + +/** Minimal client surface for handoff persistence/reads (test-injectable). */ +export interface CapacityHandoffClient { + capacityHandoff: { + create: (args: any) => Promise; + findMany: (args: any) => Promise; + deleteMany?: (args: any) => Promise<{ count: number }>; + }; +} + +export interface CapacityHandoffRecord { + kind: CapacityHandoffKind; + capacity: string; + repoFullName: string; + issueNumber?: number | null; + prNumber?: number | null; + agentName: string; + issueId?: string | null; + /** Attempt generation settled (pr_fix rows). */ + generation?: number | null; + note?: string | null; +} + +/** + * Capacity matching is case-folded: the identifier is opaque, and a worker + * that re-declares `Model-A` after handing off `model-a` IS the same + * capacity. Everything else about the value stays opaque to Dispatch. + */ +export function capacityEquals(a: string, b: string): boolean { + return a.toLowerCase() === b.toLowerCase(); +} + +/** + * Persist a handoff with its bounded cooldown. Returns the row (including + * `cooldownUntil`) so callers can echo the deadline to the worker. + */ +export async function recordCapacityHandoff( + client: CapacityHandoffClient, + record: CapacityHandoffRecord, + now: Date = new Date(), +): Promise<{ cooldownUntil: Date }> { + const cooldownUntil = new Date(now.getTime() + capacityCooldownMs(now.getTime())); + await client.capacityHandoff.create({ + data: { + kind: record.kind, + capacity: record.capacity, + repoFullName: record.repoFullName, + issueNumber: record.issueNumber ?? null, + prNumber: record.prNumber ?? null, + agentName: record.agentName, + issueId: record.issueId ?? null, + generation: record.generation ?? null, + cooldownUntil, + note: record.note ?? null, + }, + }); + return { cooldownUntil }; +} + +// Lazy-prune throttle: the prune is a write statement and next-task reads run +// on every worker poll, so at most one prune per minute per process. The +// active-row read still filters by cooldownUntil, so a skipped prune only +// delays dead-row removal; it never changes dispatch decisions. +let lastPruneAt = 0; +const PRUNE_INTERVAL_MS = 60_000; + +/** Active (not-yet-expired) handoffs, with throttled lazy pruning. */ +export async function loadActiveCapacityHandoffs( + client: CapacityHandoffClient, + now: Date = new Date(), +): Promise { + if (client.capacityHandoff.deleteMany && now.getTime() - lastPruneAt >= PRUNE_INTERVAL_MS) { + lastPruneAt = now.getTime(); + // Best-effort — a prune failure never blocks dispatch reads. + try { + await client.capacityHandoff.deleteMany({ where: { cooldownUntil: { lte: now } } }); + } catch { + // ignore + } + } + return client.capacityHandoff.findMany({ where: { cooldownUntil: { gt: now } } }); +} + +/** + * Build the skip predicates a dispatcher (next-task) applies to a request that + * declared `capacity` (undefined = the worker declared no capacity). + * + * Rule (fail-closed for undisclosed workers): + * - a handoff for an item suppresses re-hand when the requester's declared + * capacity EQUALS the exhausted capacity; + * - a requester that declared NO capacity is conservatively suppressed by ANY + * active handoff on the item, because Dispatch cannot prove it is not the + * exhausted capacity; + * - an unrelated declared capacity is never suppressed — this is the whole + * point of scoping the cooldown to the exhausted capacity. + * + * Repo comparison is case-folded, matching queue repo normalization. + */ +export function buildCapacitySkip( + handoffs: ReadonlyArray<{ + kind: string; + capacity: string; + repoFullName: string; + issueNumber?: number | null; + prNumber?: number | null; + }>, + requestedCapacity: string | undefined, +): { skipIssue: (repo: string | null | undefined, issueNumber: number) => boolean; skipPr: (repo: string, prNumber: number) => boolean } { + const fold = (repo: string | null | undefined) => (repo ?? "").toLowerCase(); + const issueKeys = new Set(); + const prKeys = new Set(); + for (const h of handoffs) { + const relevant = requestedCapacity === undefined || capacityEquals(h.capacity, requestedCapacity); + if (!relevant) continue; + if (h.kind === "issue" && h.issueNumber != null) { + issueKeys.add(`${fold(h.repoFullName)}#${h.issueNumber}`); + } else if (h.kind === "pr_fix" && h.prNumber != null) { + prKeys.add(`${fold(h.repoFullName)}#${h.prNumber}`); + } + } + return { + skipIssue: (repo, issueNumber) => issueKeys.has(`${fold(repo)}#${issueNumber}`), + skipPr: (repo, prNumber) => prKeys.has(`${fold(repo)}#${prNumber}`), + }; +} diff --git a/src/lib/groomer/run.test.ts b/src/lib/groomer/run.test.ts index a3bb551b..783f312a 100644 --- a/src/lib/groomer/run.test.ts +++ b/src/lib/groomer/run.test.ts @@ -96,7 +96,8 @@ vi.mock("@/lib/prisma", () => ({ prisma: mocks.prisma, })); -vi.mock("@/lib/lease", () => ({ +vi.mock("@/lib/lease", async (importOriginal) => ({ + ...(await importOriginal()), findActiveLeasesForIssue: mocks.findActiveLeasesForIssue, upsertLease: mocks.upsertLease, releaseLease: mocks.releaseLease, @@ -134,6 +135,7 @@ import { runHostedGroomer, UNVERIFIABLE_RETRY_BACKOFF_MINUTES, } from "./run"; +import { IssueMutationBusyError } from "@/lib/lease"; import { computeGroomingIssueFingerprint } from "./freshness"; const mockCandidate: GroomingCandidate = { @@ -1464,6 +1466,35 @@ describe("runHostedGroomer", () => { ); }); + it.each(["active capacity-release", "retained non-executing guard"])("defers %s without failure backoff and can service the next issue", async () => { + mocks.upsertLease.mockRejectedValueOnce(new IssueMutationBusyError()); + expect(await runHostedGroomer()).toBeNull(); + expect(mocks.callGroomerLLM).not.toHaveBeenCalled(); + expect(mocks.releaseLease).not.toHaveBeenCalled(); + expect(mocks.prisma.agentRun.create).not.toHaveBeenCalled(); + expect(mocks.prisma.auditLog.create).not.toHaveBeenCalled(); + expect(mocks.prisma.issue.update).not.toHaveBeenCalled(); + expect(mocks.prisma.groomingRun.update).toHaveBeenCalledWith(expect.objectContaining({ + where: { id: "gr-1" }, + data: expect.objectContaining({ status: "deferred", stage: "lease_acquisition", completedAt: expect.any(Date) }), + })); + mocks.selectGroomingCandidate.mockResolvedValueOnce({ ...mockCandidate, id: "next-issue", number: 43 }); + await runHostedGroomer(); + expect(mocks.callGroomerLLM).toHaveBeenCalledOnce(); + }); + + it("records and propagates unexpected lease-store errors", async () => { + mocks.upsertLease.mockRejectedValueOnce(new Error("database unavailable")); + await expect(runHostedGroomer()).rejects.toThrow("database unavailable"); + expect(mocks.callGroomerLLM).not.toHaveBeenCalled(); + expect(mocks.releaseLease).not.toHaveBeenCalled(); + expect(mocks.prisma.groomingRun.update).toHaveBeenCalledWith(expect.objectContaining({ + data: expect.objectContaining({ status: "failed", stage: "lease_acquisition", errorMessage: "database unavailable" }), + })); + expect(mocks.prisma.agentRun.create).toHaveBeenCalled(); + expect(mocks.prisma.auditLog.create).toHaveBeenCalled(); + }); + it("failure after groomingRun creation completes run as failed", async () => { mocks.callGroomerLLM.mockRejectedValue(new Error("LLM timeout")); @@ -1629,6 +1660,17 @@ describe("runHostedGroomer", () => { expect(result).toBeNull(); expect(mocks.upsertLease).not.toHaveBeenCalled(); expect(mocks.callGroomerLLM).not.toHaveBeenCalled(); + expect(mocks.prisma.agentRun.create).not.toHaveBeenCalled(); + expect(mocks.prisma.auditLog.create).not.toHaveBeenCalled(); + expect(mocks.prisma.issue.update).not.toHaveBeenCalled(); + expect(mocks.prisma.groomingRun.update).toHaveBeenCalledWith(expect.objectContaining({ + data: expect.objectContaining({ status: "deferred", completedAt: expect.any(Date) }), + })); + // No retry timestamp was written: the same candidate can run immediately + // once the other agent releases its lease. + mocks.findActiveLeasesForIssue.mockResolvedValue([]); + await runHostedGroomer(); + expect(mocks.callGroomerLLM).toHaveBeenCalledOnce(); }); it("force option overrides another active lease", async () => { diff --git a/src/lib/groomer/run.ts b/src/lib/groomer/run.ts index 4a46f2ea..e325d66a 100644 --- a/src/lib/groomer/run.ts +++ b/src/lib/groomer/run.ts @@ -1,7 +1,7 @@ import { prisma } from "@/lib/prisma"; import { addIssueComment, addIssueLabel, closeIssue, createIssue, updateIssueLabels, updateIssueTitleAndBody } from "@/lib/github"; import { fetchCollaboratorPermission } from "@/lib/github-issues"; -import { findActiveLeasesForIssue, releaseLease, upsertLease } from "@/lib/lease"; +import { findActiveLeasesForIssue, IssueMutationBusyError, releaseLease, upsertLease } from "@/lib/lease"; import { acquireGroomerLock, heartbeatGroomerLock, HEARTBEAT_MS, releaseGroomerLock } from "./groomer-lock"; import { selectGroomingCandidate } from "./selector"; import { buildIssueContext, fetchIssueComments } from "./context"; @@ -249,16 +249,7 @@ async function executeGroomerRun( staleReasons: candidate.staleReasons, }); - const activeLeases = await deps.findActiveLeases(candidate.id); - const hasOtherLease = activeLeases.some((lease: { agentName?: string }) => lease.agentName !== "hosted-groomer"); - if (hasOtherLease && !options.force) return null; - - const { lease } = await deps.upsertLease({ - agentName: "hosted-groomer", - issueId: candidate.id, - checkpoint: "issue_claimed", - ttlMs: GROOMER_LEASE_TTL_MS, - }); + let lease: { id: string } | undefined; const leaseExpiresAt = Date.now() + GROOMER_LEASE_TTL_MS; // Freshness (#1064): a human comment after this instant is new evidence. @@ -272,6 +263,22 @@ async function executeGroomerRun( let unreadableBackoffWritten = false; try { + stage = "lease_acquisition"; + const activeLeases = await deps.findActiveLeases(candidate.id); + const hasOtherLease = activeLeases.some((active: { agentName?: string }) => active.agentName !== "hosted-groomer"); + if (hasOtherLease && !options.force) { + await completeGroomingRunRecord(deps.prisma, groomingRun.id, { + status: "deferred", stage, errorMessage: null, retryable: true, + }); + return null; + } + ({ lease } = await deps.upsertLease({ + agentName: "hosted-groomer", + issueId: candidate.id, + checkpoint: "issue_claimed", + ttlMs: GROOMER_LEASE_TTL_MS, + })); + stage = "selected"; let comments: Awaited> = []; try { comments = await deps.fetchComments(candidate.repoFullName, candidate.number); @@ -1304,6 +1311,13 @@ async function executeGroomerRun( } catch (error) { const message = error instanceof Error ? error.message : "Unknown groomer error"; + if (stage === "lease_acquisition" && error instanceof IssueMutationBusyError) { + await completeGroomingRunRecord(deps.prisma, groomingRun.id, { + status: "deferred", stage, errorMessage: null, retryable: true, + }); + return null; + } + // Complete GroomingRun as failed try { await completeGroomingRunRecord(deps.prisma, groomingRun.id, { @@ -1347,7 +1361,7 @@ async function executeGroomerRun( }); throw error; } finally { - await deps.releaseLease(lease.id); + if (lease) await deps.releaseLease(lease.id); } } diff --git a/src/lib/issue-capacity-handoff.test.ts b/src/lib/issue-capacity-handoff.test.ts new file mode 100644 index 00000000..7d895d47 --- /dev/null +++ b/src/lib/issue-capacity-handoff.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it, beforeEach, vi } from "vitest"; + +const { githubMocks, claimGateMocks, claimMocks, guardState } = vi.hoisted(() => ({ + guardState: { resuming: false }, + githubMocks: { removeIssueLabel: vi.fn().mockResolvedValue(undefined) }, + claimGateMocks: { getLiveIssueLabels: vi.fn() }, + claimMocks: { releaseIssueClaim: vi.fn() }, +})); + +vi.mock("./github", () => ({ removeIssueLabel: githubMocks.removeIssueLabel })); +vi.mock("./claim-gate", () => ({ getLiveIssueLabels: claimGateMocks.getLiveIssueLabels })); +vi.mock("./issue-claim", () => ({ releaseIssueClaim: claimMocks.releaseIssueClaim })); +vi.mock("./issue-mutation-guard", () => ({ + withIssueMutationGuard: async (_request: unknown, callback: (...args: any[]) => Promise<{ complete: boolean; value: unknown }>) => ({ + status: "completed", value: (await callback(undefined, { resuming: guardState.resuming })).value, + }), +})); + +import { recoverIssueCapacityHandoff, IssueCapacityHandoffClient } from "./issue-capacity-handoff"; + +const REPO = "org/repo", NUMBER = 42, ISSUE_ID = "issue-42", AGENT = "alpha", TOKEN = "claim_token_abc"; + +function makeClient(overrides: Record = {}) { + const audits: any[] = [], mirrors: any[] = [], handoffs: any[] = []; + const client: any = { + audits, mirrors, handoffs, + issue: { + findUnique: async () => ({ id: ISSUE_ID, number: NUMBER, state: "open", labels: ["agent/alpha", "status/in-progress"], blockedReason: null, repository: { fullName: REPO } }), + update: async ({ where, data }: any) => mirrors.push({ where, data }), + }, + lease: { + findUnique: async () => ({ id: "lease-1", agentName: AGENT, issueId: ISSUE_ID, claimToken: TOKEN, expiredAt: new Date(Date.now() + 600_000) }), + deleteMany: vi.fn(async () => ({ count: 1 })), + }, + agentWork: { findMany: async () => [], update: async () => ({}) }, + agentWorkHistory: { create: async () => ({}) }, + auditLog: { create: async ({ data }: any) => audits.push(data) }, + capacityHandoff: { create: async ({ data }: any) => { const row = { id: `handoff-${handoffs.length + 1}`, ...data }; handoffs.push(row); return row; } }, + ...overrides, + }; + return client as IssueCapacityHandoffClient & { audits: any[]; mirrors: any[]; handoffs: any[] }; +} + +function call(client: any) { + return recoverIssueCapacityHandoff({ client, issueId: ISSUE_ID, repoFullName: REPO, issueNumber: NUMBER, agentName: AGENT, claimToken: TOKEN, capacity: "provider/model-a", actor: AGENT }); +} + +describe("recoverIssueCapacityHandoff", () => { + beforeEach(() => { + vi.clearAllMocks(); + guardState.resuming = false; + githubMocks.removeIssueLabel.mockResolvedValue(undefined); + claimGateMocks.getLiveIssueLabels.mockResolvedValue(["agent/alpha", "status/in-progress"]); + claimMocks.releaseIssueClaim.mockResolvedValue({ labels: ["status/ready"], status: "status/ready", statusNote: null }); + }); + + it("releases through the full policy after both live reads succeed", async () => { + const client = makeClient(); + expect((await call(client)).ok).toBe(true); + expect(claimMocks.releaseIssueClaim).toHaveBeenCalledTimes(1); + expect(client.lease.deleteMany).toHaveBeenCalledTimes(1); + }); + + it.each(["status/blocked", "status/done"])("refuses late %s without ownership or lease mutation", async (status) => { + claimGateMocks.getLiveIssueLabels.mockResolvedValueOnce(["agent/alpha", "status/in-progress"]).mockResolvedValueOnce(["agent/alpha", status]); + const client = makeClient(); + expect(await call(client)).toMatchObject({ ok: false, code: status === "status/blocked" ? "human_hold" : "issue_done" }); + expect(claimMocks.releaseIssueClaim).not.toHaveBeenCalled(); + expect(githubMocks.removeIssueLabel).not.toHaveBeenCalled(); + expect(client.lease.deleteMany).not.toHaveBeenCalled(); + expect(client.mirrors).toHaveLength(0); + }); + + it("second read failure returns 503 without writes and retains lease", async () => { + claimGateMocks.getLiveIssueLabels.mockResolvedValueOnce(["agent/alpha", "status/in-progress"]).mockRejectedValueOnce(new Error("offline")); + const client = makeClient(); + const result = await call(client); + expect(result).toMatchObject({ ok: false, httpStatus: 503, code: "github_unavailable" }); + expect(githubMocks.removeIssueLabel).not.toHaveBeenCalled(); + expect(client.lease.deleteMany).not.toHaveBeenCalled(); + expect(client.mirrors).toHaveLength(0); + expect(client.audits.at(-1)).toMatchObject({ success: false }); + }); + + it("failed targeted removal returns 500 without cache mirroring", async () => { + claimGateMocks.getLiveIssueLabels.mockResolvedValueOnce(["agent/alpha", "status/in-progress"]).mockResolvedValueOnce(["agent/omega", "status/in-progress"]); + githubMocks.removeIssueLabel.mockRejectedValueOnce(new Error("write failed")); + const client = makeClient(); + const result = await call(client); + expect(result).toMatchObject({ ok: false, httpStatus: 500, code: "release_failed" }); + expect(client.lease.deleteMany).not.toHaveBeenCalled(); + expect(client.mirrors).toHaveLength(0); + expect(client.audits.at(-1)).toMatchObject({ success: false }); + }); + + it("resumes exact-token cleanup after prior label removal without changing resting status", async () => { + guardState.resuming = true; + claimGateMocks.getLiveIssueLabels.mockResolvedValue(["status/in-progress"]); + const client = makeClient(); + const result = await call(client); + expect(result).toMatchObject({ ok: true, status: "status/in-progress" }); + expect(claimMocks.releaseIssueClaim).not.toHaveBeenCalled(); + expect(client.lease.deleteMany).toHaveBeenCalledWith({ where: { id: "lease-1", agentName: AGENT, issueId: ISSUE_ID, claimToken: TOKEN } }); + }); + + it("refuses a claim without a token", async () => { + const client = makeClient({ lease: { findUnique: async () => ({ id: "lease-1", claimToken: null, expiredAt: new Date(Date.now() + 600_000) }), deleteMany: vi.fn() } }); + expect(await call(client)).toMatchObject({ ok: false, code: "no_active_claim" }); + expect(client.audits).toHaveLength(0); + }); +}); diff --git a/src/lib/issue-capacity-handoff.ts b/src/lib/issue-capacity-handoff.ts new file mode 100644 index 00000000..cfcbf251 --- /dev/null +++ b/src/lib/issue-capacity-handoff.ts @@ -0,0 +1,425 @@ +// ─── Fenced capacity-exhaustion recovery for issues (#1256) ────────────────── +// +// A worker that has CLAIMED an issue but discovers its execution capacity +// cannot run the task needs a way to hand the work back that is safe under +// races. Plain unclaim accepts only (issue identity, agent name): a stale +// worker can release a claim it no longer holds the moment another agent (or +// a human reassignment) moves the label, and lease expiry alone never clears +// the `agent/*` label or guarantees rediscovery. +// +// A durable, nonexpiring issue mutation guard excludes cooperating claim +// and lease writers throughout remote cleanup. Failed cleanup retains the +// lease and guard for exact-token retry; a crashed executor requires operator +// recovery. GitHub itself provides no label CAS against external mutations. + +import { AGENT_PREFIX, getAgentFromLabels } from "@/types"; +import { removeIssueLabel } from "./github"; +import { getLiveIssueLabels } from "./claim-gate"; +import { releaseIssueClaim } from "./issue-claim"; +import { recordCapacityHandoff, type CapacityHandoffClient } from "./capacity-handoff"; +import { withIssueMutationGuard } from "./issue-mutation-guard"; + +export interface IssueCapacityHandoffClient extends CapacityHandoffClient { + issue: { + findUnique: (args: any) => Promise; + update: (args: any) => Promise; + }; + lease: { + findUnique: (args: any) => Promise; + deleteMany: (args: any) => Promise<{ count: number }>; + }; + agentWork: { + findMany: (args: any) => Promise; + update: (args: any) => Promise; + }; + agentWorkHistory: { + create: (args: any) => Promise; + }; + auditLog: { + create: (args: any) => Promise; + }; +} + +export type IssueCapacityHandoffInput = { + client: IssueCapacityHandoffClient; + issueId: string; + repoFullName: string; + issueNumber: number; + agentName: string; + claimToken: string; + capacity: string; + note?: string | null; + /** Audit actor identity (token-derived for bound workers). */ + actor: string; + now?: Date; +}; + +export type IssueCapacityHandoffResult = + | { + ok: true; + labels: string[]; + status: string | null; + statusNote: string | null; + /** null only when the best-effort post-release cooldown write failed. */ + cooldownUntil: Date | null; + } + | { + ok: false; + /** HTTP status to surface. */ + httpStatus: number; + /** Machine-readable refusal class. */ + code: + | "issue_not_found" + | "identity_mismatch" + | "issue_closed" + | "issue_done" + | "human_hold" + | "no_active_claim" + | "stale_claim_token" + | "claim_expired" + | "ownership_changed" + | "handoff_race_lost" + | "github_unavailable" + | "release_failed"; + message: string; + }; + +/** + * Fenced capacity-exhaustion recovery for a claimed issue. See file header for + * the ownership/human-hold semantics. Never consumes task-repair budget and + * never touches PR/branch identity (branch/PR metadata stays on the lease + * history and AgentWork rows; the issue itself only loses its claim). + */ +export async function recoverIssueCapacityHandoff( + input: IssueCapacityHandoffInput, +): Promise { + const guarded = await withIssueMutationGuard({ + issueId: input.issueId, + operation: "capacity-release", + agentName: input.agentName, + claimToken: input.claimToken, + }, async (_tx, { resuming }) => { + const value = await recoverIssueCapacityHandoffUnlocked(input, resuming); + return { complete: !(value.ok === false && (value.code === "release_failed" || value.code === "github_unavailable")), value }; + }); + if (guarded.status === "busy") return { + ok: false, httpStatus: 409, code: "handoff_race_lost", + message: "Issue mutation is in progress; retry after recovery completes", + }; + return guarded.value; +} + +async function recoverIssueCapacityHandoffUnlocked( + input: IssueCapacityHandoffInput, + resuming: boolean, +): Promise { + const { client, issueId, repoFullName, issueNumber, agentName, claimToken, capacity } = input; + const now = input.now ?? new Date(); + const agentLabel = `${AGENT_PREFIX}${agentName}`; + + // ── 1. Identity + non-negotiable issue state (cache is fine here) ──────── + const issue = await client.issue.findUnique({ + where: { id: issueId }, + include: { repository: true }, + }); + if (!issue) { + return { ok: false, httpStatus: 404, code: "issue_not_found", message: "Issue not found in local cache" }; + } + // #1129 identity invariant: canonical repo/number come from the DB row; any + // cross-reference mix-up is refused before a single write. + if (issue.repository?.fullName !== repoFullName || issue.number !== issueNumber) { + return { + ok: false, + httpStatus: 400, + code: "identity_mismatch", + message: `issueId does not match repoFullName/issueNumber; expected ${issue.repository?.fullName}#${issue.number}`, + }; + } + if (issue.state === "closed") { + return { ok: false, httpStatus: 400, code: "issue_closed", message: "Cannot hand off a closed issue" }; + } + const cachedStatus = issue.labels.find((l: string) => l.startsWith("status/")); + if (cachedStatus === "status/done") { + return { ok: false, httpStatus: 400, code: "issue_done", message: "Cannot hand off a done issue" }; + } + // Human hold: a deliberate block carries a blockedReason; releasing the + // claim here would drag a human-owned decision back into circulation. + if (cachedStatus === "status/blocked" && (issue.blockedReason ?? null) != null) { + return { + ok: false, + httpStatus: 409, + code: "human_hold", + message: "Issue is deliberately blocked (blockedReason is set); a capacity handoff never overrides a human hold", + }; + } + + // ── 2. Fence to the exact claim (agent + lease row + issued token) ─────── + const lease = await client.lease.findUnique({ + where: { agentName_issueId: { agentName, issueId } }, + }); + if (!lease || !lease.claimToken) { + return { + ok: false, + httpStatus: 409, + code: "no_active_claim", + message: + "No active tokened claim exists for this agent+issue; this claim was already released, superseded, or predates claim tokens — re-claim to obtain a fresh fenced token (do not force an unclaim)", + }; + } + if (lease.claimToken !== claimToken) { + // A re-claim produced a new lease row with a new token. This token belongs + // to a previous (stale) claim and is inert. + return { + ok: false, + httpStatus: 409, + code: "stale_claim_token", + message: "claimToken does not match the current claim; a re-issued claim invalidates prior tokens", + }; + } + if (!resuming && new Date(lease.expiredAt).getTime() <= now.getTime()) { + return { + ok: false, + httpStatus: 409, + code: "claim_expired", + message: "Claim has expired; stale-claim recovery owns release from here", + }; + } + + // ── 3. Ownership against LIVE GitHub labels (#1037), fail closed ───────── + let liveLabels: string[]; + try { + liveLabels = await getLiveIssueLabels(repoFullName, issueNumber); + } catch (error) { + const message = error instanceof Error ? error.message : "Unknown error"; + return { + ok: false, + httpStatus: 503, + code: "github_unavailable", + message: `Could not verify live GitHub labels: ${message}`, + }; + } + const refusal = classifyLiveLabels(liveLabels, agentLabel); + // A previous attempt may have removed the label before a later write + // failed. The durable guard still excludes a new claim epoch. + if (refusal && (!resuming || refusal.code === "human_hold" || refusal.code === "issue_done")) return refusal; + + // ── 4. Recheck live labels before any remote mutation ──────────────────── + let releaseLabels = liveLabels; + let ownershipChangedPostCas = false; + try { + const recheck = await getLiveIssueLabels(repoFullName, issueNumber); + const changed = classifyLiveLabels(recheck, agentLabel, true); + if (changed?.code === "human_hold" || changed?.code === "issue_done") return changed; + if (changed || !recheck.includes(agentLabel)) ownershipChangedPostCas = true; + releaseLabels = recheck; + } catch (error) { + const message = error instanceof Error ? error.message : "Unknown error"; + await writeFailureAudit(client, input, liveLabels, `Could not verify live GitHub labels before release: ${message}`); + return { ok: false, httpStatus: 503, code: "github_unavailable", message: `Could not verify live GitHub labels before release: ${message}` }; + } + + // ── 5. Release the claim with the shared resting-status policy ─────────── + // The fresh label set is authoritative. A changed owner or newly applied + // human hold permits only targeted own-label cleanup, never status changes. + let labels: string[]; + let status: string | null; + let statusNote: string | null; + if (ownershipChangedPostCas) { + const newOwner = getAgentFromLabels(releaseLabels); + try { + // Targeted, idempotent: only our own label. Never a status write — + // the new owner's transition stands. + await removeIssueLabel(repoFullName, issueNumber, agentLabel); + } catch (error) { + const message = error instanceof Error ? error.message : "Unknown error"; + await writeFailureAudit(client, input, liveLabels, `Could not remove claim label: ${message}`); + return { ok: false, httpStatus: 500, code: "release_failed", message: `Could not remove claim label; lease and release guard retained: ${message}` }; + } + labels = releaseLabels.filter((l: string) => l !== agentLabel); + status = labels.find((l: string) => l.startsWith("status/")) ?? null; + statusNote = newOwner + ? `claim moved to ${newOwner} before cleanup; only our own agent label was removed — no status change` + : "no agent claim present before cleanup; mirrored GitHub minus our label — no status change"; + try { + await client.issue.update({ + where: { id: issueId }, + data: { labels, lastSyncedAt: now }, + }); + } catch (error) { + console.error(`capacity handoff: cache mirror failed for ${repoFullName}#${issueNumber}:`, error); + } + } else try { + const released = await releaseIssueClaim({ + prisma: client, + issue: { ...issue, labels: releaseLabels }, + repoFullName, + issueNumber, + agentName, + }); + labels = released.labels; + status = released.status; + statusNote = released.statusNote; + } catch (error) { + const message = error instanceof Error ? error.message : "Unknown error"; + await writeFailureAudit(client, input, liveLabels, `Claim label release failed; lease retained: ${message}`); + return { + ok: false, + httpStatus: 500, + code: "release_failed", + message: `Claim label release failed; lease and release guard retained: ${message}`, + }; + } + + // Consume the exact lease only after GitHub cleanup succeeded. + const { count } = await client.lease.deleteMany({ + where: { id: lease.id, agentName, issueId, claimToken }, + }); + if (count !== 1) return { ok: false, httpStatus: 409, code: "handoff_race_lost", message: "Claim changed concurrently; no lease was released" }; + + // ── 6. Retire active AgentWork rows with history (audit trail) ─────────── + try { + const activeWork = await client.agentWork.findMany({ + where: { agentName, issueId, state: { in: ["CLAIMED", "IN_PROGRESS", "BLOCKED"] } }, + select: { id: true }, + }); + for (const work of activeWork) { + await client.agentWork.update({ + where: { id: work.id }, + data: { state: "RELEASED", leaseExpiresAt: now }, + }); + await client.agentWorkHistory.create({ + data: { + workId: work.id, + action: "capacity_handoff", + state: "RELEASED", + summary: `Capacity exhausted (${capacity}); claim released by fenced handoff`, + }, + }); + } + } catch (error) { + // AgentWork bookkeeping is visibility, not ownership: the claim is + // already released. Log and continue — never fail the handoff over it. + console.error(`capacity handoff: AgentWork release failed for ${repoFullName}#${issueNumber}:`, error); + } + + // ── 7. Capacity-scoped cooldown + audit ────────────────────────────────── + // The release itself already committed. A cooldown-write failure must not + // turn a successful handoff into a 500 (the lease is gone; a retry would + // read 409 and the worker would believe nothing happened). Degrade to "no + // cooldown": worst case the exhausted capacity may be re-handed and report + // exhaustion again — bounded, and strictly better than a false failure. + let cooldownUntil: Date | null = null; + try { + cooldownUntil = (await recordCapacityHandoff( + client, + { + kind: "issue", + capacity, + repoFullName, + issueNumber, + agentName, + issueId, + note: input.note ?? null, + }, + now, + )).cooldownUntil; + } catch (error) { + console.error(`capacity handoff: cooldown write failed for ${repoFullName}#${issueNumber}; releasing without cooldown:`, error); + } + + try { + await client.auditLog.create({ + data: { + actor: input.actor, + action: "issue_capacity_handoff", + repoFullName, + issueNumber, + issueId, + beforeLabels: liveLabels, + afterLabels: labels, + success: true, + notes: + `capacity=${capacity}; cooldownUntil=${cooldownUntil ? cooldownUntil.toISOString() : "none (cooldown write failed)"}; ` + + `lease ${lease.id} released by claim-token fence${ + ownershipChangedPostCas + ? "; ownership changed before cleanup: ownership-only release of our label" + : "" + }${input.note ? `; ${input.note}` : ""}`, + }, + }); + } catch { + // Audit failure is best-effort; the release already happened. + } + + return { ok: true, labels, status, statusNote, cooldownUntil }; +} + +/** + * Live-label refusals applied both before the lease CAS. Status checks read + * the LIVE set (not the cache): a `status/blocked` that exists on GitHub but + * has not reached the cache is still a hold the handoff must not reverse, so + * any live blocked label is refused fail-closed — the stale-recovery/operator + * paths own releasing one. + */ +type HandoffRefusal = Extract; + +function classifyLiveLabels( + liveLabels: string[], + agentLabel: string, + allowMissingOwner = false, +): HandoffRefusal | null { + if (liveLabels.includes("status/done")) { + return { ok: false, httpStatus: 409, code: "issue_done", message: "Issue is done on GitHub; nothing to hand off" }; + } + if (liveLabels.includes("status/blocked")) { + return { + ok: false, + httpStatus: 409, + code: "human_hold", + message: "Issue carries status/blocked on GitHub; a capacity handoff never reverses a block — release the block explicitly first", + }; + } + const liveAgent = getAgentFromLabels(liveLabels); + if (liveAgent && liveAgent !== agentLabel) { + return { + ok: false, + httpStatus: 409, + code: "ownership_changed", + message: `Issue claim on GitHub belongs to ${liveAgent}; refusing to release another owner's claim`, + }; + } + if (!liveAgent && !allowMissingOwner) { + return { + ok: false, + httpStatus: 409, + code: "ownership_changed", + message: "No agent claim is present on GitHub; this handoff's claim is already released or was changed by a human", + }; + } + return null; +} + +async function writeFailureAudit( + client: IssueCapacityHandoffClient, + input: IssueCapacityHandoffInput, + beforeLabels: string[], + errorMessage: string, +): Promise { + try { + await client.auditLog.create({ + data: { + actor: input.actor, + action: "issue_capacity_handoff", + repoFullName: input.repoFullName, + issueNumber: input.issueNumber, + issueId: input.issueId, + beforeLabels, + afterLabels: [], + success: false, + errorMessage, + }, + }); + } catch { + // Audit failure must not mask the refusal. + } +} diff --git a/src/lib/issue-mutation-guard.test.ts b/src/lib/issue-mutation-guard.test.ts new file mode 100644 index 00000000..87d3645a --- /dev/null +++ b/src/lib/issue-mutation-guard.test.ts @@ -0,0 +1,130 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { withIssueMutationGuard } from "./issue-mutation-guard"; + +vi.mock("@/lib/prisma", () => ({ prisma: { + issueMutationGuard: { create: vi.fn(), findUnique: vi.fn(), updateMany: vi.fn(), deleteMany: vi.fn() }, + $transaction: vi.fn(), +} })); + +const request = { issueId: "issue-1", operation: "claim" }; +const row = (overrides = {}) => ({ issueId: "issue-1", ownerToken: "existing", operation: "claim", agentName: null, claimToken: null, executing: false, ...overrides }); + +describe("issue mutation guard", () => { + let prisma: Awaited["prisma"]; + beforeEach(async () => { + vi.clearAllMocks(); + prisma = (await import("@/lib/prisma")).prisma; + }); + + it("creates a committed fence and removes only its owner on completion", async () => { + vi.mocked(prisma.issueMutationGuard.create).mockResolvedValue({} as never); + vi.mocked(prisma.issueMutationGuard.findUnique).mockResolvedValue(row({ executing: true }) as never); + vi.mocked(prisma.issueMutationGuard.deleteMany as any).mockResolvedValue({ count: 1 }); + const callback = vi.fn(async (tx, context) => ({ complete: true, value: [tx, context.resuming] })); + await expect(withIssueMutationGuard(request, callback)).resolves.toMatchObject({ status: "completed", value: [undefined, false] }); + expect(callback).toHaveBeenCalledOnce(); + expect(prisma.issueMutationGuard.deleteMany).toHaveBeenCalledWith({ where: { issueId: "issue-1", ownerToken: "existing", executing: true } }); + expect(prisma.$transaction).not.toHaveBeenCalled(); + }); + + it("allows only the exact nonexecuting release to CAS-resume", async () => { + vi.mocked(prisma.issueMutationGuard.create).mockRejectedValue({ code: "P2002" } as never); + vi.mocked(prisma.issueMutationGuard.findUnique).mockResolvedValue(row({ operation: "capacity-release", agentName: "worker", claimToken: "token" }) as never); + vi.mocked(prisma.issueMutationGuard.updateMany as any).mockResolvedValue({ count: 1 }); + const result = await withIssueMutationGuard({ issueId: "issue-1", operation: "capacity-release", agentName: "worker", claimToken: "token" }, async (_, context) => ({ complete: false, value: context.resuming })); + expect(result).toMatchObject({ status: "retained", value: true }); + expect(prisma.issueMutationGuard.updateMany).toHaveBeenCalledWith({ where: { issueId: "issue-1", ownerToken: "existing", executing: false }, data: { executing: true } }); + expect(prisma.issueMutationGuard.updateMany).toHaveBeenLastCalledWith({ where: { issueId: "issue-1", ownerToken: "existing", executing: true }, data: { executing: false } }); + }); + + it.each([ + { operation: "capacity-release", agentName: "other", claimToken: "token" }, + { operation: "capacity-release", agentName: "worker", claimToken: "other" }, + { operation: "claim", agentName: "worker", claimToken: "token" }, + ])("rejects mismatched credentials", async (attempt) => { + vi.mocked(prisma.issueMutationGuard.create).mockRejectedValue({ code: "P2002" } as never); + vi.mocked(prisma.issueMutationGuard.findUnique).mockResolvedValue(row({ operation: "capacity-release", agentName: "worker", claimToken: "token" }) as never); + const callback = vi.fn(); + expect(await withIssueMutationGuard({ issueId: "issue-1", ...attempt }, callback)).toMatchObject({ status: "busy" }); + expect(callback).not.toHaveBeenCalled(); + expect(prisma.issueMutationGuard.updateMany).not.toHaveBeenCalled(); + }); + + it("allows only the exact same agent to resume a nonexecuting claim", async () => { + vi.mocked(prisma.issueMutationGuard.create).mockRejectedValue({ code: "P2002" } as never); + vi.mocked(prisma.issueMutationGuard.findUnique).mockResolvedValue(row({ agentName: "worker" }) as never); + vi.mocked(prisma.issueMutationGuard.updateMany as any).mockResolvedValue({ count: 1 }); + const callback = vi.fn(async (_, context) => ({ complete: true, value: context.resuming })); + await expect(withIssueMutationGuard({ ...request, agentName: "worker" }, callback)).resolves.toMatchObject({ status: "completed", value: true }); + expect(callback).toHaveBeenCalledOnce(); + }); + + it("keeps an executing exact retry busy", async () => { + vi.mocked(prisma.issueMutationGuard.create).mockRejectedValue({ code: "P2002" } as never); + vi.mocked(prisma.issueMutationGuard.findUnique).mockResolvedValue(row({ operation: "capacity-release", agentName: "worker", claimToken: "token", executing: true }) as never); + expect(await withIssueMutationGuard({ issueId: "issue-1", operation: "capacity-release", agentName: "worker", claimToken: "token" }, vi.fn())).toMatchObject({ status: "busy" }); + expect(prisma.issueMutationGuard.updateMany).not.toHaveBeenCalled(); + }); + + it("blocks a same-agent reclaim while old cleanup is paused, then admits a new epoch", async () => { + let stored: any = null; + vi.mocked(prisma.issueMutationGuard.create as any).mockImplementation(async ({ data }: any) => { + if (stored) throw { code: "P2002" }; + stored = { ...data }; + return stored; + }); + vi.mocked(prisma.issueMutationGuard.findUnique as any).mockImplementation(async () => stored); + vi.mocked(prisma.issueMutationGuard.deleteMany as any).mockImplementation(async ({ where }: any) => { + if (stored?.ownerToken !== where.ownerToken) return { count: 0 }; + stored = null; + return { count: 1 }; + }); + let resume!: () => void; + let started!: () => void; + const entered = new Promise((resolve) => { started = resolve; }); + const pause = new Promise((resolve) => { resume = resolve; }); + const release = withIssueMutationGuard({ issueId: "issue-1", operation: "capacity-release", agentName: "worker", claimToken: "old-token" }, async () => { + started(); + await pause; + return { complete: true, value: "old cleanup finished" }; + }); + await entered; + const newClaim = vi.fn(async () => ({ complete: true, value: "new-token" })); + expect(await withIssueMutationGuard({ ...request, agentName: "worker" }, newClaim)).toMatchObject({ status: "busy" }); + expect(newClaim).not.toHaveBeenCalled(); + resume(); + await release; + expect(await withIssueMutationGuard({ ...request, agentName: "worker" }, newClaim)).toMatchObject({ status: "completed", value: "new-token" }); + }); + + it("removes a new lease guard after transient failure so a retry can acquire it", async () => { + let stored: any = null; + vi.mocked(prisma.issueMutationGuard.create as any).mockImplementation(async ({ data }: any) => { + if (stored) throw { code: "P2002" }; + stored = { ...data }; + return stored; + }); + vi.mocked(prisma.issueMutationGuard.findUnique as any).mockImplementation(async () => stored); + vi.mocked(prisma.issueMutationGuard.deleteMany as any).mockImplementation(async ({ where }: any) => { + if (stored?.ownerToken !== where.ownerToken) return { count: 0 }; + stored = null; + return { count: 1 }; + }); + const leaseRequest = { issueId: "issue-1", operation: "lease", agentName: "worker" }; + await expect(withIssueMutationGuard(leaseRequest, async () => { throw new Error("transient"); })).rejects.toThrow("transient"); + expect(stored).toBeNull(); + const retry = await withIssueMutationGuard(leaseRequest, async () => ({ complete: true, value: "lease created" })); + expect(retry).toMatchObject({ status: "completed", value: "lease created" }); + expect(prisma.issueMutationGuard.create).toHaveBeenCalledTimes(2); + }); + + it("reopens a thrown callback but keeps the owner barrier", async () => { + vi.mocked(prisma.issueMutationGuard.create).mockResolvedValue({} as never); + vi.mocked(prisma.issueMutationGuard.findUnique).mockResolvedValue(row({ executing: true }) as never); + vi.mocked(prisma.issueMutationGuard.updateMany as any).mockResolvedValue({ count: 1 }); + const error = new Error("failure"); + await expect(withIssueMutationGuard(request, async () => { throw error; })).rejects.toBe(error); + expect(prisma.issueMutationGuard.updateMany).toHaveBeenCalledWith({ where: { issueId: "issue-1", ownerToken: "existing", executing: true }, data: { executing: false } }); + expect(prisma.issueMutationGuard.deleteMany).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/issue-mutation-guard.ts b/src/lib/issue-mutation-guard.ts new file mode 100644 index 00000000..fde26ccc --- /dev/null +++ b/src/lib/issue-mutation-guard.ts @@ -0,0 +1,67 @@ +import { Prisma } from "@prisma/client"; +import { prisma } from "@/lib/prisma"; + +export interface IssueMutationRequest { + issueId: string; + operation: string; + agentName?: string; + claimToken?: string; +} + +export interface IssueMutationContext { resuming: boolean } +export type IssueMutationResult = + | { status: "busy"; ownerToken: string; operation: string } + | { status: "completed"; value: T } + | { status: "retained"; ownerToken: string; value: T }; + +/** A committed guard is a fail-closed executor fence; it is never time-expired. */ +export async function withIssueMutationGuard( + request: IssueMutationRequest, + callback: (tx: Prisma.TransactionClient | undefined, context: IssueMutationContext) => Promise<{ complete: boolean; value: T }>, +): Promise> { + const ownerToken = crypto.randomUUID(); + let created = false; + try { + await prisma.issueMutationGuard.create({ data: { ...request, ownerToken, executing: true } }); + created = true; + } catch (error) { + if (!(error instanceof Prisma.PrismaClientKnownRequestError) && (typeof error !== "object" || error === null || !("code" in error))) throw error; + if ((error as { code?: string }).code !== "P2002") throw error; + } + + let guard = created ? await prisma.issueMutationGuard.findUnique({ where: { issueId: request.issueId } }) : null; + let resuming = false; + if (!created) { + guard = await prisma.issueMutationGuard.findUnique({ where: { issueId: request.issueId } }); + if (!guard) return { status: "busy", ownerToken, operation: request.operation }; + const exactRelease = request.operation === "capacity-release" && guard.operation === request.operation + && request.agentName !== undefined && request.claimToken !== undefined + && guard.agentName === request.agentName && guard.claimToken === request.claimToken; + const exactClaim = request.operation === "claim" && guard.operation === "claim" + && request.agentName !== undefined && guard.agentName === request.agentName; + if ((!exactRelease && !exactClaim) || guard.executing) return { status: "busy", ownerToken: guard.ownerToken, operation: guard.operation }; + const acquired = await prisma.issueMutationGuard.updateMany({ + where: { issueId: request.issueId, ownerToken: guard.ownerToken, executing: false }, data: { executing: true }, + }); + if (!acquired.count) return { status: "busy", ownerToken: guard.ownerToken, operation: guard.operation }; + resuming = true; + } + if (!guard) return { status: "busy", ownerToken, operation: request.operation }; + + try { + const result = await callback(undefined, { resuming }); + if (result.complete) { + await prisma.issueMutationGuard.deleteMany({ where: { issueId: request.issueId, ownerToken: guard.ownerToken, executing: true } }); + return { status: "completed", value: result.value }; + } + await prisma.issueMutationGuard.updateMany({ where: { issueId: request.issueId, ownerToken: guard.ownerToken, executing: true }, data: { executing: false } }); + return { status: "retained", ownerToken: guard.ownerToken, value: result.value }; + } catch (error) { + if (request.operation === "lease" && !resuming) { + await prisma.issueMutationGuard.deleteMany({ where: { issueId: request.issueId, ownerToken: guard.ownerToken, executing: true } }); + } else { + await prisma.issueMutationGuard.updateMany({ where: { issueId: request.issueId, ownerToken: guard.ownerToken, executing: true }, data: { executing: false } }); + } + throw error; + } +} diff --git a/src/lib/lease.test.ts b/src/lib/lease.test.ts index 7bec8a95..821d4fa4 100644 --- a/src/lib/lease.test.ts +++ b/src/lib/lease.test.ts @@ -34,6 +34,13 @@ vi.mock("@/lib/prisma", () => ({ }, })); +vi.mock("@/lib/issue-mutation-guard", () => ({ + withIssueMutationGuard: async (_request: unknown, callback: any) => { + const result = await callback(undefined, { resuming: false }); + return { status: "completed", value: result.value }; + }, +})); + vi.mock("@/lib/next-action", () => ({ buildResumeContext: vi.fn((input) => ({ ...input, nextAction: "inspect_issue" })), isValidCheckpoint: vi.fn((cp) => ["issue_claimed", "branch_created", "changes_made"].includes(cp)), @@ -163,6 +170,19 @@ describe("upsertLease", () => { await expect(upsertLease({ agentName: "saffron", issueId: "i-1" })).rejects.toThrow("connection lost"); expect(mocks.leaseFindUnique).toHaveBeenCalledTimes(1); }); + + it("succeeds when a transient create failure is retried", async () => { + mocks.leaseFindUnique.mockResolvedValue(null); + mocks.leaseCreate + .mockRejectedValueOnce(new Error("connection lost")) + .mockImplementation(async ({ data }: any) => ({ id: "l-1", ...data })); + + await expect(upsertLease({ agentName: "saffron", issueId: "i-1" })).rejects.toThrow("connection lost"); + const retry = await upsertLease({ agentName: "saffron", issueId: "i-1" }); + expect(retry.created).toBe(true); + expect(retry.lease.claimToken).toMatch(/^claim_/); + expect(mocks.leaseCreate).toHaveBeenCalledTimes(2); + }); }); describe("renew path", () => { @@ -408,3 +428,84 @@ describe("findLeasedIssueIds", () => { expect(result).toEqual([]); }); }); + +describe("claim token fencing (#1256)", () => { + it("mints an opaque claim token on lease creation", async () => { + mocks.leaseFindUnique.mockResolvedValue(null); + mocks.leaseCreate.mockImplementation((args: any) => Promise.resolve({ id: "l-1", ...args.data })); + + const result = await upsertLease({ agentName: "saffron", issueId: "i-1" }); + + const token = result.lease.claimToken; + expect(typeof token).toBe("string"); + expect(token.length).toBeGreaterThan(10); + expect(token).toMatch(/^claim_/); + }); + + it("issues a distinct token for every new claim (re-claims are fenced)", async () => { + mocks.leaseFindUnique.mockResolvedValue(null); + mocks.leaseCreate.mockImplementation((args: any) => Promise.resolve({ id: "l-1", ...args.data })); + + await upsertLease({ agentName: "saffron", issueId: "i-1" }); + await upsertLease({ agentName: "saffron", issueId: "i-1" }); + + const [first, second] = mocks.leaseCreate.mock.calls.map((c: any) => c[0].data.claimToken); + expect(first).not.toBe(second); + }); + + it("keeps the claim token stable across renewals (heartbeats do not invalidate it)", async () => { + mocks.leaseFindUnique.mockResolvedValue(makeLease({ id: "l-1", claimToken: "claim_stable" })); + mocks.leaseUpdate.mockImplementation((args: any) => + Promise.resolve({ ...makeLease(args.where), claimToken: "claim_stable", ...args.data }), + ); + + await upsertLease({ agentName: "saffron", issueId: "i-1", checkpoint: "branch_created" }); + + const updateData = (mocks.leaseUpdate.mock.calls[0] as any)[0].data; + // Renewal must NOT mint or rewrite a token — the issued receipt stays valid. + expect(updateData.claimToken).toBeUndefined(); + }); + + it("exposes the claim token on the resume context so a resuming worker keeps its receipt", async () => { + mocks.leaseFindFirst.mockResolvedValue({ + id: "l-1", + agentName: "saffron", + issueId: "i-1", + checkpoint: "issue_claimed", + branch: null, + prUrl: null, + claimToken: "claim_resume-me", + expiredAt: new Date(Date.now() + 60_000), + renewedAt: makeNow(), + createdAt: makeNow(), + issue: { number: 42, repository: { fullName: "misospace/dispatch" } }, + }); + mocks.issueFindUnique.mockResolvedValue({ id: "i-1" }); + + const result = await resolveActiveWork("saffron"); + + expect(result).not.toBeNull(); + expect(result!.claimToken).toBe("claim_resume-me"); + }); + + it("leaves claimToken undefined for legacy token-less leases", async () => { + mocks.leaseFindFirst.mockResolvedValue({ + id: "l-1", + agentName: "saffron", + issueId: "i-1", + checkpoint: "issue_claimed", + branch: null, + prUrl: null, + claimToken: null, + expiredAt: new Date(Date.now() + 60_000), + renewedAt: makeNow(), + createdAt: makeNow(), + issue: { number: 42, repository: { fullName: "misospace/dispatch" } }, + }); + mocks.issueFindUnique.mockResolvedValue({ id: "i-1" }); + + const result = await resolveActiveWork("saffron"); + + expect(result!.claimToken).toBeUndefined(); + }); +}); diff --git a/src/lib/lease.ts b/src/lib/lease.ts index 006b1343..38546919 100644 --- a/src/lib/lease.ts +++ b/src/lib/lease.ts @@ -6,8 +6,10 @@ // cron workers and harnesses resume interrupted work without overlapping // another agent's claimed work. +import { withIssueMutationGuard } from "./issue-mutation-guard"; import { Prisma } from "@prisma/client"; import { prisma } from "@/lib/prisma"; +import { newClaimToken } from "@/lib/capacity-handoff"; import type { CheckpointValue, ResumeContext, ResumeContextWithLease } from "./next-action"; import { buildResumeContext, isValidCheckpoint } from "./next-action"; @@ -17,6 +19,14 @@ import { buildResumeContext, isValidCheckpoint } from "./next-action"; export const DEFAULT_LEASE_TTL_MS = 30 * 60 * 1000; // 30 minutes +/** Expected issue-mutation contention, not a lease-store failure. */ +export class IssueMutationBusyError extends Error { + constructor() { + super("Issue mutation pending; lease cannot be renewed or reissued"); + this.name = "IssueMutationBusyError"; + } +} + /** * Calculate the expiry timestamp for a lease with the given TTL. */ @@ -40,7 +50,16 @@ export async function upsertLease(params: { branch?: string; prUrl?: string; ttlMs?: number; + /** Internal: the claim route already holds the issue mutation guard. */ + mutationGuardHeld?: boolean; }): Promise<{ created: boolean; lease: any }> { + if (!params.mutationGuardHeld) { + const result = await withIssueMutationGuard({ issueId: params.issueId, operation: "lease", agentName: params.agentName }, async () => ({ + complete: true, value: await upsertLease({ ...params, mutationGuardHeld: true }), + })); + if (result.status === "busy") throw new IssueMutationBusyError(); + return result.value; + } const now = new Date(); const checkpoint = params.checkpoint ?? "issue_claimed"; const expiredAt = calculateLeaseExpiry(params.ttlMs); @@ -65,6 +84,12 @@ export async function upsertLease(params: { // @@unique([agentName, issueId]) constraint. The loser hits a P2002 // unique violation — re-read and return the lease the winner created // instead of surfacing a 500 to the caller. + // + // #1256: creation mints the opaque claim token that fences later + // capacity-handoff releases to THIS exact claim. Renewals deliberately keep + // the token (a heartbeating worker must not lose its receipt); a released or + // expired lease that is re-claimed is a NEW row with a NEW token, so old + // tokens are inert against new claims. try { const lease = await prisma.lease.create({ data: { @@ -74,6 +99,7 @@ export async function upsertLease(params: { checkpoint, branch: params.branch, prUrl: params.prUrl, + claimToken: newClaimToken(), }, }); return { created: true, lease }; @@ -214,6 +240,9 @@ export async function resolveActiveWork(agentName: string): Promise): Record void) | null; @@ -75,10 +76,45 @@ function makeClient(): PrFixQueueClient & { } { const items: any[] = []; const history: any[] = []; + // #1256: capacity-scoped cooldown rows. + const handoffs: any[] = []; let seq = 0; const client: any = { items, history, + handoffs, + capacityHandoff: { + create: async ({ data }: any) => { + const row = { id: `handoff-${handoffs.length + 1}`, createdAt: new Date(), ...data }; + handoffs.push(row); + return row; + }, + findMany: async ({ where }: any) => + handoffs.filter( + (h) => + (where?.kind === undefined || h.kind === where.kind) && + (where?.repoFullName === undefined || String(h.repoFullName).toLowerCase() === String(where.repoFullName).toLowerCase()) && + (where?.prNumber === undefined || h.prNumber === where.prNumber) && + (where?.generation === undefined || h.generation === where.generation) && + (where?.agentName === undefined || h.agentName === where.agentName) && + (where?.note?.contains === undefined || h.note?.includes(where.note.contains)) && + (where?.cooldownUntil === undefined || + (where.cooldownUntil.gt != null && new Date(h.cooldownUntil).getTime() > new Date(where.cooldownUntil.gt).getTime())), + ), + deleteMany: async ({ where }: any) => { + const before = handoffs.length; + for (let i = handoffs.length - 1; i >= 0; i--) { + const h = handoffs[i]; + if ( + where?.cooldownUntil?.lte != null && + new Date(h.cooldownUntil).getTime() <= new Date(where.cooldownUntil.lte).getTime() + ) { + handoffs.splice(i, 1); + } + } + return { count: before - handoffs.length }; + }, + }, // #1124: true while a $transaction callback is executing. Tests use it to // prove no network call (e.g. the post-dispatch head fetch) runs inside a // transaction. @@ -127,6 +163,9 @@ function makeClient(): PrFixQueueClient & { // #1211 hand-out ack/reclaim columns: default to their schema values // so a freshly created row reads back as "no ack, never reclaimed". handoutAcks: [], // mirrors the column's @default([]) + agentHandouts: [], // mirrors the column's @default([]) + // #1256 review: per-hand-out receipts (bijection with agentHandouts). + handoutReceipts: [], // mirrors the column's @default([]) handoutReclaims: 0, // mirrors the column's @default(0) retryAt: null, // #1218 outcome classification columns: default to null so a freshly @@ -172,13 +211,21 @@ function makeClient(): PrFixQueueClient & { (where.id === undefined || i.id === where.id) && (where.generation === undefined || i.generation === where.generation) && (where.status === undefined || i.status === where.status) && + (where.dispatchedGeneration === undefined || i.dispatchedGeneration === where.dispatchedGeneration) && (where.retryAt === undefined || (where.retryAt.lte && i.retryAt != null && new Date(i.retryAt).getTime() <= new Date(where.retryAt.lte).getTime()) || i.retryAt === where.retryAt) && (where.postDispatchEvidenceKeys === undefined || JSON.stringify(i.postDispatchEvidenceKeys ?? []) === JSON.stringify(where.postDispatchEvidenceKeys.equals ?? [])) && (where.handoutAcks === undefined || JSON.stringify(i.handoutAcks ?? []) === - JSON.stringify(where.handoutAcks.equals ?? [])), + JSON.stringify(where.handoutAcks.equals ?? [])) && + // #1256 review: receipt/hand-out list pins (exact-owner CAS). + (where.handoutReceipts === undefined || + JSON.stringify(i.handoutReceipts ?? []) === + JSON.stringify(where.handoutReceipts.equals ?? [])) && + (where.agentHandouts === undefined || + JSON.stringify(i.agentHandouts ?? []) === + JSON.stringify(where.agentHandouts.equals ?? [])), ); for (const match of matches) { const idx = items.findIndex((i) => i.id === match.id); @@ -4151,6 +4198,357 @@ describe("PR-fix outcome separation (#1218)", () => { githubPrsMocks.fetchPullRequestMergeState.mockResolvedValue({ mergeableState: null, mergeable: null }); }); + // #1256 review: a valid hand-out is seeded as the live triple the route + // produces — agentHandouts entry + handoutReceipts receipt + stamp. + function seedHandout(row: any, agent: string, generation: number, token: string) { + row.agentHandouts = [...(row.agentHandouts ?? []), `${agent}@${generation}`]; + row.handoutReceipts = [...(row.handoutReceipts ?? []), `${token}@${agent}@${generation}`]; + row.dispatchedGeneration = generation; + row.dispatchedAt = new Date(); + return token; + } + + it("capacity handoff releases only the issued generation without using retry budget or losing PR metadata (#1256)", async () => { + await enqueuePrFixItem(client, { + repo: "org/repo", pr: 300, lane: "NORMAL", reason: "r", feedback: "f", evidenceKey: "k300", + }); + const row = client.items[0]; + row.branch = "work/300"; + row.url = "https://github.com/org/repo/pull/300"; + seedHandout(row, "worker-a", 1, "hnd_aaaaaaaaaaaaaaaa"); + row.postDispatchEvidenceKeys = ["review:o/r#7:r1@H1"]; + const result = await resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 300, + outcome: "capacity-exhausted", capacity: "provider/model", agentName: "worker-a", + attempt: { itemId: row.id, generation: 1, handoutToken: "hnd_aaaaaaaaaaaaaaaa" }, + }); + expect(result.action).toBe("deferred"); + const released = client.items[0]; + // The item STAYS QUEUED at the same generation: no global park, no retry + // budget spent, PR/branch identity retained. + expect(released.status).toBe("QUEUED"); + expect(released.retryAt).toBeNull(); + expect(released.fixAttempts).toBe(1); + expect(released.generation).toBe(1); + expect(released.branch).toBe("work/300"); + expect(released.url).toBe("https://github.com/org/repo/pull/300"); + // The consumed hand-out is cleared so any compatible worker can take it: + // the last live receipt resets the stamps AND the post-dispatch list. + expect(released.agentHandouts).toEqual([]); + expect(released.handoutReceipts).toEqual([]); + expect(released.dispatchedGeneration).toBeNull(); + expect(released.postDispatchEvidenceKeys).toEqual([]); + expect(client.history.at(-1)).toMatchObject({ action: "capacity_handoff", status: "QUEUED", reason: "provider/model" }); + // The suppression is capacity-SCOPED, not a global wait. + expect(client.handoffs).toHaveLength(1); + expect(client.handoffs[0]).toMatchObject({ + kind: "pr_fix", capacity: "provider/model", repoFullName: "org/repo", prNumber: 300, + agentName: "worker-a", + }); + expect(client.handoffs[0].cooldownUntil).toBeInstanceOf(Date); + expect(client.handoffs[0].cooldownUntil.getTime()).toBeGreaterThan(Date.now()); + }); + + it("repeated capacity-exhausted report replays idempotently without minting a second cooldown (#1256)", async () => { + await enqueuePrFixItem(client, { repo: "org/repo", pr: 301, lane: "NORMAL", reason: "r", feedback: "f", evidenceKey: "k301" }); + const row = client.items[0]; + seedHandout(row, "worker-a", 1, "hnd_aaaaaaaaaaaaaaaa"); + const first = await resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 301, + outcome: "capacity-exhausted", capacity: "provider/model", agentName: "worker-a", + attempt: { itemId: row.id, generation: 1, handoutToken: "hnd_aaaaaaaaaaaaaaaa" }, + }); + expect(first.action).toBe("deferred"); + const repeat = await resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 301, + outcome: "capacity-exhausted", capacity: "provider/model", agentName: "worker-a", + attempt: { itemId: row.id, generation: 1, handoutToken: "hnd_aaaaaaaaaaaaaaaa" }, + }); + expect(repeat.action).toBe("deferred"); + expect(client.handoffs).toHaveLength(1); + expect(client.history.filter((h: any) => h.action === "capacity_handoff")).toHaveLength(1); + }); + + it("settles two agents at the same capacity and generation independently; replaying A cannot consume a new receipt (#1256 review)", async () => { + await enqueuePrFixItem(client, { repo: "org/repo", pr: 315, lane: "NORMAL", reason: "r", feedback: "f", evidenceKey: "k315" }); + const row = client.items[0]; + seedHandout(row, "worker-a", 1, "hnd_aaaaaaaaaaaaaaaa"); + row.agentHandouts.push("worker-b@1"); + row.handoutReceipts.push("hnd_bbbbbbbbbbbbbbbb@worker-b@1"); + const report = (token: string, agentName = "worker-a") => resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 315, + outcome: "capacity-exhausted", capacity: "same-capacity", agentName, + attempt: { itemId: row.id, generation: 1, handoutToken: token }, + }); + expect((await report("hnd_aaaaaaaaaaaaaaaa")).action).toBe("deferred"); + expect(client.items[0].handoutReceipts).toEqual(["hnd_bbbbbbbbbbbbbbbb@worker-b@1"]); + expect((await report("hnd_bbbbbbbbbbbbbbbb", "worker-b")).action).toBe("deferred"); + expect(client.items[0].handoutReceipts).toEqual([]); + expect(client.items[0].agentHandouts).toEqual([]); + expect(client.history.filter((h: any) => h.action === "capacity_handoff")).toHaveLength(2); + expect((await report("hnd_aaaaaaaaaaaaaaaa")).action).toBe("deferred"); + expect(client.handoffs).toHaveLength(2); + + // A new receipt at the same generation is not consumed by A's replay. + seedHandout(client.items[0], "worker-a", 1, "hnd_cccccccccccccccc"); + expect((await report("hnd_aaaaaaaaaaaaaaaa")).action).toBe("deferred"); + expect(client.items[0].handoutReceipts).toEqual(["hnd_cccccccccccccccc@worker-a@1"]); + expect(client.items[0].dispatchedGeneration).toBe(1); + }); + + it("legacy receiptless hand-outs keep their dispatch stamps when a receipt owner settles", async () => { + await enqueuePrFixItem(client, { repo: "org/repo", pr: 316, lane: "NORMAL", reason: "r", feedback: "f", evidenceKey: "k316" }); + const row = client.items[0]; + seedHandout(row, "worker-a", 1, "hnd_aaaaaaaaaaaaaaaa"); + row.agentHandouts.push("worker-legacy@1"); + const result = await resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 316, + outcome: "capacity-exhausted", capacity: "model-a", agentName: "worker-a", + attempt: { itemId: row.id, generation: 1, handoutToken: "hnd_aaaaaaaaaaaaaaaa" }, + }); + expect(result.action).toBe("deferred"); + expect(client.items[0].agentHandouts).toEqual(["worker-legacy@1"]); + expect(client.items[0].handoutReceipts).toEqual([]); + expect(client.items[0].dispatchedGeneration).toBe(1); + expect(client.items[0].dispatchedAt).toBeInstanceOf(Date); + }); + + it("a different capacity exhausting the same live generation records its own scoped cooldown (#1256)", async () => { + await enqueuePrFixItem(client, { repo: "org/repo", pr: 305, lane: "NORMAL", reason: "r", feedback: "f", evidenceKey: "k305" }); + const row = client.items[0]; + // Capacity A already handed this generation off (its receipt was consumed); + // capacity B was then dispatched at the same generation with its own + // receipt and exhausts too. Capacity is caller-declared trusted scoping: + // settlement is authorized by the RECEIPT, never by agent identity matching. + seedHandout(row, "worker-b", 1, "hnd_bbbbbbbbbbbbbbbb"); + client.handoffs.push({ + id: "handoff-a", kind: "pr_fix", capacity: "model-a", repoFullName: "org/repo", + prNumber: 305, generation: 1, cooldownUntil: new Date(Date.now() + 600_000), + }); + const result = await resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 305, + outcome: "capacity-exhausted", capacity: "model-b", agentName: "worker-b", + attempt: { itemId: row.id, generation: 1, handoutToken: "hnd_bbbbbbbbbbbbbbbb" }, + }); + expect(result.action).toBe("deferred"); + expect(client.handoffs).toHaveLength(2); + expect(client.handoffs[1]).toMatchObject({ capacity: "model-b", generation: 1 }); + }); + + it("a duplicate concurrent report loses the hand-out-stamp CAS (#1256)", async () => { + await enqueuePrFixItem(client, { repo: "org/repo", pr: 306, lane: "NORMAL", reason: "r", feedback: "f", evidenceKey: "k306" }); + const row = client.items[0]; + // The duplicate that loses arrives after the winner consumed the issued + // hand-out stamp (dispatchedGeneration cleared) but before the winner's + // cooldown row became visible: the stamp-pinned CAS is the second fence. + seedHandout(row, "worker-a", 1, "hnd_aaaaaaaaaaaaaaaa"); + row.dispatchedGeneration = null; + row.dispatchedAt = null; + const result = await resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 306, + outcome: "capacity-exhausted", capacity: "model-a", agentName: "worker-a", + attempt: { itemId: row.id, generation: 1, handoutToken: "hnd_aaaaaaaaaaaaaaaa" }, + }); + expect(result.action).toBe("skipped"); + expect(client.handoffs).toHaveLength(0); + expect(client.history.filter((h: any) => h.action === "capacity_handoff")).toHaveLength(0); + }); + + it("capacity handoff loses a generation race with zero mutation (#1256)", async () => { + await enqueuePrFixItem(client, { repo: "org/repo", pr: 302, lane: "NORMAL", reason: "r", feedback: "f", evidenceKey: "k302" }); + const row = client.items[0]; + seedHandout(row, "worker-a", 1, "hnd_aaaaaaaaaaaaaaaa"); + // The generation moves inside the read→write gap (fresh attempt re-issued + // concurrently): the CAS must miss and write nothing. + client.hooks.beforeUpdateMany = () => { + const idx = client.items.findIndex((i: any) => i.id === row.id); + client.items[idx] = { ...client.items[idx], generation: 2 }; + }; + const result = await resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 302, + outcome: "capacity-exhausted", capacity: "provider/model", agentName: "worker-a", + attempt: { itemId: row.id, generation: 1, handoutToken: "hnd_aaaaaaaaaaaaaaaa" }, + }); + expect(result.action).toBe("skipped"); + expect(client.handoffs).toHaveLength(0); + expect(client.history.filter((h: any) => h.action === "capacity_handoff")).toHaveLength(0); + // The newer generation's dispatch tracking is untouched. + expect(client.items[0].agentHandouts).toEqual(["worker-a@1"]); + expect(client.items[0].handoutReceipts).toEqual(["hnd_aaaaaaaaaaaaaaaa@worker-a@1"]); + expect(client.items[0].dispatchedGeneration).toBe(1); + }); + + it("an unrelated agent cannot release the current hand-out even with the live (id, generation) (#1256 review)", async () => { + await enqueuePrFixItem(client, { repo: "org/repo", pr: 310, lane: "NORMAL", reason: "r", feedback: "f", evidenceKey: "k310" }); + const row = client.items[0]; + seedHandout(row, "worker-b", 1, "hnd_bbbbbbbbbbbbbbbb"); + row.handoutAcks = ["worker-b@1"]; + const before = { ...client.items[0] }; + // A snooping agent knows the (id, generation) token from the queue but + // holds no receipt: the exact-owner CAS refuses with zero mutation. + const result = await resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 310, + outcome: "capacity-exhausted", capacity: "model-x", agentName: "worker-snoop", + attempt: { itemId: row.id, generation: 1, handoutToken: "hnd_ffffffffffffffff" }, + }); + expect(result.action).toBe("skipped"); + expect(result.reason).toMatch(/no live hand-out receipt/); + expect(client.items[0]).toEqual(before); + expect(client.handoffs).toHaveLength(0); + expect(client.history.filter((h: any) => h.action === "capacity_handoff")).toHaveLength(0); + }); + + it("a stale token from a PREVIOUS same-agent hand-out of the SAME generation cannot release the live one (#1256 review)", async () => { + await enqueuePrFixItem(client, { repo: "org/repo", pr: 311, lane: "NORMAL", reason: "r", feedback: "f", evidenceKey: "k311" }); + const row = client.items[0]; + // worker-a's first hand-out was released (its receipt consumed with it, + // per the receipt↔hand-out bijection) and worker-a was re-handed the SAME + // generation with a rotated receipt. The late report still carries the + // old token. + seedHandout(row, "worker-a", 1, "hnd_111111111111111a"); + const before = { ...client.items[0] }; + const result = await resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 311, + outcome: "capacity-exhausted", capacity: "model-a", agentName: "worker-a", + attempt: { itemId: row.id, generation: 1, handoutToken: "hnd_000000000000000a" }, + }); + expect(result.action).toBe("skipped"); + // The live rotated receipt and the dispatch stamps survive. + expect(client.items[0]).toEqual(before); + expect(client.handoffs).toHaveLength(0); + }); + + it("preserves other agents' live receipts, acks, and the dispatch stamps (#1256 review)", async () => { + await enqueuePrFixItem(client, { repo: "org/repo", pr: 312, lane: "NORMAL", reason: "r", feedback: "f", evidenceKey: "k312" }); + const row = client.items[0]; + seedHandout(row, "worker-a", 1, "hnd_aaaaaaaaaaaaaaaa"); + row.agentHandouts.push("worker-b@1"); + row.handoutReceipts.push("hnd_bbbbbbbbbbbbbbbb@worker-b@1"); + row.handoutAcks = ["worker-b@1"]; + row.postDispatchEvidenceKeys = ["review:o/r#7:r1@H1"]; + const result = await resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 312, + outcome: "capacity-exhausted", capacity: "model-a", agentName: "worker-a", + attempt: { itemId: row.id, generation: 1, handoutToken: "hnd_aaaaaaaaaaaaaaaa" }, + }); + expect(result.action).toBe("deferred"); + const after = client.items[0]; + // Only worker-a's receipt/hand-out/ack were removed; worker-b's live + // attempt survives intact. + expect(after.agentHandouts).toEqual(["worker-b@1"]); + expect(after.handoutReceipts).toEqual(["hnd_bbbbbbbbbbbbbbbb@worker-b@1"]); + expect(after.handoutAcks).toEqual(["worker-b@1"]); + // Another hand-out is still live at this generation: the stamps and the + // post-dispatch evidence list MUST survive (clearing them would + // reclassify worker-b's mid-run evidence as pre-dispatch, #1119). + expect(after.dispatchedGeneration).toBe(1); + expect(after.postDispatchEvidenceKeys).toEqual(["review:o/r#7:r1@H1"]); + expect(client.handoffs).toHaveLength(1); + }); + + it("a hand-out racing into the read→write gap pins the CAS to a no-op (#1256 review)", async () => { + await enqueuePrFixItem(client, { repo: "org/repo", pr: 313, lane: "NORMAL", reason: "r", feedback: "f", evidenceKey: "k313" }); + const row = client.items[0]; + seedHandout(row, "worker-a", 1, "hnd_aaaaaaaaaaaaaaaa"); + // A concurrent next-task hand-out to worker-c lands right before the + // settlement write: the pinned receipt/hand-out lists miss. + client.hooks.beforeUpdateMany = () => { + const idx = client.items.findIndex((i: any) => i.id === row.id); + client.items[idx] = { + ...client.items[idx], + agentHandouts: [...client.items[idx].agentHandouts, "worker-c@1"], + handoutReceipts: [...client.items[idx].handoutReceipts, "hnd_cccccccccccccccc@worker-c@1"], + }; + }; + const result = await resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 313, + outcome: "capacity-exhausted", capacity: "model-a", agentName: "worker-a", + attempt: { itemId: row.id, generation: 1, handoutToken: "hnd_aaaaaaaaaaaaaaaa" }, + }); + expect(result.action).toBe("skipped"); + const after = client.items[0]; + expect(after.agentHandouts).toEqual(["worker-a@1", "worker-c@1"]); + expect(after.handoutReceipts).toEqual([ + "hnd_aaaaaaaaaaaaaaaa@worker-a@1", + "hnd_cccccccccccccccc@worker-c@1", + ]); + expect(after.dispatchedGeneration).toBe(1); + expect(client.handoffs).toHaveLength(0); + }); + + it("a pre-receipt (legacy) hand-out cannot settle capacity-exhausted (#1256 review)", async () => { + await enqueuePrFixItem(client, { repo: "org/repo", pr: 314, lane: "NORMAL", reason: "r", feedback: "f", evidenceKey: "k314" }); + const row = client.items[0]; + // A hand-out issued before receipts existed: record + stamp, no receipt. + row.agentHandouts = ["worker-a@1"]; + row.dispatchedGeneration = 1; + row.dispatchedAt = new Date(); + const result = await resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 314, + outcome: "capacity-exhausted", capacity: "model-a", agentName: "worker-a", + attempt: { itemId: row.id, generation: 1, handoutToken: "hnd_aaaaaaaaaaaaaaaa" }, + }); + expect(result.action).toBe("skipped"); + expect(client.items[0].agentHandouts).toEqual(["worker-a@1"]); + expect(client.items[0].dispatchedGeneration).toBe(1); + expect(client.handoffs).toHaveLength(0); + }); + + it("capacity handoff refuses a stale generation before any write (#1256)", async () => { + await enqueuePrFixItem(client, { repo: "org/repo", pr: 303, lane: "NORMAL", reason: "r", feedback: "f", evidenceKey: "k303" }); + const row = client.items[0]; + row.generation = 2; // a fresh attempt already exists + const result = await resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 303, + outcome: "capacity-exhausted", capacity: "provider/model", + attempt: { itemId: row.id, generation: 1 }, + }); + expect(result.action).toBe("skipped"); + expect(client.handoffs).toHaveLength(0); + }); + + it("does not overwrite a human hold on capacity handoff (#1256)", async () => { + await enqueuePrFixItem(client, { repo: "org/repo", pr: 299, lane: "NORMAL", reason: "r", feedback: "f", evidenceKey: "k299" }); + const row = client.items[0]; + row.status = "BLOCKED"; + row.lane = "NEEDS_HUMAN"; + const result = await resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 299, + outcome: "capacity-exhausted", capacity: "provider/model", + attempt: { itemId: row.id, generation: 1 }, + }); + expect(result.action).toBe("skipped"); + expect(row.status).toBe("BLOCKED"); + expect(row.lane).toBe("NEEDS_HUMAN"); + expect(client.handoffs).toHaveLength(0); + expect(client.history).toHaveLength(1); + }); + + it("capacity handoff requires an explicit capacity and an issued attempt token (#1256)", async () => { + await enqueuePrFixItem(client, { repo: "org/repo", pr: 304, lane: "NORMAL", reason: "r", feedback: "f", evidenceKey: "k304" }); + const row = client.items[0]; + const noCapacity = await resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 304, + outcome: "capacity-exhausted", attempt: { itemId: row.id, generation: 1 }, + }); + expect(noCapacity.action).toBe("skipped"); + const legacy = await resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 304, + outcome: "capacity-exhausted", capacity: "provider/model", attempt: null, + }); + expect(legacy.action).toBe("skipped"); + // #1256 review: the receipt token itself is required — (id, generation) + // alone identifies the item, not the exact hand-out. + const noReceiptToken = await resolvePrFixFromAgentReport({ + client: client as PrFixQueueClient, repoFullName: "org/repo", pullRequestNumber: 304, + outcome: "capacity-exhausted", capacity: "provider/model", agentName: "worker-a", + attempt: { itemId: row.id, generation: 1 }, + }); + expect(noReceiptToken.action).toBe("skipped"); + expect(client.handoffs).toHaveLength(0); + expect(client.items[0].status).toBe("QUEUED"); + }); + it("parks a failed report past the fix-attempt cap and never labels it human", async () => { const prev = process.env.PR_FIX_MAX_ATTEMPTS; process.env.PR_FIX_MAX_ATTEMPTS = "1"; diff --git a/src/lib/pr-fix-queue.ts b/src/lib/pr-fix-queue.ts index e35a0715..5a41043f 100644 --- a/src/lib/pr-fix-queue.ts +++ b/src/lib/pr-fix-queue.ts @@ -1,7 +1,9 @@ +import { randomBytes } from "node:crypto"; import { normalizePrFixLane, normalizePrFixStatus, normalizePrFixType, normalizePrFixOutcome, PrFixLane, PrFixOutcome, PrFixStatus, PrFixType, PrFixDecisionContext, PR_FIX_TYPE_PRIORITY } from "@/types"; import { surfacePrFixBlocked, surfacePrFixParked, surfacePrFixRequeued, surfacePrFixUnblocked, extractUrlsFromText, type PrFixUnblockOutcome } from "./pr-fix-surfacing"; import { prisma } from "@/lib/prisma"; import { fetchPullRequestMergeState, fetchPullRequestHeadSha, fetchPullRequestState } from "./github-prs"; +import { capacityCooldownMs, capacityEquals } from "./capacity-handoff"; import { fetchRepositoryMetadata } from "./github-code-search"; import { sanitizeForStorage } from "./groomer/sanitize"; @@ -20,6 +22,13 @@ export type PrFixQueueClient = { create: (args: any) => Promise; findMany?: (args: any) => Promise; }; + // #1256: capacity-scoped cooldown rows. Optional so legacy/test clients + // without the table still settle other outcomes; capacity-exhausted + // settlement records the cooldown whenever the surface exists. + capacityHandoff?: { + create: (args: any) => Promise; + findMany?: (args: any) => Promise; + }; $transaction: (fn: (tx: PrFixQueueClient) => Promise) => Promise; }; @@ -562,7 +571,7 @@ async function reactivateDueExternalWaits(client: PrFixQueueClient, now: Date): await client.$transaction(async (tx) => { const { count } = await tx.prFixQueueItem.updateMany({ where: { id: row.id, status: "WAITING_EXTERNAL", generation: row.generation, retryAt: row.retryAt }, - data: { status: "QUEUED", ...freshAttemptGeneration(), retryAt: null, dispatchedGeneration: null, dispatchedAt: null, postDispatchEvidenceKeys: [], attemptHeadSha: row.headSha ?? row.attemptHeadSha ?? null, outcome: null, decisionDetail: null, agentHandouts: [], handoutAcks: [] }, + data: { status: "QUEUED", ...freshAttemptGeneration(), retryAt: null, dispatchedGeneration: null, dispatchedAt: null, postDispatchEvidenceKeys: [], attemptHeadSha: row.headSha ?? row.attemptHeadSha ?? null, outcome: null, decisionDetail: null, agentHandouts: [], handoutAcks: [], handoutReceipts: [] }, }); if (count === 1) await tx.prFixHistory.create({ data: { itemId: row.id, action: "mark", status: "QUEUED", lane: row.lane, note: "External-wait cooldown elapsed; retrying prerequisite check." } }); }); @@ -912,6 +921,7 @@ export async function reclaimStalePrFixHandouts( status: "STALE", agentHandouts: [], handoutAcks: [], + handoutReceipts: [], dispatchedGeneration: null, dispatchedAt: null, }, @@ -958,6 +968,7 @@ export async function reclaimStalePrFixHandouts( decisionDetail: null, agentHandouts: [], handoutAcks: [], + handoutReceipts: [], dispatchedGeneration: null, dispatchedAt: null, }, @@ -1026,6 +1037,7 @@ export async function reclaimStalePrFixHandouts( handoutReclaims: { increment: 1 }, agentHandouts: [], handoutAcks: [], + handoutReceipts: [], postDispatchEvidenceKeys: [], dispatchedGeneration: null, dispatchedAt: null, @@ -1240,7 +1252,7 @@ export async function enqueuePrFixItem(client: PrFixQueueClient, rawInput: Enque // way, and the skip check must match only the current identity // (#1119, #1133, #1211). ...(isFreshAttempt - ? { postDispatchEvidenceKeys: [], agentHandouts: [], handoutAcks: [] } + ? { postDispatchEvidenceKeys: [], agentHandouts: [], handoutAcks: [], handoutReceipts: [] } : {}), // Structured outcome (#1218): recorded on the same write as the // status. Only BLOCKED/PARKED carry one; decisionDetail is persisted @@ -1618,6 +1630,7 @@ export async function markPrFixItem( data.dispatchedGeneration = null; data.dispatchedAt = null; data.agentHandouts = []; + data.handoutReceipts = []; data.handoutAcks = []; } @@ -1688,6 +1701,7 @@ export async function markPrFixItem( postDispatchEvidenceKeys: [], agentHandouts: [], handoutAcks: [], + handoutReceipts: [], } : { status: "QUEUED", @@ -1703,6 +1717,7 @@ export async function markPrFixItem( // prior generation's acknowledgements are dead (#1211). agentHandouts: [], handoutAcks: [], + handoutReceipts: [], }; const { count } = await tx.prFixQueueItem.updateMany({ where: { @@ -1806,6 +1821,7 @@ export async function markPrFixItem( // match must not survive the identity change (#1133). data.postDispatchEvidenceKeys = []; data.agentHandouts = []; + data.handoutReceipts = []; data.handoutAcks = []; } // QUEUED → QUEUED: leave the key list untouched — the entries still refer @@ -1851,6 +1867,7 @@ export async function markPrFixItem( postDispatchEvidenceKeys: [], agentHandouts: [], handoutAcks: [], + handoutReceipts: [], } : { status: "QUEUED", @@ -1867,6 +1884,7 @@ export async function markPrFixItem( // acknowledgements (#1133, #1211). agentHandouts: [], handoutAcks: [], + handoutReceipts: [], }; const refusal = await client.$transaction(async (tx) => { const { count } = await tx.prFixQueueItem.updateMany({ @@ -2134,6 +2152,7 @@ export async function markPrFixItem( postDispatchEvidenceKeys: [], agentHandouts: [], handoutAcks: [], + handoutReceipts: [], } : { status: "QUEUED", @@ -2149,6 +2168,7 @@ export async function markPrFixItem( // prior generation's acknowledgements are dead (#1211). agentHandouts: [], handoutAcks: [], + handoutReceipts: [], }; const forced = await client.$transaction(async (tx) => { const { count } = await tx.prFixQueueItem.updateMany({ @@ -2351,6 +2371,78 @@ export function parseAgentHandoutToken(entry: unknown): { agentName: string; gen return { agentName: entry.slice(0, idx), generation }; } +// ── Per-hand-out receipts (#1256 review) ─────────────────────────────────── +// +// `agentHandouts` answers "was this generation already handed to agent X", +// which is all ordinary dispatch needs. Capacity-exhausted settlement needs +// more: it must authorize the EXACT hand-out attempt a worker was issued, so +// that (a) an unrelated agent that merely knows the (id, generation) token can +// never release the current hand-out, and (b) a stale token from a PREVIOUS +// hand-out of the SAME agent at the SAME generation (release/reclaim keep the +// generation) cannot release the live one. Generation alone cannot express +// either — the receipt carries a fresh opaque token per hand-out. +// +// Receipt entry format: `@@`. The token is +// Dispatch-minted hex (never contains "@"), so the token boundary is the +// FIRST "@"; the agent name may contain "@", so its boundary with the +// generation stays the LAST "@" (same rule as `agentHandoutToken`). +// +// Invariant (enforced at every write site below): a receipt exists exactly +// while the matching `agentHandouts` entry exists — issuance pushes both in +// one conditional write, every site that clears `agentHandouts` clears +// `handoutReceipts` in the same data object. A released/reclaimed/ +// superseded hand-out therefore has no live receipt, and every NEW hand-out +// rotates in a fresh token. + +/** Fresh opaque per-hand-out token. Opaque to everyone but the matcher. */ +export function newHandoutToken(): string { + return `hnd_${randomBytes(12).toString("hex")}`; +} + +/** Encoding of a persisted hand-out owner receipt. */ +export function handoutReceipt(token: string, agentName: string, generation: number): string { + return `${token}@${agentName}@${generation}`; +} + +export function parseHandoutReceipt( + entry: unknown, +): { token: string; agentName: string; generation: number } | null { + if (typeof entry !== "string") return null; + const first = entry.indexOf("@"); + const last = entry.lastIndexOf("@"); + // Token and agent name must both be non-empty; token shape is Dispatch's. + if (first <= 0 || last <= first) return null; + const token = entry.slice(0, first); + if (!/^hnd_[0-9a-f]{8,}$/.test(token)) return null; + const generation = Number(entry.slice(last + 1)); + if (!Number.isInteger(generation) || generation < 1) return null; + return { token, agentName: entry.slice(first + 1, last), generation }; +} + +/** + * The receipt in `entries` matching the exact (agent, generation, token) + * triple, or null. All three must match: the agent binds the source owner, + * the token binds the exact hand-out issuance, the generation is defense in + * depth against a cleared-then-reissued entry. + */ +export function findHandoutReceipt( + entries: readonly unknown[], + owner: { agentName: string; generation: number; token: string }, +): string | null { + for (const entry of entries) { + const parsed = parseHandoutReceipt(entry); + if ( + parsed && + parsed.agentName === owner.agentName && + parsed.generation === owner.generation && + parsed.token === owner.token + ) { + return entry as string; + } + } + return null; +} + /** * Whether this item was already handed to `agentName` at its CURRENT * generation (#1133). Entries for older generations and malformed entries @@ -2461,6 +2553,7 @@ export async function requeuePrFixItem(client: PrFixQueueClient, rawInput: Reque postDispatchEvidenceKeys: [], agentHandouts: [], handoutAcks: [], + handoutReceipts: [], handoutReclaims: 0, }, }); @@ -2517,7 +2610,8 @@ export type AgentReportOutcome = | "waiting-external" // #1143: an issue close, not a PR-fix settlement; never touches the queue. | "already_done" - | "handed-off"; + | "handed-off" + | "capacity-exhausted"; /** * The (id, generation) attempt token `next-task` issues on a followup-pr @@ -2526,6 +2620,13 @@ export type AgentReportOutcome = export interface PrFixReportAttempt { itemId: string; generation: number; + /** + * #1256: the per-hand-out receipt token next-task issued with this + * hand-out. REQUIRED by `capacity-exhausted` settlement (exact source- + * attempt fencing); ignored by every other outcome, so legacy reports and + * token-less hand-outs settle exactly as before. + */ + handoutToken?: string | null; } export interface ResolvePrFixFromAgentReportInput { @@ -2542,6 +2643,13 @@ export interface ResolvePrFixFromAgentReportInput { // give-up) marks PARKED — a bare give-up is NOT a human decision. blockReason?: string | null; decision?: PrFixDecisionContext | null; + // #1256: opaque worker-declared capacity identifier, required (with an + // attempt token) by the `capacity-exhausted` outcome. + capacity?: string | null; + // #1256: reporting agent — recorded on the capacity-handoff history row and, + // for `capacity-exhausted`, part of the exact-owner CAS (the presented + // handoutToken must belong to THIS agent's live receipt). + agentName?: string | null; client?: PrFixQueueClient; // #1074: required for queue settlement. When present, the report settles // exactly the item + generation the token identifies; when absent (legacy @@ -2713,6 +2821,132 @@ export async function resolvePrFixFromAgentReport( const expectedGeneration = attempt?.generation ?? undefined; + // #1256: explicit capacity exhaustion on a handed PR-fix attempt. + // + // Semantics (deliberately NOT a WAITING_EXTERNAL park): the item STAYS + // QUEUED at the same generation with the same fixAttempts count, branch, and + // PR identity — the attempt token is consumed (hand-out stamps cleared so + // any compatible worker can take it) but no repair budget is spent, and no + // global wait hides the item from workers whose capacity is unrelated. The + // suppression is scoped to the exhausted capacity via a CapacityHandoff + // cooldown row that next-task applies when the requester declares a + // capacity (workers declaring none skip conservatively until expiry). + // + // Fencing (#1256 review): the release is authorized by the EXACT source + // attempt — the reporting agent plus the per-hand-out receipt token + // next-task issued with the hand-out (`prFixItem.handoutToken`). An + // unrelated agent that merely knows (id, generation), and the same agent + // presenting a token from a PREVIOUS hand-out of the same generation + // (release keeps the generation), are both refused with zero mutation. The + // write pins the full hand-out state (receipts/hand-outs/acks) and removes + // ONLY the reporting agent's receipt, hand-out entry, and own ack; other + // agents' live hand-outs and acknowledgements at the same generation are + // preserved, and the dispatch stamps survive while any other hand-out is + // still live. A generation race (fresh attempt re-issued meanwhile) or a + // human hold (BLOCKED/PARKED/NEEDS_HUMAN — the non-QUEUED gate above) + // lands as count=0 → skipped with zero mutation. + if (input.outcome === "capacity-exhausted") { + const capacity = input.capacity?.trim(); + const handoutToken = attempt?.handoutToken?.trim(); + const reportingAgent = input.agentName?.trim(); + if (expectedGeneration === undefined || !capacity) { + return { matched: true, action: "skipped", itemId: existing.id ?? null, reason: "capacity-exhausted requires a capacity identifier and prFixItem attempt token" }; + } + if (!handoutToken || !reportingAgent) { + return { matched: true, action: "skipped", itemId: existing.id ?? null, reason: "capacity-exhausted requires the issued prFixItem handoutToken receipt (exact source-attempt fencing)" }; + } + const cooldownUntil = new Date(Date.now() + capacityCooldownMs()); + const queueClient = client as PrFixQueueClient; + const result = await queueClient.$transaction(async (tx) => { + // Read the live hand-out set and pin it below. The CAS still pins the + // ISSUED hand-out stamp (dispatchedGeneration = the generation + // next-task stamped): exactly one concurrent report can consume the + // stamps, so two racing duplicates cannot both mint a cooldown. + const live = await tx.prFixQueueItem.findUnique({ where: { id: existing.id } }); + if (!live) return "stale" as const; + if ( + live.status !== "QUEUED" || + live.generation !== expectedGeneration || + live.dispatchedGeneration !== expectedGeneration + ) { + const prior = await tx.capacityHandoff?.findMany?.({ + where: { kind: "pr_fix", repoFullName: repo, prNumber: pr, generation: expectedGeneration, agentName: reportingAgent, note: { contains: `receipt=${handoutReceipt(handoutToken, reportingAgent, expectedGeneration)};` } }, + }) ?? []; + return prior.length ? "replayed" as const : "stale" as const; + } + const receipts: string[] = Array.isArray(live.handoutReceipts) ? live.handoutReceipts : []; + const handouts: string[] = Array.isArray(live.agentHandouts) ? live.agentHandouts : []; + const acks: string[] = Array.isArray(live.handoutAcks) ? live.handoutAcks : []; + // Exact source-owner authorization: agent + generation + issued token + // must all match a live receipt. An unknown or rotated token (the + // hand-out was already released, reclaimed, or re-issued to this same + // agent at this same generation) has no authority over the current one. + const owned = findHandoutReceipt(receipts, { + agentName: reportingAgent, + generation: expectedGeneration, + token: handoutToken, + }); + if (!owned) { + const prior = await tx.capacityHandoff?.findMany?.({ + where: { kind: "pr_fix", repoFullName: repo, prNumber: pr, generation: expectedGeneration, agentName: reportingAgent, note: { contains: `receipt=${handoutReceipt(handoutToken, reportingAgent, expectedGeneration)};` } }, + }) ?? []; + return prior.length ? "replayed" as const : "unauthorized" as const; + } + const remainingReceipts = receipts.filter((r) => r !== owned); + const ownEntry = agentHandoutToken(reportingAgent, expectedGeneration); + const remainingHandouts = handouts.filter((h) => h !== ownEntry); + const remainingAcks = acks.filter((a) => a !== ownEntry); + // The dispatch stamps survive while ANY other agent's hand-out at this + // generation is still live (#1119: clearing them would reclassify + // mid-run evidence as pre-dispatch for the surviving workers). Only the + // report that consumes the last live hand-out resets the stamps and the + // post-dispatch evidence list, making the generation freshly + // dispatchable. + // Legacy hand-outs may predate receipt tokens; handouts remain the + // authoritative liveness set until all agents at this generation settle. + const lastHandout = remainingHandouts.length === 0; + const { count } = await tx.prFixQueueItem.updateMany({ + where: { + id: live.id, + status: "QUEUED", + generation: expectedGeneration, + dispatchedGeneration: expectedGeneration, + // Receipt/hand-out/ack pins: a concurrent hand-out, ack, or reclaim + // landing in the read→write gap makes this a no-op. + handoutReceipts: { equals: receipts }, + agentHandouts: { equals: handouts }, + handoutAcks: { equals: acks }, + }, + data: { + handoutReceipts: remainingReceipts, + agentHandouts: remainingHandouts, + handoutAcks: remainingAcks, + ...(lastHandout + ? { postDispatchEvidenceKeys: [], dispatchedGeneration: null, dispatchedAt: null } + : {}), + }, + }); + if (count !== 1) return "stale" as const; + await tx.prFixHistory.create({ data: { itemId: existing.id, action: "capacity_handoff", status: "QUEUED", lane: existing.lane, reason: capacity, note: `Capacity exhausted: ${capacity}; attempt generation ${expectedGeneration} released by ${reportingAgent} without consuming retry budget; item stays QUEUED and discoverable to compatible capacities.${remainingReceipts.length > 0 ? ` ${remainingReceipts.length} other live hand-out(s) at this generation preserved (dispatch stamps retained).` : ""}` } }); + if (tx.capacityHandoff) { + await tx.capacityHandoff.create({ data: { kind: "pr_fix", capacity, repoFullName: repo, prNumber: pr, issueNumber: existing.issue ?? null, agentName: reportingAgent, generation: expectedGeneration, cooldownUntil, note: `PR-fix attempt generation ${expectedGeneration} handed off by capacity report; receipt=${handoutReceipt(handoutToken, reportingAgent, expectedGeneration)};` } }); + } + return true; + }); + return { + matched: true, + action: result === true || result === "replayed" ? "deferred" : "skipped", + itemId: existing.id ?? null, + reason: + result === true + ? `capacity ${capacity} exhausted; attempt released with a capacity-scoped cooldown until ${cooldownUntil.toISOString()}` + : result === "unauthorized" + ? `capacity handoff refused: no live hand-out receipt for ${reportingAgent} at generation ${expectedGeneration} matches the presented handoutToken (already released, reclaimed, or rotated by a newer hand-out); the queue was not modified` + : "capacity handoff lost a status/generation race", + attemptGeneration: expectedGeneration, + }; + } + if (input.outcome === "waiting-external") { if (expectedGeneration === undefined) { return { matched: true, action: "skipped", itemId: existing.id ?? null, reason: "waiting-external requires a prFixItem attempt token" }; @@ -2809,6 +3043,7 @@ export async function resolvePrFixFromAgentReport( postDispatchEvidenceKeys: [], agentHandouts: [], handoutAcks: [], + handoutReceipts: [], } : { status: "QUEUED", @@ -2824,6 +3059,7 @@ export async function resolvePrFixFromAgentReport( // acknowledgements are dead (#1211). agentHandouts: [], handoutAcks: [], + handoutReceipts: [], }; // The head-snapshot pin guards the fresh-attempt branch's baseline on // EVERY pass; the capped BLOCKED branch carries no baseline, so it diff --git a/src/lib/prisma.ts b/src/lib/prisma.ts index be7f143d..647594fd 100644 --- a/src/lib/prisma.ts +++ b/src/lib/prisma.ts @@ -136,6 +136,7 @@ export function asPrFixQueueClient(client: PrismaClient): PrFixQueueClient { issue: client.issue, prFixQueueItem: client.prFixQueueItem, prFixHistory: client.prFixHistory, + capacityHandoff: client.capacityHandoff, $transaction: (fn: (tx: PrFixQueueClient) => Promise): Promise => client.$transaction(fn as (tx: Prisma.TransactionClient) => Promise), };