Harden worker federation and operator UI

This commit is contained in:
2026-07-29 13:30:55 +04:00
parent 95a96d87a5
commit 1ca9d64e89
35 changed files with 1195 additions and 581 deletions
+138 -201
View File
@@ -17,117 +17,80 @@ document.
---
## Current verdict (as of 2026-07-28)
## Current verdict (as of 2026-07-29)
The substrate (Layer 1) is solid. Layers 24 were originally shaped-but-not-
wired; most of the blocking defects below are now closed, verified by
reading the current code (not just by trusting this file) and by targeted
tests. Three things are not yet proven:
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.
1. **A real end-to-end unattended run.** The live attempts of 2026-07-28
surfaced B12 through B17. All are now closed in code (B17 on the worker
path only), but **B13 through B17 were each found live and none has been
re-verified live** — occupancy/rotation/handoff/completion still haven't
been exercised end-to-end against a session guaranteed to exist. Treat
"closed" here as "the code no longer contains the defect," not as
operational proof.
2. **The operator surface was unauthenticated in the live deployment.**
Fixed in code (**B18**: mandatory Web token, cookie sessions for the
browser), but the fix is a breaking config change — the service will not
start until `.orchestra-config/orchestra.env` sets
`ORCHESTRA_WEB_TOKEN`. See B18 and the command-channel section.
3. **Cross-machine (federation) correctness.** Deliberately deferred — see
"The federation fork" below.
### Live evidence, checked 2026-07-29
`go build ./...`, `go vet ./...`, and `go test ./...` all pass.
- 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 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.
- A real browser loaded the deployed SPA with a valid Web session and crashed
before painting: `overview.sessions` is JSON `null`, while the bundle calls
`sessions.length`. Task detail has the same fault for `events: null`.
Source now emits empty arrays and the frontend is defensive; the current
local build renders the live board and task record. **The deployment still
needs a rebuild/redeploy.**
- 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.
- The live image predates the source session/logout work: `DELETE
/v1/ui/session` returns `405`. Do not treat token rotation as session
revocation until that endpoint is deployed.
### Follow-up live verification (2026-07-28)
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.
The OpenCode launch path has now been exercised against workpc herdr through
an isolated `test-e2e` worktree. The verified outcomes are deliberately
narrow:
### Operator UI: remaining work
- Initial prompts use a bounded retry only after an explicit JSON-RPC
rejection (herdr's short post-start readiness window). Transport timeouts
and disconnects are never replayed. A pane is inspected first; blocked or
permission-dialog panes are refused.
- Herdr accepts task-scoped unique names (`oc-<task-id>`); active sessions
are visible by that name in `agent.list`. Prompt routing uses that name,
while pane reads remain pane-scoped.
- The initial launch message carries the task title and description. This is
necessary under Design A because homesrv cannot write `TASK.md` inside a
workpc checkout. A live OpenCode run received the full instruction and,
after explicit operator approval of two file edits, created the requested
marker and report without touching other files.
- The initial launch does **not** wait for the agent to become idle. Waiting
converted an ordinary long-running first turn into a false `TaskBlocked`;
a fresh run now remains `TaskLeased` while OpenCode is paused at its normal
permission boundary.
The current UI should become a diagnosis surface, not a five-column task
catalogue.
### Deployment state observed live (2026-07-29)
Checked against the running system, not the docs:
- The deployment **moved from `orchestra.service` to Docker Compose**
(`/home/kami/docker-apps/orchestra-web-ui`). `orchestra-api` and
`orchestra-web-ui` are both up; `/healthz` and the UI answer 200. The
stopped systemd unit is the retired predecessor, not an outage — see the
deployment section of `CLAUDE.md`.
- The **running image predates the last two commits** (built 2026-07-28
21:28; `0b4b52a` "Require a token for the web UI" landed 23:15). So B18's
auth fix is *not* in the live container. Rebuild before drawing any
conclusion about the live surface's auth behaviour.
- **All six herdrs are unreachable** — workpc `192.168.1.105:9245-7` refuses,
homesrv `192.168.1.104:9245-7` times out (filtered). Probed directly, since
a herdr that connects logs nothing. Nothing can be leased; every task the
API serves is history. This is the sole remaining blocker on live proof for
B13B17.
- `GET /v1/tasks` has **no authz check at all** (`cmd/orchestra/main.go:286`
returns before any gate). B18 gated the mutation and approval controls, not
the read path. Accepted for now: the `0.0.0.0:9145` bind is deliberate and
ufw restricts the port to a single other LAN machine, so the read path's
trust boundary is the LAN, not the host. Revisit if that bind is ever
widened.
- ntfy delivery still fails `403 Forbidden` on every send (last observed
2026-07-28) — a wrong or expired credential, separate from S12's
`ORCHESTRA_NTFY_SURFACE_TOKEN`.
This does not complete the cross-machine continuity design. `clients/herdr-
bridge.go` is only a byte proxy; it is not a worker. The worker-side process
described below remains required before a remote checkout can author
`TASK.md`, derive a canonical handoff anchor, scratch-commit WIP, or safely
release it. Until then, homesrv must not rotate or clean up a non-local
worktree.
| Spec layer | State |
|---|---|
| L1 substrate (§3, §4) | Built and correct in the main path. |
| L2 harness (§5) | Occupancy, rotation, and completion all wired and reachable. B12 fixed and confirmed live; B13B16 fixed in code but not re-verified live. Unique per-task agent names (B16) and ambiguity-safe prompting (B14) are the substantive changes. |
| L3 continuity (§6) | Handoff schema, pickup validation, scratch branches, TASK.md are all wired into the live path (Phase 4 complete). |
| L4 surfaces (§7) | Brief/standup/delivery real; quota has a post-hoc producer only (no live push feed yet); B8's authz bypass closed, but the new web UI reintroduces an equivalent one (B18). |
1. **Persist and show a blocked diagnosis.** Project the structured blocking
reason/error into the task read model. Group the board by reason and age
(for example: lease failure, worker offline, expired lease, approval,
handoff validation, operator block) rather than rendering a giant generic
“Blocked” lane.
2. **Persist last-session and pane evidence.** Before a worker/coordinator
drops a terminal session mapping, retain harness, pane ID, last successful
capture/check, and a pane status of `open`, `closed`, `unreachable`, or
`unknown`. The UI must show the source and timestamp. For old records that
lack this evidence, say **unknown — legacy record has no retained
blocker/pane evidence**, never imply a live or closed pane.
3. **Make the task page lead with the diagnosis.** Put reason, last activity,
pane state, and next safe action first. Hide unavailable lifecycle forms
behind an “Unavailable actions” disclosure; an empty session must not
consume most of the page with disabled controls.
4. **Separate active work from history.** Default the board to active,
waiting, and needs-attention work; move completed/failed/test residue to
filters or a compact history view. Add search, project filtering, and an
explicit “no live work” state.
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
B14, B15 and B16 are now closed by code and covered by tests; none has been
re-verified against a live cross-machine run. B17 is closed on the worker
path only and remains open for Design A. B18 (the unauthenticated web
surface) is closed in code but **requires an env change before the service
will start** — see B18 and "What's next".
Nothing is currently blocking in the sense B12/B13 were. B19B21 and S12S13
— the correctness and hygiene gaps found while implementing B18 — are now
fixed in code and covered by tests (2026-07-28); each is marked closed in
its own section below. The flaky router test is also fixed, and the flake
was in the test, not in assignment.
The real remaining risk is entirely evidential now: B13 through B17 were
each found live and **none has been re-verified live**, and none of the
2026-07-28 fixes has run on the deployed instance either — the service is
stopped, `/usr/local/bin/orchestra` predates all of them, and installing a
new binary or editing `/etc/orchestra/orchestra.env` needs privileges this
sandbox does not have.
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)
@@ -371,7 +334,7 @@ Delivery` asserting the prompt target is the unique name. Nothing yet starts
two same-harness sessions in distinct panes and verifies both attach, so the
`agent_name_taken` failure mode itself is untested end-to-end.
### B17 — opaque harnesses must not author canonical handoff anchors (found live 2026-07-28) — closed on the worker path; open for Design A
### B17 — opaque harnesses must not author canonical handoff anchors (found live 2026-07-28) — closed in code; live success still unproven
The real OpenCode healthcheck run showed that prompting an opaque agent to
write the full `continuity.Handoff` schema is the wrong ownership boundary.
@@ -427,7 +390,7 @@ attached Claude at 08:00:43Z. The renewed attempt then hit B14's prompt
timeout and became blocked. This is expected retry behavior, but illustrates
why lifecycle retries must be visible to operators.
**Status (2026-07-28): satisfied on the worker path, unchanged for Design A.**
**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
@@ -437,11 +400,23 @@ canonical artifact is validated before publication. The handoff-provenance
correction documented at the end of this file supplies the parsing and
validation half.
The homesrv coordinator's Design A path is unchanged and still cannot
satisfy this: it does not own the workpc checkout, so any Git-derived fact
it seals is derived from the wrong filesystem. B17 is therefore only closed
for tasks that actually run through a worker. The standing rule holds —
homesrv must not rotate or clean up a non-local worktree.
The coordinator no longer accepts a non-local herdr operation; the standing
rule is enforced rather than advisory. A successful live worker handoff,
release, and pickup remains the required operational proof.
**QA attempt (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
successful handoff or a release-path regression. The worker must be given a
fresh fixture whose combined `NEXT`/`WHY` action does not mention a handoff
(for example, `WHY: verify canonical artifact construction`), then the
release and successor pickup must be observed. The coordinator is token-gated
and this host cannot read the protected worker credential, so this session
could not enqueue that replacement task. The failed disposable pane was left
untouched; no destructive herdr calls were issued.
### B13 — `agent.start` silently no-ops under back-to-back leases (found live 2026-07-28) — closed (code fix; not yet re-verified live)
@@ -639,14 +614,7 @@ action without releasing the pane.
**Known weaknesses of this channel.** The defects found in it are filed
individually below — **B19** (federated approvals emit no event), **B20**
(the local capture revision is a timestamp, not a change counter) and
**B21** (`Registry.commands` never prunes). Two more, not separately filed:
- **Captures and commands are in-memory only.** `Registry.captures` and
`Registry.commands` have no persistence, so a coordinator restart drops
pending approvals silently and resets revision counters — which, with a
reset counter, could let a stale command match a new capture. This is the
other half of B21: the lifecycle forgets across restarts and never forgets
within one.
**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
@@ -729,14 +697,16 @@ rescans the entire history on every worker poll. Combined with the
in-memory-only storage already noted, the lifecycle is wrong at both ends:
it forgets across restarts and never forgets within one.
**Fix (within-process half only).** `Registry.pruneCommands` drops resolved
**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. Covered by
`TestResolvedCommandsArePrunedButPendingOnesSurvive`. **The persistence half
of this defect is still open:** captures and commands remain in-memory only,
so a coordinator restart still drops pending approvals silently.
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)
@@ -775,41 +745,18 @@ bypass is closed by construction rather than by the header check alone.
In dependency order, not importance order:
1. **Finish applying the B18 env change on the deployed instance.** The
repo-side `.orchestra-config/orchestra.env` now sets a generated
`ORCHESTRA_WEB_TOKEN` and `ORCHESTRA_UI_INSECURE_COOKIE=1`, but the unit
reads `/etc/orchestra/orchestra.env`, which is `orchestra:orchestra 0600`
and unreadable from this sandbox — whether it carries the token could not
be confirmed. It was last modified 2026-07-28 14:30 and the service did
start at 16:17 with the B18 build absent, so this is unverified either
way. Also newly required: `ORCHESTRA_NTFY_SURFACE_TOKEN` (S12) if the
ntfy surface should stay gated, and the token for every existing
unauthenticated `/v1/` client, which now defaults to the Web surface.
2. **Install the new binary and restart.** `orchestra.service` has been
stopped since 2026-07-28 19:46 and `/usr/local/bin/orchestra` predates
every fix in this section. Needs privileges the sandbox lacks:
`go build -o /tmp/orchestra ./cmd/orchestra && sudo install /tmp/orchestra
/usr/local/bin/orchestra && sudo systemctl restart orchestra.service`.
3. **The live re-verification** that B13B17 all still lack, plus a first
live exercise of B18B21. Each of B13B17 was found live and closed on
paper; the code fixes are unproven against a real cross-machine run, and
that remains the single biggest gap between this document and reality.
Note that no herdr is currently reachable at all — the 16:17 startup logs
`connection refused` for **all six**, including workpc's, which was live
on 2026-07-27 — so a live run needs a herdr brought up first.
4. **Two operational faults visible in the journal**, unrelated to this
audit's defects but blocking a clean live run: every herdr is refusing
connections (above), and ntfy delivery is failing `403 Forbidden` on
every send (16:43 and 16:46), i.e. the ntfy server credential is wrong or
expired.
5. **The persistence half of B21** — captures and commands are in-memory
only, so a restart still silently drops pending approvals. The retention
fix bounds growth within a process; it does not make the lifecycle
durable.
6. **`RespondApproval` (coordinator path) still has no test**, unlike its
worker counterpart. With B20 fixed its revision is now meaningful, but
its text-comparison guard remains the thing actually binding a decision
to what the operator saw, and that guard is untested.
1. **Prove a successful B17 worker handoff.** Use a bounded disposable task
with a concrete next action and verify canonical report upload, release,
follow-up pickup, and cleanup. The rejection path is proven; success is
not.
2. **Fix health semantics and ntfy.** Report worker/local-herdr health
separately from legacy TCP registry probes, and correct the ntfy server
credential causing `403 Forbidden`.
3. **Address the Web UI backlog above**, starting with queue explanations,
actionable worker health, and a usable approval/recovery workflow.
4. **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
@@ -864,9 +811,9 @@ event appended with `Surface: system` set directly in Go, consistent with
B8. Extended to Codex/opencode via an optional `harness` field in the
request body (`"codex"` → `CodexUsage`, `"opencode"` → `OpenCodeUsage`).
**Still open:** no automated test for the HTTP handler (`cmd/orchestra/main.go`
has zero handler test coverage of any kind, pre-existing gap — this follows
the existing pattern rather than introducing a one-off harness).
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
@@ -957,11 +904,10 @@ Covered by `TestGitWorktreesCommitsTaskFile`, `TestStartBlocksOnInvalidPickup`,
`TestReleaseRefusesOnStaleDirtyFile`, `TestConventionsDriftNotifiesActiveSession`,
`TestRotationRequestsHandoffBeforeReleasing` (internal/herdr, internal/orchestrator).
**Caveat still open:** TASK.md hashing is best-effort/untested for the
herdr-hosted (`WorktreeCreator`) worktree path specifically — no adapter or
test exercises that path with a real `handoff_ref`, so pickup validation
there runs with an empty `taskFileSHA` (anchor + dirty-file hashes still
checked). Same cross-host caveat as the federation fork, below.
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)
@@ -1221,42 +1167,25 @@ per-tool-call source at all.
Two incompatible federation designs coexist in the tree.
**Design A — "drive the remote socket" (currently deployed,
`clients/herdr-bridge.go`)**: homesrv calls `worktree.create`/`agent.start`/
etc. directly on workpc's herdr over TCP as if it were local — meaning
anchor validation (`git rev-parse HEAD`) executes on the *wrong machine*
relative to the actual checkout. Two outcomes if the coordinator's
`session.Worktree` path happens to also exist on homesrv (likely, since
every project shares the same directory layout): either `HeadSHA` errors and
rotation silently skips forever, or — the dangerous case — it returns
**homesrv's HEAD for an unrelated checkout**, passing validation while
certifying a commit the agent never touched. Same class of bug applies to
per-host quota accounting and to `cleanupCompleted`'s `git worktree remove`,
which runs on homesrv for a worktree that lives on workpc.
**Design 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/*`)**: fully built
server-side (registration, heartbeat/TTL offline detection, event-cursor
polling/ack, lease claim), zero clients — no worker binary exists anywhere
in this repo. This is what the spec actually describes (§2.1: "everything
crossing a machine boundary is git + a validated artifact, never live state
over the wire"), but every endpoint is currently unreachable in the real
deployment. Latent, never-surfaced defects in this unused half: offline
detection only runs inside `Snapshot()`, called solely from a GET endpoint —
nothing ticks it on its own, so `OnOffline` (the hook that releases leases
held by a vanished worker) only fires if a human hits that endpoint; the
registry is in-memory with no persistence, so a restart forgets all workers
and cursors.
**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, unchanged:** keep Design A through Phase 5 (single-host concerns
— occupancy, Face B, rotation, continuity — are provable on homesrv alone
with workpc's herdr as just another pane host); commit to Design B in Phase
6. Two guardrails were meant to land immediately so Design A can't corrupt
state in the meantime: (1) refuse to rotate a lease held by a non-local
herdr rather than validate against the wrong checkout, since the protocol
schema can't run `rev-parse` where the checkout is; (2) same treatment for
`cleanupCompleted`'s worktree removal. **Status of these two guardrails is
unverified in this pass** — re-check `internal/orchestrator` before assuming
they landed; they are not confirmed closed above the way B1B11/S1S11 are.
**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
@@ -1305,6 +1234,14 @@ Numbered defects are not repeated here — B19, B20, B21, S12 and S13 are in
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
@@ -1323,11 +1260,11 @@ known, accepted, or not actionable as a single fix.
*which* of two equal-priority tasks wins is still nondeterministic, since
`sort.SliceStable` is applied to a randomly ordered slice; that is a real
property of the router, not a test artifact.
- **B14/B15/B16 are code-fixed but not re-verified live**, and B17 is closed
only for tasks that run through a worker. Each was found live, so a code
fix plus unit tests is weaker evidence than the failure that produced it.
In particular B16 still lacks the requested two-pane same-harness attach
contract test, so its actual `agent_name_taken` failure mode is untested.
- **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.