Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 61 additions & 4 deletions .claude/skills/pr-management-triage/actions.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,16 @@ on a still-open PR is a worse state than no comment at all.
The PR bypassed F4 because of post-label regression (rebase /
push re-introduced a deterministic failure — see
[`strip-ready-on-downgrade`](classify-and-act.md#hard-rules-cross-cutting-the-table)).
Strip the label as the **first** mutation, before converting
to draft, so the queue position is corrected even if a later
step fails:

**Branch on the merit-discussion exception.** Before mutating,
evaluate
[`merit_discussion_thread_present`](classify-and-act.md#merit_discussion_thread_present)
on the PR.

**Case A — no merit discussion present** (the strip-and-draft
default). Strip the label as the **first** mutation, before
converting to draft, so the queue position is corrected even
if a later step fails:

```bash
# 0. Remove the now-stale ready-for-review label (idempotent —
Expand All @@ -75,6 +82,26 @@ manually), but stranding the PR in a half-state would be
worse. The maintainer-facing preview should note when step 0
will run so the proposal is honest about both state changes.

**Case B — merit discussion present** (per the exception in
[`strip-ready-on-downgrade`](classify-and-act.md#hard-rules-cross-cutting-the-table)).
Skip step 0 (label stays) and step 1 (PR stays out of draft).
Post only the violations comment:

```bash
# 1. Post the violations comment (label stays; PR stays open).
gh pr comment <N> --repo <repo> --body-file /tmp/pr-<N>-draft-body.md
```

The maintainer-facing preview MUST surface that the merit
discussion was detected and that the action is being
de-escalated from `draft` to `comment-only` for this reason
— include the URLs of the maintainer-opened unresolved review
thread(s) that triggered the exception so the maintainer can
sanity-check the call. The violations comment body is
unchanged from Case A; it informs the author that mechanical
issues remain even though the discussion is what's keeping
the label on.

### If the PR is already a draft

Skip the `gh pr ready --undo` step. Post only the comment. The
Expand Down Expand Up @@ -116,10 +143,14 @@ the people it's for.
When the upstream classification is `deterministic_flag` and the
PR carries the label (regression bypass of F4 — see
[`strip-ready-on-downgrade`](classify-and-act.md#hard-rules-cross-cutting-the-table)),
strip the label **before** posting the comment:
strip the label **before** posting the comment — **unless**
[`merit_discussion_thread_present`](classify-and-act.md#merit_discussion_thread_present)
holds, in which case the label stays and only the comment is
posted.

```bash
# 0. Remove the now-stale ready-for-review label.
# SKIP this step when merit_discussion_thread_present holds.
gh pr edit <N> --repo <repo> --remove-label "ready for maintainer review"

# 1. Post the violations comment
Expand All @@ -136,6 +167,11 @@ or static-check-only failures). `stale_review` and explicit
signals and the ready-for-review queue position is still valid
information for the reviewer.

When the merit-discussion exception applies, the
maintainer-facing preview MUST surface that step 0 is being
skipped and quote the URL(s) of the maintainer-opened
unresolved review thread(s) that triggered the exception.

---

## `close` — close with comment and quality-violations label
Expand Down Expand Up @@ -166,6 +202,27 @@ inside a `close` group, the maintainer confirms each PR
individually — a wrongly-closed PR is the hardest mistake to
recover from.

### If the PR carries `ready for maintainer review` and a merit discussion is in flight

When the PR carries `ready for maintainer review` AND
[`merit_discussion_thread_present`](classify-and-act.md#merit_discussion_thread_present)
holds, the
[`strip-ready-on-downgrade`](classify-and-act.md#hard-rules-cross-cutting-the-table)
exception applies: **skip step 2** (do not close the PR) and
**do not strip the ready-for-maintainer-review label**. Steps
1 and 3 still run — the close comment surfaces the
queue-pressure reasoning, and the quality-violations label
records that the PR was flagged. The PR remains open with
both labels, surfaced for human review.

The maintainer-facing preview MUST surface that step 2 is
being skipped and quote the URL(s) of the maintainer-opened
unresolved review thread(s) that triggered the exception.
Closing a PR with an active maintainer review discussion is
strictly more destructive than the queue-pressure problem
`close` exists to solve — a human maintainer must make that
call, not the skill.

---

## `mark-ready` — add `ready for maintainer review` label
Expand Down
72 changes: 69 additions & 3 deletions .claude/skills/pr-management-triage/classify-and-act.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,47 @@ Action verbs are defined in [`actions.md`](actions.md).
NOT strip the label: those classify the regression as
transient (flaky CI, missing base merge, reviewer hasn't
responded) and the label is still informative if the
follow-up succeeds. Implementation: see
[`actions.md#draft`](actions.md#draft--convert-to-draft-and-post-violations-comment)
and [`actions.md#comment`](actions.md#comment--post-violations--stale-review--ping-comment).
follow-up succeeds.

**Exception — merit-discussion-in-flight.** If
[`merit_discussion_thread_present`](#merit_discussion_thread_present)
holds on the regressed PR, the strip-ready-on-downgrade
rule does **not** fire. Additionally:

- A `draft` action skips the `gh pr ready --undo` step
(the PR stays out of draft). The violations comment is
still posted so mechanical issues remain surfaced for
the author. The action effectively degrades to a
`comment` action that preserves the label.
- A `close` action skips the close step (the PR stays
open). The comment is still posted; the
quality-violations label is still applied. Closing a PR
with an active maintainer review discussion is more
destructive than the queue-pressure problem `close`
exists to solve.
- A `comment` action posts the violations comment but
does not strip the label.

Rationale: the `ready for maintainer review` label exists
to attract senior eyes, and an unresolved maintainer review
thread is exactly the moment those eyes are most valuable.
Stripping the label or pushing the PR back to draft
mid-discussion makes it disappear from the maintainer queue
at the worst possible time. CI red / lint failures / merge
conflicts and a live design debate are orthogonal: a
maintainer can weigh in on design even when CI is red. The
precondition is deliberately broad — contributor-author
threads alone do not satisfy it (those are not the merit
signal the label defers to), but any maintainer-opened
unresolved thread does, regardless of body length or when
it was opened relative to the label-add. Source: user-scope
feedback memory
`feedback-ready-for-maintainer-review-label`.

Implementation: see
[`actions.md#draft`](actions.md#draft--convert-to-draft-and-post-violations-comment),
[`actions.md#comment`](actions.md#comment--post-violations--stale-review--ping-comment),
and [`actions.md#close`](actions.md#close--close-with-comment-and-quality-violations-label).

---

Expand Down Expand Up @@ -241,6 +279,34 @@ but only `failed_checks` feeds the decision table.
fired is unresolved threads (`statusCheckRollup.state` is
`SUCCESS`, `mergeable != CONFLICTING`).

### `merit_discussion_thread_present`

True when the PR has at least one unresolved review thread
whose first comment is from a `COLLABORATOR`/`MEMBER`/`OWNER`
(the same collaborator-author qualifier as
[`unresolved_threads_only`](#unresolved_threads_only)).

This is the "active maintainer review discussion" signal. No
timing qualifier is applied — a substantive design / approach
/ scope / correctness discussion can have started either
before or after the `ready for maintainer review` label was
added, and in either case the label must not be stripped
while the discussion is in flight. The precondition
deliberately does not filter by body length or thread
content: an explicit maintainer act of opening a review
thread is treated as substantive engagement on its own.
Contributor-author unresolved threads do NOT satisfy this
precondition (mirrors the
[`unresolved_threads_only`](#unresolved_threads_only)
collaborator qualifier — contributor-to-contributor side
chatter is not a merit discussion the label should defer to).

Source: user-scope feedback memory
`feedback-ready-for-maintainer-review-label`. See the
[`strip-ready-on-downgrade`](#hard-rules-cross-cutting-the-table)
hard rule for how this precondition gates the label-strip,
draft-conversion, and close behavior on a regressed PR.

### `unresolved_threads_only_likely_addressed`

All of:
Expand Down
44 changes: 44 additions & 0 deletions .claude/skills/pr-management-triage/rationale.md
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,50 @@ draft is an overreach.

---

## Merit-discussion exception to `strip-ready-on-downgrade`

The `strip-ready-on-downgrade` hard rule
(see [`classify-and-act.md`](classify-and-act.md#hard-rules-cross-cutting-the-table))
otherwise strips `ready for maintainer review` whenever a
regressed PR matches a `deterministic_flag` row with action
`draft` / `comment` / `close`. The
[`merit_discussion_thread_present`](classify-and-act.md#merit_discussion_thread_present)
exception suspends that strip — and additionally suspends the
draft conversion and the close — when an unresolved
maintainer-opened review thread is present on the PR.

Why this matters:

- The `ready for maintainer review` label exists to attract
senior eyes. An unresolved maintainer review thread is the
moment senior eyes are most valuable. Stripping the label
or pushing the PR back to draft mid-discussion makes the
PR disappear from the maintainer queue exactly when it
shouldn't.
- CI red / lint failures / merge conflicts and a live design
debate are orthogonal axes. A maintainer can usefully weigh
in on the design discussion even when CI is red — the
mechanical blockers belong to the author, the design
question belongs to the maintainers.
- The exception's precondition is deliberately broad — any
maintainer-opened unresolved review thread counts,
regardless of body length or when it was opened relative
to the label-add. A narrower "substantive content"
heuristic would mis-classify short-but-substantive prompts
("is this really the right layer for this change?") as
trivial and strip the label anyway. Erring toward keeping
the label is the safer asymmetry: a stale-but-kept label
costs a maintainer a glance; a stripped label mid-discussion
costs the discussion its audience.
- Contributor-author unresolved threads do NOT satisfy the
precondition. The label defers to maintainer judgment, not
contributor-to-contributor side chatter.

Originating user-scope feedback memory:
`feedback-ready-for-maintainer-review-label`.

---

## Group-level overrides

The interaction loop lets the maintainer override the suggested
Expand Down
Loading