From c23b4ab4ce523a96703feac9f657c4a2aebc0d67 Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Sat, 15 Aug 2026 00:56:35 +0200 Subject: [PATCH 1/4] feat: sweep open PRs for CI and conflicts in implement-issue Step 4 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 4 only pruned worktrees whose PR had merged. It never looked at an open PR, so it could not see either way a parked PR rots: red CI, or a branch that went CONFLICTING because other PRs merged into main ahead of it. A CONFLICTING PR creates no workflow run at all, so it presents as "CI never fired" and nobody investigates. Step 4 becomes 4a (fleet sweep) + 4b (worktree + branch). The sweep keeps the merged-prune arm unchanged and adds an open-PR arm that merges origin/main into stale branches, auto-resolves mechanical conflicts only, and reports everything else. A skip gate runs before both arms so the sweep never touches a worktree another agent owns — any live session at that cwd, uncommitted tracked changes, or a HEAD under 30 minutes old. The same section is what to run under /loop for continuous fleet maintenance, rather than forking a second skill that drifts. Step 9 gains the matching pre-push `git merge origin/main`: Step 4b cuts the branch from a current origin/main, but Steps 5-8 take hours and other PRs merge during them. Two behaviours found by dry-running the sweep against the real fleet: GitHub computes `mergeable` lazily, so the first query on a cold PR returns UNKNOWN and only triggers the computation — a single pass reports UNKNOWN for precisely the stale PRs the sweep exists to find, hence the retry. And 11 worktrees hold branches with local commits and no PR; those are reported, never deleted. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MYV1WdkadzkeUUgULhC3Dk --- .claude/skills/implement-issue/SKILL.md | 156 +++++++++++++++++++----- 1 file changed, 125 insertions(+), 31 deletions(-) diff --git a/.claude/skills/implement-issue/SKILL.md b/.claude/skills/implement-issue/SKILL.md index 27f0072e..7c2a396b 100644 --- a/.claude/skills/implement-issue/SKILL.md +++ b/.claude/skills/implement-issue/SKILL.md @@ -41,12 +41,12 @@ runners — only repo-level `.claude/skills/` and `.claude/agents/` exist there. |---|---| | 2. Diagnose | Stage 2 comment absent → STOP. Post "No deep analysis found. Run `@claude-bot analyze` first" and exit — never self-diagnose in CI; the analyze/fix split *is* the human gate. | | 3. Confirm gate | The owner's `@claude-bot fix` comment is the go-ahead. Still perform the workaround check and scope assessment — put them in a `## Scope assessment` section of the PR body instead of chat. Escalation path (can't confidently pass the workaround check) still applies: dispatch a fresh general-purpose `Agent` to critique the design before implementing. | -| 4. Worktree | Skip — the CI checkout is already isolated. Create the branch directly (naming per Step 1). | +| 4. Fleet sweep + worktree | Skip both — the CI checkout is already isolated and has no worktree fleet to sweep. Create the branch directly (naming per Step 1). | | 5. TDD | The substance applies verbatim (RED test first, required test shape); there is just no `superpowers:test-driven-development` skill to invoke — follow this section's own rules. | | 6. Quality gate + code review | Run inline, no background agent (CI is one throwaway session — the cost-discipline reason to background doesn't exist). The `code-review` plugin is unavailable; the Stage-4 `@claude-bot` PR review covers it. Checks 1–3 (fast suite, slow suite, required-test-shape) still apply. | | 7. Confirm gate 2 | Replaced by the draft PR itself — the owner reviews the draft before anything merges. | | 8. Local run & observe | Structurally unavailable in CI — this is the documented reason the local flow exists. Skip, and say so in the PR body's test plan so the reviewer knows verification is still owed. | -| 9. Commit + draft PR | Applies verbatim, including the `CHANGELOG.md` `## [Unreleased]` entry and the documentation check. Add the `## Scope assessment` section (Step 3 above). The workflow file owns CI-only mechanics: issue comment with the PR link, `has-fix-pr` label. | +| 9. Commit + draft PR | Applies verbatim, including the pre-push `git merge origin/main` and the `CHANGELOG.md` `## [Unreleased]` entry and the documentation check. Add the `## Scope assessment` section (Step 3 above). The workflow file owns CI-only mechanics: issue comment with the PR link, `has-fix-pr` label. | | 10. Hard constraints | Apply verbatim. | ## Process @@ -96,45 +96,117 @@ owner is not ready to present. Wait for explicit go-ahead before touching code. One message — cheap insurance against building an entire implementation on a wrong diagnosis *or* a wrong placement. -### 4. Worktree + branch +### 4. Fleet sweep, then worktree + branch -**Prune merged worktrees FIRST — this is the cleanup step, and it lives here -on purpose.** The "After Merge" section at the bottom also removes a -worktree, but it is defined as a separate later invocation, so it depends on -someone choosing to come back — and nobody does. That postcondition ran zero -times in ~40 issues and left 39 worktrees on disk, 24 of them long merged. -Running the prune as a *precondition* of the next issue needs no memory: the -next person to do issue work cleans up the last one's mess automatically. +#### 4a. Fleet sweep (runs FIRST, before creating anything) + +**This is the cleanup-and-refresh step, and it lives here on purpose.** The +"After Merge" section at the bottom also removes a worktree, but it is +defined as a separate later invocation, so it depends on someone choosing to +come back — and nobody does. That postcondition ran zero times in ~40 issues +and left 39 worktrees on disk, 24 of them long merged. Running the sweep as a +*precondition* of the next issue needs no memory: the next person to do issue +work cleans up the last one's mess automatically. + +The sweep has two arms, because pruning merged worktrees alone leaves the +other half of the fleet to rot. A PR that was green and mergeable when its +session ended goes `CONFLICTING` the moment two or three others merge into +`main` ahead of it — and a `CONFLICTING` PR **creates no workflow run at +all**, so it presents as "CI never fired", not as a conflict. Nobody is +watching for that, which is exactly why it has to be swept rather than +noticed. + +**The skip gate comes before both arms.** Another agent may be mid-`implement-issue` +in any of these worktrees; merging `origin/main` underneath it moves its HEAD +mid-run and puts two sessions on the same branch pushing to the same PR. ```bash -# Worktrees whose PR has merged. NOTE: `git branch --merged` / `rev-list -# origin/main..branch` DO NOT WORK here -- this repo squash-merges, so a -# merged branch's commits are never reachable from main and every worktree -# looks unmerged forever. PR state is the only authoritative signal. +git fetch origin --prune +# NOTE: `git branch --merged` / `rev-list origin/main..branch` DO NOT WORK +# here -- this repo squash-merges, so a merged branch's commits are never +# reachable from main and every worktree looks unmerged forever. PR state is +# the only authoritative signal. merged=$(gh pr list --state merged --limit 200 --json headRefName -q '.[].headRefName') +# Live sessions own their worktree at ANY status -- idle and blocked included. +# UNSCOPED on purpose: sibling worktrees never appear in the project-scoped +# view (CLAUDE.md, Worktree Conventions), and missing one is the collision. +owned=$(cd ~ && claude agents --json 2>/dev/null | jq -r '.[].cwd') + git worktree list | awk 'NR>1 {print $1}' | while read -r wt; do b=$(git -C "$wt" branch --show-current 2>/dev/null) - [ -n "$b" ] || continue # detached: leave alone - echo "$merged" | grep -qx "$b" || continue # not merged: leave alone - if [ -n "$(git -C "$wt" status --porcelain -uno)" ]; then # tracked edits: never auto-delete - echo "KEEP (uncommitted changes): $wt"; continue + [ -n "$b" ] || continue # detached: leave alone + if echo "$owned" | grep -qF "$wt"; then + echo "SKIP (live session): $wt"; continue + fi + if [ -n "$(git -C "$wt" status --porcelain -uno)" ]; then # tracked edits: never touch + echo "SKIP (uncommitted changes): $wt"; continue + fi + if echo "$merged" | grep -qx "$b"; then # arm 1: merged -> prune + git worktree remove "$wt" && git branch -D "$b"; continue + fi + age=$(( ($(date +%s) - $(git -C "$wt" log -1 --format=%ct)) / 60 )) + if [ "$age" -lt 30 ]; then + echo "SKIP (HEAD ${age}m old, likely active): $wt"; continue fi - git worktree remove "$wt" && git branch -D "$b" + # arm 2: open -> refresh. GitHub computes `mergeable` LAZILY: the first + # query on a cold PR returns UNKNOWN *and* triggers the computation, so a + # single pass reports UNKNOWN for every stale PR -- i.e. exactly the ones + # the sweep exists to find. Ask again until it resolves. + for _ in 1 2 3; do + pr=$(gh pr view "$b" --json number,mergeable,mergeStateStatus,statusCheckRollup \ + -q '"#\(.number) \(.mergeable) \(.mergeStateStatus) " + + ([.statusCheckRollup[]?|select(.conclusion=="FAILURE").name]|join(","))' \ + 2>/dev/null) + [ -z "$pr" ] && { echo "NO PR (never pushed): $b"; break; } + case "$pr" in *UNKNOWN*) sleep 3; continue;; esac + echo "OPEN $pr <- $wt"; break + done done -git fetch origin --prune ``` -Two guards that matter: never remove a worktree with **uncommitted tracked -changes** — report it and let a human decide (one such worktree held a -375-line module that existed nowhere else) — and never touch a **detached or -locked** worktree, which is usually another agent's live session. +**Arm 1 — merged: prune.** Two guards that matter: never remove a worktree +with **uncommitted tracked changes** — report it and let a human decide (one +such worktree held a 375-line module that existed nowhere else) — and never +touch a **detached or locked** worktree, which is usually another agent's +live session. + +**Arm 2 — open: refresh.** For each PR the loop printed, act on its state. +This is judgment, not script: -Then `git fetch origin main` — `using-git-worktrees`' git fallback branches -from the current local `HEAD`, not `origin/main`, so a stale local checkout -silently cuts the branch behind main (missed release cuts, changelog -rewrites, other merged fixes), surfacing later as an avoidable merge -conflict. Then invoke `superpowers:using-git-worktrees`, basing the new -branch on `origin/main`. +| State | Action | +|---|---| +| `CONFLICTING`, or `mergeStateStatus: BEHIND` | In that worktree: `git merge origin/main`. Auto-resolve **mechanical** conflicts only — `CHANGELOG.md` under `## [Unreleased]` (keep both bullets), import ordering, lockfiles. Then `./scripts/quality-check.sh`, then push. | +| Conflict is semantic | `git merge --abort` and report it. Do not guess at someone else's fix. | +| Checks `FAILURE` | Read the failing job log (`gh run view --log-failed`). Auto-fix only Black/Ruff formatting and a missing `## [Unreleased]` changelog entry — the mechanical CI failures. A real test failure is reported, never patched blind. | +| Green and mergeable | Nothing. | +| `NO PR (never pushed)` | Report only — never delete. This is a branch with local commits and no PR, which is either abandoned work or someone's parked experiment. The first dry run found 11 of them; they are the same "uncommitted changes" hazard one step later, and the sweep is not what decides their fate. | + +Hard rules for arm 2, all of them learned the expensive way: never push +without `quality-check.sh` green; never `git stash` (denied repo-wide, see +`CLAUDE.md`); never `--force`; never merge the PR or take it out of draft; +one commit per PR per sweep (`chore: merge main into `). Everything +skipped is **reported with its reason** — "3 skipped: #589 owned by live +session" is the useful output; a sweep that silently does nothing is not. + +**Running the sweep on a loop.** Section 4a is also the thing to `/loop` when +you want the fleet held green between issues — it is the same instructions, +not a second mechanism, so do not write a separate skill for it. The one +difference: as a Step 4 precondition it takes a single snapshot of +`statusCheckRollup` and moves on, because blocking the start of issue work on +another PR turning green is exactly backwards. Under `/loop` it re-reads CI +each tick, and a fully-green fleet is a noop tick. Run **one** sweep loop at a +time — two of them collide with each other for the same reason arm 2 skips +owned worktrees, and there is no lock enforcing it. + +#### 4b. Worktree + branch + +The sweep above already ran `git fetch origin --prune`, which is what makes +`origin/main` current — that ordering is the point, not incidental. +`using-git-worktrees`' git fallback branches from the current local `HEAD`, +not `origin/main`, so a stale local checkout silently cuts the branch behind +main (missed release cuts, changelog rewrites, other merged fixes), surfacing +later as an avoidable merge conflict. Invoke +`superpowers:using-git-worktrees`, basing the new branch on `origin/main`. Then run `./scripts/worktree-setup.sh` in the new worktree — once, before any test, build or `verify` step. It shares `.venv` and both `node_modules` trees @@ -337,6 +409,20 @@ to guess whether it was checked. Commit per `docs/agents/workflow.md` format (subject + blank line + body explaining WHY). +**Then bring the branch up to date before pushing — not after.** + +```bash +git fetch origin && git merge origin/main +``` + +Resolve any conflicts here, in the worktree, where you have the context; if +the merge brought changes in, re-run `./scripts/quality-check.sh` before +pushing. Step 4b cut this branch from a current `origin/main`, but Steps 5–8 +take hours (slow suite, `verify`), and other PRs merge during them. Opening a +PR that is already `CONFLICTING` is worse than it sounds: GitHub creates **no +workflow run at all** for it, so the PR shows no checks rather than a +conflict, and the first reader concludes CI dropped the event. + **Frontend diffs only:** before pushing, show the user the diff and the Step 8 verification output (screenshot/dev-server observation) and wait for explicit go-ahead. This is the one point in the fully-automatic flow where a @@ -457,6 +543,10 @@ net is upstream, not this section. | "the plan doc is useful context, keep it in the PR" | Once code and tests exist, the plan only drifts — it's not the source of truth. Delete it before Step 9; keep the spec if one exists. | | "the user is in a hurry, just open the PR" | Time pressure from the user is not permission to skip Step 8 — it's the reason to say so explicitly and give a real ETA instead. | | "I'll clean up the worktree after it merges" | You won't — that's the postcondition that already failed 24 times. Prune at Step 4, before creating the next one. | +| "the branch was current when I cut it, no need to merge before pushing" | Steps 5–8 take hours and other PRs merge during them. And a CONFLICTING PR gets no workflow run at all, so it reads as "CI never fired" — the conflict is invisible until someone digs. | +| "the sweep found an open PR with red CI — I'll fix it properly while I'm here" | Arm 2 auto-fixes Black/Ruff/changelog and nothing else. A real test failure on someone else's PR is reported, not patched blind — you don't have their diagnosis. | +| "that worktree's session is idle, so nobody's using it" | Idle and blocked both mean owned. An agent parked between Step 6 and Step 8 is idle and very much still holds that branch. Any live session at that cwd skips it. | +| "I'll write a babysit-PRs skill to keep the fleet green" | That's Step 4a, run under `/loop`. A second skill with the same instructions is a fork that drifts, and two sweeps running at once collide exactly the way arm 2's skip gate exists to prevent. | | "`git branch --merged` will tell me what's safe to delete" | Not in this repo. Squash-merge means a merged branch is never an ancestor of main, so that check reports *everything* as unmerged and the cleanup silently never fires. Use `gh pr list --state merged`. | | "I'll just hop into the other worktree for a second" | Not while a background agent spawned from this session is running — its isolation follows you and its tooling starts resolving against the wrong checkout. | | "I'll just watch the background agent run" | Defeats the point — the whole reason it's backgrounded is so the session isn't held open through the slow suite. Let the notification bring you back. | @@ -482,6 +572,9 @@ net is upstream, not this section. in `docs/agents/bess-knowledge.md` or `docs/SOFTWARE_DESIGN.md`. - About to write only a synthetic-input unit test for a DP/intent/control- mapping change instead of a plan-faithfulness (`R == P`) scenario test. +- About to push the branch without having merged `origin/main` since Step 4b. +- About to run arm 2 of the sweep against a worktree that has a live session, + uncommitted tracked changes, or a HEAD less than 30 minutes old. - About to write a repro test from hand-built data when a user debug log/ bundle is available and `from_debug_log.py` could build it from real data. @@ -492,7 +585,8 @@ net is upstream, not this section. | 1. Fetch & scope | `gh issue view` | No | | 2. Diagnose | `bess-analyst` (if no bot comment) | Conditional | | 3. Confirm gate | — | No | -| 4. Worktree | `using-git-worktrees` | No | +| 4a. Fleet sweep | `gh pr list/view` + `claude agents --json` (also `/loop`-able) | No | +| 4b. Worktree | `using-git-worktrees` | No | | 5. TDD | `test-driven-development` | No | | 6. Quality gate + code review | `quality-check.sh` + slow suite + `code-review` (background agent) | No | | 7. Confirm gate 2 | — | Conditional (auto-continue if backend-only + clean; otherwise No) | From ad2b7b29f024f14f853872c9a698439e83148443 Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Sat, 15 Aug 2026 08:37:57 +0200 Subject: [PATCH 2/4] feat: split fleet PR maintenance out of implement-issue into sweep-prs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit implement-issue owns exactly one PR — the issue it was invoked for — and had no idea whether that PR ever went green. It stopped at draft-PR-open, so a PR that failed the CI matrix (which quality-check.sh does not reproduce) or went CONFLICTING minutes later sat there unreviewed. Step 9 now merges origin/main before pushing: Step 4 cuts the branch from a current origin/main, but Steps 5-8 take hours and other PRs merge during them. Opening an already-CONFLICTING PR is worse than it sounds, because GitHub creates no workflow run at all for one — it presents as "CI never fired" rather than as a conflict. New Step 10 watches that PR to green via `gh pr checks --watch`, fixing failures in the worktree, and is explicitly scoped to this PR alone. Step 4 is unchanged: it still prunes merged worktrees only. Fleet-wide maintenance moves to a new sweep-prs skill, which walks every worktree, prunes merged ones, merges main into stale branches, and reports red CI. It has the piece implement-issue must not grow: an ownership skip gate. A live session at that cwd (any status — idle and blocked included), uncommitted tracked changes, or a HEAD under 30 minutes old means hands off, because merging under a running implement-issue moves its HEAD and puts two sessions on one branch. Two behaviours found by dry-running the sweep against the real fleet: GitHub computes `mergeable` lazily, so the first query on a cold PR returns UNKNOWN and only triggers the computation — a single pass reports UNKNOWN for precisely the stale PRs the sweep exists to find, hence the retry. And 11 worktrees hold branches with local commits and no PR; those are reported, never deleted. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MYV1WdkadzkeUUgULhC3Dk --- .claude/skills/implement-issue/SKILL.md | 195 ++++++++++-------------- .claude/skills/sweep-prs/SKILL.md | 160 +++++++++++++++++++ 2 files changed, 238 insertions(+), 117 deletions(-) create mode 100644 .claude/skills/sweep-prs/SKILL.md diff --git a/.claude/skills/implement-issue/SKILL.md b/.claude/skills/implement-issue/SKILL.md index 7c2a396b..55a416ef 100644 --- a/.claude/skills/implement-issue/SKILL.md +++ b/.claude/skills/implement-issue/SKILL.md @@ -41,13 +41,14 @@ runners — only repo-level `.claude/skills/` and `.claude/agents/` exist there. |---|---| | 2. Diagnose | Stage 2 comment absent → STOP. Post "No deep analysis found. Run `@claude-bot analyze` first" and exit — never self-diagnose in CI; the analyze/fix split *is* the human gate. | | 3. Confirm gate | The owner's `@claude-bot fix` comment is the go-ahead. Still perform the workaround check and scope assessment — put them in a `## Scope assessment` section of the PR body instead of chat. Escalation path (can't confidently pass the workaround check) still applies: dispatch a fresh general-purpose `Agent` to critique the design before implementing. | -| 4. Fleet sweep + worktree | Skip both — the CI checkout is already isolated and has no worktree fleet to sweep. Create the branch directly (naming per Step 1). | +| 4. Worktree | Skip — the CI checkout is already isolated. Create the branch directly (naming per Step 1). | | 5. TDD | The substance applies verbatim (RED test first, required test shape); there is just no `superpowers:test-driven-development` skill to invoke — follow this section's own rules. | | 6. Quality gate + code review | Run inline, no background agent (CI is one throwaway session — the cost-discipline reason to background doesn't exist). The `code-review` plugin is unavailable; the Stage-4 `@claude-bot` PR review covers it. Checks 1–3 (fast suite, slow suite, required-test-shape) still apply. | | 7. Confirm gate 2 | Replaced by the draft PR itself — the owner reviews the draft before anything merges. | | 8. Local run & observe | Structurally unavailable in CI — this is the documented reason the local flow exists. Skip, and say so in the PR body's test plan so the reviewer knows verification is still owed. | -| 9. Commit + draft PR | Applies verbatim, including the pre-push `git merge origin/main` and the `CHANGELOG.md` `## [Unreleased]` entry and the documentation check. Add the `## Scope assessment` section (Step 3 above). The workflow file owns CI-only mechanics: issue comment with the PR link, `has-fix-pr` label. | -| 10. Hard constraints | Apply verbatim. | +| 9. Commit + draft PR | Applies verbatim, including the `CHANGELOG.md` `## [Unreleased]` entry and the documentation check. Add the `## Scope assessment` section (Step 3 above). The workflow file owns CI-only mechanics: issue comment with the PR link, `has-fix-pr` label. | +| 10. Watch this PR to green | Applies verbatim — `gh pr checks --watch` on the PR just opened, fix failures, never widen to other PRs. | +| 11. Hard constraints | Apply verbatim. | ## Process @@ -96,117 +97,45 @@ owner is not ready to present. Wait for explicit go-ahead before touching code. One message — cheap insurance against building an entire implementation on a wrong diagnosis *or* a wrong placement. -### 4. Fleet sweep, then worktree + branch +### 4. Worktree + branch -#### 4a. Fleet sweep (runs FIRST, before creating anything) - -**This is the cleanup-and-refresh step, and it lives here on purpose.** The -"After Merge" section at the bottom also removes a worktree, but it is -defined as a separate later invocation, so it depends on someone choosing to -come back — and nobody does. That postcondition ran zero times in ~40 issues -and left 39 worktrees on disk, 24 of them long merged. Running the sweep as a -*precondition* of the next issue needs no memory: the next person to do issue -work cleans up the last one's mess automatically. - -The sweep has two arms, because pruning merged worktrees alone leaves the -other half of the fleet to rot. A PR that was green and mergeable when its -session ended goes `CONFLICTING` the moment two or three others merge into -`main` ahead of it — and a `CONFLICTING` PR **creates no workflow run at -all**, so it presents as "CI never fired", not as a conflict. Nobody is -watching for that, which is exactly why it has to be swept rather than -noticed. - -**The skip gate comes before both arms.** Another agent may be mid-`implement-issue` -in any of these worktrees; merging `origin/main` underneath it moves its HEAD -mid-run and puts two sessions on the same branch pushing to the same PR. +**Prune merged worktrees FIRST — this is the cleanup step, and it lives here +on purpose.** The "After Merge" section at the bottom also removes a +worktree, but it is defined as a separate later invocation, so it depends on +someone choosing to come back — and nobody does. That postcondition ran zero +times in ~40 issues and left 39 worktrees on disk, 24 of them long merged. +Running the prune as a *precondition* of the next issue needs no memory: the +next person to do issue work cleans up the last one's mess automatically. ```bash -git fetch origin --prune -# NOTE: `git branch --merged` / `rev-list origin/main..branch` DO NOT WORK -# here -- this repo squash-merges, so a merged branch's commits are never -# reachable from main and every worktree looks unmerged forever. PR state is -# the only authoritative signal. +# Worktrees whose PR has merged. NOTE: `git branch --merged` / `rev-list +# origin/main..branch` DO NOT WORK here -- this repo squash-merges, so a +# merged branch's commits are never reachable from main and every worktree +# looks unmerged forever. PR state is the only authoritative signal. merged=$(gh pr list --state merged --limit 200 --json headRefName -q '.[].headRefName') -# Live sessions own their worktree at ANY status -- idle and blocked included. -# UNSCOPED on purpose: sibling worktrees never appear in the project-scoped -# view (CLAUDE.md, Worktree Conventions), and missing one is the collision. -owned=$(cd ~ && claude agents --json 2>/dev/null | jq -r '.[].cwd') - git worktree list | awk 'NR>1 {print $1}' | while read -r wt; do b=$(git -C "$wt" branch --show-current 2>/dev/null) - [ -n "$b" ] || continue # detached: leave alone - if echo "$owned" | grep -qF "$wt"; then - echo "SKIP (live session): $wt"; continue + [ -n "$b" ] || continue # detached: leave alone + echo "$merged" | grep -qx "$b" || continue # not merged: leave alone + if [ -n "$(git -C "$wt" status --porcelain -uno)" ]; then # tracked edits: never auto-delete + echo "KEEP (uncommitted changes): $wt"; continue fi - if [ -n "$(git -C "$wt" status --porcelain -uno)" ]; then # tracked edits: never touch - echo "SKIP (uncommitted changes): $wt"; continue - fi - if echo "$merged" | grep -qx "$b"; then # arm 1: merged -> prune - git worktree remove "$wt" && git branch -D "$b"; continue - fi - age=$(( ($(date +%s) - $(git -C "$wt" log -1 --format=%ct)) / 60 )) - if [ "$age" -lt 30 ]; then - echo "SKIP (HEAD ${age}m old, likely active): $wt"; continue - fi - # arm 2: open -> refresh. GitHub computes `mergeable` LAZILY: the first - # query on a cold PR returns UNKNOWN *and* triggers the computation, so a - # single pass reports UNKNOWN for every stale PR -- i.e. exactly the ones - # the sweep exists to find. Ask again until it resolves. - for _ in 1 2 3; do - pr=$(gh pr view "$b" --json number,mergeable,mergeStateStatus,statusCheckRollup \ - -q '"#\(.number) \(.mergeable) \(.mergeStateStatus) " + - ([.statusCheckRollup[]?|select(.conclusion=="FAILURE").name]|join(","))' \ - 2>/dev/null) - [ -z "$pr" ] && { echo "NO PR (never pushed): $b"; break; } - case "$pr" in *UNKNOWN*) sleep 3; continue;; esac - echo "OPEN $pr <- $wt"; break - done + git worktree remove "$wt" && git branch -D "$b" done +git fetch origin --prune ``` -**Arm 1 — merged: prune.** Two guards that matter: never remove a worktree -with **uncommitted tracked changes** — report it and let a human decide (one -such worktree held a 375-line module that existed nowhere else) — and never -touch a **detached or locked** worktree, which is usually another agent's -live session. - -**Arm 2 — open: refresh.** For each PR the loop printed, act on its state. -This is judgment, not script: +Two guards that matter: never remove a worktree with **uncommitted tracked +changes** — report it and let a human decide (one such worktree held a +375-line module that existed nowhere else) — and never touch a **detached or +locked** worktree, which is usually another agent's live session. -| State | Action | -|---|---| -| `CONFLICTING`, or `mergeStateStatus: BEHIND` | In that worktree: `git merge origin/main`. Auto-resolve **mechanical** conflicts only — `CHANGELOG.md` under `## [Unreleased]` (keep both bullets), import ordering, lockfiles. Then `./scripts/quality-check.sh`, then push. | -| Conflict is semantic | `git merge --abort` and report it. Do not guess at someone else's fix. | -| Checks `FAILURE` | Read the failing job log (`gh run view --log-failed`). Auto-fix only Black/Ruff formatting and a missing `## [Unreleased]` changelog entry — the mechanical CI failures. A real test failure is reported, never patched blind. | -| Green and mergeable | Nothing. | -| `NO PR (never pushed)` | Report only — never delete. This is a branch with local commits and no PR, which is either abandoned work or someone's parked experiment. The first dry run found 11 of them; they are the same "uncommitted changes" hazard one step later, and the sweep is not what decides their fate. | - -Hard rules for arm 2, all of them learned the expensive way: never push -without `quality-check.sh` green; never `git stash` (denied repo-wide, see -`CLAUDE.md`); never `--force`; never merge the PR or take it out of draft; -one commit per PR per sweep (`chore: merge main into `). Everything -skipped is **reported with its reason** — "3 skipped: #589 owned by live -session" is the useful output; a sweep that silently does nothing is not. - -**Running the sweep on a loop.** Section 4a is also the thing to `/loop` when -you want the fleet held green between issues — it is the same instructions, -not a second mechanism, so do not write a separate skill for it. The one -difference: as a Step 4 precondition it takes a single snapshot of -`statusCheckRollup` and moves on, because blocking the start of issue work on -another PR turning green is exactly backwards. Under `/loop` it re-reads CI -each tick, and a fully-green fleet is a noop tick. Run **one** sweep loop at a -time — two of them collide with each other for the same reason arm 2 skips -owned worktrees, and there is no lock enforcing it. - -#### 4b. Worktree + branch - -The sweep above already ran `git fetch origin --prune`, which is what makes -`origin/main` current — that ordering is the point, not incidental. -`using-git-worktrees`' git fallback branches from the current local `HEAD`, -not `origin/main`, so a stale local checkout silently cuts the branch behind -main (missed release cuts, changelog rewrites, other merged fixes), surfacing -later as an avoidable merge conflict. Invoke -`superpowers:using-git-worktrees`, basing the new branch on `origin/main`. +Then `git fetch origin main` — `using-git-worktrees`' git fallback branches +from the current local `HEAD`, not `origin/main`, so a stale local checkout +silently cuts the branch behind main (missed release cuts, changelog +rewrites, other merged fixes), surfacing later as an avoidable merge +conflict. Then invoke `superpowers:using-git-worktrees`, basing the new +branch on `origin/main`. Then run `./scripts/worktree-setup.sh` in the new worktree — once, before any test, build or `verify` step. It shares `.venv` and both `node_modules` trees @@ -417,8 +346,8 @@ git fetch origin && git merge origin/main Resolve any conflicts here, in the worktree, where you have the context; if the merge brought changes in, re-run `./scripts/quality-check.sh` before -pushing. Step 4b cut this branch from a current `origin/main`, but Steps 5–8 -take hours (slow suite, `verify`), and other PRs merge during them. Opening a +pushing. Step 4 cut this branch from a current `origin/main`, but Steps 5–8 +take hours (slow suite, `verify`) and other PRs merge during them. Opening a PR that is already `CONFLICTING` is worse than it sounds: GitHub creates **no workflow run at all** for it, so the PR shows no checks rather than a conflict, and the first reader concludes CI dropped the event. @@ -475,7 +404,39 @@ If you cannot produce a mutation that reddens your test, you have not demonstrated the bug — say so in the PR and stop, rather than filling the section in with the suite result. -### 10. Hard constraints +### 10. Watch this PR to green (and only this PR) + +The draft PR is open, but CI has not run yet. Local `quality-check.sh` and +the slow suite are not the same as the CI matrix, and a PR left red or +`CONFLICTING` is a PR the user cannot review. + +```bash +gh pr checks --watch --fail-fast # blocks until the run settles +gh pr view --json mergeable,mergeStateStatus +``` + +Then, on this PR only: + +- **Checks fail:** read `gh run view --log-failed`, fix in the worktree, + re-run `./scripts/quality-check.sh`, push. This is your diff, so a real + test failure is yours to fix — not merely to report. +- **Went `CONFLICTING`** (another PR merged in the minutes since Step 9): + `git merge origin/main`, resolve, `quality-check.sh`, push. +- **Green and mergeable:** report the PR link and stop. Do not merge, do not + take it out of draft — Step 11 still holds. + +**Scope: this issue's PR, nothing else.** If the sweep in Step 4 or your own +`gh pr list` shows other PRs red or conflicted, that is not this session's +job — hand it to the `sweep-prs` skill, which owns fleet-wide maintenance +and has the ownership skip gate needed to touch a worktree another agent may +be sitting in. Widening a single-issue session into fleet cleanup is how two +sessions end up pushing to the same branch. + +`gh pr checks --watch` blocks rather than polls, so this costs one wait, not +a re-read of the whole session context every 60s. If CI is badly backed up, +say so and leave the PR — don't hold the session open indefinitely. + +### 11. Hard constraints - Draft PR only. Never auto-merge. - Never push directly to `main`. @@ -495,7 +456,7 @@ section in with the suite result. A **separate, later invocation** — often a different session, sometimes days later once CI is green and the user has reviewed. Not part of the numbered -flow above, which stops at draft-PR-open per the Step 10 constraints. +flow above, which stops at draft-PR-green per the Step 11 constraints. **Treat this as best-effort, not the cleanup mechanism.** Because it depends on someone returning after the merge, it reliably does not happen; Step 4's @@ -540,13 +501,12 @@ net is upstream, not this section. | "I'll clean up this other thing while I'm in here" | Out of scope. Minimal fix only. | | "code review can wait until after I've verified it works" | Reordered on purpose — catch cheap issues before spending time on manual verification, not after. | | "there's already a bot diagnosis, let me re-derive it anyway to be safe" | Re-verify the cited evidence; don't redo the whole investigation. | +| "the branch was current when I cut it, no need to merge before pushing" | Steps 5–8 take hours and other PRs merge during them. And a CONFLICTING PR gets no workflow run at all, so it reads as "CI never fired" — the conflict stays invisible until someone digs. | +| "while I'm watching my PR I may as well fix the other red ones" | That's `sweep-prs`, which has the ownership skip gate this skill doesn't. Another agent may be sitting in that worktree; merging under it puts two sessions on one branch. | +| "the PR is open, my job is done" | Open isn't green. CI runs a matrix `quality-check.sh` doesn't, and the user can't review a red or conflicted PR. Step 10 finishes the job. | | "the plan doc is useful context, keep it in the PR" | Once code and tests exist, the plan only drifts — it's not the source of truth. Delete it before Step 9; keep the spec if one exists. | | "the user is in a hurry, just open the PR" | Time pressure from the user is not permission to skip Step 8 — it's the reason to say so explicitly and give a real ETA instead. | | "I'll clean up the worktree after it merges" | You won't — that's the postcondition that already failed 24 times. Prune at Step 4, before creating the next one. | -| "the branch was current when I cut it, no need to merge before pushing" | Steps 5–8 take hours and other PRs merge during them. And a CONFLICTING PR gets no workflow run at all, so it reads as "CI never fired" — the conflict is invisible until someone digs. | -| "the sweep found an open PR with red CI — I'll fix it properly while I'm here" | Arm 2 auto-fixes Black/Ruff/changelog and nothing else. A real test failure on someone else's PR is reported, not patched blind — you don't have their diagnosis. | -| "that worktree's session is idle, so nobody's using it" | Idle and blocked both mean owned. An agent parked between Step 6 and Step 8 is idle and very much still holds that branch. Any live session at that cwd skips it. | -| "I'll write a babysit-PRs skill to keep the fleet green" | That's Step 4a, run under `/loop`. A second skill with the same instructions is a fork that drifts, and two sweeps running at once collide exactly the way arm 2's skip gate exists to prevent. | | "`git branch --merged` will tell me what's safe to delete" | Not in this repo. Squash-merge means a merged branch is never an ancestor of main, so that check reports *everything* as unmerged and the cleanup silently never fires. Use `gh pr list --state merged`. | | "I'll just hop into the other worktree for a second" | Not while a background agent spawned from this session is running — its isolation follows you and its tooling starts resolving against the wrong checkout. | | "I'll just watch the background agent run" | Defeats the point — the whole reason it's backgrounded is so the session isn't held open through the slow suite. Let the notification bring you back. | @@ -572,9 +532,10 @@ net is upstream, not this section. in `docs/agents/bess-knowledge.md` or `docs/SOFTWARE_DESIGN.md`. - About to write only a synthetic-input unit test for a DP/intent/control- mapping change instead of a plan-faithfulness (`R == P`) scenario test. -- About to push the branch without having merged `origin/main` since Step 4b. -- About to run arm 2 of the sweep against a worktree that has a live session, - uncommitted tracked changes, or a HEAD less than 30 minutes old. +- About to push the branch without having merged `origin/main` since Step 4. +- About to stop at "draft PR opened" without watching CI settle (Step 10). +- About to touch another PR or another worktree from inside this session — + that is `sweep-prs`, not this skill. - About to write a repro test from hand-built data when a user debug log/ bundle is available and `from_debug_log.py` could build it from real data. @@ -585,10 +546,10 @@ net is upstream, not this section. | 1. Fetch & scope | `gh issue view` | No | | 2. Diagnose | `bess-analyst` (if no bot comment) | Conditional | | 3. Confirm gate | — | No | -| 4a. Fleet sweep | `gh pr list/view` + `claude agents --json` (also `/loop`-able) | No | -| 4b. Worktree | `using-git-worktrees` | No | +| 4. Worktree | `using-git-worktrees` | No | | 5. TDD | `test-driven-development` | No | | 6. Quality gate + code review | `quality-check.sh` + slow suite + `code-review` (background agent) | No | | 7. Confirm gate 2 | — | Conditional (auto-continue if backend-only + clean; otherwise No) | | 8. Local run & observe | `verify` | **Never** | -| 9. Commit + PR | `finishing-a-development-branch` | No | +| 9. Commit + PR | `finishing-a-development-branch` (incl. pre-push `git merge origin/main`) | No | +| 10. Watch this PR to green | `gh pr checks --watch` — this PR only | No | diff --git a/.claude/skills/sweep-prs/SKILL.md b/.claude/skills/sweep-prs/SKILL.md new file mode 100644 index 00000000..f0921fe0 --- /dev/null +++ b/.claude/skills/sweep-prs/SKILL.md @@ -0,0 +1,160 @@ +--- +name: sweep-prs +description: Use when asked to check, refresh, or keep the open bess-manager PRs green and mergeable across the whole fleet — pruning merged worktrees, merging main into stale branches, and reporting red CI. Run under /loop for continuous maintenance. +--- + +# Sweep PRs + +## Overview + +Walk every git worktree in the repo and bring the fleet back to a known +state: prune the ones whose PR has merged, refresh the ones whose PR has +gone stale, report everything else. + +This exists because a PR rots without anyone touching it. A branch that was +green and mergeable when its session ended goes `CONFLICTING` the moment two +or three others merge into `main` ahead of it — and a `CONFLICTING` PR +**creates no workflow run at all**, so it presents as "CI never fired", not +as a conflict. Nobody investigates a PR that appears to have no checks. The +first dry run of this sweep found two such PRs (#437, #579) sitting +conflicted with no one aware. + +## When to Use + +- Asked to check whether the open PRs are green/mergeable, or to fix up the + ones that aren't. +- Under `/loop` to hold the fleet green between issues. Self-paced is right: + a fully-green fleet costs a noop tick. +- **Not** from inside an `implement-issue` session. That skill owns exactly + one PR — its own — and deliberately does not widen into fleet cleanup + (see its Step 10). Run this separately. + +## Scope + +Every worktree in `git worktree list`. A branch is only *acted on* when it +survives the skip gate below; everything else is reported with its reason. + +## Process + +### 1. Skip gate (before anything else) + +Another agent may be mid-`implement-issue` in any of these worktrees. +Merging `origin/main` underneath it moves its HEAD mid-run and puts two +sessions on the same branch pushing to the same PR. Leave a worktree alone +when any of these hold: + +| Condition | Why | +|---|---| +| A live Claude session has its `cwd` at or under the worktree | Owned. **Any status counts — `idle` and `blocked` included.** An agent parked between Step 6 and Step 8 reads as idle and still holds that branch. | +| `git status --porcelain -uno` is non-empty | Uncommitted tracked work. One such worktree held a 375-line module that existed nowhere else. | +| HEAD commit is under 30 minutes old | Likely active even if its session already exited. | +| Detached or locked | Almost always another agent's live session. | + +Read ownership with `claude agents --json` run **unscoped** (from `~`) — +per `CLAUDE.md`'s Worktree Conventions, sibling worktrees never appear in +the project-scoped view, and missing one is exactly the collision this gate +exists to prevent. + +### 2. Classify + +```bash +git fetch origin --prune +# NOTE: `git branch --merged` / `rev-list origin/main..branch` DO NOT WORK +# here -- this repo squash-merges, so a merged branch's commits are never +# reachable from main and every worktree looks unmerged forever. PR state is +# the only authoritative signal. +merged=$(gh pr list --state merged --limit 200 --json headRefName -q '.[].headRefName') +owned=$(cd ~ && claude agents --json 2>/dev/null | jq -r '.[].cwd') + +git worktree list | awk 'NR>1 {print $1}' | while read -r wt; do + b=$(git -C "$wt" branch --show-current 2>/dev/null) + [ -n "$b" ] || { echo "SKIP (detached): $wt"; continue; } + if echo "$owned" | grep -qF "$wt"; then + echo "SKIP (live session): $b"; continue + fi + if [ -n "$(git -C "$wt" status --porcelain -uno)" ]; then + echo "SKIP (uncommitted changes): $b"; continue + fi + if echo "$merged" | grep -qx "$b"; then + echo "PRUNE: $b"; continue + fi + age=$(( ($(date +%s) - $(git -C "$wt" log -1 --format=%ct)) / 60 )) + if [ "$age" -lt 30 ]; then + echo "SKIP (HEAD ${age}m old, likely active): $b"; continue + fi + # GitHub computes `mergeable` LAZILY: the first query on a cold PR returns + # UNKNOWN *and* triggers the computation, so a single pass reports UNKNOWN + # for every stale PR -- i.e. exactly the ones this sweep exists to find. + # Ask again until it resolves. Verified: all three resolved on retry. + for _ in 1 2 3; do + pr=$(gh pr view "$b" --json number,mergeable,mergeStateStatus,statusCheckRollup \ + -q '"#\(.number) \(.mergeable) \(.mergeStateStatus) " + + ([.statusCheckRollup[]?|select(.conclusion=="FAILURE").name]|join(","))' \ + 2>/dev/null) + [ -z "$pr" ] && { echo "NO PR (never pushed): $b"; break; } + case "$pr" in *UNKNOWN*) sleep 3; continue;; esac + echo "OPEN $pr <- $wt"; break + done +done +``` + +### 3. Act + +**`PRUNE`** — the PR merged: + +```bash +git worktree remove "$wt" && git branch -D "$b" +``` + +Force-delete is expected, not a warning sign: squash-merge means the +branch's commits never become reachable from `main`, so `git branch -d`'s +ancestry check always refuses. + +**`OPEN`** — act on the state. This is judgment, not script: + +| State | Action | +|---|---| +| `CONFLICTING`, or `mergeStateStatus: BEHIND` | In that worktree: `git merge origin/main`. Auto-resolve **mechanical** conflicts only — `CHANGELOG.md` under `## [Unreleased]` (keep both bullets), import ordering, lockfiles. Then `./scripts/quality-check.sh`, then push. | +| Conflict is semantic | `git merge --abort` and report it. Do not guess at someone else's fix. | +| Checks `FAILURE` | Read the failing job log (`gh run view --log-failed`). Auto-fix only Black/Ruff formatting and a missing `## [Unreleased]` changelog entry — the mechanical CI failures. A real test failure belongs to whoever wrote the diff; report it, never patch it blind. | +| Green and mergeable | Nothing. | +| `NO PR (never pushed)` | Report only, never delete. A branch with local commits and no PR is abandoned work or someone's parked experiment; the first dry run found 11. The sweep is not what decides their fate. | + +### 4. Report + +Print what was pruned, what was refreshed and pushed, what failed, and +**what was skipped with the reason** — "3 skipped: #589 owned by live +session `bess-manager-34`" is the useful output. A sweep that silently does +nothing is indistinguishable from a broken one. + +## Hard constraints + +- Never push without `./scripts/quality-check.sh` green. +- Never `git stash` — denied repo-wide, one shared stack (`CLAUDE.md`). +- Never `--force` / `--force-with-lease` on someone else's branch. +- Never merge a PR, take it out of draft, or close it. +- One commit per PR per sweep: `chore: merge main into `. +- Never resolve a semantic conflict or fix a real test failure on a PR that + isn't yours to diagnose. Report and move on. +- **Run one sweep at a time.** Two collide with each other for exactly the + reason the skip gate exists, and nothing enforces it. + +## Rationalizations — Reality + +| Excuse | Reality | +|---|---| +| "that worktree's session is idle, so nobody's using it" | Idle and blocked both mean owned. An agent parked between Step 6 and Step 8 is idle and still holds that branch. | +| "the PR shows no CI checks, so the workflow must have failed to trigger" | A CONFLICTING PR creates no workflow run at all. Check `mergeable` before blaming a dropped event. | +| "`mergeable` came back UNKNOWN, so gh can't tell" | It's computed lazily — the first query only triggers the computation. Ask again. A single pass reports UNKNOWN for precisely the stale PRs you're looking for. | +| "`git branch --merged` will tell me what's safe to delete" | Not in this repo. Squash-merge means a merged branch is never an ancestor of main, so that check reports everything as unmerged and cleanup never fires. Use `gh pr list --state merged`. | +| "this conflict is small, I can see what they meant" | Mechanical means CHANGELOG/imports/lockfiles. Anything touching logic is a guess at someone else's fix — abort and report. | +| "I'm already in an implement-issue session, I'll sweep while I'm here" | That session owns one PR and lacks the skip gate. Widening it is how two sessions end up pushing to one branch. | + +## Red Flags — Stop and Go Back + +- About to act on a worktree with a live session, uncommitted tracked + changes, or a HEAD under 30 minutes old. +- About to treat `mergeable: UNKNOWN` as a final answer. +- About to push a merge without `quality-check.sh` green. +- About to fix a real test failure on a PR you didn't write. +- About to run this from inside an `implement-issue` session. From bb77e4abc39b5be5e5ed216e735391e82c0ba6c4 Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Sat, 15 Aug 2026 08:53:24 +0200 Subject: [PATCH 3/4] feat: make sweep-prs report-only by default, selectable per PR The sweep acted on everything that survived the skip gate, in one pass, with no way to choose. Merging and pushing to a PR the user has not looked at should not be what happens because they forgot to pass a flag. Three modes: bare `/sweep-prs` classifies the fleet and reports what it would do, changing nothing; `/sweep-prs 437 579` acts on the listed PRs only; `/sweep-prs --all` acts on everything eligible and is the one to pair with /loop. The safe mode is now the default. Pruning merged worktrees stays automatic in every mode. It deletes only work already merged into main, so nothing is at risk, and gating it behind a flag recreates the failure it was added to fix -- a cleanup nobody chooses to run doesn't run, which is how 39 worktrees accumulated. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MYV1WdkadzkeUUgULhC3Dk --- .claude/skills/sweep-prs/SKILL.md | 53 +++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 9 deletions(-) diff --git a/.claude/skills/sweep-prs/SKILL.md b/.claude/skills/sweep-prs/SKILL.md index f0921fe0..1b052f84 100644 --- a/.claude/skills/sweep-prs/SKILL.md +++ b/.claude/skills/sweep-prs/SKILL.md @@ -1,6 +1,6 @@ --- name: sweep-prs -description: Use when asked to check, refresh, or keep the open bess-manager PRs green and mergeable across the whole fleet — pruning merged worktrees, merging main into stale branches, and reporting red CI. Run under /loop for continuous maintenance. +description: Use when asked to check, refresh, or keep the open bess-manager PRs green and mergeable across the whole fleet — pruning merged worktrees, merging main into stale branches, and reporting red CI. Reports only by default; pass PR numbers to act on those, or --all (pair with /loop) for unattended maintenance. --- # Sweep PRs @@ -23,16 +23,40 @@ conflicted with no one aware. - Asked to check whether the open PRs are green/mergeable, or to fix up the ones that aren't. -- Under `/loop` to hold the fleet green between issues. Self-paced is right: - a fully-green fleet costs a noop tick. +- Under `/loop /sweep-prs --all` to hold the fleet green between issues. + Self-paced is right: a fully-green fleet costs a noop tick. `--all` is + required here — a bare `/sweep-prs` on a loop just re-reports the same + fleet forever without fixing anything. - **Not** from inside an `implement-issue` session. That skill owns exactly one PR — its own — and deliberately does not widen into fleet cleanup (see its Step 10). Run this separately. -## Scope +## Modes -Every worktree in `git worktree list`. A branch is only *acted on* when it -survives the skip gate below; everything else is reported with its reason. +Classification always covers every worktree in `git worktree list`. What +differs is **what gets acted on**, and that is chosen by the invocation: + +| Invocation | Behaviour | +|---|---| +| `/sweep-prs` | **Report only — the default.** Classify the whole fleet, print what each PR *would* get, change nothing. No merges, no pushes. | +| `/sweep-prs 437 579` | Act, restricted to the listed PR numbers. Everything else is classified and reported, untouched. This is the normal way to use it: read the report, pick. | +| `/sweep-prs --all` | Act on every PR that survives the skip gate. Pair this one with `/loop` for unattended maintenance. | + +**Report-only is the default on purpose.** Merging and pushing to a PR the +user has not looked at is not something to do because they forgot to pass a +flag. The safe mode is the one you get when you don't think about it; acting +is the one you have to ask for. + +**Pruning merged worktrees runs in every mode, including report-only.** It +is not gated, because it deletes only work that is already merged into +`main` — nothing is at risk — and because gating it recreates exactly the +failure it was introduced to fix: a cleanup that depends on someone choosing +to run it doesn't run. That is how 39 worktrees accumulated on disk, 24 of +them long merged. The uncommitted-changes and live-session guards still +apply to it. + +Whatever the mode, a branch is only ever *acted on* when it survives the +skip gate below; everything else is reported with its reason. ## Process @@ -110,7 +134,9 @@ Force-delete is expected, not a warning sign: squash-merge means the branch's commits never become reachable from `main`, so `git branch -d`'s ancestry check always refuses. -**`OPEN`** — act on the state. This is judgment, not script: +**`OPEN`** — in report-only mode (the default), print the row from the table +below that this PR matches and take no action. In `--all` mode, or when the +PR number was listed explicitly, act on it. This is judgment, not script: | State | Action | |---|---| @@ -122,8 +148,11 @@ ancestry check always refuses. ### 4. Report -Print what was pruned, what was refreshed and pushed, what failed, and -**what was skipped with the reason** — "3 skipped: #589 owned by live +In report-only mode, end with the invocation that would act on what you +found — `/sweep-prs 437 579` — so choosing is one paste, not a re-read. + +Print what was pruned, what was refreshed and pushed (or *would* be), what +failed, and **what was skipped with the reason** — "3 skipped: #589 owned by live session `bess-manager-34`" is the useful output. A sweep that silently does nothing is indistinguishable from a broken one. @@ -138,6 +167,8 @@ nothing is indistinguishable from a broken one. isn't yours to diagnose. Report and move on. - **Run one sweep at a time.** Two collide with each other for exactly the reason the skip gate exists, and nothing enforces it. +- Never act on a PR the invocation didn't select. A bare `/sweep-prs` + reports; it does not merge or push. ## Rationalizations — Reality @@ -147,6 +178,8 @@ nothing is indistinguishable from a broken one. | "the PR shows no CI checks, so the workflow must have failed to trigger" | A CONFLICTING PR creates no workflow run at all. Check `mergeable` before blaming a dropped event. | | "`mergeable` came back UNKNOWN, so gh can't tell" | It's computed lazily — the first query only triggers the computation. Ask again. A single pass reports UNKNOWN for precisely the stale PRs you're looking for. | | "`git branch --merged` will tell me what's safe to delete" | Not in this repo. Squash-merge means a merged branch is never an ancestor of main, so that check reports everything as unmerged and cleanup never fires. Use `gh pr list --state merged`. | +| "they asked me to sweep, obviously they want it fixed" | A bare `/sweep-prs` reports. Merging and pushing to a PR the user hasn't looked at is not something to infer from a missing flag — hand back the report and the `/sweep-prs ` line. | +| "they named #437, and #579 has the same problem, I'll do both" | They picked one. Selecting PRs is the whole point of the mode; extending the selection silently makes it meaningless. | | "this conflict is small, I can see what they meant" | Mechanical means CHANGELOG/imports/lockfiles. Anything touching logic is a guess at someone else's fix — abort and report. | | "I'm already in an implement-issue session, I'll sweep while I'm here" | That session owns one PR and lacks the skip gate. Widening it is how two sessions end up pushing to one branch. | @@ -158,3 +191,5 @@ nothing is indistinguishable from a broken one. - About to push a merge without `quality-check.sh` green. - About to fix a real test failure on a PR you didn't write. - About to run this from inside an `implement-issue` session. +- About to merge or push on a bare `/sweep-prs` — that mode reports only. +- About to act on a PR the user didn't list. From c2cfa1dfb07d7060f88f9de7d32e35417cd750c5 Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Sat, 15 Aug 2026 09:00:29 +0200 Subject: [PATCH 4/4] fix: distinguish the two causes of "no checks reported" in Step 10 Step 10 told you to run `gh pr checks --watch` and read the result. On this skill's own PR that command returned "no checks reported on the 'feat/sweep-prs-skill' branch" about 8 seconds after the push -- the run existed and was in_progress, but --watch returned before it registered. That string has two causes that need opposite responses. A CONFLICTING PR has no run and never will, because GitHub does not build one; the fix is to merge origin/main. A just-pushed PR has a run that hasn't appeared yet; the fix is to wait on the run id. Reading either as green is how a red PR gets handed over as finished. Step 10 now requires telling them apart via `gh pr view --json mergeable` plus `gh run list --branch`, then `gh run watch --exit-status`. Same correction in sweep-prs' rationalization table. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MYV1WdkadzkeUUgULhC3Dk --- .claude/skills/implement-issue/SKILL.md | 18 ++++++++++++++++++ .claude/skills/sweep-prs/SKILL.md | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/.claude/skills/implement-issue/SKILL.md b/.claude/skills/implement-issue/SKILL.md index 55a416ef..eeed3532 100644 --- a/.claude/skills/implement-issue/SKILL.md +++ b/.claude/skills/implement-issue/SKILL.md @@ -415,6 +415,22 @@ gh pr checks --watch --fail-fast # blocks until the run settles gh pr view --json mergeable,mergeStateStatus ``` +**`no checks reported on the '' branch` is not a result.** It has +two entirely different causes and you must tell them apart before doing +anything else: + +```bash +gh pr view --json mergeable,mergeStateStatus # CONFLICTING -> merge origin/main +gh run list --branch --limit 3 # in_progress -> --watch just raced it +gh run watch --exit-status # then wait on the run directly +``` + +If `mergeable` is `CONFLICTING`, there is genuinely no run and never will +be — GitHub does not build a conflicted PR. If a run is `in_progress`, +`--watch` simply returned before the run was registered (observed on this +skill's own PR, ~8s after the push) and you wait on the run id instead. +Reading "no checks" as green is how a red PR gets handed over as finished. + Then, on this PR only: - **Checks fail:** read `gh run view --log-failed`, fix in the worktree, @@ -534,6 +550,8 @@ net is upstream, not this section. mapping change instead of a plan-faithfulness (`R == P`) scenario test. - About to push the branch without having merged `origin/main` since Step 4. - About to stop at "draft PR opened" without watching CI settle (Step 10). +- About to read `no checks reported` as green. It means either a conflict + or a run that hasn't registered yet — distinguish before reporting. - About to touch another PR or another worktree from inside this session — that is `sweep-prs`, not this skill. - About to write a repro test from hand-built data when a user debug log/ diff --git a/.claude/skills/sweep-prs/SKILL.md b/.claude/skills/sweep-prs/SKILL.md index 1b052f84..769e198b 100644 --- a/.claude/skills/sweep-prs/SKILL.md +++ b/.claude/skills/sweep-prs/SKILL.md @@ -175,7 +175,7 @@ nothing is indistinguishable from a broken one. | Excuse | Reality | |---|---| | "that worktree's session is idle, so nobody's using it" | Idle and blocked both mean owned. An agent parked between Step 6 and Step 8 is idle and still holds that branch. | -| "the PR shows no CI checks, so the workflow must have failed to trigger" | A CONFLICTING PR creates no workflow run at all. Check `mergeable` before blaming a dropped event. | +| "the PR shows no CI checks, so the workflow must have failed to trigger" | Two causes, never a dropped event. A CONFLICTING PR creates no run at all; a just-pushed PR has a run that hasn't registered yet. Check `mergeable`, then `gh run list --branch`. | | "`mergeable` came back UNKNOWN, so gh can't tell" | It's computed lazily — the first query only triggers the computation. Ask again. A single pass reports UNKNOWN for precisely the stale PRs you're looking for. | | "`git branch --merged` will tell me what's safe to delete" | Not in this repo. Squash-merge means a merged branch is never an ancestor of main, so that check reports everything as unmerged and cleanup never fires. Use `gh pr list --state merged`. | | "they asked me to sweep, obviously they want it fixed" | A bare `/sweep-prs` reports. Merging and pushing to a PR the user hasn't looked at is not something to infer from a missing flag — hand back the report and the `/sweep-prs ` line. |