diff --git a/packages/ai-bot/lib/read-realm-file-fulfillment.ts b/packages/ai-bot/lib/read-realm-file-fulfillment.ts index 9b0d8fe24c5..348a8ebc7bc 100644 --- a/packages/ai-bot/lib/read-realm-file-fulfillment.ts +++ b/packages/ai-bot/lib/read-realm-file-fulfillment.ts @@ -13,7 +13,9 @@ import { executeReadRealmFile, fileLabelFromUrl, type ReadRealmFileArgs, + type ReadRealmFileTool, } from './read-realm-file.ts'; +import type { DiscoveredToolDefinition } from '@cardstack/base/matrix-event'; import type { DelegatedUserRealmSessionManager } from './user-delegated-realm-server-session.ts'; let log = logger('ai-bot:read-realm-file'); @@ -119,9 +121,44 @@ export async function fulfillReadRealmFileCalls( } type FileRead = - | { url: string; attachment: Record } + | { + url: string; + attachment: Record; + // Definitions of the tools the read file's indexed frontmatter + // declares, when the file is a skill whose index row carries usable + // (schema-stamped) entries. + discoveredTools?: DiscoveredToolDefinition[]; + } | { url: string; error: string }; +// The read's tool entries that are usable as LLM tool definitions, tagged +// with the skill file they came from. Entries without a well-formed +// definition (e.g. a skill indexed before schema enrichment) are dropped +// here — the prompt can only offer a tool it has a definition for. +function discoveredToolDefinitions( + url: string, + tools: ReadRealmFileTool[] | undefined, +): DiscoveredToolDefinition[] { + return (tools ?? []) + .filter( + (tool) => + tool.definition?.type === 'function' && + typeof tool.definition.function?.name === 'string' && + tool.definition.function.name.length > 0, + ) + .map((tool) => ({ + sourceSkillUrl: url, + ...(tool.codeRef ? { codeRef: tool.codeRef } : {}), + ...(typeof tool.functionName === 'string' + ? { functionName: tool.functionName } + : {}), + ...(typeof tool.requiresApproval === 'boolean' + ? { requiresApproval: tool.requiresApproval } + : {}), + definition: tool.definition as DiscoveredToolDefinition['definition'], + })); +} + // Reads and uploads a single file of a call. An upload failure is folded into // the same shape as a read failure so the caller reports both alike. async function readAndUpload( @@ -144,6 +181,7 @@ async function readAndUpload( log.error(`readRealmFile: upload failed for ${url}: ${e?.message ?? e}`); return { url, error: `could not store ${url} for reading` }; } + let discoveredTools = discoveredToolDefinitions(url, result.tools); return { url, attachment: { @@ -153,6 +191,7 @@ async function readAndUpload( contentType: READ_FILE_CONTENT_TYPE, contentSize: Buffer.byteLength(result.content), }, + ...(discoveredTools.length ? { discoveredTools } : {}), }; } @@ -187,12 +226,19 @@ async function fulfillOne( let reads = await Promise.all( urls.map((url) => readAndUpload(url, deps, upload)), ); - let attachedFiles = reads - .filter( - (read): read is Extract => - 'attachment' in read, - ) - .map((read) => read.attachment); + let successfulReads = reads.filter( + (read): read is Extract => + 'attachment' in read, + ); + let attachedFiles = successfulReads.map((read) => read.attachment); + // Tool definitions the read skills contributed ride the result event next + // to the attachments, so prompt assembly can offer them on later turns + // from room events alone. Kept out of attachedFiles: those entries are + // SerializedFileDefs consumed by the attachment-download path (and the + // timeline), which must not change shape. + let discoveredTools = successfulReads.flatMap( + (read) => read.discoveredTools ?? [], + ); let failureReason = reads .filter( @@ -223,6 +269,7 @@ async function fulfillOne( data: { context: { agentId: deps.agentId }, attachedFiles, + ...(discoveredTools.length ? { discoveredTools } : {}), }, }); return { diff --git a/packages/ai-bot/lib/read-realm-file.ts b/packages/ai-bot/lib/read-realm-file.ts index fc46f80b9fb..a126ebe470b 100644 --- a/packages/ai-bot/lib/read-realm-file.ts +++ b/packages/ai-bot/lib/read-realm-file.ts @@ -1,5 +1,8 @@ import { logger, SupportedMimeType } from '@cardstack/runtime-common'; -import { ensureTrailingSlash } from '@cardstack/runtime-common/paths'; +import { + ensureTrailingSlash, + isMarkdownFile, +} from '@cardstack/runtime-common/paths'; import { DelegatedUserRealmSessionError } from '@cardstack/runtime-common/user-delegated-realm-server-session'; import type { Tool } from '@cardstack/base/matrix-event'; import type { @@ -31,7 +34,10 @@ export const readRealmFileTool: Tool = { 'reference files it lists. Reads all the given URLs at once, so when ' + 'you already know several files you need (a skill’s reference ' + 'list usually tells you), request them all in a single call rather ' + - 'than a few at a time — every extra round of reads delays your answer.', + 'than a few at a time — every extra round of reads delays your answer. ' + + "Reading a skill's markdown file also unlocks the tools that skill " + + 'declares: they become callable on your next turn, so read the skill ' + + 'file first when you intend to use its tools.', parameters: { type: 'object', properties: { @@ -63,33 +69,137 @@ export interface ReadRealmFileArgs { // without the model having to name it (and get it wrong). const REALM_URL_HEADER = 'x-boxel-realm-url'; +// One tool entry from a skill markdown file's indexed frontmatter, as +// stamped by the realm indexer: resolved absolute codeRef, functionName, +// requiresApproval, and — once the skill's realm has indexed with schema +// enrichment — the ready-to-use LLM tool `definition`. Passed through +// structurally (no validation beyond "is an object") so the fulfillment +// layer decides what a usable entry is. +export interface ReadRealmFileTool { + codeRef?: { module?: string; name?: string }; + functionName?: string; + requiresApproval?: boolean; + definition?: Tool; + [key: string]: unknown; +} + export type ReadRealmFileResult = - | { ok: true; url: string; content: string } + | { + ok: true; + url: string; + content: string; + // Present only when the file is a markdown file whose indexed + // frontmatter declares tools (i.e. a skill read via file-meta). + tools?: ReadRealmFileTool[]; + } | { ok: false; error: string }; -// Reads one of a readRealmFile call's files inside the bot process: GETs the -// file as raw source, discovering the owning realm from the response so a -// delegated, read-only token can be minted for `onBehalfOf` only when the -// realm actually gates the file. A public file (e.g. anything in the skills -// realm) is returned from the first unauthenticated fetch with no token at -// all. Never throws — returns a result the caller hands back to the model as -// part of the tool result, so a missing file or a permission failure becomes +interface ReadRealmFileOptions { + onBehalfOf: string; + delegatedUserRealmSessions: Pick< + DelegatedUserRealmSessionManager, + 'getToken' | 'invalidate' + >; + fetch?: typeof globalThis.fetch; +} + +// Reads one of a readRealmFile call's files inside the bot process. Never +// throws — returns a result the caller hands back to the model as part of +// the tool result, so a missing file or a permission failure becomes // information the model can act on rather than a crashed turn. +// +// Markdown files are fetched as their indexed file-meta document, which +// yields the frontmatter-stripped body as `content` (the prompt-inlining +// path downstream is unchanged) plus the skill's frontmatter tools +// structurally. When the index can't serve a usable document — no row yet +// for a just-written file, an error-state row, or a document without a +// `content` string — the read degrades to raw source and returns +// instructions-only: index lag must never fail a read. Every other file is +// fetched as raw source, exactly as before. export async function executeReadRealmFile( url: string, + options: ReadRealmFileOptions, +): Promise { + if (isMarkdownFile(url)) { + let fileMeta = await fetchRealmFile( + url, + SupportedMimeType.FileMeta, + options, + ); + if (fileMeta.ok) { + let parsed = parseFileMetaBody(fileMeta.body); + if (parsed) { + return { + ok: true, + url, + content: parsed.content, + ...(parsed.tools ? { tools: parsed.tools } : {}), + }; + } + log.info( + `readRealmFile: file-meta for ${url} had no usable content; falling back to raw source`, + ); + } else if (fileMeta.kind !== 'http') { + // Auth and network failures would fail the raw-source fetch the same + // way; only an HTTP failure (404 for an unindexed file, an + // error-state row's 5xx, …) is worth the fallback. + return { ok: false, error: fileMeta.error }; + } + } + let raw = await fetchRealmFile(url, SupportedMimeType.CardSource, options); + return raw.ok + ? { ok: true, url, content: raw.body } + : { ok: false, error: raw.error }; +} + +// The `attributes.content` and `attributes.frontmatter.tools` of a file-meta +// document, or undefined when the body isn't a document carrying a string +// `content` (malformed JSON, an error document, a row indexed without a +// body). Tools pass through as indexed. +function parseFileMetaBody( + body: string, +): { content: string; tools?: ReadRealmFileTool[] } | undefined { + let document: any; + try { + document = JSON.parse(body); + } catch { + return undefined; + } + let attributes = document?.data?.attributes; + let content = attributes?.content; + if (typeof content !== 'string') { + return undefined; + } + let rawTools = attributes?.frontmatter?.tools; + let tools = Array.isArray(rawTools) + ? rawTools.filter( + (tool): tool is ReadRealmFileTool => + typeof tool === 'object' && tool !== null && !Array.isArray(tool), + ) + : []; + return { content, ...(tools.length ? { tools } : {}) }; +} + +type RealmFileFetch = + | { ok: true; body: string } + // `kind` routes the markdown fallback: only 'http' failures degrade to a + // raw-source retry — 'auth' and 'network' would fail it identically. + | { ok: false; error: string; kind: 'http' | 'auth' | 'network' }; + +// GETs a realm file under the given Accept type, discovering the owning +// realm from the response so a delegated, read-only token can be minted for +// `onBehalfOf` only when the realm actually gates the file. A public file +// (e.g. anything in the skills realm) is returned from the first +// unauthenticated fetch with no token at all. +async function fetchRealmFile( + url: string, + accept: SupportedMimeType, { onBehalfOf, delegatedUserRealmSessions, fetch = globalThis.fetch, - }: { - onBehalfOf: string; - delegatedUserRealmSessions: Pick< - DelegatedUserRealmSessionManager, - 'getToken' | 'invalidate' - >; - fetch?: typeof globalThis.fetch; - }, -): Promise { + }: ReadRealmFileOptions, +): Promise { // `redirect: 'manual'` keeps a stray redirect from being silently followed // (it surfaces as a non-2xx instead). No Authorization on the first try: // public files (the skills realm) come back 200, and a gated file comes back @@ -98,7 +208,7 @@ export async function executeReadRealmFile( fetch(url, { redirect: 'manual', headers: { - Accept: SupportedMimeType.CardSource, + Accept: accept, ...(token ? { Authorization: `Bearer ${token}` } : {}), }, }); @@ -108,12 +218,12 @@ export async function executeReadRealmFile( response = await get(); } catch (e: any) { log.error(`readRealmFile: fetch failed for ${url}: ${e?.message ?? e}`); - return { ok: false, error: `could not fetch ${url}` }; + return { ok: false, error: `could not fetch ${url}`, kind: 'network' }; } // Public read (or an already-authorized cache/CDN hit): done, no token. if (response.ok) { - return { ok: true, url, content: await response.text() }; + return { ok: true, body: await response.text() }; } // Anything other than an auth challenge is a real failure (404, 302, …). @@ -121,6 +231,7 @@ export async function executeReadRealmFile( return { ok: false, error: `could not load ${url} (HTTP ${response.status})`, + kind: 'http', }; } @@ -130,6 +241,7 @@ export async function executeReadRealmFile( return { ok: false, error: `could not determine which realm ${url} belongs to`, + kind: 'auth', }; } let realm = ensureTrailingSlash(realmHeader); @@ -144,12 +256,13 @@ export async function executeReadRealmFile( return { ok: false, error: `could not load ${url}: it does not belong to the realm that claimed it (${realm})`, + kind: 'auth', }; } let authorized = async (): Promise<{ response?: Response; - error?: string; + failure?: { error: string; kind: 'auth' | 'network' }; }> => { let token: string; try { @@ -158,12 +271,17 @@ export async function executeReadRealmFile( if (e instanceof DelegatedUserRealmSessionError) { if (e.kind === 'disabled') { return { - error: - 'reading realm files is unavailable (delegation is not configured)', + failure: { + error: + 'reading realm files is unavailable (delegation is not configured)', + kind: 'auth', + }, }; } if (e.kind === 'forbidden') { - return { error: `no read access to ${realm}` }; + return { + failure: { error: `no read access to ${realm}`, kind: 'auth' }, + }; } } log.error( @@ -171,27 +289,34 @@ export async function executeReadRealmFile( e?.message ?? e }`, ); - return { error: `could not obtain realm access for ${realm}` }; + return { + failure: { + error: `could not obtain realm access for ${realm}`, + kind: 'auth', + }, + }; } try { return { response: await get(token) }; } catch (e: any) { log.error(`readRealmFile: fetch failed for ${url}: ${e?.message ?? e}`); - return { error: `could not fetch ${url}` }; + return { + failure: { error: `could not fetch ${url}`, kind: 'network' }, + }; } }; - let { response: authed, error } = await authorized(); - if (error) { - return { ok: false, error }; + let { response: authed, failure } = await authorized(); + if (failure) { + return { ok: false, ...failure }; } // A cached token whose access was revoked inside its staleness window gets a // 401/403. Drop it and try once with a freshly minted token before failing. if (authed && (authed.status === 401 || authed.status === 403)) { delegatedUserRealmSessions.invalidate({ onBehalfOf, realm }); - ({ response: authed, error } = await authorized()); - if (error) { - return { ok: false, error }; + ({ response: authed, failure } = await authorized()); + if (failure) { + return { ok: false, ...failure }; } } @@ -199,10 +324,14 @@ export async function executeReadRealmFile( return { ok: false, error: `could not load ${url} (HTTP ${authed?.status ?? 'unknown'})`, + kind: + authed && (authed.status === 401 || authed.status === 403) + ? 'auth' + : 'http', }; } - return { ok: true, url, content: await authed.text() }; + return { ok: true, body: await authed.text() }; } export interface ClassifiedToolCalls { diff --git a/packages/ai-bot/tests/prompt-construction-test.ts b/packages/ai-bot/tests/prompt-construction-test.ts index 9590786c9fb..3026c854b27 100644 --- a/packages/ai-bot/tests/prompt-construction-test.ts +++ b/packages/ai-bot/tests/prompt-construction-test.ts @@ -44,6 +44,7 @@ import { import { absolutizeSkillLinks, buildPromptForModel, + constructHistory, getPromptParts, getRelevantCards, getTools, @@ -7563,6 +7564,232 @@ module('markdown skill tools', (hooks) => { assert.strictEqual(tools.length, 1); assert.strictEqual(tools[0].function.name, 'switch-submode_dd88'); }); + + const DISCOVERED_SKILL_URL = 'https://realm/skills/trip-planner/SKILL.md'; + + // A definition as the fulfillment layer embeds it: the skill's indexed + // frontmatter entry tagged with the skill file it came from. + function discoveredDef( + name: string, + { + skillUrl = DISCOVERED_SKILL_URL, + description = 'Plans a trip', + }: { skillUrl?: string; description?: string } = {}, + ) { + return { + sourceSkillUrl: skillUrl, + codeRef: { module: 'https://realm/commands/plan-trip', name: 'default' }, + functionName: name, + requiresApproval: true, + definition: { + type: 'function', + function: { + name, + description, + parameters: { type: 'object', properties: {} }, + }, + }, + }; + } + + function readResultEvent( + eventId: string, + ts: number, + discoveredTools: unknown[], + ): DiscreteMatrixEvent { + return { + type: APP_BOXEL_TOOL_RESULT_EVENT_TYPE, + event_id: eventId, + origin_server_ts: ts, + sender: '@aibot:localhost', + room_id: 'room1', + content: { + msgtype: APP_BOXEL_TOOL_RESULT_WITH_OUTPUT_MSGTYPE, + commandRequestId: `req-${eventId}`, + 'm.relates_to': { + rel_type: APP_BOXEL_TOOL_RESULT_REL_TYPE, + key: 'applied', + event_id: '$bot-message', + }, + data: { attachedFiles: [], discoveredTools }, + }, + unsigned: { age: 100, transaction_id: 't' }, + status: EventStatus.SENT, + } as unknown as DiscreteMatrixEvent; + } + + test('getTools offers tools discovered by a readRealmFile result event', async () => { + const tools = await getTools( + [readResultEvent('read-1', 2000, [discoveredDef('plan-trip_ab12')])], + [], + '@aibot:localhost', + fakeMatrixClient, + ); + assert.strictEqual(tools.length, 1); + assert.strictEqual(tools[0].function.name, 'plan-trip_ab12'); + assert.strictEqual(tools[0].function.description, 'Plans a trip'); + }); + + test('the latest read of a skill replaces its earlier definitions wholesale', async () => { + const tools = await getTools( + [ + readResultEvent('read-1', 1000, [ + discoveredDef('plan-trip_ab12', { description: 'stale' }), + discoveredDef('book-hotel_cd34'), + ]), + readResultEvent('read-2', 2000, [ + discoveredDef('plan-trip_ab12', { description: 'fresh' }), + ]), + ], + [], + '@aibot:localhost', + fakeMatrixClient, + ); + assert.strictEqual( + tools.length, + 1, + 'a tool the skill no longer declares disappears with the newer read', + ); + assert.strictEqual(tools[0].function.description, 'fresh'); + }); + + test('discovered tools on a non-bot result event are ignored', async () => { + let forged = readResultEvent('read-1', 2000, [ + discoveredDef('plan-trip_ab12'), + ]) as any; + forged.sender = '@someone-else:localhost'; + const tools = await getTools( + [forged], + [], + '@aibot:localhost', + fakeMatrixClient, + ); + assert.strictEqual( + tools.length, + 0, + 'only the bot publishes readRealmFile results', + ); + }); + + test('an entry whose definition is not a function tool is skipped', async () => { + const tools = await getTools( + [ + readResultEvent('read-1', 2000, [ + { + ...discoveredDef('plan-trip_ab12'), + definition: { function: { name: 'plan-trip_ab12' } }, + }, + discoveredDef('book-hotel_cd34'), + ]), + ], + [], + '@aibot:localhost', + fakeMatrixClient, + ); + assert.strictEqual(tools.length, 1, 'the malformed entry is dropped'); + assert.strictEqual(tools[0].function.name, 'book-hotel_cd34'); + }); + + test('discoveries on a non-applied result are ignored', async () => { + let failed = readResultEvent('read-1', 2000, [ + discoveredDef('plan-trip_ab12'), + ]) as any; + failed.content['m.relates_to'].key = 'invalid'; + const tools = await getTools( + [failed], + [], + '@aibot:localhost', + fakeMatrixClient, + ); + assert.strictEqual( + tools.length, + 0, + 'only an applied read is evidence the skill was actually fetched', + ); + }); + + test('a skill disabled in room state contributes no discovered tools', async () => { + const skillsEvent = { + type: APP_BOXEL_ROOM_SKILLS_EVENT_TYPE, + event_id: 'skills-1', + origin_server_ts: 3000, + state_key: '', + content: { + enabledSkillCards: [], + disabledSkillCards: [ + { + sourceUrl: DISCOVERED_SKILL_URL, + url: 'mxc://mock-server/trip-planner', + name: 'SKILL.md', + contentType: 'text/plain', + }, + ], + }, + sender: '@user:localhost', + room_id: 'room1', + unsigned: { age: 1000 }, + status: EventStatus.SENT, + } as unknown as DiscreteMatrixEvent; + const tools = await getTools( + [ + readResultEvent('read-1', 2000, [discoveredDef('plan-trip_ab12')]), + skillsEvent, + ], + [], + '@aibot:localhost', + fakeMatrixClient, + ); + assert.strictEqual(tools.length, 0); + }); + + test('a skill both enabled and read yields one definition; the uploaded one wins', async () => { + const { eventList, enabledSkills } = skillsRoomFixture('toolDefinitions'); + eventList.push( + readResultEvent('read-1', 2000, [ + discoveredDef('switch-submode_dd88', { + skillUrl: 'https://realm/skills/boxel-environment/SKILL.md', + description: 'discovered copy that must lose to the upload', + }), + ]), + ); + const tools = await getTools( + eventList, + enabledSkills, + '@aibot:localhost', + fakeMatrixClient, + ); + assert.strictEqual( + tools.length, + 1, + 'one definition for the shared functionName', + ); + assert.strictEqual( + tools[0].function.description, + 'Switch between interact and code submodes', + 'the enabled skill uploaded definition wins the conflict', + ); + }); + + test('discovered definitions round-trip the event encode/decode path', async () => { + // sendMatrixEvent JSON-stringifies `data` on the way out; constructHistory + // parses it back. Feed getTools a history built from the wire shape. + let rawEvent = readResultEvent('read-1', 2000, [ + discoveredDef('plan-trip_ab12'), + ]) as any; + rawEvent.content = { + ...rawEvent.content, + data: JSON.stringify(rawEvent.content.data), + }; + const history = await constructHistory([rawEvent], fakeMatrixClient); + const tools = await getTools( + history, + [], + '@aibot:localhost', + fakeMatrixClient, + ); + assert.strictEqual(tools.length, 1); + assert.strictEqual(tools[0].function.name, 'plan-trip_ab12'); + }); }); module('absolutizeSkillLinks', () => { diff --git a/packages/ai-bot/tests/read-realm-file-fulfillment-test.ts b/packages/ai-bot/tests/read-realm-file-fulfillment-test.ts index dcbf20769bc..cf08c06fb7a 100644 --- a/packages/ai-bot/tests/read-realm-file-fulfillment-test.ts +++ b/packages/ai-bot/tests/read-realm-file-fulfillment-test.ts @@ -76,6 +76,98 @@ function dataOf(sentEvent: { content: any }) { } module('fulfillReadRealmFileCalls', () => { + const STAMPED_TOOL = { + codeRef: { + module: 'https://localhost:4201/user/jane/tools/plan-trip', + name: 'default', + }, + functionName: 'plan-trip_ab12', + requiresApproval: false, + definition: { + type: 'function', + function: { + name: 'plan-trip_ab12', + description: 'Plans a trip', + parameters: { type: 'object', properties: {} }, + }, + }, + }; + + // A skill entry the index has not (yet) enriched: no definition, so the + // fulfillment layer must not embed it. + const UNSTAMPED_TOOL = { + codeRef: { + module: 'https://localhost:4201/user/jane/tools/book-hotel', + name: 'default', + }, + }; + + function fileMetaFetch(tools: unknown[]) { + return (async () => + new Response( + JSON.stringify({ + data: { + id: FILE_URL, + type: 'file-meta', + attributes: { + sourceUrl: FILE_URL, + content: '# Trip Planner', + frontmatter: { tools }, + }, + }, + }), + { status: 200 }, + )) as unknown as typeof globalThis.fetch; + } + + test('a skill read embeds its usable tool definitions beside the attachments', async () => { + let { client, sent } = fakeClient(); + + let outcomes = await fulfillReadRealmFileCalls( + [readRealmFileCall('c1', { urls: [FILE_URL] })], + baseDeps(client, { + fetch: fileMetaFetch([STAMPED_TOOL, UNSTAMPED_TOOL]), + uploadText: async () => 'https://localhost/media/trip-planner', + }), + ); + + assert.deepEqual(outcomes, [{ commandRequestId: 'c1', ok: true }]); + let data = dataOf(sent[0]); + assert.strictEqual( + data.attachedFiles.length, + 1, + 'the attachment path is unchanged', + ); + assert.deepEqual( + data.discoveredTools, + [{ sourceSkillUrl: FILE_URL, ...STAMPED_TOOL }], + 'stamped definitions ride the result tagged with the skill URL; the ' + + 'unstamped entry is dropped', + ); + }); + + test('no discoveredTools key when the read carries no definitions', async () => { + let { client, sent } = fakeClient(); + let fetch = (async () => + new Response('# Plain notes', { + status: 200, + })) as unknown as typeof globalThis.fetch; + + await fulfillReadRealmFileCalls( + [readRealmFileCall('c1', { urls: [FILE_URL] })], + baseDeps(client, { + fetch, + uploadText: async () => 'https://localhost/media/notes', + }), + ); + + let data = dataOf(sent[0]); + assert.false( + 'discoveredTools' in data, + 'the result data stays exactly as before for tool-less reads', + ); + }); + test('a successful read attaches the file to an applied command-result event', async () => { let { client, sent } = fakeClient(); let fetch = (async () => diff --git a/packages/ai-bot/tests/read-realm-file-test.ts b/packages/ai-bot/tests/read-realm-file-test.ts index 71fc25b21a3..ccef3b49c6c 100644 --- a/packages/ai-bot/tests/read-realm-file-test.ts +++ b/packages/ai-bot/tests/read-realm-file-test.ts @@ -14,8 +14,13 @@ import { const ON_BEHALF_OF = '@user:localhost'; const REALM = 'https://localhost:4201/user/jane/'; +// A markdown file: read via its indexed file-meta document. const FILE_URL = 'https://localhost:4201/user/jane/skills/trip-planner/SKILL.md'; +// A non-markdown file: read as raw source, exactly as before file-meta reads +// existed — the transport/auth-flow tests use this one. +const RAW_FILE_URL = + 'https://localhost:4201/user/jane/skills/trip-planner/reference/cities.txt'; // A gated file's response: the realm server names the owning realm even on the // auth challenge, which is how executeReadRealmFile discovers the realm. @@ -26,6 +31,43 @@ function gated(status: 401 | 403 = 401): Response { }); } +// The tool entry stamped onto the skill's indexed file-meta (resolved +// codeRef, functionName, requiresApproval, ready-to-use LLM definition). +const STAMPED_TOOL = { + codeRef: { + module: 'https://localhost:4201/user/jane/tools/plan-trip', + name: 'default', + }, + functionName: 'plan-trip_ab12', + requiresApproval: true, + definition: { + type: 'function', + function: { + name: 'plan-trip_ab12', + description: 'Plans a trip', + parameters: { type: 'object', properties: {} }, + }, + }, +}; + +// A skill markdown file's file-meta document: frontmatter-stripped body in +// `attributes.content`, tools on `attributes.frontmatter.tools`. +function fileMetaResponse( + attributes: Record, + status = 200, +): Response { + return new Response( + JSON.stringify({ + data: { + id: FILE_URL, + type: 'file-meta', + attributes: { sourceUrl: FILE_URL, ...attributes }, + }, + }), + { status }, + ); +} + // A fake fetch that records each call and returns a scripted Response. function recordingFetch( handler: (url: string, init: RequestInit) => Response, @@ -89,6 +131,11 @@ module('readRealmFile tool definition', () => { properties['realm'], 'realm is discovered, not asked of the model', ); + assert.true( + readRealmFileTool.function.description.includes('next turn'), + 'description tells the model that reading a skill file unlocks its ' + + 'tools on the next turn — without this the mechanism goes unused', + ); }); }); @@ -99,7 +146,7 @@ module('executeReadRealmFile', () => { () => new Response('# Trip Planner\n\ninstructions', { status: 200 }), ); - let result = await executeReadRealmFile(FILE_URL, { + let result = await executeReadRealmFile(RAW_FILE_URL, { onBehalfOf: ON_BEHALF_OF, delegatedUserRealmSessions: sessions, fetch, @@ -107,7 +154,7 @@ module('executeReadRealmFile', () => { assert.strictEqual(sessions.calls.length, 0, 'no token minted'); assert.strictEqual(calls.length, 1, 'a single fetch'); - assert.strictEqual(calls[0].url, FILE_URL, 'fetches the given url'); + assert.strictEqual(calls[0].url, RAW_FILE_URL, 'fetches the given url'); let headers = calls[0].init.headers as Record; assert.notOk(headers['Authorization'], 'no Authorization on a public read'); assert.strictEqual(headers['Accept'], SupportedMimeType.CardSource); @@ -128,7 +175,7 @@ module('executeReadRealmFile', () => { : new Response('# Gated\n\ninstructions', { status: 200 }); }); - let result = await executeReadRealmFile(FILE_URL, { + let result = await executeReadRealmFile(RAW_FILE_URL, { onBehalfOf: ON_BEHALF_OF, delegatedUserRealmSessions: sessions, fetch, @@ -205,7 +252,7 @@ module('executeReadRealmFile', () => { let { fetch } = recordingFetch( () => new Response('not found', { status: 404 }), ); - let result = await executeReadRealmFile(FILE_URL, { + let result = await executeReadRealmFile(RAW_FILE_URL, { onBehalfOf: ON_BEHALF_OF, delegatedUserRealmSessions: sessions, fetch, @@ -266,7 +313,7 @@ module('executeReadRealmFile', () => { return n === 2 ? gated() : new Response('# Fresh', { status: 200 }); }); - let result = await executeReadRealmFile(FILE_URL, { + let result = await executeReadRealmFile(RAW_FILE_URL, { onBehalfOf: ON_BEHALF_OF, delegatedUserRealmSessions: sessions, fetch, @@ -283,6 +330,257 @@ module('executeReadRealmFile', () => { }); }); +module('executeReadRealmFile markdown file-meta', () => { + const BODY = '# Trip Planner\n\ninstructions'; + + test('reads a skill .md via file-meta: body + stamped tools', async () => { + let sessions = stubSessions({ token: 'unused' }); + let { fetch, calls } = recordingFetch(() => + fileMetaResponse({ + content: BODY, + frontmatter: { name: 'Trip Planner', tools: [STAMPED_TOOL] }, + }), + ); + + let result = await executeReadRealmFile(FILE_URL, { + onBehalfOf: ON_BEHALF_OF, + delegatedUserRealmSessions: sessions, + fetch, + }); + + assert.strictEqual(calls.length, 1, 'a single file-meta fetch'); + assert.strictEqual( + (calls[0].init.headers as Record)['Accept'], + SupportedMimeType.FileMeta, + 'markdown requests the indexed file-meta document', + ); + assert.true(result.ok, 'result ok'); + let ok = result as { ok: true; content: string; tools?: unknown[] }; + assert.strictEqual( + ok.content, + BODY, + 'content is the frontmatter-stripped body from the index', + ); + assert.deepEqual( + ok.tools, + [STAMPED_TOOL], + 'stamped tools pass through structurally', + ); + }); + + test('a plain .md (no frontmatter tools) returns body and no tools', async () => { + let sessions = stubSessions({ token: 'unused' }); + let { fetch } = recordingFetch(() => + fileMetaResponse({ content: '# A realm README' }), + ); + + let result = await executeReadRealmFile(FILE_URL, { + onBehalfOf: ON_BEHALF_OF, + delegatedUserRealmSessions: sessions, + fetch, + }); + + assert.true(result.ok, 'result ok'); + let ok = result as { ok: true; content: string; tools?: unknown[] }; + assert.strictEqual(ok.content, '# A realm README'); + assert.false('tools' in ok, 'no tools key when the file declares none'); + }); + + test('non-object tool entries are dropped', async () => { + let sessions = stubSessions({ token: 'unused' }); + let { fetch } = recordingFetch(() => + fileMetaResponse({ + content: BODY, + frontmatter: { + tools: [null, 'bogus', ['not', 'a', 'tool'], 7, STAMPED_TOOL], + }, + }), + ); + + let result = await executeReadRealmFile(FILE_URL, { + onBehalfOf: ON_BEHALF_OF, + delegatedUserRealmSessions: sessions, + fetch, + }); + + assert.true(result.ok); + assert.deepEqual( + (result as { ok: true; tools?: unknown[] }).tools, + [STAMPED_TOOL], + 'only object entries survive — arrays are objects to typeof, so the ' + + 'filter must exclude them explicitly', + ); + }); + + test('an unindexed .md (file-meta 404) falls back to raw source', async () => { + let sessions = stubSessions({ token: 'unused' }); + let raw = '---\nname: trip\n---\n# Raw body'; + let { fetch, calls } = recordingFetch((_url, init) => { + let accept = (init.headers as Record)['Accept']; + return accept === SupportedMimeType.FileMeta + ? new Response('not found', { status: 404 }) + : new Response(raw, { status: 200 }); + }); + + let result = await executeReadRealmFile(FILE_URL, { + onBehalfOf: ON_BEHALF_OF, + delegatedUserRealmSessions: sessions, + fetch, + }); + + assert.strictEqual(calls.length, 2, 'file-meta attempt, then raw source'); + assert.strictEqual( + (calls[1].init.headers as Record)['Accept'], + SupportedMimeType.CardSource, + 'fallback requests raw source', + ); + assert.true(result.ok, 'index lag never fails a read'); + let ok = result as { ok: true; content: string; tools?: unknown[] }; + assert.strictEqual(ok.content, raw, 'raw source returned as-is'); + assert.false('tools' in ok, 'instructions-only on the fallback path'); + }); + + test('a file-meta document without content falls back to raw source', async () => { + let sessions = stubSessions({ token: 'unused' }); + let { fetch, calls } = recordingFetch((_url, init) => { + let accept = (init.headers as Record)['Accept']; + // e.g. an error-state row served as a document with no content + return accept === SupportedMimeType.FileMeta + ? fileMetaResponse({ frontmatter: { name: 'broken' } }) + : new Response('# Raw', { status: 200 }); + }); + + let result = await executeReadRealmFile(FILE_URL, { + onBehalfOf: ON_BEHALF_OF, + delegatedUserRealmSessions: sessions, + fetch, + }); + + assert.strictEqual(calls.length, 2, 'file-meta attempt, then raw source'); + assert.true(result.ok); + assert.strictEqual( + (result as { ok: true; content: string }).content, + '# Raw', + ); + }); + + test('a non-JSON file-meta body falls back to raw source', async () => { + let sessions = stubSessions({ token: 'unused' }); + let { fetch, calls } = recordingFetch((_url, init) => { + let accept = (init.headers as Record)['Accept']; + return accept === SupportedMimeType.FileMeta + ? new Response('oops', { status: 200 }) + : new Response('# Raw', { status: 200 }); + }); + + let result = await executeReadRealmFile(FILE_URL, { + onBehalfOf: ON_BEHALF_OF, + delegatedUserRealmSessions: sessions, + fetch, + }); + + assert.strictEqual(calls.length, 2, 'file-meta attempt, then raw source'); + assert.true(result.ok); + assert.strictEqual( + (result as { ok: true; content: string }).content, + '# Raw', + ); + }); + + test('a gated .md mints a token and reads file-meta with it', async () => { + let sessions = stubSessions({ token: 'tok-md' }); + let n = 0; + let { fetch, calls } = recordingFetch(() => { + n += 1; + return n === 1 + ? gated() + : fileMetaResponse({ + content: BODY, + frontmatter: { tools: [STAMPED_TOOL] }, + }); + }); + + let result = await executeReadRealmFile(FILE_URL, { + onBehalfOf: ON_BEHALF_OF, + delegatedUserRealmSessions: sessions, + fetch, + }); + + assert.strictEqual(calls.length, 2, 'probe, then authed file-meta fetch'); + for (let call of calls) { + assert.strictEqual( + (call.init.headers as Record)['Accept'], + SupportedMimeType.FileMeta, + 'both fetches request file-meta', + ); + } + assert.strictEqual( + (calls[1].init.headers as Record)['Authorization'], + 'Bearer tok-md', + ); + assert.true(result.ok); + assert.deepEqual( + (result as { ok: true; tools?: unknown[] }).tools, + [STAMPED_TOOL], + 'gated skills still surface their tools', + ); + }); + + test('the raw-source fallback re-runs the auth flow for a gated .md', async () => { + let sessions = stubSessions({ token: 'tok-md' }); + let { fetch, calls } = recordingFetch((_url, init) => { + let headers = init.headers as Record; + if (!headers['Authorization']) { + return gated(); + } + // Authed: the index has no row yet, but raw source exists. + return headers['Accept'] === SupportedMimeType.FileMeta + ? new Response('not found', { status: 404 }) + : new Response('# Raw gated body', { status: 200 }); + }); + + let result = await executeReadRealmFile(FILE_URL, { + onBehalfOf: ON_BEHALF_OF, + delegatedUserRealmSessions: sessions, + fetch, + }); + + assert.strictEqual( + calls.length, + 4, + 'probe + authed file-meta, then probe + authed raw source', + ); + assert.true(result.ok, 'gated index lag still degrades to raw source'); + assert.strictEqual( + (result as { ok: true; content: string }).content, + '# Raw gated body', + ); + }); + + test('an access failure on the file-meta path does not retry as raw source', async () => { + let sessions = stubSessions({ + throws: new DelegatedUserRealmSessionError('forbidden', 'nope', 403), + }); + let { fetch, calls } = recordingFetch(() => gated(403)); + + let result = await executeReadRealmFile(FILE_URL, { + onBehalfOf: ON_BEHALF_OF, + delegatedUserRealmSessions: sessions, + fetch, + }); + + assert.false(result.ok, 'no access is a real failure'); + assert.strictEqual( + calls.length, + 1, + 'raw source would be forbidden identically — no fallback fetch', + ); + assert.true( + (result as { ok: false; error: string }).error.includes('no read access'), + ); + }); +}); + function assistantMessage(toolCalls: any[]): any { return { role: 'assistant', content: null, tool_calls: toolCalls }; } diff --git a/packages/base/matrix-event.gts b/packages/base/matrix-event.gts index e41e54b6b56..8b5f14a7124 100644 --- a/packages/base/matrix-event.gts +++ b/packages/base/matrix-event.gts @@ -341,6 +341,22 @@ export interface ToolDefinitionSchema { export type ToolResultStatus = 'applied' | 'failed' | 'invalid'; +// One tool definition the bot discovered by reading a skill markdown file +// (readRealmFile): the entry from the skill's indexed frontmatter, tagged +// with the skill file it came from. Embedded on the read's result event so +// prompt assembly can offer the tool on later turns from room events alone +// (event-sourced, replay-correct), and so execution can verify a call +// against the declaring skill's indexed frontmatter via `sourceSkillUrl`. +export interface DiscoveredToolDefinition { + // The skill markdown file the definition came from (the readRealmFile URL). + sourceSkillUrl: string; + codeRef?: { module?: string; name?: string }; + functionName?: string; + requiresApproval?: boolean; + // The ready-to-use LLM tool definition stamped on the skill's file-meta. + definition: Tool; +} + export interface ToolResultWithOutputContent { 'm.relates_to': { rel_type: ToolResultRelType; @@ -358,6 +374,11 @@ export interface ToolResultWithOutputContent { context?: BoxelContext; attachedFiles?: (SerializedFile & { content?: string; error?: string })[]; attachedCards?: (SerializedFile & { content?: string; error?: string })[]; + // Tool definitions discovered by the read this result reports (present + // only on readRealmFile results whose files carried usable definitions). + // Timeline rendering ignores this key; prompt assembly's getTools reads + // it. + discoveredTools?: DiscoveredToolDefinition[]; }; msgtype: ToolResultWithOutputMsgtype; } diff --git a/packages/host/app/lib/matrix-classes/message-builder.ts b/packages/host/app/lib/matrix-classes/message-builder.ts index 987bf14e644..d7068d58aa9 100644 --- a/packages/host/app/lib/matrix-classes/message-builder.ts +++ b/packages/host/app/lib/matrix-classes/message-builder.ts @@ -32,6 +32,7 @@ import { } from '@cardstack/runtime-common/matrix-constants'; import { + findDiscoveredToolSkillUrl, getSkillSourceTools, loadSkillSource, } from '@cardstack/host/lib/skill-tools'; @@ -379,6 +380,42 @@ export default class MessageBuilder { } } + // Tool from a read (not enabled) skill: the model may call a tool it + // discovered by reading a skill file via readRealmFile. The bot's result + // event names the declaring skill, but that annotation is strictly a + // lookup hint, never an authorization — the codeRef the host executes is + // re-derived here from the skill's realm-indexed frontmatter, loaded + // through the store with the user's own permissions. A forged annotation + // can't execute anything the named skill doesn't declare, and a skill the + // user can't read resolves nothing; either way the tool stays unresolved + // and surfaces through the existing unrecognized-command failure path. + // `requiresApproval` likewise comes from the verified declaration (absent + // means approval required), exactly as for enabled skills. + if (!skillTool && toolRequest.name) { + let sourceSkillUrl = findDiscoveredToolSkillUrl( + this.builderContext.events, + toolRequest.name, + ); + if (sourceSkillUrl) { + // The URL comes from a bot event, so a load blowing up on a + // malformed or unreadable id must degrade to "unresolved tool", not + // break message building for the whole timeline. + try { + let source = await loadSkillSource(this.store, sourceSkillUrl); + if (source) { + skillTool = getSkillSourceTools(source).find( + (candidate) => candidate.functionName === toolRequest.name, + ); + } + } catch (e) { + console.warn( + `could not load skill ${sourceSkillUrl} to resolve tool "${toolRequest.name}":`, + e, + ); + } + } + } + let actionVerb = 'Apply'; if (skillTool?.codeRef) { let CommandKlass = (await getClass( diff --git a/packages/host/app/lib/skill-tools.ts b/packages/host/app/lib/skill-tools.ts index d29acc10eee..92d1b5b1e01 100644 --- a/packages/host/app/lib/skill-tools.ts +++ b/packages/host/app/lib/skill-tools.ts @@ -1,9 +1,18 @@ -import { isCardInstance } from '@cardstack/runtime-common'; +import { isCardInstance, isMarkdownFile } from '@cardstack/runtime-common'; +import { + isToolResultEventType, + isToolResultWithOutputContent, +} from '@cardstack/runtime-common/matrix-constants'; import { isSkillCard } from './file-def-manager'; import type StoreService from '../services/store'; import type { MarkdownDef } from '@cardstack/base/markdown-file-def'; +import type { + DiscoveredToolDefinition, + MatrixEvent as DiscreteMatrixEvent, + ToolResultEvent, +} from '@cardstack/base/matrix-event'; import type * as SkillModule from '@cardstack/base/skill'; // A skill is either a `Skill` card (commands on `Skill.commands`) or a @@ -13,8 +22,6 @@ import type * as SkillModule from '@cardstack/base/skill'; // `ToolField` instances feed the command-definition upload flow identically. export type SkillSource = SkillModule.Skill | MarkdownDef; -const MARKDOWN_EXTENSION = /\.(md|markdown)$/i; - // A markdown skill carries its kind on the indexed `MarkdownDef.kind` field. export const SKILL_MARKDOWN_KIND = 'skill'; @@ -63,15 +70,10 @@ export function getSkillSourceTools( } // True for ids that name a markdown file (skills live in `*.md` / `*.markdown` -// files). Such ids load through the `file-meta` read type rather than as cards. +// files). Such ids load through the `file-meta` read type rather than as +// cards. Skill-flavored spelling of the shared markdown-file predicate. export function isMarkdownSkillId(id: string): boolean { - let pathname: string; - try { - pathname = new URL(id).pathname; - } catch { - pathname = id; - } - return MARKDOWN_EXTENSION.test(pathname); + return isMarkdownFile(id); } // Loads a skill by id, dispatching markdown files to the `file-meta` read type @@ -102,3 +104,45 @@ export function peekSkillSource( let card = store.peek(id); return isCardInstance(card) && isSkillCardInstance(card) ? card : undefined; } + +// The declaring-skill hint for a tool the model discovered by reading a +// skill file: the latest readRealmFile result event whose +// `data.discoveredTools` names this function returns that entry's +// `sourceSkillUrl`. The hint only says where to look — a caller must +// re-derive the codeRef from that skill's realm-indexed frontmatter (see +// `message-builder`) before executing anything, so a forged or stale +// annotation can point at a skill but never make it declare a tool it +// doesn't have. +export function findDiscoveredToolSkillUrl( + events: DiscreteMatrixEvent[], + functionName: string, +): string | undefined { + // Walk newest-first and return on the first match: the latest read of a + // skill is the freshest claim about where the tool lives. + for (let i = events.length - 1; i >= 0; i--) { + let event = events[i]; + if (!isToolResultEventType(event.type)) { + continue; + } + let content = (event as ToolResultEvent).content; + if (!isToolResultWithOutputContent(content)) { + continue; + } + let discovered = content.data?.discoveredTools; + if (!Array.isArray(discovered)) { + continue; + } + for (let def of discovered as DiscoveredToolDefinition[]) { + if (!def?.sourceSkillUrl) { + continue; + } + if ( + def.definition?.function?.name === functionName || + def.functionName === functionName + ) { + return def.sourceSkillUrl; + } + } + } + return undefined; +} diff --git a/packages/host/tests/acceptance/tools-test.gts b/packages/host/tests/acceptance/tools-test.gts index f133e683a13..77c647dbb65 100644 --- a/packages/host/tests/acceptance/tools-test.gts +++ b/packages/host/tests/acceptance/tools-test.gts @@ -476,6 +476,39 @@ module('Acceptance | Tools tests', function (hooks) { 'https://i.postimg.cc/VNvHH93M/pawel-czerwinski-Ly-ZLa-A5jti-Y-unsplash.jpg', iconURL: 'https://i.postimg.cc/L8yXRvws/icon.png', }), + // Markdown skills for the read-skill (pull model) execution tests. + // Neither is enabled in any room; their tools only become callable + // after a readRealmFile result event names them. + 'skills/read-tools/SKILL.md': [ + '---', + 'name: read-tools', + 'boxel:', + ' kind: skill', + ' tools:', + ' - codeRef:', + " module: '@cardstack/boxel-host/commands/switch-submode'", + ' name: default', + '---', + '# Read Tools', + '', + 'Use switch-submode to move between interact and code modes.', + '', + ].join('\n'), + 'skills/decoy/SKILL.md': [ + '---', + 'name: decoy', + 'boxel:', + ' kind: skill', + ' tools:', + ' - codeRef:', + ' module: /test/maybe-boom-command', + ' name: default', + '---', + '# Decoy', + '', + 'Declares only maybe-boom.', + '', + ].join('\n'), 'hi.txt': 'hi', }, }); @@ -908,6 +941,175 @@ module('Acceptance | Tools tests', function (hooks) { ); }); + // Simulates the result event the bot publishes after reading a skill via + // readRealmFile: the discovered tool definitions ride `data.discoveredTools` + // tagged with the declaring skill's URL. + function simulateSkillReadResult( + roomId: string, + sourceSkillUrl: string, + functionName: string, + ) { + simulateRemoteMessage( + roomId, + '@aibot:localhost', + { + msgtype: APP_BOXEL_TOOL_RESULT_WITH_OUTPUT_MSGTYPE, + commandRequestId: 'read-1', + 'm.relates_to': { + rel_type: APP_BOXEL_TOOL_RESULT_REL_TYPE, + key: 'applied', + event_id: 'earlier-bot-message', + }, + data: { + attachedFiles: [], + discoveredTools: [ + { + sourceSkillUrl, + codeRef: { + module: '@cardstack/boxel-host/commands/switch-submode', + name: 'default', + }, + functionName, + requiresApproval: true, + definition: { + type: 'function', + function: { + name: functionName, + description: 'Switch between interact and code submodes', + parameters: { type: 'object', properties: {} }, + }, + }, + }, + ], + }, + }, + { type: APP_BOXEL_TOOL_RESULT_EVENT_TYPE }, + ); + } + + test('a tool from a skill read via readRealmFile executes after approval', async function (assert) { + await visitOperatorMode({ + stacks: [[{ id: `${testRealmURL}index`, format: 'isolated' }]], + aiAssistantOpen: true, + }); + await waitFor('[data-room-settled]'); + let roomId = getRoomIds().pop()!; + // The skill is never enabled in the room; the model learned about its + // tool from an earlier readRealmFile result event. + simulateSkillReadResult( + roomId, + `${testRealmURL}skills/read-tools/SKILL.md`, + 'switch-submode_dd88', + ); + simulateRemoteMessage(roomId, '@aibot:localhost', { + body: '', + msgtype: APP_BOXEL_MESSAGE_MSGTYPE, + format: 'org.matrix.custom.html', + isStreamingFinished: true, + [APP_BOXEL_TOOL_REQUESTS_KEY]: [ + { + id: 'call-1', + name: 'switch-submode_dd88', + arguments: JSON.stringify({ + description: 'Switching to code submode', + attributes: { submode: 'code' }, + }), + }, + ], + }); + await waitFor('[data-test-message-idx="0"]'); + await settled(); + + // The skill's frontmatter has no requiresApproval on the tool, so + // approval is required: the request renders an Apply button and nothing + // auto-runs. + assert + .dom('[data-test-message-idx="0"] [data-test-tool-call-apply]') + .exists('the verified read-skill tool offers approval, not auto-run'); + + await click('[data-test-message-idx="0"] [data-test-tool-call-apply]'); + await waitFor('[data-test-submode-switcher=code]', { timeout: 5000 }); + assert.dom('[data-test-submode-switcher=code]').exists(); + + await waitUntil( + () => + getRoomEvents(roomId).find( + (m) => + m.content.msgtype === + APP_BOXEL_TOOL_RESULT_WITH_NO_OUTPUT_MSGTYPE && + m.content.commandRequestId === 'call-1', + ), + { + timeout: 5000, + timeoutMessage: 'timed out waiting for command result event', + }, + ); + let message = getRoomEvents(roomId).find( + (m) => + m.content.msgtype === APP_BOXEL_TOOL_RESULT_WITH_NO_OUTPUT_MSGTYPE && + m.content.commandRequestId === 'call-1', + )!; + assert.strictEqual(message.content['m.relates_to']?.key, 'applied'); + }); + + test('a discovered-tool annotation naming a skill that does not declare the tool is rejected', async function (assert) { + await visitOperatorMode({ + stacks: [[{ id: `${testRealmURL}index`, format: 'isolated' }]], + aiAssistantOpen: true, + }); + await waitFor('[data-room-settled]'); + let roomId = getRoomIds().pop()!; + // The annotation names the decoy skill, which declares only maybe-boom — + // re-deriving from the decoy's indexed frontmatter finds no + // switch-submode, so the codeRef asserted by the event never executes. + simulateSkillReadResult( + roomId, + `${testRealmURL}skills/decoy/SKILL.md`, + 'switch-submode_dd88', + ); + simulateRemoteMessage(roomId, '@aibot:localhost', { + body: '', + msgtype: APP_BOXEL_MESSAGE_MSGTYPE, + format: 'org.matrix.custom.html', + isStreamingFinished: true, + [APP_BOXEL_TOOL_REQUESTS_KEY]: [ + { + id: 'call-forged', + name: 'switch-submode_dd88', + arguments: JSON.stringify({ + description: 'Switching to code submode', + attributes: { submode: 'code' }, + }), + }, + ], + data: { + context: { + agentId: getService('matrix-service').agentId, + }, + }, + }); + await waitFor('[data-test-message-idx="0"]'); + + await waitFor( + '[data-test-message-idx="0"] [data-test-apply-state="invalid"]', + ); + assert + .dom('[data-test-boxel-alert="warning"]') + .containsText('No command for the name "switch-submode_dd88" was found'); + assert + .dom('[data-test-submode-switcher=code]') + .doesNotExist('the forged tool call never executed'); + + let message = getRoomEvents(roomId) + .filter( + (m) => + m.content.msgtype === APP_BOXEL_TOOL_RESULT_WITH_NO_OUTPUT_MSGTYPE, + ) + .pop()!; + assert.strictEqual(message.content['m.relates_to']?.key, 'invalid'); + assert.strictEqual(message.content.commandRequestId, 'call-forged'); + }); + test('rendering a command request without a description fallsback to attributes.description', async function (assert) { await visitOperatorMode({ stacks: [ diff --git a/packages/runtime-common/ai/prompt.ts b/packages/runtime-common/ai/prompt.ts index d1997dfaa74..c7b80d126a4 100644 --- a/packages/runtime-common/ai/prompt.ts +++ b/packages/runtime-common/ai/prompt.ts @@ -31,6 +31,7 @@ import type { CardMessageContent, CardMessageEvent, CodePatchResultEvent, + DiscoveredToolDefinition, ToolResultEvent, MatrixEvent as DiscreteMatrixEvent, EncodedToolRequest, @@ -75,6 +76,7 @@ import type { ToolChoice } from '../helpers/ai.ts'; import { logger } from '../log.ts'; import { parseFrontmatter } from '../frontmatter-parse.ts'; +import { isMarkdownFile } from '../paths.ts'; import { SKILL_INSTRUCTIONS_MESSAGE, SYSTEM_MESSAGE } from './constants.ts'; import { MAX_CORRECTNESS_FIX_ATTEMPTS } from './correctness-constants.ts'; import { humanReadable } from '../code-ref.ts'; @@ -496,7 +498,7 @@ export interface EnabledSkill { // A skill enabled as a markdown file (SKILL.md), rather than a Skill card. export function isMarkdownSkillFile(fileDef: SerializedFileDef): boolean { - return /\.(md|markdown)$/i.test(fileDef.sourceUrl ?? fileDef.name ?? ''); + return isMarkdownFile(fileDef.sourceUrl ?? fileDef.name ?? ''); } // A command a markdown skill contributes via its `boxel.tools` frontmatter @@ -907,6 +909,66 @@ export async function getTools( } } + // Tools discovered by reading a skill file (readRealmFile): each read's + // result event embeds the definitions in `data.discoveredTools`, so this + // source is event-sourced like every other prompt input. A skill read many + // times contributes only its latest read's definitions (the file may have + // changed between reads); a skill toggled off in room state contributes + // none. Merged into the map first, so an enabled skill's uploaded + // definition or a message-context tool with the same functionName wins on + // conflict. + let disabledSkillIds = new Set(await getDisabledSkillIds(eventList)); + let discoveredBySkill = new Map(); + for (let event of eventList) { + if (!isToolResultEvent(event)) { + continue; + } + // Only the bot publishes readRealmFile results; a discoveredTools block + // on anyone else's result event is not a legitimate discovery. + if (event.sender !== aiBotUserId) { + continue; + } + let content = event.content; + if (!isToolResultWithOutputContent(content)) { + continue; + } + // Only an applied read actually fetched the skill; discoveries claimed + // by a failed or invalid result are not evidence of anything. + if (content['m.relates_to']?.key !== 'applied') { + continue; + } + let discovered = content.data?.discoveredTools; + if (!discovered?.length) { + continue; + } + let bySkill = new Map(); + for (let def of discovered) { + if ( + !def?.sourceSkillUrl || + def.definition?.type !== 'function' || + !def.definition.function?.name + ) { + continue; + } + let defs = bySkill.get(def.sourceSkillUrl) ?? []; + defs.push(def); + bySkill.set(def.sourceSkillUrl, defs); + } + // eventList is chronological, so a later read of the same skill + // replaces the earlier one wholesale. + for (let [skillUrl, defs] of bySkill) { + discoveredBySkill.set(skillUrl, defs); + } + } + for (let [skillUrl, defs] of discoveredBySkill) { + if (disabledSkillIds.has(skillUrl)) { + continue; + } + for (let def of defs) { + toolMap.set(def.definition.function.name, def.definition); + } + } + let skillsConfigEvent = findLast( eventList, (event) => event.type === APP_BOXEL_ROOM_SKILLS_EVENT_TYPE, diff --git a/packages/runtime-common/paths.ts b/packages/runtime-common/paths.ts index 578fa3aaf07..606dc4c11cb 100644 --- a/packages/runtime-common/paths.ts +++ b/packages/runtime-common/paths.ts @@ -182,3 +182,21 @@ export function ensureTrailingSlash(url: string) { // http://example.com/my-realm/hello/world/ maps to local path "hello/world" // export type LocalPath = string; + +const MARKDOWN_FILE_EXTENSION = /\.(md|markdown)$/i; + +// True when the id/URL names a markdown file (`.md` / `.markdown`). Matches +// on the URL pathname so a querystring or fragment can't confuse the test; a +// value that isn't a parseable URL (e.g. a bare local path) is tested as-is. +// The one definition of "is a markdown file" — markdown ids dispatch to +// file-meta reads (host skill loading, ai-bot readRealmFile) rather than +// card/raw-source reads. +export function isMarkdownFile(id: string): boolean { + let pathname: string; + try { + pathname = new URL(id).pathname; + } catch { + pathname = id; + } + return MARKDOWN_FILE_EXTENSION.test(pathname); +}