Skip to content

Commit 23331b8

Browse files
authored
fix: includes workspace mcp in legacy mcp (#2516)
* legacy mcp json also sources from workspace * updates doc
1 parent a14f278 commit 23331b8

File tree

5 files changed

+88
-38
lines changed

5 files changed

+88
-38
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ pub async fn migrate(os: &mut Os, force: bool) -> eyre::Result<Option<Vec<Agent>
7777
};
7878

7979
let mcp_servers = {
80-
let config_path = directories::chat_legacy_mcp_config(os)?;
80+
let config_path = directories::chat_legacy_global_mcp_config(os)?;
8181
if os.fs.exists(&config_path) {
8282
match McpServerConfig::load_from_file(os, config_path).await {
8383
Ok(mut config) => {

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

Lines changed: 53 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -208,14 +208,14 @@ impl Agent {
208208
/// This function mutates the agent to a state that is usable for runtime.
209209
/// Practically this means to convert some of the fields value to their usable counterpart.
210210
/// For example, converting the mcp array to actual mcp config and populate the agent file path.
211-
fn thaw(&mut self, path: &Path, global_mcp_config: Option<&McpServerConfig>) -> Result<(), AgentConfigError> {
211+
fn thaw(&mut self, path: &Path, legacy_mcp_config: Option<&McpServerConfig>) -> Result<(), AgentConfigError> {
212212
let Self { mcp_servers, .. } = self;
213213

214214
self.path = Some(path.to_path_buf());
215215

216-
if let (true, Some(global_mcp_config)) = (self.use_legacy_mcp_json, global_mcp_config) {
216+
if let (true, Some(legacy_mcp_config)) = (self.use_legacy_mcp_json, legacy_mcp_config) {
217217
let mut stderr = std::io::stderr();
218-
for (name, legacy_server) in &global_mcp_config.mcp_servers {
218+
for (name, legacy_server) in &legacy_mcp_config.mcp_servers {
219219
if mcp_servers.mcp_servers.contains_key(name) {
220220
let _ = queue!(
221221
stderr,
@@ -269,16 +269,13 @@ impl Agent {
269269
Ok(config_path) => {
270270
let content = os.fs.read(&config_path).await?;
271271
let mut agent = serde_json::from_slice::<Agent>(&content)?;
272-
273-
let global_mcp_path = directories::chat_legacy_mcp_config(os)?;
274-
let global_mcp_config = match McpServerConfig::load_from_file(os, global_mcp_path).await {
275-
Ok(config) => Some(config),
276-
Err(e) => {
277-
tracing::error!("Error loading global mcp json path: {e}.");
278-
None
279-
},
272+
let legacy_mcp_config = if agent.use_legacy_mcp_json {
273+
load_legacy_mcp_config(os).await.unwrap_or(None)
274+
} else {
275+
None
280276
};
281-
agent.thaw(&config_path, global_mcp_config.as_ref())?;
277+
278+
agent.thaw(&config_path, legacy_mcp_config.as_ref())?;
282279
Ok((agent, config_path))
283280
},
284281
_ => bail!("Agent {agent_name} does not exist"),
@@ -288,27 +285,22 @@ impl Agent {
288285
pub async fn load(
289286
os: &Os,
290287
agent_path: impl AsRef<Path>,
291-
global_mcp_config: &mut Option<McpServerConfig>,
288+
legacy_mcp_config: &mut Option<McpServerConfig>,
292289
) -> Result<Agent, AgentConfigError> {
293290
let content = os.fs.read(&agent_path).await?;
294291
let mut agent = serde_json::from_slice::<Agent>(&content).map_err(|e| AgentConfigError::InvalidJson {
295292
error: e,
296293
path: agent_path.as_ref().to_path_buf(),
297294
})?;
298295

299-
if agent.use_legacy_mcp_json && global_mcp_config.is_none() {
300-
let global_mcp_path = directories::chat_legacy_mcp_config(os)?;
301-
let legacy_mcp_config = if global_mcp_path.exists() {
302-
McpServerConfig::load_from_file(os, global_mcp_path)
303-
.await
304-
.map_err(AgentConfigError::BadLegacyMcpConfig)?
305-
} else {
306-
McpServerConfig::default()
307-
};
308-
global_mcp_config.replace(legacy_mcp_config);
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);
300+
}
309301
}
310302

311-
agent.thaw(agent_path.as_ref(), global_mcp_config.as_ref())?;
303+
agent.thaw(agent_path.as_ref(), legacy_mcp_config.as_ref())?;
312304
Ok(agent)
313305
}
314306
}
@@ -611,7 +603,7 @@ impl Agents {
611603
let mut agent = Agent::default();
612604
'load_legacy_mcp_json: {
613605
if global_mcp_config.is_none() {
614-
let Ok(global_mcp_path) = directories::chat_legacy_mcp_config(os) else {
606+
let Ok(global_mcp_path) = directories::chat_legacy_global_mcp_config(os) else {
615607
tracing::error!("Error obtaining legacy mcp json path. Skipping");
616608
break 'load_legacy_mcp_json;
617609
};
@@ -769,6 +761,42 @@ async fn load_agents_from_entries(
769761
res
770762
}
771763

764+
/// Loads legacy mcp config by combining workspace and global config.
765+
/// In case of a server naming conflict, the workspace config is prioritized.
766+
async fn load_legacy_mcp_config(os: &Os) -> eyre::Result<Option<McpServerConfig>> {
767+
let global_mcp_path = directories::chat_legacy_global_mcp_config(os)?;
768+
let global_mcp_config = match McpServerConfig::load_from_file(os, global_mcp_path).await {
769+
Ok(config) => Some(config),
770+
Err(e) => {
771+
tracing::error!("Error loading global mcp json path: {e}.");
772+
None
773+
},
774+
};
775+
776+
let workspace_mcp_path = directories::chat_legacy_workspace_mcp_config(os)?;
777+
let workspace_mcp_config = match McpServerConfig::load_from_file(os, workspace_mcp_path).await {
778+
Ok(config) => Some(config),
779+
Err(e) => {
780+
tracing::error!("Error loading global mcp json path: {e}.");
781+
None
782+
},
783+
};
784+
785+
Ok(match (workspace_mcp_config, global_mcp_config) {
786+
(Some(mut wc), Some(gc)) => {
787+
for (server_name, config) in gc.mcp_servers {
788+
// We prioritize what is in the workspace
789+
wc.mcp_servers.entry(server_name).or_insert(config);
790+
}
791+
792+
Some(wc)
793+
},
794+
(None, Some(gc)) => Some(gc),
795+
(Some(wc), None) => Some(wc),
796+
_ => None,
797+
})
798+
}
799+
772800
fn default_schema() -> String {
773801
"https://raw.githubusercontent.com/aws/amazon-q-developer-cli/refs/heads/main/schemas/agent-v1.json".into()
774802
}

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

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,9 @@ pub struct AddArgs {
9090
/// Name for the server
9191
#[arg(long)]
9292
pub name: String,
93+
/// Scope. This parameter is only meaningful in the absence of agent name.
94+
#[arg(long)]
95+
pub scope: Option<Scope>,
9396
/// The command used to launch the server
9497
#[arg(long)]
9598
pub command: String,
@@ -145,14 +148,17 @@ impl AddArgs {
145148
writeln!(output, "✓ Added MCP server '{}' to agent {}\n", self.name, agent_name)?;
146149
},
147150
None => {
148-
let global_config_path = directories::chat_legacy_mcp_config(os)?;
149-
let mut mcp_servers = McpServerConfig::load_from_file(os, &global_config_path).await?;
151+
let legacy_mcp_config_path = match self.scope {
152+
Some(Scope::Workspace) => directories::chat_legacy_workspace_mcp_config(os)?,
153+
_ => directories::chat_legacy_global_mcp_config(os)?,
154+
};
155+
let mut mcp_servers = McpServerConfig::load_from_file(os, &legacy_mcp_config_path).await?;
150156

151157
if mcp_servers.mcp_servers.contains_key(&self.name) && !self.force {
152158
bail!(
153159
"\nMCP server '{}' already exists in global config (path {}). Use --force to overwrite.",
154160
self.name,
155-
&global_config_path.display(),
161+
&legacy_mcp_config_path.display(),
156162
);
157163
}
158164

@@ -166,12 +172,12 @@ impl AddArgs {
166172
}))?;
167173

168174
mcp_servers.mcp_servers.insert(self.name.clone(), tool);
169-
mcp_servers.save_to_file(os, &global_config_path).await?;
175+
mcp_servers.save_to_file(os, &legacy_mcp_config_path).await?;
170176
writeln!(
171177
output,
172178
"✓ Added MCP server '{}' to global config in {}\n",
173179
self.name,
174-
global_config_path.display()
180+
legacy_mcp_config_path.display()
175181
)?;
176182
},
177183
};
@@ -184,6 +190,9 @@ impl AddArgs {
184190
pub struct RemoveArgs {
185191
#[arg(long)]
186192
pub name: String,
193+
/// Scope. This parameter is only meaningful in the absence of agent name.
194+
#[arg(long)]
195+
pub scope: Option<Scope>,
187196
#[arg(long, value_enum)]
188197
pub agent: Option<String>,
189198
}
@@ -221,25 +230,28 @@ impl RemoveArgs {
221230
}
222231
},
223232
None => {
224-
let global_config_path = directories::chat_legacy_mcp_config(os)?;
225-
let mut config = McpServerConfig::load_from_file(os, &global_config_path).await?;
233+
let legacy_mcp_config_path = match self.scope {
234+
Some(Scope::Workspace) => directories::chat_legacy_workspace_mcp_config(os)?,
235+
_ => directories::chat_legacy_global_mcp_config(os)?,
236+
};
237+
let mut config = McpServerConfig::load_from_file(os, &legacy_mcp_config_path).await?;
226238

227239
match config.mcp_servers.remove(&self.name) {
228240
Some(_) => {
229-
config.save_to_file(os, &global_config_path).await?;
241+
config.save_to_file(os, &legacy_mcp_config_path).await?;
230242
writeln!(
231243
output,
232244
"\n✓ Removed MCP server '{}' from global config (path {})\n",
233245
self.name,
234-
&global_config_path.display(),
246+
&legacy_mcp_config_path.display(),
235247
)?;
236248
},
237249
None => {
238250
writeln!(
239251
output,
240252
"\nNo MCP server named '{}' found in global config (path {})\n",
241253
self.name,
242-
&global_config_path.display(),
254+
&legacy_mcp_config_path.display(),
243255
)?;
244256
},
245257
}
@@ -540,6 +552,7 @@ mod tests {
540552
// 1. add
541553
AddArgs {
542554
name: "local".into(),
555+
scope: None,
543556
command: "echo hi".into(),
544557
args: vec![
545558
"awslabs.eks-mcp-server".to_string(),
@@ -564,6 +577,7 @@ mod tests {
564577
// 2. remove
565578
RemoveArgs {
566579
name: "local".into(),
580+
scope: None,
567581
agent: None,
568582
}
569583
.execute(&os, &mut vec![])
@@ -591,6 +605,7 @@ mod tests {
591605
],
592606
RootSubcommand::Mcp(McpSubcommand::Add(AddArgs {
593607
name: "test_server".to_string(),
608+
scope: None,
594609
command: "test_command".to_string(),
595610
args: vec![
596611
"awslabs.eks-mcp-server".to_string(),
@@ -619,6 +634,7 @@ mod tests {
619634
["mcp", "remove", "--name", "old"],
620635
RootSubcommand::Mcp(McpSubcommand::Remove(RemoveArgs {
621636
name: "old".into(),
637+
scope: None,
622638
agent: None,
623639
}))
624640
);

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,16 @@ pub fn example_agent_config(os: &Os) -> Result<PathBuf> {
144144
}
145145

146146
/// Legacy global MCP server config path
147-
pub fn chat_legacy_mcp_config(os: &Os) -> Result<PathBuf> {
147+
pub fn chat_legacy_global_mcp_config(os: &Os) -> Result<PathBuf> {
148148
Ok(home_dir(os)?.join(".aws").join("amazonq").join("mcp.json"))
149149
}
150150

151+
/// Legacy workspace MCP server config path
152+
pub fn chat_legacy_workspace_mcp_config(os: &Os) -> Result<PathBuf> {
153+
let cwd = os.env.current_dir()?;
154+
Ok(cwd.join(".amazonq").join("mcp.json"))
155+
}
156+
151157
/// The directory to the directory containing global agents
152158
pub fn chat_global_agent_path(os: &Os) -> Result<PathBuf> {
153159
Ok(home_dir(os)?.join(GLOBAL_AGENT_DIR_RELATIVE_TO_HOME))

docs/agent-format.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ Available hook triggers:
225225

226226
## UseLegacyMcpJson Field
227227

228-
The `useLegacyMcpJson` field determines whether to include MCP servers defined in the legacy global MCP configuration file (`~/.aws/amazonq/mcp.json`).
228+
The `useLegacyMcpJson` field determines whether to include MCP servers defined in the legacy MCP configuration files (`~/.aws/amazonq/mcp.json` for global and `cwd/.amazonq/mcp.json` for workspace).
229229

230230
```json
231231
{

0 commit comments

Comments
 (0)