Skip to content

Commit fdb82a0

Browse files
cutecatfanncutecatfann
authored
Fix profile/catalog server add to upsert and allow positional args on remove
Fix profile/catalog server add to upsert and allow positional args on remove
2 parents f6db572 + 4538090 commit fdb82a0

File tree

6 files changed

+387
-34
lines changed

6 files changed

+387
-34
lines changed

‎cmd/docker-mcp/commands/catalog_next.go‎

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,33 @@ func createCatalogNextCommand() *cobra.Command {
4444

4545
cmd := &cobra.Command{
4646
Use: "create <oci-reference> [--server <ref1> --server <ref2> ...] [--from-profile <profile-id>] [--from-legacy-catalog <url>] [--from-community-registry <hostname>] [--title <title>]",
47-
Short: "Create a new catalog from a profile, legacy catalog, or community registry",
48-
Args: cobra.ExactArgs(1),
47+
Short: "Create a new catalog from server references, a profile, legacy catalog, or community registry",
48+
Long: `Create a new catalog. You can build a catalog directly from server references,
49+
from an existing profile, from a legacy catalog URL, or from a community registry.
50+
51+
When using --server without --from-profile, --from-legacy-catalog, or --from-community-registry, the --title flag is required.`,
52+
Example: ` # Build a catalog directly from server references
53+
docker mcp catalog create myorg/catalog:latest --title "My Team Catalog" \
54+
--server docker://mcp/custom-tool:latest \
55+
--server catalog://my-org/team-catalog:v1/custom-tool
56+
57+
# Build a catalog with a specific version tag
58+
docker mcp catalog create myorg/catalog:v1 --title "v1 Release" \
59+
--server catalog://vonwig/private-catalog:v1/bigquery-mcp
60+
61+
# Copy all servers from one catalog into a new one
62+
docker mcp catalog create myorg/new-catalog:latest --title "Combined" \
63+
--server catalog://my-org/team-catalog:latest
64+
65+
# Create from a profile
66+
docker mcp catalog create my-catalog --from-profile my-profile --title "My Catalog"
67+
68+
# Create from a legacy catalog
69+
docker mcp catalog create docker-mcp-catalog --from-legacy-catalog https://desktop.docker.com/mcp/catalog/v3/catalog.json
70+
71+
# Create from a community registry
72+
docker mcp catalog create my-catalog --from-community-registry registry.modelcontextprotocol.io --title "Community Servers"`,
73+
Args: cobra.ExactArgs(1),
4974
RunE: func(cmd *cobra.Command, args []string) error {
5075
sourceCount := 0
5176
if opts.FromWorkingSet != "" {
@@ -320,6 +345,9 @@ func addCatalogNextServersCommand() *cobra.Command {
320345
Example: ` # Add servers from another catalog
321346
docker mcp catalog server add mcp/my-catalog:latest --server catalog://mcp/docker-mcp-catalog:latest/github
322347
348+
# Add a server from a specific catalog version
349+
docker mcp catalog server add mcp/my-catalog:latest --server catalog://vonwig/private-catalog:v1/bigquery-mcp
350+
323351
# Add servers with OCI references
324352
docker mcp catalog server add mcp/my-catalog:latest --server docker://my-server:latest
325353
@@ -350,22 +378,28 @@ func removeCatalogNextServersCommand() *cobra.Command {
350378
var names []string
351379

352380
cmd := &cobra.Command{
353-
Use: "remove <oci-reference> --name <name1> --name <name2> ...",
381+
Use: "remove <oci-reference> [<name1> <name2> ...] [--name <name>]",
354382
Aliases: []string{"rm"},
355383
Short: "Remove MCP servers from a catalog",
356-
Long: "Remove MCP servers from a catalog by server name.",
357-
Example: ` # Remove servers by name
358-
docker mcp catalog server remove mcp/my-catalog:latest --name github --name slack
384+
Long: "Remove MCP servers from a catalog by server name. Server names can be passed as positional arguments or with the --name flag.",
385+
Example: ` # Remove servers by name (positional)
386+
docker mcp catalog server remove mcp/my-catalog:latest github slack
359387
360388
# Remove a single server
361-
docker mcp catalog server remove mcp/my-catalog:latest --name github`,
362-
Args: cobra.ExactArgs(1),
389+
docker mcp catalog server remove mcp/my-catalog:latest github
390+
391+
# Remove servers using --name flag
392+
docker mcp catalog server remove mcp/my-catalog:latest --name github --name slack`,
393+
Args: cobra.MinimumNArgs(1),
363394
RunE: func(cmd *cobra.Command, args []string) error {
395+
allNames := make([]string, 0, len(args)-1+len(names))
396+
allNames = append(allNames, args[1:]...)
397+
allNames = append(allNames, names...)
364398
dao, err := db.New()
365399
if err != nil {
366400
return err
367401
}
368-
return catalognext.RemoveServers(cmd.Context(), dao, args[0], names)
402+
return catalognext.RemoveServers(cmd.Context(), dao, args[0], allNames)
369403
},
370404
}
371405

‎cmd/docker-mcp/commands/workingset.go‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -389,11 +389,17 @@ func addServerCommand() *cobra.Command {
389389
Example: ` # Add servers from a catalog
390390
docker mcp profile server add dev-tools --server catalog://mcp/docker-mcp-catalog/github+obsidian
391391
392+
# Add a server from a specific catalog version
393+
docker mcp profile server add dev-tools --server catalog://vonwig/private-catalog:v1/bigquery-mcp
394+
395+
# Add (or update) a server that already exists in the profile
396+
docker mcp profile server add dev-tools --server catalog://vonwig/private-catalog:latest/bigquery-mcp
397+
392398
# Add servers with OCI references
393399
docker mcp profile server add my-profile --server docker://my-server:latest
394400
395401
# Add servers with MCP Registry references
396-
docker mcp profile server add my-profile --server http://registry.modelcontextprotocol.io/v0/servers/71de5a2a-6cfb-4250-a196-f93080ecc860
402+
docker mcp profile server add my-profile --server https://registry.modelcontextprotocol.io/v0/servers/71de5a2a-6cfb-4250-a196-f93080ecc860
397403
398404
# Mix server references
399405
docker mcp profile server add dev-tools --server catalog://mcp/docker-mcp-catalog/github+obsidian --server docker://my-server:latest`,
@@ -419,22 +425,28 @@ func removeServerCommand() *cobra.Command {
419425
var names []string
420426

421427
cmd := &cobra.Command{
422-
Use: "remove <profile-id> --name <name1> --name <name2> ...",
428+
Use: "remove <profile-id> [<name1> <name2> ...] [--name <name>]",
423429
Aliases: []string{"rm"},
424430
Short: "Remove MCP servers from a profile",
425-
Long: "Remove MCP servers from a profile by server name.",
426-
Example: ` # Remove servers by name
427-
docker mcp profile server remove dev-tools --name github --name slack
431+
Long: "Remove MCP servers from a profile by server name. Server names can be passed as positional arguments or with the --name flag.",
432+
Example: ` # Remove servers by name (positional)
433+
docker mcp profile server remove dev-tools github slack
428434
429435
# Remove a single server
430-
docker mcp profile server remove dev-tools --name github`,
431-
Args: cobra.ExactArgs(1),
436+
docker mcp profile server remove dev-tools github
437+
438+
# Remove servers using --name flag
439+
docker mcp profile server remove dev-tools --name github --name slack`,
440+
Args: cobra.MinimumNArgs(1),
432441
RunE: func(cmd *cobra.Command, args []string) error {
442+
allNames := make([]string, 0, len(args)-1+len(names))
443+
allNames = append(allNames, args[1:]...)
444+
allNames = append(allNames, names...)
433445
dao, err := db.New()
434446
if err != nil {
435447
return err
436448
}
437-
return workingset.RemoveServers(cmd.Context(), dao, args[0], names)
449+
return workingset.RemoveServers(cmd.Context(), dao, args[0], allNames)
438450
},
439451
}
440452

‎pkg/catalog_next/server.go‎

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -302,22 +302,34 @@ func AddServers(ctx context.Context, dao db.DAO, registryClient registryapi.Clie
302302
return fmt.Errorf("no servers found in provided references")
303303
}
304304

305-
// Convert workingset.Server to catalog Server and add to catalog
306-
addedCount := 0
307-
for _, wsServer := range allServers {
308-
if wsServer.Snapshot == nil {
309-
continue
305+
// Build set of incoming server names for upsert detection
306+
newServerNames := make(map[string]bool)
307+
for _, ws := range allServers {
308+
if ws.Snapshot != nil {
309+
newServerNames[ws.Snapshot.Server.Name] = true
310310
}
311+
}
311312

312-
serverName := wsServer.Snapshot.Server.Name
313+
// Remove existing servers that will be replaced (upsert)
314+
replacedCount := 0
315+
filtered := make([]Server, 0, len(catalog.Servers))
316+
for _, existing := range catalog.Servers {
317+
if existing.Snapshot != nil && newServerNames[existing.Snapshot.Server.Name] {
318+
fmt.Printf("Replaced server %s in catalog %s\n", existing.Snapshot.Server.Name, catalogRef)
319+
replacedCount++
320+
} else {
321+
filtered = append(filtered, existing)
322+
}
323+
}
324+
catalog.Servers = filtered
313325

314-
// Check if server already exists
315-
if catalog.FindServer(serverName) != nil {
316-
fmt.Printf("Server '%s' already exists in catalog (skipping)\n", serverName)
326+
// Convert workingset.Server to catalog Server and append
327+
addedCount := 0
328+
for _, wsServer := range allServers {
329+
if wsServer.Snapshot == nil {
317330
continue
318331
}
319332

320-
// Convert to catalog server
321333
catalogServer := Server{
322334
Type: wsServer.Type,
323335
Snapshot: wsServer.Snapshot,
@@ -336,11 +348,6 @@ func AddServers(ctx context.Context, dao db.DAO, registryClient registryapi.Clie
336348
addedCount++
337349
}
338350

339-
if addedCount == 0 {
340-
fmt.Println("No new servers added (all already exist)")
341-
return nil
342-
}
343-
344351
// Save the updated catalog
345352
dbCatalogUpdated, err := catalog.ToDb()
346353
if err != nil {
@@ -351,7 +358,11 @@ func AddServers(ctx context.Context, dao db.DAO, registryClient registryapi.Clie
351358
return fmt.Errorf("failed to update catalog: %w", err)
352359
}
353360

354-
fmt.Printf("Added %d server(s) to catalog '%s'\n", addedCount, catalogRef)
361+
if replacedCount > 0 {
362+
fmt.Printf("Added %d server(s) to catalog '%s' (replaced %d)\n", addedCount, catalogRef, replacedCount)
363+
} else {
364+
fmt.Printf("Added %d server(s) to catalog '%s'\n", addedCount, catalogRef)
365+
}
355366
return nil
356367
}
357368

‎pkg/catalog_next/server_test.go‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1068,6 +1068,116 @@ func TestAddServers(t *testing.T) {
10681068
})
10691069
}
10701070

1071+
func TestAddServersUpsert(t *testing.T) {
1072+
dao := setupTestDB(t)
1073+
ctx := t.Context()
1074+
1075+
mockOci := mocks.NewMockOCIService(mocks.WithLocalImages([]mocks.MockImage{
1076+
{
1077+
Ref: "existing-server:v2",
1078+
Labels: map[string]string{
1079+
"io.docker.server.metadata": "name: existing-server\ntype: server\nimage: existing-server:v2",
1080+
},
1081+
DigestString: "sha256:abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcd",
1082+
},
1083+
}))
1084+
1085+
t.Run("upsert replaces existing", func(t *testing.T) {
1086+
catalogObj := Catalog{
1087+
Ref: "test/upsert-catalog:latest",
1088+
CatalogArtifact: CatalogArtifact{
1089+
Title: "Upsert Catalog",
1090+
Servers: []Server{
1091+
{
1092+
Type: workingset.ServerTypeImage,
1093+
Image: "existing-server:v1",
1094+
Snapshot: &workingset.ServerSnapshot{
1095+
Server: catalog.Server{
1096+
Name: "existing-server",
1097+
Type: "server",
1098+
Image: "existing-server:v1",
1099+
},
1100+
},
1101+
},
1102+
},
1103+
},
1104+
}
1105+
1106+
dbCat, err := catalogObj.ToDb()
1107+
require.NoError(t, err)
1108+
err = dao.UpsertCatalog(ctx, dbCat)
1109+
require.NoError(t, err)
1110+
1111+
// Add a server with the same name but different image -- should upsert
1112+
err = AddServers(ctx, dao, mocks.NewMockRegistryAPIClient(), mockOci, catalogObj.Ref, []string{
1113+
"docker://existing-server:v2",
1114+
})
1115+
require.NoError(t, err)
1116+
1117+
dbCat2, err := dao.GetCatalog(ctx, catalogObj.Ref)
1118+
require.NoError(t, err)
1119+
cat := NewFromDb(dbCat2)
1120+
assert.Len(t, cat.Servers, 1)
1121+
assert.Equal(t, "existing-server", cat.Servers[0].Snapshot.Server.Name)
1122+
assert.Equal(t, "existing-server:v2", cat.Servers[0].Image)
1123+
})
1124+
1125+
t.Run("upsert preserves other servers", func(t *testing.T) {
1126+
catalogObj := Catalog{
1127+
Ref: "test/upsert-catalog2:latest",
1128+
CatalogArtifact: CatalogArtifact{
1129+
Title: "Upsert Catalog 2",
1130+
Servers: []Server{
1131+
{
1132+
Type: workingset.ServerTypeImage,
1133+
Image: "existing-server:v1",
1134+
Snapshot: &workingset.ServerSnapshot{
1135+
Server: catalog.Server{
1136+
Name: "existing-server",
1137+
Type: "server",
1138+
Image: "existing-server:v1",
1139+
},
1140+
},
1141+
},
1142+
{
1143+
Type: workingset.ServerTypeImage,
1144+
Image: "other-server:v1",
1145+
Snapshot: &workingset.ServerSnapshot{
1146+
Server: catalog.Server{
1147+
Name: "other-server",
1148+
Type: "server",
1149+
Image: "other-server:v1",
1150+
},
1151+
},
1152+
},
1153+
},
1154+
},
1155+
}
1156+
1157+
dbCat, err := catalogObj.ToDb()
1158+
require.NoError(t, err)
1159+
err = dao.UpsertCatalog(ctx, dbCat)
1160+
require.NoError(t, err)
1161+
1162+
// Upsert only existing-server
1163+
err = AddServers(ctx, dao, mocks.NewMockRegistryAPIClient(), mockOci, catalogObj.Ref, []string{
1164+
"docker://existing-server:v2",
1165+
})
1166+
require.NoError(t, err)
1167+
1168+
dbCat2, err := dao.GetCatalog(ctx, catalogObj.Ref)
1169+
require.NoError(t, err)
1170+
cat := NewFromDb(dbCat2)
1171+
assert.Len(t, cat.Servers, 2)
1172+
// other-server should be first (preserved in place)
1173+
assert.Equal(t, "other-server", cat.Servers[0].Snapshot.Server.Name)
1174+
assert.Equal(t, "other-server:v1", cat.Servers[0].Image)
1175+
// existing-server should be at the end (re-added after filtering)
1176+
assert.Equal(t, "existing-server", cat.Servers[1].Snapshot.Server.Name)
1177+
assert.Equal(t, "existing-server:v2", cat.Servers[1].Image)
1178+
})
1179+
}
1180+
10711181
func TestRemoveServers(t *testing.T) {
10721182
dao := setupTestDB(t)
10731183
ctx := t.Context()

‎pkg/workingset/server.go‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,27 @@ func AddServers(ctx context.Context, dao db.DAO, registryClient registryapi.Clie
5656

5757
RegisterOAuthProvidersForServers(ctx, newServers)
5858

59+
// Build set of incoming server names for upsert detection
60+
newServerNames := make(map[string]bool)
61+
for _, s := range newServers {
62+
if s.Snapshot != nil {
63+
newServerNames[s.Snapshot.Server.Name] = true
64+
}
65+
}
66+
67+
// Remove existing servers that will be replaced (upsert)
68+
replacedCount := 0
69+
filtered := make([]Server, 0, len(workingSet.Servers))
70+
for _, existing := range workingSet.Servers {
71+
if existing.Snapshot != nil && newServerNames[existing.Snapshot.Server.Name] {
72+
fmt.Printf("Replaced server %s in profile %s\n", existing.Snapshot.Server.Name, id)
73+
replacedCount++
74+
} else {
75+
filtered = append(filtered, existing)
76+
}
77+
}
78+
workingSet.Servers = filtered
79+
5980
workingSet.Servers = append(workingSet.Servers, newServers...)
6081

6182
if err := workingSet.Validate(); err != nil {
@@ -67,7 +88,11 @@ func AddServers(ctx context.Context, dao db.DAO, registryClient registryapi.Clie
6788
return fmt.Errorf("failed to update profile: %w", err)
6889
}
6990

70-
fmt.Printf("Added %d server(s) to profile %s\n", len(newServers), id)
91+
if replacedCount > 0 {
92+
fmt.Printf("Added %d server(s) to profile %s (replaced %d)\n", len(newServers), id, replacedCount)
93+
} else {
94+
fmt.Printf("Added %d server(s) to profile %s\n", len(newServers), id)
95+
}
7196

7297
return nil
7398
}

0 commit comments

Comments
 (0)