delete the dead serve-turn client, and answer phase 2's blocking question first
Phase 1 item 7, done in the order the parity doc asks: read the dead design into a note, then delete it. `docs/opencode-serve-path.md` records what `event-mapper.ts` and the SSE half of `client.ts` did, and what a rebuild would want back from them — the delta model and tool-state transitions, which are exactly parity Phase 3's token streaming rather than new work. Deleted: `event-mapper.ts` entirely, and `subscribe`/the shared `GET /event` SSE loop, `createSession`, `postMessage`, `abort` from the client, plus `isServerHealthy` from server-manager. All had no callers. `client.ts` goes 200-odd lines to 99. What stays is the REST reads the chat list and transcript use: listSessions, getSession, getMessages, deleteSession, renameSession. While in there, Phase 2's blocking question turned out to be cheap to settle, so it is answered rather than left open. The review asked whether the serve can take a per-request directory, since without one a serve-based turn path would reintroduce the single-directory coupling that shelved this work: POST /session?directory=/tmp/oc-phase2-probe -> directory: "/tmp/oc-phase2-probe" honoured POST /session with directory in the BODY -> directory: "<serve cwd>" ignored It is a query parameter on every /session* route. So the coupling is gone on both architectures and the blocker is cleared. The note does NOT start the migration: which of the three options to take is a product call, and it lays them out rather than presuming one. The first probe put `directory` in the body and appeared to prove the opposite. Recorded in the note, because it is the obvious way to test this and it gives a confident wrong answer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -0,0 +1,120 @@
|
||||
# The serve-based turn path: what it was, and whether to rebuild it
|
||||
|
||||
Written 2026-08-10, before deleting `event-mapper.ts` and the unused half of
|
||||
`api/chat/opencode/client.ts`. `docs/opencode-parity.md` item 7 asks for exactly this: read the dead
|
||||
design into a note first, because Phase 2 may rebuild it.
|
||||
|
||||
It also answers Phase 2's blocking question, which turned out to be cheap to settle.
|
||||
|
||||
---
|
||||
|
||||
## The question Phase 2 was waiting on
|
||||
|
||||
`docs/opencode-phase0-review.md` narrowed the fork to one thing:
|
||||
|
||||
> Moving turns onto the serve would reintroduce exactly the single-directory coupling that caused the
|
||||
> original pain, **unless the serve's API can take a per-request directory**. That question — not "why
|
||||
> was it replaced" — is now the one to answer first.
|
||||
|
||||
**It can. Tested against the running serve (opencode 1.17.9 on this machine):**
|
||||
|
||||
```
|
||||
POST /session?directory=/tmp/oc-phase2-probe → { directory: "/tmp/oc-phase2-probe" } ✅ matches
|
||||
POST /session body { directory: … } → { directory: "<serve cwd>" } ❌ ignored
|
||||
```
|
||||
|
||||
`directory` is a **query parameter on every `/session*` route** — create, message, abort, fork,
|
||||
summarize, prompt_async, shell, all of them — alongside a `workspace` param. It is not a body field,
|
||||
which is why a first probe that sent it in the body appeared to disprove the whole idea.
|
||||
|
||||
**So the coupling is gone on both paths.** `opencode run --dir` anchors a subprocess turn (verified in
|
||||
the phase-0 review), and `?directory=` anchors a serve turn. The single-server constraint that shelved
|
||||
this work does not exist in either architecture any more.
|
||||
|
||||
That removes the reason not to move. It does not by itself decide the move — see the trade below.
|
||||
|
||||
---
|
||||
|
||||
## What the dead code actually was
|
||||
|
||||
Two files, ~200 of ~390 platform-side lines, all reachable from nothing:
|
||||
|
||||
**`event-mapper.ts` (146 lines)** — maps OpenCode's SSE event stream to officer's `ChatEvent`s. Handles
|
||||
`message.part.updated` (text deltas, tool state transitions), `message.updated`, `session.idle` and
|
||||
`session.error`. Its shape assumes a _streaming_ source: partial text arriving as deltas, tool calls
|
||||
transitioning pending → running → completed as separate events.
|
||||
|
||||
**The SSE half of `client.ts`** — `subscribe(sessionId, listener)`, one shared `GET /event` stream per
|
||||
base URL demultiplexed to per-session listeners, with reconnect. Plus `createSession`, `postMessage`,
|
||||
`abort`, `isServerHealthy`.
|
||||
|
||||
The live half of `client.ts` stays: `listSessions`, `getSession`, `getMessages`, `deleteSession`,
|
||||
`renameSession`, all used by `opencode-sessions.ts` for the chat list and transcript reads.
|
||||
|
||||
### Why this matters for a rebuild
|
||||
|
||||
The dead mapper is **not** a sketch to be dusted off — it is a finished, working shape for a design that
|
||||
was measured against a real event stream. Two things in it are worth keeping if the serve path returns:
|
||||
|
||||
1. **The delta model.** `runner.ts` emits whole `text` blocks because `opencode run --format json` emits
|
||||
whole blocks; the mapper emits deltas because SSE emits deltas. Token streaming (parity Phase 3, item 11) is not new work on the serve path — it is this file.
|
||||
2. **Tool-state transitions.** The mapper tracks a tool call across pending/running/completed. The
|
||||
subprocess path only ever sees the finished call.
|
||||
|
||||
Both are recoverable from git after deletion (`5d077a4` is the last commit where the serve path was
|
||||
live), which is the argument for deleting rather than keeping it compiled-but-unreachable: an unused
|
||||
file rots silently against a moving API, and this one is already pinned to a version two minor releases
|
||||
behind what some machines run.
|
||||
|
||||
---
|
||||
|
||||
## The trade, now that the blocker is gone
|
||||
|
||||
**Keep the subprocess (`opencode run --dir`)**
|
||||
|
||||
- No rewrite; it works today.
|
||||
- Permanently forfeits: token streaming, mid-turn injection, background tasks, live-session enumeration,
|
||||
reattach-by-id, interrupt-without-teardown. Every one of those is a `stdin: 'ignore'` consequence.
|
||||
- One process per turn, no warm state to leak or garbage-collect.
|
||||
|
||||
**Move turns onto the serve (`POST /session/{id}/message?directory=…`)**
|
||||
|
||||
- Unblocks the whole of parity Phase 3 at once — those six capabilities are all downstream of a
|
||||
persistent, addressable session.
|
||||
- Re-adopts an SSE stream officer must keep alive, demultiplex and reconnect. That machinery already
|
||||
exists in the deleted code, so the cost is smaller than it looks.
|
||||
- Introduces warm sessions and therefore a lifetime question OpenCode currently does not have: idle GC,
|
||||
orphan adoption after a restart, the same problems the Claude path spent months getting right.
|
||||
- The serve becomes load-bearing rather than a convenience. Today a serve crash costs session listing;
|
||||
then it would cost every turn in flight.
|
||||
|
||||
**A third option, not in the parity doc:** move only what needs the serve. Keep `opencode run` for turns
|
||||
and add `?directory=`-scoped serve calls for enumeration and reattach. That buys the Live panel and
|
||||
reattach-by-id without warm sessions or an SSE loop. It does not buy streaming or mid-turn injection,
|
||||
which are the two most visible gaps.
|
||||
|
||||
## Recommendation
|
||||
|
||||
**Do not start the migration on this pass.** The blocker is cleared and that is the deliverable here;
|
||||
choosing between the three is a product call about how much OpenCode should behave like Claude, and it
|
||||
should be made deliberately rather than as a side effect of a cleanup branch.
|
||||
|
||||
If it is taken: the third option first. It is incremental, it is the only one with no lifetime
|
||||
questions, and it makes the Live panel — which today silently omits every running OpenCode turn — tell
|
||||
the truth.
|
||||
|
||||
---
|
||||
|
||||
## Verified facts this note rests on
|
||||
|
||||
| Claim | How |
|
||||
| ---------------------------------------------------------------------------------- | --------------------------------------------------------- |
|
||||
| `?directory=` on `POST /session` is honoured | Created a session, read `directory` back: matched |
|
||||
| A body `directory` is ignored | Same call with the field in the body: got the serve's cwd |
|
||||
| `directory` is on every `/session*` route | Read the serve's own `/doc` (OpenAPI) |
|
||||
| `event-mapper.ts` has no importers outside `client.ts` | grep |
|
||||
| `createSession`/`postMessage`/`abort`/`isServerHealthy` have no callers | grep for call sites |
|
||||
| `listSessions`/`getSession`/`getMessages`/`deleteSession`/`renameSession` are live | all from `opencode-sessions.ts` |
|
||||
|
||||
Machine note: this server runs opencode **1.17.9**; the phase-0 review's tests ran against **1.18.11**
|
||||
elsewhere. The `?directory=` result above is from 1.17.9, so it holds on the older of the two.
|
||||
Reference in New Issue
Block a user