Tell the reviewer when its findings file is missing or malformed

Submission needs a sealed review, so a reviewing session that writes
.orchestra/done without .orchestra/review.json refuses every five seconds
with the failure recorded only in worker health. That is the silent loop this
codebase keeps producing: F39 at a phase boundary, F42 at the result commit,
and now the completion tail.

A review-artifact refusal is separated from every other submission failure,
delivered to the pane that can fix it, and clears the done marker so the
corrected file is what finishes the phase.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CVbaKucEYBjMqVeUgJUsc1
This commit is contained in:
2026-08-28 10:43:00 +04:00
parent e8d04d719d
commit c11bf0eff2
+34 -3
View File
@@ -578,6 +578,14 @@ func (w *worker) releaseReady(ctx context.Context) {
continue
}
outcome, err := w.submit(ctx, id, s, evidence)
if errors.Is(err, errReviewArtifact) {
// Only the reviewing agent can fix this, and it is still alive
// to be told. Dropping the done marker stops the five-second
// retry and makes the corrected file the thing that finishes.
w.recordError(fmt.Errorf("submit %s: %w", id, err))
w.answerRefusedReview(ctx, id, s, err)
continue
}
if err != nil {
w.recordError(fmt.Errorf("submit %s: %w", id, err))
log.Printf("submit %s: %v", id, err)
@@ -1026,6 +1034,29 @@ func (w *worker) finalize(ctx context.Context, id string, s herdr.Session) (comp
// assert.
const reviewFile = "review.json"
// errReviewArtifact is a refusal only the reviewing agent can fix. It is
// separated from every other submission failure because recording it in worker
// health alone would leave a live session being retried every five seconds
// with nothing telling it what is wrong — the silent-loop shape this codebase
// keeps producing (F39, F42).
var errReviewArtifact = errors.New("review artifact")
// answerRefusedReview tells the reviewing session why its finish was refused
// and clears the done marker so a corrected artifact is what finishes the
// phase. The refusal survives a failed send: the marker is dropped either way,
// and the agent writes it again when it has written the file.
func (w *worker) answerRefusedReview(ctx context.Context, id string, s herdr.Session, cause error) {
text := "Orchestra refused your completion: " + cause.Error() +
"\n\nWrite .orchestra/" + reviewFile + " as {\"findings\": [...]}, an empty list if you found nothing, then write .orchestra/done again. Do not set a commit sha."
if err := w.sendPrompt(ctx, s, text); err != nil {
w.recordError(fmt.Errorf("deliver review refusal %s: %w", id, err))
}
if err := os.Remove(filepath.Join(s.Worktree, ".orchestra", "done")); err != nil {
w.recordError(fmt.Errorf("clear done marker %s: %w", id, err))
}
log.Printf("completion %s refused: %v", id, cause)
}
// submit seals the review and hands the result to the human through a pull
// request. It is the only path from a reviewed change to TaskSubmitted; the
// direct completion call remains for a project with no forge configured.
@@ -1036,14 +1067,14 @@ func (w *worker) submit(ctx context.Context, id string, s herdr.Session, e compl
// An absent review file is not a pass. Submission needs a sealed
// review, and inventing an empty one would launder "the reviewer wrote
// nothing" into "the reviewer found nothing".
return "", fmt.Errorf("review phase sealed no .orchestra/%s: %w", reviewFile, err)
return "", fmt.Errorf("%w: the review phase sealed no .orchestra/%s: %v", errReviewArtifact, reviewFile, err)
}
if err := json.Unmarshal(b, &result); err != nil {
return "", fmt.Errorf(".orchestra/%s: %w", reviewFile, err)
return "", fmt.Errorf("%w: .orchestra/%s is not valid JSON: %v", errReviewArtifact, reviewFile, err)
}
result.ResultSHA = e.ResultSHA
if err := result.Validate(); err != nil {
return "", fmt.Errorf(".orchestra/%s: %w", reviewFile, err)
return "", fmt.Errorf("%w: .orchestra/%s: %v", errReviewArtifact, reviewFile, err)
}
l := w.leases[id]
// The gate result is bound to the commit the gate ran against, which