fix(commit-push-pr): branch from fresh remote base to prevent stale-base contamination#708
fix(commit-push-pr): branch from fresh remote base to prevent stale-base contamination#708
Conversation
…ase contamination Step 4 of ce-commit-push-pr now creates feature branches from `origin/<base>` after a `git fetch`, instead of from local `<base>` 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 <name>` 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) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35ff6a200b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Step 4.1 only fell back when `git fetch` failed, but `git checkout -b <name> origin/<base>` 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/<base>`, 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) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d61af8dd9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…hing from default Round 2 of feedback on Step 4.1: the previous version always branched from `origin/<base>`, 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/<base>..HEAD` is non-empty: carry the commits onto the new branch (HEAD) or leave them on local `<base>` (origin/`<base>`). 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) <noreply@anthropic.com>
…ase contamination (EveryInc#708) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Closes #707.
ce-commit-push-prStep 4 now creates feature branches from a freshly fetchedorigin/<base>instead of from local<base>. This prevents the new branch from silently inheriting unpushed commits left on local default by another session — a real failure mode in multi-agent and multi-worktree setups that previously required force-push surgery at PR review time.What changed
plugins/compound-engineering/skills/ce-commit-push-pr/SKILL.mdgit fetch --no-tags origin <base>thengit checkout -b <name> origin/<base>. Graceful fallback to local-base creation when fetch fails, surfaced in the user-facing summary so freshness is not silently assumed.docs/solutions/workflow/stale-local-base-contamination.mdWhy prevention, not detection
Two detection approaches were considered before landing on prevention:
%aeagainstgit config user.email) misses the dominant case in the issue's intended setup. Multi-agent sessions on one clone shareuser.email, so foreign commits leak in carrying the user's own author email. The check fires on intentional cherry-picks and stays silent on the actual contamination pattern.<base>..HEADreachable from anotherorigin/*) catches same-user contamination, but it is the defining characteristic of stacked-PR workflows. With GitHub-native stacked PRs heading to general availability, this signal inverts from useful to default-noise for a meaningful fraction of pushes.Prevention at branch creation generalizes cleanly to stacked workflows —
<base>is just a different ref when stacking on top of a parent PR — and does not depend on any "is this commit suspicious" heuristic. The skill's own branch-creation path becomes safe by construction.The solutions doc records this rationale so the structural reasoning survives future readers.
What this does not cover
git checkout -b, IDE actions) — user-side pre-push hooks remain the right tool for that broader surfaceTest plan
bun test— 960 pass, 0 failbun run release:validate— clean, no manifest drift