diff --git a/lib/hire-agent.sh b/lib/hire-agent.sh index 2bbea63..49ab8ae 100644 --- a/lib/hire-agent.sh +++ b/lib/hire-agent.sh @@ -229,37 +229,6 @@ disinto_hire_an_agent() { export "${pass_var}=${user_pass}" fi - # Step 1.6: Add the new agent as a write collaborator on the project repo (#856). - # Without this, PATCH /issues/{n} {assignees:[agent]} returns 403 Forbidden and - # the dev-agent polls forever logging "claim lost to — skipping" (see - # issue_claim()'s post-PATCH verify). Mirrors the collaborator setup applied - # to the canonical bot users in lib/forge-setup.sh. Idempotent: Forgejo's PUT - # returns 204 whether the user is being added for the first time or already a - # collaborator at the same permission. - if [ -n "${FORGE_REPO:-}" ]; then - echo "" - echo "Step 1.6: Adding '${agent_name}' as write collaborator on '${FORGE_REPO}'..." - local collab_code - collab_code=$(curl -s -o /dev/null -w '%{http_code}' -X PUT \ - -H "Authorization: token ${admin_token}" \ - -H "Content-Type: application/json" \ - "${forge_url}/api/v1/repos/${FORGE_REPO}/collaborators/${agent_name}" \ - -d '{"permission":"write"}') - case "$collab_code" in - 204|201|200) - echo " ${agent_name} is a write collaborator on ${FORGE_REPO} (HTTP ${collab_code})" - ;; - *) - echo " Warning: failed to add '${agent_name}' as collaborator on '${FORGE_REPO}' (HTTP ${collab_code})" >&2 - echo " The agent will not be able to claim issues until this is fixed." >&2 - ;; - esac - else - echo "" - echo "Step 1.6: FORGE_REPO not set — skipping collaborator step" >&2 - echo " Warning: the agent will not be able to claim issues on the project repo" >&2 - fi - # Step 2: Create .profile repo on Forgejo echo "" echo "Step 2: Creating '${agent_name}/.profile' repo (if not exists)..." diff --git a/lib/issue-lifecycle.sh b/lib/issue-lifecycle.sh index 1ad3239..80f9afa 100644 --- a/lib/issue-lifecycle.sh +++ b/lib/issue-lifecycle.sh @@ -126,21 +126,11 @@ issue_claim() { # Assign to self BEFORE adding in-progress label (issue #471). # This ordering ensures the assignee is set by the time other pollers # see the in-progress label, reducing the stale-detection race window. - # - # Capture the HTTP status instead of silently swallowing failures (#856). - # A 403 here means the bot user is not a write collaborator on the repo — - # previously the silent failure fell through to the post-PATCH verify which - # only reported "claim lost to ", hiding the real root cause. - local patch_code - patch_code=$(curl -s -o /dev/null -w '%{http_code}' -X PATCH \ + curl -sf -X PATCH \ -H "Authorization: token ${FORGE_TOKEN}" \ -H "Content-Type: application/json" \ "${FORGE_API}/issues/${issue}" \ - -d "{\"assignees\":[\"${me}\"]}") - if [ "$patch_code" != "201" ] && [ "$patch_code" != "200" ]; then - _ilc_log "issue #${issue} PATCH assignee failed: HTTP ${patch_code} (403 = missing write collaborator permission on ${FORGE_REPO:-repo})" - return 1 - fi + -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 diff --git a/tests/lib-issue-claim.bats b/tests/lib-issue-claim.bats index 85bcc83..d7a2c91 100644 --- a/tests/lib-issue-claim.bats +++ b/tests/lib-issue-claim.bats @@ -52,13 +52,12 @@ setup() { # canned responses per endpoint. Every call gets logged as # `METHOD URL` (one line) to $CALLS_LOG for later grep-based asserts. curl() { - local method="GET" url="" arg want_code="" + local method="GET" url="" arg while [ $# -gt 0 ]; do arg="$1" case "$arg" in -X) method="$2"; shift 2 ;; -H|-d|--data-binary|-o) shift 2 ;; - -w) want_code="$2"; shift 2 ;; -sf|-s|-f|--silent|--fail) shift ;; *) url="$arg"; shift ;; esac @@ -90,13 +89,7 @@ setup() { fi ;; "PATCH ${FORGE_API}/issues/"*) - # Accept any PATCH; body ignored. When caller asked for the HTTP - # status via `-w '%{http_code}'` (issue_claim does this since #856 - # to surface 403s from missing collaborator permission), emit the - # code configured by the scenario (default 200). - if [ "$want_code" = '%{http_code}' ]; then - printf '%s' "${MOCK_PATCH_CODE:-200}" - fi + : # accept any PATCH; body is ignored by the mock ;; "GET ${FORGE_API}/labels") printf '[]' @@ -172,28 +165,6 @@ count_calls() { [ "$(count_calls GET "${FORGE_API}/labels")" -eq 0 ] } -# ── PATCH HTTP error surfacing (#856) ─────────────────────────────────────── - -@test "issue_claim logs specific HTTP code on PATCH failure (403 = missing collaborator)" { - export MOCK_ME="bot" - export MOCK_INITIAL_ASSIGNEE="" - export MOCK_RECHECK_ASSIGNEE="" - export MOCK_PATCH_CODE="403" - - run issue_claim 42 - [ "$status" -eq 1 ] - - # The new log message names the HTTP code explicitly — without this, - # a missing-collaborator setup (#856) falls through to the post-PATCH - # verify and masquerades as "claim lost to ". - [[ "$output" == *"PATCH assignee failed: HTTP 403"* ]] - - # No re-read on PATCH failure (we bail before reaching the verify step). - [ "$(count_calls GET "${FORGE_API}/issues/42")" -eq 1 ] - [ "$(count_calls PATCH "${FORGE_API}/issues/42")" -eq 1 ] - [ "$(count_calls POST "${FORGE_API}/issues/42/labels")" -eq 0 ] -} - # ── pre-check skip ────────────────────────────────────────────────────────── @test "issue_claim skips early (no PATCH) when pre-check shows another assignee" {