From 35ff6a200b13110c5816ef6f3f859b795ac415ed Mon Sep 17 00:00:00 2001 From: Trevin Chow Date: Mon, 27 Apr 2026 13:53:00 -0700 Subject: [PATCH 1/3] fix(commit-push-pr): branch from fresh remote base to prevent stale-base contamination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 4 of ce-commit-push-pr now creates feature branches from `origin/` after a `git fetch`, instead of from local `` directly. This prevents new branches from silently inheriting unpushed local commits when another session (Claude Code, Cursor, Codex, or a human) has advanced local default beyond the remote — a real failure mode reported in #707 that previously required force-push surgery to fix at PR review time. Falls back to the prior `git checkout -b ` form if the fetch fails (offline, restricted network, expired auth) and surfaces the fallback in the user-facing summary so freshness is not silently assumed. Also adds docs/solutions/workflow/stale-local-base-contamination.md documenting the failure mode, the rejected detection approaches (authorship-based misses same-user contamination; reachability-based inverts under stacked PRs) and why prevention at branch creation generalizes cleanly to stacked-PR workflows. Co-authored-by: Claude Opus 4.7 (1M context) --- .../stale-local-base-contamination.md | 98 +++++++++++++++++++ .../skills/ce-commit-push-pr/SKILL.md | 2 +- 2 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 docs/solutions/workflow/stale-local-base-contamination.md diff --git a/docs/solutions/workflow/stale-local-base-contamination.md b/docs/solutions/workflow/stale-local-base-contamination.md new file mode 100644 index 000000000..b6dcaba82 --- /dev/null +++ b/docs/solutions/workflow/stale-local-base-contamination.md @@ -0,0 +1,98 @@ +--- +title: "Stale local base contamination in multi-session branch creation" +category: workflow +date: 2026-04-27 +created: 2026-04-27 +severity: medium +component: ce-commit-push-pr +problem_type: workflow_issue +tags: + - branching + - multi-agent + - multi-session + - pre-push + - stacked-prs + - contamination +--- + +# Stale local base contamination in multi-session branch creation + +## Problem + +When multiple agent sessions (Claude Code, Cursor, Codex, plus any humans) share one local clone, local `` can drift relative to its remote counterpart. Two specific drifts cause downstream pain: + +1. **Local default behind remote.** Another session pushed and merged work; this session's local `main` doesn't know yet. +2. **Local default ahead of remote with unpushed work.** Another session committed locally to `main`, or merged a feature branch into local `main`, before pushing — and never pushed those commits to `origin/main`. + +When a session creates a feature branch from local `main` while drift type 2 holds, the new branch silently inherits the unpushed work. The eventual PR opens looking clean to the originating session but appears contaminated on GitHub. Resolving it requires force-push surgery during PR review. + +This came in as [issue #707](https://github.com/EveryInc/compound-engineering-plugin/issues/707). + +## Why post-facto detection is the wrong tool + +The intuitive fix is to detect the contamination before pushing or before opening a PR. Two detection approaches were considered and rejected: + +### Approach A: surface foreign commit authors + +Read `git log ..HEAD --pretty=format:'%h %ae %s'` and warn when any commit's author email differs from `git config user.email`. + +Catches the cross-author case (cherry-picks, teammate-authored work) but misses the dominant scenario: multi-agent setups where every session uses the same `user.email`. The check fires on intentional cherry-picks and stays silent on the actual contamination pattern. + +### Approach B: cross-branch reachability + +For each commit in `..HEAD`, check whether it is reachable from any other `origin/*` ref. If yes, treat as suspect. + +Authorship-agnostic, so it catches same-user contamination. But the signal it measures — "this commit is on another remote branch" — is the **defining characteristic** of stacked-PR workflows, where parent commits in the stack are intentionally shared with sibling branches. Tools like Graphite and git-spice rely on this. With GitHub-native stacked PRs moving toward general availability and likely broad adoption, the false-positive rate moves from "narrow population" to "majority of pushes for sophisticated users." The check would invert from useful signal to default noise. + +You can patch around it (parse stack metadata from PR base refs) but the patches multiply with every adjacent workflow (first push before PR exists, multi-level stacks, fork-based stacks). Each patch is a heuristic that will be wrong somewhere. + +## Solution + +Prevent at branch creation rather than detect at push or PR time. + +`ce-commit-push-pr` Step 4 — the branch-creation path used when the user invokes the skill while on the default branch with working-tree changes — was changed from: + +```bash +git checkout -b +``` + +to: + +```bash +git fetch --no-tags origin +git checkout -b origin/ +``` + +with a graceful fallback to the local-base form when the fetch fails (offline, restricted network, expired auth). The fallback is documented to the user so they know base freshness was not verified. + +This makes the skill's branch-creation path safe by construction: + +- Drift type 1 (local behind remote): the new branch starts at fresh remote ``, not stale local ``. +- Drift type 2 (local ahead of remote with unpushed work): unpushed local commits stay on local `` (recoverable via reflog or branch ref); the new feature branch starts clean. + +The principle generalizes cleanly to stacked PRs: when a user wants to stack on top of an open PR, the same `git fetch && git checkout -b origin/` pattern works — `` is just a different ref. Nothing about prevention depends on detecting "is this commit suspicious." + +## What this does not cover + +- **Branches created outside the skill.** Users who run `git checkout -b` manually, or whose IDE creates branches without fetching, can still produce contaminated branches. The skill's path becomes safe; the user's general workflow is not. A pre-push hook (which the original reporter installed) covers this case — opt-in hooks remain a reasonable user-side mitigation. +- **Already-contaminated branches.** Once a branch carries foreign commits, this change does nothing for it. Recovery is still manual: identify the foreign commits, drop them via interactive rebase or `git reset` to a clean base, force-push. +- **Step 1 branch-creation paths with different semantics.** When the user is on the default branch with unpushed commits and asks to create a feature branch to "rescue" those commits, the desired behavior is to carry the local commits onto the new branch — opposite of the Step 4 case. Step 1's behavior is unchanged. + +## User-side mitigations + +For workflows where branch creation happens outside the skill, recommend: + +- `git switch -c origin/` instead of `git checkout -b ` +- A `git config --global alias.nb '!f() { git fetch origin "${2:-main}" && git switch -c "$1" "origin/${2:-main}"; }; f'` style alias +- An opt-in pre-push hook that compares HEAD's parent chain against `origin/` for unexpected commits — useful for individual users, but not shipped from this plugin because the cost of getting stacked-PR semantics right in a hook outweighs the benefit at the plugin level + +## Why we did not ship a detection check at all + +The reporter framed their issue as "a pattern, not a request for a merge." Taking that at its word and acting on the structural signal — a real failure mode worth a permanent fix — produced this outcome: + +- One small preventive change in the skill that is safe by construction +- A documented pattern with rationale for future readers +- No behavioral prompt added to a heavy-traffic skill +- No detection heuristic that risks being obsoleted by stacked PRs + +A detection check at push or PR time was not free even when scoped tightly: it adds a prompt to a frictionless workflow, false-positives on legitimate workflows that share commits across branches, and would require ongoing tuning as stacked-PR conventions evolve. Prevention at the right layer avoids all of that. diff --git a/plugins/compound-engineering/skills/ce-commit-push-pr/SKILL.md b/plugins/compound-engineering/skills/ce-commit-push-pr/SKILL.md index fb579b9c2..1d8639fc4 100644 --- a/plugins/compound-engineering/skills/ce-commit-push-pr/SKILL.md +++ b/plugins/compound-engineering/skills/ce-commit-push-pr/SKILL.md @@ -136,7 +136,7 @@ If the PR check returned `state: OPEN`, note the URL -- this is the existing-PR ### Step 4: Branch, stage, and commit -1. If on the default branch, create a feature branch first with `git checkout -b `. +1. If on the default branch, create the feature branch from a fresh remote base. Run `git fetch --no-tags origin `, then `git checkout -b origin/`. This avoids inheriting unpushed local commits from another session that advanced local `` (a real failure mode in multi-agent and multi-worktree setups). If the fetch fails, fall back to `git checkout -b ` and note in the user-facing summary that base freshness was not verified. 2. Scan changed files for naturally distinct concerns. If files clearly group into separate logical changes, create separate commits (2-3 max). Group at the file level only (no `git add -p`). When ambiguous, one commit is fine. 3. Stage and commit each group in a single call. Avoid `git add -A` or `git add .`. Follow conventions from Step 2: ```bash From 8d61af8dd9cd4667630e049f2168a9de1e28b74f Mon Sep 17 00:00:00 2001 From: Trevin Chow Date: Mon, 27 Apr 2026 15:07:02 -0700 Subject: [PATCH 2/3] fix(commit-push-pr): handle checkout failure during clean-base creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 4.1 only fell back when `git fetch` failed, but `git checkout -b origin/` can also abort with "Your local changes would be overwritten" when uncommitted edits collide with the local-base-vs- origin-base diff — exactly the contamination scenario the previous commit aimed to protect against. The skill would strand users with their work and no path forward. Now stashes with `git stash push -u`, retries the checkout from `origin/`, then `git stash pop` onto the new branch. If the pop conflicts (rare same-files case), surfaces the conflict and stash ref so the user can resolve manually. Addresses Codex review feedback on #708. Co-authored-by: Claude Opus 4.7 (1M context) --- plugins/compound-engineering/skills/ce-commit-push-pr/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/compound-engineering/skills/ce-commit-push-pr/SKILL.md b/plugins/compound-engineering/skills/ce-commit-push-pr/SKILL.md index 1d8639fc4..86d63dd1b 100644 --- a/plugins/compound-engineering/skills/ce-commit-push-pr/SKILL.md +++ b/plugins/compound-engineering/skills/ce-commit-push-pr/SKILL.md @@ -136,7 +136,7 @@ If the PR check returned `state: OPEN`, note the URL -- this is the existing-PR ### Step 4: Branch, stage, and commit -1. If on the default branch, create the feature branch from a fresh remote base. Run `git fetch --no-tags origin `, then `git checkout -b origin/`. This avoids inheriting unpushed local commits from another session that advanced local `` (a real failure mode in multi-agent and multi-worktree setups). If the fetch fails, fall back to `git checkout -b ` and note in the user-facing summary that base freshness was not verified. +1. If on the default branch, create the feature branch from a fresh remote base. Run `git fetch --no-tags origin `, then `git checkout -b origin/`. This avoids inheriting unpushed local commits from another session that advanced local `` (a real failure mode in multi-agent and multi-worktree setups). If the fetch fails, fall back to `git checkout -b ` and note in the user-facing summary that base freshness was not verified. If the checkout fails because uncommitted changes would be overwritten (local `` diverges from `origin/` in files you've also edited), run `git stash push -u -m "ce-commit-push-pr: pre-branch "`, retry `git checkout -b origin/`, then `git stash pop` to restore the changes onto the new branch. If the pop reports conflicts, surface the conflict and the stash ref to the user so they can resolve manually. 2. Scan changed files for naturally distinct concerns. If files clearly group into separate logical changes, create separate commits (2-3 max). Group at the file level only (no `git add -p`). When ambiguous, one commit is fine. 3. Stage and commit each group in a single call. Avoid `git add -A` or `git add .`. Follow conventions from Step 2: ```bash From d8826490a550cf543af48d7414acd1d9d3f37251 Mon Sep 17 00:00:00 2001 From: Trevin Chow Date: Mon, 27 Apr 2026 16:34:34 -0700 Subject: [PATCH 3/3] fix(commit-push-pr): preserve intentional unpushed commits when branching from default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2 of feedback on Step 4.1: the previous version always branched from `origin/`, which silently dropped unpushed commits the user intended for the new feature branch (the "forgot to branch first" workflow). Local git state cannot distinguish that intent from stale-base contamination — the unpushed commits look identical and shared `user.email` makes authorship unreliable across multi-agent sessions. Step 4.1 now asks the user when `origin/..HEAD` is non-empty: carry the commits onto the new branch (HEAD) or leave them on local `` (origin/``). The user is the only reliable source of intent here. Step 4.1's prose has accumulated three conditional branches across two review rounds (fetch, unpushed-commits, checkout overwrite). Extracted the full decision flow to `references/branch-creation.md` per the extraction guidance for conditional content; Step 4.1 in SKILL.md is now a 1-line stub. The round-1 stash-retry-pop fallback is preserved in the reference file. Stable/Beta sync: not propagating — no `-beta` counterpart exists for `ce-commit-push-pr`. Addresses Codex review feedback on #708. Co-authored-by: Claude Opus 4.7 (1M context) --- .../skills/ce-commit-push-pr/SKILL.md | 2 +- .../references/branch-creation.md | 67 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 plugins/compound-engineering/skills/ce-commit-push-pr/references/branch-creation.md diff --git a/plugins/compound-engineering/skills/ce-commit-push-pr/SKILL.md b/plugins/compound-engineering/skills/ce-commit-push-pr/SKILL.md index 86d63dd1b..7b27f9183 100644 --- a/plugins/compound-engineering/skills/ce-commit-push-pr/SKILL.md +++ b/plugins/compound-engineering/skills/ce-commit-push-pr/SKILL.md @@ -136,7 +136,7 @@ If the PR check returned `state: OPEN`, note the URL -- this is the existing-PR ### Step 4: Branch, stage, and commit -1. If on the default branch, create the feature branch from a fresh remote base. Run `git fetch --no-tags origin `, then `git checkout -b origin/`. This avoids inheriting unpushed local commits from another session that advanced local `` (a real failure mode in multi-agent and multi-worktree setups). If the fetch fails, fall back to `git checkout -b ` and note in the user-facing summary that base freshness was not verified. If the checkout fails because uncommitted changes would be overwritten (local `` diverges from `origin/` in files you've also edited), run `git stash push -u -m "ce-commit-push-pr: pre-branch "`, retry `git checkout -b origin/`, then `git stash pop` to restore the changes onto the new branch. If the pop reports conflicts, surface the conflict and the stash ref to the user so they can resolve manually. +1. If on the default branch, branch creation needs to handle three conditional cases: stale local ``, unpushed commits on local `` (intent unclear without asking), and uncommitted changes that collide with the fresh remote base. Read `references/branch-creation.md` and follow its decision flow, then continue to step 2 below. 2. Scan changed files for naturally distinct concerns. If files clearly group into separate logical changes, create separate commits (2-3 max). Group at the file level only (no `git add -p`). When ambiguous, one commit is fine. 3. Stage and commit each group in a single call. Avoid `git add -A` or `git add .`. Follow conventions from Step 2: ```bash diff --git a/plugins/compound-engineering/skills/ce-commit-push-pr/references/branch-creation.md b/plugins/compound-engineering/skills/ce-commit-push-pr/references/branch-creation.md new file mode 100644 index 000000000..f03acf6b5 --- /dev/null +++ b/plugins/compound-engineering/skills/ce-commit-push-pr/references/branch-creation.md @@ -0,0 +1,67 @@ +# Branch creation from default branch + +When Step 4 fires on the default branch, the local `` (e.g., local `main`) is untrustworthy as a starting point. Two failure modes drive this: + +- **Stale-base contamination.** Another agent session, worktree, or background process may have advanced local `` past `origin/` with commits unrelated to this work. Branching from local HEAD would carry those into the new feature branch and the eventual PR. +- **Forgot-to-branch.** The user authored real commits on local `` intending them for a feature branch. Branching from `origin/` would silently drop them. + +Local git state alone cannot distinguish these two — the unpushed commits look identical. Author email is unreliable in multi-session setups where every session shares `user.email`. The skill therefore surfaces the choice to the user when unpushed commits are present. + +## Decision flow + +Run the steps in order. Each step's outcome determines whether the next runs. + +### 1. Fetch fresh remote base + +```bash +git fetch --no-tags origin +``` + +- **Fetch succeeds:** continue to step 2. +- **Fetch fails:** skip to the bottom of this file, "Fetch failure fallback". + +### 2. Check for unpushed local commits on `` + +```bash +git log origin/..HEAD --oneline +``` + +- **Empty output:** no unpushed commits — proceed to step 3 with `BASE_REF=origin/`. +- **Non-empty output:** unpushed commits exist on local ``. Show the user the commit list and ask (per the platform's blocking question tool — see the "Asking the user" convention at the top of `SKILL.md`): + + > "Local `` has N unpushed commits not on `origin/`. Carry them onto the new feature branch, or leave them on local ``?" + + Two options: + - **Carry forward** (intent: "I forgot to branch first") — set `BASE_REF=HEAD`. The new branch starts from current local HEAD, preserving the commits. + - **Leave on ``** (intent: "stale-base contamination from another session") — set `BASE_REF=origin/`. The new branch starts clean; the commits remain on local `` for the user to handle separately. + + If the blocking tool is unavailable and you must fall back to chat, default to **leaving on ``** only after the user replies — never silently. Carrying foreign commits into a PR is a worse failure than asking again. + +### 3. Create the feature branch + +```bash +git checkout -b "$BASE_REF" +``` + +- **Checkout succeeds:** branch created, continue to Step 4.2 in `SKILL.md`. +- **Checkout fails because uncommitted changes would be overwritten:** local `` diverges from `origin/` in files the user has also edited. Stash, retry, pop: + + ```bash + git stash push -u -m "ce-commit-push-pr: pre-branch " + git checkout -b "$BASE_REF" + git stash pop + ``` + + If `git stash pop` reports conflicts (rare same-files case), surface the conflict output and the stash ref to the user so they can resolve manually. Do not attempt to auto-resolve. + + Note: this stash-retry-pop applies whether `BASE_REF` is `HEAD` or `origin/`. In the `HEAD` case the overwrite collision is unlikely (the index was already at HEAD), but the same recovery sequence is safe to run. + +## Fetch failure fallback + +If `git fetch --no-tags origin ` fails (network, auth, no remote), the skill cannot verify base freshness or detect unpushed commits remotely. Fall back to: + +```bash +git checkout -b +``` + +This branches from current local HEAD. Note in the user-facing summary that base freshness was not verified. Do not attempt the unpushed-commits check — without a fresh `origin/`, the answer is unreliable.