Skip to content

Commit b318654

Browse files
jamesadevineCopilot
andcommitted
feat: apply max budget enforcement to all safe-output tools
Previously only update-work-item and comment-on-work-item had the max frontmatter option and budget enforcement. This change: - Adds max field (default: 1) to CreateWorkItemConfig, CreatePrConfig, CreateWikiPageConfig, and UpdateWikiPageConfig - Replaces per-tool budget tracking in execute.rs with a generic HashMap-based approach that automatically applies to all budgeted tools - Adds MaxConfig helper for extracting max from any tool config JSON - Adds extract_entry_context() for human-readable log context across all tool types (work item IDs, titles, wiki paths) - Updates AGENTS.md to document max for all safe-output tools - Adds tests for new budget enforcement, context extraction, and MaxConfig deserialization Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 7417ae9 commit b318654

6 files changed

Lines changed: 258 additions & 37 deletions

File tree

AGENTS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -800,6 +800,7 @@ Creates an Azure DevOps work item.
800800
- `assignee` - User to assign (email or display name)
801801
- `tags` - List of tags to apply
802802
- `custom-fields` - Map of custom field reference names to values (e.g., `Custom.MyField: "value"`)
803+
- `max` - Maximum number of create-work-item outputs allowed per run (default: 1)
803804
- `artifact-link` - Configuration for GitHub Copilot artifact linking:
804805
- `enabled` - Whether to add an artifact link (default: false)
805806
- `repository` - Repository name override (defaults to BUILD_REPOSITORY_NAME)
@@ -897,6 +898,7 @@ Note: The source branch name is auto-generated from a sanitized version of the P
897898
- `reviewers` - List of reviewer emails to add
898899
- `labels` - List of labels to apply
899900
- `work-items` - List of work item IDs to link
901+
- `max` - Maximum number of create-pull-request outputs allowed per run (default: 1)
900902
901903
**Multi-repository support:**
902904
When `workspace: root` and multiple repositories are checked out, agents can create PRs for any allowed repository:
@@ -974,6 +976,7 @@ safe-outputs:
974976
path-prefix: "/agent-output" # Optional — prepended to the agent-supplied path (restricts write scope)
975977
title-prefix: "[Agent] " # Optional — prepended to the last path segment (the page title)
976978
comment: "Created by agent" # Optional — default commit comment when agent omits one
979+
max: 1 # Maximum number of create-wiki-page outputs allowed per run (default: 1)
977980
```
978981

979982
Note: `wiki-name` is required. If it is not set, execution fails with an explicit error message.
@@ -995,6 +998,7 @@ safe-outputs:
995998
path-prefix: "/agent-output" # Optional — prepended to the agent-supplied path (restricts write scope)
996999
title-prefix: "[Agent] " # Optional — prepended to the last path segment (the page title)
9971000
comment: "Updated by agent" # Optional — default commit comment when agent omits one
1001+
max: 1 # Maximum number of update-wiki-page outputs allowed per run (default: 1)
9981002
```
9991003

10001004
Note: `wiki-name` is required. If it is not set, execution fails with an explicit error message.

src/execute.rs

Lines changed: 214 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,15 @@
55
66
use anyhow::{Result, bail};
77
use log::{debug, error, info, warn};
8+
use serde::Deserialize;
89
use serde_json::Value;
10+
use std::collections::HashMap;
911
use std::path::Path;
1012

1113
use crate::ndjson::{self, SAFE_OUTPUT_FILENAME};
1214
use crate::tools::{
13-
CommentOnWorkItemConfig, CommentOnWorkItemResult, CreatePrResult, CreateWikiPageResult, CreateWorkItemResult, ExecutionContext, ExecutionResult,
14-
Executor, UpdateWikiPageResult, UpdateWorkItemConfig, UpdateWorkItemResult,
15+
CreatePrResult, CreateWikiPageResult, CreateWorkItemResult, CommentOnWorkItemResult,
16+
ExecutionContext, ExecutionResult, Executor, UpdateWikiPageResult, UpdateWorkItemResult,
1517
};
1618

1719
// Re-export memory types for use by main.rs
@@ -87,15 +89,22 @@ pub async fn execute_safe_outputs(
8789
}
8890
}
8991

90-
// Fetch the update-work-item max once; used to skip excess entries without aborting the batch
91-
let update_wi_config: UpdateWorkItemConfig = ctx.get_tool_config("update-work-item");
92-
let max_update_wi = update_wi_config.max as usize;
93-
let mut update_wi_executed: usize = 0;
94-
95-
// Fetch the comment-on-work-item max once; same skip-and-continue pattern
96-
let comment_wi_config: CommentOnWorkItemConfig = ctx.get_tool_config("comment-on-work-item");
97-
let max_comment_wi = comment_wi_config.max as usize;
98-
let mut comment_wi_executed: usize = 0;
92+
// Build budget map: tool_name → (executed_count, max_allowed).
93+
// All safe-output tools that perform side-effects are budgeted. The `max` field
94+
// is extracted generically from each tool's config JSON (defaulting to 1).
95+
let budgeted_tools = [
96+
"create-work-item",
97+
"create-pull-request",
98+
"update-work-item",
99+
"comment-on-work-item",
100+
"create-wiki-page",
101+
"update-wiki-page",
102+
];
103+
let mut budgets: HashMap<&str, (usize, usize)> = HashMap::new();
104+
for tool_name in &budgeted_tools {
105+
let max_config: MaxConfig = ctx.get_tool_config(tool_name);
106+
budgets.insert(tool_name, (0, max_config.max as usize));
107+
}
99108

100109
let mut results = Vec::new();
101110
for (i, entry) in entries.iter().enumerate() {
@@ -107,35 +116,18 @@ pub async fn execute_safe_outputs(
107116
entry_json
108117
);
109118

110-
// Enforce update-work-item max: skip excess entries rather than aborting the whole batch.
119+
// Generic budget enforcement: skip excess entries rather than aborting the whole batch.
111120
// Budget is consumed before execution so that failed attempts (target policy rejection,
112121
// network errors) still count — this prevents unbounded retries against a failing endpoint.
113-
if entry.get("name").and_then(|n| n.as_str()) == Some("update-work-item") {
114-
let wi_id = entry
115-
.get("id")
116-
.and_then(|v| v.as_u64())
117-
.map(|id| format!(" (work item #{})", id))
118-
.unwrap_or_default();
119-
if let Some(result) = check_budget(entries.len(), i, "update-work-item", &wi_id, update_wi_executed, max_update_wi) {
120-
results.push(result);
121-
continue;
122-
}
123-
update_wi_executed += 1;
124-
}
125-
126-
// Enforce comment-on-work-item max: same skip-and-continue pattern as update-work-item.
127-
// Budget is consumed before execution so that failed attempts still count.
128-
if entry.get("name").and_then(|n| n.as_str()) == Some("comment-on-work-item") {
129-
let wi_id = entry
130-
.get("work_item_id")
131-
.and_then(|v| v.as_i64())
132-
.map(|id| format!(" (work item #{})", id))
133-
.unwrap_or_default();
134-
if let Some(result) = check_budget(entries.len(), i, "comment-on-work-item", &wi_id, comment_wi_executed, max_comment_wi) {
135-
results.push(result);
136-
continue;
122+
if let Some(tool_name) = entry.get("name").and_then(|n| n.as_str()) {
123+
if let Some((executed, max)) = budgets.get_mut(tool_name) {
124+
let context_id = extract_entry_context(entry);
125+
if let Some(result) = check_budget(entries.len(), i, tool_name, &context_id, *executed, *max) {
126+
results.push(result);
127+
continue;
128+
}
129+
*executed += 1;
137130
}
138-
comment_wi_executed += 1;
139131
}
140132

141133
match execute_safe_output(entry, ctx).await {
@@ -284,6 +276,42 @@ pub async fn execute_safe_output(
284276
Ok((tool_name.to_string(), result))
285277
}
286278

279+
/// Helper struct for extracting the `max` field from any tool's config JSON.
280+
/// All safe-output tool configs use `max` with a default of 1.
281+
#[derive(Deserialize)]
282+
struct MaxConfig {
283+
#[serde(default = "default_max")]
284+
max: u32,
285+
}
286+
287+
fn default_max() -> u32 {
288+
1
289+
}
290+
291+
impl Default for MaxConfig {
292+
fn default() -> Self {
293+
Self { max: default_max() }
294+
}
295+
}
296+
297+
/// Extract a human-readable context identifier from a safe-output entry for log messages.
298+
fn extract_entry_context(entry: &Value) -> String {
299+
if let Some(id) = entry.get("id").and_then(|v| v.as_u64()) {
300+
return format!(" (work item #{})", id);
301+
}
302+
if let Some(id) = entry.get("work_item_id").and_then(|v| v.as_i64()) {
303+
return format!(" (work item #{})", id);
304+
}
305+
if let Some(title) = entry.get("title").and_then(|v| v.as_str()) {
306+
let truncated = if title.len() > 40 { &title[..40] } else { title };
307+
return format!(" (\"{}\")", truncated);
308+
}
309+
if let Some(path) = entry.get("path").and_then(|v| v.as_str()) {
310+
return format!(" (path: {})", path);
311+
}
312+
String::new()
313+
}
314+
287315
/// Returns `Some(result)` when the budget for `tool_name` is exhausted so the caller can push the
288316
/// result and `continue` to the next entry. Returns `None` when a budget slot is still available
289317
/// and the caller should proceed with execution.
@@ -735,4 +763,153 @@ mod tests {
735763
let r = result.unwrap();
736764
assert!(r.message.contains("(work item #42)"));
737765
}
766+
767+
// --- extract_entry_context unit tests ---
768+
769+
#[test]
770+
fn test_extract_entry_context_with_id() {
771+
let entry = serde_json::json!({"name": "update-work-item", "id": 42});
772+
assert_eq!(extract_entry_context(&entry), " (work item #42)");
773+
}
774+
775+
#[test]
776+
fn test_extract_entry_context_with_work_item_id() {
777+
let entry = serde_json::json!({"name": "comment-on-work-item", "work_item_id": 99});
778+
assert_eq!(extract_entry_context(&entry), " (work item #99)");
779+
}
780+
781+
#[test]
782+
fn test_extract_entry_context_with_title() {
783+
let entry = serde_json::json!({"name": "create-work-item", "title": "Fix the bug"});
784+
assert_eq!(extract_entry_context(&entry), " (\"Fix the bug\")");
785+
}
786+
787+
#[test]
788+
fn test_extract_entry_context_with_path() {
789+
let entry = serde_json::json!({"name": "create-wiki-page", "path": "/Overview/NewPage"});
790+
assert_eq!(extract_entry_context(&entry), " (path: /Overview/NewPage)");
791+
}
792+
793+
#[test]
794+
fn test_extract_entry_context_empty() {
795+
let entry = serde_json::json!({"name": "noop"});
796+
assert_eq!(extract_entry_context(&entry), "");
797+
}
798+
799+
// --- MaxConfig unit tests ---
800+
801+
#[test]
802+
fn test_max_config_default() {
803+
let config = MaxConfig::default();
804+
assert_eq!(config.max, 1);
805+
}
806+
807+
#[test]
808+
fn test_max_config_from_json_with_max() {
809+
let json = serde_json::json!({"max": 5, "other_field": true});
810+
let config: MaxConfig = serde_json::from_value(json).unwrap();
811+
assert_eq!(config.max, 5);
812+
}
813+
814+
#[test]
815+
fn test_max_config_from_json_without_max() {
816+
let json = serde_json::json!({"other_field": true});
817+
let config: MaxConfig = serde_json::from_value(json).unwrap();
818+
assert_eq!(config.max, 1);
819+
}
820+
821+
// --- Generic budget enforcement for all tool types ---
822+
823+
#[tokio::test]
824+
async fn test_budget_enforcement_create_work_item_max() {
825+
let temp_dir = tempfile::tempdir().unwrap();
826+
let safe_output_path = temp_dir.path().join(SAFE_OUTPUT_FILENAME);
827+
828+
// Write 3 create-work-item entries + 1 noop; max set to 2
829+
let ndjson = r#"{"name":"create-work-item","title":"First item","description":"A description that is definitely longer than thirty characters."}
830+
{"name":"create-work-item","title":"Second item","description":"A description that is definitely longer than thirty characters."}
831+
{"name":"create-work-item","title":"Third item","description":"A description that is definitely longer than thirty characters."}
832+
{"name":"noop","context":"still runs"}
833+
"#;
834+
tokio::fs::write(&safe_output_path, ndjson).await.unwrap();
835+
836+
let mut tool_configs = HashMap::new();
837+
tool_configs.insert("create-work-item".to_string(), serde_json::json!({"max": 2}));
838+
839+
let ctx = ExecutionContext {
840+
ado_org_url: Some("https://dev.azure.com/org".to_string()),
841+
ado_organization: Some("org".to_string()),
842+
ado_project: Some("Proj".to_string()),
843+
access_token: Some("token".to_string()),
844+
working_directory: PathBuf::from("."),
845+
source_directory: PathBuf::from("."),
846+
tool_configs,
847+
repository_id: None,
848+
repository_name: None,
849+
allowed_repositories: HashMap::new(),
850+
};
851+
852+
let results = execute_safe_outputs(temp_dir.path(), &ctx).await;
853+
assert!(results.is_ok(), "Batch should not abort when max is exceeded");
854+
let results = results.unwrap();
855+
assert_eq!(results.len(), 4, "Expected 4 results");
856+
857+
// Only 1 should be skipped (max=2 allows first 2, third is skipped)
858+
let skipped: Vec<_> = results
859+
.iter()
860+
.filter(|r| r.message.contains("maximum create-work-item count"))
861+
.collect();
862+
assert_eq!(skipped.len(), 1, "Expected 1 skipped entry, got: {:?}", skipped);
863+
864+
// noop still runs
865+
assert!(results[3].success, "noop should still succeed");
866+
}
867+
868+
#[tokio::test]
869+
async fn test_budget_enforcement_mixed_tools_independent_budgets() {
870+
let temp_dir = tempfile::tempdir().unwrap();
871+
let safe_output_path = temp_dir.path().join(SAFE_OUTPUT_FILENAME);
872+
873+
// Mix of tools: each has max=1 (default), so only the first of each type should pass budget
874+
let ndjson = r#"{"name":"create-work-item","title":"WI 1","description":"A description that is definitely longer than thirty characters."}
875+
{"name":"create-work-item","title":"WI 2","description":"A description that is definitely longer than thirty characters."}
876+
{"name":"create-wiki-page","path":"/Page1","content":"Some valid wiki content here."}
877+
{"name":"create-wiki-page","path":"/Page2","content":"Some valid wiki content here."}
878+
{"name":"noop","context":"always runs"}
879+
"#;
880+
tokio::fs::write(&safe_output_path, ndjson).await.unwrap();
881+
882+
let ctx = ExecutionContext {
883+
ado_org_url: Some("https://dev.azure.com/org".to_string()),
884+
ado_organization: Some("org".to_string()),
885+
ado_project: Some("Proj".to_string()),
886+
access_token: Some("token".to_string()),
887+
working_directory: PathBuf::from("."),
888+
source_directory: PathBuf::from("."),
889+
tool_configs: HashMap::new(), // defaults: max=1 for all
890+
repository_id: None,
891+
repository_name: None,
892+
allowed_repositories: HashMap::new(),
893+
};
894+
895+
let results = execute_safe_outputs(temp_dir.path(), &ctx).await.unwrap();
896+
assert_eq!(results.len(), 5);
897+
898+
// Second create-work-item should be skipped
899+
let cwi_skipped: Vec<_> = results
900+
.iter()
901+
.filter(|r| r.message.contains("maximum create-work-item count"))
902+
.collect();
903+
assert_eq!(cwi_skipped.len(), 1, "Expected 1 skipped create-work-item");
904+
905+
// Second create-wiki-page should be skipped
906+
let cwp_skipped: Vec<_> = results
907+
.iter()
908+
.filter(|r| r.message.contains("maximum create-wiki-page count"))
909+
.collect();
910+
assert_eq!(cwp_skipped.len(), 1, "Expected 1 skipped create-wiki-page");
911+
912+
// noop always runs
913+
assert!(results[4].success, "noop should still succeed");
914+
}
738915
}

src/tools/create_pr.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,10 @@ pub struct CreatePrConfig {
261261
/// Work item IDs to link to the PR
262262
#[serde(default, rename = "work-items")]
263263
pub work_items: Vec<i32>,
264+
265+
/// Maximum number of create-pull-request outputs allowed per pipeline run (default: 1)
266+
#[serde(default = "default_max")]
267+
pub max: u32,
264268
}
265269

266270
fn default_target_branch() -> String {
@@ -271,6 +275,10 @@ fn default_true() -> bool {
271275
true
272276
}
273277

278+
fn default_max() -> u32 {
279+
1
280+
}
281+
274282
impl Default for CreatePrConfig {
275283
fn default() -> Self {
276284
Self {
@@ -281,6 +289,7 @@ impl Default for CreatePrConfig {
281289
reviewers: Vec::new(),
282290
labels: Vec::new(),
283291
work_items: Vec::new(),
292+
max: default_max(),
284293
}
285294
}
286295
}
@@ -1119,6 +1128,7 @@ mod tests {
11191128
assert!(!config.auto_complete);
11201129
assert!(config.delete_source_branch);
11211130
assert!(config.squash_merge);
1131+
assert_eq!(config.max, 1);
11221132
}
11231133

11241134
#[test]

src/tools/create_wiki_page.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,14 @@ pub struct CreateWikiPageConfig {
123123
/// Default commit comment used when the agent does not supply one.
124124
#[serde(default)]
125125
pub comment: Option<String>,
126+
127+
/// Maximum number of create-wiki-page outputs allowed per pipeline run (default: 1)
128+
#[serde(default = "default_max")]
129+
pub max: u32,
130+
}
131+
132+
fn default_max() -> u32 {
133+
1
126134
}
127135

128136
impl Default for CreateWikiPageConfig {
@@ -133,6 +141,7 @@ impl Default for CreateWikiPageConfig {
133141
path_prefix: None,
134142
title_prefix: None,
135143
comment: None,
144+
max: default_max(),
136145
}
137146
}
138147
}
@@ -510,6 +519,7 @@ mod tests {
510519
assert!(config.path_prefix.is_none());
511520
assert!(config.title_prefix.is_none());
512521
assert!(config.comment.is_none());
522+
assert_eq!(config.max, 1);
513523
}
514524

515525
#[test]

0 commit comments

Comments
 (0)