From 5049ca702a2317ae6d0f20fd6832646bbcea52f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 20:49:24 +0000 Subject: [PATCH 01/12] Push updated heads before retargeting PR bases in the fan-out path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The squash-merge fan-out retargeted every updated child PR onto the target branch and only afterwards pushed the new heads, batched into a single non-atomic push together with the merged-branch deletion. If the push failed (e.g. someone pushed to a child mid-run, rejecting the plain push) or a pr edit died partway through the loop, set -e aborted the run with PRs already retargeted but their heads stale - and unlike the conflict-resume path there is no label to re-trigger the action, so nothing ever repaired them. Apply the ordering the resume path already uses: push the updated heads first, then flip the bases, and delete the merged branch last (deleting a PR's base branch closes the PR, so every child must be off it first). A failed push now leaves the PRs untouched on their old base. The unit test captures the run transcript and asserts the push -> retarget -> delete order; it fails against the previous code. Also corrects the README: pushes are plain, not forced, and branch deletion is its own final step. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JHvKryT4QUpHYdNq9YEQxX --- README.md | 5 +++-- tests/test_update_pr_stack.sh | 20 ++++++++++++++++++-- update-pr-stack.sh | 22 ++++++++++++++-------- 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index f55449e..9b6094e 100644 --- a/README.md +++ b/README.md @@ -19,8 +19,9 @@ This action tries to fix that in a transparent way. Install it, and hopefully th 1. Triggers when a PR is squash merged 2. Finds PRs that were based on the merged branch (direct children only) 3. Creates a synthetic merge commit with three parents (child tip, deleted branch tip, squash commit) to preserve history without re-introducing code -4. Updates the direct child PRs to base on trunk now that the bottom change has landed -5. Force-pushes updated branches and deletes the merged branch +4. Pushes the updated branches +5. Updates the direct child PRs to base on trunk now that the bottom change has landed +6. Deletes the merged branch **Note:** Indirect descendants (grandchildren, etc.) are intentionally not modified. Their PR diffs remain correct because the merge-base calculation still works—the synthetic merge commit includes the original parent commit as an ancestor. When their direct parent is eventually merged, they become direct children and get updated at that point. diff --git a/tests/test_update_pr_stack.sh b/tests/test_update_pr_stack.sh index 4317f5e..2930aac 100755 --- a/tests/test_update_pr_stack.sh +++ b/tests/test_update_pr_stack.sh @@ -1,6 +1,6 @@ #!/bin/bash -set -e +set -eo pipefail # Get script directory (needed for static mock files) SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -71,6 +71,8 @@ echo "Simulated Squash commit (via cherry-pick): $SQUASH_COMMIT" echo "Running update-pr-stack.sh..." # The update script sources command_utils.sh itself +# Capture stdout+stderr interleaved so command ordering can be asserted. +RUN_LOG="$TEST_REPO/update_run.log" run_update_pr_stack() { log_cmd \ env \ @@ -79,10 +81,24 @@ run_update_pr_stack() { TARGET_BRANCH=main \ GH="$SCRIPT_DIR/mock_gh.sh" \ GIT="$SCRIPT_DIR/mock_git.sh" \ - $SCRIPT_DIR/../update-pr-stack.sh + $SCRIPT_DIR/../update-pr-stack.sh 2>&1 | tee "$RUN_LOG" } run_update_pr_stack +# The head must be pushed before the PR is retargeted (a failed push must leave +# the PR untouched on its old base), and the merged branch deleted only after +# the retarget (deleting a PR's base branch closes the PR). +push_line=$(grep -n "git push origin feature2" "$RUN_LOG" | head -1 | cut -d: -f1 || true) +edit_line=$(grep -n "pr edit feature2 --base main" "$RUN_LOG" | head -1 | cut -d: -f1 || true) +delete_line=$(grep -n "git push origin :feature1" "$RUN_LOG" | head -1 | cut -d: -f1 || true) +if [[ -n "$push_line" && -n "$edit_line" && -n "$delete_line" \ + && "$push_line" -lt "$edit_line" && "$edit_line" -lt "$delete_line" ]]; then + echo "✅ Ordering: push head, then retarget base, then delete merged branch" +else + echo "❌ Wrong ordering (push=$push_line edit=$edit_line delete=$delete_line)" + exit 1 +fi + # Verify the results cd "$TEST_REPO" diff --git a/update-pr-stack.sh b/update-pr-stack.sh index a2d8ca5..0669f82 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -311,20 +311,26 @@ main() { fi done - # Only update base branches for successfully updated PRs + # Push the updated heads before retargeting, for the same reasons as the + # conflict-resume path: the head already contains TARGET_BRANCH when the + # base flips to it, keeping the PR mergeable (GitHub suppresses CI on a PR + # that conflicts with its base), and a failed push leaves the PR untouched + # on its old base instead of retargeted onto a stale head with nothing to + # re-trigger this action. + if [[ "${#UPDATED_TARGETS[@]}" -gt 0 ]]; then + log_cmd git push origin "${UPDATED_TARGETS[@]}" + fi + for BRANCH in "${UPDATED_TARGETS[@]}"; do log_cmd gh pr edit "$BRANCH" --base "$TARGET_BRANCH" done - # Push updated branches; only delete merged branch if no conflicts + # Delete the merged branch last: deleting a PR's base branch closes the PR, + # so every child must be retargeted off it first. Keep it while conflicted + # PRs remain so it can be referenced during manual resolution. if [[ "${#CONFLICTED_TARGETS[@]}" -eq 0 ]]; then - # No conflicts - safe to delete merged branch - log_cmd git push origin ":$MERGED_BRANCH" "${UPDATED_TARGETS[@]}" + log_cmd git push origin ":$MERGED_BRANCH" else - # Some conflicts - keep merged branch for reference during manual resolution - if [[ "${#UPDATED_TARGETS[@]}" -gt 0 ]]; then - log_cmd git push origin "${UPDATED_TARGETS[@]}" - fi echo "⚠️ Keeping branch '$MERGED_BRANCH' - still referenced by conflicted PRs: ${CONFLICTED_TARGETS[*]}" fi } From f1f2a0e8b907932b8a05afa89f924efedd833f33 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 21:07:27 +0000 Subject: [PATCH 02/12] Fix e2e merge-command assertion broken by the ff-only step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since #40 the conflict comment's fast-forward step reads `git merge --ff-only origin/`, which assert_conflict_comment_merges picks up with its `^git merge` grep, so the extracted commands never match the expected conflict merges. Skip the --ff-only line when extracting. Also trim the new comments in the fan-out push/retarget/delete sequence. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JHvKryT4QUpHYdNq9YEQxX --- tests/test_e2e.sh | 2 +- update-pr-stack.sh | 14 +++++--------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/tests/test_e2e.sh b/tests/test_e2e.sh index 1da4439..420f2cd 100755 --- a/tests/test_e2e.sh +++ b/tests/test_e2e.sh @@ -291,7 +291,7 @@ assert_conflict_comment_merges() { expected+="git merge $conflict"$'\n' done expected=${expected%$'\n'} - actual=$(echo "$comment" | grep -E '^git merge' | sed 's/ *#.*//' || true) + actual=$(echo "$comment" | grep -E '^git merge' | grep -v -- '--ff-only' | sed 's/ *#.*//' || true) if [[ "$actual" == "$expected" ]]; then echo >&2 "✅ Verification Passed: conflict comment lists expected merge command(s)." diff --git a/update-pr-stack.sh b/update-pr-stack.sh index 0669f82..aa08cc7 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -311,12 +311,9 @@ main() { fi done - # Push the updated heads before retargeting, for the same reasons as the - # conflict-resume path: the head already contains TARGET_BRANCH when the - # base flips to it, keeping the PR mergeable (GitHub suppresses CI on a PR - # that conflicts with its base), and a failed push leaves the PR untouched - # on its old base instead of retargeted onto a stale head with nothing to - # re-trigger this action. + # Push the heads before retargeting: a failed push then leaves each PR + # intact on its old base, and the head already contains TARGET_BRANCH when + # the base flips to it. if [[ "${#UPDATED_TARGETS[@]}" -gt 0 ]]; then log_cmd git push origin "${UPDATED_TARGETS[@]}" fi @@ -325,9 +322,8 @@ main() { log_cmd gh pr edit "$BRANCH" --base "$TARGET_BRANCH" done - # Delete the merged branch last: deleting a PR's base branch closes the PR, - # so every child must be retargeted off it first. Keep it while conflicted - # PRs remain so it can be referenced during manual resolution. + # Deleting a PR's base branch closes the PR, so this must come after the + # retargets. Keep the branch for reference while conflicted PRs remain. if [[ "${#CONFLICTED_TARGETS[@]}" -eq 0 ]]; then log_cmd git push origin ":$MERGED_BRANCH" else From 972442ab373a1e3c9792ac788394bcae2b135482 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 21:31:02 +0000 Subject: [PATCH 03/12] Drop the e2e assertion fix, split out into its own PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fix for the --ff-only line breaking assert_conflict_comment_merges moved to a separate PR; the e2e job here stays red until that lands and main is merged back in. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JHvKryT4QUpHYdNq9YEQxX --- tests/test_e2e.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_e2e.sh b/tests/test_e2e.sh index 420f2cd..1da4439 100755 --- a/tests/test_e2e.sh +++ b/tests/test_e2e.sh @@ -291,7 +291,7 @@ assert_conflict_comment_merges() { expected+="git merge $conflict"$'\n' done expected=${expected%$'\n'} - actual=$(echo "$comment" | grep -E '^git merge' | grep -v -- '--ff-only' | sed 's/ *#.*//' || true) + actual=$(echo "$comment" | grep -E '^git merge' | sed 's/ *#.*//' || true) if [[ "$actual" == "$expected" ]]; then echo >&2 "✅ Verification Passed: conflict comment lists expected merge command(s)." From 735200350adeeee8c091718fbfec74b83d1b089b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 21:40:23 +0000 Subject: [PATCH 04/12] Address PRs by number instead of head branch name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A head branch can carry several PRs (one per base), so gh calls keyed by branch name can comment, label, or retarget the wrong one. Every gh call that acts on a specific PR now uses the PR number: the fan-out carries number/branch pairs from gh pr list, and the conflict-resolved run gets PR_NUMBER from the event payload via action.yml. The payload also already carries the PR's base branch, so the resume takes it from a new PR_BASE variable instead of querying the API; the resume test's gh mock no longer answers baseRefName queries, so a reintroduced lookup fails loudly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JHvKryT4QUpHYdNq9YEQxX --- action.yml | 2 + tests/mock_gh.sh | 4 +- tests/test_conflict_resolution_resume.sh | 18 ++--- tests/test_update_pr_stack.sh | 2 +- update-pr-stack.sh | 85 ++++++++++++++---------- 5 files changed, 63 insertions(+), 48 deletions(-) diff --git a/action.yml b/action.yml index 4eb9363..eb69e0e 100644 --- a/action.yml +++ b/action.yml @@ -54,6 +54,8 @@ runs: MERGED_BRANCH: ${{ github.event.pull_request.head.ref }} TARGET_BRANCH: ${{ github.event.pull_request.base.ref }} PR_BRANCH: ${{ github.event.pull_request.head.ref }} + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_BASE: ${{ github.event.pull_request.base.ref }} run: | echo "Running in $ACTION_MODE mode" ${{ github.action_path }}/update-pr-stack.sh diff --git a/tests/mock_gh.sh b/tests/mock_gh.sh index 74bbfd3..5a0b1ab 100755 --- a/tests/mock_gh.sh +++ b/tests/mock_gh.sh @@ -14,8 +14,8 @@ if [[ "$1" == "pr" && "$2" == "list" ]]; then done if [[ "$base" == "feature1" ]]; then - # feature2 is a direct child of feature1 - echo 'feature2' + # feature2 is a direct child of feature1 (PR #2) + echo '2 feature2' else # No other bases have direct children in our test scenario : diff --git a/tests/test_conflict_resolution_resume.sh b/tests/test_conflict_resolution_resume.sh index 3f0956a..205b101 100644 --- a/tests/test_conflict_resolution_resume.sh +++ b/tests/test_conflict_resolution_resume.sh @@ -21,8 +21,9 @@ ok() { echo "✅ $1"; PASS=$((PASS+1)); } # Build a configurable gh mock in a temp dir. It records every invocation to # $CALLS and is driven by env vars set per scenario: # MOCK_LABELS newline-separated labels returned by `pr view --json labels` -# MOCK_BASE base branch returned by `pr view --json baseRefName` # MOCK_COMMENTS_FILE file whose contents are returned by `pr view --json comments` +# The PR's base branch is not mocked: the script must take it from PR_BASE +# (event payload), so a baseRefName query is an unhandled call and fails. make_mock_gh() { local dir="$1" cat > "$dir/mock_gh.sh" <<'EOF' @@ -32,7 +33,6 @@ echo "gh $*" >> "$CALLS" if [[ "$1 $2" == "pr view" ]]; then case "$*" in *--json\ labels*) printf '%s\n' "${MOCK_LABELS:-}";; - *--json\ baseRefName*) printf '%s\n' "${MOCK_BASE:-}";; *--json\ comments*) cat "${MOCK_COMMENTS_FILE:-/dev/null}";; *) echo "unhandled pr view: $*" >&2; exit 1;; esac @@ -88,9 +88,9 @@ setup_repo() { } run_resume() { - env ACTION_MODE=conflict-resolved PR_BRANCH=child \ + env ACTION_MODE=conflict-resolved PR_BRANCH=child PR_NUMBER=5 PR_BASE="$PR_BASE" \ GH="$MOCK_DIR/mock_gh.sh" GIT="$MOCK_DIR/mock_git.sh" \ - MOCK_LABELS="$MOCK_LABELS" MOCK_BASE="$MOCK_BASE" \ + MOCK_LABELS="$MOCK_LABELS" \ MOCK_COMMENTS_FILE="$MOCK_COMMENTS_FILE" CALLS="$CALLS" \ bash "$ROOT_DIR/update-pr-stack.sh" >"$WORK/out.log" 2>&1 || echo "EXIT=$?" >>"$WORK/out.log" } @@ -103,7 +103,7 @@ marker() { # base target squash echo "### Scenario A: user manually retargeted the base -> no mutation" setup_repo MOCK_LABELS="autorestack-needs-conflict-resolution" -MOCK_BASE="spark" # human changed it; marker says parent +PR_BASE="spark" # human changed it; marker says parent MOCK_COMMENTS_FILE="$WORK/comments.txt" { echo "### conflict"; echo; marker parent main "$SQUASH"; } > "$MOCK_COMMENTS_FILE" run_resume @@ -119,7 +119,7 @@ ok "A: manual retarget detected, no branch mutation, label removed" echo "### Scenario B: no state marker -> no mutation" setup_repo MOCK_LABELS="autorestack-needs-conflict-resolution" -MOCK_BASE="parent" +PR_BASE="parent" MOCK_COMMENTS_FILE="$WORK/comments.txt" { echo "### some old conflict comment with no marker"; } > "$MOCK_COMMENTS_FILE" run_resume @@ -137,13 +137,13 @@ setup_repo git -C "$WORK" merge -q --no-edit main git -C "$WORK" push -q origin child MOCK_LABELS="autorestack-needs-conflict-resolution" -MOCK_BASE="parent" # matches marker -> not a manual retarget +PR_BASE="parent" # matches marker -> not a manual retarget MOCK_COMMENTS_FILE="$WORK/comments.txt" { echo "### conflict"; echo; marker parent main "$SQUASH"; } > "$MOCK_COMMENTS_FILE" run_resume grep -q -- "git push origin child" "$CALLS" || fail "C: child not pushed" -grep -q -- "pr edit child --base main" "$CALLS" || fail "C: base not retargeted to main" +grep -q -- "pr edit 5 --base main" "$CALLS" || fail "C: base not retargeted to main" grep -q "remove-label autorestack-needs-conflict-resolution" "$CALLS" || fail "C: label not removed" push_line=$(grep -n -- "git push origin child" "$CALLS" | head -1 | cut -d: -f1) base_line=$(grep -n -- "--base main" "$CALLS" | head -1 | cut -d: -f1) @@ -156,7 +156,7 @@ ok "C: resume pushes, retargets base, then removes label" echo "### Scenario D: recorded target branch is gone -> give up cleanly" setup_repo MOCK_LABELS="autorestack-needs-conflict-resolution" -MOCK_BASE="parent" # matches marker -> not a manual retarget +PR_BASE="parent" # matches marker -> not a manual retarget MOCK_COMMENTS_FILE="$WORK/comments.txt" { echo "### conflict"; echo; marker parent ghost-target "$SQUASH"; } > "$MOCK_COMMENTS_FILE" run_resume diff --git a/tests/test_update_pr_stack.sh b/tests/test_update_pr_stack.sh index 2930aac..6c18b28 100755 --- a/tests/test_update_pr_stack.sh +++ b/tests/test_update_pr_stack.sh @@ -89,7 +89,7 @@ run_update_pr_stack # the PR untouched on its old base), and the merged branch deleted only after # the retarget (deleting a PR's base branch closes the PR). push_line=$(grep -n "git push origin feature2" "$RUN_LOG" | head -1 | cut -d: -f1 || true) -edit_line=$(grep -n "pr edit feature2 --base main" "$RUN_LOG" | head -1 | cut -d: -f1 || true) +edit_line=$(grep -n "pr edit 2 --base main" "$RUN_LOG" | head -1 | cut -d: -f1 || true) delete_line=$(grep -n "git push origin :feature1" "$RUN_LOG" | head -1 | cut -d: -f1 || true) if [[ -n "$push_line" && -n "$edit_line" && -n "$delete_line" \ && "$push_line" -lt "$edit_line" && "$edit_line" -lt "$delete_line" ]]; then diff --git a/update-pr-stack.sh b/update-pr-stack.sh index aa08cc7..eb7487b 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -2,11 +2,16 @@ # # Updates PR stack after merging a PR # -# Required environment variables: +# Required environment variables (squash-merge mode): # SQUASH_COMMIT - The hash of the squash commit that was merged # MERGED_BRANCH - The name of the branch that was merged and will be deleted # TARGET_BRANCH - The name of the branch that the PR was merged into # +# Required environment variables (conflict-resolved mode): +# PR_BRANCH - The head branch of the PR being resumed +# PR_NUMBER - Its PR number, from the event payload +# PR_BASE - Its base branch, from the event payload +# # Design note: # This script aims to output a transcript of "plain" git/gh commands that a # human could follow through manually. For this reason: @@ -36,8 +41,8 @@ format_state_marker() { # Echoes the most recent state-marker line found in our PR comments, or nothing. read_state_marker() { - local PR_BRANCH="$1" - gh pr view "$PR_BRANCH" --json comments --jq '.comments[].body' 2>/dev/null \ + local PR_NUMBER="$1" + gh pr view "$PR_NUMBER" --json comments --jq '.comments[].body' 2>/dev/null \ | { grep -F "$STATE_MARKER_PREFIX" || true; } | tail -n1 } @@ -74,9 +79,12 @@ has_squash_commit() { && git merge-base --is-ancestor SQUASH_COMMIT "$BRANCH" } +# Args: head branch, base branch, PR number. git commands use the branch; gh +# commands use the number, since a head branch can carry several PRs. update_direct_target() { local BRANCH="$1" local BASE_BRANCH="$2" + local PR_NUMBER="$3" # Checkout first to ensure the local branch exists (created from origin if # needed). This allows has_squash_commit to compare local refs, which matters @@ -144,10 +152,10 @@ update_direct_target() { echo "Once you push, this action will resume and finish updating this pull request." echo format_state_marker "$MERGED_BRANCH" "$TARGET_BRANCH" "$(git rev-parse SQUASH_COMMIT)" - } | log_cmd gh pr comment "$BRANCH" -F - + } | log_cmd gh pr comment "$PR_NUMBER" -F - # Create the label if it doesn't exist, then add it to the PR gh label create "$CONFLICT_LABEL" --description "PR needs manual conflict resolution" --color "d73a4a" 2>/dev/null || true - log_cmd gh pr edit "$BRANCH" --add-label "$CONFLICT_LABEL" + log_cmd gh pr edit "$PR_NUMBER" --add-label "$CONFLICT_LABEL" return 1 else log_cmd git merge --no-edit -s ours SQUASH_COMMIT @@ -167,9 +175,9 @@ See $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" # Check if a PR has the conflict resolution label pr_has_conflict_label() { - local BRANCH="$1" + local PR_NUMBER="$1" local LABELS - LABELS=$(gh pr view "$BRANCH" --json labels --jq '.labels[].name' 2>/dev/null || echo "") + LABELS=$(gh pr view "$PR_NUMBER" --json labels --jq '.labels[].name' 2>/dev/null || echo "") echo "$LABELS" | grep -q "^${CONFLICT_LABEL}$" } @@ -196,20 +204,22 @@ has_sibling_conflicts() { # the conflict label so this action stops re-triggering. Used for the dead-end # cases where we cannot or must not finish automatically. abandon_resume() { - local PR_BRANCH="$1" + local PR_NUMBER="$1" local MESSAGE="$2" - echo "$MESSAGE" | log_cmd gh pr comment "$PR_BRANCH" -F - - log_cmd gh pr edit "$PR_BRANCH" --remove-label "$CONFLICT_LABEL" + echo "$MESSAGE" | log_cmd gh pr comment "$PR_NUMBER" -F - + log_cmd gh pr edit "$PR_NUMBER" --remove-label "$CONFLICT_LABEL" } # Continue processing after user manually resolved conflicts continue_after_resolution() { check_env_var "PR_BRANCH" + check_env_var "PR_NUMBER" + check_env_var "PR_BASE" - echo "Checking if $PR_BRANCH needs continuation after conflict resolution..." + echo "Checking if PR #$PR_NUMBER ($PR_BRANCH) needs continuation after conflict resolution..." # Check if the PR has the conflict label - if ! pr_has_conflict_label "$PR_BRANCH"; then + if ! pr_has_conflict_label "$PR_NUMBER"; then echo "✓ $PR_BRANCH does not have conflict label; nothing to do" return fi @@ -221,10 +231,10 @@ continue_after_resolution() { # Recover them from the marker the squash-merge run left in the conflict # comment. local MARKER - MARKER=$(read_state_marker "$PR_BRANCH") + MARKER=$(read_state_marker "$PR_NUMBER") if [[ -z "$MARKER" ]]; then echo "⚠️ No autorestack state marker on $PR_BRANCH; cannot resume safely. Removing the label." - abandon_resume "$PR_BRANCH" "ℹ️ autorestack could not find its state marker on this PR, so it will not update the stack automatically. If this PR still needs its base updated, update its base manually." + abandon_resume "$PR_NUMBER" "ℹ️ autorestack could not find its state marker on this PR, so it will not update the stack automatically. If this PR still needs its base updated, update its base manually." return fi @@ -232,17 +242,12 @@ continue_after_resolution() { read -r OLD_BASE NEW_TARGET SQUASH_HASH < <(parse_state_marker "$MARKER") echo "Recorded state: base=$OLD_BASE target=$NEW_TARGET squash=$SQUASH_HASH" - # The base we left the PR on while waiting for conflict resolution was the - # merged parent branch. If it no longer matches, a human retargeted the PR - # (e.g. straight onto the integration branch); we are no longer the authority - # on its base, so we step back without touching the branch. This runs before - # any mutation: once the base diverges, the recorded target is stale and a - # merge built against it would be wrong. - local CURRENT_BASE - CURRENT_BASE=$(gh pr view "$PR_BRANCH" --json baseRefName --jq '.baseRefName') - if [[ "$CURRENT_BASE" != "$OLD_BASE" ]]; then - echo "⚠️ Base of $PR_BRANCH changed manually ($OLD_BASE -> $CURRENT_BASE); not updating the stack." - abandon_resume "$PR_BRANCH" "ℹ️ The base branch of this PR was changed manually, so autorestack stepped back and will not update it automatically." + # The PR was left based on the merged parent branch. If the payload shows a + # different base, a human retargeted the PR; the recorded target is stale, + # so step back before any mutation. + if [[ "$PR_BASE" != "$OLD_BASE" ]]; then + echo "⚠️ Base of $PR_BRANCH changed manually ($OLD_BASE -> $PR_BASE); not updating the stack." + abandon_resume "$PR_NUMBER" "ℹ️ The base branch of this PR was changed manually, so autorestack stepped back and will not update it automatically." return fi @@ -252,7 +257,7 @@ continue_after_resolution() { # can succeed, so give up cleanly rather than stranding the PR under the label. if ! git rev-parse --verify --quiet "origin/$NEW_TARGET" >/dev/null; then echo "⚠️ Recorded target branch '$NEW_TARGET' no longer exists; abandoning resume of $PR_BRANCH." - abandon_resume "$PR_BRANCH" "ℹ️ The branch this PR was being retargeted onto (\`$NEW_TARGET\`) no longer exists, so autorestack stepped back. If this PR still needs its base updated, update its base manually." + abandon_resume "$PR_NUMBER" "ℹ️ The branch this PR was being retargeted onto (\`$NEW_TARGET\`) no longer exists, so autorestack stepped back. If this PR still needs its base updated, update its base manually." return fi @@ -265,7 +270,7 @@ continue_after_resolution() { log_cmd git update-ref SQUASH_COMMIT "$SQUASH_HASH" MERGED_BRANCH="$OLD_BASE" TARGET_BRANCH="$NEW_TARGET" - if ! update_direct_target "$PR_BRANCH" "$NEW_TARGET"; then + if ! update_direct_target "$PR_BRANCH" "$NEW_TARGET" "$PR_NUMBER"; then echo "⚠️ '$PR_BRANCH' still conflicts; re-posted the conflict comment, will retry on next push" return 1 fi @@ -276,8 +281,8 @@ continue_after_resolution() { # NEW_TARGET when the base flips to it, keeping the PR mergeable (GitHub # suppresses CI on a PR that conflicts with its base). log_cmd git push origin "$PR_BRANCH" - log_cmd gh pr edit "$PR_BRANCH" --base "$NEW_TARGET" - log_cmd gh pr edit "$PR_BRANCH" --remove-label "$CONFLICT_LABEL" + log_cmd gh pr edit "$PR_NUMBER" --base "$NEW_TARGET" + log_cmd gh pr edit "$PR_NUMBER" --remove-label "$CONFLICT_LABEL" # Check if old base branch should be deleted if has_sibling_conflicts "$OLD_BASE" "$PR_BRANCH"; then @@ -297,17 +302,25 @@ main() { log_cmd git update-ref SQUASH_COMMIT "$SQUASH_COMMIT" # Find all PRs directly targeting the merged PR's head - INITIAL_TARGETS=($(log_cmd gh pr list --base "$MERGED_BRANCH" --json headRefName --jq '.[].headRefName')) + INITIAL_NUMBERS=() + INITIAL_TARGETS=() + while read -r NUMBER BRANCH; do + [[ -n "$BRANCH" ]] || continue + INITIAL_NUMBERS+=("$NUMBER") + INITIAL_TARGETS+=("$BRANCH") + done < <(log_cmd gh pr list --base "$MERGED_BRANCH" --json number,headRefName --jq '.[] | "\(.number) \(.headRefName)"') # Track successfully updated vs conflicted branches separately UPDATED_TARGETS=() + UPDATED_NUMBERS=() CONFLICTED_TARGETS=() - for BRANCH in "${INITIAL_TARGETS[@]}"; do - if update_direct_target "$BRANCH" "$TARGET_BRANCH"; then - UPDATED_TARGETS+=("$BRANCH") + for i in "${!INITIAL_TARGETS[@]}"; do + if update_direct_target "${INITIAL_TARGETS[$i]}" "$TARGET_BRANCH" "${INITIAL_NUMBERS[$i]}"; then + UPDATED_TARGETS+=("${INITIAL_TARGETS[$i]}") + UPDATED_NUMBERS+=("${INITIAL_NUMBERS[$i]}") else - CONFLICTED_TARGETS+=("$BRANCH") + CONFLICTED_TARGETS+=("${INITIAL_TARGETS[$i]}") fi done @@ -318,8 +331,8 @@ main() { log_cmd git push origin "${UPDATED_TARGETS[@]}" fi - for BRANCH in "${UPDATED_TARGETS[@]}"; do - log_cmd gh pr edit "$BRANCH" --base "$TARGET_BRANCH" + for NUMBER in "${UPDATED_NUMBERS[@]}"; do + log_cmd gh pr edit "$NUMBER" --base "$TARGET_BRANCH" done # Deleting a PR's base branch closes the PR, so this must come after the From 49cfcc7e5677f797f1f6c1d021bbe4759fa6db1d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 22:03:43 +0000 Subject: [PATCH 05/12] Abort the resume when label or comment reads fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pr_has_conflict_label swallowed gh failures as "no label", so a transient API error made the resume silently skip a labeled PR, and a failed comments fetch in read_state_marker read as "no marker", which made the caller abandon the resume and drop the conflict label for good. Both now abort the run instead; the label stays on, so the next push retries. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JHvKryT4QUpHYdNq9YEQxX --- update-pr-stack.sh | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/update-pr-stack.sh b/update-pr-stack.sh index eb7487b..504580b 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -40,10 +40,16 @@ format_state_marker() { } # Echoes the most recent state-marker line found in our PR comments, or nothing. +# A failed comments fetch aborts the run: treating it as "no marker" would make +# the caller abandon the resume and drop the conflict label for good. read_state_marker() { local PR_NUMBER="$1" - gh pr view "$PR_NUMBER" --json comments --jq '.comments[].body' 2>/dev/null \ - | { grep -F "$STATE_MARKER_PREFIX" || true; } | tail -n1 + local BODIES + if ! BODIES=$(gh pr view "$PR_NUMBER" --json comments --jq '.comments[].body'); then + echo "Error: could not read comments of PR #$PR_NUMBER" >&2 + exit 1 + fi + { grep -F "$STATE_MARKER_PREFIX" <<<"$BODIES" || true; } | tail -n1 } # Args: a marker line. Echoes "base target squash". @@ -173,11 +179,15 @@ See $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" return 0 } -# Check if a PR has the conflict resolution label +# Check if a PR has the conflict resolution label. A failed labels fetch aborts +# the run rather than reading as "no label", which would silently skip a resume. pr_has_conflict_label() { local PR_NUMBER="$1" local LABELS - LABELS=$(gh pr view "$PR_NUMBER" --json labels --jq '.labels[].name' 2>/dev/null || echo "") + if ! LABELS=$(gh pr view "$PR_NUMBER" --json labels --jq '.labels[].name'); then + echo "Error: could not read labels of PR #$PR_NUMBER" >&2 + exit 1 + fi echo "$LABELS" | grep -q "^${CONFLICT_LABEL}$" } From 7ac9aea797cdebc273aeb242f2f21e809d904c63 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 22:05:08 +0000 Subject: [PATCH 06/12] Trust only our own comments for the state marker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit read_state_marker accepted a marker from any comment, so anyone able to comment on a public repo could plant one; if its base matched, the resume would merge an attacker-chosen commit (fork-pushed objects are reachable by hash) into the branch and push it with the action's token. A quote-reply of an old conflict comment could likewise resurrect a stale marker, since HTML comments survive quoting and the newest marker wins. Filter the comments to viewerDidAuthor, i.e. those posted with the same token the action runs under. Also reject markers with missing fields instead of passing empty values to git: a marker missing squash= used to crash the run on update-ref and strand the PR under the label. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JHvKryT4QUpHYdNq9YEQxX --- tests/test_conflict_resolution_resume.sh | 21 ++++++++++++++++++++- update-pr-stack.sh | 18 ++++++++++++++---- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/tests/test_conflict_resolution_resume.sh b/tests/test_conflict_resolution_resume.sh index 205b101..b7c1c88 100644 --- a/tests/test_conflict_resolution_resume.sh +++ b/tests/test_conflict_resolution_resume.sh @@ -33,7 +33,11 @@ echo "gh $*" >> "$CALLS" if [[ "$1 $2" == "pr view" ]]; then case "$*" in *--json\ labels*) printf '%s\n' "${MOCK_LABELS:-}";; - *--json\ comments*) cat "${MOCK_COMMENTS_FILE:-/dev/null}";; + *--json\ comments*) + # The comments file stands for our own comments only, so the query + # must restrict itself to those. + [[ "$*" == *viewerDidAuthor* ]] || { echo "comments query must filter by viewerDidAuthor" >&2; exit 1; } + cat "${MOCK_COMMENTS_FILE:-/dev/null}";; *) echo "unhandled pr view: $*" >&2; exit 1;; esac elif [[ "$1 $2" == "pr comment" ]]; then @@ -167,5 +171,20 @@ grep -q -- "--base" "$CALLS" && fail "D: base must NOT be edited" [[ "$(git -C "$ORIGIN" rev-parse child)" == "$CHILD_BEFORE" ]] || fail "D: child was pushed" ok "D: missing target detected, no branch mutation, label removed" +# --------------------------------------------------------------------------- +echo "### Scenario E: malformed state marker -> no mutation" +setup_repo +MOCK_LABELS="autorestack-needs-conflict-resolution" +PR_BASE="parent" +MOCK_COMMENTS_FILE="$WORK/comments.txt" +{ echo "### conflict"; echo; echo ''; } > "$MOCK_COMMENTS_FILE" +run_resume + +grep -q "remove-label autorestack-needs-conflict-resolution" "$CALLS" || fail "E: label not removed" +grep -q "gh pr comment" "$CALLS" || fail "E: no explanatory comment posted" +grep -q -- "--base" "$CALLS" && fail "E: base must NOT be edited" +[[ "$(git -C "$ORIGIN" rev-parse child)" == "$CHILD_BEFORE" ]] || fail "E: child was pushed" +ok "E: malformed marker handled, no branch mutation, label removed" + echo echo "All conflict-resume tests passed 🎉 ($PASS scenarios)" diff --git a/update-pr-stack.sh b/update-pr-stack.sh index 504580b..2596fd8 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -39,13 +39,17 @@ format_state_marker() { "$STATE_MARKER_PREFIX" "$1" "$2" "$3" } -# Echoes the most recent state-marker line found in our PR comments, or nothing. -# A failed comments fetch aborts the run: treating it as "no marker" would make -# the caller abandon the resume and drop the conflict label for good. +# Echoes the most recent state-marker line found in PR comments, or nothing. +# Only comments posted with our own token count (viewerDidAuthor): anyone can +# comment a marker, and acting on a forged one would merge and push an +# attacker-chosen commit. A failed comments fetch aborts the run: treating it +# as "no marker" would make the caller abandon the resume and drop the +# conflict label for good. read_state_marker() { local PR_NUMBER="$1" local BODIES - if ! BODIES=$(gh pr view "$PR_NUMBER" --json comments --jq '.comments[].body'); then + if ! BODIES=$(gh pr view "$PR_NUMBER" --json comments \ + --jq '.comments[] | select(.viewerDidAuthor) | .body'); then echo "Error: could not read comments of PR #$PR_NUMBER" >&2 exit 1 fi @@ -252,6 +256,12 @@ continue_after_resolution() { read -r OLD_BASE NEW_TARGET SQUASH_HASH < <(parse_state_marker "$MARKER") echo "Recorded state: base=$OLD_BASE target=$NEW_TARGET squash=$SQUASH_HASH" + if [[ -z "$OLD_BASE" || -z "$NEW_TARGET" || -z "$SQUASH_HASH" ]]; then + echo "⚠️ State marker on $PR_BRANCH is malformed; cannot resume safely. Removing the label." + abandon_resume "$PR_NUMBER" "ℹ️ autorestack found a malformed state marker on this PR, so it will not update the stack automatically. If this PR still needs its base updated, update its base manually." + return + fi + # The PR was left based on the merged parent branch. If the payload shows a # different base, a human retargeted the PR; the recorded target is stale, # so step back before any mutation. From 33f91fd06b3df5780bc2c199ae72e4cebd7342fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9=20Rubinstein?= Date: Wed, 10 Jun 2026 16:16:58 +0200 Subject: [PATCH 07/12] Reword the marker-trust comment "comments posted with our own token count" parsed as the noun phrase "token count". Also viewerDidAuthor matches the token's identity, not the token itself. Co-Authored-By: Claude Fable 5 --- update-pr-stack.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/update-pr-stack.sh b/update-pr-stack.sh index 2596fd8..9731fd0 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -40,9 +40,9 @@ format_state_marker() { } # Echoes the most recent state-marker line found in PR comments, or nothing. -# Only comments posted with our own token count (viewerDidAuthor): anyone can -# comment a marker, and acting on a forged one would merge and push an -# attacker-chosen commit. A failed comments fetch aborts the run: treating it +# Only comments posted under our own identity are trusted (viewerDidAuthor): +# anyone can comment a marker, and acting on a forged one would merge and push +# an attacker-chosen commit. A failed comments fetch aborts the run: treating it # as "no marker" would make the caller abandon the resume and drop the # conflict label for good. read_state_marker() { From bad55abd92efdf3be99ac182f8653c8cb5d32c2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9=20Rubinstein?= Date: Thu, 11 Jun 2026 13:48:48 +0200 Subject: [PATCH 08/12] Apply suggestion from @Phlogistique --- update-pr-stack.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/update-pr-stack.sh b/update-pr-stack.sh index 6d4d718..4f7749e 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -41,8 +41,6 @@ format_state_marker() { } # Echoes the most recent state-marker line found in our PR comments, or nothing. -# A failed comments fetch aborts the run: treating it as "no marker" would make -# the caller abandon the resume and drop the conflict label for good. read_state_marker() { local PR_NUMBER="$1" local BODIES From 6ab8295cf74cf37b51bcca00a6b0943a507f6fd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9=20Rubinstein?= Date: Thu, 11 Jun 2026 13:49:12 +0200 Subject: [PATCH 09/12] Apply suggestion from @Phlogistique --- update-pr-stack.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/update-pr-stack.sh b/update-pr-stack.sh index 4f7749e..dde781f 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -235,8 +235,7 @@ See $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" return 0 } -# Check if a PR has the conflict resolution label. A failed labels fetch aborts -# the run rather than reading as "no label", which would silently skip a resume. +# Check if a PR has the conflict resolution label. pr_has_conflict_label() { local PR_NUMBER="$1" local LABELS From 55e48aef9bc14517ca4142694d5a79b62b9545ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9=20Rubinstein?= Date: Thu, 11 Jun 2026 13:51:00 +0200 Subject: [PATCH 10/12] Apply suggestion from @Phlogistique --- update-pr-stack.sh | 5 ----- 1 file changed, 5 deletions(-) diff --git a/update-pr-stack.sh b/update-pr-stack.sh index 9731fd0..466fdbc 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -40,11 +40,6 @@ format_state_marker() { } # Echoes the most recent state-marker line found in PR comments, or nothing. -# Only comments posted under our own identity are trusted (viewerDidAuthor): -# anyone can comment a marker, and acting on a forged one would merge and push -# an attacker-chosen commit. A failed comments fetch aborts the run: treating it -# as "no marker" would make the caller abandon the resume and drop the -# conflict label for good. read_state_marker() { local PR_NUMBER="$1" local BODIES From f9030b401bf1f33324a370911197f95370d6ecea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9=20Rubinstein?= Date: Thu, 11 Jun 2026 13:52:30 +0200 Subject: [PATCH 11/12] Update update-pr-stack.sh --- update-pr-stack.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/update-pr-stack.sh b/update-pr-stack.sh index 466fdbc..e10edd6 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -39,7 +39,7 @@ format_state_marker() { "$STATE_MARKER_PREFIX" "$1" "$2" "$3" } -# Echoes the most recent state-marker line found in PR comments, or nothing. +# Echoes the most recent state-marker line found in our PR comments, or nothing. read_state_marker() { local PR_NUMBER="$1" local BODIES From 77ab69fcd0aa07067d55a65c57700f6a065734a7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 11:54:41 +0000 Subject: [PATCH 12/12] Die on a malformed state marker instead of commenting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A malformed marker is an internal error: just exit. The label stays on, so the run stays retryable and the failure is visible in the workflow instead of a PR comment. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JHvKryT4QUpHYdNq9YEQxX --- tests/test_conflict_resolution_resume.sh | 9 +++++---- update-pr-stack.sh | 5 ++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/test_conflict_resolution_resume.sh b/tests/test_conflict_resolution_resume.sh index b7c1c88..415083c 100644 --- a/tests/test_conflict_resolution_resume.sh +++ b/tests/test_conflict_resolution_resume.sh @@ -172,7 +172,7 @@ grep -q -- "--base" "$CALLS" && fail "D: base must NOT be edited" ok "D: missing target detected, no branch mutation, label removed" # --------------------------------------------------------------------------- -echo "### Scenario E: malformed state marker -> no mutation" +echo "### Scenario E: malformed state marker -> internal error, die without touching the PR" setup_repo MOCK_LABELS="autorestack-needs-conflict-resolution" PR_BASE="parent" @@ -180,11 +180,12 @@ MOCK_COMMENTS_FILE="$WORK/comments.txt" { echo "### conflict"; echo; echo ''; } > "$MOCK_COMMENTS_FILE" run_resume -grep -q "remove-label autorestack-needs-conflict-resolution" "$CALLS" || fail "E: label not removed" -grep -q "gh pr comment" "$CALLS" || fail "E: no explanatory comment posted" +grep -q "EXIT=1" "$WORK/out.log" || fail "E: run must die on a malformed marker" +grep -q "remove-label" "$CALLS" && fail "E: label must stay on" +grep -q "gh pr comment" "$CALLS" && fail "E: no PR comment for an internal error" grep -q -- "--base" "$CALLS" && fail "E: base must NOT be edited" [[ "$(git -C "$ORIGIN" rev-parse child)" == "$CHILD_BEFORE" ]] || fail "E: child was pushed" -ok "E: malformed marker handled, no branch mutation, label removed" +ok "E: malformed marker dies, PR untouched, label kept" echo echo "All conflict-resume tests passed 🎉 ($PASS scenarios)" diff --git a/update-pr-stack.sh b/update-pr-stack.sh index d527138..e3fec22 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -309,9 +309,8 @@ continue_after_resolution() { echo "Recorded state: base=$OLD_BASE target=$NEW_TARGET squash=$SQUASH_HASH" if [[ -z "$OLD_BASE" || -z "$NEW_TARGET" || -z "$SQUASH_HASH" ]]; then - echo "⚠️ State marker on $PR_BRANCH is malformed; cannot resume safely. Removing the label." - abandon_resume "$PR_NUMBER" "ℹ️ autorestack found a malformed state marker on this PR, so it will not update the stack automatically. If this PR still needs its base updated, update its base manually." - return + echo "Error: malformed state marker on $PR_BRANCH: $MARKER" >&2 + exit 1 fi # The PR was left based on the merged parent branch. If the payload shows a