diff --git a/.github/workflows/carto-upstream-sync-resolver.yml b/.github/workflows/carto-upstream-sync-resolver.yml index c13cb04be12..60c7963104f 100644 --- a/.github/workflows/carto-upstream-sync-resolver.yml +++ b/.github/workflows/carto-upstream-sync-resolver.yml @@ -1,12 +1,23 @@ name: CARTO Upstream Sync - Conflict Resolver -# This workflow uses Claude Code to automatically create conflict resolution proposals -# for upstream sync PRs that have merge conflicts. +# This workflow uses Claude Code to automatically resolve merge conflicts +# for upstream sync PRs, pushing resolution directly to the sync branch. +# +# Architecture (Single-PR, Deterministic): +# 1. Workflow starts merge (HARDCODED) - brings conflicts into working tree +# 2. Claude edits files ONLY - resolves conflict markers +# 3. Workflow completes merge commit (HARDCODED) - preserves both parents +# 4. Workflow pushes to sync branch (HARDCODED) - updates existing PR +# +# This ensures: +# - Single PR workflow (no separate resolution PR) +# - Merge commit always has both parents (upstream + carto/main) +# - Git operations are deterministic (not dependent on Claude's actions) # # Security features: # - Only runs when triggered by authorized users (Cartofante, mateo-di) # - Only runs on PRs labeled 'upstream-sync' -# - Only runs on main branch (upstream sync PRs) +# - Only runs on upstream-sync/* branches (not main directly) # - Only runs when conflicts are detected # - Blocks external forks # - Rate limited (max-turns: 250, job timeout: 90 mins) @@ -94,14 +105,14 @@ jobs: fi echo "[Security] ✅ PR source verified: internal branch" - # Check branch flow - if [[ "${HEAD_BRANCH}" != "main" ]] || [[ "${BASE_BRANCH}" != "carto/main" ]]; then + # Check branch flow - allow upstream-sync/* branches + if [[ ! "${HEAD_BRANCH}" =~ ^upstream-sync/ ]] || [[ "${BASE_BRANCH}" != "carto/main" ]]; then echo "[Security] ❌ Invalid branch flow: ${HEAD_BRANCH} → ${BASE_BRANCH}" - echo "[Security] Required: main → carto/main" + echo "[Security] Required: upstream-sync/* → carto/main" echo "authorized=false" >> $GITHUB_OUTPUT exit 0 fi - echo "[Security] ✅ Branch flow verified: main → carto/main" + echo "[Security] ✅ Branch flow verified: ${HEAD_BRANCH} → carto/main" else # Manual trigger: fetch PR details via gh CLI PR_NUMBER="${{ github.event.inputs.pr-number }}" @@ -129,14 +140,14 @@ jobs: fi echo "[Security] ✅ PR source verified: internal branch" - # Check branch flow - if [[ "${HEAD_BRANCH}" != "main" ]] || [[ "${BASE_BRANCH}" != "carto/main" ]]; then + # Check branch flow - allow upstream-sync/* branches + if [[ ! "${HEAD_BRANCH}" =~ ^upstream-sync/ ]] || [[ "${BASE_BRANCH}" != "carto/main" ]]; then echo "[Security] ❌ Invalid branch flow: ${HEAD_BRANCH} → ${BASE_BRANCH}" - echo "[Security] Required: main → carto/main" + echo "[Security] Required: upstream-sync/* → carto/main" echo "authorized=false" >> $GITHUB_OUTPUT exit 0 fi - echo "[Security] ✅ Branch flow verified: main → carto/main" + echo "[Security] ✅ Branch flow verified: ${HEAD_BRANCH} → carto/main" fi # All security checks passed @@ -266,12 +277,13 @@ jobs: echo "head-ref=${HEAD_REF}" >> $GITHUB_OUTPUT echo "base-ref=${BASE_REF}" >> $GITHUB_OUTPUT echo "[Resolver] PR: ${HEAD_REF} → ${BASE_REF}" - echo "[Resolver] Will checkout ${HEAD_REF} (upstream) and merge ${BASE_REF} (CARTO) into it" + echo "[Resolver] Will checkout sync branch (${HEAD_REF}) and merge ${BASE_REF} into it" + echo "[Resolver] Resolution commits will be pushed directly to this PR's branch" - # IMPORTANT: Checkout HEAD branch (main) which has upstream commit history + # IMPORTANT: Checkout the sync branch (upstream-sync/vX.Y.Z) which has upstream commit history # Then merge BASE branch (carto/main) into it to bring CARTO customizations - # This preserves upstream commit history in the resulting PR - - name: Checkout repository (head branch - upstream with history) + # Resolution commits are pushed directly to this branch, updating the existing PR + - name: Checkout repository (sync branch - upstream with history) uses: actions/checkout@v4 with: ref: ${{ steps.pr-info.outputs.head-ref }} @@ -283,9 +295,10 @@ jobs: git config --global user.name "Cartofante" git config --global user.email "cartofante@carto.com" - - name: Attempt merge to create conflicts + - name: Start merge with carto/main (HARDCODED - creates conflicts) + id: start-merge run: | - echo "::group::Merging CARTO customizations into upstream" + echo "::group::Starting merge (WORKFLOW CONTROLLED)" echo "[Resolver] Strategy: Checkout ${HEAD_REF} (upstream) → merge ${BASE_REF} (CARTO) into it" echo "[Resolver] This preserves upstream commit history in the PR" @@ -301,16 +314,28 @@ jobs: echo "[Resolver] ✅ Clean merge - no conflicts found" echo "[Resolver] This PR should be mergeable without Claude's help" git merge --abort || true + echo "has-conflicts=false" >> $GITHUB_OUTPUT exit 1 else - echo "[Resolver] ⚠️ Conflicts detected - Claude will resolve them" - echo "[Resolver] Conflicted files:" - git diff --name-only --diff-filter=U + echo "[Resolver] ⚠️ Conflicts detected - Claude will resolve file contents" + + # Save list of conflicted files for Claude + CONFLICTS=$(git diff --name-only --diff-filter=U) + echo "conflicts<> $GITHUB_OUTPUT + echo "$CONFLICTS" >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT + + CONFLICT_COUNT=$(echo "$CONFLICTS" | wc -l) + echo "conflict-count=${CONFLICT_COUNT}" >> $GITHUB_OUTPUT + echo "has-conflicts=true" >> $GITHUB_OUTPUT + + echo "[Resolver] Conflicted files (${CONFLICT_COUNT}):" + echo "$CONFLICTS" fi echo "::endgroup::" - - name: Comment on original PR - Starting + - name: Comment on PR - Starting env: GH_TOKEN: ${{ secrets.X_GITHUB_SUPERCARTOFANTE }} run: | @@ -327,7 +352,7 @@ jobs: 3. ⚖️ Applying resolution priorities (CARTO first, then upstream) 4. ✏️ Resolving conflicts file by file 5. 🧪 Running tests (lint, mypy, unit tests) - 6. 🔀 Creating resolution PR + 6. 📌 **Pushing resolution directly to this PR** (single-PR workflow!) **Status:** In progress... @@ -337,7 +362,7 @@ jobs: EOF )" - - name: Run Claude Code for conflict resolution + - name: Run Claude Code for conflict resolution (FILE EDITS ONLY) id: claude-run uses: anthropics/claude-code-action@v1 with: @@ -346,473 +371,284 @@ jobs: show_full_output: true claude_args: "--model claude-sonnet-4-5-20250929 --max-turns 250 --allowed-tools Read,Write,Edit,Bash,Grep,Glob ${{ github.event.inputs.debug == 'true' && '--debug' || '' }}" prompt: | - # Upstream Sync Conflict Resolution Task + # Conflict Resolution Task - EDIT FILES ONLY - ## 📚 IMPORTANT: Read Project Guidelines First - Before starting, read `CARTO_CLAUDE.md` to understand CARTO-specific conventions, - coding standards, and infrastructure patterns. This file contains essential context - about how CARTO customizes LiteLLM. + ## ⚠️ CRITICAL: YOUR ONLY JOB IS TO EDIT FILES - ## Branch Strategy & Your Context + The git merge has ALREADY been started by the workflow. + You are in a merge conflict state with ${{ steps.start-merge.outputs.conflict-count }} conflicted files. - ``` - upstream/main → BerriAI's development branch - ↓ - main → Mirrors upstream/main (has ALL upstream commits) < YOU ARE HERE - ↑ (merge with conflicts) - carto/main → Production branch (CARTO customizations merged IN) - ``` + **DO NOT RUN THESE COMMANDS - THE WORKFLOW WILL HANDLE THEM:** + - ❌ `git commit` - Workflow does this AFTER you finish + - ❌ `git checkout -b` - Workflow does this AFTER you finish + - ❌ `git push` - Workflow does this AFTER you finish + - ❌ `gh pr create` - Workflow does this AFTER you finish - **What happened:** - 1. `main` branch was synced from upstream BerriAI/litellm (has full commit history) - 2. PR #${{ needs.check-eligibility.outputs.pr-number }} was created: `main → carto/main` - 3. To preserve upstream commit history, we REVERSED the merge direction: - - Checked out `main` (upstream with full history) - - Merged `carto/main` INTO it (bringing CARTO customizations) - 4. You are now on `main` branch with `carto/main` merged in (in conflicted state) - - **Your task:** Resolve the conflicts and create a resolution PR to `carto/main` - - **WHY this approach:** This preserves upstream commit history in the final PR. - The resolution PR will show ALL upstream commits + your resolution commits. - - ## Context - - **Current branch**: `main` (upstream, with `carto/main` merged in - conflicted) - - **Project Guidelines**: See CARTO_CLAUDE.md for detailed conventions - - **Original PR**: #${{ needs.check-eligibility.outputs.pr-number }} (main → carto/main) - - **Your mission**: Resolve conflicts, create PR: `upstream-sync-resolver/${{ needs.check-eligibility.outputs.pr-number }} → carto/main` - - ## ⚠️ CRITICAL: Resolution Priorities (In Order) - - When resolving conflicts, follow this strict priority order: - - ### Priority 1: 🏢 CARTO Customizations (HIGHEST PRIORITY) - **ALWAYS preserve CARTO customizations from carto/main** - - Any code marked with `# CARTO:` comments → KEEP CARTO version - - CARTO-specific workflows, configs, documentation → KEEP CARTO version - - CARTO infrastructure code (Docker, DB scripts) → KEEP CARTO customizations - - When in doubt between CARTO vs upstream customization → KEEP CARTO - - ### Priority 2: 🔧 LiteLLM Core Functionalities - **Accept upstream improvements to core library** - - Bug fixes in `litellm/` core → ACCEPT upstream - - New LLM provider support → ACCEPT upstream - - API improvements and new features → ACCEPT upstream - - Performance optimizations → ACCEPT upstream - - ### Priority 3: ⚖️ Conflict Resolution Strategy - **When both CARTO and upstream modified the same functionality:** - 1. If CARTO modified for infrastructure/deployment → KEEP CARTO - 2. If upstream improved core LiteLLM functionality → ACCEPT upstream - 3. If both are substantive changes → MANUALLY MERGE both (combine carefully) - 4. Document your reasoning in commit message - - ## Your Task - - ### Step 1: Verify Current State & Read Guidelines - ```bash - # FIRST: Read CARTO project guidelines - cat CARTO_CLAUDE.md + **YOUR ONLY JOB:** + 1. Read conflicted files + 2. Edit files to remove conflict markers + 3. Run tests and fix issues + 4. Say "RESOLUTION COMPLETE" when done - # Verify you're on main (upstream) with carto/main merged in (conflicted) - git status - # Should show: "You have unmerged paths" or similar - # Branch should be: main (or detached at main) + ## Conflicted Files (${{ steps.start-merge.outputs.conflict-count }} files) - # List all conflicted files - git diff --name-only --diff-filter=U - - # Review a sample conflict (optional) - # git diff ``` - - ### Step 2: Understand File-Specific Rules - - **NOTE:** Because we're on `main` and merged `carto/main` INTO it: - - **"ours"** = HEAD = main (upstream code) - - **"theirs"** = carto/main (CARTO customizations) - - **✅ ALWAYS KEEP CARTO VERSION (theirs / from carto/main):** - - `.github/workflows/carto_*.yaml` - CARTO workflows - - `.github/workflows/carto-*.yml` - CARTO workflows - - `CARTO_*.md` - CARTO documentation - - `docs/CARTO_*.md` - CARTO documentation - - Any file with CARTO-specific infrastructure code - - **🔄 ALWAYS ACCEPT UPSTREAM (ours / HEAD):** - - `pyproject.toml` - Version field (use upstream version) - - `litellm/llms/**` - LLM provider implementations - - `litellm/main.py` - Core completion functions - - `litellm/router.py` - Router logic - - `tests/` - Upstream test files (unless CARTO added custom tests) - - `requirements.txt` - Dependencies (unless CARTO added specific versions) - - **⚠️ CAREFULLY MERGE BOTH (manual resolution):** - - `Dockerfile` - Look for `# CARTO:` comments - - Keep CARTO sections marked with comments (theirs) - - Accept upstream improvements to base image/dependencies (ours) - - `docker/Dockerfile.non_root` - CARTO customizations - - Preserve CARTO user/permissions setup (theirs) - - Accept upstream dependencies/entrypoint improvements (ours) - - `db_scripts/` - May have CARTO custom scripts - - Keep CARTO scripts (theirs) - - Accept upstream schema improvements (ours) - - ### Step 3: Create Resolution Branch (from conflicted state) - ```bash - # Create branch from current conflicted state - git checkout -b upstream-sync-resolver/${{ needs.check-eligibility.outputs.pr-number }} + ${{ steps.start-merge.outputs.conflicts }} ``` - ### Step 3: Resolve Each Conflict + ## Step 1: Read CARTO Guidelines - For each conflicted file: - - 1. **Open the file and locate conflict markers:** - ``` - <<<<<<< HEAD (current: main branch - UPSTREAM code) - [upstream version - this is the new upstream code] - ======= - [CARTO version - this is what's coming from carto/main] - >>>>>>> carto/main (or commit hash - CARTO customizations) - ``` + ```bash + cat CARTO_CLAUDE.md + ``` - **IMPORTANT:** The markers are REVERSED from typical PRs because we're - on `main` and merging `carto/main` INTO it (to preserve upstream history). - - HEAD = upstream (main) - - Incoming = CARTO (carto/main) + ## Step 2: Resolution Rules - 2. **Apply the priority rules above** - - Keep CARTO infrastructure customizations (from the >>>>>>> side) - - Accept upstream core functionality improvements (from the <<<<<<< side) - - Manually merge when both are important + **Conflict markers look like this:** + ``` + <<<<<<< HEAD + [upstream code from main branch] + ======= + [CARTO code from carto/main branch] + >>>>>>> carto/main + ``` - 3. **Remove ALL conflict markers** (`<<<<<<<`, `=======`, `>>>>>>>`) + **Resolution priorities:** - 4. **Verify syntax** - ensure file is valid (use linter if needed) + | File Pattern | Keep | Reason | + |--------------|------|--------| + | `carto_*.yaml`, `carto-*.yml` | carto/main (bottom) | CARTO workflows | + | `CARTO_*.md` | carto/main (bottom) | CARTO docs | + | `litellm/**` (core) | HEAD (top) | Upstream improvements | + | `tests/**` | HEAD (top) | Upstream tests | + | `Dockerfile`, `Makefile` | Merge carefully | Keep `# CARTO:` sections | - 5. **Stage the resolved file:** - ```bash - git add - ``` + ## Step 3: Resolve Each File - 6. **Repeat for all conflicted files** until `git status` shows no unmerged paths + For EACH conflicted file: + 1. **Read** the file + 2. **Find** all `<<<<<<<`, `=======`, `>>>>>>>` markers + 3. **Decide** which version to keep (using rules above) + 4. **Edit** the file to remove markers and keep correct content + 5. **Verify** no markers remain in the file - ### Step 4: Comprehensive Testing (CRITICAL) + ## Step 4: Run Tests - **YOU MUST RUN ALL THESE TESTS AND ENSURE THEY PASS:** + After resolving ALL files: ```bash - # 1. Install dependencies - make install-dev + # Verify no conflict markers remain + grep -r "^<<<<<<< \|^=======\|^>>>>>>>" --include="*.py" litellm/ tests/ || echo "✅ No conflict markers found" - # 2. Linting (MUST PASS) + # Install deps and run tests + make install-dev make lint - - # 3. Type checking (MUST PASS) make lint-mypy - - # 4. Unit tests (MUST PASS) - make test-unit - - # 5. If time permits, run integration tests - # make test-integration ``` - **If any test fails:** - - Investigate the failure - - Fix the issue (likely a conflict resolution error) - - Re-run tests until ALL pass - - DO NOT proceed to PR creation if tests fail + Fix any errors that occur. - ### Step 5: Commit Your Changes - ```bash - git commit -m "fix: resolve upstream sync conflicts for PR #${{ needs.check-eligibility.outputs.pr-number }} + ## Step 5: Confirm Completion - Conflict resolution strategy: - - Preserved CARTO customizations in infrastructure files - - Accepted upstream improvements to core LiteLLM functionality - - Manually merged files with both CARTO and upstream changes + When ALL files are resolved and tests pass, say: - Testing: - - ✅ make lint passed - - ✅ make lint-mypy passed - - ✅ make test-unit passed + **"RESOLUTION COMPLETE"** - Files with manual merge: - [List files where you combined both versions] - " - ``` - - ### Step 6: Push Branch - ```bash - git push -u origin upstream-sync-resolver/${{ needs.check-eligibility.outputs.pr-number }} - ``` - - ### Step 7: Create Pull Request - - **CRITICAL: PR must target carto/main (base branch)** - - Use `gh pr create` with this template: - - ```bash - gh pr create \ - --repo ${{ github.repository }} \ - --base ${{ steps.pr-info.outputs.base-ref }} \ - --head upstream-sync-resolver/${{ needs.check-eligibility.outputs.pr-number }} \ - --title "fix: resolve conflicts for upstream sync PR #${{ needs.check-eligibility.outputs.pr-number }}" \ - --label "conflict-resolution" \ - --label "automated" \ - --label "needs-review" \ - --body "$(cat <<'PRBODY' - ## ⛔ CRITICAL: MERGE INSTRUCTIONS ⛔ - - > **🚨 DO NOT USE "Squash and merge" 🚨** - > - > You MUST select **"Create a merge commit"** when merging this PR. - - ### Why This Matters - - | Merge Type | Result | - |------------|--------| - | ✅ **Create a merge commit** | Preserves upstream history, future syncs work correctly | - | ❌ Squash and merge | **DESTROYS** upstream history, breaks future syncs | - | ❌ Rebase and merge | **DESTROYS** upstream history, breaks future syncs | - - ### What Happens If You Squash - - - 📈 Future sync PRs will show **thousands of extra commits/files** - - 🔍 `git bisect` and `git blame` become useless for upstream code - - 🔄 Every future sync will look like a massive change - - 😱 This ALREADY happened with PR #26 - don't repeat it! + The workflow will then: + 1. Stage all changes (`git add -A`) + 2. Create merge commit (`git commit`) + 3. Push directly to the sync branch (this PR will update automatically!) --- - ## 🔧 Automated Conflict Resolution - - This PR resolves merge conflicts in #${{ needs.check-eligibility.outputs.pr-number }}. - - ### 🎯 Resolution Strategy - - Followed strict priority order: - 1. **🏢 CARTO Customizations** - Preserved all CARTO-specific infrastructure - 2. **🔧 LiteLLM Core** - Accepted upstream improvements to core functionality - 3. **⚖️ Manual Merge** - Combined both when needed - - ### 📋 Files Modified + **Begin resolving conflicts now!** - **CARTO versions kept:** - [List files where you kept CARTO version] - - **Upstream versions accepted:** - [List files where you accepted upstream] + env: + GH_TOKEN: ${{ secrets.X_GITHUB_SUPERCARTOFANTE }} + GITHUB_TOKEN: ${{ secrets.X_GITHUB_SUPERCARTOFANTE }} - **Manually merged:** - [List files where you combined both - explain reasoning] + # ============================================================ + # POST-CLAUDE: Complete merge, create branch, push, create PR + # These steps are HARDCODED to ensure correct git operations + # ============================================================ - ### ✅ Testing Results + - name: Verify no conflict markers remain + id: verify-resolution + run: | + echo "::group::Verifying conflict resolution" - - [x] `make lint` - PASSED - - [x] `make lint-mypy` - PASSED - - [x] `make test-unit` - PASSED - - [ ] Manual review of Dockerfile CARTO customizations - - [ ] Manual review of Makefile CARTO sections + # Check for conflict markers in all files + if grep -r "^<<<<<<< \|^=======\$\|^>>>>>>>" --include="*.py" --include="*.yaml" --include="*.yml" --include="*.md" . 2>/dev/null | grep -v ".git"; then + echo "❌ ERROR: Conflict markers still present!" + echo "Claude did not fully resolve all conflicts." + echo "resolved=false" >> $GITHUB_OUTPUT + exit 1 + fi - ### 🔍 Review Guidelines + echo "✅ No conflict markers found" + echo "resolved=true" >> $GITHUB_OUTPUT + echo "::endgroup::" - **Please verify:** - 1. ✅ CARTO workflows intact (`.github/workflows/carto_*.yaml`) - 2. ✅ Dockerfile CARTO customizations preserved (`# CARTO:` comments) - 3. ✅ Makefile CARTO sections intact - 4. ✅ Core LiteLLM functionality improved with upstream changes - 5. ✅ All tests passing + - name: Complete merge commit (HARDCODED - creates merge with both parents) + id: complete-merge + run: | + echo "::group::Completing merge commit (WORKFLOW CONTROLLED)" - ### 📝 Notes + PR_NUMBER="${{ needs.check-eligibility.outputs.pr-number }}" - [Add any important notes about complex conflict resolutions here] + # Stage all resolved files + git add -A - ### 🔄 What Happens After Merge + # Complete the merge - this creates a MERGE COMMIT with BOTH parents: + # - Parent 1: main (upstream) + # - Parent 2: carto/main (CARTO customizations) + git commit -m "fix: resolve upstream sync conflicts for PR #${PR_NUMBER} - **When this PR is merged into `carto/main`:** - 1. ✅ All upstream changes from `main` will be in `carto/main` (with conflicts resolved) - 2. ✅ CARTO customizations will be preserved - 3. ❌ **Close PR #${{ needs.check-eligibility.outputs.pr-number }}** - it becomes redundant (changes already merged) - 4. 🎉 Upstream sync complete! + Conflicts resolved by Claude Code following CARTO priority rules. - **Branch flow:** - ``` - upstream → main (mirror) → carto/main (via this PR) - ``` + Resolution strategy: + - Preserved CARTO customizations (workflows, docs, infrastructure) + - Accepted upstream improvements (core litellm, tests, dependencies) + - Manually merged mixed files (Dockerfile, Makefile) - --- - 🤖 *Automated conflict resolution by Claude Code* - Resolves conflicts in #${{ needs.check-eligibility.outputs.pr-number }} - PRBODY - )" - ``` + This is a MERGE COMMIT with both main and carto/main as parents, + preserving full git history from upstream. - ## 🚨 CRITICAL REQUIREMENTS + Resolves: #${PR_NUMBER}" - 1. **ALL TESTS MUST PASS** - Do not create PR if tests fail - 2. **PRIORITIZE CARTO** - When unsure, preserve CARTO customizations - 3. **DOCUMENT REASONING** - Explain complex merge decisions in PR body - 4. **NO CONFLICT MARKERS** - Ensure all `<<<<<<<`, `=======`, `>>>>>>>` removed - 5. **VERIFY SYNTAX** - All files must be syntactically valid + # Verify we created a merge commit (should have 2 parents) + PARENT_COUNT=$(git cat-file -p HEAD | grep "^parent" | wc -l) + if [ "$PARENT_COUNT" -lt 2 ]; then + echo "⚠️ WARNING: Commit has only ${PARENT_COUNT} parent(s), expected 2 for merge commit" + else + echo "✅ Merge commit created with ${PARENT_COUNT} parents" + fi - ## Success Criteria Checklist + echo "merge-commit=$(git rev-parse HEAD)" >> $GITHUB_OUTPUT + echo "::endgroup::" - - [ ] All conflicts resolved (no conflict markers remain) - - [ ] `make lint` passes - - [ ] `make lint-mypy` passes - - [ ] `make test-unit` passes - - [ ] CARTO customizations preserved - - [ ] Core LiteLLM functionality improved - - [ ] PR created with comprehensive description - - [ ] Branch pushed successfully + - name: Push resolution to sync branch (HARDCODED - updates existing PR) + id: push-resolution + run: | + echo "::group::Pushing resolution to sync branch (WORKFLOW CONTROLLED)" - --- + HEAD_REF="${{ steps.pr-info.outputs.head-ref }}" + PR_NUMBER="${{ needs.check-eligibility.outputs.pr-number }}" - **Begin conflict resolution now!** + # Push directly to the sync branch - this updates the existing PR! + # No need to create a new branch or PR + echo "[Resolver] Pushing merge commit to ${HEAD_REF}..." + git push origin HEAD:${HEAD_REF} - env: - GH_TOKEN: ${{ secrets.X_GITHUB_SUPERCARTOFANTE }} - GITHUB_TOKEN: ${{ secrets.X_GITHUB_SUPERCARTOFANTE }} + echo "✅ Pushed resolution to: ${HEAD_REF}" + echo "✅ PR #${PR_NUMBER} will automatically update with the resolved changes" + echo "branch-name=${HEAD_REF}" >> $GITHUB_OUTPUT + echo "::endgroup::" - name: Summary if: always() + env: + GH_TOKEN: ${{ secrets.X_GITHUB_SUPERCARTOFANTE }} run: | - echo "::group::Claude Code Execution Summary" - echo "[Resolver] Claude Code conflict resolution completed" - echo "[Resolver] Check the action logs above for details" - echo "[Resolver] A new PR should have been created if successful" + echo "::group::Workflow Execution Summary" + echo "[Resolver] Conflict resolution workflow completed" + echo "[Resolver] Claude edited files, workflow pushed to existing PR" echo "::endgroup::" - cat >> $GITHUB_STEP_SUMMARY << 'EOF' - ## 🤖 Claude Code Conflict Resolution + # Get PR URL for the summary + PR_NUMBER="${{ needs.check-eligibility.outputs.pr-number }}" + PR_URL=$(gh pr view ${PR_NUMBER} --repo ${{ github.repository }} --json url --jq '.url') - Claude Code has attempted to resolve conflicts in PR #${{ needs.check-eligibility.outputs.pr-number }}. + cat >> $GITHUB_STEP_SUMMARY << EOF + ## 🤖 Conflict Resolution Summary - **Configuration:** - - Model: Sonnet 4.5 - - Max turns: 250 (very generous - timeout is real limit) - - Job timeout: 90 minutes (workflow-level) - - Tools: Read, Write, Edit, Bash, Grep, Glob (restricted) - - Estimated cost: Limited by 90-min timeout (~$3-10 max) + **PR Updated:** ${PR_URL} + **Sync Branch:** ${{ steps.push-resolution.outputs.branch-name || 'N/A' }} - **Resolution Priorities Applied:** - 1. 🏢 CARTO customizations (highest priority) - 2. 🔧 LiteLLM core functionality improvements - 3. ⚖️ Manual merge when both are important + ### Workflow Architecture (Single-PR, Deterministic) - **Testing Requirements:** - - ✅ All linting and type checks must pass - - ✅ All unit tests must pass - - ✅ CARTO customizations must be preserved + | Step | Handled By | Status | + |------|------------|--------| + | Start merge | Workflow (hardcoded) | ✅ | + | Edit conflicted files | Claude Code | ${{ steps.claude-run.outcome == 'success' && '✅' || '❌' }} | + | Verify no markers | Workflow (hardcoded) | ${{ steps.verify-resolution.outcome == 'success' && '✅' || '❌' }} | + | Create merge commit | Workflow (hardcoded) | ${{ steps.complete-merge.outcome == 'success' && '✅' || '❌' }} | + | Push to sync branch | Workflow (hardcoded) | ${{ steps.push-resolution.outcome == 'success' && '✅' || '❌' }} | - **Next Steps:** - 1. Review the PR created by Claude Code - 2. Verify conflict resolutions follow priorities - 3. Check test results in the PR - 4. Merge the resolution PR to update the sync PR - - **Review Checklist:** - - [ ] CARTO workflows preserved - - [ ] Dockerfile CARTO customizations intact (`# CARTO:` comments) - - [ ] Makefile CARTO sections preserved - - [ ] Core LiteLLM improvements accepted - - [ ] All tests passing (`make lint`, `make lint-mypy`, `make test-unit`) + ### Key Improvements + + 1. **Single-PR workflow** - No separate resolution PR, changes pushed directly to sync branch + 2. **Deterministic** - Git operations are hardcoded in workflow, not in Claude's prompt + 3. **Merge commit preserved** - Both parents (upstream + carto/main) for proper history + + **Merge commit:** \`${{ steps.complete-merge.outputs.merge-commit || 'N/A' }}\` EOF - - name: Comment on original PR - Completion + - name: Comment on PR - Completion if: always() env: GH_TOKEN: ${{ secrets.X_GITHUB_SUPERCARTOFANTE }} run: | - # Determine if Claude succeeded or failed - if [ "${{ steps.claude-run.outcome }}" == "success" ]; then - # Try to find the resolution PR that Claude created - RESOLUTION_PR=$(gh pr list \ + PR_NUMBER="${{ needs.check-eligibility.outputs.pr-number }}" + PUSH_SUCCESS="${{ steps.push-resolution.outcome }}" + + if [ "${PUSH_SUCCESS}" == "success" ]; then + # Success - resolution pushed to sync branch + gh pr comment ${PR_NUMBER} \ --repo ${{ github.repository }} \ - --head upstream-sync-resolver/${{ needs.check-eligibility.outputs.pr-number }} \ - --json number,url \ - --jq '.[0].url' || echo "") - - if [ -n "${RESOLUTION_PR}" ]; then - # Success with PR created - gh pr comment ${{ needs.check-eligibility.outputs.pr-number }} \ - --repo ${{ github.repository }} \ - --body "$(cat < ⚠️ **IMPORTANT:** Use "Create a merge commit" - do NOT squash or rebase! **Workflow Run:** ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} EOF )" - fi else - # Failure - gh pr comment ${{ needs.check-eligibility.outputs.pr-number }} \ + # Failure - determine which step failed + gh pr comment ${PR_NUMBER} \ --repo ${{ github.repository }} \ --body "$(cat </dev/null || echo "") + --json url,headRefName \ + --jq '.[] | select(.headRefName | startswith("upstream-sync/")) | .url' 2>/dev/null | head -1 || echo "") if [[ -n "${PR_URL}" ]]; then echo "[Sync] PR already exists: ${PR_URL}" @@ -287,12 +287,42 @@ jobs: if: needs.check-carto-main-sync.outputs.needs-sync == 'true' && needs.check-carto-main-sync.outputs.pr-exists == 'false' outputs: pr-url: ${{ steps.create-pr.outputs.pr-url }} + sync-branch: ${{ steps.create-sync-branch.outputs.branch-name }} steps: - name: Checkout main branch uses: actions/checkout@v4 with: ref: main fetch-depth: 0 + token: ${{ secrets.X_GITHUB_SUPERCARTOFANTE }} + + - name: Configure Git + run: | + git config --global user.email "cartofante@carto.com" + git config --global user.name "Cartofante" + + - name: Create sync branch from main + id: create-sync-branch + env: + NEW_VERSION: ${{ needs.check-new-release.outputs.new-version }} + run: | + set -eu + + echo "::group::Creating sync branch from main" + + # Create a dedicated sync branch that we can push conflict resolutions to + # Format: upstream-sync/vX.Y.Z-stable (e.g., upstream-sync/v1.79.3-stable) + BRANCH_NAME="upstream-sync/${NEW_VERSION}" + + echo "[Sync] Creating branch: ${BRANCH_NAME}" + git checkout -b "${BRANCH_NAME}" + + # Push the branch to origin + git push -u origin "${BRANCH_NAME}" + + echo "branch-name=${BRANCH_NAME}" >> $GITHUB_OUTPUT + echo "[Sync] ✅ Created and pushed branch: ${BRANCH_NAME}" + echo "::endgroup::" - name: Generate PR body id: generate-pr-body @@ -333,7 +363,10 @@ jobs: ### 🔀 Branch Flow 1. ✅ \`BerriAI/litellm:main\` merged into \`CartoDB/litellm:main\` - 2. 📝 This PR: \`CartoDB/litellm:main\` → \`CartoDB/litellm:carto/main\` + 2. ✅ Created dedicated sync branch: \`upstream-sync/${NEW_VERSION}\` + 3. 📝 This PR: \`upstream-sync/${NEW_VERSION}\` → \`carto/main\` + + > **Why a dedicated branch?** Allows pushing conflict resolution commits directly to this PR instead of creating separate PRs. ### 📝 CARTO-Specific File Guidelines @@ -382,27 +415,27 @@ jobs: **It will:** 1. 🤖 Detect conflicts in this PR - 2. 🔀 Checkout \`carto/main\` and merge \`main\` into it + 2. 🔀 Checkout the sync branch and merge \`carto/main\` into it 3. ✏️ Resolve conflicts following CARTO priority rules 4. ✅ Run all tests (\`make lint\`, \`make lint-mypy\`, \`make test-unit\`) - 5. 📝 Create a resolution PR: \`upstream-sync-resolver/ → carto/main\` + 5. 📌 Push resolution commits directly to this PR's branch **What you need to do:** - 1. Review the resolution PR created by Claude Code + 1. Wait for this PR to update with conflict resolution commits 2. Verify CARTO customizations are preserved - 3. Merge the resolution PR into \`carto/main\` - 4. **Close this PR** (changes already merged via resolution PR) + 3. Merge this PR into \`carto/main\` + + > ✨ **Single PR workflow!** No separate resolution PR - everything is in this PR. #### Option 2: Manual Resolution If you prefer manual resolution or Claude fails: - 1. **Checkout and merge locally:** + 1. **Checkout the sync branch and merge carto/main:** \`\`\`bash git fetch origin - git checkout carto/main - git pull origin carto/main - git merge main # Creates conflicts + git checkout upstream-sync/${NEW_VERSION} + git merge origin/carto/main # Creates conflicts \`\`\` 2. **Resolve conflicts following guidelines above** @@ -414,12 +447,12 @@ jobs: make test-unit \`\`\` - 4. **Push resolved changes:** + 4. **Push resolved changes to the sync branch:** \`\`\`bash - git push origin carto/main + git push origin upstream-sync/${NEW_VERSION} \`\`\` - 5. **Close this PR** (changes already in carto/main) + 5. **Merge this PR** (this PR will update automatically) --- @@ -433,6 +466,7 @@ jobs: id: create-pr env: NEW_VERSION: ${{ needs.check-new-release.outputs.new-version }} + SYNC_BRANCH: ${{ steps.create-sync-branch.outputs.branch-name }} GH_TOKEN: ${{ secrets.X_GITHUB_SUPERCARTOFANTE }} run: | set -eu @@ -441,11 +475,12 @@ jobs: PR_BODY_FILE="sync-pr-body.md" - # Create PR from main to carto/main + # Create PR from sync branch to carto/main + # Using dedicated branch (not main) allows pushing conflict resolution commits PR_URL=$(gh pr create \ --repo CartoDB/litellm \ --base carto/main \ - --head main \ + --head "${SYNC_BRANCH}" \ --title "🔄 sync: upstream ${NEW_VERSION}" \ --body-file ${PR_BODY_FILE} \ --label "upstream-sync" \ diff --git a/CARTO_CLAUDE.md b/CARTO_CLAUDE.md index 463b12596a3..76d28dcc450 100644 --- a/CARTO_CLAUDE.md +++ b/CARTO_CLAUDE.md @@ -37,16 +37,22 @@ CARTO maintains a fork of [BerriAI/litellm](https://github.com/BerriAI/litellm) --- +## When creating PRs + +- Always use the --repo CartoDB/litellm by default. Only use Upstream repo to PR if explicify written in the prompt. +- Always create in Draft. +- Follow the claude.md global configuration. + ## Branch Strategy ``` upstream/main → BerriAI's development branch (may be unstable) ↓ - main → Mirrors upstream/main (tracking/reference only) - -upstream/v1.75.2 → Stable upstream release tag + main → Mirrors upstream/main (sync happens here first) ↓ - carto/main → Production branch (stable tag + CARTO mods) +upstream-sync/vX.Y.Z → Dedicated sync branch for PR to carto/main + ↓ + carto/main → Production branch (stable + CARTO mods) ↓ feature/* → Development branches ``` @@ -55,15 +61,30 @@ upstream/v1.75.2 → Stable upstream release tag | Branch | Purpose | Base | Stability | |--------|---------|------|-----------| -| `main` | Track upstream development | `upstream/main` | Unstable (reference only) | +| `main` | Mirror upstream (sync target) | `upstream/main` | Unstable (reference only) | +| `upstream-sync/*` | Sync PRs to carto/main | `main` | Temporary (for PRs) | | `carto/main` | CARTO production deployments | Stable upstream tags | Stable | | `feature/*` | Development work | `carto/main` | Development | ### Critical Rules -⚠️ **NEVER merge `main` into `carto/main`** - `main` may contain unstable upstream commits +⚠️ **NEVER merge `main` directly into `carto/main`** - Use sync workflow instead + +✅ **ALWAYS use "Create a merge commit"** when merging sync PRs (never squash/rebase!) + +✅ **ALWAYS merge stable upstream release tags** (e.g., `v1.76.5`) via the sync workflow -✅ **ALWAYS merge stable upstream release tags** (e.g., `v1.76.5`) into `carto/main` +### ⛔ Merge Instructions for Upstream Sync PRs + +When merging any `upstream-sync/*` PR into `carto/main`: + +| Merge Type | Result | +|------------|--------| +| ✅ **Create a merge commit** | Preserves upstream history, future syncs work correctly | +| ❌ Squash and merge | **DESTROYS** upstream history, breaks future syncs | +| ❌ Rebase and merge | **DESTROYS** upstream history, breaks future syncs | + +**Why this matters:** Squash/rebase rewrites commit SHAs, causing future syncs to show thousands of "new" commits that were already merged. This happened with PR #26 - don't repeat it! --- @@ -266,9 +287,38 @@ CARTO uses an automated workflow to sync with upstream LiteLLM stable releases. 🔄 **Automated Sync:** CARTO runs an automated workflow that detects new upstream stable releases and creates sync PRs for team review. +#### Architecture: Single-PR Workflow + +``` +BerriAI/litellm:main + │ + │ (1) Sync upstream to main + ▼ +CartoDB/litellm:main ←── upstream mirror + │ + │ (2) Create dedicated sync branch + ▼ +upstream-sync/vX.Y.Z ←── pushable branch for PR + │ + │ (3) Single PR to carto/main + ▼ │ +CartoDB/litellm:carto/main │ + ▲ │ + │ (4) If conflicts: │ + │ Claude resolves and pushes + │ to SAME branch (PR auto-updates) + └────────────────────┘ +``` + +**Key benefits:** +- ✅ Single PR for entire sync (no separate resolution PR) +- ✅ Pushable branch allows conflict resolution commits +- ✅ Deterministic git operations (hardcoded in workflow) +- ✅ Merge commit preserves full upstream history + #### How It Works -Every 8 hours, the `carto-upstream-sync.yml` workflow: +Every Monday at noon, the `carto-upstream-sync.yml` workflow: 1. **Detects** new upstream stable releases (e.g., `v1.78.5-stable`) - Uses `gh CLI` to fetch releases from BerriAI/litellm @@ -280,32 +330,59 @@ Every 8 hours, the `carto-upstream-sync.yml` workflow: - Merges `BerriAI/litellm:main` → `CartoDB/litellm:main` - Pushes updated main branch automatically -3. **Creates PR** from main to carto/main - - PR: `CartoDB/litellm:main` → `CartoDB/litellm:carto/main` +3. **Creates dedicated sync branch** from main + - Branch: `upstream-sync/vX.Y.Z-stable` (e.g., `upstream-sync/v1.79.3-stable`) + - This branch is pushable (unlike `main` which is read-only for CARTO commits) + +4. **Creates PR** from sync branch to carto/main + - PR: `upstream-sync/vX.Y.Z` → `carto/main` - Detects if conflicts exist - - Creates detailed PR with resolution guidelines - Labels PR appropriately (`upstream-sync`, `automated`) + - If conflicts detected, triggers resolver workflow -4. **Provides comprehensive PR** with: +5. **Provides comprehensive PR** with: - Link to upstream changes and release notes - Branch flow diagram - Summary of commits and files changed - Detailed conflict resolution guidelines - Testing checklist - - Step-by-step resolution instructions -#### Conflict Handling Strategy +#### Automated Conflict Resolution + +When conflicts are detected, the `carto-upstream-sync-resolver.yml` workflow: -The workflow **detects but does not automatically resolve** conflicts. This ensures: -- ✅ No silent breaking changes -- ✅ Human review of important conflicts -- ✅ Clear documentation of what needs resolution -- ✅ Safe, conservative approach +1. **Checkouts** the sync branch (`upstream-sync/vX.Y.Z`) +2. **Merges** `carto/main` into it (creates conflict state) +3. **Claude Code** edits files to resolve conflicts (following CARTO priority rules) +4. **Workflow** creates merge commit with both parents (preserves history) +5. **Pushes** directly to the sync branch (PR auto-updates!) -When conflicts are detected, the PR includes detailed guidelines on which files to: -- **Keep CARTO versions:** `carto_*.yaml`, `CARTO_*.md` -- **Accept upstream:** Core `litellm/` code, `tests/` -- **Manually review:** `Dockerfile`, `Makefile` (check `# CARTO:` comments) +**Resolution priorities:** +| File Pattern | Keep | Reason | +|--------------|------|--------| +| `carto_*.yaml`, `carto-*.yml` | CARTO version | CARTO workflows | +| `CARTO_*.md` | CARTO version | CARTO docs | +| `litellm/**` (core) | Upstream | Upstream improvements | +| `tests/**` | Upstream | Upstream tests | +| `Dockerfile`, `Makefile` | Merge carefully | Keep `# CARTO:` sections | + +#### Why Single-PR Workflow? + +**Before (2 PRs, confusing):** +``` +main → PR #1 to carto/main (conflicts!) + └── Can't push to main (read-only) + └── PR #2: resolver/XX → carto/main (separate PR) + └── Must close PR #1 after merging PR #2 +``` + +**After (1 PR, clean):** +``` +main → upstream-sync/vX.Y.Z (dedicated, pushable) + └── Single PR to carto/main + └── Conflicts? Push resolution to same branch + └── PR auto-updates, merge when ready ✅ +``` #### Setup @@ -455,7 +532,8 @@ These modifications exist in `carto/main` but NOT in upstream. Be careful to pre ### 1. Custom GitHub Workflows **Added:** -- `.github/workflows/carto-upstream-sync.yml` - Automated upstream sync (runs every 8 hours, all bash) +- `.github/workflows/carto-upstream-sync.yml` - Automated upstream sync (runs weekly, all bash) +- `.github/workflows/carto-upstream-sync-resolver.yml` - Claude Code conflict resolution (single-PR workflow) - `.github/workflows/carto_ghcr_deploy.yaml` - CI/CD for CARTO Docker images - `.github/workflows/carto_release.yaml` - Automated release creation @@ -746,29 +824,42 @@ gh workflow run carto-upstream-sync.yml #### Issue: PR Created with Conflicts -**Symptoms:** Sync PR is labeled with `conflicts`. +**Symptoms:** Sync PR shows merge conflicts. **Solution:** -This is expected when upstream changes conflict with CARTO modifications. Follow the resolution guide in the PR: +With the single-PR workflow, conflicts are resolved automatically by Claude Code: -1. Checkout the PR branch: - ```bash - gh pr checkout - ``` -2. Review conflicts using the guidelines in the PR body -3. Resolve conflicts: - ```bash - # Fix the conflicts - git add - git commit -m "resolve: conflicts from upstream sync" - git push - ``` -4. Run tests to verify: - ```bash - make lint - make test-unit - ``` +1. **Wait for resolver workflow** - It triggers automatically when conflicts are detected +2. **Check PR for updates** - Resolution commits are pushed directly to the sync branch +3. **Review the resolution** - Verify CARTO customizations are preserved +4. **Merge using "Create a merge commit"** - Never squash or rebase! + +**If automatic resolution fails**, resolve manually: + +```bash +# Checkout the sync branch +gh pr checkout + +# The branch is already in conflict state, or merge carto/main: +git merge origin/carto/main + +# Resolve conflicts following priority rules: +# - CARTO files (carto_*, CARTO_*): Keep CARTO version +# - Core litellm/: Keep upstream version +# - Dockerfile, Makefile: Merge carefully (keep # CARTO: sections) + +# After resolving: +git add +git commit -m "fix: resolve upstream sync conflicts" +git push + +# Run tests +make lint +make test-unit +``` + +**Important:** The PR will auto-update when you push to the sync branch. #### Issue: Slack Notifications Not Received @@ -974,6 +1065,6 @@ For questions about this fork: --- -**Last Updated:** 2025-11-12 +**Last Updated:** 2025-12-03 **Maintained By:** CARTO Engineering Team **For:** AI Assistants & Developers working on CARTO's LiteLLM fork