From 1b2f70cb8513ab2247ea70772f315f3fc949cfa4 Mon Sep 17 00:00:00 2001 From: Luke Melia Date: Mon, 13 Jul 2026 14:19:08 -0400 Subject: [PATCH 1/2] Move skill tool schema generation into SkillFrontmatterField.fromFrontmatter Review feedback on the index-time tool schema stamping asked whether the schema work could live in SkillFrontmatterField.fromFrontmatter instead of a skill-shape-aware stamping pass inside the generic host file extractor. It can: fromFrontmatter is now async and receives a context ({ fileURL, toolContext? }); the skill subclass generates tool definitions itself (via import.meta.loader, direct card-api import, and basicMappings), and hands back a structured result. The enriched frontmatter (resource-only) and any ToolSchemaErrors ride back to the extractor on the same out-of-band symbol channels the frontmatter parse error uses, so schemas still never land in search_doc. The file-extract route now mints the owner-carrying ToolContext and threads it through extractAttributes options; its presence is what gates the module-loading schema work to the indexing path. Also renames the stamped ToolField JsonField from `tool` to `definition` per review, and adds a loader shim for @cardstack/runtime-common/helpers/ai so base modules can value-import basicMappings. Co-Authored-By: Claude Fable 5 --- packages/base/frontmatter-field.gts | 45 +++- packages/base/markdown-file-def.gts | 35 ++- packages/base/skill-frontmatter-field.gts | 235 ++++++++++++++++- packages/base/tool-field.gts | 6 +- packages/host/app/lib/externals.ts | 4 + .../host/app/routes/render/file-extract.ts | 15 +- .../utils/file-def-attributes-extractor.ts | 245 +++--------------- .../prerender-file-extract-test.gts | 19 +- .../tests/integration/realm-indexing-test.gts | 4 +- packages/runtime-common/index.ts | 19 ++ 10 files changed, 383 insertions(+), 244 deletions(-) diff --git a/packages/base/frontmatter-field.gts b/packages/base/frontmatter-field.gts index a423cfabf6e..7caca8f1133 100644 --- a/packages/base/frontmatter-field.gts +++ b/packages/base/frontmatter-field.gts @@ -1,3 +1,5 @@ +import type { ToolContext, ToolSchemaError } from '@cardstack/runtime-common'; + import { Component, FieldDef, @@ -7,6 +9,34 @@ import { } from './card-api'; import { JsonField } from './json-field'; +// Extraction-time inputs handed to `fromFrontmatter` by +// `MarkdownDef.extractAttributes`. +export interface FromFrontmatterContext { + // The markdown file's URL; relative references in the frontmatter (e.g. a + // skill tool's codeRef module) resolve against it. + fileURL: string; + // Owner-carrying context for constructing tool classes during index-time + // schema generation. Only the indexing path provides one; its presence is + // what enables the (module-loading, hence costly) schema work, so + // interactive extract paths never pay for it. + toolContext?: ToolContext; +} + +// What `fromFrontmatter` hands back to `MarkdownDef.extractAttributes`. +export interface FromFrontmatterResult { + // The frontmatter field's serialized attributes — the value that lands in + // the search doc and, absent `fileMetaAttributes`, in the file-meta + // resource too. + attributes: Record; + // Index-only enrichment of `attributes` (e.g. a skill's generated tool + // definitions) destined for the file-meta resource. Kept separate because + // multi-KB generated content must never land in `search_doc`. + fileMetaAttributes?: Record; + // Skill frontmatter tools whose schema generation failed. The extract + // still succeeds; these surface via `diagnostics.toolSchemaErrors`. + toolSchemaErrors?: ToolSchemaError[]; +} + // The parsed YAML frontmatter of a markdown file, captured as JSON. The base // type holds the entire frontmatter in `rawContent`; when the frontmatter // declares a recognized `boxel.kind` (e.g. `skill`), the concrete instance is a @@ -22,12 +52,13 @@ export class FrontmatterField extends FieldDef { // Map a file's parsed frontmatter into this field's serialized attributes. // The base keeps the whole frontmatter as the raw copy; subclasses add their - // own typed fields. A subclass is the only thing that knows its own - // frontmatter schema. - static fromFrontmatter( + // own typed fields and any index-time enrichment of them. A subclass is the + // only thing that knows its own frontmatter schema. + static async fromFrontmatter( frontmatter: Record, - ): Record { - return { rawContent: frontmatter }; + _context?: FromFrontmatterContext, + ): Promise { + return { attributes: { rawContent: frontmatter } }; } static embedded: BaseDefComponent = class Embedded extends Component< @@ -39,8 +70,6 @@ export class FrontmatterField extends FieldDef { | undefined; return raw?.boxel?.kind ?? ''; } - + }; } diff --git a/packages/base/markdown-file-def.gts b/packages/base/markdown-file-def.gts index 574003808f0..330243d7182 100644 --- a/packages/base/markdown-file-def.gts +++ b/packages/base/markdown-file-def.gts @@ -2,9 +2,12 @@ import { byteStreamToUint8Array, extractCardReferenceUrls, extractFileReferenceUrls, + FRONTMATTER_FILE_META_VALUE_SYMBOL, FRONTMATTER_PARSE_ERROR_SYMBOL, identifyCard, + TOOL_SCHEMA_ERRORS_SYMBOL, type FrontmatterParseError, + type ToolContext, } from '@cardstack/runtime-common'; import MarkdownIcon from '@cardstack/boxel-icons/align-box-left-middle'; import { @@ -548,7 +551,10 @@ export class MarkdownDef extends FileDef { static async extractAttributes( url: string, getStream: () => Promise, - options: { contentHash?: string } = {}, + // `toolContext` is the owner-carrying context index-time tool schema + // generation constructs tool classes with; only the indexing path + // provides one (see `FromFrontmatterContext`). + options: { contentHash?: string; toolContext?: ToolContext } = {}, ): Promise< SerializedFile<{ title: string; @@ -640,17 +646,32 @@ export class MarkdownDef extends FileDef { } // `boxel.kind` selects the FrontmatterField subclass; the subclass maps the - // parsed frontmatter into its own field value (the base keeps the raw copy). - // MarkdownDef stays ignorant of any kind's schema. A recognized kind is - // recorded so the field rehydrates as that subclass on read. + // parsed frontmatter into its own field value (the base keeps the raw copy) + // and produces any index-time enrichment of it (e.g. a skill's generated + // tool definitions). MarkdownDef stays ignorant of any kind's schema. A + // recognized kind is recorded so the field rehydrates as that subclass on + // read; the enriched copy and any tool schema errors ride out-of-band on + // the same symbol channels the parse error uses, so neither leaks into the + // flat `search_doc`. if (Object.keys(frontmatterData).length > 0) { let frontmatterFieldClass = frontmatterFieldForKind(kind); - attributes.frontmatter = - frontmatterFieldClass.fromFrontmatter(frontmatterData); + let frontmatterResult = await frontmatterFieldClass.fromFrontmatter( + frontmatterData, + { fileURL: url, toolContext: options.toolContext }, + ); + attributes.frontmatter = frontmatterResult.attributes; + let bag = attributes as Record; + if (frontmatterResult.fileMetaAttributes) { + bag[FRONTMATTER_FILE_META_VALUE_SYMBOL] = + frontmatterResult.fileMetaAttributes; + } + if (frontmatterResult.toolSchemaErrors?.length) { + bag[TOOL_SCHEMA_ERRORS_SYMBOL] = frontmatterResult.toolSchemaErrors; + } if (isKnownFrontmatterKind(kind)) { let adoptsFrom = identifyCard(frontmatterFieldClass); if (adoptsFrom) { - (attributes as Record)[fileFieldMetaSymbol] = { + bag[fileFieldMetaSymbol] = { frontmatter: { adoptsFrom }, }; } diff --git a/packages/base/skill-frontmatter-field.gts b/packages/base/skill-frontmatter-field.gts index 9e39039dd99..ed8634f0fc9 100644 --- a/packages/base/skill-frontmatter-field.gts +++ b/packages/base/skill-frontmatter-field.gts @@ -1,3 +1,16 @@ +import { + buildToolFunctionNameFromResolvedRef, + codeRefWithAbsoluteIdentifier, + getClass, + rri, + type Loader, + type ResolvedCodeRef, + type ToolContext, + type ToolSchemaError, +} from '@cardstack/runtime-common'; +import { basicMappings } from '@cardstack/runtime-common/helpers/ai'; + +import * as CardAPI from './card-api'; import { Component, field, @@ -6,9 +19,15 @@ import { type BaseDefComponent, } from './card-api'; import StringField from './string'; -import { FrontmatterField } from './frontmatter-field'; +import { + FrontmatterField, + type FromFrontmatterContext, + type FromFrontmatterResult, +} from './frontmatter-field'; import { ToolField } from './tool-field'; +import type { Tool as LLMTool } from './matrix-event'; + // A skill markdown file's frontmatter (`boxel.kind: skill`). Adds typed fields // on top of the base `FrontmatterField` (which holds the raw frontmatter in // `rawContent`). Mirrors the field shape of the legacy `Skill` card so the @@ -32,28 +51,226 @@ export class SkillFrontmatterField extends FrontmatterField { // `tools` from the `boxel:` namespace (`boxel.tools`, with the pre-rename // `boxel.commands` key still accepted; `tools` wins when both are present). // Only this subclass knows that mapping. - static fromFrontmatter( + // + // When the context carries a `toolContext` (the indexing path), each tool + // additionally gets its LLM tool definition generated and stamped — with + // the resolved absolute codeRef, functionName, and normalized + // requiresApproval — into `fileMetaAttributes`, so consumers (ai-bot + // first) obtain ready-to-use tool definitions from the index without a + // module loader. The search-doc `attributes` keep the tools as authored. + static async fromFrontmatter( frontmatter: Record, - ): Record { + context?: FromFrontmatterContext, + ): Promise { + let { attributes: base } = await super.fromFrontmatter( + frontmatter, + context, + ); let boxel = frontmatter.boxel && typeof frontmatter.boxel === 'object' && !Array.isArray(frontmatter.boxel) ? (frontmatter.boxel as Record) : undefined; - return { - ...super.fromFrontmatter(frontmatter), + let authored = boxel?.tools ?? boxel?.commands; + let attributes = { + ...base, name: frontmatter.name, description: frontmatter.description, - tools: boxel?.tools ?? boxel?.commands, + tools: authored, + }; + if ( + !context?.toolContext || + !Array.isArray(authored) || + authored.length === 0 + ) { + return { attributes }; + } + let enrichment: Awaited>; + try { + enrichment = await generateToolDefinitions( + authored as Record[], + context.fileURL, + context.toolContext, + ); + } catch (err) { + // Schema generation must never fail the file row — the skill still + // indexes with its tools as authored. Failures inside the generation + // are already attributed per tool; anything reaching here is a bug in + // the generation code itself. + console.warn( + `[skill-frontmatter] tool schema generation failed for ${context.fileURL}:`, + err, + ); + return { attributes }; + } + let { tools, toolSchemaErrors } = enrichment; + return { + attributes, + fileMetaAttributes: { ...attributes, tools }, + ...(toolSchemaErrors.length ? { toolSchemaErrors } : {}), }; } static embedded: BaseDefComponent = class Embedded extends Component< typeof this > { - + + }; +} + +// Generate each authored tool's LLM tool definition. A tool that fails +// (module won't load, missing export, schema generation throws) stays in the +// returned list as authored and contributes a `ToolSchemaError`; the +// remaining tools still enrich. +// +// Loading realm-hosted tool modules through the loader also records them as +// runtime dependencies of the surrounding extract, so editing such a module +// reindexes the referencing skill. Host-package tool modules +// (`@cardstack/boxel-host/...`) resolve inside the host bundle and don't +// participate in invalidation — their stamped schemas can go stale across +// host deploys until the skill's realm reindexes. +async function generateToolDefinitions( + authoredTools: Record[], + fileURL: string, + toolContext: ToolContext, +): Promise<{ + tools: Record[]; + toolSchemaErrors: ToolSchemaError[]; +}> { + // The authored coordinates, coerced once — used both to validate an entry + // and to name it in an error. + let coordinatesOf = (entry: Record) => ({ + module: + typeof entry?.codeRef?.module === 'string' ? entry.codeRef.module : '', + name: typeof entry?.codeRef?.name === 'string' ? entry.codeRef.name : '', + }); + + let loader = myLoader(); + let mappings: Awaited>; + try { + mappings = await basicMappings(loader); + } catch (err) { + // Setup failed before any tool could be attempted (e.g. a transient + // failure loading the base field modules). Attribute the failure to + // every declared tool so the diagnostics name real coordinates instead + // of a single anonymous entry. + console.warn( + `[skill-frontmatter] tool schema generation failed for ${fileURL}:`, + err, + ); + let message = `tool schema generation failed before this tool was attempted: ${ + err instanceof Error ? err.message : String(err) + }`; + return { + tools: authoredTools, + toolSchemaErrors: authoredTools.map((entry) => ({ + ...coordinatesOf(entry), + message, + })), + }; + } + + let skillURL = new URL(fileURL); + // Tools are independent; the loader dedupes concurrent module imports, so + // enriching in parallel overlaps the per-tool module fetches. + let results = await Promise.all( + authoredTools.map( + async ( + entry, + ): Promise<{ + entry: Record; + error?: ToolSchemaError; + }> => { + let { module, name } = coordinatesOf(entry); + if (!module || !name) { + return { + entry, + error: { + module, + name, + message: 'tool entry is missing a codeRef module/name', + }, + }; + } + // Resolve in RRI space (no VirtualNetwork), matching how ToolField + // computes `functionName` — so the stamped name and a host-side + // recomputation from the stamped codeRef always agree. Package + // specifiers pass through verbatim. + let resolvedRef = codeRefWithAbsoluteIdentifier( + { module: rri(module), name }, + skillURL, + undefined, + ) as ResolvedCodeRef; + try { + let ToolClass = await getClass(resolvedRef, loader); + if (typeof ToolClass !== 'function') { + throw new Error( + `module does not export a tool class named "${resolvedRef.name}"`, + ); + } + let tool = new ToolClass(toolContext); + let functionName = buildToolFunctionNameFromResolvedRef(resolvedRef); + let definition = { + type: 'function' as LLMTool['type'], + function: { + name: functionName, + description: tool.description, + parameters: { + type: 'object', + properties: { + description: { + type: 'string', + }, + ...(await tool.getInputJsonSchema(CardAPI, mappings)), + }, + required: ['attributes', 'description'], + }, + }, + }; + return { + entry: { + ...entry, + codeRef: resolvedRef, + // Absent means "requires approval"; stamp that decision so + // consumers don't each re-implement the default. + requiresApproval: entry.requiresApproval !== false, + functionName, + definition, + }, + }; + } catch (err) { + console.warn( + `[skill-frontmatter] tool schema generation failed for ${resolvedRef.module}#${resolvedRef.name} (skill ${fileURL}):`, + err, + ); + return { + entry, + error: { + module: resolvedRef.module, + name: resolvedRef.name, + message: err instanceof Error ? err.message : String(err), + }, + }; + } + }, + ), + ); + return { + tools: results.map((result) => result.entry), + toolSchemaErrors: results.flatMap((result) => + result.error ? [result.error] : [], + ), }; } + +function myLoader(): Loader { + // we know this code is always loaded by an instance of our Loader, which + // sets import.meta.loader. + + // When type-checking realm-server, tsc sees this file and thinks it will be + // transpiled to CommonJS and so it complains about this line. But this file + // is always loaded through our loader and always has access to import.meta. + // @ts-ignore + return (import.meta as any).loader; +} diff --git a/packages/base/tool-field.gts b/packages/base/tool-field.gts index 0d1916d21e7..67a0789d7c8 100644 --- a/packages/base/tool-field.gts +++ b/packages/base/tool-field.gts @@ -43,9 +43,9 @@ export class ToolField extends FieldDef { // class's input schema at indexing time and stamped onto the skill's // file-meta resource. Present only on tools rehydrated from an enriched // index row — a tool authored in frontmatter has no value here until the - // file indexes. Consumers that need a schema and find none must generate - // it themselves (see the host's `uploadToolDefinitions`). - @field tool = contains(JsonField); + // file indexes. Consumers that need a definition and find none must + // generate it themselves (see the host's `uploadToolDefinitions`). + @field definition = contains(JsonField); @field functionName = contains(StringField, { description: 'The name of the function to be executed', diff --git a/packages/host/app/lib/externals.ts b/packages/host/app/lib/externals.ts index 29c76c1a745..2d9c57f44c4 100644 --- a/packages/host/app/lib/externals.ts +++ b/packages/host/app/lib/externals.ts @@ -206,6 +206,10 @@ export function shimExternals(virtualNetwork: VirtualNetwork) { id: '@cardstack/runtime-common/bfm-card-references', resolve: () => import('@cardstack/runtime-common/bfm-card-references'), }); + virtualNetwork.shimAsyncModule({ + id: '@cardstack/runtime-common/helpers/ai', + resolve: () => import('@cardstack/runtime-common/helpers/ai'), + }); shimModulesForLiveTests(virtualNetwork); diff --git a/packages/host/app/routes/render/file-extract.ts b/packages/host/app/routes/render/file-extract.ts index 5f0410eabfa..21d0cdd7f80 100644 --- a/packages/host/app/routes/render/file-extract.ts +++ b/packages/host/app/routes/render/file-extract.ts @@ -1,4 +1,5 @@ import { registerDestructor } from '@ember/destroyable'; +import { getOwner, setOwner } from '@ember/owner'; import Route from '@ember/routing/route'; import type Transition from '@ember/routing/transition'; import { service } from '@ember/service'; @@ -10,9 +11,11 @@ import { baseRealm, formattedError, snapshotRuntimeDependencies, + ToolContextStamp, trackRuntimeModuleDependency, withRuntimeDependencyTrackingContext, type RenderError, + type ToolContext, } from '@cardstack/runtime-common'; import { errorJsonApiToErrorEntry } from '../../lib/window-error-handler'; @@ -88,6 +91,14 @@ export default class RenderFileExtractRoute extends Route { let fileDefCodeRef = parsedOptions.fileDefCodeRef ?? baseFileRef; let contentHash: string | undefined = parsedOptions.fileContentHash; let contentSize: number | undefined = parsedOptions.fileContentSize; + // This route is the indexing path, so skill tool schemas are generated + // during the extract and persisted with the row. Tool classes never read + // the context during schema generation, but host-package tools resolve + // services (e.g. the loader) through the context's owner at construction — + // so the context must carry one, the same shape the tool service hands to + // real invocations. + let toolContext: ToolContext = { [ToolContextStamp]: true }; + setOwner(toolContext, getOwner(this)!); let extractor = new FileDefAttributesExtractor({ loaderService: this.loaderService, network: this.network, @@ -98,9 +109,7 @@ export default class RenderFileExtractRoute extends Route { contentHash, contentSize, buildError: this.#buildError.bind(this), - // This route is the indexing path, so skill tool schemas are generated - // here and persisted with the row. - generateToolSchemas: true, + toolContext, }); let fileApiURL = `${baseRealm.url}file-api`; let result: FileDefExtractResult; diff --git a/packages/host/app/utils/file-def-attributes-extractor.ts b/packages/host/app/utils/file-def-attributes-extractor.ts index 9612c3db469..2b22c2cd537 100644 --- a/packages/host/app/utils/file-def-attributes-extractor.ts +++ b/packages/host/app/utils/file-def-attributes-extractor.ts @@ -1,20 +1,15 @@ -import { getOwner, setOwner } from '@ember/owner'; - import { isEqual } from 'lodash-es'; import { baseRef, - buildToolFunctionNameFromResolvedRef, CardError, - codeRefWithAbsoluteIdentifier, + FRONTMATTER_FILE_META_VALUE_SYMBOL, FRONTMATTER_PARSE_ERROR_SYMBOL, - getClass, identifyCard, inferContentType, internalKeyFor, - rri, SupportedMimeType, - ToolContextStamp, + TOOL_SCHEMA_ERRORS_SYMBOL, type CodeRef, type FileMetaResource, type FrontmatterParseError, @@ -26,7 +21,6 @@ import { type ToolSchemaError, } from '@cardstack/runtime-common'; import { getFieldDefinitions } from '@cardstack/runtime-common/definitions'; -import { basicMappings } from '@cardstack/runtime-common/helpers/ai'; import type { createAuthErrorGuard } from './auth-error-guard'; @@ -34,13 +28,16 @@ import type LoaderService from '../services/loader-service'; import type NetworkService from '../services/network'; import type { BaseDef } from '@cardstack/base/card-api'; import type * as CardAPI from '@cardstack/base/card-api'; -import type { Tool as LLMTool } from '@cardstack/base/matrix-event'; export type FileDefExport = { extractAttributes: ( url: string, getStream: () => Promise, - options?: { contentHash?: string; contentSize?: number }, + options?: { + contentHash?: string; + contentSize?: number; + toolContext?: ToolContext; + }, ) => Promise; }; export type FileDefModule = Record; @@ -77,7 +74,7 @@ export class FileDefAttributesExtractor { #contentSize: number | undefined; #fileBytes: Uint8Array | undefined; #buildError: (url: string, error: unknown) => RenderError; - #generateToolSchemas: boolean; + #toolContext: ToolContext | undefined; #fallbackBytes: Uint8Array | null = null; #primaryUsed = false; @@ -92,7 +89,7 @@ export class FileDefAttributesExtractor { contentSize, fileBytes, buildError, - generateToolSchemas, + toolContext, }: { loaderService: LoaderService; network: NetworkService; @@ -104,12 +101,13 @@ export class FileDefAttributesExtractor { contentSize: number | undefined; fileBytes?: Uint8Array; buildError: (url: string, error: unknown) => RenderError; - // Generate LLM tool definitions for a skill's frontmatter tools and stamp - // them onto the file-meta resource. Indexing-only (the file-extract - // render route): it loads each tool's module, which the interactive + // Owner-carrying context that index-time tool schema generation + // constructs tool classes with (see SkillFrontmatterField.fromFrontmatter). + // Only the indexing path (the file-extract render route) provides one: + // schema generation loads each tool's module, which the interactive // extract paths (room file attach, the store's direct fallback) shouldn't // pay for — their consumers generate schemas on demand instead. - generateToolSchemas?: boolean; + toolContext?: ToolContext; }) { this.#loaderService = loaderService; this.#network = network; @@ -121,7 +119,7 @@ export class FileDefAttributesExtractor { this.#contentSize = contentSize; this.#fileBytes = fileBytes; this.#buildError = buildError; - this.#generateToolSchemas = generateToolSchemas ?? false; + this.#toolContext = toolContext; } async extract(): Promise { @@ -175,7 +173,11 @@ export class FileDefAttributesExtractor { return await klass.extractAttributes( this.#fileURL, this.#getStreamForAttempt, - { contentHash: this.#contentHash, contentSize: this.#contentSize }, + { + contentHash: this.#contentHash, + contentSize: this.#contentSize, + toolContext: this.#toolContext, + }, ); } catch (err) { console.warn( @@ -251,6 +253,24 @@ export class FileDefAttributesExtractor { if (frontmatterParseError) { delete cleanedBag[FRONTMATTER_PARSE_ERROR_SYMBOL]; } + // And for a skill's tool schema failures, persisted onto + // `diagnostics.toolSchemaErrors`. + let toolSchemaErrors = cleanedBag[TOOL_SCHEMA_ERRORS_SYMBOL] as + | ToolSchemaError[] + | undefined; + if (toolSchemaErrors) { + delete cleanedBag[TOOL_SCHEMA_ERRORS_SYMBOL]; + } + // A frontmatter value enriched for the index (e.g. a skill's + // generated tool definitions) replaces the resource's `frontmatter` + // only; the search doc keeps the value as authored, because multi-KB + // generated content must never land in `search_doc`. + let fileMetaFrontmatter = cleanedBag[ + FRONTMATTER_FILE_META_VALUE_SYMBOL + ] as Record | undefined; + if (fileMetaFrontmatter) { + delete cleanedBag[FRONTMATTER_FILE_META_VALUE_SYMBOL]; + } let resource = buildFileResource( this.#fileURL, cleanedDoc, @@ -258,20 +278,9 @@ export class FileDefAttributesExtractor { queryFieldDefs, fieldsMeta, ); - let toolSchemaErrors: ToolSchemaError[] | undefined; - if (this.#generateToolSchemas) { - try { - toolSchemaErrors = await this.stampSkillToolSchemas(resource); - } catch (err) { - // Schema generation must never fail the file row — the skill - // still indexes instructions-only. Failures inside the stamp are - // already attributed per tool; anything reaching here is a bug in - // the stamping code itself. - console.warn( - `[file-extract] tool schema generation failed for ${this.#fileURL}:`, - err, - ); - } + if (fileMetaFrontmatter) { + (resource.attributes as Record).frontmatter = + fileMetaFrontmatter; } return { status: 'ready', @@ -383,178 +392,6 @@ export class FileDefAttributesExtractor { return response; } - // For a skill markdown file (`kind: 'skill'`), generate each frontmatter - // tool's LLM tool definition and stamp it — with the resolved absolute - // codeRef, functionName, and normalized requiresApproval — onto the - // file-meta resource, so consumers (ai-bot first) obtain ready-to-use tool - // definitions from the index without a module loader. Runs after - // `buildFileResource` and replaces the resource's `frontmatter` with an - // enriched copy: the search doc shares the original object by reference, - // and multi-KB schemas must never land in `search_doc`. - // - // A tool that fails (module won't load, missing export, schema generation - // throws) stays in the tools list as authored and contributes a - // `ToolSchemaError`; the remaining tools still enrich. - // - // Loading realm-hosted tool modules through the loader also records them - // as runtime dependencies of this extract, so editing such a module - // reindexes the referencing skill. Host-package tool modules - // (`@cardstack/boxel-host/...`) resolve inside the host bundle and don't - // participate in invalidation — their stamped schemas can go stale across - // host deploys until the skill's realm reindexes. - private async stampSkillToolSchemas( - resource: FileMetaResource, - ): Promise { - let attributes = resource.attributes as Record; - // A fresh extract always serializes skill tools under `tools` — the - // frontmatter field maps the pre-rename `boxel.commands` key there too - // (see SkillFrontmatterField.fromFrontmatter). - let authoredTools = - attributes.kind === 'skill' && - Array.isArray(attributes.frontmatter?.tools) - ? (attributes.frontmatter.tools as Record[]) - : undefined; - if (!authoredTools?.length) { - return undefined; - } - - // The authored coordinates, coerced once — used both to validate an - // entry and to name it in an error. - let coordinatesOf = (entry: Record) => ({ - module: - typeof entry?.codeRef?.module === 'string' ? entry.codeRef.module : '', - name: typeof entry?.codeRef?.name === 'string' ? entry.codeRef.name : '', - }); - - let loader = this.#loaderService.loader; - let cardApi: typeof CardAPI; - let mappings: Awaited>; - let toolContext: ToolContext; - try { - [cardApi, mappings] = await Promise.all([ - loader.import('https://cardstack.com/base/card-api'), - basicMappings(loader), - ]); - // Tool classes never read the context during schema generation, but - // host-package tools resolve services (e.g. the loader) through the - // context's owner at construction — so the stamp object must carry - // one, the same shape the tool service hands to real invocations. - toolContext = { [ToolContextStamp]: true }; - setOwner(toolContext, getOwner(this.#loaderService)!); - } catch (err) { - // Setup failed before any tool could be attempted (e.g. a transient - // failure loading the base modules). Attribute the failure to every - // declared tool so the diagnostics name real coordinates instead of a - // single anonymous entry. - console.warn( - `[file-extract] tool schema generation failed for ${this.#fileURL}:`, - err, - ); - let message = `tool schema generation failed before this tool was attempted: ${ - err instanceof Error ? err.message : String(err) - }`; - return authoredTools.map((entry) => ({ - ...coordinatesOf(entry), - message, - })); - } - - let skillURL = new URL(this.#fileURL); - // Tools are independent; the loader dedupes concurrent module imports, - // so enriching in parallel overlaps the per-tool module fetches. - let results = await Promise.all( - authoredTools.map( - async ( - entry, - ): Promise<{ - entry: Record; - error?: ToolSchemaError; - }> => { - let { module, name } = coordinatesOf(entry); - if (!module || !name) { - return { - entry, - error: { - module, - name, - message: 'tool entry is missing a codeRef module/name', - }, - }; - } - // Resolve in RRI space (no VirtualNetwork), matching how ToolField - // computes `functionName` — so the stamped name and a host-side - // recomputation from the stamped codeRef always agree. Package - // specifiers pass through verbatim. - let resolvedRef = codeRefWithAbsoluteIdentifier( - { module: rri(module), name }, - skillURL, - undefined, - ) as ResolvedCodeRef; - try { - let ToolClass = await getClass(resolvedRef, loader); - if (typeof ToolClass !== 'function') { - throw new Error( - `module does not export a tool class named "${resolvedRef.name}"`, - ); - } - let tool = new ToolClass(toolContext); - let functionName = - buildToolFunctionNameFromResolvedRef(resolvedRef); - let toolDefinition = { - type: 'function' as LLMTool['type'], - function: { - name: functionName, - description: tool.description, - parameters: { - type: 'object', - properties: { - description: { - type: 'string', - }, - ...(await tool.getInputJsonSchema(cardApi, mappings)), - }, - required: ['attributes', 'description'], - }, - }, - }; - return { - entry: { - ...entry, - codeRef: resolvedRef, - // Absent means "requires approval"; stamp that decision so - // consumers don't each re-implement the default. - requiresApproval: entry.requiresApproval !== false, - functionName, - tool: toolDefinition, - }, - }; - } catch (err) { - console.warn( - `[file-extract] tool schema generation failed for ${resolvedRef.module}#${resolvedRef.name} (skill ${this.#fileURL}):`, - err, - ); - return { - entry, - error: { - module: resolvedRef.module, - name: resolvedRef.name, - message: err instanceof Error ? err.message : String(err), - }, - }; - } - }, - ), - ); - let errors = results.flatMap((result) => - result.error ? [result.error] : [], - ); - attributes.frontmatter = { - ...attributes.frontmatter, - tools: results.map((result) => result.entry), - }; - return errors.length ? errors : undefined; - } - // Extract query field definitions (linksTo/linksToMany with a query) from // the FileDef class so they can be stored in the index and used at serve // time without a runtime definition lookup. diff --git a/packages/host/tests/acceptance/prerender-file-extract-test.gts b/packages/host/tests/acceptance/prerender-file-extract-test.gts index e9ef864fce5..d0f3cd09f66 100644 --- a/packages/host/tests/acceptance/prerender-file-extract-test.gts +++ b/packages/host/tests/acceptance/prerender-file-extract-test.gts @@ -365,11 +365,14 @@ boxel: assert.strictEqual(realmTool.codeRef.name, 'default'); assert.strictEqual(realmTool.functionName, expectedRealmFn); assert.false(realmTool.requiresApproval, 'authored false is preserved'); - assert.strictEqual(realmTool.tool.type, 'function'); - assert.strictEqual(realmTool.tool.function.name, expectedRealmFn); - assert.strictEqual(realmTool.tool.function.description, 'Sends a greeting'); + assert.strictEqual(realmTool.definition.type, 'function'); + assert.strictEqual(realmTool.definition.function.name, expectedRealmFn); + assert.strictEqual( + realmTool.definition.function.description, + 'Sends a greeting', + ); assert.ok( - realmTool.tool.function.parameters.properties.attributes.properties + realmTool.definition.function.parameters.properties.attributes.properties .greeting, 'generated schema carries the input card fields', ); @@ -392,7 +395,7 @@ boxel: 'absent requiresApproval stamps as true', ); assert.ok( - hostTool.tool.function.parameters.properties.attributes.properties + hostTool.definition.function.parameters.properties.attributes.properties .submode, 'host tool schema carries its input card fields', ); @@ -405,7 +408,7 @@ boxel: 'search doc keeps the tools as authored', ); assert.false( - 'tool' in searchDocTools[0], + 'definition' in searchDocTools[0], 'generated schemas stay out of the search doc', ); assert.false( @@ -452,11 +455,11 @@ boxel: 'failed tool entry stays as authored', ); assert.strictEqual( - tools[0].tool, + tools[0].definition, undefined, 'failed tool entry has no schema', ); - assert.ok(tools[1].tool, 'the remaining tool still enriches'); + assert.ok(tools[1].definition, 'the remaining tool still enriches'); assert.strictEqual( tools[1].functionName, buildToolFunctionNameFromResolvedRef({ diff --git a/packages/host/tests/integration/realm-indexing-test.gts b/packages/host/tests/integration/realm-indexing-test.gts index 57342a5245f..5a31235af63 100644 --- a/packages/host/tests/integration/realm-indexing-test.gts +++ b/packages/host/tests/integration/realm-indexing-test.gts @@ -1260,7 +1260,7 @@ module(`Integration | realm indexing`, function (hooks) { functionName: 'switch-submode_dd88', requiresApproval: false, cardTitle: 'Switch Submode', - tool: null, + definition: null, }, ], cardDescription: null, @@ -1302,7 +1302,7 @@ module(`Integration | realm indexing`, function (hooks) { functionName: 'switch-submode_dd88', requiresApproval: false, cardTitle: 'Switch Submode', - tool: null, + definition: null, }, ], cardTheme: null, diff --git a/packages/runtime-common/index.ts b/packages/runtime-common/index.ts index 77b9f20ecbb..09b466cb19d 100644 --- a/packages/runtime-common/index.ts +++ b/packages/runtime-common/index.ts @@ -103,6 +103,25 @@ export const FRONTMATTER_PARSE_ERROR_SYMBOL = Symbol.for( 'boxel:file-frontmatter-parse-error', ); +// Same symbol-channel pattern for a frontmatter value that must differ +// between the search doc and the file-meta resource: when a +// `FrontmatterField` subclass produces index-only enrichment (e.g. a skill's +// generated tool definitions, which are multi-KB and must never land in +// `search_doc`), `extractAttributes` routes the enriched copy under this key +// and the host file extractor stamps it onto the resource, leaving the +// search doc's `frontmatter` as authored. +export const FRONTMATTER_FILE_META_VALUE_SYMBOL = Symbol.for( + 'boxel:file-frontmatter-file-meta-value', +); + +// Same symbol-channel pattern for the `ToolSchemaError[]` a skill's +// frontmatter enrichment produced; the host file extractor lifts it into the +// extract result so the indexer can persist it onto +// `diagnostics.toolSchemaErrors`. +export const TOOL_SCHEMA_ERRORS_SYMBOL = Symbol.for( + 'boxel:file-tool-schema-errors', +); + // One tool a skill's frontmatter declared whose schema generation failed // during file extraction — the module wouldn't load, the export was missing, // or the tool's input-schema generation threw. The skill still indexes From 2efd971c464c2b824288d51cac1092c2211eb441 Mon Sep 17 00:00:00 2001 From: Luke Melia Date: Mon, 13 Jul 2026 15:10:27 -0400 Subject: [PATCH 2/2] Generalize the frontmatter diagnostics channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback: naming toolSchemaErrors in FromFrontmatterResult leaked tool knowledge out of SkillFrontmatterField. The contract is now a kind-agnostic Partial bag — a FrontmatterField subclass contributes whatever diagnostics keys it owns, and the plumbing (markdown-file-def symbol channel, host extractor, extract response, file-indexer merge) carries it opaquely as frontmatterDiagnostics. Only SkillFrontmatterField (producer) and the /_indexing-errors surfacing (consumer) name the toolSchemaErrors key. Co-Authored-By: Claude Fable 5 --- packages/base/frontmatter-field.gts | 11 +++++--- packages/base/markdown-file-def.gts | 8 +++--- packages/base/skill-frontmatter-field.gts | 4 ++- .../utils/file-def-attributes-extractor.ts | 28 +++++++++---------- .../prerender-file-extract-test.gts | 13 +++++---- .../index-runner/file-indexer.ts | 20 ++++++------- packages/runtime-common/index.ts | 25 +++++++++-------- 7 files changed, 58 insertions(+), 51 deletions(-) diff --git a/packages/base/frontmatter-field.gts b/packages/base/frontmatter-field.gts index 7caca8f1133..c538eb537d0 100644 --- a/packages/base/frontmatter-field.gts +++ b/packages/base/frontmatter-field.gts @@ -1,4 +1,4 @@ -import type { ToolContext, ToolSchemaError } from '@cardstack/runtime-common'; +import type { Diagnostics, ToolContext } from '@cardstack/runtime-common'; import { Component, @@ -32,9 +32,12 @@ export interface FromFrontmatterResult { // definitions) destined for the file-meta resource. Kept separate because // multi-KB generated content must never land in `search_doc`. fileMetaAttributes?: Record; - // Skill frontmatter tools whose schema generation failed. The extract - // still succeeds; these surface via `diagnostics.toolSchemaErrors`. - toolSchemaErrors?: ToolSchemaError[]; + // Diagnostics findings this frontmatter contributes to the indexed row, + // merged onto the row's `diagnostics` by the file indexer and surfaced via + // `/_indexing-errors`. The extract still succeeds — findings never fail the + // row. Which keys a subclass populates is its own knowledge (the base + // contract is just "a bag of Diagnostics"). + diagnostics?: Partial; } // The parsed YAML frontmatter of a markdown file, captured as JSON. The base diff --git a/packages/base/markdown-file-def.gts b/packages/base/markdown-file-def.gts index 330243d7182..859832aacc6 100644 --- a/packages/base/markdown-file-def.gts +++ b/packages/base/markdown-file-def.gts @@ -2,10 +2,10 @@ import { byteStreamToUint8Array, extractCardReferenceUrls, extractFileReferenceUrls, + FRONTMATTER_DIAGNOSTICS_SYMBOL, FRONTMATTER_FILE_META_VALUE_SYMBOL, FRONTMATTER_PARSE_ERROR_SYMBOL, identifyCard, - TOOL_SCHEMA_ERRORS_SYMBOL, type FrontmatterParseError, type ToolContext, } from '@cardstack/runtime-common'; @@ -650,7 +650,7 @@ export class MarkdownDef extends FileDef { // and produces any index-time enrichment of it (e.g. a skill's generated // tool definitions). MarkdownDef stays ignorant of any kind's schema. A // recognized kind is recorded so the field rehydrates as that subclass on - // read; the enriched copy and any tool schema errors ride out-of-band on + // read; the enriched copy and any diagnostics findings ride out-of-band on // the same symbol channels the parse error uses, so neither leaks into the // flat `search_doc`. if (Object.keys(frontmatterData).length > 0) { @@ -665,8 +665,8 @@ export class MarkdownDef extends FileDef { bag[FRONTMATTER_FILE_META_VALUE_SYMBOL] = frontmatterResult.fileMetaAttributes; } - if (frontmatterResult.toolSchemaErrors?.length) { - bag[TOOL_SCHEMA_ERRORS_SYMBOL] = frontmatterResult.toolSchemaErrors; + if (frontmatterResult.diagnostics) { + bag[FRONTMATTER_DIAGNOSTICS_SYMBOL] = frontmatterResult.diagnostics; } if (isKnownFrontmatterKind(kind)) { let adoptsFrom = identifyCard(frontmatterFieldClass); diff --git a/packages/base/skill-frontmatter-field.gts b/packages/base/skill-frontmatter-field.gts index ed8634f0fc9..4bbc93cc293 100644 --- a/packages/base/skill-frontmatter-field.gts +++ b/packages/base/skill-frontmatter-field.gts @@ -108,7 +108,9 @@ export class SkillFrontmatterField extends FrontmatterField { return { attributes, fileMetaAttributes: { ...attributes, tools }, - ...(toolSchemaErrors.length ? { toolSchemaErrors } : {}), + // Only this subclass knows the diagnostics key its findings live + // under; the plumbing back to the indexed row is kind-agnostic. + ...(toolSchemaErrors.length ? { diagnostics: { toolSchemaErrors } } : {}), }; } diff --git a/packages/host/app/utils/file-def-attributes-extractor.ts b/packages/host/app/utils/file-def-attributes-extractor.ts index 2b22c2cd537..18d9e33dc0f 100644 --- a/packages/host/app/utils/file-def-attributes-extractor.ts +++ b/packages/host/app/utils/file-def-attributes-extractor.ts @@ -3,14 +3,15 @@ import { isEqual } from 'lodash-es'; import { baseRef, CardError, + FRONTMATTER_DIAGNOSTICS_SYMBOL, FRONTMATTER_FILE_META_VALUE_SYMBOL, FRONTMATTER_PARSE_ERROR_SYMBOL, identifyCard, inferContentType, internalKeyFor, SupportedMimeType, - TOOL_SCHEMA_ERRORS_SYMBOL, type CodeRef, + type Diagnostics, type FileMetaResource, type FrontmatterParseError, type QueryFieldMeta, @@ -18,7 +19,6 @@ import { type RenderError, type ResolvedCodeRef, type ToolContext, - type ToolSchemaError, } from '@cardstack/runtime-common'; import { getFieldDefinitions } from '@cardstack/runtime-common/definitions'; @@ -57,10 +57,10 @@ export type FileDefExtractResult = { // indexes body-only); lifted out of the searchDoc here so the indexer can // persist it onto `diagnostics.frontmatterParseError`. See markdown-file-def. frontmatterParseError?: FrontmatterParseError; - // Skill frontmatter tools whose schema generation failed. The extract - // still succeeds (the skill indexes with the tools that did enrich); the - // indexer persists these onto `diagnostics.toolSchemaErrors`. - toolSchemaErrors?: ToolSchemaError[]; + // Diagnostics findings the frontmatter contributed (e.g. a skill's + // `toolSchemaErrors`). The extract still succeeds; the indexer merges the + // bag onto the row's `diagnostics`. + frontmatterDiagnostics?: Partial; }; export class FileDefAttributesExtractor { @@ -253,13 +253,13 @@ export class FileDefAttributesExtractor { if (frontmatterParseError) { delete cleanedBag[FRONTMATTER_PARSE_ERROR_SYMBOL]; } - // And for a skill's tool schema failures, persisted onto - // `diagnostics.toolSchemaErrors`. - let toolSchemaErrors = cleanedBag[TOOL_SCHEMA_ERRORS_SYMBOL] as - | ToolSchemaError[] - | undefined; - if (toolSchemaErrors) { - delete cleanedBag[TOOL_SCHEMA_ERRORS_SYMBOL]; + // And for any diagnostics findings the frontmatter contributed, + // merged onto the row's `diagnostics` by the indexer. + let frontmatterDiagnostics = cleanedBag[ + FRONTMATTER_DIAGNOSTICS_SYMBOL + ] as Partial | undefined; + if (frontmatterDiagnostics) { + delete cleanedBag[FRONTMATTER_DIAGNOSTICS_SYMBOL]; } // A frontmatter value enriched for the index (e.g. a skill's // generated tool definitions) replaces the resource's `frontmatter` @@ -292,7 +292,7 @@ export class FileDefAttributesExtractor { ...(error ? { error } : {}), ...(mismatch ? { mismatch: true } : {}), ...(frontmatterParseError ? { frontmatterParseError } : {}), - ...(toolSchemaErrors ? { toolSchemaErrors } : {}), + ...(frontmatterDiagnostics ? { frontmatterDiagnostics } : {}), }; } } diff --git a/packages/host/tests/acceptance/prerender-file-extract-test.gts b/packages/host/tests/acceptance/prerender-file-extract-test.gts index d0f3cd09f66..bd28e2ace62 100644 --- a/packages/host/tests/acceptance/prerender-file-extract-test.gts +++ b/packages/host/tests/acceptance/prerender-file-extract-test.gts @@ -342,9 +342,9 @@ boxel: let result = await captureFileExtractResult('ready'); assert.strictEqual(result.status, 'ready'); assert.strictEqual( - result.toolSchemaErrors, + result.frontmatterDiagnostics, undefined, - 'no tool schema errors', + 'no frontmatter diagnostics', ); let tools = (result.resource?.attributes as Record) @@ -436,8 +436,9 @@ boxel: let result = await captureFileExtractResult('ready'); assert.strictEqual(result.status, 'ready', 'extract still succeeds'); - assert.strictEqual(result.toolSchemaErrors?.length, 1); - let [toolError] = result.toolSchemaErrors!; + let toolSchemaErrors = result.frontmatterDiagnostics?.toolSchemaErrors; + assert.strictEqual(toolSchemaErrors?.length, 1); + let [toolError] = toolSchemaErrors!; assert.strictEqual( toolError.module, fileURL('skills/broken-tool/missing-tool'), @@ -483,9 +484,9 @@ boxel: let result = await captureFileExtractResult('ready'); assert.strictEqual(result.status, 'ready'); assert.strictEqual( - result.toolSchemaErrors, + result.frontmatterDiagnostics, undefined, - 'no tool schema errors', + 'no frontmatter diagnostics', ); // A non-skill kind keeps only the raw frontmatter (no typed `tools` // field), and the kind gate skips enrichment before ever looking. diff --git a/packages/runtime-common/index-runner/file-indexer.ts b/packages/runtime-common/index-runner/file-indexer.ts index 98759ed7bbe..1b3a436eb2d 100644 --- a/packages/runtime-common/index-runner/file-indexer.ts +++ b/packages/runtime-common/index-runner/file-indexer.ts @@ -191,20 +191,20 @@ export async function performFileIndexing({ _title: name, }; - // A frontmatter parse failure or a skill tool's schema-generation failure - // doesn't fail the file (it still indexes, body-only / minus that tool's - // schema), so each rides on the row's diagnostics — mirroring brokenLinks — - // where `/_indexing-errors` surfaces it for the author. Merge them onto - // whatever render-side diagnostics the visit already produced. Computed - // before the dependency-error branch below so those findings persist on - // dependency-error rows too. - let { frontmatterParseError, toolSchemaErrors } = extractResult; + // A frontmatter parse failure — or any diagnostics bag the frontmatter + // contributed (e.g. a skill's tool schema failures) — doesn't fail the + // file (it still indexes), so each rides on the row's diagnostics — + // mirroring brokenLinks — where `/_indexing-errors` surfaces it for the + // author. Merge them onto whatever render-side diagnostics the visit + // already produced. Computed before the dependency-error branch below so + // those findings persist on dependency-error rows too. + let { frontmatterParseError, frontmatterDiagnostics } = extractResult; let fileDiagnostics: Diagnostics | undefined = - frontmatterParseError || toolSchemaErrors + frontmatterParseError || frontmatterDiagnostics ? { ...(diagnostics ?? {}), + ...(frontmatterDiagnostics ?? {}), ...(frontmatterParseError ? { frontmatterParseError } : {}), - ...(toolSchemaErrors ? { toolSchemaErrors } : {}), } : diagnostics; diff --git a/packages/runtime-common/index.ts b/packages/runtime-common/index.ts index 09b466cb19d..7d1660933e6 100644 --- a/packages/runtime-common/index.ts +++ b/packages/runtime-common/index.ts @@ -114,12 +114,13 @@ export const FRONTMATTER_FILE_META_VALUE_SYMBOL = Symbol.for( 'boxel:file-frontmatter-file-meta-value', ); -// Same symbol-channel pattern for the `ToolSchemaError[]` a skill's -// frontmatter enrichment produced; the host file extractor lifts it into the -// extract result so the indexer can persist it onto -// `diagnostics.toolSchemaErrors`. -export const TOOL_SCHEMA_ERRORS_SYMBOL = Symbol.for( - 'boxel:file-tool-schema-errors', +// Same symbol-channel pattern for the `Partial` a +// `FrontmatterField` subclass contributes to the indexed row (e.g. a skill's +// `toolSchemaErrors`); the host file extractor lifts it into the extract +// result so the indexer can merge it onto the row's `diagnostics`. Which +// keys the bag carries is the subclass's own knowledge. +export const FRONTMATTER_DIAGNOSTICS_SYMBOL = Symbol.for( + 'boxel:file-frontmatter-diagnostics', ); // One tool a skill's frontmatter declared whose schema generation failed @@ -499,12 +500,12 @@ export interface FileExtractResponse { // the file indexer merges this onto `diagnostics.frontmatterParseError` so // the failure surfaces via `/_indexing-errors` instead of vanishing. frontmatterParseError?: FrontmatterParseError; - // Set when one or more of a skill's frontmatter tools failed schema - // generation during the extract. The extract still succeeds (the skill - // indexes with the tools that did enrich); the file indexer merges this - // onto `diagnostics.toolSchemaErrors` so each failure surfaces via - // `/_indexing-errors`. - toolSchemaErrors?: ToolSchemaError[]; + // Diagnostics findings the file's frontmatter contributed during the + // extract (e.g. a skill's `toolSchemaErrors`). The extract still succeeds; + // the file indexer merges the bag onto the row's `diagnostics` so each + // finding surfaces via `/_indexing-errors`. Which keys the bag carries is + // the producing `FrontmatterField` subclass's own knowledge. + frontmatterDiagnostics?: Partial; } export interface FileRenderResponse {