fix(orchestrator): wire TASK.md writing and §6.2 pickup validation (B6, partial)
Fixes AUDIT.md's B6: nothing wrote a TASK.md into a worktree, so continuity.ValidatePickup had no caller and no file to check. - continuity.RenderTaskFile/TaskFileHash: render and hash the immutable §6.2 TASK.md from a domain.Task. - GitWorktrees.Create writes and commits TASK.md into every freshly created worktree (must be committed, not dirty, for ScratchCommit's immutability check and for a stable hash). - Coordinator.Start now runs continuity.ValidatePickup (anchor SHA, dirty-file hashes, TASK.md hash) against the real worktree before bootstrapping a successor onto a handoff_ref, and blocks the task instead of bootstrapping on a validation failure. Still open from Phase 4: handoff production (agent writing the real handoff; Release still refuses per B5), ScratchCommit wiring before release, and the §6.2 bootstrap-prompt rewrite — see AUDIT.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W1rkJ2hBMybnJctPbcy4tT
This commit is contained in:
@@ -558,13 +558,70 @@ handler test coverage of any kind (everything is inline in `main()`), so this
|
||||
follows the existing gap rather than introducing an isolated test harness for
|
||||
one handler.
|
||||
|
||||
**Fixed 2026-07-27 (later same day):** `CLIAdapter.Lease`'s bootstrap prompt now
|
||||
passes `time.Minute` instead of `wait=0`, matching `Bootstrap`'s inline-wait
|
||||
pattern — closes the race B5 named (send-into-a-half-rendered-prompt).
|
||||
`internal/herdr/adapter.go`. `go build`/`vet`/`test` all still pass; no
|
||||
existing test asserted the old `wait=0` value.
|
||||
|
||||
**Still open from B5** (not attempted this pass — larger, needs design, not
|
||||
just a method-name swap):
|
||||
- `agent.prompt` inline `wait` on `CLIAdapter.Lease` (still `wait=0`, per B5's
|
||||
note that only `Bootstrap` passes a real wait).
|
||||
- `Release`'s real implementation, which depends on Phase 4 (§6) handoff
|
||||
production existing at all.
|
||||
- The stuck live task (`06FT6CKD9Y98AZRX6X8K3QXFZG`) was deliberately **not**
|
||||
manipulated directly (no `pane.close`/`pane.release_agent` call against it)
|
||||
— 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.
|
||||
|
||||
## 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
|
||||
below).
|
||||
|
||||
1. **TASK.md is now actually written.** `continuity.RenderTaskFile(t)`
|
||||
produces the §6.2 immutable spec content from `domain.Task`;
|
||||
`GitWorktrees.Create` (`internal/orchestrator/orchestrator.go`) writes it
|
||||
into every freshly created worktree and commits it immediately — it must
|
||||
be committed, not left dirty, both so `ScratchCommit`'s "TASK.md is
|
||||
immutable" check (which inspects `git status`) sees it as clean, and so
|
||||
its hash is stable across whatever the agent does afterward. A worktree
|
||||
that already has a `TASK.md` (recreation on restart) is left untouched.
|
||||
New `continuity.TaskFileHash(root)` reads it back and hashes it — this is
|
||||
what makes `w.TaskFileSHA`/`VerifyTaskFile`, previously dead because
|
||||
nothing ever set `TaskFileSHA`, actually reachable.
|
||||
2. **Pickup validation now gates bootstrap.** `Coordinator.Start`
|
||||
(`internal/orchestrator/orchestrator.go`) computes the new worktree's
|
||||
`TaskFileHash`, and — whenever the lease carries a `handoff_ref` (i.e.
|
||||
this is a rotation continuation, not a fresh lease) — loads the handoff
|
||||
from CAS and runs `continuity.ValidatePickup(worktree, handoff,
|
||||
taskFileSHA)` **before** calling `Adapter.Bootstrap`. A validation failure
|
||||
kills the just-started session and emits `TaskBlocked` instead of handing
|
||||
the successor an unverified anchor. This is exactly the gap B6 named:
|
||||
`ValidatePickup` had no caller outside its own tests.
|
||||
`TestStartBlocksOnInvalidPickup` (`rotation_test.go`) drives this against
|
||||
a handoff whose anchor SHA doesn't exist in the repo and asserts the task
|
||||
ends `Blocked`, never bootstrapped.
|
||||
`TestGitWorktreesCommitsTaskFile` (`worktrees_test.go`) asserts the
|
||||
written/committed content matches `RenderTaskFile` and survives
|
||||
recreation.
|
||||
|
||||
Caveat recorded in code: TASK.md hashing is best-effort — if a worktree came
|
||||
from the `WorktreeCreator` (herdr-hosted, potentially remote) path rather
|
||||
than `GitWorktrees`, `TaskFileHash` fails silently and pickup validation runs
|
||||
with an empty `taskFileSHA` (so it still checks anchor SHA and dirty-file
|
||||
hashes, just not TASK.md). No current adapter or test exercises
|
||||
`WorktreeCreator` with a real `handoff_ref`, so this is unverified, not
|
||||
proven safe — same cross-host caveat as the federation-fork section.
|
||||
|
||||
**Still open from B6/Phase 4** (unchanged, larger and needs live-agent
|
||||
cooperation):
|
||||
- Item 2: handoff *production* — the rotating agent writing the §6.1
|
||||
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.
|
||||
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"orchestra/internal/domain"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
@@ -14,6 +15,41 @@ import (
|
||||
"time"
|
||||
)
|
||||
|
||||
// RenderTaskFile produces the immutable §6.2 TASK.md content for a task.
|
||||
// Deterministic in every field that comes from the task itself, so the same
|
||||
// task always hashes to the same content.
|
||||
func RenderTaskFile(t domain.Task) []byte {
|
||||
var b strings.Builder
|
||||
fmt.Fprintf(&b, "# Task %s\n\n", t.ID)
|
||||
if t.Title != "" {
|
||||
fmt.Fprintf(&b, "%s\n\n", t.Title)
|
||||
}
|
||||
fmt.Fprintf(&b, "- Project: %s\n", t.Project)
|
||||
fmt.Fprintf(&b, "- Source: %s/%s\n", t.Source, t.ExternalID)
|
||||
fmt.Fprintf(&b, "- Priority: %d\n", t.InherentPriority)
|
||||
if len(t.Capability) > 0 {
|
||||
fmt.Fprintf(&b, "- Capability: %s\n", strings.Join(t.Capability, ", "))
|
||||
}
|
||||
if t.Due != nil {
|
||||
fmt.Fprintf(&b, "- Due: %s\n", t.Due.UTC().Format(time.RFC3339))
|
||||
}
|
||||
if t.Parent != "" {
|
||||
fmt.Fprintf(&b, "- Parent: %s\n", t.Parent)
|
||||
}
|
||||
b.WriteString("\nThis file is immutable for the lifetime of the task (§6.2) — its hash is\ncarried in every handoff and re-verified on every pickup. Do not edit it.\n")
|
||||
return []byte(b.String())
|
||||
}
|
||||
|
||||
// TaskFileHash returns the sha256 of the TASK.md at the root of a worktree.
|
||||
func TaskFileHash(root string) (string, error) {
|
||||
b, err := os.ReadFile(filepath.Join(root, "TASK.md"))
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
sum := sha256.Sum256(b)
|
||||
return hex.EncodeToString(sum[:]), nil
|
||||
}
|
||||
|
||||
type Dirty struct {
|
||||
Path string `json:"path"`
|
||||
SHA256 string `json:"sha256"`
|
||||
|
||||
@@ -149,6 +149,10 @@ type Session struct {
|
||||
// CLIAdapter.Occupancy), since the file may not exist yet immediately
|
||||
// after lease.
|
||||
SessionFile string `json:"session_file,omitempty"`
|
||||
// TaskFileSHA is the sha256 of the worktree's TASK.md at the time this
|
||||
// session's lease was created — the immutable-spec hash continuity's
|
||||
// pickup validation compares against on the next rotation (§6.2).
|
||||
TaskFileSHA string `json:"task_file_sha,omitempty"`
|
||||
}
|
||||
|
||||
func (c *Client) Prompt(ctx context.Context, pane, text string, wait time.Duration) error {
|
||||
|
||||
@@ -67,6 +67,9 @@ func (w GitWorktrees) Create(ctx context.Context, t domain.Task) (string, error)
|
||||
if out, err := cmd.CombinedOutput(); err != nil {
|
||||
return "", fmt.Errorf("%s: %w", string(out), err)
|
||||
}
|
||||
if err := writeTaskFile(ctx, p, t); err != nil {
|
||||
return "", err
|
||||
}
|
||||
if w.TaskFileSHA != "" {
|
||||
if err := continuity.VerifyTaskFile(p, w.TaskFileSHA); err != nil {
|
||||
return "", err
|
||||
@@ -75,6 +78,27 @@ func (w GitWorktrees) Create(ctx context.Context, t domain.Task) (string, error)
|
||||
return p, nil
|
||||
}
|
||||
|
||||
// writeTaskFile commits the §6.2 immutable TASK.md into a freshly created
|
||||
// worktree. It must be committed, not left dirty, so ScratchCommit's
|
||||
// "TASK.md is immutable" check (which inspects `git status`) sees it as
|
||||
// clean, and so its hash survives independent of any later scratch commits.
|
||||
func writeTaskFile(ctx context.Context, worktree string, t domain.Task) error {
|
||||
path := filepath.Join(worktree, "TASK.md")
|
||||
if _, err := os.Stat(path); err == nil {
|
||||
return nil
|
||||
}
|
||||
if err := os.WriteFile(path, continuity.RenderTaskFile(t), 0644); err != nil {
|
||||
return err
|
||||
}
|
||||
for _, args := range [][]string{{"add", "TASK.md"}, {"commit", "-m", "orchestra: TASK.md"}} {
|
||||
cmd := exec.CommandContext(ctx, "git", append([]string{"-C", worktree}, args...)...)
|
||||
if out, err := cmd.CombinedOutput(); err != nil {
|
||||
return fmt.Errorf("%s: %w", string(out), err)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (w GitWorktrees) Remove(ctx context.Context, _ domain.Task, path string) error {
|
||||
if path == "" {
|
||||
return fmt.Errorf("worktree: path required")
|
||||
@@ -541,17 +565,37 @@ func (c *Coordinator) Start(ctx context.Context, e domain.Event) error {
|
||||
if err != nil {
|
||||
return c.block(t, "worktree: "+err.Error())
|
||||
}
|
||||
// Best-effort: TASK.md only exists for worktrees this process can read
|
||||
// locally (the GitWorktrees path). A herdr-hosted worktree on a remote
|
||||
// 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)
|
||||
if err != nil {
|
||||
return c.block(t, "lease: "+err.Error())
|
||||
}
|
||||
if p.HandoffRef != "" {
|
||||
// §6.2 pickup validation: never bootstrap a successor onto a handoff
|
||||
// whose anchor/dirty-file/TASK.md hashes don't match what's actually
|
||||
// in the worktree. A failure here blocks the task rather than
|
||||
// silently trusting an unvalidated ref (this is the gap AUDIT.md's
|
||||
// B6 named as unreached from the live path).
|
||||
h, err := continuity.Load(p.HandoffRef, c.Store)
|
||||
if err != nil {
|
||||
_ = a.Kill(ctx, s)
|
||||
return c.block(t, "handoff: "+err.Error())
|
||||
}
|
||||
if err := continuity.ValidatePickup(w, h, taskFileSHA); err != nil {
|
||||
_ = a.Kill(ctx, s)
|
||||
return c.block(t, "pickup: "+err.Error())
|
||||
}
|
||||
if err = a.Bootstrap(ctx, s, p.HandoffRef); err != nil {
|
||||
_ = a.Kill(ctx, s)
|
||||
return c.block(t, "bootstrap: "+err.Error())
|
||||
}
|
||||
}
|
||||
s.HerdrID = p.HarnessID
|
||||
s.TaskFileSHA = taskFileSHA
|
||||
c.mu.Lock()
|
||||
if c.sessions == nil {
|
||||
c.sessions = map[string]herdr.Session{}
|
||||
|
||||
@@ -136,6 +136,67 @@ func TestRotationEmitsValidReleaseWithAnchorSHA(t *testing.T) {
|
||||
|
||||
func mustJSON(v any) []byte { b, _ := json.Marshal(v); return b }
|
||||
|
||||
// TestStartBlocksOnInvalidPickup guards AUDIT.md's B6/Phase 4 item 4:
|
||||
// Coordinator.Start must run §6.2 pickup validation against the real
|
||||
// worktree before bootstrapping a successor onto a handoff_ref, and refuse
|
||||
// (TaskBlocked) rather than bootstrap on a mismatched anchor.
|
||||
func TestStartBlocksOnInvalidPickup(t *testing.T) {
|
||||
repo := t.TempDir()
|
||||
run(t, repo, "init")
|
||||
run(t, repo, "config", "user.email", "t@t")
|
||||
run(t, repo, "config", "user.name", "t")
|
||||
run(t, repo, "commit", "--allow-empty", "-m", "init")
|
||||
|
||||
s, err := store.Open(t.TempDir())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := s.Append(domain.Event{ID: domain.NewID(), Type: "TaskCreated", TaskID: "t1", Surface: string(authz.System), Payload: mustJSON(map[string]any{
|
||||
"source": "jsonl", "external_id": "1", "project": "p",
|
||||
})}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
task := s.Tasks()[0]
|
||||
|
||||
// A handoff whose anchor doesn't match anything in this fresh repo.
|
||||
badHandoff := map[string]any{
|
||||
"meta": map[string]any{"id": "h1", "reason": "manual", "rotation_index": 0},
|
||||
"anchor": map[string]any{"git_sha": strings0(40, 'a'), "branch": "orchestra/t1"},
|
||||
"goal": "g", "done_when": []string{"x"}, "action": "prompt", "command": "go test ./...",
|
||||
}
|
||||
ref, err := s.PutArtifact(mustJSON(badHandoff))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
a := &fakeAdapter{occupancy: 0}
|
||||
wt := orchestrator.GitWorktrees{Root: t.TempDir(), Repo: repo}
|
||||
c := &orchestrator.Coordinator{Store: s, Worktrees: wt, Adapters: adapters{a}, StatePath: t.TempDir() + "/sessions.json"}
|
||||
|
||||
leaseEvt, err := s.Lease(task.ID, "h1", time.Minute)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
b, _ := json.Marshal(map[string]string{"harness_id": "h1", "handoff_ref": ref})
|
||||
leaseEvt.Payload = b
|
||||
if err := c.Start(context.Background(), leaseEvt); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
got, ok := s.Task(task.ID)
|
||||
if !ok || got.State != domain.StateBlocked {
|
||||
t.Fatalf("expected TaskBlocked on invalid pickup, got state=%v ok=%v", got.State, ok)
|
||||
}
|
||||
}
|
||||
|
||||
func strings0(n int, c byte) string {
|
||||
b := make([]byte, n)
|
||||
for i := range b {
|
||||
b[i] = c
|
||||
}
|
||||
return string(b)
|
||||
}
|
||||
|
||||
// keyedHarnessAdapter reports Session.Harness as the harness kind ("claude"),
|
||||
// distinct from the herdr instance id ("homesrv-claude") under which it is
|
||||
// registered in AdapterFactory.Herdrs — reproducing production's real key
|
||||
|
||||
@@ -2,6 +2,7 @@ package orchestrator_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"orchestra/internal/continuity"
|
||||
"orchestra/internal/domain"
|
||||
"orchestra/internal/orchestrator"
|
||||
"os"
|
||||
@@ -61,3 +62,62 @@ func TestPerProjectGitWorktreesResolvesByProject(t *testing.T) {
|
||||
t.Fatalf("expected unconfigured project to use default worktree root, got %s", pathDefault)
|
||||
}
|
||||
}
|
||||
|
||||
// TestGitWorktreesCommitsTaskFile guards AUDIT.md's B6: nothing wrote a
|
||||
// TASK.md into a worktree in the first place, so §6.2 pickup validation had
|
||||
// nothing to check. GitWorktrees.Create must now write and commit an
|
||||
// immutable TASK.md whose on-disk hash matches continuity.RenderTaskFile.
|
||||
func TestGitWorktreesCommitsTaskFile(t *testing.T) {
|
||||
base := t.TempDir()
|
||||
repo := filepath.Join(base, "repo")
|
||||
initRepo(t, repo)
|
||||
|
||||
w := orchestrator.GitWorktrees{Root: filepath.Join(base, "wt"), Repo: repo}
|
||||
task := domain.Task{ID: "t1", Project: "p", Source: "jsonl", ExternalID: "1", Title: "do the thing"}
|
||||
|
||||
path, err := w.Create(context.Background(), task)
|
||||
if err != nil {
|
||||
t.Fatalf("create: %v", err)
|
||||
}
|
||||
|
||||
want := continuity.RenderTaskFile(task)
|
||||
got, err := os.ReadFile(filepath.Join(path, "TASK.md"))
|
||||
if err != nil {
|
||||
t.Fatalf("read TASK.md: %v", err)
|
||||
}
|
||||
if string(got) != string(want) {
|
||||
t.Fatalf("TASK.md content mismatch:\ngot: %s\nwant: %s", got, want)
|
||||
}
|
||||
|
||||
status, err := exec.Command("git", "-C", path, "status", "--porcelain", "--", "TASK.md").Output()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(status) != 0 {
|
||||
t.Fatalf("TASK.md not committed, status: %s", status)
|
||||
}
|
||||
|
||||
sha, err := continuity.TaskFileHash(path)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := continuity.VerifyTaskFile(path, sha); err != nil {
|
||||
t.Fatalf("VerifyTaskFile: %v", err)
|
||||
}
|
||||
|
||||
// Re-creating (path already exists) must not touch the committed file.
|
||||
path2, err := w.Create(context.Background(), task)
|
||||
if err != nil {
|
||||
t.Fatalf("recreate: %v", err)
|
||||
}
|
||||
if path2 != path {
|
||||
t.Fatalf("recreate returned different path: %s vs %s", path2, path)
|
||||
}
|
||||
got2, err := os.ReadFile(filepath.Join(path, "TASK.md"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if string(got2) != string(want) {
|
||||
t.Fatalf("TASK.md changed on recreate")
|
||||
}
|
||||
}
|
||||
|
||||
+25
-1
@@ -75,7 +75,31 @@ Fixed so far:
|
||||
`main()`) so this follows the existing (untested) pattern rather than
|
||||
introducing a one-off test harness.
|
||||
|
||||
Not yet started: B6 (Layer 3 wiring), B7 (quota projection has no producer),
|
||||
- **B5 (loose end)** — `CLIAdapter.Lease`'s initial prompt used `wait=0`,
|
||||
skipping the inline wait `Bootstrap` already used; the spec (§5.1) requires
|
||||
inline `wait` on `agent.prompt` for bootstrap injection to avoid sending
|
||||
into a half-rendered prompt. Changed to `time.Minute`, matching `Bootstrap`.
|
||||
Small, contained fix — `Release`'s real implementation (needs Phase 4
|
||||
handoff production) is still outstanding from B5.
|
||||
|
||||
- **B6 (partial — Phase 4 items 1 and 4)** — nothing wrote a `TASK.md` into a
|
||||
worktree, so pickup validation had nothing to check and never ran anyway.
|
||||
Fixed both halves: `GitWorktrees.Create` now writes and commits an
|
||||
immutable `TASK.md` (`continuity.RenderTaskFile`) into every freshly
|
||||
created worktree, and `Coordinator.Start` now runs
|
||||
`continuity.ValidatePickup` (loading the handoff from CAS, checking anchor
|
||||
SHA + dirty-file hashes + TASK.md hash) before bootstrapping a successor
|
||||
onto a `handoff_ref` — a failure kills the session and emits `TaskBlocked`
|
||||
instead of trusting an unvalidated ref. Covered by
|
||||
`TestGitWorktreesCommitsTaskFile` and `TestStartBlocksOnInvalidPickup` in
|
||||
`internal/orchestrator`. **Not done:** handoff *production* (nothing yet
|
||||
writes a real §6.1 handoff — `Release` still refuses per B5), wiring
|
||||
`ScratchCommit` before release, and the §6.2 bootstrap-prompt rewrite. See
|
||||
AUDIT.md's "B6 — partial fix" section for the full breakdown, including a
|
||||
named caveat: TASK.md hashing is best-effort and untested for the
|
||||
herdr-hosted (`WorktreeCreator`) worktree path.
|
||||
|
||||
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.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user