Skip to content

feat(acp): serve the agent client protocol over stdio - #359

Open
ZuyiZhou wants to merge 4 commits into
feat/rpc_engine_assemblyfrom
feat/acp_agent_over_stdio
Open

feat(acp): serve the agent client protocol over stdio#359
ZuyiZhou wants to merge 4 commits into
feat/rpc_engine_assemblyfrom
feat/acp_agent_over_stdio

Conversation

@ZuyiZhou

Copy link
Copy Markdown

Summary

raven acp is now an agent an editor can launch: it negotiates, mints and reloads sessions, runs a turn, streams the reply and its tool calls, asks before acting, and answers a cancel while a turn is in flight.

Four commits, each one concern: the wire framing, the lane-to-session mapping, the shell approval families this surface needs, and the protocol layer itself.

The process model is a self-contained engine, not a client of a resident gateway. An ACP agent IS a stdio child of the editor, so there is nothing to mount onto unless a gateway happens to be running, and an agent that only works when another service is up is not an agent an editor can launch.

Notes on the parts that are not obvious:

  • Every inbound frame is handled in its own task, because session/prompt is suspended for as long as the turn takes and session/cancel has to be read during it. Shutdown answers instead of cancelling: on EOF the pending prompts are settled as cancelled and the handlers return through their own code, so a finite input (a scripted client, a smoke test) still gets its replies.
  • A prompt is never answered with a JSON-RPC error. A failed turn is explained as message content and then ends with a stop reason. Measured from the other side on another implementation: an error in reply to a turn-shaped request makes clients tear the whole turn down.
  • The schema is vendored and pinned, and its blindness is measured rather than assumed: additionalProperties appears 118 times and is true in every one, method is an unconstrained string, and params accepts null. Validating a frame against the union therefore accepts an invented method name, so the tests validate payloads against their own definition and check method names against the metaschema separately.
  • Questions take one of two paths by what the client declared. With elicitation forms, a question with no preset answers can come back as text. Without them it rides a permission request and a synthesised tool call, which the payload marks as synthesised rather than dressing up as a real one. The case that has no answer at all, a free-text question to a client with no elicitation, is recorded in the compatibility matrix instead of being papered over.
  • Secrets are redacted before anything reaches the client, including the value under a key whose name says it is a secret. A credential file's path is deliberately NOT redacted: a path is not a secret, and hiding it would hide that the agent read credentials at all.
  • Six command families ask before running on this surface. What that does not reach is written down: a sub-agent builds its own tool with its own policy and no responder, so a delegated command still runs unprompted.

The write tools now publish what they changed. Until now the diff field declared on tool.complete had no producer here, so it was always null. The file tools return a ToolResult carrying both the unified diff and the two file versions, and the registry, the loop and the outlet carry them through. The protocol needs the second form specifically: its diff content block is {path, newText, oldText}, which a rendered diff cannot fill. A reply's files reach the client the same way, as a media event carrying paths rather than bytes.

Divergences from the same layer in the sibling runtime, each measured rather than assumed:

  • The compatibility matrix is corrected for this build in two rows. A model change during a turn is not refused, because config.set here has no busy guard. A sub-agent's output is untagged, because there is no direct sub-agent chat for the tag to distinguish.
  • The self-verification suite is not included. It drives the ACP client's own capability probe, which this repo does not have, and porting a client subsystem to run one test file is the tail wagging the dog.
  • The session/load smoke test is not included either, and the reason is worth knowing: nothing here can point a spawned agent at a throwaway session store. RAVEN_HOME steers tracing and the terminal runtime root only, and the config path has no override at all, so that test would either read the developer's real store or prove nothing. Load and replay keep their unit coverage; what is lost is the leg that read an actual file.
  • The working-directory guard is checked against the configured home for the same reason, rather than against a temporary one.

Known fidelity losses, all in docs/specs/2026-08-21-acp-agent-compatibility.md: tool_call_update always reports completed because the event carries no success flag; rawInput is not sent because it carries a command line verbatim; there is no plan because its priority field is required and has no source here.

Stacked on #357, which is stacked on #356, which is stacked on #353. Merge order: 353, 356, 357, then this.

Reviewers: 0xKT, gloryfromca.

Type

  • Fix
  • Feature
  • Docs
  • CI / tooling
  • Refactor
  • Other

Verification

Commands run and their results:

  • uv run --frozen --all-extras pytest -q -> 7375 passed, 19 failed. The same 19 that fail on the base branch: test_provider_rates.py (15) and test_agent_loop_usage_sink.py (4), pricing-table and network-cache tests unrelated to this change. Zero new failures.

  • uv run --frozen python scripts/coverage_gate.py diff --base-ref feat/rpc_engine_assembly -> 100.00% (1718/1718 executable changed lines). The first run was 99.30%; the twelve uncovered lines were the framing module's public API and the oversized-diff branch, and cases were added for both.

  • uv run --frozen python scripts/coverage_gate.py ratchet -> line +2.03pp, branch +3.08pp, PASS.

  • raven/acp/ unit coverage: 100 percent line and 100 percent branch across 540 tests.

  • Integration against the real binary, run with -m "": test_acp_stdio_smoke.py (6) and test_acp_adversarial_smoke.py (26) pass.

  • uv run --frozen ruff check raven/ tests/ and ruff format --check -> clean.

  • cd ui-tui && npm run lint:rpc -> generated.ts in sync. npm run type-check -> exit 0. npm test -> 998 tests passed.

  • raven acp --help resolves through the real CLI app.

  • Relevant tests pass locally

  • Relevant lint / type checks pass locally

  • User-facing docs or screenshots are updated when needed

Risk

A new CLI surface plus three optional wire fields and one new event variant, all defaulted, so a client that ignores them behaves as before.

Two changes reach beyond the new surface and deserve a second look. The file tools now return a ToolResult where they returned a string: the model-facing text is unchanged and the registry already accepted both shapes, but every caller of write_file and edit_file goes through that boundary. And shell approval gains six command families, though only for a surface that registers them; the terminal registers none and behaves exactly as before.

One dodge in the approval walk stays open and is pinned by a test: a command runner (timeout 60 npm install) hides its inner command. Verified that the same wrapper already hides a delete from the existing check, so the new gate is no weaker than the surface it joins, and no stronger.

Rollback is a plain revert: no data migration and no persisted format change.

  • Security impact considered
  • Backward compatibility considered
  • Rollback path is clear for risky changes

Related Issues

N/A

ZuyiZhou and others added 4 commits August 22, 2026 22:23
Newline-delimited JSON-RPC 2.0: one complete JSON object per line, in both
directions. Verified against a real third-party ACP server, which answers a
single-line initialize request with a single-line result.

The error types are separate because they mean different things to a caller: a
remote error is the agent saying no, a protocol error is the agent saying
something that cannot be parsed, and a connection error is the agent not being
there at all. A caller reports the first two and retries none of them the same
way.

The framing alone, in a package that will also hold the client half. Both
directions share it deliberately: two serialisers would be free to drift on
ensure_ascii or on whether the newline is part of the frame, and a frame that
round-trips through one and not the other is the kind of bug that only shows up
against somebody else's implementation.

Co-authored-by: Claude (claude-opus-5[1m]) <[email protected]>
A consumer that maps a turn's lane to the subscription it belongs to should not
have to know whether the lane happens to be a plain session key. Nothing in
this repo formats a sub-conversation key yet, so every lane IS its session and
this returns what it was given; the encoding rule is written down so the first
channel that does format one cannot get the split wrong.

Splits on the FIRST separator, which is what makes the encoding safe: a session
key is channel:chat_id and never contains one, while whatever follows is
free-form text.

Co-authored-by: Claude (claude-opus-5[1m]) <[email protected]>
…pace

The built-in policy asks about exactly one family, deletion. That fits a
terminal the reader is watching and does not fit an agent running behind an
editor, where git push, npm install and curl -o would otherwise run with
nothing on screen.

Six matchers are added as an opt-in table rather than as policy: a surface
registers the families it wants asked about, because which effects need a
prompt is a property of the surface, not of the tool. The terminal registers
none and behaves exactly as before.

Matching walks the argv of every command in a pipeline or a chain, so a matcher
cannot be dodged by wrapping the command in sh -c or hiding it after a
semicolon. Quoting the parser cannot read closes the gate rather than opening
it, and the recursion is depth-bounded.

The prompt now says which family it is about. It used to read "Delete files
using a shell command" for every approval, because deletion was the only family
there was.

One dodge stays open, measured and pinned by a test: a command runner
(`timeout 60 npm install`, `nice rm -rf x`) hides its inner command from the
walk, because reading a runner's arguments needs a helper this module does not
have. Verified that the same wrapper already hides a delete from the existing
check, so this gate is no weaker than the surface it joins -- and no stronger.
The test asserts the current answer, so whoever adds the runner unwrap finds it
failing and moves the case into the group above it.

Co-authored-by: Claude (claude-opus-5[1m]) <[email protected]>
raven acp is now an agent an editor can launch: it negotiates, mints and
reloads sessions, runs a turn, streams the reply and its tool calls, asks
before acting, and answers a cancel while a turn is in flight.

The process model is a self-contained engine, not a client of a resident
gateway. An ACP agent IS a stdio child of the editor, so there is nothing to
mount onto unless a gateway happens to be running, and an agent that only works
when another service is up is not an agent an editor can launch.

Notes on the parts that are not obvious:

Every inbound frame is handled in its own task, because session/prompt is
suspended for as long as the turn takes and session/cancel has to be read
during it. Shutdown answers instead of cancelling: on EOF the pending prompts
are settled as cancelled and the handlers return through their own code, so a
finite input (a scripted client, a smoke test) still gets its replies.

A prompt is never answered with a JSON-RPC error. A failed turn is explained as
message content and then ends with a stop reason. Measured from the other side
on another implementation: an error in reply to a turn-shaped request makes
clients tear the whole turn down.

The schema is vendored and pinned, and its blindness is measured rather than
assumed: additionalProperties appears 118 times and is true in every one,
method is an unconstrained string, and params accepts null. Validating a frame
against the union therefore accepts an invented method name, so the tests
validate payloads against their own definition and check method names against
the metaschema separately.

Questions take one of two paths by what the client declared. With elicitation
forms, a question with no preset answers can come back as text. Without them it
rides a permission request and a synthesised tool call, which the payload marks
as synthesised rather than dressing up as a real one. The case that has no
answer at all -- a free-text question to a client with no elicitation -- is
recorded in the compatibility matrix instead of being papered over.

Secrets are redacted before anything reaches the client, including the value
under a key whose name says it is a secret. A credential file's path is
deliberately NOT redacted: a path is not a secret, and hiding it would hide
that the agent read credentials at all.

This surface asks about six command families. What that does not reach is
written down: a sub-agent builds its own tool with its own policy and no
responder, so a delegated command still runs unprompted.

The write tools now publish what they changed. Until now the diff field
declared on tool.complete had no producer here, so it was always null; the file
tools return a ToolResult carrying both the unified diff and the two file
versions, and the registry, the loop and the outlet carry them through. The
protocol needs the second form specifically: its diff content block is
{path, newText, oldText}, which a rendered diff cannot fill. A reply's files
reach the client the same way, as a media event carrying paths rather than
bytes.

Differences from the same layer in the sibling runtime, all deliberate and each
one measured rather than assumed:

* The compatibility matrix is corrected for this build in two rows. A model
  change during a turn is not refused, because config.set here has no busy
  guard. A sub-agent's output is untagged, because there is no direct sub-agent
  chat for the tag to distinguish.
* The self-verification suite is not included. It drives the ACP client's own
  capability probe, which this repo does not have, and porting a client
  subsystem to run one test file is the tail wagging the dog.
* The session/load smoke test is not included either, and the reason is worth
  knowing: nothing here can point a spawned agent at a throwaway session store.
  RAVEN_HOME steers tracing and the terminal runtime root only, and the config
  path has no override at all, so that test would either read the developer's
  real store or prove nothing. The load and replay paths keep their unit
  coverage; what is lost is the leg that read an actual file.
* The working-directory guard is checked against the configured home for the
  same reason, rather than against a temporary one.

Known fidelity losses, all in the matrix: tool_call_update always reports
completed because the event carries no success flag; rawInput is not sent
because it carries a command line verbatim; there is no plan because its
priority field is required and has no source here.

Co-authored-by: Claude (claude-opus-5[1m]) <[email protected]>
@ZuyiZhou
ZuyiZhou requested review from 0xKT and gloryfromca August 22, 2026 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant