docs(skills): file-collision check + goal-prompt size discipline for plan-pr-batch#3914
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR tightens the plan-pr-batch skill: require a verified file-touch map to keep parallel batches file-disjoint, add the map to the Goal Prompt, mandate measuring goal-prompt character count (e.g., ChangesStricter file-disjoint batching safety
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code ReviewSummary: Docs-only change to Strengths
Issues1. Newly-created files are invisible to
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agents/skills/plan-pr-batch/SKILL.md:
- Line 42: Update the guidance text in .agents/skills/plan-pr-batch/SKILL.md
that currently says the preflight+execution-rules boilerplate costs “~900
characters”: change that estimate to ~1084 characters (round to ~1100 chars
reserved) and update the remaining budget statement so the fenced goal prompt
guidance reads that you should keep the goal under 4000 chars reserving ~1100
chars for preflight+execution-rules (leaving ~2916 chars for Worker
notes/items); keep the existing instruction to measure the actual length rather
than eyeballing it and ensure the phrasing still recommends using a
character-count check (e.g., wc -m).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f01a5fe7-6ecc-41d7-b2c1-cfea99bef78e
📒 Files selected for processing (1)
.agents/skills/plan-pr-batch/SKILL.md
Greptile SummaryThis is a focused docs-only update to
Confidence Score: 4/5Safe to merge — docs-only change with no runtime impact; the one finding is a minor inaccuracy in a character-count estimate. The change is narrowly scoped to a single Markdown skill file and introduces no executable code. The only issue found is that the ~900-character boilerplate estimate is already out of date because this PR itself adds a new execution-rules bullet, pushing the real fixed cost to ~1050 characters. Agents following the guidance will still tend to overshoot the 4000-char limit, which undermines the measurement discipline the PR is trying to establish — but this is advisory documentation, so the practical impact is low. .agents/skills/plan-pr-batch/SKILL.md — the boilerplate character estimate should be re-measured against the post-PR template before finalizing. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Intake: collect batch members] --> B[Verify: gh pr/issue view each item]
B --> C[Shape: filter excluded/deferred items]
C --> D["Build file-touch map\n(new step)"]
D --> E{Items share files?}
E -->|No conflicts| F[Parallel first batch\nfile-disjoint items only]
E -->|Collision / dependency| G[Sequenced sub-batch\nor defer to later batch]
F --> H["Output: Batch Plan +\nGoal Prompt for pr-batch"]
G --> H
H --> I["Measure goal prompt\nwith wc -m"]
I --> J{Under 4000 chars?}
J -->|Yes| K[Hand fenced prompt to user\nrun $pr-batch on request]
J -->|No| L[Split into smaller goals\noutput first ready goal only]
Reviews (1): Last reviewed commit: "docs(skills): add file-collision check a..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Code Review — PR #3914docs(skills): file-collision check + goal-prompt size discipline for plan-pr-batch OverviewA focused, well-scoped docs-only update to Correctness IssuesBoilerplate budget discrepancy (line 58) Issue-target new-file gap (line 51) File-touch map template ambiguous for multi-item batches (line 87) Style / ReadabilityThe new bullets on lines 51 and 102 are notably long (~500 and ~450 chars respectively). The surrounding text wraps at 80–100 chars. This is not a correctness issue, but they are harder to scan in editors and diffs. Splitting each into two shorter bullets would match the document's existing style. SummaryThe core logic is correct and the additions close real gaps. The main actionable item before merge is the 2300 vs ~2440 boilerplate budget discrepancy — that is the one number planners will rely on when sizing goal prompts, and having it understated means prompts will routinely slip past the 4000-char cap. |
justin808
force-pushed
the
jg-conductor/plan-pr-batch
branch
from
0f16026 to
b3fd6b7
Compare
Code Review: docs(skills): file-collision check + goal-prompt size discipline for plan-pr-batchSummary: Solid docs-only improvement. Both goals — preventing parallel worktree conflicts and enforcing a measurable prompt-size cap — are real operational problems. The approach is sound. A few issues worth addressing before merge. Issues1. Boilerplate budget number is inconsistent Three different numbers appear across the PR:
The authoritative number (2500) is in the skill itself, but the PR description's conflicting figures (2300 and 2440) undermine confidence in the estimate. This also reveals a tight arithmetic issue: if fixed boilerplate is ~2440 chars and the file-touch map is also inside the goal prompt template, the remaining budget for variable content across 8–10 items is roughly 1,500 chars — about 150–190 chars per item (URL + Goal + Worker notes + Done when). That's achievable with terse writing, but a per-item budget hint (e.g. "target ~150 chars per item") would make the constraint actionable. 2. "Selected owner lane" is undefined in the execution rule Line 102 says: "the selected owner lane may edit its declared files even when a later deferred item will also need them." A worker agent reading this execution rule has no mechanism to determine which lane is the "selected owner" without referring back to the File-touch map. The rule should explicitly direct workers to consult the map: e.g. "the lane listed as owner in the File-touch map for that path". 3. File-touch map template format is ambiguous Line 87 conflates two distinct concepts in a single-line format description:
It is unclear whether deferred paths are a separate footer line, interspersed with item lines, or part of each item's row. A concrete two-or-three-item example (including one deferred entry) would resolve the ambiguity without adding many characters. 4. Shape bullet (line 51) is a wall of text The new file-touch-map bullet is a single 280-word sentence chain. Consider sub-bullets: Minor observations
VerdictThe two functional additions (file-touch map requirement, measured prompt length) are the right calls. The main actionable issue is the undefined "owner" concept in the execution rule — a worker agent cannot reliably follow that rule as currently written. The budget inconsistency and template format ambiguity are lower priority but worth a quick pass before merge. |
justin808
force-pushed
the
jg-conductor/plan-pr-batch
branch
from
b3fd6b7 to
0b07567
Compare
Review: docs(skills): file-collision check + goal-prompt size disciplineDocumentation-only update to Strengths
Issues"Files API" is underspecified (line 51) No fallback for issues with wholly unknown paths (line 51) Boilerplate character-count estimate may already be stale (line 58) Long run-on bullets reduce readability (lines 51, 104) |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agents/skills/plan-pr-batch/SKILL.md:
- Around line 87-89: Summary: The file-touch map is too narrow by documenting
only "touched/created paths" and should instead document a generic "changed
paths" list so delete/rename-only items are included in collision checks. Fix:
update the SKILL documentation text that currently shows "touched/created paths"
to describe and show a generic "changed paths" (or "changed/affected paths")
list used by the Shape step and disjointness check; replace examples and any
bullet entries that reference "touched/created" with the new wording and include
deletes/renames as examples so the disjointness check logic (as described) will
not skip those cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5a23c933-386d-4d93-9981-27cb509f56f5
📒 Files selected for processing (1)
.agents/skills/plan-pr-batch/SKILL.md
justin808
force-pushed
the
jg-conductor/plan-pr-batch
branch
from
0b07567 to
c486b73
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c486b732bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Code Review - PR 3914: plan-pr-batch file-collision + prompt-size disciplineOverall: Well-scoped docs-only improvement. Two substantive findings below. Finding 1: Character budget figure is misleading (line 68) The text says Reserve ~2800 characters for preflight and execution-rules boilerplate. But the PR body itself states the actual fixed boilerplate is ~2440-2465 characters; the 2780/2800 figure is the full template skeleton (boilerplate + placeholder item fields combined). Using 2800 as the boilerplate reservation leaves planners thinking only ~1200 characters remain for item content, when the real headroom is ~1535 characters (4000 - 2465). This risks unnecessary prompt splitting. Suggested fix: say reserve ~2500 chars for boilerplate (matching the actual measured value), or call it template skeleton to match the PR description. Finding 2: gh pr view --json files requires a follow-up extraction (line 54) The files JSON field returns an array of objects, not a flat path list. A planner copy-pasting this command will not get usable paths without a jq extraction step. The already-shown gh pr diff --name-only is simpler and more direct. Consider preferring it, or showing the complete pipeline: gh pr view PR --json files --jq [.files[].path] Minor: File-touch map template line is verbose (lines 97-99) The placeholder PR/Issue #N -> changed/affected paths, including create/delete/rename (owner: lane/name) is instructional prose rather than a compact example. A terse concrete example would serve better: #123 -> lib/foo.rb, app/bar.rb (owner: lane-a) No blocking issues. Finding 1 is the most likely to cause real planner confusion. |
justin808
force-pushed
the
jg-conductor/plan-pr-batch
branch
from
c486b73 to
23b256b
Compare
Code Review: docs(skills): file-collision check + goal-prompt size discipline for plan-pr-batchSummary: Docs-only change to What Works Well
IssuesMinor1. 2. Double-arrow format for deferred paths is ambiguous (line 102) 3. Nested sub-bullets inside the execution rule block (lines 117–125) Not an Issue (but worth noting)The PR body notes Verdict: The changes are correct, well-scoped, and address real coordination risks. The issues above are minor — issue #2 (double-arrow format) carries the most practical risk since an agent misreading the template format could silently misroute deferred paths. |
Code Review —
|
Adversarial PR Review — #3914Scope: Docs-only change to DISCUSS1. PR body Agent Merge Confidence block is stale 2. CodeRabbit review is paused and stale FOLLOWUP3. 4. Redundant NON_BLOCKING_DECISION
OverallNo runtime, security, or compatibility risk. The two DISCUSS items (stale PR body SHA block; stale CodeRabbit summary) should be resolved before merging. The FOLLOWUP items are optional improvements. |
| API response is capped, incomplete, or unavailable. Use the API as the | ||
| scheduling source only when the local diff cannot run. Run the API | ||
| pipeline in one shell invocation with `pipefail` enabled, for example | ||
| `bash -o pipefail -c 'gh api --paginate --method GET "repos/OWNER/REPO/pulls/N/files?per_page=100" | jq -s "add // []"'`; |
There was a problem hiding this comment.
FOLLOWUP — redundant --method GET
gh api defaults to GET; the explicit flag is harmless but adds noise to example commands that agents may copy verbatim.
| `bash -o pipefail -c 'gh api --paginate --method GET "repos/OWNER/REPO/pulls/N/files?per_page=100" | jq -s "add // []"'`; | |
| `bash -o pipefail -c 'gh api --paginate "repos/OWNER/REPO/pulls/N/files?per_page=100" | jq -s "add // []"'`; | |
| ```" | |
| <parameter name="confirmed">true |
Review: docs(skills) — file-collision check + goal-prompt size disciplineOverall this is a solid, security-conscious documentation update. The attention to shell-injection prevention (argument validation, refspec quoting, pipefail), fallback sequencing, and parallel-wave collision avoidance is thorough. A few technical accuracy and robustness notes below. Summary of findings
No blocking issues — the new collision-detection logic and byte-budget discipline are valuable additions. See inline comments for specifics. |
| `headRefName` with a fully qualified source ref | ||
| (`refs/heads/<headRefName>:refs/tmp/pr-N-<session-id>-head`). If | ||
| `headRefName` validation fails or the branch ref is unavailable, validate | ||
| `headRefOid` as the full 40-character lowercase hexadecimal SHA-1 object ID |
There was a problem hiding this comment.
The OID validation pattern ^[0-9a-f]{40}$ is hard-coded for SHA-1 (40 hex chars). GitHub is actively researching SHA-256 object storage (64 hex chars); if the OID length ever changes, this check silently rejects valid OIDs without falling back correctly.
The mitigation caveat ("If GitHub changes the repository hash format, update this validation…") is present, but reactive. A slightly more forward-compatible pattern would be:
| `headRefOid` as the full 40-character lowercase hexadecimal SHA-1 object ID | |
| `headRefOid` as a lowercase hexadecimal SHA object ID GitHub returns | |
| today (40 characters for SHA-1; validate with `^[0-9a-f]{40}$` or | |
| `^[0-9a-f]{40,64}$` to tolerate a future SHA-256 transition), and try an OID fetch |
Using ^[0-9a-f]{40,64}$ accepts both SHA-1 and SHA-256 without any additional maintenance burden, while still rejecting obviously malformed values.
| Treat `baseRefName` and `headRefName` as untrusted shell and refspec data. | ||
| Validate each branch name with Git's branch-name rules using an | ||
| argument-array API equivalent to | ||
| `["git", "check-ref-format", "--branch", baseRefName]` and | ||
| `["git", "check-ref-format", "--branch", headRefName]`, and reject any |
There was a problem hiding this comment.
The explicit : rejection is redundant with git check-ref-format --branch. From the manpage:
Branch names must not contain
:(colon)
So git check-ref-format --branch "$baseRefName" already returns non-zero for any name containing :. Having both checks is harmless but adds noise and could mislead a reader into thinking the colon check catches something check-ref-format would miss.
Suggest removing the : pre-check and keeping only the check-ref-format validation, or at minimum adding a note that the colon check is an early-exit optimisation, not an additional gate.
| is missing | ||
| `.previous_filename`, record the PR paths as `UNKNOWN` and treat the item | ||
| as serial. If the paginated count differs from `changedFiles` in either |
There was a problem hiding this comment.
The re-read to handle "transient metadata lag" has no delay before it fires. GitHub's changedFiles count can lag the actual PR state by a few seconds after a fresh push, so an immediate re-read may still return the stale value.
Consider adding a short wait (e.g. 3–5 s) before the re-read:
If the paginated count differs from `changedFiles` in either direction,
wait 5 s and re-read `changedFiles` once (one retry only) before recording
`UNKNOWN`; the delay gives GitHub time to settle metadata for freshly pushed PRs.
Without any delay, the "one re-read" retry may be a no-op for pushes made within the last few seconds.
| parallel first batch and sequence or defer collisions. A directory rename | ||
| reserves descendants under both the old and new directory names, so any | ||
| create/delete/edit under either tree collides with that rename. An `UNKNOWN` | ||
| item runs as a serial "discovery lane" — a lane that first determines its | ||
| real paths instead of editing in parallel. Never run discovery lanes | ||
| concurrently with active editor lanes. For items already in the scheduling | ||
| set, complete discovery before the editor wave starts. If the coordinator | ||
| adds items after an editor wave has already started, wait for that wave to | ||
| finish before starting discovery for those new items. A collision |
There was a problem hiding this comment.
The scheduling rule says "Never run discovery lanes concurrently with active editor lanes," which clearly prevents a discovery lane from racing an edit. However, it does not say whether multiple UNKNOWN discovery lanes may run concurrently with each other.
If concurrent discovery is allowed (since they haven't identified their paths yet and therefore can't be proven disjoint), the rule should say so explicitly. If it isn't (because discovery itself might write stub files or claim paths), it should say that too.
Suggested addition to clarify:
An `UNKNOWN` item runs as a serial "discovery lane" — a lane that first
determines its real paths instead of editing in parallel. Multiple discovery
lanes may run concurrently with each other (they do not edit files), but
never concurrently with active editor lanes.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cc62859e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| - File-touch map, PR path discovery: get refs from the verified target repo | ||
| with | ||
| `gh pr view N --repo OWNER/REPO --json baseRefName,headRefName,headRepository,headRepositoryOwner`, |
There was a problem hiding this comment.
When the target repo pull ref is unavailable and the head branch cannot be fetched, the later OID fallback depends on headRefOid, but this metadata command does not request it. The gh pr view manual lists headRefOid as a separate --json field, so planners following this flow will not have the value needed for the documented OID fetch and can unnecessarily fall through to API/UNKNOWN handling for deleted or renamed fork branches; include headRefOid here or add an explicit second gh pr view before using it.
Useful? React with 👍 / 👎.
|
|
||
| - File-touch map, PR path discovery: get refs from the verified target repo | ||
| with | ||
| `gh pr view N --repo OWNER/REPO --json baseRefName,headRefName,headRepository,headRepositoryOwner`, |
There was a problem hiding this comment.
headRepositoryOwner is fetched here but never explicitly referenced in any subsequent step — headRepository.nameWithOwner already encodes the owner. Either use headRepositoryOwner in the fallback logic (e.g., to construct the fork clone URL as https://github.com/<headRepositoryOwner.login>/<headRepository.name>.git) or drop it from the JSON query to avoid confusing planners about its intended role.
| repository URL derived from `headRepository.nameWithOwner` and | ||
| `headRefName` with a fully qualified source ref | ||
| (`refs/heads/<headRefName>:refs/tmp/pr-N-<session-id>-head`). If |
There was a problem hiding this comment.
Edge case not covered: if the source fork was deleted, GitHub sets headRepository to null, making headRepository.nameWithOwner a null-dereference. The doc should instruct planners to check headRepository != null before attempting this fallback, and fall through to the OID fetch or PR Files API if it is null.
| `headRefOid` as the full 40-character lowercase hexadecimal SHA-1 object ID | ||
| GitHub returns today, for example `^[0-9a-f]{40}$`, and try an OID fetch | ||
| from the verified head repository URL | ||
| (`<headRefOid>:refs/tmp/pr-N-<session-id>-head`) before using the PR Files | ||
| API fallback. Treat an OID fetch rejection as an expected portability | ||
| outcome on older Git clients or servers that do not advertise reachable SHA | ||
| fetch support, not as a planner setup failure. If GitHub changes the |
There was a problem hiding this comment.
The forward-compatibility note ("If GitHub changes the repository hash format, update this validation") is good but leaves planners with no path forward when they actually encounter a different OID. Consider making this actionable:
If the OID from the API response is 64 lowercase hex characters, validate against
^[0-9a-f]{64}$and proceed with the longer OID — Git's SHA-256 transition uses 64-char hex. Any other length is invalid; fall back to the PR Files API.
This avoids a hard stop the next time a host enables SHA-256 object storage.
| as serial. If the paginated count differs from `changedFiles` in either | ||
| direction, re-read `changedFiles` once before recording `UNKNOWN` so freshly | ||
| pushed PRs do not fail the sanity check on transient metadata lag. If counts | ||
| still diverge after the re-read, record paths as `UNKNOWN` and treat the | ||
| item as serial; note any known submodule or binary-file count mismatch in |
There was a problem hiding this comment.
An immediate re-read of changedFiles can return the same stale value — GitHub's metadata replication lag is typically a few seconds, not sub-second. The guidance should say "wait a few seconds (e.g., sleep 3) before the single re-read" so the retry has a realistic chance of seeing the updated count. Without a delay, the one-shot retry offers little real protection against transient lag.
| item runs as a serial "discovery lane" — a lane that first determines its | ||
| real paths instead of editing in parallel. Never run discovery lanes | ||
| concurrently with active editor lanes. For items already in the scheduling | ||
| set, complete discovery before the editor wave starts. If the coordinator | ||
| adds items after an editor wave has already started, wait for that wave to | ||
| finish before starting discovery for those new items. A collision | ||
| discovered mid-flight cannot safely redirect an active editor lane; the | ||
| coordinator would have to abort the wave, release claims, and restart it, | ||
| which is worse than waiting. |
There was a problem hiding this comment.
"Never run discovery lanes concurrently with active editor lanes" is the right safety rule, but "active" is ambiguous. An editor lane that has already pushed its branch and is waiting for CI/review is no longer touching the worktree, yet it still holds an agent-coord claim. Does it block discovery?
Suggest clarifying: "active" means the lane holds a live agent-coord claim (i.e., has not emitted a terminal heartbeat status like done, failed, or skipped). A lane that has pushed and is only waiting for CI review would typically still hold its claim and therefore still counts as active for serialization purposes.
Code ReviewDocumentation-only change to What's strong here
Issues flagged via inline commentsFive inline comments posted; brief summary:
Minor observations (not blocking)
Overall the design is solid and the security reasoning (untrusted branch names, argument arrays, verified repo URL) is exactly right. The five inline issues are the only things I'd want addressed before merge. |
…c-streaming * origin/main: Keep plan-pr-batch goal prompts under 4000 chars (#4025) docs(skills): file-collision check + goal-prompt size discipline for plan-pr-batch (#3914) Verify React 19.2 <Activity> with react_component (CSR + SSR-hydrate) + docs (#3938) Allow Pro RSC peer check for React 19.2 (#4026)
…e-demo-cost-docs * origin/main: Add RSC FOUC acceptance coverage (#4033) Keep plan-pr-batch goal prompts under 4000 chars (#4025) docs(skills): file-collision check + goal-prompt size discipline for plan-pr-batch (#3914) Verify React 19.2 <Activity> with react_component (CSR + SSR-hydrate) + docs (#3938) Allow Pro RSC peer check for React 19.2 (#4026) Split llms-full.txt into OSS and Pro tiers to clear the 2048 KiB gate (#4021) Codify maintainer attention contract (#3987) Docs: correct vm.Script caching analysis caveats (#3997) CI: handle unavailable PR head repo for +ci-run-full (#3986) Fix Pro dummy lockfile drift and rich text demo (#3989) Add FOUC integration tests for generated CSS (#4005) Make per-PR benchmarks opt-in and trim per-route warm-up (#4012) (#4013) Treat docs-internal/ tree as documentation in CI change detection (#4016) Add issue triage prompt skill (#3983) Point coordination docs at agent-coord bootstrap (#4008) Docs: backfill async RSC manifest changelog entry (#3993) Migrate conductor.json to .conductor/settings.toml (#4007) Bump react-hooks lint to v6 and document RSC compiler boundary (#3963)
* origin/main: Expose React 19 root error callbacks (rootErrorHandlers) + hydration-mismatch debugging guide (#3933) Add post-merge audit scope resolver (#4029) Document continuous agent-run evaluation loop (#4028) Tools: add PR batch merge ledger (#3996) Add RSC FOUC acceptance coverage (#4033) Keep plan-pr-batch goal prompts under 4000 chars (#4025) docs(skills): file-collision check + goal-prompt size discipline for plan-pr-batch (#3914) Verify React 19.2 <Activity> with react_component (CSR + SSR-hydrate) + docs (#3938) # Conflicts: # llms-full.txt
* origin/main: Expose React 19 root error callbacks (rootErrorHandlers) + hydration-mismatch debugging guide (#3933) Add post-merge audit scope resolver (#4029) Document continuous agent-run evaluation loop (#4028) Tools: add PR batch merge ledger (#3996) Add RSC FOUC acceptance coverage (#4033) Keep plan-pr-batch goal prompts under 4000 chars (#4025) docs(skills): file-collision check + goal-prompt size discipline for plan-pr-batch (#3914) Verify React 19.2 <Activity> with react_component (CSR + SSR-hydrate) + docs (#3938) # Conflicts: # CHANGELOG.md # llms-full.txt
…ter-slice * origin/main: Expose React 19 root error callbacks (rootErrorHandlers) + hydration-mismatch debugging guide (#3933) Add post-merge audit scope resolver (#4029) Document continuous agent-run evaluation loop (#4028) Tools: add PR batch merge ledger (#3996) Add RSC FOUC acceptance coverage (#4033) Keep plan-pr-batch goal prompts under 4000 chars (#4025) docs(skills): file-collision check + goal-prompt size discipline for plan-pr-batch (#3914) Verify React 19.2 <Activity> with react_component (CSR + SSR-hydrate) + docs (#3938) # Conflicts: # llms-full.txt
…ce-maps * origin/main: Expose React 19 root error callbacks (rootErrorHandlers) + hydration-mismatch debugging guide (#3933) Add post-merge audit scope resolver (#4029) Document continuous agent-run evaluation loop (#4028) Tools: add PR batch merge ledger (#3996) Add RSC FOUC acceptance coverage (#4033) Keep plan-pr-batch goal prompts under 4000 chars (#4025) docs(skills): file-collision check + goal-prompt size discipline for plan-pr-batch (#3914) Verify React 19.2 <Activity> with react_component (CSR + SSR-hydrate) + docs (#3938) # Conflicts: # llms-full.txt
## Summary - Add a lockfile content-diff handoff contract for workflow/build/dependency/lockfile gate changes. - Require sibling-lock comparison, rationale, and platform/source-build or build-time dependency notes when committed lockfiles change. - Add closing-evidence routing for investigation/benchmark conclusions so `close` and `document/work around` outcomes require reproducible artifacts or explicit caveats. - Keep workflow mirrors aligned while pointing broad gates back to the canonical skill text to avoid future drift. ## Issue Disposition - #3908: implemented process gate for lockfile content-diff evidence in `pr-batch`, `pr-processing`, `post-merge-audit`, and `address-review` triage. - #3910: implemented close-evidence gate in `evaluate-issue`, `pr-processing`, and `post-merge-audit`; workflow entry points either carry the gate at the correct decision point or point back to canonical SKILL text. - #3980/#3997: analysis overlap was removed during rebase. This PR no longer changes `analysis/vm-script-caching-investigation-2026-06-07.md`. - #3982/#3993: changelog backfill remains handled by merged PR #3993. This PR does not touch `CHANGELOG.md`. Refs #3908, #3910. Related #3980/#3997 and #3982/#3993. ## Release/Readiness Evidence Release mode: accelerated-rc from release gate #3823. Current head SHA: `3dc5b426078bd4c2230e8ce4d23a6a44a0934389`. Checks: - Current-head GitHub checks completed for `3dc5b426078bd4c2230e8ce4d23a6a44a0934389`; no failures. - `script/ci-changes-detector origin/main` -> documentation-only; recommended CI jobs: none. Review-thread status: - GraphQL unresolved review threads: 1. Current unresolved thread: #3985 (comment) notes that the `step 4d` label in `.agents/workflows/pr-processing.md` does not exist in the referenced SKILL. Feedback triage: - Rebased onto current `origin/main` after #3914, #3987, #3997, and #3993 merged. - Dropped duplicate vm.Script analysis and CHANGELOG content during rebase; current diff is limited to eight `.agents` skill/workflow files. - `codex review --base origin/main` initially found two P2 over-scope issues in the closing-evidence workflow mirrors; both were fixed in `c8fd3b87b` and `3dc5b4260`. - Final `codex review --base origin/main` -> clean: no discrete correctness, safety, or maintainability issues identified. Validation: - `/Users/justin/.codex/worktrees/7e0e/react_on_rails/node_modules/.bin/prettier --check .agents/skills/address-review/SKILL.md .agents/skills/evaluate-issue/SKILL.md .agents/skills/post-merge-audit/SKILL.md .agents/skills/pr-batch/SKILL.md .agents/workflows/address-review.md .agents/workflows/evaluate-issue.md .agents/workflows/post-merge-audit.md .agents/workflows/pr-processing.md` -> passed. - `PATH="/tmp/lychee-v0.23.0:$PATH" lychee --config .lychee.toml .agents/skills/address-review/SKILL.md .agents/skills/evaluate-issue/SKILL.md .agents/skills/post-merge-audit/SKILL.md .agents/skills/pr-batch/SKILL.md .agents/workflows/address-review.md .agents/workflows/evaluate-issue.md .agents/workflows/post-merge-audit.md .agents/workflows/pr-processing.md` -> 5 links / 5 OK / 0 errors. - `git diff --check origin/main...HEAD` and `git diff --check` -> passed. - Pre-commit/pre-push hooks passed with local toolchain shim: `PATH=/tmp/lychee-v0.23.0:$PATH` and ignored `node_modules/.bin` symlink to the coordinator worktree. Label/CI decision: Labels: none. Docs/process-only; full-ci/benchmark not recommended. Residual risk: - Low-to-medium until the current unresolved review thread is addressed and the resulting head is rechecked. - The semantic risk is workflow wording drift; mitigated by using canonical SKILL pointers and a clean local Codex review before the new thread arrived. Merge recommendation: keep draft until the current unresolved review thread is addressed. Checks are green for the current head, but the PR should not be undrafted or merged while that thread remains open.
1 participant
What
Two focused improvements to the
plan-pr-batchskill, updated on currentmainafter #3977's coordination workflow landed.UNKNOWN, and treat unknown/colliding paths as serial.wc -c, documentedwc -mlocale 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 --checkandgit diff --check origin/main...HEAD-> passed.script/ci-changes-detector origin/main-> documentation-only, no CI jobs recommended.4cc62859e39c42ef62381ba3d20ebaa252b5a74d.4cc62859e39c42ef62381ba3d20ebaa252b5a74dwith repo-compatible lychee onPATH.Review/check gate:
4cc62859e39c42ef62381ba3d20ebaa252b5a74d; 12 pass / 2 skipped / 0 pending / 0 failed.claudeworkflow rows skipped after theclaude-reviewcheck completed successfully;docs-format-checkskipped by the Lint JS and Ruby selector whiledetect-changes,build, markdown checks, and CodeQL all passed.4cc62859e39c42ef62381ba3d20ebaa252b5a74d.claude-reviewcompleted SUCCESS for current head; Cursor Bugbot and CodeRabbit are successful/advisory.Feedback triage:
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 head4cc62859e39c42ef62381ba3d20ebaa252b5a74din run27513994273.Current Readiness
Merge-ready for current head
4cc62859e39c42ef62381ba3d20ebaa252b5a74dafter final live recheck: checks complete, merge state CLEAN, unresolved review-thread count 0, and no known blocker remains.Note
Low Risk
Documentation-only internal agent skill update; no runtime, auth, or application code changes.
Overview
plan-pr-batchnow requires a file-touch map before parallel fan-out, with PR paths from verified-repogit fetch+ three-dot rename-aware diffs (session-uniquerefs/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, withUNKNOWNforcing 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 andpr-batchgoal template (owned paths only, disjoint waves). Common mistakes add no path guessing, no same-path parallel worktrees, and no eyeballing prompt length.Reviewed by Cursor Bugbot for commit 4cc6285. Bugbot is set up for automated code reviews on this repo. Configure here.