kami
97a9c65302
Acts on the seven review comments on PR #1. Design A is gone (comment 4). clients/ deleted rather than tracked: with workers carrying cross-machine work the bridge is undeployed, which supersedes the 2026-07-27 "keep through Phase 5" decision. CLAUDE.md, AGENTS.md and AUDIT.md updated from "retained" to "deleted". The harness-hook completion path is gone (comment 10). Investigation of the live OpenCode QA run showed orchestra-worker owns completion end to end: it watches for .orchestra/done, confirms via AgentStatus that the agent is not busy, then posts through /v1/federation/* with both lease epoch and expected version. The hook scripts used a different, older convention (.orchestra-report.md) and posted to /v1/harness/complete, which had already been reduced to a 410 stub - so that path could not have completed a task. Nothing exercised it, because the live run never used it. Deleted: the three deploy/hooks scripts, the 410 route, the unmounted harnessCompletion handler, and its test. That test passed against a handler no mux routed to, which is the exact "looks wired but isn't" pattern CLAUDE.md warns about; the constant-time token compare added to it earlier today goes with it, having never been reachable. /v1/harness/turn is untouched and still live. Retired deployment files (comments 8, 12, 14): deploy/orchestra.service and deploy/redeploy.sh (which sudo-installed to /usr/local/bin and restarted that unit), plus deploy/docker-api-entrypoint.sh. The entrypoint was safe to remove once its premise was checked: env vars reach the container through `env_file: .env` in compose.yaml, not by sourcing /etc/orchestra/orchestra.env - only config.jsonc is bind-mounted there - and Dockerfile.api's line 17 already sets ORCHESTRA_DATA/ORCHESTRA_PORT. Dockerfile.api now execs /app/orchestra directly. orchestra-worker.service is a different, current unit and is kept. deploy/config.example.json deleted as a duplicate (comment 6); the annotated .jsonc is the one registry.go points at, and its header no longer tells the reader to copy the file that just went away. Documentation corrected beyond the deletions: - CLAUDE.md's deployment section claimed the container bind-mounts /etc/orchestra:ro and its entrypoint sources the env file. Both wrong. - AGENTS.md still described a systemd deployment on homesrv as of 2026-07-27. - AUDIT.md's H5 row still described a "retained compatibility handler". - deploy/DEPLOYMENT.md still named redeploy.sh as the deployment path. - deploy/orchestra.env.example still cited EnvironmentFile=. TOKEN_MINIMAL_WORKFLOW_PLAN.md (comment 2) is untouched: it and WEB_UI_PLAN.md were both missed by REVIEW.md's documentation sweep, and reconciling a 534-line forward-looking plan against AUDIT.md is its own task, not a review fixup. Verified: go build ./..., go vet ./..., go test ./... all pass after the deletions, and go list ./... has no node_modules entry. No live herdr or pane was touched; nothing was deployed. The running image still predates this commit until compose is rebuilt. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GEugbHVYfAXFpTqDYbByEB
171 lines
12 KiB
Markdown
171 lines
12 KiB
Markdown
# 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:** the source-level P0/P1/P2 fixes are in place, the live
|
|
coordinator can replay its repaired event log, and the current OpenCode worker
|
|
is connected. The system is not safe to run unattended until the controlled
|
|
QA matrix has passed for all three harnesses.
|
|
|
|
## Evidence
|
|
|
|
- `go build ./...`, `go vet ./...`, `go test ./...`, and `go test -race ./...`: pass.
|
|
- 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 follow-up: Docker owns the coordinator; the old systemd unit is
|
|
inactive. The pre-v2 repeated-`seq=1` event prefix was migrated with a
|
|
backup-preserving, explicit tool before the current coordinator replayed it.
|
|
- Deployment follow-up: coordinator and installed workpc OpenCode worker are
|
|
clean revision `d6cab133b56666f81f569f4c1c3c9a6f104088d1`. The worker service
|
|
restarted at 2026-07-30 16:07 +04, has emitted no federation failures since,
|
|
and its configured Unix-socket herdr answered `ping` with protocol `17`.
|
|
|
|
## Federation design status (updated 2026-07-30)
|
|
|
|
Design B ("workers pull tasks", `/v1/federation/*`) is the **live** design. It
|
|
is no longer clientless: `cmd/orchestra-worker/main.go` (~1,131 lines, tests in
|
|
`cmd/orchestra-worker/main_test.go`) is the deployed worker, and the workpc
|
|
OpenCode worker runs it. Any earlier statement here or in `CLAUDE.md` that
|
|
Design B "has zero clients — no worker binary exists" is obsolete.
|
|
|
|
The Design A guardrail from the 2026-07-27 decision has landed:
|
|
`Coordinator.adapterFor` (`internal/orchestrator/orchestrator.go`) refuses to
|
|
resolve an adapter for a session owned by a non-local herdr, so rotation and
|
|
cleanup can no longer validate a git anchor against the wrong machine's
|
|
checkout.
|
|
|
|
Design A is **deleted as of 2026-07-31**, superseding the 2026-07-27 "retain
|
|
through Phase 5" decision: the operator confirmed the bridge is undeployed now
|
|
that workers carry cross-machine work, so `clients/` was removed rather than
|
|
tracked. The Phase 6 cutover is therefore already done on this axis.
|
|
|
|
The legacy harness-hook completion path was removed in the same pass, once it
|
|
was confirmed that nothing calls it. `orchestra-worker` owns completion — it
|
|
watches for `.orchestra/done`, confirms via `AgentStatus` that the agent is not
|
|
busy, then posts through `/v1/federation/*` with the lease epoch and expected
|
|
version. Deleted: the `/v1/harness/complete` route (a `410` stub), its unmounted
|
|
`harnessCompletion` handler, that handler's test (green against unreachable
|
|
code — the pattern this audit exists to catch), and the three `deploy/hooks/`
|
|
scripts, which still used the older `.orchestra-report.md` marker and would
|
|
have failed against the `410`. `/v1/harness/turn` is unaffected and still live.
|
|
|
|
Note for the QA matrix: the OpenCode run completed through the worker path, so
|
|
no hook script was exercised. Nothing about hook-based completion was ever
|
|
verified live, which is why deleting it costs nothing.
|
|
|
|
## Remaining release blockers
|
|
|
|
- **Only OpenCode capacity is ready.** Workpc's `workpc-opencode` worker has
|
|
a configured project file and a reachable local herdr Unix socket. Homesrv
|
|
has no reachable herdr, and no Claude/Codex worker/herdr pair has been
|
|
verified, so the full three-harness matrix cannot begin yet.
|
|
- **B17 needs a fresh controlled run.** The historical probe's six approvals,
|
|
one `409 lease version conflict`, and `consumed:0` receipt came from the
|
|
old worker. They cannot be treated as evidence for the current worker until
|
|
a live OpenCode run is repeated; Claude and Codex require their own runs.
|
|
|
|
## QA handoff — next agent
|
|
|
|
1. **Preflight before creating work.** Read `GET /v1/federation/workers` and
|
|
coordinator diagnostics through an authenticated operator session. Confirm
|
|
each target worker reports revision `d6cab13`, supported `test-e2e`, and
|
|
fresh `herdr_status: reachable`; raw-ping its configured local Unix socket
|
|
with `params:{}` and confirm protocol 17. Confirm no pre-existing agents
|
|
or leased task on the target harness.
|
|
2. **OpenCode controlled continuity run.** Submit one new disposable
|
|
`test-e2e` task that makes a deterministic marker, releases at a clear turn
|
|
boundary, validates pickup from the resulting anchor, then completes.
|
|
Record event sequence, handoff ref, anchor SHA, transaction id, lease epoch,
|
|
native session evidence, quality-gate result, remote SHA, and a receipt
|
|
with known non-zero (or explicitly explained known-zero) usage. Do not use
|
|
destructive herdr calls against unrelated panes.
|
|
3. **Exercise rotation and recovery.** In separate disposable tasks, trigger
|
|
soft, hard, milestone, thrash, coordinator restart, worker restart/lost
|
|
response, stale completion, and corrupted-worker-state paths. Verify each
|
|
result is a fenced lifecycle event or durable `needs_attention`, never a
|
|
silent retry or orphaned pane. Preserve the predecessor until matching
|
|
pickup validation.
|
|
4. **Repeat on Claude and Codex only after provisioning their own reachable
|
|
worker/herdr pairs.** Do not treat OpenCode evidence as cross-harness
|
|
proof. After all runs, compare worker/coordinator revisions and checksums,
|
|
attach the artifacts/event ranges to this audit, and only then clear the
|
|
live release gate.
|
|
|
|
## 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 | **Closed 2026-07-30.** Every new lease carries an opaque durable `lease_epoch`; renew/release/pickup/complete validate the exact harness owner and epoch at the store boundary and federation API. Offline heartbeats retain leases until expiry, new workers require a fresh reachable local-herdr probe, and local/worker ownership loss stops or durably quarantines the old pane before its mapping is dropped. | `TestLeaseEpochFencesStaleOwnerLifecycleWrites`, `TestAvailableRequiresFreshReachableLocalHerdrHealth`, plus the full race suite cover stale re-lease/completion and health admission. |
|
|
| H5 | **Closed 2026-07-30.** `Store.Append` validates legal state/owner/epoch transitions, fsyncs the event before applying its projection, and replays projections solely from `events.jsonl` (snapshots are disposable caches). CAS, worker/federation/coordinator state use temp-file + fsync + rename; corrupt worker state aborts startup. The legacy `/v1/harness/complete` route and its handler were deleted outright on 2026-07-31 (previously a 410 stub plus an unmounted, separately-fenced handler). | `TestOpenRebuildsOnlyFromLogAndIgnoresCorruptSnapshot`, `TestWorkerRefusesCorruptDurableState`, and `go test -race ./...` pass. |
|
|
|
|
## P1 — autonomy and recovery
|
|
|
|
- **Recovery:** **Closed 2026-07-30.** Launch/recovery faults now emit
|
|
`TaskNeedsAttention`, retaining the durable harness owner and lease epoch.
|
|
Renew, release, expiry, and a late reconciled completion accept that same
|
|
fenced lease; worker state advances its expected aggregate version without
|
|
dropping the live session. `TaskBlocked` remains terminal for an explicit
|
|
operator block. `TestNeedsAttentionRetainsFencedLeaseForLateCompletion`
|
|
covers the durable recovery path.
|
|
- **Retries:** **Closed 2026-07-30.** Hand-off-less `TaskReleased` is the
|
|
single durable reclaim transition. It projects exponential `attempt`,
|
|
`next_retry_at`, and `failure_class`; router assignment reads those fields,
|
|
so coordinator restarts cannot reset a backoff or retry limit.
|
|
`TestReclaimPersistsAttemptAndBackoffAcrossReopen` covers replay.
|
|
- **Launch:** **Closed 2026-07-30.** Workers emit a fenced
|
|
`TaskLaunchAcknowledged` only after a local start/prompt is persisted.
|
|
Typed NACKs immediately reclaim transient unusable capacity, terminally
|
|
block invalid handoffs, and retain uncertain live panes for reconciliation.
|
|
- **Completion:** **Closed 2026-07-30.** `.orchestra/done` is explicit
|
|
intent only; the worker also requires native non-busy identity, runs its
|
|
quality gate, verifies immutable `TASK.md`, commits, pushes, and checks
|
|
the remote SHA before it emits completion.
|
|
- **Quota:** **Closed 2026-07-30.** Completion receipts contain native
|
|
per-lease deltas plus a known/unknown marker. Five-hour and weekly
|
|
projections are published from the same receipts; any bounded harness
|
|
without fresh known usage fails routing closed.
|
|
- **Approvals:** **Closed 2026-07-30.** Projects have a validated audited
|
|
`safe_operations` policy limited to worktree-local read/edit/test/Git.
|
|
Workers inject it into the task prompt; network, secrets, destructive
|
|
actions, and paths outside the worktree remain operator-gated.
|
|
- **Observability:** **Closed 2026-07-30.** Task projections now retain
|
|
lifecycle phase, last error, retry time/failure class, lease epoch, pane
|
|
state, and anchor. Release/anchor certification faults enter durable
|
|
`needs_attention` instead of disappearing through retry `continue` paths.
|
|
|
|
## P2 — performance
|
|
|
|
- **Closed 2026-07-30.** Each scheduling pass takes one atomic task/lease
|
|
snapshot, batches cached (TTL) reachability probes concurrently, and
|
|
evaluates candidate availability once. It no longer probes candidates or
|
|
scans active tasks once per queued task.
|
|
- **Closed 2026-07-30.** Active leases and per-harness, time-ordered quota
|
|
receipts are projection indexes. Availability uses indexed rolling-window
|
|
sums rather than decoding the event log; the task snapshot is a one-time
|
|
disposable compatibility cache rather than a full rewrite on every append.
|
|
- **Closed 2026-07-30.** `BenchmarkAssignPending{1K,10K}` and
|
|
`BenchmarkAppend{1K,10K}` report and enforce p95 budgets, with durable
|
|
fsync cost included in their respective paths.
|
|
|
|
## 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
|
|
|
|
- **Pass:** build, vet, test, and race checks pass; unit/integration coverage
|
|
includes the defined fault and cross-machine cases.
|
|
- **Pending QA:** run the controlled soft, hard, milestone, thrash,
|
|
completion, and late-recovery paths on OpenCode, Claude, and Codex without
|
|
manual intervention for safe repository work. Follow the QA handoff above
|
|
and attach the resulting event ranges/artifacts before clearing this gate.
|