3fe3aee5b7
Release already validated and uploaded a §6.1 handoff, but nothing ever told the agent the .orchestra-handoff.json convention existed, so the file it waited on never got written. rotate() now prompts the agent once via a new optional herdr.HandoffRequester capability (CLIAdapter.RequestHandoff) when the file is missing, and defers Release until it appears, mirroring the .orchestra-report.md/B3 ask pattern rather than inventing a handoff. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W1rkJ2hBMybnJctPbcy4tT
745 lines
41 KiB
Markdown
745 lines
41 KiB
Markdown
# Orchestra — spec conformance audit & remediation plan
|
||
|
||
Audited 2026-07-27 against `orchestra-spec (1).md` at commit `325c684`.
|
||
Method: read every non-test file in `internal/` and `cmd/`, traced each spec
|
||
section to its call site, and checked whether the live path (`main.go` →
|
||
router → coordinator → adapter) actually reaches it.
|
||
|
||
---
|
||
|
||
## Verdict
|
||
|
||
The substrate (Layer 1) is genuinely built. Layers 2–4 are **shaped** but not
|
||
**wired**: the packages exist, have tests, and compile — but the code paths
|
||
that would make an unattended run work are either dead, mis-keyed, or built
|
||
against herdr methods that appear to be invented.
|
||
|
||
Concretely: **a task can be created, routed, and leased. It can never
|
||
complete, and it can never rotate.** After 3 lease expiries the router marks
|
||
it `TaskFailed`. That is the whole of "connection works but features are
|
||
underdeveloped."
|
||
|
||
`progress.md` overstates completion in several places (see §Corrections).
|
||
|
||
| Spec layer | State |
|
||
|---|---|
|
||
| L1 substrate (§3, §4) | Built and correct in the main path. Some defects (below), no missing mechanism. |
|
||
| L2 harness (§5) | **Not functional.** Occupancy broken, rotation unreachable, adapter protocol unverified. |
|
||
| L3 continuity (§6) | **Dead code.** Handoff schema, pickup validation, scratch branches, TASK.md — none reached from the live path. |
|
||
| L4 surfaces (§7) | Partial. Brief/standup/delivery exist; quota projection has no producer; authz has a bypass. |
|
||
|
||
---
|
||
|
||
## Blocking defects — in dependency order
|
||
|
||
### B1. Occupancy is measured from the wrong value (§5.2.1)
|
||
|
||
`herdr/adapter.go:191`
|
||
|
||
```go
|
||
u, e := a.Usage(s.PaneID) // ClaudeUsage(path string) wants a transcript file
|
||
```
|
||
|
||
`ClaudeUsage`/`CodexUsage`/`OpenCodeUsage` all take a **filesystem path** to
|
||
session state. They are handed a **herdr pane ID**. Every call returns
|
||
`open <pane-id>: no such file`, so `Coordinator.rotate` hits
|
||
`if err != nil { continue }` on line 430 and never rotates anything.
|
||
|
||
The spec is unusually explicit here — §5.2.1: *"Build this measurement and
|
||
verify it against a live session **before** wiring any trigger — the whole
|
||
rotation system rests on this number."* That step was skipped, and the
|
||
verification gap is exactly where the bug is.
|
||
|
||
Compounding: `CodexActiveUsage` (the discovery function that resolves the
|
||
active rollout via `state_*.sqlite`) and `ClaudeStopHookUsage` and
|
||
`OpenCodeStatus` are written but **called from nowhere** outside tests.
|
||
|
||
### B2. The adapter lookup key is wrong in three of four call sites (§5.3, §5.4)
|
||
|
||
`AdapterFactory.Herdrs` is keyed by **herdr instance id** (`homesrv-claude`).
|
||
`Session.Harness` is set by `CLIAdapter.Lease` to the **harness kind**
|
||
(`claude`). Then:
|
||
|
||
| Call site | Key used | Resolves? |
|
||
|---|---|---|
|
||
| `refreshSessionHealth` (orchestrator.go:221) | `HerdrID`, falls back to lease | yes |
|
||
| `Reconcile` (:291) | `session.Harness` | **no** |
|
||
| `expire` (:376, :397) | `session.Harness` | **no** |
|
||
| `rotate` (:419) | `session.Harness` | **no** |
|
||
|
||
So: orphaned panes are never killed on restart, `pane.exited` fast-path never
|
||
fires, expired leases never kill their pane, and rotation exits before it
|
||
begins. All silently — every one is a bare `continue`.
|
||
|
||
### B3. Nothing emits `TaskCompleted` (§4, §5.2)
|
||
|
||
Grep for producers: there is exactly one, `POST /v1/tasks/{id}/complete`,
|
||
which a human has to call. The spec assigns this to the **stop-hook/wrapper**
|
||
("the plane emits events, not the agent" — Invariant 2). There is no stop
|
||
hook, no wrapper, no completion detection of any kind.
|
||
|
||
Consequence: an agent that finishes its work sits idle until the 30-minute
|
||
lease TTL expires → `TaskReleased` → re-leased → repeat → `TaskFailed` at
|
||
attempt 3. Unattended runtime, the one metric in §0, is currently bounded at
|
||
90 minutes per task and always ends in a false failure.
|
||
|
||
### B4. The router counts rotation as a retry (§5.3, §5.4)
|
||
|
||
`router.go:127` increments `attempts[TaskID]` on **every** `TaskReleased`,
|
||
and `:169` increments again on every lease. `:149` fails the task at
|
||
`MaxAttempts` (3, from `main.go:64`).
|
||
|
||
Rotation *is* `TaskReleased` (§5.3: "rotation = intra-task lease transfer").
|
||
So a task healthy enough to rotate twice is killed by the retry limit. The
|
||
spec's retry policy is for *failures* (§5.4), not lease transfers. These need
|
||
separate counters — `attempts` should only advance on expiry/crash releases,
|
||
never on a release carrying a valid `handoff_ref`.
|
||
|
||
### B5. Herdr protocol methods are unverified and at least partly invented
|
||
|
||
`adapter.go` calls `pane.release`, `pane.kill`, `pane.rotation_signal`,
|
||
`pane.read`, `agent.get`, `agent.start`, `worktree.create/open`,
|
||
`agent.prompt`. The file itself documents that a previous method
|
||
(`pane.status`) *was not a valid protocol method* — so this surface has a
|
||
history of being written against an imagined API.
|
||
|
||
`pane.rotation_signal` is near-certainly not real: herdr is a generic
|
||
multiplexer with no concept of Orchestra rotation. And `pane.release`
|
||
returning a `handoff_ref` is architecturally wrong regardless of whether the
|
||
method exists — **herdr does not write handoffs; the agent does** (§6.1, the
|
||
handoff is a CAS artifact produced under the Face-B stop hook).
|
||
|
||
Nothing here can be settled from source. It needs `herdr api schema --json`
|
||
run against your protocol-17 instance, diffed against every method the
|
||
adapter calls.
|
||
|
||
Related: §5.1 requires `agent.prompt` with **inline `wait`** for bootstrap
|
||
injection, because that's what closes the send-into-a-half-rendered-prompt
|
||
race. `CLIAdapter.Lease` calls `Prompt(..., wait=0)` — no wait. Only
|
||
`Bootstrap` passes one.
|
||
|
||
### B6. Layer 3 is entirely dead code (§6)
|
||
|
||
Never called from anything but tests:
|
||
`continuity.ValidatePickup`, `ScratchCommit`, `ScratchPush`, `ScratchPull`,
|
||
`ScratchSync`, `MarkdownChanges`, `Save`, `Load`, `Encode`, `Decode`.
|
||
|
||
`VerifyTaskFile` *is* called — but only when `GitWorktrees.TaskFileSHA` is
|
||
non-empty, and `main.go:112` constructs the worktrees **without ever setting
|
||
it**. Nothing writes a `TASK.md` into a worktree in the first place.
|
||
|
||
So the entire §6.2 pickup contract — the part the spec calls
|
||
"the make-or-break" and "the defense against the telephone game" — does not
|
||
execute. Rotation, if B1/B2 were fixed, would hand the next agent a
|
||
`handoff_ref` that was never schema-validated, never anchor-checked, and
|
||
never accompanied by an immutable spec.
|
||
|
||
### B7. Quota projection has no producer (§7.2)
|
||
|
||
`QuotaAvailability.sumSince` folds `QuotaReported` events. `AggregateQuota`
|
||
does the same for the brief. **No code appends a `QuotaReported` event.**
|
||
Your config sets `quota_limit_5h: 50` / `quota_limit_weekly: 500` on all six
|
||
herdrs; those limits are compared against a permanent 0. The availability
|
||
filter is a no-op and the brief's `quota_consumed` is always `{}`.
|
||
|
||
### B8. `Surface: system` is an unauthenticated full-control bypass (§7.1)
|
||
|
||
`authz.CapabilityFor` grants `System` → `FullControl`. `authz.HTTP` reads the
|
||
surface straight from the `X-Orchestra-Surface` header, and the token map in
|
||
`main.go:760` has **no entry for `system`** — so `tokens[System]` is `""` and
|
||
the token check is skipped entirely. Any request on the LAN with
|
||
`X-Orchestra-Surface: system` can emit any event on any task.
|
||
|
||
The surface is also absent from `ParseSurface`'s intent: `System` is supposed
|
||
to mean "the plane itself, in-process", but it's reachable over HTTP. It
|
||
should never be accepted from a request header.
|
||
|
||
(Also: unset `ORCHESTRA_*_TOKEN` means that surface is unauthenticated; your
|
||
`.orchestra-config/orchestra.env` sets none of them. Defensible on WireGuard,
|
||
worth a deliberate decision rather than an accident.)
|
||
|
||
---
|
||
|
||
## Secondary defects
|
||
|
||
| # | Where | Issue |
|
||
|---|---|---|
|
||
| S1 | `operations.go:16,25` | `go vet` fails: `Brief.From/To` both serialize as `"from"`; `GitSync.Branch/Head/Status` all as `"branch"`. The brief's git state is unreadable by any client. |
|
||
| S2 | `main.go:238` | Brief's git state is read from `ORCHESTRA_DATA` (the event-log dir), not from the project worktrees. §7.4 wants "what pushed, what's on which branch, what workpc still needs to pull" — per project. |
|
||
| S3 | `operations.BuildBrief` | Counts completions but never surfaces `report_ref`/`receipt`, which §7.4 names as the proofs the brief exists to carry. |
|
||
| S4 | `delivery.Fanout.Run:110` | A single send error `return`s and permanently kills the notification goroutine — silently, since `main.go:729` only logs. One ntfy hiccup at 2am = no notifications for the rest of the night. Cursor is also in-memory, so a restart re-notifies the entire log from seq 0. |
|
||
| S5 | `store.Lease:341`, `ExpireLeases:350` | `Event.ID` is set to the **task id**, so event IDs collide across every lease of a task. `ApplyAdvisory` and any future ID-based lookup are unsound. |
|
||
| S6 | `store.Append:191` | Ingest dedup returns `nil` (success) without appending; `main.go:175` then returns `s.Events(0)[len-1]` — an unrelated event — with `201 Created`. |
|
||
| S7 | `store.apply:154` | `TaskAmended` applies only `title`. §4 lists title, due, description, `inherent_priority`. Amendments to the others are accepted, logged, and silently ignored by the projection. |
|
||
| S8 | §3.1 | No compensation-event mechanism exists. Append-only holds, but the spec's *correction* path ("a compensating event is appended") has no implementation. |
|
||
| S9 | `main.go:482` | Two independent lease-expiry loops (`main.go` 1s ticker and `Coordinator.expire` 30s) race on the same reclaim. Harmless today only because version CAS rejects the loser. |
|
||
| S10 | federation | `federation.Registry.Register` accepts a self-declared `id` + self-chosen `token` from any caller — registration is admission-control-free. |
|
||
| S11 | §5.3 | Thrash detection, the soft ~55% threshold, milestone rotation, and agent-initiated `ROTATE` are all absent. Only the hard threshold exists, and it's unreachable (B1). |
|
||
|
||
---
|
||
|
||
## The architectural fork — decided 2026-07-27
|
||
|
||
There are **two incompatible federation designs** in the tree, and only one is
|
||
deployed.
|
||
|
||
### Design A — "drive the remote socket" (deployed)
|
||
|
||
`clients/herdr-bridge.go` runs on workpc and proxies its local herdr Unix
|
||
socket to `192.168.1.105:9245`. `.orchestra-config/config.jsonc` registers
|
||
`workpc-claude` / `workpc-codex` / `workpc-opencode` against that machine, so
|
||
the **homesrv** coordinator resolves a workpc herdr as a candidate and calls
|
||
`worktree.create`, `agent.start`, `agent.prompt` on it over TCP as if it were
|
||
local. workpc is a dumb pane host; all orchestration logic and all state live
|
||
on homesrv, and live state crosses the machine boundary continuously.
|
||
|
||
### Design B — "workers pull tasks" (`/v1/federation/*`, unused)
|
||
|
||
workpc would run its own Orchestra process that registers itself
|
||
(`POST /v1/federation/workers`), heartbeats, polls the event log with a cursor
|
||
(`GET /v1/federation/events` + `/ack`), claims its own lease
|
||
(`POST .../claim`), runs a **local** coordinator against its **local** herdr
|
||
socket, and reports a handoff carrying an `anchor_sha` it computed from its
|
||
own checkout (`POST .../handoff`). homesrv stays authoritative for the log;
|
||
nothing but git and validated artifacts crosses the wire.
|
||
|
||
This is what the spec describes — §2.1: *"Everything crossing a machine
|
||
boundary is git + a validated artifact — never live state over the wire."*
|
||
**It is fully written on the server side and has zero clients.** There is no
|
||
worker binary in this repo; every one of those endpoints is unreachable in
|
||
the current deployment.
|
||
|
||
### Why keeping both is harmful, not merely untidy
|
||
|
||
Design A breaks a specific correctness property Design B was written to hold.
|
||
|
||
**The anchor is validated on the wrong machine.** `Coordinator.rotate`
|
||
(orchestrator.go:460) calls `herdr.HeadSHA(session.Worktree)`, which shells
|
||
out to `git -C <path> rev-parse HEAD`. The coordinator runs on homesrv;
|
||
`session.Worktree` is a path on **workpc**. Two outcomes, both bad:
|
||
|
||
- the path doesn't exist on homesrv → `HeadSHA` errors → rotation silently
|
||
skips forever (another bare `continue`); or
|
||
- the path *does* exist on homesrv — likely, since every project shares the
|
||
`/var/lib/orchestra/worktrees/<project>/<task>` layout — → it returns
|
||
**homesrv's HEAD for an unrelated checkout**, and the emitted `TaskReleased`
|
||
certifies a commit the agent never worked on.
|
||
|
||
The second is the dangerous one: it passes validation, looks correct, and
|
||
hands the successor an anchor describing another repo's state. That is exactly
|
||
the failure §9 item 8 names — *"worktree/anchor validation when a lease's git
|
||
checkout lives on a different host than the router"* — against the
|
||
degrade-safe default the spec already supplies: *"validate against the local
|
||
checkout wherever the harness runs."* Design B satisfies this by construction;
|
||
Design A cannot satisfy it without moving validation out of the coordinator.
|
||
|
||
The same class of bug applies to per-host quota accounting (§9 item 8) and to
|
||
`cleanupCompleted`, which runs `git worktree remove` on homesrv for a worktree
|
||
that lives on workpc.
|
||
|
||
### Latent defects in the unused half
|
||
|
||
Never surfaced because nothing exercises them:
|
||
|
||
- **Offline detection only runs inside `Snapshot()`** (federation.go:103-118),
|
||
called solely from `GET /v1/federation/workers`. Nothing polls it, so
|
||
`OnOffline` — the hook at `main.go:131` that releases leases held by a
|
||
vanished worker — fires only if a human hits that endpoint. The TTL backstop
|
||
§5.4 relies on does not tick on its own.
|
||
- **The registry is in-memory with no persistence.** Every restart forgets all
|
||
workers and their cursors; a reconnecting worker re-reads the entire log
|
||
from seq 0.
|
||
|
||
### Decision
|
||
|
||
**Keep Design A through Phase 5; commit to Design B in Phase 6.** Phases 1–4
|
||
(occupancy, Face B, rotation, continuity) are single-host concerns, provable
|
||
on homesrv alone with workpc's herdr as just another pane host. Resolving the
|
||
federation question first would block the fixes that actually unblock
|
||
unattended runtime.
|
||
|
||
Two guardrails land **immediately**, so Design A cannot corrupt state in the
|
||
meantime:
|
||
|
||
1. Anchor computation happens where the checkout is. Under the bridge that
|
||
means having herdr run the `rev-parse` in the pane's worktree rather than
|
||
shelling out locally — or, if the protocol schema won't support it,
|
||
**refusing to rotate any lease held by a non-local herdr** until Phase 6. A
|
||
loud refusal beats a false anchor.
|
||
2. Same treatment for `cleanupCompleted`'s `git worktree remove`.
|
||
|
||
Phase 6 then builds the worker binary against the endpoints that already
|
||
exist, adds server-issued tokens (S10), persists the registry, and drives
|
||
offline detection from a ticker rather than a request handler.
|
||
|
||
What is explicitly **not** acceptable is leaving both designs in place
|
||
unmarked. If the worker binary is ever abandoned, delete `/v1/federation/*`
|
||
and record the deviation — as it stands the repo reads as though the
|
||
spec-conformant path is implemented when nothing can reach it.
|
||
|
||
---
|
||
|
||
## Remediation plan
|
||
|
||
Ordered so each phase is verifiable on its own and nothing depends on an
|
||
unproven layer below it — the spec's own build discipline (§8).
|
||
|
||
### Phase 0 — Ground truth (half a day, blocks everything)
|
||
|
||
Nothing below is safe to write until the herdr surface is known.
|
||
|
||
1. On workpc: `herdr api schema --json > herdr-schema.json`, commit it to
|
||
`deploy/`.
|
||
2. Write `internal/herdr/schema_test.go`: for every method string in
|
||
`adapter.go`, assert it exists in the committed schema with the params we
|
||
send. This is the test that would have caught `pane.status` and will catch
|
||
the next one.
|
||
3. Manually drive one pane end to end against real herdr — create worktree,
|
||
start agent, prompt with inline wait, read status, kill — and record the
|
||
actual request/response shapes. Fix `adapter.go` to match.
|
||
4. Delete `pane.rotation_signal` and the `RotationSignal` interface unless the
|
||
schema proves it exists.
|
||
|
||
**Done when:** `schema_test.go` passes against the committed schema, and a
|
||
scripted manual run starts and stops a real Claude Code pane on workpc.
|
||
|
||
### Phase 1 — Occupancy, measured for real (§5.2.1) — fixes B1
|
||
|
||
The spec says build this first and verify against a live session. Do exactly
|
||
that, and nothing else in this phase.
|
||
|
||
1. Add `SessionFile string` to `herdr.Session`. Resolve it at lease time, per
|
||
harness:
|
||
- **claude** — the transcript path. Cleanest source is the Stop hook's
|
||
stdin (`transcript_path`), which you need for Phase 2 anyway; until then
|
||
resolve `~/.claude/projects/<enc-worktree-path>/<session>.jsonl` by
|
||
newest-mtime under the encoded worktree dir.
|
||
- **codex** — `CodexActiveUsage` already does the `state_*.sqlite →
|
||
threads.rollout_path` discovery. Call it. Filter by the rollout whose cwd
|
||
matches the worktree.
|
||
- **opencode** — `OpenCodeStatus` against `:4096` as fast path,
|
||
`~/.local/share/opencode/storage/message/` as backstop, per §5.2.1's
|
||
"the SSE stream is not rock-solid".
|
||
2. Change `CLIAdapter.Occupancy` to use `s.SessionFile`, and make a missing
|
||
session file a **hard error surfaced in `MonitorHealth`**, never a silent
|
||
`continue`.
|
||
3. Keep the §5.2.1 trap explicit: assert in a test that a fixture with a large
|
||
cumulative total but small last-turn usage yields *low* occupancy. This is
|
||
the failure mode the spec singles out.
|
||
4. Add `GET /v1/tasks/{id}/occupancy` returning the raw numerator, the window,
|
||
and the fraction — so you can eyeball it against a live session before
|
||
trusting it to drive rotation.
|
||
|
||
**Done when:** a real Claude Code session at a known context fill reports a
|
||
fraction within a few points of what `/context` says.
|
||
|
||
### Phase 2 — Face B and completion (§5.2, §4) — fixes B3, B5
|
||
|
||
1. **Claude Code Stop hook** (`deploy/hooks/orchestra-stop.sh`, bash, no
|
||
deps — §5.6). Reads hook JSON on stdin, POSTs `transcript_path` + task id
|
||
to a new `POST /v1/harness/turn` endpoint. Exit 2 with stderr when the
|
||
plane says "rotate but no valid handoff yet" — that's the spec's refuse-the-
|
||
turn mechanism, and `ClaudeStopHookUsage` already parses the input shape.
|
||
2. `POST /v1/harness/turn` computes occupancy, evaluates the rotation triggers,
|
||
and returns a decision: `continue` | `prepare_handoff` (soft) |
|
||
`rotate_now` (hard) | `refuse` (over threshold, handoff missing/invalid).
|
||
3. **Completion**: `POST /v1/harness/complete` — the wrapper/hook path that
|
||
uploads the report to CAS and emits `TaskCompleted` with `report_ref` +
|
||
`receipt`. Wire the receipt from Phase 1's occupancy reader, **summed
|
||
across lease intervals** per §4.
|
||
4. Codex/opencode Face B: rollout-tail and SSE `session.status` respectively,
|
||
polled by the coordinator rather than hook-pushed. Same decision endpoint.
|
||
|
||
**Done when:** a task given a trivial goal on a real harness reaches
|
||
`TaskCompleted` with a `report_ref` resolvable from CAS, with no human action.
|
||
|
||
### Phase 3 — Make rotation reachable and correct (§5.3, §5.4) — fixes B2, B4
|
||
|
||
1. Key sessions by `HerdrID` everywhere. Add a single
|
||
`Coordinator.adapterFor(session) (herdr.Adapter, error)` and route all four
|
||
call sites through it. Add a regression test that registers an adapter
|
||
under `homesrv-claude`, leases with `Harness: "claude"`, and asserts
|
||
rotation still fires.
|
||
2. Replace every silent `continue` in `rotate`/`expire` with a recorded reason
|
||
on `MonitorHealth`. The class of bug in B1/B2 is only invisible because of
|
||
those bare continues.
|
||
3. Split the router's counters: `releases` (informational) vs `failures`
|
||
(drives `MaxAttempts`). A `TaskReleased` carrying a valid `handoff_ref`
|
||
must not advance `failures`. Test: a task that rotates 5 times is still
|
||
alive.
|
||
4. Implement the missing triggers (§5.3): soft 55% → `prepare_handoff`;
|
||
milestone; thrash (N failed test runs / same file M times / identical tool
|
||
calls) as a circuit breaker with `reason=thrash` and populated `dead_ends`;
|
||
agent-initiated `ROTATE`.
|
||
5. Make TTL and the retry policy config, not the hardcoded `30*time.Minute` at
|
||
`router.go:165` and `1800` in `main.go` (§5.4: "N, ttl, retry backoff =
|
||
config").
|
||
6. **Land the two Design-A guardrails** (see the federation decision above):
|
||
compute the anchor where the checkout is, or refuse to rotate a lease held
|
||
by a non-local herdr; same for `cleanupCompleted`'s worktree removal. These
|
||
are the fixes that make it safe to defer federation to Phase 6.
|
||
|
||
**Done when:** a long-running task crosses 75% occupancy, rotates at a turn
|
||
boundary, and the successor continues — twice in a row, unattended.
|
||
|
||
### Phase 4 — Wire Layer 3 into the live path (§6) — fixes B6
|
||
|
||
This is where the dead code becomes load-bearing.
|
||
|
||
1. **Write `TASK.md`** into the worktree at creation from the `TaskCreated`
|
||
payload; hash it; store the hash on the task; pass it as `TaskFileSHA` into
|
||
`PerProjectGitWorktrees` (`main.go:112`). Re-inject it on every rotation
|
||
bootstrap — §6.2 is explicit that this is what defeats the telephone game.
|
||
2. **Handoff production**: the rotating agent writes the §6.1 TOML/JSON
|
||
handoff; the stop hook uploads it via `POST /v1/artifacts`; the plane
|
||
`continuity.Decode`s it, rejecting free-form `[knowledge]` (Invariant 4),
|
||
and only then emits `TaskReleased` with the hash. Never accept a
|
||
`handoff_ref` the plane hasn't validated.
|
||
3. **Scratch-branch commit before release** (`ScratchCommit`), so §6.2 step 3
|
||
collapses to one sha compare. `ScratchCommit` already refuses to commit a
|
||
dirty `TASK.md` — good, keep that.
|
||
4. **Pickup**: `ValidatePickup` runs in `Coordinator.Start` before the
|
||
successor's bootstrap prompt. Fail → do not start; emit `TaskBlocked`.
|
||
5. Replace `CLIAdapter.Bootstrap`'s freeform prose with the §6.2 procedure
|
||
(~200 tokens: read handoff, run validate-handoff, re-read TASK.md,
|
||
proceed), and use `agent.prompt`'s inline wait.
|
||
6. Wire `MarkdownChanges` to the §6.3 adjacent-task notice, or delete it and
|
||
record the deferral. Dead code that looks implemented is what produced this
|
||
audit.
|
||
|
||
**Done when:** a rotation whose anchor has drifted is *refused*, visibly, and
|
||
one whose anchor is clean proceeds without the successor re-deriving context.
|
||
|
||
### Phase 5 — Close the surfaces (§7) — fixes B7, B8, S1–S4
|
||
|
||
1. Emit `QuotaReported`. Source: the same per-harness session state as
|
||
Phase 1, on a timer per `harness+window` (§7.2 — "a projection keyed by
|
||
`harness+window`, fed by the same per-harness session state"). Until then,
|
||
your quota limits are decorative.
|
||
2. Reject `X-Orchestra-Surface: system` at the HTTP boundary unconditionally.
|
||
`System` must be constructible in-process only. Then decide explicitly
|
||
whether the remaining surfaces require tokens on your LAN, and set them.
|
||
3. Fix the `go vet` json-tag collisions (S1) — the brief is currently
|
||
unparseable for git state.
|
||
4. Brief: per-project git sync state from the actual worktrees (S2), and
|
||
include `report_ref`/`receipt` in the rollup (S3).
|
||
5. Delivery: never `return` on send error — log, back off, continue. Persist
|
||
the cursor next to the event log (S4). Deliver the morning brief itself.
|
||
|
||
**Done when:** `/v1/brief` for an overnight window shows real quota per
|
||
harness, real per-project git state, and the receipts for every completion.
|
||
|
||
### Phase 6 — Federation, decided (§2.1, §9 item 8) — fixes the fork, S10
|
||
|
||
Only after Phases 1–5 run clean on a single host.
|
||
|
||
1. Anchor validation and `HeadSHA` execute **where the checkout is**. Today
|
||
they run on homesrv against a workpc path.
|
||
2. Either build the worker binary against `/v1/federation/*` (git-only
|
||
transport, per spec) or delete that API and document the bridge as a
|
||
deliberate deviation. Do not keep both.
|
||
3. Add admission control to worker registration (S10): server-issued tokens,
|
||
not self-declared.
|
||
4. Then run the real two-machine overnight batch that §9 item 8 asks for.
|
||
|
||
---
|
||
|
||
## Corrections to `progress.md`
|
||
|
||
Worth fixing, because the next session will otherwise trust it:
|
||
|
||
- "Continuity: strict handoff schema/validation, CAS save/load, pickup
|
||
validation, scratch-branch commit/push/pull helpers" — accurate as a
|
||
description of the *package*, misleading as a description of the *system*.
|
||
None of it is reachable at runtime.
|
||
- "herdr adapters with native occupancy readers" — the readers exist; they are
|
||
called with the wrong argument and always fail.
|
||
- "Router: ... quota availability at conservative 80% threshold" — the
|
||
threshold logic is correct, but no `QuotaReported` event is ever produced,
|
||
so it evaluates against zero.
|
||
- The rotation fix described in "Verified fixed this pass" is real and
|
||
correct — but `rotate` cannot reach that code, because the adapter lookup
|
||
above it (B2) fails first. The test passes because it registers the adapter
|
||
under the harness kind rather than the herdr id, which the production
|
||
config never does.
|
||
- "`go build ./...` and `go test ./...` both pass" — true; `go vet ./...`
|
||
does not.
|
||
|
||
---
|
||
|
||
## Suggested order of attack
|
||
|
||
If you want one thing to do today: **Phase 0**, then **B2** (a ~20-line fix
|
||
that makes three subsystems reachable), then **B1**. Those three turn a system
|
||
that cannot rotate into one that can, and everything else in the plan is
|
||
building on top rather than repairing underneath.
|
||
|
||
---
|
||
|
||
## Phase 0 — done, 2026-07-27
|
||
|
||
B1, B2, B4, B8, S1, S5, S6 were already fixed and landed as of this session
|
||
(confirmed by reading the current code, not just trusting progress.md — see
|
||
`adapterFor` in `internal/orchestrator/orchestrator.go:190` and
|
||
`CLIAdapter.Occupancy` in `internal/herdr/adapter.go:197`).
|
||
|
||
This box (homesrv) turned out to have live TCP reachability to the real herdr
|
||
instance at `192.168.1.105:9245` (workpc) the whole time — the `unavailable:
|
||
connection refused` lines in `journalctl -u orchestra.service` are for
|
||
`homesrv-*` herdrs dialing `192.168.1.104:9245`, which has no local herdr
|
||
running; `workpc-*` herdrs were connecting fine but main.go never logs a
|
||
success, only a failure, so there was no positive signal either way. Also
|
||
found: **a real task is stuck live right now** — workspace `wA`, task
|
||
`06FT6CKD9Y98AZRX6X8K3QXFZG`, opencode agent, pane `wA:p1`, `agent_status:
|
||
"blocked"` — almost certainly stuck because `Release`/rotation could never
|
||
reach it (see below).
|
||
|
||
Ran the actual Phase 0 steps against this live instance (raw JSON-RPC probes
|
||
over TCP, params-omitted/empty-object tricks to read Rust serde's
|
||
missing-field errors — no `herdr` CLI available locally, so `herdr api schema
|
||
--json` itself wasn't run, but the equivalent info was extracted this way).
|
||
Full method list and findings committed to `deploy/herdr-schema.json`.
|
||
|
||
**Confirmed, with a real server response, not just static reading of
|
||
adapter.go:**
|
||
- `pane.release`, `pane.kill`, `pane.rotation_signal` — **none of these exist**
|
||
in the real protocol. Confirms B5's suspicion exactly.
|
||
- Real replacement for `pane.kill` is `pane.close({pane_id})` — same shape,
|
||
drop-in. **Fixed** in `internal/herdr/adapter.go`.
|
||
- Real replacement for `pane.release` is `pane.release_agent({pane_id,
|
||
source, agent})` — structurally different, and per B5's own analysis it
|
||
cannot return a `handoff_ref` regardless (herdr doesn't write handoffs, the
|
||
agent does, §6.1). Wiring this for real needs Phase 4's handoff-production
|
||
path first. `CLIAdapter.Release` now returns a loud error naming exactly
|
||
that instead of calling a method that doesn't exist. **Not a full fix** —
|
||
Phase 4 still owns making Release do something real.
|
||
- `pane.rotation_signal` doesn't exist and never will (herdr has no rotation
|
||
concept) — deleted `RotationSignal` interface, its `CLIAdapter` method, and
|
||
the call site in `Coordinator.rotate`, per this doc's own instruction
|
||
("Delete ... unless the schema proves it exists").
|
||
- `agent.get`, `pane.read`, `agent.prompt`, `worktree.create`, `worktree.open`,
|
||
`agent.start` — all confirmed real, no changes needed there.
|
||
- Protocol version confirmed live: `17`, matching `config.jsonc`'s
|
||
`"protocol": "17"` (returned as a bare JSON number by the server; the
|
||
existing string-fallback parse in `CheckProtocol` happens to handle that
|
||
correctly already).
|
||
|
||
`go build ./...`, `go vet ./...`, `go test ./...` all pass after these
|
||
changes.
|
||
|
||
## B3 — partial fix, 2026-07-27
|
||
|
||
Added `POST /v1/harness/complete` (`cmd/orchestra/main.go`) — the first
|
||
automatic `TaskCompleted` producer. Design: a Claude Code Stop hook
|
||
(`deploy/hooks/orchestra-stop.sh`) runs on every turn boundary but only POSTs
|
||
when the agent has written `.orchestra-report.md` at the worktree root —
|
||
that file is the explicit "I'm done" signal, since Stop fires on every pause
|
||
and treating every stop as completion would be wrong (this is exactly the
|
||
distinction Phase 2 items 1–2, the `continue`/`prepare_handoff`/`rotate_now`/
|
||
`refuse` turn-decision endpoint, are meant to own — that endpoint is still
|
||
unbuilt, so there is currently no plane-side signal telling the agent when to
|
||
rotate vs. finish; the marker-file convention is a stopgap that only covers
|
||
the completion half).
|
||
|
||
The handler builds the `receipt` server-side from `herdr.ClaudeUsage` against
|
||
the transcript path the hook supplies (same local-filesystem assumption as
|
||
`CLIAdapter.Occupancy` — doesn't hold for a session hosted on a herdr that
|
||
isn't local to the machine running orchestra, i.e. the federation-fork
|
||
caveat applies here too) rather than trusting a self-reported number, and
|
||
uploads the report body via `Store.PutArtifact` for `report_ref`. Event is
|
||
appended with `Surface: string(authz.System)` hardcoded in Go — not read from
|
||
a request header — consistent with B8 (system must never be
|
||
header-controlled); gated instead by an optional `ORCHESTRA_HARNESS_TOKEN`
|
||
bearer check.
|
||
|
||
**Not done:** Codex/opencode completion producers (only Claude wired), the
|
||
turn-decision endpoint itself, and no test — `cmd/orchestra/main.go` has zero
|
||
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):
|
||
- `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.
|
||
|
||
## 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
|
||
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.
|
||
|
||
## 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/<handoff-meta-id>` 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/<ref>` 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.
|
||
|
||
**Phase 4 item 2 — closed 2026-07-27.** The remaining gap after B5 was not
|
||
"which harness" (Release was always harness-agnostic) — it was that *nothing,
|
||
for any harness*, ever told the agent the `.orchestra-handoff.json` convention
|
||
existed. `Coordinator.rotate` (internal/orchestrator/orchestrator.go) now
|
||
checks whether the resolved adapter implements a new optional
|
||
`herdr.HandoffRequester` capability; if `HandoffFile` isn't present in the
|
||
worktree yet, it calls `RequestHandoff` once (`herdr.Session.HandoffRequested`
|
||
guards against re-prompting every tick) and skips `Release` for that tick,
|
||
leaving the lease intact — exactly the same "ask, don't invent" shape as B3's
|
||
`.orchestra-report.md` convention. `CLIAdapter.RequestHandoff`
|
||
(internal/herdr/adapter.go) sends a prompt naming the exact §6.1 JSON shape
|
||
(`meta.id`, `anchor.git_sha`/`branch`/`dirty[].{path,sha256}`) and explicitly
|
||
tells the agent not to fabricate the SHA/hashes. Once the file exists, the
|
||
existing Release path (validate → scratch-commit dirty files → upload → CAS
|
||
→ `pane.release_agent`) is unchanged. Covered by
|
||
`TestRotationRequestsHandoffBeforeReleasing`
|
||
(internal/orchestrator/rotation_test.go), which asserts `Release` is never
|
||
called while the file is absent and fires once it's written. `go build`,
|
||
`go vet`, `go test ./...` all pass.
|
||
|
||
Not attempted here (separate, deployment-level question, not a code gap):
|
||
whether Codex's/opencode's own turn-boundary mechanism actually surfaces this
|
||
in-pane prompt to the agent before it exits the way Claude Code's Stop hook
|
||
does — that's Phase 2 item 4 territory (native Face B per harness), not
|
||
Phase 4.
|