Repository navigation
Undo the branch when create's commit fails - #29
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Cancellation can prevent cleanup and leave the created branch checked out but untracked.
3 open findings
What changed in this PR
Adds rollback handling when branch creation succeeds but the commit fails.
Changes:
- Removes failed-create branches while preserving staged changes.
- Improves signing-failure recovery guidance.
- Adds rollback tests and updates MCP/architecture documentation.
| File | Description |
|---|---|
pkg/app/create.go |
Implements failed-create rollback. |
pkg/app/modify.go |
Adds Modify retry guidance. |
pkg/app/staging.go |
Makes signing guidance command-specific. |
pkg/app/mutate_test.go |
Tests rollback outcomes. |
pkg/mcp/tools.go |
Documents MCP create behavior. |
docs/architecture.md |
Documents rollback design. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ecf4d78 to
a41f85d
Compare
Create makes the branch, checks it out and registers it with gh stack before it commits, so a failed commit (a signing key nobody could unlock, a pre-commit hook saying no) left a tracked branch sat at the parent's commit with nothing of its own and the changes still staged. The error didn't say so either; from the MCP side it looked like nothing had happened, and calling stack_create again fails because the branch already exists. The only way out was to notice it in stack_view and reach for stack_modify instead. Create now undoes itself when the commit fails: the branch is forgotten through Update (an empty stack goes with it), we switch back to the parent and delete the branch. Both point at the same commit so the staged changes are untouched, and the error says nothing was created and to run create again once the cause is fixed. If the undo itself fails the error says what was left behind and that modify -c commits on it. The tool description and architecture notes say the same.
a41f85d to
e3f17b9
Compare
| if tip, ok, terr := a.d.Git.Tip(ctx, repo, name); terr == nil && ok { | ||
| if parentTip, _, perr := a.d.Git.Tip(ctx, repo, parent); perr == nil && tip != parentTip { |
There was a problem hiding this comment.
If git can't resolve either ref, the undo's own git calls fail the same way and the error reports what was left behind rather than silently deleting anything, since each step checks its error. A branch -D after a commit that did land is also still in the reflog. Not going to add more ceremony for that corner.
| and the staged changes are still staged; the error says to run it again. If the undo itself | ||
| fails the error says what was left behind and that `modify -c` commits on it. |
There was a problem hiding this comment.
The step text already covers both states: it says to check git stack log, commit with modify -c if the branch is still tracked, otherwise delete it. The architecture note is the summary; I'll leave it at that.
| Name: "stack_create", | ||
| Title: "Create a stacked branch", | ||
| Description: "Create a new branch on top of the current branch, commit staged changes with the given message, and register it in the stack. From the trunk this starts a new stack. Fails with not_at_top when the current branch is not the top of its stack.", | ||
| Description: "Create a new branch on top of the current branch, commit staged changes with the given message, and register it in the stack. From the trunk this starts a new stack. Fails with not_at_top when the current branch is not the top of its stack. If the commit fails (signing_failed, a hook) the branch is removed again and the changes stay staged, so fix the cause and call it again; should removing it fail too, next_steps says what was left behind and how to commit on it.", |
There was a problem hiding this comment.
Same answer as on the architecture note: next_steps for the failed-undo case already distinguishes still tracked (modify -c) from untracked (delete it), so a client reads that rather than the description. Leaving the description as the summary.
Create makes the branch, checks it out and registers it with gh stack before it commits, so a failed commit (a signing key nobody could unlock, a pre-commit hook saying no) left a tracked branch sat at the parent's commit with nothing of its own and the changes still staged. The error didn't say so either; from the MCP side it looked like nothing had happened, and calling
stack_createagain fails because the branch already exists. The only way out was to notice it instack_viewand reach forstack_modifyinstead.Create now undoes itself when the commit fails: the branch is forgotten through
Update(an empty stack goes with it), we switch back to the parent and delete the branch. Both point at the same commit so the staged changes are untouched, and the error says nothing was created and to run create again once the cause is fixed. If the undo itself fails the error says what was left behind and thatmodify -ccommits on it. The tool description and architecture notes say the same.