From 08a17799f7acb37e957412d2649beedc251a2cf8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 12 May 2026 17:43:38 +0000 Subject: [PATCH 1/6] feat(safeoutputs): add work-item config for noop and missing-tool safe outputs Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/f7059573-7767-4cc5-b10c-53fac12aa232 Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> --- docs/safe-outputs.md | 32 ++++ src/safeoutputs/missing_tool.rs | 111 +++++++++++- src/safeoutputs/mod.rs | 311 ++++++++++++++++++++++++++++++++ src/safeoutputs/noop.rs | 111 +++++++++++- 4 files changed, 557 insertions(+), 8 deletions(-) diff --git a/docs/safe-outputs.md b/docs/safe-outputs.md index e7a76345..df9e44b6 100644 --- a/docs/safe-outputs.md +++ b/docs/safe-outputs.md @@ -202,6 +202,22 @@ Reports that no action was needed. Use this to provide visibility when analysis **Agent parameters:** - `context` - Optional context about why no action was taken +**Configuration options (front matter):** +```yaml +safe-outputs: + noop: + work-item: # Optional — file or append to a work item when noop is reached + title: "Agent reported no operation" # Required — work item title (used to find existing items) + work-item-type: Task # Work item type (default: "Task") + area-path: "MyProject\\MyTeam" # Optional — area path + iteration-path: "MyProject\\Sprint 1" # Optional — iteration path + tags: # Optional — tags to apply + - agent-noop + include-stats: true # Append agent stats to description/comment (default: true) +``` + +When `work-item` is configured, the executor searches for a non-closed work item with the same `title` in the project. If one is found, a comment is appended; otherwise a new work item is created. + ### missing-data Reports that data or information needed to complete the task is not available. @@ -217,6 +233,22 @@ Reports that a tool or capability needed to complete the task is not available. - `tool_name` - Name of the tool that was expected but not found - `context` - Optional context about why the tool was needed +**Configuration options (front matter):** +```yaml +safe-outputs: + missing-tool: + work-item: # Optional — file or append to a work item when a tool is missing + title: "Agent encountered missing tool" # Required — work item title (used to find existing items) + work-item-type: Bug # Work item type (default: "Task") + area-path: "MyProject\\MyTeam" # Optional — area path + iteration-path: "MyProject\\Sprint 1" # Optional — iteration path + tags: # Optional — tags to apply + - agent-missing-tool + include-stats: true # Append agent stats to description/comment (default: true) +``` + +When `work-item` is configured, the executor searches for a non-closed work item with the same `title` in the project. If one is found, a comment is appended; otherwise a new work item is created. + ### report-incomplete Reports that a task could not be completed. diff --git a/src/safeoutputs/missing_tool.rs b/src/safeoutputs/missing_tool.rs index 81671786..9fdb14ad 100644 --- a/src/safeoutputs/missing_tool.rs +++ b/src/safeoutputs/missing_tool.rs @@ -1,11 +1,11 @@ //! Missing tool reporting schemas use schemars::JsonSchema; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; -use crate::sanitize::{SanitizeContent, sanitize as sanitize_text}; +use crate::sanitize::{SanitizeConfig, SanitizeContent, sanitize as sanitize_text}; use crate::tool_result; -use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate, WorkItemReportConfig, file_or_append_work_item}; /// Parameters for reporting a missing tool #[derive(Deserialize, JsonSchema)] @@ -37,17 +37,56 @@ impl SanitizeContent for MissingToolResult { } } +/// Configuration for the missing-tool tool (specified in front matter). +/// +/// When `work-item` is configured, the executor will file a new Azure DevOps work item +/// or append a comment to an existing one with the same title. +/// +/// Example front matter: +/// ```yaml +/// safe-outputs: +/// missing-tool: +/// work-item: +/// title: "Agent encountered missing tool" +/// work-item-type: Bug +/// area-path: "MyProject\\MyTeam" +/// tags: +/// - agent-missing-tool +/// ``` +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct MissingToolConfig { + /// Optional work item to file (or append to) when a tool is reported missing. + /// If absent, the executor only logs a message. + #[serde(default, rename = "work-item")] + pub work_item: Option, +} + +impl SanitizeConfig for MissingToolConfig { + fn sanitize_config_fields(&mut self) { + if let Some(wi) = &mut self.work_item { + wi.sanitize_config_fields(); + } + } +} + #[async_trait::async_trait] impl Executor for MissingToolResult { fn dry_run_summary(&self) -> String { format!("report missing tool '{}'", self.tool_name) } - async fn execute_impl(&self, _: &ExecutionContext) -> anyhow::Result { + async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result { let message = match &self.context { Some(context) => format!("Missing tool reported: {} ({context})", self.tool_name), None => format!("Missing tool reported: {}", self.tool_name), }; + + let config: MissingToolConfig = ctx.get_tool_config("missing-tool"); + + if let Some(wi_config) = &config.work_item { + return file_or_append_work_item(wi_config, &message, ctx).await; + } + Ok(ExecutionResult::success(message)) } } @@ -102,4 +141,68 @@ mod tests { let result: Result = serde_json::from_str(json); assert!(result.is_err()); } + + #[test] + fn test_config_defaults_to_no_work_item() { + let config = MissingToolConfig::default(); + assert!(config.work_item.is_none()); + } + + #[test] + fn test_config_deserializes_with_work_item() { + let yaml = r#" +work-item: + title: "Agent encountered missing tool" + work-item-type: Bug + area-path: "MyProject\\MyTeam" + tags: + - agent-missing-tool +"#; + let config: MissingToolConfig = serde_yaml::from_str(yaml).unwrap(); + let wi = config.work_item.unwrap(); + assert_eq!(wi.title, "Agent encountered missing tool"); + assert_eq!(wi.work_item_type, "Bug"); + assert_eq!(wi.area_path.as_deref(), Some("MyProject\\MyTeam")); + assert_eq!(wi.tags, vec!["agent-missing-tool"]); + } + + #[test] + fn test_config_deserializes_without_work_item() { + let yaml = r#"{}"#; + let config: MissingToolConfig = serde_yaml::from_str(yaml).unwrap(); + assert!(config.work_item.is_none()); + } + + #[test] + fn test_work_item_config_default_type() { + let yaml = r#" +work-item: + title: "Missing tool" +"#; + let config: MissingToolConfig = serde_yaml::from_str(yaml).unwrap(); + let wi = config.work_item.unwrap(); + assert_eq!(wi.work_item_type, "Task"); + assert!(wi.area_path.is_none()); + assert!(wi.iteration_path.is_none()); + assert!(wi.tags.is_empty()); + assert!(wi.include_stats); + } + + #[tokio::test] + async fn test_execute_impl_without_work_item_config() { + let result: MissingToolResult = MissingToolParams { + tool_name: "bash".to_string(), + context: Some("needed for script execution".to_string()), + } + .try_into() + .unwrap(); + + let exec = result + .execute_impl(&crate::safeoutputs::ExecutionContext::default()) + .await + .unwrap(); + assert!(exec.success); + assert!(exec.message.contains("Missing tool reported")); + assert!(exec.message.contains("bash")); + } } diff --git a/src/safeoutputs/mod.rs b/src/safeoutputs/mod.rs index fba1fa72..ee202677 100644 --- a/src/safeoutputs/mod.rs +++ b/src/safeoutputs/mod.rs @@ -1,8 +1,11 @@ //! Tool parameter and result structs for MCP tools use crate::{all_safe_output_names, tool_names}; +use anyhow::Context; use log::{debug, warn}; use percent_encoding::{AsciiSet, CONTROLS, utf8_percent_encode}; +use serde::{Deserialize, Serialize}; +use ado_aw_derive::SanitizeConfig; /// Characters to percent-encode in a URL path segment. /// Encodes the structural delimiters that would break URL parsing if left raw: @@ -375,6 +378,314 @@ pub(crate) fn validate_git_ref_name(name: &str, label: &str) -> anyhow::Result<( Ok(()) } +fn work_item_report_default_type() -> String { + "Task".to_string() +} + +/// Configuration for optionally filing (or appending to) an Azure DevOps work item +/// when a diagnostic safe output (`noop`, `missing-tool`) is called. +/// +/// If a work item with the same title already exists in the project in a non-closed +/// state, a comment is appended instead of creating a new work item. +/// +/// Example: +/// ```yaml +/// safe-outputs: +/// noop: +/// work-item: +/// title: "Agent reported no operation" +/// work-item-type: Task +/// area-path: "MyProject\\MyTeam" +/// tags: +/// - agent-noop +/// ``` +#[derive(Debug, Clone, SanitizeConfig, Serialize, Deserialize)] +pub struct WorkItemReportConfig { + /// Title of the work item to file or append a comment to. + /// If a non-closed work item with this exact title already exists, + /// a comment is appended rather than creating a new work item. + pub title: String, + + /// Work item type to create (default: "Task") + #[serde(default = "work_item_report_default_type", rename = "work-item-type")] + pub work_item_type: String, + + /// Area path for the work item + #[serde(default, rename = "area-path")] + pub area_path: Option, + + /// Iteration path for the work item + #[serde(default, rename = "iteration-path")] + pub iteration_path: Option, + + /// Tags to apply to the work item + #[serde(default)] + pub tags: Vec, + + /// Whether to include agent execution stats in the work item description/comment (default: true) + #[serde(default = "crate::agent_stats::default_include_stats", rename = "include-stats")] + pub include_stats: bool, +} + +impl Default for WorkItemReportConfig { + fn default() -> Self { + Self { + title: String::new(), + work_item_type: work_item_report_default_type(), + area_path: None, + iteration_path: None, + tags: Vec::new(), + include_stats: true, + } + } +} + +/// Search for a non-closed work item by exact title using WIQL. +/// Returns the ID of the most-recently-changed matching work item, or `None` if none found. +async fn wiql_find_work_item_by_title( + client: &reqwest::Client, + org_url: &str, + project: &str, + token: &str, + title: &str, +) -> anyhow::Result> { + // Escape single quotes in WIQL by doubling them + let escaped_title = title.replace('\'', "''"); + let query = format!( + "SELECT [System.Id] FROM WorkItems \ + WHERE [System.Title] = '{escaped_title}' \ + AND [System.TeamProject] = @project \ + AND [System.State] NOT IN ('Closed', 'Resolved', 'Done') \ + ORDER BY [System.ChangedDate] DESC" + ); + + let url = format!( + "{}/{}/_apis/wit/wiql?api-version=7.0", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + ); + + debug!("WIQL search URL: {}", url); + let body = serde_json::json!({ "query": query }); + + let response = client + .post(&url) + .header("Content-Type", "application/json") + .basic_auth("", Some(token)) + .json(&body) + .send() + .await + .context("Failed to query work items via WIQL")?; + + if !response.status().is_success() { + let status = response.status(); + let error_body = response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + anyhow::bail!("WIQL query failed (HTTP {}): {}", status, error_body); + } + + let result: serde_json::Value = response + .json() + .await + .context("Failed to parse WIQL response")?; + + let first_id = result + .get("workItems") + .and_then(|v| v.as_array()) + .and_then(|arr| arr.first()) + .and_then(|item| item.get("id")) + .and_then(|id| id.as_i64()); + + debug!("WIQL search found work item id: {:?}", first_id); + Ok(first_id) +} + +/// File a new work item or append a comment to an existing one with the same title. +/// +/// If a non-closed work item matching `config.title` exists in the project, +/// a comment with `body` is appended. Otherwise a new work item is created +/// with `body` as the description. +/// +/// Returns an [`ExecutionResult`] describing what was done. +pub(crate) async fn file_or_append_work_item( + config: &WorkItemReportConfig, + body: &str, + ctx: &ExecutionContext, +) -> anyhow::Result { + let org_url = match &ctx.ado_org_url { + Some(u) => u, + None => { + return Ok(ExecutionResult::failure( + "AZURE_DEVOPS_ORG_URL not set; cannot file work item".to_string(), + )); + } + }; + let project = match &ctx.ado_project { + Some(p) => p, + None => { + return Ok(ExecutionResult::failure( + "SYSTEM_TEAMPROJECT not set; cannot file work item".to_string(), + )); + } + }; + let token = match &ctx.access_token { + Some(t) => t, + None => { + return Ok(ExecutionResult::failure( + "No access token available (SYSTEM_ACCESSTOKEN or AZURE_DEVOPS_EXT_PAT); \ + cannot file work item" + .to_string(), + )); + } + }; + + let client = reqwest::Client::new(); + + // Search for an existing non-closed work item with the same title + let existing_id = + match wiql_find_work_item_by_title(&client, org_url, project, token, &config.title).await { + Ok(id) => id, + Err(e) => { + warn!("WIQL search for existing work item failed: {e} — aborting work item filing"); + return Ok(ExecutionResult::failure(format!( + "Failed to search for existing work item: {e}" + ))); + } + }; + + let body_with_stats = + crate::agent_stats::append_stats_to_body(body, ctx, config.include_stats); + + if let Some(work_item_id) = existing_id { + // Append a comment to the existing work item + debug!( + "Found existing work item #{}, appending comment", + work_item_id + ); + let comment_payload = serde_json::json!({ "text": body_with_stats }); + + let url = format!( + "{}/{}/_apis/wit/workItems/{}/comments?api-version=7.1-preview.4", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + work_item_id, + ); + + let resp = client + .post(&url) + .header("Content-Type", "application/json") + .basic_auth("", Some(token)) + .json(&comment_payload) + .send() + .await + .context("Failed to add comment to work item")?; + + if resp.status().is_success() { + let resp_body: serde_json::Value = resp + .json() + .await + .context("Failed to parse comment response")?; + let comment_id = resp_body.get("id").and_then(|v| v.as_i64()).unwrap_or(0); + Ok(ExecutionResult::success_with_data( + format!( + "Appended comment #{} to existing work item #{}: {}", + comment_id, work_item_id, config.title + ), + serde_json::json!({ + "action": "appended", + "work_item_id": work_item_id, + "comment_id": comment_id, + }), + )) + } else { + let status = resp.status(); + let error_body = resp + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + Ok(ExecutionResult::failure(format!( + "Failed to append comment to work item #{} (HTTP {}): {}", + work_item_id, status, error_body + ))) + } + } else { + // Create a new work item + debug!("No existing work item found, creating new one"); + + let mut patch_doc = vec![ + serde_json::json!({"op": "add", "path": "/fields/System.Title", "value": &config.title}), + serde_json::json!({"op": "add", "path": "/fields/System.Description", "value": body_with_stats}), + serde_json::json!({"op": "add", "path": "/multilineFieldsFormat/System.Description", "value": "Markdown"}), + ]; + + if let Some(area_path) = &config.area_path { + patch_doc.push( + serde_json::json!({"op": "add", "path": "/fields/System.AreaPath", "value": area_path}), + ); + } + if let Some(iteration_path) = &config.iteration_path { + patch_doc.push( + serde_json::json!({"op": "add", "path": "/fields/System.IterationPath", "value": iteration_path}), + ); + } + if !config.tags.is_empty() { + patch_doc.push( + serde_json::json!({"op": "add", "path": "/fields/System.Tags", "value": config.tags.join("; ")}), + ); + } + + let url = format!( + "{}/{}/_apis/wit/workitems/${}?api-version=7.0", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + utf8_percent_encode(&config.work_item_type, PATH_SEGMENT), + ); + + let resp = client + .post(&url) + .header("Content-Type", "application/json-patch+json") + .basic_auth("", Some(token)) + .json(&patch_doc) + .send() + .await + .context("Failed to create work item")?; + + if resp.status().is_success() { + let resp_body: serde_json::Value = resp + .json() + .await + .context("Failed to parse work item response")?; + let work_item_id = resp_body.get("id").and_then(|v| v.as_i64()).unwrap_or(0); + let work_item_url = resp_body + .get("_links") + .and_then(|l| l.get("html")) + .and_then(|h| h.get("href")) + .and_then(|h| h.as_str()) + .unwrap_or(""); + Ok(ExecutionResult::success_with_data( + format!("Created work item #{}: {}", work_item_id, config.title), + serde_json::json!({ + "action": "created", + "work_item_id": work_item_id, + "url": work_item_url, + }), + )) + } else { + let status = resp.status(); + let error_body = resp + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + Ok(ExecutionResult::failure(format!( + "Failed to create work item (HTTP {}): {}", + status, error_body + ))) + } + } +} + mod add_build_tag; mod add_pr_comment; mod comment_on_work_item; diff --git a/src/safeoutputs/noop.rs b/src/safeoutputs/noop.rs index ee02c27c..f4953388 100644 --- a/src/safeoutputs/noop.rs +++ b/src/safeoutputs/noop.rs @@ -1,9 +1,9 @@ use schemars::JsonSchema; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; -use crate::sanitize::{SanitizeContent, sanitize as sanitize_text}; +use crate::sanitize::{SanitizeConfig, SanitizeContent, sanitize as sanitize_text}; use crate::tool_result; -use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate, WorkItemReportConfig, file_or_append_work_item}; /// Parameters for describing a no operation. Use this if there is no work to do. #[derive(Deserialize, JsonSchema)] @@ -31,17 +31,57 @@ impl SanitizeContent for NoopResult { } } +/// Configuration for the noop tool (specified in front matter). +/// +/// When `work-item` is configured, the executor will file a new Azure DevOps work item +/// or append a comment to an existing one with the same title. +/// +/// Example front matter: +/// ```yaml +/// safe-outputs: +/// noop: +/// work-item: +/// title: "Agent reported no operation" +/// work-item-type: Task +/// area-path: "MyProject\\MyTeam" +/// iteration-path: "MyProject\\Sprint 1" +/// tags: +/// - agent-noop +/// ``` +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct NoopConfig { + /// Optional work item to file (or append to) when a noop is reached. + /// If absent, the executor only logs a message. + #[serde(default, rename = "work-item")] + pub work_item: Option, +} + +impl SanitizeConfig for NoopConfig { + fn sanitize_config_fields(&mut self) { + if let Some(wi) = &mut self.work_item { + wi.sanitize_config_fields(); + } + } +} + #[async_trait::async_trait] impl Executor for NoopResult { fn dry_run_summary(&self) -> String { "noop".to_string() } - async fn execute_impl(&self, _: &ExecutionContext) -> anyhow::Result { + async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result { let message = match &self.context { Some(context) => format!("No operation needed: {context}"), None => "No operation needed".to_string(), }; + + let config: NoopConfig = ctx.get_tool_config("noop"); + + if let Some(wi_config) = &config.work_item { + return file_or_append_work_item(wi_config, &message, ctx).await; + } + Ok(ExecutionResult::success(message)) } } @@ -112,4 +152,67 @@ mod tests { let params = NoopParams { context: None }; assert!(params.validate().is_ok()); } + + #[test] + fn test_config_defaults_to_no_work_item() { + let config = NoopConfig::default(); + assert!(config.work_item.is_none()); + } + + #[test] + fn test_config_deserializes_with_work_item() { + let yaml = r#" +work-item: + title: "Agent reported no operation" + work-item-type: Task + area-path: "MyProject\\MyTeam" + tags: + - agent-noop +"#; + let config: NoopConfig = serde_yaml::from_str(yaml).unwrap(); + let wi = config.work_item.unwrap(); + assert_eq!(wi.title, "Agent reported no operation"); + assert_eq!(wi.work_item_type, "Task"); + assert_eq!(wi.area_path.as_deref(), Some("MyProject\\MyTeam")); + assert_eq!(wi.tags, vec!["agent-noop"]); + } + + #[test] + fn test_config_deserializes_without_work_item() { + let yaml = r#"{}"#; + let config: NoopConfig = serde_yaml::from_str(yaml).unwrap(); + assert!(config.work_item.is_none()); + } + + #[test] + fn test_work_item_config_default_type() { + let yaml = r#" +work-item: + title: "No op" +"#; + let config: NoopConfig = serde_yaml::from_str(yaml).unwrap(); + let wi = config.work_item.unwrap(); + assert_eq!(wi.work_item_type, "Task"); + assert!(wi.area_path.is_none()); + assert!(wi.iteration_path.is_none()); + assert!(wi.tags.is_empty()); + assert!(wi.include_stats); + } + + #[tokio::test] + async fn test_execute_impl_without_work_item_config() { + let result: NoopResult = NoopParams { + context: Some("nothing to do".to_string()), + } + .try_into() + .unwrap(); + + let exec = result + .execute_impl(&crate::safeoutputs::ExecutionContext::default()) + .await + .unwrap(); + assert!(exec.success); + assert!(exec.message.contains("No operation needed")); + assert!(exec.message.contains("nothing to do")); + } } From 0119635267360e57f9e9d6a938b7472f650c555a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 12 May 2026 17:48:40 +0000 Subject: [PATCH 2/6] fix(safeoutputs): address code review - document WIQL escaping, fix ID handling Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/f7059573-7767-4cc5-b10c-53fac12aa232 Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> --- src/safeoutputs/mod.rs | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/src/safeoutputs/mod.rs b/src/safeoutputs/mod.rs index ee202677..e89d500d 100644 --- a/src/safeoutputs/mod.rs +++ b/src/safeoutputs/mod.rs @@ -449,8 +449,16 @@ async fn wiql_find_work_item_by_title( token: &str, title: &str, ) -> anyhow::Result> { - // Escape single quotes in WIQL by doubling them + // The WIQL API does not support parameterized queries; string literals must be + // manually escaped. Doubling single quotes is the standard WIQL escaping convention + // (analogous to SQL). This title value comes from operator-controlled front-matter + // configuration and is sanitized via `sanitize_config_fields()` before reaching + // here, so it is not agent-supplied content. No other characters are WIQL-special + // inside a single-quoted literal. let escaped_title = title.replace('\'', "''"); + // The state filter covers the three built-in ADO process templates: + // Agile: "Closed", Scrum: "Done", CMMI: "Closed" (also "Resolved" in Agile/CMMI). + // Work items in any other state are considered active and eligible for commenting. let query = format!( "SELECT [System.Id] FROM WorkItems \ WHERE [System.Title] = '{escaped_title}' \ @@ -587,12 +595,19 @@ pub(crate) async fn file_or_append_work_item( .json() .await .context("Failed to parse comment response")?; - let comment_id = resp_body.get("id").and_then(|v| v.as_i64()).unwrap_or(0); - Ok(ExecutionResult::success_with_data( - format!( + let comment_id = resp_body.get("id").and_then(|v| v.as_i64()); + let message = match comment_id { + Some(id) => format!( "Appended comment #{} to existing work item #{}: {}", - comment_id, work_item_id, config.title + id, work_item_id, config.title + ), + None => format!( + "Appended comment to existing work item #{}: {}", + work_item_id, config.title ), + }; + Ok(ExecutionResult::success_with_data( + message, serde_json::json!({ "action": "appended", "work_item_id": work_item_id, @@ -657,15 +672,20 @@ pub(crate) async fn file_or_append_work_item( .json() .await .context("Failed to parse work item response")?; - let work_item_id = resp_body.get("id").and_then(|v| v.as_i64()).unwrap_or(0); + let work_item_id = resp_body.get("id").and_then(|v| v.as_i64()); let work_item_url = resp_body .get("_links") .and_then(|l| l.get("html")) .and_then(|h| h.get("href")) .and_then(|h| h.as_str()) - .unwrap_or(""); + .unwrap_or("") + .to_string(); + let message = match work_item_id { + Some(id) => format!("Created work item #{}: {}", id, config.title), + None => format!("Created work item: {}", config.title), + }; Ok(ExecutionResult::success_with_data( - format!("Created work item #{}: {}", work_item_id, config.title), + message, serde_json::json!({ "action": "created", "work_item_id": work_item_id, From d0257b8c9133ac5ace794a369680b5665af5b98d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 12 May 2026 20:00:07 +0000 Subject: [PATCH 3/6] feat(safeoutputs): make noop and missing-tool always file/append work items with sensible defaults Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/e309e3af-fbce-4cfe-91d8-420be5233943 Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> --- docs/safe-outputs.md | 36 +++++++----- src/execute.rs | 5 +- src/safeoutputs/missing_tool.rs | 99 +++++++++++++++++-------------- src/safeoutputs/mod.rs | 41 ++++++------- src/safeoutputs/noop.rs | 101 +++++++++++++++++--------------- 5 files changed, 151 insertions(+), 131 deletions(-) diff --git a/docs/safe-outputs.md b/docs/safe-outputs.md index df9e44b6..d7bb5d5d 100644 --- a/docs/safe-outputs.md +++ b/docs/safe-outputs.md @@ -199,6 +199,8 @@ The `repository` value must be `"self"`, an alias from the `checkout:` list in t ### noop Reports that no action was needed. Use this to provide visibility when analysis is complete but no changes or outputs are required. +The executor always files an Azure DevOps work item or appends a comment to an existing one. Override the defaults in front matter to customise the title, type, or area path. If ADO credentials are not available the tool succeeds with a warning. + **Agent parameters:** - `context` - Optional context about why no action was taken @@ -206,17 +208,17 @@ Reports that no action was needed. Use this to provide visibility when analysis ```yaml safe-outputs: noop: - work-item: # Optional — file or append to a work item when noop is reached - title: "Agent reported no operation" # Required — work item title (used to find existing items) - work-item-type: Task # Work item type (default: "Task") - area-path: "MyProject\\MyTeam" # Optional — area path - iteration-path: "MyProject\\Sprint 1" # Optional — iteration path - tags: # Optional — tags to apply + work-item: # Work item config — always active with these defaults + title: "Agent reported no operation" # Default title (used to find existing items too) + work-item-type: Task # Work item type (default: "Task") + area-path: "MyProject\\MyTeam" # Optional — area path + iteration-path: "MyProject\\Sprint 1" # Optional — iteration path + tags: # Optional — tags to apply - agent-noop - include-stats: true # Append agent stats to description/comment (default: true) + include-stats: true # Append agent stats to description/comment (default: true) ``` -When `work-item` is configured, the executor searches for a non-closed work item with the same `title` in the project. If one is found, a comment is appended; otherwise a new work item is created. +The executor searches for a non-closed work item with the same `title` in the project. If one is found, a comment is appended; otherwise a new work item is created. ### missing-data Reports that data or information needed to complete the task is not available. @@ -229,6 +231,8 @@ Reports that data or information needed to complete the task is not available. ### missing-tool Reports that a tool or capability needed to complete the task is not available. +The executor always files an Azure DevOps work item or appends a comment to an existing one. Override the defaults in front matter to customise the title, type, or area path. If ADO credentials are not available the tool succeeds with a warning. + **Agent parameters:** - `tool_name` - Name of the tool that was expected but not found - `context` - Optional context about why the tool was needed @@ -237,17 +241,17 @@ Reports that a tool or capability needed to complete the task is not available. ```yaml safe-outputs: missing-tool: - work-item: # Optional — file or append to a work item when a tool is missing - title: "Agent encountered missing tool" # Required — work item title (used to find existing items) - work-item-type: Bug # Work item type (default: "Task") - area-path: "MyProject\\MyTeam" # Optional — area path - iteration-path: "MyProject\\Sprint 1" # Optional — iteration path - tags: # Optional — tags to apply + work-item: # Work item config — always active with these defaults + title: "Agent encountered missing tool" # Default title (used to find existing items too) + work-item-type: Task # Work item type (default: "Task") + area-path: "MyProject\\MyTeam" # Optional — area path + iteration-path: "MyProject\\Sprint 1" # Optional — iteration path + tags: # Optional — tags to apply - agent-missing-tool - include-stats: true # Append agent stats to description/comment (default: true) + include-stats: true # Append agent stats to description/comment (default: true) ``` -When `work-item` is configured, the executor searches for a non-closed work item with the same `title` in the project. If one is found, a comment is appended; otherwise a new work item is created. +The executor searches for a non-closed work item with the same `title` in the project. If one is found, a comment is appended; otherwise a new work item is created. ### report-incomplete Reports that a task could not be completed. diff --git a/src/execute.rs b/src/execute.rs index 0d0e5352..3bec689f 100644 --- a/src/execute.rs +++ b/src/execute.rs @@ -578,8 +578,9 @@ mod tests { assert!(result.is_ok()); let (tool_name, result) = result.unwrap(); assert_eq!(tool_name, "noop"); + // noop always attempts to file a work item; without ADO credentials it + // returns a warning (success=true) rather than failing hard. assert!(result.success); - assert!(result.message.contains("No operation")); } #[tokio::test] @@ -591,6 +592,8 @@ mod tests { assert!(result.is_ok()); let (tool_name, result) = result.unwrap(); assert_eq!(tool_name, "missing-tool"); + // missing-tool always attempts to file a work item; without ADO credentials + // it returns a warning (success=true) rather than failing hard. assert!(result.success); } diff --git a/src/safeoutputs/missing_tool.rs b/src/safeoutputs/missing_tool.rs index 9fdb14ad..fc75e4be 100644 --- a/src/safeoutputs/missing_tool.rs +++ b/src/safeoutputs/missing_tool.rs @@ -37,10 +37,28 @@ impl SanitizeContent for MissingToolResult { } } +fn missing_tool_default_work_item_title() -> String { + "Agent encountered missing tool".to_string() +} + +fn missing_tool_default_work_item() -> WorkItemReportConfig { + WorkItemReportConfig { + title: missing_tool_default_work_item_title(), + work_item_type: "Task".to_string(), + area_path: None, + iteration_path: None, + tags: Vec::new(), + include_stats: true, + } +} + /// Configuration for the missing-tool tool (specified in front matter). /// -/// When `work-item` is configured, the executor will file a new Azure DevOps work item -/// or append a comment to an existing one with the same title. +/// The executor always files a new Azure DevOps work item or appends a comment to an +/// existing one with the same title. Override the defaults to customise the work item. +/// +/// If ADO credentials are not available (e.g. the pipeline has no write service +/// connection), the executor succeeds with a warning rather than failing hard. /// /// Example front matter: /// ```yaml @@ -53,19 +71,25 @@ impl SanitizeContent for MissingToolResult { /// tags: /// - agent-missing-tool /// ``` -#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct MissingToolConfig { - /// Optional work item to file (or append to) when a tool is reported missing. - /// If absent, the executor only logs a message. - #[serde(default, rename = "work-item")] - pub work_item: Option, + /// Work item to file (or append to) when a tool is reported missing. + /// Defaults to a Task titled "Agent encountered missing tool". + #[serde(default = "missing_tool_default_work_item", rename = "work-item")] + pub work_item: WorkItemReportConfig, +} + +impl Default for MissingToolConfig { + fn default() -> Self { + Self { + work_item: missing_tool_default_work_item(), + } + } } impl SanitizeConfig for MissingToolConfig { fn sanitize_config_fields(&mut self) { - if let Some(wi) = &mut self.work_item { - wi.sanitize_config_fields(); - } + self.work_item.sanitize_config_fields(); } } @@ -82,12 +106,7 @@ impl Executor for MissingToolResult { }; let config: MissingToolConfig = ctx.get_tool_config("missing-tool"); - - if let Some(wi_config) = &config.work_item { - return file_or_append_work_item(wi_config, &message, ctx).await; - } - - Ok(ExecutionResult::success(message)) + file_or_append_work_item(&config.work_item, &message, ctx).await } } @@ -143,53 +162,43 @@ mod tests { } #[test] - fn test_config_defaults_to_no_work_item() { + fn test_config_default_has_sensible_work_item() { let config = MissingToolConfig::default(); - assert!(config.work_item.is_none()); + assert_eq!(config.work_item.title, "Agent encountered missing tool"); + assert_eq!(config.work_item.work_item_type, "Task"); + assert!(config.work_item.area_path.is_none()); + assert!(config.work_item.iteration_path.is_none()); + assert!(config.work_item.tags.is_empty()); + assert!(config.work_item.include_stats); } #[test] - fn test_config_deserializes_with_work_item() { + fn test_config_deserializes_with_work_item_overrides() { let yaml = r#" work-item: - title: "Agent encountered missing tool" + title: "Custom missing tool title" work-item-type: Bug area-path: "MyProject\\MyTeam" tags: - agent-missing-tool "#; let config: MissingToolConfig = serde_yaml::from_str(yaml).unwrap(); - let wi = config.work_item.unwrap(); - assert_eq!(wi.title, "Agent encountered missing tool"); - assert_eq!(wi.work_item_type, "Bug"); - assert_eq!(wi.area_path.as_deref(), Some("MyProject\\MyTeam")); - assert_eq!(wi.tags, vec!["agent-missing-tool"]); + assert_eq!(config.work_item.title, "Custom missing tool title"); + assert_eq!(config.work_item.work_item_type, "Bug"); + assert_eq!(config.work_item.area_path.as_deref(), Some("MyProject\\MyTeam")); + assert_eq!(config.work_item.tags, vec!["agent-missing-tool"]); } #[test] - fn test_config_deserializes_without_work_item() { + fn test_config_deserializes_empty_uses_defaults() { let yaml = r#"{}"#; let config: MissingToolConfig = serde_yaml::from_str(yaml).unwrap(); - assert!(config.work_item.is_none()); - } - - #[test] - fn test_work_item_config_default_type() { - let yaml = r#" -work-item: - title: "Missing tool" -"#; - let config: MissingToolConfig = serde_yaml::from_str(yaml).unwrap(); - let wi = config.work_item.unwrap(); - assert_eq!(wi.work_item_type, "Task"); - assert!(wi.area_path.is_none()); - assert!(wi.iteration_path.is_none()); - assert!(wi.tags.is_empty()); - assert!(wi.include_stats); + assert_eq!(config.work_item.title, "Agent encountered missing tool"); + assert_eq!(config.work_item.work_item_type, "Task"); } #[tokio::test] - async fn test_execute_impl_without_work_item_config() { + async fn test_execute_impl_without_ado_credentials_returns_warning() { let result: MissingToolResult = MissingToolParams { tool_name: "bash".to_string(), context: Some("needed for script execution".to_string()), @@ -197,12 +206,12 @@ work-item: .try_into() .unwrap(); + // Default ExecutionContext has no ADO credentials — should warn, not fail let exec = result .execute_impl(&crate::safeoutputs::ExecutionContext::default()) .await .unwrap(); assert!(exec.success); - assert!(exec.message.contains("Missing tool reported")); - assert!(exec.message.contains("bash")); + assert!(exec.is_warning()); } } diff --git a/src/safeoutputs/mod.rs b/src/safeoutputs/mod.rs index e89d500d..11aa5a44 100644 --- a/src/safeoutputs/mod.rs +++ b/src/safeoutputs/mod.rs @@ -382,12 +382,15 @@ fn work_item_report_default_type() -> String { "Task".to_string() } -/// Configuration for optionally filing (or appending to) an Azure DevOps work item +/// Configuration for filing (or appending to) an Azure DevOps work item /// when a diagnostic safe output (`noop`, `missing-tool`) is called. /// /// If a work item with the same title already exists in the project in a non-closed /// state, a comment is appended instead of creating a new work item. /// +/// Both `noop` and `missing-tool` default to always creating/appending a work item. +/// Override the defaults in front matter to customise the title, type, area path, etc. +/// /// Example: /// ```yaml /// safe-outputs: @@ -427,19 +430,6 @@ pub struct WorkItemReportConfig { pub include_stats: bool, } -impl Default for WorkItemReportConfig { - fn default() -> Self { - Self { - title: String::new(), - work_item_type: work_item_report_default_type(), - area_path: None, - iteration_path: None, - tags: Vec::new(), - include_stats: true, - } - } -} - /// Search for a non-closed work item by exact title using WIQL. /// Returns the ID of the most-recently-changed matching work item, or `None` if none found. async fn wiql_find_work_item_by_title( @@ -516,6 +506,11 @@ async fn wiql_find_work_item_by_title( /// a comment with `body` is appended. Otherwise a new work item is created /// with `body` as the description. /// +/// When ADO credentials are not available (e.g. the pipeline has no write token) the +/// function returns [`ExecutionResult::warning`] instead of a hard failure so that +/// always-on diagnostic tools (`noop`, `missing-tool`) do not break pipelines that +/// run without a write service connection. +/// /// Returns an [`ExecutionResult`] describing what was done. pub(crate) async fn file_or_append_work_item( config: &WorkItemReportConfig, @@ -525,25 +520,25 @@ pub(crate) async fn file_or_append_work_item( let org_url = match &ctx.ado_org_url { Some(u) => u, None => { - return Ok(ExecutionResult::failure( - "AZURE_DEVOPS_ORG_URL not set; cannot file work item".to_string(), + return Ok(ExecutionResult::warning( + "AZURE_DEVOPS_ORG_URL not set; work item not filed".to_string(), )); } }; let project = match &ctx.ado_project { Some(p) => p, None => { - return Ok(ExecutionResult::failure( - "SYSTEM_TEAMPROJECT not set; cannot file work item".to_string(), + return Ok(ExecutionResult::warning( + "SYSTEM_TEAMPROJECT not set; work item not filed".to_string(), )); } }; let token = match &ctx.access_token { Some(t) => t, None => { - return Ok(ExecutionResult::failure( + return Ok(ExecutionResult::warning( "No access token available (SYSTEM_ACCESSTOKEN or AZURE_DEVOPS_EXT_PAT); \ - cannot file work item" + work item not filed" .to_string(), )); } @@ -556,9 +551,9 @@ pub(crate) async fn file_or_append_work_item( match wiql_find_work_item_by_title(&client, org_url, project, token, &config.title).await { Ok(id) => id, Err(e) => { - warn!("WIQL search for existing work item failed: {e} — aborting work item filing"); - return Ok(ExecutionResult::failure(format!( - "Failed to search for existing work item: {e}" + warn!("WIQL search for existing work item failed: {e} — skipping work item filing"); + return Ok(ExecutionResult::warning(format!( + "Work item not filed (WIQL search failed): {e}" ))); } }; diff --git a/src/safeoutputs/noop.rs b/src/safeoutputs/noop.rs index f4953388..048f327c 100644 --- a/src/safeoutputs/noop.rs +++ b/src/safeoutputs/noop.rs @@ -31,10 +31,28 @@ impl SanitizeContent for NoopResult { } } +fn noop_default_work_item_title() -> String { + "Agent reported no operation".to_string() +} + +fn noop_default_work_item() -> WorkItemReportConfig { + WorkItemReportConfig { + title: noop_default_work_item_title(), + work_item_type: "Task".to_string(), + area_path: None, + iteration_path: None, + tags: Vec::new(), + include_stats: true, + } +} + /// Configuration for the noop tool (specified in front matter). /// -/// When `work-item` is configured, the executor will file a new Azure DevOps work item -/// or append a comment to an existing one with the same title. +/// The executor always files a new Azure DevOps work item or appends a comment to an +/// existing one with the same title. Override the defaults to customise the work item. +/// +/// If ADO credentials are not available (e.g. the pipeline has no write service +/// connection), the executor succeeds with a warning rather than failing hard. /// /// Example front matter: /// ```yaml @@ -48,19 +66,25 @@ impl SanitizeContent for NoopResult { /// tags: /// - agent-noop /// ``` -#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct NoopConfig { - /// Optional work item to file (or append to) when a noop is reached. - /// If absent, the executor only logs a message. - #[serde(default, rename = "work-item")] - pub work_item: Option, + /// Work item to file (or append to) when a noop is reached. + /// Defaults to a Task titled "Agent reported no operation". + #[serde(default = "noop_default_work_item", rename = "work-item")] + pub work_item: WorkItemReportConfig, +} + +impl Default for NoopConfig { + fn default() -> Self { + Self { + work_item: noop_default_work_item(), + } + } } impl SanitizeConfig for NoopConfig { fn sanitize_config_fields(&mut self) { - if let Some(wi) = &mut self.work_item { - wi.sanitize_config_fields(); - } + self.work_item.sanitize_config_fields(); } } @@ -77,12 +101,7 @@ impl Executor for NoopResult { }; let config: NoopConfig = ctx.get_tool_config("noop"); - - if let Some(wi_config) = &config.work_item { - return file_or_append_work_item(wi_config, &message, ctx).await; - } - - Ok(ExecutionResult::success(message)) + file_or_append_work_item(&config.work_item, &message, ctx).await } } @@ -154,65 +173,55 @@ mod tests { } #[test] - fn test_config_defaults_to_no_work_item() { + fn test_config_default_has_sensible_work_item() { let config = NoopConfig::default(); - assert!(config.work_item.is_none()); + assert_eq!(config.work_item.title, "Agent reported no operation"); + assert_eq!(config.work_item.work_item_type, "Task"); + assert!(config.work_item.area_path.is_none()); + assert!(config.work_item.iteration_path.is_none()); + assert!(config.work_item.tags.is_empty()); + assert!(config.work_item.include_stats); } #[test] - fn test_config_deserializes_with_work_item() { + fn test_config_deserializes_with_work_item_overrides() { let yaml = r#" work-item: - title: "Agent reported no operation" - work-item-type: Task + title: "My custom noop title" + work-item-type: Bug area-path: "MyProject\\MyTeam" tags: - agent-noop "#; let config: NoopConfig = serde_yaml::from_str(yaml).unwrap(); - let wi = config.work_item.unwrap(); - assert_eq!(wi.title, "Agent reported no operation"); - assert_eq!(wi.work_item_type, "Task"); - assert_eq!(wi.area_path.as_deref(), Some("MyProject\\MyTeam")); - assert_eq!(wi.tags, vec!["agent-noop"]); + assert_eq!(config.work_item.title, "My custom noop title"); + assert_eq!(config.work_item.work_item_type, "Bug"); + assert_eq!(config.work_item.area_path.as_deref(), Some("MyProject\\MyTeam")); + assert_eq!(config.work_item.tags, vec!["agent-noop"]); } #[test] - fn test_config_deserializes_without_work_item() { + fn test_config_deserializes_empty_uses_defaults() { let yaml = r#"{}"#; let config: NoopConfig = serde_yaml::from_str(yaml).unwrap(); - assert!(config.work_item.is_none()); - } - - #[test] - fn test_work_item_config_default_type() { - let yaml = r#" -work-item: - title: "No op" -"#; - let config: NoopConfig = serde_yaml::from_str(yaml).unwrap(); - let wi = config.work_item.unwrap(); - assert_eq!(wi.work_item_type, "Task"); - assert!(wi.area_path.is_none()); - assert!(wi.iteration_path.is_none()); - assert!(wi.tags.is_empty()); - assert!(wi.include_stats); + assert_eq!(config.work_item.title, "Agent reported no operation"); + assert_eq!(config.work_item.work_item_type, "Task"); } #[tokio::test] - async fn test_execute_impl_without_work_item_config() { + async fn test_execute_impl_without_ado_credentials_returns_warning() { let result: NoopResult = NoopParams { context: Some("nothing to do".to_string()), } .try_into() .unwrap(); + // Default ExecutionContext has no ADO credentials — should warn, not fail let exec = result .execute_impl(&crate::safeoutputs::ExecutionContext::default()) .await .unwrap(); assert!(exec.success); - assert!(exec.message.contains("No operation needed")); - assert!(exec.message.contains("nothing to do")); + assert!(exec.is_warning()); } } From c2421358e198c8529d9872cdfe24ff15f0f42478 Mon Sep 17 00:00:00 2001 From: James Devine Date: Tue, 12 May 2026 23:11:57 +0100 Subject: [PATCH 4/6] retitle defaults --- docs/safe-outputs.md | 4 ++-- src/safeoutputs/missing_tool.rs | 10 +++++----- src/safeoutputs/mod.rs | 2 +- src/safeoutputs/noop.rs | 10 +++++----- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/docs/safe-outputs.md b/docs/safe-outputs.md index d7bb5d5d..34afb4d9 100644 --- a/docs/safe-outputs.md +++ b/docs/safe-outputs.md @@ -209,7 +209,7 @@ The executor always files an Azure DevOps work item or appends a comment to an e safe-outputs: noop: work-item: # Work item config — always active with these defaults - title: "Agent reported no operation" # Default title (used to find existing items too) + title: "[ado-aw] Agent reported no operation" # Default title (used to find existing items too) work-item-type: Task # Work item type (default: "Task") area-path: "MyProject\\MyTeam" # Optional — area path iteration-path: "MyProject\\Sprint 1" # Optional — iteration path @@ -242,7 +242,7 @@ The executor always files an Azure DevOps work item or appends a comment to an e safe-outputs: missing-tool: work-item: # Work item config — always active with these defaults - title: "Agent encountered missing tool" # Default title (used to find existing items too) + title: "[ado-aw] Agent encountered missing tool" # Default title (used to find existing items too) work-item-type: Task # Work item type (default: "Task") area-path: "MyProject\\MyTeam" # Optional — area path iteration-path: "MyProject\\Sprint 1" # Optional — iteration path diff --git a/src/safeoutputs/missing_tool.rs b/src/safeoutputs/missing_tool.rs index fc75e4be..3c2a873a 100644 --- a/src/safeoutputs/missing_tool.rs +++ b/src/safeoutputs/missing_tool.rs @@ -38,7 +38,7 @@ impl SanitizeContent for MissingToolResult { } fn missing_tool_default_work_item_title() -> String { - "Agent encountered missing tool".to_string() + "[ado-aw] Agent encountered missing tool".to_string() } fn missing_tool_default_work_item() -> WorkItemReportConfig { @@ -65,7 +65,7 @@ fn missing_tool_default_work_item() -> WorkItemReportConfig { /// safe-outputs: /// missing-tool: /// work-item: -/// title: "Agent encountered missing tool" +/// title: "[ado-aw] Agent encountered missing tool" /// work-item-type: Bug /// area-path: "MyProject\\MyTeam" /// tags: @@ -74,7 +74,7 @@ fn missing_tool_default_work_item() -> WorkItemReportConfig { #[derive(Debug, Clone, Serialize, Deserialize)] pub struct MissingToolConfig { /// Work item to file (or append to) when a tool is reported missing. - /// Defaults to a Task titled "Agent encountered missing tool". + /// Defaults to a Task titled "[ado-aw] Agent encountered missing tool". #[serde(default = "missing_tool_default_work_item", rename = "work-item")] pub work_item: WorkItemReportConfig, } @@ -164,7 +164,7 @@ mod tests { #[test] fn test_config_default_has_sensible_work_item() { let config = MissingToolConfig::default(); - assert_eq!(config.work_item.title, "Agent encountered missing tool"); + assert_eq!(config.work_item.title, "[ado-aw] Agent encountered missing tool"); assert_eq!(config.work_item.work_item_type, "Task"); assert!(config.work_item.area_path.is_none()); assert!(config.work_item.iteration_path.is_none()); @@ -193,7 +193,7 @@ work-item: fn test_config_deserializes_empty_uses_defaults() { let yaml = r#"{}"#; let config: MissingToolConfig = serde_yaml::from_str(yaml).unwrap(); - assert_eq!(config.work_item.title, "Agent encountered missing tool"); + assert_eq!(config.work_item.title, "[ado-aw] Agent encountered missing tool"); assert_eq!(config.work_item.work_item_type, "Task"); } diff --git a/src/safeoutputs/mod.rs b/src/safeoutputs/mod.rs index 11aa5a44..4022ef91 100644 --- a/src/safeoutputs/mod.rs +++ b/src/safeoutputs/mod.rs @@ -396,7 +396,7 @@ fn work_item_report_default_type() -> String { /// safe-outputs: /// noop: /// work-item: -/// title: "Agent reported no operation" +/// title: "[ado-aw] Agent reported no operation" /// work-item-type: Task /// area-path: "MyProject\\MyTeam" /// tags: diff --git a/src/safeoutputs/noop.rs b/src/safeoutputs/noop.rs index 048f327c..35d100c6 100644 --- a/src/safeoutputs/noop.rs +++ b/src/safeoutputs/noop.rs @@ -32,7 +32,7 @@ impl SanitizeContent for NoopResult { } fn noop_default_work_item_title() -> String { - "Agent reported no operation".to_string() + "[ado-aw] Agent reported no operation".to_string() } fn noop_default_work_item() -> WorkItemReportConfig { @@ -59,7 +59,7 @@ fn noop_default_work_item() -> WorkItemReportConfig { /// safe-outputs: /// noop: /// work-item: -/// title: "Agent reported no operation" +/// title: "[ado-aw] Agent reported no operation" /// work-item-type: Task /// area-path: "MyProject\\MyTeam" /// iteration-path: "MyProject\\Sprint 1" @@ -69,7 +69,7 @@ fn noop_default_work_item() -> WorkItemReportConfig { #[derive(Debug, Clone, Serialize, Deserialize)] pub struct NoopConfig { /// Work item to file (or append to) when a noop is reached. - /// Defaults to a Task titled "Agent reported no operation". + /// Defaults to a Task titled "[ado-aw] Agent reported no operation". #[serde(default = "noop_default_work_item", rename = "work-item")] pub work_item: WorkItemReportConfig, } @@ -175,7 +175,7 @@ mod tests { #[test] fn test_config_default_has_sensible_work_item() { let config = NoopConfig::default(); - assert_eq!(config.work_item.title, "Agent reported no operation"); + assert_eq!(config.work_item.title, "[ado-aw] Agent reported no operation"); assert_eq!(config.work_item.work_item_type, "Task"); assert!(config.work_item.area_path.is_none()); assert!(config.work_item.iteration_path.is_none()); @@ -204,7 +204,7 @@ work-item: fn test_config_deserializes_empty_uses_defaults() { let yaml = r#"{}"#; let config: NoopConfig = serde_yaml::from_str(yaml).unwrap(); - assert_eq!(config.work_item.title, "Agent reported no operation"); + assert_eq!(config.work_item.title, "[ado-aw] Agent reported no operation"); assert_eq!(config.work_item.work_item_type, "Task"); } From 5ebf736a80752ac203ff629814a49c5e4929ba0b Mon Sep 17 00:00:00 2001 From: James Devine Date: Tue, 12 May 2026 23:22:48 +0100 Subject: [PATCH 5/6] feat(safe-outputs): auto-assign work items to last committer of agent file When create-work-item has no explicit assignee configured in front matter, Stage 3 now falls back to the email of the person who last committed changes to the agent source markdown file. The lookup uses git log -1 --format=%ae and degrades gracefully (no assignee set) when git history is unavailable (e.g. shallow clone). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/safe-outputs.md | 2 +- src/main.rs | 48 ++++++++++++++++++++++++++++- src/safeoutputs/create_work_item.rs | 5 +-- src/safeoutputs/result.rs | 10 ++++++ 4 files changed, 61 insertions(+), 4 deletions(-) diff --git a/docs/safe-outputs.md b/docs/safe-outputs.md index 34afb4d9..5fca2cfc 100644 --- a/docs/safe-outputs.md +++ b/docs/safe-outputs.md @@ -71,7 +71,7 @@ Creates an Azure DevOps work item. - `work-item-type` - Work item type (default: "Task") - `area-path` - Area path for the work item - `iteration-path` - Iteration path for the work item -- `assignee` - User to assign (email or display name) +- `assignee` - User to assign (email or display name). When omitted, falls back to the email of the last person who committed changes to the agent source markdown file (discovered via `git log` at Stage 3). - `tags` - Static list of tags always applied to the work item (regardless of agent input) - `allowed-tags` - Allowlist of tags the agent is permitted to use via the `tags` parameter. If empty, any agent-provided tags are accepted. Supports `*` wildcards anywhere in the pattern (e.g., `"agent-*"` matches `"agent-created"`; `"copilot:repo=org/project/*@main"` matches any repo name). - `custom-fields` - Map of custom field reference names to values (e.g., `Custom.MyField: "value"`) diff --git a/src/main.rs b/src/main.rs index 6dcb07c6..9aa8926a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -264,7 +264,7 @@ async fn run_execute( let tools = front_matter.tools.clone(); // Build execution context from front matter, CLI args, and environment - let ctx = build_execution_context( + let mut ctx = build_execution_context( front_matter, &safe_output_dir, ado_org_url, @@ -273,6 +273,13 @@ async fn run_execute( ) .await; + // Discover the last committer of the agent source file for use as a + // fallback assignee in create-work-item. + ctx.agent_last_committer = discover_last_committer(&source).await; + if let Some(ref email) = ctx.agent_last_committer { + log::info!("Agent source last committer: {}", email); + } + let results = execute::execute_safe_outputs(&safe_output_dir, &ctx).await?; // Process agent memory if cache-memory tool is enabled @@ -349,6 +356,45 @@ async fn build_execution_context( ctx } +/// Look up the email of the person who last committed changes to `path`. +/// +/// Runs `git log -1 --format='%ae' -- ` in the file's parent directory. +/// Returns `None` (with a debug log) when the lookup fails — e.g. shallow +/// clone with no relevant history, or git is unavailable. +async fn discover_last_committer(path: &Path) -> Option { + let dir = path.parent().unwrap_or(Path::new(".")); + let output = tokio::process::Command::new("git") + .args(["log", "-1", "--format=%ae", "--"]) + .arg(path.file_name()?) + .current_dir(dir) + .output() + .await; + + match output { + Ok(o) if o.status.success() => { + let email = String::from_utf8_lossy(&o.stdout).trim().to_string(); + if email.is_empty() { + log::debug!("git log returned no committer for {}", path.display()); + None + } else { + Some(email) + } + } + Ok(o) => { + log::debug!( + "git log failed for {}: {}", + path.display(), + String::from_utf8_lossy(&o.stderr).trim() + ); + None + } + Err(e) => { + log::debug!("Failed to run git log for {}: {}", path.display(), e); + None + } + } +} + async fn process_cache_memory( tools: Option<&compile::types::ToolsConfig>, safe_output_dir: &PathBuf, diff --git a/src/safeoutputs/create_work_item.rs b/src/safeoutputs/create_work_item.rs index 5e0db165..7da126aa 100644 --- a/src/safeoutputs/create_work_item.rs +++ b/src/safeoutputs/create_work_item.rs @@ -300,7 +300,8 @@ impl Executor for CreateWorkItemResult { debug!("Work item type: {}", config.work_item_type); debug!("Area path: {:?}", config.area_path); debug!("Iteration path: {:?}", config.iteration_path); - debug!("Assignee: {:?}", config.assignee); + debug!("Assignee (config): {:?}", config.assignee); + debug!("Assignee (last committer fallback): {:?}", ctx.agent_last_committer); // Validate agent-provided tags against allowed-tags (if configured) if !self.tags.is_empty() && !config.allowed_tags.is_empty() { @@ -357,7 +358,7 @@ impl Executor for CreateWorkItemResult { if let Some(iteration_path) = &config.iteration_path { patch_doc.push(field_op("System.IterationPath", iteration_path)); } - if let Some(assignee) = &config.assignee { + if let Some(assignee) = config.assignee.as_ref().or(ctx.agent_last_committer.as_ref()) { patch_doc.push(field_op("System.AssignedTo", assignee)); } // Merge static config tags with validated agent-provided tags (dedup, case-insensitive) diff --git a/src/safeoutputs/result.rs b/src/safeoutputs/result.rs index 6096d08b..3c2e6f27 100644 --- a/src/safeoutputs/result.rs +++ b/src/safeoutputs/result.rs @@ -117,6 +117,13 @@ pub struct ExecutionContext { #[allow(dead_code)] pub pull_request_target_branch: Option, + /// Email of the last git committer of the agent source file. + /// + /// Populated at Stage 3 startup by running `git log -1 --format='%ae'` + /// against the agent markdown. Used as a fallback assignee for + /// `create-work-item` when no explicit `assignee` is configured. + pub agent_last_committer: Option, + /// Per-run dedupe set for `upload-pipeline-artifact` when the /// `require-unique-names` config is set. Stores `format!("{}/{}", /// effective_build_id, final_name)` keys; the executor checks-and-inserts @@ -220,6 +227,9 @@ impl ExecutionContext { pull_request_source_branch: env("SYSTEM_PULLREQUEST_SOURCEBRANCH"), pull_request_target_branch: env("SYSTEM_PULLREQUEST_TARGETBRANCH"), + // Populated later by run_execute via git log on the source file + agent_last_committer: None, + // Per-run state for upload-pipeline-artifact dedupe. uploaded_pipeline_artifact_keys: Arc::new(Mutex::new(HashSet::new())), } From 4ca23e548391ee1ea2f48f3b59b398470f741556 Mon Sep 17 00:00:00 2001 From: James Devine Date: Tue, 12 May 2026 23:28:12 +0100 Subject: [PATCH 6/6] fix(safe-outputs): prevent silent config loss in WorkItemReportConfig WorkItemReportConfig.title was a bare String with no serde default. When users provided a partial work-item config (e.g. only work-item-type: Bug), serde deserialization failed and get_tool_config's unwrap_or_default() silently discarded all overrides with no error or warning. Fix: change title to Option with #[serde(default)]. Each caller (noop, missing-tool) passes its context-specific default title to file_or_append_work_item, which resolves the effective title via .unwrap_or(default_title). Add regression tests for partial work-item configs to both noop and missing-tool. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/safeoutputs/missing_tool.rs | 24 +++++++++++++++++++----- src/safeoutputs/mod.rs | 23 +++++++++++++++-------- src/safeoutputs/noop.rs | 25 ++++++++++++++++++++----- 3 files changed, 54 insertions(+), 18 deletions(-) diff --git a/src/safeoutputs/missing_tool.rs b/src/safeoutputs/missing_tool.rs index 3c2a873a..c9d9f2b4 100644 --- a/src/safeoutputs/missing_tool.rs +++ b/src/safeoutputs/missing_tool.rs @@ -43,7 +43,7 @@ fn missing_tool_default_work_item_title() -> String { fn missing_tool_default_work_item() -> WorkItemReportConfig { WorkItemReportConfig { - title: missing_tool_default_work_item_title(), + title: Some(missing_tool_default_work_item_title()), work_item_type: "Task".to_string(), area_path: None, iteration_path: None, @@ -106,7 +106,7 @@ impl Executor for MissingToolResult { }; let config: MissingToolConfig = ctx.get_tool_config("missing-tool"); - file_or_append_work_item(&config.work_item, &message, ctx).await + file_or_append_work_item(&config.work_item, &missing_tool_default_work_item_title(), &message, ctx).await } } @@ -164,7 +164,7 @@ mod tests { #[test] fn test_config_default_has_sensible_work_item() { let config = MissingToolConfig::default(); - assert_eq!(config.work_item.title, "[ado-aw] Agent encountered missing tool"); + assert_eq!(config.work_item.title.as_deref(), Some("[ado-aw] Agent encountered missing tool")); assert_eq!(config.work_item.work_item_type, "Task"); assert!(config.work_item.area_path.is_none()); assert!(config.work_item.iteration_path.is_none()); @@ -183,7 +183,7 @@ work-item: - agent-missing-tool "#; let config: MissingToolConfig = serde_yaml::from_str(yaml).unwrap(); - assert_eq!(config.work_item.title, "Custom missing tool title"); + assert_eq!(config.work_item.title.as_deref(), Some("Custom missing tool title")); assert_eq!(config.work_item.work_item_type, "Bug"); assert_eq!(config.work_item.area_path.as_deref(), Some("MyProject\\MyTeam")); assert_eq!(config.work_item.tags, vec!["agent-missing-tool"]); @@ -193,10 +193,24 @@ work-item: fn test_config_deserializes_empty_uses_defaults() { let yaml = r#"{}"#; let config: MissingToolConfig = serde_yaml::from_str(yaml).unwrap(); - assert_eq!(config.work_item.title, "[ado-aw] Agent encountered missing tool"); + assert_eq!(config.work_item.title.as_deref(), Some("[ado-aw] Agent encountered missing tool")); assert_eq!(config.work_item.work_item_type, "Task"); } + #[test] + fn test_config_partial_work_item_preserves_overrides() { + let yaml = r#" +work-item: + work-item-type: Bug + tags: + - agent-missing-tool +"#; + let config: MissingToolConfig = serde_yaml::from_str(yaml).unwrap(); + assert!(config.work_item.title.is_none(), "title should be None when omitted"); + assert_eq!(config.work_item.work_item_type, "Bug"); + assert_eq!(config.work_item.tags, vec!["agent-missing-tool"]); + } + #[tokio::test] async fn test_execute_impl_without_ado_credentials_returns_warning() { let result: MissingToolResult = MissingToolParams { diff --git a/src/safeoutputs/mod.rs b/src/safeoutputs/mod.rs index 4022ef91..9b06636c 100644 --- a/src/safeoutputs/mod.rs +++ b/src/safeoutputs/mod.rs @@ -407,7 +407,11 @@ pub struct WorkItemReportConfig { /// Title of the work item to file or append a comment to. /// If a non-closed work item with this exact title already exists, /// a comment is appended rather than creating a new work item. - pub title: String, + /// + /// When `None`, each caller supplies a context-appropriate default + /// (e.g. noop vs missing-tool). + #[serde(default)] + pub title: Option, /// Work item type to create (default: "Task") #[serde(default = "work_item_report_default_type", rename = "work-item-type")] @@ -502,7 +506,8 @@ async fn wiql_find_work_item_by_title( /// File a new work item or append a comment to an existing one with the same title. /// -/// If a non-closed work item matching `config.title` exists in the project, +/// If a non-closed work item matching the title (from `config.title` or +/// `default_title` when the config omits it) exists in the project, /// a comment with `body` is appended. Otherwise a new work item is created /// with `body` as the description. /// @@ -514,9 +519,11 @@ async fn wiql_find_work_item_by_title( /// Returns an [`ExecutionResult`] describing what was done. pub(crate) async fn file_or_append_work_item( config: &WorkItemReportConfig, + default_title: &str, body: &str, ctx: &ExecutionContext, ) -> anyhow::Result { + let title = config.title.as_deref().unwrap_or(default_title); let org_url = match &ctx.ado_org_url { Some(u) => u, None => { @@ -548,7 +555,7 @@ pub(crate) async fn file_or_append_work_item( // Search for an existing non-closed work item with the same title let existing_id = - match wiql_find_work_item_by_title(&client, org_url, project, token, &config.title).await { + match wiql_find_work_item_by_title(&client, org_url, project, token, title).await { Ok(id) => id, Err(e) => { warn!("WIQL search for existing work item failed: {e} — skipping work item filing"); @@ -594,11 +601,11 @@ pub(crate) async fn file_or_append_work_item( let message = match comment_id { Some(id) => format!( "Appended comment #{} to existing work item #{}: {}", - id, work_item_id, config.title + id, work_item_id, title ), None => format!( "Appended comment to existing work item #{}: {}", - work_item_id, config.title + work_item_id, title ), }; Ok(ExecutionResult::success_with_data( @@ -625,7 +632,7 @@ pub(crate) async fn file_or_append_work_item( debug!("No existing work item found, creating new one"); let mut patch_doc = vec![ - serde_json::json!({"op": "add", "path": "/fields/System.Title", "value": &config.title}), + serde_json::json!({"op": "add", "path": "/fields/System.Title", "value": title}), serde_json::json!({"op": "add", "path": "/fields/System.Description", "value": body_with_stats}), serde_json::json!({"op": "add", "path": "/multilineFieldsFormat/System.Description", "value": "Markdown"}), ]; @@ -676,8 +683,8 @@ pub(crate) async fn file_or_append_work_item( .unwrap_or("") .to_string(); let message = match work_item_id { - Some(id) => format!("Created work item #{}: {}", id, config.title), - None => format!("Created work item: {}", config.title), + Some(id) => format!("Created work item #{}: {}", id, title), + None => format!("Created work item: {}", title), }; Ok(ExecutionResult::success_with_data( message, diff --git a/src/safeoutputs/noop.rs b/src/safeoutputs/noop.rs index 35d100c6..41b521da 100644 --- a/src/safeoutputs/noop.rs +++ b/src/safeoutputs/noop.rs @@ -37,7 +37,7 @@ fn noop_default_work_item_title() -> String { fn noop_default_work_item() -> WorkItemReportConfig { WorkItemReportConfig { - title: noop_default_work_item_title(), + title: Some(noop_default_work_item_title()), work_item_type: "Task".to_string(), area_path: None, iteration_path: None, @@ -101,7 +101,7 @@ impl Executor for NoopResult { }; let config: NoopConfig = ctx.get_tool_config("noop"); - file_or_append_work_item(&config.work_item, &message, ctx).await + file_or_append_work_item(&config.work_item, &noop_default_work_item_title(), &message, ctx).await } } @@ -175,7 +175,7 @@ mod tests { #[test] fn test_config_default_has_sensible_work_item() { let config = NoopConfig::default(); - assert_eq!(config.work_item.title, "[ado-aw] Agent reported no operation"); + assert_eq!(config.work_item.title.as_deref(), Some("[ado-aw] Agent reported no operation")); assert_eq!(config.work_item.work_item_type, "Task"); assert!(config.work_item.area_path.is_none()); assert!(config.work_item.iteration_path.is_none()); @@ -194,7 +194,7 @@ work-item: - agent-noop "#; let config: NoopConfig = serde_yaml::from_str(yaml).unwrap(); - assert_eq!(config.work_item.title, "My custom noop title"); + assert_eq!(config.work_item.title.as_deref(), Some("My custom noop title")); assert_eq!(config.work_item.work_item_type, "Bug"); assert_eq!(config.work_item.area_path.as_deref(), Some("MyProject\\MyTeam")); assert_eq!(config.work_item.tags, vec!["agent-noop"]); @@ -204,10 +204,25 @@ work-item: fn test_config_deserializes_empty_uses_defaults() { let yaml = r#"{}"#; let config: NoopConfig = serde_yaml::from_str(yaml).unwrap(); - assert_eq!(config.work_item.title, "[ado-aw] Agent reported no operation"); + assert_eq!(config.work_item.title.as_deref(), Some("[ado-aw] Agent reported no operation")); assert_eq!(config.work_item.work_item_type, "Task"); } + #[test] + fn test_config_partial_work_item_preserves_overrides() { + // Regression: a partial work-item config (no title) must not + // silently discard the user's other overrides. + let yaml = r#" +work-item: + work-item-type: Bug + area-path: "MyProject\\MyTeam" +"#; + let config: NoopConfig = serde_yaml::from_str(yaml).unwrap(); + assert!(config.work_item.title.is_none(), "title should be None when omitted"); + assert_eq!(config.work_item.work_item_type, "Bug"); + assert_eq!(config.work_item.area_path.as_deref(), Some("MyProject\\MyTeam")); + } + #[tokio::test] async fn test_execute_impl_without_ado_credentials_returns_warning() { let result: NoopResult = NoopParams {