Skip to content

Commit a68841a

Browse files
authored
fix: Remove profile scope from mcp commands (#1846)
1 parent d4e9350 commit a68841a

File tree

4 files changed

+39
-122
lines changed

4 files changed

+39
-122
lines changed

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

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,9 @@ pub struct McpAdd {
6060
/// The command used to launch the server
6161
#[arg(long)]
6262
pub command: String,
63-
/// Where to add the server to. For profile scope, the name of the profile must specified with
64-
/// --profile.
63+
/// Where to add the server to.
6564
#[arg(long, value_enum)]
6665
pub scope: Option<Scope>,
67-
/// Name of the profile to add the server config to. Not compatible with workspace scope or
68-
/// global scope.
69-
#[arg(long)]
70-
pub profile: Option<String>,
7166
/// Environment variables to use when launching the server
7267
#[arg(long, value_parser = parse_env_vars)]
7368
pub env: Vec<HashMap<String, String>>,
@@ -85,15 +80,13 @@ pub struct McpRemove {
8580
pub name: String,
8681
#[arg(long, value_enum)]
8782
pub scope: Option<Scope>,
88-
#[arg(long)]
89-
pub profile: Option<String>,
9083
}
9184

9285
#[derive(Debug, Clone, PartialEq, Eq, Args)]
9386
pub struct McpList {
9487
#[arg(value_enum)]
9588
pub scope: Option<Scope>,
96-
#[arg(long)]
89+
#[arg(long, hide = true)]
9790
pub profile: Option<String>,
9891
}
9992

@@ -103,8 +96,6 @@ pub struct McpImport {
10396
pub file: String,
10497
#[arg(value_enum)]
10598
pub scope: Option<Scope>,
106-
#[arg(long)]
107-
pub profile: Option<String>,
10899
/// Overwrite an existing server with the same name
109100
#[arg(long, default_value_t = false)]
110101
pub force: bool,
@@ -113,15 +104,13 @@ pub struct McpImport {
113104
#[derive(Debug, Copy, Clone, PartialEq, Eq, ValueEnum)]
114105
pub enum Scope {
115106
Workspace,
116-
Profile,
117107
Global,
118108
}
119109

120110
impl std::fmt::Display for Scope {
121111
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
122112
match self {
123113
Scope::Workspace => write!(f, "workspace"),
124-
Scope::Profile => write!(f, "profile"),
125114
Scope::Global => write!(f, "global"),
126115
}
127116
}

crates/chat-cli/src/cli/chat/mcp.rs

Lines changed: 34 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ use eyre::{
1111
Result,
1212
bail,
1313
};
14-
use tokio::fs;
1514
use tracing::warn;
1615

1716
use crate::cli::chat::cli::{
@@ -25,7 +24,6 @@ use crate::cli::chat::cli::{
2524
use crate::cli::chat::tool_manager::{
2625
McpServerConfig,
2726
global_mcp_config_path,
28-
profile_mcp_config_path,
2927
workspace_mcp_config_path,
3028
};
3129
use crate::cli::chat::tools::custom_tool::{
@@ -34,7 +32,6 @@ use crate::cli::chat::tools::custom_tool::{
3432
};
3533
use crate::cli::chat::util::shared_writer::SharedWriter;
3634
use crate::platform::Context;
37-
use crate::util::directories::chat_profiles_dir;
3835

3936
pub async fn execute_mcp(args: Mcp) -> Result<ExitCode> {
4037
let ctx = Context::new();
@@ -54,9 +51,9 @@ pub async fn execute_mcp(args: Mcp) -> Result<ExitCode> {
5451

5552
pub async fn add_mcp_server(ctx: &Context, output: &mut SharedWriter, args: McpAdd) -> Result<()> {
5653
let scope = args.scope.unwrap_or(Scope::Workspace);
57-
let config_path = resolve_scope_profile(ctx, args.scope, args.profile.as_ref())?;
54+
let config_path = resolve_scope_profile(ctx, args.scope)?;
5855

59-
let mut config: McpServerConfig = ensure_config_file(ctx, &config_path, scope, output).await?;
56+
let mut config: McpServerConfig = ensure_config_file(ctx, &config_path, output).await?;
6057

6158
if config.mcp_servers.contains_key(&args.name) && !args.force {
6259
bail!(
@@ -85,14 +82,14 @@ pub async fn add_mcp_server(ctx: &Context, output: &mut SharedWriter, args: McpA
8582
output,
8683
"✓ Added MCP server '{}' to {}\n",
8784
args.name,
88-
scope_display(&scope, &args.profile)
85+
scope_display(&scope)
8986
)?;
9087
Ok(())
9188
}
9289

9390
pub async fn remove_mcp_server(ctx: &Context, output: &mut SharedWriter, args: McpRemove) -> Result<()> {
9491
let scope = args.scope.unwrap_or(Scope::Workspace);
95-
let config_path = resolve_scope_profile(ctx, args.scope, args.profile.as_ref())?;
92+
let config_path = resolve_scope_profile(ctx, args.scope)?;
9693

9794
if !ctx.fs().exists(&config_path) {
9895
writeln!(output, "\nNo MCP server configurations found.\n")?;
@@ -107,31 +104,31 @@ pub async fn remove_mcp_server(ctx: &Context, output: &mut SharedWriter, args: M
107104
output,
108105
"\n✓ Removed MCP server '{}' from {}\n",
109106
args.name,
110-
scope_display(&scope, &args.profile)
107+
scope_display(&scope)
111108
)?;
112109
},
113110
None => {
114111
writeln!(
115112
output,
116113
"\nNo MCP server named '{}' found in {}\n",
117114
args.name,
118-
scope_display(&scope, &args.profile)
115+
scope_display(&scope)
119116
)?;
120117
},
121118
}
122119
Ok(())
123120
}
124121

125122
pub async fn list_mcp_server(ctx: &Context, output: &mut SharedWriter, args: McpList) -> Result<()> {
126-
let configs = get_mcp_server_configs(ctx, args.scope, args.profile).await?;
123+
let configs = get_mcp_server_configs(ctx, args.scope).await?;
127124
if configs.is_empty() {
128125
writeln!(output, "No MCP server configurations found.\n")?;
129126
return Ok(());
130127
}
131128

132-
for (scope, profile, path, cfg_opt) in configs {
129+
for (scope, path, cfg_opt) in configs {
133130
writeln!(output)?;
134-
writeln!(output, "{}:\n {}", scope_display(&scope, &profile), path.display())?;
131+
writeln!(output, "{}:\n {}", scope_display(&scope), path.display())?;
135132
match cfg_opt {
136133
Some(cfg) if !cfg.mcp_servers.is_empty() => {
137134
for (name, tool_cfg) in &cfg.mcp_servers {
@@ -149,8 +146,8 @@ pub async fn list_mcp_server(ctx: &Context, output: &mut SharedWriter, args: Mcp
149146

150147
pub async fn import_mcp_server(ctx: &Context, output: &mut SharedWriter, args: McpImport) -> Result<()> {
151148
let scope: Scope = args.scope.unwrap_or(Scope::Workspace);
152-
let config_path = resolve_scope_profile(ctx, args.scope, args.profile.as_ref())?;
153-
let mut dst_cfg = ensure_config_file(ctx, &config_path, scope, output).await?;
149+
let config_path = resolve_scope_profile(ctx, args.scope)?;
150+
let mut dst_cfg = ensure_config_file(ctx, &config_path, output).await?;
154151

155152
let src_path = expand_path(ctx, &args.file)?;
156153
let src_cfg: McpServerConfig = McpServerConfig::load_from_file(ctx, &src_path).await?;
@@ -178,22 +175,22 @@ pub async fn import_mcp_server(ctx: &Context, output: &mut SharedWriter, args: M
178175
writeln!(
179176
output,
180177
"✓ Imported {added} MCP server(s) into {}\n",
181-
scope_display(&scope, &args.profile)
178+
scope_display(&scope)
182179
)?;
183180
Ok(())
184181
}
185182

186183
pub async fn get_mcp_server_status(ctx: &Context, output: &mut SharedWriter, name: String) -> Result<()> {
187-
let configs = get_mcp_server_configs(ctx, None, None).await?;
184+
let configs = get_mcp_server_configs(ctx, None).await?;
188185
let mut found = false;
189186

190-
for (sc, prof, path, cfg_opt) in configs {
187+
for (sc, path, cfg_opt) in configs {
191188
if let Some(cfg) = cfg_opt.and_then(|c| c.mcp_servers.get(&name).cloned()) {
192189
found = true;
193190
execute!(
194191
output,
195192
style::Print("\n─────────────\n"),
196-
style::Print(format!("Scope : {}\n", scope_display(&sc, &prof))),
193+
style::Print(format!("Scope : {}\n", scope_display(&sc))),
197194
style::Print(format!("File : {}\n", path.display())),
198195
style::Print(format!("Command : {}\n", cfg.command)),
199196
style::Print(format!("Timeout : {} ms\n", cfg.timeout)),
@@ -217,43 +214,16 @@ pub async fn get_mcp_server_status(ctx: &Context, output: &mut SharedWriter, nam
217214
async fn get_mcp_server_configs(
218215
ctx: &Context,
219216
scope: Option<Scope>,
220-
profile: Option<String>,
221-
) -> Result<Vec<(Scope, Option<String>, PathBuf, Option<McpServerConfig>)>> {
222-
let all_profiles = if matches!((scope, &profile), (None, None)) {
223-
let mut v = Vec::new();
224-
if let Ok(dir) = chat_profiles_dir(ctx) {
225-
let mut rd = fs::read_dir(dir).await?;
226-
while let Some(e) = rd.next_entry().await? {
227-
if e.file_type().await?.is_dir() {
228-
if let Some(n) = e.file_name().to_str() {
229-
v.push(n.to_owned());
230-
}
231-
}
232-
}
233-
}
234-
v
235-
} else {
236-
Vec::new()
237-
};
238-
217+
) -> Result<Vec<(Scope, PathBuf, Option<McpServerConfig>)>> {
239218
let mut targets = Vec::new();
240-
match (scope, profile) {
241-
(Some(Scope::Workspace), _) => targets.push((Scope::Workspace, None)),
242-
(Some(Scope::Global), _) => targets.push((Scope::Global, None)),
243-
(Some(Scope::Profile), Some(p)) => targets.push((Scope::Profile, Some(p))),
244-
(None, Some(p)) => targets.push((Scope::Profile, Some(p))),
245-
(None, None) => {
246-
targets.extend([(Scope::Workspace, None), (Scope::Global, None)]);
247-
for p in all_profiles {
248-
targets.push((Scope::Profile, Some(p)));
249-
}
250-
},
251-
(Some(Scope::Profile), None) => bail!("profile must be specified for scope: profile"),
219+
match scope {
220+
Some(scope) => targets.push(scope),
221+
None => targets.extend([Scope::Workspace, Scope::Global]),
252222
}
253223

254224
let mut results = Vec::new();
255-
for (sc, prof) in targets {
256-
let path = resolve_scope_profile(ctx, Some(sc), prof.as_ref())?;
225+
for sc in targets {
226+
let path = resolve_scope_profile(ctx, Some(sc))?;
257227
let cfg_opt = if ctx.fs().exists(&path) {
258228
match McpServerConfig::load_from_file(ctx, &path).await {
259229
Ok(cfg) => Some(cfg),
@@ -265,25 +235,22 @@ async fn get_mcp_server_configs(
265235
} else {
266236
None
267237
};
268-
results.push((sc, prof.clone(), path, cfg_opt));
238+
results.push((sc, path, cfg_opt));
269239
}
270240
Ok(results)
271241
}
272242

273-
fn scope_display(scope: &Scope, profile: &Option<String>) -> String {
243+
fn scope_display(scope: &Scope) -> String {
274244
match scope {
275245
Scope::Workspace => "📄 workspace".into(),
276246
Scope::Global => "🌍 global".into(),
277-
Scope::Profile => format!("👤 profile({})", profile.as_deref().unwrap_or("default")),
278247
}
279248
}
280249

281-
fn resolve_scope_profile(ctx: &Context, scope: Option<Scope>, profile: Option<&impl AsRef<str>>) -> Result<PathBuf> {
282-
Ok(match (scope, profile) {
283-
(None | Some(Scope::Workspace), _) => workspace_mcp_config_path(ctx)?,
284-
(Some(Scope::Global), _) => global_mcp_config_path(ctx)?,
285-
(Some(scope @ Scope::Profile), None) => bail!("profile must be specified for scope: {scope}"),
286-
(_, Some(profile)) => profile_mcp_config_path(ctx, profile)?,
250+
fn resolve_scope_profile(ctx: &Context, scope: Option<Scope>) -> Result<PathBuf> {
251+
Ok(match scope {
252+
Some(Scope::Global) => global_mcp_config_path(ctx)?,
253+
_ => workspace_mcp_config_path(ctx)?,
287254
})
288255
}
289256

@@ -296,19 +263,15 @@ fn expand_path(ctx: &Context, p: &str) -> Result<PathBuf> {
296263
Ok(path)
297264
}
298265

299-
async fn ensure_config_file(
300-
ctx: &Context,
301-
path: &PathBuf,
302-
scope: Scope,
303-
out: &mut SharedWriter,
304-
) -> Result<McpServerConfig> {
305-
if !ctx.fs().exists(path) && scope != Scope::Profile {
266+
async fn ensure_config_file(ctx: &Context, path: &PathBuf, out: &mut SharedWriter) -> Result<McpServerConfig> {
267+
if !ctx.fs().exists(path) {
306268
if let Some(parent) = path.parent() {
307269
ctx.fs().create_dir_all(parent).await?;
308270
}
309271
McpServerConfig::default().save_to_file(ctx, path).await?;
310272
writeln!(out, "\n📁 Created MCP config in '{}'", path.display())?;
311273
}
274+
312275
load_cfg(ctx, path).await
313276
}
314277

@@ -327,34 +290,24 @@ mod tests {
327290
#[test]
328291
fn test_scope_and_profile_defaults_to_workspace() {
329292
let ctx = Context::new();
330-
let path = resolve_scope_profile(&ctx, None, None::<&String>).unwrap();
293+
let path = resolve_scope_profile(&ctx, None).unwrap();
331294
assert_eq!(
332295
path.to_str(),
333296
workspace_mcp_config_path(&ctx).unwrap().to_str(),
334297
"No scope or profile should default to the workspace path"
335298
);
336299
}
337300

338-
#[test]
339-
fn test_resolve_profile_without_name_err() {
340-
let ctx = Context::new();
341-
assert!(resolve_scope_profile(&ctx, Some(Scope::Profile), None::<&String>).is_err());
342-
}
343-
344301
#[test]
345302
fn test_resolve_paths() {
346303
let ctx = Context::new();
347304
// workspace
348-
let p = resolve_scope_profile(&ctx, Some(Scope::Workspace), None::<&String>).unwrap();
305+
let p = resolve_scope_profile(&ctx, Some(Scope::Workspace)).unwrap();
349306
assert_eq!(p, workspace_mcp_config_path(&ctx).unwrap());
350307

351308
// global
352-
let p = resolve_scope_profile(&ctx, Some(Scope::Global), None::<&String>).unwrap();
309+
let p = resolve_scope_profile(&ctx, Some(Scope::Global)).unwrap();
353310
assert_eq!(p, global_mcp_config_path(&ctx).unwrap());
354-
355-
// profile
356-
let p = resolve_scope_profile(&ctx, Some(Scope::Profile), Some(&"qa")).unwrap();
357-
assert_eq!(p, profile_mcp_config_path(&ctx, "qa").unwrap(), "profile path mismatch");
358311
}
359312

360313
#[ignore = "TODO: fix in CI"]
@@ -364,9 +317,7 @@ mod tests {
364317
let mut out = SharedWriter::null();
365318
let path = workspace_mcp_config_path(&ctx).unwrap();
366319

367-
let cfg = super::ensure_config_file(&ctx, &path, Scope::Workspace, &mut out)
368-
.await
369-
.unwrap();
320+
let cfg = super::ensure_config_file(&ctx, &path, &mut out).await.unwrap();
370321
assert!(path.exists(), "config file should be created");
371322
assert!(cfg.mcp_servers.is_empty());
372323
}
@@ -388,7 +339,6 @@ mod tests {
388339
env: vec![],
389340
timeout: None,
390341
scope: None,
391-
profile: None,
392342
force: false,
393343
};
394344
add_mcp_server(&ctx, &mut out, add_args).await.unwrap();
@@ -401,7 +351,6 @@ mod tests {
401351
let rm_args = McpRemove {
402352
name: "local".into(),
403353
scope: None,
404-
profile: None,
405354
};
406355
remove_mcp_server(&ctx, &mut out, rm_args).await.unwrap();
407356

crates/chat-cli/src/cli/chat/tool_manager.rs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,7 @@ use crate::mcp_client::{
8787
};
8888
use crate::platform::Context;
8989
use crate::telemetry::TelemetryThread;
90-
use crate::util::directories::{
91-
chat_profiles_dir,
92-
home_dir,
93-
};
90+
use crate::util::directories::home_dir;
9491

9592
const NAMESPACE_DELIMITER: &str = "___";
9693
// This applies for both mcp server and tool name since in the end the tool name as seen by the
@@ -102,10 +99,6 @@ pub fn workspace_mcp_config_path(ctx: &Context) -> eyre::Result<PathBuf> {
10299
Ok(ctx.env().current_dir()?.join(".amazonq").join("mcp.json"))
103100
}
104101

105-
pub fn profile_mcp_config_path(ctx: &Context, profile_name: impl AsRef<str>) -> eyre::Result<PathBuf> {
106-
Ok(chat_profiles_dir(ctx)?.join(profile_name.as_ref()).join("mcp.json"))
107-
}
108-
109102
pub fn global_mcp_config_path(ctx: &Context) -> eyre::Result<PathBuf> {
110103
Ok(home_dir(ctx)?.join(".aws").join("amazonq").join("mcp.json"))
111104
}

0 commit comments

Comments
 (0)