diff --git a/internal/openapi/schema.go b/internal/openapi/schema.go index 3816c4c..35b191b 100644 --- a/internal/openapi/schema.go +++ b/internal/openapi/schema.go @@ -174,6 +174,17 @@ func fieldNotFoundErr(seg string, props map[string]*base.SchemaProxy, sch *base. } } +// allOfShapeKeys are the non-object schema facets simplify adopts from allOf +// members so a typed ref wrapped in an allOf (the `.describe()`-on-a-ref +// pattern) keeps its shape instead of collapsing to an empty object. properties +// and required are merged separately, so they are deliberately excluded here. A +// later member's value overrides an earlier one's; the schema's own facets, +// applied after the allOf loop, override these in turn. +var allOfShapeKeys = []string{ + "type", "items", "anyOf", "oneOf", "additionalProperties", + "enum", "format", "description", "default", "example", +} + // simplify turns a libopenapi schema into a compact, agent-friendly map. It // merges allOf composition into a single object, preserves descriptions, enums, // formats, required fields and examples, and guards against deep nesting (via @@ -230,9 +241,13 @@ func (d *describer) simplify(proxy *base.SchemaProxy, depth int, seen map[string } } - // allOf composes at the same level: merge member properties and required - // into this object. Members are expanded at the same depth, since allOf is - // composition rather than nesting. + // allOf composes at the same level. Object members contribute their + // properties and required, merged here at the same depth since allOf is + // composition rather than nesting. A member that instead carries a + // non-object shape is the `.describe()`-on-a-ref pattern: a typed ref — + // array, scalar, or union — wrapped in an allOf alongside a description-only + // sibling. We adopt that shape directly (see allOfShapeKeys); without it the + // field would lose its type/items and collapse to an empty object. for _, member := range sch.AllOf { sub, ok := d.simplify(member, depth, childSeen).(map[string]interface{}) if !ok { @@ -246,6 +261,11 @@ func (d *describer) simplify(proxy *base.SchemaProxy, depth int, seen map[string if req, ok := sub["required"].([]string); ok { addRequired(req) } + for _, k := range allOfShapeKeys { + if v, ok := sub[k]; ok { + out[k] = v + } + } } // This schema's own properties (nested one level deeper). @@ -356,6 +376,14 @@ func (d *describer) synth(proxy *base.SchemaProxy, name string, depth int, seen return obj } reqSet, props := gatherObject(sch, childSeen) + // A `.describe()`-on-a-ref wrapper around a non-object (e.g. an array or + // scalar ref beside a description-only sibling) has nothing to gather as + // an object. Synthesize the underlying typed member instead of {}. + if len(props) == 0 && len(reqSet) == 0 && len(sch.AllOf) > 0 { + if m := substantiveAllOfMember(sch); m != nil { + return d.synth(m, name, depth, childSeen) + } + } for _, fieldName := range sortedKeys(reqSet) { if p, ok := props[fieldName]; ok { obj[fieldName] = d.synth(p, fieldName, depth+1, childSeen) @@ -428,6 +456,46 @@ func gatherObject(sch *base.Schema, seen map[string]bool) (map[string]bool, map[ return reqSet, props } +// substantiveAllOfMember returns the single allOf member that carries an actual +// shape — a $ref or any typed/composed schema — ignoring description-only +// siblings. It lets synth handle the `.describe()`-on-a-ref pattern, where a +// typed ref is wrapped in an allOf next to a metadata-only member. It returns +// nil unless exactly one member is substantive, so an ambiguous composition +// falls back to plain object handling. +func substantiveAllOfMember(sch *base.Schema) *base.SchemaProxy { + var found *base.SchemaProxy + for _, m := range sch.AllOf { + if m == nil { + continue + } + if !m.IsReference() { + ms := m.Schema() + if ms == nil || isMetadataOnly(ms) { + continue + } + } + if found != nil { + return nil + } + found = m + } + return found +} + +// isMetadataOnly reports whether a schema carries no structural shape — only +// annotations like description. Such a member is the description-only sibling in +// the `.describe()`-on-a-ref pattern. +func isMetadataOnly(sch *base.Schema) bool { + return len(sch.Type) == 0 && + sch.Properties == nil && + len(sch.AllOf) == 0 && + len(sch.AnyOf) == 0 && + len(sch.OneOf) == 0 && + sch.Items == nil && + len(sch.Enum) == 0 && + (sch.AdditionalProperties == nil || !sch.AdditionalProperties.IsA()) +} + // placeholder returns a typed stand-in value for a required field that carries // no example. Strings echo the field name so the agent knows what to fill in. func placeholder(name, typ string) interface{} { diff --git a/internal/openapi/schema_test.go b/internal/openapi/schema_test.go index 8a6702c..cc0e31a 100644 --- a/internal/openapi/schema_test.go +++ b/internal/openapi/schema_test.go @@ -302,6 +302,152 @@ func TestSchema_MapExampleShowsRepresentativeEntry(t *testing.T) { } } +// describeRefTestSpec exercises the `.describe()`-on-a-ref pattern: a typed ref +// wrapped in an allOf alongside a description-only sibling. `containers` wraps an +// array ref (Layout) and `identifier` wraps a scalar ref (DocId). A naive +// allOf merge that only harvests object properties/required drops these to an +// empty object; the describer must instead adopt the underlying shape. +const describeRefTestSpec = `{ + "openapi": "3.1.0", + "info": {"title": "test", "version": "1.0"}, + "paths": { + "/api/v1/docs": { + "post": { + "operationId": "docsCreate", + "tags": ["docs"], + "requestBody": { + "content": { + "application/json": { + "schema": {"$ref": "#/components/schemas/CreateDoc"} + } + } + }, + "responses": {"200": {"description": "ok"}} + } + } + }, + "components": { + "schemas": { + "Grid": {"type": "object", "required": ["kind"], "properties": {"kind": {"type": "string", "example": "grid"}}}, + "Stack": {"type": "object", "required": ["kind"], "properties": {"kind": {"type": "string", "example": "stack"}}}, + "Layout": { + "type": "array", + "description": "Container layout array.", + "items": {"anyOf": [{"$ref": "#/components/schemas/Grid"}, {"$ref": "#/components/schemas/Stack"}]} + }, + "DocId": {"type": "string", "minLength": 2, "description": "Base identifier description."}, + "CreateDoc": { + "type": "object", + "required": ["containers", "identifier"], + "properties": { + "containers": { + "allOf": [ + {"$ref": "#/components/schemas/Layout"}, + {"description": "Override: when present, replaces the existing layout."} + ] + }, + "identifier": { + "allOf": [ + {"$ref": "#/components/schemas/DocId"}, + {"description": "Override: identifier for the new document."} + ] + } + } + } + } + } +}` + +// An array ref wrapped in a describe()-allOf must keep its array shape and items +// union, not collapse to an empty object. +func TestSchema_DescribeOnRefArrayKeepsShape(t *testing.T) { + doc, err := runSchemaWith(t, describeRefTestSpec) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + body := doc.Body.(map[string]interface{}) + props := body["properties"].(map[string]interface{}) + containers, ok := props["containers"].(map[string]interface{}) + if !ok { + t.Fatalf("containers property missing or not an object") + } + if containers["type"] != "array" { + t.Errorf("containers.type = %v, want array (shape lost to allOf merge)", containers["type"]) + } + items, ok := containers["items"].(map[string]interface{}) + if !ok { + t.Fatalf("containers.items missing — array shape was dropped: %v", containers) + } + anyOf, ok := items["anyOf"].([]interface{}) + if !ok || len(anyOf) != 2 { + t.Errorf("containers.items.anyOf = %v, want 2 members (Grid/Stack)", items["anyOf"]) + } + // The description-only sibling overrides the ref's own description. + if got, _ := containers["description"].(string); !strings.HasPrefix(got, "Override:") { + t.Errorf("containers.description = %q, want the sibling override to win", got) + } +} + +// A scalar ref wrapped in a describe()-allOf must keep its scalar type and take +// the sibling's overriding description. +func TestSchema_DescribeOnRefScalarKeepsType(t *testing.T) { + doc, err := runSchemaWith(t, describeRefTestSpec) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + body := doc.Body.(map[string]interface{}) + props := body["properties"].(map[string]interface{}) + identifier, ok := props["identifier"].(map[string]interface{}) + if !ok { + t.Fatalf("identifier property missing or not an object") + } + if identifier["type"] != "string" { + t.Errorf("identifier.type = %v, want string (scalar shape lost)", identifier["type"]) + } + if got, _ := identifier["description"].(string); !strings.HasPrefix(got, "Override:") { + t.Errorf("identifier.description = %q, want the sibling override to win", got) + } +} + +// The synthesized example must reflect the underlying typed member: an array for +// the array ref and a scalar placeholder for the scalar ref, not empty objects. +func TestSchema_DescribeOnRefExampleSynthesizesShape(t *testing.T) { + doc, err := runSchemaWith(t, describeRefTestSpec) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + ex, ok := doc.Example.(map[string]interface{}) + if !ok { + t.Fatalf("example is not an object: %T", doc.Example) + } + arr, ok := ex["containers"].([]interface{}) + if !ok || len(arr) != 1 { + t.Errorf("example.containers = %v, want a one-element array (got empty object?)", ex["containers"]) + } + if s, ok := ex["identifier"].(string); !ok || !strings.HasPrefix(s, "<") { + t.Errorf("example.identifier = %v, want a scalar placeholder", ex["identifier"]) + } +} + +// Drilling --field into a describe()-on-a-ref array resolves and expands it, +// rather than returning an empty body. +func TestSchema_DescribeOnRefFieldDrill(t *testing.T) { + doc, err := runSchemaWith(t, describeRefTestSpec, "--field", "containers") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + body, ok := doc.Body.(map[string]interface{}) + if !ok { + t.Fatalf("body is not an object: %T", doc.Body) + } + if body["type"] != "array" { + t.Errorf("drilled containers.type = %v, want array", body["type"]) + } + if _, ok := body["items"].(map[string]interface{}); !ok { + t.Errorf("drilled containers.items missing: %v", body) + } +} + // runSchemaWith runs "create --schema" plus extra flags against a spec. On // success it returns the parsed doc; on failure it returns the execution error // (usage/errors silenced so the error carries only the message under test).