[itsmiso-ai]

Expose Dispatch-owned, persistent work identity for PR-fix follow-ups through next-task, and make tasks/report safely retryable across distributed failure boundaries.

Required by misospace/courier#71. Consumers treat the identities opaquely; this PR does not change queue ranking, issue selection, lease semantics, or retry limits.

Part 1 — PR-fix work generation

GET /api/agents/{agentName}/next-task returns, for followup-pr tasks backed by a PrFixQueueItem:

{
  "type": "followup-pr",
  "prFixItem": { "id": "cktz…", "generation": 2 }
}

• prFixItem.id — stable for the persistent PR-fix queue item.
• prFixItem.generation — integer, starts at 1, changes only when Dispatch creates a fresh dispatchable attempt.

Linked-PR follow-ups that are not backed by a queue item omit prFixItem (tracked separately in #1045). Consumers must not derive queue policy from the number. /queue surfaces generation on each pr-review-fix item as well.

Generation semantics

Generation bumps — atomically with the status flip, via Prisma increment — exactly when a non-QUEUED item becomes dispatchable again as a fresh attempt:

• explicit requeuePrFixItem from BLOCKED or FIXED;
• genuinely new evidence reopening a resolved item (enqueuePrFixItem);
• the [P2] A pr-fix item marked FIXED on workload exit alone strands the PR forever, with no way back #940 no-progress FIXED-tombstone recovery;
• markPrFixItem's refused-FIXED head-SHA rollback, and a direct mark → QUEUED.

It never bumps for ordinary reads, repeated sync of known evidence, or updates that stay within the same active attempt — repeated reads of one pending unit of work return the same identity. The bump rule is centralized in freshAttemptGeneration(); the previous SHA-256 derivation (prFixGeneration) is removed entirely (queuedAt is the row's original creation time and never changes on requeue, so a hash could not distinguish fresh attempts).

Part 2 — Idempotent tasks/report

POST /api/agents/{agentName}/tasks/report accepts an optional opaque idempotencyKey (string, trimmed, ≤200 chars):

{
  "taskType": "followup-pr",
  "outcome": "pr_updated",
  "repoFullName": "org/repo",
  "pullRequestNumber": 12,
  "idempotencyKey": "<worker-chosen opaque key>"
}

When present, (agentName, idempotencyKey) uniquely identifies the logical report:

• the key is claimed and the AgentRun created in one transaction, guarded by a unique (agentName, idempotencyKey) constraint on the new AgentReportDedupe table — concurrent duplicates lose the index race (P2002) and replay the stored result;
• a retry with the same key and payload returns success with the original agentRunId (plus duplicate: true), creates no second AgentRun, and re-runs no PR-fix resolution (stored resolution replayed when persisted; explicit action: "skipped" marker otherwise — never a re-run);
• the same key with a different payload is rejected with 409 (canonical sorted-key payload hash, key-order insensitive);
• keys are agent-scoped (different agents may share a key) and treated as fully opaque; a claim whose referenced run was deleted returns 409 (recovery: fresh key, or delete the stale claim row);
• unexpected persistence failures return a structured 500 (Failed to report task) — a retry after it lands in the duplicate branch;
• reports without a key are unchanged — current at-least-once behavior, no client migration.

The resolver intentionally stays outside the claim transaction: it makes GitHub round-trips and itself opens a Prisma transaction (nesting is unsupported), so holding the claim transaction across it is not viable. Contract documented in AGENTS.md ("Idempotent reporting").

Migrations (both additive)

Validated by applying the full chain against scratch Postgres clusters — including a pre-migration PrFixQueueItem row that lands at generation = 1 — and pinned in CI by the database-integration job:

• 20260921000000_add_pr_fix_generation — PrFixQueueItem.generation INTEGER NOT NULL DEFAULT 1. Backfilling existing rows to 1 is the intended initial state: no generation concept existed before this migration (the superseded SHA-256 form in this PR's first revision never shipped), so there is no bump history to preserve.
• 20260921100000_add_agent_report_idempotency — AgentReportDedupe table with UNIQUE (agentName, idempotencyKey) and an ON DELETE SET NULL FK to AgentRun.

Tests

• Unit — generation lifecycle (pr-fix-queue.test.ts): stable identity across repeated reads/repeated evidence; bumps on BLOCKED→requeue→re-requeue (1→2→3), FIXED reopened by new evidence, [P2] A pr-fix item marked FIXED on workload exit alone strands the PR forever, with no way back #940 recovery, refused-FIXED rollback, mark→QUEUED; no bump for same-attempt updates or a trusted FIXED tombstone.
• Unit — next-task route: exact wire shape { id, generation: 1 } → { id, generation: 2 } for the same PR across a requeue; plus a route-level lifecycle test driving the real enqueue → BLOCKED → requeue state machine against an in-memory store.
• Unit — tasks/report (23 idempotency cases): no-key at-least-once unchanged; first keyed report (claim + one AgentRun + resolution + in-transaction stamp); identical retry replays agentRunId/stored resolution; no second AgentRun; no repeated side effects; concurrent same-key produces one logical report; key-order-insensitive identity; trim + length validation with bodies; same-key/different-payload → 409; different keys/agents independent; defensive no-result 409; resolution-store failure → structured 500 → retry dedupes to skip marker; AgentRun-write and resolver failures → structured 500.
• Integration — real Postgres (route.integration.test.ts, wired into the database-integration CI job): two concurrent same-key POSTs through the real route produce exactly one AgentRun, one claim carrying the result, and one duplicate: true; sequential retry replays the stored result; different payload → 409; run deletion nulls the FK reference and later retries 409; generation backfill pinned at the DDL level plus a real enqueue→BLOCKED→requeue bump; unique index and SetNull FK pinned via pg_catalog.

[itsmiso-ai]

⚠️ This PR fix item has been marked as BLOCKED and needs human attention.

Reason: Review comment on src/lib/pr-fix-queue.test.ts:154

Total attempts: 6

Attempts by lane:

• NEEDS_HUMAN: 1 attempt
• NORMAL: 5 attempts

Last attempt: src/lib/pr-fix-queue.test.ts:154 — Minor: Specialist lead: no direct unit test asserts that prFixGeneration returns a 64-character lowercase hex string (the route test asserts this indirectly via a regex, but a focused test for the helper itself is missing).

Automated finding from AI PR review.

Posted automatically by Dispatch on 2026-09-21T18:58:09.919Z

[joryirving]

Reworked in 02be91e: the SHA-256 derivation is gone, replaced by a first-class PrFixQueueItem.generation column. Findings addressed individually:

Blocker 1 (generation stable across requeues) — The underlying defect was real, though via a different mechanism than stated. fetchAgentQueueData() does carry queuedAt and evidenceKeys: listQueuedPrFixItems(...) → toAgentQueuePrFixItem(...), which has forwarded both fields since before this PR (they were added for the /queue endpoint). The actual failure was that PrFixQueueItem.queuedAt is the row's original creation time — requeuePrFixItem() reuses the same row and never touches it — so sha256(evidenceKeys + headSha + queuedAt) was identical for every requeue of the same evidence+head. Either way the conclusion held: the derived identity could not distinguish fresh attempts. It is now a persisted generation Int @default(1) that bumps atomically (Prisma increment, in the same update as the status flip) on every fresh-attempt transition: requeue from BLOCKED/FIXED, new evidence reopening a resolved item, the #940 no-progress FIXED recovery, and the refused-FIXED head-SHA rollback.

Blocker 2 (silent defaults collapsing items) — Moot: prFixGeneration() and the node:crypto import are removed entirely.

Major (unit test never exercised requeue) — Replaced with a lifecycle suite: stable identity across repeated reads/repeated evidence, BLOCKED→requeue (1→2)→re-requeue (2→3), FIXED reopened by new evidence, #940 recovery, refused-FIXED rollback, mark→QUEUED, and no-bump cases (same-attempt updates, trusted FIXED).

Major (route test lacked same-PR/two-generations) — Added: the same PR is served { id: "prfix-1", generation: 1 }, then after a requeue { id: "prfix-1", generation: 2 }.

Minor (per-field forwarding) — agent-task.test.ts now asserts id and generation individually; generation is an integer (PrFixItemIdentity.generation: number), not a stringified revision.

Minor (64-hex helper test) — Moot with the helper's removal.

Minor (docs) — docs/pr-review-fix-queue.md gains a "Work generation identity" section covering the contract, when generation changes, opaque-pair consumption, and the prFixItem omission for linked-PR follow-ups.

Also noted from the review's unknowns: the /queue endpoint now surfaces generation on each pr-review-fix item too (single change in toAgentQueuePrFixItem), so both endpoints expose the same field. The linked-PR follow-up path has no persistent identity of its own — that is a separate lifecycle design question, split out to #1045 rather than smuggled in here.

Migration is additive (ADD COLUMN "generation" INTEGER NOT NULL DEFAULT 1); validated by applying the full chain against a scratch Postgres, including a pre-migration row that lands at generation = 1.

Superseded by a newer automated review for this pull request.

[joryirving]

Extended in 308b347 per the Courier integration review: tasks/report is now safely retryable via an optional opaque idempotencyKey.

Mechanism. A new AgentReportDedupe table carries a unique (agentName, idempotencyKey) constraint. When a key is present, the claim insert and the AgentRun create run in one Prisma transaction — a committed claim always carries its result, and a concurrent or retried duplicate loses the unique-index race (P2002) rather than racing a SELECT then INSERT. The loser reads the stored claim and replays it: same agentRunId, duplicate: true, no second AgentRun, and no re-run of PR-fix resolution (the stored resolution is replayed; if the resolution-persist step raced, an explicit action: "skipped" marker is returned instead — side effects are never repeated).

Payload mismatch. The claim stores a canonical SHA-256 of the validated report fields (sorted-key JSON, so key order does not matter). Same key + different payload → 409 with a clear message; no AgentRun, no side effects.

Compatibility. Reports without a key take the exact previous path (no claim, no transaction, at-least-once). The key is agent-scoped, so different agents may reuse the same opaque value, and Dispatch performs no parsing of the key's content.

Coverage (9 scenarios in tasks/report/route.test.ts): no-key behavior unchanged; first keyed report (claim + one AgentRun + resolution + in-transaction agentRunId stamp); identical retry replays agentRunId/stored resolution; retry creates no second AgentRun; retry does not repeat resolution; concurrent same-key produces exactly one logical report (one request wins the insert, the other replays); same key/different payload → 409; different keys independent; different agents sharing a key independent; plus 400 validation for non-string/empty keys.

Contract documented in AGENTS.md ("Idempotent reporting"). Migration 20260921100000_add_agent_report_idempotency is additive and validated by applying the full chain against a scratch Postgres — the unique index it creates is the atomic mechanism the concurrency guarantee rests on.

Superseded by a newer automated review for this pull request.

[joryirving]

Addressed in 4802b5b. Findings taken individually:

Blocker — stored-resolution replay race. Valid window, fixed to the extent the architecture allows. The resolution persist is now awaited before responding (no .catch swallow): in practice the stored result is committed before the worker can see the first response, and if the persist fails the first response fails loudly — the worker's retry then lands in the duplicate branch (same agentRunId, explicit action: "skipped" marker) with side effects never repeated. Covered by a new test (failed resolution-store update → first response rejects → retry dedupes to the skip marker, one AgentRun, one resolution call). Moving the resolver inside the claim transaction is not viable and I want to be explicit about why: resolvePrFixFromAgentReport makes GitHub round-trips and, on the matched path, calls markPrFixItem, which opens its own $transaction — Prisma rejects nested interactive transactions, so this would require both holding a DB transaction across network I/O and refactoring the marking path. AGENTS.md now states the residual semantics precisely (stored result replayed when persisted; explicit skip while the original is still completing — never a re-run).

Major — key validation. Fixed: keys are trimmed on ingest (a padded and a bare form of one key now dedupe — tested) and capped at 200 characters (tested).

Minor — missing FK on agentRunId. Fixed: AgentReportDedupe.agentRun relation with ON DELETE SET NULL (schema + migration + reverse relation on AgentRun), per the strict-relations standard. The integration suite proves deletion nulls the reference and the claim stays recognizable (a post-deletion retry gets the defensive 409, not a silent re-run).

Major — untested no-result 409 guard. Tested now, including asserting no AgentRun and no resolution run on that branch.

Minor — concurrent test ordering fragility. Rewritten to capture the winner's claim data in a local variable at create time instead of scanning mock.calls.

Minor — canonicalJson determinism untested. A behavior-level test now sends the same logical payload with reversed JSON key order and asserts the retry is recognized as a duplicate rather than a 409. (On Unicode: payloads that differ by any byte — NBSP vs space included — are different payloads; fuzzy-matching them would weaken the 409 conflict contract, so no normalization is intended.)

Minor — replay response shape parity. The premise doesn't hold: the first response never contained touchedIssueUrls or other AgentRun fields — both shapes are {ok, agentName, report, agentRunId, prFixResolution} plus duplicate: true on retries only. No change needed.

Minor — no index on agentRunId. Deliberate: no query path looks claims up by run; the only access path is the unique (agentName, idempotencyKey) index, which exists. Adding an unused index would only tax writes.

Unknowns — real-Postgres concurrency. Addressed: new route.integration.test.ts (same opt-in gating as sync-lock.integration.test.ts, wired into the database-integration CI job) runs the actual route handler against a real PostgreSQL and proves two concurrent same-key POSTs produce exactly one AgentRun, one claim row carrying the result, one duplicate: true, and identical agentRunIds — plus sequential-retry replay, the 409 conflict, and the FK SetNull behavior. All four pass locally against a scratch Postgres with the full migration chain applied.

Carried-forward P1/P3/P5 — DEFAULT 1 "collapse". These rest on a premise that doesn't hold: no generation state existed before this migration. The SHA-256 derivation existed only in this PR's first revision (f6df8ef) and never shipped — the branch is unmerged, so no worker could have observed or persisted a hex generation, and nothing ever "bumped" a generation before this PR introduced bumping. Backfilling every existing row to 1 is therefore the only coherent initial state, and the migration comment now states this explicitly. P3's ?? 1 fallback in toAgentQueuePrFixItem is nonetheless removed — the column is NOT NULL, so the fallback could only ever mask a broken fixture.

Carried-forward P2 — docs shape change. docs/pr-review-fix-queue.md already described the integer form (updated in 02be91e); it now also notes explicitly that the superseded hex form never shipped, so no released contract exposed a string.

Carried-forward P4 — route test mutates a mock. That test intentionally verifies the route surfaces the persisted row faithfully (same id, new generation); the production requeue path itself is exercised end-to-end in pr-fix-queue.test.ts ("bumps generation on explicit requeue from BLOCKED, and again from FIXED", 1→2→3). The two tests cover the two layers respectively.

Superseded by a newer automated review for this pull request.

[joryirving]

Round 3 addressed in bdc3847.

Actionable items, fixed:

• Failed-store test asserted a raw rejection instead of the HTTP envelope — valid. The post-validation report path is now wrapped in try/catch returning the standard handleApiError 500 envelope (also an improvement for pre-existing failure paths like an AgentRun write failure, which previously bubbled as an unhandled rejection). The test now asserts a real 500 with { error: "Failed to report task" }, then the retry dedupes to the skip marker.
• Integration assertions tightened — the concurrent test now asserts exactly one of the two responses carries duplicate: true while the other omits the field entirely; the post-deletion retry asserts the 409 message; the surviving claim's payloadHash is inspected.
• Fixtures — generation: 1 added to the queue-route test fixtures, so no hand-rolled PrFixQueueItem fixture predates the NOT NULL column; production rows cannot lack it.
• Docs — docs/pr-review-fix-queue.md now mirrors the retry-resolution semantics (stored result replayed when persisted; explicit action: "skipped" otherwise; never a re-run). Migration comment notes the FK-before-index ordering is safe because the table is new and empty.

Positions held, with reasons:

• DEFAULT 1 "collapse" (P1/P5) — this review's own must-check verification states the resolution directly: "No DROP, RENAME, type change, NOT-NULL-without-default... No prior generation column existed, so there is no historical bump data being overwritten... the breaking-change risk is zero." There is no bump history to preserve because nothing before this unmerged PR ever bumped a generation; backfill-to-1 is the only coherent initial state and the task contract for this change mandates it. No code change can satisfy the finding beyond the documentation already added.
• Unicode normalization for canonicalJson — byte-inequality means different payload, by design. Normalizing NBSP vs space would make genuinely different summaries (different content) collide into silent deduplication; the conservative direction is to 409 and let the worker send the same bytes it sent originally. The contract only promises idempotency for the same payload.
• Response-shape parity (touchedIssueUrls on retries) — the first-time response has never contained touchedIssueUrls or other AgentRun fields; both response shapes are {ok, agentName, report, agentRunId, prFixResolution} with duplicate: true added on retries. There is no field a consumer sees on first report and misses on retry.
• Route-level test not exercising requeuePrFixItem — deliberate layering: the production requeue path is end-to-end tested in pr-fix-queue.test.ts (requeuePrFixItem 1→2→3, markPrFixItem rollback, [P2] A pr-fix item marked FIXED on workload exit alone strands the PR forever, with no way back #940 reopen), and the route test verifies the route faithfully surfaces the persisted (id, generation). Duplicating the queue lifecycle inside the route's prisma mocks would test the mock, not the product.

All checks green locally: 2764 unit tests, integration suite passing against a real PostgreSQL with the full migration chain, lint/typecheck clean.

Superseded by a newer automated review for this pull request.

[joryirving]

Round 4 addressed in 068ad8f:

• Docs — AGENTS.md's idempotency section now documents both exceptional paths: a claim without a recorded result (only possible after the referenced run was deleted, via the FK) is a 409, and an unexpected persistence failure is a structured 500 whose retry lands in the duplicate branch.
• 500-path coverage — two new tests: AgentRun write failure on the no-key branch returns the structured envelope, and resolver failure on the keyed branch returns the structured envelope while the committed claim still identifies the report for the retry.
• Queue fixture verification — the queue route test now asserts generation surfaces in the response body, so the fixture value is verified rather than decorative.

On the recurring items, the positions from the previous reply stand and are backed by the review's own must-check text ("No prior generation column existed... the breaking-change risk is zero" — there is no bump history to backfill-preserve, so DEFAULT 1 is the intended initial state, not data loss); byte-different payloads are different payloads by design (fuzzy Unicode matching would silently deduplicate genuinely different reports); and the first-time response has never carried touchedIssueUrls, so retry responses do not drop any field the first response had.

On the minor test-coupling notes: exact-string assertions ("Failed to report task", "no recorded result") follow the repo's existing convention (e.g. "Repository is already tracked" in the repos route tests) — the strings are the contract. The 64-hex assertion pins SHA-256 deliberately; a future algorithm switch should consciously update the test.

Superseded by a newer automated review for this pull request.

[joryirving]

Round 5 addressed in d8fdf27:

• P3 (route test exercising the production requeue path) — done properly: a new route-level lifecycle test drives the real enqueuePrFixItem → markPrFixItem(BLOCKED) → requeuePrFixItem state machine against an in-memory store (surfacing mocked, no network) and asserts next-task serves generation: 1 on the first attempt, then the same id at generation: 2 after the production requeue bumps it. The identity the route surfaces is now proven to be the one the production path persists.
• P2 (docs) — docs/pr-review-fix-queue.md now documents both exceptional idempotency paths (structured 500 on persistence failure → retry lands in the duplicate branch; 409 for a claim whose run was deleted), mirroring AGENTS.md.
• Formatting regression (collapsed test header) — fixed.

Worth noting for maintainers, discovered while writing that test: listQueuedPrFixItems normalizes any unrecognized lane string to NEEDS_HUMAN, so next-task?lane=<unknown> silently filters the PR-fix queue down to nothing (existing route tests masked this because their prFixFindMany mocks ignore the where clause). That is pre-existing queue-selection behavior, explicitly out of this PR's scope, but likely worth its own issue.

The remaining carried findings rest on premises already refuted in-thread and confirmed by this review's own must-check ("No prior generation column existed... the breaking-change risk is zero" — DEFAULT 1 is the intended initial state with no bump history to preserve; byte-different payloads are different payloads by design; the first-time response has never carried touchedIssueUrls so retries drop nothing; exact-string assertions match this repo's existing test convention).

Superseded by a newer automated review for this pull request.

[joryirving]

Round 6 addressed in 1dc6055:

• Migration-backfill test — the integration suite now pins the backfill semantics against real Postgres at the DDL level: information_schema asserts generation is NOT NULL DEFAULT '1' (the exact mechanism ALTER TABLE ADD COLUMN uses to value every pre-existing row), a row inserted without referencing the column lands at 1, and the production enqueue → BLOCKED → requeue lifecycle driven through the real client bumps generation to 2 via Prisma's atomic increment. This automates what the PR description previously demonstrated manually on a scratch cluster (row inserted pre-migration, migration applied, row lands at 1).
• PII note — AGENTS.md now states keys persist in the database and must not carry secrets or personal data.
• 400 bodies — the idempotencyKey validation tests assert the response messages, not just status codes.

Positions unchanged on the recurring items, each already refuted in-thread: the resolver cannot run inside the claim/run transaction (GitHub round-trips + Prisma forbids the nested $transaction that markPrFixItem opens), and a duplicate arriving in the pre-persist window receiving the explicit action: "skipped" marker is the documented, correct outcome — side effects are never repeated, agentRunId is always returned; byte-different payloads are different payloads; the first-time response has never carried touchedIssueUrls; the unique index and FK are exercised against real DDL by the integration suite through the only consumer they exist for (the route).

Superseded by a newer automated review for this pull request.

[joryirving]

Round 7 addressed in 121320d:

• Migration-level DDL pins — the integration suite now asserts, via pg_indexes and pg_constraint, the UNIQUE ("agentName", "idempotencyKey") index and the ON DELETE SET NULL FK against real Postgres. (Verified the hard way: pg_constraint's delete-action code for SET NULL is 'n', not 'a', and the raw "char" column needs a ::text cast through Prisma's adapter — the intermediate commits that got this wrong were amended away before anything built on them.)
• Deleted-run recovery path documented — AGENTS.md now states the recovery when a referenced AgentRun is deleted: the worker proceeds under a fresh key, or an operator deletes the stale AgentReportDedupe row.

On the remaining carried items, the analysis in earlier replies stands and is not changed by this delta: the resolver cannot move inside the claim/run transaction (GitHub round-trips; Prisma rejects the nested interactive transaction that markPrFixItem opens), a duplicate arriving in the pre-persist window correctly receives the explicit action: "skipped" marker with side effects never repeated, and the next-task wire shape IS positively asserted (expect(body.prFixItem).toEqual({ id: "prfix-1", generation: 1 }) — toEqual is an exact-shape match, present in two tests).

One process note for the human reviewer: the automated review requested error-message assertions in round 4 ("the response body is not asserted … only status codes are checked") and now flags those same assertions as coupling in round 7. The literal messages are the contract (matching this repo's convention, e.g. "Repository is already tracked"), so the assertions stay.

Superseded by a newer automated review for this pull request.

Superseded by a newer automated review for this pull request.

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M3@https://litellm.jory.dev/v1 (anthropic) — routed smart (risk match: db_or_migration_changes)

Recommendation

Request changes. The PR delivers two well-scoped features — Dispatch-owned PR-fix work generation and idempotent tasks/report — and the database integration suite, schema, and CI gating all look sound. However, the PR is labeled db_or_migration_changes and path_handling_changes, and the linked issue misospace/courier#71 plus the must-check list both demand evidence that the migrations are safe on a copy of production and that any path-handling changes are covered. The required-checks list is the dominant signal here: while the diff is internally consistent and standards-compliant, several must-checks cannot be conclusively verified from the corpus, and a real production-schema copy test is not surfaced as part of the CI evidence the action captured.

Change-by-change findings

prisma/migrations/20260921000000_add_pr_fix_generation/migration.sql

• Adds generation INTEGER NOT NULL DEFAULT 1 to PrFixQueueItem. The migration is additive (no data loss for the column itself), and the migration comment correctly states that no prior bump history exists. Backfill to 1 is acceptable here.
• must_check (data loss): Verified from diff that the migration is purely additive (single ADD COLUMN ... NOT NULL DEFAULT 1). No UPDATE, no DROP, no destructive index changes. Satisfied from diff.
• However, the standards file AGENTS.md does not document any pre-migration backfill test gate; the PR claims it was "Validated by applying the full chain against scratch Postgres clusters — including a pre-migration PrFixQueueItem row that lands at generation = 1". The integration test route.integration.test.ts pins the column default via information_schema.columns, but the corpus does not surface evidence that a production-shaped dataset (existing PrFixQueueItems with non-null feedback, evidenceKeys, headSha) was migrated end-to-end.

prisma/migrations/20260921100000_add_agent_report_idempotency/migration.sql

• Creates AgentReportDedupe with (agentName, idempotencyKey) unique index, an FK to AgentRun with ON DELETE SET NULL, and a secondary agentName index. The migration is purely additive.
• must_check (data loss): Verified from diff — new table, no ALTER on existing tables beyond adding the FK. Satisfied from diff.
• A potential issue: the FK is added before the unique index in the migration SQL. On a non-empty AgentReportDedupe this would still be fine (the table is empty), but ordering FK → UNIQUE INDEX would block future evolution. Flagged for the operator to confirm migrations are run on an empty table only (corpus confirms empty at apply time).

prisma/schema.prisma

• New AgentReportDedupe model + generation field on PrFixQueueItem + back-relation on AgentRun. Schema follows standards ("Prisma schema - Keep relations strict; do not make foreign keys nullable to hide bugs"). The agentRunId field is nullable, but only because SetNull is required to preserve the claim when the run is deleted — this is the documented recovery path, not a bug-hiding nullable.

src/app/api/agents/[agentName]/next-task/route.ts and src/lib/agent-task.ts

• Surfaces prFixItem: { id, generation } on followup-pr tasks. Optional and omitted for linked-PR follow-ups not backed by a queue item. Test suite covers the new fields (route.test.ts, agent-task.test.ts).

src/lib/pr-fix-queue.ts

• Centralizes generation bumps in freshAttemptGeneration() and applies it in enqueuePrFixItem, markPrFixItem, requeuePrFixItem, and the #940 refused-FIXED rollback. Tests in pr-fix-queue.test.ts cover each transition: requeue from BLOCKED → +1, requeue from FIXED → +1, new evidence reopening FIXED → +1, PR 940 recovery → +1, refused-FIXED rollback → +1, repeated reads while QUEUED → no bump, additional evidence on QUEUED → no bump, mark-to-QUEUED from BLOCKED → +1, mark-to-QUEUED when already QUEUED → no bump.
• The bump is applied via Prisma's { increment: 1 } operator, which is atomic and safe under concurrent transitions (verified by diff).

src/app/api/agents/[agentName]/tasks/report/route.ts

• Adds optional opaque idempotencyKey (trimmed, ≤200 chars). The route claims the key and creates the AgentRun in one transaction (prisma.$transaction(async (tx) => { create dedupe; create agentRun; update dedupe.agentRunId; })). On P2002, the route reads back the existing claim and replays the stored agentRunId/prFixResolution (or returns a skipped marker if no resolution has been persisted yet). 409 on hash mismatch or missing run; structured 500 on unexpected persistence failure. Tests cover all branches.

.github/workflows/ci.yaml

• Renames the sync-lock DB integration job to also run the new route.integration.test.ts against a real PostgreSQL. CI evidence confirms Database integration: success and Database migrations: success, so both migrations apply and the integration suite passes on the project's standard scratch DB.

must_check coverage

• review migration for data loss risk: Both migrations are purely additive (new column with default; new table with new index/FK). No DROP, no UPDATE, no destructive renames. Satisfied.
• test migration on a copy of production schema: The corpus surfaces integration tests against a real PostgreSQL via RUN_DB_INTEGRATION=1 (Database integration: success). The PR description claims "Validated by applying the full chain against scratch Postgres clusters — including a pre-migration PrFixQueueItem row". Partially satisfied from corpus — the CI job runs the migration against the project's scratch DB, but evidence of running against a production-shaped schema (with realistic row counts, pre-existing PrFixQueueItem data, and FK relationships) is not surfaced. See Needs Verification.
• review for path traversal vulnerabilities: This PR does not touch file system paths. path_handling_changes is a classifier flag, but no diff hunk in the corpus modifies any file-path handling, request-body parsing for paths, or URL construction with user-controlled segments. The schema/migration changes cannot introduce a path-traversal vulnerability on their own. Not applicable to this diff.
• test with edge-case paths (null bytes, symlinks): No path-handling code is added. The closest edge-case validation in this PR is idempotencyKey length trimming (≤200 chars, non-string rejection, post-trim empty rejection). The PR validates the idempotencyKey boundary at 201 chars (per route.test.ts), and rejects non-string types, empty strings, and whitespace-only strings. Not applicable to the path-handling must_check — the path-traversal must-checks are for path input; the idempotency key edge cases are not path inputs. N/A.

Standards Compliance

The standards file is AGENTS.md (Dispatch self-hosted Next.js/TS Kanban). The PR is consistent with documented conventions:

• No agent-specific names in generic docs — the new Idempotent reporting section uses generic worker language.
• Prisma relations — agentRunId is nullable only to support the documented ON DELETE SET NULL recovery path, which is explicit in the schema comment and the route's 409 handler.
• API routes return appropriate status codes: 200 for first/retry success, 400 for bad idempotency key, 409 for hash-mismatch and orphan-claim, 500 for unexpected persistence failure. Documented in AGENTS.md.
• Error handling uses error instanceof Error and the centralized handleApiError envelope.
• Validation runs before DB operations (idempotencyKey type/length checks precede prisma.$transaction).
• No .env / secrets committed; idempotencyKey is documented as opaque and warned against embedding secrets/PII.
• Migration files live under prisma/migrations/ with timestamped names and a corresponding schema update.

Tool Harness Findings

The native tool-calling loop issued no tool calls; this review was produced from the corpus directly. No tool harness output to summarize.

Unknowns / Needs Verification

• Production-schema migration rehearsal: The CI runs against a scratch DB, not a snapshot of production data. The PR description mentions "scratch Postgres clusters" but the corpus does not surface a row-count or FK-shape that matches production. Recommend running the migrations against a pg_dump-restored production copy before merge, with at least one PrFixQueueItem row in each terminal status (QUEUED, FIXED, BLOCKED, STALE, IGNORED) to confirm the generation backfill and the agentReportDedupe table creation under realistic cardinality.
• metadataPatch(input) field ordering in enqueuePrFixItem: A specialist lead flagged that ...freshAttemptGeneration() is spread before ...metadataPatch(input), and that metadataPatch could overwrite generation if it ever sets that field. I cannot confirm from the diff alone whether metadataPatch touches generation — the function is not in the corpus. Marked unknown. Recommend confirming metadataPatch does not set generation.
• payloadHash sensitivity to legitimate payload enrichment: The integration test asserts that key-order-insensitive retries dedupe, but a specialist lead correctly notes that any legitimate enrichment after the first attempt (e.g., adding summary) would change the hash and 409. The PR's docstring says "Different payloads = different reports = different idempotency units", which is consistent with the implementation but may surprise workers that incrementally fill in summary. Recommend documenting explicitly that retries must carry the same payload bytes (or close enough that the canonical JSON matches) in AGENTS.md.
• Concurrent race visibility window on P2002: The route's P2002 handler does findUnique outside the original transaction; if the winner has not yet committed, existing is null and the error is rethrown (surfaced as a 500). The PR does not address this race; an immediate retry typically resolves it, but the 500 vs 409 semantics differ from the documented contract. Marked unknown — recommend documenting the "rare 500 from concurrent duplicates, retry to land in duplicate branch" behavior.

Specialist review leads (advisory, verified where adopted)

• Adopted: metadataPatch overwrite risk, payloadHash sensitivity, concurrent P2002 visibility window, idempotency-key length validation nuance (post-trim). All flagged in Needs Verification above.
• Discarded: The "data loss for already-dispatched rows" lead — false positive. DEFAULT 1 backfill is the intended initial state per the migration comment; no prior bump history exists to lose.
• Discarded: The "claim commit + agentRun + update is not atomic" lead — false positive. The three-step sequence runs inside prisma.$transaction, which is atomic at the PostgreSQL boundary.
• Discarded: The "JSONB render safety" lead — speculative; no render path was modified.
• Discarded: The "exact HTTP shape for delete-then-retry 409" lead — the integration suite covers this against real Postgres; the response shape is asserted via the error field containing "no recorded result".

Why request_changes (not approve)

Two blocking concerns from the corpus:

1. The must-check "test migration on a copy of production schema" cannot be conclusively verified from the corpus. The CI runs against a scratch DB, not a production snapshot. While this is acceptable for low-risk additive migrations, the PR is explicitly labeled db_or_migration_changes, and AGENTS.md does not document an exemption. The migration is low-risk (additive + backfilled default), but the corpus does not surface the rehearsal.
2. The metadataPatch ordering concern is unresolved from the diff alone; until confirmed safe, the generation bump could be silently overwritten in enqueuePrFixItem. This is a data-integrity risk on the central feature of the PR.

If the operator can confirm that (a) the migration was rehearsed against a pg_dump copy of production with non-empty PrFixQueueItem rows, and (b) metadataPatch does not write to generation, this PR is mergeable.

[its-saffron]

⚠️ Major: Field-ordering risk in enqueuePrFixItem: ...freshAttemptGeneration() is spread before ...metadataPatch(input); if metadataPatch ever assigns generation, it silently overwrites the increment — needs confirmation that metadataPatch does not write to generation.

Automated finding from AI PR review.

[its-saffron]

⚠️ Major: Production-schema migration rehearsal is not surfaced in the corpus: CI runs against a scratch DB, not a pg_dump of production, despite db_or_migration_changes risk flag and must_check requiring a production-schema copy.

Automated finding from AI PR review.

[its-saffron]

⚠️ Major (bug): On a P2002 race where the winner's transaction has not yet committed, findUnique returns null and the error is rethrown (5xx) instead of retrying or returning 409 — this differs from the documented contract that returns 409 only when the run was deleted.

Automated finding from AI PR review.

[its-saffron]

Minor (docs): Payload hash includes the full report body; legitimate worker-side enrichment (e.g., adding summary after first attempt) will 409 the retry as a "different payload" — workers need explicit guidance that retries must carry identical payload bytes.

Automated finding from AI PR review.

[its-saffron]

Minor: prFixItem is added unconditionally for queue-backed followups even when the row's generation field could in theory be undefined for very old rows on a mid-deploy schema state — no defensive ?? 1 fallback in the route response.

Automated finding from AI PR review.