feat(acp): kaibo acp — ACP v1 front door serving the consult loop (prototype) - #124
Draft
tobert wants to merge 3 commits into
Draft
feat(acp): kaibo acp — ACP v1 front door serving the consult loop (prototype)#124tobert wants to merge 3 commits into
tobert wants to merge 3 commits into
Conversation
Adds a second protocol front door alongside MCP: `kaibo acp` runs kaibo as an Agent Client Protocol v1 agent on stdio, so Zed/Toad/JetBrains-style clients can drive kaibo directly instead of through an MCP client. This chunk is scaffold and handshake only — initialize, session/new, session/prompt, session/set_mode, and session/cancel all answer over the real wire, but session/prompt returns a canned notice instead of running the consult loop. Wiring run_kaish/the model team into the prompt turn is chunk 2. Decisions worth a second look: - `agent-client-protocol` is exact-pinned at `=1.3.0` deliberately, not a floating `"1"`. Read the real crate source (not memory) via a scratch probe crate and docs.rs: the 1.x line reshaped substantially even within minor versions (a builder + typed-dispatch API replacing an older fixed-trait/`AgentSideConnection` shape our starting assumptions expected from a possible older release) and crates.io's `1.x` moved fast (0.x -> 1.x -> 2.0.0 in one summer, 2.0.0 being the v2 DRAFT with its own `unstable_protocol_v2` feature gate). Verified pure-Rust and aws-lc/mimalloc/TLS-free with this dep present (`cargo tree -i` both empty) — its transport stack is async-io/blocking/futures, no TLS of its own. It carries `async-process` transitively for its CLIENT role (spawning a subprocess agent); dead weight on our agent-only surface. - Bumped `rust-version` 1.85 -> 1.88: 1.1.0+ of the crate requires it (async closures in its handler-registration API), so the old MSRV would have silently resolved the pin backward to 1.0.1's superseded API. The installed toolchain (1.96) already exceeds both floors; this only makes the manifest honest about the new one now that ACP is a hard dependency. - No dedicated thread or `LocalSet`, contrary to the working assumption going in. The crate's connection is executor-agnostic and `Send` throughout (`ConnectTo::connect_to` returns `impl Future<..> + Send`, every handler closure must itself be `Send`) — unlike the `!Send` kaish kernel, which stays on its own `KaishWorker` thread for an unrelated reason (rig tools need `Send` futures). `kaibo acp` runs on the existing multi-thread tokio runtime, same as `main`'s other front doors. Chunk 2 drives `KaishWorker` through its already-`Send` channel handle from inside a `Send` ACP handler, the same way `run_kaish` does today. - Session modes are wired to real cast names now, not stubbed: each configured cast becomes one advertised `SessionMode`, mode id == cast name, current mode starts on the config's default cast. `session/set_mode` validates against the roster and records the selection per session; chunk 2 is what actually reads it. - ACP sessions get their own tiny in-memory table (`session-N` ids, the `job-N` style already used in jobs.rs), not the existing consult `SessionStore`/durable store — an ACP session (cwd, MCP servers, mode) is a different shape than a consult session's question/answer replay history. Chunk 2 decides how much of this rides on the existing store. Tested with a real Client role from the same crate driving the Agent builder over a `tokio::io::duplex` (via `tokio-util`'s `compat` bridge to `futures::AsyncRead`/`AsyncWrite`) — actual JSON-RPC bytes on the wire, no mocks, no network: initialize negotiates v1 with no auth methods, session/new returns an id and advertises every configured cast as a mode, and session/prompt streams the canned `session/update` before a `StopReason::EndTurn` response. Full existing suite stays green (637 lib tests + all integration suites, including tests/no_write_path.rs and tests/sandbox.rs unchanged — this adds no filesystem writes). `cargo tree -i aws-lc-rs`/`-i mimalloc` both empty. Local prototype per instructions: no push, no PR. Co-authored-by: Claude Sonnet <noreply@anthropic.com>
session/prompt now drives the real consult loop instead of chunk 1's canned reply. It resolves the session's current mode (cast) through the same shared Resolver every other front door uses (Arm::from_slot — the single live construction point), extracts the prompt's text content blocks, and runs consult() with the ACP session id doubling as the key into kaibo's own in-memory multi-turn Sessions store — so successive prompts in one ACP session replay through the real consult_session_turn history/record machinery, not a cut-down stand-in. Decisions worth a second look: - The event loop can't read a new message while a request handler's own future is still running (per the crate's own ConnectionTo::spawn doc), so session/prompt can't just .await the consult call inline — that would make session/cancel unreachable for the whole turn. The handler resolves cast/ arms/context synchronously, spawns the actual consult() on its own tokio::spawn (kept for its AbortHandle — cx.spawn doesn't hand one back), and returns immediately; the response is sent from a second task (via cx.spawn, matching the SDK's own documented cancellation pattern) that awaits the first and answers once it lands. - session/cancel aborts that AbortHandle directly — the same spawn-then- AbortHandle shape src/jobs.rs already uses for job_cancel. A cancelled task's JoinError::is_cancelled() becomes StopReason::Cancelled, per spec. This is ACP's own domain-level session/cancel notification, distinct from the SDK's low-level JSON-RPC $/cancel_request (a different mechanism for cancelling one outstanding request by id) — not touched here. - Progress renders as session/update through AcpProgressSink. kaibo's PhaseEvent granularity is per-milestone, not token-streamed: SweepStarted/ SweepFinished open and close one ACP ToolCall (the one pair the engine actually emits 1:1); Attached updates that same open call; KaishRun opens its own ToolCall left InProgress and never explicitly closed, because kaibo has no matching "finished" beat for it today — reporting only what kaibo actually knows rather than fabricating a completion. The bookending PhaseStarted/PhaseFinished/TurnCapReached beats are meta narration, not a tool call, so they render as AgentThoughtChunk text (kaibo's own PhaseEvent::message() one-liner — the same text the MCP progress notifications and the CLI's TerminalSink already show). - session/prompt accepts only ContentBlock::Text in this build; an image/ audio/resource block is refused with a clear error. consult's vision plumbing (view_image, ConsultAttachment::Image) expects a file path under the project root, not inline wire bytes, so accepting one honestly needs a real spooling hand-off this chunk doesn't build. Text-only is the honest baseline ACP itself requires every agent to support. - AcpAgentState::new is now fallible (Resolver::from_config can fail on a bad --root/--allow-path) and takes Arc<Config> directly rather than &Config — cli.rs's run_acp handles the error as a setup rejection, same as every other front door. - Test seam: AcpAgentState::new_scripted builds a real Resolver (so cast metadata, prompts, sandbox all come from genuine config) but resolves session/prompt's arms from a cast-name-keyed map of pre-built ScriptedClient arms instead of Resolver::arm's real backend construction, which can only ever build a networked client. Mirrors how consult's own offline tests hand a ScriptedClient through Arm::new; no parallel harness. Ten tests cover the wire end to end over tokio::io::duplex: the full prompt -> real consult loop -> session/update sequence -> EndTurn path (with the provenance footer asserted), multi-turn replay, a set_mode switch actually routing to the new cast's model, cancel mid-turn yielding StopReason::Cancelled, a no-op cancel on an idle session, and a non-text content block being refused. All ten pass; the full suite (642 lib tests + every integration suite, including tests/no_write_path.rs and tests/sandbox.rs) stays green. append_warnings moves from a private server::render import to the same pub(crate) re-export CLI-shared helpers already use (with_provenance, consultation_failure_text) — ACP is now a third consumer needing the same answer-rendering fold. cargo build, cargo clippy --all-targets (zero warnings), and cargo tree -i for both aws-lc-rs and mimalloc (both empty) all pass. rustfmt was run only on the three edited leaf files (src/acp.rs, src/cli.rs, src/server/mod.rs), never a crate-root file alongside others. Local prototype per instructions: no push, no PR. Co-authored-by: Claude Sonnet <noreply@anthropic.com>
chunk 2's real consult loop threaded an ACP session's cwd straight into ConsultConfig/the sandbox root without ever running it through the same containment boundary the MCP path argument and the CLI --root/--allow-path enforce (Resolver::resolve_root, src/server/containment.rs). The gap: an ACP client's session/new could aim kaibo's read-only shell at any directory the process could read, not just the configured allowed set. Read-scope is supposed to be bounded for every front door alike, so this was a structural hole in an otherwise-enforced invariant, flagged in the chunk-2 report. Fix: AcpAgentState::resolve_session_cwd wraps the same Resolver::resolve_root the MCP consult tool calls, so a session/new cwd is canonicalized (symlinks, `..` resolved) and checked against the allowed set built from --root/ --allow-path/launch cwd -- no parallel canonicalizer. session/new now validates before minting a session id: a cwd outside the allowed set or one that can't canonicalize (nonexistent, not a directory) is refused with a clear invalid_params JSON-RPC error, matching resolve_root's own message text, never silently substituted or waved through. The canonicalized path becomes the SessionRecord's cwd, which is what every later session/prompt turn already threads into house_rules/orientation/ConsultConfig -- chunk 2's plumbing was honest, just unvalidated at the door. Tests (src/acp.rs, over the existing duplex-wire harness -- tests/ containment.rs is MCP-shaped around KaiboHandler/run_kaish and has no ACP transport harness to extend, so the wire tests carry this): - session_new_refuses_a_cwd_that_does_not_exist: chunk 1's original session_new_returns_an_id_and_advertises_cast_modes test predates containment and passed a deliberately nonexistent cwd expecting success; renamed and updated to expect refusal, since accepting that cwd was exactly the bug. - session_new_refuses_a_cwd_outside_the_allowed_set: a real, existing directory outside every allowed tree is refused the same way. - session_new_with_an_in_bounds_cwd_returns_an_id_and_advertises_cast_modes: a cwd equal to the configured root is accepted, and the session record holds the canonicalized path. - Every existing turn-driving test (prompt/cancel/set_mode/second-turn/ non-text-block) now builds its Config with root pinned to its project_dir() fixture (test_config_rooted), since those fixtures live outside the crate's inferred cwd and would otherwise be refused by the same check. Verified failing-first: temporarily bypassed resolve_session_cwd (return the raw cwd unchecked) and confirmed both refusal tests fail without the fix, then restored it. Gates: cargo build, clippy --all-targets (zero warnings), full cargo test (644+ passed, 0 failed), cargo tree -i aws-lc-rs / -i mimalloc both empty. Co-authored-by: Claude Sonnet <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A second protocol front door beside MCP:
kaibo acpruns kaibo as an Agent Client Protocol v1 agent on stdio, so ACP clients (Toad, Zed, JetBrains, Neovim) can drive kaibo's read-only consult loop directly — no MCP client in between. Three commits:9c0e237— scaffold + handshake: initialize, session/new, session/set_mode, session/cancel over the real wire; casts advertised as ACP session modes.1b7c823— the real consult loop: prompt → the sameResolver/Arm::from_slotresolve-then-run sequence as the MCPconsulttool; progress events forwarded assession/update(sweeps as ToolCalls, phase beats as thought chunks); provenance footer on answers; session replay across prompts viaconsult_session_turn; cancel aborts the in-flight turn withStopReason::Cancelled.7163f06— containment:session/new's client-supplied cwd goes through the sameresolve_rootallowed-set boundary as MCP/CLI; out-of-bounds or nonexistent cwd is a loud InvalidParams refusal, never a silent fallback. Failing-first proven (check removed → refusal tests fail → restored).Why / decisions
agent-client-protocol = "=1.3.0"): the v2 draft (2026-07-20) is two weeks old and explicitly not for default shipping; the crate's 2.0.0 tracks that draft. Dep tree verified pure Rust —cargo tree -i aws-lc-rsand-i mimallocboth empty, so the static-musl TLS invariant is untouched. Note: the crate carriesasync-processfor its client role; dead code in kaibo's agent role.tests/no_write_path.rs,tests/sandbox.rsunchanged and green); stdio-only holds (ACP is stdio JSON-RPC); operator-vs-model-team line holds (ACP client is the operator's proxy).Testing
Offline-only, no network: 12 ACP tests speak real JSON-RPC over
tokio::io::duplex, driving the actual consult loop throughScriptedClientresponders (test_support.rs) — progress-before-answer ordering, second-turn transcript replay, mode-switch model routing, cancel mid-turn, containment refusals. Full suite green: 644 lib tests + all integration suites; clippy zero warnings.Deliberately deferred (before this leaves draft)
run_kaishToolCalls stayInProgressin client UIs (the engine has no finish event for them).Attribution
Implementation by Claude Sonnet subagents in three passes (commits carry
Co-authored-by), orchestrated and reviewed in-session by Claude Fable 5, directed by Amy.🤖 Generated with Claude Code