Address profile cli feedback

cmd/docker-mcp/commands/workingset.go

@@ -128,14 +128,13 @@ To view enabled tools, use: docker mcp profile show <profile-id>`,
 
 func createWorkingSetCommand(cfg *client.Config) *cobra.Command {
 	var opts struct {
-		ID      string
-		Name    string
+		Title   string
 		Servers []string
 		Connect []string
 	}
 
 	cmd := &cobra.Command{
-		Use:   "create --name <name> [--id <id>] --server <ref1> --server <ref2> ... [--connect <client1> --connect <client2> ...]",
+		Use:   "create <profile-id> [--title <title>] [--server <ref1> --server <ref2> ...] [--connect <client1> --connect <client2> ...]",
 		Short: "Create a new profile of MCP servers",
 		Long: `Create a new profile that groups multiple MCP servers together.
 A profile allows you to organize and manage related servers as a single unit.
@@ -145,34 +144,36 @@ Profiles are decoupled from catalogs. Servers can be:
   - Catalog references with catalog:// prefix (e.g., "catalog://mcp/docker-mcp-catalog/github+obsidian").
   - Local file references with file:// prefix (e.g., "file://./server.yaml").`,
 		Example: `  # Create a profile with servers from a catalog
-  docker mcp profile create --name dev-tools --server catalog://mcp/docker-mcp-catalog/github+obsidian
+  docker mcp profile create dev_tools --title "Dev Tools" --server catalog://mcp/docker-mcp-catalog/github+obsidian
 
   # Create a profile with multiple servers (OCI references)
-  docker mcp profile create --name my-profile --server docker://my-server:latest --server docker://my-other-server:latest
+  docker mcp profile create my_profile --title "My Profile" --server docker://my-server:latest --server docker://my-other-server:latest
 
   # Create a profile with MCP Registry references
-  docker mcp profile create --name my-profile --server http://registry.modelcontextprotocol.io/v0/servers/71de5a2a-6cfb-4250-a196-f93080ecc860
+  docker mcp profile create my_profile --title "My Profile" --server http://registry.modelcontextprotocol.io/v0/servers/71de5a2a-6cfb-4250-a196-f93080ecc860
 
   # Connect to clients upon creation
-  docker mcp profile create --name dev-tools --connect cursor`,
-		Args: cobra.NoArgs,
-		RunE: func(cmd *cobra.Command, _ []string) error {
+  docker mcp profile create dev_tools --title "Dev Tools" --connect cursor
+
+  # Create profile without title (title will be same as id)
+  docker mcp profile create dev_tools --server docker://my-server:latest`,
+		Args: cobra.ExactArgs(1),
+		RunE: func(cmd *cobra.Command, args []string) error {
 			dao, err := db.New()
 			if err != nil {
 				return err
 			}
 			registryClient := registryapi.NewClient()
 			ociService := oci.NewService()
-			return workingset.Create(cmd.Context(), dao, registryClient, ociService, opts.ID, opts.Name, opts.Servers, opts.Connect)
+			profileID := args[0]
+			return workingset.Create(cmd.Context(), dao, registryClient, ociService, profileID, opts.Title, opts.Servers, opts.Connect)
 		},
 	}
 
 	flags := cmd.Flags()
-	flags.StringVar(&opts.Name, "name", "", "Name of the profile (required)")
-	flags.StringVar(&opts.ID, "id", "", "ID of the profile (defaults to a slugified version of the name)")
+	flags.StringVar(&opts.Title, "title", "", "Title of the profile (optional, defaults to profile-id if not provided)")
 	flags.StringArrayVar(&opts.Servers, "server", []string{}, "Server to include specified with a URI: https:// (MCP Registry reference) or docker:// (Docker Image reference) or catalog:// (Catalog reference) or file:// (Local file path). Can be specified multiple times.")
 	flags.StringArrayVar(&opts.Connect, "connect", []string{}, fmt.Sprintf("Clients to connect to: mcp-client (can be specified multiple times). Supported clients: %s", client.GetSupportedMCPClients(*cfg)))
-	_ = cmd.MarkFlagRequired("name")
 
 	return cmd
 }
@@ -416,31 +417,28 @@ func addServerCommand() *cobra.Command {
 }
 
 func removeServerCommand() *cobra.Command {
-	var names []string
-
 	cmd := &cobra.Command{
-		Use:     "remove <profile-id> --name <name1> --name <name2> ...",
+		Use:     "remove <profile-id> <server-name>",
 		Aliases: []string{"rm"},
-		Short:   "Remove MCP servers from a profile",
-		Long:    "Remove MCP servers from a profile by server name.",
-		Example: ` # Remove servers by name
-  docker mcp profile server remove dev-tools --name github --name slack
-
-  # Remove a single server
-  docker mcp profile server remove dev-tools --name github`,
-		Args: cobra.ExactArgs(1),
+		Short:   "Remove MCP server from a profile",
+		Long:    "Remove MCP server from a profile by server name.",
+		Example: ` # Remove a server by name
+  docker mcp profile server remove dev-tools github
+
+  # Remove another server
+  docker mcp profile server remove dev-tools slack`,
+		Args: cobra.ExactArgs(2),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			dao, err := db.New()
 			if err != nil {
 				return err
 			}
-			return workingset.RemoveServers(cmd.Context(), dao, args[0], names)
+			profileID := args[0]
+			serverName := args[1]
+			return workingset.RemoveServers(cmd.Context(), dao, profileID, []string{serverName})
 		},
 	}
 
-	flags := cmd.Flags()
-	flags.StringArrayVar(&names, "name", []string{}, "Server name to remove (can be specified multiple times)")
-
 	return cmd
 }
 

pkg/workingset/create.go

@@ -31,20 +31,23 @@ func Create(ctx context.Context, dao db.DAO, registryClient registryapi.Client,
 		}
 	}
 
-	var err error
-	if id != "" {
-		_, err := dao.GetWorkingSet(ctx, id)
-		if err == nil {
-			return fmt.Errorf("profile with id %s already exists", id)
-		}
-		if !errors.Is(err, sql.ErrNoRows) {
-			return fmt.Errorf("failed to look for existing profile: %w", err)
-		}
-	} else {
-		id, err = createWorkingSetID(ctx, name, dao)
-		if err != nil {
-			return fmt.Errorf("failed to create profile id: %w", err)
-		}
+	// ID is mandatory
+	if id == "" {
+		return fmt.Errorf("id is required")
+	}
+
+	// Check if profile with this ID already exists
+	_, err := dao.GetWorkingSet(ctx, id)
+	if err == nil {
+		return fmt.Errorf("profile with id %s already exists", id)
+	}
+	if !errors.Is(err, sql.ErrNoRows) {
+		return fmt.Errorf("failed to look for existing profile: %w", err)
+	}
+
+	// Generate title from ID if not provided
+	if name == "" {
+		name = generateTitleFromID(id)
 	}
 
 	// Add default secrets

pkg/workingset/create_test.go

@@ -94,7 +94,7 @@ func TestCreateWithDockerImages(t *testing.T) {
 	dao := setupTestDB(t)
 	ctx := t.Context()
 
-	err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "", "My Test Set", []string{
+	err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "my_test_set", "My Test Set", []string{
 		"docker://myimage:latest",
 		"docker://anotherimage:v1.0",
 	}, []string{})
@@ -120,7 +120,7 @@ func TestCreateWithRegistryServers(t *testing.T) {
 	dao := setupTestDB(t)
 	ctx := t.Context()
 
-	err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "", "Registry Set", []string{
+	err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "registry_set", "Registry Set", []string{
 		"https://example.com/v0/servers/server1",
 		"https://example.com/v0/servers/server2",
 	}, []string{})
@@ -144,7 +144,7 @@ func TestCreateWithMixedServers(t *testing.T) {
 	dao := setupTestDB(t)
 	ctx := t.Context()
 
-	err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "", "Mixed Set", []string{
+	err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "mixed_set", "Mixed Set", []string{
 		"docker://myimage:latest",
 		"https://example.com/v0/servers/server1",
 	}, []string{})
@@ -196,32 +196,36 @@ func TestCreateWithExistingId(t *testing.T) {
 	assert.Contains(t, err.Error(), "already exists")
 }
 
-func TestCreateGeneratesUniqueIds(t *testing.T) {
+func TestCreateGeneratesTitleFromId(t *testing.T) {
 	dao := setupTestDB(t)
 	ctx := t.Context()
 
-	// Create first working set
-	err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "", "Test Set", []string{
+	// Create working set with ID but no title
+	err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "test_set_one", "", []string{
 		"docker://myimage:latest",
 	}, []string{})
 	require.NoError(t, err)
 
-	// Create second with same name
-	err = Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "", "Test Set", []string{
-		"docker://anotherimage:v1.0",
-	}, []string{})
+	// Verify title equals ID when not explicitly provided
+	dbSet, err := dao.GetWorkingSet(ctx, "test_set_one")
 	require.NoError(t, err)
+	assert.Equal(t, "test_set_one", dbSet.Name)
 
-	// Create third with same name
-	err = Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "", "Test Set", []string{
+	// Create another with different ID
+	err = Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "my_profile_v2", "", []string{
 		"docker://anotherimage:v1.0",
 	}, []string{})
 	require.NoError(t, err)
 
+	// Verify title equals ID when not explicitly provided
+	dbSet, err = dao.GetWorkingSet(ctx, "my_profile_v2")
+	require.NoError(t, err)
+	assert.Equal(t, "my_profile_v2", dbSet.Name)
+
 	// List all working sets
 	sets, err := dao.ListWorkingSets(ctx)
 	require.NoError(t, err)
-	assert.Len(t, sets, 3)
+	assert.Len(t, sets, 2)
 
 	// Verify IDs are unique
 	ids := make(map[string]bool)
@@ -230,39 +234,43 @@ func TestCreateGeneratesUniqueIds(t *testing.T) {
 		ids[set.ID] = true
 	}
 
-	// Verify ID pattern
-	assert.Contains(t, ids, "test_set")
-	assert.Contains(t, ids, "test_set_2")
-	assert.Contains(t, ids, "test_set_3")
+	// Verify the expected IDs exist
+	assert.Contains(t, ids, "test_set_one")
+	assert.Contains(t, ids, "my_profile_v2")
 }
 
 func TestCreateWithInvalidServerFormat(t *testing.T) {
 	dao := setupTestDB(t)
 	ctx := t.Context()
 
-	err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "", "Test Set", []string{
+	err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "test_set", "Test Set", []string{
 		"invalid-format",
 	}, []string{})
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "invalid server value")
 }
 
-func TestCreateWithEmptyName(t *testing.T) {
+func TestCreateWithEmptyTitle(t *testing.T) {
 	dao := setupTestDB(t)
 	ctx := t.Context()
 
-	err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "test-id", "", []string{
+	// Create profile with ID but no title - title should equal ID
+	err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "test_id", "", []string{
 		"docker://myimage:latest",
 	}, []string{})
-	require.Error(t, err)
-	assert.Contains(t, err.Error(), "invalid profile")
+	require.NoError(t, err)
+
+	// Verify title equals ID when not explicitly provided
+	dbSet, err := dao.GetWorkingSet(ctx, "test_id")
+	require.NoError(t, err)
+	assert.Equal(t, "test_id", dbSet.Name)
 }
 
 func TestCreateWithEmptyServers(t *testing.T) {
 	dao := setupTestDB(t)
 	ctx := t.Context()
 
-	err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "", "Empty Set", []string{}, []string{})
+	err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "empty_set", "Empty Set", []string{}, []string{})
 	require.NoError(t, err)
 
 	// Verify the working set was created with no servers
@@ -277,7 +285,7 @@ func TestCreateAddsDefaultSecrets(t *testing.T) {
 	dao := setupTestDB(t)
 	ctx := t.Context()
 
-	err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "", "Test Set", []string{
+	err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "test_set", "Test Set", []string{
 		"docker://myimage:latest",
 	}, []string{})
 	require.NoError(t, err)
@@ -292,31 +300,31 @@ func TestCreateAddsDefaultSecrets(t *testing.T) {
 	assert.Equal(t, "docker-desktop-store", dbSet.Secrets["default"].Provider)
 }
 
-func TestCreateNameWithSpecialCharacters(t *testing.T) {
+func TestCreateTitleGenerationFromID(t *testing.T) {
 	tests := []struct {
-		name       string
-		inputName  string
-		expectedID string
+		name          string
+		inputID       string
+		expectedTitle string
 	}{
 		{
-			name:       "name with spaces",
-			inputName:  "My Test Set",
-			expectedID: "my_test_set",
+			name:          "id with underscores",
+			inputID:       "my_test_set",
+			expectedTitle: "my_test_set",
 		},
 		{
-			name:       "name with special chars",
-			inputName:  "Test@Set#123!",
-			expectedID: "test_set_123_",
+			name:          "id with multiple underscores",
+			inputID:       "test_set_123",
+			expectedTitle: "test_set_123",
 		},
 		{
-			name:       "name with multiple spaces",
-			inputName:  "Test   Set",
-			expectedID: "test_set",
+			name:          "simple id",
+			inputID:       "testset",
+			expectedTitle: "testset",
 		},
 		{
-			name:       "name with underscores",
-			inputName:  "Test_Set_Name",
-			expectedID: "test_set_name",
+			name:          "id with numbers",
+			inputID:       "test_set_v2",
+			expectedTitle: "test_set_v2",
 		},
 	}
 
@@ -326,16 +334,17 @@ func TestCreateNameWithSpecialCharacters(t *testing.T) {
 			dao := setupTestDB(t)
 			ctx := t.Context()
 
-			err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "", tt.inputName, []string{
+			// Create profile with ID but no title - title should equal ID
+			err := Create(ctx, dao, getMockRegistryClient(), getMockOciService(), tt.inputID, "", []string{
 				"docker://myimage:latest",
 			}, []string{})
 			require.NoError(t, err)
 
-			// Verify the ID was generated correctly
-			dbSet, err := dao.GetWorkingSet(ctx, tt.expectedID)
+			// Verify the title equals ID when not explicitly provided
+			dbSet, err := dao.GetWorkingSet(ctx, tt.inputID)
 			require.NoError(t, err)
 			require.NotNil(t, dbSet)
-			assert.Equal(t, tt.expectedID, dbSet.ID)
+			assert.Equal(t, tt.expectedTitle, dbSet.Name)
 		})
 	}
 }

pkg/workingset/list.go

@@ -71,7 +71,7 @@ func printListHumanReadable(workingSets []WorkingSet, showPolicy bool) string {
 	}
 	lines = strings.TrimSuffix(lines, "\n")
 	if showPolicy {
-		return fmt.Sprintf("ID\tName\tPolicy\n----\t----\t------\n%s", lines)
+		return fmt.Sprintf("ID\tTitle\tPolicy\n----\t-----\t------\n%s", lines)
 	}
-	return fmt.Sprintf("ID\tName\n----\t----\n%s", lines)
+	return fmt.Sprintf("ID\tTitle\n----\t-----\n%s", lines)
 }

pkg/workingset/list_test.go

@@ -70,8 +70,8 @@ func TestListHumanReadable(t *testing.T) {
 	})
 
 	// Verify header
-	assert.Contains(t, output, "ID\tName")
-	assert.Contains(t, output, "----\t----")
+	assert.Contains(t, output, "ID\tTitle")
+	assert.Contains(t, output, "----\t-----")
 
 	// Verify data
 	assert.Contains(t, output, "set-1\tFirst Set")