diff --git a/apps/x/packages/core/src/agents/runtime.ts b/apps/x/packages/core/src/agents/runtime.ts index 004526e0..95cdd51b 100644 --- a/apps/x/packages/core/src/agents/runtime.ts +++ b/apps/x/packages/core/src/agents/runtime.ts @@ -8,6 +8,7 @@ import { LanguageModel, stepCountIs, streamText, tool, Tool, ToolSet } from "ai" import { z } from "zod"; import { LlmStepStreamEvent } from "@x/shared/dist/llm-step-events.js"; import { execTool } from "../application/lib/exec-tool.js"; +import { TOOL_ADDITIONS_KEY } from "../application/lib/tool-additions.js"; import { AskHumanRequestEvent, RunEvent, ToolPermissionMetadata, ToolPermissionRequestEvent } from "@x/shared/dist/runs.js"; import { BuiltinTools } from "../application/lib/builtin-tools.js"; import { buildCopilotAgent } from "../application/assistant/agent.js"; @@ -1503,6 +1504,19 @@ export async function* streamAgent({ toolName: toolCall.toolName, }; } + // This legacy loop has no mid-run tool extension; drop the + // reserved tool-additions key (turns-runtime contract) so tool + // schemas never leak into the model-visible result text. + if ( + result !== null && + typeof result === "object" && + !Array.isArray(result) && + TOOL_ADDITIONS_KEY in result + ) { + const rest = { ...(result as Record) }; + delete rest[TOOL_ADDITIONS_KEY]; + result = rest; + } const resultPayload = result === undefined ? null : result; const resultMsg: z.infer = { role: "tool", diff --git a/apps/x/packages/core/src/application/assistant/agent.ts b/apps/x/packages/core/src/application/assistant/agent.ts index 0096601c..3bcf083b 100644 --- a/apps/x/packages/core/src/application/assistant/agent.ts +++ b/apps/x/packages/core/src/application/assistant/agent.ts @@ -1,16 +1,17 @@ import { Agent, ToolAttachment } from "@x/shared/dist/agent.js"; import z from "zod"; import { buildCopilotInstructions } from "./instructions.js"; -import { BuiltinTools } from "../lib/builtin-tools.js"; +import { COPILOT_BASE_TOOLS } from "./base-tools.js"; /** * Build the CopilotAgent dynamically. - * Tools are derived from the current BuiltinTools (which include Composio meta-tools), - * and instructions include the live Composio connection status. + * Only the hardcoded base toolset is attached here; everything else is + * skill-scoped — loading a skill attaches its declared tools (turns runtime + * only). Instructions include the live Composio connection status. */ export async function buildCopilotAgent(): Promise> { const tools: Record> = {}; - for (const name of Object.keys(BuiltinTools)) { + for (const name of COPILOT_BASE_TOOLS) { tools[name] = { type: "builtin", name }; } const instructions = await buildCopilotInstructions(); diff --git a/apps/x/packages/core/src/application/assistant/base-tools.ts b/apps/x/packages/core/src/application/assistant/base-tools.ts new file mode 100644 index 00000000..a69c272a --- /dev/null +++ b/apps/x/packages/core/src/application/assistant/base-tools.ts @@ -0,0 +1,30 @@ +// The copilot's always-attached toolset. Everything else is skill-scoped: +// skills declare the BuiltinTools they own, and loading a skill attaches its +// tools for the rest of the session. Keep this list small — every entry is +// schema bytes on every single model call, and tool-selection accuracy +// degrades as the attached count grows. +// +// code_agent_run and launch-code-task are here for the legacy code-mode path +// (runs/), which shares buildCopilotAgent and cannot gain tools mid-run; +// revisit once code-mode migrates to the turns runtime. +export const COPILOT_BASE_TOOLS: readonly string[] = [ + "loadSkill", + "file-getRoot", + "file-exists", + "file-list", + "file-readText", + "file-glob", + "file-grep", + // Attachment reading is a hot path with no skill signal: users drop PDFs, + // Office docs, and images into chat/calls as path references the model + // must read immediately. + "parseFile", + "LLMParse", + "web-search", + "fetch-url", + "save-to-memory", + "executeCommand", + "spawn-agent", + "code_agent_run", + "launch-code-task", +]; diff --git a/apps/x/packages/core/src/application/assistant/instructions.ts b/apps/x/packages/core/src/application/assistant/instructions.ts index 1b709589..2e8ff56b 100644 --- a/apps/x/packages/core/src/application/assistant/instructions.ts +++ b/apps/x/packages/core/src/application/assistant/instructions.ts @@ -105,7 +105,7 @@ function buildStaticInstructions(composioEnabled: boolean, catalog: string, code : ''; const composioToolsLine = composioEnabled - ? `- \`composio-list-toolkits\`, \`composio-search-tools\`, \`composio-execute-tool\`, \`composio-connect-toolkit\` — Composio integration tools. Load the \`composio-integration\` skill for usage guidance.\n` + ? `- Composio tools (\`composio-list-toolkits\`, \`composio-search-tools\`, \`composio-execute-tool\`, \`composio-connect-toolkit\`) attach when you load the \`composio-integration\` skill.\n` : ''; return `You are Rowboat Copilot - an AI assistant for everyday work. You help users with anything they want. For instance, drafting emails, prepping for meetings, tracking projects, or answering questions - with memory that compounds from their emails, calendar, and notes. Everything runs locally on the user's machine. The nerdy coworker who remembers everything. @@ -268,11 +268,12 @@ In addition to Rowboat-specific workflow management, you can help users with gen Use the catalog below to decide which skills to load for each user request. Before acting: - Call the \`loadSkill\` tool with the skill's name or path so you can read its guidance string. +- Loading a skill also ATTACHES the tools listed in its catalog entry: they become real, callable tools on your very next step and stay attached for the rest of the session. Skills own their tools — this is how you gain capabilities beyond your small starting toolset. - Apply the instructions from every loaded skill while working on the request. ${catalog} -Always consult this catalog first so you load the right skills before taking action. +Always consult this catalog first so you load the right skills before taking action. Your starting toolset is deliberately small: if a capability seems missing, find the skill that owns it in the catalog and load it — NEVER tell the user you can't do something before checking the catalog. If no specialized skill covers the tool you need, load the \`builtin-tools\` skill to attach the full builtin toolset. ## Communication Principles - Be concise and direct. Avoid verbose explanations unless the user asks for details. @@ -318,20 +319,21 @@ ${runtimeContextPrompt} ## Builtin Tools vs Shell Commands -**IMPORTANT**: Rowboat provides builtin tools: -- \`file-readText\`, \`file-writeText\`, \`file-editText\`, \`file-remove\` - File operations -- \`file-list\`, \`file-exists\`, \`file-stat\`, \`file-glob\`, \`file-grep\` - Directory exploration and file search -- \`file-mkdir\`, \`file-rename\`, \`file-copy\` - File/directory management +**IMPORTANT**: Rowboat provides builtin tools. Your always-attached base set: +- \`file-readText\`, \`file-list\`, \`file-exists\`, \`file-glob\`, \`file-grep\`, \`file-getRoot\` - Read-side file operations, directory exploration, and search - \`parseFile\` - Parse and extract text from files (PDF, Excel, CSV, Word .docx). Accepts absolute, ~/..., or relative paths — no need to copy files into the workspace first. Best for well-structured digital documents. - \`LLMParse\` - Send a file to the configured LLM as a multimodal attachment to extract content as markdown. Use this instead of \`parseFile\` for scanned PDFs, images with text, complex layouts, presentations, or any format where local parsing falls short. Supports documents and images. -- \`analyzeAgent\` - Agent analysis -- \`addMcpServer\`, \`listMcpServers\`, \`listMcpTools\`, \`executeMcpTool\` - MCP server management and execution -- \`loadSkill\` - Skill loading -${slackToolsLine}- \`web-search\` - Search the web. Returns rich results with full text, highlights, and metadata. The \`category\` parameter defaults to \`general\` (full web search) — only use a specific category like \`news\`, \`company\`, \`research paper\` etc. when the query is clearly about that type. For everyday queries (weather, restaurants, prices, how-to), use \`general\`. -- \`app-navigation\` - Drive the app UI: open any view, read a view's contents (emails / background agents / chat history), open specific items (email thread, note, agent, past chat), filter/search the knowledge base, manage saved views. **Load the \`app-navigation\` skill before using this tool.** -- \`browser-control\` - Control the embedded browser pane: open sites, inspect the live page, switch tabs, and interact with indexed page elements. **Load the \`browser-control\` skill before using this tool.** +- \`web-search\` - Search the web. Returns rich results with full text, highlights, and metadata. The \`category\` parameter defaults to \`general\` (full web search) — only use a specific category like \`news\`, \`company\`, \`research paper\` etc. when the query is clearly about that type. For everyday queries (weather, restaurants, prices, how-to), use \`general\`. +- \`fetch-url\` - Fetch a URL's contents - \`save-to-memory\` - Save observations about the user to the agent memory system. Use this proactively during conversations. -${composioToolsLine} +- \`loadSkill\` - Load a skill's guidance AND attach the tools it owns +- \`spawn-agent\` - Run sub-agents in isolated headless threads +- \`executeCommand\` - Shell commands (see below) +${slackToolsLine}${composioToolsLine} +Every other builtin is skill-scoped — load the owning skill from the catalog to attach it: +- Write-side file tools (\`file-writeText\`, \`file-editText\`, \`file-mkdir\`, \`file-rename\`, \`file-copy\`, \`file-remove\`, \`file-stat\`) via \`organize-files\`, \`doc-collab\`, and related skills +- MCP server management (\`addMcpServer\`, \`listMcpServers\`, \`listMcpTools\`, \`executeMcpTool\`) via \`mcp-integration\` +- \`app-navigation\` / \`app-set-data\` via the \`app-navigation\` skill; \`browser-control\` via the \`browser-control\` skill; notifications via \`notify-user\`; background tasks via \`background-task\`; everything else via the \`builtin-tools\` escape hatch **Prefer these tools whenever possible.** For file operations anywhere on the machine, use file tools instead of \`executeCommand\`. diff --git a/apps/x/packages/core/src/application/assistant/skills/disk-loader.ts b/apps/x/packages/core/src/application/assistant/skills/disk-loader.ts index 510e4140..8aec5d85 100644 --- a/apps/x/packages/core/src/application/assistant/skills/disk-loader.ts +++ b/apps/x/packages/core/src/application/assistant/skills/disk-loader.ts @@ -15,6 +15,11 @@ export type DiskSkill = { content: string; // full raw SKILL.md text dir: string; // absolute path to the skill folder skillFile: string; // absolute path to the SKILL.md + // Optional frontmatter `tools:` (or the Agent Skills spec's + // `allowed-tools:`) — BuiltinTools names the skill attaches when loaded. + // Names are validated against the live catalog where descriptors are + // built; unknown names are dropped there with a warning. + tools?: string[]; }; // Locations scanned for /SKILL.md subfolders. The rowboat root is @@ -31,6 +36,25 @@ export const SKILL_ROOTS = [ const slugify = (value: string): string => value.trim().toLowerCase().replace(/[\s_]+/g, "-"); +// A YAML list of tool names ("tools: [a, b]" or block form). A single scalar +// ("tools: a") and comma-separated string are accepted too; anything else +// yields []. +const asStringList = (value: unknown): string[] => { + if (typeof value === "string") { + return value + .split(",") + .map((item) => item.trim()) + .filter(Boolean); + } + if (Array.isArray(value)) { + return value + .filter((item): item is string => typeof item === "string") + .map((item) => item.trim()) + .filter(Boolean); + } + return []; +}; + /** * Synchronously scan the known skill roots (one level deep) for disk skills. * A missing/empty directory or an unreadable/invalid SKILL.md simply yields no @@ -91,7 +115,16 @@ export function loadDiskSkills(): DiskSkill[] { } seen.set(id, dir); - skills.push({ id, title: name, summary: description, content: raw, dir, skillFile }); + const tools = asStringList(frontmatter.tools ?? frontmatter["allowed-tools"]); + skills.push({ + id, + title: name, + summary: description, + content: raw, + dir, + skillFile, + ...(tools.length > 0 ? { tools } : {}), + }); } } diff --git a/apps/x/packages/core/src/application/assistant/skills/index.ts b/apps/x/packages/core/src/application/assistant/skills/index.ts index e6e60a7e..0923ed3d 100644 --- a/apps/x/packages/core/src/application/assistant/skills/index.ts +++ b/apps/x/packages/core/src/application/assistant/skills/index.ts @@ -29,6 +29,11 @@ type SkillDefinition = { title: string; summary: string; content: string; + // BuiltinTools keys this skill owns. Loading the skill attaches these as + // native tool definitions (they are NOT in the copilot's base toolset). + // Names are validated against the live catalog where descriptors are + // built (loadSkill / the agent resolver); unknown names are dropped there. + tools?: string[]; }; type ResolvedSkill = { @@ -43,18 +48,21 @@ const definitions: SkillDefinition[] = [ title: "Create Presentations", summary: "Create PDF presentations and slide decks from natural language requests using knowledge base context.", content: createPresentationsSkill, + tools: ["file-writeText", "file-mkdir"], }, { id: "doc-collab", title: "Document Collaboration", summary: "Collaborate on documents - create, edit, and refine notes and documents in the knowledge base.", content: docCollabSkill, + tools: ["file-writeText", "file-editText", "file-mkdir"], }, { id: "draft-emails", title: "Draft Emails", summary: "Process incoming emails and create draft responses using calendar and knowledge base for context.", content: draftEmailsSkill, + tools: ["composio-search-tools", "composio-execute-tool"], }, { id: "meeting-prep", @@ -67,36 +75,58 @@ const definitions: SkillDefinition[] = [ title: "Organize Files", summary: "Find, organize, and tidy up files on the user's machine. Move files to folders, clean up Desktop/Downloads, locate specific files.", content: organizeFilesSkill, + tools: [ + "file-stat", + "file-writeText", + "file-editText", + "file-mkdir", + "file-rename", + "file-copy", + "file-remove", + ], }, { id: "builtin-tools", title: "Builtin Tools Reference", - summary: "Understanding and using builtin tools (especially executeCommand for bash/shell) in agent definitions.", + summary: "Understanding and using builtin tools (especially executeCommand for bash/shell) in agent definitions. Loading this attaches the FULL builtin toolset — the escape hatch when no specialized skill covers a tool you need.", content: builtinToolsSkill, + // Every non-base builtin: kept in sync by a startup assertion in + // builtin-tools.ts rather than by hand. + tools: [], }, { id: "mcp-integration", title: "MCP Integration Guidance", summary: "Discovering, executing, and integrating MCP tools. Use this to check what external capabilities are available and execute MCP tools on behalf of users.", content: mcpIntegrationSkill, + tools: ["addMcpServer", "listMcpServers", "listMcpTools", "executeMcpTool"], }, { id: "composio-integration", title: "Composio Integration", summary: "Interact with third-party services (Gmail, GitHub, Slack, LinkedIn, Notion, Jira, Google Sheets, etc.) via Composio. Search, connect, and execute tools.", content: composioIntegrationSkill, + tools: [ + "composio-list-toolkits", + "composio-search-tools", + "composio-execute-tool", + "composio-connect-toolkit", + "app-navigation", + ], }, { id: "deletion-guardrails", title: "Deletion Guardrails", summary: "Following the confirmation process before removing workflows or agents and their dependencies.", content: deletionGuardrailsSkill, + tools: ["file-remove"], }, { id: "app-navigation", title: "App Navigation", summary: "Navigate the app UI - open notes, switch views, filter/search the knowledge base, and manage saved views.", content: appNavigationSkill, + tools: ["app-navigation", "app-set-data"], }, { id: "code-with-agents", @@ -109,30 +139,49 @@ const definitions: SkillDefinition[] = [ title: "Rowboat Apps", summary: "Build a Rowboat App the user opens inside Rowboat — a static web app on its own origin, powered by their integrations and an optional background agent. Use when the user asks to make/build/create an app or dashboard; for ambiguous 'show me X' requests, confirm whether they want an app first.", content: appsSkill, + tools: [ + "app-navigation", + "app-set-data", + "list-models", + "composio-search-tools", + "composio-execute-tool", + "create-background-task", + "run-background-task-agent", + ], }, { id: "background-task", title: "Background Tasks", summary: "Set up a recurring background task — persistent instructions the agent fires on a schedule and/or on matching events (Gmail, Calendar). Either maintains an `index.md` digest (OUTPUT mode) or performs a recurring side-effect like drafting a reply / posting to Slack / calling an API (ACTION mode). Flagship surface for anything recurring.", content: backgroundTaskSkill, + tools: [ + "create-background-task", + "patch-background-task", + "run-background-task-agent", + "file-writeText", + "file-editText", + ], }, { id: "live-note", title: "Live Notes", summary: "Make a specific markdown note self-updating — a single `live:` objective in the frontmatter that the live-note agent maintains on a schedule or on incoming events. Load only when the user explicitly says 'live note' / 'live-note'; for anything else recurring, prefer the background-task skill.", content: liveNoteSkill, + tools: ["run-live-note-agent", "file-writeText", "file-editText"], }, { id: "browser-control", title: "Browser Control", summary: "Control the embedded browser pane - open sites, inspect page state, and interact with indexed page elements.", content: browserControlSkill, + tools: ["browser-control", "load-browser-skill"], }, { id: "notify-user", title: "Notify User", summary: "Send native desktop notifications with optional clickable links — including rowboat:// deep links that open a specific note, chat, or view inside the app.", content: notifyUserSkill, + tools: ["notify-user"], }, ]; @@ -167,6 +216,7 @@ function loadDiskEntries(): SkillEntry[] { title: skill.title, summary: skill.summary, content: skill.content, + ...(skill.tools ? { tools: skill.tools } : {}), // For disk skills the catalog/loadSkill reference is the absolute SKILL.md path. catalogPath: skill.skillFile, skillFile: skill.skillFile, @@ -190,6 +240,9 @@ export function buildSkillCatalog(options?: { excludeIds?: string[] }): string { `## ${entry.title}`, `- **Skill file:** \`${entry.catalogPath}\``, `- **Use it for:** ${entry.summary}`, + ...(entry.tools && entry.tools.length > 0 + ? [`- **Tools it attaches:** ${entry.tools.join(", ")}`] + : []), ].join("\n")); return [ "# Rowboat Skill Catalog", @@ -310,3 +363,27 @@ export function resolveSkill(identifier: string): ResolvedSkill | null { // Bundled wins over disk on any alias collision. return aliasMap.get(normalized) ?? diskAliasMap.get(normalized) ?? null; } + +/** + * The BuiltinTools names a skill declares (empty for guidance-only skills). + * Read live so it reflects disk-skill refreshes and the derived + * builtin-tools escape-hatch list. + */ +export function skillToolNames(skillId: string): string[] { + const entry = getSkillEntries().find((e) => e.id === skillId); + return entry?.tools ? [...entry.tools] : []; +} + +/** + * The builtin-tools skill attaches the full non-base toolset. Its list is + * derived from the live BuiltinTools catalog by builtin-tools.ts at module + * init — a hand-written list here would drift, and importing the catalog + * from this module would be an import cycle (builtin-tools.ts imports + * resolveSkill). + */ +export function setBuiltinToolsSkillTools(names: string[]): void { + const entry = bundledEntries.find((e) => e.id === "builtin-tools"); + if (entry) { + entry.tools = [...names]; + } +} diff --git a/apps/x/packages/core/src/application/lib/builtin-tools.ts b/apps/x/packages/core/src/application/lib/builtin-tools.ts index 0190c309..c986acef 100644 --- a/apps/x/packages/core/src/application/lib/builtin-tools.ts +++ b/apps/x/packages/core/src/application/lib/builtin-tools.ts @@ -4,7 +4,11 @@ import * as os from "os"; import * as fs from "fs/promises"; import { executeCommand, executeCommandAbortable } from "./command-executor.js"; import { agentSlackShimEnv } from "../../slack/agent-slack-exec.js"; -import { resolveSkill, availableSkills } from "../assistant/skills/index.js"; +import { resolveSkill, availableSkills, skillToolNames, setBuiltinToolsSkillTools } from "../assistant/skills/index.js"; +import { COPILOT_BASE_TOOLS } from "../assistant/base-tools.js"; +import { builtinToolDescriptor } from "../../turns/bridges/builtin-descriptors.js"; +import { TOOL_ADDITIONS_KEY } from "./tool-additions.js"; +import type { ToolDescriptor } from "@x/shared/dist/turns.js"; import { executeTool, listServers, listTools } from "../../mcp/mcp.js"; import container from "../../di/container.js"; import { IMcpConfigRepo } from "../..//mcp/repo.js"; @@ -184,11 +188,26 @@ export const BuiltinTools: z.infer = { }; } + // The skill's declared tools ride the reserved additions key: the + // turn runtime records a durable tools_extended event and the + // model gets them as NATIVE tool definitions on its next call — + // never as schema text in this result. attachedTools names them + // so the model knows the capability landed. + const additions = await skillToolAdditions(resolved.id); return { success: true, skillName: resolved.id, path: resolved.catalogPath, content: resolved.content, + ...(additions.length > 0 + ? { + attachedTools: additions.map((tool) => tool.name), + [TOOL_ADDITIONS_KEY]: { + source: resolved.id, + tools: additions, + }, + } + : {}), }; }, }, @@ -2037,3 +2056,36 @@ export const BuiltinTools: z.infer = { }, }, }; + +// Native ToolDescriptors for a skill's declared tools. Unknown names are +// dropped with a warning (they may come from a downloaded SKILL.md); +// availability-gated builtins (Composio, browser) drop out exactly as they +// do at agent resolution. +async function skillToolAdditions( + skillId: string, +): Promise>> { + const descriptors: Array> = []; + for (const name of skillToolNames(skillId)) { + const builtin = BuiltinTools[name]; + if (!builtin) { + console.warn( + `[skills] Skill '${skillId}' declares unknown tool '${name}'; skipping.`, + ); + continue; + } + if (builtin.isAvailable && !(await builtin.isAvailable())) { + continue; + } + descriptors.push(builtinToolDescriptor(name, builtin)); + } + return descriptors; +} + +// The builtin-tools skill is the escape hatch: loading it attaches every +// builtin the copilot's base set leaves out. Derived here (not hand-written +// in the skill catalog) so new builtins can never silently fall outside it. +setBuiltinToolsSkillTools( + Object.keys(BuiltinTools).filter( + (name) => !COPILOT_BASE_TOOLS.includes(name), + ), +); diff --git a/apps/x/packages/core/src/application/lib/tool-additions.ts b/apps/x/packages/core/src/application/lib/tool-additions.ts new file mode 100644 index 00000000..a44a7e27 --- /dev/null +++ b/apps/x/packages/core/src/application/lib/tool-additions.ts @@ -0,0 +1,17 @@ +import type { z } from "zod"; +import type { ToolDescriptor } from "@x/shared/dist/turns.js"; + +// Contract between builtin tools and the tool registries: a builtin whose +// result should extend the turn's toolset (loadSkill attaching a skill's +// declared tools) returns its value with this reserved key. The registry +// wrapper lifts it out of the model-visible output into +// ToolResultData.metadata.toolAdditions, where the turn runtime picks it up +// and records a durable tools_extended event. The legacy runs loop strips the +// key and ignores it (no mid-run tool extension there). +export const TOOL_ADDITIONS_KEY = "$toolAdditions"; + +export interface ToolAdditions { + // Human-readable origin for the durable event, e.g. the skill id. + source: string; + tools: Array>; +} diff --git a/apps/x/packages/core/src/sessions/sessions.test.ts b/apps/x/packages/core/src/sessions/sessions.test.ts index d4efdaa1..93fc1a40 100644 --- a/apps/x/packages/core/src/sessions/sessions.test.ts +++ b/apps/x/packages/core/src/sessions/sessions.test.ts @@ -842,3 +842,194 @@ describe("headless standalone turns (13.8)", () => { expect(fake.advanceCalls).toEqual([{ turnId, input: undefined }]); }); }); + +describe("active-skill carry-forward", () => { + const skillTool = { + toolId: "builtin:file-writeText", + name: "file-writeText", + description: "Write", + inputSchema: {}, + execution: "sync" as const, + requiresHuman: false, + }; + + // A completed turn whose history loaded a skill mid-turn. + function skillLoadLog( + turnId: string, + sessionId: string, + agent: CreateTurnInput["agent"], + source = "organize-files", + ): TEvent[] { + const created = createdEvent(turnId, { + agent, + sessionId, + context: [], + input: user("hi"), + config: { humanAvailable: true }, + }); + return [ + created, + { + type: "model_call_requested", + turnId, + ts: TS, + modelCallIndex: 0, + request: { messages: ["input"], parameters: {} }, + }, + { + type: "model_call_completed", + turnId, + ts: TS, + modelCallIndex: 0, + message: { + role: "assistant", + content: [ + { + type: "tool-call", + toolCallId: "A", + toolName: "loadSkill", + arguments: {}, + }, + ], + }, + finishReason: "tool-calls", + usage: {}, + }, + { + type: "tool_invocation_requested", + turnId, + ts: TS, + toolCallId: "A", + toolId: "builtin:loadSkill", + toolName: "loadSkill", + execution: "sync", + input: {}, + }, + { + type: "tool_result", + turnId, + ts: TS, + toolCallId: "A", + toolName: "loadSkill", + source: "sync", + result: { output: { success: true }, isError: false }, + }, + { + type: "tools_extended", + turnId, + ts: TS, + toolCallId: "A", + source, + tools: [skillTool], + }, + { + type: "model_call_requested", + turnId, + ts: TS, + modelCallIndex: 1, + request: { messages: ["assistant:0", "toolResult:A"], parameters: {} }, + }, + { + type: "model_call_completed", + turnId, + ts: TS, + modelCallIndex: 1, + message: assistantText("ok"), + finishReason: "stop", + usage: {}, + }, + { + type: "turn_completed", + turnId, + ts: TS, + output: assistantText("ok"), + finishReason: "stop", + usage: {}, + }, + ]; + } + + it("the next turn's composition carries skills recorded by tools_extended", async () => { + const { sessions, fake } = makeSessions(); + const sessionId = await sessions.createSession(); + const { turnId } = await sessions.sendMessage(sessionId, user("one"), { + agent: { agentId: "copilot" }, + }); + await flush(); + fake.setLog(turnId, skillLoadLog(turnId, sessionId, { agentId: "copilot" })); + + await sessions.sendMessage(sessionId, user("two"), { + agent: { agentId: "copilot" }, + }); + const second = fake.createTurnInputs[1]; + expect(second.agent).toMatchObject({ + agentId: "copilot", + overrides: { composition: { activeSkills: ["organize-files"] } }, + }); + }); + + it("accumulates across turns and unions with caller-supplied skills, preserving order", async () => { + const { sessions, fake } = makeSessions(); + const sessionId = await sessions.createSession(); + const first = await sessions.sendMessage(sessionId, user("one"), { + agent: { agentId: "copilot" }, + }); + await flush(); + fake.setLog( + first.turnId, + skillLoadLog(first.turnId, sessionId, { agentId: "copilot" }), + ); + + const second = await sessions.sendMessage(sessionId, user("two"), { + agent: { agentId: "copilot" }, + }); + await flush(); + // Turn 2 carried organize-files in its request and loaded another + // skill mid-turn. + fake.setLog( + second.turnId, + skillLoadLog( + second.turnId, + sessionId, + { + agentId: "copilot", + overrides: { + composition: { activeSkills: ["organize-files"] }, + }, + }, + "doc-collab", + ), + ); + + await sessions.sendMessage(sessionId, user("three"), { + agent: { + agentId: "copilot", + overrides: { composition: { activeSkills: ["notify-user"] } }, + }, + }); + const third = fake.createTurnInputs[2]; + expect(third.agent).toMatchObject({ + overrides: { + composition: { + activeSkills: ["organize-files", "doc-collab", "notify-user"], + }, + }, + }); + }); + + it("a session with no skill loads adds no composition key", async () => { + const { sessions, fake } = makeSessions(); + const sessionId = await sessions.createSession(); + const { turnId } = await sessions.sendMessage(sessionId, user("one"), { + agent: { agentId: "copilot" }, + }); + await flush(); + fake.setLog(turnId, turnLog(turnId, sessionId, "completed")); + + await sessions.sendMessage(sessionId, user("two"), { + agent: { agentId: "copilot" }, + }); + const second = fake.createTurnInputs[1]; + expect(second.agent).toEqual({ agentId: "copilot" }); + }); +}); diff --git a/apps/x/packages/core/src/sessions/sessions.ts b/apps/x/packages/core/src/sessions/sessions.ts index e996be14..acd9d7ef 100644 --- a/apps/x/packages/core/src/sessions/sessions.ts +++ b/apps/x/packages/core/src/sessions/sessions.ts @@ -13,6 +13,7 @@ import { type JsonValue, type ModelDescriptor, type ToolResultData, + type TurnState, deriveTurnStatus, inlineAgentId, isInlineAgentRequest, @@ -151,8 +152,11 @@ export class SessionsImpl implements ISessions { const events = await this.sessionRepo.read(sessionId); const state = reduceSession(events); + let agentRequest = config.agent; if (state.latestTurnId) { - const status = await this.latestTurnStatus(state); + const turn = await this.turnRuntime.getTurn(state.latestTurnId); + const turnState = reduceTurn(turn.events); + const status = deriveTurnStatus(turnState); if ( status !== "completed" && status !== "failed" && @@ -164,10 +168,14 @@ export class SessionsImpl implements ISessions { status, ); } + agentRequest = withActiveSkills( + agentRequest, + deriveActiveSkills(turnState), + ); } const turnId = await this.turnRuntime.createTurn({ - agent: config.agent, + agent: agentRequest, sessionId, context: state.latestTurnId ? { previousTurnId: state.latestTurnId } @@ -416,6 +424,72 @@ function errorMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } +// ---- Skill-scoped tool carry-forward -------------------------------------- +// Skills loaded in earlier turns stay active for the whole session: the next +// turn's activeSkills = the previous turn's requested activeSkills plus any +// skills its durable tools_extended events recorded, in first-load order. +// Stable ordering keeps agent snapshots byte-identical across turns, which is +// what lets snapshot inheritance keep working. + +function parseActiveSkills(composition: JsonValue | undefined): string[] { + if ( + composition === undefined || + composition === null || + typeof composition !== "object" || + Array.isArray(composition) + ) { + return []; + } + const value = (composition as Record).activeSkills; + return Array.isArray(value) + ? value.filter((item): item is string => typeof item === "string") + : []; +} + +function deriveActiveSkills(turnState: TurnState): string[] { + const requested = turnState.definition.agent.requested; + const skills = isInlineAgentRequest(requested) + ? [] + : parseActiveSkills(requested.overrides?.composition); + for (const extension of turnState.toolExtensions) { + if (!skills.includes(extension.event.source)) { + skills.push(extension.event.source); + } + } + return skills; +} + +function withActiveSkills( + agent: SendMessageConfig["agent"], + activeSkills: string[], +): SendMessageConfig["agent"] { + if (isInlineAgentRequest(agent) || activeSkills.length === 0) { + return agent; + } + const composition = agent.overrides?.composition; + const base = + composition !== undefined && + composition !== null && + typeof composition === "object" && + !Array.isArray(composition) + ? composition + : {}; + // Carried-forward skills first (stable order), then any caller-supplied + // extras not already present. + const provided = parseActiveSkills(composition); + const merged = [ + ...activeSkills, + ...provided.filter((id) => !activeSkills.includes(id)), + ]; + return { + ...agent, + overrides: { + ...agent.overrides, + composition: { ...base, activeSkills: merged }, + }, + }; +} + function defaultTitle(input: z.infer): string { const text = typeof input.content === "string" diff --git a/apps/x/packages/core/src/turns/bridges/real-agent-resolver.test.ts b/apps/x/packages/core/src/turns/bridges/real-agent-resolver.test.ts index 8bbcbf60..e33bb3aa 100644 --- a/apps/x/packages/core/src/turns/bridges/real-agent-resolver.test.ts +++ b/apps/x/packages/core/src/turns/bridges/real-agent-resolver.test.ts @@ -222,3 +222,70 @@ describe("RealAgentResolver", () => { ); }); }); + +describe("active skills (skill-scoped tools)", () => { + const skillTools = (skillId: string): string[] => + skillId === "organize-files" + ? ["file-list", "web-search", "ghost"] + : []; + + it("appends declared tools of active skills for copilot, skipping unknown and unavailable ones", async () => { + const resolver = makeResolver( + makeAgent({ + tools: { "spawn-agent": { type: "builtin", name: "spawn-agent" } }, + }), + { skillTools }, + ); + const resolved = await resolver.resolve({ + agentId: "copilot", + overrides: { + composition: { activeSkills: ["organize-files", "gone-skill"] }, + }, + }); + // spawn-agent (base) + file-list from the skill; web-search is + // unavailable and ghost unknown; gone-skill contributes nothing. + expect(resolved.tools.map((t) => t.name)).toEqual([ + "spawn-agent", + "file-list", + ]); + expect(resolved.tools[1]).toMatchObject({ toolId: "builtin:file-list" }); + }); + + it("never duplicates a tool already in the base set", async () => { + const resolver = makeResolver( + makeAgent({ + tools: { "file-list": { type: "builtin", name: "file-list" } }, + }), + { skillTools }, + ); + const resolved = await resolver.resolve({ + agentId: "copilot", + overrides: { composition: { activeSkills: ["organize-files"] } }, + }); + expect( + resolved.tools.filter((t) => t.name === "file-list"), + ).toHaveLength(1); + }); + + it("identical activeSkills yield byte-identical snapshots", async () => { + const resolver = makeResolver(makeAgent(), { skillTools }); + const request = { + agentId: "copilot", + overrides: { composition: { activeSkills: ["organize-files"] } }, + }; + const a = await resolver.resolve(request); + const b = await resolver.resolve(request); + expect(JSON.stringify(a.tools)).toBe(JSON.stringify(b.tools)); + }); + + it("ignores activeSkills for non-copilot agents", async () => { + const resolver = makeResolver(makeAgent({ name: "writer" }), { + skillTools, + }); + const resolved = await resolver.resolve({ + agentId: "writer", + overrides: { composition: { activeSkills: ["organize-files"] } }, + }); + expect(resolved.tools).toEqual([]); + }); +}); diff --git a/apps/x/packages/core/src/turns/bridges/real-agent-resolver.ts b/apps/x/packages/core/src/turns/bridges/real-agent-resolver.ts index 674a5b48..a2cdaf6f 100644 --- a/apps/x/packages/core/src/turns/bridges/real-agent-resolver.ts +++ b/apps/x/packages/core/src/turns/bridges/real-agent-resolver.ts @@ -13,6 +13,7 @@ import { loadUserWorkDir, } from "../../agents/runtime.js"; import { BuiltinTools } from "../../application/lib/builtin-tools.js"; +import { skillToolNames } from "../../application/assistant/skills/index.js"; import { getDefaultModelAndProvider } from "../../models/defaults.js"; import { builtinToolDescriptor, @@ -62,6 +63,11 @@ const CompositionOverrides = z.object({ // Set by spawn-agent for by-id children: strips the spawn tool so depth // is capped at 1 regardless of which stored agent is spawned. subagent: z.boolean().optional(), + // Skills the session has loaded (maintained by the sessions layer from + // prior turns' tools_extended events): their declared tools attach on + // top of the copilot's base set. Order is preserved so identical sets + // produce byte-identical snapshots and inheritance keeps working. + activeSkills: z.array(z.string()).optional(), }); export interface RealAgentResolverDeps { @@ -70,6 +76,7 @@ export interface RealAgentResolverDeps { defaultModel?: () => Promise<{ model: string; provider: string }>; loadNotes?: () => string | null; loadWorkDir?: (workDirId: string) => string | null; + skillTools?: typeof skillToolNames; } // Bridges the existing agent system (loadAgent + dynamic builders, the @@ -84,6 +91,7 @@ export class RealAgentResolver { private readonly defaultModel: () => Promise<{ model: string; provider: string }>; private readonly loadNotes: () => string | null; private readonly loadWorkDir: (workDirId: string) => string | null; + private readonly skillTools: typeof skillToolNames; constructor(deps: RealAgentResolverDeps = {}) { this.load = deps.load ?? loadAgent; @@ -91,6 +99,7 @@ export class RealAgentResolver { this.defaultModel = deps.defaultModel ?? getDefaultModelAndProvider; this.loadNotes = deps.loadNotes ?? loadAgentNotesContext; this.loadWorkDir = deps.loadWorkDir ?? loadUserWorkDir; + this.skillTools = deps.skillTools ?? skillToolNames; } async resolve( @@ -138,6 +147,9 @@ export class RealAgentResolver { const tools = await this.resolveTools(agent, { subagent: composition.subagent ?? false, }); + if (copilotContext && composition.activeSkills?.length) { + await this.appendSkillTools(tools, composition.activeSkills); + } return ResolvedAgent.parse({ agentId: requested.agentId, systemPrompt, @@ -146,6 +158,35 @@ export class RealAgentResolver { }); } + // Skill-scoped tools carried across turns: each active skill's declared + // BuiltinTools attach on top of the base set. Unknown skill ids (deleted + // disk skill) and unknown/unavailable tool names skip gracefully. + // Iteration order (skills, then declaration order) is deterministic, so + // an unchanged activeSkills list yields a byte-identical snapshot and + // agent-snapshot inheritance keeps working. + private async appendSkillTools( + tools: Array>, + activeSkills: string[], + ): Promise { + const attached = new Set(tools.map((tool) => tool.name)); + for (const skillId of activeSkills) { + for (const name of this.skillTools(skillId)) { + if (attached.has(name)) { + continue; + } + const builtin = this.builtins[name]; + if (!builtin) { + continue; + } + if (builtin.isAvailable && !(await builtin.isAvailable())) { + continue; + } + tools.push(builtinToolDescriptor(name, builtin)); + attached.add(name); + } + } + } + private async resolveTools( agent: z.infer, options: { subagent: boolean }, diff --git a/apps/x/packages/core/src/turns/bridges/real-tool-registry.ts b/apps/x/packages/core/src/turns/bridges/real-tool-registry.ts index c3eb6214..1f1ce9b9 100644 --- a/apps/x/packages/core/src/turns/bridges/real-tool-registry.ts +++ b/apps/x/packages/core/src/turns/bridges/real-tool-registry.ts @@ -8,6 +8,7 @@ import { } from "@x/shared/dist/turns.js"; import { execTool } from "../../application/lib/exec-tool.js"; import { BuiltinTools } from "../../application/lib/builtin-tools.js"; +import { TOOL_ADDITIONS_KEY } from "../../application/lib/tool-additions.js"; import { type IAbortRegistry, InMemoryAbortRegistry, @@ -214,9 +215,15 @@ export class RealToolRegistry implements IToolRegistry { }, }, ); + const { output, additions } = splitToolAdditions( + toJsonValue(value === undefined ? null : value), + ); return { - output: toJsonValue(value === undefined ? null : value), + output, isError: false, + ...(additions === undefined + ? {} + : { metadata: { toolAdditions: additions } }), }; } finally { ctx.signal.removeEventListener("abort", onAbort); @@ -227,6 +234,26 @@ export class RealToolRegistry implements IToolRegistry { } } +// Lift a builtin's reserved tool-additions key out of the model-visible +// output into result metadata: the model gets the added tools as native +// definitions on its next call, so repeating their schemas inside the tool +// result would only burn tokens. Shape validation happens in the runtime. +function splitToolAdditions(value: JsonValue): { + output: JsonValue; + additions?: JsonValue; +} { + if ( + value !== null && + typeof value === "object" && + !Array.isArray(value) && + TOOL_ADDITIONS_KEY in value + ) { + const { [TOOL_ADDITIONS_KEY]: additions, ...output } = value; + return { output, additions }; + } + return { output: value }; +} + function asArgs(input: unknown): Record { return input && typeof input === "object" ? (input as Record) diff --git a/apps/x/packages/core/src/turns/compose-model-request.ts b/apps/x/packages/core/src/turns/compose-model-request.ts index 9811e9c0..308b20a2 100644 --- a/apps/x/packages/core/src/turns/compose-model-request.ts +++ b/apps/x/packages/core/src/turns/compose-model-request.ts @@ -5,14 +5,16 @@ import { type ResolvedAgent, type ToolDescriptor, type TurnState, + effectiveTools, requestMessagesFor, } from "@x/shared/dist/turns.js"; import type { IContextResolver } from "./context-resolver.js"; // The exact provider payload for one model call, rebuilt deterministically // from durable state (turn-runtime-design.md §8.3): -// - systemPrompt and tools come from the resolved agent snapshot (their -// single canonical copy in turn_created), +// - systemPrompt and base tools come from the resolved agent snapshot +// (their single canonical copy in turn_created); tools_extended events +// add descriptors for the calls requested after them, // - messages are the cross-turn prefix plus every request's reference list // resolved against the turn's own events, encoded to wire form. // This is the SAME code path the loop sends through, so the debug view and @@ -47,7 +49,9 @@ export function composeModelRequest( return { systemPrompt: agent.systemPrompt, messages: encode(structural), - tools: agent.tools, + // The snapshot's base tools plus any durable mid-turn extensions + // recorded before this call (tools_extended events). + tools: effectiveTools(state, modelCallIndex, agent.tools), parameters: call.request.parameters, }; } diff --git a/apps/x/packages/core/src/turns/runtime.test.ts b/apps/x/packages/core/src/turns/runtime.test.ts index 83027868..cec4a2c4 100644 --- a/apps/x/packages/core/src/turns/runtime.test.ts +++ b/apps/x/packages/core/src/turns/runtime.test.ts @@ -2071,3 +2071,246 @@ describe("concurrent sync tool execution (10.5)", () => { ]); }); }); + +describe("mid-turn tool extension", () => { + const writeDescriptor: z.infer = { + toolId: "tool.write", + name: "write", + description: "Write tool", + inputSchema: {}, + execution: "sync", + requiresHuman: false, + }; + + function additionsTool( + descriptor: z.infer, + tools: Array>, + source = "organize-files", + ): SyncRuntimeTool { + return syncTool(descriptor, async () => ({ + output: { success: true }, + isError: false, + metadata: { toolAdditions: { source, tools } }, + })); + } + + const writeImpl = syncTool(writeDescriptor, async () => ({ + output: "written", + isError: false, + })); + + it("appends tools_extended and the added tool is callable on the next step", async () => { + const { runtime, repo, models } = makeRuntime({ + models: [ + respond(completedResp(assistantCalls(toolCallPart("tc1", "echo")))), + respond(completedResp(assistantCalls(toolCallPart("tc2", "write")))), + respond(completedResp(assistantText("done"))), + ], + tools: [ + additionsTool(echoDescriptor, [writeDescriptor]), + writeImpl, + ...defaultTools.slice(1), + ], + }); + const turnId = await newTurn(runtime); + const { outcome, error } = await advanceAndSettle(runtime, turnId); + expect(error).toBeUndefined(); + expect(outcome?.status).toBe("completed"); + + const log = await persisted(repo, turnId); + const extensions = log.filter((e) => e.type === "tools_extended"); + expect(extensions).toHaveLength(1); + expect(extensions[0]).toMatchObject({ + toolCallId: "tc1", + source: "organize-files", + tools: [writeDescriptor], + }); + + // Call 0 was composed without the tool; call 1 carries it natively. + expect(models.requests[0].tools.map((t) => t.name)).not.toContain("write"); + expect(models.requests[1].tools.map((t) => t.name)).toContain("write"); + + // The reducer accepts the full history and the added tool executed. + const state = reduceTurn(log); + const tc2 = state.toolCalls.find((tc) => tc.toolCallId === "tc2"); + expect(tc2?.toolId).toBe("tool.write"); + expect(tc2?.result?.result.output).toBe("written"); + }); + + it("additions already attached dedupe to no event", async () => { + const { runtime, repo } = makeRuntime({ + models: [ + respond(completedResp(assistantCalls(toolCallPart("tc1", "echo")))), + respond(completedResp(assistantText("done"))), + ], + tools: [ + additionsTool(echoDescriptor, [echoDescriptor], "self"), + ...defaultTools.slice(1), + ], + }); + const turnId = await newTurn(runtime); + const { outcome } = await advanceAndSettle(runtime, turnId); + expect(outcome?.status).toBe("completed"); + const log = await persisted(repo, turnId); + expect(log.filter((e) => e.type === "tools_extended")).toHaveLength(0); + }); + + it("additions without a live implementation are dropped, not fatal", async () => { + const ghost: z.infer = { + ...writeDescriptor, + toolId: "tool.ghost", + name: "ghost", + }; + const { runtime, repo } = makeRuntime({ + models: [ + respond(completedResp(assistantCalls(toolCallPart("tc1", "echo")))), + respond(completedResp(assistantText("done"))), + ], + tools: [ + additionsTool(echoDescriptor, [ghost]), + ...defaultTools.slice(1), + ], + }); + const turnId = await newTurn(runtime); + const { outcome } = await advanceAndSettle(runtime, turnId); + expect(outcome?.status).toBe("completed"); + const log = await persisted(repo, turnId); + expect(log.filter((e) => e.type === "tools_extended")).toHaveLength(0); + }); + + it("concurrent overlapping additions dedupe atomically to one descriptor", async () => { + const echo2Descriptor: z.infer = { + ...echoDescriptor, + toolId: "tool.echo2", + name: "echo2", + }; + const { runtime, repo } = makeRuntime({ + models: [ + respond( + completedResp( + assistantCalls( + toolCallPart("tc1", "echo"), + toolCallPart("tc2", "echo2"), + ), + ), + ), + respond(completedResp(assistantText("done"))), + ], + tools: [ + additionsTool(echoDescriptor, [writeDescriptor], "skill-a"), + additionsTool(echo2Descriptor, [writeDescriptor], "skill-b"), + writeImpl, + ...defaultTools.slice(1), + ], + agent: { + ...defaultAgent, + tools: [ + echoDescriptor, + echo2Descriptor, + fetchDescriptor, + askHumanDescriptor, + ], + }, + }); + const turnId = await newTurn(runtime); + const { outcome } = await advanceAndSettle(runtime, turnId); + expect(outcome?.status).toBe("completed"); + + const log = await persisted(repo, turnId); + const added = log + .filter((e) => e.type === "tools_extended") + .flatMap((e) => (e.type === "tools_extended" ? e.tools : [])); + expect(added.filter((t) => t.name === "write")).toHaveLength(1); + // reduceTurn would throw on a collision; acceptance is the assertion. + expect(() => reduceTurn(log)).not.toThrow(); + }); + + it("recovers from a log ending right after tools_extended", async () => { + const SEED_ID = "2026-07-02T10-00-00Z-0000009-000"; + const repo = new InMemoryTurnRepo(); + repo.seed([ + { + type: "turn_created", + schemaVersion: 1, + turnId: SEED_ID, + ts: TS, + sessionId: null, + agent: { requested: { agentId: "copilot" }, resolved: defaultAgent }, + context: [], + input: user("hello"), + config: { + autoPermission: false, + humanAvailable: true, + maxModelCalls: 20, + }, + }, + { + type: "model_call_requested", + turnId: SEED_ID, + ts: TS, + modelCallIndex: 0, + request: { messages: ["input"], parameters: {} }, + }, + { + type: "model_call_completed", + turnId: SEED_ID, + ts: TS, + modelCallIndex: 0, + message: assistantCalls(toolCallPart("tc1", "echo")), + finishReason: "stop", + usage: {}, + }, + { + type: "tool_invocation_requested", + turnId: SEED_ID, + ts: TS, + toolCallId: "tc1", + toolId: echoDescriptor.toolId, + toolName: "echo", + execution: "sync", + input: {}, + }, + { + type: "tool_result", + turnId: SEED_ID, + ts: TS, + toolCallId: "tc1", + toolName: "echo", + source: "sync", + result: { output: { success: true }, isError: false }, + }, + { + type: "tools_extended", + turnId: SEED_ID, + ts: TS, + toolCallId: "tc1", + source: "organize-files", + tools: [writeDescriptor], + }, + ]); + const { runtime, models } = makeRuntime({ + repo, + models: [ + respond(completedResp(assistantCalls(toolCallPart("tc2", "write")))), + respond(completedResp(assistantText("done"))), + ], + tools: [ + additionsTool(echoDescriptor, [writeDescriptor]), + writeImpl, + ...defaultTools.slice(1), + ], + }); + const { outcome } = await advanceAndSettle(runtime, SEED_ID); + expect(outcome?.status).toBe("completed"); + // The re-advance rebuilt the extended toolset from the durable log: + // its first composed request already includes the added tool, and the + // model's call to it executes. + expect(models.requests[0].tools.map((t) => t.name)).toContain("write"); + const log = await persisted(repo, SEED_ID); + const state = reduceTurn(log); + expect( + state.toolCalls.find((tc) => tc.toolCallId === "tc2")?.result?.result + .output, + ).toBe("written"); + }); +}); diff --git a/apps/x/packages/core/src/turns/runtime.ts b/apps/x/packages/core/src/turns/runtime.ts index 0fd4337c..2b1b7065 100644 --- a/apps/x/packages/core/src/turns/runtime.ts +++ b/apps/x/packages/core/src/turns/runtime.ts @@ -1,4 +1,4 @@ -import type { z } from "zod"; +import { z } from "zod"; import { DEFAULT_MAX_MODEL_CALLS, MODEL_CALL_LIMIT_ERROR_CODE, @@ -11,7 +11,7 @@ import { assistantRef, toolResultRef, type ToolCallState, - type ToolDescriptor, + ToolDescriptor, type ToolInvocationRequested, type ToolPermissionRequired, type ToolPermissionResolved, @@ -22,6 +22,7 @@ import { type TurnState, type TurnStreamEvent, type TurnSuspended, + effectiveTools, outstandingAsyncTools, outstandingPermissions, reduceTurn, @@ -228,8 +229,14 @@ export class TurnRuntime implements ITurnRuntime { definition.agent.resolved, ); const model = await this.modelRegistry.resolve(resolvedAgent.model); + // Base snapshot plus every durable mid-turn extension already in the + // log — rebuilding from durable state IS the crash-recovery path. const toolsByName = new Map(); - for (const descriptor of resolvedAgent.tools) { + for (const descriptor of effectiveTools( + state, + state.modelCalls.length, + resolvedAgent.tools, + )) { const tool = await this.toolRegistry.resolve(descriptor); if ( tool.descriptor.toolId !== descriptor.toolId || @@ -264,6 +271,7 @@ export class TurnRuntime implements ITurnRuntime { model, usageReporter: this.usageReporter, toolsByName, + resolveTool: (descriptor) => this.toolRegistry.resolve(descriptor), signal: controller.signal, turnRepo: this.turnRepo, clock: this.clock, @@ -293,6 +301,9 @@ class TurnAdvance { private readonly model: ResolvedModel; private readonly usageReporter: IUsageReporter; private readonly toolsByName: Map; + private readonly resolveTool: ( + descriptor: z.infer, + ) => Promise; private readonly signal: AbortSignal; private readonly turnRepo: ITurnRepo; private readonly clock: IClock; @@ -315,6 +326,9 @@ class TurnAdvance { model: ResolvedModel; usageReporter: IUsageReporter; toolsByName: Map; + resolveTool: ( + descriptor: z.infer, + ) => Promise; signal: AbortSignal; turnRepo: ITurnRepo; clock: IClock; @@ -330,6 +344,7 @@ class TurnAdvance { this.model = init.model; this.usageReporter = init.usageReporter; this.toolsByName = init.toolsByName; + this.resolveTool = init.resolveTool; this.signal = init.signal; this.turnRepo = init.turnRepo; this.clock = init.clock; @@ -363,6 +378,26 @@ class TurnAdvance { return task; } + // Like append, but the batch is computed inside this task's serialized + // commit slot, so the builder reads this.state with no interleaving + // commits. Required for the tools_extended dedupe: two concurrent tool + // results may each carry overlapping additions, and the second filter + // must see the first's committed extension. Returning undefined commits + // nothing. + private appendWith(build: () => TEvent[] | undefined): Promise { + const task = this.appendChain.then(async () => { + const batch = build(); + if (batch && batch.length > 0) { + await this.commit(batch); + } + }); + this.appendChain = task.then( + () => undefined, + () => undefined, + ); + return task; + } + private async commit(batch: TEvent[]): Promise { await this.turnRepo.append(this.turnId, batch); this.events.push(...batch); @@ -808,6 +843,7 @@ class TurnAdvance { tc: ToolCallState, syncTool: SyncRuntimeTool, ): Promise { + let settled: z.infer | undefined; try { const result = await syncTool.execute(tc.input, { turnId: this.turnId, @@ -824,6 +860,7 @@ class TurnAdvance { }); }, }); + settled = ToolResultData.parse(result); await this.append({ type: "tool_result", turnId: this.turnId, @@ -831,7 +868,7 @@ class TurnAdvance { toolCallId: tc.toolCallId, toolName: tc.toolName, source: "sync", - result: ToolResultData.parse(result), + result: settled, }); } catch (error) { if (this.signal.aborted) { @@ -852,6 +889,87 @@ class TurnAdvance { source: "sync", result: { output: errorMessage(error), isError: true }, }); + return; + } + await this.maybeExtendTools(tc, settled); + } + + // Mid-turn toolset extension (the one sanctioned exception to per-turn + // tool immutability): a successful sync tool may carry additional + // descriptors in result.metadata.toolAdditions — loadSkill attaching a + // skill's declared tools. Descriptors resolve to live implementations + // first; unresolvable ones are dropped so the durable event only ever + // records tools a re-advance can also resolve. The dedupe against the + // current effective set and the tools_extended append run atomically in + // one serialized commit slot, so concurrent extensions cannot race the + // reducer's collision check. Subsequent model calls in this turn see the + // added tools via effectiveTools. + private async maybeExtendTools( + tc: ToolCallState, + result: z.infer, + ): Promise { + if (result.isError) { + return; + } + const additions = parseToolAdditions(result.metadata); + if (!additions) { + return; + } + const live = new Map(); + const candidates: Array> = []; + for (const descriptor of additions.tools) { + if (live.has(descriptor.name)) { + continue; // duplicate within the payload + } + try { + const tool = await this.resolveTool(descriptor); + if ( + tool.descriptor.toolId !== descriptor.toolId || + tool.descriptor.execution !== descriptor.execution + ) { + continue; + } + live.set(descriptor.name, tool); + candidates.push(descriptor); + } catch { + // No live implementation — skip the tool; the result that + // carried it (e.g. the skill guidance) still stands. + } + } + if (candidates.length === 0) { + return; + } + let added: Array> = []; + await this.appendWith(() => { + const attached = new Set( + effectiveTools( + this.state, + this.state.modelCalls.length, + this.resolvedAgent.tools, + ).map((tool) => tool.name), + ); + added = candidates.filter( + (descriptor) => !attached.has(descriptor.name), + ); + if (added.length === 0) { + return undefined; + } + return [ + { + type: "tools_extended", + turnId: this.turnId, + ts: this.now(), + toolCallId: tc.toolCallId, + source: additions.source, + tools: added, + }, + ]; + }); + for (const descriptor of added) { + const tool = live.get(descriptor.name); + if (tool) { + this.toolsByName.set(descriptor.name, tool); + } } } @@ -1114,6 +1232,27 @@ class TurnAdvance { } } +// The metadata contract populated by the tool-registry bridge from a +// builtin's reserved $toolAdditions return key (application/lib/ +// tool-additions.ts). Anything malformed is ignored — a bad payload must +// never corrupt the turn. +const ToolAdditionsMetadata = z.object({ + toolAdditions: z.object({ + source: z.string(), + tools: z.array(ToolDescriptor).min(1), + }), +}); + +function parseToolAdditions( + metadata: JsonValue | undefined, +): { source: string; tools: Array> } | undefined { + if (metadata === undefined) { + return undefined; + } + const parsed = ToolAdditionsMetadata.safeParse(metadata); + return parsed.success ? parsed.data.toolAdditions : undefined; +} + function errorMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } diff --git a/apps/x/packages/shared/src/turns.test.ts b/apps/x/packages/shared/src/turns.test.ts index 84a5a9c7..11d24f84 100644 --- a/apps/x/packages/shared/src/turns.test.ts +++ b/apps/x/packages/shared/src/turns.test.ts @@ -15,6 +15,7 @@ import { ToolPermissionResolved, ToolProgress, ToolResult, + ToolsExtended, TurnCancelled, TurnCompleted, TurnCorruptionError, @@ -23,6 +24,8 @@ import { TurnFailed, TurnSuspended, deriveTurnStatus, + effectiveTools, + extendedToolsFor, outstandingAsyncTools, outstandingPermissions, reduceTurn, @@ -1419,3 +1422,159 @@ describe("derivations", () => { expect(() => turnTranscript(state)).toThrowError(/unresolved/); }); }); + +describe("mid-turn tool extension", () => { + const writeTool: z.infer = { + toolId: "builtin:file-writeText", + name: "file-writeText", + description: "Write a text file", + inputSchema: {}, + execution: "sync", + requiresHuman: false, + }; + + function extendedEv( + toolCallId: string, + tools: Array> = [writeTool], + source = "organize-files", + ): z.infer { + return { + type: "tools_extended", + turnId: TURN_ID, + ts: TS, + toolCallId, + source, + tools, + }; + } + + // created → call 0 → echo tc1 (success) → tools_extended riding tc1. + function extensionSequence(): TEvent[] { + const call0 = assistantCalls(toolCallPart("tc1", "echo")); + return [ + created(), + requested(0, ["input"]), + completed(0, call0), + permRequired("tc1", "echo"), + permResolved("tc1", "allow"), + invocation("tc1"), + result("tc1", "echo"), + extendedEv("tc1"), + ]; + } + + it("round-trips through the TurnEvent schema", () => { + expect(TurnEvent.parse(extendedEv("tc1"))).toEqual(extendedEv("tc1")); + }); + + it("records the extension and applies it from the next model call on", () => { + const state = reduceTurn(extensionSequence()); + expect(state.toolExtensions).toHaveLength(1); + expect(state.toolExtensions[0].firstAffectedModelCallIndex).toBe(1); + + const base = [echoTool, fetchTool]; + expect(effectiveTools(state, 0, base)).toEqual(base); + expect(effectiveTools(state, 1, base)).toEqual([...base, writeTool]); + expect(extendedToolsFor(state, 0)).toEqual([]); + expect(extendedToolsFor(state, 1)).toEqual([writeTool]); + }); + + it("stamps descriptor identity on calls to extended tools", () => { + const state = reduceTurn([ + ...extensionSequence(), + requested(1, ["assistant:0", "toolResult:tc1"]), + completed(1, assistantCalls(toolCallPart("tc2", "file-writeText"))), + ]); + const tc2 = state.toolCalls.find((tc) => tc.toolCallId === "tc2"); + expect(tc2?.toolId).toBe("builtin:file-writeText"); + expect(tc2?.execution).toBe("sync"); + }); + + it("a full turn with a mid-turn extension completes cleanly", () => { + const state = reduceTurn([ + ...extensionSequence(), + requested(1, ["assistant:0", "toolResult:tc1"]), + completed(1, assistantText("done")), + turnCompletedEv(), + ]); + expect(state.terminal?.type).toBe("turn_completed"); + }); + + it("rejects an extension referencing an unknown tool call", () => { + expectCorruption( + [created(), extendedEv("nope")], + /unknown tool call/, + ); + }); + + it("rejects an extension before its tool call has a result", () => { + const call0 = assistantCalls(toolCallPart("tc1", "echo")); + expectCorruption( + [ + created(), + requested(0, ["input"]), + completed(0, call0), + permRequired("tc1", "echo"), + permResolved("tc1", "allow"), + invocation("tc1"), + extendedEv("tc1"), + ], + /without a tool result/, + ); + }); + + it("rejects an extension riding a failed tool result", () => { + const call0 = assistantCalls(toolCallPart("tc1", "echo")); + expectCorruption( + [ + created(), + requested(0, ["input"]), + completed(0, call0), + permRequired("tc1", "echo"), + permResolved("tc1", "allow"), + invocation("tc1"), + result("tc1", "echo", "sync", "boom", true), + extendedEv("tc1"), + ], + /failed tool call/, + ); + }); + + it("rejects a name colliding with the base snapshot", () => { + expectCorruption( + [ + ...extensionSequence().slice(0, -1), + extendedEv("tc1", [{ ...writeTool, name: "echo" }]), + ], + /colliding tool name: echo/, + ); + }); + + it("rejects a name colliding with a prior extension", () => { + expectCorruption( + [...extensionSequence(), extendedEv("tc1", [writeTool])], + /colliding tool name: file-writeText/, + ); + }); + + it("rejects duplicate names within one extension", () => { + expectCorruption( + [ + ...extensionSequence().slice(0, -1), + extendedEv("tc1", [writeTool, { ...writeTool }]), + ], + /colliding tool name: file-writeText/, + ); + }); + + it("rejects an extension while a model call is unsettled", () => { + expectCorruption( + [ + ...extensionSequence().slice(0, -1), + requested(1, ["assistant:0", "toolResult:tc1"]), + extendedEv("tc1"), + ], + /model call is unsettled/, + ); + }); +}); diff --git a/apps/x/packages/shared/src/turns.ts b/apps/x/packages/shared/src/turns.ts index cb565246..c290607c 100644 --- a/apps/x/packages/shared/src/turns.ts +++ b/apps/x/packages/shared/src/turns.ts @@ -361,6 +361,22 @@ export const ToolResult = z.object({ result: ToolResultData, }); +// A successful sync tool extended the turn's toolset mid-turn (e.g. loadSkill +// attaching a skill's declared tools). The descriptors are snapshotted in the +// event so replay needs no external lookups; they apply to every model call +// requested after this point. This is the one sanctioned exception to the +// tool-set immutability rule — explicit and durable, never silent. +export const ToolsExtended = z.object({ + type: z.literal("tools_extended"), + turnId: z.string(), + ts: z.string(), + // The tool result that carried these additions (metadata.addTools). + toolCallId: z.string(), + // Human-readable origin, e.g. the skill id that declared the tools. + source: z.string(), + tools: z.array(ToolDescriptor).min(1), +}); + export const TurnSuspended = z.object({ type: z.literal("turn_suspended"), turnId: z.string(), @@ -424,6 +440,7 @@ export const TurnEvent = z.discriminatedUnion("type", [ ToolInvocationRequested, ToolProgress, ToolResult, + ToolsExtended, TurnSuspended, TurnCompleted, TurnFailed, @@ -496,10 +513,18 @@ export interface ToolCallState { result?: z.infer; } +export interface ToolExtensionState { + event: z.infer; + // Extensions land between model calls; every call requested from this + // index onward sees the added tools. + firstAffectedModelCallIndex: number; +} + export interface TurnState { definition: z.infer; modelCalls: ModelCallState[]; toolCalls: ToolCallState[]; + toolExtensions: ToolExtensionState[]; suspension?: z.infer; terminal?: | z.infer @@ -675,11 +700,17 @@ function applyModelCallCompleted( fail(`duplicate tool call id: ${part.toolCallId}`); } const resolved = state.definition.agent.resolved; - // Inherited snapshots resolve outside the reducer; identity fields - // then arrive via tool_invocation_requested events. - const descriptor = isInheritedSnapshot(resolved) - ? undefined - : resolved.tools.find((tool) => tool.name === part.toolName); + // Mid-turn extensions are searched first (their descriptors are + // always in the log); inherited base snapshots resolve outside the + // reducer, so identity fields for those tools arrive via + // tool_invocation_requested events instead. + const descriptor = + extendedToolsFor(state, event.modelCallIndex).find( + (tool) => tool.name === part.toolName, + ) ?? + (isInheritedSnapshot(resolved) + ? undefined + : resolved.tools.find((tool) => tool.name === part.toolName)); state.toolCalls.push({ modelCallIndex: event.modelCallIndex, order: order++, @@ -841,6 +872,44 @@ function applyToolResult( toolCall.result = event; } +function applyToolsExtended( + state: TurnState, + event: z.infer, +): void { + if (openModelCall(state)) { + fail("tools extended while a model call is unsettled"); + } + const toolCall = findToolCall(state, event.toolCallId); + if (!toolCall.result) { + fail(`tools extended without a tool result: ${event.toolCallId}`); + } + if (toolCall.result.result.isError) { + fail(`tools extended by a failed tool call: ${event.toolCallId}`); + } + // Collision check covers prior extensions and (when visible) the base + // snapshot; inherited snapshots are opaque to the reducer, so their base + // names are enforced by the runtime's dedupe instead. + const existing = new Set( + extendedToolsFor(state, state.modelCalls.length).map((t) => t.name), + ); + const resolved = state.definition.agent.resolved; + if (!isInheritedSnapshot(resolved)) { + for (const tool of resolved.tools) { + existing.add(tool.name); + } + } + for (const tool of event.tools) { + if (existing.has(tool.name)) { + fail(`tools extended with a colliding tool name: ${tool.name}`); + } + existing.add(tool.name); + } + state.toolExtensions.push({ + event, + firstAffectedModelCallIndex: state.modelCalls.length, + }); +} + function sortedIds(ids: string[]): string { return [...ids].sort().join(","); } @@ -929,6 +998,7 @@ export function reduceTurn( definition: first, modelCalls: [], toolCalls: [], + toolExtensions: [], usage: {}, }; @@ -978,6 +1048,9 @@ export function reduceTurn( case "tool_result": applyToolResult(state, event); break; + case "tools_extended": + applyToolsExtended(state, event); + break; case "turn_suspended": applyTurnSuspended(state, event); break; @@ -1006,6 +1079,29 @@ export function reduceTurn( // Pure derivations over TurnState // --------------------------------------------------------------------------- +// Descriptors added by durable extensions in effect for a given model call. +export function extendedToolsFor( + state: TurnState, + modelCallIndex: number, +): Array> { + return state.toolExtensions + .filter((ext) => ext.firstAffectedModelCallIndex <= modelCallIndex) + .flatMap((ext) => ext.event.tools); +} + +// The full toolset a given model call sees: the agent snapshot's base tools +// plus every extension recorded before that call was requested. The base set +// comes from the materialized agent (inherited snapshots are opaque here), +// so callers pass it in. +export function effectiveTools( + state: TurnState, + modelCallIndex: number, + baseTools: Array>, +): Array> { + const extended = extendedToolsFor(state, modelCallIndex); + return extended.length === 0 ? baseTools : [...baseTools, ...extended]; +} + export function outstandingPermissions(state: TurnState): ToolCallState[] { return state.toolCalls.filter( (tc) => tc.permission && !tc.permission.resolved && !tc.result,