Skip to content

Fix merge conflict handling in the rebuild workflow - #4191

Open
henrymercer wants to merge 2 commits into
mainfrom
henrymercer/ci-job-failure-investigation
Open

henrymercer wants to merge 2 commits into
mainfrom
henrymercer/ci-job-failure-investigation

Conversation

@henrymercer

@henrymercer henrymercer commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

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 with bash -e, so a conflicting git merge ended the step before its exit code was recorded. The step now runs git merge as an if condition, which -e doesn't apply to, and continues only if MERGE_HEAD shows the merge is still in progress.

Fixing that alone would have exposed a second bug: under pipefail, the check for conflicts outside lib never triggered, because git diff --check exits non-zero whenever it finds a conflict marker, including in lib. The check now lists unmerged paths with git 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:

  • Low risk: Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.

Which use cases does this change impact?

Environments:

  • Testing/None - This change does not impact any CodeQL workflows in production.

How did/will you validate this change?

  • Other - I ran the merge step's script from the workflow under bash --noprofile --norc -e -o pipefail against scratch repositories. A clean merge passes, conflicts only in lib continue and set merge-in-progress, and content or modify/delete conflicts in src fail and list the file. I also ran the whole workflow on a temporary branch with the same conflict in lib/entry-points.js as 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?

  • Development/testing only - This change cannot cause any failures in production.

How will you know if something goes wrong after this change is released?

  • Other - This change only affects the Rebuild Action workflow, so problems will show up as failed or incorrect rebuild runs.

Are there any special considerations for merging or releasing this change?

  • No special considerations - This change can be merged at any time.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Consider adding a changelog entry for this change.
  • Confirm the readme and docs have been updated if necessary.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@henrymercer
henrymercer requested a balanced review from Copilot October 2, 2026 15:48
@github-actions github-actions Bot added the size/XS Should be very easy to review label Oct 2, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 merge exit status under bash -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>
@henrymercer
henrymercer force-pushed the henrymercer/ci-job-failure-investigation branch from 500a66b to 56b0d5f Compare October 2, 2026 16:12
@github-actions github-actions Bot added size/S Should be easy to review and removed size/XS Should be very easy to review labels Oct 2, 2026
@henrymercer
henrymercer requested a balanced review from Copilot October 2, 2026 16:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The focused workflow changes correctly address both failure modes and align with the rebuild process.

Review effort: Balanced
Findings: None

@henrymercer
henrymercer marked this pull request as ready for review October 2, 2026 16:22
@henrymercer
henrymercer requested a review from a team as a code owner October 2, 2026 16:22

@mbg mbg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines -73 to -74
# Check for merge conflicts outside of `lib`. Disable git diff's trailing whitespace check
# since `node_modules/@types/semver/README.md` fails it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this no longer applicable?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Should be easy to review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants