Skip to content

Commit 04eb752

Browse files
peteski22peteski22
authored
General refactor (1) (#130)
* Add cmd: Don't add 'version' if it's 'latest' * Don't include version => latest in default resolve options (add) * Package: Rename InstallationDetails => Installations * Package: Add Example to ArgumentMetadata * Update text - entry printer (prints results after 'add') * Update text - package printer
1 parent 1e4e1a7 commit 04eb752

File tree

15 files changed

+407
-144
lines changed

15 files changed

+407
-144
lines changed

‎cmd/add.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ func parseServerEntry(
219219
return config.ServerEntry{}, fmt.Errorf("error matching requested tools: %w", err)
220220
}
221221

222-
selectedRuntime, runtimeErr := selectRuntime(pkg.InstallationDetails, requestedRuntime, supportedRuntimes)
222+
selectedRuntime, runtimeErr := selectRuntime(pkg.Installations, requestedRuntime, supportedRuntimes)
223223
if runtimeErr != nil {
224224
return config.ServerEntry{}, fmt.Errorf("error selecting runtime from available installations: %w", runtimeErr)
225225
}
@@ -229,7 +229,7 @@ func parseServerEntry(
229229
v = pkg.Version
230230
}
231231

232-
runtimeSpecificName := pkg.InstallationDetails[selectedRuntime].Package
232+
runtimeSpecificName := pkg.Installations[selectedRuntime].Package
233233
if runtimeSpecificName == "" {
234234
return config.ServerEntry{}, fmt.Errorf(
235235
"installation package name is missing for runtime '%s'",
@@ -255,7 +255,7 @@ func parseServerEntry(
255255
func (c *AddCmd) options() []regopts.ResolveOption {
256256
var o []regopts.ResolveOption
257257

258-
if c.Version != "" {
258+
if c.Version != "" && c.Version != "latest" {
259259
o = append(o, regopts.WithResolveVersion(c.Version))
260260
}
261261
if c.Runtime != "" {

‎cmd/add_test.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func TestAddCmd_Success(t *testing.T) {
8282
{Name: "toolB"},
8383
},
8484
Version: "1.2.3",
85-
InstallationDetails: map[runtime.Runtime]packages.Installation{
85+
Installations: map[runtime.Runtime]packages.Installation{
8686
runtime.UVX: {
8787
Command: "uvx",
8888
Package: "mcp-server-1",
@@ -150,7 +150,7 @@ func TestAddCmd_BasicServerAdd(t *testing.T) {
150150
{Name: "tool2"},
151151
{Name: "tool3"},
152152
},
153-
InstallationDetails: map[runtime.Runtime]packages.Installation{
153+
Installations: map[runtime.Runtime]packages.Installation{
154154
"uvx": {
155155
Command: "uvx",
156156
Package: "mcp-server-testserver",
@@ -207,7 +207,7 @@ func TestAddCmd_ServerWithArguments(t *testing.T) {
207207
{Name: "create_repo"},
208208
{Name: "list_repos"},
209209
},
210-
InstallationDetails: map[runtime.Runtime]packages.Installation{
210+
Installations: map[runtime.Runtime]packages.Installation{
211211
runtime.UVX: {
212212
Command: "uvx",
213213
Package: "mcp-server-github",
@@ -236,7 +236,7 @@ func TestAddCmd_ServerWithArguments(t *testing.T) {
236236
Tools: []packages.Tool{
237237
{Name: "query"},
238238
},
239-
InstallationDetails: map[runtime.Runtime]packages.Installation{
239+
Installations: map[runtime.Runtime]packages.Installation{
240240
runtime.UVX: {
241241
Command: "uvx",
242242
Package: "mcp-server-db",
@@ -261,7 +261,7 @@ func TestAddCmd_ServerWithArguments(t *testing.T) {
261261
Tools: []packages.Tool{
262262
{Name: "call_api"},
263263
},
264-
InstallationDetails: map[runtime.Runtime]packages.Installation{
264+
Installations: map[runtime.Runtime]packages.Installation{
265265
runtime.UVX: {
266266
Command: "uvx",
267267
Package: "mcp-server-api",
@@ -288,7 +288,7 @@ func TestAddCmd_ServerWithArguments(t *testing.T) {
288288
Tools: []packages.Tool{
289289
{Name: "hello"},
290290
},
291-
InstallationDetails: map[runtime.Runtime]packages.Installation{
291+
Installations: map[runtime.Runtime]packages.Installation{
292292
runtime.UVX: {
293293
Command: "uvx",
294294
Package: "mcp-server-simple",
@@ -619,11 +619,11 @@ func TestParseServerEntry(t *testing.T) {
619619
}
620620

621621
pkg := packages.Package{
622-
ID: tc.pkgID,
623-
Name: tc.pkgName,
624-
Tools: tools,
625-
InstallationDetails: tc.installations,
626-
Arguments: tc.arguments,
622+
ID: tc.pkgID,
623+
Name: tc.pkgName,
624+
Tools: tools,
625+
Installations: tc.installations,
626+
Arguments: tc.arguments,
627627
}
628628

629629
entry, err := parseServerEntry(pkg, tc.requestedRuntime, tc.requestedTools, tc.supportedRuntimes)

‎internal/context/context.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ func (c *ExecutionContextConfig) List() []ServerExecutionContext {
9696
servers := slices.Collect(maps.Values(c.Servers))
9797

9898
slices.SortFunc(servers, func(a, b ServerExecutionContext) int {
99-
return cmp.Compare(a.Name, b.Name)
99+
return cmp.Compare(strings.ToLower(a.Name), strings.ToLower(b.Name))
100100
})
101101

102102
return servers

‎internal/packages/argument.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package packages
22

33
import (
4+
"cmp"
45
"maps"
56
"regexp"
67
"slices"
8+
"strings"
79
)
810

911
const (
@@ -33,6 +35,7 @@ type ArgumentMetadata struct {
3335
Name string `json:"name"`
3436
Description string `json:"description"`
3537
Required bool `json:"required"`
38+
Example string `json:"example"`
3639
VariableType VariableType `json:"type"`
3740
Position *int `json:"position,omitempty"` // Position in args array for positional arguments
3841
}
@@ -72,12 +75,7 @@ func (a Arguments) Ordered() []ArgumentMetadata {
7275

7376
// Sort others alphabetically by name
7477
slices.SortFunc(others, func(a, b ArgumentMetadata) int {
75-
if a.Name < b.Name {
76-
return -1
77-
} else if a.Name > b.Name {
78-
return 1
79-
}
80-
return 0
78+
return cmp.Compare(strings.ToLower(a.Name), strings.ToLower(b.Name))
8179
})
8280

8381
// Combine: positional first, then others

‎internal/packages/argument_test.go‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,38 +13,52 @@ func testArguments(t *testing.T) Arguments {
1313

1414
return Arguments{
1515
"DATABASE_URL": {
16+
Name: "DATABASE_URL",
1617
Description: "Database connection URL",
1718
Required: true,
19+
Example: "postgresql://localhost:5432/mydb",
1820
VariableType: VariableTypeEnv,
1921
},
2022
"API_KEY": {
23+
Name: "API_KEY",
2124
Description: "API key for external service",
2225
Required: true,
26+
Example: "sk-1234567890abcdef",
2327
VariableType: VariableTypeEnv,
2428
},
2529
"DEBUG_MODE": {
30+
Name: "DEBUG_MODE",
2631
Description: "Enable debug mode",
2732
Required: false,
33+
Example: "true",
2834
VariableType: VariableTypeEnv,
2935
},
3036
"OPTIONAL_CONFIG": {
37+
Name: "OPTIONAL_CONFIG",
3138
Description: "",
3239
Required: false,
40+
Example: "",
3341
VariableType: VariableTypeEnv,
3442
},
3543
"--port": {
44+
Name: "--port",
3645
Description: "Port to listen on",
3746
Required: true,
47+
Example: "8080",
3848
VariableType: VariableTypeArg,
3949
},
4050
"--verbose": {
51+
Name: "--verbose",
4152
Description: "Enable verbose output",
4253
Required: false,
54+
Example: "true",
4355
VariableType: VariableTypeArg,
4456
},
4557
"--config": {
58+
Name: "--config",
4659
Description: "",
4760
Required: true,
61+
Example: "/path/to/config.json",
4862
VariableType: VariableTypeArg,
4963
},
5064
}
@@ -457,3 +471,149 @@ func TestFilterArguments_EdgeEases(t *testing.T) {
457471
})
458472
}
459473
}
474+
475+
func TestArgumentMetadata_NameAndExample(t *testing.T) {
476+
t.Parallel()
477+
478+
tests := []struct {
479+
name string
480+
metadata ArgumentMetadata
481+
expected ArgumentMetadata
482+
}{
483+
{
484+
name: "all fields set",
485+
metadata: ArgumentMetadata{
486+
Name: "TEST_VAR",
487+
Description: "Test variable",
488+
Required: true,
489+
Example: "example_value",
490+
VariableType: VariableTypeEnv,
491+
},
492+
expected: ArgumentMetadata{
493+
Name: "TEST_VAR",
494+
Description: "Test variable",
495+
Required: true,
496+
Example: "example_value",
497+
VariableType: VariableTypeEnv,
498+
},
499+
},
500+
{
501+
name: "empty name and example",
502+
metadata: ArgumentMetadata{
503+
Name: "",
504+
Description: "Test variable",
505+
Required: false,
506+
Example: "",
507+
VariableType: VariableTypeArg,
508+
},
509+
expected: ArgumentMetadata{
510+
Name: "",
511+
Description: "Test variable",
512+
Required: false,
513+
Example: "",
514+
VariableType: VariableTypeArg,
515+
},
516+
},
517+
{
518+
name: "positional argument with position",
519+
metadata: ArgumentMetadata{
520+
Name: "POS_ARG",
521+
Description: "Positional argument",
522+
Required: true,
523+
Example: "/path/to/file",
524+
VariableType: VariableTypePositionalArg,
525+
Position: &[]int{1}[0],
526+
},
527+
expected: ArgumentMetadata{
528+
Name: "POS_ARG",
529+
Description: "Positional argument",
530+
Required: true,
531+
Example: "/path/to/file",
532+
VariableType: VariableTypePositionalArg,
533+
Position: &[]int{1}[0],
534+
},
535+
},
536+
}
537+
538+
for _, tc := range tests {
539+
t.Run(tc.name, func(t *testing.T) {
540+
t.Parallel()
541+
542+
require.Equal(t, tc.expected.Name, tc.metadata.Name)
543+
require.Equal(t, tc.expected.Example, tc.metadata.Example)
544+
require.Equal(t, tc.expected.Description, tc.metadata.Description)
545+
require.Equal(t, tc.expected.Required, tc.metadata.Required)
546+
require.Equal(t, tc.expected.VariableType, tc.metadata.VariableType)
547+
if tc.expected.Position != nil {
548+
require.NotNil(t, tc.metadata.Position)
549+
require.Equal(t, *tc.expected.Position, *tc.metadata.Position)
550+
} else {
551+
require.Nil(t, tc.metadata.Position)
552+
}
553+
})
554+
}
555+
}
556+
557+
func TestArguments_Ordered_NameSetting(t *testing.T) {
558+
t.Parallel()
559+
560+
args := Arguments{
561+
"ENV_VAR": {
562+
Description: "Environment variable",
563+
Required: true,
564+
Example: "env_example",
565+
VariableType: VariableTypeEnv,
566+
},
567+
"POS_ARG": {
568+
Description: "Positional argument",
569+
Required: true,
570+
Example: "pos_example",
571+
VariableType: VariableTypePositionalArg,
572+
Position: &[]int{1}[0],
573+
},
574+
"--flag": {
575+
Description: "Command flag",
576+
Required: false,
577+
Example: "flag_example",
578+
VariableType: VariableTypeArg,
579+
},
580+
}
581+
582+
ordered := args.Ordered()
583+
require.Len(t, ordered, 3)
584+
585+
for _, arg := range ordered {
586+
require.NotEmpty(t, arg.Name, "Name should be set on ordered arguments")
587+
588+
switch arg.Name {
589+
case "POS_ARG":
590+
require.Equal(t, "pos_example", arg.Example)
591+
require.Equal(t, VariableTypePositionalArg, arg.VariableType)
592+
require.NotNil(t, arg.Position)
593+
require.Equal(t, 1, *arg.Position)
594+
case "ENV_VAR":
595+
require.Equal(t, "env_example", arg.Example)
596+
require.Equal(t, VariableTypeEnv, arg.VariableType)
597+
case "--flag":
598+
require.Equal(t, "flag_example", arg.Example)
599+
require.Equal(t, VariableTypeArg, arg.VariableType)
600+
default:
601+
t.Errorf("Unexpected argument name: %s", arg.Name)
602+
}
603+
}
604+
605+
require.Equal(t, "POS_ARG", ordered[0].Name, "Positional argument should be first")
606+
607+
require.Contains(
608+
t,
609+
[]string{"--flag", "ENV_VAR"},
610+
ordered[1].Name,
611+
"Non-positional should be in alphabetical order",
612+
)
613+
require.Contains(
614+
t,
615+
[]string{"--flag", "ENV_VAR"},
616+
ordered[2].Name,
617+
"Non-positional should be in alphabetical order",
618+
)
619+
}

‎internal/packages/installation.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,37 @@
11
package packages
22

3+
import "github.com/mozilla-ai/mcpd/v2/internal/runtime"
4+
5+
type Installations map[runtime.Runtime]Installation
6+
37
type Installation struct {
48
Command string `json:"command"`
59
Args []string `json:"args"`
610
Package string `json:"package,omitempty"`
711
Env map[string]string `json:"env,omitempty"`
812
Description string `json:"description,omitempty"`
913
Recommended bool `json:"recommended,omitempty"`
14+
Deprecated bool `json:"deprecated,omitempty"`
15+
}
16+
17+
// AnyDeprecated can be used to determine if any of the installations are deprecated.
18+
func (i Installations) AnyDeprecated() bool {
19+
for _, installation := range i {
20+
if installation.Deprecated {
21+
return true
22+
}
23+
}
24+
25+
return false
26+
}
27+
28+
// AllDeprecated can be used to determine if all the installations are deprecated.
29+
func (i Installations) AllDeprecated() bool {
30+
for _, installation := range i {
31+
if !installation.Deprecated {
32+
return false
33+
}
34+
}
35+
36+
return true
1037
}

0 commit comments

Comments
 (0)