From 62bb17e05d3f3692e9b92b7e035061e845f7f605 Mon Sep 17 00:00:00 2001 From: kami Date: Mon, 27 Jul 2026 22:06:06 +0400 Subject: [PATCH] fix(continuity): wire scratch-commit-before-release and rewrite bootstrap prompt (Phase 4 items 3, 5, 6) Release now re-verifies every handoff Anchor.Dirty file hash (previously unchecked after the top-level anchor SHA compare), snapshots dirty state onto a per-task scratch branch before uploading, and rewrites the anchor to the new commit so successor pickup collapses to a single HEAD compare. ScratchCommit made idempotent for repeated rotations of the same task. Bootstrap's prompt now points the agent at the scratch-branch commit history instead of vague "read the handoff" prose, and does not claim a GET /v1/artifacts/ endpoint that doesn't exist. MarkdownChanges had zero callers and zero tests; deleted per AUDIT.md's explicit deletion option rather than half-wiring an undesigned feature. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01W1rkJ2hBMybnJctPbcy4tT --- AUDIT.md | 108 +++++++++++++- cmd/orchestra/main.go | 6 +- internal/continuity/continuity.go | 43 ++---- internal/herdr/adapter.go | 126 +++++++++++++--- internal/herdr/adapter_test.go | 235 ++++++++++++++++++++++++++++++ progress.md | 41 ++++++ 6 files changed, 504 insertions(+), 55 deletions(-) create mode 100644 internal/herdr/adapter_test.go diff --git a/AUDIT.md b/AUDIT.md index 8f16613..1e73e30 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -573,6 +573,56 @@ just a method-name swap): — killing or releasing a real running agent from an audit session without the user present is exactly the kind of action that warrants asking first. +## B5 — closed, 2026-07-27 (later same day) + +`CLIAdapter.Release` now does something real instead of refusing. Design +mirrors the `.orchestra-report.md` marker convention B3 already established +for completion, since the same problem applies to handoffs: the plane must +never invent a handoff, only validate and forward the one the agent wrote +(§6.1). Concretely: + +1. The agent is expected to write `.orchestra-handoff.json` + (`herdr.HandoffFile`) at the worktree root before its stop hook lets + rotation proceed — a §6.1 handoff schema, not free prose. +2. `Release` reads that file, decodes it with `continuity.Decode` (schema + + required-field validation, same as pickup), and cross-checks + `Anchor.GitSHA` against `herdr.HeadSHA(session.Worktree)` — the anchor is + re-verified against the real checkout, not trusted from the agent's + self-report, closing the same class of gap as B3's receipt-from-transcript + choice. +3. Only if both checks pass does it upload the handoff via `continuity.Save` + (`CLIAdapter.CAS`, wired to the same `*store.Store` used everywhere else) + and return the resulting ref — this is what `Coordinator.rotate` puts in + `TaskReleased.handoff_ref`. +4. Only *then* does it call the real `pane.release_agent({pane_id, source: + "herdr:"+harness, agent: harness})` to drop herdr's claim — sequenced last + so a herdr-side failure can't strand an already-uploaded handoff with no + way to retry the release call (retrying `Release` re-reads the same file + and is idempotent). + +A missing or invalid handoff file, an anchor mismatch, or a `pane.release_agent` +error are all refused (non-nil error, no event emitted) — `Coordinator.rotate` +already treats an errored `Release` as "leave the lease intact, retry next +tick," so this gives the agent room to finish writing the handoff rather than +stranding the task. + +`herdr.Claude`/`Codex`/`OpenCode` constructors now take a `continuity.CAS` +parameter; `cmd/orchestra/main.go` passes the existing `*store.Store` (which +already implements `PutArtifact`/`Artifact`). + +New tests in `internal/herdr/adapter_test.go` drive `Release` against a real +git worktree and a fake in-process herdr TCP listener (`fakeHerdr`) responding +to `pane.release_agent`: upload-and-release on a valid handoff, refusal with +no handoff file, refusal on anchor mismatch, refusal with no CAS configured. + +**Still not done** (unchanged, separate from B5 itself): nothing yet makes +the *agent* actually write `.orchestra-handoff.json` — that's Phase 4 item 2's +other half (a stop-hook-side convention, analogous to +`.orchestra-report.md`/`deploy/hooks/orchestra-stop.sh` for completion) and +Phase 4 items 3/5/6 (`ScratchCommit` before release, the §6.2 bootstrap-prompt +rewrite, `MarkdownChanges` wiring). `go build ./...`, `go vet ./...`, and +`go test ./...` all still pass. + ## B6 — partial fix, 2026-07-27 (Phase 4 items 1 and 4) Two of Phase 4's six items landed; the rest are unchanged (still open, listed @@ -619,9 +669,55 @@ cooperation): handoff and a stop-hook path uploading it via `POST /v1/artifacts` before `Coordinator.rotate` calls `Adapter.Release`. `Release` still just refuses (see B5 above) — there is nothing yet to validate-and-mint a ref from. -- Item 3: `ScratchCommit` before release — not wired into `rotate` at all. -- Item 5: `CLIAdapter.Bootstrap`'s prompt is still ad hoc prose, not the - §6.2 ~200-token procedure (read handoff → validate-handoff → re-read - TASK.md → proceed). -- Item 6: `MarkdownChanges` (§6.3 adjacent-task notice) still uncalled from - anything but its own test. + +## Phase 4 items 3, 5, 6 — landed 2026-07-27 + +1. **`ScratchCommit` wired into `Release`, not into `rotate`.** Rather than + calling it from `Coordinator.rotate` (which only has a `herdr.Session`, + not the handoff), `CLIAdapter.Release` now runs it itself, after + validating the agent-authored handoff's `Anchor.GitSHA` against the + worktree's real HEAD and re-verifying every `Anchor.Dirty` file's hash + still matches what the agent recorded (previously untested — a file + edited *after* the handoff was written but before release would have + silently sailed through). If the handoff has dirty entries, `Release` + commits them atomically onto `orchestra/scratch/` via + `continuity.ScratchCommit`, then **rewrites the handoff's anchor** to the + new scratch commit SHA with `Dirty` cleared, before uploading to CAS — + this is what "collapses §6.2 step 3 to one sha compare" means in + practice: the successor's `ValidatePickup` now only needs + `git rev-parse HEAD == handoff.anchor.git_sha`, no per-file rehashing, + because everything was committed before the ref was minted. + `ScratchCommit` itself was changed to be idempotent — reuse an existing + scratch branch (`git switch` before falling back to `git switch -c`) and + skip the commit if there's nothing to snapshot — since a task can rotate, + and therefore hit this path, more than once. + Covered by `TestReleaseScratchCommitsDirtyFilesBeforeUpload` (asserts the + anchor advances to the new commit, dirty is cleared, and the worktree + ends up on the scratch branch) and `TestReleaseRefusesOnStaleDirtyFile` + (internal/herdr/adapter_test.go). +2. **Item 5 — Bootstrap prompt rewritten.** `CLIAdapter.Bootstrap` no longer + sends the one-line "read handoff, validate anchor, continue" prose. It + now tells the agent the plane has *already* validated anchor/TASK.md + (true, per B6's `ValidatePickup` gate in `Coordinator.Start` — no need to + ask the agent to redundantly re-verify trust), and points it at + `git log --stat -5` / `git branch --show-current` in the worktree as the + actual source of "what the prior agent did and what's left," since that's + now a real, inspectable scratch-branch commit rather than an opaque ref. + Deliberately does **not** claim a `GET /v1/artifacts/` fetch path — + no such HTTP route exists (`/v1/artifacts` is POST-only, upload only, + confirmed by reading `cmd/orchestra/main.go`); an earlier draft of this + prompt invented that endpoint and was corrected before landing, which is + exactly the class of bug this audit exists to catch. +3. **Item 6 — `MarkdownChanges` deleted, not wired.** Confirmed zero + callers anywhere (including its own tests — there were none, despite + being listed as "believed accurate" in a prior progress.md snapshot). + Wiring it for real needs a design for what "adjacent task" means and + where the notice surfaces (brief? a new event type?), which is a real + feature, not a wiring fix — AUDIT.md explicitly allows "delete it and + record the deviation" as the alternative to half-implementing that. Taking + that option rather than bolting on an undesigned notification path. + +**Still open from Phase 4**: item 2 (handoff production / stop-hook write of +`.orchestra-handoff.json` for non-Claude harnesses is untouched; Claude's +own stop-hook convention exists per B3/B5 but nothing yet drives Codex/ +opencode to write one). diff --git a/cmd/orchestra/main.go b/cmd/orchestra/main.go index 0d6e001..a84f887 100644 --- a/cmd/orchestra/main.go +++ b/cmd/orchestra/main.go @@ -95,11 +95,11 @@ func main() { } switch h.Harness { case "claude": - adapters[h.ID] = herdr.Claude(client, 200000) + adapters[h.ID] = herdr.Claude(client, 200000, s) case "opencode": - adapters[h.ID] = herdr.OpenCode(client, 200000) + adapters[h.ID] = herdr.OpenCode(client, 200000, s) case "codex", "": - adapters[h.ID] = herdr.Codex(client, 200000) + adapters[h.ID] = herdr.Codex(client, 200000, s) default: log.Printf("herdr %s has unsupported harness %q", h.ID, h.Harness) } diff --git a/internal/continuity/continuity.go b/internal/continuity/continuity.go index 71134de..d995796 100644 --- a/internal/continuity/continuity.go +++ b/internal/continuity/continuity.go @@ -200,28 +200,6 @@ func Load(ref string, cas CAS) (Handoff, error) { return Decode(b) } -type Notice struct { - Path string - SHA256 string - At time.Time -} - -func MarkdownChanges(root string, paths []string) ([]Notice, error) { - out := []Notice{} - for _, p := range paths { - if !strings.HasSuffix(strings.ToLower(p), ".md") { - continue - } - b, e := os.ReadFile(filepath.Join(root, p)) - if e != nil { - return nil, e - } - s := sha256.Sum256(b) - out = append(out, Notice{p, hex.EncodeToString(s[:]), time.Now().UTC()}) - } - return out, nil -} - // ScratchCommit records WIP atomically on a dedicated branch before rotation. func ScratchCommit(root, branch, message string) error { if branch == "" || strings.ContainsAny(branch, " \t\n") { @@ -237,15 +215,24 @@ func ScratchCommit(root, branch, message string) error { if len(status) != 0 { return errors.New("TASK.md is immutable") } - if strings.TrimSpace(message) == "" { - return errors.New("scratch commit message required") - } - for _, args := range [][]string{{"switch", "-c", branch}, {"add", "-A"}, {"commit", "-m", message}} { - if err := exec.Command("git", append([]string{"-C", root}, args...)...).Run(); err != nil { + // Reuse the branch across rotations of the same task rather than failing + // on "branch already exists" — a task can rotate more than once. + if err := exec.Command("git", "-C", root, "switch", branch).Run(); err != nil { + if err := exec.Command("git", "-C", root, "switch", "-c", branch).Run(); err != nil { return err } } - return nil + if err := exec.Command("git", "-C", root, "add", "-A").Run(); err != nil { + return err + } + full, err := exec.Command("git", "-C", root, "status", "--porcelain").Output() + if err != nil { + return err + } + if len(full) == 0 { + return nil // nothing to snapshot; branch already reflects the worktree + } + return exec.Command("git", "-C", root, "commit", "-m", message).Run() } func ScratchPush(root, branch, remote string) error { diff --git a/internal/herdr/adapter.go b/internal/herdr/adapter.go index 0e6b4ec..1d4df27 100644 --- a/internal/herdr/adapter.go +++ b/internal/herdr/adapter.go @@ -2,13 +2,29 @@ package herdr import ( "context" + "crypto/sha256" + "encoding/hex" "encoding/json" "fmt" + "orchestra/internal/continuity" + "os" "path/filepath" "strings" "time" ) +// sha256sum returns the sha256 of a file, or nil if it can't be read — the +// caller compares against a known-good hex digest, so a nil/short mismatch +// naturally fails that comparison rather than needing its own error path. +func sha256sum(path string) []byte { + b, err := os.ReadFile(path) + if err != nil { + return nil + } + sum := sha256.Sum256(b) + return sum[:] +} + type Adapter interface { Lease(context.Context, string, string) (Session, error) Bootstrap(context.Context, Session, string) error @@ -46,8 +62,18 @@ type CLIAdapter struct { Harness string Window int64 Usage func(string) (Usage, error) + // CAS is where the agent-authored §6.1 handoff artifact is uploaded on + // release. Nil disables Release (adapters built without one refuse + // loudly rather than skip validation). + CAS continuity.CAS } +// HandoffFile is the convention the agent writes its §6.1 handoff to before +// stopping, mirroring the .orchestra-report.md convention B3 established for +// completion: the plane never invents a handoff, it only validates and +// uploads the one the agent wrote (herdr does not write handoffs, §6.1). +const HandoffFile = ".orchestra-handoff.json" + 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 { @@ -72,21 +98,85 @@ func (a CLIAdapter) Lease(ctx context.Context, task, worktree string) (Session, } return s, nil } +// 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 +// agent to re-derive trust in the handoff — it orients the agent inside a +// checkout the plane has already certified, and tells it what NOT to touch. +const bootstrapPrompt = `You are picking up an in-progress Orchestra task (handoff ref %s). +This worktree's anchor and TASK.md have already been verified by the plane before you were started — you do not need to re-derive trust in them. +1. Re-read TASK.md at the worktree root. It is immutable; never edit it. +2. Run 'git log --stat -5' and 'git branch --show-current' in this worktree — the prior agent's uncommitted work was snapshotted onto a scratch branch with a descriptive commit message before rotation; that commit is the record of what it did and what's left. +3. Do not repeat work already recorded as done or as a dead end in that commit history. +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("Read handoff %s, validate the anchor and TASK.md, then continue.", ref), time.Minute) + return a.Client.Prompt(ctx, s.PaneID, fmt.Sprintf(bootstrapPrompt, ref), time.Minute) } -// Release previously called the invented "pane.release" method expecting a -// handoff_ref back. Neither exists in the real protocol (confirmed against a -// live herdr instance, AUDIT.md Phase 0): the real method is -// pane.release_agent(pane_id, source, agent), which only releases herdr's -// claim on the agent session — it cannot return a handoff_ref, because herdr -// does not write handoffs, the agent does (§6.1). Wiring this correctly needs -// the Phase 4 handoff-production path (agent writes handoff, stop hook -// uploads it to CAS, plane validates and mints the ref) before Release has -// anything real to return. Refusing loudly until then rather than calling a -// method that doesn't exist. +// 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 +// via the real pane.release_agent(pane_id, source, agent) method (confirmed +// live against herdr, AUDIT.md Phase 0 — the invented "pane.release" never +// existed and could never have returned a handoff_ref regardless, since +// herdr does not write handoffs, the agent does). A missing or invalid +// handoff is refused rather than guessed at: the caller (Coordinator.rotate) +// leaves the lease intact and retries next tick, giving the agent time to +// finish writing it. func (a CLIAdapter) Release(ctx context.Context, s Session) (string, error) { - return "", fmt.Errorf("adapter: Release not implemented — pane.release is not a real herdr method and handoff production (AUDIT.md Phase 4) is not wired yet") + if a.CAS == nil { + return "", fmt.Errorf("adapter: CAS store required to upload handoff") + } + path := filepath.Join(s.Worktree, HandoffFile) + b, err := os.ReadFile(path) + if err != nil { + return "", fmt.Errorf("adapter: handoff not written yet (%s): %w", path, err) + } + h, err := continuity.Decode(b) + if err != nil { + return "", fmt.Errorf("adapter: invalid handoff: %w", 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) + } + 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) + } + } + // Atomically commit whatever the handoff described as dirty onto a + // per-task scratch branch (§6.2 step 3) *before* uploading, so the + // successor's pickup validation collapses to a single HEAD compare + // instead of re-hashing every dirty file individually. + if len(h.Anchor.Dirty) > 0 { + branch := "orchestra/scratch/" + h.Meta.ID + if err := continuity.ScratchCommit(s.Worktree, branch, "orchestra: pre-release WIP snapshot ("+h.Meta.ID+")"); err != nil { + return "", fmt.Errorf("adapter: scratch commit: %w", err) + } + newSHA, err := HeadSHA(s.Worktree) + if err != nil { + return "", fmt.Errorf("adapter: read scratch HEAD: %w", err) + } + h.Anchor.GitSHA = newSHA + h.Anchor.Branch = branch + h.Anchor.Dirty = nil + } + ref, err := continuity.Save(h, a.CAS) + if err != nil { + return "", fmt.Errorf("adapter: upload handoff: %w", err) + } + if err := a.Client.Call(ctx, "pane.release_agent", map[string]any{ + "pane_id": s.PaneID, + "source": "herdr:" + a.Harness, + "agent": a.Harness, + }, nil); err != nil { + return "", fmt.Errorf("adapter: pane.release_agent: %w", err) + } + return ref, nil } 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) @@ -224,12 +314,12 @@ func (a CLIAdapter) resolveSessionFile(s Session) (string, error) { } } -var Claude = func(c *Client, w int64) CLIAdapter { - return CLIAdapter{Client: c, Harness: "claude", Window: w, Usage: ClaudeUsage} +var Claude = func(c *Client, w int64, cas continuity.CAS) CLIAdapter { + return CLIAdapter{Client: c, Harness: "claude", Window: w, Usage: ClaudeUsage, CAS: cas} } -var Codex = func(c *Client, w int64) CLIAdapter { - return CLIAdapter{Client: c, Harness: "codex", Window: w, Usage: CodexUsage} +var Codex = func(c *Client, w int64, cas continuity.CAS) CLIAdapter { + return CLIAdapter{Client: c, Harness: "codex", Window: w, Usage: CodexUsage, CAS: cas} } -var OpenCode = func(c *Client, w int64) CLIAdapter { - return CLIAdapter{Client: c, Harness: "opencode", Window: w, Usage: OpenCodeUsage} +var OpenCode = func(c *Client, w int64, cas continuity.CAS) CLIAdapter { + return CLIAdapter{Client: c, Harness: "opencode", Window: w, Usage: OpenCodeUsage, CAS: cas} } diff --git a/internal/herdr/adapter_test.go b/internal/herdr/adapter_test.go new file mode 100644 index 0000000..a6e32b2 --- /dev/null +++ b/internal/herdr/adapter_test.go @@ -0,0 +1,235 @@ +package herdr + +import ( + "bufio" + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "net" + "orchestra/internal/continuity" + "os" + "os/exec" + "path/filepath" + "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"}, + Goal: "finish the thing", + DoneWhen: []string{"tests pass"}, + Action: "continue", + Command: "go test ./...", + } +} + +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) + } + + b, err := continuity.Encode(validHandoff(head)) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(repo, HandoffFile), b, 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.Fatalf("stored handoff anchor=%s want=%s", got.Anchor.GitSHA, head) + } +} + +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 TestReleaseRefusesOnAnchorMismatch(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") + + b, err := continuity.Encode(validHandoff("deaddeaddeaddeaddeaddeaddeaddeaddeaddead")) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(repo, HandoffFile), 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") + } +} + +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) + } + sum := sha256.Sum256([]byte("in progress")) + h := validHandoff(head) + h.Anchor.Dirty = []continuity.Dirty{{Path: "wip.txt", SHA256: hex.EncodeToString(sum[:])}} + b, err := continuity.Encode(h) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(repo, HandoffFile), b, 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/t1" || 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) { + 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, "wip.txt"), []byte("changed after handoff written"), 0644); err != nil { + t.Fatal(err) + } + stale := sha256.Sum256([]byte("original content")) + h := validHandoff(head) + h.Anchor.Dirty = []continuity.Dirty{{Path: "wip.txt", SHA256: hex.EncodeToString(stale[:])}} + b, err := continuity.Encode(h) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(repo, HandoffFile), 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") + } +} + +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") + } +} diff --git a/progress.md b/progress.md index 1fc52bd..c1d3516 100644 --- a/progress.md +++ b/progress.md @@ -99,6 +99,47 @@ Fixed so far: named caveat: TASK.md hashing is best-effort and untested for the herdr-hosted (`WorktreeCreator`) worktree path. +- **B5 (closed)** — `CLIAdapter.Release` previously just refused (no real + herdr method existed to call and there was nothing to validate against). + Now: reads the agent-authored `.orchestra-handoff.json` from the worktree + root, validates it with `continuity.Decode`, cross-checks its anchor SHA + against the worktree's real `HeadSHA` (never trusts the agent's self-report + outright), uploads it to CAS via `continuity.Save` to mint the + `handoff_ref`, and only then calls the real `pane.release_agent({pane_id, + source, agent})` to drop herdr's claim — sequenced last so a herdr-side + error can't strand an uploaded handoff. Any failure (missing file, invalid + schema, anchor mismatch, herdr error) is a refusal, which `rotate` already + treats as "retry next tick" rather than stranding the task. `herdr.Claude/ + Codex/OpenCode` now take a `continuity.CAS` (main.go passes the existing + `*store.Store`). New tests in `internal/herdr/adapter_test.go` cover all + four paths against a real git worktree and a fake in-process herdr + listener. **Not done:** nothing yet makes the agent actually *write* + `.orchestra-handoff.json` (needs a stop-hook convention analogous to + `.orchestra-report.md`) — that and the rest of Phase 4 (ScratchCommit + before release, §6.2 bootstrap-prompt rewrite, `MarkdownChanges`) remain + open. + +- **Phase 4 items 3, 5, 6** — `CLIAdapter.Release` now re-verifies every + `Anchor.Dirty` file hash (previously only the top-level `Anchor.GitSHA` + was checked; a file edited after the handoff was written but before + release would have gone through unnoticed), then, if there were dirty + entries, snapshots them atomically onto a per-task scratch branch + (`continuity.ScratchCommit`, made idempotent so a task can rotate more + than once) and rewrites the handoff's anchor to that new commit with + `Dirty` cleared before uploading — so the successor's pickup check is a + single HEAD compare, not N file rehashes. `CLIAdapter.Bootstrap`'s prompt + was rewritten to point the agent at `git log`/the scratch branch instead + of a vague "read the handoff" instruction, and deliberately avoids + claiming a `GET /v1/artifacts/` endpoint, since no such route exists + (`/v1/artifacts` is POST-only). `continuity.MarkdownChanges` (§6.3 + adjacent-task notice) had zero callers and zero tests despite being + listed as implemented in an earlier snapshot — deleted rather than + half-wired, per AUDIT.md's explicit "delete and record the deviation" + option. New tests: `TestReleaseScratchCommitsDirtyFilesBeforeUpload`, + `TestReleaseRefusesOnStaleDirtyFile` (internal/herdr/adapter_test.go). + **Not done:** Phase 4 item 2 (handoff production for Codex/opencode — + nothing yet drives those harnesses to write `.orchestra-handoff.json`). + Not yet started: B7 (quota projection has no producer), Codex/opencode completion producers, the turn-decision endpoint, S2–S4, S7–S11. See `AUDIT.md` for the full plan.