Skip to content

Commit 5abe360

Browse files
justin808claude
andauthored
docs(skills): file-collision check + goal-prompt size discipline for plan-pr-batch (#3914)
## What Two focused improvements to the `plan-pr-batch` skill, updated on current `main` after #3977's coordination workflow landed. 1. Require a file-touch map before fanning out parallel work. PR targets use verified target-repo refs, session-unique temporary refs, rename-aware changed-file lists, and a paginated PR Files API fallback; issue targets record proposed paths from issue/design notes, mark unknown paths as `UNKNOWN`, and treat unknown/colliding paths as serial. 2. Make the goal-prompt budget measurable as an approximate byte budget with `wc -c`, documented `wc -m` locale caveats, measured-overhead guidance, terse per-item guidance, and an explicit Batch Plan path-evidence slot. ## Scope Docs-only change to `.agents/skills/plan-pr-batch/SKILL.md`. No CHANGELOG entry because this is internal agent tooling. This does not touch #3963-reserved docs areas. ## Agent Merge Confidence Mode: accelerated-rc (#3823) Current head SHA: 4cc6285 Score: 9/10 Auto-merge recommendation: yes Affected areas: plan-pr-batch skill/process docs CI detector: `script/ci-changes-detector origin/main` -> documentation-only changes; recommended CI jobs none. Validation run: - `/Users/justin/.codex/worktrees/7e0e/react_on_rails/node_modules/.bin/prettier --check .agents/skills/plan-pr-batch/SKILL.md` -> passed. - `PATH=/tmp/lychee-v0.23.0:/Users/justin/.codex/worktrees/7e0e/react_on_rails/node_modules/.bin:$PATH lychee --config .lychee.toml .agents/skills/plan-pr-batch/SKILL.md` -> passed. - `git diff --check` and `git diff --check origin/main...HEAD` -> passed. - `script/ci-changes-detector origin/main` -> documentation-only, no CI jobs recommended. - Pre-commit hooks passed through current head `4cc62859e39c42ef62381ba3d20ebaa252b5a74d`. - Pre-push hooks passed for current head `4cc62859e39c42ef62381ba3d20ebaa252b5a74d` with repo-compatible lychee on `PATH`. Review/check gate: - GitHub checks: complete for current head `4cc62859e39c42ef62381ba3d20ebaa252b5a74d`; 12 pass / 2 skipped / 0 pending / 0 failed. - Skips: `claude` workflow rows skipped after the `claude-review` check completed successfully; `docs-format-check` skipped by the Lint JS and Ruby selector while `detect-changes`, `build`, markdown checks, and CodeQL all passed. - Review threads: GraphQL unresolved count is 0 for current head `4cc62859e39c42ef62381ba3d20ebaa252b5a74d`. - Current-head reviewer verdicts: `claude-review` completed SUCCESS for current head; Cursor Bugbot and CodeRabbit are successful/advisory. Feedback triage: - Current-head review threads are resolved or triaged; no unresolved review debt remains. - Earlier review fixes added verified base/head fetch guidance, temporary-ref cleanup behavior, cross-fork ref safety, Files API fallback guards, discovery-wave sequencing, shell quoting guidance, and prompt-size measurement corrections. Labels: none. Documentation-only process update; path detector recommends no CI expansion. Known residual risk: Low; this is internal docs/process guidance, with no runtime or release artifact changes. Finalized by: GitHub `claude-review` / Claude Code Review check, completed SUCCESS for current head `4cc62859e39c42ef62381ba3d20ebaa252b5a74d` in run `27513994273`. ## Current Readiness Merge-ready for current head `4cc62859e39c42ef62381ba3d20ebaa252b5a74d` after final live recheck: checks complete, merge state CLEAN, unresolved review-thread count 0, and no known blocker remains. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only internal agent skill update; no runtime, auth, or application code changes. > > **Overview** > **`plan-pr-batch` now requires a file-touch map before parallel fan-out**, with PR paths from verified-repo `git fetch` + three-dot rename-aware diffs (session-unique `refs/tmp/` refs, branch/OID validation, shallow deepen, cleanup) and a hardened paginated PR Files API fallback when git cannot run. Issues get paths from issue/design notes plus repo grep, with **`UNKNOWN` forcing serial discovery lanes** and collision-aware wave scheduling so only file-disjoint items run in parallel. > > **Output and handoff tighten**: goal prompts use a **~4000-byte budget measured with `wc -c`**, measured template overhead, terse per-item fields, path evidence in the Batch Plan with compressed but collision-safe summaries in the prompt, and new **File-touch map** sections in the Batch Plan format and `pr-batch` goal template (owned paths only, disjoint waves). Common mistakes add no path guessing, no same-path parallel worktrees, and no eyeballing prompt length. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 4cc6285. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 3f78fe0 commit 5abe360

1 file changed

Lines changed: 150 additions & 3 deletions

File tree

.agents/skills/plan-pr-batch/SKILL.md

Lines changed: 150 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,144 @@ Plan a PR batch
5050
Coordinators must create or update the private backend
5151
`batches/<batch-id>.json` with those lane refs before dependent workers
5252
start; otherwise `agent-coord status` cannot report `blocked_on` lanes.
53+
- Build a File-touch map for the batch: list the paths each item changes or
54+
intends to affect, including creates, deletes, and renames. Never guess
55+
paths.
56+
57+
- File-touch map, PR path discovery: get refs from the verified target repo
58+
with
59+
`gh pr view N --repo OWNER/REPO --json baseRefName,headRefName,headRepository,headRepositoryOwner`,
60+
then resolve a local remote or fetch URL that points at the verified base
61+
repo. If no verified remote or URL can be resolved, use the PR Files API
62+
fallback before recording paths as `UNKNOWN`; do not diff the current
63+
checkout's default remote. Choose a session-unique temporary-ref suffix
64+
first, such as `pr-N-<session-id>` where `<session-id>` comes from
65+
`openssl rand -hex 4` or another git-ref-safe random token, so concurrent
66+
planners for the same PR cannot overwrite each other's refs.
67+
Treat `baseRefName` and `headRefName` as untrusted shell and refspec data.
68+
Validate each branch name with Git's branch-name rules using an
69+
argument-array API equivalent to
70+
`["git", "check-ref-format", "--branch", baseRefName]` and
71+
`["git", "check-ref-format", "--branch", headRefName]`, and reject any
72+
name containing `:` before constructing a refspec. Pass the branch name as a
73+
single command argument instead of interpolating it into a shell string. If
74+
base branch validation fails, fall back to the PR Files API or `UNKNOWN`;
75+
do not sanitize a failing branch name. Fetch the current base branch and PR
76+
head into temporary refs without checking out untrusted PR code:
77+
`git fetch <verified-base-repo-url> refs/heads/<baseRefName>:refs/tmp/pr-N-<session-id>-base`
78+
and
79+
`git fetch <verified-base-repo-url> pull/N/head:refs/tmp/pr-N-<session-id>-head`.
80+
Fully qualifying the base branch avoids tag/branch name ambiguity.
81+
GitHub keeps the target repo's pull ref pointing at fork heads too. If the
82+
target repo pull ref is unavailable, fetch the head from the verified head
83+
repository URL derived from `headRepository.nameWithOwner` and
84+
`headRefName` with a fully qualified source ref
85+
(`refs/heads/<headRefName>:refs/tmp/pr-N-<session-id>-head`). If
86+
`headRefName` validation fails or the branch ref is unavailable, validate
87+
`headRefOid` as the full 40-character lowercase hexadecimal SHA-1 object ID
88+
GitHub returns today, for example `^[0-9a-f]{40}$`, and try an OID fetch
89+
from the verified head repository URL
90+
(`<headRefOid>:refs/tmp/pr-N-<session-id>-head`) before using the PR Files
91+
API fallback. Treat an OID fetch rejection as an expected portability
92+
outcome on older Git clients or servers that do not advertise reachable SHA
93+
fetch support, not as a planner setup failure. If GitHub changes the
94+
repository hash format, update this validation before accepting a different
95+
OID length. Pass each refspec as one quoted shell argument or via an
96+
argument-array API, and never interpolate a raw PR branch name into a shell
97+
command. A plain
98+
`git fetch origin` does not fetch cross-fork heads unless `origin` has
99+
already been verified as the PR's target repo.
100+
Run
101+
`git diff --name-status --find-renames refs/tmp/pr-N-<session-id>-base...refs/tmp/pr-N-<session-id>-head`;
102+
three-dot diffs from the merge-base, which matches GitHub's PR file list.
103+
If the three-dot diff fails because the merge base is missing in a shallow
104+
clone, run a bounded deepen for the relevant base branch such as
105+
`git fetch --deepen=200 <verified-base-repo-url> refs/heads/<baseRefName>`
106+
and retry the same
107+
`git diff --name-status --find-renames refs/tmp/pr-N-<session-id>-base...refs/tmp/pr-N-<session-id>-head`
108+
command once before falling back to the PR Files API; the deepen value is a
109+
best-effort heuristic, not a guarantee.
110+
Delete the temporary refs on both success and failure, then proceed to the
111+
API fallback or `UNKNOWN` decision:
112+
`git update-ref -d refs/tmp/pr-N-<session-id>-base` and
113+
`git update-ref -d refs/tmp/pr-N-<session-id>-head`. If cleanup fails, log
114+
the ref name and continue to fallback or `UNKNOWN` recording. If repeated
115+
cleanup failures leave stale `refs/tmp/pr-*` refs, run a periodic sweep with
116+
`git for-each-ref refs/tmp/ --format='%(refname)'` and delete only stale
117+
planner-owned refs with `git update-ref -d "$ref"`. A rename row
118+
(`R100 old new`) owns **both** the old and new path; a directory rename
119+
implicitly reserves descendants under both old and new directory names. If
120+
a ref cannot be fetched or the diff cannot run, try the PR Files API
121+
fallback before marking the paths `UNKNOWN`.
122+
- File-touch map, PR Files API fallback: prefer the local `git diff` above
123+
as the authoritative source; treat the API as a best-effort cross-check.
124+
When the local diff succeeds, keep those paths authoritative even if the
125+
API response is capped, incomplete, or unavailable. Use the API as the
126+
scheduling source only when the local diff cannot run. Run the API
127+
pipeline in one shell invocation with `pipefail` enabled, for example
128+
`bash -o pipefail -c 'gh api --paginate --method GET "repos/OWNER/REPO/pulls/N/files?per_page=100" | jq -s "add // []"'`;
129+
if either command fails, the response is an error-shaped object such as
130+
`{"message": ...}` / `{"errors": ...}`, or any row lacks `.filename`, record
131+
the paths as `UNKNOWN` instead of trusting an empty array from a broken
132+
pipeline. Do not confuse API/auth/rate-limit failures with a real empty PR
133+
file list.
134+
The default page size is 30, so a small unpaginated page can look complete
135+
while truncated. `jq -s 'add // []'` collects all paginated arrays before
136+
counting paths or extracting filenames and returns an empty array for an
137+
empty stream; no Link header check is needed after the command returns.
138+
The response is acceptable only when every row records `.filename`, every
139+
row with `status: "renamed"` records `.previous_filename`, and the
140+
listed-file count is sane against the PR's `changedFiles` value from
141+
`gh pr view N --repo OWNER/REPO --json changedFiles`. GitHub caps the
142+
Files API at ~3000 files; if the API is the only available source and
143+
`changedFiles` is at or above that cap, or any row with `status: "renamed"`
144+
is missing
145+
`.previous_filename`, record the PR paths as `UNKNOWN` and treat the item
146+
as serial. If the paginated count differs from `changedFiles` in either
147+
direction, re-read `changedFiles` once before recording `UNKNOWN` so freshly
148+
pushed PRs do not fail the sanity check on transient metadata lag. If counts
149+
still diverge after the re-read, record paths as `UNKNOWN` and treat the
150+
item as serial; note any known submodule or binary-file count mismatch in
151+
the Batch Plan instead of silently trusting an incomplete API path list.
152+
- File-touch map, issue path discovery: read the issue body, record proposed
153+
new paths from issue/design notes, and grep the repo to confirm existing
154+
paths. If paths cannot be determined from the issue body or design notes,
155+
record them as `UNKNOWN` and treat the item as serial.
156+
- File-touch map, collision and wave scheduling: items that affect the same
157+
path cannot run as parallel worktrees; keep only file-disjoint items in the
158+
parallel first batch and sequence or defer collisions. A directory rename
159+
reserves descendants under both the old and new directory names, so any
160+
create/delete/edit under either tree collides with that rename. An `UNKNOWN`
161+
item runs as a serial "discovery lane" — a lane that first determines its
162+
real paths instead of editing in parallel. Never run discovery lanes
163+
concurrently with active editor lanes. For items already in the scheduling
164+
set, complete discovery before the editor wave starts. If the coordinator
165+
adds items after an editor wave has already started, wait for that wave to
166+
finish before starting discovery for those new items. A collision
167+
discovered mid-flight cannot safely redirect an active editor lane; the
168+
coordinator would have to abort the wave, release claims, and restart it,
169+
which is worse than waiting.
53170
- Cap at 8 with shared/risky files, else 10 independent items; propose a smaller first batch.
54171
- For PRs with review feedback, route the worker to use the repo review workflow before code changes.
55172
- For issues, define the expected deliverable: fix, investigation, reproduction, docs update, or no-PR audit.
56173

57174
4. Output
58175
- Return a concise "Batch Plan" and a fenced "Goal Prompt for pr-batch".
59-
- Keep the fenced goal prompt under 4000 characters total so bulky audit detail stays in the Batch Plan.
176+
- Keep the fenced goal prompt under ~4000 bytes total so bulky audit detail stays in the Batch Plan. Measure it, do not eyeball it: `wc -c` gives a locale-independent byte count (`wc -m` counts characters using the current locale's multibyte rules — UTF-8 gives code points, C/POSIX gives bytes). Treat 4000 as an approximate budget.
177+
- Measure the actual filled template overhead when the prompt is near the
178+
byte budget; do not rely on a fixed estimate. Prefer splitting into
179+
multiple goals over trimming the safety, ownership, or review content.
180+
- Keep full path evidence in the Batch Plan when it would bloat the prompt,
181+
but do not leave the worker handoff with an external-only pointer. In the
182+
goal prompt, use the narrowest unambiguous directory/pattern summary that
183+
still proves ownership, and include any exceptions, renames, deletes, or
184+
collision-relevant exact paths inline. If compression would hide a collision
185+
or make ownership unclear, mark the item `UNKNOWN` and run it serially.
186+
- Keep each filled entry terse (target ~150 chars for `Worker notes` and `Done when`). The worker reads the issue/PR URL for full detail; push evidence and audit notes to the Batch Plan instead.
60187
- If the batch will not fit, split it into smaller goals and output only the first ready goal.
61-
- Do not start `$pr-batch` unless the user asks; then hand them the fenced goal prompt and tell them to run `$pr-batch` with it.
188+
- Do not start `$pr-batch` unless the user asks; then hand them the fenced
189+
goal prompt and any Batch Plan path appendix that the prompt explicitly
190+
depends on, in the same request.
62191

63192
## Batch Plan Format
64193

@@ -67,6 +196,7 @@ Plan a PR batch
67196
- Included items:
68197
- `PR #N` or `Issue #N`: title, URL, state, role in batch
69198
- Excluded or deferred:
199+
- File-touch map and path evidence:
70200
- Dependencies and sequencing:
71201
- Subagent split:
72202
- Concurrent activity and dependency status:
@@ -85,6 +215,12 @@ Preflight first: if this session cannot run workers without blocking approval pr
85215
86216
Repository: OWNER/REPO
87217
Batch objective: ...
218+
File-touch map (one line per item; pick the applicable format):
219+
- PR/Issue #N -> changed/affected paths, including create/delete/rename (owner: lane/name)
220+
- PR/Issue #N -> summarized path pattern(s) plus collision-relevant exact paths/renames/deletes (owner: lane/name)
221+
- PR/Issue #N -> UNKNOWN (paths not determinable from issue body/design notes; treat as serial)
222+
Batch-level reservations, not tied to a single item:
223+
- Deferred/reserved paths -> path(s) (reason: ... / later owner: lane/name)
88224
89225
Items:
90226
- PR #N: URL
@@ -98,7 +234,14 @@ Items:
98234
99235
Execution rules:
100236
- Follow `.agents/skills/pr-batch/SKILL.md` "Goal Prompt Template"; if skill autoloading is unavailable, copy its safety, review, /simplify, CI, and readiness gates before running.
101-
- Dispatch one subagent per independent item; group dependent items only when shared context is required.
237+
- Dispatch only the current file-disjoint wave. Hold serial and `UNKNOWN`
238+
discovery lanes until no active editor lane can collide with them.
239+
- Workers edit only owned File-touch map paths; this map is how the batch makes
240+
pr-batch's "disjoint write scopes" concrete, since pr-batch's own template has
241+
no File-touch map slot. If an `UNKNOWN`, unlisted, or other-lane path is
242+
needed, stop, report discovered paths, and wait for an updated map or explicit
243+
coordinator confirmation before editing.
244+
- Sequenced lanes may share declared files only in the stated order.
102245
- Each subagent must verify current GitHub state before edits and report UNKNOWN for unverifiable facts.
103246
- For concurrent or dependency-sensitive batches, assign a stable agent id and
104247
lane name per lane. Declare lane dependencies with `depends_on` refs such as
@@ -124,5 +267,9 @@ Execution rules:
124267
- Do not infer PR vs issue from a bare number.
125268
- Do not batch unrelated risky changes just because they are small.
126269
- Do not hide missing GitHub data; say `UNKNOWN`.
270+
- Do not guess file paths; record unverifiable paths as `UNKNOWN` and treat that
271+
item as serial.
127272
- Do not omit links; use GitHub URLs for every item.
128273
- Do not put full audit evidence in the goal prompt; put bulky details in the Batch Plan outside the goal.
274+
- Do not fan out items that change the same path as parallel worktrees; they will conflict — sequence them or split into a later batch.
275+
- Do not eyeball the goal-prompt length; apply the Output-section size gate and split into smaller goals if it is over budget.

0 commit comments

Comments
 (0)