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 54154f3e3b6..92d1b5b1e01 100644 --- a/packages/host/app/lib/skill-tools.ts +++ b/packages/host/app/lib/skill-tools.ts @@ -1,9 +1,18 @@ 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 @@ -95,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: [