Skip to content

A resumed pass takes the implementer seat before the step loop - #106

Merged
abedegno merged 1 commit into
mainfrom
fix/resume-implementer-generation
Sep 26, 2026
Merged

abedegno merged 1 commit into
mainfrom
fix/resume-implementer-generation

Conversation

@abedegno

@abedegno abedegno commented Sep 26, 2026 •

Copy link
Copy Markdown
Owner

What and why

Live on muesli #768 this morning, after the plan passed and the implementation reached pull request #777: every wave re-reviewed the same head and recorded nothing, spending a full codex review each time.

The journal shows two refusals in a row:

record_implementation_output  refused: only an attempt dispatched in the implementer role may record an implementation output
record_review                 refused: review binds artifact dc9e4c61…, but this run's current output is 7fc35aa0…

A wave that resumes a run already at implementing enters the step loop holding the runner's resume fence, which is an operator attempt. The step loop records the implementation output under it, and the kernel correctly refuses: only an implementer may record an output. _kernel_record_output still echoes the hash it stored, so the review then binds a blob the kernel never accepted as the run's output, and that is refused too. No verdict lands, next_step says review again, the loop waits, and the next wave repeats it. The fix-wave guard that stops a refused verdict being reported as a silent reviewer did its job, which is why this showed up as a quiet wait rather than a misleading park.

Two changes:

  • The root cause. _resume_back_half dispatches the implementer after the seats are chosen and before _step_loop runs, so a resumed pass holds the same kind of generation a first pass does. First passes are unchanged: they already hold the implementer generation at that point. An implementer dispatch costs no seat; the budget counts authors and reviewers.
  • The second refusal, independently. The review's binding is now read back from the kernel after the output is recorded, rather than taken from the helper's echo. A stored-but-refused output can then never be what a verdict binds, whatever the reason it was refused.

Effect on the merge gate

None. Nothing about what a merge requires changes. A verdict still has to bind the artifact the kernel holds, which is precisely the check that refused the bad binding; this change makes the runner present the right binding instead of a wrong one.

Testing

  • bash batch/run-queue.sh --self-test passes on Linux
  • New behaviour I depend on has a self-test case
  • Where there is a guard, I showed it going red as well as green

Python suite 1992 passed, 3 skipped; --self-test rc 0 on macOS. CI ran the self-test on Linux for this branch and it passed (6m21s).

Two new tests, each mutation-checked: removing the resume-point dispatch fails test_a_resumed_pass_takes_the_implementer_seat_before_the_loop, and binding the helper's echo instead of the kernel's answer fails test_a_refused_output_leaves_the_review_bound_to_what_the_kernel_holds. The step-loop test's kernel stubs now model the kernel faithfully: an accepted output becomes the current artifact, a refused one does not. The end-to-end run_item harness gains the read-back call in its expected sequence and a stub for it; no generation numbers move, because first passes gain no dispatch.

Anything a reviewer should look at twice

  • The first version of this fix dispatched inside the step loop instead, which shifted every first pass's generations and broke thirteen sequence assertions for no benefit. Moving it to the resume point is both smaller and more precise; it is worth checking that no other entry into _step_loop arrives at implementing without an implementer generation. I found none: a repair round dispatches one, and a recorded review redispatches one.
  • A latent limit worth knowing about, not changed here. Reviewer dispatches count against the seat budget, so the back half's repair loop is unbounded in rounds but still bounded in total reviews by max_seats. That will surface on a long enough run.

@abedegno
abedegno merged commit 7cb65b6 into main Sep 26, 2026
2 checks passed
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