Guard pr merge --delete-branch against worktree conflicts - #14007

…esolved comments

`skills/dev-loop-pr-comments/read-comments.mjs` returns a pull request's unresolved comments as
one list: thread-anchored review comments, review bodies and issue comments, in a single entry
shape a consumer switches on by `origin`.

Exclusions are signals, never inferences. A resolved thread goes on GraphQL's own `isResolved`,
because REST cannot express it and a guess returns every comment the pull request ever carried; a
minimised comment goes because hiding one is a human saying it is dealt with; a PENDING review is
unsubmitted and a bodyless review is the envelope around inline comments already collected.

Absence is carried, not filled in. An outdated comment keeps `line: null` while its stale
`originalLine` travels separately, so *no line* and *line moved* stay distinguishable.

Bodies never reach a shell: `gh` is spawned with an argument array, `shell: false`, and its stdout
is parsed. All four connections paginate, the nested one by count because `gh --paginate` follows
only the first pageInfo in a response, and any failure exits non-zero with no JSON so that an empty
list can only mean no unresolved comments.

The folder holds the module alone — no SKILL.md, no manifest entry — so nothing installs or invokes
it yet. The normaliser is exported and importing it runs no fetch.

Verified live, read-only, no mutation issued: cli/cli#14007 → 5 review-thread + 2 review-body
entries against an independent query's 9 threads / 4 resolved / 5 unresolved comments / 3 reviews
of which 2 carry a body; bodies byte-identical for all 5; two entries with line=null,
originalLine=431, outdated=true. cli/cli#14027 → 3 threads all resolved, 2 review bodies, 1 issue
comment. cli/cli#14099 with the page size forced to 1 reproduces its full-page output byte for
byte, exercising every cursor including the per-thread comment refetch. Missing gh, an unknown pull
request, an unresolvable repository and a bad argument each exit non-zero with empty stdout.

Deviation: the changeset is hand-written as .changeset/140-read-unresolved-pr-comments.md rather
than generated by `npm run changeset`, which names files after human-id words; every tracked
changeset here is named <issue>-<slug>.md.

…checked, not just parsed

The check suite proved read-comments.mjs PARSED. Everything that makes it safe to point at a
pull request — which comments it drops, which fields it refuses to invent, and whether a body
survives — was unobserved. `scripts/pr-comment-read.mjs` observes it, at the same seam the
state-machine harness uses: the exported pure function, driven over fixtures, spawning nothing
and reaching no network.

Seven scenarios over the normaliser's public interface, none touching an internal. A resolved
thread is excluded whole while an unresolved one is kept whole; a minimised comment goes at all
three origins; a bodyless, whitespace-only or PENDING review is not a comment while a review that
said something is. An outdated comment keeps `line: null` with `originalLine` beside it, so the
scenario fails the moment anything substitutes the stale anchor. A body carrying backticks,
`$(...)`, quotes, a backslash, a pipe and a newline comes back byte for byte, through the JSON
round trip the reader actually prints. Every entry carries the same eleven keys whatever its
origin, and an empty pull request is an empty list.

Each scenario was confirmed to FAIL against a deliberately broken normaliser before being kept:
dropping the `isResolved` guard admits the resolved thread's two comments, and substituting
`originalLine` into `line` turns null into 431.

check.sh gains one stage after the bundled-module syntax stage, streaming its own ok/FAIL line per
scenario with only its exit code read — the state-machine stage's shape.

Verified live, read-only, no mutation issued (the module sends query documents only):
cli/cli#14027 → 3 entries (2 review-body, 1 issue-comment) against an independent query's 3
threads all resolved, 5 reviews of which 2 carry a body and 3 are bodyless, 1 issue comment.
cli/cli#14007 → 7 entries (5 review-thread, 2 review-body) against 9 threads / 4 resolved /
5 unresolved carrying 5 comments none minimised, 3 reviews of which 2 carry a body, 0 issue
comments; both null-line entries report originalLine=431, outdated=true, matching the independent
query exactly. Both runs exited 0 with empty stderr and one parseable JSON document on stdout.

`npm run check` green, including `claude plugin validate . --strict` with the manifest-unlisted
skills/dev-loop-pr-comments/ folder in place.

…te their intent

Step 3's table gains a `#` key and a Reason column, and stays one line per
comment: what runs past a clause goes to a keyed expansion beneath it. A
`disagreed with` row is marked `(!)` — plain ASCII, so it reads the same in a
terminal as in GitHub's renderer — and its expansion is mandatory. A fix row's
one-clause statement of intent is written once and is the string Step 5 hands
the writer. Steps 4, 5 and 10 now render that one definition rather than
restating it, and the ledger comment carries it as the copy that outlives the
run.

AC 8 — live, read-only verification of the classification as written
====================================================================
Comments read through skills/dev-loop-pr-comments/read-comments.mjs. No `gh`
write of any kind, no worktree provisioned, no gate answered: this exercised
Step 3 only.

cli/cli#14007 — "Guard `pr merge --delete-branch` against worktree conflicts",
7 unresolved comments, all 7 classified. Supplied three of the four reasons:

- out of scope for this branch — babakks, pkg/cmd/pr/merge/merge.go:410,
  discussion_r3719732624 ("In a follow-up we can extract git commands used in
  `pr checkout --worktree`"). Evidence: the separate work is a follow-up
  extracting the git commands `pr checkout --worktree` runs so they can reuse
  this branch's `ToplevelDir` — the commenter names it a follow-up himself.

- already addressed — babakks, merge.go stale anchor 431 (outdated),
  discussion_r3719948135 ("**nitpick:** `len(worktrees)` is always `> 0`").
  Evidence: ea3d3191f "Simplify main-worktree head check, drop misleading len
  guard" drops the `len(worktrees) > 0 &&` conjunct the comment calls
  misleading; merge.go is its only file.
  Two more rows took the same reason: discussion_r3720203296 (same commit — its
  new `if` is that comment's suggestion block character for character) and
  copilot-pull-request-reviewer's review body pullrequestreview-4810038780,
  whose two stated grounds are both met by d7fe64365 "Handle head branch checked
  out in main worktree during pr merge cleanup" — it handles that case in
  `deleteLocalBranch` and adds `TestParseWorktrees` to git/client_test.go, the
  direct unit tests the review says are missing.

- disagreed with (!) — babakks, review body pullrequestreview-4863309209, the
  proposal to restructure `deleteLocalBranch` around "where is the head branch
  checked out". Evidence, in full, is the expansion; in short: the reviewer
  states in the same comment that the scenarios are already covered, so the
  restructure changes no behaviour, and rewriting the whole cleanup flow inside
  a pull request that exists to guard one command widens the blast radius of a
  bug fix. Declined outright rather than deferred, which is what separates it
  from the `out of scope` row above.

Two rows classified fix: discussion_r3719711480 (rename `Worktree.Branch` to
`Ref` at git/objects.go:82) and discussion_r3720001340 (return the errors from
`Worktrees` and `ToplevelDir` instead of discarding them — still `worktrees, _
:=` at head). More than one fix row, so a real run would stop at Step 4.

No comment on 14007 classified `question`: the one opening "**question:** why
can't we simplify this `if`-statement" carries a ```suggestion``` block, so it
asks for a change, and ea3d3191f made it — `already addressed`, above.

cli/cli#13948 — "Add issue field support to gh CLI" — supplied the fourth:

- question — zwick, pkg/cmd/issue/create/create.go:102,
  discussion_r3639448805. Evidence, verbatim: "Can I create an issue with
  multiple Issue Field Values?"

No comment across either pull request came back `unclassified`: every one fit an
intent, and every skip's evidence was produced.

Deviation: the changeset was hand-written at .changeset/142-skip-reasons.md
rather than generated with `npm run changeset`, which names files with a random
human-id. Every existing changeset here is named `<issue>-<slug>.md`.

…n resolves to its default

A leading `auto` puts the harness in unattended mode, read off the arguments once
and carried as one value: no later step re-derives it, and no other argument and
no profile key overrides it.

Suppression removes Step 4's question and nothing else. Every comment is still
classified, the table is still rendered and still written to Step 5's file, and
each of the gate's questions resolves to a stated default — every fix row
proceeds, a skip stays skipped with its reason and evidence, an unclassified row
is reported and acted on by nothing, and a table with no fix row stops the run
having provisioned nothing. Step 1's five refusals and Step 6's three profile
reads are preconditions rather than gates and fire under both modes, so no
unattended default is invented for a profile key.

Where the gate asked, an unattended run posts the table on the pull request
instead — one rendering of Step 3's table, quoted heredoc, body on stdin. Step 7
now passes the run's real mode; `skillDir` stays absent under both modes, which
is the execute phase's documented no-notifier configuration.

Verified: npm run check (exit 0); the table rendered from cli/cli#14007's seven
unresolved comments through read-comments.mjs, read-only, seven columns on every
row; a quoted heredoc carried a body holding backticks, $(...), quotes and $HOME
byte for byte with nothing executed.

…on the pull request

Step 10 becomes the run's conclusion rather than only its ledger, and under
`unattended` it posts on every path past Step 4's table — the ones that ended
before the phase was ever dispatched included. That table told the pull request a
worktree, these commits and a push were coming, and the run that promised them is
the only thing that can say they never came. It is a second comment beside it and
never an edit to it, which keeps the two-comment count and the append-only rule
one fact rather than two.

A run that ended fills what sections it has, leaves out the ones it cannot, adds
Step 9's account — label, stage, reason verbatim, diagnosis, attempts — and
closes on what nothing else records once this session goes: the kept worktree by
path, the kept table file by path, and the run handle. That handle is a derived
fact, `$CLAUDE_CODE_SESSION_ID` read once, written in exactly one place and never
in a message; unset or empty is a missing line and nothing else about the run
changes.

The table file's lifetime follows the work rather than the comment alone: deleted
on the pushed path once the conclusion carrying its content has posted and never
before it, kept beside every worktree that is kept, and named in the comment
there — a worktree without the plan its commits were made against is one nobody
can pick up.

Deviation: the plan's assumption that everything after `start` gets an
explanation comment is narrowed to the paths past Step 4's table. A run that ends
at Step 2 — a failed read, or no unresolved comments — promised the pull request
nothing and posted no table, so its account is its closing message and it writes
nothing on somebody else's artifact.

Verified: npm run check (exit 0); a conclusion comment for an ended run rendered
from cli/cli#14007's seven real unresolved comments through read-comments.mjs,
read-only with no gh write and no worktree, then fed through a quoted heredoc
byte for byte — backticks, $(id), ${HOME}, quotes and apostrophes intact, nothing
executed and nothing expanded; the handle line present with
$CLAUDE_CODE_SESSION_ID set and absent rather than empty with it unset; an
unconfigured channel silent and exit 0.

Not verified here, and reported rather than claimed: acceptance criterion 8's
unattended end-to-end run. The harness dispatches the Workflow tool and a
dispatched subagent has none, so this lane cannot drive the execute phase.