From 30a8ae7b5a162d3bf0fc7d536a4eedecab23d415 Mon Sep 17 00:00:00 2001 From: Clay McGinnis Date: Thu, 11 Jun 2026 20:13:14 -0700 Subject: [PATCH] fix(spec): resolve composed object/array body field types FastAPI/Pydantic emits optional object fields as `anyOf: [{$ref -> object}, {type: "null"}]`, leaving the wrapper schema with no direct type or properties. resolveSchemaType only inspected the top-level type, properties, and items, so these fields fell through to "string". The generated `api` command then registered a plain scalar flag (e.g. --runpython) that serialized the raw value as a JSON string, producing `"runPython":"s3://..."` and an HTTP 422 from POST /runs, which requires an object `{"uri": "..."}`. Recurse into the allOf/anyOf/oneOf composition keywords and return the first member that resolves to a concrete (non-null) type, with a depth bound to guard against circular refs. The recursive worker returns "" (not "string") for an unresolved schema, so a genuine string alternative in a polymorphic union is preferred over a later, stricter type rather than skipped. Object/array fields are now correctly typed, so the builder generates the existing JSON flag form (--runpython-json) that accepts and emits the object body. This fixes runPython, runJar, and every other composed object/array body field uniformly. Fixes WBC-43. --- internal/commands/builder_test.go | 43 ++++++++++ internal/spec/parser.go | 54 ++++++++++-- internal/spec/parser_test.go | 138 ++++++++++++++++++++++++++++++ 3 files changed, 229 insertions(+), 6 deletions(-) diff --git a/internal/commands/builder_test.go b/internal/commands/builder_test.go index 17e179a..5686942 100644 --- a/internal/commands/builder_test.go +++ b/internal/commands/builder_test.go @@ -222,6 +222,49 @@ func TestInvalidArgsReturnsUsageHint(t *testing.T) { } } +func TestObjectBodyFieldSerializesAsJSONObject(t *testing.T) { + t.Parallel() + + cfg := config.Config{AppName: "wherobots", APIKey: "test-key", HTTPTimeout: time.Second} + runtimeSpec := &spec.RuntimeSpec{ + BaseURL: "https://api.example.com", + Operations: []*spec.Operation{ + { + Method: "POST", + Path: "/runs", + RequestBody: &spec.RequestBodyInfo{ + Required: true, + SchemaType: "object", + Fields: []spec.BodyField{ + {Name: "name", Type: "string", Required: true}, + {Name: "runPython", Type: "object", Required: true}, + }, + }, + }, + }, + } + + root := BuildRootCommand(cfg, runtimeSpec) + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{ + "api", "runs", "create", + "--name", "ftw-top5", + "--runpython-json", `{"uri":"s3://bucket/ftw_top5.py"}`, + "--dry-run", + }) + + if err := root.Execute(); err != nil { + t.Fatalf("Execute() error = %v", err) + } + + got := out.String() + if !strings.Contains(got, `"runPython":{"uri":"s3://bucket/ftw_top5.py"}`) { + t.Fatalf("object body field should serialize as a JSON object, got:\n%s", got) + } +} + func TestHelpShowsTypedFlagSamples(t *testing.T) { t.Parallel() diff --git a/internal/spec/parser.go b/internal/spec/parser.go index 7f97bfa..04c1575 100644 --- a/internal/spec/parser.go +++ b/internal/spec/parser.go @@ -314,22 +314,37 @@ func resolveParamType(param *highv3.Parameter) string { return resolveSchemaType(param.Schema) } +// maxSchemaResolveDepth bounds recursion through composition keywords so a +// self-referential schema (a circular $ref) cannot cause infinite recursion. +const maxSchemaResolveDepth = 16 + func resolveSchemaType(proxy *highbase.SchemaProxy) string { - if proxy == nil { - return "string" + if resolved := resolveSchemaTypeDepth(proxy, 0); resolved != "" { + return resolved + } + return "string" +} + +// resolveSchemaTypeDepth returns the resolved type, or "" when the schema has +// no discernible concrete type (so callers can fall back or keep searching). +// "string" is never used as the unresolved sentinel, so a genuine string +// alternative in a union is not skipped over. +func resolveSchemaTypeDepth(proxy *highbase.SchemaProxy, depth int) string { + if proxy == nil || depth > maxSchemaResolveDepth { + return "" } schema := proxy.Schema() if schema == nil { rebuilt, err := proxy.BuildSchema() if err != nil || rebuilt == nil { - return "string" + return "" } schema = rebuilt } - if len(schema.Type) > 0 && schema.Type[0] != "" { - return schema.Type[0] + if t := firstConcreteType(schema.Type); t != "" { + return t } if schema.Properties != nil && orderedmap.Len(schema.Properties) > 0 { return "object" @@ -337,7 +352,34 @@ func resolveSchemaType(proxy *highbase.SchemaProxy) string { if schema.Items != nil && schema.Items.IsA() && schema.Items.A != nil { return "array" } - return "string" + + // FastAPI/Pydantic models an optional object field as + // `anyOf: [{$ref -> object}, {type: "null"}]`, leaving the wrapper schema + // with no direct type or properties. Recurse into the composition keywords + // and return the first member that resolves to a concrete (non-null) type + // so these fields are treated as JSON objects/arrays rather than plain + // strings, while still preferring an earlier string alternative over a + // later stricter one in a polymorphic union. + for _, group := range [][]*highbase.SchemaProxy{schema.AllOf, schema.AnyOf, schema.OneOf} { + for _, sub := range group { + if resolved := resolveSchemaTypeDepth(sub, depth+1); resolved != "" { + return resolved + } + } + } + + return "" +} + +// firstConcreteType returns the first non-empty, non-"null" type from an +// OpenAPI 3.1 type list (e.g. ["string", "null"]), or "" when none qualifies. +func firstConcreteType(types []string) string { + for _, t := range types { + if t != "" && t != "null" { + return t + } + } + return "" } func isParamRequired(param *highv3.Parameter) bool { diff --git a/internal/spec/parser_test.go b/internal/spec/parser_test.go index 9fe20fe..c834927 100644 --- a/internal/spec/parser_test.go +++ b/internal/spec/parser_test.go @@ -122,3 +122,141 @@ func TestParseExtractsOperationsAndSchema(t *testing.T) { t.Fatalf("field = %+v, want enabled:boolean required", op.RequestBody.Fields[0]) } } + +// TestParseResolvesComposedObjectBodyFields covers the WBC-43 bug: FastAPI/Pydantic +// emits optional object fields as `anyOf: [{$ref -> object}, {type: "null"}]` and +// required object fields as a bare `$ref`. Both must resolve to "object" (and arrays +// to "array") so the command builder generates JSON-object flags rather than treating +// them as plain strings. +func TestParseResolvesComposedObjectBodyFields(t *testing.T) { + t.Parallel() + + raw := []byte(`{ + "openapi": "3.1.0", + "info": { "title": "x", "version": "1" }, + "servers": [{ "url": "https://api.example.com" }], + "paths": { + "/runs": { + "post": { + "operationId": "createRun", + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": ["runPython"], + "properties": { + "name": { "type": "string" }, + "runPython": { "anyOf": [ { "$ref": "#/components/schemas/RunPython" }, { "type": "null" } ] }, + "runJar": { "$ref": "#/components/schemas/RunJar" }, + "tags": { "anyOf": [ { "type": "array", "items": { "type": "string" } }, { "type": "null" } ] } + } + } + } + } + }, + "responses": { "200": { "description": "ok" } } + } + } + }, + "components": { + "schemas": { + "RunPython": { + "type": "object", + "required": ["uri"], + "properties": { + "uri": { "type": "string" }, + "args": { "type": "array", "items": { "type": "string" } } + } + }, + "RunJar": { + "type": "object", + "required": ["uri"], + "properties": { "uri": { "type": "string" } } + } + } + } +}`) + + parsed, err := Parse(raw, "") + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + if len(parsed.Operations) != 1 { + t.Fatalf("operations = %d, want 1", len(parsed.Operations)) + } + + byField := make(map[string]BodyField) + for _, field := range parsed.Operations[0].RequestBody.Fields { + byField[field.Name] = field + } + + cases := map[string]string{ + "name": "string", + "runPython": "object", + "runJar": "object", + "tags": "array", + } + for name, wantType := range cases { + field, ok := byField[name] + if !ok { + t.Fatalf("field %q missing from parsed body fields", name) + } + if field.Type != wantType { + t.Errorf("field %q type = %q, want %q", name, field.Type, wantType) + } + } +} + +// TestParseScalarUnionPrefersFirstConcreteType guards against over-eager +// composition resolution: a `str | int` union must resolve to its first +// concrete alternative ("string"), not skip past it to a stricter type. The +// resolver must only treat "null" (and unresolvable members) as skippable. +func TestParseScalarUnionPrefersFirstConcreteType(t *testing.T) { + t.Parallel() + + raw := []byte(`{ + "openapi": "3.1.0", + "info": { "title": "x", "version": "1" }, + "paths": { + "/things": { + "post": { + "operationId": "createThing", + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "mode": { "anyOf": [ { "type": "string" }, { "type": "integer" } ] } + } + } + } + } + }, + "responses": { "200": { "description": "ok" } } + } + } + } +}`) + + parsed, err := Parse(raw, "") + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + + var mode *BodyField + for i := range parsed.Operations[0].RequestBody.Fields { + if parsed.Operations[0].RequestBody.Fields[i].Name == "mode" { + mode = &parsed.Operations[0].RequestBody.Fields[i] + } + } + if mode == nil { + t.Fatal("field \"mode\" missing from parsed body fields") + } + if mode.Type != "string" { + t.Errorf("field \"mode\" type = %q, want \"string\"", mode.Type) + } +}