From 4686dd5b908f3f66d59e00265f9f356292eea011 Mon Sep 17 00:00:00 2001 From: Ankit Goswami Date: Mon, 15 Jun 2026 10:22:26 -0700 Subject: [PATCH 01/13] docs(agents): add PR description standard + /pr-describe command Establish a default PR-description format so reviewers can judge the architecture and technical decisions without reading low-level code: high-level mechanism narrative, GitHub-rendered mermaid diagrams, a code-area map, and explicit technical trade-offs. - AGENTS.md: new "PR descriptions" section (cross-harness; followed by any agent that reads AGENTS.md), linked from the Claude Code commands list - .claude/commands/pr-describe.md: /pr-describe slash command that operationalizes the standard against the current branch's diff Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/commands/pr-describe.md | 55 +++++++++++++++++++++++++++++++++ AGENTS.md | 33 ++++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 .claude/commands/pr-describe.md diff --git a/.claude/commands/pr-describe.md b/.claude/commands/pr-describe.md new file mode 100644 index 000000000..a3b414bcb --- /dev/null +++ b/.claude/commands/pr-describe.md @@ -0,0 +1,55 @@ +--- +description: Write or update a PR description that lets reviewers judge the architecture and technical decisions without reading low-level code — high-level mechanism, mermaid diagrams, and a code-area map. +argument-hint: (no arguments; run on the current branch, or pass a PR number/URL to describe an existing PR) +--- + +Write (or update) the description for this PR so a reviewer can understand what +it does and judge the architecture and technical decisions **without reading the +low-level code**. Inspect the actual diff, commits, and changed files first; +describe what the code does, not the decisions made getting there. + +## Steps + +1. Determine the target. If `$ARGUMENTS` is a PR number or URL, use that PR. + Otherwise describe the current branch's PR (`gh pr view --json + number,url,baseRefName,headRefName`); if none exists yet, draft the body for + the PR the user is about to open. +2. Read the change: `git diff ...HEAD --stat`, `gh pr diff` (or + `git diff ...HEAD`), and `git log ..HEAD --oneline`. Identify + which subsystems are touched (`server/`, `client/`, `plugin/`, `proto/`, + `migrations/`, `packages/proto-python-gen/`). +3. Draft the description in this structure: + + 1. **Summary** — 2-4 sentences: what this PR delivers and why it exists. + Lead with the user- or operator-facing capability, not the implementation. + 2. **How it works** — the end-to-end mechanism in plain language. Walk the + primary flow(s) step by step (who triggers it, what crosses each boundary, + where state is persisted, what comes back). Assume the reader does not + know Go/TS idioms; explain workflows and mechanisms, not syntax. + 3. **Diagrams** — include mermaid diagrams in ```mermaid fenced blocks so + they render on GitHub. At minimum a component/flow diagram of the main + path; add a state or sequence diagram where lifecycle or ordering matters. + Keep syntax GitHub-safe: quote labels containing special characters, avoid + fragile edge styles (e.g. dotted/labelled edges that GitHub mis-renders). + 4. **Areas of the code involved** — a table so reviewers know where to focus: + `| Area / package / file | What changed | Why it matters for review |`. + Group by subsystem. Call out new vs. modified files, and flag generated + code (`**/generated/**`, `*.pb.go`, `*.pb.ts`) as "generated — skip". + 5. **Key technical decisions & trade-offs** — bullet the choices a reviewer + should scrutinize: new abstractions, data-model/migration changes, + security or validation boundaries, backward-compat or rollout concerns. + One line each: the decision and the alternative it was chosen over. + 6. **Testing & validation** — how correctness was verified (tests added, + manual checks, migrations run) and what is explicitly NOT covered. + +4. If a PR exists, update it with `gh pr edit --body-file ` + (write the body to a temp file to preserve mermaid fences and tables). + If no PR exists yet, output the body for the user to use when opening it. + +## Rules + +- Mechanism and architecture over line-by-line detail. If a reviewer needs to + open a file to understand the shape of the change, the description has failed. +- Don't narrate the back-and-forth or rejected approaches — describe the final + state. +- No filler praise. Be concise; prefer tables and diagrams over long paragraphs. diff --git a/AGENTS.md b/AGENTS.md index 8582be948..f74d3bd58 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -77,6 +77,38 @@ These are the rules that recur in code review and that contributors most often m - After a PR merges, do not reuse that branch for follow-up work — cut a fresh branch from the updated `main`. +## PR descriptions + +When opening a PR, write the description so a reviewer can judge the +architecture and technical decisions **without reading the low-level code**. +Inspect the actual diff, commits, and changed files first; describe what the +code does, not the decisions made getting there. Structure it as: + +1. **Summary** — 2-4 sentences: what this PR delivers and why. Lead with the + user- or operator-facing capability, not the implementation. +2. **How it works** — the end-to-end mechanism in plain language. Walk the + primary flow(s): who triggers it, what crosses each boundary, where state + is persisted, what comes back. Explain workflows, not language syntax. +3. **Diagrams** — mermaid in ```mermaid fenced blocks so they render on + GitHub. At least a component/flow diagram of the main path; add a state or + sequence diagram where lifecycle or ordering matters. Keep syntax + GitHub-safe: quote labels with special characters, avoid fragile edge styles. +4. **Areas of the code involved** — a table mapping each changed area to its + role so reviewers know where to focus: `| Area / package / file | What + changed | Why it matters for review |`. Group by subsystem (`proto/`, + `server/`, domain logic, migrations, `client/`, `plugin/`); flag generated + code as "generated — skip". +5. **Key technical decisions & trade-offs** — the choices a reviewer should + scrutinize (new abstractions, data-model/migration changes, security or + validation boundaries, backward-compat or rollout concerns). One line each: + the decision and the alternative it was chosen over. +6. **Testing & validation** — how correctness was verified and what is + explicitly not covered. + +Keep it concise: tables and diagrams over long paragraphs, no filler praise, +no narration of rejected approaches. Claude Code users can generate this with +`/pr-describe`. + ## Verification - Don't pin versions, package versions, or upstream behaviors from training @@ -129,6 +161,7 @@ stays private. - `/regen` — run `just gen`, surface what changed, flag generated files to commit - `/pr-ready` — lint + targeted tests + diff summary suitable for a PR description +- `/pr-describe` — write/update a PR description (high-level mechanism, mermaid diagrams, code-area map) per the "PR descriptions" standard above - `/release-notes ` — draft release notes from commits since the previous tag - `/triage-pr <#>` — fetch PR status, summarize failing CI, propose next action - `/plan ` — scaffold a new TDD, PRD, or plan under `docs/plans/` From a5bb9a70b82cf6ed4217800e2e5c56f0dcc26da2 Mon Sep 17 00:00:00 2001 From: Ankit Goswami <ankitg@squareup.com> Date: Mon, 15 Jun 2026 10:25:43 -0700 Subject: [PATCH 02/13] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Ankit Goswami <ankit.goswami@gmail.com> --- .claude/commands/pr-describe.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/commands/pr-describe.md b/.claude/commands/pr-describe.md index a3b414bcb..1693d44c9 100644 --- a/.claude/commands/pr-describe.md +++ b/.claude/commands/pr-describe.md @@ -1,6 +1,6 @@ --- description: Write or update a PR description that lets reviewers judge the architecture and technical decisions without reading low-level code — high-level mechanism, mermaid diagrams, and a code-area map. -argument-hint: (no arguments; run on the current branch, or pass a PR number/URL to describe an existing PR) +argument-hint: (optional: PR number/URL; defaults to current branch PR or draft body) --- Write (or update) the description for this PR so a reviewer can understand what From 51cc158c334f54534f3e439b90df5eb447f82443 Mon Sep 17 00:00:00 2001 From: Ankit Goswami <ankitg@squareup.com> Date: Mon, 15 Jun 2026 10:26:29 -0700 Subject: [PATCH 03/13] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Ankit Goswami <ankit.goswami@gmail.com> --- .claude/commands/pr-describe.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/commands/pr-describe.md b/.claude/commands/pr-describe.md index 1693d44c9..6c834de93 100644 --- a/.claude/commands/pr-describe.md +++ b/.claude/commands/pr-describe.md @@ -26,7 +26,7 @@ describe what the code does, not the decisions made getting there. primary flow(s) step by step (who triggers it, what crosses each boundary, where state is persisted, what comes back). Assume the reader does not know Go/TS idioms; explain workflows and mechanisms, not syntax. - 3. **Diagrams** — include mermaid diagrams in ```mermaid fenced blocks so + 3. **Diagrams** — include mermaid diagrams in fenced code blocks labeled `mermaid` so they render on GitHub. At minimum a component/flow diagram of the main path; add a state or sequence diagram where lifecycle or ordering matters. Keep syntax GitHub-safe: quote labels containing special characters, avoid From f07cbf9ecc703116db8124b00d8db85569884f37 Mon Sep 17 00:00:00 2001 From: Ankit Goswami <ankitg@squareup.com> Date: Mon, 15 Jun 2026 10:26:44 -0700 Subject: [PATCH 04/13] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Ankit Goswami <ankit.goswami@gmail.com> --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index f74d3bd58..db1d913cf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -89,7 +89,7 @@ code does, not the decisions made getting there. Structure it as: 2. **How it works** — the end-to-end mechanism in plain language. Walk the primary flow(s): who triggers it, what crosses each boundary, where state is persisted, what comes back. Explain workflows, not language syntax. -3. **Diagrams** — mermaid in ```mermaid fenced blocks so they render on +3. **Diagrams** — mermaid in fenced code blocks labeled `mermaid` so they render on GitHub. At least a component/flow diagram of the main path; add a state or sequence diagram where lifecycle or ordering matters. Keep syntax GitHub-safe: quote labels with special characters, avoid fragile edge styles. From dddab134ca5ec5110e5c31dad7154eee2306070f Mon Sep 17 00:00:00 2001 From: Ankit Goswami <ankitg@squareup.com> Date: Mon, 15 Jun 2026 10:26:53 -0700 Subject: [PATCH 05/13] fix(pr-describe): use target PR's refs on the numbered-PR path The numbered-PR path resolved the PR in step 1 but then read the change via local git (HEAD), so running `/pr-describe <n>` from any other branch drafted from an unrelated checkout and could overwrite the requested PR with the wrong description. Split the steps into two explicit paths that never mix: the numbered-PR path reads exclusively via `gh pr diff <n>` / `gh pr view <n> --json commits` and edits by the metadata-derived `number`; the current-branch path keeps using local `git diff <base>...HEAD`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --- .claude/commands/pr-describe.md | 47 +++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/.claude/commands/pr-describe.md b/.claude/commands/pr-describe.md index 6c834de93..6a64fd4e8 100644 --- a/.claude/commands/pr-describe.md +++ b/.claude/commands/pr-describe.md @@ -10,14 +10,35 @@ describe what the code does, not the decisions made getting there. ## Steps -1. Determine the target. If `$ARGUMENTS` is a PR number or URL, use that PR. - Otherwise describe the current branch's PR (`gh pr view --json - number,url,baseRefName,headRefName`); if none exists yet, draft the body for - the PR the user is about to open. -2. Read the change: `git diff <base>...HEAD --stat`, `gh pr diff` (or - `git diff <base>...HEAD`), and `git log <base>..HEAD --oneline`. Identify - which subsystems are touched (`server/`, `client/`, `plugin/`, `proto/`, - `migrations/`, `packages/proto-python-gen/`). +1. Determine the target and pick the path. Decide once, here, and use the same + path for every command below — never mix PR-derived refs with the local + checkout. + + - **Numbered-PR path** — `$ARGUMENTS` is a PR number or URL. The target is + that PR, which may be on a branch you do not have checked out. Resolve its + refs from metadata, not from local HEAD: + `gh pr view "$ARGUMENTS" --json number,url,headRefName,baseRefName,headRepositoryOwner,title`. + Capture `number` (use this, not `$ARGUMENTS`, for the `gh pr edit` in + step 4). + - **Current-branch path** — no `$ARGUMENTS`. The target is the current + branch. `gh pr view --json number,url,headRefName,baseRefName` tells you + whether a PR already exists; if none does, you will draft the body for the + PR the user is about to open from this branch. + +2. Read the change using the path chosen in step 1 — do not fall back to local + `git` on the numbered-PR path, since local HEAD may be an unrelated branch: + + - **Numbered-PR path:** `gh pr diff "$ARGUMENTS"` for the full diff and + `gh pr diff "$ARGUMENTS" --name-only` for the file list. Pull the commit + list from `gh pr view "$ARGUMENTS" --json commits`. All of these read the + PR head as it exists on the remote, regardless of what is checked out. + - **Current-branch path:** `git diff <base>...HEAD` (full diff), + `git diff <base>...HEAD --stat`, and `git log <base>..HEAD --oneline`, + where `<base>` is the `baseRefName` from step 1 (default `main`). + + From the file list, identify which subsystems are touched (`server/`, + `client/`, `plugin/`, `proto/`, `migrations/`, `packages/proto-python-gen/`). + 3. Draft the description in this structure: 1. **Summary** — 2-4 sentences: what this PR delivers and why it exists. @@ -42,9 +63,13 @@ describe what the code does, not the decisions made getting there. 6. **Testing & validation** — how correctness was verified (tests added, manual checks, migrations run) and what is explicitly NOT covered. -4. If a PR exists, update it with `gh pr edit <number> --body-file <tmp>` - (write the body to a temp file to preserve mermaid fences and tables). - If no PR exists yet, output the body for the user to use when opening it. +4. Apply the result against the target resolved in step 1: + - If a PR exists, update **that** PR by its `number`: + `gh pr edit <number> --body-file <tmp>` (write the body to a temp file to + preserve mermaid fences and tables). Use the `number` from step 1, never + `$ARGUMENTS` raw and never the current branch's PR on the numbered-PR path. + - If no PR exists yet (current-branch path only), output the body for the + user to use when opening it. ## Rules From 769d4da06e9dc885173ee059528d6f484d934188 Mon Sep 17 00:00:00 2001 From: Ankit Goswami <ankitg@squareup.com> Date: Mon, 15 Jun 2026 10:32:16 -0700 Subject: [PATCH 06/13] docs(pr-ready): align PR description with the PR descriptions standard Both /pr-ready step 5 and CONTRIBUTING.md still drafted the old "Summary + Test Plan" body, diverging from the six-part PR descriptions standard added to AGENTS.md. Point both at that standard (the same structure /pr-describe produces) so the three stay consistent; /pr-ready folds its lint/test results into Testing & validation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --- .claude/commands/pr-ready.md | 10 +++++++--- CONTRIBUTING.md | 23 +++++++++++++++++++---- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/.claude/commands/pr-ready.md b/.claude/commands/pr-ready.md index e30e2677e..dc16b6510 100644 --- a/.claude/commands/pr-ready.md +++ b/.claude/commands/pr-ready.md @@ -40,9 +40,13 @@ PR description they can paste — or open the PR directly if asked. imports (`client-boundaries`), edits to deployed migrations (`migration-immutability`), `--no-verify` slipping in (`lefthook-bypass-guard`). -5. Draft a PR description following the format in CONTRIBUTING.md: - - **Summary** (1–3 bullets, focus on the *why*) - - **Test Plan** (what was run, how to verify manually) +5. Draft a PR description following the **PR descriptions** standard in + AGENTS.md — the same six-part structure `/pr-describe` produces: Summary, + How it works, Diagrams (mermaid), Areas of the code involved, Key technical + decisions & trade-offs, and Testing & validation. Fold the lint and test + results from steps 2–4 into the Testing & validation section. You already + have the diff and subsystem list from step 1, so draft inline rather than + re-deriving; run `/pr-describe` instead if you want the full standalone pass. 6. **Open the PR only if the user's invocation explicitly asked you to** (e.g. "open it", "create the PR", "ship it"). Otherwise stop after presenting the draft so the user can edit it before running diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 16f2d6e0a..6c4179c6b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -112,16 +112,31 @@ Prefixes: ### Pull Requests -Create PRs with a clear summary and test plan: +Write the description so a reviewer can judge the architecture and technical +decisions without reading the low-level code. Use the six-part structure +documented in the **PR descriptions** section of [AGENTS.md](./AGENTS.md): +Summary, How it works, Diagrams (mermaid, so they render on GitHub), Areas of +the code involved, Key technical decisions & trade-offs, and Testing & +validation. Scale each section to the change — a one-line fix does not need a +diagram, a new subsystem does. ```bash gh pr create --title "Brief description" --body "## Summary -- Bullet point summary of changes +- What this delivers and why -## Test Plan -- How to verify the changes work" +## How it works +- The end-to-end mechanism in plain language + +## Areas of the code involved +| Area / file | What changed | Why it matters for review | +| --- | --- | --- | + +## Testing & validation +- What was run and how to verify; what is not covered" ``` +Claude Code users can generate a conforming description with `/pr-describe`. + ## Cross-Component Workflows ### Adding a New API Endpoint From 7c75535d817dba2872844f52353b97e1dc94eb61 Mon Sep 17 00:00:00 2001 From: Ankit Goswami <ankitg@squareup.com> Date: Mon, 15 Jun 2026 10:34:48 -0700 Subject: [PATCH 07/13] fix(pr-describe): quote argument-hint so the command loads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The argument-hint contained "optional: PR" — a colon followed by a space inside an unquoted YAML scalar, which strict parsers (e.g. Psych) reject as a nested mapping. Claude Code reads slash-command metadata from this frontmatter, so the command could fail to load. Quote the value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --- .claude/commands/pr-describe.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/commands/pr-describe.md b/.claude/commands/pr-describe.md index 6a64fd4e8..e3e8685f7 100644 --- a/.claude/commands/pr-describe.md +++ b/.claude/commands/pr-describe.md @@ -1,6 +1,6 @@ --- description: Write or update a PR description that lets reviewers judge the architecture and technical decisions without reading low-level code — high-level mechanism, mermaid diagrams, and a code-area map. -argument-hint: (optional: PR number/URL; defaults to current branch PR or draft body) +argument-hint: "(optional: PR number/URL; defaults to current branch PR or draft body)" --- Write (or update) the description for this PR so a reviewer can understand what From 1d01536daffee1b5e32fca6a0681f70a502df0c3 Mon Sep 17 00:00:00 2001 From: Ankit Goswami <ankitg@squareup.com> Date: Mon, 15 Jun 2026 10:47:00 -0700 Subject: [PATCH 08/13] fix(pr-describe): preserve PR repo when reading and editing On the numbered-PR path the argument may be a URL for a different repo, but the read/edit calls used a bare PR number, which gh resolves in the current repo. A cross-repo URL could read one PR and then edit a same-numbered PR locally. Parse owner/repo from the resolved url and pass -R <owner>/<repo> on every gh pr call (diff, view, edit) for that path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --- .claude/commands/pr-describe.md | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/.claude/commands/pr-describe.md b/.claude/commands/pr-describe.md index e3e8685f7..28488eee2 100644 --- a/.claude/commands/pr-describe.md +++ b/.claude/commands/pr-describe.md @@ -15,11 +15,15 @@ describe what the code does, not the decisions made getting there. checkout. - **Numbered-PR path** — `$ARGUMENTS` is a PR number or URL. The target is - that PR, which may be on a branch you do not have checked out. Resolve its - refs from metadata, not from local HEAD: - `gh pr view "$ARGUMENTS" --json number,url,headRefName,baseRefName,headRepositoryOwner,title`. - Capture `number` (use this, not `$ARGUMENTS`, for the `gh pr edit` in - step 4). + that PR, which may live in a different repository and on a branch you do + not have checked out. Resolve its refs from metadata, not from local HEAD: + `gh pr view "$ARGUMENTS" --json number,url,headRefName,baseRefName,title`. + Capture `number` and parse `owner`/`repo` from the `url` field (which is + `https://github.com/<owner>/<repo>/pull/<number>`). The `url` is gh's + output, so it is safe to parse. You need `owner`/`repo` because a bare + `number` resolves in the *current* repo — a `$ARGUMENTS` URL pointing at + another repo would otherwise read one PR and edit a same-numbered PR here. + Pass `-R <owner>/<repo> <number>` to every `gh pr` call on this path. - **Current-branch path** — no `$ARGUMENTS`. The target is the current branch. `gh pr view --json number,url,headRefName,baseRefName` tells you whether a PR already exists; if none does, you will draft the body for the @@ -28,10 +32,12 @@ describe what the code does, not the decisions made getting there. 2. Read the change using the path chosen in step 1 — do not fall back to local `git` on the numbered-PR path, since local HEAD may be an unrelated branch: - - **Numbered-PR path:** `gh pr diff "$ARGUMENTS"` for the full diff and - `gh pr diff "$ARGUMENTS" --name-only` for the file list. Pull the commit - list from `gh pr view "$ARGUMENTS" --json commits`. All of these read the - PR head as it exists on the remote, regardless of what is checked out. + - **Numbered-PR path:** using the `owner`/`repo`/`number` from step 1, + `gh pr diff <number> -R <owner>/<repo>` for the full diff and + `gh pr diff <number> -R <owner>/<repo> --name-only` for the file list. + Pull the commit list from + `gh pr view <number> -R <owner>/<repo> --json commits`. All of these read + the PR head in its own repo, regardless of what is checked out locally. - **Current-branch path:** `git diff <base>...HEAD` (full diff), `git diff <base>...HEAD --stat`, and `git log <base>..HEAD --oneline`, where `<base>` is the `baseRefName` from step 1 (default `main`). @@ -64,10 +70,11 @@ describe what the code does, not the decisions made getting there. manual checks, migrations run) and what is explicitly NOT covered. 4. Apply the result against the target resolved in step 1: - - If a PR exists, update **that** PR by its `number`: - `gh pr edit <number> --body-file <tmp>` (write the body to a temp file to - preserve mermaid fences and tables). Use the `number` from step 1, never - `$ARGUMENTS` raw and never the current branch's PR on the numbered-PR path. + - If a PR exists, update **that** PR by its `number`, scoped to its repo: + `gh pr edit <number> -R <owner>/<repo> --body-file <tmp>` (write the body + to a temp file to preserve mermaid fences and tables). The `-R` is what + keeps a cross-repo URL target from editing a same-numbered PR in the local + repo. On the current-branch path `-R` is unnecessary (the PR is local). - If no PR exists yet (current-branch path only), output the body for the user to use when opening it. From 2fa19cc9b23b6c2d5c6538bd9a5531d9c3e04e9b Mon Sep 17 00:00:00 2001 From: Ankit Goswami <ankitg@squareup.com> Date: Mon, 15 Jun 2026 16:33:55 -0700 Subject: [PATCH 09/13] docs(pr-describe): handle stacked PRs Resolve the diff base from the PR's baseRefName against origin (not the bare local ref) so a moved parent branch can't produce a stale diff, detect when a PR is stacked (base is not the default branch), walk the ancestor chain, and require the description to carry the load-bearing context from upstream PRs a reviewer needs to judge the change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --- .claude/commands/pr-describe.md | 33 ++++++++++++++++++++++++++++++--- AGENTS.md | 5 ++++- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/.claude/commands/pr-describe.md b/.claude/commands/pr-describe.md index 28488eee2..74ee02b07 100644 --- a/.claude/commands/pr-describe.md +++ b/.claude/commands/pr-describe.md @@ -29,6 +29,16 @@ describe what the code does, not the decisions made getting there. whether a PR already exists; if none does, you will draft the body for the PR the user is about to open from this branch. + After resolving refs, check whether the target is **stacked**: if + `baseRefName` is not the repository's default branch (usually `main`), this + PR sits on another PR's branch, so its diff and review only make sense + relative to that base. Walk the stack upward by finding the parent PR whose + head is this PR's base (`gh pr list --head <baseRefName> --state all --json + number,title,url,baseRefName`, adding `-R <owner>/<repo>` on the numbered-PR + path), then repeating on the parent's base until you reach the default + branch. Record the chain (each ancestor's number, title, url) for steps 2 + and 3. + 2. Read the change using the path chosen in step 1 — do not fall back to local `git` on the numbered-PR path, since local HEAD may be an unrelated branch: @@ -38,17 +48,34 @@ describe what the code does, not the decisions made getting there. Pull the commit list from `gh pr view <number> -R <owner>/<repo> --json commits`. All of these read the PR head in its own repo, regardless of what is checked out locally. - - **Current-branch path:** `git diff <base>...HEAD` (full diff), - `git diff <base>...HEAD --stat`, and `git log <base>..HEAD --oneline`, - where `<base>` is the `baseRefName` from step 1 (default `main`). + - **Current-branch path:** fetch the base first (`git fetch origin <base>`) + and diff against `origin/<base>`, never the bare local ref, so a parent + branch that has moved (common in a stack) can't produce a stale diff: + `git diff origin/<base>...HEAD` (full diff), + `git diff origin/<base>...HEAD --stat`, and + `git log origin/<base>..HEAD --oneline`, where `<base>` is the + `baseRefName` from step 1 (default `main`). From the file list, identify which subsystems are touched (`server/`, `client/`, `plugin/`, `proto/`, `migrations/`, `packages/proto-python-gen/`). + If the target is stacked (step 1), also read each ancestor PR's description + (`gh pr view <number> --json title,body,url`, adding `-R` on the numbered-PR + path) and extract the load-bearing context this PR depends on: the contracts, + abstractions, schema, or decisions established upstream that a reviewer must + understand to judge this change. + 3. Draft the description in this structure: 1. **Summary** — 2-4 sentences: what this PR delivers and why it exists. Lead with the user- or operator-facing capability, not the implementation. + If the PR is stacked (step 1), follow the summary with a short **Stack** + note: the chain with PR numbers/links (this PR up to the default branch), + a line stating the diff is relative to the immediate base so the reviewer + should not re-review ancestors, and the required context from the stack, + meaning the upstream contracts, abstractions, or decisions this PR builds + on, distilled to what a reviewer needs here rather than a re-summary of + the parent PRs. 2. **How it works** — the end-to-end mechanism in plain language. Walk the primary flow(s) step by step (who triggers it, what crosses each boundary, where state is persisted, what comes back). Assume the reader does not diff --git a/AGENTS.md b/AGENTS.md index db1d913cf..18cd3910f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -85,7 +85,10 @@ Inspect the actual diff, commits, and changed files first; describe what the code does, not the decisions made getting there. Structure it as: 1. **Summary** — 2-4 sentences: what this PR delivers and why. Lead with the - user- or operator-facing capability, not the implementation. + user- or operator-facing capability, not the implementation. For a stacked + PR (base is not the default branch), add a short *Stack* note: the chain with + PR links, that the diff is relative to the immediate base, and the + load-bearing context from upstream PRs a reviewer needs to judge this change. 2. **How it works** — the end-to-end mechanism in plain language. Walk the primary flow(s): who triggers it, what crosses each boundary, where state is persisted, what comes back. Explain workflows, not language syntax. From c3c451925de467be03d7f5ab316e651c6d60156d Mon Sep 17 00:00:00 2001 From: Ankit Goswami <ankitg@squareup.com> Date: Mon, 15 Jun 2026 16:37:01 -0700 Subject: [PATCH 10/13] docs(pr-describe): surface downstream stack context Walk the stack downward as well as upward: find child PRs (whose base is this PR's head) so the Stack note can show the full chain and tell the reviewer what is deferred to descendant PRs. A reviewer of a deliberately scoped PR needs to know what remains and where it lands, not just what the PR builds on. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --- .claude/commands/pr-describe.md | 25 ++++++++++++++++--------- AGENTS.md | 8 +++++--- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/.claude/commands/pr-describe.md b/.claude/commands/pr-describe.md index 74ee02b07..f33f2a00a 100644 --- a/.claude/commands/pr-describe.md +++ b/.claude/commands/pr-describe.md @@ -36,8 +36,11 @@ describe what the code does, not the decisions made getting there. head is this PR's base (`gh pr list --head <baseRefName> --state all --json number,title,url,baseRefName`, adding `-R <owner>/<repo>` on the numbered-PR path), then repeating on the parent's base until you reach the default - branch. Record the chain (each ancestor's number, title, url) for steps 2 - and 3. + branch. Also walk downward to find what is stacked on top: child PRs whose + base is this PR's head (`gh pr list --base <headRefName> --state open --json + number,title,url,baseRefName`, adding `-R <owner>/<repo>` on the numbered-PR + path), repeating on each child's head. Record both ancestors and descendants + (each one's number, title, url) for steps 2 and 3. 2. Read the change using the path chosen in step 1 — do not fall back to local `git` on the numbered-PR path, since local HEAD may be an unrelated branch: @@ -63,19 +66,23 @@ describe what the code does, not the decisions made getting there. (`gh pr view <number> --json title,body,url`, adding `-R` on the numbered-PR path) and extract the load-bearing context this PR depends on: the contracts, abstractions, schema, or decisions established upstream that a reviewer must - understand to judge this change. + understand to judge this change. For each descendant, capture a one-line + scope from its title (skim its body only if the title is opaque) so you can + tell the reviewer what is deferred to later PRs and where it lands. 3. Draft the description in this structure: 1. **Summary** — 2-4 sentences: what this PR delivers and why it exists. Lead with the user- or operator-facing capability, not the implementation. If the PR is stacked (step 1), follow the summary with a short **Stack** - note: the chain with PR numbers/links (this PR up to the default branch), - a line stating the diff is relative to the immediate base so the reviewer - should not re-review ancestors, and the required context from the stack, - meaning the upstream contracts, abstractions, or decisions this PR builds - on, distilled to what a reviewer needs here rather than a re-summary of - the parent PRs. + note: the full chain with PR numbers/links (ancestors down to the default + branch, this PR, and any PRs stacked on top), with this PR marked; a line + stating the diff is relative to the immediate base so the reviewer should + not re-review ancestors; the required context from upstream, meaning the + contracts, abstractions, or decisions this PR builds on, distilled to what + a reviewer needs here rather than a re-summary of the parent PRs; and + what is deferred to descendant PRs, so the reviewer knows what is + intentionally out of scope here and where the remaining work lands. 2. **How it works** — the end-to-end mechanism in plain language. Walk the primary flow(s) step by step (who triggers it, what crosses each boundary, where state is persisted, what comes back). Assume the reader does not diff --git a/AGENTS.md b/AGENTS.md index 18cd3910f..3a73cb2d3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -86,9 +86,11 @@ code does, not the decisions made getting there. Structure it as: 1. **Summary** — 2-4 sentences: what this PR delivers and why. Lead with the user- or operator-facing capability, not the implementation. For a stacked - PR (base is not the default branch), add a short *Stack* note: the chain with - PR links, that the diff is relative to the immediate base, and the - load-bearing context from upstream PRs a reviewer needs to judge this change. + PR (base is not the default branch), add a short *Stack* note: the full chain + with PR links (ancestors, this PR, and any PRs stacked on top), that the diff + is relative to the immediate base, the load-bearing context from upstream PRs + a reviewer needs to judge this change, and what is deferred to descendant PRs + so the reviewer knows what is intentionally out of scope here. 2. **How it works** — the end-to-end mechanism in plain language. Walk the primary flow(s): who triggers it, what crosses each boundary, where state is persisted, what comes back. Explain workflows, not language syntax. From dbef14b3d034c5ea825eb907b982375cf1ff6d4d Mon Sep 17 00:00:00 2001 From: Ankit Goswami <ankitg@squareup.com> Date: Mon, 15 Jun 2026 16:38:21 -0700 Subject: [PATCH 11/13] docs(pr-describe): source remaining-work context from plan and chat Deferred work is often not an open PR yet. Treat the effort's plan docs (docs/plans/) and the interactive conversation as first-class sources for what is out of scope and where it lands, alongside descendant PRs, and record the tracking references even when no PR exists. Keep it to scope facts, not a narration of how the work was planned. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --- .claude/commands/pr-describe.md | 13 +++++++++++-- AGENTS.md | 5 +++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/.claude/commands/pr-describe.md b/.claude/commands/pr-describe.md index f33f2a00a..d0f13b3d7 100644 --- a/.claude/commands/pr-describe.md +++ b/.claude/commands/pr-describe.md @@ -70,6 +70,14 @@ describe what the code does, not the decisions made getting there. scope from its title (skim its body only if the title is opaque) so you can tell the reviewer what is deferred to later PRs and where it lands. + Remaining work is often not open as a PR yet, so do not stop at descendant + PRs. Also draw on the effort's plan documents (e.g. `docs/plans/`) for the + phasing and explicit out-of-scope items, and, when running interactively, on + this conversation, which may already name the deferred scope and the tracking + issues/PRs it lands in. Record those deferred items with their tracking + references even when no PR exists for them yet (state facts about scope, not + the back-and-forth of how the work was planned). + 3. Draft the description in this structure: 1. **Summary** — 2-4 sentences: what this PR delivers and why it exists. @@ -81,8 +89,9 @@ describe what the code does, not the decisions made getting there. not re-review ancestors; the required context from upstream, meaning the contracts, abstractions, or decisions this PR builds on, distilled to what a reviewer needs here rather than a re-summary of the parent PRs; and - what is deferred to descendant PRs, so the reviewer knows what is - intentionally out of scope here and where the remaining work lands. + what is intentionally out of scope here and where the remaining work + lands, drawn from descendant PRs, the plan docs, this conversation, and + any tracking issues (later phases may not be open as PRs yet). 2. **How it works** — the end-to-end mechanism in plain language. Walk the primary flow(s) step by step (who triggers it, what crosses each boundary, where state is persisted, what comes back). Assume the reader does not diff --git a/AGENTS.md b/AGENTS.md index 3a73cb2d3..f64e5e9e2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -89,8 +89,9 @@ code does, not the decisions made getting there. Structure it as: PR (base is not the default branch), add a short *Stack* note: the full chain with PR links (ancestors, this PR, and any PRs stacked on top), that the diff is relative to the immediate base, the load-bearing context from upstream PRs - a reviewer needs to judge this change, and what is deferred to descendant PRs - so the reviewer knows what is intentionally out of scope here. + a reviewer needs to judge this change, and what is intentionally out of scope + here and where the remaining work lands (from descendant PRs, the plan docs, + or tracking issues, since later phases may not be open as PRs yet). 2. **How it works** — the end-to-end mechanism in plain language. Walk the primary flow(s): who triggers it, what crosses each boundary, where state is persisted, what comes back. Explain workflows, not language syntax. From f962769b51e93d2e6adbb6c5ff5b58f9fa65b3fb Mon Sep 17 00:00:00 2001 From: Ankit Goswami <ankitg@squareup.com> Date: Mon, 15 Jun 2026 16:44:54 -0700 Subject: [PATCH 12/13] docs(pr-describe): trigger Stack note for any series, not just stacked PRs A foundation PR that targets the default branch but has descendants (or a title marked N/M) is still part of a series and needs the Stack note. Detect a series from any of three signals (base is not the default branch, has descendant PRs, or an N/M / part-N title) instead of the base ref alone. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --- .claude/commands/pr-describe.md | 37 ++++++++++++++++++--------------- AGENTS.md | 9 ++++---- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/.claude/commands/pr-describe.md b/.claude/commands/pr-describe.md index d0f13b3d7..07b3a6de0 100644 --- a/.claude/commands/pr-describe.md +++ b/.claude/commands/pr-describe.md @@ -29,18 +29,20 @@ describe what the code does, not the decisions made getting there. whether a PR already exists; if none does, you will draft the body for the PR the user is about to open from this branch. - After resolving refs, check whether the target is **stacked**: if - `baseRefName` is not the repository's default branch (usually `main`), this - PR sits on another PR's branch, so its diff and review only make sense - relative to that base. Walk the stack upward by finding the parent PR whose - head is this PR's base (`gh pr list --head <baseRefName> --state all --json + After resolving refs, check whether the target is part of a **series** + (stacked or multi-part). Any one of these signals counts: `baseRefName` is + not the repository's default branch (it is stacked on a parent PR); it has + descendant PRs (others are stacked on it); or its title carries an `N/M` or + `part N` marker. A foundation PR that targets the default branch but has + descendants still counts, so do not gate on the base ref alone. Walk the + chain both ways. Upward: find the parent PR whose head is this PR's base + (`gh pr list --head <baseRefName> --state all --json number,title,url,baseRefName`, adding `-R <owner>/<repo>` on the numbered-PR - path), then repeating on the parent's base until you reach the default - branch. Also walk downward to find what is stacked on top: child PRs whose - base is this PR's head (`gh pr list --base <headRefName> --state open --json - number,title,url,baseRefName`, adding `-R <owner>/<repo>` on the numbered-PR - path), repeating on each child's head. Record both ancestors and descendants - (each one's number, title, url) for steps 2 and 3. + path), repeating on the parent's base until you reach the default branch. + Downward: find child PRs whose base is this PR's head (`gh pr list --base + <headRefName> --state open --json number,title,url,baseRefName`, adding `-R`), + repeating on each child's head. Record both ancestors and descendants (each + one's number, title, url) for steps 2 and 3. 2. Read the change using the path chosen in step 1 — do not fall back to local `git` on the numbered-PR path, since local HEAD may be an unrelated branch: @@ -62,7 +64,7 @@ describe what the code does, not the decisions made getting there. From the file list, identify which subsystems are touched (`server/`, `client/`, `plugin/`, `proto/`, `migrations/`, `packages/proto-python-gen/`). - If the target is stacked (step 1), also read each ancestor PR's description + If the target is part of a series (step 1), also read each ancestor PR's description (`gh pr view <number> --json title,body,url`, adding `-R` on the numbered-PR path) and extract the load-bearing context this PR depends on: the contracts, abstractions, schema, or decisions established upstream that a reviewer must @@ -82,11 +84,12 @@ describe what the code does, not the decisions made getting there. 1. **Summary** — 2-4 sentences: what this PR delivers and why it exists. Lead with the user- or operator-facing capability, not the implementation. - If the PR is stacked (step 1), follow the summary with a short **Stack** - note: the full chain with PR numbers/links (ancestors down to the default - branch, this PR, and any PRs stacked on top), with this PR marked; a line - stating the diff is relative to the immediate base so the reviewer should - not re-review ancestors; the required context from upstream, meaning the + If the PR is part of a series (step 1), follow the summary with a short + **Stack** note: the full chain with PR numbers/links (ancestors down to + the default branch, this PR, and any PRs stacked on top), with this PR + marked; if it has ancestors, a line stating the diff is relative to its + immediate base so the reviewer does not re-review them; the required + context from upstream, meaning the contracts, abstractions, or decisions this PR builds on, distilled to what a reviewer needs here rather than a re-summary of the parent PRs; and what is intentionally out of scope here and where the remaining work diff --git a/AGENTS.md b/AGENTS.md index f64e5e9e2..8a5b925b4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -85,10 +85,11 @@ Inspect the actual diff, commits, and changed files first; describe what the code does, not the decisions made getting there. Structure it as: 1. **Summary** — 2-4 sentences: what this PR delivers and why. Lead with the - user- or operator-facing capability, not the implementation. For a stacked - PR (base is not the default branch), add a short *Stack* note: the full chain - with PR links (ancestors, this PR, and any PRs stacked on top), that the diff - is relative to the immediate base, the load-bearing context from upstream PRs + user- or operator-facing capability, not the implementation. For a PR in a + series (stacked on a parent, has descendant PRs, or marked `N/M`), add a + short *Stack* note: the full chain with PR links (ancestors, this PR, and any + PRs stacked on top), that the diff is relative to the immediate base when it + has ancestors, the load-bearing context from upstream PRs a reviewer needs to judge this change, and what is intentionally out of scope here and where the remaining work lands (from descendant PRs, the plan docs, or tracking issues, since later phases may not be open as PRs yet). From 28c0b8998a838591508292aec042ee719488148d Mon Sep 17 00:00:00 2001 From: Ankit Goswami <ankitg@squareup.com> Date: Tue, 16 Jun 2026 08:28:19 -0700 Subject: [PATCH 13/13] docs(pr-commands): quote branch refs; keep pr-ready base-aware - pr-describe: quote JSON-derived branch refs (headRefName/baseRefName) in the shell examples and add a rule, so a branch name with shell metacharacters can't run unintended commands. - pr-ready: only reuse the step-1 main...HEAD diff for the description when the branch targets the default branch; for a stacked/series branch, delegate to /pr-describe so the description is scoped to the real base instead of folding in ancestor changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --- .claude/commands/pr-describe.md | 17 +++++++++++------ .claude/commands/pr-ready.md | 10 +++++++--- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/.claude/commands/pr-describe.md b/.claude/commands/pr-describe.md index 07b3a6de0..e48c80c01 100644 --- a/.claude/commands/pr-describe.md +++ b/.claude/commands/pr-describe.md @@ -36,11 +36,11 @@ describe what the code does, not the decisions made getting there. `part N` marker. A foundation PR that targets the default branch but has descendants still counts, so do not gate on the base ref alone. Walk the chain both ways. Upward: find the parent PR whose head is this PR's base - (`gh pr list --head <baseRefName> --state all --json + (`gh pr list --head "<baseRefName>" --state all --json number,title,url,baseRefName`, adding `-R <owner>/<repo>` on the numbered-PR path), repeating on the parent's base until you reach the default branch. Downward: find child PRs whose base is this PR's head (`gh pr list --base - <headRefName> --state open --json number,title,url,baseRefName`, adding `-R`), + "<headRefName>" --state open --json number,title,url,baseRefName`, adding `-R`), repeating on each child's head. Record both ancestors and descendants (each one's number, title, url) for steps 2 and 3. @@ -53,12 +53,12 @@ describe what the code does, not the decisions made getting there. Pull the commit list from `gh pr view <number> -R <owner>/<repo> --json commits`. All of these read the PR head in its own repo, regardless of what is checked out locally. - - **Current-branch path:** fetch the base first (`git fetch origin <base>`) + - **Current-branch path:** fetch the base first (`git fetch origin "<base>"`) and diff against `origin/<base>`, never the bare local ref, so a parent branch that has moved (common in a stack) can't produce a stale diff: - `git diff origin/<base>...HEAD` (full diff), - `git diff origin/<base>...HEAD --stat`, and - `git log origin/<base>..HEAD --oneline`, where `<base>` is the + `git diff "origin/<base>...HEAD"` (full diff), + `git diff "origin/<base>...HEAD" --stat`, and + `git log "origin/<base>..HEAD" --oneline`, where `<base>` is the `baseRefName` from step 1 (default `main`). From the file list, identify which subsystems are touched (`server/`, @@ -131,3 +131,8 @@ describe what the code does, not the decisions made getting there. - Don't narrate the back-and-forth or rejected approaches — describe the final state. - No filler praise. Be concise; prefer tables and diagrams over long paragraphs. +- Always quote JSON-derived branch refs (`headRefName`, `baseRefName`) when + passing them to a shell command. Branch names may contain shell + metacharacters (a branch literally named `feature;rm -rf foo` is valid on + GitHub), so an unquoted ref can run unintended commands instead of just + querying `gh`/`git`. diff --git a/.claude/commands/pr-ready.md b/.claude/commands/pr-ready.md index dc16b6510..e9e1a852b 100644 --- a/.claude/commands/pr-ready.md +++ b/.claude/commands/pr-ready.md @@ -44,9 +44,13 @@ PR description they can paste — or open the PR directly if asked. AGENTS.md — the same six-part structure `/pr-describe` produces: Summary, How it works, Diagrams (mermaid), Areas of the code involved, Key technical decisions & trade-offs, and Testing & validation. Fold the lint and test - results from steps 2–4 into the Testing & validation section. You already - have the diff and subsystem list from step 1, so draft inline rather than - re-deriving; run `/pr-describe` instead if you want the full standalone pass. + results from steps 2–4 into the Testing & validation section. Reuse the + step-1 diff inline only when this branch targets the default branch. If it is + stacked or part of a series (its base is not the default branch, or sibling/ + child PRs exist), do **not** reuse the `main...HEAD` scope: run `/pr-describe`, + which resolves the PR's real base, scopes the diff to the immediate base, and + adds the Stack note. Reusing `main...HEAD` there would fold in ancestor + changes and misrepresent what this PR actually changes. 6. **Open the PR only if the user's invocation explicitly asked you to** (e.g. "open it", "create the PR", "ship it"). Otherwise stop after presenting the draft so the user can edit it before running