aboutsummaryrefslogtreecommitdiff
path: root/publish/references
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-07-27 14:13:32 -0500
committerCraig Jennings <c@cjennings.net>2026-07-27 14:13:32 -0500
commit2f45b6e0010ce375e2c52c91a6537c6d6e8bc0a5 (patch)
treec89e06112df046d72905531123bfbfc1bce339a1 /publish/references
parent931f364864441676eb250cecd074c4876012a1cd (diff)
downloadrulesets-2f45b6e0010ce375e2c52c91a6537c6d6e8bc0a5.tar.gz
rulesets-2f45b6e0010ce375e2c52c91a6537c6d6e8bc0a5.zip
refactor(rules): split testing.md, fix the approval gate, require first person
Three changes to the same layer. I split testing.md the way I split commits.md, by what has to be resident rather than by size. What stays is the standing directive: TDD is the default, write the failing test first, and every unit needs Normal, Boundary, and Error cases. That has to fire before any code is written, which is exactly when no skill has been summoned, so it can't ride a trigger. Everything else moved to the testing-standards skill: characterization recipes, the per-category detail, property-based and mutation testing, the pyramid, integration rules, naming, the test-quality and mocking rules, coverage targets, the spike exception, and the anti-patterns. 2,824 words down to 347. I fixed the approval gate in the publish flow. It decided whether to ask for approval by checking whether .ai/ is tracked, using that as a proxy for "team repo." The proxy was wrong in the direction that matters: rulesets, home, and work all track .ai/ while all three are private single-user repos, so the rule skipped the gate on the three projects I use most. It now checks whether any remote is on a host other than cjennings.net, which is the thing that actually decides whether someone else reads the log. Every current project resolves to gate-applies, which matches how the flow has actually been run. I also added a first-person directive to the always-loaded core. One already existed for commit bodies and PR prose, but it moved into the publish skill with everything else, and it never covered code comments at all. Now everything I author in or about the repo is first person, with one carve-out: a comment describing what the code does stays third person, because there the code is the actor and not me. Separately I split the publish skill internally. PR descriptions and the three review shapes moved to references/pull-requests.md, since a plain commit never needs them. Always-loaded rules are now about 28,900 tokens, down from 57,800 this morning. One risk on the record. testing.md's margin is thinner than commits.md's was. If testing-standards fails to trigger while I'm writing tests I lose the mocking-boundary rules, which is a quality regression rather than a permanent one, but it is a real bet where commits.md's was not. I also moved the TDD rationalization table rather than cutting it. The posts argue that kind of over-argument is counterproductive now, but removing your defense against me skipping TDD is your call, not mine.
Diffstat (limited to 'publish/references')
-rw-r--r--publish/references/pull-requests.md105
1 files changed, 105 insertions, 0 deletions
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.
+