acp runtime: write the session id by rename, and fail the first prompt when the system prompt is unreadable

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<P: CompactionPolicy> AcpRuntime<P> {
         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<String, AcpError> {
     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();