From 1ff0af2e69ff4b560d878fb7844d2bdd22ce1e31 Mon Sep 17 00:00:00 2001 From: kami Date: Thu, 30 Jul 2026 01:30:59 +0400 Subject: [PATCH] fix: make worker handoff rotation durable --- AUDIT.md | 1419 ++--------------------- cmd/orchestra-worker/main.go | 609 ++++++++-- cmd/orchestra/main.go | 160 ++- deploy/orchestra.env.example | 18 +- internal/continuity/continuity.go | 50 +- internal/continuity/continuity_test.go | 48 + internal/domain/domain.go | 131 ++- internal/federation/client.go | 24 +- internal/herdr/adapter.go | 198 ++-- internal/herdr/herdr.go | 4 + internal/herdr/occupancy.go | 56 + internal/orchestrator/orchestrator.go | 36 +- internal/orchestrator/rotation_state.go | 82 ++ internal/orchestrator/rotation_test.go | 42 +- internal/store/store.go | 64 + internal/store/store_test.go | 113 ++ 16 files changed, 1488 insertions(+), 1566 deletions(-) create mode 100644 internal/orchestrator/rotation_state.go diff --git a/AUDIT.md b/AUDIT.md index c720a2e..a6464ec 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -1,1337 +1,82 @@ -# 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-29) - -The core code and targeted tests are substantially ahead of the deployed -operator surface. The live API is reachable over HTTP at `orchestra.kvmx.ru` -(HTTPS currently returns nginx `502`), but its historical `test-e2e` backlog -is not an operationally useful control plane yet. - -### Live evidence, checked 2026-07-29 - -- One `workpc-opencode` worker is heartbeating and reports capacity 1. Its - Unix-socket herdr path is the meaningful reachability signal; legacy - coordinator TCP probes are not. The coordinator now skips remote herdr - protocol probes entirely and logs them as worker-owned; local herdr probes - remain separate. -- The API reports 28 tasks: **22 blocked, 5 completed, 1 failed, and no - queued, leased, approval-pending, or captured sessions**. The visible queue - is therefore historical E2E residue, not current work. -- The API and SPA were rebuilt/redeployed on 2026-07-29. API health and the - browser shell returned `200`; the deployed logout endpoint returns `204`. - This ships the empty-collection fixes that prevent the prior - `sessions.length`/`events` null crashes. A real authenticated browser smoke - is still outstanding. -- The task record for a blocked task and the failed E2E task had no retained - session and no lifecycle events. Every lifecycle control was correctly - disabled, but neither record could explain its state. -- ntfy delivery now has a configured publisher token. A read-only - authenticated account check returned `200`, resolving the prior publish - `403 Forbidden`; no test notification was sent. - -Earlier successful OpenCode approval and worker-flow checks remain useful -historical evidence, but they do not prove the current deployment or a -successful worker-owned handoff/release/pickup. Cross-machine continuity is -still deliberately incomplete; Design A must not operate a non-local -checkout. - -### Operator UI: remaining work - -The current UI should become a diagnosis surface, not a five-column task -catalogue. - -1. ~~**Persist and show a blocked diagnosis.**~~ **Closed in code, - deployment verification outstanding (2026-07-29).** `TaskBlocked` now - carries a validated `block_reason`; coordinator and browser-created blocks - set it, while older events receive a deterministic projection fallback. - The board groups blocked tasks by diagnosis and orders each group oldest - first instead of rendering one generic “Blocked” lane. -2. ~~**Persist last-session and pane evidence.**~~ **Closed in code, - deployment verification outstanding (2026-07-29).** Before worker - completion/release cleanup, the worker captures a `session_evidence` - snapshot (harness, pane, observed pane status, capture/check timestamps, - and source); coordinator blocks record their own check snapshot. The task - projection retains it and blocked cards state the source and timestamp. - Legacy records still say **unknown — legacy record has no retained - blocker/pane evidence**, never implying a live or closed pane. -3. ~~**Make the task page lead with the diagnosis.**~~ **Closed in code, - deployment verification outstanding (2026-07-29).** The task detail now - leads with reason, last activity, pane state, retained evidence, and the - next safe action. Only server-enabled lifecycle actions are shown directly; - unavailable actions are kept in an “Unavailable actions” disclosure. -4. ~~**Separate active work from history.**~~ **Closed in code, - deployment verification outstanding (2026-07-29).** The board defaults to - queued, leased, and blocked work; completed and failed records are in a - History view, with an All view for audits. Search, project filtering, and - an explicit no-live-work state keep historical E2E residue out of normal - operations. -5. **Complete the worker truth model.** Show worker-owned heartbeat, local - herdr reachability, active task/pane, and last error separately from legacy - coordinator probes. -6. **Deploy and verify the browser path.** Ship the null-collection fixes and - session logout endpoint, then record real login, refresh, expiry, board, - task-detail, and approval browser flows. Keep `web/dist` and embedded - assets synchronized as part of that build. - ---- - -## Blocking defects - -The sections below are retained defect provenance and implementation detail. -Their dated deployment claims do not supersede the current verdict above. -Read them when changing the affected path; use the current verdict for the -live state and remaining operator work. - -### 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; worker path live-verified - -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:** satisfied on the worker path. -`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 coordinator no longer accepts a non-local herdr operation; the standing -rule is enforced rather than advisory. - -**Failed QA fixture (2026-07-29):** the bounded disposable task -`06FTSHBPYHQXN8MM849PFA1V6M` reached the worker and wrote its semantic -report, but publication correctly refused it. Its prescribed fields combined -to the canonical action `inspect the marker file — verify worker-owned -handoff construction`; the circular-action guard rejects that word even when -it occurs in the explanatory `WHY` text. This is an invalid QA fixture, not a -release-path regression. - -**Live success (2026-07-29):** after configuring `test-e2e` on the -`workpc-opencode` worker, disposable task `06FTY8ZA45CCZZHQ7H44SMQG14` -completed the full path. Its first pane (`w1B:p1`) created the marker and the -bounded semantic report. The worker, not the harness, published canonical -handoff `5e9e942f269f6d6394896186377e35d596d403bd04a58f0c307818af0499b208`: -its anchor was worker-derived (`git_sha` -`d73b576882b07bac34abaf2beabeacfaa8f79529`, branch -`orchestra/scratch/oc-06fty8za45cczzhq7h44smqg14`) and its semantic action -was `Verify the B17 probe marker file contents. — Verify canonical artifact -construction.` The worker recorded `TaskReleased`, routed a successor lease -with that handoff reference to a new pane (`w1C:p1`), and the successor -verified the marker through the scratch checkpoint. It then completed the -task with report artifact -`9a41950548118a47232747ff57fb7ac109dbdb2e45e0b8a8141d4828a6351f18`. -After `TaskCompleted`, the worker state had no sessions or leases. The -disposable worktree was intentionally retained; pane/session cleanup passed. - -### 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/` 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 further gaps remain: -- **`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.** `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. Captures, -commands, and the worker identity/token binding now persist atomically in -`$ORCHESTRA_DATA/federation-state.json` (mode 0600), so the worker must -re-register with its original token after restart before it can consume a -recovered command. Covered by `TestResolvedCommandsArePrunedButPendingOnesSurvive` -and `TestPendingApprovalSurvivesRegistryRestart`. - -### 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. **Address the Web UI backlog above**, starting with queue explanations, - actionable worker health, and a usable approval/recovery workflow. -2. **Record a browser smoke.** Confirm login, refresh, invalid token, and - expired-session behavior in an actual browser after the deployed assets - settle. - -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`). - -Covered by `TestHarnessCompletionBuildsReceiptAndQuotaFromTranscript`, which -exercises authentication, transcript-derived receipts, `TaskCompleted`, and -the matching `QuotaReported` event through the actual HTTP handler. - -### 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/` 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). - -The coordinator no longer supports a herdr-hosted `WorktreeCreator` path. -It always creates the local, committed `TASK.md` before leasing a local pane; -remote worktrees are exclusively worker-owned, so coordinator pickup -validation never runs against a remote filesystem path. - -### 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" (`clients/herdr-bridge.go`)** remains -as a legacy byte proxy but is no longer a coordinator execution path. A -multi-machine registry now requires `ORCHESTRA_MACHINE_ID`; the coordinator -rejects every non-local herdr before leasing, capture, approval, rotation, -release, expiry cleanup, or worktree creation. It always creates the local -committed `TASK.md`; remote panes/checkouts are worker-owned. This prevents -the old wrong-host `git rev-parse`, anchor validation, and cleanup failures. - -**Design B — "workers pull tasks" (`/v1/federation/*`)** is the sole remote -execution path. `cmd/orchestra-worker` registers, heartbeats, polls/acks -leases, and performs Git/herdr operations on the owning machine; coordinator -state for captures and pending approvals is durable across restart. Worker -offline detection is ticked by the API process rather than depending on a -human health request. - -**Decision, now enforced:** federation workers are the sole remote execution -path. The coordinator-side Design A operations are fail-closed rather than -guarded piecemeal; `clients/herdr-bridge.go` should be retired from deployment -once no other consumer needs it. - -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. - -- **Vikunja must become a first-class automatic task source.** The intended - path is `Vikunja task → Orchestra task → agent work → Vikunja update`, not - manual re-entry through the UI. Its provider needs stable external-ID - deduplication, explicit list/project and status/label eligibility mapping, - and guarded completion/blocker write-back that cannot create an ingestion - loop. Treat this alongside Gitea and JSONL ingestion when building the - automatic task-source surface. - -- **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 under their original live - races**, and B17 is closed only for tasks that run through a worker. Each - was found live, so deterministic coverage is weaker evidence than the - failure that produced it; B16's two-same-harness naming contract is now - covered by fake-herdr tests. -- **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`. +# Orchestra audit — handoff first + +Audited 2026-07-30 against the working tree, spec, deployed coordinator, +workpc worker, event log, and live herdr (read-only). + +**Verdict:** one worker handoff completed, but the system is not safe to run +unattended. It can skip rotation, omit Git state, split ownership, strand a +released agent, or reject a valid completion. + +## Evidence + +- `go build ./...`, `go vet ./...`, `go test ./...`: pass. + `go test -race ./...`: fails in orchestrator monitor tests. +- Live B17: release `seq=251`, re-lease `252`, completion `262`; the simple + probe needed six approvals, logged a `409 lease version conflict`, and + recorded `consumed:0`. +- Live now: Docker owns the coordinator; the old systemd unit is inactive. + Workpc runs a dirty `1ca9d64` worker build. No task is active and live herdr + reports no agents. This does not prove the current working tree. + +## P0 — correctness + +| ID | Current failure | Required fix | +|---|---|---| +| H1 | **Closed 2026-07-30.** The checkout-owning worker and coordinator turn path now use `RotationStateMachine`. Workers persist harness-native identity (Claude/Codex transcript, OpenCode SQLite session id), apply soft/milestone/thrash/hard-boundary decisions, and record unknown activity/occupancy/boundary as degraded health rather than zero usage. | Verified by `go test -race ./...`; the existing turn-policy coverage now exercises the shared state machine. | +| H2 | **Closed 2026-07-30.** `PrepareRelease` verifies immutable `TASK.md`, checkpoints all repository work except protocol markers, always pushes the per-task project's scratch anchor, verifies it with `ls-remote`, and only then seals the CAS handoff. | `TestScratchCommitCapturesAllGitStatesExceptProtocolMarkers` covers staged, deleted, renamed, untracked, and protocol-marker cases; release uses the configured project remote. | +| H3 | **Closed 2026-07-30.** Worker state persists idempotent release transactions through `prepared → anchor_pushed → event_committed → pickup_validated → predecessor_retired`. Release/pickup endpoints bind transaction, anchor, and lease version; a predecessor remains mapped and is retired only after matching pickup validation. | `TestReleaseTransactionSurvivesReLeaseUntilMatchingPickup` covers transaction propagation and pickup epoch binding; full race suite passes. | +| H4 | Lease loss can create split-brain work. Worker-offline releases after heartbeat TTL, while the old worker drops release/block mappings without stopping the pane (`cmd/orchestra/main.go:384`; `cmd/orchestra-worker/main.go:592`). Worker health may say herdr is unreachable but routing checks only heartbeat. | Give every lease a durable epoch/fencing token. Accept renew/release/complete only from that owner+epoch. Reassign only after explicit relinquish or lease expiry. On ownership loss, quarantine/stop the old pane before forgetting it. Admit workers only with fresh local-herdr health. | +| H5 | The event log can diverge from memory: `Store.Append` mutates the projection before the event write/fsync (`internal/store/store.go:328-345`). Legal lifecycle transitions are not enforced; the legacy completion endpoint is not lease-owner fenced (`cmd/orchestra/main.go:134-185`). | Validate transition+owner+epoch, append/fsync first, then project. Recover projections only from the log. Make CAS/state writes temp+fsync+rename and fail closed on corrupt worker state. Remove or fence the legacy harness endpoint. | + +## P1 — autonomy and recovery + +- **Recovery:** `TaskBlocked` destroys the worker session needed for late + completion; aggregate-version changes also stale the lease. Use a separate + lease epoch and a recoverable `needs_attention` state that retains ownership + until explicit release, expiry, or reconciled completion. +- **Retries:** expiry bypasses `Router.HandleEvent`; attempts/backoff are + in-memory and unsynchronised. Project durable `attempt`, `next_retry_at`, + and failure class; route every reclaim through one transition. +- **Launch:** repeated start failures hold a lease for up to 30 minutes. + Workers must ACK start or NACK with typed evidence; retry transient failures, + block invalid handoffs, and immediately free unusable capacity. +- **Completion:** `.orchestra/done` is the only worker completion signal. + Combine an explicit completion intent with native idle/exit identity, the + worker-owned quality gate, verified commit, and verified push. +- **Quota:** worker sessions do not retain a usage source, so live receipts + are zero and quota routing is ineffective. Record per-lease deltas and + publish both 5-hour and weekly projections; unknown quota fails closed. +- **Approvals:** the continuity probe required six manual grants. Add audited + per-project policy for safe worktree-local reads, edits, tests, and Git; + keep destructive, secret, network, and out-of-worktree actions gated. +- **Observability:** replace release/rotation `continue` paths with durable + phase, last error, retry time, lease epoch, pane state, and anchor fields. + +## P2 — performance + +- Cache/parallelise health probes; schedule from one task/worker snapshot. + Current routing repeatedly scans tasks and probes candidates per queued task. +- Index active leases and quota windows. Do not scan the whole event log per + availability check or rewrite the full task snapshot after every event. +- Add 1k/10k-task benchmarks with assignment and append p95 budgets. + +## Delivery order + +1. Durable event transitions + lease fencing. +2. Idempotent checkpoint/release/pickup transaction. +3. Worker-local rotation, completion, quota, and typed recovery. +4. Approval policy and performance indexes. +5. Only then: ingestion/UI expansion. + +## Release gate + +- All build/vet/test/race checks pass. +- Fault-inject every handoff phase, coordinator/worker restart, lost response, + worker partition/rejoin, stale completion, and corrupt state file. +- Cross-machine tests cover staged/deleted/clean-committed work and prove the + predecessor remains recoverable until successor pickup validation. +- Live controlled runs pass soft, hard, milestone, thrash, completion, and + late-recovery paths on each harness without manual intervention for safe + repository work. +- Coordinator and workers report the same immutable build revision; staged + worker checksum and Go build revision match before restart. diff --git a/cmd/orchestra-worker/main.go b/cmd/orchestra-worker/main.go index 19539a4..1964a88 100644 --- a/cmd/orchestra-worker/main.go +++ b/cmd/orchestra-worker/main.go @@ -6,8 +6,10 @@ package main import ( "context" "encoding/json" + "errors" "fmt" "log" + "orchestra/internal/buildinfo" "orchestra/internal/continuity" "orchestra/internal/domain" "orchestra/internal/federation" @@ -17,6 +19,7 @@ import ( "os/exec" "os/signal" "path/filepath" + "sort" "strconv" "strings" "syscall" @@ -27,15 +30,19 @@ type worker struct { api federation.Client herdr *herdr.Client harnessID, harness, repo, root, remote string + projects map[string]projectConfig cursor uint64 tasks map[string]domain.Task sessions map[string]herdr.Session leases map[string]lease + releases map[string]releaseTransaction statePath string hard float64 registration federation.Worker lastError string lastErrorAt time.Time + soft float64 + window int64 } func (w *worker) recordError(err error) { @@ -72,13 +79,49 @@ func (w *worker) health(ctx context.Context) federation.WorkerHealth { } type lease struct { - HandoffRef string `json:"handoff_ref,omitempty"` + HandoffRef string `json:"handoff_ref,omitempty"` + TransactionID string `json:"transaction_id,omitempty"` + AnchorSHA string `json:"anchor_sha,omitempty"` + PickupAcknowledged bool `json:"pickup_acknowledged,omitempty"` + Version int `json:"version"` + Until time.Time `json:"until"` +} +type releaseTransaction struct { + ID string `json:"id"` + LeaseVersion int `json:"lease_version"` + Ref string `json:"handoff_ref,omitempty"` + AnchorSHA string `json:"anchor_sha,omitempty"` + Phase string `json:"phase"` // prepared, anchor_pushed, event_committed, pickup_validated, predecessor_retired + AgentReleased bool `json:"agent_released,omitempty"` + LastError string `json:"last_error,omitempty"` + UpdatedAt time.Time `json:"updated_at"` +} +type projectConfig struct { + Repo string `json:"repo"` + Root string `json:"worktree_root"` + Remote string `json:"remote"` + QualityGate string `json:"quality_gate,omitempty"` +} +type completionEvidence struct { + TaskID string `json:"task_id"` + Project string `json:"project"` + Worker string `json:"worker"` + Harness string `json:"harness"` + PaneID string `json:"pane_id"` + BaseSHA string `json:"base_sha"` + ResultSHA string `json:"result_sha"` + Branch string `json:"branch"` + Remote string `json:"remote"` + QualityGate string `json:"quality_gate,omitempty"` + GateExit int `json:"gate_exit"` + CompletedAt time.Time `json:"completed_at"` } type workerState struct { - Cursor uint64 `json:"cursor"` - Sessions map[string]herdr.Session `json:"sessions"` - Tasks map[string]domain.Task `json:"tasks"` - Leases map[string]lease `json:"leases"` + Cursor uint64 `json:"cursor"` + Sessions map[string]herdr.Session `json:"sessions"` + Tasks map[string]domain.Task `json:"tasks"` + Leases map[string]lease `json:"leases"` + Releases map[string]releaseTransaction `json:"releases"` } func (w *worker) load() { @@ -90,6 +133,7 @@ func (w *worker) load() { w.sessions = s.Sessions w.tasks = s.Tasks w.leases = s.Leases + w.releases = s.Releases } } if w.sessions == nil { @@ -101,9 +145,12 @@ func (w *worker) load() { if w.leases == nil { w.leases = map[string]lease{} } + if w.releases == nil { + w.releases = map[string]releaseTransaction{} + } } func (w *worker) save() error { - b, e := json.Marshal(workerState{Cursor: w.cursor, Sessions: w.sessions, Tasks: w.tasks, Leases: w.leases}) + b, e := json.Marshal(workerState{Cursor: w.cursor, Sessions: w.sessions, Tasks: w.tasks, Leases: w.leases, Releases: w.releases}) if e != nil { return e } @@ -133,23 +180,35 @@ func created(e domain.Event) (domain.Task, bool) { Capability []string `json:"capability"` Title string `json:"title"` Description string `json:"description"` + Acceptance []string `json:"acceptance"` + QualityGate string `json:"quality_gate"` } if json.Unmarshal(e.Payload, &p) != nil || p.Source == "" || p.ExternalID == "" || p.Project == "" { return domain.Task{}, false } - return domain.Task{ID: e.TaskID, Source: p.Source, ExternalID: p.ExternalID, Project: p.Project, Capability: p.Capability, Title: p.Title, Description: p.Description}, true + return domain.Task{ID: e.TaskID, Source: p.Source, ExternalID: p.ExternalID, Project: p.Project, Capability: p.Capability, Title: p.Title, Description: p.Description, Acceptance: p.Acceptance, QualityGate: p.QualityGate}, true } -func (w *worker) syncBase(ctx context.Context) error { - if out, err := exec.CommandContext(ctx, "git", "-C", w.repo, "fetch", w.remote, "--prune").CombinedOutput(); err != nil { +func (w *worker) project(t domain.Task) (projectConfig, error) { + if w.projects != nil { + if p, ok := w.projects[t.Project]; ok && p.Repo != "" && p.Root != "" && p.Remote != "" { + return p, nil + } + return projectConfig{}, fmt.Errorf("project %q is not configured on worker", t.Project) + } + return projectConfig{Repo: w.repo, Root: w.root, Remote: w.remote}, nil +} + +func (w *worker) syncBase(ctx context.Context, p projectConfig) error { + if out, err := exec.CommandContext(ctx, "git", "-C", p.Repo, "fetch", p.Remote, "--prune").CombinedOutput(); err != nil { return fmt.Errorf("fetch base checkout: %s: %w", out, err) } - branch, err := exec.CommandContext(ctx, "git", "-C", w.repo, "symbolic-ref", "--quiet", "--short", "HEAD").Output() + branch, err := exec.CommandContext(ctx, "git", "-C", p.Repo, "symbolic-ref", "--quiet", "--short", "HEAD").Output() if err != nil { return fmt.Errorf("identify base branch: %w", err) } branchName := strings.TrimSpace(string(branch)) - if out, err := exec.CommandContext(ctx, "git", "-C", w.repo, "merge", "--ff-only", w.remote+"/"+branchName).CombinedOutput(); err != nil { + if out, err := exec.CommandContext(ctx, "git", "-C", p.Repo, "merge", "--ff-only", p.Remote+"/"+branchName).CombinedOutput(); err != nil { return fmt.Errorf("fast-forward base checkout: %s: %w", out, err) } return nil @@ -159,10 +218,14 @@ func (w *worker) start(ctx context.Context, t domain.Task, ref string) error { var wt string var h continuity.Handoff var err error + p, err := w.project(t) + if err != nil { + return err + } // Synchronize the local base before any worktree operation. A worker never // treats a coordinator-side path as truth; the Git remote is the only // cross-machine transport. - if err := w.syncBase(ctx); err != nil { + if err := w.syncBase(ctx, p); err != nil { return err } if ref != "" { @@ -174,12 +237,12 @@ func (w *worker) start(ctx context.Context, t domain.Task, ref string) error { if err != nil { return err } - if out, err := exec.CommandContext(ctx, "git", "-C", w.repo, "fetch", w.remote, "--prune").CombinedOutput(); err != nil { + if out, err := exec.CommandContext(ctx, "git", "-C", p.Repo, "fetch", p.Remote, "--prune").CombinedOutput(); err != nil { return fmt.Errorf("fetch pickup anchor: %s: %w", out, err) } - wt = filepath.Join(w.root, t.ID) + wt = filepath.Join(p.Root, t.ID) if _, err := os.Stat(wt); os.IsNotExist(err) { - if out, err := exec.CommandContext(ctx, "git", "-C", w.repo, "worktree", "add", "-b", "orchestra/"+t.ID, wt, h.Anchor.GitSHA).CombinedOutput(); err != nil { + if out, err := exec.CommandContext(ctx, "git", "-C", p.Repo, "worktree", "add", "-b", "orchestra/"+t.ID, wt, h.Anchor.GitSHA).CombinedOutput(); err != nil { return fmt.Errorf("create pickup worktree: %s: %w", out, err) } } @@ -187,21 +250,22 @@ func (w *worker) start(ctx context.Context, t domain.Task, ref string) error { return err } } else { - wt, err = (orchestrator.GitWorktrees{Repo: w.repo, Root: w.root}).Create(ctx, t) + wt, err = (orchestrator.GitWorktrees{Repo: p.Repo, Root: p.Root}).Create(ctx, t) if err != nil { return err } } - if _, err = w.herdr.Worktree(ctx, w.repo, wt, "orchestra/"+t.ID); err != nil { + if _, err = w.herdr.Worktree(ctx, p.Repo, wt, "orchestra/"+t.ID); err != nil { return err } s, err := w.herdr.StartAgent(ctx, wt, wt, "orchestra/"+t.ID, w.harness, t.ID) if err != nil { return err } - p := "Begin Orchestra task " + t.ID + ".\nTitle: " + t.Title + "\nInstructions:\n" + t.Description + "\nWork only in this worktree. Do not edit TASK.md." + s.TaskFileSHA = taskHash(t) + prompt := "Read TASK.md at the worktree root and execute it." if ref != "" { - p += "\nA validated handoff exists. Read TASK.md and inspect local git history before continuing." + prompt += " A validated handoff exists; inspect local Git history and the recorded checkpoint before continuing." } w.sessions[t.ID] = s if err := w.save(); err != nil { @@ -209,65 +273,362 @@ func (w *worker) start(ctx context.Context, t domain.Task, ref string) error { } // A prompt response can be lost after herdr accepted it. Persist the // session first so the worker can reconcile/release it after restart. - return w.herdr.Prompt(ctx, s.PaneID, p, 0) + if err := w.herdr.Prompt(ctx, s.PaneID, prompt, 0); err != nil { + return err + } + if ref != "" { + return w.ackPickup(ctx, t.ID, s) + } + return nil } func taskHash(t domain.Task) string { b := continuity.RenderTaskFile(t); return domain.Hash(b) } func (w *worker) releaseReady(ctx context.Context) { for id, s := range w.sessions { - if report, err := os.ReadFile(filepath.Join(s.Worktree, ".orchestra-report.md")); err == nil && len(report) > 0 { - if ref, err := w.api.PutArtifact(ctx, report); err == nil { - if err = w.api.Complete(ctx, id, ref); err == nil { - delete(w.sessions, id) - delete(w.leases, id) - _ = w.save() - continue - } + if l := w.leases[id]; l.HandoffRef != "" && !l.PickupAcknowledged { + if err := w.ackPickup(ctx, id, s); err != nil { + w.recordError(err) + continue + } + } + if _, err := os.Stat(filepath.Join(s.Worktree, ".orchestra", "done")); err == nil { + evidence, err := w.finalize(ctx, id, s) + if err != nil { + w.recordError(fmt.Errorf("complete %s: %w", id, err)) log.Printf("complete %s: %v", id, err) - } else { + continue + } + report, _ := json.Marshal(evidence) + ref, err := w.api.PutArtifact(ctx, report) + if err != nil { + w.recordError(fmt.Errorf("upload completion %s: %w", id, err)) log.Printf("upload completion %s: %v", id, err) + continue } - } - if _, err := os.Stat(filepath.Join(s.Worktree, herdr.HandoffReportFile)); err != nil { - if !s.HandoffRequested { - a := herdr.CLIAdapter{Client: w.herdr, Harness: w.harness} - if occ, occErr := a.Occupancy(s); occErr == nil && occ >= w.hard { - if boundary, boundaryErr := a.AtTurnBoundary(ctx, s); boundaryErr == nil && boundary { - if err := a.RequestHandoff(ctx, s); err == nil { - s.HandoffRequested, s.HandoffReason = true, "threshold" - w.sessions[id] = s - _ = w.save() - } - } - } + if err = w.api.Complete(ctx, id, ref, evidence.ResultSHA, evidence.Branch, evidence.Remote, w.leases[id].Version, w.usageReceipt(s), w.sessionEvidence(ctx, id, s)); err != nil { + w.recordError(fmt.Errorf("complete %s: %w", id, err)) + log.Printf("complete %s: %v", id, err) + continue } + // Completion is durable before closing the exact pane. If close + // fails, retain the session mapping for a later explicit cleanup. + a := herdr.CLIAdapter{Client: w.herdr, Harness: w.harness} + if err := a.Kill(ctx, s); err != nil { + w.recordError(fmt.Errorf("close completed pane %s: %w", id, err)) + log.Printf("close completed pane %s: %v", id, err) + continue + } + _ = os.Remove(filepath.Join(s.Worktree, ".orchestra", "done")) + _ = os.Remove(filepath.Join(s.Worktree, ".orchestra")) + delete(w.sessions, id) + delete(w.leases, id) + _ = w.save() continue } - a := herdr.CLIAdapter{Client: w.herdr, Harness: w.harness, CAS: artifactCAS{w.api}, Remote: w.remote} - ref, err := a.Release(ctx, s) - if err != nil { - log.Printf("release %s: %v", id, err) + if _, err := os.Stat(filepath.Join(s.Worktree, herdr.HandoffReportFile)); err == nil || w.releases[id].ID != "" { + w.advanceRelease(ctx, id, s) continue } - sha, err := herdr.HeadSHA(s.Worktree) - if err != nil { - log.Printf("release %s anchor: %v", id, err) - continue - } - if err = w.api.Release(ctx, id, ref, sha); err != nil { - log.Printf("publish release %s: %v", id, err) - continue - } - // release_agent only removes herdr's binding. Closing the released pane - // after the handoff is durable prevents the next StartAgent from - // inheriting the predecessor's still-running terminal process. - if err := a.Kill(ctx, s); err != nil { - log.Printf("close released pane %s: %v", id, err) - } - delete(w.sessions, id) + w.rotationTick(ctx, id, s) + } +} + +func (w *worker) adapter(s herdr.Session, remote string) herdr.CLIAdapter { + a := herdr.CLIAdapter{Client: w.herdr, Harness: w.harness, Window: w.window, CAS: artifactCAS{w.api}, Remote: remote} + switch w.harness { + case "claude": + a.Usage = herdr.ClaudeUsage + case "codex": + a.Usage = herdr.CodexUsage + case "opencode": + a.Usage = herdr.OpenCodeUsage + } + return a +} + +// rotationTick is the checkout-owner state machine. Occupancy, tool activity, +// and pane status are all read from the persisted harness session identity; +// any unknown source is recorded and never treated as zero usage. +func (w *worker) rotationTick(ctx context.Context, id string, s herdr.Session) { + t, ok := w.tasks[id] + if !ok { + w.recordError(fmt.Errorf("rotation %s: task cache missing", id)) + return + } + p, err := w.project(t) + if err != nil { + w.recordError(err) + return + } + a := w.adapter(s, p.Remote) + resolved, err := a.ResolveSessionIdentity(s) + if err != nil { + w.recordError(fmt.Errorf("rotation %s occupancy degraded: %w", id, err)) + return + } + if resolved != s { + w.sessions[id] = resolved + s = resolved _ = w.save() } + d := (orchestrator.RotationStateMachine{Soft: w.soft, Hard: w.hard}).Evaluate(ctx, a, s) + if d.ActivityDegraded != nil { + w.recordError(fmt.Errorf("rotation %s activity degraded: %w", id, d.ActivityDegraded)) + } + if d.Degraded != nil { + w.recordError(fmt.Errorf("rotation %s degraded: %w", id, d.Degraded)) + if d.Action == orchestrator.TurnContinue || d.Action == "" { + return + } + } + if d.Action == orchestrator.TurnContinue || d.Action == orchestrator.TurnRefuse || s.HandoffRequested { + return + } + if d.Reason == "milestone" || d.Reason == "thrash" { + if err := a.RequestHandoffReason(ctx, s, d.Reason, d.DeadEnds); err != nil { + w.recordError(fmt.Errorf("rotation %s %s prompt: %w", id, d.Reason, err)) + return + } + } else if err := a.RequestHandoff(ctx, s); err != nil { + w.recordError(fmt.Errorf("rotation %s threshold prompt: %w", id, err)) + return + } + s.HandoffRequested, s.HandoffReason = true, d.Reason + w.sessions[id] = s + _ = w.save() +} + +func (w *worker) advanceRelease(ctx context.Context, id string, s herdr.Session) { + t, ok := w.tasks[id] + if !ok { + w.recordError(fmt.Errorf("release %s: task cache missing", id)) + return + } + p, err := w.project(t) + if err != nil { + w.recordError(fmt.Errorf("release %s: %w", id, err)) + return + } + if w.releases == nil { + w.releases = map[string]releaseTransaction{} + } + tx := w.releases[id] + if tx.ID == "" { + l, ok := w.leases[id] + if !ok { + w.recordError(fmt.Errorf("release %s: lease missing", id)) + return + } + tx = releaseTransaction{ID: domain.NewID(), LeaseVersion: l.Version, Phase: "prepared", UpdatedAt: time.Now().UTC()} + w.releases[id] = tx + _ = w.save() + } + a := w.adapter(s, p.Remote) + if tx.Phase == "prepared" { + prepared, err := a.PrepareRelease(ctx, s) + if err != nil { + tx.LastError, tx.UpdatedAt = err.Error(), time.Now().UTC() + w.releases[id] = tx + _ = w.save() + w.recordError(fmt.Errorf("release %s prepare: %w", id, err)) + return + } + tx.Ref, tx.AnchorSHA, tx.Phase, tx.LastError, tx.UpdatedAt = prepared.Ref, prepared.AnchorSHA, "anchor_pushed", "", time.Now().UTC() + w.releases[id] = tx + _ = w.save() + } + if tx.Phase == "anchor_pushed" { + if err := w.api.Release(ctx, id, tx.Ref, tx.AnchorSHA, tx.ID, tx.LeaseVersion, w.sessionEvidence(ctx, id, s)); err != nil { + tx.LastError, tx.UpdatedAt = err.Error(), time.Now().UTC() + w.releases[id] = tx + _ = w.save() + w.recordError(fmt.Errorf("release %s commit: %w", id, err)) + return + } + tx.Phase, tx.LastError, tx.UpdatedAt = "event_committed", "", time.Now().UTC() + w.releases[id] = tx + _ = w.save() + } + if tx.Phase == "event_committed" && !tx.AgentReleased { + if err := a.ReleaseAgent(ctx, s); err != nil { + tx.LastError, tx.UpdatedAt = err.Error(), time.Now().UTC() + w.releases[id] = tx + _ = w.save() + w.recordError(fmt.Errorf("release %s release agent: %w", id, err)) + return + } + tx.AgentReleased, tx.LastError, tx.UpdatedAt = true, "", time.Now().UTC() + w.releases[id] = tx + _ = w.save() + } + if tx.Phase == "pickup_validated" { + if err := a.Kill(ctx, s); err != nil { + tx.LastError, tx.UpdatedAt = err.Error(), time.Now().UTC() + w.releases[id] = tx + _ = w.save() + w.recordError(fmt.Errorf("release %s retire predecessor: %w", id, err)) + return + } + _ = os.Remove(filepath.Join(s.Worktree, herdr.HandoffReportFile)) + tx.Phase, tx.UpdatedAt = "predecessor_retired", time.Now().UTC() + w.releases[id] = tx + _ = w.save() + delete(w.sessions, id) + delete(w.releases, id) + _ = w.save() + } +} + +// ackPickup retries the successor acknowledgement from persisted lease state. +// It is safe after a lost response: the coordinator recognizes the exact +// transaction/lease epoch as an idempotent pickup. +func (w *worker) ackPickup(ctx context.Context, id string, s herdr.Session) error { + l, ok := w.leases[id] + if !ok || l.HandoffRef == "" || l.TransactionID == "" || l.AnchorSHA == "" { + return fmt.Errorf("pickup %s: lease is missing its release transaction", id) + } + if l.PickupAcknowledged { + return nil + } + if err := w.api.Pickup(ctx, id, l.HandoffRef, l.AnchorSHA, l.TransactionID, l.Version, w.sessionEvidence(ctx, id, s)); err != nil { + return fmt.Errorf("pickup %s acknowledgement: %w", id, err) + } + l.PickupAcknowledged = true + l.Version++ // TaskPickupValidated increments the task version. + w.leases[id] = l + return w.save() +} + +func (w *worker) sessionEvidence(ctx context.Context, taskID string, s herdr.Session) domain.SessionEvidence { + e := domain.SessionEvidence{PaneID: s.PaneID, HarnessID: w.harnessID, PaneState: "open", Source: "worker", CheckedAt: time.Now().UTC()} + text, err := (herdr.CLIAdapter{Client: w.herdr, Harness: w.harness}).PaneCapture(ctx, s, "recent") + if err != nil { + e.PaneState = "unreachable" + return e + } + if capture, err := w.api.PublishCapture(ctx, federation.Capture{TaskID: taskID, PaneID: s.PaneID, Text: text}); err == nil { + e.CapturedAt = capture.At + } + return e +} + +func (w *worker) usageReceipt(s herdr.Session) map[string]any { + if s.SessionFile == "" && !(w.harness == "opencode" && s.SessionID != "") { + return map[string]any{"harness_id": w.harnessID, "consumed": 0} + } + var usage herdr.Usage + var err error + switch w.harness { + case "claude": + usage, err = herdr.ClaudeUsage(s.SessionFile) + case "codex": + usage, err = herdr.CodexUsage(s.SessionFile) + case "opencode": + usage, err = herdr.OpenCodeSessionUsage(s.SessionID) + } + if err != nil { + return map[string]any{"harness_id": w.harnessID, "consumed": 0, "error": err.Error()} + } + return map[string]any{"harness_id": w.harnessID, "input_tokens": usage.Input, "cache_read_tokens": usage.CacheRead, "cache_write_tokens": usage.CacheWrite, "output_tokens": usage.Output, "consumed": usage.Numerator()} +} + +func git(ctx context.Context, dir string, args ...string) ([]byte, error) { + return exec.CommandContext(ctx, "git", append([]string{"-C", dir}, args...)...).CombinedOutput() +} + +// finalize performs only mechanical delivery work. It never asks the harness +// to narrate Git state, gates, or a report; those are generated from the +// worker-owned checkout and then verified against the configured remote. +func (w *worker) finalize(ctx context.Context, id string, s herdr.Session) (completionEvidence, error) { + t, ok := w.tasks[id] + if !ok { + return completionEvidence{}, fmt.Errorf("task cache missing") + } + p, err := w.project(t) + if err != nil { + return completionEvidence{}, err + } + if s.TaskFileSHA != "" { + if err := continuity.VerifyTaskFile(s.Worktree, s.TaskFileSHA); err != nil { + return completionEvidence{}, fmt.Errorf("verify immutable TASK.md: %w", err) + } + } + base, err := git(ctx, s.Worktree, "rev-parse", "HEAD") + if err != nil { + return completionEvidence{}, fmt.Errorf("base sha: %s: %w", base, err) + } + e := completionEvidence{TaskID: id, Project: t.Project, Worker: w.harnessID, Harness: w.harness, PaneID: s.PaneID, BaseSHA: strings.TrimSpace(string(base)), Remote: p.Remote, QualityGate: t.QualityGate, CompletedAt: time.Now().UTC()} + gateCommand := t.QualityGate + if gateCommand == "" { + gateCommand = p.QualityGate + } + e.QualityGate = gateCommand + if gateCommand != "" { + gate := exec.CommandContext(ctx, "sh", "-c", gateCommand) + gate.Dir = s.Worktree + if out, err := gate.CombinedOutput(); err != nil { + e.GateExit = 1 + return completionEvidence{}, fmt.Errorf("quality gate %q: %s: %w", gateCommand, out, err) + } + } + if _, err := git(ctx, s.Worktree, "diff", "--quiet", "--", "TASK.md"); err != nil { + return completionEvidence{}, errors.New("TASK.md was modified") + } + if out, err := git(ctx, s.Worktree, "add", "-A", "--", ".", ":!.orchestra/done"); err != nil { + return completionEvidence{}, fmt.Errorf("stage result: %s: %w", out, err) + } + if _, err := git(ctx, s.Worktree, "diff", "--cached", "--quiet"); err != nil { + if out, err := git(ctx, s.Worktree, "commit", "-m", "orchestra: complete "+id); err != nil { + return completionEvidence{}, fmt.Errorf("commit result: %s: %w", out, err) + } + } + branch, err := git(ctx, s.Worktree, "branch", "--show-current") + if err != nil || strings.TrimSpace(string(branch)) == "" { + return completionEvidence{}, fmt.Errorf("result branch: %s: %w", branch, err) + } + e.Branch = strings.TrimSpace(string(branch)) + sha, err := git(ctx, s.Worktree, "rev-parse", "HEAD") + if err != nil { + return completionEvidence{}, fmt.Errorf("result sha: %s: %w", sha, err) + } + e.ResultSHA = strings.TrimSpace(string(sha)) + if out, err := git(ctx, s.Worktree, "push", p.Remote, "HEAD:refs/heads/"+e.Branch); err != nil { + return completionEvidence{}, fmt.Errorf("push result: %s: %w", out, err) + } + remote, err := git(ctx, s.Worktree, "ls-remote", p.Remote, "refs/heads/"+e.Branch) + if err != nil || !strings.HasPrefix(string(remote), e.ResultSHA+"\t") { + return completionEvidence{}, fmt.Errorf("verify pushed sha: got %q: %w", strings.TrimSpace(string(remote)), err) + } + return e, nil +} + +func (w *worker) renewLeases(ctx context.Context) { + if w.herdr == nil { + return + } + now := time.Now() + for taskID, l := range w.leases { + s, ok := w.sessions[taskID] + if !ok || l.Version == 0 || l.Until.After(now.Add(10*time.Minute)) { + continue + } + if _, err := (herdr.CLIAdapter{Client: w.herdr, Harness: w.harness}).PaneCapture(ctx, s, "recent"); err != nil { + w.recordError(fmt.Errorf("validate lease %s: %w", taskID, err)) + continue + } + if err := w.api.Renew(ctx, taskID, l.Version, int((30 * time.Minute).Seconds())); err != nil { + w.recordError(fmt.Errorf("renew lease %s: %w", taskID, err)) + log.Printf("renew lease %s: %v", taskID, err) + } else { + // RenewLease appends one event. Retain that epoch locally until its + // replay arrives so a release transaction uses the same version. + l.Version++ + l.Until = now.Add(30 * time.Minute) + w.leases[taskID] = l + _ = w.save() + } + } } // publishCaptures makes remote panes observable without allowing the @@ -364,16 +725,79 @@ func (w *worker) once(ctx context.Context) error { } if e.Type == "TaskLeased" { var p struct { - HarnessID string `json:"harness_id"` - HandoffRef string `json:"handoff_ref"` + HarnessID string `json:"harness_id"` + HandoffRef string `json:"handoff_ref"` + TransactionID string `json:"transaction_id"` + AnchorSHA string `json:"anchor_sha"` } if json.Unmarshal(e.Payload, &p) == nil && p.HarnessID == w.harnessID { - w.leases[e.TaskID] = lease{HandoffRef: p.HandoffRef} + var until struct { + UntilNS int64 `json:"until_ns"` + } + _ = json.Unmarshal(e.Payload, &until) + w.leases[e.TaskID] = lease{HandoffRef: p.HandoffRef, TransactionID: p.TransactionID, AnchorSHA: p.AnchorSHA, Version: e.Version, Until: time.Unix(0, until.UntilNS)} } } - if e.Type == "TaskReleased" || e.Type == "TaskCompleted" || e.Type == "TaskFailed" || e.Type == "TaskBlocked" { + if e.Type == "TaskLeaseRenewed" { + var p struct { + HarnessID string `json:"harness_id"` + UntilNS int64 `json:"until_ns"` + } + if json.Unmarshal(e.Payload, &p) == nil && p.HarnessID == w.harnessID { + l := w.leases[e.TaskID] + l.Version, l.Until = e.Version, time.Unix(0, p.UntilNS) + w.leases[e.TaskID] = l + } + } + if e.Type == "TaskCompleted" { delete(w.leases, e.TaskID) - delete(w.sessions, e.TaskID) + if session, active := w.sessions[e.TaskID]; active { + if w.herdr == nil { + delete(w.sessions, e.TaskID) + } else if err := (herdr.CLIAdapter{Client: w.herdr, Harness: w.harness}).Kill(ctx, session); err != nil { + w.recordError(fmt.Errorf("close completed pane %s: %w", e.TaskID, err)) + continue + } else { + _ = os.Remove(filepath.Join(session.Worktree, ".orchestra", "done")) + _ = os.Remove(filepath.Join(session.Worktree, ".orchestra")) + delete(w.sessions, e.TaskID) + } + } + } + if e.Type == "TaskPickupValidated" { + var p struct { + TransactionID string `json:"transaction_id"` + } + if json.Unmarshal(e.Payload, &p) == nil { + if tx := w.releases[e.TaskID]; tx.ID != "" && tx.ID == p.TransactionID { + tx.Phase, tx.UpdatedAt = "pickup_validated", time.Now().UTC() + w.releases[e.TaskID] = tx + } + } + if l, ok := w.leases[e.TaskID]; ok { + l.Version = e.Version + w.leases[e.TaskID] = l + } + } + if e.Type == "TaskReleased" || e.Type == "TaskFailed" || e.Type == "TaskBlocked" { + if e.Type == "TaskReleased" { + var p struct { + TransactionID string `json:"transaction_id"` + } + if json.Unmarshal(e.Payload, &p) == nil { + if tx := w.releases[e.TaskID]; tx.ID != "" && tx.ID == p.TransactionID && tx.Phase == "anchor_pushed" { + tx.Phase, tx.LastError, tx.UpdatedAt = "event_committed", "", time.Now().UTC() + w.releases[e.TaskID] = tx + } + } + } + delete(w.leases, e.TaskID) + // A releasing predecessor remains intentionally recoverable until + // TaskPickupValidated for its transaction. Do not erase its pane + // mapping merely because our own release event was replayed. + if _, releasing := w.releases[e.TaskID]; !releasing { + delete(w.sessions, e.TaskID) + } } if e.Seq > w.cursor { w.cursor = e.Seq @@ -422,6 +846,7 @@ func (w *worker) once(ctx context.Context) error { if w.herdr != nil { w.publishCaptures(ctx) w.runCommands(ctx) + w.renewLeases(ctx) } w.releaseReady(ctx) if err := w.save(); err != nil { @@ -439,7 +864,7 @@ func (w *worker) reconcileLeases(ctx context.Context) error { for _, task := range tasks { w.tasks[task.ID] = task if task.State == domain.StateLeased && task.Lease != nil && task.Lease.HarnessID == w.harnessID { - active[task.ID] = lease{HandoffRef: task.HandoffRef} + active[task.ID] = lease{HandoffRef: task.HandoffRef, TransactionID: task.ReleaseTransaction, AnchorSHA: task.ReleaseAnchor, Version: task.Version, Until: task.Lease.Until} } } for taskID := range w.leases { @@ -480,7 +905,53 @@ func main() { if v, err := strconv.ParseFloat(os.Getenv("ORCHESTRA_OCCUPANCY_HARD"), 64); err == nil && v > 0 && v < 1 { hard = v } - w := &worker{api: federation.Client{BaseURL: required("ORCHESTRA_URL"), WorkerID: id, Token: token, AdmitToken: os.Getenv("ORCHESTRA_FEDERATION_ADMIT_TOKEN")}, harnessID: required("ORCHESTRA_WORKER_HERDR_ID"), harness: required("ORCHESTRA_WORKER_HARNESS"), repo: required("ORCHESTRA_REPO"), root: required("ORCHESTRA_WORKTREE_ROOT"), remote: required("ORCHESTRA_GIT_REMOTE"), tasks: map[string]domain.Task{}, sessions: map[string]herdr.Session{}, leases: map[string]lease{}, statePath: os.Getenv("ORCHESTRA_WORKER_STATE"), hard: hard, registration: federation.Worker{ID: id, Address: os.Getenv("ORCHESTRA_WORKER_ADDRESS"), Capacity: 1}} + projects := map[string]projectConfig{} + if path := os.Getenv("ORCHESTRA_WORKER_PROJECT_CONFIG_FILE"); path != "" { + b, err := os.ReadFile(path) + if err != nil { + log.Fatalf("read ORCHESTRA_WORKER_PROJECT_CONFIG_FILE: %v", err) + } + if err := json.Unmarshal(b, &projects); err != nil { + log.Fatalf("parse ORCHESTRA_WORKER_PROJECT_CONFIG_FILE: %v", err) + } + } else { + for _, project := range strings.Split(os.Getenv("ORCHESTRA_WORKER_PROJECTS"), ",") { + if project = strings.TrimSpace(project); project != "" { + projects[project] = projectConfig{Repo: required("ORCHESTRA_REPO"), Root: required("ORCHESTRA_WORKTREE_ROOT"), Remote: required("ORCHESTRA_GIT_REMOTE")} + } + } + } + if len(projects) == 0 { + // Existing deployments may be upgraded before their protected systemd + // environment is amended. Stay observable and fail closed in that + // interval: an empty declaration makes this worker ineligible for all + // new leases instead of turning a configuration rollout into a crash + // loop or treating its legacy global checkout as every project. + log.Printf("no ORCHESTRA_WORKER_PROJECT_CONFIG_FILE/ORCHESTRA_WORKER_PROJECTS; registering with no supported projects") + } + for project, config := range projects { + if config.Repo == "" || config.Root == "" || config.Remote == "" { + log.Fatalf("project %q requires repo, worktree_root, and remote", project) + } + } + supported := make([]string, 0, len(projects)) + for project := range projects { + supported = append(supported, project) + } + sort.Strings(supported) + repo, root, remote := required("ORCHESTRA_REPO"), required("ORCHESTRA_WORKTREE_ROOT"), required("ORCHESTRA_GIT_REMOTE") + if len(supported) > 0 { + first := projects[supported[0]] + repo, root, remote = first.Repo, first.Root, first.Remote + } + soft, window := .55, int64(200000) + if v, err := strconv.ParseFloat(os.Getenv("ORCHESTRA_OCCUPANCY_SOFT"), 64); err == nil && v > 0 && v < hard { + soft = v + } + if v, err := strconv.ParseInt(os.Getenv("ORCHESTRA_CONTEXT_WINDOW"), 10, 64); err == nil && v > 0 { + window = v + } + w := &worker{api: federation.Client{BaseURL: required("ORCHESTRA_URL"), WorkerID: id, Token: token, AdmitToken: os.Getenv("ORCHESTRA_FEDERATION_ADMIT_TOKEN")}, harnessID: required("ORCHESTRA_WORKER_HERDR_ID"), harness: required("ORCHESTRA_WORKER_HARNESS"), repo: repo, root: root, remote: remote, projects: projects, tasks: map[string]domain.Task{}, sessions: map[string]herdr.Session{}, leases: map[string]lease{}, releases: map[string]releaseTransaction{}, statePath: os.Getenv("ORCHESTRA_WORKER_STATE"), hard: hard, soft: soft, window: window, registration: federation.Worker{ID: id, Address: os.Getenv("ORCHESTRA_WORKER_ADDRESS"), Capacity: 1, SupportedProjects: supported, Build: buildinfo.Current()}} if w.statePath == "" { w.statePath = filepath.Join(w.root, ".orchestra-worker-state.json") } diff --git a/cmd/orchestra/main.go b/cmd/orchestra/main.go index 4b02e7c..73c4210 100644 --- a/cmd/orchestra/main.go +++ b/cmd/orchestra/main.go @@ -2,7 +2,6 @@ package main import ( "context" - "crypto/subtle" "encoding/json" "errors" "fmt" @@ -12,6 +11,7 @@ import ( "net/http" "orchestra/internal/admin" "orchestra/internal/authz" + "orchestra/internal/buildinfo" "orchestra/internal/delivery" "orchestra/internal/domain" "orchestra/internal/federation" @@ -83,6 +83,14 @@ func remoteHerdrAddresses(rr registry.Registry, localMachine string) map[string] return remote } +// coordinatorOwnsHerdr identifies the only herdr sockets the coordinator may +// probe or adapt. In federation mode a remote pane belongs to its worker; +// reaching into that machine would turn a worker-owned health signal back +// into a misleading coordinator TCP result. +func coordinatorOwnsHerdr(h registry.Herdr, localMachine string) bool { + return localMachine == "" || h.MachineID == localMachine +} + func (a federatedAvailability) Available(h registry.Herdr) bool { if a.base != nil && !a.base.Available(h) { return false @@ -93,6 +101,16 @@ func (a federatedAvailability) Available(h registry.Herdr) bool { return a.workers.Available(h.ID) } +func (a federatedAvailability) Supports(h registry.Herdr, project string) bool { + // Locally-owned herdrs keep their static registry/project affinity. A + // remote worker must additionally prove it has a local checkout for the + // project before the router can offer it a lease. + if a.localMachine == "" || h.MachineID == a.localMachine { + return true + } + return a.workers.Supports(h.ID, project) +} + func validateLocalMachine(rr registry.Registry, localMachine string) error { machines := rr.Machines() if len(machines) <= 1 { @@ -231,6 +249,10 @@ func main() { if repo, root := os.Getenv("ORCHESTRA_REPO"), os.Getenv("ORCHESTRA_WORKTREE_ROOT"); repo != "" && root != "" { adapters := map[string]herdr.Adapter{} for _, h := range rr.Herdrs() { + if !coordinatorOwnsHerdr(h, localMachine) { + log.Printf("herdr %s is worker-owned on %s; coordinator probe skipped", h.ID, h.MachineID) + continue + } address := herdrAddress(rr, h) if address == "" { continue @@ -244,6 +266,7 @@ func main() { log.Printf("herdr %s unavailable: %v", h.ID, err) continue } + log.Printf("herdr %s reachable at %s (protocol %s, harness %s)", h.ID, address, protocol, h.Harness) switch h.Harness { case "claude": adapters[h.ID] = herdr.Claude(client, 200000, s) @@ -267,7 +290,7 @@ func main() { } coordinator = &orchestrator.Coordinator{Store: s, StatePath: filepath.Join(dir, "runtime-sessions.json"), Worktrees: worktrees, Adapters: orchestrator.AdapterFactory{Herdrs: adapters}, LocalHerdr: func(id string) bool { h, ok := rr.Herdr(id) - return ok && (localMachine == "" || h.MachineID == localMachine) + return ok && coordinatorOwnsHerdr(h, localMachine) }} rt.OnLease = func(e domain.Event) error { // In federated mode the coordinator must never inspect a remote @@ -301,17 +324,14 @@ func main() { } mux := http.NewServeMux() // B18: the UI is a full control plane — it can create tasks, release or - // complete them, and inject approval keystrokes into live panes. Refuse - // to serve it unauthenticated rather than silently exposing that on - // whatever interface the listener binds to. - webToken := os.Getenv("ORCHESTRA_WEB_TOKEN") - if webToken == "" { - log.Fatal("ORCHESTRA_WEB_TOKEN must be set: it gates the web UI's task, lifecycle and approval controls") + // complete them, and inject approval keystrokes into live panes. It has + // one explicit operator identity and is never enabled by a missing env var. + webCredentials := authz.WebCredentials{Username: os.Getenv("ORCHESTRA_WEB_USERNAME"), PasswordHash: os.Getenv("ORCHESTRA_WEB_PASSWORD_HASH")} + if err := webCredentials.Validate(); err != nil { + log.Fatalf("web login configuration: %v", err) } sessions := &authz.Sessions{} - // A browser cannot put a Bearer token on a document load, so it trades - // the token once for an HttpOnly cookie. Same credential, presentable - // form; no new authority is created here. + // A browser login exchanges verified credentials for an HttpOnly cookie. mux.HandleFunc("/v1/ui/session", func(w http.ResponseWriter, r *http.Request) { if r.Method == http.MethodDelete { // Expire the browser credential even if it is already absent or stale. @@ -329,15 +349,13 @@ func main() { return } var body struct { - Token string `json:"token"` + Username string `json:"username"` + Password string `json:"password"` } - _ = json.NewDecoder(http.MaxBytesReader(w, r.Body, 4096)).Decode(&body) - supplied := body.Token - if supplied == "" { - supplied = strings.TrimPrefix(r.Header.Get("Authorization"), "Bearer ") - } - if subtle.ConstantTimeCompare([]byte(supplied), []byte(webToken)) != 1 { - http.Error(w, "invalid token", http.StatusUnauthorized) + decoder := json.NewDecoder(http.MaxBytesReader(w, r.Body, 4096)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&body); err != nil || !webCredentials.Authenticate(body.Username, body.Password) { + http.Error(w, "invalid credentials", http.StatusUnauthorized) return } v, err := sessions.Issue() @@ -414,6 +432,17 @@ func main() { http.Error(w, "invalid json", 400) return } + // Make the selected project's deterministic gate part of the immutable + // task contract before any worker can create TASK.md. A caller may + // override it only when it has deliberately supplied a task-specific + // gate; routing never asks a harness to choose one. + if _, set := p["quality_gate"]; !set { + if projectID, _ := p["project"].(string); projectID != "" { + if project, ok := rr.Project(projectID); ok && project.QualityGate != "" { + p["quality_gate"] = project.QualityGate + } + } + } b, _ := json.Marshal(p) e := domain.Event{ID: id(), Type: "TaskCreated", TaskID: id(), Version: 1, Payload: b, Surface: string(surface(r))} if err := s.Append(e); err != nil { @@ -665,7 +694,7 @@ func main() { } json.NewEncoder(w).Encode(out) }) - adminServer := &admin.Server{Store: s, RouterReady: rt != nil, Providers: providerHealth, Probes: map[string]admin.ProbeFunc{ + adminServer := &admin.Server{Store: s, RouterReady: rt != nil, Build: buildinfo.Current(), Providers: providerHealth, Probes: map[string]admin.ProbeFunc{ "router": func() (bool, string) { return rt != nil, "configured router" }, "gitea": func() (bool, string) { configured := os.Getenv("ORCHESTRA_GITEA_URL") != "" || os.Getenv("ORCHESTRA_GITEA_CONFIG") != "" @@ -1033,7 +1062,7 @@ func main() { w.WriteHeader(http.StatusNoContent) }) mux.HandleFunc("/v1/federation/workers/", func(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodPost || (!strings.HasSuffix(r.URL.Path, "/heartbeat") && !strings.HasSuffix(r.URL.Path, "/handoff") && !strings.HasSuffix(r.URL.Path, "/complete") && !strings.HasSuffix(r.URL.Path, "/captures")) { + if r.Method != http.MethodPost || (!strings.HasSuffix(r.URL.Path, "/heartbeat") && !strings.HasSuffix(r.URL.Path, "/renew") && !strings.HasSuffix(r.URL.Path, "/handoff") && !strings.HasSuffix(r.URL.Path, "/complete") && !strings.HasSuffix(r.URL.Path, "/captures")) { http.Error(w, "not found", 404) return } @@ -1083,10 +1112,18 @@ func main() { return } var b struct { - TaskID string `json:"task_id"` - TTLSeconds int `json:"ttl_seconds"` - HandoffRef string `json:"handoff_ref"` - AnchorSHA string `json:"anchor_sha"` + TaskID string `json:"task_id"` + TTLSeconds int `json:"ttl_seconds"` + ExpectedVersion int `json:"expected_version"` + HandoffRef string `json:"handoff_ref"` + AnchorSHA string `json:"anchor_sha"` + TransactionID string `json:"transaction_id"` + LeaseVersion int `json:"lease_version"` + ResultSHA string `json:"result_sha"` + Branch string `json:"branch"` + Remote string `json:"remote"` + Receipt map[string]any `json:"receipt"` + SessionEvidence domain.SessionEvidence `json:"session_evidence"` } if json.NewDecoder(r.Body).Decode(&b) != nil || b.TaskID == "" { http.Error(w, "invalid lease body", 400) @@ -1098,6 +1135,13 @@ func main() { return } ownedLease := t.State == domain.StateLeased && t.Lease != nil && t.Lease.HarnessID == parts[3] + // A response can be lost after the append/fsync. Retrying the exact + // release transaction is therefore a successful no-op, never a second + // TaskReleased event and never a reason to discard the predecessor. + if strings.HasSuffix(r.URL.Path, "/handoff") && t.State == domain.StateQueued && b.TransactionID != "" && b.TransactionID == t.ReleaseTransaction && b.HandoffRef == t.HandoffRef && b.AnchorSHA == t.ReleaseAnchor { + w.WriteHeader(http.StatusNoContent) + return + } // A prompt timeout can block the coordinator after herdr already // accepted the request. If that same authenticated worker later reports // a durable completion, reconcile it rather than preserving a known @@ -1107,17 +1151,73 @@ func main() { http.Error(w, "lease not owned", 409) return } + if strings.HasSuffix(r.URL.Path, "/complete") && b.ExpectedVersion != t.Version { + http.Error(w, "lease version conflict", http.StatusConflict) + return + } + if strings.HasSuffix(r.URL.Path, "/renew") { + ttl := b.TTLSeconds + if ttl == 0 { + ttl = int((30 * time.Minute).Seconds()) + } + e, err := s.RenewLease(b.TaskID, parts[3], b.ExpectedVersion, time.Duration(ttl)*time.Second) + if err != nil { + http.Error(w, err.Error(), http.StatusConflict) + return + } + json.NewEncoder(w).Encode(e) + return + } + if strings.HasSuffix(r.URL.Path, "/pickup") { + if !ownedLease || b.TransactionID == "" || b.TransactionID != t.ReleaseTransaction || b.HandoffRef != t.HandoffRef || b.AnchorSHA != t.ReleaseAnchor { + http.Error(w, "pickup does not match active release", http.StatusConflict) + return + } + if t.PickupTransaction == b.TransactionID && t.PickupLeaseVersion == b.LeaseVersion { + w.WriteHeader(http.StatusNoContent) + return + } + if b.LeaseVersion != t.Version { + http.Error(w, "lease version conflict", http.StatusConflict) + return + } + p, _ := json.Marshal(map[string]any{"transaction_id": b.TransactionID, "handoff_ref": b.HandoffRef, "anchor_sha": b.AnchorSHA, "harness_id": parts[3], "lease_version": b.LeaseVersion, "expected_version": t.Version, "session_evidence": b.SessionEvidence}) + e := domain.Event{ID: id(), Type: "TaskPickupValidated", TaskID: b.TaskID, Version: t.Version + 1, Payload: p, Surface: string(authz.System)} + if err := s.Append(e); err != nil { + http.Error(w, err.Error(), http.StatusConflict) + return + } + json.NewEncoder(w).Encode(e) + return + } if strings.HasSuffix(r.URL.Path, "/complete") { if b.HandoffRef == "" { http.Error(w, "report_ref required", 400) return } - p, _ := json.Marshal(map[string]any{"report_ref": b.HandoffRef, "receipt": map[string]any{"harness_id": parts[3], "consumed": 0}}) + if len(b.ResultSHA) != 40 || b.Branch == "" || b.Remote == "" { + http.Error(w, "verified result_sha, branch, and remote required", 400) + return + } + if b.Receipt == nil { + b.Receipt = map[string]any{} + } + b.Receipt["harness_id"] = parts[3] + if _, ok := b.Receipt["consumed"]; !ok { + b.Receipt["consumed"] = 0 + } + p, _ := json.Marshal(map[string]any{"report_ref": b.HandoffRef, "receipt": b.Receipt, "result_sha": b.ResultSHA, "branch": b.Branch, "remote": b.Remote, "session_evidence": b.SessionEvidence}) e := domain.Event{ID: id(), Type: "TaskCompleted", TaskID: b.TaskID, Version: t.Version + 1, Payload: p, Surface: string(authz.System)} if err := s.Append(e); err != nil { http.Error(w, err.Error(), 409) return } + if consumed, ok := b.Receipt["consumed"].(float64); ok && consumed > 0 { + qp, _ := json.Marshal(map[string]any{"harness_id": parts[3], "consumed": consumed}) + if err := s.Append(domain.Event{ID: id(), Type: "QuotaReported", TaskID: "system", Payload: qp, Surface: string(authz.System)}); err != nil { + log.Printf("federated quota report %s: %v", b.TaskID, err) + } + } json.NewEncoder(w).Encode(e) return } @@ -1129,7 +1229,11 @@ func main() { http.Error(w, "anchor_sha required", 400) return } - p, _ := json.Marshal(map[string]any{"handoff_ref": b.HandoffRef, "harness_id": parts[3], "anchor_sha": b.AnchorSHA}) + if b.TransactionID == "" || b.ExpectedVersion != t.Version { + http.Error(w, "release transaction and current lease version required", http.StatusConflict) + return + } + p, _ := json.Marshal(map[string]any{"handoff_ref": b.HandoffRef, "harness_id": parts[3], "anchor_sha": b.AnchorSHA, "transaction_id": b.TransactionID, "expected_version": t.Version, "session_evidence": b.SessionEvidence}) e := domain.Event{ID: id(), Type: "TaskReleased", TaskID: b.TaskID, Version: t.Version + 1, Payload: p, Surface: string(authz.System)} if err := s.Append(e); err != nil { http.Error(w, err.Error(), 409) @@ -1257,7 +1361,7 @@ func main() { } log.Println("orchestra listening on :" + port) tokens := map[authz.Surface]string{ - authz.TUI: os.Getenv("ORCHESTRA_TUI_TOKEN"), authz.Web: os.Getenv("ORCHESTRA_WEB_TOKEN"), + authz.TUI: os.Getenv("ORCHESTRA_TUI_TOKEN"), authz.MCP: os.Getenv("ORCHESTRA_MCP_TOKEN"), authz.Maven: os.Getenv("ORCHESTRA_MAVEN_TOKEN"), // S12: this is the credential callers present *to* Orchestra on the // ntfy surface. ORCHESTRA_NTFY_TOKEN is a different secret entirely — diff --git a/deploy/orchestra.env.example b/deploy/orchestra.env.example index dc6248f..5943da1 100644 --- a/deploy/orchestra.env.example +++ b/deploy/orchestra.env.example @@ -36,6 +36,14 @@ ORCHESTRA_WORKTREE_ROOT=/var/lib/orchestra/worktrees # Hard rotation occupancy threshold (0 < x < 1). Default 0.75 if unset/invalid. ORCHESTRA_OCCUPANCY_HARD=0.75 +# Advisory handoff threshold and the harness context window used to turn +# per-session token counts into occupancy. Both values are worker-local. +ORCHESTRA_OCCUPANCY_SOFT=0.55 +ORCHESTRA_CONTEXT_WINDOW=200000 +# OpenCode stores per-session token counters in SQLite. This optional override +# must point at the worker-local database; the worker persists the resolved +# session ID for each lease, never "the latest" session. +#ORCHESTRA_OPENCODE_DB=/home/orchestra/.local/share/opencode/opencode.db # --- Providers --- # Local JSONL task ingestion (baseline adapter). @@ -74,10 +82,12 @@ ORCHESTRA_OCCUPANCY_HARD=0.75 # --- Bus authorization tokens (bearer auth per surface; a surface with no # token set has no auth requirement — set these once you have real clients) --- #ORCHESTRA_TUI_TOKEN= -# Required: the service refuses to start without it. It gates the web UI's -# task, lifecycle and approval controls, and it is also the token for any -# /v1/ caller that does not declare a surface (they default to Web). -ORCHESTRA_WEB_TOKEN= +# Required: the service refuses to start without both. The browser UI's +# task, lifecycle and approval controls are session-gated; it no longer +# accepts a shared Web bearer token. Generate the bcrypt hash with: +# go run ./cmd/orchestra-password +ORCHESTRA_WEB_USERNAME=operator +ORCHESTRA_WEB_PASSWORD_HASH= # Set when the UI is served over plain HTTP, so the session cookie can be # sent without Secure. Leave unset behind TLS. #ORCHESTRA_UI_INSECURE_COOKIE=1 diff --git a/internal/continuity/continuity.go b/internal/continuity/continuity.go index a395b5d..c83f671 100644 --- a/internal/continuity/continuity.go +++ b/internal/continuity/continuity.go @@ -40,7 +40,17 @@ func RenderTaskFile(t domain.Task) []byte { if strings.TrimSpace(t.Description) != "" { fmt.Fprintf(&b, "\n## Instructions\n\n%s\n", t.Description) } - b.WriteString("\nThis file is immutable for the lifetime of the task (§6.2) — its hash is\ncarried in every handoff and re-verified on every pickup. Do not edit it.\n") + if len(t.Acceptance) > 0 { + b.WriteString("\n## Acceptance criteria\n") + for _, criterion := range t.Acceptance { + fmt.Fprintf(&b, "\n- %s", criterion) + } + b.WriteByte('\n') + } + if t.QualityGate != "" { + fmt.Fprintf(&b, "\n## Quality gate\n\n%s\n", t.QualityGate) + } + b.WriteString("\n## Completion\n\nRun the configured quality gate. When the task is ready for the worker to verify and deliver, create `.orchestra/done`. Do not write a prose completion report.\n\nThis file is immutable for the lifetime of the task (§6.2) — its hash is\ncarried in every handoff and re-verified on every pickup. Do not edit it.\n") return []byte(b.String()) } @@ -80,8 +90,9 @@ func ConventionsHash(root string) (string, error) { } type Dirty struct { - Path string `json:"path"` - SHA256 string `json:"sha256"` + Path string `json:"path"` + SHA256 string `json:"sha256"` + Deleted bool `json:"deleted,omitempty"` } type Completed struct { What string `json:"what"` @@ -147,7 +158,7 @@ func (h Handoff) Validate() error { } } for _, d := range h.Anchor.Dirty { - if filepath.IsAbs(d.Path) || d.Path == "" || len(d.SHA256) != 64 { + if filepath.IsAbs(d.Path) || d.Path == "" || (!d.Deleted && len(d.SHA256) != 64) { return errors.New("invalid dirty anchor") } } @@ -199,6 +210,12 @@ func ValidatePickup(root string, h Handoff, taskFileSHA string) error { return errors.New("handoff anchor HEAD mismatch") } for _, d := range h.Anchor.Dirty { + if d.Deleted { + if _, e := os.Stat(filepath.Join(root, d.Path)); !errors.Is(e, os.ErrNotExist) { + return fmt.Errorf("handoff deleted file restored: %s", d.Path) + } + continue + } b, e := os.ReadFile(filepath.Join(root, d.Path)) if e != nil { return e @@ -257,7 +274,12 @@ func Load(ref string, cas CAS) (Handoff, error) { return Decode(b) } -// ScratchCommit records WIP atomically on a dedicated branch before rotation. +// ScratchCommit records every piece of repository work except Orchestra's +// ephemeral protocol markers. In particular, git add -A is intentional: it +// includes already-staged changes, deletions, renames, and untracked files. +// TASK.md is checked before touching the index; it is an immutable input, not +// deliverable work. The report/done markers remain local so a successor never +// mistakes an old protocol signal for a new one. func ScratchCommit(root, branch, message string) error { if branch == "" || strings.ContainsAny(branch, " \t\n") { return errors.New("invalid scratch branch") @@ -279,14 +301,20 @@ func ScratchCommit(root, branch, message string) error { return err } } - if err := exec.Command("git", "-C", root, "add", "-A").Run(); err != nil { + // A harness may have staged a protocol marker itself. Remove it from the + // index before staging the real checkpoint; this does not alter its working + // tree contents and makes the exclusion apply to staged state too. + for _, marker := range []string{".orchestra", ".orchestra-handoff.json", ".orchestra-handoff-report.md"} { + if err := exec.Command("git", "-C", root, "reset", "-q", "HEAD", "--", marker).Run(); err != nil { + return err + } + } + if err := exec.Command("git", "-C", root, "add", "-A", "--", ".", ":(exclude)TASK.md", ":(exclude).orchestra", ":(exclude).orchestra-handoff.json", ":(exclude).orchestra-handoff-report.md").Run(); err != nil { return err } - full, err := exec.Command("git", "-C", root, "status", "--porcelain").Output() - if err != nil { - return err - } - if len(full) == 0 { + // Only staged non-protocol work is committed. Remaining marker files are + // expected and must not suppress a clean committed-anchor checkpoint. + if exec.Command("git", "-C", root, "diff", "--cached", "--quiet").Run() == nil { return nil // nothing to snapshot; branch already reflects the worktree } return exec.Command("git", "-C", root, "commit", "-m", message).Run() diff --git a/internal/continuity/continuity_test.go b/internal/continuity/continuity_test.go index b70883d..5585695 100644 --- a/internal/continuity/continuity_test.go +++ b/internal/continuity/continuity_test.go @@ -12,6 +12,54 @@ import ( "orchestra/internal/store" ) +func TestScratchCommitCapturesAllGitStatesExceptProtocolMarkers(t *testing.T) { + repo := t.TempDir() + run := func(args ...string) { + t.Helper() + if out, err := exec.Command("git", append([]string{"-C", repo}, args...)...).CombinedOutput(); err != nil { + t.Fatalf("git %v: %v: %s", args, err, out) + } + } + run("init") + run("config", "user.email", "t@t") + run("config", "user.name", "t") + for _, name := range []string{"TASK.md", "deleted.txt", "renamed.txt", "staged.txt"} { + if err := os.WriteFile(filepath.Join(repo, name), []byte(name), 0644); err != nil { + t.Fatal(err) + } + } + run("add", "-A") + run("commit", "-m", "base") + if err := os.WriteFile(filepath.Join(repo, "staged.txt"), []byte("staged change"), 0644); err != nil { + t.Fatal(err) + } + run("add", "staged.txt") + if err := os.Remove(filepath.Join(repo, "deleted.txt")); err != nil { + t.Fatal(err) + } + run("mv", "renamed.txt", "renamed-new.txt") + if err := os.WriteFile(filepath.Join(repo, "untracked.txt"), []byte("new"), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(repo, ".orchestra-handoff-report.md"), []byte("protocol"), 0644); err != nil { + t.Fatal(err) + } + if err := ScratchCommit(repo, "orchestra/scratch/test", "checkpoint"); err != nil { + t.Fatal(err) + } + for _, want := range []string{"staged.txt", "renamed-new.txt", "untracked.txt"} { + if err := exec.Command("git", "-C", repo, "cat-file", "-e", "HEAD:"+want).Run(); err != nil { + t.Fatalf("checkpoint omitted %s: %v", want, err) + } + } + if err := exec.Command("git", "-C", repo, "cat-file", "-e", "HEAD:deleted.txt").Run(); err == nil { + t.Fatal("checkpoint retained deleted file") + } + if err := exec.Command("git", "-C", repo, "cat-file", "-e", "HEAD:.orchestra-handoff-report.md").Run(); err == nil { + t.Fatal("checkpoint committed protocol marker") + } +} + func TestHandoffCASAndPickup(t *testing.T) { root := t.TempDir() run := func(a ...string) { diff --git a/internal/domain/domain.go b/internal/domain/domain.go index 2e3128f..d1cef94 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -38,11 +38,69 @@ const ( StateBlocked TaskState = "blocked" ) +// BlockReason is the machine-readable diagnosis for a TaskBlocked event. +// Blocker remains the operator-facing detail; this field lets projections +// group attention without repeatedly parsing prose at read time. +type BlockReason string + +const ( + BlockReasonLeaseFailure BlockReason = "lease_failure" + BlockReasonWorkerOffline BlockReason = "worker_offline" + BlockReasonLeaseExpired BlockReason = "lease_expired" + BlockReasonApproval BlockReason = "approval" + BlockReasonHandoffValidation BlockReason = "handoff_validation" + BlockReasonOperator BlockReason = "operator_block" + BlockReasonSystem BlockReason = "system_error" + BlockReasonUnknown BlockReason = "unknown" +) + +func (r BlockReason) Valid() bool { + switch r { + case BlockReasonLeaseFailure, BlockReasonWorkerOffline, BlockReasonLeaseExpired, + BlockReasonApproval, BlockReasonHandoffValidation, BlockReasonOperator, + BlockReasonSystem, BlockReasonUnknown: + return true + } + return false +} + +// InferBlockReason supplies a stable category for older events which only +// recorded a prose blocker. New producers should send block_reason directly. +func InferBlockReason(blocker string) BlockReason { + v := strings.ToLower(blocker) + switch { + case strings.Contains(v, "handoff"): + return BlockReasonHandoffValidation + case strings.Contains(v, "approval") || strings.Contains(v, "permission"): + return BlockReasonApproval + case strings.Contains(v, "expired") && strings.Contains(v, "lease"): + return BlockReasonLeaseExpired + case strings.Contains(v, "worker") && (strings.Contains(v, "offline") || strings.Contains(v, "unreachable")): + return BlockReasonWorkerOffline + case strings.Contains(v, "lease") || strings.Contains(v, "agent.start") || strings.Contains(v, "pane"): + return BlockReasonLeaseFailure + default: + return BlockReasonSystem + } +} + type Estimate struct { Value float64 `json:"value"` Who string `json:"who"` Confidence float64 `json:"confidence"` } + +// SessionEvidence is captured by the machine that owns a pane immediately +// before it drops its mapping. It is deliberately observation-only: it never +// claims that a pane is still live after the worker has closed it. +type SessionEvidence struct { + PaneID string `json:"pane_id,omitempty"` + HarnessID string `json:"harness_id,omitempty"` + PaneState string `json:"pane_state,omitempty"` + Source string `json:"source,omitempty"` + CapturedAt time.Time `json:"captured_at,omitempty"` + CheckedAt time.Time `json:"checked_at,omitempty"` +} type Lease struct { HarnessID string `json:"harness_id"` Until time.Time `json:"until"` @@ -62,17 +120,27 @@ type Task struct { // HandoffRef survives the queued interval between TaskReleased and the // next router-owned TaskLeased event; it is the only artifact the worker // may use for local pickup validation. - HandoffRef string `json:"handoff_ref,omitempty"` - Version int `json:"version"` - Title string `json:"title,omitempty"` - Description string `json:"description,omitempty"` + HandoffRef string `json:"handoff_ref,omitempty"` + // ReleaseTransaction and ReleaseAnchor bind successor pickup to the exact + // durable predecessor checkpoint. They survive queueing and re-lease. + ReleaseTransaction string `json:"release_transaction,omitempty"` + ReleaseAnchor string `json:"release_anchor,omitempty"` + PickupTransaction string `json:"pickup_transaction,omitempty"` + PickupLeaseVersion int `json:"pickup_lease_version,omitempty"` + Version int `json:"version"` + Title string `json:"title,omitempty"` + Description string `json:"description,omitempty"` + Acceptance []string `json:"acceptance,omitempty"` + QualityGate string `json:"quality_gate,omitempty"` // Block evidence is projected from TaskBlocked so terminal records remain // diagnosable after the live coordinator mapping is gone. - Blocker string `json:"blocker,omitempty"` - BlockedAt time.Time `json:"blocked_at,omitempty"` - LastPaneID string `json:"last_pane_id,omitempty"` - LastHarness string `json:"last_harness_id,omitempty"` - PaneState string `json:"pane_state,omitempty"` // open, closed, unreachable, unknown + Blocker string `json:"blocker,omitempty"` + BlockReason BlockReason `json:"block_reason,omitempty"` + BlockedAt time.Time `json:"blocked_at,omitempty"` + LastPaneID string `json:"last_pane_id,omitempty"` + LastHarness string `json:"last_harness_id,omitempty"` + PaneState string `json:"pane_state,omitempty"` // open, closed, unreachable, unknown + LastSession SessionEvidence `json:"last_session,omitempty"` } type Event struct { @@ -112,7 +180,7 @@ func ValidateEvent(e Event) error { if e.SchemaVersion >= 2 && strings.TrimSpace(e.Surface) == "" { return fmt.Errorf("%w: surface required", ErrInvalid) } - allowed := map[string]bool{"TaskCreated": true, "TaskLeased": true, "TaskReleased": true, "TaskCompleted": true, "TaskFailed": true, "TaskBlocked": true, "ApprovalRequested": true, "ApprovalGranted": true, "ApprovalDenied": true, "TaskAmended": true, "TaskCorrected": true, "QuotaReported": true, "StandupAdvisory": true} + allowed := map[string]bool{"TaskCreated": true, "TaskLeased": true, "TaskLeaseRenewed": true, "TaskReleased": true, "TaskPickupValidated": true, "TaskCompleted": true, "TaskFailed": true, "TaskBlocked": true, "ApprovalRequested": true, "ApprovalGranted": true, "ApprovalDenied": true, "TaskAmended": true, "TaskCorrected": true, "QuotaReported": true, "StandupAdvisory": true} if !allowed[e.Type] { return fmt.Errorf("%w: unknown type %q", ErrInvalid, e.Type) } @@ -160,10 +228,36 @@ func ValidatePayload(typ string, p map[string]any) error { if v, ok := p["expected_version"].(float64); !ok || v < 0 || v != float64(int(v)) { return fmt.Errorf("%w: expected_version invalid", ErrInvalid) } + case "TaskLeaseRenewed": + if err := requiredString("harness_id"); err != nil { + return err + } + until, ok := p["until_ns"].(float64) + if !ok || until <= float64(time.Now().UnixNano()) { + return fmt.Errorf("%w: until_ns required", ErrInvalid) + } + if v, ok := p["expected_version"].(float64); !ok || v < 0 || v != float64(int(v)) { + return fmt.Errorf("%w: expected_version invalid", ErrInvalid) + } case "TaskReleased": if err := requiredString("handoff_ref"); err != nil && p["reason"] == nil { return err } + case "TaskPickupValidated": + for _, key := range []string{"transaction_id", "handoff_ref", "anchor_sha", "harness_id"} { + if err := requiredString(key); err != nil { + return err + } + } + if err := requiredHash(p, "handoff_ref"); err != nil { + return err + } + if v, ok := p["anchor_sha"].(string); !ok || len(v) != 40 { + return fmt.Errorf("%w: anchor_sha invalid", ErrInvalid) + } + if v, ok := p["lease_version"].(float64); !ok || v < 1 || v != float64(int(v)) { + return fmt.Errorf("%w: lease_version invalid", ErrInvalid) + } if _, ok := p["handoff_ref"]; ok { if err := requiredHash(p, "handoff_ref"); err != nil { return err @@ -183,6 +277,17 @@ func ValidatePayload(typ string, p map[string]any) error { if receipt, ok := p["receipt"].(map[string]any); !ok || len(receipt) == 0 { return fmt.Errorf("%w: receipt required", ErrInvalid) } + if v, ok := p["result_sha"]; ok { + if s, ok := v.(string); !ok || len(s) != 40 { + return fmt.Errorf("%w: result_sha invalid", ErrInvalid) + } + if err := requiredString("branch"); err != nil { + return err + } + if err := requiredString("remote"); err != nil { + return err + } + } case "TaskFailed": if err := requiredString("reason"); err != nil { return err @@ -191,6 +296,12 @@ func ValidatePayload(typ string, p map[string]any) error { if err := requiredString("blocker"); err != nil { return err } + if v, ok := p["block_reason"]; ok { + s, ok := v.(string) + if !ok || !BlockReason(s).Valid() { + return fmt.Errorf("%w: block_reason invalid", ErrInvalid) + } + } if _, ok := p["handoff_ref"]; ok { if err := requiredHash(p, "handoff_ref"); err != nil { return err diff --git a/internal/federation/client.go b/internal/federation/client.go index 990eb52..611e029 100644 --- a/internal/federation/client.go +++ b/internal/federation/client.go @@ -24,7 +24,7 @@ type Client struct { } func (c Client) Register(ctx context.Context, w Worker) error { - b, err := json.Marshal(map[string]any{"id": w.ID, "address": w.Address, "capacity": w.Capacity, "token": c.Token}) + b, err := json.Marshal(map[string]any{"id": w.ID, "address": w.Address, "capacity": w.Capacity, "supported_projects": w.SupportedProjects, "build": w.Build, "token": c.Token}) if err != nil { return err } @@ -131,6 +131,13 @@ func (c Client) Heartbeat(ctx context.Context, health WorkerHealth) error { } return err } +func (c Client) Renew(ctx context.Context, taskID string, expectedVersion, ttlSeconds int) error { + resp, err := c.request(ctx, http.MethodPost, "/v1/federation/workers/"+url.PathEscape(c.WorkerID)+"/renew", map[string]any{"task_id": taskID, "expected_version": expectedVersion, "ttl_seconds": ttlSeconds}) + if resp != nil { + resp.Body.Close() + } + return err +} func (c Client) Artifact(ctx context.Context, ref string) ([]byte, error) { resp, err := c.request(ctx, http.MethodGet, "/v1/artifacts/"+url.PathEscape(ref), nil) if err != nil { @@ -169,15 +176,22 @@ func (c Client) PutArtifact(ctx context.Context, b []byte) (string, error) { } return out.Ref, nil } -func (c Client) Release(ctx context.Context, taskID, ref, anchor string) error { - resp, err := c.request(ctx, http.MethodPost, "/v1/federation/workers/"+url.PathEscape(c.WorkerID)+"/handoff", map[string]string{"task_id": taskID, "handoff_ref": ref, "anchor_sha": anchor}) +func (c Client) Release(ctx context.Context, taskID, ref, anchor, transactionID string, expectedVersion int, evidence domain.SessionEvidence) error { + resp, err := c.request(ctx, http.MethodPost, "/v1/federation/workers/"+url.PathEscape(c.WorkerID)+"/handoff", map[string]any{"task_id": taskID, "handoff_ref": ref, "anchor_sha": anchor, "transaction_id": transactionID, "expected_version": expectedVersion, "session_evidence": evidence}) if resp != nil { resp.Body.Close() } return err } -func (c Client) Complete(ctx context.Context, taskID, reportRef string) error { - resp, err := c.request(ctx, http.MethodPost, "/v1/federation/workers/"+url.PathEscape(c.WorkerID)+"/complete", map[string]string{"task_id": taskID, "handoff_ref": reportRef}) +func (c Client) Pickup(ctx context.Context, taskID, ref, anchor, transactionID string, leaseVersion int, evidence domain.SessionEvidence) error { + resp, err := c.request(ctx, http.MethodPost, "/v1/federation/workers/"+url.PathEscape(c.WorkerID)+"/pickup", map[string]any{"task_id": taskID, "handoff_ref": ref, "anchor_sha": anchor, "transaction_id": transactionID, "lease_version": leaseVersion, "session_evidence": evidence}) + if resp != nil { + resp.Body.Close() + } + return err +} +func (c Client) Complete(ctx context.Context, taskID, reportRef, resultSHA, branch, remote string, expectedVersion int, receipt map[string]any, evidence domain.SessionEvidence) error { + resp, err := c.request(ctx, http.MethodPost, "/v1/federation/workers/"+url.PathEscape(c.WorkerID)+"/complete", map[string]any{"task_id": taskID, "handoff_ref": reportRef, "result_sha": resultSHA, "branch": branch, "remote": remote, "expected_version": expectedVersion, "receipt": receipt, "session_evidence": evidence}) if resp != nil { resp.Body.Close() } diff --git a/internal/herdr/adapter.go b/internal/herdr/adapter.go index 4426b54..1978edd 100644 --- a/internal/herdr/adapter.go +++ b/internal/herdr/adapter.go @@ -248,80 +248,99 @@ func (a CLIAdapter) NotifyConventionsChanged(ctx context.Context, s Session) err // handoff is refused rather than guessed at: the caller (Coordinator.rotate) // leaves the lease intact and retries next tick, giving the agent time to // finish writing it. -func (a CLIAdapter) Release(ctx context.Context, s Session) (string, error) { +// PreparedRelease is the durable, coordinator-independent half of a release. +// The worker persists it before publishing TaskReleased so a lost HTTP reply +// never requires reconstructing (or deleting) the agent's report. +type PreparedRelease struct { + Ref string + AnchorSHA string +} + +// PrepareRelease seals an immutable Git checkpoint and uploads its canonical +// handoff, but deliberately leaves both the pane claim and report in place. +// The caller controls the retryable transaction around coordinator acceptance. +func (a CLIAdapter) PrepareRelease(ctx context.Context, s Session) (PreparedRelease, error) { if a.CAS == nil { - return "", fmt.Errorf("adapter: CAS store required to upload handoff") + return PreparedRelease{}, fmt.Errorf("adapter: CAS store required to upload handoff") + } + // A federation worker always supplies the immutable task hash and remote. + // The empty-hash case is retained solely for old in-process adapter users; + // it is not reachable from the worker release path. + if s.TaskFileSHA != "" { + if a.Remote == "" { + return PreparedRelease{}, fmt.Errorf("adapter: project remote required for checkpoint") + } + if err := continuity.VerifyTaskFile(s.Worktree, s.TaskFileSHA); err != nil { + return PreparedRelease{}, fmt.Errorf("adapter: verify immutable TASK.md: %w", err) + } } path := filepath.Join(s.Worktree, HandoffReportFile) b, err := os.ReadFile(path) if err != nil { - return "", fmt.Errorf("adapter: semantic handoff report not written yet (%s): %w", path, err) + return PreparedRelease{}, fmt.Errorf("adapter: semantic handoff report not written yet (%s): %w", path, err) } if strings.TrimSpace(string(b)) == "" { - return "", fmt.Errorf("adapter: semantic handoff report is empty") + return PreparedRelease{}, fmt.Errorf("adapter: semantic handoff report is empty") } h, err := canonicalHandoff(s, string(b), a.lastObservedCommand(s)) if err != nil { - return "", err + return PreparedRelease{}, err } - sha, err := HeadSHA(s.Worktree) + // Always checkpoint and push, including already-committed clean work. Git + // is the cross-machine transport, so merely observing a local clean HEAD is + // not a sufficient anchor. + branch := "orchestra/scratch/" + h.Meta.ID + if err := continuity.ScratchCommit(s.Worktree, branch, "orchestra: pre-release WIP snapshot ("+h.Meta.ID+")"); err != nil { + return PreparedRelease{}, fmt.Errorf("adapter: scratch commit: %w", err) + } + anchor, err := HeadSHA(s.Worktree) if err != nil { - return "", fmt.Errorf("adapter: read worktree HEAD: %w", err) + return PreparedRelease{}, fmt.Errorf("adapter: read checkpoint HEAD: %w", err) } - _ = sha - for _, d := range h.Anchor.Dirty { - if hex.EncodeToString(sha256sum(filepath.Join(s.Worktree, d.Path))) != d.SHA256 { - return "", fmt.Errorf("adapter: handoff dirty file changed since it was written: %s", d.Path) + if a.Remote != "" { + if err := continuity.ScratchPush(s.Worktree, branch, a.Remote); err != nil { + return PreparedRelease{}, fmt.Errorf("adapter: push checkpoint: %w", err) + } + out, err := exec.CommandContext(ctx, "git", "-C", s.Worktree, "ls-remote", a.Remote, "refs/heads/"+branch).Output() + if err != nil || !strings.HasPrefix(string(out), anchor+"\t") { + return PreparedRelease{}, fmt.Errorf("adapter: verify pushed anchor: got %q: %w", strings.TrimSpace(string(out)), err) } } - // The semantic report is transferred in the CAS handoff, not in the - // scratch checkout. Keeping it in the scratch commit makes a successor - // mistake the predecessor's report for a newly requested handoff and can - // cause an immediate release/pickup loop. - dirty := h.Anchor.Dirty[:0] - for _, d := range h.Anchor.Dirty { - if filepath.Clean(d.Path) != HandoffReportFile { - dirty = append(dirty, d) - } - } - h.Anchor.Dirty = dirty - if err := os.Remove(path); err != nil && !os.IsNotExist(err) { - return "", fmt.Errorf("adapter: remove transferred semantic report: %w", err) - } - // Atomically commit whatever the handoff described as dirty onto a - // per-task scratch branch (§6.2 step 3) *before* uploading, so the - // successor's pickup validation collapses to a single HEAD compare - // instead of re-hashing every dirty file individually. - if len(h.Anchor.Dirty) > 0 { - branch := "orchestra/scratch/" + h.Meta.ID - if err := continuity.ScratchCommit(s.Worktree, branch, "orchestra: pre-release WIP snapshot ("+h.Meta.ID+")"); err != nil { - return "", fmt.Errorf("adapter: scratch commit: %w", err) - } - if a.Remote != "" { - if err := continuity.ScratchPush(s.Worktree, branch, a.Remote); err != nil { - return "", fmt.Errorf("adapter: push scratch branch: %w", err) - } - } - newSHA, err := HeadSHA(s.Worktree) - if err != nil { - return "", fmt.Errorf("adapter: read scratch HEAD: %w", err) - } - h.Anchor.GitSHA = newSHA - h.Anchor.Branch = branch - h.Anchor.Dirty = nil - } + h.Anchor.GitSHA, h.Anchor.Branch, h.Anchor.Dirty = anchor, branch, nil ref, err := continuity.Save(h, a.CAS) if err != nil { - return "", fmt.Errorf("adapter: upload handoff: %w", err) + return PreparedRelease{}, fmt.Errorf("adapter: upload handoff: %w", err) } + return PreparedRelease{Ref: ref, AnchorSHA: anchor}, nil +} + +// ReleaseAgent drops only herdr's harness binding. It does not close the pane: +// a predecessor stays recoverable until the successor has validated pickup. +func (a CLIAdapter) ReleaseAgent(ctx context.Context, s Session) error { if err := a.Client.Call(ctx, "pane.release_agent", map[string]any{ "pane_id": s.PaneID, "source": "herdr:" + a.Harness, "agent": agentForSession(s, a.Harness), }, nil); err != nil { - return "", fmt.Errorf("adapter: pane.release_agent: %w", err) + return fmt.Errorf("adapter: pane.release_agent: %w", err) } - return ref, nil + return nil +} + +// Release is retained for the coordinator's legacy local path. Federation +// workers use PrepareRelease and ReleaseAgent as separate durable phases. +func (a CLIAdapter) Release(ctx context.Context, s Session) (string, error) { + p, err := a.PrepareRelease(ctx, s) + if err != nil { + return "", err + } + if err := a.ReleaseAgent(ctx, s); err != nil { + return "", err + } + if err := os.Remove(filepath.Join(s.Worktree, HandoffReportFile)); err != nil && !os.IsNotExist(err) { + return "", fmt.Errorf("adapter: remove transferred semantic report: %w", err) + } + return p.Ref, nil } // canonicalHandoff keeps Git-derived protocol facts on the worker that owns @@ -483,16 +502,27 @@ func handoffID(s Session) string { } func dirtyFiles(root string) ([]continuity.Dirty, error) { + out, err := exec.Command("git", "-C", root, "status", "--porcelain=v1", "-z").Output() + if err != nil { + return nil, err + } paths := map[string]bool{} - for _, args := range [][]string{{"diff", "--name-only", "-z"}, {"ls-files", "--others", "--exclude-standard", "-z"}} { - out, err := exec.Command("git", append([]string{"-C", root}, args...)...).Output() - if err != nil { - return nil, err + deleted := map[string]bool{} + parts := strings.Split(string(out), "\x00") + for i := 0; i < len(parts); i++ { + record := parts[i] + if len(record) < 4 { + continue } - for _, path := range strings.Split(string(out), "\x00") { - if path != "" && path != HandoffFile { - paths[path] = true - } + status, path := record[:2], record[3:] + if path == HandoffFile || path == HandoffReportFile || path == ".orchestra/done" || strings.HasPrefix(path, ".orchestra/") { + continue + } + paths[path] = true + deleted[path] = strings.Contains(status, "D") + // A rename/copy record has the original path as the next NUL item. + if status[0] == 'R' || status[0] == 'C' || status[1] == 'R' || status[1] == 'C' { + i++ } } keys := make([]string, 0, len(paths)) @@ -502,11 +532,15 @@ func dirtyFiles(root string) ([]continuity.Dirty, error) { sort.Strings(keys) dirty := make([]continuity.Dirty, 0, len(keys)) for _, path := range keys { - sum := sha256sum(filepath.Join(root, path)) - if len(sum) == 0 { - return nil, fmt.Errorf("adapter: hash dirty file %s", path) + d := continuity.Dirty{Path: path, Deleted: deleted[path]} + if !d.Deleted { + sum := sha256sum(filepath.Join(root, path)) + if len(sum) == 0 { + return nil, fmt.Errorf("adapter: hash dirty file %s", path) + } + d.SHA256 = hex.EncodeToString(sum) } - dirty = append(dirty, continuity.Dirty{Path: path, SHA256: hex.EncodeToString(sum)}) + dirty = append(dirty, d) } return dirty, nil } @@ -641,6 +675,13 @@ var _ = json.RawMessage{} // callers (Coordinator.rotate, refreshSessionHealth) surface it instead of // mistaking "we don't know" for "occupancy is zero". func (a CLIAdapter) Occupancy(s Session) (float64, error) { + if a.Harness == "opencode" { + u, err := OpenCodeSessionUsage(s.SessionID) + if err != nil { + return 0, err + } + return Fraction(u, a.Window), nil + } if a.Usage == nil { return 0, fmt.Errorf("adapter: usage reader required") } @@ -659,6 +700,32 @@ func (a CLIAdapter) Occupancy(s Session) (float64, error) { return Fraction(u, a.Window), nil } +// ResolveSessionIdentity discovers and returns the harness-native session +// identity. Callers persist the returned Session before relying on occupancy, +// so restart recovery keeps observing the same harness session. +func (a CLIAdapter) ResolveSessionIdentity(s Session) (Session, error) { + if a.Harness == "opencode" { + if s.SessionID != "" { + return s, nil + } + id, err := OpenCodeSessionID(s.Worktree) + if err != nil { + return s, err + } + s.SessionID = id + return s, nil + } + if s.SessionFile != "" { + return s, nil + } + path, err := a.resolveSessionFile(s) + if err != nil { + return s, err + } + s.SessionFile = path + return s, nil +} + func (a CLIAdapter) resolveSessionFile(s Session) (string, error) { switch a.Harness { case "claude": @@ -667,12 +734,7 @@ func (a CLIAdapter) resolveSessionFile(s Session) (string, error) { _, path, err := CodexActiveUsage("") return path, err default: - // opencode's session-file resolution needs the running session id, - // which is only available via the SSE/status API (OpenCodeStatus), - // not derivable from the worktree alone. Per AUDIT.md Phase 1, wiring - // this needs verification against a live opencode instance before it - // can drive rotation — refuse loudly rather than guess a path. - return "", fmt.Errorf("adapter: harness %q has no session-file resolver; verify against a live session first (AUDIT.md Phase 1)", a.Harness) + return "", fmt.Errorf("adapter: harness %q has no session-file resolver", a.Harness) } } diff --git a/internal/herdr/herdr.go b/internal/herdr/herdr.go index 56e1ea8..d8f13c4 100644 --- a/internal/herdr/herdr.go +++ b/internal/herdr/herdr.go @@ -152,6 +152,10 @@ type Session struct { // CLIAdapter.Occupancy), since the file may not exist yet immediately // after lease. SessionFile string `json:"session_file,omitempty"` + // SessionID is the harness-native identity when its usage is stored in a + // database rather than a transcript. OpenCode's SQLite session ID is kept + // here so rotation never guesses "the newest session" after a restart. + SessionID string `json:"session_id,omitempty"` // TaskFileSHA is the sha256 of the worktree's TASK.md at the time this // session's lease was created — the immutable-spec hash continuity's // pickup validation compares against on the next rotation (§6.2). diff --git a/internal/herdr/occupancy.go b/internal/herdr/occupancy.go index 8d997a1..43b0268 100644 --- a/internal/herdr/occupancy.go +++ b/internal/herdr/occupancy.go @@ -10,6 +10,7 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" "time" ) @@ -30,6 +31,7 @@ func Fraction(u Usage, w int64) float64 { } return f } + // ClaudeSessionFile resolves the transcript file for a Claude Code session // running against worktree, by newest-mtime under Claude Code's encoded // project directory (~/.claude/projects/