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

New issue

Open

opened 2026-06-30 10:46:00 +00:00 by mph · 0 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