fix: make backlog grooming reflect what actually blocks an issue (#623)

* fix: make backlog grooming reflect what actually blocks an issue

The digest misclassified enough of the board that grooming could not be
trusted, and every misclassification pushed work in the same direction:
towards looking more ready than it was. Measured against the live board,
Ready went 1 -> 0 and In Progress 5 -> 1.

Five defects, each traced to a real item.

1. `analyzed` was tested BEFORE any wait, so an analysed-but-blocked item
   reported Ready. #96 was labelled `analyzed`, prioritised P2, carried no
   blocking label, and still could not be built because its approach was
   undecided. It read as dispatchable, a session was dispatched at it, and
   that session deadlocked on three design questions. Waits now outrank
   `analyzed`.

2. The board's `Awaiting` field was never read. 17 items have it set
   (`discussion` x11, `reporter` x6) and the digest derived its own value from
   labels instead, so recorded grooming had no effect on anything. The field is
   now authoritative, with `awaiting_source` and `awaiting_suggested` exposed
   so an unset field can be reconciled rather than silently invented.

3. `awaiting: discussion` was returned for ANY human comment, which is not a
   blocker -- thanks, a "me too" and a follow-up question all pushed an item to
   Analysis. Only a recorded wait or a blocking label does that now.

4. A worktree left on disk pinned its issue to In Progress forever. #593,
   #571, #542 and #466 all reported In Progress while their PRs (#618, #579,
   #591, #517) had merged. Staleness is decided by comparing the worktree's own
   branch against merged PRs -- an exact match, deliberately not the fuzzy
   issue-number match used to associate a worktree with an issue.

5. `blocked` did not fail Ready, so #571 reported Ready for Dev while labelled
   `blocked`. Definition of Ready criterion 5 now holds, including an
   unresolved `Blocked by #N`.

Ready for Dev also finally requires a Priority, which the design always
specified and the code deferred "until a board exists". It exists.

New: `last_comment` {author, days, is_reporter, is_bot}. Without it the digest
could not represent the transition that matters most to grooming -- the
reporter answering us. A count and a date cannot tell that from a nudge we
posted, which is why #621 crossed the Definition of Ready line unnoticed.

REJECTED while building this: mapping a merged PR to Done. It reclassified 7
open issues (#118, #120, #403 among them) as finished, and it contradicts this
project's rule that beta PRs omit `Closes #N` until graduation -- an open issue
with a merged fix is the NORMAL state. `merged_pr` is reported; it moves no
column, and a test pins that.

Merged PR bodies are reduced to their closing references before reaching jq;
passing 200 of them through --argjson overflows the argument list.

Tests: 25 pass. The `gh` shim now branches on the full argument string, because
`pr list` is called twice with different `--state` values and matching only the
subcommand returned the open list for both -- which would have made every open
PR look merged. Fixtures gained `createdAt` on comments, which real gh always
sends and whose absence failed `strptime` rather than testing anything. `_run`
now surfaces the digest's stderr instead of a bare "exit status 5".

* fix: only a still-open Blocked by #N fails Ready

Review of #623 found a real gap, and it contradicted an explicit claim: both
the commit message and SKILL.md said this enforced an "unresolved" blocker,
while `blocked_by` was a pure text scan that never checked whether the blocker
was still open.

A `Blocked by #N` line is never edited out of an issue body once N lands, so
that scan pins the item out of Ready for Dev permanently. That is the same
failure this script exists to fix, pointing the other way: an item reading
wrong relative to its real state.

`$issues` is already the open-issue list, so membership decides it with no
extra API call. `blocked_by` keeps the raw parse so the reference stays
visible; the new `blocked_by_open` is the subset that actually blocks, and
`$blocked` reads that.

Also pins the precedence question the review asked me to confirm rather than
guess at: a recorded wait DOES outrank a live worktree in `column`, because
unsettled scope must not read as progress. The risk is hiding active
undelivered code, so the worktree stays reported on the item — the wait
changes the column, not the evidence. Now tested and documented in the
reconciliation table instead of being implied.

27 tests pass.

* feat: a Rhythm pass that carries work from incoming to a ready PR

Every follow-up rule in the backlog skill had been written down and NONE had
ever fired. The 14-day reporter chase, the 28-day park, the reporter-replied
re-check, the stale-worktree handoff: all decoration, because each needed a
model to notice it and nothing scheduled one.

So the noticing is deterministic now and lives in scripts/backlog-rhythm.sh.
Every rule is a comparison over the digest — no judgement, no tokens. A quiet
backlog prints "RHYTHM: nothing due." for the cost of one process, which is
what makes it worth running on a timer at all. The PO agent is needed only to
ACT, and only when something is due.

The pass covers BOTH halves of the path to an approvable PR:

Issue side — recheck_ready, nudge_reporter, park, surface_discussion,
set_awaiting, set_priority, triage_labels, dispatchable.

PR side — and this is the half that actually hands the maintainer something:
  mark_ready           approved but still a draft   <- the finish line
  awaiting_maintainer  approved and out of draft
  request_review       draft with no review at all
  rework               changes requested
  resolve_conflict     CONFLICTING (produces no CI run, so it reads as
                       "checks never fired" and nobody investigates)

Those two states were invisible in practice. #615 and #617 sat APPROVED and
still drafts overnight with nothing left but the merge; #619 was never
reviewed at all. Nothing was watching either transition.

Ordering is load-bearing. PR actions come first because they are closest to
the finish line, and recheck_ready outranks the chases: nudging someone who
has already replied is the worst output this pass could produce.

Quiet time is measured from the LAST COMMENT, not updatedAt — a label change
or a board move bumps updatedAt, so an issue nobody has spoken on for a month
would look active and never age into a chase.

A bare COMMENTED review is not treated as a verdict, because the review bot
posts its inline notes as one before the summary.

Against the live board the pass finds 30 due actions, including PR #490
awaiting the maintainer, #162 park (quiet 54d), three stale worktrees and
three conflicted PRs.

RHYTHM_DIGEST_FILE / RHYTHM_PRS_FILE are test seams, the same shape as
BESS_ENV_FILE in gh-agent.sh. 16 tests pin the rules, including that a quiet
backlog is a noop and that a reply beats the chase.

Still not wired to a schedule — that is the invocation, not the logic, and it
is deliberately a separate step.

* refactor: hand unfinished PRs back to implement-issue instead of duplicating it

The previous commit built request_review / mark_ready / rework into the Rhythm
pass, which is a second copy of implement-issue Step 11. That contradicts the
argument used to put resume in Step 0 rather than in a separate skill: two
copies of one review loop means one of them goes stale.

It also mis-diagnosed the symptom. #615 and #617 did not sit APPROVED-but-draft
because nothing was watching for that state; they sat there because the
sessions that owned them exited before Step 11 finished. The fix belongs where
the loop already lives.

So every unfinished draft now resolves to ONE action, `resume_implementation`,
carrying the issue number so the handoff is directly runnable. Step 0 re-enters
at the earliest incomplete step, whether the PR needs a first review, a rework,
or just the ready flag it never got. Two fleet-level exceptions stay in the
pass, because implement-issue deliberately does not widen to them:
`awaiting_maintainer` (report only) and `resolve_conflict` (sweep-prs).

Adds the stalled-work rule this was missing: a LIVE worktree with no session
behind it is an implementation that stopped mid-flight -- the machine
restarted, the session was killed, or the agent exited between steps. Nothing
picked these up, and an audit found 34 such worktrees, 8 holding real unpushed
commits and one with 32. Against the live board it finds #466 and #602.

`pr == null` guards that rule so work with a PR is reported once, by the PR
branch, rather than twice.

It is always a RESUME, never a restart: Step 4 branches fresh from origin/main
and would delete commits that exist nowhere else. The detail string says so,
and a test pins it.

SKILL.md gains the reasons a future pass must not re-learn this: do not drive
the review loop here; a session reporting `working` may have written nothing
(three dispatches produced zero writes in one day while reporting healthy
state); and read `claude agents --json` unsandboxed, since ~/.claude/jobs is
sandbox-denied and a sandboxed listing returned 1 session where the truth was
17.

19 rhythm tests, 27 digest tests, gate green.

* fix: match Blocked by #N per line, and drop dead code the refactor left

Review of #623 found a real misclassification path in a PR whose whole point is
eliminating them.

`blocked_by` was a free `scan` over the issue body, so it matched the substring
regardless of what preceded it. "not blocked by #500 anymore" and "no longer
blocked by #500" both registered as live blockers -- and those are the natural
way to update an issue once its blocker resolves, so the false positive fired
exactly when the blocker was GONE. The item would be pinned out of Ready for
Dev permanently.

The severity is new, not latent: on main `blocked_by` was extracted and never
fed into `column()`, so a bad parse was inert. Gating `column()` on it is what
gave it teeth.

Matched per LINE and anchored to the line start now, optionally bulleted, which
is the convention the skill documents ("a `Blocked by #N` line in the issue
body"). Anchoring rejects the negations without a blacklist that would only
cover the phrasings someone happened to think of. It also fixes the reviewer's
third point: an untriaged issue merely mentioning a blocker in prose no longer
moves Backlog -> Analysis with no human triage behind it.

Four tests: the bulleted form still counts, three negated phrasings do not, and
an incidental mid-sentence mention does not reclassify.

Also from the same review:

- `human_comments` became dead code when `awaiting` stopped deriving
  `discussion` from comment activity. Removed, and the stale comment on
  `comments:` that still described that mechanism is corrected.
- SKILL.md claimed the board's custom-field JSON shape was "confirmed" where
  the previous text had explicitly said unconfirmed, without showing the
  evidence. It was verified live; the command and its result are now recorded,
  with a note that the tests fabricate that shape and so cannot prove it.

49 tests pass (27 digest + 19 rhythm, plus the 3 new negation cases).

* fix: resume a PR by its own number when no issue is linked

`implement-issue` is used for TODO.md items and refactors, not only for
issues, so a draft PR with no linked issue is normal rather than a defect. The
pass reported "no issue references this PR; finish it by hand", which left every
self-directed PR with no owner in the loop -- exactly how #620, #622 and #623
all ended up driven by hand today.

No flag distinguishes the two cases: GitHub numbers issues and PRs from ONE
sequence per repo, so a bare number is already unambiguous and Step 0 can
resolve whichever it is. An earlier draft of this used `--pr <n>`; that
distinction carries no information.

Where an issue IS linked it is still named, because it carries the diagnosis.
Where none is, the PR number is the handle, and a strong one: it holds the
branch, the diff, the scope assessment and the review verdict, which is
everything Step 0 reads.

Step 0 accepting a PR number is a matching change to implement-issue's SKILL.md,
which lives on the fix/review-verdict-placeholder branch (#622) where Step 0 was
added. Both have to land for the loop to cover this case.

Author: johanzander

.claude/skills/backlog/SKILL.md

@@ -14,26 +14,47 @@ never implement, and you never assign. Implementers pull the top of Ready.
 Every pass starts from one command. Do not read issues one by one to build a
 picture:
 
-    ./scripts/backlog-digest.sh
+    ./scripts/backlog-rhythm.sh      # what is DUE now — start here
+    ./scripts/backlog-digest.sh      # the full evidence, when you need detail
 
 Open an individual issue only when you are deciding about that issue.
 
 ## Prerequisites
 
-Board reads need `PROJECT_NUMBER` set and the board created (deferred —
-`scripts/backlog-board-init.sh`). Board writes need `BESS_PO_TOKEN` with
-`project` scope (also deferred). Until both exist, a pass fails loudly at the
-first board access in `backlog-digest.sh` — that failure is expected, not a
-bug to route around.
-
-**One thing to verify the first time a board exists.** No board has ever
-existed, so the JSON shape `gh project item-list --format json` uses for a
-custom field is unconfirmed; the digest assumes each item carries a top-level
-`.priority`. On the first real run, check that `priority` is populated rather
-than `null` for an item you have set a priority on. If it is `null`, fix the
-jq path in `scripts/backlog-digest.sh` — do not add a fallback that tries
-several shapes. A silently-null `priority` disables ranking axis 2 without
-any error.
+The board exists: **Project #1, "BESS Manager Backlog"**. `PROJECT_NUMBER=1`
+lives in the repo's `.env`, which `backlog-digest.sh` does **not** source — so
+export it first, or the digest exits with a "board has not been created yet"
+message that is misleading rather than wrong:
+
+    set -a; . ./.env; set +a; ./scripts/backlog-digest.sh
+
+Board writes need `BESS_PO_TOKEN` with `project` scope.
+
+The custom-field JSON shape is **confirmed against the live board**, not
+assumed: `gh project item-list --format json` puts each single-select value at
+the item's top level, so `.priority` and `.awaiting` read directly. Re-verify
+with the command itself rather than trusting this paragraph — the tests fabricate
+that shape, so they cannot prove it:
+
+    gh project item-list 1 --owner johanzander --format json \
+      | jq '[.items[] | {n: .content.number, p: .priority, a: .awaiting}] | .[0:3]'
+
+That returned populated `P1`–`P3` values and 17 items with `Awaiting` set. It
+matters because a wrong path fails silently — `.priority?` / `.awaiting?` resolve
+to `null` for every item with no error, which reads exactly like an ungroomed
+board.
+
+Field options, as they actually exist — do not invent values outside these sets:
+
+| Field | Options |
+|---|---|
+| `Status` | `Backlog`, `Analysis`, `Ready for Dev`, `In Progress`, `In Review`, `Done` |
+| `Priority` | `P1`, `P2`, `P3`, `P4` — **there is no `P0`** |
+| `Awaiting` | `reporter`, `discussion`, `upstream`, `analysis` |
+| `Source` | `issue`, `TODO` |
+
+The digest's `column` values match `Status` exactly, so reconciling a card is a
+string comparison, not a translation.
 
 ## When to Use
 
@@ -54,6 +75,36 @@ Post as the PO identity: `scripts/gh-agent.sh --as po issue comment ...`.
 If a board write fails for missing scope, stop and report
 `gh auth refresh -s project`. Never fall back to a file.
 
+## What the digest tells you, and why each field exists
+
+Four fields carry the grooming signal. Each was added because its absence
+caused a real misclassification:
+
+| Field | Meaning |
+|---|---|
+| `awaiting` | The blocking wait. **Outranks `analyzed`** — an item whose scope is unsettled is in *Analysis* no matter how far it got. Comes from the board field when set, else from a blocking label. |
+| `awaiting_source` | `board` or `label`. A `label` source with no board value is a triage action: set the field. |
+| `awaiting_suggested` | What the labels imply, so an unset field can be reconciled without guessing. |
+| `last_comment` | `{author, days, is_reporter, is_bot}`. **The reporter-replied signal.** A comment count and a last-activity date cannot tell "the reporter answered us" from "we posted a nudge", which is why the follow-up chase never fired. |
+| `stale_worktree` | The worktree's own branch has already merged, so it is rot, not progress. Hand it to `sweep-prs`. |
+| `blocked` | `blocked` label, or a `Blocked by #N` whose blocker is **still open**. Fails Ready outright. |
+| `blocked_by` / `blocked_by_open` | Every parsed reference, and the subset still open. Only the latter blocks — a `Blocked by #N` line is never edited out once N lands, so treating the raw scan as unresolved pins an item out of Ready forever. |
+
+**A wait outranks a live worktree, deliberately.** An item with a recorded wait
+reports *Analysis* even when a worktree is checked out for it, because unsettled
+scope must not read as progress. The worktree is still reported on the item
+(`worktree`, `worktree_branch`), so active undelivered code stays visible — the
+wait changes the column, not the evidence. Check those fields before assuming an
+*Analysis* item has no code behind it.
+
+**A human comment is not a wait.** `awaiting: discussion` used to be returned
+for any human comment, which pushed items to *Analysis* for ordinary traffic —
+thanks, a "me too", a follow-up question. Only a recorded wait or a blocking
+label does that now; `last_comment` is what you read to judge the rest.
+
+**`Ready for Dev` requires a `Priority`.** An analysed item with no priority is
+un-ranked, so it cannot be "next" — it stays in *Analysis* as a triage action.
+
 ## Definition of Ready
 
 Nothing is dispatched that has not crossed this line. A bug is Ready when:
@@ -64,6 +115,16 @@ Nothing is dispatched that has not crossed this line. A bug is Ready when:
 4. An approach is agreed (Stage 2 analysis, or the maintainer's say-so)
 5. No unresolved blocker
 
+**Criterion 4 is the one that gets skipped, and the `analyzed` label is not
+proof of it.** Stage 2 can diagnose a request correctly and still leave its
+design open — #96 was labelled `analyzed`, prioritised `P2`, carried no blocking
+label, and was still not implementable, because *how* to build it was undecided.
+It reported *Ready*, an implementation session was dispatched at it, and that
+session deadlocked on three design questions nobody was there to answer. When
+an approach is genuinely undecided, record it: set `Awaiting` and post the open
+questions on the issue, so a later attempt can rehydrate them rather than
+rediscover them.
+
 An enhancement is Ready when 3–5 hold and the user-visible outcome is stated.
 An item failing any criterion stays in Backlog or Analysis and becomes your
 follow-up, not a developer's problem.
@@ -90,8 +151,10 @@ always wins** — never trust a card's current position. Act on each mismatch:
 
 | Mismatch | Action |
 |---|---|
-| card *In progress*, no worktree, no PR | abandoned — move to *Ready*, report it |
-| worktree present, no session, no PR | the session died mid-issue. Report it and offer to relaunch; the branch's commits survive. **Never silently relaunch** — a session that died twice is telling you something |
+| card *In Progress*, no worktree, no PR | abandoned — move to *Ready for Dev*, report it |
+| `stale_worktree: true` | the branch already merged; the worktree is rot, not work. Hand to `sweep-prs`, and do not read it as progress |
+| worktree present, no session, no PR | the session died mid-issue. The branch's commits survive — resuming is `/implement-issue <n>`, whose Step 0 detects the prior work and re-enters at the right step. **Never silently relaunch**: a session that died twice is telling you something, and a background dispatch that reports `working` may have written nothing at all — verify by work product (`git -C <wt> log`, file mtimes), never by session state |
+| `last_comment.is_reporter` and `awaiting: reporter` | the reporter answered. Re-check the Definition of Ready — this wait may be satisfied, and it is the transition nothing used to notice |
 | PR `CONFLICTING` | hand to `sweep-prs` |
 | worktree whose PR merged | prune via `sweep-prs` |
 | issue closed, card not *Done* | move the card |
@@ -106,6 +169,70 @@ Also review the digest's `orphans` list (worktrees with no matching open
 issue, PRs with no `fixes/closes/resolves` reference) and hand any worktree
 or PR rot found there to `sweep-prs`.
 
+## Verb: rhythm — the unattended pass
+
+The one that carries work from incoming to a **ready PR**. Start here on every
+`/loop /backlog` tick:
+
+    scripts/backlog-rhythm.sh
+
+It answers "what is due right now" deterministically — every rule is a
+comparison over the digest, so a quiet backlog costs one process instead of a
+model pass, and `RHYTHM: nothing due.` is a legitimate noop tick. **Do not
+re-derive these by reading issues; act on what it lists.**
+
+Why it exists: every follow-up rule in this skill had been written down and
+**none had ever fired.** They each needed a model to notice them and nothing
+scheduled one, so the 14-day chase, the 28-day park and the reporter-replied
+re-check were decoration.
+
+Actions, and who does what:
+
+| Action | Do |
+|---|---|
+| `resume_implementation` | `/implement-issue <n>`. **The action that produces a ready PR** — Step 11 requests the review, acts on the verdict and runs `gh pr ready`. Covers a draft needing a first review, a rework, an approved PR that never got flipped, *and* a worktree whose session died |
+| `awaiting_maintainer` | nothing; report it. Out of draft is the finish line |
+| `resolve_conflict` | hand to `sweep-prs` |
+| `recheck_ready` | the reporter answered: re-check Definition of Ready, clear `Awaiting` if satisfied |
+| `nudge_reporter` | one nudge, as the PO identity |
+| `park` | move to *Backlog* — the chase went unanswered |
+| `surface_discussion` | summarise the thread, put the open question to the maintainer. **Never auto-park an open conversation** |
+| `set_awaiting` / `set_priority` / `triage_labels` | grooming debt: write the board field or label |
+| `dispatchable` | propose for dispatch — needs the maintainer's go-ahead |
+
+**This pass does not drive the review loop, and must not learn to.**
+`implement-issue` owns a PR from its first commit to `gh pr ready`; Step 11
+already requests the review, acts on the verdict and flips the PR. So every
+unfinished draft resolves to one action — hand it back — and a second copy of
+that loop is never built here. It is the same argument that put resume in Step 0
+instead of a separate skill: two copies of one loop means one of them goes stale.
+
+**Restarting a stalled issue is always a resume, never a fresh start.** Step 0
+re-enters at the earliest incomplete step. A restart runs Step 4, which branches
+from `origin/main` and deletes commits that exist nowhere else — an audit found 8
+abandoned branches carrying real work, one with 32 commits.
+
+**A session reporting `working` may have written nothing.** Three background
+dispatches in one day produced zero tracked-file writes while reporting healthy
+state, and `claude logs` returns only spinner frames. Read `claude agents --json`
+**unsandboxed** (`~/.claude/jobs` is sandbox-denied, so a sandboxed listing
+silently truncates — it returned 1 session where the truth was 17), and confirm
+progress by work product: commits, `MERGE_HEAD`, file mtimes.
+
+**Ordering matters.** Work `resume_implementation` first — it is the only action
+that ends in something approvable. `recheck_ready` outranks the chases, because
+nudging someone who has already replied is the worst output this pass can
+produce.
+
+**Quiet time is measured from the last comment, not `updatedAt`** — a label
+change or a board move bumps `updatedAt`, so an issue nobody has spoken on for a
+month would otherwise look active and never age into a chase.
+
+**A `dispatchable` item is a proposal, not a launch.** Dispatch spends real
+money and needs the go-ahead. And verify the item truly meets criterion 4 first:
+`Ready for Dev` is derived, and a design-heavy item will stop and ask a question
+no unattended session can answer.
+
 ## Verb: next
 
 Rank Backlog and Ready items in this order: