orchestra/AUDIT.md

# 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.