Skip to content

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

Merged
ajianaz merged 6 commits into
developfrom
feat/acp-d2
Sep 28, 2026
Merged

ajianaz merged 6 commits into
developfrom
feat/acp-d2

Conversation

@ajianaz

@ajianaz ajianaz commented Sep 28, 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.

(Supersedes #100 — that PR's branch had workflow delivery stuck; identical changes.)

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 (CodeCora finding, fixed).
  • Session-map lifetime fixes from CodeCora review: the map persists across prompts (a first-run bug replaced it with a fresh map per turn — sessions vanished after one prompt); poisoning-tolerant locking; sessionId validation blocks path traversal; busy flag serializes concurrent turns panic-safely.
  • CI-safe integration test spawns the real binary: handshake, session lifecycle (multi-session persistence), load, traversal rejection, unknown-method error.
  • Docs: README ecosystem bullet, CHANGELOG Added, epics D2 marked done.

Testing

  • fmt / clippy -D warnings / test --workspace ✅ (210 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; three consecutive prompts in one session (map-lifetime regression)
  • Integration test: acp_handshake green
  • cora review --staged: No issues found

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).
…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.
CodeCora review deadlock finding: run_prompt held the session-map lock
for the WHOLE turn — including the up-to-600s permission wait — and the
reader loop's session/new/load arm takes the same lock. A client
opening a session while a permission request was pending froze protocol
routing until the timeout denied the tool.

- SessionState storage/registry/first_prompt_done are now Arc-wrapped;
  the map lock is held only to take handles and reject a busy session;
  the running turn locks its OWN storage mutex.
- Busy flag with a panic-safe Drop guard: concurrent turns on one
  session are refused ("session is busy"), and a panicking turn
  un-busies via Drop.
- Live-verified: three consecutive prompts in one session all
  end_turn (previously the map swap broke even single-turn
  persistence); handshake integration tests stay green.
Resolves crates/tole-cli/src/main.rs: the scan-3 wave evolved the Mcp
arm (plan-mode filter, loud-bail for pre/posttool hooks) while this
branch added the Acp arm — both kept; the Acp arm gains the same
loud-bail rule for unwired hooks and threads host.plan_mode through
run_acp/open_session (plan-mode serves a read-only registry).

Also takes develop's newer chat help copy (CLI copy sweep #104).
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 28, 2026 •

Copy link
Copy Markdown

🔍 Cora AI Code Review

✅ No issues found. Code looks good!


Review powered by cora-code · BYOK · MIT

// A turn holds its session's storage lock and marks it
// busy; loading a busy id would open a SECOND handle on
// the same JSONL mid-turn (CodeCora scan 2026-09-28).
let busy_now = {
…error routing

- plan_mode now REALLY filters: open_session registered every tool and
  the early return still handed back the full registry — under --yes
  that pre-authorized writes in a supposedly read-only session. Write/
  Destructive registrations are now skipped entirely when plan_mode is
  active (read_file/cora_search/uteke_recall/job_poll remain).
- session/load refuses a busy session: the old path opened a second
  JsonlStorage handle on the same JSONL while a turn was running,
  allowing divergent concurrent appends (the orphaned busy flag no
  longer guarded anything after the state was replaced).
- client ERROR replies to session/request_permission are routed like
  results — an errored/cancelled permission now fails closed
  immediately instead of hanging the turn for the full 600s timeout.
- also: duplicated too_many_arguments attribute removed (CI clippy).
@ajianaz
ajianaz merged commit 065c820 into develop Sep 28, 2026
16 checks passed
@ajianaz
ajianaz deleted the feat/acp-d2 branch September 28, 2026 04:42
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.

2 participants