From 8d80224a1e68f9edb8de06de1fe37904a3000477 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sat, 29 Aug 2026 01:25:58 +0300 Subject: [PATCH] docs(pr-review): decide PUSH-BACK vs MERGE-THEN-FIX by who owns the residue, not its size --- .agents/skills/pr-review/SKILL.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.agents/skills/pr-review/SKILL.md b/.agents/skills/pr-review/SKILL.md index 6841e709..94fb7ee0 100644 --- a/.agents/skills/pr-review/SKILL.md +++ b/.agents/skills/pr-review/SKILL.md @@ -9,7 +9,7 @@ The maintainer directs the project at the product level; they plan and understan ## Verdicts -Choose exactly one. When torn between two, the deciding question is always: **what does accepting this cost the maintainer over the next year?** +Choose exactly one. When torn between two, the deciding question is always: **what does accepting this cost the maintainer over the next year?** Between PUSH-BACK and MERGE-THEN-FIX specifically, size of the residue never decides — its owner does: *does the fix close the symptom?* then *whose knowledge finishes it?* then *what does a round-trip cost?* **Product fit is the maintainer's call, not yours.** For a PR that adds or changes user-facing functionality, judge the code but never silently decide the feature is wanted: state the product question explicitly (who asks for this, what it costs the product) and make the verdict conditional on the maintainer's answer when desirability is genuinely open — "PUSH-BACK if you want this feature; DECLINE if you don't". A bug fix has no product question; a new surface always does. @@ -23,11 +23,11 @@ Choose exactly one. When torn between two, the deciding question is always: **wh **Salvage the ache.** A decline closes the PR, never the problem. Decide first whether a real ache exists — a whim or a false premise has none, and proposing to track those is noise. When the ache is real: search the tracker for an existing issue (`gh issue list --search`), reference it if found; if untracked, the ready action additionally includes a drafted issue (title + a few lines: the ache, the evidence from the PR, the welcome fix shape) for the maintainer to approve. -2. **PUSH-BACK** — right direction, roughly 80% good, but the missing 20% is the contributor's work, not the maintainer's: incomplete runtime coverage, an unhandled failure path, a broken workflow hunk, discipline gaps. The PR stays open. +2. **PUSH-BACK** — right direction, but what remains is the contributor's to do. Two grounds, checked in order: the fix does not close the reported symptom (then it is always PUSH-BACK — merging a non-fix closes the issue on paper and leaves the bug live, whatever the size of the gap); or the residue needs knowledge only the author has — why they guarded that branch, what their test was meant to prove, what their own scenario requires. The PR stays open. Ready action: a review comment with a **finite, checkable list** of what to change — each item states what is wrong, why it matters, and what done looks like. The list must be completable: a contributor who does every item has earned a merge, so include nothing you would not merge over. -3. **MERGE-THEN-FIX** — correct at the 90–95% level; the residue is small enough that commenting would cost more than fixing. Merge it and immediately do the follow-ups in-house. +3. **MERGE-THEN-FIX** — the fix closes the symptom, and the residue needs knowledge the contributor does not have: repo conventions, a second path with the same defect, runtime parity, product shape already decided. That residue is ours regardless of size — sending it back buys a round-trip of days and a real chance the PR dies, against minutes of in-house work. Two hard conditions: the symptom is closed (else PUSH-BACK), and the follow-up list contains no product decision the maintainer has not already made — a "decide whether X" item is either a question in the report or a PUSH-BACK. The follow-ups are executed the same day as the merge; a list that waits becomes debt nobody remembers. Ready action: merge recommendation plus a **follow-up list precise enough for an agent to execute without re-reviewing the PR** — exact files, exact defects, exact intended behavior. Every known defect goes on the list; merging is never a reason to drop one (the repo rule: every merged contribution is fully de-slopified).