Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions .agents/skills/drive-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <branch>` 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
<branch>`. 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/<branch>"` matches the
`headRefOid` captured above, `--` before `origin` and the fully-qualified ref. `--heads origin
"<branch>"` 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.
`<branch>` 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 -- "<branch>"`. 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.
Expand Down
87 changes: 60 additions & 27 deletions .agents/skills/merge-and-release/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
ptr727 marked this conversation as resolved.
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
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`.
Expand All @@ -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
Expand All @@ -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 <branch>
--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
"<branch>" --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 `<branch>`
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
Comment thread
coderabbitai[bot] marked this conversation as resolved.
only where a remote branch actually exists,
`git ls-remote --heads origin <branch>` 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/<branch>"` is the exact-match form and must be, in that argument order. `--heads
origin "<branch>"` 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/<branch>"` matched both that ref and the intended one, while `--heads --
origin "refs/heads/<branch>"` 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 <branch>`) 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/<branch>"`) 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 -- "<branch>"` 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/<branch>` 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 <branch> 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 <exact-branch>` 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 <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 "<worktree-path>"`, `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 -- "<exact-branch>"` 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 -- "<branch>"`.
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`
Expand Down
2 changes: 1 addition & 1 deletion .claude-plugin/fleet-skills/.source-digest
Original file line number Diff line number Diff line change
@@ -1 +1 @@
4259e0af969b1580
1b7f99942fd01422
22 changes: 17 additions & 5 deletions .claude-plugin/fleet-skills/skills/drive-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <branch>` 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
<branch>`. 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/<branch>"` matches the
`headRefOid` captured above, `--` before `origin` and the fully-qualified ref. `--heads origin
"<branch>"` 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.
`<branch>` 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 -- "<branch>"`. 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.
Expand Down
Loading