review-state update cannot write phase or step, so wf-implement's fast-path resume never fires #487

New issue

Closed

opened 2026-08-31 08:46:16 +00:00 by mph · 2 comments

mph commented

2026-08-31 08:46:16 +00:00

Owner

wf-implement Step 1 ("Fast Path Resume") resumes from state.phase and state.step. Neither
field can be written by iw review-state update, so no workflow run ever produces a state that
Step 1 accepts, and every resume falls through to the slow path.

Observed

The update flag list carries no --phase and no --step:

--status  --display-text  --display-subtext  --display-type  --message
--artifact  --append-artifact  --action  --append-action  --pr-url
--checkpoint  --activity  --needs-attention  --issue-id  --workflow-type
--version  --output  --from-stdin

Confirmed end to end on MEDECA-447 (CMI/mdr/medeca-modul-poptavky). After a full waterfall phase
— context, tasks, implementation, review, PR — review-state.json held:

"status": "awaiting_review",
"phase":  null,
"step":   null

Step 1's sanity check ("phase file existence matches state.step", "first unchecked phase in
tasks.md matches state.phase") cannot pass against null, so the run fell back to Step 2
auto-detection: re-read the branch, re-query the forge for the phase MR, re-run phase-advance.

Effect

Resume is correct but never fast. Step 1 is dead code in practice: it is written, documented and
executed, and its only reachable outcome is the fallback. Two consequences:

every resume pays a forge round-trip and a full detection pass it was designed to skip;
Step 1's sanity checks are never exercised, so a real inconsistency they were meant to catch
would be caught (if at all) by Step 2's looser reasoning instead.

Expected

Either review-state update gains --phase / --step and the wf-* command blocks set them at
each transition, or Step 1 stops reading fields nothing writes and resumes from what the state
actually carries (status plus phase_checkpoints). The current arrangement documents a fast path
that cannot fire.

Distinct from #436, which lists five review-state flag/update defects but not this one.

Found during MEDECA-447 (medeca-modul-poptavky); recorded in that issue's LESSONS.md and promoted
at triage.

`wf-implement` Step 1 ("Fast Path Resume") resumes from `state.phase` and `state.step`. Neither field can be written by `iw review-state update`, so no workflow run ever produces a state that Step 1 accepts, and every resume falls through to the slow path. ## Observed The `update` flag list carries no `--phase` and no `--step`: ``` --status --display-text --display-subtext --display-type --message --artifact --append-artifact --action --append-action --pr-url --checkpoint --activity --needs-attention --issue-id --workflow-type --version --output --from-stdin ``` Confirmed end to end on MEDECA-447 (`CMI/mdr/medeca-modul-poptavky`). After a full waterfall phase — context, tasks, implementation, review, PR — `review-state.json` held: ```json "status": "awaiting_review", "phase": null, "step": null ``` Step 1's sanity check ("phase file existence matches `state.step`", "first unchecked phase in tasks.md matches `state.phase`") cannot pass against `null`, so the run fell back to Step 2 auto-detection: re-read the branch, re-query the forge for the phase MR, re-run `phase-advance`. ## Effect Resume is correct but never fast. Step 1 is dead code in practice: it is written, documented and executed, and its only reachable outcome is the fallback. Two consequences: - every resume pays a forge round-trip and a full detection pass it was designed to skip; - Step 1's sanity checks are never exercised, so a real inconsistency they were meant to catch would be caught (if at all) by Step 2's looser reasoning instead. ## Expected Either `review-state update` gains `--phase` / `--step` and the `wf-*` command blocks set them at each transition, or Step 1 stops reading fields nothing writes and resumes from what the state actually carries (`status` plus `phase_checkpoints`). The current arrangement documents a fast path that cannot fire. Distinct from #436, which lists five review-state flag/update defects but not this one. --- Found during MEDECA-447 (medeca-modul-poptavky); recorded in that issue's `LESSONS.md` and promoted at triage.

mph referenced this issue from a commit

2026-09-02 06:22:12 +00:00

fix(review-state): let update record the workflow's phase, step and branch

mph referenced this issue

2026-09-02 06:22:57 +00:00

Squash the batch-implement blockers: nine fixes for the phase gate, the CI gate and the review-state card #501

mph commented

2026-09-02 20:55:30 +00:00

Author

Owner

The CLI half landed in PR #501 (merge d1eba9c4): review-state update gains --phase, --step and --branch, plus --clear-* for each. Values are stored verbatim as JSON strings, so --phase 3 writes "3". The schema accepts integer-or-string for phase so older numeric cards still validate.

Note the scope was widened beyond this issue on purpose: --branch was added as well, because wf-implement Step 1 sanity-checks state.branch (wf-implement.md:69) alongside state.step (:71) and state.phase (:72). Restoring only two of the three would have left the fast path falling through anyway.

Deliberately left open. Nothing yet writes these fields. The wf-* command blocks live in the kanon plugin, outside this repo, and must set them at each transition before Step 1 can ever fire. Until then this issue is exactly half fixed: the card can record the position, but nothing asks it to. Guidance has been sent to the kanon session.

The CLI half landed in PR #501 (merge d1eba9c4): `review-state update` gains `--phase`, `--step` and `--branch`, plus `--clear-*` for each. Values are stored verbatim as JSON strings, so `--phase 3` writes `"3"`. The schema accepts integer-or-string for `phase` so older numeric cards still validate. Note the scope was widened beyond this issue on purpose: `--branch` was added as well, because `wf-implement` Step 1 sanity-checks `state.branch` (wf-implement.md:69) alongside `state.step` (:71) and `state.phase` (:72). Restoring only two of the three would have left the fast path falling through anyway. **Deliberately left open.** Nothing yet writes these fields. The `wf-*` command blocks live in the kanon plugin, outside this repo, and must set them at each transition before Step 1 can ever fire. Until then this issue is exactly half fixed: the card can record the position, but nothing asks it to. Guidance has been sent to the kanon session.

mph referenced this issue

2026-09-02 22:13:37 +00:00

review-state write cannot record phase, step or branch, so the fast path stays dead at analysis_ready #505

claude commented

2026-10-15 22:46:05 +00:00

Member

Intake: fixed by d1eba9c4

Both halves of the fix are on main:

iw-cli d1eba9c4 (PR #501): review-state update gains --phase, --step and --branch, with --clear-* flags (core/commands/ReviewState.scala:305-318, :513-526).
kanon a2743b4 ("docs(workflows): ... record the workflow position"), on kanon origin/main: the wf-* blocks now write --phase, --step and --branch at each transition (for example blueprints/snippets/review_state_tasks_ready.md:8-10, review_state_context_ready.md, review_state_phase_tasks_ready.md, implementation-workflow-pipeline.md, skills/wf-implementation-workflow/SKILL.md). commands/wf-implement.md Step 1 compares state.phase as text, which matches the string values the CLI writes.

The half that the 2026-09-02 comment left open has landed, so nothing in this repository is still open. Reopen if a resume still falls through to Step 2.

Intake: fixed by d1eba9c4 Both halves of the fix are on main: - iw-cli d1eba9c4 (PR #501): `review-state update` gains `--phase`, `--step` and `--branch`, with `--clear-*` flags (`core/commands/ReviewState.scala:305-318`, `:513-526`). - kanon a2743b4 ("docs(workflows): ... record the workflow position"), on kanon `origin/main`: the `wf-*` blocks now write `--phase`, `--step` and `--branch` at each transition (for example `blueprints/snippets/review_state_tasks_ready.md:8-10`, `review_state_context_ready.md`, `review_state_phase_tasks_ready.md`, `implementation-workflow-pipeline.md`, `skills/wf-implementation-workflow/SKILL.md`). `commands/wf-implement.md` Step 1 compares `state.phase` as text, which matches the string values the CLI writes. The half that the 2026-09-02 comment left open has landed, so nothing in this repository is still open. Reopen if a resume still falls through to Step 2.

claude closed this issue

2026-10-15 22:46:05 +00:00