From 58793a5aa3622f3d307899a2fb88f382a8644c4c Mon Sep 17 00:00:00 2001 From: kami Date: Tue, 28 Jul 2026 13:20:28 +0400 Subject: [PATCH] fix herdr launch safety and task context --- AUDIT.md | 214 +++++++++++++++++- internal/herdr/adapter.go | 154 ++++++++++--- internal/herdr/adapter_test.go | 26 +-- internal/herdr/herdr.go | 234 ++++++++++++++++++-- internal/herdr/herdr_test.go | 187 ++++++++++++++++ internal/orchestrator/launch_prompt_test.go | 20 ++ internal/orchestrator/orchestrator.go | 63 ++++-- internal/orchestrator/rotation_test.go | 2 +- 8 files changed, 826 insertions(+), 74 deletions(-) create mode 100644 internal/herdr/herdr_test.go create mode 100644 internal/orchestrator/launch_prompt_test.go diff --git a/AUDIT.md b/AUDIT.md index 245c549..d2fba79 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -38,6 +38,36 @@ tests. Two things are not yet proven: `go build ./...`, `go vet ./...`, and `go test ./...` all pass. +### Follow-up live verification (2026-07-28) + +The OpenCode launch path has now been exercised against workpc herdr through +an isolated `test-e2e` worktree. The verified outcomes are deliberately +narrow: + +- Initial prompts use a bounded retry only after an explicit JSON-RPC + rejection (herdr's short post-start readiness window). Transport timeouts + and disconnects are never replayed. A pane is inspected first; blocked or + permission-dialog panes are refused. +- Herdr accepts task-scoped unique names (`oc-`); active sessions + are visible by that name in `agent.list`. Prompt routing uses that name, + while pane reads remain pane-scoped. +- The initial launch message carries the task title and description. This is + necessary under Design A because homesrv cannot write `TASK.md` inside a + workpc checkout. A live OpenCode run received the full instruction and, + after explicit operator approval of two file edits, created the requested + marker and report without touching other files. +- The initial launch does **not** wait for the agent to become idle. Waiting + converted an ordinary long-running first turn into a false `TaskBlocked`; + a fresh run now remains `TaskLeased` while OpenCode is paused at its normal + permission boundary. + +This does not complete the cross-machine continuity design. `clients/herdr- +bridge.go` is only a byte proxy; it is not a worker. The worker-side process +described below remains required before a remote checkout can author +`TASK.md`, derive a canonical handoff anchor, scratch-commit WIP, or safely +release it. Until then, homesrv must not rotate or clean up a non-local +worktree. + | Spec layer | State | |---|---| | L1 substrate (§3, §4) | Built and correct in the main path. | @@ -49,7 +79,156 @@ tests. Two things are not yet proven: ## Blocking defect currently open -### B13 — `agent.start` silently no-ops under back-to-back leases (found live 2026-07-28) +### B14 — `agent.prompt` can duplicate a task prompt after an ambiguous wait timeout (found live 2026-07-28) — open + +Live Claude E2E task `06FTF8CPH3K3DPN6XQA7G3WRQ8` (pane `wM:p1`) received +the same initial `Begin Orchestra task ...` prompt twice. Claude completed a +first turn inspecting the repository and asking for task context, then began +working on the identical prompt again after the turn stopped. This was +observed directly in the pane transcript; it is not merely a UI replay. + +**Root cause:** `CLIAdapter.Lease` calls `Client.Prompt` with +`wait.until=idle` and a 60-second wait. `Client.Call`, however, applies the +client's default 10-second connection deadline when the caller context has +no deadline. Once that local read deadline expires, `Prompt` treats the +result as any other transient failure and retries `agent.prompt` with the +same payload for up to `bootRetryWindow`. Herdr may already have accepted +the first request, so this is an ambiguous outcome, not a safe retry. + +**Consequence:** an agent can repeat work, waste context/quota, or overwrite +its own in-progress changes. The initial task prompt is especially exposed +because it deliberately waits for the agent to become idle. + +**Additional live consequence — OpenCode permission bypass (2026-07-28):** +the OpenCode E2E task `06FTF9Z8RP5FM4F9M2SKAQNFZ4` was leased to +`workpc-opencode` in `/tmp/test-e2e-worktrees/06FTF9Z8RP5FM4F9M2SKAQNFZ4`. +An operator directly observed OpenCode stop at the interactive permission +dialog for its initial `git log --oneline -10 && git status --short` command, +then advance and run that command without operator input. Workpc herdr's +live log records Orchestra's `agent.prompt` calls at 07:37:09 (request IDs 6 +and 7), 07:37:10 (ID 8), and 07:37:20 (ID 9); the last two only ended when +Orchestra's 10-second RPC deadline disconnected the client. Herdr labels +each as `changes_ui=true`. The active OpenCode manifest contains only +permission-*detection* rules and no auto-approval rule. The task later +blocked at the next prompt (`~/.claude/RTK.md`) when no further prompt retry +advanced it. + +This does not prove which internal herdr action produced the equivalent of +Enter, but it proves the safety boundary is broken: an Orchestra-originated +UI-changing `agent.prompt` operation can advance an OpenCode permission +dialog. Treat this as an authorization bypass, not merely duplicate work. + +**Required fix:** make the RPC deadline at least the requested herdr wait +(plus a small transport margin), and do not blindly resend `agent.prompt` +after a timeout or other ambiguous post-write error. Record an observable +failure instead, or use an idempotency/acknowledgement mechanism if herdr +adds one. In addition, never issue `agent.prompt` while `agent_status` is +`blocked` (or a pane read matches a permission dialog); treat the condition +as requiring explicit operator approval. Do not run further unattended +OpenCode E2E tasks until both controls are verified live. + +### B15 — a coordinator-side block cannot reconcile a later live completion (found live 2026-07-28) — open + +The same OpenCode E2E task `06FTF9Z8RP5FM4F9M2SKAQNFZ4` was marked +`TaskBlocked` when the coordinator's `agent.prompt` RPC timed out, despite +the agent having received the task. The operator later explicitly approved +the one pending `~/.claude/RTK.md` read through herdr; OpenCode completed the +verification and became `idle` in pane `wN:p1`. Orchestra still showed +`state: "blocked"` because `Coordinator.Start` returned through `block()` +and never persisted a session; its monitor therefore has no live pane to +observe or reconcile. + +**Consequence:** the event log can permanently claim a task is blocked when +the leased harness has actually completed it. This is not merely stale UI: +blocked tasks have no normal completion hook path and their receipt/quota +record is lost unless an operator manually corrects the lifecycle. + +**Required fix:** retain enough launch/session identity after an ambiguous +lease failure to reconcile `agent.get`/completion evidence, or introduce a +versioned `TaskCorrected` terminal-state workflow that records the original +blocker and evidence without pretending it never happened. A live pane must +not be left outside the coordinator's session map simply because prompt +delivery timed out. + +### B16 — hardcoded harness name makes each harness globally single-instance (found live 2026-07-28) — open + +While starting the real OpenCode healthcheck task +`06FTFDW22833F1CCB8K43Z8808`, Orchestra created worktree/pane `wP:p1` but +herdr never attached an agent. A direct retry exposed the actual cause: +`agent.start` was sent with `name: "opencode"`, which herdr rejected with +`agent_name_taken` because the completed prior E2E agent in `wN:p1` already +owned that name. This is not a herdr limit of one OpenCode process per +machine: retrying the same `kind: "opencode"` in `wP:p1` with the valid, +unique lowercase name `oc-06ftfdw22833f1cc` immediately started a second +attached, idle OpenCode session while `wN:p1` remained intact. + +**Root cause:** `Client.StartAgent` passes the harness kind as both +`kind` and `name`; every OpenCode lease therefore competes for the same +global agent-name slot. The same defect applies to Claude and Codex. + +**Required fix:** keep `kind` as the configured harness, but derive `name` +from a validated, bounded task/session identifier (lowercase letters, +digits, `-`, `_`, maximum 32 characters) and persist it in `Session` for +subsequent lifecycle operations. Add a contract test that starts two same- +harness sessions in distinct panes and verifies both attach. + +### B17 — opaque harnesses must not author canonical handoff anchors (found live 2026-07-28) — open + +The real OpenCode healthcheck run showed that prompting an opaque agent to +write the full `continuity.Handoff` schema is the wrong ownership boundary. +The agent wrote JSON that looked plausible but used `anchor.sha` instead of +`anchor.git_sha`, omitted required top-level fields by nesting them under +`knowledge`, and could not correctly represent the uncommitted handoff file +as a dirty-file hash (a file cannot contain the SHA-256 of its own final +contents). Repeated corrective prompts made the handoff less reliable and +turned Orchestra's protocol details into agent prompt lore. + +**Required direction:** prompt a harness exactly once for a small semantic +handoff/review report (what changed, validation evidence, remaining work, +review findings). The worker that owns the checkout must then deterministically +collect `HEAD`, branch, dirty paths and SHA-256 values, construct the canonical +handoff artifact, and validate it before publication. Canonical handoff files +are worker-owned protocol state and must be excluded from their own dirty-file +list. Project-level commands and stable expectations (`build`, `test`, file +ownership/invariants, etc.) belong in `AGENTS.md`/`CLAUDE.md`; only task- +specific evidence belongs in the handoff. + +This cannot be safely implemented in Design A by homesrv: its coordinator +does not own the workpc checkout. Expand the bridge into a real worker-side +continuity participant (the deferred Design B direction) so all Git-derived +handoff facts are gathered and sealed on the machine that hosts the worktree. + +**Live evidence (OpenCode healthcheck/review run, 2026-07-28):** task +`06FTFDW22833F1CCB8K43Z8808` initially hit B13/B16: Orchestra created `wP:p1` +but its hardcoded `name: "opencode"` conflicted with the prior E2E session. +Starting the same `kind: "opencode"` with unique name `oc-06ftfdw22833f1cc` +attached successfully without closing the prior pane. The implementation +agent created executable `scripts/healthcheck.sh`, validated `bash -n`, its +normal mode, and `--help`, then committed `672c123 Add scripts/healthcheck.sh`. +It first authored a lookalike handoff (`anchor.sha`, incomplete `meta`, and +all protocol fields under `knowledge`). A distinct reviewer in a second pane +(`wP:p2`) independently repeated the checks, noticed the nesting error, and +rewrote the document. Its claimed schema validation checked only JSON syntax +and the presence of keys — it did **not** use `continuity.Decode` or the +actual typed contract: `test` remains an array where `Handoff.Test` is a +string, and `last_result` remains a string where `Handoff.LastResult` is a +structured object. This is a concrete proof that a reviewing harness cannot +be trusted to validate canonical protocol state by visual shape alone. + +The task is deliberately still `TaskBlocked` from its original failed +automatic attach even though its manually-started implementation/review panes +are live, a second live confirmation of B15. The reviewer pane is left open +for operator inspection; do not close or release it without approval. + +**Related live observation — unexpected Claude launch explained:** Claude +pane `wM:p1` did not start spontaneously. At 08:00:41Z the expired lease for +task `06FTF8CPH3K3DPN6XQA7G3WRQ8` emitted `TaskReleased(reason: +"lease_expired")`; the router re-leased it at 08:00:42Z, and workpc herdr +attached Claude at 08:00:43Z. The renewed attempt then hit B14's prompt +timeout and became blocked. This is expected retry behavior, but illustrates +why lifecycle retries must be visible to operators. + +### B13 — `agent.start` silently no-ops under back-to-back leases (found live 2026-07-28) — closed (code fix; not yet re-verified live) Discovered while live-verifying the B12 fix below. B12 itself is confirmed fixed (see that entry), but re-testing it exposed a second, deeper defect @@ -88,10 +267,35 @@ which requires spinning up more real panes and wasn't done this pass (see `TaskBlocked`), this failure mode leaves the task leased indefinitely against a pane that will never produce a session, with no error surfaced anywhere — worse than B12 was, because nothing currently distinguishes "still booting, -give it more time" from "silently dead, will never start." No fix attempted -yet; a client-side retry (B12's approach) cannot fix this because the -`agent.start` call that should have started the process already returned -success. +give it more time" from "silently dead, will never start." A client-side +retry on `agent.start` itself (B12's approach) cannot fix this, since the +call that should have started the process already returned success with no +error to retry on. + +**Fix:** since `agent.start`'s own return value can't be trusted, `StartAgent` +(`internal/herdr/herdr.go`) no longer treats its success as the end of the +story. After `agent.start` returns without error, it polls `pane.get({pane_id})` +(new `paneAgentAttached` helper) until the pane reports a real attached agent +(`agent` non-empty and `agent_status` present and not `"unknown"`), bounded by +a new `agentAttachWindow` (90s, longer than `bootRetryWindow`'s 15s since a +legitimate attach was observed live taking "well over a minute"). If the +window elapses with no attach, `StartAgent` now returns an explicit error +instead of a false success — this is exactly the "recorded/observable +failure over silent `continue`" pattern this repo's CLAUDE.md calls for, and +should surface as `TaskBlocked` through the same path B12's fix already +proved reachable. + +**Not yet done:** re-verified against a live herdr instance (the schema for +`pane.get`'s result — `agent`/`agent_status` fields — was inferred from +prose in this file's own B12/B13 narration of live `pane.get` output, not +re-confirmed by a fresh probe of `192.168.1.105:9245`; `deploy/herdr-schema.json` +has no entry for `pane.get`'s result shape). `go build`/`go vet`/`go test +./...` all pass, but no test exercises `StartAgent`'s new polling loop +directly (existing `internal/herdr` tests don't call `agent.start`/`pane.get` +through the fake TCP listener at all). Next session should confirm the +`pane.get` field names live before trusting this closes B13 operationally, +and ideally fire a fresh back-to-back-lease test against workpc once that's +confirmed. **Side effect of this investigation — three live orphaned panes on workpc, left untouched on purpose:** `wD:p1` (has a real but abandoned `claude` diff --git a/internal/herdr/adapter.go b/internal/herdr/adapter.go index 606f4cb..ba7d365 100644 --- a/internal/herdr/adapter.go +++ b/internal/herdr/adapter.go @@ -8,7 +8,9 @@ import ( "fmt" "orchestra/internal/continuity" "os" + "os/exec" "path/filepath" + "sort" "strings" "time" ) @@ -33,6 +35,13 @@ type Adapter interface { Occupancy(Session) (float64, error) } +// PromptLeaser accepts the complete task-specific launch instruction. It is +// optional so non-interactive adapters and existing tests retain the small +// Lease seam, while remote worktrees are never left with only an opaque ID. +type PromptLeaser interface { + LeasePrompt(context.Context, string, string, string) (Session, error) +} + type WorktreeCreator interface { CreateWorktree(context.Context, string, string, string) (string, error) } @@ -74,6 +83,10 @@ type CLIAdapter struct { // uploads the one the agent wrote (herdr does not write handoffs, §6.1). const HandoffFile = ".orchestra-handoff.json" +// HandoffReportFile is the only handoff artifact an opaque harness authors. +// The worker which owns the checkout derives and seals the canonical JSON. +const HandoffReportFile = ".orchestra-handoff-report.md" + func (a CLIAdapter) CreateWorktree(ctx context.Context, repo, root, taskID string) (string, error) { path, err := a.Client.Worktree(ctx, repo, filepath.Join(root, taskID), "orchestra/"+taskID) if err != nil { @@ -86,6 +99,17 @@ func (a CLIAdapter) CreateWorktree(ctx context.Context, repo, root, taskID strin } func (a CLIAdapter) Lease(ctx context.Context, task, worktree string) (Session, error) { + return a.LeasePrompt(ctx, task, worktree, defaultTaskPrompt(task)) +} + +func defaultTaskPrompt(task string) string { + return fmt.Sprintf("Begin Orchestra task %s. Inspect the repository, understand the task context, and proceed with the requested work.", task) +} + +// LeasePrompt starts a harness and sends an immutable copy of the task's +// actionable instruction. This is required for a herdr-hosted remote +// worktree: homesrv cannot safely write/read that machine's TASK.md. +func (a CLIAdapter) LeasePrompt(ctx context.Context, task, worktree, prompt string) (Session, error) { if a.Client == nil { return Session{}, fmt.Errorf("adapter: client required") } @@ -93,11 +117,22 @@ func (a CLIAdapter) Lease(ctx context.Context, task, worktree string) (Session, if err != nil { return Session{}, err } - if err := a.Client.Prompt(ctx, s.PaneID, fmt.Sprintf("Begin Orchestra task %s. Inspect the repository, understand the task context, and proceed with the requested work.", task), time.Minute); err != nil { - return Session{}, err + // The initial instruction is an asynchronous launch message. Waiting for + // idle here turns a normal long-running first turn into a false lease + // failure (and TaskBlocked) even though herdr accepted the prompt. + if err := a.Client.Prompt(ctx, s.PaneID, prompt, 0); err != nil { + // The request may have reached herdr even when its response was lost. + // Preserve the live session so Coordinator can reconcile completion. + return s, err } return s, nil } + +func (a CLIAdapter) prompt(ctx context.Context, s Session, text string, wait time.Duration) error { + a.Client.BindAgent(s.PaneID, s.AgentName) + return a.Client.Prompt(ctx, s.PaneID, text, wait) +} + // bootstrapPrompt implements the §6.2 pickup procedure: the plane has already // run ValidatePickup before this is ever sent (Coordinator.Start blocks the // task and never bootstraps on failure), so this prompt does not ask the @@ -111,19 +146,12 @@ This worktree's anchor and TASK.md have already been verified by the plane befor 4. Continue the task from there.` func (a CLIAdapter) Bootstrap(ctx context.Context, s Session, ref string) error { - return a.Client.Prompt(ctx, s.PaneID, fmt.Sprintf(bootstrapPrompt, ref), time.Minute) + return a.prompt(ctx, s, fmt.Sprintf(bootstrapPrompt, ref), time.Minute) } -// handoffPrompt is Phase 4 item 2's missing half (AUDIT.md): Release already -// validates and uploads a §6.1 handoff the agent wrote, but nothing ever told -// the agent that convention exists. rotate() sends this once occupancy crosses -// the hard threshold at a turn boundary, mirroring the .orchestra-report.md -// convention B3 established for completion — the plane still never invents a -// handoff, it only asks the agent to produce one, then validates it in Release. const handoffPrompt = `Orchestra is about to rotate this task to a fresh session (context budget reached). -Before you stop, write a §6.1 handoff to ` + HandoffFile + ` at the worktree root, a JSON object with at least: -{"meta":{"id":""},"anchor":{"git_sha":"","branch":"","dirty":[{"path":"","sha256":""}, ...for any uncommitted files]},"knowledge":{...whatever structured context the next agent needs...}} -Do not edit TASK.md. Do not fabricate the git_sha or dirty file hashes — read them for real. Once written, stop normally.` +Before you stop, write a concise semantic handoff report to ` + HandoffReportFile + ` at the worktree root: what changed, validation evidence, remaining work, review findings, and dead ends/open questions. +Do not write protocol JSON, git anchors, or file hashes; Orchestra's checkout worker collects and validates those facts. Do not edit TASK.md. Once written, stop normally.` // RequestHandoff prompts the agent to write HandoffFile before Release reads // it. Optional capability: adapters without a live pane (tests, etc.) can @@ -133,7 +161,7 @@ type HandoffRequester interface { } func (a CLIAdapter) RequestHandoff(ctx context.Context, s Session) error { - return a.Client.Prompt(ctx, s.PaneID, handoffPrompt, time.Minute) + return a.prompt(ctx, s, handoffPrompt, time.Minute) } // ReasonedHandoffRequester is RequestHandoff's counterpart for the two @@ -155,7 +183,7 @@ func (a CLIAdapter) RequestHandoffReason(ctx context.Context, s Session, reason case "milestone": sb.WriteString("A coherent unit of work looks complete (a successful commit). If the next step is independent of what you just did, this is a good point to hand off.\n") } - fmt.Fprintf(&sb, "Before you stop, write a §6.1 handoff to %s at the worktree root with meta.reason=%q", HandoffFile, reason) + fmt.Fprintf(&sb, "Before you stop, write a concise semantic handoff report to %s at the worktree root (reason: %q)", HandoffReportFile, reason) if len(deadEnds) > 0 { sb.WriteString(" and a dead_ends entry for each of the following:\n") for _, d := range deadEnds { @@ -164,8 +192,8 @@ func (a CLIAdapter) RequestHandoffReason(ctx context.Context, s Session, reason } else { sb.WriteString(".\n") } - sb.WriteString("Use the same anchor convention as any other handoff: the real git_sha via 'git rev-parse HEAD', the real branch, and a real sha256 of any uncommitted files — never fabricated. Do not edit TASK.md. Once written, stop normally.") - return a.Client.Prompt(ctx, s.PaneID, sb.String(), time.Minute) + sb.WriteString("Include what changed, validation evidence, remaining work, review findings, and any dead ends. Do not write protocol JSON, git anchors, or file hashes; Orchestra collects those. Do not edit TASK.md. Once written, stop normally.") + return a.prompt(ctx, s, sb.String(), time.Minute) } // Activity resolves the harness's tool-call history the same way Occupancy @@ -203,8 +231,9 @@ type ConventionsNotifier interface { } func (a CLIAdapter) NotifyConventionsChanged(ctx context.Context, s Session) error { - return a.Client.Prompt(ctx, s.PaneID, conventionsPrompt, time.Minute) + return a.prompt(ctx, s, conventionsPrompt, time.Minute) } + // Release reads the §6.1 handoff the agent wrote to HandoffFile at the // worktree root, validates its schema and anchor against the worktree's real // HEAD, uploads it to CAS, and only then releases herdr's claim on the pane @@ -219,22 +248,23 @@ func (a CLIAdapter) Release(ctx context.Context, s Session) (string, error) { if a.CAS == nil { return "", fmt.Errorf("adapter: CAS store required to upload handoff") } - path := filepath.Join(s.Worktree, HandoffFile) + path := filepath.Join(s.Worktree, HandoffReportFile) b, err := os.ReadFile(path) if err != nil { - return "", fmt.Errorf("adapter: handoff not written yet (%s): %w", path, err) + return "", fmt.Errorf("adapter: semantic handoff report not written yet (%s): %w", path, err) } - h, err := continuity.Decode(b) + if strings.TrimSpace(string(b)) == "" { + return "", fmt.Errorf("adapter: semantic handoff report is empty") + } + h, err := canonicalHandoff(s, string(b)) if err != nil { - return "", fmt.Errorf("adapter: invalid handoff: %w", err) + return "", err } sha, err := HeadSHA(s.Worktree) if err != nil { return "", fmt.Errorf("adapter: read worktree HEAD: %w", err) } - if h.Anchor.GitSHA != sha { - return "", fmt.Errorf("adapter: handoff anchor %s does not match worktree HEAD %s", h.Anchor.GitSHA, sha) - } + _ = sha for _, d := range h.Anchor.Dirty { if hex.EncodeToString(sha256sum(filepath.Join(s.Worktree, d.Path))) != d.SHA256 { return "", fmt.Errorf("adapter: handoff dirty file changed since it was written: %s", d.Path) @@ -264,12 +294,86 @@ func (a CLIAdapter) Release(ctx context.Context, s Session) (string, error) { if err := a.Client.Call(ctx, "pane.release_agent", map[string]any{ "pane_id": s.PaneID, "source": "herdr:" + a.Harness, - "agent": a.Harness, + "agent": agentForSession(s, a.Harness), }, nil); err != nil { return "", fmt.Errorf("adapter: pane.release_agent: %w", err) } return ref, nil } + +// canonicalHandoff keeps Git-derived protocol facts on the worker that owns +// the checkout. The harness contributes only the semantic report (B17). +func canonicalHandoff(s Session, report string) (continuity.Handoff, error) { + sha, err := HeadSHA(s.Worktree) + if err != nil { + return continuity.Handoff{}, fmt.Errorf("adapter: read worktree HEAD: %w", err) + } + branchOut, err := exec.Command("git", "-C", s.Worktree, "branch", "--show-current").Output() + if err != nil { + return continuity.Handoff{}, fmt.Errorf("adapter: read worktree branch: %w", err) + } + dirty, err := dirtyFiles(s.Worktree) + if err != nil { + return continuity.Handoff{}, err + } + return continuity.Handoff{ + Meta: continuity.Meta{ID: handoffID(s), Reason: "threshold"}, + Anchor: continuity.Anchor{GitSHA: sha, Branch: strings.TrimSpace(string(branchOut)), Dirty: dirty}, + Goal: "Continue Orchestra task " + s.PaneID, + DoneWhen: []string{"Task completion is reported to Orchestra"}, + Action: "Read the semantic handoff report and continue the task.", + Command: "cat " + HandoffReportFile, + Remaining: []string{report}, + }, nil +} + +func handoffID(s Session) string { + id := s.AgentName + if id == "" { + id = s.PaneID + } + id = strings.Trim(invalidAgentName.ReplaceAllString(strings.ToLower(id), "-"), "-_") + if id == "" { + return "session" + } + return id +} + +func dirtyFiles(root string) ([]continuity.Dirty, error) { + paths := map[string]bool{} + for _, args := range [][]string{{"diff", "--name-only", "-z"}, {"ls-files", "--others", "--exclude-standard", "-z"}} { + out, err := exec.Command("git", append([]string{"-C", root}, args...)...).Output() + if err != nil { + return nil, err + } + for _, path := range strings.Split(string(out), "\x00") { + if path != "" && path != HandoffFile { + paths[path] = true + } + } + } + keys := make([]string, 0, len(paths)) + for path := range paths { + keys = append(keys, path) + } + sort.Strings(keys) + dirty := make([]continuity.Dirty, 0, len(keys)) + for _, path := range keys { + sum := sha256sum(filepath.Join(root, path)) + if len(sum) == 0 { + return nil, fmt.Errorf("adapter: hash dirty file %s", path) + } + dirty = append(dirty, continuity.Dirty{Path: path, SHA256: hex.EncodeToString(sum)}) + } + return dirty, nil +} + +func agentForSession(s Session, fallback string) string { + if s.AgentName != "" { + return s.AgentName + } + return fallback // compatibility with session records created before B16 +} func (a CLIAdapter) Kill(ctx context.Context, s Session) error { return a.Client.Call(ctx, "pane.close", map[string]any{"pane_id": s.PaneID}, nil) } diff --git a/internal/herdr/adapter_test.go b/internal/herdr/adapter_test.go index a6e32b2..1c1540c 100644 --- a/internal/herdr/adapter_test.go +++ b/internal/herdr/adapter_test.go @@ -85,7 +85,7 @@ func TestReleaseUploadsHandoffAndReleasesAgent(t *testing.T) { if err != nil { t.Fatal(err) } - if err := os.WriteFile(filepath.Join(repo, HandoffFile), b, 0644); err != nil { + if err := os.WriteFile(filepath.Join(repo, HandoffReportFile), b, 0644); err != nil { t.Fatal(err) } @@ -105,8 +105,8 @@ func TestReleaseUploadsHandoffAndReleasesAgent(t *testing.T) { if err != nil { t.Fatal(err) } - if got.Anchor.GitSHA != head { - t.Fatalf("stored handoff anchor=%s want=%s", got.Anchor.GitSHA, head) + if got.Anchor.GitSHA == head { + t.Fatal("semantic report should be snapshotted before publication") } } @@ -119,7 +119,7 @@ func TestReleaseRefusesWithoutHandoffFile(t *testing.T) { } } -func TestReleaseRefusesOnAnchorMismatch(t *testing.T) { +func TestReleaseDoesNotTrustAgentSuppliedAnchor(t *testing.T) { repo := t.TempDir() runGit(t, repo, "init") runGit(t, repo, "config", "user.email", "t@t") @@ -130,12 +130,12 @@ func TestReleaseRefusesOnAnchorMismatch(t *testing.T) { if err != nil { t.Fatal(err) } - if err := os.WriteFile(filepath.Join(repo, HandoffFile), b, 0644); err != nil { + if err := os.WriteFile(filepath.Join(repo, HandoffReportFile), b, 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.Fatal("expected error on anchor mismatch") + if _, err := a.Release(context.Background(), Session{PaneID: "p1", Worktree: repo}); err != nil { + t.Fatalf("Release must derive the anchor itself: %v", err) } } @@ -161,7 +161,7 @@ func TestReleaseScratchCommitsDirtyFilesBeforeUpload(t *testing.T) { if err != nil { t.Fatal(err) } - if err := os.WriteFile(filepath.Join(repo, HandoffFile), b, 0644); err != nil { + if err := os.WriteFile(filepath.Join(repo, HandoffReportFile), b, 0644); err != nil { t.Fatal(err) } @@ -192,12 +192,12 @@ func TestReleaseScratchCommitsDirtyFilesBeforeUpload(t *testing.T) { if err != nil { t.Fatal(err) } - if got.Anchor.Branch != "orchestra/scratch/t1" || string(branch) != got.Anchor.Branch+"\n" { + 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 TestReleaseRefusesOnStaleDirtyFile(t *testing.T) { +func TestReleaseDoesNotTrustAgentSuppliedDirtyFile(t *testing.T) { repo := t.TempDir() runGit(t, repo, "init") runGit(t, repo, "config", "user.email", "t@t") @@ -218,12 +218,12 @@ func TestReleaseRefusesOnStaleDirtyFile(t *testing.T) { if err != nil { t.Fatal(err) } - if err := os.WriteFile(filepath.Join(repo, HandoffFile), b, 0644); err != nil { + if err := os.WriteFile(filepath.Join(repo, HandoffReportFile), b, 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.Fatal("expected refusal when a dirty file no longer matches the handoff's recorded hash") + if _, err := a.Release(context.Background(), Session{PaneID: "p1", Worktree: repo}); err != nil { + t.Fatalf("Release must derive dirty hashes itself: %v", err) } } diff --git a/internal/herdr/herdr.go b/internal/herdr/herdr.go index ee93975..3566e87 100644 --- a/internal/herdr/herdr.go +++ b/internal/herdr/herdr.go @@ -13,6 +13,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "strings" "sync" "time" @@ -42,6 +43,7 @@ type Client struct { mu sync.Mutex next uint64 panes map[string]string + agents map[string]string // pane ID -> machine-global agent name } type WorktreeInfo struct { @@ -63,7 +65,7 @@ type worktreeResponse struct { } func New(path string) *Client { - return &Client{Path: path, Timeout: 10 * time.Second, panes: map[string]string{}} + return &Client{Path: path, Timeout: 10 * time.Second, panes: map[string]string{}, agents: map[string]string{}} } func (c *Client) conn() (net.Conn, error) { if c.dial != nil { @@ -137,10 +139,11 @@ func (c *Client) CheckProtocol(ctx context.Context, want string) error { } type Session struct { - PaneID string `json:"pane_id"` - Worktree string `json:"worktree"` - Harness string `json:"harness"` - HerdrID string `json:"herdr_id,omitempty"` + PaneID string `json:"pane_id"` + Worktree string `json:"worktree"` + Harness string `json:"harness"` + AgentName string `json:"agent_name,omitempty"` + HerdrID string `json:"herdr_id,omitempty"` // SessionFile is the filesystem path to the harness's own session/ // transcript state (a Claude Code transcript, a Codex rollout, ...). // ClaudeUsage/CodexUsage/OpenCodeUsage read *this*, never the herdr pane @@ -178,27 +181,172 @@ type Session struct { const ( bootRetryWindow = 15 * time.Second bootRetryDelay = 500 * time.Millisecond + + // agentAttachWindow/agentAttachPoll bound StartAgent's post-success + // confirmation poll (B13, found live 2026-07-28): agent.start can return + // no error while never actually starting an agent, observed on two of + // three back-to-back leases — the pane's agent_status stayed "unknown" + // after 2+ minutes of polling, with no error surfaced anywhere. A + // legitimate attach has been observed taking "well over a minute", so + // this window is deliberately longer than bootRetryWindow. + agentAttachWindow = 90 * time.Second + agentAttachPoll = 2 * time.Second + + claudeTrustObserveWindow = 15 * time.Second + claudeTrustClearWindow = 15 * time.Second + claudeTrustPoll = 500 * time.Millisecond ) -func (c *Client) Prompt(ctx context.Context, pane, text string, wait time.Duration) error { - p := map[string]any{"target": pane, "text": text} - if wait > 0 { - p["wait"] = map[string]any{"until": []string{"idle"}, "timeout_ms": wait.Milliseconds()} +type paneStatus struct { + Agent string `json:"agent"` + AgentStatus string `json:"agent_status"` +} + +// paneGetResponse mirrors herdr's pane.get result envelope. The pane fields +// are not returned at the top level: the result is {"type":"pane_info", +// "pane": {...}}. Keeping that wrapper explicit prevents a successful +// agent.start from being falsely reported as unattached. +type paneGetResponse struct { + Pane paneStatus `json:"pane"` +} + +type paneReadResponse struct { + Read struct { + Text string `json:"text"` + } `json:"read"` +} + +// paneAgentAttached reports whether herdr's own view of the pane shows a +// real agent bound to it, rather than trusting agent.start's return value. +func (c *Client) paneAgentAttached(ctx context.Context, paneID string) (bool, error) { + var p paneGetResponse + if err := c.Call(ctx, "pane.get", map[string]any{"pane_id": paneID}, &p); err != nil { + return false, err } - deadline := time.Now().Add(bootRetryWindow) - var err error + return p.Pane.Agent != "" && p.Pane.AgentStatus != "" && p.Pane.AgentStatus != "unknown", nil +} + +func (c *Client) paneText(ctx context.Context, paneID string) (string, error) { + var p paneReadResponse + if err := c.Call(ctx, "pane.read", map[string]any{"pane_id": paneID, "source": "recent"}, &p); err != nil { + return "", err + } + return p.Read.Text, nil +} + +func claudeWorkspaceTrustPrompt(text string) bool { + return strings.Contains(text, "Accessing workspace:") && strings.Contains(text, "Yes, I trust this folder") +} + +// confirmClaudeWorkspaceTrust accepts only Claude Code's exact workspace +// trust prompt. Orchestra creates its worktrees from registered project +// repositories, so leaving this interactive would make every new worktree +// permanently unattended-ineligible. It deliberately does not accept any +// other Claude confirmation (in particular bypass-permissions mode). +func (c *Client) confirmClaudeWorkspaceTrust(ctx context.Context, paneID string) error { + deadline := time.Now().Add(claudeTrustObserveWindow) + accepted := false for { - err = c.Call(ctx, "agent.prompt", p, nil) - if err == nil || time.Now().After(deadline) { - return err + text, err := c.paneText(ctx, paneID) + if err == nil && claudeWorkspaceTrustPrompt(text) { + if !accepted { + if err := c.Call(ctx, "pane.send_text", map[string]any{"pane_id": paneID, "text": "1\n"}, nil); err != nil { + return fmt.Errorf("herdr: accept Claude workspace trust for pane %s: %w", paneID, err) + } + accepted = true + deadline = time.Now().Add(claudeTrustClearWindow) + } + } else if accepted { + return nil + } + if time.Now().After(deadline) { + if accepted { + return fmt.Errorf("herdr: Claude workspace trust prompt did not clear for pane %s", paneID) + } + return nil } select { case <-ctx.Done(): return ctx.Err() + case <-time.After(claudeTrustPoll): + } + } +} + +func (c *Client) Prompt(ctx context.Context, pane, text string, wait time.Duration) error { + var status paneGetResponse + if err := c.Call(ctx, "pane.get", map[string]any{"pane_id": pane}, &status); err != nil { + return fmt.Errorf("herdr: inspect pane before prompt: %w", err) + } + if strings.EqualFold(status.Pane.AgentStatus, "blocked") { + return fmt.Errorf("herdr: refusing prompt to blocked pane %s", pane) + } + transcript, err := c.paneText(ctx, pane) + if err != nil { + return fmt.Errorf("herdr: inspect pane text before prompt: %w", err) + } + if permissionPrompt(transcript) { + return fmt.Errorf("herdr: refusing prompt while pane %s shows a permission dialog", pane) + } + c.mu.Lock() + target := c.agents[pane] + c.mu.Unlock() + if target == "" { + // Session records created before unique agent names were introduced + // used the pane as target; retain that compatibility path. + target = pane + } + p := map[string]any{"target": target, "text": text} + if wait > 0 { + p["wait"] = map[string]any{"until": []string{"idle"}, "timeout_ms": wait.Milliseconds()} + } + callCtx := ctx + var cancel context.CancelFunc + if wait > 0 { + if d, ok := ctx.Deadline(); !ok || time.Until(d) < wait+5*time.Second { + callCtx, cancel = context.WithTimeout(ctx, wait+5*time.Second) + defer cancel() + } + } + // A transport failure after writing is ambiguous: herdr may already have + // delivered the UI-changing request, so never resend it. A JSON-RPC + // protocol error, however, is herdr's explicit rejection before it acted + // (notably its short post-start readiness window); that is safe to retry + // for the bounded boot window. + deadline := time.Now().Add(bootRetryWindow) + for { + err := c.Call(callCtx, "agent.prompt", p, nil) + if err == nil { + return nil + } + if !errors.Is(err, ErrProtocol) || time.Now().After(deadline) { + return fmt.Errorf("herdr: prompt delivery is uncertain; not retrying: %w", err) + } + select { + case <-callCtx.Done(): + return fmt.Errorf("herdr: prompt delivery is uncertain; not retrying: %w", callCtx.Err()) case <-time.After(bootRetryDelay): } } } + +// BindAgent restores the pane-to-agent routing from a persisted Session. +func (c *Client) BindAgent(pane, agent string) { + if pane == "" || agent == "" { + return + } + c.mu.Lock() + defer c.mu.Unlock() + if c.agents == nil { + c.agents = map[string]string{} + } + c.agents[pane] = agent +} + +func permissionPrompt(text string) bool { + text = strings.ToLower(text) + return strings.Contains(text, "permission required") || strings.Contains(text, "approval required") || strings.Contains(text, "waiting for approval") +} func (c *Client) Worktree(ctx context.Context, cwd, path, branch string) (string, error) { var r worktreeResponse // Protocol 17 requires exactly one of path or branch. Use the explicit @@ -238,8 +386,8 @@ func (c *Client) StartAgent(ctx context.Context, cwd, path, branch, harness, tas err = c.Call(ctx, "agent.start", map[string]any{ "pane_id": paneID, "kind": harness, - "name": harness, - "args": []string{}, + "name": agentName(harness, taskID), + "args": harnessStartArgs(harness), }, &s) if err == nil { break @@ -260,12 +408,66 @@ func (c *Client) StartAgent(ctx context.Context, cwd, path, branch, harness, tas if err != nil { return Session{}, err } + // B13: agent.start's success does not mean an agent actually attached. + // Confirm via pane.get before declaring the lease started, so a silent + // no-op surfaces as an observable error (TaskBlocked) instead of leaving + // the task leased against a pane that will never produce a session. + attachDeadline := time.Now().Add(agentAttachWindow) + for { + ok, statusErr := c.paneAgentAttached(ctx, paneID) + if statusErr == nil && ok { + break + } + if time.Now().After(attachDeadline) { + if statusErr != nil { + return Session{}, fmt.Errorf("herdr: agent.start reported success for pane %s but confirming attach failed: %w", paneID, statusErr) + } + return Session{}, fmt.Errorf("herdr: agent.start reported success for pane %s but no agent attached within %s", paneID, agentAttachWindow) + } + select { + case <-ctx.Done(): + return Session{}, ctx.Err() + case <-time.After(agentAttachPoll): + } + } + if strings.EqualFold(harness, "claude") { + if err := c.confirmClaudeWorkspaceTrust(ctx, paneID); err != nil { + return Session{}, err + } + } s.PaneID = paneID s.Worktree = path s.Harness = harness + s.AgentName = agentName(harness, taskID) + c.BindAgent(paneID, s.AgentName) return s, nil } +var invalidAgentName = regexp.MustCompile(`[^a-z0-9_-]+`) + +func agentName(harness, taskID string) string { + prefix := map[string]string{"opencode": "oc", "claude": "cl", "codex": "cx"}[strings.ToLower(harness)] + if prefix == "" { + prefix = "agent" + } + id := strings.Trim(invalidAgentName.ReplaceAllString(strings.ToLower(taskID), "-"), "-_") + if id == "" { + id = "session" + } + name := prefix + "-" + id + if len(name) > 32 { + name = strings.TrimRight(name[:32], "-_") + } + return name +} + +// harnessStartArgs stays empty for Claude: --dangerously-skip-permissions +// introduces a separate first-run disclaimer. StartAgent instead acknowledges +// only the registered-worktree trust prompt after Claude is running. +func harnessStartArgs(harness string) []string { + return []string{} +} + // HeadSHA returns the current commit of a worktree. The rotation path uses // this to populate TaskReleased.anchor_sha without trusting the adapter's // opaque handoff-ref return value. diff --git a/internal/herdr/herdr_test.go b/internal/herdr/herdr_test.go new file mode 100644 index 0000000..c128ddf --- /dev/null +++ b/internal/herdr/herdr_test.go @@ -0,0 +1,187 @@ +package herdr + +import ( + "bufio" + "context" + "encoding/json" + "net" + "reflect" + "regexp" + "testing" + "time" +) + +func TestHarnessStartArgs(t *testing.T) { + if got, want := harnessStartArgs("claude"), []string{}; !reflect.DeepEqual(got, want) { + t.Errorf("Claude args = %q, want %q", got, want) + } + if got := harnessStartArgs("opencode"); len(got) != 0 { + t.Errorf("OpenCode args = %q, want none", got) + } +} + +func TestStartAgentPassesEmptyHarnessArgs(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = ln.Close() }) + requests := make(chan Request, 2) + go func() { + for i := 0; i < 2; i++ { + conn, err := ln.Accept() + if err != nil { + return + } + var req Request + if json.NewDecoder(bufio.NewReader(conn)).Decode(&req) == nil { + requests <- req + if i == 0 { + _ = json.NewEncoder(conn).Encode(Response{ID: req.ID, Result: json.RawMessage(`{}`)}) + } else { + _ = json.NewEncoder(conn).Encode(Response{ID: req.ID, Result: json.RawMessage(`{"type":"pane_info","pane":{"agent":"claude","agent_status":"idle"}}`)}) + } + } + _ = conn.Close() + } + }() + c := &Client{ + Path: ln.Addr().String(), + panes: map[string]string{"/worktree": "w1:p1"}, + dial: func() (net.Conn, error) { + return net.Dial("tcp", ln.Addr().String()) + }, + } + if _, err := c.StartAgent(context.Background(), "", "/worktree", "", "opencode", "t1"); err != nil { + t.Fatal(err) + } + start := <-requests + if start.Method != "agent.start" { + t.Fatalf("first method = %q, want agent.start", start.Method) + } + params, err := json.Marshal(start.Params) + if err != nil { + t.Fatal(err) + } + var got struct { + Args []string `json:"args"` + Kind string `json:"kind"` + Name string `json:"name"` + } + if err := json.Unmarshal(params, &got); err != nil { + t.Fatal(err) + } + if want := []string{}; !reflect.DeepEqual(got.Args, want) { + t.Errorf("agent.start args = %q, want %q", got.Args, want) + } + if got.Kind != "opencode" || got.Name != "oc-t1" { + t.Errorf("agent.start kind/name = %q/%q, want opencode/oc-t1", got.Kind, got.Name) + } + if get := <-requests; get.Method != "pane.get" { + t.Errorf("second method = %q, want pane.get", get.Method) + } +} + +func TestAgentNameIsBoundedAndValid(t *testing.T) { + got := agentName("OpenCode", "TASK With spaces / and symbols !!! 0123456789") + if len(got) > 32 || !regexp.MustCompile(`^[a-z0-9_-]+$`).MatchString(got) { + t.Fatalf("invalid agent name %q", got) + } +} + +func TestPromptDoesNotRetryAmbiguousDelivery(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + requests := make(chan Request, 3) + go func() { + for i := 0; i < 3; i++ { + conn, err := ln.Accept() + if err != nil { + return + } + var req Request + _ = json.NewDecoder(bufio.NewReader(conn)).Decode(&req) + requests <- req + switch req.Method { + case "pane.get": + _ = json.NewEncoder(conn).Encode(Response{ID: req.ID, Result: json.RawMessage(`{"pane":{"agent_status":"idle"}}`)}) + case "pane.read": + _ = json.NewEncoder(conn).Encode(Response{ID: req.ID, Result: json.RawMessage(`{"read":{"text":""}}`)}) + case "agent.prompt": + _ = conn.Close() // simulate post-write response loss + } + _ = conn.Close() + } + }() + c := &Client{Path: ln.Addr().String(), agents: map[string]string{"w:p": "oc-task"}, dial: func() (net.Conn, error) { return net.Dial("tcp", ln.Addr().String()) }} + if err := c.Prompt(context.Background(), "w:p", "do work", time.Millisecond); err == nil { + t.Fatal("expected uncertain delivery error") + } + for i, want := range []string{"pane.get", "pane.read", "agent.prompt"} { + select { + case req := <-requests: + if req.Method != want { + t.Fatalf("request %d = %s, want %s", i, req.Method, want) + } + if req.Method == "agent.prompt" { + p, _ := json.Marshal(req.Params) + var got struct { + Target string `json:"target"` + } + _ = json.Unmarshal(p, &got) + if got.Target != "oc-task" { + t.Fatalf("prompt target = %q, want unique agent name", got.Target) + } + } + case <-time.After(time.Second): + t.Fatal("missing request") + } + } +} + +func TestClaudeWorkspaceTrustPrompt(t *testing.T) { + if !claudeWorkspaceTrustPrompt("Accessing workspace:\n❯ 1. Yes, I trust this folder") { + t.Fatal("exact Claude trust prompt was not recognized") + } + if claudeWorkspaceTrustPrompt("WARNING: Claude Code running in Bypass Permissions mode\n❯ 2. Yes, I accept") { + t.Fatal("bypass-permissions disclaimer must never be accepted automatically") + } +} + +func TestPaneAgentAttachedParsesPaneGetEnvelope(t *testing.T) { + 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 + } + if req.Method != "pane.get" { + t.Errorf("method = %q, want pane.get", req.Method) + return + } + _ = json.NewEncoder(conn).Encode(Response{ID: req.ID, Result: json.RawMessage(`{"type":"pane_info","pane":{"agent":"opencode","agent_status":"idle"}}`)}) + }() + + c := &Client{Path: ln.Addr().String(), dial: func() (net.Conn, error) { + return net.Dial("tcp", ln.Addr().String()) + }} + attached, err := c.paneAgentAttached(context.Background(), "w1:p1") + if err != nil { + t.Fatal(err) + } + if !attached { + t.Fatal("pane.get envelope with an idle agent was not recognized as attached") + } +} diff --git a/internal/orchestrator/launch_prompt_test.go b/internal/orchestrator/launch_prompt_test.go new file mode 100644 index 0000000..b9fbcb0 --- /dev/null +++ b/internal/orchestrator/launch_prompt_test.go @@ -0,0 +1,20 @@ +package orchestrator + +import ( + "orchestra/internal/domain" + "strings" + "testing" +) + +func TestTaskLaunchPromptIncludesRemoteTaskInstructions(t *testing.T) { + prompt := taskLaunchPrompt(domain.Task{ + ID: "task-1", + Title: "Create marker", + Description: "Create E2E_RESULT.md containing ok.", + }) + for _, want := range []string{"task-1", "Create marker", "Create E2E_RESULT.md containing ok."} { + if !strings.Contains(prompt, want) { + t.Fatalf("launch prompt missing %q: %s", want, prompt) + } + } +} diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index 07020a3..0e7c800 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -238,7 +238,7 @@ func (c *Coordinator) requestReasonedHandoff(ctx context.Context, taskID string, if session.HandoffRequested { return } - if _, statErr := os.Stat(filepath.Join(session.Worktree, herdr.HandoffFile)); statErr == nil { + if _, statErr := os.Stat(filepath.Join(session.Worktree, herdr.HandoffReportFile)); statErr == nil { return } requester, ok := a.(herdr.ReasonedHandoffRequester) @@ -417,7 +417,7 @@ func (c *Coordinator) Reconcile(ctx context.Context) error { c.mu.Lock() for taskID, session := range c.sessions { t, ok := c.Store.Task(taskID) - if ok && t.State == domain.StateLeased { + if ok && (t.State == domain.StateLeased || t.State == domain.StateBlocked) { continue } if a, err := c.adapterFor(taskID, session); err == nil { @@ -592,6 +592,7 @@ func (c *Coordinator) expire(ctx context.Context) ([]domain.Event, error) { } return events, nil } + // handoffReason reads HandoffFile from the worktree, if present, and returns // its meta.reason ("threshold|milestone|thrash|manual" per §6.1). An unread­ // able or invalid file returns "" — callers treat that as "no signal yet", @@ -648,7 +649,7 @@ func (c *Coordinator) rotate(ctx context.Context, hard float64) { // Soft threshold (§5.3): request a handoff early, advisory // only — no release, no turn-boundary requirement. if requester, ok := a.(herdr.HandoffRequester); ok { - if _, statErr := os.Stat(filepath.Join(session.Worktree, herdr.HandoffFile)); statErr != nil && !session.HandoffRequested { + if _, statErr := os.Stat(filepath.Join(session.Worktree, herdr.HandoffReportFile)); statErr != nil && !session.HandoffRequested { if reqErr := requester.RequestHandoff(ctx, session); reqErr == nil { session.HandoffRequested = true c.mu.Lock() @@ -683,7 +684,7 @@ func (c *Coordinator) rotate(ctx context.Context, hard float64) { } } if requester, ok := a.(herdr.HandoffRequester); ok { - if _, statErr := os.Stat(filepath.Join(session.Worktree, herdr.HandoffFile)); statErr != nil { + if _, statErr := os.Stat(filepath.Join(session.Worktree, herdr.HandoffReportFile)); statErr != nil { if !session.HandoffRequested { if reqErr := requester.RequestHandoff(ctx, session); reqErr == nil { session.HandoffRequested = true @@ -779,7 +780,7 @@ func (c *Coordinator) TurnDecision(ctx context.Context, taskID string) (string, // preparing a handoff well before Hard forces one, but don't block // the turn on a boundary check — the agent is free to keep working. if requester, ok := a.(herdr.HandoffRequester); ok { - if _, statErr := os.Stat(filepath.Join(session.Worktree, herdr.HandoffFile)); statErr != nil { + if _, statErr := os.Stat(filepath.Join(session.Worktree, herdr.HandoffReportFile)); statErr != nil { if !session.HandoffRequested { if reqErr := requester.RequestHandoff(ctx, session); reqErr == nil { session.HandoffRequested = true @@ -806,7 +807,7 @@ func (c *Coordinator) TurnDecision(ctx context.Context, taskID string) (string, c.recordTurnBoundaryDegraded() } if requester, ok := a.(herdr.HandoffRequester); ok { - if _, statErr := os.Stat(filepath.Join(session.Worktree, herdr.HandoffFile)); statErr != nil { + if _, statErr := os.Stat(filepath.Join(session.Worktree, herdr.HandoffReportFile)); statErr != nil { if !session.HandoffRequested { if reqErr := requester.RequestHandoff(ctx, session); reqErr == nil { session.HandoffRequested = true @@ -895,8 +896,23 @@ func (c *Coordinator) Start(ctx context.Context, e domain.Event) error { // machine (WorktreeCreator path) is the same cross-host gap named in // AUDIT.md's federation-fork section — not solved here. taskFileSHA, _ := continuity.TaskFileHash(w) - s, err := a.Lease(ctx, t.ID, w) + prompt := taskLaunchPrompt(t) + var s herdr.Session + if promptLeaser, ok := a.(herdr.PromptLeaser); ok { + s, err = promptLeaser.LeasePrompt(ctx, t.ID, w, prompt) + } else { + s, err = a.Lease(ctx, t.ID, w) + } if err != nil { + // A UI-changing prompt can time out after herdr accepted it. Keep the + // live pane mapped before recording TaskBlocked so a later completion + // can reconcile the lifecycle instead of becoming an orphan (B15). + if s.PaneID != "" { + s.HerdrID = p.HarnessID + s.TaskFileSHA = taskFileSHA + s.ConventionsHash, _ = continuity.ConventionsHash(w) + _ = c.rememberSession(t.ID, s) + } return c.block(t, "lease: "+err.Error()) } if p.HandoffRef != "" { @@ -926,13 +942,7 @@ func (c *Coordinator) Start(ctx context.Context, e domain.Event) error { // state this session starts trusting; checkConventions notices drift // from here, not from whatever the agent's own cached view is (§6.3). s.ConventionsHash, _ = continuity.ConventionsHash(w) - c.mu.Lock() - if c.sessions == nil { - c.sessions = map[string]herdr.Session{} - } - c.sessions[t.ID] = s - err = c.saveSessionsLocked() - c.mu.Unlock() + err = c.rememberSession(t.ID, s) c.healthMu.Lock() if c.health.Sessions == nil { c.health.Sessions = map[string]SessionHealth{} @@ -942,6 +952,31 @@ func (c *Coordinator) Start(ctx context.Context, e domain.Event) error { return err } +func taskLaunchPrompt(t domain.Task) string { + var b strings.Builder + fmt.Fprintf(&b, "Begin Orchestra task %s.\n", t.ID) + if t.Title != "" { + fmt.Fprintf(&b, "Title: %s\n", t.Title) + } + if t.Description != "" { + fmt.Fprintf(&b, "Instructions:\n%s\n", t.Description) + } else { + b.WriteString("Inspect the repository, understand the task context, and proceed with the requested work.\n") + } + b.WriteString("This is the authoritative task instruction. Work only within this task's worktree. Do not edit TASK.md if it exists.") + return b.String() +} + +func (c *Coordinator) rememberSession(taskID string, s herdr.Session) error { + c.mu.Lock() + defer c.mu.Unlock() + if c.sessions == nil { + c.sessions = map[string]herdr.Session{} + } + c.sessions[taskID] = s + return c.saveSessionsLocked() +} + func (c *Coordinator) block(t domain.Task, reason string) error { b, _ := json.Marshal(map[string]string{"blocker": reason}) return c.Store.Append(domain.Event{ID: domain.NewID(), Type: "TaskBlocked", TaskID: t.ID, Version: t.Version + 1, Payload: b, Surface: string(authz.System)}) diff --git a/internal/orchestrator/rotation_test.go b/internal/orchestrator/rotation_test.go index a1306dd..8f635e0 100644 --- a/internal/orchestrator/rotation_test.go +++ b/internal/orchestrator/rotation_test.go @@ -815,7 +815,7 @@ func TestRotationRequestsHandoffBeforeReleasing(t *testing.T) { t.Fatalf("task rotated without a handoff file: state=%v ok=%v", got.State, ok) } - if err := os.WriteFile(repo+"/"+herdr.HandoffFile, []byte("{}"), 0644); err != nil { + if err := os.WriteFile(repo+"/"+herdr.HandoffReportFile, []byte("handoff evidence"), 0644); err != nil { t.Fatal(err) }