diff --git a/.changeset/schema-preview-compositions.md b/.changeset/schema-preview-compositions.md new file mode 100644 index 000000000..b45f05f8d --- /dev/null +++ b/.changeset/schema-preview-compositions.md @@ -0,0 +1,5 @@ +--- +"executor": patch +--- + +Keep sibling `properties` and `required` beside an inline `allOf`, `anyOf`, or `oneOf` in TypeScript tool previews, matching what referenced definitions already did. A titled definition names the whole composition, and a composed definition whose branch refers back to it no longer falls back to `unknown`. diff --git a/e2e/scenarios/tool-descriptions.test.ts b/e2e/scenarios/tool-descriptions.test.ts index e646ad293..cbf3ccd66 100644 --- a/e2e/scenarios/tool-descriptions.test.ts +++ b/e2e/scenarios/tool-descriptions.test.ts @@ -153,6 +153,13 @@ const ordersOpenApiSpec = (baseUrl: string): string => type: "string", description: "Free-form note shown to the warehouse packer.", }, + metadata: { + type: "object", + description: "Extensible metadata for the order.", + properties: { type: { type: "string" } }, + required: ["type"], + allOf: [{ type: "object", additionalProperties: {} }], + }, }, }, }, @@ -488,6 +495,10 @@ scenario( expect(createOrder?.listDescription, "summary is the fallback description").toBe( "Create an order", ); + expect( + createOrder?.inputTypeScript, + "sibling properties survive an inline allOf", + ).toContain("metadata?: ({ [k: string]: unknown; } & { type: string; })"); expect(getOrder?.inputTypeScript, "input shape is compiled to TypeScript").toContain( "orderId", ); diff --git a/packages/core/sdk/src/schema-types.test.ts b/packages/core/sdk/src/schema-types.test.ts index b64468e6a..77748d4e4 100644 --- a/packages/core/sdk/src/schema-types.test.ts +++ b/packages/core/sdk/src/schema-types.test.ts @@ -467,4 +467,81 @@ describe("schema-types", () => { outputTypeScript: "{ metadata: { [k: string]: unknown; }; }", }); }); + + it.each(["allOf", "anyOf", "oneOf"])( + "keeps sibling properties and required keys beside an inline %s", + async (composition) => { + const siblings = { + properties: { type: { type: "string" }, label: { type: "string" } }, + required: ["type"], + [composition]: [{ type: "object", additionalProperties: {} }], + }; + const expected = { + type: "({ [k: string]: unknown; } & { type: string; label?: string; })", + definitions: {}, + }; + expect(await schemaToTypeScriptPreview({ type: "object", ...siblings })).toEqual(expected); + expect(await schemaToTypeScriptPreview(siblings)).toEqual(expected); + }, + ); + + it("applies sibling properties per member of a nullable composed type", async () => { + expect( + await schemaToTypeScriptPreview({ + type: ["object", "null"], + properties: { type: { type: "string" } }, + required: ["type"], + allOf: [{ type: "object", additionalProperties: {} }], + }), + ).toEqual({ + type: "({ [k: string]: unknown; } & ({ type: string; } | null))", + definitions: {}, + }); + }); + + it("names the whole composition when a titled definition has sibling properties", async () => { + expect( + await schemaToTypeScriptPreview({ + type: "object", + properties: { first: { $ref: "#/$defs/Metadata" }, second: { $ref: "#/$defs/Metadata" } }, + $defs: { + Metadata: { + title: "Metadata", + type: "object", + properties: { type: { type: "string" } }, + required: ["type"], + allOf: [{ type: "object", additionalProperties: {} }], + }, + }, + }), + ).toEqual({ + type: "{ first?: Metadata; second?: Metadata; }", + definitions: { Metadata: "({ [k: string]: unknown; } & { type: string; })" }, + }); + }); + + it("compiles a composed definition whose branch refers back to it", async () => { + expect( + await schemaToTypeScriptPreview({ + $ref: "#/$defs/Comment", + $defs: { + Comment: { + type: "object", + properties: { body: { type: "string" } }, + allOf: [{ $ref: "#/$defs/Base" }], + }, + Base: { + type: "object", + properties: { replies: { type: "array", items: { $ref: "#/$defs/Comment" } } }, + }, + }, + }), + ).toEqual({ + type: "Comment", + definitions: { + Base: "{ replies?: Comment[]; }", + Comment: "(Base & { body?: string; })", + }, + }); + }); }); diff --git a/packages/core/sdk/src/vendor/json-schema-to-typescript/README.md b/packages/core/sdk/src/vendor/json-schema-to-typescript/README.md index 92881eda6..d545c176c 100644 --- a/packages/core/sdk/src/vendor/json-schema-to-typescript/README.md +++ b/packages/core/sdk/src/vendor/json-schema-to-typescript/README.md @@ -10,5 +10,15 @@ It also resolves only same-document JSON Pointer `$ref`s; external file and URL refs are rejected rather than fetched or read. It is not a public package surface. +Sibling `properties` and `patternProperties` beside `allOf`, `anyOf`, or +`oneOf` join the composition as one more intersection member, for inline +schemas as well as `$id`-named ones. A definition's name belongs to the whole +composition, and re-entering a composition through a recursive reference reuses +its cached node. The sibling keywords intersect every branch, including +non-object ones, so a `{ "type": "null" }` alternative beside sibling +properties is absorbed; this matches how `$id`-named definitions already +behaved. Keeping non-object alternatives would need per-branch kind analysis, +which is deliberately not implemented. + The upstream project is MIT licensed; the original copyright notice is included in `LICENCE.md`. diff --git a/packages/core/sdk/src/vendor/json-schema-to-typescript/parser.ts b/packages/core/sdk/src/vendor/json-schema-to-typescript/parser.ts index 0b8494096..4f80ab36c 100644 --- a/packages/core/sdk/src/vendor/json-schema-to-typescript/parser.ts +++ b/packages/core/sdk/src/vendor/json-schema-to-typescript/parser.ts @@ -51,6 +51,11 @@ export function parse( const types = normalizedSchema[Types]; if (intersection) { + // A recursive reference can re-enter this schema while its branches are + // still being parsed. Reuse the cached intersection instead of appending + // the sibling types again or reading a placeholder's params too early. + const cached = processed.get(intersection)?.get("ALL_OF"); + if (cached) return cached; const ast = parseAsTypeWithCache( intersection, "ALL_OF", @@ -146,7 +151,11 @@ function parseNonLiteral( usedNames: UsedNames, ): AST { const definitions = getDefinitionsMemoized(getRootSchema(schema as any)); // TODO - const keyNameFromDefinition = findKey(definitions, (_) => _ === schema); + // A synthesized intersection stands for the whole definition. Its source + // schema now holds only the sibling constraints and must not take the name. + const keyNameFromDefinition = schema[Intersection] + ? undefined + : findKey(definitions, (_) => _ === schema || _[Intersection] === schema); switch (type) { case "ALL_OF": diff --git a/packages/core/sdk/src/vendor/json-schema-to-typescript/typesOfSchema.ts b/packages/core/sdk/src/vendor/json-schema-to-typescript/typesOfSchema.ts index 098b43ff7..dc3b910a4 100644 --- a/packages/core/sdk/src/vendor/json-schema-to-typescript/typesOfSchema.ts +++ b/packages/core/sdk/src/vendor/json-schema-to-typescript/typesOfSchema.ts @@ -141,8 +141,17 @@ const matchers: Record boolean> = { } return "enum" in schema; }, - UNNAMED_SCHEMA() { - return false; // Explicitly handled as the default case + UNNAMED_SCHEMA(schema) { + // Sibling object keywords still constrain a composed schema. `$id` schemas + // already join the composition through NAMED_SCHEMA; match inline ones here + // so the parser builds the same intersection. Type arrays are handled by + // UNION, which applies the object keywords to each member itself. + return ( + !("$id" in schema) && + (schema.type === undefined || schema.type === "object") && + ("allOf" in schema || "anyOf" in schema || "oneOf" in schema) && + ("properties" in schema || "patternProperties" in schema) + ); }, UNTYPED_ARRAY(schema) { return schema.type === "array" && !("items" in schema);