webgrip/ploeg

Archived

Fork 0

fix(shiftengine,store): a failed writing Run re-opens its Round #36

Merged

ryangr0 merged 1 commit from ryangr0/failed-writer-advances into development

2026-08-08 21:50:19 +00:00

AGit

ryangr0 commented

2026-08-08 06:54:41 +00:00

Owner

shiftengine.evaluate froze the plan on a stuck Outcome and had no case for failed. A Run the sweeper reclaimed is, as far as RoundComplete is concerned, simply finished — so the Round completed and the next one opened over work that was never done.

round | role     | writes | outcome          | failure_reason
    2 | builder  | t      | failed           | lease_lost
    3 | reviewer | f      | no_change_needed | -          verdict=approve

shifts.close_reason = "review_approved"     pull requests opened = 0

The reviewer reviewed a branch that had never been written, approved it, and the Shift closed with the most reassuring reason there is, having produced nothing. The tracker comment told the truth — "Ploeg stopped working this item without opening a pull request" — and the two disagreed.

The distinction the spec was missing

The shift-orchestration spec says "a swept Run does not block its Round forever … the Round can complete and the Shift advances." That was reasoned about readers, and for a reader it is right: a dead reader costs an opinion, and stalling an item over a missing opinion is worse. For a writer the same rule means every later Round reasons about a branch that does not exist.

failed is the sweeper's verdict on a pod that stopped renewing — never an agent's report. That is what makes it retryable, and what separates it from stuck (R4).

The fix

A failed writing Role re-opens the round the Shift is already on.

In place, because shifts.round doubles as the index into the plan (tp.Rounds[si.Round]). Opening a fresh Round to retry would consume the slot belonging to the next planned Round and silently skip it — and that bug, a reviewer that never runs, is harder to see than the one being fixed.

store.ReopenRound inserts a pending Run at the current round number without incrementing it. RoundComplete keys on the Shift's current round, so the Round correctly becomes incomplete again.
The attempt count is derived from the Runs in that (shift, round, role), never stored — the discipline ADR-0012 sets for reserved.
At store.MaxRunAttempts the Shift closes at needs_human with close_reason = writing_run_failed_repeatedly.
Readers are untouched. The spec's swept-Run scenario stands, narrowed to what it was always about.

Planned Shifts only. Under uniform dispatch a synthesized one-writer Shift already settles by its run's own Outcome, and failed maps to queued — the attempt-capped requeue R5 requires. There is no later Round there to step over, which is why TestUniform_FailedRequeuesAndRespectsTheAttemptCap passes unchanged.

MaxRunAttempts is deliberately separate from MaxAttempts: work_items.attempts increments per role claim, so it stopped meaning "attempts at this work" the day Shifts landed, and reusing it would let a three-role plan exhaust its budget in one clean pass.

Verification

Unit — go test ./pkg/shiftengine/, all three proven to fail against the unfixed engine first:

--- FAIL: TestFailedWriter_ReopensItsOwnRound
    the Shift closed ("plan_exhausted") instead of retrying a writer that never wrote anything
--- FAIL: TestFailedWriter_ParksTheShiftAtTheAttemptCap
    attempt 2: nothing to claim: no claimable work item

End to end — ploeg-bench's hang scenario SIGKILLs the worker mid-run so the lease genuinely lapses (sleeping does not: the worker renews on its own goroutine while the agent works). Against this build:

Shift closed after 56s: 3 Rounds, close_reason="review_approved"
L1 conformance: 17 passed, 0 failed, 2 skipped

round | role     | writes | outcome
    2 | builder  | t      | failed      ← the pod died
    2 | builder  | t      | pr_opened   ← the retry, same round, plan index intact
    3 | reviewer | f      | no_change_needed

All seven bench scenarios green, including fix-cap, budget-floor and reader-push.

Also here

ADR-0019 records the decision, its bounds, and the three options rejected.
The spec gains a writer requirement and narrows the swept-Run scenario to readers.
architecture.md §4 and §9.19 corrected; backlog #116 closed.

Numbered 0019 because #35 already claims 0018.

🤖 Generated with Claude Code

`shiftengine.evaluate` froze the plan on a `stuck` Outcome and had **no case for `failed`**. A Run the sweeper reclaimed is, as far as `RoundComplete` is concerned, simply finished — so the Round completed and the next one opened over work that was never done. ``` round | role | writes | outcome | failure_reason 2 | builder | t | failed | lease_lost 3 | reviewer | f | no_change_needed | - verdict=approve shifts.close_reason = "review_approved" pull requests opened = 0 ``` The reviewer reviewed a branch that had never been written, approved it, and the Shift closed with the most reassuring reason there is, having produced nothing. The tracker comment told the truth — *"Ploeg stopped working this item without opening a pull request"* — and the two disagreed. ## The distinction the spec was missing The shift-orchestration spec says *"a swept Run does not block its Round forever … the Round can complete and the Shift advances."* That was reasoned about **readers**, and for a reader it is right: a dead reader costs an opinion, and stalling an item over a missing opinion is worse. For a **writer** the same rule means every later Round reasons about a branch that does not exist. `failed` is the sweeper's verdict on a pod that stopped renewing — never an agent's report. That is what makes it retryable, and what separates it from `stuck` (R4). ## The fix A failed writing Role re-opens **the round the Shift is already on**. In place, because `shifts.round` doubles as the index into the plan (`tp.Rounds[si.Round]`). Opening a fresh Round to retry would consume the slot belonging to the next planned Round and silently skip it — and *that* bug, a reviewer that never runs, is harder to see than the one being fixed. - `store.ReopenRound` inserts a pending Run at the current round number without incrementing it. `RoundComplete` keys on the Shift's current round, so the Round correctly becomes incomplete again. - The attempt count is **derived** from the Runs in that `(shift, round, role)`, never stored — the discipline ADR-0012 sets for `reserved`. - At `store.MaxRunAttempts` the Shift closes at `needs_human` with `close_reason = writing_run_failed_repeatedly`. - Readers are untouched. The spec's swept-Run scenario stands, narrowed to what it was always about. **Planned Shifts only.** Under uniform dispatch a synthesized one-writer Shift already settles by its run's own Outcome, and `failed` maps to `queued` — the attempt-capped requeue R5 requires. There is no later Round there to step over, which is why `TestUniform_FailedRequeuesAndRespectsTheAttemptCap` passes unchanged. `MaxRunAttempts` is deliberately separate from `MaxAttempts`: `work_items.attempts` increments **per role claim**, so it stopped meaning "attempts at this work" the day Shifts landed, and reusing it would let a three-role plan exhaust its budget in one clean pass. ## Verification Unit — `go test ./pkg/shiftengine/`, all three proven to fail against the unfixed engine first: ``` --- FAIL: TestFailedWriter_ReopensItsOwnRound the Shift closed ("plan_exhausted") instead of retrying a writer that never wrote anything --- FAIL: TestFailedWriter_ParksTheShiftAtTheAttemptCap attempt 2: nothing to claim: no claimable work item ``` End to end — `ploeg-bench`'s `hang` scenario SIGKILLs the worker mid-run so the lease genuinely lapses (sleeping does not: the worker renews on its own goroutine while the agent works). Against this build: ``` Shift closed after 56s: 3 Rounds, close_reason="review_approved" L1 conformance: 17 passed, 0 failed, 2 skipped round | role | writes | outcome 2 | builder | t | failed ← the pod died 2 | builder | t | pr_opened ← the retry, same round, plan index intact 3 | reviewer | f | no_change_needed ``` All seven bench scenarios green, including `fix-cap`, `budget-floor` and `reader-push`. ## Also here - **ADR-0019** records the decision, its bounds, and the three options rejected. - The spec gains a writer requirement and narrows the swept-Run scenario to readers. - `architecture.md` §4 and §9.19 corrected; backlog #116 closed. Numbered 0019 because #35 already claims 0018. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

ryangr0 added 1 commit

2026-08-08 06:54:41 +00:00

docs: a failed writing Run does not stop the plan

On Pull Request / checks (pull_request) Successful in 47s

Details

41eb7a627f

shiftengine.evaluate freezes the plan when a Run reports stuck and has
no case for failed. A Run the sweeper reclaimed is, to RoundComplete,
simply finished — so the Round completes and the next Round opens over
work that was never done.

Observed in the bench's crash drill, which SIGKILLs the worker so the
lease genuinely lapses (sleeping does not: the worker renews on its own
goroutine while the agent works, so a slow agent keeps its lease alive
indefinitely and merely ends up stuck). The writing Round's only Run
died, the engine advanced, the reviewer reviewed a branch that had never
been written, approved it, and the Shift closed review_approved with no
pull request — while the tracker comment correctly said Ploeg had
stopped without opening one. The two disagree, and the reassuring one is
the close reason an operator greps for.

Records the divergence and the observation only. The fix is a decision —
retry the writing Round, or park at needs_human — and it interacts with
attempts being incremented per role claim, so it is not a one-liner.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

ryangr0 added 1 commit

2026-08-08 12:40:42 +00:00

fix(shiftengine,store): a failed writing Run re-opens its Round

On Pull Request / checks (pull_request) Successful in 2m47s

Details

11963fb269

evaluate froze the plan on a stuck Outcome and had no case for failed.
A Run the sweeper reclaimed is, to RoundComplete, simply finished — so
the Round completed and the next one opened over work that was never
done: the reviewer reviewed a branch that had never been written,
approved it, and the Shift closed review_approved with no pull request.

The spec's "a swept Run does not block its Round forever" was reasoned
about READERS, and for a reader it still holds — a missing opinion is
not worth stalling an item over. The distinction it was missing is
`writes`.

A failed writing Role now re-opens the round the Shift is ALREADY on.
In place, because shifts.round doubles as the index into the plan
(tp.Rounds[si.Round]): opening a fresh Round to retry would silently
skip the next planned one, and the resulting bug — a reviewer that never
runs — is harder to see than the one this fixes. The attempt count is
derived from the Runs in that (shift, round, role), never stored, which
is the discipline ADR-0012 sets for reserved. At MaxRunAttempts the
Shift parks at needs_human naming the repeated failure.

Planned Shifts only. Under uniform dispatch a synthesized one-writer
Shift already settles by its run's own Outcome, and failed maps to
queued — the attempt-capped requeue R5 requires. There is no later Round
there to step over, which is why TestUniform_FailedRequeues keeps
passing unchanged.

MaxRunAttempts is deliberately separate from MaxAttempts:
work_items.attempts increments per role claim, so it stopped meaning
"attempts at this work" the day Shifts landed, and reusing it would have
let a three-role plan exhaust its budget in one clean pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

gitea_admin changed title from ~~docs: a failed writing Run does not stop the plan~~ to fix(shiftengine,store): a failed writing Run re-opens its Round

2026-08-08 12:41:12 +00:00

ryangr0 force-pushed ryangr0/failed-writer-advances from 11963fb269

On Pull Request / checks (pull_request) Successful in 2m47s

Details

to 107d5f760f

On Pull Request / checks (pull_request) Successful in 3m24s

Details

2026-08-08 13:23:56 +00:00

Compare

ryangr0 merged commit a89abd3b12 into development

2026-08-08 21:50:19 +00:00

ryangr0 referenced this pull request from a commit