Cover conflict resolution matrix in e2e#35
Merged
Conversation
Alternative to #32 for the same stuck-PR bug — pick one. After a parent PR is squash-merged, updating a descendant branch runs three merges: the parent branch, the target's pre-squash state, then the squash recorded with `-s ours`. When the pre-squash merge conflicted, the action commented and labeled without pushing anything, so the head was left missing both the base merge and the squash record. The branch never became mergeable, and because GitHub skips `pull_request` workflows on a PR that conflicts with its base, the `synchronize` event that resumes the action never fired again — permanently stuck. This splits the work so the user only resolves the genuine conflict: - When the base-branch merge is clean and only the pre-squash merge conflicts, push the base merge to the branch before commenting and labeling. The head stays a descendant of its base, so the PR stays mergeable and the resume trigger keeps firing. If the base merge itself conflicts there is nothing safe to pre-push, so the existing comment-and-label fallback runs. - After the user resolves and pushes, continue_after_resolution records the squash with `-s ours` and pushes before retargeting. The synchronize payload is the child PR, so the squash commit and new target are reconstructed from the merged parent PR (its merge commit and base). The conflict comment is unchanged: it already lists the genuine conflict. Offline test covers both halves: the squash-merge run pushes the base merge before the unchanged comment, and the follow-up run records the squash so the branch ends up mergeable into the new target. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pushing the base merge before commenting leaves the user's local branch behind origin, so following the comment verbatim resolved on a stale head and the final push was rejected as non-fast-forward. The recipe now fast-forwards with `git pull --ff-only origin <branch>` first. The standalone offline test is dropped. The existing e2e conflict scenario already triggers the clean-base-merge / conflicting-pre-squash case, so it now asserts the action pushed the base merge on top of the pre-conflict head and that the comment asks only for the genuine conflict, then resolves by following the comment verbatim (re-sync included) and checks the squash gets recorded so the branch ends up mergeable into the new target. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The sibling conflict scenario still resolved branches with a hardcoded merge against main, so it could pass even if the generated instructions regressed. Reusing the comment-following path makes PR6 and PR7 validate the same workflow users run, while keeping the PR3 flow on the shared helper.
- Note that the base-merge push must precede the label, or the synchronize it emits re-triggers this action against an unresolved branch. - Collapse the duplicated `git pull --ff-only` recipe line to one echo plus a conditional trailing comment. - Fold the new-target and squash-commit lookups into a single `gh pr list` call; `// ""` keeps the absent-PR guard firing. - Reword the continuation conflict message to say it re-posted the comment and will retry, since the path is self-healing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Noé Rubinstein <noe.rubinstein@gmail.com>
The conflict flow now has distinct behavior for base-branch conflicts, trunk conflicts, and follow-up conflicts after a manual resolution. The e2e test exercises those paths explicitly and follows the generated bash blocks rather than a parallel hardcoded recipe, so regressions in the posted instructions are caught by the same workflow users run.
The base branch accepted review suggestions while the stacked branch split conflict instructions into separately executable command blocks. Resolve that overlap by keeping the simplified pull command from the base branch and preserving the literal multi-block recipe covered by the stacked tests.
The conflict matrix creates several pull_request runs on the same head branch. Matching only the branch can pick a stale synchronize run when the test is waiting for the closed run from a merge. Name the installed workflow with the pull_request action and PR number, then have the e2e waiters ignore runs that existed before the trigger and match the expected action-specific run name.
The e2e waiters need to distinguish closed and synchronize pull_request runs that can share the same head branch. Quote the generated run name so the PR number is preserved instead of parsed as a YAML comment, and align the local e2e wrapper with the token generator's file-based private key lookup.
The pre-pushed base merge leaves the user's local branch strictly behind origin, so the pull is always a fast-forward and the flag is a no-op in the expected case. Plain `git pull` is one less thing in the recipe. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lict-resolution-matrix # Conflicts: # tests/test_e2e.sh # update-pr-stack.sh
The helper checked that the conflict comment contains `git pull origin <branch>`, but that line is static text the action always emits, so the assertion only restated the script. The helper still runs the pull. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
…comment' into test-conflict-resolution-matrix # Conflicts: # tests/test_e2e.sh
Phlogistique
commented
Jun 1, 2026
Phlogistique
commented
Jun 1, 2026
Phlogistique
commented
Jun 1, 2026
Comment on lines
+114
to
+127
| for i in "${!CONFLICTS[@]}"; do | ||
| echo '```bash' | ||
| if [[ "$i" -eq 0 ]]; then | ||
| echo "git fetch origin" | ||
| echo "git switch $BRANCH" | ||
| echo "git pull origin $BRANCH" | ||
| fi | ||
| echo "git merge ${CONFLICTS[$i]}" | ||
| echo '```' | ||
| echo | ||
| echo 'Fix the conflicts (for instance with `git mergetool`), then run `git commit` before continuing.' | ||
| echo | ||
| done | ||
| echo "git push" | ||
| echo '```bash' |
Collaborator
Author
There was a problem hiding this comment.
Suggested change
| for i in "${!CONFLICTS[@]}"; do | |
| echo '```bash' | |
| if [[ "$i" -eq 0 ]]; then | |
| echo "git fetch origin" | |
| echo "git switch $BRANCH" | |
| echo "git pull origin $BRANCH" | |
| fi | |
| echo "git merge ${CONFLICTS[$i]}" | |
| echo '```' | |
| echo | |
| echo 'Fix the conflicts (for instance with `git mergetool`), then run `git commit` before continuing.' | |
| echo | |
| done | |
| echo "git push" | |
| echo '```bash' | |
| echo '```bash' | |
| echo "git fetch origin" | |
| echo "git switch $BRANCH" | |
| echo "git pull origin $BRANCH" | |
| for i in "${!CONFLICTS[@]}"; do | |
| echo "git merge ${CONFLICTS[$i]}" | |
| echo '```' | |
| echo | |
| echo 'Fix the conflicts (for instance with `git mergetool`), then run `git commit` before continuing.' | |
| echo | |
| echo '```bash' | |
| done |
Lift the first conflict block's fetch/switch/pull out of the loop and open the next block at each iteration's tail, dropping the per-iteration `if [[ "$i" -eq 0 ]]` conditional. Rendered comment is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…helpers - follow_conflict_comment now rewrites only the conflicted region to a sentinel line, keeping changes the merge brought in cleanly. The previous whole-file `git checkout --ours` dropped any clean incoming change, so the test only matched a human resolution by accident of the fixtures. - The workflow-run waiters match the run-name prefix through " - ", so "PR #2" no longer prefix-matches "PR #20"; the head-branch tiebreaker is gone. - edit_and_commit / create_pr replace the stack-setup boilerplate repeated across every scenario. - Add the missing Scenario 3 entry to the top-of-file index. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reverts #34's switch to a .gh-app-private-key.pem file. The env-var form (GH_APP_PRIVATE_KEY) lets the token generator run in Claude Code web, where the key is set as an environment variable rather than a file. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reverts #34. Reading GH_APP_PRIVATE_KEY from the environment instead of a .gh-app-private-key.pem file lets the token generator run in Claude Code web, where the key is provided as an environment variable rather than a file. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The conflict-resolution path has several distinct states now: a clean automatic update, an immediate base-branch conflict, an immediate trunk conflict, both conflicts in one run, and a follow-up trunk conflict after the user resolves the base conflict. The existing e2e coverage mostly exercised the trunk-conflict case.
This makes the generated conflict comment easier to test by splitting executable commands into separate bash blocks, then teaches the e2e test to run those blocks directly. The new matrix scenarios assert the exact merge refs printed in the comment and verify that the final PR diffs contain only the intended resolved conflict changes.
Validated with
bash -n update-pr-stack.sh,bash -n tests/test_e2e.sh,bash tests/test_update_pr_stack.sh,bash tests/test_rebase_workflow.sh,bash tests/test_mixed_workflows.sh,git diff --check, andshellcheck update-pr-stack.sh tests/test_e2e.sh(existing warnings only). The full GitHub e2e was left to CI because it creates and deletes a real repository.