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/shared/agent-core/skills.test.ts b/shared/agent-core/skills.test.ts new file mode 100644 index 000000000..139a21f6d --- /dev/null +++ b/shared/agent-core/skills.test.ts @@ -0,0 +1,114 @@ +/* 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 { + buildFallbackSkillDisclosure, + 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('buildFallbackSkillDisclosure', () => { + it('includes compact catalog plus every full instruction body', () => { + const disclosure = buildFallbackSkillDisclosure(skills) + + expect(disclosure).toContain('- daily-brief: Use for a daily rundown. Includes weather and calendar.') + expect(disclosure).toContain('- meeting-notes: Use when summarizing meetings.') + expect(disclosure).toContain('### daily-brief\nGather current weather, news, email, and calendar details.') + expect(disclosure).toContain('### meeting-notes\nExtract decisions and action items.') + }) + + it('omits the section when no skills are available', () => { + expect(buildFallbackSkillDisclosure([])).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..78d5b35e6 --- /dev/null +++ b/shared/agent-core/skills.ts @@ -0,0 +1,122 @@ +/* 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 complete inline skill guidance for agents without skill-loading support. + * + * @param skills - enabled skills available to current agent + * @returns compact catalog plus full instruction bodies, or undefined when empty + */ +export 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}` +} + +/** + * 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/acp-adapter.test.ts b/src/acp/acp-adapter.test.ts index 5173d9496..5230c8d54 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,109 @@ 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('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(), { + openTransport: async () => transport, + ClientSideConnection: FakeConnection as never, + getEnabledSkills: async () => enabledSkills, + }) + + const firstResponse = await adapter.fetch( + promptInit('/daily-brief'), + threadCtx('thread-fallback', { skillInstructions: [enabledSkills[0].instruction] }), + ) + 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.split(enabledSkills[0].instruction)).toHaveLength(2) + expect(firstPrompt).toContain('Extract decisions and action items.') + expect(firstPrompt.endsWith('/daily-brief')).toBe(true) + + const secondResponse = await adapter.fetch( + promptInit('/daily-brief'), + threadCtx('thread-fallback', { skillInstructions: [enabledSkills[0].instruction] }), + ) + await act(async () => { + await getClock().runAllAsync() + await readSse(secondResponse) + }) + + const secondPrompt = sentPromptText(calls, 1) + expect(secondPrompt).toContain(enabledSkills[0].instruction) + expect(secondPrompt.endsWith('/daily-brief')).toBe(true) + }) +}) + 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..27007129f 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 { + buildFallbackSkillDisclosure, + 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,19 +293,20 @@ 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. */ +/** 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 => [ - skillInstructions && skillInstructions.length > 0 ? skillInstructions.join('\n\n') : undefined, + sessionSkillDisclosure, + sessionSkillDisclosure === undefined && skillInstructions && skillInstructions.length > 0 + ? skillInstructions.join('\n\n') + : undefined, priorTranscript ? `Conversation so far:\n\n${priorTranscript}` : undefined, userText, ] @@ -316,6 +325,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 +454,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 +466,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 +488,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 +517,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 +561,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 +598,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/built-in-adapter.test.ts b/src/acp/built-in-adapter.test.ts index f91d4ba88..c6424637b 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}`, @@ -196,10 +203,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' }] }, ]), @@ -207,7 +214,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' }] }, ]), @@ -221,10 +228,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 094a4a8af..6cf5c6bc0 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' @@ -438,7 +439,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/acp/connect.test.ts b/src/acp/connect.test.ts index 8a3f67c91..f37e87189 100644 --- a/src/acp/connect.test.ts +++ b/src/acp/connect.test.ts @@ -175,7 +175,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. @@ -198,7 +198,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' })) @@ -217,7 +217,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 }) @@ -241,7 +241,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 })) @@ -257,7 +257,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/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..b8eebbff4 100644 --- a/src/ai/eval/types.ts +++ b/src/ai/eval/types.ts @@ -3,6 +3,9 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import type { ThunderboltUIMessage } from '@/types' +import type { WidgetName } from '@/widgets' + +export type { WidgetName } /** A single evaluation scenario: one prompt tested against one model in one mode */ export type EvalScenario = { @@ -26,6 +29,8 @@ export type EvalCriteria = { mustProduceOutput: boolean minCitations?: number mustUseLinkPreviews?: boolean + mustUseWidget?: WidgetName + mustNotUseWidgets?: boolean noHomepageLinks?: boolean noReviewSites?: boolean maxSteps?: number diff --git a/src/ai/fetch.test.ts b/src/ai/fetch.test.ts index c04ce542d..f125bf35b 100644 --- a/src/ai/fetch.test.ts +++ b/src/ai/fetch.test.ts @@ -3,12 +3,22 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import { describe, expect, it, mock } from 'bun:test' +import { createPrompt } from '@/ai/prompt' +import { defaultSkillResearch, defaultSkillWeather } from '@/defaults/skills' import { fetch as baseFetch } from '@/lib/fetch' import type { MCPClient, NamedMCPClient } from '@/lib/mcp-provider' import type { FetchFn } from '@/lib/proxy-fetch' -import type { Model } from '@/types' +import { resolveSkillTokenInstructions } from '@/skills/resolve-skill-system-messages' +import { selectEnabledSkillDefinitions } from '@/skills/skill-tool' +import type { Model, Skill } from '@/types' import type { Tool } from 'ai' -import { mergeMcpTools, resolveOpenAiCompatConnection, sanitizeToolPrefix } from './fetch' +import { + addSkillTool, + mergeMcpTools, + resolveOpenAiCompatConnection, + sanitizeToolPrefix, + selectPromptSkillDefinitions, +} from './fetch' /** Mirror the `MCPClientError` the SDK throws after a transport drop. The * runtime instance `name` is `'MCPClientError'` (the `AI_MCPClientError` @@ -51,6 +61,87 @@ describe('sanitizeToolPrefix', () => { }) }) +describe('addSkillTool', () => { + const skills = [ + { + name: 'weather', + description: 'Use for weather forecasts.', + instruction: 'Emit the weather widget contract.', + }, + ] + + it('registers the skill tool only for tool-capable models', () => { + expect(Object.keys(addSkillTool({}, skills, true))).toEqual(['skill']) + expect(addSkillTool({}, skills, false)).toEqual({}) + }) +}) + +describe('selectPromptSkillDefinitions', () => { + const storedSkills: Skill[] = [ + { ...defaultSkillWeather, instruction: 'WIDGET_CONTRACT_BODY' }, + { ...defaultSkillResearch, instruction: 'TASK_SKILL_BODY' }, + { + ...defaultSkillResearch, + id: 'user-authored-skill', + name: 'user-authored', + label: 'User Authored', + instruction: 'USER_AUTHORED_SKILL_BODY', + }, + ] + + /** Build a prompt with only skill capability varying between cases. */ + const createSkillPrompt = (supportsTools: boolean) => + createPrompt({ + modelName: 'Test Model', + profile: null, + preferredName: '', + location: {}, + localization: { + distanceUnit: 'imperial', + temperatureUnit: 'f', + dateFormat: 'MM/DD/YYYY', + timeFormat: '12h', + currency: 'USD', + }, + integrationStatus: 'READY', + hasWebTools: false, + skills: selectPromptSkillDefinitions(storedSkills, supportsTools), + supportsTools, + }) + + it('inlines only widget rendering contracts for non-tool models', () => { + const prompt = createSkillPrompt(false) + + expect(prompt).toContain('### weather\nWIDGET_CONTRACT_BODY') + expect(prompt).not.toContain('TASK_SKILL_BODY') + expect(prompt).not.toContain('USER_AUTHORED_SKILL_BODY') + expect(prompt).not.toContain('- research:') + expect(prompt).not.toContain('- user-authored:') + }) + + it('keeps the full skill listing for tool-capable models', () => { + const prompt = createSkillPrompt(true) + + expect(prompt).toContain('Use the `skill` tool') + expect(prompt).toContain('- weather:') + expect(prompt).toContain('- research:') + expect(prompt).toContain('- user-authored:') + expect(prompt).not.toContain('WIDGET_CONTRACT_BODY') + expect(prompt).not.toContain('TASK_SKILL_BODY') + expect(prompt).not.toContain('USER_AUTHORED_SKILL_BODY') + }) + + it('keeps every enabled skill in the slash-token resolution map', () => { + const skills = selectEnabledSkillDefinitions(storedSkills) + const instructionBySlug = new Map(skills.map(({ name, instruction }) => [name, instruction])) + + expect(resolveSkillTokenInstructions('/research /user-authored', instructionBySlug)).toEqual([ + 'TASK_SKILL_BODY', + 'USER_AUTHORED_SKILL_BODY', + ]) + }) +}) + describe('mergeMcpTools', () => { it('prefixes each tool with its sanitized server name', async () => { const render = named('render', async () => ({ list_services: tool('ls'), get_service: tool('gs') })) diff --git a/src/ai/fetch.ts b/src/ai/fetch.ts index ac1d766a5..2453ab56b 100644 --- a/src/ai/fetch.ts +++ b/src/ai/fetch.ts @@ -14,7 +14,9 @@ import { } from '@/ai/step-logic' import { getAllSkills, getIntegrationStatus, getModel, getModelProfile, getSettings } from '@/dal' import { getMessage } from '@/dal/chat-messages' +import { isWidgetSkillId } from '@/defaults/skills' 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' @@ -27,7 +29,7 @@ import { isLoopbackHost } from '@/lib/mcp-url-validation' import { normalizeOpenAiBaseUrl } from '@/lib/openai-base-url' import type { FetchFn } from '@/lib/proxy-fetch' import { createToolset, getAvailableTools, type ToolCallCache } from '@/lib/tools' -import type { Model, ModelProfile, ThunderboltUIMessage, UIMessageMetadata } from '@/types' +import type { Model, ModelProfile, Skill, ThunderboltUIMessage, UIMessageMetadata } from '@/types' import type { SourceMetadata } from '@/types/source' import { createAnthropic } from '@ai-sdk/anthropic' import { createOpenAI } from '@ai-sdk/openai' @@ -60,6 +62,7 @@ import { import type { MCPClient, 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' @@ -548,6 +551,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 @@ -561,6 +565,27 @@ export type PrepareAiRequestConfigOptions = { readonly httpClient: HttpClient } +/** Register progressive skill loading only for models that support tools. */ +export const addSkillTool = ( + toolset: Record, + skills: readonly SkillDefinition[], + supportsTools: boolean, +): Record => { + if (supportsTools) { + toolset.skill = createSkillTool(skills) + } + return toolset +} + +/** + * Select skills disclosed in the built-in model's system prompt. + * + * Tool-capable models receive every enabled skill, while non-tool models only + * receive widget rendering contracts inline. + */ +export const selectPromptSkillDefinitions = (skills: readonly Skill[], supportsTools: boolean): SkillDefinition[] => + selectEnabledSkillDefinitions(supportsTools ? skills : skills.filter(({ id }) => isWidgetSkillId(id))) + /** Load model/profile/settings and build one send's app + MCP tools and prompt. */ export const prepareAiRequestConfig = async ({ modelId, @@ -589,13 +614,15 @@ export const prepareAiRequestConfig = async ({ throw new Error('Model not found') } const profile = await getModelProfile(db, modelId) + const storedSkills = await getAllSkills(db) + const skills = selectEnabledSkillDefinitions(storedSkills) const supportsTools = model.toolUsage !== 0 const sourceCollector: SourceMetadata[] = [] const toolCallCache: ToolCallCache = new Map() const availableTools = supportsTools ? await getAvailableTools(httpClient, sourceCollector, { settings, integrationStatus }) : [] - const appToolset = createToolset(availableTools, toolCallCache) + const appToolset = addSkillTool(createToolset(availableTools, toolCallCache), skills, supportsTools) const hasWebTools = 'search' in appToolset && 'fetch_content' in appToolset const merged = supportsTools ? await mergeMcpTools(appToolset, mcpClients, reconnectClient) @@ -625,6 +652,8 @@ export const prepareAiRequestConfig = async ({ integrationStatus: integrationStatuses.length > 0 ? integrationStatuses.join(', ') : 'READY', hasWebTools, mcpServersSummary: merged.summary, + skills: selectPromptSkillDefinitions(storedSkills, supportsTools), + supportsTools, }) return { @@ -633,6 +662,7 @@ export const prepareAiRequestConfig = async ({ supportsTools, sourceCollector, toolset: merged.toolset, + skills, mcpToolsMetadata: merged.mcpTools, stableSystemPrompt: prompt.stablePrompt, volatileSystemPrompt: prompt.volatilePrompt, @@ -658,7 +688,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, mcpClients, @@ -803,13 +833,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 82085ba5e..643afe8a0 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' @@ -134,6 +135,61 @@ describe('createPrompt', () => { expect(result).toContain('never state facts without verifying them first') }) + test('tool-capable models get the skill listing without instruction bodies', () => { + const result = createPrompt({ + ...baseParams, + supportsTools: true, + skills: [ + { + name: 'daily-brief', + description: 'Use for a daily rundown.', + instruction: 'Gather private full instructions here.', + }, + ], + }) + + expect(result).toContain('## Skills') + expect(result).toContain('Use the `skill` tool') + expect(result).toContain('- daily-brief: Use for a daily rundown.') + expect(result).not.toContain('Gather private full instructions here.') + }) + + test('non-tool models get the skill catalog and full instruction bodies inline', () => { + const result = createPrompt({ + ...baseParams, + supportsTools: false, + skills: [ + { + name: 'weather', + description: 'Use for weather forecasts.', + instruction: 'Emit the weather widget contract.', + }, + ], + }) + + expect(result).toContain('- weather: Use for weather forecasts.') + expect(result).toContain('Full skill instructions:') + expect(result).toContain('### weather\nEmit the weather widget contract.') + expect(result).not.toContain('Use the `skill` tool') + }) + + test('does not inject widget instruction bodies into every prompt', () => { + const result = createPrompt(baseParams) + + for (const widget of widgetRegistry) { + if ('instructions' in widget.module) { + 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 ad5ca4d74..e4a70370b 100644 --- a/src/ai/prompt.ts +++ b/src/ai/prompt.ts @@ -4,8 +4,8 @@ import { chatPrompt } from '@/ai/prompts/chat' import { webToolsPrompt } from '@/ai/prompts/web-tools' -import { widgetPrompts } from '@/widgets' import type { ModelProfile } from '@/types' +import { buildFallbackSkillDisclosure, buildSkillListing, type SkillDefinition } from '@shared/agent-core/skills' /** Parameters to build the system prompt */ export type PromptParams = { @@ -26,6 +26,10 @@ export type PromptParams = { hasWebTools: boolean /** Summary of connected MCP servers (name + tool count) */ mcpServersSummary?: string + /** Enabled skills available to the model */ + skills?: readonly SkillDefinition[] + /** Whether the model can load skill instructions through tools */ + supportsTools?: boolean } export type PromptParts = { @@ -45,6 +49,8 @@ export const createPromptParts = ( integrationStatus, hasWebTools, mcpServersSummary, + skills = [], + supportsTools = true, }: PromptParams, currentDate: Date = new Date(), ): PromptParts => { @@ -75,6 +81,7 @@ export const createPromptParts = ( ] .filter(Boolean) .join('\n') + const skillDisclosure = supportsTools ? buildSkillListing(skills) : buildFallbackSkillDisclosure(skills) // Output Format asks models to format math as `$…$` / `$$…$$` only (never // `\(…\)` / `\[…\]`). The chat renderer (src/components/chat/memoized-markdown.tsx) @@ -125,6 +132,7 @@ Don't mention tool names unless asked. ${hasWebTools ? `\n${webToolsPrompt}` : ''} ${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.` : ''} +${skillDisclosure ? `\n${skillDisclosure}` : ''} ## Link Previews • Aggregate pages (listicles, "Top 10") are for DISCOVERY ONLY @@ -132,11 +140,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/components/chat/chat-skills-bar.test.tsx b/src/components/chat/chat-skills-bar.test.tsx index cb536ba78..de8e23455 100644 --- a/src/components/chat/chat-skills-bar.test.tsx +++ b/src/components/chat/chat-skills-bar.test.tsx @@ -8,6 +8,7 @@ import { MemoryRouter } from 'react-router' import { CreateItemProvider } from '@/components/create-item/context' import { TooltipProvider } from '@/components/ui/tooltip' +import { defaultSkillWeather } from '@/defaults/skills' import { CreateRequestProbe } from '@/test-utils/create-request-probe' import { waitForElement } from '@/test-utils/powersync-reactivity-test' import { forceMobileViewport, restoreViewport } from '@/test-utils/viewport' @@ -109,6 +110,34 @@ describe('ChatSkillsBar', () => { expect(screen.getByLabelText('Add a skill')).toBeTruthy() }) + it('excludes widget skills from pin candidates', () => { + const task = { ...skill('task', 'daily-brief'), label: 'Daily Brief' } + renderBar({ + useLibrarySkills: fakeUseLibrarySkills([task, defaultSkillWeather]), + useEnabledSkills: fakeUseEnabledSkills(new Set([task.id, defaultSkillWeather.id])), + }) + + fireEvent.click(screen.getByLabelText('Add a skill')) + + expect(screen.getByText('Daily Brief')).toBeTruthy() + expect(screen.queryByText('Weather')).toBeNull() + }) + + it('keeps pinned widget skill actions read-only', async () => { + renderBar({ + usePinnedSkills: fakeUsePinnedSkills({ pinned: [defaultSkillWeather] }), + useLibrarySkills: fakeUseLibrarySkills([defaultSkillWeather]), + useEnabledSkills: fakeUseEnabledSkills(new Set([defaultSkillWeather.id])), + }) + + fireEvent.contextMenu(screen.getByText('Weather')) + + expect(await waitForElement(() => screen.queryByText('Add to chat'))).toBeTruthy() + expect(screen.queryByText('Edit skill')).toBeNull() + expect(screen.queryByText('Reorder')).toBeNull() + expect(screen.queryByText('Unpin')).toBeNull() + }) + it('keeps the "+ Add a skill" trigger clickable when every enabled skill is already pinned (popover still offers New skill)', () => { const a = skill('a', 'daily-brief') renderBar({ diff --git a/src/components/chat/chat-skills-bar.tsx b/src/components/chat/chat-skills-bar.tsx index c9fd68c16..4e900f529 100644 --- a/src/components/chat/chat-skills-bar.tsx +++ b/src/components/chat/chat-skills-bar.tsx @@ -13,6 +13,7 @@ import { ResponsivePopover } from '@/components/ui/responsive-popover' import { SearchInput } from '@/components/ui/search-input' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { maxPinnedSkills } from '@/dal' +import { isWidgetSkillId } from '@/defaults/skills' import { useIsMobile } from '@/hooks/use-mobile' import { cn } from '@/lib/utils' import { skillDisplayName, skillMatchesQuery } from '@/skills/display' @@ -107,6 +108,7 @@ export const ChatSkillsBar = ({ const { openCreateItem } = useCreateItem() const [{ reorderMode, addOpen, addQuery, actionError }, dispatch] = useReducer(barReducer, initialBarState) + const lockedPinnedIds = new Set(pinned.filter((skill) => isWidgetSkillId(skill.id)).map((skill) => skill.id)) // One shared pin/unpin path: telemetry only fires after the mutation // settles (so a rejection never records a phantom action), and a failure @@ -135,6 +137,7 @@ export const ChatSkillsBar = ({ { // `move` comes from dnd-kit's `active.id` / index lookup — unambiguous @@ -162,7 +165,7 @@ export const ChatSkillsBar = ({ // Pinnable = enabled and not already pinned. The popover only ever lists // pin candidates, never a dual "pin / unpin" surface — unpin lives on the // chip's own dropdown. - const pinnable = library.filter((s) => isEnabled(s.id) && !pinnedSet.has(s.id)) + const pinnable = library.filter((s) => !isWidgetSkillId(s.id) && isEnabled(s.id) && !pinnedSet.has(s.id)) const query = addQuery.trim() const pinnableFiltered = pinnable.filter((s) => skillMatchesQuery(s, query)) const pinCapReached = pinnedSet.size >= maxPinnedSkills @@ -304,12 +307,16 @@ export const ChatSkillsBar = ({ label={skillDisplayName(skill)} onClick={() => onAddToChat(skill)} onAddInstruction={() => onAddInstruction(skill.instruction)} - onEdit={() => openCreateItem({ kind: 'skill', skillId: skill.id })} - onReorder={() => { - void loadReorderPanel() - dispatch({ type: 'REORDER_OPENED' }) - }} - onUnpin={() => handleTogglePin(skill, 'unpin')} + onEdit={isWidgetSkillId(skill.id) ? undefined : () => openCreateItem({ kind: 'skill', skillId: skill.id })} + onReorder={ + isWidgetSkillId(skill.id) + ? undefined + : () => { + void loadReorderPanel() + dispatch({ type: 'REORDER_OPENED' }) + } + } + onUnpin={isWidgetSkillId(skill.id) ? undefined : () => handleTogglePin(skill, 'unpin')} /> ))} {addControl} diff --git a/src/dal/skills.test.ts b/src/dal/skills.test.ts index 193b30246..62e9c248e 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 { defaultSkillDailyBrief, defaultSkillWeather } 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(defaultSkillWeather) + return defaultSkillWeather +} + describe('validateSkillName (AgentSkills spec)', () => { it('accepts canonical slugs', () => { expect(validateSkillName('meeting-notes')).toBeNull() @@ -191,6 +199,36 @@ describe('skills DAL', () => { const a = await seed({ name: 'legit-name' }) await expect(updateSkill(getDb(), a.id, { name: 'SHOUTING' })).rejects.toBeInstanceOf(SkillNameInvalidError) }) + + it('allows enabled patches for widget skills', async () => { + const widget = await seedWidgetSkill() + + await updateSkill(getDb(), widget.id, { enabled: 0 }) + + const after = await getSkill(getDb(), widget.id) + expect(after?.enabled).toBe(0) + expect(after?.pinnedOrder).toBe(defaultSkillWeather.pinnedOrder) + }) + + it('rejects locked-field patches for widget skills', async () => { + const widget = await seedWidgetSkill() + const disallowedPatches: UpdateSkillInput[] = [ + { name: 'renamed-widget' }, + { label: 'Renamed Widget' }, + { description: 'Changed description' }, + { instruction: 'Changed instruction' }, + { pinnedOrder: 2 }, + { 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(defaultSkillWeather.description) + }) }) describe('softDeleteSkill', () => { @@ -224,9 +262,34 @@ 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', () => { + it('rejects pinning widget skills but allows unpinning them', async () => { + const widget = await seedWidgetSkill() + + await expect(setPinned(getDb(), widget.id, 0)).rejects.toThrow(/widget skill/i) + await setPinned(getDb(), widget.id, null) + + expect((await getSkill(getDb(), widget.id))?.pinnedOrder).toBeNull() + }) + + it('pins task skills', async () => { + const task = { ...defaultSkillDailyBrief, pinnedOrder: null } + await getDb().insert(skillsTable).values(task) + + await setPinned(getDb(), task.id, 0) + + expect((await getSkill(getDb(), task.id))?.pinnedOrder).toBe(0) + }) + it('pins and unpins', async () => { const skill = await seed({ name: 'p' }) await setPinned(getDb(), skill.id, 0) @@ -286,6 +349,80 @@ describe('skills DAL', () => { const ids = Array.from({ length: maxPinnedSkills + 1 }, () => crypto.randomUUID()) await expect(reorderPins(getDb(), ids)).rejects.toBeInstanceOf(PinLimitExceededError) }) + + it('permits reordering other pins before a stationary widget when stored orders have gaps', async () => { + const a = await seed({ name: 'a' }) + const b = await seed({ name: 'b' }) + await setPinned(getDb(), a.id, 0) + await setPinned(getDb(), b.id, 1) + await getDb() + .insert(skillsTable) + .values({ ...defaultSkillWeather, pinnedOrder: 4 }) + + await reorderPins(getDb(), [b.id, a.id, defaultSkillWeather.id]) + + const pinned = await getPinnedSkills(getDb()) + expect(pinned.map((skill) => skill.id)).toEqual([b.id, a.id, defaultSkillWeather.id]) + }) + + it('rejects moving a pinned widget relative to other pins when stored orders have gaps', async () => { + const a = await seed({ name: 'a' }) + const b = await seed({ name: 'b' }) + await setPinned(getDb(), a.id, 0) + await setPinned(getDb(), b.id, 1) + await getDb() + .insert(skillsTable) + .values({ ...defaultSkillWeather, pinnedOrder: 4 }) + + await expect(reorderPins(getDb(), [a.id, defaultSkillWeather.id, b.id])).rejects.toThrow(/widget skill/i) + + const pinned = await getPinnedSkills(getDb()) + expect(pinned.map(({ id, pinnedOrder }) => ({ id, pinnedOrder }))).toEqual([ + { id: a.id, pinnedOrder: 0 }, + { id: b.id, pinnedOrder: 1 }, + { id: defaultSkillWeather.id, pinnedOrder: 4 }, + ]) + }) + + it('rejects adding an unpinned widget to the submitted pin order', async () => { + const a = await seed({ name: 'a' }) + await setPinned(getDb(), a.id, 0) + await getDb() + .insert(skillsTable) + .values({ ...defaultSkillWeather, pinnedOrder: null }) + + await expect(reorderPins(getDb(), [a.id, defaultSkillWeather.id])).rejects.toThrow(/widget skill/i) + + expect((await getSkill(getDb(), defaultSkillWeather.id))?.pinnedOrder).toBeNull() + }) + + it('rejects an order that omits a pinned widget and moves another pin across it', async () => { + const a = await seed({ name: 'a' }) + const b = await seed({ name: 'b' }) + await setPinned(getDb(), a.id, 0) + await setPinned(getDb(), b.id, 4) + await getDb().insert(skillsTable).values(defaultSkillWeather) + + await expect(reorderPins(getDb(), [b.id, a.id])).rejects.toThrow(/widget skill/i) + + const pinned = await getPinnedSkills(getDb()) + expect(pinned.map((skill) => skill.id)).toEqual([a.id, defaultSkillWeather.id, b.id]) + }) + + it('allows an order that omits a pinned widget when submitted pins stay on the same side', async () => { + const a = await seed({ name: 'a' }) + const b = await seed({ name: 'b' }) + await setPinned(getDb(), a.id, 0) + await setPinned(getDb(), b.id, 1) + await getDb() + .insert(skillsTable) + .values({ ...defaultSkillWeather, pinnedOrder: 4 }) + + await reorderPins(getDb(), [b.id, a.id]) + + const pinned = await getPinnedSkills(getDb()) + expect(pinned.map((skill) => skill.id)).toEqual([b.id, a.id, defaultSkillWeather.id]) + }) }) describe('getSkillsByIds', () => { diff --git a/src/dal/skills.ts b/src/dal/skills.ts index 25a904c06..a9b273384 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,21 @@ 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 any patch other + * than `enabled`. */ export const updateSkill = async (db: AnyDrizzleDatabase, id: string, patch: UpdateSkillInput): Promise => { + const changesLockedWidgetField = Object.keys(patch).some((field) => field !== 'enabled') + if (isWidgetSkillId(id) && changesLockedWidgetField) { + throw new Error(`updateSkill: widget skill "${id}" only supports enabled updates`) + } if (patch.name !== undefined) { const slugError = validateSkillName(patch.name) if (slugError) { @@ -206,9 +214,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({ @@ -224,9 +236,13 @@ export const softDeleteSkill = async (db: AnyDrizzleDatabase, id: string): Promi /** * Pin or unpin a skill. Pass `null` to unpin. Pass a number to set the pin position. - * Throws {@link PinLimitExceededError} if pinning would exceed {@link maxPinnedSkills}. + * Throws `Error` when pinning a widget contract or {@link PinLimitExceededError} + * if pinning would exceed {@link maxPinnedSkills}. Widget contracts may be unpinned. */ export const setPinned = async (db: AnyDrizzleDatabase, id: string, order: number | null): Promise => { + if (order !== null && isWidgetSkillId(id)) { + throw new Error(`setPinned: refusing to pin widget skill "${id}"`) + } if (order !== null) { const pinned = await countPinned(db, id) if (pinned >= maxPinnedSkills) { @@ -236,7 +252,7 @@ export const setPinned = async (db: AnyDrizzleDatabase, id: string, order: numbe await db.update(skillsTable).set({ pinnedOrder: order }).where(eq(skillsTable.id, id)) } -/** Toggle the `enabled` flag. SkillsView auto-unpins on disable as a side-effect at the call site. */ +/** Toggle `enabled`. SkillsView auto-unpins pinned skills on disable at the call site. */ export const setEnabled = async (db: AnyDrizzleDatabase, id: string, next: boolean): Promise => { await db .update(skillsTable) @@ -246,7 +262,8 @@ export const setEnabled = async (db: AnyDrizzleDatabase, id: string, next: boole /** * Rewrite the `pinned_order` of the supplied ids in a single transaction (index = position). - * Ids not in the list keep their existing order. Bounded by the 10-pin cap. + * Ids not in the list keep their existing order. Widget ids must keep their + * positions relative to the other pinned skills. Bounded by the 10-pin cap. */ export const reorderPins = async (db: AnyDrizzleDatabase, ids: string[]): Promise => { if (ids.length === 0) { @@ -256,6 +273,30 @@ export const reorderPins = async (db: AnyDrizzleDatabase, ids: string[]): Promis throw new PinLimitExceededError() } await db.transaction(async (tx) => { + const storedPinned = await tx + .select({ id: skillsTable.id, pinnedOrder: skillsTable.pinnedOrder }) + .from(skillsTable) + .where(and(isNull(skillsTable.deletedAt), isNotNull(skillsTable.pinnedOrder))) + .orderBy(asc(skillsTable.pinnedOrder)) + const storedPinnedIds = new Set(storedPinned.map(({ id }) => id)) + const submittedIndexById = new Map(ids.map((id, index) => [id, index])) + const pinsWidgetSkill = ids.some((id) => isWidgetSkillId(id) && !storedPinnedIds.has(id)) + const movesWidgetSkill = storedPinned.some((widget, widgetStoredIndex) => { + if (!isWidgetSkillId(widget.id)) { + return false + } + const widgetFinalOrder = submittedIndexById.get(widget.id) ?? widget.pinnedOrder! + return storedPinned.some((skill, skillStoredIndex) => { + if (skill.id === widget.id) { + return false + } + const skillFinalOrder = submittedIndexById.get(skill.id) ?? skill.pinnedOrder! + return Math.sign(skillStoredIndex - widgetStoredIndex) !== Math.sign(skillFinalOrder - widgetFinalOrder) + }) + }) + if (pinsWidgetSkill || movesWidgetSkill) { + throw new Error('reorderPins: refusing to move widget skills') + } // Two-phase update to avoid hitting the (id, pinned_order) collision space // mid-rewrite: stage everything to negative ordinals first, then settle. for (let i = 0; i < ids.length; i++) { diff --git a/src/defaults/skills.test.ts b/src/defaults/skills.test.ts index d741f3ad3..9cf754622 100644 --- a/src/defaults/skills.test.ts +++ b/src/defaults/skills.test.ts @@ -4,14 +4,25 @@ import { describe, expect, it, test } from 'bun:test' +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 { + defaultSkillDailyBrief, defaultSkillImportantEmails, + defaultSkillAsk, + defaultSkillConnectIntegration, + defaultSkillLinkPreview, + defaultSkillMap, defaultSkillResearch, defaultSkills, defaultSkillSearch, defaultSkillsVersion, defaultSkillWeather, hashSkill, + isWidgetSkillId, } from './skills' /** @@ -31,8 +42,8 @@ const computeSnapshotHash = () => defaultSkills.map((skill, index) => `${index}:${skill.id}:${hashSkill(skill)}`).join('|') const expectedSnapshot = { - version: 4, - hash: '0:01996330-0000-7000-8000-000000000001:mfmi05|1:01996330-0000-7000-8000-000000000002:-669lkj|2:01996330-0000-7000-8000-000000000003:-30vmih|3:01996330-0000-7000-8000-000000000004:-cz2tdq|4:01996330-0000-7000-8000-000000000005:-d0466u', + version: 5, + hash: '0:01996330-0000-7000-8000-000000000001:mfmi05|1:01996330-0000-7000-8000-000000000002:-669lkj|2:01996330-0000-7000-8000-000000000003:-30vmih|3:01996330-0000-7000-8000-000000000004:-cz2tdq|4:01996330-0000-7000-8000-000000000005:-hc6mv3|5:01996330-0000-7000-8000-000000000006:-o0c0ul|6:01996330-0000-7000-8000-000000000007:atrnpq|7:01996330-0000-7000-8000-000000000008:ejr8vn|8:01996330-0000-7000-8000-000000000009:o1nire', } describe('defaultSkills version snapshot', () => { @@ -45,16 +56,85 @@ describe('defaultSkills version snapshot', () => { }) describe('defaultSkills', () => { + it('ships spontaneous widget skills with load-bearing descriptions and canonical instruction bodies', () => { + const widgetSkills = [ + { + skill: defaultSkillWeather, + description: 'Use this skill when the user asks about the weather or wants a forecast for a location.', + 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('pins exactly Search, Research, Weather (in that order) as the starter chips for new users', () => { - // Regression guard — Chris chose this exact pinned starter set. Pinning is - // manageable only from the chat composer, so the seed decides what new - // users see in the chip bar. const pinned = defaultSkills - .filter((s) => s.pinnedOrder !== null) + .filter((skill) => skill.pinnedOrder !== null) .sort((a, b) => (a.pinnedOrder ?? 0) - (b.pinnedOrder ?? 0)) expect(pinned).toEqual([defaultSkillSearch, defaultSkillResearch, defaultSkillWeather]) }) + it('identifies widget contracts by stable default id', () => { + for (const skill of [ + defaultSkillWeather, + defaultSkillLinkPreview, + defaultSkillConnectIntegration, + defaultSkillAsk, + defaultSkillMap, + ]) { + expect(isWidgetSkillId(skill.id)).toBe(true) + } + + expect(isWidgetSkillId(defaultSkillDailyBrief.id)).toBe(false) + expect(isWidgetSkillId(defaultSkillImportantEmails.id)).toBe(false) + expect(isWidgetSkillId(defaultSkillSearch.id)).toBe(false) + expect(isWidgetSkillId(defaultSkillResearch.id)).toBe(false) + expect(isWidgetSkillId('user-skill-id')).toBe(false) + }) + + it('excludes user-controlled state from widget hashes only', () => { + expect(hashSkill({ ...defaultSkillWeather, enabled: 0, pinnedOrder: 4 })).toBe(hashSkill(defaultSkillWeather)) + expect(hashSkill({ ...defaultSkillDailyBrief, enabled: 0, pinnedOrder: 4 })).not.toBe( + hashSkill(defaultSkillDailyBrief), + ) + }) + it('seeds Important Emails disabled and everything else enabled', () => { for (const skill of defaultSkills) { expect(skill.enabled).toBe(skill === defaultSkillImportantEmails ? 0 : 1) diff --git a/src/defaults/skills.ts b/src/defaults/skills.ts index 6f656cffc..f14a17f22 100644 --- a/src/defaults/skills.ts +++ b/src/defaults/skills.ts @@ -4,27 +4,32 @@ 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 - * 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. Widget contracts hash only their locked + * content so enabled or legacy pinned state cannot block contract updates. + * Editable task defaults also include state and `deletedAt`, so those user + * changes remain protected from reconciliation. * * Accepts raw (nullable) rows as well as `Skill` so the hash-restamp data * migration can stamp exactly what reconciliation will later recompute. */ export const hashSkill = ( - skill: Pick, -): string => - hashValues([ - skill.name, - skill.label, - skill.description, - skill.instruction, - skill.enabled, - skill.pinnedOrder, - skill.deletedAt, - ]) + skill: Pick< + SkillRow, + 'id' | 'name' | 'label' | 'description' | 'instruction' | 'enabled' | 'pinnedOrder' | 'deletedAt' + >, +): string => { + const contentFields = [skill.name, skill.label, skill.description, skill.instruction] + return hashValues( + isWidgetSkillId(skill.id) ? contentFields : [...contentFields, skill.enabled, skill.pinnedOrder, skill.deletedAt], + ) +} const dailyBriefInstruction = `Create a daily brief with the following sections. Do not ask the user for any missing information — just skip sections for which you are missing information or tools. @@ -83,13 +88,6 @@ CRITICAL QUALITY RULES: Do NOT answer questions directly. Do NOT write paragraphs. Just search and show links.` -const weatherInstruction = `Show the user the weather forecast. - -1. Determine the location: use the location from the user's message if given, otherwise use the user's known location. If you have neither, ask. -2. Render the forecast widget: - -The widget fetches data automatically — do NOT search the web for weather data. Add at most one short sentence of context; the widget carries the content.` - /** Former "Research" chat mode, now shipped as a default skill (`/research`). */ const researchInstruction = `You are **Deep Research**. The user wants EXHAUSTIVE research, not a quick answer. @@ -130,7 +128,9 @@ Do not add a Sources or References section at the end — inline [N] citations a * * New users get Search, Research, and Weather pinned (in that order) as their * starter chips; Daily Brief ships enabled but unpinned, and Important Emails - * ships disabled. A user who soft-deletes one will not see it re-seeded. + * ships disabled. Additional model-facing widget contracts ship enabled and + * unpinned. Task skills may be edited or soft-deleted; widget contracts only + * expose enabled state. */ export const defaultSkillDailyBrief: Skill = { id: '01996330-0000-7000-8000-000000000001', @@ -193,7 +193,7 @@ export const defaultSkillWeather: Skill = { name: 'weather', label: 'Weather', description: 'Use this skill when the user asks about the weather or wants a forecast for a location.', - instruction: weatherInstruction, + instruction: weatherForecastWidgetInstruction, enabled: 1, pinnedOrder: 2, deletedAt: null, @@ -201,12 +201,82 @@ export const defaultSkillWeather: Skill = { userId: null, } +export const defaultSkillLinkPreview: Skill = { + id: '01996330-0000-7000-8000-000000000006', + 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: null, + deletedAt: null, + defaultHash: null, + userId: null, +} + +export const defaultSkillConnectIntegration: Skill = { + id: '01996330-0000-7000-8000-000000000007', + 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: null, + deletedAt: null, + defaultHash: null, + userId: null, +} + +export const defaultSkillAsk: Skill = { + id: '01996330-0000-7000-8000-000000000008', + 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: null, + deletedAt: null, + defaultHash: null, + userId: null, +} + +export const defaultSkillMap: Skill = { + id: '01996330-0000-7000-8000-000000000009', + 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: null, + deletedAt: null, + defaultHash: null, + userId: null, +} + +const widgetSkillIds = new Set([ + defaultSkillWeather.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, defaultSkillSearch, defaultSkillResearch, defaultSkillWeather, + defaultSkillLinkPreview, + defaultSkillConnectIntegration, + defaultSkillAsk, + defaultSkillMap, ] as const /** @@ -220,4 +290,4 @@ export const defaultSkills: ReadonlyArray = [ * 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 = 4 +export const defaultSkillsVersion = 5 diff --git a/src/lib/data-migrations/index.ts b/src/lib/data-migrations/index.ts index 5aa740ad4..47f560429 100644 --- a/src/lib/data-migrations/index.ts +++ b/src/lib/data-migrations/index.ts @@ -5,6 +5,7 @@ import type { AnyDrizzleDatabase } from '@/db/database-interface' import { automationsToSkills } from './automations-to-skills' import { restampSkillDefaultHashes } from './restamp-skill-default-hashes' +import { restampWidgetSkillDefaultHashes } from './restamp-widget-skill-default-hashes' /** * Runtime data migrations. @@ -38,7 +39,11 @@ export type DataMigration = { run: (db: AnyDrizzleDatabase) => Promise } -const migrations: readonly DataMigration[] = [automationsToSkills, restampSkillDefaultHashes] as const +const migrations: readonly DataMigration[] = [ + automationsToSkills, + restampSkillDefaultHashes, + restampWidgetSkillDefaultHashes, +] as const /** * Run every registered migration in order. One migration failing logs and diff --git a/src/lib/data-migrations/restamp-widget-skill-default-hashes.test.ts b/src/lib/data-migrations/restamp-widget-skill-default-hashes.test.ts new file mode 100644 index 000000000..caa601749 --- /dev/null +++ b/src/lib/data-migrations/restamp-widget-skill-default-hashes.test.ts @@ -0,0 +1,80 @@ +/* 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 { resetTestDatabase, setupTestDatabase, teardownTestDatabase } from '@/dal/test-utils' +import { getDb } from '@/db/database' +import { skillsTable } from '@/db/tables' +import { defaultSkillDailyBrief, defaultSkillWeather, hashSkill } from '@/defaults/skills' +import { hashValues } from '@/lib/utils' +import type { Skill } from '@/types' +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'bun:test' +import { eq } from 'drizzle-orm' + +import { restampWidgetSkillDefaultHashes } from './restamp-widget-skill-default-hashes' + +/** Hash formula used before widget contracts moved to content-only hashes. */ +const legacyHashSkill = (skill: Skill): string => + hashValues([ + skill.name, + skill.label, + skill.description, + skill.instruction, + skill.enabled, + skill.pinnedOrder, + skill.deletedAt, + ]) + +beforeAll(async () => { + await setupTestDatabase() +}) + +afterAll(async () => { + await teardownTestDatabase() +}) + +beforeEach(async () => { + await resetTestDatabase() +}) + +describe('restampWidgetSkillDefaultHashes', () => { + it('re-stamps toggled widget rows without changing user state', async () => { + const toggledWidget = { + ...defaultSkillWeather, + enabled: 0, + pinnedOrder: 4, + defaultHash: legacyHashSkill(defaultSkillWeather), + } + await getDb().insert(skillsTable).values(toggledWidget) + + await restampWidgetSkillDefaultHashes.run(getDb()) + + const row = await getDb().select().from(skillsTable).where(eq(skillsTable.id, toggledWidget.id)).get() + expect(row?.defaultHash).toBe(hashSkill(toggledWidget)) + expect(row?.enabled).toBe(0) + expect(row?.pinnedOrder).toBe(4) + }) + + it('leaves task-skill hashes untouched', async () => { + const staleHash = legacyHashSkill(defaultSkillDailyBrief) + const toggledTask = { ...defaultSkillDailyBrief, enabled: 0, defaultHash: staleHash } + await getDb().insert(skillsTable).values(toggledTask) + + await restampWidgetSkillDefaultHashes.run(getDb()) + + const row = await getDb().select().from(skillsTable).where(eq(skillsTable.id, toggledTask.id)).get() + expect(row?.defaultHash).toBe(staleHash) + }) + + it('is idempotent for widget rows already using the content-only hash', async () => { + const currentHash = hashSkill(defaultSkillWeather) + await getDb() + .insert(skillsTable) + .values({ ...defaultSkillWeather, defaultHash: currentHash }) + + await restampWidgetSkillDefaultHashes.run(getDb()) + + const row = await getDb().select().from(skillsTable).where(eq(skillsTable.id, defaultSkillWeather.id)).get() + expect(row?.defaultHash).toBe(currentHash) + }) +}) diff --git a/src/lib/data-migrations/restamp-widget-skill-default-hashes.ts b/src/lib/data-migrations/restamp-widget-skill-default-hashes.ts new file mode 100644 index 000000000..8af9cf43e --- /dev/null +++ b/src/lib/data-migrations/restamp-widget-skill-default-hashes.ts @@ -0,0 +1,43 @@ +/* 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 { skillsTable } from '@/db/tables' +import { hashSkill, isWidgetSkillId } from '@/defaults/skills' +import { eq, isNotNull } from 'drizzle-orm' +import type { DataMigration } from './index' + +/** + * Re-stamp widget contracts from the former full-row hash to the content-only + * hash. Widget enabled state and any legacy pinned state must survive contract + * updates, so every known widget row with a default hash can safely adopt the + * new formula even when that state no longer matches its shipped default. + * Task-skill hashes remain untouched because their state changes still + * represent user edits. + * + * Idempotent: rows already stamped with the content-only hash are skipped. + * + * Reconciliation invokes this migration immediately before its skills pass so + * shipped full-row hashes are recognized on the same boot as a contract + * update. The normal post-reconcile migration runner invokes it again as an + * idempotent no-op. + * + * DELETE ME: once telemetry shows the active population has upgraded past the + * first release containing defaults v5, this migration can be removed. + */ +export const restampWidgetSkillDefaultHashes: DataMigration = { + id: 'restamp-widget-skill-default-hashes', + run: async (db) => { + const rows = await db.select().from(skillsTable).where(isNotNull(skillsTable.defaultHash)) + for (const row of rows) { + if (!isWidgetSkillId(row.id)) { + continue + } + const defaultHash = hashSkill(row) + if (row.defaultHash === defaultHash) { + continue + } + await db.update(skillsTable).set({ defaultHash }).where(eq(skillsTable.id, row.id)) + } + }, +} diff --git a/src/lib/reconcile-defaults.test.ts b/src/lib/reconcile-defaults.test.ts index f9767f80a..432ac5340 100644 --- a/src/lib/reconcile-defaults.test.ts +++ b/src/lib/reconcile-defaults.test.ts @@ -20,10 +20,22 @@ import { type SharedModel, } from '@shared/defaults/models' import { defaultSettings, defaultSettingsVersion, hashSetting } from '../defaults/settings' -import { defaultSkills, defaultSkillsVersion, hashSkill } from '../defaults/skills' +import { + defaultSkillWeather, + defaultSkills, + defaultSkillsVersion, + hashSkill, + isWidgetSkillId, +} from '../defaults/skills' import { defaultTasks, defaultTasksVersion, hashTask } from '../defaults/tasks' +import { hashValues } from './utils' import type { ModelsDefaults } from './pick-defaults' -import { cleanupRemovedDefaults, reconcileDefaults, reconcileDefaultsForTable } from './reconcile-defaults' +import { + cleanupRemovedDefaults, + reconcileDefaults, + reconcileDefaultsForTable, + versionMarkerKeys, +} from './reconcile-defaults' import type { Model, ModelProfile, Prompt } from '@/types' /** A model id no current default uses — stands in for any retired default. */ @@ -1125,16 +1137,14 @@ describe('reconcileDefaults version gate (THU-637)', () => { expect(await readStoredSkillsVersion()).toBe(defaultSkillsVersion) }) - test('runGatedPass does not advance marker when every skill row is user-edited', async () => { + test('runGatedPass does not advance marker when every editable skill row is user-edited', async () => { const db = getDb() await reconcileDefaults(db) - // Every skills row user-edited (hash mismatch) and marker rewound so the - // version gate opens. Reconcile skips every row → `mutated=false` AND - // `everyBundleRowAtTarget=false` (hash-mismatch branch). Marker must - // stay at the rewound value — mirrors the existing models-path - // regression guard for the runGatedPass loop. - for (const skill of defaultSkills) { + // Every editable skill row is user-edited (hash mismatch) and the marker + // is rewound. Locked widget contracts remain at target. Reconcile skips + // each editable row, so `mutated=false` and the marker stays rewound. + for (const skill of defaultSkills.filter((defaultSkill) => !isWidgetSkillId(defaultSkill.id))) { await db .update(skillsTable) .set({ label: `user-edited ${skill.id}` }) @@ -1460,6 +1470,68 @@ describe('reconcileDefaults version gate (THU-637)', () => { }) }) +describe('widget skill reconciliation', () => { + test('upgrades shipped v4 weather to canonical widget instructions on the first v5 pass', async () => { + const db = getDb() + const shippedV4Weather = { + ...defaultSkillWeather, + instruction: 'Shipped v4 inline weather instruction', + } + await db.insert(skillsTable).values({ + ...shippedV4Weather, + defaultHash: hashValues([ + shippedV4Weather.name, + shippedV4Weather.label, + shippedV4Weather.description, + shippedV4Weather.instruction, + shippedV4Weather.enabled, + shippedV4Weather.pinnedOrder, + shippedV4Weather.deletedAt, + ]), + }) + await db.insert(settingsTable).values({ + key: versionMarkerKeys.skills, + value: String(defaultSkillsVersion - 1), + }) + + await reconcileDefaults(db) + + const updated = await db.select().from(skillsTable).where(eq(skillsTable.id, defaultSkillWeather.id)).get() + const marker = await db.select().from(settingsTable).where(eq(settingsTable.key, versionMarkerKeys.skills)).get() + expect(updated?.instruction).toBe(defaultSkillWeather.instruction) + expect(updated?.pinnedOrder).toBe(defaultSkillWeather.pinnedOrder) + expect(marker?.value).toBe(String(defaultSkillsVersion)) + }) + + test('newer defaults update contract content while preserving enabled and pinned state', async () => { + const db = getDb() + await reconcileDefaults(db) + + const staleWidget = { ...defaultSkillWeather, description: 'stale widget contract' } + await db + .update(skillsTable) + .set({ + description: staleWidget.description, + enabled: 0, + pinnedOrder: 4, + defaultHash: hashSkill(staleWidget), + }) + .where(eq(skillsTable.id, staleWidget.id)) + await db + .update(settingsTable) + .set({ value: String(defaultSkillsVersion - 1) }) + .where(eq(settingsTable.key, versionMarkerKeys.skills)) + + await reconcileDefaults(db) + + const updated = await db.select().from(skillsTable).where(eq(skillsTable.id, staleWidget.id)).get() + expect(updated?.description).toBe(defaultSkillWeather.description) + expect(updated?.enabled).toBe(0) + expect(updated?.pinnedOrder).toBe(4) + expect(updated?.defaultHash).toBe(hashSkill({ ...defaultSkillWeather, enabled: 0, pinnedOrder: 4 })) + }) +}) + /** * THU-677 extends the THU-637 version-gate pattern from models to every * other reconciled table (modes, tasks, skills, settings). The scenarios diff --git a/src/lib/reconcile-defaults.ts b/src/lib/reconcile-defaults.ts index 0eba8502c..9150da3f3 100644 --- a/src/lib/reconcile-defaults.ts +++ b/src/lib/reconcile-defaults.ts @@ -12,9 +12,10 @@ import { modelProfilesTable, modelsTable, settingsTable, skillsTable, tasksTable import { defaultModelProfiles, hashModelProfile } from '../defaults/model-profiles' import { defaultModels, defaultModelsVersion, hashModel, type SharedModel } from '@shared/defaults/models' import { defaultSettings, defaultSettingsVersion, hashSetting } from '../defaults/settings' -import { defaultSkills, defaultSkillsVersion, hashSkill } from '../defaults/skills' +import { defaultSkills, defaultSkillsVersion, hashSkill, isWidgetSkillId } from '../defaults/skills' import { defaultTasks, defaultTasksVersion, hashTask } from '../defaults/tasks' import type { ModelsDefaults } from './pick-defaults' +import { restampWidgetSkillDefaultHashes } from './data-migrations/restamp-widget-skill-default-hashes' import { nowIso } from './utils' const bundledModelsDefaults: ModelsDefaults = { version: defaultModelsVersion, data: defaultModels } @@ -142,25 +143,27 @@ const advanceVersionMarker = async ( * partial delivery of an authoritative retirement. Defaults to * `canOverwrite` for callers that don't split the two signals. * @property frozenFields - Field names that must never change on an existing - * row via reconcile. When updating, the existing row's value is kept for - * each listed field and the stored `defaultHash` reflects that - * post-freeze state. Protects identity-critical columns whose values - * establish downstream contracts — e.g. `isConfidential` on models - * (encrypted threads bind to it at creation) and `provider` (routing). - * A server-shipped OTA payload cannot flip these on a bundle-known id; - * a new value ships under a fresh id. Only applies to updates — inserts - * use the default as-is. + * row via reconcile, or a selector returning those names per default item. + * When updating, the existing row's value is kept for each listed field and + * the stored `defaultHash` reflects that post-freeze state. Protects + * identity-critical columns whose values establish downstream contracts — + * e.g. `isConfidential` on models (encrypted threads bind to it at creation) + * and `provider` (routing). A server-shipped OTA payload cannot flip these + * on a bundle-known id; a new value ships under a fresh id. Only applies to + * updates — inserts use the default as-is. * @property metadataFields - Server-owned fields intentionally excluded from * the user-edit hash. Differences in these fields still trigger an update * after the row's hashed fields are verified as unmodified. */ -export type ReconcileDefaultsForTableOptions = { +type FrozenField = Extract + +export type ReconcileDefaultsForTableOptions = { keyField?: string canOverwrite?: boolean insertMissing?: boolean canResurrect?: boolean - frozenFields?: readonly string[] - metadataFields?: readonly string[] + frozenFields?: readonly FrozenField[] | ((defaultItem: T) => readonly FrozenField[]) + metadataFields?: readonly FrozenField[] } /** @@ -197,7 +200,7 @@ export const reconcileDefaultsForTable = async , defaults: readonly T[], hashFn: (item: any) => string, - options: ReconcileDefaultsForTableOptions = {}, + options: ReconcileDefaultsForTableOptions = {}, ): Promise => { const { keyField = 'id', @@ -305,11 +308,15 @@ export const reconcileDefaultsForTable = async ((acc, field) => ({ ...acc, [field]: (existing as any)[field] }), defaultItem) as T) - const effectiveHash = frozenFields.length === 0 ? hashFn(defaultItem) : hashFn(effectiveDefault) + : (itemFrozenFields.reduce( + (acc, field) => ({ ...acc, [field]: (existing as any)[field] }), + defaultItem, + ) as T) + const effectiveHash = itemFrozenFields.length === 0 ? hashFn(defaultItem) : hashFn(effectiveDefault) const metadataChanged = metadataFields.some( (field) => (existing as Record)[field] !== (effectiveDefault as Record)[field], ) @@ -598,13 +605,13 @@ export const reconcileDefaults = async (db: AnyDrizzleDatabase, overrides?: Reco versionKey: string, currentVersion: number, hasAnyRow: boolean, - keyField?: string, + reconcileOptions: Pick, 'keyField' | 'frozenFields'> = {}, ): Promise => { const gate = await computeCanOverwrite(tx, versionKey, currentVersion, hasAnyRow, initialSyncCompleted) const pass = await reconcileDefaultsForTable(tx, table, defaults, hashFn, { canOverwrite: gate.canOverwrite, canResurrect: initialSyncCompleted, - ...(keyField ? { keyField } : {}), + ...reconcileOptions, }) // Advance when we wrote something OR verified every row is at target. // See the models-path guard above for the full rationale — single-table @@ -615,6 +622,10 @@ export const reconcileDefaults = async (db: AnyDrizzleDatabase, overrides?: Reco } await runGatedPass(tasksTable, defaultTasks, hashTask, versionMarkerKeys.tasks, defaultTasksVersion, hasAnyTaskRow) + // Main shipped weather with a full-row v4 hash before weather became a + // locked widget contract. Re-stamp first so v5 can recognize and replace + // that pristine row with the canonical widget instructions in this pass. + await restampWidgetSkillDefaultHashes.run(tx) await runGatedPass( skillsTable, defaultSkills, @@ -622,6 +633,7 @@ export const reconcileDefaults = async (db: AnyDrizzleDatabase, overrides?: Reco versionMarkerKeys.skills, defaultSkillsVersion, hasAnySkillRow, + { frozenFields: (skill) => (isWidgetSkillId(skill.id) ? ['enabled', 'pinnedOrder'] : []) }, ) await runGatedPass( settingsTable, @@ -630,7 +642,7 @@ export const reconcileDefaults = async (db: AnyDrizzleDatabase, overrides?: Reco versionMarkerKeys.settings, defaultSettingsVersion, hasAnySettingsRow, - 'key', + { keyField: 'key' }, ) // Initialize anonymous ID for analytics (unique per user) diff --git a/src/skills/library-row.tsx b/src/skills/library-row.tsx index b40cf9ea9..2e8b1a830 100644 --- a/src/skills/library-row.tsx +++ b/src/skills/library-row.tsx @@ -8,6 +8,7 @@ import { SquarePen, Trash2 } from 'lucide-react' import { SettingsSelectableRow } from '@/components/settings/settings-list' 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' @@ -49,37 +50,43 @@ export const LibraryRow = ({ onEdit: (id: string) => void onDelete: (id: string) => void }) => { + const row = ( + onSelect(skill.id)} + ariaLabel={`Open ${skillDisplayName(skill)}`} + trailing={ + onToggleEnabled(skill.id, next)} + aria-label={`${enabled ? 'Disable' : 'Enable'} ${skillDisplayName(skill)}`} + /> + } + /> + ) + return ( - - - onSelect(skill.id)} - ariaLabel={`Open ${skillDisplayName(skill)}`} - trailing={ - 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/reorder-panel.tsx b/src/skills/reorder-panel.tsx index fa79d5183..ed162b021 100644 --- a/src/skills/reorder-panel.tsx +++ b/src/skills/reorder-panel.tsx @@ -27,8 +27,11 @@ import { verticalAxisModifiers } from '@/lib/dnd' import { cn } from '@/lib/utils' import { skillDisplayName } from './display' -const SortableRow = ({ skill }: { skill: Skill }) => { - const { attributes, listeners, setNodeRef, transform, transition, isDragging } = useSortable({ id: skill.id }) +const SortableRow = ({ skill, locked }: { skill: Skill; locked: boolean }) => { + const { attributes, listeners, setNodeRef, transform, transition, isDragging } = useSortable({ + id: skill.id, + disabled: locked, + }) const style = { transform: CSS.Transform.toString(transform), transition, @@ -39,11 +42,11 @@ const SortableRow = ({ skill }: { skill: Skill }) => { style={style} {...attributes} {...listeners} - className={`flex h-9 touch-none cursor-grab items-center gap-2 rounded-md px-2 active:cursor-grabbing ${ - isDragging ? 'opacity-40' : 'hover:bg-accent' - }`} + className={`flex h-9 touch-none items-center gap-2 rounded-md px-2 ${ + locked ? 'cursor-default' : 'cursor-grab active:cursor-grabbing' + } ${isDragging ? 'opacity-40' : 'hover:bg-accent'}`} > - + {skillDisplayName(skill)} ) @@ -57,7 +60,8 @@ export type ReorderMove = { id: string; from: number; to: number } * entire new order is reported to the parent along with the moved item's * id and indices (sourced from dnd-kit's `active.id` — the ground truth, * unambiguous even for adjacent swaps). Caller commits via `reorderPins(ids)` - * (single transaction in the DAL). + * (single transaction in the DAL). Locked rows stay fixed and cannot be + * crossed by a drag. * * TouchSensor uses a 120ms delay so vertical page-scroll still works when * the user isn't actually dragging. @@ -67,11 +71,13 @@ export const ReorderPanel = ({ onReorder, onClose, embedded = false, + lockedIds = new Set(), }: { pinned: Skill[] onReorder: (ids: string[], move: ReorderMove) => void onClose: () => void embedded?: boolean + lockedIds?: ReadonlySet }) => { const sensors = useSensors( useSensor(PointerSensor, { activationConstraint: { distance: 4 } }), @@ -89,6 +95,12 @@ export const ReorderPanel = ({ if (oldIndex < 0 || newIndex < 0) { return } + const crossesLockedSkill = pinned + .slice(Math.min(oldIndex, newIndex), Math.max(oldIndex, newIndex) + 1) + .some((skill) => lockedIds.has(skill.id)) + if (crossesLockedSkill) { + return + } const next = arrayMove(pinned, oldIndex, newIndex) onReorder( next.map((s) => s.id), @@ -125,7 +137,7 @@ export const ReorderPanel = ({ > s.id)} strategy={verticalListSortingStrategy}> {pinned.map((skill) => ( - + ))} diff --git a/src/skills/skill-detail.tsx b/src/skills/skill-detail.tsx index 975bbc1b5..a3f0074f5 100644 --- a/src/skills/skill-detail.tsx +++ b/src/skills/skill-detail.tsx @@ -11,12 +11,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, @@ -25,19 +26,25 @@ export const SkillDetail = ({ name: string description: string instruction: string + readOnly: boolean onEdit: () => void onDelete: () => void /** Close (X) — dismisses the desktop slide-in panel or the mobile overlay. */ onClose: () => void }) => { - const actionsMenu = ( + const actionsMenu = !readOnly && ( ) return ( - +
Description 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 + }, + }) diff --git a/src/skills/skills-view-state.test.ts b/src/skills/skills-view-state.test.ts index 23342ae1c..0aca632ad 100644 --- a/src/skills/skills-view-state.test.ts +++ b/src/skills/skills-view-state.test.ts @@ -3,6 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import { describe, expect, it } from 'bun:test' +import { defaultSkillDailyBrief, defaultSkillWeather } from '@/defaults/skills' import type { Skill } from '@/types' import { initialSkillsViewState, @@ -51,6 +52,21 @@ describe('skillsViewReducer', () => { expect(next.activeId).toBe('b') }) + it('keeps widget skills read-only when an edit route selects them', () => { + const widget = skillsViewReducer(initialSkillsViewState, { + type: 'START_EDIT', + id: defaultSkillWeather.id, + }) + const task = skillsViewReducer(initialSkillsViewState, { + type: 'START_EDIT', + id: defaultSkillDailyBrief.id, + }) + + expect(widget.mode).toBe('detail') + expect(widget.activeId).toBe(defaultSkillWeather.id) + expect(task.mode).toBe('edit') + }) + it('START_CREATE bumps resetSignal so the form re-mounts on back-to-back opens', () => { const next = skillsViewReducer({ ...initialSkillsViewState, resetSignal: 4 }, { type: 'START_CREATE' }) expect(next.resetSignal).toBe(5) @@ -127,6 +143,17 @@ describe('skillsViewReducer', () => { expect(next.resetSignal).toBe(2) }) + it('an edit intent lands on read-only detail for a widget skill', () => { + const next = skillsViewReducer(initialSkillsViewState, { + type: 'PERFORM_LEAVE', + leave: { type: 'edit', id: defaultSkillWeather.id }, + }) + + expect(next.mode).toBe('detail') + expect(next.activeId).toBe(defaultSkillWeather.id) + expect(next.panelView).toBe('panel') + }) + it('a create intent lands in a blank create form', () => { const editing: SkillsViewState = { ...initialSkillsViewState, @@ -215,6 +242,18 @@ describe('skillsViewReducer', () => { expect(next.pendingDependents).toBeNull() }) + it('JUMP_TO_DEPENDENT opens read-only detail when the dependent is a widget skill', () => { + const open: SkillsViewState = { + ...initialSkillsViewState, + pendingDependents: { action: 'disable', skill: skill('a', 'foo'), dependents: [] }, + } + const next = skillsViewReducer(open, { type: 'JUMP_TO_DEPENDENT', id: defaultSkillWeather.id }) + + expect(next.mode).toBe('detail') + expect(next.activeId).toBe(defaultSkillWeather.id) + expect(next.pendingDependents).toBeNull() + }) + it('opens the panel so the edit form is visible even when jumping from a list-row action', () => { // The dependents dialog can open from the list (panelView 'list') on // both mobile and desktop — the jump must reveal the edit surface. diff --git a/src/skills/skills-view-state.ts b/src/skills/skills-view-state.ts index 2d86f43e1..4a642bed9 100644 --- a/src/skills/skills-view-state.ts +++ b/src/skills/skills-view-state.ts @@ -2,6 +2,7 @@ * 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 { isWidgetSkillId } from '@/defaults/skills' import type { Skill } from '@/types' import type { DependentsAction } from './dependents-dialog' @@ -61,6 +62,9 @@ export const initialSkillsViewState: SkillsViewState = { submitError: null, } +/** Resolve an edit request without exposing widget rendering contracts to forms. */ +const modeForSkillEdit = (id: string): Mode => (isWidgetSkillId(id) ? 'detail' : 'edit') + /** * Action type for the SkillsView state machine. Each action describes a * user-meaningful event (a click, a confirm, a successful mutation) — the @@ -73,7 +77,7 @@ export type SkillsViewAction = | { type: 'SELECT_SKILL'; id: string } /** User opened the create form. Side effect: panel slides in on mobile. */ | { type: 'START_CREATE' } - /** User opened the edit form for a specific skill. */ + /** User requested editing a skill. Widget contracts stay in read-only detail. */ | { type: 'START_EDIT'; id: string } /** Leave the form (confirmed) and apply the parked intent. */ | { type: 'PERFORM_LEAVE'; leave: LeaveIntent } @@ -90,7 +94,7 @@ export type SkillsViewAction = | { type: 'CLOSE_DELETE' } /** Close the dependents confirm dialog (cancelled or confirmed). */ | { type: 'CLOSE_DEPENDENTS' } - /** User clicked a row in the dependents dialog — jump to edit that skill. */ + /** User clicked a row in the dependents dialog — open that skill. */ | { type: 'JUMP_TO_DEPENDENT'; id: string } /** Form reports its dirty state changed. */ | { type: 'DIRTY_CHANGED'; dirty: boolean } @@ -123,14 +127,22 @@ export const skillsViewReducer = (state: SkillsViewState, action: SkillsViewActi } case 'START_EDIT': - return { ...state, mode: 'edit', activeId: action.id, slugError: null, submitError: null, panelView: 'panel' } + return { + ...state, + mode: modeForSkillEdit(action.id), + activeId: action.id, + slugError: null, + submitError: null, + panelView: 'panel', + } case 'PERFORM_LEAVE': { const { leave } = action const nextActiveId = leave.type === 'select' || leave.type === 'edit' ? leave.id : state.activeId - // `edit`/`create` land in a fresh form on the target; `cancel`/`select` - // land in detail. - const nextMode: Mode = leave.type === 'edit' || leave.type === 'create' ? leave.type : 'detail' + // Editable `edit` targets and `create` land in fresh forms. Widget edit + // targets, `cancel`, and `select` land in detail. + const nextMode: Mode = + leave.type === 'edit' ? modeForSkillEdit(leave.id) : leave.type === 'create' ? 'create' : 'detail' // `edit`/`create` need the panel open — they can be triggered from a // list-row action while panelView is still 'list'. Cancel returns to the // selected item's detail view on both mobile and desktop; closing the @@ -160,10 +172,8 @@ export const skillsViewReducer = (state: SkillsViewState, action: SkillsViewActi return { ...state, activeId: action.skill.id, pendingDelete: action.skill } case 'OPEN_DEPENDENTS': - // Opening the dependents dialog from inside an edit session can later - // trigger JUMP_TO_DEPENDENT, which starts a fresh edit on another skill. - // Reset `isDirty` and `slugError` now so the inherited edit-session state - // doesn't bleed into the new form. + // Opening from an edit session can later jump to another skill. Reset + // form state so it does not bleed into the next detail or edit surface. return { ...state, activeId: action.payload.skill.id, @@ -180,23 +190,18 @@ export const skillsViewReducer = (state: SkillsViewState, action: SkillsViewActi return { ...state, pendingDependents: null } case 'JUMP_TO_DEPENDENT': - // Fresh edit session on a different skill: clear `isDirty` and - // the form errors so a stale dirty flag from the prior form doesn't trigger - // a spurious discard-changes dialog on the new (untouched) form. - // SkillForm remounts via its `key` change, so the values themselves are - // already clean — this resets the parent's tracking state to match. + // Clear form state before opening the dependent. Editable skills get a + // fresh form; widget contracts stay in read-only detail. return { ...state, activeId: action.id, - mode: 'edit', + mode: modeForSkillEdit(action.id), pendingDependents: null, isDirty: false, slugError: null, submitError: null, // The dependents dialog can be opened from a list-row action while - // panelView is still 'list'; opening the panel here gives the edit - // form a surface to render on (full-screen overlay on mobile, the - // right-hand slide-in on desktop). + // panelView is still 'list'; opening the panel here reveals the target. panelView: 'panel', } diff --git a/src/skills/skills-view.test.tsx b/src/skills/skills-view.test.tsx index 888a8f5fb..5264d72c8 100644 --- a/src/skills/skills-view.test.tsx +++ b/src/skills/skills-view.test.tsx @@ -19,6 +19,7 @@ import '@/test-utils/framer-motion-mock' import { resetTestDatabase, setupTestDatabase, teardownTestDatabase } from '@/dal/test-utils' import { getDb } from '@/db/database' import { skillsTable } from '@/db/tables' +import { defaultSkillWeather } from '@/defaults/skills' import { renderWithReactivity, waitForElement } from '@/test-utils/powersync-reactivity-test' import { getClock } from '@/testing-library' import { SkillsView } from './skills-view' @@ -69,6 +70,30 @@ const flush = async () => { } describe('SkillsView state machine', () => { + describe('widget rendering contracts', () => { + it('offers enable/disable without edit or delete affordances', async () => { + await getDb().insert(skillsTable).values(defaultSkillWeather) + + renderWithReactivity(, { tables: ['skills'], wrapper: Wrapper }) + + const rowLabel = await waitForElement(() => screen.queryByText('Weather')) + fireEvent.contextMenu(rowLabel) + await flush() + expect(screen.queryByText('Edit')).not.toBeInTheDocument() + expect(screen.queryByText('Delete')).not.toBeInTheDocument() + + fireEvent.click(rowLabel) + await flush() + expect(screen.getByText('Built-in skill · Read-only')).toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'More' })).not.toBeInTheDocument() + + fireEvent.click(screen.getByRole('switch', { name: 'Disable Weather' })) + await flush() + const disabledWeather = await getSkill(getDb(), defaultSkillWeather.id) + expect(disabledWeather?.enabled).toBe(0) + }) + }) + describe('handleToggleEnabled — auto-unpin on disable', () => { it('unpins a pinned skill when its row switch is turned off', async () => { const skill = await createSkill(getDb(), { @@ -95,6 +120,20 @@ describe('SkillsView state machine', () => { expect(after?.pinnedOrder).toBeNull() }) + it('unpins a legacy-pinned widget skill when disabled', async () => { + await getDb().insert(skillsTable).values(defaultSkillWeather) + + renderWithReactivity(, { tables: ['skills'], wrapper: Wrapper }) + + const switchEl = await waitForElement(() => screen.queryByRole('switch', { name: /Disable Weather/ })) + fireEvent.click(switchEl) + await flush() + + const after = await getSkill(getDb(), defaultSkillWeather.id) + expect(after?.enabled).toBe(0) + expect(after?.pinnedOrder).toBeNull() + }) + it('does not auto-repin when toggling enabled back on', async () => { const skill = await createSkill(getDb(), { name: 'weekly-review', diff --git a/src/skills/skills-view.tsx b/src/skills/skills-view.tsx index d231b8f31..6e2fa0813 100644 --- a/src/skills/skills-view.tsx +++ b/src/skills/skills-view.tsx @@ -5,6 +5,7 @@ import { useCallback, useReducer } from 'react' import { DetailPanel, DetailPanelSurface } from '@/components/detail-panel' +import { isWidgetSkillId } from '@/defaults/skills' import { useConsumeNavState } from '@/hooks/use-consume-nav-state' import { DeleteSkillDialog } from './delete-skill-dialog' import { DependentsDialog } from './dependents-dialog' @@ -46,8 +47,8 @@ export const SkillsView = () => { // Route-state deep links retained for settings integrations. // `editSkill` selects an existing skill; `startEditSkill` lands directly in - // its edit form. Composer create/edit actions use the route-preserving - // CreateItemContext surface instead. + // its edit form, except widget contracts remain on read-only detail. + // Composer create/edit actions use route-preserving CreateItemContext. useConsumeNavState('editSkill', (id) => dispatch({ type: 'SELECT_SKILL', id })) useConsumeNavState('startEditSkill', (id) => dispatch({ type: 'START_EDIT', id })) @@ -56,8 +57,7 @@ export const SkillsView = () => { // slide-in-from-the-right behavior. `undefined` means "nothing selected". const activeSkill = skills.find((s) => s.id === activeId) - // Disabling a pinned skill auto-unpins it. Re-enabling does NOT auto-repin; - // the user pins again deliberately from the chat composer. + // Disabling a pinned skill auto-unpins it so no inert chip remains. const disableSkill = useCallback( async (id: string) => { await setEnabled(id, false) @@ -225,6 +225,7 @@ export const SkillsView = () => { name={skillDisplayName(activeSkill)} description={activeSkill.description} instruction={activeSkill.instruction} + readOnly={isWidgetSkillId(activeSkill.id)} onEdit={() => onEdit(activeSkill.id)} onDelete={() => onDelete(activeSkill.id)} onClose={() => dispatch({ type: 'BACK_TO_LIST' })} diff --git a/src/skills/suggestion-chip.tsx b/src/skills/suggestion-chip.tsx index 887663b1e..1b1ac7f26 100644 --- a/src/skills/suggestion-chip.tsx +++ b/src/skills/suggestion-chip.tsx @@ -23,8 +23,8 @@ export const chipSurfaceClass = /** * Pinned-skill chip shown above the chat input. Click → adds the slash * token to the input (does not auto-submit). Right-click / long-press on - * mobile → context menu with add-to-chat / add-instructions / edit / - * reorder / unpin. + * mobile → context menu with add-to-chat / add-instructions plus optional + * edit / reorder / unpin actions for mutable skills. */ export const SuggestionChip = ({ label, @@ -40,9 +40,9 @@ export const SuggestionChip = ({ onClick: () => void onAddInstruction: () => void /** Open the route-preserving skill editor. */ - onEdit: () => void - onReorder: () => void - onUnpin: () => void + onEdit?: () => void + onReorder?: () => void + onUnpin?: () => void }) => { const [open, setOpen] = useState(false) @@ -125,9 +125,13 @@ export const SuggestionChip = ({ icon: , onSelect: onAddInstruction, }, - { label: 'Edit skill', icon: , onSelect: onEdit }, - { label: 'Reorder', icon: , onSelect: onReorder }, - { label: 'Unpin', icon: , onSelect: onUnpin }, + ...(onEdit + ? [{ label: 'Edit skill', icon: , onSelect: onEdit }] + : []), + ...(onReorder + ? [{ label: 'Reorder', icon: , onSelect: onReorder }] + : []), + ...(onUnpin ? [{ label: 'Unpin', icon: , onSelect: onUnpin }] : []), ] return ( diff --git a/src/types/acp.ts b/src/types/acp.ts index 4dd785be8..2a6b9d942 100644 --- a/src/types/acp.ts +++ b/src/types/acp.ts @@ -22,6 +22,9 @@ import type { ChatThread, 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 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 abc023689..e4173fa0d 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 (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) @@ -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 */