docs(agents): require disposition reply before resolving review threads (#3693) (#3740)

* docs(agents): require disposition reply before resolving review threads (#3693)

Teach the disposition-reply convention across every agent/skill surface
that calls or references resolveReviewThread, ahead of #3732's
resolved_without_disposition convergence gate going live. Without this,
the gate would block ~every new PR since no doc currently instructs a
disposition reply before resolving.

Convention: before any resolveReviewThread call, post a reply carrying
  Disposition: fixed | refuted | superseded | follow-up
  Evidence: <commit sha + test name> / <file:line + why> /
            <superseding head sha + seam> / <issue #N + why non-blocking>
A thread must never be resolved with zero reply — the resolved-to-clear
anti-pattern the #3647 incident shipped 6 live P1 defects through.

- .claude/reference/review-convergence.md: new canonical
  "Disposition-reply convention" section — single source of truth every
  other doc links to, avoiding the five-file drift this doc already
  documents as this repo's recurring control-plane failure mode.
- .claude/commands/pr-respond.md: step 4/4.5 require the disposition
  reply before the resolveReviewThread mutation call.
- .claude/agents/pr-responder.md: steps 4/5 and the Principles section
  require the same before resolving.
- .claude/commands/pr-ready.md, .claude/agents/ops.md, CLAUDE.md:
  verification-side references updated to cite the canonical convention.

Does not touch scripts/ci/check-pr-review-convergence or its fixtures/
tests (PR #3732, already reviewed) — docs/skills only.

* fix(docs): describe resolved_without_disposition gate as upcoming, not live

Addresses review feedback on PR #3740 (chatgpt-codex-connector, P2):
scripts/ci/check-pr-review-convergence still only blocks on pending
reviewers, stale human reviews, and unresolved threads — it never reads
comments.totalCount or emits resolved_without_disposition. The previous
wording in all 6 docs implied that gate already mechanically enforces
the disposition-reply convention (and review-convergence.md referenced
a nonexistent "item 4 in The contract"), which would let /pr-ready and
ops get exit 0 on a PR whose threads were resolved with zero reply and
incorrectly believe it's blocked.

Reworded every surface to state plainly: the resolved_without_disposition
detection is proposed in #3732, which is deliberately held back for a
dogfood-advisory-first rollout (this convention lands and is followed
first, so the mechanical gate doesn't retroactively block PRs already
in flight when it goes live). Until #3732 merges, the convention is
process discipline verified by the agent/reviewer doing the work, not
yet proven by the script's exit code.

Disposition: fixed
Evidence: this commit — reworded review-convergence.md, pr-respond.md,
pr-responder.md, pr-ready.md, ops.md, CLAUDE.md to describe the gate as
upcoming (#3732, held back) rather than live; removed the dangling
"item 4 in The contract" cross-reference.

* fix(docs): align disposition-format examples with the canonical spec

Addresses review feedback on PR #3740 (factory-droid, 4x P2):

1. pr-respond.md step 4 examples used ';' as the evidence separator and
   inline '/' between Disposition and Evidence, drifting from the
   canonical `Evidence: <commit sha + test name>` ('+' joins compound
   evidence; '/' only separates the four alternative evidence shapes)
   spec in review-convergence.md. An agent copy-pasting the example
   would emit non-canonical replies.
2. CLAUDE.md, ops.md, pr-ready.md still said "accepted-with-follow-up"
   in the parenthetical disposition list, while the canonical convention
   (and every other list in this same PR) uses "follow-up". Two names
   for one concept in docs that link to each other.
3. pr-responder.md's Principles section "Reply with evidence" example
   was never migrated to the Disposition:/Evidence: format the step
   list two sections above now requires — an agent following the
   Principles bullet for a CI title fix would emit a free-form reply
   that satisfies neither the canonical format nor any future
   content-quality check.

Disposition: fixed
Evidence: this commit — pr-respond.md examples now use '+' within
evidence and separate Disposition:/Evidence: lines; CLAUDE.md/ops.md/
pr-ready.md now say "follow-up" consistently; pr-responder.md's
Principles bullet now demonstrates the canonical format for both a
fix and a refute.

Not fixed here (out of scope, flagged for follow-up): the reviewer also
noted scripts/ci/check-pr-review-convergence:32,365 still says
"accepted-with-follow-up" in its own BLOCK-line wording. That script is
PR #3732's territory (already reviewed, deliberately not touched by
this docs-only PR) — the drift there should be fixed when #3732 lands.

Author: EffortlessSteven

.claude/agents/ops.md

@@ -20,7 +20,7 @@ review code — that's the reviewers' job. You gate trusted change.
 - If CI fails, route to a fixer — don't debug yourself.
 - After parser merges, ratchet the corpus with `just cpan-corpus-ratchet`.
 - **Check both label AND draft state.** `merge-ready` label and `isDraft: false` are independent — a PR needs both. Use `/pr-ready` to exit draft if missed.
-- **Never merge — and never enable/retain auto-merge — while any requested review is still active or any substantive review conversation thread remains unresolved.** Threads must be resolved for a reason (fixed/refuted/superseded/accepted-with-follow-up), not performatively. `reviewDecision` alone doesn't prove either condition (it says nothing about thread resolution, and a review can predate the current push) — run the canonical review-convergence check (`scripts/ci/check-pr-review-convergence <number>`, see [.claude/reference/review-convergence.md](../reference/review-convergence.md)) in `/ops-merge-batch` step 2 before merging. Do not reproduce or modify its query locally.
+- **Never merge — and never enable/retain auto-merge — while any requested review is still active or any substantive review conversation thread remains unresolved.** Threads must be resolved for a reason (fixed/refuted/superseded/follow-up), each backed by a machine-readable `Disposition:`/`Evidence:` reply posted BEFORE resolution — never performatively. See the canonical convention in [.claude/reference/review-convergence.md § Disposition-reply convention](../reference/review-convergence.md#disposition-reply-convention-before-calling-resolvereviewthread) — the resolved-to-clear pattern that shipped 6 live P1 defects through #3647. **Not yet mechanically enforced:** the `resolved_without_disposition` gate proposed in #3732 (which would flag any resolved thread with no reply) is deliberately held back for a dogfood-advisory-first rollout, so `check-pr-review-convergence` does not yet detect this — verify it yourself before merging, don't rely on the script's exit code for it. `reviewDecision` alone doesn't prove either condition (it says nothing about thread resolution, and a review can predate the current push) — run the canonical review-convergence check (`scripts/ci/check-pr-review-convergence <number>`, see [.claude/reference/review-convergence.md](../reference/review-convergence.md)) in `/ops-merge-batch` step 2 before merging. Do not reproduce or modify its query locally.
 - **PR titles must end with `(#NNN)`.** validate-title CI check enforces this. If a PR fails on title, fix the title, don't skip the check.
 - **Don't rebase unless conflicts exist.** Unnecessary rebases trigger CI cascades on parallel PRs.
 - When main gets a CI fix, use `gh pr update-branch` on queued PRs, not `gh run rerun` (stale context).

.claude/agents/pr-responder.md

@@ -41,8 +41,23 @@ For each bot comment / CI failure / review conversation:
 1. **Classify** — fix / refute / supersede / follow-up
 2. **Fix it on the branch** (or gather the refute/supersede/follow-up evidence) — checkout, edit, commit, push. For **follow-up**, don't just note it — create or identify the tracked issue first; a follow-up with no issue number is deferred work that silently disappears once the thread closes.
 3. **Prove it** — re-run the relevant check/test
-4. **Reply with evidence** — state what you fixed, with commit hash or reasoning; for follow-up, cite the issue number
-5. **Resolve the thread** — for the real reason (fixed/refuted/superseded/accepted-with-follow-up), never performatively
+4. **Reply with a machine-readable disposition** — BEFORE resolving,
+   post a reply on the thread carrying the canonical format (see
+   [.claude/reference/review-convergence.md § Disposition-reply
+   convention](../reference/review-convergence.md#disposition-reply-convention-before-calling-resolvereviewthread)):
+   ```
+   Disposition: fixed | refuted | superseded | follow-up
+   Evidence: <commit sha + test name>  /  <file:line + why>  /  <superseding head sha + seam>  /  <issue #N + why non-blocking>
+   ```
+5. **Resolve the thread** — only after step 4's disposition reply exists.
+   **A thread must never be resolved with zero reply** — that's the
+   resolved-to-clear anti-pattern the #3647 incident shipped through (15
+   threads `resolveReviewThread`'d with no reply, 6 live P1 defects merged
+   on main). Required now as **process discipline**: the mechanical
+   `resolved_without_disposition` detection is proposed in #3732 (held
+   back for a dogfood-advisory-first rollout, so it doesn't retroactively
+   block PRs already in flight) and does not yet block in
+   `check-pr-review-convergence`.
 6. **Verify review convergence** before treating the PR as ready — run the
    canonical review-convergence check (see
    [.claude/reference/review-convergence.md](../reference/review-convergence.md)):
@@ -53,8 +68,16 @@ For each bot comment / CI failure / review conversation:
 
 - **Fix everything, argue nothing you can't back with evidence.** If CI says title is wrong, fix the title. If clippy warns, fix the warning. If a test fails, fix the code. If a comment is wrong, refute it with evidence rather than silently ignoring it.
 - **Verify after fixing** — `cargo test -p <crate>` after each commit.
-- **Reply with evidence** — "Fixed: updated PR title to include (#NNN). CI should re-run."
-- **Resolve conversations for a reason** — fixed/refuted/superseded/accepted-with-follow-up, not performatively. Never resolve a thread just to clear it.
+- **Reply with the canonical disposition** — every thread reply carries `Disposition:` and `Evidence:` per the convention, e.g. `"Disposition: fixed\nEvidence: <commit-sha> + test <name>"` for a title fix (`(#NNN)` added, CI re-run confirms); `"Disposition: refuted\nEvidence: <file:line>: <reasoning>"` for a refute.
+- **Resolve conversations for a reason, never performatively.** Post the
+  `Disposition:`/`Evidence:` reply (see
+  [.claude/reference/review-convergence.md](../reference/review-convergence.md#disposition-reply-convention-before-calling-resolvereviewthread))
+  BEFORE calling `resolveReviewThread`. Never resolve a thread just to
+  clear it — zero-reply resolution is the resolved-to-clear anti-pattern
+  #3647 shipped 6 live P1s through. The `resolved_without_disposition`
+  gate that will mechanically block on this (#3732) is deliberately held
+  back for a dogfood-advisory-first rollout — follow the convention now
+  regardless of whether the script enforces it yet.
 - **Never enable or retain auto-merge while any requested review is still active or any substantive thread is unresolved** — main mechanically requires conversation resolution before merge; verify reviewer completion before signaling readiness.
 - **Don't add improvements.** Fix what's broken, nothing more. Extra changes confuse the deep reviewer.
 

.claude/commands/pr-ready.md

@@ -55,11 +55,21 @@ scripts/ci/check-pr-review-convergence $NUMBER
 Do not reproduce or modify its query locally.
 
 **Never enable or retain auto-merge, and never mark a PR ready for merge
-pickup, while the check above exits non-zero.** Resolve threads for a reason
-(fixed/refuted/superseded/accepted-with-follow-up), not performatively — main
-mechanically requires conversation resolution before merge. If the script
-exits non-zero, **STOP** and report which reviewer or thread is still
-pending (its `BLOCK` lines name them) instead of proceeding.
+pickup, while the check above exits non-zero.** Threads must be resolved
+for a reason (fixed/refuted/superseded/follow-up), each with
+a machine-readable disposition reply posted BEFORE resolution — see the
+canonical convention in
+[.claude/reference/review-convergence.md § Disposition-reply
+convention](../reference/review-convergence.md#disposition-reply-convention-before-calling-resolvereviewthread).
+Never performatively — main mechanically requires conversation resolution
+before merge. **Note:** the `resolved_without_disposition` detection
+(flagging a resolved thread with no reply) is proposed in #3732 and is
+**not yet live** in `check-pr-review-convergence` — it's held back for a
+dogfood-advisory-first rollout. Until it lands, treat the disposition
+requirement as process discipline you verify yourself, not something the
+script's exit code proves. If the script exits non-zero, **STOP** and
+report which reviewer or thread is still pending (its `BLOCK` lines name
+them) instead of proceeding.
 
 ### 4. Mark ready and signal merge-readiness
 

.claude/commands/pr-respond.md

@@ -38,24 +38,50 @@ been classified `Fix`):
 3. Commit: `fix(review): address <reviewer> feedback — <what>`
 
 ### 4. Reply to comments
+
+**Before resolving any thread**, reply with a machine-readable disposition —
+see the canonical convention in
+[.claude/reference/review-convergence.md § Disposition-reply
+convention](../reference/review-convergence.md#disposition-reply-convention-before-calling-resolvereviewthread):
+
+```
+Disposition: fixed | refuted | superseded | follow-up
+Evidence: <commit sha + test name>  /  <file:line + why>  /  <superseding head sha + seam>  /  <issue #N + why non-blocking>
+```
+
 ```bash
 gh api repos/:owner/:repo/pulls/$ARGUMENTS/comments/<comment-id>/replies \
-  -f body="Fixed in <commit-hash>. <brief explanation>"
+  -f body="Disposition: fixed
+Evidence: <commit-hash> + test <name>"
 ```
 
 Or for general review comments:
 ```bash
 gh pr comment $ARGUMENTS --body "Addressed review feedback:
-- <comment 1>: fixed in <hash>
-- <comment 2>: <explanation>
+- <comment 1>: Disposition: fixed
+  Evidence: <hash> + test <name>
+- <comment 2>: Disposition: refuted
+  Evidence: <file:line>: <reasoning>
 "
 ```
 
 ### 4.5 Resolve the conversation thread
 
-After replying with evidence, resolve the GitHub review thread — only once it
-has a real disposition (fixed/refuted/superseded/accepted-with-follow-up), not
-performatively:
+**A thread must never be resolved with zero reply** — that's the
+resolved-to-clear anti-pattern the #3647 incident shipped through (a
+responder silently `resolveReviewThread`'d 15 threads with no reply and no
+evidence; the PR merged with 6 live P1 defects). Follow this now as
+process discipline: `check-pr-review-convergence` does **not yet**
+mechanically detect a missing disposition reply — that detection
+(`resolved_without_disposition`) is proposed in #3732, held back for a
+dogfood-advisory-first rollout so it doesn't retroactively block PRs
+already in flight. A resolved thread with zero reply currently passes the
+script silently. Once #3732 lands, a resolved thread whose
+`comments.totalCount <= 1` (no reply beyond the original comment) will be
+`BLOCK`ing.
+
+Only after step 4's disposition reply has been posted, resolve the GitHub
+review thread:
 
 ```bash
 gh api graphql -f query='mutation { resolveReviewThread(input: {threadId: "<thread-id>"}) { thread { isResolved } } }'

.claude/reference/review-convergence.md

@@ -129,3 +129,60 @@ outside of tests.
 If you are adding a new surface that needs this check, call the script —
 do not write a new GraphQL query for it, even if it looks like "just this
 one field."
+
+## Disposition-reply convention (before calling `resolveReviewThread`)
+
+**This is the canonical statement of the convention — every other doc
+that mentions resolving a review thread links here instead of restating
+it, to avoid the same five-file drift described in "Why this exists"
+above.**
+
+Before **any** `resolveReviewThread` GraphQL call, the agent MUST first
+post a reply comment on that thread carrying a machine-readable
+disposition:
+
+```
+Disposition: fixed | refuted | superseded | follow-up
+Evidence: <commit sha + test name>  /  <file:line + why>  /  <superseding head sha + seam>  /  <issue #N + why non-blocking>
+```
+
+- `fixed` — the commit SHA that fixed it, plus the test name that proves it.
+- `refuted` — the `file:line` and the invariant/reasoning showing the
+  concern doesn't apply.
+- `superseded` — the head SHA of the change that overtook this thread and
+  the seam it replaces.
+- `follow-up` — the tracked issue number and why it's non-blocking here.
+  Never write "will follow up" without a real issue number — untracked
+  follow-up work silently disappears once the thread closes.
+
+**A thread must never be resolved with zero reply.** That is the
+resolved-to-clear anti-pattern the #3647 incident shipped through: a
+responder silently `resolveReviewThread`'d 15 threads with no reply and no
+evidence, and the PR merged with 6 live P1 defects because nothing forced
+a reason to exist. Sequence is always **reply, then resolve** — never
+resolve first and explain later, never resolve without replying at all.
+
+**Mechanical enforcement status (as of this writing): NOT YET LIVE.**
+`scripts/ci/check-pr-review-convergence` currently blocks only on pending
+reviewers, stale human reviews, and unresolved threads (items 1-3 above)
+— it does not read `comments.totalCount` and does not emit
+`resolved_without_disposition`. That detection (flagging any resolved
+thread whose `comments.totalCount <= 1` — no reply posted beyond the
+original review comment — as `BLOCK`ing) is proposed in #3732, which is
+deliberately **held back** for a dogfood-advisory-first rollout: this
+convention lands and is followed by agents first, so the mechanical gate
+doesn't retroactively block PRs already in flight when it goes live. Until
+#3732 merges, a resolved thread with zero reply passes
+`check-pr-review-convergence` silently — follow this convention as
+**process discipline**, verified by the agent/reviewer doing the work, not
+yet by the script's exit code. Once #3732 lands, the script enforces it
+mechanically. Even then, the script can only verify a reply's *existence*,
+not its *content* — following the exact `Disposition:`/`Evidence:` format
+above is what makes the reply useful to a human reader and to any future
+content-quality check, not just sufficient to pass the mechanical gate.
+
+Every agent/skill that calls or instructs `resolveReviewThread` follows
+this convention: `.claude/commands/pr-respond.md` step 4.5,
+`.claude/agents/pr-responder.md` step 5, and the verification-side
+references in `.claude/commands/pr-ready.md` step 3.5 and
+`.claude/agents/ops.md`.

CLAUDE.md

@@ -123,8 +123,16 @@ recipe: [CI_GATE_PLAYBOOK.md](docs/reference/CI_GATE_PLAYBOOK.md).
 
 **Never enable or retain auto-merge while any requested review is still active or any
 substantive review conversation remains unresolved.** Resolve threads for a reason
-(fixed/refuted/superseded/accepted-with-follow-up), not performatively — main
-mechanically requires conversation resolution before merge.
+(fixed/refuted/superseded/follow-up), each backed by a machine-readable
+`Disposition:`/`Evidence:` reply posted BEFORE resolution — never performatively. Main
+mechanically requires conversation resolution before merge. The
+`resolved_without_disposition` gate — which will mechanically block any resolved
+thread with no reply, the resolved-to-clear pattern that shipped 6 live P1 defects
+through #3647 — is proposed in #3732 but **deliberately held back** for a
+dogfood-advisory-first rollout, so it doesn't retroactively block PRs already in
+flight; until it lands, follow the convention as process discipline, not yet
+mechanically enforced. Canonical convention: [review-convergence.md § Disposition-reply
+convention](.claude/reference/review-convergence.md#disposition-reply-convention-before-calling-resolvereviewthread).
 
 ## Quick reference