diff --git a/AGENTS.md b/AGENTS.md index c4a2c493..cd653834 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,19 +37,33 @@ Alongside the correctly generated pipeline yaml, an agent file is generated from │ ├── ndjson.rs # NDJSON parsing utilities │ ├── proxy.rs # Network proxy implementation │ ├── sanitize.rs # Input sanitization for safe outputs -│ └── tools/ # MCP tool implementations +│ ├── safeoutputs/ # Safe-output MCP tool implementations (Stage 1 → NDJSON → Stage 2) +│ │ ├── mod.rs +│ │ ├── add_build_tag.rs +│ │ ├── add_pr_comment.rs +│ │ ├── comment_on_work_item.rs +│ │ ├── create_branch.rs +│ │ ├── create_git_tag.rs +│ │ ├── create_pr.rs +│ │ ├── create_wiki_page.rs +│ │ ├── create_work_item.rs +│ │ ├── link_work_items.rs +│ │ ├── missing_data.rs +│ │ ├── missing_tool.rs +│ │ ├── noop.rs +│ │ ├── queue_build.rs +│ │ ├── reply_to_pr_comment.rs +│ │ ├── report_incomplete.rs +│ │ ├── resolve_pr_thread.rs +│ │ ├── result.rs +│ │ ├── submit_pr_review.rs +│ │ ├── update_pr.rs +│ │ ├── update_wiki_page.rs +│ │ ├── update_work_item.rs +│ │ └── upload_attachment.rs +│ └── tools/ # First-class tool implementations (compiler auto-configures) │ ├── mod.rs -│ ├── comment_on_work_item.rs -│ ├── create_pr.rs -│ ├── create_wiki_page.rs -│ ├── create_work_item.rs -│ ├── update_work_item.rs -│ ├── update_wiki_page.rs -│ ├── memory.rs -│ ├── missing_data.rs -│ ├── missing_tool.rs -│ ├── noop.rs -│ └── result.rs +│ └── cache_memory.rs ├── templates/ │ ├── base.yml # Base pipeline template for standalone │ ├── 1es-base.yml # Base pipeline template for 1ES target @@ -125,6 +139,14 @@ checkout: # optional list of repository aliases for the agent to checkout and wo tools: # optional tool configuration bash: ["cat", "ls", "grep"] # bash command allow-list (defaults to safe built-in list) edit: true # enable file editing tool (default: true) + cache-memory: true # persistent memory across runs (see Cache Memory section) + # cache-memory: # Alternative object format (with options) + # allowed-extensions: [.md, .json] + azure-devops: true # first-class ADO MCP integration (see Azure DevOps MCP section) + # azure-devops: # Alternative object format (with scoping) + # toolsets: [repos, wit] + # allowed: [wit_get_work_item] + # org: myorg # env: # RESERVED: workflow-level environment variables (not yet implemented) # CUSTOM_VAR: "value" mcp-servers: @@ -392,6 +414,53 @@ tools: edit: false ``` +#### Cache Memory (`cache-memory:`) + +Persistent memory storage across agent runs. The agent reads/writes files to a memory directory that persists between pipeline executions via pipeline artifacts. + +```yaml +# Simple enablement +tools: + cache-memory: true + +# With options +tools: + cache-memory: + allowed-extensions: [.md, .json, .txt] +``` + +When enabled, the compiler auto-generates pipeline steps to: +- Download previous memory from the last successful run's artifact +- Restore files to `/tmp/awf-tools/staging/agent_memory/` +- Append a memory prompt to the agent instructions +- Auto-inject a `clearMemory` pipeline parameter (allows clearing memory from the ADO UI) + +During Stage 2 execution, memory files are validated (path safety, extension filtering, `##vso[` injection detection, 5 MB size limit) and published as a pipeline artifact. + +#### Azure DevOps MCP (`azure-devops:`) + +First-class Azure DevOps MCP integration. Auto-configures the ADO MCP container, token mapping, MCPG entry, and network allowlist. + +```yaml +# Simple enablement (auto-infers org from git remote) +tools: + azure-devops: true + +# With scoping options +tools: + azure-devops: + toolsets: [repos, wit, core] # ADO API toolset groups + allowed: [wit_get_work_item, core_list_projects] # Explicit tool allow-list + org: myorg # Optional override (inferred from git remote) +``` + +When enabled, the compiler: +- Generates a containerized stdio MCP entry (`node:20-slim` + `npx @azure-devops/mcp`) in the MCPG config +- Auto-maps `AZURE_DEVOPS_EXT_PAT` token passthrough when `permissions.read` is configured +- Adds ADO-specific hosts to the network allowlist +- Auto-infers org from the git remote URL at compile time (overridable via `org:` field) +- Fails compilation if org cannot be determined (no explicit override and no ADO git remote) + ### Target Platforms The `target` field in the front matter determines the output format and execution environment for the compiled pipeline. @@ -1066,35 +1135,8 @@ 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 -#### memory -Provides persistent memory across agent runs. When enabled, the agent can read and write files to a memory directory that persists between pipeline executions. - -**Configuration options (front matter):** -```yaml -safe-outputs: - memory: - allowed-extensions: # Optional: restrict file types (defaults to all) - - .md - - .json - - .txt -``` - -**How it works:** -1. During Stage 1 (agent execution), the agent can write files to `/tmp/awf-tools/staging/agent_memory/` -2. A prompt is automatically appended to inform the agent about its memory location -3. During Stage 2 execution, memory files are validated and sanitized: - - Path traversal attempts are blocked - - Files are checked for `##vso[` command injection - - Total size is limited to 5 MB - - File extensions can be restricted via configuration -4. Sanitized memory files are published as a pipeline artifact -5. On the next run, the previous memory is downloaded and restored to the staging directory - -**Security validations:** -- Maximum total memory size: 5 MB -- Path validation: no `..`, `.git`, absolute paths, or null bytes -- Content validation: text files are scanned for `##vso[` commands -- Extension filtering: can restrict to specific file types +#### cache-memory (moved to `tools:`) +Memory is now configured as a first-class tool under `tools: cache-memory:` instead of `safe-outputs: memory:`. See the [Cache Memory](#cache-memory-cache-memory) section under Tools Configuration for details. #### create-wiki-page Creates a new Azure DevOps wiki page. The page must **not** already exist; the tool enforces an atomic create-only operation (via `If-Match: ""`). Attempting to create a page that already exists results in an explicit failure. @@ -1154,7 +1196,9 @@ When extending the compiler: 2. **New compile targets**: Implement the `Compiler` trait in a new file under `src/compile/` 3. **New front matter fields**: Add fields to `FrontMatter` in `src/compile/types.rs` 4. **New template markers**: Handle replacements in the target-specific compiler (e.g., `standalone.rs` or `onees.rs`) -5. **Validation**: Add compile-time validation for safe outputs and permissions +5. **New safe-output tools**: Add to `src/safeoutputs/` — implement `ToolResult`, `Executor`, register in `mod.rs`, `mcp.rs`, `execute.rs` +6. **New first-class tools**: Add to `src/tools/` — extend `ToolsConfig` in `types.rs`, wire in compilers +7. **Validation**: Add compile-time validation for safe outputs and permissions ### Security Considerations diff --git a/examples/azure-devops-mcp.md b/examples/azure-devops-mcp.md index 2388f483..5c717097 100644 --- a/examples/azure-devops-mcp.md +++ b/examples/azure-devops-mcp.md @@ -3,13 +3,9 @@ name: "ADO Work Item Triage" description: "Triages work items using the Azure DevOps MCP" schedule: daily around 9:00 engine: claude-sonnet-4.5 -mcp-servers: +tools: azure-devops: - container: "node:20-slim" - entrypoint: "npx" - entrypoint-args: ["-y", "@azure-devops/mcp", "myorg", "-d", "core", "work", "work-items"] - env: - AZURE_DEVOPS_EXT_PAT: "" + toolsets: [core, work, work-items] allowed: - core_list_projects - wit_my_work_items @@ -31,11 +27,6 @@ safe-outputs: comment-on-work-item: max: 10 target: "*" -network: - allow: - - "dev.azure.com" - - "*.dev.azure.com" - - "*.visualstudio.com" --- ## ADO Work Item Triage Agent diff --git a/src/compile/common.rs b/src/compile/common.rs index 69249a42..ff20b52e 100644 --- a/src/compile/common.rs +++ b/src/compile/common.rs @@ -625,9 +625,25 @@ pub fn generate_header_comment(input_path: &std::path::Path) -> String { /// See: https://github.com/github/gh-aw-mcpg/releases pub const MCPG_VERSION: &str = "0.2.18"; +/// Docker image for the MCPG container. +pub const MCPG_IMAGE: &str = "ghcr.io/github/gh-aw-mcpg"; + /// Default port MCPG listens on inside the container (host network mode). pub const MCPG_PORT: u16 = 80; +/// Docker image for the Azure DevOps MCP container. +/// This is the container used when `tools: azure-devops:` is configured. +pub const ADO_MCP_IMAGE: &str = "node:20-slim"; + +/// Default entrypoint for the Azure DevOps MCP container. +pub const ADO_MCP_ENTRYPOINT: &str = "npx"; + +/// Default entrypoint args for the Azure DevOps MCP npm package. +pub const ADO_MCP_PACKAGE: &str = "@azure-devops/mcp"; + +/// Reserved MCPG server name for the auto-configured ADO MCP. +pub const ADO_MCP_SERVER_NAME: &str = "azure-devops"; + /// Generate source path for the execute command. /// /// Returns a path using `{{ workspace }}` as the base, which gets resolved @@ -805,14 +821,11 @@ fn is_safe_tool_name(name: &str) -> bool { /// diagnostic tools. If `safe-outputs:` is empty, returns an empty string /// (all tools enabled for backward compatibility). /// -/// Non-MCP keys (like `memory`) are silently skipped — they are handled by the -/// executor and have no corresponding MCP route. -/// /// Tool names are validated to contain only ASCII alphanumerics and hyphens /// to prevent shell injection when the args are embedded in bash commands. /// Unrecognized tool names emit a compile-time warning and are skipped. pub fn generate_enabled_tools_args(front_matter: &FrontMatter) -> String { - use crate::tools::{ALL_KNOWN_SAFE_OUTPUTS, ALWAYS_ON_TOOLS, NON_MCP_SAFE_OUTPUT_KEYS}; + use crate::safeoutputs::{ALL_KNOWN_SAFE_OUTPUTS, ALWAYS_ON_TOOLS, NON_MCP_SAFE_OUTPUT_KEYS}; use std::collections::HashSet; if front_matter.safe_outputs.is_empty() { @@ -835,6 +848,14 @@ pub fn generate_enabled_tools_args(front_matter: &FrontMatter) -> String { if NON_MCP_SAFE_OUTPUT_KEYS.contains(&key.as_str()) { continue; } + if key == "memory" { + eprintln!( + "Warning: Agent '{}': 'safe-outputs: memory:' has moved to \ + 'tools: cache-memory:'. Update your front matter to restore memory support.", + front_matter.name + ); + continue; + } if !ALL_KNOWN_SAFE_OUTPUTS.contains(&key.as_str()) { eprintln!( "Warning: unrecognized safe-output tool '{}' — skipping (no registered tool matches this name)", @@ -849,8 +870,8 @@ pub fn generate_enabled_tools_args(front_matter: &FrontMatter) -> String { } if effective_mcp_tool_count == 0 { - // Every user-specified key was either invalid, unrecognized, or non-MCP - // (e.g. memory-only). Return empty to keep all tools available (backward compat). + // Every user-specified key was either invalid or unrecognized. + // Return empty to keep all tools available (backward compat). return String::new(); } @@ -878,7 +899,7 @@ pub fn generate_enabled_tools_args(front_matter: &FrontMatter) -> String { /// Validate that write-requiring safe-outputs have a write service connection configured. pub fn validate_write_permissions(front_matter: &FrontMatter) -> Result<()> { - use crate::tools::WRITE_REQUIRING_SAFE_OUTPUTS; + use crate::safeoutputs::WRITE_REQUIRING_SAFE_OUTPUTS; let has_write_sc = front_matter .permissions @@ -1170,6 +1191,8 @@ mod tests { fm.tools = Some(crate::compile::types::ToolsConfig { bash: Some(vec![":*".to_string()]), edit: None, + cache_memory: None, + azure_devops: None, }); let params = generate_copilot_params(&fm); assert!(params.contains("--allow-tool \"shell(:*)\"")); @@ -1181,6 +1204,8 @@ mod tests { fm.tools = Some(crate::compile::types::ToolsConfig { bash: Some(vec![]), edit: None, + cache_memory: None, + azure_devops: None, }); let params = generate_copilot_params(&fm); assert!(!params.contains("shell(")); @@ -1886,26 +1911,26 @@ mod tests { } #[test] - fn test_generate_enabled_tools_args_skips_memory_key() { - // `memory` is a non-MCP executor-only key. It must not appear in - // --enabled-tools or it would cause real MCP tools to be filtered out. + fn test_generate_enabled_tools_args_memory_no_longer_safe_output() { + // `memory` is no longer a safe-output key — it moved to `tools: cache-memory:`. + // If someone still puts it in safe-outputs, it should be treated as unrecognized + // and the real MCP tool should still be present. let (fm, _) = parse_markdown( - "---\nname: test\ndescription: test\nsafe-outputs:\n memory:\n allowed-extensions:\n - .md\n create-pull-request:\n target-branch: main\n---\n" + "---\nname: test\ndescription: test\nsafe-outputs:\n create-pull-request:\n target-branch: main\n---\n" ).unwrap(); let args = generate_enabled_tools_args(&fm); - assert!(!args.contains("--enabled-tools memory"), "Non-MCP key 'memory' should be skipped"); assert!(args.contains("--enabled-tools create-pull-request"), "Real MCP tool should be present"); } #[test] - fn test_generate_enabled_tools_args_memory_only_does_not_filter() { - // When `memory` is the only safe-output key, no --enabled-tools args should - // be generated so all tools remain available (backward compat). + fn test_generate_enabled_tools_args_empty_safe_outputs_no_filter() { + // When safe-outputs is empty, no --enabled-tools args should be generated + // so all tools remain available. let (fm, _) = parse_markdown( - "---\nname: test\ndescription: test\nsafe-outputs:\n memory:\n allowed-extensions:\n - .md\n---\n" + "---\nname: test\ndescription: test\n---\n" ).unwrap(); let args = generate_enabled_tools_args(&fm); - assert!(args.is_empty(), "memory-only safe-outputs should produce no args (all tools available)"); + assert!(args.is_empty(), "empty safe-outputs should produce no args (all tools available)"); } // ─── parameter name validation ────────────────────────────────────────── diff --git a/src/compile/onees.rs b/src/compile/onees.rs index ce846ca5..a20ab8d2 100644 --- a/src/compile/onees.rs +++ b/src/compile/onees.rs @@ -62,7 +62,11 @@ impl Compiler for OneESCompiler { let checkout_steps = generate_checkout_steps(&front_matter.checkout); let checkout_self = generate_checkout_self(); let copilot_params = generate_copilot_params(front_matter); - let has_memory = front_matter.safe_outputs.contains_key("memory"); + let has_memory = front_matter + .tools + .as_ref() + .and_then(|t| t.cache_memory.as_ref()) + .is_some_and(|cm| cm.is_enabled()); let parameters = build_parameters(&front_matter.parameters, has_memory); let parameters_yaml = generate_parameters(¶meters)?; diff --git a/src/compile/standalone.rs b/src/compile/standalone.rs index defd25da..d9799ca8 100644 --- a/src/compile/standalone.rs +++ b/src/compile/standalone.rs @@ -14,7 +14,8 @@ use std::path::Path; use super::Compiler; use super::common::{ - self, AWF_VERSION, COPILOT_CLI_VERSION, DEFAULT_POOL, MCPG_PORT, MCPG_VERSION, + self, AWF_VERSION, COPILOT_CLI_VERSION, DEFAULT_POOL, MCPG_PORT, MCPG_VERSION, MCPG_IMAGE, + ADO_MCP_IMAGE, ADO_MCP_ENTRYPOINT, ADO_MCP_PACKAGE, ADO_MCP_SERVER_NAME, build_parameters, compute_effective_workspace, generate_acquire_ado_token, generate_cancel_previous_builds, generate_checkout_self, generate_checkout_steps, generate_ci_trigger, generate_copilot_ado_env, generate_copilot_params, @@ -99,7 +100,11 @@ impl Compiler for StandaloneCompiler { // Generate hooks let setup_job = generate_setup_job(&front_matter.setup, &front_matter.name, &pool); let teardown_job = generate_teardown_job(&front_matter.teardown, &front_matter.name, &pool); - let has_memory = front_matter.safe_outputs.contains_key("memory"); + let has_memory = front_matter + .tools + .as_ref() + .and_then(|t| t.cache_memory.as_ref()) + .is_some_and(|cm| cm.is_enabled()); // Build parameters list: user-defined + auto-injected clearMemory for memory let parameters = build_parameters(&front_matter.parameters, has_memory); @@ -168,6 +173,7 @@ impl Compiler for StandaloneCompiler { ("{{ compiler_version }}", compiler_version), ("{{ firewall_version }}", AWF_VERSION), ("{{ mcpg_version }}", MCPG_VERSION), + ("{{ mcpg_image }}", MCPG_IMAGE), ("{{ copilot_version }}", COPILOT_CLI_VERSION), ("{{ pool }}", &pool), ("{{ setup_job }}", &setup_job), @@ -207,9 +213,48 @@ impl Compiler for StandaloneCompiler { replace_with_indent(&yaml, placeholder, replacement) }); + // Infer ADO org from git remote at compile time (for tools.azure-devops) + let inferred_org = if front_matter + .tools + .as_ref() + .and_then(|t| t.azure_devops.as_ref()) + .is_some_and(|ado| ado.is_enabled() && ado.org().is_none()) + { + let input_dir = input_path.parent().unwrap_or(std::path::Path::new(".")); + match crate::configure::get_git_remote_url(input_dir).await { + Ok(url) => match crate::configure::parse_ado_remote(&url) { + Ok(ctx) => { + let org = ctx + .org_url + .trim_end_matches('/') + .rsplit('/') + .next() + .unwrap_or("") + .to_string(); + if org.is_empty() { + None + } else { + info!("Inferred ADO org '{}' from git remote", org); + Some(org) + } + } + Err(_) => { + log::debug!("Git remote is not an ADO URL — cannot infer org"); + None + } + }, + Err(_) => { + log::debug!("No git remote found — cannot infer org"); + None + } + } + } else { + None + }; + // Always generate MCPG config — safeoutputs is always required regardless // of whether additional mcp-servers are configured in front matter. - let config = generate_mcpg_config(front_matter); + let config = generate_mcpg_config(front_matter, inferred_org.as_deref())?; let mcpg_config_json = serde_json::to_string_pretty(&config).context("Failed to serialize MCPG config")?; @@ -279,6 +324,18 @@ fn generate_allowed_domains(front_matter: &FrontMatter) -> String { } } + // Add ADO-specific hosts when tools.azure-devops is enabled + if front_matter + .tools + .as_ref() + .and_then(|t| t.azure_devops.as_ref()) + .is_some_and(|ado| ado.is_enabled()) + { + for host in mcp_required_hosts("ado") { + hosts.insert((*host).to_string()); + } + } + // Add user-specified hosts for host in &user_hosts { hosts.insert(host.clone()); @@ -443,7 +500,14 @@ pub struct McpgConfig { /// Converts the front matter `mcp-servers` definitions into MCPG-compatible JSON. /// SafeOutputs is always included as an HTTP backend. Custom MCPs with explicit /// `command:` are included as stdio servers. -pub fn generate_mcpg_config(front_matter: &FrontMatter) -> McpgConfig { +/// +/// `inferred_org` is the ADO organization name extracted from the git remote URL +/// at compile time. Used as the default org for `tools.azure-devops` when no +/// explicit `org:` override is provided. +/// +/// Returns an error if `tools.azure-devops` is enabled but no org can be determined +/// (neither explicit override nor git remote inference). +pub fn generate_mcpg_config(front_matter: &FrontMatter, inferred_org: Option<&str>) -> Result { let mut mcp_servers = HashMap::new(); // SafeOutputs is always included as an HTTP backend. @@ -469,6 +533,93 @@ pub fn generate_mcpg_config(front_matter: &FrontMatter) -> McpgConfig { }, ); + // Auto-configure ADO MCP when tools.azure-devops is enabled. + // This generates a containerized stdio MCP entry using the ADO MCP npm package. + if let Some(ado_config) = front_matter + .tools + .as_ref() + .and_then(|t| t.azure_devops.as_ref()) + { + if ado_config.is_enabled() { + // Warn if user also has a manual mcp-servers entry for azure-devops + if front_matter.mcp_servers.contains_key(ADO_MCP_SERVER_NAME) { + eprintln!( + "Warning: Agent '{}' has both tools.azure-devops and mcp-servers.azure-devops configured. \ + The tools.azure-devops auto-configuration takes precedence. \ + Remove the mcp-servers entry to silence this warning.", + front_matter.name + ); + } + + // Build entrypoint args: npx -y @azure-devops/mcp [-d toolset1 toolset2 ...] + let mut entrypoint_args = vec!["-y".to_string(), ADO_MCP_PACKAGE.to_string()]; + + // Org: use explicit override, then compile-time inferred, then fail + let org = if let Some(explicit) = ado_config.org() { + explicit.to_string() + } else if let Some(inferred) = inferred_org { + inferred.to_string() + } else { + anyhow::bail!( + "Agent '{}' has tools.azure-devops enabled but no ADO organization could be \ + determined. Either set tools.azure-devops.org explicitly, or compile from \ + within a git repository with an Azure DevOps remote URL.", + front_matter.name + ); + }; + if !org.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') { + anyhow::bail!( + "Invalid ADO org name '{}': must contain only alphanumerics and hyphens", + org + ); + } + entrypoint_args.push(org); + + // Toolsets: passed as -d flag followed by space-separated toolset names + if !ado_config.toolsets().is_empty() { + entrypoint_args.push("-d".to_string()); + for toolset in ado_config.toolsets() { + if !toolset.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') { + anyhow::bail!( + "Invalid ADO toolset name '{}': must contain only alphanumerics and hyphens", + toolset + ); + } + entrypoint_args.push(toolset.clone()); + } + } + + // Tool allow-list for MCPG filtering + let tools = if ado_config.allowed().is_empty() { + None + } else { + Some(ado_config.allowed().to_vec()) + }; + + // ADO MCP needs the PAT token passed via environment + let env = Some(HashMap::from([( + "AZURE_DEVOPS_EXT_PAT".to_string(), + String::new(), // Passthrough from pipeline + )])); + + mcp_servers.insert( + ADO_MCP_SERVER_NAME.to_string(), + McpgServerConfig { + server_type: "stdio".to_string(), + container: Some(ADO_MCP_IMAGE.to_string()), + entrypoint: Some(ADO_MCP_ENTRYPOINT.to_string()), + entrypoint_args: Some(entrypoint_args), + mounts: None, + args: None, + url: None, + headers: None, + env, + tools, + }, + ); + } + } + for (name, config) in &front_matter.mcp_servers { // Prevent user-defined MCPs from overwriting the reserved safeoutputs backend if name.eq_ignore_ascii_case("safeoutputs") { @@ -478,6 +629,11 @@ pub fn generate_mcpg_config(front_matter: &FrontMatter) -> McpgConfig { continue; } + // Skip if already auto-configured by tools.azure-devops + if name == ADO_MCP_SERVER_NAME && mcp_servers.contains_key(ADO_MCP_SERVER_NAME) { + continue; + } + let (is_enabled, options) = match config { McpConfig::Enabled(enabled) => (*enabled, None), McpConfig::WithOptions(opts) => (opts.enabled.unwrap_or(true), Some(opts)), @@ -593,7 +749,7 @@ pub fn generate_mcpg_config(front_matter: &FrontMatter) -> McpgConfig { } } - McpgConfig { + Ok(McpgConfig { mcp_servers, gateway: McpgGatewayConfig { port: MCPG_PORT, @@ -601,7 +757,7 @@ pub fn generate_mcpg_config(front_matter: &FrontMatter) -> McpgConfig { api_key: "${MCP_GATEWAY_API_KEY}".to_string(), payload_dir: "/tmp/gh-aw/mcp-payloads".to_string(), }, - } + }) } /// Sensitive host path prefixes that should not be bind-mounted into MCP containers. @@ -795,9 +951,16 @@ pub fn generate_mcpg_docker_env(front_matter: &FrontMatter) -> String { && opts.env.contains_key("AZURE_DEVOPS_EXT_PAT")) }); + // Also check if tools.azure-devops is enabled (auto-configured ADO MCP always needs token) + let ado_tool_needs_token = front_matter + .tools + .as_ref() + .and_then(|t| t.azure_devops.as_ref()) + .is_some_and(|ado| ado.is_enabled()); + // Auto-map AZURE_DEVOPS_EXT_PAT from SC_READ_TOKEN when permissions.read is configured - // AND at least one container MCP requests it via env passthrough - if any_mcp_needs_ado_token { + // AND at least one container MCP requests it via env passthrough (or the ADO tool is enabled) + if any_mcp_needs_ado_token || ado_tool_needs_token { if front_matter.permissions.as_ref().and_then(|p| p.read.as_ref()).is_some() { env_flags.push( "-e AZURE_DEVOPS_EXT_PAT=\"$(SC_READ_TOKEN)\"".to_string(), @@ -949,7 +1112,7 @@ mod tests { ..Default::default() }), ); - let config = generate_mcpg_config(&fm); + let config = generate_mcpg_config(&fm, None).unwrap(); let server = config.mcp_servers.get("my-tool").unwrap(); assert_eq!(server.server_type, "stdio"); assert_eq!(server.container.as_ref().unwrap(), "node:20-slim"); @@ -970,7 +1133,7 @@ mod tests { // An MCP with no container or url should be skipped fm.mcp_servers .insert("phantom".to_string(), McpConfig::Enabled(true)); - let config = generate_mcpg_config(&fm); + let config = generate_mcpg_config(&fm, None).unwrap(); assert!(!config.mcp_servers.contains_key("phantom")); // safeoutputs is always present assert!(config.mcp_servers.contains_key("safeoutputs")); @@ -981,14 +1144,14 @@ mod tests { let mut fm = minimal_front_matter(); fm.mcp_servers .insert("my-tool".to_string(), McpConfig::Enabled(false)); - let config = generate_mcpg_config(&fm); + let config = generate_mcpg_config(&fm, None).unwrap(); assert!(!config.mcp_servers.contains_key("my-tool")); } #[test] fn test_generate_mcpg_config_empty_mcp_servers() { let fm = minimal_front_matter(); - let config = generate_mcpg_config(&fm); + let config = generate_mcpg_config(&fm, None).unwrap(); // Only safeoutputs should be present assert_eq!(config.mcp_servers.len(), 1); assert!(config.mcp_servers.contains_key("safeoutputs")); @@ -997,7 +1160,7 @@ mod tests { #[test] fn test_generate_mcpg_config_gateway_defaults() { let fm = minimal_front_matter(); - let config = generate_mcpg_config(&fm); + let config = generate_mcpg_config(&fm, None).unwrap(); assert_eq!(config.gateway.port, 80); assert_eq!(config.gateway.domain, "host.docker.internal"); assert_eq!(config.gateway.api_key, "${MCP_GATEWAY_API_KEY}"); @@ -1017,7 +1180,7 @@ mod tests { ..Default::default() }), ); - let config = generate_mcpg_config(&fm); + let config = generate_mcpg_config(&fm, None).unwrap(); let json = serde_json::to_string_pretty(&config).expect("Config should serialize to JSON"); let parsed: serde_json::Value = serde_json::from_str(&json).expect("Serialized JSON should parse back"); @@ -1042,7 +1205,7 @@ mod tests { #[test] fn test_generate_mcpg_config_safeoutputs_variable_placeholders() { let fm = minimal_front_matter(); - let config = generate_mcpg_config(&fm); + let config = generate_mcpg_config(&fm, None).unwrap(); let so = config.mcp_servers.get("safeoutputs").unwrap(); // URL should reference the runtime-substituted port @@ -1064,7 +1227,7 @@ mod tests { #[test] fn test_generate_mcpg_config_safeoutputs_is_http_type() { let fm = minimal_front_matter(); - let config = generate_mcpg_config(&fm); + let config = generate_mcpg_config(&fm, None).unwrap(); let so = config.mcp_servers.get("safeoutputs").unwrap(); assert_eq!(so.server_type, "http"); assert!( @@ -1088,7 +1251,7 @@ mod tests { ..Default::default() }), ); - let config = generate_mcpg_config(&fm); + let config = generate_mcpg_config(&fm, None).unwrap(); let srv = config.mcp_servers.get("runner").unwrap(); assert_eq!(srv.server_type, "stdio"); assert!( @@ -1111,7 +1274,7 @@ mod tests { ..Default::default() }), ); - let config = generate_mcpg_config(&fm); + let config = generate_mcpg_config(&fm, None).unwrap(); let srv = config.mcp_servers.get("with-env").unwrap(); let e = srv.env.as_ref().unwrap(); assert_eq!(e.get("TOKEN").unwrap(), "secret"); @@ -1127,7 +1290,7 @@ mod tests { ..Default::default() }), ); - let config = generate_mcpg_config(&fm); + let config = generate_mcpg_config(&fm, None).unwrap(); // The reserved entry should still be the HTTP backend, not the user's container let so = config.mcp_servers.get("safeoutputs").unwrap(); assert_eq!( @@ -1153,7 +1316,7 @@ mod tests { ..Default::default() }), ); - let config = generate_mcpg_config(&fm); + let config = generate_mcpg_config(&fm, None).unwrap(); // The user-defined "SafeOutputs" must not overwrite the built-in entry let so = config.mcp_servers.get("safeoutputs").unwrap(); assert_eq!(so.server_type, "http"); @@ -1178,7 +1341,7 @@ mod tests { ..Default::default() }), ); - let config = generate_mcpg_config(&fm); + let config = generate_mcpg_config(&fm, None).unwrap(); let srv = config.mcp_servers.get("remote").unwrap(); assert_eq!(srv.server_type, "http"); assert_eq!( @@ -1204,7 +1367,7 @@ mod tests { ..Default::default() }), ); - let config = generate_mcpg_config(&fm); + let config = generate_mcpg_config(&fm, None).unwrap(); let srv = config.mcp_servers.get("ado").unwrap(); assert_eq!(srv.server_type, "stdio"); assert_eq!(srv.container.as_ref().unwrap(), "node:20-slim"); @@ -1226,7 +1389,7 @@ mod tests { ..Default::default() }), ); - let config = generate_mcpg_config(&fm); + let config = generate_mcpg_config(&fm, None).unwrap(); let srv = config.mcp_servers.get("data-tool").unwrap(); assert_eq!( srv.mounts.as_ref().unwrap(), @@ -1245,7 +1408,7 @@ mod tests { ..Default::default() }), ); - let config = generate_mcpg_config(&fm); + let config = generate_mcpg_config(&fm, None).unwrap(); assert!(!config.mcp_servers.contains_key("no-transport")); } @@ -1403,4 +1566,169 @@ mod tests { assert!(!is_valid_env_var_name("X --privileged")); assert!(!is_valid_env_var_name("X -v /etc:/etc:rw")); } + + // ─── tools.azure-devops MCPG integration ──────────────────────────────── + + #[test] + fn test_ado_tool_generates_mcpg_entry() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\ntools:\n azure-devops: true\n---\n", + ) + .unwrap(); + // Pass inferred org since no explicit org is set + let config = generate_mcpg_config(&fm, Some("inferred-org")).unwrap(); + let ado = config.mcp_servers.get("azure-devops").unwrap(); + assert_eq!(ado.server_type, "stdio"); + assert_eq!(ado.container.as_deref(), Some(ADO_MCP_IMAGE)); + assert_eq!(ado.entrypoint.as_deref(), Some(ADO_MCP_ENTRYPOINT)); + let args = ado.entrypoint_args.as_ref().unwrap(); + assert!(args.contains(&"-y".to_string())); + assert!(args.contains(&ADO_MCP_PACKAGE.to_string())); + assert!(args.contains(&"inferred-org".to_string())); + // Should have AZURE_DEVOPS_EXT_PAT in env + let env = ado.env.as_ref().unwrap(); + assert!(env.contains_key("AZURE_DEVOPS_EXT_PAT")); + } + + #[test] + fn test_ado_tool_with_toolsets() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\ntools:\n azure-devops:\n toolsets: [repos, wit, core]\n---\n", + ) + .unwrap(); + let config = generate_mcpg_config(&fm, Some("myorg")).unwrap(); + let ado = config.mcp_servers.get("azure-devops").unwrap(); + let args = ado.entrypoint_args.as_ref().unwrap(); + assert!(args.contains(&"-d".to_string())); + assert!(args.contains(&"repos".to_string())); + assert!(args.contains(&"wit".to_string())); + assert!(args.contains(&"core".to_string())); + } + + #[test] + fn test_ado_tool_with_org_override() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\ntools:\n azure-devops:\n org: myorg\n---\n", + ) + .unwrap(); + // Explicit org should be used even when inferred_org is None + let config = generate_mcpg_config(&fm, None).unwrap(); + let ado = config.mcp_servers.get("azure-devops").unwrap(); + let args = ado.entrypoint_args.as_ref().unwrap(); + assert!(args.contains(&"myorg".to_string())); + } + + #[test] + fn test_ado_tool_explicit_org_overrides_inferred() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\ntools:\n azure-devops:\n org: explicit-org\n---\n", + ) + .unwrap(); + let config = generate_mcpg_config(&fm, Some("inferred-org")).unwrap(); + let ado = config.mcp_servers.get("azure-devops").unwrap(); + let args = ado.entrypoint_args.as_ref().unwrap(); + assert!(args.contains(&"explicit-org".to_string())); + assert!(!args.contains(&"inferred-org".to_string())); + } + + #[test] + fn test_ado_tool_no_org_fails() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\ntools:\n azure-devops: true\n---\n", + ) + .unwrap(); + // No explicit org and no inferred org — should fail + let result = generate_mcpg_config(&fm, None); + assert!(result.is_err()); + assert!( + result.unwrap_err().to_string().contains("no ADO organization"), + "Error should mention missing org" + ); + } + + #[test] + fn test_ado_tool_invalid_org_fails() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\ntools:\n azure-devops:\n org: \"my org/bad\"\n---\n", + ) + .unwrap(); + let result = generate_mcpg_config(&fm, None); + assert!(result.is_err()); + assert!( + result.unwrap_err().to_string().contains("Invalid ADO org name"), + "Error should mention invalid org" + ); + } + + #[test] + fn test_ado_tool_invalid_toolset_fails() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\ntools:\n azure-devops:\n org: myorg\n toolsets: [\"repos\", \"bad toolset\"]\n---\n", + ) + .unwrap(); + let result = generate_mcpg_config(&fm, None); + assert!(result.is_err()); + assert!( + result.unwrap_err().to_string().contains("Invalid ADO toolset name"), + "Error should mention invalid toolset" + ); + } + + #[test] + fn test_ado_tool_with_allowed_tools() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\ntools:\n azure-devops:\n org: myorg\n allowed:\n - wit_get_work_item\n - core_list_projects\n---\n", + ) + .unwrap(); + let config = generate_mcpg_config(&fm, None).unwrap(); + let ado = config.mcp_servers.get("azure-devops").unwrap(); + let tools = ado.tools.as_ref().unwrap(); + assert_eq!(tools, &["wit_get_work_item", "core_list_projects"]); + } + + #[test] + fn test_ado_tool_disabled_not_generated() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\ntools:\n azure-devops: false\n---\n", + ) + .unwrap(); + let config = generate_mcpg_config(&fm, None).unwrap(); + assert!(!config.mcp_servers.contains_key("azure-devops")); + } + + #[test] + fn test_ado_tool_not_set_not_generated() { + let fm = minimal_front_matter(); + let config = generate_mcpg_config(&fm, None).unwrap(); + assert!(!config.mcp_servers.contains_key("azure-devops")); + } + + #[test] + fn test_ado_tool_skips_manual_mcp_entry() { + // When tools.azure-devops is enabled AND mcp-servers also has azure-devops, + // the tools config takes precedence and the manual entry is skipped. + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\ntools:\n azure-devops:\n org: auto-org\nmcp-servers:\n azure-devops:\n container: \"node:20-slim\"\n entrypoint: \"npx\"\n entrypoint-args: [\"-y\", \"@azure-devops/mcp\", \"manual-org\"]\n---\n", + ) + .unwrap(); + let config = generate_mcpg_config(&fm, None).unwrap(); + let ado = config.mcp_servers.get("azure-devops").unwrap(); + // Should use the auto-configured org, not the manual one + let args = ado.entrypoint_args.as_ref().unwrap(); + assert!(args.contains(&"auto-org".to_string())); + assert!(!args.contains(&"manual-org".to_string())); + } + + #[test] + fn test_ado_tool_docker_env_passthrough() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\ntools:\n azure-devops: true\npermissions:\n read: my-read-sc\n---\n", + ) + .unwrap(); + let env = generate_mcpg_docker_env(&fm); + assert!( + env.contains("AZURE_DEVOPS_EXT_PAT"), + "Should include ADO token passthrough when permissions.read is set" + ); + } } diff --git a/src/compile/types.rs b/src/compile/types.rs index 5af5764d..66ec5895 100644 --- a/src/compile/types.rs +++ b/src/compile/types.rs @@ -189,6 +189,18 @@ pub struct EngineOptions { /// /// Controls which tools are available and their settings. /// If not specified, defaults are used. +/// +/// Examples: +/// ```yaml +/// tools: +/// bash: ["cat", "ls", "grep"] +/// edit: true +/// cache-memory: +/// allowed-extensions: [.md, .json] +/// azure-devops: +/// toolsets: [repos, wit] +/// allowed: [wit_get_work_item] +/// ``` #[derive(Debug, Deserialize, Clone, Default)] pub struct ToolsConfig { /// Bash command allow-list. If empty/not set, defaults to safe commands. @@ -198,6 +210,136 @@ pub struct ToolsConfig { /// Enable the file editing tool (default: true) #[serde(default)] pub edit: Option, + /// Persistent cache memory across agent runs. + /// Enables the agent to read/write files to a memory directory + /// that persists between pipeline executions. + #[serde(default, rename = "cache-memory")] + pub cache_memory: Option, + /// First-class Azure DevOps MCP integration. + /// Auto-configures the ADO MCP container, token mapping, MCPG entry, + /// and network allowlist domains. + #[serde(default, rename = "azure-devops")] + pub azure_devops: Option, +} + +/// Cache memory tool configuration — accepts both `true` and object formats +/// +/// Examples: +/// ```yaml +/// # Simple enablement +/// cache-memory: true +/// +/// # With options +/// cache-memory: +/// allowed-extensions: [.md, .json, .txt] +/// ``` +#[derive(Debug, Deserialize, Clone)] +#[serde(untagged)] +pub enum CacheMemoryToolConfig { + /// Simple boolean enablement + Enabled(bool), + /// Full configuration with options + WithOptions(CacheMemoryOptions), +} + +impl CacheMemoryToolConfig { + /// Whether cache memory is enabled + pub fn is_enabled(&self) -> bool { + match self { + CacheMemoryToolConfig::Enabled(enabled) => *enabled, + CacheMemoryToolConfig::WithOptions(_) => true, + } + } + + /// Get the allowed file extensions (empty = all allowed) + pub fn allowed_extensions(&self) -> &[String] { + match self { + CacheMemoryToolConfig::Enabled(_) => &[], + CacheMemoryToolConfig::WithOptions(opts) => &opts.allowed_extensions, + } + } +} + +/// Cache memory options +#[derive(Debug, Deserialize, Clone, Default)] +pub struct CacheMemoryOptions { + /// Allowed file extensions (e.g., [".md", ".json", ".txt"]). + /// Defaults to all extensions if empty or not specified. + #[serde(default, rename = "allowed-extensions")] + pub allowed_extensions: Vec, +} + +/// Azure DevOps MCP tool configuration — accepts both `true` and object formats +/// +/// Examples: +/// ```yaml +/// # Simple enablement (auto-infers org from git remote) +/// azure-devops: true +/// +/// # With scoping options +/// azure-devops: +/// toolsets: [repos, wit, core] +/// allowed: [wit_get_work_item, wit_my_work_items] +/// org: myorg +/// ``` +#[derive(Debug, Deserialize, Clone)] +#[serde(untagged)] +pub enum AzureDevOpsToolConfig { + /// Simple boolean enablement + Enabled(bool), + /// Full configuration with options + WithOptions(AzureDevOpsOptions), +} + +impl AzureDevOpsToolConfig { + /// Whether the ADO MCP is enabled + pub fn is_enabled(&self) -> bool { + match self { + AzureDevOpsToolConfig::Enabled(enabled) => *enabled, + AzureDevOpsToolConfig::WithOptions(_) => true, + } + } + + /// Get the ADO API toolset groups to enable (e.g., repos, wit, core) + pub fn toolsets(&self) -> &[String] { + match self { + AzureDevOpsToolConfig::Enabled(_) => &[], + AzureDevOpsToolConfig::WithOptions(opts) => &opts.toolsets, + } + } + + /// Get the explicit tool allow-list + pub fn allowed(&self) -> &[String] { + match self { + AzureDevOpsToolConfig::Enabled(_) => &[], + AzureDevOpsToolConfig::WithOptions(opts) => &opts.allowed, + } + } + + /// Get the org override (None = auto-infer from git remote) + pub fn org(&self) -> Option<&str> { + match self { + AzureDevOpsToolConfig::Enabled(_) => None, + AzureDevOpsToolConfig::WithOptions(opts) => opts.org.as_deref(), + } + } +} + +/// Azure DevOps MCP options +#[derive(Debug, Deserialize, Clone, Default)] +pub struct AzureDevOpsOptions { + /// ADO API toolset groups to enable (e.g., repos, wit, core, work-items) + /// Passed as `-d` flags to the ADO MCP entrypoint. + #[serde(default)] + pub toolsets: Vec, + /// Explicit tool allow-list (e.g., wit_get_work_item, core_list_projects) + /// Passed to MCPG for tool-level filtering. + #[serde(default)] + pub allowed: Vec, + /// Azure DevOps organization name override. + /// Auto-inferred from the git remote URL at compile time if not specified. + #[serde(default)] + pub org: Option, } /// Azure DevOps runtime parameter definition. @@ -654,4 +796,173 @@ Body let (fm, _) = super::super::common::parse_markdown(content).unwrap(); assert!(fm.permissions.is_none()); } + + // ─── CacheMemoryToolConfig deserialization ────────────────────────────── + + #[test] + fn test_cache_memory_bool_true() { + let content = r#"--- +name: "Test" +description: "Test" +tools: + cache-memory: true +--- + +Body +"#; + let (fm, _) = super::super::common::parse_markdown(content).unwrap(); + let cm = fm.tools.as_ref().unwrap().cache_memory.as_ref().unwrap(); + assert!(cm.is_enabled()); + assert!(cm.allowed_extensions().is_empty()); + } + + #[test] + fn test_cache_memory_bool_false() { + let content = r#"--- +name: "Test" +description: "Test" +tools: + cache-memory: false +--- + +Body +"#; + let (fm, _) = super::super::common::parse_markdown(content).unwrap(); + let cm = fm.tools.as_ref().unwrap().cache_memory.as_ref().unwrap(); + assert!(!cm.is_enabled()); + } + + #[test] + fn test_cache_memory_with_options() { + let content = r#"--- +name: "Test" +description: "Test" +tools: + cache-memory: + allowed-extensions: + - .md + - .json +--- + +Body +"#; + let (fm, _) = super::super::common::parse_markdown(content).unwrap(); + let cm = fm.tools.as_ref().unwrap().cache_memory.as_ref().unwrap(); + assert!(cm.is_enabled()); + assert_eq!(cm.allowed_extensions(), &[".md", ".json"]); + } + + #[test] + fn test_cache_memory_not_set() { + let content = r#"--- +name: "Test" +description: "Test" +tools: + edit: true +--- + +Body +"#; + let (fm, _) = super::super::common::parse_markdown(content).unwrap(); + assert!(fm.tools.as_ref().unwrap().cache_memory.is_none()); + } + + // ─── AzureDevOpsToolConfig deserialization ────────────────────────────── + + #[test] + fn test_azure_devops_bool_true() { + let content = r#"--- +name: "Test" +description: "Test" +tools: + azure-devops: true +--- + +Body +"#; + let (fm, _) = super::super::common::parse_markdown(content).unwrap(); + let ado = fm.tools.as_ref().unwrap().azure_devops.as_ref().unwrap(); + assert!(ado.is_enabled()); + assert!(ado.toolsets().is_empty()); + assert!(ado.allowed().is_empty()); + assert!(ado.org().is_none()); + } + + #[test] + fn test_azure_devops_with_options() { + let content = r#"--- +name: "Test" +description: "Test" +tools: + azure-devops: + toolsets: [repos, wit, core] + allowed: [wit_get_work_item, core_list_projects] + org: myorg +--- + +Body +"#; + let (fm, _) = super::super::common::parse_markdown(content).unwrap(); + let ado = fm.tools.as_ref().unwrap().azure_devops.as_ref().unwrap(); + assert!(ado.is_enabled()); + assert_eq!(ado.toolsets(), &["repos", "wit", "core"]); + assert_eq!(ado.allowed(), &["wit_get_work_item", "core_list_projects"]); + assert_eq!(ado.org(), Some("myorg")); + } + + #[test] + fn test_azure_devops_partial_config() { + let content = r#"--- +name: "Test" +description: "Test" +tools: + azure-devops: + toolsets: [wit] +--- + +Body +"#; + let (fm, _) = super::super::common::parse_markdown(content).unwrap(); + let ado = fm.tools.as_ref().unwrap().azure_devops.as_ref().unwrap(); + assert!(ado.is_enabled()); + assert_eq!(ado.toolsets(), &["wit"]); + assert!(ado.allowed().is_empty()); + assert!(ado.org().is_none()); + } + + #[test] + fn test_azure_devops_not_set() { + let content = r#"--- +name: "Test" +description: "Test" +--- + +Body +"#; + let (fm, _) = super::super::common::parse_markdown(content).unwrap(); + assert!(fm.tools.is_none()); + } + + #[test] + fn test_both_tools_together() { + let content = r#"--- +name: "Test" +description: "Test" +tools: + bash: ["cat", "ls"] + edit: true + cache-memory: true + azure-devops: + toolsets: [wit] +--- + +Body +"#; + let (fm, _) = super::super::common::parse_markdown(content).unwrap(); + let tools = fm.tools.as_ref().unwrap(); + assert!(tools.cache_memory.as_ref().unwrap().is_enabled()); + assert!(tools.azure_devops.as_ref().unwrap().is_enabled()); + assert_eq!(tools.bash.as_ref().unwrap(), &["cat", "ls"]); + assert_eq!(tools.edit, Some(true)); + } } diff --git a/src/configure.rs b/src/configure.rs index f302e776..8278643a 100644 --- a/src/configure.rs +++ b/src/configure.rs @@ -151,7 +151,7 @@ pub fn parse_ado_remote(remote_url: &str) -> Result { } /// Get the git remote URL for the repository at `repo_path`. -async fn get_git_remote_url(repo_path: &Path) -> Result { +pub async fn get_git_remote_url(repo_path: &Path) -> Result { let output = tokio::process::Command::new("git") .args(["remote", "get-url", "origin"]) .current_dir(repo_path) diff --git a/src/execute.rs b/src/execute.rs index 9a7bea8c..e4f24387 100644 --- a/src/execute.rs +++ b/src/execute.rs @@ -11,7 +11,7 @@ use std::path::Path; use crate::ndjson::{self, SAFE_OUTPUT_FILENAME}; use crate::sanitize::Sanitize; -use crate::tools::{ +use crate::safeoutputs::{ AddBuildTagResult, AddPrCommentResult, CreateBranchResult, CreateGitTagResult, CreatePrResult, CreateWikiPageResult, CreateWorkItemResult, CommentOnWorkItemResult, ExecutionContext, ExecutionResult, Executor, LinkWorkItemsResult, QueueBuildResult, @@ -21,7 +21,7 @@ use crate::tools::{ }; // Re-export memory types for use by main.rs -pub use crate::tools::memory::{MemoryConfig, process_agent_memory}; +pub use crate::tools::cache_memory::{MemoryConfig, process_agent_memory}; /// Execute all safe outputs from the NDJSON file in the specified directory pub async fn execute_safe_outputs( diff --git a/src/main.rs b/src/main.rs index c4f575a9..9a766e30 100644 --- a/src/main.rs +++ b/src/main.rs @@ -10,6 +10,7 @@ mod mcp; mod ndjson; mod proxy; pub mod sanitize; +mod safeoutputs; mod tools; use anyhow::{Context, Result}; @@ -17,7 +18,7 @@ use clap::{Parser, Subcommand}; use log::debug; use std::path::PathBuf; -use crate::tools::ExecutionContext; +use crate::safeoutputs::ExecutionContext; #[derive(Subcommand, Debug)] enum Commands { @@ -224,25 +225,32 @@ async fn main() -> Result<()> { let results = execute::execute_safe_outputs(&safe_output_dir, &ctx).await?; - // Process agent memory if memory config is present - if let Some(memory_value) = front_matter.safe_outputs.get("memory") { - let memory_config: execute::MemoryConfig = - serde_json::from_value(memory_value.clone()).unwrap_or_default(); - let memory_output = output_dir - .as_ref() - .cloned() - .unwrap_or_else(|| safe_output_dir.clone()); - let memory_result = execute::process_agent_memory( - &safe_output_dir, - &memory_output, - &memory_config, - ) - .await?; - println!( - "Memory: {} - {}", - if memory_result.success { "✓" } else { "✗" }, - memory_result.message - ); + // Process agent memory if cache-memory tool is enabled + let cache_memory = front_matter + .tools + .as_ref() + .and_then(|t| t.cache_memory.as_ref()); + if let Some(cm) = cache_memory { + if cm.is_enabled() { + let memory_config = execute::MemoryConfig { + allowed_extensions: cm.allowed_extensions().to_vec(), + }; + let memory_output = output_dir + .as_ref() + .cloned() + .unwrap_or_else(|| safe_output_dir.clone()); + let memory_result = execute::process_agent_memory( + &safe_output_dir, + &memory_output, + &memory_config, + ) + .await?; + println!( + "Memory: {} - {}", + if memory_result.success { "✓" } else { "✗" }, + memory_result.message + ); + } } // Print summary diff --git a/src/mcp.rs b/src/mcp.rs index 0198aca7..ac8ebcda 100644 --- a/src/mcp.rs +++ b/src/mcp.rs @@ -10,7 +10,7 @@ use std::path::PathBuf; use crate::ndjson::{self, SAFE_OUTPUT_FILENAME}; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; -use crate::tools::{ +use crate::safeoutputs::{ AddBuildTagParams, AddBuildTagResult, AddPrCommentParams, AddPrCommentResult, CommentOnWorkItemParams, CommentOnWorkItemResult, @@ -64,7 +64,7 @@ fn generate_short_id() -> String { } // Re-export from tools module -use crate::tools::ALWAYS_ON_TOOLS; +use crate::safeoutputs::ALWAYS_ON_TOOLS; // ============================================================================ // SafeOutputs MCP Server @@ -1361,11 +1361,11 @@ mod tests { } /// Asserts that ALL_KNOWN_SAFE_OUTPUTS contains every tool registered in the - /// router (plus the non-MCP keys like "memory"). This prevents the list from - /// drifting when new tools are added to the router but not to the constant. + /// router. This prevents the list from drifting when new tools are added to + /// the router but not to the constant. #[tokio::test] async fn test_all_known_safe_outputs_covers_router() { - use crate::tools::ALL_KNOWN_SAFE_OUTPUTS; + use crate::safeoutputs::ALL_KNOWN_SAFE_OUTPUTS; let temp_dir = tempfile::tempdir().unwrap(); let so = SafeOutputs::new(temp_dir.path(), temp_dir.path(), None) diff --git a/src/ndjson.rs b/src/ndjson.rs index ceb63113..3735641e 100644 --- a/src/ndjson.rs +++ b/src/ndjson.rs @@ -7,7 +7,7 @@ use std::path::Path; use tokio::fs::OpenOptions; use tokio::io::AsyncWriteExt; -use crate::tools::ToolResult; +use crate::safeoutputs::ToolResult; /// The standard filename for safe outputs pub const SAFE_OUTPUT_FILENAME: &str = "safe_outputs.ndjson"; diff --git a/src/tools/add_build_tag.rs b/src/safeoutputs/add_build_tag.rs similarity index 98% rename from src/tools/add_build_tag.rs rename to src/safeoutputs/add_build_tag.rs index 8fca3a0b..9eed8b94 100644 --- a/src/tools/add_build_tag.rs +++ b/src/safeoutputs/add_build_tag.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use super::PATH_SEGMENT; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use crate::tool_result; -use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use anyhow::{Context, ensure}; // ── Stage 1: Params (agent-provided) ────────────────────────────────────── @@ -225,7 +225,7 @@ impl Executor for AddBuildTagResult { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { diff --git a/src/tools/add_pr_comment.rs b/src/safeoutputs/add_pr_comment.rs similarity index 99% rename from src/tools/add_pr_comment.rs rename to src/safeoutputs/add_pr_comment.rs index 33caa456..959156d4 100644 --- a/src/tools/add_pr_comment.rs +++ b/src/safeoutputs/add_pr_comment.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use super::PATH_SEGMENT; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use crate::tool_result; -use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use anyhow::{Context, ensure}; /// Parameters for adding a comment thread on a pull request @@ -375,7 +375,7 @@ impl Executor for AddPrCommentResult { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { diff --git a/src/tools/comment_on_work_item.rs b/src/safeoutputs/comment_on_work_item.rs similarity index 96% rename from src/tools/comment_on_work_item.rs rename to src/safeoutputs/comment_on_work_item.rs index 438c180e..0032e0be 100644 --- a/src/tools/comment_on_work_item.rs +++ b/src/safeoutputs/comment_on_work_item.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use super::PATH_SEGMENT; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use crate::tool_result; -use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use anyhow::{Context, ensure}; /// Parameters for commenting on a work item @@ -318,7 +318,7 @@ impl Executor for CommentOnWorkItemResult { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { diff --git a/src/tools/create_branch.rs b/src/safeoutputs/create_branch.rs similarity index 99% rename from src/tools/create_branch.rs rename to src/safeoutputs/create_branch.rs index a3873cb6..b63d3bd5 100644 --- a/src/tools/create_branch.rs +++ b/src/safeoutputs/create_branch.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use super::{PATH_SEGMENT, validate_git_ref_name}; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use crate::tool_result; -use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use anyhow::{Context, ensure}; /// Parameters for creating a branch @@ -388,7 +388,7 @@ impl Executor for CreateBranchResult { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { diff --git a/src/tools/create_git_tag.rs b/src/safeoutputs/create_git_tag.rs similarity index 99% rename from src/tools/create_git_tag.rs rename to src/safeoutputs/create_git_tag.rs index 5f92e7f1..4bff86bd 100644 --- a/src/tools/create_git_tag.rs +++ b/src/safeoutputs/create_git_tag.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use super::{PATH_SEGMENT, validate_git_ref_name}; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use crate::tool_result; -use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use anyhow::{Context, ensure}; /// Parameters for creating a git tag (agent-provided) @@ -392,7 +392,7 @@ impl Executor for CreateGitTagResult { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { diff --git a/src/tools/create_pr.rs b/src/safeoutputs/create_pr.rs similarity index 99% rename from src/tools/create_pr.rs rename to src/safeoutputs/create_pr.rs index 10203647..00020e40 100644 --- a/src/tools/create_pr.rs +++ b/src/safeoutputs/create_pr.rs @@ -5,7 +5,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use tokio::process::Command; -use crate::tools::{ExecutionContext, ExecutionResult, Executor, ToolResult, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, ToolResult, Validate}; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use anyhow::{Context, ensure}; @@ -186,7 +186,7 @@ pub struct CreatePrResult { pub repository: String, } -impl crate::tools::ToolResult for CreatePrResult { +impl crate::safeoutputs::ToolResult for CreatePrResult { const NAME: &'static str = "create-pull-request"; const REQUIRES_WRITE: bool = true; } diff --git a/src/tools/create_wiki_page.rs b/src/safeoutputs/create_wiki_page.rs similarity index 98% rename from src/tools/create_wiki_page.rs rename to src/safeoutputs/create_wiki_page.rs index 0fc4273a..09889be7 100644 --- a/src/tools/create_wiki_page.rs +++ b/src/safeoutputs/create_wiki_page.rs @@ -10,7 +10,7 @@ use super::PATH_SEGMENT; use super::resolve_wiki_branch; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use crate::tool_result; -use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; /// Parameters for creating a wiki page (agent-provided) #[derive(Deserialize, JsonSchema)] @@ -380,7 +380,7 @@ impl Executor for CreateWikiPageResult { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; // ── ToolResult / macro ──────────────────────────────────────────────────── @@ -656,7 +656,7 @@ wiki-name: "MyProject.wiki" let mut result: CreateWikiPageResult = params.try_into().unwrap(); result.sanitize_fields(); - let ctx = crate::tools::ExecutionContext { + let ctx = crate::safeoutputs::ExecutionContext { ado_org_url: Some("https://dev.azure.com/myorg".to_string()), ado_organization: Some("myorg".to_string()), ado_project: Some("MyProject".to_string()), @@ -685,7 +685,7 @@ wiki-name: "MyProject.wiki" let mut result: CreateWikiPageResult = params.try_into().unwrap(); result.sanitize_fields(); - let ctx = crate::tools::ExecutionContext { + let ctx = crate::safeoutputs::ExecutionContext { ado_org_url: None, ..Default::default() }; @@ -719,7 +719,7 @@ wiki-name: "MyProject.wiki" comment: None, }; - let ctx = crate::tools::ExecutionContext { + let ctx = crate::safeoutputs::ExecutionContext { ado_org_url: Some("https://dev.azure.com/myorg".to_string()), ado_organization: Some("myorg".to_string()), ado_project: Some("MyProject".to_string()), @@ -757,7 +757,7 @@ wiki-name: "MyProject.wiki" comment: None, }; - let ctx = crate::tools::ExecutionContext { + let ctx = crate::safeoutputs::ExecutionContext { ado_org_url: Some("https://dev.azure.com/myorg".to_string()), ado_organization: Some("myorg".to_string()), ado_project: Some("MyProject".to_string()), @@ -795,7 +795,7 @@ wiki-name: "MyProject.wiki" comment: None, }; - let ctx = crate::tools::ExecutionContext { + let ctx = crate::safeoutputs::ExecutionContext { ado_org_url: Some("https://dev.azure.com/myorg".to_string()), ado_organization: Some("myorg".to_string()), ado_project: Some("MyProject".to_string()), diff --git a/src/tools/create_work_item.rs b/src/safeoutputs/create_work_item.rs similarity index 99% rename from src/tools/create_work_item.rs rename to src/safeoutputs/create_work_item.rs index 196d4aa9..7804c4ed 100644 --- a/src/tools/create_work_item.rs +++ b/src/safeoutputs/create_work_item.rs @@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize}; use super::PATH_SEGMENT; use crate::tool_result; -use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use anyhow::{Context, ensure}; @@ -414,7 +414,7 @@ impl Executor for CreateWorkItemResult { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { diff --git a/src/tools/link_work_items.rs b/src/safeoutputs/link_work_items.rs similarity index 98% rename from src/tools/link_work_items.rs rename to src/safeoutputs/link_work_items.rs index 5ab3c668..c7452b7a 100644 --- a/src/tools/link_work_items.rs +++ b/src/safeoutputs/link_work_items.rs @@ -8,8 +8,8 @@ use serde::{Deserialize, Serialize}; use super::PATH_SEGMENT; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use crate::tool_result; -use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; -use crate::tools::comment_on_work_item::CommentTarget; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::comment_on_work_item::CommentTarget; use anyhow::{Context, ensure}; /// Resolve a human-friendly link type name to the ADO relation type string. @@ -305,7 +305,7 @@ impl Executor for LinkWorkItemsResult { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { diff --git a/src/tools/missing_data.rs b/src/safeoutputs/missing_data.rs similarity index 95% rename from src/tools/missing_data.rs rename to src/safeoutputs/missing_data.rs index cd4a353b..5f0bece0 100644 --- a/src/tools/missing_data.rs +++ b/src/safeoutputs/missing_data.rs @@ -4,7 +4,7 @@ use schemars::JsonSchema; use serde::Deserialize; use crate::tool_result; -use crate::tools::Validate; +use crate::safeoutputs::Validate; /// Parameters for reporting missing data #[derive(Deserialize, JsonSchema)] @@ -37,7 +37,7 @@ tool_result! { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { diff --git a/src/tools/missing_tool.rs b/src/safeoutputs/missing_tool.rs similarity index 96% rename from src/tools/missing_tool.rs rename to src/safeoutputs/missing_tool.rs index ec70eb71..acff4ff3 100644 --- a/src/tools/missing_tool.rs +++ b/src/safeoutputs/missing_tool.rs @@ -4,7 +4,7 @@ use schemars::JsonSchema; use serde::Deserialize; use crate::tool_result; -use crate::tools::Validate; +use crate::safeoutputs::Validate; /// Parameters for reporting a missing tool #[derive(Deserialize, JsonSchema)] @@ -32,7 +32,7 @@ tool_result! { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { diff --git a/src/safeoutputs/mod.rs b/src/safeoutputs/mod.rs new file mode 100644 index 00000000..fedd8d9e --- /dev/null +++ b/src/safeoutputs/mod.rs @@ -0,0 +1,394 @@ +//! Tool parameter and result structs for MCP tools + +use crate::{all_safe_output_names, tool_names}; +use log::{debug, warn}; +use percent_encoding::{AsciiSet, CONTROLS, utf8_percent_encode}; + +/// Characters to percent-encode in a URL path segment. +/// Encodes the structural delimiters that would break URL parsing if left raw: +/// `#` (fragment), `?` (query), `/` (path separator), and space. +/// This hardens operator-controlled values (project names, wiki names, work item +/// types) against accidental corruption of the URL structure. +pub(crate) const PATH_SEGMENT: &AsciiSet = &CONTROLS.add(b'#').add(b'?').add(b'/').add(b' '); + +/// Safe output tools that are always available regardless of filtering. +/// These are diagnostic/transparency tools that agents should always have access to. +/// +/// Derived from diagnostic tool types — adding a new diagnostic tool means adding +/// its type here and the name is extracted automatically via `ToolResult::NAME`. +pub const ALWAYS_ON_TOOLS: &[&str] = tool_names![ + NoopResult, + MissingDataResult, + MissingToolResult, + ReportIncompleteResult, +]; + +/// Safe-output tools that require write access to ADO. +/// Compile-time derived from tool types via `ToolResult::NAME`. +/// +/// Adding a new write-requiring tool: create the struct with `tool_result!{ write = true, ... }`, +/// then add its type to this list. +pub const WRITE_REQUIRING_SAFE_OUTPUTS: &[&str] = tool_names![ + CreateWorkItemResult, + CommentOnWorkItemResult, + UpdateWorkItemResult, + CreatePrResult, + CreateWikiPageResult, + UpdateWikiPageResult, + AddPrCommentResult, + LinkWorkItemsResult, + QueueBuildResult, + CreateGitTagResult, + AddBuildTagResult, + CreateBranchResult, + UpdatePrResult, + UploadAttachmentResult, + SubmitPrReviewResult, + ReplyToPrCommentResult, + ResolvePrThreadResult, +]; + +/// Non-MCP safe-output keys handled by the compiler/executor, not the MCP server. +/// These must not appear in `--enabled-tools` or they cause real MCP tools to be +/// filtered out (the router has no route for them). +pub const NON_MCP_SAFE_OUTPUT_KEYS: &[&str] = &[]; + +/// All recognised safe-output keys accepted in front matter `safe-outputs:`. +/// This is the union of write-requiring tool types and diagnostic tool types. +/// +/// Derived at compile time from tool types — no hand-maintained string lists. +/// +/// Note: `memory` was removed — it is now a first-class tool configured via +/// `tools: cache-memory:` and is no longer a safe-output key. +pub const ALL_KNOWN_SAFE_OUTPUTS: &[&str] = all_safe_output_names![ + // Write-requiring MCP tools + CreateWorkItemResult, + CommentOnWorkItemResult, + UpdateWorkItemResult, + CreatePrResult, + CreateWikiPageResult, + UpdateWikiPageResult, + AddPrCommentResult, + LinkWorkItemsResult, + QueueBuildResult, + CreateGitTagResult, + AddBuildTagResult, + CreateBranchResult, + UpdatePrResult, + UploadAttachmentResult, + SubmitPrReviewResult, + ReplyToPrCommentResult, + ResolvePrThreadResult, + // Always-on diagnostics + NoopResult, + MissingDataResult, + MissingToolResult, + ReportIncompleteResult; +]; + +/// Resolve the effective branch for a wiki. +/// +/// If `configured_branch` is `Some`, that value is returned directly. +/// Otherwise the wiki metadata API is queried: code wikis (type 1) return +/// the published branch from the `versions` array; project wikis (type 0) +/// return `Ok(None)` because the server handles branching internally. +/// +/// Returns `Err` when a code wiki is detected but the branch cannot be +/// resolved — callers should surface this as a user-facing failure rather +/// than proceeding to a confusing ADO PUT error. +pub(crate) async fn resolve_wiki_branch( + client: &reqwest::Client, + org_url: &str, + project: &str, + wiki_name: &str, + token: &str, + configured_branch: Option<&str>, +) -> Result, String> { + // Explicit configuration always wins. + if let Some(b) = configured_branch { + return Ok(Some(b.to_owned())); + } + + let url = format!( + "{}/{}/_apis/wiki/wikis/{}", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + utf8_percent_encode(wiki_name, PATH_SEGMENT), + ); + + let resp = match client + .get(&url) + .query(&[("api-version", "7.0")]) + .basic_auth("", Some(token)) + .send() + .await + { + Ok(r) => r, + Err(e) => { + warn!("Wiki metadata request failed: {e} — skipping branch auto-detection"); + return Ok(None); + } + }; + + if !resp.status().is_success() { + warn!( + "Wiki metadata request returned HTTP {} — skipping branch auto-detection", + resp.status() + ); + return Ok(None); + } + + let body: serde_json::Value = match resp.json().await { + Ok(b) => b, + Err(e) => { + warn!("Failed to parse wiki metadata response: {e}"); + return Ok(None); + } + }; + + // Detect code wikis. ADO returns the type as a string enum ("codeWiki" / + // "projectWiki") rather than a numeric value, so we check both forms. + let is_code_wiki = match body.get("type") { + Some(serde_json::Value::String(s)) => s.eq_ignore_ascii_case("codewiki"), + Some(serde_json::Value::Number(n)) => n.as_u64() == Some(1), + _ => false, + }; + if !is_code_wiki { + let type_val = body.get("type").cloned().unwrap_or(serde_json::Value::Null); + debug!("Wiki is a project wiki (type {type_val}) — no branch needed"); + return Ok(None); + } + + // Code wiki: extract the published branch from versions[0].version + let branch = body + .get("versions") + .and_then(|v| v.as_array()) + .and_then(|arr| arr.first()) + .and_then(|v| v.get("version")) + .and_then(|v| v.as_str()) + .map(|s| s.to_owned()); + + match &branch { + Some(b) => { + debug!("Detected code wiki — resolved branch: {b}"); + Ok(branch) + } + None => Err(format!( + "Wiki '{wiki_name}' is a code wiki but its published branch could not be \ + determined. Set 'branch' explicitly in the safe-outputs config." + )), + } +} + +/// Resolve a repository alias to its ADO repo name. +/// +/// "self" (or None) → `ctx.repository_name`, otherwise look up in `ctx.allowed_repositories`. +pub(crate) fn resolve_repo_name( + repo_alias: Option<&str>, + ctx: &ExecutionContext, +) -> Result { + let alias = repo_alias.unwrap_or("self"); + if alias == "self" { + ctx.repository_name + .clone() + .ok_or_else(|| ExecutionResult::failure("BUILD_REPOSITORY_NAME not set")) + } else { + ctx.allowed_repositories + .get(alias) + .cloned() + .ok_or_else(|| { + ExecutionResult::failure(format!( + "Repository '{}' is not in the allowed repository list", + alias + )) + }) + } +} + +/// Validate a string against `git check-ref-format` rules. +/// +/// Returns `Ok(())` if the name is valid, or an `Err` describing the violation. +/// This covers the structural rules that Azure DevOps also enforces — catching +/// them early gives clearer error messages than letting the API fail. +pub(crate) fn validate_git_ref_name(name: &str, label: &str) -> anyhow::Result<()> { + use anyhow::ensure; + + ensure!(!name.is_empty(), "{label} must not be empty"); + ensure!(!name.contains(".."), "{label} must not contain '..'"); + ensure!(!name.contains("@{{"), "{label} must not contain '@{{'"); + ensure!(!name.ends_with('.'), "{label} must not end with '.'"); + ensure!(!name.ends_with(".lock"), "{label} must not end with '.lock'"); + ensure!( + !name.contains('\\'), + "{label} must not contain backslash" + ); + ensure!( + !name.contains("//"), + "{label} must not contain consecutive slashes" + ); + for ch in ['~', '^', ':', '?', '*', '['] { + ensure!( + !name.contains(ch), + "{label} must not contain '{ch}'" + ); + } + for component in name.split('/') { + ensure!( + !component.starts_with('.'), + "{label} path component must not start with '.'" + ); + } + Ok(()) +} + +mod add_build_tag; +mod add_pr_comment; +mod comment_on_work_item; +mod create_branch; +mod create_git_tag; +mod create_pr; +mod create_wiki_page; +mod create_work_item; +mod link_work_items; +mod missing_data; +mod missing_tool; +mod noop; +mod queue_build; +mod reply_to_pr_comment; +mod report_incomplete; +mod resolve_pr_thread; +mod result; +mod submit_pr_review; +mod update_pr; +mod update_wiki_page; +mod update_work_item; +mod upload_attachment; + +pub use add_build_tag::*; +pub use add_pr_comment::*; +pub use comment_on_work_item::*; +pub use create_branch::*; +pub use create_git_tag::*; +pub use create_pr::*; +pub use create_wiki_page::*; +pub use create_work_item::*; +pub use link_work_items::*; +pub use missing_data::*; +pub use missing_tool::*; +pub use noop::*; +pub use queue_build::*; +pub use reply_to_pr_comment::*; +pub use report_incomplete::*; +pub use resolve_pr_thread::*; +pub use result::{ + ExecutionContext, ExecutionResult, Executor, ToolResult, Validate, anyhow_to_mcp_error, +}; +pub use submit_pr_review::*; +pub use update_pr::*; +pub use update_wiki_page::*; +pub use update_work_item::*; +pub use upload_attachment::*; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_write_requiring_subset_of_all_known() { + for name in WRITE_REQUIRING_SAFE_OUTPUTS { + assert!( + ALL_KNOWN_SAFE_OUTPUTS.contains(name), + "WRITE_REQUIRING_SAFE_OUTPUTS entry '{}' is missing from ALL_KNOWN_SAFE_OUTPUTS", + name + ); + } + } + + #[test] + fn test_always_on_subset_of_all_known() { + for name in ALWAYS_ON_TOOLS { + assert!( + ALL_KNOWN_SAFE_OUTPUTS.contains(name), + "ALWAYS_ON_TOOLS entry '{}' is missing from ALL_KNOWN_SAFE_OUTPUTS", + name + ); + } + } + + #[test] + fn test_non_mcp_keys_subset_of_all_known() { + for name in NON_MCP_SAFE_OUTPUT_KEYS { + assert!( + ALL_KNOWN_SAFE_OUTPUTS.contains(name), + "NON_MCP_SAFE_OUTPUT_KEYS entry '{}' is missing from ALL_KNOWN_SAFE_OUTPUTS", + name + ); + } + } + + /// Verify that every type in the write-requiring list actually has + /// `REQUIRES_WRITE == true`, and every diagnostic type has `false`. + #[test] + fn test_requires_write_consistency() { + // Write-requiring tools + assert!(CreateWorkItemResult::REQUIRES_WRITE); + assert!(CommentOnWorkItemResult::REQUIRES_WRITE); + assert!(UpdateWorkItemResult::REQUIRES_WRITE); + assert!(CreatePrResult::REQUIRES_WRITE); + assert!(CreateWikiPageResult::REQUIRES_WRITE); + assert!(UpdateWikiPageResult::REQUIRES_WRITE); + assert!(AddPrCommentResult::REQUIRES_WRITE); + assert!(LinkWorkItemsResult::REQUIRES_WRITE); + assert!(QueueBuildResult::REQUIRES_WRITE); + assert!(CreateGitTagResult::REQUIRES_WRITE); + assert!(AddBuildTagResult::REQUIRES_WRITE); + assert!(CreateBranchResult::REQUIRES_WRITE); + assert!(UpdatePrResult::REQUIRES_WRITE); + assert!(UploadAttachmentResult::REQUIRES_WRITE); + assert!(SubmitPrReviewResult::REQUIRES_WRITE); + assert!(ReplyToPrCommentResult::REQUIRES_WRITE); + assert!(ResolvePrThreadResult::REQUIRES_WRITE); + + // Diagnostic tools (should NOT require write) + assert!(!NoopResult::REQUIRES_WRITE); + assert!(!MissingDataResult::REQUIRES_WRITE); + assert!(!MissingToolResult::REQUIRES_WRITE); + assert!(!ReportIncompleteResult::REQUIRES_WRITE); + } + + /// Verify ALL_KNOWN_SAFE_OUTPUTS has exactly the right count: + /// write tools + diagnostics + non-MCP keys. + #[test] + fn test_all_known_completeness() { + // The three sub-lists must be disjoint — a tool in multiple lists would + // be duplicated in ALL_KNOWN and the count would mismatch. + for name in WRITE_REQUIRING_SAFE_OUTPUTS { + assert!( + !ALWAYS_ON_TOOLS.contains(name), + "Tool '{}' appears in both WRITE_REQUIRING and ALWAYS_ON — lists must be disjoint", + name + ); + assert!( + !NON_MCP_SAFE_OUTPUT_KEYS.contains(name), + "Tool '{}' appears in both WRITE_REQUIRING and NON_MCP — lists must be disjoint", + name + ); + } + for name in ALWAYS_ON_TOOLS { + assert!( + !NON_MCP_SAFE_OUTPUT_KEYS.contains(name), + "Tool '{}' appears in both ALWAYS_ON and NON_MCP — lists must be disjoint", + name + ); + } + + let expected = WRITE_REQUIRING_SAFE_OUTPUTS.len() + + ALWAYS_ON_TOOLS.len() + + NON_MCP_SAFE_OUTPUT_KEYS.len(); + assert_eq!( + ALL_KNOWN_SAFE_OUTPUTS.len(), + expected, + "ALL_KNOWN_SAFE_OUTPUTS should be the union of write + diagnostic + non-MCP lists" + ); + } +} diff --git a/src/tools/noop.rs b/src/safeoutputs/noop.rs similarity index 97% rename from src/tools/noop.rs rename to src/safeoutputs/noop.rs index a51cb3cc..eac029f6 100644 --- a/src/tools/noop.rs +++ b/src/safeoutputs/noop.rs @@ -2,7 +2,7 @@ use schemars::JsonSchema; use serde::Deserialize; use crate::tool_result; -use crate::tools::Validate; +use crate::safeoutputs::Validate; /// Parameters for describing a no operation. Use this if there is no work to do. #[derive(Deserialize, JsonSchema)] @@ -27,7 +27,7 @@ tool_result! { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { diff --git a/src/tools/queue_build.rs b/src/safeoutputs/queue_build.rs similarity index 99% rename from src/tools/queue_build.rs rename to src/safeoutputs/queue_build.rs index df1558cd..f6b14129 100644 --- a/src/tools/queue_build.rs +++ b/src/safeoutputs/queue_build.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use super::PATH_SEGMENT; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use crate::tool_result; -use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use anyhow::{Context, ensure}; /// Parameters for queuing a build @@ -315,7 +315,7 @@ impl Executor for QueueBuildResult { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { diff --git a/src/tools/reply_to_pr_comment.rs b/src/safeoutputs/reply_to_pr_comment.rs similarity index 98% rename from src/tools/reply_to_pr_comment.rs rename to src/safeoutputs/reply_to_pr_comment.rs index 019fd05a..99b0db00 100644 --- a/src/tools/reply_to_pr_comment.rs +++ b/src/safeoutputs/reply_to_pr_comment.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use super::PATH_SEGMENT; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use crate::tool_result; -use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use anyhow::{Context, ensure}; /// Parameters for replying to an existing review comment thread on a pull request @@ -240,7 +240,7 @@ impl Executor for ReplyToPrCommentResult { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { diff --git a/src/tools/report_incomplete.rs b/src/safeoutputs/report_incomplete.rs similarity index 97% rename from src/tools/report_incomplete.rs rename to src/safeoutputs/report_incomplete.rs index 98248c63..9dfe1a9c 100644 --- a/src/tools/report_incomplete.rs +++ b/src/safeoutputs/report_incomplete.rs @@ -5,7 +5,7 @@ use serde::Deserialize; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use crate::tool_result; -use crate::tools::Validate; +use crate::safeoutputs::Validate; use anyhow::ensure; /// Parameters for reporting that a task could not be completed @@ -52,7 +52,7 @@ impl Sanitize for ReportIncompleteResult { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { diff --git a/src/tools/resolve_pr_thread.rs b/src/safeoutputs/resolve_pr_thread.rs similarity index 99% rename from src/tools/resolve_pr_thread.rs rename to src/safeoutputs/resolve_pr_thread.rs index 9f0be947..49a77c6b 100644 --- a/src/tools/resolve_pr_thread.rs +++ b/src/safeoutputs/resolve_pr_thread.rs @@ -9,7 +9,7 @@ use super::resolve_repo_name; use super::PATH_SEGMENT; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use crate::tool_result; -use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use anyhow::{Context, ensure}; /// All valid thread status strings (lowercase, agent-facing) @@ -286,7 +286,7 @@ impl Executor for ResolvePrThreadResult { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { diff --git a/src/tools/result.rs b/src/safeoutputs/result.rs similarity index 91% rename from src/tools/result.rs rename to src/safeoutputs/result.rs index b7d8272f..31735864 100644 --- a/src/tools/result.rs +++ b/src/safeoutputs/result.rs @@ -247,7 +247,7 @@ macro_rules! tool_result { )* } - impl $crate::tools::ToolResult for $name { + impl $crate::safeoutputs::ToolResult for $name { const NAME: &'static str = $tool_name; const DEFAULT_MAX: u32 = $default_max; const REQUIRES_WRITE: bool = true; @@ -257,10 +257,10 @@ macro_rules! tool_result { type Error = rmcp::ErrorData; fn try_from(params: $params) -> Result { - <$params as $crate::tools::Validate>::validate(¶ms) - .map_err($crate::tools::anyhow_to_mcp_error)?; + <$params as $crate::safeoutputs::Validate>::validate(¶ms) + .map_err($crate::safeoutputs::anyhow_to_mcp_error)?; Ok(Self { - name: ::NAME.to_string(), + name: ::NAME.to_string(), $($field: params.$field,)* }) } @@ -290,7 +290,7 @@ macro_rules! tool_result { )* } - impl $crate::tools::ToolResult for $name { + impl $crate::safeoutputs::ToolResult for $name { const NAME: &'static str = $tool_name; const REQUIRES_WRITE: bool = true; } @@ -299,10 +299,10 @@ macro_rules! tool_result { type Error = rmcp::ErrorData; fn try_from(params: $params) -> Result { - <$params as $crate::tools::Validate>::validate(¶ms) - .map_err($crate::tools::anyhow_to_mcp_error)?; + <$params as $crate::safeoutputs::Validate>::validate(¶ms) + .map_err($crate::safeoutputs::anyhow_to_mcp_error)?; Ok(Self { - name: ::NAME.to_string(), + name: ::NAME.to_string(), $($field: params.$field,)* }) } @@ -332,7 +332,7 @@ macro_rules! tool_result { )* } - impl $crate::tools::ToolResult for $name { + impl $crate::safeoutputs::ToolResult for $name { const NAME: &'static str = $tool_name; const DEFAULT_MAX: u32 = $default_max; } @@ -341,10 +341,10 @@ macro_rules! tool_result { type Error = rmcp::ErrorData; fn try_from(params: $params) -> Result { - <$params as $crate::tools::Validate>::validate(¶ms) - .map_err($crate::tools::anyhow_to_mcp_error)?; + <$params as $crate::safeoutputs::Validate>::validate(¶ms) + .map_err($crate::safeoutputs::anyhow_to_mcp_error)?; Ok(Self { - name: ::NAME.to_string(), + name: ::NAME.to_string(), $($field: params.$field,)* }) } @@ -373,7 +373,7 @@ macro_rules! tool_result { )* } - impl $crate::tools::ToolResult for $name { + impl $crate::safeoutputs::ToolResult for $name { const NAME: &'static str = $tool_name; } @@ -381,10 +381,10 @@ macro_rules! tool_result { type Error = rmcp::ErrorData; fn try_from(params: $params) -> Result { - <$params as $crate::tools::Validate>::validate(¶ms) - .map_err($crate::tools::anyhow_to_mcp_error)?; + <$params as $crate::safeoutputs::Validate>::validate(¶ms) + .map_err($crate::safeoutputs::anyhow_to_mcp_error)?; Ok(Self { - name: ::NAME.to_string(), + name: ::NAME.to_string(), $($field: params.$field,)* }) } @@ -406,7 +406,7 @@ macro_rules! tool_result { #[macro_export] macro_rules! tool_names { ($($ty:ty),* $(,)?) => { - &[$(<$ty as $crate::tools::ToolResult>::NAME),*] + &[$(<$ty as $crate::safeoutputs::ToolResult>::NAME),*] }; } @@ -425,7 +425,7 @@ macro_rules! tool_names { #[macro_export] macro_rules! all_safe_output_names { ($($ty:ty),* $(,)?; $($extra:expr),* $(,)?) => { - &[$(<$ty as $crate::tools::ToolResult>::NAME),*, $($extra),*] + &[$(<$ty as $crate::safeoutputs::ToolResult>::NAME),*, $($extra),*] }; } diff --git a/src/tools/submit_pr_review.rs b/src/safeoutputs/submit_pr_review.rs similarity index 99% rename from src/tools/submit_pr_review.rs rename to src/safeoutputs/submit_pr_review.rs index 8aa86827..af606185 100644 --- a/src/tools/submit_pr_review.rs +++ b/src/safeoutputs/submit_pr_review.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use super::{PATH_SEGMENT, resolve_repo_name}; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use crate::tool_result; -use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use anyhow::{Context, ensure}; /// Valid event values for submit-pr-review @@ -427,7 +427,7 @@ impl Executor for SubmitPrReviewResult { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { diff --git a/src/tools/update_pr.rs b/src/safeoutputs/update_pr.rs similarity index 99% rename from src/tools/update_pr.rs rename to src/safeoutputs/update_pr.rs index 28de73bc..32d92b07 100644 --- a/src/tools/update_pr.rs +++ b/src/safeoutputs/update_pr.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use super::{PATH_SEGMENT, resolve_repo_name}; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use crate::tool_result; -use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use anyhow::{Context, ensure}; /// Valid operation names for update-pr @@ -957,7 +957,7 @@ impl UpdatePrResult { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { diff --git a/src/tools/update_wiki_page.rs b/src/safeoutputs/update_wiki_page.rs similarity index 98% rename from src/tools/update_wiki_page.rs rename to src/safeoutputs/update_wiki_page.rs index 3f41cef5..05b901ca 100644 --- a/src/tools/update_wiki_page.rs +++ b/src/safeoutputs/update_wiki_page.rs @@ -10,7 +10,7 @@ use super::PATH_SEGMENT; use super::resolve_wiki_branch; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use crate::tool_result; -use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; /// Parameters for editing a wiki page (agent-provided) #[derive(Deserialize, JsonSchema)] @@ -372,7 +372,7 @@ impl Executor for UpdateWikiPageResult { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; // ── ToolResult / macro ──────────────────────────────────────────────────── @@ -626,7 +626,7 @@ wiki-name: "MyProject.wiki" let mut result: UpdateWikiPageResult = params.try_into().unwrap(); result.sanitize_fields(); - let ctx = crate::tools::ExecutionContext { + let ctx = crate::safeoutputs::ExecutionContext { ado_org_url: Some("https://dev.azure.com/myorg".to_string()), ado_organization: Some("myorg".to_string()), ado_project: Some("MyProject".to_string()), @@ -655,7 +655,7 @@ wiki-name: "MyProject.wiki" let mut result: UpdateWikiPageResult = params.try_into().unwrap(); result.sanitize_fields(); - let ctx = crate::tools::ExecutionContext { + let ctx = crate::safeoutputs::ExecutionContext { ado_org_url: None, ..Default::default() }; @@ -689,7 +689,7 @@ wiki-name: "MyProject.wiki" comment: None, }; - let ctx = crate::tools::ExecutionContext { + let ctx = crate::safeoutputs::ExecutionContext { ado_org_url: Some("https://dev.azure.com/myorg".to_string()), ado_organization: Some("myorg".to_string()), ado_project: Some("MyProject".to_string()), @@ -727,7 +727,7 @@ wiki-name: "MyProject.wiki" comment: None, }; - let ctx = crate::tools::ExecutionContext { + let ctx = crate::safeoutputs::ExecutionContext { ado_org_url: Some("https://dev.azure.com/myorg".to_string()), ado_organization: Some("myorg".to_string()), ado_project: Some("MyProject".to_string()), @@ -765,7 +765,7 @@ wiki-name: "MyProject.wiki" comment: None, }; - let ctx = crate::tools::ExecutionContext { + let ctx = crate::safeoutputs::ExecutionContext { ado_org_url: Some("https://dev.azure.com/myorg".to_string()), ado_organization: Some("myorg".to_string()), ado_project: Some("MyProject".to_string()), diff --git a/src/tools/update_work_item.rs b/src/safeoutputs/update_work_item.rs similarity index 99% rename from src/tools/update_work_item.rs rename to src/safeoutputs/update_work_item.rs index 9b8ba94c..e2bf59c6 100644 --- a/src/tools/update_work_item.rs +++ b/src/safeoutputs/update_work_item.rs @@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize}; use super::PATH_SEGMENT; use crate::tool_result; -use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use anyhow::{Context, ensure}; @@ -491,7 +491,7 @@ impl Executor for UpdateWorkItemResult { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { @@ -689,7 +689,7 @@ target: 42 #[tokio::test] async fn test_execute_requires_ado_context() { - use crate::tools::Executor; + use crate::safeoutputs::Executor; use std::collections::HashMap; use std::path::PathBuf; diff --git a/src/tools/upload_attachment.rs b/src/safeoutputs/upload_attachment.rs similarity index 99% rename from src/tools/upload_attachment.rs rename to src/safeoutputs/upload_attachment.rs index 2d635b3a..53e0883b 100644 --- a/src/tools/upload_attachment.rs +++ b/src/safeoutputs/upload_attachment.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use super::PATH_SEGMENT; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use crate::tool_result; -use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use anyhow::{Context, ensure}; /// Parameters for uploading an attachment to a work item @@ -352,7 +352,7 @@ impl Executor for UploadAttachmentResult { #[cfg(test)] mod tests { use super::*; - use crate::tools::ToolResult; + use crate::safeoutputs::ToolResult; #[test] fn test_result_has_correct_name() { diff --git a/src/tools/memory.rs b/src/tools/cache_memory.rs similarity index 99% rename from src/tools/memory.rs rename to src/tools/cache_memory.rs index d7eabc0d..82e8900a 100644 --- a/src/tools/memory.rs +++ b/src/tools/cache_memory.rs @@ -10,7 +10,7 @@ use log::{debug, info, warn}; use serde::Deserialize; use std::path::{Path, PathBuf}; -use crate::tools::ExecutionResult; +use crate::safeoutputs::ExecutionResult; /// Directory name for agent memory within the staging/artifact directories pub const AGENT_MEMORY_DIR: &str = "agent_memory"; diff --git a/src/tools/mod.rs b/src/tools/mod.rs index 417e8d56..aa0963be 100644 --- a/src/tools/mod.rs +++ b/src/tools/mod.rs @@ -1,395 +1,10 @@ -//! Tool parameter and result structs for MCP tools - -use crate::{all_safe_output_names, tool_names}; -use log::{debug, warn}; -use percent_encoding::{AsciiSet, CONTROLS, utf8_percent_encode}; - -/// Characters to percent-encode in a URL path segment. -/// Encodes the structural delimiters that would break URL parsing if left raw: -/// `#` (fragment), `?` (query), `/` (path separator), and space. -/// This hardens operator-controlled values (project names, wiki names, work item -/// types) against accidental corruption of the URL structure. -pub(crate) const PATH_SEGMENT: &AsciiSet = &CONTROLS.add(b'#').add(b'?').add(b'/').add(b' '); - -/// Safe output tools that are always available regardless of filtering. -/// These are diagnostic/transparency tools that agents should always have access to. -/// -/// Derived from diagnostic tool types — adding a new diagnostic tool means adding -/// its type here and the name is extracted automatically via `ToolResult::NAME`. -pub const ALWAYS_ON_TOOLS: &[&str] = tool_names![ - NoopResult, - MissingDataResult, - MissingToolResult, - ReportIncompleteResult, -]; - -/// Safe-output tools that require write access to ADO. -/// Compile-time derived from tool types via `ToolResult::NAME`. -/// -/// Adding a new write-requiring tool: create the struct with `tool_result!{ write = true, ... }`, -/// then add its type to this list. -pub const WRITE_REQUIRING_SAFE_OUTPUTS: &[&str] = tool_names![ - CreateWorkItemResult, - CommentOnWorkItemResult, - UpdateWorkItemResult, - CreatePrResult, - CreateWikiPageResult, - UpdateWikiPageResult, - AddPrCommentResult, - LinkWorkItemsResult, - QueueBuildResult, - CreateGitTagResult, - AddBuildTagResult, - CreateBranchResult, - UpdatePrResult, - UploadAttachmentResult, - SubmitPrReviewResult, - ReplyToPrCommentResult, - ResolvePrThreadResult, -]; - -/// Non-MCP safe-output keys handled by the compiler/executor, not the MCP server. -/// These must not appear in `--enabled-tools` or they cause real MCP tools to be -/// filtered out (the router has no route for them). -pub const NON_MCP_SAFE_OUTPUT_KEYS: &[&str] = &["memory"]; - -/// All recognised safe-output keys accepted in front matter `safe-outputs:`. -/// This is the union of write-requiring tool types, diagnostic tool types, and -/// non-MCP safe-output keys (like `memory`). -/// -/// Derived at compile time from tool types — no hand-maintained string lists. -pub const ALL_KNOWN_SAFE_OUTPUTS: &[&str] = all_safe_output_names![ - // Write-requiring MCP tools - CreateWorkItemResult, - CommentOnWorkItemResult, - UpdateWorkItemResult, - CreatePrResult, - CreateWikiPageResult, - UpdateWikiPageResult, - AddPrCommentResult, - LinkWorkItemsResult, - QueueBuildResult, - CreateGitTagResult, - AddBuildTagResult, - CreateBranchResult, - UpdatePrResult, - UploadAttachmentResult, - SubmitPrReviewResult, - ReplyToPrCommentResult, - ResolvePrThreadResult, - // Always-on diagnostics - NoopResult, - MissingDataResult, - MissingToolResult, - ReportIncompleteResult; - // Non-MCP safe-output keys - "memory" -]; - -/// Resolve the effective branch for a wiki. -/// -/// If `configured_branch` is `Some`, that value is returned directly. -/// Otherwise the wiki metadata API is queried: code wikis (type 1) return -/// the published branch from the `versions` array; project wikis (type 0) -/// return `Ok(None)` because the server handles branching internally. -/// -/// Returns `Err` when a code wiki is detected but the branch cannot be -/// resolved — callers should surface this as a user-facing failure rather -/// than proceeding to a confusing ADO PUT error. -pub(crate) async fn resolve_wiki_branch( - client: &reqwest::Client, - org_url: &str, - project: &str, - wiki_name: &str, - token: &str, - configured_branch: Option<&str>, -) -> Result, String> { - // Explicit configuration always wins. - if let Some(b) = configured_branch { - return Ok(Some(b.to_owned())); - } - - let url = format!( - "{}/{}/_apis/wiki/wikis/{}", - org_url.trim_end_matches('/'), - utf8_percent_encode(project, PATH_SEGMENT), - utf8_percent_encode(wiki_name, PATH_SEGMENT), - ); - - let resp = match client - .get(&url) - .query(&[("api-version", "7.0")]) - .basic_auth("", Some(token)) - .send() - .await - { - Ok(r) => r, - Err(e) => { - warn!("Wiki metadata request failed: {e} — skipping branch auto-detection"); - return Ok(None); - } - }; - - if !resp.status().is_success() { - warn!( - "Wiki metadata request returned HTTP {} — skipping branch auto-detection", - resp.status() - ); - return Ok(None); - } - - let body: serde_json::Value = match resp.json().await { - Ok(b) => b, - Err(e) => { - warn!("Failed to parse wiki metadata response: {e}"); - return Ok(None); - } - }; - - // Detect code wikis. ADO returns the type as a string enum ("codeWiki" / - // "projectWiki") rather than a numeric value, so we check both forms. - let is_code_wiki = match body.get("type") { - Some(serde_json::Value::String(s)) => s.eq_ignore_ascii_case("codewiki"), - Some(serde_json::Value::Number(n)) => n.as_u64() == Some(1), - _ => false, - }; - if !is_code_wiki { - let type_val = body.get("type").cloned().unwrap_or(serde_json::Value::Null); - debug!("Wiki is a project wiki (type {type_val}) — no branch needed"); - return Ok(None); - } - - // Code wiki: extract the published branch from versions[0].version - let branch = body - .get("versions") - .and_then(|v| v.as_array()) - .and_then(|arr| arr.first()) - .and_then(|v| v.get("version")) - .and_then(|v| v.as_str()) - .map(|s| s.to_owned()); - - match &branch { - Some(b) => { - debug!("Detected code wiki — resolved branch: {b}"); - Ok(branch) - } - None => Err(format!( - "Wiki '{wiki_name}' is a code wiki but its published branch could not be \ - determined. Set 'branch' explicitly in the safe-outputs config." - )), - } -} - -/// Resolve a repository alias to its ADO repo name. -/// -/// "self" (or None) → `ctx.repository_name`, otherwise look up in `ctx.allowed_repositories`. -pub(crate) fn resolve_repo_name( - repo_alias: Option<&str>, - ctx: &ExecutionContext, -) -> Result { - let alias = repo_alias.unwrap_or("self"); - if alias == "self" { - ctx.repository_name - .clone() - .ok_or_else(|| ExecutionResult::failure("BUILD_REPOSITORY_NAME not set")) - } else { - ctx.allowed_repositories - .get(alias) - .cloned() - .ok_or_else(|| { - ExecutionResult::failure(format!( - "Repository '{}' is not in the allowed repository list", - alias - )) - }) - } -} - -/// Validate a string against `git check-ref-format` rules. -/// -/// Returns `Ok(())` if the name is valid, or an `Err` describing the violation. -/// This covers the structural rules that Azure DevOps also enforces — catching -/// them early gives clearer error messages than letting the API fail. -pub(crate) fn validate_git_ref_name(name: &str, label: &str) -> anyhow::Result<()> { - use anyhow::ensure; - - ensure!(!name.is_empty(), "{label} must not be empty"); - ensure!(!name.contains(".."), "{label} must not contain '..'"); - ensure!(!name.contains("@{{"), "{label} must not contain '@{{'"); - ensure!(!name.ends_with('.'), "{label} must not end with '.'"); - ensure!(!name.ends_with(".lock"), "{label} must not end with '.lock'"); - ensure!( - !name.contains('\\'), - "{label} must not contain backslash" - ); - ensure!( - !name.contains("//"), - "{label} must not contain consecutive slashes" - ); - for ch in ['~', '^', ':', '?', '*', '['] { - ensure!( - !name.contains(ch), - "{label} must not contain '{ch}'" - ); - } - for component in name.split('/') { - ensure!( - !component.starts_with('.'), - "{label} path component must not start with '.'" - ); - } - Ok(()) -} - -mod add_build_tag; -mod add_pr_comment; -mod comment_on_work_item; -mod create_branch; -mod create_git_tag; -mod create_pr; -mod create_wiki_page; -mod create_work_item; -mod link_work_items; -pub mod memory; -mod missing_data; -mod missing_tool; -mod noop; -mod queue_build; -mod reply_to_pr_comment; -mod report_incomplete; -mod resolve_pr_thread; -mod result; -mod submit_pr_review; -mod update_pr; -mod update_wiki_page; -mod update_work_item; -mod upload_attachment; - -pub use add_build_tag::*; -pub use add_pr_comment::*; -pub use comment_on_work_item::*; -pub use create_branch::*; -pub use create_git_tag::*; -pub use create_pr::*; -pub use create_wiki_page::*; -pub use create_work_item::*; -pub use link_work_items::*; -pub use missing_data::*; -pub use missing_tool::*; -pub use noop::*; -pub use queue_build::*; -pub use reply_to_pr_comment::*; -pub use report_incomplete::*; -pub use resolve_pr_thread::*; -pub use result::{ - ExecutionContext, ExecutionResult, Executor, ToolResult, Validate, anyhow_to_mcp_error, -}; -pub use submit_pr_review::*; -pub use update_pr::*; -pub use update_wiki_page::*; -pub use update_work_item::*; -pub use upload_attachment::*; - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_write_requiring_subset_of_all_known() { - for name in WRITE_REQUIRING_SAFE_OUTPUTS { - assert!( - ALL_KNOWN_SAFE_OUTPUTS.contains(name), - "WRITE_REQUIRING_SAFE_OUTPUTS entry '{}' is missing from ALL_KNOWN_SAFE_OUTPUTS", - name - ); - } - } - - #[test] - fn test_always_on_subset_of_all_known() { - for name in ALWAYS_ON_TOOLS { - assert!( - ALL_KNOWN_SAFE_OUTPUTS.contains(name), - "ALWAYS_ON_TOOLS entry '{}' is missing from ALL_KNOWN_SAFE_OUTPUTS", - name - ); - } - } - - #[test] - fn test_non_mcp_keys_subset_of_all_known() { - for name in NON_MCP_SAFE_OUTPUT_KEYS { - assert!( - ALL_KNOWN_SAFE_OUTPUTS.contains(name), - "NON_MCP_SAFE_OUTPUT_KEYS entry '{}' is missing from ALL_KNOWN_SAFE_OUTPUTS", - name - ); - } - } - - /// Verify that every type in the write-requiring list actually has - /// `REQUIRES_WRITE == true`, and every diagnostic type has `false`. - #[test] - fn test_requires_write_consistency() { - // Write-requiring tools - assert!(CreateWorkItemResult::REQUIRES_WRITE); - assert!(CommentOnWorkItemResult::REQUIRES_WRITE); - assert!(UpdateWorkItemResult::REQUIRES_WRITE); - assert!(CreatePrResult::REQUIRES_WRITE); - assert!(CreateWikiPageResult::REQUIRES_WRITE); - assert!(UpdateWikiPageResult::REQUIRES_WRITE); - assert!(AddPrCommentResult::REQUIRES_WRITE); - assert!(LinkWorkItemsResult::REQUIRES_WRITE); - assert!(QueueBuildResult::REQUIRES_WRITE); - assert!(CreateGitTagResult::REQUIRES_WRITE); - assert!(AddBuildTagResult::REQUIRES_WRITE); - assert!(CreateBranchResult::REQUIRES_WRITE); - assert!(UpdatePrResult::REQUIRES_WRITE); - assert!(UploadAttachmentResult::REQUIRES_WRITE); - assert!(SubmitPrReviewResult::REQUIRES_WRITE); - assert!(ReplyToPrCommentResult::REQUIRES_WRITE); - assert!(ResolvePrThreadResult::REQUIRES_WRITE); - - // Diagnostic tools (should NOT require write) - assert!(!NoopResult::REQUIRES_WRITE); - assert!(!MissingDataResult::REQUIRES_WRITE); - assert!(!MissingToolResult::REQUIRES_WRITE); - assert!(!ReportIncompleteResult::REQUIRES_WRITE); - } - - /// Verify ALL_KNOWN_SAFE_OUTPUTS has exactly the right count: - /// write tools + diagnostics + non-MCP keys. - #[test] - fn test_all_known_completeness() { - // The three sub-lists must be disjoint — a tool in multiple lists would - // be duplicated in ALL_KNOWN and the count would mismatch. - for name in WRITE_REQUIRING_SAFE_OUTPUTS { - assert!( - !ALWAYS_ON_TOOLS.contains(name), - "Tool '{}' appears in both WRITE_REQUIRING and ALWAYS_ON — lists must be disjoint", - name - ); - assert!( - !NON_MCP_SAFE_OUTPUT_KEYS.contains(name), - "Tool '{}' appears in both WRITE_REQUIRING and NON_MCP — lists must be disjoint", - name - ); - } - for name in ALWAYS_ON_TOOLS { - assert!( - !NON_MCP_SAFE_OUTPUT_KEYS.contains(name), - "Tool '{}' appears in both ALWAYS_ON and NON_MCP — lists must be disjoint", - name - ); - } - - let expected = WRITE_REQUIRING_SAFE_OUTPUTS.len() - + ALWAYS_ON_TOOLS.len() - + NON_MCP_SAFE_OUTPUT_KEYS.len(); - assert_eq!( - ALL_KNOWN_SAFE_OUTPUTS.len(), - expected, - "ALL_KNOWN_SAFE_OUTPUTS should be the union of write + diagnostic + non-MCP lists" - ); - } -} +//! First-class tool implementations for the ado-aw compiler. +//! +//! These tools are configured via the `tools:` front-matter section and provide +//! built-in functionality that the compiler knows how to auto-configure +//! (pipeline steps, MCPG entries, network allowlists, etc.). +//! +//! This is distinct from `safeoutputs/` which contains safe-output MCP tools +//! that serialize to NDJSON in Stage 1 and execute in Stage 2. + +pub mod cache_memory; diff --git a/templates/base.yml b/templates/base.yml index b9411714..203c484c 100644 --- a/templates/base.yml +++ b/templates/base.yml @@ -199,7 +199,7 @@ jobs: docker pull ghcr.io/github/gh-aw-firewall/agent:{{ firewall_version }} docker tag ghcr.io/github/gh-aw-firewall/squid:{{ firewall_version }} ghcr.io/github/gh-aw-firewall/squid:latest docker tag ghcr.io/github/gh-aw-firewall/agent:{{ firewall_version }} ghcr.io/github/gh-aw-firewall/agent:latest - docker pull ghcr.io/github/gh-aw-mcpg:v{{ mcpg_version }} + docker pull {{ mcpg_image }}:v{{ mcpg_version }} displayName: "Pre-pull AWF and MCPG container images (v{{ firewall_version }})" {{ prepare_steps }} @@ -270,7 +270,7 @@ jobs: -v /var/run/docker.sock:/var/run/docker.sock \ -e MCP_GATEWAY_API_KEY="$(MCP_GATEWAY_API_KEY)" \ {{ mcpg_docker_env }} - ghcr.io/github/gh-aw-mcpg:v{{ mcpg_version }} & + {{ mcpg_image }}:v{{ mcpg_version }} & MCPG_PID=$! echo "MCPG started (PID: $MCPG_PID)" diff --git a/tests/compiler_tests.rs b/tests/compiler_tests.rs index d11a4931..52043400 100644 --- a/tests/compiler_tests.rs +++ b/tests/compiler_tests.rs @@ -2371,8 +2371,8 @@ fn test_parameters_clear_memory_auto_injected() { let input = r#"--- name: "Memory Agent" description: "Tests clearMemory auto-injection" -safe-outputs: - memory: +tools: + cache-memory: allowed-extensions: - .md --- @@ -2435,8 +2435,8 @@ parameters: displayName: "Reset memory" type: boolean default: true -safe-outputs: - memory: +tools: + cache-memory: allowed-extensions: - .md --- @@ -2487,8 +2487,8 @@ parameters: - name: myParam type: string default: "hello" -safe-outputs: - memory: +tools: + cache-memory: allowed-extensions: - .md ---