Skip to content

fix(gh-workflows): handle tree-level merge conflicts in upstream sync - #98

Merged
mateo-di merged 2 commits into
carto/mainfrom
fix/upstream-sync-tree-level-conflicts
Mar 16, 2026
Merged

fix(gh-workflows): handle tree-level merge conflicts in upstream sync#98
mateo-di merged 2 commits into
carto/mainfrom
fix/upstream-sync-tree-level-conflicts

Conversation

@mateo-di

@mateo-di mateo-di commented Mar 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes the upstream sync workflow (carto-upstream-sync-main.yml) which has failed on the last 2 runs due to unhandled tree-level merge conflicts in Next.js build artifacts.

What Changed

Modified:

  • .github/workflows/carto-upstream-sync-main.yml — Merge + conflict resolution logic in the sync-main-branch job

Root Cause

Two compounding bugs:

  1. set -eu kills the fallback — The git merge command was a standalone statement, so when it returned non-zero, set -e terminated the script immediately. The conflict resolution fallback on the else branch was dead code that never executed.

  2. Fallback only detected content conflicts — Even if it ran, git diff --name-only --diff-filter=U only captures content-level (both-modified) conflicts. The actual conflicts were tree-level types that -X theirs cannot resolve:

    • rename/rename (3) — Next.js build hash directories renamed differently on both sides
    • rename/delete (8) — JS chunks renamed in HEAD but deleted in upstream
    • modify/delete (11) — Files modified on one side, deleted on the other

All 22 conflicts were in litellm/proxy/_experimental/out/ (generated Next.js static build output).

Fix

Commit 1: Core bug fix

  1. Capture merge exit code safely — Use git merge ... || MERGE_RESULT=$? pattern. The || makes this a compound command, so set -e doesn't trigger on the left side's failure.

  2. Detect all conflict types — Replace git diff --diff-filter=U with git ls-files --unmerged | awk '{print $4}' | sort -u, which captures every unmerged entry regardless of conflict type.

  3. Resolve per-file — For each unmerged path, check if it exists at MERGE_HEAD (the upstream tag):

    • Exists → git checkout MERGE_HEAD -- "$file" (accept upstream version)
    • Deleted → git rm -f "$file" (remove from tree)
    • Then git add -A to clean up orphaned rename artifacts

Commit 2: Observability & safety improvements

  1. Conflict classification — Capture merge output and parse CONFLICT (type) lines to produce a typed summary (rename/rename, modify/delete, rename/delete, content) with collapsible ::group:: blocks.

  2. Directory grouping — Group unmerged files by top-level directory (3 path components) for at-a-glance understanding.

  3. Post-resolution validation — After resolving all conflicts, verify git ls-files --unmerged returns 0 entries before committing. Fails the job if anything was missed.

  4. Commit message traceability — Include conflict type counts in the merge commit message.

Failed Runs (evidence)

Run Date Target Error
#22852970280 2026-03-09 v1.81.14-stable Tree-level conflicts in _experimental/out/
#23143385304 2026-03-16 v1.82.0-stable Same pattern, same directory

Local Dry-Run Test Results

Ran the exact script against the real v1.82.0-stable tag on an isolated local branch (never pushed). Results:

Script Execution: PASSED

The merge failed as expected (same 22 CONFLICT lines as CI), then the fallback resolved all conflicts automatically.

Conflict Classification: PASSED

Conflict types:
  rename/rename:  3
  modify/delete:  11
  rename/delete:  8

Affected Directories (20 files):
  litellm/proxy/_experimental/  20 files

Resolution Actions

Action Count Description
Accepting upstream 6 Files existing in upstream tag (3 hash dir files + 3 index.html)
Removing (deleted upstream) 14 Stale HEAD-only files (old build hashes + old rename targets)

Post-Resolution Validation: PASSED

git ls-files --unmerged returned 0 entries.

File Integrity: PASSED

Check Result
Upstream files missing from our tree 0 (all preserved)
Content differences vs upstream 0
organizations/index.html restored Yes
teams/index.html restored Yes
tools/mcp-servers/index.html restored Yes
Old hash dir FNzcPu... removed Yes
Old hash dir C_XKHL... (HEAD rename) removed Yes
Upstream hash dir U_YrOO... present Yes
pyproject.toml version 1.82.0
Working tree clean after commit Yes
Merge commit has 2 parents Yes

Productive Branches: UNTOUCHED

Branch Hash Changed?
carto/main 6452946 No
origin/main eccba906 No

Downstream Workflow Impact: NONE

Deep-dive analysis of all 6 upstream sync workflows confirmed no interference:

  • Resolver (carto-upstream-sync-resolver.yml) — merges in opposite direction (carto/main into sync branch), unaffected
  • CI Fixer — operates on sync branch only, unaffected
  • Ready Checker — read-only PR status checks, unaffected
  • Conflict Detector — triggered by pushes to carto/main, unaffected
  • Customizations Analyzer — reads files from sync branch + carto/main, unaffected
  • No workflow depends on .gitattributes or the specific conflict resolution method used in sync-main-branch

Architectural Context

EAD: N/A - small change

Review Focus Areas

Critical areas:

  1. .github/workflows/carto-upstream-sync-main.yml:162-264 — The new merge + fallback logic. Verify the || MERGE_RESULT=$? pattern is safe with set -eu, and that git ls-files --unmerged + git cat-file -e MERGE_HEAD:path correctly handles all conflict types.

Safe to skip: No other files changed.

Deployment Impact

  • Not applicable (CI workflow only)

Migration & Breaking Changes

  • No migrations or breaking changes

Security Considerations

  • No security impact

Performance Impact

  • No performance impact

Tests

  • No automated tests needed (CI workflow change)
  • Local dry-run test passed against real v1.82.0-stable tag (see results above)

Dependencies

  • None

How to Validate

  1. Merge this PR to carto/main
  2. Manually trigger the upstream sync workflow (workflow_dispatch)
  3. Verify the sync-main-branch job succeeds and resolves tree-level conflicts automatically
  4. Check the merge commit message includes "Conflicts resolved: N files (X rename/rename, Y modify/delete, ...)"

AI-Generated Code Notice

  • This PR contains AI-generated code
  • Areas requiring extra verification: Conflict resolution logic — ensure git ls-files --unmerged output parsing and git cat-file -e MERGE_HEAD:path behave as expected for all conflict types

Checklist

  • PR title follows convention
  • One issue per PR
  • Local dry-run test passed
  • Downstream workflow impact analyzed (no interference)
  • AI review findings addressed

The upstream sync workflow failed on the last 2 runs because set -e
killed the script before the fallback conflict resolution could run,
and the fallback only detected content-level conflicts (--diff-filter=U)
missing tree-level conflicts (rename/rename, modify/delete, rename/delete)
in litellm/proxy/_experimental/out/ (Next.js build artifacts).

Fix: capture merge exit code with || pattern to survive set -e, and use
git ls-files --unmerged to detect all conflict types. Resolve each by
checking MERGE_HEAD: accept upstream version or remove if deleted.

Co-authored-by: Claude <noreply@anthropic.com>
@mateo-di

Copy link
Copy Markdown
Collaborator Author

/gemini review

…alidation

Capture merge output for conflict type classification (rename/rename,
modify/delete, rename/delete, content) and group unmerged files by
directory using collapsible ::group:: blocks for cleaner CI logs.

Add post-resolution validation that verifies no unmerged files remain
after conflict resolution, failing the job early if something was missed
rather than creating a broken merge commit.

Include conflict type counts in the merge commit message for traceability.

Co-authored-by: Claude <noreply@anthropic.com>
@mateo-di
mateo-di marked this pull request as ready for review March 16, 2026 19:57
@mateo-di
mateo-di merged commit b111369 into carto/main Mar 16, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant