Skip to content

Commit 0af0330

Browse files
Phlogistiqueclaudegithub-actions
authored
Address PRs by number instead of head branch name (#45)
A head branch can carry several PRs (one per base), so `gh` calls keyed by branch name can comment on, label, or retarget the wrong PR — the same ambiguous-lookup family as the bug fixed in #39. Every `gh` call that acts on a specific PR now uses the PR number: - The squash-merge fan-out carries number/branch pairs from `gh pr list` (numbers for `gh`, branches for `git`). - The conflict-resolved run gets `PR_NUMBER` from the event payload via `action.yml`. This is also an opportunity to drop an API call: the synchronize payload already carries the PR's base branch, so the resume's manual-retarget check now reads it from a new `PR_BASE` env var instead of `gh pr view --json baseRefName`. The resume test's gh mock no longer answers `baseRefName` queries, so a reintroduced lookup fails loudly. Stacked on #42 (the changes overlap in `main()`); this PR retargets onto `main` once #42 lands. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JHvKryT4QUpHYdNq9YEQxX --- _Generated by [Claude Code](https://claude.ai/code/session_01JHvKryT4QUpHYdNq9YEQxX)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: github-actions <github-actions@github.com>
1 parent 2d54909 commit 0af0330

5 files changed

Lines changed: 63 additions & 48 deletions

File tree

action.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ runs:
5454
MERGED_BRANCH: ${{ github.event.pull_request.head.ref }}
5555
TARGET_BRANCH: ${{ github.event.pull_request.base.ref }}
5656
PR_BRANCH: ${{ github.event.pull_request.head.ref }}
57+
PR_NUMBER: ${{ github.event.pull_request.number }}
58+
PR_BASE: ${{ github.event.pull_request.base.ref }}
5759
run: |
5860
echo "Running in $ACTION_MODE mode"
5961
${{ github.action_path }}/update-pr-stack.sh

tests/mock_gh.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ if [[ "$1" == "pr" && "$2" == "list" ]]; then
1414
done
1515

1616
if [[ "$base" == "feature1" ]]; then
17-
# feature2 is a direct child of feature1
18-
echo 'feature2'
17+
# feature2 is a direct child of feature1 (PR #2)
18+
echo '2 feature2'
1919
else
2020
# No other bases have direct children in our test scenario
2121
:

tests/test_conflict_resolution_resume.sh

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,9 @@ ok() { echo "✅ $1"; PASS=$((PASS+1)); }
2121
# Build a configurable gh mock in a temp dir. It records every invocation to
2222
# $CALLS and is driven by env vars set per scenario:
2323
# MOCK_LABELS newline-separated labels returned by `pr view --json labels`
24-
# MOCK_BASE base branch returned by `pr view --json baseRefName`
2524
# MOCK_COMMENTS_FILE file whose contents are returned by `pr view --json comments`
25+
# The PR's base branch is not mocked: the script must take it from PR_BASE
26+
# (event payload), so a baseRefName query is an unhandled call and fails.
2627
make_mock_gh() {
2728
local dir="$1"
2829
cat > "$dir/mock_gh.sh" <<'EOF'
@@ -32,7 +33,6 @@ echo "gh $*" >> "$CALLS"
3233
if [[ "$1 $2" == "pr view" ]]; then
3334
case "$*" in
3435
*--json\ labels*) printf '%s\n' "${MOCK_LABELS:-}";;
35-
*--json\ baseRefName*) printf '%s\n' "${MOCK_BASE:-}";;
3636
*--json\ comments*) cat "${MOCK_COMMENTS_FILE:-/dev/null}";;
3737
*) echo "unhandled pr view: $*" >&2; exit 1;;
3838
esac
@@ -88,9 +88,9 @@ setup_repo() {
8888
}
8989

9090
run_resume() {
91-
env ACTION_MODE=conflict-resolved PR_BRANCH=child \
91+
env ACTION_MODE=conflict-resolved PR_BRANCH=child PR_NUMBER=5 PR_BASE="$PR_BASE" \
9292
GH="$MOCK_DIR/mock_gh.sh" GIT="$MOCK_DIR/mock_git.sh" \
93-
MOCK_LABELS="$MOCK_LABELS" MOCK_BASE="$MOCK_BASE" \
93+
MOCK_LABELS="$MOCK_LABELS" \
9494
MOCK_COMMENTS_FILE="$MOCK_COMMENTS_FILE" CALLS="$CALLS" \
9595
bash "$ROOT_DIR/update-pr-stack.sh" >"$WORK/out.log" 2>&1 || echo "EXIT=$?" >>"$WORK/out.log"
9696
}
@@ -103,7 +103,7 @@ marker() { # base target squash
103103
echo "### Scenario A: user manually retargeted the base -> no mutation"
104104
setup_repo
105105
MOCK_LABELS="autorestack-needs-conflict-resolution"
106-
MOCK_BASE="spark" # human changed it; marker says parent
106+
PR_BASE="spark" # human changed it; marker says parent
107107
MOCK_COMMENTS_FILE="$WORK/comments.txt"
108108
{ echo "### conflict"; echo; marker parent main "$SQUASH"; } > "$MOCK_COMMENTS_FILE"
109109
run_resume
@@ -119,7 +119,7 @@ ok "A: manual retarget detected, no branch mutation, label removed"
119119
echo "### Scenario B: no state marker -> no mutation"
120120
setup_repo
121121
MOCK_LABELS="autorestack-needs-conflict-resolution"
122-
MOCK_BASE="parent"
122+
PR_BASE="parent"
123123
MOCK_COMMENTS_FILE="$WORK/comments.txt"
124124
{ echo "### some old conflict comment with no marker"; } > "$MOCK_COMMENTS_FILE"
125125
run_resume
@@ -137,13 +137,13 @@ setup_repo
137137
git -C "$WORK" merge -q --no-edit main
138138
git -C "$WORK" push -q origin child
139139
MOCK_LABELS="autorestack-needs-conflict-resolution"
140-
MOCK_BASE="parent" # matches marker -> not a manual retarget
140+
PR_BASE="parent" # matches marker -> not a manual retarget
141141
MOCK_COMMENTS_FILE="$WORK/comments.txt"
142142
{ echo "### conflict"; echo; marker parent main "$SQUASH"; } > "$MOCK_COMMENTS_FILE"
143143
run_resume
144144

145145
grep -q -- "git push origin child" "$CALLS" || fail "C: child not pushed"
146-
grep -q -- "pr edit child --base main" "$CALLS" || fail "C: base not retargeted to main"
146+
grep -q -- "pr edit 5 --base main" "$CALLS" || fail "C: base not retargeted to main"
147147
grep -q "remove-label autorestack-needs-conflict-resolution" "$CALLS" || fail "C: label not removed"
148148
push_line=$(grep -n -- "git push origin child" "$CALLS" | head -1 | cut -d: -f1)
149149
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"
156156
echo "### Scenario D: recorded target branch is gone -> give up cleanly"
157157
setup_repo
158158
MOCK_LABELS="autorestack-needs-conflict-resolution"
159-
MOCK_BASE="parent" # matches marker -> not a manual retarget
159+
PR_BASE="parent" # matches marker -> not a manual retarget
160160
MOCK_COMMENTS_FILE="$WORK/comments.txt"
161161
{ echo "### conflict"; echo; marker parent ghost-target "$SQUASH"; } > "$MOCK_COMMENTS_FILE"
162162
run_resume

tests/test_update_pr_stack.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ run_update_pr_stack
8989
# the PR untouched on its old base), and the merged branch deleted only after
9090
# the retarget (deleting a PR's base branch closes the PR).
9191
push_line=$(grep -n "git push origin feature2" "$RUN_LOG" | head -1 | cut -d: -f1 || true)
92-
edit_line=$(grep -n "pr edit feature2 --base main" "$RUN_LOG" | head -1 | cut -d: -f1 || true)
92+
edit_line=$(grep -n "pr edit 2 --base main" "$RUN_LOG" | head -1 | cut -d: -f1 || true)
9393
delete_line=$(grep -n "git push origin :feature1" "$RUN_LOG" | head -1 | cut -d: -f1 || true)
9494
if [[ -n "$push_line" && -n "$edit_line" && -n "$delete_line" \
9595
&& "$push_line" -lt "$edit_line" && "$edit_line" -lt "$delete_line" ]]; then

update-pr-stack.sh

Lines changed: 49 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,16 @@
22
#
33
# Updates PR stack after merging a PR
44
#
5-
# Required environment variables:
5+
# Required environment variables (squash-merge mode):
66
# SQUASH_COMMIT - The hash of the squash commit that was merged
77
# MERGED_BRANCH - The name of the branch that was merged and will be deleted
88
# TARGET_BRANCH - The name of the branch that the PR was merged into
99
#
10+
# Required environment variables (conflict-resolved mode):
11+
# PR_BRANCH - The head branch of the PR being resumed
12+
# PR_NUMBER - Its PR number, from the event payload
13+
# PR_BASE - Its base branch, from the event payload
14+
#
1015
# Design note:
1116
# This script aims to output a transcript of "plain" git/gh commands that a
1217
# human could follow through manually. For this reason:
@@ -36,8 +41,8 @@ format_state_marker() {
3641

3742
# Echoes the most recent state-marker line found in our PR comments, or nothing.
3843
read_state_marker() {
39-
local PR_BRANCH="$1"
40-
gh pr view "$PR_BRANCH" --json comments --jq '.comments[].body' 2>/dev/null \
44+
local PR_NUMBER="$1"
45+
gh pr view "$PR_NUMBER" --json comments --jq '.comments[].body' 2>/dev/null \
4146
| { grep -F "$STATE_MARKER_PREFIX" || true; } | tail -n1
4247
}
4348

@@ -74,9 +79,12 @@ has_squash_commit() {
7479
&& git merge-base --is-ancestor SQUASH_COMMIT "$BRANCH"
7580
}
7681

82+
# Args: head branch, base branch, PR number. git commands use the branch; gh
83+
# commands use the number, since a head branch can carry several PRs.
7784
update_direct_target() {
7885
local BRANCH="$1"
7986
local BASE_BRANCH="$2"
87+
local PR_NUMBER="$3"
8088

8189
# Checkout first to ensure the local branch exists (created from origin if
8290
# needed). This allows has_squash_commit to compare local refs, which matters
@@ -144,10 +152,10 @@ update_direct_target() {
144152
echo "Once you push, this action will resume and finish updating this pull request."
145153
echo
146154
format_state_marker "$MERGED_BRANCH" "$TARGET_BRANCH" "$(git rev-parse SQUASH_COMMIT)"
147-
} | log_cmd gh pr comment "$BRANCH" -F -
155+
} | log_cmd gh pr comment "$PR_NUMBER" -F -
148156
# Create the label if it doesn't exist, then add it to the PR
149157
gh label create "$CONFLICT_LABEL" --description "PR needs manual conflict resolution" --color "d73a4a" 2>/dev/null || true
150-
log_cmd gh pr edit "$BRANCH" --add-label "$CONFLICT_LABEL"
158+
log_cmd gh pr edit "$PR_NUMBER" --add-label "$CONFLICT_LABEL"
151159
return 1
152160
else
153161
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"
167175

168176
# Check if a PR has the conflict resolution label
169177
pr_has_conflict_label() {
170-
local BRANCH="$1"
178+
local PR_NUMBER="$1"
171179
local LABELS
172-
LABELS=$(gh pr view "$BRANCH" --json labels --jq '.labels[].name' 2>/dev/null || echo "")
180+
LABELS=$(gh pr view "$PR_NUMBER" --json labels --jq '.labels[].name' 2>/dev/null || echo "")
173181
echo "$LABELS" | grep -q "^${CONFLICT_LABEL}$"
174182
}
175183

@@ -196,20 +204,22 @@ has_sibling_conflicts() {
196204
# the conflict label so this action stops re-triggering. Used for the dead-end
197205
# cases where we cannot or must not finish automatically.
198206
abandon_resume() {
199-
local PR_BRANCH="$1"
207+
local PR_NUMBER="$1"
200208
local MESSAGE="$2"
201-
echo "$MESSAGE" | log_cmd gh pr comment "$PR_BRANCH" -F -
202-
log_cmd gh pr edit "$PR_BRANCH" --remove-label "$CONFLICT_LABEL"
209+
echo "$MESSAGE" | log_cmd gh pr comment "$PR_NUMBER" -F -
210+
log_cmd gh pr edit "$PR_NUMBER" --remove-label "$CONFLICT_LABEL"
203211
}
204212

205213
# Continue processing after user manually resolved conflicts
206214
continue_after_resolution() {
207215
check_env_var "PR_BRANCH"
216+
check_env_var "PR_NUMBER"
217+
check_env_var "PR_BASE"
208218

209-
echo "Checking if $PR_BRANCH needs continuation after conflict resolution..."
219+
echo "Checking if PR #$PR_NUMBER ($PR_BRANCH) needs continuation after conflict resolution..."
210220

211221
# Check if the PR has the conflict label
212-
if ! pr_has_conflict_label "$PR_BRANCH"; then
222+
if ! pr_has_conflict_label "$PR_NUMBER"; then
213223
echo "$PR_BRANCH does not have conflict label; nothing to do"
214224
return
215225
fi
@@ -221,28 +231,23 @@ continue_after_resolution() {
221231
# Recover them from the marker the squash-merge run left in the conflict
222232
# comment.
223233
local MARKER
224-
MARKER=$(read_state_marker "$PR_BRANCH")
234+
MARKER=$(read_state_marker "$PR_NUMBER")
225235
if [[ -z "$MARKER" ]]; then
226236
echo "⚠️ No autorestack state marker on $PR_BRANCH; cannot resume safely. Removing the label."
227-
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."
237+
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."
228238
return
229239
fi
230240

231241
local OLD_BASE NEW_TARGET SQUASH_HASH
232242
read -r OLD_BASE NEW_TARGET SQUASH_HASH < <(parse_state_marker "$MARKER")
233243
echo "Recorded state: base=$OLD_BASE target=$NEW_TARGET squash=$SQUASH_HASH"
234244

235-
# The base we left the PR on while waiting for conflict resolution was the
236-
# merged parent branch. If it no longer matches, a human retargeted the PR
237-
# (e.g. straight onto the integration branch); we are no longer the authority
238-
# on its base, so we step back without touching the branch. This runs before
239-
# any mutation: once the base diverges, the recorded target is stale and a
240-
# merge built against it would be wrong.
241-
local CURRENT_BASE
242-
CURRENT_BASE=$(gh pr view "$PR_BRANCH" --json baseRefName --jq '.baseRefName')
243-
if [[ "$CURRENT_BASE" != "$OLD_BASE" ]]; then
244-
echo "⚠️ Base of $PR_BRANCH changed manually ($OLD_BASE -> $CURRENT_BASE); not updating the stack."
245-
abandon_resume "$PR_BRANCH" "ℹ️ The base branch of this PR was changed manually, so autorestack stepped back and will not update it automatically."
245+
# The PR was left based on the merged parent branch. If the payload shows a
246+
# different base, a human retargeted the PR; the recorded target is stale,
247+
# so step back before any mutation.
248+
if [[ "$PR_BASE" != "$OLD_BASE" ]]; then
249+
echo "⚠️ Base of $PR_BRANCH changed manually ($OLD_BASE -> $PR_BASE); not updating the stack."
250+
abandon_resume "$PR_NUMBER" "ℹ️ The base branch of this PR was changed manually, so autorestack stepped back and will not update it automatically."
246251
return
247252
fi
248253

@@ -252,7 +257,7 @@ continue_after_resolution() {
252257
# can succeed, so give up cleanly rather than stranding the PR under the label.
253258
if ! git rev-parse --verify --quiet "origin/$NEW_TARGET" >/dev/null; then
254259
echo "⚠️ Recorded target branch '$NEW_TARGET' no longer exists; abandoning resume of $PR_BRANCH."
255-
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."
260+
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."
256261
return
257262
fi
258263

@@ -265,7 +270,7 @@ continue_after_resolution() {
265270
log_cmd git update-ref SQUASH_COMMIT "$SQUASH_HASH"
266271
MERGED_BRANCH="$OLD_BASE"
267272
TARGET_BRANCH="$NEW_TARGET"
268-
if ! update_direct_target "$PR_BRANCH" "$NEW_TARGET"; then
273+
if ! update_direct_target "$PR_BRANCH" "$NEW_TARGET" "$PR_NUMBER"; then
269274
echo "⚠️ '$PR_BRANCH' still conflicts; re-posted the conflict comment, will retry on next push"
270275
return 1
271276
fi
@@ -276,8 +281,8 @@ continue_after_resolution() {
276281
# NEW_TARGET when the base flips to it, keeping the PR mergeable (GitHub
277282
# suppresses CI on a PR that conflicts with its base).
278283
log_cmd git push origin "$PR_BRANCH"
279-
log_cmd gh pr edit "$PR_BRANCH" --base "$NEW_TARGET"
280-
log_cmd gh pr edit "$PR_BRANCH" --remove-label "$CONFLICT_LABEL"
284+
log_cmd gh pr edit "$PR_NUMBER" --base "$NEW_TARGET"
285+
log_cmd gh pr edit "$PR_NUMBER" --remove-label "$CONFLICT_LABEL"
281286

282287
# Check if old base branch should be deleted
283288
if has_sibling_conflicts "$OLD_BASE" "$PR_BRANCH"; then
@@ -297,17 +302,25 @@ main() {
297302
log_cmd git update-ref SQUASH_COMMIT "$SQUASH_COMMIT"
298303

299304
# Find all PRs directly targeting the merged PR's head
300-
INITIAL_TARGETS=($(log_cmd gh pr list --base "$MERGED_BRANCH" --json headRefName --jq '.[].headRefName'))
305+
INITIAL_NUMBERS=()
306+
INITIAL_TARGETS=()
307+
while read -r NUMBER BRANCH; do
308+
[[ -n "$BRANCH" ]] || continue
309+
INITIAL_NUMBERS+=("$NUMBER")
310+
INITIAL_TARGETS+=("$BRANCH")
311+
done < <(log_cmd gh pr list --base "$MERGED_BRANCH" --json number,headRefName --jq '.[] | "\(.number) \(.headRefName)"')
301312

302313
# Track successfully updated vs conflicted branches separately
303314
UPDATED_TARGETS=()
315+
UPDATED_NUMBERS=()
304316
CONFLICTED_TARGETS=()
305317

306-
for BRANCH in "${INITIAL_TARGETS[@]}"; do
307-
if update_direct_target "$BRANCH" "$TARGET_BRANCH"; then
308-
UPDATED_TARGETS+=("$BRANCH")
318+
for i in "${!INITIAL_TARGETS[@]}"; do
319+
if update_direct_target "${INITIAL_TARGETS[$i]}" "$TARGET_BRANCH" "${INITIAL_NUMBERS[$i]}"; then
320+
UPDATED_TARGETS+=("${INITIAL_TARGETS[$i]}")
321+
UPDATED_NUMBERS+=("${INITIAL_NUMBERS[$i]}")
309322
else
310-
CONFLICTED_TARGETS+=("$BRANCH")
323+
CONFLICTED_TARGETS+=("${INITIAL_TARGETS[$i]}")
311324
fi
312325
done
313326

@@ -318,8 +331,8 @@ main() {
318331
log_cmd git push origin "${UPDATED_TARGETS[@]}"
319332
fi
320333

321-
for BRANCH in "${UPDATED_TARGETS[@]}"; do
322-
log_cmd gh pr edit "$BRANCH" --base "$TARGET_BRANCH"
334+
for NUMBER in "${UPDATED_NUMBERS[@]}"; do
335+
log_cmd gh pr edit "$NUMBER" --base "$TARGET_BRANCH"
323336
done
324337

325338
# Deleting a PR's base branch closes the PR, so this must come after the

0 commit comments

Comments
 (0)