fix(ci): stop gating upstream-sync CI on checks that never run - #123
Closed
mateo-di wants to merge 1 commit into
Closed
fix(ci): stop gating upstream-sync CI on checks that never run#123mateo-di wants to merge 1 commit into
mateo-di wants to merge 1 commit into
Conversation
carto-upstream-sync-ready-checker.yml and carto-upstream-sync-ci-fixer.yml both listened for "LiteLLM Mock Tests (folder - tests/test_litellm)" and "LiteLLM Linting" alongside CARTO's own Docker CI. Neither of the first two can ever complete for these PRs: - "LiteLLM Mock Tests" is upstream-deprecated - its pull_request trigger is commented out, workflow_dispatch only (see test-litellm.yml). - "LiteLLM Linting" only triggers for PRs against upstream's own branches (main, litellm_internal_staging, litellm_oss_branch, litellm_**) - never carto/main, so it never runs against upstream-sync/* PRs either. Only "CARTO - Deploy Docker Image (CI)" actually exercises these PRs, so that's the only workflow_run trigger they need.
3 tasks
mateo-di
added a commit
that referenced
this pull request
Aug 4, 2026
…sts (#125) * fix(ci): restore agent reaction to test failures via CARTO Feature Tests The upstream-sync fixer and ready-checker were wired to react to "LiteLLM Mock Tests" and "LiteLLM Linting" workflow_run completions. Both are dead on carto/main: Mock Tests is upstream-deprecated (workflow_dispatch only) and Linting only triggers for upstream's own branches. So no test ever ran on a carto/main sync PR, the fixer never fired, and the only failure signal was cloud-native integration tests three repos downstream (see the v1.92.0 sync, where three broken CARTO wirings shipped that way). Wire both workflows to "CARTO Feature Tests" (carto-feature-tests.yml, CARTO's own unit-test gate) instead: - ready-checker: replace the two dead names with Docker CI + Feature Tests. workflow_run only re-invokes ready-checker when a listed workflow completes, so listing Feature Tests is what makes sync-ready wait for the tests rather than just the build. - ci-fixer: same trigger list; swap the log-extraction query from the dead Mock Tests workflow to Feature Tests; align the prompt's local verification with the gate's real command (uv + make install-test-deps + manifest-derived scope) instead of the stale pip/pytest invocation. Supersedes #123 (which only removed the dead names). Depends on the CARTO Feature Tests workflow from #124. * docs(ci): align sync resolver/fixer/analyzer prompts with wiring-aware reasoning (#126) The v1.92.0 sync shipped three CARTO features that passed every automated check yet were functionally broken (dropped call site across an auto-merged file, an orphaned helper with no caller, and a store/lookup key that mismatched after upstream changed id encoding). None were catchable by the prompts' existing "does the pattern still grep" verification, and two of the prompts' heuristics actively caused the loss. This aligns the prompts with the reasoning that actually found and fixed those bugs. Resolver prompt: - Verbatim-first rule: restore CARTO blocks byte-identical from carto/main (diff-verified); adapt only where an upstream API change makes verbatim impossible, minimally and marked # CARTO PATCH. Replaces "preserve the BEHAVIOR, not necessarily the exact file versions", which licensed the paraphrase that dropped a session read-path. - Remove harmful heuristics: delete "bigger file = probably correct" and reframe "upstream TAG code WORKS" as "works for upstream's call graph, not necessarily CARTO's - re-verify callers, attributes, data formats". - Add three post-resolution checks the grep cannot do: orphan sweep (helper with no caller), cross-file wiring (auto-merged siblings), and cross-version data-flow (format drift across the version boundary). - Reference the regression canaries as the definition of "wired correctly". Fixer prompt: - Same verbatim-first rule, orphan sweep, and call-graph reframing. - Manifest-aware loop guard: the "3+ fixes -> sync entire file from upstream" escape hatch now excludes manifest files; for those, take upstream as base and re-apply the CARTO block verbatim (blind sync is how wirings get erased). - Log extraction / verification already retargeted to CARTO Feature Tests in the agent-reaction PR this is stacked on. Analyzer prompt: - Judge PRESERVED_CARTO by WIRING, not string presence: an orphaned helper, a missing call site, or broken data-flow is INCORRECTLY_DROPPED even when the def/pattern greps OK. It had reported the dead session read as PASS. CARTO_UPSTREAM_SYNC.md: - New troubleshooting section documenting the three v1.92.0 wiring-loss classes with detection commands. Manifest wiring-pattern enrichment (adding call-site patterns to carto-features.yml) is deliberately deferred to a follow-up to avoid clobbering the manifest changes on the open v1.92.0 sync PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
carto-upstream-sync-ready-checker.ymlandcarto-upstream-sync-ci-fixer.ymlboth listen for"LiteLLM Mock Tests (folder - tests/test_litellm)"and"LiteLLM Linting"alongside CARTO's own Docker CI. Neither of the first two can ever complete for these PRs:"LiteLLM Mock Tests"is upstream-deprecated — itspull_requesttrigger is commented out intest-litellm.yml,workflow_dispatch-only now (manual debugging only)."LiteLLM Linting"only triggers for PRs against upstream's own branches (main,litellm_internal_staging,litellm_oss_branch,litellm_**) —carto/mainis never in that list, so it never runs againstupstream-sync/*PRs either.Only
"CARTO - Deploy Docker Image (CI)"actually exercises these PRs. Dropped the two dead names from both workflows'workflow_runtriggers.Split out from #122 (which is scoped to the separate n8n-notifier image-case bug) to keep review focused.
Verification
Confirmed via
gh run list --branch upstream-sync/v1.92.0on PR #121 that neither dead workflow has ever run against a real sync PR, while the Docker CI workflow completes normally.Test plan
python3 -c "import yaml; yaml.safe_load(...)")sync-readyadded automatically once Docker CI passes