Promote dev to main: getCheckSuiteStatus dedupes re-run workflows#1255
Merged
zbigniewsobiecki merged 1 commit intomainfrom May 2, 2026
Merged
Promote dev to main: getCheckSuiteStatus dedupes re-run workflows#1255zbigniewsobiecki merged 1 commit intomainfrom
zbigniewsobiecki merged 1 commit intomainfrom
Conversation
ucho/PR #231 had its CI fully green at the latest attempt but cascade dispatched respond-to-ci anyway, with triggerType=check-failure. Wasteful agent run; respond-to-ci is the agent that fixes failing CI, so running it on a green PR confuses the loop semantics. Root cause: each CI workflow on the PR's head_sha ran TWICE on the same SHA (push-then-rerun, or two pushes that resolved to the same SHA). The first attempt of `Rebuild ucho-cli template` FAILED at 19:44:40; the second attempt SUCCEEDED at 19:45:28. GitHub's `listWorkflowRunsForRepo` returns BOTH workflow_runs. cascade's `getCheckSuiteStatus` iterated both and concatenated their jobs — including the stale FAILURE record. `check-suite-success.handle` (the fork from PR #1241/#1243) computes `anyFailed = checkRuns.some(cr => cr.conclusion === 'failure' || ...)`. With the stale failure in the list, `anyFailed=true` even though the PR is green at the latest attempt. The handler mistakenly forks to `dispatchRespondToCi(...)`. The triggerType=check-failure in the run record confirms it came through this fork. GitHub's `listJobsForWorkflowRun` accepts `filter='latest'` (default) which dedupes job ATTEMPTS within a single workflow_run (the "Re-run failed jobs" case). It does NOT dedupe across multiple workflow_runs of the same workflow on the same SHA — which is what bit us. Fix: dedupe `workflowRuns` by `workflow_id` BEFORE fetching jobs. GitHub returns runs sorted by `created_at` desc, so the first occurrence per workflow_id is the latest. Three-line addition in `getCheckSuiteStatus` at the GitHub-client layer — every caller (`check-suite-success`, `check-suite-failure`, anywhere else that asks "current state of CI?") benefits without changing. Why at the client layer: - Source of truth match: GitHub's PR UI uses the latest attempt's status; cascade should match that. - Centralization: the same dedup bug would otherwise surface separately per caller. Closing it once at the boundary eliminates the class. Edge cases handled: - Same `workflow_id` across different events (push vs pull_request) — keeps the most recent regardless of event. - Different `workflow_id`s (e.g. CI + CodeQL) — both kept; new test pins this so over-aggressive dedup can't ship. - No workflow runs — Map yields empty list, downstream code already handles `checkRuns.length === 0`. Tests: - New `dedupes workflow runs by workflow_id, keeping only the latest re-run` test directly pins the PR #231 incident: 2 workflow_runs same workflow_id, first failed second succeeded — assert allPassing=true and only 1 check_run returned. Also asserts `listJobsForWorkflowRun` is NOT called for the older run (saves API quota and proves the dedup actually skipped the stale jobs). - New `keeps separate workflow_ids distinct (CI vs CodeQL on same SHA)` guard against over-aggressive dedup. - Updated `mockWorkflowRuns` helper to default `workflow_id` to the run id so existing tests (which don't care about workflow_id) keep semantically matching. - Updated two pagination tests to include `workflow_id` so the dedup doesn't collapse multiple-runs-with-undefined-workflow_id to one. Out of scope (follow-up): - Audit `getFailedWorkflowRunJobs` (used by respond-to-ci agent) — different semantics (it WANTS to show what failed even if subsequently succeeded). - Eventually-consistent GitHub API — the workflow-runs-list endpoint may take a few seconds to reflect a new attempt. Orthogonal to dedup. Verification: vitest 7687/7687, typecheck clean, lint clean. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Promotes 1 commit from `dev`:
What
Live regression fix: PR #231's CI was green at the latest attempt but cascade dispatched `respond-to-ci` because `getCheckSuiteStatus` was double-counting workflow re-runs. Dedupes `workflowRuns` by `workflow_id` (keeping the most recent per workflow) before fetching jobs.
Reviewed and approved on #1254 by @nhopeatall. CI green on dev.
Risk
Low. 3-line addition at the github-client boundary; no caller changes. New tests pin the regression and the over-aggressive-dedup edge case.
🤖 Generated with Claude Code