Skip to content

fix(github): dedupe re-run check-runs before CI classification - #3923

Merged
JSONbored merged 1 commit into
mainfrom
fix/dedupe-rerun-check-runs-live-ci
Jul 7, 2026
Merged

fix(github): dedupe re-run check-runs before CI classification#3923
JSONbored merged 1 commit into
mainfrom
fix/dedupe-rerun-check-runs-live-ci

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • reduceLiveCiAggregate (the shared classification the gate's live-CI aggregate uses for both the REST path and the flag-gated GraphQL path) had no deduplication-by-check-run-name step. GitHub's check-runs API can return multiple entries with the same name when a job is re-run (e.g. "Re-run failed jobs" after a flake) — the stale run is left in the list, not replaced. The classification loop pushed every failing conclusion into failingDetails unconditionally, so a stale failing duplicate resolved ciState to "failed" even though the same-named check currently passes.
  • Reproduced against a real commit in this repo: 7d145f032eb3b03b5ac5868aa3cecf3e002bb6e2 has a "Deploy UI preview version" check-run appearing twice — one with conclusion: "failure" from the original run, one with conclusion: "skipped" from the re-run. That stale "failed" ciState flows into planAgentMaintenanceActions as a terminal, immediate-close-driving signal for contributor PRs (it bypasses the pending-CI wait entirely), so a contributor whose CI transiently failed and was legitimately re-run to green could still get auto-closed on the stale failure.
  • Fix: added dedupeLatestCheckRunsByName, called at the top of reduceLiveCiAggregate's check-run loop, which collapses same-named check-runs down to the one with the latest started_at before any classification happens. It lives in the one shared reducer, so both fetchLiveCiAggregate (REST) and fetchLiveCiAggregateViaGraphQl get the fix from a single place.
  • Tiebreak field: I used started_at, not check-run id. The shared LiveCiCheckRun shape the GraphQL path populates has no id (GraphQL CheckRun nodes don't expose one), so id isn't usable in a fix that has to live in the shared reducer. started_at is available on both the REST payload and (once added to the GraphQL query/mapping, which this PR also does) the GraphQL side, so it's the one recency signal both paths can supply. When neither duplicate has a started_at (e.g. both still queued), the fallback is array order — GitHub does not document an ordering guarantee for /check-runs, so this is a last-resort tiebreak, not an assumption the classifier depends on.
  • I checked whether classic commit-statuses (statuses) have the same bug before deciding to leave them alone: GitHub's Combined Status API (/commits/{ref}/status) is documented to already return one entry per unique context, using the most recent status for that context — unlike /check-runs, which does not dedupe re-runs. I did not find any evidence in this codebase (existing comments, tests, or the statuses accumulation code) that this documented behavior doesn't hold, so statuses is intentionally untouched.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves — no issue filed; this was found and reproduced directly against a real commit while auditing the live-CI aggregate path, and is a self-contained bug fix with its own regression tests.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally (full unsharded run) — src/github/backfill.ts: 98.97% stmts / 97.2% branch / 96.81% funcs / 99.81% lines; every new line and branch in dedupeLatestCheckRunsByName and the GraphQL started_at plumbing shows non-zero hits in coverage/lcov.info (verified by grepping the exact added line numbers).
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate — 0 vulnerabilities
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries — see the new dedup describe block in test/unit/backfill.test.ts and the matching GraphQL-path tests + REST-equivalence scenario in test/unit/graphql-status-rollup.test.ts.

Ran the full local gate via npm run test:ci, which chains all of the above plus migration/schema-drift/self-host-env-reference checks, docs:drift-check, command-reference:check, and ui:test/ui:build — exit code 0.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. — N/A, this PR touches CI-status classification only, not auth/session/CORS.
  • API/OpenAPI/MCP behavior is updated and tested where needed. — No public API/OpenAPI/MCP surface changed; this is internal classification logic.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — N/A, no UI changes.
  • Visible UI changes include a UI Evidence section. — N/A, backend-only change, no visible UI/frontend/docs changes.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. — No changelog edit.

Notes

  • The fix is deliberately scoped to check-runs only; classic commit-statuses were checked and intentionally left unchanged (see Summary).
  • started_at is now also read/mapped on the GraphQL rollup path (fetchLiveCiAggregateViaGraphQl's query and node mapping) purely so the shared dedup tiebreak has the same signal available on both paths — no other GraphQL behavior changes.

reduceLiveCiAggregate had no deduplication-by-name step. GitHub's
check-runs API (both the REST /check-runs endpoint and the GraphQL
statusCheckRollup) can return multiple entries with the same `name`
after a job is re-run (e.g. "Re-run failed jobs" following a flake):
the stale run is left in the list rather than replaced. The
classification loop unconditionally pushed every failing conclusion
into failingDetails with no removal step, so a stale failing entry
resolved ciState to "failed" even when the same-named check currently
passes.

This was reproduced against a real commit in this repo: a
"Deploy UI preview version" check-run appears twice on
7d145f0 (conclusion "failure" from
the original run, conclusion "skipped" from the re-run). That
stale-failed ciState flows into planAgentMaintenanceActions as a
terminal, immediate-close-driving signal for contributor PRs, bypassing
the pending-CI wait entirely — so a contributor whose CI transiently
failed and was legitimately re-run to green could still get
auto-closed on the stale failure.

Fix: add dedupeLatestCheckRunsByName, which collapses same-named
check-runs to the one with the latest `started_at` before
classification. It lives inside reduceLiveCiAggregate so both the REST
path (fetchLiveCiAggregate) and the GraphQL path
(fetchLiveCiAggregateViaGraphQl) get it from one place. `started_at`
is used as the tiebreaker rather than check-run `id`, because the
shared LiveCiCheckRun shape the GraphQL path populates has no `id`
field (GraphQL check-run nodes don't expose one) — `started_at` is the
one recency signal available on both REST and GraphQL, so the GraphQL
query and mapping now also carry it through. When neither duplicate
has a started_at (e.g. both still queued), array order is the
fallback tiebreak, since GitHub does not document an ordering
guarantee for /check-runs.

Classic commit-statuses (the `statuses` array) are intentionally left
alone: GitHub's Combined Status API is documented to already return
one entry per unique context (the latest), so this duplicate-name
failure mode does not apply there.

Added regression tests covering: the stale-failure/fresh-pass case,
the opposite case where the latest duplicate is the one that fails
(proving the fix is recency-aware and not just duplicate-blind), an
out-of-order duplicate list, the no-timestamp fallback, and the same
scenarios through the GraphQL rollup path and its REST-equivalence
suite.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 7, 2026
@loopover-orb

loopover-orb Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-07 06:59:34 UTC

3 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This PR adds `dedupeLatestCheckRunsByName` to `reduceLiveCiAggregate` in src/github/backfill.ts, collapsing same-named check-runs (as GitHub produces on a job re-run) down to the one with the latest `started_at` before classification, and threads `started_at`/`startedAt` through both the REST payload type and the GraphQL query/mapping so both paths share the fix. The described bug is real and well-reproduced (a genuine commit where 'Deploy UI preview version' appears twice with differing conclusions), and the fix is applied at the correct shared layer so both fetch paths benefit. Test coverage is thorough — passing/failing/out-of-order/no-timestamp cases are all exercised on both the REST and GraphQL equivalence suites.

Nits — 5 non-blocking
  • src/github/backfill.ts's dedupeLatestCheckRunsByName fallback branch (no comparable started_at) always keeps the later array entry regardless of which side actually has a timestamp, so a timestamped earlier entry can be silently overridden by an untimed later one — worth a one-line comment or test for that specific mixed case.
  • Classic commit-statuses are explicitly left un-deduped based on the assumption that GitHub's Combined Status API always returns one entry per context; this is accurate per GitHub's docs but is asserted rather than defended by a test, unlike the check-run path.
  • The external brief flags the `8601` magic number reference in a comment at backfill.ts:2691 — it's just a code-comment mention of ISO-8601 format, not an actual literal in code, so no action needed there.
  • Consider a test where one duplicate has `started_at` and the other doesn't, to lock in the fallback-to-array-order behavior described in the comment at backfill.ts (dedupeLatestCheckRunsByName).
  • The GraphQL query change (adding `startedAt`) should be double-checked against GitHub's actual CheckRun GraphQL schema field casing during a live smoke test, since this can't be verified from unit tests with stubbed fetch alone.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 51 registered-repo PR(s), 43 merged, 343 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 51 PR(s), 343 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 51 PR(s), 343 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.58%. Comparing base (61109fe) to head (db8c78e).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3923   +/-   ##
=======================================
  Coverage   93.58%   93.58%           
=======================================
  Files         358      358           
  Lines       34342    34353   +11     
  Branches    12570    12574    +4     
=======================================
+ Hits        32138    32149   +11     
  Misses       1580     1580           
  Partials      624      624           
Files with missing lines Coverage Δ
src/github/backfill.ts 96.92% <100.00%> (+0.02%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 7, 2026
@JSONbored
JSONbored merged commit 70c4396 into main Jul 7, 2026
10 checks passed
@JSONbored
JSONbored deleted the fix/dedupe-rerun-check-runs-live-ci branch July 7, 2026 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

1 participant