Repository navigation
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
The adapter is opt-in and well contained, and steering, prompt acknowledgement and replay look right to me. I'm asking for one structural change and one fix before merge. I ran bun test test/acp in packages/cli (124 pass, including the new v1-fallback, v2 wire and v2 subprocess tests) and bun run typecheck (passes). The subprocess test runs the real CLI through a v2 prompt, a mid-run steer, /compact and resume with replay.
Public API impact: adds the OPENCODE_EXPERIMENTAL_ACP_V2=1 environment variable, which enables ACP protocol v2 negotiation in opencode acp. It bumps @agentclientprotocol/sdk from 1.6.0 to 1.8.0. Without the flag, v1 clients and clients requesting v2 still get v1, so this is backwards compatible.
Concerns:
- Every connection now goes through the SDK's experimental v2 code, even without the flag. The handler always uses the v2
ndJsonStreamandagentProtocolRouterfrom@agentclientprotocol/sdk/experimental/v2. That contradicts "V1 behavior stays on its existing execution path", and it exposes default users to churn in an experimental SDK module (see inline). - v2
session/closecan stop part-way. It doesn't ignore inbox-cancel failures and turns observer setup failures into crashes (see inline).
There was a problem hiding this comment.
Both earlier concerns are fixed, and the follow-up commit adds no new problems. I re-ran bun test test/acp in packages/cli (126 pass, 0 fail) and bun run typecheck (passes).
Resolved:
- v1 stays on the stable SDK. Without
OPENCODE_EXPERIMENTAL_ACP_V2=1, the handler uses the stablendJsonStreamandACP.connect. The experimental router andagent-v2are loaded only on the opt-in path, andagent.tskeeps only a type import from the experimental module. - v2 close and cancel finish their cleanup. They now share
stopObserver. It no longer crashes when observer setup fails, settles every pending acknowledgement before any calls to the server, logs individual inbox-cancel failures and still interrupts the session. The new test covers this with failing inbox cancels.
Public API impact is unchanged since my last review: adds the OPENCODE_EXPERIMENTAL_ACP_V2=1 environment variable for opt-in ACP v2 negotiation and bumps @agentclientprotocol/sdk from 1.6.0 to 1.8.0. It is backwards compatible.
No further concerns.
Issue for this PR
Refs #44877. Related earlier implementation: #44524.
Type of change
What does this PR do?
Adds ACP v2 behind
OPENCODE_EXPERIMENTAL_ACP_V2=1, using SDK 1.8.0's experimental entry point. Without the flag, clients still negotiate v1, including clients requesting v2.The v2 adapter shares the existing session service. A session-scoped observer handles streaming and aggregate state independently of prompt requests, allowing steering and resumed parent output after idle. Regular prompt acknowledgments wait for durable delivery; cancelling an undelivered steer removes only that inbox item.
Ports the existing session lifecycle, replay, config options, terminal authentication, commands, compaction, MCP transports, permissions, elicitation, and child-session extension. V2 uses replacement messages during replay, tool-call upserts, and structured diffs with patches. Commands use live-only ACP user-message echoes;
auth/logoutcloses the ACP connection without deleting shared credentials.This is an opt-in draft-protocol implementation, not an unrestricted production rollout. External-editor/live-provider interoperability and Windows/macOS validation remain outstanding. V1 behavior stays on its existing execution path.
How did you verify your code works?
bun test test/acpinpackages/cli: 124 pass, 0 fail on currentdev.bun run check: pass (repository warnings remain).services/www:bun typecheck,bun run check:generated, andbun run buildpass.git diff --check: pass.Screenshots / recordings
Not applicable: protocol and CLI changes.
Checklist
Added by the opencode bot (AI-generated).
Related: #44877, #53042
Public API: adds the OPENCODE_EXPERIMENTAL_ACP_V2=1 environment variable for opt-in ACP v2 negotiation and bumps @agentclientprotocol/sdk to 1.8.0 (backwards compatible)