Visitar URL original
fix(model): preserve explicitly empty argument values through JSON · modelcontextprotocol/registry@1ac3761 · GitHub
Skip to content

Commit 1ac3761

Browse files
rdimitrovrdimitrov
committed
fix(model): preserve explicitly empty argument values through JSON
1 parent 18938f3 commit 1ac3761

5 files changed

Lines changed: 218 additions & 2 deletions

File tree

‎internal/api/handlers/v0/publish_argument_test.go‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
package v0_test
2+
3+
import (
4+
"bytes"
5+
"context"
6+
"crypto/ed25519"
7+
"crypto/rand"
8+
"encoding/hex"
9+
"encoding/json"
10+
"fmt"
11+
"net/http"
12+
"net/http/httptest"
13+
"testing"
14+
15+
"github.com/danielgtaylor/huma/v2"
16+
"github.com/danielgtaylor/huma/v2/adapters/humago"
17+
v0 "github.com/modelcontextprotocol/registry/internal/api/handlers/v0"
18+
"github.com/modelcontextprotocol/registry/internal/auth"
19+
"github.com/modelcontextprotocol/registry/internal/config"
20+
"github.com/modelcontextprotocol/registry/internal/database"
21+
"github.com/modelcontextprotocol/registry/internal/service"
22+
"github.com/modelcontextprotocol/registry/pkg/model"
23+
"github.com/stretchr/testify/require"
24+
)
25+
26+
// Use raw JSON: marshaling a Go string with omitempty would hide the difference
27+
// between a missing value and an explicitly empty CLI argument.
28+
func TestPublishEndpoint_ArgumentPresence(t *testing.T) {
29+
seed := make([]byte, ed25519.SeedSize)
30+
_, err := rand.Read(seed)
31+
require.NoError(t, err)
32+
cfg := &config.Config{JWTPrivateKey: hex.EncodeToString(seed)}
33+
token, err := generateTestJWTToken(cfg, auth.JWTClaims{
34+
AuthMethod: auth.MethodNone,
35+
Permissions: []auth.Permission{
36+
{Action: auth.PermissionActionPublish, ResourcePattern: "example/*"},
37+
},
38+
})
39+
require.NoError(t, err)
40+
41+
for _, prefix := range []string{"/v0", "/v0.1"} {
42+
for _, field := range []string{"runtimeArguments", "packageArguments"} {
43+
t.Run(prefix+"/"+field, func(t *testing.T) {
44+
registry := service.NewRegistryService(database.NewTestDB(t), cfg)
45+
mux := http.NewServeMux()
46+
api := humago.New(mux, huma.DefaultConfig("Test API", "1.0.0"))
47+
v0.RegisterPublishEndpoint(api, prefix, registry, cfg)
48+
49+
for i, tc := range []struct {
50+
name string
51+
arg string
52+
status int
53+
}{
54+
{"empty value", `{"type":"positional","value":""}`, http.StatusOK},
55+
{"empty hint", `{"type":"positional","valueHint":""}`, http.StatusOK},
56+
{"both empty", `{"type":"positional","value":"","valueHint":""}`, http.StatusOK},
57+
{"value", `{"type":"positional","value":"file.txt"}`, http.StatusOK},
58+
{"hint", `{"type":"positional","valueHint":"file"}`, http.StatusOK},
59+
{"named", `{"type":"named","name":"--file","value":""}`, http.StatusOK},
60+
{"missing", `{"type":"positional"}`, http.StatusUnprocessableEntity},
61+
{"null", `{"type":"positional","value":null}`, http.StatusUnprocessableEntity},
62+
{"invalid type", `{"type":"literal","value":"file.txt"}`, http.StatusUnprocessableEntity},
63+
} {
64+
t.Run(tc.name, func(t *testing.T) {
65+
name := fmt.Sprintf("example/argument-%d", i)
66+
body := fmt.Sprintf(`{"$schema":%q,"name":%q,"description":"Argument presence regression test","version":"1.0.0","packages":[{"registryType":"npm","identifier":"test-package","version":"1.0.0","transport":{"type":"stdio"},%q:[%s]}]}`, model.CurrentSchemaURL, name, field, tc.arg)
67+
req := httptest.NewRequest(http.MethodPost, prefix+"/publish", bytes.NewBufferString(body))
68+
req.Header.Set("Content-Type", "application/json")
69+
req.Header.Set("Authorization", "Bearer "+token)
70+
rr := httptest.NewRecorder()
71+
mux.ServeHTTP(rr, req)
72+
require.Equal(t, tc.status, rr.Code, rr.Body.String())
73+
if tc.status != http.StatusOK {
74+
return
75+
}
76+
77+
// Read through the service again to exercise JSONB storage decoding,
78+
// then check that the value survives response serialization too.
79+
stored, err := registry.GetServerByNameAndVersion(context.Background(), name, "1.0.0", false)
80+
require.NoError(t, err)
81+
args := stored.Server.Packages[0].RuntimeArguments
82+
if field == "packageArguments" {
83+
args = stored.Server.Packages[0].PackageArguments
84+
}
85+
require.Len(t, args, 1)
86+
encoded, err := json.Marshal(args[0])
87+
require.NoError(t, err)
88+
require.JSONEq(t, tc.arg, string(encoded))
89+
require.Contains(t, rr.Body.String(), string(encoded))
90+
})
91+
}
92+
})
93+
}
94+
}
95+
}

‎internal/validators/validators.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,8 +430,9 @@ func validateArgument(ctx *ValidationContext, obj *model.Argument) *ValidationRe
430430
valueResult := validateArgumentValueFields(ctx, obj.Name, obj.Value, obj.Default)
431431
result.Merge(valueResult)
432432
case model.ArgumentTypePositional:
433-
// Positional arguments need a value or a valueHint to be usable
434-
if obj.Value == "" && obj.ValueHint == "" {
433+
// The schema requires value or valueHint to be present; an explicitly
434+
// empty value is a valid CLI argument.
435+
if !obj.HasValueOrHint() {
435436
issue := NewValidationIssueFromError(
436437
ValidationIssueTypeSemantic,
437438
ctx.String(),

‎pkg/model/argument.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package model
2+
3+
import "encoding/json"
4+
5+
// argumentJSON avoids calling Argument's JSON methods recursively.
6+
type argumentJSON Argument
7+
8+
// UnmarshalJSON preserves explicitly empty value and valueHint strings. The
9+
// positional argument schema requires property presence, not nonempty strings.
10+
func (a *Argument) UnmarshalJSON(data []byte) error {
11+
var decoded struct {
12+
argumentJSON
13+
Value *string `json:"value"`
14+
ValueHint *string `json:"valueHint"`
15+
}
16+
if err := json.Unmarshal(data, &decoded); err != nil {
17+
return err
18+
}
19+
*a = Argument(decoded.argumentJSON)
20+
a.valuePresent = decoded.Value != nil
21+
a.valueHintPresent = decoded.ValueHint != nil
22+
if decoded.Value != nil {
23+
a.Value = *decoded.Value
24+
}
25+
if decoded.ValueHint != nil {
26+
a.ValueHint = *decoded.ValueHint
27+
}
28+
return nil
29+
}
30+
31+
// MarshalJSON retains explicitly empty strings when persisting or returning an
32+
// argument, while preserving omission for fields that were never supplied.
33+
func (a Argument) MarshalJSON() ([]byte, error) {
34+
var value, valueHint *string
35+
if a.valuePresent || a.Value != "" {
36+
value = &a.Value
37+
}
38+
if a.valueHintPresent || a.ValueHint != "" {
39+
valueHint = &a.ValueHint
40+
}
41+
return json.Marshal(struct {
42+
argumentJSON
43+
Value *string `json:"value,omitempty"`
44+
ValueHint *string `json:"valueHint,omitempty"`
45+
}{argumentJSON(a), value, valueHint})
46+
}
47+
48+
// HasValueOrHint reports whether either positional argument property was
49+
// supplied, including an explicitly empty string decoded from JSON.
50+
func (a Argument) HasValueOrHint() bool {
51+
return a.valuePresent || a.valueHintPresent || a.Value != "" || a.ValueHint != ""
52+
}

‎pkg/model/argument_test.go‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package model_test
2+
3+
import (
4+
"encoding/json"
5+
"testing"
6+
7+
"github.com/modelcontextprotocol/registry/pkg/model"
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
func TestArgumentJSONRoundTrip(t *testing.T) {
12+
for _, tc := range []struct {
13+
name string
14+
input string
15+
hasAny bool
16+
}{
17+
{"missing", `{"type":"positional"}`, false},
18+
{"empty value", `{"type":"positional","value":""}`, true},
19+
{"empty hint", `{"type":"positional","valueHint":""}`, true},
20+
{"both empty", `{"type":"positional","value":"","valueHint":""}`, true},
21+
{"value", `{"type":"positional","value":"input.txt"}`, true},
22+
{"hint", `{"type":"positional","valueHint":"file"}`, true},
23+
{"all fields", `{"type":"named","name":"--file","value":"{file}","valueHint":"file","description":"Input file","isRequired":true,"format":"filepath","isSecret":true,"default":"default.txt","placeholder":"example.txt","choices":["a","b"],"isRepeated":true,"variables":{"file":{"value":"example.txt","isSecret":true}}}`, true},
24+
} {
25+
t.Run(tc.name, func(t *testing.T) {
26+
var arg model.Argument
27+
require.NoError(t, json.Unmarshal([]byte(tc.input), &arg))
28+
require.Equal(t, tc.hasAny, arg.HasValueOrHint())
29+
encoded, err := json.Marshal(arg)
30+
require.NoError(t, err)
31+
require.JSONEq(t, tc.input, string(encoded))
32+
var stored model.Argument
33+
require.NoError(t, json.Unmarshal(encoded, &stored))
34+
require.Equal(t, tc.hasAny, stored.HasValueOrHint())
35+
})
36+
}
37+
}
38+
39+
func TestArgumentJSONMissingAndInvalidValues(t *testing.T) {
40+
var arg model.Argument
41+
for _, input := range []string{
42+
`{"type":"positional","value":1}`,
43+
`{"type":"positional","valueHint":false}`,
44+
} {
45+
require.Error(t, json.Unmarshal([]byte(input), &arg))
46+
}
47+
for _, input := range []string{
48+
`{"type":"positional","value":null}`,
49+
`{"type":"positional","valueHint":null}`,
50+
} {
51+
require.NoError(t, json.Unmarshal([]byte(input), &arg))
52+
require.False(t, arg.HasValueOrHint())
53+
}
54+
}
55+
56+
func TestArgumentJSONFromGoValue(t *testing.T) {
57+
arg := model.Argument{
58+
Type: model.ArgumentTypePositional,
59+
InputWithVariables: model.InputWithVariables{Input: model.Input{Value: "file.txt"}},
60+
}
61+
encoded, err := json.Marshal(arg)
62+
require.NoError(t, err)
63+
require.JSONEq(t, `{"type":"positional","value":"file.txt"}`, string(encoded))
64+
require.True(t, arg.HasValueOrHint())
65+
}

‎pkg/model/types.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,9 @@ type Argument struct {
101101
Name string `json:"name,omitempty" doc:"The flag name (for named arguments), including any leading dashes. Empty for positional arguments." example:"--port"`
102102
ValueHint string `json:"valueHint,omitempty" doc:"An identifier for positional arguments. Used in transport URL variable substitution." example:"file_path"`
103103
IsRepeated bool `json:"isRepeated,omitempty" doc:"Whether the argument can be repeated multiple times."`
104+
105+
valuePresent bool
106+
valueHintPresent bool
104107
}
105108

106109
type Icon struct {

0 commit comments

Comments
 (0)