Skip to content

US-45.6: a thread-mode merge requires green CI on the checkpoint's exact commit - #909

Closed
mkreyman wants to merge 3 commits into
masterfrom
feature/us-45.6-ci-evidence
Closed

mkreyman wants to merge 3 commits into
masterfrom
feature/us-45.6-ci-evidence

Conversation

@mkreyman

Copy link
Copy Markdown
Owner

US-45.6 (Epic 45, change threads): CI evidence by exact SHA. The review gate is still running.

What

A thread has no pull request, and the loopctl App pushes the squash itself (US-45.5), so no forge rule holds a thread merge to green CI. The merge gate now does:

  • intake_sources.required_checks (migration 20260927100000, text array, default empty). A thread source must name at least one, judged over the source as it will be on every write; local-gate is refused. POST/PATCH /api/v1/intake/sources and the MCP tools take it; a change is audited as intake_source_required_checks_set.
  • PullRequestSource.check_evidence/3 reads, for the checkpoint's OWN sha, the check runs under each required name (check_name, filter=latest) and the combined commit statuses. Both APIs, because the combined status never lists check runs (Actions). A truncated list is an error, never a partial answer.
  • Loopctl.Delivery.CiEvidence (pure) judges it. Failed -> required_check_failed (refuse). Running -> required_check_pending, not reported -> required_check_missing, both unevaluated with Retry-After: 60. Only a MISSING check counts toward the unevaluated bound, so a slow CI never escalates while a workflow that does not run on the thread branch does. No requirement -> required_checks_unset. Failed read -> ci_evidence_unavailable. local-gate is recorded and never counted (AC-45.6.2).
  • enforce/3 copies the evidence onto the checkpoint's gate_evidence["ci"] (AC-45.6.1) for every decision; for an allow the copy happens BEFORE the allow is recorded, and a copy that does not land refuses ci_evidence_not_recorded.
  • Verdict and API answer carry ci_evidence; OpenAPI, MCP (2.108.0), README, CHANGELOG, FLY_SECRETS (checks: read, commit statuses: read on GITHUB_TOKEN) and the delivery-loop doc updated.

Design choices:

  • Required checks come from the SOURCE, not a branch ruleset: in thread mode master's ruleset admits only the App, which pushes the squash before any CI ran on it, so a ruleset cannot carry the requirement.
  • The policy is read CURRENT (like the gate triggers), not bound at placement: tightening what a thread must pass applies to the next evaluation.
  • Operator impact: a thread-mode source enrolled before this migration has no checks and every story is refused required_checks_unset until one is named (in CHANGELOG).

Tests

TC-45.6.1 (green on the parent, pending on the checkpoint -> not allowed), TC-45.6.2 (local-gate alone -> not allowed), CiEvidence table tests, judge tests for every reason, the bound exemption for running checks, evidence copy on allow and refusal, intake validation, adapter parsing/truncation, record_gate_evidence isolation, MCP tool tests.

Mutations (all exit 0 = caught)

id mutation check
M01 failed checks produce no refusal judge test
M02 CI waits dropped from unevaluated judge test
M03 every unevaluated counted integration
M04 local-gate looked up as a required check ci_evidence test
M05 required_checks_unset dropped judge test
M06 gather never reads CI (wiring) integration
M08 enforce never copies evidence integration
M09 allow skips the evidence copy integration
M10 thread source may require nothing intake test
M11 local-gate accepted as required intake test
M12 enrolment drops required_checks intake test
M13 update ignores required_checks intake test
M14 truncated check-run list accepted adapter test
M15 filter=latest not sent adapter test
M16 pending status read as passed ci_evidence test
M17 neutral/skipped not passing ci_evidence test
M18 evidence write replaces other keys threads test
M19 evidence write ignores story_id threads test
M20 controller ci_evidence not rendered controller test
M22 failure precedence removed ci_evidence test
M23 pending waits counted judge test
M24 MCP accepts local-gate node test
M25 MCP drops required_checks at enrolment node test
M26 pending precedence removed ci_evidence test
M27 CI wait carries no retry_after judge test

M07 and M21 were withdrawn before running (malformed). The migration was not mutated (KB 346ba61e: a mutated migration leaves the test DB in its mutated state).

…act commit

intake_sources gains required_checks (migration 20260927100000). A thread source must
name at least one, judged over the source as it will be; local-gate is refused.

The merge gate reads check runs (by name, filter=latest) and commit statuses for the
checkpoint's own SHA through PullRequestSource.check_evidence/3 and judges them in the new
pure Loopctl.Delivery.CiEvidence:
- a failed required check refuses required_check_failed;
- one still running or not yet reported is unevaluated (required_check_pending /
  required_check_missing, Retry-After 60); only a missing one counts toward the bound;
- a source requiring none refuses required_checks_unset; a failed read is
  ci_evidence_unavailable;
- local-gate is recorded and never satisfies a required check.

enforce/3 copies what was read onto the checkpoint's gate_evidence under "ci"
(Threads.record_gate_evidence/5); an allow whose copy did not land is refused
ci_evidence_not_recorded. The verdict and the API answer carry ci_evidence.

MCP 2.108.0: intake_source_enroll/update take required_checks; merge_precondition names
the new reasons.

Mutations M01-M27 (M07/M21 withdrawn as malformed), all exit 0.
…cement

1+2. A CI wait (running or missing) never counts toward the poll bound; it is bounded in
     TIME from the checkpoint's recording (ci_wait_limit_seconds, 6h, GitHub Actions' job
     ceiling) and then refused required_check_timed_out. claim_checkpoints now selects the
     checkpoint's inserted_at, which the limit is measured from.
3.   A null element in required_checks is a 422, not a String.trim raise.
4.   An evidence write that met contention turns an allow into unevaluated (counted, like
     any transient fault), never an escalation (allow_evidence_outcome/2).
5.   Only the LATEST result under a name counts: highest check-run id, and between a run
     and a status the later timestamp; the adapter now returns id/started_at/completed_at
     and a status's updated_at.
6.   record_gate_evidence never writes over a record read later (read_at compared in SQL;
     read_at is always six fractional digits so it orders as text).
7.   One paged check-runs read per commit (filter=latest, up to 3 pages) instead of one
     request per required name; the moduledoc states the thread path's call ceiling.
8.   required_checks is bound at placement on runner_dispatches (migration
     20260927100100), read by the gate from the claim's row, falling back to the source's
     current list only for a row that recorded none.
9.   Verdict docs: unevaluated and retry_after now describe CI waits.
10.  MCP enrol comment moved back onto the mode check it describes.

Mutations re-run: M02-M27 as cited (M14, M22, M23, M26 retargeted as M14b/M23b or
superseded by N06/N07; M01, M19 retargeted), plus N01-N09, N11-N14 for the fixes. All
exit 0.
1. A commit status can be posted by the implementer's own runner, so only a check run
   created by GitHub Actions (CiEvidence.trusted_check_apps/0) satisfies a required
   name; statuses are read and recorded, never trusted. The adapter returns the App slug.
   Among trusted runs of a name the highest id decides.
2. An EMPTY list bound at placement falls back to the source's list like NULL.
3. A CI wait holds back only an allow: undecided/5 splits CI waits from forge faults and
   decides everything else now (await_ci/2).
4. record_gate_evidence answers :superseded (not :ok) when a newer read stands; the allow
   path re-evaluates instead of recording an allow.
5. Required checks with nothing read refuse ci_evidence_not_read (fail closed).
6. CI waits retry after 300s; an evidence record identical to the stored one (read_at
   aside) is not rewritten. The write now decides under a FOR UPDATE row lock.
7. A pure CI wait clears the consecutive-unevaluated count.
8. OpenAPI limits reference Source.max_required_checks/0 and max_check_name_bytes/0.
9. stub_all_defaults stubs check_evidence/2 fail-closed.
10. check_evidence drops the unused names parameter; its doc says the adapter returns
    every run and CiEvidence matches names.

Mutations: the cited set re-run (M01b, M02-M25, M14b, M23b, N01-N04, N09, N11-N14) plus
R01-R13 for these fixes. All exit 0.
@mkreyman

Copy link
Copy Markdown
Owner Author

Superseded by #910. Round 3 on this PR still found material defects in its trust model (self-attestation through workflow files, a green suite hiding a red one, the wait's origin, placement-bound policy), so US-45.6 was rewritten on a fresh branch rather than patched a fourth time.

@mkreyman mkreyman closed this Sep 27, 2026
mkreyman added a commit that referenced this pull request Sep 27, 2026
… exact commit (#910)

* US-45.6 (v2): a thread merges only on trusted CI for the checkpoint's exact commit

Rewrite of #909, whose third review round still found material defects in one design
element: which CI results the gate trusts and against which policy.

- intake_sources.required_checks (migration 20260927100000), read LIVE by the gate, so a
  renamed job can be corrected for stories already in flight. A thread source must name at
  least one; local-gate and non-string names are refused.
- Only a check run created by GitHub Actions satisfies a required name. Commit statuses are
  read and recorded, never trusted: the implementer's runner can post them.
- A checkpoint changing .github/workflows/ or .github/actions/ (renames included) is refused
  ci_definition_changed for a human: Actions runs the workflow files of the commit under test.
- Per name, the latest run of each check suite counts and every suite must pass.
- A check still running or not reported is a CI wait: unevaluated, Retry-After 300, never
  counted toward the unevaluated bound (and it clears the count), refused
  required_check_timed_out 24 hours after the story ENTERED ci (Stages.entered_at/3). A
  wait holds back only an allow; any other refusal is decided at once.
- Evidence (required runs and local-gate only) is copied onto the checkpoint's
  gate_evidence["ci"] under a row lock; a record read no later than the stored one is
  :superseded (the allow path waits and re-evaluates), an identical judgement is not
  rewritten. An allow whose copy met contention is unevaluated, never an escalation.
- Required checks with nothing read fail closed (ci_evidence_not_read).
- MCP 2.108.0: intake_source_enroll/update take required_checks; merge_precondition names
  the reasons.

Mutations A01-A47 (A31 re-run as A31b after strengthening its test), all exit 0.

* US-45.6 v2 review round 1: trust only the thread's own push-run jobs

1+2. Only a JOB of a GitHub Actions workflow run that a PUSH of the thread branch at the
     checkpoint's exact commit triggered satisfies a required check (Actions runs + jobs
     APIs, filtered by the API and again locally), and only by concluding success: a job
     skipped because a needs: failed, or neutral, fails. A check run created by any other
     workflow, or a status, is never trusted. check_evidence/3 now takes the branch.
3.   Jobs are de-duplicated by id and a short list is refused as truncated; more than 10
     workflow runs per push is refused rather than read.
4.   Commit statuses are read best effort ({:unread, reason}), feeding only local_gate.
5.   Stages.entered_at/3 runs under answering_busy with a bounded lock wait; contention is
     the ci_entry_unreadable fact, a retry.
6.   Scope stated in the CiEvidence moduledoc: scripts the jobs run are code under review,
     judged by the thread review (US-45.3), not by this gate.
7.   A diff that could not be listed refuses ci_definition_unknown (fail closed).
8.   A story with no recorded ci entry measures its wait from the checkpoint's recording.
9.   A required check name with surrounding whitespace is refused.
10.  OpenAPI interpolates ci_wait_retry_after/0 and ci_wait_limit_seconds/0; MCP, verdict
     and CHANGELOG no longer restate the numbers. Token scope is now actions: read.

Per-workflow judgement uses the highest job id (ids only grow); the separate newest-run
filter was redundant and removed (mutation B02 showed it could not fail).

Mutations A01-A47 (A14-A18, A20-A22, A25 superseded by the B set; A19 by B01; A30
retargeted) and B01, B03-B15, all exit 0.

* US-45.6 v2 review round 2: every job in the newest run counts

1. Per workflow, only its newest run counts, and in it EVERY job carrying a required name
   must pass: matrix legs sharing a name are separate jobs (round 1 wrongly removed the
   newest-run step and judged the highest job id alone).
2. The ci entry time is read on exactly the path that reads CI, so a lock wait on it can no
   longer mask a merged or moved head's decision.
3. An identical evidence read that is later advances the stored read_at, so a slower read
   from in between is :superseded; an identical earlier read is :ok.
4. OpenAPI ci_evidence, the delivery-loop doc and the Source field comment describe the jobs
   design, not v1's check runs and statuses.
5. A composite action's action.yml anywhere in the diff is a CI definition change.
6. The jobs list's shape is judged before de-duplicating, so a malformed entry is
   unreadable_jobs, never a crash.
7. Per-run jobs reads run concurrently (4 at a time); the moduledoc restates the ceiling.
8. The migration names the manual step for thread sources enrolled before it; CHANGELOG
   gives the real column type.
9. ci_result/1 uses Map.fetch!: CI is judged once, in judge/1.

Mutations re-run: A01-A47 and B01-B15 as retargeted, plus C01, C02, C04-C10 (C10 and A40
re-run as C10b/A40b after a test for the identical-earlier case). All exit 0. Finding 2's
gating has no falsifiable test: which path reads the entry time is not observable from a
test.

* US-45.6 v2 review round 3 (the ceiling): runs judged from runs, evidence on decisions

Fixed in place (no round 4; every fix carries mutation proof), following #902's round 3:
none of the findings touched the trust model the rewrite settled.

- Runs (findings 2, 3, 4, 7): the adapter reduces the push runs to each workflow's NEWEST
  run before bounding and before any jobs read, and returns those runs. CiEvidence takes the
  newest run per workflow from the runs, once per judgement: a newest run with no jobs yet
  holds a name pending; one that ended with no jobs (startup_failure) fails a name no job
  carries (run_<conclusion>).
- Evidence (findings 1, 5): read_at is stamped when the read STARTS; evidence is copied onto
  the checkpoint only for a decision (allow, refuse), so CI-wait polls write nothing.
- Latency (finding 6): the gate's three forge reads (two trees, CI evidence) run
  concurrently.
- MCP (finding 9): required_checks refuses surrounding whitespace and duplicates locally,
  matching the server.
- Counter (finding 8): kept clearing on a pure CI wait (round 2's decision); documented why
  the wait is still bounded — every answered poll is judged against the CI wait limit.

Mutations: the cited set re-run (A06b, A40b, C10b retargeted) and D01-D05, D07-D09 (D03 re-proved as D03b after a complexity split), all 60
exit 0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant