83 lines
6.2 KiB
Markdown
83 lines
6.2 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:** 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.
|