johanzander
diff --git a/‎.claude/skills/implement-issue/SKILL.md‎
Lines changed: 77 additions & 4 deletions b/‎.claude/skills/implement-issue/SKILL.md‎
Lines changed: 77 additions & 4 deletions
diff --git a/‎.claude/skills/sweep-prs/SKILL.md‎
Lines changed: 195 additions & 0 deletions b/‎.claude/skills/sweep-prs/SKILL.md‎
Lines changed: 195 additions & 0 deletions
@@ -47,7 +47,8 @@ runners — only repo-level `.claude/skills/` and `.claude/agents/` exist there.
 | 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. |
-| 10. Hard constraints | Apply verbatim. |
+| 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
 
@@ -337,6 +338,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 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.
+
 **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
@@ -389,7 +404,55 @@ 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 <n> --watch --fail-fast     # blocks until the run settles
+gh pr view <n> --json mergeable,mergeStateStatus
+```
+
+**`no checks reported on the '<branch>' 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 <n> --json mergeable,mergeStateStatus   # CONFLICTING -> merge origin/main
+gh run list --branch <branch> --limit 3            # in_progress -> --watch just raced it
+gh run watch <run-id> --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,
+  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`.
@@ -409,7 +472,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
@@ -454,6 +517,9 @@ 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. |
@@ -482,6 +548,12 @@ 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 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/
   bundle is available and `from_debug_log.py` could build it from real data.
 
@@ -497,4 +569,5 @@ net is upstream, not this section.
 | 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 |
@@ -0,0 +1,195 @@
+---
+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. Reports only by default; pass PR numbers to act on those, or --all (pair with /loop) for unattended 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 /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.
+
+## Modes
+
+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
+
+### 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`** — 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 |
+|---|---|
+| `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
+
+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.
+
+## 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 <branch>`.
+- 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.
+- Never act on a PR the invocation didn't select. A bare `/sweep-prs`
+  reports; it does not merge or push.
+
+## 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" | 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 <numbers>` 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. |
+
+## 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.
+- 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.