fix(actions): recover the action array from one-shot agent responses

crates/acp-client/README.md

@@ -81,7 +81,7 @@ if let Some(agent) = find_acp_agent_by_id("goose") {
 
 ### Prompt Functions
 
-- `run_acp_prompt(agent, working_dir, prompt) -> anyhow::Result<String>` - Send a one-shot prompt
+- `run_acp_prompt(agent, working_dir, prompt) -> anyhow::Result<String>` - Send a one-shot prompt; returns the agent's text only (tool calls and results are omitted)
 
 ### Types
 

crates/acp-client/src/driver.rs

@@ -2595,6 +2595,11 @@ fn extract_content_preview(content: &[ToolCallContent]) -> Option<String> {
 // =============================================================================
 
 /// Simple in-memory message writer for basic usage.
+///
+/// Only agent *text* is recorded. Tool calls and tool results are deliberately
+/// dropped: this writer backs one-shot prompting ([`crate::run_acp_prompt`]),
+/// whose callers parse the accumulated text as machine-readable output (JSON).
+/// Interleaving tool markers would corrupt that payload.
 pub struct BasicMessageWriter {
     text: Mutex<String>,
     last_flush_at: Mutex<Instant>,
@@ -2634,11 +2639,11 @@ impl MessageWriter for BasicMessageWriter {
     async fn record_tool_call(
         &self,
         _tool_call_id: &str,
-        title: &str,
+        _title: &str,
         _raw_input: Option<&serde_json::Value>,
     ) {
-        let mut current = self.text.lock().await;
-        current.push_str(&format!("\n[Tool: {}]\n", title));
+        // Intentionally ignored: see the type-level docs. Tool activity must not
+        // land in the buffer that one-shot callers parse.
     }
 
     async fn update_tool_call_title(
@@ -2650,9 +2655,8 @@ impl MessageWriter for BasicMessageWriter {
         // Nothing to do for basic implementation
     }
 
-    async fn record_tool_result(&self, _tool_call_id: &str, content: &str) {
-        let mut current = self.text.lock().await;
-        current.push_str(&format!("\n[Result: {}]\n", content));
+    async fn record_tool_result(&self, _tool_call_id: &str, _content: &str) {
+        // Intentionally ignored: see the type-level docs.
     }
 }
 
@@ -3523,6 +3527,20 @@ agent: http=false, sse=false). Select a provider that supports MCP over HTTP/SSE
         assert_eq!(buffer.match_cursor, 2);
     }
 
+    #[tokio::test]
+    async fn basic_writer_records_only_agent_text() {
+        let writer = BasicMessageWriter::new();
+
+        writer.append_text("[").await;
+        writer.record_tool_call("call-1", "Terminal", None).await;
+        writer.record_tool_result("call-1", "total 42").await;
+        writer.append_text("]").await;
+
+        // One-shot callers parse this buffer as JSON, so tool activity must not
+        // leak into it.
+        assert_eq!(writer.get_text().await, "[]");
+    }
+
     #[tokio::test]
     async fn replay_handler_treats_user_only_boundaries_as_complete() {
         let writer: Arc<dyn MessageWriter> = Arc::new(BasicMessageWriter::new());

crates/acp-client/src/simple.rs

@@ -96,7 +96,9 @@ impl AgentDriver for SimpleDriverWrapper {
 ///
 /// # Returns
 ///
-/// The agent's text response
+/// The agent's text response. Tool calls and tool results the agent made along
+/// the way are not included, so the result stays parseable when the prompt asks
+/// for structured output.
 pub async fn run_acp_prompt(agent: &AcpAgent, working_dir: &Path, prompt: &str) -> Result<String> {
     run_acp_prompt_with_options(agent, working_dir, prompt, None).await
 }

crates/builderbot-actions/src/detector.rs

@@ -297,67 +297,160 @@ fn has_git_hooks_path_override(working_dir: &Path) -> bool {
         .unwrap_or(false)
 }
 
-/// Parse the AI response and extract suggested actions
+/// Parse the AI response and extract suggested actions.
+///
+/// The response is not guaranteed to be bare JSON: agents wrap the array in a
+/// markdown code fence, prefix it with prose, and may emit brackets of their own
+/// (`[1]`, glob patterns, transcript markers) before the answer. So candidate
+/// `[` positions are scanned from the **end** of the response — the agent's
+/// final array wins — and each candidate is prefix-parsed, which lets trailing
+/// text such as a closing fence be ignored. A candidate only counts if it
+/// deserializes into `Vec<SuggestedAction>`, so unrelated arrays are skipped.
+///
+/// When nothing parses, a candidate that is a valid JSON array of objects but
+/// fails the `SuggestedAction` shape is almost certainly the agent's answer
+/// with a drifted field, so the typed error for the closest such near-miss is
+/// surfaced instead of the generic not-found message.
 fn parse_ai_response(response: &str) -> Result<Vec<SuggestedAction>> {
-    let json_str = extract_json_array(response)?;
-
-    let actions: Vec<SuggestedAction> = serde_json::from_str(&json_str).map_err(|e| {
-        anyhow::anyhow!(
-            "Failed to parse AI response as JSON: {}. Response was: {}",
-            e,
-            json_str
-        )
-    })?;
-
-    Ok(actions)
-}
-
-/// Extract JSON array from AI response that might contain extra text
-fn extract_json_array(text: &str) -> Result<String> {
-    // First try to parse the entire response as JSON
-    if text.trim().starts_with('[') && serde_json::from_str::<serde_json::Value>(text).is_ok() {
-        return Ok(text.to_string());
-    }
-
-    // Look for JSON array in the text
-    if let Some(start) = text.find('[') {
-        if let Some(end) = text.rfind(']') {
-            if end > start {
-                let json_str = &text[start..=end];
-                if serde_json::from_str::<serde_json::Value>(json_str).is_ok() {
-                    return Ok(json_str.to_string());
+    let mut near_miss: Option<serde_json::Error> = None;
+
+    for (start, _) in response.rmatch_indices('[') {
+        let candidate = &response[start..];
+        let mut stream =
+            serde_json::Deserializer::from_str(candidate).into_iter::<Vec<SuggestedAction>>();
+        match stream.next() {
+            Some(Ok(actions)) => return Ok(actions),
+            Some(Err(shape_err)) if near_miss.is_none() => {
+                let mut untyped = serde_json::Deserializer::from_str(candidate)
+                    .into_iter::<Vec<serde_json::Value>>();
+                if let Some(Ok(values)) = untyped.next() {
+                    if values.iter().all(|v| v.is_object()) {
+                        near_miss = Some(shape_err);
+                    }
                 }
             }
+            _ => {}
         }
     }
 
-    Err(anyhow::anyhow!(
-        "Could not find valid JSON array in AI response. Response was: {}",
-        text
-    ))
+    match near_miss {
+        Some(shape_err) => Err(anyhow::anyhow!(
+            "Found a JSON array of objects in the AI response, but it does not match the expected action shape ({}). Response was: {}",
+            shape_err,
+            response
+        )),
+        None => Err(anyhow::anyhow!(
+            "Could not find valid JSON array in AI response. Response was: {}",
+            response
+        )),
+    }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
 
+    const TEST_ACTION: &str = r#"{"name": "Test", "command": "npm test", "actionType": "check", "autoCommit": false, "source": "package.json"}"#;
+
     #[test]
-    fn test_extract_json_array() {
-        let text = r#"Here are some actions:
-[
-  {"name": "Test", "command": "npm test", "actionType": "check", "autoCommit": false, "source": "package.json"}
-]
-That's all!"#;
+    fn parses_array_surrounded_by_prose() {
+        let text = format!("Here are some actions:\n[\n  {TEST_ACTION}\n]\nThat's all!");
+
+        let actions = parse_ai_response(&text).expect("array between prose should parse");
+
+        assert_eq!(actions.len(), 1);
+        assert_eq!(actions[0].name, "Test");
+    }
+
+    #[test]
+    fn parses_bare_array() {
+        let text = format!("[{TEST_ACTION}]");
+
+        let actions = parse_ai_response(&text).expect("bare array should parse");
+
+        assert_eq!(actions.len(), 1);
+    }
+
+    #[test]
+    fn parses_fenced_array_after_tool_transcript() {
+        // Regression: one-shot transcripts used to interleave `[Tool: …]` /
+        // `[Result: …]` markers, whose leading `[` swallowed the real array.
+        let text = format!(
+            "I'll explore the project structure.\n\
+             [Tool: Terminal]\n\
+             [Result: List top-level project files]\n\
+             ```json\n\
+             [{TEST_ACTION}]\n\
+             ```\n"
+        );
+
+        let actions = parse_ai_response(&text).expect("fenced array after markers should parse");
+
+        assert_eq!(actions.len(), 1);
+        assert_eq!(actions[0].command, "npm test");
+    }
+
+    #[test]
+    fn prefers_the_final_array_over_earlier_ones() {
+        let text = format!(
+            "Example of the shape I will return:\n\
+             [{{\"name\": \"Example\", \"command\": \"echo hi\", \"actionType\": \"run\", \"autoCommit\": false, \"source\": \"README.md\"}}]\n\
+             Here is the real answer:\n\
+             [{TEST_ACTION}]"
+        );
+
+        let actions = parse_ai_response(&text).expect("last valid array should win");
+
+        assert_eq!(actions.len(), 1);
+        assert_eq!(actions[0].name, "Test");
+    }
+
+    #[test]
+    fn skips_arrays_that_are_not_actions() {
+        let text = format!("Candidates: [\"justfile\", \"package.json\"]\n[{TEST_ACTION}]\n[1, 2]");
+
+        let actions = parse_ai_response(&text).expect("non-action arrays should be skipped");
+
+        assert_eq!(actions.len(), 1);
+        assert_eq!(actions[0].source, "package.json");
+    }
+
+    #[test]
+    fn errors_when_no_action_array_is_present() {
+        let err = parse_ai_response("I could not find any build files. [Tool: Terminal]")
+            .expect_err("responses without an action array should fail");
+
+        assert!(err.to_string().contains("Could not find valid JSON array"));
+    }
+
+    #[test]
+    fn surfaces_the_shape_error_for_near_miss_arrays() {
+        // One malformed actionType in an otherwise valid array: the serde
+        // error should reach the caller instead of the generic message.
+        let text = r#"[{"name": "Lint", "command": "just lint", "actionType": "lintfix", "autoCommit": false, "source": "justfile"}]"#;
+
+        let err = parse_ai_response(text).expect_err("near-miss arrays should fail");
+
+        let message = err.to_string();
+        assert!(message.contains("does not match the expected action shape"));
+        assert!(message.contains("lintfix"));
+    }
+
+    #[test]
+    fn near_miss_error_is_not_masked_by_trailing_junk_arrays() {
+        let text = r#"[{"name": "Lint", "command": "just lint", "actionType": "lintfix", "autoCommit": false, "source": "justfile"}]
+Exit codes seen: [1, 2]"#;
+
+        let err = parse_ai_response(text).expect_err("near-miss arrays should fail");
 
-        let result = extract_json_array(text);
-        assert!(result.is_ok());
+        assert!(err.to_string().contains("lintfix"));
     }
 
     #[test]
-    fn test_extract_json_array_clean() {
-        let text = r#"[{"name": "Test", "command": "npm test", "actionType": "check", "autoCommit": false, "source": "package.json"}]"#;
+    fn non_object_arrays_do_not_count_as_near_misses() {
+        let err = parse_ai_response(r#"Files found: ["justfile", "package.json"]"#)
+            .expect_err("responses without an action array should fail");
 
-        let result = extract_json_array(text);
-        assert!(result.is_ok());
+        assert!(err.to_string().contains("Could not find valid JSON array"));
     }
 }