pastilhasandClaude Opus 5
140288031a
Implementation report for whoever wrote opencode-parity.md and opencode-phase0-review.md: what landed, the three places the specs were wrong and how the reproduce-first rule caught each, the three places I deliberately did not follow them, and what is unverified. Phase 2's blocking question is answered in full — the serve takes a per-request `?directory=`, so the coupling is gone in both architectures. The migration is not started; that decision is framed in opencode-serve-path.md and left open. Flags `opencode:list` as the one new capability whose happy path is unproven, and B7 as newly more masked: the B2 fix makes the client send `model` more reliably, which hides the spurious cut-off rather than removing it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
148 lines
8.1 KiB
Markdown
148 lines
8.1 KiB
Markdown
# Phases 0 and 1: implementation report, and Phase 2's question answered
|
|
|
|
**To:** whoever wrote `opencode-parity.md` and `opencode-phase0-review.md`.
|
|
**From:** the implementation pass, 2026-08-10 (overnight).
|
|
**Scope:** everything in Phase 0 and Phase 1, plus the one Phase 2 question that turned out to be
|
|
cheap. Nothing of the Phase 2 migration was started.
|
|
|
|
Both documents were good to work from. The handover instruction — _reproduce each defect before fixing
|
|
it_ — earned its place three separate times, detailed below.
|
|
|
|
---
|
|
|
|
## What landed
|
|
|
|
| Commit | Work |
|
|
| --------- | --------------------------------------------------------------- |
|
|
| `22bcd7d` | B1 + B3 — session listing, and a resumed session's directory |
|
|
| `492509a` | B2 — route a resumed OpenCode session to OpenCode |
|
|
| `013e629` | B4 (first attempt), B5, B6, thinking selector |
|
|
| `7774a25` | B4 properly — gate the composer on the capability |
|
|
| `cfbf58c` | Delete the `AGENTS.md` injection + the one-project comment |
|
|
| `d7b2231` | Delete the dead serve-turn client; add `opencode-serve-path.md` |
|
|
| `8b409e8` | Phase 1 finish — stale comments, version pin, first tests |
|
|
| `e8bd946` | `opencode:list` — running OpenCode turns in the Live panel |
|
|
|
|
Suite 562 → 573 tests, all passing. `tsgo` clean throughout.
|
|
|
|
---
|
|
|
|
## Phase 2: the blocking question is answered
|
|
|
|
`opencode-phase0-review.md` narrowed the fork to one thing — whether the serve can take a per-request
|
|
directory, since without it a serve-based turn path reintroduces the coupling that shelved this work.
|
|
|
|
**It can.** Against the running serve (1.17.9 on the home-lab box):
|
|
|
|
```
|
|
POST /session?directory=/tmp/oc-phase2-probe → directory: "/tmp/oc-phase2-probe" honoured
|
|
POST /session with directory in the BODY → directory: "<serve cwd>" ignored
|
|
```
|
|
|
|
`directory` is a **query parameter on every `/session*` route** — create, message, abort, fork,
|
|
summarize, prompt_async, shell — alongside `workspace`. Read off the serve's own `/doc`.
|
|
|
|
**Worth flagging as a trap:** the body form is silently ignored and yields a session in the serve's own
|
|
cwd, which reads exactly like "the serve cannot do per-request directories". My first probe did this and
|
|
gave a confident wrong answer. Anyone re-checking this should use the query string.
|
|
|
|
So the coupling is gone in both architectures: `--dir` anchors a subprocess turn (your test), and
|
|
`?directory=` anchors a serve turn (this one). `docs/opencode-serve-path.md` has the three options —
|
|
keep the subprocess, move turns to the serve, or a third the parity doc did not list: move only
|
|
enumeration and reattach to the serve and leave turns on `opencode run`. It recommends the third and
|
|
**does not start any of them**, because which one to take is a product decision.
|
|
|
|
The first slice of that third option is already in (`e8bd946`, below).
|
|
|
|
---
|
|
|
|
## Where your documents were wrong, and how
|
|
|
|
Three corrections. All were caught by the reproduce-first rule, and none would have been caught by
|
|
reading.
|
|
|
|
**1. `location.directory` does not exist** (parity doc B1/B3, and its suggested fix). opencode 1.17.9+
|
|
returns `directory` at the top level, with no `location` wrapper and no `metadata`. The suggested fix —
|
|
"derive the directory from `location.directory`" — would have compiled and silently produced `''`
|
|
forever. You have since corrected this in `29d9912`.
|
|
|
|
**2. B2 is right in outline, and the interesting part is not in the outline.** The doc says
|
|
`selected.model` is never passed. In fact it _is_ produced correctly — `/chat/:id` resolves
|
|
`detail.model` to `opencode/big-pickle` and puts it on `selected` — and then dropped at the `<NewChat>`
|
|
boundary, while `initialModel` reads `locationState?.model`, a field **nothing in the tree ever writes**.
|
|
So the value existed and was read from the wrong place. Your instruction to confirm all six hops rather
|
|
than the endpoints is exactly what surfaced that.
|
|
|
|
**3. `runner.ts`'s mapping was untested but not pure** (Phase 1 item 9). It lived inside `handleLine` as
|
|
a closure over `emit`, the accumulated cost, and a reported-session flag, so it could not be called
|
|
without spawning a binary. Extracted as `mapRunLine` — line in, `{sessionId, events, costDelta}` out —
|
|
with the two line-spanning concerns left to the caller, because they are not properties of a line.
|
|
Behaviour unchanged; 11 tests now pin it.
|
|
|
|
---
|
|
|
|
## Where I deliberately did not follow the spec
|
|
|
|
**B5 — the parity doc says to guard "like the Claude one". Copying that guard would have broken
|
|
OpenCode.** Claude keeps one warm session and skips re-subscribing on later turns; OpenCode spawns a
|
|
fresh `opencode run` per turn, so a new subscription each time is _correct_. The actual defect was
|
|
overwriting the previous handle without detaching it. Fixed that instead. (Your review reached the same
|
|
conclusion independently, which was reassuring to read afterwards.)
|
|
|
|
**B4 — closed the way your review asked, not the way the parity doc did.** The doc offered the flag flip
|
|
as "the honest one-liner"; you correctly pointed out that flipping it changed nothing observable because
|
|
no code read the capability. The composer now gates on it — drop zone, paste path, attach menu — so the
|
|
flag is load-bearing. Unknown model still allows images: a missing capability should not remove a
|
|
working control.
|
|
|
|
**Thinking selector — removed, not hidden.** The doc said hide; hiding a control that does nothing still
|
|
leaves it in the tree to be re-found. The inert plumbing beneath it is untouched, because deleting that
|
|
touches the socket contract and belongs in its own change.
|
|
|
|
---
|
|
|
|
## `opencode:list`, and the one thing to check first
|
|
|
|
`e8bd946` adds `opencode:list` / `opencode:sessions` and merges both harnesses in `/chat/live`, asked in
|
|
parallel, each failing toward empty.
|
|
|
|
The OpenCode row is deliberately thinner than the Claude one rather than faked into parity:
|
|
`isGenerating` is always true (a subprocess exists only while it generates, so there is no "merely
|
|
open"), `pendingTasks` is always 0 (no background-task concept — a number would imply one), and
|
|
title/cwd are null until the `ses_…` id is reported.
|
|
|
|
**It has never returned a non-empty list.** No OpenCode turn was running to enumerate. Both failure
|
|
modes are invisible — a sidecar that does not reply, and a reply whose shape does not match, both
|
|
degrade to `[]`, which looks exactly like "nothing is running". **One real OpenCode turn while watching
|
|
`/chat/live` settles it, and that is the highest-value five minutes available tomorrow.**
|
|
|
|
---
|
|
|
|
## Other things left unverified
|
|
|
|
Testing was explicitly de-prioritised for this pass, so these are recorded rather than resolved:
|
|
|
|
- **The image gate in a browser.** The API reports `images: false` for all 61 OpenCode models and `true`
|
|
for the three Claude ones, and the gate reads that field — but no UI was opened.
|
|
- **`mapRunLine` fixtures are hand-written**, not captured from a live turn. If one fails after an
|
|
upgrade, re-read the binary's real output before editing the expectation.
|
|
- **No OpenCode turn was run after deleting `AGENTS.md`.** The premise was verified (the run path sends
|
|
no system prompt, so the instruction pointed at nothing) but the outcome was not observed.
|
|
|
|
`COMMS/BLOCKERS.md` has the same list plus the mistakes I made getting here.
|
|
|
|
---
|
|
|
|
## Suggested next, if you are writing the following spec
|
|
|
|
1. **Exercise `opencode:list`** — it is the only new capability whose happy path is unproven.
|
|
2. **Decide the fork.** The blocker is gone; `opencode-serve-path.md` frames it. If the answer is "not
|
|
yet", say so in the parity doc so it stops reading as pending work.
|
|
3. **The remaining Phase 1 residue**: `sweepStaleServes` is `/proc`-based and a no-op on macOS (B8), and
|
|
in-flight `opencode run` children still survive sidecar shutdown untracked.
|
|
4. **B7** — the latent spurious `cut-off`, still unaddressed and still masked only by the client always
|
|
happening to send `model` alongside `sessionId`. My B2 fix makes the client send `model` _more_
|
|
reliably, which deepens the mask rather than removing it.
|
|
|
|
Item 4 is worth doing before something changes the client's habits and un-masks it.
|