-
Notifications
You must be signed in to change notification settings - Fork 518
Enhanced server.json validation (phase 1) #636
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
56a23b8
0d3950d
0ad650e
3d80156
0dbbc8e
1b59820
2144c55
19b6621
e4ba595
28ae714
4a35d9b
6467163
823d0c0
8259279
02392d6
8e00ac6
838f20d
5a16478
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| name: Sync Schema | ||
|
|
||
| on: | ||
| workflow_dispatch: # Manual trigger | ||
| # TODO: Add daily schedule later | ||
| # schedule: | ||
| # - cron: '0 2 * * *' # Run daily at 2 AM UTC | ||
|
|
||
| permissions: | ||
| contents: write | ||
|
|
||
| jobs: | ||
| sync-schema: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Checkout static repo | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| repository: modelcontextprotocol/static | ||
| path: static-repo | ||
|
|
||
| - name: Sync schemas from static repo | ||
| run: | | ||
| echo "🔍 Syncing schemas from modelcontextprotocol/static..." | ||
| mkdir -p internal/validators/schemas | ||
|
|
||
| # Copy all versioned schema files | ||
| for dir in static-repo/schemas/*/; do | ||
| if [ -f "$dir/server.schema.json" ]; then | ||
| version=$(basename "$dir") | ||
| # Skip draft directory if it exists | ||
| if [ "$version" != "draft" ]; then | ||
| output_file="internal/validators/schemas/${version}.json" | ||
| if [ ! -f "$output_file" ] || ! cmp -s "$dir/server.schema.json" "$output_file"; then | ||
| echo "⬇ Adding/updating ${version}/server.schema.json -> ${version}.json" | ||
| cp "$dir/server.schema.json" "$output_file" | ||
| else | ||
| echo "✓ ${version} is already up to date" | ||
| fi | ||
| fi | ||
| fi | ||
| done | ||
|
|
||
| echo "✅ Schema sync complete" | ||
|
|
||
| - name: Check for changes | ||
| id: changes | ||
| run: | | ||
| # Check for both modified and untracked files | ||
| if [ -n "$(git status --porcelain internal/validators/schemas/)" ]; then | ||
| echo "changed=true" >> $GITHUB_OUTPUT | ||
| git status --porcelain internal/validators/schemas/ | ||
| else | ||
| echo "changed=false" >> $GITHUB_OUTPUT | ||
| echo "No changes to schemas" | ||
| fi | ||
|
|
||
| - name: Commit and push changes | ||
| if: steps.changes.outputs.changed == 'true' | ||
| run: | | ||
| git config --local user.email "action@github.com" | ||
| git config --local user.name "GitHub Action" | ||
| git add internal/validators/schemas/ | ||
| git commit -m "Sync schemas from modelcontextprotocol/static [skip ci]" | ||
| git push |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,4 +12,7 @@ validate-schemas | |
| coverage.out | ||
| coverage.html | ||
| deploy/infra/infra | ||
| ./registry | ||
| registry | ||
|
|
||
| # Generated schema directory for embedding | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "embedding" as a term is a little misdirecting at first glance (given the prominence of vector embeddings in AI); wherever you use this can we please make clear it's about "embedding the static file into the Go binary" |
||
| internal/validators/schema/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,12 +5,17 @@ help: ## Show this help message | |
| @echo "Available targets:" | ||
| @grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf " %-20s %s\n", $$1, $$2}' | ||
|
|
||
| # Preparation targets | ||
| prep-schema: ## Copy schema file for embedding | ||
| @mkdir -p internal/validators/schema | ||
| @cp docs/reference/server-json/server.schema.json internal/validators/schema/server.schema.json | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we lock this into being a static, live version of the schema (instead of this linked one, which is a I think in general it would be smart to not make changes in the |
||
|
|
||
| # Build targets | ||
| build: ## Build the registry application with version info | ||
| build: prep-schema ## Build the registry application with version info | ||
| @mkdir -p bin | ||
| go build -ldflags="-X main.Version=dev-$(shell git rev-parse --short HEAD) -X main.GitCommit=$(shell git rev-parse HEAD) -X main.BuildTime=$(shell date -u +%Y-%m-%dT%H:%M:%SZ)" -o bin/registry ./cmd/registry | ||
|
|
||
| publisher: ## Build the publisher tool with version info | ||
| publisher: prep-schema ## Build the publisher tool with version info | ||
| @mkdir -p bin | ||
| go build -ldflags="-X main.Version=dev-$(shell git rev-parse --short HEAD) -X main.GitCommit=$(shell git rev-parse HEAD) -X main.BuildTime=$(shell date -u +%Y-%m-%dT%H:%M:%SZ)" -o bin/mcp-publisher ./cmd/publisher | ||
|
|
||
|
|
@@ -26,7 +31,7 @@ check-schema: ## Check if server.schema.json is in sync with openapi.yaml | |
| @./bin/extract-server-schema -check | ||
|
|
||
| # Test targets | ||
| test-unit: ## Run unit tests with coverage (requires PostgreSQL) | ||
| test-unit: prep-schema ## Run unit tests with coverage (requires PostgreSQL) | ||
| @echo "Starting PostgreSQL for unit tests..." | ||
| @docker compose up -d postgres 2>&1 | grep -v "Pulling\|Pulled\|Creating\|Created\|Starting\|Started" || true | ||
| @echo "Waiting for PostgreSQL to be ready..." | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,8 +12,8 @@ import ( | |
| "path/filepath" | ||
| "strings" | ||
|
|
||
| "github.com/modelcontextprotocol/registry/internal/validators" | ||
| apiv0 "github.com/modelcontextprotocol/registry/pkg/api/v0" | ||
| "github.com/modelcontextprotocol/registry/pkg/model" | ||
| ) | ||
|
|
||
| func PublishCommand(args []string) error { | ||
|
|
@@ -40,13 +40,24 @@ func PublishCommand(args []string) error { | |
|
|
||
| // Check for deprecated schema and recommend migration | ||
| // Allow empty schema (will use default) but reject old schemas | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Allowing empty schema seems weird here - do you agree? Realize it's out of scope but if you agree I'll file an Issue |
||
| if serverJSON.Schema != "" && !strings.Contains(serverJSON.Schema, model.CurrentSchemaVersion) { | ||
| return fmt.Errorf(`deprecated schema detected: %s. | ||
| if serverJSON.Schema != "" { | ||
| // Get current schema version from embedded schema | ||
| currentSchema, err := validators.GetCurrentSchemaVersion() | ||
| if err != nil { | ||
| // Schema is embedded, so this should never happen | ||
| return fmt.Errorf("failed to get current schema version: %w", err) | ||
| } | ||
|
|
||
| if serverJSON.Schema != currentSchema { | ||
| return fmt.Errorf(`deprecated schema detected: %s. | ||
|
|
||
| Expected current schema: %s | ||
|
|
||
| Migrate to the current schema format for new servers. | ||
|
|
||
| 📋 Migration checklist: https://github.com/modelcontextprotocol/registry/blob/main/docs/reference/server-json/CHANGELOG.md#migration-checklist-for-publishers | ||
| 📖 Full changelog with examples: https://github.com/modelcontextprotocol/registry/blob/main/docs/reference/server-json/CHANGELOG.md`, serverJSON.Schema) | ||
| 📖 Full changelog with examples: https://github.com/modelcontextprotocol/registry/blob/main/docs/reference/server-json/CHANGELOG.md`, serverJSON.Schema, currentSchema) | ||
| } | ||
| } | ||
|
|
||
| // Load saved token | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| package commands | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "fmt" | ||
| "os" | ||
| "strings" | ||
|
|
||
| "github.com/modelcontextprotocol/registry/internal/validators" | ||
| apiv0 "github.com/modelcontextprotocol/registry/pkg/api/v0" | ||
| ) | ||
|
|
||
| func ValidateCommand(args []string) error { | ||
| // Parse arguments | ||
| serverFile := "server.json" | ||
|
|
||
| for _, arg := range args { | ||
| if !strings.HasPrefix(arg, "-") { | ||
| serverFile = arg | ||
| } | ||
| } | ||
|
|
||
| // Read server file | ||
| serverData, err := os.ReadFile(serverFile) | ||
| if err != nil { | ||
| if os.IsNotExist(err) { | ||
| return fmt.Errorf("%s not found, please check the file path", serverFile) | ||
| } | ||
| return fmt.Errorf("failed to read %s: %w", serverFile, err) | ||
| } | ||
|
|
||
| // Validate JSON | ||
| var serverJSON apiv0.ServerJSON | ||
| if err := json.Unmarshal(serverData, &serverJSON); err != nil { | ||
| return fmt.Errorf("invalid JSON: %w", err) | ||
| } | ||
|
|
||
| // Check for deprecated schema and recommend migration | ||
| // Allow empty schema (will use default) but reject old schemas | ||
| if serverJSON.Schema != "" { | ||
| // Get current schema version from embedded schema | ||
| currentSchema, err := validators.GetCurrentSchemaVersion() | ||
| if err != nil { | ||
| // Should never happen (schema is embedded) | ||
| return fmt.Errorf("failed to get current schema version: %w", err) | ||
| } | ||
|
|
||
| if serverJSON.Schema != currentSchema { | ||
| return fmt.Errorf(`deprecated schema detected: %s. | ||
|
|
||
| Expected current schema: %s | ||
|
|
||
| Migrate to the current schema format for new servers. | ||
|
|
||
| 📋 Migration checklist: https://github.com/modelcontextprotocol/registry/blob/main/docs/reference/server-json/CHANGELOG.md#migration-checklist-for-publishers | ||
| 📖 Full changelog with examples: https://github.com/modelcontextprotocol/registry/blob/main/docs/reference/server-json/CHANGELOG.md`, serverJSON.Schema, currentSchema) | ||
| } | ||
| } | ||
|
|
||
| // Run detailed validation (this is the whole point of the validate command) | ||
| // Include schema validation for comprehensive validation | ||
| result := validators.ValidateServerJSONExhaustive(&serverJSON, true) | ||
|
|
||
| if result.Valid { | ||
| _, _ = fmt.Fprintln(os.Stdout, "✅ server.json is valid") | ||
| return nil | ||
| } | ||
|
|
||
| // Print all issues | ||
| _, _ = fmt.Fprintf(os.Stdout, "❌ Validation failed with %d issue(s):\n\n", len(result.Issues)) | ||
| for i, issue := range result.Issues { | ||
| _, _ = fmt.Fprintf(os.Stdout, "%d. [%s] %s (%s)\n", i+1, issue.Severity, issue.Path, issue.Type) | ||
| _, _ = fmt.Fprintf(os.Stdout, " %s\n", issue.Message) | ||
| if issue.Reference != "" { | ||
| _, _ = fmt.Fprintf(os.Stdout, " Reference: %s\n", issue.Reference) | ||
| } | ||
| _, _ = fmt.Fprintln(os.Stdout) | ||
| } | ||
|
|
||
| return fmt.Errorf("validation failed") | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as below on "embedding"