Skip to content

Commit 07fee48

Browse files
committed
refactor(pr-review-toolkit): trim SKILL.md to resolve token budget warning
Tighten prose across all sections to bring estimated token count under the 3,000-token skillsaw warn threshold (~3,379 → ~2,450). Preserves all behavioral instructions including ad-hoc script prohibition and allowed-tools constraints. Assisted-by: Claude:claude-opus-4-6
1 parent c4a9fe6 commit 07fee48

1 file changed

Lines changed: 48 additions & 141 deletions

File tree

  • pr-review-toolkit/skills/review-pr

pr-review-toolkit/skills/review-pr/SKILL.md

Lines changed: 48 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -28,21 +28,13 @@ allowed-tools:
2828

2929
## Constraints
3030

31-
Use only the tools listed in `allowed-tools`. Do not generate ad-hoc scripts to
32-
process GitHub data. Workflow return values and MCP responses are structured
33-
JSON; read them directly.
31+
Use only `allowed-tools`. Do not generate ad-hoc processing scripts. Workflow
32+
return values and MCP responses are structured JSON; read them directly. Bash is
33+
limited to the git patterns in `allowed-tools` for preflight and diff steps
34+
below.
3435

35-
Bash access is intentionally limited to the `git fetch`, `git checkout`,
36-
`git rev-parse *`, and `git diff *` patterns listed in `allowed-tools`. Use them
37-
only for the exact local-git preflight and diff commands below. The three git
38-
environment checks above were injected during skill preprocessing and do not
39-
require Bash tool calls.
36+
The workflow and its agents are read-only — they must not call GitHub write tools.
4037

41-
The bundled workflow and workflow-spawned agents are analysis-only. They must
42-
use GitHub read tools only and must not draft pending reviews, add comments,
43-
submit reviews, resolve threads, or call GitHub write tools.
44-
45-
The skill conversation may draft comment text after the user selects findings.
4638
GitHub write tools may be used only after an exact preview and explicit final
4739
posting approval from the user.
4840

@@ -56,172 +48,104 @@ https://github.com/{owner}/{repo}/pull/{number}
5648

5749
## Fetch PR Metadata
5850

59-
Before local git preflight, fetch PR metadata to obtain the head SHA and base
60-
repository information. Call `mcp__plugin_github_github__pull_request_read` with
61-
method `get` for the parsed owner, repo, and pull number.
62-
63-
Extract and record:
51+
Call `pull_request_read` with method `get`. Extract and record:
6452

6553
- `headSha`: the current head commit SHA of the PR
6654
- `changedFiles`: the number of changed files
6755
- base repository clone URL (typically `https://github.com/{owner}/{repo}`)
6856

69-
These values are needed to verify the local git checkout.
70-
7157
## Local Git Preflight
7258

73-
Attempt to set up a verified local git checkout of the PR merge result. If any
74-
check fails, record the failure reason, skip to the Launch Analysis Workflow
75-
section, and pass no `localGitManifest` or `fullDiff` in the workflow args. The
76-
workflow will fall back to MCP-based file collection.
59+
Set up a verified local checkout of the PR merge result. If any check below
60+
fails, record the reason as `fallbackReason`, skip remaining preflight, and
61+
launch the workflow without `localGitManifest` or `fullDiff`.
7762

7863
### Check injected git environment
7964

8065
Read the three values from the Git Environment section above.
8166

82-
1. If repository root is `__NOT_A_GIT_REPO__`, record fallback reason "not a
83-
git repository" and skip to workflow launch.
84-
2. If worktree state is `__WORKTREE_DIRTY__`, record fallback reason "worktree
85-
has uncommitted changes" and skip to workflow launch.
86-
3. If origin URL is `__NO_ORIGIN_REMOTE__`, record fallback reason "no origin
87-
remote" and skip to workflow launch.
88-
4. Compare the origin URL to the PR base repository URL. The origin URL may use
89-
HTTPS (`https://github.com/{owner}/{repo}.git` or
90-
`https://github.com/{owner}/{repo}`) or SSH
91-
(`git@github.com:{owner}/{repo}.git`). Normalize both to `{owner}/{repo}`
92-
for comparison (case-insensitive). If they do not match, record fallback
93-
reason "origin does not match PR base repository" and skip to workflow
94-
launch.
67+
1. `__NOT_A_GIT_REPO__` → "not a git repository"
68+
2. `__WORKTREE_DIRTY__` → "worktree has uncommitted changes"
69+
3. `__NO_ORIGIN_REMOTE__` → "no origin remote"
70+
4. Normalize origin URL and PR base URL to `{owner}/{repo}` (strip protocol,
71+
`.git` suffix; case-insensitive). Mismatch → "origin does not match PR base
72+
repository"
9573

9674
### Fetch merge ref
9775

98-
Run this exact command, substituting the PR number:
99-
10076
```bash
10177
git fetch origin refs/pull/{number}/merge
10278
```
10379

104-
If this fails, the merge ref may not exist (e.g., the PR has merge conflicts or
105-
is closed). Record fallback reason "merge ref fetch failed" and skip to workflow
106-
launch.
80+
Failure → "merge ref fetch failed".
10781

10882
### Checkout merge commit
10983

110-
Run this exact command:
111-
11284
```bash
11385
git checkout --detach FETCH_HEAD
11486
```
11587

116-
This checks out the merge result as a detached HEAD. Do NOT auto-restore the
117-
original ref afterward.
88+
Do NOT auto-restore the original ref afterward.
11889

11990
### Verify merge parents
12091

121-
Run this exact command (all three refs in one call):
122-
12392
```bash
12493
git rev-parse HEAD HEAD^1 HEAD^2
12594
```
12695

127-
This outputs three lines:
128-
129-
- first line = `mergeCommit` (the merge commit SHA)
130-
- second line = `baseSha` (the base branch parent)
131-
- third line = `headSha` (the PR head parent)
132-
133-
Verify that the third line matches the `headSha` from the PR metadata fetched
134-
earlier. If they do not match, record fallback reason "HEAD^2 does not match PR
135-
headSha" and skip to workflow launch. Do NOT trust local diff data when the
136-
merge parents do not match.
96+
Output: line 1 = `mergeCommit`, line 2 = `baseSha`, line 3 = `headSha` (PR
97+
head). If line 3 does not match the PR metadata `headSha`, record "HEAD^2 does
98+
not match PR headSha". Do not trust local diff data on mismatch.
13799

138100
## Build Local Git Manifest
139101

140102
If all preflight checks passed, build the file manifest from local git.
141103

142104
### Collect file statuses
143105

144-
Run this exact command:
145-
146106
```bash
147107
git diff --name-status -z HEAD^1 HEAD
148108
```
149109

150-
Parse the NUL-delimited output into `{path, status}` entries. Map `A`, `M`, `D`,
151-
`R*`, and `C*` to `added`, `modified`, `deleted`, `renamed`, and `copied`. For
152-
renames and copies, use the destination path.
110+
Parse NUL-delimited output into `{path, status}` entries. Map `A`, `M`, `D`,
111+
`R*`, `C*` to `added`, `modified`, `deleted`, `renamed`, `copied`. For renames
112+
and copies, use the destination path.
153113

154114
### Collect line counts
155115

156-
Run this exact command:
157-
158116
```bash
159117
git diff --numstat -z HEAD^1 HEAD
160118
```
161119

162-
Parse the NUL-delimited numstat output and merge it with the status list. Normal
163-
files have additions, deletions, and path. Rename/copy entries include source
164-
and destination paths; use the destination path as the merge key. Binary files
165-
show `-` for additions and deletions; store both as 0. Each manifest entry must
166-
have this shape:
167-
168-
```json
169-
{
170-
"path": "pkg/auth/session.go",
171-
"status": "modified",
172-
"additions": 42,
173-
"deletions": 12
174-
}
175-
```
120+
Parse NUL-delimited numstat and merge with the status list. For renames/copies
121+
use destination path as key. Binary files show `-` for additions/deletions;
122+
store as 0. Each entry has shape: `{path, status, additions, deletions}`.
176123

177124
The resulting array is the `localGitManifest`.
178125

179126
## Collect Full Diff (Optional)
180127

181-
If the local git manifest was built successfully, attempt to collect the full
182-
merge diff.
183-
184-
Run this exact command:
128+
If the manifest was built, collect the full merge diff:
185129

186130
```bash
187131
git diff --no-ext-diff --no-textconv HEAD^1 HEAD
188132
```
189133

190-
If the output is 200,000 characters or fewer, store it as the `fullDiff` string
191-
to pass to the workflow. If the output exceeds 200,000 characters, do not pass
192-
`fullDiff`.
134+
Store as `fullDiff` if 200,000 characters or fewer; otherwise omit.
193135

194136
## Launch Analysis Workflow
195137

196138
Invoke the Workflow tool with:
197139

198140
- `scriptPath`: `${CLAUDE_SKILL_DIR}/review-pr.js`
199-
- `args`:
200-
- `owner`
201-
- `repo`
202-
- `pullNumber`
203-
- `localGitManifest` (array of manifest entries, or omit if preflight failed)
204-
- `fullDiff` (string, or omit if not collected or preflight failed)
205-
- `sources` (object with preflight metadata; the workflow derives
206-
`manifestSource` and `patchSource` from the presence of `localGitManifest`
207-
and `fullDiff`):
208-
- `mergeCommit`: the merge commit SHA (or empty string)
209-
- `baseSha`: the base parent SHA (or empty string)
210-
- `headSha`: the head parent SHA (or empty string)
211-
- `fullDiffIncluded`: true if fullDiff is being passed
212-
- `fallbackReason`: reason preflight failed (or empty string)
213-
214-
If preflight failed, omit `localGitManifest` and `fullDiff`, but still pass
215-
`sources` with empty `mergeCommit`, `baseSha`, and `headSha`,
216-
`fullDiffIncluded: false`, and the preserved `fallbackReason` so the workflow
217-
records why local git was not used.
218-
219-
The workflow owns PR metadata collection (via MCP), reviewer selection,
220-
specialist analysis, and review-board synthesis. It also fetches review threads
221-
(always from MCP).
222-
223-
The workflow returns grouped findings, positive observations, PR metadata,
224-
summary, and review metadata.
141+
- `args`: `owner`, `repo`, `pullNumber`, `localGitManifest` (omit if preflight
142+
failed), `fullDiff` (omit if not collected), and `sources`:
143+
- `mergeCommit`, `baseSha`, `headSha` (empty strings if preflight failed)
144+
- `fullDiffIncluded`: whether fullDiff is included
145+
- `fallbackReason`: reason preflight failed (or empty string)
146+
147+
The workflow collects PR metadata via MCP, runs specialist analysis, fetches
148+
review threads, and returns grouped findings with review metadata.
225149

226150
## Present Review Board
227151

@@ -257,16 +181,11 @@ For each finding, include:
257181

258182
### 3. Related to existing threads (full detail)
259183

260-
For each finding, include the same fields as recommended findings, plus:
261-
262-
- existing review overlap rationale
263-
- recommendation rationale explaining why this is related to an existing thread
264-
rather than standalone
184+
Same fields as recommended, plus existing review overlap rationale.
265185

266186
### 4. Discussion-worthy (full detail)
267187

268-
For each finding, include the same fields as recommended findings, with the
269-
recommendation rationale explaining why this is not recommended to post.
188+
Same fields as recommended; rationale explains why not recommended to post.
270189

271190
### 5. Already covered (one-liner per finding)
272191

@@ -308,11 +227,8 @@ then offer options:
308227
2. "I spotted something"
309228
3. "Done"
310229

311-
### Exploratory actions
312-
313-
There are no dedicated commands. The user can type anything via the Other field,
314-
such as "Tell me more about F3" or "I disagree with F1". Interpret natural
315-
language flexibly, respond accordingly, and loop back to present updated options.
230+
The user may type free-form text via Other (e.g., "Tell me more about F3").
231+
Respond accordingly and loop back to updated options.
316232

317233
## Draft Selected Comments
318234

@@ -325,14 +241,10 @@ Draft comments only in the conversation. Drafts should:
325241
- avoid duplicating comments already covered elsewhere
326242
- distinguish blocking concerns from optional suggestions
327243

328-
### Drafting for overlap findings (relatedToExisting)
329-
330-
For findings in `relatedToExisting`, draft as thread replies rather than
331-
standalone comments. The draft should:
244+
### Overlap findings
332245

333-
- acknowledge the original comment and add the new perspective
334-
- avoid restating the concern from scratch
335-
- read naturally as a reply in the existing conversation
246+
Draft `relatedToExisting` findings as thread replies: acknowledge the original
247+
comment, add the new perspective, and avoid restating the concern.
336248

337249
### Line comments vs review body
338250

@@ -394,16 +306,11 @@ If the approved preview has new line comments:
394306

395307
### Posting thread replies for overlap findings
396308

397-
For findings with `existingReviewOverlap.status === 'overlaps'` and a valid
398-
numeric `existingReviewOverlap.commentId`, post as a reply to the existing
399-
thread using `mcp__plugin_github_github__add_reply_to_pull_request_comment`
400-
with the `commentId` and `pullNumber`.
401-
402-
If the `commentId` is missing or invalid, fall back to posting as a new line
403-
comment via the pending review flow.
404-
405-
Thread replies are posted independently of the pending review flow — they do
406-
not need to be part of the pending review submission.
309+
Post overlapping findings as replies using
310+
`add_reply_to_pull_request_comment` with the numeric `commentId` and
311+
`pullNumber`. If `commentId` is missing or invalid, post as a new line comment
312+
via the pending review flow instead. Thread replies are independent of the
313+
pending review submission.
407314

408315
### Review body only
409316

0 commit comments

Comments
 (0)