From ba07bafc97f6e717cd473ff043694af29d64111a Mon Sep 17 00:00:00 2001 From: lucaronin Date: Sat, 2 May 2026 19:47:33 +0200 Subject: [PATCH] fix: expose codex mcp tools reliably --- docs/ARCHITECTURE.md | 4 +- src-tauri/src/codex_cli.rs | 146 +++++++++++++++++++++++++++++++++++-- 2 files changed, 142 insertions(+), 8 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2862b4e3..e539e78b 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -246,8 +246,8 @@ Full agent mode — spawns the selected local CLI agent as a subprocess with too 1. **Frontend** (`AiPanel` + `useCliAiAgent` + `aiAgentSession.ts` + `aiAgents.ts`) — one normalized session lifecycle for message state, reasoning blocks, tool action cards, response display, onboarding, default-agent selection, and the per-vault Safe / Power User permission mode shown in the panel header 2. **Backend orchestration** (`ai_agents.rs`) — normalizes agent availability, streaming, and the request permission mode before dispatching to per-agent adapters 3. **Shared runtime scaffold** (`cli_agent_runtime.rs`) — owns the common request shape, prompt wrapping, JSON-line subprocess lifecycle, normalized error/done handling, version probing, and Tolaria MCP server path resolution used by app-managed CLI agents -4. **Agent adapters** — Shared prompts are mode-aware on every turn, including turns with note context snapshots: Vault Safe tells agents not to use or advertise shell, while Power User tells shell-capable agents to keep local commands scoped to the active vault. Claude Code still uses `claude_cli.rs` with `acceptEdits`, strict Tolaria MCP config, and a scoped tool list: Safe enables file/search/edit tools only, while Power User adds Bash to the available tools and pre-approves Bash with `--allowedTools` without using dangerous bypass flags. Codex runtime specifics live in `codex_cli.rs`; Safe runs `codex --sandbox read-only --ask-for-approval untrusted exec --json`, while Power User runs `codex --sandbox workspace-write --ask-for-approval never exec --json` so shell execution stays enabled across repeated turns. OpenCode runs through `opencode run --format json` with transient permissions: Safe denies bash and external directories, while Power User allows bash but still denies external directories. Pi runs through `pi --mode json --no-session` with `npm:pi-mcp-adapter`; both modes currently share the same transient MCP config and the prompt does not promise shell for Pi Power User. Gemini runs through `gemini --output-format stream-json --prompt` so assistant message chunks, tool calls, and final errors are mapped from the CLI event stream instead of relying on a buffered `response` field. Gemini Safe uses `auto_edit` plus `tools.exclude=["run_shell_command"]`; Power User intentionally uses `yolo` against a trusted transient Tolaria MCP entry. OpenCode, Pi, and Gemini all launch from the active vault cwd with closed stdin and transient MCP config. All app-launched paths use hidden Windows launches and avoid dangerous permission-bypass flags. -5. **MCP Integration** — Claude receives the generated MCP config file path, Codex receives the same Tolaria MCP server via transient `-c mcp_servers.tolaria.*` config overrides, OpenCode receives it through `OPENCODE_CONFIG_CONTENT`, Pi receives it through a temporary `PI_CODING_AGENT_DIR/mcp.json` consumed by `pi-mcp-adapter`, and Gemini receives it through a temporary settings file pointed at by `GEMINI_CLI_SYSTEM_SETTINGS_PATH` +4. **Agent adapters** — Shared prompts are mode-aware on every turn, including turns with note context snapshots: Vault Safe tells agents not to use or advertise shell, while Power User tells shell-capable agents to keep local commands scoped to the active vault. Claude Code still uses `claude_cli.rs` with `acceptEdits`, strict Tolaria MCP config, and a scoped tool list: Safe enables file/search/edit tools only, while Power User adds Bash to the available tools and pre-approves Bash with `--allowedTools` without using dangerous bypass flags. Codex runtime specifics live in `codex_cli.rs`; Safe runs `codex --sandbox read-only --ask-for-approval untrusted exec --json`, while Power User runs `codex --sandbox workspace-write --ask-for-approval never exec --json` so shell execution stays enabled across repeated turns. OpenCode runs through `opencode run --format json` with transient permissions: Safe denies bash and external directories, while Power User allows bash but still denies external directories. Pi runs through `pi --mode json --no-session` with `npm:pi-mcp-adapter`; both modes currently share the same transient MCP config and the prompt does not promise shell for Pi Power User. Gemini runs through `gemini --output-format stream-json --prompt` so assistant message chunks, tool calls, and final errors are mapped from the CLI event stream instead of relying on a buffered `response` field. Gemini Safe uses `auto_edit` plus `tools.exclude=["run_shell_command"]`; Power User intentionally uses `yolo` against a trusted transient Tolaria MCP entry. Codex, OpenCode, Pi, and Gemini all launch from the active vault cwd with closed stdin and transient MCP config. All app-launched paths use hidden Windows launches and avoid dangerous permission-bypass flags. +5. **MCP Integration** — Claude receives the generated MCP config file path, Codex receives the same Tolaria MCP server via transient `-c mcp_servers.tolaria.*` config overrides using Tolaria's resolved Node path plus `VAULT_PATH` and `WS_UI_PORT`, OpenCode receives it through `OPENCODE_CONFIG_CONTENT`, Pi receives it through a temporary `PI_CODING_AGENT_DIR/mcp.json` consumed by `pi-mcp-adapter`, and Gemini receives it through a temporary settings file pointed at by `GEMINI_CLI_SYSTEM_SETTINGS_PATH` CLI-agent availability intentionally does not depend only on the desktop app's inherited `PATH`. The detectors check the current process path, the user's login shell, and supported local/toolchain install locations such as native `~/.local/bin`, local `~/.claude/local`, Mise/asdf shims, nvm-managed Node installs, npm-global, Homebrew, Windows `%APPDATA%\npm`/pnpm/Scoop shims, Windows `.exe` launchers, and the macOS Codex app resource path so first-run onboarding works on fresh macOS and Windows installs. App-managed CLI spawns also extend the child process `PATH` with the resolved binary directory plus those common toolchain directories, which lets GUI-launched macOS sessions run Homebrew/npm shims and their `node`-backed MCP subprocesses even when Finder/Dock did not inherit a terminal shell path. diff --git a/src-tauri/src/codex_cli.rs b/src-tauri/src/codex_cli.rs index 92a2e2fb..dedb9ca3 100644 --- a/src-tauri/src/codex_cli.rs +++ b/src-tauri/src/codex_cli.rs @@ -230,6 +230,7 @@ fn build_codex_command( .args(args) .arg(prompt) .current_dir(vault_path) + .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::piped()); command @@ -240,6 +241,7 @@ fn build_codex_args( last_message_path: Option<&Path>, ) -> Result, String> { let mcp_server_path = crate::cli_agent_runtime::mcp_server_path_string()?; + let node_path = crate::mcp::find_node()?; let mut args = vec![ "--sandbox".into(), @@ -251,14 +253,11 @@ fn build_codex_args( "-C".into(), request.vault_path.clone(), "-c".into(), - r#"mcp_servers.tolaria.command="node""#.into(), + codex_config_string("mcp_servers.tolaria.command", &node_path.to_string_lossy()), "-c".into(), - format!(r#"mcp_servers.tolaria.args=["{}"]"#, mcp_server_path), + codex_config_string_list("mcp_servers.tolaria.args", &[mcp_server_path.as_str()]), "-c".into(), - format!( - r#"mcp_servers.tolaria.env={{VAULT_PATH="{}"}}"#, - request.vault_path - ), + codex_mcp_env_config(&request.vault_path), ]; if let Some(path) = last_message_path { @@ -269,6 +268,30 @@ fn build_codex_args( Ok(args) } +fn codex_config_string(key: &str, value: &str) -> String { + format!(r#"{key}="{}""#, toml_escape(value)) +} + +fn codex_config_string_list(key: &str, values: &[&str]) -> String { + let values = values + .iter() + .map(|value| format!(r#""{}""#, toml_escape(value))) + .collect::>() + .join(","); + format!("{key}=[{values}]") +} + +fn codex_mcp_env_config(vault_path: &str) -> String { + format!( + r#"mcp_servers.tolaria.env={{VAULT_PATH="{}",WS_UI_PORT="9711"}}"#, + toml_escape(vault_path) + ) +} + +fn toml_escape(value: &str) -> String { + value.replace('\\', r#"\\"#).replace('"', r#"\""#) +} + fn codex_sandbox(permission_mode: crate::ai_agents::AiAgentPermissionMode) -> &'static str { match permission_mode { crate::ai_agents::AiAgentPermissionMode::Safe => "read-only", @@ -565,6 +588,35 @@ mod tests { } } + #[test] + fn build_codex_args_uses_resolved_mcp_node_and_ui_bridge_env() { + let args = build_codex_args( + &AgentStreamRequest { + message: "Read [[Test note]]".into(), + system_prompt: None, + vault_path: "/tmp/vault".into(), + permission_mode: AiAgentPermissionMode::Safe, + }, + None, + ) + .unwrap(); + + let command_override = args + .iter() + .find(|arg| arg.starts_with("mcp_servers.tolaria.command=")) + .expect("Codex should receive a transient Tolaria MCP command"); + + assert!( + !command_override.ends_with(r#""node""#), + "Codex MCP command should use Tolaria's resolved Node path, got {command_override}" + ); + assert!( + command_override.contains('/'), + "Codex MCP command should be an absolute Node path, got {command_override}" + ); + assert!(args.iter().any(|arg| arg.contains(r#"WS_UI_PORT="9711""#))); + } + #[test] fn build_codex_command_keeps_agent_process_contract() { let binary = PathBuf::from("codex"); @@ -686,6 +738,88 @@ exit 2 assert!(matches!(events.last(), Some(AiAgentStreamEvent::Done))); } + #[cfg(unix)] + #[test] + fn run_codex_agent_stream_closes_stdin_even_when_parent_stdin_pipe_is_open() { + use std::io::Read; + use std::time::{Duration, Instant}; + + let mut child = std::process::Command::new(std::env::current_exe().unwrap()) + .arg("codex_stdin_probe_parent_child") + .arg("--ignored") + .arg("--nocapture") + .env("TOLARIA_CODEX_STDIN_PROBE_PARENT_CHILD", "1") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let child_stdin = child.stdin.take().unwrap(); + let mut stdout = child.stdout.take().unwrap(); + let mut stderr = child.stderr.take().unwrap(); + let deadline = Instant::now() + Duration::from_secs(5); + + let status = loop { + if let Some(status) = child.try_wait().unwrap() { + break status; + } + if Instant::now() >= deadline { + child.kill().unwrap(); + drop(child_stdin); + panic!("Codex stdin probe child timed out"); + } + std::thread::sleep(Duration::from_millis(10)); + }; + + drop(child_stdin); + let mut stdout_text = String::new(); + let mut stderr_text = String::new(); + stdout.read_to_string(&mut stdout_text).unwrap(); + stderr.read_to_string(&mut stderr_text).unwrap(); + + assert!( + status.success(), + "Codex stdin probe child failed with {status}\nstdout:\n{stdout_text}\nstderr:\n{stderr_text}" + ); + } + + #[cfg(unix)] + #[ignore = "spawned by run_codex_agent_stream_closes_stdin_even_when_parent_stdin_pipe_is_open"] + #[test] + fn codex_stdin_probe_parent_child() { + if std::env::var_os("TOLARIA_CODEX_STDIN_PROBE_PARENT_CHILD").is_none() { + return; + } + + let dir = tempfile::tempdir().unwrap(); + let vault = tempfile::tempdir().unwrap(); + let binary = executable_script( + dir.path(), + "codex", + r#"stdin="$(cat)" +if [ -n "$stdin" ]; then + echo "stdin was not closed" >&2 + exit 9 +fi +printf '%s\n' '{"type":"thread.started","thread_id":"stdin-ok"}' +printf '%s\n' '{"type":"item.completed","item":{"id":"msg_1","type":"agent_message","text":"stdin closed"}}' +"#, + ); + let mut events = Vec::new(); + let result = run_agent_stream_with_binary( + &binary, + codex_request(vault.path(), AiAgentPermissionMode::Safe), + |event| events.push(event), + ); + + assert_eq!(result.unwrap(), "stdin-ok"); + assert!(events.iter().any(|event| matches!( + event, + AiAgentStreamEvent::TextDelta { text } if text == "stdin closed" + ))); + assert!(matches!(events.last(), Some(AiAgentStreamEvent::Done))); + } + #[test] fn codex_binary_candidates_include_supported_macos_installs() { let home = PathBuf::from("/Users/alex");