diff --git a/crates/maestro-acp/tests/spawn.rs b/crates/maestro-acp/tests/spawn.rs index dd860b8..015b139 100644 --- a/crates/maestro-acp/tests/spawn.rs +++ b/crates/maestro-acp/tests/spawn.rs @@ -1,5 +1,5 @@ //! `maestro-acp` against the fake adapter in `tests/fake_acp_agent.py`, plus -//! one ignored test against the real `claude-code-acp`. +//! ignored smoke tests against each maintained adapter. use std::collections::HashMap; use std::path::{Path, PathBuf}; @@ -268,7 +268,7 @@ async fn advertised_settings_are_normalized_and_mutable() { async fn terminate_sends_cancel_then_sigterm() { let fake = Fake::new(); let cwd = TempDir::new().unwrap(); - let session = maestro_acp::spawn(fake.config(cwd.path())) + let session = maestro_acp::spawn(fake.config(cwd.path()).adapter(AcpAdapter::Codex)) .await .expect("spawn"); let pid = session.pid(); @@ -309,7 +309,7 @@ async fn terminate_is_satisfied_by_an_exit_on_cancel() { async fn handshake_error_fails_the_spawn_and_reaps_the_child() { let fake = Fake::new(); let cwd = TempDir::new().unwrap(); - let mut config = fake.config(cwd.path()); + let mut config = fake.config(cwd.path()).adapter(AcpAdapter::Codex); config.env.insert("FAKE_ACP_INIT_FAIL".into(), "1".into()); let err = maestro_acp::spawn(config).await.expect_err("init fails"); @@ -404,7 +404,10 @@ fn chunk_text(n: &maestro_acp::schema::SessionNotification) -> String { async fn load_session_resumes_the_stored_id_and_drops_the_replay() { let fake = Fake::new(); let cwd = TempDir::new().unwrap(); - let mut config = fake.config(cwd.path()).load_session("fake-old"); + let mut config = fake + .config(cwd.path()) + .adapter(AcpAdapter::Codex) + .load_session("fake-old"); config.env.insert("FAKE_ACP_LOAD_REPLAY".into(), "3".into()); let session = maestro_acp::spawn(config).await.expect("spawn"); @@ -437,7 +440,10 @@ async fn load_session_resumes_the_stored_id_and_drops_the_replay() { async fn load_session_requires_the_load_capability() { let fake = Fake::new(); let cwd = TempDir::new().unwrap(); - let mut config = fake.config(cwd.path()).load_session("fake-old"); + let mut config = fake + .config(cwd.path()) + .adapter(AcpAdapter::Codex) + .load_session("fake-old"); config.env.insert("FAKE_ACP_NO_LOAD".into(), "1".into()); let err = maestro_acp::spawn(config) @@ -452,7 +458,10 @@ async fn load_session_requires_the_load_capability() { async fn load_session_error_fails_the_spawn() { let fake = Fake::new(); let cwd = TempDir::new().unwrap(); - let mut config = fake.config(cwd.path()).load_session("fake-old"); + let mut config = fake + .config(cwd.path()) + .adapter(AcpAdapter::Codex) + .load_session("fake-old"); config.env.insert("FAKE_ACP_LOAD_FAIL".into(), "1".into()); let err = maestro_acp::spawn(config).await.expect_err("load fails"); @@ -476,3 +485,21 @@ async fn real_adapter_handshake() { .await; assert!(session.has_exited()); } + +/// End to end against the maintained Codex adapter. It needs a locally +/// authenticated Codex CLI, so CI runs it only when explicitly requested. +#[tokio::test] +#[ignore] +async fn real_codex_adapter_handshake() { + let cwd = TempDir::new().unwrap(); + let session = + maestro_acp::spawn(SpawnConfig::new(cwd.path(), HashMap::new()).adapter(AcpAdapter::Codex)) + .await + .expect("real codex-acp handshake"); + assert!(!session.session_id().is_empty()); + assert!(pid_alive(session.pid())); + session + .terminate(maestro_acp::CANCEL_GRACE, maestro_acp::TERM_GRACE) + .await; + assert!(session.has_exited()); +} diff --git a/crates/maestro-daemon/src/projection.rs b/crates/maestro-daemon/src/projection.rs index f320863..6629b2b 100644 --- a/crates/maestro-daemon/src/projection.rs +++ b/crates/maestro-daemon/src/projection.rs @@ -573,6 +573,58 @@ mod tests { ); } + #[test] + fn projects_the_codex_adapter_fixture_without_provider_specific_rows() { + let fixture = include_str!("../tests/fixtures/codex-acp-session.jsonl"); + let events = fixture + .lines() + .map(serde_json::from_str::) + .collect::, _>>() + .expect("Codex ACP fixture follows the supported schema"); + let mut projection = Projection::default(); + + for (seq, event) in events.iter().enumerate() { + projection.apply((seq + 1) as u64, event); + } + + let rows = projection.snapshot(events.len() as u64).rows; + assert_eq!( + rows.iter().map(|row| row.kind).collect::>(), + [ + TranscriptRowKind::User, + TranscriptRowKind::AgentThought, + TranscriptRowKind::ToolCall, + TranscriptRowKind::ToolDiff, + TranscriptRowKind::ToolCall, + TranscriptRowKind::ToolCall, + TranscriptRowKind::ToolCall, + TranscriptRowKind::ToolCall, + TranscriptRowKind::Plan, + TranscriptRowKind::Notice, + TranscriptRowKind::Notice, + TranscriptRowKind::AgentText, + ] + ); + assert_eq!(rows[2].data["rawInput"]["command"], "rg TODO"); + assert_eq!(rows[2].data["rawOutput"]["exit_code"], 0); + assert_eq!(rows[4].data["_meta"]["is_mcp_tool_call"], true); + assert_eq!(rows[5].data["rawInput"]["query"], "Agent Client Protocol"); + assert_eq!( + rows[7].data["content"][0]["content"]["mimeType"], + "image/png" + ); + + let state = projection.state(); + assert_eq!(state.model.as_deref(), Some("gpt-5.3-codex")); + assert_eq!(state.mode.as_deref(), Some("agent")); + assert_eq!(state.usage.as_ref().unwrap()["used"], 1200); + assert_eq!(state.context.as_ref().unwrap()["size"], 258400); + assert_eq!( + state.config_options.as_ref().unwrap()["configOptions"][2]["currentValue"], + "high" + ); + } + #[test] fn coalesces_anonymous_stream_chunks_without_joining_turns() { let mut projection = Projection::default(); diff --git a/crates/maestro-daemon/tests/fixtures/codex-acp-session.jsonl b/crates/maestro-daemon/tests/fixtures/codex-acp-session.jsonl new file mode 100644 index 0000000..bd7ae2a --- /dev/null +++ b/crates/maestro-daemon/tests/fixtures/codex-acp-session.jsonl @@ -0,0 +1,16 @@ +{"sessionId":"codex-session","update":{"sessionUpdate":"user_message_chunk","messageId":"user-1","content":{"type":"text","text":"Inspect the repository"}}} +{"sessionId":"codex-session","update":{"sessionUpdate":"agent_thought_chunk","messageId":"thought-1","content":{"type":"text","text":"I should inspect the files first."}}} +{"sessionId":"codex-session","update":{"sessionUpdate":"tool_call","toolCallId":"command-1","kind":"execute","title":"rg TODO","status":"in_progress","rawInput":{"command":"rg TODO","cwd":"/workspace"}}} +{"sessionId":"codex-session","update":{"sessionUpdate":"tool_call_update","toolCallId":"command-1","status":"completed","rawOutput":{"formatted_output":"src/lib.rs: TODO","exit_code":0}}} +{"sessionId":"codex-session","update":{"sessionUpdate":"tool_call","toolCallId":"change-1","kind":"edit","title":"Editing files","status":"completed","content":[{"type":"diff","path":"src/lib.rs","oldText":"old","newText":"new"}]}} +{"sessionId":"codex-session","update":{"sessionUpdate":"tool_call","toolCallId":"mcp-1","kind":"execute","title":"mcp.github.search","status":"completed","rawInput":{"server":"github","tool":"search","arguments":{"query":"ACP"}},"rawOutput":{"result":"found"},"_meta":{"is_mcp_tool_call":true}}} +{"sessionId":"codex-session","update":{"sessionUpdate":"tool_call","toolCallId":"web-1","kind":"search","title":"Search the web for ACP","status":"completed","rawInput":{"type":"webSearch","query":"Agent Client Protocol"}}} +{"sessionId":"codex-session","update":{"sessionUpdate":"tool_call","toolCallId":"image-view-1","kind":"read","title":"View Image /workspace/diagram.png","status":"completed","content":[{"type":"content","content":{"type":"resource_link","name":"/workspace/diagram.png","uri":"/workspace/diagram.png"}}]}} +{"sessionId":"codex-session","update":{"sessionUpdate":"tool_call","toolCallId":"image-gen-1","title":"Image generation","status":"completed","content":[{"type":"content","content":{"type":"image","data":"aW1hZ2U=","mimeType":"image/png"}}],"rawOutput":{"path":"/workspace/generated.png"}}} +{"sessionId":"codex-session","update":{"sessionUpdate":"plan","entries":[{"content":"Implement compatibility fixture","priority":"medium","status":"in_progress"}]}} +{"sessionId":"codex-session","update":{"sessionUpdate":"usage_update","used":1200,"size":258400}} +{"sessionId":"codex-session","update":{"sessionUpdate":"current_mode_update","currentModeId":"agent"}} +{"sessionId":"codex-session","update":{"sessionUpdate":"config_option_update","configOptions":[{"id":"agent-mode","name":"Agent mode","category":"mode","type":"select","currentValue":"agent","options":[{"value":"read-only","name":"Read-only"},{"value":"agent","name":"Agent"}]},{"id":"model","name":"Model","category":"model","type":"select","currentValue":"gpt-5.3-codex","options":[{"value":"gpt-5.3-codex","name":"GPT-5.3-Codex"}]},{"id":"reasoning_effort","name":"Reasoning effort","category":"thought_level","type":"select","currentValue":"high","options":[{"value":"high","name":"High"}]},{"id":"fast-mode","name":"Fast mode","category":"model_config","type":"select","currentValue":"off","options":[{"value":"off","name":"Off"}]}]}} +{"sessionId":"codex-session","update":{"sessionUpdate":"available_commands_update","availableCommands":[{"name":"status","description":"Show session status"}]}} +{"sessionId":"codex-session","update":{"sessionUpdate":"session_info_update","title":"Codex session"}} +{"sessionId":"codex-session","update":{"sessionUpdate":"agent_message_chunk","messageId":"assistant-1","content":{"type":"text","text":"Done."}}} diff --git a/docs/STRUCTURED-VIEW.md b/docs/STRUCTURED-VIEW.md index d8600a1..1a928bf 100644 --- a/docs/STRUCTURED-VIEW.md +++ b/docs/STRUCTURED-VIEW.md @@ -16,10 +16,10 @@ does not exist yet, and nothing here describes it. A terminal session's agent runs inside a tmux pane; a structured session's agent runs as a subprocess of the daemon itself, speaking ACP over stdio. The -daemon is the ACP **client**; the adapter process, `claude-code-acp`, is the -ACP **agent** — it wraps Claude Code's own SDK and translates the model's -tool calls into ACP `session/update` notifications instead of drawing them to -a pane. `crates/maestro-acp` owns the daemon's side of that connection; +daemon is the ACP **client**; the provider adapter process is the ACP +**agent**. It translates the provider's events into ACP `session/update` +notifications instead of drawing them to a pane. `crates/maestro-acp` owns the +daemon's side of that connection; `crates/maestro-daemon/src/projection.rs` folds what it receives into the two projections below. @@ -37,30 +37,26 @@ ACP handle instead of a tmux pane). ## The adapter -`ADAPTER_BIN` (`claude-code-acp`) is resolved by walking `PATH` — the -session's own `PATH` env var if the spawn request set one, the daemon's -otherwise — and taking the first executable file named `claude-code-acp` on -it. Install it once per host: +The resolved structured command selects a provider adapter. Its binary is +resolved by walking the session's `PATH` (when set on the spawn request), then +the daemon's `PATH`, and taking the first executable with that name. -```sh -npm install -g --prefix ~/.local @zed-industries/claude-code-acp -``` +| Command | Adapter binary | Install command | Provider detail | +| --- | --- | --- | --- | +| `claude` | `claude-code-acp` | `npm install -g --prefix ~/.local @zed-industries/claude-code-acp` | Needs a logged-in Claude Code. | +| `codex` | `codex-acp` | `npm install -g --prefix ~/.local @agentclientprotocol/codex-acp` | Uses Codex CLI or adapter-configured authentication; it advertises model, reasoning-effort, agent-mode, and optional fast-mode configuration. | -That is `maestro_acp::INSTALL_COMMAND`, and it is not just documentation: it -is baked into `AcpError::AdapterMissing`'s message. When the adapter cannot -be found, spawn still creates the session — **Dead**, with that install -command as its `activity_detail` — so the sidebar names the fix instead of -the session silently never appearing (see `docs/INSTALL.md` for the full -install writeup, including the systemd `PATH` caveat). The adapter also needs -a logged-in Claude Code (`claude /login`); a missing login surfaces the same -way, via the adapter's own stderr tail in the failure message. +The selected install command is baked into `AcpError::AdapterMissing`. When an +adapter cannot be found, spawn still creates a **Dead** session with that +command as `activity_detail` (see `docs/INSTALL.md` for the systemd `PATH` +caveat). Provider authentication failures surface through the adapter's stderr +tail in the same field. The handshake is bounded by `HANDSHAKE_TIMEOUT` (60s) — long enough for a -slow machine to boot a node process and the Claude Code SDK behind it, never -long enough to hang a spawn indefinitely. Any handshake failure — missing -adapter, early exit, a protocol error, or the timeout — leaves no orphaned -process: `spawn` closes and reaps the child itself before returning the -error. +slow machine to boot an adapter and its provider process, never long enough +to hang a spawn indefinitely. Any handshake failure — missing adapter, early +exit, a protocol error, or the timeout — leaves no orphaned process: `spawn` +closes and reaps the child itself before returning the error. ## The ACP handshake @@ -69,17 +65,17 @@ declares, deliberately, almost nothing: | Capability | Declared | Why | | --- | --- | --- | -| `fs.readTextFile` | `false` | The adapter uses Claude Code's own Read tool instead of asking the daemon to read a file on its behalf — the same tool a terminal-view Claude Code session uses. | -| `fs.writeTextFile` | `false` | Same reasoning, for Write. | -| `terminal` | `false` | The adapter runs Claude Code's own Bash tool rather than asking the daemon to open a terminal for it. | +| `fs.readTextFile` | `false` | Providers read the session workspace through their own tool surface. | +| `fs.writeTextFile` | `false` | Providers write the session workspace through their own tool surface. | +| `terminal` | `false` | Providers run commands through their own tool surface. | Declining all three keeps the daemon out of the file/terminal surface entirely — the adapter already has direct access to the session's `cwd`, and routing every read, write, and shell command through the daemon would only add a hop with no new capability. Every other capability namespace ACP defines (session extensions, auth, elicitation) is simply left at its -default; the daemon does not advertise support because `claude-code-acp` has -never asked for it, not because it was weighed and refused. +default; the daemon does not advertise support because neither supported +adapter needs it, not because it was weighed and refused. The one client method that *is* implemented for real is `session/request_permission` (#139) — see [Approvals](#approvals) below. @@ -96,6 +92,14 @@ flight is dropped on arrival, deliberately: the daemon's own event log already holds that history, and forwarding it too would duplicate every row in the projections below. +Codex and Claude differ in what they emit, not in this lifecycle. Codex maps +shell commands, file changes, MCP calls, web search, image generation/view, +reasoning, plans, and token usage into standard ACP chunks and tool calls; +those fold into the same eight rows. Codex's `available_commands_update` and +`session_info_update`, like equivalent Claude updates, remain opaque +`Notice` rows. Its `session/load` capability is required for daemon recovery +just as it is for Claude. + ## The eight transcript row kinds `TranscriptRowKind` (`maestro-protocol`) is the alphabet a structured