kami
95a96d87a5
The deployment moved from orchestra.service to Docker Compose, but both CLAUDE.md and AUDIT.md still described the systemd unit. A stopped unit plus a stale /usr/local/bin/orchestra reads exactly like a dead service, so document where the live deployment actually is and that deploying now means rebuilding the compose images. Also record what a live check found: the running image predates the B18 auth commit, all six herdrs are unreachable (the sole remaining blocker on live proof for B13-B17), GET /v1/tasks has no authz gate but sits behind a ufw-restricted bind, and ntfy still 403s. Sharpen the herdr-logging note: successes are never logged, so a herdr with no log line is up, not down, and the logs can't confirm one coming back. Probe the ports directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01535A3Y8RtkAi8wYuWhtkEd
1391 lines
80 KiB
Markdown
1391 lines
80 KiB
Markdown
# Orchestra — spec conformance audit & remediation status
|
||
|
||
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. This file is the single
|
||
merged record — a prior split between `AUDIT.md` (audit + plan) and
|
||
`progress.md` (chronological session log) has been flattened here; treat any
|
||
older chronological git history of either file as session notes, not ground
|
||
truth.
|
||
|
||
**Ground truth rule for future sessions:** this repo has a documented history
|
||
of code that *looks* wired but isn't (packages with tests that pass in
|
||
isolation while the live call path silently no-ops). Before trusting any
|
||
"done"/"fixed" claim below, check the actual call site, not just this
|
||
document.
|
||
|
||
---
|
||
|
||
## Current verdict (as of 2026-07-28)
|
||
|
||
The substrate (Layer 1) is solid. Layers 2–4 were originally shaped-but-not-
|
||
wired; most of the blocking defects below are now closed, verified by
|
||
reading the current code (not just by trusting this file) and by targeted
|
||
tests. Three things are not yet proven:
|
||
|
||
1. **A real end-to-end unattended run.** The live attempts of 2026-07-28
|
||
surfaced B12 through B17. All are now closed in code (B17 on the worker
|
||
path only), but **B13 through B17 were each found live and none has been
|
||
re-verified live** — occupancy/rotation/handoff/completion still haven't
|
||
been exercised end-to-end against a session guaranteed to exist. Treat
|
||
"closed" here as "the code no longer contains the defect," not as
|
||
operational proof.
|
||
2. **The operator surface was unauthenticated in the live deployment.**
|
||
Fixed in code (**B18**: mandatory Web token, cookie sessions for the
|
||
browser), but the fix is a breaking config change — the service will not
|
||
start until `.orchestra-config/orchestra.env` sets
|
||
`ORCHESTRA_WEB_TOKEN`. See B18 and the command-channel section.
|
||
3. **Cross-machine (federation) correctness.** Deliberately deferred — see
|
||
"The federation fork" below.
|
||
|
||
`go build ./...`, `go vet ./...`, and `go test ./...` all pass.
|
||
|
||
### Follow-up live verification (2026-07-28)
|
||
|
||
The OpenCode launch path has now been exercised against workpc herdr through
|
||
an isolated `test-e2e` worktree. The verified outcomes are deliberately
|
||
narrow:
|
||
|
||
- Initial prompts use a bounded retry only after an explicit JSON-RPC
|
||
rejection (herdr's short post-start readiness window). Transport timeouts
|
||
and disconnects are never replayed. A pane is inspected first; blocked or
|
||
permission-dialog panes are refused.
|
||
- Herdr accepts task-scoped unique names (`oc-<task-id>`); active sessions
|
||
are visible by that name in `agent.list`. Prompt routing uses that name,
|
||
while pane reads remain pane-scoped.
|
||
- The initial launch message carries the task title and description. This is
|
||
necessary under Design A because homesrv cannot write `TASK.md` inside a
|
||
workpc checkout. A live OpenCode run received the full instruction and,
|
||
after explicit operator approval of two file edits, created the requested
|
||
marker and report without touching other files.
|
||
- The initial launch does **not** wait for the agent to become idle. Waiting
|
||
converted an ordinary long-running first turn into a false `TaskBlocked`;
|
||
a fresh run now remains `TaskLeased` while OpenCode is paused at its normal
|
||
permission boundary.
|
||
|
||
### Deployment state observed live (2026-07-29)
|
||
|
||
Checked against the running system, not the docs:
|
||
|
||
- The deployment **moved from `orchestra.service` to Docker Compose**
|
||
(`/home/kami/docker-apps/orchestra-web-ui`). `orchestra-api` and
|
||
`orchestra-web-ui` are both up; `/healthz` and the UI answer 200. The
|
||
stopped systemd unit is the retired predecessor, not an outage — see the
|
||
deployment section of `CLAUDE.md`.
|
||
- The **running image predates the last two commits** (built 2026-07-28
|
||
21:28; `0b4b52a` "Require a token for the web UI" landed 23:15). So B18's
|
||
auth fix is *not* in the live container. Rebuild before drawing any
|
||
conclusion about the live surface's auth behaviour.
|
||
- **All six herdrs are unreachable** — workpc `192.168.1.105:9245-7` refuses,
|
||
homesrv `192.168.1.104:9245-7` times out (filtered). Probed directly, since
|
||
a herdr that connects logs nothing. Nothing can be leased; every task the
|
||
API serves is history. This is the sole remaining blocker on live proof for
|
||
B13–B17.
|
||
- `GET /v1/tasks` has **no authz check at all** (`cmd/orchestra/main.go:286`
|
||
returns before any gate). B18 gated the mutation and approval controls, not
|
||
the read path. Accepted for now: the `0.0.0.0:9145` bind is deliberate and
|
||
ufw restricts the port to a single other LAN machine, so the read path's
|
||
trust boundary is the LAN, not the host. Revisit if that bind is ever
|
||
widened.
|
||
- ntfy delivery still fails `403 Forbidden` on every send (last observed
|
||
2026-07-28) — a wrong or expired credential, separate from S12's
|
||
`ORCHESTRA_NTFY_SURFACE_TOKEN`.
|
||
|
||
This does not complete the cross-machine continuity design. `clients/herdr-
|
||
bridge.go` is only a byte proxy; it is not a worker. The worker-side process
|
||
described below remains required before a remote checkout can author
|
||
`TASK.md`, derive a canonical handoff anchor, scratch-commit WIP, or safely
|
||
release it. Until then, homesrv must not rotate or clean up a non-local
|
||
worktree.
|
||
|
||
| Spec layer | State |
|
||
|---|---|
|
||
| L1 substrate (§3, §4) | Built and correct in the main path. |
|
||
| L2 harness (§5) | Occupancy, rotation, and completion all wired and reachable. B12 fixed and confirmed live; B13–B16 fixed in code but not re-verified live. Unique per-task agent names (B16) and ambiguity-safe prompting (B14) are the substantive changes. |
|
||
| L3 continuity (§6) | Handoff schema, pickup validation, scratch branches, TASK.md are all wired into the live path (Phase 4 complete). |
|
||
| L4 surfaces (§7) | Brief/standup/delivery real; quota has a post-hoc producer only (no live push feed yet); B8's authz bypass closed, but the new web UI reintroduces an equivalent one (B18). |
|
||
|
||
---
|
||
|
||
## Blocking defects
|
||
|
||
B14, B15 and B16 are now closed by code and covered by tests; none has been
|
||
re-verified against a live cross-machine run. B17 is closed on the worker
|
||
path only and remains open for Design A. B18 (the unauthenticated web
|
||
surface) is closed in code but **requires an env change before the service
|
||
will start** — see B18 and "What's next".
|
||
|
||
Nothing is currently blocking in the sense B12/B13 were. B19–B21 and S12–S13
|
||
— the correctness and hygiene gaps found while implementing B18 — are now
|
||
fixed in code and covered by tests (2026-07-28); each is marked closed in
|
||
its own section below. The flaky router test is also fixed, and the flake
|
||
was in the test, not in assignment.
|
||
|
||
The real remaining risk is entirely evidential now: B13 through B17 were
|
||
each found live and **none has been re-verified live**, and none of the
|
||
2026-07-28 fixes has run on the deployed instance either — the service is
|
||
stopped, `/usr/local/bin/orchestra` predates all of them, and installing a
|
||
new binary or editing `/etc/orchestra/orchestra.env` needs privileges this
|
||
sandbox does not have.
|
||
|
||
### B18 — the web UI surface is unauthenticated (found by audit 2026-07-28, uncommitted working tree) — closed (code fix; requires an env change before restart)
|
||
|
||
`cmd/orchestra/main.go:205` mounts the browser read/write surface with no
|
||
handler-local credential check:
|
||
|
||
```go
|
||
mux.Handle("/v1/ui/", ui.Server{Store: s, Workers: workers, Coordinator: coordinator, ...}.Handler())
|
||
```
|
||
|
||
Unlike the worker/harness endpoints on the same mux (`main.go:342, 372,
|
||
467, 821, 840`), it performs no check of its own and stamps its events
|
||
`Surface: string(authz.Web)` (`internal/ui/ui.go:222, 337, 368`).
|
||
|
||
**It is not, however, ungated in principle.** `authz.HTTP` wraps the entire
|
||
mux (`main.go:1167`) and defaults a request with no `X-Orchestra-Surface`
|
||
header to `Web`, so `/v1/ui/` is covered by `ORCHESTRA_WEB_TOKEN`. The
|
||
defect is that this gate is opt-in — `authz.HTTP`'s own comment says
|
||
"authentication is optional for local development," and `expected != ""` is
|
||
the only thing standing between the LAN and the surface.
|
||
|
||
**The live deployment has opted out.** `.orchestra-config/orchestra.env`
|
||
sets no `ORCHESTRA_*_TOKEN` at all, and the server binds `":"+port`
|
||
(all interfaces, 9145). So on the deployed homesrv instance this surface is
|
||
in fact reachable unauthenticated from the LAN.
|
||
|
||
**Consequence.** Unauthenticated reachable actions include `TaskCreated`,
|
||
and per-task `release`, `block`, `complete` and `handoff`
|
||
(`ui.go:345-361`). Most seriously, `grant_approval` / `deny_approval`
|
||
(`ui.go:301`) terminate in `pane.send_text` against a live pane — so an
|
||
unauthenticated caller can inject keystrokes into a running agent session on
|
||
workpc. B14 was filed because Orchestra could *accidentally* advance a
|
||
permission dialog; this surface allows it deliberately, from the network.
|
||
|
||
**Why simply setting `ORCHESTRA_WEB_TOKEN` does not fix it.**
|
||
`mux.Handle("/", webui.Handler())` serves the SPA from the same listener,
|
||
behind the same middleware. A browser cannot attach an `Authorization`
|
||
header to a top-level document load, so setting the token returns 401 for
|
||
`index.html` and the web UI stops loading entirely. Bearer-token auth and a
|
||
browser-served SPA are incompatible as currently wired; that is likely why
|
||
no token is set.
|
||
|
||
**Required fix (design decision, not a one-liner).** The browser needs a
|
||
credential it can actually present on a document load — a login endpoint
|
||
that exchanges the Web token for an `HttpOnly`, `SameSite=Strict` session
|
||
cookie, with `authz.HTTP` accepting either that cookie or a bearer token for
|
||
the `Web` surface. Alongside it: make the token mandatory rather than
|
||
opt-in whenever the UI is mounted (refuse to start, rather than silently
|
||
serving an open control plane), and bind to loopback by default so exposure
|
||
is a deliberate configuration act. Approval commands in particular must not
|
||
be reachable without it.
|
||
|
||
**Fix (2026-07-28):**
|
||
|
||
1. **The token is now mandatory** (`cmd/orchestra/main.go`): startup calls
|
||
`log.Fatal` if `ORCHESTRA_WEB_TOKEN` is empty. Serving the control plane
|
||
openly is no longer something a missing env var can cause silently.
|
||
2. **`authz.Sessions`** (`internal/authz/authz.go`) issues 32 random bytes as
|
||
a session value and stores only its SHA-256, so a leaked snapshot of the
|
||
map yields nothing usable. Sessions expire (12h default) and are swept on
|
||
each issue.
|
||
3. **`POST /v1/ui/session`** verifies the Web token in constant time and sets
|
||
the value as an `HttpOnly`, `SameSite=Strict`, `Secure` cookie. `Secure`
|
||
is dropped only if `ORCHESTRA_UI_INSECURE_COOKIE` is set, which is
|
||
required to reach the UI over plain HTTP.
|
||
4. **`authz.HTTPWithSessions`** accepts that cookie *in place of* the bearer
|
||
token, and only for the `Web` surface; the bearer comparison is also now
|
||
constant-time. Two paths are exempt from the gate, both necessarily: the
|
||
login endpoint (which performs its own check — gating it would make login
|
||
unreachable) and non-`/v1/` GET/HEAD, i.e. the SPA shell and its static
|
||
assets, which are not secrets. Every `/v1/` control path stays gated.
|
||
|
||
`TestWebSessionCookieGatesControlPathsOnly` asserts the whole shape: an
|
||
uncredentialed control path is 401, the shell and login endpoint are
|
||
reachable, a valid cookie authorizes, a forged one does not, and a cookie
|
||
presented on the TUI surface is rejected. `TestSessionExpires` covers TTL.
|
||
|
||
**Operational precondition — this is a breaking config change.**
|
||
`.orchestra-config/orchestra.env` currently sets no tokens, so
|
||
`orchestra.service` will refuse to start until `ORCHESTRA_WEB_TOKEN` is
|
||
added (plus `ORCHESTRA_UI_INSECURE_COOKIE=1` if the UI is served over plain
|
||
HTTP). Setting a Web token also newly gates the other `/v1/` surfaces that
|
||
default to `Web` — any existing unauthenticated client of this instance will
|
||
begin getting 401s and needs the token too. Do not restart the service
|
||
without making that env change first.
|
||
|
||
**Still open:** the listener binds all interfaces (`":"+port`); loopback-by-
|
||
default was considered and deliberately not taken, so exposure remains a
|
||
network-level concern.
|
||
|
||
### B14 — `agent.prompt` can duplicate a task prompt after an ambiguous wait timeout (found live 2026-07-28) — closed (code fix; not re-verified live)
|
||
|
||
Live Claude E2E task `06FTF8CPH3K3DPN6XQA7G3WRQ8` (pane `wM:p1`) received
|
||
the same initial `Begin Orchestra task ...` prompt twice. Claude completed a
|
||
first turn inspecting the repository and asking for task context, then began
|
||
working on the identical prompt again after the turn stopped. This was
|
||
observed directly in the pane transcript; it is not merely a UI replay.
|
||
|
||
**Root cause:** `CLIAdapter.Lease` calls `Client.Prompt` with
|
||
`wait.until=idle` and a 60-second wait. `Client.Call`, however, applies the
|
||
client's default 10-second connection deadline when the caller context has
|
||
no deadline. Once that local read deadline expires, `Prompt` treats the
|
||
result as any other transient failure and retries `agent.prompt` with the
|
||
same payload for up to `bootRetryWindow`. Herdr may already have accepted
|
||
the first request, so this is an ambiguous outcome, not a safe retry.
|
||
|
||
**Consequence:** an agent can repeat work, waste context/quota, or overwrite
|
||
its own in-progress changes. The initial task prompt is especially exposed
|
||
because it deliberately waits for the agent to become idle.
|
||
|
||
**Additional live consequence — OpenCode permission bypass (2026-07-28):**
|
||
the OpenCode E2E task `06FTF9Z8RP5FM4F9M2SKAQNFZ4` was leased to
|
||
`workpc-opencode` in `/tmp/test-e2e-worktrees/06FTF9Z8RP5FM4F9M2SKAQNFZ4`.
|
||
An operator directly observed OpenCode stop at the interactive permission
|
||
dialog for its initial `git log --oneline -10 && git status --short` command,
|
||
then advance and run that command without operator input. Workpc herdr's
|
||
live log records Orchestra's `agent.prompt` calls at 07:37:09 (request IDs 6
|
||
and 7), 07:37:10 (ID 8), and 07:37:20 (ID 9); the last two only ended when
|
||
Orchestra's 10-second RPC deadline disconnected the client. Herdr labels
|
||
each as `changes_ui=true`. The active OpenCode manifest contains only
|
||
permission-*detection* rules and no auto-approval rule. The task later
|
||
blocked at the next prompt (`~/.claude/RTK.md`) when no further prompt retry
|
||
advanced it.
|
||
|
||
This does not prove which internal herdr action produced the equivalent of
|
||
Enter, but it proves the safety boundary is broken: an Orchestra-originated
|
||
UI-changing `agent.prompt` operation can advance an OpenCode permission
|
||
dialog. Treat this as an authorization bypass, not merely duplicate work.
|
||
|
||
**Required fix:** make the RPC deadline at least the requested herdr wait
|
||
(plus a small transport margin), and do not blindly resend `agent.prompt`
|
||
after a timeout or other ambiguous post-write error. Record an observable
|
||
failure instead, or use an idempotency/acknowledgement mechanism if herdr
|
||
adds one. In addition, never issue `agent.prompt` while `agent_status` is
|
||
`blocked` (or a pane read matches a permission dialog); treat the condition
|
||
as requiring explicit operator approval. Do not run further unattended
|
||
OpenCode E2E tasks until both controls are verified live.
|
||
|
||
**Fix (2026-07-28), in `Client.Prompt` (`internal/herdr/herdr.go:280`):**
|
||
|
||
1. The RPC deadline is now derived from the requested wait, not the client
|
||
default: when the caller's context has no deadline within `wait + 5s`,
|
||
`Prompt` installs one (`herdr.go:307-314`). A 60s `wait.until=idle` no
|
||
longer expires against a 10s transport deadline.
|
||
2. Retry is split by error class. A JSON-RPC protocol error is herdr's
|
||
explicit rejection *before* it acted and stays retryable for the bounded
|
||
boot window; a transport failure after the write is ambiguous and is
|
||
never resent (`herdr.go:315-325`). Covered by
|
||
`TestPromptDoesNotRetryAmbiguousDelivery`, which closes the connection
|
||
mid-`agent.prompt` and asserts exactly one `agent.prompt` is issued.
|
||
3. The authorization-bypass half is addressed by inspecting the pane before
|
||
prompting: `Prompt` refuses a pane whose `agent_status` is `blocked`
|
||
(`herdr.go:285`) and refuses any pane whose transcript matches
|
||
`permissionPrompt` (`herdr.go:292`). Approval is now an explicit,
|
||
separately authorized operation — see the command-channel section below.
|
||
|
||
Not re-verified live. The original live reproduction was cross-machine, and
|
||
the "do not run further unattended OpenCode E2E tasks until both controls
|
||
are verified live" instruction above still stands.
|
||
|
||
### B15 — a coordinator-side block cannot reconcile a later live completion (found live 2026-07-28) — closed (code fix; operator-mediated, not re-verified live)
|
||
|
||
The same OpenCode E2E task `06FTF9Z8RP5FM4F9M2SKAQNFZ4` was marked
|
||
`TaskBlocked` when the coordinator's `agent.prompt` RPC timed out, despite
|
||
the agent having received the task. The operator later explicitly approved
|
||
the one pending `~/.claude/RTK.md` read through herdr; OpenCode completed the
|
||
verification and became `idle` in pane `wN:p1`. Orchestra still showed
|
||
`state: "blocked"` because `Coordinator.Start` returned through `block()`
|
||
and never persisted a session; its monitor therefore has no live pane to
|
||
observe or reconcile.
|
||
|
||
**Consequence:** the event log can permanently claim a task is blocked when
|
||
the leased harness has actually completed it. This is not merely stale UI:
|
||
blocked tasks have no normal completion hook path and their receipt/quota
|
||
record is lost unless an operator manually corrects the lifecycle.
|
||
|
||
**Required fix:** retain enough launch/session identity after an ambiguous
|
||
lease failure to reconcile `agent.get`/completion evidence, or introduce a
|
||
versioned `TaskCorrected` terminal-state workflow that records the original
|
||
blocker and evidence without pretending it never happened. A live pane must
|
||
not be left outside the coordinator's session map simply because prompt
|
||
delivery timed out.
|
||
|
||
**Fix (2026-07-28), in `Coordinator.Start`
|
||
(`internal/orchestrator/orchestrator.go:911-922`):** when the lease call
|
||
fails but a pane was actually created, the session is completed with
|
||
`HerdrID`, `TaskFileSHA` and `ConventionsHash` and persisted via
|
||
`rememberSession` *before* `block()` records `TaskBlocked`. `Reconcile`
|
||
(`orchestrator.go:421`) explicitly retains mappings for `StateBlocked` as
|
||
well as `StateLeased`, so a restart does not orphan them either. The live
|
||
pane therefore stays observable through `Capture` and the monitor.
|
||
|
||
**Scope limit — reconciliation is operator-mediated, not automatic.**
|
||
Nothing promotes a blocked task to completed on its own; the router still
|
||
never retries a blocked task. What exists now is the evidence and the
|
||
surface: the retained session plus the `complete` / `release` / `handoff`
|
||
actions on the UI task endpoint (`internal/ui/ui.go:345-361`), and the
|
||
pre-existing versioned `TaskCorrected` workflow (S8) for recording the
|
||
correction without pretending the block never happened. That satisfies the
|
||
"must not be left outside the session map" requirement; it does not
|
||
implement autonomous reconciliation, which remains unbuilt by choice.
|
||
|
||
### B16 — hardcoded harness name makes each harness globally single-instance (found live 2026-07-28) — closed (code fix; contract test only partially satisfied)
|
||
|
||
While starting the real OpenCode healthcheck task
|
||
`06FTFDW22833F1CCB8K43Z8808`, Orchestra created worktree/pane `wP:p1` but
|
||
herdr never attached an agent. A direct retry exposed the actual cause:
|
||
`agent.start` was sent with `name: "opencode"`, which herdr rejected with
|
||
`agent_name_taken` because the completed prior E2E agent in `wN:p1` already
|
||
owned that name. This is not a herdr limit of one OpenCode process per
|
||
machine: retrying the same `kind: "opencode"` in `wP:p1` with the valid,
|
||
unique lowercase name `oc-06ftfdw22833f1cc` immediately started a second
|
||
attached, idle OpenCode session while `wN:p1` remained intact.
|
||
|
||
**Root cause:** `Client.StartAgent` passes the harness kind as both
|
||
`kind` and `name`; every OpenCode lease therefore competes for the same
|
||
global agent-name slot. The same defect applies to Claude and Codex.
|
||
|
||
**Required fix:** keep `kind` as the configured harness, but derive `name`
|
||
from a validated, bounded task/session identifier (lowercase letters,
|
||
digits, `-`, `_`, maximum 32 characters) and persist it in `Session` for
|
||
subsequent lifecycle operations. Add a contract test that starts two same-
|
||
harness sessions in distinct panes and verifies both attach.
|
||
|
||
**Fix (2026-07-28):** `agentName(harness, taskID)`
|
||
(`internal/herdr/herdr.go:452`) derives a bounded, validated name — a
|
||
per-harness prefix (`oc`/`cl`/`cx`), the lowercased task ID with everything
|
||
outside `[a-z0-9_-]` collapsed, capped at 32 characters. Over-long IDs keep
|
||
a leading stem plus an 8-hex-character SHA-256 suffix, because plain
|
||
truncation made distinct long task IDs collide in herdr's machine-global
|
||
namespace. `StartAgent` passes it as `name` while `kind` stays the
|
||
configured harness (`herdr.go:393`), persists it as `Session.AgentName`
|
||
(`herdr.go:145, 445`), and binds it for prompt routing (`herdr.go:446`,
|
||
`adapter.go:144`). Pane-scoped operations deliberately keep using
|
||
`PaneID` — `AgentName` is only for agent-targeted calls (`adapter.go:531`).
|
||
|
||
**Requirement not fully met:** the requested contract test does not exist.
|
||
What is covered is name derivation — `TestAgentNameIsBoundedAndValid` and
|
||
`TestAgentNameLongIDsDoNotCollide` — plus `TestPromptDoesNotRetryAmbiguous
|
||
Delivery` asserting the prompt target is the unique name. Nothing yet starts
|
||
two same-harness sessions in distinct panes and verifies both attach, so the
|
||
`agent_name_taken` failure mode itself is untested end-to-end.
|
||
|
||
### B17 — opaque harnesses must not author canonical handoff anchors (found live 2026-07-28) — closed on the worker path; open for Design A
|
||
|
||
The real OpenCode healthcheck run showed that prompting an opaque agent to
|
||
write the full `continuity.Handoff` schema is the wrong ownership boundary.
|
||
The agent wrote JSON that looked plausible but used `anchor.sha` instead of
|
||
`anchor.git_sha`, omitted required top-level fields by nesting them under
|
||
`knowledge`, and could not correctly represent the uncommitted handoff file
|
||
as a dirty-file hash (a file cannot contain the SHA-256 of its own final
|
||
contents). Repeated corrective prompts made the handoff less reliable and
|
||
turned Orchestra's protocol details into agent prompt lore.
|
||
|
||
**Required direction:** prompt a harness exactly once for a small semantic
|
||
handoff/review report (what changed, validation evidence, remaining work,
|
||
review findings). The worker that owns the checkout must then deterministically
|
||
collect `HEAD`, branch, dirty paths and SHA-256 values, construct the canonical
|
||
handoff artifact, and validate it before publication. Canonical handoff files
|
||
are worker-owned protocol state and must be excluded from their own dirty-file
|
||
list. Project-level commands and stable expectations (`build`, `test`, file
|
||
ownership/invariants, etc.) belong in `AGENTS.md`/`CLAUDE.md`; only task-
|
||
specific evidence belongs in the handoff.
|
||
|
||
This cannot be safely implemented in Design A by homesrv: its coordinator
|
||
does not own the workpc checkout. Expand the bridge into a real worker-side
|
||
continuity participant (the deferred Design B direction) so all Git-derived
|
||
handoff facts are gathered and sealed on the machine that hosts the worktree.
|
||
|
||
**Live evidence (OpenCode healthcheck/review run, 2026-07-28):** task
|
||
`06FTFDW22833F1CCB8K43Z8808` initially hit B13/B16: Orchestra created `wP:p1`
|
||
but its hardcoded `name: "opencode"` conflicted with the prior E2E session.
|
||
Starting the same `kind: "opencode"` with unique name `oc-06ftfdw22833f1cc`
|
||
attached successfully without closing the prior pane. The implementation
|
||
agent created executable `scripts/healthcheck.sh`, validated `bash -n`, its
|
||
normal mode, and `--help`, then committed `672c123 Add scripts/healthcheck.sh`.
|
||
It first authored a lookalike handoff (`anchor.sha`, incomplete `meta`, and
|
||
all protocol fields under `knowledge`). A distinct reviewer in a second pane
|
||
(`wP:p2`) independently repeated the checks, noticed the nesting error, and
|
||
rewrote the document. Its claimed schema validation checked only JSON syntax
|
||
and the presence of keys — it did **not** use `continuity.Decode` or the
|
||
actual typed contract: `test` remains an array where `Handoff.Test` is a
|
||
string, and `last_result` remains a string where `Handoff.LastResult` is a
|
||
structured object. This is a concrete proof that a reviewing harness cannot
|
||
be trusted to validate canonical protocol state by visual shape alone.
|
||
|
||
The task is deliberately still `TaskBlocked` from its original failed
|
||
automatic attach even though its manually-started implementation/review panes
|
||
are live, a second live confirmation of B15. The reviewer pane is left open
|
||
for operator inspection; do not close or release it without approval.
|
||
|
||
**Related live observation — unexpected Claude launch explained:** Claude
|
||
pane `wM:p1` did not start spontaneously. At 08:00:41Z the expired lease for
|
||
task `06FTF8CPH3K3DPN6XQA7G3WRQ8` emitted `TaskReleased(reason:
|
||
"lease_expired")`; the router re-leased it at 08:00:42Z, and workpc herdr
|
||
attached Claude at 08:00:43Z. The renewed attempt then hit B14's prompt
|
||
timeout and became blocked. This is expected retry behavior, but illustrates
|
||
why lifecycle retries must be visible to operators.
|
||
|
||
**Status (2026-07-28): satisfied on the worker path, unchanged for Design A.**
|
||
`cmd/orchestra-worker` now implements the required ownership boundary. The
|
||
harness is asked once for a bounded semantic report; `releaseReady`
|
||
(`cmd/orchestra-worker/main.go:180`) then runs `CLIAdapter.Release` and
|
||
`herdr.HeadSHA` **on the machine that hosts the worktree**, so `HEAD`,
|
||
branch, dirty paths and SHA-256 values are collected locally and the
|
||
canonical artifact is validated before publication. The handoff-provenance
|
||
correction documented at the end of this file supplies the parsing and
|
||
validation half.
|
||
|
||
The homesrv coordinator's Design A path is unchanged and still cannot
|
||
satisfy this: it does not own the workpc checkout, so any Git-derived fact
|
||
it seals is derived from the wrong filesystem. B17 is therefore only closed
|
||
for tasks that actually run through a worker. The standing rule holds —
|
||
homesrv must not rotate or clean up a non-local worktree.
|
||
|
||
### B13 — `agent.start` silently no-ops under back-to-back leases (found live 2026-07-28) — closed (code fix; not yet re-verified live)
|
||
|
||
Discovered while live-verifying the B12 fix below. B12 itself is confirmed
|
||
fixed (see that entry), but re-testing it exposed a second, deeper defect
|
||
that B12's retry logic cannot address.
|
||
|
||
Three fresh tasks were leased in quick succession against `workpc-claude`
|
||
(`06FTAKSJZTB73FZQE3QT7XQ1J0`/pane `wD:p1`, `06FTAMFKVEPGPKR0H86C08ZZNG`/pane
|
||
`wE:p1`, `06FTANAYZPA55BABR2J4MWQ050`/pane `wF:p1`). All three got a
|
||
`worktree.create` success and an `agent.start` call that returned **no
|
||
error**. Checked live via `pane.get`/`pane.list` (and independently
|
||
confirmed by the user cd'ing into each worktree by hand):
|
||
|
||
- `wD:p1` (the first, given time alone to settle): a real `claude` process
|
||
did eventually attach — `agent: claude`, `agent_status: idle`,
|
||
`revision: 2`. This is consistent with B12's model: `agent.start`
|
||
acknowledges the request but the actual attach is asynchronous, sometimes
|
||
taking well over a minute.
|
||
- `wE:p1` and `wF:p1` (started moments after `wD:p1`, while it was presumably
|
||
still initializing): **still no agent attached after 2+ minutes of
|
||
polling** — `agent_status: unknown`, no `agent` field, `revision` never
|
||
incremented past its creation value. Empty shells, confirmed both via
|
||
`pane.get` and by the user directly `cd`ing into the worktree and finding
|
||
no session running.
|
||
|
||
So `agent.start`'s "success" is not just delayed for these two — it never
|
||
happened at all, and `agent.start` never returned an error to tell Orchestra
|
||
that. One plausible cause: something about launching a second/third `claude`
|
||
CLI process while a prior one on the same host/account is still mid-startup
|
||
(auth handshake, config lock, etc.) silently drops the later request(s)
|
||
rather than queuing or erroring them. Not confirmed — would need a
|
||
controlled single-at-a-time repro to isolate from herdr's own internals,
|
||
which requires spinning up more real panes and wasn't done this pass (see
|
||
"three orphaned panes" below).
|
||
|
||
**Consequence:** unlike B12 (which at least produced an observable
|
||
`TaskBlocked`), this failure mode leaves the task leased indefinitely against
|
||
a pane that will never produce a session, with no error surfaced anywhere —
|
||
worse than B12 was, because nothing currently distinguishes "still booting,
|
||
give it more time" from "silently dead, will never start." A client-side
|
||
retry on `agent.start` itself (B12's approach) cannot fix this, since the
|
||
call that should have started the process already returned success with no
|
||
error to retry on.
|
||
|
||
**Fix:** since `agent.start`'s own return value can't be trusted, `StartAgent`
|
||
(`internal/herdr/herdr.go`) no longer treats its success as the end of the
|
||
story. After `agent.start` returns without error, it polls `pane.get({pane_id})`
|
||
(new `paneAgentAttached` helper) until the pane reports a real attached agent
|
||
(`agent` non-empty and `agent_status` present and not `"unknown"`), bounded by
|
||
a new `agentAttachWindow` (90s, longer than `bootRetryWindow`'s 15s since a
|
||
legitimate attach was observed live taking "well over a minute"). If the
|
||
window elapses with no attach, `StartAgent` now returns an explicit error
|
||
instead of a false success — this is exactly the "recorded/observable
|
||
failure over silent `continue`" pattern this repo's CLAUDE.md calls for, and
|
||
should surface as `TaskBlocked` through the same path B12's fix already
|
||
proved reachable.
|
||
|
||
**Not yet done:** re-verified against a live herdr instance (the schema for
|
||
`pane.get`'s result — `agent`/`agent_status` fields — was inferred from
|
||
prose in this file's own B12/B13 narration of live `pane.get` output, not
|
||
re-confirmed by a fresh probe of `192.168.1.105:9245`; `deploy/herdr-schema.json`
|
||
has no entry for `pane.get`'s result shape). `go build`/`go vet`/`go test
|
||
./...` all pass, but no test exercises `StartAgent`'s new polling loop
|
||
directly (existing `internal/herdr` tests don't call `agent.start`/`pane.get`
|
||
through the fake TCP listener at all). Next session should confirm the
|
||
`pane.get` field names live before trusting this closes B13 operationally,
|
||
and ideally fire a fresh back-to-back-lease test against workpc once that's
|
||
confirmed.
|
||
|
||
**Side effect of this investigation — three live orphaned panes on workpc,
|
||
left untouched on purpose:** `wD:p1` (has a real but abandoned `claude`
|
||
session, task now stuck in `blocked` from before B12's fix), `wE:p1` and
|
||
`wF:p1` (empty shells, no agent ever attached). Not cleaned up — same
|
||
standing rule as the pre-existing `wA:p1` stuck task: don't call
|
||
`pane.close`/`pane.release_agent` against live panes without asking first.
|
||
|
||
---
|
||
|
||
## B12 — `StartAgent` races the pane's shell readiness (found live 2026-07-28) — closed (this specific race; see B13 for a second issue it exposed)
|
||
|
||
Discovered during the first real end-to-end run against a freshly rebuilt/
|
||
redeployed binary (the running service had been on stale pre-audit commit
|
||
`325c684` the whole time).
|
||
|
||
Created a fresh task (`06FTAH4MCRCAJYY7V75Z4V2YGG`, project `test-e2e`,
|
||
capability `opencode`). It reached `TaskLeased` onto `workpc-opencode`
|
||
(worktree created for real at `/tmp/test-e2e-worktrees/<task-id>` on workpc,
|
||
confirmed via a live `pane.list` — pane `wB:p1`, correct `cwd`), then
|
||
**445ms later** hit `TaskBlocked`:
|
||
|
||
```
|
||
lease: herdr protocol error: agent target pane wB:p1 is not an available shell
|
||
```
|
||
|
||
`internal/herdr/herdr.go:199` (`StartAgent`) calls `agent.start` on the pane
|
||
`Worktree` just recorded, with no wait or retry after `worktree.create`
|
||
returns. herdr hands the pane back before its shell has actually finished
|
||
initializing, and `agent.start` refuses it as not-yet-a-shell. None of B1–B11
|
||
exercised this — every prior fix assumed a session already existed
|
||
(occupancy, rotation, handoff); nothing in the remediation plan tested the
|
||
very first `Lease` call against a real herdr pane end-to-end. Phase 0's
|
||
probing used `params:{}`/missing-field tricks, never a real
|
||
`worktree.create` → `agent.start` sequence timed against actual pane boot.
|
||
|
||
**Reproduced a second time, 2026-07-28**, same session: a second fresh task
|
||
(`06FTAHW8GZP0S6DCHEQ6PJXWB0`) leased onto a brand-new pane (`wC:p1`) and hit
|
||
the identical error on the same sub-second timeline. Two for two — a
|
||
systematic race, not a flake.
|
||
|
||
**Consequence:** every fresh lease that hits this race goes straight to
|
||
`TaskBlocked`, and the router **never retries a blocked task**
|
||
(`internal/router/router.go` has no reference to `StateBlocked` at all,
|
||
confirmed by grep) — so it sits there permanently until a human intervenes
|
||
via the approval surface. This is now the single blocking defect for proving
|
||
the rest of the remediation plan against a real live task.
|
||
|
||
**Fix, iterated live across three rounds, 2026-07-28:**
|
||
|
||
1. First pass: bounded-retry `agent.start` (10×, 500ms apart) specifically on
|
||
the `"not an available shell"` string. Redeployed and re-tested against a
|
||
real fresh lease — this cleared that exact error, but the very next call
|
||
in the sequence (`Lease`'s post-`agent.start` `Prompt`) then hit a
|
||
*different* transient error, `"agent ... is not an active named agent"` —
|
||
same underlying readiness race, one call later, worded differently.
|
||
2. Second pass: added the identical string-matched retry to `Prompt` for that
|
||
specific message. Redeployed, re-tested — this time hit a *third* distinct
|
||
wording, `"agent target ... not found"`, on the same call.
|
||
3. Given three different error strings for the same race across three live
|
||
attempts, string-matching was abandoned as unwinnable (herdr's wording for
|
||
"not ready yet" isn't fixed enough to enumerate). **Final shape:** both
|
||
`StartAgent`'s `agent.start` call and `Prompt`'s `agent.prompt` call now
|
||
retry on *any* error, bounded by wall-clock time (15s, `bootRetryDelay` of
|
||
500ms) rather than attempt count or specific wording — there is no other
|
||
legitimate reason a call against a pane/worktree orchestra itself just
|
||
created would fail immediately. `StartAgent` still special-cases `"already"`
|
||
as a non-error (idempotent re-lease).
|
||
|
||
Confirmed this closes the original race: task `06FTAKSJZTB73FZQE3QT7XQ1J0`
|
||
(pane `wD:p1`) leased successfully, and polling `pane.get` live shows a real
|
||
`agent: claude` / `agent_status: idle` attach — independently confirmed by
|
||
the user `cd`-ing into that worktree by hand. `go build`/`go vet`/`go test
|
||
./...` all pass throughout.
|
||
|
||
**Not fully clean, however:** re-testing this fix by firing two more tasks
|
||
shortly after the first one surfaced a second, separate defect — `agent.start`
|
||
returning success while never actually starting an agent, with no error for
|
||
the retry logic to even see. That's **B13**, above — this entry only covers
|
||
the specific shell/agent-readiness race B12 was originally filed for, which
|
||
*is* fixed; B13 is a distinct root cause the same investigation exposed.
|
||
|
||
---
|
||
|
||
## Web UI and the capture / approval command channel (uncommitted, 2026-07-28)
|
||
|
||
Roughly 520 lines of Go across eight tracked files, plus the untracked
|
||
`internal/ui`, `internal/webui` and `web/` trees, were added after the last
|
||
commit and were previously undocumented here. Recording the design so the
|
||
next session does not have to re-derive it.
|
||
|
||
**Capture/command model (`internal/federation/federation.go`).** Two new
|
||
worker-scoped maps on `Registry`. `PutCapture` stores the latest pane text
|
||
per (worker, task) and bumps a monotonic `Revision` only when the text or
|
||
pane actually changes. `Queue` accepts only `grant_approval` /
|
||
`deny_approval`, and requires a non-zero `CaptureRevision` — a command is
|
||
bound to the exact capture an operator was looking at. `CompleteCommand`
|
||
refuses to resolve a command twice.
|
||
|
||
**Worker side (`cmd/orchestra-worker/main.go`).** `publishCaptures` pushes
|
||
recent pane text for every held session; `runCommands` polls
|
||
`/v1/federation/commands` and, for each command, re-reads the pane,
|
||
re-publishes it, and refuses with `stale` if the resulting revision differs
|
||
from the one the command was issued against. `approvalResponse` decides the
|
||
keystroke: it sends `y`/`n` only for a visible `[y/n]`/`(y/n)` prompt, and
|
||
sends bare Enter only for OpenCode's fully-labelled `Allow once / Allow
|
||
always / Reject … enter confirm` selector — where Enter is a bounded
|
||
one-time grant. It refuses to *deny* through that selector, because doing so
|
||
would require unobservable navigation. Unknown dialog layouts are rejected
|
||
outright rather than guessed at. Both branches are covered by
|
||
`TestApprovalResponseOpenCodeAllowOnce` and
|
||
`TestWorkerApprovalCommandIsRevisionBoundAndAcknowledged`.
|
||
|
||
This is the right shape for B14's authorization half: approval becomes an
|
||
explicit, revision-bound, separately-issued operation instead of a side
|
||
effect of prompting, and it executes on the worker that owns the pane rather
|
||
than over a coordinator-driven remote socket.
|
||
|
||
**Coordinator side.** `Coordinator.RespondApproval` and
|
||
`herdr.ApprovalResponder` (`internal/herdr/adapter.go:602`) are the
|
||
local-herdr equivalent, applying the same re-read-before-send rule via an
|
||
`expectedCapture` comparison and the same y/n-only refusal.
|
||
`Coordinator.RequestHandoff` exposes the handoff request as an operator
|
||
action without releasing the pane.
|
||
|
||
**Known weaknesses of this channel.** The defects found in it are filed
|
||
individually below — **B19** (federated approvals emit no event), **B20**
|
||
(the local capture revision is a timestamp, not a change counter) and
|
||
**B21** (`Registry.commands` never prunes). Two more, not separately filed:
|
||
|
||
- **Captures and commands are in-memory only.** `Registry.captures` and
|
||
`Registry.commands` have no persistence, so a coordinator restart drops
|
||
pending approvals silently and resets revision counters — which, with a
|
||
reset counter, could let a stale command match a new capture. This is the
|
||
other half of B21: the lifecycle forgets across restarts and never forgets
|
||
within one.
|
||
- **`RespondApproval` (coordinator path) has no test**, unlike its worker
|
||
counterpart — and per B20 it is the path whose revision is meaningless,
|
||
so its text-comparison guard is the only thing actually binding the
|
||
decision to what the operator saw. That guard is untested.
|
||
- **`web/` is a full Vite/React app whose build output is embedded** via
|
||
`internal/webui`'s `go:embed assets/*`. The embedded assets are checked in
|
||
as build output; there is no documented step tying `web/` sources to a
|
||
rebuild of `internal/webui/assets`, so the two can silently diverge.
|
||
|
||
## Found while implementing B18 (2026-07-28) — all fixed the same day
|
||
|
||
These were noticed while wiring the session gate, and all five were fixed in
|
||
the following pass. Each section states the original defect first, then how
|
||
it was closed.
|
||
|
||
### B19 — federated approvals leave no audit trail — closed (2026-07-28)
|
||
|
||
`Server.action`'s `grant_approval` / `deny_approval` branch
|
||
(`internal/ui/ui.go:301-341`) has two exits that are **not** symmetric. The
|
||
local-coordinator path calls `RespondApproval` and then appends
|
||
`ApprovalGranted` / `ApprovalDenied` to the store. The federated path calls
|
||
`s.Workers.Queue(...)`, writes the command to the response, and **returns
|
||
before appending any event**.
|
||
|
||
So the single most safety-critical operation in the system — injecting a
|
||
keystroke that advances a permission dialog on another machine — is
|
||
invisible in the event log precisely when it crosses a machine boundary.
|
||
There is no record of who approved what, or that an approval happened at
|
||
all. §7.1's authorization model assumes the event log is the record.
|
||
|
||
Fixing this needs a decision the local path did not have to make: the
|
||
command is *queued*, not executed, so the honest event is a request at queue
|
||
time plus a resolution when the worker acknowledges (`CompleteCommand`
|
||
already carries `acknowledged` / `rejected` / `stale`). Emitting
|
||
`ApprovalGranted` at queue time would claim an outcome that has not
|
||
happened yet.
|
||
|
||
**Fix.** The resolution half already existed and this entry understated it:
|
||
`/v1/federation/commands/` in `cmd/orchestra/main.go` appends
|
||
`ApprovalGranted`/`ApprovalDenied` — but only on `status == "acknowledged"`,
|
||
so a queued click never claims an outcome. What was missing was the request
|
||
half. `Server.action` now appends `ApprovalRequested` at queue time, with
|
||
`subject_ref` set to the command ID that the later resolution event carries,
|
||
plus `decision`, `worker`, `pane_id` and `capture_revision`. If that append
|
||
fails the queued command is immediately resolved `rejected`, so the worker
|
||
can never execute a keystroke that left no audit trail. `rejected`/`stale`
|
||
outcomes still emit no resolution event; the `ApprovalRequested` stays
|
||
unresolved and surfaces in `operations.NeedsAttention`, which is the honest
|
||
reading. Covered by
|
||
`TestFederatedApprovalIsQueuedAtTheWorkerRevisionAndAudited`.
|
||
|
||
### B20 — the local capture revision is a timestamp, not a change counter — closed (2026-07-28)
|
||
|
||
`Server.capture` (`ui.go:80`) fabricates `Revision:
|
||
uint64(time.Now().UnixNano())` for the coordinator path. The federation
|
||
path's revision is a real per-change counter (`Registry.PutCapture` bumps it
|
||
only when pane text or pane ID actually changes), and the worker's staleness
|
||
check depends on that meaning.
|
||
|
||
Two consequences:
|
||
|
||
1. The local revision changes on every read, so it conveys nothing about
|
||
whether the pane changed. The local path is still safe, but only because
|
||
`RespondApproval` re-reads and compares capture *text* — the revision it
|
||
reports to the browser is decorative.
|
||
2. If a task has both a live coordinator session and a published worker
|
||
capture, `capture()` prefers the coordinator (`ui.go:77`) and returns a
|
||
timestamp revision, which `action()` then passes to `Queue` as
|
||
`CaptureRevision`. That value can never equal the worker's counter, so
|
||
the worker resolves every such command `stale` and the approval silently
|
||
never happens. The precedence between the two capture sources needs to be
|
||
explicit rather than incidental.
|
||
|
||
### B21 — `Registry.commands` is append-only — closed (2026-07-28)
|
||
|
||
`r.commands[worker]` is only ever appended to (`federation.go:119`);
|
||
`CompleteCommand` flips a status in place and nothing is ever deleted.
|
||
Resolved commands accumulate for the process lifetime, and `Commands()`
|
||
rescans the entire history on every worker poll. Combined with the
|
||
in-memory-only storage already noted, the lifecycle is wrong at both ends:
|
||
it forgets across restarts and never forgets within one.
|
||
|
||
**Fix (within-process half only).** `Registry.pruneCommands` drops resolved
|
||
commands older than `CommandRetention` (30 minutes), running on both `Queue`
|
||
and `Commands`, so the per-worker list is bounded and the poll path no
|
||
longer rescans unbounded history. Pending commands are never pruned, at any
|
||
age — dropping one would silently discard an operator decision. Covered by
|
||
`TestResolvedCommandsArePrunedButPendingOnesSurvive`. **The persistence half
|
||
of this defect is still open:** captures and commands remain in-memory only,
|
||
so a coordinator restart still drops pending approvals silently.
|
||
|
||
### S12 — `ORCHESTRA_NTFY_TOKEN` serves two unrelated purposes — closed (2026-07-28)
|
||
|
||
The same env var is the ntfy *server* credential (`main.go:1153`,
|
||
`delivery.Ntfy{Token: ...}`) and the authz *surface* credential for
|
||
`authz.Ntfy` (`main.go:1165`). These are unrelated secrets with different
|
||
trust boundaries: one is handed to a third-party notification server, the
|
||
other authenticates callers to Orchestra. Setting the former silently makes
|
||
it a valid inbound credential. They need separate variables.
|
||
|
||
**Fix.** The `authz.Ntfy` surface token is now `ORCHESTRA_NTFY_SURFACE_TOKEN`;
|
||
`ORCHESTRA_NTFY_TOKEN` is once again only the credential handed to the ntfy
|
||
server. Documented in `deploy/orchestra.env.example`. **Operator action:**
|
||
any deployment that relied on the old dual use must now set the new variable
|
||
explicitly, or the ntfy surface reverts to having no inbound gate.
|
||
|
||
### S13 — dead `auth()` middleware in `cmd/orchestra/main.go` — closed (2026-07-28)
|
||
|
||
`func auth(next http.Handler)` (`main.go:1212`) is defined and never
|
||
referenced; Go does not flag unused functions, so `go vet` stays quiet. It
|
||
reimplements the notify-only rule that `authz.HTTP` already enforces. It is
|
||
harmless today and a trap tomorrow — a second, divergent copy of the
|
||
authorization policy sitting next to the real one. Delete it.
|
||
|
||
### Positive note — B8 is more strongly closed than it was
|
||
|
||
`authz.HTTP` downgrades a caller-declared `system` surface to `Web` before
|
||
the token comparison. Previously that landed on a surface whose token was
|
||
unset in production, i.e. no gate at all. With `ORCHESTRA_WEB_TOKEN` now
|
||
mandatory, the downgrade lands on a genuinely gated surface, so the B8
|
||
bypass is closed by construction rather than by the header check alone.
|
||
|
||
---
|
||
|
||
## What's next
|
||
|
||
In dependency order, not importance order:
|
||
|
||
1. **Finish applying the B18 env change on the deployed instance.** The
|
||
repo-side `.orchestra-config/orchestra.env` now sets a generated
|
||
`ORCHESTRA_WEB_TOKEN` and `ORCHESTRA_UI_INSECURE_COOKIE=1`, but the unit
|
||
reads `/etc/orchestra/orchestra.env`, which is `orchestra:orchestra 0600`
|
||
and unreadable from this sandbox — whether it carries the token could not
|
||
be confirmed. It was last modified 2026-07-28 14:30 and the service did
|
||
start at 16:17 with the B18 build absent, so this is unverified either
|
||
way. Also newly required: `ORCHESTRA_NTFY_SURFACE_TOKEN` (S12) if the
|
||
ntfy surface should stay gated, and the token for every existing
|
||
unauthenticated `/v1/` client, which now defaults to the Web surface.
|
||
2. **Install the new binary and restart.** `orchestra.service` has been
|
||
stopped since 2026-07-28 19:46 and `/usr/local/bin/orchestra` predates
|
||
every fix in this section. Needs privileges the sandbox lacks:
|
||
`go build -o /tmp/orchestra ./cmd/orchestra && sudo install /tmp/orchestra
|
||
/usr/local/bin/orchestra && sudo systemctl restart orchestra.service`.
|
||
3. **The live re-verification** that B13–B17 all still lack, plus a first
|
||
live exercise of B18–B21. Each of B13–B17 was found live and closed on
|
||
paper; the code fixes are unproven against a real cross-machine run, and
|
||
that remains the single biggest gap between this document and reality.
|
||
Note that no herdr is currently reachable at all — the 16:17 startup logs
|
||
`connection refused` for **all six**, including workpc's, which was live
|
||
on 2026-07-27 — so a live run needs a herdr brought up first.
|
||
4. **Two operational faults visible in the journal**, unrelated to this
|
||
audit's defects but blocking a clean live run: every herdr is refusing
|
||
connections (above), and ntfy delivery is failing `403 Forbidden` on
|
||
every send (16:43 and 16:46), i.e. the ntfy server credential is wrong or
|
||
expired.
|
||
5. **The persistence half of B21** — captures and commands are in-memory
|
||
only, so a restart still silently drops pending approvals. The retention
|
||
fix bounds growth within a process; it does not make the lifecycle
|
||
durable.
|
||
6. **`RespondApproval` (coordinator path) still has no test**, unlike its
|
||
worker counterpart. With B20 fixed its revision is now meaningful, but
|
||
its text-comparison guard remains the thing actually binding a decision
|
||
to what the operator saw, and that guard is untested.
|
||
|
||
Deliberately *not* next: building further on Design A's cross-machine calls,
|
||
and closing B17 for the coordinator path. Both wait on the federation-fork
|
||
decision below.
|
||
|
||
---
|
||
|
||
## Closed defects
|
||
|
||
Each entry is the flattened final state — design + what's verified — not the
|
||
session-by-session narration. Grouped by original defect ID.
|
||
|
||
### B1 — Occupancy measured from the wrong value (§5.2.1) — closed
|
||
|
||
`CLIAdapter.Occupancy` was calling `a.Usage(s.PaneID)` (a herdr pane id)
|
||
against readers (`ClaudeUsage`/`CodexUsage`/`OpenCodeUsage`) that want a
|
||
filesystem path to session state — every call failed and was swallowed by a
|
||
bare `continue`. Fixed: `herdr.Session.SessionFile` is resolved at lease time
|
||
per harness (Claude: newest-mtime transcript under Claude Code's own project
|
||
dir; Codex: existing `CodexActiveUsage` sqlite discovery; opencode: refuses
|
||
loudly rather than guess, since it needs a live session id not resolvable
|
||
from the worktree alone). A missing/unreadable session file is now a hard
|
||
error surfaced via `SessionHealth.Occupancy`/`OccupancyError` on
|
||
`GET /v1/tasks/{id}/health`, never a silent zero.
|
||
|
||
**Still open:** live verification against a real Claude Code session at a
|
||
known context fill — not possible from this sandbox, blocked further by B13
|
||
now that fresh leases don't reliably survive.
|
||
|
||
### B2 — Adapter lookup keyed wrong in 3 of 4 call sites (§5.3, §5.4) — closed
|
||
|
||
`AdapterFactory.Herdrs` is keyed by herdr instance id (`homesrv-claude`);
|
||
`Reconcile`/`expire`/`rotate` were looking it up by `session.Harness` (the
|
||
harness kind, `claude`) and silently no-op'ing (`continue`) on every miss —
|
||
orphaned panes never killed, expired leases never killed their pane,
|
||
rotation never began. Fixed with a single `Coordinator.adapterFor(session)`
|
||
routed through at all four call sites. Regression test registers an adapter
|
||
under a herdr-id key distinct from the harness kind and asserts rotation
|
||
still fires.
|
||
|
||
### B3 — Nothing emitted `TaskCompleted` (§4, §5.2) — closed
|
||
|
||
Only producer used to be a human calling `POST /v1/tasks/{id}/complete`.
|
||
Added `POST /v1/harness/complete`: a Claude Code Stop hook
|
||
(`deploy/hooks/orchestra-stop.sh`) fires every turn boundary but only
|
||
reports completion once the agent has written a `.orchestra-report.md`
|
||
marker at the worktree root (an ordinary turn boundary is a no-op). The
|
||
server builds the `receipt` itself from `herdr.ClaudeUsage` against the
|
||
transcript (never trusts a self-reported number) and uploads the report
|
||
body to CAS for `report_ref`. Gated by optional `ORCHESTRA_HARNESS_TOKEN`;
|
||
event appended with `Surface: system` set directly in Go, consistent with
|
||
B8. Extended to Codex/opencode via an optional `harness` field in the
|
||
request body (`"codex"` → `CodexUsage`, `"opencode"` → `OpenCodeUsage`).
|
||
|
||
**Still open:** no automated test for the HTTP handler (`cmd/orchestra/main.go`
|
||
has zero handler test coverage of any kind, pre-existing gap — this follows
|
||
the existing pattern rather than introducing a one-off harness).
|
||
|
||
### B4 — Router counted rotation as a retry (§5.3, §5.4) — closed
|
||
|
||
Every `TaskReleased` (including rotation, which carries a valid
|
||
`handoff_ref`) advanced `MaxAttempts`, and lease also double-counted — a task
|
||
healthy enough to rotate twice was killed. Now only a release *without* a
|
||
`handoff_ref` (expiry/crash) advances the counter.
|
||
|
||
### B5 — Herdr protocol methods unverified/invented — closed
|
||
|
||
Confirmed live against `192.168.1.105:9245` (raw JSON-RPC probing, no local
|
||
`herdr` CLI available — schema reconstructed from Rust serde's
|
||
"unknown variant"/"missing field" errors). `pane.release`, `pane.kill`,
|
||
`pane.rotation_signal`, `pane.status` **do not exist**. Real replacements:
|
||
`pane.close({pane_id})` for kill (drop-in); `pane.release_agent({pane_id,
|
||
source, agent})` for release — structurally different, does **not** return
|
||
a `handoff_ref` (herdr never writes handoffs; the agent does, §6.1).
|
||
`RotationSignal` interface/method/call site deleted outright (no replacement
|
||
exists; herdr has no concept of Orchestra rotation).
|
||
|
||
`CLIAdapter.Release` now: reads the agent-authored `.orchestra-handoff.json`,
|
||
validates with `continuity.Decode`, cross-checks its anchor SHA against the
|
||
worktree's real `HeadSHA`, re-verifies every `Anchor.Dirty` file hash,
|
||
scratch-commits any dirty files (`continuity.ScratchCommit`, idempotent
|
||
across multiple rotations) and rewrites the handoff's anchor to the new
|
||
scratch commit before uploading to CAS via `continuity.Save` (minting the
|
||
real `handoff_ref`), and only then calls `pane.release_agent` — sequenced
|
||
last so a herdr-side error can't strand an uploaded handoff. Any failure
|
||
(missing file, invalid schema, anchor mismatch, stale dirty hash, herdr
|
||
error) is a refusal, which `rotate` already treats as "retry next tick."
|
||
`CLIAdapter.Lease`'s bootstrap prompt also fixed to pass `time.Minute` (not
|
||
`wait=0`) for inline-wait, matching `Bootstrap` and closing the
|
||
send-into-a-half-rendered-prompt race §5.1 requires guarding against.
|
||
|
||
Covered by `internal/herdr/adapter_test.go` against a real git worktree and a
|
||
fake in-process herdr TCP listener: upload-and-release on a valid handoff,
|
||
refusal with no handoff file, refusal on anchor mismatch, refusal on stale
|
||
dirty-file hash, refusal with no CAS configured.
|
||
|
||
Protocol version confirmed live as a bare JSON number (`17`), not the string
|
||
`config.jsonc` declares — `CheckProtocol`'s raw-bytes fallback happens to
|
||
compare correctly today; don't "clean up" that code without re-checking
|
||
this, or it may start doing a real numeric-vs-string comparison and break.
|
||
|
||
**Still open:** nothing makes the agent actually write
|
||
`.orchestra-handoff.json` unprompted (closed separately, see B6/Phase 4 item
|
||
2 below) — that's the piece this entry originally deferred.
|
||
|
||
### B6 — Layer 3 (continuity) was entirely dead code (§6) — closed
|
||
|
||
Nothing wrote `TASK.md`, so pickup validation had nothing to check and never
|
||
ran. Fixed in full:
|
||
|
||
- `GitWorktrees.Create` now writes and commits an immutable `TASK.md`
|
||
(`continuity.RenderTaskFile`) into every fresh worktree;
|
||
`continuity.TaskFileHash` reads it back for `w.TaskFileSHA`.
|
||
- `Coordinator.Start` runs `continuity.ValidatePickup` (anchor SHA +
|
||
dirty-file hashes + TASK.md hash) before bootstrapping a successor onto a
|
||
`handoff_ref` — failure kills the session and emits `TaskBlocked` instead
|
||
of trusting an unvalidated ref.
|
||
- `ScratchCommit` wired into `Release` (see B5) so pickup collapses to a
|
||
single HEAD compare instead of per-file rehashing.
|
||
- `CLIAdapter.Bootstrap`'s prompt rewritten to point the agent at
|
||
`git log`/the scratch branch (trust already established by the plane's own
|
||
`ValidatePickup`) instead of vague "read the handoff" prose, and
|
||
deliberately does not claim a `GET /v1/artifacts/<ref>` endpoint since none
|
||
exists (`/v1/artifacts` is POST-only — an earlier draft invented this and
|
||
was corrected before landing).
|
||
- `continuity.MarkdownChanges` (zero callers, zero tests despite being
|
||
listed as implemented in an earlier snapshot) was deleted rather than
|
||
half-wired. §6.3's actual requirement ("notice to agents whose task is
|
||
adjacent") was rebuilt independently: `continuity.ConventionsHash(root)`
|
||
hashes `AGENTS.md`/`CLAUDE.md`/`VOCAB.md`; `Coordinator.checkConventions`
|
||
runs every `Monitor` tick, recomputes the project base repo's hash for
|
||
every leased session, and on drift calls the optional
|
||
`herdr.ConventionsNotifier` capability (in-pane prompt) once per drift.
|
||
- **Handoff production, the item that most of B6 hinged on:**
|
||
`Coordinator.rotate` checks the adapter's optional `herdr.HandoffRequester`
|
||
capability; if `.orchestra-handoff.json` is missing, it prompts the agent
|
||
once (`CLIAdapter.RequestHandoff`, naming the exact §6.1 JSON shape and
|
||
explicitly telling the agent not to fabricate SHA/hashes) and skips
|
||
`Release` that tick, retrying every subsequent tick — mirrors the
|
||
`.orchestra-report.md`/B3 convention exactly. `Session.HandoffRequested`
|
||
avoids re-prompting every tick.
|
||
|
||
Covered by `TestGitWorktreesCommitsTaskFile`, `TestStartBlocksOnInvalidPickup`,
|
||
`TestReleaseScratchCommitsDirtyFilesBeforeUpload`,
|
||
`TestReleaseRefusesOnStaleDirtyFile`, `TestConventionsDriftNotifiesActiveSession`,
|
||
`TestRotationRequestsHandoffBeforeReleasing` (internal/herdr, internal/orchestrator).
|
||
|
||
**Caveat still open:** TASK.md hashing is best-effort/untested for the
|
||
herdr-hosted (`WorktreeCreator`) worktree path specifically — no adapter or
|
||
test exercises that path with a real `handoff_ref`, so pickup validation
|
||
there runs with an empty `taskFileSHA` (anchor + dirty-file hashes still
|
||
checked). Same cross-host caveat as the federation fork, below.
|
||
|
||
### B7 — Quota projection had no producer (§7.2) — closed (post-hoc only)
|
||
|
||
`POST /v1/harness/complete` now appends a `QuotaReported` event
|
||
(`consumed` from the same `usage.Numerator()` used for the receipt), so the
|
||
router's 5h/weekly filter and the brief's `quota_consumed` stop evaluating
|
||
against a permanent zero.
|
||
|
||
**Design gap recorded, not yet built** — a live push producer, since a
|
||
harness that never completes a task cleanly (e.g. the stuck `wA` pane, see
|
||
below) currently under-counts its consumption forever:
|
||
- **Claude**: the statusline stdin JSON already carries real
|
||
`.rate_limits.five_hour.used_percentage` / `.seven_day.used_percentage` —
|
||
confirmed by reading `~/.claude/statusline.sh` on this machine. Prefer this
|
||
over `claude.ai/api/.../usage`, which needs browser session cookies, not
|
||
an API key.
|
||
- **Codex**: every `token_count` event in the rollout carries a `rate_limits`
|
||
object with `used_percent`/`window_minutes`/`resets_at` — confirmed
|
||
directly from a real rollout file. No `codex usage` subcommand; the
|
||
rollout tail (or `codex app-server`, same events live) is the only
|
||
transport.
|
||
- **opencode**: no first-class quota surface. `opencode stats` is historical
|
||
only. Zen's free-tier daily quota arrives as `x-ratelimit-*` response
|
||
headers, not a queryable endpoint — would need an opencode plugin to
|
||
intercept.
|
||
- Structural consequence if this is ever built: `sumSince`'s *additive* model
|
||
is wrong for a used-*percentage* feed (need a second `Availability` reading
|
||
the latest report per harness, not summing); static `quota_limit_5h/weekly`
|
||
become unnecessary for Claude/Codex once real fractions are reported;
|
||
`QuotaWindowLimits{FiveHour,Weekly}` is too narrow for Codex's
|
||
self-describing `window_minutes` or opencode's daily Zen window.
|
||
|
||
### B8 — `Surface: system` unauthenticated bypass (§7.1) — closed
|
||
|
||
`X-Orchestra-Surface: system` was reachable from any HTTP request header in
|
||
both `authz.HTTP` and `main.go`'s `surface` closure — since no deployment
|
||
sets `ORCHESTRA_SYSTEM_TOKEN`, this was an unauthenticated full-control
|
||
bypass reachable from any LAN caller. Both call sites now downgrade `system`
|
||
to `web` before doing anything else with it. `System` is only constructible
|
||
in-process (router leases/failures, coordinator releases/blocks, standup
|
||
advisory/apply) via the bus-level `authz.AuthorizeEvent` check now enforced
|
||
inside `store.Append` itself.
|
||
|
||
### S1 — duplicate JSON struct tags — closed
|
||
|
||
`Brief.From`/`To` and `GitSync.Branch`/`Head`/`Status` each shared one JSON
|
||
tag (Go only honors the first `json:"..."` tag on a combined field
|
||
declaration), making `go vet ./...` fail and the brief's git state
|
||
unparseable. Fixed; `go vet ./...` passes clean.
|
||
|
||
### S2 + S3 — brief git state and completion receipts — closed
|
||
|
||
`Brief.Git` used to read from `ORCHESTRA_DATA` (the event-log dir, never a
|
||
git checkout — always "git unavailable") and completions were counted but
|
||
never surfaced with proof. `operations.Brief.Git` is now `map[string]GitSync`
|
||
keyed by project ID, built from `registry.Project.Repo` (falling back to a
|
||
single `"default"` entry off `ORCHESTRA_REPO`), with `Ahead`/`Behind` vs
|
||
upstream added. `Brief.Receipts []CompletionReceipt` pulls `report_ref`/
|
||
`receipt` straight out of each `TaskCompleted` event's existing payload.
|
||
|
||
### S4 — notification goroutine died on first send error — closed
|
||
|
||
`delivery.Fanout.Run` used to `return` on the first sender error,
|
||
permanently killing notifications for the rest of the process's lifetime.
|
||
Failed sends now go through an `OnError` hook instead of aborting the loop.
|
||
Cursor is persisted (`SaveCursor` → a `delivery-cursor` file next to
|
||
`ORCHESTRA_DATA`), so a restart resumes from the last delivered event
|
||
instead of re-notifying the entire log from seq 0.
|
||
|
||
### S5 — colliding event IDs on lease — closed
|
||
|
||
`Store.Lease`/`ExpireLeases` set `Event.ID` to the task id, so every lease of
|
||
a task produced colliding event IDs. Now uses `domain.NewID()`.
|
||
|
||
### S6 — ingest dedup returned the wrong event with 200 — closed
|
||
|
||
Dedup path returned `nil` (success) without appending; the HTTP handler then
|
||
returned an unrelated event with `201`. Added `domain.ErrDuplicate` and
|
||
`Store.TaskBySource`; `POST /v1/tasks` now returns the existing task with
|
||
`200` on a duplicate. Every other `Append` caller (Gitea poll/webhook, JSONL
|
||
ingest) now treats `ErrDuplicate` as expected.
|
||
|
||
### S7 — `TaskAmended` dropped most fields — closed
|
||
|
||
Only `title` was ever applied from an amendment payload; `due`/
|
||
`description`/`inherent_priority` were accepted, logged, and silently
|
||
dropped by the projection. Also added the missing `Task.Description` field
|
||
entirely (it didn't exist, so `TaskCreated` dropped it too). Both
|
||
`TaskCreated` and `TaskAmended` now populate all four fields.
|
||
|
||
### S8 — no compensating-event mechanism (§3.1) — closed
|
||
|
||
Added `TaskCorrected`: payload requires `corrects` (the id of the event it
|
||
repairs) plus at least one change (`state`, or the existing amend-style
|
||
fields). `Store.Append` rejects a `corrects` that doesn't name a real prior
|
||
event on the same task; `apply` applies the changes and clears `Lease` like
|
||
every other terminal-state branch. Covered by a mistaken `TaskFailed`
|
||
reverted to `queued`, an unknown-`corrects` rejection, and both events
|
||
surviving snapshot+replay.
|
||
|
||
### S9 — duplicate lease-expiry tickers — closed
|
||
|
||
`Coordinator.Monitor`'s 30s ticker and `main.go`'s own 1s reclaim ticker both
|
||
called `Store.ExpireLeases` independently. Previously filed as "harmless —
|
||
CAS rejects the loser," but the real effect was worse: the coordinator's
|
||
`expire()` is the *only* place that kills the herdr session/pane for an
|
||
expired lease, and the 1s ticker running 30x more often almost always won
|
||
the race, leaving the coordinator's own call with nothing left to expire —
|
||
silently orphaning herdr panes past TTL. Fixed: `main.go`'s ticker skips
|
||
`ExpireLeases` entirely when a coordinator is configured (deferring reclaim
|
||
to the coordinator's loop), keeping only `AssignPending` as periodic retry.
|
||
|
||
### S10 — federation worker registration had no admission control — closed
|
||
|
||
Any caller could self-declare a worker `id` and self-choose a `token`, and a
|
||
second caller could silently re-register an existing worker id with a
|
||
*different* token, hijacking its identity/capacity. Added
|
||
`Registry.AdmitToken` (pre-shared secret via
|
||
`ORCHESTRA_FEDERATION_ADMIT_TOKEN`, checked against the registration
|
||
request's bearer header); same-ID re-registration now requires presenting
|
||
the existing worker's own token (legitimate restarts still succeed;
|
||
different-token same-id is rejected as a hijack).
|
||
|
||
### S11 — soft threshold, milestone, thrash, agent-initiated ROTATE (§5.3) — closed
|
||
|
||
All four of §5.3's rotation triggers now land:
|
||
|
||
- **Soft threshold**: `Coordinator.Soft` (default 0.55,
|
||
`ORCHESTRA_OCCUPANCY_SOFT`) — both `rotate()` and `TurnDecision` request a
|
||
handoff once occupancy crosses `Soft`, before `Hard` forces one;
|
||
`TurnDecision` returns `prepare_handoff` advisory (task stays leased, no
|
||
turn boundary required).
|
||
- **Agent-initiated ROTATE**: `handoffReason(worktree)` reads a written
|
||
`.orchestra-handoff.json`; if `reason == "manual"`, both `rotate()` and
|
||
`TurnDecision` skip occupancy and the turn-boundary probe entirely and
|
||
release immediately — the agent's own handoff *is* the boundary signal.
|
||
Shared release-and-certify tail extracted into `Coordinator.finishRelease`
|
||
so this path gets the same anchor-safety guarantee as the threshold path.
|
||
- **Milestone + thrash**: needed a transcript/tool-call introspection source
|
||
this repo didn't have — built new, `internal/herdr/activity.go`:
|
||
- `ToolCall{Name, Kind, Key, Success, IsTest}` normalizes one tool/function
|
||
call across harnesses.
|
||
- `ClaudeActivity` pairs `tool_use`/`tool_result` blocks by `tool_use_id`
|
||
from the transcript already opened for occupancy (an unresolved
|
||
tool_use — mid-turn — is dropped, not reported).
|
||
- `CodexActivity` — **verified against a real live rollout, 2026-07-28**,
|
||
after the original guessed `function_call`/`function_call_output` shape
|
||
turned out not to exist anywhere in `~/.codex/sessions`. Real shape:
|
||
file edits arrive as `event_msg`/`patch_apply_end` (has `changes` +
|
||
top-level `success` directly, no pairing needed); shell commands arrive
|
||
as a freeform `response_item`/`custom_tool_call` named `"exec"` whose
|
||
`input` is a JS snippet — `codexExecCommand` regex-extracts the first
|
||
embedded `cmd:"..."`; failure is signaled by the literal output prefix
|
||
`"Script error:"` (confirmed for both a JS syntax error and an
|
||
`apply_patch` verification failure on this machine's real transcripts).
|
||
Manually re-ran the rewritten parser against a real multi-hundred-line
|
||
rollout end-to-end and spot-checked the output by eye, in addition to
|
||
`TestCodexActivityParsesRealRolloutShape` /
|
||
`TestCodexActivityMarksScriptErrorAsFailure`.
|
||
- `OpenCodeActivity` **refuses outright** — opencode's on-disk message
|
||
storage is only confirmed to carry aggregate token counts, not
|
||
per-tool-call records; fabricating a parser against an unconfirmed shape
|
||
would repeat the exact mistake this audit exists to catch.
|
||
- `DetectThrash(calls, ThrashConfig)` implements all three rules: N
|
||
consecutive failed test runs (regex-matched build/test commands), the
|
||
same file edited M times (edit tools only, explicitly excluding `Read` —
|
||
caught by an initially-failing test), and an identical tool call
|
||
repeated K times back-to-back (excluding test re-runs and file reads —
|
||
also caught by an initially-failing test). Defaults 3/5/4, overridable
|
||
via `Coordinator.Thrash`.
|
||
- `DetectMilestone(calls)` — deliberately narrow: only "the most recent
|
||
call was a successful `git commit`." Fuzzier definitions (passing test
|
||
suite, finished subtask) were not guessed at.
|
||
- `CLIAdapter.Activity` resolves the session file the same way `Occupancy`
|
||
does; `ActivityReader` is an optional Face-B capability like the others.
|
||
- New `ReasonedHandoffRequester`/`CLIAdapter.RequestHandoffReason` names
|
||
*why* (thrash's dead ends, milestone reasoning) instead of the generic
|
||
threshold framing.
|
||
- `rotate()`/`TurnDecision` generalize "reason=manual bypasses occupancy"
|
||
to `manual`/`milestone`/`thrash` alike: `checkActivityTriggers` runs
|
||
before falling through to occupancy/soft/hard (thrash takes priority
|
||
over milestone), and a hit calls `requestReasonedHandoff` — request
|
||
only, never release, same as the soft-threshold path.
|
||
|
||
Covered by `internal/herdr/activity_test.go` and
|
||
`internal/orchestrator/rotation_test.go`'s
|
||
`TestActivityTriggersRequestReasonedHandoffWithoutReleasing`,
|
||
`TestTurnDecision` subtests for soft-threshold and manual-reason, and
|
||
`TestRotationRequestsHandoffBeforeReleasing`.
|
||
|
||
**Known remaining limitation:** `DetectMilestone`'s single-command
|
||
extraction only sees the *first* `cmd:"..."` in a chained Codex exec script,
|
||
so a `git commit` issued later in the same script isn't recognized as the
|
||
"last call" even though it executed last. Opencode still has no verified
|
||
per-tool-call source at all.
|
||
|
||
### Also closed this pass (not separately numbered in the original audit)
|
||
|
||
- **Codex/opencode Stop-hook-equivalent scripts.** Neither harness has a
|
||
native Stop hook, so the server-side `/v1/harness/turn` and
|
||
`/v1/harness/complete` (harness dispatch) existed with no caller for
|
||
either. Added `deploy/hooks/orchestra-codex-poll.sh` and
|
||
`orchestra-opencode-poll.sh` — background poll loops (default 60s,
|
||
`ORCHESTRA_POLL_INTERVAL`) that find the newest session-state file (codex:
|
||
newest `rollout-*.jsonl`; opencode: newest file under
|
||
`~/.local/share/opencode/storage/message/`), POST to `/complete` when
|
||
`.orchestra-report.md` exists, otherwise POST to `/turn` and log (not act
|
||
on) `refuse`/`rotate_now` — advisory-only, since there's no real turn
|
||
boundary to refuse *at* from outside either harness process without deeper
|
||
app-server/SSE integration.
|
||
- **Bus-level authorization.** `authz.AuthorizeEvent` enforced inside
|
||
`store.Append` itself (the single choke point every event passes
|
||
through), not just at HTTP handlers. Event schema bumped to v2, requiring
|
||
every event to declare a `Surface`; schema v1 events on disk still replay
|
||
(tolerant reader).
|
||
- **Dual quota windows.** `router.QuotaAvailability` tracks a 5-hour rolling
|
||
window and a 7-day weekly window independently per harness, applying the
|
||
conservative 80% rule to each separately.
|
||
- **Turn-boundary detection made observable.** An adapter that fails to
|
||
answer `TurnBoundary` now blocks that tick's release rather than treating
|
||
the failure as "safe to proceed," and increments
|
||
`MonitorHealth.TurnBoundaryDegraded`.
|
||
- **Cross-machine lease correctness has a primitive-level test.**
|
||
`TestCrossMachineLeaseAnchorAndQuotaArePerHost` exercises a lease claimed
|
||
through the federation worker HTTP API, validates the anchor against that
|
||
worker's own local checkout, and asserts quota is accounted per-host —
|
||
proof for the primitives that exist today; does not yet run against two
|
||
real physical machines (see federation fork, below).
|
||
- **Fuzz coverage** (`internal/domain/fuzz_test.go`,
|
||
`FuzzValidatePayload`/`FuzzValidateEvent`) for all event types including
|
||
malformed nested payloads — no panic, always a typed error.
|
||
- **Multi-repo Gitea ingestion.** `provider.Gitea` gained a `Project` field
|
||
and namespaced `SourceName()`; `provider.MultiGitea` dispatches by
|
||
`task.Source`; `LoadGiteaConfigs` loads a JSON array of per-project
|
||
sources. Legacy single-repo env vars still work unchanged. Previously zero
|
||
tests existed for the Gitea provider at all (an earlier progress.md claim
|
||
of coverage was inaccurate) — `internal/provider/gitea_test.go` added.
|
||
- **Per-project repos.** `registry.Project` gained optional `repo`/
|
||
`worktree_root`; `main.go` builds a `PerProjectGitWorktrees`, falling back
|
||
to the global default for any project that omits these fields.
|
||
- **Rotation's `TaskReleased` payload validity.** `Coordinator.rotate` used
|
||
to build `{"handoff_ref","reason"}`, omitting the `anchor_sha` the spec and
|
||
`domain.ValidatePayload` require whenever `handoff_ref` is present —
|
||
`store.Append` would reject it, the error was discarded
|
||
(`if c.Store.Append(e) == nil`), and rotation silently never happened, no
|
||
visible failure. Fixed: `herdr.HeadSHA(worktree)` populates `anchor_sha`
|
||
from the real worktree HEAD before appending; if the anchor can't be read,
|
||
rotation now skips that tick instead of emitting a payload guaranteed to
|
||
fail validation. The federation worker release endpoint
|
||
(`/v1/federation/workers/{id}/release`) had the identical gap and now
|
||
requires/forwards a 40-hex-char `anchor_sha`, rejecting with 400
|
||
otherwise.
|
||
|
||
---
|
||
|
||
## The federation fork — decided 2026-07-27, still the deferred half
|
||
|
||
Two incompatible federation designs coexist in the tree.
|
||
|
||
**Design A — "drive the remote socket" (currently deployed,
|
||
`clients/herdr-bridge.go`)**: homesrv calls `worktree.create`/`agent.start`/
|
||
etc. directly on workpc's herdr over TCP as if it were local — meaning
|
||
anchor validation (`git rev-parse HEAD`) executes on the *wrong machine*
|
||
relative to the actual checkout. Two outcomes if the coordinator's
|
||
`session.Worktree` path happens to also exist on homesrv (likely, since
|
||
every project shares the same directory layout): either `HeadSHA` errors and
|
||
rotation silently skips forever, or — the dangerous case — it returns
|
||
**homesrv's HEAD for an unrelated checkout**, passing validation while
|
||
certifying a commit the agent never touched. Same class of bug applies to
|
||
per-host quota accounting and to `cleanupCompleted`'s `git worktree remove`,
|
||
which runs on homesrv for a worktree that lives on workpc.
|
||
|
||
**Design B — "workers pull tasks" (`/v1/federation/*`)**: fully built
|
||
server-side (registration, heartbeat/TTL offline detection, event-cursor
|
||
polling/ack, lease claim), zero clients — no worker binary exists anywhere
|
||
in this repo. This is what the spec actually describes (§2.1: "everything
|
||
crossing a machine boundary is git + a validated artifact, never live state
|
||
over the wire"), but every endpoint is currently unreachable in the real
|
||
deployment. Latent, never-surfaced defects in this unused half: offline
|
||
detection only runs inside `Snapshot()`, called solely from a GET endpoint —
|
||
nothing ticks it on its own, so `OnOffline` (the hook that releases leases
|
||
held by a vanished worker) only fires if a human hits that endpoint; the
|
||
registry is in-memory with no persistence, so a restart forgets all workers
|
||
and cursors.
|
||
|
||
**Decision, unchanged:** keep Design A through Phase 5 (single-host concerns
|
||
— occupancy, Face B, rotation, continuity — are provable on homesrv alone
|
||
with workpc's herdr as just another pane host); commit to Design B in Phase
|
||
6. Two guardrails were meant to land immediately so Design A can't corrupt
|
||
state in the meantime: (1) refuse to rotate a lease held by a non-local
|
||
herdr rather than validate against the wrong checkout, since the protocol
|
||
schema can't run `rev-parse` where the checkout is; (2) same treatment for
|
||
`cleanupCompleted`'s worktree removal. **Status of these two guardrails is
|
||
unverified in this pass** — re-check `internal/orchestrator` before assuming
|
||
they landed; they are not confirmed closed above the way B1–B11/S1–S11 are.
|
||
|
||
If the worker binary is ever abandoned, delete `/v1/federation/*` and record
|
||
the deviation — leaving both designs in place unmarked is explicitly not
|
||
acceptable; the repo would read as though the spec-conformant path is
|
||
implemented when nothing can reach it.
|
||
|
||
---
|
||
|
||
## Live deployment facts (as of 2026-07-28)
|
||
|
||
- Runs as `orchestra.service` on **homesrv** via this machine's own systemd.
|
||
`homesrv` has no local herdr running (connection refused on 9245) — only
|
||
`workpc`'s herdr (`192.168.1.105:9245`) is live and reachable. `main.go`
|
||
only logs herdr connection *failures* at startup, never successes, so "no
|
||
log line" for a herdr does not mean it's down.
|
||
- **Stuck task, deliberately left untouched:** workspace `wA`, task
|
||
`06FT6CKD9Y98AZRX6X8K3QXFZG`, opencode harness, pane `wA:p1`. From
|
||
Orchestra's own point of view it is no longer "stuck" — it now reads
|
||
`state: "failed"` (retries exhausted, `MaxAttempts` hit before B4's fix
|
||
landed). But the underlying herdr pane is still live and
|
||
`agent_status: "blocked"`, confirmed via a direct `agent.get` probe — the
|
||
router gave up and moved on, but nothing ever released or killed the
|
||
actual agent, confirming the orphaned-pane prediction from B2/B5. Left
|
||
untouched on purpose: user was asked and chose to leave it rather than
|
||
have it cleaned up mid-audit. Do not call `pane.close`/`pane.release_agent`
|
||
against it without asking again first.
|
||
- First live end-to-end task run (2026-07-28, after rebuilding/redeploying —
|
||
the running service had been on stale pre-audit commit `325c684`) is what
|
||
surfaced **B12** above, since fixed and confirmed live; re-verifying it
|
||
surfaced **B13**, still open.
|
||
- **Three more live orphaned panes from this pass, left untouched:** `wD:p1`
|
||
(task `06FTAKSJZTB73FZQE3QT7XQ1J0`, worktree
|
||
`/tmp/test-e2e-worktrees/06FTAKSJZTB73FZQE3QT7XQ1J0` — has a real attached
|
||
`claude` session but the task itself is stuck `blocked` from before B12's
|
||
fix landed), `wE:p1` and `wF:p1` (tasks `06FTAMFKVEPGPKR0H86C08ZZNG` /
|
||
`06FTANAYZPA55BABR2J4MWQ050` — empty shells, no agent ever attached, see
|
||
B13). Same rule as `wA:p1`: don't `pane.close`/`pane.release_agent` any of
|
||
these without asking first.
|
||
|
||
---
|
||
|
||
## Known open gaps (as of 2026-07-28)
|
||
|
||
Numbered defects are not repeated here — B19, B20, B21, S12 and S13 are in
|
||
"Found while implementing B18" above, and the ordering across all of them is
|
||
in "What's next". This list is the unnumbered residue: conditions that are
|
||
known, accepted, or not actionable as a single fix.
|
||
|
||
- **B18's env change is applied repo-side only.** `.orchestra-config/
|
||
orchestra.env` now sets `ORCHESTRA_WEB_TOKEN`, but the unit loads
|
||
`/etc/orchestra/orchestra.env`, which is not readable or writable from
|
||
this sandbox. See "What's next" item 1.
|
||
- **The listener still binds all interfaces**; loopback-by-default was
|
||
considered and not taken (B18).
|
||
- ~~**`TestAssignsByAffinityCapabilityAndConcurrency` is flaky**~~ —
|
||
diagnosed and fixed 2026-07-28. The flake was in the test, not in
|
||
assignment. `Store.Tasks()` ranges a map, so its order is randomized per
|
||
call, and the assertion indexed **two separate `Tasks()` calls**
|
||
(`s.Tasks()[0] ... && s.Tasks()[1] ...`); it failed whenever the two
|
||
orderings disagreed. Instrumenting the failure showed a valid `TaskLeased`
|
||
event and a genuinely leased task on every "failing" run. It now snapshots
|
||
once and asserts the real invariant — concurrency 1 means exactly one of
|
||
the two tasks is leased — and passes at `-count=60`. Worth recording that
|
||
*which* of two equal-priority tasks wins is still nondeterministic, since
|
||
`sort.SliceStable` is applied to a randomly ordered slice; that is a real
|
||
property of the router, not a test artifact.
|
||
- **B14/B15/B16 are code-fixed but not re-verified live**, and B17 is closed
|
||
only for tasks that run through a worker. Each was found live, so a code
|
||
fix plus unit tests is weaker evidence than the failure that produced it.
|
||
In particular B16 still lacks the requested two-pane same-harness attach
|
||
contract test, so its actual `agent_name_taken` failure mode is untested.
|
||
- **B15's reconciliation is operator-mediated only.** Blocked tasks keep
|
||
their session and are correctable through the UI, but nothing promotes
|
||
them automatically and the router still never retries a blocked task.
|
||
- **`web/` build output and `internal/webui/assets` can silently diverge**;
|
||
no documented rebuild step ties them together.
|
||
- **Cross-machine lease correctness proven at the primitive level only** —
|
||
needs an actual homesrv/workpc pair over the real mesh; this repo cannot
|
||
exercise that by itself.
|
||
- **Turn-boundary Face B degrades to occupancy-only** for adapters that
|
||
don't implement it, by design — degradation is observable
|
||
(`MonitorHealth.TurnBoundaryDegraded`) and blocks-on-failure, but whether
|
||
Claude/Codex/opencode's native hooks are wired in a live deployment is a
|
||
deployment-config fact, not provable from source alone.
|
||
- **Quota has no live push producer** — only the post-hoc
|
||
`/v1/harness/complete` producer exists; see B7's "design consequences" for
|
||
the (unbuilt) Claude statusline / Codex rollout-tail feeds.
|
||
- **Federation guardrails' landed-status is unverified** — see federation
|
||
fork section above.
|
||
- **opencode has no verified per-tool-call activity source** and no
|
||
first-class quota surface (Zen headers only, would need a plugin) —
|
||
both named, not attempted, in S11/B7.
|
||
- **`DetectMilestone`'s single-command extraction** can miss a Codex commit
|
||
issued later in a chained exec script.
|
||
|
||
Everything else audited (provider layer, continuity schema/validation,
|
||
router matching, delivery, federation registration primitives) was
|
||
spot-checked against the code and its tests and matched described behavior.
|
||
|
||
---
|
||
|
||
## Handoff provenance correction (2026-07-28)
|
||
|
||
The previous rotation path had a **primacy inversion**: the harness wrote a
|
||
free-form Markdown “semantic report”, while `canonicalHandoff` fabricated
|
||
the canonical fields around it (`goal`, `done_when`, a circular action and a
|
||
`cat` command) and placed the entire report in one `remaining` element. That
|
||
made the apparent schema a wrapper around prose rather than an authoritative
|
||
handoff.
|
||
|
||
The release path now asks the harness for only six labelled, bounded answers:
|
||
`NEXT`, `WHY`, `REMAINING`, `DEAD ENDS`, `OPEN Q`, and `LEARNED`, each with a
|
||
`NONE` escape. `canonicalHandoff` parses those answers and derives only
|
||
worker-owned facts (Git anchor, dirty state, metadata, and last observed
|
||
harness command). It never invents task intent. `goal` and `done_when` were
|
||
removed from `continuity.Handoff`; pickup must obtain task scope and success
|
||
criteria from immutable `TASK.md`.
|
||
|
||
`continuity.Handoff.Validate` is the shared semantic gate for both release
|
||
and pickup. Strict decoding rejects removed/unknown fields, and validation
|
||
rejects circular handoff actions, commands that point at report/handoff
|
||
files, oversize or Markdown-smuggled authored list items, and malformed dead
|
||
ends. A failed release remains refused and is retried by the existing
|
||
rotation loop; a bad artifact that somehow reaches CAS is also refused at
|
||
pickup. The fuzzy “empty dead ends after a non-trivial later rotation” signal
|
||
is intentionally not yet enforced: no reliable diff-size/rotation-index
|
||
evidence is available at this validation boundary, so inventing a hard rule
|
||
would create false refusals.
|
||
|
||
Verified after the change with `go test ./...`, `go build ./...`, `go vet
|
||
./...`, and `git diff --check`.
|