fix(vloer): mark a pull request whose reviewer kept failing as unreviewed #207

Merged

ryangr0 merged 3 commits from ryangr0/replace/195-reviewer-retry into development 2026-10-04 08:38:46 +00:00 AGit

Conversation 0 Commits 3 Files changed 6 +26 -3

ryangr0 commented 2026-10-03 22:42:22 +00:00

Owner

Copy link

Replaces #195 (fix(ploeg): retry a failed reviewer and close review_failed when no review came) after the Ploeg separation in #206. Its Ploeg part is ploeg-hq/ploeg#47; this PR moves Unfold's pin to that branch and carries the rest.

Commits

• 42001ecc build(ploeg): pin ploeg-hq/ploeg#47 to retry a failed reviewer
• 55c57ce4 docs: list review_failed in how to review an agent pull request (from bb9b3861fa)
• d58ff0ac fix(vloer): mark a pull request whose reviewer kept failing as unreviewed (from 146cf69fee)
• a4dabce0 chore(site): re-record the demo replay with the review_failed wording (from c2ced7378a)

Merge order

1. #206, the cutover. Until it merges, this PR's diff also shows the cutover commits it is stacked on.
2. ploeg-hq/ploeg#47.
3. Move the pin in this PR's build(ploeg) commit to the merge commit on Ploeg's main, then merge. Until then the ploeg-pin check is red by design: it requires the pinned commit on Ploeg's main.

Verification (local, at a4dabce0 with Ploeg 376ac60c)

• mise run verify with the result cache, as a pull request runs it: all gates passed. That includes the Ploeg group (its own scripts/verify.sh at the pin), Vloer, the extension, the demo replay check, integration (managed qualification) and docs.
• Ploeg side: GitHub CI passed on ploeg-hq/ploeg#47.

What happened to each commit of #195

Commit	Subject	Here
bb9b3861fa	fix(ploeg): retry a failed reviewer and close review_failed when no review came	Ploeg part ported to ploeg-hq/ploeg#47; Unfold part re-applied as 55c57ce4 ("docs: list review_failed in how to review an agent pull request")
146cf69fee	fix(vloer): mark a pull request whose reviewer kept failing as unreviewed	Unfold part re-applied as d58ff0ac
c2ced7378a	chore(site): re-record the demo replay with the review_failed wording	re-recorded on the new base with mise run demo-record as a4dabce0

The original had no reviews or comments. Its checks were red because development itself failed at mise install --locked; #206 fixes that lock. No earlier check result carries over.

Original description of #195

Implements the accepted ADR-0043 (VIK-1304, handoff EXEC-05).

Problem

When a reading Run failed, retryFailedWriter logged "a reading Run failed; its findings are missing" and the plan moved on. When that reader was the reviewer of a writer's pull request, the Shift closed plan_exhausted and readyForReview settled the item awaiting_review. The tracker comment then said "plan complete" about a review that never happened. This happened to Shift 90 (OOMKilled reviewer) and Shift 118 (ACP idle watchdog). TestFailedReader_StillAdvancesTheRound pinned the old behaviour.

Invariant

• A reading Role whose Runs in a Round all failed is retried in that Round, under the writer's two budgets, before the plan moves on.
• A Shift whose last reading Round after the last writer still has such a Role once the budgets are spent closes review_failed, never plan_exhausted.
• No surface reports that Shift as reviewed, approved or "plan complete".

Change (Ploeg)

• retryFailedRuns handles the writer first, as before (ADR-0019). Otherwise retryFailedReaders reopens every failed reading Role that still has attempts left. It does this in one store.ReopenRound call in the current Round, so the counter does not advance and readers that reported are not re-run.
• Attempts are limited by MaxRunAttempts (agent failures: agent_error, idle, timeout, so an ACP watchdog kill stays an agent failure) and by MaxInfraFailures (infrastructure failures).
• The pool is checked when the retry is claimed. An empty pool opens no Run, and the sweep parks the Shift with the spend named, as for any Run nobody can pay for.
• When the budgets run out, the plan advances. If it then ends plan_exhausted and the last reading Round after the last writer has a Role with no non-failed Outcome, the close reason is review_failed. With a writer's pull request, the item still settles awaiting_review.
• Close message and tracker comment: "Ploeg finished this item, but an agent review of its pull request is missing" / "not reviewed by an agent: reviewer Run failed (agent_error) after 3 attempt(s). Agent review unavailable; a person is asked to review and merge". When another reader of that Round did report, the comment says "not reviewed by every agent … Agent review incomplete".
• Unchanged: a reader that reports without a verdict still closes plan_exhausted, a stuck reader still freezes the plan, and an approval still closes review_approved.
• store.RunReport.FailureReason is new (read from the existing column; no migration).
• The shift-orchestration spec rewords the swept-reader scenario as the ADR asks and adds a requirement for the reader retry. docs/how-to/review-an-agent-pr.md lists review_failed.

Change (Vloer)

In the Ready for review lane, the row chip reads "Agent review unavailable" (attention). The Work Item page says no agent reviewed the pull request. closeReasonLabel('review_failed') reads "No agent reviewed it: the reviewer kept failing". The demo replay is re-recorded because public/ changed.

Tests

New in pkg/shiftengine/failedreader_test.go, against real embedded Postgres. All of them except the stuck and wording tests failed on development before the change:

• TestFailedReader_ReopensItsOwnRound (the ADR's named replacement)
• agent budget ends after 3 attempts for agent_error, idle and timeout
• infrastructure kills use their own budget of 10
• the Shift 118 replay: writer pr_opened and three failed reviewer attempts give review_failed, awaiting_review, and a tracker comment without "plan complete" or "approved"
• a retry that approves (review_approved) or reports without a verdict (plan_exhausted)
• one of two reviewers fails and is retried alone
• a failed analyst before the writer is not a missing review
• a stuck reviewer still freezes the plan
• a retry with an empty pool opens no Run (ErrBudgetExhausted) and is parked
• 8 concurrent evaluators, the sweep and a restarted engine create exactly one retry
• the wording test

TestFailedReader_StillAdvancesTheRound is removed, as the ADR says. TestExpiredReaderDoesNotBlockTheRound becomes TestSweptReaderIsRetriedByTheSweepAndNeverBlocksTheRound. Vloer adds assertions in test/ploeg-view.test.mjs and test/reasons.test.mjs.

Commands (at c2ced737, Go 1.27.1, Node 24.21.0)

• mise exec -- go test ./pkg/shiftengine/ ./pkg/store/ -count=1 -v: 273 top-level tests passed, 0 failed
• mise exec -- openspec validate --all --strict: 17 passed
• mise run demo-record: re-recorded (391 requests)
• mise run verify: all gates passed (vloer 7 incl. npm test 700/700, demo-replay 1, vloer-extension 4, ploeg 7, brand 1, site 5, site-demo 1, helm 15, release 3, integration 1, docs 1)

Not in this PR

• The reviewed SHA surviving retries depends on EXEC-04 (VIK-1738).
• Durable delivery of the tracker comment is AUTH-02's outbox (VIK-1728).
• After deploy, check that the next failed reviewer shows close_reason = review_failed.

Refs VIK-1304

🤖 Generated with Claude Code

🤖 Generated with Claude Code

Replaces #195 (fix(ploeg): retry a failed reviewer and close review_failed when no review came) after the Ploeg separation in #206. Its Ploeg part is [ploeg-hq/ploeg#47](https://github.com/ploeg-hq/ploeg/pull/47); this PR moves Unfold's pin to that branch and carries the rest. ## Commits - `42001ecc` build(ploeg): pin ploeg-hq/ploeg#47 to retry a failed reviewer - `55c57ce4` docs: list review_failed in how to review an agent pull request (from `bb9b3861fa`) - `d58ff0ac` fix(vloer): mark a pull request whose reviewer kept failing as unreviewed (from `146cf69fee`) - `a4dabce0` chore(site): re-record the demo replay with the review_failed wording (from `c2ced7378a`) ## Merge order 1. #206, the cutover. Until it merges, this PR's diff also shows the cutover commits it is stacked on. 2. [ploeg-hq/ploeg#47](https://github.com/ploeg-hq/ploeg/pull/47). 3. Move the pin in this PR's `build(ploeg)` commit to the merge commit on Ploeg's `main`, then merge. Until then the `ploeg-pin` check is red by design: it requires the pinned commit on Ploeg's `main`. ## Verification (local, at `a4dabce0` with Ploeg `376ac60c`) - `mise run verify` with the result cache, as a pull request runs it: all gates passed. That includes the Ploeg group (its own `scripts/verify.sh` at the pin), Vloer, the extension, the demo replay check, integration (managed qualification) and docs. - Ploeg side: GitHub CI passed on [ploeg-hq/ploeg#47](https://github.com/ploeg-hq/ploeg/pull/47). ## What happened to each commit of #195 | Commit | Subject | Here | | --- | --- | --- | | `bb9b3861fa` | fix(ploeg): retry a failed reviewer and close review_failed when no review came | Ploeg part ported to ploeg-hq/ploeg#47; Unfold part re-applied as `55c57ce4` ("docs: list review_failed in how to review an agent pull request") | | `146cf69fee` | fix(vloer): mark a pull request whose reviewer kept failing as unreviewed | Unfold part re-applied as `d58ff0ac` | | `c2ced7378a` | chore(site): re-record the demo replay with the review_failed wording | re-recorded on the new base with `mise run demo-record` as `a4dabce0` | The original had no reviews or comments. Its checks were red because `development` itself failed at `mise install --locked`; #206 fixes that lock. No earlier check result carries over. <details><summary>Original description of #195</summary> Implements the accepted [ADR-0043](apps/ploeg/docs/adrs/0043-a-failed-reading-run-is-retried-and-a-missing-review-closes-review-failed.md) (VIK-1304, handoff EXEC-05). ## Problem When a reading Run failed, `retryFailedWriter` logged "a reading Run failed; its findings are missing" and the plan moved on. When that reader was the reviewer of a writer's pull request, the Shift closed `plan_exhausted` and `readyForReview` settled the item `awaiting_review`. The tracker comment then said "plan complete" about a review that never happened. This happened to Shift 90 (OOMKilled reviewer) and Shift 118 (ACP idle watchdog). `TestFailedReader_StillAdvancesTheRound` pinned the old behaviour. ## Invariant - A reading Role whose Runs in a Round all failed is retried in that Round, under the writer's two budgets, before the plan moves on. - A Shift whose last reading Round after the last writer still has such a Role once the budgets are spent closes `review_failed`, never `plan_exhausted`. - No surface reports that Shift as reviewed, approved or "plan complete". ## Change (Ploeg) - `retryFailedRuns` handles the writer first, as before (ADR-0019). Otherwise `retryFailedReaders` reopens every failed reading Role that still has attempts left. It does this in one `store.ReopenRound` call in the current Round, so the counter does not advance and readers that reported are not re-run. - Attempts are limited by `MaxRunAttempts` (agent failures: `agent_error`, `idle`, `timeout`, so an ACP watchdog kill stays an agent failure) and by `MaxInfraFailures` (infrastructure failures). - The pool is checked when the retry is claimed. An empty pool opens no Run, and the sweep parks the Shift with the spend named, as for any Run nobody can pay for. - When the budgets run out, the plan advances. If it then ends `plan_exhausted` and the last reading Round after the last writer has a Role with no non-failed Outcome, the close reason is `review_failed`. With a writer's pull request, the item still settles `awaiting_review`. - Close message and tracker comment: "Ploeg finished this item, but an agent review of its pull request is missing" / "not reviewed by an agent: reviewer Run failed (agent_error) after 3 attempt(s). Agent review unavailable; a person is asked to review and merge". When another reader of that Round did report, the comment says "not reviewed by every agent … Agent review incomplete". - Unchanged: a reader that reports without a verdict still closes `plan_exhausted`, a `stuck` reader still freezes the plan, and an approval still closes `review_approved`. - `store.RunReport.FailureReason` is new (read from the existing column; no migration). - The `shift-orchestration` spec rewords the swept-reader scenario as the ADR asks and adds a requirement for the reader retry. `docs/how-to/review-an-agent-pr.md` lists `review_failed`. ## Change (Vloer) In the Ready for review lane, the row chip reads "Agent review unavailable" (attention). The Work Item page says no agent reviewed the pull request. `closeReasonLabel('review_failed')` reads "No agent reviewed it: the reviewer kept failing". The demo replay is re-recorded because `public/` changed. ## Tests New in `pkg/shiftengine/failedreader_test.go`, against real embedded Postgres. All of them except the stuck and wording tests failed on `development` before the change: - `TestFailedReader_ReopensItsOwnRound` (the ADR's named replacement) - agent budget ends after 3 attempts for `agent_error`, `idle` and `timeout` - infrastructure kills use their own budget of 10 - the Shift 118 replay: writer `pr_opened` and three failed reviewer attempts give `review_failed`, `awaiting_review`, and a tracker comment without "plan complete" or "approved" - a retry that approves (`review_approved`) or reports without a verdict (`plan_exhausted`) - one of two reviewers fails and is retried alone - a failed analyst before the writer is not a missing review - a stuck reviewer still freezes the plan - a retry with an empty pool opens no Run (`ErrBudgetExhausted`) and is parked - 8 concurrent evaluators, the sweep and a restarted engine create exactly one retry - the wording test `TestFailedReader_StillAdvancesTheRound` is removed, as the ADR says. `TestExpiredReaderDoesNotBlockTheRound` becomes `TestSweptReaderIsRetriedByTheSweepAndNeverBlocksTheRound`. Vloer adds assertions in `test/ploeg-view.test.mjs` and `test/reasons.test.mjs`. ## Commands (at c2ced737, Go 1.27.1, Node 24.21.0) - `mise exec -- go test ./pkg/shiftengine/ ./pkg/store/ -count=1 -v`: 273 top-level tests passed, 0 failed - `mise exec -- openspec validate --all --strict`: 17 passed - `mise run demo-record`: re-recorded (391 requests) - `mise run verify`: all gates passed (vloer 7 incl. `npm test` 700/700, demo-replay 1, vloer-extension 4, ploeg 7, brand 1, site 5, site-demo 1, helm 15, release 3, integration 1, docs 1) ## Not in this PR - The reviewed SHA surviving retries depends on EXEC-04 (VIK-1738). - Durable delivery of the tracker comment is AUTH-02's outbox (VIK-1728). - After deploy, check that the next failed reviewer shows `close_reason = review_failed`. Refs VIK-1304 🤖 Generated with [Claude Code](https://claude.com/claude-code) </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code)

ryangr0 added 9 commits 2026-10-03 22:42:22 +00:00

build: lock uv 0.12.22 in the root mise.lock ... 2cabad41a7

711a4814 moved uv to 0.12.22 in mise.toml but left the lockfile at
0.12.21. mise-action runs `mise install --locked`, which refuses a
version the lockfile does not hold, so the checks job has failed before
any gate since that commit, on development and on every pull request.

`mise lock uv` (mise 2026.9.18) records 0.12.22 for all seven platforms.
The openspec lock files it deletes under .mise/locks are kept.

Refs: https://github.com/webgrip/unfold/issues/2
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

build(ploeg): consume Ploeg from ploeg-hq/ploeg as a submodule pinned at v0.1.0 ... 26a27b683d

Replace the vendored apps/ploeg tree with a gitlink to
https://github.com/ploeg-hq/ploeg.git at v0.1.0
(87f8dc45a0ea768c6ab95196d8b99d481df10c65), which was extracted from
this repository at 9c1d53f.

- mise run setup, the verify, docs and demo checkouts and the TechDocs
  prepare commands initialise the submodule.
- scripts/ploeg-pin.mjs refuses vendored source, another repository and
  an uninitialised or modified checkout. The ploeg-pin job also requires
  the pinned commit on Ploeg's main, and the release waits for it.
- verify runs Ploeg's own scripts/verify.sh at the pin and compiles the
  unified demo helper, which now imports github.com/ploeg-hq/ploeg.
- The docs build still renders Ploeg's pinned pages, but no longer
  regenerates or validates Ploeg's configuration reference, domain pages
  or decision ledger, and it links Ploeg's source files on GitHub at the
  pinned commit. The combined glossary keeps a decision that a pinned
  model cites by URL instead of mangling it into a relative path.
- Renovate ignores apps/ploeg, drops the Go overlay and leaves the pin
  to people. CI no longer builds the ploegd image context.

Refs: https://github.com/webgrip/unfold/issues/2
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ci(release): stop versioning and publishing Ploeg from Unfold ... 52a7c89f35

Unfold's train now versions Vloer only; github.com/ploeg-hq/ploeg
versions and publishes Ploeg with GitHub Actions.

- on_release_published.yml drops the Ploeg chart, image, signing and
  distribution jobs. The Vloer publisher is the only one, so it takes
  the GitHub release out of draft itself.
- publish_release.py and publish_chart.py refuse ploeg before any Git,
  network or file access. The Go module export to github.com/webgrip/ploeg
  is gone, including the call that disabled GitHub Actions there.
- release-prepare.mjs and apps/.releaserc.cjs touch only Vloer's chart,
  and a commit scoped ploeg never releases Unfold.
- release-floors.json keeps Ploeg's floor and withdrawn 1.0.0-rc.1, marks
  the component retired after 0.4.0-rc.35, and both loaders refuse a
  train that versions a retired component.
- The release preflight no longer checks registry access for ploegd or
  charts/ploeg.

Refs: https://github.com/webgrip/unfold/issues/2
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs: record that Unfold pins Ploeg and releases only Vloer ... 1bca2ac69b

ADR-0019 records the consumer side of the separation approved in
webgrip/unfold#1: Ploeg lives in github.com/ploeg-hq/ploeg, Unfold pins
it as a submodule, and the unfold-v train versions Vloer only. It
supersedes ADR-0004; ADR-0001 and ADR-0018 get dated notes.

README, AGENTS.md, NOTICE, the team-silver skill and the current pages
now say where Ploeg lives, how the pin moves, what Unfold releases and
where Ploeg's artifacts come from.

Refs: https://github.com/webgrip/unfold/issues/2
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

build(docs): treat Ploeg's archived history pages as records ... 3870a8b560

Ploeg's main keeps its pre-separation release history in
docs/history/legacy-changelog.md, a record no current page links. Unfold
renders Ploeg's docs from the pinned commit, so any pin past v0.1.0 failed
the docs build with that page as an orphan. ploeg/history now joins
ploeg/backlog as a record path: kept, marked "not current guidance" and
left out of the nav and search.

Refs: https://github.com/webgrip/unfold/issues/2
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

build(ploeg): pin ploeg-hq/ploeg#47 to retry a failed reviewer ... 42001ecc12

Moves apps/ploeg to 376ac60c, the head of ploeg-hq/ploeg#47, which is
the Ploeg side of Unfold PR 195. Once that pull request merges, move
the pin to its merge commit on Ploeg's main. Until then the ploeg-pin check
stays red, as it should.

Refs: https://github.com/ploeg-hq/ploeg/pull/47
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs: list review_failed in how to review an agent pull request ... 55c57ce42b

The Unfold side of "fix(ploeg): retry a failed reviewer and close
review_failed when no review came" (ploeg-hq/ploeg#47): the review guide
names the new close reason and says it still settles a delivered pull
request as awaiting_review.

Refs VIK-1304

Replaces-commit: bb9b3861fa (#195)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

fix(vloer): mark a pull request whose reviewer kept failing as unreviewed ... d58ff0ac0c

Ploeg now closes a Shift review_failed when its reviewer Run failed and
its retries ran out. The pull request still waits for review, so the
item sits in Ready for review, where its row read "No agent verdict".

The review lane chip now reads "Agent review unavailable" with the
reason in its title, the Work Item page says no agent reviewed the pull
request, and the close reason reads "No agent reviewed it: the reviewer
kept failing" wherever Shift close reasons are listed.

VIK-1304

Replaces-commit: 146cf69fee (#195)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

chore(site): re-record the demo replay with the review_failed wording ... a4dabce0df

VIK-1304

Re-recorded with `mise run demo-record` on top of the Ploeg separation
(webgrip/unfold#206) instead of merging the recorded JSON.

Replaces-commit: c2ced7378a (#195)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

ryangr0 referenced this pull request 2026-10-04 00:30:58 +00:00

test(vloer): give the relay test's timed exec a load-scaled budget #214

ryangr0 force-pushed ryangr0/replace/195-reviewer-retry from a4dabce0df to 7e5e0e8e40 2026-10-04 07:08:52 +00:00 Compare

ryangr0 force-pushed ryangr0/replace/195-reviewer-retry from 7e5e0e8e40 to d84285390c 2026-10-04 07:12:15 +00:00 Compare

ryangr0 referenced this pull request from a commit 2026-10-04 08:38:25 +00:00

test(vloer): give the relay test's timed exec a load-scaled budget

ryangr0 merged commit 7da69f5be7 into development 2026-10-04 08:38:46 +00:00

ryangr0 referenced this pull request from a commit 2026-10-04 08:38:47 +00:00

Merge pull request 'fix(vloer): mark a pull request whose reviewer kept failing as unreviewed' (#207) from ryangr0/replace/195-reviewer-retry into development

webgrip-ci referenced this pull request from a commit 2026-10-04 08:55:24 +00:00

chore(release): unfold-v0.4.0-rc.37 [skip ci]

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

webgrip/unfold!207

No description provided.