feat: Add ability to add/remove servers #223
Conversation
Add functionality to add and remove servers from a workingset. Can add/remove by OCI ref or registry URI.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| func RemoveServers(ctx context.Context, dao db.DAO, id string, serverRefs []string) error { | ||
| if len(serverRefs) == 0 { | ||
| return fmt.Errorf("at least one server must be specified") | ||
| } | ||
|
|
||
| dbWorkingSet, err := dao.GetWorkingSet(ctx, id) | ||
| if err != nil { | ||
| if errors.Is(err, sql.ErrNoRows) { | ||
| return fmt.Errorf("working set %s not found", id) | ||
| } | ||
| return fmt.Errorf("failed to get working set: %w", err) | ||
| } | ||
|
|
||
| workingSet := NewFromDb(dbWorkingSet) | ||
|
|
||
| // Build a set of servers to remove (strip protocol scheme for comparison) | ||
| refsToRemove := make(map[string]bool) | ||
| for _, ref := range serverRefs { | ||
| normalized := stripProtocol(ref) | ||
| refsToRemove[normalized] = true | ||
| } | ||
|
|
||
| // Filter out the servers to remove | ||
| originalCount := len(workingSet.Servers) | ||
| filtered := make([]Server, 0, len(workingSet.Servers)) | ||
| for _, server := range workingSet.Servers { | ||
| shouldKeep := true | ||
|
|
||
| switch server.Type { | ||
| case ServerTypeImage: | ||
| if refsToRemove[stripProtocol(server.Image)] { | ||
| shouldKeep = false | ||
| } | ||
| case ServerTypeRegistry: | ||
| if refsToRemove[stripProtocol(server.Source)] { | ||
| shouldKeep = false | ||
| } | ||
| } | ||
|
|
||
| if shouldKeep { | ||
| filtered = append(filtered, server) | ||
| } | ||
| } | ||
|
|
||
| removedCount := originalCount - len(filtered) | ||
| if removedCount == 0 { | ||
| return fmt.Errorf("no matching servers found to remove") | ||
| } |
There was a problem hiding this comment.
The new RemoveServers API only strips protocol schemes from the provided references and then does a raw string comparison against the stored Image/Source fields. When a working set was created from an OCI tag or an unversioned registry URL, the AddServers path canonicalizes those references (tags are expanded to digests and registry URLs are rewritten to concrete version URLs). Deleting using the same tag or …/versions/latest URL shown in the CLI examples will not match the canonicalized entry, causing "no matching servers found to remove" even though the server exists. Removal should resolve the incoming references the same way as addition before attempting to match.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@cmrigney should removals be done on the name in the snapshot?
What I did
Added commands to add and remove servers from a working set.
docker mcp workingset server add [working-set-id] --server [ref or uri]Example
docker mcp workingset server add test-set --server docker://roberthouse224/mcp-dice:latest --server https://mcp.docker.com/v0/servers/slackdocker mcp workingset server remove [working-set-id] --server [ref or uri]Example
docker mcp workingset server remove test-set --server docker://roberthouse224/mcp-dice:latest --server https://mcp.docker.com/v0/servers/slack/versions/latestRelated issue
(very mandatory) A picture of a cute animal, if possible in relation to what you did
