Skip to content

feat: tole acp — Agent Client Protocol host (D2, issue #95) - #100

Open
ajianaz wants to merge 2 commits into
developfrom
feat/tole-acp
Open

ajianaz wants to merge 2 commits into
developfrom
feat/tole-acp

Conversation

@ajianaz

@ajianaz ajianaz commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

What

tole acp: tole presents itself as an Agent Client Protocol agent over line-delimited JSON-RPC stdio — editors (Zed et al.) drive durable tole sessions; tool approvals surface as permission requests in the editor.

Closes #95 (Phase D2).

Why

Editors are not MCP clients. ACP is the editor-facing protocol, and tole's model maps to it almost 1:1: ACP sessions ↔ durable JSONL sessions, the approval gate ↔ permission requests, session continuation ↔ crash-safe replay.

Changes

  • tole-cli/src/acp.rs: initialize / session-new / session-load / session-prompt; the final answer streams as an agent_message_chunk update; Write/Destructive calls surface session/request_permission to the editor human (Destructive consent is a genuine per-call decision — structurally stronger than MCP server mode's blanket absence); prompt accepts the ACP content-block array (and plain strings); run_command/git/gh operate on the SESSION cwd.
  • CI-safe integration test spawns the real binary: handshake, session lifecycle, durable session file, unknown-method error (no LLM needed).
  • Docs: README ecosystem bullet, CHANGELOG Added, epics D2 marked done.

Testing

  • fmt / clippy -D warnings / test --workspace ✅ (208 tests)
  • Live E2E (release binary, real provider): initialize → session/new → prompt → permission request surfaced and honored → tool executed → agent_message_chunk → stopReason end_turn → file on disk
  • Integration test: acp_handshake (handshake + session lifecycle + load) green
  • cora review --staged: No issues found (it also caught run_command/git using process cwd — fixed to session cwd)

tole as an ACP agent over stdio: editors and ACP-capable clients (Zed
et al.) drive durable tole sessions via line-delimited JSON-RPC, with
no new dependencies.

- initialize / session/new / session/load / session/prompt: ACP
  sessions map to the durable JSONL store (session jail = the client's
  cwd; load replays the existing log).
- session/prompt runs one full tole turn and delivers the final answer
  as an agent_message_chunk update before responding (v1: no intra-turn
  streaming — the synchronous turn loop is untouched).
- Approval bridge: Write/Destructive tool calls surface as
  session/request_permission requests to the EDITOR — the human in the
  client is the approver, which is why Destructive tools CAN be
  registered here with genuine per-call consent (unlike MCP server
  mode), without weakening anything.
- Prompt accepts the ACP content-block array form (and plain strings).
- run_command/git/gh-detect operate on the SESSION cwd, not the process
  cwd (CodeCora review finding on this PR).
- Provider config is only required when a prompt actually runs.
- CI-safe integration test spawns the real binary: initialize, session
  lifecycle, unknown-method error; live E2E covered permission flow,
  streaming chunk, and stop reasons.

Closes #95 (Phase D2).
Comment thread crates/tole-cli/src/acp.rs Fixed
Comment thread crates/tole-cli/src/acp.rs Fixed
Comment thread crates/tole-cli/src/acp.rs Fixed
@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown

🔍 Cora AI Code Review

Blocked — critical issues found.

🔴 Error (1)

  • crates/tole-cli/src/acp.rs:316 — run_prompt acquires lock_sessions(&sessions) at entry and holds the guard for the whole turn, including AcpPrompt's rx.recv_timeout(PERMISSION_TIMEOUT) (600s) — but that response can only be delivered by conn.route_response inside the reader loop. The reader's session/new/session/load arm itself calls lock_sessions(&sessions).map.insert(...) on the reader thread, so if the client sends session/new while a permission request is pending, the reader blocks, the permission reply is never routed, and the entire protocol freezes until the 600s timeout fires and denies the tool. This contradicts the in-code claim that 'this reader loop stays live for permission requests while the turn runs'.

Review powered by cora-code · BYOK · MIT

…ardening

CodeCora review on the ACP PR caught a real architectural bug: the
prompt arm wrapped a FRESH empty map per prompt (share_sessions used
mem::replace), so every session vanished after its first turn and
session/load could open a divergent second handle mid-turn.

- The session map now lives for the whole server lifetime (Arc clone
  per prompt thread); the map lock is held for the duration of a turn,
  which serializes a busy session instead of allowing divergent
  appends.
- Poisoning-tolerant locking: one panicking turn no longer bricks the
  ACP server.
- sessionId validation (charset, length, no separators/parent refs)
  blocks path traversal via session/load before ids touch the
  filesystem; regression tests cover multi-session persistence and
  traversal rejection.
conn.clone(),
) {
Ok(state) => {
lock_sessions(&sessions)
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.

feat: tole acp — Agent Client Protocol host (Phase D2)

2 participants