From f471b63640016574e22a8a4f607f0b9b902faf1a Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 21 Jul 2026 14:07:08 -0400 Subject: [PATCH 01/10] fix(source-control): assert babysit worktree HEAD == PR head before mutate; merge-only, upstream refspec push MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A babysit worker can be assigned a worktree in detached HEAD (its PR branch checked out in a sibling/foreign worktree) or on a stale local branch tip; the checkout/freshness mechanics then merged and pushed from that tip, so a stale-tip integration could silently revert the newest branch commit โ€” a near-miss where safety depended on the assigned HEAD happening to match, not a guard (#548). Fix the safety.md contract AND the worker mechanics that implement it: - Assert the worktree HEAD equals the true PR head (`gh pr view --json headRefOid`, authoritative for same-repo and fork PRs) before any merge/edit/push; stop on a stale/detached mismatch. safety.md Checkout And Push Invariants + loop.md ยง5.1.2 (acquire the head via `gh pr checkout`, `--detach` when the branch is locked in a sibling worktree) + SKILL.md Step 0.2 + orchestration.md's conflict-worker all follow it, degrading to read-only on mismatch. - Push by refspec to the branch's CONFIGURED UPSTREAM โ€” `git push "$(git config --get branch..remote)" HEAD:` โ€” which resolves to origin for a same-repo head and to the fork's remote for a write-allowed cross-repo (in-owner fork) head. Never hardcode origin: for a fork head that writes a same-named branch on the base repo instead of the fork head. Fast-forward given the head assertion, never force. - Freshness is now MERGE-ONLY. The prior loop.md rebase + --force-with-lease path violated the skill's own never-force-push invariant (safety.md "Never Do Automatically", orchestration.md) and was the silent-revert vector. Behavior change: linear-history branches now carry an interim merge commit during freshness instead of being rebased (the final squash merge still flattens history). evals.json id 6 updated to the merge-only + upstream refspec contract. Enforcement remains agent discipline; a deterministic push-safety guard is tracked in #885. Per-plugin bump 0.15.9 -> 0.15.10 (patch) + CHANGELOG. Closes #548 Refs #885 ๐Ÿค– Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 --- .../source-control/.claude-plugin/plugin.json | 2 +- plugins/source-control/CHANGELOG.md | 31 ++++ .../skills/babysit-prs/SKILL.md | 10 +- .../skills/babysit-prs/evals/evals.json | 12 +- .../skills/babysit-prs/reference/loop.md | 161 +++++++++--------- .../babysit-prs/reference/orchestration.md | 17 +- .../skills/babysit-prs/reference/safety.md | 27 ++- .../skills/babysit-prs/reference/worktrees.md | 3 + 8 files changed, 160 insertions(+), 103 deletions(-) diff --git a/plugins/source-control/.claude-plugin/plugin.json b/plugins/source-control/.claude-plugin/plugin.json index 9b2b397c7d..bb3d15fb6a 100644 --- a/plugins/source-control/.claude-plugin/plugin.json +++ b/plugins/source-control/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "source-control", - "version": "0.15.9", + "version": "0.15.10", "description": "Git and GitHub delivery workflow: /commit (Conventional Commits + Co-Authored-By trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, CI-log fetch), /babysit-prs (self-pacing fleet loop โ€” safe by default; opt-in worker/autopilot tiers add gate-checked merge and thread resolution behind a deterministic Python engine), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (check the effective commit-subject / PR-title convention merged across its config layers and the babysit-prs config, or apply โ€” interview the repo and write the convention config to a chosen layer), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep โ€” never --abort). The commit-subject / PR-title convention is configurable via a source-control.md config written by a re-runnable setup skill, layered across a ~/.claude user-global file, the tracked team file, and a gitignored .claude/source-control.local.md personal overlay merged per key; Conventional Commits is the default when no convention is declared.", "author": { "name": "Melodic Software", diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index 0e07baae5d..d91d3a7928 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -3,6 +3,37 @@ All notable changes to the `source-control` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.15.10] + +### Fixed + +- **Babysit worker-worktree head-safety + merge-only freshness (`#548`).** A babysit worker can be + assigned a worktree in detached HEAD (its PR branch locked in a sibling/foreign worktree) or on a + stale local branch tip behind `origin`; the checkout/freshness mechanics then merged and pushed + from that tip, so a stale-tip integration could silently revert the newest branch commit โ€” a + near-miss where safety depended on the assigned `HEAD` happening to match, not a guard. + - `reference/safety.md` Checkout And Push Invariants now require asserting the assigned worktree's + `HEAD` equals the true PR head (`gh pr view --json headRefOid`) before any merge/edit/push (stop + on a stale/detached mismatch) and pushing via an explicit refspec to the branch's configured + upstream (`git push "$(git config --get branch..remote)" HEAD:` โ€” + `origin` for a same-repo head, the fork's remote for a write-allowed cross-repo head) โ€” + fast-forward by construction, never `--force` โ€” so a branch locked by a sibling worktree is not a + `git checkout` dead-end. + - The worker mechanics are reconciled to that contract: `reference/loop.md` ยง5.1.2 acquires the head + via `gh pr checkout` and asserts `HEAD == the live headRefOid` in every checkout path (already-at- + head, sibling-locked `--detach` reuse, and heal-via-checkout), degrading to read-only on mismatch; + `SKILL.md` Step 0.2 + cross-tier invariants and `reference/orchestration.md`'s conflict-worker + follow the same assertion + upstream refspec push. + - **Freshness is now merge-only.** The prior `loop.md` path rebased-and-`--force-with-lease`d + linear-history branches, which both violated the skill's own never-force-push invariant + (`safety.md` "Never Do Automatically", `orchestration.md`) and was the silent-revert vector. + Behind-default branches now always integrate via `git merge` + a fast-forward refspec push (the + final squash merge still flattens interim history). **Behavior change:** linear-history branches + now carry an interim merge commit during freshness instead of being rebased. + + Enforcement remains agent discipline; whether the head assertion belongs in a deterministic helper + is tracked in `#885`. + ## [0.15.9] ### Changed diff --git a/plugins/source-control/skills/babysit-prs/SKILL.md b/plugins/source-control/skills/babysit-prs/SKILL.md index 0324a8ccf0..40931bbbdd 100644 --- a/plugins/source-control/skills/babysit-prs/SKILL.md +++ b/plugins/source-control/skills/babysit-prs/SKILL.md @@ -114,9 +114,9 @@ the safe tier the table's `no โ€” report` still governs: report the blocker exac and do not spawn the conflict worker. Only `worker` and `autopilot` read that same string as license to act. -**Cross-tier invariants** โ€” hold in every tier including autopilot: never an unprotected -force-push (a rebase integration pushes only `--force-with-lease` per -[reference/loop.md](reference/loop.md)); never `--admin`; never delete a branch or worktree +**Cross-tier invariants** โ€” hold in every tier including autopilot: never a force-push (freshness is +merge-only, refspec-pushed fast-forward โ€” [reference/loop.md](reference/loop.md)); never `--admin`; +never delete a branch or worktree that is dirty or unmerged; never change GitHub settings, secrets, branch protection, or billing; never act on a repository outside the watched owners; never resolve a thread whose finding is not actually addressed. A merge always requires the deterministic gate โ€” autopilot @@ -337,8 +337,8 @@ Execute for EACH PR discovered, oldest first. Detailed mechanics: `${CLAUDE_PLUGIN_ROOT}/scripts/fetch-all-pr-comments.sh` (derives owner/repo from the current directory; from a cwd that is not a checkout of the target repo, export `FETCH_COMMENTS_OWNER`/`FETCH_COMMENTS_REPO` first โ€” also unblocks the readiness gate's exit 4), filter own prior replies, classify addressed/unaddressed from GitHub evidence (ยง5.0.3). GitHub is the source of truth, not model memory -- [ ] **Step 0.2 โ€” Branch checkout:** `gh pr checkout ` with worktree/dirty-tree pre-checks; - read-only mode when the branch is owned elsewhere (ยง5.1.2) +- [ ] **Step 0.2 โ€” Branch checkout:** put this worktree's HEAD at the true PR head (`gh pr view --json headRefOid`) โ€” `gh pr checkout `, or `--detach` when the branch is locked in a sibling worktree (never `git checkout` the locked branch); + assert HEAD == that head before any mutate, read-only on mismatch or dirty tree (ยง5.1.2) - [ ] **Step 0.3 โ€” Branch freshness:** fetch + `git merge-base --is-ancestor`; integrate (merge vs rebase per the branch's own history), graduated conflict handling (ยง5.1.2) - [ ] **Step 1 โ€” Event-delivery gate:** cloud poll / push channel / Monitor watch, re-armed diff --git a/plugins/source-control/skills/babysit-prs/evals/evals.json b/plugins/source-control/skills/babysit-prs/evals/evals.json index 11d8304a1e..baedba4d0f 100644 --- a/plugins/source-control/skills/babysit-prs/evals/evals.json +++ b/plugins/source-control/skills/babysit-prs/evals/evals.json @@ -66,14 +66,14 @@ }, { "id": 6, - "name": "babysit-preserves-merge-workflow", - "prompt": "/source-control:babysit-prs โ€” PR #482's checked-out branch is behind main and `git log --merges origin/main..HEAD` returns a merge commit.", - "expected_output": "The skill integrates `origin/main` with `git merge`, preserving the branch's merge workflow, and uses a plain `git push`. It does not rebase or force-push the merge-commit branch.", + "name": "babysit-freshness-is-merge-only", + "prompt": "/source-control:babysit-prs โ€” PR #482's checked-out branch is behind the default branch and needs freshening before CI.", + "expected_output": "The skill integrates the default branch with `git merge` (never a rebase) and pushes the merge commit by refspec to the branch's configured upstream (`git push \"$PUSH_REMOTE\" HEAD:`, where PUSH_REMOTE is `origin` for a same-repo head or the fork's remote for a write-allowed cross-repo head). It never rebases or force-pushes a PR branch as freshness maintenance.", "files": [], "expectations": [ - "It checks for merge commits before choosing an integration mode", - "It uses git merge origin/main for a merge-commit branch", - "It uses a plain push, not a force-with-lease push, after the merge" + "It integrates the default branch with git merge, never a rebase", + "It pushes by refspec to the branch's configured upstream, not a hardcoded origin and not a plain push that assumes a tracking branch", + "It never force-pushes or force-with-lease-pushes a PR branch as freshness maintenance" ] }, { diff --git a/plugins/source-control/skills/babysit-prs/reference/loop.md b/plugins/source-control/skills/babysit-prs/reference/loop.md index a50b3fc8fb..b7578b7558 100644 --- a/plugins/source-control/skills/babysit-prs/reference/loop.md +++ b/plugins/source-control/skills/babysit-prs/reference/loop.md @@ -134,123 +134,117 @@ A push channel arms for ONE PR at a time. Re-arm for each new PR in the loop. (`main` below โ€” substitute the repo's default branch.) ```bash -# Pre-check 0: already on the PR branch? This session owns it โ€” no checkout -# needed (the current worktree also shows up in `git worktree list`, so the -# other-worktree grep below would otherwise false-trip to read-only). -# Pre-check 1: is the branch checked out in ANOTHER worktree? -# Pre-check 2: does THIS worktree have uncommitted changes? They may be -# another session's WIP โ€” never reset/clean work this loop did not create. +# Decide checkout mode by asserting this worktree's HEAD against the TRUE PR head +# โ€” `gh pr view โ€ฆ headRefOid`, queried live, authoritative for same-repo and +# fork PRs alike (never a possibly-stale `origin/` remote-tracking ref, +# which is itself the silent-revert vector this guards; safety.md, Checkout And +# Push Invariants). Acquire the head with `gh pr checkout`, never a bare +# fetch/checkout by branch name. Mutate only when HEAD == the PR head; otherwise +# read-only. Only the fetch of the default branch (always on origin) is done up +# front, for the freshness merge below. BRANCH="" -CUR_BRANCH=$(git branch --show-current) +PR_HEAD=$(gh pr view "$PR_NUMBER" --json headRefOid -q .headRefOid) CUR_WT=$(git rev-parse --show-toplevel) DEFAULT_BRANCH=$(gh repo view --json defaultBranchRef -q .defaultBranchRef.name) -if [ "$CUR_BRANCH" = "$BRANCH" ]; then - # Already own the branch โ€” no checkout; freshness check below still runs. - # The dirty-tree guard still applies: uncommitted changes may be another - # session's WIP even on this branch โ€” full mode only on a clean tree. - git fetch origin "$DEFAULT_BRANCH" - if [ -n "$(git status --porcelain)" ]; then - echo "Working tree has uncommitted changes โ€” processing read-only" - CHECKOUT_MODE="read-only" - else - CHECKOUT_MODE="full" - fi -elif git worktree list | grep -vF "$CUR_WT " | grep -q "\[$BRANCH\]"; then - echo "Branch $BRANCH checked out in another worktree โ€” processing read-only" - CHECKOUT_MODE="read-only" -elif [ -n "$(git status --porcelain)" ]; then - echo "Working tree has uncommitted changes (possibly another session's WIP) โ€” no checkout, processing read-only" +git fetch origin "$DEFAULT_BRANCH" +if [ -n "$(git status --porcelain)" ]; then + # Uncommitted changes may be another session's WIP โ€” never reset/clean work + # this loop did not create. + echo "Working tree has uncommitted changes (possibly another session's WIP) โ€” processing read-only" CHECKOUT_MODE="read-only" +elif [ "$(git rev-parse HEAD)" = "$PR_HEAD" ]; then + # HEAD already IS the true PR head โ€” on the branch and current, or already + # detached at it. Safe to mutate. + CHECKOUT_MODE="full" +elif git worktree list | grep -vF "$CUR_WT " | grep -q "\[$BRANCH\]"; then + # Branch locked in a sibling worktree โ€” `git checkout $BRANCH` here dead-ends + # (`already used by worktree at ...`). `gh pr checkout --detach` resolves the + # head for same-repo AND fork PRs; operate in detached HEAD at it and push by + # refspec below. + gh pr checkout "$PR_NUMBER" --detach + [ "$(git rev-parse HEAD)" = "$PR_HEAD" ] && CHECKOUT_MODE="full" || CHECKOUT_MODE="read-only" else - git fetch origin "$DEFAULT_BRANCH" - # gh pr checkout handles fork-sourced PRs (head branch not fetchable from - # origin) and same-repo branches alike โ€” never bare fetch/checkout by name. + # gh pr checkout heals a behind-origin local branch up to the true head and + # handles fork-sourced PRs; a genuinely divergent local (unpushed commits) + # leaves HEAD != PR_HEAD and correctly falls to read-only. gh pr checkout "$PR_NUMBER" - CHECKOUT_MODE="full" + [ "$(git rev-parse HEAD)" = "$PR_HEAD" ] && CHECKOUT_MODE="full" || CHECKOUT_MODE="read-only" fi -# Branch freshness โ€” preserve the branch's integration workflow (full mode only) +# Branch freshness โ€” MERGE-ONLY (full mode only). Rebasing would rewrite history +# and require a force-push, which safety.md ("Never Do Automatically") and +# orchestration.md's never-force-push invariant forbid; the final squash merge +# flattens interim history, so a merge commit here costs nothing the skill cares +# about. if [ "$CHECKOUT_MODE" = "full" ]; then if ! git merge-base --is-ancestor "origin/$DEFAULT_BRANCH" HEAD; then - if git log --merges --format='%H' "origin/$DEFAULT_BRANCH..HEAD" | grep -q .; then - INTEGRATION_MODE="merge" - echo "Branch $BRANCH uses merge commits โ€” merging origin/$DEFAULT_BRANCH" - git merge --no-edit "origin/$DEFAULT_BRANCH" - INTEGRATION_EXIT=$? - else - INTEGRATION_MODE="rebase" - echo "Branch $BRANCH is behind origin/$DEFAULT_BRANCH โ€” rebasing" - git rebase "origin/$DEFAULT_BRANCH" - INTEGRATION_EXIT=$? - fi - - if [ "$INTEGRATION_EXIT" -eq 0 ]; then - REBASE_STATUS="integrated" - # A merge preserves existing commits and pushes normally. A rebase - # rewrites them and therefore needs a lease-protected force push. - if [ "$INTEGRATION_MODE" = "merge" ]; then - git push - else - git push --force-with-lease - fi + echo "Branch $BRANCH is behind origin/$DEFAULT_BRANCH โ€” merging" + git merge --no-edit "origin/$DEFAULT_BRANCH" + MERGE_EXIT=$? + if [ "$MERGE_EXIT" -eq 0 ]; then + INTEGRATION_STATUS="integrated" + # Push to the remote gh pr checkout configured for this branch โ€” origin for + # a same-repo head, the fork's remote for a write-allowed cross-repo (fork) + # head โ€” never hardcoded origin, which would write a same-named branch on + # the base repo instead of the fork head. Refspec form works from detached + # HEAD too; fast-forward given the head assertion, never force. A rejected + # non-fast-forward push means the head moved: re-fetch and stop. + PUSH_REMOTE=$(git config --get "branch.$BRANCH.remote" || echo origin) + git push "$PUSH_REMOTE" "HEAD:$BRANCH" else # Graduated conflict handling โ€” attempt simple, abort complex. - # conflict-attempting is a TRANSIENT state: resolve it (merge/rebase + # conflict-attempting is a TRANSIENT state: resolve it (git merge # --continue) or abort BEFORE any further processing โ€” never leave an # integration in progress (unmerged paths break later checkouts + parking). CONFLICT_COUNT=$(git diff --name-only --diff-filter=U | grep -c . || true) if [ "$CONFLICT_COUNT" -le 3 ]; then echo "Simple conflict ($CONFLICT_COUNT files) โ€” attempting resolution" - REBASE_STATUS="conflict-attempting" + INTEGRATION_STATUS="conflict-attempting" else - echo "Complex conflict ($CONFLICT_COUNT files) โ€” aborting $INTEGRATION_MODE" - if [ "$INTEGRATION_MODE" = "merge" ]; then - git merge --abort - else - git rebase --abort - fi - REBASE_STATUS="conflict-aborted" + echo "Complex conflict ($CONFLICT_COUNT files) โ€” aborting merge" + git merge --abort + INTEGRATION_STATUS="conflict-aborted" fi fi else - REBASE_STATUS="current" + INTEGRATION_STATUS="current" fi - # conflict-attempting: resolve NOW โ€” per file, take the mechanical - # resolution; if ANY file needs intent judgment, abort the active merge or - # rebase and set REBASE_STATUS="conflict-aborted". On success: `git add - # ` + the matching `git merge --continue` / `git rebase --continue`, - # then plain `git push` for a merge or `git push --force-with-lease` for a - # rebase. Set REBASE_STATUS="integrated". Only terminal states pass this point. + # conflict-attempting: resolve NOW โ€” per file, take the mechanical resolution; + # if ANY file needs intent judgment, `git merge --abort` and set + # INTEGRATION_STATUS="conflict-aborted". On success: `git add ` + + # `git merge --continue`, then `git push "$PUSH_REMOTE" HEAD:$BRANCH` (fast-forward, + # never force). Set INTEGRATION_STATUS="integrated". Only terminal states pass here. # Safe fallback: ONLY the terminal success states keep full mode. A # lingering conflict-attempting (resolution skipped) degrades to read-only - # rather than granting write access mid-rebase. - if [ "$REBASE_STATUS" != "integrated" ] && [ "$REBASE_STATUS" != "current" ]; then + # rather than granting write access mid-merge. + if [ "$INTEGRATION_STATUS" != "integrated" ] && [ "$INTEGRATION_STATUS" != "current" ]; then CHECKOUT_MODE="read-only" fi fi ``` -**Integration conflict handling (graduated).** Check for merge commits first -(`git log --merges origin/$DEFAULT_BRANCH..HEAD`) โ€” a branch that previously merged the default -branch integrates via `git merge origin/$DEFAULT_BRANCH` plus a plain push; other branches -rebase and force-push with lease. Then: - -- **Zero conflicts** (`REBASE_STATUS=integrated`) โ€” merge or rebase succeeded, push with the - mode-appropriate command, continue normally -- **Simple conflicts** (โ‰ค3 files, `REBASE_STATUS=conflict-attempting`) โ€” TRANSIENT: attempt - resolution immediately; on success continue the active merge/rebase and push with the - mode-appropriate command โ†’ `integrated`; if ANY file requires intent judgment, abort the - active integration โ†’ `conflict-aborted`. Never proceed to comment processing, parking, or the - next PR with an integration in progress. Resolve via `/source-control:resolve-conflicts` - discipline (understand both sides' intent; compose, don't side-pick) -- **Complex conflicts** (>3 files, `REBASE_STATUS=conflict-aborted`) โ€” abort the merge/rebase, +**Integration conflict handling (graduated).** Freshness is merge-only: integrate a behind-default +branch via `git merge origin/$DEFAULT_BRANCH` and push by refspec to the branch's configured upstream +(`git push "$PUSH_REMOTE" HEAD:$BRANCH` โ€” `origin` for a same-repo head, the fork's remote for a +write-allowed cross-repo head; fast-forward, never force โ€” rebasing or force-pushing a PR branch as +freshness maintenance is forbidden, safety.md and orchestration.md). Then: + +- **Zero conflicts** (`INTEGRATION_STATUS=integrated`) โ€” the merge succeeded; push + `git push "$PUSH_REMOTE" HEAD:$BRANCH` and continue normally +- **Simple conflicts** (โ‰ค3 files, `INTEGRATION_STATUS=conflict-attempting`) โ€” TRANSIENT: attempt + resolution immediately; on success continue the merge and push `git push "$PUSH_REMOTE" HEAD:$BRANCH` + โ†’ `integrated`; if ANY file requires intent judgment, abort the merge โ†’ `conflict-aborted`. + Never proceed to comment processing, parking, or the next PR with an integration in progress. + Resolve via `/source-control:resolve-conflicts` discipline (understand both sides' intent; + compose, don't side-pick) +- **Complex conflicts** (>3 files, `INTEGRATION_STATUS=conflict-aborted`) โ€” abort the merge, post a PR comment: `"โš ๏ธ Branch is behind $DEFAULT_BRANCH with integration conflicts ({N} files). Manual resolution is required before CI will trigger."`. If an interactive terminal, also surface to the user directly. Process comments read-only (classification + reply, no fixes โ€” the code may be stale) -- **Already current** (`REBASE_STATUS=current`) โ€” no action needed +- **Already current** (`INTEGRATION_STATUS=current`) โ€” no action needed **Why mandatory:** exploration and research read files from the working tree. Without checkout, findings are validated against the wrong code. Branch freshness prevents CI failures from stale @@ -268,7 +262,8 @@ after each wave of fixes. ### 5.1.3 Per-PR iteration checklist -Must be on the PR branch (ยง5.1.2) before starting. D steps run **per-finding** with +Must hold a full-mode checkout โ€” HEAD asserted equal to the true PR head (`gh pr view --json +headRefOid`), on the branch or in detached HEAD (ยง5.1.2) โ€” before starting. D steps run **per-finding** with verification gates per [review-discipline.md](../../../reference/review-discipline.md) ยง3. - [ ] **A** โ€” Terminal state check (`gh pr view --json state`) diff --git a/plugins/source-control/skills/babysit-prs/reference/orchestration.md b/plugins/source-control/skills/babysit-prs/reference/orchestration.md index e571c69d4a..819b77c488 100644 --- a/plugins/source-control/skills/babysit-prs/reference/orchestration.md +++ b/plugins/source-control/skills/babysit-prs/reference/orchestration.md @@ -447,12 +447,17 @@ The dedicated conflict-resolution worker's contract: Merging that stale local ref can find no conflict โ€” because the stale view predates the base update that actually caused it โ€” and push or report success without resolving anything. Fetch first, unconditionally, then merge. -- **Merge, never rebase.** Resolve with `git merge origin/` into the PR branch. This - is deliberate: a rebase rewrites the branch's commit history and would require a force-push to - update the remote PR branch, violating this skill's absolute never-force-push cross-tier - invariant. A merge commit needs only a normal `git push`, preserves both histories, and is fully - compatible with a repo that requires linear history on its default branch โ€” that requirement is - enforced by the final squash merge, not by the PR branch's own interim history. +- **Assert the head, merge, never rebase.** Before merging, assert the worktree's `HEAD` equals the + true PR head (`gh pr view --json headRefOid`) โ€” refuse to resolve onto a stale or detached tip + (`reference/safety.md`, Checkout And Push Invariants). Resolve with `git merge origin/` + into the PR branch. This is deliberate: a rebase rewrites the branch's commit history and would + require a force-push to update the remote PR branch, violating this skill's absolute + never-force-push cross-tier invariant. The merge commit is pushed by refspec to the branch's + configured upstream โ€” `git push "$(git config --get branch..remote)" HEAD:` + (`origin` for a same-repo head, the fork's remote for a write-allowed cross-repo head), a + fast-forward given the head assertion, never force โ€” preserving both histories and staying + compatible with a repo that requires linear history on its default branch, which the final squash + merge enforces, not the PR branch's own interim history. - **Understand both sides before touching markers.** Read and reconcile the actual semantic intent of the PR branch's own diff and of whatever changed on the base branch since divergence. Never resolve by blindly keeping "ours" or "theirs" without understanding what each side was trying to diff --git a/plugins/source-control/skills/babysit-prs/reference/safety.md b/plugins/source-control/skills/babysit-prs/reference/safety.md index bb3e6bf66c..d1526e521a 100644 --- a/plugins/source-control/skills/babysit-prs/reference/safety.md +++ b/plugins/source-control/skills/babysit-prs/reference/safety.md @@ -27,10 +27,33 @@ value and its unset fallback. ## Checkout And Push Invariants - Reuse an existing clean worktree for a PR rather than creating a second checkout โ€” reuse only - when it is already on the target PR branch and `git status --porcelain` is clean; otherwise - report it (`worktrees.md`). + when `git status --porcelain` is clean and its `HEAD` is the true PR head (the head assertion + below), whether it is checked out on the PR branch or in detached HEAD because the branch is + locked elsewhere; otherwise report it (`worktrees.md`). +- **Assigned-worktree head assertion.** Before any merge, edit, or push, resolve the assigned + worktree's `HEAD` to a commit and assert it equals the true PR head โ€” `gh pr view --json + headRefOid` (authoritative for same-repo and fork PRs; equal to a freshly re-fetched + `origin/` for a same-repo PR). This holds whether the worktree is on the PR branch, + in **detached HEAD** (the branch is checked out in a sibling worktree, or lives in a foreign dev + worktree outside ``), or on a **stale local branch tip** behind the PR head. If + `HEAD` differs from that head, **stop** โ€” never merge, edit, or push onto a stale tip: a naive + `git merge origin/` + push from a behind-head tip silently reverts the newest branch + commit(s). Safety comes from this assertion, not from the assigned `HEAD` happening to match. This + extends the head-SHA re-check below โ€” which covered only the head moving *mid-work* โ€” to the moment + the worktree is first assigned. - Re-check the PR head SHA immediately before editing and again immediately before pushing. Stop if it changed unexpectedly โ€” someone else moved the branch. +- **Refspec push to the branch's upstream, never branch checkout.** Do not depend on `git checkout + ` to reach the branch: when it is locked by a sibling worktree that command dead-ends + (`fatal: '' is already used by worktree at ...`). Once the head assertion holds, push the + integrated work with an explicit refspec to the remote `gh pr checkout` configured for the branch โ€” + `git push "$(git config --get branch..remote)" HEAD:` โ€” which resolves to + `origin` for a same-repo head and to the fork's remote for a write-allowed cross-repo (in-owner + fork) head. Never hardcode `origin`: for a fork head that silently writes a same-named branch on the + base repo instead of updating the fork head. Because `HEAD` equalled the PR head and you only added + commits on top, this push is a fast-forward; never `--force` or `--force-with-lease`. A rejected + non-fast-forward push means the assertion no longer holds โ€” re-fetch and stop, never force past it. + (An external-fork head outside `` remains the read-only stop-and-ask case below.) - Honor `mutation_policy.branch_write_allowed`: never push, and never create a write-capable worker or refresh a PR head, when it is false. - Head-ref uniqueness guard: two open PRs sharing one head repository/branch is a stop-and-ask โ€” diff --git a/plugins/source-control/skills/babysit-prs/reference/worktrees.md b/plugins/source-control/skills/babysit-prs/reference/worktrees.md index 27d036aadc..30c9144531 100644 --- a/plugins/source-control/skills/babysit-prs/reference/worktrees.md +++ b/plugins/source-control/skills/babysit-prs/reference/worktrees.md @@ -26,6 +26,9 @@ subdirectory. running in it โ€” lease-protected removal: hold that PR's worker lease for any per-PR removal. - Never use raw filesystem deletion for Git worktrees. Use `git worktree remove` through the cleanup helper. +- When a PR branch is already checked out in a sibling or foreign dev worktree, `git checkout` + dead-ends โ€” operate from the assigned worktree in detached HEAD under the head assertion and push + by refspec rather than sharing the foreign checkout (`safety.md`, Checkout And Push Invariants). ## Commands From 1d7a992dc6adafc4cb8a09074c7170c18e137dbd Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 21 Jul 2026 18:20:51 -0400 Subject: [PATCH 02/10] fix(source-control): fail-closed push-remote resolution for babysit push paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve $PUSH_REMOTE once at full-mode entry so the freshness, conflict-continue, and fix-cycle push paths share one destination, and make it fail closed: a fork head whose remote cannot be validated (e.g. a --detach checkout with no branch config) degrades to read-only instead of defaulting to origin โ€” origin is the base repo, so that fallback silently wrote a same-named branch on base rather than updating the fork head, the exact cross-repo regression this branch guards. Addresses three codex review findings on the loop.md push contract: - fail-open origin fallback for an unresolved fork remote; - PUSH_REMOTE left unset on the <=3-file conflict-continue push; - plain `git push` in the fix cycle, rejected from a detached HEAD (now refspec). safety.md's Checkout And Push Invariants aligned to the same fail-closed contract. Refs #548 ๐Ÿค– Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) --- .../skills/babysit-prs/reference/loop.md | 35 ++++++++++++++----- .../skills/babysit-prs/reference/safety.md | 10 +++--- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/plugins/source-control/skills/babysit-prs/reference/loop.md b/plugins/source-control/skills/babysit-prs/reference/loop.md index b7578b7558..37402d24f9 100644 --- a/plugins/source-control/skills/babysit-prs/reference/loop.md +++ b/plugins/source-control/skills/babysit-prs/reference/loop.md @@ -171,6 +171,27 @@ else [ "$(git rev-parse HEAD)" = "$PR_HEAD" ] && CHECKOUT_MODE="full" || CHECKOUT_MODE="read-only" fi +# Resolve the push destination ONCE, here, for EVERY full-mode push path โ€” the +# freshness push, the conflict-continue push, and the ยง5.1.4 fix-cycle push all +# read $PUSH_REMOTE. FAIL CLOSED: a same-repo head pushes to origin; a fork head +# pushes to the remote `gh pr checkout` configured for the branch. If a fork +# remote cannot be validated โ€” a fork head reached via `--detach` leaves no +# branch config โ€” degrade to read-only rather than defaulting to origin, which +# is the BASE repo and would write a same-named branch there instead of updating +# the fork head (the cross-repo regression this guards; safety.md). +if [ "$CHECKOUT_MODE" = "full" ]; then + if [ "$(gh pr view "$PR_NUMBER" --json isCrossRepository -q .isCrossRepository)" = "false" ]; then + PUSH_REMOTE=origin + else + PUSH_REMOTE=$(git config --get "branch.$BRANCH.remote" || true) + case "$PUSH_REMOTE" in + ""|.|origin) + echo "Fork head with no validated fork remote (detached or unmodifiable) โ€” read-only" + CHECKOUT_MODE="read-only" ;; + esac + fi +fi + # Branch freshness โ€” MERGE-ONLY (full mode only). Rebasing would rewrite history # and require a force-push, which safety.md ("Never Do Automatically") and # orchestration.md's never-force-push invariant forbid; the final squash merge @@ -183,13 +204,9 @@ if [ "$CHECKOUT_MODE" = "full" ]; then MERGE_EXIT=$? if [ "$MERGE_EXIT" -eq 0 ]; then INTEGRATION_STATUS="integrated" - # Push to the remote gh pr checkout configured for this branch โ€” origin for - # a same-repo head, the fork's remote for a write-allowed cross-repo (fork) - # head โ€” never hardcoded origin, which would write a same-named branch on - # the base repo instead of the fork head. Refspec form works from detached - # HEAD too; fast-forward given the head assertion, never force. A rejected - # non-fast-forward push means the head moved: re-fetch and stop. - PUSH_REMOTE=$(git config --get "branch.$BRANCH.remote" || echo origin) + # Push by refspec (works from a detached HEAD too) to the pre-resolved + # $PUSH_REMOTE; fast-forward given the head assertion, never force. A + # rejected non-fast-forward push means the head moved: re-fetch and stop. git push "$PUSH_REMOTE" "HEAD:$BRANCH" else # Graduated conflict handling โ€” attempt simple, abort complex. @@ -302,7 +319,9 @@ When on the PR branch AND a comment is classified VALID after D3 validation: - [ ] Edit code to fix the issue - [ ] `git add ` (never `-A` or `.`) - [ ] `git commit -m ": "` -- [ ] `git push` +- [ ] `git push "$PUSH_REMOTE" HEAD:$BRANCH` โ€” refspec form against the same + pre-resolved `$PUSH_REMOTE` the freshness push used; a plain `git push` + is rejected from the `--detach` checkout a sibling-locked branch uses - [ ] Post a follow-up reply citing the commit SHA (D7) **One wave at a time:** address all current comments on this PR โ†’ commit + push โ†’ then diff --git a/plugins/source-control/skills/babysit-prs/reference/safety.md b/plugins/source-control/skills/babysit-prs/reference/safety.md index d1526e521a..0025821042 100644 --- a/plugins/source-control/skills/babysit-prs/reference/safety.md +++ b/plugins/source-control/skills/babysit-prs/reference/safety.md @@ -47,10 +47,12 @@ value and its unset fallback. ` to reach the branch: when it is locked by a sibling worktree that command dead-ends (`fatal: '' is already used by worktree at ...`). Once the head assertion holds, push the integrated work with an explicit refspec to the remote `gh pr checkout` configured for the branch โ€” - `git push "$(git config --get branch..remote)" HEAD:` โ€” which resolves to - `origin` for a same-repo head and to the fork's remote for a write-allowed cross-repo (in-owner - fork) head. Never hardcode `origin`: for a fork head that silently writes a same-named branch on the - base repo instead of updating the fork head. Because `HEAD` equalled the PR head and you only added + `git push "$PUSH_REMOTE" HEAD:`, where `PUSH_REMOTE` resolves **fail-closed**: `origin` + for a same-repo head; for a write-allowed cross-repo (in-owner fork) head, the fork's remote from + `git config --get branch..remote`. Never hardcode `origin`, and never fall back to it + when that remote is unresolved โ€” a fork head reached via `--detach` leaves no branch config, and + `origin` is then the base repo, so pushing there silently writes a same-named branch on base instead + of updating the fork head; **stop (read-only) instead**. Because `HEAD` equalled the PR head and you only added commits on top, this push is a fast-forward; never `--force` or `--force-with-lease`. A rejected non-fast-forward push means the assertion no longer holds โ€” re-fetch and stop, never force past it. (An external-fork head outside `` remains the read-only stop-and-ask case below.) From 180793cf762f15c35958701e57a769ea2778d357 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 21 Jul 2026 18:32:14 -0400 Subject: [PATCH 03/10] fix(source-control): reconcile conflict-worker + safety contract to fail-closed push remote MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend the fail-closed push-remote resolution to the sibling contracts that document the same invariant, so no tier is left with the fail-open origin fallback: - orchestration.md conflict-worker: push via the fail-closed $PUSH_REMOTE (points at safety.md as the canonical contract) instead of the bare `git config --get branch..remote` that defaulted to origin. - safety.md: name the decision mechanism โ€” same-repo vs fork is decided from `gh pr view --json isCrossRepository`, never inferred from whether git config resolves โ€” closing the re-implementation gap that could reintroduce the root-cause pattern. Refs #548 ๐Ÿค– Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) --- .../skills/babysit-prs/reference/orchestration.md | 9 ++++++--- .../skills/babysit-prs/reference/safety.md | 8 +++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/plugins/source-control/skills/babysit-prs/reference/orchestration.md b/plugins/source-control/skills/babysit-prs/reference/orchestration.md index 819b77c488..b96921a743 100644 --- a/plugins/source-control/skills/babysit-prs/reference/orchestration.md +++ b/plugins/source-control/skills/babysit-prs/reference/orchestration.md @@ -453,9 +453,12 @@ The dedicated conflict-resolution worker's contract: into the PR branch. This is deliberate: a rebase rewrites the branch's commit history and would require a force-push to update the remote PR branch, violating this skill's absolute never-force-push cross-tier invariant. The merge commit is pushed by refspec to the branch's - configured upstream โ€” `git push "$(git config --get branch..remote)" HEAD:` - (`origin` for a same-repo head, the fork's remote for a write-allowed cross-repo head), a - fast-forward given the head assertion, never force โ€” preserving both histories and staying + configured upstream โ€” `git push "$PUSH_REMOTE" HEAD:`, where `PUSH_REMOTE` resolves + **fail-closed** per `reference/safety.md` (Checkout And Push Invariants): `origin` for a same-repo + head, the fork's remote for a write-allowed cross-repo head, and **stop (read-only)** rather than + defaulting to `origin` when a fork remote is unresolved (an `origin` fallback writes a same-named + branch on the base repo, not the fork head). A fast-forward given the head assertion, never force โ€” + preserving both histories and staying compatible with a repo that requires linear history on its default branch, which the final squash merge enforces, not the PR branch's own interim history. - **Understand both sides before touching markers.** Read and reconcile the actual semantic intent diff --git a/plugins/source-control/skills/babysit-prs/reference/safety.md b/plugins/source-control/skills/babysit-prs/reference/safety.md index 0025821042..0bfcb20c8e 100644 --- a/plugins/source-control/skills/babysit-prs/reference/safety.md +++ b/plugins/source-control/skills/babysit-prs/reference/safety.md @@ -47,9 +47,11 @@ value and its unset fallback. ` to reach the branch: when it is locked by a sibling worktree that command dead-ends (`fatal: '' is already used by worktree at ...`). Once the head assertion holds, push the integrated work with an explicit refspec to the remote `gh pr checkout` configured for the branch โ€” - `git push "$PUSH_REMOTE" HEAD:`, where `PUSH_REMOTE` resolves **fail-closed**: `origin` - for a same-repo head; for a write-allowed cross-repo (in-owner fork) head, the fork's remote from - `git config --get branch..remote`. Never hardcode `origin`, and never fall back to it + `git push "$PUSH_REMOTE" HEAD:`, where `PUSH_REMOTE` resolves **fail-closed**. Decide + same-repo vs fork from `gh pr view --json isCrossRepository`, never by whether `git config` happens + to resolve: `origin` for a same-repo head; for a write-allowed cross-repo (in-owner fork) head, the + fork's remote from `git config --get branch..remote` (guarding a value that is empty, + `.`, or literally `origin`). Never hardcode `origin`, and never fall back to it when that remote is unresolved โ€” a fork head reached via `--detach` leaves no branch config, and `origin` is then the base repo, so pushing there silently writes a same-named branch on base instead of updating the fork head; **stop (read-only) instead**. Because `HEAD` equalled the PR head and you only added From d2b62d6b05d0cb8c332c6101e708f1728f41c994 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 21 Jul 2026 18:36:24 -0400 Subject: [PATCH 04/10] fix(source-control): reconcile babysit checklists to merge-only + read-only D1-D5 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two top-level checklist lines drifted from the contract this branch establishes: - SKILL.md Step 0.3 still said "merge vs rebase per the branch's own history", contradicting the merge-only freshness (ยง5.1.2) and re-admitting the rebase + force-push path this branch forbids; now states merge-only, never rebase. - loop.md ยง5.1.3 gated the whole per-finding D-cycle behind a full-mode checkout, which would skip D1-D5 (investigate/classify/reply) in the read-only safety-fallback cases where the contract still runs them; now requires only a HEAD-asserted checkout and scopes full mode to the D6-D7 fix cycle. Refs #548 ๐Ÿค– Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) --- plugins/source-control/skills/babysit-prs/SKILL.md | 2 +- .../source-control/skills/babysit-prs/reference/loop.md | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/plugins/source-control/skills/babysit-prs/SKILL.md b/plugins/source-control/skills/babysit-prs/SKILL.md index 2348725507..e71bfa8107 100644 --- a/plugins/source-control/skills/babysit-prs/SKILL.md +++ b/plugins/source-control/skills/babysit-prs/SKILL.md @@ -340,7 +340,7 @@ Execute for EACH PR discovered, oldest first. Detailed mechanics: - [ ] **Step 0.2 โ€” Branch checkout:** put this worktree's HEAD at the true PR head (`gh pr view --json headRefOid`) โ€” `gh pr checkout `, or `--detach` when the branch is locked in a sibling worktree (never `git checkout` the locked branch); assert HEAD == that head before any mutate, read-only on mismatch or dirty tree (ยง5.1.2) - [ ] **Step 0.3 โ€” Branch freshness:** fetch + `git merge-base --is-ancestor`; integrate - (merge vs rebase per the branch's own history), graduated conflict handling (ยง5.1.2) + merge-only (never rebase โ€” rebasing a PR branch needs a forbidden force-push), graduated conflict handling (ยง5.1.2) - [ ] **Step 1 โ€” Event-delivery gate:** cloud poll / push channel / Monitor watch, re-armed per PR (ยง5.1.1) - [ ] **Steps Aโ€“F โ€” Per-PR iteration checklist** (ยง5.1.3): terminal check, CI classification, diff --git a/plugins/source-control/skills/babysit-prs/reference/loop.md b/plugins/source-control/skills/babysit-prs/reference/loop.md index 37402d24f9..20554f1ff7 100644 --- a/plugins/source-control/skills/babysit-prs/reference/loop.md +++ b/plugins/source-control/skills/babysit-prs/reference/loop.md @@ -279,9 +279,10 @@ after each wave of fixes. ### 5.1.3 Per-PR iteration checklist -Must hold a full-mode checkout โ€” HEAD asserted equal to the true PR head (`gh pr view --json -headRefOid`), on the branch or in detached HEAD (ยง5.1.2) โ€” before starting. D steps run **per-finding** with -verification gates per [review-discipline.md](../../../reference/review-discipline.md) ยง3. +Must hold a HEAD-asserted checkout โ€” HEAD equal to the true PR head (`gh pr view --json +headRefOid`), on the branch or in detached HEAD (ยง5.1.2) โ€” before starting. Read-only mode still runs +D1-D5 (investigate/classify/reply); only the D6-D7 fix cycle requires full mode. D steps run +**per-finding** with verification gates per [review-discipline.md](../../../reference/review-discipline.md) ยง3. - [ ] **A** โ€” Terminal state check (`gh pr view --json state`) - [ ] **B** โ€” CI checks โ€” classify every non-pending check (pass/fail/skipped) From 33f1fbea8ed5fd2ef49203df4ddde8b4e39e2eaf Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 21 Jul 2026 18:44:19 -0400 Subject: [PATCH 05/10] fix(source-control): reconcile branch-attached phrasing with detached-HEAD support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This branch adds sibling-locked `gh pr checkout --detach` full-mode support, but several contracts still assumed an attached checkout, so a worker on a valid detached HEAD (== the true PR head) would wrongly stop or skip the fix cycle in exactly the sibling-locked case this branch enables: - orchestration.md conflict worker: "refuse a stale or detached tip" now reads "stale or head-mismatched tip" (a detached HEAD equal to the head is fine). - loop.md ยง5.1.4 gate and the full-mode / wrong-branch guards now key on the HEAD-at-true-head assertion (attached or detached) instead of branch attachment. - SKILL.md Step 5 push clarified as a refspec push that works from detached HEAD. Refs #548 ๐Ÿค– Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) --- plugins/source-control/skills/babysit-prs/SKILL.md | 2 +- .../skills/babysit-prs/reference/loop.md | 10 ++++++---- .../skills/babysit-prs/reference/orchestration.md | 5 +++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/plugins/source-control/skills/babysit-prs/SKILL.md b/plugins/source-control/skills/babysit-prs/SKILL.md index e71bfa8107..1bdedf48f5 100644 --- a/plugins/source-control/skills/babysit-prs/SKILL.md +++ b/plugins/source-control/skills/babysit-prs/SKILL.md @@ -350,7 +350,7 @@ Execute for EACH PR discovered, oldest first. Detailed mechanics: the configured extra self identities are `${user_config.babysit_self_logins}` โ€” when that value is non-empty and not a literal unexpanded token, append `--extra-self ""`), report -- [ ] **Step 5 โ€” Commit + push** fixes on the PR branch; clean working tree; follow-up replies +- [ ] **Step 5 โ€” Commit + push** fixes to the PR branch (refspec; works from a detached HEAD); clean working tree; follow-up replies cite commit SHAs - [ ] **Step 6 โ€” PR transition:** next-oldest PR needing attention (ยง5.1.6) - [ ] **Step 7 โ€” Self-pace:** schedule the next wake per the cadence contract (ยง5.3) diff --git a/plugins/source-control/skills/babysit-prs/reference/loop.md b/plugins/source-control/skills/babysit-prs/reference/loop.md index 20554f1ff7..592ca9dd53 100644 --- a/plugins/source-control/skills/babysit-prs/reference/loop.md +++ b/plugins/source-control/skills/babysit-prs/reference/loop.md @@ -274,8 +274,8 @@ NOT passive โ€” every comment still gets investigated and replied to. Fixes that are described in the reply with exact code changes so the user or the PR's own worktree session can apply them. -**Full mode:** full flow including the fix cycle (D1-D7). Commit and push on the PR branch -after each wave of fixes. +**Full mode:** full flow including the fix cycle (D1-D7). Commit and push to the PR branch (by +refspec โ€” works from a detached HEAD too) after each wave of fixes. ### 5.1.3 Per-PR iteration checklist @@ -315,7 +315,8 @@ is not evidence โ€” re-query the API. ### 5.1.4 Fix cycle (full mode only) -When on the PR branch AND a comment is classified VALID after D3 validation: +When in full mode (HEAD asserted at the true PR head โ€” attached or detached per ยง5.1.2) AND a +comment is classified VALID after D3 validation: - [ ] Edit code to fix the issue - [ ] `git add ` (never `-A` or `.`) @@ -438,7 +439,8 @@ These constraints override any other instruction within the babysit loop: - **Never trust a finding without validating** โ€” bot/AI assertions have demonstrated error rates. Always verify against actual code (D3) before implementing. Explore the referenced code; research non-trivial claims -- **Never process comments from the wrong branch** โ€” must be on the PR branch before D2-D3. +- **Never process comments from the wrong branch** โ€” HEAD must be asserted at the true PR head + (attached or detached, ยง5.1.2) before D2-D3. Exploring code on the default branch or another branch produces wrong classifications - **Never advance to the next PR with unaddressed comments on the current PR** โ€” focus-first rule (ยง5.0). Complete the current wave before moving on diff --git a/plugins/source-control/skills/babysit-prs/reference/orchestration.md b/plugins/source-control/skills/babysit-prs/reference/orchestration.md index b96921a743..7cfb3393c6 100644 --- a/plugins/source-control/skills/babysit-prs/reference/orchestration.md +++ b/plugins/source-control/skills/babysit-prs/reference/orchestration.md @@ -448,8 +448,9 @@ The dedicated conflict-resolution worker's contract: update that actually caused it โ€” and push or report success without resolving anything. Fetch first, unconditionally, then merge. - **Assert the head, merge, never rebase.** Before merging, assert the worktree's `HEAD` equals the - true PR head (`gh pr view --json headRefOid`) โ€” refuse to resolve onto a stale or detached tip - (`reference/safety.md`, Checkout And Push Invariants). Resolve with `git merge origin/` + true PR head (`gh pr view --json headRefOid`) โ€” refuse to resolve onto a stale or head-mismatched + tip (a detached HEAD that equals the head is fine โ€” the sibling-locked case; `reference/safety.md`, + Checkout And Push Invariants). Resolve with `git merge origin/` into the PR branch. This is deliberate: a rebase rewrites the branch's commit history and would require a force-push to update the remote PR branch, violating this skill's absolute never-force-push cross-tier invariant. The merge commit is pushed by refspec to the branch's From eaaa57362dbcf3d1c38888a16e12b5d34ca0889a Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 21 Jul 2026 19:46:55 -0400 Subject: [PATCH 06/10] fix(source-control): validate babysit fork push remote by URL, not by name The full-mode PUSH_REMOTE resolution rejected only the literal remote names "", ".", and "origin" before treating a cross-repo branch..remote as a validated fork destination. Remote names are arbitrary: gh pr checkout can wire a fork branch to the base checkout remote (commonly "upstream", but any name) when it cannot create a writable fork remote, so a non-origin name could push to the BASE repo and write a same-named branch there instead of the fork head. Validate the destination by URL identity instead: resolve pushRemote or remote (each may hold a remote name OR a bare URL) to owner/repo and require it to equal the PR head repository, else degrade to read-only. Update safety.md's canonical Checkout And Push Invariants prose to match. --- .../skills/babysit-prs/reference/loop.md | 30 ++++++++++++------- .../skills/babysit-prs/reference/safety.md | 12 ++++---- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/plugins/source-control/skills/babysit-prs/reference/loop.md b/plugins/source-control/skills/babysit-prs/reference/loop.md index 592ca9dd53..e167ec43d8 100644 --- a/plugins/source-control/skills/babysit-prs/reference/loop.md +++ b/plugins/source-control/skills/babysit-prs/reference/loop.md @@ -174,21 +174,29 @@ fi # Resolve the push destination ONCE, here, for EVERY full-mode push path โ€” the # freshness push, the conflict-continue push, and the ยง5.1.4 fix-cycle push all # read $PUSH_REMOTE. FAIL CLOSED: a same-repo head pushes to origin; a fork head -# pushes to the remote `gh pr checkout` configured for the branch. If a fork -# remote cannot be validated โ€” a fork head reached via `--detach` leaves no -# branch config โ€” degrade to read-only rather than defaulting to origin, which -# is the BASE repo and would write a same-named branch there instead of updating -# the fork head (the cross-repo regression this guards; safety.md). +# pushes to the destination whose URL resolves to the PR HEAD repository. +# Validate the fork destination by URL, NOT by name: `gh pr checkout` wires the +# branch to whatever remote (or bare URL) reaches the head, and when it cannot +# create a writable fork remote it may point the branch at the base checkout +# remote โ€” commonly named `upstream`, but any name โ€” so accepting any non-origin +# name would push to the BASE repo, writing a same-named branch there instead of +# updating the fork head (the cross-repo regression this guards; safety.md). +# `branch..pushRemote`/`remote` may hold a remote name OR a URL; resolve +# either to owner/repo and require it to equal the PR head repo, else read-only. if [ "$CHECKOUT_MODE" = "full" ]; then if [ "$(gh pr view "$PR_NUMBER" --json isCrossRepository -q .isCrossRepository)" = "false" ]; then PUSH_REMOTE=origin else - PUSH_REMOTE=$(git config --get "branch.$BRANCH.remote" || true) - case "$PUSH_REMOTE" in - ""|.|origin) - echo "Fork head with no validated fork remote (detached or unmodifiable) โ€” read-only" - CHECKOUT_MODE="read-only" ;; - esac + HEAD_REPO=$(gh pr view "$PR_NUMBER" --json headRepository -q .headRepository.nameWithOwner) + # git push consults pushRemote before remote; either may be a remote NAME or a URL. + PUSH_REMOTE=$(git config --get "branch.$BRANCH.pushRemote" \ + || git config --get "branch.$BRANCH.remote" || true) + REMOTE_URL=$(git remote get-url "$PUSH_REMOTE" 2>/dev/null || printf '%s' "$PUSH_REMOTE") + REMOTE_SLUG=$(printf '%s\n' "$REMOTE_URL" | sed -E 's#^[^/]+://[^/]+/##; s#^[^@]+@[^:]+:##; s#\.git$##') + if [ -z "$PUSH_REMOTE" ] || [ -z "$HEAD_REPO" ] || [ "$REMOTE_SLUG" != "$HEAD_REPO" ]; then + echo "Fork push remote does not resolve to the PR head repo ($HEAD_REPO) โ€” read-only" + CHECKOUT_MODE="read-only" + fi fi fi diff --git a/plugins/source-control/skills/babysit-prs/reference/safety.md b/plugins/source-control/skills/babysit-prs/reference/safety.md index 0bfcb20c8e..eb131f97b4 100644 --- a/plugins/source-control/skills/babysit-prs/reference/safety.md +++ b/plugins/source-control/skills/babysit-prs/reference/safety.md @@ -50,11 +50,13 @@ value and its unset fallback. `git push "$PUSH_REMOTE" HEAD:`, where `PUSH_REMOTE` resolves **fail-closed**. Decide same-repo vs fork from `gh pr view --json isCrossRepository`, never by whether `git config` happens to resolve: `origin` for a same-repo head; for a write-allowed cross-repo (in-owner fork) head, the - fork's remote from `git config --get branch..remote` (guarding a value that is empty, - `.`, or literally `origin`). Never hardcode `origin`, and never fall back to it - when that remote is unresolved โ€” a fork head reached via `--detach` leaves no branch config, and - `origin` is then the base repo, so pushing there silently writes a same-named branch on base instead - of updating the fork head; **stop (read-only) instead**. Because `HEAD` equalled the PR head and you only added + fork destination from `branch..pushRemote` or `branch..remote`, validated + by URL โ€” resolve that value (a remote name or a bare URL) to owner/repo and require it to equal the + PR head repository (`gh pr view --json headRepository`), not merely reject the literal `origin` name. + Never hardcode `origin`, and never fall back to it when the destination cannot be validated โ€” a fork + head reached via `--detach` leaves no branch config, and a remote named `upstream` (or any name) can + point at the base repo, so pushing there silently writes a same-named branch on base instead of + updating the fork head; **stop (read-only) instead**. Because `HEAD` equalled the PR head and you only added commits on top, this push is a fast-forward; never `--force` or `--force-with-lease`. A rejected non-fast-forward push means the assertion no longer holds โ€” re-fetch and stop, never force past it. (An external-fork head outside `` remains the read-only stop-and-ask case below.) From 635d5d4784350ba7b991a410ada7aac4cba00329 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 21 Jul 2026 20:19:08 -0400 Subject: [PATCH 07/10] fix(source-control): extend babysit failed-fix cleanup to detached-HEAD checkouts The fix cycle runs in full mode from either an attached PR branch or a detached HEAD (sibling-locked branch reused via `gh pr checkout --detach`), but the transition cleanup and the never-leave-dirty invariant were still gated on being "on a PR branch." A detached worker that left uncommitted edits from a failed fix would skip the reset/clean and carry a dirty worktree into parking or the next PR. Both now cover any full-mode PR checkout, attached or detached. --- .../source-control/skills/babysit-prs/reference/loop.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/plugins/source-control/skills/babysit-prs/reference/loop.md b/plugins/source-control/skills/babysit-prs/reference/loop.md index e167ec43d8..f8fd5d17d6 100644 --- a/plugins/source-control/skills/babysit-prs/reference/loop.md +++ b/plugins/source-control/skills/babysit-prs/reference/loop.md @@ -367,8 +367,9 @@ pauses for approval in interactive sessions. Report to the user in the babysit i When the readiness gate passes OR all actionable items are handled for this PR: -1. If on a PR branch with uncommitted changes from a failed fix: `git reset --hard HEAD` then - `git clean -fd` (unstage + revert tracked + remove untracked) +1. If a full-mode PR checkout (attached or detached per ยง5.1.2) has uncommitted changes from a + failed fix: `git reset --hard HEAD` then `git clean -fd` (unstage + revert tracked + remove + untracked) 2. Report PR status (ready / blockers remaining / items deferred to human) 3. Move to the next PR in the discovery list @@ -460,7 +461,8 @@ These constraints override any other instruction within the babysit loop: - **Never auto-fix human reviewer comments** โ€” classify + reply + report to the user - **Never skip the event-delivery gate** โ€” run ยง5.1.1 for every PR - **Never exceed 3 CI fix iterations** per PR per babysit pass -- **Never leave uncommitted changes** on a PR branch when transitioning to the next PR +- **Never leave uncommitted changes** on a full-mode PR checkout (attached or detached) when + transitioning to the next PR - **Never skip emoji reactions** โ€” every classified finding gets a reaction on its parent comment (+1 VALID, -1 INCORRECT, eyes UNCERTAIN). Reactions are the fastest audit signal for reviewers scanning a PR From 1acce0c1d4918b510b6b0429de02fb6c59db0006 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 21 Jul 2026 20:24:55 -0400 Subject: [PATCH 08/10] fix(source-control): validate babysit fork push by host, and by push URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The round-1 URL-identity gate closed the name-based fail-open but left two holes in the full-mode PUSH_REMOTE validation: - Host was stripped: reducing both sides to owner/repo ignored the host, so a remote on a different host with the same path (e.g. git@evil.example.com:owner/claude-code-plugins) matched the head repo and passed. Canonicalize to host + owner/repo via a shared repo_id() and compare the head repo's authoritative html_url (gh api repos/) โ€” since headRepository exposes no URL โ€” so both host and path must match. - Fetch URL was validated, push URL was used: git remote get-url returns the fetch URL, but git push honors remote..pushurl. A remote whose fetch URL is a real fork but whose pushurl targets the base repo passed, then pushed to base. Validate git remote get-url --push, the address git push actually writes. safety.md's Checkout And Push Invariants prose updated in lockstep. --- .../skills/babysit-prs/reference/loop.md | 27 ++++++++++++++----- .../skills/babysit-prs/reference/safety.md | 14 ++++++---- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/plugins/source-control/skills/babysit-prs/reference/loop.md b/plugins/source-control/skills/babysit-prs/reference/loop.md index f8fd5d17d6..96d8d93792 100644 --- a/plugins/source-control/skills/babysit-prs/reference/loop.md +++ b/plugins/source-control/skills/babysit-prs/reference/loop.md @@ -181,20 +181,33 @@ fi # remote โ€” commonly named `upstream`, but any name โ€” so accepting any non-origin # name would push to the BASE repo, writing a same-named branch there instead of # updating the fork head (the cross-repo regression this guards; safety.md). -# `branch..pushRemote`/`remote` may hold a remote name OR a URL; resolve -# either to owner/repo and require it to equal the PR head repo, else read-only. +# `branch..pushRemote`/`remote` may hold a remote name OR a URL, and a named +# remote can carry a separate `pushurl` that `git push` honors โ€” so resolve the +# actual PUSH url (`git remote get-url --push`) and canonicalize it to +# host + owner/repo, then require BOTH to equal the head repo's own canonical URL +# (`gh api repos/ --jq .html_url`), else read-only. This rejects a +# same-path remote on a DIFFERENT host and a fork fetch url masking a base pushurl. if [ "$CHECKOUT_MODE" = "full" ]; then if [ "$(gh pr view "$PR_NUMBER" --json isCrossRepository -q .isCrossRepository)" = "false" ]; then PUSH_REMOTE=origin else + # Canonicalize any git URL โ€” scheme://[user@]host[:port]/owner/repo, + # user@host:owner/repo, or a bare host/owner/repo โ€” to host/owner/repo so the + # comparison includes the HOST, not just the path: a same-path remote on a + # different host (git@evil.example.com:owner/repo) must NOT satisfy it. + repo_id() { printf '%s\n' "$1" | sed -E 's#\.git$##; s#/+$##; s#^[a-zA-Z][a-zA-Z0-9+.-]*://##; s#^[^/@]*@##; s#:[0-9]+/#/#; s#:#/#'; } HEAD_REPO=$(gh pr view "$PR_NUMBER" --json headRepository -q .headRepository.nameWithOwner) - # git push consults pushRemote before remote; either may be a remote NAME or a URL. + # headRepository carries no URL; resolve the head repo's canonical host+path + # from its authoritative html_url (a fork head is always on the base instance). + HEAD_ID=$(repo_id "$(gh api "repos/$HEAD_REPO" --jq .html_url 2>/dev/null)") + # git push consults pushRemote before remote; either may be a remote NAME or a + # URL. Resolve the PUSH url (--push honors remote..pushurl, which can + # differ from the fetch url) โ€” the address git push will actually write to. PUSH_REMOTE=$(git config --get "branch.$BRANCH.pushRemote" \ || git config --get "branch.$BRANCH.remote" || true) - REMOTE_URL=$(git remote get-url "$PUSH_REMOTE" 2>/dev/null || printf '%s' "$PUSH_REMOTE") - REMOTE_SLUG=$(printf '%s\n' "$REMOTE_URL" | sed -E 's#^[^/]+://[^/]+/##; s#^[^@]+@[^:]+:##; s#\.git$##') - if [ -z "$PUSH_REMOTE" ] || [ -z "$HEAD_REPO" ] || [ "$REMOTE_SLUG" != "$HEAD_REPO" ]; then - echo "Fork push remote does not resolve to the PR head repo ($HEAD_REPO) โ€” read-only" + REMOTE_URL=$(git remote get-url --push "$PUSH_REMOTE" 2>/dev/null || printf '%s' "$PUSH_REMOTE") + if [ -z "$PUSH_REMOTE" ] || [ -z "$HEAD_ID" ] || [ "$(repo_id "$REMOTE_URL")" != "$HEAD_ID" ]; then + echo "Fork push remote does not resolve to the PR head repo ($HEAD_ID) โ€” read-only" CHECKOUT_MODE="read-only" fi fi diff --git a/plugins/source-control/skills/babysit-prs/reference/safety.md b/plugins/source-control/skills/babysit-prs/reference/safety.md index 1e3a86e48d..f33abb241b 100644 --- a/plugins/source-control/skills/babysit-prs/reference/safety.md +++ b/plugins/source-control/skills/babysit-prs/reference/safety.md @@ -51,11 +51,15 @@ value and its unset fallback. same-repo vs fork from `gh pr view --json isCrossRepository`, never by whether `git config` happens to resolve: `origin` for a same-repo head; for a write-allowed cross-repo (in-owner fork) head, the fork destination from `branch..pushRemote` or `branch..remote`, validated - by URL โ€” resolve that value (a remote name or a bare URL) to owner/repo and require it to equal the - PR head repository (`gh pr view --json headRepository`), not merely reject the literal `origin` name. - Never hardcode `origin`, and never fall back to it when the destination cannot be validated โ€” a fork - head reached via `--detach` leaves no branch config, and a remote named `upstream` (or any name) can - point at the base repo, so pushing there silently writes a same-named branch on base instead of + by URL โ€” a named remote can carry a separate `pushurl` that `git push` honors, so resolve the actual + push URL (`git remote get-url --push`) and canonicalize it (a remote name, a bare URL, or that + `pushurl`) to **host + owner/repo**, then require both to equal the head repo's own canonical URL + (`gh api repos/ --jq .html_url`; `gh pr view --json headRepository` exposes no URL), + not merely reject the literal `origin` name or match `owner/repo` on any host. Never hardcode + `origin`, and never fall back to it when the destination cannot be validated โ€” a fork head reached via + `--detach` leaves no branch config, and a remote named `upstream` (or any name), a same-`owner/repo` + path on a different host, or a fork fetch URL masking a base-repo `pushurl`, can point at the base + repo, so pushing there silently writes a same-named branch on base instead of updating the fork head; **stop (read-only) instead**. Because `HEAD` equalled the PR head and you only added commits on top, this push is a fast-forward; never `--force` or `--force-with-lease`. A rejected non-fast-forward push means the assertion no longer holds โ€” re-fetch and stop, never force past it. From 74a419818f170ba138f9641ec2a82580dc031232 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 21 Jul 2026 20:35:05 -0400 Subject: [PATCH 09/10] docs(source-control): align 0.16.2 changelog with host+push-URL fork validation The [0.16.2] safety.md push bullet still described name-based push resolution (git config branch..remote, "the fork's remote for a cross-repo head"), which predates the URL-identity gate. Update it to what now ships: a fail-closed destination validated by host + owner/repo identity against the head repo's canonical URL, using the actual push URL (git remote get-url --push). --- plugins/source-control/CHANGELOG.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index 2b3d15390e..893c10552f 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -14,11 +14,14 @@ All notable changes to the `source-control` plugin are documented here. Format f near-miss where safety depended on the assigned `HEAD` happening to match, not a guard. - `reference/safety.md` Checkout And Push Invariants now require asserting the assigned worktree's `HEAD` equals the true PR head (`gh pr view --json headRefOid`) before any merge/edit/push (stop - on a stale/detached mismatch) and pushing via an explicit refspec to the branch's configured - upstream (`git push "$(git config --get branch..remote)" HEAD:` โ€” - `origin` for a same-repo head, the fork's remote for a write-allowed cross-repo head) โ€” - fast-forward by construction, never `--force` โ€” so a branch locked by a sibling worktree is not a - `git checkout` dead-end. + on a stale/detached mismatch) and pushing via an explicit refspec (`git push "$PUSH_REMOTE" + HEAD:`) to a **fail-closed** destination โ€” `origin` for a same-repo head; for a + write-allowed cross-repo head, the fork destination validated by **host + owner/repo** identity, + not by remote name: canonicalize the URL `git push` will actually use (`git remote get-url + --push`, which honors a `pushurl` that can differ from the fetch URL) and require it to equal the + head repo's own URL (`gh api repos/ --jq .html_url`), else read-only โ€” fast-forward + by construction, never `--force` โ€” so a branch locked by a sibling worktree is not a `git + checkout` dead-end. - The worker mechanics are reconciled to that contract: `reference/loop.md` ยง5.1.2 acquires the head via `gh pr checkout` and asserts `HEAD == the live headRefOid` in every checkout path (already-at- head, sibling-locked `--detach` reuse, and heal-via-checkout), degrading to read-only on mismatch; From a407f088ac0c22889dfdae690bbd8d27b7ab581c Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 21 Jul 2026 20:46:03 -0400 Subject: [PATCH 10/10] fix(source-control): complete babysit fork-push guard (owner + all push urls + branch identity) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three coherent gaps in the full-mode push guard, closed together: - Every push url, not just the first. `git remote get-url --push` returned only the first push url; git push writes to ALL of a remote's pushurls, so a second base-repo or attacker pushurl received the refspec after the first matched. Resolve --push --all and require EVERY url to canonicalize to the head repo. - Watched-owner trust boundary. A cross-repo head whose remote url matched headRepository stayed full-mode even when its owner was outside (external fork with maintainer edits) โ€” a push outside the trust boundary the safety contract's Stop And Ask forbids. Gate the fork path on the head owner being within before accepting the remote. - PR-branch identity before full mode. A clean worktree whose tip merely equalled headRefOid while on some OTHER local branch entered full mode; a fix committed there advanced that unrelated branch while only the refspec push landed on the PR branch. Require the checkout to be on the PR branch or detached. The stdin canonicalizer is shared for the head-repo html_url and every push url. safety.md Checkout And Push Invariants updated to describe the complete guard, and records its threat-model boundary: git's push-time url rewrites (url..pushInsteadOf and similar) are outside a static guard's scope, as they do not arise from the documented gh pr checkout flow. --- .../skills/babysit-prs/reference/loop.md | 58 +++++++++++++------ .../skills/babysit-prs/reference/safety.md | 29 +++++++--- 2 files changed, 60 insertions(+), 27 deletions(-) diff --git a/plugins/source-control/skills/babysit-prs/reference/loop.md b/plugins/source-control/skills/babysit-prs/reference/loop.md index 96d8d93792..891af4421c 100644 --- a/plugins/source-control/skills/babysit-prs/reference/loop.md +++ b/plugins/source-control/skills/babysit-prs/reference/loop.md @@ -145,6 +145,7 @@ A push channel arms for ONE PR at a time. Re-arm for each new PR in the loop. BRANCH="" PR_HEAD=$(gh pr view "$PR_NUMBER" --json headRefOid -q .headRefOid) CUR_WT=$(git rev-parse --show-toplevel) +CUR_BRANCH=$(git symbolic-ref --short -q HEAD || true) # empty when detached DEFAULT_BRANCH=$(gh repo view --json defaultBranchRef -q .defaultBranchRef.name) git fetch origin "$DEFAULT_BRANCH" if [ -n "$(git status --porcelain)" ]; then @@ -152,9 +153,12 @@ if [ -n "$(git status --porcelain)" ]; then # this loop did not create. echo "Working tree has uncommitted changes (possibly another session's WIP) โ€” processing read-only" CHECKOUT_MODE="read-only" -elif [ "$(git rev-parse HEAD)" = "$PR_HEAD" ]; then - # HEAD already IS the true PR head โ€” on the branch and current, or already - # detached at it. Safe to mutate. +elif [ "$(git rev-parse HEAD)" = "$PR_HEAD" ] && { [ -z "$CUR_BRANCH" ] || [ "$CUR_BRANCH" = "$BRANCH" ]; }; then + # HEAD already IS the true PR head AND we are on $BRANCH or detached โ€” not some + # other local branch that merely points at the same commit (a fix committed there + # would advance that unrelated branch while only the refspec push lands on + # $BRANCH). A coincidental same-tip match on another branch falls through to the + # gh pr checkout paths below, which put us on the actual PR head. CHECKOUT_MODE="full" elif git worktree list | grep -vF "$CUR_WT " | grep -q "\[$BRANCH\]"; then # Branch locked in a sibling worktree โ€” `git checkout $BRANCH` here dead-ends @@ -182,32 +186,50 @@ fi # name would push to the BASE repo, writing a same-named branch there instead of # updating the fork head (the cross-repo regression this guards; safety.md). # `branch..pushRemote`/`remote` may hold a remote name OR a URL, and a named -# remote can carry a separate `pushurl` that `git push` honors โ€” so resolve the -# actual PUSH url (`git remote get-url --push`) and canonicalize it to -# host + owner/repo, then require BOTH to equal the head repo's own canonical URL -# (`gh api repos/ --jq .html_url`), else read-only. This rejects a -# same-path remote on a DIFFERENT host and a fork fetch url masking a base pushurl. +# remote can carry separate `pushurl`(s) that `git push` honors โ€” so resolve the +# actual PUSH urls (`git remote get-url --push --all`) and canonicalize EACH to +# host + owner/repo, then require EVERY one to equal the head repo's own canonical +# URL (`gh api repos/ --jq .html_url`), else read-only. A cross-repo +# head is additionally gated on its owner being within . This +# rejects a same-path remote on a DIFFERENT host, a fork fetch url masking a base +# pushurl, an extra base/attacker pushurl past a matching first one, and an +# external-fork head outside the trust boundary. if [ "$CHECKOUT_MODE" = "full" ]; then if [ "$(gh pr view "$PR_NUMBER" --json isCrossRepository -q .isCrossRepository)" = "false" ]; then PUSH_REMOTE=origin else - # Canonicalize any git URL โ€” scheme://[user@]host[:port]/owner/repo, + # Canonicalize each line of stdin โ€” scheme://[user@]host[:port]/owner/repo, # user@host:owner/repo, or a bare host/owner/repo โ€” to host/owner/repo so the # comparison includes the HOST, not just the path: a same-path remote on a # different host (git@evil.example.com:owner/repo) must NOT satisfy it. - repo_id() { printf '%s\n' "$1" | sed -E 's#\.git$##; s#/+$##; s#^[a-zA-Z][a-zA-Z0-9+.-]*://##; s#^[^/@]*@##; s#:[0-9]+/#/#; s#:#/#'; } + canon() { sed -E 's#\.git$##; s#/+$##; s#^[a-zA-Z][a-zA-Z0-9+.-]*://##; s#^[^/@]*@##; s#:[0-9]+/#/#; s#:#/#'; } HEAD_REPO=$(gh pr view "$PR_NUMBER" --json headRepository -q .headRepository.nameWithOwner) - # headRepository carries no URL; resolve the head repo's canonical host+path - # from its authoritative html_url (a fork head is always on the base instance). - HEAD_ID=$(repo_id "$(gh api "repos/$HEAD_REPO" --jq .html_url 2>/dev/null)") + # Trust boundary: a cross-repo head is writable only when its owner is within + # (safety.md Stop And Ask โ€” an external-fork head outside the + # watched owners stays read-only even with maintainer edits enabled). Enforce it + # before accepting the fork remote so the pure-bash degrade path never pushes to + # a repository outside the trust boundary. + HEAD_OWNER=${HEAD_REPO%%/*} + case ",," in + *",$HEAD_OWNER,"*) : ;; + *) echo "Cross-repo head owner $HEAD_OWNER outside โ€” read-only"; CHECKOUT_MODE="read-only" ;; + esac + # headRepository carries no URL; canonicalize the head repo's authoritative + # html_url (a fork head is always on the base instance). + HEAD_ID=$(gh api "repos/$HEAD_REPO" --jq .html_url 2>/dev/null | canon) # git push consults pushRemote before remote; either may be a remote NAME or a - # URL. Resolve the PUSH url (--push honors remote..pushurl, which can - # differ from the fetch url) โ€” the address git push will actually write to. + # URL, and it writes to EVERY configured push url (`--push --all`; `--push` + # honors remote..pushurl, which can differ from the fetch url). Require ALL + # of them to canonicalize to the head repo โ€” any non-matching line (an extra + # base-repo or attacker pushurl past a matching first one) fails closed. A + # bare-URL config value is not a known remote, so get-url errors and the literal + # value is validated instead. PUSH_REMOTE=$(git config --get "branch.$BRANCH.pushRemote" \ || git config --get "branch.$BRANCH.remote" || true) - REMOTE_URL=$(git remote get-url --push "$PUSH_REMOTE" 2>/dev/null || printf '%s' "$PUSH_REMOTE") - if [ -z "$PUSH_REMOTE" ] || [ -z "$HEAD_ID" ] || [ "$(repo_id "$REMOTE_URL")" != "$HEAD_ID" ]; then - echo "Fork push remote does not resolve to the PR head repo ($HEAD_ID) โ€” read-only" + PUSH_URLS=$(git remote get-url --push --all "$PUSH_REMOTE" 2>/dev/null || printf '%s\n' "$PUSH_REMOTE") + BAD=$(printf '%s\n' "$PUSH_URLS" | canon | grep -vxF "$HEAD_ID" || true) + if [ -z "$PUSH_REMOTE" ] || [ -z "$HEAD_ID" ] || [ -z "$PUSH_URLS" ] || [ -n "$BAD" ]; then + echo "Fork push URL(s) do not all resolve to the PR head repo ($HEAD_ID) โ€” read-only" CHECKOUT_MODE="read-only" fi fi diff --git a/plugins/source-control/skills/babysit-prs/reference/safety.md b/plugins/source-control/skills/babysit-prs/reference/safety.md index f33abb241b..6f11b2e15c 100644 --- a/plugins/source-control/skills/babysit-prs/reference/safety.md +++ b/plugins/source-control/skills/babysit-prs/reference/safety.md @@ -38,7 +38,12 @@ value and its unset fallback. worktree outside ``), or on a **stale local branch tip** behind the PR head. If `HEAD` differs from that head, **stop** โ€” never merge, edit, or push onto a stale tip: a naive `git merge origin/` + push from a behind-head tip silently reverts the newest branch - commit(s). Safety comes from this assertion, not from the assigned `HEAD` happening to match. This + commit(s). Safety comes from this assertion, not from the assigned `HEAD` happening to match. The + assertion is also on **identity, not just the commit**: a clean worktree whose tip merely equals + `headRefOid` while checked out on some OTHER local branch must not enter full mode โ€” a fix committed + there advances that unrelated branch while only the refspec push lands on the PR branch, leaving the + other branch locally carrying this PR's work. Require the checkout to be on the PR branch or in + detached HEAD (a coincidental same-tip match on another branch heals via `gh pr checkout`). This extends the head-SHA re-check below โ€” which covered only the head moving *mid-work* โ€” to the moment the worktree is first assigned. - Re-check the PR head SHA immediately before editing and again immediately before pushing. Stop @@ -51,16 +56,22 @@ value and its unset fallback. same-repo vs fork from `gh pr view --json isCrossRepository`, never by whether `git config` happens to resolve: `origin` for a same-repo head; for a write-allowed cross-repo (in-owner fork) head, the fork destination from `branch..pushRemote` or `branch..remote`, validated - by URL โ€” a named remote can carry a separate `pushurl` that `git push` honors, so resolve the actual - push URL (`git remote get-url --push`) and canonicalize it (a remote name, a bare URL, or that - `pushurl`) to **host + owner/repo**, then require both to equal the head repo's own canonical URL - (`gh api repos/ --jq .html_url`; `gh pr view --json headRepository` exposes no URL), - not merely reject the literal `origin` name or match `owner/repo` on any host. Never hardcode + by URL and gated on the trust boundary. First require the cross-repo head's OWNER to be within + ``, else read-only (Stop And Ask, below) โ€” an external-fork head with maintainer + edits enabled must not receive a push just because its URL matches. Then, because a named remote can + carry separate `pushurl`(s) that `git push` honors and writes to ALL of, resolve the actual push URLs + (`git remote get-url --push --all`) and canonicalize EACH (a remote name, a bare URL, or those + `pushurl`s) to **host + owner/repo**, then require EVERY one to equal the head repo's own canonical + URL (`gh api repos/ --jq .html_url`; `gh pr view --json headRepository` exposes no + URL), not merely reject the literal `origin` name or match `owner/repo` on any host. Never hardcode `origin`, and never fall back to it when the destination cannot be validated โ€” a fork head reached via `--detach` leaves no branch config, and a remote named `upstream` (or any name), a same-`owner/repo` - path on a different host, or a fork fetch URL masking a base-repo `pushurl`, can point at the base - repo, so pushing there silently writes a same-named branch on base instead of - updating the fork head; **stop (read-only) instead**. Because `HEAD` equalled the PR head and you only added + path on a different host, a fork fetch URL masking a base-repo `pushurl`, or an extra base/attacker + `pushurl` past a matching first one, can point at the base repo, so pushing there silently writes a + same-named branch on base instead of updating the fork head; **stop (read-only) instead**. Known + limitation: this validates the push URLs resolvable at guard time; git's own push-time URL rewrites + (`url..pushInsteadOf` and similar) are outside the static guard's threat model, as they do not + arise from the documented `gh pr checkout` flow. Because `HEAD` equalled the PR head and you only added commits on top, this push is a fast-forward; never `--force` or `--force-with-lease`. A rejected non-fast-forward push means the assertion no longer holds โ€” re-fetch and stop, never force past it. (An external-fork head outside `` remains the read-only stop-and-ask case below.)