diff --git a/.agents/skills/drive-pr/SKILL.md b/.agents/skills/drive-pr/SKILL.md index 37daa859..b52c5522 100644 --- a/.agents/skills/drive-pr/SKILL.md +++ b/.agents/skills/drive-pr/SKILL.md @@ -67,11 +67,23 @@ 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 --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 + 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 `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..0ae097f2 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,31 +141,60 @@ 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 - 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` + 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,headRefName,headRepository --jq 'if length == 1 then + .[0] else error("expected exactly one merged PR for this head, got \(length)") end'`. + `--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 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 --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 `) must also match `headRefOid`. + 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 + 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 `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 `. + 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 + 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..6baf2545 100644 --- a/.claude-plugin/fleet-skills/.source-digest +++ b/.claude-plugin/fleet-skills/.source-digest @@ -1 +1 @@ -4259e0af969b1580 +1b7f99942fd01422 diff --git a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md index 37daa859..b52c5522 100644 --- a/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md @@ -67,11 +67,23 @@ 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 --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 + 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 `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..0ae097f2 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,31 +141,60 @@ 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 - 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` + 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,headRefName,headRepository --jq 'if length == 1 then + .[0] else error("expected exactly one merged PR for this head, got \(length)") end'`. + `--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 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 --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 `) must also match `headRefOid`. + 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 + 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 `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 `. + 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 + 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..b52c5522 100644 --- a/.github/skills/drive-pr/SKILL.md +++ b/.github/skills/drive-pr/SKILL.md @@ -67,11 +67,23 @@ 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 --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 + 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 `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..0ae097f2 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,31 +141,60 @@ 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 - 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` + 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,headRefName,headRepository --jq 'if length == 1 then + .[0] else error("expected exactly one merged PR for this head, got \(length)") end'`. + `--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 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 --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 `) must also match `headRefOid`. + 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 + 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 `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 `. + 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 + 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`