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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 71 additions & 3 deletions internal/openapi/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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).
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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{} {
Expand Down
146 changes: 146 additions & 0 deletions internal/openapi/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
Loading