Skip to content

A run stranded at merge_requested can record what it observes - #105

Merged
abedegno merged 1 commit into
mainfrom
fix/observe-at-merge-requested
Sep 20, 2026
Merged

abedegno merged 1 commit into
mainfrom
fix/observe-at-merge-requested

Conversation

@abedegno

@abedegno abedegno commented Sep 20, 2026 •

Copy link
Copy Markdown
Owner

What and why

The second wave after the closed loop deployed got one state further and stopped again, for the same reason in a different place.

A run stranded at merge_requested — a pass that died between the merge request and the merge — is resumed by the loop and derived. It now finds its pull request (#104) and sees it merged. It then tries to record that:

record_ci_observation {"status":"success","head_git_sha":"ae9cf74…","pr_state":"merged","pr":"730"}
  -- refused: record_ci_observation is not legal from state 'merge_requested';
     legal from ['implementing', 'planned', 'reviewing']

So latest_pr_state stays unobserved, the ending rule reads that as open and refuses the terminal fact, and the run comes back on the next wave to do it all again. The loop resumes from four states but could only record an observation from three.

This adds merge_requested to that from-set, for the reason the same table already gives for planned: a CI observation is a fact about the world, not about the phase the run is in, and the ending rule depends on its being writable from every state the loop resumes at. The authorisation for a merge happened at request_merge, which is already past, so nothing here can retroactively authorise anything.

To stop the from-set and the resume set drifting apart a third time, the four states are now a named constant, BACK_HALF_DERIVING_STATES, with a test asserting the from-set covers it.

Effect on the merge gate

None. _ci_is_green is consulted when a merge is authorised, at request_merge; these observations are recorded after that point, on a run whose merge was already authorised or already done. No change to _merge_gate, merge_ready_pr, the kernel's merge authorisation, or what evidence any of them require.

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 1990 passed, 3 skipped. No bash changed; CI ran the self-test on Linux for this branch and it passed (6m16s).

Three tests: an observation is accepted from merge_requested; a run there whose pull request merged can then end; and the from-set covers every state the loop derives at. The first two fail without the change — they are the live case above, reduced. The third is the one that matters longest: it fails if anyone adds a resume state without widening the from-set.

Anything a reviewer should look at twice

  • Whether merged and cancelled belong in the set too. They do not: _resume_back_half intercepts both before the step loop runs, so no observation is ever attempted there, and the ending rule does not apply to them anyway.
  • This is the second of two identical near-misses (planned, then merge_requested). Both were invisible to every test and every review, and both surfaced on the first waves after deploy, which is an argument for watching the next few waves rather than trusting the suite.

@abedegno
abedegno merged commit 830f68c into main Sep 20, 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