pastilhas
027b10bd6e
44 files of prose — CLAUDE.md, AGENTS.md, TODO.md, 20 docs, both plugin design
documents, and the comment surface the earlier steps could not reach.
Applied against an explicit keep-list, not swept, because the word turned out to
have SIX meanings in this repository rather than the three the offscale doc
recorded:
permissions renamed (steps 1–2)
$OFFICER_ROOT/capabilities/ KEPT — the item store, and now the only thing
the word means that is ours
sidecar routing keys renamed to `handles` (step 3)
Lightning wallet KEPT — a domain term, and on the wire to the mobile apps
terminfo queries KEPT — XTGETTCAP, in the pty sidecar
InvoiceShelf KEPT — per-resource { write, bulkDelete } flags
The sweep still falsified two things, both caught by checking rather than by
review, and both in prose that discusses more than one meaning at once:
CLAUDE.md began claiming the item store lives at `$OFFICER_ROOT/permissions`.
It does not; that directory is on disk and full of skills and tools.
And the offscale doc's own note about the collision became
"Named `permissions`, NOT `permissions`" — a sentence that had eaten the thing
it existed to warn about.
Both restored, and the note rewritten to say what is now true: capability means
one thing of ours, and three that belong to somebody else's vocabulary.
Verified live after restart: self and admin permission endpoints 200, gated
route 200, agent-status 200, 9 grants intact with 6 permissions offered.
tsgo clean, 797 tests, 787 pass, same 7.
The rename is done. Four steps, no data lost, no client break that survived
the step it was introduced in.
108 lines
6.3 KiB
Markdown
108 lines
6.3 KiB
Markdown
# 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: true` → `false` 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 permission — 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 permission.** 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 permission 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 `pwd`s 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` permission (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.
|