Skip to content

Conversation

aliok
Copy link
Collaborator

@aliok aliok commented Oct 7, 2025

Fixes #140

Instructions to test it:
Create an mcpfile.yaml that starts like this

# yaml-language-server: $schema=[../../specs/mcpfile-schema.json](https://raw.githubusercontent.com/genmcp/gen-mcp/eab4c564008f33b88d015aec50f935f2d5b23622/specs/mcpfile-schema-0.1.0.json)
mcpFileVersion: 0.1.0
...

Summary by CodeRabbit

  • New Features

    • Added a JSON Schema for MCP files enabling validation and editor autocompletion.
    • CLI now prepends a schema header to generated YAML for immediate editor support.
    • Improved invocation and runtime configuration models with clearer fields and validation.
  • Documentation

    • Added guidance on authoring mcpfile.yaml with autocomplete and editor setup tips.
    • Documented a JSON Schema generator utility.
  • Chores

    • Added CI verification for generated artifacts and a Makefile target to update codegen.
  • Examples

    • Updated sample MCP files to reference the schema and simplified tool definitions.

@Cali0707 Cali0707 self-requested a review October 7, 2025 12:10
@aliok
Copy link
Collaborator Author

aliok commented Oct 7, 2025

@coderabbitai review

Copy link

coderabbitai bot commented Oct 7, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (2)
.github/workflows/verify-codegen.yaml (1)

32-51: Apply shellcheck recommendations for robustness.

The verification logic is correct, but shellcheck has identified several style and robustness issues:

  • Use $(...) notation instead of legacy backticks
  • Quote variables to prevent globbing and word splitting

Apply this diff to address the shellcheck issues:

           if [[ -z "$(git status --porcelain)" ]]; then
               echo "${{ github.repository }} up to date."
           else
               # Print it both ways because sometimes we haven't modified the file (e.g. zz_generated),
               # and sometimes we have (e.g. configmap checksum).
-              echo "Found diffs in: $(git diff-index --name-only HEAD --)"
-              for x in $(git diff-index --name-only HEAD --); do
-                  echo "::error file=$x::Please run `make update-codegen`.%0A$(git diff $x | urlencode)"
+              echo "Found diffs in: $(git diff-index --name-only HEAD --)"
+              for x in $(git diff-index --name-only HEAD --); do
+                  echo "::error file=$x::Please run \`make update-codegen\`.%0A$(git diff "$x" | urlencode)"
               done
               echo "${{ github.repository }} is out of date. Please run \`make update-codegen\`"
               exit 1
           fi

Note: The backticks in the error message string are already escaped, but the command substitution on line 47 should use $(git diff "$x" | urlencode) instead of backticks, and $x should be quoted.

Based on static analysis hints.

mcpfile-schema.json (1)

381-386: Add placeholder comment for StdioConfig
StdioConfig is currently an empty marker type. If it’s intended as a placeholder for future stdio transport options, update the Go struct’s doc comment (e.g. “// Placeholder for future stdio configuration options”) to make this explicit.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 40d43fd and 4ee39af.

⛔ Files ignored due to path filters (2)
  • hack/jsonschemagen/go.sum is excluded by !**/*.sum
  • hack/jsonschemagen/mcpfile-autocomplete-screenshot.png is excluded by !**/*.png
📒 Files selected for processing (24)
  • .github/workflows/verify-codegen.yaml (1 hunks)
  • Makefile (1 hunks)
  • README.md (1 hunks)
  • examples/http-conversion/feature-requests/mcpfile-localhost.yaml (1 hunks)
  • examples/http-conversion/feature-requests/mcpfile.yaml (1 hunks)
  • examples/http-conversion/mcpfile.yaml (1 hunks)
  • examples/http-conversion/oauth-mcpfile.yaml (1 hunks)
  • hack/jsonschemagen/README.md (1 hunks)
  • hack/jsonschemagen/go.mod (1 hunks)
  • hack/jsonschemagen/main.go (1 hunks)
  • hack/library.sh (1 hunks)
  • hack/update-codegen.sh (1 hunks)
  • mcpfile-schema.json (1 hunks)
  • pkg/mcpfile/testdata/full-demo.yaml (1 hunks)
  • pkg/mcpfile/testdata/no-servers.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-cli-tools.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-no-tools.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-prompts.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-stateful.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-tls.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-tools-http-params.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-tools.yaml (1 hunks)
  • pkg/mcpfile/testdata/server-runtime-stdio.yaml (1 hunks)
  • pkg/mcpfile/types.go (4 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/verify-codegen.yaml

32-32: shellcheck reported issue in this script: SC2006:style:15:43: Use $(...) notation instead of legacy backticks ...

(shellcheck)


32-32: shellcheck reported issue in this script: SC2086:info:15:79: Double quote to prevent globbing and word splitting

(shellcheck)


32-32: shellcheck reported issue in this script: SC2006:style:17:63: Use $(...) notation instead of legacy backticks ...

(shellcheck)

🪛 Shellcheck (0.11.0)
hack/library.sh

[warning] 5-5: ROOT_DIR appears unused. Verify use (or export if used externally).

(SC2034)


[warning] 11-11: Declare and assign separately to avoid masking return values.

(SC2155)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Unit Tests
🔇 Additional comments (3)
hack/jsonschemagen/main.go (1)

14-32: LGTM! Schema generation logic is sound.

The schema generation workflow is well-structured with proper error handling at each step. The relative paths for package location and output file assume execution from hack/jsonschemagen/, which should work correctly when invoked via the documented build scripts.

mcpfile-schema.json (2)

1-4: LGTM! Schema metadata is correctly structured.

The schema uses JSON Schema Draft 2020-12 and properly references the root MCPFile definition.


24-65: LGTM! MCPFile root schema is well-structured.

The schema correctly defines required fields (mcpFileVersion, name, version) and optional collections (tools, prompts), with additionalProperties: false for strict validation.

@Cali0707
Copy link
Collaborator

Cali0707 commented Oct 7, 2025

@aliok can we also add this annotation as a comment to the top of the converted mcpfiles ?

I'm fine with that being either in this PR or in a follow up

Copy link
Collaborator

@Cali0707 Cali0707 left a comment

Choose a reason for hiding this comment

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

Thanks for all the progress here @aliok - it's super cool to see the language server in the IDE picking up on the schema 🤯

@aliok aliok changed the title JSON schema for mcpfile [WIP] JSON schema for mcpfile Oct 7, 2025
@aliok aliok force-pushed the 2025-10-07-json-schema branch from 3db4435 to eab4c56 Compare October 8, 2025 11:15
@genmcp genmcp deleted a comment from coderabbitai bot Oct 8, 2025
@aliok aliok marked this pull request as ready for review October 8, 2025 11:25
@aliok aliok requested a review from a team as a code owner October 8, 2025 11:25
Copy link

coderabbitai bot commented Oct 8, 2025

Walkthrough

Adds JSON Schema generation for MCP types, CI verification for generated schema, Makefile integration, CLI schema header injection, expanded public MCP types with jsonschema tags, generated schema artifacts under specs/, and YAML examples/testdata annotated with schema directives.

Changes

Cohort / File(s) Summary
CI: Verify codegen
​.github/workflows/verify-codegen.yaml
New GitHub Actions workflow that runs make update-codegen on push/PR and fails with annotated diffs if generated artifacts are out of date.
Build integration
Makefile, hack/update-codegen.sh
Adds update-codegen make target and makes build-cli depend on it; hack/update-codegen.sh runs the schema generator.
Schema generator utility
hack/jsonschemagen/... (README.md, go.mod, main.go)
New Go module/program that reflects Go types and emits versioned and "latest" JSON Schemas into specs/.
Schema artifacts
specs/mcpfile-schema.json, specs/mcpfile-schema-0.1.0.json
Generated JSON Schema files describing MCPFile, Tool, Prompt, invocation configs (HTTP/CLI), runtime, auth, templates, etc.
Public type surface and tags
pkg/mcpfile/types.go, pkg/invocation/http/config.go, pkg/invocation/cli/config.go
Exported/augmented structs and interfaces; added HttpInvocationData, CliInvocationData; enriched json/jsonschema tags and validation logic.
CLI output schema header
pkg/cli/convert.go, pkg/cli/convert-cli.go, pkg/cli/utils/schema.go
New utils.AppendSchemaHeader and usage in CLI paths to prepend YAML language-server $schema header to emitted MCP YAML.
Examples updated with schema directive
examples/http-conversion/... (mcpfile.yaml, mcpfile-localhost.yaml, oauth-mcpfile.yaml)
Added # yaml-language-server: $schema=... header lines; removed some tool entries in select example files.
Testdata updated with schema directive
pkg/mcpfile/testdata/*
Added YAML schema directive comments to testdata files; adjusted some tool blocks (add/remove) in selected fixtures.
Docs
README.md, hack/jsonschemagen/README.md
Documentation on authoring MCP files with autocomplete and README for the jsonschemagen utility.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as Developer
  participant GA as GitHub Actions
  participant Make as make
  participant Gen as jsonschemagen (go run)
  participant Repo as specs/

  Dev->>GA: Push / PR
  GA->>Make: make update-codegen
  Make->>Gen: go run hack/jsonschemagen/main.go
  Gen->>Repo: Write mcpfile-schema-<version>.json
  Gen->>Repo: Write mcpfile-schema.json (latest)
  GA->>GA: git diff
  alt No changes
    GA-->>Dev: "Repository up to date"
  else Changes detected
    GA-->>Dev: Annotated errors with diffs
    GA-->>Dev: Fail job
  end
Loading
sequenceDiagram
  autonumber
  participant User as User
  participant CLI as gen-mcp CLI
  participant Util as utils.AppendSchemaHeader
  participant FS as Filesystem

  User->>CLI: Convert config -> MCP YAML
  CLI->>CLI: Marshal MCPFile to YAML
  CLI->>Util: AppendSchemaHeader(bytes)
  Util-->>CLI: YAML with `# yaml-language-server: $schema=...` prefixed
  CLI->>FS: Write YAML with schema header
  CLI-->>User: Output file
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Suggested reviewers

  • Cali0707

Poem

A rabbit taps keys with a sprightly grin,
“Schemas now bloom where types have been!”
CI stands guard, ears tall and keen,
YAML wears headers—polished and clean.
Hop, hop—codegen done, specs align; 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning This PR also removes tool definitions in example YAML files and introduces runtime validation methods and utility logic for HttpInvocationConfig, which are not directly required by the JSON schema objective and extend beyond the scope of issue #140. Please separate or revert changes unrelated to JSON schema support—such as example tool removals and added HttpInvocationConfig validation—and address them in distinct follow-up PRs to keep this feature focused.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “JSON schema for mcpfile” succinctly and accurately captures the primary change of this pull request, which is to introduce JSON schema support for the MCP file format.
Linked Issues Check ✅ Passed The pull request delivers on issue #140 by adding the JSON schema files in specs, implementing a code generator with associated CI and Makefile targets, updating documentation and examples for yaml-language-server support, and generating the schema from Go structs, thereby fulfilling all core requirements for schema publication and IDE integration.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 2025-10-07-json-schema

Comment @coderabbitai help to get the list of available commands and usage tips.

@aliok aliok changed the title [WIP] JSON schema for mcpfile JSON schema for mcpfile Oct 8, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

♻️ Duplicate comments (1)
pkg/mcpfile/types.go (1)

132-140: Make stateless optional to match actual configs.

StreamableHTTPConfig.Stateless is still tagged jsonschema:"required". Our YAML/JSON readers happily omit this field (it defaults to false), so the generated schema now rejects valid MCP files. Drop the required tag and add omitempty to the JSON tag so the schema reflects reality.

-	// Indicates whether the server is stateless.
-	Stateless bool `json:"stateless" jsonschema:"required"`
+	// Indicates whether the server is stateless.
+	Stateless bool `json:"stateless,omitempty" jsonschema:"optional"`
🧹 Nitpick comments (2)
README.md (1)

144-161: Consider recommending versioned schema URL.

The documentation is clear and helpful. However, the example schema URL points to refs/heads/main which references the unstable main branch:

# yaml-language-server: $schema=https://raw.githubusercontent.com/genmcp/gen-mcp/refs/heads/main/specs/mcpfile-schema.json

For production use, consider also documenting a versioned approach:

  • Reference a specific release tag: refs/tags/v0.1.0/specs/mcpfile-schema-0.1.0.json
  • Or use a "latest" symlink if available: specs/mcpfile-schema-latest.json

This prevents schema changes on main from unexpectedly affecting existing projects.

Alternatively, you could add a note recommending users pin to a specific version in production environments.

.github/workflows/verify-codegen.yaml (1)

32-51: Improve shell script robustness.

The verification script has minor shell style and safety issues flagged by shellcheck:

  1. Legacy backtick syntax on Lines 45 and 46
  2. Unquoted variable expansion on Line 45 could cause word splitting/globbing issues

Apply this diff to address the shellcheck warnings:

           if [[ -z "$(git status --porcelain)" ]]; then
               echo "${{ github.repository }} up to date."
           else
               # Print it both ways because sometimes we haven't modified the file (e.g. zz_generated),
               # and sometimes we have (e.g. configmap checksum).
-              echo "Found diffs in: $(git diff-index --name-only HEAD --)"
-              for x in $(git diff-index --name-only HEAD --); do
+              echo "Found diffs in: $(git diff-index --name-only HEAD --)"
+              while IFS= read -r x; do
                   echo "::error file=$x::Please run \`make update-codegen\`.%0A$(git diff $x | urlencode)"
-              done
+              done < <(git diff-index --name-only HEAD --)
               echo "${{ github.repository }} is out of date. Please run \`make update-codegen\`"
               exit 1
           fi

This refactor:

  • Keeps modern command substitution syntax $(...) (already in use on Line 45)
  • Replaces the for x in $(...) loop with a safer while read pattern that handles filenames with spaces correctly
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 266f68a and eab4c56.

⛔ Files ignored due to path filters (2)
  • hack/jsonschemagen/go.sum is excluded by !**/*.sum
  • hack/jsonschemagen/mcpfile-autocomplete-screenshot.png is excluded by !**/*.png
📒 Files selected for processing (29)
  • .github/workflows/verify-codegen.yaml (1 hunks)
  • Makefile (1 hunks)
  • README.md (1 hunks)
  • examples/http-conversion/feature-requests/mcpfile-localhost.yaml (1 hunks)
  • examples/http-conversion/feature-requests/mcpfile.yaml (1 hunks)
  • examples/http-conversion/mcpfile.yaml (1 hunks)
  • examples/http-conversion/oauth-mcpfile.yaml (1 hunks)
  • hack/jsonschemagen/README.md (1 hunks)
  • hack/jsonschemagen/go.mod (1 hunks)
  • hack/jsonschemagen/main.go (1 hunks)
  • hack/update-codegen.sh (1 hunks)
  • pkg/cli/convert-cli.go (2 hunks)
  • pkg/cli/convert.go (2 hunks)
  • pkg/cli/utils/schema.go (1 hunks)
  • pkg/invocation/cli/config.go (3 hunks)
  • pkg/invocation/http/config.go (1 hunks)
  • pkg/mcpfile/testdata/full-demo.yaml (1 hunks)
  • pkg/mcpfile/testdata/no-servers.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-cli-tools.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-no-tools.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-prompts.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-stateful.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-tls.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-tools-http-params.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-tools.yaml (1 hunks)
  • pkg/mcpfile/testdata/server-runtime-stdio.yaml (1 hunks)
  • pkg/mcpfile/types.go (4 hunks)
  • specs/mcpfile-schema-0.1.0.json (1 hunks)
  • specs/mcpfile-schema.json (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-07T17:29:19.408Z
Learnt from: Cali0707
PR: genmcp/gen-mcp#155
File: hack/jsonschemagen/go.mod:3-8
Timestamp: 2025-10-07T17:29:19.408Z
Learning: The gen-mcp project targets Go 1.24+ and does not require backward compatibility with Go 1.21.

Applied to files:

  • hack/jsonschemagen/go.mod
🧬 Code graph analysis (5)
pkg/cli/convert-cli.go (1)
pkg/cli/utils/schema.go (1)
  • AppendSchemaHeader (9-16)
pkg/cli/convert.go (1)
pkg/cli/utils/schema.go (1)
  • AppendSchemaHeader (9-16)
hack/update-codegen.sh (1)
hack/lib/common.sh (1)
  • header_text (12-14)
pkg/cli/utils/schema.go (1)
pkg/mcpfile/types.go (1)
  • MCPFileVersion (10-10)
hack/jsonschemagen/main.go (3)
pkg/mcpfile/types.go (2)
  • MCPFile (200-206)
  • MCPFileVersion (10-10)
pkg/invocation/http/config.go (1)
  • HttpInvocationData (23-26)
pkg/invocation/cli/config.go (1)
  • CliInvocationData (11-14)
🪛 actionlint (1.7.7)
.github/workflows/verify-codegen.yaml

32-32: shellcheck reported issue in this script: SC2006:style:15:43: Use $(...) notation instead of legacy backticks ...

(shellcheck)


32-32: shellcheck reported issue in this script: SC2086:info:15:79: Double quote to prevent globbing and word splitting

(shellcheck)


32-32: shellcheck reported issue in this script: SC2006:style:17:63: Use $(...) notation instead of legacy backticks ...

(shellcheck)

🪛 markdownlint-cli2 (0.18.1)
hack/jsonschemagen/README.md

41-41: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🔇 Additional comments (25)
examples/http-conversion/oauth-mcpfile.yaml (1)

1-1: LGTM! Schema directive correctly configured.

The YAML language server directive enables IDE autocompletion and validation. The relative path ../../specs/mcpfile-schema.json correctly resolves from examples/http-conversion/ to the repository root specs/ directory.

examples/http-conversion/feature-requests/mcpfile-localhost.yaml (2)

1-1: Schema directive correctly configured.

The relative path ../../../specs/mcpfile-schema.json correctly resolves from examples/http-conversion/feature-requests/ to the repository root specs/ directory.


9-17: AI summary inconsistency: tool is still present.

The AI-generated summary claims a tool was removed ("Returns a list of all features sorted by upvotes (highest first)"), but this tool is still present in the code at lines 9-17 with name features-get. The provided code shows the final state after changes, so the summary appears incorrect.

hack/jsonschemagen/go.mod (1)

1-23: LGTM! Module configuration aligns with project standards.

The Go version 1.24.7 is appropriate for this project, as the gen-mcp project targets Go 1.24+ (per retrieved learnings). The module structure is correct:

  • Replace directive properly references the parent repository
  • Dependencies are appropriate for JSON schema generation (invopop/jsonschema)
  • Indirect dependencies are properly listed

Based on learnings.

pkg/invocation/cli/config.go (3)

16-28: LGTM! Well-documented schema metadata.

The addition of jsonschema tags enables proper JSON schema generation. The comments clearly explain field purposes, and ParameterIndices is appropriately marked as internal with json:"-" and a clear explanation.


40-52: LGTM! Clear schema annotations.

The jsonschema tags properly document required vs. optional fields. Comments provide helpful context about field usage, including the example of "--user=%s" or "--verbose".


60-65: LGTM! Improved type safety.

The safe type assertion b, ok := value.(bool) prevents panics when the value is not a boolean. This is a good defensive programming practice.

pkg/mcpfile/testdata/one-server-tools-http-params.yaml (1)

1-1: LGTM! Test data now schema-validated.

Adding the schema directive to test data helps ensure test files remain valid as the schema evolves. The relative path correctly resolves to the repository root specs/ directory.

pkg/mcpfile/testdata/no-servers.yaml (1)

1-1: LGTM! Minimal test case includes schema validation.

Even this minimal test file benefits from schema validation, ensuring the bare minimum configuration remains valid.

pkg/mcpfile/testdata/one-server-stateful.yaml (1)

1-1: LGTM! Test data aligns with schema and code changes.

The schema directive enables validation for this CLI invocation test case. The file content at lines 30-37 demonstrates the templateVariables feature that now has proper JSON schema support via the changes in pkg/invocation/cli/config.go.

pkg/mcpfile/testdata/one-server-tools.yaml (1)

1-1: LGTM! Schema directive correctly added.

The YAML language server directive enables editor validation and autocompletion without affecting runtime behavior.

pkg/mcpfile/testdata/one-server-tls.yaml (1)

1-1: LGTM! Schema directive correctly added.

The directive enables schema-based editor tooling without modifying the file's runtime semantics.

pkg/mcpfile/testdata/server-runtime-stdio.yaml (1)

1-1: LGTM! Schema directive correctly added.

The directive enables schema-based editor tooling.

Note: The AI summary mentions removal of the clone_repo tool, but it is still present in the provided code (lines 8-33).

pkg/mcpfile/testdata/one-server-cli-tools.yaml (1)

1-1: LGTM! Schema directive correctly added.

The directive is consistent with other testdata files and enables editor validation.

pkg/mcpfile/testdata/one-server-no-tools.yaml (1)

1-1: LGTM! Schema directive correctly added.

The directive aligns with the schema integration pattern across testdata files.

pkg/mcpfile/testdata/one-server-prompts.yaml (1)

1-1: LGTM! Schema directive correctly added.

The directive is properly formatted and consistent with the schema integration approach.

examples/http-conversion/feature-requests/mcpfile.yaml (1)

1-1: LGTM! Schema directive correctly added.

The directive extends schema validation to example files, maintaining consistency with testdata.

pkg/cli/convert-cli.go (1)

7-7: LGTM! Import correctly added.

The utils package import is necessary for the schema header functionality.

pkg/cli/convert.go (1)

62-63: LGTM!

The schema header is correctly appended to the marshalled MCP file bytes before writing to disk, ensuring generated YAML files include the yaml-language-server directive for IDE support.

pkg/mcpfile/testdata/full-demo.yaml (1)

1-1: LGTM!

The schema header is correctly added with an appropriate relative path for testdata files, enabling IDE validation and autocomplete for this example mcpfile.

examples/http-conversion/mcpfile.yaml (2)

1-1: LGTM!

The schema header is correctly added with an appropriate relative path for example files.


8-86: Summary inconsistency detected.

The AI-generated summary claims "Removed all tool definitions under the 'tools' section (six tools removed...)", but the provided code clearly shows 6 tool definitions still present (get_features, post_features, get_features-top, get_features-id, post_features-vote, post_features-complete).

hack/update-codegen.sh (1)

1-13: LGTM!

The codegen script is well-structured with proper error handling (set -e and set -o pipefail), clean directory navigation using pushd/popd, and appropriate status output using the header_text helper from common.sh.

Makefile (2)

25-29: LGTM!

The new update-codegen target is well-defined and provides clear status messages.


32-32: LGTM!

Adding update-codegen as a prerequisite for build-cli ensures the schema is always up-to-date before building the CLI, preventing potential inconsistencies.

@Cali0707
Copy link
Collaborator

Cali0707 commented Oct 8, 2025

@aliok can you fix the merge conflicts on this?

@aliok aliok force-pushed the 2025-10-07-json-schema branch from eab4c56 to ef793cb Compare October 8, 2025 20:51
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (3)
pkg/mcpfile/types.go (1)

227-243: Make stateless optional again.

Keeping stateless marked jsonschema:"required" means every MCP file must now set it, yet none of our shipped examples do. Validation will fail for real configs. Please drop the required tag and add omitempty to the JSON tag so existing files stay valid.

Apply this diff:

 type StreamableHTTPConfig struct {
   // Port number to listen on.
   Port int `json:"port" jsonschema:"required"`
@@
-  // Indicates whether the server is stateless.
-  Stateless bool `json:"stateless" jsonschema:"required"`
+  // Indicates whether the server is stateless.
+  Stateless bool `json:"stateless,omitempty" jsonschema:"optional"`
pkg/invocation/cli/config.go (1)

10-14: Rename the CliInvocationData field to match its content.

The field holds CLI config but is still called Http, which is confusing and contradicts the json:"cli" tag. Please rename it to Cli (and update call sites) so the API reflects its purpose.

Suggested change:

 type CliInvocationData struct {
   // Detailed CLI invocation configuration.
-  Http CliInvocationConfig `json:"cli" jsonschema:"required"`
+  Cli CliInvocationConfig `json:"cli" jsonschema:"required"`
 }
hack/jsonschemagen/README.md (1)

41-45: Add a language hint to the directory tree block

Please tag this fenced block with a language (e.g. ```text) so markdownlint stops flagging it and the tree renders correctly.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eab4c56 and ef793cb.

⛔ Files ignored due to path filters (2)
  • hack/jsonschemagen/go.sum is excluded by !**/*.sum
  • hack/jsonschemagen/mcpfile-autocomplete-screenshot.png is excluded by !**/*.png
📒 Files selected for processing (29)
  • .github/workflows/verify-codegen.yaml (1 hunks)
  • Makefile (1 hunks)
  • README.md (1 hunks)
  • examples/http-conversion/feature-requests/mcpfile-localhost.yaml (1 hunks)
  • examples/http-conversion/feature-requests/mcpfile.yaml (1 hunks)
  • examples/http-conversion/mcpfile.yaml (1 hunks)
  • examples/http-conversion/oauth-mcpfile.yaml (1 hunks)
  • hack/jsonschemagen/README.md (1 hunks)
  • hack/jsonschemagen/go.mod (1 hunks)
  • hack/jsonschemagen/main.go (1 hunks)
  • hack/update-codegen.sh (1 hunks)
  • pkg/cli/convert-cli.go (2 hunks)
  • pkg/cli/convert.go (2 hunks)
  • pkg/cli/utils/schema.go (1 hunks)
  • pkg/invocation/cli/config.go (3 hunks)
  • pkg/invocation/http/config.go (1 hunks)
  • pkg/mcpfile/testdata/full-demo.yaml (1 hunks)
  • pkg/mcpfile/testdata/no-servers.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-cli-tools.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-no-tools.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-prompts.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-stateful.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-tls.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-tools-http-params.yaml (1 hunks)
  • pkg/mcpfile/testdata/one-server-tools.yaml (1 hunks)
  • pkg/mcpfile/testdata/server-runtime-stdio.yaml (1 hunks)
  • pkg/mcpfile/types.go (6 hunks)
  • specs/mcpfile-schema-0.1.0.json (1 hunks)
  • specs/mcpfile-schema.json (1 hunks)
✅ Files skipped from review due to trivial changes (2)
  • pkg/mcpfile/testdata/one-server-tools.yaml
  • examples/http-conversion/oauth-mcpfile.yaml
🚧 Files skipped from review as they are similar to previous changes (18)
  • examples/http-conversion/feature-requests/mcpfile.yaml
  • pkg/mcpfile/testdata/one-server-prompts.yaml
  • pkg/mcpfile/testdata/no-servers.yaml
  • examples/http-conversion/feature-requests/mcpfile-localhost.yaml
  • pkg/mcpfile/testdata/one-server-no-tools.yaml
  • pkg/cli/convert.go
  • hack/jsonschemagen/main.go
  • Makefile
  • examples/http-conversion/mcpfile.yaml
  • pkg/mcpfile/testdata/server-runtime-stdio.yaml
  • pkg/cli/utils/schema.go
  • pkg/mcpfile/testdata/one-server-stateful.yaml
  • hack/jsonschemagen/go.mod
  • pkg/cli/convert-cli.go
  • pkg/mcpfile/testdata/one-server-tools-http-params.yaml
  • pkg/mcpfile/testdata/one-server-tls.yaml
  • pkg/mcpfile/testdata/full-demo.yaml
  • README.md
🧰 Additional context used
🧬 Code graph analysis (1)
hack/update-codegen.sh (1)
hack/lib/common.sh (1)
  • header_text (12-14)
🪛 actionlint (1.7.7)
.github/workflows/verify-codegen.yaml

32-32: shellcheck reported issue in this script: SC2006:style:15:43: Use $(...) notation instead of legacy backticks ...

(shellcheck)


32-32: shellcheck reported issue in this script: SC2086:info:15:79: Double quote to prevent globbing and word splitting

(shellcheck)


32-32: shellcheck reported issue in this script: SC2006:style:17:63: Use $(...) notation instead of legacy backticks ...

(shellcheck)

🪛 markdownlint-cli2 (0.18.1)
hack/jsonschemagen/README.md

34-34: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


44-44: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Unit Tests

@aliok
Copy link
Collaborator Author

aliok commented Oct 9, 2025

@Cali0707 no more conflicts

Copy link
Collaborator

@Cali0707 Cali0707 left a comment

Choose a reason for hiding this comment

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

LGTM

This is awesome @aliok 🎉

Comment on lines +558 to +702
"oneOf": [
{
"$ref": "#/$defs/HttpInvocationData"
},
{
"$ref": "#/$defs/CliInvocationData"
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Woohoo this is awesome @aliok 🎉

@Cali0707 Cali0707 merged commit 51ca93c into main Oct 9, 2025
4 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Oct 9, 2025
@Cali0707 Cali0707 deleted the 2025-10-07-json-schema branch October 14, 2025 17:13
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.

JSON schema for MCP file format

2 participants