fix: expose codex mcp tools reliably
This commit is contained in:
@@ -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.
|
||||
|
||||
|
||||
@@ -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<Vec<String>, 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::<Vec<_>>()
|
||||
.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");
|
||||
|
||||
Reference in New Issue
Block a user