Repository navigation
docs(migration): list v1 to v2 client breaking changes - #15291
marius-kilocode wants to merge 3 commits into
Conversation
|
➖ v2 migration progress: 0.9% → 0.9% (0 done, 0 total, 0 dropped; 90/9829 cells) Placement: 0 new marked lines in shared files, 0 new Ratchets: 9 core-internal imports (base 9); shared patches by rung 20 seam, 7 patch (base 20 seam, 7 patch). No change to the progress map. |
Code Review SummaryStatus: 7 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (1 file)
Fix these issues in Kilo Cloud Previous Review Summary (commit 542c45c)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 542c45c)Status: 5 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (2 files)
Reviewed by deepseek-flash-latest · Input: 54K · Output: 12.8K · Cached: 1.4M Review guidance: REVIEW.md from base branch |
| |---|---|---| | ||
| | Start | `kilo serve --port 0`; stdout line `kilo server listening on http://127.0.0.1:<port>` | `kilo2 serve --port 0 --stdio`; stdout prints one JSON line `{"url":"http://127.0.0.1:<port>"}` (`packages/cli/src/server-process.ts`) | | ||
| | Process lifetime | The server watches `KILO_PARENT_PID` and exits when the parent dies | The server exits when its stdin closes (`waitForStdinClose`). The client keeps the stdin pipe open and closes it to stop the server. There is no parent PID watchdog | | ||
| | Password | `KILO_SERVER_PASSWORD` | `KILO_SERVER_PASSWORD` still works: the host copies it to `OPENCODE_SERVER_PASSWORD` (`packages/kilo-cli/src/env.ts`). In `--stdio` mode the server removes it from the environment that tools inherit | |
There was a problem hiding this comment.
WARNING: The password is only removed under its OPENCODE_ name
packages/cli/src/server-process.ts:87-90 deletes OPENCODE_PASSWORD and OPENCODE_SERVER_PASSWORD in --stdio mode. The host's translate() copies KILO_SERVER_PASSWORD to the OPENCODE_ name but leaves KILO_SERVER_PASSWORD in process.env (packages/kilo-cli/src/env.ts:28-34), and shell tools inherit process.env (packages/core/src/shell.ts:275-278). A client that follows this row and sets KILO_SERVER_PASSWORD therefore still exposes the credential to every tool it runs. Either the host should delete the KILO_ name as well, or the note should say that only the OPENCODE_ copy is scrubbed.
| | Process lifetime | The server watches `KILO_PARENT_PID` and exits when the parent dies | The server exits when its stdin closes (`waitForStdinClose`). The client keeps the stdin pipe open and closes it to stop the server. There is no parent PID watchdog | | ||
| | Password | `KILO_SERVER_PASSWORD` | `KILO_SERVER_PASSWORD` still works: the host copies it to `OPENCODE_SERVER_PASSWORD` (`packages/kilo-cli/src/env.ts`). In `--stdio` mode the server removes it from the environment that tools inherit | | ||
| | Auth | HTTP Basic, user `kilo` | HTTP Basic, user `opencode` (`packages/server/src/auth.ts`). Browser and WebSocket clients can send the same credential as `?auth_token=<base64 user:password>` | | ||
| | Path prefix | none (`/session`, `/provider`) | `/api` (`/api/session`, `/api/provider`). Paths outside `/api` return 404 | |
There was a problem hiding this comment.
SUGGESTION: Paths outside /api do not return 404
/auth/connect/:code is a route (packages/protocol/src/groups/server.ts:33), and the CLI's Web UI handler serves the app shell for every other non-/api path (packages/cli/src/services/web-ui.ts:30-44, wired in packages/cli/src/server-process.ts:99). A client that treats any non-404 response as an API answer reads HTML. Consider saying that non-/api paths are not JSON API routes, and mention the pairing path if it is in scope for these clients.
| | Directory scope | `?directory=<path>` on almost every route, plus `?workspace=` | Location-scoped routes take `?location[directory]=<path>` (deepObject). `POST /api/session` takes `location: { directory }` in the body. Without the query, the header `x-opencode-directory` (URI-encoded) applies, then the server working directory. Session-scoped routes (`/api/session/:sessionID/...`) resolve the location from the session and take no directory | | ||
| | Response body | The resource itself | An envelope: `{ data }`, `{ data, cursor: { previous, next } }` for pages, or `{ location, data }` for location-scoped reads (`Location.response`) | | ||
| | Events | `GET /global/event`, SSE of `{ directory, project, workspace, payload }`; `payload.type === "sync"` wraps durable events (`session.updated.1`, `message.part.updated.1`) | `GET /api/event`, SSE of `{ id, created, type, location, data, metadata }`. One stream for all locations. Volatile: no replay, a slow consumer fails the stream, and events during a disconnect are lost. After a reconnect the client reloads state with list calls, or replays the durable events of one session with `GET /api/experimental/session/:sessionID/log?after=<seq>&follow=true` (SSE; durable events carry `durable: { aggregateID, seq, version }`) | | ||
| | Health | `GET /global/health` | `GET /api/info` (server name, version, channel) | |
There was a problem hiding this comment.
SUGGESTION: GET /api/info has no server name or channel
The response is { version, pid, urls, paths: { tmp }, capabilities? } (packages/protocol/src/groups/server.ts:4-20). The app name and channel are passed into the server process but are not exposed here, so the field list should name version, urls, paths and capabilities instead.
| | session.get | `GET /session/{id}` | session.get | `GET /api/session/:sessionID`. The session carries `agent`, `model`, `cost`, `tokens`, `location.directory` and `time.archived` | | ||
| | session.list | `GET /session` | session.list | `GET /api/session?directory=&project=&subpath=&parentID=null&search=&order=&limit=&cursor=`. `parentID=null` replaces `roots=true` | | ||
| | experimental.session.list | `GET /experimental/session` | session.list | Same route, with `archived=true\|false`. v1 `worktrees` and `current` filters have no v2 query | | ||
| | session.update | `PATCH /session/{id}` | session.update | `PATCH /api/session/:sessionID` body `{ title?, metadata?, permissions?, archived? }`. Returns 204; the change arrives as `session.renamed` or `session.metadata.updated` | |
There was a problem hiding this comment.
SUGGESTION: session.update publishes more than two events
Besides session.renamed and session.metadata.updated, changing permissions publishes session.permissions, and changing archived publishes session.archived or session.unarchived (packages/schema/src/session-event.ts:120-150; the handler applies all four fields at packages/server/src/handlers/session.ts:228-252). A client that listens only for the two named events misses permission and archive changes.
| | v1 route | v2 | Notes | | ||
| |---|---|---| | ||
| | `GET /global/health` | server.info | `GET /api/info`. A health poll is optional: the stdio pipe and the event stream show a dead server | | ||
| | WebSocket `/pty/{id}/connect?directory=&cursor=&auth_token=` | pty.connectToken | `POST /api/pty/:ptyID/connect-token` returns `{ ticket, expires_in }`; then WebSocket `/api/pty/:ptyID/connect?ticket=<ticket>&cursor=` | |
There was a problem hiding this comment.
WARNING: Minting a PTY connect token also needs a header and returns a location envelope
The handler requires the x-opencode-ticket: 1 header and rejects the request with 403 otherwise (packages/server/src/handlers/pty.ts:117-131; the header name is PTY_CONNECT_TOKEN_HEADER in packages/protocol/src/groups/pty.ts:10). The response is Location.response(PtyTicket.ConnectToken), so it is { location, data: { ticket, expires_in } }, not { ticket, expires_in } (packages/protocol/src/groups/pty.ts:103-108). A client built from this row is rejected.
| | Response body | The resource itself | An envelope: `{ data }`, `{ data, cursor: { previous, next } }` for pages, or `{ location, data }` for location-scoped reads (`Location.response`) | | ||
| | Events | `GET /global/event`, SSE of `{ directory, project, workspace, payload }`; `payload.type === "sync"` wraps durable events (`session.updated.1`, `message.part.updated.1`) | `GET /api/event`, SSE of `{ id, created, type, location, data, metadata }`. One stream for all locations. Volatile: no replay, a slow consumer fails the stream, and events during a disconnect are lost. After a reconnect the client reloads state with list calls, or replays the durable events of one session with `GET /api/experimental/session/:sessionID/log?after=<seq>&follow=true` (SSE; durable events carry `durable: { aggregateID, seq, version }`) | | ||
| | Health | `GET /global/health` | `GET /api/info` (server name, version, channel) | | ||
| | Kilo routes | Hono routes under `/kilo/*`, `/kilocode/*` and others | Plugin RPC: `POST /api/rpc/:rpcID/:method?location[directory]=<path>`, body `{ "input": ... }`, response `{ "output": ... }`. RPC events arrive on `/api/event` with `type: "rpc.<rpcID>.<event>"`. The Kilo RPCs on v2 today are `kilocode.gateway` and `kilocode.handshake` | |
There was a problem hiding this comment.
SUGGESTION: The cited v2 source predates the handshake RPC
kilocode.handshake landed on v2 after the commit this note cites as its v2 source (0311551d521): it arrived with 6b405b8d9f (PR #15285), which is on origin/v2 today. The statement is correct for current v2, but a reader who checks the note against the pinned SHA cannot find the RPC. Consider moving the Sources line to a SHA that contains the handshake, or noting that this row is newer than the pin.
| | instance.dispose | `POST /instance/dispose` | location.reload | As above | | ||
| | instance.reload | `POST /instance/reload` | location.reload | As above | | ||
| | global.event | `GET /global/event` | event.subscribe | `GET /api/event`, see "Wire basics" | | ||
| | experimental.capabilities.get | `GET /experimental/capabilities` | kilocode.handshake.get for identity; needs Kilo RPC for capabilities | `POST /api/rpc/kilocode.handshake/get` body `{ "input": {} }` returns `{ product: "kilo", version, channel }`. An upstream server answers `rpc.unavailable`, a v1 server 404. Feature capabilities: U-E1-08, U-E6-02, shape per Q-G2-02 | |
There was a problem hiding this comment.
SUGGESTION: The HTTP body wraps the output
The route success schema is { output: unknown } (packages/protocol/src/groups/rpc.ts:8-18), so the response body is { "output": { "product": "kilo", "version": ..., "channel": ... } }, not the bare object. The wire-basics row already says { "output": ... }, but this row reads as a bare response. Consider saying it returns the output { product, version, channel } to make the wrapper explicit.
What Problem This Solves
The VS Code extension for the v2 line is a native client of the Kilo host over
@opencode/client. There was no single list of what a client must change to move from the v1 server to the v2 server. JetBrains and the cloud consumers need the same facts at the wire level, because they do not use the TypeScript client.Why This Change Was Made
Adds
migration-tracking/technical-notes/v1-v2-client-breaking-changes.md:serve --port 0 --stdio, one JSON line, stdin as process lifetime), Basic auth user (kiloon v1,opencodeon v2), the/apiprefix,location[directory]scoping, response envelopes, the volatile event stream atGET /api/event, plugin RPC framing.origin/main(5cc65d1212b): 99 matchclient.<group>.<method>(, 30 use other call shapes (multi-line chains, aliases, four-level chains). Each row gives the v1 route, then the v2 operation and route, akilocode.gatewayRPC method,needs Kilo RPCwith the plan unit, ordroppedwith the reason. Plus the 7 raw routes outside the client.session.statusevent, part order, message cursors, a catalog that fills in after startup, tool names and inputs) and how a v2-native client handles each.session.execution.*events, which match@opencode/client/solidas the TUI uses it.The facts come from reading
origin/mainandorigin/v2(0311551d521, after thev2.1.0sync). Thevscode-v2-bootbranch (a v1-shaped adapter, commits47b491ef2c2and5aa92629895) is cited as a mechanism reference only.The D-5 decision record (native rebuild for VS Code, no v1 wire facade) and the G2 plan rows are in the coordinator PR on branch
plan-foundation, not here.User Impact
None. Documentation only.
Evidence
git grep -h -oE "client\.[a-zA-Z]+\.[a-zA-Z]+(\.[a-zA-Z]+)?\(" origin/main -- packages/kilo-vscode/src | sort -ugives 99 names; the 30 others were found by call-site review. A diff of the doc rows against the grep list shows exactly those 30 extra names and no missing name.bun run script/check-md-table-padding.ts <note>: no padded tables. The script exists only onmain; it was run from a temporary copy.bun run script/check-shared-patches.ts --worktree: ok.bun run script/check-opencode-annotations.ts --worktree: ok.bun script/upstream/progress.ts --check: up to date (the note does not change the map).