From e727afdd81176d39d86cb6da3c8503c1d5e07fea Mon Sep 17 00:00:00 2001 From: lucaronin Date: Mon, 11 May 2026 12:30:44 +0200 Subject: [PATCH] fix: pipe oversized claude prompts on windows --- src-tauri/src/claude_cli.rs | 752 +++++++++-------------------- src-tauri/src/claude_invocation.rs | 606 +++++++++++++++++++++++ src-tauri/src/cli_agent_runtime.rs | 85 +++- src-tauri/src/lib.rs | 1 + 4 files changed, 908 insertions(+), 536 deletions(-) create mode 100644 src-tauri/src/claude_invocation.rs diff --git a/src-tauri/src/claude_cli.rs b/src-tauri/src/claude_cli.rs index 6603a9a8..108085fe 100644 --- a/src-tauri/src/claude_cli.rs +++ b/src-tauri/src/claude_cli.rs @@ -1,4 +1,3 @@ -use crate::ai_agents::AiAgentPermissionMode; pub use crate::cli_agent_runtime::AgentStreamRequest; use serde::{Deserialize, Serialize}; use std::collections::HashMap; @@ -51,15 +50,6 @@ pub struct ChatStreamRequest { pub session_id: Option, } -const CLAUDE_SAFE_AGENT_TOOLS: &str = "Read,Edit,MultiEdit,Write,Glob,Grep,LS"; -const CLAUDE_POWER_USER_AGENT_TOOLS: &str = "Read,Edit,MultiEdit,Write,Glob,Grep,LS,Bash"; -const CLAUDE_CHAT_DISALLOWED_TOOLS_COMPAT: &str = - "Bash,Glob,Grep,Read,Edit,Write,NotebookEdit,WebFetch,WebSearch,TodoWrite,Task,MultiEdit,LS"; -const CLAUDE_SAFE_DISALLOWED_TOOLS_COMPAT: &str = - "Bash,NotebookEdit,WebFetch,WebSearch,TodoWrite,Task"; -const CLAUDE_POWER_USER_DISALLOWED_TOOLS_COMPAT: &str = - "NotebookEdit,WebFetch,WebSearch,TodoWrite,Task"; - // --------------------------------------------------------------------------- // Finding the `claude` binary // --------------------------------------------------------------------------- @@ -103,7 +93,7 @@ fn find_claude_binary_in_user_shell() -> Option { user_shell_candidates() .into_iter() .filter(|shell| shell.exists()) - .find_map(|shell| command_path_from_shell(&shell, "claude")) + .find_map(|shell| claude_path_from_shell(&shell)) } fn user_shell_candidates() -> Vec { @@ -118,32 +108,30 @@ fn user_shell_candidates() -> Vec { shells } -fn command_path_from_shell(shell: &Path, command: &str) -> Option { +fn claude_path_from_shell(shell: &Path) -> Option { crate::hidden_command(shell) .arg("-lc") - .arg(format!("command -v {command}")) + .arg("command -v claude") .output() .ok() .and_then(|output| path_from_successful_output(&output)) } fn path_from_successful_output(output: &std::process::Output) -> Option { - if output.status.success() { - first_existing_path(&String::from_utf8_lossy(&output.stdout)) - } else { - None + if !output.status.success() { + return None; } -} -fn first_existing_path(stdout: &str) -> Option { - stdout.lines().find_map(|line| { - let trimmed = line.trim(); - if trimmed.is_empty() { - return None; - } - let candidate = PathBuf::from(trimmed); - candidate.exists().then_some(candidate) - }) + String::from_utf8_lossy(&output.stdout) + .lines() + .find_map(|line| { + let trimmed = line.trim(); + if trimmed.is_empty() { + return None; + } + let candidate = PathBuf::from(trimmed); + candidate.exists().then_some(candidate) + }) } fn claude_binary_candidates() -> Vec { @@ -178,11 +166,11 @@ fn claude_binary_candidates_for_home(home: &Path) -> Vec { PathBuf::from("/opt/homebrew/bin/claude"), PathBuf::from("/usr/local/bin/claude"), ]; - candidates.extend(nvm_node_binary_candidates_for_home(home, "claude")); + candidates.extend(nvm_claude_binary_candidates_for_home(home)); candidates } -fn nvm_node_binary_candidates_for_home(home: &Path, binary_name: &str) -> Vec { +fn nvm_claude_binary_candidates_for_home(home: &Path) -> Vec { let Ok(entries) = std::fs::read_dir(home.join(".nvm/versions/node")) else { return Vec::new(); }; @@ -191,7 +179,7 @@ fn nvm_node_binary_candidates_for_home(home: &Path, binary_name: &str) -> Vec>(); candidates.sort(); candidates @@ -226,227 +214,38 @@ where F: FnMut(ClaudeStreamEvent), { let bin = find_claude_binary()?; - let args = build_chat_args(&req); - let fallback_args = [build_chat_args_compat(&req)]; + let invocation = crate::claude_invocation::chat(&req); run_claude_subprocess( ClaudeSubprocessRequest { bin: &bin, - args: &args, - fallback_args: &fallback_args, + args: &invocation.args, + fallback_args: &invocation.fallback_args, + stdin_text: invocation.stdin_text.as_deref(), cwd: None, }, &mut emit, ) } -/// Build CLI arguments for a chat stream request. -fn build_chat_args(req: &ChatStreamRequest) -> Vec { - build_chat_args_with_tool_policy(req, "--tools", String::new()) -} - -fn build_chat_args_compat(req: &ChatStreamRequest) -> Vec { - build_chat_args_with_tool_policy( - req, - "--disallowedTools", - CLAUDE_CHAT_DISALLOWED_TOOLS_COMPAT.into(), - ) -} - -fn build_chat_args_with_tool_policy( - req: &ChatStreamRequest, - tool_flag: &str, - tool_value: String, -) -> Vec { - let mut args: Vec = vec![ - "-p".into(), - req.message.clone(), - "--output-format".into(), - "stream-json".into(), - "--verbose".into(), - "--include-partial-messages".into(), - tool_flag.into(), - tool_value, - ]; - - if let Some(ref sp) = req.system_prompt { - if !sp.is_empty() { - args.push("--system-prompt".into()); - args.push(sp.clone()); - } - } - - if let Some(ref sid) = req.session_id { - args.push("--resume".into()); - args.push(sid.clone()); - } - - args -} - /// Spawn `claude -p` with full tool access and MCP vault tools for an agent task. pub fn run_agent_stream(req: AgentStreamRequest, mut emit: F) -> Result where F: FnMut(ClaudeStreamEvent), { let bin = find_claude_binary()?; - let args = build_agent_args(&req)?; - let fallback_args = vec![ - build_agent_args_without_session_persistence(&req)?, - build_agent_args_compat(&req)?, - ]; + let invocation = crate::claude_invocation::agent(&req)?; run_claude_subprocess( ClaudeSubprocessRequest { bin: &bin, - args: &args, - fallback_args: &fallback_args, + args: &invocation.args, + fallback_args: &invocation.fallback_args, + stdin_text: invocation.stdin_text.as_deref(), cwd: Some(&req.vault_path), }, &mut emit, ) } -/// Build CLI arguments for an agent stream request. -/// Native tools (bash, read, write, edit) are enabled by default — no `--tools ""`. -fn build_agent_args(req: &AgentStreamRequest) -> Result, String> { - build_agent_args_with_tool_policy(req, ClaudeAgentToolPolicy::strict(req.permission_mode)) -} - -fn build_agent_args_without_session_persistence( - req: &AgentStreamRequest, -) -> Result, String> { - build_agent_args_with_tool_policy( - req, - ClaudeAgentToolPolicy::strict_without_session_persistence(req.permission_mode), - ) -} - -fn build_agent_args_compat(req: &AgentStreamRequest) -> Result, String> { - build_agent_args_with_tool_policy(req, ClaudeAgentToolPolicy::compat(req.permission_mode)) -} - -fn build_agent_args_with_tool_policy( - req: &AgentStreamRequest, - policy: ClaudeAgentToolPolicy, -) -> Result, String> { - let mcp_config = build_mcp_config(&req.vault_path)?; - - let mut args: Vec = vec![ - "-p".into(), - req.message.clone(), - "--output-format".into(), - "stream-json".into(), - "--verbose".into(), - "--include-partial-messages".into(), - "--mcp-config".into(), - mcp_config, - "--strict-mcp-config".into(), - "--permission-mode".into(), - "acceptEdits".into(), - policy.tool_flag.into(), - policy.tool_value.into(), - ]; - - if policy.include_session_persistence_flag { - args.push("--no-session-persistence".into()); - } - - if let Some(allowed_tools) = policy.preapproved_tools { - args.push("--allowedTools".into()); - args.push(allowed_tools.into()); - } - - if let Some(disallowed_tools) = policy.disallowed_tools { - args.push("--disallowedTools".into()); - args.push(disallowed_tools.into()); - } - - if let Some(ref sp) = req.system_prompt { - if !sp.is_empty() { - args.push("--append-system-prompt".into()); - args.push(sp.clone()); - } - } - - Ok(args) -} - -struct ClaudeAgentToolPolicy { - tool_flag: &'static str, - tool_value: &'static str, - include_session_persistence_flag: bool, - preapproved_tools: Option<&'static str>, - disallowed_tools: Option<&'static str>, -} - -impl ClaudeAgentToolPolicy { - fn strict(permission_mode: AiAgentPermissionMode) -> Self { - Self { - tool_flag: "--tools", - tool_value: agent_tools(permission_mode), - include_session_persistence_flag: true, - preapproved_tools: preapproved_agent_tools(permission_mode), - disallowed_tools: None, - } - } - - fn strict_without_session_persistence(permission_mode: AiAgentPermissionMode) -> Self { - Self { - include_session_persistence_flag: false, - ..Self::strict(permission_mode) - } - } - - fn compat(permission_mode: AiAgentPermissionMode) -> Self { - Self { - tool_flag: "--allowedTools", - tool_value: agent_tools(permission_mode), - include_session_persistence_flag: false, - preapproved_tools: None, - disallowed_tools: Some(disallowed_agent_tools_compat(permission_mode)), - } - } -} - -fn agent_tools(permission_mode: AiAgentPermissionMode) -> &'static str { - match permission_mode { - AiAgentPermissionMode::Safe => CLAUDE_SAFE_AGENT_TOOLS, - AiAgentPermissionMode::PowerUser => CLAUDE_POWER_USER_AGENT_TOOLS, - } -} - -fn preapproved_agent_tools(permission_mode: AiAgentPermissionMode) -> Option<&'static str> { - match permission_mode { - AiAgentPermissionMode::Safe => None, - AiAgentPermissionMode::PowerUser => Some("Bash"), - } -} - -fn disallowed_agent_tools_compat(permission_mode: AiAgentPermissionMode) -> &'static str { - match permission_mode { - AiAgentPermissionMode::Safe => CLAUDE_SAFE_DISALLOWED_TOOLS_COMPAT, - AiAgentPermissionMode::PowerUser => CLAUDE_POWER_USER_DISALLOWED_TOOLS_COMPAT, - } -} - -// --------------------------------------------------------------------------- -// Internal helpers -// --------------------------------------------------------------------------- - -/// Build a temporary MCP config JSON string pointing to the vault MCP server. -fn build_mcp_config(vault_path: &str) -> Result { - let mcp_server_path = crate::cli_agent_runtime::mcp_server_path_string()?; - let config = serde_json::json!({ - "mcpServers": { - "tolaria": { - "command": "node", - "args": [mcp_server_path], - "env": { "VAULT_PATH": vault_path } - } - } - }); - serde_json::to_string(&config).map_err(|e| format!("Failed to serialise MCP config: {e}")) -} - /// Mutable state accumulated across the JSON stream for a single subprocess. struct StreamState { session_id: String, @@ -459,12 +258,63 @@ struct StreamState { } struct ClaudeSubprocessRequest<'a> { - bin: &'a PathBuf, + bin: &'a Path, args: &'a [String], fallback_args: &'a [Vec], + stdin_text: Option<&'a str>, cwd: Option<&'a str>, } +struct ClaudeCommandRequest<'a> { + bin: &'a Path, + args: &'a [String], + cwd: Option<&'a str>, +} + +#[derive(Clone, Copy)] +struct ClaudeStderr<'a>(&'a str); + +impl<'a> ClaudeStderr<'a> { + fn is_empty(self) -> bool { + self.0.is_empty() + } + + fn lines(self) -> std::str::Lines<'a> { + self.0.lines() + } + + fn to_ascii_lowercase(self) -> String { + self.0.to_ascii_lowercase() + } +} + +struct ClaudeFailure<'a> { + stderr: ClaudeStderr<'a>, + status: ExitStatus, +} + +#[derive(Clone, Copy)] +struct TextDeltaChunk<'a>(&'a str); + +impl<'a> TextDeltaChunk<'a> { + fn is_empty(self) -> bool { + self.0.is_empty() + } + + fn as_str(self) -> &'a str { + self.0 + } +} + +#[derive(Clone, Copy)] +struct ToolUseId<'a>(&'a str); + +impl<'a> ToolUseId<'a> { + fn as_str(self) -> &'a str { + self.0 + } +} + /// Core subprocess runner shared by chat and agent modes. /// When `cwd` is `Some`, the subprocess starts with that working directory. fn run_claude_subprocess( @@ -486,10 +336,15 @@ where emitted_text: false, }; - let cmd = build_claude_command(request.bin, attempt_args, request.cwd)?; - let run = crate::cli_agent_runtime::run_json_line_process( - cmd, - "claude", + let cmd = build_claude_command(ClaudeCommandRequest { + bin: request.bin, + args: attempt_args, + cwd: request.cwd, + })?; + let process = crate::cli_agent_runtime::JsonLineProcess::new(cmd, "claude") + .with_stdin(request.stdin_text); + let run = crate::cli_agent_runtime::run_json_line_process_with_stdin( + process, emit, |message| ClaudeStreamEvent::Error { message }, |json, emit, session_id| { @@ -500,12 +355,16 @@ where if !run.status.success() && state.session_id.is_empty() { let has_fallback = index < request.fallback_args.len(); - if has_fallback && is_unsupported_claude_flag_error(&run.stderr_output) { + let stderr = ClaudeStderr(&run.stderr_output); + if has_fallback && is_unsupported_claude_flag_error(stderr) { continue; } emit(ClaudeStreamEvent::Error { - message: format_failed_claude_exit(&run.stderr_output, run.status), + message: format_failed_claude_exit(ClaudeFailure { + stderr, + status: run.status, + }), }); } @@ -517,48 +376,51 @@ where } fn build_claude_command( - bin: &Path, - args: &[String], - cwd: Option<&str>, + request: ClaudeCommandRequest<'_>, ) -> Result { - let target = crate::cli_agent_runtime::command_target_avoiding_windows_cmd_shim(bin)?; + let target = crate::cli_agent_runtime::command_target_avoiding_windows_cmd_shim(request.bin)?; let mut cmd = crate::hidden_command(&target.program); - crate::cli_agent_runtime::configure_agent_command_environment(&mut cmd, bin); + crate::cli_agent_runtime::configure_agent_command_environment(&mut cmd, request.bin); if let Some(first_arg) = target.first_arg { cmd.arg(first_arg); } - cmd.args(args) + cmd.args(request.args) .env_remove("CLAUDECODE") // prevent "nested session" guard .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::piped()); - if let Some(dir) = cwd { + if let Some(dir) = request.cwd { cmd.current_dir(dir); } Ok(cmd) } -fn format_failed_claude_exit(stderr_output: &str, status: ExitStatus) -> String { - if is_claude_auth_error(stderr_output) { +fn format_failed_claude_exit(failure: ClaudeFailure<'_>) -> String { + if is_claude_auth_error(failure.stderr) { return "Claude CLI is not authenticated. Run `claude auth login` in your terminal.".into(); } - if stderr_output.is_empty() { - format!("claude exited with status {status}") + if failure.stderr.is_empty() { + format!("claude exited with status {}", failure.status) } else { - stderr_output.lines().take(3).collect::>().join("\n") + failure + .stderr + .lines() + .take(3) + .collect::>() + .join("\n") } } -fn is_claude_auth_error(stderr_output: &str) -> bool { - let lower = stderr_output.to_ascii_lowercase(); +fn is_claude_auth_error(stderr: ClaudeStderr<'_>) -> bool { + let lower = stderr.to_ascii_lowercase(); ["not logged in", "authentication", "auth"] .iter() .any(|pattern| lower.contains(pattern)) } -fn is_unsupported_claude_flag_error(stderr_output: &str) -> bool { - let lower = stderr_output.to_ascii_lowercase(); +fn is_unsupported_claude_flag_error(stderr: ClaudeStderr<'_>) -> bool { + let lower = stderr.to_ascii_lowercase(); let mentions_compat_flag = ["--tools", "--no-session-persistence"] .iter() .any(|flag| lower.contains(flag)); @@ -671,7 +533,7 @@ where match delta["type"].as_str() { Some("text_delta") => { if let Some(text) = delta["text"].as_str() { - emit_text_delta(text, state, emit); + emit_text_delta(TextDeltaChunk(text), state, emit); } } Some("thinking_delta") => { @@ -727,12 +589,12 @@ fn dispatch_assistant_content_block( match block["type"].as_str() { Some("text") if emit_text => { if let Some(text) = block["text"].as_str() { - emit_text_delta(text, state, emit); + emit_text_delta(TextDeltaChunk(text), state, emit); } } Some("tool_use") => { if let (Some(id), Some(name)) = (block["id"].as_str(), block["name"].as_str()) { - let input = format_tool_input(&block["input"], state, id); + let input = format_tool_input(&block["input"], state, ToolUseId(id)); emit(ClaudeStreamEvent::ToolStart { tool_name: name.to_string(), tool_id: id.to_string(), @@ -744,7 +606,7 @@ fn dispatch_assistant_content_block( } } -fn emit_text_delta(text: &str, state: &mut StreamState, emit: &mut F) +fn emit_text_delta(text: TextDeltaChunk<'_>, state: &mut StreamState, emit: &mut F) where F: FnMut(ClaudeStreamEvent), { @@ -752,7 +614,7 @@ where state.emitted_text = true; } emit(ClaudeStreamEvent::TextDelta { - text: text.to_string(), + text: text.as_str().to_string(), }); } @@ -761,9 +623,9 @@ where fn format_tool_input( block_input: &serde_json::Value, state: &StreamState, - tool_id: &str, + tool_id: ToolUseId<'_>, ) -> Option { - if let Some(accumulated) = state.tool_inputs.get(tool_id) { + if let Some(accumulated) = state.tool_inputs.get(tool_id.as_str()) { if !accumulated.is_empty() { return Some(accumulated.clone()); } @@ -794,6 +656,7 @@ fn extract_tool_result_text(json: &serde_json::Value) -> Option { #[cfg(test)] mod tests { use super::*; + use crate::ai_agents::AiAgentPermissionMode; use std::ffi::OsStr; use std::ffi::OsString; use std::process::Command; @@ -814,76 +677,6 @@ mod tests { PathBuf::from(path.trim()) } - macro_rules! chat_request { - ($message:expr, None, None $(,)?) => { - ChatStreamRequest { - message: $message.into(), - system_prompt: None, - session_id: None, - } - }; - ($message:expr, Some($system_prompt:expr), None $(,)?) => { - ChatStreamRequest { - message: $message.into(), - system_prompt: Some($system_prompt.to_string()), - session_id: None, - } - }; - ($message:expr, None, Some($session_id:expr) $(,)?) => { - ChatStreamRequest { - message: $message.into(), - system_prompt: None, - session_id: Some($session_id.to_string()), - } - }; - } - - macro_rules! agent_request { - ($message:expr, None, $permission_mode:expr $(,)?) => { - AgentStreamRequest { - message: $message.into(), - system_prompt: None, - vault_path: "/tmp/vault".into(), - permission_mode: $permission_mode, - } - }; - ($message:expr, Some($system_prompt:expr), $permission_mode:expr $(,)?) => { - AgentStreamRequest { - message: $message.into(), - system_prompt: Some($system_prompt.to_string()), - vault_path: "/tmp/vault".into(), - permission_mode: $permission_mode, - } - }; - } - - macro_rules! assert_args_contain { - ($args:expr, [$($value:expr),+ $(,)?] $(,)?) => { - $( - assert!($args.contains(&$value.to_string()), "missing {}", $value); - )+ - }; - } - - macro_rules! assert_args_lack { - ($args:expr, [$($value:expr),+ $(,)?] $(,)?) => { - $( - assert!(!$args.contains(&$value.to_string()), "unexpected {}", $value); - )+ - }; - } - - macro_rules! assert_no_arg_contains { - ($args:expr, $fragment:expr $(,)?) => { - assert!(!$args.iter().any(|arg| arg.contains($fragment))); - }; - } - - fn arg_value_after<'a>(args: &'a [String], name: &str) -> Option<&'a str> { - let index = args.iter().position(|arg| arg == name)?; - args.get(index + 1).map(String::as_str) - } - fn assert_binary_candidates_include(home: &Path, expected: &[PathBuf]) { let candidates = claude_binary_candidates_for_home(home); for candidate in expected { @@ -917,115 +710,17 @@ mod tests { assert!(candidates.contains(&claude), "missing {}", claude.display()); } - #[test] - fn agent_args_use_safe_mode_without_bash_by_default() { - let args = build_agent_args(&agent_request!( - "Rename the note", - None, - AiAgentPermissionMode::Safe, - )) - .unwrap(); - - assert_args_contain!( - args, - ["--strict-mcp-config", "--permission-mode", "acceptEdits"] - ); - assert_args_contain!(args, ["Read,Edit,MultiEdit,Write,Glob,Grep,LS"]); - assert_no_arg_contains!(args, "Bash"); - assert_args_lack!(args, ["--allowedTools"]); - assert_args_lack!(args, ["--dangerously-skip-permissions"]); - } - - #[test] - fn agent_args_allow_bash_in_power_user_mode_without_dangerous_bypass() { - let args = build_agent_args(&agent_request!( - "Rename the note", - None, - AiAgentPermissionMode::PowerUser, - )) - .unwrap(); - - assert_args_contain!(args, ["--strict-mcp-config"]); - assert_args_contain!(args, ["Read,Edit,MultiEdit,Write,Glob,Grep,LS,Bash"]); - assert_args_lack!(args, ["--dangerously-skip-permissions"]); - } - - #[test] - fn agent_args_preapprove_bash_for_power_user_runs() { - let args = build_agent_args(&agent_request!( - "Run a local script", - None, - AiAgentPermissionMode::PowerUser, - )) - .unwrap(); - - assert_eq!(arg_value_after(&args, "--allowedTools"), Some("Bash")); - } - - #[test] - fn chat_args_compat_disallows_builtin_tools_when_tools_flag_is_unavailable() { - let args = build_chat_args_compat(&chat_request!("hello", None, None)); - - assert_args_contain!(args, ["--disallowedTools"]); - assert_args_lack!(args, ["--tools"]); - assert!(arg_value_after(&args, "--disallowedTools") - .is_some_and(|tools| tools.contains("NotebookEdit"))); - } - - #[test] - fn agent_args_compat_uses_allowed_and_disallowed_tools_without_removed_flags() { - let args = build_agent_args_compat(&agent_request!( - "Rename the note", - None, - AiAgentPermissionMode::Safe, - )) - .unwrap(); - - assert_eq!( - arg_value_after(&args, "--allowedTools"), - Some("Read,Edit,MultiEdit,Write,Glob,Grep,LS") - ); - assert_args_contain!(args, ["--disallowedTools"]); - assert_args_lack!(args, ["--tools"]); - assert_args_lack!(args, ["--no-session-persistence"]); - } - - #[test] - fn agent_args_without_session_persistence_keeps_tools_allowlist() { - let args = build_agent_args_without_session_persistence(&agent_request!( - "Rename the note", - None, - AiAgentPermissionMode::Safe, - )) - .unwrap(); - - assert_args_contain!(args, ["--tools", "Read,Edit,MultiEdit,Write,Glob,Grep,LS"]); - assert_args_lack!(args, ["--no-session-persistence"]); - } - #[test] fn unsupported_claude_flag_errors_are_detected() { - assert!(is_unsupported_claude_flag_error( + assert!(is_unsupported_claude_flag_error(ClaudeStderr( "error: unknown option '--tools'" - )); - assert!(is_unsupported_claude_flag_error( + ))); + assert!(is_unsupported_claude_flag_error(ClaudeStderr( "Unknown argument: --no-session-persistence" - )); - assert!(!is_unsupported_claude_flag_error( + ))); + assert!(!is_unsupported_claude_flag_error(ClaudeStderr( "Claude CLI is not authenticated" - )); - } - - #[test] - fn build_mcp_config_is_valid_json() { - if let Ok(config_str) = build_mcp_config("/tmp/test-vault") { - let parsed: serde_json::Value = serde_json::from_str(&config_str).unwrap(); - assert!(parsed["mcpServers"]["tolaria"]["command"].is_string()); - assert_eq!( - parsed["mcpServers"]["tolaria"]["env"]["VAULT_PATH"], - "/tmp/test-vault" - ); - } + ))); } // --- dispatch_event / dispatch_stream_event --- @@ -1047,13 +742,16 @@ mod tests { (state.session_id, events) } + #[derive(Clone, Copy)] + struct InitialSessionId<'a>(&'a str); + /// Run dispatch_event with a pre-set session_id. fn run_dispatch_with_sid( json: serde_json::Value, - initial_sid: &str, + initial_sid: InitialSessionId<'_>, ) -> (String, Vec) { let mut state = new_state(); - state.session_id = initial_sid.to_string(); + state.session_id = initial_sid.0.to_string(); let mut events = vec![]; dispatch_event(&json, &mut state, &mut |e| events.push(e)); (state.session_id, events) @@ -1131,7 +829,7 @@ mod tests { fn dispatch_event_result_with_empty_session_id() { let (sid, events) = run_dispatch_with_sid( serde_json::json!({ "type": "result", "result": "text here" }), - "prev-session", + InitialSessionId("prev-session"), ); assert_eq!(sid, "prev-session"); assert!( @@ -1348,7 +1046,12 @@ mod tests { fn build_claude_command_keeps_streaming_process_contract() { let bin = PathBuf::from("claude"); let args = vec!["-p".to_string(), "hello".to_string()]; - let command = build_claude_command(&bin, &args, Some("/tmp/vault")).unwrap(); + let command = build_claude_command(ClaudeCommandRequest { + bin: &bin, + args: &args, + cwd: Some("/tmp/vault"), + }) + .unwrap(); let actual_args: Vec = command.get_args().map(OsStr::to_os_string).collect(); let claude_code_env = command .get_envs() @@ -1395,7 +1098,12 @@ mod tests { "-p".to_string(), "Rename the note after reading the active vault".to_string(), ]; - let command = build_claude_command(&shim, &args, Some("/tmp/vault")).unwrap(); + let command = build_claude_command(ClaudeCommandRequest { + bin: &shim, + args: &args, + cwd: Some("/tmp/vault"), + }) + .unwrap(); let actual_args = command.get_args().collect::>(); assert_ne!( @@ -1412,26 +1120,50 @@ mod tests { } #[cfg(unix)] - fn run_mock_script(script: &str) -> (Result, Vec) { - run_mock_script_with_args(script, &[]) + #[derive(Clone, Copy)] + struct MockClaudeScript<'a>(&'a str); + + #[cfg(unix)] + #[derive(Clone, Copy)] + struct MockClaudeArgs<'a>(&'a [String]); + + #[cfg(unix)] + #[derive(Clone, Copy)] + struct MockClaudeStdin<'a>(Option<&'a str>); + + #[cfg(unix)] + fn run_mock_script( + script: MockClaudeScript<'_>, + ) -> (Result, Vec) { + run_mock_script_with_args(script, MockClaudeArgs(&[])) } #[cfg(unix)] fn run_mock_script_with_args( - script: &str, - args: &[String], + script: MockClaudeScript<'_>, + args: MockClaudeArgs<'_>, + ) -> (Result, Vec) { + run_mock_script_with_args_and_stdin(script, args, MockClaudeStdin(None)) + } + + #[cfg(unix)] + fn run_mock_script_with_args_and_stdin( + script: MockClaudeScript<'_>, + args: MockClaudeArgs<'_>, + stdin_text: MockClaudeStdin<'_>, ) -> (Result, Vec) { use std::os::unix::fs::PermissionsExt; let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("mock-claude"); - std::fs::write(&path, script).unwrap(); + std::fs::write(&path, script.0).unwrap(); std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).unwrap(); let mut events = vec![]; let result = run_claude_subprocess( ClaudeSubprocessRequest { bin: &path, - args, + args: args.0, fallback_args: &[], + stdin_text: stdin_text.0, cwd: None, }, &mut |e| events.push(e), @@ -1442,12 +1174,12 @@ mod tests { #[cfg(unix)] #[test] fn run_subprocess_parses_ndjson_stream() { - let (result, events) = run_mock_script(concat!( + let (result, events) = run_mock_script(MockClaudeScript(concat!( "#!/bin/sh\n", "echo '{\"type\":\"system\",\"subtype\":\"init\",\"session_id\":\"s1\"}'\n", "echo '{\"type\":\"stream_event\",\"event\":{\"type\":\"content_block_delta\",\"index\":0,\"delta\":{\"type\":\"text_delta\",\"text\":\"Hi\"}}}'\n", "echo '{\"type\":\"result\",\"result\":\"Done\",\"session_id\":\"s1\"}'\n", - )); + ))); assert_eq!(result.unwrap(), "s1"); assert!(matches!(&events[0], ClaudeStreamEvent::Init { session_id } if session_id == "s1")); assert!(matches!(&events[1], ClaudeStreamEvent::TextDelta { text } if text == "Hi")); @@ -1487,6 +1219,7 @@ mod tests { bin: &path, args: &args, fallback_args: &fallback_args, + stdin_text: None, cwd: None, }, &mut |event| events.push(event), @@ -1567,6 +1300,7 @@ mod tests { bin: &fake_bin, args: &args, fallback_args: &[], + stdin_text: None, cwd: None, }, &mut |event| events.push(event), @@ -1607,12 +1341,12 @@ mod tests { #[cfg(unix)] #[test] fn run_subprocess_skips_blank_and_non_json_lines() { - let (result, events) = run_mock_script(concat!( + let (result, events) = run_mock_script(MockClaudeScript(concat!( "#!/bin/sh\n", "echo ''\n", "echo 'not json at all'\n", "echo '{\"type\":\"result\",\"result\":\"ok\",\"session_id\":\"s2\"}'\n", - )); + ))); assert_eq!(result.unwrap(), "s2"); assert!(matches!(&events[0], ClaudeStreamEvent::Result { text, .. } if text == "ok")); assert!(matches!(&events[1], ClaudeStreamEvent::Done)); @@ -1621,7 +1355,9 @@ mod tests { #[cfg(unix)] #[test] fn run_subprocess_emits_error_on_nonzero_exit() { - let (_, events) = run_mock_script("#!/bin/sh\necho 'auth problem' >&2\nexit 1\n"); + let (_, events) = run_mock_script(MockClaudeScript( + "#!/bin/sh\necho 'auth problem' >&2\nexit 1\n", + )); assert!(events .iter() .any(|e| matches!(e, ClaudeStreamEvent::Error { .. }))); @@ -1631,14 +1367,16 @@ mod tests { #[cfg(unix)] #[test] fn run_subprocess_detects_auth_error_in_stderr() { - let (_, events) = run_mock_script("#!/bin/sh\necho 'not logged in' >&2\nexit 1\n"); + let (_, events) = run_mock_script(MockClaudeScript( + "#!/bin/sh\necho 'not logged in' >&2\nexit 1\n", + )); assert!(events.iter().any(|e| matches!(e, ClaudeStreamEvent::Error { message } if message.contains("not authenticated")))); } #[cfg(unix)] #[test] fn run_subprocess_reports_exit_code_on_empty_stderr() { - let (_, events) = run_mock_script("#!/bin/sh\nexit 2\n"); + let (_, events) = run_mock_script(MockClaudeScript("#!/bin/sh\nexit 2\n")); assert!(events.iter().any( |e| matches!(e, ClaudeStreamEvent::Error { message } if message.contains("exited with")) )); @@ -1647,7 +1385,7 @@ mod tests { #[cfg(unix)] #[test] fn run_subprocess_success_with_no_events() { - let (result, events) = run_mock_script("#!/bin/sh\nexit 0\n"); + let (result, events) = run_mock_script(MockClaudeScript("#!/bin/sh\nexit 0\n")); assert!(result.is_ok()); assert!(matches!(events.last().unwrap(), ClaudeStreamEvent::Done)); } @@ -1656,10 +1394,13 @@ mod tests { #[test] fn run_subprocess_passes_args_through() { let args: Vec = vec!["--foo".into(), "bar".into()]; - let (_, events) = run_mock_script_with_args(concat!( - "#!/bin/sh\n", - "echo \"{\\\"type\\\":\\\"result\\\",\\\"result\\\":\\\"$*\\\",\\\"session_id\\\":\\\"sx\\\"}\"\n", - ), &args); + let (_, events) = run_mock_script_with_args( + MockClaudeScript(concat!( + "#!/bin/sh\n", + "echo \"{\\\"type\\\":\\\"result\\\",\\\"result\\\":\\\"$*\\\",\\\"session_id\\\":\\\"sx\\\"}\"\n", + )), + MockClaudeArgs(&args), + ); let text = events.iter().find_map(|e| match e { ClaudeStreamEvent::Result { text, .. } => Some(text.as_str()), _ => None, @@ -1667,82 +1408,28 @@ mod tests { assert!(text.unwrap().contains("--foo")); } - // --- build_chat_args --- - + #[cfg(unix)] #[test] - fn build_chat_args_basic() { - let args = build_chat_args(&chat_request!("hello", None, None)); + fn run_subprocess_writes_prompt_to_stdin() { + let args: Vec = vec!["-p".into()]; + let (result, events) = run_mock_script_with_args_and_stdin( + MockClaudeScript(concat!( + "#!/bin/sh\n", + "stdin=$(cat)\n", + "if [ \"$stdin\" != \"hello from stdin\" ]; then\n", + " echo \"unexpected stdin: $stdin\" >&2\n", + " exit 3\n", + "fi\n", + "echo '{\"type\":\"result\",\"result\":\"stdin ok\",\"session_id\":\"sx\"}'\n", + )), + MockClaudeArgs(&args), + MockClaudeStdin(Some("hello from stdin")), + ); - assert_args_contain!(args, ["-p", "hello", "stream-json"]); - assert_args_lack!(args, ["--system-prompt", "--resume"]); - } - - #[test] - fn build_chat_args_with_system_prompt() { - let args = build_chat_args(&chat_request!("hi", Some("You are helpful."), None)); - - assert_args_contain!(args, ["--system-prompt", "You are helpful."]); - } - - #[test] - fn build_chat_args_empty_system_prompt_is_skipped() { - let args = build_chat_args(&chat_request!("hi", Some(""), None)); - - assert!(!args.contains(&"--system-prompt".to_string())); - } - - #[test] - fn build_chat_args_with_session_id() { - let args = build_chat_args(&chat_request!("continue", None, Some("sess-abc"))); - - assert_args_contain!(args, ["--resume", "sess-abc"]); - } - - // --- build_agent_args --- - - #[test] - fn build_agent_args_basic() { - // build_agent_args calls build_mcp_config which needs mcp_server_dir - if let Ok(args) = build_agent_args(&agent_request!( - "create note", - None, - AiAgentPermissionMode::Safe, - )) { - assert_args_contain!(args, ["-p", "create note", "--mcp-config"]); - assert_args_contain!(args, ["--strict-mcp-config", "--permission-mode"]); - assert_args_contain!(args, ["acceptEdits", "--tools"]); - assert_args_contain!(args, ["Read,Edit,MultiEdit,Write,Glob,Grep,LS"]); - assert_args_contain!(args, ["--no-session-persistence"]); - assert_args_lack!( - args, - [ - "--dangerously-skip-permissions", - "bypassPermissions", - "--append-system-prompt", - ], - ); - assert_no_arg_contains!(args, "Bash"); - } - } - - #[test] - fn build_agent_args_with_system_prompt() { - if let Ok(args) = build_agent_args(&agent_request!( - "do it", - Some("Act as expert."), - AiAgentPermissionMode::Safe, - )) { - assert_args_contain!(args, ["--append-system-prompt", "Act as expert."]); - } - } - - #[test] - fn build_agent_args_empty_system_prompt_is_skipped() { - if let Ok(args) = - build_agent_args(&agent_request!("x", Some(""), AiAgentPermissionMode::Safe)) - { - assert!(!args.contains(&"--append-system-prompt".to_string())); - } + assert_eq!(result.unwrap(), "sx"); + assert!(events.iter().any( + |event| matches!(event, ClaudeStreamEvent::Result { text, .. } if text == "stdin ok") + )); } // --- find_claude_binary --- @@ -1862,6 +1549,7 @@ mod tests { bin: &fake_bin, args: &[], fallback_args: &[], + stdin_text: None, cwd: None, }, &mut |e| events.push(e), @@ -1873,13 +1561,13 @@ mod tests { #[cfg(unix)] #[test] fn run_subprocess_with_tool_progress_and_assistant() { - let (result, events) = run_mock_script(concat!( + let (result, events) = run_mock_script(MockClaudeScript(concat!( "#!/bin/sh\n", "echo '{\"type\":\"system\",\"subtype\":\"init\",\"session_id\":\"s3\"}'\n", "echo '{\"type\":\"tool_progress\",\"tool_name\":\"search\",\"tool_use_id\":\"t1\"}'\n", "echo '{\"type\":\"assistant\",\"message\":{\"content\":[{\"type\":\"tool_use\",\"id\":\"t2\",\"name\":\"read\",\"input\":{}}]}}'\n", "echo '{\"type\":\"result\",\"result\":\"fin\",\"session_id\":\"s3\"}'\n", - )); + ))); assert_eq!(result.unwrap(), "s3"); assert!(events.len() >= 4); } @@ -1887,12 +1575,12 @@ mod tests { #[cfg(unix)] #[test] fn run_subprocess_success_exit_with_session_id_skips_error() { - let (_, events) = run_mock_script(concat!( + let (_, events) = run_mock_script(MockClaudeScript(concat!( "#!/bin/sh\n", "echo '{\"type\":\"system\",\"subtype\":\"init\",\"session_id\":\"s4\"}'\n", "echo 'some warning' >&2\n", "exit 1\n", - )); + ))); // Should NOT have an error event because session_id is non-empty assert!(!events .iter() diff --git a/src-tauri/src/claude_invocation.rs b/src-tauri/src/claude_invocation.rs new file mode 100644 index 00000000..7a323e25 --- /dev/null +++ b/src-tauri/src/claude_invocation.rs @@ -0,0 +1,606 @@ +use crate::ai_agents::AiAgentPermissionMode; +use crate::claude_cli::ChatStreamRequest; +use crate::cli_agent_runtime::AgentStreamRequest; + +const CLAUDE_SAFE_AGENT_TOOLS: &str = "Read,Edit,MultiEdit,Write,Glob,Grep,LS"; +const CLAUDE_POWER_USER_AGENT_TOOLS: &str = "Read,Edit,MultiEdit,Write,Glob,Grep,LS,Bash"; +const CLAUDE_CHAT_DISALLOWED_TOOLS_COMPAT: &str = + "Bash,Glob,Grep,Read,Edit,Write,NotebookEdit,WebFetch,WebSearch,TodoWrite,Task,MultiEdit,LS"; +const CLAUDE_SAFE_DISALLOWED_TOOLS_COMPAT: &str = + "Bash,NotebookEdit,WebFetch,WebSearch,TodoWrite,Task"; +const CLAUDE_POWER_USER_DISALLOWED_TOOLS_COMPAT: &str = + "NotebookEdit,WebFetch,WebSearch,TodoWrite,Task"; +const WINDOWS_COMMAND_LINE_STDIN_THRESHOLD: usize = 24 * 1024; + +#[derive(Debug)] +pub(crate) struct ClaudeInvocation { + pub(crate) args: Vec, + pub(crate) fallback_args: Vec>, + pub(crate) stdin_text: Option, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum PromptSource { + Argument, + Stdin, +} + +pub(crate) fn chat(req: &ChatStreamRequest) -> ClaudeInvocation { + chat_with_windows_limit(req, cfg!(windows)) +} + +pub(crate) fn agent(req: &AgentStreamRequest) -> Result { + agent_with_windows_limit(req, cfg!(windows)) +} + +fn chat_with_windows_limit( + req: &ChatStreamRequest, + enforce_windows_limit: bool, +) -> ClaudeInvocation { + let args = chat_args(req); + let fallback_args = vec![chat_args_compat(req)]; + if should_pipe_prompt_for_windows(enforce_windows_limit, &args, &fallback_args) { + return ClaudeInvocation { + args: chat_args_with_tool_policy(req, "--tools", String::new(), PromptSource::Stdin), + fallback_args: vec![chat_args_with_tool_policy( + req, + "--disallowedTools", + CLAUDE_CHAT_DISALLOWED_TOOLS_COMPAT.into(), + PromptSource::Stdin, + )], + stdin_text: Some(stdin_prompt(&req.message, req.system_prompt.as_deref())), + }; + } + + ClaudeInvocation { + args, + fallback_args, + stdin_text: None, + } +} + +fn agent_with_windows_limit( + req: &AgentStreamRequest, + enforce_windows_limit: bool, +) -> Result { + let args = agent_args(req)?; + let fallback_args = vec![ + agent_args_without_session_persistence(req)?, + agent_args_compat(req)?, + ]; + if should_pipe_prompt_for_windows(enforce_windows_limit, &args, &fallback_args) { + return Ok(ClaudeInvocation { + args: agent_args_with_tool_policy( + req, + AgentToolPolicy::strict(req.permission_mode), + PromptSource::Stdin, + )?, + fallback_args: vec![ + agent_args_with_tool_policy( + req, + AgentToolPolicy::strict_without_session_persistence(req.permission_mode), + PromptSource::Stdin, + )?, + agent_args_with_tool_policy( + req, + AgentToolPolicy::compat(req.permission_mode), + PromptSource::Stdin, + )?, + ], + stdin_text: Some(stdin_prompt(&req.message, req.system_prompt.as_deref())), + }); + } + + Ok(ClaudeInvocation { + args, + fallback_args, + stdin_text: None, + }) +} + +fn chat_args(req: &ChatStreamRequest) -> Vec { + chat_args_with_tool_policy(req, "--tools", String::new(), PromptSource::Argument) +} + +fn chat_args_compat(req: &ChatStreamRequest) -> Vec { + chat_args_with_tool_policy( + req, + "--disallowedTools", + CLAUDE_CHAT_DISALLOWED_TOOLS_COMPAT.into(), + PromptSource::Argument, + ) +} + +fn chat_args_with_tool_policy( + req: &ChatStreamRequest, + tool_flag: &str, + tool_value: String, + prompt_source: PromptSource, +) -> Vec { + let mut args: Vec = vec!["-p".into()]; + if prompt_source == PromptSource::Argument { + args.push(req.message.clone()); + } + args.extend([ + "--output-format".into(), + "stream-json".into(), + "--verbose".into(), + "--include-partial-messages".into(), + tool_flag.into(), + tool_value, + ]); + + if prompt_source == PromptSource::Argument { + append_non_empty_arg_pair(&mut args, "--system-prompt", req.system_prompt.as_deref()); + } + if let Some(ref session_id) = req.session_id { + args.push("--resume".into()); + args.push(session_id.clone()); + } + + args +} + +fn agent_args(req: &AgentStreamRequest) -> Result, String> { + agent_args_with_tool_policy( + req, + AgentToolPolicy::strict(req.permission_mode), + PromptSource::Argument, + ) +} + +fn agent_args_without_session_persistence(req: &AgentStreamRequest) -> Result, String> { + agent_args_with_tool_policy( + req, + AgentToolPolicy::strict_without_session_persistence(req.permission_mode), + PromptSource::Argument, + ) +} + +fn agent_args_compat(req: &AgentStreamRequest) -> Result, String> { + agent_args_with_tool_policy( + req, + AgentToolPolicy::compat(req.permission_mode), + PromptSource::Argument, + ) +} + +fn agent_args_with_tool_policy( + req: &AgentStreamRequest, + policy: AgentToolPolicy, + prompt_source: PromptSource, +) -> Result, String> { + let mut args: Vec = vec!["-p".into()]; + if prompt_source == PromptSource::Argument { + args.push(req.message.clone()); + } + args.extend([ + "--output-format".into(), + "stream-json".into(), + "--verbose".into(), + "--include-partial-messages".into(), + "--mcp-config".into(), + mcp_config(&req.vault_path)?, + "--strict-mcp-config".into(), + "--permission-mode".into(), + "acceptEdits".into(), + policy.tool_flag.into(), + policy.tool_value.into(), + ]); + + if policy.include_session_persistence_flag { + args.push("--no-session-persistence".into()); + } + if let Some(allowed_tools) = policy.preapproved_tools { + args.push("--allowedTools".into()); + args.push(allowed_tools.into()); + } + if let Some(disallowed_tools) = policy.disallowed_tools { + args.push("--disallowedTools".into()); + args.push(disallowed_tools.into()); + } + if prompt_source == PromptSource::Argument { + append_non_empty_arg_pair( + &mut args, + "--append-system-prompt", + req.system_prompt.as_deref(), + ); + } + + Ok(args) +} + +fn append_non_empty_arg_pair(args: &mut Vec, flag: &str, value: Option<&str>) { + if let Some(value) = value.map(str::trim).filter(|value| !value.is_empty()) { + args.push(flag.into()); + args.push(value.into()); + } +} + +fn stdin_prompt(message: &str, system_prompt: Option<&str>) -> String { + crate::cli_agent_runtime::build_prompt(message, system_prompt) +} + +fn mcp_config(vault_path: &str) -> Result { + let mcp_server_path = crate::cli_agent_runtime::mcp_server_path_string()?; + let config = serde_json::json!({ + "mcpServers": { + "tolaria": { + "command": "node", + "args": [mcp_server_path], + "env": { "VAULT_PATH": vault_path } + } + } + }); + serde_json::to_string(&config).map_err(|e| format!("Failed to serialise MCP config: {e}")) +} + +fn should_pipe_prompt_for_windows( + enforce_windows_limit: bool, + args: &[String], + fallback_args: &[Vec], +) -> bool { + enforce_windows_limit + && std::iter::once(args) + .chain(fallback_args.iter().map(Vec::as_slice)) + .any(args_exceed_windows_stdin_threshold) +} + +fn args_exceed_windows_stdin_threshold(args: &[String]) -> bool { + windows_command_line_utf16_units(args) >= WINDOWS_COMMAND_LINE_STDIN_THRESHOLD +} + +fn windows_command_line_utf16_units(args: &[String]) -> usize { + args.iter().map(|arg| arg.encode_utf16().count() + 3).sum() +} + +struct AgentToolPolicy { + tool_flag: &'static str, + tool_value: &'static str, + include_session_persistence_flag: bool, + preapproved_tools: Option<&'static str>, + disallowed_tools: Option<&'static str>, +} + +impl AgentToolPolicy { + fn strict(permission_mode: AiAgentPermissionMode) -> Self { + Self { + tool_flag: "--tools", + tool_value: agent_tools(permission_mode), + include_session_persistence_flag: true, + preapproved_tools: preapproved_agent_tools(permission_mode), + disallowed_tools: None, + } + } + + fn strict_without_session_persistence(permission_mode: AiAgentPermissionMode) -> Self { + Self { + include_session_persistence_flag: false, + ..Self::strict(permission_mode) + } + } + + fn compat(permission_mode: AiAgentPermissionMode) -> Self { + Self { + tool_flag: "--allowedTools", + tool_value: agent_tools(permission_mode), + include_session_persistence_flag: false, + preapproved_tools: None, + disallowed_tools: Some(disallowed_agent_tools_compat(permission_mode)), + } + } +} + +fn agent_tools(permission_mode: AiAgentPermissionMode) -> &'static str { + match permission_mode { + AiAgentPermissionMode::Safe => CLAUDE_SAFE_AGENT_TOOLS, + AiAgentPermissionMode::PowerUser => CLAUDE_POWER_USER_AGENT_TOOLS, + } +} + +fn preapproved_agent_tools(permission_mode: AiAgentPermissionMode) -> Option<&'static str> { + match permission_mode { + AiAgentPermissionMode::Safe => None, + AiAgentPermissionMode::PowerUser => Some("Bash"), + } +} + +fn disallowed_agent_tools_compat(permission_mode: AiAgentPermissionMode) -> &'static str { + match permission_mode { + AiAgentPermissionMode::Safe => CLAUDE_SAFE_DISALLOWED_TOOLS_COMPAT, + AiAgentPermissionMode::PowerUser => CLAUDE_POWER_USER_DISALLOWED_TOOLS_COMPAT, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + macro_rules! assert_args_contain { + ($args:expr, [$($value:expr),+ $(,)?] $(,)?) => { + $( + assert!($args.contains(&$value.to_string()), "missing {}", $value); + )+ + }; + } + + macro_rules! assert_args_lack { + ($args:expr, [$($value:expr),+ $(,)?] $(,)?) => { + $( + assert!(!$args.iter().any(|arg| arg == &$value.to_string()), "unexpected {}", $value); + )+ + }; + } + + macro_rules! assert_no_arg_contains { + ($args:expr, $fragment:expr $(,)?) => { + assert!(!$args.iter().any(|arg| arg.contains($fragment))); + }; + } + + macro_rules! chat_request { + ($message:expr, None, None $(,)?) => { + ChatStreamRequest { + message: $message.into(), + system_prompt: None, + session_id: None, + } + }; + ($message:expr, Some($system_prompt:expr), None $(,)?) => { + ChatStreamRequest { + message: $message.into(), + system_prompt: Some($system_prompt.to_string()), + session_id: None, + } + }; + ($message:expr, None, Some($session_id:expr) $(,)?) => { + ChatStreamRequest { + message: $message.into(), + system_prompt: None, + session_id: Some($session_id.to_string()), + } + }; + ($message:expr, Some($system_prompt:expr), Some($session_id:expr) $(,)?) => { + ChatStreamRequest { + message: $message.into(), + system_prompt: Some($system_prompt.to_string()), + session_id: Some($session_id.to_string()), + } + }; + } + + fn agent_request( + message: &str, + system_prompt: Option<&str>, + permission_mode: AiAgentPermissionMode, + ) -> AgentStreamRequest { + AgentStreamRequest { + message: message.into(), + system_prompt: system_prompt.map(str::to_string), + vault_path: "/tmp/vault".into(), + permission_mode, + } + } + + fn arg_value_after<'a>(args: &'a [String], name: &str) -> Option<&'a str> { + let index = args.iter().position(|arg| arg == name)?; + args.get(index + 1).map(String::as_str) + } + + #[test] + fn chat_args_basic() { + let args = chat_args(&chat_request!("hello", None, None)); + + assert_args_contain!(args, ["-p", "hello", "stream-json"]); + assert_args_lack!(args, ["--system-prompt", "--resume"]); + } + + #[test] + fn chat_args_with_system_prompt() { + let args = chat_args(&chat_request!("hi", Some("You are helpful."), None)); + + assert_args_contain!(args, ["--system-prompt", "You are helpful."]); + } + + #[test] + fn chat_args_empty_system_prompt_is_skipped() { + let args = chat_args(&chat_request!("hi", Some(""), None)); + + assert!(!args.contains(&"--system-prompt".to_string())); + } + + #[test] + fn chat_args_with_session_id() { + let args = chat_args(&chat_request!("continue", None, Some("sess-abc"))); + + assert_args_contain!(args, ["--resume", "sess-abc"]); + } + + #[test] + fn chat_args_compat_disallows_builtin_tools_when_tools_flag_is_unavailable() { + let args = chat_args_compat(&chat_request!("hello", None, None)); + + assert_args_contain!(args, ["--disallowedTools"]); + assert_args_lack!(args, ["--tools"]); + assert!(arg_value_after(&args, "--disallowedTools") + .is_some_and(|tools| tools.contains("NotebookEdit"))); + } + + #[test] + fn oversized_windows_invocations_pipe_prompt_to_stdin() { + let long_message = "x".repeat(WINDOWS_COMMAND_LINE_STDIN_THRESHOLD); + let chat_req = chat_request!(&long_message, Some("Use context."), Some("sess-abc")); + let chat_invocation = chat_with_windows_limit(&chat_req, true); + + assert_eq!(chat_invocation.args.first().map(String::as_str), Some("-p")); + assert_args_lack!( + chat_invocation.args, + [long_message.as_str(), "--system-prompt", "Use context."] + ); + assert_args_contain!(chat_invocation.args, ["--resume", "sess-abc"]); + assert!(chat_invocation + .fallback_args + .iter() + .flatten() + .all(|arg| arg != &long_message)); + let chat_stdin = chat_invocation.stdin_text.as_deref().unwrap(); + assert!(chat_stdin.contains("System instructions:\nUse context.")); + assert!(chat_stdin.contains("User request:\n")); + assert!(chat_stdin.contains(&long_message)); + + let agent_req = agent_request( + &long_message, + Some("Read the active vault first."), + AiAgentPermissionMode::Safe, + ); + let agent_invocation = agent_with_windows_limit(&agent_req, true).unwrap(); + + assert_eq!( + agent_invocation.args.first().map(String::as_str), + Some("-p") + ); + assert_args_lack!( + agent_invocation.args, + [ + long_message.as_str(), + "--append-system-prompt", + "Read the active vault first." + ] + ); + assert_args_contain!( + agent_invocation.args, + ["--mcp-config", "--strict-mcp-config"] + ); + assert!(agent_invocation + .fallback_args + .iter() + .flatten() + .all(|arg| arg != &long_message)); + let agent_stdin = agent_invocation.stdin_text.as_deref().unwrap(); + assert!(agent_stdin.contains("System instructions:\nRead the active vault first.")); + assert!(agent_stdin.contains("User request:\n")); + assert!(agent_stdin.contains(&long_message)); + } + + #[test] + fn agent_args_with_system_prompt() { + if let Ok(args) = agent_args(&agent_request( + "do it", + Some("Act as expert."), + AiAgentPermissionMode::Safe, + )) { + assert_args_contain!(args, ["--append-system-prompt", "Act as expert."]); + } + } + + #[test] + fn agent_args_empty_system_prompt_is_skipped() { + if let Ok(args) = agent_args(&agent_request("x", Some(""), AiAgentPermissionMode::Safe)) { + assert!(!args.contains(&"--append-system-prompt".to_string())); + } + } + + #[test] + fn agent_args_without_session_persistence_keeps_tools_allowlist() { + let args = agent_args_without_session_persistence(&agent_request( + "Rename the note", + None, + AiAgentPermissionMode::Safe, + )) + .unwrap(); + + assert_args_contain!(args, ["--tools", "Read,Edit,MultiEdit,Write,Glob,Grep,LS"]); + assert_args_lack!(args, ["--no-session-persistence"]); + } + + #[test] + fn agent_args_compat_uses_allowed_and_disallowed_tools_without_removed_flags() { + let args = agent_args_compat(&agent_request( + "Rename the note", + None, + AiAgentPermissionMode::Safe, + )) + .unwrap(); + + assert_eq!( + arg_value_after(&args, "--allowedTools"), + Some("Read,Edit,MultiEdit,Write,Glob,Grep,LS") + ); + assert_args_contain!(args, ["--disallowedTools"]); + assert_args_lack!(args, ["--tools"]); + assert_args_lack!(args, ["--no-session-persistence"]); + } + + #[test] + fn agent_args_use_safe_mode_without_bash_by_default() { + let args = agent_args(&agent_request( + "Rename the note", + None, + AiAgentPermissionMode::Safe, + )) + .unwrap(); + + assert_args_contain!( + args, + ["--strict-mcp-config", "--permission-mode", "acceptEdits"] + ); + assert_args_contain!(args, ["Read,Edit,MultiEdit,Write,Glob,Grep,LS"]); + assert_no_arg_contains!(args, "Bash"); + assert_args_lack!(args, ["--allowedTools"]); + assert_args_lack!(args, ["--dangerously-skip-permissions"]); + } + + #[test] + fn agent_args_allow_bash_in_power_user_mode_without_dangerous_bypass() { + let args = agent_args(&agent_request( + "Rename the note", + None, + AiAgentPermissionMode::PowerUser, + )) + .unwrap(); + + assert_args_contain!(args, ["--strict-mcp-config"]); + assert_args_contain!(args, ["Read,Edit,MultiEdit,Write,Glob,Grep,LS,Bash"]); + assert_args_lack!(args, ["--dangerously-skip-permissions"]); + } + + #[test] + fn agent_args_preapprove_bash_for_power_user_runs() { + let args = agent_args(&agent_request( + "Run a local script", + None, + AiAgentPermissionMode::PowerUser, + )) + .unwrap(); + + assert_eq!(arg_value_after(&args, "--allowedTools"), Some("Bash")); + } + + #[test] + fn agent_invocation_keeps_short_windows_prompt_on_args() { + let req = agent_request( + "summarize the inbox", + Some("Read the active vault first."), + AiAgentPermissionMode::Safe, + ); + let invocation = agent_with_windows_limit(&req, true).unwrap(); + + assert_args_contain!( + invocation.args, + ["-p", "summarize the inbox", "--append-system-prompt"] + ); + assert!(invocation.stdin_text.is_none()); + } + + #[test] + fn mcp_config_is_valid_json() { + if let Ok(config_str) = mcp_config("/tmp/test-vault") { + let parsed: serde_json::Value = serde_json::from_str(&config_str).unwrap(); + assert!(parsed["mcpServers"]["tolaria"]["command"].is_string()); + assert_eq!( + parsed["mcpServers"]["tolaria"]["env"]["VAULT_PATH"], + "/tmp/test-vault" + ); + } + } +} diff --git a/src-tauri/src/cli_agent_runtime.rs b/src-tauri/src/cli_agent_runtime.rs index 6c247719..4ae63ac4 100644 --- a/src-tauri/src/cli_agent_runtime.rs +++ b/src-tauri/src/cli_agent_runtime.rs @@ -1,7 +1,7 @@ use crate::ai_agents::{AiAgentPermissionMode, AiAgentStreamEvent}; use serde::Deserialize; use std::ffi::OsString; -use std::io::BufRead; +use std::io::{BufRead, Write}; use std::path::{Path, PathBuf}; use std::process::{Command, ExitStatus}; @@ -24,6 +24,27 @@ pub(crate) struct AgentCommandTarget { pub first_arg: Option, } +pub(crate) struct JsonLineProcess<'a> { + command: Command, + process_name: &'static str, + stdin_input: Option<&'a str>, +} + +impl<'a> JsonLineProcess<'a> { + pub(crate) fn new(command: Command, process_name: &'static str) -> Self { + Self { + command, + process_name, + stdin_input: None, + } + } + + pub(crate) fn with_stdin(mut self, stdin_input: Option<&'a str>) -> Self { + self.stdin_input = stdin_input; + self + } +} + pub(crate) fn build_prompt(message: &str, system_prompt: Option<&str>) -> String { match system_prompt .map(str::trim) @@ -266,19 +287,44 @@ where } pub(crate) fn run_json_line_process( - mut command: Command, + command: Command, process_name: &'static str, emit: &mut F, error_event: impl Fn(String) -> Event, + handle_json: H, +) -> Result +where + F: FnMut(Event), + H: FnMut(&serde_json::Value, &mut F, &mut String), +{ + run_json_line_process_with_stdin( + JsonLineProcess::new(command, process_name), + emit, + error_event, + handle_json, + ) +} + +pub(crate) fn run_json_line_process_with_stdin( + mut process: JsonLineProcess<'_>, + emit: &mut F, + error_event: impl Fn(String) -> Event, mut handle_json: H, ) -> Result where F: FnMut(Event), H: FnMut(&serde_json::Value, &mut F, &mut String), { - let mut child = command + if process.stdin_input.is_some() { + process.command.stdin(std::process::Stdio::piped()); + } + + let mut child = process + .command .spawn() - .map_err(|error| format_spawn_error(process_name, &error))?; + .map_err(|error| format_spawn_error(process.process_name, &error))?; + let stdin_write_error = + write_stdin_input(&mut child, process.process_name, process.stdin_input); let stdout = child.stdout.take().ok_or("No stdout handle")?; let reader = std::io::BufReader::new(stdout); let mut session_id = String::new(); @@ -302,6 +348,7 @@ where let status = child .wait() .map_err(|error| format!("Wait failed: {error}"))?; + let stderr_output = with_stdin_write_error(stderr_output, stdin_write_error); Ok(JsonLineRun { session_id, @@ -310,6 +357,36 @@ where }) } +fn write_stdin_input( + child: &mut std::process::Child, + process_name: &str, + stdin_input: Option<&str>, +) -> Option { + let input = stdin_input?; + let Some(mut stdin) = child.stdin.take() else { + return Some(format!( + "Failed to write {process_name} stdin: no stdin handle" + )); + }; + + stdin + .write_all(input.as_bytes()) + .err() + .map(|error| format!("Failed to write {process_name} stdin: {error}")) +} + +fn with_stdin_write_error(mut stderr_output: String, stdin_write_error: Option) -> String { + let Some(error) = stdin_write_error else { + return stderr_output; + }; + + if !stderr_output.is_empty() { + stderr_output.push('\n'); + } + stderr_output.push_str(&error); + stderr_output +} + fn format_spawn_error(process_name: &str, error: &std::io::Error) -> String { if error.kind() == std::io::ErrorKind::NotFound { return format!( diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index ff2063d8..6da7c65d 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -2,6 +2,7 @@ pub mod ai_agents; pub mod ai_models; pub mod app_updater; pub mod claude_cli; +mod claude_invocation; mod cli_agent_runtime; pub mod codex_cli; mod commands;