US-45.4: merge gate and stage machine speak checkpoints (v3, mode bound at placement) - #908
Merged
Merged
Conversation
… 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.
…ase 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.
…nly 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.
…im'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.
mkreyman
enabled auto-merge (squash)
September 27, 2026 03:37
This was referenced Sep 28, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
US-45.4: the merge gate and stage machine speak checkpoints (v3)
This replaces #906, which was closed after its round-3 review still found material defects. Most of them came from one design choice: the gate read the intake source's CURRENT mode. So a mode change made while a story was in flight changed which route that story was judged on. #906 tried to prevent that with a
stories_in_flight409 on update and enrol. That refusal had race and settled-stage holes of its own.The decision behind v3: the mode is bound per implement dispatch at placement.
DispatchLedger.record_sent/3copies the source'smodeonto the newrunner_dispatches.modecolumn (migration20260926150000, nullable; a NULL row reads aspr, the only route legacy rows had).claim_epoch, statusacceptedorsuperseded, and the shared implement-kind filter. That read isDispatchPayload.dispatch_route/3.stories_in_flight409, the in-flight set,previous_modeandmax_blocking_namedare deleted, along with their docs and tests.docs/agent-delivery-loop.mdall say it.Also in v3
merge_base_shato equal the base head (base_sha). Otherwise the gate answersbase_moved_since_checkpoint, routed like the other moved-head reasons: with a live claim it goes back to implementing to rebase; without one it is refused withclaim_not_live. Both routes are tested.contains?error that is not transient (a 404 included) falls through to judging the checkpoint as open. Only a transient error surfaces asunevaluated.base_shaorcheckpoint_id) records a neweffect_recordedevent. The latest event therefore always matches the returned verdict.Stages.put_effectrefreshes the event only when the payload differs.dispatch_route/3: the stage's recordedbrancheffect, then the ledger row, thenbranch_for. Each step has a test.Threads.claim_checkpointsis one query (a story left-joined to its checkpoints), selects only the fields it needs, and does nogate_evidenceload and noexists?query.[:loopctl, :delivery, :dispatch_route_busy].commit/2returns the tree only; the function isdispatch_route/3;GitHubPullRequestSource's reads are described without counting them.Not covered by a test
dispatch_route/3: no test fires thedispatch_route_busytelemetry. It goes through the sameStages.answering_busy/4seam that other reads already exercise.Gate
mix precommitran through the commit hook: 11607 tests, 0 failures (88 excluded); credo and dialyzer clean. MCPnode --testis green.Review
The review gate is still to run.
Mutation table
Every mutation below was run with
~/workspace/claude-config/bin/mutate.sh <file> --old ... --new ... -- mix test <check>(ornode --testfor the W-series). Exit 0 means the check went red under the mutation. Wiring mutations are included: V3-02, 04, 08, 13, 16, 22, 28, 35, 37, 46, 49, 52, 60, 64 and 66.The first V3-11 was a mutation that could not change behaviour: it added a clause guarded by
when false, which never matches. It exited 1 for that reason, not because the test was weak. It was replaced with the real regression (the gate following the source's CURRENT mode), which exited 0. V3-41's first run was refused (exit 2) because its target text did not match. It was retargeted and exited 0.Review round 3 (the ceiling), fixed in place
Round 3 found 8 findings. Four shared one cause, placement-bound facts read partially from the claim's ledger row, so they were fixed in place rather than rewritten, following #902's round 3. No round 4.