diff --git a/README.md b/README.md index 40eb0c9..bd481b5 100644 --- a/README.md +++ b/README.md @@ -77,6 +77,12 @@ func main() { For the full walk-through — the input and output types, building the binary, and adding it to an agent — follow [Build your first CodeMode server](docs/docs/tutorials/first-server.md). + +Capability inputs support lists, string-keyed dicts, nested structs, and optional +values as well as strings, Booleans, integers, and floats. See the +[supported types and composite example](docs/docs/reference/public-api.md#supported-input-and-output-types) +for binding rules and the nested argument map passed to authorization. + Shorter compile-checked examples: [`example_test.go`](example_test.go) and [`mcpserver/example_test.go`](mcpserver/example_test.go). diff --git a/builder_test.go b/builder_test.go index 3e6f31f..a9432c8 100644 --- a/builder_test.go +++ b/builder_test.go @@ -29,8 +29,8 @@ type builderOutput struct { // invalidBuilderInput contains a type outside the restricted binder contract. type invalidBuilderInput struct { - // Count is intentionally unsupported as a 32-bit integer. - Count int32 `json:"count"` + // Values is intentionally unsupported as a fixed-size array. + Values [2]int64 `json:"values"` } // interfaceBuilderOutput is an unsupported interface/any nested output. diff --git a/docs/docs/explanation/security-model.md b/docs/docs/explanation/security-model.md index e2d2eb0..e170e57 100644 --- a/docs/docs/explanation/security-model.md +++ b/docs/docs/explanation/security-model.md @@ -47,8 +47,15 @@ A native call passes four stages in a fixed order: This order prevents policy from interpreting malformed Starlark values and prevents a handler from running before policy has evaluated the exact input the -handler will receive. The canonical map is separate from the typed handler -input, so policy cannot rewrite the handler's arguments by mutating the map. +handler will receive. Canonical arguments can contain nested objects and arrays, +not only scalars. Optional struct fields that are omitted or `None` are absent +at every level; `None` inside a list or dict value remains `nil` (`null` in Rego). +Integers supplied for float fields become `float64` values before authorization. +Custom authorizers and Rego policies must account for these shapes when checking +composite inputs. Every container is a fresh copy, separate from the typed +handler input, so policy cannot rewrite the handler's arguments by mutating the +map or its nested containers. `MaxValueDepth` and `MaxValueBytes` bound the +entire native-call argument map. An authorizer reports a recognized denial with an error that wraps `authz.ErrDenied`. CodeMode classifies that outcome as `permission denied` and does not dispatch the handler. Any other authorizer error, and an authorizer panic recovered at the boundary, becomes `authorization policy failure`. Policy diagnostic text does not cross the MCP boundary. diff --git a/docs/docs/how-to/use-rego-authorization.md b/docs/docs/how-to/use-rego-authorization.md index 020684b..64d6390 100644 --- a/docs/docs/how-to/use-rego-authorization.md +++ b/docs/docs/how-to/use-rego-authorization.md @@ -58,7 +58,7 @@ The adapter supplies only this input shape: } ``` -`subject.id` comes from the trusted `authz.Subject`. The capability fields come from registration. `arguments` contains the canonical, validated keyword arguments for that native call. An omitted optional argument is absent rather than set to `null`. The input does not include credentials, request metadata, Starlark source, environment data, or the current time. +`subject.id` comes from the trusted `authz.Subject`. The capability fields come from registration. `arguments` contains the canonical, validated keyword arguments for that native call. It may contain nested objects and arrays, so write policy checks against the registered input shape rather than assuming every argument is a scalar. Optional struct fields that are omitted or `None` are absent at every level; `None` in a list or dict value remains `null`. Integers passed to float fields become floating-point values before authorization. The input does not include credentials, request metadata, Starlark source, environment data, or the current time. ## Prepare the authorizer before the builder diff --git a/docs/docs/reference/mcp-tools.md b/docs/docs/reference/mcp-tools.md index 4ac8ae7..bbb79a9 100644 --- a/docs/docs/reference/mcp-tools.md +++ b/docs/docs/reference/mcp-tools.md @@ -238,18 +238,46 @@ Input field shapes are: | Go field type | `type` | `required` | | --- | --- | --- | | `string` | `str` | `true` | -| `int64` | `int` | `true` | +| `int`, `int8`–`int64`, `uint`, `uint8`–`uint64` | `int` | `true` | | `bool` | `bool` | `true` | -| `float64` | `float` | `true` | -| `*string` | str | None | `false` | -| `*int64` | int | None | `false` | -| `*bool` | bool | None | `false` | -| `*float64` | float | None | `false` | - -Named input types with these underlying kinds have the same shapes. Required -integers use the signed 64-bit range, and all floats must be finite. An omitted -optional argument and explicit `None` both produce a nil pointer and omit the -field from the canonical authorization map. +| `float32`, `float64` | `float` | `true` | +| `[]T`, including `[]byte` | `list[T]` | `true` | +| `map[K]T` with string-kind `K` | `dict[str, T]` | `true` | +| Nested struct | {a: T, b: U | None} | `true` | +| `*T` for a supported non-pointer type | T | None | `false` | + +Named types with these underlying kinds have the same shapes. These types +compose recursively. Lists accept Starlark lists or tuples; `[]byte` accepts +integers from 0 through 255. Dicts require string keys. Nested structs accept +dicts with declared keys only. Non-pointer fields are required at every level; +pointer fields may be omitted or `None`, with or without `omitempty`. Nested +input optionality uses `field: T | None`, not the output notation `field?: T`. + +Signed integers must fit their Go type's range. Unsigned integers must be +nonnegative and no greater than the smaller of the Go type's maximum and +`math.MaxInt64`. Float fields accept floats or integers converted to floats. +All floats must be finite, and `float32` values must fit the `float32` range. +A `float32` value is rounded to the nearest `float32` before authorization, so +policy sees the same number the handler receives. Float values are not +accepted for integer fields. + +Root input names must be Starlark identifiers that are not keywords. Nested +input names and all output names may be any non-empty tag name accepted by +`encoding/json`, including keywords such as `from`. Type notation quotes names +that are not plain identifiers, as in `{"created-at": str}`. + +An omitted optional struct field and explicit `None` both produce a nil pointer +and omit the field from canonical authorization arguments at every level. +`None` inside a list or dict value remains `nil`. Canonical arguments may +contain nested objects and arrays. See the +[composite input example](public-api.md#composite-input-example) for a Go input, +its generated signature, a Starlark call, and the authorizer's argument map. + +Invalid arguments identify nested paths such as `filter.owner`, `tags[1]`, +`labels['env']`, or `window['created-at']`. For example: +`invalid capability arguments: missing required argument "window.end"`. +The [input reference](public-api.md#supported-input-and-output-types) lists +registration rejections, range checks, and diagnostic forms. Output `type` strings use this compact notation: diff --git a/docs/docs/reference/public-api.md b/docs/docs/reference/public-api.md index 6a9c6f3..2a5408e 100644 --- a/docs/docs/reference/public-api.md +++ b/docs/docs/reference/public-api.md @@ -131,37 +131,76 @@ The policy and deployment-filter examples use this explicit capability identity: #### Supported input and output types -Both generic type arguments must be non-pointer structs. Input fields must be -direct exported fields. Output structs can be nested, but every struct in the -output graph must contain only direct exported fields. Embedded fields are not -supported. - -A field name defaults to its Go name. One `json` tag can replace it with a -valid Starlark identifier. Other struct tags, ignored fields, duplicate names, -and unsupported JSON tag options are rejected. - -Inputs support these scalar fields and named types with the same underlying -kind: - -| Go field type | Starlark input | Required | `omitempty` | -| --- | --- | --- | --- | -| `string` | `str` | Yes | Rejected | -| `int64` | `int` | Yes | Rejected | -| `bool` | `bool` | Yes | Rejected | -| `float64` | `float` | Yes | Rejected | -| `*string` | `str` or `None` | No | Accepted but not required | -| `*int64` | `int` or `None` | No | Accepted but not required | -| `*bool` | `bool` or `None` | No | Accepted but not required | -| `*float64` | `float` or `None` | No | Accepted but not required | - -Inputs accept keyword arguments only. Integers must fit the signed 64-bit -range, and floats must be finite. CodeMode does not coerce an integer to a -float or a Boolean to an integer. An omitted optional input and an explicit -`None` both produce a nil pointer and omit that key from the fresh canonical -authorization map. Duplicate keyword syntax is rejected by the Starlark parser +Both generic type arguments must be non-pointer structs. Every struct in the +input or output graph must contain only direct exported fields. Embedded fields +are not supported. + +A field name defaults to its Go name. One `json` tag can replace it. Root input +names must be Starlark identifiers that are not keywords because they become +keyword arguments. Nested input names and all output names can be any non-empty +tag name accepted by `encoding/json`, including keywords such as `from`. +Compact type notation quotes names that are not plain identifiers, as in +`{"created-at": str}`. Other struct tags, ignored fields (`json:"-"`), duplicate +names, and unsupported JSON tag options are rejected. + +Inputs recursively support these types, including named types with the same +underlying kinds: + +| Go field type | Starlark input | Type notation | +| --- | --- | --- | +| `string` | `str` | `str` | +| `bool` | `bool` | `bool` | +| `int`, `int8`, `int16`, `int32`, `int64` | `int` within the Go type's range | `int` | +| `uint`, `uint8`, `uint16`, `uint32`, `uint64` | `int` from 0 through the smaller of the Go type's maximum and `math.MaxInt64` | `int` | +| `float32`, `float64` | Finite `float` or `int`; the converted value must fit the Go type's range | `float` | +| `*T` | A supported `T` value or `None`; a struct field may also be omitted | T | None | +| `[]T` | `list` or `tuple` of supported `T` values | `list[T]` | +| `map[K]T` with string-kind `K` | `dict` with `str` keys and supported `T` values | `dict[str, T]` | +| Nested struct | `dict` with declared keys only; all non-pointer fields must be present | {a: T, b: U | None} | + +`[]byte` is a list of integers from 0 through 255, not base64 data. +Non-pointer fields are required at every nesting level, including lists, maps, +and structs. Pointer fields are optional; `omitempty` is accepted but not +required for them and is rejected on non-pointer fields. Omission and explicit +`None` both produce a nil pointer and omit that struct field from canonical +authorization arguments. `None` in a list or dict value is accepted only for a +pointer element type and remains `nil` in canonical arguments. + +Inputs accept keyword arguments only. Integers passed to float fields convert +to floats and appear as `float64` in canonical arguments. Floats are rejected +for integer fields, and Booleans are not integers. All floats must be finite; +`float32` values must fit the `float32` range and are rounded to the nearest +`float32` before authorization, so canonical arguments hold the same number the +handler receives. Duplicate keyword syntax is rejected by the Starlark parser as `ErrInvalidProgram` before authorization or handler dispatch. Positional, -unknown, missing required, incorrectly typed, and out-of-range arguments reach -binding and map to `ErrInvalidArguments`. +unknown, missing required, incorrectly typed, and out-of-range arguments map to +`ErrInvalidArguments`. + +Registration rejects input arrays, interfaces including `any`, channels, +functions, complex numbers, pointer-to-pointer types, non-string map keys, +recursive types, and types implementing `json.Unmarshaler` or +`encoding.TextUnmarshaler` (for example, `time.Time`), in addition to the field +and tag restrictions above. + +Argument diagnostics use dotted struct paths, list indexes, and quoted dict +keys: `filter.owner`, `tags[1]`, and `labels['env']`. Non-identifier struct keys +use brackets, as in `window['created-at']`. After the model-visible prefix +`invalid capability arguments: `, diagnostics include: + +- `argument "filter.owner" must be a string` (or `must be a string or None` for an optional value) +- `unknown argument "filter.bogus"` +- `missing required argument "window.end"` +- `argument "labels" keys must be strings` +- `argument "count" must be between 0 and 255` +- `argument "n" overflows int64` +- `argument "x" is not finite` +- `argument "x" overflows float32` + +`MaxValueDepth` and `MaxValueBytes` apply to the entire native-call argument map, +including nested containers. The worker charges each argument value against +these budgets before copying it, so an argument map that exceeds them, +including one built from repeated references to the same list, fails with +`ErrResourceLimit` before authorization or handler dispatch. An output can recursively contain these Go types: @@ -205,6 +244,66 @@ Returned values are checked at call time. An unsigned result above Output-depth, per-value byte, and aggregate intermediate-byte exhaustion map to `ErrResourceLimit`. +#### Composite input example + +For a capability registered as `records.search`, this input combines a list, +a nested struct with an optional field, and a dict: + +```go +// searchFilter selects records by date and optional owner. +type searchFilter struct { + // CreatedAt is the date to match. + CreatedAt string `json:"created-at"` + // Owner limits results to one owner when supplied. + Owner *string `json:"owner,omitempty"` +} + +// searchInput contains the keyword arguments for records.search. +type searchInput struct { + // Tags are the tags to match. + Tags []string `json:"tags"` + // Filter selects record fields. + Filter searchFilter `json:"filter"` + // Labels maps label names to optional values. + Labels map[string]*string `json:"labels"` +} +``` + +The generated signature preserves declaration order: + +```text +records.search(*, tags: list[str], filter: {"created-at": str, owner: str | None}, labels: dict[str, str | None]) +``` + +A Starlark program can make this native call: + +```python +def main(): + return records.search( + tags=["release", "review"], + filter={"created-at": "2026-10-06", "owner": None}, + labels={"env": "prod", "team": None}, + ) +``` + +The authorizer receives this `AuthorizationInput.Arguments` value: + +```go +map[string]any{ + "tags": []any{"release", "review"}, + "filter": map[string]any{ + "created-at": "2026-10-06", + }, + "labels": map[string]any{ + "env": "prod", + "team": nil, + }, +} +``` + +Omitting `filter.owner` produces the same map. The optional struct field is +absent, while the optional dict value remains present as `nil`. + ### Options and builder lifecycle `Options` configures one server build: @@ -444,9 +543,12 @@ Describe(name CapabilityName) (Description, error) | `input` | array of field shapes | Ordered input fields. | | `output` | array of field shapes | Stable result contract. Models and clients must use these ordered field shapes. | -Each field shape remains the flat object `{name, type, required}`. Input -`type` values are `str`, `int`, `bool`, and `float`; optional pointer inputs -append ` | None` and set `required` to `false`. +Each field shape remains the flat object `{name, type, required}`. Input `type` +strings use the recursive [input type notation](#supported-input-and-output-types): +`str`, `int`, `bool`, `float`, `list[T]`, `dict[str, T]`, and `{field: T}`. +Pointer inputs append ` | None` and set `required` to `false`; other root +fields set it to `true`. Inside an input struct, optional fields use +`field: T | None`, not the output-only `field?: T` notation. Output `type` strings use deterministic compact notation: @@ -546,10 +648,17 @@ calling `WithSubject`; these functions do not authenticate a caller. | `CapabilityName string` | Dotted model-facing name. | | `Arguments map[string]any` | Fresh canonical projection of validated keyword arguments. | -Canonical arguments contain JSON-shaped scalar values from the supported input -matrix: strings, `int64` integers, Booleans, and finite `float64` values. An -omitted optional value or explicit `None` is absent from the map. The canonical -map is separate from the typed input passed to the handler. +Canonical arguments contain nested JSON-shaped values: `string`, `int64`, +`bool`, finite `float64`, `[]any`, and `map[string]any`. Integers supplied for +float fields appear as `float64`. Optional struct fields that are omitted or +explicitly `None` are absent at every nesting level; `None` inside a list or +dict value remains `nil`. Every container is a fresh copy, separate from the +typed input passed to the handler. + +Custom authorizers must handle objects and arrays when the registered input +contains composite types, rather than assuming all argument values are scalars. +Rego policies receive those containers as objects and arrays; optional list +elements and dict values can be `null`. ### Authorizers @@ -613,7 +722,7 @@ The Rego input contains exactly these fields: } ``` -`subject.id` is the trusted subject ID. `capability.id` is the stable policy identity; `capability.name` is the dotted discovery and Starlark name. `arguments` is the canonical map from `AuthorizationInput`, borrowed read-only for the synchronous evaluation. An omitted optional argument is absent from the map. +`subject.id` is the trusted subject ID. `capability.id` is the stable policy identity; `capability.name` is the dotted discovery and Starlark name. `arguments` is the canonical map from `AuthorizationInput`, borrowed read-only for the synchronous evaluation. It can contain nested objects and arrays. Optional struct fields that are omitted or `None` are absent at every level; `None` inside arrays or dict values is `null`. A ground decision is either undefined or yields one value. That value must be Boolean. diff --git a/internal/binding/bind.go b/internal/binding/bind.go new file mode 100644 index 0000000..bd954da --- /dev/null +++ b/internal/binding/bind.go @@ -0,0 +1,295 @@ +package binding + +import ( + "fmt" + "math" + + "go.starlark.net/starlark" +) + +// keywordTupleSize is the length of one Starlark keyword argument tuple. +const keywordTupleSize = 2 + +// argumentBinder carries one worker-side call's schema, budget, and diagnostic path. +type argumentBinder struct { + // schema is the validated input schema. + schema InputSchema + + // converter enforces the depth and byte-derived node budget. + converter valueConverter + + // path is the active diagnostic path, rendered only on error. + path argumentPath +} + +// Bind binds keyword-only Starlark arguments against a validated schema. +// +// It returns only a fresh canonical JSON-shaped map. Explicit Starlark None and +// omission both leave an optional member absent at every struct level. Callers +// must not treat the result as the authoritative authorization map; parent +// re-binding constructs that. +// +// maxDepth and maxNodes must be positive and bound the canonical map exactly +// as [FromStarlark] bounds a value: the argument map is depth 1, optional +// wrappers add no depth, and every container's child count is charged against +// the node budget before the container is materialized. Starlark values that +// alias one container many times therefore fail with [ErrValueLimit] instead +// of being copied. Argument mistakes wrap [ErrInvalidArguments]. +func (schema InputSchema) Bind( + args starlark.Tuple, + kwargs []starlark.Tuple, + maxDepth int, + maxNodes int, +) (map[string]any, error) { + if len(args) != 0 { + return nil, fmt.Errorf("%w: positional arguments are not accepted", ErrInvalidArguments) + } + for _, keyword := range kwargs { + if len(keyword) != keywordTupleSize { + return nil, fmt.Errorf("%w: malformed keyword argument", ErrInvalidArguments) + } + } + converter, err := newValueConverter(maxDepth, maxNodes) + if err != nil { + return nil, err + } + binder := argumentBinder{schema: schema, converter: converter, path: newArgumentPath()} + if err := binder.converter.consumeNode(1); err != nil { + return nil, err + } + if _, err := binder.converter.containerLength(len(kwargs)); err != nil { + return nil, err + } + return binder.bindStruct(schema.Nodes[schema.Root], kwargs, 1) +} + +// bindStruct binds name/value pairs against a struct node's declared fields. +// +// The caller has already charged the struct node and its entry count. +func (binder *argumentBinder) bindStruct(node InputNode, entries []starlark.Tuple, depth int) (map[string]any, error) { + converted := make(map[string]any, len(entries)) + seen := make([]bool, len(node.Fields)) + for _, entry := range entries { + name, ok := entry[0].(starlark.String) + if !ok { + return nil, binder.keyError() + } + binder.path.push(pathSegment{kind: segmentMember, name: string(name)}) + position := fieldPosition(node, string(name)) + if position < 0 { + return nil, fmt.Errorf("%w: unknown argument %q", ErrInvalidArguments, binder.path.String()) + } + if seen[position] { + return nil, fmt.Errorf("%w: duplicate argument %q", ErrInvalidArguments, binder.path.String()) + } + seen[position] = true + field := node.Fields[position] + if binder.schema.Nodes[field.Node].Kind != InputOptional || entry[1] != starlark.None { + value, err := binder.bindValue(field.Node, entry[1], depth+1, false) + if err != nil { + return nil, err + } + converted[field.Name] = value + } + binder.path.pop() + } + for position, field := range node.Fields { + if !seen[position] && binder.schema.Nodes[field.Node].Kind != InputOptional { + binder.path.push(pathSegment{kind: segmentMember, name: field.Name}) + return nil, fmt.Errorf("%w: missing required argument %q", ErrInvalidArguments, binder.path.String()) + } + } + return converted, nil +} + +// bindValue binds one Starlark value against the node at index. +// +// nullable reports that an enclosing optional node also accepts None, which +// only changes the type-mismatch wording. +func (binder *argumentBinder) bindValue(index int, value starlark.Value, depth int, nullable bool) (any, error) { + node := binder.schema.Nodes[index] + if node.Kind == InputOptional { + if value == starlark.None { + if err := binder.converter.consumeNode(depth); err != nil { + return nil, err + } + return nil, nil //nolint:nilnil // None maps to JSON null inside lists and dicts. + } + return binder.bindValue(node.Elem, value, depth, true) + } + if err := binder.converter.consumeNode(depth); err != nil { + return nil, err + } + switch node.Kind { + case InputList: + return binder.bindList(node, value, depth, nullable) + case InputMap: + return binder.bindMap(node, value, depth, nullable) + case InputStruct: + if dict, ok := value.(*starlark.Dict); ok { + if _, err := binder.converter.containerLength(dict.Len()); err != nil { + return nil, err + } + return binder.bindStruct(node, dict.Items(), depth) + } + return nil, typeError(node.Kind, &binder.path, nullable) + case InputString, InputInt, InputBool, InputFloat, InputOptional: + } + return binder.bindScalar(node, value, nullable) +} + +// bindScalar binds one already-charged Starlark scalar against a scalar node. +func (binder *argumentBinder) bindScalar(node InputNode, value starlark.Value, nullable bool) (any, error) { + switch node.Kind { + case InputString: + if text, ok := value.(starlark.String); ok { + return string(text), nil + } + case InputInt: + if integer, ok := value.(starlark.Int); ok { + converted, fits := integer.Int64() + if !fits { + return nil, integerRangeError(node, &binder.path) + } + return checkInteger(node, converted, &binder.path) + } + case InputBool: + if boolean, ok := value.(starlark.Bool); ok { + return bool(boolean), nil + } + case InputFloat: + switch number := value.(type) { + case starlark.Float: + return checkFloat(node, float64(number), &binder.path) + case starlark.Int: + return checkFloat(node, float64(number.Float()), &binder.path) + } + case InputList, InputMap, InputStruct, InputOptional: + } + return nil, typeError(node.Kind, &binder.path, nullable) +} + +// bindList binds a Starlark list or tuple element by element after charging its length. +func (binder *argumentBinder) bindList(node InputNode, value starlark.Value, depth int, nullable bool) (any, error) { + var sequence starlark.Indexable + switch typed := value.(type) { + case *starlark.List: + sequence = typed + case starlark.Tuple: + sequence = typed + default: + return nil, typeError(InputList, &binder.path, nullable) + } + length, err := binder.converter.containerLength(sequence.Len()) + if err != nil { + return nil, err + } + converted := make([]any, length) + for index := range converted { + binder.path.push(pathSegment{kind: segmentIndex, index: index}) + element, err := binder.bindValue(node.Elem, sequence.Index(index), depth+1, false) + if err != nil { + return nil, err + } + binder.path.pop() + converted[index] = element + } + return converted, nil +} + +// bindMap binds a str-keyed Starlark dict value by value after charging its length. +func (binder *argumentBinder) bindMap(node InputNode, value starlark.Value, depth int, nullable bool) (any, error) { + dict, ok := value.(*starlark.Dict) + if !ok { + return nil, typeError(InputMap, &binder.path, nullable) + } + length, err := binder.converter.containerLength(dict.Len()) + if err != nil { + return nil, err + } + converted := make(map[string]any, length) + for _, item := range dict.Items() { + key, isString := item[0].(starlark.String) + if !isString { + return nil, binder.keyError() + } + binder.path.push(pathSegment{kind: segmentKey, name: string(key)}) + element, err := binder.bindValue(node.Elem, item[1], depth+1, false) + if err != nil { + return nil, err + } + binder.path.pop() + converted[string(key)] = element + } + return converted, nil +} + +// keyError reports a non-string keyword name or dict key at the current path. +func (binder *argumentBinder) keyError() error { + if len(binder.path.segments) == 0 { + return fmt.Errorf("%w: keyword name must be a string", ErrInvalidArguments) + } + return fmt.Errorf("%w: argument %q keys must be strings", ErrInvalidArguments, binder.path.String()) +} + +// checkInteger enforces an int node's inclusive range. +func checkInteger(node InputNode, value int64, path *argumentPath) (int64, error) { + if value < node.IntMin || value > node.IntMax { + return 0, integerRangeError(node, path) + } + return value, nil +} + +// integerRangeError describes an integer outside the node's range. +func integerRangeError(node InputNode, path *argumentPath) error { + if node.IntMin == math.MinInt64 && node.IntMax == math.MaxInt64 { + return fmt.Errorf("%w: argument %q overflows int64", ErrInvalidArguments, path.String()) + } + return fmt.Errorf("%w: argument %q must be between %d and %d", + ErrInvalidArguments, path.String(), node.IntMin, node.IntMax) +} + +// checkFloat enforces finiteness and the float32 range, then canonicalizes. +// +// A float32 node returns the value rounded to float32, so authorization sees +// exactly the number the handler receives. +func checkFloat(node InputNode, value float64, path *argumentPath) (float64, error) { + if !isFiniteFloat(value) { + return 0, fmt.Errorf("%w: argument %q is not finite", ErrInvalidArguments, path.String()) + } + if !node.Float32 { + return value, nil + } + if math.Abs(value) > math.MaxFloat32 { + return 0, fmt.Errorf("%w: argument %q overflows float32", ErrInvalidArguments, path.String()) + } + return float64(float32(value)), nil +} + +// typeError describes the value a node expected. +func typeError(kind InputKind, path *argumentPath, nullable bool) error { + if nullable { + return fmt.Errorf("%w: argument %q must be %s or None", ErrInvalidArguments, path.String(), expectedValue(kind)) + } + return fmt.Errorf("%w: argument %q must be %s", ErrInvalidArguments, path.String(), expectedValue(kind)) +} + +// expectedValue names the accepted Starlark value for kind. +func expectedValue(kind InputKind) string { + switch kind { + case InputString: + return "a string" + case InputInt: + return "an integer" + case InputBool: + return "a bool" + case InputFloat: + return "a float" + case InputList: + return "a list" + case InputMap, InputStruct: + return "a dict" + case InputOptional: + } + return "a supported value" +} diff --git a/internal/binding/bind_bounds_test.go b/internal/binding/bind_bounds_test.go new file mode 100644 index 0000000..dec6b3f --- /dev/null +++ b/internal/binding/bind_bounds_test.go @@ -0,0 +1,142 @@ +package binding + +import ( + "runtime" + "runtime/debug" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.starlark.net/starlark" + "go.starlark.net/syntax" +) + +// cubeInput takes a three-level nested list. +type cubeInput struct { + // Cube is a list of lists of integer lists. + Cube [][][]int64 `json:"cube"` +} + +// groupedInput takes integer lists under model-chosen keys. +type groupedInput struct { + // Groups maps names to integer lists. + Groups map[string][]int64 `json:"groups"` +} + +// ratioInput takes float32 values directly and inside a list. +type ratioInput struct { + // Ratio is a required float32. + Ratio float32 `json:"ratio"` + + // Ratios is a list of float32 values. + Ratios []float32 `json:"ratios"` +} + +// measureAllocations reports heap bytes allocated while run executes. +func measureAllocations(t *testing.T, run func()) uint64 { + t.Helper() + previousGCPercent := debug.SetGCPercent(-1) + defer debug.SetGCPercent(previousGCPercent) + var before, after runtime.MemStats + runtime.ReadMemStats(&before) + run() + runtime.ReadMemStats(&after) + return after.TotalAlloc - before.TotalAlloc +} + +// starlarkGlobal evaluates source and returns the named global. +func starlarkGlobal(t *testing.T, source string, name string) starlark.Value { + t.Helper() + globals, err := starlark.ExecFileOptions(&syntax.FileOptions{}, &starlark.Thread{}, "test.star", source, nil) + require.NoError(t, err) + return globals[name] +} + +// TestBindChargesAliasedContainersBeforeMaterializing proves repeated references +// to one Starlark list cannot expand past the node budget. +func TestBindChargesAliasedContainersBeforeMaterializing(t *testing.T) { + plan, err := CompileFor[cubeInput, representativeOutput]() + require.NoError(t, err) + cube := starlarkGlobal(t, "a = [0] * 300\nb = [a] * 300\nc = [b] * 300\n", "c") + schema := plan.InputSchema() + + var bindErr error + allocated := measureAllocations(t, func() { + _, bindErr = schema.Bind(nil, []starlark.Tuple{keyword("cube", cube)}, testValueDepth, 64*1024) + }) + + require.ErrorIs(t, bindErr, ErrValueLimit) + assert.Less(t, allocated, uint64(8<<20), "27 million logical elements must fail before being copied") +} + +// TestBindEnforcesArgumentDepth proves the argument map counts as depth 1. +func TestBindEnforcesArgumentDepth(t *testing.T) { + plan, err := CompileFor[cubeInput, representativeOutput]() + require.NoError(t, err) + cube := starlarkGlobal(t, "c = [[[1]]]\n", "c") + schema := plan.InputSchema() + + _, err = schema.Bind(nil, []starlark.Tuple{keyword("cube", cube)}, 4, testValueNodes) + require.ErrorIs(t, err, ErrValueLimit) + + canonical, err := schema.Bind(nil, []starlark.Tuple{keyword("cube", cube)}, 5, testValueNodes) + require.NoError(t, err) + assert.Equal(t, map[string]any{"cube": []any{[]any{[]any{int64(1)}}}}, canonical) +} + +// TestBindingCostIsLinearInKeyLength proves long model-chosen keys are not +// copied once per nested element on either side of the worker boundary. +func TestBindingCostIsLinearInKeyLength(t *testing.T) { + plan, err := CompileFor[groupedInput, representativeOutput]() + require.NoError(t, err) + key := strings.Repeat("k", 60_000) + items := make([]starlark.Value, 1_000) + neutral := make([]any, len(items)) + for index := range items { + items[index] = starlark.MakeInt(index) + neutral[index] = int64(index) + } + groups := starlark.NewDict(1) + require.NoError(t, groups.SetKey(starlark.String(key), starlark.NewList(items))) + schema := plan.InputSchema() + + var bindErr error + childAllocated := measureAllocations(t, func() { + _, bindErr = schema.Bind(nil, []starlark.Tuple{keyword("groups", groups)}, testValueDepth, testValueNodes) + }) + require.NoError(t, bindErr) + + var rebindErr error + parentAllocated := measureAllocations(t, func() { + _, _, rebindErr = plan.BindValue(map[string]any{"groups": map[string]any{key: neutral}}) + }) + require.NoError(t, rebindErr) + + const linearBound = 1 << 20 + assert.Less(t, childAllocated, uint64(linearBound)) + assert.Less(t, parentAllocated, uint64(linearBound)) +} + +// TestFloat32InputsCanonicalizeToTheHandlerValue proves policy sees the rounded value the handler receives. +func TestFloat32InputsCanonicalizeToTheHandlerValue(t *testing.T) { + plan, err := CompileFor[ratioInput, representativeOutput]() + require.NoError(t, err) + + child, err := plan.InputSchema().Bind(nil, []starlark.Tuple{ + keyword("ratio", starlark.Float(0.1)), + keyword("ratios", starlark.NewList([]starlark.Value{starlark.Float(1e-50), starlark.MakeInt(16_777_217)})), + }, testValueDepth, testValueNodes) + require.NoError(t, err) + + bound, canonical, err := plan.BindValue(child) + require.NoError(t, err) + typed, ok := bound.(ratioInput) + require.True(t, ok) + assert.Equal(t, ratioInput{Ratio: 0.1, Ratios: []float32{0, 16_777_216}}, typed) + assert.Equal(t, map[string]any{ + "ratio": float64(typed.Ratio), + "ratios": []any{float64(typed.Ratios[0]), float64(typed.Ratios[1])}, + }, canonical) + assert.Equal(t, child, canonical, "worker and parent canonicalization must agree") +} diff --git a/internal/binding/compile.go b/internal/binding/compile.go new file mode 100644 index 0000000..ba708d8 --- /dev/null +++ b/internal/binding/compile.go @@ -0,0 +1,369 @@ +package binding + +import ( + "encoding" + "encoding/json" + "fmt" + "reflect" + "strings" +) + +// arenaHint is the initial compiled-arena capacity for typical capability graphs. +const arenaHint = 8 + +// direction selects the per-direction rules of the shared type compiler. +type direction uint8 + +const ( + // directionInput compiles a type that CodeMode builds from model arguments. + directionInput direction = iota + 1 + + // directionOutput compiles a type that a handler returns to the model. + directionOutput +) + +// label names the direction in registration diagnostics. +func (dir direction) label() string { + if dir == directionInput { + return "input" + } + return "output" +} + +// nodeKind identifies one compiled type node. +type nodeKind uint8 + +const ( + nodeString nodeKind = iota + 1 + nodeInt + nodeUint + nodeBool + nodeFloat + nodeBytes + nodeList + nodeMap + nodeStruct + nodePointer +) + +// typeNode is one immutable compiled type node. +type typeNode struct { + // kind selects the conversion and notation strategy. + kind nodeKind + + // typ is the exact Go type the node describes. + typ reflect.Type + + // elem is the child node index for pointers, lists, and maps. + elem int + + // fields are declaration-ordered struct members. + fields []structField + + // notation is the exact model-facing type string for this node. + notation string +} + +// structField is one compiled exported struct member. +type structField struct { + // name is the JSON and Starlark field name. + name string + + // index is the direct field index on the struct type. + index int + + // node is the compiled field type. + node int + + // omitempty omits a nil pointer from converted output. + omitempty bool +} + +// typeCompiler builds a flat post-order node arena with cycle detection. +type typeCompiler struct { + // dir selects input or output rules. + dir direction + + // nodes is the arena under construction. + nodes []typeNode + + // done maps a completed type onto its arena index. + done map[reflect.Type]int + + // active is the stack of types currently being compiled. + active map[reflect.Type]struct{} + + // forbidden are the custom codec interfaces this direction rejects. + forbidden [2]reflect.Type +} + +// compileType compiles root and every reachable type for dir. +// +// Children are appended before their parents, so every elem and field node +// index is lower than the index of the node that references it. +func compileType(dir direction, root reflect.Type) (int, []typeNode, error) { + compiler := typeCompiler{ + dir: dir, + nodes: make([]typeNode, 0, arenaHint), + done: make(map[reflect.Type]int), + active: make(map[reflect.Type]struct{}), + } + if dir == directionInput { + compiler.forbidden = [2]reflect.Type{ + reflect.TypeFor[json.Unmarshaler](), + reflect.TypeFor[encoding.TextUnmarshaler](), + } + } else { + compiler.forbidden = [2]reflect.Type{ + reflect.TypeFor[json.Marshaler](), + reflect.TypeFor[encoding.TextMarshaler](), + } + } + index, err := compiler.compile(root, "") + if err != nil { + return 0, nil, err + } + return index, compiler.nodes, nil +} + +// compile returns the arena index for typ, reusing completed nodes and rejecting cycles. +func (compiler *typeCompiler) compile(typ reflect.Type, path string) (int, error) { + if typ == nil { + return 0, fmt.Errorf("%w: %s field %q has unsupported type ", ErrInvalidPlan, compiler.dir.label(), path) + } + if index, ok := compiler.done[typ]; ok { + return index, nil + } + if _, exists := compiler.active[typ]; exists { + return 0, fmt.Errorf("%w: cyclic type at %q", ErrInvalidPlan, path) + } + compiler.active[typ] = struct{}{} + defer delete(compiler.active, typ) + + index, err := compiler.compileNew(typ, path) + if err != nil { + return 0, err + } + compiler.done[typ] = index + return index, nil +} + +// compileNew appends one newly compiled node for typ. +func (compiler *typeCompiler) compileNew(typ reflect.Type, path string) (int, error) { + if compiler.implementsForbidden(typ) { + return 0, compiler.unsupported(path, typ) + } + switch typ.Kind() { //nolint:exhaustive // Unsupported reflect kinds share registration-time rejection. + case reflect.String: + return compiler.append(typeNode{kind: nodeString, typ: typ, notation: stringType}), nil + case reflect.Bool: + return compiler.append(typeNode{kind: nodeBool, typ: typ, notation: boolType}), nil + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + return compiler.append(typeNode{kind: nodeInt, typ: typ, notation: integerType}), nil + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + return compiler.append(typeNode{kind: nodeUint, typ: typ, notation: integerType}), nil + case reflect.Float32, reflect.Float64: + return compiler.append(typeNode{kind: nodeFloat, typ: typ, notation: floatType}), nil + case reflect.Pointer: + return compiler.compilePointer(typ, path) + case reflect.Slice: + return compiler.compileSequence(typ, path) + case reflect.Array: + if compiler.dir == directionInput { + return 0, compiler.unsupported(path, typ) + } + return compiler.compileSequence(typ, path) + case reflect.Map: + return compiler.compileMap(typ, path) + case reflect.Struct: + return compiler.compileStruct(typ, path) + default: + return 0, compiler.unsupported(path, typ) + } +} + +// compilePointer compiles a pointer to a supported node. +// +// Inputs reject pointer-to-pointer because one None cannot select a level. +func (compiler *typeCompiler) compilePointer(typ reflect.Type, path string) (int, error) { + if compiler.dir == directionInput && typ.Elem().Kind() == reflect.Pointer { + return 0, compiler.unsupported(path, typ) + } + elem, err := compiler.compile(typ.Elem(), path) + if err != nil { + return 0, err + } + return compiler.append(typeNode{ + kind: nodePointer, + typ: typ, + elem: elem, + notation: pointerNotation(compiler.nodes[elem].notation), + }), nil +} + +// compileSequence compiles a slice or array. +// +// Output byte sequences use the dedicated byte conversion; input byte slices +// are ordinary lists of bounded integers. +func (compiler *typeCompiler) compileSequence(typ reflect.Type, path string) (int, error) { + if compiler.dir == directionOutput && typ.Elem().Kind() == reflect.Uint8 && + !compiler.implementsForbidden(typ.Elem()) { + return compiler.append(typeNode{kind: nodeBytes, typ: typ, notation: listNotation(integerType)}), nil + } + elem, err := compiler.compile(typ.Elem(), path) + if err != nil { + return 0, err + } + return compiler.append(typeNode{ + kind: nodeList, + typ: typ, + elem: elem, + notation: listNotation(compiler.nodes[elem].notation), + }), nil +} + +// compileMap compiles a string-keyed map. +func (compiler *typeCompiler) compileMap(typ reflect.Type, path string) (int, error) { + keyType := typ.Key() + if compiler.implementsForbidden(keyType) || keyType.Kind() != reflect.String { + if path == "" { + return 0, fmt.Errorf("%w: %s type %s has unsupported map key type %s", + ErrInvalidPlan, compiler.dir.label(), typ, keyType) + } + return 0, fmt.Errorf("%w: %s field %q has unsupported map key type %s", + ErrInvalidPlan, compiler.dir.label(), path, keyType) + } + elem, err := compiler.compile(typ.Elem(), path) + if err != nil { + return 0, err + } + return compiler.append(typeNode{ + kind: nodeMap, + typ: typ, + elem: elem, + notation: mapNotation(compiler.nodes[elem].notation), + }), nil +} + +// compileStruct compiles exported non-embedded fields in declaration order. +// +// Root input field names are Starlark keyword arguments and must be +// identifiers. Every other field name is a dict key and only needs to be a +// valid JSON field name. +func (compiler *typeCompiler) compileStruct(typ reflect.Type, path string) (int, error) { + label := compiler.dir.label() + keywords := compiler.dir == directionInput && path == "" + fields := make([]structField, 0, typ.NumField()) + seen := make(map[string]struct{}, typ.NumField()) + for index := range typ.NumField() { + field := typ.Field(index) + name, options, err := compileFieldName(field, keywords) + if err != nil { + return 0, fmt.Errorf("%w: %s field %s: %w", ErrInvalidPlan, label, fieldPath(path, field.Name), err) + } + namePath := fieldPath(path, name) + if options.omitempty && field.Type.Kind() != reflect.Pointer { + if compiler.dir == directionInput { + return 0, fmt.Errorf("%w: required input %q cannot use omitempty", ErrInvalidPlan, namePath) + } + return 0, fmt.Errorf("%w: output field %q cannot use omitempty", ErrInvalidPlan, namePath) + } + if _, exists := seen[name]; exists { + return 0, fmt.Errorf("%w: duplicate %s name %q", ErrInvalidPlan, label, namePath) + } + node, err := compiler.compile(field.Type, namePath) + if err != nil { + return 0, err + } + seen[name] = struct{}{} + fields = append(fields, structField{ + name: name, + index: index, + node: node, + omitempty: options.omitempty, + }) + } + return compiler.append(typeNode{ + kind: nodeStruct, + typ: typ, + fields: fields, + notation: compiler.structNotation(fields), + }), nil +} + +// append stores node and returns its arena index. +func (compiler *typeCompiler) append(node typeNode) int { + compiler.nodes = append(compiler.nodes, node) + return len(compiler.nodes) - 1 +} + +// structNotation renders a declaration-ordered struct literal. +// +// Output omitempty pointers render as "name?: T"; every other optional field +// renders its pointer notation "name: T | None". +func (compiler *typeCompiler) structNotation(fields []structField) string { + var notation strings.Builder + notation.WriteByte('{') + for index, field := range fields { + if index > 0 { + notation.WriteString(", ") + } + notation.WriteString(notationName(field.name)) + if compiler.dir == directionOutput && field.omitempty { + notation.WriteString("?: ") + notation.WriteString(compiler.nodes[compiler.nodes[field.node].elem].notation) + continue + } + notation.WriteString(": ") + notation.WriteString(compiler.nodes[field.node].notation) + } + notation.WriteByte('}') + return notation.String() +} + +// implementsForbidden reports whether typ or *typ implements a rejected codec interface. +func (compiler *typeCompiler) implementsForbidden(typ reflect.Type) bool { + for _, forbidden := range compiler.forbidden { + if typ.Implements(forbidden) { + return true + } + if typ.Kind() != reflect.Pointer && reflect.PointerTo(typ).Implements(forbidden) { + return true + } + } + return false +} + +// unsupported classifies a rejected type at path. +func (compiler *typeCompiler) unsupported(path string, typ reflect.Type) error { + if path == "" { + return fmt.Errorf("%w: %s type %s is unsupported", ErrInvalidPlan, compiler.dir.label(), typ) + } + return fmt.Errorf("%w: %s field %q has unsupported type %s", ErrInvalidPlan, compiler.dir.label(), path, typ) +} + +// fieldPath joins a parent compile path with a field name. +func fieldPath(parent string, name string) string { + if parent == "" { + return name + } + return parent + "/" + name +} + +// pointerNotation appends one nullable suffix. +func pointerNotation(elem string) string { + if strings.HasSuffix(elem, noneSuffix) { + return elem + } + return elem + noneSuffix +} + +// listNotation renders list[T]. +func listNotation(elem string) string { + return "list[" + elem + "]" +} + +// mapNotation renders dict[str, T]. +func mapNotation(elem string) string { + return "dict[str, " + elem + "]" +} diff --git a/internal/binding/composite_test.go b/internal/binding/composite_test.go new file mode 100644 index 0000000..36508c4 --- /dev/null +++ b/internal/binding/composite_test.go @@ -0,0 +1,479 @@ +package binding + +import ( + "encoding/json" + "maps" + "math" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.starlark.net/starlark" +) + +const ( + // testValueDepth is a generous worker binding depth for behavior tests. + testValueDepth = 16 + + // testValueNodes is a generous worker binding node budget for behavior tests. + testValueNodes = 1 << 20 +) + +// compositeInput exercises every composite input form in one capability. +type compositeInput struct { + // Tags is a required list of strings. + Tags []string `json:"tags"` + + // Filter is a required nested struct. + Filter compositeFilter `json:"filter"` + + // Labels is a required string-keyed map. + Labels map[string]int64 `json:"labels"` + + // Window is an optional nested struct with non-identifier member names. + Window *compositeWindow `json:"window"` + + // Payload is a byte slice bound as a list of bounded integers. + Payload []byte `json:"payload"` + + // Scores is a list of optional float32 values. + Scores []*float32 `json:"scores"` +} + +// compositeFilter is a nested input struct with a narrow integer and an optional float. +type compositeFilter struct { + // Owner is a required nested string. + Owner string `json:"owner"` + + // Level is a required unsigned 8-bit integer. + Level uint8 `json:"level"` + + // MinScore is an optional nested float. + MinScore *float64 `json:"min_score"` +} + +// compositeWindow uses a Starlark keyword and a quoted name as nested members. +type compositeWindow struct { + // From is named after a reserved Starlark word. + From string `json:"from"` + + // UntilMS needs quoting in notation and bracket paths in diagnostics. + UntilMS int32 `json:"until-ms"` +} + +// compositeSignature is the exact generated signature for compositeInput. +const compositeSignature = "records.query(*, tags: list[str], " + + "filter: {owner: str, level: int, min_score: float | None}, labels: dict[str, int], " + + `window: {from: str, "until-ms": int} | None, payload: list[int], scores: list[float | None])` + +// compositeKeywords returns a valid composite call with selected arguments replaced. +func compositeKeywords(overrides map[string]starlark.Value) []starlark.Tuple { + filter := starlark.NewDict(2) + _ = filter.SetKey(starlark.String("owner"), starlark.String("ops")) + _ = filter.SetKey(starlark.String("level"), starlark.MakeInt(7)) + values := map[string]starlark.Value{ + "tags": starlark.NewList(nil), + "filter": filter, + "labels": starlark.NewDict(0), + "payload": starlark.NewList(nil), + "scores": starlark.NewList(nil), + } + maps.Copy(values, overrides) + kwargs := make([]starlark.Tuple, 0, len(values)) + for _, name := range []string{"tags", "filter", "labels", "window", "payload", "scores"} { + if value, ok := values[name]; ok { + kwargs = append(kwargs, keyword(name, value)) + } + } + return kwargs +} + +// starlarkDict builds a dict from alternating string keys and values. +func starlarkDict(t *testing.T, pairs ...any) *starlark.Dict { + t.Helper() + dict := starlark.NewDict(len(pairs) / 2) + for index := 0; index < len(pairs); index += 2 { + key, ok := pairs[index].(starlark.Value) + if !ok { + key = starlark.String(pairs[index].(string)) + } + require.NoError(t, dict.SetKey(key, pairs[index+1].(starlark.Value))) + } + return dict +} + +// TestCompositeInputDiscoveryNotation proves signatures and shapes describe nested inputs exactly. +func TestCompositeInputDiscoveryNotation(t *testing.T) { + plan, err := CompileFor[compositeInput, representativeOutput]() + require.NoError(t, err) + + assert.Equal(t, compositeSignature, plan.Signature("records.query")) + assert.Equal(t, []FieldShape{ + {Name: "tags", Type: "list[str]", Required: true}, + {Name: "filter", Type: "{owner: str, level: int, min_score: float | None}", Required: true}, + {Name: "labels", Type: "dict[str, int]", Required: true}, + {Name: "window", Type: `{from: str, "until-ms": int} | None`, Required: false}, + {Name: "payload", Type: "list[int]", Required: true}, + {Name: "scores", Type: "list[float | None]", Required: true}, + }, plan.InputShape()) +} + +// TestCompositeOutputNamesNeedOnlyBeJSONNames proves output members may use keywords and quoted names. +func TestCompositeOutputNamesNeedOnlyBeJSONNames(t *testing.T) { + plan, err := CompileFor[representativeInput, struct { + // From is named after a reserved Starlark word. + From string `json:"from"` + + // Window carries a non-identifier nested name. + Window compositeWindow `json:"time-window"` + }]() + require.NoError(t, err) + + assert.Equal(t, []FieldShape{ + {Name: "from", Type: "str", Required: true}, + {Name: "time-window", Type: `{from: str, "until-ms": int}`, Required: true}, + }, plan.OutputShape()) +} + +// TestCompositeInputsBindAndRebind proves worker binding and parent re-binding agree on nested values. +func TestCompositeInputsBindAndRebind(t *testing.T) { + plan, err := CompileFor[compositeInput, representativeOutput]() + require.NoError(t, err) + minScore := 2.0 + firstScore, thirdScore := float32(1.5), float32(2) + + tests := []struct { + // name identifies the call. + name string + + // kwargs is the Starlark call. + kwargs []starlark.Tuple + + // want is the exact typed input the handler receives. + want compositeInput + + // canonical is the exact authorization argument map. + canonical map[string]any + }{ + { + name: "every form populated", + kwargs: compositeKeywords(map[string]starlark.Value{ + "tags": starlark.Tuple{starlark.String("a"), starlark.String("b")}, + "filter": starlarkDict(t, + "owner", starlark.String("ops"), + "level", starlark.MakeInt(255), + "min_score", starlark.MakeInt(2), + ), + "labels": starlarkDict(t, "x", starlark.MakeInt(1)), + "window": starlarkDict(t, "from", starlark.String("monday"), "until-ms", starlark.MakeInt(-5)), + "payload": starlark.NewList([]starlark.Value{starlark.MakeInt(0), starlark.MakeInt(255)}), + "scores": starlark.NewList([]starlark.Value{ + starlark.Float(1.5), starlark.None, starlark.MakeInt(2), + }), + }), + want: compositeInput{ + Tags: []string{"a", "b"}, + Filter: compositeFilter{Owner: "ops", Level: 255, MinScore: &minScore}, + Labels: map[string]int64{"x": 1}, + Window: &compositeWindow{From: "monday", UntilMS: -5}, + Payload: []byte{0, 255}, + Scores: []*float32{&firstScore, nil, &thirdScore}, + }, + canonical: map[string]any{ + "tags": []any{"a", "b"}, + "filter": map[string]any{"owner": "ops", "level": int64(255), "min_score": 2.0}, + "labels": map[string]any{"x": int64(1)}, + "window": map[string]any{"from": "monday", "until-ms": int64(-5)}, + "payload": []any{int64(0), int64(255)}, + "scores": []any{1.5, nil, 2.0}, + }, + }, + { + name: "optional members omitted or None", + kwargs: compositeKeywords(map[string]starlark.Value{ + "filter": starlarkDict(t, + "owner", starlark.String("ops"), + "level", starlark.MakeInt(0), + "min_score", starlark.None, + ), + "window": starlark.None, + }), + want: compositeInput{ + Tags: []string{}, + Filter: compositeFilter{Owner: "ops"}, + Labels: map[string]int64{}, + Payload: []byte{}, + Scores: []*float32{}, + }, + canonical: map[string]any{ + "tags": []any{}, + "filter": map[string]any{"owner": "ops", "level": int64(0)}, + "labels": map[string]any{}, + "payload": []any{}, + "scores": []any{}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + child, err := plan.InputSchema().Bind(nil, tt.kwargs, testValueDepth, testValueNodes) + require.NoError(t, err) + assert.Equal(t, tt.canonical, child) + + bound, canonical, err := plan.BindValue(child) + require.NoError(t, err) + assert.Equal(t, tt.want, bound) + assert.Equal(t, tt.canonical, canonical) + + child["filter"].(map[string]any)["owner"] = "mutated" + child["tags"] = []any{"mutated"} + assert.Equal(t, tt.canonical, canonical, "canonical arguments must share no container with the child map") + }) + } +} + +// TestCompositeInputDiagnostics proves every nested rejection names the exact argument path. +func TestCompositeInputDiagnostics(t *testing.T) { + plan, err := CompileFor[compositeInput, representativeOutput]() + require.NoError(t, err) + filter := func(pairs ...any) *starlark.Dict { + return starlarkDict(t, append([]any{"owner", starlark.String("o"), "level", starlark.MakeInt(1)}, pairs...)...) + } + + tests := []struct { + // name identifies the rejected call. + name string + + // overrides replaces arguments in the valid base call. + overrides map[string]starlark.Value + + // want is the exact model-facing error text. + want string + }{ + {name: "nested type", overrides: map[string]starlark.Value{ + "filter": starlarkDict(t, "owner", starlark.MakeInt(1), "level", starlark.MakeInt(1)), + }, want: `argument "filter.owner" must be a string`}, + {name: "list element", overrides: map[string]starlark.Value{ + "tags": starlark.NewList([]starlark.Value{starlark.String("a"), starlark.MakeInt(2)}), + }, want: `argument "tags[1]" must be a string`}, + {name: "string for list", overrides: map[string]starlark.Value{ + "tags": starlark.String("ab"), + }, want: `argument "tags" must be a list`}, + {name: "list for struct", overrides: map[string]starlark.Value{ + "filter": starlark.NewList(nil), + }, want: `argument "filter" must be a dict`}, + {name: "unknown nested key", overrides: map[string]starlark.Value{ + "filter": filter("bogus", starlark.MakeInt(1)), + }, want: `unknown argument "filter.bogus"`}, + {name: "missing nested key", overrides: map[string]starlark.Value{ + "filter": starlarkDict(t, "owner", starlark.String("o")), + }, want: `missing required argument "filter.level"`}, + {name: "missing quoted nested key", overrides: map[string]starlark.Value{ + "window": starlarkDict(t, "from", starlark.String("m")), + }, want: `missing required argument "window['until-ms']"`}, + {name: "map value", overrides: map[string]starlark.Value{ + "labels": starlarkDict(t, "env", starlark.None), + }, want: `argument "labels['env']" must be an integer`}, + {name: "non-string map key", overrides: map[string]starlark.Value{ + "labels": starlarkDict(t, starlark.MakeInt(1), starlark.MakeInt(2)), + }, want: `argument "labels" keys must be strings`}, + {name: "optional nested mismatch", overrides: map[string]starlark.Value{ + "filter": filter("min_score", starlark.String("x")), + }, want: `argument "filter.min_score" must be a float or None`}, + {name: "optional struct mismatch", overrides: map[string]starlark.Value{ + "window": starlark.NewList(nil), + }, want: `argument "window" must be a dict or None`}, + {name: "unsigned range", overrides: map[string]starlark.Value{ + "filter": starlarkDict(t, "owner", starlark.String("o"), "level", starlark.MakeInt(256)), + }, want: `argument "filter.level" must be between 0 and 255`}, + {name: "byte range", overrides: map[string]starlark.Value{ + "payload": starlark.NewList([]starlark.Value{starlark.MakeInt(-1)}), + }, want: `argument "payload[0]" must be between 0 and 255`}, + {name: "signed range", overrides: map[string]starlark.Value{ + "window": starlarkDict(t, "from", starlark.String("m"), "until-ms", starlark.MakeInt64(math.MaxInt32+1)), + }, want: `argument "window['until-ms']" must be between -2147483648 and 2147483647`}, + {name: "float32 range", overrides: map[string]starlark.Value{ + "scores": starlark.NewList([]starlark.Value{starlark.Float(1e39)}), + }, want: `argument "scores[0]" overflows float32`}, + {name: "non-finite float", overrides: map[string]starlark.Value{ + "filter": filter("min_score", starlark.Float(math.Inf(1))), + }, want: `argument "filter.min_score" is not finite`}, + {name: "float for unsigned", overrides: map[string]starlark.Value{ + "payload": starlark.NewList([]starlark.Value{starlark.Float(1)}), + }, want: `argument "payload[0]" must be an integer`}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := plan.InputSchema().Bind(nil, compositeKeywords(tt.overrides), testValueDepth, testValueNodes) + + require.ErrorIs(t, err, ErrInvalidArguments) + assert.EqualError(t, err, "invalid capability arguments: "+tt.want) + }) + } +} + +// TestBindValueRejectsCompositeWireMismatches proves the parent re-checks every nested value. +func TestBindValueRejectsCompositeWireMismatches(t *testing.T) { + plan, err := CompileFor[compositeInput, representativeOutput]() + require.NoError(t, err) + valid := func(overrides map[string]any) map[string]any { + arguments := map[string]any{ + "tags": []any{}, + "filter": map[string]any{"owner": "o", "level": int64(1)}, + "labels": map[string]any{}, + "payload": []any{}, + "scores": []any{}, + } + maps.Copy(arguments, overrides) + return arguments + } + + tests := []struct { + // name identifies the forged normalized map. + name string + + // arguments is the normalized map received from the worker. + arguments map[string]any + + // contains is the expected diagnostic fragment. + contains string + }{ + {name: "nested type", arguments: valid(map[string]any{ + "filter": map[string]any{"owner": int64(1), "level": int64(1)}, + }), contains: `"filter.owner" must be a string`}, + {name: "nested range", arguments: valid(map[string]any{ + "filter": map[string]any{"owner": "o", "level": int64(300)}, + }), contains: `"filter.level" must be between 0 and 255`}, + {name: "integer for float on the wire", arguments: valid(map[string]any{ + "filter": map[string]any{"owner": "o", "level": int64(1), "min_score": int64(2)}, + }), contains: `"filter.min_score" must be a float or None`}, + {name: "unknown nested key", arguments: valid(map[string]any{ + "filter": map[string]any{"owner": "o", "level": int64(1), "bogus": true}, + }), contains: `unknown argument "filter.bogus"`}, + {name: "missing nested key", arguments: valid(map[string]any{ + "filter": map[string]any{"owner": "o"}, + }), contains: `missing required argument "filter.level"`}, + {name: "non-neutral Go list", arguments: valid(map[string]any{ + "tags": []string{"a"}, + }), contains: `"tags" must be a list`}, + {name: "optional element type", arguments: valid(map[string]any{ + "scores": []any{"x"}, + }), contains: `"scores[0]" must be a float or None`}, + {name: "map value type", arguments: valid(map[string]any{ + "labels": map[string]any{"x": "y"}, + }), contains: `"labels['x']" must be an integer`}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, _, err := plan.BindValue(tt.arguments) + + require.ErrorIs(t, err, ErrInvalidArguments) + assert.Contains(t, err.Error(), tt.contains) + }) + } +} + +// TestInputSchemaSurvivesTheWorkerManifestEncoding proves JSON framing preserves every schema field. +func TestInputSchemaSurvivesTheWorkerManifestEncoding(t *testing.T) { + plan, err := CompileFor[compositeInput, representativeOutput]() + require.NoError(t, err) + schema := plan.InputSchema() + + encoded, err := json.Marshal(schema) + require.NoError(t, err) + var decoded InputSchema + require.NoError(t, json.Unmarshal(encoded, &decoded)) + + assert.Equal(t, schema, decoded) + require.NoError(t, decoded.Validate()) + _, err = decoded.Bind(nil, compositeKeywords(map[string]starlark.Value{ + "payload": starlark.NewList([]starlark.Value{starlark.MakeInt(256)}), + }), testValueDepth, testValueNodes) + require.EqualError(t, err, `invalid capability arguments: argument "payload[0]" must be between 0 and 255`) +} + +// TestInputSchemaValidateAcceptsCompiledSchemas proves every compiled schema validates. +func TestInputSchemaValidateAcceptsCompiledSchemas(t *testing.T) { + representative, err := CompileFor[representativeInput, representativeOutput]() + require.NoError(t, err) + empty, err := CompileFor[struct{}, representativeOutput]() + require.NoError(t, err) + widened, err := CompileFor[widenedInput, representativeOutput]() + require.NoError(t, err) + composite, err := CompileFor[compositeInput, representativeOutput]() + require.NoError(t, err) + + for _, plan := range []*Plan{representative, empty, widened, composite} { + require.NoError(t, plan.InputSchema().Validate()) + } +} + +// TestInputSchemaValidateRejectsMalformedArenas proves a forged manifest cannot smuggle an invalid schema. +func TestInputSchemaValidateRejectsMalformedArenas(t *testing.T) { + str := InputNode{Kind: InputString} + root := func(fields ...InputField) InputNode { return InputNode{Kind: InputStruct, Fields: fields} } + + tests := []struct { + // name identifies the malformed schema. + name string + + // schema is the candidate arena. + schema InputSchema + + // contains is the expected diagnostic fragment. + contains string + }{ + {name: "root out of range", schema: InputSchema{Root: 1, Nodes: []InputNode{root()}}, + contains: "root must be a struct"}, + {name: "root not a struct", schema: InputSchema{Nodes: []InputNode{str}}, + contains: "root must be a struct"}, + {name: "forward element", schema: InputSchema{Root: 2, Nodes: []InputNode{ + {Kind: InputList, Elem: 1}, str, root(InputField{Name: "a", Node: 0}), + }}, contains: "invalid element"}, + {name: "self element", schema: InputSchema{Root: 1, Nodes: []InputNode{ + {Kind: InputOptional}, root(InputField{Name: "a", Node: 0}), + }}, contains: "invalid element"}, + {name: "optional wraps optional", schema: InputSchema{Root: 3, Nodes: []InputNode{ + str, {Kind: InputOptional}, {Kind: InputOptional, Elem: 1}, root(InputField{Name: "a", Node: 2}), + }}, contains: "nests optional"}, + {name: "unknown kind", schema: InputSchema{Root: 1, Nodes: []InputNode{ + {Kind: "tuple"}, root(InputField{Name: "a", Node: 0}), + }}, contains: "unknown kind"}, + {name: "scalar with children", schema: InputSchema{Root: 1, Nodes: []InputNode{ + {Kind: InputString, Fields: []InputField{}}, root(InputField{Name: "a", Node: 0}), + }}, contains: "has children"}, + {name: "bounds on a string", schema: InputSchema{Root: 1, Nodes: []InputNode{ + {Kind: InputString, IntMax: 1}, root(InputField{Name: "a", Node: 0}), + }}, contains: "integer bounds"}, + {name: "empty integer range", schema: InputSchema{Root: 1, Nodes: []InputNode{ + {Kind: InputInt, IntMin: 5, IntMax: 1}, root(InputField{Name: "a", Node: 0}), + }}, contains: "empty integer range"}, + {name: "float32 flag on an integer", schema: InputSchema{Root: 1, Nodes: []InputNode{ + {Kind: InputInt, Float32: true}, root(InputField{Name: "a", Node: 0}), + }}, contains: "float32 flag"}, + {name: "keyword root name", schema: InputSchema{Root: 1, Nodes: []InputNode{ + str, root(InputField{Name: "from", Node: 0}), + }}, contains: "Starlark identifier"}, + {name: "invalid nested name", schema: InputSchema{Root: 2, Nodes: []InputNode{ + str, root(InputField{Name: "a'b", Node: 0}), root(InputField{Name: "a", Node: 1}), + }}, contains: "not a valid JSON field name"}, + {name: "duplicate name", schema: InputSchema{Root: 1, Nodes: []InputNode{ + str, root(InputField{Name: "a", Node: 0}, InputField{Name: "a", Node: 0}), + }}, contains: "duplicate input name"}, + {name: "forward field node", schema: InputSchema{Root: 0, Nodes: []InputNode{ + root(InputField{Name: "a", Node: 1}), str, + }}, contains: "invalid node"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.schema.Validate() + + require.ErrorIs(t, err, ErrInvalidPlan) + assert.Contains(t, err.Error(), tt.contains) + }) + } +} diff --git a/internal/binding/doc.go b/internal/binding/doc.go index c2ac84c..dbc3e63 100644 --- a/internal/binding/doc.go +++ b/internal/binding/doc.go @@ -1,11 +1,24 @@ // Package binding compiles restricted Go input and output types into immutable // conversion plans and owns process-neutral value conversion. // +// One shared type compiler builds a post-order node arena for each direction. +// Inputs and outputs accept the same recursive structural universe (scalars of +// every integer and float width, pointers, slices, string-keyed maps, and +// nested structs) with direction-specific exceptions: inputs reject arrays, +// pointer-to-pointer, and custom unmarshalers; outputs reject custom +// marshalers. Root input field names must be Starlark identifiers because they +// are keyword arguments; every other field name only needs to be a valid JSON +// field name. +// +// The input arena projects to InputSchema, a process-neutral description the +// worker validates once and uses to bind Starlark keyword arguments. The +// parent re-binds the worker's canonical map against the same arena to build +// the typed handler input and the authoritative canonical arguments. +// // Typed handler values, Starlark values, and JSON-shaped maps share one allowed // matrix: nil, bool, string, int64, finite float64, []any, and map[string]any. // ValidateValue, FromStarlark, and ToStarlark enforce that matrix plus positive // depth and materialization limits. [json.Number] and other numeric types are -// rejected. Plan.InputShape remains the only descriptor source for compiled -// input fields. Plan.OutputShape remains a flat FieldShape slice whose Type -// strings carry nested list, dict, struct, and nullable notation. +// rejected. Plan.InputShape and Plan.OutputShape remain flat FieldShape slices +// whose Type strings carry nested list, dict, struct, and nullable notation. package binding diff --git a/internal/binding/input.go b/internal/binding/input.go index 0bffdf7..4356c55 100644 --- a/internal/binding/input.go +++ b/internal/binding/input.go @@ -3,321 +3,184 @@ package binding import ( "fmt" "reflect" - - "go.starlark.net/starlark" ) -const keywordTupleSize = 2 - -// BindShape binds keyword-only Starlark arguments against a validated input shape. -// -// It returns only a fresh canonical JSON-shaped map. Explicit Starlark None and -// omission both leave an optional key absent. Callers must not treat the result -// as the authoritative authorization map; parent re-binding constructs that. -func BindShape(fields []FieldShape, args starlark.Tuple, kwargs []starlark.Tuple) (map[string]any, error) { - if err := ValidateInputShape(fields); err != nil { - return nil, err - } - if len(args) != 0 { - return nil, fmt.Errorf("%w: positional arguments are not accepted", ErrInvalidArguments) - } +// argumentRebinder carries one parent-side re-binding's plan and diagnostic path. +type argumentRebinder struct { + // plan supplies the schema and index-aligned Go type arena. + plan *Plan - byName := make(map[string]FieldShape, len(fields)) - for _, field := range fields { - byName[field.Name] = field - } - canonical := make(map[string]any, len(kwargs)) - seen := make(map[string]struct{}, len(kwargs)) - for _, keyword := range kwargs { - name, value, err := keywordArgument(keyword) - if err != nil { - return nil, err - } - field, known := byName[name] - if !known { - return nil, fmt.Errorf("%w: unknown argument %q", ErrInvalidArguments, name) - } - if _, duplicate := seen[name]; duplicate { - return nil, fmt.Errorf("%w: duplicate argument %q", ErrInvalidArguments, name) - } - seen[name] = struct{}{} - converted, omitted, err := convertShapeArgument(field, name, value) - if err != nil { - return nil, err - } - if !omitted { - canonical[name] = converted - } - } - for _, field := range fields { - if !field.Required { - continue - } - if _, ok := seen[field.Name]; !ok { - return nil, fmt.Errorf("%w: missing required argument %q", ErrInvalidArguments, field.Name) - } - } - return canonical, nil + // path is the active diagnostic path, rendered only on error. + path argumentPath } // BindValue reconstructs the exact registered Go input and a fresh canonical map. // -// The decoded child map is never returned or retained. A present optional key -// with a nil value is treated as explicit None and omitted from the canonical -// map. Invalid normalized maps are classified with ErrInvalidArguments. +// arguments is the normalized map produced by worker-side binding. The parent +// re-checks every value against the compiled schema, builds the typed input by +// reflection, and returns a canonical copy that shares no container with +// arguments. An optional struct member that is absent or nil stays a nil +// pointer and is absent from the canonical map. Work is linear in the size of +// arguments: diagnostic paths are rendered only on error. Invalid maps are +// classified with ErrInvalidArguments. func (plan *Plan) BindValue(arguments map[string]any) (any, map[string]any, error) { if plan == nil { return nil, nil, fmt.Errorf("%w: nil plan", ErrInvalidPlan) } - + rebinder := argumentRebinder{plan: plan, path: newArgumentPath()} input := reflect.New(plan.inputType).Elem() - canonical := make(map[string]any, len(plan.inputFields)) - for _, field := range plan.inputFields { - value, present := arguments[field.name] - converted, omitted, err := convertCanonicalArgument(field, present, value) - if err != nil { - return nil, nil, err - } - if omitted { - continue - } - if err := assignInputField(input.Field(field.index), field, converted); err != nil { - return nil, nil, err - } - canonical[field.name] = converted - } - for name := range arguments { - if _, known := plan.inputByName[name]; !known { - return nil, nil, fmt.Errorf("%w: unknown argument %q", ErrInvalidArguments, name) - } + canonical, err := rebinder.rebindStruct(plan.inputSchema.Root, input, arguments) + if err != nil { + return nil, nil, err } return input.Interface(), canonical, nil } -// keywordArgument extracts the string name and value from one keyword tuple. -func keywordArgument(keyword starlark.Tuple) (string, starlark.Value, error) { - if len(keyword) != keywordTupleSize { - return "", nil, fmt.Errorf("%w: malformed keyword argument", ErrInvalidArguments) - } - name, ok := starlark.AsString(keyword[0]) - if !ok { - return "", nil, fmt.Errorf("%w: keyword name must be a string", ErrInvalidArguments) - } - return name, keyword[1], nil -} - -// convertShapeArgument converts one Starlark argument against an exact field shape. -func convertShapeArgument(field FieldShape, name string, value starlark.Value) (any, bool, error) { - switch { - case isRequiredStringShape(field): - return convertStarlarkString(name, value, false) - case isOptionalStringShape(field): - if value == starlark.None { - return nil, true, nil - } - return convertStarlarkString(name, value, true) - case isRequiredIntegerShape(field): - return convertStarlarkInteger(name, value, false) - case isOptionalIntegerShape(field): - if value == starlark.None { - return nil, true, nil - } - return convertStarlarkInteger(name, value, true) - case isRequiredBoolShape(field): - return convertStarlarkBool(name, value, false) - case isOptionalBoolShape(field): - if value == starlark.None { - return nil, true, nil - } - return convertStarlarkBool(name, value, true) - case isRequiredFloatShape(field): - return convertStarlarkFloat(name, value, false) - case isOptionalFloatShape(field): - if value == starlark.None { - return nil, true, nil - } - return convertStarlarkFloat(name, value, true) - default: - return nil, false, fmt.Errorf("%w: argument %q has an unsupported shape", ErrInvalidPlan, name) - } -} - -// convertCanonicalArgument converts one normalized parent argument against a compiled field. -func convertCanonicalArgument(field inputField, present bool, value any) (any, bool, error) { - if field.required && !present { - return nil, false, fmt.Errorf("%w: missing required argument %q", ErrInvalidArguments, field.name) - } - if !field.required && (!present || value == nil) { - return nil, true, nil - } - switch field.kind { - case fieldString: - return convertCanonicalString(field.name, value, false) - case fieldOptionalString: - return convertCanonicalString(field.name, value, true) - case fieldInt64: - return convertCanonicalInteger(field.name, value, false) - case fieldOptionalInt64: - return convertCanonicalInteger(field.name, value, true) - case fieldBool: - return convertCanonicalBool(field.name, value, false) - case fieldOptionalBool: - return convertCanonicalBool(field.name, value, true) - case fieldFloat64: - return convertCanonicalFloat(field.name, value, false) - case fieldOptionalFloat64: - return convertCanonicalFloat(field.name, value, true) - default: - return nil, false, fmt.Errorf("%w: argument %q has an unknown compiled kind", ErrInvalidPlan, field.name) - } -} - -// assignInputField writes one converted scalar onto the reconstructed Go input. -func assignInputField(target reflect.Value, field inputField, converted any) error { - switch field.kind { - case fieldString, fieldOptionalString: - value, err := compiledScalar[string](converted, field.name) - if err != nil { - return err - } - return writeScalar(target, field.required, value, target.SetString) - case fieldInt64, fieldOptionalInt64: - value, err := compiledScalar[int64](converted, field.name) - if err != nil { - return err +// rebindStruct writes a normalized object onto a struct target and returns its canonical copy. +func (rebinder *argumentRebinder) rebindStruct( + index int, + target reflect.Value, + object map[string]any, +) (map[string]any, error) { + node := rebinder.plan.inputSchema.Nodes[index] + goFields := rebinder.plan.inputNodes[index].fields + canonical := make(map[string]any, len(node.Fields)) + for position, field := range node.Fields { + value, present := object[field.Name] + if rebinder.plan.inputSchema.Nodes[field.Node].Kind == InputOptional && (!present || value == nil) { + continue } - return writeScalar(target, field.required, value, target.SetInt) - case fieldBool, fieldOptionalBool: - value, err := compiledScalar[bool](converted, field.name) - if err != nil { - return err + rebinder.path.push(pathSegment{kind: segmentMember, name: field.Name}) + if !present { + return nil, fmt.Errorf("%w: missing required argument %q", ErrInvalidArguments, rebinder.path.String()) } - return writeScalar(target, field.required, value, target.SetBool) - case fieldFloat64, fieldOptionalFloat64: - value, err := compiledScalar[float64](converted, field.name) + converted, err := rebinder.rebindValue(field.Node, target.Field(goFields[position].index), value, false) if err != nil { - return err + return nil, err } - return writeScalar(target, field.required, value, target.SetFloat) - default: - return fmt.Errorf("%w: argument %q has an unknown compiled kind", ErrInvalidPlan, field.name) - } -} - -// compiledScalar reports an impossible compiled/value mismatch as an invalid plan. -func compiledScalar[T any](converted any, name string) (T, error) { - typed, ok := converted.(T) - if !ok { - var zero T - return zero, fmt.Errorf("%w: argument %q has an invalid compiled value", ErrInvalidPlan, name) - } - return typed, nil -} - -// writeScalar stores a required scalar or a non-nil optional pointer. -func writeScalar[T any](target reflect.Value, required bool, value T, set func(T)) error { - if required { - set(value) - return nil - } - assignOptionalScalar(target, value) - return nil -} - -// assignOptionalScalar stores a non-nil pointer to converted on target. -func assignOptionalScalar(target reflect.Value, converted any) { - pointer := reflect.New(target.Type().Elem()) - pointer.Elem().Set(reflect.ValueOf(converted).Convert(target.Type().Elem())) - target.Set(pointer) -} - -// convertStarlarkString converts a Starlark string argument. -func convertStarlarkString(name string, value starlark.Value, optional bool) (any, bool, error) { - converted, ok := starlark.AsString(value) - if !ok { - return nil, false, scalarArgumentError(name, "a string", optional) - } - return converted, false, nil -} - -// convertStarlarkInteger converts a Starlark integer that fits signed 64-bit range. -func convertStarlarkInteger(name string, value starlark.Value, optional bool) (any, bool, error) { - integer, ok := value.(starlark.Int) - if !ok { - return nil, false, scalarArgumentError(name, "an integer", optional) - } - converted, ok := integer.Int64() - if !ok { - return nil, false, fmt.Errorf("%w: argument %q overflows int64", ErrInvalidArguments, name) + rebinder.path.pop() + canonical[field.Name] = converted } - return converted, false, nil -} - -// convertStarlarkBool converts a Starlark Boolean argument. -func convertStarlarkBool(name string, value starlark.Value, optional bool) (any, bool, error) { - boolean, ok := value.(starlark.Bool) - if !ok { - return nil, false, scalarArgumentError(name, "a bool", optional) + if len(object) > len(canonical) { + for name := range object { + if fieldPosition(node, name) < 0 { + rebinder.path.push(pathSegment{kind: segmentMember, name: name}) + return nil, fmt.Errorf("%w: unknown argument %q", ErrInvalidArguments, rebinder.path.String()) + } + } } - return bool(boolean), false, nil + return canonical, nil } -// convertStarlarkFloat converts a finite Starlark floating-point argument. -func convertStarlarkFloat(name string, value starlark.Value, optional bool) (any, bool, error) { - number, ok := value.(starlark.Float) - if !ok { - return nil, false, scalarArgumentError(name, "a float", optional) - } - converted := float64(number) - if !isFiniteFloat(converted) { - return nil, false, fmt.Errorf("%w: argument %q is not finite", ErrInvalidArguments, name) +// rebindValue writes one normalized value onto target and returns its canonical copy. +// +// nullable reports that an enclosing optional node also accepts nil, which +// only changes the type-mismatch wording. +func (rebinder *argumentRebinder) rebindValue(index int, target reflect.Value, value any, nullable bool) (any, error) { + node := rebinder.plan.inputSchema.Nodes[index] + switch node.Kind { + case InputString: + if text, ok := value.(string); ok { + target.SetString(text) + return text, nil + } + case InputInt: + if integer, ok := value.(int64); ok { + return rebinder.rebindInteger(node, target, integer) + } + case InputBool: + if boolean, ok := value.(bool); ok { + target.SetBool(boolean) + return boolean, nil + } + case InputFloat: + if number, ok := value.(float64); ok { + checked, err := checkFloat(node, number, &rebinder.path) + if err != nil { + return nil, err + } + target.SetFloat(checked) + return checked, nil + } + case InputOptional: + return rebinder.rebindOptional(node, target, value) + case InputList: + if items, ok := value.([]any); ok { + return rebinder.rebindList(node, target, items) + } + case InputMap: + if object, ok := value.(map[string]any); ok { + return rebinder.rebindMap(node, target, object) + } + case InputStruct: + if object, ok := value.(map[string]any); ok { + return rebinder.rebindStruct(index, target, object) + } + } + return nil, typeError(node.Kind, &rebinder.path, nullable) +} + +// rebindInteger range-checks and stores a signed or unsigned integer. +func (rebinder *argumentRebinder) rebindInteger(node InputNode, target reflect.Value, value int64) (any, error) { + checked, err := checkInteger(node, value, &rebinder.path) + if err != nil { + return nil, err } - return converted, false, nil -} - -// convertCanonicalString converts a normalized string argument. -func convertCanonicalString(name string, value any, optional bool) (any, bool, error) { - converted, ok := value.(string) - if !ok { - return nil, false, scalarArgumentError(name, "a string", optional) + if target.CanUint() { + target.SetUint(uint64(checked)) //nolint:gosec // checkInteger bounds unsigned nodes to 0..IntMax. + } else { + target.SetInt(checked) } - return converted, false, nil + return checked, nil } -// convertCanonicalInteger converts a normalized signed 64-bit integer argument. -func convertCanonicalInteger(name string, value any, optional bool) (any, bool, error) { - converted, ok := value.(int64) - if !ok { - return nil, false, scalarArgumentError(name, "an integer", optional) +// rebindOptional stores nil or a freshly allocated pointer to the element value. +func (rebinder *argumentRebinder) rebindOptional(node InputNode, target reflect.Value, value any) (any, error) { + if value == nil { + return nil, nil //nolint:nilnil // None maps to JSON null inside lists and dicts. } - return converted, false, nil -} - -// convertCanonicalBool converts a normalized Boolean argument. -func convertCanonicalBool(name string, value any, optional bool) (any, bool, error) { - converted, ok := value.(bool) - if !ok { - return nil, false, scalarArgumentError(name, "a bool", optional) + pointer := reflect.New(target.Type().Elem()) + converted, err := rebinder.rebindValue(node.Elem, pointer.Elem(), value, true) + if err != nil { + return nil, err } - return converted, false, nil + target.Set(pointer) + return converted, nil } -// convertCanonicalFloat converts a normalized finite floating-point argument. -func convertCanonicalFloat(name string, value any, optional bool) (any, bool, error) { - converted, ok := value.(float64) - if !ok { - return nil, false, scalarArgumentError(name, "a float", optional) - } - if !isFiniteFloat(converted) { - return nil, false, fmt.Errorf("%w: argument %q is not finite", ErrInvalidArguments, name) +// rebindList writes a normalized list onto a slice target. +func (rebinder *argumentRebinder) rebindList(node InputNode, target reflect.Value, items []any) (any, error) { + slice := reflect.MakeSlice(target.Type(), len(items), len(items)) + canonical := make([]any, len(items)) + for index, item := range items { + rebinder.path.push(pathSegment{kind: segmentIndex, index: index}) + converted, err := rebinder.rebindValue(node.Elem, slice.Index(index), item, false) + if err != nil { + return nil, err + } + rebinder.path.pop() + canonical[index] = converted } - return converted, false, nil + target.Set(slice) + return canonical, nil } -// scalarArgumentError classifies a mistyped scalar before authorization. -func scalarArgumentError(name string, expected string, optional bool) error { - if optional { - return fmt.Errorf("%w: argument %q must be %s or None", ErrInvalidArguments, name, expected) +// rebindMap writes a normalized string-keyed object onto a map target. +func (rebinder *argumentRebinder) rebindMap(node InputNode, target reflect.Value, object map[string]any) (any, error) { + mapType := target.Type() + result := reflect.MakeMapWithSize(mapType, len(object)) + canonical := make(map[string]any, len(object)) + for key, item := range object { + rebinder.path.push(pathSegment{kind: segmentKey, name: key}) + element := reflect.New(mapType.Elem()).Elem() + converted, err := rebinder.rebindValue(node.Elem, element, item, false) + if err != nil { + return nil, err + } + rebinder.path.pop() + result.SetMapIndex(reflect.ValueOf(key).Convert(mapType.Key()), element) + canonical[key] = converted } - return fmt.Errorf("%w: argument %q must be %s", ErrInvalidArguments, name, expected) + target.Set(result) + return canonical, nil } diff --git a/internal/binding/input_test.go b/internal/binding/input_test.go index 1498e96..345eb4b 100644 --- a/internal/binding/input_test.go +++ b/internal/binding/input_test.go @@ -59,7 +59,7 @@ func TestBindShapeAndBindValueAgreeOnSupportedCalls(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - child, err := BindShape(plan.InputShape(), nil, tt.kwargs) + child, err := plan.InputSchema().Bind(nil, tt.kwargs, testValueDepth, testValueNodes) require.NoError(t, err) assert.Equal(t, tt.canonical, child) @@ -170,7 +170,7 @@ func TestBindShapeAndBindValueAgreeOnWidenedScalars(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - child, err := BindShape(plan.InputShape(), nil, tt.kwargs) + child, err := plan.InputSchema().Bind(nil, tt.kwargs, testValueDepth, testValueNodes) require.NoError(t, err) assert.Equal(t, tt.canonical, child) @@ -215,7 +215,7 @@ func TestBindShapeAndBindValueAgreeOnNamedAliases(t *testing.T) { enabled := flag(true) weight := score(2.5) - child, err := BindShape(plan.InputShape(), nil, []starlark.Tuple{ + child, err := plan.InputSchema().Bind(nil, []starlark.Tuple{ keyword("org", starlark.String("meigma")), keyword("count", starlark.MakeInt64(3)), keyword("active", starlark.True), @@ -224,7 +224,7 @@ func TestBindShapeAndBindValueAgreeOnNamedAliases(t *testing.T) { keyword("limit", starlark.MakeInt64(25)), keyword("enabled", starlark.True), keyword("weight", starlark.Float(2.5)), - }) + }, testValueDepth, testValueNodes) require.NoError(t, err) bound, canonical, err := plan.BindValue(child) @@ -258,7 +258,7 @@ func TestBindShapeHandlesAnEmptyInputShape(t *testing.T) { plan, err := CompileFor[struct{}, representativeOutput]() require.NoError(t, err) - child, err := BindShape(plan.InputShape(), nil, nil) + child, err := plan.InputSchema().Bind(nil, nil, testValueDepth, testValueNodes) require.NoError(t, err) assert.Empty(t, child) @@ -341,7 +341,7 @@ func TestBindShapeRejectsMalformedArguments(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := BindShape(plan.InputShape(), tt.args, tt.kwargs) + _, err := plan.InputSchema().Bind(tt.args, tt.kwargs, testValueDepth, testValueNodes) require.Error(t, err) require.ErrorIs(t, err, ErrInvalidArguments) @@ -428,9 +428,9 @@ func TestBindShapeRejectsWidenedScalarMismatches(t *testing.T) { keyword("active", starlark.True), keyword("score", starlark.Float(1.5)), }, contains: "missing required"}, - {name: "integer supplied as float", kwargs: append(append([]starlark.Tuple{}, required[:2]...), + {name: "string supplied as float", kwargs: append(append([]starlark.Tuple{}, required[:2]...), keyword("active", starlark.True), - keyword("score", starlark.MakeInt64(1)), + keyword("score", starlark.String("1.5")), ), contains: "must be a float"}, {name: "float supplied as integer", kwargs: []starlark.Tuple{ keyword("org", starlark.String("meigma")), @@ -481,7 +481,7 @@ func TestBindShapeRejectsWidenedScalarMismatches(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := BindShape(plan.InputShape(), nil, tt.kwargs) + _, err := plan.InputSchema().Bind(nil, tt.kwargs, testValueDepth, testValueNodes) require.Error(t, err) require.ErrorIs(t, err, ErrInvalidArguments) diff --git a/internal/binding/names.go b/internal/binding/names.go new file mode 100644 index 0000000..e8bf3fa --- /dev/null +++ b/internal/binding/names.go @@ -0,0 +1,236 @@ +package binding + +import ( + "errors" + "fmt" + "reflect" + "strconv" + "strings" + "unicode" +) + +// tagOptions contains the supported JSON tag option state. +type tagOptions struct { + // omitempty reports whether the field may be omitted when absent. + omitempty bool +} + +// compileFieldName validates field visibility, embedding, tags, and the name rule. +// +// keyword selects the Starlark identifier rule for keyword arguments; otherwise +// the name only needs to be a valid JSON field name. +func compileFieldName(field reflect.StructField, keyword bool) (string, tagOptions, error) { + if field.Anonymous { + return "", tagOptions{}, errors.New("embedded fields are not supported") + } + if field.PkgPath != "" { + return "", tagOptions{}, errors.New("field must be exported") + } + + name, options, err := parseJSONTag(field) + if err != nil { + return "", tagOptions{}, err + } + if keyword && !ValidIdentifier(name) { + return "", tagOptions{}, fmt.Errorf("JSON name %q is not a Starlark identifier", name) + } + if !keyword && !validJSONName(name) { + return "", tagOptions{}, fmt.Errorf("JSON name %q is not a valid JSON field name", name) + } + return name, options, nil +} + +// parseJSONTag returns the field's JSON name and options from its only struct tag. +func parseJSONTag(field reflect.StructField) (string, tagOptions, error) { + if field.Tag == "" { + return field.Name, tagOptions{}, nil + } + jsonTag, ok := field.Tag.Lookup("json") + if !ok || string(field.Tag) != fmt.Sprintf("json:%q", jsonTag) { + return "", tagOptions{}, errors.New("only one json struct tag is supported") + } + parts := strings.Split(jsonTag, ",") + if parts[0] == "-" { + return "", tagOptions{}, errors.New("ignored JSON fields are not supported") + } + name := field.Name + if parts[0] != "" { + name = parts[0] + } + var options tagOptions + for _, option := range parts[1:] { + switch option { + case "omitempty": + if options.omitempty { + return "", tagOptions{}, errors.New("duplicate omitempty option") + } + options.omitempty = true + case "": + return "", tagOptions{}, errors.New("empty JSON tag option") + default: + return "", tagOptions{}, fmt.Errorf("unsupported JSON tag option %q", option) + } + } + return name, options, nil +} + +// ValidIdentifier reports whether name is accepted as an identifier by the pinned Starlark scanner. +func ValidIdentifier(name string) bool { + return !isKeyword(name) && isPlainName(name) +} + +// isPlainName reports whether name has identifier syntax, ignoring reserved words. +func isPlainName(name string) bool { + if name == "" { + return false + } + for index, character := range name { + if character == '_' || unicode.IsLetter(character) || + index > 0 && character >= '0' && character <= '9' { + continue + } + return false + } + return true +} + +// validJSONName reports whether encoding/json accepts name as a struct tag name. +// +// This mirrors the unexported encoding/json isValidTag rule so every accepted +// name round-trips through standard JSON tooling unchanged. +func validJSONName(name string) bool { + if name == "" { + return false + } + for _, character := range name { + switch { + case strings.ContainsRune("!#$%&()*+-./:;<=>?@[]^_{|}~ ", character): + case !unicode.IsLetter(character) && !unicode.IsDigit(character): + return false + } + } + return true +} + +// isKeyword reports whether name is reserved by Starlark syntax. +func isKeyword(name string) bool { + switch name { + case "False", "None", "True", + "and", "as", "async", "await", "break", "class", "continue", "def", "del", "elif", "else", + "except", "finally", "for", "from", "global", "if", "import", "in", "is", "lambda", "load", //nolint:goconst // Scanner keyword list. + "nonlocal", "not", "or", "pass", "raise", "return", "try", "while", "with", "yield": + return true + default: + return false + } +} + +// notationName renders a struct member name inside type notation. +// +// Plain names render bare, including Starlark keywords; any other name is +// double-quoted so separators inside the name cannot be misread. +func notationName(name string) string { + if isPlainName(name) { + return name + } + return strconv.Quote(name) +} + +// segmentKind identifies one diagnostic path segment form. +type segmentKind uint8 + +const ( + // segmentMember is a struct member or root keyword name. + segmentMember segmentKind = iota + 1 + + // segmentIndex is a list position. + segmentIndex + + // segmentKey is a dict key. + segmentKey +) + +// pathSegment is one step in a diagnostic argument path. +type pathSegment struct { + // kind selects how the segment renders. + kind segmentKind + + // name is the member name or dict key; it references caller-owned data. + name string + + // index is the list position for segmentIndex. + index int +} + +// pathStackHint is the initial segment capacity for typical argument nesting. +const pathStackHint = 8 + +// argumentPath is a reusable stack of diagnostic path segments. +// +// Binding pushes and pops segments without building strings; String renders +// the path only when an error needs it, so success costs no copying in +// proportion to model-controlled key lengths. +type argumentPath struct { + // segments are the active steps from the root keyword to the current value. + segments []pathSegment +} + +// newArgumentPath returns an empty path with preallocated segment capacity. +func newArgumentPath() argumentPath { + return argumentPath{segments: make([]pathSegment, 0, pathStackHint)} +} + +// push appends one segment. +func (path *argumentPath) push(segment pathSegment) { + path.segments = append(path.segments, segment) +} + +// pop removes the most recent segment. +func (path *argumentPath) pop() { + path.segments = path.segments[:len(path.segments)-1] +} + +// String renders the path as root.member[index]['key']. +// +// Plain member names use dots; other member names and every dict key use a +// single-quoted bracket so the segment stays readable after %q quoting. +func (path *argumentPath) String() string { + var rendered strings.Builder + for position, segment := range path.segments { + switch segment.kind { + case segmentMember: + switch { + case position == 0: + rendered.WriteString(segment.name) + case isPlainName(segment.name): + rendered.WriteByte('.') + rendered.WriteString(segment.name) + default: + writeKeySegment(&rendered, segment.name) + } + case segmentIndex: + rendered.WriteByte('[') + rendered.WriteString(strconv.Itoa(segment.index)) + rendered.WriteByte(']') + case segmentKey: + writeKeySegment(&rendered, segment.name) + } + } + return rendered.String() +} + +// writeKeySegment renders a dict key or non-plain member name as ['key']. +// +// Keys containing quotes, backslashes, or non-printable characters fall back +// to a Go-quoted ["key"] form. +func writeKeySegment(rendered *strings.Builder, key string) { + if strings.ContainsAny(key, `'\`) || !strconv.CanBackquote(key) { + rendered.WriteByte('[') + rendered.WriteString(strconv.Quote(key)) + rendered.WriteByte(']') + return + } + rendered.WriteString("['") + rendered.WriteString(key) + rendered.WriteString("']") +} diff --git a/internal/binding/output.go b/internal/binding/output.go index f285c9f..9c25547 100644 --- a/internal/binding/output.go +++ b/internal/binding/output.go @@ -48,40 +48,40 @@ func (plan *Plan) convertNode( ) (any, error) { node := plan.outputNodes[index] switch node.kind { - case outputNodePointer: + case nodePointer: return plan.convertPointer(node, value, path, depth, converter) - case outputNodeString: + case nodeString: if err := converter.consumeNode(depth); err != nil { return nil, err } return value.String(), nil - case outputNodeInt: + case nodeInt: if err := converter.consumeNode(depth); err != nil { return nil, err } return value.Int(), nil - case outputNodeUint: + case nodeUint: if err := converter.consumeNode(depth); err != nil { return nil, err } return convertOutputUint(value, path) - case outputNodeBool: + case nodeBool: if err := converter.consumeNode(depth); err != nil { return nil, err } return value.Bool(), nil - case outputNodeFloat: + case nodeFloat: if err := converter.consumeNode(depth); err != nil { return nil, err } return convertOutputFloat(value, path) - case outputNodeBytes: + case nodeBytes: return convertOutputBytes(value, path, depth, converter) - case outputNodeList: + case nodeList: return plan.convertList(node, value, path, depth, converter) - case outputNodeMap: + case nodeMap: return plan.convertMap(node, value, path, depth, converter) - case outputNodeStruct: + case nodeStruct: return plan.convertStruct(node, value, path, depth, converter) default: return nil, fmt.Errorf("%w: %s has an unknown compiled kind", ErrInvalidPlan, path) @@ -90,7 +90,7 @@ func (plan *Plan) convertNode( // convertPointer follows a pointer without adding depth and maps nil to None. func (plan *Plan) convertPointer( - node outputNode, + node typeNode, value reflect.Value, path string, depth int, @@ -107,7 +107,7 @@ func (plan *Plan) convertPointer( // convertStruct materializes included fields after preflighting their count. func (plan *Plan) convertStruct( - node outputNode, + node typeNode, value reflect.Value, path string, depth int, @@ -149,7 +149,7 @@ func (plan *Plan) convertStruct( // convertList materializes array and non-nil slice elements after preflight. func (plan *Plan) convertList( - node outputNode, + node typeNode, value reflect.Value, path string, depth int, @@ -187,7 +187,7 @@ func (plan *Plan) convertList( // convertMap materializes a string-keyed map in sorted key order after preflight. func (plan *Plan) convertMap( - node outputNode, + node typeNode, value reflect.Value, path string, depth int, @@ -274,7 +274,7 @@ func convertOutputFloat(value reflect.Value, path string) (float64, error) { } // omitOutputField reports whether a nil pointer+omitempty field is excluded. -func omitOutputField(field outputStructField, value reflect.Value) bool { +func omitOutputField(field structField, value reflect.Value) bool { return field.omitempty && value.Kind() == reflect.Pointer && value.IsNil() } diff --git a/internal/binding/plan.go b/internal/binding/plan.go index 53b8588..d39798d 100644 --- a/internal/binding/plan.go +++ b/internal/binding/plan.go @@ -1,13 +1,9 @@ package binding import ( - "encoding" - "encoding/json" "errors" "fmt" "reflect" - "strings" - "unicode" ) var ( @@ -24,102 +20,6 @@ var ( ErrValueLimit = errors.New("converted value limit exceeded") ) -// outputArenaHint is the initial compiled-output arena capacity for typical capability graphs. -const outputArenaHint = 8 - -// fieldKind identifies one supported input conversion. -type fieldKind uint8 - -const ( - fieldString fieldKind = iota + 1 - fieldInt64 - fieldBool - fieldFloat64 - fieldOptionalString - fieldOptionalInt64 - fieldOptionalBool - fieldOptionalFloat64 -) - -// outputNodeKind identifies one compiled output conversion node. -type outputNodeKind uint8 - -const ( - outputNodeString outputNodeKind = iota + 1 - outputNodeInt - outputNodeUint - outputNodeBool - outputNodeFloat - outputNodeBytes - outputNodeList - outputNodeMap - outputNodeStruct - outputNodePointer -) - -// inputField is one immutable input-field conversion step. -type inputField struct { - // name is the Starlark keyword and canonical authorization key. - name string - - // index is the direct field index in the non-embedded input struct. - index int - - // kind determines validation and direct conversion behavior. - kind fieldKind - - // required reports whether omission is invalid. - required bool -} - -// outputNode is one immutable compiled output conversion node. -type outputNode struct { - // kind selects the conversion and notation strategy. - kind outputNodeKind - - // elem is the child node index for pointers, lists, and maps. - elem int - - // fields are declaration-ordered struct members. - fields []outputStructField - - // notation is the exact model-facing type string for this node. - notation string -} - -// outputStructField is one compiled exported struct member. -type outputStructField struct { - // name is the JSON and Starlark field name. - name string - - // index is the direct field index on the struct type. - index int - - // node is the compiled field type. - node int - - // omitempty omits a nil pointer from the converted object. - omitempty bool -} - -// outputCompiler builds a flat immutable node arena with cycle detection. -type outputCompiler struct { - // nodes is the arena under construction. - nodes []outputNode - - // done maps a completed type onto its arena index. - done map[reflect.Type]int - - // active is the stack of types currently being compiled. - active map[reflect.Type]struct{} - - // jsonMarshaler is the json.Marshaler interface type used for method-set checks. - jsonMarshaler reflect.Type - - // textMarshaler is the encoding.TextMarshaler interface type used for method-set checks. - textMarshaler reflect.Type -} - // Plan is an immutable compiled input, output, signature, and canonical-argument plan. type Plan struct { // inputType is the exact non-pointer Go input struct type. @@ -128,17 +28,17 @@ type Plan struct { // outputType is the exact non-pointer Go output struct type. outputType reflect.Type - // inputFields preserves declaration order for binding and signatures. - inputFields []inputField + // inputNodes is the compiled input type arena, index-aligned with inputSchema.Nodes. + inputNodes []typeNode - // inputByName maps each accepted keyword to its field-plan index. - inputByName map[string]int + // inputSchema is the process-neutral projection of inputNodes shared with the worker. + inputSchema InputSchema // outputRoot is the arena index of the compiled root struct. outputRoot int // outputNodes is the immutable compiled output type arena. - outputNodes []outputNode + outputNodes []typeNode } // CompileFor compiles the exact generic input and output types once. @@ -148,21 +48,27 @@ func CompileFor[Input, Output any]() (*Plan, error) { // Compile creates an immutable restricted plan for exact input and output struct types. func Compile(inputType reflect.Type, outputType reflect.Type) (*Plan, error) { - inputFields, inputByName, err := compileInput(inputType) + if inputType == nil || inputType.Kind() != reflect.Struct { + return nil, fmt.Errorf("%w: input must be a non-pointer struct", ErrInvalidPlan) + } + inputRoot, inputNodes, err := compileType(directionInput, inputType) if err != nil { return nil, err } - root, nodes, err := compileOutput(outputType) + if outputType == nil || outputType.Kind() != reflect.Struct { + return nil, fmt.Errorf("%w: output must be a non-pointer struct", ErrInvalidPlan) + } + outputRoot, outputNodes, err := compileType(directionOutput, outputType) if err != nil { return nil, err } return &Plan{ inputType: inputType, outputType: outputType, - inputFields: inputFields, - inputByName: inputByName, - outputRoot: root, - outputNodes: nodes, + inputNodes: inputNodes, + inputSchema: projectInputSchema(inputRoot, inputNodes), + outputRoot: outputRoot, + outputNodes: outputNodes, }, nil } @@ -176,396 +82,7 @@ func (plan *Plan) OutputType() reflect.Type { return plan.outputType } -// compileInput validates and compiles the restricted input struct. -func compileInput(inputType reflect.Type) ([]inputField, map[string]int, error) { - if inputType == nil || inputType.Kind() != reflect.Struct { - return nil, nil, fmt.Errorf("%w: input must be a non-pointer struct", ErrInvalidPlan) - } - fields := make([]inputField, 0, inputType.NumField()) - byName := make(map[string]int, inputType.NumField()) - for index := range inputType.NumField() { - field := inputType.Field(index) - name, options, err := compileFieldName(field) - if err != nil { - return nil, nil, fmt.Errorf("%w: input field %s: %w", ErrInvalidPlan, field.Name, err) - } - if _, exists := byName[name]; exists { - return nil, nil, fmt.Errorf("%w: duplicate input name %q", ErrInvalidPlan, name) - } - - compiled := inputField{name: name, index: index} - kind, required, ok := compileInputKind(field.Type) - if !ok { - return nil, nil, fmt.Errorf("%w: input field %q has unsupported type %s", ErrInvalidPlan, name, field.Type) - } - if required && options.omitempty { - return nil, nil, fmt.Errorf("%w: required input %q cannot use omitempty", ErrInvalidPlan, name) - } - compiled.kind = kind - compiled.required = required - byName[name] = len(fields) - fields = append(fields, compiled) - } - return fields, byName, nil -} - -// compileInputKind maps one input field type onto a supported scalar conversion. -// -// Named aliases are accepted by underlying kind. Pointers to the four scalar -// kinds are optional; non-pointer scalars are required. -func compileInputKind(fieldType reflect.Type) (fieldKind, bool, bool) { - if fieldType.Kind() == reflect.Pointer { - elementKind := fieldType.Elem().Kind() - switch elementKind { //nolint:exhaustive // Unsupported pointer element kinds share registration-time rejection. - case reflect.String: - return fieldOptionalString, false, true - case reflect.Int64: - return fieldOptionalInt64, false, true - case reflect.Bool: - return fieldOptionalBool, false, true - case reflect.Float64: - return fieldOptionalFloat64, false, true - default: - return 0, false, false - } - } - switch fieldType.Kind() { //nolint:exhaustive // Unsupported reflect kinds share registration-time rejection. - case reflect.String: - return fieldString, true, true - case reflect.Int64: - return fieldInt64, true, true - case reflect.Bool: - return fieldBool, true, true - case reflect.Float64: - return fieldFloat64, true, true - default: - return 0, false, false - } -} - -// compileOutput validates and compiles the restricted output struct into an arena. -func compileOutput(outputType reflect.Type) (int, []outputNode, error) { - if outputType == nil || outputType.Kind() != reflect.Struct { - return 0, nil, fmt.Errorf("%w: output must be a non-pointer struct", ErrInvalidPlan) - } - compiler := outputCompiler{ - nodes: make([]outputNode, 0, outputArenaHint), - done: make(map[reflect.Type]int), - active: make(map[reflect.Type]struct{}), - jsonMarshaler: reflect.TypeFor[json.Marshaler](), - textMarshaler: reflect.TypeFor[encoding.TextMarshaler](), - } - root, err := compiler.compile(outputType, "") - if err != nil { - return 0, nil, err - } - return root, compiler.nodes, nil -} - -// compile returns the arena index for typ, reusing completed nodes and rejecting cycles. -func (compiler *outputCompiler) compile(typ reflect.Type, path string) (int, error) { - if typ == nil { - return 0, fmt.Errorf("%w: output field %q has unsupported type ", ErrInvalidPlan, path) - } - if index, ok := compiler.done[typ]; ok { - return index, nil - } - if _, exists := compiler.active[typ]; exists { - return 0, fmt.Errorf("%w: cyclic type at %q", ErrInvalidPlan, path) - } - compiler.active[typ] = struct{}{} - defer delete(compiler.active, typ) - - index, err := compiler.compileNew(typ, path) - if err != nil { - return 0, err - } - compiler.done[typ] = index - return index, nil -} - -// compileNew appends one newly compiled node for typ. -func (compiler *outputCompiler) compileNew(typ reflect.Type, path string) (int, error) { - if compiler.implementsMarshaler(typ) { - return 0, unsupportedOutputType(path, typ) - } - switch typ.Kind() { //nolint:exhaustive // Unsupported reflect kinds share registration-time rejection. - case reflect.String: - return compiler.append(outputNode{kind: outputNodeString, notation: stringType}), nil - case reflect.Bool: - return compiler.append(outputNode{kind: outputNodeBool, notation: boolType}), nil - case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: - return compiler.append(outputNode{kind: outputNodeInt, notation: integerType}), nil - case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - return compiler.append(outputNode{kind: outputNodeUint, notation: integerType}), nil - case reflect.Float32, reflect.Float64: - return compiler.append(outputNode{kind: outputNodeFloat, notation: floatType}), nil - case reflect.Pointer: - return compiler.compilePointer(typ, path) - case reflect.Slice: - return compiler.compileSlice(typ, path) - case reflect.Array: - return compiler.compileArray(typ, path) - case reflect.Map: - return compiler.compileMap(typ, path) - case reflect.Struct: - return compiler.compileStruct(typ, path) - default: - return 0, unsupportedOutputType(path, typ) - } -} - -// compilePointer compiles a pointer to a supported node. -func (compiler *outputCompiler) compilePointer(typ reflect.Type, path string) (int, error) { - elem, err := compiler.compile(typ.Elem(), path) - if err != nil { - return 0, err - } - return compiler.append(outputNode{ - kind: outputNodePointer, - elem: elem, - notation: pointerNotation(compiler.nodes[elem].notation), - }), nil -} - -// compileSlice compiles a slice, treating uint8 elements as integer byte lists. -func (compiler *outputCompiler) compileSlice(typ reflect.Type, path string) (int, error) { - if typ.Elem().Kind() == reflect.Uint8 && !compiler.implementsMarshaler(typ.Elem()) { - return compiler.append(outputNode{kind: outputNodeBytes, notation: listNotation(integerType)}), nil - } - elem, err := compiler.compile(typ.Elem(), path) - if err != nil { - return 0, err - } - return compiler.append(outputNode{ - kind: outputNodeList, - elem: elem, - notation: listNotation(compiler.nodes[elem].notation), - }), nil -} - -// compileArray compiles a fixed array, treating uint8 elements as integer byte lists. -func (compiler *outputCompiler) compileArray(typ reflect.Type, path string) (int, error) { - if typ.Elem().Kind() == reflect.Uint8 && !compiler.implementsMarshaler(typ.Elem()) { - return compiler.append(outputNode{kind: outputNodeBytes, notation: listNotation(integerType)}), nil - } - elem, err := compiler.compile(typ.Elem(), path) - if err != nil { - return 0, err - } - return compiler.append(outputNode{ - kind: outputNodeList, - elem: elem, - notation: listNotation(compiler.nodes[elem].notation), - }), nil -} - -// compileMap compiles a string-keyed map. -func (compiler *outputCompiler) compileMap(typ reflect.Type, path string) (int, error) { - keyType := typ.Key() - if compiler.implementsMarshaler(keyType) || keyType.Kind() != reflect.String { - if path == "" { - return 0, fmt.Errorf("%w: output type %s has unsupported map key type %s", ErrInvalidPlan, typ, keyType) - } - return 0, fmt.Errorf("%w: output field %q has unsupported map key type %s", ErrInvalidPlan, path, keyType) - } - elem, err := compiler.compile(typ.Elem(), path) - if err != nil { - return 0, err - } - return compiler.append(outputNode{ - kind: outputNodeMap, - elem: elem, - notation: mapNotation(compiler.nodes[elem].notation), - }), nil -} - -// compileStruct compiles exported non-embedded fields in declaration order. -func (compiler *outputCompiler) compileStruct(typ reflect.Type, path string) (int, error) { - fields := make([]outputStructField, 0, typ.NumField()) - seen := make(map[string]struct{}, typ.NumField()) - for index := range typ.NumField() { - field := typ.Field(index) - name, options, err := compileFieldName(field) - if err != nil { - return 0, fmt.Errorf("%w: output field %s: %w", ErrInvalidPlan, outputFieldPath(path, field.Name), err) - } - fieldPath := outputFieldPath(path, name) - if options.omitempty && field.Type.Kind() != reflect.Pointer { - return 0, fmt.Errorf("%w: output field %q cannot use omitempty", ErrInvalidPlan, fieldPath) - } - if _, exists := seen[name]; exists { - return 0, fmt.Errorf("%w: duplicate output name %q", ErrInvalidPlan, fieldPath) - } - node, err := compiler.compile(field.Type, fieldPath) - if err != nil { - return 0, err - } - seen[name] = struct{}{} - fields = append(fields, outputStructField{ - name: name, - index: index, - node: node, - omitempty: options.omitempty, - }) - } - return compiler.append(outputNode{ - kind: outputNodeStruct, - fields: fields, - notation: compiler.structNotation(fields), - }), nil -} - -// append stores node and returns its arena index. -func (compiler *outputCompiler) append(node outputNode) int { - compiler.nodes = append(compiler.nodes, node) - return len(compiler.nodes) - 1 -} - -// structNotation renders a declaration-ordered struct literal. -func (compiler *outputCompiler) structNotation(fields []outputStructField) string { - var notation strings.Builder - notation.WriteByte('{') - for index, field := range fields { - if index > 0 { - notation.WriteString(", ") - } - notation.WriteString(field.name) - if field.omitempty { - notation.WriteByte('?') - notation.WriteString(": ") - notation.WriteString(compiler.nodes[compiler.nodes[field.node].elem].notation) - continue - } - notation.WriteString(": ") - notation.WriteString(compiler.nodes[field.node].notation) - } - notation.WriteByte('}') - return notation.String() -} - -// implementsMarshaler reports whether typ or *typ implements a forbidden marshaler. -func (compiler *outputCompiler) implementsMarshaler(typ reflect.Type) bool { - if typ == nil { - return false - } - if typ.Implements(compiler.jsonMarshaler) || typ.Implements(compiler.textMarshaler) { - return true - } - if typ.Kind() == reflect.Pointer { - return false - } - pointer := reflect.PointerTo(typ) - return pointer.Implements(compiler.jsonMarshaler) || pointer.Implements(compiler.textMarshaler) -} - -// unsupportedOutputType classifies a rejected output type at path. -func unsupportedOutputType(path string, typ reflect.Type) error { - if path == "" { - return fmt.Errorf("%w: output type %s is unsupported", ErrInvalidPlan, typ) - } - return fmt.Errorf("%w: output field %q has unsupported type %s", ErrInvalidPlan, path, typ) -} - -// outputFieldPath joins a parent compile path with a JSON field name. -func outputFieldPath(parent string, name string) string { - if parent == "" { - return name - } - return parent + "/" + name -} - -// pointerNotation appends one nullable suffix. -func pointerNotation(elem string) string { - if strings.HasSuffix(elem, noneSuffix) { - return elem - } - return elem + noneSuffix -} - -// listNotation renders list[T]. -func listNotation(elem string) string { - return "list[" + elem + "]" -} - -// mapNotation renders dict[str, T]. -func mapNotation(elem string) string { - return "dict[str, " + elem + "]" -} - -// tagOptions contains the supported JSON tag option state. -type tagOptions struct { - // omitempty reports whether the field may be omitted when absent. - omitempty bool -} - -// compileFieldName validates field visibility, embedding, tags, and Starlark identifier syntax. -func compileFieldName(field reflect.StructField) (string, tagOptions, error) { - if field.Anonymous { - return "", tagOptions{}, errors.New("embedded fields are not supported") - } - if field.PkgPath != "" { - return "", tagOptions{}, errors.New("field must be exported") - } - - name := field.Name - var options tagOptions - if field.Tag != "" { - jsonTag, ok := field.Tag.Lookup("json") - if !ok || string(field.Tag) != fmt.Sprintf("json:%q", jsonTag) { - return "", tagOptions{}, errors.New("only one json struct tag is supported") - } - parts := strings.Split(jsonTag, ",") - if parts[0] == "-" { - return "", tagOptions{}, errors.New("ignored JSON fields are not supported") - } - if parts[0] != "" { - name = parts[0] - } - for _, option := range parts[1:] { - switch option { - case "omitempty": - if options.omitempty { - return "", tagOptions{}, errors.New("duplicate omitempty option") - } - options.omitempty = true - case "": - return "", tagOptions{}, errors.New("empty JSON tag option") - default: - return "", tagOptions{}, fmt.Errorf("unsupported JSON tag option %q", option) - } - } - } - if !ValidIdentifier(name) { - return "", tagOptions{}, fmt.Errorf("JSON name %q is not a Starlark identifier", name) - } - return name, options, nil -} - -// ValidIdentifier reports whether name is accepted as an identifier by the pinned Starlark scanner. -func ValidIdentifier(name string) bool { - if name == "" || isKeyword(name) { - return false - } - for index, character := range name { - if character == '_' || unicode.IsLetter(character) || - index > 0 && character >= '0' && character <= '9' { - continue - } - return false - } - return true -} - -// isKeyword reports whether name is reserved by Starlark syntax. -func isKeyword(name string) bool { - switch name { - case "False", "None", "True", - "and", "as", "async", "await", "break", "class", "continue", "def", "del", "elif", "else", - "except", "finally", "for", "from", "global", "if", "import", "in", "is", "lambda", "load", - "nonlocal", "not", "or", "pass", "raise", "return", "try", "while", "with", "yield": - return true - default: - return false - } +// InputSchema returns a fresh process-neutral schema for worker-side binding. +func (plan *Plan) InputSchema() InputSchema { + return plan.inputSchema.Clone() } diff --git a/internal/binding/plan_test.go b/internal/binding/plan_test.go index 5f2d060..dcf0364 100644 --- a/internal/binding/plan_test.go +++ b/internal/binding/plan_test.go @@ -3,6 +3,7 @@ package binding import ( "reflect" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -175,22 +176,48 @@ func TestCompileRejectsUnsupportedInputShapes(t *testing.T) { contains string }{ {name: "pointer input", inputType: reflect.TypeFor[*representativeInput](), contains: "non-pointer struct"}, - {name: "unsupported integer width", inputType: reflect.TypeOf(struct { + {name: "array", inputType: reflect.TypeOf(struct { // Value is the field under validation. - Value int32 `json:"value"` + Value [2]string `json:"value"` }{}), contains: "unsupported type"}, - {name: "unsupported unsigned integer", inputType: reflect.TypeOf(struct { + {name: "interface", inputType: reflect.TypeOf(struct { // Value is the field under validation. - Value uint64 `json:"value"` + Value any `json:"value"` }{}), contains: "unsupported type"}, - {name: "unsupported optional integer width", inputType: reflect.TypeOf(struct { + {name: "complex", inputType: reflect.TypeOf(struct { // Value is the field under validation. - Value *int `json:"value,omitempty"` + Value complex128 `json:"value"` }{}), contains: "unsupported type"}, - {name: "unsupported float width", inputType: reflect.TypeOf(struct { + {name: "pointer to pointer", inputType: reflect.TypeOf(struct { // Value is the field under validation. - Value float32 `json:"value"` + Value **string `json:"value"` }{}), contains: "unsupported type"}, + {name: "non-string map key", inputType: reflect.TypeOf(struct { + // Value is the field under validation. + Value map[int]string `json:"value"` + }{}), contains: "unsupported map key type"}, + {name: "recursive type", inputType: reflect.TypeOf(struct { + // Value is the field under validation. + Value directCycleOutput `json:"value"` + }{}), contains: "cyclic type"}, + {name: "text unmarshaler", inputType: reflect.TypeOf(struct { + // Value is the field under validation. + Value time.Time `json:"value"` + }{}), contains: "unsupported type"}, + {name: "nested invalid JSON name", inputType: reflect.TypeOf(struct { + // Value is the field under validation. + Value struct { + // Name carries a symbol, which encoding/json rejects as a tag name. + Name string `json:"a★b"` + } `json:"value"` + }{}), contains: "not a valid JSON field name"}, + {name: "nested required omitempty", inputType: reflect.TypeOf(struct { + // Value is the field under validation. + Value struct { + // Name is required but tagged omitempty. + Name string `json:"name,omitempty"` + } `json:"value"` + }{}), contains: "cannot use omitempty"}, {name: "embedded field", inputType: reflect.TypeFor[embeddedInput](), contains: "embedded fields"}, {name: "unexported field", inputType: reflect.TypeOf(struct { // value is intentionally unexported. diff --git a/internal/binding/schema.go b/internal/binding/schema.go new file mode 100644 index 0000000..46e561e --- /dev/null +++ b/internal/binding/schema.go @@ -0,0 +1,230 @@ +package binding + +import ( + "fmt" + "math" + "reflect" +) + +// maxIntegerBits is the widest Go integer; unsigned 64-bit inputs cap at [math.MaxInt64] on the wire. +const maxIntegerBits = 64 + +// InputKind identifies one node kind in a process-neutral input schema. +type InputKind string + +const ( + // InputString accepts a Starlark str. + InputString InputKind = "str" + + // InputInt accepts a Starlark int within the node's inclusive IntMin..IntMax range. + InputInt InputKind = "int" + + // InputBool accepts a Starlark bool. + InputBool InputKind = "bool" + + // InputFloat accepts a finite Starlark float or int; Float32 narrows the range. + InputFloat InputKind = "float" + + // InputList accepts a Starlark list or tuple of Elem values. + InputList InputKind = "list" + + // InputMap accepts a Starlark dict with str keys and Elem values. + InputMap InputKind = "dict" + + // InputStruct accepts a Starlark dict whose keys are a subset of the declared + // fields and include every required field. The root struct binds keyword arguments. + InputStruct InputKind = "struct" + + // InputOptional accepts None or an Elem value; a struct member may also be omitted. + InputOptional InputKind = "optional" +) + +// InputSchema is the process-neutral input description the worker uses to +// bind keyword arguments without access to Go types. +// +// Nodes form a post-order arena: every Elem and field Node index refers to an +// earlier node, which makes the graph acyclic by construction. Root is a +// struct node whose fields are the keyword arguments. +type InputSchema struct { + // Root is the arena index of the keyword-argument struct. + Root int `json:"root"` + + // Nodes is the post-order node arena. + Nodes []InputNode `json:"nodes"` +} + +// InputNode is one schema node. +type InputNode struct { + // Kind selects the accepted Starlark value shape. + Kind InputKind `json:"kind"` + + // Elem is the element node for list, dict, and optional kinds. + Elem int `json:"elem,omitempty"` + + // Fields are the declaration-ordered members of a struct node. + Fields []InputField `json:"fields,omitempty"` + + // IntMin is the inclusive lower bound of an int node. + IntMin int64 `json:"int_min,omitempty"` + + // IntMax is the inclusive upper bound of an int node. + IntMax int64 `json:"int_max,omitempty"` + + // Float32 reports that a float node must fit the float32 range. + Float32 bool `json:"float32,omitempty"` +} + +// InputField is one named struct member in an input schema. +type InputField struct { + // Name is the exact Starlark keyword or dict key. + Name string `json:"name"` + + // Node is the arena index of the field's type. + Node int `json:"node"` +} + +// projectInputSchema derives the process-neutral schema from a compiled input arena. +// +// The schema is index-aligned with nodes so parent re-binding can pair each +// schema node with its Go type. +func projectInputSchema(root int, nodes []typeNode) InputSchema { + schema := InputSchema{Root: root, Nodes: make([]InputNode, len(nodes))} + for index, node := range nodes { + schema.Nodes[index] = projectInputNode(node) + } + return schema +} + +// projectInputNode maps one compiled input node onto its schema node. +func projectInputNode(node typeNode) InputNode { + switch node.kind { + case nodeString: + return InputNode{Kind: InputString} + case nodeBool: + return InputNode{Kind: InputBool} + case nodeInt: + bits := node.typ.Bits() + return InputNode{ + Kind: InputInt, + IntMin: math.MinInt64 >> (maxIntegerBits - bits), + IntMax: math.MaxInt64 >> (maxIntegerBits - bits), + } + case nodeUint: + bits := node.typ.Bits() + if bits == maxIntegerBits { + return InputNode{Kind: InputInt, IntMax: math.MaxInt64} + } + return InputNode{Kind: InputInt, IntMax: 1<= len(schema.Nodes) || schema.Nodes[schema.Root].Kind != InputStruct { + return fmt.Errorf("%w: input schema root must be a struct node", ErrInvalidPlan) + } + for _, field := range schema.Nodes[schema.Root].Fields { + if !ValidIdentifier(field.Name) { + return fmt.Errorf("%w: input field %q is not a Starlark identifier", ErrInvalidPlan, field.Name) + } + } + for index, node := range schema.Nodes { + if err := validateInputNode(schema.Nodes, index, node); err != nil { + return err + } + } + return nil +} + +// validateInputNode checks one node's kind, bounds, back-references, and member names. +func validateInputNode(nodes []InputNode, index int, node InputNode) error { + if node.Kind != InputInt && (node.IntMin != 0 || node.IntMax != 0) { + return fmt.Errorf("%w: input node %d has integer bounds", ErrInvalidPlan, index) + } + if node.Kind != InputFloat && node.Float32 { + return fmt.Errorf("%w: input node %d has a float32 flag", ErrInvalidPlan, index) + } + switch node.Kind { + case InputString, InputBool, InputFloat, InputInt: + if node.Elem != 0 || node.Fields != nil { + return fmt.Errorf("%w: scalar input node %d has children", ErrInvalidPlan, index) + } + if node.Kind == InputInt && node.IntMin > node.IntMax { + return fmt.Errorf("%w: input node %d has an empty integer range", ErrInvalidPlan, index) + } + case InputList, InputMap, InputOptional: + if node.Elem < 0 || node.Elem >= index || node.Fields != nil { + return fmt.Errorf("%w: input node %d has an invalid element", ErrInvalidPlan, index) + } + if node.Kind == InputOptional && nodes[node.Elem].Kind == InputOptional { + return fmt.Errorf("%w: input node %d nests optional values", ErrInvalidPlan, index) + } + case InputStruct: + return validateInputStruct(index, node) + default: + return fmt.Errorf("%w: input node %d has unknown kind %q", ErrInvalidPlan, index, node.Kind) + } + return nil +} + +// validateInputStruct checks struct member names, uniqueness, and back-references. +func validateInputStruct(index int, node InputNode) error { + if node.Elem != 0 { + return fmt.Errorf("%w: struct input node %d has an element", ErrInvalidPlan, index) + } + seen := make(map[string]struct{}, len(node.Fields)) + for _, field := range node.Fields { + if !validJSONName(field.Name) { + return fmt.Errorf("%w: input field %q is not a valid JSON field name", ErrInvalidPlan, field.Name) + } + if _, duplicate := seen[field.Name]; duplicate { + return fmt.Errorf("%w: duplicate input name %q", ErrInvalidPlan, field.Name) + } + if field.Node < 0 || field.Node >= index { + return fmt.Errorf("%w: input field %q has an invalid node", ErrInvalidPlan, field.Name) + } + seen[field.Name] = struct{}{} + } + return nil +} + +// fieldPosition returns the index of name among a struct node's fields, or -1. +func fieldPosition(node InputNode, name string) int { + for position, field := range node.Fields { + if field.Name == name { + return position + } + } + return -1 +} diff --git a/internal/binding/signature.go b/internal/binding/signature.go index 3c06238..9ef2e42 100644 --- a/internal/binding/signature.go +++ b/internal/binding/signature.go @@ -1,17 +1,14 @@ package binding import ( - "fmt" "strings" ) const ( - unsupportedTypeSignature = "unsupported" - // stringType is the compact Starlark-facing notation for a string value. stringType = "str" - // integerType is the compact Starlark-facing notation for a signed integer. + // integerType is the compact Starlark-facing notation for an integer of any width. integerType = "int" // boolType is the compact Starlark-facing notation for a Boolean value. @@ -20,20 +17,8 @@ const ( // floatType is the compact Starlark-facing notation for a finite floating-point value. floatType = "float" - // noneSuffix is the exact InputShape suffix for an optional scalar. + // noneSuffix is the nullable notation suffix for optional values. noneSuffix = " | None" - - // optionalStringType is the exact InputShape notation for an optional string. - optionalStringType = stringType + noneSuffix - - // optionalIntegerType is the exact InputShape notation for an optional integer. - optionalIntegerType = integerType + noneSuffix - - // optionalBoolType is the exact InputShape notation for an optional Boolean. - optionalBoolType = boolType + noneSuffix - - // optionalFloatType is the exact InputShape notation for an optional float. - optionalFloatType = floatType + noneSuffix ) // FieldShape is one model-facing field in a supported capability input or output structure. @@ -53,29 +38,34 @@ func (plan *Plan) Signature(capabilityName string) string { var signature strings.Builder signature.WriteString(capabilityName) signature.WriteByte('(') - if len(plan.inputFields) > 0 { + fields := plan.inputNodes[plan.inputSchema.Root].fields + if len(fields) > 0 { signature.WriteString("*, ") - for index, field := range plan.inputFields { + for index, field := range fields { if index > 0 { signature.WriteString(", ") } signature.WriteString(field.name) signature.WriteString(": ") - signature.WriteString(inputKindSignature(field.kind)) + signature.WriteString(plan.inputNodes[field.node].notation) } } signature.WriteByte(')') return signature.String() } -// InputShape returns a fresh model-facing description of the compiled input fields. +// InputShape returns a fresh model-facing description of the root input fields. +// +// Type carries the full nested notation; Required is false only for pointer fields. func (plan *Plan) InputShape() []FieldShape { - shape := make([]FieldShape, len(plan.inputFields)) - for index, field := range plan.inputFields { + fields := plan.inputNodes[plan.inputSchema.Root].fields + shape := make([]FieldShape, len(fields)) + for index, field := range fields { + node := plan.inputNodes[field.node] shape[index] = FieldShape{ Name: field.name, - Type: inputKindSignature(field.kind), - Required: field.required, + Type: node.notation, + Required: node.kind != nodePointer, } } return shape @@ -92,7 +82,7 @@ func (plan *Plan) OutputShape() []FieldShape { } // outputFieldShape renders one root field's flat discovery descriptor. -func outputFieldShape(plan *Plan, field outputStructField) FieldShape { +func outputFieldShape(plan *Plan, field structField) FieldShape { if field.omitempty { return FieldShape{ Name: field.name, @@ -106,102 +96,3 @@ func outputFieldShape(plan *Plan, field outputStructField) FieldShape { Required: true, } } - -// ValidateInputShape reports whether fields is a combination Plan.InputShape can produce. -// -// Required scalars are exactly Type "str", "int", "bool", or "float" with -// Required true. Optional scalars are exactly those types with " | None" and -// Required false. An empty shape is valid. Names must be unique Starlark -// identifiers. Any other pair, including the literal "unsupported" notation, is -// rejected. -func ValidateInputShape(fields []FieldShape) error { - seen := make(map[string]struct{}, len(fields)) - for _, field := range fields { - if !ValidIdentifier(field.Name) { - return fmt.Errorf("%w: input field %q is not a Starlark identifier", ErrInvalidPlan, field.Name) - } - if _, exists := seen[field.Name]; exists { - return fmt.Errorf("%w: duplicate input name %q", ErrInvalidPlan, field.Name) - } - if !isSupportedInputShape(field) { - return fmt.Errorf("%w: input field %q has an unsupported shape", ErrInvalidPlan, field.Name) - } - seen[field.Name] = struct{}{} - } - return nil -} - -// isSupportedInputShape reports whether field is an exact compiled input pair. -func isSupportedInputShape(field FieldShape) bool { - return isRequiredStringShape(field) || - isRequiredIntegerShape(field) || - isRequiredBoolShape(field) || - isRequiredFloatShape(field) || - isOptionalStringShape(field) || - isOptionalIntegerShape(field) || - isOptionalBoolShape(field) || - isOptionalFloatShape(field) -} - -// isRequiredStringShape reports whether field is a required string descriptor. -func isRequiredStringShape(field FieldShape) bool { - return field.Type == stringType && field.Required -} - -// isRequiredIntegerShape reports whether field is a required integer descriptor. -func isRequiredIntegerShape(field FieldShape) bool { - return field.Type == integerType && field.Required -} - -// isRequiredBoolShape reports whether field is a required Boolean descriptor. -func isRequiredBoolShape(field FieldShape) bool { - return field.Type == boolType && field.Required -} - -// isRequiredFloatShape reports whether field is a required float descriptor. -func isRequiredFloatShape(field FieldShape) bool { - return field.Type == floatType && field.Required -} - -// isOptionalStringShape reports whether field is an optional string descriptor. -func isOptionalStringShape(field FieldShape) bool { - return field.Type == optionalStringType && !field.Required -} - -// isOptionalIntegerShape reports whether field is an optional integer descriptor. -func isOptionalIntegerShape(field FieldShape) bool { - return field.Type == optionalIntegerType && !field.Required -} - -// isOptionalBoolShape reports whether field is an optional Boolean descriptor. -func isOptionalBoolShape(field FieldShape) bool { - return field.Type == optionalBoolType && !field.Required -} - -// isOptionalFloatShape reports whether field is an optional float descriptor. -func isOptionalFloatShape(field FieldShape) bool { - return field.Type == optionalFloatType && !field.Required -} - -// inputKindSignature returns the model-facing notation for one supported input conversion. -func inputKindSignature(kind fieldKind) string { - switch kind { - case fieldString: - return stringType - case fieldInt64: - return integerType - case fieldBool: - return boolType - case fieldFloat64: - return floatType - case fieldOptionalString: - return optionalStringType - case fieldOptionalInt64: - return optionalIntegerType - case fieldOptionalBool: - return optionalBoolType - case fieldOptionalFloat64: - return optionalFloatType - } - return unsupportedTypeSignature -} diff --git a/internal/binding/signature_test.go b/internal/binding/signature_test.go index 5570fda..9440915 100644 --- a/internal/binding/signature_test.go +++ b/internal/binding/signature_test.go @@ -92,189 +92,3 @@ func TestSignatureOmitsHostOutputTypeNames(t *testing.T) { }) } } - -// TestValidateInputShapeAcceptsCompiledAndEmptyShapes proves only InputShape combinations are valid. -func TestValidateInputShapeAcceptsCompiledAndEmptyShapes(t *testing.T) { - plan, err := CompileFor[representativeInput, representativeOutput]() - require.NoError(t, err) - empty, err := CompileFor[struct{}, representativeOutput]() - require.NoError(t, err) - - tests := []struct { - // name identifies the accepted descriptor. - name string - - // fields is the candidate input shape. - fields []FieldShape - }{ - {name: "compiled representative", fields: plan.InputShape()}, - {name: "empty compiled shape", fields: empty.InputShape()}, - {name: "nil empty shape", fields: nil}, - { - name: "required string", - fields: []FieldShape{{ - Name: "org", - Type: "str", - Required: true, - }}, - }, - { - name: "required integer", - fields: []FieldShape{{ - Name: "count", - Type: "int", - Required: true, - }}, - }, - { - name: "required bool", - fields: []FieldShape{{ - Name: "active", - Type: "bool", - Required: true, - }}, - }, - { - name: "required float", - fields: []FieldShape{{ - Name: "score", - Type: "float", - Required: true, - }}, - }, - { - name: "optional string", - fields: []FieldShape{{ - Name: "label", - Type: "str | None", - Required: false, - }}, - }, - { - name: "optional integer", - fields: []FieldShape{{ - Name: "limit", - Type: "int | None", - Required: false, - }}, - }, - { - name: "optional bool", - fields: []FieldShape{{ - Name: "enabled", - Type: "bool | None", - Required: false, - }}, - }, - { - name: "optional float", - fields: []FieldShape{{ - Name: "weight", - Type: "float | None", - Required: false, - }}, - }, - {name: "compiled widened", fields: mustCompileWidenedInputShape(t)}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - require.NoError(t, ValidateInputShape(tt.fields)) - }) - } -} - -// TestValidateInputShapeRejectsNonCompiledPairs proves the child cannot invent another descriptor matrix. -func TestValidateInputShapeRejectsNonCompiledPairs(t *testing.T) { - tests := []struct { - // name identifies the rejected descriptor. - name string - - // fields is the invalid input shape. - fields []FieldShape - - // contains is the expected diagnostic fragment. - contains string - }{ - { - name: "optional string Type/Required mismatch", - fields: []FieldShape{{ - Name: "org", - Type: "str", - Required: false, - }}, - contains: "unsupported shape", - }, - { - name: "required optional-integer Type/Required mismatch", - fields: []FieldShape{{ - Name: "limit", - Type: "int | None", - Required: true, - }}, - contains: "unsupported shape", - }, - { - name: "required bool marked optional", - fields: []FieldShape{{ - Name: "active", - Type: "bool", - Required: false, - }}, - contains: "unsupported shape", - }, - { - name: "optional float marked required", - fields: []FieldShape{{ - Name: "weight", - Type: "float | None", - Required: true, - }}, - contains: "unsupported shape", - }, - { - name: "unsupported notation", - fields: []FieldShape{{ - Name: "value", - Type: "unsupported", - Required: true, - }}, - contains: "unsupported shape", - }, - { - name: "duplicate name", - fields: []FieldShape{ - {Name: "org", Type: "str", Required: true}, - {Name: "org", Type: "int | None", Required: false}, - }, - contains: "duplicate input name", - }, - { - name: "invalid identifier", - fields: []FieldShape{{ - Name: "not-valid", - Type: "str", - Required: true, - }}, - contains: "Starlark identifier", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := ValidateInputShape(tt.fields) - - require.Error(t, err) - require.ErrorIs(t, err, ErrInvalidPlan) - assert.Contains(t, err.Error(), tt.contains) - }) - } -} - -// mustCompileWidenedInputShape returns the compiled eight-form input shape. -func mustCompileWidenedInputShape(t *testing.T) []FieldShape { - t.Helper() - plan, err := CompileFor[widenedInput, representativeOutput]() - require.NoError(t, err) - return plan.InputShape() -} diff --git a/internal/execution/detail_test.go b/internal/execution/detail_test.go index e6f751f..fe42fcf 100644 --- a/internal/execution/detail_test.go +++ b/internal/execution/detail_test.go @@ -83,7 +83,7 @@ func TestExecuteAttachesApprovedProgramDiagnostics(t *testing.T) { } } -// TestExecuteAttachesOneBindingDiagnostic proves BindShape suffixes omit the coarse prefix. +// TestExecuteAttachesOneBindingDiagnostic proves binding suffixes omit the coarse prefix. func TestExecuteAttachesOneBindingDiagnostic(t *testing.T) { var nativeCalls atomic.Int64 _, err := buildEngine(t).Execute( diff --git a/internal/execution/engine.go b/internal/execution/engine.go index dfbdc48..fec598d 100644 --- a/internal/execution/engine.go +++ b/internal/execution/engine.go @@ -20,8 +20,8 @@ type CapabilityBinding struct { // Name is the dotted Starlark path used to assemble the frozen namespace. Name string - // Input is the exact compiled input shape used to bind keyword arguments. - Input []binding.FieldShape + // Input is the private compiled input schema used to bind keyword arguments. + Input binding.InputSchema } // NativeCall invokes one capability with a fresh canonical argument map. @@ -52,12 +52,10 @@ func New(bindings []CapabilityBinding) (*Engine, error) { func copyBindings(bindings []CapabilityBinding) []CapabilityBinding { copied := make([]CapabilityBinding, len(bindings)) for index, capability := range bindings { - input := make([]binding.FieldShape, len(capability.Input)) - copy(input, capability.Input) copied[index] = CapabilityBinding{ ID: capability.ID, Name: capability.Name, - Input: input, + Input: capability.Input.Clone(), } } return copied @@ -84,7 +82,7 @@ func validateBindings(bindings []CapabilityBinding) error { if _, duplicate := names[capability.Name]; duplicate { return fmt.Errorf("%w: duplicate namespace function", ErrInternal) } - if err := binding.ValidateInputShape(capability.Input); err != nil { + if err := capability.Input.Validate(); err != nil { return fmt.Errorf("%w: %w", ErrInternal, err) } ids[capability.ID] = struct{}{} diff --git a/internal/execution/execute.go b/internal/execution/execute.go index 4a02ce5..70761aa 100644 --- a/internal/execution/execute.go +++ b/internal/execution/execute.go @@ -61,7 +61,9 @@ func execute( nativeCalls: checkedCounter{ maximum: limits.MaxNativeCalls, }, - call: wrapNativeCall(nativeCall, limits), + call: wrapNativeCall(nativeCall, limits), + maxValueDepth: limits.MaxValueDepth, + maxValueBytes: limits.MaxValueBytes, } thread := newThread(state, limits.MaxExecutionSteps) @@ -161,7 +163,7 @@ func requireMain(globals starlark.StringDict) (*starlark.Function, error) { func callCapability( thread *starlark.Thread, id string, - input []binding.FieldShape, + input binding.InputSchema, args starlark.Tuple, kwargs []starlark.Tuple, ) (starlark.Value, error) { @@ -175,12 +177,16 @@ func callCapability( if counterErr := state.nativeCalls.increment(); counterErr != nil { return nil, counterErr } - canonical, bindingErr := binding.BindShape(input, args, kwargs) + canonical, bindingErr := input.Bind(args, kwargs, state.maxValueDepth, state.maxValueBytes) if bindingErr != nil { - if errors.Is(bindingErr, binding.ErrInvalidArguments) { + switch { + case errors.Is(bindingErr, binding.ErrInvalidArguments): return nil, invalidArgumentDetail(bindingErr) + case errors.Is(bindingErr, binding.ErrValueLimit): + return nil, fmt.Errorf("%w: %w", ErrResourceLimit, bindingErr) + default: + return nil, fmt.Errorf("%w: %w", ErrInternal, bindingErr) } - return nil, fmt.Errorf("%w: %w", ErrInternal, bindingErr) } return state.call(id, canonical) } diff --git a/internal/execution/execute_test.go b/internal/execution/execute_test.go index 90e032c..854f45f 100644 --- a/internal/execution/execute_test.go +++ b/internal/execution/execute_test.go @@ -36,22 +36,7 @@ func TestNewRejectsInvalidBindings(t *testing.T) { }}, {name: "namespace collision", bindings: []execution.CapabilityBinding{ valid, - { - ID: "cap.detail", - Name: "records.lookup.detail", - Input: []binding.FieldShape{ - {Name: "value", Type: "str", Required: true}, - }, - }, - }}, - {name: "invalid input shape", bindings: []execution.CapabilityBinding{ - { - ID: "cap.lookup", - Name: "records.lookup", - Input: []binding.FieldShape{ - {Name: "value", Type: "unsupported", Required: true}, - }, - }, + withName(withID(valid, "cap.detail"), "records.lookup.detail"), }}, } @@ -59,6 +44,13 @@ func TestNewRejectsInvalidBindings(t *testing.T) { t.Run(tt.name, func(t *testing.T) { _, err := execution.New(tt.bindings) + require.ErrorIs(t, err, execution.ErrInternal) + }) + } + for _, malformed := range malformedInputSchemas() { + t.Run(malformed.name, func(t *testing.T) { + _, err := execution.New([]execution.CapabilityBinding{withInput(valid, malformed.schema)}) + require.ErrorIs(t, err, execution.ErrInternal) }) } @@ -69,7 +61,7 @@ func TestNewCopiesCallerBindings(t *testing.T) { capability := lookupBinding() engine, err := execution.New([]execution.CapabilityBinding{capability}) require.NoError(t, err) - capability.Input[0].Name = "mutated" + capability.Input.Nodes[capability.Input.Root].Fields[0].Name = "mutated" result, err := engine.Execute( `def main(): return records.lookup(value="alpha")`, @@ -182,6 +174,18 @@ def main(): "score": 1.5, }, }, + { + name: "integer supplied for float", + engine: buildWidenedEngine, + source: `def main(): return records.lookup(org="meigma", count=3, active=True, score=2, weight=4)`, + wantArguments: map[string]any{ + "org": "meigma", + "count": int64(3), + "active": true, + "score": 2.0, + "weight": 4.0, + }, + }, } for _, tt := range tests { @@ -443,11 +447,9 @@ func buildEngine(t *testing.T) *execution.Engine { // lookupBinding returns the representative records.lookup capability. func lookupBinding() execution.CapabilityBinding { return execution.CapabilityBinding{ - ID: "cap.lookup", - Name: "records.lookup", - Input: []binding.FieldShape{ - {Name: "value", Type: "str", Required: true}, - }, + ID: "cap.lookup", + Name: "records.lookup", + Input: mustInputSchema[valueInput](), } } @@ -462,18 +464,9 @@ func buildWidenedEngine(t *testing.T) *execution.Engine { // widenedLookupBinding returns records.lookup with every supported scalar input form. func widenedLookupBinding() execution.CapabilityBinding { return execution.CapabilityBinding{ - ID: "cap.lookup", - Name: "records.lookup", - Input: []binding.FieldShape{ - {Name: "org", Type: "str", Required: true}, - {Name: "count", Type: "int", Required: true}, - {Name: "active", Type: "bool", Required: true}, - {Name: "score", Type: "float", Required: true}, - {Name: "label", Type: "str | None", Required: false}, - {Name: "limit", Type: "int | None", Required: false}, - {Name: "enabled", Type: "bool | None", Required: false}, - {Name: "weight", Type: "float | None", Required: false}, - }, + ID: "cap.lookup", + Name: "records.lookup", + Input: mustInputSchema[widenedInput](), } } @@ -489,6 +482,12 @@ func withName(capability execution.CapabilityBinding, name string) execution.Cap return capability } +// withInput returns a copy of capability with a replacement input schema. +func withInput(capability execution.CapabilityBinding, input binding.InputSchema) execution.CapabilityBinding { + capability.Input = input + return capability +} + // echoNativeCall returns the bound value field as a normalized object. func echoNativeCall() execution.NativeCall { return func(_ string, arguments map[string]any) (any, error) { diff --git a/internal/execution/input_schema_test.go b/internal/execution/input_schema_test.go new file mode 100644 index 0000000..8ebf53d --- /dev/null +++ b/internal/execution/input_schema_test.go @@ -0,0 +1,319 @@ +package execution_test + +import ( + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/meigma/codemode/internal/binding" + "github.com/meigma/codemode/internal/execution" +) + +// valueInput is the single required string input of the representative capabilities. +type valueInput struct { + // Value is the required string argument. + Value string `json:"value"` +} + +// widenedInput exposes every required and optional scalar input form. +type widenedInput struct { + // Org is the required string argument. + Org string `json:"org"` + + // Count is the required signed integer argument. + Count int64 `json:"count"` + + // Active is the required Boolean argument. + Active bool `json:"active"` + + // Score is the required finite floating-point argument. + Score float64 `json:"score"` + + // Label is the optional string argument. + Label *string `json:"label,omitempty"` + + // Limit is the optional signed integer argument. + Limit *int64 `json:"limit,omitempty"` + + // Enabled is the optional Boolean argument. + Enabled *bool `json:"enabled,omitempty"` + + // Weight is the optional finite floating-point argument. + Weight *float64 `json:"weight,omitempty"` +} + +// searchFilter is the nested struct argument of the composite search capability. +type searchFilter struct { + // Owner is the required nested string member. + Owner string `json:"owner"` + + // Label is the optional nested string member. + Label *string `json:"label,omitempty"` +} + +// searchInput combines a list, a nested struct, and a string-keyed dict. +type searchInput struct { + // Tags is the required list of strings. + Tags []string `json:"tags"` + + // Filter is the required nested struct. + Filter searchFilter `json:"filter"` + + // Counts is the required string-keyed dict of integers. + Counts map[string]int64 `json:"counts"` +} + +// schemaOutput is the minimal output type paired with test inputs at compile time. +type schemaOutput struct { + // Value is a string output field. + Value string `json:"value"` +} + +// malformedInputSchema is one structurally invalid input schema fixture. +type malformedInputSchema struct { + // name identifies the structural defect. + name string + + // schema is the invalid input schema. + schema binding.InputSchema +} + +// TestExecuteBindsCompositeArguments proves nested arguments reach the native port as canonical maps. +func TestExecuteBindsCompositeArguments(t *testing.T) { + tests := []struct { + // name identifies the bound composite call. + name string + + // source is the Starlark program that performs one native call. + source string + + // wantArguments is the canonical map forwarded to the native port. + wantArguments map[string]any + }{ + { + name: "every nested member", + source: `def main(): return records.search(` + + `tags=["alpha", "beta"], filter={"owner": "meigma", "label": "beta"}, counts={"open": 3, "closed": 0})`, + wantArguments: map[string]any{ + "tags": []any{"alpha", "beta"}, + "filter": map[string]any{"owner": "meigma", "label": "beta"}, + "counts": map[string]any{"open": int64(3), "closed": int64(0)}, + }, + }, + { + name: "tuple list and omitted nested optional", + source: `def main(): return records.search(` + + `tags=("alpha",), filter={"owner": "meigma"}, counts={"open": 1})`, + wantArguments: map[string]any{ + "tags": []any{"alpha"}, + "filter": map[string]any{"owner": "meigma"}, + "counts": map[string]any{"open": int64(1)}, + }, + }, + { + name: "explicit None nested optional", + source: `def main(): return records.search(` + + `tags=["alpha"], filter={"owner": "meigma", "label": None}, counts={"open": 1})`, + wantArguments: map[string]any{ + "tags": []any{"alpha"}, + "filter": map[string]any{"owner": "meigma"}, + "counts": map[string]any{"open": int64(1)}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var gotID string + var gotArguments map[string]any + nativeCall := func(id string, arguments map[string]any) (any, error) { + gotID = id + gotArguments = arguments + return "ok", nil + } + result, err := buildSearchEngine(t).Execute(tt.source, nativeCall, defaultExecutionLimits()) + + require.NoError(t, err) + assert.Equal(t, "cap.search", gotID) + assert.Equal(t, tt.wantArguments, gotArguments) + assert.Equal(t, "ok", result) + }) + } +} + +// TestExecuteAttachesCompositeArgumentPaths proves nested binding failures carry the path as safe detail. +func TestExecuteAttachesCompositeArgumentPaths(t *testing.T) { + tests := []struct { + // name identifies the malformed composite call. + name string + + // source is the Starlark program that performs one native call. + source string + + // detail is the expected model-facing suffix. + detail string + }{ + { + name: "nested struct member type", + source: `def main(): return records.search(tags=["alpha"], filter={"owner": 1}, counts={})`, + detail: `argument "filter.owner" must be a string`, + }, + { + name: "nested optional member type", + source: `def main(): return records.search(tags=["alpha"], filter={"owner": "m", "label": 1}, counts={})`, + detail: `argument "filter.label" must be a string or None`, + }, + { + name: "list element type", + source: `def main(): return records.search(tags=["alpha", 2], filter={"owner": "m"}, counts={})`, + detail: `argument "tags[1]" must be a string`, + }, + { + name: "unknown nested member", + source: `def main(): return records.search(tags=[], filter={"owner": "m", "bogus": 1}, counts={})`, + detail: `unknown argument "filter.bogus"`, + }, + { + name: "missing nested member", + source: `def main(): return records.search(tags=[], filter={}, counts={})`, + detail: `missing required argument "filter.owner"`, + }, + { + name: "dict value overflow", + source: `def main(): return records.search(tags=[], filter={"owner": "m"}, counts={"open": 1 << 64})`, + detail: `argument "counts['open']" overflows int64`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var nativeCalls atomic.Int64 + _, err := buildSearchEngine( + t, + ).Execute(tt.source, countingNativeCall(&nativeCalls), defaultExecutionLimits()) + + require.ErrorIs(t, err, execution.ErrInvalidArguments) + assert.Equal(t, execution.ErrInvalidArguments.Error(), err.Error()) + detail, ok := execution.SafeDetail(err) + require.True(t, ok) + assert.Equal(t, tt.detail, detail) + assert.Zero(t, nativeCalls.Load()) + }) + } +} + +// TestExecuteBoundsCompositeArgumentsBeforeNativeCall proves oversized or aliased +// arguments fail as resource limits before materialization or dispatch. +func TestExecuteBoundsCompositeArgumentsBeforeNativeCall(t *testing.T) { + tests := []struct { + // name identifies the exhausted budget. + name string + + // source is the over-budget program. + source string + + // limits adjusts the default execution limits. + limits func(*execution.Limits) + }{ + { + name: "list exceeds node budget", + source: `def main(): return records.search(tags=["x"] * 100000, filter={"owner": "m"}, counts={})`, + limits: func(*execution.Limits) {}, + }, + { + name: "nesting exceeds depth", + source: `def main(): return records.search(tags=[], filter={"owner": "m"}, counts={"open": 1})`, + limits: func(limits *execution.Limits) { limits.MaxValueDepth = 2 }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + limits := defaultExecutionLimits() + tt.limits(&limits) + var nativeCalls atomic.Int64 + + _, err := buildSearchEngine(t).Execute(tt.source, countingNativeCall(&nativeCalls), limits) + + require.ErrorIs(t, err, execution.ErrResourceLimit) + assert.Zero(t, nativeCalls.Load()) + }) + } +} + +// buildSearchEngine creates one engine exposing the composite records.search capability. +func buildSearchEngine(t *testing.T) *execution.Engine { + t.Helper() + engine, err := execution.New([]execution.CapabilityBinding{{ + ID: "cap.search", + Name: "records.search", + Input: mustInputSchema[searchInput](), + }}) + require.NoError(t, err) + return engine +} + +// mustInputSchema compiles In as a capability input and returns its private schema. +// +// It panics when In is not a supported input type, which is a fixture bug. +func mustInputSchema[In any]() binding.InputSchema { + plan, err := binding.CompileFor[In, schemaOutput]() + if err != nil { + panic(err) + } + return plan.InputSchema() +} + +// malformedInputSchemas returns hand-built schemas that no compiled plan can produce. +func malformedInputSchemas() []malformedInputSchema { + str := binding.InputNode{Kind: binding.InputString} + return []malformedInputSchema{ + {name: "empty schema", schema: binding.InputSchema{}}, + {name: "root out of range", schema: binding.InputSchema{ + Root: 1, + Nodes: []binding.InputNode{{Kind: binding.InputStruct}}, + }}, + {name: "root not a struct", schema: binding.InputSchema{Nodes: []binding.InputNode{str}}}, + {name: "forward element index", schema: binding.InputSchema{Root: 2, Nodes: []binding.InputNode{ + {Kind: binding.InputList, Elem: 1}, + str, + {Kind: binding.InputStruct, Fields: []binding.InputField{{Name: "tags", Node: 0}}}, + }}}, + {name: "self element index", schema: binding.InputSchema{Root: 1, Nodes: []binding.InputNode{ + {Kind: binding.InputList, Elem: 0}, + {Kind: binding.InputStruct, Fields: []binding.InputField{{Name: "tags", Node: 0}}}, + }}}, + {name: "forward field index", schema: binding.InputSchema{Root: 0, Nodes: []binding.InputNode{ + {Kind: binding.InputStruct, Fields: []binding.InputField{{Name: "value", Node: 1}}}, + str, + }}}, + {name: "optional wrapping optional", schema: binding.InputSchema{Root: 3, Nodes: []binding.InputNode{ + str, + {Kind: binding.InputOptional, Elem: 0}, + {Kind: binding.InputOptional, Elem: 1}, + {Kind: binding.InputStruct, Fields: []binding.InputField{{Name: "label", Node: 2}}}, + }}}, + {name: "unknown kind", schema: binding.InputSchema{Root: 1, Nodes: []binding.InputNode{ + {Kind: binding.InputKind("tuple")}, + {Kind: binding.InputStruct, Fields: []binding.InputField{{Name: "value", Node: 0}}}, + }}}, + {name: "invalid root field name", schema: binding.InputSchema{Root: 1, Nodes: []binding.InputNode{ + str, + {Kind: binding.InputStruct, Fields: []binding.InputField{{Name: "not-valid", Node: 0}}}, + }}}, + {name: "keyword root field name", schema: binding.InputSchema{Root: 1, Nodes: []binding.InputNode{ + str, + {Kind: binding.InputStruct, Fields: []binding.InputField{{Name: "from", Node: 0}}}, + }}}, + {name: "duplicate field name", schema: binding.InputSchema{Root: 2, Nodes: []binding.InputNode{ + str, + str, + { + Kind: binding.InputStruct, + Fields: []binding.InputField{{Name: "value", Node: 0}, {Name: "value", Node: 1}}, + }, + }}}, + } +} diff --git a/internal/execution/phase.go b/internal/execution/phase.go index 210990f..88c1a89 100644 --- a/internal/execution/phase.go +++ b/internal/execution/phase.go @@ -54,6 +54,12 @@ type executionState struct { // call forwards a bound canonical map through the request-specific native port. call nativeInvoker + + // maxValueDepth bounds the nesting of each bound native-call argument map. + maxValueDepth int + + // maxValueBytes supplies the byte-derived node budget for each bound argument map. + maxValueBytes int } // beginMain transitions exactly once from loading to running. diff --git a/internal/execution/stdlib_test.go b/internal/execution/stdlib_test.go index ecc65f1..613c3d2 100644 --- a/internal/execution/stdlib_test.go +++ b/internal/execution/stdlib_test.go @@ -6,7 +6,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/meigma/codemode/internal/binding" "github.com/meigma/codemode/internal/execution" "github.com/meigma/codemode/internal/universe" ) @@ -163,11 +162,9 @@ def main(): // statsSumBinding returns a nonreserved nested sum capability. func statsSumBinding() execution.CapabilityBinding { return execution.CapabilityBinding{ - ID: "cap.stats.sum", - Name: "stats.sum", - Input: []binding.FieldShape{ - {Name: "value", Type: "str", Required: true}, - }, + ID: "cap.stats.sum", + Name: "stats.sum", + Input: mustInputSchema[valueInput](), } } diff --git a/internal/worker/child_test.go b/internal/worker/child_test.go index 657b9e6..171aa8c 100644 --- a/internal/worker/child_test.go +++ b/internal/worker/child_test.go @@ -93,14 +93,14 @@ func TestServeProtocolFailure(t *testing.T) { assert.Zero(t, out.Len()) } -// TestServeManifestRejection proves an illegal advertised shape never starts Engine. +// TestServeManifestRejection proves an illegal advertised input schema never starts Engine. func TestServeManifestRejection(t *testing.T) { payload := []byte( `{"type":"exec","version":1,"source":"def main():\n return 1\n",` + `"limits":{"max_source_bytes":256,"max_execution_steps":1000,` + `"max_native_calls":8,"max_value_depth":8,"max_value_bytes":256},` + `"manifest":[{"id":"cap.lookup","name":"records.lookup",` + - `"input":[{"name":"org","type":"str","required":false}]}]}`, + `"input":{"root":0,"nodes":[{"kind":"str"}]}}]}`, ) var in bytes.Buffer diff --git a/internal/worker/frame_test.go b/internal/worker/frame_test.go index d3a3ed8..88d90a1 100644 --- a/internal/worker/frame_test.go +++ b/internal/worker/frame_test.go @@ -15,7 +15,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/meigma/codemode/internal/binding" "github.com/meigma/codemode/internal/execution" ) @@ -594,7 +593,7 @@ func TestFrameLimitsValidateChildLimits(t *testing.T) { } } -// TestFrameLimitsValidateManifest proves the child cannot invent IDs, names, or field shapes. +// TestFrameLimitsValidateManifest proves the child cannot invent IDs, names, or input schemas. func TestFrameLimitsValidateManifest(t *testing.T) { valid := validManifest() @@ -651,15 +650,6 @@ func TestFrameLimitsValidateManifest(t *testing.T) { {ID: "cap.lookup", Name: "records.lookup", Input: valid[0].Input}, {ID: "cap.detail", Name: "records.lookup.detail", Input: valid[0].Input}, }}, - {name: "unsupported input shape", manifest: []manifestEntry{{ - ID: "cap.lookup", - Name: "records.lookup", - Input: []binding.FieldShape{{ - Name: "org", - Type: "str", - Required: false, - }}, - }}}, } require.NoError(t, validateManifest(nil)) @@ -676,6 +666,15 @@ func TestFrameLimitsValidateManifest(t *testing.T) { require.ErrorIs(t, err, errInvalidManifest) }) } + for _, malformed := range malformedInputSchemas() { + t.Run(malformed.name, func(t *testing.T) { + err := validateManifest( + []manifestEntry{{ID: "cap.lookup", Name: "records.lookup", Input: malformed.schema}}, + ) + require.Error(t, err) + require.ErrorIs(t, err, errInvalidManifest) + }) + } } // TestFrameLimitsCheckedCaps proves payload caps fit uint32 and follow envelope math. @@ -860,12 +859,9 @@ func validExecFrame() execFrame { func validManifest() []manifestEntry { return []manifestEntry{{ - ID: "cap.lookup", - Name: "records.lookup", - Input: []binding.FieldShape{ - {Name: "org", Type: "str", Required: true}, - {Name: "limit", Type: "int | None", Required: false}, - }, + ID: "cap.lookup", + Name: "records.lookup", + Input: mustInputSchema[lookupInput](), }} } diff --git a/internal/worker/input_schema_test.go b/internal/worker/input_schema_test.go new file mode 100644 index 0000000..2a58996 --- /dev/null +++ b/internal/worker/input_schema_test.go @@ -0,0 +1,221 @@ +package worker + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/meigma/codemode/authz" + "github.com/meigma/codemode/internal/binding" + "github.com/meigma/codemode/internal/execution" +) + +// valueInput is the single required string input of the runner lookup capability. +type valueInput struct { + // Value is the required string argument. + Value string `json:"value"` +} + +// lookupInput is the frame-test lookup input with one optional member. +type lookupInput struct { + // Org is the required string argument. + Org string `json:"org"` + + // Limit is the optional signed integer argument. + Limit *int64 `json:"limit,omitempty"` +} + +// searchFilter is the nested struct argument of the composite search capability. +type searchFilter struct { + // Owner is the required nested string member. + Owner string `json:"owner"` + + // Label is the optional nested string member. + Label *string `json:"label,omitempty"` +} + +// searchInput combines a list, a nested struct, and a string-keyed dict. +type searchInput struct { + // Tags is the required list of strings. + Tags []string `json:"tags"` + + // Filter is the required nested struct. + Filter searchFilter `json:"filter"` + + // Counts is the required string-keyed dict of integers. + Counts map[string]int64 `json:"counts"` +} + +// schemaOutput is the minimal output type paired with test inputs at compile time. +type schemaOutput struct { + // Value is a string output field. + Value string `json:"value"` +} + +// malformedInputSchema is one structurally invalid input schema fixture. +type malformedInputSchema struct { + // name identifies the structural defect. + name string + + // schema is the invalid input schema. + schema binding.InputSchema +} + +// TestExecFrameRoundTripsCompositeInputSchema proves the manifest carries the full schema arena. +func TestExecFrameRoundTripsCompositeInputSchema(t *testing.T) { + exec := validExecFrame() + exec.Manifest = []manifestEntry{{ + ID: "cap.search", + Name: "records.search", + Input: mustInputSchema[searchInput](), + }} + payload, err := encodeExec(exec) + require.NoError(t, err) + + decoded, err := decodePayload(payload) + + require.NoError(t, err) + got, ok := decoded.(execFrame) + require.True(t, ok) + assert.Equal(t, exec.Manifest, got.Manifest) +} + +// TestRunnerNewRejectsMalformedInputSchemas proves the parent validates schemas before any spawn. +func TestRunnerNewRejectsMalformedInputSchemas(t *testing.T) { + for _, malformed := range malformedInputSchemas() { + t.Run(malformed.name, func(t *testing.T) { + capability := lookupBinding() + capability.Input = malformed.schema + + _, err := NewRunner([]execution.CapabilityBinding{capability}, testLimits(), nopDispatch()) + + require.ErrorIs(t, err, errInvalidManifest) + }) + } +} + +// TestRunnerCarriesCompositeArguments proves nested arguments cross the worker boundary canonically. +func TestRunnerCarriesCompositeArguments(t *testing.T) { + var gotID string + var gotArguments map[string]any + runner := newSearchRunner(t, func( + _ context.Context, + _ authz.Subject, + id string, + arguments map[string]any, + _ int, + ) (any, error) { + gotID = id + gotArguments = arguments + return "ok", nil + }) + + got, err := runner.Execute(context.Background(), authz.Subject{ID: "s"}, "def main():\n"+ + " return records.search(tags=[\"alpha\", \"beta\"], filter={\"owner\": \"meigma\", \"label\": None},"+ + " counts={\"open\": 3})\n") + + require.NoError(t, err) + assert.Equal(t, "ok", got) + assert.Equal(t, "cap.search", gotID) + assert.Equal(t, map[string]any{ + "tags": []any{"alpha", "beta"}, + "filter": map[string]any{"owner": "meigma"}, + "counts": map[string]any{"open": int64(3)}, + }, gotArguments) +} + +// TestRunnerReportsCompositeArgumentPath proves nested type errors keep their path as safe detail. +func TestRunnerReportsCompositeArgumentPath(t *testing.T) { + var calls int + runner := newSearchRunner(t, func(context.Context, authz.Subject, string, map[string]any, int) (any, error) { + calls++ + return "ok", nil + }) + + _, err := runner.Execute(context.Background(), authz.Subject{ID: "s"}, "def main():\n"+ + " return records.search(tags=[\"alpha\"], filter={\"owner\": 1}, counts={})\n") + + require.ErrorIs(t, err, execution.ErrInvalidArguments) + assert.Equal(t, execution.ErrInvalidArguments.Error(), err.Error()) + detail, ok := execution.SafeDetail(err) + require.True(t, ok) + assert.Equal(t, `argument "filter.owner" must be a string`, detail) + assert.Zero(t, calls) +} + +// newSearchRunner constructs a runner exposing the composite records.search capability. +func newSearchRunner(t *testing.T, dispatch Dispatch) *Runner { + t.Helper() + runner, err := NewRunner([]execution.CapabilityBinding{{ + ID: "cap.search", + Name: "records.search", + Input: mustInputSchema[searchInput](), + }}, testLimits(), dispatch) + require.NoError(t, err) + return runner +} + +// mustInputSchema compiles In as a capability input and returns its private schema. +// +// It panics when In is not a supported input type, which is a fixture bug. +func mustInputSchema[In any]() binding.InputSchema { + plan, err := binding.CompileFor[In, schemaOutput]() + if err != nil { + panic(err) + } + return plan.InputSchema() +} + +// malformedInputSchemas returns hand-built schemas that no compiled plan can produce. +func malformedInputSchemas() []malformedInputSchema { + str := binding.InputNode{Kind: binding.InputString} + return []malformedInputSchema{ + {name: "empty schema", schema: binding.InputSchema{}}, + {name: "root out of range", schema: binding.InputSchema{ + Root: 1, + Nodes: []binding.InputNode{{Kind: binding.InputStruct}}, + }}, + {name: "root not a struct", schema: binding.InputSchema{Nodes: []binding.InputNode{str}}}, + {name: "forward element index", schema: binding.InputSchema{Root: 2, Nodes: []binding.InputNode{ + {Kind: binding.InputList, Elem: 1}, + str, + {Kind: binding.InputStruct, Fields: []binding.InputField{{Name: "tags", Node: 0}}}, + }}}, + {name: "self element index", schema: binding.InputSchema{Root: 1, Nodes: []binding.InputNode{ + {Kind: binding.InputList, Elem: 0}, + {Kind: binding.InputStruct, Fields: []binding.InputField{{Name: "tags", Node: 0}}}, + }}}, + {name: "forward field index", schema: binding.InputSchema{Root: 0, Nodes: []binding.InputNode{ + {Kind: binding.InputStruct, Fields: []binding.InputField{{Name: "value", Node: 1}}}, + str, + }}}, + {name: "optional wrapping optional", schema: binding.InputSchema{Root: 3, Nodes: []binding.InputNode{ + str, + {Kind: binding.InputOptional, Elem: 0}, + {Kind: binding.InputOptional, Elem: 1}, + {Kind: binding.InputStruct, Fields: []binding.InputField{{Name: "label", Node: 2}}}, + }}}, + {name: "unknown kind", schema: binding.InputSchema{Root: 1, Nodes: []binding.InputNode{ + {Kind: binding.InputKind("tuple")}, + {Kind: binding.InputStruct, Fields: []binding.InputField{{Name: "value", Node: 0}}}, + }}}, + {name: "invalid root field name", schema: binding.InputSchema{Root: 1, Nodes: []binding.InputNode{ + str, + {Kind: binding.InputStruct, Fields: []binding.InputField{{Name: "not-valid", Node: 0}}}, + }}}, + {name: "keyword root field name", schema: binding.InputSchema{Root: 1, Nodes: []binding.InputNode{ + str, + {Kind: binding.InputStruct, Fields: []binding.InputField{{Name: "from", Node: 0}}}, + }}}, + {name: "duplicate field name", schema: binding.InputSchema{Root: 2, Nodes: []binding.InputNode{ + str, + str, + { + Kind: binding.InputStruct, + Fields: []binding.InputField{{Name: "value", Node: 0}, {Name: "value", Node: 1}}, + }, + }}}, + } +} diff --git a/internal/worker/limits.go b/internal/worker/limits.go index 55cf45d..fbd811a 100644 --- a/internal/worker/limits.go +++ b/internal/worker/limits.go @@ -43,8 +43,8 @@ type manifestEntry struct { // The first segment must not collide with a reserved Starlark universe root. Name string `json:"name"` - // Input is the exact compiled input shape. - Input []binding.FieldShape `json:"input"` + // Input is the private compiled input schema. + Input binding.InputSchema `json:"input"` } // childPayloadCap is the largest legal child-originated execution payload. @@ -190,7 +190,7 @@ func validateManifest(entries []manifestEntry) error { if _, duplicate := names[entry.Name]; duplicate { return errInvalidManifest } - if err := binding.ValidateInputShape(entry.Input); err != nil { + if err := entry.Input.Validate(); err != nil { return fmt.Errorf("%w: %w", errInvalidManifest, err) } ids[entry.ID] = struct{}{} diff --git a/internal/worker/parent.go b/internal/worker/parent.go index ecd6ad6..c44f4e2 100644 --- a/internal/worker/parent.go +++ b/internal/worker/parent.go @@ -15,7 +15,6 @@ import ( "unicode/utf8" "github.com/meigma/codemode/authz" - "github.com/meigma/codemode/internal/binding" "github.com/meigma/codemode/internal/execution" ) @@ -363,16 +362,14 @@ func (r *Runner) Execute(ctx context.Context, subject authz.Subject, source stri return interpretExec(runCtx, out, waitErr) } -// copyManifest deep-copies ID, name, and input shapes into child manifest entries. +// copyManifest deep-copies ID, name, and input schemas into child manifest entries. func copyManifest(bindings []execution.CapabilityBinding) []manifestEntry { entries := make([]manifestEntry, len(bindings)) for index, capability := range bindings { - input := make([]binding.FieldShape, len(capability.Input)) - copy(input, capability.Input) entries[index] = manifestEntry{ ID: capability.ID, Name: capability.Name, - Input: input, + Input: capability.Input.Clone(), } } return entries diff --git a/internal/worker/parent_test.go b/internal/worker/parent_test.go index c75cfb4..ad3c691 100644 --- a/internal/worker/parent_test.go +++ b/internal/worker/parent_test.go @@ -19,7 +19,6 @@ import ( "github.com/stretchr/testify/require" "github.com/meigma/codemode/authz" - "github.com/meigma/codemode/internal/binding" "github.com/meigma/codemode/internal/execution" ) @@ -48,11 +47,9 @@ func testLimits() Limits { // lookupBinding is one valid capability used by runner tests. func lookupBinding() execution.CapabilityBinding { return execution.CapabilityBinding{ - ID: "cap.lookup", - Name: "records.lookup", - Input: []binding.FieldShape{ - {Name: "value", Type: "str", Required: true}, - }, + ID: "cap.lookup", + Name: "records.lookup", + Input: mustInputSchema[valueInput](), } } @@ -387,10 +384,11 @@ func TestRunnerNewCopiesManifest(t *testing.T) { require.NoError(t, err) capability.ID = "mutated" - capability.Input[0].Name = "mutated" + capability.Input.Nodes[capability.Input.Root].Fields[0].Name = "mutated" require.Len(t, runner.manifest, 1) assert.Equal(t, "cap.lookup", runner.manifest[0].ID) - assert.Equal(t, "value", runner.manifest[0].Input[0].Name) + manifestInput := runner.manifest[0].Input + assert.Equal(t, "value", manifestInput.Nodes[manifestInput.Root].Fields[0].Name) } // TestRunnerNilStateAndOversizeSource proves nil runner, nil context, and oversized source fail before spawn. diff --git a/mcpserver/composite_input_e2e_test.go b/mcpserver/composite_input_e2e_test.go new file mode 100644 index 0000000..ed8db3f --- /dev/null +++ b/mcpserver/composite_input_e2e_test.go @@ -0,0 +1,230 @@ +package mcpserver_test + +import ( + "context" + "sync" + "testing" + + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/meigma/codemode" + "github.com/meigma/codemode/authz" + "github.com/meigma/codemode/mcpserver" +) + +// queryInput is a composite capability input with lists, nested structs, maps, and optionals. +type queryInput struct { + // Tags is a required list of strings. + Tags []string `json:"tags"` + + // Filter is a required nested struct. + Filter queryFilter `json:"filter"` + + // Labels is a required string-keyed map. + Labels map[string]string `json:"labels"` + + // Window is an optional nested struct whose members need not be identifiers. + Window *queryWindow `json:"window"` +} + +// queryFilter is a nested input struct. +type queryFilter struct { + // Owner is a required nested string. + Owner string `json:"owner"` + + // States is a required nested list. + States []string `json:"states"` + + // MinScore is an optional nested float. + MinScore *float64 `json:"min_score"` +} + +// queryWindow uses a Starlark keyword and a hyphenated name as nested members. +type queryWindow struct { + // From is named after a reserved Starlark word. + From string `json:"from"` + + // Until carries a non-identifier name. + Until string `json:"until-day"` +} + +// queryResult echoes what the handler received. +type queryResult struct { + // Owner echoes the nested filter owner. + Owner string `json:"owner"` + + // TagCount is the number of received tags. + TagCount int64 `json:"tag_count"` +} + +// queryRecorder records typed composite handler inputs. +type queryRecorder struct { + // mu protects inputs. + mu sync.Mutex + + // inputs are the typed inputs observed at dispatch. + inputs []queryInput +} + +// invoke records one typed input and echoes part of it. +func (recorder *queryRecorder) invoke(_ context.Context, _ authz.Subject, input queryInput) (queryResult, error) { + recorder.mu.Lock() + defer recorder.mu.Unlock() + recorder.inputs = append(recorder.inputs, input) + return queryResult{Owner: input.Filter.Owner, TagCount: int64(len(input.Tags))}, nil +} + +// snapshot returns a copy of recorded inputs. +func (recorder *queryRecorder) snapshot() []queryInput { + recorder.mu.Lock() + defer recorder.mu.Unlock() + return append([]queryInput(nil), recorder.inputs...) +} + +// TestActualMCPCompositeInputProgram proves composite arguments travel from a +// model program through the worker, parent re-binding, and authorization to +// the typed handler, and that malformed nested arguments return path-precise +// diagnostics without reaching policy. +func TestActualMCPCompositeInputProgram(t *testing.T) { + authorizer := &recordingAuthorizer{} + recorder := &queryRecorder{} + builder := codemode.New(codemode.Options{Authorizer: authorizer, Limits: codemode.DefaultLimits()}) + codemode.Register(builder, codemode.Capability[queryInput, queryResult]{ + ID: "records.entry.query", + Name: "records.query", + Summary: "Query records with a composite filter.", + Handler: recorder.invoke, + }) + root, err := builder.Build() + require.NoError(t, err) + mcpServer, err := mcpserver.New(root, contextResolver{}, mcpserver.Options{}) + require.NoError(t, err) + + trustedCtx := withInvocationIdentity(t.Context(), invocationIdentity{ + Subject: authz.Subject{ID: trustedSubjectID}, + Canary: credentialCanary, + }) + serverTransport, clientTransport := mcp.NewInMemoryTransports() + serverSession, err := mcpServer.Connect(trustedCtx, serverTransport, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = serverSession.Close() }) + client := mcp.NewClient(&mcp.Implementation{Name: "codemode-e2e", Version: "test"}, nil) + session, err := client.Connect(t.Context(), clientTransport, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = session.Close() }) + + described, err := session.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "describe_api", + Arguments: map[string]any{"name": "records.query"}, + }) + require.NoError(t, err) + assertSuccessfulTool(t, described) + description := decodeStructured[codemode.Description](t, described) + assert.Equal(t, + "records.query(*, tags: list[str], filter: {owner: str, states: list[str], min_score: float | None}, "+ + `labels: dict[str, str], window: {from: str, "until-day": str} | None)`, + description.Signature) + require.Len(t, description.Input, 4) + assert.Equal(t, `{from: str, "until-day": str} | None`, description.Input[3].Type) + assert.False(t, description.Input[3].Required) + + execute := func(source string) *mcp.CallToolResult { + t.Helper() + result, callErr := session.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "execute", + Arguments: map[string]any{"source": source}, + }) + require.NoError(t, callErr) + return result + } + + first := execute(` +def main(): + return records.query( + tags = ["a", "b"], + filter = {"owner": "ops", "states": ("open", "triaged")}, + labels = {"env": "prod"}, + ) +`) + assertSuccessfulTool(t, first) + assert.Equal(t, queryResult{Owner: "ops", TagCount: 2}, decodeStructured[struct { + // Result is main's final converted value. + Result queryResult `json:"result"` + }](t, first).Result) + + second := execute(` +def main(): + return records.query( + tags = [], + filter = {"owner": "sec", "states": [], "min_score": 1}, + labels = {}, + window = {"from": "2026-01-01", "until-day": "2026-02-01"}, + ) +`) + assertSuccessfulTool(t, second) + + minScore := 1.0 + assert.Equal(t, []queryInput{ + { + Tags: []string{"a", "b"}, + Filter: queryFilter{Owner: "ops", States: []string{"open", "triaged"}}, + Labels: map[string]string{"env": "prod"}, + }, + { + Tags: []string{}, + Filter: queryFilter{Owner: "sec", States: []string{}, MinScore: &minScore}, + Labels: map[string]string{}, + Window: &queryWindow{From: "2026-01-01", Until: "2026-02-01"}, + }, + }, recorder.snapshot()) + + authorizations := authorizer.snapshot() + require.Len(t, authorizations, 2) + assert.Equal(t, map[string]any{ + "tags": []any{"a", "b"}, + "filter": map[string]any{"owner": "ops", "states": []any{"open", "triaged"}}, + "labels": map[string]any{"env": "prod"}, + }, authorizations[0].Arguments) + assert.Equal(t, map[string]any{ + "tags": []any{}, + "filter": map[string]any{"owner": "sec", "states": []any{}, "min_score": 1.0}, + "labels": map[string]any{}, + "window": map[string]any{"from": "2026-01-01", "until-day": "2026-02-01"}, + }, authorizations[1].Arguments) + + failures := []struct { + // call is the malformed capability call. + call string + + // want is the exact model-facing tool error. + want string + }{ + { + call: `records.query(tags=[], filter={"owner": 1, "states": []}, labels={})`, + want: `invalid capability arguments: argument "filter.owner" must be a string`, + }, + { + call: `records.query(tags=["a", 2], filter={"owner": "o", "states": []}, labels={})`, + want: `invalid capability arguments: argument "tags[1]" must be a string`, + }, + { + call: `records.query(tags=[], filter={"owner": "o", "states": [], "bogus": 1}, labels={})`, + want: `invalid capability arguments: unknown argument "filter.bogus"`, + }, + { + call: `records.query(tags=[], filter={"owner": "o", "states": []}, labels={"env": None})`, + want: `invalid capability arguments: argument "labels['env']" must be a string`, + }, + { + call: `records.query(tags=[], filter={"owner": "o", "states": []}, labels={}, window={"from": "x"})`, + want: `invalid capability arguments: missing required argument "window['until-day']"`, + }, + } + for _, failure := range failures { + assertToolError(t, execute("def main():\n return "+failure.call+"\n"), failure.want) + } + assert.Len(t, authorizer.snapshot(), 2, "rejected calls must never reach authorization") + assert.Len(t, recorder.snapshot(), 2, "rejected calls must never reach the handler") +} diff --git a/moon.yml b/moon.yml index 25335d7..08f1f97 100644 --- a/moon.yml +++ b/moon.yml @@ -72,7 +72,7 @@ tasks: # trailing single-dash `-flag=value` into two arguments; `script` runs the line # through the shell verbatim. mcp-smoke: - script: "go test ./mcpserver -run '^(TestActualMCPSecureLoop|TestActualMCPCompositeProgram|TestActualMCPPureComputeProgram)$' -count=1" + script: "go test ./mcpserver -run '^(TestActualMCPSecureLoop|TestActualMCPCompositeProgram|TestActualMCPCompositeInputProgram|TestActualMCPPureComputeProgram)$' -count=1" inputs: - '@group(goSources)' options: diff --git a/server.go b/server.go index 5c9d8d7..1a51bac 100644 --- a/server.go +++ b/server.go @@ -81,7 +81,7 @@ func capabilityBindings(capabilityCatalog *catalog.Catalog) []execution.Capabili bindings[index] = execution.CapabilityBinding{ ID: entry.ID, Name: entry.Name, - Input: entry.Plan.InputShape(), + Input: entry.Plan.InputSchema(), } } return bindings