Skip to content

Abort the run when a git/gh command fails unexpectedly#52

Open
Phlogistique wants to merge 6 commits into
mainfrom
claude/run-try-die-wrappers
Open

Abort the run when a git/gh command fails unexpectedly#52
Phlogistique wants to merge 6 commits into
mainfrom
claude/run-try-die-wrappers

Conversation

@Phlogistique

@Phlogistique Phlogistique commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

set -e is suppressed inside if/&&/|| conditions and everything they call, including the whole body of update_direct_target, which both entry points invoke as a condition. So most failures there fell through silently: a failed git checkout let the merges and the final git reset --hard run on the previously checked-out branch (corrupting it, in main's loop over several child PRs), and a failed git commit-tree let the function return success, after which main retargeted the PR onto a base its head does not contain. A failed conflict comment still added the label, leaving a resume with no state marker.

Every log_cmd call site now states its failure policy: run (log, execute, die on failure; die's explicit exit aborts from any context) or try (log, execute, hand the status to the caller) for the commands whose failure is an expected outcome. The explicit read-failure aborts from #50 become die too, with regression scenarios for both reads. set -e stays as a backstop only.

https://claude.ai/code/session_01STkeSJ7cLrmrNn4aTDYkwH


Generated by Claude Code

claude added 2 commits June 9, 2026 21:37
…esolution

If origin/<old base> was deleted while a child PR sat in conflict
(auto-delete head branches left enabled despite the README, or manual
deletion), the resume merge failed with "not something we can merge" and
git merge --abort failed too ("There is no merge to abort"). The run then
exited nonzero after re-posting the misleading conflict comment and the
label, repeating on every push. (Not a mid-function set -e kill:
update_direct_target is called in an if condition, which suppresses
errexit; observed via the new test scenario.)

Check origin/<old base> up front like #39 did for the target branch, and
only run git merge --abort when MERGE_HEAD exists.

https://claude.ai/code/session_01STkeSJ7cLrmrNn4aTDYkwH
set -e is suppressed inside if/&&/|| conditions and everything they call,
including the whole body of update_direct_target, which both entry points
invoke as a condition. So most failures there fell through silently: a
failed git checkout let the merges and the final git reset --hard run on
the previously checked-out branch (corrupting it, in main's loop over
several child PRs), and a failed git commit-tree let the function return
success, after which main retargeted the PR onto a base its head does not
contain. A failed conflict comment still added the label, leaving a resume
with no state marker.

Every log_cmd call site now states its failure policy: run (log, execute,
die on failure; die's explicit exit aborts from any context) or try (log,
execute, hand the status to the caller) for the merges whose failure is an
expected outcome. set -e stays as a backstop only.

https://claude.ai/code/session_01STkeSJ7cLrmrNn4aTDYkwH
@Phlogistique

Copy link
Copy Markdown
Collaborator Author

huh how come this wasnt updated automatically?

Re-applied onto the restructured script: the merge --abort guard and the
origin/OLD_BASE existence check (now via the PR-number abandon_resume).
The regression test becomes scenario F, adapted to the PR_BASE payload.

https://claude.ai/code/session_01STkeSJ7cLrmrNn4aTDYkwH
Phlogistique added a commit that referenced this pull request Jun 11, 2026
#55)

If `origin/<old base>` was deleted while a child PR sat in conflict
(auto-delete head branches left enabled despite the README, or manual
deletion), the resume merge failed with "not something we can merge" and
`git merge --abort` failed too ("There is no merge to abort"). The run
then exited nonzero after re-posting the misleading conflict comment and
the label, repeating on every push. (Not a mid-function `set -e` kill:
`update_direct_target` is called in an `if` condition, which suppresses
errexit; observed via the new test scenario.)

Check `origin/<old base>` up front like the existing target-branch
check, and only run `git merge --abort` when `MERGE_HEAD` exists.

Base of the stack continued in #52 and #53.

https://claude.ai/code/session_01STkeSJ7cLrmrNn4aTDYkwH

---
_Generated by [Claude
Code](https://claude.ai/code/session_01STkeSJ7cLrmrNn4aTDYkwH)_

Co-authored-by: Claude <noreply@anthropic.com>
@Phlogistique Phlogistique marked this pull request as ready for review June 11, 2026 13:09
@github-actions

Copy link
Copy Markdown

⚠️ Automatic update blocked by merge conflicts

Resolve them like this:

git fetch origin
git switch claude/run-try-die-wrappers
git merge --ff-only origin/claude/run-try-die-wrappers
git merge origin/claude/git-merge-abort-crash-sc64z6

Fix the conflicts (for instance with git mergetool), then run git commit before continuing.

git push origin claude/run-try-die-wrappers

Once you push, this action will resume and finish updating this pull request.

@github-actions github-actions Bot added the autorestack-needs-conflict-resolution PR needs manual conflict resolution label Jun 11, 2026
claude and others added 2 commits June 11, 2026 13:12
…on main

Re-applied the run/try/die conversion over the restructured script: the
helpers from the PR-number and merge-method work are converted too, and the
explicit read-failure aborts in read_state_marker / pr_has_conflict_label
become die (regression scenarios G and H). list_child_prs gets try, not run:
its callers consume it through a process substitution, which swallows the
status either way.

https://claude.ai/code/session_01STkeSJ7cLrmrNn4aTDYkwH
@github-actions github-actions Bot changed the base branch from claude/git-merge-abort-crash-sc64z6 to main June 11, 2026 13:13
@github-actions github-actions Bot removed the autorestack-needs-conflict-resolution PR needs manual conflict resolution label Jun 11, 2026
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.

2 participants