Skip to content

Commit 2c1068b

Browse files
refactor: unify runtime/tool requirements via CompilerExtension trait (#211)
* refactor: unify runtime/tool requirements via CompilerExtension trait Introduce a CompilerExtension trait that runtimes and first-party tools implement to declare their compilation requirements (network hosts, bash commands, prompt supplements, prepare steps, MCPG entries). Previously, each runtime/tool required scattered special-case code across standalone.rs, common.rs, and other files. Now: - LeanExtension: declares hosts, bash cmds, install steps, prompt - AzureDevOpsExtension: declares hosts, MCPG entry, validation - CacheMemoryExtension: declares download steps, prompt supplement The compiler collects all enabled extensions via collect_extensions() and iterates over them generically -- no more bespoke if-blocks. Adding a new runtime or tool is now a 2-step process: 1. Implement CompilerExtension 2. Add to collect_extensions() MCPG types (McpgServerConfig, McpgConfig, etc.) moved from standalone.rs to extensions.rs for shared access. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address review feedback on CompilerExtension - Add validation loop to OneESCompiler (bug: Lean/ADO warnings were silently dropped for target: 1es agents) - Add debug_assert on display_name in wrap_prompt_append to guard against future extension names with shell/YAML metacharacters - Document why CacheMemoryExtension.config is #[allow(dead_code)] (options consumed at Stage 2, not compile time) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat: add ExtensionPhase ordering policy (runtimes before tools) Extensions now declare their phase via phase() -> ExtensionPhase. collect_extensions() sorts by phase using stable sort, guaranteeing runtimes install before tools — critical when a tool depends on a runtime (e.g., uv requires Python to be installed first). Phases: Runtime (0) → Tool (1). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: harden wrap_prompt_append and document validate() limitations - Replace debug_assert! with anyhow::ensure! in wrap_prompt_append so unsafe display_name characters are rejected in release builds too. Function now returns Result<String>. - Add doc comment to validate() explaining that inferred_org is not available at validation time — org-dependent checks belong in mcpg_servers() which receives the fully populated CompileContext. - Add test for unsafe display_name rejection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 85ac3ab commit 2c1068b

6 files changed

Lines changed: 1151 additions & 361 deletions

File tree

AGENTS.md

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1447,9 +1447,27 @@ When extending the compiler:
14471447
3. **New front matter fields**: Add fields to `FrontMatter` in `src/compile/types.rs`
14481448
4. **New template markers**: Handle replacements in the target-specific compiler (e.g., `standalone.rs` or `onees.rs`)
14491449
5. **New safe-output tools**: Add to `src/safeoutputs/` — implement `ToolResult`, `Executor`, register in `mod.rs`, `mcp.rs`, `execute.rs`
1450-
6. **New first-class tools**: Add to `src/tools/` — extend `ToolsConfig` in `types.rs`, wire in compilers
1451-
7. **New runtimes**: Add to `src/runtimes/` — extend `RuntimesConfig` in `types.rs`, wire in compilers
1452-
7. **Validation**: Add compile-time validation for safe outputs and permissions
1450+
6. **New first-class tools**: Add to `src/tools/` — extend `ToolsConfig` in `types.rs`, implement `CompilerExtension` trait in `src/compile/extensions.rs`, add collection in `collect_extensions()`
1451+
7. **New runtimes**: Add to `src/runtimes/` — extend `RuntimesConfig` in `types.rs`, implement `CompilerExtension` trait in `src/compile/extensions.rs`, add collection in `collect_extensions()`
1452+
8. **Validation**: Add compile-time validation for safe outputs and permissions
1453+
1454+
#### `CompilerExtension` Trait
1455+
1456+
Runtimes and first-party tools declare their compilation requirements via the `CompilerExtension` trait (`src/compile/extensions.rs`). Instead of scattering special-case `if` blocks across the compiler, each runtime/tool implements this trait and the compiler collects requirements generically:
1457+
1458+
```rust
1459+
pub trait CompilerExtension: Send {
1460+
fn name(&self) -> &str; // Display name
1461+
fn required_hosts(&self) -> Vec<String>; // AWF network allowlist
1462+
fn required_bash_commands(&self) -> Vec<String>; // Agent bash allow-list
1463+
fn prompt_supplement(&self) -> Option<String>; // Agent prompt markdown
1464+
fn prepare_steps(&self) -> Vec<String>; // Pipeline steps (install, etc.)
1465+
fn mcpg_servers(&self, ctx) -> Result<Vec<(String, McpgServerConfig)>>; // MCPG entries
1466+
fn validate(&self, ctx) -> Result<Vec<String>>; // Compile-time warnings
1467+
}
1468+
```
1469+
1470+
To add a new runtime or tool: (1) create a struct implementing `CompilerExtension`, (2) add a collection check in `collect_extensions()`. No other files need modification.
14531471

14541472
### Security Considerations
14551473

src/compile/common.rs

Lines changed: 30 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
use anyhow::{Context, Result};
44

55
use super::types::{FrontMatter, PipelineParameter, Repository, TriggerConfig};
6+
use super::extensions::CompilerExtension;
67
use crate::compile::types::McpConfig;
78
use crate::fuzzy_schedule;
89

@@ -470,7 +471,10 @@ const DEFAULT_BASH_COMMANDS: &[&str] = &[
470471
];
471472

472473
/// Generate copilot CLI params from front matter configuration
473-
pub fn generate_copilot_params(front_matter: &FrontMatter) -> Result<String> {
474+
pub fn generate_copilot_params(
475+
front_matter: &FrontMatter,
476+
extensions: &[super::extensions::Extension],
477+
) -> Result<String> {
474478
let mut allowed_tools: Vec<String> = vec!["github".to_string(), "safeoutputs".to_string()];
475479

476480
// Edit tool: enabled by default, can be disabled with `edit: false`
@@ -484,7 +488,7 @@ pub fn generate_copilot_params(front_matter: &FrontMatter) -> Result<String> {
484488
}
485489

486490
// Bash tool: use configured list, or defaults if not specified
487-
let mut bash_commands: Vec<&str> =
491+
let mut bash_commands: Vec<String> =
488492
match front_matter.tools.as_ref().and_then(|t| t.bash.as_ref()) {
489493
Some(cmds) if cmds.len() == 1 && cmds[0] == ":*" => {
490494
// Unrestricted: single wildcard entry
@@ -497,50 +501,32 @@ pub fn generate_copilot_params(front_matter: &FrontMatter) -> Result<String> {
497501
}
498502
Some(cmds) => {
499503
// Explicit list of commands
500-
cmds.iter().map(|s| s.as_str()).collect()
504+
cmds.clone()
501505
}
502506
None => {
503507
// Default safe commands
504-
DEFAULT_BASH_COMMANDS.to_vec()
508+
DEFAULT_BASH_COMMANDS.iter().map(|s| (*s).to_string()).collect()
505509
}
506510
};
507511

508-
// Auto-add lean/lake/elan when runtimes.lean is enabled
509-
let has_lean = front_matter
510-
.runtimes
511-
.as_ref()
512-
.and_then(|r| r.lean.as_ref())
513-
.is_some_and(|l| l.is_enabled());
514-
512+
// Auto-add extension-declared bash commands (runtimes + first-party tools)
515513
let is_unrestricted_bash = front_matter
516514
.tools
517515
.as_ref()
518516
.and_then(|t| t.bash.as_ref())
519517
.is_some_and(|cmds| cmds.len() == 1 && cmds[0] == ":*");
520518

521-
if has_lean && !is_unrestricted_bash {
522-
let bash_disabled = front_matter
523-
.tools
524-
.as_ref()
525-
.and_then(|t| t.bash.as_ref())
526-
.is_some_and(|cmds| cmds.is_empty());
527-
528-
if bash_disabled {
529-
eprintln!(
530-
"Warning: Agent '{}' has runtimes.lean enabled but tools.bash is empty. \
531-
Lean requires bash access (lean, lake, elan commands).",
532-
front_matter.name
533-
);
534-
} else {
535-
for cmd in crate::runtimes::lean::LEAN_BASH_COMMANDS {
536-
if !bash_commands.contains(cmd) {
519+
if !is_unrestricted_bash {
520+
for ext in extensions {
521+
for cmd in ext.required_bash_commands() {
522+
if !bash_commands.contains(&cmd) {
537523
bash_commands.push(cmd);
538524
}
539525
}
540526
}
541527
}
542528

543-
for cmd in bash_commands {
529+
for cmd in &bash_commands {
544530
// Reject single quotes in bash commands — copilot_params are embedded inside
545531
// a single-quoted bash string in the AWF command.
546532
if cmd.contains('\'') {
@@ -1336,7 +1322,7 @@ mod tests {
13361322
cache_memory: None,
13371323
azure_devops: None,
13381324
});
1339-
let params = generate_copilot_params(&fm).unwrap();
1325+
let params = generate_copilot_params(&fm, &crate::compile::extensions::collect_extensions(&fm)).unwrap();
13401326
assert!(params.contains("--allow-tool \"shell(:*)\""));
13411327
}
13421328

@@ -1349,7 +1335,7 @@ mod tests {
13491335
cache_memory: None,
13501336
azure_devops: None,
13511337
});
1352-
let params = generate_copilot_params(&fm).unwrap();
1338+
let params = generate_copilot_params(&fm, &crate::compile::extensions::collect_extensions(&fm)).unwrap();
13531339
assert!(!params.contains("shell("));
13541340
}
13551341

@@ -1359,7 +1345,7 @@ mod tests {
13591345
fm.runtimes = Some(crate::compile::types::RuntimesConfig {
13601346
lean: Some(crate::runtimes::lean::LeanRuntimeConfig::Enabled(true)),
13611347
});
1362-
let params = generate_copilot_params(&fm).unwrap();
1348+
let params = generate_copilot_params(&fm, &crate::compile::extensions::collect_extensions(&fm)).unwrap();
13631349
assert!(params.contains("shell(lean)"), "lean command should be allowed");
13641350
assert!(params.contains("shell(lake)"), "lake command should be allowed");
13651351
assert!(params.contains("shell(elan)"), "elan command should be allowed");
@@ -1379,7 +1365,7 @@ mod tests {
13791365
fm.runtimes = Some(crate::compile::types::RuntimesConfig {
13801366
lean: Some(crate::runtimes::lean::LeanRuntimeConfig::Enabled(true)),
13811367
});
1382-
let params = generate_copilot_params(&fm).unwrap();
1368+
let params = generate_copilot_params(&fm, &crate::compile::extensions::collect_extensions(&fm)).unwrap();
13831369
assert!(params.contains("shell(:*)"), "unrestricted should be preserved");
13841370
// Should NOT add individual lean commands when unrestricted
13851371
assert!(!params.contains("shell(lean)"), "lean not needed with :*");
@@ -1395,7 +1381,7 @@ mod tests {
13951381
..Default::default()
13961382
}),
13971383
);
1398-
let params = generate_copilot_params(&fm).unwrap();
1384+
let params = generate_copilot_params(&fm, &crate::compile::extensions::collect_extensions(&fm)).unwrap();
13991385
assert!(!params.contains("--mcp my-tool"));
14001386
}
14011387

@@ -1404,7 +1390,7 @@ mod tests {
14041390
let mut fm = minimal_front_matter();
14051391
fm.mcp_servers
14061392
.insert("ado".to_string(), McpConfig::Enabled(true));
1407-
let params = generate_copilot_params(&fm).unwrap();
1393+
let params = generate_copilot_params(&fm, &crate::compile::extensions::collect_extensions(&fm)).unwrap();
14081394
// Copilot CLI has no built-in MCPs — all MCPs are handled via the MCP firewall
14091395
assert!(!params.contains("--mcp ado"));
14101396
}
@@ -1415,14 +1401,14 @@ mod tests {
14151401
"---\nname: test\ndescription: test\nengine:\n model: claude-opus-4.5\n max-turns: 50\n---\n",
14161402
)
14171403
.unwrap();
1418-
let params = generate_copilot_params(&fm).unwrap();
1404+
let params = generate_copilot_params(&fm, &crate::compile::extensions::collect_extensions(&fm)).unwrap();
14191405
assert!(!params.contains("--max-turns"), "max-turns should not be emitted as a CLI arg");
14201406
}
14211407

14221408
#[test]
14231409
fn test_copilot_params_no_max_turns_when_simple_engine() {
14241410
let fm = minimal_front_matter();
1425-
let params = generate_copilot_params(&fm).unwrap();
1411+
let params = generate_copilot_params(&fm, &crate::compile::extensions::collect_extensions(&fm)).unwrap();
14261412
assert!(!params.contains("--max-turns"));
14271413
}
14281414

@@ -1432,14 +1418,14 @@ mod tests {
14321418
"---\nname: test\ndescription: test\nengine:\n model: claude-opus-4.5\n timeout-minutes: 30\n---\n",
14331419
)
14341420
.unwrap();
1435-
let params = generate_copilot_params(&fm).unwrap();
1421+
let params = generate_copilot_params(&fm, &crate::compile::extensions::collect_extensions(&fm)).unwrap();
14361422
assert!(!params.contains("--max-timeout"), "timeout-minutes should not be emitted as a CLI arg");
14371423
}
14381424

14391425
#[test]
14401426
fn test_copilot_params_no_max_timeout_when_simple_engine() {
14411427
let fm = minimal_front_matter();
1442-
let params = generate_copilot_params(&fm).unwrap();
1428+
let params = generate_copilot_params(&fm, &crate::compile::extensions::collect_extensions(&fm)).unwrap();
14431429
assert!(!params.contains("--max-timeout"));
14441430
}
14451431

@@ -1449,7 +1435,7 @@ mod tests {
14491435
"---\nname: test\ndescription: test\nengine:\n model: claude-opus-4.5\n max-turns: 0\n---\n",
14501436
)
14511437
.unwrap();
1452-
let params = generate_copilot_params(&fm).unwrap();
1438+
let params = generate_copilot_params(&fm, &crate::compile::extensions::collect_extensions(&fm)).unwrap();
14531439
assert!(!params.contains("--max-turns"), "max-turns should not be emitted as a CLI arg");
14541440
}
14551441

@@ -1459,7 +1445,7 @@ mod tests {
14591445
"---\nname: test\ndescription: test\nengine:\n model: claude-opus-4.5\n timeout-minutes: 0\n---\n",
14601446
)
14611447
.unwrap();
1462-
let params = generate_copilot_params(&fm).unwrap();
1448+
let params = generate_copilot_params(&fm, &crate::compile::extensions::collect_extensions(&fm)).unwrap();
14631449
assert!(!params.contains("--max-timeout"), "timeout-minutes should not be emitted as a CLI arg");
14641450
}
14651451

@@ -2384,7 +2370,7 @@ mod tests {
23842370
)
23852371
.unwrap();
23862372
fm.engine = crate::compile::types::EngineConfig::Simple("model' && echo pwned".to_string());
2387-
let result = generate_copilot_params(&fm);
2373+
let result = generate_copilot_params(&fm, &crate::compile::extensions::collect_extensions(&fm));
23882374
assert!(result.is_err());
23892375
assert!(result.unwrap_err().to_string().contains("invalid characters"));
23902376
}
@@ -2393,7 +2379,7 @@ mod tests {
23932379
fn test_model_name_rejects_space() {
23942380
let mut fm = minimal_front_matter();
23952381
fm.engine = crate::compile::types::EngineConfig::Simple("model && curl evil.com".to_string());
2396-
let result = generate_copilot_params(&fm);
2382+
let result = generate_copilot_params(&fm, &crate::compile::extensions::collect_extensions(&fm));
23972383
assert!(result.is_err());
23982384
}
23992385

@@ -2402,7 +2388,7 @@ mod tests {
24022388
for name in &["claude-opus-4.5", "gpt-5.2-codex", "gemini-3-pro-preview", "my_model:latest"] {
24032389
let mut fm = minimal_front_matter();
24042390
fm.engine = crate::compile::types::EngineConfig::Simple(name.to_string());
2405-
let result = generate_copilot_params(&fm);
2391+
let result = generate_copilot_params(&fm, &crate::compile::extensions::collect_extensions(&fm));
24062392
assert!(result.is_ok(), "Model name '{}' should be valid", name);
24072393
}
24082394
}
@@ -2416,7 +2402,7 @@ mod tests {
24162402
cache_memory: None,
24172403
azure_devops: None,
24182404
});
2419-
let result = generate_copilot_params(&fm);
2405+
let result = generate_copilot_params(&fm, &crate::compile::extensions::collect_extensions(&fm));
24202406
assert!(result.is_err());
24212407
assert!(result.unwrap_err().to_string().contains("single quote"));
24222408
}

0 commit comments

Comments
 (0)