diff --git a/hive-runtime/src/acp/mod.rs b/hive-runtime/src/acp/mod.rs index c92b7321f..fade13c64 100644 --- a/hive-runtime/src/acp/mod.rs +++ b/hive-runtime/src/acp/mod.rs @@ -112,6 +112,15 @@ pub enum AcpError { NoHttpMcp, #[error("reading the MCP config {path}: {detail}")] McpConfig { path: PathBuf, detail: String }, + #[error( + "the system prompt {path} is unreadable ({source}), so the first prompt of the session \ + was not sent; the session stays pending and the next turn sends it" + )] + SystemPrompt { + path: PathBuf, + #[source] + source: std::io::Error, + }, #[error( "ACP agent sent nothing for {silent_secs}s, so its turn was cancelled; a \ provider error the agent retries without reporting it, such as HTTP 429, looks like this" @@ -558,7 +567,7 @@ impl AcpRuntime

{ let (servers, names) = mcp_servers(config)?; let (session, created) = self.attach(live, &cwd, &servers).await?; choose_model_and_effort(live, &session, config, sink).await?; - let text = prompt_text(config, prompt, created); + let text = prompt_text(config, prompt, created)?; let params = json!({ "sessionId": session, "prompt": [{ "type": "text", "text": text }] }); discard_stale(live)?; let mut reply = live.conn.send("session/prompt", params).await?; @@ -1007,16 +1016,22 @@ async fn choose( } /// The prompt as sent: the first one of a session carries the system prompt. -fn prompt_text(config: &Config, prompt: &str, first: bool) -> String { +/// An unreadable prompt file fails the turn instead of sending the bare +/// prompt, because the session would then run without it for good. +fn prompt_text( + config: &Config, + prompt: &str, + first: bool, +) -> std::result::Result { match (first, &config.system_prompt_file) { (true, Some(path)) => match std::fs::read_to_string(path) { - Ok(system) => format!("{system}\n\n{prompt}"), - Err(e) => { - tracing::warn!(path = %path.display(), error = %e, "system prompt unreadable"); - prompt.to_owned() - } + Ok(system) => Ok(format!("{system}\n\n{prompt}")), + Err(source) => Err(AcpError::SystemPrompt { + path: path.clone(), + source, + }), }, - _ => prompt.to_owned(), + _ => Ok(prompt.to_owned()), } } @@ -1031,7 +1046,13 @@ fn write_id(file: &Path, id: &str) -> std::result::Result<(), AcpError> { if let Some(dir) = file.parent() { std::fs::create_dir_all(dir)?; } - std::fs::write(file, id)?; + // Write beside the target and rename over it: a crash leaves the old id or + // the new one, never an empty file that `read_id` would treat as no session. + let mut tmp = file.as_os_str().to_owned(); + tmp.push(".tmp"); + let tmp = PathBuf::from(tmp); + std::fs::write(&tmp, id)?; + std::fs::rename(&tmp, file)?; Ok(()) } @@ -1370,6 +1391,43 @@ done assert_eq!(recorded(dir.path()), "s1"); } + #[tokio::test] + async fn an_unreadable_system_prompt_fails_the_turn_and_the_next_one_sends_it() { + let dir = tempfile::tempdir().unwrap(); + let (runtime, config) = (runtime(dir.path(), "ok"), config(dir.path())); + std::fs::remove_file(dir.path().join("system.md")).unwrap(); + + let failed = runtime.run(&config, "one", &NoopSink).await; + assert!( + matches!(failed, Err(Error::Acp(AcpError::SystemPrompt { .. }))), + "{failed:?}" + ); + assert!( + !dir.path().join("prompts").exists(), + "no prompt reached the agent" + ); + assert!(!dir.path().join("session").exists()); + + std::fs::write(dir.path().join("system.md"), "SYSTEM PROMPT").unwrap(); + let retried = runtime.run(&config, "two", &NoopSink).await.unwrap(); + assert!(retried.created); + assert_eq!(carried_system_prompt(dir.path()), [true]); + } + + #[test] + fn a_session_id_is_written_by_rename_and_leaves_no_temporary_file() { + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("session"); + super::write_id(&file, "s1").unwrap(); + super::write_id(&file, "s2").unwrap(); + assert_eq!(super::read_id(&file).as_deref(), Some("s2")); + let names: Vec<_> = std::fs::read_dir(dir.path()) + .unwrap() + .map(|e| e.unwrap().file_name()) + .collect(); + assert_eq!(names, ["session"]); + } + #[tokio::test] async fn a_turn_silent_for_the_idle_window_is_cancelled_and_fails() { let dir = tempfile::tempdir().unwrap();