feat: refine representation of commands in the API #33

connor4312 · 2025-05-10T02:50:40Z

Refs #28. After some thinking, this tries for a 'best of both worlds' approach. Simple arguments can be present essentially as the original spec recommended. As discussed, we don't specify the args for npx and friends directly, so a simple package is merely...

- name: my-api-mcp
  # ...
  packages:
    - type: npm
      name: "@c4312/my-api-mcp"
      version: "0.0.1"
      arguments:
        # Named inputs in 'arguments' generate a `--name=<value>`
        - type: named
          name: "api-key"
          is_required: true
          is_secret: true

However, I think we need to be able to specify runtime args in some cases, otherwise the registry eventually mirrors all docker configs and options which is not a great situation to be in. So, I suggest a runtime_args in the same format. Say I wanted that for npm, I could:

- name: my-api-mcp
  # ...
  packages:
    - type: npm
      name: "@c4312/my-api-mcp"
      version: "0.0.1"
      runtime_arguments:
        # Arguments with a pre-specified `value` don't get prompted for input
        - type: positional
          value: "--experimental-eventsource"

But Docker mounts are... complex, and ultimately we may deal with cases that need multiple inputs in an argument like that. So for that I suggest the third kind of "templated" input that does basic replacements and has properties that follow the same general input schema:

- name: server-filesystem
  # ...
  packages:
    - type: docker
      name: "mcp/filesystem"
      version: "0.0.1"
      runtime_arguments:
        - type: template
          description: Mount paths to access.
          name: mount_config
          is_required: true
          is_repeated: true
          # Templates support simple replacement of `{property_names}`
          template: "--mount=type=bind,src={host_path},dst={container_path}"
          properties:
            host_path:
              description: Path to access on the local machine
              is_required: true
              format: filepath
            container_path:
              description: Path to mount in the container. It should be rooted in `/project` directory.
              is_required: true
              default: "/project"

      arguments:
        - type: positional
          value: "/project"

And then client config ends up being pretty simple, for the above case for example a client could have this config:

{
  "filesystem": {
    "server": "@modelcontextprotocol/servers/src/[email protected]",
    "package": "docker",
    "settings": {
      "mount_config": [
        { "source_path": "/Users/username/Desktop", "target_path": "/project/desktop" },
        { "source_path": "/path/to/other/allowed/dir", "target_path": "/project/other/allowed/dir" },
      ]
    }
  }
}

Which might generate a CLI:

docker run \
  --mount=type=bind,src=/Users/username/Desktop,dst=/project/desktop \
  --mount=type=bind,src=/path/to/other/allowed/dir,dst=/project/other/allowed/dir \
  mcp/filesystem:1.0.2 \
  /project

I think this does a good job of accomplishing the goals we care about:

We get nicely defined inputs that are easy to extend, easy to process, easy for clients to configure, and also easy to author.
Packages can generally be updated in a backwards compatible way, given there are no changes to required arguments.
Versioning is enforced as we don't require the package to specify the identifier to npx/uvx/etc.
We usually can break away from a dependency on a specific runtime. This promise is broken a bit with runtime_arguments, but most packages should not need this and those that do are those inherently more tightly coupled to a specific runtime like Docker (e.g. they would be pretty uncommon for Node/npx or Python/uvx.)
We don't have a suple complex template system with loops and fallbacks and conditionals, like we would if we tried to have a single template string for a command.

cc @tadasant @sandy081 @digitarald @sridharavinash @toby

Refs #28. After some thinking, this tries for a 'best of both worlds' approach. Simple arguments can be present essentially as the original spec recommended. As discussed, we don't specify the args for `npx` and friends directly, so a simple package is merely... ```yaml - name: my-api-mcp # ... packages: - type: npm name: "@c4312/my-api-mcp" version: "0.0.1" arguments: # Named inputs in 'arguments' generate a `--name=<value>` - type: named name: "api-key" is_required: true is_secret: true ``` However, I think we need to be able to specify runtime args in some cases, otherwise the registry eventually mirrors all docker configs and options which is not a great situation to be in. So, I suggest a `runtime_args` in the same format. Say I wanted that for npm, I could: ```yaml - name: my-api-mcp # ... packages: - type: npm name: "@c4312/my-api-mcp" version: "0.0.1" runtime_arguments: # Arguments with a pre-specified `value` don't get prompted for input - type: positional value: "--experimental-eventsource" ``` But Docker mounts are... complex, and ultimately we may deal with cases that need multiple inputs in an argument like that. So for that I suggest the third kind of "templated" input that does basic replacements and has `properties` that follow the same general input schema: ```yaml - name: server-filesystem # ... packages: - type: docker name: "mcp/filesystem" version: "0.0.1" runtime_arguments: - type: template description: Mount paths to access. required: true repated: true template: "--mount=type=bind,src={host_path},dst={container_path}" values: host_path: description: Path to access on the local machine required: true format: filepath container_path: description: Path to mount in the container. It should be rooted in `/project` directory. required: true format: string default: "/project" arguments: - type: positional value: "/project" ``` And then client config ends up being pretty simple, for the above case for example a client could have this config: ``` { "filesystem": { "server": "@modelcontextprotocol/servers/src/[email protected]", "package": "docker", "settings": { "mount_config": [ { "source_path": "/Users/username/Desktop", "target_path": "/project/desktop" }, { "source_path": "/path/to/other/allowed/dir", "target_path": "//projects/other/allowed/dir,ro" }, ] } } } ``` Which might generate a CLI: ``` docker run \ --mount=type=bind,src=/Users/username/Desktop,dst=/project/desktop \ --mount=type=bind,src=/path/to/other/allowed/dir,dst=/project/other/allowed/dir,ro \ mcp/filesystem:1.0.2 \ /project ``` I think this does a good job of accomplishing the goals we care about: - We get nicely defined inputs that are easy to extend, easy to process, and easy for clients to configure. - Versioning is enforced as we don't require the package to specify the identifier to npx/uvx/etc. - We _usually_ can break away from a dependency on a specific runtime. This promise is broken a bit with `runtime_arguments`, most packages should not need this and those that do are those inherently more tightly coupled to a specific runtime like Docker (e.g. they would be pretty uncommon for Node or Python.) - We don't have a suple complex template system with loops and fallbacks and conditionals, like we would if we tried to have a single template string for a command.

tadasant

I like this hybrid approach a lot. named, positional, and template as "types" seem to be a good answer here. Keeps it quite minimalist and clean insofar as the config we're asking devs to write.

Just some small comments. @sridharavinash would be interesting to see if our seed data is able to conform to this without issues.

I'm a 👍 to merge and move forward with this but would appreciate some more eyes on it before proceeding.

api/README.md

api/openapi.yaml

sandy081

+1 for this approach.

However, I think we need to be able to specify runtime args in some cases, otherwise the registry eventually mirrors all docker configs and options which is not a great situation to be in. So, I suggest a runtime_args in the same format. Say I wanted that for npm, I could:

I am wondering how runtime args work for npm type package? Since we are leaving the decision about which runtime executable to run to the client, should the runtime_args also specify to what exec these args are meant for? For example, how can a client now the given runtime_args are for npx or for someother exec?

connor4312 · 2025-05-12T12:35:01Z

Since we are leaving the decision about which runtime executable to run to the client, should the runtime_args also specify to what exec these args are meant for? For example, how can a client now the given runtime_args are for npx or for someother exec?

I thought about this, perhaps with some runtime_hint?: string option. The alternative which would work for now is just for the package to, in its readme or description, describe the runtime it needs to operate. Whether a client is able to serve the runtime is very much a mileage-may-vary situation (for cases where there is that dependency there's no getting around it) and I felt unsure about putting in such an open-ended field into the spec.

For npm packages, I think I it would just be the runtime flags for Node.js itself. npx doesn't (currently) have any of its own relevant flags when running from a package, but it does pass through Node.js flags.

sandy081 · 2025-05-12T13:43:11Z

I thought about this, perhaps with some runtime_hint?: string option.

Yeah something in these lines would be good

sridharavinash · 2025-05-12T19:07:12Z

Just some small comments. @sridharavinash would be interesting to see if our seed data is able to conform to this without issues.

Shouldn't be too much of big lift. Once we're good here and I can finagle the seed data to make the new changes.

api/openapi.yaml

api/README.md

api/openapi.yaml

alexhancock · 2025-05-13T13:56:06Z

This looks great to me! I like the design of the three options.

Maybe I am missing something about the intended client logic, but what validations would we imagine for template strings? Both for validity but also command injection. For example, what would we do if someone submitted a server with:

"template": “;rm -rf ~/Development”

connor4312 · 2025-05-13T17:13:16Z

Maybe I am missing something about the intended client logic, but what validations would we imagine for template strings? Both for validity but also command injection. For example, what would we do if someone submitted a server with:

In general we have to assume that clients do the right shell escaping for arguments (or don't run the in shell, e.g. child_process.spawn). Users can do very valid things like put in quotation/punctuation marks and spaces into arguments, and if the client runs them in a shell it takes on the responsibility of escaping those.

…nv vars

sandy081

All looks good except for few minor comments.

Approving.

api/openapi.yaml

tadasant · 2025-05-13T18:50:51Z

In general we have to assume that clients do the right shell escaping for arguments (or don't run the in shell, e.g. child_process.spawn). Users can do very valid things like put in quotation/punctuation marks and spaces into arguments, and if the client runs them in a shell it takes on the responsibility of escaping those.

I think we should probably continue this discussion: #41

tadasant

Lgtm! Thank you very much for driving this @connor4312 and @sandy081

connor4312 · 2025-05-13T18:55:27Z

🚀

tiovikram · 2025-05-13T20:01:50Z

api/README.md

This greatly simplifies the command structure from the previous draft of the API specification.

There are a few matters I would like to discuss with the current iteration and get your thoughts on the validity of these matters.

1. Making the explicitly detailed parts of the command implicit:

Example: For the command, docker run --rm -i -e BRAVE_API_KEY mcp/brave-search, this change makes it such that the parts of the command such as run, --rm, -i, -e (possibly) would all have to be implicitly known to be added to the constructed config by the client for the server to work as expected.

I am more in favour of the arguments being a single array wherein a consumer of this API can convert the arguments into the MCP config format through a single statement as follows.

arguments.map((argument: Argument) => { /* some argument handling logic */})

The aforementioned approach would differ to looking up each separate type of argument in the configuration object as follows.

arguments.named_arguments((argument: Argument) => { /* some named argument handling logic */ } + arguments.runtime_arguments((argument: Argument) => { /* some named argument handling logic */ } + arguments.package_arguments((argument: Argument) => { /* some named argument handling logic */ } + arguments.template_arguments((argument: Argument) => { /* some named argument handling logic */ }

Having sets of different arguments in different places also may complicate ordering (although the effects of this may only be minor, if the arguments are mostly constructed well by all package uploaders).

2. Deleting command causes tight-coupling of "registry_name" and "command":

Deleting the "command" section with "name": "npx" or "name": "docker" then tightly couples the command to the source of the registry.

While this is not a problem if the registries are simply limited to NPM, Docker Hub or PyPI (https://pypi.org) which imply the runtime npx, docker and uvx as the command, a large benefit of this being open-source software is that enterprises may want to deploy their own versions internally for MCP use which may result in package registries being JFrog, GitHub Container Registry (ghcr.io), the artifact registry of the chosen cloud provider (AWS, Azure, GCP) or any variant that obeys the same API as NPM, PyPI or Docker.

This would result in the "registry_name" field being changed to "JFrog", "GHCR" or whatever the respective registry is while the command would still remain npx, docker, or uvx and have to be handled by the client.

I propose de-coupling the run command from the registry name as a good pattern from the previous iteration of this API draft. Please let me know your thoughts on this.

@tiovikram for visibility, I think you should open up an Issue for each of these topics, as this PR is closed and will be hard to get eyeballs on.

I'm not quite following the first suggestion, maybe some more explicit examples of what that means for the API shape would be helpful.

For the second one, it does seem worth a discussion whether we want an analog to runtime_hint

Example: For the command, docker run --rm -i -e BRAVE_API_KEY mcp/brave-search, this change makes it such that the parts of the command such as run, --rm, -i, -e (possibly) would all have to be implicitly known to be added to the constructed config by the client for the server to work as expected.

If arguments are needed to make the server work as expected, they should be in the runtime_arguemtns. Of those you included, that is only -e BRAVE_API_KEY. --rm is mostly up to the user/client if they want that behavior, -i will be something clients always will add for docker runs that communicate over stdio.

I am more in favour of the arguments being a single array wherein a consumer of this API can convert the arguments into the MCP config format through a single statement as follows.

Please see previous discussion on package versioning here for context on why this was changed #3 (comment)

Deleting the "command" section with "name": "npx" or "name": "docker" then tightly couples the command to the source of the registry.

For that reason in the past I have suggested a shell fallback type, however I didn't add that in this PR to keep things initially scoped

* Update the model to handle new changes to schema based on #33 * Made the import script a little simpler. * Add a new updated seed file * update template mcp.json file for publisher

tadasant · 2025-05-22T00:52:38Z

api/README.md

 claude_desktop_config.json:
 ```json
 {
  "filesystem": {


@connor4312 I just realized I overlooked this diff -- it's meant to be a claude_desktop_config.json file, so I don't think this change is accurate as it prescribes what Claude Desktop should be doing. I think we should have left this as-is? Or am I misreading something?

* Update the model to handle new changes to schema based on #33 * Made the import script a little simpler. * Add a new updated seed file * update template mcp.json file for publisher

* Update the model to handle new changes to schema based on modelcontextprotocol/registry#33 * Made the import script a little simpler. * Add a new updated seed file * update template mcp.json file for publisher

connor4312 force-pushed the connor4312/cmd-refinements branch from 5d411ed to c4e51cc Compare May 10, 2025 02:53

bring back choices

7652d22

connor4312 mentioned this pull request May 10, 2025

Incorporate HTTP headers for OpenAPI spec #26

Closed

tadasant reviewed May 11, 2025

View reviewed changes

api/README.md Outdated Show resolved Hide resolved

api/README.md Outdated Show resolved Hide resolved

connor4312 force-pushed the connor4312/cmd-refinements branch from ea642de to a392793 Compare May 12, 2025 02:13

allow template inputs for headers and environment variables

54609ee

connor4312 force-pushed the connor4312/cmd-refinements branch from a392793 to 54609ee Compare May 12, 2025 02:14

sandy081 reviewed May 12, 2025

View reviewed changes

api/openapi.yaml Outdated Show resolved Hide resolved

api/openapi.yaml Outdated Show resolved Hide resolved

sandy081 reviewed May 12, 2025

View reviewed changes

arguments -> package_arguments

4fc5ee0

rm UserInput array format

368afdf

sandy081 reviewed May 13, 2025

View reviewed changes

api/openapi.yaml Outdated Show resolved Hide resolved

api/README.md Outdated Show resolved Hide resolved

api/openapi.yaml Show resolved Hide resolved

api/openapi.yaml Outdated Show resolved Hide resolved

is_repeated tweaks, name positional args, template properties->variables

6e35e82

connor4312 mentioned this pull request May 13, 2025

OpenAPI spec for official registry API #3

Merged

alexhancock self-requested a review May 13, 2025 13:56

alexhancock previously approved these changes May 13, 2025

View reviewed changes

connor4312 dismissed alexhancock’s stale review via fcbe59d May 13, 2025 17:11

connor4312 force-pushed the connor4312/cmd-refinements branch from fcbe59d to 2cbca6c Compare May 13, 2025 17:16

embed template in basic arg type, simpler k/v input for headers and e…

db7f9e7

…nv vars

connor4312 force-pushed the connor4312/cmd-refinements branch from 2cbca6c to db7f9e7 Compare May 13, 2025 17:16

make positional name->name_hint, keep name for kv and flags

7b02966

sandy081 previously approved these changes May 13, 2025

View reviewed changes

api/openapi.yaml Outdated Show resolved Hide resolved

api/openapi.yaml Show resolved Hide resolved

refine descriptions, add runtime_hint, name_hint->value_hint

e2d1d76

connor4312 dismissed sandy081’s stale review via e2d1d76 May 13, 2025 17:56

connor4312 requested review from sandy081 and tadasant May 13, 2025 18:37

tadasant mentioned this pull request May 13, 2025

Provide clear guidance on avoiding security problems with template type package arguments #41

Closed

tadasant approved these changes May 13, 2025

View reviewed changes

connor4312 merged commit 314f5f4 into main May 13, 2025
2 checks passed

connor4312 deleted the connor4312/cmd-refinements branch May 13, 2025 18:55

connor4312 mentioned this pull request May 13, 2025

Simplify command in OpenAPI spec #28

Closed

tiovikram reviewed May 13, 2025

View reviewed changes

sridharavinash mentioned this pull request May 17, 2025

Api-schema-updates-1 #46

Merged

tadasant reviewed May 22, 2025

View reviewed changes

connor4312 mentioned this pull request Jun 4, 2025

Server Detail should provide a well-defined method to automate server execution #123

Closed

feat: refine representation of commands in the API #33

feat: refine representation of commands in the API #33

Uh oh!

Conversation

connor4312 commented May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tadasant left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sandy081 left a comment

Choose a reason for hiding this comment

Uh oh!

connor4312 commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sandy081 commented May 12, 2025

Uh oh!

sridharavinash commented May 12, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexhancock commented May 13, 2025

Uh oh!

connor4312 commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sandy081 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tadasant commented May 13, 2025

Uh oh!

tadasant left a comment

Choose a reason for hiding this comment

Uh oh!

connor4312 commented May 13, 2025

Uh oh!

Uh oh!

tiovikram May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tadasant May 14, 2025

Choose a reason for hiding this comment

Uh oh!

connor4312 May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tadasant May 22, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

connor4312 commented May 10, 2025 •

edited

Loading

tadasant left a comment •

edited

Loading

connor4312 commented May 12, 2025 •

edited

Loading

connor4312 commented May 13, 2025 •

edited

Loading

tiovikram May 13, 2025 •

edited

Loading

connor4312 May 14, 2025 •

edited

Loading