Make mcpb package version optional and fix schema validation (#686)
<!-- Provide a brief summary of your changes --> ## Motivation and Context <!-- Why is this change needed? What problem does it solve? --> The following PR: * Makes mcpb package version property optional * Bumps the schema and reflects that in the code * Makes sure the publisher tests are being covered by doing `make check` ## How Has This Been Tested? <!-- Have you tested this in a real application? Which scenarios were tested? --> ## Breaking Changes <!-- Will users need to update their code or configurations? --> ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. --> - [ ] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [ ] My code follows the repository's style guidelines - [ ] New and existing tests pass locally - [ ] I have added appropriate error handling - [ ] I have added or updated documentation as needed ## Additional context <!-- Add any other context, implementation notes, or design decisions --> --------- Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
This commit is contained in:
Radoslav Dimitrov
GitHub
parent
f870a4c000
commit
0828bc483c
3 changed files with 131 additions and 81 deletions
|
|
@ -27,14 +27,13 @@ func ValidateMCPB(ctx context.Context, pkg model.Package, _ string) error {
|
|||
return ErrMissingIdentifierForMCPB
|
||||
}
|
||||
|
||||
// Validate that old format fields are not present
|
||||
// MCPB packages use URL in identifier, version is embedded in the URL
|
||||
if pkg.Version != "" {
|
||||
return fmt.Errorf("MCPB packages must not have 'version' field - version should be embedded in the download URL")
|
||||
}
|
||||
// Validate that registryBaseUrl is not present
|
||||
// MCPB packages use full download URLs in identifier
|
||||
if pkg.RegistryBaseURL != "" {
|
||||
return fmt.Errorf("MCPB packages must not have 'registryBaseUrl' field - use the full download URL in 'identifier' instead")
|
||||
}
|
||||
// Note: version field is optional for MCPB packages
|
||||
// It can be included for clarity or omitted if the version is embedded in the download URL
|
||||
|
||||
err := validateMCPBUrl(pkg.Identifier)
|
||||
if err != nil {
|
||||
|
|
|
|||
|
|
@ -120,67 +120,68 @@ func TestValidateMCPB(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestValidateMCPB_RejectsExtraFields(t *testing.T) {
|
||||
func TestValidateMCPB_OptionalFields(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
pkg model.Package
|
||||
expectError bool
|
||||
errorMessage string
|
||||
}{
|
||||
{
|
||||
name: "MCPB package with version field should be rejected",
|
||||
name: "MCPB package with optional version field should pass",
|
||||
pkg: model.Package{
|
||||
RegistryType: model.RegistryTypeMCPB,
|
||||
Identifier: "https://github.com/example/repo/releases/download/v1.0.0/server.mcpb",
|
||||
Version: "1.0.0",
|
||||
FileSHA256: "fe333e598595000ae021bd27117db32ec69af6987f507ba7a63c90638ff633ce",
|
||||
Identifier: "https://github.com/domdomegg/airtable-mcp-server/releases/download/v1.7.2/airtable-mcp-server.mcpb",
|
||||
Version: "1.7.2",
|
||||
FileSHA256: "8220de07a08ebe908f04da139ea03dbfe29758141347e945da60535fb7bcca20",
|
||||
},
|
||||
errorMessage: "MCPB packages must not have 'version' field",
|
||||
expectError: false,
|
||||
},
|
||||
{
|
||||
name: "MCPB package without version field should pass",
|
||||
pkg: model.Package{
|
||||
RegistryType: model.RegistryTypeMCPB,
|
||||
Identifier: "https://github.com/domdomegg/airtable-mcp-server/releases/download/v1.7.2/airtable-mcp-server.mcpb",
|
||||
FileSHA256: "8220de07a08ebe908f04da139ea03dbfe29758141347e945da60535fb7bcca20",
|
||||
},
|
||||
expectError: false,
|
||||
},
|
||||
{
|
||||
name: "MCPB package with registryBaseUrl should be rejected",
|
||||
pkg: model.Package{
|
||||
RegistryType: model.RegistryTypeMCPB,
|
||||
Identifier: "https://github.com/example/repo/releases/download/v1.0.0/server.mcpb",
|
||||
Identifier: "https://github.com/domdomegg/airtable-mcp-server/releases/download/v1.7.2/airtable-mcp-server.mcpb",
|
||||
RegistryBaseURL: "https://github.com",
|
||||
FileSHA256: "fe333e598595000ae021bd27117db32ec69af6987f507ba7a63c90638ff633ce",
|
||||
FileSHA256: "8220de07a08ebe908f04da139ea03dbfe29758141347e945da60535fb7bcca20",
|
||||
},
|
||||
expectError: true,
|
||||
errorMessage: "MCPB packages must not have 'registryBaseUrl' field",
|
||||
},
|
||||
{
|
||||
name: "MCPB package with both extra fields should fail on version first",
|
||||
name: "MCPB package with both version and registryBaseUrl should fail on registryBaseUrl",
|
||||
pkg: model.Package{
|
||||
RegistryType: model.RegistryTypeMCPB,
|
||||
Identifier: "https://github.com/example/repo/releases/download/v1.0.0/server.mcpb",
|
||||
Version: "1.0.0",
|
||||
Identifier: "https://github.com/domdomegg/airtable-mcp-server/releases/download/v1.7.2/airtable-mcp-server.mcpb",
|
||||
Version: "1.7.2",
|
||||
RegistryBaseURL: "https://github.com",
|
||||
FileSHA256: "fe333e598595000ae021bd27117db32ec69af6987f507ba7a63c90638ff633ce",
|
||||
FileSHA256: "8220de07a08ebe908f04da139ea03dbfe29758141347e945da60535fb7bcca20",
|
||||
},
|
||||
errorMessage: "MCPB packages must not have 'version' field",
|
||||
},
|
||||
{
|
||||
name: "MCPB package with canonical format should pass extra field validation",
|
||||
pkg: model.Package{
|
||||
RegistryType: model.RegistryTypeMCPB,
|
||||
Identifier: "https://github.com/domdomegg/airtable-mcp-server/releases/download/v1.7.2/airtable-mcp-server.mcpb",
|
||||
FileSHA256: "fe333e598595000ae021bd27117db32ec69af6987f507ba7a63c90638ff633ce",
|
||||
},
|
||||
errorMessage: "", // Should pass extra field check (will succeed as this is a real package)
|
||||
expectError: true,
|
||||
errorMessage: "MCPB packages must not have 'registryBaseUrl' field",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
err := registries.ValidateMCPB(ctx, tt.pkg, "com.example/test")
|
||||
err := registries.ValidateMCPB(ctx, tt.pkg, "io.github.domdomegg/airtable-mcp-server")
|
||||
|
||||
if tt.errorMessage != "" {
|
||||
if tt.expectError {
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), tt.errorMessage)
|
||||
} else if err != nil {
|
||||
// Should not fail with extra field error (may pass completely for real package)
|
||||
assert.NotContains(t, err.Error(), "must not have 'version'")
|
||||
assert.NotContains(t, err.Error(), "must not have 'registryBaseUrl'")
|
||||
} else {
|
||||
assert.NoError(t, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -47,7 +47,7 @@ func TestValidate(t *testing.T) {
|
|||
expectedError: "schema version https://static.modelcontextprotocol.io/schemas/2025-01-27/server.schema.json is not supported",
|
||||
},
|
||||
{
|
||||
name: "Schema version accepts current schema (2025-09-29)",
|
||||
name: "Schema version accepts current schema (2025-10-17)",
|
||||
serverDetail: apiv0.ServerJSON{
|
||||
Schema: model.CurrentSchemaURL,
|
||||
Name: "com.example/test-server",
|
||||
|
|
|
|||
Loading…
Reference in a new issue