Self-review: the stop gate blocks on a node the work item never entered
The self/critic review before requesting human review (
reference/reviewing.md). I read the branch diff adversarially, asking two things: what would make CI or a reviewer reject it, and in which direction the gate could now be wrong.
Review cycles
Cycle 1: the ticket's diagnosis against the code
The ticket's
current: nulldoes not reproduce onmain.- The gate reads
currentNode, notcurrent. core.graphs.checknever returns a nullcurrentNodefor a repository that resolves.--recomputesets it to the first unmet node.- The ticket's own table (
WAIT phase-selectionfirst) would have put the gate onphase-selection, which is await, so no block.
I reproduced the symptom a different way: a recorded selection with the pointer still at
phase-selection(bugfix.md§ Steps to reproduce). The bugfix names the real mechanism and says why it differs from the ticket. Both suggested fixes are still taken in substance: fix 1 is the new field, fix 2 is the explicit inconclusive rule.- The gate reads
Fix 1 taken literally would have broken CI. Making
--recomputereport the pointer ascurrentNodechangesokand--fail-onfor every CI run, and CI's contract is "the artifacts alone". I rejected that in favour of a second field (design.md§ Trade-offs).
Cycle 2: the diff
- Found and fixed: a wrong rationale in the first draft. The first
run_checkdocstring and a design bullet said--recomputeis what keeps verdicts from being forged. ReadingRuntime.status: both modes callevaluateidentically. OnlycurrentNode,forcedandparkeddiffer. Both texts now state the real reason, which is that the walk from the graph's start keeps a forward-moved pointer from hiding a broken node. - The walk needs the pointer to be in
nodesfirst. Otherwise a pointer naming no node would walk the whole graph and reach the downstream blocks, which is the bug again by another route. The gate checks membership up front. A test pins it (test_a_pointer_naming_no_node_in_the_report_is_inconclusive). - A running service serves the old report. During verification a local control-plane service that had auto-started on the pre-change code answered without
pointer. The gate then took the R2.3 path,currentNodeas the bound, which is the old behaviour. It did not crash. So an operator's fix takes effect once the service restarts. I noted this in the reviewer briefing rather than coded around it: every CLI change reaching a long-running service has this property.
Checked and left as is
- Forced nodes. A pointer
forced past an unmet node still gets that node demanded. This is unchanged, and it is a finding rather than an invention: the item did walk past it. It is out of scope inbugfix.md. waitbefore the pointer. The first unmet node is a humanwait, so the gate does not look further, even when the pointer node itself isblock. This is the behaviour before this change, whencurrentNodewas that samewaitnode, and it is kept deliberately.--fail-on blockincheck. It still bounds bycurrentNodeand reads no pointer. That is CI's contract and it is unchanged.- The long
_state_linef-string. It is over 88 columns.ruff formatleaves strings alone andruff checkpasses, matching neighbouring lines in the module.
Verdict
Ready for human review. No open findings.