Files
platform/docs/sidecar-audit-2026-07.md
T
pastilhasandClaude Opus 5 829b034d13 docs: fold the workspace-root docs into the repo, with honest status
The root held four Markdown files that were not in any git repo and were being read as
current. CLAUDE_SIDECAR_ISOLATION.md is the one that prompted this: it describes, in the
present tense, an agent that dies whenever officer restarts. That was true when it was
written and has not been true for two days.

Rather than delete analysis that version control was not holding, the two substantial ones
moved into platform/docs/ with headers that say what has since happened:

- claude-sidecar-isolation.md — stages 0-2 are done and running (R1, R2, R4, R5 all
  satisfied); stages 3-5 are the only live part.
- sidecar-audit-2026-07.md — a snapshot audit, largely executed. Email, pty, music and vnc
  have been done since; claude, opencode and the cross-cutting notes are still open. The
  vault stays off-limits.

MUSIC_IMAGES_SPEC.md is deleted outright: the sidecar serves /image and /poster, so the
spec is the feature. The workspace root now holds one Markdown file, CLAUDE.md, which is
where cross-cutting operational reality belongs.

Also corrected, in the same pass:
- root CLAUDE.md listed email as "the big one, and untouched" and pty stage 4 as
  outstanding; both are done. It now names what actually remains (claude stages 3-5, the
  terminal orphan leak) and what landed.
- docs/sidecar-topology.md said "nothing built yet". Two sidecars now serve their own
  transport; what has NOT happened is the part the document is about — fixed ports, the
  shared table, .env toggles — so it says exactly that rather than implying the design is
  underway. Migration order updated: pty and email went first, not music.
- docs/navigation-audit.md is cited as authoritative but still planned work on Projects,
  a feature since deleted. A status header marks H3 void, H1/H2 done and H4 the one open
  item, so nobody follows it into a directory that no longer exists.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 16:45:26 +00:00

1378 lines
93 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
> **Historical — a snapshot audit, largely executed.**
> Two-pass audit from 2026-07-30. Most of what it recommends has since been done: email moved into its
> sidecar entirely (store, routes, accounts, resync and both syncs), pty became a byte relay onto the
> sidecar's own listener, music's cliamp pipeline moved out of the platform, vnc stopped reading the
> owner's password, and every HTTP sidecar now shares one proxy factory. Still open: **claude** (stages
> 35), **opencode**, the frontend-speaks-slskd smell, and the cross-cutting notes on the protocol and
> the registry. **The vault is off-limits by standing instruction.**
> Read it for the reasoning and the line counts, not as a description of today.
# Sidecar architecture — the rule, and where we currently break it
Written 2026-07-30, after an audit of the Soulseek work. This is a findings document, not a plan:
nothing here has been changed. Discussion pending.
## The rule
> The main Officer process is a **thin auth proxy, forever**. Each sidecar owns everything related to
> its job. The custom UI exists so the upstream service (slskd) stays unforked while everything
> custom is implemented on top of it.
Three consequences worth stating explicitly, because the audit turned on the third one:
1. The platform holds **no credentials** for the upstream service, and no knowledge of its API.
2. The platform's router for a sidecar has **no routes of its own** — it authenticates, injects
`X-Officer-User`, forwards, and returns the response untouched.
3. **The sidecar, not the browser, owns the upstream contract.** A thin proxy that forwards raw
upstream calls to a frontend that knows the upstream's URLs and JSON shapes has moved the
coupling rather than removed it. This is the part we are currently getting wrong.
## What is compliant (verified)
`src/servers/api/slskd/` is **70 lines total** and does exactly the two things it should:
| File | Lines | Role |
|---|---|---|
| `router.ts` | 51 | `all('/*')` catch-all. Forwards subpath + query + body, injects `X-Officer-User`, streams the response back. No routes of its own. |
| `sidecar-server.ts` | 19 | Remembers the port the sidecar reports on connect (`slskd:server`). Nothing else. |
- `SLSKD_URL` / `SLSKD_API_KEY` are read in **exactly one file**: `src/servers/sidecar/slskd/upstream.ts`.
The platform never sees either.
- `hono.ts` contains 3 slskd lines: two imports and `protectedRouter.route('/slskd', slskdRouter)`.
- `protocol.ts` contains 1: the `slskd:server` port message.
- No platform-server file imports any `soulseek*` query from `officerdb`.
Today's three commits (`8032c8b`, `b7b91a2`, `dea9ee2`) touched **zero** platform-server files.
### Not a violation, but worth naming
The `soulseek_*` tables live in the shared `officer_db` package (`schema/soulseek.ts`,
`queries/soulseek.ts`) rather than in the sidecar. Only the sidecar reads them — this was a
deliberate call (one database, schema isolated in its own file, `soulseek_` prefix) and it stands.
The cost to remember: `bun db:push` diffs the *whole* schema, which is why soulseek DDL is
hand-applied.
## The smell: the frontend speaks slskd
**37 raw `/slskd/api/v0/…` calls from React, against 10 `/slskd/_officer/…` calls.**
| File | Raw slskd calls |
|---|---|
| `SoulseekTransfers.tsx` | 8 |
| `SoulseekRooms.tsx` | 6 |
| `SoulseekChat.tsx` | 5 |
| `SoulseekDashboard.tsx` | 4 |
| `SearchView.tsx` | 4 |
| `SoulseekSystem.tsx` | 3 |
| `SearchResults.tsx` | 3 |
| `useSoulseekUser.ts` | 2 |
| `SoulseekUploads.tsx` | 2 |
The proxy stays thin, so rule (1) and (2) hold. Rule (3) does not: `shared.ts` defines slskd's wire
types (`SlskdUserStatus`, `SlskdUserInfo`, `SlskdTransferUser`, …), and the panels know slskd's URL
shapes, field names and failure modes. If slskd renames a field, the break lands in React.
Three places where it's not just a pass-through call but **domain logic that belongs in the sidecar**:
### 1. `useSoulseekUser.ts` — introduced today (`b7b91a2`)
Fans out to `/users/<u>/status` and `/users/<u>/info`, then reconciles in the browser: both
rejected → "couldn't reach them"; one rejected → show the half we got. That "what does a partially
reachable peer look like" decision is Soulseek domain knowledge living in a React hook.
Target shape: `GET /_officer/users/<u>` in the sidecar does both calls, merges, and returns one
Officer-shaped peer. The hook becomes a single fetch that doesn't know slskd exists.
### 2. `SoulseekDashboard.tsx`
Four parallel calls (`/application`, `/transfers/downloads`, `/transfers/uploads`, `/searches`)
aggregated client-side into dashboard figures. The aggregation is the product; slskd just supplies
inputs. Belongs behind one `/_officer/dashboard`.
### 3. `SoulseekTransfers.tsx`
Eight call sites, including state rollups across users and multi-delete fan-outs
(`Promise.all` over per-file `DELETE`s). Bulk operations over an upstream that has no bulk endpoint
is precisely the "custom on top of unforked slskd" the sidecar exists for — and doing it from the
browser means a closed tab is a half-finished operation.
The remaining panels (`Rooms`, `Chat`, `SearchView`, `SearchResults`, `Uploads`, `System`) are
mostly genuine 1:1 pass-throughs. They still leak slskd's shapes into the UI, but there's no
composition logic to move — lower priority, and arguably fine to leave until they grow one.
## Where the line actually is
Worth agreeing on before any refactor, since "everything through `/_officer/*`" and "proxy raw
where it's 1:1" are both defensible:
- **Sidecar route** when the answer involves more than one upstream call, any merging or
reconciliation, any Officer-only data (Postgres), any bulk operation, or any decision about what
a partial failure means.
- **Raw proxy** when it is one upstream call, forwarded, rendered — no composition of any kind.
Under that line, the three files above are the work. The other six are a naming/typing question
(does the UI import `SlskdRoom`, or an Officer type?) rather than an architecture one.
## Reference: the compliant pieces of the Soulseek sidecar
`src/servers/sidecar/slskd/` — what "the sidecar owns its job" already looks like:
| File | Role |
|---|---|
| `index.ts` | Reverse proxy to slskd on a random loopback port; documents the whole `/api/slskd/*` contract; reports its port to the platform. |
| `upstream.ts` | The only holder of `SLSKD_URL` / `SLSKD_API_KEY`. |
| `officer.ts` | The `/_officer/*` routes — favourites, browse snapshots, tree levels, filtered search, downloads. Features slskd has no concept of. |
| `browse.ts` | Multi-minute background share-tree fetch, tree built at ingest. Outlives any request. |
| `download.ts` | Expands a browsed folder into a file list from cache and enqueues it. `MAX_ENQUEUE` guard. |
Other sidecars for comparison: `claude`, `email`, `music`, `opencode`, `vault`, `vnc`.
---
# Pass 1 — backend / API code
One section per sidecar: what logic still runs in the main `officer` process. Frontend is deliberately
excluded here; it gets its own pass below. Every claim carries a `file:line` so nothing has to be
re-derived later.
The eight sidecars, from `ecosystem.config.cjs`:
| PM2 process | Entry point |
|---|---|
| `officer-claude` | `src/servers/sidecar/claude/index.ts` |
| `officer-opencode` | `src/servers/sidecar/opencode/index.ts` |
| `officer-email` | `src/servers/sidecar/email/index.ts` |
| `officer-pty` | `src/servers/api/terminal/pty-sidecar.mjs` ← note the path |
| `officer-vnc` | `src/servers/sidecar/vnc/index.ts` |
| `officer-music` | `src/servers/sidecar/music/index.ts` |
| `officer-vault` | `src/servers/sidecar/vault/index.ts` |
| `officer-slskd` | `src/servers/sidecar/slskd/index.ts` |
## music — partially compliant
`/api/music/*` is a near-exact clone of the slskd proxy and is **clean**. The violation is the
**cliamp audio subsystem**: the same domain — playing local audio to the browser — implemented
entirely in the main process, with no sidecar owning any of it.
| Surface | Lines |
|---|---|
| Compliant proxy: `api/music/router.ts` + `api/music/sidecar-server.ts` | 88 |
| `hono.ts` (3) + `protocol.ts` (1) | 4 |
| `api/cliamp/websocket.ts` | 201 |
| `api/cliamp/audio-ws.ts` | 91 |
| `server.tsx` PulseAudio bootstrap | 46 |
| `api/cliamp/asoundrc` + `server.tsx` cliamp wiring | 18 |
| **Audio domain in the main process** | **~356** |
1. **cliamp player process management**`api/cliamp/websocket.ts:1-201`. Locates the `cliamp` binary
by probing `Bun.which` plus three GOPATH candidates (`:48-66`), resolves and traversal-validates the
requested file against the owner's real home (`:40-46`, `:85-98`), shell-escapes it (`:68`, `:102`),
then spawns `script -qfc '<cliamp> <file>' /dev/null` to fake a PTY (`:106-113`) with
`PULSE_SINK: 'virtual_out'` and `ALSA_CONFIG_PATH` injected. Pumps stdout/stderr into JSON frames
(`:124-160`), forwards `{type:'input'}` to stdin (`:173-185`), kills the child on close (`:187-198`).
Child processes are held in a module-level `Map` (`:22`).
*Belongs in* `sidecar/music/`, which already runs its own loopback HTTP server
(`sidecar/music/index.ts:140`). *Obstacle:* a browser-held WebSocket with bidirectional keystroke
traffic — but the relay pattern already exists twice (`server.tsx:164-228` for dev-server,
`server.tsx:323-326` for vault).
2. **PulseAudio host-daemon bootstrap**`server.tsx:391-436`. A startup IIFE that locates
`pulseaudio`/`pactl`, runs `pulseaudio --start -D` if the daemon is down (`:401-411`), then greps
`pactl list short sinks` and loads `module-null-sink sink_name=virtual_out` if absent (`:414-435`).
Runs unconditionally at every boot even if nobody opens the player.
*Belongs in* the music sidecar's startup. *Obstacle:* none technical — same host, `pactl` works
identically. Must move together with (1) and (3), since the sink must exist before they start.
3. **Host audio capture → browser PCM**`api/cliamp/audio-ws.ts:1-91`. Spawns
`parec --format=s16le --rate=44100 --channels=2 -d virtual_out.monitor` (`:28-33`) and pushes each
chunk to the browser as a binary frame (`:44-73`). Hardcoded format, sample rate, channel count and
monitor device name — pipeline domain knowledge. *Obstacle:* continuous binary PCM, so a relay hop
costs a copy per chunk.
4. **ALSA config shipped inside the API tree**`api/cliamp/asoundrc:1-9`, passed via
`ALSA_CONFIG_PATH` (`websocket.ts:8`, `:112`). Upstream config in the thin-proxy process. Moves for
free — it's resolved by `__dirname`.
5. **Music-domain param in the WS envelope**`server.tsx:253`, `:265`, `WSData.files` at `:53`.
`upgradeWs` plumbs a `files` query param that exists only for this feature. The JWT verify +
blacklist check in `upgradeWs` (`:231-272`) is legitimate platform work; the feature-specific
parameter is not.
6. **Route names leaking into the proxy**`api/music/router.ts:29-36`. The catch-all special-cases
`/reindex` and `/reindex/stream` by name to call `server.timeout(req, 1800)`. Minor, but the slskd
reference router is uniform for every path. Simple fix: extend the timeout for the whole prefix.
7. **Done — the scope list is generic now** (2026-07-30, `17b1da2`) — `_middlewares/origin-validation.ts`.
The finding was that the music app's origin was allowlisted by hand and its scope named the feature,
with `NON_OWNER_PATHS = ['/api/auth', '/api/music']` backstopping a music-only account class. None of
it belonged in the sidecar — this is authorisation, i.e. the platform's actual job, and an origin is
**not a credential** — so the only cleanup available was making the scope list generic instead of
naming music. That is what landed:
**Any `OFFICER_<APP>_ORIGIN` in the environment is an app origin, and its rule is derived from its own
name** — `/api/auth` (it has to sign in) plus `/api/<slug>`. So `OFFICER_MUSIC_ORIGIN` (renamed from
`MUSIC_APP_ORIGIN`) gets `/api/music` without being mentioned in code, and adding an app is adding an
env var. Two declared exceptions, because they are not that convention: `OWNER_ONLY_APPS` (`APP` — the
main app is the whole platform rather than one feature, so full access but owner-only) and
`APP_SCOPE_OVERRIDES` (`TAIL``/api/vpn`, the one app whose name and API surface differ).
`NON_OWNER_PATHS` stays hand-written, and should not be mistaken for the music app's rule even though
the value matches: it is the **account** backstop, enforced whatever Origin a caller claims or omits,
and it is the airtight half of the pair. The origin rules are defence in depth on top of it.
Verified clean: no music credential or library path in the main process (`MUSIC_ROOT` exists only in
`sidecar/music/indexer.ts:24`); all range/206/`Content-Range` handling is sidecar-side
(`stream-audio.ts:77-91`); `queries/music.ts` (231 lines) has exactly one consumer,
`sidecar/music/index.ts:25-39`.
**Adjacent, flagged for the file-browser pass rather than counted here:**
`api/file-browser/router.ts` does real audio work in the main process — `/audio-meta` shells out to
`ffprobe` twice for ID3 tags and USLT/SYLT lyrics sniffing (`:459-523`), `/extract-audio` transcodes
video→mp3 with `ffmpeg -codec:a libmp3lame` (`:909-942`), `/audio-tracks` probes streams (`:407-455`),
`ensureAudioRemux` remuxes for range-seekable playback (`:80-132`). Same kind of work the music
sidecar owns, but it belongs to the file-browser contract.
## vault — partially compliant, and the placement is security-relevant
The transport proxy is right; the platform owns the entire Vaultwarden **auth/session/key-custody**
domain. This is the worst offender of the eight, and the one where placement has real consequences.
| Surface | Lines |
|---|---|
| `api/vault/router.ts` | 169 |
| `api/vault/websocket.ts` | 164 |
| `api/vault/broker.ts` | 79 |
| `api/vault/token-store.ts` | 34 |
| `api/vault/proxy-util.ts` | 29 |
| `api/vault/sidecar-server.ts` | 24 |
| **`api/vault/` total** | **499** |
| `hono.ts` (7) + `server.tsx` (8) + `protocol.ts` (2) + auth handlers (8) | 25 |
| `officerdb` vault layer the platform calls: `queries/vault.ts` + `crypto.ts` + `schema/vault.ts` | 176 |
| **Total** | **~702** (~525 excluding the shared DB package) |
For scale: the sidecar itself is 295 lines and is a genuine dumb pass-through
(`sidecar/vault/index.ts:153-183` streams bodies verbatim; `upstream.ts:12-22` is the only reader of
`VAULTWARDEN_URL`).
Two structural notes before the findings:
- `api/vault/router.ts:123` *is* an `all('/*')` catch-all, but it is not thin — it **replaces** the
`Authorization` header with a platform-held upstream credential (`:137-141`) and implements
401-refresh-retry (`:158-165`).
- It does **not** inject `X-Officer-User` (contrast `api/slskd/router.ts:34`,
`api/music/router.ts:50`). The vault sidecar receives no user identity at all — only a bearer token.
Everything below follows from that one inversion.
1. **Vaultwarden token broker in the main process**`api/vault/broker.ts:1-79` +
`api/vault/router.ts:52-104`. `broker.ts:46-57` builds an OAuth `grant_type=password` form
(`username=email`, `password=authHash`) and POSTs it to `/identity/connect/token` (`:15-20`);
`:60-66` does the `refresh_token` grant. `router.ts:52-104` is a hand-written `POST /session/login`
that parses the body, calls `passwordGrant`, extracts the token pair, persists it and assembles the
crypto-material response. Pure Vaultwarden protocol knowledge, including its version-dependent
casing quirks (`broker.ts:69-73`) and inconsistent error-message locations (`:27-31`).
**Security-relevant:** the master-password-derived `authHash` (`router.ts:57`, `broker.ts:50`)
transits main-process memory, and the main process is what mints and holds the upstream token set.
"Platform holds no upstream credentials" is violated directly here.
2. **Token custody + refresh in the main process**`api/vault/token-store.ts:1-34`, called at
`router.ts:137`, `:158-165`, `websocket.ts:73`. Loads the decrypted token set from Postgres, applies
a 60s expiry skew (`:8`, `:30`), re-derives and persists a new pair on expiry (`:11-24`).
This is the direct analogue of the slskd API key — which the platform deliberately never sees.
**Security-relevant:** plaintext access + refresh tokens exist as JS strings in the main process on
every proxied request.
3. **Notifications WebSocket proxied twice, with token injection**`api/vault/websocket.ts:1-164`.
`injectToken` (`:45-52`) rewrites the SignalR query string: drops `token`, sets `access_token=<vw
token>`. `open` (`:55-118`) verifies the platform JWT, fetches the upstream token, dials
`ws://127.0.0.1:<sidecarPort>` and runs a full buffered bidirectional pipe — **which the sidecar
already implements** (`sidecar/vault/index.ts:45-114`, `:143-151`). Frames are relayed twice.
*Obstacle:* Bun requires a synchronous upgrade, hence the deferred validation at `:60-70`; that
pattern stays, the token lookup at `:73` should not.
4. **The platform is the vault's key escrow**`api/vault/router.ts:107-120`. `PUT /unlock-key`
persists a `wrappedKey` (`:111`); `GET /unlock-key` hands it back to any owner session (`:117-119`).
This is the protector key that unwraps the on-device wrapped user key — i.e. what opens the vault
without the master password (`schema/vault.ts:22-25`).
**Security-relevant, sharpest one:** combined with (2), one compromised main process yields both the
transport token and the key material that decrypts the vault, and the read side is a plain endpoint
gated only by the platform session.
5. **Vault at-rest crypto executes in the main process**`officerdb/src/crypto.ts:1-41`, called from
`queries/vault.ts:22-23,34-35,67-68,83,88`. Derives an AES-256-GCM key as
`SHA-256(VAULT_STORE_KEY)` (`:12-20`) and runs `createCipheriv`/`createDecipheriv` (`:23-39`).
Because the vault router imports `officerdb` (`router.ts:10`, `token-store.ts:1`), all of this runs
inside `officer`. *Obstacle:* `crypto.ts` lives in the shared package, so it is importable from
anywhere; moving it means moving the vault queries out of the shared package or enforcing a
sidecar-only import boundary. No config obstacle — both processes read the same `.env`.
6. **Vault tables are read/written by the platform, not the sidecar**`queries/vault.ts:18-101`,
`schema/vault.ts:10-32`, exported globally at `officerdb/src/index.ts:130-138`. Every current caller
is main-process: `router.ts:78,111,117,159`, `token-store.ts:17,28`, plus (7). The vault sidecar
imports **no** DB module at all. Exact inverse of the intended ownership.
7. **Auth flows reach into vault storage directly**`api/auth/signout.ts:10`,
`revoke-handler.ts:18-19`, `panic-handler.ts:15-16`. Signout deletes the token row; distress and
panic also burn the protector key. The *policy* is platform-level; the *mechanism* — direct DELETEs
against the sidecar's tables — is not. All three are already best-effort `.catch(() => {})`, so
failure semantics wouldn't worsen behind a sidecar call.
8. **Dead weight**`router.ts:38-48` is a hand-written `GET /_health` passthrough the catch-all
would cover for free. `proxy-util.ts:1-29` is a byte-for-byte duplicate of the sidecar's
`stripHopByHop`/`redactPath` (`sidecar/vault/upstream.ts:33-58`), identical `HOP_BY_HOP` set and
`REDACT` regex. The redaction exists only because the main process handles token-bearing query
strings — fix (2) and it's unnecessary.
9. **Mounted outside the protected tree**`hono.ts:73-77`. `route('/api/vault', …)` sits outside
`protectedRouter`, so the router re-implements its own stack (`router.ts:31-33`: `originMiddleware`,
`userMiddleware`, `ownerGate`). *Real constraint, probably why:* it deliberately avoids
`bodyParser()` so bodies stream (`router.ts:14`), and `protectedRouter` would inherit it from
`hono.ts:88` and buffer vault attachments.
10. **Stale comments on a security boundary**`hono.ts:73-76` and
`origin-validation.ts:36-39,61-64` both claim vault requests "carry their own Bitwarden bearer
token, not a platform session JWT" and that `userMiddleware` would 401 them. Untrue since
`router.ts:32-33` requires a valid platform JWT *and* owner status on every request. Also,
`VAULT_AUTH_SPEC.md` (cited at `router.ts:12`, `schema/vault.ts:4`) and
`BITWARDEN_SIDECAR_PROMPT.md` (cited at `sidecar/vault/upstream.ts:3`) **do not exist** in the repo.
Not logic, but exactly the drift that makes someone loosen a gate by mistake.
Verified clean: `protocol.ts:61-62` carries only `{type:'vault:server', port}` — no secrets cross the
registration socket. **No other platform code and no other sidecar reads vault secrets** — grepped
every `officerdb` vault export across `src/servers` and `src/databases`; the only consumers are
`api/vault/*` and the three auth handlers. No MCP tool, queue job, or agent path pulls vault items.
User-key derivation is genuinely client-side (the platform only relays `Kdf*` params at
`router.ts:96-101`) — the one credential decision that is correctly placed.
*Shortest path to compliance (inferred, not attempted):* move `broker.ts`, `token-store.ts`,
`/session/login`, `/unlock-key`, the vault queries and `crypto.ts` into `sidecar/vault/`; have the
router inject `X-Officer-User` instead of `Authorization`; reduce `websocket.ts` to
origin-check + verify + upgrade + dumb pipe; delete `/_health` and `proxy-util.ts`; replace the three
auth-handler DB calls with a sidecar call. Result: a slskd-sized router plus `sidecar-server.ts`, and
no plaintext token or key buffer anywhere in `officer`.
## claude — not compliant. ~2,7003,050 platform lines against slskd's 70
The largest violation after email, and the one with the worst consequences, because it is the reason a
`pm2 restart officer` kills a running agent. **The process-topology half of this has its own document
`CLAUDE_SIDECAR_ISOLATION.md`.** This section is the inventory of domain logic; that one is the fix
for survivability. They overlap deliberately.
There is **no `/api/claude` mount, no proxy router, and no `X-Officer-User` anywhere on this path.**
Nothing here is shaped like slskd. The platform does not forward to the claude sidecar; it *drives* it,
over a typed RPC vocabulary, and interprets everything that comes back.
A structural fact worth stating before the list, because it inverts the usual reading: **the sidecar
imports its own event type from the platform.** `sidecar/protocol.ts:1` and
`sidecar/claude/user-instance.ts:121` both import `ChatEvent` from `../../api/chat/types`. The
dependency arrow points the wrong way — the sidecar is a client of the platform's domain model rather
than the owner of it. Every other item below is downstream of that.
1. **`api/chat/websocket.ts:1-636` — the turn loop.** The single biggest concentration. The default
model is decided here (`DEFAULT_MODEL = 'claude-code'`, `:16`), the harness is chosen by string
test (`isClaudeModel`, `:20`), the prompt is constructed here (`:336-338`), cwd is resolved here
(`:42-82`), and `:407-435` encodes the first-turn-vs-subsequent-turn distinction (whether to pass
a resume id) — a fact about the Claude CLI's contract living in the platform. `:564-589` owns stop
semantics. `:443-518` is shared with opencode.
2. **`api/chat/websocket.ts:168-312` — a ChatEvent state machine.** 145 lines interpreting the
sidecar's event stream: mutating tool-call records in place (`:247-254`), accruing cost
(`:279-282`), and persisting via `appendChatEvent` (`:103-116`) with replay at `:612-629`. This is
the machinery that makes a restart lossy — it is the durable writer, and it sits on the far side
of the socket from the process producing the events.
3. **`api/chat/claude-sessions.ts:1-361` — a reimplementation of Claude's transcript format.** The
platform reads and *writes* `~/.claude/projects/<slug>/<uuid>.jsonl` directly: the slug encoding
(`:39`), the entry schema (`:69-78`), content-block decoding (`:143-227`), listing (`:350-361`),
delete-by-unlink (`:253-258`), a 32KB `readSync` plus a `"cwd":"…"` regex to recover a session's
directory (`:294-306`), and — the sharpest example — **rename implemented by appending a
`{type:'summary'}` entry and reconstructing `leafUuid`** (`:265-287`). The platform is writing
into another program's private on-disk format. If the CLI changes that format, `officer` breaks.
4. **`api/tasks/pipeline-executor.ts:98-249, 585-586` — a second, independent interpreter.** Its own
`refreshProxyToken()` POSTing to `127.0.0.1:${ANTHROPIC_PROXY_PORT}/refresh` (`:98-105`), its own
model branch (`:126-129`), its own ChatEvent interpretation (`:151-201`), its own prompt builder
(`:257-268`), and `:585-586` silently coerces any model to `claude-code`. Two divergent
implementations of the same protocol is the cost of not having a sidecar boundary.
5. **`sidecar-registry.ts:198-274` — the platform spawns the agent.** `ensureClaudeSidecar` /
`spawnAndWaitForRegistration`: a per-email `Bun.spawn` of `user-instance.ts` with
`stdout: 'inherit', stderr: 'inherit'` (`:240-241`), a `claudeProcs` Map, a `claudeSpawnWaiters`
Map, a 15s timeout and a 50ms registration poll (`:259-267`). Plus the claude verbs at `:306-358`
and a broadcast fallback at `:339-346`. `:88-90` uses `capabilities.includes('proxy')` as a
stand-in for "is this the claude sidecar", which is only true by accident of naming.
6. **`generate-container-context.ts:135-182` (+ `:50-133`) — the platform writes the CLI's config.**
It authors `~/.claude/settings.json`: a `Stop` hook curling
`localhost:5000/api/hooks/claude-done` (`:144-153`), `defaultMode: 'bypassPermissions'` and
`skipDangerousModePermissionPrompt: true` (`:155-156`), and pre-seeded `hasTrustDialogAccepted`
(`:163-179`). Called from `users/provision.ts:28-38`. Two notes: the hook points at the platform,
so it fails during exactly the restart window that matters; and the permission posture is a
deliberate documented choice (`platform/CLAUDE.md`: agents run unsandboxed as the owner) that is
being *implemented in the wrong process*, not a mistake.
7. **`api/activity/router.ts:1-191` — the platform walks the agent's scratch tree.** Reads
`/tmp/claude-<uid>/<encoded-cwd>/tasks/<id>.output` (`:24-61`, keyed on
`startsWith('claude-')` at `:34`) and tails it over SSE (`:116-191`). Another private layout the
platform has learned. (`/announce`, `:103-113`, is unrelated and fine.)
8. **`api/chat/session-manager.ts:1-152` — a mirror of sidecar state** in the platform (`:29-32`),
with its own GC (`:126-138`) and teardown (`:83-88`). Two copies of the same truth.
9. **`api/chat/chat.ts:1-123` — session CRUD** (`:33-85`) plus hardcoded provider names (`:88-97`).
(`/chat/stt`, `:100-123`, is Whisper — a different domain that happens to live here.)
10. **`api/chat/list-models.ts:5-9` — the model catalogue is hardcoded in the platform:**
`claude-code/opus|sonnet|haiku` with context windows, max tokens and reasoning flags. This is the
sidecar's knowledge by definition; it changes when the CLI changes, not when Officer does.
11. **`api/server-settings/claude-code.ts:1-80` — installing and authenticating the CLI from the
platform:** `curl … install.sh | bash` (`:24`), `claude auth login` (`:45-47`),
`auth status` (`:56-61`), `--version` (`:9, :71`).
12. **`channels/send-claude-code.ts:1-68` — a third execution path**, running the SDK in-process, with
a subscription-lifetime rule at `:53-58`.
13. **`sidecar/protocol.ts:14-19, 34-43, 68-103` — nine claude message types in the shared protocol,**
including `ClaudeState` (`:70-73`, which exposes `proxySecret` and the whole session map) and
`ClaudeSpawnParams` (`:77-96`). Domain vocabulary in what should be transport.
14. **Smaller, still real:** `channels/send-and-await.ts:6, 53, 75-78`;
`hono.ts:79-86` — the `/api/hooks/claude-done` endpoint is **unauthenticated** (deliberate, since
the hook has no credential, but it means an unauthenticated local caller can nudge chat state);
`hono.ts:119`; `api/chat/retention.ts:1-22` (started from `bootstrap.ts:17`);
`queries/chat-events.ts:1-29` (read only by the platform);
`mcp-tool-server.ts:1-296`; `api/chat/logger.ts:1-62`.
**Dead code on this path, verified unimported (≈193 lines):** `api/task-logger.ts:1-101`; and
`api/anthropic-proxy.ts:1-92`, which reads `~/.claude/.credentials.json` (`:6, 18-27`) and mints
`sk-ant-api03-<uuid>` keys (`:10`) — the live copy is `sidecar/claude/proxy.ts:135`. A stale second
implementation of the credential path is worth deleting on security grounds alone, not just tidiness.
*What the sidecar already has right:* the Anthropic proxy genuinely lives in the PM2-managed sidecar
(`sidecar/claude/index.ts:20`), so the platform never holds an API key at rest, and `ANTHROPIC_BASE_URL`
points at the sidecar (`sidecar-registry.ts:234`). The credential path is roughly correct. It is the
process topology, the transport direction and the domain logic that are not.
## email — not compliant, and it is the exact inverse of the pattern
The worst of the eight by volume, and the only one where the arrow points backwards end to end:
**≈3,238 platform lines** (2,875 of them in seven files) against a **314-line sidecar** — and the
sidecar *imports platform code back out* (`sidecar/email/email-idle.ts:3` imports
`../../api/email/resync`). There is no proxy router, no `email:server` port event, and no forwarding of
any kind. `emailRouter` implements 18 concrete endpoints itself.
Read plainly: the sidecar is a cron/IDLE trigger, and the platform is the mail client.
1. **`api/email/email-db.ts:1-746` — the entire mail store lives in the platform.** SQLite schema
(`:9-55`), migrations with a `thread_id` backfill (`:167-217`), an FTS5 index (`:223-251`), a
hand-written **Gmail-style query parser** (`:253-349`), threading (`:81-132`), and — the part that
is unambiguously protocol knowledge — RFC-2047 decoding, quoted-printable, base64, charset
conversion, multipart walking and attachment extraction (`:496-746`). `openEmailDb` (`:134-154`)
carries a documented race at `:138-139`.
2. **`queue/handlers/gmail-sync.ts:1-712` — Gmail sync runs in the main process.** Gmail REST calls
(`:103-213`), label mapping (`:65-96`, `:329-396`), an IMAP first-sync path (`:398-546`), OAuth
refresh (`:586-608`). It runs in `officer` because `server.tsx:385` calls `initQueue()`
`queue/init.ts:6``handlers/index.ts:1-2`. So a full mailbox sync competes with request serving,
and dies on restart.
3. **`queue/handlers/email-sync.ts:1-381` — the IMAP sync, same placement.** `imapflow` (`:118`), a
label map (`:48-74`), UIDVALIDITY tracking, a ten-attempt reconnect loop (`:134-335`), a
Message-Id hash (`:40-44`), OAuth refresh (`:78-107`), and dock mutation (`:353-377`). A
ten-attempt reconnect loop is precisely the kind of long-lived stateful work sidecars exist for.
4. **`api/email/email.ts:1-414` — sending and reading.** SMTP send (`:16-69`) where the SMTP host is
derived by **string-munging the IMAP host** (`:24`); an SSE hub (`:94-146`); conversation
collapsing (`:167-202`); thread fetch (`:246-285`); row shaping (`:204-228`); attachment
extraction (`:287-318`); contacts (`:72-92`); stats (`:362-414`).
**Two unparameterised SQL interpolations:** `labels LIKE '%${folder}%'` at `:174` and `:366`.
The `folder` value reaches those lines from request input. Worth checking before anything else in
this section — it is the one item here that is a defect rather than a placement problem.
5. **`api/email/resync.ts:1-309` — shared mutable coordination across a process boundary that cannot
work.** `gmailResync` (`:43-63`), `imapResync` (`:148-264`), `resolveImapAuth` (`:118-146`),
`refreshCredentials` (`:21-41`), and `performResync` (`:276-284`) which coalesces concurrent
resyncs through an **in-process Map**. It is imported by both `sidecar/email/email-cron.ts:2` and
`email-idle.ts:3` *and* by `accounts.ts:159` — i.e. by two different processes. Each gets its own
copy of the Map, so the coalescing silently does nothing across the boundary. This is what
"importing platform code back out" costs.
6. **`api/email/accounts.ts:1-259` — account setup does live IMAP.** Validation by real connection on
create (`:87-121`), provider dispatch (`:180`), `resolveAuth` (`:231-259`), stale-status
reconciliation (`:38-85`), and **raw credentials written into queue job metadata** (`:198`,
`:200`) — so mail passwords land in a Postgres job row.
7. **Three channel handlers duplicate ~218 lines of email logic each:**
`channels/telegram/handler.ts:48-119` (`:53`, `:87`, enqueue at `:65`),
`channels/discord/handler.ts:47-122` (`:53`, `:89`),
`channels/whatsapp/handler.ts:52-121` (`:57`, `:91`). Same work, three times, none of it in the
sidecar.
8. **`api/integrations/google-auth.ts:1-61` + `integrations.ts:162-194` — a second Gmail client.**
Token exchange (`:6-33`), refresh-and-persist (`:38-61`), and at `integrations.ts:162-194` an
independent Gmail REST proxy. `:15-19` requests the `https://mail.google.com/` scope — full
mailbox access — and `:307-317` mutates the dock.
9. **`api/chat/websocket.ts:57-70``resolveEmailCwd` mkdirs email account directories** from inside
the chat socket. Cross-domain reach: the chat path owns email's filesystem layout.
10. **`data-path.ts:44-50` — the platform owns the mail store's paths**, with a second consumer at
`sidecar/claude/user-instance.ts:40, 75` (the `email_db` MCP tool). So three processes agree on a
layout by convention rather than by asking one owner.
11. **Glue (routing only, would survive a proxy):** `hono.ts:34` + `:114`; `server.tsx:20` + `:85-89`;
`protocol.ts:50-51` carries only `email:new`, which is correctly transport-shaped.
12. **`src/servers/sidecar/email-cron.ts` — 92 dead lines**, imported by nothing (the live one is
`sidecar/email/email-cron.ts`).
*Shortest path (inferred):* this one is a rewrite, not a move. The realistic first step is not
relocating `email-db.ts` — it is deleting the duplicate clients (items 7 and 8) and moving the two
queue handlers (items 2 and 3) into the sidecar so sync stops dying with `officer`. The store itself
can follow later, behind a proxy router.
## opencode — not compliant. ≈792 platform lines (≈1,424 counting the credential file)
Structurally the same failure as claude, at a third of the size — and with the largest proportion of
**dead code** of any sidecar: roughly 330 lines that can be deleted outright, because the sidecar
already does the same job. That makes this the cheapest of the non-compliant surfaces to improve.
1. **`api/chat/opencode/client.ts:1-208` — a full HTTP + SSE client in the platform**, including a
hand-rolled SSE parser (`:48-96`), REST wrappers (`:98-167`), wire shapes (`:171-195`), the
`metadata.officer` convention (`:179-182`), and a comment pinning it to **"opencode 1.17.9"**
(`:170`) — a version dependency in `officer`'s source. **`:16-96` and `:108-139` are dead.**
2. **`api/chat/opencode/event-mapper.ts:1-140` — entirely dead.** The sidecar already maps events at
`sidecar/opencode/runner.ts:150-205`. This is a stale duplicate of a sidecar concern.
3. **`api/server-settings/chat-providers.ts:1-632` — provider credentials handled in the platform.**
Reads and writes `~/.pi/agent/auth.json` (`:10`, `:334-346`) and `models.json` (`:11`, `:61-73`),
exposes API keys over GET/PUT (`:384-406`), and calls `api.anthropic.com`, `api.openai.com` and
`opencode.ai/zen/v1/models` with raw keys (`:444-471`). `PROVIDERS` at `:353-367`;
`invalidateModelCache()` at `:284, 293, 328, 404`; `:474-551` is stale `pi` install logic.
**Open question before moving any of it:** `AGENT_CONFIG_DIR = ~/.pi/agent` (`data-path.ts:23-24`)
may be dead config — I did not verify that the current opencode binary reads that path at all. If
it doesn't, a large part of this file is elaborate no-op and should be deleted rather than moved.
4. **`api/chat/opencode-sessions.ts:1-113` — transcript reconstruction** (`:50-82`), filtering on
`metadata.officer.cwd` (`:21`), epoch→ISO conversion (`:29-30`), and a synthesised
`opencode/<modelID>` model string (`:84`). Same shape as `claude-sessions.ts`: the platform reading
another program's session store.
5. **`api/chat/websocket.ts:443-518` (+ `:6, 18-20, 345-348, 570-584`) — `handleOpenCodeChat`**,
including deriving a session title from the prompt (`:490`) and selecting the harness by string
test.
6. **`channels/send-opencode.ts:29-66` — the terminal-event set (`:33-37`) and a resume policy keyed
on the `ses_` id prefix (`:42-45`).** Protocol knowledge encoded as a string prefix, in the
platform.
7. **`api/chat/list-models.ts:2, 11-59` — fetches `/config/providers` and then invents capability
metadata for the results (`:42-45`).**
8. **`api/chat/chat.ts:13-19, 44, 56-57, 68, 80-81, 91` — CRUD dispatch on `isOpenCodeSessionId`**
(`opencode-sessions.ts:112`, a `startsWith('ses_')` test).
9. **`sidecar-registry.ts:360-392` — typed opencode RPC**, same anti-pattern as the claude verbs.
10. **`sidecar/protocol.ts:20-22, 52-58, 105-113` — CLI flag mapping in the shared protocol**
(`--dir`, `--model`, `--session`). The platform's wire format encodes the sidecar's command line.
11. **More dead code:** `api/chat/opencode/server-manager.ts:7-23` (`isServerHealthy`, unused);
`api/chat/opencode/state.ts:13-15` (`clearOpenCodeSession`, unused). `state.ts:1-15` otherwise
holds a sessionKey→`ses_` Map — sidecar state mirrored in the platform again.
**The one compliant piece:** `api/chat/opencode/sidecar-server.ts:12-26` memoises the port the sidecar
reports — the slskd `sidecar-server.ts` pattern, already present and working. The proxy that should sit
on top of it was never written. (`:19-21` adds a session subscription, which is domain logic that
doesn't belong in a port memo.)
Verified clean: **no `OPENCODE_*` env var, no binary path, and no CLI spawn in the main process** — the
process boundary itself is right here, unlike claude. It is only the data and protocol knowledge that
leaked.
## pty — not compliant. 202 platform lines (228 with protocol) against a 73-line reference
The smallest violation in absolute terms and the **least compliant in proportion**: essentially the
entire platform-side file is sidecar logic. It is also the most tractable, because the fix template
already exists in the same codebase.
1. **`api/terminal/websocket.ts:1-169` — the whole file.** Broken out:
- **`:59-68` — the platform builds the `PtyInitConfig`:** `process.env.SHELL ?? '/bin/zsh'`,
`args: ['-i']`, and `host: true` **hardcoded** at `:61`. That last one means the sidecar's own
sandbox branch (`pty-sidecar.mjs:138-151`) is unreachable — dead code kept alive only by a
constant on the wrong side of the boundary.
- **`:38-43``resolveCwd` tilde expansion** in the platform.
- **`:71-89` — per-session fan-out.** The platform subscribes to a **global** `pty:output` /
`pty:exit` stream, filters by `msg.sessionId === sessionId`, and rewraps each frame. This is the
hard part of any fix, and **the template already exists** at `server.tsx:164-228`
(`devServerWebsocket` — a real byte relay).
- **`:106-143` — inbound translation**, including `:135` **synthesizing a shell command**:
`` `cd ${JSON.stringify(msg.path)}\r` ``. The platform is typing into the user's shell.
- **`:145-153` — detach-not-kill on disconnect.** Correct policy, and worth preserving: this is
why terminals survive a browser reload. Note that **`pty:close` has no sender anywhere in the
platform**, so there is currently no kill path at all.
- **`:51-54`, `:97-103` — ANSI-framed error text** composed in the platform.
- **`:23, 25-28, 30-36` — a duplicated session Map, `nextId` and `sendOutput`.**
- **`:158-169` — `broadcastPanelRefresh`**, a Claude concern parked in the terminal module; its
only caller is `hono.ts:83` (the `claude-done` hook).
2. **A placement anomaly worth fixing on its own:** `ecosystem.config.cjs:27-32` is the only sidecar
**outside `src/servers/sidecar/`**, the only **`.mjs`**, and the only one run by **`node`** rather
than `bun`. It has zero relative imports (`pty-sidecar.mjs:13-19, 31`) and does **not** use
`sidecar/connect.ts` — it carries **66 duplicated lines** (`:232-297`) including a byte-identical
backoff table (`:37` vs `connect.ts:22`). Its types are JSDoc (`:39`), so `protocol.ts:145-156` is
unenforced against it. The actual blocker to moving it is mundane: sibling `templates/` files on
disk (`:27-29, 61, 66, 71` — `.zshrc`, `.tmux.conf`, `starship-officer.toml`, and an unused
`.zshenv`). So a `git mv`, not a rewrite. *(Inferred: the `.mjs`/node choice is probably a
node-pty native-addon workaround — corroborated by the comment at `api/cliamp/websocket.ts:100`.)*
3. **Every PTY byte transits the main process, double-JSON-encoded.** Plus terminal-specific query
parsing in the shared upgrade handler (`server.tsx:248-252`, `WSData:51-52`) and wiring at `:7, 38,
143, 234, 335`. Auth at `:236-246` is correct. Identity ships **inside the payload** as
`userLabel` / `sessionId` (`websocket.ts:56, 65`) instead of as `X-Officer-User`.
4. **`sidecar-registry.ts:395-407` plus PTY types threaded through generic plumbing** at
`:12-13, 35, 93, 125, 153, 171, 180, 194`. One subtlety to preserve: the 30s
`sendPtyCommandAsync` timeout at `websocket.ts:96` is load-bearing backpressure for `pty:ready`.
5. **`hono.ts` has no terminal router at all** — `terminal` appears only at `:42`. There is nothing to
thin down; there is something to create.
6. **`data-path.ts` is bypassed:** terminal uses `process.env.HOME!` (`websocket.ts:63-64`) rather
than `getOwnerHomeDir` (`data-path.ts:34`), unlike the eight other host-executing surfaces. Same
result on this machine (`HOME_DIR` is set and equals `HOME`), divergent anywhere it isn't.
*Shortest path (inferred):* `git mv` the sidecar into `src/servers/sidecar/pty/` with its templates,
switch it to `connect.ts`, move the `PtyInitConfig` construction and cwd resolution into it, and
replace `websocket.ts` with the `devServerWebsocket` relay shape. The detach-on-disconnect policy moves
with it.
## vnc — not compliant. ~239 platform lines, 170 of them in `api/desktop/`
Structurally closer to compliant than most, and blocked on one concrete missing piece rather than a
pile of leaked logic.
1. **`api/desktop/websocket.ts:1-122` — session lifecycle plus a raw RFB TCP tunnel.** `startVnc`
(`:21-25`), `Bun.connect` to the VNC port (`:35-37`), both pump directions (`:41`, `:103`), a
`pendingMessages` buffer for pre-connect frames (`:9, 73-76, 97-100`), and close code `4004`
(`:30, 67, 87`).
**The blocker:** the vnc sidecar **exposes no HTTP or WS listener at all** — there is no
`Bun.serve` anywhere in `sidecar/vnc/`, and no `vnc:server` port event, unlike `slskd:server`
(`protocol.ts:64`), `vault:server` (`:62`) and `music:server` (`:60`). So the platform cannot
forward to it even in principle; the TCP tunnel exists because there is nothing else to talk to.
Adding a listener is the prerequisite for everything else here.
2. **`sidecar-registry.ts:409-439` — five VNC verbs.** `stopVnc` (`:427`) has **no callers**.
3. **`sidecar/protocol.ts:23-28, 44-49, 115-129` — four commands and five events**, with
`VncStartParams` (`:117-121`) and `VncSessionInfo` (`:123-129`) exposing `display` and `pid` to the
platform — internal process detail crossing the boundary.
4. **`api/desktop/rest.ts:11-29` — a `/vnc-password` fallback ladder** in the platform, when the
sidecar is already idempotent about this (`sidecar/vnc/index.ts:28-36` → `vnc-manager.ts:57`).
`:31-37` also invents its own response envelope.
5. **`api/desktop/vnc-config.ts:1-10` — the main process reads `~/.vnc/password` in plaintext**
(`:4, 7, 9`). The same layout is duplicated at `vnc-manager.ts:58-60`, so the sidecar already
knows it. This is the one security-relevant item on this path: a credential read that has no
reason to happen in `officer`.
6. **`server.tsx:309-312` — `/novnc/*` maps the request pathname onto `public/<pathname>` with no
auth and no traversal guard.** Flagging as observed, not diagnosed — whether it is exploitable
depends on how `pathname` is normalised before it gets there, which I did not trace. Worth a
deliberate look given it is the one unauthenticated static path in the file.
7. Wiring at `server.tsx:13, 45, 149, 234, 339` is fine, and **`hono.ts:37, 122` is already
reference-shaped** (two lines).
*Adjacent, and its own domain rather than a vnc violation:* the browser relay —
`server.tsx:369, 371` plus `api/browser/relay.ts` (677 lines), `api/browser/router.ts` (198, including
`Bun.spawn(['zip', …])` at `:26-30`), `cdp.ts` (99) and `relay-auth.ts` (42); and
`api/scrape/scrape.ts:9-19, 49+` launches chromium in-process. Noted for a future pass; not counted
above.
## Cross-cutting: the shared infrastructure
The per-sidecar sections above are symptoms. These are the four things that let the same mistake happen
eight times.
### 1. The proxy-size table
Sorted by how far each is from the reference. This is the whole audit in one view:
| sidecar | platform lines | verdict |
|---|---:|---|
| slskd | 70 | ✅ reference |
| music | 88 | ✅ compliant (the cliamp subsystem beside it is not) |
| pty | 169 | ✗ ~all of it is sidecar logic |
| vnc | 170 | ✗ blocked on a missing listener |
| vault | 499 | ◐ partial, security-relevant |
| opencode | 478 | ✗ (~330 lines deletable as dead) |
| claude | ~2,300 | ✗ |
| email | ~2,875 | ✗ no proxy exists at all |
`hono.ts` mounts **36 routers. Three are thin sidecar proxies** — `:106` (music), `:107` (slskd), and
`:77` (vault, mounted *outside* `protectedRouter`).
For contrast, sidecar-side LOC: music 1,630 · claude 1,523 · slskd 653 · opencode 427 · vnc 326 ·
email 314 · vault 295. Note the inversion on email: 314 sidecar lines to 2,875 platform lines.
And `api/*` by size, which shows where the mass actually sits: tasks 2,399 · chat 2,227 · email 1,782 ·
file-browser 1,465 · server-settings 1,452 · browser 1,105 · auth 607 · system-monitor 500 · vault 499
· … · desktop 170 · terminal 169 · music 88 · slskd 70.
### 2. The protocol is not a transport
`sidecar/protocol.ts` is a **closed union of ~34 message types: 7 transport, 25+ domain.** Every new
sidecar capability requires editing a shared platform file — which is why domain knowledge keeps
landing there (CLI flags, `display`/`pid`, `proxySecret`, spawn params).
Two specific consequences:
- **`protocol.ts:1` imports `ChatEvent` and `MessageCost` from `../api/chat/types`.** The transport
layer depends on one feature's domain model.
- **Nine `queue:*` message types are undeclared**, passed through by a `msg.type.startsWith('queue:')`
string test (`server.tsx:80`, `sidecar/email/index.ts:31, 38`). So the "closed" union is already
being bypassed where it was inconvenient — evidence that the closed shape is the wrong shape.
By contrast `registration-protocol.ts` (16 lines: `name` + `capabilities: string[]`) is genuinely
generic. The registration handshake got this right; the command channel did not.
### 3. Ten WebSocket providers, and only three are tunnels
`platform/CLAUDE.md` says eight; it is **ten** (add `vault` and `sidecar`). Of those, only **`vault`,
`desktop` and `dev-server`** are byte tunnels. **`task-runner`, `pipeline`, `cliamp` and `cliamp-audio`,
plus the browser relay, spawn and manage child processes inside the auth proxy — ~2,200 lines.** So the
"thin proxy" rule is violated by roughly as much code in the WS layer as in all the routers combined.
`api/desktop/` is the right model to copy (an opaque byte pump). `dev-server` is the right model for the
relay itself — including the detail that it defers JWT verification into `open()`
(`server.tsx:168-186`) because Bun's upgrade must be synchronous. Anyone building a claude or pty relay
will hit that same constraint.
Port discovery across all of them depends on **ES-module side-effect imports** (`hono.ts:27, 28, 29`,
`server.tsx:19`) — a sidecar's port is remembered because a module happened to be imported. Fragile,
and invisible at the call site.
### 4. Three copies of the queue engine, two of them dead
- `queue/init.ts` — 290 lines, **live**
- `queue/engine.ts` + `queue/index.ts` — 283 lines, **dead**
- `sidecar/queue-runner.ts` — 285 lines, **dead**
`queue/init.ts:8` comments that it was "moved from sidecar/queue-runner.ts" — i.e. **work migrated from
a sidecar back into the main process**, the opposite direction to the rule. The comments at
`bootstrap.ts:4, 15` are false. And **both live job kinds are email-domain and run in `officer`**, which
is item 2 of the email section arriving from a different direction.
### 5. Registry bugs that will bite during any migration
- **`unregisterSidecar` (`sidecar-registry.ts:80-85`) rejects the entire global pending-command map
when *any single* sidecar disconnects.** So restarting `officer-music` fails in-flight claude, pty and
vault commands. This will look like random unrelated breakage the moment sidecars restart
independently — which is the entire goal.
- **`:88-90` treats `capabilities.includes('proxy')` as "is this claude"** — true only by accident of
the naming confusion documented in `CLAUDE_SIDECAR_ISOLATION.md`.
### 6. What the database says (the clearest signal in the audit)
Table ownership tracks compliance exactly:
- **`queries/music.ts` and `queries/soulseek.ts` — read only by their sidecars.** Exemplary, and it is
no coincidence these are the two compliant proxies.
- **`queries/vault.ts` — platform-only**, including the three auth handlers
(`signout.ts`, `revoke-handler.ts`, `panic-handler.ts`).
- **`queries/chat-events.ts` — platform-only**, which is exactly what makes an `officer` restart lossy.
- **`queries/email-accounts.ts` — split**, with `api/chat/websocket.ts:11, 61-62` reaching across
domains into it.
**A useful rule falls out of this:** *if a table is read by exactly one sidecar and nothing else, that
sidecar is probably compliant. If the platform reads it, the platform probably owns logic it shouldn't.*
Cheaper to check than reading 3,000 lines.
---
**End of Pass 1 (backend / API).** Pass 2 covers the frontend — how each sidecar's UI talks to it,
mirroring the slskd findings at the top of this document.
# Pass 2 — frontend code
Same rule, applied one layer out. The question here is not "what logic runs in `officer`" but **"does
the browser know things only the sidecar should know?"** — upstream URL shapes, wire formats, session-id
conventions, retry and reconnect policy, capability catalogues.
The slskd case at the top of this document is the template: **37 raw `/slskd/api/v0/…` calls against 10
`/slskd/_officer/…` calls**, meaning the browser is a second client of the upstream API rather than a
client of Officer. Each section below asks the same question of one sidecar.
Ordered most-coupled first, not alphabetically — so the ordering differs from Pass 1 deliberately.
## claude / chat — the frontend is better than the backend
An unexpected result, and the most important single finding in Pass 2: **the chat frontend already
implements the reconnect-and-replay contract that the backend cannot honour.** Where the slskd browser
code is a second client of an upstream API, the chat browser code is a well-behaved client of Officer.
The leakage here is vocabulary, not architecture.
Main files: `officerdev/src/hooks/useChat.ts` (384), `hooks/src/useChatWebSocket.ts` (84),
`apps/Chat/types.ts` (75), `state/src/useClaudeSessions.ts` (97), `state/src/useModels.ts` (97),
`Chat/components/ModelSelector.tsx` (170), `ChatHistory/ChatDetailPanel.tsx` (156),
`ChatHistory/SessionList.tsx` (210), `Chat/EmbeddableChat/useEmbeddableChat.ts` (274),
`Chat/components/MessageBubble.tsx` (204).
### Reconnect and replay — verified working, and it changes the isolation plan
`hooks/src/useChatWebSocket.ts:46-58` auto-reconnects with linear-capped backoff
(`Math.min(5000, 300 * retry)`), resetting the counter on a successful open (`:33`). And the browser
already tracks a **sequence cursor**:
```ts
// useChat.ts:145-146
const seq = (data as { seq?: number }).seq;
if (typeof seq === 'number' && seq > cursorRef.current) cursorRef.current = seq;
```
```ts
// useChat.ts:282-285 — on every (re)connect once OPEN
const sid = sessionIdRef.current;
if (sid) sendRef.current({ type: 'resume-cursor', sessionId: sid, cursor: cursorRef.current });
```
The comment at `useChatWebSocket.ts:6-8` states the intent exactly: re-bind to the session and replay
events missed while briefly disconnected. On top of that, `sync:messages` rebuilds full transcript
state including re-arming a mid-stream buffer (`useChat.ts:209-237`, `:233-236`), and there is real
disconnected UI — a red "Disconnected" indicator (`ChatDetailPanel.tsx:38-52`), send disabled
(`InputArea.tsx:84`), model switching locked (`ModelSelector.tsx:67`).
**So `seq` + `resume-cursor` already exist end to end.** Pass 1 found the matching backend half at
`chat/websocket.ts:612-629` (`getChatEventsSince`). The protocol is not missing; the *writer* is simply
on the wrong side of the socket. That makes the durability stage of `CLAUDE_SIDECAR_ISOLATION.md`
substantially smaller than I estimated — a relocation, not a new mechanism.
One real gap: **a silent replay gap is undetectable by the browser.** `resume-cursor` is only sent when
`sessionIdRef.current` is already set, no `sync:messages` is forced on reconnect, and nothing validates
that the server actually had events from `cursor` onward. If the server's retained range were shorter
than the gap, the UI would show a seamless conversation with a hole in it. Worth a sequence-continuity
check when the writer moves.
### What the browser knows that it shouldn't
1. **Hardcoded model ids and provider names.** `state/src/useSettings.ts:86-87, 99` hardcodes
`'claude-code'` three times as a default. `state/src/useModels.ts:68, 96` branches access policy on
`m.provider === 'claude-code'` — twice. `Chat/components/ModelSelector.tsx:7-23` carries a
`PROVIDER_DISPLAY` map of 15 provider ids.
2. **A fully-qualified Claude model id, in an unrelated app.**
`FileBrowser/FileBrowserApp/components/TaskRunnerModal.tsx:1362` —
`useState(settings.tasks.defaultModel ?? 'claude-haiku-4-5-20251001')`. A dated upstream model
string, in the task runner. This one silently goes stale.
3. **The CLI invocation string is in the browser.** `apps/Terminal/index.tsx:32-33` —
`command="claude --dangerously-skip-permissions"`, `statePrefix="claude-code"`. The browser decides
how the agent binary is invoked, including its permission flag. *(The unsandboxed posture is
deliberate per `platform/CLAUDE.md`; the objection is only to where the decision lives — the
browser is the furthest possible place from the sidecar that owns it.)*
4. **Capability metadata crosses to the client.** `Chat/types.ts:11-19` types `contextWindow`,
`maxTokens` and `reasoning?`, and `ModelSelector.tsx:116` branches the UI on `reasoning`. The
browser doesn't compute these, so this is acceptable *if* they come from the sidecar — but Pass 1
found them hardcoded in the platform at `api/chat/list-models.ts:5-9`, so today the numbers
originate two layers away from the thing they describe.
5. **Claude CLI session conventions are documented in the browser.**
`state/src/useClaudeSessions.ts:8-9` comments that the id *is* the transcript filename;
`SessionList.tsx:10-11` explains that clicking a session continues it "via --resume"; `:15` types
`harness?: 'claude' | 'opencode'`. And the magic string **`'general_chat_sessions'`** — Claude's
own default directory bucket — appears as a literal in `PwdSelector.tsx:7, 22, 62`,
`ChatDetailPanel.tsx:86, 101` and `useClaudeSessions.ts:42`.
6. **`GET /chat/models` returns `hostHome`** (`useModels.ts:38`) — a host filesystem path handed to
the browser.
7. **Route balance: 7 chat-domain routes against ~6 generic ones.** `/chat/models`, `/chat/pwds`,
`/chat/sessions`, `/chat/sessions/:id` (with `?before=&limit=` pagination straight against the
transcript store), `/chat/sessions/:id/title`, `/api/chat/ws`. Against generic `/scrape`,
`/upload`, `/user/settings`, `/server-settings/chat-providers/*`, `/tasks/:id`. Unlike slskd these
are all Officer-owned paths — nothing bypasses the platform — but they are shaped around Claude's
session model rather than a neutral conversation resource.
8. **The event state machine is duplicated a third time — in the browser.** `Chat/types.ts:43-55`
declares a 12-member `ServerMessage` union and `useChat.ts:148-277` switches on all of it,
assembling deltas, tracking tool calls by `toolCallId`, and handling
`task:started` / `task:notification`. Pass 1 found the same interpretation in
`chat/websocket.ts:168-312` and again in `pipeline-executor.ts:151-201`. **Three
implementations.** The browser's copy is legitimate — it renders — but it means the wire format has
three consumers to keep in step.
### Verified clean
- **No JSONL parsing, no `leafUuid`, no `~/.claude` path construction in the browser.** Those stop at
the backend. The browser knows the concepts by name only.
- **No pricing table or cost formula client-side.** `MessageBubble.tsx:163-170` sums
`inputTokens + outputTokens` and formats a `totalUSD` it was handed (`Chat/types.ts:5-9`). Correct
placement.
- **Lifecycle control is two logical verbs, not process control:** `stop` (turn) and `disconnect`
(session) — `useChat.ts:352-360`. No PIDs, no process assumptions. This is exactly the abstraction a
relay-based architecture needs, and it already exists.
## email — routes are compliant, payloads and realtime are not
The mirror image of chat: **every one of the 20 API paths is Officer-shaped** — there is no
`/imap/uid/…` anywhere — but the request *bodies* carry IMAP configuration, the compose path builds
MIME, and the realtime channel cannot recover from a restart at all.
There is no windowed panel app; the email UI is screen-level under
`officer-web/Screens/Dashboard/Email/**` plus account setup in `Settings/`. ~2,451 lines total:
`EmailAccounts.tsx` (512), `Compose.tsx` (393), `EmailList.tsx` (377), `SMTPSection.tsx` (292),
`GoogleOAuthConfig.tsx` (288), `EmailReader.tsx` (224), `GoogleAccount.tsx` (98, **dead** — commented
out of `IntegrationsSettings/index.tsx:11, 71-79`), `EmailScreen.tsx` (60), `types/email.ts` (29).
1. **The browser owns IMAP connection parameters.** `EmailAccounts.tsx:21-37` holds `imapHost`,
`imapPort` (default `'993'`) and `imapSecure` as first-class client state; `:82` hardcodes the
Gmail preset `imap.gmail.com:993`; `:98-107` and `:124-133` POST `imapHost`/`imapPort`/`imapSecure`
/`authType`. **Provider detection is a hostname string comparison in the browser** (`:123`):
`const provider = form.imapHost === 'imap.gmail.com' ? 'gmail' : 'imap';`. Provider presets are
the sidecar's knowledge by definition.
2. **MIME construction in the browser.** `Compose.tsx:218-236` clones the editor DOM, rewrites each
inline `<img>` to `cid:inline-N`, and ships the parts — the browser authoring the `cid:` URI scheme
of a `multipart/related` message. Its own comment admits the contract is positional ("backend
matches by order"), which is a fragile coupling in both directions. Inline-vs-attachment is decided
by `f.type.startsWith('image/')` (`:176`, `:195`).
3. **No HTML sanitisation of message bodies.** `EmailReader.tsx:19-53` writes server-supplied HTML
directly into an iframe via `doc.write(...)`, with `sandbox="allow-same-origin"` and no
`allow-scripts` (`:52`). No DOMPurify or equivalent anywhere. So the **iframe sandbox is the only
thing preventing script execution from hostile mail** — it does hold, since scripts aren't
allowed, but it is a single un-defended layer, and `allow-same-origin` without `allow-scripts` is a
combination worth a deliberate second look rather than an accident.
4. **Two plaintext secrets are fetched into the browser and cached.**
- `SMTPSection.tsx:39` — `GET /server-settings/smtp` returns the SMTP `password` (and Resend
`apiKey`); `:63` writes it into state; `:208-213` binds it to a `type="password"` input. It lives
in the React Query cache under `['SMTP_CONFIG']` for the life of the tab.
- `GoogleOAuthConfig.tsx:197-210` — `GET /integrations/google/config` returns `clientSecret` in
plaintext; held in `useState` (`:184`), shown at `:255-262`.
Neither is a mail credential *the sidecar owns*, and both are the owner's own secrets on the
owner's own machine — but "GET returns the secret so the form can prefill" is the pattern worth
changing, since a write-only field would work identically.
5. **The session bearer token is passed in a URL.** `EmailList.tsx:110-112` builds
`new EventSource('/api/email/events?token=' + …)` from `localStorage`. Unavoidable for `EventSource`
(it can't set headers), but it puts the JWT into browser history and any proxy access log. Worth
noting that the chat socket does the same at `useChat.ts:121-123`.
6. **The realtime channel cannot survive a restart — explicit, not inferred.**
`EmailList.tsx:109-125` opens the SSE stream, expects `{ type: 'new-mail' }`, invalidates three
query keys, and closes on unmount. There is **no `es.onerror`, no backoff, no reconnect, and no
`Last-Event-ID` handling.** And the server never sends an `id:` field — Pass 1's
`api/email/email.ts:101` emits only `data: {"type":"new-mail"}` — so even the browser's *native*
`EventSource` retry cannot request replay. Any `new-mail` event emitted during a restart is lost
silently until the next event arrives or the user hits Sync manually (`:134-154`).
**Direct contrast with chat, in the same codebase: one channel has cursor-based replay, the other
has nothing.** That gap is the template for what to fix, and chat is the template to copy.
7. **Retry and polling policy decided in the browser, duplicated.** A fixed 5,000 ms account poll
while any account is `syncing`/`queued` — `EmailList.tsx:157-167`, duplicated verbatim at
`EmailAccounts.tsx:65-70`. Search debounce 300 ms (`EmailList.tsx:63-66`), contact autocomplete
180 ms (`Compose.tsx:28`). No backoff anywhere; a failed sync gets one `toast.error`.
8. **A client-side folder heuristic.** `EmailList.tsx:95-105`: if `inbox` returns zero but an `all`
probe finds mail, the browser silently switches folder. Business logic about mailbox state, decided
in the UI.
9. **Gmail knowledge, mostly as copy rather than behaviour.** `GoogleOAuthConfig.tsx:15-18` lists the
OAuth scopes (`gmail.readonly`, `calendar.readonly`) and `:104-111`, `:116` explain Google's
"restricted" classification — documentation text, not requests the browser makes.
`EmailScreen.tsx:13` puts Gmail-vs-local-DB routing into an **agent system prompt**
("Do not use the Gmail integration for questions about existing emails"), which is domain routing
logic expressed as English in the frontend. `Compose.tsx:241` knows Gmail files its own sent copy.
### Verified clean
- **Threading is server-side.** The browser receives `EmailThread` pre-grouped (`EmailReader.tsx:130-134`,
`types/email.ts:24-29`) and consumes precomputed `threadCount`/`threadUnread`. No participant
computation, no subject normalisation, no dedup. (`Compose.tsx:385-393` does `Re:` prefixing for a
reply draft — cosmetic.)
- **Folder names are Officer's, not IMAP's.** `EmailList.tsx:35-41` is a fixed
`inbox/all/sent/spam/trash` list passed as `folder=`; no special-use flag mapping.
- **No charset, quoted-printable, base64 or RFC-2047 decoding in the browser** — it receives decoded
`text`/`html`/`snippet`. Reading is compliant; only composing leaks.
- **No Gmail label ids and no Gmail query syntax constructed client-side.** The search box passes `q=`
through untouched (`EmailList.tsx:83-85`); `:309`'s placeholder only *hints* at the syntax.
- **Mail credentials are write-only.** The password is POSTed at `EmailAccounts.tsx:132` and never read
back — `GET /email/accounts` returns no credential field. OAuth tokens never reach the browser at
all: `:88-107` either redirects the page to `/api/integrations/google/authorize` or POSTs
`credentials: { userIntegrationId: true }`, a boolean. This is the right shape, and it is worth
noting that the *account* credential path is stricter than the *settings* ones in item 4.
- **No Message-Id handling** — and `Compose.tsx:382-384` documents the absence, noting `m.id` is a local
hash and that threading currently leans on `Re:` + participants.
## opencode — the most compliant frontend of the eight
Genuinely surprising given Pass 1 found ≈792 non-compliant *backend* lines. **The string `opencode`
appears in exactly four frontend files, and only one of those is logic.** Everything the backend leaks
— the `ses_` prefix, the `opencode/<modelID>` id shape, `metadata.officer`, `auth.json`,
`models.json`, the version pin — stops at the server. Verified by exhaustive grep: **zero frontend hits
for `ses_`, `opencode/`, `metadata.officer`, `auth.json`, `models.json`.**
The four mentions: `state/src/useClaudeSessions.ts:15` (`harness?: 'claude' | 'opencode'`),
`ChatHistory/SessionList.tsx:153-156` (renders an "OpenCode" badge),
`Chat/components/ModelSelector.tsx:22` (`opencode: 'OpenCode Zen'` display label), and
`Chat/EmbeddableChat/useEmbeddableChat.ts:108-109` — a **comment only**, explaining that `cwd` is sent
every turn because OpenCode rebuilds its working directory each turn. The code isn't gated on harness;
it just always sends `cwd`.
1. **Harness selection is not opencode-aware at all — it is `claude-code`-aware.** The only string
test is a Claude one, and everything else falls through to the opencode path:
`state/src/useModels.ts:68` (`if (m.provider === 'claude-code') return true;`) and `:96`. Both are
**access-policy bypasses**: Claude models always pass the admin policy filter; every
opencode-routed provider is subject to it. Worth knowing that's the actual semantic, since it isn't
obvious from the code.
2. **Session UI is shared, not forked.** `SessionList.tsx` / `ChatDetailPanel.tsx` serve both
harnesses off the same `/chat/pwds`, `/chat/sessions`, `/chat/sessions/:id`,
`/chat/sessions/:id/title` routes, and the browser treats `id` as an **opaque string** — no format
validation, no `ses_` regex, no branch on id shape. `ChatDetailPanel.tsx:96-98` resume/pagination is
harness-agnostic.
3. **The WebSocket machinery is fully shared with no opencode branch.**
`useChat.ts:121-123` builds one URL unconditionally; `hooks/src/useChatWebSocket.ts` takes only
`{url, onMessage, onOpen}`; the 12-member `ServerMessage` union (`Chat/types.ts:34-46`) has zero
opencode-specific variants. So the reconnect-and-replay work described in the chat section above
benefits both harnesses for free.
4. **API keys are never echoed to the browser — verified against the server.**
`AIHarnessesSection.tsx:57` GETs `/server-settings/chat-providers/api-keys`, but
`api/server-settings/chat-providers.ts:384-391` masks server-side first
(`value.slice(0,3) + '...' + value.slice(-3)`), and the browser uses the result only as a
placeholder (`AIHarnessesSection.tsx:467`). A freshly typed key lives transiently in
`keyInputs` state (`:74`) and is **deleted after the PUT** (`:121-125`). Never in `localStorage`,
`sessionStorage`, or the query cache. Local-provider config returns the auth *type* only, never key
material. **This is the pattern the email settings surface (Pass 2, email item 4) should copy.**
5. **No hardcoded model catalogue.** `state/src/useModels.ts:30-51` fetches everything from
`/chat/models`. The only hardcoded data is display-name maps — `ModelSelector.tsx:7-23` (14 pairs)
and a near-duplicate at `AISettings.tsx:19-34` (13 pairs, falling back to server-supplied
`providerNames` via `useModels.ts:18-20`). Two overlapping copies of a label map is a small
duplication, not a boundary violation.
6. **Naming debt, not coupling.** The shared hook is called `useClaudeSessions`, and `:9` comments
`id: string; // Claude session uuid (= transcript filename)` while that field also carries
opencode's `ses_…` ids; `:15` treats absent harness as claude; `:58` calls the store "Claude's own
transcript store". Accurate when written, misleading now.
7. **No dead opencode frontend code.** Verified importers for all four files — `ModelSelector.tsx`
(via `InputArea.tsx:92`, `ChatLauncher.tsx:107`, `TaskRunnerModal.tsx:202`), `SessionList.tsx`
(routed at `App.tsx:38-40`), `useClaudeSessions.ts`, `useEmbeddableChat.ts`. All reachable.
**One thing the frontend displays that isn't real, and the cause is in the backend.**
`api/chat/list-models.ts:24` stubs *every* opencode-routed model with constant metadata —
`contextWindow: 200000, maxTokens: 8192, reasoning: false, images: true`, with the comment "metadata is
left at neutral defaults for now". The browser faithfully renders these (`ModelSelector.tsx:116`
branches the thinking toggle on `reasoning`). So the capability numbers shown to the user for opencode
models are placeholders, and `reasoning: false` will suppress the thinking toggle for models that do
support it. A backend defect, surfaced by a compliant frontend.
## terminal / pty — the browser reconnects, and then loses the session anyway
The mirror of the backend result. Pass 1 called pty the least compliant *backend* surface; the frontend
is mostly well-behaved, has real reconnect logic, and yet contains **one bug that defeats the entire
detach-not-kill design.**
All terminal UI funnels through one component. `Terminal/Terminal.tsx` (329) holds every piece of logic;
`CommandTerminalWrapper.tsx` (59), `TerminalWrapper.tsx` (57), `HostTerminalWrapper.tsx` (50),
`Terminal/index.tsx` (76), `Headers.tsx` (79). Reused by `FileBrowser/CliampPanel.tsx` (36) and
`Settings/SystemSettings.tsx` (151). Verified: **no code editor, dev-server or task-runner panel embeds
a PTY** — the apparent matches were lucide icons and an unrelated `isTerminal(status)`.
### The session-orphaning bug
The backend deliberately detaches rather than kills, so a shell outlives a dropped socket. The browser
throws that away on any ordinary unmount:
```ts
// CommandTerminalWrapper.tsx:32-39 — and identically TerminalWrapper.tsx:35-42,
// HostTerminalWrapper.tsx:28-35
useEffect(() => {
return () => {
setTerminalsRef.current((prev) => {
const { [panelId]: _, ...rest } = prev;
return rest;
});
};
}, [panelId]);
```
That cleanup deletes the `panelId → sessionId` mapping when the panel closes, the workspace layout
changes, or a client-side route change unmounts the wrapper. The next mount calls
`crypto.randomUUID()` again (`CommandTerminalWrapper.tsx:28`, `TerminalWrapper.tsx:30`,
`HostTerminalWrapper.tsx:24`) and connects with a **new** session id — leaving the previous detached
shell running server-side with no reference to it, ever. Combined with Pass 1's finding that
**`pty:close` has no sender anywhere**, there is no kill path either: those shells accumulate.
So: reconnect works for network drops and tab backgrounding; **closing a panel silently leaks a shell.**
### Reconnect — implemented, and independent of the chat implementation
```ts
// Terminal.tsx:141-145
const MAX_RECONNECT_ATTEMPTS = 5;
const RECONNECT_DELAYS = [1000, 2000, 3000, 5000, 5000];
```
`handleClose` (`:235-254`) reschedules `connect` unless the process exited or the effect was disposed,
gives up after 5 attempts, then reports disconnected (`:249-253`); the counter resets on a successful
open (`:173`). There is also a **visibility-triggered reconnect** (`:272-280`) that resets the counter
and reconnects immediately when the tab is refocused. Because `connect()` rebuilds the URL from the same
`sessionId` prop (`:162`), every retry re-presents the same id — re-attachment is correct by
construction.
Note this is a **third, unrelated backoff implementation**: a 5-entry table here, `Math.min(5000, 300 *
retry)` in `useChatWebSocket.ts:46-58`, and a byte-identical copy of the sidecar table duplicated
between `pty-sidecar.mjs:37` and `connect.ts:22` (Pass 1). Four backoff policies, no shared helper.
**Scrollback survives, but the browser doesn't know it.** There is no replay message type at all. The
sidecar keeps a capped 50KB buffer (`pty-sidecar.mjs:36`, `BUFFER_MAX`) and re-emits it on re-init
(`:99-101`); the bridge forwards it as an ordinary `output` frame
(`api/terminal/websocket.ts:71-79`), and `Terminal.tsx:181` `term.write()`s it indistinguishably from
live output. No dedup, no historical marker. It works, passively. *(INFERRED: survival across a hard
page reload depends on React cleanup not running during navigation teardown — standard behaviour, but
not verified against `pagehide` here.)*
Session ids are **chosen by the browser** and persisted server-side through `useDashboardState` →
`GET/PATCH /dashboards` (React Query key `['DASHBOARD_STATE']`, `staleTime: Infinity`), so they survive
a reload. Key derivation is inconsistent across the three wrappers: `CommandTerminalWrapper.tsx:19` and
`HostTerminalWrapper.tsx:12` template `dashboardId` directly, while `TerminalWrapper.tsx:12-18`
regex-matches `ws-layout-(.+)` / `proj-layout-(.+)` to build its key. `SystemSettings.tsx:92` opts out
with an ephemeral `` `run-cmd-${Date.now()}` `` in local state — deliberate for a one-shot panel.
### Shell knowledge in the browser
1. **The browser composes shell commands by string concatenation, unescaped.**
```ts
// Terminal.tsx:187-190
const wrapped = onCommandDoneRef.current
? `${commandRef.current}; echo "${EXIT_MARKER}$?__"`
: commandRef.current;
ws.send(JSON.stringify({ type: 'input', data: wrapped + '\r' }));
```
That assumes a POSIX shell (`;`, `$?`, `echo`) and does not escape `command`. Same pattern at
`CommandTerminalWrapper.tsx:46-47`, which also does **tilde/home expansion in the browser** and
builds `cd <path> && <command>`:
```ts
const cwdPath = cwd && cwd !== '~' ? (cwd.startsWith('~') ? cwd : `~/${cwd.replace(/^\//, '')}`) : null;
const fullCommand = cwdPath ? `cd ${cwdPath} && ${command}` : command;
```
The inputs are the app's own hardcoded commands and a cwd from the workspace, not free user text —
so this is a correctness and placement problem rather than an injection hole today. It becomes one
the moment a command string is user-supplied.
2. **A sentinel protocol invented in the browser.** `Terminal.tsx:151`
(`EXIT_MARKER = '__OFFICER_EXIT_'`), a hand-rolled ANSI stripper at `:152-153` (CSI plus
BEL-terminated OSC only — misses ST-terminated OSC), and echo detection by
`l.includes(commandRef.current!.slice(0, 20))` at `:212`. Command completion is inferred by
scraping terminal output for a marker the browser injected.
3. **Hardcoded CLI invocations in the bundle:** `Terminal/index.tsx:13` (`tmux`), `:17` (`nvim`), and
`:32` — `command="claude --dangerously-skip-permissions"`. Each is then wrapped with the `cd` prefix
and the exit-marker suffix above and typed into the pty as keystrokes.
4. No hardcoded shell **path** in the browser — `/bin/zsh` lives at
`api/terminal/websocket.ts:62` (Pass 1, item 1a). Default cwd `'~'` comes from
`WorkspaceContext.ts:34`.
### Two cross-checks that confirm Pass 1 findings
- **The backend's `cwd` handler is unreachable.** Pass 1 flagged
`api/terminal/websocket.ts:129-138` for synthesizing `` `cd ${JSON.stringify(msg.path)}\r` ``.
Repo-wide grep finds **zero** frontend senders of `{type:'cwd'}` — the browser does its own `cd`
composition instead (item 1 above). So that branch is dead, and the capability it implements is
duplicated in the client.
- **`detached` is dead in the other direction.** `Terminal.tsx:225-226` handles a `'detached'` message
and writes `[Session taken over]`, but **no backend code ever emits it** — the only `detached` in
`src/servers` is an unrelated field at `activity/router.ts:98`. The frontend author modelled a
takeover concept the server never implemented.
Also dead: **`officerdev/terminal-host`** (`Terminal/index.tsx:47-54`, `HostTerminalWrapper.tsx`) is
registered `availableOnPanel: false` and referenced by no route, layout or panel map.
### Ongoing resize is not implemented
`fitAddon.fit()` is called exactly once per `connect()` (`Terminal.tsx:158`). There is **no
`ResizeObserver` and no `window` resize listener** in the file. The browser is the authority on
dimensions — it creates the terminal at a fixed 80×24 (`:120-121`), fits after layout settles, then
sends `cols`/`rows` both as query params (`:59-60`) and as an initial `resize` message (`:175`), and the
server never overrides them. **Consequence: resizing the window or dragging a panel splitter does not
re-fit or notify the server until the next reconnect.** Not an architecture violation — a real bug, and
the one item in this section a user would notice daily.
### Verified clean
- **No PIDs, no tmux internals, no process-lifetime or kill semantics in the browser.** `exit` carries
no code and the backend sends none (`api/terminal/websocket.ts:84`), so there is nothing to leak.
- **The browser never generates ANSI for rendering** — `term.write(msg.data)` (`:182`) hands raw bytes
to xterm.js. The only parsing is the sentinel stripper above.
- **`CliampPanel.tsx:25, 35` reuses the same component against `/api/cliamp/ws`** with a different
`wsPath` — evidence the client really is a generic PTY-over-WS view, agnostic to whether a shell or
mpv is on the far end. The env the cliamp sidecar sets (`api/cliamp/websocket.ts:112`) is never
surfaced to the browser.
## music — the compliant frontend reference
**12 routes, all Officer-owned `/music/…`, zero upstream-shaped calls.** Put beside slskd's 37-vs-10,
this is what the rule looks like when it's followed on both sides. Unsurprising given Pass 1 found the
music backend proxy compliant at 88 lines — and the pairing is the point, see the cross-cutting note
below.
Files: `MusicPlayer/MusicPlayerHost.tsx` (370), `apps/Music/MusicDetail.tsx` (363),
`MusicPlayer/gapless-engine.ts` (325), `apps/Music/MusicBrowser.tsx` (233),
`widgets/MusicPlayer/index.tsx` (200), `apps/Music/FavoritesView.tsx` (188),
`apps/Music/shared.ts` (123), `useMusicFavorites.ts` (52), `useMusicPlayer.ts` (47),
`MusicScreen.tsx` (44), `MusicHeart.tsx` (43).
Routes: `/music/manifest`, `/music/meta`, `/music/cover`, `/music/discography`, `/music/reindex`,
`/music/favorites` (GET/POST/DELETE), `/music/now-playing` (GET/PUT/DELETE), `/music/stream`. Plus the
generic `/file-browser/ls` for directory walking — deliberate per `MUSIC_API.md:11-13`, and the right
call: no music-specific listing endpoint was invented.
Documented but **never called by the frontend**: `/music/reindex/status`, `/music/reindex/stream`,
`/music/poster`, `/music/lyrics`.
What little leaks:
1. **`MUSIC_ROOT = 'Music'` hardcoded twice** — `shared.ts:4` and `widgets/MusicPlayer/index.tsx:12`. A
home-relative prefix convention, not a filesystem path (no absolute path, no env var in the
browser). Mild.
2. **The client re-sorts what the sidecar returns**, and says why: `shared.ts:63` comments that
"meta.json is in ffprobe/readdir order (arbitrary)", so `sortTracks` (`:65-75`) re-orders by track
tag. **The browser is compensating for the sidecar's output format.** The correct fix is in the
indexer, not the client.
3. **`AUDIO_EXT` duplicated verbatim** — `shared.ts:96` and `widgets/MusicPlayer/index.tsx:19` — a
9-extension allowlist used to filter raw `file-browser/ls` results. Plus an album-name convention
regex (`ALBUM_NAME_RE`, `shared.ts:12`) for `"[year] Album Name"`.
4. **A playback design decision worth surfacing, though not a compliance issue.**
`gapless-engine.ts:86-89` does a plain `fetch(url)` → `arrayBuffer()` → `decodeAudioData()` with
**no `Range` header**, downloading and decoding each whole file to PCM. Its own comment
(`:9-11`) puts a ~10-minute track at **≈200 MB in memory**, bounded only by `CACHE_MAX = 3`
(`:26`). So the byte-range support the API offers is never exercised by this path, and
`X-Audio-Duration` is ignored in favour of `AudioBuffer.duration` (`:199`, `:231`). A deliberate
trade for sample-accurate gapless transitions — just an expensive one.
Verified clean: **no codec, container, MIME, remux or MediaSource logic anywhere** (grepped —
zero hits); no cover-art filename convention (always the opaque `/music/cover?path=`, with `meta.cover`
used only as a boolean); no tag-format knowledge; no reindex internals (`POST /music/reindex` is a
black-box trigger and the status/stream routes are never read).
## cliamp — the browser hardcodes the sidecar's capture format
Two sockets, and they behave very differently. The control channel is exemplary; the audio channel is
the most tightly coupled thing in Pass 2.
`FileBrowser/AudioStreamPlayer.tsx` (164), `FileBrowser/CliampPanel.tsx` (36),
`FileBrowser/pcm-worklet-processor.js` (41, **dead** — zero importers; `AudioStreamPlayer.tsx:19-58`
inlines its own copy of the same worklet as a string and loads it via `Blob`/`createObjectURL` at `:60`).
### The control channel is genuinely opaque
`useFileBrowserApp.ts:443-446` sets `?play=<path>`; `CliampPanel.tsx:25` builds
`/api/cliamp/ws?files=<path>`; `:35` renders the **generic `TerminalView`**. That's the entire
integration. Verified: **the browser has no idea mpv is on the other end** — no keybinding table, no
command syntax, no output parsing. Keystrokes are forwarded byte-for-byte
(`Terminal.tsx:265-269`) and output goes straight to xterm's generic ANSI parser (`:182`).
The consequence is a UI observation rather than a violation: **there are no transport controls.** No
play/pause, seek or volume — `CliampPanelHeader` (`CliampPanel.tsx:7-19`) shows only the filename split
off the URL (`:10`). The user reads mpv's own status line as rendered raw. That is the honest cost of a
fully opaque tunnel, and worth deciding deliberately rather than by default.
### The audio channel hardcodes the wire format on both sides independently
The sidecar spawns `parec --format=s16le --rate=44100 --channels=2` (`api/cliamp/audio-ws.ts:29`) and
pipes raw bytes with **no framing and no header** (`:57`). The browser hardcodes the matching
assumptions, with **no negotiation or handshake of any kind**:
```ts
// AudioStreamPlayer.tsx:9-10
const SAMPLE_RATE = 44100;
const CHANNELS = 2;
```
- `s16le` is implied by `new Int16Array(ev.data)` (`:108`) and `int16[i] / 32768` (`:111`).
- `new AudioContext({ sampleRate: SAMPLE_RATE })` (`:76`) — the browser **forces** its output rate to
match rather than resampling.
- Ring buffer capped at `SAMPLE_RATE * CHANNELS * 2` samples (`:30`) ≈ 4 seconds.
- Graph: WS binary → Int16→Float32 on the main thread → `postMessage` → `AudioWorkletNode`
(`pcm-processor`, deinterleaving at `:42-48`) → `GainNode` (mute) → destination.
Change the `parec` flags in the sidecar and the browser plays noise. Two files, no shared constant, no
version marker. This is the clearest single instance in Pass 2 of a sidecar's internal format being
duplicated into the client — and cliamp is precisely the subsystem Pass 1 found **has no sidecar at
all**.
**The audio socket has no reconnect.** `AudioStreamPlayer.tsx:121-123` handles `close` by flipping a UI
flag and nothing else; the effect only reconnects if the `wsUrl` prop identity changes (`:143`) or the
component remounts. A dropped capture socket stays dead until the panel is closed and reopened. The
control socket, by contrast, inherits `TerminalView`'s full backoff — so in the same panel, the terminal
comes back and the audio doesn't.
Also: the music and cliamp paths share no code despite both ending in Web Audio.
## vault — there is no vault frontend
Worth stating plainly rather than padding: **zero lines of client code.** Six grep passes across
`src/workspaces/officerdev/src/apps/**`, `src/apps/officer-web/**` and the shared workspaces for
`vault|bitwarden|cipher|kdf|PBKDF2|Argon2|crypto.subtle|masterPassword|unlock|/api/vault|TOTP` produced
only false positives: `helpers/slug.ts:25` (the word "vault" in a slug word list), a base64 audio blob,
and `components/ui/input-otp.tsx` (the generic shadcn primitive, matched on the substring in
`InputOTP`). `AppRegistry.tsx:22` lists every windowed app — FileBrowser, Terminal, CodeEditor, Chat,
FileViewer, Dashboards, Projects, ChatHistory, Preview, Widgets, Desktop, Music, Soulseek,
SystemMonitor — and there is no Vault entry. Nothing in `Settings/**`, nothing in the sidebar, nothing
in `TODO.md`.
So: no client-side crypto, no key custody, no decrypted-secret handling, no lock state machine, no
`/api/vault` calls, no WebSocket client. Nothing to assess.
**But this reframes a Pass 1 finding, and it matters.** Pass 1 credited the vault with getting one
thing right: "user-key derivation is genuinely client-side (the platform only relays `Kdf*` params at
`router.ts:96-101`)". Since Officer has no vault client, **the client doing that derivation is a
third-party Bitwarden app.** Which means the platform-side login broker, token store, WebSocket token
injection and at-rest crypto documented in Pass 1 exist to serve **external clients**, not Officer's own
UI. That is a materially different security story than "our own frontend needs this", and it should be
settled before any of that code is moved: who is actually meant to talk to the vault?
---
# Cross-cutting findings from Pass 2
## 1. The inverse correlation — and the mechanism behind it
The single most useful thing in this pass. Ranked by frontend compliance:
| sidecar | backend verdict (Pass 1) | frontend verdict (Pass 2) |
|---|---|---|
| slskd | ✅ compliant, 70 lines | ✗ **worst** — 37 raw upstream calls vs 10 Officer routes |
| music | ✅ compliant, 88 lines | ✅ 12 routes, all Officer-owned |
| opencode | ✗ ≈792 lines | ✅ **best** — 4 mentions, 1 of them logic |
| claude | ✗ ≈2,300 lines | ✅ mostly clean; vocabulary leaks only |
| email | ✗ ≈2,875 lines, no proxy | ◐ routes clean, payloads and realtime not |
| pty | ✗ ~all of 169 lines | ◐ clean boundary, but shell composition + a session leak |
| vnc | ✗ blocked on a missing listener | ✅ 149 lines, one route, no internals |
| vault | ◐ partial | — no frontend exists |
**A thin proxy alone does not produce a clean frontend — it relocates the problem.** When the platform
translates (claude, opencode, email), the browser receives Officer-shaped data and stays clean at the
cost of thousands of platform lines. When the platform is a dumb pass-through (slskd), the browser has
to speak the upstream API itself. Music is the only case that escapes the trade, and it escapes it
because **the sidecar exposes Officer-shaped routes** — the `/_officer/*` namespace — rather than only
proxying the upstream one.
So the rule as stated ("main server is a thin proxy") is necessary but not sufficient. The complete
version is: *the sidecar owns the contract the browser consumes.* Thinning a router without adding
`/_officer/*` routes to the sidecar just moves domain logic from the platform into the browser, which is
strictly worse — it is further from the data and unversioned.
## 2. Resilience is inconsistent, and that is the frontend's real problem
Pass 1 found an architecture problem. Pass 2 mostly finds a **resilience** problem — and it is
per-socket rather than systemic:
| channel | reconnect | replay |
|---|---|---|
| chat WS | ✅ `min(5000, 300 × retry)` | ✅ `seq` + `resume-cursor` (best in repo) |
| terminal / cliamp control WS | ✅ 5-entry table + visibility trigger | ◐ passive 50 KB sidecar buffer; browser unaware |
| cliamp audio WS | ✗ none | — n/a (live capture) |
| desktop / VNC WS | ✗ **none at all** | — n/a |
| email SSE | ✗ none, and no `id:` field server-side | ✗ so native `EventSource` retry can't replay either |
Two channels have thought-through recovery; three have none. The chat implementation is the template
and it already works — see the isolation document, where this finding shortened the plan.
**Four independent backoff implementations, no shared helper:** `useChatWebSocket.ts:46-58`,
`Terminal.tsx:141-145`, and the byte-identical pair `pty-sidecar.mjs:37` / `connect.ts:22` (Pass 1).
## 3. The bearer token travels in URLs on every socket
`useChat.ts:121-123` (chat WS), `Terminal.tsx:53-55` (terminal WS), `DesktopView.tsx:13-17` (VNC WS),
`EmailList.tsx:110-112` (email SSE) all read `localStorage.getItem('BEARER_TOKEN')` and append
`?token=…`. Unavoidable for `EventSource` and awkward for `WebSocket` (neither can set headers), and
this is the owner's own machine behind their own HTTPS proxy — but it does put a 30-day JWT into browser
history and any proxy access log, on four separate paths. If it's ever worth fixing, a short-lived
single-use ticket exchanged for the socket is the usual shape.
Related, from Pass 1: `server.tsx:308-311` serves `/novnc/*` with **no auth**, which Pass 2 confirms is
how the vendored noVNC client (`public/novnc/rfb.js`, 3,415 lines + ~40 modules) is loaded, via a
memoized dynamic `import()` (`DesktopView.tsx:28-37`).
## 4. Credential handling in the browser — one clear right answer, two wrong ones
- ✅ **VNC password**: fetched once into a local closure variable (`DesktopView.tsx:55`), passed to RFB
(`:83-85`, `:104-106`), never in state, storage, the query cache, or a URL.
- ✅ **Provider API keys**: masked server-side before the GET
(`api/server-settings/chat-providers.ts:384-391`), used only as a placeholder
(`AIHarnessesSection.tsx:467`), and the typed value is deleted from state after the PUT (`:121-125`).
- ✅ **Mail account passwords / OAuth**: write-only; the account list returns no credential, and OAuth
tokens never reach the browser (`EmailAccounts.tsx:88-107`).
- ✗ **SMTP password / Resend API key**: `GET /server-settings/smtp` returns them in plaintext and they
live in the React Query cache under `['SMTP_CONFIG']` for the life of the tab
(`SMTPSection.tsx:39, 63, 208-213`).
- ✗ **Google OAuth client secret**: returned in plaintext to prefill a form
(`GoogleOAuthConfig.tsx:197-210, 184, 255-262`).
The masking pattern in the chat-providers route is the fix for both failures, and it already exists in
this codebase.
## 5. Dead frontend code, verified by grepping for importers
- `FileBrowser/pcm-worklet-processor.js` (41) — zero importers; the worklet is inlined instead.
- `Settings/IntegrationsSettings/GoogleAccount.tsx` (98) — commented out of
`IntegrationsSettings/index.tsx:11, 71-79`.
- `officerdev/terminal-host` (`Terminal/index.tsx:47-54` + `HostTerminalWrapper.tsx`) — registered
`availableOnPanel: false`, referenced by no route, layout or panel map.
- `Terminal.tsx:225-226` — handles a `'detached'` message **no backend ever sends**.
- `GET /desktop/vnc-status` (`api/desktop/rest.ts:31-38`) — defined, never called.
- `api/terminal/websocket.ts:129-138` — the `cwd` handler; **no frontend sender exists** (the browser
composes its own `cd`), confirming Pass 1's flag from the other direction.
## 6. Duplicated constants across the frontend
`PROVIDER_DISPLAY` twice (`ModelSelector.tsx:7-23`, 14 entries; `AISettings.tsx:19-34`, 13 entries);
`AUDIO_EXT` twice (`shared.ts:96`, `widgets/MusicPlayer/index.tsx:19`); `MUSIC_ROOT` twice
(`shared.ts:4`, `widgets/MusicPlayer/index.tsx:12`); the PCM worklet twice (one copy dead). Small, but
each is a place where two files must be changed together and nothing enforces it.
## 7. Three bugs a user would notice, none of them architectural
Recorded because they surfaced during the audit, not because they're in scope:
1. **Closing a terminal panel leaks a shell.** `CommandTerminalWrapper.tsx:32-39` (and the two sibling
wrappers) delete the `panelId → sessionId` mapping on unmount, so the next mount generates a new
uuid and the detached shell becomes unreachable — with no kill path, since `pty:close` has no sender.
2. **Terminals don't re-fit after a resize.** `fitAddon.fit()` runs once per `connect()`
(`Terminal.tsx:158`); there is no `ResizeObserver` or window listener, so dragging a splitter leaves
the pty on stale dimensions until the next reconnect.
3. **opencode model capabilities shown to the user are placeholder constants.**
`api/chat/list-models.ts:24` stubs every opencode model at `contextWindow: 200000, maxTokens: 8192,
reasoning: false`, and `ModelSelector.tsx:116` hides the thinking toggle based on that `false`.
---
**End of Pass 2.** Both passes are complete. Nothing in either document has been changed in code.