diff --git a/github-workflows/skills/finalize-pr/SKILL.md b/github-workflows/skills/finalize-pr/SKILL.md index ce45737..ec496cf 100644 --- a/github-workflows/skills/finalize-pr/SKILL.md +++ b/github-workflows/skills/finalize-pr/SKILL.md @@ -18,6 +18,21 @@ No manual intervention required. For manual review-focused workflows, use `/revi > asynchronously. CI may have re-run. Merge conflicts may have appeared. > Re-fetch live PR state from Step 1. +## Load-bearing rules + +These three rules govern the entire Phase 2 → Phase 3 loop. Apply them as written. + +1. **NO COMPLETION CLAIMS WITHOUT FRESH VERIFICATION EVIDENCE.** (See + `superpowers:verification-before-completion`.) Subagent self-reports are not + state. Re-query the live count from `/gh-cli-patterns` after every `/resolve-*` + call. +2. **PENDING IS NOT FAILURE.** If any required check is still running, wait — don't + abort. Phase 2.6 drains pending checks before Phase 3 fires. +3. **If a `/resolve-*` call did not strictly decrease the corresponding count, the + subagent did nothing.** Track no-ops independently per `/resolve-*` invocation + type. Two consecutive no-ops on the same fix type → exit Phase 5 with + `Blocked on: `. + ## Critical Rules 1. **Wait for user approval to merge** - Report ready status, then pause for user merge command @@ -85,6 +100,13 @@ start of each iteration. For org-wide mode, use `repository.nameWithOwner` from Steps 2.1 and 2.2 start concurrently (2.1 is non-blocking). Steps 2.3 and 2.4 run sequentially after 2.2. +### 2.0 Iteration Counter (REQUIRED) + +Initialize `phase_2_iteration = 0` at first entry to Phase 2. Increment on every +re-entry from a failed Phase 3 gate. **Hard cap: 5.** On the 6th entry, emit a +state dump (last gate result, CodeQL count, unresolved threads, CI rollup, HEAD +SHA) and exit straight to Phase 5 with `Blocked on: iteration-cap`. + ### 2.1 Start CI Monitoring (BACKGROUND) Launch CI monitoring in a background Task agent (`run_in_background: true` on the Task tool). @@ -92,6 +114,11 @@ Monitor CI checks using `--watch` so the agent blocks until all complete. Do NOT wait for the agent to finish — proceed to 2.2 immediately. +**Background-agent fallback**: if the Task agent exits without a final status +(non-zero, network error, or no result), fall back to direct polling — +`gh pr checks --watch` with a 10-minute timeout. Log the fallback +visibly; treat the direct-poll output as authoritative for Step 2.3. + ### 2.2 Parallel Fixes Run these checks simultaneously. Launch independent fixes in parallel via @@ -102,12 +129,12 @@ Task agents when they touch different files. Invoke `superpowers:dispatching-par Run the **canonical code-scanning alert count** from /gh-cli-patterns. Replace `` and `` per the placeholder convention in that skill. -**If violations found**: Invoke `/resolve-codeql fix`, validate locally. +**If violations found**: Invoke `/resolve-codeql fix`, validate locally. Verify in Step 2.2.5. #### Review Threads Invoke `/resolve-pr-threads`. It exits cleanly when no threads exist. -After completion, validate locally. +After completion, validate locally. Verify in Step 2.2.5. #### Merge Conflicts @@ -117,6 +144,19 @@ attempt merge, report unresolvable conflicts for user. After resolution, validat readiness verification (including `mergeStateStatus`, CI, CodeQL, review decision, threads) happens in Phase 3. +### 2.2.5 Post-Fix Verification (REQUIRED — applies to every `/resolve-*` call) + +After any `/resolve-codeql` or `/resolve-pr-threads` invocation, re-query the +corresponding count from `/gh-cli-patterns` (canonical code-scanning alert count, +canonical unresolved thread count). Per the load-bearing rules: + +- Count strictly decreased → continue. +- Count unchanged → the subagent did nothing. Increment that fix type's local + no-op counter. At 2 consecutive no-ops for the same fix type, exit Phase 5 + with `Blocked on: `. + +Track `codeql_noop_count` and `threads_noop_count` independently; do not merge. + ### 2.3 CI Failure Fixes Check background CI results from 2.1: @@ -138,6 +178,23 @@ and push before proceeding to 2.4. Verify final PR state, mergeability, and check status. If fixes introduced new issues, loop back to 2.2. +### 2.6 Drain Pending Checks (REQUIRED before Phase 3) + +PENDING IS NOT FAILURE. CodeQL, required-reviewer hooks, and some third-party checks complete async after push. Phase 3 must not see PENDING; drain first. + +Poll every 30 seconds until no PENDING checks remain, OR for 5 minutes: + +```bash +end=$(($(date +%s) + 300)) +while [ "$(date +%s)" -lt "$end" ]; do + pending=$(gh pr checks --json bucket --jq '[.[] | select(.bucket == "pending")] | length') + [ "$pending" = "0" ] && break + sleep 30 +done +``` + +CodeQL is NOT in `pr checks`. Also re-run the canonical code-scanning alert count from /gh-cli-patterns before advancing. + ## Phase 3: Pre-Handoff Verification > ⛔ **NO SHORT-CIRCUIT — EVERY INVOCATION, EVERY TIME.** @@ -153,7 +210,7 @@ Run the **canonical PR-readiness gate** from /gh-cli-patterns against ``. Replace ``, ``, `` per the placeholder convention in that skill. -**Required values — if any fail, return to Phase 2:** +**Required values — if any fail, loop back to Phase 2 (subject to the iteration cap from Step 2.0):** | Field | Required | Abort message | |-------|---------|---------------| @@ -171,6 +228,13 @@ that skill. > status check), `DIRTY` (conflicts), `DRAFT`, `UNKNOWN` (GitHub computing), > `UNSTABLE` (checks failed or pending). Any of these = return to Phase 2. +If a failed field is something the AI cannot fix (e.g., +`reviewDecision = REVIEW_REQUIRED`, `isDraft = true`, `state ≠ OPEN`, or +`mergeStateStatus = BLOCKED` with no resolvable cause), skip the loop and exit +Phase 5 with `Blocked on: `. Loop only when the failure has +an in-skill fix path (CI failure, CodeQL alert, unresolved thread, merge +conflict, BEHIND). + ### 3.2 CodeQL Gate (REST — separate from CI, re-run now) `statusCheckRollup` does NOT include CodeQL alert state. Run the **canonical code-scanning @@ -226,9 +290,11 @@ Proceed to Phase 5. ## Phase 5: Record Result -**Single/current-branch mode**: Emit the **Canonical PR Status Summary** (Section 1 = -this PR, Section 2 = all open PRs in current repo) as defined in /gh-cli-patterns, -titled `PR Status`. Then append: +**Single/current-branch mode**: Emit the **Canonical PR Status Summary** +(Section 1 = this PR, Section 2 = all open PRs in current repo) as defined in +/gh-cli-patterns, titled `PR Status`. + +If every gate in Phase 3 passed, the PR row in Section 1 reads `Ready for review` (no extra annotation). Then append: ```text IMPORTANT: Do NOT merge this PR. Wait for the human to review and invoke @@ -236,14 +302,27 @@ IMPORTANT: Do NOT merge this PR. Wait for the human to review and invoke /rebase-pr # Rebase commits onto main (preserves history) ``` -**Multi-PR mode**: Record the per-PR result (ready / blocked / needs-human). Restore the original -branch and continue to the next PR. Do NOT emit a ready report — that happens in Phase 6. +If any gate failed, the PR row in Section 1 includes `Blocked on: ` naming +the specific failed gate (e.g., `Blocked on: REVIEW_REQUIRED`, `Blocked on: +CodeQL`, `Blocked on: iteration-cap`, `Blocked on: threads`). State the single +manual action that would unblock it — never a generic "review manually." + +**Multi-PR mode**: Record the per-PR row (`Ready for review` or +`Blocked on: `). Restore the original branch and continue to the next PR. +Do NOT emit a ready report — that happens in Phase 6. ## Stop Condition -MUST NOT return until ALL conditions pass for EVERY targeted PR: -CI green, CodeQL clean, threads resolved, no conflicts, code simplified, local linters and tests pass, metadata updated. -If ANY fails, loop back to Phase 2. CRITICAL: CodeQL is SEPARATE from CI — check both independently. +Loop Phase 2.0 → 2.6 → 3.1 → 3.2 → 3.3 until either: + +- All three Phase 3 gates pass → run Phase 4, then Phase 5 reports `Ready for review`. +- A failed gate has no in-skill fix path (per Phase 3.1) → exit Phase 5 with `Blocked on: `. +- `phase_2_iteration` reaches 5 → exit Phase 5 with `Blocked on: iteration-cap` and emit the state dump from Step 2.0. +- A `codeql_noop_count` or `threads_noop_count` reaches 2 → exit Phase 5 with `Blocked on: `. + +Pending checks (`mergeStateStatus = UNKNOWN`, only PENDING checks in `UNSTABLE`, +or `statusCheckRollup.state = PENDING`) do NOT increment the iteration counter — +re-run Phase 2.6 and re-evaluate Phase 3. **MERGE PROHIBITION**: FORBIDDEN from merging, auto-merging, enabling auto-merge, or approving any PR. @@ -270,3 +349,4 @@ For `all`/`org` modes: Phases 2-5 loop per PR, Phase 6 aggregates results. - pr-standards (git-standards) — PR authoring and review standards - code-quality-standards (code-standards) — code quality guidelines applied during fixes - gh-cli-patterns (github-workflows) — canonical gh CLI command shapes, placeholder convention, PR gate, code-scanning query +- verification-before-completion (superpowers) — iron-law verification before claiming work complete diff --git a/github-workflows/skills/gh-cli-patterns/SKILL.md b/github-workflows/skills/gh-cli-patterns/SKILL.md index 0767404..255cc86 100644 --- a/github-workflows/skills/gh-cli-patterns/SKILL.md +++ b/github-workflows/skills/gh-cli-patterns/SKILL.md @@ -250,6 +250,26 @@ Append after the URL, separated by ` | `. Omit when no issues exist ("Ready for gh pr view --json url --jq '.url' ``` +**Canonical Unresolved Thread Count** (post-fix verification after +`/resolve-pr-threads`): + +```bash +QUERY='query($owner:String!,$repo:String!,$prNumber:Int!){ + repository(owner:$owner,name:$repo){ + pullRequest(number:$prNumber){ + reviewThreads(first:100){nodes{id isResolved}} + } + } +}' +gh api graphql -f query="$QUERY" \ + -f owner="" -f repo="" -F prNumber= \ + --jq '[.data.repository.pullRequest.reviewThreads.nodes[] + | select(.isResolved == false)] | length' +``` + +Caller compares this against the pre-fix count: strict decrease → continue, +unchanged → subagent did nothing. + **Fetch all open PRs** (for Section 2 — one GraphQL call per affected repo): `gh pr list --json` does NOT support `mergeStateStatus` — use GraphQL instead: diff --git a/github-workflows/skills/ship/SKILL.md b/github-workflows/skills/ship/SKILL.md index 3d0a69a..3ac49ca 100644 --- a/github-workflows/skills/ship/SKILL.md +++ b/github-workflows/skills/ship/SKILL.md @@ -177,18 +177,40 @@ any `reviewThreads.isResolved` = `false`, `reviewDecision` = `CHANGES_REQUESTED`/`REVIEW_REQUIRED`, `statusCheckRollup.state` ≠ `SUCCESS`, or CodeQL count > 0. -If any abort condition hits: re-invoke `/finalize-pr `, wait for completion, -then re-run both gates. Only list a PR as "Ready to merge" after both gates pass. +### 3.1 Post-Finalize Wait (REQUIRED before re-verify) -Then emit the **Canonical PR Status Summary** as defined in /gh-cli-patterns, titled -`Ship Summary`. Affected repos = current repo. Fetch each PR's full URL via: +If the most recent commit on the PR's `headRefName` landed within the last 60 seconds: ```bash -gh pr view --json url --jq '.url' +lastCommitTs=$(gh api repos///commits/ --jq '.commit.committer.date | fromdate') +now=$(date +%s) +[ $((now - lastCommitTs)) -lt 60 ] && sleep $((60 - (now - lastCommitTs))) ``` -Section 1 lists the PRs targeted by this `/ship` invocation. Section 2 lists all open -PRs in the current repo (including unrelated ones). +GitHub recomputes `mergeStateStatus` and branch protection asynchronously after a push. +A gate query within the first 60 seconds frequently returns `UNKNOWN` or stale state. +Waiting once here is dramatically cheaper than burning a `/finalize-pr` retry on a +transient pending check. + +### 3.2 Retry Cap + +If any abort condition hits: re-invoke `/finalize-pr `, wait for completion, +then re-run both gates. + +**Hard cap: 3 re-invocations per PR per `/ship` invocation.** After the 3rd, do NOT +silently report the PR as "Ready to merge." Instead, categorize the result and report +it explicitly per Step 3.3 below. + +### 3.3 Surface Per-PR Result + +Surface whatever `/finalize-pr` reported. Do not re-classify. If `/finalize-pr` +returned `Blocked on: `, the Section 1 row includes that exact string and +states the single manual action that would unblock it. + +Then emit the **Canonical PR Status Summary** as defined in /gh-cli-patterns, +titled `Ship Summary`. Affected repos = current repo. Section 1 lists the PRs +targeted by this `/ship` invocation. Section 2 lists all open PRs in the +current repo (including unrelated ones). ## Safety