From c11bf0eff28716c9569d41d3ec066164f73ee752 Mon Sep 17 00:00:00 2001
From: kami <kamixtrash@gmail.com>
Date: Fri, 28 Aug 2026 10:43:00 +0400
Subject: [PATCH] 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
---
 cmd/orchestra-worker/main.go | 37 +++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/cmd/orchestra-worker/main.go b/cmd/orchestra-worker/main.go
index 12a0ec4..8579e9a 100644
--- a/cmd/orchestra-worker/main.go
+++ b/cmd/orchestra-worker/main.go
@@ -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