Skip to content

orchestrate: give the reviewer path a bounded repair loop — a failed verdict has no needs-fix and takes the dependency subtree with it #387

Description

@rodrigorjsf

Found during orchestrate run prd352-20260803-015333 (PRD #352, 12 slices, all passed).

Symptom

On slice #358 the reviewer returned a failed envelope carrying a verified rootCause: it drove the committed dist/context-watchdog.js bundle end to end with real PostToolUse hook-event JSON and demonstrated the bug reproducibly. Its diagnosis was precise, it named a bounded fix, and it confirmed both halves of the required plumbing already existed. It explicitly left filesChanged empty so the fix could be made properly in one pass rather than half-fixed into a churned diff.

Per the documented taxonomy (SKILL.md Failure handling; references/slice-pipeline.md step 5), a validated reviewer envelope with status: "failed" means the slice has FAILED, full stop. There is no sanctioned repair loop.

Impact

The blast radius is set by the DAG, not by the defect's size. #358 blocks #362 (the integration keystone), which blocks #363 — so one fixable defect in one slice would have taken three slices out of the run, including the keystone that gives the rest of the run its value. That is a large, structural penalty for a defect the reviewer itself characterised as small and well-bounded.

The asymmetry

The implementer path has a bounded repair loop: incomplete drives continue-in-place against continuationBudget, with a no-progress guard, and only FAILs when the budget is exhausted or progress stalls.

The reviewer path has nothing equivalent — even though a reviewer failed is often more actionable than an incomplete: it arrives with a diagnosis and, frequently, a named fix.

Workaround used

One bounded repair cycle: SendMessage to the still-resident implementer carrying the reviewer's verbatim rootCause, its evidence, its recommended fix, its explicit rejection of the tempting-but-wrong alternative, and its three smaller corrections — then a fresh review.

No verification was bypassed: verify_changeset, a full re-review, and the orchestrator's own run_build + run_tests gate all ran again on the fixed version. If the re-review had failed, the slice would have FAILed for real.

Suggested fix

Give the reviewer path the same shape the implementer path already has:

  • Add a reviewFixBudget (default 1).
  • Add a needs-fix reviewer status distinct from failed. needs-fix carries the rootCause back to the implementer in the same worktree and re-reviews, bounded by the budget and guarded by the same worktree content-fingerprint no-progress check.
  • failed stays the terminal verdict for a defect the reviewer judges unfixable in place.

The reviewer is best placed to make that call, and today the schema gives it no way to express it.

Relationship to existing issues (read before deduping)

Environment

orchestrate 1.6.0 (plugins/orchestrate/.claude-plugin/plugin.json:3 on development) plus the PRD #352 umbrella branch (slice-executor delegation layer). Run prd352-20260803-015333; slice #358, reviewer role.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions