8000 fix(protocol)!: preserve elicitation metadata numbers by dominicnunez · Pull Request #317 · dominicnunez/codex-sdk-go · GitHub
[go: up one dir, main page]

Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions appserver/protocol/approval_additional.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ func (p *McpServerElicitationRequestParams) UnmarshalJSON(data []byte) error {
type elicitationWire struct {
*wire
RequestedSchema *json.RawMessage `json:"requestedSchema"`
Meta exactJSONValue `json:"_meta"`
}
w := elicitationWire{wire: &decoded, RequestedSchema: &schema}
required := []string{"serverName", "threadId"}
Expand Down Expand Up @@ -210,6 +211,7 @@ func (p *McpServerElicitationRequestParams) UnmarshalJSON(data []byte) error {
if err := validateEnumOccurrences(data, "mode", false, validateElicitationMode); err != nil {
return err
}
decoded.Meta = w.Meta.value
*p = McpServerElicitationRequestParams(decoded)
return nil
}
Expand Down
153 changes: 153 additions & 0 deletions appserver/protocol/elicitation_metadata_contract_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
package protocol_test

import (
"context"
"encoding/json"
"errors"
"fmt"
"reflect"
"strconv"
"strings"
"testing"

p "github.com/dominicnunez/codex-sdk-go/appserver/protocol"
)

var elicitationMetadataModes = []p.McpServerElicitationMode{
p.McpServerElicitationModeForm, p.McpServerElicitationModeURL,
p.McpServerElicitationModeOpenAIForm, p.McpServerElicitationModeOpenAIFormLegacy,
}

func TestElicitationMetadataSelection(t *testing.T) {
for _, mode := range elicitationMetadataModes {
for _, tc := range []struct{ members, want string }{
{"", "null"},
{`,"_meta":null`, "null"},
{`,"_META":42`, "null"},
{`,"_m\u0065ta":9007199254740993`, "9007199254740993"},
{`,"_meta":{"old":1},"_meta":{"new":2}`, `{"new":2}`},
{`,"_meta":[1],"_meta":null`, "null"},
{`,"_meta":null,"_meta":[-0,1e400]`, "[-0,1e400]"},
{`,"_meta":{"n":1,"n":2}`, `{"n":2}`},
} {
t.Run(string(mode)+tc.members, func(t *testing.T) {
var params p.McpServerElicitationRequestParams
seed := elicitationModeBody(fmt.Sprintf(`"mode":%q,"_meta":{"old":[1]}`, mode))
if err := json.Unmarshal([]byte(seed), &params); err != nil {
t.Fatal(err)
}
body := []byte(elicitationModeBody(fmt.Sprintf(`"mode":%q`, mode) + tc.members))
if err := json.Unmarshal(body, &params); err != nil {
t.Fatal(err)
}
for i := range body {
body[i] = ' '
}
got, err := json.Marshal(params.Meta)
if err != nil || string(got) != tc.want {
t.Fatalf("selection or ownership changed: %s / %s: %v", got, tc.want, err)
}
})
}
}
}

func TestElicitationMetadataAdmission(t *testing.T) {
for _, mode := range elicitationMetadataModes {
t.Run(string(mode), func(t *testing.T) {
seed := elicitationModeBody(fmt.Sprintf(`"mode":%q,"_meta":{"n":[9007199254740993]}`, mode))
var params p.McpServerElicitationRequestParams
if err := json.Unmarshal([]byte(seed), &params); err != nil {
t.Fatal(err)
}
before := params
bad := elicitationModeBody(fmt.Sprintf(`"mode":"future","mode":%q,"_meta":{"replacement":1e400}`, mode))
if err := json.Unmarshal([]byte(bad), &params); err == nil {
t.Fatal("invalid mode accepted")
}
if !reflect.DeepEqual(params, before) || reflect.ValueOf(params.Meta).Pointer() != reflect.ValueOf(before.Meta).Pointer() {
t.Fatal("failed admission changed retained metadata")
}
mock := NewMockTransport()
client := p.NewClient(mock)
defer client.Close()
calls := 0
client.SetApprovalHandlers(p.ApprovalHandlers{OnMcpServerElicitationRequest: func(_ context.Context, got p.McpServerElicitationRequestParams) (p.McpServerElicitationRequestResponse, error) {
calls++
if !reflect.DeepEqual(got.Meta, before.Meta) {
t.Fatal("recovery changed exact metadata")
}
return p.McpServerElicitationRequestResponse{Action: p.McpServerElicitationActionCancel, Meta: got.Meta}, nil
}})
request := p.Request{ID: p.RequestID{Value: "r"}, Method: "mcpServer/elicitation/request", Params: json.RawMessage(bad)}
resp, err := mock.InjectServerRequest(context.Background(), request)
if !errors.Is(err, p.ErrInvalidParams) || calls != 0 || !reflect.DeepEqual(resp, p.Response{}) {
t.Fatalf("invalid request published: %+v %v calls=%d", resp, err, calls)
}
request.Params = json.RawMessage(seed)
resp, err = mock.InjectServerRequest(context.Background(), request)
if err != nil || calls != 1 || resp.Error != nil {
t.Fatalf("recovery failed: %v", err)
}
var fields map[string]json.RawMessage
if err := json.Unmarshal(resp.Result, &fields); err != nil {
t.Fatal(err)
}
if string(fields["_meta"]) != `{"n":[9007199254740993]}` {
t.Fatal("reply changed exact metadata")
}
})
}
}

func FuzzElicitationMetadata(f *testing.F) {
for mode := uint8(0); mode < 4; mode++ {
f.Add(int64(9007199254740993), int16(400), "metadata", mode, uint8(0))
}
f.Fuzz(func(t *testing.T, n int64, exp int16, text string, mode, kind uint8) {
if len(text) > 4096 {
return
}
// Construct the expected graph independently, with every JSON kind and
// numbers that ordinary float64 decoding cannot preserve.
text = strings.ToValidUTF8(text, "\ufffd")
integer := json.Number(strconv.FormatInt(n, 10))
exponent := json.Number("-1.000000000000000001e" + strconv.Itoa(int(exp)))
var want any
switch kind % 6 {
case 0:
want = map[string]any{"integer": integer, "nested": []any{exponent, text, nil, true}}
case 1:
want = integer
case 2:
want = []any{integer, exponent}
case 3:
want = text
case 4:
want = false
}
raw, err := json.Marshal(want)
if err != nil {
t.Fatal(err)
}
body := []byte(elicitationModeBody(fmt.Sprintf(`"mode":%q,"_meta":%s`, elicitationMetadataModes[mode%4], raw)))
var params p.McpServerElicitationRequestParams
if err := json.Unmarshal(body, &params); err != nil {
t.Fatal(err)
}
for i := range body {
body[i] = ' '
}
if !reflect.DeepEqual(params.Meta, want) {
t.Fatalf("metadata changed: %#v / %#v", params.Meta, want)
}
encoded, err := json.Marshal(params)
if err != nil {
t.Fatal(err)
}
var round p.McpServerElicitationRequestParams
if err := json.Unmarshal(encoded, &round); err != nil || !reflect.DeepEqual(params, round) {
t.Fatalf("roundtrip changed request: %v", err)
}
})
}
106 changes: 106 additions & 0 deletions appserver/protocol/elicitation_metadata_error_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package protocol

import (
"encoding/json"
"errors"
"fmt"
"reflect"
"testing"

"github.com/dominicnunez/codex-sdk-go/internal/jsondecode"
"github.com/dominicnunez/codex-sdk-go/internal/jsonobject"
)

// The pre-fix decoder is an independent reference for unchanged admission errors.
type priorElicitationParams McpServerElicitationRequestParams

func (p *priorElicitationParams) UnmarshalJSON(data []byte) error {
type wire McpServerElicitationRequestParams
var decoded wire
var schema json.RawMessage
type elicitationWire struct {
*wire
RequestedSchema *json.RawMessage `json:"requestedSchema"`
}
w := elicitationWire{wire: &decoded, RequestedSchema: &schema}
required := []string{"serverName", "threadId"}
// Select only a supported form branch. The actual wire decoder retains
// ownership of selector errors and exact field matching.
rawMode, _, object := jsonobject.SelectField(data, "mode")
form := object && isElicitationFormMode(rawMode)
if form {
extra := struct {
*elicitationWire
ElicitationID stringExtra `json:"elicitationId"`
URL stringExtra `json:"url"`
}{elicitationWire: &w}
if err := unmarshalInboundObject(data, &extra, required, required); err != nil {
return err
}
decoded.ElicitationID = extra.ElicitationID.Value
decoded.URL = extra.URL.Value
} else if err := unmarshalInboundObject(data, &w, required, required); err != nil {
return err
}
if err := validateNonEmptyStringField("serverName", decoded.ServerName); err != nil {
return err
}
if err := validateNonEmptyStringField("threadId", decoded.ThreadID); err != nil {
return err
}
if err := validateNonEmptyStringPointerField("turnId", decoded.TurnID); err != nil {
return err
}
if err := validateMcpServerElicitationVariant(data, decoded.Mode); err != nil {
return err
}
switch decoded.Mode {
case McpServerElicitationModeForm:
if err := jsondecode.Unmarshal(schema, &decoded.RequestedSchema); err != nil {
return err
}
case McpServerElicitationModeOpenAIForm, McpServerElicitationModeOpenAIFormLegacy:
// An explicitly null schema is valid arbitrary JSON.
if w.RequestedSchema == nil {
schema = json.RawMessage("null")
}
decoded.OpenAIRequestedSchema = schema
}
if err := validateEnumOccurrences(data, "mode", false, validateElicitationMode); err != nil {
return err
}
*p = priorElicitationParams(decoded)
return nil
}

func TestElicitationMetadataPriorErrors(t *testing.T) {
for _, mode := range []string{"form", "url", "openaiForm", "openai/form"} {
base := fmt.Sprintf(`"serverName":"s","threadId":"t","mode":%q,"message":"m","requestedSchema":{"type":"object","properties":{}},"elicitationId":"e","url":"opaque"`, mode)
for _, raw := range []string{
`null`, `[]`, `{}`, `{"serverName":`,
`{` + base + `,"serverName":17,"_meta":{"n":42}}`,
`{` + base + `,"turnId":[],"_meta":42}`,
`{` + base + `,"message":17,"mode":"future"}`,
`{` + base + `,"threadId":"","mode":"future"}`,
`{` + base + `,"mode":null}`,
`{` + base + `,"mode":"future","mode":` + fmt.Sprintf("%q", mode) + `}`,
`{` + base + `,"requestedSchema":[]}`,
`{` + base + `,"url":42,"elicitationId":[]}`,
} {
var prior priorElicitationParams
want := prior.UnmarshalJSON([]byte(raw))
if want == nil {
continue
} // Inactive variant extras remain unrestricted.
var current McpServerElicitationRequestParams
got := current.UnmarshalJSON([]byte(raw))
if got == nil || got.Error() != want.Error() {
t.Fatalf("%s %s error changed: %v / %v", mode, raw, got, want)
}
var a, b *json.UnmarshalTypeError
if errors.As(got, &a) != errors.As(want, &b) || !reflect.DeepEqual(a, b) {
t.Fatalf("native error context changed: %#v / %#v", a, b)
}
}
}
}
79 changes: 79 additions & 0 deletions appserver/protocol/elicitation_metadata_precision_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package protocol_test

import (
"context"
"encoding/json"
"testing"

p "github.com/dominicnunez/codex-sdk-go/appserver/protocol"
)

func TestElicitationMetadataPrecision(t *testing.T) {
for _, mode := range elicitationMetadataModes {
for _, tc := range []struct{ name, raw string }{
{"small-object", `{"n":42}`},
{"nested-large", `{"ids":[9007199254740993,18446744073709551615]}`},
{"large-scalar", "9007199254740993"},
{"extreme-exponent", "1e400"},
{"signed-zero", "-0"},
{"decimal", "0.12345678901234567890123456789"},
{"nested-exponents", `{"a":[1e-400,-1e400,{"b":2.000000000000000001}]}`},
{"null", "null"},
{"empty-array", "[]"},
{"numeric-string", `"9007199254740993"`},
{"explicit-response", "null"},
} {
t.Run(string(mode)+"/"+tc.name, func(t *testing.T) {
mock := NewMockTransport()
client := p.NewClient(mock)
defer client.Close()
calls := 0
var observed []byte
schema := json.RawMessage(`{"type":"object","properties":{}}`)
if mode == p.McpServerElicitationModeOpenAIForm || mode == p.McpServerElicitationModeOpenAIFormLegacy {
schema = json.RawMessage(`{"const":9007199254740993}`)
}
client.SetApprovalHandlers(p.ApprovalHandlers{OnMcpServerElicitationRequest: func(_ context.Context, params p.McpServerElicitationRequestParams) (p.McpServerElicitationRequestResponse, error) {
calls++
var e error
observed, e = json.Marshal(params.Meta)
if e != nil {
t.Fatal(e)
}
if mode == p.McpServerElicitationModeOpenAIForm || mode == p.McpServerElicitationModeOpenAIFormLegacy {
if string(params.OpenAIRequestedSchema) != string(schema) {
t.Fatal("raw OpenAI schema control changed")
}
}
meta := params.Meta
if tc.name == "explicit-response" {
meta = map[string]any{"n": json.Number("9007199254740993")}
}
return p.McpServerElicitationRequestResponse{Action: p.McpServerElicitationActionCancel, Meta: meta}, nil
}})
body, e := json.Marshal(map[string]any{"serverName": "s", "threadId": "t", "message": "fixture", "mode": mode, "requestedSchema": schema, "url": "https://example.test", "elicitationId": "e", "_meta": json.RawMessage(tc.raw)})
if e != nil {
t.Fatal(e)
}
response, e := mock.InjectServerRequest(context.Background(), p.Request{ID: p.RequestID{Value: "e"}, Method: "mcpServer/elicitation/request", Params: body})
if e != nil || calls != 1 || response.Error != nil || len(response.Result) == 0 {
t.Fatalf("valid request rejected: %v", e)
}
if string(observed) != tc.raw {
t.Errorf("handler metadata changed: got=%s want=%s", observed, tc.raw)
}
var fields map[string]json.RawMessage
if e := json.Unmarshal(response.Result, &fields); e != nil {
t.Fatal(e)
}
expected := tc.raw
if tc.name == "explicit-response" {
expected = `{"n":9007199254740993}`
}
if expected != "null" && string(fields["_meta"]) != expected {
t.Errorf("returned metadata changed: got=%s want=%s", fields["_meta"], expected)
}
})
}
}
}
6 changes: 6 additions & 0 deletions docs/protocol.md
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,12 @@ The `openaiForm` and `openai/form` modes retain arbitrary JSON in
`OpenAIRequestedSchema`. Schema admission does not validate answers or decide
whether the application should collect the requested information.

In all four elicitation modes, request `Meta` preserves arbitrary JSON numbers
as `json.Number`, including nested numbers and values outside float64 range.
Returning that metadata in the response preserves its numeric tokens. Missing
or null metadata remains nil; strings and other JSON kinds retain their meaning.
Application-constructed response values keep their existing encoding behavior.

Legacy additional-filesystem permission paths and approval cwd, command-action,
parsed-command and grantRoot strings are preserved as supplied. They may be
relative, empty or contain dot segments. The SDK does not resolve them against
Expand Down
7 changes: 7 additions & 0 deletions docs/threat-model.md
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,13 @@ form-schema errors keep their precedence, and rejected parameters do not replace
a seeded receiver. These checks validate request shape; they do not authorize
input collection, choose a trusted URL or fetch external resources.

The same request owner preserves arbitrary `_meta` numbers as `json.Number`
before callback publication in every supported mode. It retains nested numeric
tokens and accepts values outside float64 range without imposing form-schema
integer limits on opaque metadata. Echoing received metadata in a response does
not round those numbers; application-owned response codecs remain unchanged.
Metadata stays untrusted data and does not grant approval authority.

Standard `form` requests also validate the typed `requestedSchema` before
callback publication: its root must be an object schema, and property definitions
must match a supported string, numeric, boolean or select alternative. Closed
Expand Down
Loading
0