From fda78cf6e6f3e584376ec10680c2a0c5df244563 Mon Sep 17 00:00:00 2001 From: kami Date: Fri, 28 Aug 2026 15:38:34 +0400 Subject: [PATCH] Stop refusing a handoff over words the arrow already says 98f1b2d fixed the arrow and left the other half. "tried" and "failed because" restate the field names either side of a separator that already says what the line means, and the parser required both. Run 9 lost three leases to this. Every refused line carried exactly the data the fields want: tried run line ["bash", "scripts/test_healthcheck.sh"] for phase-3 -> refused, policy allows only bash -n on one file tried finding ids "F1".."F8" in research.json -> schema requires lowercase letters, digits, dash or underscore Both prefixes are now optional. A line with no cause and effect is still refused, because the split is the thing being validated. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CVbaKucEYBjMqVeUgJUsc1 --- internal/herdr/adapter.go | 15 ++++++++------- internal/herdr/adapter_test.go | 35 ++++++++++++++++++++++++---------- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/internal/herdr/adapter.go b/internal/herdr/adapter.go index 199f790..7aefe0a 100644 --- a/internal/herdr/adapter.go +++ b/internal/herdr/adapter.go @@ -198,7 +198,7 @@ const handoffPrompt = `Orchestra is about to rotate this task. Write ONLY the fo NEXT: the single next action (one line, at most 200 characters). WHY: why that is next (one line, at most 200 characters). REMAINING: outstanding items, one line each, at most 200 characters each. If none: NONE. -DEAD ENDS: approaches tried that failed — "tried X → failed because Y", one per line ("->" is also accepted). If none: NONE. +DEAD ENDS: approaches tried that failed — "X → why it failed", one per line. Either arrow, and the words "tried"/"failed because" are optional. If none: NONE. OPEN Q: unresolved decisions, one line each. If none: NONE. LEARNED: constraints discovered that are NOT in TASK.md, one line each. If none: NONE. @@ -511,12 +511,13 @@ func parseHandoffAnswer(answer string) (handoffAnswer, error) { if len(parts) != 2 { return out, fmt.Errorf("DEAD ENDS: want 'tried X → failed because Y'") } - left, right := strings.TrimSpace(parts[0]), strings.TrimSpace(parts[1]) - if !strings.HasPrefix(left, "tried ") || !strings.HasPrefix(right, "failed because ") { - return out, fmt.Errorf("DEAD ENDS: want 'tried X → failed because Y'") - } - tried := strings.TrimSpace(strings.TrimPrefix(left, "tried ")) - why := strings.TrimSpace(strings.TrimPrefix(right, "failed because ")) + // Both prefixes are optional. "tried" and "failed because" restate the + // field names either side of an arrow that already says what the line + // means, and run 9 lost three leases to an agent that dropped them + // while writing exactly the right content. What must be there is a + // cause and an effect, which is what the split establishes. + tried := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(parts[0]), "tried ")) + why := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(parts[1]), "failed because ")) if tried == "" || why == "" { return out, fmt.Errorf("DEAD ENDS: want 'tried X → failed because Y'") } diff --git a/internal/herdr/adapter_test.go b/internal/herdr/adapter_test.go index b850dab..5796ad3 100644 --- a/internal/herdr/adapter_test.go +++ b/internal/herdr/adapter_test.go @@ -366,30 +366,45 @@ func TestLastObservedCommandSkipsOrchestrasOwnHandoffWrite(t *testing.T) { } } -// Run 9 lost a lease because the agent wrote "->" instead of "→". The content -// was exactly right, the handoff was refused, and a refused handoff fails the -// release rather than the turn. -func TestDeadEndAcceptsEitherArrowAndStripsThePrefixes(t *testing.T) { - for _, arrow := range []string{"→", "->"} { +// Run 9 lost three leases to a handoff whose content was exactly right. The +// agent wrote "->" for the arrow and dropped the "tried"/"failed because" +// words, and each refusal failed the release rather than the turn. +func TestDeadEndAcceptsEitherArrowAndOptionalPrefixes(t *testing.T) { + for _, line := range []string{ + `tried finding ids "F1".."F8" → failed because the schema requires lowercase`, + `tried finding ids "F1".."F8" -> failed because the schema requires lowercase`, + `finding ids "F1".."F8" -> the schema requires lowercase`, + `tried finding ids "F1".."F8" -> the schema requires lowercase`, + } { a, err := parseHandoffAnswer(`NEXT: inspect the failing integration test WHY: isolate the regression before changing production code REMAINING: fix the assertion after identifying the cause -DEAD ENDS: tried finding ids "F1".."F8" ` + arrow + ` failed because the schema requires lowercase +DEAD ENDS: ` + line + ` OPEN Q: whether the remote worker has the updated fixture LEARNED: the fixture requires a committed scratch branch `) if err != nil { - t.Fatalf("arrow %q: %v", arrow, err) + t.Fatalf("%q: %v", line, err) } if len(a.DeadEnds) != 1 { - t.Fatalf("arrow %q: parsed %d dead ends", arrow, len(a.DeadEnds)) + t.Fatalf("%q: parsed %d dead ends", line, len(a.DeadEnds)) } d := a.DeadEnds[0] if d.Tried != `finding ids "F1".."F8"` { - t.Errorf("arrow %q: tried = %q", arrow, d.Tried) + t.Errorf("%q: tried = %q", line, d.Tried) } if d.WhyFailed != "the schema requires lowercase" { - t.Errorf("arrow %q: why_failed = %q", arrow, d.WhyFailed) + t.Errorf("%q: why_failed = %q", line, d.WhyFailed) } } + // A line with no cause and effect at all is still refused. + if _, err := parseHandoffAnswer(`NEXT: a +WHY: b +REMAINING: c +DEAD ENDS: this line names no outcome +OPEN Q: d +LEARNED: e +`); err == nil { + t.Error("a dead end with no separator was accepted") + } }