iterative-works/iw-cli

Watch

Fork 0

batch-implement: ci_pending/ci_fixing should poll for CI, not consume the retry budget #400

New issue

Closed

opened 2026-06-30 10:46:00 +00:00 by mph · 2 comments

mph commented

2026-06-30 10:46:00 +00:00

Owner

Problem

The batch-implement orchestrator conflates two unrelated situations and draws
both from the same scarce retry budget:

"re-implement" — something went wrong, re-dispatch the implementer (e.g.
implementing, review_failed, context_ready).
"wait for CI" — nothing is wrong, the phase MR is open and CI is running
or being fixed (ci_pending, ci_fixing).

core/model/BatchImplement.scala maps both to PhaseOutcome.Recover:

// phase-merge-internal statuses: it is waiting on or fixing CI. Recover by
// re-invoking implementation so the orchestrator self-heals, rather than
// failing the batch on a transient.
case "ci_pending" => PhaseOutcome.Recover
case "ci_fixing"  => PhaseOutcome.Recover

Recover re-runs the implementer (the wrong tool for "CI is running") and
consumes one unit of the --max-retries budget (default 1). So a phase that
legitimately needs to wait for CI can fail with
Exhausted N recovery attempts; still in status 'ci_pending' — especially if an
earlier transient already spent the budget.

Observed failure

Seen in the field (kanon iw-plugin/commands/batch-implement.scala driving a
waterfall phase against a Forgejo/GitLab forge):

First claude session terminated early (the print-mode background-wait
ceiling — fixed separately downstream), leaving status implementing.
implementing → Recover → recovery attempt 1/1 (the only retry spent),
which opened the phase MR and reached ci_pending.
ci_pending → Recover again, but 0 retries left → batch failed, even
though CI was simply still running.

The downstream fix (making the session wait for the backgrounded CI watch)
makes this far less likely to trigger, but the design wart remains: a "CI is
running" status should never consume an implementation retry.

Root cause

There is no distinct "poll the forge until CI concludes" path. The orchestrator
only knows three moves — merge, re-dispatch the implementer (Recover), or fail
— so the CI-wait case is forced through Recover and is rate-limited by a
budget meant for genuine recovery.

The plumbing to do this right already exists: queryForgeState (and the
phase-advance catch-up logic) can poll the forge for MR/CI state without
re-dispatching the implementer.

Proposed direction (not prescribing)

Give ci_pending/ci_fixing their own outcome (e.g. PhaseOutcome.AwaitCI)
distinct from Recover.
Handle it in the orchestrator loop by polling the forge (reuse
queryForgeState) on an interval until CI concludes — merged → mark done,
failed → hand back to claude to fix in-context, still running → keep waiting —
bounded by the per-phase --claude-timeout, not by --max-retries.
Keep --max-retries exclusively for genuine re-implementation recovery.

Acceptance criteria

A phase whose MR is open with CI running/fixing does not consume the
--max-retries budget and is not failed as "Exhausted recovery attempts"
while CI is still in progress.
CI failure is still surfaced and handed back for an in-context fix.
The per-phase timeout remains the authoritative ceiling for the CI wait.

References

Sibling/just-fixed downstream issue: claude print-mode background-wait ceiling
terminating the in-agent CI watch (fixed in kanon batch-implement.scala).
IW-398 — stable iw-cli core API (so consumers like kanon's batch-implement
track a published surface).
core/model/BatchImplement.scala decideOutcome; kanon
iw-plugin/commands/batch-implement.scala orchestration loop.

## Problem The batch-implement orchestrator conflates two unrelated situations and draws both from the same scarce retry budget: - **"re-implement"** — something went wrong, re-dispatch the implementer (e.g. `implementing`, `review_failed`, `context_ready`). - **"wait for CI"** — nothing is wrong, the phase MR is open and CI is running or being fixed (`ci_pending`, `ci_fixing`). `core/model/BatchImplement.scala` maps both to `PhaseOutcome.Recover`: ```scala // phase-merge-internal statuses: it is waiting on or fixing CI. Recover by // re-invoking implementation so the orchestrator self-heals, rather than // failing the batch on a transient. case "ci_pending" => PhaseOutcome.Recover case "ci_fixing" => PhaseOutcome.Recover ``` `Recover` re-runs the **implementer** (the wrong tool for "CI is running") and consumes one unit of the `--max-retries` budget (default **1**). So a phase that legitimately needs to wait for CI can fail with `Exhausted N recovery attempts; still in status 'ci_pending'` — especially if an earlier transient already spent the budget. ## Observed failure Seen in the field (kanon `iw-plugin/commands/batch-implement.scala` driving a waterfall phase against a Forgejo/GitLab forge): 1. First claude session terminated early (the print-mode background-wait ceiling — fixed separately downstream), leaving status `implementing`. 2. `implementing` → `Recover` → recovery attempt 1/1 (the only retry spent), which opened the phase MR and reached `ci_pending`. 3. `ci_pending` → `Recover` again, but 0 retries left → batch failed, even though CI was simply still running. The downstream fix (making the session wait for the backgrounded CI watch) makes this far less likely to trigger, but the design wart remains: a "CI is running" status should never consume an implementation retry. ## Root cause There is no distinct "poll the forge until CI concludes" path. The orchestrator only knows three moves — merge, re-dispatch the implementer (`Recover`), or fail — so the CI-wait case is forced through `Recover` and is rate-limited by a budget meant for genuine recovery. The plumbing to do this right already exists: `queryForgeState` (and the `phase-advance` catch-up logic) can poll the forge for MR/CI state without re-dispatching the implementer. ## Proposed direction (not prescribing) - Give `ci_pending`/`ci_fixing` their own outcome (e.g. `PhaseOutcome.AwaitCI`) distinct from `Recover`. - Handle it in the orchestrator loop by polling the forge (reuse `queryForgeState`) on an interval until CI concludes — merged → mark done, failed → hand back to claude to fix in-context, still running → keep waiting — bounded by the per-phase `--claude-timeout`, **not** by `--max-retries`. - Keep `--max-retries` exclusively for genuine re-implementation recovery. ## Acceptance criteria - A phase whose MR is open with CI running/fixing does not consume the `--max-retries` budget and is not failed as "Exhausted recovery attempts" while CI is still in progress. - CI failure is still surfaced and handed back for an in-context fix. - The per-phase timeout remains the authoritative ceiling for the CI wait. ## References - Sibling/just-fixed downstream issue: claude print-mode background-wait ceiling terminating the in-agent CI watch (fixed in kanon `batch-implement.scala`). - IW-398 — stable iw-cli core API (so consumers like kanon's batch-implement track a published surface). - `core/model/BatchImplement.scala` `decideOutcome`; kanon `iw-plugin/commands/batch-implement.scala` orchestration loop.

mph referenced this issue

2026-08-21 17:27:45 +00:00

phase-merge: false CI timeout on a green PR — ci_deadline is inherited from an earlier gate, ci_gated_sha goes stale #433

mph referenced this issue from a commit

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

fix(batch): give a CI wait its own phase outcome

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 iw-cli half landed in PR #501 (merge d1eba9c4): BatchImplement.decideOutcome now returns a new PhaseOutcome.AwaitCI for ci_pending and ci_fixing, so a CI wait no longer draws on the recovery budget.

Deliberately left open. The orchestrator that consumes PhaseOutcome is the kanon plugin, outside this repo, and it is not yet patched. Until it is, the reported symptom is not gone — it is replaced by a worse one: iw-plugin/commands/batch-implement.scala handleOutcome matches four cases with no wildcard and nothing sets -Xfatal-warnings, so it compiles and then throws scala.MatchError: AwaitCI on the first ci_pending.

The plugin needs case PhaseOutcome.AwaitCI => polling via ./iw ci-gate (exit 3 = call again — Michal decided this over a new --ci-timeout), plus a REQUIRES: iw-cli >= bump. Guidance has been sent to the kanon session.

This couples the repos in both directions — new iw-cli with old plugin throws at runtime; old iw-cli with new plugin does not compile. Close this only when the plugin has adopted the case and the two releases are sequenced.

The iw-cli half landed in PR #501 (merge d1eba9c4): `BatchImplement.decideOutcome` now returns a new `PhaseOutcome.AwaitCI` for `ci_pending` and `ci_fixing`, so a CI wait no longer draws on the recovery budget. **Deliberately left open.** The orchestrator that consumes `PhaseOutcome` is the kanon plugin, outside this repo, and it is not yet patched. Until it is, the reported symptom is not gone — it is replaced by a worse one: `iw-plugin/commands/batch-implement.scala` `handleOutcome` matches four cases with no wildcard and nothing sets `-Xfatal-warnings`, so it compiles and then throws `scala.MatchError: AwaitCI` on the first `ci_pending`. The plugin needs `case PhaseOutcome.AwaitCI =>` polling via `./iw ci-gate` (exit 3 = call again — Michal decided this over a new `--ci-timeout`), plus a `REQUIRES: iw-cli >=` bump. Guidance has been sent to the kanon session. **This couples the repos in both directions** — new iw-cli with old plugin throws at runtime; old iw-cli with new plugin does not compile. Close this only when the plugin has adopted the case and the two releases are sequenced.

claude referenced this issue

2026-09-15 02:46:23 +00:00

phase-pr cannot tell a forge that is down from a forge that refused, so both stop a batch #513

claude commented

2026-09-21 11:23:42 +00:00

Member

Done: the code already does what this issue asks.

core/model/BatchImplement.scala has a distinct PhaseOutcome.AwaitCI, and decideOutcome maps ci_pending and ci_fixing to it, not to Recover.
iw-plugin/lib/batch/CiGate.scala awaitCi recurses on PollAgain and KilledByGuard without a decrement. Only GateVerdict.Red calls PhaseRecovery.recoveryAttempt and passes recoveryAttemptsLeft.decremented.

So a wait costs no retry, and a fix costs one, which is the behaviour this issue proposed. #566 moved the wait out of the phase session and landed the rest.

Closing as implemented.

Done: the code already does what this issue asks. - `core/model/BatchImplement.scala` has a distinct `PhaseOutcome.AwaitCI`, and `decideOutcome` maps `ci_pending` and `ci_fixing` to it, not to `Recover`. - `iw-plugin/lib/batch/CiGate.scala` `awaitCi` recurses on `PollAgain` and `KilledByGuard` without a decrement. Only `GateVerdict.Red` calls `PhaseRecovery.recoveryAttempt` and passes `recoveryAttemptsLeft.decremented`. So a wait costs no retry, and a fix costs one, which is the behaviour this issue proposed. #566 moved the wait out of the phase session and landed the rest. Closing as implemented.

claude closed this issue

2026-09-21 11:23:42 +00:00