diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 20111b7ca0..a81bc1e94d 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -678,48 +678,50 @@ pub fn delete_managed_agent( app: AppHandle, state: State<'_, AppState>, ) -> Result<(), String> { - let _store_guard = state - .managed_agents_store_lock - .lock() - .map_err(|error| error.to_string())?; - let mut records = load_managed_agents(&app)?; - let mut runtimes = state - .managed_agent_processes - .lock() - .map_err(|error| error.to_string())?; + { + let _store_guard = state + .managed_agents_store_lock + .lock() + .map_err(|error| error.to_string())?; + let mut records = load_managed_agents(&app)?; + let mut runtimes = state + .managed_agent_processes + .lock() + .map_err(|error| error.to_string())?; - if sync_managed_agent_processes(&mut records, &mut runtimes) { + if sync_managed_agent_processes(&mut records, &mut runtimes) { + save_managed_agents(&app, &records)?; + } + + // Guard: reject deletion of deployed remote agents unless explicitly forced. + // This turns "don't orphan remote infra" from a UI convention into a backend + // invariant — a buggy or compromised IPC caller cannot silently orphan a live + // remote deployment. The frontend sends force_remote_delete: true only after + // the user confirms the orphan warning. + if let Some(record) = records.iter().find(|r| r.pubkey == pubkey) { + if record.backend != BackendKind::Local + && record.backend_agent_id.is_some() + && !force_remote_delete.unwrap_or(false) + { + return Err( + "cannot delete a deployed remote agent without force_remote_delete: true" + .to_string(), + ); + } + } + + if let Some(record) = records.iter_mut().find(|record| record.pubkey == pubkey) { + // For local agents: kills the process. For remote agents: no-op (the frontend + // sends !shutdown via WebSocket before calling delete). Either way, safe. + stop_managed_agent_process(&app, record, &mut runtimes)?; + } + let initial_len = records.len(); + records.retain(|record| record.pubkey != pubkey); + if records.len() == initial_len { + return Err(format!("agent {pubkey} not found")); + } save_managed_agents(&app, &records)?; } - - // Guard: reject deletion of deployed remote agents unless explicitly forced. - // This turns "don't orphan remote infra" from a UI convention into a backend - // invariant — a buggy or compromised IPC caller cannot silently orphan a live - // remote deployment. The frontend sends force_remote_delete: true only after - // the user confirms the orphan warning. - if let Some(record) = records.iter().find(|r| r.pubkey == pubkey) { - if record.backend != BackendKind::Local - && record.backend_agent_id.is_some() - && !force_remote_delete.unwrap_or(false) - { - return Err( - "cannot delete a deployed remote agent without force_remote_delete: true" - .to_string(), - ); - } - } - - if let Some(record) = records.iter_mut().find(|record| record.pubkey == pubkey) { - // For local agents: kills the process. For remote agents: no-op (the frontend - // sends !shutdown via WebSocket before calling delete). Either way, safe. - stop_managed_agent_process(&app, record, &mut runtimes)?; - } - let initial_len = records.len(); - records.retain(|record| record.pubkey != pubkey); - if records.len() == initial_len { - return Err(format!("agent {pubkey} not found")); - } - save_managed_agents(&app, &records)?; if let Err(error) = regenerate_nest_context(&app) { eprintln!("sprout-desktop: nest context regeneration failed: {error}"); } diff --git a/desktop/src-tauri/src/commands/personas.rs b/desktop/src-tauri/src/commands/personas.rs index 5312429967..39db8a987e 100644 --- a/desktop/src-tauri/src/commands/personas.rs +++ b/desktop/src-tauri/src/commands/personas.rs @@ -128,22 +128,20 @@ pub fn update_persona( .filter(|s| !s.is_empty()) .collect(); if let Some(env_vars) = input.env_vars { - // Caller explicitly sent env_vars — replace entirely (empty = clear). crate::managed_agents::validate_user_env_keys(&env_vars)?; persona.env_vars = env_vars; } - // Absent env_vars means "don't touch" — preserve existing creds when - // the caller only meant to edit a different field. persona.updated_at = now_iso(); save_personas(&app, &personas)?; + let result = personas + .into_iter() + .find(|record| record.id == input.id) + .ok_or_else(|| format!("persona {} disappeared unexpectedly", input.id))?; if let Err(error) = regenerate_nest_context(&app) { eprintln!("sprout-desktop: nest context regeneration failed: {error}"); } - personas - .into_iter() - .find(|record| record.id == input.id) - .ok_or_else(|| format!("persona {} disappeared unexpectedly", input.id)) + Ok(result) } #[tauri::command] diff --git a/desktop/src-tauri/src/managed_agents/nest.rs b/desktop/src-tauri/src/managed_agents/nest.rs index b0314da912..fc40f3a7e8 100644 --- a/desktop/src-tauri/src/managed_agents/nest.rs +++ b/desktop/src-tauri/src/managed_agents/nest.rs @@ -31,6 +31,9 @@ const NEST_DIRS: &[&str] = &[ /// Fully static — no runtime interpolation, no secrets, no user paths. const AGENTS_MD: &str = include_str!("nest_agents.md"); +const BEGIN_MARKER: &str = ""; + /// Returns the nest root path (`~/.sprout`), or `None` if the home /// directory cannot be resolved. pub fn nest_dir() -> Option { @@ -137,6 +140,10 @@ const CLI_QUICK_REFERENCE: &str = "\ `sprout workflows trigger --workflow ` — trigger a workflow Run `sprout --help` for the full command reference."; +fn escape_md_cell(s: &str) -> String { + s.replace('|', "\\|").replace('\n', " ") +} + pub fn render_dynamic_section( personas: &[PersonaRecord], agents: &[ManagedAgentRecord], @@ -147,7 +154,7 @@ pub fn render_dynamic_section( .to_string() } else { let mut table = - "## Active Agents\n\n| Name | Role | How to address |\n|------|------|----------------|" + "## Active Agents\n\n| Name | Persona | How to address |\n|------|---------|----------------|" .to_string(); for agent in agents { let role = agent @@ -156,10 +163,9 @@ pub fn render_dynamic_section( .and_then(|pid| personas.iter().find(|p| p.id == pid)) .map(|p| p.display_name.as_str()) .unwrap_or("—"); - table.push_str(&format!( - "\n| {} | {} | @{} |", - agent.name, role, agent.name - )); + let name = escape_md_cell(&agent.name); + let role_escaped = escape_md_cell(role); + table.push_str(&format!("\n| {name} | {role_escaped} | @{name} |")); } table }; @@ -167,45 +173,80 @@ pub fn render_dynamic_section( format!("{active_agents}\n\n## Workspace\n- Relay: {relay_url}\n\n{CLI_QUICK_REFERENCE}") } +/// Find a marker that appears at the start of a line (position 0 or preceded by `\n`). +fn find_marker_at_line_start(content: &str, marker: &str) -> Option { + let mut search_from = 0; + while let Some(pos) = content[search_from..].find(marker) { + let abs_pos = search_from + pos; + if abs_pos == 0 || content.as_bytes()[abs_pos - 1] == b'\n' { + return Some(abs_pos); + } + search_from = abs_pos + 1; + } + None +} + +/// Find the first valid ordered BEGIN/END marker pair, both at line starts. +/// Returns `(begin_line_start, after_end)` byte offsets for slicing. +fn find_managed_markers(content: &str) -> Option<(usize, usize)> { + let begin_pos = find_marker_at_line_start(content, BEGIN_MARKER)?; + let begin_line_start = content[..begin_pos].rfind('\n').map(|p| p + 1).unwrap_or(0); + let end_pos = content[begin_pos..].find(END_MARKER).map(|p| p + begin_pos)?; + let end_of_end = end_pos + END_MARKER.len(); + let after_end = if content[end_of_end..].starts_with('\n') { + end_of_end + 1 + } else { + end_of_end + }; + Some((begin_line_start, after_end)) +} + +/// Remove an orphan BEGIN marker line (one with no matching END after it). +fn strip_orphan_begin_marker(content: &str) -> String { + if let Some(pos) = find_marker_at_line_start(content, BEGIN_MARKER) { + let line_start = content[..pos].rfind('\n').map(|p| p + 1).unwrap_or(0); + let line_end = content[pos..].find('\n').map(|p| pos + p + 1).unwrap_or(content.len()); + format!( + "{}{}", + &content[..line_start], + content[line_end..].trim_start_matches('\n') + ) + } else { + content.to_string() + } +} + pub fn upsert_managed_section(file_path: &Path, new_section_content: &str) -> io::Result<()> { let current = fs::read_to_string(file_path)?; - const BEGIN: &str = ""; - let replacement = format!( - "\n{new_section_content}\n\n" + "{BEGIN_MARKER} — regenerated automatically, do not edit below -->\n{new_section_content}\n{END_MARKER}\n" ); - let new_content = - if let (Some(begin_pos), Some(end_pos)) = (current.find(BEGIN), current.find(END)) { - // Find the start of the BEGIN marker's line. - let line_start = current[..begin_pos].rfind('\n').map(|p| p + 1).unwrap_or(0); - // END marker spans to the end of its content + the newline after it. - let end_of_end = end_pos + END.len(); - let after_end = if current[end_of_end..].starts_with('\n') { - end_of_end + 1 - } else { - end_of_end - }; + let new_content = match find_managed_markers(¤t) { + Some((begin_line_start, after_end)) => { format!( "{}{}{}", - ¤t[..line_start], + ¤t[..begin_line_start], replacement, ¤t[after_end..] ) - } else { - format!("{}\n\n{}", current.trim_end_matches('\n'), replacement) - }; + } + None => { + let cleaned = strip_orphan_begin_marker(¤t); + format!("{}\n\n{}", cleaned.trim_end_matches('\n'), replacement) + } + }; - let tmp_path = file_path.with_extension( - file_path - .extension() - .map(|e| format!("{}.tmp", e.to_string_lossy())) - .unwrap_or_else(|| "tmp".to_string()), - ); - fs::write(&tmp_path, new_content)?; - fs::rename(&tmp_path, file_path)?; + let parent = file_path.parent().ok_or_else(|| { + io::Error::new(io::ErrorKind::InvalidInput, "file path has no parent directory") + })?; + let mut tmp = tempfile::NamedTempFile::new_in(parent)?; + { + use std::io::Write; + tmp.write_all(new_content.as_bytes())?; + } + tmp.persist(file_path).map_err(|e| e.error)?; Ok(()) } @@ -399,6 +440,7 @@ mod tests { let agents = vec![make_agent("Kit", Some("p1"))]; let output = render_dynamic_section(&personas, &agents, "ws://example.com:3000"); assert!(output.contains("| Kit | Builder | @Kit |")); + assert!(output.contains("| Name | Persona | How to address |")); assert!(output.contains("## CLI Quick Reference")); } @@ -468,10 +510,214 @@ mod tests { upsert_managed_section(&file, "content").unwrap(); - let tmp_path = file.with_extension("md.tmp"); + // Verify no stray temp files in the directory + let entries: Vec<_> = fs::read_dir(tmp.path()) + .unwrap() + .filter_map(|e| e.ok()) + .collect(); + assert_eq!(entries.len(), 1, "only AGENTS.md should remain, no temp files"); + assert_eq!(entries[0].file_name(), "AGENTS.md"); + } + + #[test] + fn test_upsert_end_before_begin() { + // An END marker that precedes a BEGIN marker forms no valid ordered pair. + // find_managed_markers returns None (BEGIN found, but no END after it), + // so the orphan BEGIN line is stripped and a new block is appended. + // The stray END line and content between END and BEGIN remain in the file + // because strip_orphan_begin_marker only removes the BEGIN line itself. + let tmp = tempfile::tempdir().unwrap(); + let file = tmp.path().join("AGENTS.md"); + fs::write( + &file, + "# Header\n\n\nsome middle content\n\nold section\n", + ) + .unwrap(); + + upsert_managed_section(&file, "new section").unwrap(); + + let result = fs::read_to_string(&file).unwrap(); + + assert!(result.contains("# Header"), "original header must survive"); + assert!(result.contains("new section"), "new content must be present"); + assert!(result.contains("some middle content"), "content between markers must survive"); + + // Exactly one BEGIN marker in the output (the orphan was stripped, new one appended). + assert_eq!( + result.matches(BEGIN_MARKER).count(), + 1, + "exactly one BEGIN marker after orphan cleanup" + ); + + // The single BEGIN marker must have a matching END marker after it. + let begin_pos = result.find(BEGIN_MARKER).expect("BEGIN marker must be present"); + let end_pos = result[begin_pos..].find(END_MARKER).map(|p| begin_pos + p); assert!( - !tmp_path.exists(), - ".tmp file should not exist after successful upsert" + end_pos.is_some(), + "an END marker must appear after the appended BEGIN marker" + ); + } + + #[test] + fn test_upsert_begin_only_no_end() { + // A file with BEGIN but no END has an orphan marker. + // find_managed_markers returns None (no END found after BEGIN), + // so strip_orphan_begin_marker removes the BEGIN line. + // Content that followed the orphan BEGIN is preserved (only the marker line is stripped, + // not the body that came after it). + let tmp = tempfile::tempdir().unwrap(); + let file = tmp.path().join("AGENTS.md"); + fs::write( + &file, + "# Header\n\nsome content\n\n\norphaned section without end marker\n", + ) + .unwrap(); + + upsert_managed_section(&file, "fresh section").unwrap(); + + let result = fs::read_to_string(&file).unwrap(); + + assert!(result.contains("# Header"), "original header must survive"); + assert!(result.contains("some content"), "original body must survive"); + assert!(result.contains("fresh section"), "new content must be present"); + + let begin_pos = result.find(BEGIN_MARKER).expect("BEGIN marker must be present"); + let end_pos = result.find(END_MARKER).expect("END marker must be present"); + assert!( + begin_pos < end_pos, + "the appended BEGIN marker must precede the appended END marker" + ); + + // Exactly one BEGIN marker after orphan cleanup. + assert_eq!( + result.matches(BEGIN_MARKER).count(), + 1, + "exactly one BEGIN marker after orphan cleanup" + ); + } + + #[test] + fn test_upsert_duplicate_markers() { + let tmp = tempfile::tempdir().unwrap(); + let file = tmp.path().join("AGENTS.md"); + fs::write( + &file, + "# Header\n\n\nfirst block\n\n\nbetween blocks\n\n\nsecond block\n\n", + ) + .unwrap(); + + upsert_managed_section(&file, "replaced").unwrap(); + + let result = fs::read_to_string(&file).unwrap(); + + assert!(result.contains("replaced"), "replacement content must be present"); + assert!(!result.contains("first block"), "first block must be replaced"); + assert!(result.contains("second block"), "second pair content must survive"); + assert!(result.contains("between blocks"), "text between pairs must survive"); + } + + #[test] + fn test_upsert_marker_in_code_block() { + let tmp = tempfile::tempdir().unwrap(); + let file = tmp.path().join("AGENTS.md"); + // Indented by 4 spaces — not at column 0, so should NOT match as a real marker. + fs::write( + &file, + "# Header\n\n \n\nReal content here\n", + ) + .unwrap(); + + upsert_managed_section(&file, "appended content").unwrap(); + + let result = fs::read_to_string(&file).unwrap(); + + assert!( + result.contains(" "), + "indented marker inside code block must be preserved verbatim" + ); + assert!(result.contains("appended content"), "new content must be appended"); + assert!(result.contains("Real content here"), "existing body must survive"); + + // The real markers appended at the end must be at line-start (column 0). + let begin_pos = result + .find("\nexisting section\n\n", + ) + .unwrap(); + + upsert_managed_section(&file, "same content").unwrap(); + let after_first = fs::read_to_string(&file).unwrap(); + + upsert_managed_section(&file, "same content").unwrap(); + let after_second = fs::read_to_string(&file).unwrap(); + + assert_eq!( + after_first, after_second, + "upsert must be idempotent: second call must not alter the file" ); } }