Files
platform/docs/opencode-phase0-review.md
T
pastilhasandClaude Opus 5 8ac36ed8b8 review phase 0, and settle the opencode cwd question by testing it
Phase 0 accepted except B4, which is reopened: flipping images to false made the metadata honest but
nothing reads that capability, so the composer still accepts a drop and the image is still discarded
before the model sees it. The defect described a user-visible lie and the lie is unchanged. Gate the
composer on the flag, or plumb images through — the first is the Phase 0 one-liner.

The cwd question is answered empirically rather than argued. Against opencode 1.18.11: --dir anchors the
agent's file operations, not just the process cwd, and the anchor survives a four-step turn with a write
in the middle. So the single-server constraint that shelved this work is already gone — it went away when
turns moved off the serve to `opencode run --dir`.

Two consequences. Per-directory servers are unnecessary; don't build them. And the injected AGENTS.md
telling the agent its working directory is safe to delete — it points at a system prompt the run path
never sends, and --dir does the job it was standing in for.

Phase 2's fork also narrows: moving turns onto the serve would reintroduce the original coupling unless
the serve takes a per-request directory, so that is the question to answer, not why it was replaced.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-10 03:44:34 +01:00

6.3 KiB

Phase 0 review, and what the cwd tests found

Review of 22bcd7d, 492509a, 013e629 against docs/opencode-parity.md, plus two empirical tests of the OpenCode cwd behaviour that change what Phase 1 and 2 should be.


Phase 0 — accepted, with one item to reopen

B1, B2, B3 — good, and better than the spec. Each was reproduced against a running server before being changed, which is what the handover asked for and is what caught the document being wrong: location.directory does not exist in opencode 1.17.9+, which returns directory at the top level with no location and no metadata. Deleting officerMeta and the metadata tag rather than fixing them was the right call — the only writer had no callers, and tagging would have been a second source of truth for something directory already answers.

The B2 write-up is a model of what a fix note should be: the six-hop chain confirmed rather than inferred, and the finding that the value was produced correctly and read from the wrong place rather than never produced.

B5 — correct, including the part where it deliberately does not copy Claude. A fresh subscription per turn IS right for OpenCode, because a fresh opencode run is spawned per turn; the defect was overwriting the old handle without detaching it. Mirroring Claude's _claudeKill guard would have been wrong, and the commit says so.

B6 — correct, and the restraint is the good part. Clearing the ses_… map in deleteSession only, never in releaseSession, with the reason stated: releasing means "let go, leave it running", so a returning browser must be able to find the same ses_… again. Clearing it there would have quietly orphaned every released OpenCode session.

Thinking selector — good. Removing the control rather than fixing it, and leaving the inert plumbing for a follow-up that touches the socket contract, is the right split.

B4 — reopen: the flag is now honest but inert

013e629 flipped images: truefalse for OpenCode models, and the commit says "61 OpenCode models now decline, the three Claude ones still accept".

Nothing declines. No code in src/workspaces or src/apps reads that capability — the composer's image affordances are ungated. Grep for a consumer of the model's images field returns nothing: InputArea's drop zone, the paste handler, and AttachButton all accept images regardless of model, and useAttachments collects them regardless.

So the observable defect is unchanged: on an OpenCode chat you can still drop an image, watch it render in your own bubble, and have it discarded before the model sees it. The metadata is more truthful now, which is worth having, but B4 described a user-visible lie and that lie is still on screen.

Two ways to close it, and they are not equivalent:

  1. Gate the composer on the capability. Read the selected model's images flag and hide the drop zone, the paste path and the attach-image button when it is false. Cheap. Makes the flag load-bearing, so the flip in 013e629 starts doing something.
  2. Plumb images through OpenCodeRunParams (currently Phase 4). Removes the limitation rather than surfacing it.

(1) is the honest one-liner Phase 0 was for; (2) is the real fix. Doing (1) now costs nothing if (2) happens later — the gate simply stops firing once the capability is true.


What the cwd tests found — this changes Phase 1 and 2

Andre's memory of why OpenCode was shelved: everything ran through one opencode serve, which anchored every call to the directory that server was started in. That broke listing sessions by cwd and forced the injected AGENTS.md telling the agent what its working directory was supposed to be.

That constraint is gone, and it went away when turns moved off the serve. Turns are now opencode run --dir <cwd> subprocesses (runner.ts:65). Tested against the installed binary (1.18.11; our comments still pin 1.17.9):

Test 1 — does --dir anchor file operations, or only the process cwd? Ran a turn in a scratch directory containing two marker files. The agent's ls returned exactly those two files and nothing from the repo. --dir anchors the agent.

Test 2 — does the anchor survive a multi-step turn? Four steps: pwd, write a file, pwd again, ls. Both pwds returned /private/tmp/oc-cwd-test, the file landed there, and the platform repo was untouched. No mid-turn drift.

One cosmetic residue: each run ends with Shell cwd was reset to <the platform repo> — some global notion of "the project" that is neither the invoking directory nor --dir. It prints after the process finishes and had no effect on either test. Worth knowing it exists; not worth chasing.

Consequences

  • Do not build a server per directory. It was a sound response to the old architecture and is now unnecessary — N processes, N ports and a lifecycle to manage, to solve what --dir already solves.
  • The injected AGENTS.md is safe to delete. index.ts:22-35 seeds a file telling the agent to read "Working directory for this session" from the system prompt; the run path sends no system prompt, so that instruction points at nothing. --dir is the real anchor and it works. This is the "very nasty hack" — it can go, and nothing needs to replace it.
  • The stale comment goes with it. opencode-sessions.ts:6-9 still claims all sessions live in one server's project. They do not, and the listing now proves it: one serve returned 7 sessions across several directories.
  • Phase 2's fork is narrower than the doc implies. 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.

Suggested next work, in order

  1. B4 properly — gate the composer on the model's images capability (above).
  2. Delete the AGENTS.md injection and the stale one-project comment, now that --dir is verified. This is Phase 1 work and it is the thing Andre most wanted gone.
  3. Then the rest of Phase 1 — the dead event-mapper.ts and SSE machinery, the wrong path names in comments, and the first tests over runner.ts's NDJSON mapping, which is pure, untested, and pinned to a version two minor releases behind what is installed.