diff --git a/hooks/hooks.json b/hooks/hooks.json index 219bb35..6b82988 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -22,6 +22,16 @@ "timeout": 15 } ] + }, + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "'${CLAUDE_PLUGIN_ROOT}/hooks/run-hook.cmd' pretool-pr-review-reminder", + "timeout": 10 + } + ] } ], "PostToolUse": [ @@ -34,6 +44,16 @@ "timeout": 10 } ] + }, + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "'${CLAUDE_PLUGIN_ROOT}/hooks/run-hook.cmd' posttool-pr-created", + "timeout": 10 + } + ] } ], "UserPromptSubmit": [ diff --git a/hooks/posttool-pr-created b/hooks/posttool-pr-created new file mode 100755 index 0000000..8879de7 --- /dev/null +++ b/hooks/posttool-pr-created @@ -0,0 +1,74 @@ +#!/usr/bin/env bash +# hooks/posttool-pr-created +# PostToolUse hook: reminds the agent to invoke pr-monitoring after gh pr create. +# +# Fires whenever a Bash tool call contains "gh pr create". Extracts the PR +# URL from the tool response (if present) and injects an IMPORTANT reminder +# that pr-monitoring must be started immediately for the new PR. +# +# Why this exists: agents sometimes bypass the finishing-a-development-branch +# skill chain and run gh pr create directly. Without the skill chain the +# automatic "invoke pr-monitoring" step never fires, leaving CI failures and +# bot review comments unattended. This hook closes that gap by triggering +# the reminder mechanically — regardless of which code path created the PR. +# +# Global opt-out: set SUPERPOWERS_HOOKS_DISABLE=1 + +set -euo pipefail + +[ "${SUPERPOWERS_HOOKS_DISABLE:-}" = "1" ] && exit 0 + +# Require stdin (PostToolUse always sends a JSON payload). +[ -t 0 ] && exit 0 +command -v jq >/dev/null 2>&1 || exit 0 + +hook_input=$(cat || true) +[ -z "$hook_input" ] && exit 0 + +tool_name=$(printf '%s' "$hook_input" | jq -r '.tool_name // empty' 2>/dev/null || true) +[ "$tool_name" != "Bash" ] && exit 0 + +cmd=$(printf '%s' "$hook_input" | jq -r '.tool_input.command // empty' 2>/dev/null || true) +[ -z "$cmd" ] && exit 0 + +# Only act when the command created a PR (not a simple `gh pr list`, etc.). +printf '%s' "$cmd" | grep -q 'gh pr create' || exit 0 + +# Try to extract the PR URL from the tool response to confirm the PR was created. +# If no URL is found, gh pr create likely failed — do not inject the reminder. +tool_response=$(printf '%s' "$hook_input" | jq -r '.tool_response // empty' 2>/dev/null || true) +pr_url=$(printf '%s' "$tool_response" \ + | grep -oE 'https://github\.com/[^[:space:]]+/pull/[0-9]+' \ + | head -1 || true) + +# Only remind when we can confirm a PR was successfully created. +[ -z "$pr_url" ] && exit 0 + +pr_hint="for the PR you just created: ${pr_url}" + +# Escape a string for embedding inside a JSON string value. +escape_for_json() { + local s="$1" + s="${s//\\/\\\\}" + s="${s//\"/\\\"}" + s="${s//$'\n'/\\n}" + s="${s//$'\r'/\\r}" + s="${s//$'\t'/\\t}" + printf '%s' "$s" +} + +reminder="" +reminder+="You just ran \`gh pr create\`. " +reminder+="You MUST now invoke \`superpowers:pr-monitoring\` ${pr_hint}. " +reminder+="Prefer a single monitoring agent covering all PRs in this session to avoid GitHub API rate limits. " +reminder+="One agent per PR is acceptable if the PRs are on unrelated codebases or a previous shared monitor was rate-limited. " +reminder+="Draft PRs still trigger CI and receive bot reviews, so monitoring is required regardless of draft state. " +reminder+="Do NOT declare the task complete until pr-monitoring has been started. " +reminder+="See superpowers:pr-monitoring for usage." +reminder+="" + +escaped=$(escape_for_json "$reminder") + +printf '{"additional_context":"%s"}\n' "$escaped" + +exit 0 diff --git a/hooks/pretool-pr-review-reminder b/hooks/pretool-pr-review-reminder new file mode 100755 index 0000000..925676c --- /dev/null +++ b/hooks/pretool-pr-review-reminder @@ -0,0 +1,61 @@ +#!/usr/bin/env bash +# hooks/pretool-pr-review-reminder +# PreToolUse hook: reminds the agent to add Copilot as a reviewer after creating a PR. +# +# When an agent is about to run gh pr create, inject a reminder about: +# 1. gh CLI version requirement (v2.88+) +# 2. Adding Copilot as a reviewer via `gh pr edit --add-reviewer copilot-pull-request-reviewer` +# immediately after the PR is created (not as a flag to gh pr create). +# +# Global opt-out: set SUPERPOWERS_HOOKS_DISABLE=1 + +set -euo pipefail + +[ "${SUPERPOWERS_HOOKS_DISABLE:-}" = "1" ] && exit 0 + +# Require stdin (PreToolUse always sends a JSON payload). +[ -t 0 ] && exit 0 +command -v jq >/dev/null 2>&1 || exit 0 + +hook_input=$(cat || true) +[ -z "$hook_input" ] && exit 0 + +tool_name=$(printf '%s' "$hook_input" | jq -r '.tool_name // empty' 2>/dev/null || true) +[ "$tool_name" != "Bash" ] && exit 0 + +cmd=$(printf '%s' "$hook_input" | jq -r '.tool_input.command // empty' 2>/dev/null || true) +[ -z "$cmd" ] && exit 0 + +# Only act on PR creation commands. +printf '%s' "$cmd" | grep -q 'gh pr create' || exit 0 + +# Escape a string for embedding inside a JSON string value. +escape_for_json() { + local s="$1" + s="${s//\\/\\\\}" + s="${s//\"/\\\"}" + s="${s//$'\n'/\\n}" + s="${s//$'\r'/\\r}" + s="${s//$'\t'/\\t}" + printf '%s' "$s" +} + +reminder=$(cat <<'REMINDER' + +Before and after running `gh pr create`: +(1) Ensure `gh` is version 2.88 or higher (`gh --version`). + If it is older, upgrade gh (e.g. `gh upgrade` or follow https://cli.github.com/manual/) + or inform the user that a manual upgrade is needed. +(2) After the PR is created, add Copilot as a reviewer by running: + gh pr edit --add-reviewer copilot-pull-request-reviewer + Do NOT use `--reviewer Copilot` as a flag to `gh pr create` — + the correct method is `gh pr edit --add-reviewer copilot-pull-request-reviewer` post-creation. + +REMINDER +) + +escaped=$(escape_for_json "$reminder") + +printf '{"additional_context":"%s"}\n' "$escaped" + +exit 0 diff --git a/skills/finishing-a-development-branch/SKILL.md b/skills/finishing-a-development-branch/SKILL.md index 2a3cec7..119448f 100644 --- a/skills/finishing-a-development-branch/SKILL.md +++ b/skills/finishing-a-development-branch/SKILL.md @@ -61,7 +61,7 @@ When running in the autonomous pipeline (invoked from subagent-driven-developmen EOF )" ``` -5. **Invoke pr-monitoring** — spawn one background monitor per PR created +5. **Invoke pr-monitoring** — spawn a background monitor for all PRs created in this session; prefer a single agent covering all PRs to avoid GitHub API rate limits, but one agent per PR is acceptable if the PRs are on unrelated codebases or a previous shared monitor was rate-limited 6. **Report PR URLs** — output every PR link for the user (one per row in the manifest's PR Grouping table) **Do NOT:** diff --git a/skills/pr-monitoring/SKILL.md b/skills/pr-monitoring/SKILL.md index fb17911..aefa7db 100644 --- a/skills/pr-monitoring/SKILL.md +++ b/skills/pr-monitoring/SKILL.md @@ -7,7 +7,7 @@ description: Use after creating a PR to automatically monitor CI checks and revi ## Overview -Monitor a pull request for CI failures and review comments, automatically fixing issues and pushing updates. Runs as a background agent after autonomous PR creation. +Monitor a pull request (or a set of PRs) for CI failures and review comments, automatically fixing issues and pushing updates. Runs as a background agent after autonomous PR creation. **Core principle:** The PR is the final quality gate. Monitor it, fix what breaks, respond to feedback — all without human intervention. @@ -15,96 +15,232 @@ Monitor a pull request for CI failures and review comments, automatically fixing Invoked automatically by `finishing-a-development-branch` in autonomous mode after creating a PR. Can also be invoked manually for any open PR. +## Single Agent vs. One Agent per PR + +When multiple PRs were created in the same session, prefer launching **one monitor agent that covers all PRs** to reduce GitHub API request volume. You may launch one agent per PR instead if the PRs are on unrelated codebases, if you anticipate heavy parallel CI load, or if a previous shared monitor was rate-limited. Default to the single-agent approach and only deviate with a reason. + ## The Process -Run a `balanced`-tier agent that monitors the PR in a loop until all CI checks pass and no unresolved reviews remain. +Run a `balanced`-tier agent that monitors PRs in a loop for up to **60 minutes**, polling every **10 minutes**, until all CI checks pass and no unresolved reviews remain. Use the Agent tool to run the monitor in the background: ```` Agent tool (general-purpose, model: balanced, run_in_background: true): - description: "Monitor PR #N for CI and reviews" + description: "Monitor PR(s) for CI and reviews" prompt: | - You are monitoring PR # on and automatically fixing issues. + You are monitoring one or more pull requests and automatically fixing issues. ## Setup - PR URL: - Branch: - Base: - Design doc: - Plan doc: + PRs to monitor (repeat the loop below for each): + | PR # | URL | Branch | Base | Design doc | Plan doc | + |------|-----|--------|------|------------|----------| + | | | | | | | + + ## Worktree Setup + + Before starting the loop, ensure every branch has its own git worktree so + you never need to `git checkout` mid-session (which would discard in-flight + changes on the current branch): + + ```bash + # For each PR branch that doesn't already have a worktree: + git worktree add ../-monitor + ``` + + When working on a fix for PR #N, `cd` into that branch's worktree directory. + Always confirm which branch you are on with `git branch --show-current` before + making any edits, commits, or pushes. + + ## Session Limits - Repeat the Monitor Loop until exit conditions are met: + - **Maximum session duration:** 60 minutes from when this agent starts. + - **Poll interval:** 10 minutes between full check cycles across all PRs. + - Track elapsed time; when fewer than 10 minutes remain, finish the current + cycle and then write the timeout status report (see step 3) before exiting. - ### 1. Check CI Status + ## Monitor Loop + + Repeat until exit conditions are met or the 60-minute session limit is reached: + + ### 1. Check CI Status (per PR) + + For each PR: ```bash - gh pr checks --json name,state,conclusion + gh pr checks --json name,state,conclusion,detailsUrl ``` **If any check fails:** - a. Read the failure logs: `gh run view --log-failed` - b. Identify the root cause - c. Fix the issue in the codebase - d. Run the relevant tests locally to verify - e. Commit and push: + a. Extract the run ID from the `detailsUrl` field of the failing check + (the URL has the form `.../runs//...`): + ```bash + gh pr checks --json name,state,conclusion,detailsUrl \ + | jq '.[] | select(.conclusion == "FAILURE") | .detailsUrl' + # run-id is the numeric segment after /runs/ in that URL + ``` + b. Read the failure logs: `gh run view --log-failed` + c. Identify the root cause + d. `cd` into that PR's worktree directory and confirm the branch: + ```bash + cd ../-monitor + git branch --show-current # must equal + ``` + e. Fix the issue in the codebase + f. Run the relevant tests locally to verify + g. Commit and push: ```bash git add git commit -m "fix: address CI failure in " git push ``` - f. Wait 60s, re-check + h. Return to the next PR in the loop **Safety:** Max 5 fix attempts per unique CI failure. After 5, comment on the PR: "Unable to automatically resolve CI failure in after 5 attempts. Manual intervention needed." - ### 2. Check Review Comments + ### 2. Check Review Comments (per PR) + + Use GraphQL to fetch review threads — this gives both the thread IDs needed + for resolution and the author login and comment IDs needed to reply in-thread: + + ```bash + gh api graphql -f query=' + query($owner: String!, $repo: String!, $number: Int!) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) { + reviewThreads(first: 100) { + nodes { + id + isResolved + isOutdated + comments(first: 1) { + nodes { + databaseId + url + author { login } + body + } + } + } + } + } + } + } + ' -f owner= -f repo= -F number= + ``` + To reply to a review thread, use the REST API with the `databaseId` of the + first comment in the thread (the `in_reply_to` parameter): ```bash - gh api repos///pulls//comments --jq '.[] | select(.position != null) | {id, body, path, line: .original_line, user: .user.login}' - gh api repos///pulls//reviews --jq '.[] | select(.state == "CHANGES_REQUESTED") | {id, body, user: .user.login}' + gh api repos///pulls//comments \ + --method POST \ + --field body="Addressed in " \ + --field in_reply_to= ``` - **If new unresolved comments found:** + Also fetch any "CHANGES_REQUESTED" reviews: + + ```bash + gh api repos///pulls//reviews \ + --jq '.[] | select(.state == "CHANGES_REQUESTED") | {id, body, user: .user.login}' + ``` + + **Bot detection:** treat a commenter as a bot if their login ends in `[bot]` + or matches a known review bot (e.g. `copilot-pull-request-reviewer`, + `github-advanced-security`, `datadog`, `codeclimate`, `sonarcloud`). Apply + the same address-then-resolve flow to bot comments as to human comments. + + **If new unresolved, non-outdated threads are found:** a. Read the comment carefully - b. Implement the requested change - c. Run tests to verify - d. Commit and push - e. Reply to the comment: "Addressed in " + b. `cd` into that PR's worktree directory and confirm the branch: + ```bash + cd ../-monitor + git branch --show-current # must equal + ``` + c. Implement the requested change + d. Run tests to verify + e. Commit and push + f. Reply to the comment: "Addressed in " + g. **Resolve the thread** after verification unless the max revision rounds + safety limit was exceeded (required for addressed comments, especially bot comments): + ```bash + gh api graphql -f query=' + mutation($threadId: ID!) { + resolveReviewThread(input: {threadId: $threadId}) { + thread { isResolved } + } + } + ' -f threadId="" + ``` **Safety:** Max 3 revision rounds per review comment. After 3, reply: "I've attempted to address this feedback but may need clarification. Flagging for manual review." + Leave the thread **unresolved** — do NOT resolve it, as that would mask the outstanding concern. + It will be surfaced in the timeout status report as needing manual intervention. ### 3. Check Exit Conditions - Exit when ALL of: + A PR is **complete** when ALL of: - All CI checks passing (green) - - No unresolved review comments + - No unresolved, non-outdated review threads - No pending "changes requested" reviews - On exit: - - If the PR is merged AND base-branch CI is green for the merge commit AND a design + plan exist in `docs/plans/` for this branch, invoke `superpowers:post-merge-retrospective` to produce a retro in `docs/retros/`. This is the autonomous closing-the-loop step. - - If the PR is closed without merge, skip the retrospective and exit cleanly. - - Report final status either way. + **On all PRs complete (clean exit):** + - For each PR that is merged AND whose base-branch CI is green for the merge + commit AND that has a design + plan in `docs/plans/` for its branch, invoke + `superpowers:post-merge-retrospective` to produce a retro in `docs/retros/`. + - Report final status. + + **On session timeout (60-minute limit reached):** + Write a status report to stdout in this format so the orchestrator can decide + whether to restart: - ### 4. Wait Between Checks + ``` + PR-MONITOR TIMEOUT REPORT + Elapsed: ~60 min + PRs still needing attention: + - PR # : + PRs complete: + - PR # : all checks green, no open threads + Action required: restart superpowers:pr-monitoring for the PRs listed above. + ``` - Sleep 60 seconds between check cycles. Do not poll more frequently. + Then exit. The orchestrator (lead agent) will read this report via the + activity log and start a new monitor agent for the remaining PRs. + + ### 4. Wait Between Cycles + + Sleep **10 minutes** (600 seconds) between full check cycles across all PRs. + Do not poll more frequently — this keeps GitHub API usage well within limits. ```` -Use your host's equivalent mechanism to periodically poll the following in a loop: -- `gh pr checks ` — fix any failing CI checks -- `gh api repos///pulls//comments` — respond to inline review comments +Use your host's equivalent mechanism to periodically poll the following in a loop, +with a **10-minute** wait between full cycles and a **60-minute** total session cap: + +- `gh pr checks ` — fix any failing CI checks; use a separate worktree + per branch so you never switch branches mid-session +- GraphQL `reviewThreads` query — fetch unresolved, non-outdated threads; address + each one, reply "Addressed in ", then resolve the thread via the + `resolveReviewThread` GraphQL mutation unless the 3 revision-round safety limit + was exceeded. Apply this to bot comments (any login ending in `[bot]`, or known + bots such as `copilot-pull-request-reviewer`, `github-advanced-security`, + `datadog`) the same as human comments. - `gh api repos///pulls//reviews` — handle any "CHANGES_REQUESTED" reviews -Continue until all checks pass, no unresolved inline comments remain, and no "changes requested" reviews are pending. +When the 60-minute limit is reached before all PRs are clean, write a timeout +status report listing which PRs still need attention, then exit. The orchestrator +should start a new monitor for the remaining PRs. -When the PR has merged with green base-branch CI and a design + plan exist in `docs/plans/` for this branch, invoke `superpowers:post-merge-retrospective` to write a retro in `docs/retros/`. If the PR was closed without merge, skip the retro and exit cleanly. +When a PR has merged with green base-branch CI and a design + plan exist in +`docs/plans/` for its branch, invoke `superpowers:post-merge-retrospective` to +write a retro in `docs/retros/`. If the PR was closed without merge, skip the +retro and exit cleanly. @@ -114,10 +250,18 @@ When the PR has merged with green base-branch CI and a design + plan exist in `d | Limit | Value | On Exceed | |-------|-------|-----------| | CI fix attempts per failure | 5 | Comment on PR, stop fixing that check | -| Revision rounds per comment | 3 | Reply with escalation, stop revising | -| Total monitoring duration | 30 min | Exit with status report | +| Revision rounds per comment | 3 | Reply with escalation, leave thread unresolved, surface in timeout report | +| Total monitoring duration | 60 min | Write timeout status report; orchestrator restarts a new monitor | +| Poll interval | 10 min (600s) | — do not poll more frequently | | Push frequency | Max 1 per 60s | Queue fixes, batch push | +## Orchestrator Restart Guidance + +When the monitor agent times out (60 min), it writes a `PR-MONITOR TIMEOUT REPORT` +to its output. The orchestrator (lead agent) should watch for this via the activity +log, read the report, and start a new `superpowers:pr-monitoring` agent scoped to +only the PRs still listed as needing attention. Repeat until all PRs are clean. + ## Integration **Called by:**