Repository navigation
feat(openclaw): ACP-to-HTTP adapter so OpenClaw shares the standing opencode serve (#1243) - #3997
Conversation
…pencode serve (#1243) bin/opencode-acp-adapter.mjs speaks ACP (JSON-RPC over stdio) to OpenClaw acpx and the OpenCode serve HTTP API to the standing shared server on 127.0.0.1:13420: session/new -> POST /session (cwd as directory), session/prompt -> prompt_async + GET /event SSE streamed back as session/update notifications, permission.updated -> session/request_permission -> POST permissions, session/cancel -> POST abort. No private per-process server is spawned. home/openclaw/default.nix repoints the opencode agent at the adapter in both spots (~/.acpx/config.json and the embedded acpx override), with opencode acp kept as a documented one-line fallback. Tests: scripted ACP client vs stub serve API, 5/5 pass (node --test).
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 40 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e67215808
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const session = await serveJson(withDirectory('/session', cwd), { method: 'POST', body: {} }); | ||
| if (!session?.id) throw new Error('serve POST /session returned no session id'); |
There was a problem hiding this comment.
The configured target is the OpenCode 2.x server pinned in flake.nix, but this adapter implements the older session contract. The existing client for that same standing server, pm-stack/dispatch/src/opencode.ts, creates sessions with POST /api/session, supplies location.directory in the body, unwraps the {data: …} response, and submits prompts through /api/session/:id/prompt. Here, session creation instead uses /session?directory=… and expects a bare id; subsequent prompts likewise use the incompatible prompt_async route. Consequently, switching OpenClaw to this adapter does not provide a working session flow against its intended server. Implement and test against the pinned server's contract rather than the v1-shaped stub.
Useful? React with 👍 / 👎.
| const password = process.env.OPENCODE_SERVER_PASSWORD; | ||
| if (password) { | ||
| headers.Authorization = `Basic ${Buffer.from(`opencode:${password}`).toString('base64')}`; |
There was a problem hiding this comment.
In the configured OpenClaw gateway, the adapter only authenticates if OPENCODE_SERVER_PASSWORD happens to be inherited, but home/openclaw/default.nix neither supplies that variable nor installs a wrapper to load it. The standing server requires the password stored in ~/.local/state/opencode/service-prod.json: bin/openchamber-start.sh loads it explicitly, as does the dispatch client. Unlike the previous private-server ACP command, this adapter therefore sends unauthenticated session and event requests in the declared gateway environment, preventing session creation even after the API contract is corrected. Read the standing server's credential at runtime or explicitly wire it into the adapter's launch environment.
Useful? React with 👍 / 👎.
| const pending = pendingPrompts.get(sessionId); | ||
| if (!pending) return; | ||
| pendingPrompts.delete(sessionId); | ||
| partTextSent.clear(); |
There was a problem hiding this comment.
When two sessions have prompts in flight, completing either one clears the text offsets for both. The next cumulative text update for the still-running session is then emitted from character zero, duplicating content already delivered to its ACP client. Exercising the current handlers with session B emitting Hello, session A becoming idle, and B updating its part to Hello world produces the concatenated output HelloHello world. Keep offsets scoped to each session and remove only the completed session's entries.
Useful? React with 👍 / 👎.
| debug('event stream closed; reconnecting'); | ||
| } catch (err) { | ||
| debug(`event stream error: ${err.message}; reconnecting`); | ||
| } | ||
| if (!shuttingDown) await new Promise((resolve) => setTimeout(resolve, 1000)); |
There was a problem hiding this comment.
If the SSE connection drops during an active prompt and the server completes that prompt before reconnection, its final session.idle event is lost. This reconnect path neither resolves or rejects pending prompts nor queries their current status, and session/prompt has no other completion mechanism. The ACP request therefore remains pending indefinitely, with subsequent prompts rejected as already in flight even after the stream reconnects. Reconcile pending sessions after reconnecting, or explicitly fail their outstanding requests when event continuity is lost.
Useful? React with 👍 / 👎.
| const sessionId = params?.sessionId; | ||
| if (!sessionId || !sessions.has(sessionId)) return; | ||
| cancelledSessions.add(sessionId); |
There was a problem hiding this comment.
When a cancellation notification races with the prompt's final idle event, the prompt can finish before this handler runs. The handler still records the session as cancelled, but an ensuing idle event cannot remove that marker because cleanup in handleServeEvent requires a pending prompt. The next normally completed prompt then incorrectly returns stopReason: "cancelled". The marker also survives a cancelled prompt ending through session.error. Only mark active prompts as cancelled and clear their cancellation state on every terminal path.
Useful? React with 👍 / 👎.
Refs #1243 (task 4972). Does not close #1243: per its acceptance, closing waits until the live-smoke result is recorded (post-activation
/acp spawn opencodesession visible inGET /sessionon the 13420 server).What
bin/opencode-acp-adapter.mjs— a single-purpose Node executable following the bin/*.mjs pattern that speaks ACP (JSON-RPC 2.0 over stdio, newline-delimited) on one side and the OpenCode serve HTTP API on the other, targeting the standing shared server (defaulthttp://127.0.0.1:13420,--serverflag /OPENCODE_ACP_SERVERenv;OPENCODE_SERVER_PASSWORDsent as basic auth when set). The adapter spawns no server of its own.Mapping (per #1243):
session/new→POST /session(cwd →?directory=)session/prompt→POST /session/:id/prompt_async+GET /eventSSE streamed back as ACPsession/updatenotifications (text/reasoning part deltas →agent_message_chunk/agent_thought_chunk); prompt resolves onsession.idle(session.error→ JSON-RPC error)permission.updated→ ACPsession/request_permission→ reply posted toPOST /session/:id/permissions/:permissionID(once/always/reject; a cancelled or failed client request denies rather than hanging)session/cancel→POST /session/:id/abort; the in-flight prompt resolves with stop reasoncancelledNix wiring
home/openclaw/default.nixrepoints OpenClaw's opencode agent at the adapter in both config spots — the~/.acpx/config.jsonentry and the embedded acpx override — withopencode acpretained as a documented one-line fallback in comments at both spots. Nix parse check passes; module evaluation is left to CI.Tests
bin/opencode-acp-adapter.test.mjs(run:node --test bin/opencode-acp-adapter.test.mjs): a scripted ACP client drives the adapter over stdio against a stub serve HTTP API — initialize, session/new creating a session on the stub with the cwd as directory, a streamed prompt round-trip, a permission round-trip landing on the permissions endpoint, and cancel aborting on the serve. 5/5 pass on the laptop.Coordination
PR #3275 (#1711, Claude/Codex ACP wrapper packaging) also edits
home/openclaw/default.nixbut does not touch the opencode agent entries; this PR's two hunks are confined to the opencode agent wiring and its surrounding comments, so any conflict on merge is textual and small. Whichever lands second rebases those hunks.Still owed after merge + activation
Live smoke per #1243 acceptance:
/acp spawn opencodein OpenClaw creates a session visible inGET /sessionon the 13420 server, with no new private opencode serve process for the adapter-routed session. #1243 stays open until that is recorded.Need help on this PR? Tag
@codesmith-botwith what you need.