Skip to content

Conversation

@bobbyhouse
Copy link
Contributor

What I did

  • Add an option for atomic updates of add and remove servers from a profile with docker mcp profile server update --add uri --remove uri
  • Add ability to specify server to remove by URI docker mcp server remove --server [uri]

Related issue

(not mandatory) A picture of a cute animal, if possible in relation to what you did

To address issue where too much data was being printed when showing an
entire catalog, introduce a option to specify --no-tools to exclude
tools from the response.

Additionally, add a `docker mcp catalog-next server ls` command so that
we can get individual server information from a catalog that does
include the tools.
Using --format json or yaml would return the entire contents of all the
catalogs. Instead, return the data that is returned for human readable
output.
@bobbyhouse bobbyhouse requested a review from a team as a code owner December 2, 2025 10:07
@bobbyhouse bobbyhouse force-pushed the mcpt-243-244-remove-and-update branch from fc781e1 to d34deb5 Compare December 2, 2025 10:08
@chatgpt-codex-connector
Copy link

💡 Codex Review

removedCount = originalCount - len(filtered)
workingSet.Servers = filtered
}

P1 Badge Detect missing removals in UpdateServers

UpdateServers calculates removedCount but never checks whether any servers were actually removed, so docker mcp profile server update --remove … succeeds even when the requested URIs don’t match the profile and still proceeds to add new servers. This breaks the advertised atomic add/remove behavior and differs from RemoveServers, which errors when nothing is removed.

ℹ️ 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".

- Add ability to remove servers by URI
- Add an update command for atomic add/remove
@bobbyhouse bobbyhouse force-pushed the mcpt-243-244-remove-and-update branch from d34deb5 to 3358a57 Compare December 2, 2025 10:18
@bobbyhouse bobbyhouse changed the base branch from main to mcpt-185-add-servers-from-catalog December 2, 2025 10:19
Daniel-Kolev
Daniel-Kolev previously approved these changes Dec 3, 2025
Base automatically changed from mcpt-185-add-servers-from-catalog to main December 3, 2025 18:00
@cmrigney cmrigney dismissed Daniel-Kolev’s stale review December 3, 2025 18:00

The base branch was changed.

Copy link
Contributor

@cmrigney cmrigney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't looked at the code yet, but wanted to provide feedback that I think we should avoid removing servers by URI for a few reasons:

  1. I think it's an anti-pattern. Servers are identified by <profile-id>.<server-name>, not by where it came from. If someone is trying to remove by URI, then they're probably trying to use the CLI in a way that wasn't intended or fits with the data model. Adding a server is different, because you do need to specify where to "download" it from. Analogously, when you download a file from a url, you don't delete it by specifying the url again. Instead, you delete it by its file name.
  2. It's unreliable. For example, docker://myimage might have the name my-image when added but then a later build has the name other-image when removed. Not only would it not get removed as the user might expect (because there's no match), but it could remove the wrong server.
  3. It adds unnecessary overhead of fetching information about the server again, which we already have stored in the profile.

@bobbyhouse bobbyhouse closed this Dec 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants