feat: implement issue #405 — petry-projects — workflow failures detected 2026-05-28

.github/workflows/pr-review.yml

@@ -49,16 +49,16 @@ on:
       GH_PAT:
         required: false
 
-  # ── Reusable-only (#497 self-host pinning) ───────────────────────────────
-  # Event-driven triggering now lives in the thin caller `pr-review-trigger.yml`,
-  # which invokes this workflow pinned to `pr-review.yml@pr-review/stable` and
-  # passes `agent_ref: pr-review/stable` so the agent's own scripts run at the
-  # pinned version too. Keeping this file reusable-only is what lets
-  # .github-private's OWN PR reviews run the known-good `stable` release — not
-  # whatever is on the branch under review — which is what breaks the
-  # self-hosting circular dependency. The job below still reads `github.event.*`
-  # (inherited from the caller's triggering event) and `inputs.*` (forwarded by
-  # the stub), so its behaviour is unchanged.
+# ── Reusable-only (#497 self-host pinning) ───────────────────────────────
+# Event-driven triggering now lives in the thin caller `pr-review-trigger.yml`,
+# which invokes this workflow pinned to `pr-review.yml@pr-review/stable` and
+# passes `agent_ref: pr-review/stable` so the agent's own scripts run at the
+# pinned version too. Keeping this file reusable-only is what lets
+# .github-private's OWN PR reviews run the known-good `stable` release — not
+# whatever is on the branch under review — which is what breaks the
+# self-hosting circular dependency. The job below still reads `github.event.*`
+# (inherited from the caller's triggering event) and `inputs.*` (forwarded by
+# the stub), so its behaviour is unchanged.
 
 permissions:
   contents: read

scripts/dev-lead-intent.sh

@@ -303,6 +303,15 @@ case "$EVENT_NAME" in
       exit 0
     fi
 
+    # Skip comments on closed/merged PRs — nothing to modify on a merged branch.
+    # This is a fast-path guard; checkout_pr_in_worktree provides a deeper safety
+    # net, but failing early here avoids spending tokens on a no-op run (issue #405).
+    pr_state=$(jq -r '.issue.state // empty' "$EVENT_PATH" 2>/dev/null || true)
+    if [[ "${pr_state:-}" == "closed" ]]; then
+      emit_skip "pr-already-closed"
+      exit 0
+    fi
+
     commenter=$(jq -r '.comment.user.login // empty' "$EVENT_PATH" 2>/dev/null || true)
     comment_body=$(jq -r '.comment.body // empty' "$EVENT_PATH" 2>/dev/null || true)
     pr_number=$(jq -r '.issue.number // empty' "$EVENT_PATH" 2>/dev/null || true)

scripts/lib/pr-worktree.sh

@@ -51,6 +51,25 @@ PR_WORKTREE_AGENT_DIR=""
 checkout_pr_in_worktree() {
   local pr="$1" repo="$2"
 
+  # Exit cleanly when the PR is already merged or closed (issue #405).
+  # `gh pr checkout` fails with "fatal: couldn't find remote ref" when the
+  # branch was deleted after merging. Detect this early and skip gracefully.
+  # MUST run before the EXIT trap is registered — if exit 0 fires after the
+  # trap is set, the trap captures and replays the caller's EXIT handler
+  # prematurely (observable as bats reporting "Executed 2 instead of 1 tests").
+  local _pr_json=""
+  local _pr_state=""
+  local _pr_branch=""
+  _pr_json="$(gh pr view "$pr" --repo "$repo" --json state,headRefName 2>/dev/null || true)"
+  if [[ -n "$_pr_json" ]]; then
+    _pr_state="$(echo "$_pr_json" | jq -r '.state // empty' 2>/dev/null || true)"
+    _pr_branch="$(echo "$_pr_json" | jq -r '.headRefName // empty' 2>/dev/null || true)"
+  fi
+  if [[ -n "$_pr_state" && "$_pr_state" != "OPEN" ]]; then
+    echo "::notice::PR #${pr} is ${_pr_state} — skipping checkout (nothing to modify on a merged/closed PR)"
+    exit 0
+  fi
+
   PR_WORKTREE_AGENT_DIR="$(pwd)"
 
   # mktemp -d gives us a private parent dir; the worktree leaf must NOT pre-exist
@@ -82,8 +101,7 @@ checkout_pr_in_worktree() {
   #      and release whatever worktree currently holds it: detach the main agent
   #      checkout in place (it cannot be removed), force-remove any other.
   git worktree prune 2>/dev/null || true
-  local _pr_branch=""
-  _pr_branch="$(gh pr view "$pr" --repo "$repo" --json headRefName --jq '.headRefName' 2>/dev/null || true)"
+
   if [ -n "$_pr_branch" ]; then
     # Resolve the agent worktree's ROOT for comparison. `git worktree list`
     # emits each worktree's top-level, fully-resolved path, but

tests/dev-lead/fixtures/events/issue_comment_sonarqube_closed_pr.json

@@ -0,0 +1,23 @@
+{
+  "_test_expected_intent": "skip",
+  "action": "created",
+  "issue": {
+    "number": 519,
+    "title": "feat(pr-review): mirror dispatch inputs as workflow_call inputs",
+    "state": "closed",
+    "pull_request": {
+      "url": "https://api.github.com/repos/petry-projects/.github-private/pulls/519",
+      "merged_at": "2026-06-09T21:00:00Z"
+    }
+  },
+  "comment": {
+    "id": 4001,
+    "body": "## Quality Gate passed\n\nNo new issues.",
+    "user": {
+      "login": "sonarqubecloud[bot]",
+      "type": "Bot"
+    }
+  },
+  "repository": { "full_name": "petry-projects/.github-private" },
+  "sender": { "login": "sonarqubecloud[bot]", "type": "Bot" }
+}

tests/dev-lead/unit/test_intent_reviews.bats

@@ -273,3 +273,17 @@ EOF
   [ "$status" -eq 0 ]
   [ "$(_get_env INTENT_TYPE)" = "rebase" ]
 }
+
+@test "reviews: issue_comment trusted-bot on already-closed PR → skip pr-already-closed" {
+  # Regression guard for issue #405: a SonarQube comment on a merged PR used to
+  # trigger fix-bot-comment, which then crashed at checkout_pr_in_worktree because
+  # the branch was deleted. The classifier must emit skip before that point.
+  export GITHUB_EVENT_NAME="issue_comment"
+  export GITHUB_EVENT_PATH="$FIXTURES_DIR/issue_comment_sonarqube_closed_pr.json"
+
+  run bash "$INTENT_SCRIPT"
+
+  [ "$status" -eq 0 ]
+  [ "$(_get_env INTENT_TYPE)" = "skip" ]
+  [ "$(_get_env INTENT_REASON)" = "pr-already-closed" ]
+}

tests/dev-lead/unit/test_pr_worktree.bats

@@ -11,17 +11,21 @@ LIB="$SCRIPT_DIR/scripts/lib/pr-worktree.sh"
 
 setup() {
   STUB_BIN_DIR="$(mktemp -d)"
+  # PR_STATE_API controls the state returned by the gh stub's `pr view` JSON response.
+  # Set unconditionally so the runner environment cannot leak in and break unrelated tests.
+  export PR_STATE_API="OPEN"
+
   # gh stub: `gh pr checkout <n>` switches the worktree to the PR branch,
   # mirroring the real command (which is what made the old prompt files vanish).
-  # `gh pr view --json headRefName --jq .headRefName` returns the PR's head
-  # branch (`old` in this fixture) so the collision-prevention logic in
-  # checkout_pr_in_worktree can identify and release a stale holder (issue #470).
+  # `gh pr view` returns a JSON object with state and headRefName so the combined
+  # call in checkout_pr_in_worktree handles both the closed-PR guard (issue #405)
+  # and the stale-worktree release (issue #470) in a single round-trip.
   cat > "$STUB_BIN_DIR/gh" <<'GHEOF'
 #!/usr/bin/env bash
-case "$1 $2" in
-  "pr checkout") git checkout -q old ;;
-  "pr view") echo "old" ;;
-  *) echo "{}" ;;
+case "$*" in
+  "pr checkout"*) git checkout -q old ;;
+  "pr view"*)     echo "{\"state\": \"${PR_STATE_API:-OPEN}\", \"headRefName\": \"old\"}" ;;
+  *)              echo "{}" ;;
 esac
 GHEOF
   chmod +x "$STUB_BIN_DIR/gh"
@@ -224,3 +228,35 @@ teardown() {
   run git -C "$AGENT_REPO" symbolic-ref -q HEAD
   [ "$status" -ne 0 ]
 }
+
+# --- issue #405: merged/closed PR early-exit guard ---------------------------
+# When a trusted-bot comment arrives on an already-merged PR (branch deleted),
+# checkout_pr_in_worktree must exit 0 cleanly rather than crashing with
+# "fatal: couldn't find remote ref" from `gh pr checkout`.
+
+@test "checkout_pr_in_worktree: exits cleanly when PR is already merged (MERGED state)" {
+  source "$LIB"
+  cd "$AGENT_REPO"
+  export PR_STATE_API="MERGED"
+
+  run checkout_pr_in_worktree 519 owner/repo
+
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"PR #519 is MERGED"* ]]
+  [[ "$output" == *"skipping checkout"* ]]
+  # The agent repo must NOT have been switched to another branch by gh pr checkout.
+  [ "$(git -C "$AGENT_REPO" rev-parse --abbrev-ref HEAD)" = "main" ]
+}
+
+@test "checkout_pr_in_worktree: exits cleanly when PR is closed without merge (CLOSED state)" {
+  source "$LIB"
+  cd "$AGENT_REPO"
+  export PR_STATE_API="CLOSED"
+
+  run checkout_pr_in_worktree 99 owner/repo
+
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"PR #99 is CLOSED"* ]]
+  [[ "$output" == *"skipping checkout"* ]]
+  [ "$(git -C "$AGENT_REPO" rev-parse --abbrev-ref HEAD)" = "main" ]
+}