Skip to content

Commit 5c6fe28

Browse files
authored
feat: added mcp admin level configuration with GetProfile (#2639)
* first pass * add notification when /mcp & /tools * clear all tool related filed in agent * store mcp_enabled in chatsession & conversationstate * delete duplicate api call * set mcp_enabled value after load * remove clear mcp configs method * clippy * remain@builtin/ and *, add a ut for clear mcp config
1 parent 0ce09b4 commit 5c6fe28

File tree

11 files changed

+272
-42
lines changed

11 files changed

+272
-42
lines changed

crates/chat-cli/src/api_client/error.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use amzn_codewhisperer_client::operation::create_subscription_token::CreateSubscriptionTokenError;
22
use amzn_codewhisperer_client::operation::generate_completions::GenerateCompletionsError;
3+
use amzn_codewhisperer_client::operation::get_profile::GetProfileError;
34
use amzn_codewhisperer_client::operation::list_available_customizations::ListAvailableCustomizationsError;
45
use amzn_codewhisperer_client::operation::list_available_models::ListAvailableModelsError;
56
use amzn_codewhisperer_client::operation::list_available_profiles::ListAvailableProfilesError;
@@ -100,6 +101,9 @@ pub enum ApiClientError {
100101

101102
#[error("No default model found in the ListAvailableModels API response")]
102103
DefaultModelNotFound,
104+
105+
#[error(transparent)]
106+
GetProfileError(#[from] SdkError<GetProfileError, HttpResponse>),
103107
}
104108

105109
impl ApiClientError {
@@ -125,6 +129,7 @@ impl ApiClientError {
125129
Self::Credentials(_e) => None,
126130
Self::ListAvailableModelsError(e) => sdk_status_code(e),
127131
Self::DefaultModelNotFound => None,
132+
Self::GetProfileError(e) => sdk_status_code(e),
128133
}
129134
}
130135
}
@@ -152,6 +157,7 @@ impl ReasonCode for ApiClientError {
152157
Self::Credentials(_) => "CredentialsError".to_string(),
153158
Self::ListAvailableModelsError(e) => sdk_error_code(e),
154159
Self::DefaultModelNotFound => "DefaultModelNotFound".to_string(),
160+
Self::GetProfileError(e) => sdk_error_code(e),
155161
}
156162
}
157163
}
@@ -199,6 +205,10 @@ mod tests {
199205
ListAvailableCustomizationsError::unhandled("<unhandled>"),
200206
response(),
201207
)),
208+
ApiClientError::GetProfileError(SdkError::service_error(
209+
GetProfileError::unhandled("<unhandled>"),
210+
response(),
211+
)),
202212
ApiClientError::ListAvailableModelsError(SdkError::service_error(
203213
ListAvailableModelsError::unhandled("<unhandled>"),
204214
response(),

crates/chat-cli/src/api_client/mod.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use amzn_codewhisperer_client::operation::create_subscription_token::CreateSubsc
1616
use amzn_codewhisperer_client::types::Origin::Cli;
1717
use amzn_codewhisperer_client::types::{
1818
Model,
19+
OptInFeatureToggle,
1920
OptOutPreference,
2021
SubscriptionStatus,
2122
TelemetryEvent,
@@ -334,6 +335,21 @@ impl ApiClient {
334335
Ok(res)
335336
}
336337

338+
pub async fn is_mcp_enabled(&self) -> Result<bool, ApiClientError> {
339+
let request = self
340+
.client
341+
.get_profile()
342+
.set_profile_arn(self.profile.as_ref().map(|p| p.arn.clone()));
343+
344+
let response = request.send().await?;
345+
let mcp_enabled = response
346+
.profile()
347+
.opt_in_features()
348+
.and_then(|features| features.mcp_configuration())
349+
.is_none_or(|config| matches!(config.toggle(), OptInFeatureToggle::On));
350+
Ok(mcp_enabled)
351+
}
352+
337353
pub async fn create_subscription_token(&self) -> Result<CreateSubscriptionTokenOutput, ApiClientError> {
338354
if cfg!(test) {
339355
return Ok(CreateSubscriptionTokenOutput::builder()

crates/chat-cli/src/cli/agent/mod.rs

Lines changed: 143 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -286,23 +286,52 @@ impl Agent {
286286
os: &Os,
287287
agent_path: impl AsRef<Path>,
288288
legacy_mcp_config: &mut Option<McpServerConfig>,
289+
mcp_enabled: bool,
289290
) -> Result<Agent, AgentConfigError> {
290291
let content = os.fs.read(&agent_path).await?;
291292
let mut agent = serde_json::from_slice::<Agent>(&content).map_err(|e| AgentConfigError::InvalidJson {
292293
error: e,
293294
path: agent_path.as_ref().to_path_buf(),
294295
})?;
295296

296-
if agent.use_legacy_mcp_json && legacy_mcp_config.is_none() {
297-
let config = load_legacy_mcp_config(os).await.unwrap_or_default();
298-
if let Some(config) = config {
299-
legacy_mcp_config.replace(config);
297+
if mcp_enabled {
298+
if agent.use_legacy_mcp_json && legacy_mcp_config.is_none() {
299+
let config = load_legacy_mcp_config(os).await.unwrap_or_default();
300+
if let Some(config) = config {
301+
legacy_mcp_config.replace(config);
302+
}
300303
}
304+
agent.thaw(agent_path.as_ref(), legacy_mcp_config.as_ref())?;
305+
} else {
306+
agent.clear_mcp_configs();
307+
// Thaw the agent with empty MCP config to finalize normalization.
308+
agent.thaw(agent_path.as_ref(), None)?;
301309
}
302-
303-
agent.thaw(agent_path.as_ref(), legacy_mcp_config.as_ref())?;
304310
Ok(agent)
305311
}
312+
313+
/// Clear all MCP configurations while preserving built-in tools
314+
pub fn clear_mcp_configs(&mut self) {
315+
self.mcp_servers = McpServerConfig::default();
316+
self.use_legacy_mcp_json = false;
317+
318+
// Transform tools: "*" → "@builtin", remove MCP refs
319+
self.tools = self
320+
.tools
321+
.iter()
322+
.filter_map(|tool| match tool.as_str() {
323+
"*" => Some("@builtin".to_string()),
324+
t if !is_mcp_tool_ref(t) => Some(t.to_string()),
325+
_ => None,
326+
})
327+
.collect();
328+
329+
// Remove MCP references from other fields
330+
self.allowed_tools.retain(|tool| !is_mcp_tool_ref(tool));
331+
self.tool_aliases.retain(|orig, _| !is_mcp_tool_ref(&orig.to_string()));
332+
self.tools_settings
333+
.retain(|target, _| !is_mcp_tool_ref(&target.to_string()));
334+
}
306335
}
307336

308337
/// Result of evaluating tool permissions, indicating whether a tool should be allowed,
@@ -382,7 +411,19 @@ impl Agents {
382411
agent_name: Option<&str>,
383412
skip_migration: bool,
384413
output: &mut impl Write,
414+
mcp_enabled: bool,
385415
) -> (Self, AgentsLoadMetadata) {
416+
if !mcp_enabled {
417+
let _ = execute!(
418+
output,
419+
style::SetForegroundColor(Color::Yellow),
420+
style::Print("\n"),
421+
style::Print("⚠️ WARNING: "),
422+
style::SetForegroundColor(Color::Reset),
423+
style::Print("MCP functionality has been disabled by your administrator.\n\n"),
424+
);
425+
}
426+
386427
// Tracking metadata about the performed load operation.
387428
let mut load_metadata = AgentsLoadMetadata::default();
388429

@@ -429,7 +470,7 @@ impl Agents {
429470
};
430471

431472
let mut agents = Vec::<Agent>::new();
432-
let results = load_agents_from_entries(files, os, &mut global_mcp_config).await;
473+
let results = load_agents_from_entries(files, os, &mut global_mcp_config, mcp_enabled).await;
433474
for result in results {
434475
match result {
435476
Ok(agent) => agents.push(agent),
@@ -467,7 +508,7 @@ impl Agents {
467508
};
468509

469510
let mut agents = Vec::<Agent>::new();
470-
let results = load_agents_from_entries(files, os, &mut global_mcp_config).await;
511+
let results = load_agents_from_entries(files, os, &mut global_mcp_config, mcp_enabled).await;
471512
for result in results {
472513
match result {
473514
Ok(agent) => agents.push(agent),
@@ -607,27 +648,30 @@ impl Agents {
607648

608649
all_agents.push({
609650
let mut agent = Agent::default();
610-
'load_legacy_mcp_json: {
611-
if global_mcp_config.is_none() {
612-
let Ok(global_mcp_path) = directories::chat_legacy_global_mcp_config(os) else {
613-
tracing::error!("Error obtaining legacy mcp json path. Skipping");
614-
break 'load_legacy_mcp_json;
615-
};
616-
let legacy_mcp_config = match McpServerConfig::load_from_file(os, global_mcp_path).await {
617-
Ok(config) => config,
618-
Err(e) => {
619-
tracing::error!("Error loading global mcp json path: {e}. Skipping");
651+
if mcp_enabled {
652+
'load_legacy_mcp_json: {
653+
if global_mcp_config.is_none() {
654+
let Ok(global_mcp_path) = directories::chat_legacy_global_mcp_config(os) else {
655+
tracing::error!("Error obtaining legacy mcp json path. Skipping");
620656
break 'load_legacy_mcp_json;
621-
},
622-
};
623-
global_mcp_config.replace(legacy_mcp_config);
657+
};
658+
let legacy_mcp_config = match McpServerConfig::load_from_file(os, global_mcp_path).await {
659+
Ok(config) => config,
660+
Err(e) => {
661+
tracing::error!("Error loading global mcp json path: {e}. Skipping");
662+
break 'load_legacy_mcp_json;
663+
},
664+
};
665+
global_mcp_config.replace(legacy_mcp_config);
666+
}
624667
}
625-
}
626668

627-
if let Some(config) = &global_mcp_config {
628-
agent.mcp_servers = config.clone();
669+
if let Some(config) = &global_mcp_config {
670+
agent.mcp_servers = config.clone();
671+
}
672+
} else {
673+
agent.mcp_servers = McpServerConfig::default();
629674
}
630-
631675
agent
632676
});
633677

@@ -763,6 +807,7 @@ async fn load_agents_from_entries(
763807
mut files: ReadDir,
764808
os: &Os,
765809
global_mcp_config: &mut Option<McpServerConfig>,
810+
mcp_enabled: bool,
766811
) -> Vec<Result<Agent, AgentConfigError>> {
767812
let mut res = Vec::<Result<Agent, AgentConfigError>>::new();
768813

@@ -773,7 +818,7 @@ async fn load_agents_from_entries(
773818
.and_then(OsStr::to_str)
774819
.is_some_and(|s| s == "json")
775820
{
776-
res.push(Agent::load(os, file_path, global_mcp_config).await);
821+
res.push(Agent::load(os, file_path, global_mcp_config, mcp_enabled).await);
777822
}
778823
}
779824

@@ -820,6 +865,13 @@ fn default_schema() -> String {
820865
"https://raw.githubusercontent.com/aws/amazon-q-developer-cli/refs/heads/main/schemas/agent-v1.json".into()
821866
}
822867

868+
// Check if a tool reference is MCP-specific (not @builtin and starts with @)
869+
pub fn is_mcp_tool_ref(s: &str) -> bool {
870+
// @builtin is not MCP, it's a reference to all built-in tools
871+
// Any other @ prefix is MCP (e.g., "@git", "@git/git_status")
872+
!s.starts_with("@builtin") && s.starts_with('@')
873+
}
874+
823875
#[cfg(test)]
824876
fn validate_agent_name(name: &str) -> eyre::Result<()> {
825877
// Check if name is empty
@@ -840,8 +892,9 @@ fn validate_agent_name(name: &str) -> eyre::Result<()> {
840892

841893
#[cfg(test)]
842894
mod tests {
843-
use super::*;
895+
use serde_json::json;
844896

897+
use super::*;
845898
const INPUT: &str = r#"
846899
{
847900
"name": "some_agent",
@@ -956,6 +1009,69 @@ mod tests {
9561009
assert!(validate_agent_name("invalid space").is_err());
9571010
}
9581011

1012+
#[test]
1013+
fn test_clear_mcp_configs_with_builtin_variants() {
1014+
let mut agent: Agent = serde_json::from_value(json!({
1015+
"name": "test",
1016+
"tools": [
1017+
"@builtin",
1018+
"@builtin/fs_read",
1019+
"@builtin/execute_bash",
1020+
"@git",
1021+
"@git/status",
1022+
"fs_write"
1023+
],
1024+
"allowedTools": [
1025+
"@builtin/fs_read",
1026+
"@git/status",
1027+
"fs_write"
1028+
],
1029+
"toolAliases": {
1030+
"@builtin/fs_read": "read",
1031+
"@git/status": "git_st"
1032+
},
1033+
"toolsSettings": {
1034+
"@builtin/fs_write": { "allowedPaths": ["~/**"] },
1035+
"@git/commit": { "sign": true }
1036+
}
1037+
}))
1038+
.unwrap();
1039+
1040+
agent.clear_mcp_configs();
1041+
1042+
// All @builtin variants should be preserved while MCP tools should be removed
1043+
assert!(agent.tools.contains(&"@builtin".to_string()));
1044+
assert!(agent.tools.contains(&"@builtin/fs_read".to_string()));
1045+
assert!(agent.tools.contains(&"@builtin/execute_bash".to_string()));
1046+
assert!(agent.tools.contains(&"fs_write".to_string()));
1047+
assert!(!agent.tools.contains(&"@git".to_string()));
1048+
assert!(!agent.tools.contains(&"@git/status".to_string()));
1049+
1050+
assert!(agent.allowed_tools.contains("@builtin/fs_read"));
1051+
assert!(agent.allowed_tools.contains("fs_write"));
1052+
assert!(!agent.allowed_tools.contains("@git/status"));
1053+
1054+
// Check tool aliases - need to iterate since we can't construct OriginalToolName directly
1055+
let has_builtin_alias = agent
1056+
.tool_aliases
1057+
.iter()
1058+
.any(|(k, v)| k.to_string() == "@builtin/fs_read" && v == "read");
1059+
assert!(has_builtin_alias, "@builtin/fs_read alias should be preserved");
1060+
1061+
let has_git_alias = agent.tool_aliases.iter().any(|(k, _)| k.to_string() == "@git/status");
1062+
assert!(!has_git_alias, "@git/status alias should be removed");
1063+
1064+
// Check tool settings - need to iterate since we can't construct ToolSettingTarget directly
1065+
let has_builtin_setting = agent
1066+
.tools_settings
1067+
.iter()
1068+
.any(|(k, _)| k.to_string() == "@builtin/fs_write");
1069+
assert!(has_builtin_setting, "@builtin/fs_write settings should be preserved");
1070+
1071+
let has_git_setting = agent.tools_settings.iter().any(|(k, _)| k.to_string() == "@git/commit");
1072+
assert!(!has_git_setting, "@git/commit settings should be removed");
1073+
}
1074+
9591075
#[test]
9601076
fn test_display_label_no_active_agent() {
9611077
let agents = Agents::default();

crates/chat-cli/src/cli/agent/root_command_args.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,16 @@ pub struct AgentArgs {
7474
impl AgentArgs {
7575
pub async fn execute(self, os: &mut Os) -> Result<ExitCode> {
7676
let mut stderr = std::io::stderr();
77+
let mcp_enabled = match os.client.is_mcp_enabled().await {
78+
Ok(enabled) => enabled,
79+
Err(err) => {
80+
tracing::warn!(?err, "Failed to check MCP configuration, defaulting to enabled");
81+
true
82+
},
83+
};
7784
match self.cmd {
7885
Some(AgentSubcommands::List) | None => {
79-
let agents = Agents::load(os, None, true, &mut stderr).await.0;
86+
let agents = Agents::load(os, None, true, &mut stderr, mcp_enabled).await.0;
8087
let agent_with_path =
8188
agents
8289
.agents
@@ -101,7 +108,7 @@ impl AgentArgs {
101108
writeln!(stderr, "{}", output_str)?;
102109
},
103110
Some(AgentSubcommands::Create { name, directory, from }) => {
104-
let mut agents = Agents::load(os, None, true, &mut stderr).await.0;
111+
let mut agents = Agents::load(os, None, true, &mut stderr, mcp_enabled).await.0;
105112
let path_with_file_name = create_agent(os, &mut agents, name.clone(), directory, from).await?;
106113
let editor_cmd = std::env::var("EDITOR").unwrap_or_else(|_| "vi".to_string());
107114
let mut cmd = std::process::Command::new(editor_cmd);
@@ -133,7 +140,7 @@ impl AgentArgs {
133140
},
134141
Some(AgentSubcommands::Validate { path }) => {
135142
let mut global_mcp_config = None::<McpServerConfig>;
136-
let agent = Agent::load(os, path.as_str(), &mut global_mcp_config).await;
143+
let agent = Agent::load(os, path.as_str(), &mut global_mcp_config, mcp_enabled).await;
137144

138145
'validate: {
139146
match agent {
@@ -251,7 +258,7 @@ impl AgentArgs {
251258
}
252259
},
253260
Some(AgentSubcommands::SetDefault { name }) => {
254-
let mut agents = Agents::load(os, None, true, &mut stderr).await.0;
261+
let mut agents = Agents::load(os, None, true, &mut stderr, mcp_enabled).await.0;
255262
match agents.switch(&name) {
256263
Ok(agent) => {
257264
os.database

0 commit comments

Comments
 (0)