Skip to content

Commit e63f3ce

Browse files
jamesadevineCopilot
jamesadevine
and
Copilot
authored
refactor: redesign CompileContext with rich metadata and async constructor (#212)
* refactor: redesign CompileContext with rich metadata and async constructor Replace the ad-hoc inferred_org: Option<&str> field with a properly structured CompileContext that follows the ExecutionContext pattern: - ado_context: Option<AdoContext> — full ADO metadata (org, project, repo) - compile_dir: &Path — for future extensions needing path resolution - async fn new() — resolves ADO context from git remote eagerly - ado_org() convenience accessor This eliminates the split where standalone.rs inferred the org inline (30-line block) and passed it separately to generate_mcpg_config(). Now CompileContext is built once, early, and passed uniformly to all extension methods — validate() and mcpg_servers() both receive the same fully-populated context. Test helpers: for_test() and for_test_with_org() avoid async in tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: remove unused compile_dir field and simplify ado_org() - Remove compile_dir from CompileContext (unused, was speculative). compile_dir is still accepted by new() for ADO inference but not stored — it can be re-added when an extension actually needs it. - Simplify ado_org() to use and_then + ? instead of map + unwrap_or + filter chain. rsplit("/").next() always returns Some, so the unwrap_or("") was unreachable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 2c1068b commit e63f3ce

3 files changed

Lines changed: 148 additions & 141 deletions

File tree

‎src/compile/extensions.rs‎

Lines changed: 102 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,96 @@ pub struct McpgConfig {
8282
// Compile context
8383
// ──────────────────────────────────────────────────────────────────────
8484

85-
/// Shared context passed to extension methods that need cross-cutting information.
85+
use crate::configure::AdoContext;
86+
use std::path::Path;
87+
88+
/// Metadata resolved at compile time from the local environment.
89+
///
90+
/// Built once via [`CompileContext::new`] and passed to all extension
91+
/// methods. Follows the same pattern as
92+
/// [`ExecutionContext`](crate::safeoutputs::result::ExecutionContext)
93+
/// for Stage 2 — a single context struct with all resolved metadata.
8694
pub struct CompileContext<'a> {
8795
/// The agent name from front matter.
8896
pub agent_name: &'a str,
8997
/// The full front matter (for cross-cutting checks like bash access level).
9098
pub front_matter: &'a FrontMatter,
91-
/// ADO org inferred from the git remote at compile time. Used by
92-
/// `AzureDevOpsExtension` when no explicit `org:` is provided.
93-
pub inferred_org: Option<&'a str>,
99+
/// ADO context inferred from the git remote (org URL, project, repo name).
100+
/// `None` if the compile directory has no ADO remote.
101+
pub ado_context: Option<AdoContext>,
102+
}
103+
104+
impl<'a> CompileContext<'a> {
105+
/// Build a fully-resolved compile context.
106+
///
107+
/// Infers ADO context from the git remote in `compile_dir`. This is
108+
/// async because it shells out to `git remote get-url origin`.
109+
pub async fn new(front_matter: &'a FrontMatter, compile_dir: &Path) -> Self {
110+
let ado_context = Self::infer_ado_context(compile_dir).await;
111+
Self {
112+
agent_name: &front_matter.name,
113+
front_matter,
114+
ado_context,
115+
}
116+
}
117+
118+
/// Convenience accessor: extract the ADO org name from the inferred context.
119+
pub fn ado_org(&self) -> Option<&str> {
120+
self.ado_context.as_ref().and_then(|ctx| {
121+
let org = ctx.org_url.trim_end_matches('/').rsplit('/').next()?;
122+
if org.is_empty() { None } else { Some(org) }
123+
})
124+
}
125+
126+
async fn infer_ado_context(dir: &Path) -> Option<AdoContext> {
127+
match crate::configure::get_git_remote_url(dir).await {
128+
Ok(url) => match crate::configure::parse_ado_remote(&url) {
129+
Ok(ctx) => {
130+
log::info!(
131+
"Inferred ADO org from git remote: {}",
132+
ctx.org_url
133+
.trim_end_matches('/')
134+
.rsplit('/')
135+
.next()
136+
.unwrap_or("?")
137+
);
138+
Some(ctx)
139+
}
140+
Err(_) => {
141+
log::debug!("Git remote is not an ADO URL — cannot infer org");
142+
None
143+
}
144+
},
145+
Err(_) => {
146+
log::debug!("No git remote found — cannot infer ADO context");
147+
None
148+
}
149+
}
150+
}
151+
152+
/// Create a context for tests (no async, no git remote inference).
153+
#[cfg(test)]
154+
pub fn for_test(front_matter: &'a FrontMatter) -> Self {
155+
Self {
156+
agent_name: &front_matter.name,
157+
front_matter,
158+
ado_context: None,
159+
}
160+
}
161+
162+
/// Create a context for tests with a specific ADO org.
163+
#[cfg(test)]
164+
pub fn for_test_with_org(front_matter: &'a FrontMatter, org: &str) -> Self {
165+
Self {
166+
agent_name: &front_matter.name,
167+
front_matter,
168+
ado_context: Some(AdoContext {
169+
org_url: format!("https://dev.azure.com/{}", org),
170+
project: "test-project".to_string(),
171+
repo_name: "test-repo".to_string(),
172+
}),
173+
}
174+
}
94175
}
95176

96177
// ──────────────────────────────────────────────────────────────────────
@@ -169,12 +250,6 @@ pub trait CompilerExtension {
169250
/// Compile-time warnings to emit. Errors in the `Result` abort
170251
/// compilation; the inner `Vec<String>` contains non-fatal warnings
171252
/// printed to stderr.
172-
///
173-
/// **Note:** `validate` is called early in compilation before
174-
/// `inferred_org` is available (it requires an async git remote
175-
/// lookup). Org-dependent validation (e.g., verifying an ADO org
176-
/// can be resolved) should go in [`mcpg_servers`](Self::mcpg_servers)
177-
/// instead, which receives the fully populated [`CompileContext`].
178253
fn validate(&self, _ctx: &CompileContext) -> Result<Vec<String>> {
179254
Ok(vec![])
180255
}
@@ -371,19 +446,20 @@ impl CompilerExtension for AzureDevOpsExtension {
371446
// Build entrypoint args: npx -y @azure-devops/mcp <org> [-d toolset1 toolset2 ...]
372447
let mut entrypoint_args = vec!["-y".to_string(), ADO_MCP_PACKAGE.to_string()];
373448

374-
// Org: use explicit override, then compile-time inferred, then fail
375-
let org = if let Some(explicit) = self.config.org() {
376-
explicit.to_string()
377-
} else if let Some(inferred) = ctx.inferred_org {
378-
inferred.to_string()
379-
} else {
380-
anyhow::bail!(
381-
"Agent '{}' has tools.azure-devops enabled but no ADO organization could be \
382-
determined. Either set tools.azure-devops.org explicitly, or compile from \
383-
within a git repository with an Azure DevOps remote URL.",
384-
ctx.agent_name
385-
);
386-
};
449+
// Org: use explicit override, then inferred from git remote, then fail
450+
let org = self
451+
.config
452+
.org()
453+
.map(|s| s.to_string())
454+
.or_else(|| ctx.ado_org().map(|s| s.to_string()))
455+
.ok_or_else(|| {
456+
anyhow::anyhow!(
457+
"Agent '{}' has tools.azure-devops enabled but no ADO organization could be \
458+
determined. Either set tools.azure-devops.org explicitly, or compile from \
459+
within a git repository with an Azure DevOps remote URL.",
460+
ctx.agent_name
461+
)
462+
})?;
387463
if !org.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') {
388464
anyhow::bail!(
389465
"Invalid ADO org name '{}': must contain only alphanumerics and hyphens",
@@ -678,11 +754,7 @@ mod tests {
678754
}
679755

680756
fn ctx_from(fm: &FrontMatter) -> CompileContext<'_> {
681-
CompileContext {
682-
agent_name: &fm.name,
683-
front_matter: fm,
684-
inferred_org: None,
685-
}
757+
CompileContext::for_test(fm)
686758
}
687759

688760
// ── collect_extensions ──────────────────────────────────────────
@@ -849,11 +921,7 @@ mod tests {
849921
#[test]
850922
fn test_ado_mcpg_servers_with_inferred_org() {
851923
let fm = minimal_front_matter();
852-
let ctx = CompileContext {
853-
agent_name: &fm.name,
854-
front_matter: &fm,
855-
inferred_org: Some("myorg"),
856-
};
924+
let ctx = CompileContext::for_test_with_org(&fm, "myorg");
857925
let ext = AzureDevOpsExtension::new(AzureDevOpsToolConfig::Enabled(true));
858926
let servers = ext.mcpg_servers(&ctx).unwrap();
859927
assert_eq!(servers.len(), 1);
@@ -870,11 +938,7 @@ mod tests {
870938
#[test]
871939
fn test_ado_mcpg_servers_no_org_fails() {
872940
let fm = minimal_front_matter();
873-
let ctx = CompileContext {
874-
agent_name: &fm.name,
875-
front_matter: &fm,
876-
inferred_org: None,
877-
};
941+
let ctx = CompileContext::for_test(&fm);
878942
let ext = AzureDevOpsExtension::new(AzureDevOpsToolConfig::Enabled(true));
879943
assert!(ext.mcpg_servers(&ctx).is_err());
880944
}

‎src/compile/onees.rs‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,17 +67,14 @@ impl Compiler for OneESCompiler {
6767
let checkout_self = generate_checkout_self();
6868
let extensions = super::extensions::collect_extensions(front_matter);
6969

70+
// Build compile context with inferred metadata
71+
let input_dir = input_path.parent().unwrap_or(std::path::Path::new("."));
72+
let ctx = super::extensions::CompileContext::new(front_matter, input_dir).await;
73+
7074
// Run extension validations (warnings + errors)
71-
{
72-
let validation_ctx = super::extensions::CompileContext {
73-
agent_name: &front_matter.name,
74-
front_matter,
75-
inferred_org: None,
76-
};
77-
for ext in &extensions {
78-
for warning in ext.validate(&validation_ctx)? {
79-
eprintln!("Warning: {}", warning);
80-
}
75+
for ext in &extensions {
76+
for warning in ext.validate(&ctx)? {
77+
eprintln!("Warning: {}", warning);
8178
}
8279
}
8380

0 commit comments

Comments
 (0)