Fix merge conflict handling in the rebuild workflow - #4191
henrymercer wants to merge 2 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The workflow now safely continues for lib-only conflicts while rejecting conflicts elsewhere.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes rebuild workflow handling for merge conflicts limited to generated lib files.
Changes:
- Preserves
git mergeexit status underbash -e. - Detects all unmerged paths outside
lib, including modify/delete conflicts.
| File | Description |
|---|---|
.github/workflows/rebuild.yml |
Corrects merge failure handling and conflict-path detection. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
500a66b to
56b0d5f
Compare
mbg
left a comment
There was a problem hiding this comment.
Generally makes sense to me and it's a low risk / dev-only change in any case, so we can iterate on it if we find something doesn't quite work.
Ideally, we'd also start moving away from shell for these things and have TS-based scripts in pr-checks for this (which would have avoided the two bugs I think), but not something we should do here as part of this PR obviously.
| # Check for merge conflicts outside of `lib`. Disable git diff's trailing whitespace check | ||
| # since `node_modules/@types/semver/README.md` fails it. |
The rebuild workflow is meant to continue when merging the base branch only conflicts in
lib, since rebuilding regenerates it. Since #3568, any conflict has failed the merge step instead, for example in this run. Steps run withbash -e, so a conflictinggit mergeended the step before its exit code was recorded. The step now runsgit mergeas anifcondition, which-edoesn't apply to, and continues only ifMERGE_HEADshows the merge is still in progress.Fixing that alone would have exposed a second bug: under
pipefail, the check for conflicts outsidelibnever triggered, becausegit diff --checkexits non-zero whenever it finds a conflict marker, including inlib. The check now lists unmerged paths withgit diff --name-only --diff-filter=U, which also catches conflicts without markers, such as modify/delete conflicts.Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Environments:
How did/will you validate this change?
bash --noprofile --norc -e -o pipefailagainst scratch repositories. A clean merge passes, conflicts only inlibcontinue and setmerge-in-progress, and content or modify/delete conflicts insrcfail and list the file. I also ran the whole workflow on a temporary branch with the same conflict inlib/entry-points.jsas the linked run. It succeeded, and the merge commit it pushed had no conflict markers.If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist