diff options
Diffstat (limited to 'publish')
| -rw-r--r-- | publish/SKILL.md | 295 | ||||
| -rw-r--r-- | publish/references/pull-requests.md | 105 |
2 files changed, 277 insertions, 123 deletions
diff --git a/publish/SKILL.md b/publish/SKILL.md index 48924b7..d2b906c 100644 --- a/publish/SKILL.md +++ b/publish/SKILL.md @@ -222,28 +222,142 @@ Step 0. Upstream can advance during a long session, especially across machines or with teammates pushing in parallel. Run Step 0 every time the publish flow starts. -### Step 1: local code review (mandatory) - -Run the `review-code` skill against the change: - -- Before a commit: `/review-code --staged` -- Before a PR: `/review-code` (branch diff against `main` merge-base) -- Before commenting on someone else's PR: `/review-code <PR#>` - -Surface **all** findings to the user: Critical, Important, and Minor. - -**Default block:** any Critical or Important finding stops the flow. Fix the -issues and re-run `/review-code` until the diff is clean. Minor findings are -shown but do not block. - -**Override:** the user can bypass the block with an explicit "proceed anyway" -(or equivalent wording). Without the explicit override, do not proceed to -Step 2. - -The `review-code` skill already has a Phase 0 eligibility gate that handles -trivial and ineligible diffs (whitespace-only, revert with obvious -justification, already-reviewed SHA). Trust that gate; there is no "trivial -enough to skip review" exemption on top of it. +### Step 1: adversarial review by an isolated reviewer (mandatory) + +The review runs in a **subagent**, never inline, and it runs on **every** +commit. The author does not review their own work. + +**Why isolation, not just review.** A self-review checks the diff against the +author's own model of what the diff should do. It cannot check the model. The +errors that survive a self-review are the ones that were never visible in the +diff — a scope inherited from whoever reported the problem, a blast radius +estimated instead of measured, a fix that is correct for the case the author +had in mind and wrong for the one they never considered. Only a reviewer that +does not hold the author's model catches those, so the isolation is the point +and the adversarial stance is the method. + +**Dispatch contract.** Spawn the reviewer via the Agent tool and give it these +three things, the third whenever one exists: + +1. **The diff** — `git diff --cached` for a commit, the branch diff for a PR. +2. **The claim** — one line from the author stating what the change does. Write + it before dispatching. This is the thing under test: the reviewer's job is + to check the diff against the claim. +3. **The requirement source, when one exists** — the ticket, plan, ADR, or task + body the work was done against. Pass it verbatim. + +Withhold everything else: the conversation, the exploration, the dead ends, and +above all the author's reasoning for why the change is right. Those are what +transmit the author's model, which is what the reviewer exists to not have. A +reviewer given the rationale reviews the rationale. + +**Why the requirement source is not withheld.** A ticket or plan is not the +author's model of the change — it is the independent record of what was asked, +written before the work and usually by someone else. It is the only artifact +that can contradict the author's one-line claim. Withhold it and the claim +becomes self-certifying: the reviewer checks the diff against a sentence the +author wrote, which cannot surface scope creep or a missing requirement. That +also strands `review-code`'s Intent-vs-Delivery criterion, which is skipped +outright when no intent context is supplied and is the one criterion aimed at +the inherited-scope error this whole gate exists to catch. + +Invoke the review with `/review-code --staged` (commit), `/review-code` (branch +diff against the `main` merge-base), or `/review-code <PR#>` (someone else's +PR), and tell it to run its adversarial pass. + +**Adversarial, with substantiation.** The reviewer is prompted to *refute* the +change rather than to bless it. But an agent told to attack will manufacture +findings to satisfy the instruction, so the stance carries a floor: a finding +that cannot be substantiated against the diff is not a finding and must be +dropped. `review-code`'s confidence filter and false-positive filter are what +enforce that floor — adversarial raises the appetite for looking, never the +tolerance for a weak claim. + +**Scope is every commit; the reviewer decides triviality, not the author.** +There is no "trivial enough to skip" exemption. A floor written in terms of +"small" or "mechanical" puts the judgment back with the author, whose judgment +is the thing being checked. Dispatch always, and let `review-code`'s own Phase 0 +eligibility gate return fast on a whitespace-only diff, an obvious revert, or an +already-reviewed SHA. A cheap spawn on a trivial commit is the price of the +author never getting to rule on their own diff. + +**Verdict, and the re-review loop.** The reviewer returns one of four outcomes, +and all four are defined exits: + +- **Approve** — the gate is satisfied. Proceed to Step 2. +- **Skipped** — `review-code`'s Phase 0 found the diff ineligible (whitespace + only, an obvious revert, an already-reviewed SHA). This **satisfies the gate** + and the flow proceeds. A skip is a reviewer's ruling, which is the point; what + is forbidden is the *author* ruling their own diff too trivial to look at. +- **Request Changes** — blocking findings stand. Enter the loop below. +- **Needs Discussion** — the reviewer has a disagreement it cannot settle from + the diff: an architectural objection, a question about whether the change + should exist at all. This **stops immediately and goes to the user**; it does + not enter the loop. Routing it to the loop would answer "should we do this?" + with "fix these findings," which is the wrong question and burns rounds on a + disagreement no amount of editing resolves. Unattended callers park it exactly + as they park a bound hit. + +Approval is the reviewer's to give; the author never declares their own change +clean. + +That set is closed. `review-code` emits Approve, Request Changes, or Needs +Discussion, and its Phase 0 emits Skipped; every one has a defined exit above. A +verdict outside those four means the reviewer went off-contract — surface it +rather than interpreting it. + +**The loop turns on blocking findings, not on the verdict token.** It ends when +no Critical or Important finding stands. A Minor-only result is not grounds for +another round: fix it or don't, but do not spend a round on it, and do not +escalate to the user over one. A reviewer holding only Minor findings should +return Approve and say what it left. + +On Request Changes: + +1. Surface **all** findings — Critical, Important, Minor. Critical and Important + block; Minor is shown and does not block. +2. Fix the blocking findings. +3. **Re-review, and keep re-reviewing until the reviewer approves.** A fix is a + new change and gets the same scrutiny as the original. Fixing under review + pressure is exactly when a regression gets introduced, so an unreviewed fix + is the hole this loop closes. + +**Continue the same reviewer, don't spawn a fresh one.** Send the updated diff +back to the existing reviewer (`SendMessage` with its agent ID). It holds its own +findings, so it can confirm each one is actually addressed. A fresh reviewer each +round cannot tell "addressed" from "never existed", re-litigates settled points, +and drifts to a new set of findings every round, which never converges. + +The continued reviewer must **re-verify each finding against the new diff**, not +against the author's description of the fix. "I fixed it" is a claim, and taking +it at face value is how a review round becomes a rubber stamp. + +**Bounds, so the loop terminates.** Two conditions end it early and hand the +decision to the user: + +- **Three rounds without approval** (the initial review plus two re-reviews). + This matches the two-fix-attempts limit in `subagents.md`: past that, the + problem is usually the approach rather than the diff. +- **A finding recurs after being reported fixed.** That is oscillation — the + fix for one finding reintroducing another — and another round will not + resolve it. Stop on the first recurrence rather than spending the remaining + rounds. + +In both cases, stop and surface: the standing findings, what was tried, and the +decision needed. + +**Override.** The user can bypass the block with an explicit "proceed anyway" (or +equivalent). The user is also the adjudicator when the author believes a finding +is wrong: say so with the reasoning and let the user rule. Do not resolve a +disagreement with the reviewer by overruling it silently. Without an explicit +override, do not proceed to Step 2. + +**When the Agent tool is unavailable.** Per `subagents.md`, don't block: run the +review in the main thread, but hold it to the same contract — review against the +stated claim, refute rather than bless, substantiate every finding, loop on fixes +until clean, same bounds. State plainly that the review was not isolated, because +a self-review under an adversarial prompt is weaker evidence and the user should +know which one they got. ### Step 2: draft, review, publish @@ -251,16 +365,37 @@ enough to skip review" exemption on top of it. *Voice patterns are always personal for publish artifacts.* Commit messages, PR titles + bodies, and PR review comments all go out under the user's name, so they always run through `/voice personal` (the full pattern walk — general + Craig's-voice + the artifact-mechanics patterns: first-person rewrite, public-artifact scope flag, praise/correction asymmetry, finding stems), regardless of whether `.ai/` is tracked. These three are personal-voice artifacts by definition — the skill's personal mode exists for exactly them. Pattern #39 (public-artifact scope flag) matters *most* on team-visible artifacts, so it must never be skipped on a PR comment or PR body. There is no "general-voice mode" for publish artifacts. -*The approval gate is the only thing `.ai/`-tracking decides.* Before drafting, run this command: +*The approval gate turns on whether anyone else reads the history.* The gate +exists so Craig sees the exact words that go out under his name. Skipping it +trades that for velocity, and that trade is only worth making where the repo is +genuinely shared with other people. + +The old signal for this was whether `.ai/` is tracked, used as a proxy for +"team repo." It was the wrong proxy and it failed in the direction that +matters: rulesets, home, and work all track `.ai/` — rulesets as a committed +mirror, the others because the project history *is* the project — while all +three are Craig's private single-user repos. The rule as written skipped the +gate on his three most-used projects. + +Check the remote host instead, which is what actually distinguishes them: ``` -git ls-files :/.ai/ 2>/dev/null | head -1 +git remote -v 2>/dev/null | grep -v 'cjennings\.net' | head -1 ``` -The `:/` pathspec anchors the search to the repo root, so the command works from any subdirectory. Without it, running from a subdir returns no matches even when `.ai/` is tracked at the repo root, which silently misclassifies the project. +- **No output** — every remote is on `cjennings.net`, so the repo is Craig's + own and nobody else reads the log. **Gate applies**: write to `/tmp`, run + `/voice personal`, print inline, ask approve / request changes / open in + editor, and publish only on explicit approval. +- **Any output** — a remote on a host someone else can read (GitHub, a GHE + instance, a team server). **Gate skipped for velocity**: write to `/tmp`, run + `/voice personal`, print inline, publish immediately. +- **No remote at all** — a local-only repo. Gate applies; there is no + velocity argument without a reader. -- **No output** — `.ai/` is gitignored, missing, or empty (the user's personal repos). **Gate applies**: write to `/tmp`, run `/voice personal`, print inline, ask approve / request changes / open in editor, then publish only on explicit approval. -- **Any output** — one or more files under `.ai/` are tracked (a shared / team repo). **Gate skipped for velocity**: write to `/tmp`, run `/voice personal`, print inline, publish immediately. +As of 2026-07-27 every project resolves to gate-applies, because every remote +is `cjennings.net`. That is the correct answer, not a bug: it matches how the +flow has actually been run. Either way the draft runs through `/voice personal` first. The subflows below describe the full gated path. For the gate-skipped path, run the same `/voice personal` pass, then collapse the "Ask: approve, request changes, or open in editor" step — the draft prints inline and the publish step runs immediately afterward. @@ -274,108 +409,22 @@ Either way the draft runs through `/voice personal` first. The subflows below de - **Request changes** → make them, re-run `/voice personal`, re-print inline, ask again. - **Open in editor** → only if the user asks. `emacsclient -n /tmp/commit-<short-slug>.md`. After the editor closes, re-read the file, re-print the contents inline, and ask again. -**For PR descriptions:** - -1. Write the title as line 1 and the body below it to `/tmp/pr-<slug>.md`. **Title format:** the conventional-commit subject (`refactor: remove dead if-count-is-not-None check in admin`). If the project defines a publishing overlay with a ticket system, follow it for the ticket suffix in the title and the cross-link line in the body (see the overlay). -2. Run `/voice personal` on the file. The PR title stays imperative per Conventional Commits — `/voice personal` rewrites the body, not the title. -3. Print the final draft inline in the terminal. Title on line 1, blank line, then body — exactly as it'll be posted. State that the skill ran. Surface any pattern #39 (public-artifact scope) warnings. -4. Ask: approve, request changes, or open in editor. Wait for an explicit answer. Do not open the file in `emacsclient` (or any editor) by default. - - **Approve** → continue to step 5. - - **Request changes** → make them, re-run `/voice personal`, re-print inline, ask again. - - **Open in editor** → only if the user asks. `emacsclient -n /tmp/pr-<ticket-or-slug>.md`. After the editor closes, re-read the file, re-print inline, ask again. -5. Split the file on the first blank line and pass the title and body to `gh pr create --title "..." --body "$(tail -n +3 <file>)"` (or a heredoc) so formatting is preserved. Add `--reviewer <user[,user...]>` in the same call when you already know who should review. -6. Request reviewers on the new PR if you didn't pass `--reviewer` at create time. Use `gh pr edit <N> --add-reviewer <user>`. If the repo has a `CODEOWNERS` file, GitHub auto-suggests based on touched paths. Still issue the explicit request so the reviewer gets notified. Pick reviewers per the team's convention for the area touched (often documented in the per-repo `CLAUDE.md`). For follow-up PRs, consider tagging the parent PR's author if their context would help. PRs without a human reviewer request stall — "checks passed" is not a substitute for review. -7. **Project publishing overlay (if present).** If the project defines a publishing overlay — a `publishing-<team>.md` rule loaded from its `.claude/rules/` — run its post-create steps now: ticket cross-linking, ticket-state moves, and any other tracker integration it specifies. A project with no overlay skips this; the PR is already open and reviewers are requested, which is the complete universal flow. - -**For PR review comments and replies (review verdicts, threaded discussion, follow-up notes on someone else's PR or your own):** - -Pick the shape first. Most reviews are Shape 1. - -- **Shape 1 — Single review** (verdict + summary body + 0+ inline pins). The default for any post that carries a verdict (`APPROVE`, `REQUEST_CHANGES`, `COMMENT`), even when the verdict has no line-specific findings. One `gh api` call posts the summary, every inline pin, and the verdict together. review notification fires once for `APPROVE` or `REQUEST_CHANGES`. -- **Shape 2 — Issue-thread comment** (no verdict). General PR discussion, not a review. No inline pins. No review notification. -- **Shape 3 — Reply on an existing inline thread**. Responding to a specific prior reviewer comment. Threads under that comment. No review notification. - -**Inline threshold for Shape 1.** Any finding that names a `path:line` belongs as an inline comment pinned to that line. Cross-cutting observations (verdict rationale, "third PR with the same pattern", overall test-coverage gaps that don't pin to one place) stay in the summary body. There's no "fold one inline into the summary" exception — a single line-specific finding still goes inline. - -**Shape 1: Single review (bundled summary + inline)** - -1. Identify findings, split into **inline-eligible** (each names a specific `path:line`) and **summary-only** (cross-cutting). Decide the verdict. - -2. Write one concatenated draft to `/tmp/pr-<N>-review.md` with explicit separators: - - ``` - === SUMMARY === - <verdict summary body> - - === INLINE path=frontend/src/foo.tsx line=440 === - <inline body 1> - - === INLINE path=frontend/src/bar.tsx line=137 === - <inline body 2> - ``` - - The separator format is exactly `=== SUMMARY ===` and `=== INLINE path=<path> line=<n> ===`. The summary block is mandatory even for verdict-only reviews. Inline blocks are zero-or-more. - -3. Run `/voice personal` on the file once. The skill walks its full pattern list across every block at the same time. The separators stay intact because they aren't prose. - -4. Print the final draft inline in the terminal. Every block — the summary body AND the full prose of every inline comment — exactly as it'll be posted, with its separator header. Print the inline in full; never describe it in place of printing it ("I'd pair it with one inline on…"). Craig approves the exact words that post under his name, so the exact words must be on screen. State that the skill ran (e.g. "/voice personal — full pattern walk across summary + 3 inline"). Surface any pattern #39 warnings. - -5. Ask: approve, request changes, or open in editor. Wait for an explicit answer. Do not open the file in `emacsclient` (or any editor) by default. - - **Approve** → continue to step 6. - - **Request changes** → make them, re-run `/voice personal` on the whole file, re-print inline, ask again. - - **Open in editor** → only if the user asks. `emacsclient -n /tmp/pr-<N>-review.md`. After the editor closes, re-read, re-print inline, ask again. - -6. Split the file on the separator lines and post in **a single** `gh api` call: - - ``` - gh api repos/<owner>/<repo>/pulls/<N>/reviews \ - --hostname <ghe-host-or-omit> \ - -F event=REQUEST_CHANGES \ - -F body="<summary block>" \ - -F "comments[][path]=<path1>" \ - -F "comments[][line]=<line1>" \ - -F "comments[][body]=<inline 1>" \ - -F "comments[][path]=<path2>" \ - -F "comments[][line]=<line2>" \ - -F "comments[][body]=<inline 2>" - ``` - - `event` is one of `APPROVE`, `REQUEST_CHANGES`, `COMMENT`. The `comments[]` array can be empty for verdicts with zero line-specific findings — the call still uses the same endpoint. Pass `--hostname` for non-`github.com` hosts (a project's publishing overlay names its host when it's a GitHub Enterprise instance). - -7. Verify the review landed. `gh api repos/<owner>/<repo>/pulls/<N>/reviews --hostname ...` returns the latest review with bundled inlines. Confirm `state` matches the verdict and the inline count matches what was posted. - -8. **Project review-notification overlay (if present).** If the project defines a publishing overlay with a review-notification step (e.g. a Slack ping to the PR author), run it now — but only for `APPROVE` and `REQUEST_CHANGES` verdicts. The overlay owns the channel, the message format, the author-mention lookup, and the threading. A project with no overlay skips notification entirely. `COMMENT` verdicts and Shapes 2-3 below never notify, overlay or not. - -**Shape 2: Issue-thread comment (no verdict)** - -Use when the post is informal discussion that shouldn't appear as a review verdict (e.g. "I'd like to discuss the X approach before you continue"). - -1. Write the proposed comment to `/tmp/pr-<N>-comment.md`. -2. Run `/voice personal`. -3. Print inline, ask approve/changes/edit, gate as in Shape 1 step 5. -4. Post: `gh pr comment <N> --body-file /tmp/pr-<N>-comment.md`. -5. Verify: `gh api repos/<owner>/<repo>/issues/<N>/comments`. -6. No review notification. - -**Shape 3: Reply on an existing inline thread** - -Use when responding to a specific prior reviewer comment. - -1. Find the parent comment ID: `gh api repos/<owner>/<repo>/pulls/<N>/comments`. -2. Write the reply to `/tmp/pr-<N>-reply-<comment-id>.md`. -3. Run `/voice personal`. -4. Print inline, ask approve/changes/edit, gate as in Shape 1 step 5. -5. Post: `gh api repos/<owner>/<repo>/pulls/<N>/comments -F in_reply_to=<comment-id> -F body="$(cat /tmp/pr-<N>-reply-<comment-id>.md)"`. -6. Verify in the same `comments` list. -7. No review notification. +**For PR descriptions, and for PR review comments and replies:** read +`references/pull-requests.md` in this skill directory. It carries the PR +description shape, the three review shapes (bundled review with inline pins, +issue-thread comment, threaded reply), the `gh api` calls, and the +publishing-overlay hooks. A plain commit needs none of it, so it is kept out of +this file — load it when the artifact is a PR. **Approve does not authorize a merge.** Reviewing a PR never authorizes merging it. Anything in `## Merge Strategy` below applies only to merges *you* are about to perform on your own branches — and even then, the merge needs its own explicit user confirmation per the rules there. A project's publishing overlay may add a team merge practice (e.g. approve-then-author-merges, where the review notification hands the merge decision to the PR author); that's an overlay concern, not a global one. **Exception:** trivial one-liners the user dictated verbatim in the conversation (e.g. "commit this as `chore: bump version`", "reply just 'thanks for the review'") can skip the draft-file step in Step 2. -`/review-code` in Step 1 still runs when it applies; Phase 0 of that skill -handles trivial diffs, and acknowledgment-only replies don't need it at all. +Step 1's review still runs on every commit — this carve-out is about the +draft-file step, not the review. Phase 0 is what rules a trivial diff out, and +its Skipped result satisfies the gate. An acknowledgment-only PR reply commits +nothing, so there is no diff to review. **Single-skill gate.** Each of the three subflows above runs `/voice personal` before printing the draft — the full pattern walk covering AI-writing signs, universal good-writing rules, Craig's voice patterns, and the artifact-mechanics patterns (first-person rewrite, public-artifact scope flag, praise/correction asymmetry, finding stems). Publish artifacts (commits, PR titles + bodies, PR review comments) always use personal mode; the `.ai/`-tracking check at the top of Step 2 decides only whether the approval gate fires, not which patterns run. Running the skill is mandatory; the printed draft must have been through it. When the user asks mid-flow for "the voice pass" on an in-progress draft, that means re-run the full pattern walk — not a subset. Always state that the skill ran when announcing the printed draft (e.g. "/voice personal — full pattern walk"). Skipping the pass without flagging it is a defect. The terse/omit-needless-words cut (pattern #38) is the *last* thing the skill does before the draft is printed: read each sentence and cut it in half, keeping only what changes meaning. The draft the user first sees must already be terse — if they have to ask for an Orwell pass after seeing it, the pass was skipped. diff --git a/publish/references/pull-requests.md b/publish/references/pull-requests.md new file mode 100644 index 0000000..9ac4ded --- /dev/null +++ b/publish/references/pull-requests.md @@ -0,0 +1,105 @@ +# Pull requests and PR review comments + +Loaded from the `publish` skill when the artifact is a PR description or a PR +review comment. A plain commit never needs any of this, which is why it lives +here rather than in SKILL.md. + +Steps 0 and 1 of the publish flow (pre-flight reconcile, local code review) and +the `/voice personal` pass still apply — see SKILL.md. This file carries only +what is specific to PRs. + +**For PR descriptions:** + +1. Write the title as line 1 and the body below it to `/tmp/pr-<slug>.md`. **Title format:** the conventional-commit subject (`refactor: remove dead if-count-is-not-None check in admin`). If the project defines a publishing overlay with a ticket system, follow it for the ticket suffix in the title and the cross-link line in the body (see the overlay). +2. Run `/voice personal` on the file. The PR title stays imperative per Conventional Commits — `/voice personal` rewrites the body, not the title. +3. Print the final draft inline in the terminal. Title on line 1, blank line, then body — exactly as it'll be posted. State that the skill ran. Surface any pattern #39 (public-artifact scope) warnings. +4. Ask: approve, request changes, or open in editor. Wait for an explicit answer. Do not open the file in `emacsclient` (or any editor) by default. + - **Approve** → continue to step 5. + - **Request changes** → make them, re-run `/voice personal`, re-print inline, ask again. + - **Open in editor** → only if the user asks. `emacsclient -n /tmp/pr-<ticket-or-slug>.md`. After the editor closes, re-read the file, re-print inline, ask again. +5. Split the file on the first blank line and pass the title and body to `gh pr create --title "..." --body "$(tail -n +3 <file>)"` (or a heredoc) so formatting is preserved. Add `--reviewer <user[,user...]>` in the same call when you already know who should review. +6. Request reviewers on the new PR if you didn't pass `--reviewer` at create time. Use `gh pr edit <N> --add-reviewer <user>`. If the repo has a `CODEOWNERS` file, GitHub auto-suggests based on touched paths. Still issue the explicit request so the reviewer gets notified. Pick reviewers per the team's convention for the area touched (often documented in the per-repo `CLAUDE.md`). For follow-up PRs, consider tagging the parent PR's author if their context would help. PRs without a human reviewer request stall — "checks passed" is not a substitute for review. +7. **Project publishing overlay (if present).** If the project defines a publishing overlay — a `publishing-<team>.md` rule loaded from its `.claude/rules/` — run its post-create steps now: ticket cross-linking, ticket-state moves, and any other tracker integration it specifies. A project with no overlay skips this; the PR is already open and reviewers are requested, which is the complete universal flow. + +**For PR review comments and replies (review verdicts, threaded discussion, follow-up notes on someone else's PR or your own):** + +Pick the shape first. Most reviews are Shape 1. + +- **Shape 1 — Single review** (verdict + summary body + 0+ inline pins). The default for any post that carries a verdict (`APPROVE`, `REQUEST_CHANGES`, `COMMENT`), even when the verdict has no line-specific findings. One `gh api` call posts the summary, every inline pin, and the verdict together. review notification fires once for `APPROVE` or `REQUEST_CHANGES`. +- **Shape 2 — Issue-thread comment** (no verdict). General PR discussion, not a review. No inline pins. No review notification. +- **Shape 3 — Reply on an existing inline thread**. Responding to a specific prior reviewer comment. Threads under that comment. No review notification. + +**Inline threshold for Shape 1.** Any finding that names a `path:line` belongs as an inline comment pinned to that line. Cross-cutting observations (verdict rationale, "third PR with the same pattern", overall test-coverage gaps that don't pin to one place) stay in the summary body. There's no "fold one inline into the summary" exception — a single line-specific finding still goes inline. + +**Shape 1: Single review (bundled summary + inline)** + +1. Identify findings, split into **inline-eligible** (each names a specific `path:line`) and **summary-only** (cross-cutting). Decide the verdict. + +2. Write one concatenated draft to `/tmp/pr-<N>-review.md` with explicit separators: + + ``` + === SUMMARY === + <verdict summary body> + + === INLINE path=frontend/src/foo.tsx line=440 === + <inline body 1> + + === INLINE path=frontend/src/bar.tsx line=137 === + <inline body 2> + ``` + + The separator format is exactly `=== SUMMARY ===` and `=== INLINE path=<path> line=<n> ===`. The summary block is mandatory even for verdict-only reviews. Inline blocks are zero-or-more. + +3. Run `/voice personal` on the file once. The skill walks its full pattern list across every block at the same time. The separators stay intact because they aren't prose. + +4. Print the final draft inline in the terminal. Every block — the summary body AND the full prose of every inline comment — exactly as it'll be posted, with its separator header. Print the inline in full; never describe it in place of printing it ("I'd pair it with one inline on…"). Craig approves the exact words that post under his name, so the exact words must be on screen. State that the skill ran (e.g. "/voice personal — full pattern walk across summary + 3 inline"). Surface any pattern #39 warnings. + +5. Ask: approve, request changes, or open in editor. Wait for an explicit answer. Do not open the file in `emacsclient` (or any editor) by default. + - **Approve** → continue to step 6. + - **Request changes** → make them, re-run `/voice personal` on the whole file, re-print inline, ask again. + - **Open in editor** → only if the user asks. `emacsclient -n /tmp/pr-<N>-review.md`. After the editor closes, re-read, re-print inline, ask again. + +6. Split the file on the separator lines and post in **a single** `gh api` call: + + ``` + gh api repos/<owner>/<repo>/pulls/<N>/reviews \ + --hostname <ghe-host-or-omit> \ + -F event=REQUEST_CHANGES \ + -F body="<summary block>" \ + -F "comments[][path]=<path1>" \ + -F "comments[][line]=<line1>" \ + -F "comments[][body]=<inline 1>" \ + -F "comments[][path]=<path2>" \ + -F "comments[][line]=<line2>" \ + -F "comments[][body]=<inline 2>" + ``` + + `event` is one of `APPROVE`, `REQUEST_CHANGES`, `COMMENT`. The `comments[]` array can be empty for verdicts with zero line-specific findings — the call still uses the same endpoint. Pass `--hostname` for non-`github.com` hosts (a project's publishing overlay names its host when it's a GitHub Enterprise instance). + +7. Verify the review landed. `gh api repos/<owner>/<repo>/pulls/<N>/reviews --hostname ...` returns the latest review with bundled inlines. Confirm `state` matches the verdict and the inline count matches what was posted. + +8. **Project review-notification overlay (if present).** If the project defines a publishing overlay with a review-notification step (e.g. a Slack ping to the PR author), run it now — but only for `APPROVE` and `REQUEST_CHANGES` verdicts. The overlay owns the channel, the message format, the author-mention lookup, and the threading. A project with no overlay skips notification entirely. `COMMENT` verdicts and Shapes 2-3 below never notify, overlay or not. + +**Shape 2: Issue-thread comment (no verdict)** + +Use when the post is informal discussion that shouldn't appear as a review verdict (e.g. "I'd like to discuss the X approach before you continue"). + +1. Write the proposed comment to `/tmp/pr-<N>-comment.md`. +2. Run `/voice personal`. +3. Print inline, ask approve/changes/edit, gate as in Shape 1 step 5. +4. Post: `gh pr comment <N> --body-file /tmp/pr-<N>-comment.md`. +5. Verify: `gh api repos/<owner>/<repo>/issues/<N>/comments`. +6. No review notification. + +**Shape 3: Reply on an existing inline thread** + +Use when responding to a specific prior reviewer comment. + +1. Find the parent comment ID: `gh api repos/<owner>/<repo>/pulls/<N>/comments`. +2. Write the reply to `/tmp/pr-<N>-reply-<comment-id>.md`. +3. Run `/voice personal`. +4. Print inline, ask approve/changes/edit, gate as in Shape 1 step 5. +5. Post: `gh api repos/<owner>/<repo>/pulls/<N>/comments -F in_reply_to=<comment-id> -F body="$(cat /tmp/pr-<N>-reply-<comment-id>.md)"`. +6. Verify in the same `comments` list. +7. No review notification. + |