fix(ci): restore agent reaction to test failures via CARTO Feature Tests - #125
Merged
Conversation
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.
3 tasks
…e 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.
mateo-di
marked this pull request as ready for review
August 4, 2026 12:23
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
Restores the upstream-sync automation's ability to react to failing tests. The fixer (
carto-upstream-sync-ci-fixer.yml) and ready-checker (carto-upstream-sync-ready-checker.yml) were wired to react toLiteLLM Mock TestsandLiteLLM Lintingworkflow_runcompletions. Both are dead oncarto/main: Mock Tests is upstream-deprecated (workflow_dispatch-only) and Linting only triggers for upstream's own branches. The consequence: no test ran on acarto/mainsync PR, the fixer never fired, and the only failure signal was cloud-native integration tests three repos downstream, which is exactly how the v1.92.0 sync shipped three broken CARTO wirings.Changes
Both workflows now react to
CARTO Feature Tests(carto-feature-tests.yml, CARTO's own unit-test gate from #124):workflow_run.workflows->["CARTO - Deploy Docker Image (CI)", "CARTO Feature Tests"].workflow_runonly re-invokes ready-checker when a listed workflow completes, so listing Feature Tests is what makessync-readywait for the tests, not just the Docker build.CARTO Feature Tests; the Claude prompt's local verification now reproduces the gate's real command (make install-test-deps+ uv + manifest-derived scope) rather than the stalepip install -e ".[dev]"/pytest tests/test_litellm/.check-ci-statuswas audited and is workflow-name-agnostic (keys off branch + labels), so no change there.Ordering
CARTO Feature Testsworkflow from fix(ci): gate carto/main PRs on CARTO feature tests and harden the sync resolver #124; the trigger references it by name and is harmless until fix(ci): gate carto/main PRs on CARTO feature tests and harden the sync resolver #124 merges (workflow_runsimply won't match a non-existent workflow).Test plan
run:blocks passbash -nCARTO Feature Testsrun triggers the fixer and thatsync-readyonly lands once both Docker CI and Feature Tests are green (end-to-end verification intentionally deferred per plan)