diff --git a/PLAN.md b/PLAN.md index c7b2b43..a68d856 100644 --- a/PLAN.md +++ b/PLAN.md @@ -27,6 +27,13 @@ execution targets one self-contained Mac. starting tool set, unlimited tool rounds, queued user guidance between tool rounds, session date/time context, periodic tool-contract reminders, cooperative Stop, and explicit activity/failure states are implemented. +- Local tools are hardened for daily use: canonical project boundaries reject + parent and symlink escapes, shell commands receive a deliberate environment, + risky shell and visible-browser actions share one cancellable Allow once/Deny + approval path, and compact tool cards expose bounded parameters, results, and + parsing/approval/queue/run/completion lifecycle state without showing DSML. + Background jobs and bounded output files stop and clean up with Stop, session + switches, and application shutdown. - Context compaction uses the reference soft and exact token-counted hard triggers, private live-model summaries, bounded summary and tool-result retries, a recent verbatim tail, running-job observations, and compatible KV @@ -38,21 +45,20 @@ execution targets one self-contained Mac. - Focused coverage exercises triggers, summary bounds and sanitizing, tail selection, queued guidance, checkpoint identity, running jobs, durable compaction markers, relaunch, and continued tool work after rebuild. -- The next baseline gap is tool hardening and safety. SSD streaming, - speculative decoding, steering, GLM 5.2 execution, and DeepSeek V4 Pro - execution are not implemented in the Rust executor. Related catalog, - validation, and preference plumbing must not be treated as runtime support. +- The next baseline gap is SSD streaming. Speculative decoding, steering, GLM + 5.2 execution, and DeepSeek V4 Pro execution are not implemented in the Rust + executor. Related catalog, validation, and preference plumbing must not be + treated as runtime support. ## Delivery order -1. **Next:** tool hardening, approvals, and productive tool presentation. -2. Remaining DS4 execution technology, starting with SSD streaming, then +1. **Next:** remaining DS4 execution technology, starting with SSD streaming, then speculative decoding and the other Metal/runtime parity work. -3. Additional model execution: GLM 5.2 and DeepSeek V4 Pro. -4. Product completion, exhaustive parity verification, and distribution. -5. Optional extensions: Dev Brain and A2UI. +2. Additional model execution: GLM 5.2 and DeepSeek V4 Pro. +3. Product completion, exhaustive parity verification, and distribution. +4. Optional extensions: Dev Brain and A2UI. -## 1. Next — tool hardening and safety +## 1. Completed — tool hardening and safety Goal: make the existing tool set safe and clear enough for productive daily use without weakening its ability to inspect, edit, build, and test a project. diff --git a/src/agent.rs b/src/agent.rs index 9d5dbd6..749a95b 100644 --- a/src/agent.rs +++ b/src/agent.rs @@ -1,19 +1,33 @@ use crate::model::ModelChoice; -use rfd::{MessageButtons, MessageDialog, MessageDialogResult, MessageLevel}; use serde_json::{Map, Value}; -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::ffi::{CStr, CString, c_char, c_int, c_void}; use std::fs::{self, File}; use std::os::unix::process::CommandExt; use std::path::{Path, PathBuf}; use std::process::{Child, Command, Stdio}; use std::sync::atomic::{AtomicBool, AtomicPtr, Ordering}; -use std::sync::mpsc::{self, Receiver, TryRecvError}; +use std::sync::mpsc::{self, Receiver, RecvTimeoutError, Sender, TryRecvError}; use std::sync::{Arc, Mutex}; use std::thread; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; const MAX_FILE_BYTES: u64 = 16 * 1024 * 1024; +const SHELL_ENV_ALLOWLIST: &[&str] = &[ + "PATH", + "HOME", + "USER", + "LOGNAME", + "SHELL", + "TMPDIR", + "LANG", + "LC_ALL", + "TERM", + "DEVELOPER_DIR", + "SDKROOT", + "MACOSX_DEPLOYMENT_TARGET", + "RUSTUP_TOOLCHAIN", +]; pub(crate) const COMPACTION_OBSERVATION_PREFIX: &str = "Bash job update after context compaction."; #[repr(C)] @@ -150,22 +164,12 @@ unsafe extern "C" fn web_confirm( error: *mut c_char, error_len: usize, ) -> c_int { - let message = if message.is_null() { - "The web tool wants to start a visible Chrome browser. Allow?".into() - } else { - unsafe { CStr::from_ptr(message) }.to_string_lossy() - }; - let allowed = MessageDialog::new() - .set_level(MessageLevel::Warning) - .set_title("Allow browser tools?") - .set_description(message) - .set_buttons(MessageButtons::YesNo) - .show() - == MessageDialogResult::Yes; - if !allowed { - write_c_error(error, error_len, "user denied Chrome browser start"); - } - c_int::from(allowed) + let _ = message; + let _ = error; + let _ = error_len; + // Approval is obtained by the shared tool worker before entering C. Keeping + // this callback lets the reference browser retain its launch guard. + 1 } unsafe extern "C" fn web_cancel(data: *mut c_void) -> bool { @@ -177,18 +181,6 @@ unsafe extern "C" fn web_cancel(data: *mut c_void) -> bool { !cancel.is_null() && unsafe { (*cancel).load(Ordering::Relaxed) } } -fn write_c_error(output: *mut c_char, length: usize, message: &str) { - if output.is_null() || length == 0 { - return; - } - let bytes = message.as_bytes(); - let count = bytes.len().min(length - 1); - unsafe { - std::ptr::copy_nonoverlapping(bytes.as_ptr().cast(), output, count); - *output.add(count) = 0; - } -} - const TOOL_SCHEMAS: &str = r#"{"type":"function","function":{"name":"google_search","description":"Search Google in a visible browser and return compact Markdown links.","parameters":{"type":"object","properties":{"query":{"type":"string"}},"required":["query"]}}} {"type":"function","function":{"name":"visit_page","description":"Open a URL in a visible browser and return rendered page text.","parameters":{"type":"object","properties":{"url":{"type":"string"}},"required":["url"]}}} {"type":"function","function":{"name":"bash","description":"Run a shell command.","parameters":{"type":"object","properties":{"command":{"type":"string"},"timeout_sec":{"type":"number"},"refresh_sec":{"type":"number"}},"required":["command"]}}} @@ -209,7 +201,87 @@ pub(crate) struct ToolCall { pub(crate) struct ActiveTools { pub(crate) results: Receiver, + pub(crate) events: Receiver, pub(crate) cancel: Arc, + worker: Option>, +} + +impl Drop for ActiveTools { + fn drop(&mut self) { + self.cancel.store(true, Ordering::Relaxed); + if let Some(worker) = self.worker.take() { + let _ = worker.join(); + } + } +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum ToolLifecycle { + Parsing, + AwaitingApproval, + Queued, + Running, + Completed, + Failed, + Stopped, +} + +impl ToolLifecycle { + pub(crate) fn label(self) -> &'static str { + match self { + Self::Parsing => "Parsing", + Self::AwaitingApproval => "Awaiting approval", + Self::Queued => "Queued", + Self::Running => "Running", + Self::Completed => "Completed", + Self::Failed => "Failed", + Self::Stopped => "Stopped", + } + } +} + +#[derive(Clone, Debug)] +pub(crate) struct ToolCard { + pub(crate) call: ToolCall, + pub(crate) state: ToolLifecycle, + pub(crate) result: Option, +} + +impl ToolCard { + pub(crate) fn parsing(call: ToolCall) -> Self { + Self { + call, + state: ToolLifecycle::Parsing, + result: None, + } + } + + pub(crate) fn streaming() -> Self { + Self::parsing(ToolCall { + name: "Tool call".into(), + arguments: Map::new(), + }) + } +} + +#[derive(Clone, Debug)] +pub(crate) struct ApprovalPrompt { + pub(crate) title: String, + pub(crate) detail: String, + pub(crate) working_directory: PathBuf, +} + +pub(crate) enum ToolEvent { + State { + index: usize, + state: ToolLifecycle, + result: Option, + }, + Approval { + index: usize, + prompt: ApprovalPrompt, + decision: Sender, + }, } struct BashJob { @@ -226,8 +298,8 @@ pub(crate) struct Tools { root: PathBuf, context_tokens: i32, more: Option<(PathBuf, usize, bool)>, + more_text: Option<(String, usize)>, jobs: HashMap, - temporary_files: HashSet, next_job: u32, browser: Browser, } @@ -240,8 +312,8 @@ impl Tools { .map_err(|error| format!("Could not open the project directory: {error}"))?, context_tokens, more: None, + more_text: None, jobs: HashMap::new(), - temporary_files: HashSet::new(), next_job: 1, browser: Browser::new()?, }) @@ -264,11 +336,15 @@ impl Tools { }; match result { Ok(result) if result.len() <= self.result_limit() => result, - Ok(result) => format!( - "Tool error: {} result is too large for this context ({} bytes). Retry with a smaller read/search/bash output.\n", - call.name, - result.len() - ), + Ok(result) => { + let length = result.len(); + self.more_text = Some((result, 0)); + let chunk = self.continue_text(self.default_lines()).unwrap_or_default(); + format!( + "{} result is too large for this context ({length} bytes); showing a bounded chunk. Use more to continue.\n{chunk}", + call.name + ) + } Err(error) => format!("Tool error: {error}\n"), } } @@ -304,6 +380,12 @@ impl Tools { } fn existing_path(&self, value: &str) -> Result { + if Path::new(value) + .components() + .any(|component| matches!(component, std::path::Component::ParentDir)) + { + return Err(format!("path contains a parent traversal: {value}")); + } let path = if Path::new(value).is_absolute() { PathBuf::from(value) } else { @@ -312,29 +394,43 @@ impl Tools { let path = path .canonicalize() .map_err(|error| format!("open {value}: {error}"))?; - if self.temporary_files.contains(&path) { - Ok(path) - } else { - self.inside_project(path, value) - } + self.inside_project(path, value) } fn writable_path(&self, value: &str) -> Result { + if Path::new(value) + .components() + .any(|component| matches!(component, std::path::Component::ParentDir)) + { + return Err(format!("path contains a parent traversal: {value}")); + } let path = if Path::new(value).is_absolute() { PathBuf::from(value) } else { self.root.join(value) }; - if path.exists() { + if fs::symlink_metadata(&path).is_ok() { return self.existing_path(value); } - let parent = path - .parent() - .ok_or_else(|| format!("invalid path: {value}"))? + let mut ancestor = path.as_path(); + let mut suffix = Vec::new(); + while fs::symlink_metadata(ancestor).is_err() { + let name = ancestor + .file_name() + .ok_or_else(|| format!("invalid path: {value}"))?; + suffix.push(name.to_owned()); + ancestor = ancestor + .parent() + .ok_or_else(|| format!("invalid path: {value}"))?; + } + let mut resolved = ancestor .canonicalize() - .map_err(|error| format!("open parent of {value}: {error}"))?; - self.inside_project(parent, value)?; - Ok(path) + .map_err(|error| format!("open ancestor of {value}: {error}"))?; + self.inside_project(resolved.clone(), value)?; + for name in suffix.into_iter().rev() { + resolved.push(name); + } + Ok(resolved) } fn inside_project(&self, path: PathBuf, original: &str) -> Result { @@ -367,6 +463,10 @@ impl Tools { } fn more(&mut self, call: &ToolCall) -> Result { + if self.more_text.is_some() { + let count = integer(call, "count", self.default_lines(), 1, usize::MAX); + return Ok(self.continue_text(count).unwrap()); + } let (path, start, raw) = self .more .clone() @@ -375,6 +475,22 @@ impl Tools { self.read_range(&path, start, count, false, raw) } + fn continue_text(&mut self, count: usize) -> Option { + let (text, start) = self.more_text.take()?; + let byte_limit = count.saturating_mul(80).min(self.result_limit() / 2); + let rest = &text[start.min(text.len())..]; + let end = rest + .char_indices() + .map(|(index, _)| index) + .find(|index| *index >= byte_limit) + .unwrap_or(rest.len()); + let output = rest[..end].to_owned(); + if start + end < text.len() { + self.more_text = Some((text, start + end)); + } + Some(output) + } + fn read_range( &mut self, path: &Path, @@ -383,6 +499,7 @@ impl Tools { whole: bool, raw: bool, ) -> Result { + self.more_text = None; let metadata = path.metadata().map_err(|error| error.to_string())?; if metadata.len() > MAX_FILE_BYTES { return Err(format!( @@ -554,16 +671,29 @@ impl Tools { )); let stdout = File::create(&output).map_err(|error| error.to_string())?; let output = output.canonicalize().map_err(|error| error.to_string())?; - self.temporary_files.insert(output.clone()); let stderr = stdout.try_clone().map_err(|error| error.to_string())?; - let child = Command::new("/bin/sh") + let mut process = Command::new("/bin/sh"); + process .arg("-c") + .arg(format!( + "ulimit -f {}; exec /bin/sh -c \"$1\"", + MAX_FILE_BYTES / 512 + )) + .arg("ds4-agent") .arg(&command) .current_dir(&self.root) .stdin(Stdio::null()) .stdout(stdout) .stderr(stderr) .process_group(0) + .env_clear(); + for name in SHELL_ENV_ALLOWLIST { + if let Some(value) = std::env::var_os(name) { + process.env(name, value); + } + } + process.env("PWD", &self.root); + let child = process .spawn() .map_err(|error| format!("bash failed to start: {error}"))?; self.jobs.insert( @@ -614,7 +744,7 @@ impl Tools { id: u32, refresh: usize, cancel: &AtomicBool, - remove_done: bool, + _remove_done: bool, ) -> Result { let deadline = Instant::now() + Duration::from_secs(refresh as u64); loop { @@ -633,19 +763,15 @@ impl Tools { } thread::sleep(Duration::from_millis(100)); } + let output_limit = self.result_limit().saturating_sub(1024).max(1024); let job = self.jobs.get_mut(&id).unwrap(); let status = job.child.try_wait().map_err(|error| error.to_string())?; - let bytes = fs::read(&job.output).unwrap_or_default(); + let bytes = fs::read(&job.output) + .map_err(|error| format!("read {}: {error}", job.output.display()))?; let first_observation = job.observed == 0; let new = &bytes[job.observed.min(bytes.len())..]; job.observed = bytes.len(); - let shown = if first_observation && new.len() > 8 * 1024 { - &new[..8 * 1024] - } else if new.len() > 32 * 1024 { - &new[new.len() - 32 * 1024..] - } else { - new - }; + let truncated = new.len() > output_limit; let mut result = format!( "bash job={} pid={} status={} command={}\noutput_path={}\n", job.id, @@ -654,23 +780,36 @@ impl Tools { job.command, job.output.display() ); - result.push_str(&String::from_utf8_lossy(shown)); - if shown.len() < new.len() { - if status.is_some() { - let tail = &new[new.len().saturating_sub(32 * 1024).max(shown.len())..]; - result.push_str("\n... middle output omitted ...\n"); - result.push_str(&String::from_utf8_lossy(tail)); + if truncated && status.is_some() { + let half = output_limit / 2; + result.push_str(&String::from_utf8_lossy(&new[..half])); + result.push_str("\n... middle output omitted; open output_path to inspect it ...\n"); + result.push_str(&String::from_utf8_lossy(&new[new.len() - half..])); + } else { + result.push_str(&String::from_utf8_lossy( + &new[..new.len().min(output_limit)], + )); + } + if truncated && status.is_none() { + if first_observation { + result.push_str("\n... output truncated; open output_path or use bash_status for new output ...\n"); } else { - result.push_str("\n... output truncated; use bash_status for new output ...\n"); + result.push_str( + "\n... output truncated; open output_path for the complete output ...\n", + ); } } if !result.ends_with('\n') { result.push('\n'); } - if remove_done && status.is_some() { + if status.is_some() && !truncated { let job = self.jobs.remove(&id).unwrap(); - self.temporary_files.remove(&job.output); - let _ = fs::remove_file(job.output); + if let Err(error) = fs::remove_file(&job.output) { + result.push_str(&format!( + "Tool warning: could not remove {}: {error}\n", + job.output.display() + )); + } } Ok(result) } @@ -689,33 +828,53 @@ impl Tools { return Err("visit_page requires an HTTP or HTTPS URL".into()); } let markdown = self.browser.visit_page(url, cancel)?; - let output = std::env::temp_dir().join(format!( - "ds4_agent_web_{}_{}", - std::process::id(), - SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap_or_default() - .as_nanos() - )); - fs::write(&output, &markdown).map_err(|error| error.to_string())?; - let output = output.canonicalize().map_err(|error| error.to_string())?; - self.temporary_files.insert(output.clone()); - let head = markdown - .lines() - .take(100) - .collect::>() - .join("\n") - .chars() - .take(8 * 1024) - .collect::(); + let line_end: usize = markdown.split_inclusive('\n').take(100).map(str::len).sum(); + let mut head_end = line_end.min(8 * 1024).min(markdown.len()); + while !markdown.is_char_boundary(head_end) { + head_end -= 1; + } + let head = &markdown[..head_end]; + if head_end < markdown.len() { + self.more = None; + self.more_text = Some((markdown.clone(), head_end)); + } Ok(format!( - "visit_page url={url}\noutput_path={} ({} bytes, {} lines)\n\n{head}\n\nUse read with raw=true to inspect more rendered text.\n", - output.display(), + "visit_page url={url} ({} bytes, {} lines)\n\n{head}\n\nPage output is bounded; use more to continue.\n", markdown.len(), - markdown.lines().count(), - output.display() + markdown.lines().count() )) } + + fn approval(&self, call: &ToolCall) -> Option { + if matches!(call.name.as_str(), "google_search" | "visit_page") { + return Some(ApprovalPrompt { + title: "Allow visible browser?".into(), + detail: "Start a visible Chrome browser for this tool call.".into(), + working_directory: self.root.clone(), + }); + } + let command = (call.name == "bash").then(|| string(call, "command"))??; + risky_shell_reason(command, &self.root).map(|reason| ApprovalPrompt { + title: "Allow shell command?".into(), + detail: format!("{reason}\n\n{command}"), + working_directory: self.root.clone(), + }) + } + + pub(crate) fn stop_all_jobs(&mut self) -> Vec { + let mut failures = Vec::new(); + for job in self.jobs.values_mut() { + stop_job(job); + if let Err(error) = fs::remove_file(&job.output) { + failures.push(format!( + "Could not remove {}: {error}", + job.output.display() + )); + } + } + self.jobs.clear(); + failures + } } struct SearchOptions<'a> { @@ -906,12 +1065,8 @@ fn wildcard_match(pattern: &str, value: &str) -> bool { impl Drop for Tools { fn drop(&mut self) { - for job in self.jobs.values_mut() { - stop_job(job); - let _ = fs::remove_file(&job.output); - } - for path in &self.temporary_files { - let _ = fs::remove_file(path); + for failure in self.stop_all_jobs() { + eprintln!("DS4Server tool cleanup: {failure}"); } } } @@ -920,37 +1075,139 @@ pub(crate) fn execute_async(tools: Arc>, calls: Vec) -> A let cancel = Arc::new(AtomicBool::new(false)); let worker_cancel = Arc::clone(&cancel); let (sender, results) = mpsc::channel(); - thread::Builder::new() + let (event_sender, events) = mpsc::channel(); + let worker = thread::Builder::new() .name("agent-tools".into()) .spawn(move || { let mut output = String::new(); let mut tools = tools .lock() .unwrap_or_else(|poisoned| poisoned.into_inner()); + for index in 0..calls.len() { + send_state(&event_sender, index, ToolLifecycle::Queued, None); + } for (index, call) in calls.iter().enumerate() { if worker_cancel.load(Ordering::Relaxed) { output.push_str("Tool error: interrupted\n"); + send_state(&event_sender, index, ToolLifecycle::Stopped, None); break; } + if let Some(prompt) = tools.approval(call) { + match request_approval(&event_sender, index, prompt, &worker_cancel) { + Ok(()) => {} + Err(error) => { + let result = format!("Tool error: {error}\n"); + output.push_str(&format!( + "Tool result {} ({}):\n{result}", + index + 1, + call.name + )); + send_state( + &event_sender, + index, + if worker_cancel.load(Ordering::Relaxed) { + ToolLifecycle::Stopped + } else { + ToolLifecycle::Failed + }, + Some(result), + ); + if worker_cancel.load(Ordering::Relaxed) { + break; + } + continue; + } + } + } + send_state(&event_sender, index, ToolLifecycle::Running, None); output.push_str(&format!("Tool result {} ({}):\n", index + 1, call.name)); - output.push_str(&tools.execute(call, &worker_cancel)); + let result = tools.execute(call, &worker_cancel); + let state = if worker_cancel.load(Ordering::Relaxed) + || result.contains("Tool error: interrupted") + { + ToolLifecycle::Stopped + } else if result.starts_with("Tool error:") { + ToolLifecycle::Failed + } else { + ToolLifecycle::Completed + }; + output.push_str(&result); + send_state(&event_sender, index, state, Some(result)); if !output.ends_with('\n') { output.push('\n'); } } + if worker_cancel.load(Ordering::Relaxed) { + for failure in tools.stop_all_jobs() { + output.push_str(&format!("Tool warning: {failure}\n")); + } + } let _ = sender.send(output); }) .expect("agent tool worker must start"); - ActiveTools { results, cancel } + ActiveTools { + results, + events, + cancel, + worker: Some(worker), + } } pub(crate) fn error_async(error: String) -> ActiveTools { let cancel = Arc::new(AtomicBool::new(false)); let (sender, results) = mpsc::channel(); + let (_event_sender, events) = mpsc::channel(); let _ = sender.send(format!( "Tool error: invalid tool call: {error}\nRetry using the exact tool syntax from the system prompt.\n" )); - ActiveTools { results, cancel } + ActiveTools { + results, + events, + cancel, + worker: None, + } +} + +fn send_state( + events: &Sender, + index: usize, + state: ToolLifecycle, + result: Option, +) { + let _ = events.send(ToolEvent::State { + index, + state, + result, + }); +} + +fn request_approval( + events: &Sender, + index: usize, + prompt: ApprovalPrompt, + cancel: &AtomicBool, +) -> Result<(), String> { + let (decision, response) = mpsc::channel(); + events + .send(ToolEvent::Approval { + index, + prompt, + decision, + }) + .map_err(|_| "approval UI is unavailable".to_owned())?; + loop { + if cancel.load(Ordering::Relaxed) { + return Err("interrupted while awaiting approval".into()); + } + match response.recv_timeout(Duration::from_millis(50)) { + Ok(true) => return Ok(()), + Ok(false) => return Err("user denied this action".into()), + Err(RecvTimeoutError::Timeout) => {} + Err(RecvTimeoutError::Disconnected) => { + return Err("approval UI closed without a decision".into()); + } + } + } } pub(crate) fn parse_tool_calls( @@ -1030,6 +1287,10 @@ pub(crate) fn try_tool_result(active: &ActiveTools) -> Result, St } } +pub(crate) fn try_tool_event(active: &ActiveTools) -> Option { + active.events.try_recv().ok() +} + pub(crate) fn visible_content(content: &str) -> &str { [ "<｜DSML｜tool_calls>", @@ -1043,28 +1304,182 @@ pub(crate) fn visible_content(content: &str) -> &str { .map_or(content, |end| content[..end].trim_end()) } -pub(crate) fn tool_summaries(model: ModelChoice, content: &str) -> Vec { - parse_tool_calls(model, content) - .map(|(_, calls)| { - calls - .into_iter() - .map(|call| { - let detail = ["path", "command", "query", "url"] - .into_iter() - .find_map(|name| string(&call, name)) - .map(|value| { - let mut value = value.replace('\n', " "); - if value.chars().count() > 120 { - value = value.chars().take(119).collect::() + "…"; - } - format!(" {value}") - }) - .unwrap_or_default(); - format!("🛠 {}{detail}", call.name) - }) - .collect() +pub(crate) fn has_tool_markup(content: &str) -> bool { + visible_content(content).len() < content.len() +} + +pub(crate) fn stored_tool_cards( + model: ModelChoice, + assistant: &str, + result: Option<&str>, +) -> Vec { + let calls = parse_tool_calls(model, assistant) + .map(|(_, calls)| calls) + .unwrap_or_default(); + let results = result.map(split_tool_results).unwrap_or_default(); + calls + .into_iter() + .enumerate() + .map(|(index, call)| { + let result = results.get(index).cloned(); + let state = result.as_deref().map_or(ToolLifecycle::Stopped, |result| { + if result.starts_with("Tool error:") { + ToolLifecycle::Failed + } else { + ToolLifecycle::Completed + } + }); + ToolCard { + call, + state, + result, + } }) - .unwrap_or_default() + .collect() +} + +fn split_tool_results(result: &str) -> Vec { + let starts = result + .match_indices("Tool result ") + .map(|(index, _)| index) + .collect::>(); + starts + .iter() + .enumerate() + .map(|(index, start)| { + let body = &result[*start..starts.get(index + 1).copied().unwrap_or(result.len())]; + body.split_once("\n") + .map_or(body, |(_, result)| result) + .trim_end() + .to_owned() + }) + .collect() +} + +pub(crate) fn bounded_tool_text(text: &str, limit: usize) -> String { + let mut output = text.replace('\n', " "); + if output.chars().count() > limit { + output = output + .chars() + .take(limit.saturating_sub(1)) + .collect::() + + "…"; + } + output +} + +pub(crate) fn tool_parameters(call: &ToolCall) -> String { + ["path", "command", "query", "url"] + .into_iter() + .find_map(|name| string(call, name).map(|value| (name, value))) + .map_or_else( + || bounded_tool_text(&Value::Object(call.arguments.clone()).to_string(), 240), + |(name, value)| format!("{name}: {}", bounded_tool_text(value, 240)), + ) +} + +pub(crate) fn tool_call_text(call: &ToolCall) -> String { + let mut tool = Map::new(); + tool.insert("name".into(), Value::String(call.name.clone())); + tool.insert("arguments".into(), Value::Object(call.arguments.clone())); + serde_json::to_string_pretty(&Value::Object(tool)).unwrap_or_default() +} + +pub(crate) fn tool_output_path(result: &str) -> Option { + result + .lines() + .find_map(|line| line.strip_prefix("output_path=")) + .map(PathBuf::from) + .filter(|path| path.is_absolute() && path.is_file()) +} + +fn risky_shell_reason(command: &str, root: &Path) -> Option<&'static str> { + let words = command + .split_whitespace() + .map(|word| { + word.trim_matches(|character: char| { + matches!( + character, + '\'' | '"' | ';' | '|' | '&' | '(' | ')' | '{' | '}' | '<' | '>' + ) + }) + .to_ascii_lowercase() + }) + .collect::>(); + if words + .iter() + .any(|word| matches!(word.as_str(), "sudo" | "doas" | "su")) + { + return Some("This command elevates privileges."); + } + if words + .iter() + .any(|word| matches!(word.as_str(), "open" | "osascript")) + { + return Some("This command launches or controls another application."); + } + if words.iter().any(|word| { + matches!( + word.as_str(), + "rm" | "rmdir" + | "unlink" + | "shred" + | "truncate" + | "dd" + | "mkfs" + | "diskutil" + | "kill" + | "killall" + | "pkill" + ) + }) || (words.iter().any(|word| word == "git") + && words + .iter() + .any(|word| matches!(word.as_str(), "clean" | "reset"))) + { + return Some("This command can delete data or discard state."); + } + if words.iter().any(|word| { + matches!( + word.as_str(), + "curl" | "wget" | "ssh" | "scp" | "sftp" | "ftp" | "nc" | "ncat" | "telnet" + ) + }) || (words.iter().any(|word| word == "git") + && words + .iter() + .any(|word| matches!(word.as_str(), "push" | "pull" | "fetch" | "clone"))) + || (words.iter().any(|word| { + matches!( + word.as_str(), + "cargo" | "npm" | "pnpm" | "yarn" | "pip" | "pip3" + ) + }) && words + .iter() + .any(|word| matches!(word.as_str(), "install" | "publish" | "login"))) + { + return Some("This command can create a network side effect."); + } + let root = root.to_string_lossy(); + if words.iter().any(|word| { + (word.contains("../") + || word == ".." + || word.starts_with("~/") + || word.contains("$home") + || word.starts_with('/')) + && !word.starts_with(root.as_ref()) + && !matches!( + word.as_str(), + "/bin/sh" | "/bin/bash" | "/usr/bin/env" | "/usr/bin/make" + ) + }) || words.iter().any(|word| { + matches!( + word.as_str(), + "brew" | "launchctl" | "defaults" | "mount" | "umount" | "chown" + ) + }) { + return Some("This command can access or change state outside the project."); + } + None } fn parse_glm_calls(text: &str) -> Result<(String, Vec<(String, Value)>), String> { @@ -1305,9 +1720,173 @@ mod tests { .as_nanos() )); fs::create_dir_all(&directory).unwrap(); + let outside = directory.with_extension("outside"); + fs::create_dir_all(&outside).unwrap(); + fs::write(outside.join("secret.txt"), "secret").unwrap(); + std::os::unix::fs::symlink(&outside, directory.join("escape")).unwrap(); let tools = Tools::new(&directory, 32_768).unwrap(); assert!(tools.writable_path("inside.txt").is_ok()); + assert!(tools.writable_path("missing/inside.txt").is_ok()); assert!(tools.writable_path("../outside.txt").is_err()); + assert!(tools.writable_path("missing/../../outside.txt").is_err()); + assert!(tools.existing_path("escape/secret.txt").is_err()); + assert!(tools.writable_path("escape/new.txt").is_err()); + fs::remove_dir_all(directory).unwrap(); + fs::remove_dir_all(outside).unwrap(); + } + + #[test] + fn risky_shell_commands_require_one_time_approval() { + let root = Path::new("/tmp/project"); + assert!(risky_shell_reason("cargo test --all-features", root).is_none()); + assert!(risky_shell_reason("git status --short", root).is_none()); + for command in [ + "rm -rf target", + "sudo make install", + "curl https://example.com", + "open report.html", + "touch ../outside", + "git push origin main", + ] { + assert!(risky_shell_reason(command, root).is_some(), "{command}"); + } + assert!(!SHELL_ENV_ALLOWLIST.contains(&"GITHUB_TOKEN")); + assert!(!SHELL_ENV_ALLOWLIST.contains(&"AWS_SECRET_ACCESS_KEY")); + assert!(!SHELL_ENV_ALLOWLIST.contains(&"SSH_AUTH_SOCK")); + } + + #[test] + fn denial_and_stop_cancel_commands_awaiting_approval() { + let directory = std::env::temp_dir().join(format!( + "ds4-agent-approval-{}", + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + fs::create_dir_all(&directory).unwrap(); + fs::write(directory.join("keep.txt"), "keep").unwrap(); + + let tools = Arc::new(Mutex::new(Tools::new(&directory, 4096).unwrap())); + let active = execute_async( + Arc::clone(&tools), + vec![call("bash", [("command", "rm keep.txt")])], + ); + let decision = loop { + match active.events.recv_timeout(Duration::from_secs(2)).unwrap() { + ToolEvent::Approval { + prompt, decision, .. + } => { + assert!(prompt.detail.contains("rm keep.txt")); + assert_eq!(prompt.working_directory, directory.canonicalize().unwrap()); + break decision; + } + ToolEvent::State { .. } => {} + } + }; + decision.send(false).unwrap(); + assert!( + active + .results + .recv_timeout(Duration::from_secs(2)) + .unwrap() + .contains("user denied") + ); + assert!(directory.join("keep.txt").exists()); + + let active = execute_async(tools, vec![call("google_search", [("query", "DS4")])]); + let _decision = loop { + match active.events.recv_timeout(Duration::from_secs(2)).unwrap() { + ToolEvent::Approval { + prompt, decision, .. + } => { + assert!(prompt.title.contains("browser")); + break decision; + } + ToolEvent::State { .. } => {} + } + }; + active.cancel.store(true, Ordering::Relaxed); + assert!( + active + .results + .recv_timeout(Duration::from_secs(2)) + .unwrap() + .contains("interrupted") + ); + fs::remove_dir_all(directory).unwrap(); + } + + #[test] + fn oversized_shell_output_is_bounded_and_retained_until_cleanup() { + let directory = std::env::temp_dir().join(format!( + "ds4-agent-output-{}", + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + fs::create_dir_all(&directory).unwrap(); + let mut tools = Tools::new(&directory, 4096).unwrap(); + let result = tools.execute( + &call( + "bash", + [("command", "/usr/bin/yes x | /usr/bin/head -c 20000")], + ), + &AtomicBool::new(false), + ); + assert!(result.len() <= tools.result_limit()); + assert!(result.contains("middle output omitted")); + let output = tool_output_path(&result).unwrap(); + assert_eq!(fs::metadata(&output).unwrap().len(), 20_000); + fs::write(directory.join("large.txt"), "match xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\n".repeat(500)).unwrap(); + let search = tools.execute( + &call("search", [("query", "match"), ("max_results", "500")]), + &AtomicBool::new(false), + ); + assert!(search.contains("Use more to continue")); + assert!( + !tools + .execute(&call("more", []), &AtomicBool::new(false)) + .is_empty() + ); + drop(tools); + assert!(!output.exists()); + fs::remove_dir_all(directory).unwrap(); + } + + #[test] + fn dropping_session_tools_stops_live_jobs_and_removes_output() { + let directory = std::env::temp_dir().join(format!( + "ds4-agent-switch-{}", + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + fs::create_dir_all(&directory).unwrap(); + let mut tools = Tools::new(&directory, 4096).unwrap(); + let result = tools.execute( + &call("bash", [("command", "sleep 30"), ("refresh_sec", "1")]), + &AtomicBool::new(false), + ); + assert!(result.contains("status=running")); + let pid = result + .split_whitespace() + .find_map(|field| field.strip_prefix("pid=")) + .unwrap() + .parse::() + .unwrap(); + let output = tool_output_path(&result).unwrap(); + drop(tools); + assert!( + !Command::new("/bin/kill") + .args(["-0", &pid.to_string()]) + .status() + .unwrap() + .success() + ); + assert!(!output.exists()); fs::remove_dir_all(directory).unwrap(); } diff --git a/src/app.rs b/src/app.rs index 3b90ae5..5bbedf9 100644 --- a/src/app.rs +++ b/src/app.rs @@ -103,6 +103,11 @@ pub(crate) struct App { #[cfg(target_os = "macos")] active_tools: Option, #[cfg(target_os = "macos")] + pub(super) tool_cards: Vec, + #[cfg(target_os = "macos")] + pub(super) pending_tool_approval: + Option<(crate::agent::ApprovalPrompt, std::sync::mpsc::Sender)>, + #[cfg(target_os = "macos")] active_titling: Option, #[cfg(target_os = "macos")] runtime_config: Arc>, @@ -199,6 +204,10 @@ pub(crate) enum Message { ComposerChanged(String), ToggleReasoning(usize), OpenLink(markdown::Url), + CopyToolText(String), + OpenToolOutput(PathBuf), + AllowToolOnce, + DenyTool, SubmitPrompt, StopGeneration, GenerationTick, @@ -316,6 +325,10 @@ impl App { #[cfg(target_os = "macos")] active_tools: None, #[cfg(target_os = "macos")] + tool_cards: Vec::new(), + #[cfg(target_os = "macos")] + pending_tool_approval: None, + #[cfg(target_os = "macos")] active_titling: None, #[cfg(target_os = "macos")] runtime_config, @@ -415,6 +428,10 @@ impl App { #[cfg(target_os = "macos")] active_tools: None, #[cfg(target_os = "macos")] + tool_cards: Vec::new(), + #[cfg(target_os = "macos")] + pending_tool_approval: None, + #[cfg(target_os = "macos")] active_titling: None, #[cfg(target_os = "macos")] runtime_config, @@ -778,6 +795,25 @@ impl App { self.error = Some(format!("Could not open the link: {error}")); } } + Message::CopyToolText(value) => return iced::clipboard::write(value), + Message::OpenToolOutput(path) => { + if let Err(error) = std::process::Command::new("open").arg(path).spawn() { + self.error = Some(format!("Could not open the tool output: {error}")); + } + } + Message::AllowToolOnce => { + #[cfg(target_os = "macos")] + if let Some((_, decision)) = self.pending_tool_approval.take() { + let _ = decision.send(true); + } + } + Message::DenyTool => + { + #[cfg(target_os = "macos")] + if let Some((_, decision)) = self.pending_tool_approval.take() { + let _ = decision.send(false); + } + } Message::SubmitPrompt => { self.start_generation(); return scroll_chat_to_end(); @@ -794,6 +830,12 @@ impl App { active.cancel.store(true, Ordering::Relaxed); } #[cfg(target_os = "macos")] + if let Some((_, decision)) = self.pending_tool_approval.take() { + let _ = decision.send(false); + } + #[cfg(target_os = "macos")] + self.stop_agent_jobs(); + #[cfg(target_os = "macos")] if let Some(compaction) = &self.active_compaction { compaction.active.cancel.store(true, Ordering::Relaxed); } @@ -1019,6 +1061,10 @@ impl App { if self.selected_session == Some(session_id) { return Task::none(); } + #[cfg(target_os = "macos")] + self.stop_agent_jobs(); + #[cfg(target_os = "macos")] + self.tool_cards.clear(); let saved_context = self .projects .iter() @@ -1298,6 +1344,10 @@ impl Drop for App { if let Some(active) = &self.active_tools { active.cancel.store(true, Ordering::Relaxed); } + #[cfg(target_os = "macos")] + if let Some((_, decision)) = self.pending_tool_approval.take() { + let _ = decision.send(false); + } } } diff --git a/src/app/generation.rs b/src/app/generation.rs index db5f6f6..139b62d 100644 --- a/src/app/generation.rs +++ b/src/app/generation.rs @@ -210,6 +210,8 @@ impl App { return; } #[cfg(target_os = "macos")] + self.tool_cards.clear(); + #[cfg(target_os = "macos")] if !std::mem::take(&mut self.skip_compaction_once) && crate::compaction::should_compact(self.context_used, self.context_limit) { @@ -388,11 +390,47 @@ impl App { return self.poll_tool_result_check(); } #[cfg(target_os = "macos")] + if self.active_tools.is_some() { + while let Some(event) = self + .active_tools + .as_ref() + .and_then(crate::agent::try_tool_event) + { + match event { + crate::agent::ToolEvent::State { + index, + state, + result, + } => { + if let Some(card) = self.tool_cards.get_mut(index) { + card.state = state; + if result.is_some() { + card.result = result; + } + } + self.activity = Some(format!("Tool {} · {}", index + 1, state.label())); + } + crate::agent::ToolEvent::Approval { + index, + prompt, + decision, + } => { + if let Some(card) = self.tool_cards.get_mut(index) { + card.state = crate::agent::ToolLifecycle::AwaitingApproval; + } + self.pending_tool_approval = Some((prompt, decision)); + self.activity = Some(format!("Tool {} · Awaiting approval", index + 1)); + } + } + } + } + #[cfg(target_os = "macos")] if let Some(active) = &self.active_tools { match crate::agent::try_tool_result(active) { Ok(Some(result)) => { let cancelled = active.cancel.load(Ordering::Relaxed); self.active_tools = None; + self.pending_tool_approval = None; if cancelled { self.generating = false; self.activity = Some("Stopped".into()); @@ -453,6 +491,13 @@ impl App { && !message.user { message.append(reasoning, &content); + if !reasoning + && self.tool_cards.is_empty() + && crate::agent::has_tool_markup(&message.content) + { + self.tool_cards.push(crate::agent::ToolCard::streaming()); + self.activity = Some("Parsing tool call…".into()); + } transcript_changed = true; } } @@ -501,6 +546,7 @@ impl App { Ok(_) => { self.generating = false; self.activity = None; + self.tool_cards.clear(); start_queued = !self.queued_inputs.is_empty() || self.manual_compaction_queued; } @@ -628,8 +674,13 @@ impl App { self.agent_tools = Some((session_id, Arc::new(Mutex::new(tools)))); } let tools = Arc::clone(&self.agent_tools.as_ref().unwrap().1); + self.tool_cards = calls + .iter() + .cloned() + .map(crate::agent::ToolCard::parsing) + .collect(); self.active_tools = Some(crate::agent::execute_async(tools, calls)); - self.activity = Some("Running tools…".into()); + self.activity = Some("Parsing tool calls…".into()); Ok(()) } @@ -685,6 +736,7 @@ impl App { .collect(); assistant.reasoning_open = assistant_reasoning; self.conversation.push(assistant); + self.tool_cards.clear(); let idle_timeout = Duration::from_secs(self.config.idle_timeout_minutes.max(1) as u64 * 60); self.active_generation = Some( self.generation_service @@ -704,6 +756,20 @@ impl App { Ok(()) } + #[cfg(target_os = "macos")] + pub(super) fn stop_agent_jobs(&mut self) { + let Some((_, tools)) = &self.agent_tools else { + return; + }; + let Ok(mut tools) = tools.try_lock() else { + return; + }; + let failures = tools.stop_all_jobs(); + if !failures.is_empty() { + self.error = Some(failures.join("\n")); + } + } + #[cfg(target_os = "macos")] fn start_tool_result_check( &mut self, diff --git a/src/app/projects.rs b/src/app/projects.rs index 3a377bd..a2922fc 100644 --- a/src/app/projects.rs +++ b/src/app/projects.rs @@ -69,6 +69,10 @@ impl App { if self.database.is_none() { return; } + #[cfg(target_os = "macos")] + self.stop_agent_jobs(); + #[cfg(target_os = "macos")] + self.tool_cards.clear(); let title = draft_title(&self.projects, project_id); self.drafts.entry(project_id).or_insert(title); self.remember_project(project_id); diff --git a/src/app/view.rs b/src/app/view.rs index 81478ec..0d7d82e 100644 --- a/src/app/view.rs +++ b/src/app/view.rs @@ -69,6 +69,16 @@ impl App { || self.pending_project_path.is_some() || self.session_rename.is_some() || self.menu_session().is_some() + || { + #[cfg(target_os = "macos")] + { + self.pending_tool_approval.is_some() + } + #[cfg(not(target_os = "macos"))] + { + false + } + } } fn main_view(&self) -> Element<'_, Message> { @@ -112,6 +122,19 @@ impl App { let content: Element<'_, Message> = shell.into(); let mut layers = vec![content]; + #[cfg(target_os = "macos")] + if let Some((prompt, _)) = &self.pending_tool_approval { + layers.push(self.tool_approval_panel(prompt)); + } else if self.preferences_open { + layers.push(self.preferences_panel()); + } else if let Some(path) = &self.pending_project_path { + layers.push(self.project_dialog(path)); + } else if let Some((_, title)) = &self.session_rename { + layers.push(self.rename_dialog(title)); + } else if let Some(session) = self.menu_session() { + layers.push(self.session_menu_panel(session)); + } + #[cfg(not(target_os = "macos"))] if self.preferences_open { layers.push(self.preferences_panel()); } else if let Some(path) = &self.pending_project_path { @@ -128,6 +151,39 @@ impl App { .into() } + #[cfg(target_os = "macos")] + fn tool_approval_panel<'a>( + &'a self, + prompt: &'a crate::agent::ApprovalPrompt, + ) -> Element<'a, Message> { + let dialog = container( + column![ + text(&prompt.title).size(22), + text(&prompt.detail).size(13), + text("Working directory").size(11).color(muted_text()), + text(prompt.working_directory.display().to_string()).size(13), + row![ + Space::with_width(Length::Fill), + action_button("Deny").on_press(Message::DenyTool), + action_button("Allow once").on_press(Message::AllowToolOnce), + ] + .spacing(8), + ] + .spacing(12), + ) + .padding(22) + .width(560) + .style(overview_style); + opaque( + container(dialog) + .center_x(Length::Fill) + .center_y(Length::Fill) + .style(|_| { + container::Style::default().background(Color::from_rgba8(0, 0, 0, 0.68)) + }), + ) + } + fn sidebar(&self) -> Element<'_, Message> { let preference_content = row![ icon(ICON_SETTINGS, 17), diff --git a/src/app/view/chat.rs b/src/app/view/chat.rs index fa8d38c..963c85e 100644 --- a/src/app/view/chat.rs +++ b/src/app/view/chat.rs @@ -75,6 +75,17 @@ impl App { if message.system { continue; } + if message.tool + && index > 0 + && !crate::agent::stored_tool_cards( + self.config.model, + &self.conversation[index - 1].content, + None, + ) + .is_empty() + { + continue; + } let label = if message.user { "You" } else if message.tool { @@ -135,11 +146,39 @@ impl App { text(self.activity.as_deref().unwrap_or("Loading model…")).size(14), ); } - if !message.user && !message.tool { - for summary in - crate::agent::tool_summaries(self.config.model, &message.content) - { - body = body.push(text(summary).size(13).color(muted_text())); + if !message.user { + let stored_result = self + .conversation + .get(index + 1) + .filter(|message| message.tool) + .map(|message| message.content.as_str()); + let cards = if stored_result.is_none() && { + #[cfg(target_os = "macos")] + { + !self.tool_cards.is_empty() + } + #[cfg(not(target_os = "macos"))] + { + false + } + } { + #[cfg(target_os = "macos")] + { + self.tool_cards.clone() + } + #[cfg(not(target_os = "macos"))] + { + Vec::new() + } + } else { + crate::agent::stored_tool_cards( + self.config.model, + &message.content, + stored_result, + ) + }; + if !cards.is_empty() { + body = body.push(tool_cards(cards)); } } let user = message.user; @@ -295,3 +334,80 @@ impl App { .into() } } + +fn tool_cards(cards: Vec) -> Element<'static, Message> { + let mut rows = column![].spacing(0); + for (index, card) in cards.into_iter().enumerate() { + if index > 0 { + rows = rows.push(horizontal_rule(1)); + } + let parameters = crate::agent::tool_parameters(&card.call); + let call = crate::agent::tool_call_text(&card.call); + let copy_call = tooltip( + action_button(text("Copy call").size(11)) + .padding([5, 9]) + .on_press(Message::CopyToolText(call)), + container(text("Copy tool name and all arguments").size(11)) + .padding(8) + .style(preference_group_style), + tooltip::Position::Top, + ) + .gap(6); + let copy_result = action_button(text("Copy result").size(11)).padding([5, 9]); + let copy_result = if let Some(result) = &card.result { + copy_result.on_press(Message::CopyToolText(result.clone())) + } else { + copy_result + }; + let copy_result = tooltip( + copy_result, + container(text("Copy the complete tool result").size(11)) + .padding(8) + .style(preference_group_style), + tooltip::Position::Top, + ) + .gap(6); + let mut actions = row![copy_call, copy_result] + .spacing(6) + .align_y(Alignment::Center); + if let Some(path) = card + .result + .as_deref() + .and_then(crate::agent::tool_output_path) + { + actions = actions.push( + tooltip( + action_button(text("Open output").size(11)) + .padding([5, 9]) + .on_press(Message::OpenToolOutput(path)), + container(text("Open the complete output file").size(11)) + .padding(8) + .style(preference_group_style), + tooltip::Position::Top, + ) + .gap(6), + ); + } + let mut content = column![ + row![ + text(card.call.name).size(13), + Space::with_width(Length::Fill), + text(card.state.label()).size(11).color(muted_text()), + actions, + ] + .spacing(8) + .align_y(Alignment::Center), + text(parameters).size(12).color(muted_text()), + ] + .spacing(5); + if let Some(result) = card.result { + let bounded = crate::agent::bounded_tool_text(&result, 1_200); + content = content.push(text(bounded).size(12)); + } + rows = rows.push(container(content).padding(10).width(Length::Fill)); + } + container(rows) + .width(Length::Fill) + .style(preference_group_style) + .into() +}