From 471920dbc1059aa9a6d67f289722b79379e6c31d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8Dtalo=20Menezes?= Date: Fri, 24 Jul 2026 08:52:36 -0300 Subject: [PATCH 01/12] feat: add progressive skill disclosure with listing and on-demand skill tool --- shared/agent-core/skills.test.ts | 98 ++++++++++++++++++++++++++++ shared/agent-core/skills.ts | 107 +++++++++++++++++++++++++++++++ src/acp/built-in-adapter.test.ts | 17 +++-- src/acp/built-in-adapter.ts | 6 +- src/ai/fetch.ts | 19 +++--- src/ai/prompt.test.ts | 33 ++++++++++ src/ai/prompt.ts | 10 ++- src/skills/skill-tool.test.ts | 73 +++++++++++++++++++++ src/skills/skill-tool.ts | 44 +++++++++++++ 9 files changed, 390 insertions(+), 17 deletions(-) create mode 100644 shared/agent-core/skills.test.ts create mode 100644 shared/agent-core/skills.ts create mode 100644 src/skills/skill-tool.test.ts create mode 100644 src/skills/skill-tool.ts diff --git a/shared/agent-core/skills.test.ts b/shared/agent-core/skills.test.ts new file mode 100644 index 000000000..c59270b7a --- /dev/null +++ b/shared/agent-core/skills.test.ts @@ -0,0 +1,98 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import { describe, expect, it } from 'bun:test' +import { + buildSkillCatalog, + buildSkillListing, + buildWireSkillsMeta, + readWireSkills, + resolveSkill, + skillsCapabilityMeta, + supportsWireSkills, + thunderboltAcpMetaKey, + type SkillDefinition, +} from './skills.ts' + +const skills: SkillDefinition[] = [ + { + name: 'daily-brief', + description: 'Use for a daily rundown.\nIncludes weather and calendar.', + instruction: 'Gather current weather, news, email, and calendar details.', + }, + { + name: 'meeting-notes', + description: 'Use when summarizing meetings.', + instruction: 'Extract decisions and action items.', + }, +] + +describe('buildSkillListing', () => { + it('lists one skill name and description per line without instruction bodies', () => { + const listing = buildSkillListing(skills) + + expect(listing?.split('\n').filter((line) => line.startsWith('- '))).toEqual([ + '- daily-brief: Use for a daily rundown. Includes weather and calendar.', + '- meeting-notes: Use when summarizing meetings.', + ]) + expect(listing).not.toContain('Gather current weather') + expect(listing).not.toContain('Extract decisions') + }) + + it('omits the section when no skills are available', () => { + expect(buildSkillListing([])).toBeUndefined() + }) +}) + +describe('buildSkillCatalog', () => { + it('builds compact entries without tool guidance', () => { + expect(buildSkillCatalog(skills)).toBe( + '- daily-brief: Use for a daily rundown. Includes weather and calendar.\n' + + '- meeting-notes: Use when summarizing meetings.', + ) + expect(buildSkillCatalog([])).toBeUndefined() + }) +}) + +describe('resolveSkill', () => { + it('resolves bare and slash-prefixed names', () => { + expect(resolveSkill(skills, 'daily-brief')).toBe(skills[0]) + expect(resolveSkill(skills, ' /meeting-notes ')).toBe(skills[1]) + }) + + it('returns null for an unavailable name', () => { + expect(resolveSkill(skills, 'unknown')).toBeNull() + }) +}) + +describe('ACP wire skills metadata', () => { + it('advertises support through namespaced capability metadata', () => { + expect(skillsCapabilityMeta).toEqual({ + [thunderboltAcpMetaKey]: { skills: true }, + }) + expect(supportsWireSkills(skillsCapabilityMeta)).toBe(true) + expect(supportsWireSkills({ [thunderboltAcpMetaKey]: { skills: false } })).toBe(false) + expect(supportsWireSkills(undefined)).toBe(false) + }) + + it('round-trips full skill definitions through namespaced session metadata', () => { + const meta = buildWireSkillsMeta(skills) + + expect(meta).toEqual({ + [thunderboltAcpMetaKey]: { skills }, + }) + expect(readWireSkills(meta)).toEqual(skills) + }) + + it('ignores missing and malformed wire entries', () => { + expect(readWireSkills(undefined)).toEqual([]) + expect( + readWireSkills({ + [thunderboltAcpMetaKey]: { + skills: [skills[0], { name: 'missing-body', description: 'Broken' }], + }, + }), + ).toEqual([skills[0]]) + }) +}) diff --git a/shared/agent-core/skills.ts b/shared/agent-core/skills.ts new file mode 100644 index 000000000..f00816edf --- /dev/null +++ b/shared/agent-core/skills.ts @@ -0,0 +1,107 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +/** Skill data needed by progressive disclosure, independent of its data source. */ +export type SkillDefinition = { + readonly name: string + readonly description: string + readonly instruction: string +} + +/** ACP extension namespace owned by Thunderbolt. */ +export const thunderboltAcpMetaKey = 'thunderbird.net/thunderbolt' + +/** ACP agent-capability metadata advertising wire-delivered skills support. */ +export const skillsCapabilityMeta = { + [thunderboltAcpMetaKey]: { skills: true }, +} as const + +type AcpMeta = Readonly> | null | undefined + +/** Narrow unknown ACP metadata values to plain records. */ +const isRecord = (value: unknown): value is Readonly> => + typeof value === 'object' && value !== null && !Array.isArray(value) + +/** Validate one complete skill definition received through ACP metadata. */ +const isSkillDefinition = (value: unknown): value is SkillDefinition => + isRecord(value) && + typeof value.name === 'string' && + typeof value.description === 'string' && + typeof value.instruction === 'string' + +/** + * Detect Thunderbolt's custom ACP skills capability. + * + * @param meta - agent capability metadata from initialize response + * @returns whether agent accepts skills on session lifecycle requests + */ +export const supportsWireSkills = (meta: AcpMeta): boolean => { + const thunderbolt = meta?.[thunderboltAcpMetaKey] + return isRecord(thunderbolt) && thunderbolt.skills === true +} + +/** + * Build ACP session metadata containing full skill definitions. + * + * @param skills - enabled skills disclosed to agent + * @returns namespaced ACP metadata payload + */ +export const buildWireSkillsMeta = (skills: readonly SkillDefinition[]): Record => ({ + [thunderboltAcpMetaKey]: { skills }, +}) + +/** + * Read valid skill definitions from Thunderbolt ACP session metadata. + * + * @param meta - metadata received on session/new or session/resume + * @returns wire-delivered skills, excluding malformed entries + */ +export const readWireSkills = (meta: AcpMeta): SkillDefinition[] => { + const thunderbolt = meta?.[thunderboltAcpMetaKey] + if (!isRecord(thunderbolt) || !Array.isArray(thunderbolt.skills)) { + return [] + } + return thunderbolt.skills.filter(isSkillDefinition) +} + +/** + * Build compact skill catalog entries without tool-specific guidance. + * + * @param skills - enabled skills available to current agent + * @returns one name and description per line, or undefined when empty + */ +export const buildSkillCatalog = (skills: readonly SkillDefinition[]): string | undefined => { + if (skills.length === 0) { + return undefined + } + return skills.map(({ name, description }) => `- ${name}: ${description.replace(/\s+/g, ' ').trim()}`).join('\n') +} + +/** + * Build compact system-prompt guidance for available skills. + * + * @param skills - enabled skills available to the current agent + * @returns prompt section containing names and descriptions, or undefined when empty + */ +export const buildSkillListing = (skills: readonly SkillDefinition[]): string | undefined => { + const catalog = buildSkillCatalog(skills) + if (!catalog) { + return undefined + } + return `## Skills +Use the \`skill\` tool to load full instructions before using a relevant skill. A \`/name\` token means its instructions are already loaded. +${catalog}` +} + +/** + * Resolve a skill by bare name or slash-prefixed slug. + * + * @param skills - enabled skills available to the current agent + * @param requestedName - name supplied to the skill tool or slash resolver + * @returns matching skill, or null when unavailable + */ +export const resolveSkill = (skills: readonly SkillDefinition[], requestedName: string): SkillDefinition | null => { + const name = requestedName.trim().replace(/^\//, '') + return skills.find((skill) => skill.name === name) ?? null +} diff --git a/src/acp/built-in-adapter.test.ts b/src/acp/built-in-adapter.test.ts index 160b82909..ee001f464 100644 --- a/src/acp/built-in-adapter.test.ts +++ b/src/acp/built-in-adapter.test.ts @@ -126,6 +126,13 @@ describe('createBuiltInAdapter persistent harness', () => { supportsTools: true, sourceCollector: [], toolset, + skills: [ + { + name: 'review', + description: 'Use for reviews.', + instruction: 'Follow project style.', + }, + ], mcpToolsMetadata: undefined, stableSystemPrompt: 'stable prompt', volatileSystemPrompt: `timestamp ${index + 1}`, @@ -197,10 +204,10 @@ describe('createBuiltInAdapter persistent harness', () => { await response.text() } - await send(request([{ role: 'user', parts: [{ type: 'text', text: 'first' }] }])) + await send(request([{ role: 'user', parts: [{ type: 'text', text: '/review' }] }])) await send( request([ - { role: 'user', parts: [{ type: 'text', text: 'first' }] }, + { role: 'user', parts: [{ type: 'text', text: '/review' }] }, { role: 'assistant', parts: [{ type: 'text', text: 'reply' }] }, { role: 'user', parts: [{ type: 'text', text: 'second' }] }, ]), @@ -208,7 +215,7 @@ describe('createBuiltInAdapter persistent harness', () => { context.regenerationRevision = 1 await send( request([ - { role: 'user', parts: [{ type: 'text', text: 'first' }] }, + { role: 'user', parts: [{ type: 'text', text: '/review' }] }, { role: 'assistant', parts: [{ type: 'text', text: 'reply' }] }, { role: 'user', parts: [{ type: 'text', text: 'second' }] }, ]), @@ -222,10 +229,10 @@ describe('createBuiltInAdapter persistent harness', () => { ['read', 'third'], ]) expect(toPiCalls).toEqual(toolsets) - expect(promptCalls.map((call) => call.text)).toEqual(['first', 'second', 'second']) + expect(promptCalls.map((call) => call.text)).toEqual(['Follow project style.\n\n/review', 'second', 'second']) expect(buildCalls[0]?.history).toEqual([]) expect(buildCalls[1]?.history).toEqual([ - { role: 'user', text: 'first' }, + { role: 'user', text: '/review' }, { role: 'assistant', text: 'reply' }, ]) const firstSystemPrompt = buildCalls[0]?.systemPrompt as () => string diff --git a/src/acp/built-in-adapter.ts b/src/acp/built-in-adapter.ts index cd58482af..84c65d5be 100644 --- a/src/acp/built-in-adapter.ts +++ b/src/acp/built-in-adapter.ts @@ -54,6 +54,7 @@ import { } from '@/ai/fetch' import type { Agent, AgentAdapter, AgentAdapterContext } from '@/types/acp' import type { Model, ModelProfile, ThunderboltUIMessage } from '@/types' +import { extractLastUserText, resolveSkillTokenInstructions } from '@/skills/resolve-skill-system-messages' import type { PiModelDescriptor, SeedTurn } from '@shared/agent-core' import { APP_HARNESS_ENVIRONMENT_PROMPT } from '@shared/agent-core/environment-prompt' import type { AgentHarness, AgentTool, ThinkingLevel } from '@earendil-works/pi-agent-core' @@ -440,7 +441,10 @@ const fetchViaHarness = async ( return fallback() } - const { history, prompt } = await prepareBuiltInConversation(parseMessages(init), context.skillInstructions) + const messages = parseMessages(init) + const instructionBySlug = new Map(config.skills.map(({ name, instruction }) => [name, instruction])) + const skillInstructions = resolveSkillTokenInstructions(extractLastUserText(messages), instructionBySlug) + const { history, prompt } = await prepareBuiltInConversation(messages, skillInstructions) // Build the thread's harness on its first turn (seeding `history`); reuse it on // every later turn whose config signature is unchanged, and rebuild it when the diff --git a/src/ai/fetch.ts b/src/ai/fetch.ts index edc62e71e..b15eb813b 100644 --- a/src/ai/fetch.ts +++ b/src/ai/fetch.ts @@ -15,6 +15,7 @@ import { import { getAllSkills, getIntegrationStatus, getModel, getModelProfile, getSettings } from '@/dal' import { getMessage } from '@/dal/chat-messages' import { extractLastUserText, resolveSkillTokenInstructions } from '@/skills/resolve-skill-system-messages' +import { createSkillTool, selectEnabledSkillDefinitions } from '@/skills/skill-tool' import { collectAskEntriesFromCache, formatAskResponsesNote } from '@/widgets/ask/lib' import { getDb } from '@/db/database' import { getLocalSetting } from '@/stores/local-settings-store' @@ -61,6 +62,7 @@ import { type MCPClient } from '@ai-sdk/mcp' import type { NamedMCPClient } from '@/lib/mcp-provider' import { isClosedConnectionError } from '@/lib/mcp-errors' import { smoothStreamWordDelayMs } from '@/chats/chat-throttle' +import type { SkillDefinition } from '@shared/agent-core/skills' import { detectStreamChunk } from './smooth-chunking' import { createMessageMetadata } from './message-metadata' @@ -551,6 +553,7 @@ export type PreparedAiRequestConfig = { readonly supportsTools: boolean readonly sourceCollector: SourceMetadata[] readonly toolset: Record + readonly skills: readonly SkillDefinition[] readonly mcpToolsMetadata: UIMessageMetadata['mcpTools'] readonly stableSystemPrompt: string readonly volatileSystemPrompt: string @@ -596,6 +599,7 @@ export const prepareAiRequestConfig = async ({ throw new Error('Model not found') } const profile = await getModelProfile(db, modelId) + const skills = selectEnabledSkillDefinitions(await getAllSkills(db)) const supportsTools = model.toolUsage !== 0 const sourceCollector: SourceMetadata[] = [] const toolCallCache: ToolCallCache = new Map() @@ -603,6 +607,9 @@ export const prepareAiRequestConfig = async ({ ? await getAvailableTools(httpClient, sourceCollector, { settings, integrationStatus }) : [] const appToolset = createToolset(availableTools, toolCallCache) + if (supportsTools) { + appToolset.skill = createSkillTool(skills) + } const merged = supportsTools ? await mergeMcpTools(appToolset, mcpClients, reconnectClient) : { toolset: appToolset, summary: undefined, mcpTools: undefined } @@ -632,6 +639,7 @@ export const prepareAiRequestConfig = async ({ integrationStatus: integrationStatuses.length > 0 ? integrationStatuses.join(', ') : 'READY', modeSystemPrompt, mcpServersSummary: merged.summary, + skills: supportsTools ? skills : [], }) return { @@ -640,6 +648,7 @@ export const prepareAiRequestConfig = async ({ supportsTools, sourceCollector, toolset: merged.toolset, + skills, mcpToolsMetadata: merged.mcpTools, stableSystemPrompt: prompt.stablePrompt, volatileSystemPrompt: prompt.volatilePrompt, @@ -667,7 +676,7 @@ export const aiFetchStreamingResponse = async ({ // reach this function the user turn is already persisted. const db = getDb() - const { model, profile, supportsTools, sourceCollector, toolset, mcpToolsMetadata, systemPrompt } = + const { model, profile, supportsTools, sourceCollector, toolset, skills, mcpToolsMetadata, systemPrompt } = await prepareAiRequestConfig({ modelId, modeSystemPrompt, @@ -820,13 +829,7 @@ export const aiFetchStreamingResponse = async ({ // the context-overflow estimate so the budget and the actual prepend // stay in lockstep. const lastUserText = extractLastUserText(messages) - const allSkills = await getAllSkills(db) - const instructionBySlug = new Map() - for (const skill of allSkills) { - if (skill.enabled === 1 && skill.name && skill.instruction) { - instructionBySlug.set(skill.name, skill.instruction) - } - } + const instructionBySlug = new Map(skills.map(({ name, instruction }) => [name, instruction])) const skillSystemMessages = resolveSkillTokenInstructions(lastUserText, instructionBySlug) // Preserve the upstream status (and detail) when surfacing an API error to diff --git a/src/ai/prompt.test.ts b/src/ai/prompt.test.ts index 38accbc74..b64f6f6ac 100644 --- a/src/ai/prompt.test.ts +++ b/src/ai/prompt.test.ts @@ -4,6 +4,7 @@ import { describe, expect, test } from 'bun:test' import type { ModelProfile } from '@/types' +import { widgetRegistry } from '@/widgets' import { APP_HARNESS_ENVIRONMENT_PROMPT } from '@shared/agent-core/environment-prompt' import { createPrompt, createPromptParts, type PromptParams } from './prompt' @@ -145,6 +146,38 @@ describe('createPrompt', () => { expect(result).toContain('never state facts without verifying them first') }) + test('lists enabled skill names and descriptions without instruction bodies', () => { + const result = createPrompt({ + ...baseParams, + skills: [ + { + name: 'daily-brief', + description: 'Use for a daily rundown.', + instruction: 'Gather private full instructions here.', + }, + ], + }) + + expect(result).toContain('## Skills') + expect(result).toContain('- daily-brief: Use for a daily rundown.') + expect(result).not.toContain('Gather private full instructions here.') + }) + + test('does not inject widget instruction bodies into every prompt', () => { + const result = createPrompt(baseParams) + + for (const widget of widgetRegistry) { + expect(result).not.toContain(widget.module.instructions) + } + expect(result).not.toContain('# Widget Components') + }) + + test('keeps citation tags forbidden after removing widget instruction injection', () => { + const result = createPrompt(baseParams) + + expect(result).toContain('Do not emit tags') + }) + test('keeps the per-turn timestamp in the suffix (prefix-cache friendly)', () => { const result = createPrompt(baseParams) // The timestamp is the only per-turn-volatile field, so it comes after the diff --git a/src/ai/prompt.ts b/src/ai/prompt.ts index 305e078c6..d80586059 100644 --- a/src/ai/prompt.ts +++ b/src/ai/prompt.ts @@ -2,8 +2,8 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { widgetPrompts } from '@/widgets' import type { ModelProfile } from '@/types' +import { buildSkillListing, type SkillDefinition } from '@shared/agent-core/skills' /** Parameters to build the system prompt */ export type PromptParams = { @@ -26,6 +26,8 @@ export type PromptParams = { modeSystemPrompt?: string /** Summary of connected MCP servers (name + tool count) */ mcpServersSummary?: string + /** Enabled skills available through progressive disclosure */ + skills?: readonly SkillDefinition[] } export type PromptParts = { @@ -46,6 +48,7 @@ export const createPromptParts = ( integrationStatus, modeSystemPrompt, mcpServersSummary, + skills = [], }: PromptParams, currentDate: Date = new Date(), ): PromptParts => { @@ -82,6 +85,7 @@ export const createPromptParts = ( ] .filter(Boolean) .join('\n') + const skillListing = buildSkillListing(skills) // Output Format asks models to format math as `$…$` / `$$…$$` only (never // `\(…\)` / `\[…\]`). The chat renderer (src/components/chat/memoized-markdown.tsx) @@ -131,6 +135,7 @@ Think about what widget components to show the user, then work backwards to the Don't mention tool names unless asked. ${toolsOverride ? `\n${toolsOverride}` : ''} ${mcpServersSummary ? `\n## Connected MCP Servers\nYou have tools from these external services (tool names prefixed by server name):\n${mcpServersSummary}\nUse these when the user asks about these services.` : ''} +${skillListing ? `\n${skillListing}` : ''} ## Link Previews • Aggregate pages (listicles, "Top 10") are for DISCOVERY ONLY @@ -138,11 +143,10 @@ ${mcpServersSummary ? `\n## Connected MCP Servers\nYou have tools from these ext • For products: link to official manufacturer pages ${linkPreviewsOverride ? `\n${linkPreviewsOverride}` : ''} -${widgetPrompts} - # Output Format Cite sources with [N] INLINE at the end of the sentence, on the SAME LINE — never on a new line or separate paragraph. Place each [N] once after the period of the last sentence using that source. +Do not emit tags, 【1】 brackets, footnotes, or source lists at the end. Correct: "The metro area has 37 million residents. [1] [2]" Wrong: "The metro area has 37 million residents.\n[1]" (citation on new line) Wrong: "Tokyo has 14 million residents. [1] The metro area has 37 million. [1]" (repeated [1]) diff --git a/src/skills/skill-tool.test.ts b/src/skills/skill-tool.test.ts new file mode 100644 index 000000000..1644448dc --- /dev/null +++ b/src/skills/skill-tool.test.ts @@ -0,0 +1,73 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import { describe, expect, it } from 'bun:test' +import { toPiAgentTools } from '@shared/agent-core/mcp-tools' +import type { ToolCallOptions } from 'ai' +import { resolveSkillTokenInstructions } from './resolve-skill-system-messages' +import { createSkillTool, selectEnabledSkillDefinitions } from './skill-tool' + +const toolCallOptions: ToolCallOptions = { toolCallId: 'skill-call', messages: [] } +const storedSkills = [ + { + name: 'daily-brief', + description: 'Use for a daily rundown.', + instruction: 'Gather weather, news, email, and calendar details.', + enabled: 1, + }, + { + name: 'disabled-skill', + description: 'Unavailable.', + instruction: 'Never expose this instruction.', + enabled: 0, + }, +] + +describe('createSkillTool', () => { + it('returns full instructions for an enabled skill name or slug', async () => { + const skillTool = createSkillTool(selectEnabledSkillDefinitions(storedSkills)) + + expect(await skillTool.execute!({ name: 'daily-brief' }, toolCallOptions)).toBe( + 'Gather weather, news, email, and calendar details.', + ) + expect(await skillTool.execute!({ name: '/daily-brief' }, toolCallOptions)).toBe( + 'Gather weather, news, email, and calendar details.', + ) + }) + + it('rejects an unknown skill', async () => { + const skillTool = createSkillTool(selectEnabledSkillDefinitions(storedSkills)) + + await expect(skillTool.execute!({ name: 'unknown' }, toolCallOptions)).rejects.toThrow( + 'Skill "unknown" was not found or is disabled.', + ) + }) + + it('does not resolve a disabled skill', async () => { + const skillTool = createSkillTool(selectEnabledSkillDefinitions(storedSkills)) + + await expect(skillTool.execute!({ name: 'disabled-skill' }, toolCallOptions)).rejects.toThrow( + 'Skill "disabled-skill" was not found or is disabled.', + ) + }) + + it('returns the same full instruction through the Pi tool bridge', async () => { + const skillTool = createSkillTool(selectEnabledSkillDefinitions(storedSkills)) + const [piSkillTool] = await toPiAgentTools({ skill: skillTool }) + + expect(await piSkillTool.execute('skill-call', { name: 'daily-brief' })).toEqual({ + content: [{ type: 'text', text: 'Gather weather, news, email, and calendar details.' }], + details: 'Gather weather, news, email, and calendar details.', + }) + }) + + it('keeps /slug as a forced trigger without requiring a tool call', () => { + const skills = selectEnabledSkillDefinitions(storedSkills) + const instructionBySlug = new Map(skills.map(({ name, instruction }) => [name, instruction])) + + expect(resolveSkillTokenInstructions('Run /daily-brief now', instructionBySlug)).toEqual([ + 'Gather weather, news, email, and calendar details.', + ]) + }) +}) diff --git a/src/skills/skill-tool.ts b/src/skills/skill-tool.ts new file mode 100644 index 000000000..8285d7076 --- /dev/null +++ b/src/skills/skill-tool.ts @@ -0,0 +1,44 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import { resolveSkill, type SkillDefinition } from '@shared/agent-core/skills' +import { tool, type Tool } from 'ai' +import { z } from 'zod' + +type StoredSkillDefinition = SkillDefinition & { + readonly enabled: number +} + +/** + * Select enabled skills and discard persistence-only fields. + * + * @param skills - skill rows from any app data source + * @returns enabled skill definitions suitable for prompt and tool injection + */ +export const selectEnabledSkillDefinitions = (skills: readonly StoredSkillDefinition[]): SkillDefinition[] => + skills + .filter((skill) => skill.enabled === 1) + .map(({ name, description, instruction }) => ({ name, description, instruction })) + +/** + * Create AI SDK tool that loads one enabled skill's full instructions. + * + * @param skills - enabled skills available for this request + * @returns skill lookup tool shared by classic and Pi request paths + */ +export const createSkillTool = (skills: readonly SkillDefinition[]): Tool<{ name: string }, string> => + tool({ + description: + 'Load full instructions for an enabled skill. Use the exact skill name from the system prompt skill list.', + inputSchema: z.object({ + name: z.string().describe('Skill name or slash-prefixed slug from the system prompt skill list'), + }), + execute: async ({ name }) => { + const skill = resolveSkill(skills, name) + if (!skill) { + throw new Error(`Skill "${name}" was not found or is disabled.`) + } + return skill.instruction + }, + }) From e2ae12c04a2b6dd2cc215c9ec422a941c4193c04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8Dtalo=20Menezes?= Date: Fri, 24 Jul 2026 08:52:46 -0300 Subject: [PATCH 02/12] feat: convert spontaneous widget instructions into seeded default skills --- src/defaults/skills.test.ts | 67 ++++++++++++++- src/defaults/skills.ts | 85 ++++++++++++++++++- .../restamp-skill-default-hashes.test.ts | 10 ++- src/widgets/index.ts | 16 +--- 4 files changed, 157 insertions(+), 21 deletions(-) diff --git a/src/defaults/skills.test.ts b/src/defaults/skills.test.ts index 4a9534ea8..d886184c1 100644 --- a/src/defaults/skills.test.ts +++ b/src/defaults/skills.test.ts @@ -4,7 +4,21 @@ import { describe, expect, it, test } from 'bun:test' -import { defaultSkills, defaultSkillsVersion, hashSkill } from './skills' +import { instructions as askWidgetInstruction } from '@/widgets/ask/instructions' +import { instructions as connectIntegrationWidgetInstruction } from '@/widgets/connect-integration/instructions' +import { instructions as linkPreviewWidgetInstruction } from '@/widgets/link-preview/instructions' +import { instructions as mapWidgetInstruction } from '@/widgets/map/instructions' +import { instructions as weatherForecastWidgetInstruction } from '@/widgets/weather-forecast/instructions' +import { + defaultSkillAsk, + defaultSkillConnectIntegration, + defaultSkillLinkPreview, + defaultSkillMap, + defaultSkills, + defaultSkillsVersion, + defaultSkillWeatherForecast, + hashSkill, +} from './skills' /** * Snapshot pinning the shipped defaults to their declared version. When you @@ -23,8 +37,8 @@ const computeSnapshotHash = () => defaultSkills.map((skill, index) => `${index}:${skill.id}:${hashSkill(skill)}`).join('|') const expectedSnapshot = { - version: 2, - hash: '0:01996330-0000-7000-8000-000000000001:-eur3ct|1:01996330-0000-7000-8000-000000000002:lp36jd', + version: 3, + hash: '0:01996330-0000-7000-8000-000000000001:-eur3ct|1:01996330-0000-7000-8000-000000000002:lp36jd|2:01996330-0000-7000-8000-000000000003:-oawvjh|3:01996330-0000-7000-8000-000000000004:22br3x|4:01996330-0000-7000-8000-000000000005:-72ymfz|5:01996330-0000-7000-8000-000000000006:-31t7et|6:01996330-0000-7000-8000-000000000007:-rhvl8t', } describe('defaultSkills version snapshot', () => { @@ -37,6 +51,53 @@ describe('defaultSkills version snapshot', () => { }) describe('defaultSkills', () => { + it('ships spontaneous widget skills with load-bearing descriptions and canonical instruction bodies', () => { + const widgetSkills = [ + { + skill: defaultSkillWeatherForecast, + description: 'Use this skill when the user asks for a current or upcoming weather forecast.', + instruction: weatherForecastWidgetInstruction, + }, + { + skill: defaultSkillLinkPreview, + description: + 'Use this skill when the user wants web results, news, products, recommendations, or other fetched pages shown as rich link previews.', + instruction: linkPreviewWidgetInstruction, + }, + { + skill: defaultSkillConnectIntegration, + description: + 'Use this skill when the user asks to access email or calendar but required Google or Microsoft tools are unavailable.', + instruction: connectIntegrationWidgetInstruction, + }, + { + skill: defaultSkillAsk, + description: 'Use this skill when asking the user to choose from options or answer an interactive quiz prompt.', + instruction: askWidgetInstruction, + }, + { + skill: defaultSkillMap, + description: + 'Use this skill when the user asks to see locations, routes, regions, or other geographic results on an interactive map.', + instruction: mapWidgetInstruction, + }, + ] + + for (const { skill, description, instruction } of widgetSkills) { + expect(defaultSkills).toContain(skill) + expect(skill.description).toBe(description) + expect(skill.description).not.toContain('\n') + expect(skill.instruction).toBe(instruction) + } + }) + + it('does not seed flow-coupled citation or document-result contracts as user-invoked skills', () => { + const names = defaultSkills.map((skill) => skill.name) + + expect(names).not.toContain('citation') + expect(names).not.toContain('document-result') + }) + it('seeds every default with a pinnedOrder so new users start with pinned chips in chat', () => { // Regression guard — Chris flagged that seeded skills must be pinned by // default. Pinning is now manageable only from the chat composer; a new diff --git a/src/defaults/skills.ts b/src/defaults/skills.ts index 68f591361..aa0500585 100644 --- a/src/defaults/skills.ts +++ b/src/defaults/skills.ts @@ -4,6 +4,11 @@ import { hashValues } from '@/lib/utils' import type { Skill, SkillRow } from '@/types' +import { instructions as askWidgetInstruction } from '@/widgets/ask/instructions' +import { instructions as connectIntegrationWidgetInstruction } from '@/widgets/connect-integration/instructions' +import { instructions as linkPreviewWidgetInstruction } from '@/widgets/link-preview/instructions' +import { instructions as mapWidgetInstruction } from '@/widgets/map/instructions' +import { instructions as weatherForecastWidgetInstruction } from '@/widgets/weather-forecast/instructions' /** * Hash of user-editable fields. Includes `deletedAt` so soft-deletes are @@ -101,7 +106,83 @@ export const defaultSkillImportantEmails: Skill = { userId: null, } -export const defaultSkills: ReadonlyArray = [defaultSkillDailyBrief, defaultSkillImportantEmails] as const +export const defaultSkillWeatherForecast: Skill = { + id: '01996330-0000-7000-8000-000000000003', + name: 'weather-forecast', + label: 'Weather Forecast', + description: 'Use this skill when the user asks for a current or upcoming weather forecast.', + instruction: weatherForecastWidgetInstruction, + enabled: 1, + pinnedOrder: 2, + deletedAt: null, + defaultHash: null, + userId: null, +} + +export const defaultSkillLinkPreview: Skill = { + id: '01996330-0000-7000-8000-000000000004', + name: 'link-preview', + label: 'Link Preview', + description: + 'Use this skill when the user wants web results, news, products, recommendations, or other fetched pages shown as rich link previews.', + instruction: linkPreviewWidgetInstruction, + enabled: 1, + pinnedOrder: 3, + deletedAt: null, + defaultHash: null, + userId: null, +} + +export const defaultSkillConnectIntegration: Skill = { + id: '01996330-0000-7000-8000-000000000005', + name: 'connect-integration', + label: 'Connect Integration', + description: + 'Use this skill when the user asks to access email or calendar but required Google or Microsoft tools are unavailable.', + instruction: connectIntegrationWidgetInstruction, + enabled: 1, + pinnedOrder: 4, + deletedAt: null, + defaultHash: null, + userId: null, +} + +export const defaultSkillAsk: Skill = { + id: '01996330-0000-7000-8000-000000000006', + name: 'ask', + label: 'Ask', + description: 'Use this skill when asking the user to choose from options or answer an interactive quiz prompt.', + instruction: askWidgetInstruction, + enabled: 1, + pinnedOrder: 5, + deletedAt: null, + defaultHash: null, + userId: null, +} + +export const defaultSkillMap: Skill = { + id: '01996330-0000-7000-8000-000000000007', + name: 'map', + label: 'Map', + description: + 'Use this skill when the user asks to see locations, routes, regions, or other geographic results on an interactive map.', + instruction: mapWidgetInstruction, + enabled: 1, + pinnedOrder: 6, + deletedAt: null, + defaultHash: null, + userId: null, +} + +export const defaultSkills: ReadonlyArray = [ + defaultSkillDailyBrief, + defaultSkillImportantEmails, + defaultSkillWeatherForecast, + defaultSkillLinkPreview, + defaultSkillConnectIntegration, + defaultSkillAsk, + defaultSkillMap, +] as const /** * Monotonic version of the shipped skill defaults. Bump every time @@ -114,4 +195,4 @@ export const defaultSkills: ReadonlyArray = [defaultSkillDailyBrief, defa * The paired snapshot test in `skills.test.ts` fails on any change to this * file's defaults without a matching version bump. */ -export const defaultSkillsVersion = 2 +export const defaultSkillsVersion = 3 diff --git a/src/lib/data-migrations/restamp-skill-default-hashes.test.ts b/src/lib/data-migrations/restamp-skill-default-hashes.test.ts index 62d02e0b5..83c21c58d 100644 --- a/src/lib/data-migrations/restamp-skill-default-hashes.test.ts +++ b/src/lib/data-migrations/restamp-skill-default-hashes.test.ts @@ -3,7 +3,13 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import { resetTestDatabase, setupTestDatabase, teardownTestDatabase } from '@/dal/test-utils' -import { defaultSkillDailyBrief, defaultSkillImportantEmails, defaultSkills, hashSkill } from '@/defaults/skills' +import { + defaultSkillDailyBrief, + defaultSkillImportantEmails, + defaultSkills, + defaultSkillsVersion, + hashSkill, +} from '@/defaults/skills' import { getDb } from '@/db/database' import { settingsTable, skillsTable } from '@/db/tables' import { reconcileDefaults, versionMarkerKeys } from '@/lib/reconcile-defaults' @@ -117,6 +123,6 @@ describe('restampSkillDefaultHashes', () => { .from(settingsTable) .where(eq(settingsTable.key, versionMarkerKeys.skills)) .get() - expect(marker?.value).toBe('2') + expect(marker?.value).toBe(String(defaultSkillsVersion)) }) }) diff --git a/src/widgets/index.ts b/src/widgets/index.ts index abc023689..85adc2c28 100644 --- a/src/widgets/index.ts +++ b/src/widgets/index.ts @@ -3,11 +3,11 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ /** - * Central registry for all widget components and their AI instructions + * Central registry for all widget components * * To add a new widget: * 1. Create a new directory under src/widgets/ with: - * - instructions.ts (AI prompt instructions) + * - instructions.ts (model-facing instructions, seeded as a skill when the model emits the widget directly) * - schema.ts (Zod schema + parse function) * - [widget-name].tsx (React component) * - [widget-name].stories.tsx (Storybook stories - optional) @@ -70,18 +70,6 @@ export const widgetRegistry = [ }, ] as const -/** - * Aggregated instructions for all widgets to be included in the AI system prompt - */ -export const widgetPrompts = [ - '# Widget Components', - 'Use these XML-like tags in your response to show rich widgets:', - '', - ...widgetRegistry.flatMap((widget) => [widget.module.instructions, '']), -] - .join('\n') - .trim() - /** * Widget name type - auto-generated from registry */ From 898956aed6fe7173a0aa95d02ca2d12130fce5db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8Dtalo=20Menezes?= Date: Fri, 24 Jul 2026 08:52:56 -0300 Subject: [PATCH 03/12] feat: deliver skills to acp agents via capability meta with prompt fallback --- cli/src/acp/harness-agent.test.ts | 58 ++++++++++++++- cli/src/acp/harness-agent.ts | 16 ++-- cli/src/agent/harness.test.ts | 10 +++ cli/src/agent/harness.ts | 7 +- cli/src/agent/skill-tool.test.ts | 34 +++++++++ cli/src/agent/skill-tool.ts | 34 +++++++++ cli/src/agent/system-prompt.test.ts | 16 ++++ cli/src/agent/system-prompt.ts | 28 +++++-- cli/src/agent/types.ts | 3 + src/acp/acp-adapter.test.ts | 111 ++++++++++++++++++++++++++++ src/acp/acp-adapter.ts | 95 ++++++++++++++++++------ src/acp/connect.test.ts | 10 +-- src/acp/connect.ts | 7 ++ src/acp/connection-test.test.ts | 18 +++++ src/types/acp.ts | 3 + 15 files changed, 406 insertions(+), 44 deletions(-) create mode 100644 cli/src/agent/skill-tool.test.ts create mode 100644 cli/src/agent/skill-tool.ts diff --git a/cli/src/acp/harness-agent.test.ts b/cli/src/acp/harness-agent.test.ts index 7981ae034..fe1d902e7 100644 --- a/cli/src/acp/harness-agent.test.ts +++ b/cli/src/acp/harness-agent.test.ts @@ -27,7 +27,13 @@ import type { } from '@agentclientprotocol/sdk' import type { StopReason as PiStopReason, AssistantMessage } from '@earendil-works/pi-ai' import { InMemorySessionRepo } from '@earendil-works/pi-agent-core' -import type { AgentHarnessEvent, Session as PiSession, ToolCallEvent, ToolCallResult } from '@earendil-works/pi-agent-core' +import type { + AgentHarnessEvent, + Session as PiSession, + ToolCallEvent, + ToolCallResult, +} from '@earendil-works/pi-agent-core' +import { buildWireSkillsMeta, skillsCapabilityMeta, type SkillDefinition } from '../../../shared/agent-core/skills.ts' import { createHarnessAgent } from './harness-agent.ts' import type { BuildServeHarness } from './harness-agent.ts' import type { SessionStore } from './session-store.ts' @@ -170,9 +176,47 @@ describe('createHarnessAgent (ACP server)', () => { expect(init.protocolVersion).toBe(PROTOCOL_VERSION) expect(init.agentCapabilities?.loadSession).toBe(false) expect(init.agentCapabilities?.sessionCapabilities?.resume).toBeDefined() + expect(init.agentCapabilities?._meta).toEqual(skillsCapabilityMeta) expect(init.agentInfo?.name).toBe('thunderbolt') }) + test('session/new and session/resume pass wire-delivered skills into harness config', async () => { + const capturedSkills: Array = [] + const capturingBuilder: BuildServeHarness = async (harnessConfig) => { + capturedSkills.push(harnessConfig.skills ?? []) + return { + harness: { + subscribe: () => () => {}, + registerToolCallGate: () => {}, + prompt: async () => assistantMessage('stop'), + waitForIdle: async () => {}, + abort: async () => {}, + }, + dispose: async () => {}, + } + } + const skills: SkillDefinition[] = [ + { + name: 'daily-brief', + description: 'Build a concise daily rundown.', + instruction: 'Gather current weather and calendar details.', + }, + ] + const meta = buildWireSkillsMeta(skills) + const { client } = connectPair(capturingBuilder) + await client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} }) + + await client.newSession({ cwd: '/', mcpServers: [], _meta: meta }) + await client.resumeSession({ + sessionId: '11111111-1111-4111-8111-111111111111', + cwd: '/', + mcpServers: [], + _meta: meta, + }) + + expect(capturedSkills).toEqual([skills, skills]) + }) + test('session/resume opens the stored session by id and injects it into the harness (no replay)', async () => { const store = fakeStore() const injected: PiSession[] = [] @@ -336,7 +380,7 @@ describe('createHarnessAgent (ACP server)', () => { expect(decisions).toEqual([undefined, undefined, undefined]) }) - test('webfetch is auto-allowed without prompting while bash stays gated', async () => { + test('webfetch and skill are auto-allowed without prompting while bash stays gated', async () => { const decisions: Array = [] const webBuilder: BuildServeHarness = async () => { let gate: ((event: ToolCallEvent) => Promise) | null = null @@ -355,6 +399,14 @@ describe('createHarnessAgent (ACP server)', () => { input: { url: 'https://example.com' }, }), ) + decisions.push( + await gate?.({ + type: 'tool_call', + toolCallId: 'skill', + toolName: 'skill', + input: { name: 'daily-brief' }, + }), + ) decisions.push( await gate?.({ type: 'tool_call', toolCallId: 'shell', toolName: 'bash', input: { command: 'curl x' } }), ) @@ -372,7 +424,7 @@ describe('createHarnessAgent (ACP server)', () => { const { sessionId } = await client.newSession({ cwd: '/', mcpServers: [] }) await client.prompt({ sessionId, prompt: [{ type: 'text', text: 'web' }] }) - expect(decisions).toEqual([undefined, { block: true, reason: 'user rejected bash' }]) + expect(decisions).toEqual([undefined, undefined, { block: true, reason: 'user rejected bash' }]) expect(permissions.map((request) => request.toolCall.toolCallId)).toEqual(['shell']) }) diff --git a/cli/src/acp/harness-agent.ts b/cli/src/acp/harness-agent.ts index 8da78c357..58803d43a 100644 --- a/cli/src/acp/harness-agent.ts +++ b/cli/src/acp/harness-agent.ts @@ -50,6 +50,7 @@ import type { } from '@earendil-works/pi-agent-core' import type { AssistantMessage } from '@earendil-works/pi-ai' import { isReadOnlyAgentTool, resolveToolPermission } from '../../../shared/agent-tool-permissions.ts' +import { readWireSkills, skillsCapabilityMeta, type SkillDefinition } from '../../../shared/agent-core/skills.ts' import { cliVersion } from '../cli.ts' import { buildHarness } from '../agent/harness.ts' import type { HarnessConfig, ServeConfig } from '../agent/types.ts' @@ -176,7 +177,7 @@ const attachAcpPermissionGate = ( const sessionAllowed = new Set() harness.registerToolCallGate(async ({ toolCallId, toolName, input }) => { - if (toolName === 'webfetch') return undefined + if (toolName === 'webfetch' || toolName === 'skill') return undefined if (isReadOnlyAgentTool(toolName)) { const path = typeof input === 'object' && input !== null && 'path' in input && typeof input.path === 'string' @@ -263,12 +264,13 @@ export const createHarnessAgent = ( loadSession: false, sessionCapabilities: { resume: {} }, promptCapabilities: { image: false, audio: false, embeddedContext: false }, + _meta: skillsCapabilityMeta, }, authMethods: [], }) /** Per-session harness config rooted at server-owned launch directory. */ - const harnessConfigFor = (workspaceRoot: string): HarnessConfig => ({ + const harnessConfigFor = (workspaceRoot: string, skills: readonly SkillDefinition[]): HarnessConfig => ({ model: config.model, cwd: workspaceRoot, workspaceRoot, @@ -278,6 +280,7 @@ export const createHarnessAgent = ( baseUrl: config.baseUrl, apiKey: config.apiKey, announceModel: true, + skills, }) /** Build the harness on `session`, wire its run events + permission gate to the @@ -287,9 +290,10 @@ export const createHarnessAgent = ( sessionId: SessionId, workspaceRoot: string, session: PiSession, + skills: readonly SkillDefinition[], phase: string, ): Promise => { - const { harness, dispose } = await buildServeHarness(harnessConfigFor(workspaceRoot), session) + const { harness, dispose } = await buildServeHarness(harnessConfigFor(workspaceRoot, skills), session) // If the client vanished while the harness was being built, the cleanup // microtask already ran against a map without this session — dispose now so @@ -321,11 +325,11 @@ export const createHarnessAgent = ( } } - const newSession = async (_params: NewSessionRequest): Promise => { + const newSession = async (params: NewSessionRequest): Promise => { const sessionId = crypto.randomUUID() const workspaceRoot = await trustedWorkspace const session = await store.createSession(sessionId, workspaceRoot) - await activate(sessionId, workspaceRoot, session, 'session/new') + await activate(sessionId, workspaceRoot, session, readWireSkills(params._meta), 'session/new') return { sessionId } } @@ -343,7 +347,7 @@ export const createHarnessAgent = ( } const workspaceRoot = await trustedWorkspace const session = await store.openSession(params.sessionId, workspaceRoot) - await activate(params.sessionId, workspaceRoot, session, 'session/resume') + await activate(params.sessionId, workspaceRoot, session, readWireSkills(params._meta), 'session/resume') return {} } diff --git a/cli/src/agent/harness.test.ts b/cli/src/agent/harness.test.ts index 8850f62ef..53f8f0f7f 100644 --- a/cli/src/agent/harness.test.ts +++ b/cli/src/agent/harness.test.ts @@ -3,6 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import { describe, expect, test } from 'bun:test' +import type { SkillDefinition } from '../../../shared/agent-core/skills.ts' import { createHarnessTools } from './harness.ts' describe('createHarnessTools', () => { @@ -21,4 +22,13 @@ describe('createHarnessTools', () => { 'webfetch', ]) }) + + test('registers skill tool only when session provides skills', () => { + const skills: SkillDefinition[] = [ + { name: 'daily-brief', description: 'Build a daily rundown.', instruction: 'Gather current details.' }, + ] + + expect(createHarnessTools({ cwd: '/work' }).map((tool) => tool.name)).not.toContain('skill') + expect(createHarnessTools({ cwd: '/work', skills }).map((tool) => tool.name)).toContain('skill') + }) }) diff --git a/cli/src/agent/harness.ts b/cli/src/agent/harness.ts index 208b04b39..2402c8236 100644 --- a/cli/src/agent/harness.ts +++ b/cli/src/agent/harness.ts @@ -15,17 +15,19 @@ import type { AgentTool, Session } from '@earendil-works/pi-agent-core' import { NodeExecutionEnv } from '@earendil-works/pi-agent-core/node' import { createBashTool, createEditTool, createReadTool, createWriteTool } from '@earendil-works/pi-coding-agent' import { configureNativeWebSearch, resolveModel } from './model.ts' +import { createSkillTool } from './skill-tool.ts' import { buildSystemPrompt } from './system-prompt.ts' import type { HarnessBundle, HarnessConfig } from './types.ts' import { createWorkspaceTools } from './workspace-jail.ts' import { createWebFetchTool } from './webfetch.ts' /** Build complete toolset shared by local CLI and ACP-served harnesses. */ -export const createHarnessTools = (config: Pick): AgentTool[] => { +export const createHarnessTools = (config: Pick): AgentTool[] => { const codingTools = config.workspaceRoot ? createWorkspaceTools(config.workspaceRoot) : [createBashTool(config.cwd), createReadTool(config.cwd), createWriteTool(config.cwd), createEditTool(config.cwd)] - return [...codingTools, createWebFetchTool()] + const skillTools = config.skills?.length ? [createSkillTool(config.skills)] : [] + return [...codingTools, createWebFetchTool(), ...skillTools] } /** @@ -68,6 +70,7 @@ export const buildHarness = async (config: HarnessConfig, session?: Session): Pr cwd: config.cwd, modelId: config.announceModel ? config.model : undefined, bashEnabled: tools.some((tool) => tool.name === 'bash'), + skills: config.skills, }), }) harness.on('before_provider_payload', ({ model: requestModel, payload }) => ({ diff --git a/cli/src/agent/skill-tool.test.ts b/cli/src/agent/skill-tool.test.ts new file mode 100644 index 000000000..c4d1799e2 --- /dev/null +++ b/cli/src/agent/skill-tool.test.ts @@ -0,0 +1,34 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import { describe, expect, test } from 'bun:test' +import { buildWireSkillsMeta, readWireSkills, type SkillDefinition } from '../../../shared/agent-core/skills.ts' +import { createSkillTool } from './skill-tool.ts' +import { buildSystemPrompt } from './system-prompt.ts' + +const skills: SkillDefinition[] = [ + { + name: 'daily-brief', + description: 'Build a concise daily rundown.', + instruction: 'Gather current weather and calendar details.', + }, +] + +describe('wire-delivered CLI skills', () => { + test('lists compact metadata and resolves full instructions through tool', async () => { + const wireSkills = readWireSkills(buildWireSkillsMeta(skills)) + const prompt = buildSystemPrompt({ cwd: '/work', skills: wireSkills }) + const result = await createSkillTool(wireSkills).execute('call-1', { name: '/daily-brief' }) + + expect(prompt).toContain('- daily-brief: Build a concise daily rundown.') + expect(prompt).not.toContain('Gather current weather') + expect(result.content).toEqual([{ type: 'text', text: skills[0].instruction }]) + }) + + test('rejects unavailable skill names', async () => { + await expect(createSkillTool(skills).execute('call-1', { name: 'unknown' })).rejects.toThrow( + 'Skill "unknown" was not found or is disabled.', + ) + }) +}) diff --git a/cli/src/agent/skill-tool.ts b/cli/src/agent/skill-tool.ts new file mode 100644 index 000000000..142ce89b6 --- /dev/null +++ b/cli/src/agent/skill-tool.ts @@ -0,0 +1,34 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import { Type } from '@earendil-works/pi-ai' +import type { AgentTool } from '@earendil-works/pi-agent-core' +import { resolveSkill, type SkillDefinition } from '../../../shared/agent-core/skills.ts' + +const skillSchema = Type.Object({ + name: Type.String({ description: 'Skill name or slash-prefixed slug from the system prompt skill list' }), +}) + +/** + * Create Pi tool that resolves full instructions from wire-delivered skills. + * + * @param skills - skill definitions received during ACP session setup + * @returns read-only skill lookup tool + */ +export const createSkillTool = (skills: readonly SkillDefinition[]): AgentTool => ({ + name: 'skill', + label: 'skill', + description: 'Load full instructions for an available skill using its exact name.', + parameters: skillSchema, + execute: async (_toolCallId, { name }) => { + const skill = resolveSkill(skills, name) + if (!skill) { + throw new Error(`Skill "${name}" was not found or is disabled.`) + } + return { + content: [{ type: 'text', text: skill.instruction }], + details: skill.instruction, + } + }, +}) diff --git a/cli/src/agent/system-prompt.test.ts b/cli/src/agent/system-prompt.test.ts index 1b9732a8d..a7c6298cc 100644 --- a/cli/src/agent/system-prompt.test.ts +++ b/cli/src/agent/system-prompt.test.ts @@ -9,6 +9,7 @@ */ import { describe, expect, test } from 'bun:test' +import type { SkillDefinition } from '../../../shared/agent-core/skills.ts' import { buildSystemPrompt } from './system-prompt.ts' describe('buildSystemPrompt', () => { @@ -48,4 +49,19 @@ describe('buildSystemPrompt', () => { expect(ladder.slice(curl)).toMatch(/permission/i) expect(prompt).not.toMatch(/no web access/i) }) + + test('lists available skills without full instruction bodies', () => { + const skills: SkillDefinition[] = [ + { + name: 'daily-brief', + description: 'Build a concise daily rundown.', + instruction: 'Gather current weather and calendar details.', + }, + ] + const prompt = buildSystemPrompt({ cwd: '/work', skills }) + + expect(prompt).toContain('- skill') + expect(prompt).toContain('- daily-brief: Build a concise daily rundown.') + expect(prompt).not.toContain('Gather current weather') + }) }) diff --git a/cli/src/agent/system-prompt.ts b/cli/src/agent/system-prompt.ts index e3dfda859..567710fe1 100644 --- a/cli/src/agent/system-prompt.ts +++ b/cli/src/agent/system-prompt.ts @@ -2,20 +2,24 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +import { buildSkillListing, type SkillDefinition } from '../../../shared/agent-core/skills.ts' + type BuildSystemPromptParams = { cwd: string modelId?: string bashEnabled?: boolean + skills?: readonly SkillDefinition[] } /** Describe only tools registered on the harness. */ -const toolInstructions = (bashEnabled: boolean): string => { +const toolInstructions = (bashEnabled: boolean, skillEnabled: boolean): string => { + const skillInstruction = skillEnabled ? '\n- skill — load full instructions for an available skill' : '' if (!bashEnabled) { - return `You have four tools: + return `You have ${skillEnabled ? 'five' : 'four'} tools: - read — read a file - write — create or overwrite a file - edit — replace a span within a file -- webfetch — read a specific HTTP or HTTPS URL +- webfetch — read a specific HTTP or HTTPS URL${skillInstruction} Web access priority: 1. Use web_search when available to search for current information and discover URLs. @@ -25,12 +29,12 @@ Bash is unavailable in this workspace-confined session, so do not try curl. Use read before edit. Make the smallest change that fully solves the task.` } - return `You have five tools: + return `You have ${skillEnabled ? 'six' : 'five'} tools: - bash — run shell commands (grep, sed, find, git, language toolchains, tests, …) - read — read a file - write — create or overwrite a file - edit — replace a span within a file -- webfetch — read a specific HTTP or HTTPS URL +- webfetch — read a specific HTTP or HTTPS URL${skillInstruction} Web access priority: 1. Use web_search when available to search for current information and discover URLs. @@ -50,16 +54,25 @@ read before edit. Make the smallest change that fully solves the task.` * @param params.modelId - when set, names the underlying model so an exposed ACP * agent can self-identify; omitted for the standalone CLI * @param params.bashEnabled - whether the harness exposes shell execution + * @param params.skills - wire-delivered skills available through skill tool * @returns the system prompt string */ -export const buildSystemPrompt = ({ cwd, modelId, bashEnabled = true }: BuildSystemPromptParams): string => `\ +export const buildSystemPrompt = ({ + cwd, + modelId, + bashEnabled = true, + skills = [], +}: BuildSystemPromptParams): string => { + const skillListing = buildSkillListing(skills) + return `\ You are thunderbolt, a terminal coding agent${modelId ? `, powered by ${modelId}` : ''}. You operate directly in the user's \ working directory and complete software tasks end-to-end. Working directory: ${cwd} # Tools -${toolInstructions(bashEnabled)} +${toolInstructions(bashEnabled, skills.length > 0)} +${skillListing ? `\n${skillListing}\n` : ''} # How to work - When you have enough information to act, act. Don't re-derive facts already \ @@ -80,3 +93,4 @@ ${toolInstructions(bashEnabled)} When the task is complete, end with one or two sentences on what changed and any \ follow-up the user should know about. Lead with the outcome. Don't recap every \ file you touched — the user watched it happen.` +} diff --git a/cli/src/agent/types.ts b/cli/src/agent/types.ts index 2e5138d6e..a0f418c25 100644 --- a/cli/src/agent/types.ts +++ b/cli/src/agent/types.ts @@ -12,6 +12,7 @@ */ import type { AgentHarness } from '@earendil-works/pi-agent-core' +import type { SkillDefinition } from '../../../shared/agent-core/skills.ts' /** * A constructed harness paired with a teardown function. `buildHarness` @@ -128,6 +129,8 @@ export type HarnessConfig = { /** When true, the system prompt names the underlying model so an exposed ACP * agent can self-identify. The standalone CLI leaves this off. */ readonly announceModel?: boolean + /** Skill definitions delivered by ACP session metadata. */ + readonly skills?: readonly SkillDefinition[] } /** diff --git a/src/acp/acp-adapter.test.ts b/src/acp/acp-adapter.test.ts index 5173d9496..74852eaaa 100644 --- a/src/acp/acp-adapter.test.ts +++ b/src/acp/acp-adapter.test.ts @@ -39,6 +39,7 @@ import type { import { describe, expect, it } from 'bun:test' import { getClock } from '@/testing-library' import type { Agent, AgentAdapterContext } from '@/types/acp' +import { buildWireSkillsMeta, skillsCapabilityMeta, type SkillDefinition } from '@shared/agent-core/skills' import type { AcpTransport } from './types' import { connectAcpAdapter, type AcpAdapterContext } from './acp-adapter' import type { AcpCommand } from './translators/acp-to-ai-sdk' @@ -106,6 +107,7 @@ const buildFakeConnection = ( hangInitialize?: boolean loadSession?: boolean resume?: boolean + skills?: boolean /** Reject the resume/load call so a test exercises the tier fallthrough. */ rejectResume?: boolean rejectLoad?: boolean @@ -143,6 +145,7 @@ const buildFakeConnection = ( agentCapabilities: { loadSession: opts.loadSession ?? false, sessionCapabilities: opts.resume ? { resume: {} } : {}, + _meta: opts.skills ? skillsCapabilityMeta : undefined, }, }) } @@ -228,6 +231,19 @@ const conversationInit = (turns: { role: 'user' | 'assistant'; text: string }[]) const sentPromptText = (calls: { prompt: PromptRequest[] }, index = 0): string => (calls.prompt[index]?.prompt?.[0] as { type: string; text: string }).text +const enabledSkills: SkillDefinition[] = [ + { + name: 'daily-brief', + description: 'Build a concise daily rundown.', + instruction: 'Gather current weather and calendar details.', + }, + { + name: 'meeting-notes', + description: 'Summarize a meeting.', + instruction: 'Extract decisions and action items.', + }, +] + describe('connectAcpAdapter — handshake failure modes', () => { it('rejects after handshakeTimeoutMs when initialize never resolves and tears down the transport', async () => { const { transport, closeCalls } = buildFakeTransport() @@ -353,6 +369,101 @@ describe('connectAcpAdapter — handshake failure modes', () => { }) }) +describe('connectAcpAdapter — skills capability', () => { + it('detects capability metadata from initialize and sends full skills on session/new', async () => { + const { transport } = buildFakeTransport() + const { FakeConnection, calls } = buildFakeConnection({ skills: true }) + const adapter = await connectAcpAdapter(remoteAgent, baseCtx(), { + openTransport: async () => transport, + ClientSideConnection: FakeConnection as never, + getEnabledSkills: async () => enabledSkills, + }) + + expect(adapter.capabilities?.skills).toBe(true) + await adapter.ensureSession(threadCtx('thread-new')) + + expect(calls.newSession[0]?._meta).toEqual(buildWireSkillsMeta(enabledSkills)) + }) + + it('sends full skills on supported session/resume and session/load requests', async () => { + const resumeTransport = buildFakeTransport().transport + const resumeConnection = buildFakeConnection({ skills: true, resume: true }) + const resumeAdapter = await connectAcpAdapter(remoteAgent, baseCtx(), { + openTransport: async () => resumeTransport, + ClientSideConnection: resumeConnection.FakeConnection as never, + getEnabledSkills: async () => enabledSkills, + }) + await resumeAdapter.ensureSession(threadCtx('thread-resume', { acpSessionId: 'existing-resume' })) + + expect(resumeConnection.calls.resumeSession[0]?._meta).toEqual(buildWireSkillsMeta(enabledSkills)) + + const loadTransport = buildFakeTransport().transport + const loadConnection = buildFakeConnection({ skills: true, loadSession: true }) + const loadAdapter = await connectAcpAdapter(remoteAgent, baseCtx(), { + openTransport: async () => loadTransport, + ClientSideConnection: loadConnection.FakeConnection as never, + getEnabledSkills: async () => enabledSkills, + }) + await loadAdapter.ensureSession(threadCtx('thread-load', { acpSessionId: 'existing-load' })) + + expect(loadConnection.calls.loadSession[0]?._meta).toEqual(buildWireSkillsMeta(enabledSkills)) + }) + + it('keeps catalog bodies out of capability prompts while preserving forced invocation', async () => { + const { transport } = buildFakeTransport() + const { FakeConnection, calls, releasePrompts } = buildFakeConnection({ skills: true }) + const adapter = await connectAcpAdapter(remoteAgent, baseCtx(), { + openTransport: async () => transport, + ClientSideConnection: FakeConnection as never, + getEnabledSkills: async () => enabledSkills, + }) + + const response = await adapter.fetch( + promptInit('/daily-brief'), + threadCtx('thread-capability', { skillInstructions: [enabledSkills[0].instruction] }), + ) + await act(async () => { + releasePrompts() + await getClock().runAllAsync() + await readSse(response) + }) + + expect(sentPromptText(calls)).toBe('Gather current weather and calendar details.\n\n/daily-brief') + expect(sentPromptText(calls)).not.toContain('Extract decisions and action items.') + }) + + it('folds listing and full bodies into first prompt only for non-capability agents', async () => { + const { transport } = buildFakeTransport() + const { FakeConnection, calls, releasePrompts } = buildFakeConnection() + const adapter = await connectAcpAdapter(remoteAgent, baseCtx(), { + openTransport: async () => transport, + ClientSideConnection: FakeConnection as never, + getEnabledSkills: async () => enabledSkills, + }) + + const firstResponse = await adapter.fetch(promptInit('Plan my morning'), threadCtx('thread-fallback')) + await act(async () => { + releasePrompts() + await getClock().runAllAsync() + await readSse(firstResponse) + }) + + const firstPrompt = sentPromptText(calls) + expect(firstPrompt).toContain('- daily-brief: Build a concise daily rundown.') + expect(firstPrompt).toContain('Gather current weather and calendar details.') + expect(firstPrompt).toContain('Extract decisions and action items.') + expect(firstPrompt.endsWith('Plan my morning')).toBe(true) + + const secondResponse = await adapter.fetch(promptInit('Now summarize'), threadCtx('thread-fallback')) + await act(async () => { + await getClock().runAllAsync() + await readSse(secondResponse) + }) + + expect(sentPromptText(calls, 1)).toBe('Now summarize') + }) +}) + describe('connectAcpAdapter — per-thread session multiplexing over one connection', () => { it('resolves a separate ACP session per thread and persists each independently', async () => { const { transport } = buildFakeTransport() diff --git a/src/acp/acp-adapter.ts b/src/acp/acp-adapter.ts index 4cd2c9f88..c20272b49 100644 --- a/src/acp/acp-adapter.ts +++ b/src/acp/acp-adapter.ts @@ -48,6 +48,12 @@ import type { import { ClientSideConnection as ClientSideConnectionImpl } from '@agentclientprotocol/sdk' import type { Agent, AgentAdapter, AgentAdapterContext, AgentCapabilities, EnsureSessionContext } from '@/types/acp' import type { ThunderboltUIMessage } from '@/types' +import { + buildSkillCatalog, + buildWireSkillsMeta, + supportsWireSkills, + type SkillDefinition, +} from '@shared/agent-core/skills' import { blobToBase64, getAttachments, type HydratedAttachment } from '@/lib/attachments' import { renderMessageQuotesAsText } from '@/lib/quotes' import { getTransformer } from '@/files/transformers' @@ -101,6 +107,7 @@ export const adaptCapabilities = (response: InitializeResponse): AgentCapabiliti const caps = response.agentCapabilities return { loadSession: caps?.loadSession ?? false, + skills: supportsWireSkills(caps?._meta), // `sessionCapabilities.resume` is an empty `SessionResumeCapabilities` // object (`{}`) when supported, `null`/absent otherwise — so presence, not // truthiness, is the signal. @@ -162,6 +169,7 @@ export const buildPromptBlocks = async ( embeddedContext: boolean, deps: PromptBlockDeps = defaultPromptBlockDeps, priorTranscript?: string, + sessionSkillDisclosure?: string, ): Promise => { const lastUser = [...parseRequestMessages(init)].reverse().find((m) => m.role === 'user') if (!lastUser) { @@ -176,7 +184,7 @@ export const buildPromptBlocks = async ( const userText = [quotesText, replyText].filter(Boolean).join('\n\n') // Skill instructions + (fallback) prior transcript ride the text block (ACP // has no system channel) — see composeAcpPrompt. - const text = composeAcpPrompt(skillInstructions, userText, priorTranscript) + const text = composeAcpPrompt(skillInstructions, userText, priorTranscript, sessionSkillDisclosure) const attachments = getAttachments(lastUser) if (attachments.length === 0) { @@ -285,18 +293,27 @@ const extractPriorTranscript = (init: RequestInit): string | undefined => { return transcript.length > 0 ? transcript : undefined } -/** Fold resolved user-skill instructions + (fallback) prior transcript into the - * single prompt-text channel ACP gives us. Order: skill instructions first - * (behavioral, system-like), the prior-conversation context block next, the - * live user text last — mirroring how the built-in pipeline layers system → - * history → prompt. Absent blocks are omitted; with none, the user text is - * sent unchanged. */ +/** Build one complete skill disclosure for an ACP agent without wire support. */ +const buildFallbackSkillDisclosure = (skills: readonly SkillDefinition[]): string | undefined => { + const catalog = buildSkillCatalog(skills) + if (!catalog) { + return undefined + } + const instructions = skills.map(({ name, instruction }) => `### ${name}\n${instruction}`).join('\n\n') + return `## Skills\n${catalog}\n\nFull skill instructions:\n\n${instructions}` +} + +/** Fold session skill disclosure, forced user-skill instructions, and fallback + * prior transcript into ACP's single prompt-text channel. Absent blocks are + * omitted; with none, user text stays unchanged. */ const composeAcpPrompt = ( skillInstructions: string[] | undefined, userText: string, priorTranscript?: string, + sessionSkillDisclosure?: string, ): string => [ + sessionSkillDisclosure, skillInstructions && skillInstructions.length > 0 ? skillInstructions.join('\n\n') : undefined, priorTranscript ? `Conversation so far:\n\n${priorTranscript}` : undefined, userText, @@ -316,6 +333,8 @@ export type AcpAdapterDeps = { webSocketFactory?: WebSocketFactory /** Override throttle for tests of the prompt → translator pipeline. */ textDeltaThrottleMs?: number + /** Resolve enabled app skills when a thread creates or restores its ACP session. */ + getEnabledSkills?: () => Promise /** Connect-phase timeout (ms). Defaults to a generous 30s. Tests inject a * small value to exercise the timeout path deterministically. */ handshakeTimeoutMs?: number @@ -443,6 +462,7 @@ export const connectAcpAdapter = async ( } const capabilities = await runInitialize() + const getEnabledSkills = deps.getEnabledSkills ?? (async () => []) // Resolved ACP session id per chat thread. `null` while a thread's first // resolution is in flight is impossible — we store the in-flight promise so @@ -454,6 +474,7 @@ export const connectAcpAdapter = async ( // prompt never persists an empty session id. type FreshSessionState = { readonly sessionId: string + readonly skillDisclosure?: string readonly persistence?: Promise } const freshPending = new Map() @@ -475,10 +496,22 @@ export const connectAcpAdapter = async ( if (existing) { return existing } - const resolveNew = async (): Promise => { - const newSession = await guardHandshake(connection.newSession({ cwd: sessionCwd, mcpServers: [] })) + const resolveNew = async ( + skills: readonly SkillDefinition[], + skillsMeta: Record | undefined, + ): Promise => { + const newSession = await guardHandshake( + connection.newSession({ + cwd: sessionCwd, + mcpServers: [], + ...(skillsMeta ? { _meta: skillsMeta } : {}), + }), + ) // Defer persistence + transcript seeding to the first real send. - freshPending.set(context.threadId, { sessionId: newSession.sessionId }) + freshPending.set(context.threadId, { + sessionId: newSession.sessionId, + skillDisclosure: capabilities.skills ? undefined : buildFallbackSkillDisclosure(skills), + }) return newSession.sessionId } // Try a stored-session restore, swallowing a runtime rejection (session @@ -492,23 +525,39 @@ export const connectAcpAdapter = async ( ) const resolve = (async (): Promise => { const stored = context.acpSessionId + const skills = await getEnabledSkills() + const skillsMeta = capabilities.skills ? buildWireSkillsMeta(skills) : undefined // `resume` restores execution state with no replay; `load` has the agent // replay its own transcript over `session/update`. Resume is tried first. if ( stored && capabilities.resume && - (await tryRestore(connection.resumeSession({ sessionId: stored, cwd: sessionCwd, mcpServers: [] }))) + (await tryRestore( + connection.resumeSession({ + sessionId: stored, + cwd: sessionCwd, + mcpServers: [], + ...(skillsMeta ? { _meta: skillsMeta } : {}), + }), + )) ) { return stored } if ( stored && capabilities.loadSession && - (await tryRestore(connection.loadSession({ sessionId: stored, cwd: sessionCwd, mcpServers: [] }))) + (await tryRestore( + connection.loadSession({ + sessionId: stored, + cwd: sessionCwd, + mcpServers: [], + ...(skillsMeta ? { _meta: skillsMeta } : {}), + }), + )) ) { return stored } - return resolveNew() + return resolveNew(skills, skillsMeta) })() // Evict on failure so a transient handshake error doesn't poison the thread // — the next send retries a fresh resolution. @@ -520,27 +569,30 @@ export const connectAcpAdapter = async ( /** Persist and consume one freshly minted session marker. A failed write * restores the marker so the next send retries instead of silently losing * both durable session continuity and first-send transcript replay. */ - const consumeFreshSession = async (context: AgentAdapterContext, sessionId: string): Promise => { + const consumeFreshSession = async ( + context: AgentAdapterContext, + sessionId: string, + ): Promise => { const pending = freshPending.get(context.threadId) if (!pending || pending.sessionId !== sessionId) { - return false + return null } if (pending.persistence) { await pending.persistence - return false + return null } const persistence = context.onAcpSessionId(sessionId) - freshPending.set(context.threadId, { sessionId, persistence }) + freshPending.set(context.threadId, { ...pending, persistence }) try { await persistence if (freshPending.get(context.threadId)?.persistence === persistence) { freshPending.delete(context.threadId) } - return true + return pending } catch (error) { if (freshPending.get(context.threadId)?.persistence === persistence) { - freshPending.set(context.threadId, { sessionId }) + freshPending.set(context.threadId, pending) } throw error } @@ -554,14 +606,15 @@ export const connectAcpAdapter = async ( // is keyed on "fresh session's first prompt" (not "we prepended a // transcript") so a brand-new thread's second prompt never re-injects its // first exchange. Consumption happens only after persistence succeeds. - const isFirstSendOfFreshSession = await consumeFreshSession(context, sessionId) - const priorTranscript = isFirstSendOfFreshSession ? extractPriorTranscript(init) : undefined + const freshSession = await consumeFreshSession(context, sessionId) + const priorTranscript = freshSession ? extractPriorTranscript(init) : undefined const prompt = await buildPromptBlocks( init, context.skillInstructions, capabilities.promptCapabilities.embeddedContext, defaultPromptBlockDeps, priorTranscript, + freshSession?.skillDisclosure, ) const { body, translator, close } = createTranslatorStream({ diff --git a/src/acp/connect.test.ts b/src/acp/connect.test.ts index a6c6cc8bb..133c3d04a 100644 --- a/src/acp/connect.test.ts +++ b/src/acp/connect.test.ts @@ -179,7 +179,7 @@ describe('connectToAgent — remote-acp dispatch', () => { const adapter = await connectToAgent( remoteAgent, { httpClient, getProxyFetch }, - { openTransport, ClientSideConnection: FakeConnection as never }, + { openTransport, ClientSideConnection: FakeConnection as never, getEnabledSkills: async () => [] }, ) // initialize runs at connect; session resolution is now lazy/per-thread. @@ -202,7 +202,7 @@ describe('connectToAgent — remote-acp dispatch', () => { const adapter = await connectToAgent( remoteAgent, { httpClient, getProxyFetch }, - { openTransport, ClientSideConnection: FakeConnection as never }, + { openTransport, ClientSideConnection: FakeConnection as never, getEnabledSkills: async () => [] }, ) await adapter.fetch(promptInit('hi'), baseAdapterContext({ acpSessionId: 'existing-sess' })) @@ -221,7 +221,7 @@ describe('connectToAgent — remote-acp dispatch', () => { const adapter = await connectToAgent( remoteAgent, { httpClient, getProxyFetch }, - { openTransport, ClientSideConnection: FakeConnection as never }, + { openTransport, ClientSideConnection: FakeConnection as never, getEnabledSkills: async () => [] }, ) expect(adapter.capabilities).toMatchObject({ resume: true }) @@ -245,7 +245,7 @@ describe('connectToAgent — remote-acp dispatch', () => { const adapter = await connectToAgent( remoteAgent, { httpClient, getProxyFetch }, - { openTransport, ClientSideConnection: FakeConnection as never }, + { openTransport, ClientSideConnection: FakeConnection as never, getEnabledSkills: async () => [] }, ) await adapter.fetch(promptInit('hi'), baseAdapterContext({ acpSessionId: 'old-stale', onAcpSessionId })) @@ -261,7 +261,7 @@ describe('connectToAgent — remote-acp dispatch', () => { const adapter = await connectToAgent( remoteAgent, { httpClient, getProxyFetch }, - { openTransport, ClientSideConnection: FakeConnection as never }, + { openTransport, ClientSideConnection: FakeConnection as never, getEnabledSkills: async () => [] }, ) const init: RequestInit = { diff --git a/src/acp/connect.ts b/src/acp/connect.ts index 4278397da..e040f6fe3 100644 --- a/src/acp/connect.ts +++ b/src/acp/connect.ts @@ -22,6 +22,9 @@ import type { HttpClient } from '@/lib/http' import type { FetchFn } from '@/lib/proxy-fetch' import type { Agent, AgentAdapter } from '@/types/acp' +import { getAllSkills } from '@/dal' +import { getDb } from '@/db/database' +import { selectEnabledSkillDefinitions } from '@/skills/skill-tool' import { connectAcpAdapter, type AcpAdapterDeps } from './acp-adapter' import type { AcpCommand } from './translators/acp-to-ai-sdk' import { createBuiltInAdapter, type BuiltInAdapterOptions } from './built-in-adapter' @@ -40,6 +43,9 @@ export type ConnectToAgentContext = { export type ConnectToAgentDeps = BuiltInAdapterOptions & AcpAdapterDeps +/** Read enabled skills from same DAL source used by built-in agent. */ +const getEnabledSkills = async () => selectEnabledSkillDefinitions(await getAllSkills(getDb())) + /** Build an `AgentAdapter` for the given agent. */ export const connectToAgent = async ( agent: Agent, @@ -58,6 +64,7 @@ export const connectToAgent = async ( webSocketFactory: deps.webSocketFactory, textDeltaThrottleMs: deps.textDeltaThrottleMs, handshakeTimeoutMs: deps.handshakeTimeoutMs, + getEnabledSkills: deps.getEnabledSkills ?? getEnabledSkills, }, ) } diff --git a/src/acp/connection-test.test.ts b/src/acp/connection-test.test.ts index 9cbbe9f86..8143cafe7 100644 --- a/src/acp/connection-test.test.ts +++ b/src/acp/connection-test.test.ts @@ -64,6 +64,7 @@ describe('testAcpConnection', () => { success: true, capabilities: { loadSession: true, + skills: false, resume: false, promptCapabilities: { image: true, audio: false, embeddedContext: true }, }, @@ -85,6 +86,7 @@ describe('testAcpConnection', () => { success: true, capabilities: { loadSession: false, + skills: false, resume: false, promptCapabilities: { image: false, audio: false, embeddedContext: false }, }, @@ -105,6 +107,22 @@ describe('testAcpConnection', () => { expect(result).toMatchObject({ success: true, capabilities: { resume: true, loadSession: false } }) }) + it('maps Thunderbolt skills capability metadata to skills: true', async () => { + const { openTransport, FakeConnection } = buildFakeDeps({ + capabilities: { + _meta: { 'thunderbird.net/thunderbolt': { skills: true } }, + }, + }) + + const result = await testAcpConnection({ + url: 'wss://example.test/ws', + openTransport: openTransport as never, + ClientSideConnection: FakeConnection as never, + }) + + expect(result).toMatchObject({ success: true, capabilities: { skills: true } }) + }) + it('returns the error message when initialize rejects', async () => { const { close, openTransport, FakeConnection } = buildFakeDeps({ initialize: async () => { diff --git a/src/types/acp.ts b/src/types/acp.ts index 0952a26c5..dbbf47e6d 100644 --- a/src/types/acp.ts +++ b/src/types/acp.ts @@ -22,6 +22,9 @@ import type { ChatThread, Mode, Model, SaveMessagesFunction } from '@/types' * prompt-capability flags surface to the composer. */ export type AgentCapabilities = { loadSession: boolean + /** Agent accepts enabled skill definitions through Thunderbolt's namespaced + * ACP session metadata extension. */ + skills: boolean /** Agent advertises `sessionCapabilities.resume` (`session/resume`): it can * restore a prior session's private execution state from its own store * WITHOUT replaying the transcript (unlike `loadSession`). Lets the app hand From 0d03c6fc731024b40068b3ee9c847bf184cced59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8Dtalo=20Menezes?= Date: Fri, 24 Jul 2026 08:53:05 -0300 Subject: [PATCH 04/12] test: add widget regression eval scenarios for disclosure parity --- src/ai/eval/README.md | 6 ++++- src/ai/eval/scenarios.ts | 50 +++++++++++++++++++++++++++++++++++ src/ai/eval/scoring.test.ts | 52 +++++++++++++++++++++++++++++++++++++ src/ai/eval/scoring.ts | 13 ++++++++-- src/ai/eval/types.ts | 11 ++++++++ 5 files changed, 129 insertions(+), 3 deletions(-) diff --git a/src/ai/eval/README.md b/src/ai/eval/README.md index 364aeab56..9cc7ec519 100644 --- a/src/ai/eval/README.md +++ b/src/ai/eval/README.md @@ -169,7 +169,7 @@ Use these names in `EVAL_MODES`: ## Scenarios -15 prompts per mode, tested against each registered model. +Core suites contain 15 prompts per mode, tested against each registered model. Validation, multi-turn, and widget-regression scenarios add focused coverage. **Chat mode** covers: news queries, product recommendations, factual lookups, comparisons, multi-part travel queries, medical info, stock market data, and more. @@ -177,6 +177,8 @@ Use these names in `EVAL_MODES`: **Research mode** covers: multi-country analyses, scientific consensus questions, education system comparisons, gene therapy reviews, housing/migration data correlation, and other prompts requiring 5+ searches and 10+ source citations. +**Widget regression** covers spontaneous weather forecasts, link previews, integration connection prompts, interactive questions, and maps, plus factual and coding prompts that must remain plain text. Citation tags are excluded because citation instructions explicitly forbid them. Document-result tags are excluded because they require Document Search mode and tool results, which this runner does not support. + All scenarios are defined in `scenarios.ts`. ## Scoring @@ -186,6 +188,8 @@ The runner automatically checks: - **`mustProduceOutput`** — Response text must not be empty - **`minCitations`** — Minimum count of `[N]` citation markers - **`mustUseLinkPreviews`** — Must contain `` tags +- **`mustUseWidget`** — Must contain the configured widget tag +- **`mustNotUseWidgets`** — Must not contain any widget tag - **`noHomepageLinks`** — URLs must have deep paths (no `/` or `/section/` only) - **`noReviewSites`** — No links to pcmag.com, cnet.com, wirecutter.com, etc. - **`maxSteps`** — Tool call count must not exceed limit diff --git a/src/ai/eval/scenarios.ts b/src/ai/eval/scenarios.ts index f77ab7bda..988b4f4cd 100644 --- a/src/ai/eval/scenarios.ts +++ b/src/ai/eval/scenarios.ts @@ -298,6 +298,54 @@ const multiTurnChatPrompts: PromptDef[] = [ }, ] +// ────────────────────────────────────────────── +// Widget Regression Prompts — baseline vs progressive disclosure parity +// Citation is excluded because its contract forbids . +// Document-result is excluded because it requires Document Search mode/tool +// results, which this runner does not support. +// ────────────────────────────────────────────── + +const widgetChatPrompts: PromptDef[] = [ + { + id: 'WIDGET_WEATHER_FORECAST', + prompt: "What's the weather like in Berlin this weekend?", + criteria: { mustProduceOutput: true, mustUseWidget: 'weather-forecast' }, + }, + { + id: 'WIDGET_CONNECT_INTEGRATION', + prompt: 'What meetings do I have on my Outlook calendar today?', + criteria: { mustProduceOutput: true, mustUseWidget: 'connect-integration' }, + }, + { + id: 'WIDGET_ASK', + prompt: 'Quiz me with one multiple-choice question about email protocols.', + criteria: { mustProduceOutput: true, mustUseWidget: 'ask' }, + }, + { + id: 'WIDGET_MAP', + prompt: 'Plot Seattle, Portland, and San Francisco together so I can compare their locations.', + criteria: { mustProduceOutput: true, mustUseWidget: 'map' }, + }, + { + id: 'WIDGET_NONE_FACTUAL', + prompt: 'Why does the sky appear blue?', + criteria: { mustProduceOutput: true, mustNotUseWidgets: true }, + }, + { + id: 'WIDGET_NONE_CODING', + prompt: 'Write a TypeScript function that returns the larger of two numbers.', + criteria: { mustProduceOutput: true, mustNotUseWidgets: true }, + }, +] + +const widgetSearchPrompts: PromptDef[] = [ + { + id: 'WIDGET_LINK_PREVIEW', + prompt: 'Find me three beginner-friendly TypeScript tutorials.', + criteria: { ...searchCriteria, mustUseWidget: 'link-preview' }, + }, +] + // ────────────────────────────────────────────── // Scenario Generation // ────────────────────────────────────────────── @@ -328,6 +376,8 @@ const allScenarios: EvalScenario[] = [ ...buildScenarios(validationSearchPrompts, 'search', searchCriteria), ...buildScenarios(validationResearchPrompts, 'research', researchCriteria), ...buildScenarios(multiTurnChatPrompts, 'chat', chatCriteria), + ...buildScenarios(widgetChatPrompts, 'chat', chatCriteria), + ...buildScenarios(widgetSearchPrompts, 'search', searchCriteria), ] /** Get scenarios filtered by model names and mode names */ diff --git a/src/ai/eval/scoring.test.ts b/src/ai/eval/scoring.test.ts index 440af4c4a..cc5b97284 100644 --- a/src/ai/eval/scoring.test.ts +++ b/src/ai/eval/scoring.test.ts @@ -70,3 +70,55 @@ describe('scoreResult — maxToolCalls + duplicate reporting', () => { expect(result.toolCallCount).toBe(2) }) }) + +describe('scoreResult — widget criteria', () => { + const scenario: EvalScenario = { + id: 'opus/chat/WIDGET_TEST', + modelName: 'opus', + modeName: 'chat', + prompt: 'p', + criteria: { mustProduceOutput: true }, + } + + test('requires the configured widget instead of accepting another widget', () => { + const result = scoreResult( + { ...scenario, criteria: { mustProduceOutput: true, mustUseWidget: 'map' } }, + makeParsed({ text: '' }), + 100, + ) + + expect(result.passed).toBe(false) + expect(result.failures).toContain('No tag found in response') + }) + + test('passes when the configured widget is present', () => { + const result = scoreResult( + { ...scenario, criteria: { mustProduceOutput: true, mustUseWidget: 'ask' } }, + makeParsed({ text: `` }), + 100, + ) + + expect(result.passed).toBe(true) + }) + + test('rejects every widget when widgets are forbidden', () => { + const result = scoreResult( + { ...scenario, criteria: { mustProduceOutput: true, mustNotUseWidgets: true } }, + makeParsed({ text: 'Plain answer ' }), + 100, + ) + + expect(result.passed).toBe(false) + expect(result.failures).toContain('Unexpected widget tags found: unknown') + }) + + test('allows plain text when widgets are forbidden', () => { + const result = scoreResult( + { ...scenario, criteria: { mustProduceOutput: true, mustNotUseWidgets: true } }, + makeParsed({ text: 'Plain answer' }), + 100, + ) + + expect(result.passed).toBe(true) + }) +}) diff --git a/src/ai/eval/scoring.ts b/src/ai/eval/scoring.ts index a4ab99f5b..91d99dbb2 100644 --- a/src/ai/eval/scoring.ts +++ b/src/ai/eval/scoring.ts @@ -25,7 +25,7 @@ export const extractCitations = (text: string): string[] => { export const extractLinkPreviewUrls = (text: string): string[] => [...text.matchAll(/url="(https?:\/\/[^"]+)"/g)].map((m) => m[1]) -/** Extract all widget tags from the response (weather-forecast, link-preview, connect-integration) */ +/** Extract all widget tags from the response */ export const extractWidgets = (text: string): string[] => [...text.matchAll(/ m[1]) /** @@ -121,7 +121,7 @@ export const scoreResult = (scenario: EvalScenario, parsed: ParsedStream, durati failures.push(`Error: ${parsed.error}`) } - checkCriteria(criteria, parsed, citations, linkPreviewUrls, homepageUrls, reviewSiteUrls, failures) + checkCriteria(criteria, parsed, citations, widgets, linkPreviewUrls, homepageUrls, reviewSiteUrls, failures) return { scenario, @@ -146,6 +146,7 @@ const checkCriteria = ( criteria: EvalCriteria, parsed: ParsedStream, citations: string[], + widgets: string[], linkPreviewUrls: string[], homepageUrls: string[], reviewSiteUrls: string[], @@ -163,6 +164,14 @@ const checkCriteria = ( failures.push('No tags found in response') } + if (criteria.mustUseWidget && !widgets.includes(criteria.mustUseWidget)) { + failures.push(`No tag found in response`) + } + + if (criteria.mustNotUseWidgets && widgets.length > 0) { + failures.push(`Unexpected widget tags found: ${widgets.join(', ')}`) + } + if (criteria.noHomepageLinks && homepageUrls.length > 0) { failures.push(`Homepage/section URLs found: ${homepageUrls.join(', ')}`) } diff --git a/src/ai/eval/types.ts b/src/ai/eval/types.ts index e73f215c5..fa7d79b93 100644 --- a/src/ai/eval/types.ts +++ b/src/ai/eval/types.ts @@ -4,6 +4,15 @@ import type { ThunderboltUIMessage } from '@/types' +export type WidgetName = + | 'ask' + | 'citation' + | 'connect-integration' + | 'document-result' + | 'link-preview' + | 'map' + | 'weather-forecast' + /** A single evaluation scenario: one prompt tested against one model in one mode */ export type EvalScenario = { id: string @@ -26,6 +35,8 @@ export type EvalCriteria = { mustProduceOutput: boolean minCitations?: number mustUseLinkPreviews?: boolean + mustUseWidget?: WidgetName + mustNotUseWidgets?: boolean noHomepageLinks?: boolean noReviewSites?: boolean maxSteps?: number From f4fc677e9a0cd1777fced51dbb87e24e8257a46d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8Dtalo=20Menezes?= Date: Fri, 24 Jul 2026 09:39:20 -0300 Subject: [PATCH 05/12] fix: seed widget skills unpinned to keep composer chips task-only --- src/defaults/skills.test.ts | 27 +++++++++++---------------- src/defaults/skills.ts | 14 +++++++------- 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/defaults/skills.test.ts b/src/defaults/skills.test.ts index d886184c1..af6a494e5 100644 --- a/src/defaults/skills.test.ts +++ b/src/defaults/skills.test.ts @@ -38,7 +38,7 @@ const computeSnapshotHash = () => const expectedSnapshot = { version: 3, - hash: '0:01996330-0000-7000-8000-000000000001:-eur3ct|1:01996330-0000-7000-8000-000000000002:lp36jd|2:01996330-0000-7000-8000-000000000003:-oawvjh|3:01996330-0000-7000-8000-000000000004:22br3x|4:01996330-0000-7000-8000-000000000005:-72ymfz|5:01996330-0000-7000-8000-000000000006:-31t7et|6:01996330-0000-7000-8000-000000000007:-rhvl8t', + hash: '0:01996330-0000-7000-8000-000000000001:-eur3ct|1:01996330-0000-7000-8000-000000000002:lp36jd|2:01996330-0000-7000-8000-000000000003:-7noodp|3:01996330-0000-7000-8000-000000000004:6xuyso|4:01996330-0000-7000-8000-000000000005:-bonu0t|5:01996330-0000-7000-8000-000000000006:-bjz9so|6:01996330-0000-7000-8000-000000000007:swem9b', } describe('defaultSkills version snapshot', () => { @@ -98,21 +98,16 @@ describe('defaultSkills', () => { expect(names).not.toContain('document-result') }) - it('seeds every default with a pinnedOrder so new users start with pinned chips in chat', () => { - // Regression guard — Chris flagged that seeded skills must be pinned by - // default. Pinning is now manageable only from the chat composer; a new - // user with no pinned defaults would see the chip bar empty until they - // open the `+` popover and pin one manually, which loses the "starter - // chip is ready" affordance that the legacy automations gave them. - for (const skill of defaultSkills) { - expect(typeof skill.pinnedOrder).toBe('number') - expect(skill.pinnedOrder).not.toBeNull() - } - }) - - it('assigns each default a unique pinnedOrder so the order is stable on seed', () => { - const orders = defaultSkills.map((s) => s.pinnedOrder) - expect(new Set(orders).size).toBe(orders.length) + it('pins task skills but not model-facing widget contracts', () => { + expect(defaultSkills.map((skill) => [skill.name, skill.pinnedOrder])).toEqual([ + ['daily-brief', 0], + ['important-emails', 1], + ['weather-forecast', null], + ['link-preview', null], + ['connect-integration', null], + ['ask', null], + ['map', null], + ]) }) it('seeds every default as enabled — disabled defaults would never reach the chat resolver', () => { diff --git a/src/defaults/skills.ts b/src/defaults/skills.ts index aa0500585..75dbff2db 100644 --- a/src/defaults/skills.ts +++ b/src/defaults/skills.ts @@ -75,8 +75,8 @@ const importantEmailsInstruction = `Review the user's inbox and summarize the 5 * The starter set mirrors the legacy `defaultAutomations` so new users get * the same content under the Skills model. * - * Each lands enabled and pinned in the order listed; a user who soft-deletes - * one will not see it re-seeded. + * Each lands enabled. User-facing task skills are pinned; model-facing widget + * contracts are not. A user who soft-deletes one will not see it re-seeded. */ export const defaultSkillDailyBrief: Skill = { id: '01996330-0000-7000-8000-000000000001', @@ -113,7 +113,7 @@ export const defaultSkillWeatherForecast: Skill = { description: 'Use this skill when the user asks for a current or upcoming weather forecast.', instruction: weatherForecastWidgetInstruction, enabled: 1, - pinnedOrder: 2, + pinnedOrder: null, deletedAt: null, defaultHash: null, userId: null, @@ -127,7 +127,7 @@ export const defaultSkillLinkPreview: Skill = { 'Use this skill when the user wants web results, news, products, recommendations, or other fetched pages shown as rich link previews.', instruction: linkPreviewWidgetInstruction, enabled: 1, - pinnedOrder: 3, + pinnedOrder: null, deletedAt: null, defaultHash: null, userId: null, @@ -141,7 +141,7 @@ export const defaultSkillConnectIntegration: Skill = { 'Use this skill when the user asks to access email or calendar but required Google or Microsoft tools are unavailable.', instruction: connectIntegrationWidgetInstruction, enabled: 1, - pinnedOrder: 4, + pinnedOrder: null, deletedAt: null, defaultHash: null, userId: null, @@ -154,7 +154,7 @@ export const defaultSkillAsk: Skill = { description: 'Use this skill when asking the user to choose from options or answer an interactive quiz prompt.', instruction: askWidgetInstruction, enabled: 1, - pinnedOrder: 5, + pinnedOrder: null, deletedAt: null, defaultHash: null, userId: null, @@ -168,7 +168,7 @@ export const defaultSkillMap: Skill = { 'Use this skill when the user asks to see locations, routes, regions, or other geographic results on an interactive map.', instruction: mapWidgetInstruction, enabled: 1, - pinnedOrder: 6, + pinnedOrder: null, deletedAt: null, defaultHash: null, userId: null, From 7f6c8ff886e2634e4fd77703358b97b8d340ffa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8Dtalo=20Menezes?= Date: Fri, 24 Jul 2026 10:05:57 -0300 Subject: [PATCH 06/12] refactor: drop dead citation and document-result instruction exports --- src/ai/prompt.test.ts | 4 +++- src/widgets/citation/index.ts | 1 - src/widgets/citation/instructions.ts | 15 --------------- src/widgets/citation/schema.ts | 4 +++- src/widgets/document-result/index.ts | 1 - src/widgets/document-result/instructions.ts | 8 -------- src/widgets/document-result/schema.ts | 5 +++++ src/widgets/index.ts | 2 +- 8 files changed, 12 insertions(+), 28 deletions(-) delete mode 100644 src/widgets/citation/instructions.ts delete mode 100644 src/widgets/document-result/instructions.ts diff --git a/src/ai/prompt.test.ts b/src/ai/prompt.test.ts index b64f6f6ac..46333849c 100644 --- a/src/ai/prompt.test.ts +++ b/src/ai/prompt.test.ts @@ -167,7 +167,9 @@ describe('createPrompt', () => { const result = createPrompt(baseParams) for (const widget of widgetRegistry) { - expect(result).not.toContain(widget.module.instructions) + if ('instructions' in widget.module) { + expect(result).not.toContain(widget.module.instructions) + } } expect(result).not.toContain('# Widget Components') }) diff --git a/src/widgets/citation/index.ts b/src/widgets/citation/index.ts index dfcae3f9b..90903bbaf 100644 --- a/src/widgets/citation/index.ts +++ b/src/widgets/citation/index.ts @@ -4,7 +4,6 @@ export { CitationBadge } from '@/components/chat/citation-badge' export { Component, CitationWidgetComponent } from './widget' -export { instructions } from './instructions' export { parse, schema } from './schema' export type { CitationWidget } from './schema' diff --git a/src/widgets/citation/instructions.ts b/src/widgets/citation/instructions.ts deleted file mode 100644 index a4dfcc013..000000000 --- a/src/widgets/citation/instructions.ts +++ /dev/null @@ -1,15 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -/** - * AI Instructions for the citation widget - */ -export const instructions = `## Citations -Cite each source ONCE with [N] after the period at the end of the LAST sentence using that source. Add a space before the bracket. Do not repeat the same [N] across paragraphs or bullets. - -Good: Fortaleza was founded in 1726. It is the fifth largest city in Brazil. [1] Recife has 1.6 million residents. [2] -Bad: Fortaleza was founded in 1726. [1] It is the fifth largest city in Brazil. [1] -Bad: According to [Source 1], Fortaleza was founded in 1726. - -Do not use tags, 【1】 brackets, footnotes, or source lists at the end.` diff --git a/src/widgets/citation/schema.ts b/src/widgets/citation/schema.ts index d7091c52c..a7fcaf90d 100644 --- a/src/widgets/citation/schema.ts +++ b/src/widgets/citation/schema.ts @@ -16,7 +16,9 @@ const validateSourcesString = (sources: string): boolean => { } /** - * Zod schema for citation widget + * Citation payload schema. Models cite each source once with `[N]` after the + * final sentence using it; they never emit `` tags, alternate + * brackets, footnotes, or source lists. */ export const schema = z.object({ widget: z.literal('citation'), diff --git a/src/widgets/document-result/index.ts b/src/widgets/document-result/index.ts index 2e8ba5bdc..0b96c9d3d 100644 --- a/src/widgets/document-result/index.ts +++ b/src/widgets/document-result/index.ts @@ -3,6 +3,5 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ export { Component, DocumentResultWidget } from './widget' -export { instructions } from './instructions' export { parse, schema } from './schema' export type { CacheData, DocumentResultWidget as DocumentResultWidgetType } from './schema' diff --git a/src/widgets/document-result/instructions.ts b/src/widgets/document-result/instructions.ts deleted file mode 100644 index 3ac0711d8..000000000 --- a/src/widgets/document-result/instructions.ts +++ /dev/null @@ -1,8 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -export const instructions = `## Document Result - -Shows a source document card with file name and content snippet. -Used in Document Search mode to display source references for grounded answers.` diff --git a/src/widgets/document-result/schema.ts b/src/widgets/document-result/schema.ts index afa5db07d..cfda6c7cf 100644 --- a/src/widgets/document-result/schema.ts +++ b/src/widgets/document-result/schema.ts @@ -8,6 +8,11 @@ import { z } from 'zod' /** * Schema for the document-result widget, used by the Haystack pipeline to * surface a single source document inline with a chat response. + * + * Tag contract: ``. It is reserved for a future Document + * Search/Haystack mode, whose mode prompt must carry model-facing guidance + * when it ships. */ export const schema = z.object({ widget: z.literal('document-result'), diff --git a/src/widgets/index.ts b/src/widgets/index.ts index 85adc2c28..e4173fa0d 100644 --- a/src/widgets/index.ts +++ b/src/widgets/index.ts @@ -7,7 +7,7 @@ * * To add a new widget: * 1. Create a new directory under src/widgets/ with: - * - instructions.ts (model-facing instructions, seeded as a skill when the model emits the widget directly) + * - instructions.ts (only when a seeded skill supplies model-facing instructions) * - schema.ts (Zod schema + parse function) * - [widget-name].tsx (React component) * - [widget-name].stories.tsx (Storybook stories - optional) From b97d8df057f76b19788f9bed1103251c49d1dd16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8Dtalo=20Menezes?= Date: Fri, 24 Jul 2026 10:33:14 -0300 Subject: [PATCH 07/12] fix: skip forced skill instructions already carried by acp session disclosure --- src/acp/acp-adapter.test.ts | 20 ++++++++++++++------ src/acp/acp-adapter.ts | 4 +++- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/acp/acp-adapter.test.ts b/src/acp/acp-adapter.test.ts index 74852eaaa..5230c8d54 100644 --- a/src/acp/acp-adapter.test.ts +++ b/src/acp/acp-adapter.test.ts @@ -432,7 +432,7 @@ describe('connectAcpAdapter — skills capability', () => { expect(sentPromptText(calls)).not.toContain('Extract decisions and action items.') }) - it('folds listing and full bodies into first prompt only for non-capability agents', async () => { + it('discloses each forced skill once on the first prompt and preserves forced invocation later', async () => { const { transport } = buildFakeTransport() const { FakeConnection, calls, releasePrompts } = buildFakeConnection() const adapter = await connectAcpAdapter(remoteAgent, baseCtx(), { @@ -441,7 +441,10 @@ describe('connectAcpAdapter — skills capability', () => { getEnabledSkills: async () => enabledSkills, }) - const firstResponse = await adapter.fetch(promptInit('Plan my morning'), threadCtx('thread-fallback')) + const firstResponse = await adapter.fetch( + promptInit('/daily-brief'), + threadCtx('thread-fallback', { skillInstructions: [enabledSkills[0].instruction] }), + ) await act(async () => { releasePrompts() await getClock().runAllAsync() @@ -450,17 +453,22 @@ describe('connectAcpAdapter — skills capability', () => { const firstPrompt = sentPromptText(calls) expect(firstPrompt).toContain('- daily-brief: Build a concise daily rundown.') - expect(firstPrompt).toContain('Gather current weather and calendar details.') + expect(firstPrompt.split(enabledSkills[0].instruction)).toHaveLength(2) expect(firstPrompt).toContain('Extract decisions and action items.') - expect(firstPrompt.endsWith('Plan my morning')).toBe(true) + expect(firstPrompt.endsWith('/daily-brief')).toBe(true) - const secondResponse = await adapter.fetch(promptInit('Now summarize'), threadCtx('thread-fallback')) + const secondResponse = await adapter.fetch( + promptInit('/daily-brief'), + threadCtx('thread-fallback', { skillInstructions: [enabledSkills[0].instruction] }), + ) await act(async () => { await getClock().runAllAsync() await readSse(secondResponse) }) - expect(sentPromptText(calls, 1)).toBe('Now summarize') + const secondPrompt = sentPromptText(calls, 1) + expect(secondPrompt).toContain(enabledSkills[0].instruction) + expect(secondPrompt.endsWith('/daily-brief')).toBe(true) }) }) diff --git a/src/acp/acp-adapter.ts b/src/acp/acp-adapter.ts index c20272b49..0b214a109 100644 --- a/src/acp/acp-adapter.ts +++ b/src/acp/acp-adapter.ts @@ -314,7 +314,9 @@ const composeAcpPrompt = ( ): string => [ sessionSkillDisclosure, - skillInstructions && skillInstructions.length > 0 ? skillInstructions.join('\n\n') : undefined, + sessionSkillDisclosure === undefined && skillInstructions && skillInstructions.length > 0 + ? skillInstructions.join('\n\n') + : undefined, priorTranscript ? `Conversation so far:\n\n${priorTranscript}` : undefined, userText, ] From 9683b47528d3360e0505aaac7f83d107f5f16d8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8Dtalo=20Menezes?= Date: Fri, 24 Jul 2026 10:33:28 -0300 Subject: [PATCH 08/12] fix: regenerate bun.lock for oauth2-mock-server 9.1.0 --- bun.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bun.lock b/bun.lock index 5de979170..78b8b1e79 100644 --- a/bun.lock +++ b/bun.lock @@ -150,7 +150,7 @@ "jsdom": "^29.1.1", "lint-staged": "^17.0.4", "msw": "^2.10.5", - "oauth2-mock-server": "^8.2.2", + "oauth2-mock-server": "^9.1.0", "playwright": "^1.58.2", "prettier": "^3.6.2", "rsocket-core": "1.0.0-alpha.3", @@ -2074,7 +2074,7 @@ "node-rsa": ["node-rsa@1.1.1", "", { "dependencies": { "asn1": "^0.2.4" } }, "sha512-Jd4cvbJMryN21r5HgxQOpMEqv+ooke/korixNNK3mGqfGJmy0M77WDDzo/05969+OkMy3XW1UuZsSmW9KQm7Fw=="], - "oauth2-mock-server": ["oauth2-mock-server@8.2.2", "", { "dependencies": { "basic-auth": "^2.0.1", "cors": "^2.8.6", "express": "^5.2.1", "is-plain-obj": "^4.1.0", "jose": "^6.1.3" }, "bin": { "oauth2-mock-server": "dist/oauth2-mock-server.js" } }, "sha512-ZjMFtomGM4q1DflxEJpq2WVtSLbOjqlJYkkuGWdq9nL3oAbDExamFyhDN/bUqcooCHWeI0n99K/NU+gXEZLNiA=="], + "oauth2-mock-server": ["oauth2-mock-server@9.1.0", "", { "dependencies": { "basic-auth": "^2.0.1", "is-plain-obj": "^4.1.0", "jose": "^6.2.3" }, "bin": { "oauth2-mock-server": "dist/oauth2-mock-server.mjs" } }, "sha512-1Aug6KQhD9IoxyCogFb0XQqovSOhkvOSRUz5Zm98o96H4omt1HEbUMxwzgwl7GO42eh+BqBH/uCbcKIxi4OBbg=="], "object-assign": ["object-assign@4.1.1", "", {}, "sha512-rJgTQnkUnH1sFw8yT6VSU3zD3sWmu6sZhIseY8VX+GRu3P6F7Fu+JNDoXfklElbLJSnc3FUQHVe4cU5hj+BcUg=="], From 6975d1573e45a1aecd50493d857b27092c00599b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8Dtalo=20Menezes?= Date: Fri, 24 Jul 2026 11:10:42 -0300 Subject: [PATCH 09/12] feat: lock widget default skills to enable and disable only --- src/dal/skills.test.ts | 44 +++++++++++++ src/dal/skills.ts | 19 ++++-- src/defaults/skills.test.ts | 19 ++++++ src/defaults/skills.ts | 20 ++++-- src/skills/library-row.tsx | 95 +++++++++++++++------------- src/skills/skill-detail.tsx | 13 +++- src/skills/skills-view-state.test.ts | 40 ++++++++++++ src/skills/skills-view-state.ts | 43 +++++++------ src/skills/skills-view.test.tsx | 24 +++++++ src/skills/skills-view.tsx | 7 +- 10 files changed, 248 insertions(+), 76 deletions(-) diff --git a/src/dal/skills.test.ts b/src/dal/skills.test.ts index 193b30246..fcae052df 100644 --- a/src/dal/skills.test.ts +++ b/src/dal/skills.test.ts @@ -4,6 +4,7 @@ import { getDb } from '@/db/database' import { skillsTable } from '@/db/tables' +import { defaultSkillWeatherForecast } from '@/defaults/skills' import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'bun:test' import { eq } from 'drizzle-orm' import { @@ -24,6 +25,7 @@ import { softDeleteSkill, updateSkill, validateSkillName, + type UpdateSkillInput, } from './skills' import { resetTestDatabase, setupTestDatabase, teardownTestDatabase } from './test-utils' @@ -47,6 +49,12 @@ const seed = async (input: { name: string; label?: string; description?: string; instruction: input.instruction ?? `instruction for ${input.name}`, }) +/** Insert one stable widget contract for DAL mutation tests. */ +const seedWidgetSkill = async () => { + await getDb().insert(skillsTable).values(defaultSkillWeatherForecast) + return defaultSkillWeatherForecast +} + describe('validateSkillName (AgentSkills spec)', () => { it('accepts canonical slugs', () => { expect(validateSkillName('meeting-notes')).toBeNull() @@ -191,6 +199,35 @@ describe('skills DAL', () => { const a = await seed({ name: 'legit-name' }) await expect(updateSkill(getDb(), a.id, { name: 'SHOUTING' })).rejects.toBeInstanceOf(SkillNameInvalidError) }) + + it('allows enabled and pinnedOrder patches for widget skills', async () => { + const widget = await seedWidgetSkill() + + await updateSkill(getDb(), widget.id, { enabled: 0, pinnedOrder: 2 }) + + const after = await getSkill(getDb(), widget.id) + expect(after?.enabled).toBe(0) + expect(after?.pinnedOrder).toBe(2) + }) + + it('rejects content patches for widget skills', async () => { + const widget = await seedWidgetSkill() + const disallowedPatches: UpdateSkillInput[] = [ + { name: 'renamed-widget' }, + { label: 'Renamed Widget' }, + { description: 'Changed description' }, + { instruction: 'Changed instruction' }, + { enabled: 0, description: 'Mixed allowed and disallowed fields' }, + ] + + for (const patch of disallowedPatches) { + await expect(updateSkill(getDb(), widget.id, patch)).rejects.toThrow(/widget skill/i) + } + + const after = await getSkill(getDb(), widget.id) + expect(after?.enabled).toBe(1) + expect(after?.description).toBe(defaultSkillWeatherForecast.description) + }) }) describe('softDeleteSkill', () => { @@ -224,6 +261,13 @@ describe('skills DAL', () => { const all = await getAllSkills(getDb()) expect(all.find((s) => s.id === skill.id)).toBeUndefined() }) + + it('rejects deleting widget skills', async () => { + const widget = await seedWidgetSkill() + + await expect(softDeleteSkill(getDb(), widget.id)).rejects.toThrow(/widget skill/i) + expect(await getSkill(getDb(), widget.id)).toEqual(widget) + }) }) describe('setPinned', () => { diff --git a/src/dal/skills.ts b/src/dal/skills.ts index 25a904c06..960cc0fad 100644 --- a/src/dal/skills.ts +++ b/src/dal/skills.ts @@ -6,6 +6,7 @@ import { and, asc, eq, inArray, isNotNull, isNull, ne, sql } from 'drizzle-orm' import { v7 as uuidv7 } from 'uuid' import type { AnyDrizzleDatabase } from '../db/database-interface' import { skillsTable } from '../db/tables' +import { isWidgetSkillId } from '../defaults/skills' import type { DrizzleQueryWithPromise, Skill } from '../types' import { nowIso } from '../lib/utils' @@ -185,14 +186,20 @@ export const createSkill = async (db: AnyDrizzleDatabase, input: CreateSkillInpu return row } -export type UpdateSkillInput = Partial> +export type UpdateSkillInput = Partial< + Pick +> /** * Patch an existing skill. Throws {@link SkillNameInvalidError} if `name` fails - * the AgentSkills spec, or {@link SkillNameTakenError} if it collides with - * another skill. + * the AgentSkills spec, {@link SkillNameTakenError} if it collides with another + * skill, or `Error` if a widget rendering contract receives a content patch. */ export const updateSkill = async (db: AnyDrizzleDatabase, id: string, patch: UpdateSkillInput): Promise => { + const changesWidgetContent = Object.keys(patch).some((field) => field !== 'enabled' && field !== 'pinnedOrder') + if (isWidgetSkillId(id) && changesWidgetContent) { + throw new Error(`updateSkill: widget skill "${id}" only supports enabled and pinnedOrder updates`) + } if (patch.name !== undefined) { const slugError = validateSkillName(patch.name) if (slugError) { @@ -206,9 +213,13 @@ export const updateSkill = async (db: AnyDrizzleDatabase, id: string, patch: Upd /** * Soft-delete a skill: set `deleted_at` and wipe user content (`name`, `label`, * `description`, `instruction`). The tombstone (`id`, `user_id`, `deleted_at`) - * remains so PowerSync propagates the delete to other devices. + * remains so PowerSync propagates the delete to other devices. Widget + * rendering contracts cannot be deleted. */ export const softDeleteSkill = async (db: AnyDrizzleDatabase, id: string): Promise => { + if (isWidgetSkillId(id)) { + throw new Error(`softDeleteSkill: refusing to delete widget skill "${id}"`) + } await db .update(skillsTable) .set({ diff --git a/src/defaults/skills.test.ts b/src/defaults/skills.test.ts index af6a494e5..c7ea2585a 100644 --- a/src/defaults/skills.test.ts +++ b/src/defaults/skills.test.ts @@ -10,6 +10,8 @@ import { instructions as linkPreviewWidgetInstruction } from '@/widgets/link-pre import { instructions as mapWidgetInstruction } from '@/widgets/map/instructions' import { instructions as weatherForecastWidgetInstruction } from '@/widgets/weather-forecast/instructions' import { + defaultSkillDailyBrief, + defaultSkillImportantEmails, defaultSkillAsk, defaultSkillConnectIntegration, defaultSkillLinkPreview, @@ -18,6 +20,7 @@ import { defaultSkillsVersion, defaultSkillWeatherForecast, hashSkill, + isWidgetSkillId, } from './skills' /** @@ -110,6 +113,22 @@ describe('defaultSkills', () => { ]) }) + it('identifies widget contracts by stable default id', () => { + for (const skill of [ + defaultSkillWeatherForecast, + defaultSkillLinkPreview, + defaultSkillConnectIntegration, + defaultSkillAsk, + defaultSkillMap, + ]) { + expect(isWidgetSkillId(skill.id)).toBe(true) + } + + expect(isWidgetSkillId(defaultSkillDailyBrief.id)).toBe(false) + expect(isWidgetSkillId(defaultSkillImportantEmails.id)).toBe(false) + expect(isWidgetSkillId('user-skill-id')).toBe(false) + }) + it('seeds every default as enabled — disabled defaults would never reach the chat resolver', () => { for (const skill of defaultSkills) { expect(skill.enabled).toBe(1) diff --git a/src/defaults/skills.ts b/src/defaults/skills.ts index 75dbff2db..60ac515df 100644 --- a/src/defaults/skills.ts +++ b/src/defaults/skills.ts @@ -11,9 +11,9 @@ import { instructions as mapWidgetInstruction } from '@/widgets/map/instructions import { instructions as weatherForecastWidgetInstruction } from '@/widgets/weather-forecast/instructions' /** - * Hash of user-editable fields. Includes `deletedAt` so soft-deletes are - * treated as a user configuration choice — a user who deletes a default does - * NOT get it re-seeded on next app init. + * Hash of reconciled skill fields. Includes `deletedAt` so soft-deletes of + * editable defaults are treated as a user configuration choice and are not + * re-seeded on next app init. * * Accepts raw (nullable) rows as well as `Skill` so the hash-restamp data * migration can stamp exactly what reconciliation will later recompute. @@ -76,7 +76,8 @@ const importantEmailsInstruction = `Review the user's inbox and summarize the 5 * the same content under the Skills model. * * Each lands enabled. User-facing task skills are pinned; model-facing widget - * contracts are not. A user who soft-deletes one will not see it re-seeded. + * contracts are not. Task skills may be edited or soft-deleted; widget + * contracts only expose enabled and pinned state. */ export const defaultSkillDailyBrief: Skill = { id: '01996330-0000-7000-8000-000000000001', @@ -174,6 +175,17 @@ export const defaultSkillMap: Skill = { userId: null, } +const widgetSkillIds = new Set([ + defaultSkillWeatherForecast.id, + defaultSkillLinkPreview.id, + defaultSkillConnectIntegration.id, + defaultSkillAsk.id, + defaultSkillMap.id, +]) + +/** Whether a skill id belongs to a model-facing widget rendering contract. */ +export const isWidgetSkillId = (id: string): boolean => widgetSkillIds.has(id) + export const defaultSkills: ReadonlyArray = [ defaultSkillDailyBrief, defaultSkillImportantEmails, diff --git a/src/skills/library-row.tsx b/src/skills/library-row.tsx index e396eec7c..a67037691 100644 --- a/src/skills/library-row.tsx +++ b/src/skills/library-row.tsx @@ -7,6 +7,7 @@ import { SquarePen, Trash2 } from 'lucide-react' import { ContextMenu, ContextMenuContent, ContextMenuItem, ContextMenuTrigger } from '@/components/ui/context-menu' import { Switch } from '@/components/ui/switch' +import { isWidgetSkillId } from '@/defaults/skills' import type { Skill } from '@/types' import { skillDisplayName } from './display' @@ -48,52 +49,58 @@ export const LibraryRow = ({ onEdit: (id: string) => void onDelete: (id: string) => void }) => { + const row = ( +
onSelect(skill.id)} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault() + onSelect(skill.id) + } + }} + className={`group flex h-[var(--touch-height-default)] w-full cursor-pointer items-center gap-2 rounded-lg px-2.5 text-base transition-colors ${ + enabled ? 'text-foreground' : 'text-muted-foreground/60' + } ${isActive ? 'bg-accent' : 'hover:bg-accent'}`} + > + {skillDisplayName(skill)} + {/* `inline-flex items-center` keeps the Switch optically centered + with the name's text baseline. stopPropagation so toggling + doesn't also open the detail panel. */} + e.stopPropagation()} + onKeyDown={(e) => e.stopPropagation()} + > + onToggleEnabled(skill.id, next)} + aria-label={`${enabled ? 'Disable' : 'Enable'} ${skillDisplayName(skill)}`} + /> + +
+ ) + return ( - - -
onSelect(skill.id)} - onKeyDown={(e) => { - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault() - onSelect(skill.id) - } - }} - className={`group flex h-[var(--touch-height-default)] w-full cursor-pointer items-center gap-2 rounded-lg px-2.5 text-base transition-colors ${ - enabled ? 'text-foreground' : 'text-muted-foreground/60' - } ${isActive ? 'bg-accent' : 'hover:bg-accent'}`} - > - {skillDisplayName(skill)} - {/* `inline-flex items-center` keeps the Switch optically centered - with the name's text baseline. stopPropagation so toggling - doesn't also open the detail panel. */} - e.stopPropagation()} - onKeyDown={(e) => e.stopPropagation()} - > - onToggleEnabled(skill.id, next)} - aria-label={`${enabled ? 'Disable' : 'Enable'} ${skillDisplayName(skill)}`} - /> - -
-
- - onEdit(skill.id)} className="cursor-pointer"> - - Edit - - onDelete(skill.id)} className="cursor-pointer"> - - Delete - - -
+ {isWidgetSkillId(skill.id) ? ( + row + ) : ( + + {row} + + onEdit(skill.id)} className="cursor-pointer"> + + Edit + + onDelete(skill.id)} className="cursor-pointer"> + + Delete + + + + )}
) } diff --git a/src/skills/skill-detail.tsx b/src/skills/skill-detail.tsx index 5fa314c12..effe17a27 100644 --- a/src/skills/skill-detail.tsx +++ b/src/skills/skill-detail.tsx @@ -12,12 +12,13 @@ import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip /** * Detail panel for a single skill. Pinning is managed from the chat composer * and enable/disable lives on the list row's switch; this view shows the - * skill's content plus edit / delete controls. + * skill's content plus edit/delete controls when the skill is editable. */ export const SkillDetail = ({ name, description, instruction, + readOnly, onEdit, onDelete, onClose, @@ -26,13 +27,14 @@ export const SkillDetail = ({ name: string description: string instruction: string + readOnly: boolean onEdit: () => void onDelete: () => void /** Close (X, right of the actions menu) — dismisses the desktop slide-in * panel or the mobile overlay. */ onClose: () => void }) => { - const actionsMenu = ( + const actionsMenu = !readOnly && (