Skip to content

fix: improve upstream sync workflow reliability and prevent squash merges - #43

Merged
mateo-di merged 2 commits into
carto/mainfrom
fix/upstream-sync-issues
Dec 2, 2025
Merged

fix: improve upstream sync workflow reliability and prevent squash merges#43
mateo-di merged 2 commits into
carto/mainfrom
fix/upstream-sync-issues

Conversation

@mateo-di

@mateo-di mateo-di commented Dec 2, 2025

Copy link
Copy Markdown
Collaborator

Description

Shortcut

  • Autolink: [sc-521238]

Fixes two issues with the upstream sync workflow:

  1. Resolver workflow not triggering: The pull_request: opened event doesn't fire when PRs are created by workflows (even with PAT tokens). Added explicit gh workflow run dispatch after PR creation.

  2. Squash merge prevention: PR fix: resolve conflicts for upstream sync PR #24 #26 was squash-merged, which destroyed all upstream commit history (4201 commits). This caused PR 🔄 sync: upstream v1.79.3-stable #41 to show inflated diff counts (1875 files instead of ~300). Added prominent warnings to prevent this in the future.

Changes

carto-upstream-sync.yml

  • Add Check conflicts and trigger resolver step after PR creation
  • Check mergeable status with retries (GitHub computes this async)
  • Dispatch resolver workflow when conflicts detected

carto-upstream-sync-resolver.yml

  • Add prominent merge warning at TOP of resolution PR body
  • Add warning comment step after resolution PR creation
  • Clear explanation of why squash/rebase destroys history

Type of change

  • Fix

Acceptance

  1. Verify YAML syntax is valid
  2. Review the merge warning text in PR body template
  3. Check the resolver dispatch logic handles edge cases
  4. Future upstream syncs should:
    • Auto-trigger resolver when conflicts detected
    • Show clear merge warnings on resolution PRs

Basic checklist

  • Good PR name
  • Shortcut link
  • Just one issue per PR
  • GitHub labels
  • Proper status & reviewers
  • Tests (N/A - workflow changes)
  • Documentation (in commit message)

…rges

## Changes

### carto-upstream-sync.yml
- Add automatic resolver dispatch after PR creation
- Check mergeable status with retries (handles GitHub's async computation)
- Trigger resolver workflow when conflicts are detected

### carto-upstream-sync-resolver.yml
- Add prominent merge warning at top of resolution PR body
- Add warning comment step after resolution PR creation
- Warn against squash/rebase merge which destroys upstream history

## Why

1. **Resolver not triggering**: GitHub doesn't fire `pull_request` events
   when PRs are created by workflows, even with PAT tokens. This adds
   explicit workflow dispatch as a reliable trigger.

2. **Squash merge prevention**: PR #26 was squash-merged, which lost all
   upstream commit history (4201 commits). This caused PR #41 to show
   inflated diff counts. Adding clear warnings to prevent this.

Shortcut

- Autolink: [sc-521238]
@mateo-di

mateo-di commented Dec 2, 2025

Copy link
Copy Markdown
Collaborator Author

/gemini review

…history

The previous approach (checkout carto/main, merge main INTO it) lost upstream
commit history because the PR only showed Claude's resolution commits, not
the upstream commits.

New approach:
1. Checkout main (has all upstream commits)
2. Merge carto/main INTO it (brings CARTO customizations)
3. Resolve conflicts
4. Create PR to carto/main

This ensures the resolution PR shows ALL upstream commits + resolution commits,
preserving full commit history in carto/main.

Also updated Claude's prompt to reflect:
- Reversed conflict marker interpretation (HEAD=upstream, theirs=CARTO)
- Updated ours/theirs terminology in file-specific rules
- Clear explanation of why this approach preserves history
@mateo-di
mateo-di marked this pull request as ready for review December 2, 2025 21:58
@mateo-di
mateo-di merged commit d0a3d3d into carto/main Dec 2, 2025
3 of 4 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