feat: add update-work-item safe output (#65)

* feat: add update-work-item safe output

Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>

* feat: add tag-prefix guard to update-work-item configuration

Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>

* fix: address review feedback on update-work-item safe output

Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/6464dd4d-6192-48d9-b5fc-ac5bdbe709b7

* fix: skip excess update-work-item entries and reject semicolons in tags

Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/0199ff90-689d-4eb9-b34b-f38b36709424

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>

Author: Copilot

src/compile/common.rs

@@ -267,10 +267,7 @@ const DEFAULT_BASH_COMMANDS: &[&str] = &[
 
 /// Generate copilot CLI params from front matter configuration
 pub fn generate_copilot_params(front_matter: &FrontMatter) -> String {
-    let mut allowed_tools: Vec<String> = vec![
-        "github".to_string(),
-        "safeoutputs".to_string(),
-    ];
+    let mut allowed_tools: Vec<String> = vec!["github".to_string(), "safeoutputs".to_string()];
 
     // Edit tool: enabled by default, can be disabled with `edit: false`
     let edit_enabled = front_matter
@@ -515,10 +512,7 @@ pub fn generate_acquire_ado_token(service_connection: Option<&str>, variable_nam
             lines.push("    addSpnToEnvironment: true".to_string());
             lines.push("    inlineScript: |".to_string());
             lines.push("      ADO_TOKEN=$(az account get-access-token \\".to_string());
-            lines.push(format!(
-                "        --resource {} \\",
-                ADO_RESOURCE_ID
-            ));
+            lines.push(format!("        --resource {} \\", ADO_RESOURCE_ID));
             lines.push("        --query accessToken -o tsv)".to_string());
             lines.push(format!(
                 "      echo \"##vso[task.setvariable variable={variable_name};issecret=true]$ADO_TOKEN\""
@@ -534,10 +528,8 @@ pub fn generate_acquire_ado_token(service_connection: Option<&str>, variable_nam
 /// When not configured, omits ADO access tokens entirely.
 pub fn generate_copilot_ado_env(read_service_connection: Option<&str>) -> String {
     match read_service_connection {
-        Some(_) => {
-            "AZURE_DEVOPS_EXT_PAT: $(SC_READ_TOKEN)\nSYSTEM_ACCESSTOKEN: $(SC_READ_TOKEN)"
-                .to_string()
-        }
+        Some(_) => "AZURE_DEVOPS_EXT_PAT: $(SC_READ_TOKEN)\nSYSTEM_ACCESSTOKEN: $(SC_READ_TOKEN)"
+            .to_string(),
         None => String::new(),
     }
 }
@@ -553,7 +545,13 @@ pub fn generate_executor_ado_env(write_service_connection: Option<&str>) -> Stri
 }
 
 /// Safe-output names that require write access to ADO.
-const WRITE_REQUIRING_SAFE_OUTPUTS: &[&str] = &["create-pull-request", "create-work-item", "create-wiki-page", "update-wiki-page"];
+const WRITE_REQUIRING_SAFE_OUTPUTS: &[&str] = &[
+    "create-pull-request",
+    "create-work-item",
+    "update-work-item",
+    "create-wiki-page",
+    "update-wiki-page",
+];
 
 /// Validate that write-requiring safe-outputs have a write service connection configured.
 pub fn validate_write_permissions(front_matter: &FrontMatter) -> Result<()> {
@@ -760,7 +758,10 @@ mod tests {
     #[test]
     fn test_generate_pr_trigger_no_triggers_no_schedule() {
         let result = generate_pr_trigger(&None, false);
-        assert!(result.is_empty(), "Should be empty when no triggers configured");
+        assert!(
+            result.is_empty(),
+            "Should be empty when no triggers configured"
+        );
     }
 
     #[test]
@@ -804,7 +805,10 @@ mod tests {
     #[test]
     fn test_generate_ci_trigger_no_triggers_no_schedule() {
         let result = generate_ci_trigger(&None, false);
-        assert!(result.is_empty(), "Should be empty when no triggers configured");
+        assert!(
+            result.is_empty(),
+            "Should be empty when no triggers configured"
+        );
     }
 
     #[test]

src/execute.rs

@@ -10,8 +10,8 @@ use std::path::Path;
 
 use crate::ndjson::{self, SAFE_OUTPUT_FILENAME};
 use crate::tools::{
-    CreatePrResult, CreateWikiPageResult, CreateWorkItemResult, UpdateWikiPageResult,
-    ExecutionContext, ExecutionResult, Executor,
+    CreatePrResult, CreateWikiPageResult, CreateWorkItemResult, ExecutionContext, ExecutionResult,
+    Executor, UpdateWikiPageResult, UpdateWorkItemConfig, UpdateWorkItemResult,
 };
 
 // Re-export memory types for use by main.rs
@@ -87,6 +87,11 @@ pub async fn execute_safe_outputs(
         }
     }
 
+    // Fetch the update-work-item max once; used to skip excess entries without aborting the batch
+    let update_wi_config: UpdateWorkItemConfig = ctx.get_tool_config("update-work-item");
+    let max_update_wi = update_wi_config.max as usize;
+    let mut update_wi_executed: usize = 0;
+
     let mut results = Vec::new();
     for (i, entry) in entries.iter().enumerate() {
         let entry_json = serde_json::to_string(entry).unwrap_or_else(|_| "<invalid>".to_string());
@@ -97,6 +102,38 @@ pub async fn execute_safe_outputs(
             entry_json
         );
 
+        // Enforce update-work-item max: skip excess entries rather than aborting the whole batch
+        if entry.get("name").and_then(|n| n.as_str()) == Some("update-work-item") {
+            if update_wi_executed >= max_update_wi {
+                let wi_id = entry
+                    .get("id")
+                    .and_then(|v| v.as_u64())
+                    .map(|id| format!(" (work item #{})", id))
+                    .unwrap_or_default();
+                warn!(
+                    "[{}/{}] Skipping update-work-item{} entry: max ({}) already reached for this run",
+                    i + 1,
+                    entries.len(),
+                    wi_id,
+                    max_update_wi
+                );
+                let result = ExecutionResult::failure(format!(
+                    "Skipped{}: maximum update-work-item count ({}) already reached. \
+                     Increase 'max' in safe-outputs.update-work-item to allow more updates.",
+                    wi_id, max_update_wi
+                ));
+                println!(
+                    "[{}/{}] update-work-item - ✗ - {}",
+                    i + 1,
+                    entries.len(),
+                    result.message
+                );
+                results.push(result);
+                continue;
+            }
+            update_wi_executed += 1;
+        }
+
         match execute_safe_output(entry, ctx).await {
             Ok((tool_name, result)) => {
                 if result.success {
@@ -172,6 +209,13 @@ pub async fn execute_safe_output(
             );
             output.execute_sanitized(ctx).await?
         }
+        "update-work-item" => {
+            debug!("Parsing update-work-item payload");
+            let mut output: UpdateWorkItemResult = serde_json::from_value(entry.clone())
+                .map_err(|e| anyhow::anyhow!("Failed to parse update-work-item: {}", e))?;
+            debug!("update-work-item: id={}", output.id);
+            output.execute_sanitized(ctx).await?
+        }
         "create-pull-request" => {
             debug!("Parsing create-pull-request payload");
             let mut output: CreatePrResult = serde_json::from_value(entry.clone())
@@ -478,4 +522,67 @@ mod tests {
                 .contains("AZURE_DEVOPS_ORG_URL")
         );
     }
+
+    /// Excess update-work-item entries beyond `max` are skipped (failure result added) rather than
+    /// aborting the entire batch. Other tool entries must still execute.
+    #[tokio::test]
+    async fn test_execute_update_work_item_max_skips_excess_not_abort() {
+        let temp_dir = tempfile::tempdir().unwrap();
+        let safe_output_path = temp_dir.path().join(SAFE_OUTPUT_FILENAME);
+
+        // Write 3 update-work-item entries + 1 noop; max defaults to 1
+        let ndjson = r#"{"name":"update-work-item","id":1,"title":"First update"}
+{"name":"update-work-item","id":2,"title":"Second update"}
+{"name":"update-work-item","id":3,"title":"Third update"}
+{"name":"noop","context":"still runs"}
+"#;
+        tokio::fs::write(&safe_output_path, ndjson).await.unwrap();
+
+        // Config: update-work-item with max=1 (default), title=true so the field check passes
+        let update_cfg = serde_json::json!({
+            "title": true,
+            "max": 1
+        });
+        let mut tool_configs = HashMap::new();
+        tool_configs.insert("update-work-item".to_string(), update_cfg);
+
+        let ctx = ExecutionContext {
+            ado_org_url: Some("https://dev.azure.com/org".to_string()),
+            ado_organization: Some("org".to_string()),
+            ado_project: Some("Proj".to_string()),
+            access_token: Some("token".to_string()),
+            working_directory: PathBuf::from("."),
+            source_directory: PathBuf::from("."),
+            tool_configs,
+            repository_id: None,
+            repository_name: None,
+            allowed_repositories: HashMap::new(),
+        };
+
+        let results = execute_safe_outputs(temp_dir.path(), &ctx).await;
+        // The batch must NOT abort — execute_safe_outputs should return Ok
+        assert!(
+            results.is_ok(),
+            "Batch should not abort when max is exceeded; got: {:?}",
+            results
+        );
+        let results = results.unwrap();
+        // 4 entries total: 3 update-work-item + 1 noop
+        assert_eq!(results.len(), 4, "Expected 4 results (3 uwi + 1 noop)");
+
+        // The first update-work-item fails with HTTP error (no real ADO) but was attempted
+        // The 2nd and 3rd are skipped due to max
+        let skipped: Vec<_> = results
+            .iter()
+            .filter(|r| r.message.contains("maximum update-work-item count"))
+            .collect();
+        assert_eq!(skipped.len(), 2, "Expected 2 skipped entries, got: {:?}", skipped);
+
+        // The noop still executes successfully
+        let noop_result = &results[3];
+        assert!(
+            noop_result.success,
+            "noop should still succeed even when prior entries are skipped"
+        );
+    }
 }

src/mcp.rs

@@ -15,6 +15,7 @@ use crate::tools::{
     CreateWorkItemParams, CreateWorkItemResult,
     UpdateWikiPageParams, UpdateWikiPageResult, MissingDataParams, MissingDataResult,
     MissingToolParams, MissingToolResult, NoopParams, NoopResult, ToolResult,
+    UpdateWorkItemParams, UpdateWorkItemResult,
     anyhow_to_mcp_error,
 };
 
@@ -335,6 +336,39 @@ impl SafeOutputs {
         Ok(CallToolResult::success(vec![]))
     }
 
+    #[tool(
+        name = "update-work-item",
+        description = "Update an existing Azure DevOps work item. Only fields explicitly enabled \
+in the pipeline configuration (safe-outputs.update-work-item) may be changed. Updates may be \
+further restricted by target (only a specific work item ID) or title-prefix (only work items \
+whose current title starts with a configured prefix). Provide the work item ID and only the \
+fields you want to update."
+    )]
+    async fn update_work_item(
+        &self,
+        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;
+        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.",
+            result.id
+        ))]))
+    }
+
     #[tool(
         name = "create-pull-request",
         description = "Create a new pull request to propose code changes. Use this after making file edits to submit them for review and merging. The PR will be created from the current branch with your committed changes. Use 'self' for the pipeline's own repository, or a repository alias from the checkout list."

src/tools/mod.rs

@@ -18,6 +18,7 @@ mod missing_data;
 mod missing_tool;
 mod noop;
 mod result;
+mod update_work_item;
 
 pub use create_pr::*;
 pub use create_wiki_page::*;
@@ -29,3 +30,4 @@ pub use noop::*;
 pub use result::{
     ExecutionContext, ExecutionResult, Executor, ToolResult, Validate, anyhow_to_mcp_error,
 };
+pub use update_work_item::*;