Skip to content

Read check conclusions in CI watch, not in the GitHub adapter #840

Closed

Description

@JacobStephens2

What to build

Make the CI watch module the one place that decides what a check run or commit status means for a Delivery. The GitHub adapter then only reports what GitHub said about each check: whether it is still running, and the conclusion or state GitHub gave once it finished, under GitHub's own name for it. It no longer turns that into passed, failed or pending.

The point is to stop losing information. Today the adapter folds every finished conclusion that isn't success, neutral or skipped into "failed", so cancelled, timed_out, stale and the rest look just like failure before CI watch ever sees them. After this change, CI watch receives every conclusion intact and reads it. Its in-memory test adapter can then describe each conclusion, including the cancelled checks in #839.

Behaviour does not change. Every conclusion is read exactly as it is today, including cancelled as a failure. Deciding what a cancelled or superseded check should mean stays with #839, which will then change one reading in CI watch, tested through its own seam.

Acceptance criteria

  • The GitHub adapter reports each check with its conclusion as GitHub named it. That is either a typed value for each documented check-run conclusion and commit-status state, or an unrecognised value that keeps GitHub's text, which is empty when a completed check run has no conclusion. A check run that hasn't completed is reported as unfinished. The adapter's passed/failed/pending verdict type is gone.
  • CI watch alone turns a conclusion into pending, passed or failed. It does this in one exhaustive match with no wildcard arm, so a new conclusion can't compile without a decision. The readings are unchanged:
    • unfinished, or a pending status: pending
    • success, neutral or skipped: passed
    • anything else, including cancelled, timed out, action required, stale, startup failure, a status error and anything unrecognised: failed
  • A CI watch test through its in-memory adapter pins the reading of every conclusion. It covers both the head (pending, passed or the branch's own failure) and the Base branch commit, where a cancelled check counts as an Inherited failure exactly as today. A later change to the reading of cancelled then shows up as a change to this test.
  • The GitHub adapter's execution test feeds the fake gh cancelled, timed-out, in-progress and conclusion-less check runs, plus a commit status in the error state. It asserts each one is reported as GitHub named it.
  • Other code that builds checks as fixtures uses the new conclusion. That includes the prompts page, the Base fix tests and the Repair loop tests. No other module reads a verdict off a check.
  • docs/code-navigation.md names CI watch as the owner of check-state reading, beside Inherited failure.
  • cargo test passes. UPDATE_PROMPTS=1 cargo test prompts_page leaves prompts/ and site/prompts/index.html unchanged.

Blocked by

  • None (can start immediately)

Decisions

  • The adapter reports and CI watch judges. Turning a GitHub conclusion into a verdict is CI policy, and CI watch is its only reader: it counts pending checks, picks out failed ones and splits Inherited failures. Putting that verdict beside its only reader gives locality. It also lets the in-memory adapter, which is CI watch's test surface, express every case GitHub can return. Today it can express only three.
  • The conclusion is a typed value with an unrecognised fallback, not raw strings. Typed values make the CI watch match exhaustive. The fallback keeps today's rule that an unknown finished conclusion counts as failed, and it keeps GitHub's text for progress lines and debugging.
  • Check runs and commit statuses share one conclusion type. Both are checks to CI watch. A commit status's success, failure and pending reuse the check-run values, and its error gets a value of its own. CI watch then has a single match.
  • The verdict stays private to CI watch. No other module needs a verdict. The Base fix, Repair and prompt code only handle checks CI watch has already found to be failed. A public verdict type would bring back the leak.
  • Behaviour is preserved, and CI watch reads checks cancelled by a newer push as a failure and starts a Repair #839 stays separate. What a cancelled check means takes knowledge only the Delivery has: whether the head is still the pull request's head, and whose push moved it. It is also a product decision, which CI watch reads checks cancelled by a newer push as a failure and starts a Repair #839 already tracks. This change is the prefactor that makes CI watch reads checks cancelled by a newer push as a failure and starts a Repair #839 a one-reading change inside CI watch.
  • Out of scope:
  • No CONTEXT.md or ADR change. "Conclusion" and the verdict are implementation terms, and no domain term changes meaning. The change is easy to reverse and is no surprising trade-off, so it doesn't warrant an ADR.

Architecture review

Strength: Strong · Dependencies: ports & adapters

Files: src/github.rs, src/ci.rs, src/github/execution_tests.rs, src/delivery/repair_loop.rs, src/base_fix.rs, src/prompts_page.rs

Before

flowchart LR
  GH[GitHub checks] --> A[GitHub adapter]
  A -.verdict leaks: cancelled becomes failed.-> W[CI watch]
  M[In-memory adapter] -->|passed, failed or pending only| W
  W --> D[Delivery]
  classDef leak stroke:#dc2626,stroke-width:2px;
  class A leak
Loading

After

flowchart LR
  GH[GitHub checks] --> A[GitHub adapter]
  A -->|conclusion as GitHub named it| W
  M[In-memory adapter] -->|every conclusion| W
  subgraph W[CI watch]
    direction LR
    R[reading of each conclusion] --> V[pending, passed or failed and Inherited split]
  end
  W --> D[Delivery]
  classDef deep stroke-width:3px;
  class W deep
Loading

Problem: the GitHub adapter decides what a check means and keeps only three states, so CI watch, and its tests, can't distinguish a cancelled check from a red one (#839).

Solution: the adapter reports each conclusion as GitHub named it, and CI watch alone reads it, with unchanged readings.

Wins

Activity

  1. added
    needs-triageMaintainer needs to evaluate this issue
    ready-for-agentFully specified, ready for an AFK agent
    architect-planAn Architect plan: the Spec or Ticket an Architecture review published
    in-progressA Claim: thirdshift has taken this issue
    and removed
    needs-triageMaintainer needs to evaluate this issue
    ready-for-agentFully specified, ready for an AFK agent
    on Oct 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    architect-planAn Architect plan: the Spec or Ticket an Architecture review published

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions