[JacobStephens2]

Summary

The GitHub adapter now reports each check as GitHub named it, and CI watch decides what it means. Readings are unchanged; cancelled is still a failure (#839 will change that one reading).

 GitHub::checks_on                       # src/github.rs
-  check run / status -> CheckState { Pending, Passed, Failed }
+  check run  -> Conclusion::of_check_run   (or Unfinished if not completed)
+  status     -> Conclusion::of_commit_status
+  Conclusion { Unfinished, Success, Failure, Neutral, Skipped, Cancelled, TimedOut,
+               ActionRequired, Stale, StartupFailure, Pending, Error, Unrecognised(text) }

 ci::watch_to_end                        # src/ci.rs
-  check.state == CheckState::Pending / Failed
+  reading(check)   # private, one match, no wildcard arm
+    Unfinished | Pending                            -> Pending
+    Success | Neutral | Skipped                     -> Passed
+    every other conclusion, incl. Unrecognised(_)   -> Failed

Fixtures in the prompts page, Base fix and Repair loop tests now build Conclusion::Failure. docs/code-navigation.md names CI watch as the owner of check-state reading, beside Inherited failure.

Evidence

• Before: CheckState had three values, so the in-memory adapter could not express a cancelled check, and the adapter's test could only assert Passed/Failed/Pending.
  After: new tests pass:

  ci::each_conclusion_on_the_head_is_read_as_pending_passed_or_the_branchs_own_failure
    for every Conclusion (table forced complete by a no-wildcard match):
      head lists it -> Pending (still running, then passes) | Passed | the branch's own failure
  ci::each_conclusion_on_the_base_branch_commit_counts_as_an_inherited_failure_when_failed
      base lists it, head red -> Inherited failure iff read Failed (Cancelled -> Inherited)
  github::execution_tests::github_reports_each_check_as_github_named_it
      fake gh: cancelled, timed_out, in_progress, completed+null, "someday", status error
      -> Cancelled, TimedOut, Unfinished, Unrecognised(""), Unrecognised("someday"), Error
  

• cargo test: 52 suites, 2012 passed, 0 failed, 2 ignored. cargo clippy --all-targets -- -D warnings clean.
• UPDATE_PROMPTS=1 cargo test prompts_page leaves prompts/ and site/prompts/index.html unchanged (git status clean).

Merge Danger

Door: two-way

A refactor with behaviour preserved; revert restores the old type.

Blast Radius: CI-verdicts

Every Run's CI watch reads checks through the new match; a mapping slip would mis-judge CI. The two pinning tests cover each conclusion on both the head and the Base branch commit.

Review

Reviewed with thirdshift-code-review against main. Standards: 8 judgement calls, no hard violations. Spec: meets the spec; 3 minor findings. Addressed: S1 (Conclusion doc no longer claims Unfinished is GitHub's name), S3 (count parameter renamed wanted), S5/P1 (every_conclusion() opens with a no-wildcard match, so a new conclusion can't compile without a test row), P2 (navigation row names CI watch). Both reviewers read every changed file; no changed file was left unread, and no generated output was excluded.

Unaddressed findings

Standards

• S2 — Unrecognised(String) stores a missing conclusion as "" (possible Primitive Obsession). The issue fixes this shape: "an unrecognised value that keeps GitHub's text, which is empty when a completed check run has no conclusion." reading treats both alike (Failed), so nothing differs.
• S4 — Check literals repeated across test fixtures (possible Shotgun Surgery). A pre-existing pattern the reviewer flagged as not introduced here; the issue lists exactly these fixtures to update ("the prompts page, the Base fix tests and the Repair loop tests"). A shared constructor is out of this change's scope.
• S6 — head-reading test infers Pending from the "still running" progress line. The reviewer notes it follows local practice: other ci.rs tests assert on github.steps (e.g. the_base_branch_commit_is_read_once_after_every_check_has_finished), and progress lines are an observable outcome of the in-memory seam.
• S7 — repeated answers map/collect in two execution tests (possible Duplicated Code). Two copies, which the reviewer called borderline; a helper would serve only these two tests.
• S8 — "check-state reading" is not a GLOSSARY.md term. The issue's own criterion uses this wording ("names CI watch as the owner of check-state reading") and its Decisions say "No CONTEXT.md or ADR change … 'Conclusion' and the verdict are implementation terms"; the reviewer judged it acceptable.

Spec

• P3 — the navigation row also adds supporting tests and checks_on. No change needed: the reviewer judged it consistent with the file's "Keep these pointers current when ownership moves".

Closes #840

🤖 Generated with Claude Code

Built with claude · claude-opus-5-5 · medium

Security review outcome

No unaddressed introduced findings.