## Motivation and Context Per discussion with @rdimitrov - all validation now happens in a new `/validate` endpoint. For details, see below. ## Breaking Changes None. Validation on /publish only validates schema version and semantic checks (manual checks) as before. We can add schema validation by changing a flag in the called to `ValidateJSONSchema` when we're ready. ## Types of changes - [X] New feature (non-breaking change which adds functionality) ## Checklist - [X] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [X] My code follows the repository's style guidelines - [X] New and existing tests pass locally - [X] I have added appropriate error handling - [X] I have added or updated documentation as needed ## Additional context ## API Changes ### 1. New `/validate` Endpoint A new public endpoint was added for validating `server.json` files without publishing them. **Details:** - **Paths:** `/v0/validate` and `/v0.1/validate` - **Method:** `POST` - **Authentication:** None required (public endpoint) - **Request:** JSON body containing `ServerJSON` - **Response:** `200 OK` with `ValidationResult` containing: - `valid` (boolean) - `issues` (array of validation issues with type, path, message, severity, reference) **Implementation:** - New handler file: `internal/api/handlers/v0/validate.go` - Performs comprehensive validation using `ValidationAll` option: - Schema version validation - Full schema validation - Semantic validation - Always returns `200 OK` - validity is indicated in `result.Valid` field - Registered in router for both `/v0` and `/v0.1` prefixes ### 2. `/publish` Endpoint Validation Flow Changes The publish endpoint now validates earlier in the request flow and provides clearer error responses. **Before:** Validation was mixed into `ValidatePublishRequest()` (which also handled publisher extensions and registry ownership). **After:** 1. Validates `ServerJSON` structure first using `ValidateServerJSON()` (schema version + semantic validation) 2. Returns `422 Unprocessable Entity` with message: "Failed to publish server, invalid schema: call /validate for details" 3. Only proceeds to publisher extensions/registry validation if schema validation passes 4. No longer calls `ValidatePublishRequest()` internally for schema validation **Key Changes:** - Validation happens before `CreateServer()` call - Returns `422` on schema/semantic validation failures - Error message directs users to `/validate` endpoint for details - `ValidatePublishRequest()` no longer performs schema validation (added note in comments) ### 3. `/edit` Endpoint Validation Flow Changes Similar validation changes were applied to the edit endpoint. **Changes:** - Added `ValidateServerJSON()` call before server update - Returns `422` with same error message format on validation failure - Matches the `/publish` endpoint behavior ### 4. Validator Functions Refactoring Updated validator functions to reflect that schema validation is now done separately. **Changes to `ValidatePublishRequest()`:** - Removed internal `ValidateServerJSON()` call - Added note: "ValidateServerJSON should be called separately before this function" - Now only validates publisher extensions and registry ownership **Changes to `ValidateUpdateRequest()`:** - Same change (removed schema validation, added note) - Now only validates registry ownership ## CLI Changes ### 1. `validate` Command: Now Uses API Endpoint **Before:** Performed local validation using `validators.ValidateServerJSON()`. **After:** Calls the `/v0/validate` endpoint on the registry. **Details:** - Reads `server.json` and sends it to the registry API - Displays validation results returned by the server - Uses the same error formatting as before - Registry URL comes from token file (or default) - No authentication required (public endpoint) **Implementation:** - Added `validateViaAPI()` function that POSTs to `/v0/validate` - Parses `ValidationResult` response from server - Uses shared `printValidationIssues()` for formatting ### 2. `publish` Command: Validation Only on 422 Errors **Before:** Performed upfront local schema validation before publishing. **After:** Attempts publish first, then validates only if server returns 422. **Details:** - Attempts publish immediately (no upfront validation) - If publish returns 422 (Unprocessable Entity): - Calls `/v0/validate` endpoint to get detailed validation errors - Formats and displays them using same logic as `validate` command - Returns error with migration links - For non-422 errors (401, 403, etc.), returns original error (no validation call) **Implementation Changes:** - Removed upfront `runValidationAndPrintIssues()` call - Modified `publishToRegistry()` to return HTTP status code alongside response/error - Added 422 detection logic that calls `validateViaAPI()` ### 3. Shared Validation Error Formatting **Before:** `runValidationAndPrintIssues()` function handled both validation and printing together. **After:** Refactored to separate printing logic. **Details:** - Created `printValidationIssues()` function - just prints validation issues (no validation logic) - Removed `runValidationAndPrintIssues()` function (no longer used) - Both `validate` and `publish` commands use `printValidationIssues()` for consistent output **Behavior:** - Prints schema validation errors with migration guidance - Prints all other validation issues with formatting - Returns formatted error message string for schema issues ## Summary Both CLI commands now use the `/validate` API endpoint for validation instead of local validation. The `publish` command validates only after receiving a 422 error response, and both commands share the same error formatting via `printValidationIssues()`. ### Benefits - **Centralized validation logic** - All validation happens on the server - **Always up-to-date** - CLI always uses current server validation rules - **Consistent experience** - Same validation behavior whether using CLI or API directly - **Better separation of concerns** - Schema validation separated from publisher-specific validation
362 lines
11 KiB
Go
362 lines
11 KiB
Go
package commands_test
|
|
|
|
import (
|
|
"encoding/json"
|
|
"io"
|
|
"net/http"
|
|
"testing"
|
|
|
|
"github.com/modelcontextprotocol/registry/cmd/publisher/commands"
|
|
"github.com/modelcontextprotocol/registry/internal/validators"
|
|
apiv0 "github.com/modelcontextprotocol/registry/pkg/api/v0"
|
|
"github.com/modelcontextprotocol/registry/pkg/model"
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
func TestPublishCommand_Success(t *testing.T) {
|
|
// Setup mock server that returns success
|
|
server := SetupMockRegistryServer(t, nil, nil)
|
|
|
|
// Setup token
|
|
SetupTestToken(t, server.URL, "test-token")
|
|
|
|
// Create valid server.json
|
|
serverJSON := apiv0.ServerJSON{
|
|
Schema: model.CurrentSchemaURL,
|
|
Name: "com.example/test-server",
|
|
Description: "A test server",
|
|
Version: "1.0.0",
|
|
}
|
|
CreateTestServerJSON(t, serverJSON)
|
|
|
|
// Run publish command
|
|
err := commands.PublishCommand([]string{})
|
|
|
|
// Should succeed
|
|
assert.NoError(t, err)
|
|
}
|
|
|
|
func TestPublishCommand_422ValidationFlow(t *testing.T) {
|
|
validateCallCount := 0
|
|
publishCallCount := 0
|
|
|
|
// Setup mock server
|
|
server := SetupMockRegistryServer(t,
|
|
// Publish handler: return 422 for invalid schema
|
|
func(w http.ResponseWriter, _ *http.Request) {
|
|
publishCallCount++
|
|
w.WriteHeader(http.StatusUnprocessableEntity)
|
|
_, _ = w.Write([]byte(`{"message":"Failed to publish server, invalid schema: call /validate for details"}`))
|
|
},
|
|
// Validate handler: return validation errors
|
|
func(w http.ResponseWriter, r *http.Request) {
|
|
validateCallCount++
|
|
w.Header().Set("Content-Type", "application/json")
|
|
|
|
body, _ := io.ReadAll(r.Body)
|
|
var req apiv0.ServerJSON
|
|
_ = json.Unmarshal(body, &req)
|
|
|
|
// Return validation result with deprecated schema error
|
|
result := validators.ValidationResult{
|
|
Valid: false,
|
|
Issues: []validators.ValidationIssue{
|
|
{
|
|
Type: validators.ValidationIssueTypeSemantic,
|
|
Path: "schema",
|
|
Message: "schema version 2025-07-09 is not the current version",
|
|
Severity: validators.ValidationIssueSeverityWarning,
|
|
Reference: "schema-version-deprecated",
|
|
},
|
|
},
|
|
}
|
|
|
|
_ = json.NewEncoder(w).Encode(result)
|
|
},
|
|
)
|
|
|
|
// Setup token
|
|
SetupTestToken(t, server.URL, "test-token")
|
|
|
|
// Create server.json with deprecated schema
|
|
serverJSON := apiv0.ServerJSON{
|
|
Schema: "https://static.modelcontextprotocol.io/schemas/2025-07-09/server.schema.json",
|
|
Name: "com.example/test-server",
|
|
Description: "A test server",
|
|
Version: "1.0.0",
|
|
}
|
|
CreateTestServerJSON(t, serverJSON)
|
|
|
|
// Run publish command
|
|
err := commands.PublishCommand([]string{})
|
|
|
|
// Should fail with validation error
|
|
require.Error(t, err)
|
|
assert.Contains(t, err.Error(), "schema version 2025-07-09")
|
|
assert.Contains(t, err.Error(), "Migration checklist:")
|
|
assert.Contains(t, err.Error(), "Full changelog with examples:")
|
|
|
|
// Verify both endpoints were called
|
|
assert.Equal(t, 1, publishCallCount, "publish endpoint should be called once")
|
|
assert.Equal(t, 1, validateCallCount, "validate endpoint should be called once after 422")
|
|
}
|
|
|
|
func TestPublishCommand_422WithMultipleIssues(t *testing.T) {
|
|
validateCallCount := 0
|
|
|
|
// Setup mock server
|
|
server := SetupMockRegistryServer(t,
|
|
func(w http.ResponseWriter, _ *http.Request) {
|
|
w.WriteHeader(http.StatusUnprocessableEntity)
|
|
_, _ = w.Write([]byte(`{"message":"Failed to publish server, invalid schema"}`))
|
|
},
|
|
func(w http.ResponseWriter, _ *http.Request) {
|
|
validateCallCount++
|
|
w.Header().Set("Content-Type", "application/json")
|
|
|
|
result := validators.ValidationResult{
|
|
Valid: false,
|
|
Issues: []validators.ValidationIssue{
|
|
{
|
|
Type: validators.ValidationIssueTypeSemantic,
|
|
Path: "version",
|
|
Message: "version must be a specific version, not a range",
|
|
Severity: validators.ValidationIssueSeverityError,
|
|
Reference: "semantic-version-range",
|
|
},
|
|
{
|
|
Type: validators.ValidationIssueTypeSchema,
|
|
Path: "name",
|
|
Message: "name is required",
|
|
Severity: validators.ValidationIssueSeverityError,
|
|
Reference: "schema-field-required",
|
|
},
|
|
},
|
|
}
|
|
|
|
_ = json.NewEncoder(w).Encode(result)
|
|
},
|
|
)
|
|
|
|
SetupTestToken(t, server.URL, "test-token")
|
|
|
|
serverJSON := apiv0.ServerJSON{
|
|
Schema: model.CurrentSchemaURL,
|
|
Name: "com.example/test-server",
|
|
Description: "A test server",
|
|
Version: "^1.0.0", // Invalid version range
|
|
}
|
|
CreateTestServerJSON(t, serverJSON)
|
|
|
|
err := commands.PublishCommand([]string{})
|
|
|
|
require.Error(t, err)
|
|
assert.Equal(t, 1, validateCallCount, "validate endpoint should be called")
|
|
}
|
|
|
|
func TestPublishCommand_NoToken(t *testing.T) {
|
|
// Don't create a token file
|
|
serverJSON := apiv0.ServerJSON{
|
|
Schema: model.CurrentSchemaURL,
|
|
Name: "com.example/test-server",
|
|
Description: "A test server",
|
|
Version: "1.0.0",
|
|
}
|
|
CreateTestServerJSON(t, serverJSON)
|
|
|
|
err := commands.PublishCommand([]string{})
|
|
|
|
require.Error(t, err)
|
|
assert.Contains(t, err.Error(), "not authenticated")
|
|
}
|
|
|
|
func TestPublishCommand_Non422Error(t *testing.T) {
|
|
publishCallCount := 0
|
|
|
|
server := SetupMockRegistryServer(t,
|
|
func(w http.ResponseWriter, _ *http.Request) {
|
|
publishCallCount++
|
|
w.WriteHeader(http.StatusUnauthorized)
|
|
_, _ = w.Write([]byte(`{"message":"Unauthorized"}`))
|
|
},
|
|
nil, // No validate handler needed
|
|
)
|
|
|
|
SetupTestToken(t, server.URL, "invalid-token")
|
|
|
|
serverJSON := apiv0.ServerJSON{
|
|
Schema: model.CurrentSchemaURL,
|
|
Name: "com.example/test-server",
|
|
Description: "A test server",
|
|
Version: "1.0.0",
|
|
}
|
|
CreateTestServerJSON(t, serverJSON)
|
|
|
|
err := commands.PublishCommand([]string{})
|
|
|
|
require.Error(t, err)
|
|
assert.Contains(t, err.Error(), "publish failed")
|
|
assert.Equal(t, 1, publishCallCount, "publish endpoint should be called")
|
|
}
|
|
|
|
func TestPublishCommand_DeprecatedSchema(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
schema string
|
|
publishStatus int
|
|
validationOpts func(req apiv0.ServerJSON) validators.ValidationResult
|
|
expectError bool
|
|
errorSubstr string
|
|
checkLinks bool
|
|
}{
|
|
{
|
|
name: "deprecated 2025-07-09 schema should show warning",
|
|
schema: "https://static.modelcontextprotocol.io/schemas/2025-07-09/server.schema.json",
|
|
publishStatus: http.StatusUnprocessableEntity,
|
|
validationOpts: func(_ apiv0.ServerJSON) validators.ValidationResult {
|
|
return validators.ValidationResult{
|
|
Valid: false,
|
|
Issues: []validators.ValidationIssue{
|
|
{
|
|
Type: validators.ValidationIssueTypeSemantic,
|
|
Path: "schema",
|
|
Message: "schema version 2025-07-09 is not the current version",
|
|
Severity: validators.ValidationIssueSeverityWarning,
|
|
Reference: "schema-version-deprecated",
|
|
},
|
|
},
|
|
}
|
|
},
|
|
expectError: true,
|
|
errorSubstr: "schema version 2025-07-09",
|
|
checkLinks: true,
|
|
},
|
|
{
|
|
name: "current 2025-12-11 schema should pass validation",
|
|
schema: "https://static.modelcontextprotocol.io/schemas/2025-12-11/server.schema.json",
|
|
publishStatus: http.StatusCreated,
|
|
validationOpts: func(_ apiv0.ServerJSON) validators.ValidationResult {
|
|
// Should not be called since publish succeeds
|
|
return validators.ValidationResult{Valid: true}
|
|
},
|
|
expectError: false,
|
|
},
|
|
{
|
|
name: "empty schema should fail validation",
|
|
schema: "",
|
|
publishStatus: http.StatusUnprocessableEntity,
|
|
validationOpts: func(_ apiv0.ServerJSON) validators.ValidationResult {
|
|
return validators.ValidationResult{
|
|
Valid: false,
|
|
Issues: []validators.ValidationIssue{
|
|
{
|
|
Type: validators.ValidationIssueTypeSemantic,
|
|
Path: "schema",
|
|
Message: "$schema field is required",
|
|
Severity: validators.ValidationIssueSeverityError,
|
|
Reference: "schema-field-required",
|
|
},
|
|
},
|
|
}
|
|
},
|
|
expectError: true,
|
|
errorSubstr: "$schema field is required",
|
|
checkLinks: true,
|
|
},
|
|
{
|
|
name: "custom schema without valid version should fail validation",
|
|
schema: "https://example.com/custom.schema.json",
|
|
publishStatus: http.StatusUnprocessableEntity,
|
|
validationOpts: func(_ apiv0.ServerJSON) validators.ValidationResult {
|
|
return validators.ValidationResult{
|
|
Valid: false,
|
|
Issues: []validators.ValidationIssue{
|
|
{
|
|
Type: validators.ValidationIssueTypeSchema,
|
|
Path: "schema",
|
|
Message: "failed to extract schema version from URL",
|
|
Severity: validators.ValidationIssueSeverityError,
|
|
Reference: "schema-version-extraction-error",
|
|
},
|
|
},
|
|
}
|
|
},
|
|
expectError: true,
|
|
errorSubstr: "failed to extract schema version from URL",
|
|
checkLinks: true,
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
validateCallCount := 0
|
|
publishCallCount := 0
|
|
|
|
// Setup mock server
|
|
server := SetupMockRegistryServer(t,
|
|
// Publish handler
|
|
func(w http.ResponseWriter, _ *http.Request) {
|
|
publishCallCount++
|
|
if tt.publishStatus == http.StatusCreated {
|
|
w.WriteHeader(http.StatusCreated)
|
|
response := apiv0.ServerResponse{
|
|
Server: apiv0.ServerJSON{
|
|
Name: "com.example/test-server",
|
|
Version: "1.0.0",
|
|
},
|
|
}
|
|
_ = json.NewEncoder(w).Encode(response)
|
|
} else {
|
|
w.WriteHeader(tt.publishStatus)
|
|
_, _ = w.Write([]byte(`{"message":"Failed to publish server, invalid schema: call /validate for details"}`))
|
|
}
|
|
},
|
|
// Validate handler (only called on 422)
|
|
func(w http.ResponseWriter, r *http.Request) {
|
|
validateCallCount++
|
|
w.Header().Set("Content-Type", "application/json")
|
|
|
|
body, _ := io.ReadAll(r.Body)
|
|
var req apiv0.ServerJSON
|
|
_ = json.Unmarshal(body, &req)
|
|
|
|
result := tt.validationOpts(req)
|
|
_ = json.NewEncoder(w).Encode(result)
|
|
},
|
|
)
|
|
|
|
SetupTestToken(t, server.URL, "test-token")
|
|
|
|
// Create server.json with specific schema
|
|
serverJSON := apiv0.ServerJSON{
|
|
Schema: tt.schema,
|
|
Name: "com.example/test-server",
|
|
Description: "A test server",
|
|
Version: "1.0.0",
|
|
}
|
|
CreateTestServerJSON(t, serverJSON)
|
|
|
|
err := commands.PublishCommand([]string{})
|
|
|
|
if tt.expectError {
|
|
require.Error(t, err, "Expected error for test case: %s", tt.name)
|
|
if tt.errorSubstr != "" {
|
|
assert.Contains(t, err.Error(), tt.errorSubstr, "Error should contain expected substring")
|
|
}
|
|
if tt.checkLinks {
|
|
assert.Contains(t, err.Error(), "Migration checklist:", "Error should contain migration checklist link")
|
|
assert.Contains(t, err.Error(), "Full changelog with examples:", "Error should contain changelog link")
|
|
}
|
|
if tt.publishStatus == http.StatusUnprocessableEntity {
|
|
assert.Equal(t, 1, publishCallCount, "publish endpoint should be called once")
|
|
assert.Equal(t, 1, validateCallCount, "validate endpoint should be called after 422")
|
|
}
|
|
} else {
|
|
assert.NoError(t, err, "Expected success for test case: %s", tt.name)
|
|
assert.Equal(t, 1, publishCallCount, "publish endpoint should be called once")
|
|
assert.Equal(t, 0, validateCallCount, "validate endpoint should not be called on success")
|
|
}
|
|
})
|
|
}
|
|
}
|