fix(source-control): anchor finding-extractor reads to the PR worktree

[kyle-sexton]

Fixes #1300

Summary

• Finding extractor reads are anchored to the PR worktree cwd instead of the orchestrator session cwd

Test plan

• Manual verification of path resolution logic

Related

N/A

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5b98cdea58

ℹ️ 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".

[claude]

Claude finished @kyle-sexton's task in 3m 15s —— View job

---

Reviewing PR #2454

• Gather context (diff, changed file, CLAUDE.md/REVIEW.md)
• Understand the request (code review)
• Review changed file for correctness & alignment with guidelines
• Post findings

Summary

This is a small (+7/-2), docs-only change to plugins/source-control/reference/review-discipline.md §2's finding-extractor subagent dispatch template. It tightens the ALLOWED scope block so subagent reads/git-inspection are pinned to the PR's assigned worktree via absolute paths (<absolute-worktree-path>, git -C <absolute-worktree-path> ...) instead of an ambient/relative cwd.

The core change is correct and consistent with the rest of the codebase: <absolute-worktree-path> is an already-established convention (see babysit-prs/reference/orchestration.md lines ~745-761, which uses identical language — "the absolute worktree path or a <absolute-worktree-path>/… prefix" and git -C <absolute-worktree-path> for every git call). This PR correctly extends that same discipline to the finding-extractor subagent prompt, which previously only said "Read/Grep/Glob against the repo working tree" with no anchor — exactly the gap the PR title describes (reads floating to the orchestrator session cwd instead of the PR worktree).

No functional/security issues found in the diff itself. Two non-blocking suggestions below, both about things adjacent to (not inside) the changed hunk, so I'm not anchoring them as inline diff comments:

1. The "compose verbatim" instruction isn't updated to require substituting the new placeholder.
review-discipline.md#L99-L100 still reads "compose verbatim, substitute <PR> and <COMMENT_ID> / <REVIEW_ID>" — it doesn't mention <absolute-worktree-path> (or the pre-existing <BRANCH>) as required substitutions. Since the whole point of this fix is that the subagent actually gets a real, resolved worktree path, a composer following that line literally could leave <absolute-worktree-path> as a literal placeholder in the dispatched prompt, silently defeating the fix. Worth folding the new placeholder into that substitution list (and <BRANCH> while there, since it has the same gap).

2. No CHANGELOG.md entry / plugin.json version bump.
Current state: plugin.json is at 0.53.9, matching CHANGELOG.md's newest heading — i.e., no pending/unreleased entry. Every prior change to this same file that I found in the changelog (e.g. the 0.53.7 abbreviation-gate fix, 0.53.8 co-author-trailer fix) shipped with a version bump + changelog entry in the same commit. CI's changelog-parity-gate (.github/workflows/ci.yml) only enforces changelog updates when a version is bumped, so this won't fail CI — but it's a deviation from this plugin's consistent practice for changes to review-discipline.md. Worth confirming whether that's intentional (e.g., batching into a later release) or should be added here.

What I did not verify: there's no applicable lint/test suite for this docs-only change, and I did not run one. The Test plan checkbox in the PR description ("Manual verification of path resolution logic") is the author's own claim — I have not independently verified it beyond reading the diff and cross-referencing the existing convention it reuses.

[github-actions]

Claude has reviewed this PR 2 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @kyle-sexton's task in 1m 52s —— View job

---

Reviewing PR #2454

• Gather context (diff, changed file, CLAUDE.md/REVIEW.md)
• Check whether prior review findings (Codex + Claude) were addressed
• Review changed file for correctness & alignment with guidelines
• Post findings

Summary

Small (+7/-2), docs-only change to plugins/source-control/reference/review-discipline.md §2's finding-extractor subagent dispatch template. It tightens the "ALLOWED scope" block so subagent reads/git-inspection are pinned to the PR's assigned worktree via absolute paths (<absolute-worktree-path>, git -C <absolute-worktree-path> ...) instead of an ambient/relative cwd — the core fix is correct and consistent with the existing <absolute-worktree-path> convention used elsewhere (e.g. babysit-prs/reference/orchestration.md).

I confirmed this is still the single commit (2d751e5) from the original push — no follow-up commit has landed since the earlier review pass, so the two issues both Codex and the prior Claude pass flagged are still present in the current diff. I independently re-verified both by reading the file and posted them as inline comments:

1. L111 — the substitution-contract instruction at L99-100 still only lists <PR>/<COMMENT_ID>/<REVIEW_ID> as required substitutions, not the new <absolute-worktree-path> (or pre-existing <BRANCH>). A composer following that instruction literally could leave the placeholder unresolved, defeating the fix.
2. L114-116 — git -C <absolute-worktree-path> is unquoted in the template; a worktree path containing a space would split into multiple shell args and break -C resolution.

Both are suggestion-level (not blocking) since they're gaps adjacent to the fix rather than in the core logic itself, but worth closing since the doc's whole purpose is to prevent exactly this class of "path doesn't actually resolve" drift.

Not re-raised inline (cross-cutting, no changed line to anchor it, already noted by the earlier review pass): this PR carries no CHANGELOG.md entry / plugin.json version bump, which deviates from this file's prior-change pattern (e.g. 0.53.7, 0.53.8 fixes each bumped version + changelog in the same commit). Not CI-blocking, just worth confirming intentional.

What I did not verify: no lint/test suite applies to this docs-only change; the PR's "Manual verification of path resolution logic" Test plan item is the author's own unverified claim, not something I independently confirmed.

[github-actions]

Warning

Automated security review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

• Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/31620213176
• Failure class: other
• Last SDK result: (no execution file was produced)

The check is green on purpose, and it is not evidence. It certifies that a security pass ran, and this one did not complete — but the cause is outside this PR's control, so merging is deliberately left unblocked rather than locking every merge for the length of the outage. Nothing was reviewed at this head. Where this check is required, it is satisfied without that evidence; a human should review security-sensitive changes here before merging.

Re-run the job to retry the review; a new push also retries it only if the caller's pull_request triggers include synchronize (the canonical security caller keeps it). An automatic retry may already have run — it is skipped when a partial review could duplicate comments, or when the failure class needs an operator.

Re-running does NOT help for every class:

• rate-limit that persists across re-runs, or auth — the credential or usage budget needs an operator; retrying will not clear it.
• a run that exhausted its turn budget ("subtype":"error_max_turns" above) will exhaust it again. As the PR author, split the change into smaller PRs; raising --max-turns is a change to the caller workflow, not something you can set on this PR.

[github-actions]

Warning

Automated review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

• Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/31620213177
• Failure class: other
• Last SDK result: (no execution file was produced)

Re-run the job to retry the review. A new push re-triggers this lane only if the caller's pull_request triggers include synchronize (the canonical caller omits it).
An automatic retry may already have run — it is skipped when a partial review could duplicate comments, or when the failure class needs an operator (auth).