diff --git a/.changeset/describe-compile-scope.md b/.changeset/describe-compile-scope.md new file mode 100644 index 0000000000..a57cd5853c --- /dev/null +++ b/.changeset/describe-compile-scope.md @@ -0,0 +1,5 @@ +--- +"executor": patch +--- + +Compile `describe.tool` previews against only the definitions a tool references, drop the compiler's per-call retained graph, and fall back to `unknown` for schemas over a node limit. Describing a tool from a large OpenAPI spec no longer burns seconds of CPU or leaks memory in the shared session isolate. diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index 6916233cb5..efaf119212 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -5724,12 +5724,17 @@ export const createExecutor = buildToolTypeScriptPreview({ inputSchema, outputSchema: effectiveOutputSchema, - defs, + defs: new Map(Object.entries(referenced)), }), catch: (cause) => storageFailureFromUnknown("Failed to build tool TypeScript preview", cause), diff --git a/packages/core/sdk/src/schema-types.test.ts b/packages/core/sdk/src/schema-types.test.ts index 38fc1d848c..b64468e6ac 100644 --- a/packages/core/sdk/src/schema-types.test.ts +++ b/packages/core/sdk/src/schema-types.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from "@effect/vitest"; import { Schema } from "effect"; import { + MAX_PREVIEW_SCHEMA_NODES, buildToolTypeScriptPreview, schemaToTypeScriptPreview, schemaToTypeScriptPreviewWithDefs, @@ -385,6 +386,38 @@ describe("schema-types", () => { }); }); + it("falls back to unknown instead of compiling a schema over the node limit", async () => { + const defs = new Map(); + // One definition per node-limit slice, all referenced from the input, so the + // wrapped schema handed to the compiler is guaranteed to cross the cap. + const properties: Record = {}; + for (let i = 0; i < MAX_PREVIEW_SCHEMA_NODES / 2; i++) { + defs.set(`D${i}`, { type: "object", properties: { v: { type: "string" } } }); + properties[`p${i}`] = { $ref: `#/$defs/D${i}` }; + } + const preview = await buildToolTypeScriptPreview({ + inputSchema: { type: "object", properties }, + outputSchema: { type: "string" }, + defs, + }); + expect(preview).toEqual({ inputTypeScript: "unknown", outputTypeScript: "unknown" }); + }); + + it("only compiles the definitions a tool references", async () => { + // A big pile of unrelated definitions must not change the output or the + // time it takes: the caller passes the referenced subgraph, and this pins + // the contract that the preview is a pure function of that subgraph. + const referenced = new Map([ + ["Person", { type: "object", properties: { name: { type: "string" } }, required: ["name"] }], + ]); + const preview = await buildToolTypeScriptPreview({ + inputSchema: { type: "object", properties: { who: { $ref: "#/$defs/Person" } } }, + defs: referenced, + }); + expect(preview.inputTypeScript).toBe("{ who?: Person; }"); + expect(preview.typeScriptDefinitions).toEqual({ Person: "{ name: string; }" }); + }); + it("renders unconstrained schemas as unknown", async () => { await expect( buildToolTypeScriptPreview({ diff --git a/packages/core/sdk/src/schema-types.ts b/packages/core/sdk/src/schema-types.ts index c7d44e237f..d3db36060b 100644 --- a/packages/core/sdk/src/schema-types.ts +++ b/packages/core/sdk/src/schema-types.ts @@ -789,6 +789,31 @@ export type ToolTypeScriptPreview = { typeScriptDefinitions?: Record; }; +/** + * Upper bound on the number of schema nodes handed to the compiler for one + * tool preview. The compiler is fully synchronous, so nothing can interrupt + * it once it starts; a pre-check is the only protection a shared isolate has + * against a pathological schema. Sized well above any real operation (a + * PostHog or Stripe tool with its referenced definitions is a few thousand + * nodes) and well below where the compile would take seconds of CPU. + */ +export const MAX_PREVIEW_SCHEMA_NODES = 50_000; + +/** Count object/array nodes in a schema, stopping early once `limit` is hit. */ +const exceedsNodeLimit = (root: unknown, limit: number): boolean => { + let count = 0; + const stack: unknown[] = [root]; + while (stack.length > 0) { + const node = stack.pop(); + if (node === null || typeof node !== "object") continue; + if (++count > limit) return true; + for (const value of Array.isArray(node) ? node : Object.values(node as object)) { + if (value !== null && typeof value === "object") stack.push(value); + } + } + return false; +}; + export const buildToolTypeScriptPreview = async (input: { inputSchema?: unknown; outputSchema?: unknown; @@ -806,14 +831,19 @@ export const buildToolTypeScriptPreview = async (input: { return {}; } + const unknownPreview: ToolTypeScriptPreview = { + ...(input.inputSchema !== undefined ? { inputTypeScript: "unknown" } : {}), + ...(input.outputSchema !== undefined ? { outputTypeScript: "unknown" } : {}), + }; + const wrappedSchema = buildWrappedObjectSchema(properties, input.defs); + if (exceedsNodeLimit(wrappedSchema, MAX_PREVIEW_SCHEMA_NODES)) { + return unknownPreview; + } return Promise.resolve() .then(() => compile(wrappedSchema, ROOT_WRAPPER_NAME, compilerOptionsFrom(input.options ?? {}))) .then( (source) => previewToolFromCompiledTypeScript(source), - () => ({ - ...(input.inputSchema !== undefined ? { inputTypeScript: "unknown" } : {}), - ...(input.outputSchema !== undefined ? { outputTypeScript: "unknown" } : {}), - }), + () => unknownPreview, ); }; diff --git a/packages/core/sdk/src/vendor/json-schema-to-typescript/compat.ts b/packages/core/sdk/src/vendor/json-schema-to-typescript/compat.ts index 4e039008a1..bfdc6311b7 100644 --- a/packages/core/sdk/src/vendor/json-schema-to-typescript/compat.ts +++ b/packages/core/sdk/src/vendor/json-schema-to-typescript/compat.ts @@ -39,12 +39,21 @@ export const findKey = ( return undefined; }; +/** + * Memoize on object identity. The cache is a WeakMap so a memoized function + * installed at module scope (`generateType`, `getDefinitionsMemoized`) does + * not pin every AST and dereferenced schema it has ever seen for the lifetime + * of the isolate. Upstream uses a strong Map, which is harmless in a one-shot + * CLI but leaks the whole compiled graph per call inside a long-lived worker. + */ export const memoize = any>(fn: F): F => { - const cache = new Map[0], ReturnType>(); + // Every caller keys on a schema or AST node, which is always an object. + const cache = new WeakMap>(); return ((arg: Parameters[0], ...rest: unknown[]) => { - if (cache.has(arg)) return cache.get(arg); + const key = arg as object; + if (cache.has(key)) return cache.get(key); const value = fn(arg, ...rest); - cache.set(arg, value); + cache.set(key, value); return value; }) as F; };