Skip to content

Commit 1bf66e2

Browse files
committed
Update Models
* Drop args/env from Package.Installations * Drop args.env from Mozilla-ai's Server.Installations * Drop args/env from MCPM registry server installations * Update tests
1 parent a9268fc commit 1bf66e2

File tree

10 files changed

+197
-154
lines changed

10 files changed

+197
-154
lines changed

internal/packages/installation.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,13 @@ import "github.com/mozilla-ai/mcpd/v2/internal/runtime"
55
type Installations map[runtime.Runtime]Installation
66

77
type Installation struct {
8-
Command string `json:"command"`
9-
Args []string `json:"args"`
10-
Package string `json:"package,omitempty"`
11-
Version string `json:"version,omitempty"`
12-
Env map[string]string `json:"env,omitempty"`
13-
Description string `json:"description,omitempty"`
14-
Recommended bool `json:"recommended,omitempty"`
15-
Deprecated bool `json:"deprecated,omitempty"`
16-
Transports []Transport `json:"transports,omitempty"`
8+
Command string `json:"command"`
9+
Package string `json:"package,omitempty"`
10+
Version string `json:"version,omitempty"`
11+
Description string `json:"description,omitempty"`
12+
Recommended bool `json:"recommended,omitempty"`
13+
Deprecated bool `json:"deprecated,omitempty"`
14+
Transports []Transport `json:"transports,omitempty"`
1715
}
1816

1917
// AnyDeprecated can be used to determine if any of the installations are deprecated.

internal/provider/mcpm/registry.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -423,9 +423,7 @@ func convertInstallations(
423423

424424
details[rt] = packages.Installation{
425425
Command: install.Command,
426-
Args: slices.Clone(install.Args),
427426
Package: pkg,
428-
Env: maps.Clone(install.Env),
429427
Description: install.Description,
430428
Recommended: install.Recommended,
431429
Deprecated: false, // MCPM doesn't support deprecated installations

internal/provider/mozilla_ai/model.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -81,18 +81,12 @@ type Installation struct {
8181
// Runtime specifies the runtime type for this installation method.
8282
Runtime Runtime `json:"runtime"`
8383

84-
// Args contains command-line arguments for the server.
85-
Args []string `json:"args,omitempty"`
86-
87-
// Package is the package name for package manager installations.
84+
// Package is the package name that will be executed.
8885
Package string `json:"package,omitempty"`
8986

9087
// Version specifies the version for this installation method.
9188
Version string `json:"version"`
9289

93-
// Env defines environment variables required for the server.
94-
Env map[string]string `json:"env,omitempty"`
95-
9690
// Description provides additional details about this installation method.
9791
Description string `json:"description,omitempty"`
9892

internal/provider/mozilla_ai/registry.go

Lines changed: 2 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -211,16 +211,6 @@ func (r *Registry) buildPackageResult(pkgKey string) (packages.Package, bool) {
211211
return packages.Package{}, false
212212
}
213213

214-
runtimesAndPackages, err := r.supportedRuntimePackageNames(sd.Installations)
215-
if err != nil || len(runtimesAndPackages) == 0 {
216-
r.logger.Debug(
217-
"no supported runtime packages found in registry",
218-
"pkgKey", pkgKey,
219-
"error", err,
220-
)
221-
return packages.Package{}, false
222-
}
223-
224214
tools, err := sd.Tools.ToDomainType()
225215
if err != nil {
226216
r.logger.Error(
@@ -270,37 +260,6 @@ func (r *Registry) buildPackageResult(pkgKey string) (packages.Package, bool) {
270260
}, true
271261
}
272262

273-
// supportedRuntimePackageNames extracts runtime-specific package names for a given MCP server.
274-
func (r *Registry) supportedRuntimePackageNames(
275-
installations map[string]Installation,
276-
) (map[runtime.Runtime]string, error) {
277-
result := make(map[runtime.Runtime]string)
278-
279-
specs := runtime.Specs()
280-
281-
for _, inst := range installations {
282-
rt := runtime.Runtime(inst.Runtime)
283-
if _, ok := r.supportedRuntimes[rt]; !ok {
284-
continue
285-
}
286-
287-
spec, ok := specs[rt]
288-
if !ok || spec.ExtractPackageName == nil {
289-
r.logger.Debug("no package extractor for runtime", "runtime", rt)
290-
continue
291-
}
292-
293-
pkg, err := spec.ExtractPackageName(inst.Args)
294-
if err != nil {
295-
return nil, fmt.Errorf("failed to extract package for runtime %q: %w", rt, err)
296-
}
297-
298-
result[rt] = pkg
299-
}
300-
301-
return result, nil
302-
}
303-
304263
func convertInstallations(
305264
src map[string]Installation,
306265
supported map[runtime.Runtime]struct{},
@@ -309,7 +268,6 @@ func convertInstallations(
309268
return nil
310269
}
311270

312-
specs := runtime.Specs()
313271
details := make(packages.Installations, len(src))
314272

315273
for _, install := range src {
@@ -318,23 +276,14 @@ func convertInstallations(
318276
continue
319277
}
320278

321-
pkg := ""
322-
if spec, ok := specs[rt]; ok && spec.ExtractPackageName != nil {
323-
if packageName, err := spec.ExtractPackageName(install.Args); err == nil {
324-
pkg = packageName
325-
}
326-
}
327-
328279
details[rt] = packages.Installation{
329280
Command: string(install.Runtime),
330-
Args: slices.Clone(install.Args),
331-
Package: pkg,
281+
Package: install.Package,
332282
Version: install.Version,
333-
Env: maps.Clone(install.Env),
334283
Description: install.Description,
335284
Recommended: install.Recommended,
336285
Deprecated: install.Deprecated,
337-
Transports: packages.DefaultTransports(), // mozilla-ai defaults to stdio, could be extended per-installation later
286+
Transports: packages.DefaultTransports(), // TODO: mozilla-ai defaults to stdio, could be extended per-installation later
338287
}
339288
}
340289

internal/provider/mozilla_ai/registry_test.go

Lines changed: 92 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -42,32 +42,6 @@ func newTestLogger(t *testing.T) hclog.Logger {
4242
})
4343
}
4444

45-
// extractTestEnvVarNames extracts environment variable names from Arguments for testing.
46-
func extractTestEnvVarNames(t *testing.T, args packages.Arguments) []string {
47-
t.Helper()
48-
49-
var envVars []string
50-
for key, arg := range args {
51-
if arg.VariableType == packages.VariableTypeEnv {
52-
envVars = append(envVars, key)
53-
}
54-
}
55-
return envVars
56-
}
57-
58-
// extractTestCLIArgNames extracts CLI argument names from Arguments for testing.
59-
func extractTestCLIArgNames(t *testing.T, args packages.Arguments) []string {
60-
t.Helper()
61-
62-
var cliArgs []string
63-
for key, arg := range args {
64-
if arg.VariableType == packages.VariableTypeArg || arg.VariableType == packages.VariableTypeArgBool {
65-
cliArgs = append(cliArgs, key)
66-
}
67-
}
68-
return cliArgs
69-
}
70-
7145
func TestRegistry_ID(t *testing.T) {
7246
t.Parallel()
7347

@@ -456,69 +430,64 @@ func TestRegistry_Arguments_ToDomainType_EdgeCases(t *testing.T) {
456430
}
457431

458432
func TestRegistry_Arguments_ToDomainType_WithTestdata(t *testing.T) {
433+
t.Parallel()
434+
459435
testCases := []struct {
460436
name string
461437
testdataFile string
462438
serverName string
463-
description string
464439
expectedEnv []string
465440
expectedArgs []string
466441
}{
467442
{
468443
name: "environment variables only",
469444
testdataFile: "arg_test_env_only.json",
470445
serverName: "env-only-server",
471-
description: "Server using only environment variables should extract env vars correctly",
472446
expectedEnv: []string{"API_KEY", "DEBUG_MODE"},
473-
expectedArgs: []string{},
474447
},
475448
{
476449
name: "command-line arguments only",
477450
testdataFile: "arg_test_cli_only.json",
478451
serverName: "cli-only-server",
479-
description: "Server using only CLI arguments should extract args correctly",
480-
expectedEnv: []string{},
481-
expectedArgs: []string{"OUTPUT_FORMAT", "PORT", "ENABLE_CORS"},
452+
expectedArgs: []string{"--output-format", "--port", "--enable-cors"},
482453
},
483454
{
484455
name: "mixed environment and arguments",
485456
testdataFile: "arg_test_mixed_args.json",
486457
serverName: "mixed-args-server",
487-
description: "Server with mixed argument types should classify each correctly",
488458
expectedEnv: []string{"DATABASE_URL"},
489-
expectedArgs: []string{"CONFIG_PATH", "VERBOSE"},
459+
expectedArgs: []string{"--config-path", "--verbose"},
490460
},
491461
}
492462

493463
for _, tc := range testCases {
494464
t.Run(tc.name, func(t *testing.T) {
495-
t.Logf("Test description: %s", tc.description)
465+
t.Parallel()
496466

497-
// Load testdata registry
498467
registry := loadTestDataRegistry(t, tc.testdataFile)
499-
500-
// Get the specific server we're testing
501468
server, exists := registry[tc.serverName]
502-
require.True(t, exists, "Server %q not found in testdata file %q", tc.serverName, tc.testdataFile)
469+
require.True(t, exists)
503470

504-
// Convert arguments using Mozilla AI's ToDomainType method
505471
result, err := server.Arguments.ToDomainType()
506-
require.NoError(t, err, "Should convert arguments successfully")
472+
require.NoError(t, err)
507473

508474
// Verify expected environment variables
509-
envVars := extractTestEnvVarNames(t, result)
510-
require.ElementsMatch(t, tc.expectedEnv, envVars,
511-
"Unexpected environment variables for %s", tc.serverName)
475+
envVars := result.FilterBy(packages.EnvVar).Names()
476+
require.ElementsMatch(t, tc.expectedEnv, envVars)
477+
for _, key := range tc.expectedEnv {
478+
require.Equal(t, key, result[key].Name)
479+
}
512480

513481
// Verify expected CLI arguments
514-
cliArgs := extractTestCLIArgNames(t, result)
515-
require.ElementsMatch(t, tc.expectedArgs, cliArgs,
516-
"Unexpected CLI arguments for %s", tc.serverName)
482+
cliArgs := result.FilterBy(packages.Argument).Names()
483+
require.ElementsMatch(t, tc.expectedArgs, cliArgs)
484+
for _, key := range tc.expectedArgs {
485+
require.Equal(t, key, result[key].Name)
486+
}
517487

518488
// Verify argument count matches expectations
519489
expectedTotal := len(tc.expectedEnv) + len(tc.expectedArgs)
520-
require.Len(t, result, expectedTotal,
521-
"Total argument count mismatch for %s", tc.serverName)
490+
require.Len(t, result, expectedTotal)
522491
})
523492
}
524493
}
@@ -559,8 +528,7 @@ func TestRegistry_BuildPackageResult_ArgumentExtraction(t *testing.T) {
559528
Installations: map[string]Installation{
560529
"npx": {
561530
Runtime: NPX,
562-
Args: []string{"test-server", "--config", "${CONFIG_FILE}", "--debug"},
563-
Env: map[string]string{"API_TOKEN": "${API_TOKEN}"},
531+
Package: "test-server",
564532
Version: "1.0.0",
565533
Description: "Run with npx",
566534
Recommended: true,
@@ -595,11 +563,11 @@ func TestRegistry_BuildPackageResult_ArgumentExtraction(t *testing.T) {
595563
require.Len(t, pkg.Arguments, 3, "Should have 3 arguments")
596564

597565
// Check environment variable
598-
envVars := extractTestEnvVarNames(t, pkg.Arguments)
566+
envVars := pkg.Arguments.FilterBy(packages.EnvVar).Names()
599567
require.ElementsMatch(t, []string{"API_TOKEN"}, envVars, "Should extract environment variable")
600568

601569
// Check CLI arguments
602-
cliArgs := extractTestCLIArgNames(t, pkg.Arguments)
570+
cliArgs := pkg.Arguments.FilterBy(packages.Argument).Names()
603571
require.ElementsMatch(t, []string{"CONFIG_FILE", "DEBUG"}, cliArgs, "Should extract CLI arguments")
604572

605573
// Verify argument details
@@ -626,3 +594,74 @@ func TestRegistry_BuildPackageResult_ArgumentExtraction(t *testing.T) {
626594
require.False(t, debug.Required)
627595
require.Equal(t, "Enable debug mode", debug.Description)
628596
}
597+
598+
func TestRegistry_BuildPackageResult_WithOptionalArguments(t *testing.T) {
599+
t.Parallel()
600+
601+
testRegistry := loadTestDataRegistry(t, "registry_optional_args.json")
602+
603+
tests := []struct {
604+
name string
605+
serverID string
606+
checkFunc func(t *testing.T, pkg packages.Package)
607+
}{
608+
{
609+
name: "time server with optional timezone argument",
610+
serverID: "time",
611+
checkFunc: func(t *testing.T, pkg packages.Package) {
612+
require.Equal(t, "time", pkg.Name)
613+
require.Equal(t, "Time Server", pkg.DisplayName)
614+
615+
// Check optional argument
616+
tzArg, exists := pkg.Arguments["--local-timezone"]
617+
require.True(t, exists)
618+
require.False(t, tzArg.Required)
619+
require.Equal(t, packages.VariableType("argument"), tzArg.VariableType)
620+
require.Equal(t, "America/New_York", tzArg.Example)
621+
622+
// Check installation
623+
uvxInstall, exists := pkg.Installations[runtime.UVX]
624+
require.True(t, exists, "Should have UVX installation")
625+
require.Equal(t, "mcp-server-time", uvxInstall.Package)
626+
},
627+
},
628+
{
629+
name: "sqlite with required argument and package field",
630+
serverID: "sqlite-with-package",
631+
checkFunc: func(t *testing.T, pkg packages.Package) {
632+
require.Equal(t, "sqlite-with-package", pkg.Name)
633+
634+
// Check required argument
635+
dbArg, exists := pkg.Arguments["--db-path"]
636+
require.True(t, exists)
637+
require.True(t, dbArg.Required)
638+
639+
// Check installation with package field
640+
uvxInstall, exists := pkg.Installations[runtime.UVX]
641+
require.True(t, exists, "Should have UVX installation")
642+
require.Equal(t, "mcp-server-sqlite", uvxInstall.Package)
643+
},
644+
},
645+
}
646+
647+
for _, tc := range tests {
648+
t.Run(tc.name, func(t *testing.T) {
649+
t.Parallel()
650+
651+
logger := newTestLogger(t)
652+
registry := &Registry{
653+
mcpServers: testRegistry,
654+
logger: logger,
655+
supportedRuntimes: map[runtime.Runtime]struct{}{
656+
runtime.UVX: {},
657+
runtime.NPX: {},
658+
},
659+
filterOptions: []options.Option{},
660+
}
661+
662+
pkg, ok := registry.buildPackageResult(tc.serverID)
663+
require.True(t, ok, "Should successfully build package result")
664+
tc.checkFunc(t, pkg)
665+
})
666+
}
667+
}

0 commit comments

Comments
 (0)