Skip to content

fix(source-control): run ready's verify gate against the PR's worktree - #10521

Merged
melodic-automation-lanes[bot] merged 2 commits into
mainfrom
work-resolve-issue/issue-9075-40572879795-1
Oct 27, 2026
Merged

melodic-automation-lanes[bot] merged 2 commits into
mainfrom
work-resolve-issue/issue-9075-40572879795-1

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Closes #9075

Summary

Opened by the work-resolve-issue lane. The run's own summary:

Ready's verify gate ran scripts/check-affected.sh relative to the shell's directory and recorded the shell's HEAD. So a session in the main checkout running ready <pr> for a PR built in another worktree checked the main checkout. A new scripts/pr-worktree.sh finds the worktree that has the PR's head branch checked out with the PR's head commit in its history. The gate now runs that tree's check-affected.sh and records that tree's HEAD, and the flip's HEAD_SHA comes from the same worktree. When no such worktree exists, nothing is recorded and the flip refuses.

Not verified

  • pr-worktree.test.sh and ready-flip.test.sh were not run: the lane allows no interpreter, and its check-affected wrapper runs without --tests. Both were traced by hand, and the review subagent also traced them; the PR's CI test lane runs them.
  • Finding outside this issue's scope, already present before this change: 2.5.2's base refresh (the provider's git fetch "$REMOTE" "$BRANCH" && git merge --ff-only FETCH_HEAD and the local-merge fallback) runs in the shell's checkout with BRANCH from git branch --show-current. So a ready r
  • The new pr-worktree.sh was created without the executable bit (the lane cannot chmod). The lane's patch apply marks new .sh files executable, as CHECK_AFFECTED_SCRIPT_EXEC_BIT=1 models, and the suites call it with bash.
  • typos was not run (not installed in the lane); CI runs it.

Fix

  • plugins/source-control/skills/pull-request/scripts/pr-worktree.sh: new. --pr <n> reads headRefName and headRefOid with gh and prints the worktree from git worktree list --porcelain that is on that branch and whose HEAD contains the head commit. Exit 2 when the read fails, no worktree has t
  • plugins/source-control/skills/pull-request/reference/ready-for-review.md: 2.5.3 defines the PR worktree and what happens when the shell is already in it or the branch is checked out nowhere. The gate command resolves PR_TREE, runs "$PR_TREE/scripts/check-affected.sh" --tests and records `git -C "$PR
  • plugins/source-control/reference/providers/github/pull-request/ready-for-review.md: in the 'Wait for the push's CI run' block, HEAD_SHA is git -C "$PR_TREE" rev-parse HEAD, with PR_TREE from pr-worktree.sh; the prose is updated to match.
  • plugins/source-control/skills/pull-request/scripts/pr-worktree.test.sh: new suite. It covers resolution from the main checkout and from inside the PR worktree, a worktree ahead of the PR head, a branch checked out nowhere, a same-named branch without the head commit, failed, empty or malformed reads
  • plugins/source-control/skills/pull-request/scripts/ready-flip.test.sh: the gh stub answers pr-worktree.sh's head read. New cases run the wait-and-flip flow from another checkout (HEAD_SHA is the PR worktree's HEAD and the flip passes) and refuse the flip when the head branch is checked out nowhere o
  • .changes/source-control/fix-9075-ready-verify-pr-worktree-5c2e8a17.md: patch changelog fragment for source-control, written by hand because new-changelog-fragment refuses on a detached HEAD.

Verification

The runner ran each check from the base copy against a3cd124:

  • scripts/check-changelog-fragments.sh --check: passed

The run reports:

  • .lane/checks/bin/check-changelog-fragments: all 7 fragments under .changes/ are valid
  • .lane/checks/bin/check-affected (lane wrapper, --uncommitted, no --tests): 12 passed, 6 skipped (typos not installed, ruff had no .py files, and changed-skills plus 3 validate-plugins parts skip because they run the checked tree's own code), 0 failed, over 6 changed files
  • .lane/verify/bin/cant-fail-scan --file plugins/source-control/skills/pull-request/scripts/pr-worktree.test.sh --check: PASS, no gating findings
  • .lane/verify/bin/cant-fail-scan --file plugins/source-control/skills/pull-request/scripts/ready-flip.test.sh --check: PASS, no gating findings
  • fresh-context review subagent over the whole diff: 3 findings. Fixed: the skipped/fail records had no command that set PR_TREE (an empty git -C would have recorded the shell's HEAD). Fixed: a name-only match could pick a fork's same-named branch, so pr-worktree.sh now also requires the head commit
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/40572879795

Left out: the items under Not verified.

Related

#9075

Lane-Activity: work-resolve-issue/resolve-issue
@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 15a4ecbd2077e2c692ba9a96086fa5da3e0bae1a: 0 finding(s), 0 as inline comments.

No security issues in the incremental change (since a3cd124).

  • ready-for-review.md: the gate command now runs CHECK_DATA_ROOT="$PR_TREE" bash "$PR_TREE/scripts/check-affected.sh" --tests. PR_TREE is quoted. It comes from pr-worktree.sh, and the command exits if that fails. CHECK_DATA_ROOT is an existing variable that scripts/check-affected.sh already reads, so no new input surface appears. The change pins the checked tree to the PR's worktree and widens no trust control.
  • pr-worktree.test.sh: only the test stub changed. It now takes its tree from CHECK_DATA_ROOT or the working directory, so the gate assertions fail unless the gate names the PR's tree. The stub runs in a temporary directory and touches no secret or trust boundary.
  • No trusted-path file changed in this increment. No manifest, instruction-surface or hold-path change.
  • Spend: none.

critical-findings: 0

@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 15a4ecbd2077e2c692ba9a96086fa5da3e0bae1a: 0 finding(s), 0 as inline comments.

Incremental review of the change since a3cd124 (ready-for-review.md and pr-worktree.test.sh). I found nothing to flag.

  • Gate command (ready-for-review.md): the command now sets CHECK_DATA_ROOT="$PR_TREE" and still runs $PR_TREE/scripts/check-affected.sh. I read scripts/check-affected.sh lines 66-120. It resolves top from CHECK_DATA_ROOT when that is set, else from its own location. Here both name the PR tree, so the script runs in its normal mode, not the copy mode where the two trees differ. A PR branch whose script predates CHECK_DATA_ROOT support would still check its own tree, because it resolves from its own location.
  • Test stub (pr-worktree.test.sh): the stub now takes the checked tree from CHECK_DATA_ROOT, else from $PWD, never from its own path. The gate assertions (checked $PR_WT_REAL, changed: feature.md, the pass recorded for $PR_HEAD) now fail if the gate stops naming the PR tree. The earlier stub resolved its tree from the script's own location, which the PR worktree's copy always matched, so those assertions could not fail. The GATE extraction greps still match the edited line (pr-worktree.sh" --pr and check-affected.sh" --tests), so the test still finds the command.
  • Closing issue fix(source-control): ready's verify gate checks the shell's checkout, not the PR's worktree #9075: this change meets both acceptance criteria, which are a gate output that lists the PR's changed files and a recorded head equal to the PR worktree's HEAD. It also uses the CHECK_DATA_ROOT option the issue names, now that fix(repo): make check-affected list a branch's changes from any directory and fail on an empty list #9072 has landed. The new suite exercises both criteria from the main checkout.
  • Not run: I ran none of the test suites. This review is by reading the diff and the called script only.
  • Spend: no billing or paid-plan change.

@melodic-automation-lanes

melodic-automation-lanes Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head 15a4ecbd2077e2c692ba9a96086fa5da3e0bae1a. The lanes clear this reason on their own; the next scheduled run judges it again.

pr-merge's gate refuses its verifier verdict (verify:earlier-fail)

Comment thread plugins/source-control/skills/pull-request/scripts/pr-worktree.sh
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

pr-unblock / address-feedback finished round 1 of max 4 on 15a4ecb.

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 27, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The pr-unblock lane stopped on this pull request and needs a person: a review finding is uncertain or deferred, so a person decides it.

  • Reason: feedback-uncertain
  • Resume key: 15a4ecbd2077e2c692ba9a96086fa5da3e0bae1a (the head the lane stopped on)

To resume, deal with the reason above, remove the needs-human label (and do-not-merge, if set), then dispatch the pr-unblock workflow with pr-number=10521 and activity=address-feedback:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=10521 -f activity=address-feedback

@kyle-sexton kyle-sexton removed the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 27, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The unattended merge path armed native auto-merge at head 15a4ecbd2077e2c692ba9a96086fa5da3e0bae1a.

@melodic-automation-lanes
melodic-automation-lanes Bot added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit 23f7831 Oct 27, 2026
84 of 87 checks passed
@melodic-automation-lanes
melodic-automation-lanes Bot deleted the work-resolve-issue/issue-9075-40572879795-1 branch October 27, 2026 22:43
@github-actions

Copy link
Copy Markdown
Contributor

Post-merge mutation testing

Killed 7, survived 1, timeout 0, no coverage 0, invalid 0, not run 0.
Covered-code score 88%; mutation score 88%.

File Killed Survived Timeout No coverage Invalid Not run
plugins/source-control/skills/pull-request/scripts/pr-worktree.sh 7 1 0 0 0 0

Surviving mutants

Location Operator Disposition Cause
plugins/source-control/skills/pull-request/scripts/pr-worktree.sh:52 LCR productive input-gap

The run's summary has the full report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(source-control): ready's verify gate checks the shell's checkout, not the PR's worktree

1 participant