fix: propagate write errors and harden comment-on-work-item

- update_work_item MCP handler: propagate write_safe_output_file
  errors instead of silently discarding them with let _ =
- update_work_item MCP handler: remove redundant pre-sanitization;
  the Sanitize trait impl handles this in Stage 2 via
  execute_sanitized, matching the pattern other tools follow
- comment-on-work-item: use case-insensitive area path prefix
  matching since ADO area paths are case-insensitive
- Add integration tests for comment-on-work-item compile-time
  validation: requires target field, requires write SC, and
  succeeds with both

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: jamesadevine

src/mcp.rs

@@ -357,7 +357,7 @@ pipeline configuration."
         let mut sanitized = params.0;
         sanitized.body = sanitize_text(&sanitized.body);
         let result: CommentOnWorkItemResult = sanitized.try_into()?;
-        let _ = self.write_safe_output_file(&result).await
+        self.write_safe_output_file(&result).await
             .map_err(|e| anyhow_to_mcp_error(anyhow::anyhow!("Failed to write safe output: {}", e)))?;
         info!("Comment queued for work item #{}", result.work_item_id);
         Ok(CallToolResult::success(vec![Content::text(format!(
@@ -379,19 +379,9 @@ fields you want to update."
         params: Parameters<UpdateWorkItemParams>,
     ) -> Result<CallToolResult, McpError> {
         info!("Tool called: update-work-item - id={}", params.0.id);
-        // Sanitize untrusted agent-provided text fields (IS-01)
-        let mut sanitized = params.0;
-        sanitized.title = sanitized.title.map(|t| sanitize_text(&t));
-        sanitized.body = sanitized.body.map(|b| sanitize_text(&b));
-        sanitized.state = sanitized.state.map(|s| sanitize_text(&s));
-        sanitized.area_path = sanitized.area_path.map(|p| sanitize_text(&p));
-        sanitized.iteration_path = sanitized.iteration_path.map(|p| sanitize_text(&p));
-        sanitized.assignee = sanitized.assignee.map(|a| sanitize_text(&a));
-        sanitized.tags = sanitized
-            .tags
-            .map(|ts| ts.into_iter().map(|t| sanitize_text(&t)).collect());
-        let result: UpdateWorkItemResult = sanitized.try_into()?;
-        let _ = self.write_safe_output_file(&result).await;
+        let result: UpdateWorkItemResult = params.0.try_into()?;
+        self.write_safe_output_file(&result).await
+            .map_err(|e| anyhow_to_mcp_error(anyhow::anyhow!("Failed to write safe output: {}", e)))?;
         info!("Work item update queued for #{}", result.id);
         Ok(CallToolResult::success(vec![Content::text(format!(
             "Work item #{} update queued. Changes will be applied during safe output processing.",

src/tools/comment_on_work_item.rs

@@ -232,7 +232,8 @@ impl Executor for CommentOnWorkItemResult {
                     .await
                 {
                     Ok(area_path) => {
-                        if !area_path.starts_with(prefix) {
+                        // ADO area paths are case-insensitive
+                        if !area_path.to_lowercase().starts_with(&prefix.to_lowercase()) {
                             return Ok(ExecutionResult::failure(format!(
                                 "Work item #{} has area path '{}' which is not under allowed prefix '{}'",
                                 self.work_item_id, area_path, prefix

tests/compiler_tests.rs

@@ -1168,3 +1168,153 @@ Update existing work items.
 
     let _ = fs::remove_dir_all(&temp_dir);
 }
+
+/// Test that comment-on-work-item requires a target field
+#[test]
+fn test_comment_on_work_item_requires_target_field() {
+    let temp_dir = std::env::temp_dir().join(format!(
+        "agentic-pipeline-cwi-target-{}",
+        std::process::id()
+    ));
+    fs::create_dir_all(&temp_dir).expect("Failed to create temp directory");
+
+    let test_input = temp_dir.join("cwi-agent.md");
+    let test_content = r#"---
+name: "Comment Agent"
+description: "Agent that comments on work items but has no target"
+permissions:
+  write: my-write-sc
+safe-outputs:
+  comment-on-work-item:
+    max: 3
+---
+
+## Comment Agent
+
+Comment on work items.
+"#;
+    fs::write(&test_input, test_content).expect("Failed to write test input");
+
+    let output_path = temp_dir.join("cwi-agent.yml");
+    let binary_path = PathBuf::from(env!("CARGO_BIN_EXE_ado-aw"));
+    let output = std::process::Command::new(&binary_path)
+        .args([
+            "compile",
+            test_input.to_str().unwrap(),
+            "-o",
+            output_path.to_str().unwrap(),
+        ])
+        .output()
+        .expect("Failed to run compiler");
+
+    assert!(
+        !output.status.success(),
+        "Compiler should fail when comment-on-work-item lacks a target field"
+    );
+
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        stderr.contains("target"),
+        "Error message should mention target: {stderr}"
+    );
+
+    let _ = fs::remove_dir_all(&temp_dir);
+}
+
+/// Test that comment-on-work-item requires a write service connection
+#[test]
+fn test_comment_on_work_item_requires_write_sc() {
+    let temp_dir = std::env::temp_dir().join(format!(
+        "agentic-pipeline-cwi-sc-{}",
+        std::process::id()
+    ));
+    fs::create_dir_all(&temp_dir).expect("Failed to create temp directory");
+
+    let test_input = temp_dir.join("cwi-agent.md");
+    let test_content = r#"---
+name: "Comment Agent"
+description: "Agent that comments on work items but has no write SC"
+safe-outputs:
+  comment-on-work-item:
+    target: "*"
+---
+
+## Comment Agent
+
+Comment on work items.
+"#;
+    fs::write(&test_input, test_content).expect("Failed to write test input");
+
+    let output_path = temp_dir.join("cwi-agent.yml");
+    let binary_path = PathBuf::from(env!("CARGO_BIN_EXE_ado-aw"));
+    let output = std::process::Command::new(&binary_path)
+        .args([
+            "compile",
+            test_input.to_str().unwrap(),
+            "-o",
+            output_path.to_str().unwrap(),
+        ])
+        .output()
+        .expect("Failed to run compiler");
+
+    assert!(
+        !output.status.success(),
+        "Compiler should fail when comment-on-work-item lacks a write SC"
+    );
+
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        stderr.contains("permissions.write"),
+        "Error message should mention permissions.write: {stderr}"
+    );
+
+    let _ = fs::remove_dir_all(&temp_dir);
+}
+
+/// Test that comment-on-work-item compiles successfully with proper config
+#[test]
+fn test_comment_on_work_item_compiles_with_target_and_write_sc() {
+    let temp_dir = std::env::temp_dir().join(format!(
+        "agentic-pipeline-cwi-pass-{}",
+        std::process::id()
+    ));
+    fs::create_dir_all(&temp_dir).expect("Failed to create temp directory");
+
+    let test_input = temp_dir.join("cwi-agent.md");
+    let test_content = r#"---
+name: "Comment Agent"
+description: "Agent that comments on work items"
+permissions:
+  write: my-write-sc
+safe-outputs:
+  comment-on-work-item:
+    target: "*"
+    max: 5
+---
+
+## Comment Agent
+
+Comment on work items.
+"#;
+    fs::write(&test_input, test_content).expect("Failed to write test input");
+
+    let output_path = temp_dir.join("cwi-agent.yml");
+    let binary_path = PathBuf::from(env!("CARGO_BIN_EXE_ado-aw"));
+    let output = std::process::Command::new(&binary_path)
+        .args([
+            "compile",
+            test_input.to_str().unwrap(),
+            "-o",
+            output_path.to_str().unwrap(),
+        ])
+        .output()
+        .expect("Failed to run compiler");
+
+    assert!(
+        output.status.success(),
+        "Compiler should succeed when target and write SC are provided: {}",
+        String::from_utf8_lossy(&output.stderr)
+    );
+
+    let _ = fs::remove_dir_all(&temp_dir);
+}