Skip to content

post-merge-cleanup: the Phase 1 ancestry test STOPs on every squash merge, and Phase 4 assumes the local default ref is current #586

Description

@SUaDtL

Found on a real run (2026-08-01) cleaning up sprint/portable-release-and-protected-state
after PR #576 merged. Two independent defects surfaced in one pass, plus one thing that
works but is undocumented and reads like a defect.

1. Phase 1's instrument contradicts Phase 1's own prose (squash merges)

post-merge-cleanup Phase 1 step 2 mandates:

Confirm the current branch is an ancestor of the fetched default
(git merge-base --is-ancestor HEAD origin/<default>).

while the very same paragraph asserts:

a squash-merged branch whose remote still exists is fully contained in the default
branch and is safe to delete

Both cannot hold. A squash merge writes a new commit with no SHA lineage to the branch,
so --is-ancestor returns non-zero for every squash-merged branch. Observed:

$ git merge-base --is-ancestor HEAD origin/main ; echo $?
1

...while the branch was in fact fully landed:

A literal reading of the gate ("If HEAD is not an ancestor, STOP. Name the un-landed
commits and route to /ca:pr") stops the lane on this repo's default merge mode and routes
to a command with nothing to do. It also makes an ordinary cleanup look like it needs a
bypass — precisely the /ca:override reach that #308 created this skill to eliminate.

Suggested contract. The gate is content-containment, not SHA-ancestry. Accept either:

  • git merge-base --is-ancestor HEAD origin/<default> — the ff / merge-commit case; or
  • squash-merge proof: git diff --quiet origin/<default> HEAD and the PR record reports
    MERGED with headRefOid == local HEAD.

Report whichever held as a fact, including the ancestry check's negative result. Anything
else STOPs as it does today.

2. Phase 4 assumes the local default ref is current

Phase 4 step 1 says check out the default branch, then verify the checkout happened. It does
not consider that the local default ref may lag the fetched one. Observed: local main was
2 commits behind origin/main, and its .codearbiter/gate-events.log differed from HEAD's
by 513 lines, so the checkout was refused outright:

	.codearbiter/gate-events.log
Please commit your changes or stash them before you switch branches.
Aborting

The Phase 3 residue (38 appended lines, correctly classified unique and kept) was not the
blocker — the stale local ref was. Phase 3's "if anything the user chose to keep would block
the checkout, STOP and say so" therefore fires for the wrong reason and reads as though the
kept artifact is at fault, inviting exactly the silent discard Phase 3 exists to prevent.

Suggested contract. Phase 4 fast-forwards the local default ref before the checkout —
git fetch origin <default>:<default> refuses a non-fast-forward, so the --ff-only
guarantee is preserved — then checks out, then verifies. Phase 3's blocked-checkout STOP
should distinguish "a kept artifact conflicts" from "the local default ref is stale", since
only the first is the user's to resolve.

3. Not a defect, but undocumented: why -d accepts a squash-merged branch

Phase 5 mandates git branch -d, never -D, and says a refusal means the ancestry proof and
the refusal disagree. On this run -d accepted the squash-merged branch, because its
upstream was configured and equal to the tip:

warning: deleting branch 'sprint/portable-release-and-protected-state' that has been merged to
         'refs/remotes/origin/sprint/portable-release-and-protected-state', but not yet merged to HEAD
Deleted branch sprint/portable-release-and-protected-state (was 6bbf64b0).

git branch -d accepts a branch merged into its upstream, not only into HEAD. Worth
stating in Phase 5 so the next reader does not conclude -D is required and reach for it.
Where no upstream is configured, -d genuinely will refuse and the existing "report both
and stop" contract applies unchanged.

Scope of fix

Source of truth is core/surface/skills/post-merge-cleanup/SKILL.md; CI enforces
byte-identity with three vendored copies:

  • plugins/ca/skills/post-merge-cleanup/SKILL.md
  • plugins/ca-codex/routines/post-merge-cleanup/SKILL.md
  • plugins/ca-pi/routines/post-merge-cleanup/SKILL.md

core/surface/commands/cleanup.md restates the hard gate verbatim ("MUST prove the current
branch is an ancestor of the fetched default branch") and needs the same wording change,
along with its plugins/ca/commands/cleanup.md copy.

site/src/curated/commands/cleanup.md is hand-written and is not compared to the surface by
any gate — check it for the same claim so the published docs do not keep asserting the old
contract.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingsev:medTribunal/triage: medium severity

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions