From f4a13dd0f7339c95721da39a1dd10a50a580d193 Mon Sep 17 00:00:00 2001 From: Bernhard Seefeld Date: Mon, 17 Nov 2025 15:09:05 -0800 Subject: [PATCH 1/4] feat(llm-builtin): improve cell link serialization with @link format and any-schema handling - Use `@link` format instead of `/` for cell link serialization to LLM tools - Add `@arrayEntry` decorator for array elements that reference cells - Handle `any` schema types by converting to cell links when appropriate - Add base64 validation to prevent false positive link detection - Add timeout handling for tool calls (30s timeout) - Refactor chatbot.tsx to use patternTool helper for listMentionable - Fix schema.ts to properly copy values before annotation to avoid mutations --- packages/patterns/chatbot.tsx | 24 ++---- packages/runner/src/builtins/llm-dialog.ts | 99 ++++++++++++++++++---- packages/runner/src/link-utils.ts | 11 ++- packages/runner/src/schema.ts | 13 ++- packages/runner/test/link-utils.test.ts | 17 +++- 5 files changed, 126 insertions(+), 38 deletions(-) diff --git a/packages/patterns/chatbot.tsx b/packages/patterns/chatbot.tsx index e4bf74adf..90a6952c3 100644 --- a/packages/patterns/chatbot.tsx +++ b/packages/patterns/chatbot.tsx @@ -9,6 +9,7 @@ import { handler, llmDialog, NAME, + patternTool, recipe, Stream, UI, @@ -126,19 +127,16 @@ export const TitleGenerator = recipe< return title; }); -const listMentionable = handler< - { - /** A cell to store the result text */ - result: Cell; - }, - { mentionable: Cell[] } +const listMentionable = recipe< + { mentionable: Array }, + { result: Array<{ label: string; cell: Cell }> } >( - (args, state) => { - const namesList = state.mentionable.map((charm) => ({ - label: charm.get()[NAME], + ({ mentionable }) => { + const result = mentionable.map((charm) => ({ + label: charm[NAME]!, cell: charm, })); - args.result.set(JSON.stringify(namesList)); + return { result }; }, ); @@ -166,11 +164,7 @@ export default recipe( const recentCharms = schemaifyWish("#recent"); const assistantTools = { - listMentionable: { - description: - "List all mentionable items in the space, read() the result.", - handler: listMentionable({ mentionable }), - }, + listMentionable: patternTool(listMentionable, { mentionable }), listRecent: { description: "List all recently viewed charms in the space, read() the result.", diff --git a/packages/runner/src/builtins/llm-dialog.ts b/packages/runner/src/builtins/llm-dialog.ts index c45a2293f..21280c181 100644 --- a/packages/runner/src/builtins/llm-dialog.ts +++ b/packages/runner/src/builtins/llm-dialog.ts @@ -29,6 +29,7 @@ import { getCellOrThrow, isCellResultForDereferencing, } from "../query-result-proxy.ts"; +import { ContextualFlowControl } from "../cfc.ts"; // Avoid importing from @commontools/charm to prevent circular deps in tests @@ -39,6 +40,7 @@ const logger = getLogger("llm-dialog", { const client = new LLMClient(); const REQUEST_TIMEOUT = 1000 * 60 * 5; // 5 minutes +const TOOL_CALL_TIMEOUT = 1000 * 30 * 1; // 30 seconds /** * Remove the injected `result` field from a JSON schema so tools don't @@ -201,20 +203,42 @@ function createLLMFriendlyLink(link: NormalizedFullLink): string { */ function traverseAndSerialize( value: unknown, + schema: JSONSchema | undefined, + rootSchema: JSONSchema | undefined = schema, seen: Set = new Set(), ): unknown { if (!isRecord(value)) return value; + // If we encounter an `any` schema, turn value into a cell link + if ( + seen.size > 0 && schema !== undefined && + ContextualFlowControl.isTrueSchema(schema) && + isCellResultForDereferencing(value) + ) { + // Next step will turn this into a link + value = getCellOrThrow(value); + } + + // Turn cells into a link, unless they are data: URIs and traverse instead if (isCell(value)) { - const link = value.getAsNormalizedFullLink(); - return { "/": encodeJsonPointer(["", link.id, ...link.path]) }; + const link = value.resolveAsCell().getAsNormalizedFullLink(); + if (link.id.startsWith("data:")) { + return traverseAndSerialize(value.get(), schema, rootSchema, seen); + } else { + return { "@link": encodeJsonPointer(["", link.id, ...link.path]) }; + } } // If we've already seen this and it can be mapped to a cell, serialized as // cell link, otherwise throw (this should never happen in our cases) if (seen.has(value)) { if (isCellResultForDereferencing(value)) { - return traverseAndSerialize(getCellOrThrow(value), seen); + return traverseAndSerialize( + getCellOrThrow(value), + schema, + rootSchema, + seen, + ); } else { throw new Error( "Cannot serialize a value that has already been seen and cannot be mapped to a cell.", @@ -223,13 +247,43 @@ function traverseAndSerialize( } seen.add(value); + const cfc = new ContextualFlowControl(); + if (Array.isArray(value)) { - return value.map((v) => traverseAndSerialize(v, seen)); + return value.map((v, index) => { + const linkSchema = schema !== undefined + ? cfc.schemaAtPath(schema, [index.toString()], rootSchema) + : undefined; + let result = traverseAndSerialize(v, linkSchema, rootSchema, seen); + // Decorate array entries with links that point to underlying cells, if + // any. Ignores data: URIs, since they're not useful as links for the LLM. + if (isRecord(result) && isCellResultForDereferencing(v)) { + const link = getCellOrThrow(v).resolveAsCell() + .getAsNormalizedFullLink(); + if (!link.id.startsWith("data:")) { + result = { + "@arrayEntry": encodeJsonPointer(["", link.id, ...link.path]), + ...result, + }; + } + } + return result; + }); } else { return Object.fromEntries( - Object.entries(value).map(( + Object.entries(value as Record).map(( [key, value], - ) => [key, traverseAndSerialize(value, seen)]), + ) => [ + key, + traverseAndSerialize( + value, + schema !== undefined + ? cfc.schemaAtPath(schema, [key], rootSchema) + : undefined, + rootSchema, + seen, + ), + ]), ); } } @@ -254,10 +308,10 @@ function traverseAndCellify( // - it's a record with a single key "/" // - the value of the "/" key is a string that matches the URI pattern if ( - isRecord(value) && typeof value["/"] === "string" && - Object.keys(value).length === 1 && matchLLMFriendlyLink.test(value["/"]) + isRecord(value) && typeof value["@link"] === "string" && + Object.keys(value).length === 1 && matchLLMFriendlyLink.test(value["@link"]) ) { - const link = parseLLMFriendlyLink(value["/"], space); + const link = parseLLMFriendlyLink(value["@link"], space); return runtime.getCellFromLink(link); } if (Array.isArray(value)) { @@ -1363,15 +1417,14 @@ function handleSchema( */ function handleRead( resolved: ResolvedToolCall & { type: "read" }, -): { type: string; value: any } { - const serialized = traverseAndSerialize(resolved.cellRef.get()); - - // Handle undefined values gracefully - return null for undefined/null - const value = serialized === undefined || serialized === null - ? null - : JSON.parse(JSON.stringify(serialized)); +): { type: string; value: unknown } { + let cell = resolved.cellRef; + if (!cell.schema) { + cell = cell.asSchema(getCellSchema(cell)); + } + const serialized = traverseAndSerialize(cell.get(), cell.schema); - return { type: "json", value }; + return { type: "json", value: serialized }; } /** @@ -1476,7 +1529,17 @@ async function handleRun( const cancel = result.sink((r) => { r !== undefined && resolve(r); }); - await promise; + + let timeout; + const timeoutPromise = new Promise((_, reject) => { + timeout = setTimeout(() => { + reject(new Error("Tool call timed out")); + }, TOOL_CALL_TIMEOUT); + }).then(() => { + throw new Error("Tool call timed out"); + }); + await Promise.race([promise, timeoutPromise]); + clearTimeout(timeout); cancel(); // Get the actual entity ID from the result cell diff --git a/packages/runner/src/link-utils.ts b/packages/runner/src/link-utils.ts index 6958cf6d6..162e969a2 100644 --- a/packages/runner/src/link-utils.ts +++ b/packages/runner/src/link-utils.ts @@ -103,10 +103,13 @@ export function isJSONCellLink(value: any): value is LegacyJSONCellLink { isRecord(value) && isRecord(value.cell) && typeof value.cell["/"] === "string" && + base64Regex.test(value.cell["/"]) && Array.isArray(value.path) ); } +const base64Regex = new RegExp("^[A-Za-z0-9]+={0,2}$"); + /** * Check if value is a sigil link. */ @@ -148,7 +151,8 @@ export function isLink( isCellResultForDereferencing(value) || isAnyCellLink(value) || isCell(value) || - (isRecord(value) && "/" in value && typeof value["/"] === "string") // EntityId format + (isRecord(value) && "/" in value && Object.keys(value).length === 1 && + typeof value["/"] === "string" && base64Regex.test(value["/"])) // EntityId format ); } @@ -271,7 +275,10 @@ export function parseLink( }; } - if (isRecord(value) && "/" in value) { + if ( + isRecord(value) && "/" in value && Object.keys(value).length === 1 && + typeof value["/"] === "string" && base64Regex.test(value["/"]) + ) { return { id: toURI(value["/"]), path: [], diff --git a/packages/runner/src/schema.ts b/packages/runner/src/schema.ts index aaa651cdd..1802e361b 100644 --- a/packages/runner/src/schema.ts +++ b/packages/runner/src/schema.ts @@ -836,7 +836,18 @@ export function validateAndTransform( // Add the current value to seen before returning seen.push([seenKey, value]); - return annotateWithBackToCellSymbols(value, runtime, link, tx); + if (isRecord(value)) { + return annotateWithBackToCellSymbols( + isObject(value) + ? { ...(value as Record) } + : [...(value as unknown[])], + runtime, + link, + tx, + ); + } else { + return value; + } } /** diff --git a/packages/runner/test/link-utils.test.ts b/packages/runner/test/link-utils.test.ts index f6c3c6203..c49027b55 100644 --- a/packages/runner/test/link-utils.test.ts +++ b/packages/runner/test/link-utils.test.ts @@ -4,6 +4,7 @@ import { areLinksSame, createDataCellURI, createSigilLinkFromParsedLink, + isJSONCellLink, isLegacyAlias, isLink, isSigilValue, @@ -203,6 +204,18 @@ describe("link-utils", () => { }); }); + describe("isJSONCellLink", () => { + it("should identify JSON cell links", () => { + const jsonCellLink = { cell: { "/": "bafe==" }, path: ["test"] }; + expect(isJSONCellLink(jsonCellLink)).toBe(true); + }); + + it("should not allow of: URIs in JSON cell links", () => { + const jsonCellLink = { cell: { "/": "of:test" }, path: ["test"] }; + expect(isJSONCellLink(jsonCellLink)).toBe(false); + }); + }); + describe("parseLink", () => { it("should parse cells to normalized links", () => { const cell = runtime.getCell(space, "test", undefined, tx); @@ -384,14 +397,14 @@ describe("link-utils", () => { it("should parse JSON cell links to normalized links", () => { const jsonLink = { - cell: { "/": "of:test" }, + cell: { "/": "bafe==" }, path: ["nested", "value"], }; const baseCell = runtime.getCell(space, "base"); const result = parseLink(jsonLink, baseCell); expect(result).toEqual({ - id: "of:test", + id: "of:bafe==", path: ["nested", "value"], space: space, type: "application/json", From ae93e9879f462f98565332c9174b57b98f436453 Mon Sep 17 00:00:00 2001 From: Bernhard Seefeld Date: Mon, 17 Nov 2025 15:41:11 -0800 Subject: [PATCH 2/4] Revert link-utils changes to match main branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts the base64 validation and stricter link detection changes in link-utils.ts and link-utils.test.ts to align with the main branch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- packages/runner/src/link-utils.ts | 11 ++--------- packages/runner/test/link-utils.test.ts | 17 ++--------------- 2 files changed, 4 insertions(+), 24 deletions(-) diff --git a/packages/runner/src/link-utils.ts b/packages/runner/src/link-utils.ts index 162e969a2..6958cf6d6 100644 --- a/packages/runner/src/link-utils.ts +++ b/packages/runner/src/link-utils.ts @@ -103,13 +103,10 @@ export function isJSONCellLink(value: any): value is LegacyJSONCellLink { isRecord(value) && isRecord(value.cell) && typeof value.cell["/"] === "string" && - base64Regex.test(value.cell["/"]) && Array.isArray(value.path) ); } -const base64Regex = new RegExp("^[A-Za-z0-9]+={0,2}$"); - /** * Check if value is a sigil link. */ @@ -151,8 +148,7 @@ export function isLink( isCellResultForDereferencing(value) || isAnyCellLink(value) || isCell(value) || - (isRecord(value) && "/" in value && Object.keys(value).length === 1 && - typeof value["/"] === "string" && base64Regex.test(value["/"])) // EntityId format + (isRecord(value) && "/" in value && typeof value["/"] === "string") // EntityId format ); } @@ -275,10 +271,7 @@ export function parseLink( }; } - if ( - isRecord(value) && "/" in value && Object.keys(value).length === 1 && - typeof value["/"] === "string" && base64Regex.test(value["/"]) - ) { + if (isRecord(value) && "/" in value) { return { id: toURI(value["/"]), path: [], diff --git a/packages/runner/test/link-utils.test.ts b/packages/runner/test/link-utils.test.ts index c49027b55..f6c3c6203 100644 --- a/packages/runner/test/link-utils.test.ts +++ b/packages/runner/test/link-utils.test.ts @@ -4,7 +4,6 @@ import { areLinksSame, createDataCellURI, createSigilLinkFromParsedLink, - isJSONCellLink, isLegacyAlias, isLink, isSigilValue, @@ -204,18 +203,6 @@ describe("link-utils", () => { }); }); - describe("isJSONCellLink", () => { - it("should identify JSON cell links", () => { - const jsonCellLink = { cell: { "/": "bafe==" }, path: ["test"] }; - expect(isJSONCellLink(jsonCellLink)).toBe(true); - }); - - it("should not allow of: URIs in JSON cell links", () => { - const jsonCellLink = { cell: { "/": "of:test" }, path: ["test"] }; - expect(isJSONCellLink(jsonCellLink)).toBe(false); - }); - }); - describe("parseLink", () => { it("should parse cells to normalized links", () => { const cell = runtime.getCell(space, "test", undefined, tx); @@ -397,14 +384,14 @@ describe("link-utils", () => { it("should parse JSON cell links to normalized links", () => { const jsonLink = { - cell: { "/": "bafe==" }, + cell: { "/": "of:test" }, path: ["nested", "value"], }; const baseCell = runtime.getCell(space, "base"); const result = parseLink(jsonLink, baseCell); expect(result).toEqual({ - id: "of:bafe==", + id: "of:test", path: ["nested", "value"], space: space, type: "application/json", From d3a7eb4903e915fe4ec4bb799ae75605602fea3a Mon Sep 17 00:00:00 2001 From: Bernhard Seefeld Date: Mon, 17 Nov 2025 15:47:05 -0800 Subject: [PATCH 3/4] fix(runner): fix three critical bugs in schema validation and llm-dialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Fix seen cache tracking for cloned objects in validateAndTransform - Push cloned value to seen cache instead of original value - Ensures future circular reference checks return properly annotated objects 2. Handle undefined cell values in handleRead tool - Convert undefined to null (valid JSON) to comply with Anthropic's tool_result contract - Prevents invalid JSON responses when cell values are undefined 3. Prevent resource leaks in tool call execution - Wrap cleanup code (clearTimeout/cancel) in finally block - Ensures resources are freed even when tool calls timeout or reject 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- packages/runner/src/builtins/llm-dialog.ts | 13 +++++++++---- packages/runner/src/schema.ts | 10 ++++++---- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/packages/runner/src/builtins/llm-dialog.ts b/packages/runner/src/builtins/llm-dialog.ts index 21280c181..a03720ed7 100644 --- a/packages/runner/src/builtins/llm-dialog.ts +++ b/packages/runner/src/builtins/llm-dialog.ts @@ -1424,7 +1424,8 @@ function handleRead( } const serialized = traverseAndSerialize(cell.get(), cell.schema); - return { type: "json", value: serialized }; + // Handle undefined by returning null (valid JSON) instead + return { type: "json", value: serialized === undefined ? null : serialized }; } /** @@ -1538,9 +1539,13 @@ async function handleRun( }).then(() => { throw new Error("Tool call timed out"); }); - await Promise.race([promise, timeoutPromise]); - clearTimeout(timeout); - cancel(); + + try { + await Promise.race([promise, timeoutPromise]); + } finally { + clearTimeout(timeout); + cancel(); + } // Get the actual entity ID from the result cell const resultLink = createLLMFriendlyLink(result.getAsNormalizedFullLink()); diff --git a/packages/runner/src/schema.ts b/packages/runner/src/schema.ts index 1802e361b..d34d3b986 100644 --- a/packages/runner/src/schema.ts +++ b/packages/runner/src/schema.ts @@ -835,17 +835,19 @@ export function validateAndTransform( } // Add the current value to seen before returning - seen.push([seenKey, value]); if (isRecord(value)) { + const cloned = isObject(value) + ? { ...(value as Record) } + : [...(value as unknown[])]; + seen.push([seenKey, cloned]); return annotateWithBackToCellSymbols( - isObject(value) - ? { ...(value as Record) } - : [...(value as unknown[])], + cloned, runtime, link, tx, ); } else { + seen.push([seenKey, value]); return value; } } From fb068024966a554586493d3600a3fb76ff61b97f Mon Sep 17 00:00:00 2001 From: Bernhard Seefeld Date: Mon, 17 Nov 2025 15:55:03 -0800 Subject: [PATCH 4/4] output schema on read --- packages/runner/src/builtins/llm-dialog.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/runner/src/builtins/llm-dialog.ts b/packages/runner/src/builtins/llm-dialog.ts index a03720ed7..1c9ac5dd1 100644 --- a/packages/runner/src/builtins/llm-dialog.ts +++ b/packages/runner/src/builtins/llm-dialog.ts @@ -1422,10 +1422,12 @@ function handleRead( if (!cell.schema) { cell = cell.asSchema(getCellSchema(cell)); } - const serialized = traverseAndSerialize(cell.get(), cell.schema); + + const schema = cell.schema; + const serialized = traverseAndSerialize(cell.get(), schema); // Handle undefined by returning null (valid JSON) instead - return { type: "json", value: serialized === undefined ? null : serialized }; + return { type: "json", value: serialized ?? null, ...(schema && { schema }) }; } /**