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) + } +}