Skip to content

Commit c1bf552

Browse files
CopilotjamesadevineCopilot
authored
feat(safeoutputs): add work-item filing to noop and missing-tool safe outputs (#521)
* 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> * 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> * 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> * retitle defaults * 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> * 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<String> 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> * fix(safe-outputs): add missing agent_last_committer field to test ExecutionContext structs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(safe-outputs): address reviewer feedback on work-item filing - Sanitize agent_last_committer via sanitize_config() to prevent pipeline command injection from malicious git committer identity - Add enabled: bool to WorkItemReportConfig (default true) so operators can opt out of work-item filing with enabled: false - Add message-content assertions to noop/missing-tool execute tests - Document serde-default layering quirk for title: None in partial work-item configs - Document --follow trade-off in discover_last_committer Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(safe-outputs): tighten test assertions and add HTTP client timeout - Assert is_warning() and tighten message match to 'not set' in execute.rs noop/missing-tool integration tests - Add 30s timeout to reqwest client in file_or_append_work_item to prevent indefinite hangs on slow/unreachable ADO endpoints Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor(safe-outputs): rename agent_last_committer to agent_last_author git log --format=%ae returns the author email, not the committer email. In squash-merge workflows the committer is typically a service account (e.g. GitHub noreply) while the author is the PR author — which is the meaningful value for assignee fallback. Rename discover_last_committer -> discover_last_author and agent_last_committer -> agent_last_author throughout, with updated doc comments explaining the %ae vs %ce distinction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> Co-authored-by: James Devine <devinejames@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 2dc7037 commit c1bf552

10 files changed

Lines changed: 825 additions & 15 deletions

File tree

docs/safe-outputs.md

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ Creates an Azure DevOps work item.
7171
- `work-item-type` - Work item type (default: "Task")
7272
- `area-path` - Area path for the work item
7373
- `iteration-path` - Iteration path for the work item
74-
- `assignee` - User to assign (email or display name)
74+
- `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).
7575
- `tags` - Static list of tags always applied to the work item (regardless of agent input)
7676
- `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).
7777
- `custom-fields` - Map of custom field reference names to values (e.g., `Custom.MyField: "value"`)
@@ -199,9 +199,28 @@ The `repository` value must be `"self"`, an alias from the `checkout:` list in t
199199
### noop
200200
Reports that no action was needed. Use this to provide visibility when analysis is complete but no changes or outputs are required.
201201

202+
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.
203+
202204
**Agent parameters:**
203205
- `context` - Optional context about why no action was taken
204206

207+
**Configuration options (front matter):**
208+
```yaml
209+
safe-outputs:
210+
noop:
211+
work-item: # Work item config — always active with these defaults
212+
enabled: true # Set to false to disable work-item filing entirely
213+
title: "[ado-aw] Agent reported no operation" # Default title (used to find existing items too)
214+
work-item-type: Task # Work item type (default: "Task")
215+
area-path: "MyProject\\MyTeam" # Optional — area path
216+
iteration-path: "MyProject\\Sprint 1" # Optional — iteration path
217+
tags: # Optional — tags to apply
218+
- agent-noop
219+
include-stats: true # Append agent stats to description/comment (default: true)
220+
```
221+
222+
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.
223+
205224
### missing-data
206225
Reports that data or information needed to complete the task is not available.
207226

@@ -213,10 +232,29 @@ Reports that data or information needed to complete the task is not available.
213232
### missing-tool
214233
Reports that a tool or capability needed to complete the task is not available.
215234

235+
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.
236+
216237
**Agent parameters:**
217238
- `tool_name` - Name of the tool that was expected but not found
218239
- `context` - Optional context about why the tool was needed
219240

241+
**Configuration options (front matter):**
242+
```yaml
243+
safe-outputs:
244+
missing-tool:
245+
work-item: # Work item config — always active with these defaults
246+
enabled: true # Set to false to disable work-item filing entirely
247+
title: "[ado-aw] Agent encountered missing tool" # Default title (used to find existing items too)
248+
work-item-type: Task # Work item type (default: "Task")
249+
area-path: "MyProject\\MyTeam" # Optional — area path
250+
iteration-path: "MyProject\\Sprint 1" # Optional — iteration path
251+
tags: # Optional — tags to apply
252+
- agent-missing-tool
253+
include-stats: true # Append agent stats to description/comment (default: true)
254+
```
255+
256+
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.
257+
220258
### report-incomplete
221259
Reports that a task could not be completed.
222260

src/execute.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -578,8 +578,15 @@ mod tests {
578578
assert!(result.is_ok());
579579
let (tool_name, result) = result.unwrap();
580580
assert_eq!(tool_name, "noop");
581+
// noop always attempts to file a work item; without ADO credentials it
582+
// returns a warning (success=true) rather than failing hard.
581583
assert!(result.success);
582-
assert!(result.message.contains("No operation"));
584+
assert!(result.is_warning(), "noop without credentials should be a warning");
585+
assert!(
586+
result.message.contains("not set"),
587+
"noop warning should mention missing config, got: {}",
588+
result.message
589+
);
583590
}
584591

585592
#[tokio::test]
@@ -591,7 +598,15 @@ mod tests {
591598
assert!(result.is_ok());
592599
let (tool_name, result) = result.unwrap();
593600
assert_eq!(tool_name, "missing-tool");
601+
// missing-tool always attempts to file a work item; without ADO credentials
602+
// it returns a warning (success=true) rather than failing hard.
594603
assert!(result.success);
604+
assert!(result.is_warning(), "missing-tool without credentials should be a warning");
605+
assert!(
606+
result.message.contains("not set"),
607+
"missing-tool warning should mention missing config, got: {}",
608+
result.message
609+
);
595610
}
596611

597612
#[tokio::test]

src/main.rs

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ async fn run_execute(
264264
let tools = front_matter.tools.clone();
265265

266266
// Build execution context from front matter, CLI args, and environment
267-
let ctx = build_execution_context(
267+
let mut ctx = build_execution_context(
268268
front_matter,
269269
&safe_output_dir,
270270
ado_org_url,
@@ -273,6 +273,13 @@ async fn run_execute(
273273
)
274274
.await;
275275

276+
// Discover the last author of the agent source file for use as a
277+
// fallback assignee in create-work-item.
278+
ctx.agent_last_author = discover_last_author(&source).await;
279+
if let Some(ref email) = ctx.agent_last_author {
280+
log::info!("Agent source last author: {}", email);
281+
}
282+
276283
let results = execute::execute_safe_outputs(&safe_output_dir, &ctx).await?;
277284

278285
// Process agent memory if cache-memory tool is enabled
@@ -349,6 +356,54 @@ async fn build_execution_context(
349356
ctx
350357
}
351358

359+
/// Look up the email of the person who last authored changes to `path`.
360+
///
361+
/// Runs `git log -1 --format='%ae' -- <path>` in the file's parent directory.
362+
/// Returns `None` (with a debug log) when the lookup fails — e.g. shallow
363+
/// clone with no relevant history, or git is unavailable.
364+
///
365+
/// Note: we pass the bare filename (not a full path) so git resolves it
366+
/// relative to `cwd`. This means renames in history are not followed
367+
/// (`--follow` has its own edge-cases with merge commits and is not worth
368+
/// the complexity here).
369+
async fn discover_last_author(path: &Path) -> Option<String> {
370+
let dir = path.parent().unwrap_or(Path::new("."));
371+
let output = tokio::process::Command::new("git")
372+
.args(["log", "-1", "--format=%ae", "--"])
373+
.arg(path.file_name()?)
374+
.current_dir(dir)
375+
.output()
376+
.await;
377+
378+
match output {
379+
Ok(o) if o.status.success() => {
380+
let email = String::from_utf8_lossy(&o.stdout).trim().to_string();
381+
if email.is_empty() {
382+
log::debug!("git log returned no committer for {}", path.display());
383+
None
384+
} else {
385+
// Sanitize the email: git committer values can contain
386+
// arbitrary text (e.g. ADO pipeline log commands like
387+
// ##vso[task.setvariable ...]). Apply the same config-level
388+
// sanitization used for operator-supplied fields.
389+
Some(crate::sanitize::sanitize_config(&email))
390+
}
391+
}
392+
Ok(o) => {
393+
log::debug!(
394+
"git log failed for {}: {}",
395+
path.display(),
396+
String::from_utf8_lossy(&o.stderr).trim()
397+
);
398+
None
399+
}
400+
Err(e) => {
401+
log::debug!("Failed to run git log for {}: {}", path.display(), e);
402+
None
403+
}
404+
}
405+
}
406+
352407
async fn process_cache_memory(
353408
tools: Option<&compile::types::ToolsConfig>,
354409
safe_output_dir: &PathBuf,

src/safeoutputs/create_pr.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2510,6 +2510,7 @@ new file mode 100755
25102510
uploaded_pipeline_artifact_keys: std::sync::Arc::new(std::sync::Mutex::new(
25112511
std::collections::HashSet::new(),
25122512
)),
2513+
agent_last_author: None,
25132514
};
25142515
let outcome = result.execute_impl(&ctx).await.unwrap();
25152516
assert!(!outcome.success);

src/safeoutputs/create_work_item.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,8 @@ impl Executor for CreateWorkItemResult {
300300
debug!("Work item type: {}", config.work_item_type);
301301
debug!("Area path: {:?}", config.area_path);
302302
debug!("Iteration path: {:?}", config.iteration_path);
303-
debug!("Assignee: {:?}", config.assignee);
303+
debug!("Assignee (config): {:?}", config.assignee);
304+
debug!("Assignee (last author fallback): {:?}", ctx.agent_last_author);
304305

305306
// Validate agent-provided tags against allowed-tags (if configured)
306307
if !self.tags.is_empty() && !config.allowed_tags.is_empty() {
@@ -357,7 +358,7 @@ impl Executor for CreateWorkItemResult {
357358
if let Some(iteration_path) = &config.iteration_path {
358359
patch_doc.push(field_op("System.IterationPath", iteration_path));
359360
}
360-
if let Some(assignee) = &config.assignee {
361+
if let Some(assignee) = config.assignee.as_ref().or(ctx.agent_last_author.as_ref()) {
361362
patch_doc.push(field_op("System.AssignedTo", assignee));
362363
}
363364
// Merge static config tags with validated agent-provided tags (dedup, case-insensitive)

0 commit comments

Comments
 (0)