[joryirving]

Closes #1145.

next-task had two producers for the same linked PR: the PR-fix queue and the older linked-PR-health scan. After the queue skipped a PR (already handed to this agent, or BLOCKED), the health scan still emitted its own followup-pr task — no persistent identity, so a polling worker dropped it every ~30s while the stale candidate kept winning the lane ahead of ordinary issue work.

The queue is now the single owner:

• any existing queue row for the PR — any status, any lane — makes the linked scan defer, so a BLOCKED/needs-human verdict can't be bypassed;
• a linked-only PR is materialized as a real queue row via createLinkedPrFixItem (create-only against the (repo, pr) unique key, so a concurrent enqueuePrFixItem wins without being mutated or reopened) and handed out through the same per-agent generation flow as ordinary PR-fix work;
• PRs carrying the Dispatch needs-human label are skipped (read from the PR, since that's where surfacePrFixBlocked applies it);
• a non-dispatchable candidate falls through to the next one instead of starving the lane, and the derived work is only served to lanes that can consume PR-fix work (prFixLaneForRequest).

Materialized rows also capture the discovered head SHA as the attempt baseline (#1074), so the #940 no-progress guard refuses a FIXED tombstone for a worker that pushed nothing — the same protection queue-enqueued rows get.

Two judgement calls worth flagging:

• If the labels read fails, the PR is skipped for that poll rather than materialized: needs-human is a human-intervention verdict, and a transient GitHub failure shouldn't be what bypasses it. The candidate is retried on the next poll.
• A linked-only PR in a lane with no PR-fix equivalent (e.g. a claimable cloud lane with no role) is materialized as NORMAL rather than dropped, and the non-capable caller is not handed the task itself — the row is picked up by a capable lane on its next poll.

Tests cover dual qualification, the 5-status × 3-lane defer matrix, linked-only materialization plus repeat polling, the concurrent-create race, needs-human, the unmappable-lane case, and per-candidate fail-soft on a DB error.

[joryirving]

Blocking review finding:

The linked-PR path now defers correctly to the PR-fix queue, but a deferred linked candidate remains in rankedQueue and can immediately fall through as an implement task for the same issue.

That breaks #1145's settled behavior in several important cases:

• an existing QUEUED item already handed to this agent is skipped by dispatchPrFixCandidate, then the linked scan sees the existing row and defers, but the final rankedQueue[0] fallback can hand the same issue back as implement;
• a BLOCKED/terminal queue row similarly defers the linked path but can still become fresh implementation work;
• a live needs-human PR label or failed label read skips materialization, but the issue remains eligible for the implement fallback;
• the no-PR-fix-equivalent lane case materializes the PR work elsewhere, then can still hand the same issue to this caller as implementation.

The new tests currently pin this bug: e.g. the repeat-poll test and needs-human test expect the second result to be implement for the same sole issue. That is not "continue to the next ranked item"; it creates a second work identity and can bypass the queue/human verdict under a different task type.

Please suppress any issue whose linked PR needsFollowup but was deferred/non-dispatchable from the ordinary implement fallback for that poll, then select the next genuinely independent ranked issue (or idle if none). Tests should cover already-handed, BLOCKED, needs-human/label-unavailable, materialization failure/race, and no-PR-fix-equivalent lane with another ordinary issue behind the linked candidate.

Everything else I checked in the ownership/materialization direction looks sound: create-only (repo, pr) ownership, P2002 winner preservation, head baseline capture, and queue-status/lane deferral are all the right shape.

[joryirving]

Fixed in 1d74fb2 + 906e585 — you were right, and my own tests were pinning the bug. Thanks for the sharp read.

The fix. A deferredIssues set keyed <repo>#<issueNumber> (repo case-folded — the queue row's repo and the issue's repository.fullName are not normalized against each other, so an exact-match key would silently reopen the door) is populated from both places the queue takes ownership:

• every skipped pr-fix candidate that carries an issue (already handed to this agent, or a contested generation), and
• every issue in the linked scan whose PR needsFollowup, added before any defer decision — so it covers existing-row, BLOCKED/terminal, needs-human, label-read-unavailable, materialization failure/race, and no-PR-fix-equivalent-lane alike.

The implement pick is now the first ranked issue that is not deferred. When nothing is left, next-task idles with a reason that distinguishes the deferral from a grooming-admission hold (and reports both when both apply):

{ "type": "idle", "shouldRun": false,
  "reason": "No work available (1 ready issue deferred: linked PR follow-up is owned by the PR-fix queue)" }

Tests — all five combos you listed, each with an independent ranked issue behind the deferred linked candidate, asserting the lane drains to it: already-handed (two tests, incl. the case-mismatch variant), the 5-status × 3-lane defer matrix, needs-human label, label-read-unavailable (null), concurrent-create race (P2002 winner), materialization failure, and the cloud/no-PR-fix-equivalent lane. Plus an idle variant for each so the "no next issue" path is pinned too. The tests you flagged as pinning the bug (repeat-poll, needs-human) now assert idle/next-issue.

3916 tests pass, typecheck and lint clean.

Two things worth your eye on the re-review:

1. The deferral is deliberately not time-bounded. An issue whose PR sits BLOCKED stays deferred until an operator requeues/resolves the item or a health reconcile sees the PR closed. That follows from "BLOCKED means human intervention is required and must not be bypassed", but it does mean a stuck item silently idles that issue — documented in docs/pr-review-fix-queue.md#linked-pr-follow-up-ownership-1145 so it's triageable.
2. Label-read failure stays fail-closed (defer, retry next poll) rather than failing open. A transient GitHub 5xx shouldn't be what bypasses a human-intervention verdict. If you'd rather it fail open, that's a one-line change — say so.

Also corrected a now-stale line in docs/pr-review-fix-queue.md: linked-PR follow-up always carries a prFixItem token, since it's materialized into a queue row before hand-out.

[joryirving]

Re-review on 906e585: the original implement fallback blocker is fixed. The deferredIssues set now excludes the linked issue, and the expanded tests check the next independent issue rather than merely the task type. I agree with both documented policy choices: BLOCKED deferral should not expire automatically, and a failed live needs-human label read should fail closed. Current CI is green.

I did find two remaining ownership gaps worth fixing before merging:

1. Blocking: case-variant repo names can create a second queue row and bypass BLOCKED. The new issueKey lowercases repository names, but prisma.prFixQueueItem.findUnique({ where: { repo_pr: { repo, pr } } }) and createLinkedPrFixItem do not. The DB's @@unique([repo, pr]) is a case-sensitive TEXT index, and enqueuePrFixItem/the ingestion paths do not canonicalize repo. Repro shape: existing {repo: "Org/Repo", pr: 15, status: "BLOCKED"}, linked issue with repository.fullName="org/repo", needsFollowup=true, and the PR's GitHub needs-human label missing (surfacing is best-effort). The lookup misses the existing BLOCKED row, the create succeeds under org/repo, and a second QUEUED followup-pr identity can be dispatched. Please enforce a consistent case-insensitive (repo,pr) ownership boundary, including existing rows, and add the opposite-case BLOCKED regression test. The new case-folding test covers only issue deferral, not this uniqueness boundary.

2. Blocking: a BLOCKED row can still become implement when cached linkedPrNeedsFollowup is false. The queue fetch lists only QUEUED rows; the linked scan enters only on health.needsFollowup. For a ready linked issue with linkedPrNumber=15, linkedPrNeedsFollowup=false (stale or subsequently-cleared health), and an existing PrFixQueueItem(status=BLOCKED), neither loop adds it to deferredIssues; the final rankedQueue.find(!isDeferred) returns that same issue as ordinary implementation. Please ensure existing queue ownership can defer the linked issue even when the cached health flag is false, and test BLOCKED + stale/false health + independent issue behind it.

These are separate from the earlier fallback fix, which looks good. No objection to the two explicit operator-policy decisions.

[joryirving]

Both closed in c4e4c87.

1. Repo casing. You're right that this was a live bypass, and it was worse than the deferral set: the unique key is case-sensitive while GitHub is not, so Org/Repo and org/repo were two rows owning the same PR — and my earlier issueKey fold only fixed the in-memory set, not the queue identity itself. A linked materialization deriving its repo from the issue cache could therefore create a second QUEUED row beside an existing BLOCKED one and hand the work out anyway.

• New normalizeQueueRepo (trim + lowercase) is applied at every write and every repo_pr identity lookup: enqueuePrFixItem, createLinkedPrFixItem, markPrFixItem, requeuePrFixItem, resolvePrFixFromAgentReport, pr-followup-ingestion's merge-conflict clear, the history route, and the linked scan's existing-row check. Implemented by folding the input once at each function entry, so every derived read/write in that function agrees.
• Migration 20261008000000_lowercase_pr_fix_repo brings existing rows onto the same footing and merges any collisions: BLOCKED wins the merge (a human verdict must not be dropped), then most-recently-updated; history rows are repointed to the survivor before the loser is deleted (the FK cascades, so ordering matters), then survivors are folded.

I didn't just eyeball that SQL — I ran it against a real Postgres 18 with fixtures covering: a casing collision with a BLOCKED row, a collision without one, a lone mixed-case row, a trailing-space row, and an already-canonical row. Result: Org/Repo#15 BLOCKED survived over org/repo#15 QUEUED with all three history rows repointed, the newer row won the non-BLOCKED collision, and the lone/space rows folded. The unique constraint held afterwards.

2. Stale PR health. Also right, and it's the mirror of the gap I just fixed: the deferral was gated on linkedPrNeedsFollowup, but that column is refreshed on a reconcile cadence and can lag the row's creation — and since BLOCKED rows never appear in listQueuedPrFixItems, nothing else stopped the issue. The existing-row check now runs for every issue with a linked PR and only the materialization decision consults the health flag. A linked PR with no row and no follow-up health still leaves the issue as ordinary implement work — I added a counter-test for that so the fix can't over-suppress.

Tests: casing round-trips (enqueue/mark/requeue/materialize across three different casings, asserting one row), a route test where the issue cache says Org/Repo and the queue key is org/repo, stale-health deferral with an independent issue behind it, and the no-follow-up/no-row passthrough. I also made the queue test client emulate the @@unique([repo, pr]) index — without it a duplicate identity silently "succeeded" and the convergence tests couldn't tell a merge from a second row. I verified the casing route test fails when the fold is removed.

3923 tests pass, typecheck and lint clean.

One thing I want to flag rather than bury: the migration deletes duplicate rows when it finds them. If a deployment has casing duplicates where the non-BLOCKED row was the one a worker was mid-flight on, that attempt's token dies with the row (its report lands as skipped, no mutation). I think that's the right trade against leaving two owners for one PR, but it's the one destructive edge in this change — worth a look at the migration before release if you disagree.

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M3 (anthropic) — primary route · pr-reviewer-action v3.4.1

Requirement traceability gap: the review itself completed, but one or more in-scope requirements could not be verified against enforcement or test evidence. Approval is withheld until that requirement is covered.

Recommendation: Approve

This PR cleanly resolves PR 1145 (the p1 bug where two producers emitted work for the same linked PR, causing a stale followup-pr task to win the lane ahead of ordinary issue work) and adds a defensive case-insensitive (repo, pr) ownership boundary after the initial review surfaced a case-bypass path. The implementation matches the issue's "Settled behavior" and "Done when" criteria, with explicit tests covering each acceptance bullet.

Change-by-change findings

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

• The queue is now the single owner: the route builds a deferredIssues set and every implement-fallback path (rankedQueue.find(!isDeferred), the new idle reason, the implement loop) consults it. dispatchPrFixCandidate factors the per-candidate confirm + hand-out + task-build path so both the existing prFixItems queue and the freshly materialized linked row go through identical hand-out logic, including the status: "QUEUED" and generation pins added in confirmPrFixHandOut.
• The linked-PR loop now does an explicit findUnique for any issue whose linkedPrHealth.number is set, independent of linkedPrNeedsFollowup — that's the fix for the stale-cache gap flagged in the second review comment (the test defers a linked issue whose PR has a queue row even when cached health says no follow-up covers it).
• needs-human is read from the PR (fetchPullRequestLabels) and a label-read failure (null) fails closed by deferring — matches the PR's documented judgement call and the related-issue's "human-intervention verdict must not be bypassed" requirement. Both branches are explicitly tested.
• Lane gating uses prFixLaneForRequest: the caller's lane decides whether it can consume PR-fix work at all (null for cloud-like lanes). The route materializes the row (so a capable lane picks it up) but does not hand the work to the lane that discovered it — this matches claim 4 and the cloud lane test.

src/lib/pr-fix-queue.ts

• normalizeQueueRepo (trim().toLowerCase()) is applied at every queue write and repo_pr identity lookup: enqueuePrFixItem, createLinkedPrFixItem, markPrFixItem, requeuePrFixItem, resolvePrFixFromAgentReport, plus the two existing callers (pr-followup-ingestion's merge-conflict clear, the history route) and the new linked-scan lookup. parseEnqueuePrFixInput deliberately does NOT fold so the HTTP body validation preserves what callers submitted (the runtime functions do the canonicalization); this is consistent with the existing trim-only treatment of the HTTP parser.
• createLinkedPrFixItem does a single prisma.$transaction create+history; on P2002 it reads back the winner and returns { item, created: false } without mutation, so a concurrent enqueuePrFixItem wins untouched. A non-P2002 failure rethrows to the caller, which logs and moves on to the next candidate (the route's outer try/catch keeps a transient DB failure from 500-ing the poll).
• The headSha baseline is captured via fetchPullRequestHeadSha and stored on both headSha and attemptHeadSha so the PR 940/PR 1074 no-progress guard fires on a worker that reports FIXED without pushing.

prisma/migrations/20261008000000_lowercase_pr_fix_repo/migration.sql

• The migration does the correct ordering: build a temp loser→winner map, repoint PrFixHistory.itemId from losers to the winner, then delete the losers, then fold survivors. Winner selection is BLOCKED > most-recently-updated > id — a human verdict cannot be dropped. History repointing before delete is required because PrFixHistory.itemId cascades on delete (verified from the model definition). After the merge, at most one row remains per (lower(btrim(repo)), pr) so the unique constraint holds for the final fold. The author flagged that mid-flight tokens on a non-survivor will report as skipped — that's an acknowledged destructive edge but the trade against leaving two owners of one PR is the right one. The CI gate Database migrations passes against this migration.

src/lib/pr-fix-queue.test.ts and src/app/api/agents/[agentName]/next-task/route.test.ts

• The new tests cover: dual qualification (the queue's own row wins over the linked scan), the 5-status × 3-lane defer matrix (15 cases), linked-only materialization, repeat polling (the issue is not re-served as implement work), the concurrent-create P2002 race, needs-human label + read failure, the unmappable cloud lane, per-candidate fail-soft on a non-constraint DB error, head-SHA baseline on materialize, deferred-identity from queuedLinkedPrs, deferral driven by the queue row not the stale health flag, and case-insensitive (repo, pr) round-trips for write and materialize. The test client was upgraded to emit P2002 on duplicate (repo, pr) so the convergence tests are actually testing the contract.

Claims to Falsify

• Claim 1 (BLOCKED follow-up emission / followup-pr identity-less starvation) — held. The prFixItems queue still beats issue cards, every prFixItems scan adds the owning issue to deferredIssues, and confirmPrFixHandOut now pins on status: "QUEUED" and re-reads the row's lane so a BLOCKED hand-out is refused at commit time.
• Claim 2 (linked-only materialization via createLinkedPrFixItem) — held. All twelve listed call sites point at the new symbol (src/lib/pr-fix-queue.ts:738, called only from src/app/api/agents/[agentName]/next-task/route.ts:309). The enqueuePrFixItem concurrent-winner path returns it untouched (created: false); no row is mutated, no history row is added.
• Claim 3 (per-candidate fall-through; prFixLaneForRequest gating) — held. prFixLaneForRequest is used twice in the route (line 255 for the caller's lane, line 295 for the item's lane); one consumer each in agent-queue-fetch.ts:111 and lane-config.ts:263. The cloud lane test verifies the queue-only path.
• Claim 4 (linked-only materialization as NORMAL; non-capable caller not handed the task) — held. The route computes queueLane = prFixLaneForRequest(followupItem.lane) ?? "NORMAL" (line 295) and if (requestPrFixLane === null) continue (line 333). Test materializes linked-only work for a lane without a PR-fix equivalent, without handing it to that lane covers this exactly.
• Claim 5 (needs-human skip; dual qualification + 5-status × 3-lane matrix; linked-only materialization; repeat polling; concurrent-create race; needs-human; unmappable lane; per-candidate fail-soft on a DB error) — held. Each named scenario has a dedicated it(...) block: needs-human skip (does not create linked follow-up for a PR with the needs-human label), dual qualification (the PR-fix queue owns a linked PR that also qualifies as a queued issue), 5×3 matrix (linked path defers to existing %s/%s queue row), materialization (returns followup-pr when issue has linked PR needing follow-up), repeat polling (linked-only work is materialized once), concurrent-create race (does not dispatch or mutate a concurrent queue winner during linked materialization), unmappable lane (materializes linked-only work for a lane without a PR-fix equivalent), and fail-soft (does not fail the poll when linked materialization errors).

Linked Issue Fit (PR 1145)

Acceptance criteria from the issue:

1. ✅ "a PR qualifying through both discovery paths yields exactly one work identity" — confirmPrFixHandOut now pins status: "QUEUED" and re-reads status/lane/agentHandouts; the existing-row check runs before any materialization; concurrent-create race returns the winner untouched.
2. ✅ "a queue-owned PR is never separately emitted by the linked-PR path" — the deferredIssues set is checked in the implement fallback; the linked scan consults findUnique on every iteration regardless of needsFollowup.
3. ✅ "BLOCKED / needs-human PRs are not handed out through the linked path" — both branches (needs-human label, label-read error) defer the issue; BLOCKED rows cause the linked scan to continue after adding the issue to deferredIssues.
4. ✅ "a linked-only PR is materialized into persistent queue identity before hand-out" — createLinkedPrFixItem returns { id, generation } that flows into dispatchPrFixCandidate and the resulting followup-pr task.
5. ✅ "a worker that has already received that attempt is not offered it again on every poll" — confirmPrFixHandOut now consults agentAlreadyHanded on the re-read row before confirming, and the prFixItems loop adds the item's issue to deferredIssues whether the hand-out was confirmed or skipped.
6. ✅ "the next ranked issue is returned when the linked-PR candidate is non-dispatchable" — rankedQueue.find((item) => !isDeferred(item)) (line 380).
7. ✅ "regression tests cover dual qualification, linked-only materialization, already-handed-out work, and blocked/needs-human work" — see Claim 5 above; all eight named cases plus the 15-case status × lane matrix.

Standards Compliance

• Repository-specific conventions from AGENTS.md: the next-task worker loop contract is preserved (one AgentTask returned, idle as { type: "idle", shouldRun: false, reason: "..." }); bearer-auth is unchanged. No DISPATCH_AGENT_TOKEN/GITHUB_TOKEN is logged or persisted by this change. Label hygiene (umbrella only, no blocked/infra-attempt/*) is unaffected.
• Migration risk (must_check: review migration for data loss risk): the migration is destructive only in the narrow case of an existing casing collision where the loser is the only row a worker was issued against. The author flagged this explicitly in the thread and the trade against leaving two owners is documented; CI's Database migrations check has run against the migration. Production review should confirm no PR is currently being worked by a worker at the time of deploy (a brief deploy-time stalemate that would cause that one in-flight report to come back as skipped).
• High-priority verification (treat as high priority — verify correctness carefully): the deferral-set logic, the materialize/concurrent-race path, and the case-fold enforcement are each pinned to a dedicated test, all green per Tests CI.

Unknowns / Needs Verification

• None blocking merge. The two judgement calls the author flagged in the PR body — that BLOCKED deferral does not expire automatically, and that a failed label read fails closed — are explicitly tested and consistent with the linked issue's settled behavior ("BLOCKED means human intervention is required and must not be bypassed"). The one operational item is a deploy-time check that no worker is currently mid-flight against a row that the migration would delete (i.e., a casing-collision loser), so no in-flight tasks/report lands as skipped for the wrong reason at deploy.

Requirement trace

1 of 3 requirement(s) not fully traced to enforcement and a test:

• req-1b681c69493f — unverifiable (src/app/api/agents/[agentName]/next-task/route.ts:279): The route consults an existing PrFixQueueItem for the linked (repo, pr) before materializing any new follow-up task and defers the issue to implement pickup without bypassing BLOCKED rows. The parametrized it.each suite over every status/lane combination (including BLOCKED) asserts the deferral and

Approval withheld: the review completed, but an in-scope requirement lacks verifiable enforcement or test evidence, so it is publishing as an advisory comment rather than an approval.

[joryirving]

Re-review of c4e4c87: no remaining blocking findings; merge-ready from my side.

Both findings on 906e585 are addressed:

1. Case-sensitive queue ownership: normalizeQueueRepo is applied consistently to queue writes and repo_pr identity lookups, including linked materialization, enqueue, mark, requeue, report settlement, merge-conflict cleanup, and history lookup. The migration folds existing rows, elects a BLOCKED survivor ahead of newer non-BLOCKED contenders, repoints PrFixHistory before deleting duplicates, and then canonicalizes the surviving keys. Regression coverage includes cross-case queue operations and a route-level case mismatch.

2. Stale needsFollowup=false: the linked scan checks queue ownership whenever a linked PR number exists, regardless of the cached health flag, and defers the issue if any row is present. The no-row/no-followup path still permits normal implement work. Both paths are tested with an independent issue behind the deferred one.

The original implement fallback fix remains intact. I agree with indefinite BLOCKED deferral and fail-closed needs-human label reads. All GitHub checks are green at this head.

Operational note, not a blocker: if the migration merges pre-existing duplicate rows, an in-flight worker holding a deleted row's ID will have a stale token; its settlement is rejected/skipped rather than being allowed to mutate the survivor. That is preferable to retaining two owners for the same PR, but worth being aware of during rollout.

This is a review disposition in a conversation comment only, not a GitHub approval or merge.