diff --git a/cmd/orchestra-worker/main.go b/cmd/orchestra-worker/main.go index 411f9ad..b37bfa9 100644 --- a/cmd/orchestra-worker/main.go +++ b/cmd/orchestra-worker/main.go @@ -417,7 +417,21 @@ func (w *worker) start(ctx context.Context, t domain.Task, ref string) error { return fmt.Errorf("build context: %w", err) } prompt := built.System + "\n\n" + built.Task + // The transport decides what is submitted, never what the agent receives: + // the file holds the exact bytes agentctx rendered either way. + submitted, transport := prompt, herdr.LaunchInline + if lt, ok := backend.(herdr.LaunchTransporter); ok { + transport = lt.LaunchTransport(w.harness) + } + if transport == herdr.LaunchFileRef { + submitted = herdr.LaunchReference + } if writeErr := herdr.WriteLaunchContext(s.Worktree, prompt); writeErr != nil { + // Under LaunchFileRef the file is the instruction, so a failed write + // is a failed launch rather than lost evidence. + if transport == herdr.LaunchFileRef { + return fmt.Errorf("launch context %s: %w", t.ID, writeErr) + } w.recordError(fmt.Errorf("launch context %s: %w", t.ID, writeErr)) } // The launch instruction carried these, so the first turn boundary must @@ -431,9 +445,29 @@ func (w *worker) start(ctx context.Context, t domain.Task, ref string) error { } // A prompt response can be lost after the backend accepted it. Persist the // session first so the worker can reconcile/release it after restart. - if err := backend.Prompt(ctx, s.PaneID, prompt, 0); err != nil { + if err := backend.Prompt(ctx, s.PaneID, submitted, 0); err != nil { return err } + // Acknowledging a launch means the harness accepted the instruction, not + // that the adapter call returned nil. Without this the worker reported a + // started agent while the prompt sat unsubmitted in the input editor. + if c, ok := backend.(herdr.LaunchConfirmer); ok { + evidence, confirmErr := c.ConfirmLaunch(ctx, s, submitted) + if confirmErr != nil { + // An unsubmitted prompt leaves a live pane that nothing owns, and + // a retained session would make the retry skip this task + // entirely. Reclaim both so the released lease can be re-leased. + if killErr := backend.Kill(ctx, s); killErr != nil { + w.recordError(fmt.Errorf("kill unlaunched pane %s: %w", t.ID, killErr)) + } + delete(w.sessions, t.ID) + if saveErr := w.save(); saveErr != nil { + w.recordError(fmt.Errorf("save after failed launch %s: %w", t.ID, saveErr)) + } + return fmt.Errorf("launch %s: %w", t.ID, confirmErr) + } + log.Printf("launch %s confirmed: %s", t.ID, evidence) + } if l, ok := w.leases[t.ID]; ok { if err := w.api.Start(ctx, t.ID, l.Epoch, l.Version, w.sessionEvidence(ctx, t.ID, s)); err != nil { return fmt.Errorf("ack start: %w", err) @@ -451,6 +485,11 @@ func (w *worker) start(ctx context.Context, t domain.Task, ref string) error { } func classifyLaunchError(err error, sessionStarted bool) string { + // Positive evidence that the harness never accepted the prompt is not + // uncertainty. Release the lease so the existing retry path can take it. + if errors.Is(err, herdr.ErrPromptNotSubmitted) { + return "prompt_not_submitted" + } if sessionStarted { // A prompt response can be lost after herdr accepted it. Never reclaim // that pane just because its acknowledgement was uncertain. diff --git a/cmd/orchestra-worker/main_test.go b/cmd/orchestra-worker/main_test.go index fb79df7..ff124aa 100644 --- a/cmd/orchestra-worker/main_test.go +++ b/cmd/orchestra-worker/main_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "net" "net/http" "net/http/httptest" @@ -715,3 +716,121 @@ func TestFederatedTurnActsOnPrepareHandoffVerdict(t *testing.T) { t.Fatalf("handoff re-requested: %v", backend.prompts) } } + +// launchBackend is a recordingBackend that also declares a launch transport +// and a confirmation outcome, which is the seam F15 added. +type launchBackend struct { + recordingBackend + transport herdr.LaunchTransport + confirmed bool + killed int +} + +func (b *launchBackend) LaunchTransport(string) herdr.LaunchTransport { return b.transport } +func (b *launchBackend) ConfirmLaunch(context.Context, herdr.Session, string) (string, error) { + if !b.confirmed { + return "", fmt.Errorf("%w: editor still holds the prompt", herdr.ErrPromptNotSubmitted) + } + return "input editor cleared, agent busy", nil +} +func (b *launchBackend) Kill(context.Context, herdr.Session) error { b.killed++; return nil } + +// startFixture builds the git remote, local clone and coordinator stub that +// worker.start needs, and returns a worker wired to the given backend. +func startFixture(t *testing.T, backend herdr.Backend, harness string) (*worker, domain.Task, string) { + t.Helper() + remote := filepath.Join(t.TempDir(), "remote.git") + if out, err := exec.Command("git", "init", "--bare", remote).CombinedOutput(); err != nil { + t.Fatalf("remote: %v %s", err, out) + } + seed := t.TempDir() + for _, a := range [][]string{{"init", seed}, {"-C", seed, "config", "user.email", "t@t"}, {"-C", seed, "config", "user.name", "t"}, {"-C", seed, "commit", "--allow-empty", "-m", "init"}, {"-C", seed, "remote", "add", "origin", remote}, {"-C", seed, "push", "-u", "origin", "HEAD:master"}} { + if out, err := exec.Command("git", a...).CombinedOutput(); err != nil { + t.Fatalf("git %v: %v %s", a, err, out) + } + } + repo := filepath.Join(t.TempDir(), "repo") + if out, err := exec.Command("git", "clone", remote, repo).CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + api := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/intent") { + json.NewEncoder(w).Encode(domain.EffectiveIntent{Task: domain.Task{ID: "task", Title: "test", Description: "do work"}}) + return + } + w.WriteHeader(200) + })) + t.Cleanup(api.Close) + root := filepath.Join(filepath.Dir(repo), "worktrees") + w := &worker{ + api: federation.Client{BaseURL: api.URL, WorkerID: "h", Token: "t"}, harnessID: "h", + backend: backend, repo: repo, root: root, remote: "origin", harness: harness, + tasks: map[string]domain.Task{}, sessions: map[string]herdr.Session{}, leases: map[string]lease{}, + releases: map[string]releaseTransaction{}, quarantined: map[string]bool{}, + statePath: filepath.Join(t.TempDir(), "state.json"), hard: .75, + } + return w, domain.Task{ID: "task", Source: "s", ExternalID: "x", Project: "p", Title: "test", Description: "do work"}, root +} + +// The claude/tmux launch submits one line, while the agent still receives the +// exact bytes agentctx rendered, through the file the line points at. +func TestClaudeLaunchSubmitsAFileReferenceAndConfirmsIt(t *testing.T) { + backend := &launchBackend{transport: herdr.LaunchFileRef, confirmed: true} + w, task, root := startFixture(t, backend, "claude") + if err := w.start(context.Background(), task, ""); err != nil { + t.Fatal(err) + } + if len(backend.prompts) != 1 || backend.prompts[0] != herdr.LaunchReference { + t.Fatalf("submitted %q, want the one-line launch reference", backend.prompts) + } + b, err := os.ReadFile(filepath.Join(root, "task", herdr.LaunchContextFile)) + if err != nil { + t.Fatalf("launch context: %v", err) + } + for _, want := range []string{"Authority order", "## Goal", "do work"} { + if !strings.Contains(string(b), want) { + t.Fatalf("launch context missing %q", want) + } + } + if _, ok := w.sessions[task.ID]; !ok { + t.Fatal("confirmed launch did not keep its session") + } +} + +// Prompt returning nil is not an acknowledgement. An unconfirmed launch must +// reclaim the pane, drop the session so the retry can start clean, and +// classify as prompt_not_submitted so the lease is released rather than held. +func TestUnconfirmedLaunchFailsAndReclaimsThePane(t *testing.T) { + backend := &launchBackend{transport: herdr.LaunchFileRef, confirmed: false} + w, task, _ := startFixture(t, backend, "claude") + err := w.start(context.Background(), task, "") + if !errors.Is(err, herdr.ErrPromptNotSubmitted) { + t.Fatalf("start err=%v, want ErrPromptNotSubmitted", err) + } + if _, ok := w.sessions[task.ID]; ok { + t.Fatal("a launch that never happened must not leave a session behind") + } + if backend.killed != 1 { + t.Fatalf("killed %d panes, want 1", backend.killed) + } + if got := classifyLaunchError(err, false); got != "prompt_not_submitted" { + t.Fatalf("class=%q, want prompt_not_submitted", got) + } + // Even with a session still recorded, positive evidence outranks the + // uncertain classification that would otherwise hold the lease forever. + if got := classifyLaunchError(err, true); got != "prompt_not_submitted" { + t.Fatalf("class with session=%q, want prompt_not_submitted", got) + } +} + +// A backend that cannot confirm keeps the inline launch it was verified on. +func TestInlineTransportSubmitsTheWholeInstruction(t *testing.T) { + backend := &launchBackend{transport: herdr.LaunchInline, confirmed: true} + w, task, _ := startFixture(t, backend, "opencode") + if err := w.start(context.Background(), task, ""); err != nil { + t.Fatal(err) + } + if len(backend.prompts) != 1 || !strings.Contains(backend.prompts[0], "Authority order") { + t.Fatalf("submitted %q, want the whole instruction", backend.prompts) + } +} diff --git a/internal/herdr/backend.go b/internal/herdr/backend.go index d52b237..0173cc1 100644 --- a/internal/herdr/backend.go +++ b/internal/herdr/backend.go @@ -2,6 +2,7 @@ package herdr import ( "context" + "errors" "time" ) @@ -76,3 +77,48 @@ func (c *Client) ReleaseAgent(ctx context.Context, s Session, harness string) er } var _ Backend = (*Client)(nil) + +// LaunchTransport says how a backend delivers a task's launch instruction. +// +// The instruction itself never changes: agentctx renders one canonical text +// and WriteLaunchContext stores those exact bytes at LaunchContextFile. Only +// the delivery differs, because a terminal harness is not a protocol. +type LaunchTransport string + +const ( + // LaunchInline sends the whole instruction as the prompt. + LaunchInline LaunchTransport = "inline" + // LaunchFileRef sends one line pointing at LaunchContextFile. Claude Code + // coalesces a fast multi-line literal write into a paste and absorbs the + // following Enter into it, so an inline launch is delivered and never + // submitted. A one-line prompt does not trigger paste detection. Found on + // burn-in run 2, 2026-08-26. + LaunchFileRef LaunchTransport = "file_ref" +) + +// LaunchReference is the one-line prompt LaunchFileRef submits. It names the +// file two ways on purpose: the @ form is the harness's own file-reference +// convention, and the bare path stays readable if the harness declines to +// expand a reference into an ignored directory. +const LaunchReference = "@" + LaunchContextFile + " is your complete Orchestra launch instruction. Read .orchestra/launch.md now and follow it." + +// LaunchTransporter is optional. A backend that does not implement it sends +// the instruction inline. +type LaunchTransporter interface { + LaunchTransport(harness string) LaunchTransport +} + +// ErrPromptNotSubmitted means the prompt reached the harness's input and was +// never submitted. It is a launch failure with positive evidence, not an +// uncertain one: the lease must be released and retried rather than held. +var ErrPromptNotSubmitted = errors.New("prompt_not_submitted") + +// LaunchConfirmer is optional. A backend that does not implement it treats a +// successful Prompt as proof of submission, which is only sound where the +// backend's own protocol acknowledges the prompt. +// +// ConfirmLaunch returns the evidence that convinced it, or an error wrapping +// ErrPromptNotSubmitted when the submission cannot be observed. +type LaunchConfirmer interface { + ConfirmLaunch(ctx context.Context, s Session, submitted string) (string, error) +} diff --git a/internal/herdr/tmux.go b/internal/herdr/tmux.go index a4c73c0..65bc473 100644 --- a/internal/herdr/tmux.go +++ b/internal/herdr/tmux.go @@ -8,6 +8,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "strings" "time" ) @@ -25,6 +26,20 @@ type TmuxBackend struct { Command string // Binary is test/packaging override for tmux itself. Binary string + // LaunchConfirmTimeout and LaunchConfirmPoll bound ConfirmLaunch. They are + // tunable because how fast a terminal harness visibly reacts is a property + // of the host, not of this code. Zero values mean 10s and 250ms. + LaunchConfirmTimeout time.Duration + LaunchConfirmPoll time.Duration + // Now is a test seam for the confirmation deadline. + Now func() time.Time +} + +func (b *TmuxBackend) now() time.Time { + if b.Now != nil { + return b.Now() + } + return time.Now() } func NewTmuxBackend(socket, command string) *TmuxBackend { @@ -334,3 +349,79 @@ func (b *TmuxBackend) ReleaseAgent(ctx context.Context, s Session, _ string) err } var _ Backend = (*TmuxBackend)(nil) + +// LaunchTransport keeps the file-reference launch scoped to the harness whose +// paste handling requires it, rather than making it the universal prompt +// format. Every other harness keeps the inline path it was verified on. +func (b *TmuxBackend) LaunchTransport(harness string) LaunchTransport { + if harness == "claude" { + return LaunchFileRef + } + return LaunchInline +} + +// promptLine matches the harness input editor's own line. Its remainder is +// what is still sitting unsubmitted, whether that is literal text or the +// TUI's own "[Pasted text #1 +66 lines]" placeholder. +var promptLine = regexp.MustCompile(`(?m)^[ \t]*[>❯][ \t]*(.*)$`) + +// pendingInput returns whatever the input editor still holds. +func pendingInput(pane string) string { + m := promptLine.FindAllStringSubmatch(pane, -1) + if len(m) == 0 { + return "" + } + return strings.TrimSpace(m[len(m)-1][1]) +} + +// ConfirmLaunch waits for observable proof that the harness accepted the +// prompt. An input editor that still holds the submitted text is definite +// failure, which is the whole point: TaskLaunchAcknowledged previously meant +// "Prompt returned nil" and so reported a launch that never happened. +// +// This is not a timing workaround for the submit itself. The submit is +// deterministic; this waits for the harness to visibly react to it. +func (b *TmuxBackend) ConfirmLaunch(ctx context.Context, s Session, submitted string) (string, error) { + timeout, poll := b.LaunchConfirmTimeout, b.LaunchConfirmPoll + if timeout <= 0 { + timeout = 10 * time.Second + } + if poll <= 0 { + poll = 250 * time.Millisecond + } + deadline := b.now().Add(timeout) + var last string + for { + pane, err := b.PaneCapture(ctx, s, "recent") + if err != nil { + return "", err + } + if last = pendingInput(pane); last == "" { + status, statusErr := b.AgentStatus(ctx, s) + if statusErr != nil { + return "", statusErr + } + switch status { + case "exited": + return "", fmt.Errorf("%w: pane %s exited before the harness reacted", ErrPromptNotSubmitted, s.PaneID) + case "busy": + return "input editor cleared, agent busy", nil + case "blocked": + // A permission dialog is the harness acting on the + // instruction, so submission is proven even though the + // session now needs an operator. + return "input editor cleared, agent awaiting approval", nil + default: + return "input editor cleared", nil + } + } + if !b.now().Before(deadline) { + return "", fmt.Errorf("%w: pane %s still holds %q after %s", ErrPromptNotSubmitted, s.PaneID, last, timeout) + } + select { + case <-ctx.Done(): + return "", ctx.Err() + case <-time.After(poll): + } + } +} diff --git a/internal/herdr/tmux_test.go b/internal/herdr/tmux_test.go index 55eb64e..424edca 100644 --- a/internal/herdr/tmux_test.go +++ b/internal/herdr/tmux_test.go @@ -2,6 +2,8 @@ package herdr import ( "context" + "errors" + "fmt" "os" "os/exec" "path/filepath" @@ -125,3 +127,107 @@ func TestWriteLaunchContextLeavesTheWorktreeClean(t *testing.T) { t.Fatalf("worktree is dirty after a launch dump:\n%s", out) } } + +func TestLaunchTransportIsHarnessSpecific(t *testing.T) { + b := &TmuxBackend{} + if got := b.LaunchTransport("claude"); got != LaunchFileRef { + t.Fatalf("claude transport=%q, want %q", got, LaunchFileRef) + } + for _, harness := range []string{"opencode", "codex", ""} { + if got := b.LaunchTransport(harness); got != LaunchInline { + t.Fatalf("%s transport=%q, want %q", harness, got, LaunchInline) + } + } +} + +// The exact F15 signature: the TUI coalesced the launch text into a paste and +// absorbed the Enter, leaving the instruction in the input editor. +func TestPendingInputSeesAnUnsubmittedPaste(t *testing.T) { + stalled := " b141e4f6 - none | 0/1.0m ctx\n❯ [Pasted text #1 +66 lines]\n────────────\n" + if got := pendingInput(stalled); got != "[Pasted text #1 +66 lines]" { + t.Fatalf("pendingInput=%q, want the unsubmitted paste", got) + } + if got := pendingInput("esc to interrupt\n❯ \n"); got != "" { + t.Fatalf("pendingInput=%q, want empty for a cleared editor", got) + } + if got := pendingInput("❯ @.orchestra/launch.md is your complete Orchestra launch instruction."); got == "" { + t.Fatal("a one-line reference still sitting in the editor must count as pending") + } + if got := pendingInput("no editor line here"); got != "" { + t.Fatalf("pendingInput=%q, want empty when no editor line is visible", got) + } +} + +// fakeTmux scripts capture-pane so confirmation can be driven without waiting +// on a real harness. clearAfter is how many captures still show the stalled +// editor before it clears. +func fakeTmux(t *testing.T, clearAfter int, failCapture bool) *TmuxBackend { + t.Helper() + dir := t.TempDir() + bin := filepath.Join(dir, "tmux") + fail := "0" + if failCapture { + fail = "1" + } + script := `#!/bin/sh +state=` + filepath.Join(dir, "count") + ` +cmd="" +for a in "$@"; do + case "$a" in + capture-pane|has-session|display-message) cmd=$a; break;; + esac +done +case "$cmd" in + has-session) exit 0 ;; + display-message) printf '0\tclaude\n'; exit 0 ;; + capture-pane) + if [ "` + fail + `" = "1" ]; then echo "no server running" >&2; exit 1; fi + n=$(cat $state 2>/dev/null || echo 0); n=$((n+1)); echo $n > $state + if [ "$n" -le "` + fmt.Sprint(clearAfter) + `" ]; then + printf '\xe2\x9d\xaf [Pasted text #1 +66 lines]\n' + else + printf 'esc to interrupt\n\xe2\x9d\xaf \n' + fi + exit 0 ;; +esac +exit 0 +` + if err := os.WriteFile(bin, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + return &TmuxBackend{Binary: bin, LaunchConfirmTimeout: 2 * time.Second, LaunchConfirmPoll: 5 * time.Millisecond} +} + +func TestConfirmLaunchAcceptsObservedActivity(t *testing.T) { + b := fakeTmux(t, 1, false) + evidence, err := b.ConfirmLaunch(context.Background(), Session{PaneID: "s:1.0"}, LaunchReference) + if err != nil { + t.Fatalf("confirm: %v", err) + } + if !strings.Contains(evidence, "busy") { + t.Fatalf("evidence=%q, want the observed activity named", evidence) + } +} + +// Prompt returning nil is not proof. An editor that still holds the text at +// the deadline is a launch failure, and it must be the one class that releases +// the lease instead of holding it as uncertain. +func TestConfirmLaunchFailsWhileTheEditorStillHoldsThePrompt(t *testing.T) { + b := fakeTmux(t, 1000, false) + b.LaunchConfirmTimeout = 60 * time.Millisecond + _, err := b.ConfirmLaunch(context.Background(), Session{PaneID: "s:1.0"}, LaunchReference) + if !errors.Is(err, ErrPromptNotSubmitted) { + t.Fatalf("err=%v, want ErrPromptNotSubmitted", err) + } + if !strings.Contains(err.Error(), "Pasted text") { + t.Fatalf("err=%v, want the stalled editor content quoted", err) + } +} + +// A confirmation that cannot read the pane must not become an acknowledgement. +func TestConfirmLaunchSurfacesTransportErrors(t *testing.T) { + b := fakeTmux(t, 0, true) + if _, err := b.ConfirmLaunch(context.Background(), Session{PaneID: "s:1.0"}, LaunchReference); err == nil { + t.Fatal("a failed pane capture must not confirm a launch") + } +}