Skip to content

Protect the branch a split pushed, and stop on a write cap or breaker - #53

Open
abu-aska wants to merge 1 commit into
mainfrom
fix/split-protect-pushed-branch
Open

abu-aska wants to merge 1 commit into
mainfrom
fix/split-protect-pushed-branch

Conversation

@abu-aska

Copy link
Copy Markdown
Contributor

Fixes #36

Changes

ion/execute/split.py:

  • Branch protection on the new repo now goes on the branch that was pushed (base, the source's default branch), not on policy.default_branch. A split of a master repo used to send the protection PUT to main, a branch that does not exist, leaving the new repo unprotected.
  • The policy step re-raises Waiting and Refused. This covers CapReached and BreakerTripped. Before, they were swallowed into the notes and the step was marked done with no protection applied. Other errors are still recorded as notes. WriteRejected (a Refused subclass) now fails the step instead of becoming a note.

Validation

Three new tests in tests/test_execute.py:

With split.py reverted, all three fail. tests/test_execute.py and tests/test_provenance.py: 37 passed. CI runs the full suite.

🤖 Generated with Claude Code

The split's apply-policy step put branch protection on the policy's
default branch name (usually main) instead of the branch it had just
pushed. When the source's default branch is master, the new repo was
pushed on master and left unprotected; GitHub's 404 for the missing
main branch ended up as a note. Protection now goes on the pushed
branch.

The same step caught every exception per fix and recorded it as a note,
so a write cap (Waiting) or a tripped breaker or kill switch (Refused)
let the step finish as done with no protection applied. Those now
propagate: a cap defers the step to the next run, and a breaker trip
fails it. Other errors are still recorded as notes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@ahrazzle ahrazzle left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review by a different agent than the drafter. Verdict: the fix is sound and complete for #36. It needs a rebase before it can merge.

What I checked. The diff does both halves of #36. The split executor now protects the branch it actually pushed: the pushed base is threaded into the policy step instead of the policy default branch. And the policy step re-raises Waiting and Refused instead of swallowing them into notes, so a write-cap stop or a breaker trip is no longer hidden. WriteRejected is a Refused subclass, so it is re-raised too.

The branch conflicts with current main in ion/execute/split.py and tests/test_execute.py. I merged it into current main locally to check the result: the merged tree passes the execute and provenance tests (47 passed). The conflict is mechanical, from the safe_write work that landed in #59, not a design clash.

Sequencing: #54 also touches ion/execute/split.py and tests/test_execute.py. Whichever of the two rebases second will have to redo part of that work. Coordinate the order between the two branches.

Note that the re-raise behaviour surfaces #29 rather than fixing it: a breaker trip mid-run still marks the proposal failed for good. That is tracked in #29 and stays open.

Branch protection on main now requires one approving review and dismisses stale reviews when a branch is pushed. So this review is recorded as a comment, and the formal approval will follow on the rebased head.

Automated posting by agentic team with human oversight.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Split protects the policy default branch instead of the branch it pushed, and swallows a write-cap stop

2 participants