Kernel: what each check conclusion means

Merged
DO
5d ago
+454-28

Closes the story "Kernel: what each check conclusion means" from the epic "Epic: nine gaps in the Kernel, found in review" (#29). Until now, any conclusion other than success or pending counted as check_failed. So a required Lens that came back neutral admitted a fix Run with nothing to fix. Earlier in the proof of concept, neutral counted as passing (L-07).

Based on main, rebased after "Lifecycle check: bounds in the data for every cycle with no person in it" (#85).

What changes

This follows the architect's working ruling on the issue (option 1), and the review's ruling on order and attention:

  • The facts carry GitHub's own conclusion, and the port maps nothing. It only lowercases what GraphQL returns (SUCCESS, IN_PROGRESS).

    • In schemas/facts.schema.json, checks[].conclusion used to be a list of four allowed values. It is now any code matching ^[a-z_]+$.
    • The new counter-example examples/invalid/facts.a-conclusion-in-words.json keeps sentences out.

  • What each conclusion means is data. config/defaults.json gains check_conclusions, and schemas/config.schema.json describes it:

    • success is passing.
    • failure and timed_out are failing.
    • Normal waits are pending: queued, in_progress, pending, waiting, requested, expected, and a required check that has not reported.
    • neutral, skipped, cancelled, stale, action_required, startup_failure and error are unreadable. So is any value the map does not name.

  • The configuration cannot make neutral pass. The schema requires "success": "passing" and allows every other key only failing, pending or unreadable. Both new counter-examples are rejected: examples/invalid/config.conclusion-mapped-to-nothing.json (neutral mapped to null) and examples/invalid/config.a-neutral-check-passes.json. As a second guard, the Kernel treats any value other than these four readings as unreadable.

  • kernel/derive.py. A new function, check_reading(), reads each required check through the map.

    • A failed check is read first. It is readable and decisive: whatever a check with no verdict would say, the content goes back to the author.
    • With no failure, an unreadable check keeps the item in verifying with reason check_unreadable, before any waiting check is considered.
    • The duplicate-name behaviour that #60 will fix (the last entry wins) is unchanged.

  • kernel/decide.py. check_unreadable defers: no Run is admitted and no act is made. It is in NEEDS_YOU_REASONS, so the item shows as needing the Maintainer. Almost every check with no verdict needs a person: an approval, a change to the required set, or a re-run.

  • tests/test_decide.py gains six tests:

    • test_a_neutral_required_check_defers: neutral, skipped, cancelled, stale, action_required, startup_failure, error and an unknown value, on each required Lens. Each is tried unreviewed, reviewed, and reviewed with a standing approval. Each gives defer, reason check_unreadable, attention needs_you, with no admission and no act. The other side: with every check green, the review is admitted, then the approval issued, then the item enqueued.
    • test_a_timed_out_required_check_goes_back_to_the_author
    • test_a_queued_running_or_absent_required_check_is_waited_for, including waiting, requested and expected
    • test_a_failed_check_is_read_before_an_unreadable_or_waiting_one, which pins the order
    • test_a_failure_with_cancelled_or_skipped_siblings_goes_back_to_the_author: check_failed and admit, on each Lens
    • test_a_reading_the_kernel_does_not_know_is_unreadable, the fail-closed reading of a map the schema would refuse

    test_a_neutral_check_does_not_pass_a_dependency_update, from "Kernel: a dependency update needs no model review" (#70), still passes.

  • tests/test_tools.py gains test_only_success_can_pass_a_check. It shows the schema refuses every conclusion other than success mapped to passing, an unknown name mapped to passing, and any map where success does not pass.

  • sim/world.py.

    • A new fault, a_check_can_time_out_or_give_no_verdict, is listed in sim/grounding.py.
    • New invariants: no review-change is admitted and no act is made while a required check has no verdict; an author's fix Run may still be admitted for a failure or a conflict. A deferral on check_unreadable shows as needs_you.
    • settle() still has a person run again every check that did not pass. tests/test_simulator.py accepts an item resting on check_unreadable only if its attention is needs_you.
    • The invariants hold on 300 seeds.

  • tools/mutants.py gains eleven mutants and replaces "a neutral deterministic check passes", whose line is gone. 63 of 63 are killed.

    • "a neutral check is read as passing"
    • "a neutral check is read as failing"
    • "a conclusion the map does not name is read as passing"
    • "a timed-out check is read as no verdict"
    • "a running check is read as passing"
    • "a reading the Kernel does not know is trusted"
    • "an absent required check passes"
    • "a check with no verdict is ignored"
    • "a check with no verdict is read before a failed one"
    • "a check with no verdict does not defer"
    • "an unreadable check is not the Maintainer's to see"

  • Documents:

    • Design §3.3: the checks row, and a paragraph on the map, the order, the port's lowercasing and the Maintainer's attention.
    • Design §4.2 item 1, §0 and the Lifecycle diagram: "passed", not "reported".
    • Design §4.5: the new fault.
    • Design §4.6: a new boundary row.
    • L-07: its rule and checks.
    • Decision 0012: "passed" in its summary, and two boundary rows.

Acceptance criteria (from "Kernel: what each check conclusion means")

  • The facts carry GitHub's own conclusion, and the port maps nothing. check_conclusions is in the defaults and the schema. A value missing from the map is unreadable. Counter-example: examples/invalid/config.conclusion-mapped-to-nothing.json. The schema also refuses any map that reads anything but success as passing.
  • Tests written first: test_a_neutral_required_check_defers and test_a_timed_out_required_check_goes_back_to_the_author.
  • Three mutants: neutral read as passing, neutral read as failing, and an unmapped value read as passing.
  • Documents: design §3.3 checks and §4.2 item 1; L-07.

Left open, on purpose

  • The order is: failed, then unreadable, then waiting. A failure is readable and decisive, so I-13 holds: whatever a check with no verdict would say, the content goes back to the author. The other order would make an ordinary failure wait for a person. GitHub's default fail-fast reports one failure beside cancelled siblings, and a job that needs: the failed one is skipped. Both cases are boundary tests, and the reversed order is a mutant.
  • An unreadable check leaves the item in verifying and shows it as needing the Maintainer. The other unreadable reasons (mergeable_unreadable, facts_incomplete) still give no State.
  • The duplicate check name, where the last entry wins, is #60's to fix. This change keeps that behaviour as it was.
  • Lowercasing GraphQL's conclusion is the port's job, and waits on adapters/.
  • a_check_can_time_out_or_give_no_verdict names no trial in section 6, so it is listed as unverified.

Checks and reviews

  • make verify and python3 -m tools.mutants pass on a git archive copy: 245 tests, OK (3 expected failures); 63 of 63 mutants killed.
  • The tests were written first.
    • These failed until the code changed: the neutral, waiting, order, failed-beside-cancelled-or-skipped and unknown-reading cases, the needs-you attention, the schema test, and the simulator's needs-you invariant.
    • test_a_timed_out_required_check_goes_back_to_the_author passed throughout, because every conclusion except success already failed. Its mutant shows the test now holds that.
  • A first review was clean on security, and its replay of the old test facts showed only the intended change. Its two majors are fixed in the second commit: the order of failed and unreadable checks, and an unreadable check now needs a person. So are its minors: the normal waits mapped to pending, the port note, and "reported" becoming "passed".
  • The second model family's read is the Gemini code review and the other bot reviewers on this pull request, as the Maintainer directed. It merges once it is clean and green at the reviewed head, under the Maintainer's standing approval.

Closes #35

🤖 Generated with Claude Code