From 38aa0738a6a22633bdc7469b57c3be72d0e0867d Mon Sep 17 00:00:00 2001 From: kami Date: Fri, 28 Aug 2026 16:28:30 +0400 Subject: [PATCH] Tell the agent its handoff was rejected instead of looping on the file A refused handoff had no feedback loop. PrepareRelease read the report, the parser refused it, the worker recorded the error in health, and the next boundary read the same bytes and refused them again. Run 10 spent four leases that way and the agent was never told anything. The plan-progress path already had the answer: answerRefusedProgress says why, drops the file, and lets the agent write a corrected one. The release path now does the same, gated on a typed ErrInvalidHandoffAnswer so a transport or Git failure keeps its retry. This is the silent-loop shape CLAUDE.md names, in a path nobody had checked. The three format fixes above it each removed one trigger; this removes the loop. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CVbaKucEYBjMqVeUgJUsc1 --- cmd/orchestra-worker/main.go | 26 ++++++++++++++++++++++++++ internal/herdr/adapter.go | 10 +++++++++- internal/herdr/adapter_test.go | 21 +++++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/cmd/orchestra-worker/main.go b/cmd/orchestra-worker/main.go index 3e12c10..024a7c7 100644 --- a/cmd/orchestra-worker/main.go +++ b/cmd/orchestra-worker/main.go @@ -157,6 +157,7 @@ func tail(s string, max int) string { } return s[len(s)-max:] } + type workerState struct { Cursor uint64 `json:"cursor"` Sessions map[string]herdr.Session `json:"sessions"` @@ -847,6 +848,15 @@ func (w *worker) advanceRelease(ctx context.Context, id string, s herdr.Session) w.releases[id] = tx _ = w.save() w.recordError(fmt.Errorf("release %s prepare: %w", id, err)) + // A badly authored handoff is the agent's to correct, and it is + // the only release failure that is. Recording it in worker health + // alone left the release re-reading the same refused file at every + // boundary until the lease expired, four times over in run 10. + // Same shape as answerRefusedProgress: say why, drop the file, let + // the next rotation prompt produce a better one. + if errors.Is(err, herdr.ErrInvalidHandoffAnswer) { + w.answerRefusedHandoff(ctx, id, s, err) + } return } tx.Ref, tx.AnchorSHA, tx.Phase, tx.LastError, tx.UpdatedAt = prepared.Ref, prepared.AnchorSHA, "anchor_pushed", "", time.Now().UTC() @@ -2175,6 +2185,22 @@ func (w *worker) tellProgressOutcome(ctx context.Context, id string, s herdr.Ses log.Printf("plan phase %s of %s: %s", phase, id, status) } +// answerRefusedHandoff tells the agent why its handoff was rejected and drops +// the file, so the next rotation prompt is answered afresh rather than the +// same refused bytes being re-read forever. +func (w *worker) answerRefusedHandoff(ctx context.Context, id string, s herdr.Session, cause error) { + text := "Orchestra rejected your handoff answer: " + cause.Error() + + "\n\nWrite " + herdr.HandoffReportFile + " again, correcting that, and stop. Do not repeat the rejected answer." + if err := w.sendPrompt(ctx, s, text); err != nil { + w.recordError(fmt.Errorf("deliver handoff refusal %s: %w", id, err)) + return + } + if err := os.Remove(filepath.Join(s.Worktree, herdr.HandoffReportFile)); err != nil && !os.IsNotExist(err) { + w.recordError(fmt.Errorf("drop refused handoff %s: %w", id, err)) + } + log.Printf("handoff %s refused: %v", id, cause) +} + // answerRefusedProgress tells the implementer why its request was refused and // drops the file so a corrected one can be written. Recording a refusal only // in worker health leaves a live session rewriting the same rejected file at diff --git a/internal/herdr/adapter.go b/internal/herdr/adapter.go index b09c9d8..071c8c5 100644 --- a/internal/herdr/adapter.go +++ b/internal/herdr/adapter.go @@ -5,6 +5,7 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "errors" "fmt" "log" "orchestra/internal/continuity" @@ -408,13 +409,20 @@ func (a CLIAdapter) Release(ctx context.Context, s Session) (string, error) { return p.Ref, nil } +// ErrInvalidHandoffAnswer marks a handoff the agent authored badly, as opposed +// to a transport or Git failure. The distinction is the whole point: a bad +// answer is the agent's to correct, and without a way to tell one from the +// other the release loop re-read the same refused file at every boundary until +// the task hit retry_limit. Run 10 spent four leases that way. +var ErrInvalidHandoffAnswer = errors.New("invalid handoff answer") + // canonicalHandoff keeps Git-derived protocol facts on the worker that owns // the checkout. Every authored field comes from the validated agent answer; // it never fabricates task intent or a circular next action. func canonicalHandoff(s Session, answer, command string) (continuity.Handoff, error) { authored, err := parseHandoffAnswer(answer) if err != nil { - return continuity.Handoff{}, fmt.Errorf("adapter: invalid handoff answer: %w", err) + return continuity.Handoff{}, fmt.Errorf("adapter: %w: %s", ErrInvalidHandoffAnswer, err) } sha, err := HeadSHA(s.Worktree) if err != nil { diff --git a/internal/herdr/adapter_test.go b/internal/herdr/adapter_test.go index 0748c43..5a67abe 100644 --- a/internal/herdr/adapter_test.go +++ b/internal/herdr/adapter_test.go @@ -4,6 +4,7 @@ import ( "bufio" "context" "encoding/json" + "errors" "net" "orchestra/internal/continuity" "os" @@ -428,3 +429,23 @@ func TestHandoffPromptStatesTheLimitOnEveryBoundedField(t *testing.T) { } } } + +// A badly authored handoff must be distinguishable from a transport failure, +// because only the first is the agent's to correct. Run 10 spent four leases +// re-reading the same refused report at every boundary. +func TestInvalidHandoffAnswerIsTyped(t *testing.T) { + long := strings.Repeat("x", 219) + _, err := canonicalHandoff(Session{Worktree: t.TempDir()}, `NEXT: a +WHY: b +REMAINING: c +DEAD ENDS: NONE +OPEN Q: `+long+` +LEARNED: e +`, "") + if !errors.Is(err, ErrInvalidHandoffAnswer) { + t.Fatalf("an over-long authored field must be ErrInvalidHandoffAnswer, got %v", err) + } + if !strings.Contains(err.Error(), "219") { + t.Errorf("the refusal does not name the length: %v", err) + } +}