Introduce id into the bundle metadata and other minor fixes

adwsingh · adwsingh · commit 3fe46adbb2ad · 2025-06-30T16:20:37.000-07:00
diff --git a/aws/aws-mcp-cli-commands/src/main/java/software/amazon/smithy/java/aws/mcp/cli/commands/CreateAwsServiceBundle.java b/aws/aws-mcp-cli-commands/src/main/java/software/amazon/smithy/java/aws/mcp/cli/commands/CreateAwsServiceBundle.java
@@ -55,6 +55,7 @@ protected Bundle getNewBundle(CreateAwsServiceBundleInput input) {
                 .smithyBundle(SmithyMcpBundle.builder()
                         .bundle(bundleBuilder.build().bundle())
                         .metadata(BundleMetadata.builder()
+                                .id(input.id)
                                 .name(input.name)
                                 .description(input.description)
                                 .build())
diff --git a/mcp/mcp-bundle-api/src/main/resources/META-INF/smithy/mcpbundle.smithy b/mcp/mcp-bundle-api/src/main/resources/META-INF/smithy/mcpbundle.smithy
@@ -19,6 +19,9 @@ structure CommonBundleConfig {
 }
 
 structure BundleMetadata {
+    // Do not mark required for backward compatibility.
+    id: String
+
     @required
     name: String
 
diff --git a/mcp/mcp-cli-api/model/main.smithy b/mcp/mcp-cli-api/model/main.smithy
@@ -2,6 +2,8 @@ $version: "2"
 
 namespace smithy.mcp.cli
 
+use software.amazon.smithy.mcp.bundle.api#BundleMetadata
+
 structure Config {
     toolBundles: McpBundleConfigs
 
@@ -36,8 +38,8 @@ structure CommonToolConfig {
 
     @default(false)
     local: PrimitiveBoolean
-
-    description: String
+    
+    metadata: BundleMetadata
 }
 
 map Registries {
diff --git a/mcp/mcp-cli-api/src/main/java/software/amazon/smithy/java/mcp/cli/AbstractCreateBundle.java b/mcp/mcp-cli-api/src/main/java/software/amazon/smithy/java/mcp/cli/AbstractCreateBundle.java
@@ -18,11 +18,15 @@ public abstract static class CreateBundleInput {
                 defaultValue = "false")
         boolean overwrite;
 
-        @Option(names = {"-n", "--name"}, description = "Name to assign to the MCP server. Eg: (aws-dynamodb-mcp)",
+        @Option(names = {"--id"},
+                description = "Id to assign to the MCP server. This id needs to end in mcp. Eg: (aws-dynamodb-mcp)",
                 required = true)
+        public String id;
+
+        @Option(names = {"--name"}, description = "Name for this MCP server. Eg: (Pipelines MCP Server)")
         public String name;
 
-        @Option(names = {"-d", "--description"}, description = "Description of this mcp server", required = true)
+        @Option(names = {"-d", "--description"}, description = "Description of this mcp server.", required = true)
         public String description;
 
         @ArgGroup
@@ -42,15 +46,22 @@ public static class JsonInput {
     protected void execute(ExecutionContext context) throws Exception {
         var input = getInput();
         var config = context.config();
-        if (!input.overwrite && config.getToolBundles().containsKey(input.name)) {
-            throw new IllegalArgumentException("Tool bundle " + input.name
+        if (!input.overwrite && config.getToolBundles().containsKey(input.id)) {
+            throw new IllegalArgumentException("Tool bundle " + input.id
                     + " already exists. Either choose a new name or pass --overwrite to overwrite the existing tool bundle");
         }
 
+        if (input.name == null) {
+            input.name = input.id;
+        }
+        if (input.description == null) {
+            input.description = "[" + input.id + "]" + " MCP Server";
+        }
+
         var bundle = getNewBundle(input);
-        ConfigUtils.addMcpBundle(config, input.name, bundle, true);
-        ConfigUtils.createWrapperAndUpdateClientConfigs(input.name, bundle, config, input.clientsInput);
-        System.out.println("Successfully created bundle " + input.name);
+        ConfigUtils.addMcpBundle(config, input.id, bundle, true);
+        ConfigUtils.createWrapperAndUpdateClientConfigs(input.id, bundle, config, input.clientsInput);
+        System.out.println("Successfully created bundle " + input.id);
     }
 
     protected abstract T getInput();
diff --git a/mcp/mcp-cli-api/src/main/java/software/amazon/smithy/java/mcp/cli/ClientsInput.java b/mcp/mcp-cli-api/src/main/java/software/amazon/smithy/java/mcp/cli/ClientsInput.java
@@ -15,5 +15,5 @@ public final class ClientsInput {
 
     @Option(names = "--print-client-config",
             description = "If specified will not edit the client configs and only print to console.")
-    public Boolean print;
+    public boolean print = false;
 }
diff --git a/mcp/mcp-cli-api/src/main/java/software/amazon/smithy/java/mcp/cli/ConfigUtils.java b/mcp/mcp-cli-api/src/main/java/software/amazon/smithy/java/mcp/cli/ConfigUtils.java
@@ -144,8 +144,8 @@ public static Config loadOrCreateConfig() throws IOException {
         return config;
     }
 
-    public static Path getBundleFileLocation(String bundleName) {
-        return BUNDLE_DIR.resolve(bundleName + ".json");
+    public static Path getBundleFileLocation(String id) {
+        return BUNDLE_DIR.resolve(id + ".json");
     }
 
     /**
@@ -188,47 +188,47 @@ private static void addMcpBundleConfig(Config existingConfig, String name, McpBu
         updateConfig(newConfig);
     }
 
-    public static Bundle getMcpBundle(String bundleName) {
+    public static Bundle getMcpBundle(String id) {
         try {
             return Bundle.builder()
-                    .deserialize(JSON_CODEC.createDeserializer(Files.readAllBytes(getBundleFileLocation(bundleName))))
+                    .deserialize(JSON_CODEC.createDeserializer(Files.readAllBytes(getBundleFileLocation(id))))
                     .build();
         } catch (IOException e) {
             throw new RuntimeException(e);
         }
     }
 
-    public static void removeMcpBundle(Config currentConfig, String bundleName) throws IOException {
+    public static void removeMcpBundle(Config currentConfig, String id) throws IOException {
         var builder = currentConfig.toBuilder();
         var newBundles = new LinkedHashMap<>(currentConfig.getToolBundles());
-        newBundles.remove(bundleName);
+        newBundles.remove(id);
         builder.toolBundles(newBundles);
         var newConfig = builder.build();
         updateConfig(newConfig);
-        var bundleFile = getBundleFileLocation(bundleName);
+        var bundleFile = getBundleFileLocation(id);
         Files.deleteIfExists(bundleFile);
         // Remove wrapper script if it exists
-        removeWrapperScript(bundleName);
+        removeWrapperScript(id);
     }
 
-    public static void addToClientConfigs(Config config, String name, Set<String> clients, McpServerConfig serverConfig)
+    public static void addToClientConfigs(Config config, String id, Set<String> clients, McpServerConfig serverConfig)
             throws IOException {
-        updateClientConfigs(config, name, clients, serverConfig);
+        updateClientConfigs(config, id, clients, serverConfig);
     }
 
-    public static void removeFromClientConfigs(Config config, String name, Set<String> clients) throws IOException {
-        updateClientConfigs(config, name, clients, null);
+    public static void removeFromClientConfigs(Config config, String id, Set<String> clients) throws IOException {
+        updateClientConfigs(config, id, clients, null);
     }
 
-    private static void updateClientConfigs(Config config, String name, Set<String> clients, McpServerConfig newConfig)
+    private static void updateClientConfigs(Config config, String id, Set<String> clients, McpServerConfig newConfig)
             throws IOException {
         var clientConfigsToUpdate = getClientConfigsToUpdate(config, clients);
         boolean isDelete = newConfig == null;
         for (var clientConfigs : clientConfigsToUpdate) {
             var filePath = Path.of(clientConfigs.getFilePath());
             if (Files.notExists(filePath)) {
                 System.out.printf("Skipping updating Mcp config file for %s as the file path '%s' does not exist.",
-                        name,
+                        id,
                         filePath);
                 continue;
             }
@@ -243,11 +243,11 @@ private static void updateClientConfigs(Config config, String name, Set<String>
 
             var map = new LinkedHashMap<>(currentMcpConfig.getMcpServers());
             if (isDelete) {
-                if (map.remove(name) == null) {
+                if (map.remove(id) == null) {
                     continue;
                 }
             } else {
-                map.put(name, Document.of(newConfig));
+                map.put(id, Document.of(newConfig));
             }
             var newMcpConfig = McpServersClientConfig.builder().mcpServers(map).build();
             Files.write(filePath,
@@ -283,10 +283,10 @@ private static Set<ClientConfig> getClientConfigsToUpdate(Config config, Set<Str
         return clientConfigsToUpdate;
     }
 
-    private static void writeMcpBundle(String toolBundleName, Bundle bundle)
+    private static void writeMcpBundle(String id, Bundle bundle)
             throws IOException {
         var serializedBundle = toJson(bundle);
-        Files.write(getBundleFileLocation(toolBundleName),
+        Files.write(getBundleFileLocation(id),
                 serializedBundle,
                 StandardOpenOption.TRUNCATE_EXISTING,
                 StandardOpenOption.CREATE);
@@ -297,35 +297,35 @@ public static McpBundleConfig addMcpBundle(Config config, String toolBundleName,
         return addMcpBundle(config, toolBundleName, bundle, false);
     }
 
-    public static McpBundleConfig addMcpBundle(Config config, String toolBundleName, Bundle bundle, boolean isLocal)
+    public static McpBundleConfig addMcpBundle(Config config, String id, Bundle bundle, boolean isLocal)
             throws IOException {
         var location = Location.builder()
-                .fileLocation(ConfigUtils.getBundleFileLocation(toolBundleName).toString())
+                .fileLocation(ConfigUtils.getBundleFileLocation(id).toString())
                 .build();
         var builder = McpBundleConfig.builder();
         switch (bundle.getValue()) {
             case SmithyMcpBundle smithyBundle -> builder.smithyModeled(SmithyModeledBundleConfig.builder()
-                    .name(toolBundleName)
-                    .description(smithyBundle.getMetadata().getDescription())
+                    .name(id)
+                    .metadata(smithyBundle.getMetadata())
                     .bundleLocation(location)
                     .local(isLocal)
                     .build());
             case GenericBundle genericBundle -> {
                 install(genericBundle.getInstall());
                 builder.genericConfig(
                         GenericToolBundleConfig.builder()
-                                .name(toolBundleName)
+                                .name(id)
                                 .local(isLocal)
                                 .bundleLocation(location)
-                                .description(genericBundle.getMetadata().getDescription())
+                                .metadata(genericBundle.getMetadata())
                                 .build());
             }
             default -> throw new IllegalStateException("Unexpected bundle type: " + bundle.type());
         }
 
         var mcpBundleConfig = builder.build();
-        writeMcpBundle(toolBundleName, bundle);
-        addMcpBundleConfig(config, toolBundleName, mcpBundleConfig);
+        writeMcpBundle(id, bundle);
+        addMcpBundleConfig(config, id, mcpBundleConfig);
         return mcpBundleConfig;
     }
 
@@ -384,9 +384,9 @@ private static String captureProcessOutput(Process process) throws IOException {
         }
     }
 
-    public static void createWrapperScript(String bundleName) throws IOException {
-        Path scriptPath = SHIMS_DIR.resolve(bundleName);
-        String scriptContent = createScriptContent(bundleName);
+    public static void createWrapperScript(String id) throws IOException {
+        Path scriptPath = SHIMS_DIR.resolve(id);
+        String scriptContent = createScriptContent(id);
 
         Files.writeString(scriptPath, scriptContent, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING);
         scriptPath.toFile().setExecutable(true);
@@ -400,8 +400,8 @@ private static String createScriptContent(String bundleName) {
         }
     }
 
-    public static void removeWrapperScript(String bundleName) throws IOException {
-        Path scriptPath = SHIMS_DIR.resolve(bundleName);
+    public static void removeWrapperScript(String id) throws IOException {
+        Path scriptPath = SHIMS_DIR.resolve(id);
         Files.deleteIfExists(scriptPath);
     }
 
@@ -502,22 +502,22 @@ private static void printUnixPathInstructions(String shimsDirStr) {
     }
 
     public static void createWrapperAndUpdateClientConfigs(
-            String name,
+            String id,
             Bundle bundle,
             Config config,
             ClientsInput input
     ) throws IOException {
         boolean shouldCreateWrapper = true;
         List<String> args = List.of();
-        String command = name;
+        String command = id;
         if (bundle.getValue() instanceof GenericBundle genericBundle && genericBundle.isExecuteDirectly()) {
             command = genericBundle.getRun().getExecutable();
             args = genericBundle.getRun().getArgs();
             shouldCreateWrapper = false;
         }
 
         if (shouldCreateWrapper) {
-            createWrapperScript(name);
+            createWrapperScript(id);
             ensureMcpServersDirInPath();
         }
 
@@ -537,11 +537,11 @@ public static void createWrapperAndUpdateClientConfigs(
         }
 
         if (print) {
-            System.out.println("You can add the following to your MCP Servers config to use " + name);
+            System.out.println("You can add the following to your MCP Servers config to use " + id);
             var serializedConfig = ByteBufferUtils.getBytes(JSON_CODEC.serialize(newClientConfig));
             System.out.println(new String(serializedConfig, StandardCharsets.UTF_8));
         } else {
-            addToClientConfigs(config, name, clientConfigs, newClientConfig);
+            addToClientConfigs(config, id, clientConfigs, newClientConfig);
         }
     }
 }
diff --git a/mcp/mcp-cli/src/main/java/software/amazon/smithy/java/mcp/cli/commands/InstallBundle.java b/mcp/mcp-cli/src/main/java/software/amazon/smithy/java/mcp/cli/commands/InstallBundle.java
@@ -9,7 +9,7 @@
 
 import java.io.IOException;
 import java.util.Set;
-import picocli.CommandLine.Mixin;
+import picocli.CommandLine.ArgGroup;
 import picocli.CommandLine.Option;
 import picocli.CommandLine.Parameters;
 import software.amazon.smithy.java.mcp.cli.ClientsInput;
@@ -28,7 +28,7 @@ public class InstallBundle extends SmithyMcpCommand {
     @Parameters(description = "Names of the MCP bundles to install.")
     Set<String> names;
 
-    @Mixin
+    @ArgGroup
     ClientsInput clientsInput;
 
     @Override
diff --git a/mcp/mcp-cli/src/main/java/software/amazon/smithy/java/mcp/cli/commands/ListBundles.java b/mcp/mcp-cli/src/main/java/software/amazon/smithy/java/mcp/cli/commands/ListBundles.java

Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,9 @@ structure CommonBundleConfig {`
`19`	`19`	`}`
`20`	`20`
`21`	`21`	`structure BundleMetadata {`
	`22`	`+ // Do not mark required for backward compatibility.`
	`23`	`+ id: String`
	`24`	`+`
`22`	`25`	`@required`
`23`	`26`	`name: String`
`24`	`27`
Original file line number	Diff line number	Diff line change
`@@ -15,5 +15,5 @@ public final class ClientsInput {`
`15`	`15`
`16`	`16`	`@Option(names = "--print-client-config",`
`17`	`17`	`description = "If specified will not edit the client configs and only print to console.")`
`18`		`- public Boolean print;`
	`18`	`+ public boolean print = false;`
`19`	`19`	`}`