Skip to content

US-45.4: merge gate reads a recorded checkpoint for thread-mode sources - #906

Closed
mkreyman wants to merge 3 commits into
masterfrom
feature/us-45.4-checkpoint-gate-v2
Closed

mkreyman wants to merge 3 commits into
masterfrom
feature/us-45.4-checkpoint-gate-v2

Conversation

@mkreyman

Copy link
Copy Markdown
Owner

Replaces #904, which was closed after review round 3 (the round ceiling) still found material defects. Every round's worst defect was in AC-45.4.4's base-update edge. That edge has no caller until the merge executor (US-45.5), and cannot be judged safely until US-45.6 supplies CI evidence for an exact SHA, so this PR rewrites US-45.4 narrower: AC-45.4.1, .2, .3 and .5 only. The review gate has not run on this PR yet.

Moved to US-45.5

  • AC-45.4.4, the ci -> ci base_updated edge, is now AC-45.5.7 in us_45.5.json, with the added requirement that the edge must not allow a base_update head until US-45.6's CI evidence for that exact SHA is green. Its two test cases moved with it (TC-45.5.7, TC-45.5.8).
  • US-45.5 now depends on US-45.6.
  • Not ported from US-45.4: merge gate and stage machine speak checkpoints #904: record_base_update, follow_base_update, base-update chains, base_update_stale, base_update_parents_mismatch, the chained stage entry. No stage-machine change at all.

What this PR does

  • AC-45.4.1 intake_sources.mode (pr | thread, NOT NULL, default pr; migration 20260926140000, no manual step). Set by presence on POST/PATCH /api/v1/intake/sources and the intake_source_enroll / intake_source_update MCP tools; null or unknown is 422; a change is chained as intake_source_mode_set. A mode change is 409 stories_in_flight while any story of the project is past intake and not done/failed (escalated counts). Enrolling a mode that differs from the project's previous source's (pr if none) gets the same 409, so revoke + re-enrol cannot bypass it.
  • AC-45.4.2 For a thread-mode story, MergePrecondition judges the story's latest recorded checkpoint (one row, Threads.latest_recorded_checkpoint/2) through Loopctl.Delivery.CheckpointSource, which builds the pull-request facts shape from three new PullRequestSource callbacks (branch_head/2 on the exact git/ref, commit/2, compare/3). No pr_number. The branch is the one the story was dispatched on (DispatchPayload.story_branch/2: runner_dispatches.branch of the newest implement dispatch, filtered and limited in SQL; branch_for(story, []) only when nothing recorded one). A merge_commit_sha on a checkpoint counts as already_merged only when the base branch contains it.
  • AC-45.4.3 The branch fact is judged first. A missing branch (branch_missing), a branch naming a commit nobody recorded (branch_head_unrecorded), or a checkpoint the forge cannot find (checkpoint_unpushed, a 404 on its commit or comparison) goes back to implementing over base_moved, with no escalation; a moved branch is decided without reading the commit or the file lists. empty_change (tree equal to the base's, or no file changed), checkpoint_tree_mismatch and no_checkpoint_recorded refuse.
  • AC-45.4.5 A thread-mode allow's effect_recorded event carries checkpoint_id and checkpoint_sha (Stages.record_effect/5 gained :event_data); the verdict carries mode, checkpoint_id, checkpoint_sha.
  • Round-3 findings fixed in scope: 2 (an unpushed checkpoint routes back, branch judged first), 3 (enrolment 409, revoke-proof), 6 (a single-row latest-checkpoint read), 7 (story_branch filters and limits in SQL), 8/9 (the moved decision is computed once, from the real facts map, in judge/1; gather/3 asks the same function of the facts it is building).
  • One forge resolver (PullRequestSource.impl/0) for every delivery-loop caller. OpenAPI, the merge_precondition / intake_source_* tool descriptions and README rows, docs/agent-delivery-loop.md and the CHANGELOG are updated. loopctl-mcp-server 2.106.0. No env var.

Tests

Commit gate green (full suite, dialyzer, credo --strict). mcp-server node --test test/*.test.js green. Thread-mode cases are in merge_precondition_judge_test.exs (pure) and merge_precondition_integration_test.exs (committed tenant). Also covered: the exact dispatched branch, the 404 paths, the branch-first short-circuit, merged-only-when-contained, and the allow event. dispatch_payload_story_branch_test.exs covers the newest implement dispatch, triage rows being ignored, the fallback, and tenant isolation. The intake controller tests cover mode default/set/audit, 422s, 409 on update/enrol/revoke-and-re-enrol, escalated counting, project scoping and tenant isolation. Story files validate against story.schema.json.

Mutation evidence

~/workspace/claude-config/bin/mutate.sh <file> --old ... --new ... --quiet --no-baseline -- <check>, each check just watched green. Exit 0 = the check went red under the mutation.

# guard check exit
V1 thread input facts (no_checkpoint_recorded) delivery/merge_precondition_judge_test.exs:1035 0
V2 gather picks the checkpoint source (wiring) delivery/merge_precondition_integration_test.exs:467 0
V3 latest checkpoint read (wiring) delivery/merge_precondition_integration_test.exs:467 0
V4 dispatched branch wired in delivery/merge_precondition_integration_test.exs:524 0
V5 branch_head_unrecorded delivery/merge_precondition_judge_test.exs:1005 0
V6 branch_missing delivery/merge_precondition_judge_test.exs:1054 0
V7 checkpoint_unpushed delivery/merge_precondition_judge_test.exs:1061 0
V8 branch fact judged first (order) delivery/merge_precondition_judge_test.exs:1068 0
V9 branch reasons wired into moved delivery/merge_precondition_integration_test.exs:487 0
V10 moved routes back, not refused delivery/merge_precondition_integration_test.exs:487 0
V11 pr mode reads no branch fact delivery/merge_precondition_judge_test.exs:1078 0
V12 empty_change (tree) delivery/merge_precondition_judge_test.exs:1013 0
V13 empty_change (no files) delivery/merge_precondition_judge_test.exs:1020 0
V14 checkpoint_tree_mismatch delivery/merge_precondition_judge_test.exs:1027 0
V15 thread reasons wired into gated delivery/merge_precondition_integration_test.exs:500 0
V16 allow names the checkpoint delivery/merge_precondition_integration_test.exs:467 0
V17 record_effect writes the payload delivery/merge_precondition_integration_test.exs:467 0
V18 moved skips the file reads (skip? wiring) delivery/merge_precondition_integration_test.exs:604 0 (1 before the test was tightened)
V19 branch read FIRST: no commit read on a moved branch delivery/merge_precondition_integration_test.exs:571 0
V20 404 branch is missing delivery/merge_precondition_integration_test.exs:508 0
V21 404 commit is unpushed delivery/merge_precondition_integration_test.exs:555 0
V22 merged only when contained delivery/merge_precondition_integration_test.exs:582 0
V23 contains? consulted (wiring) delivery/merge_precondition_integration_test.exs:582 0
V24 contained merge adopted delivery/merge_precondition_integration_test.exs:592 0
V25 story_branch newest first delivery/dispatch_payload_story_branch_test.exs:43 0
V26 story_branch implement kind only (SQL) delivery/dispatch_payload_story_branch_test.exs:50 0
V27 story_branch reads the ledger delivery/dispatch_payload_story_branch_test.exs:43 0
V28 latest checkpoint is the newest threads_test.exs:88 0
V29 PATCH forwards mode (wiring) intake_source_controller_test.exs:463 0
V30 mode change audited intake_source_controller_test.exs:463 0
V31 enrolment forwards mode (wiring) intake_source_controller_test.exs:439 0
V32 null mode refused intake_source_controller_test.exs:495 0
V33 update in-flight refusal wired intake_source_controller_test.exs:519 0
V34 in-flight mechanism intake_source_controller_test.exs:519 0
V35 escalated is in flight intake_source_controller_test.exs:519 0
V36 detected is not in flight intake_source_controller_test.exs:519 0
V37 scoped to the source's project intake_source_controller_test.exs:660 0
V38 same mode is not a change intake_source_controller_test.exs:519 0
V39 enrolment refusal wired (F3) intake_source_controller_test.exs:581 0
V40 revoked source's mode still counts (F3) intake_source_controller_test.exs:625 0 (1 before the test was tightened)
V41 no previous source means pr (F3) intake_source_controller_test.exs:581 0
V42 create 409 rendered (wiring) intake_source_controller_test.exs:581 0
V43 compare 300-file cap delivery/github_pull_request_source_test.exs:419 0
V44 compare counts required delivery/github_pull_request_source_test.exs:407 0
V45 exact git/ref endpoint delivery/github_pull_request_source_test.exs:339 0
W1 enroll forwards mode node --test (intake_source_tools / delivery_loop_tools) 0
W2 modeRefusal node --test (intake_source_tools / delivery_loop_tools) 0
W3 update forwards mode node --test (intake_source_tools / delivery_loop_tools) 0
W4 publicSource returns mode node --test (intake_source_tools / delivery_loop_tools) 0
W5 update schema enum node --test (intake_source_tools / delivery_loop_tools) 0
W6 enroll schema enum node --test (intake_source_tools / delivery_loop_tools) 0
W7 merge_precondition names checkpoint_tree_mismatch node --test (intake_source_tools / delivery_loop_tools) 0
W8 merge_precondition names checkpoint_unpushed node --test (intake_source_tools / delivery_loop_tools) 0
W9 update names stories_in_flight node --test (intake_source_tools / delivery_loop_tools) 0
W10 enroll names stories_in_flight node --test (intake_source_tools / delivery_loop_tools) 0

V18 and V40 exited 1 on the first run: the moved-head test did not notice the file lists being read, and the revoke test could not tell a revoked pr source from no source. Both tests were tightened (a flunk stub on repo_files; a revoked thread source with a story in flight), and both mutations then exited 0.

Not tested

  • Nothing reachable in scope is left without a test.
  • The in-flight check is read without a lock. A story entering the loop between the read and the mode write is judged under the new mode; this is documented in Intake.

Narrowed rewrite of #904: AC-45.4.1, .2, .3 and .5 only.

- intake_sources.mode (pr | thread, default pr; migration 20260926140000),
  settable through POST and PATCH /api/v1/intake/sources and the
  intake_source_enroll / intake_source_update MCP tools. A mode change, or an
  enrolment in a mode other than the project's previous source's, is 409
  stories_in_flight while any story of the project is past intake and not
  done or failed; escalated counts.
- For a thread-mode story MergePrecondition judges the latest recorded
  checkpoint through Loopctl.Delivery.CheckpointSource, on the branch the
  story was dispatched on (DispatchPayload.story_branch/2), with no
  pr_number. The branch fact is judged first: branch_missing,
  branch_head_unrecorded and checkpoint_unpushed go back to implementing.
  empty_change (tree equal to the base's, or no file changed),
  checkpoint_tree_mismatch and no_checkpoint_recorded refuse. A merge commit
  on a checkpoint is already_merged only when the base branch contains it.
- A thread-mode allow is recorded naming the checkpoint id and sha.
- AC-45.4.4 (the base_updated edge) moves to US-45.5 as AC-45.5.7, gated on
  US-45.6's CI evidence; US-45.5 now depends on US-45.6.
- loopctl-mcp-server 2.106.0.
- A 404 on the checkpoint's commit or comparison, after the branch named it,
  is pull_request_unavailable and escalates; checkpoint_unpushed had no
  reachable case left and is deleted.
- A 404 on the branch ref is branch_missing only when the repository itself
  reads (new PullRequestSource.repository_readable/1); otherwise the
  repository's failure escalates.
- The gate judges the latest kind-checkpoint of the story's CURRENT claim
  epoch, read in one query with the story.
- Thread mode's dependence on runner contract 1.20.0 is stated in OpenAPI,
  both MCP tool descriptions, the README, the docs and the CHANGELOG.
- AC-45.4.3 describes the implemented routing of branch_head_unrecorded.
- The Verdict diffstat doc covers thread mode and @compare_file_cap.
- commit/2 returns the tree only.
- The implement-kind rule lives once, in DispatchLedger, as implement_kind?/1
  and where_implement_kind/1.
- repo_for_story/1 resolves through source_for_story/1 and repo_of/1.
- A thread-mode head that moved (branch_missing, branch_head_unrecorded, the
  new branch_head_regressed, or a head mismatch) goes back to implementing
  only while the claim is live by Claimant.live?/2; otherwise it is refused
  naming claim_not_live and escalates, instead of looping.
- story_branch/3 reads the implement row of the judged checkpoint's claim
  whose session ran (accepted or superseded).
- The compare's base commit is carried as base_sha: on the verdict and in
  the thread-mode allow's event data.
- The thread reads return {:error, reason} through Stages.answering_busy/4;
  a :busy read is transient, so the gate answers unevaluated.
- A released claim with earlier checkpoints is refused claim_ended, distinct
  from no_checkpoint_recorded.
- The enrolment mode check runs inside insert_source's transaction, after the
  insert, so an already-bound repository is still the 422.
- merged, deployed and verified no longer block a mode change; the 409 body
  names the blocking story ids (bounded).
@mkreyman

Copy link
Copy Markdown
Owner Author

Closed after review round 3 (the ceiling). The findings point at the source: the gate reads the intake source's CURRENT mode at judge time, so a mode change can reach in-flight stories. Every round found another hole in the in-flight refusal built to prevent that: the settled set, the previous-mode lookup, and an unfixable race. Round 3 also found that the allow does not require the checkpoint to be based on the current base head, so a tree-squash merge would revert newer master commits. US-45.4 v3 binds the mode to the story's dispatch at placement, which removes the in-flight refusal instead of guarding it. It requires merge_base == base head for a thread-mode allow and fixes round 3's other findings, on feature/us-45.4-checkpoint-gate-v3. This branch stays on GitHub as the record.

@mkreyman mkreyman closed this Sep 27, 2026
mkreyman added a commit that referenced this pull request Sep 27, 2026
…nd at placement) (#908)

* US-45.4: the merge gate and stage machine speak checkpoints (v3, mode bound at placement)

Thread mode: the merge gate judges a story's latest recorded checkpoint under the current
claim instead of a pull request, reading the branch, compare and tree through the new
CheckpointSource.

The merge route is BOUND per implement dispatch at placement. DispatchLedger.record_sent/3
copies the intake source's mode onto the new runner_dispatches.mode column (NULL read as pr),
and the gate reads it from the current claim's implement row via DispatchPayload.dispatch_route/3,
which also resolves the branch in one documented order: the stage's recorded branch effect,
then the ledger row, then branch_for. A source's mode now decides only what future placements
get, so a mode change is always allowed and the stories_in_flight refusal is deleted.

A thread-mode allow also requires the compare's merge base to equal the base head; otherwise
base_moved_since_checkpoint, routed like the other moved-head reasons (live claim: back to
implementing; not live: refuse with claim_not_live).

Also: a contains? error on the merged path falls through to judging the checkpoint as open
unless it is transient; a same-head replay with changed allow data records a fresh effect
event; claim_checkpoints reads in one query; the route read has its own busy telemetry event.

Replaces #906.

* US-45.4 review round 1: base freshness moves to the merge executor; base branch bound at placement

The gate no longer refuses a base that moved since the checkpoint (base_moved_since_checkpoint
is removed): at ci a claim is usually not live, so a busy repository escalated nearly every
thread-mode story. The gate judges the three-dot diff and records base_sha as the compare's
merge base; the merge executor (US-45.5, new AC-45.5.8) merges only while the base head still
equals it and otherwise takes the base-update path. With base_sha the merge base, a same-head
replay records the same data, so the replay-refresh event machinery is removed.

A checkpoint the base already contains (merge base equals the checkpoint) is merged, with or
without a recorded merge commit, and judged on its recorded allow instead of reading as an
empty change.

The implement ledger row now pins base_branch beside mode (same unmerged migration, renamed
add_runner_dispatches_route), and the gate reads both through dispatch_route; a NULL base
branch falls back to the source's current one. The mode is carried from the placement's own
source resolution (DispatchPayload.fill) into record_sent, or read with a project-filtered
query when the caller named the refs, instead of loading every live source of the tenant.

An unreadable route reports mode nil rather than pr, and the route read's lock wait is
bounded. dispatch_route reads accepted rows only. Docs, OpenAPI and the MCP descriptions say
what the code now does.

* US-45.4 review round 2: contained checkpoints need an allow; thread-only branch resolution

A checkpoint the base already contains is already_merged only when a recorded allow names it,
and otherwise refused checkpoint_on_base_without_allow (a checkpoint that is the base it was
cut from, or an ungated fast-forward), never the ungated-merge alarm. The same contained check
now runs on a missing thread branch before it is answered branch_missing, so a checkpoint
fast-forwarded and then deleted is not read as never pushed.

The branch is resolved for thread mode only (DispatchPayload.thread_branch/3): dispatch_route/2
returns recorded facts and derives nothing, so the pr path neither reads nor fails on a thread
branch. A claim with no accepted implement dispatch now takes the source's current mode, as it
already took the base branch, so a thread-source story with no route is judged as a thread.

A non-contention failure to read the thread is thread_unreadable, refused like
pull_request_unavailable, instead of no_checkpoint_recorded. The compare call pages commits with
per_page=1. source_for_project/2 filters the project in SQL and is the one derivation of the
exactly-one-source rule; project_source_mode/2 is removed.

Docs no longer claim the executor's base-update path comes back through this gate: it judges
claimant checkpoints only, and US-45.5 gains AC-45.5.9 (with a test case) requiring
claim_checkpoints to judge the latest base_update reaching the allowed checkpoint.

* US-45.4 review round 3: every placement-bound fact comes from the claim's own ledger row

- The gate judges the current claim's dispatched branch before the stage row's, which
  survives a release and can name an earlier claim's branch (finding 1).
- A claim with no accepted implement row is judged as a pull request whatever the source's
  mode now, so flipping a source never re-routes a session's own PR (finding 2). A checkpoint
  can only be recorded under an accepted row, so such a claim has no thread to judge.
- A resumed dispatch re-sends the base branch recorded at the first push; a retry naming a
  different one is 422 base_branch_conflict (finding 3).
- base_sha is no longer a second verdict field: an allow records merge_base_sha under that
  name, and the API derives it for thread verdicts only (finding 5).
- The claim-route query moved into DispatchLedger.claim_route_query/2 with the ledger's other
  readers; where_implement_kind/1 is gone (finding 6).
- Doc drift fixed: dispatch_route/2 arity, and the base branch pull_request/4 receives (7).
- Finding 4 disproved: checkpoints are unique per (commit_sha, claim_epoch) and a release
  clears the allowed sha, so one sha cannot be allowed under two checkpoint rows.
- Finding 8: Claimant.live? is a pure read of the story struct; the ledger read is what
  decides pr versus thread and cannot be skipped, now said at the call.

Mutations, all exit 0 (caught): R3-1 pin wiring, R3-2 base conflict, R3-3 branch order,
R3-4 no-row mode fallback, R3-5 recorded base_sha, R3-6 controller pr base_sha,
R3-7 implement-kind filter, R3-8 claim-epoch filter.
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