Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 19 additions & 15 deletions src/compile/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,7 @@ const DEFAULT_BASH_COMMANDS: &[&str] = &[

/// Generate copilot CLI params from front matter configuration
pub fn generate_copilot_params(front_matter: &FrontMatter) -> String {
let mut allowed_tools: Vec<String> = vec![
"github".to_string(),
"safeoutputs".to_string(),
];
let mut allowed_tools: Vec<String> = vec!["github".to_string(), "safeoutputs".to_string()];

// Edit tool: enabled by default, can be disabled with `edit: false`
let edit_enabled = front_matter
Expand Down Expand Up @@ -515,10 +512,7 @@ pub fn generate_acquire_ado_token(service_connection: Option<&str>, variable_nam
lines.push(" addSpnToEnvironment: true".to_string());
lines.push(" inlineScript: |".to_string());
lines.push(" ADO_TOKEN=$(az account get-access-token \\".to_string());
lines.push(format!(
" --resource {} \\",
ADO_RESOURCE_ID
));
lines.push(format!(" --resource {} \\", ADO_RESOURCE_ID));
lines.push(" --query accessToken -o tsv)".to_string());
lines.push(format!(
" echo \"##vso[task.setvariable variable={variable_name};issecret=true]$ADO_TOKEN\""
Expand All @@ -534,10 +528,8 @@ pub fn generate_acquire_ado_token(service_connection: Option<&str>, variable_nam
/// When not configured, omits ADO access tokens entirely.
pub fn generate_copilot_ado_env(read_service_connection: Option<&str>) -> String {
match read_service_connection {
Some(_) => {
"AZURE_DEVOPS_EXT_PAT: $(SC_READ_TOKEN)\nSYSTEM_ACCESSTOKEN: $(SC_READ_TOKEN)"
.to_string()
}
Some(_) => "AZURE_DEVOPS_EXT_PAT: $(SC_READ_TOKEN)\nSYSTEM_ACCESSTOKEN: $(SC_READ_TOKEN)"
.to_string(),
None => String::new(),
}
}
Expand All @@ -553,7 +545,13 @@ pub fn generate_executor_ado_env(write_service_connection: Option<&str>) -> Stri
}

/// Safe-output names that require write access to ADO.
const WRITE_REQUIRING_SAFE_OUTPUTS: &[&str] = &["create-pull-request", "create-work-item", "create-wiki-page", "update-wiki-page"];
const WRITE_REQUIRING_SAFE_OUTPUTS: &[&str] = &[
"create-pull-request",
"create-work-item",
"update-work-item",
"create-wiki-page",
"update-wiki-page",
];

/// Validate that write-requiring safe-outputs have a write service connection configured.
pub fn validate_write_permissions(front_matter: &FrontMatter) -> Result<()> {
Expand Down Expand Up @@ -760,7 +758,10 @@ mod tests {
#[test]
fn test_generate_pr_trigger_no_triggers_no_schedule() {
let result = generate_pr_trigger(&None, false);
assert!(result.is_empty(), "Should be empty when no triggers configured");
assert!(
result.is_empty(),
"Should be empty when no triggers configured"
);
}

#[test]
Expand Down Expand Up @@ -804,7 +805,10 @@ mod tests {
#[test]
fn test_generate_ci_trigger_no_triggers_no_schedule() {
let result = generate_ci_trigger(&None, false);
assert!(result.is_empty(), "Should be empty when no triggers configured");
assert!(
result.is_empty(),
"Should be empty when no triggers configured"
);
}

#[test]
Expand Down
111 changes: 109 additions & 2 deletions src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use std::path::Path;

use crate::ndjson::{self, SAFE_OUTPUT_FILENAME};
use crate::tools::{
CreatePrResult, CreateWikiPageResult, CreateWorkItemResult, UpdateWikiPageResult,
ExecutionContext, ExecutionResult, Executor,
CreatePrResult, CreateWikiPageResult, CreateWorkItemResult, ExecutionContext, ExecutionResult,
Executor, UpdateWikiPageResult, UpdateWorkItemConfig, UpdateWorkItemResult,
};

// Re-export memory types for use by main.rs
Expand Down Expand Up @@ -87,6 +87,11 @@ pub async fn execute_safe_outputs(
}
}

// Fetch the update-work-item max once; used to skip excess entries without aborting the batch
let update_wi_config: UpdateWorkItemConfig = ctx.get_tool_config("update-work-item");
let max_update_wi = update_wi_config.max as usize;
let mut update_wi_executed: usize = 0;

let mut results = Vec::new();
for (i, entry) in entries.iter().enumerate() {
let entry_json = serde_json::to_string(entry).unwrap_or_else(|_| "<invalid>".to_string());
Expand All @@ -97,6 +102,38 @@ pub async fn execute_safe_outputs(
entry_json
);

// Enforce update-work-item max: skip excess entries rather than aborting the whole batch
if entry.get("name").and_then(|n| n.as_str()) == Some("update-work-item") {
if update_wi_executed >= max_update_wi {
let wi_id = entry
.get("id")
.and_then(|v| v.as_u64())
.map(|id| format!(" (work item #{})", id))
.unwrap_or_default();
warn!(
"[{}/{}] Skipping update-work-item{} entry: max ({}) already reached for this run",
i + 1,
entries.len(),
wi_id,
max_update_wi
);
let result = ExecutionResult::failure(format!(
"Skipped{}: maximum update-work-item count ({}) already reached. \
Increase 'max' in safe-outputs.update-work-item to allow more updates.",
wi_id, max_update_wi
));
println!(
"[{}/{}] update-work-item - ✗ - {}",
i + 1,
entries.len(),
result.message
);
results.push(result);
continue;
}
update_wi_executed += 1;
}

match execute_safe_output(entry, ctx).await {
Ok((tool_name, result)) => {
if result.success {
Expand Down Expand Up @@ -172,6 +209,13 @@ pub async fn execute_safe_output(
);
output.execute_sanitized(ctx).await?
}
"update-work-item" => {
debug!("Parsing update-work-item payload");
let mut output: UpdateWorkItemResult = serde_json::from_value(entry.clone())
.map_err(|e| anyhow::anyhow!("Failed to parse update-work-item: {}", e))?;
debug!("update-work-item: id={}", output.id);
output.execute_sanitized(ctx).await?
}
"create-pull-request" => {
debug!("Parsing create-pull-request payload");
let mut output: CreatePrResult = serde_json::from_value(entry.clone())
Expand Down Expand Up @@ -478,4 +522,67 @@ mod tests {
.contains("AZURE_DEVOPS_ORG_URL")
);
}

/// Excess update-work-item entries beyond `max` are skipped (failure result added) rather than
/// aborting the entire batch. Other tool entries must still execute.
#[tokio::test]
async fn test_execute_update_work_item_max_skips_excess_not_abort() {
let temp_dir = tempfile::tempdir().unwrap();
let safe_output_path = temp_dir.path().join(SAFE_OUTPUT_FILENAME);

// Write 3 update-work-item entries + 1 noop; max defaults to 1
let ndjson = r#"{"name":"update-work-item","id":1,"title":"First update"}
{"name":"update-work-item","id":2,"title":"Second update"}
{"name":"update-work-item","id":3,"title":"Third update"}
{"name":"noop","context":"still runs"}
"#;
tokio::fs::write(&safe_output_path, ndjson).await.unwrap();

// Config: update-work-item with max=1 (default), title=true so the field check passes
let update_cfg = serde_json::json!({
"title": true,
"max": 1
});
let mut tool_configs = HashMap::new();
tool_configs.insert("update-work-item".to_string(), update_cfg);

let ctx = ExecutionContext {
ado_org_url: Some("https://dev.azure.com/org".to_string()),
ado_organization: Some("org".to_string()),
ado_project: Some("Proj".to_string()),
access_token: Some("token".to_string()),
working_directory: PathBuf::from("."),
source_directory: PathBuf::from("."),
tool_configs,
repository_id: None,
repository_name: None,
allowed_repositories: HashMap::new(),
};

let results = execute_safe_outputs(temp_dir.path(), &ctx).await;
// The batch must NOT abort — execute_safe_outputs should return Ok
assert!(
results.is_ok(),
"Batch should not abort when max is exceeded; got: {:?}",
results
);
let results = results.unwrap();
// 4 entries total: 3 update-work-item + 1 noop
assert_eq!(results.len(), 4, "Expected 4 results (3 uwi + 1 noop)");

// The first update-work-item fails with HTTP error (no real ADO) but was attempted
// The 2nd and 3rd are skipped due to max
let skipped: Vec<_> = results
.iter()
.filter(|r| r.message.contains("maximum update-work-item count"))
.collect();
assert_eq!(skipped.len(), 2, "Expected 2 skipped entries, got: {:?}", skipped);

// The noop still executes successfully
let noop_result = &results[3];
assert!(
noop_result.success,
"noop should still succeed even when prior entries are skipped"
);
}
}
34 changes: 34 additions & 0 deletions src/mcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use crate::tools::{
CreateWorkItemParams, CreateWorkItemResult,
UpdateWikiPageParams, UpdateWikiPageResult, MissingDataParams, MissingDataResult,
MissingToolParams, MissingToolResult, NoopParams, NoopResult, ToolResult,
UpdateWorkItemParams, UpdateWorkItemResult,
anyhow_to_mcp_error,
};

Expand Down Expand Up @@ -335,6 +336,39 @@ impl SafeOutputs {
Ok(CallToolResult::success(vec![]))
}

#[tool(
name = "update-work-item",
description = "Update an existing Azure DevOps work item. Only fields explicitly enabled \
in the pipeline configuration (safe-outputs.update-work-item) may be changed. Updates may be \
further restricted by target (only a specific work item ID) or title-prefix (only work items \
whose current title starts with a configured prefix). Provide the work item ID and only the \
fields you want to update."
)]
async fn update_work_item(
&self,
params: Parameters<UpdateWorkItemParams>,
) -> Result<CallToolResult, McpError> {
info!("Tool called: update-work-item - id={}", params.0.id);
// Sanitize untrusted agent-provided text fields (IS-01)
let mut sanitized = params.0;
sanitized.title = sanitized.title.map(|t| sanitize_text(&t));
sanitized.body = sanitized.body.map(|b| sanitize_text(&b));
sanitized.state = sanitized.state.map(|s| sanitize_text(&s));
sanitized.area_path = sanitized.area_path.map(|p| sanitize_text(&p));
sanitized.iteration_path = sanitized.iteration_path.map(|p| sanitize_text(&p));
sanitized.assignee = sanitized.assignee.map(|a| sanitize_text(&a));
sanitized.tags = sanitized
.tags
.map(|ts| ts.into_iter().map(|t| sanitize_text(&t)).collect());
let result: UpdateWorkItemResult = sanitized.try_into()?;
let _ = self.write_safe_output_file(&result).await;
info!("Work item update queued for #{}", result.id);
Ok(CallToolResult::success(vec![Content::text(format!(
"Work item #{} update queued. Changes will be applied during safe output processing.",
result.id
))]))
}

#[tool(
name = "create-pull-request",
description = "Create a new pull request to propose code changes. Use this after making file edits to submit them for review and merging. The PR will be created from the current branch with your committed changes. Use 'self' for the pipeline's own repository, or a repository alias from the checkout list."
Expand Down
2 changes: 2 additions & 0 deletions src/tools/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ mod missing_data;
mod missing_tool;
mod noop;
mod result;
mod update_work_item;

pub use create_pr::*;
pub use create_wiki_page::*;
Expand All @@ -29,3 +30,4 @@ pub use noop::*;
pub use result::{
ExecutionContext, ExecutionResult, Executor, ToolResult, Validate, anyhow_to_mcp_error,
};
pub use update_work_item::*;
Loading