diff --git a/apps/x/packages/core/src/agents/compose-instructions.ts b/apps/x/packages/core/src/agents/compose-instructions.ts index 025f9196..2b30402e 100644 --- a/apps/x/packages/core/src/agents/compose-instructions.ts +++ b/apps/x/packages/core/src/agents/compose-instructions.ts @@ -1,7 +1,17 @@ import { MODE_CAPABILITIES } from "../application/assistant/capabilities/modes.js"; -import { WORKSPACE_CONTEXT_CAPABILITY } from "../application/assistant/capabilities/workspace.js"; +import { + AGENT_NOTES_CAPABILITY, + WORK_DIRECTORY_CAPABILITY, +} from "../application/assistant/capabilities/workspace.js"; import type { CapabilityContext } from "../application/assistant/capabilities/types.js"; +// Everything that composes into the system prompt, in composition order. +const PROMPT_CAPABILITIES = [ + AGENT_NOTES_CAPABILITY, + WORK_DIRECTORY_CAPABILITY, + ...MODE_CAPABILITIES, +] as const; + // System-prompt composition for agent assembly: the base instructions plus // the mode blocks (voice, video, coach, search, code) appended per turn // composition. Extracted verbatim from the legacy streamAgent path so both @@ -52,26 +62,26 @@ export function composeSystemInstructions({ videoMode, coachMode, }: ComposeSystemInstructionsInput): string { - let instructionsWithDateTime = `${instructions}\n\n${USER_CONTEXT_SYSTEM_INSTRUCTIONS}`; - // App-activated mode capabilities compose here, in MODE_CAPABILITIES - // order — a fixed total order, so identical inputs yield identical - // bytes. The fragment text lives with the capability records. - const ctx: CapabilityContext = { - agentNotesContext, - userWorkDir, - voiceInput, - voiceOutput, - searchEnabled, - codeMode, - codeCwd, - videoMode: videoMode ?? false, - coachMode: coachMode ?? false, - }; - for (const capability of [WORKSPACE_CONTEXT_CAPABILITY, ...MODE_CAPABILITIES]) { - const fragment = capability.promptFragment?.(ctx); - if (fragment) { - instructionsWithDateTime += `\n\n${fragment}`; - } + let composed = `${instructions}\n\n${USER_CONTEXT_SYSTEM_INSTRUCTIONS}`; + // Capabilities compose in PROMPT_CAPABILITIES order — a fixed total + // order, so identical inputs yield identical bytes. The fragment text + // lives with the capability records. + const ctx: CapabilityContext = { + agentNotesContext, + userWorkDir, + voiceInput, + voiceOutput, + searchEnabled, + codeMode, + codeCwd, + videoMode: videoMode ?? false, + coachMode: coachMode ?? false, + }; + for (const capability of PROMPT_CAPABILITIES) { + const fragment = capability.promptFragment?.(ctx); + if (fragment) { + composed += `\n\n${fragment}`; } - return instructionsWithDateTime; + } + return composed; } diff --git a/apps/x/packages/core/src/agents/registry.test.ts b/apps/x/packages/core/src/agents/registry.test.ts index dffe0994..d89a66ba 100644 --- a/apps/x/packages/core/src/agents/registry.test.ts +++ b/apps/x/packages/core/src/agents/registry.test.ts @@ -77,4 +77,22 @@ describe("agent registry", () => { expect(agent.instructions).toBe("Just instructions."); expect(agent.tools).toBeUndefined(); }); + + it("ids colliding with Object.prototype fall through to the repo, not the table", async () => { + // A plain-object lookup would resolve builtinAgents['constructor'] to + // the inherited Object constructor and crash on .build() before ever + // reaching the user-agents repo. Whether the repo then resolves or + // rejects depends on the environment; the contract under test is only + // that the table path is not taken. + for (const id of ["constructor", "toString", "valueOf", "hasOwnProperty"]) { + expect(hasWorkspaceContext(id)).toBe(false); + const outcome = await loadAgent(id).then( + () => null, + (error: unknown) => String(error), + ); + if (outcome !== null) { + expect(outcome).not.toMatch(/is not a function/); + } + } + }); }); diff --git a/apps/x/packages/core/src/agents/registry.ts b/apps/x/packages/core/src/agents/registry.ts index e8ab223e..9508ea7f 100644 --- a/apps/x/packages/core/src/agents/registry.ts +++ b/apps/x/packages/core/src/agents/registry.ts @@ -1,6 +1,6 @@ import type { z } from "zod"; -import { parse } from "yaml"; import { Agent } from "@x/shared/dist/agent.js"; +import { parseFrontmatter } from "../application/lib/parse-frontmatter.js"; import { buildCopilotAgent } from "../application/assistant/agent.js"; import { buildBackgroundTaskAgent } from "../background-tasks/agent.js"; import { buildLiveNoteAgent } from "../knowledge/live-note/agent.js"; @@ -30,21 +30,16 @@ interface BuiltinAgentDefinition { } // Prompt-file agents: instructions ship as a raw string whose optional YAML -// frontmatter carries agent config (tools, model). One loader replaces the -// five copy-pasted parsing blocks the ladder accumulated. +// frontmatter carries agent config (tools, model). Parsing goes through the +// same parseFrontmatter helper FSAgentsRepo uses for user agents, so builtin +// and user-defined agents can never drift on the same file format. export function agentFromRaw(id: string, raw: string): z.infer { - let agent: z.infer = { name: id, instructions: raw }; - if (raw.startsWith("---")) { - const end = raw.indexOf("\n---", 3); - if (end !== -1) { - const frontmatter = raw.slice(3, end).trim(); - const content = raw.slice(end + 4).trim(); - const yaml: unknown = parse(frontmatter); - const parsed = Agent.omit({ name: true, instructions: true }).parse(yaml); - agent = { ...agent, ...parsed, instructions: content }; - } + const { frontmatter, content } = parseFrontmatter(raw); + if (frontmatter === null) { + return { name: id, instructions: raw }; } - return agent; + const parsed = Agent.omit({ name: true, instructions: true }).parse(frontmatter); + return { name: id, ...parsed, instructions: content }; } function promptFileAgent(id: string, getRaw: () => string): BuiltinAgentDefinition { @@ -79,12 +74,18 @@ export function builtinAgentIds(): string[] { export function hasWorkspaceContext(agentId: string | null | undefined): boolean { return ( agentId != null && - builtinAgents[agentId]?.traits?.workspaceContext === true + Object.hasOwn(builtinAgents, agentId) && + builtinAgents[agentId].traits?.workspaceContext === true ); } export async function loadAgent(id: string): Promise> { - const builtin = builtinAgents[id]; + // Object.hasOwn: a plain lookup would traverse Object.prototype, so a + // user agent named "constructor"/"toString" would hit an inherited + // function instead of falling through to the repo. + const builtin = Object.hasOwn(builtinAgents, id) + ? builtinAgents[id] + : undefined; if (builtin) { return builtin.build(); } diff --git a/apps/x/packages/core/src/agents/runtime.ts b/apps/x/packages/core/src/agents/runtime.ts index 58dedcb5..ac68c3ee 100644 --- a/apps/x/packages/core/src/agents/runtime.ts +++ b/apps/x/packages/core/src/agents/runtime.ts @@ -302,12 +302,6 @@ ${sections.join('\n\n')} `; } -// System-prompt assembly lives in compose-instructions.ts (owned by the -// assembly layer); re-exported so legacy callers keep their import path. -export { - composeSystemInstructions, - type ComposeSystemInstructionsInput, -} from "./compose-instructions.js"; import { composeSystemInstructions } from "./compose-instructions.js"; export interface IAgentRuntime { @@ -647,10 +641,6 @@ function formatLlmStreamError(rawError: unknown): string { return lines.length ? lines.join("\n") : "Model stream error"; } -// Agent identity now lives in the registry (registry.ts); re-exported here -// so legacy callers keep their import path until the runs engine dies. -export { loadAgent }; - function formatBytes(bytes: number): string { if (bytes < 1024) return `${bytes} B`; if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)} KB`; diff --git a/apps/x/packages/core/src/application/assistant/capabilities/modes.ts b/apps/x/packages/core/src/application/assistant/capabilities/modes.ts index b3b88fd4..e8641747 100644 --- a/apps/x/packages/core/src/application/assistant/capabilities/modes.ts +++ b/apps/x/packages/core/src/application/assistant/capabilities/modes.ts @@ -14,32 +14,24 @@ import type { CapabilityContext, CapabilityDefinition } from "./types.js"; export const MODE_CAPABILITIES: readonly CapabilityDefinition[] = [ { id: "voice-input", - title: "Voice Input", - summary: "The user's message was transcribed from speech.", activation: "app", promptFragment: (ctx: CapabilityContext) => ctx.voiceInput ? VOICE_INPUT : null, }, { id: "video-mode", - title: "Video Mode (Live Camera)", - summary: "Webcam (and optionally screen-share) frames arrive with the user's messages.", activation: "app", promptFragment: (ctx: CapabilityContext) => ctx.videoMode ? VIDEO_MODE : null, }, { id: "coach-mode", - title: "Practice Session (Coach Mode)", - summary: "The user is rehearsing something performative and wants live coaching.", activation: "app", promptFragment: (ctx: CapabilityContext) => ctx.coachMode ? COACH_MODE : null, }, { id: "voice-output", - title: "Voice Output", - summary: "Responses must lead with tags for TTS.", activation: "app", promptFragment: (ctx: CapabilityContext) => ctx.voiceOutput === "summary" @@ -50,16 +42,12 @@ export const MODE_CAPABILITIES: readonly CapabilityDefinition[] = [ }, { id: "search", - title: "Search", - summary: "The user has requested a web search.", activation: "app", promptFragment: (ctx: CapabilityContext) => ctx.searchEnabled ? SEARCH : null, }, { id: "code-mode", - title: "Code Mode", - summary: "Coding work routes to the on-device coding agent selected by the composer chip.", activation: "app", promptFragment: (ctx: CapabilityContext) => { const { codeMode, codeCwd } = ctx; diff --git a/apps/x/packages/core/src/application/assistant/capabilities/types.ts b/apps/x/packages/core/src/application/assistant/capabilities/types.ts index 6a5c7129..a3ebb7d5 100644 --- a/apps/x/packages/core/src/application/assistant/capabilities/types.ts +++ b/apps/x/packages/core/src/application/assistant/capabilities/types.ts @@ -41,8 +41,12 @@ export interface CapabilityContext { export interface CapabilityDefinition { id: string; - title: string; - summary: string; + // Catalog metadata: required for model-activated entries (rendered in + // the loadSkill catalog); app/always entries omit them — nothing + // consumes them there, and stale copies of fragment headings would + // only drift into misinformation. + title?: string; + summary?: string; // Defaults to 'model' (the historical skill behavior). activation?: CapabilityActivation; // Lazy guidance returned by loadSkill (model activation). diff --git a/apps/x/packages/core/src/application/assistant/capabilities/workspace.ts b/apps/x/packages/core/src/application/assistant/capabilities/workspace.ts index 77fb7511..1864ff5c 100644 --- a/apps/x/packages/core/src/application/assistant/capabilities/workspace.ts +++ b/apps/x/packages/core/src/application/assistant/capabilities/workspace.ts @@ -1,26 +1,24 @@ import type { CapabilityContext, CapabilityDefinition } from "./types.js"; -// The always-activated workspace-context capability: agent notes and the +// The always-activated workspace-context capabilities: agent notes and the // user work directory, composed for agents with the workspaceContext trait // (the resolver loads both inputs and leaves them null for everyone else). -// Fragment text extracted verbatim from the historical composer; the golden -// snapshots in agents/compose-instructions.test.ts pin the bytes. +// Two independent records — the composer's own '\n\n' joining makes their +// output byte-identical to the historical fused block. Fragment text +// extracted verbatim from the historical composer; the golden snapshots in +// agents/compose-instructions.test.ts pin the bytes. -export const WORKSPACE_CONTEXT_CAPABILITY: CapabilityDefinition = { - id: "workspace-context", - title: "Workspace Context", - summary: "Agent notes and the user's chosen work directory.", +export const AGENT_NOTES_CAPABILITY: CapabilityDefinition = { + id: "agent-notes", activation: "always", - promptFragment: (ctx: CapabilityContext) => { - const parts: string[] = []; - if (ctx.agentNotesContext) { - parts.push(ctx.agentNotesContext); - } - if (ctx.userWorkDir) { - parts.push(WORK_DIR_TEMPLATE(ctx.userWorkDir)); - } - return parts.length > 0 ? parts.join("\n\n") : null; - }, + promptFragment: (ctx: CapabilityContext) => ctx.agentNotesContext, +}; + +export const WORK_DIRECTORY_CAPABILITY: CapabilityDefinition = { + id: "work-directory", + activation: "always", + promptFragment: (ctx: CapabilityContext) => + ctx.userWorkDir ? WORK_DIR_TEMPLATE(ctx.userWorkDir) : null, }; const WORK_DIR_TEMPLATE = (userWorkDir: string): string => `# User Work Directory 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 85b82e56..9848c53b 100644 --- a/apps/x/packages/core/src/application/assistant/skills/index.ts +++ b/apps/x/packages/core/src/application/assistant/skills/index.ts @@ -28,10 +28,15 @@ const CATALOG_PREFIX = "src/application/assistant/skills"; // A skill is the model-activated subset of the capability record // (capabilities/types.ts): lazy guidance the model pulls in via loadSkill, -// plus the BuiltinTools it owns. `id` doubles as the folder name. Tool names +// plus the BuiltinTools it owns. `id` doubles as the folder name; title and +// summary are required here because the catalog renders them. Tool names // are validated against the live catalog where descriptors are built // (loadSkill / the agent resolver); unknown names are dropped there. -type SkillDefinition = CapabilityDefinition & { content: string }; +type SkillDefinition = CapabilityDefinition & { + title: string; + summary: string; + content: string; +}; type ResolvedSkill = { id: string; diff --git a/apps/x/packages/core/src/runs/runs.ts b/apps/x/packages/core/src/runs/runs.ts index e61b2309..c79e8077 100644 --- a/apps/x/packages/core/src/runs/runs.ts +++ b/apps/x/packages/core/src/runs/runs.ts @@ -11,7 +11,7 @@ import { IRunsLock } from "./lock.js"; import { forceCloseAllMcpClients } from "../mcp/mcp.js"; import { extractCommandNames } from "../application/lib/command-executor.js"; import { addFileAccessGrant, addToSecurityConfig } from "../config/security.js"; -import { loadAgent } from "../agents/runtime.js"; +import { loadAgent } from "../agents/registry.js"; import { getDefaultModelAndProvider } from "../models/defaults.js"; export async function createRun(opts: z.infer): Promise> { 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 e33bb3aa..1f7f46ec 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 @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { z } from "zod"; import type { Agent } from "@x/shared/dist/agent.js"; -import { composeSystemInstructions } from "../../agents/runtime.js"; +import { composeSystemInstructions } from "../../agents/compose-instructions.js"; import type { BuiltinTools } from "../../application/lib/builtin-tools.js"; import { RealAgentResolver } from "./real-agent-resolver.js";