fix: issue_claim race — verify assignee after PATCH to prevent duplicate work (#830)
All checks were successful
ci/woodpecker/push/ci Pipeline was successful
ci/woodpecker/pr/ci Pipeline was successful
ci/woodpecker/pr/smoke-init Pipeline was successful

Forgejo's assignees PATCH is last-write-wins, so two dev agents polling
concurrently could both observe .assignee == null at the pre-check, both
PATCH, and the loser would silently "succeed" and proceed to implement
the same issue — colliding at the PR/branch stage.

Re-read the assignee after the PATCH and bail out if it isn't self.
Label writes are moved AFTER this verification so a losing claim leaves
no stray in-progress label to roll back.

Adds tests/lib-issue-claim.bats covering the three paths:
  - happy path (single agent, re-read confirms self)
  - lost race (re-read shows another agent — returns 1, no labels added)
  - pre-check skip (initial GET already shows another agent)

Prerequisite for the LLAMA_BOTS parametric refactor that will run N
dev containers against the same project.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude 2026-04-16 08:35:18 +00:00
parent 2a7ae0b7ea
commit 620515634a
2 changed files with 198 additions and 0 deletions

View file

@ -132,6 +132,21 @@ issue_claim() {
"${FORGE_API}/issues/${issue}" \
-d "{\"assignees\":[\"${me}\"]}" >/dev/null 2>&1 || return 1
# Verify the PATCH stuck. Forgejo's assignees PATCH is last-write-wins, so
# under concurrent claims from multiple dev agents two invocations can both
# see .assignee == null at the pre-check, both PATCH, and the loser's write
# gets silently overwritten (issue #830). Re-reading the assignee closes
# that TOCTOU window: only the actual winner observes its own login.
# Labels are intentionally applied AFTER this check so the losing claim
# leaves no stray "in-progress" label to roll back.
local actual
actual=$(curl -sf -H "Authorization: token ${FORGE_TOKEN}" \
"${FORGE_API}/issues/${issue}" | jq -r '.assignee.login // ""') || return 1
if [ "$actual" != "$me" ]; then
_ilc_log "issue #${issue} claim lost to ${actual:-<none>} — skipping"
return 1
fi
local ip_id bl_id
ip_id=$(_ilc_in_progress_id)
bl_id=$(_ilc_backlog_id)