|
1 | 1 | --- |
2 | 2 | name: review-external-pr |
3 | | -description: Prepare an external contributor's PR for maintainer review by redirecting it into a dedicated review branch, then merging and creating a new finalization PR targeting next. Use when triaging/reviewing contributor PRs, merging external PRs with maintainer changes, or setting up a review workflow for incoming community contributions. |
| 3 | +description: Triage an external contributor's PR and prepare it for merge. Detects push permission, base branch, draft state, size, and history quality, then picks one of three paths (review-only, direct push to contributor branch, or a `reviews/` staging branch). Use when triaging or reviewing a community PR, deciding whether maintainer changes are needed, choosing a merge strategy (squash/merge/rebase), or merging an external contribution. |
4 | 4 | --- |
5 | 5 |
|
6 | 6 | # Review External PR Workflow |
7 | 7 |
|
8 | | -Redirects an external contributor's PR into a `reviews/` staging branch so a maintainer can inspect, add changes, then merge everything into `next` cleanly. |
| 8 | +A triage-first workflow for handling community PRs. The skill **inspects first, asks two questions, then executes**. It never creates branches preemptively. |
9 | 9 |
|
10 | 10 | ## When to Use |
11 | 11 |
|
12 | | -- An external contributor opened a PR targeting `next` and you want to add changes before merging |
13 | | -- You want to formally review and finalize a community contribution |
14 | | -- You want the contributor to get proper merge credit while still controlling what lands in `next` |
| 12 | +- A contributor PR is open and you want to review and merge it |
| 13 | +- Trigger phrases: "prepare this external PR", "review PR #N", "let's review this contribution", or invocation while the user is on a `pr/<owner>/<PR_NUMBER>` branch (created by `gh pr checkout`) |
| 14 | +- You want a recommendation on push path and merge strategy before doing anything |
15 | 15 |
|
16 | | -## Workflow Steps |
| 16 | +## Phase 1 — Triage (read-only, no prompts) |
17 | 17 |
|
18 | | -### 1. Gather PR Info |
| 18 | +### Identify the PR |
| 19 | + |
| 20 | +Resolve, in order: |
| 21 | + |
| 22 | +1. The PR number the user mentioned. |
| 23 | +2. Current branch matches `pr/<owner>/<PR_NUMBER>` → extract `<PR_NUMBER>`. |
| 24 | +3. `gh pr status` → active PR for current branch. |
| 25 | + |
| 26 | +### Fetch metadata in one call |
19 | 27 |
|
20 | 28 | ```bash |
21 | | -gh pr view <PR_NUMBER> --json title,author,headRefName,baseRefName,body |
| 29 | +gh pr view <PR_NUMBER> --json number,title,author,url,state,isDraft,\ |
| 30 | +headRefName,baseRefName,headRepositoryOwner,maintainerCanModify,\ |
| 31 | +mergeable,mergeStateStatus,additions,deletions,changedFiles,commits,labels |
22 | 32 | ``` |
23 | 33 |
|
24 | | -Note the **PR number**, **title**, and **author login** — you'll need them for branch naming and PR descriptions. |
| 34 | +### Derive signals |
25 | 35 |
|
26 | | -### 2. Create the Review Branch |
| 36 | +| Signal | Rule | |
| 37 | +| --------------- | ----------------------------------------------------------------------------------------------------------------------------- | ----- | -------------- | ---- | ------ | ----- | |
| 38 | +| `canPushToHead` | `headRepositoryOwner.login == "microsoft"` OR `maintainerCanModify` | |
| 39 | +| `baseBranch` | `baseRefName` (do **not** hardcode `main`) | |
| 40 | +| `isDraft` | warn if `true` | |
| 41 | +| `mergeable` | warn if `mergeable != "MERGEABLE"` (values: `MERGEABLE`, `CONFLICTING`, `UNKNOWN`) | |
| 42 | +| `mergeReady` | warn if `mergeStateStatus` is not `CLEAN` (other values: `DIRTY`, `BLOCKED`, `BEHIND`, `UNSTABLE`, `HAS_HOOKS`, `UNKNOWN`) | |
| 43 | +| `commitCount` | `commits.length` | |
| 44 | +| `messyHistory` | any `commits[].messageHeadline` (the first line of the commit message, returned by `gh pr view --json commits`) matches `/wip | fixup | address review | typo | merge( | $)/i` | |
| 45 | +| `changedLines` | `additions + deletions` | |
| 46 | +| `sizeBucket` | small ≤ 50 changed lines, medium ≤ 300, large > 300 (uses `changedLines`) | |
27 | 47 |
|
28 | | -Branch naming format: `reviews/<helpful-name>-original-pr-<number>` |
| 48 | +### Squash recommendation |
29 | 49 |
|
30 | | -```bash |
31 | | -git fetch origin |
32 | | -git checkout -b reviews/<helpful-name>-original-pr-<PR_NUMBER> origin/next |
33 | | -git push origin reviews/<helpful-name>-original-pr-<PR_NUMBER> |
| 50 | +| Condition | Recommend | |
| 51 | +| ------------------------------------------------- | ------------------------------------------------------- | |
| 52 | +| `commitCount == 1` | **No squash** (rebase or merge) — history already clean | |
| 53 | +| `commitCount ≤ 3` AND no messy subjects AND small | Ask, **default no squash** | |
| 54 | +| `commitCount > 3` OR messy subjects detected | **Squash** (default) | |
| 55 | + |
| 56 | +### Print the triage report |
| 57 | + |
| 58 | +``` |
| 59 | +PR #<PR_NUMBER> — <title> |
| 60 | + Author: <login> (<fork|same-repo>) |
| 61 | + Base: <baseBranch> |
| 62 | + State: <state>, <draft?>, mergeable=<mergeable>, mergeStateStatus=<mergeStateStatus> |
| 63 | + Push to head: <✅ allowed reason | ❌ blocked reason> |
| 64 | + Size: +<additions> / -<deletions> across <changedFiles> file(s), <commitCount> commit(s) |
| 65 | + History: <clean | messy: "<sample messageHeadline>"> |
| 66 | +
|
| 67 | +Recommendation: |
| 68 | + • Path: <direct push | reviews/ branch | review-only> |
| 69 | + • Merge: <--squash | --merge | --rebase> (<reason>) |
34 | 70 | ``` |
35 | 71 |
|
36 | | -Example: `reviews/copy-reference-original-pr-545` |
| 72 | +Stop here and present the report. |
| 73 | + |
| 74 | +## Phase 2 — Two questions |
| 75 | + |
| 76 | +Ask **only** these. Pre-select the recommended option. |
| 77 | + |
| 78 | +### Q1: Do you need to add changes before merging? |
| 79 | + |
| 80 | +- **No** → Path A (review & merge) |
| 81 | +- **Yes, small tweaks** → Path B (direct push) if `canPushToHead`, otherwise Path C |
| 82 | +- **Yes, heavy rework / contributor unresponsive** → Path C (`reviews/` staging branch) |
| 83 | + |
| 84 | +If `canPushToHead == false`, omit the "direct push" option and explain: _"Contributor disabled maintainer edits; we must use a `reviews/` branch."_ |
| 85 | + |
| 86 | +### Q2: Merge strategy? |
| 87 | + |
| 88 | +Offer `--squash`, `--merge`, `--rebase` with the recommended option marked. Justify the default in one short sentence (e.g., _"4 commits including 'fix typo' — squash recommended"_). |
| 89 | + |
| 90 | +## Phase 3 — Execute |
37 | 91 |
|
38 | | -### 3. Retarget the Contributor's PR |
| 92 | +Run commands non-interactively, echoing each one. After merge, print a one-line summary with the merged commit/PR URL. |
39 | 93 |
|
40 | | -> ⚠️ **Known issue**: `gh pr edit --base` may emit a deprecation warning about Projects (classic). This is a cosmetic warning only — the base branch change succeeds regardless. Verify with `gh pr view <PR_NUMBER> --json baseRefName`. |
| 94 | +### Path A — Review & merge (no maintainer changes) |
41 | 95 |
|
42 | 96 | ```bash |
43 | | -gh pr edit <PR_NUMBER> --base reviews/<helpful-name>-original-pr-<PR_NUMBER> |
| 97 | +gh pr checkout <PR_NUMBER> # optional, for local inspection |
| 98 | +# review, leave comments via the PR UI or `gh pr review` |
| 99 | +gh pr merge <PR_NUMBER> --<strategy> # against the PR's actual base |
44 | 100 | ``` |
45 | 101 |
|
46 | | -Verify: |
| 102 | +### Path B — Direct push to the contributor's branch |
| 103 | + |
| 104 | +Requires `canPushToHead == true`. |
47 | 105 |
|
48 | 106 | ```bash |
49 | | -gh pr view <PR_NUMBER> --json baseRefName |
| 107 | +gh pr checkout <PR_NUMBER> # sets up a remote tracking the fork branch |
| 108 | +# make changes, commit |
| 109 | +git push # updates the existing PR in place |
| 110 | +gh pr merge <PR_NUMBER> --<strategy> |
50 | 111 | ``` |
51 | 112 |
|
52 | | -### 4. Merge the Contributor's PR |
| 113 | +The existing PR updates in place; the contributor keeps authorship of their commits and maintainer commits are attributed to the maintainer. No second PR is needed. |
| 114 | + |
| 115 | +### Path C — `reviews/` staging branch |
| 116 | + |
| 117 | +Use when push to head is blocked, or when the maintainer explicitly wants to isolate rework. |
| 118 | + |
| 119 | +**Branch slug sanitization** — derive `<slug>` from the PR title: |
53 | 120 |
|
54 | | -Once the base is updated and the PR is ready: |
| 121 | +1. Lowercase. |
| 122 | +2. Replace every run of non-`[a-z0-9]` characters with a single `-`. |
| 123 | +3. Trim leading/trailing `-`. |
| 124 | +4. Truncate to 30 characters; trim trailing `-` again if the cut left one. |
| 125 | + |
| 126 | +Example: `"fix(tree): sort _id_ index first / cleanup"` → `fix-tree-sort-id-index-first-c`. |
| 127 | + |
| 128 | +Full branch name: `reviews/<slug>-pr-<PR_NUMBER>`. |
55 | 129 |
|
56 | 130 | ```bash |
57 | | -gh pr merge <PR_NUMBER> --squash |
| 131 | +git fetch origin |
| 132 | +git checkout -b reviews/<slug>-pr-<PR_NUMBER> origin/<baseBranch> |
| 133 | +git push -u origin reviews/<slug>-pr-<PR_NUMBER> |
58 | 134 | ``` |
59 | 135 |
|
60 | | -Or approve + merge via the GitHub UI to trigger any required status checks. |
| 136 | +Retarget the contributor's PR: |
| 137 | + |
| 138 | +```bash |
| 139 | +gh pr edit <PR_NUMBER> --base reviews/<slug>-pr-<PR_NUMBER> |
| 140 | +gh pr view <PR_NUMBER> --json baseRefName # verify |
| 141 | +``` |
61 | 142 |
|
62 | | -### 5. Create the Finalization PR |
| 143 | +> ⚠️ `gh pr edit --base` may print a deprecation warning about Projects (classic). Cosmetic only — the base change succeeds. |
63 | 144 |
|
64 | | -Pull the merged review branch, then open a new PR from it to `next`: |
| 145 | +Merge the contributor's PR into the review branch: |
65 | 146 |
|
66 | 147 | ```bash |
67 | | -git checkout reviews/<helpful-name>-original-pr-<PR_NUMBER> |
68 | | -git pull origin reviews/<helpful-name>-original-pr-<PR_NUMBER> |
| 148 | +gh pr merge <PR_NUMBER> --<strategy> |
69 | 149 | ``` |
70 | 150 |
|
71 | | -Create the PR: |
| 151 | +Pull and create the finalization PR back to the original base: |
72 | 152 |
|
73 | 153 | ```bash |
| 154 | +git checkout reviews/<slug>-pr-<PR_NUMBER> |
| 155 | +git pull origin reviews/<slug>-pr-<PR_NUMBER> |
| 156 | + |
74 | 157 | gh pr create \ |
75 | | - --base next \ |
76 | | - --head reviews/<helpful-name>-original-pr-<PR_NUMBER> \ |
| 158 | + --base <baseBranch> \ |
| 159 | + --head reviews/<slug>-pr-<PR_NUMBER> \ |
77 | 160 | --title "<original title> [reviewed]" \ |
78 | | - --body "This PR finalizes the review of the contribution originally submitted by @<author_login> in #<PR_NUMBER>. |
| 161 | + --body "Finalizes review of @<author>'s contribution in #<PR_NUMBER>. |
79 | 162 |
|
80 | 163 | Original PR: <PR_URL>" |
81 | 164 | ``` |
82 | 165 |
|
83 | | -### 6. Comment on the Original PR |
84 | | - |
85 | | -Go back to the contributor's original (now merged) PR and leave a comment linking to the finalization PR: |
| 166 | +Comment on the original PR: |
86 | 167 |
|
87 | 168 | ```bash |
88 | | -gh pr comment <ORIGINAL_PR_NUMBER> \ |
89 | | - --body "Thank you for the contribution! The review is continuing in #<NEW_PR_NUMBER> where maintainer changes will be finalized before merging to \`next\`." |
| 169 | +gh pr comment <PR_NUMBER> \ |
| 170 | + --body "Thanks for the contribution! Review continues in #<NEW_PR_NUMBER> where maintainer changes are finalized before merging to \`<baseBranch>\`." |
90 | 171 | ``` |
91 | 172 |
|
| 173 | +## Merge Strategy Reference |
| 174 | + |
| 175 | +| Strategy | When to use | |
| 176 | +| ---------- | -------------------------------------------------------------------------------------------------------------------- | |
| 177 | +| `--squash` | Default for messy/multi-commit external PRs. One revert undoes the change. Contributor still gets authorship credit. | |
| 178 | +| `--merge` | Large feature where individual commits are meaningful and worth preserving. | |
| 179 | +| `--rebase` | Single clean commit, or a series of clean atomic commits you want linear on the base. | |
| 180 | + |
| 181 | +## Hard Rules |
| 182 | + |
| 183 | +- **Never** hardcode `main` as the base — always read `baseRefName`. |
| 184 | +- **Never** create a `reviews/` branch in Phase 1. |
| 185 | +- **Never** force-push to a contributor's branch. |
| 186 | +- If the PR is a **draft**, refuse to merge and report it back to the maintainer. |
| 187 | +- If `mergeable == "CONFLICTING"` or `mergeStateStatus != "CLEAN"`, stop and surface that before any merge command. |
| 188 | + |
92 | 189 | ## Summary |
93 | 190 |
|
94 | | -| Step | Action | Result | |
95 | | -| ---- | ------------------------------------------ | ----------------------------------------- | |
96 | | -| 1 | Gather PR info | Know PR number, title, author | |
97 | | -| 2 | Create `reviews/...` branch off `next` | Staging branch ready | |
98 | | -| 3 | Retarget contributor's PR to review branch | Their diff is scoped to review branch | |
99 | | -| 4 | Merge contributor's PR | Contributor gets merge credit | |
100 | | -| 5 | Create finalization PR to `next` | Maintainer controls what lands in `next` | |
101 | | -| 6 | Comment on original PR with link to new PR | Contributor is informed, thread is linked | |
| 191 | +| Phase | What happens | Output | |
| 192 | +| ----- | ---------------------------------------------------------- | --------------------------- | |
| 193 | +| 1 | Read PR metadata, derive push capability + recommendations | Triage report | |
| 194 | +| 2 | Ask Q1 (path) and Q2 (merge strategy) | Decision | |
| 195 | +| 3 | Execute the chosen path with the chosen merge strategy | Merged PR / finalization PR | |
0 commit comments