38aa0738a6
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CVbaKucEYBjMqVeUgJUsc1
452 lines
15 KiB
Go
452 lines
15 KiB
Go
package herdr
|
|
|
|
import (
|
|
"bufio"
|
|
"context"
|
|
"encoding/json"
|
|
"errors"
|
|
"net"
|
|
"orchestra/internal/continuity"
|
|
"os"
|
|
"os/exec"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
)
|
|
|
|
type memCAS struct{ m map[string][]byte }
|
|
|
|
func (c *memCAS) PutArtifact(b []byte) (string, error) {
|
|
if c.m == nil {
|
|
c.m = map[string][]byte{}
|
|
}
|
|
id := string(rune(len(c.m) + 'a'))
|
|
c.m[id] = b
|
|
return id, nil
|
|
}
|
|
func (c *memCAS) Artifact(ref string) ([]byte, error) { return c.m[ref], nil }
|
|
|
|
func runGit(t *testing.T, dir string, args ...string) {
|
|
t.Helper()
|
|
cmd := exec.Command("git", append([]string{"-C", dir}, args...)...)
|
|
if out, err := cmd.CombinedOutput(); err != nil {
|
|
t.Fatalf("git %v: %v: %s", args, err, out)
|
|
}
|
|
}
|
|
|
|
// fakeHerdr accepts one JSON-RPC connection and replies with an empty result
|
|
// to every request — enough to exercise CLIAdapter.Release's pane.release_agent
|
|
// call without a live herdr instance.
|
|
func fakeHerdr(t *testing.T) *Client {
|
|
t.Helper()
|
|
ln, err := net.Listen("tcp", "127.0.0.1:0")
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
t.Cleanup(func() { ln.Close() })
|
|
go func() {
|
|
conn, err := ln.Accept()
|
|
if err != nil {
|
|
return
|
|
}
|
|
defer conn.Close()
|
|
var req Request
|
|
if err := json.NewDecoder(bufio.NewReader(conn)).Decode(&req); err != nil {
|
|
return
|
|
}
|
|
json.NewEncoder(conn).Encode(Response{ID: req.ID, Result: json.RawMessage(`{}`)})
|
|
}()
|
|
return &Client{Path: ln.Addr().String(), dial: func() (net.Conn, error) { return net.Dial("tcp", ln.Addr().String()) }}
|
|
}
|
|
|
|
func validHandoff(anchorSHA string) continuity.Handoff {
|
|
return continuity.Handoff{
|
|
Meta: continuity.Meta{ID: "t1", Reason: "threshold", RotationIndex: 1},
|
|
Anchor: continuity.Anchor{GitSHA: anchorSHA, Branch: "main"},
|
|
Action: "run the focused tests",
|
|
Command: "go test ./...",
|
|
}
|
|
}
|
|
|
|
const validAnswer = `NEXT: run the focused tests
|
|
WHY: confirm the current implementation before changing it
|
|
REMAINING: NONE
|
|
DEAD ENDS: NONE
|
|
OPEN Q: NONE
|
|
LEARNED: NONE`
|
|
|
|
func TestReleaseUploadsHandoffAndReleasesAgent(t *testing.T) {
|
|
repo := t.TempDir()
|
|
runGit(t, repo, "init")
|
|
runGit(t, repo, "config", "user.email", "t@t")
|
|
runGit(t, repo, "config", "user.name", "t")
|
|
runGit(t, repo, "commit", "--allow-empty", "-m", "init")
|
|
head, err := HeadSHA(repo)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
|
|
if err := os.WriteFile(filepath.Join(repo, HandoffReportFile), []byte(validAnswer), 0644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
|
|
cas := &memCAS{}
|
|
a := CLIAdapter{Client: fakeHerdr(t), Harness: "claude", CAS: cas}
|
|
ref, err := a.Release(context.Background(), Session{PaneID: "p1", Worktree: repo})
|
|
if err != nil {
|
|
t.Fatalf("Release: %v", err)
|
|
}
|
|
if ref == "" {
|
|
t.Fatal("expected non-empty handoff ref")
|
|
}
|
|
if _, ok := cas.m[ref]; !ok {
|
|
t.Fatal("handoff was not uploaded to CAS")
|
|
}
|
|
got, err := continuity.Load(ref, cas)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if got.Anchor.GitSHA != head {
|
|
t.Fatal("semantic report must not be included in the successor scratch anchor")
|
|
}
|
|
if _, err := os.Stat(filepath.Join(repo, HandoffReportFile)); !os.IsNotExist(err) {
|
|
t.Fatalf("transferred semantic report still present: %v", err)
|
|
}
|
|
}
|
|
|
|
func TestCanonicalHandoffCarriesCoordinatorReason(t *testing.T) {
|
|
repo := t.TempDir()
|
|
runGit(t, repo, "init")
|
|
runGit(t, repo, "config", "user.email", "t@t")
|
|
runGit(t, repo, "config", "user.name", "t")
|
|
runGit(t, repo, "commit", "--allow-empty", "-m", "init")
|
|
if err := os.WriteFile(filepath.Join(repo, HandoffReportFile), []byte(validAnswer), 0644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
cas := &memCAS{}
|
|
a := CLIAdapter{Client: fakeHerdr(t), Harness: "claude", CAS: cas}
|
|
ref, err := a.Release(context.Background(), Session{PaneID: "p1", Worktree: repo, HandoffReason: "thrash"})
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
got, err := continuity.Load(ref, cas)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if got.Meta.Reason != "thrash" {
|
|
t.Fatalf("canonical reason = %q, want thrash", got.Meta.Reason)
|
|
}
|
|
}
|
|
|
|
func TestReleaseUsesHarnessBindingNotDisplayName(t *testing.T) {
|
|
repo := t.TempDir()
|
|
runGit(t, repo, "init")
|
|
runGit(t, repo, "config", "user.email", "t@t")
|
|
runGit(t, repo, "config", "user.name", "t")
|
|
runGit(t, repo, "commit", "--allow-empty", "-m", "init")
|
|
if err := os.WriteFile(filepath.Join(repo, HandoffReportFile), []byte(validAnswer), 0644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
ln, err := net.Listen("tcp", "127.0.0.1:0")
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
t.Cleanup(func() { _ = ln.Close() })
|
|
request := make(chan Request, 1)
|
|
go func() {
|
|
conn, err := ln.Accept()
|
|
if err != nil {
|
|
return
|
|
}
|
|
defer conn.Close()
|
|
var req Request
|
|
if json.NewDecoder(bufio.NewReader(conn)).Decode(&req) == nil {
|
|
request <- req
|
|
_ = json.NewEncoder(conn).Encode(Response{ID: req.ID, Result: json.RawMessage(`{}`)})
|
|
}
|
|
}()
|
|
a := CLIAdapter{Client: &Client{Path: ln.Addr().String(), dial: func() (net.Conn, error) { return net.Dial("tcp", ln.Addr().String()) }}, Harness: "opencode", CAS: &memCAS{}}
|
|
if _, err := a.Release(context.Background(), Session{PaneID: "p1", Worktree: repo, AgentName: "oc-task-specific"}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
req := <-request
|
|
p, _ := json.Marshal(req.Params)
|
|
var got struct {
|
|
Agent string `json:"agent"`
|
|
}
|
|
_ = json.Unmarshal(p, &got)
|
|
if got.Agent != "opencode" {
|
|
t.Fatalf("release agent = %q, want harness binding opencode", got.Agent)
|
|
}
|
|
}
|
|
|
|
func TestReleaseRefusesWithoutHandoffFile(t *testing.T) {
|
|
repo := t.TempDir()
|
|
runGit(t, repo, "init")
|
|
a := CLIAdapter{Client: fakeHerdr(t), Harness: "claude", CAS: &memCAS{}}
|
|
if _, err := a.Release(context.Background(), Session{PaneID: "p1", Worktree: repo}); err == nil {
|
|
t.Fatal("expected error when no handoff file is present")
|
|
}
|
|
}
|
|
|
|
func TestReleaseRefusesNarrativeOrCircularHandoffAnswer(t *testing.T) {
|
|
repo := t.TempDir()
|
|
runGit(t, repo, "init")
|
|
runGit(t, repo, "config", "user.email", "t@t")
|
|
runGit(t, repo, "config", "user.name", "t")
|
|
runGit(t, repo, "commit", "--allow-empty", "-m", "init")
|
|
bad := `NEXT: Continue the task from the handoff
|
|
WHY: the predecessor asked for it
|
|
REMAINING: what changed\n# a markdown report
|
|
DEAD ENDS: NONE
|
|
OPEN Q: NONE
|
|
LEARNED: NONE`
|
|
if err := os.WriteFile(filepath.Join(repo, HandoffReportFile), []byte(bad), 0o644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
a := CLIAdapter{Client: fakeHerdr(t), Harness: "claude", CAS: &memCAS{}}
|
|
if _, err := a.Release(context.Background(), Session{PaneID: "p1", Worktree: repo}); err == nil {
|
|
t.Fatal("expected invalid agent answer to refuse release")
|
|
}
|
|
}
|
|
|
|
func TestParseHandoffAnswerPreservesOnlyAuthoredFields(t *testing.T) {
|
|
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 rerunning the whole suite → failed because it obscures the relevant failure
|
|
OPEN Q: whether the remote worker has the updated fixture
|
|
LEARNED: the fixture requires a committed scratch branch
|
|
`)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if len(a.Remaining) != 1 || len(a.DeadEnds) != 1 || len(a.OpenQuestions) != 1 || len(a.Learned) != 1 {
|
|
t.Fatalf("parsed answer lost authored fields: %#v", a)
|
|
}
|
|
}
|
|
|
|
func TestReleaseDoesNotTrustAgentSuppliedAnchor(t *testing.T) {
|
|
repo := t.TempDir()
|
|
runGit(t, repo, "init")
|
|
runGit(t, repo, "config", "user.email", "t@t")
|
|
runGit(t, repo, "config", "user.name", "t")
|
|
runGit(t, repo, "commit", "--allow-empty", "-m", "init")
|
|
|
|
if err := os.WriteFile(filepath.Join(repo, HandoffReportFile), []byte(validAnswer), 0644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
a := CLIAdapter{Client: fakeHerdr(t), Harness: "claude", CAS: &memCAS{}}
|
|
if _, err := a.Release(context.Background(), Session{PaneID: "p1", Worktree: repo}); err != nil {
|
|
t.Fatalf("Release must derive the anchor itself: %v", err)
|
|
}
|
|
}
|
|
|
|
func TestReleaseScratchCommitsDirtyFilesBeforeUpload(t *testing.T) {
|
|
repo := t.TempDir()
|
|
runGit(t, repo, "init")
|
|
runGit(t, repo, "config", "user.email", "t@t")
|
|
runGit(t, repo, "config", "user.name", "t")
|
|
runGit(t, repo, "commit", "--allow-empty", "-m", "init")
|
|
head, err := HeadSHA(repo)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
|
|
wipPath := filepath.Join(repo, "wip.txt")
|
|
if err := os.WriteFile(wipPath, []byte("in progress"), 0644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.WriteFile(filepath.Join(repo, HandoffReportFile), []byte(validAnswer), 0644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
|
|
cas := &memCAS{}
|
|
a := CLIAdapter{Client: fakeHerdr(t), Harness: "claude", CAS: cas}
|
|
ref, err := a.Release(context.Background(), Session{PaneID: "p1", Worktree: repo})
|
|
if err != nil {
|
|
t.Fatalf("Release: %v", err)
|
|
}
|
|
got, err := continuity.Load(ref, cas)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if got.Anchor.GitSHA == head {
|
|
t.Fatal("expected anchor to advance to the new scratch commit")
|
|
}
|
|
if len(got.Anchor.Dirty) != 0 {
|
|
t.Fatal("expected dirty entries to be cleared after scratch commit")
|
|
}
|
|
newHead, err := HeadSHA(repo)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if got.Anchor.GitSHA != newHead {
|
|
t.Fatalf("stored anchor=%s does not match worktree HEAD=%s", got.Anchor.GitSHA, newHead)
|
|
}
|
|
branch, err := exec.Command("git", "-C", repo, "branch", "--show-current").Output()
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if got.Anchor.Branch != "orchestra/scratch/p1" || string(branch) != got.Anchor.Branch+"\n" {
|
|
t.Fatalf("expected worktree on scratch branch, got %q (handoff says %q)", branch, got.Anchor.Branch)
|
|
}
|
|
}
|
|
|
|
func TestReleaseDoesNotTrustAgentSuppliedDirtyFile(t *testing.T) {
|
|
repo := t.TempDir()
|
|
runGit(t, repo, "init")
|
|
runGit(t, repo, "config", "user.email", "t@t")
|
|
runGit(t, repo, "config", "user.name", "t")
|
|
runGit(t, repo, "commit", "--allow-empty", "-m", "init")
|
|
if err := os.WriteFile(filepath.Join(repo, "wip.txt"), []byte("changed after handoff written"), 0644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.WriteFile(filepath.Join(repo, HandoffReportFile), []byte(validAnswer), 0644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
a := CLIAdapter{Client: fakeHerdr(t), Harness: "claude", CAS: &memCAS{}}
|
|
if _, err := a.Release(context.Background(), Session{PaneID: "p1", Worktree: repo}); err != nil {
|
|
t.Fatalf("Release must derive dirty hashes itself: %v", err)
|
|
}
|
|
}
|
|
|
|
func TestReleaseRequiresCAS(t *testing.T) {
|
|
a := CLIAdapter{Client: fakeHerdr(t), Harness: "claude"}
|
|
if _, err := a.Release(context.Background(), Session{PaneID: "p1", Worktree: t.TempDir()}); err == nil {
|
|
t.Fatal("expected error when CAS is nil")
|
|
}
|
|
}
|
|
|
|
// TestEveryReasonTheAdapterProducesIsAcceptedByTheValidator guards F36. The
|
|
// adapter gained "phase_changed" when phase rotations landed and the
|
|
// validator's vocabulary did not, so every phase rotation built a handoff that
|
|
// was then refused as "invalid handoff meta". A live run reached exactly this
|
|
// point after F31 unblocked the parse ahead of it.
|
|
func TestEveryReasonTheAdapterProducesIsAcceptedByTheValidator(t *testing.T) {
|
|
for _, reason := range []string{"threshold", "milestone", "thrash", "manual", "reconcile_failure", "phase_changed", "something the adapter does not know"} {
|
|
produced := handoffReason(Session{HandoffReason: reason})
|
|
h := continuity.Handoff{
|
|
Meta: continuity.Meta{ID: "agent-1", Reason: produced},
|
|
Anchor: continuity.Anchor{GitSHA: "0000000000000000000000000000000000000000", Branch: "main"},
|
|
Action: "write the research note",
|
|
}
|
|
if err := h.Validate(); err != nil {
|
|
t.Fatalf("adapter produces reason %q for %q, validator refuses it: %v", produced, reason, err)
|
|
}
|
|
}
|
|
}
|
|
|
|
// TestLastObservedCommandSkipsOrchestrasOwnHandoffWrite guards F37. The handoff
|
|
// prompt tells the agent to write HandoffReportFile and stop, so that write is
|
|
// almost always the last command in the pane. Carrying it into Handoff.Command
|
|
// made every rotation fail Validate's own circularity check. Live on run 5:
|
|
// "adapter: upload handoff: invalid handoff command: must not point to a
|
|
// handoff or report".
|
|
func TestLastObservedCommandSkipsOrchestrasOwnHandoffWrite(t *testing.T) {
|
|
for _, last := range []string{
|
|
"cat > " + HandoffReportFile,
|
|
"vim .orchestra-handoff.json",
|
|
"less handoff-report.md",
|
|
} {
|
|
if !continuity.IsCircularCommand(last) {
|
|
t.Fatalf("%q is not recognised as circular, so this test proves nothing", last)
|
|
}
|
|
h := continuity.Handoff{
|
|
Meta: continuity.Meta{ID: "agent-1", Reason: "phase_changed"},
|
|
Anchor: continuity.Anchor{GitSHA: "0000000000000000000000000000000000000000", Branch: "main"},
|
|
Action: "write the research note",
|
|
Command: "go test ./...",
|
|
}
|
|
if err := h.Validate(); err != nil {
|
|
t.Fatalf("a real command was rejected: %v", err)
|
|
}
|
|
h.Command = last
|
|
if err := h.Validate(); err == nil {
|
|
t.Fatalf("%q was accepted, so skipping it in the producer is not what keeps rotations alive", last)
|
|
}
|
|
}
|
|
}
|
|
|
|
// 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: ` + line + `
|
|
OPEN Q: whether the remote worker has the updated fixture
|
|
LEARNED: the fixture requires a committed scratch branch
|
|
`)
|
|
if err != nil {
|
|
t.Fatalf("%q: %v", line, err)
|
|
}
|
|
if len(a.DeadEnds) != 1 {
|
|
t.Fatalf("%q: parsed %d dead ends", line, len(a.DeadEnds))
|
|
}
|
|
d := a.DeadEnds[0]
|
|
if d.Tried != `finding ids "F1".."F8"` {
|
|
t.Errorf("%q: tried = %q", line, d.Tried)
|
|
}
|
|
if d.WhyFailed != "the schema requires lowercase" {
|
|
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")
|
|
}
|
|
}
|
|
|
|
// Every field the validator bounds must say so. Run 10 lost a lease to a
|
|
// 219-character OPEN Q against a limit the prompt stated for NEXT, WHY and
|
|
// REMAINING only.
|
|
func TestHandoffPromptStatesTheLimitOnEveryBoundedField(t *testing.T) {
|
|
for _, field := range []string{"NEXT", "WHY", "REMAINING", "OPEN Q", "LEARNED"} {
|
|
i := strings.Index(handoffPrompt, field+":")
|
|
if i < 0 {
|
|
t.Fatalf("the prompt never names %s", field)
|
|
}
|
|
line := handoffPrompt[i:]
|
|
if j := strings.Index(line, "\n"); j >= 0 {
|
|
line = line[:j]
|
|
}
|
|
if !strings.Contains(line, "200 characters") {
|
|
t.Errorf("%s is bounded at 200 but the prompt never says so: %q", field, line)
|
|
}
|
|
}
|
|
}
|
|
|
|
// 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)
|
|
}
|
|
}
|