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