From bbbc82fe6454b865754b9575e0fc59d88a83c7d1 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 19:56:58 -0700 Subject: [PATCH 01/11] Fix Skill-Cleanup Ordering, Quoting, and Ignored-Path Gaps Four real CodeRabbit findings on PR 936, all confirmed: - The merge-and-release preflight's 'git status --porcelain' misses gitignored paths, while skills_install.py's shutil.copytree() has no ignore filter, so a stray gitignored file could pass the preflight and still ride into the install. Widened to --untracked-files=all --ignored. - Cleanup fast-forwarded the base clone to develop before removing the promotion worktree, backwards: verified empirically that git refuses to check out a branch already checked out in another worktree. Reordered. - git worktree remove appeared with no path argument in the defensive sweep, git requires one. Named it explicitly. - Branch names appeared unquoted across six cleanup commands in both drive-pr and merge-and-release, a valid ref can start with - or carry a shell metacharacter. Quoted throughout, with -- where it helps. Co-Authored-By: Claude Sonnet 5 --- .agents/skills/drive-pr/SKILL.md | 12 +++--- .agents/skills/merge-and-release/SKILL.md | 43 +++++++++++-------- .claude-plugin/fleet-skills/.source-digest | 2 +- .../fleet-skills/skills/drive-pr/SKILL.md | 12 +++--- .../skills/merge-and-release/SKILL.md | 43 +++++++++++-------- .github/skills/drive-pr/SKILL.md | 12 +++--- .github/skills/merge-and-release/SKILL.md | 43 +++++++++++-------- 7 files changed, 97 insertions(+), 70 deletions(-) diff --git a/.agents/skills/drive-pr/SKILL.md b/.agents/skills/drive-pr/SKILL.md index 37daa859..e1b743b2 100644 --- a/.agents/skills/drive-pr/SKILL.md +++ b/.agents/skills/drive-pr/SKILL.md @@ -67,11 +67,13 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. to delete it, which fails when `develop` is already checked out somewhere else, the ordinary case in this layout. Instead run repo-worktree's post-merge cleanup from the base clone: remove the worktree, delete the now-merged local task branch, then verify before deleting the remote - one, `git ls-remote --heads origin ` matches the `headRefOid` captured above, stop and - report a mismatch rather than deleting, someone could have pushed to the branch after the - merge, or the name could have been reused. Only once it matches, `git push origin --delete - `. Never `--force-with-lease` here, git-commit-conventions forbids it unconditionally, - this plain verify-then-delete is the safety gate, not a compare-and-swap at delete time. The + one, `git ls-remote --heads origin ""` matches the `headRefOid` captured above, stop + and report a mismatch rather than deleting, someone could have pushed to the branch after the + merge, or the name could have been reused. Quote the branch name in both commands, a valid ref + can start with `-` or carry a shell metacharacter. Only once it matches, `git push origin + --delete -- ""`. Never `--force-with-lease` here, git-commit-conventions forbids it + unconditionally, this plain verify-then-delete is the safety gate, not a compare-and-swap at + delete time. The repo's auto-delete-head-branches setting is kept off fleet-wide (to protect `develop` and `main` from it, GitHub has no per-branch exception), so nothing deletes an ordinary feature branch automatically. Stop here and report the merged PR when the target is develop only. diff --git a/.agents/skills/merge-and-release/SKILL.md b/.agents/skills/merge-and-release/SKILL.md index f175e5ba..be0fc283 100644 --- a/.agents/skills/merge-and-release/SKILL.md +++ b/.agents/skills/merge-and-release/SKILL.md @@ -100,10 +100,12 @@ skill covers all of it, scoped down by what the maintainer actually asks for. report a timeout separately from a completed run's own conclusion, the tag or version it produced. A run that fails, times out, or never starts is reported, never silently retried. 7. In the hub, when the chosen scope includes a release, bring this checkout to the merged - content without discarding or mixing in anything local. First assert `git status --porcelain` - is empty, and stop and report rather than proceeding over any uncommitted content, tracked or - not, since `skills_install.py` installs from whatever ends up on disk and a leftover local file - would ride along into the install silently. Then `git fetch origin main`, `git checkout main` + content without discarding or mixing in anything local. First assert `git status --porcelain + --untracked-files=all --ignored` is empty, and stop and report rather than proceeding over any + uncommitted content, tracked, untracked, or gitignored, since `skills_install.py` installs + each skill directory with `shutil.copytree()`, which copies a gitignored stray file the same + as any other, so the plain porcelain form (silent on ignored paths) would pass this preflight + while one still rides along into the install. Then `git fetch origin main`, `git checkout main` (or `git checkout -b main origin/main` the first time this checkout carries no local `main` at all, `checkout` rather than `switch` since the fleet's own `git` floor is undeclared and `checkout` needs no minimum version for this), and `git merge --ff-only origin/main`. @@ -125,10 +127,12 @@ skill covers all of it, scoped down by what the maintainer actually asks for. failure, an ambiguous run match, a timeout, a failed run, or a hub Skills refresh all still reach this step, the merge in step 3 already landed by then. Two parts, both required, neither optional: - - The promotion PR's own worktree: fetch and prune, fast-forward the base clone to `develop`, - remove the worktree. Never delete `develop`, it is the promotion PR's own head, and the - repo's auto-delete-head-branches setting is kept off fleet-wide for exactly this reason, so - nothing does this automatically. + - The promotion PR's own worktree: fetch and prune, remove the worktree, then fast-forward the + base clone to `develop`. Removing first, not after, matters: the base clone cannot check out + `develop` while the promotion worktree still has it checked out, one branch checked out in + two worktrees at once is refused outright. Never delete `develop`, it is the promotion PR's + own head, and the repo's auto-delete-head-branches setting is kept off fleet-wide for exactly + this reason, so nothing does this automatically. - A defensive sweep for anything drive-pr's own cleanup should already have removed but might not have, an interrupted loop, a fix landed by hand outside that skill, or a maintainer merge in the GitHub UI. `git worktree list` for any worktree still registered under this @@ -137,8 +141,8 @@ skill covers all of it, scoped down by what the maintainer actually asks for. reading GitHub's own state with the exact fields this check needs, not a bare listing, and stop and report rather than guessing when selection is not exactly one match, on a non-1 count exit non-zero rather than returning empty with success, an ambiguous or missing match - must fail loud, not read as an empty value still safe to act on: `gh pr list --head - --state merged --repo owner/repo --json + must fail loud, not read as an empty value still safe to act on: `gh pr list --head + "" --state merged --repo owner/repo --json number,baseRefName,mergedAt,headRefOid,headRepositoryOwner --jq 'if length == 1 then .[0] else error("expected exactly one merged PR for this head, got \(length)") end'`. Confirm `headRepositoryOwner.login` names this same repo's owner, a fork's PR against the same base @@ -146,22 +150,25 @@ skill covers all of it, scoped down by what the maintainer actually asks for. is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips only where a remote branch actually exists, - `git ls-remote --heads origin ` empty means it is already gone, most likely a prior + `git ls-remote --heads origin ""` empty means it is already gone, most likely a prior cleanup attempt got interrupted after the remote delete but before the local one, so skip straight to the local-tip check below and never attempt the remote delete a second time. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local - branch tip (`git rev-parse `) must also match `headRefOid`. + branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch name + below is a quoted argument for the same reason, a valid ref can start with `-` or carry a + shell metacharacter, and `--` marks the end of options wherever a command supports it. `git merge-base --is-ancestor develop` must never be used for either tip check, a squash merge (drive-pr's own merge method) never makes the feature tip a literal ancestor of `develop`, so the check reports every already-finished branch as unmerged. Only once GitHub confirms it, and the worktree is clean (a dirty worktree stops cleanup rather than discarding - uncommitted work), remove the worktree, `git worktree remove`, then delete the local branch. - `git branch -d` has the identical squash blindness as `git merge-base --is-ancestor` and - refuses too, so use `git branch -D ` here, safe only because the GitHub-state - check just proved that exact branch finished, the narrow post-squash exception - git-commit-conventions describes, never applied to an unverified branch. Then, only when the - remote branch still exists, delete it the same way, `git push origin --delete `. + uncommitted work), remove the worktree by its exact path, `git worktree remove + ""`, `git worktree list` names it, then delete the local branch. `git branch + -d` has the identical squash blindness as `git merge-base --is-ancestor` and refuses too, so + use `git branch -D -- ""` here, safe only because the GitHub-state check just + proved that exact branch finished, the narrow post-squash exception git-commit-conventions + describes, never applied to an unverified branch. Then, only when the remote branch still + exists, delete it the same way, `git push origin --delete -- ""`. Never `--force-with-lease` here, git-commit-conventions forbids it unconditionally, the GitHub-state check just completed is the verification gate, not a compare-and-swap at delete time. Never apply this sweep to `develop` or `main` diff --git a/.claude-plugin/fleet-skills/.source-digest b/.claude-plugin/fleet-skills/.source-digest index 6db97af8..50c1ee66 100644 --- a/.claude-plugin/fleet-skills/.source-digest +++ b/.claude-plugin/fleet-skills/.source-digest @@ -1 +1 @@ -4259e0af969b1580 +119a17dc10e8f990 diff --git a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md index 37daa859..e1b743b2 100644 --- a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md @@ -67,11 +67,13 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. to delete it, which fails when `develop` is already checked out somewhere else, the ordinary case in this layout. Instead run repo-worktree's post-merge cleanup from the base clone: remove the worktree, delete the now-merged local task branch, then verify before deleting the remote - one, `git ls-remote --heads origin ` matches the `headRefOid` captured above, stop and - report a mismatch rather than deleting, someone could have pushed to the branch after the - merge, or the name could have been reused. Only once it matches, `git push origin --delete - `. Never `--force-with-lease` here, git-commit-conventions forbids it unconditionally, - this plain verify-then-delete is the safety gate, not a compare-and-swap at delete time. The + one, `git ls-remote --heads origin ""` matches the `headRefOid` captured above, stop + and report a mismatch rather than deleting, someone could have pushed to the branch after the + merge, or the name could have been reused. Quote the branch name in both commands, a valid ref + can start with `-` or carry a shell metacharacter. Only once it matches, `git push origin + --delete -- ""`. Never `--force-with-lease` here, git-commit-conventions forbids it + unconditionally, this plain verify-then-delete is the safety gate, not a compare-and-swap at + delete time. The repo's auto-delete-head-branches setting is kept off fleet-wide (to protect `develop` and `main` from it, GitHub has no per-branch exception), so nothing deletes an ordinary feature branch automatically. Stop here and report the merged PR when the target is develop only. diff --git a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md index f175e5ba..be0fc283 100644 --- a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md @@ -100,10 +100,12 @@ skill covers all of it, scoped down by what the maintainer actually asks for. report a timeout separately from a completed run's own conclusion, the tag or version it produced. A run that fails, times out, or never starts is reported, never silently retried. 7. In the hub, when the chosen scope includes a release, bring this checkout to the merged - content without discarding or mixing in anything local. First assert `git status --porcelain` - is empty, and stop and report rather than proceeding over any uncommitted content, tracked or - not, since `skills_install.py` installs from whatever ends up on disk and a leftover local file - would ride along into the install silently. Then `git fetch origin main`, `git checkout main` + content without discarding or mixing in anything local. First assert `git status --porcelain + --untracked-files=all --ignored` is empty, and stop and report rather than proceeding over any + uncommitted content, tracked, untracked, or gitignored, since `skills_install.py` installs + each skill directory with `shutil.copytree()`, which copies a gitignored stray file the same + as any other, so the plain porcelain form (silent on ignored paths) would pass this preflight + while one still rides along into the install. Then `git fetch origin main`, `git checkout main` (or `git checkout -b main origin/main` the first time this checkout carries no local `main` at all, `checkout` rather than `switch` since the fleet's own `git` floor is undeclared and `checkout` needs no minimum version for this), and `git merge --ff-only origin/main`. @@ -125,10 +127,12 @@ skill covers all of it, scoped down by what the maintainer actually asks for. failure, an ambiguous run match, a timeout, a failed run, or a hub Skills refresh all still reach this step, the merge in step 3 already landed by then. Two parts, both required, neither optional: - - The promotion PR's own worktree: fetch and prune, fast-forward the base clone to `develop`, - remove the worktree. Never delete `develop`, it is the promotion PR's own head, and the - repo's auto-delete-head-branches setting is kept off fleet-wide for exactly this reason, so - nothing does this automatically. + - The promotion PR's own worktree: fetch and prune, remove the worktree, then fast-forward the + base clone to `develop`. Removing first, not after, matters: the base clone cannot check out + `develop` while the promotion worktree still has it checked out, one branch checked out in + two worktrees at once is refused outright. Never delete `develop`, it is the promotion PR's + own head, and the repo's auto-delete-head-branches setting is kept off fleet-wide for exactly + this reason, so nothing does this automatically. - A defensive sweep for anything drive-pr's own cleanup should already have removed but might not have, an interrupted loop, a fix landed by hand outside that skill, or a maintainer merge in the GitHub UI. `git worktree list` for any worktree still registered under this @@ -137,8 +141,8 @@ skill covers all of it, scoped down by what the maintainer actually asks for. reading GitHub's own state with the exact fields this check needs, not a bare listing, and stop and report rather than guessing when selection is not exactly one match, on a non-1 count exit non-zero rather than returning empty with success, an ambiguous or missing match - must fail loud, not read as an empty value still safe to act on: `gh pr list --head - --state merged --repo owner/repo --json + must fail loud, not read as an empty value still safe to act on: `gh pr list --head + "" --state merged --repo owner/repo --json number,baseRefName,mergedAt,headRefOid,headRepositoryOwner --jq 'if length == 1 then .[0] else error("expected exactly one merged PR for this head, got \(length)") end'`. Confirm `headRepositoryOwner.login` names this same repo's owner, a fork's PR against the same base @@ -146,22 +150,25 @@ skill covers all of it, scoped down by what the maintainer actually asks for. is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips only where a remote branch actually exists, - `git ls-remote --heads origin ` empty means it is already gone, most likely a prior + `git ls-remote --heads origin ""` empty means it is already gone, most likely a prior cleanup attempt got interrupted after the remote delete but before the local one, so skip straight to the local-tip check below and never attempt the remote delete a second time. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local - branch tip (`git rev-parse `) must also match `headRefOid`. + branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch name + below is a quoted argument for the same reason, a valid ref can start with `-` or carry a + shell metacharacter, and `--` marks the end of options wherever a command supports it. `git merge-base --is-ancestor develop` must never be used for either tip check, a squash merge (drive-pr's own merge method) never makes the feature tip a literal ancestor of `develop`, so the check reports every already-finished branch as unmerged. Only once GitHub confirms it, and the worktree is clean (a dirty worktree stops cleanup rather than discarding - uncommitted work), remove the worktree, `git worktree remove`, then delete the local branch. - `git branch -d` has the identical squash blindness as `git merge-base --is-ancestor` and - refuses too, so use `git branch -D ` here, safe only because the GitHub-state - check just proved that exact branch finished, the narrow post-squash exception - git-commit-conventions describes, never applied to an unverified branch. Then, only when the - remote branch still exists, delete it the same way, `git push origin --delete `. + uncommitted work), remove the worktree by its exact path, `git worktree remove + ""`, `git worktree list` names it, then delete the local branch. `git branch + -d` has the identical squash blindness as `git merge-base --is-ancestor` and refuses too, so + use `git branch -D -- ""` here, safe only because the GitHub-state check just + proved that exact branch finished, the narrow post-squash exception git-commit-conventions + describes, never applied to an unverified branch. Then, only when the remote branch still + exists, delete it the same way, `git push origin --delete -- ""`. Never `--force-with-lease` here, git-commit-conventions forbids it unconditionally, the GitHub-state check just completed is the verification gate, not a compare-and-swap at delete time. Never apply this sweep to `develop` or `main` diff --git a/.github/skills/drive-pr/SKILL.md b/.github/skills/drive-pr/SKILL.md index 37daa859..e1b743b2 100644 --- a/.github/skills/drive-pr/SKILL.md +++ b/.github/skills/drive-pr/SKILL.md @@ -67,11 +67,13 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. to delete it, which fails when `develop` is already checked out somewhere else, the ordinary case in this layout. Instead run repo-worktree's post-merge cleanup from the base clone: remove the worktree, delete the now-merged local task branch, then verify before deleting the remote - one, `git ls-remote --heads origin ` matches the `headRefOid` captured above, stop and - report a mismatch rather than deleting, someone could have pushed to the branch after the - merge, or the name could have been reused. Only once it matches, `git push origin --delete - `. Never `--force-with-lease` here, git-commit-conventions forbids it unconditionally, - this plain verify-then-delete is the safety gate, not a compare-and-swap at delete time. The + one, `git ls-remote --heads origin ""` matches the `headRefOid` captured above, stop + and report a mismatch rather than deleting, someone could have pushed to the branch after the + merge, or the name could have been reused. Quote the branch name in both commands, a valid ref + can start with `-` or carry a shell metacharacter. Only once it matches, `git push origin + --delete -- ""`. Never `--force-with-lease` here, git-commit-conventions forbids it + unconditionally, this plain verify-then-delete is the safety gate, not a compare-and-swap at + delete time. The repo's auto-delete-head-branches setting is kept off fleet-wide (to protect `develop` and `main` from it, GitHub has no per-branch exception), so nothing deletes an ordinary feature branch automatically. Stop here and report the merged PR when the target is develop only. diff --git a/.github/skills/merge-and-release/SKILL.md b/.github/skills/merge-and-release/SKILL.md index f175e5ba..be0fc283 100644 --- a/.github/skills/merge-and-release/SKILL.md +++ b/.github/skills/merge-and-release/SKILL.md @@ -100,10 +100,12 @@ skill covers all of it, scoped down by what the maintainer actually asks for. report a timeout separately from a completed run's own conclusion, the tag or version it produced. A run that fails, times out, or never starts is reported, never silently retried. 7. In the hub, when the chosen scope includes a release, bring this checkout to the merged - content without discarding or mixing in anything local. First assert `git status --porcelain` - is empty, and stop and report rather than proceeding over any uncommitted content, tracked or - not, since `skills_install.py` installs from whatever ends up on disk and a leftover local file - would ride along into the install silently. Then `git fetch origin main`, `git checkout main` + content without discarding or mixing in anything local. First assert `git status --porcelain + --untracked-files=all --ignored` is empty, and stop and report rather than proceeding over any + uncommitted content, tracked, untracked, or gitignored, since `skills_install.py` installs + each skill directory with `shutil.copytree()`, which copies a gitignored stray file the same + as any other, so the plain porcelain form (silent on ignored paths) would pass this preflight + while one still rides along into the install. Then `git fetch origin main`, `git checkout main` (or `git checkout -b main origin/main` the first time this checkout carries no local `main` at all, `checkout` rather than `switch` since the fleet's own `git` floor is undeclared and `checkout` needs no minimum version for this), and `git merge --ff-only origin/main`. @@ -125,10 +127,12 @@ skill covers all of it, scoped down by what the maintainer actually asks for. failure, an ambiguous run match, a timeout, a failed run, or a hub Skills refresh all still reach this step, the merge in step 3 already landed by then. Two parts, both required, neither optional: - - The promotion PR's own worktree: fetch and prune, fast-forward the base clone to `develop`, - remove the worktree. Never delete `develop`, it is the promotion PR's own head, and the - repo's auto-delete-head-branches setting is kept off fleet-wide for exactly this reason, so - nothing does this automatically. + - The promotion PR's own worktree: fetch and prune, remove the worktree, then fast-forward the + base clone to `develop`. Removing first, not after, matters: the base clone cannot check out + `develop` while the promotion worktree still has it checked out, one branch checked out in + two worktrees at once is refused outright. Never delete `develop`, it is the promotion PR's + own head, and the repo's auto-delete-head-branches setting is kept off fleet-wide for exactly + this reason, so nothing does this automatically. - A defensive sweep for anything drive-pr's own cleanup should already have removed but might not have, an interrupted loop, a fix landed by hand outside that skill, or a maintainer merge in the GitHub UI. `git worktree list` for any worktree still registered under this @@ -137,8 +141,8 @@ skill covers all of it, scoped down by what the maintainer actually asks for. reading GitHub's own state with the exact fields this check needs, not a bare listing, and stop and report rather than guessing when selection is not exactly one match, on a non-1 count exit non-zero rather than returning empty with success, an ambiguous or missing match - must fail loud, not read as an empty value still safe to act on: `gh pr list --head - --state merged --repo owner/repo --json + must fail loud, not read as an empty value still safe to act on: `gh pr list --head + "" --state merged --repo owner/repo --json number,baseRefName,mergedAt,headRefOid,headRepositoryOwner --jq 'if length == 1 then .[0] else error("expected exactly one merged PR for this head, got \(length)") end'`. Confirm `headRepositoryOwner.login` names this same repo's owner, a fork's PR against the same base @@ -146,22 +150,25 @@ skill covers all of it, scoped down by what the maintainer actually asks for. is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips only where a remote branch actually exists, - `git ls-remote --heads origin ` empty means it is already gone, most likely a prior + `git ls-remote --heads origin ""` empty means it is already gone, most likely a prior cleanup attempt got interrupted after the remote delete but before the local one, so skip straight to the local-tip check below and never attempt the remote delete a second time. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local - branch tip (`git rev-parse `) must also match `headRefOid`. + branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch name + below is a quoted argument for the same reason, a valid ref can start with `-` or carry a + shell metacharacter, and `--` marks the end of options wherever a command supports it. `git merge-base --is-ancestor develop` must never be used for either tip check, a squash merge (drive-pr's own merge method) never makes the feature tip a literal ancestor of `develop`, so the check reports every already-finished branch as unmerged. Only once GitHub confirms it, and the worktree is clean (a dirty worktree stops cleanup rather than discarding - uncommitted work), remove the worktree, `git worktree remove`, then delete the local branch. - `git branch -d` has the identical squash blindness as `git merge-base --is-ancestor` and - refuses too, so use `git branch -D ` here, safe only because the GitHub-state - check just proved that exact branch finished, the narrow post-squash exception - git-commit-conventions describes, never applied to an unverified branch. Then, only when the - remote branch still exists, delete it the same way, `git push origin --delete `. + uncommitted work), remove the worktree by its exact path, `git worktree remove + ""`, `git worktree list` names it, then delete the local branch. `git branch + -d` has the identical squash blindness as `git merge-base --is-ancestor` and refuses too, so + use `git branch -D -- ""` here, safe only because the GitHub-state check just + proved that exact branch finished, the narrow post-squash exception git-commit-conventions + describes, never applied to an unverified branch. Then, only when the remote branch still + exists, delete it the same way, `git push origin --delete -- ""`. Never `--force-with-lease` here, git-commit-conventions forbids it unconditionally, the GitHub-state check just completed is the verification gate, not a compare-and-swap at delete time. Never apply this sweep to `develop` or `main` From b2f657b5bd0b1a45eff572a5348d82a1b54c5769 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 20:05:57 -0700 Subject: [PATCH 02/11] Add -- End-of-Options to ls-remote Too, for Uniformity Not confirmed as a live bug (tested empirically: git ls-remote does not re-parse a positional after the repository argument as an option, unlike rev-parse/branch -D/push --delete, which do), added anyway so all six cleanup commands follow the same -- convention rather than carrying an unexplained exception. Co-Authored-By: Claude Sonnet 5 --- .agents/skills/drive-pr/SKILL.md | 2 +- .agents/skills/merge-and-release/SKILL.md | 2 +- .claude-plugin/fleet-skills/.source-digest | 2 +- .claude-plugin/fleet-skills/skills/drive-pr/SKILL.md | 2 +- .claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md | 2 +- .github/skills/drive-pr/SKILL.md | 2 +- .github/skills/merge-and-release/SKILL.md | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.agents/skills/drive-pr/SKILL.md b/.agents/skills/drive-pr/SKILL.md index e1b743b2..8d6a6fda 100644 --- a/.agents/skills/drive-pr/SKILL.md +++ b/.agents/skills/drive-pr/SKILL.md @@ -67,7 +67,7 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. to delete it, which fails when `develop` is already checked out somewhere else, the ordinary case in this layout. Instead run repo-worktree's post-merge cleanup from the base clone: remove the worktree, delete the now-merged local task branch, then verify before deleting the remote - one, `git ls-remote --heads origin ""` matches the `headRefOid` captured above, stop + one, `git ls-remote --heads origin -- ""` matches the `headRefOid` captured above, stop and report a mismatch rather than deleting, someone could have pushed to the branch after the merge, or the name could have been reused. Quote the branch name in both commands, a valid ref can start with `-` or carry a shell metacharacter. Only once it matches, `git push origin diff --git a/.agents/skills/merge-and-release/SKILL.md b/.agents/skills/merge-and-release/SKILL.md index be0fc283..30238af5 100644 --- a/.agents/skills/merge-and-release/SKILL.md +++ b/.agents/skills/merge-and-release/SKILL.md @@ -150,7 +150,7 @@ skill covers all of it, scoped down by what the maintainer actually asks for. is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips only where a remote branch actually exists, - `git ls-remote --heads origin ""` empty means it is already gone, most likely a prior + `git ls-remote --heads origin -- ""` empty means it is already gone, most likely a prior cleanup attempt got interrupted after the remote delete but before the local one, so skip straight to the local-tip check below and never attempt the remote delete a second time. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` diff --git a/.claude-plugin/fleet-skills/.source-digest b/.claude-plugin/fleet-skills/.source-digest index 50c1ee66..6b2afe2b 100644 --- a/.claude-plugin/fleet-skills/.source-digest +++ b/.claude-plugin/fleet-skills/.source-digest @@ -1 +1 @@ -119a17dc10e8f990 +6d77f53359e2958d diff --git a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md index e1b743b2..8d6a6fda 100644 --- a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md @@ -67,7 +67,7 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. to delete it, which fails when `develop` is already checked out somewhere else, the ordinary case in this layout. Instead run repo-worktree's post-merge cleanup from the base clone: remove the worktree, delete the now-merged local task branch, then verify before deleting the remote - one, `git ls-remote --heads origin ""` matches the `headRefOid` captured above, stop + one, `git ls-remote --heads origin -- ""` matches the `headRefOid` captured above, stop and report a mismatch rather than deleting, someone could have pushed to the branch after the merge, or the name could have been reused. Quote the branch name in both commands, a valid ref can start with `-` or carry a shell metacharacter. Only once it matches, `git push origin diff --git a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md index be0fc283..30238af5 100644 --- a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md @@ -150,7 +150,7 @@ skill covers all of it, scoped down by what the maintainer actually asks for. is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips only where a remote branch actually exists, - `git ls-remote --heads origin ""` empty means it is already gone, most likely a prior + `git ls-remote --heads origin -- ""` empty means it is already gone, most likely a prior cleanup attempt got interrupted after the remote delete but before the local one, so skip straight to the local-tip check below and never attempt the remote delete a second time. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` diff --git a/.github/skills/drive-pr/SKILL.md b/.github/skills/drive-pr/SKILL.md index e1b743b2..8d6a6fda 100644 --- a/.github/skills/drive-pr/SKILL.md +++ b/.github/skills/drive-pr/SKILL.md @@ -67,7 +67,7 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. to delete it, which fails when `develop` is already checked out somewhere else, the ordinary case in this layout. Instead run repo-worktree's post-merge cleanup from the base clone: remove the worktree, delete the now-merged local task branch, then verify before deleting the remote - one, `git ls-remote --heads origin ""` matches the `headRefOid` captured above, stop + one, `git ls-remote --heads origin -- ""` matches the `headRefOid` captured above, stop and report a mismatch rather than deleting, someone could have pushed to the branch after the merge, or the name could have been reused. Quote the branch name in both commands, a valid ref can start with `-` or carry a shell metacharacter. Only once it matches, `git push origin diff --git a/.github/skills/merge-and-release/SKILL.md b/.github/skills/merge-and-release/SKILL.md index be0fc283..30238af5 100644 --- a/.github/skills/merge-and-release/SKILL.md +++ b/.github/skills/merge-and-release/SKILL.md @@ -150,7 +150,7 @@ skill covers all of it, scoped down by what the maintainer actually asks for. is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips only where a remote branch actually exists, - `git ls-remote --heads origin ""` empty means it is already gone, most likely a prior + `git ls-remote --heads origin -- ""` empty means it is already gone, most likely a prior cleanup attempt got interrupted after the remote delete but before the local one, so skip straight to the local-tip check below and never attempt the remote delete a second time. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` From ba67d27517180c5c4c70a79ce78049fafeb66fe1 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 20:09:20 -0700 Subject: [PATCH 03/11] Verify Head Repository Identity and Exact Branch Match Two more CodeRabbit findings, both confirmed: - 'gh pr list --head' matches by prefix (gh's own --help shows --head "typo" as its example), and headRepositoryOwner.login alone does not identify the source repository, only the owner. Requested headRefName and headRepository instead of headRepositoryOwner, asserted headRefName equals the branch exactly and headRepository.nameWithOwner equals owner/repo. - Double-quoted placeholder interpolation does not stop $() or backtick command substitution the way a genuinely separate subprocess argument does. Clarified in both skills that every / placeholder means a real argument, never text built into an executed shell command string. Co-Authored-By: Claude Sonnet 5 --- .agents/skills/drive-pr/SKILL.md | 9 ++++++--- .agents/skills/merge-and-release/SKILL.md | 20 ++++++++++++------- .claude-plugin/fleet-skills/.source-digest | 2 +- .../fleet-skills/skills/drive-pr/SKILL.md | 9 ++++++--- .../skills/merge-and-release/SKILL.md | 20 ++++++++++++------- .github/skills/drive-pr/SKILL.md | 9 ++++++--- .github/skills/merge-and-release/SKILL.md | 20 ++++++++++++------- 7 files changed, 58 insertions(+), 31 deletions(-) diff --git a/.agents/skills/drive-pr/SKILL.md b/.agents/skills/drive-pr/SKILL.md index 8d6a6fda..36101d3b 100644 --- a/.agents/skills/drive-pr/SKILL.md +++ b/.agents/skills/drive-pr/SKILL.md @@ -69,9 +69,12 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. the worktree, delete the now-merged local task branch, then verify before deleting the remote one, `git ls-remote --heads origin -- ""` matches the `headRefOid` captured above, stop and report a mismatch rather than deleting, someone could have pushed to the branch after the - merge, or the name could have been reused. Quote the branch name in both commands, a valid ref - can start with `-` or carry a shell metacharacter. Only once it matches, `git push origin - --delete -- ""`. Never `--force-with-lease` here, git-commit-conventions forbids it + merge, or the name could have been reused. `` is a quoted argument the caller + substitutes the real value into, never text built into an executed shell command string, a + valid ref can start with `-` or carry a shell metacharacter, and quoting alone does not stop + `$()` or backtick command substitution the way a genuinely separate subprocess argument does. + Only once it matches, `git push origin --delete -- ""`. Never `--force-with-lease` + here, git-commit-conventions forbids it unconditionally, this plain verify-then-delete is the safety gate, not a compare-and-swap at delete time. The repo's auto-delete-head-branches setting is kept off fleet-wide (to protect `develop` and diff --git a/.agents/skills/merge-and-release/SKILL.md b/.agents/skills/merge-and-release/SKILL.md index 30238af5..c4a71ef2 100644 --- a/.agents/skills/merge-and-release/SKILL.md +++ b/.agents/skills/merge-and-release/SKILL.md @@ -143,10 +143,13 @@ skill covers all of it, scoped down by what the maintainer actually asks for. count exit non-zero rather than returning empty with success, an ambiguous or missing match must fail loud, not read as an empty value still safe to act on: `gh pr list --head "" --state merged --repo owner/repo --json - number,baseRefName,mergedAt,headRefOid,headRepositoryOwner --jq 'if length == 1 then .[0] - else error("expected exactly one merged PR for this head, got \(length)") end'`. Confirm - `headRepositoryOwner.login` names this same repo's owner, a fork's PR against the same base - can carry an identical head branch name and must never pass this check. Confirm `baseRefName` + number,baseRefName,mergedAt,headRefOid,headRefName,headRepository --jq 'if length == 1 then + .[0] else error("expected exactly one merged PR for this head, got \(length)") end'`. + `--head` matches by prefix rather than exactly, `gh pr list --help` shows `--head "typo"` as + its own example, so confirm `headRefName` equals `` exactly before trusting anything + else this query returned. Confirm `headRepository` is non-null and its `nameWithOwner` equals + `owner/repo`, the owner alone is not enough, a same-owner PR against an identically named + branch in a different repository must never pass this check either. Confirm `baseRefName` is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips only where a remote branch actually exists, @@ -155,9 +158,12 @@ skill covers all of it, scoped down by what the maintainer actually asks for. straight to the local-tip check below and never attempt the remote delete a second time. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local - branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch name - below is a quoted argument for the same reason, a valid ref can start with `-` or carry a - shell metacharacter, and `--` marks the end of options wherever a command supports it. + branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch or + worktree-path placeholder below is a quoted argument the caller substitutes the real value + into, never text built into an executed shell command string, a valid ref can start with `-` + or carry a shell metacharacter, and quoting alone does not stop `$()` or backtick command + substitution the way passing a genuinely separate subprocess argument does. `--` marks the + end of options wherever a command supports it. `git merge-base --is-ancestor develop` must never be used for either tip check, a squash merge (drive-pr's own merge method) never makes the feature tip a literal ancestor of `develop`, so the check reports every already-finished branch as unmerged. Only once GitHub diff --git a/.claude-plugin/fleet-skills/.source-digest b/.claude-plugin/fleet-skills/.source-digest index 6b2afe2b..5c874cf4 100644 --- a/.claude-plugin/fleet-skills/.source-digest +++ b/.claude-plugin/fleet-skills/.source-digest @@ -1 +1 @@ -6d77f53359e2958d +d80080335f23c133 diff --git a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md index 8d6a6fda..36101d3b 100644 --- a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md @@ -69,9 +69,12 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. the worktree, delete the now-merged local task branch, then verify before deleting the remote one, `git ls-remote --heads origin -- ""` matches the `headRefOid` captured above, stop and report a mismatch rather than deleting, someone could have pushed to the branch after the - merge, or the name could have been reused. Quote the branch name in both commands, a valid ref - can start with `-` or carry a shell metacharacter. Only once it matches, `git push origin - --delete -- ""`. Never `--force-with-lease` here, git-commit-conventions forbids it + merge, or the name could have been reused. `` is a quoted argument the caller + substitutes the real value into, never text built into an executed shell command string, a + valid ref can start with `-` or carry a shell metacharacter, and quoting alone does not stop + `$()` or backtick command substitution the way a genuinely separate subprocess argument does. + Only once it matches, `git push origin --delete -- ""`. Never `--force-with-lease` + here, git-commit-conventions forbids it unconditionally, this plain verify-then-delete is the safety gate, not a compare-and-swap at delete time. The repo's auto-delete-head-branches setting is kept off fleet-wide (to protect `develop` and diff --git a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md index 30238af5..c4a71ef2 100644 --- a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md @@ -143,10 +143,13 @@ skill covers all of it, scoped down by what the maintainer actually asks for. count exit non-zero rather than returning empty with success, an ambiguous or missing match must fail loud, not read as an empty value still safe to act on: `gh pr list --head "" --state merged --repo owner/repo --json - number,baseRefName,mergedAt,headRefOid,headRepositoryOwner --jq 'if length == 1 then .[0] - else error("expected exactly one merged PR for this head, got \(length)") end'`. Confirm - `headRepositoryOwner.login` names this same repo's owner, a fork's PR against the same base - can carry an identical head branch name and must never pass this check. Confirm `baseRefName` + number,baseRefName,mergedAt,headRefOid,headRefName,headRepository --jq 'if length == 1 then + .[0] else error("expected exactly one merged PR for this head, got \(length)") end'`. + `--head` matches by prefix rather than exactly, `gh pr list --help` shows `--head "typo"` as + its own example, so confirm `headRefName` equals `` exactly before trusting anything + else this query returned. Confirm `headRepository` is non-null and its `nameWithOwner` equals + `owner/repo`, the owner alone is not enough, a same-owner PR against an identically named + branch in a different repository must never pass this check either. Confirm `baseRefName` is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips only where a remote branch actually exists, @@ -155,9 +158,12 @@ skill covers all of it, scoped down by what the maintainer actually asks for. straight to the local-tip check below and never attempt the remote delete a second time. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local - branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch name - below is a quoted argument for the same reason, a valid ref can start with `-` or carry a - shell metacharacter, and `--` marks the end of options wherever a command supports it. + branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch or + worktree-path placeholder below is a quoted argument the caller substitutes the real value + into, never text built into an executed shell command string, a valid ref can start with `-` + or carry a shell metacharacter, and quoting alone does not stop `$()` or backtick command + substitution the way passing a genuinely separate subprocess argument does. `--` marks the + end of options wherever a command supports it. `git merge-base --is-ancestor develop` must never be used for either tip check, a squash merge (drive-pr's own merge method) never makes the feature tip a literal ancestor of `develop`, so the check reports every already-finished branch as unmerged. Only once GitHub diff --git a/.github/skills/drive-pr/SKILL.md b/.github/skills/drive-pr/SKILL.md index 8d6a6fda..36101d3b 100644 --- a/.github/skills/drive-pr/SKILL.md +++ b/.github/skills/drive-pr/SKILL.md @@ -69,9 +69,12 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. the worktree, delete the now-merged local task branch, then verify before deleting the remote one, `git ls-remote --heads origin -- ""` matches the `headRefOid` captured above, stop and report a mismatch rather than deleting, someone could have pushed to the branch after the - merge, or the name could have been reused. Quote the branch name in both commands, a valid ref - can start with `-` or carry a shell metacharacter. Only once it matches, `git push origin - --delete -- ""`. Never `--force-with-lease` here, git-commit-conventions forbids it + merge, or the name could have been reused. `` is a quoted argument the caller + substitutes the real value into, never text built into an executed shell command string, a + valid ref can start with `-` or carry a shell metacharacter, and quoting alone does not stop + `$()` or backtick command substitution the way a genuinely separate subprocess argument does. + Only once it matches, `git push origin --delete -- ""`. Never `--force-with-lease` + here, git-commit-conventions forbids it unconditionally, this plain verify-then-delete is the safety gate, not a compare-and-swap at delete time. The repo's auto-delete-head-branches setting is kept off fleet-wide (to protect `develop` and diff --git a/.github/skills/merge-and-release/SKILL.md b/.github/skills/merge-and-release/SKILL.md index 30238af5..c4a71ef2 100644 --- a/.github/skills/merge-and-release/SKILL.md +++ b/.github/skills/merge-and-release/SKILL.md @@ -143,10 +143,13 @@ skill covers all of it, scoped down by what the maintainer actually asks for. count exit non-zero rather than returning empty with success, an ambiguous or missing match must fail loud, not read as an empty value still safe to act on: `gh pr list --head "" --state merged --repo owner/repo --json - number,baseRefName,mergedAt,headRefOid,headRepositoryOwner --jq 'if length == 1 then .[0] - else error("expected exactly one merged PR for this head, got \(length)") end'`. Confirm - `headRepositoryOwner.login` names this same repo's owner, a fork's PR against the same base - can carry an identical head branch name and must never pass this check. Confirm `baseRefName` + number,baseRefName,mergedAt,headRefOid,headRefName,headRepository --jq 'if length == 1 then + .[0] else error("expected exactly one merged PR for this head, got \(length)") end'`. + `--head` matches by prefix rather than exactly, `gh pr list --help` shows `--head "typo"` as + its own example, so confirm `headRefName` equals `` exactly before trusting anything + else this query returned. Confirm `headRepository` is non-null and its `nameWithOwner` equals + `owner/repo`, the owner alone is not enough, a same-owner PR against an identically named + branch in a different repository must never pass this check either. Confirm `baseRefName` is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips only where a remote branch actually exists, @@ -155,9 +158,12 @@ skill covers all of it, scoped down by what the maintainer actually asks for. straight to the local-tip check below and never attempt the remote delete a second time. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local - branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch name - below is a quoted argument for the same reason, a valid ref can start with `-` or carry a - shell metacharacter, and `--` marks the end of options wherever a command supports it. + branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch or + worktree-path placeholder below is a quoted argument the caller substitutes the real value + into, never text built into an executed shell command string, a valid ref can start with `-` + or carry a shell metacharacter, and quoting alone does not stop `$()` or backtick command + substitution the way passing a genuinely separate subprocess argument does. `--` marks the + end of options wherever a command supports it. `git merge-base --is-ancestor develop` must never be used for either tip check, a squash merge (drive-pr's own merge method) never makes the feature tip a literal ancestor of `develop`, so the check reports every already-finished branch as unmerged. Only once GitHub From d6f1d9df10aefe817594e70775eaee8c7bfc0037 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 20:11:28 -0700 Subject: [PATCH 04/11] Use the Fully-Qualified refs/heads Form for ls-remote CodeRabbit finding, confirmed empirically: 'git ls-remote --heads origin ""' tail-matches rather than exact-matches. A scratch repo with branches 'topic/x' and 'other/topic/x' returned both for a query of 'topic/x'; the fully-qualified 'refs/heads/topic/x' form returned exactly the one. Applied to both occurrences (drive-pr and merge-and-release). Co-Authored-By: Claude Sonnet 5 --- .agents/skills/drive-pr/SKILL.md | 8 +++++--- .agents/skills/merge-and-release/SKILL.md | 10 ++++++---- .claude-plugin/fleet-skills/.source-digest | 2 +- .claude-plugin/fleet-skills/skills/drive-pr/SKILL.md | 8 +++++--- .../fleet-skills/skills/merge-and-release/SKILL.md | 10 ++++++---- .github/skills/drive-pr/SKILL.md | 8 +++++--- .github/skills/merge-and-release/SKILL.md | 10 ++++++---- 7 files changed, 34 insertions(+), 22 deletions(-) diff --git a/.agents/skills/drive-pr/SKILL.md b/.agents/skills/drive-pr/SKILL.md index 36101d3b..c676d32b 100644 --- a/.agents/skills/drive-pr/SKILL.md +++ b/.agents/skills/drive-pr/SKILL.md @@ -67,9 +67,11 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. to delete it, which fails when `develop` is already checked out somewhere else, the ordinary case in this layout. Instead run repo-worktree's post-merge cleanup from the base clone: remove the worktree, delete the now-merged local task branch, then verify before deleting the remote - one, `git ls-remote --heads origin -- ""` matches the `headRefOid` captured above, stop - and report a mismatch rather than deleting, someone could have pushed to the branch after the - merge, or the name could have been reused. `` is a quoted argument the caller + one, `git ls-remote --heads origin -- "refs/heads/"` matches the `headRefOid` captured + above, the fully-qualified form since `--heads origin ""` alone still tail-matches a + differently-prefixed branch sharing the same suffix. Stop and report a mismatch rather than + deleting, someone could have pushed to the branch after the merge, or the name could have been + reused. `` is a quoted argument the caller substitutes the real value into, never text built into an executed shell command string, a valid ref can start with `-` or carry a shell metacharacter, and quoting alone does not stop `$()` or backtick command substitution the way a genuinely separate subprocess argument does. diff --git a/.agents/skills/merge-and-release/SKILL.md b/.agents/skills/merge-and-release/SKILL.md index c4a71ef2..f2e85831 100644 --- a/.agents/skills/merge-and-release/SKILL.md +++ b/.agents/skills/merge-and-release/SKILL.md @@ -152,10 +152,12 @@ skill covers all of it, scoped down by what the maintainer actually asks for. branch in a different repository must never pass this check either. Confirm `baseRefName` is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips - only where a remote branch actually exists, - `git ls-remote --heads origin -- ""` empty means it is already gone, most likely a prior - cleanup attempt got interrupted after the remote delete but before the local one, so skip - straight to the local-tip check below and never attempt the remote delete a second time. + only where a remote branch actually exists. `git ls-remote --heads origin -- "refs/heads/"` + is the exact-match form and must be, `--heads origin ""` alone still tail-matches, a + bare `topic/x` pattern also returns an unrelated `other/topic/x` if one exists. Empty means it + is already gone, most likely a prior cleanup attempt got interrupted after the remote delete + but before the local one, so skip straight to the local-tip check below and never attempt the + remote delete a second time. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch or diff --git a/.claude-plugin/fleet-skills/.source-digest b/.claude-plugin/fleet-skills/.source-digest index 5c874cf4..f900e657 100644 --- a/.claude-plugin/fleet-skills/.source-digest +++ b/.claude-plugin/fleet-skills/.source-digest @@ -1 +1 @@ -d80080335f23c133 +d1db5082e435922b diff --git a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md index 36101d3b..c676d32b 100644 --- a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md @@ -67,9 +67,11 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. to delete it, which fails when `develop` is already checked out somewhere else, the ordinary case in this layout. Instead run repo-worktree's post-merge cleanup from the base clone: remove the worktree, delete the now-merged local task branch, then verify before deleting the remote - one, `git ls-remote --heads origin -- ""` matches the `headRefOid` captured above, stop - and report a mismatch rather than deleting, someone could have pushed to the branch after the - merge, or the name could have been reused. `` is a quoted argument the caller + one, `git ls-remote --heads origin -- "refs/heads/"` matches the `headRefOid` captured + above, the fully-qualified form since `--heads origin ""` alone still tail-matches a + differently-prefixed branch sharing the same suffix. Stop and report a mismatch rather than + deleting, someone could have pushed to the branch after the merge, or the name could have been + reused. `` is a quoted argument the caller substitutes the real value into, never text built into an executed shell command string, a valid ref can start with `-` or carry a shell metacharacter, and quoting alone does not stop `$()` or backtick command substitution the way a genuinely separate subprocess argument does. diff --git a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md index c4a71ef2..f2e85831 100644 --- a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md @@ -152,10 +152,12 @@ skill covers all of it, scoped down by what the maintainer actually asks for. branch in a different repository must never pass this check either. Confirm `baseRefName` is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips - only where a remote branch actually exists, - `git ls-remote --heads origin -- ""` empty means it is already gone, most likely a prior - cleanup attempt got interrupted after the remote delete but before the local one, so skip - straight to the local-tip check below and never attempt the remote delete a second time. + only where a remote branch actually exists. `git ls-remote --heads origin -- "refs/heads/"` + is the exact-match form and must be, `--heads origin ""` alone still tail-matches, a + bare `topic/x` pattern also returns an unrelated `other/topic/x` if one exists. Empty means it + is already gone, most likely a prior cleanup attempt got interrupted after the remote delete + but before the local one, so skip straight to the local-tip check below and never attempt the + remote delete a second time. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch or diff --git a/.github/skills/drive-pr/SKILL.md b/.github/skills/drive-pr/SKILL.md index 36101d3b..c676d32b 100644 --- a/.github/skills/drive-pr/SKILL.md +++ b/.github/skills/drive-pr/SKILL.md @@ -67,9 +67,11 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. to delete it, which fails when `develop` is already checked out somewhere else, the ordinary case in this layout. Instead run repo-worktree's post-merge cleanup from the base clone: remove the worktree, delete the now-merged local task branch, then verify before deleting the remote - one, `git ls-remote --heads origin -- ""` matches the `headRefOid` captured above, stop - and report a mismatch rather than deleting, someone could have pushed to the branch after the - merge, or the name could have been reused. `` is a quoted argument the caller + one, `git ls-remote --heads origin -- "refs/heads/"` matches the `headRefOid` captured + above, the fully-qualified form since `--heads origin ""` alone still tail-matches a + differently-prefixed branch sharing the same suffix. Stop and report a mismatch rather than + deleting, someone could have pushed to the branch after the merge, or the name could have been + reused. `` is a quoted argument the caller substitutes the real value into, never text built into an executed shell command string, a valid ref can start with `-` or carry a shell metacharacter, and quoting alone does not stop `$()` or backtick command substitution the way a genuinely separate subprocess argument does. diff --git a/.github/skills/merge-and-release/SKILL.md b/.github/skills/merge-and-release/SKILL.md index c4a71ef2..f2e85831 100644 --- a/.github/skills/merge-and-release/SKILL.md +++ b/.github/skills/merge-and-release/SKILL.md @@ -152,10 +152,12 @@ skill covers all of it, scoped down by what the maintainer actually asks for. branch in a different repository must never pass this check either. Confirm `baseRefName` is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips - only where a remote branch actually exists, - `git ls-remote --heads origin -- ""` empty means it is already gone, most likely a prior - cleanup attempt got interrupted after the remote delete but before the local one, so skip - straight to the local-tip check below and never attempt the remote delete a second time. + only where a remote branch actually exists. `git ls-remote --heads origin -- "refs/heads/"` + is the exact-match form and must be, `--heads origin ""` alone still tail-matches, a + bare `topic/x` pattern also returns an unrelated `other/topic/x` if one exists. Empty means it + is already gone, most likely a prior cleanup attempt got interrupted after the remote delete + but before the local one, so skip straight to the local-tip check below and never attempt the + remote delete a second time. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch or From a14ca6b17bd8b9a7ee51c24d8db7472482f3ad0a Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 20:18:38 -0700 Subject: [PATCH 05/11] Correct the gh pr list --head Matching Claim Copilot review, confirmed: --head matches exactly, not by prefix. Tested directly against this repo with gh 2.97.0: querying a bare prefix of PR 940's real branch name returned nothing. Reworded the headRefName check as cheap defense-in-depth rather than a claimed workaround for partial-match behavior that doesn't happen. Also verified and refuted a suppressed finding claiming 'git push origin --delete --' mistreats -- as a literal refspec: a live delete against a scratch remote (git push origin --delete -- feature-y) correctly deleted the named branch, exit 0, no -- ref ever created. No change needed there. Co-Authored-By: Claude Sonnet 5 --- .agents/skills/merge-and-release/SKILL.md | 7 ++++--- .claude-plugin/fleet-skills/.source-digest | 2 +- .../fleet-skills/skills/merge-and-release/SKILL.md | 7 ++++--- .github/skills/merge-and-release/SKILL.md | 7 ++++--- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/.agents/skills/merge-and-release/SKILL.md b/.agents/skills/merge-and-release/SKILL.md index f2e85831..c400842f 100644 --- a/.agents/skills/merge-and-release/SKILL.md +++ b/.agents/skills/merge-and-release/SKILL.md @@ -145,9 +145,10 @@ skill covers all of it, scoped down by what the maintainer actually asks for. "" --state merged --repo owner/repo --json number,baseRefName,mergedAt,headRefOid,headRefName,headRepository --jq 'if length == 1 then .[0] else error("expected exactly one merged PR for this head, got \(length)") end'`. - `--head` matches by prefix rather than exactly, `gh pr list --help` shows `--head "typo"` as - its own example, so confirm `headRefName` equals `` exactly before trusting anything - else this query returned. Confirm `headRepository` is non-null and its `nameWithOwner` equals + `--head` is expected to match exactly (verified against `gh` 2.97.0 on this repo, a bare + prefix of a real branch name returned nothing), but confirming `headRefName` equals `` + costs one field and is cheap insurance against a future `gh` behavior change, not a workaround + for a known partial-match case. Confirm `headRepository` is non-null and its `nameWithOwner` equals `owner/repo`, the owner alone is not enough, a same-owner PR against an identically named branch in a different repository must never pass this check either. Confirm `baseRefName` is `develop` (a different merged pull request can share the same head branch name against a diff --git a/.claude-plugin/fleet-skills/.source-digest b/.claude-plugin/fleet-skills/.source-digest index f900e657..48468aa2 100644 --- a/.claude-plugin/fleet-skills/.source-digest +++ b/.claude-plugin/fleet-skills/.source-digest @@ -1 +1 @@ -d1db5082e435922b +7f917a20d67ef2df diff --git a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md index f2e85831..c400842f 100644 --- a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md @@ -145,9 +145,10 @@ skill covers all of it, scoped down by what the maintainer actually asks for. "" --state merged --repo owner/repo --json number,baseRefName,mergedAt,headRefOid,headRefName,headRepository --jq 'if length == 1 then .[0] else error("expected exactly one merged PR for this head, got \(length)") end'`. - `--head` matches by prefix rather than exactly, `gh pr list --help` shows `--head "typo"` as - its own example, so confirm `headRefName` equals `` exactly before trusting anything - else this query returned. Confirm `headRepository` is non-null and its `nameWithOwner` equals + `--head` is expected to match exactly (verified against `gh` 2.97.0 on this repo, a bare + prefix of a real branch name returned nothing), but confirming `headRefName` equals `` + costs one field and is cheap insurance against a future `gh` behavior change, not a workaround + for a known partial-match case. Confirm `headRepository` is non-null and its `nameWithOwner` equals `owner/repo`, the owner alone is not enough, a same-owner PR against an identically named branch in a different repository must never pass this check either. Confirm `baseRefName` is `develop` (a different merged pull request can share the same head branch name against a diff --git a/.github/skills/merge-and-release/SKILL.md b/.github/skills/merge-and-release/SKILL.md index f2e85831..c400842f 100644 --- a/.github/skills/merge-and-release/SKILL.md +++ b/.github/skills/merge-and-release/SKILL.md @@ -145,9 +145,10 @@ skill covers all of it, scoped down by what the maintainer actually asks for. "" --state merged --repo owner/repo --json number,baseRefName,mergedAt,headRefOid,headRefName,headRepository --jq 'if length == 1 then .[0] else error("expected exactly one merged PR for this head, got \(length)") end'`. - `--head` matches by prefix rather than exactly, `gh pr list --help` shows `--head "typo"` as - its own example, so confirm `headRefName` equals `` exactly before trusting anything - else this query returned. Confirm `headRepository` is non-null and its `nameWithOwner` equals + `--head` is expected to match exactly (verified against `gh` 2.97.0 on this repo, a bare + prefix of a real branch name returned nothing), but confirming `headRefName` equals `` + costs one field and is cheap insurance against a future `gh` behavior change, not a workaround + for a known partial-match case. Confirm `headRepository` is non-null and its `nameWithOwner` equals `owner/repo`, the owner alone is not enough, a same-owner PR against an identically named branch in a different repository must never pass this check either. Confirm `baseRefName` is `develop` (a different merged pull request can share the same head branch name against a From 54ee8527fbce7bd405c2dffd9c80fb982b79cf8f Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 20:23:43 -0700 Subject: [PATCH 06/11] Correct the Shell-Substitution Wording, Confirmed Misleading CodeRabbit finding, confirmed empirically: quoted variable expansion ("$branch") never re-triggers $() or backtick substitution even when the value literally contains that text, since expansion is a parse-time operation on command-line text, not a runtime scan of an already-expanded value. The prior wording implied quoting itself was insufficient, which is wrong. The actual hazard is a second round of shell parsing (eval, sh -c), reworded to say exactly that. Co-Authored-By: Claude Sonnet 5 --- .agents/skills/drive-pr/SKILL.md | 10 +++++----- .agents/skills/merge-and-release/SKILL.md | 9 +++++---- .claude-plugin/fleet-skills/.source-digest | 2 +- .claude-plugin/fleet-skills/skills/drive-pr/SKILL.md | 10 +++++----- .../fleet-skills/skills/merge-and-release/SKILL.md | 9 +++++---- .github/skills/drive-pr/SKILL.md | 10 +++++----- .github/skills/merge-and-release/SKILL.md | 9 +++++---- 7 files changed, 31 insertions(+), 28 deletions(-) diff --git a/.agents/skills/drive-pr/SKILL.md b/.agents/skills/drive-pr/SKILL.md index c676d32b..d49eebec 100644 --- a/.agents/skills/drive-pr/SKILL.md +++ b/.agents/skills/drive-pr/SKILL.md @@ -71,11 +71,11 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. above, the fully-qualified form since `--heads origin ""` alone still tail-matches a differently-prefixed branch sharing the same suffix. Stop and report a mismatch rather than deleting, someone could have pushed to the branch after the merge, or the name could have been - reused. `` is a quoted argument the caller - substitutes the real value into, never text built into an executed shell command string, a - valid ref can start with `-` or carry a shell metacharacter, and quoting alone does not stop - `$()` or backtick command substitution the way a genuinely separate subprocess argument does. - Only once it matches, `git push origin --delete -- ""`. Never `--force-with-lease` + reused. `` is the real value, substituted as its own quoted argument (a shell variable + expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a + second round of shell parsing, the only way an embedded `$()` or backtick would actually run. + A valid ref can start with `-` or carry a shell metacharacter, which is why it stays quoted + regardless. Only once it matches, `git push origin --delete -- ""`. Never `--force-with-lease` here, git-commit-conventions forbids it unconditionally, this plain verify-then-delete is the safety gate, not a compare-and-swap at delete time. The diff --git a/.agents/skills/merge-and-release/SKILL.md b/.agents/skills/merge-and-release/SKILL.md index c400842f..f648f9f6 100644 --- a/.agents/skills/merge-and-release/SKILL.md +++ b/.agents/skills/merge-and-release/SKILL.md @@ -162,10 +162,11 @@ skill covers all of it, scoped down by what the maintainer actually asks for. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch or - worktree-path placeholder below is a quoted argument the caller substitutes the real value - into, never text built into an executed shell command string, a valid ref can start with `-` - or carry a shell metacharacter, and quoting alone does not stop `$()` or backtick command - substitution the way passing a genuinely separate subprocess argument does. `--` marks the + worktree-path placeholder below is the real value, substituted as its own quoted argument + (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` + or `sh -c` for a second round of shell parsing, the only way an embedded `$()` or backtick + would actually run. A valid ref can start with `-` or carry a shell metacharacter, which is + why it stays quoted regardless. `--` marks the end of options wherever a command supports it. `git merge-base --is-ancestor develop` must never be used for either tip check, a squash merge (drive-pr's own merge method) never makes the feature tip a literal ancestor of diff --git a/.claude-plugin/fleet-skills/.source-digest b/.claude-plugin/fleet-skills/.source-digest index 48468aa2..98a76009 100644 --- a/.claude-plugin/fleet-skills/.source-digest +++ b/.claude-plugin/fleet-skills/.source-digest @@ -1 +1 @@ -7f917a20d67ef2df +8afede887878448e diff --git a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md index c676d32b..d49eebec 100644 --- a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md @@ -71,11 +71,11 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. above, the fully-qualified form since `--heads origin ""` alone still tail-matches a differently-prefixed branch sharing the same suffix. Stop and report a mismatch rather than deleting, someone could have pushed to the branch after the merge, or the name could have been - reused. `` is a quoted argument the caller - substitutes the real value into, never text built into an executed shell command string, a - valid ref can start with `-` or carry a shell metacharacter, and quoting alone does not stop - `$()` or backtick command substitution the way a genuinely separate subprocess argument does. - Only once it matches, `git push origin --delete -- ""`. Never `--force-with-lease` + reused. `` is the real value, substituted as its own quoted argument (a shell variable + expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a + second round of shell parsing, the only way an embedded `$()` or backtick would actually run. + A valid ref can start with `-` or carry a shell metacharacter, which is why it stays quoted + regardless. Only once it matches, `git push origin --delete -- ""`. Never `--force-with-lease` here, git-commit-conventions forbids it unconditionally, this plain verify-then-delete is the safety gate, not a compare-and-swap at delete time. The diff --git a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md index c400842f..f648f9f6 100644 --- a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md @@ -162,10 +162,11 @@ skill covers all of it, scoped down by what the maintainer actually asks for. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch or - worktree-path placeholder below is a quoted argument the caller substitutes the real value - into, never text built into an executed shell command string, a valid ref can start with `-` - or carry a shell metacharacter, and quoting alone does not stop `$()` or backtick command - substitution the way passing a genuinely separate subprocess argument does. `--` marks the + worktree-path placeholder below is the real value, substituted as its own quoted argument + (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` + or `sh -c` for a second round of shell parsing, the only way an embedded `$()` or backtick + would actually run. A valid ref can start with `-` or carry a shell metacharacter, which is + why it stays quoted regardless. `--` marks the end of options wherever a command supports it. `git merge-base --is-ancestor develop` must never be used for either tip check, a squash merge (drive-pr's own merge method) never makes the feature tip a literal ancestor of diff --git a/.github/skills/drive-pr/SKILL.md b/.github/skills/drive-pr/SKILL.md index c676d32b..d49eebec 100644 --- a/.github/skills/drive-pr/SKILL.md +++ b/.github/skills/drive-pr/SKILL.md @@ -71,11 +71,11 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. above, the fully-qualified form since `--heads origin ""` alone still tail-matches a differently-prefixed branch sharing the same suffix. Stop and report a mismatch rather than deleting, someone could have pushed to the branch after the merge, or the name could have been - reused. `` is a quoted argument the caller - substitutes the real value into, never text built into an executed shell command string, a - valid ref can start with `-` or carry a shell metacharacter, and quoting alone does not stop - `$()` or backtick command substitution the way a genuinely separate subprocess argument does. - Only once it matches, `git push origin --delete -- ""`. Never `--force-with-lease` + reused. `` is the real value, substituted as its own quoted argument (a shell variable + expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a + second round of shell parsing, the only way an embedded `$()` or backtick would actually run. + A valid ref can start with `-` or carry a shell metacharacter, which is why it stays quoted + regardless. Only once it matches, `git push origin --delete -- ""`. Never `--force-with-lease` here, git-commit-conventions forbids it unconditionally, this plain verify-then-delete is the safety gate, not a compare-and-swap at delete time. The diff --git a/.github/skills/merge-and-release/SKILL.md b/.github/skills/merge-and-release/SKILL.md index c400842f..f648f9f6 100644 --- a/.github/skills/merge-and-release/SKILL.md +++ b/.github/skills/merge-and-release/SKILL.md @@ -162,10 +162,11 @@ skill covers all of it, scoped down by what the maintainer actually asks for. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch or - worktree-path placeholder below is a quoted argument the caller substitutes the real value - into, never text built into an executed shell command string, a valid ref can start with `-` - or carry a shell metacharacter, and quoting alone does not stop `$()` or backtick command - substitution the way passing a genuinely separate subprocess argument does. `--` marks the + worktree-path placeholder below is the real value, substituted as its own quoted argument + (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` + or `sh -c` for a second round of shell parsing, the only way an embedded `$()` or backtick + would actually run. A valid ref can start with `-` or carry a shell metacharacter, which is + why it stays quoted regardless. `--` marks the end of options wherever a command supports it. `git merge-base --is-ancestor develop` must never be used for either tip check, a squash merge (drive-pr's own merge method) never makes the feature tip a literal ancestor of From 01d707604589317e0a1dbe331161803b236bc8d3 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 20:28:24 -0700 Subject: [PATCH 07/11] Move ls-remote's -- Before origin, Not After CodeRabbit's own scripted probe found this, verified independently: 'git ls-remote --heads origin -- "refs/heads/"' treats -- after origin as an additional ref pattern rather than an end-of-options marker there, so a refs/heads/other/-- ref, if one exists, is also matched. Moving -- before origin ('--heads -- origin "refs/heads/"') returns exactly the one ref intended, confirmed against a scratch repo carrying both refs. Co-Authored-By: Claude Sonnet 5 --- .agents/skills/drive-pr/SKILL.md | 9 ++++++--- .agents/skills/merge-and-release/SKILL.md | 10 +++++++--- .claude-plugin/fleet-skills/.source-digest | 2 +- .claude-plugin/fleet-skills/skills/drive-pr/SKILL.md | 9 ++++++--- .../fleet-skills/skills/merge-and-release/SKILL.md | 10 +++++++--- .github/skills/drive-pr/SKILL.md | 9 ++++++--- .github/skills/merge-and-release/SKILL.md | 10 +++++++--- 7 files changed, 40 insertions(+), 19 deletions(-) diff --git a/.agents/skills/drive-pr/SKILL.md b/.agents/skills/drive-pr/SKILL.md index d49eebec..9c210eb2 100644 --- a/.agents/skills/drive-pr/SKILL.md +++ b/.agents/skills/drive-pr/SKILL.md @@ -67,9 +67,12 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. to delete it, which fails when `develop` is already checked out somewhere else, the ordinary case in this layout. Instead run repo-worktree's post-merge cleanup from the base clone: remove the worktree, delete the now-merged local task branch, then verify before deleting the remote - one, `git ls-remote --heads origin -- "refs/heads/"` matches the `headRefOid` captured - above, the fully-qualified form since `--heads origin ""` alone still tail-matches a - differently-prefixed branch sharing the same suffix. Stop and report a mismatch rather than + one, `git ls-remote --heads -- origin "refs/heads/"` matches the `headRefOid` captured + above, `--` before `origin` and the fully-qualified ref. `--heads origin ""` alone still + tail-matches a differently-prefixed branch sharing the same suffix, and `--` placed after + `origin` instead of before it is not equivalent either, verified empirically against a + `refs/heads/other/--` ref: after-origin also matched it, before-origin matched only the one + intended. Stop and report a mismatch rather than deleting, someone could have pushed to the branch after the merge, or the name could have been reused. `` is the real value, substituted as its own quoted argument (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a diff --git a/.agents/skills/merge-and-release/SKILL.md b/.agents/skills/merge-and-release/SKILL.md index f648f9f6..1062587a 100644 --- a/.agents/skills/merge-and-release/SKILL.md +++ b/.agents/skills/merge-and-release/SKILL.md @@ -153,9 +153,13 @@ skill covers all of it, scoped down by what the maintainer actually asks for. branch in a different repository must never pass this check either. Confirm `baseRefName` is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips - only where a remote branch actually exists. `git ls-remote --heads origin -- "refs/heads/"` - is the exact-match form and must be, `--heads origin ""` alone still tail-matches, a - bare `topic/x` pattern also returns an unrelated `other/topic/x` if one exists. Empty means it + only where a remote branch actually exists. `git ls-remote --heads -- origin "refs/heads/"` + is the exact-match form and must be, in that argument order. `--heads origin ""` alone + still tail-matches, a bare `topic/x` pattern also returns an unrelated `other/topic/x` if one + exists. `--` placed after `origin` instead of before it is not equivalent either, verified + empirically: with a `refs/heads/other/--` ref present, `--heads origin -- "refs/heads/"` + matched both that ref and the intended one, while `--heads -- origin "refs/heads/"` + matched only the one intended. Empty means it is already gone, most likely a prior cleanup attempt got interrupted after the remote delete but before the local one, so skip straight to the local-tip check below and never attempt the remote delete a second time. diff --git a/.claude-plugin/fleet-skills/.source-digest b/.claude-plugin/fleet-skills/.source-digest index 98a76009..1971f9e3 100644 --- a/.claude-plugin/fleet-skills/.source-digest +++ b/.claude-plugin/fleet-skills/.source-digest @@ -1 +1 @@ -8afede887878448e +8ff056f3460d4e62 diff --git a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md index d49eebec..9c210eb2 100644 --- a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md @@ -67,9 +67,12 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. to delete it, which fails when `develop` is already checked out somewhere else, the ordinary case in this layout. Instead run repo-worktree's post-merge cleanup from the base clone: remove the worktree, delete the now-merged local task branch, then verify before deleting the remote - one, `git ls-remote --heads origin -- "refs/heads/"` matches the `headRefOid` captured - above, the fully-qualified form since `--heads origin ""` alone still tail-matches a - differently-prefixed branch sharing the same suffix. Stop and report a mismatch rather than + one, `git ls-remote --heads -- origin "refs/heads/"` matches the `headRefOid` captured + above, `--` before `origin` and the fully-qualified ref. `--heads origin ""` alone still + tail-matches a differently-prefixed branch sharing the same suffix, and `--` placed after + `origin` instead of before it is not equivalent either, verified empirically against a + `refs/heads/other/--` ref: after-origin also matched it, before-origin matched only the one + intended. Stop and report a mismatch rather than deleting, someone could have pushed to the branch after the merge, or the name could have been reused. `` is the real value, substituted as its own quoted argument (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a diff --git a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md index f648f9f6..1062587a 100644 --- a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md @@ -153,9 +153,13 @@ skill covers all of it, scoped down by what the maintainer actually asks for. branch in a different repository must never pass this check either. Confirm `baseRefName` is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips - only where a remote branch actually exists. `git ls-remote --heads origin -- "refs/heads/"` - is the exact-match form and must be, `--heads origin ""` alone still tail-matches, a - bare `topic/x` pattern also returns an unrelated `other/topic/x` if one exists. Empty means it + only where a remote branch actually exists. `git ls-remote --heads -- origin "refs/heads/"` + is the exact-match form and must be, in that argument order. `--heads origin ""` alone + still tail-matches, a bare `topic/x` pattern also returns an unrelated `other/topic/x` if one + exists. `--` placed after `origin` instead of before it is not equivalent either, verified + empirically: with a `refs/heads/other/--` ref present, `--heads origin -- "refs/heads/"` + matched both that ref and the intended one, while `--heads -- origin "refs/heads/"` + matched only the one intended. Empty means it is already gone, most likely a prior cleanup attempt got interrupted after the remote delete but before the local one, so skip straight to the local-tip check below and never attempt the remote delete a second time. diff --git a/.github/skills/drive-pr/SKILL.md b/.github/skills/drive-pr/SKILL.md index d49eebec..9c210eb2 100644 --- a/.github/skills/drive-pr/SKILL.md +++ b/.github/skills/drive-pr/SKILL.md @@ -67,9 +67,12 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. to delete it, which fails when `develop` is already checked out somewhere else, the ordinary case in this layout. Instead run repo-worktree's post-merge cleanup from the base clone: remove the worktree, delete the now-merged local task branch, then verify before deleting the remote - one, `git ls-remote --heads origin -- "refs/heads/"` matches the `headRefOid` captured - above, the fully-qualified form since `--heads origin ""` alone still tail-matches a - differently-prefixed branch sharing the same suffix. Stop and report a mismatch rather than + one, `git ls-remote --heads -- origin "refs/heads/"` matches the `headRefOid` captured + above, `--` before `origin` and the fully-qualified ref. `--heads origin ""` alone still + tail-matches a differently-prefixed branch sharing the same suffix, and `--` placed after + `origin` instead of before it is not equivalent either, verified empirically against a + `refs/heads/other/--` ref: after-origin also matched it, before-origin matched only the one + intended. Stop and report a mismatch rather than deleting, someone could have pushed to the branch after the merge, or the name could have been reused. `` is the real value, substituted as its own quoted argument (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a diff --git a/.github/skills/merge-and-release/SKILL.md b/.github/skills/merge-and-release/SKILL.md index f648f9f6..1062587a 100644 --- a/.github/skills/merge-and-release/SKILL.md +++ b/.github/skills/merge-and-release/SKILL.md @@ -153,9 +153,13 @@ skill covers all of it, scoped down by what the maintainer actually asks for. branch in a different repository must never pass this check either. Confirm `baseRefName` is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips - only where a remote branch actually exists. `git ls-remote --heads origin -- "refs/heads/"` - is the exact-match form and must be, `--heads origin ""` alone still tail-matches, a - bare `topic/x` pattern also returns an unrelated `other/topic/x` if one exists. Empty means it + only where a remote branch actually exists. `git ls-remote --heads -- origin "refs/heads/"` + is the exact-match form and must be, in that argument order. `--heads origin ""` alone + still tail-matches, a bare `topic/x` pattern also returns an unrelated `other/topic/x` if one + exists. `--` placed after `origin` instead of before it is not equivalent either, verified + empirically: with a `refs/heads/other/--` ref present, `--heads origin -- "refs/heads/"` + matched both that ref and the intended one, while `--heads -- origin "refs/heads/"` + matched only the one intended. Empty means it is already gone, most likely a prior cleanup attempt got interrupted after the remote delete but before the local one, so skip straight to the local-tip check below and never attempt the remote delete a second time. From d0bd51a8c8738e9583fd2a8663b3a843d5f06b67 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 20:35:30 -0700 Subject: [PATCH 08/11] Fix rev-parse: -- Breaks Ref Resolution, Use --verify Instead CodeRabbit finding, confirmed empirically: 'git rev-parse -- ""' treats the argument after -- as a path, not a revision, and never resolves a SHA at all (it echoes the arguments back literally rather than erroring, easy to miss). Verified: --verify combined with -- fails outright rather than silently. Dropped --, added --verify, and fully-qualified the ref, which needs no -- regardless since refs/heads/ never itself starts with -. Verified against a ref created via update-ref (bypassing branch's own name validation, which refuses leading-dash names outright). Co-Authored-By: Claude Sonnet 5 --- .agents/skills/merge-and-release/SKILL.md | 6 +++++- .claude-plugin/fleet-skills/.source-digest | 2 +- .../fleet-skills/skills/merge-and-release/SKILL.md | 6 +++++- .github/skills/merge-and-release/SKILL.md | 6 +++++- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/.agents/skills/merge-and-release/SKILL.md b/.agents/skills/merge-and-release/SKILL.md index 1062587a..04c43fe5 100644 --- a/.agents/skills/merge-and-release/SKILL.md +++ b/.agents/skills/merge-and-release/SKILL.md @@ -165,7 +165,11 @@ skill covers all of it, scoped down by what the maintainer actually asks for. remote delete a second time. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local - branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch or + branch tip (`git rev-parse --verify "refs/heads/"`) must also match `headRefOid`. No + `--` here, verified empirically: `git rev-parse -- ""` treats the argument after `--` + as a path rather than a revision and never resolves a SHA at all. The fully-qualified form + needs no `--` regardless, since `refs/heads/` never itself starts with `-`, and + `--verify` fails loudly rather than guessing when it does not resolve. Every branch or worktree-path placeholder below is the real value, substituted as its own quoted argument (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a second round of shell parsing, the only way an embedded `$()` or backtick diff --git a/.claude-plugin/fleet-skills/.source-digest b/.claude-plugin/fleet-skills/.source-digest index 1971f9e3..fee5df56 100644 --- a/.claude-plugin/fleet-skills/.source-digest +++ b/.claude-plugin/fleet-skills/.source-digest @@ -1 +1 @@ -8ff056f3460d4e62 +32cd97ef92191f0f diff --git a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md index 1062587a..04c43fe5 100644 --- a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md @@ -165,7 +165,11 @@ skill covers all of it, scoped down by what the maintainer actually asks for. remote delete a second time. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local - branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch or + branch tip (`git rev-parse --verify "refs/heads/"`) must also match `headRefOid`. No + `--` here, verified empirically: `git rev-parse -- ""` treats the argument after `--` + as a path rather than a revision and never resolves a SHA at all. The fully-qualified form + needs no `--` regardless, since `refs/heads/` never itself starts with `-`, and + `--verify` fails loudly rather than guessing when it does not resolve. Every branch or worktree-path placeholder below is the real value, substituted as its own quoted argument (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a second round of shell parsing, the only way an embedded `$()` or backtick diff --git a/.github/skills/merge-and-release/SKILL.md b/.github/skills/merge-and-release/SKILL.md index 1062587a..04c43fe5 100644 --- a/.github/skills/merge-and-release/SKILL.md +++ b/.github/skills/merge-and-release/SKILL.md @@ -165,7 +165,11 @@ skill covers all of it, scoped down by what the maintainer actually asks for. remote delete a second time. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local - branch tip (`git rev-parse -- ""`) must also match `headRefOid`. Every branch or + branch tip (`git rev-parse --verify "refs/heads/"`) must also match `headRefOid`. No + `--` here, verified empirically: `git rev-parse -- ""` treats the argument after `--` + as a path rather than a revision and never resolves a SHA at all. The fully-qualified form + needs no `--` regardless, since `refs/heads/` never itself starts with `-`, and + `--verify` fails loudly rather than guessing when it does not resolve. Every branch or worktree-path placeholder below is the real value, substituted as its own quoted argument (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a second round of shell parsing, the only way an embedded `$()` or backtick From 11240dd34123c18203e0607f38a39a77178898d7 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 20:40:29 -0700 Subject: [PATCH 09/11] Use --exit-code for a Tri-State ls-remote Result CodeRabbit finding, confirmed empirically: a failed query (bad remote, network error) and a genuinely absent branch both print nothing to stdout, so reading 'empty' alone as 'branch gone' cannot tell them apart, only the exit code can. Verified: --exit-code gives exit 2 specifically for a real no-match, versus a different nonzero exit for a failed query. Reworded merge-and-release's check to name the tri-state explicitly (2 = gone, 0 = matched, anything else = stop and report). drive-pr's existing 'no match, don't delete' default was already safe either way, added --exit-code there too for the same diagnostic precision on a failure rather than a changed outcome. Co-Authored-By: Claude Sonnet 5 --- .agents/skills/drive-pr/SKILL.md | 17 +++++++------- .agents/skills/merge-and-release/SKILL.md | 23 +++++++++++-------- .claude-plugin/fleet-skills/.source-digest | 2 +- .../fleet-skills/skills/drive-pr/SKILL.md | 17 +++++++------- .../skills/merge-and-release/SKILL.md | 23 +++++++++++-------- .github/skills/drive-pr/SKILL.md | 17 +++++++------- .github/skills/merge-and-release/SKILL.md | 23 +++++++++++-------- 7 files changed, 67 insertions(+), 55 deletions(-) diff --git a/.agents/skills/drive-pr/SKILL.md b/.agents/skills/drive-pr/SKILL.md index 9c210eb2..ee5a4553 100644 --- a/.agents/skills/drive-pr/SKILL.md +++ b/.agents/skills/drive-pr/SKILL.md @@ -67,14 +67,15 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. to delete it, which fails when `develop` is already checked out somewhere else, the ordinary case in this layout. Instead run repo-worktree's post-merge cleanup from the base clone: remove the worktree, delete the now-merged local task branch, then verify before deleting the remote - one, `git ls-remote --heads -- origin "refs/heads/"` matches the `headRefOid` captured - above, `--` before `origin` and the fully-qualified ref. `--heads origin ""` alone still - tail-matches a differently-prefixed branch sharing the same suffix, and `--` placed after - `origin` instead of before it is not equivalent either, verified empirically against a - `refs/heads/other/--` ref: after-origin also matched it, before-origin matched only the one - intended. Stop and report a mismatch rather than - deleting, someone could have pushed to the branch after the merge, or the name could have been - reused. `` is the real value, substituted as its own quoted argument (a shell variable + one, `git ls-remote --heads --exit-code -- origin "refs/heads/"` matches the + `headRefOid` captured above, `--` before `origin` and the fully-qualified ref. `--heads origin + ""` alone still tail-matches a differently-prefixed branch sharing the same suffix, and + `--` placed after `origin` instead of before it is not equivalent either, verified empirically + against a `refs/heads/other/--` ref: after-origin also matched it, before-origin matched only + the one intended. `--exit-code` distinguishes exit `2`, branch genuinely gone, from any other + non-zero exit, a failed query, an unreachable remote and a gone branch both print nothing to + stdout otherwise. Stop and report either a mismatch or a failed query rather than deleting, + someone could have pushed to the branch after the merge, or the name could have been reused. `` is the real value, substituted as its own quoted argument (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a second round of shell parsing, the only way an embedded `$()` or backtick would actually run. A valid ref can start with `-` or carry a shell metacharacter, which is why it stays quoted diff --git a/.agents/skills/merge-and-release/SKILL.md b/.agents/skills/merge-and-release/SKILL.md index 04c43fe5..84b6b965 100644 --- a/.agents/skills/merge-and-release/SKILL.md +++ b/.agents/skills/merge-and-release/SKILL.md @@ -153,16 +153,19 @@ skill covers all of it, scoped down by what the maintainer actually asks for. branch in a different repository must never pass this check either. Confirm `baseRefName` is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips - only where a remote branch actually exists. `git ls-remote --heads -- origin "refs/heads/"` - is the exact-match form and must be, in that argument order. `--heads origin ""` alone - still tail-matches, a bare `topic/x` pattern also returns an unrelated `other/topic/x` if one - exists. `--` placed after `origin` instead of before it is not equivalent either, verified - empirically: with a `refs/heads/other/--` ref present, `--heads origin -- "refs/heads/"` - matched both that ref and the intended one, while `--heads -- origin "refs/heads/"` - matched only the one intended. Empty means it - is already gone, most likely a prior cleanup attempt got interrupted after the remote delete - but before the local one, so skip straight to the local-tip check below and never attempt the - remote delete a second time. + only where a remote branch actually exists. `git ls-remote --heads --exit-code -- origin + "refs/heads/"` is the exact-match form and must be, in that argument order. `--heads + origin ""` alone still tail-matches, a bare `topic/x` pattern also returns an unrelated + `other/topic/x` if one exists. `--` placed after `origin` instead of before it is not + equivalent either, verified empirically: with a `refs/heads/other/--` ref present, `--heads + origin -- "refs/heads/"` matched both that ref and the intended one, while `--heads -- + origin "refs/heads/"` matched only the one intended. Exit status is a tri-state, not a + stdin-emptiness check: `--exit-code` makes exit `2` mean query succeeded, branch gone, most + likely a prior cleanup attempt got interrupted after the remote delete but before the local + one, so skip straight to the local-tip check below and never attempt the remote delete a + second time. Exit `0` means it matched. Anything else is a failed query, a network or auth + problem, and stops and reports rather than being read as absence, an unreachable remote and a + genuinely gone branch both print nothing to stdout, only the exit code tells them apart. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local branch tip (`git rev-parse --verify "refs/heads/"`) must also match `headRefOid`. No diff --git a/.claude-plugin/fleet-skills/.source-digest b/.claude-plugin/fleet-skills/.source-digest index fee5df56..cff09b27 100644 --- a/.claude-plugin/fleet-skills/.source-digest +++ b/.claude-plugin/fleet-skills/.source-digest @@ -1 +1 @@ -32cd97ef92191f0f +0ef08dd0fbbcedcc diff --git a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md index 9c210eb2..ee5a4553 100644 --- a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md @@ -67,14 +67,15 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. to delete it, which fails when `develop` is already checked out somewhere else, the ordinary case in this layout. Instead run repo-worktree's post-merge cleanup from the base clone: remove the worktree, delete the now-merged local task branch, then verify before deleting the remote - one, `git ls-remote --heads -- origin "refs/heads/"` matches the `headRefOid` captured - above, `--` before `origin` and the fully-qualified ref. `--heads origin ""` alone still - tail-matches a differently-prefixed branch sharing the same suffix, and `--` placed after - `origin` instead of before it is not equivalent either, verified empirically against a - `refs/heads/other/--` ref: after-origin also matched it, before-origin matched only the one - intended. Stop and report a mismatch rather than - deleting, someone could have pushed to the branch after the merge, or the name could have been - reused. `` is the real value, substituted as its own quoted argument (a shell variable + one, `git ls-remote --heads --exit-code -- origin "refs/heads/"` matches the + `headRefOid` captured above, `--` before `origin` and the fully-qualified ref. `--heads origin + ""` alone still tail-matches a differently-prefixed branch sharing the same suffix, and + `--` placed after `origin` instead of before it is not equivalent either, verified empirically + against a `refs/heads/other/--` ref: after-origin also matched it, before-origin matched only + the one intended. `--exit-code` distinguishes exit `2`, branch genuinely gone, from any other + non-zero exit, a failed query, an unreachable remote and a gone branch both print nothing to + stdout otherwise. Stop and report either a mismatch or a failed query rather than deleting, + someone could have pushed to the branch after the merge, or the name could have been reused. `` is the real value, substituted as its own quoted argument (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a second round of shell parsing, the only way an embedded `$()` or backtick would actually run. A valid ref can start with `-` or carry a shell metacharacter, which is why it stays quoted diff --git a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md index 04c43fe5..84b6b965 100644 --- a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md @@ -153,16 +153,19 @@ skill covers all of it, scoped down by what the maintainer actually asks for. branch in a different repository must never pass this check either. Confirm `baseRefName` is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips - only where a remote branch actually exists. `git ls-remote --heads -- origin "refs/heads/"` - is the exact-match form and must be, in that argument order. `--heads origin ""` alone - still tail-matches, a bare `topic/x` pattern also returns an unrelated `other/topic/x` if one - exists. `--` placed after `origin` instead of before it is not equivalent either, verified - empirically: with a `refs/heads/other/--` ref present, `--heads origin -- "refs/heads/"` - matched both that ref and the intended one, while `--heads -- origin "refs/heads/"` - matched only the one intended. Empty means it - is already gone, most likely a prior cleanup attempt got interrupted after the remote delete - but before the local one, so skip straight to the local-tip check below and never attempt the - remote delete a second time. + only where a remote branch actually exists. `git ls-remote --heads --exit-code -- origin + "refs/heads/"` is the exact-match form and must be, in that argument order. `--heads + origin ""` alone still tail-matches, a bare `topic/x` pattern also returns an unrelated + `other/topic/x` if one exists. `--` placed after `origin` instead of before it is not + equivalent either, verified empirically: with a `refs/heads/other/--` ref present, `--heads + origin -- "refs/heads/"` matched both that ref and the intended one, while `--heads -- + origin "refs/heads/"` matched only the one intended. Exit status is a tri-state, not a + stdin-emptiness check: `--exit-code` makes exit `2` mean query succeeded, branch gone, most + likely a prior cleanup attempt got interrupted after the remote delete but before the local + one, so skip straight to the local-tip check below and never attempt the remote delete a + second time. Exit `0` means it matched. Anything else is a failed query, a network or auth + problem, and stops and reports rather than being read as absence, an unreachable remote and a + genuinely gone branch both print nothing to stdout, only the exit code tells them apart. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local branch tip (`git rev-parse --verify "refs/heads/"`) must also match `headRefOid`. No diff --git a/.github/skills/drive-pr/SKILL.md b/.github/skills/drive-pr/SKILL.md index 9c210eb2..ee5a4553 100644 --- a/.github/skills/drive-pr/SKILL.md +++ b/.github/skills/drive-pr/SKILL.md @@ -67,14 +67,15 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. to delete it, which fails when `develop` is already checked out somewhere else, the ordinary case in this layout. Instead run repo-worktree's post-merge cleanup from the base clone: remove the worktree, delete the now-merged local task branch, then verify before deleting the remote - one, `git ls-remote --heads -- origin "refs/heads/"` matches the `headRefOid` captured - above, `--` before `origin` and the fully-qualified ref. `--heads origin ""` alone still - tail-matches a differently-prefixed branch sharing the same suffix, and `--` placed after - `origin` instead of before it is not equivalent either, verified empirically against a - `refs/heads/other/--` ref: after-origin also matched it, before-origin matched only the one - intended. Stop and report a mismatch rather than - deleting, someone could have pushed to the branch after the merge, or the name could have been - reused. `` is the real value, substituted as its own quoted argument (a shell variable + one, `git ls-remote --heads --exit-code -- origin "refs/heads/"` matches the + `headRefOid` captured above, `--` before `origin` and the fully-qualified ref. `--heads origin + ""` alone still tail-matches a differently-prefixed branch sharing the same suffix, and + `--` placed after `origin` instead of before it is not equivalent either, verified empirically + against a `refs/heads/other/--` ref: after-origin also matched it, before-origin matched only + the one intended. `--exit-code` distinguishes exit `2`, branch genuinely gone, from any other + non-zero exit, a failed query, an unreachable remote and a gone branch both print nothing to + stdout otherwise. Stop and report either a mismatch or a failed query rather than deleting, + someone could have pushed to the branch after the merge, or the name could have been reused. `` is the real value, substituted as its own quoted argument (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a second round of shell parsing, the only way an embedded `$()` or backtick would actually run. A valid ref can start with `-` or carry a shell metacharacter, which is why it stays quoted diff --git a/.github/skills/merge-and-release/SKILL.md b/.github/skills/merge-and-release/SKILL.md index 04c43fe5..84b6b965 100644 --- a/.github/skills/merge-and-release/SKILL.md +++ b/.github/skills/merge-and-release/SKILL.md @@ -153,16 +153,19 @@ skill covers all of it, scoped down by what the maintainer actually asks for. branch in a different repository must never pass this check either. Confirm `baseRefName` is `develop` (a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips - only where a remote branch actually exists. `git ls-remote --heads -- origin "refs/heads/"` - is the exact-match form and must be, in that argument order. `--heads origin ""` alone - still tail-matches, a bare `topic/x` pattern also returns an unrelated `other/topic/x` if one - exists. `--` placed after `origin` instead of before it is not equivalent either, verified - empirically: with a `refs/heads/other/--` ref present, `--heads origin -- "refs/heads/"` - matched both that ref and the intended one, while `--heads -- origin "refs/heads/"` - matched only the one intended. Empty means it - is already gone, most likely a prior cleanup attempt got interrupted after the remote delete - but before the local one, so skip straight to the local-tip check below and never attempt the - remote delete a second time. + only where a remote branch actually exists. `git ls-remote --heads --exit-code -- origin + "refs/heads/"` is the exact-match form and must be, in that argument order. `--heads + origin ""` alone still tail-matches, a bare `topic/x` pattern also returns an unrelated + `other/topic/x` if one exists. `--` placed after `origin` instead of before it is not + equivalent either, verified empirically: with a `refs/heads/other/--` ref present, `--heads + origin -- "refs/heads/"` matched both that ref and the intended one, while `--heads -- + origin "refs/heads/"` matched only the one intended. Exit status is a tri-state, not a + stdin-emptiness check: `--exit-code` makes exit `2` mean query succeeded, branch gone, most + likely a prior cleanup attempt got interrupted after the remote delete but before the local + one, so skip straight to the local-tip check below and never attempt the remote delete a + second time. Exit `0` means it matched. Anything else is a failed query, a network or auth + problem, and stops and reports rather than being read as absence, an unreachable remote and a + genuinely gone branch both print nothing to stdout, only the exit code tells them apart. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` before either delete proceeds, proving nothing landed on it since. Either way, the local branch tip (`git rev-parse --verify "refs/heads/"`) must also match `headRefOid`. No From 25213905b3856051a095909bebef46d55ce5c502 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 20:46:24 -0700 Subject: [PATCH 10/11] Fix Line Wrap in drive-pr Cleanup Step Copilot review, confirmed: an earlier edit merged two sentences onto one over-length line, breaking the surrounding wrap width. Rewrapped. Co-Authored-By: Claude Sonnet 5 --- .agents/skills/drive-pr/SKILL.md | 7 ++++--- .claude-plugin/fleet-skills/.source-digest | 2 +- .claude-plugin/fleet-skills/skills/drive-pr/SKILL.md | 7 ++++--- .github/skills/drive-pr/SKILL.md | 7 ++++--- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/.agents/skills/drive-pr/SKILL.md b/.agents/skills/drive-pr/SKILL.md index ee5a4553..b52c5522 100644 --- a/.agents/skills/drive-pr/SKILL.md +++ b/.agents/skills/drive-pr/SKILL.md @@ -75,12 +75,13 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. the one intended. `--exit-code` distinguishes exit `2`, branch genuinely gone, from any other non-zero exit, a failed query, an unreachable remote and a gone branch both print nothing to stdout otherwise. Stop and report either a mismatch or a failed query rather than deleting, - someone could have pushed to the branch after the merge, or the name could have been reused. `` is the real value, substituted as its own quoted argument (a shell variable + someone could have pushed to the branch after the merge, or the name could have been reused. + `` is the real value, substituted as its own quoted argument (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a second round of shell parsing, the only way an embedded `$()` or backtick would actually run. A valid ref can start with `-` or carry a shell metacharacter, which is why it stays quoted - regardless. Only once it matches, `git push origin --delete -- ""`. Never `--force-with-lease` - here, git-commit-conventions forbids it + regardless. Only once it matches, `git push origin --delete -- ""`. Never + `--force-with-lease` here, git-commit-conventions forbids it unconditionally, this plain verify-then-delete is the safety gate, not a compare-and-swap at delete time. The repo's auto-delete-head-branches setting is kept off fleet-wide (to protect `develop` and diff --git a/.claude-plugin/fleet-skills/.source-digest b/.claude-plugin/fleet-skills/.source-digest index cff09b27..7ed8dec9 100644 --- a/.claude-plugin/fleet-skills/.source-digest +++ b/.claude-plugin/fleet-skills/.source-digest @@ -1 +1 @@ -0ef08dd0fbbcedcc +b810a8fcc30a2915 diff --git a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md index ee5a4553..b52c5522 100644 --- a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md @@ -75,12 +75,13 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. the one intended. `--exit-code` distinguishes exit `2`, branch genuinely gone, from any other non-zero exit, a failed query, an unreachable remote and a gone branch both print nothing to stdout otherwise. Stop and report either a mismatch or a failed query rather than deleting, - someone could have pushed to the branch after the merge, or the name could have been reused. `` is the real value, substituted as its own quoted argument (a shell variable + someone could have pushed to the branch after the merge, or the name could have been reused. + `` is the real value, substituted as its own quoted argument (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a second round of shell parsing, the only way an embedded `$()` or backtick would actually run. A valid ref can start with `-` or carry a shell metacharacter, which is why it stays quoted - regardless. Only once it matches, `git push origin --delete -- ""`. Never `--force-with-lease` - here, git-commit-conventions forbids it + regardless. Only once it matches, `git push origin --delete -- ""`. Never + `--force-with-lease` here, git-commit-conventions forbids it unconditionally, this plain verify-then-delete is the safety gate, not a compare-and-swap at delete time. The repo's auto-delete-head-branches setting is kept off fleet-wide (to protect `develop` and diff --git a/.github/skills/drive-pr/SKILL.md b/.github/skills/drive-pr/SKILL.md index ee5a4553..b52c5522 100644 --- a/.github/skills/drive-pr/SKILL.md +++ b/.github/skills/drive-pr/SKILL.md @@ -75,12 +75,13 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent. the one intended. `--exit-code` distinguishes exit `2`, branch genuinely gone, from any other non-zero exit, a failed query, an unreachable remote and a gone branch both print nothing to stdout otherwise. Stop and report either a mismatch or a failed query rather than deleting, - someone could have pushed to the branch after the merge, or the name could have been reused. `` is the real value, substituted as its own quoted argument (a shell variable + someone could have pushed to the branch after the merge, or the name could have been reused. + `` is the real value, substituted as its own quoted argument (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a second round of shell parsing, the only way an embedded `$()` or backtick would actually run. A valid ref can start with `-` or carry a shell metacharacter, which is why it stays quoted - regardless. Only once it matches, `git push origin --delete -- ""`. Never `--force-with-lease` - here, git-commit-conventions forbids it + regardless. Only once it matches, `git push origin --delete -- ""`. Never + `--force-with-lease` here, git-commit-conventions forbids it unconditionally, this plain verify-then-delete is the safety gate, not a compare-and-swap at delete time. The repo's auto-delete-head-branches setting is kept off fleet-wide (to protect `develop` and From 77c045a5ccc38617675908f958acfede6871d1e1 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 20:53:39 -0700 Subject: [PATCH 11/11] Allow Remote-Only Cleanup Recovery in the Defensive Sweep CodeRabbit finding, confirmed: 'Either way, the local branch tip must also match headRefOid' made local-branch verification unconditionally required, so an interrupted prior cleanup that already deleted the local branch and left only a stray remote one could never be recovered, the local rev-parse --verify would fail first and block the whole sweep from reaching the remote delete. Made both sides independently conditional on actually existing, neither blocks the other's cleanup on its own absence, matching the symmetry the tri-state ls-remote exit-code check already established for the remote side alone. Co-Authored-By: Claude Sonnet 5 --- .agents/skills/merge-and-release/SKILL.md | 23 +++++++++++-------- .claude-plugin/fleet-skills/.source-digest | 2 +- .../skills/merge-and-release/SKILL.md | 23 +++++++++++-------- .github/skills/merge-and-release/SKILL.md | 23 +++++++++++-------- 4 files changed, 43 insertions(+), 28 deletions(-) diff --git a/.agents/skills/merge-and-release/SKILL.md b/.agents/skills/merge-and-release/SKILL.md index 84b6b965..0ae097f2 100644 --- a/.agents/skills/merge-and-release/SKILL.md +++ b/.agents/skills/merge-and-release/SKILL.md @@ -167,12 +167,16 @@ skill covers all of it, scoped down by what the maintainer actually asks for. problem, and stops and reports rather than being read as absence, an unreachable remote and a genuinely gone branch both print nothing to stdout, only the exit code tells them apart. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` - before either delete proceeds, proving nothing landed on it since. Either way, the local - branch tip (`git rev-parse --verify "refs/heads/"`) must also match `headRefOid`. No - `--` here, verified empirically: `git rev-parse -- ""` treats the argument after `--` - as a path rather than a revision and never resolves a SHA at all. The fully-qualified form - needs no `--` regardless, since `refs/heads/` never itself starts with `-`, and - `--verify` fails loudly rather than guessing when it does not resolve. Every branch or + before its own delete proceeds, proving nothing landed on it since. Where a local branch + still exists too, its tip (`git rev-parse --verify "refs/heads/"`) must independently + match `headRefOid` before its own delete proceeds. Neither side needs the other to exist, a + prior interrupted attempt may have deleted one side already and left only the other, so + verify and delete whichever side is still there and skip whichever already is not, never + block one side's cleanup on the other side's absence. No `--` on `rev-parse`, verified + empirically: `git rev-parse -- ""` treats the argument after `--` as a path rather + than a revision and never resolves a SHA at all. The fully-qualified form needs no `--` + regardless, since `refs/heads/` never itself starts with `-`, and `--verify` fails + loudly rather than guessing when it does not resolve. Every branch or worktree-path placeholder below is the real value, substituted as its own quoted argument (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a second round of shell parsing, the only way an embedded `$()` or backtick @@ -182,9 +186,10 @@ skill covers all of it, scoped down by what the maintainer actually asks for. `git merge-base --is-ancestor develop` must never be used for either tip check, a squash merge (drive-pr's own merge method) never makes the feature tip a literal ancestor of `develop`, so the check reports every already-finished branch as unmerged. Only once GitHub - confirms it, and the worktree is clean (a dirty worktree stops cleanup rather than discarding - uncommitted work), remove the worktree by its exact path, `git worktree remove - ""`, `git worktree list` names it, then delete the local branch. `git branch + confirms it, and only when a local worktree or branch is still there to remove, remove the + worktree by its exact path (a dirty worktree stops cleanup rather than discarding uncommitted + work), `git worktree remove ""`, `git worktree list` names it, then delete the + local branch. `git branch -d` has the identical squash blindness as `git merge-base --is-ancestor` and refuses too, so use `git branch -D -- ""` here, safe only because the GitHub-state check just proved that exact branch finished, the narrow post-squash exception git-commit-conventions diff --git a/.claude-plugin/fleet-skills/.source-digest b/.claude-plugin/fleet-skills/.source-digest index 7ed8dec9..6baf2545 100644 --- a/.claude-plugin/fleet-skills/.source-digest +++ b/.claude-plugin/fleet-skills/.source-digest @@ -1 +1 @@ -b810a8fcc30a2915 +1b7f99942fd01422 diff --git a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md index 84b6b965..0ae097f2 100644 --- a/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md @@ -167,12 +167,16 @@ skill covers all of it, scoped down by what the maintainer actually asks for. problem, and stops and reports rather than being read as absence, an unreachable remote and a genuinely gone branch both print nothing to stdout, only the exit code tells them apart. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` - before either delete proceeds, proving nothing landed on it since. Either way, the local - branch tip (`git rev-parse --verify "refs/heads/"`) must also match `headRefOid`. No - `--` here, verified empirically: `git rev-parse -- ""` treats the argument after `--` - as a path rather than a revision and never resolves a SHA at all. The fully-qualified form - needs no `--` regardless, since `refs/heads/` never itself starts with `-`, and - `--verify` fails loudly rather than guessing when it does not resolve. Every branch or + before its own delete proceeds, proving nothing landed on it since. Where a local branch + still exists too, its tip (`git rev-parse --verify "refs/heads/"`) must independently + match `headRefOid` before its own delete proceeds. Neither side needs the other to exist, a + prior interrupted attempt may have deleted one side already and left only the other, so + verify and delete whichever side is still there and skip whichever already is not, never + block one side's cleanup on the other side's absence. No `--` on `rev-parse`, verified + empirically: `git rev-parse -- ""` treats the argument after `--` as a path rather + than a revision and never resolves a SHA at all. The fully-qualified form needs no `--` + regardless, since `refs/heads/` never itself starts with `-`, and `--verify` fails + loudly rather than guessing when it does not resolve. Every branch or worktree-path placeholder below is the real value, substituted as its own quoted argument (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a second round of shell parsing, the only way an embedded `$()` or backtick @@ -182,9 +186,10 @@ skill covers all of it, scoped down by what the maintainer actually asks for. `git merge-base --is-ancestor develop` must never be used for either tip check, a squash merge (drive-pr's own merge method) never makes the feature tip a literal ancestor of `develop`, so the check reports every already-finished branch as unmerged. Only once GitHub - confirms it, and the worktree is clean (a dirty worktree stops cleanup rather than discarding - uncommitted work), remove the worktree by its exact path, `git worktree remove - ""`, `git worktree list` names it, then delete the local branch. `git branch + confirms it, and only when a local worktree or branch is still there to remove, remove the + worktree by its exact path (a dirty worktree stops cleanup rather than discarding uncommitted + work), `git worktree remove ""`, `git worktree list` names it, then delete the + local branch. `git branch -d` has the identical squash blindness as `git merge-base --is-ancestor` and refuses too, so use `git branch -D -- ""` here, safe only because the GitHub-state check just proved that exact branch finished, the narrow post-squash exception git-commit-conventions diff --git a/.github/skills/merge-and-release/SKILL.md b/.github/skills/merge-and-release/SKILL.md index 84b6b965..0ae097f2 100644 --- a/.github/skills/merge-and-release/SKILL.md +++ b/.github/skills/merge-and-release/SKILL.md @@ -167,12 +167,16 @@ skill covers all of it, scoped down by what the maintainer actually asks for. problem, and stops and reports rather than being read as absence, an unreachable remote and a genuinely gone branch both print nothing to stdout, only the exit code tells them apart. Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` - before either delete proceeds, proving nothing landed on it since. Either way, the local - branch tip (`git rev-parse --verify "refs/heads/"`) must also match `headRefOid`. No - `--` here, verified empirically: `git rev-parse -- ""` treats the argument after `--` - as a path rather than a revision and never resolves a SHA at all. The fully-qualified form - needs no `--` regardless, since `refs/heads/` never itself starts with `-`, and - `--verify` fails loudly rather than guessing when it does not resolve. Every branch or + before its own delete proceeds, proving nothing landed on it since. Where a local branch + still exists too, its tip (`git rev-parse --verify "refs/heads/"`) must independently + match `headRefOid` before its own delete proceeds. Neither side needs the other to exist, a + prior interrupted attempt may have deleted one side already and left only the other, so + verify and delete whichever side is still there and skip whichever already is not, never + block one side's cleanup on the other side's absence. No `--` on `rev-parse`, verified + empirically: `git rev-parse -- ""` treats the argument after `--` as a path rather + than a revision and never resolves a SHA at all. The fully-qualified form needs no `--` + regardless, since `refs/heads/` never itself starts with `-`, and `--verify` fails + loudly rather than guessing when it does not resolve. Every branch or worktree-path placeholder below is the real value, substituted as its own quoted argument (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a second round of shell parsing, the only way an embedded `$()` or backtick @@ -182,9 +186,10 @@ skill covers all of it, scoped down by what the maintainer actually asks for. `git merge-base --is-ancestor develop` must never be used for either tip check, a squash merge (drive-pr's own merge method) never makes the feature tip a literal ancestor of `develop`, so the check reports every already-finished branch as unmerged. Only once GitHub - confirms it, and the worktree is clean (a dirty worktree stops cleanup rather than discarding - uncommitted work), remove the worktree by its exact path, `git worktree remove - ""`, `git worktree list` names it, then delete the local branch. `git branch + confirms it, and only when a local worktree or branch is still there to remove, remove the + worktree by its exact path (a dirty worktree stops cleanup rather than discarding uncommitted + work), `git worktree remove ""`, `git worktree list` names it, then delete the + local branch. `git branch -d` has the identical squash blindness as `git merge-base --is-ancestor` and refuses too, so use `git branch -D -- ""` here, safe only because the GitHub-state check just proved that exact branch finished, the narrow post-squash exception git-commit-conventions