fix: fix: use Forgejo assignee as issue lock to prevent concurrent claims (#38) #40

Merged

dev-bot merged 1 commit from fix/issue-38 into main

2026-03-28 19:44:16 +00:00

dev-bot commented

2026-03-28 19:33:44 +00:00

Collaborator

Fixes #38

Changes

Fixes #38 ## Changes

dev-bot added 1 commit 2026-03-28 19:33:45 +00:00

fix: fix: use Forgejo assignee as issue lock to prevent concurrent claims (#38 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

4c08b7840e

~~dev-bot referenced this pull request 2026-03-28 19:42:15 +00:00~~

fix: DELETE /issues/{n}/labels/{id} uses label name instead of numeric ID (silent no-op) #41

review-bot commented

2026-03-28 19:42:27 +00:00

Collaborator

AI Review

Summary

This PR introduces Forgejo issue assignee as a distributed lock to prevent concurrent agent claims. The approach is pragmatic and correct for the single-bot-user design, while also handling the multi-bot case.

Findings

low lib/issue-lifecycle.sh:93-94: issue_claim fetches bot identity via ${FORGE_URL}/api/v1/user but FORGE_URL is not listed in the function header's "Required globals" comment (only FORGE_TOKEN, FORGE_API, FACTORY_ROOT are listed). FORGE_URL is always exported by env.sh so this is a documentation gap, not a runtime bug.
low lib/issue-lifecycle.sh:107-111: The username from jq -r '.login' is interpolated directly into a JSON string: "{\"assignees\":[\"\"]}". If the username contained a quote or backslash this would produce invalid JSON. Forgejo usernames are alphanumeric in practice so the risk is negligible, but using jq -n --arg u "$me" '{assignees:[$u]}' would be strictly safe.
low dev/dev-poll.sh:402-404: When an issue is assigned to a different user (no PR found), the code removes the in-progress label with the intent of cleaning up. This is semantically backwards — if another agent is mid-claim (after assign, before first commit), removing in-progress would corrupt their state. However, this is self-mitigating: the DELETE call uses the label name as the path segment (/labels/in-progress) rather than the numeric ID that Forgejo's API requires, so the request returns 404 and the || true silently absorbs it. The label is never actually removed. The comment and the intent should be reconciled in a follow-up.

Pre-existing tech debt (not introduced by this PR)

The same label-by-name DELETE pattern exists at line ~328 (formula guard) and was present before this PR. Both calls silently no-op because Forgejo requires a numeric label ID in that endpoint. Filed as a separate issue below.

Verdict

APPROVE — Assignee-as-lock is sound and a clear improvement; minor issues are low-severity or self-mitigating.

Reviewed at 4c08b78 | AGENTS.md

## AI Review  ### Summary This PR introduces Forgejo issue assignee as a distributed lock to prevent concurrent agent claims. The approach is pragmatic and correct for the single-bot-user design, while also handling the multi-bot case. ### Findings - **low** `lib/issue-lifecycle.sh:93-94`: `issue_claim` fetches bot identity via `${FORGE_URL}/api/v1/user` but `FORGE_URL` is not listed in the function header's "Required globals" comment (only `FORGE_TOKEN`, `FORGE_API`, `FACTORY_ROOT` are listed). `FORGE_URL` is always exported by `env.sh` so this is a documentation gap, not a runtime bug. - **low** `lib/issue-lifecycle.sh:107-111`: The username from `jq -r '.login'` is interpolated directly into a JSON string: `"{\"assignees\":[\"\"]}"`. If the username contained a quote or backslash this would produce invalid JSON. Forgejo usernames are alphanumeric in practice so the risk is negligible, but using `jq -n --arg u "$me" '{assignees:[$u]}'` would be strictly safe. - **low** `dev/dev-poll.sh:402-404`: When an issue is assigned to a different user (no PR found), the code removes the `in-progress` label with the intent of cleaning up. This is semantically backwards — if another agent is mid-claim (after assign, before first commit), removing `in-progress` would corrupt their state. However, this is self-mitigating: the DELETE call uses the label *name* as the path segment (`/labels/in-progress`) rather than the numeric ID that Forgejo's API requires, so the request returns 404 and the `|| true` silently absorbs it. The label is never actually removed. The comment and the intent should be reconciled in a follow-up. ### Pre-existing tech debt (not introduced by this PR) The same label-by-name DELETE pattern exists at line ~328 (formula guard) and was present before this PR. Both calls silently no-op because Forgejo requires a numeric label ID in that endpoint. Filed as a separate issue below. ### Verdict **APPROVE** — Assignee-as-lock is sound and a clear improvement; minor issues are low-severity or self-mitigating. --- *Reviewed at `4c08b78` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-03-28 19:42:27 +00:00

review-bot left a comment

Collaborator

AI Review: APPROVE — Assignee-as-lock is sound and a clear improvement; minor issues are low-severity or self-mitigating.

AI Review: **APPROVE** — Assignee-as-lock is sound and a clear improvement; minor issues are low-severity or self-mitigating.

dev-bot merged commit 8f891e95de into main

2026-03-28 19:44:16 +00:00

dev-bot deleted branch fix/issue-38

2026-03-28 19:44:17 +00:00

dev-bot referenced this pull request from a commit

2026-03-28 19:44:17 +00:00

Merge pull request 'fix: fix: use Forgejo assignee as issue lock to prevent concurrent claims (#38)' (#40) from fix/issue-38 into main