fix: bug: hire-an-agent does not add the new agent as collaborator on the project repo (#856) #858
3 changed files with 74 additions and 4 deletions
|
|
@ -229,6 +229,37 @@ disinto_hire_an_agent() {
|
||||||
export "${pass_var}=${user_pass}"
|
export "${pass_var}=${user_pass}"
|
||||||
fi
|
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 <none> — 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
|
# Step 2: Create .profile repo on Forgejo
|
||||||
echo ""
|
echo ""
|
||||||
echo "Step 2: Creating '${agent_name}/.profile' repo (if not exists)..."
|
echo "Step 2: Creating '${agent_name}/.profile' repo (if not exists)..."
|
||||||
|
|
|
||||||
|
|
@ -126,11 +126,21 @@ issue_claim() {
|
||||||
# Assign to self BEFORE adding in-progress label (issue #471).
|
# Assign to self BEFORE adding in-progress label (issue #471).
|
||||||
# This ordering ensures the assignee is set by the time other pollers
|
# This ordering ensures the assignee is set by the time other pollers
|
||||||
# see the in-progress label, reducing the stale-detection race window.
|
# see the in-progress label, reducing the stale-detection race window.
|
||||||
curl -sf -X PATCH \
|
#
|
||||||
|
# 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 <none>", hiding the real root cause.
|
||||||
|
local patch_code
|
||||||
|
patch_code=$(curl -s -o /dev/null -w '%{http_code}' -X PATCH \
|
||||||
-H "Authorization: token ${FORGE_TOKEN}" \
|
-H "Authorization: token ${FORGE_TOKEN}" \
|
||||||
-H "Content-Type: application/json" \
|
-H "Content-Type: application/json" \
|
||||||
"${FORGE_API}/issues/${issue}" \
|
"${FORGE_API}/issues/${issue}" \
|
||||||
-d "{\"assignees\":[\"${me}\"]}" >/dev/null 2>&1 || return 1
|
-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
|
||||||
|
|
||||||
# Verify the PATCH stuck. Forgejo's assignees PATCH is last-write-wins, so
|
# Verify the PATCH stuck. Forgejo's assignees PATCH is last-write-wins, so
|
||||||
# under concurrent claims from multiple dev agents two invocations can both
|
# under concurrent claims from multiple dev agents two invocations can both
|
||||||
|
|
|
||||||
|
|
@ -52,12 +52,13 @@ setup() {
|
||||||
# canned responses per endpoint. Every call gets logged as
|
# canned responses per endpoint. Every call gets logged as
|
||||||
# `METHOD URL` (one line) to $CALLS_LOG for later grep-based asserts.
|
# `METHOD URL` (one line) to $CALLS_LOG for later grep-based asserts.
|
||||||
curl() {
|
curl() {
|
||||||
local method="GET" url="" arg
|
local method="GET" url="" arg want_code=""
|
||||||
while [ $# -gt 0 ]; do
|
while [ $# -gt 0 ]; do
|
||||||
arg="$1"
|
arg="$1"
|
||||||
case "$arg" in
|
case "$arg" in
|
||||||
-X) method="$2"; shift 2 ;;
|
-X) method="$2"; shift 2 ;;
|
||||||
-H|-d|--data-binary|-o) shift 2 ;;
|
-H|-d|--data-binary|-o) shift 2 ;;
|
||||||
|
-w) want_code="$2"; shift 2 ;;
|
||||||
-sf|-s|-f|--silent|--fail) shift ;;
|
-sf|-s|-f|--silent|--fail) shift ;;
|
||||||
*) url="$arg"; shift ;;
|
*) url="$arg"; shift ;;
|
||||||
esac
|
esac
|
||||||
|
|
@ -89,7 +90,13 @@ setup() {
|
||||||
fi
|
fi
|
||||||
;;
|
;;
|
||||||
"PATCH ${FORGE_API}/issues/"*)
|
"PATCH ${FORGE_API}/issues/"*)
|
||||||
: # accept any PATCH; body is ignored by the mock
|
# 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
|
||||||
;;
|
;;
|
||||||
"GET ${FORGE_API}/labels")
|
"GET ${FORGE_API}/labels")
|
||||||
printf '[]'
|
printf '[]'
|
||||||
|
|
@ -165,6 +172,28 @@ count_calls() {
|
||||||
[ "$(count_calls GET "${FORGE_API}/labels")" -eq 0 ]
|
[ "$(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 <none>".
|
||||||
|
[[ "$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 ──────────────────────────────────────────────────────────
|
# ── pre-check skip ──────────────────────────────────────────────────────────
|
||||||
|
|
||||||
@test "issue_claim skips early (no PATCH) when pre-check shows another assignee" {
|
@test "issue_claim skips early (no PATCH) when pre-check shows another assignee" {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue