diff --git a/apps/x/packages/core/src/agents/compose-instructions.ts b/apps/x/packages/core/src/agents/compose-instructions.ts index 2b30402e..3a43c964 100644 --- a/apps/x/packages/core/src/agents/compose-instructions.ts +++ b/apps/x/packages/core/src/agents/compose-instructions.ts @@ -3,7 +3,10 @@ import { AGENT_NOTES_CAPABILITY, WORK_DIRECTORY_CAPABILITY, } from "../application/assistant/capabilities/workspace.js"; -import type { CapabilityContext } from "../application/assistant/capabilities/types.js"; +import type { + CapabilityContext, + ModeFlags, +} from "../application/assistant/capabilities/types.js"; // Everything that composes into the system prompt, in composition order. const PROMPT_CAPABILITIES = [ @@ -33,52 +36,33 @@ If Middle pane state is note, the supplied path and content are available so you If Middle pane state is browser, only the URL and page title are supplied; the page content itself is NOT included. If you need the page content to answer, use the browser tools available to you to read the page. The user may or may not be talking about this page. Only reference or act on this page when the user's message clearly relates to it, such as "this page", "this article", "what I'm looking at", "this site", or "summarize this". For unrelated questions, ignore this page entirely and answer normally. Do not mention that you can see the browser unless it is relevant to the answer.`; -export interface ComposeSystemInstructionsInput { +// The mode flags come straight from the shared ModeFlags shape (all +// required — callers normalize via ModeFlags.parse or pass explicit +// values), so a mode added to the schema is a compile error at every call +// site until it is threaded through, never a silently-absent prompt block. +export type ComposeSystemInstructionsInput = { instructions: string; agentNotesContext: string | null; userWorkDir: string | null; - voiceInput: boolean; - voiceOutput: 'summary' | 'full' | null; - searchEnabled: boolean; - codeMode: 'claude' | 'codex' | null; - codeCwd: string | null; - // Optional so legacy callers (old streamAgent path) are unaffected. - videoMode?: boolean; - coachMode?: boolean; -} +} & ModeFlags; -// System-prompt assembly, extracted verbatim from streamAgent so the new turn -// runtime's agent resolver composes byte-identical prompts. Pure: callers -// load agent notes / work dir themselves. +// System-prompt assembly: base instructions + hidden-user-context + the +// capability fragments. Pure: callers load agent notes / work dir +// themselves. export function composeSystemInstructions({ instructions, agentNotesContext, userWorkDir, - voiceInput, - voiceOutput, - searchEnabled, - codeMode, - codeCwd, - videoMode, - coachMode, + ...modeFlags }: ComposeSystemInstructionsInput): string { 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, - }; + // lives with the capability records; the rest-spread means new schema + // keys flow through without a hand-maintained copy list. + const ctx: CapabilityContext = { agentNotesContext, userWorkDir, ...modeFlags }; for (const capability of PROMPT_CAPABILITIES) { - const fragment = capability.promptFragment?.(ctx); + const fragment = capability.promptFragment(ctx); if (fragment) { composed += `\n\n${fragment}`; } diff --git a/apps/x/packages/core/src/agents/registry.ts b/apps/x/packages/core/src/agents/registry.ts index 9508ea7f..f09a5034 100644 --- a/apps/x/packages/core/src/agents/registry.ts +++ b/apps/x/packages/core/src/agents/registry.ts @@ -10,6 +10,7 @@ import { getRaw as getLabelingAgentRaw } from "../knowledge/labeling_agent.js"; import { getRaw as getNoteTaggingAgentRaw } from "../knowledge/note_tagging_agent.js"; import { getRaw as getInlineTaskAgentRaw } from "../knowledge/inline_task_agent.js"; import { getRaw as getAgentNotesAgentRaw } from "../knowledge/agent_notes_agent.js"; +import { lazyResolve } from "../di/lazy-resolve.js"; import type { IAgentsRepo } from "./repo.js"; // The registry of built-in agents: one table instead of the historical @@ -89,9 +90,7 @@ export async function loadAgent(id: string): Promise> { if (builtin) { return builtin.build(); } - // User-defined agents. The container is imported lazily so this module - // adds no static edge into the DI graph (mirrors spawn-agent). - const { default: container } = await import("../di/container.js"); - const repo = container.resolve("agentsRepo"); + // User-defined agents (lazyResolve: no static DI edge from this module). + const repo = await lazyResolve("agentsRepo"); return repo.fetch(id); } diff --git a/apps/x/packages/core/src/agents/runtime.ts b/apps/x/packages/core/src/agents/runtime.ts index ac68c3ee..d4d9b9c3 100644 --- a/apps/x/packages/core/src/agents/runtime.ts +++ b/apps/x/packages/core/src/agents/runtime.ts @@ -12,6 +12,7 @@ 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 { hasWorkspaceContext, loadAgent } from "./registry.js"; +import { loadWorkspaceContext } from "./workspace-context.js"; import { isBlocked, extractCommandNames } from "../application/lib/command-executor.js"; import { getFileAccessAllowList, type FileAccessGrant, type FileAccessOperation } from "../config/security.js"; import { resolveFilePathForPermission } from "../filesystem/files.js"; @@ -31,14 +32,6 @@ import { captureLlmUsage } from "../analytics/usage.js"; import { enterUseCase, withUseCase, type UseCase } from "../analytics/use_case.js"; import { classifyToolPermissions, type AutoPermissionCandidate } from "../security/auto-permission-classifier.js"; -const AGENT_NOTES_DIR = path.join(WorkDir, 'knowledge', 'Agent Notes'); - -// Work directory is scoped per run (per chat). Each run gets its own sidecar -// config file so setting it in one chat does not leak into others. -function workDirConfigFile(runId: string): string { - return path.join(WorkDir, 'config', `workdir-${runId}.json`); -} - type ToolPermissionMetadataValue = z.infer; function isPathInside(parent: string, child: string): boolean { @@ -164,71 +157,6 @@ export async function getToolPermissionMetadata( }; } -export function loadUserWorkDir(runId: string): string | null { - try { - const file = workDirConfigFile(runId); - if (!fs.existsSync(file)) return null; - const raw = fs.readFileSync(file, 'utf-8'); - const parsed = JSON.parse(raw) as { path?: unknown }; - const value = typeof parsed.path === 'string' ? parsed.path.trim() : ''; - return value || null; - } catch { - return null; - } -} - -export function loadAgentNotesContext(): string | null { - const sections: string[] = []; - - const userFile = path.join(AGENT_NOTES_DIR, 'user.md'); - const prefsFile = path.join(AGENT_NOTES_DIR, 'preferences.md'); - - try { - if (fs.existsSync(userFile)) { - const content = fs.readFileSync(userFile, 'utf-8').trim(); - if (content) { - sections.push(`## About the User\nThese are notes you took about the user in previous chats.\n\n${content}`); - } - } - } catch { /* ignore */ } - - try { - if (fs.existsSync(prefsFile)) { - const content = fs.readFileSync(prefsFile, 'utf-8').trim(); - if (content) { - sections.push(`## User Preferences\nThese are notes you took on their general preferences.\n\n${content}`); - } - } - } catch { /* ignore */ } - - // List other Agent Notes files for on-demand access - const otherFiles: string[] = []; - const skipFiles = new Set(['user.md', 'preferences.md', 'inbox.md']); - try { - if (fs.existsSync(AGENT_NOTES_DIR)) { - function listMdFiles(dir: string, prefix: string) { - for (const entry of fs.readdirSync(dir)) { - const fullPath = path.join(dir, entry); - const stat = fs.statSync(fullPath); - if (stat.isDirectory()) { - listMdFiles(fullPath, `${prefix}${entry}/`); - } else if (entry.endsWith('.md') && !skipFiles.has(`${prefix}${entry}`)) { - otherFiles.push(`${prefix}${entry}`); - } - } - } - listMdFiles(AGENT_NOTES_DIR, ''); - } - } catch { /* ignore */ } - - if (otherFiles.length > 0) { - sections.push(`## More Specific Preferences\nFor more specific preferences, you can read these files using file-readText. Only read them when relevant to the current task.\n\n${otherFiles.map(f => `- knowledge/Agent Notes/${f}`).join('\n')}`); - } - - if (sections.length === 0) return null; - return `# Agent Memory\n\n${sections.join('\n\n')}`; -} - function formatCurrentDateTime(now: Date): string { return now.toLocaleString('en-US', { weekday: 'long', @@ -1312,16 +1240,19 @@ export async function* streamAgent({ loopLogger.log('running llm turn'); // stream agent response and build message const messageBuilder = new StreamStepMessageBuilder(); - const composeCopilotContext = hasWorkspaceContext(state.agentName); + const workspace = loadWorkspaceContext(state.agentName, runId); const instructionsWithDateTime = composeSystemInstructions({ instructions: agent.instructions, - agentNotesContext: composeCopilotContext ? loadAgentNotesContext() : null, - userWorkDir: composeCopilotContext ? loadUserWorkDir(runId) : null, + agentNotesContext: workspace.agentNotesContext, + userWorkDir: workspace.userWorkDir, voiceInput, voiceOutput, searchEnabled, codeMode, codeCwd, + // The legacy runs engine never composes video/coach modes. + videoMode: false, + coachMode: false, }); let streamError: string | null = null; for await (const event of streamLlm( diff --git a/apps/x/packages/core/src/agents/spawn-agent.ts b/apps/x/packages/core/src/agents/spawn-agent.ts index 432edaab..72cbf26c 100644 --- a/apps/x/packages/core/src/agents/spawn-agent.ts +++ b/apps/x/packages/core/src/agents/spawn-agent.ts @@ -219,13 +219,13 @@ export async function runSpawnedAgent( async function resolveServices(): Promise< NonNullable > { - const { default: container } = await import("../di/container.js"); + const { lazyResolve } = await import("../di/lazy-resolve.js"); return { turnRuntime: - container.resolve( + await lazyResolve( "turnRuntime", ), - headlessRunner: container.resolve< + headlessRunner: await lazyResolve< import("./headless.js").IHeadlessAgentRunner >("headlessAgentRunner"), }; diff --git a/apps/x/packages/core/src/agents/workspace-context.test.ts b/apps/x/packages/core/src/agents/workspace-context.test.ts new file mode 100644 index 00000000..a185b6db --- /dev/null +++ b/apps/x/packages/core/src/agents/workspace-context.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from "vitest"; +import { loadWorkspaceContext } from "./workspace-context.js"; + +const loaders = { + loadNotes: () => "# Agent Memory\n\nnotes", + loadWorkDir: (key: string) => (key === "chat-1" ? "/Users/me/work" : null), +}; + +describe("loadWorkspaceContext", () => { + it("loads notes and work dir for workspaceContext-trait agents", () => { + expect(loadWorkspaceContext("copilot", "chat-1", loaders)).toEqual({ + agentNotesContext: "# Agent Memory\n\nnotes", + userWorkDir: "/Users/me/work", + }); + // rowboatx is the copilot alias. + expect(loadWorkspaceContext("rowboatx", "chat-1", loaders).userWorkDir).toBe( + "/Users/me/work", + ); + }); + + it("returns nulls for non-workspace agents even when the loaders would yield values", () => { + // THE gate this module exists for: a caller cannot leak the user's + // agent-memory into a background/knowledge agent's prompt by + // forgetting a trait check — the check lives here. + for (const agentId of ["background-task-agent", "note_creation", "some-user-agent"]) { + expect(loadWorkspaceContext(agentId, "chat-1", loaders)).toEqual({ + agentNotesContext: null, + userWorkDir: null, + }); + } + expect(loadWorkspaceContext(null, "chat-1", loaders).agentNotesContext).toBeNull(); + expect(loadWorkspaceContext(undefined, "chat-1", loaders).agentNotesContext).toBeNull(); + }); + + it("skips the work-dir lookup without a key", () => { + expect(loadWorkspaceContext("copilot", null, loaders)).toEqual({ + agentNotesContext: "# Agent Memory\n\nnotes", + userWorkDir: null, + }); + }); +}); diff --git a/apps/x/packages/core/src/agents/workspace-context.ts b/apps/x/packages/core/src/agents/workspace-context.ts new file mode 100644 index 00000000..ea0039e1 --- /dev/null +++ b/apps/x/packages/core/src/agents/workspace-context.ts @@ -0,0 +1,115 @@ +import fs from "fs"; +import path from "path"; +import { WorkDir } from "../config/config.js"; +import { hasWorkspaceContext } from "./registry.js"; + +// Workspace context for agent assembly: agent notes (global) and the user's +// per-chat work directory. loadWorkspaceContext is THE chokepoint — the +// workspaceContext trait gate lives inside it, so no assembly site can +// forget the gate and leak the user's agent-memory into a non-workspace +// agent's prompt (or silently omit it for the copilot). Loaders extracted +// verbatim from the legacy runtime file. + +const AGENT_NOTES_DIR = path.join(WorkDir, 'knowledge', 'Agent Notes'); + +// Work directory is scoped per run (per chat). Each run gets its own sidecar +// config file so setting it in one chat does not leak into others. +function workDirConfigFile(runId: string): string { + return path.join(WorkDir, 'config', `workdir-${runId}.json`); +} + +export function loadUserWorkDir(runId: string): string | null { + try { + const file = workDirConfigFile(runId); + if (!fs.existsSync(file)) return null; + const raw = fs.readFileSync(file, 'utf-8'); + const parsed = JSON.parse(raw) as { path?: unknown }; + const value = typeof parsed.path === 'string' ? parsed.path.trim() : ''; + return value || null; + } catch { + return null; + } +} + +export function loadAgentNotesContext(): string | null { + const sections: string[] = []; + + const userFile = path.join(AGENT_NOTES_DIR, 'user.md'); + const prefsFile = path.join(AGENT_NOTES_DIR, 'preferences.md'); + + try { + if (fs.existsSync(userFile)) { + const content = fs.readFileSync(userFile, 'utf-8').trim(); + if (content) { + sections.push(`## About the User\nThese are notes you took about the user in previous chats.\n\n${content}`); + } + } + } catch { /* ignore */ } + + try { + if (fs.existsSync(prefsFile)) { + const content = fs.readFileSync(prefsFile, 'utf-8').trim(); + if (content) { + sections.push(`## User Preferences\nThese are notes you took on their general preferences.\n\n${content}`); + } + } + } catch { /* ignore */ } + + // List other Agent Notes files for on-demand access + const otherFiles: string[] = []; + const skipFiles = new Set(['user.md', 'preferences.md', 'inbox.md']); + try { + if (fs.existsSync(AGENT_NOTES_DIR)) { + function listMdFiles(dir: string, prefix: string) { + for (const entry of fs.readdirSync(dir)) { + const fullPath = path.join(dir, entry); + const stat = fs.statSync(fullPath); + if (stat.isDirectory()) { + listMdFiles(fullPath, `${prefix}${entry}/`); + } else if (entry.endsWith('.md') && !skipFiles.has(`${prefix}${entry}`)) { + otherFiles.push(`${prefix}${entry}`); + } + } + } + listMdFiles(AGENT_NOTES_DIR, ''); + } + } catch { /* ignore */ } + + if (otherFiles.length > 0) { + sections.push(`## More Specific Preferences\nFor more specific preferences, you can read these files using file-readText. Only read them when relevant to the current task.\n\n${otherFiles.map(f => `- knowledge/Agent Notes/${f}`).join('\n')}`); + } + + if (sections.length === 0) return null; + return `# Agent Memory\n\n${sections.join('\n\n')}`; +} + +export interface WorkspaceContext { + agentNotesContext: string | null; + userWorkDir: string | null; +} + +const NO_WORKSPACE: WorkspaceContext = { + agentNotesContext: null, + userWorkDir: null, +}; + +// workDirKey is the per-chat work-directory sidecar key: the composition's +// workDirId on the turn runtime, the runId on the legacy engine. +export function loadWorkspaceContext( + agentId: string | null | undefined, + workDirKey: string | null | undefined, + loaders: { + loadNotes?: typeof loadAgentNotesContext; + loadWorkDir?: typeof loadUserWorkDir; + } = {}, +): WorkspaceContext { + if (!hasWorkspaceContext(agentId)) { + return NO_WORKSPACE; + } + const loadNotes = loaders.loadNotes ?? loadAgentNotesContext; + const loadWorkDir = loaders.loadWorkDir ?? loadUserWorkDir; + return { + agentNotesContext: loadNotes(), + userWorkDir: workDirKey ? loadWorkDir(workDirKey) : null, + }; +} 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 e8641747..5d76c4c0 100644 --- a/apps/x/packages/core/src/application/assistant/capabilities/modes.ts +++ b/apps/x/packages/core/src/application/assistant/capabilities/modes.ts @@ -1,4 +1,4 @@ -import type { CapabilityContext, CapabilityDefinition } from "./types.js"; +import type { CapabilityContext, EagerCapability } from "./types.js"; // The app-activated capabilities: the modes the app (not the model) toggles — // facts about the world like "the camera is on" whose guidance must be in the @@ -11,7 +11,7 @@ import type { CapabilityContext, CapabilityDefinition } from "./types.js"; // code_agent_run/launch-code-task, but they stay in COPILOT_BASE_TOOLS until // the legacy runs engine (which cannot attach tools mid-run) is retired. -export const MODE_CAPABILITIES: readonly CapabilityDefinition[] = [ +export const MODE_CAPABILITIES: readonly EagerCapability[] = [ { id: "voice-input", activation: "app", 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 a3ebb7d5..f37035d4 100644 --- a/apps/x/packages/core/src/application/assistant/capabilities/types.ts +++ b/apps/x/packages/core/src/application/assistant/capabilities/types.ts @@ -11,54 +11,88 @@ // system prompt from token zero and tools attach at assembly. // - 'always' — unconditional contributions (workspace context traits). // +// The two shapes are a discriminated union so the design's rules are +// unrepresentable to break: a model capability MUST carry catalog metadata +// and lazy content and CANNOT carry an eager fragment; an app/always +// capability MUST carry a fragment and never appears in the catalog. +// // Trust boundary: disk-loaded skills (~/.rowboat/skills, ~/.agents/skills) -// are structurally limited to the 'model' subset — eager prompt fragments -// and app activation are bundled-only powers. A model-activated skill's -// prose lands in conversation because the model chose to read it; a disk -// file injecting into every turn's system prompt would be a standing +// are typed as ModelCapability, so eager prompt fragments and app activation +// are structurally bundled-only powers. A model-activated skill's prose +// lands in conversation because the model chose to read it; a disk file +// injecting into every turn's system prompt would be a standing // prompt-injection channel. +import { z } from "zod"; + export type CapabilityActivation = "model" | "app" | "always"; -// Inputs an eager fragment may read. All fields mirror persisted -// RequestedAgent.overrides.composition values, resolved before composition — -// a fragment must be a pure function of this context so composed prompts -// stay byte-identical for identical inputs (provider prefix caching and -// agent-snapshot inheritance both depend on it). -export interface CapabilityContext { +// THE single source of truth for the app-toggled mode flags. Every key is a +// persisted RequestedAgent.overrides.composition value; defaults make the +// parsed output fully concrete, so the wire shape (all-optional, what turn +// files carry), the resolver's parse result, and the composition context are +// one declaration. Adding a mode = one key here + one record in modes.ts; +// forgetting anything else is a compile error, not a silently-absent prompt +// block. Every key alters system-prompt bytes, so prompt-affecting inputs +// must stay session-sticky (prefix caching). +export const ModeFlags = z.object({ + voiceInput: z.boolean().default(false), + voiceOutput: z.enum(["summary", "full"]).nullable().default(null), + searchEnabled: z.boolean().default(false), + codeMode: z.enum(["claude", "codex"]).nullable().default(null), + codeCwd: z.string().nullable().default(null), + videoMode: z.boolean().default(false), + coachMode: z.boolean().default(false), +}); +export type ModeFlags = z.infer; + +// Inputs an eager fragment may read: the mode flags plus resolved workspace +// context. A fragment must be a pure function of this context so composed +// prompts stay byte-identical for identical inputs (provider prefix caching +// and agent-snapshot inheritance both depend on it). +export interface CapabilityContext extends ModeFlags { // Workspace context (workspaceContext trait agents only; the resolver // loads these and leaves them null for everyone else). agentNotesContext: string | null; userWorkDir: string | null; - voiceInput: boolean; - voiceOutput: "summary" | "full" | null; - searchEnabled: boolean; - codeMode: "claude" | "codex" | null; - codeCwd: string | null; - videoMode: boolean; - coachMode: boolean; } -export interface CapabilityDefinition { +// Model-activated (a "skill"): advertised in the loadSkill catalog, loaded +// by the model's judgment, guidance delivered lazily as a tool result. +export interface ModelCapability { id: 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). - content?: string; - // BuiltinTools keys this capability owns. + // The discriminant; omitted means 'model' (the historical default). + activation?: "model"; + // Catalog metadata — the loadSkill catalog renders both. + title: string; + summary: string; + // Lazy guidance returned by loadSkill. + content: string; + // BuiltinTools keys this capability owns; attached mid-turn on load. tools?: string[]; - // Eager system-prompt fragment (app/always activation). Returns null - // when the capability contributes nothing for this context. MUST be - // pure: same context, same bytes. - promptFragment?: (ctx: CapabilityContext) => string | null; + // Catalog visibility: when false, the capability is omitted from the + // loadSkill catalog (e.g. slack when no workspace is connected). + // Deliberately NOT a loadSkill fence — an explicitly-requested id still + // resolves, matching the historical excludeIds behavior. Defaults true. + availability?: () => Promise | boolean; } -export function isModelActivated(def: Pick): boolean { +// App/always-activated: an eager system-prompt contribution composed at +// assembly time. Bundled-only (see trust boundary above). +export interface EagerCapability { + id: string; + activation: "app" | "always"; + // Returns null when the capability contributes nothing for this + // context. MUST be pure: same context, same bytes. + promptFragment: (ctx: CapabilityContext) => string | null; + // BuiltinTools keys this capability owns; attached at assembly. + tools?: string[]; +} + +export type CapabilityDefinition = ModelCapability | EagerCapability; + +export function isModelActivated( + def: CapabilityDefinition, +): def is ModelCapability { return def.activation === undefined || def.activation === "model"; } 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 1864ff5c..bd8db4fd 100644 --- a/apps/x/packages/core/src/application/assistant/capabilities/workspace.ts +++ b/apps/x/packages/core/src/application/assistant/capabilities/workspace.ts @@ -1,4 +1,4 @@ -import type { CapabilityContext, CapabilityDefinition } from "./types.js"; +import type { CapabilityContext, EagerCapability } from "./types.js"; // The always-activated workspace-context capabilities: agent notes and the // user work directory, composed for agents with the workspaceContext trait @@ -8,13 +8,13 @@ import type { CapabilityContext, CapabilityDefinition } from "./types.js"; // extracted verbatim from the historical composer; the golden snapshots in // agents/compose-instructions.test.ts pin the bytes. -export const AGENT_NOTES_CAPABILITY: CapabilityDefinition = { +export const AGENT_NOTES_CAPABILITY: EagerCapability = { id: "agent-notes", activation: "always", promptFragment: (ctx: CapabilityContext) => ctx.agentNotesContext, }; -export const WORK_DIRECTORY_CAPABILITY: CapabilityDefinition = { +export const WORK_DIRECTORY_CAPABILITY: EagerCapability = { id: "work-directory", activation: "always", promptFragment: (ctx: CapabilityContext) => diff --git a/apps/x/packages/core/src/application/assistant/instructions.ts b/apps/x/packages/core/src/application/assistant/instructions.ts index f4009352..d5fb4953 100644 --- a/apps/x/packages/core/src/application/assistant/instructions.ts +++ b/apps/x/packages/core/src/application/assistant/instructions.ts @@ -1,4 +1,4 @@ -import { skillCatalog, buildSkillCatalog } from "./skills/index.js"; +import { skillCatalog, buildAvailableSkillCatalog } from "./skills/index.js"; import { getRuntimeContext, getRuntimeContextPrompt } from "./runtime-context.js"; import { composioAccountsRepo } from "../../composio/repo.js"; import { isConfigured as isComposioConfigured } from "../../composio/client.js"; @@ -409,13 +409,11 @@ export async function buildCopilotInstructions(): Promise { // knowledge sources unavailable — fall back to no channel hint } } - const excludeIds: string[] = []; - if (!composioEnabled) excludeIds.push('composio-integration'); - if (!codeModeEnabled) excludeIds.push('code-with-agents'); - if (!slackConnected) excludeIds.push('slack'); - // Always build from the live skill set so disk skills added/removed at - // runtime (after refreshDiskSkills + cache invalidation) are reflected. - const catalog = buildSkillCatalog({ excludeIds }); + // Catalog membership is each skill's own availability() (connection + // gates declared on the entries in skills/index.ts), evaluated against + // the live skill set so disk skills added/removed at runtime (after + // refreshDiskSkills + cache invalidation) are reflected. + const catalog = await buildAvailableSkillCatalog(); const baseInstructions = buildStaticInstructions(composioEnabled, catalog, codeModeEnabled, slackConnected, slackChannelsHint, googleConnected); const composioPrompt = await getComposioToolsPrompt(slackConnected, googleConnected); cachedInstructions = composioPrompt 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 8aec5d85..56b6670d 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 @@ -2,24 +2,22 @@ import fs from "node:fs"; import path from "node:path"; import { homedir } from "node:os"; import { WorkDir } from "../../../config/config.js"; +import type { ModelCapability } from "../capabilities/types.js"; import { splitFrontmatter } from "../../lib/parse-frontmatter.js"; /** - * A skill discovered on disk. Matches the bundled SkillDefinition shape - * (id/title/summary/content) plus provenance about where it came from. + * A skill discovered on disk: structurally the model-activated capability + * variant (id = slugified folder name, title = frontmatter `name`, summary = + * frontmatter `description`, content = full raw SKILL.md text, tools = + * frontmatter `tools:`/`allowed-tools:` BuiltinTools names — validated where + * descriptors are built; unknown names dropped there with a warning) plus + * provenance. Typed as ModelCapability on purpose: disk content can never + * carry app/always activation or an eager prompt fragment — that is the + * trust boundary (capabilities/types.ts), enforced by the type system. */ -export type DiskSkill = { - id: string; // slugified folder name - title: string; // frontmatter `name` - summary: string; // frontmatter `description` - content: string; // full raw SKILL.md text +export type DiskSkill = ModelCapability & { 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 diff --git a/apps/x/packages/core/src/application/assistant/skills/index.test.ts b/apps/x/packages/core/src/application/assistant/skills/index.test.ts index 02f38e54..71056630 100644 --- a/apps/x/packages/core/src/application/assistant/skills/index.test.ts +++ b/apps/x/packages/core/src/application/assistant/skills/index.test.ts @@ -147,24 +147,85 @@ describe("skills index (disk skill merge layer)", () => { }); }); -describe("capability record (types)", () => { - it("defaults to model activation — every bundled skill is model-activated", async () => { +describe("capability activation boundary", () => { + it("defaults to model activation; eager variants are excluded", async () => { const { isModelActivated } = await import("../capabilities/types.js"); - // Bundled definitions carry no activation field today; they are the - // model-activated subset by default and must stay in the catalog. - expect(isModelActivated({})).toBe(true); - expect(isModelActivated({ activation: "model" })).toBe(true); - expect(isModelActivated({ activation: "app" })).toBe(false); - expect(isModelActivated({ activation: "always" })).toBe(false); + const { MODE_CAPABILITIES } = await import("../capabilities/modes.js"); + const modelSkill = { + id: "m1", + title: "M1", + summary: "model skill", + content: "guidance", + }; + expect(isModelActivated(modelSkill)).toBe(true); + expect(isModelActivated({ ...modelSkill, activation: "model" as const })).toBe(true); + for (const eager of MODE_CAPABILITIES) { + expect(isModelActivated(eager)).toBe(false); + } }); - it("keeps app/always capabilities out of the loadSkill catalog", async () => { + it("fences the catalog and the tool lookup against eager capabilities", async () => { + // The real failure mode this pins: a future commit merges eager + // capabilities into the shared entry list — the catalog must hide them + // AND the loadSkill tool-attachment path must refuse them (hiding the + // menu entry is not enough; the kitchen must refuse to serve it). + const { buildCatalogFromEntries, toolNamesFromEntries } = await import("./index.js"); + const { MODE_CAPABILITIES } = await import("../capabilities/modes.js"); + const eager = { ...MODE_CAPABILITIES[0], tools: ["file-writeText"] }; + const model = { + id: "real-skill", + title: "Real Skill", + summary: "a genuine model-activated skill", + content: "guidance", + tools: ["file-readText"], + catalogPath: "src/application/assistant/skills/real-skill/skill.ts", + }; + const mixed = [model, eager]; + + const catalog = buildCatalogFromEntries(mixed); + expect(catalog).toContain("real-skill"); + expect(catalog).not.toContain(eager.id); + + expect(toolNamesFromEntries(mixed, "real-skill")).toEqual(["file-readText"]); + // The eager capability's tools must NOT attach via the loadSkill path, + // even though the entry is present in the list and declares tools. + expect(toolNamesFromEntries(mixed, eager.id)).toEqual([]); + }); + + it("every advertised catalog id resolves via loadSkill", async () => { const { buildSkillCatalog, availableSkills } = await import("./index.js"); const catalog = buildSkillCatalog(); - // Every advertised id resolves via loadSkill (the catalog and the - // resolver operate on the same model-activated subset). for (const id of availableSkills) { expect(catalog).toContain(id); } }); }); + +describe("availability (catalog visibility)", () => { + it("drops unavailable entries from the catalog but keeps ungated ones", async () => { + const { filterAvailableEntries, buildCatalogFromEntries } = await import("./index.js"); + const entries = [ + { id: "always-on", title: "Always", summary: "s", content: "c" }, + { id: "connected", title: "Connected", summary: "s", content: "c", availability: () => true }, + { id: "disconnected", title: "Disconnected", summary: "s", content: "c", availability: async () => false }, + ]; + const available = await filterAvailableEntries(entries); + expect(available.map((e) => e.id)).toEqual(["always-on", "connected"]); + + const catalog = buildCatalogFromEntries(available); + expect(catalog).toContain("always-on"); + expect(catalog).toContain("connected"); + expect(catalog).not.toContain("disconnected"); + }); + + it("the connection-gated bundled skills declare availability", async () => { + const skills = await import("./index.js"); + const catalog = skills.buildSkillCatalog(); + // The ungated builder still lists them (loadSkill resolves explicit ids + // regardless of availability — visibility gating is catalog-only). + for (const id of ["composio-integration", "code-with-agents", "slack"]) { + expect(catalog).toContain(id); + expect(skills.resolveSkill(id)).not.toBeNull(); + } + }); +}); 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 9848c53b..d21c7d47 100644 --- a/apps/x/packages/core/src/application/assistant/skills/index.ts +++ b/apps/x/packages/core/src/application/assistant/skills/index.ts @@ -1,6 +1,10 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; -import { isModelActivated, type CapabilityDefinition } from "../capabilities/types.js"; +import { + isModelActivated, + type CapabilityDefinition, + type ModelCapability, +} from "../capabilities/types.js"; import { loadDiskSkills } from "./disk-loader.js"; import builtinToolsSkill from "./builtin-tools/skill.js"; import deletionGuardrailsSkill from "./deletion-guardrails/skill.js"; @@ -26,17 +30,12 @@ const CATALOG_PREFIX = "src/application/assistant/skills"; // console.log(liveNoteSkill); -// A skill is the model-activated subset of the capability record +// A skill IS the model-activated variant 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; title and -// summary are required here because the catalog renders them. Tool names +// plus the BuiltinTools it owns. `id` doubles as the folder name. Tool names // are validated against the live catalog where descriptors are built // (loadSkill / the agent resolver); unknown names are dropped there. -type SkillDefinition = CapabilityDefinition & { - title: string; - summary: string; - content: string; -}; +type SkillDefinition = ModelCapability; type ResolvedSkill = { id: string; @@ -105,6 +104,7 @@ const definitions: SkillDefinition[] = [ }, { id: "composio-integration", + availability: isComposioAvailable, 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, @@ -132,6 +132,7 @@ const definitions: SkillDefinition[] = [ }, { id: "code-with-agents", + availability: isCodeModeAvailable, title: "Code with Agents", summary: "Write code, build projects, create scripts, or fix bugs by delegating to Claude Code or Codex.", content: codeWithAgentsSkill, @@ -179,9 +180,8 @@ const definitions: SkillDefinition[] = [ tools: ["browser-control", "load-browser-skill"], }, { - // Excluded from the catalog when Slack isn't connected (see - // buildCopilotInstructions excludeIds), mirroring composio-integration. id: "slack", + availability: isSlackAvailable, title: "Slack", summary: "Read, search, and send Slack messages via the agent-slack CLI — catch up on channels, summarize threads, list users. Load FIRST for ANY Slack request; Slack is connected natively, never through Composio.", content: slackSkill, @@ -238,21 +238,83 @@ function getSkillEntries(): SkillEntry[] { return [...bundledEntries, ...diskEntries]; } +// ---- Availability checks (catalog visibility) ---- +// Connection-state gates for the catalog, evaluated per build. Repos resolve +// lazily so this module keeps no static edge into the DI graph; any failure +// reads as "not available" (the historical default). + +async function isComposioAvailable(): Promise { + try { + const { isConfigured } = await import("../../../composio/client.js"); + return await isConfigured(); + } catch { + return false; + } +} + +async function isCodeModeAvailable(): Promise { + try { + const { lazyResolve } = await import("../../../di/lazy-resolve.js"); + const repo = await lazyResolve("codeModeConfigRepo"); + return (await repo.getConfig()).enabled; + } catch { + return false; + } +} + +async function isSlackAvailable(): Promise { + try { + const { lazyResolve } = await import("../../../di/lazy-resolve.js"); + const repo = await lazyResolve("slackConfigRepo"); + const config = await repo.getConfig(); + return config.enabled && config.workspaces.length > 0; + } catch { + return false; + } +} + +// Pure availability filter over an explicit entry list (exported for tests): +// entries without an availability check are kept; checks are evaluated +// concurrently and a false drops the entry from the CATALOG only (loadSkill +// still resolves explicit ids, matching the historical excludeIds behavior). +export async function filterAvailableEntries Promise | boolean }>( + entries: readonly T[], +): Promise { + const verdicts = await Promise.all( + entries.map((entry) => (entry.availability ? entry.availability() : true)), + ); + return entries.filter((_, i) => verdicts[i]); +} + +// The catalog restricted to currently-available skills — what +// buildCopilotInstructions embeds in the system prompt. +export async function buildAvailableSkillCatalog(): Promise { + return buildCatalogFromEntries(await filterAvailableEntries(getSkillEntries())); +} + /** - * Build a skill catalog string, optionally excluding specific skills by ID. - * Reads the live skill set, so it reflects disk skills added/removed at runtime. + * Build a skill catalog string from the live skill set (availability + * ignored), so it reflects disk skills added/removed at runtime. The + * system prompt uses buildAvailableSkillCatalog instead. */ -export function buildSkillCatalog(options?: { excludeIds?: string[] }): string { - // The catalog advertises the model-activated subset only: app/always - // capabilities are composed into the system prompt by the assembly layer, - // not offered for loadSkill. - const modelEntries = getSkillEntries().filter(isModelActivated); - const entries = options?.excludeIds - ? modelEntries.filter(e => !options.excludeIds!.includes(e.id)) - : modelEntries; +export function buildSkillCatalog(): string { + return buildCatalogFromEntries(getSkillEntries()); +} + +// Pure catalog builder over an explicit entry list (exported for the +// activation-boundary tests). The catalog advertises the model-activated +// subset only: app/always capabilities are composed into the system prompt +// by the assembly layer, never offered for loadSkill. +export function buildCatalogFromEntries( + all: ReadonlyArray, +): string { + // The type-guard filter must keep the catalogPath intersection alive. + const entries = all.filter( + (e): e is ModelCapability & { catalogPath?: string } => isModelActivated(e), + ); const sections = entries.map((entry) => [ `## ${entry.title}`, - `- **Skill file:** \`${entry.catalogPath}\``, + `- **Skill file:** \`${entry.catalogPath ?? `${CATALOG_PREFIX}/${entry.id}/skill.ts`}\``, `- **Use it for:** ${entry.summary}`, ...(entry.tools && entry.tools.length > 0 ? [`- **Tools it attaches:** ${entry.tools.join(", ")}`] @@ -384,8 +446,22 @@ export function resolveSkill(identifier: string): ResolvedSkill | null { * builtin-tools escape-hatch list. */ export function skillToolNames(skillId: string): string[] { - const entry = getSkillEntries().find((e) => e.id === skillId); - return entry?.tools ? [...entry.tools] : []; + return toolNamesFromEntries(getSkillEntries(), skillId); +} + +// Pure variant over an explicit entry list (exported for the +// activation-boundary tests). Only model-activated entries may attach tools +// through the loadSkill path — an eager capability's tools attach at +// assembly, never mid-turn on the model's request. +export function toolNamesFromEntries( + entries: ReadonlyArray, + skillId: string, +): string[] { + const entry = entries.find((e) => e.id === skillId); + if (!entry || !isModelActivated(entry)) { + return []; + } + return entry.tools ? [...entry.tools] : []; } /** diff --git a/apps/x/packages/core/src/application/notification/notifier.ts b/apps/x/packages/core/src/application/notification/notifier.ts index a1cacc76..050279d3 100644 --- a/apps/x/packages/core/src/application/notification/notifier.ts +++ b/apps/x/packages/core/src/application/notification/notifier.ts @@ -19,8 +19,8 @@ export async function notifyIfEnabled( ): Promise { try { if (!isNotificationCategoryEnabled(category)) return; - const { default: container } = await import('../../di/container.js'); - const service = container.resolve('notificationService'); + const { lazyResolve } = await import('../../di/lazy-resolve.js'); + const service = await lazyResolve('notificationService'); if (!service.isSupported()) return; service.notify(input); } catch (err) { diff --git a/apps/x/packages/core/src/background-tasks/runner.ts b/apps/x/packages/core/src/background-tasks/runner.ts index 810b9aac..1463d27b 100644 --- a/apps/x/packages/core/src/background-tasks/runner.ts +++ b/apps/x/packages/core/src/background-tasks/runner.ts @@ -128,8 +128,8 @@ export async function runBackgroundTask( let codeProject: { id: string; path: string; name: string } | undefined; if (task.projectId) { try { - const { default: container } = await import('../di/container.js'); - const projectsRepo = container.resolve('codeProjectsRepo'); + const { lazyResolve } = await import('../di/lazy-resolve.js'); + const projectsRepo = await lazyResolve('codeProjectsRepo'); const project = await projectsRepo.get(task.projectId); if (project) codeProject = { id: project.id, path: project.path, name: project.name }; } catch (err) { diff --git a/apps/x/packages/core/src/di/lazy-resolve.ts b/apps/x/packages/core/src/di/lazy-resolve.ts new file mode 100644 index 00000000..e3eb65fe --- /dev/null +++ b/apps/x/packages/core/src/di/lazy-resolve.ts @@ -0,0 +1,11 @@ +// Lazily import the DI container and resolve one token. Use this instead of +// a static `import container from "./container.js"` when the importing +// module must not add a static edge into the DI graph (the container's +// module tree is large, and some importers — the agent registry, tool +// handlers, capability availability checks — are themselves imported while +// the container initializes). One helper holds the pattern and this +// rationale instead of hand-rolled copies at every call site. +export async function lazyResolve(token: string): Promise { + const { default: container } = await import("./container.js"); + return container.resolve(token); +} 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 1f7f46ec..0f0e090f 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 @@ -289,3 +289,43 @@ describe("active skills (skill-scoped tools)", () => { expect(resolved.tools).toEqual([]); }); }); + +describe("historical composition compatibility", () => { + // Persisted RequestedAgent.overrides.composition values are replayed + // from turn files; the ModeFlags-derived schema must keep accepting + // every historical shape byte-for-byte. + it("composes identically for sparse, null-heavy, and unknown-key compositions", async () => { + const resolver = makeResolver(makeAgent()); + const sparse = await resolver.resolve({ + agentId: "copilot", + overrides: { composition: { voiceInput: true } }, + }); + const explicit = await resolver.resolve({ + agentId: "copilot", + overrides: { + composition: { + voiceInput: true, + voiceOutput: null, + searchEnabled: false, + codeMode: null, + codeCwd: null, + videoMode: false, + coachMode: false, + workDirId: null, + // Unknown keys have always been ignored. + futureUnknownKey: "ignored", + }, + }, + }); + expect(sparse.systemPrompt).toBe(explicit.systemPrompt); + expect(sparse.systemPrompt).toContain("# Voice Input"); + + // Garbage compositions fall back to all-defaults, not a throw. + const garbage = await resolver.resolve({ + agentId: "copilot", + overrides: { composition: { voiceInput: "yes" } }, + }); + const none = await resolver.resolve({ agentId: "copilot" }); + expect(garbage.systemPrompt).toBe(none.systemPrompt); + }); +}); 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 24dd787e..635906b9 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 @@ -10,10 +10,12 @@ import { composeSystemInstructions } from "../../agents/compose-instructions.js" import { loadAgentNotesContext, loadUserWorkDir, -} from "../../agents/runtime.js"; + loadWorkspaceContext, +} from "../../agents/workspace-context.js"; import { hasWorkspaceContext, loadAgent } from "../../agents/registry.js"; import { BuiltinTools } from "../../application/lib/builtin-tools.js"; import { skillToolNames } from "../../application/assistant/skills/index.js"; +import { ModeFlags } from "../../application/assistant/capabilities/types.js"; import { getDefaultModelAndProvider } from "../../models/defaults.js"; import { builtinToolDescriptor, @@ -51,15 +53,11 @@ const ASK_HUMAN_DESCRIPTOR: z.infer = { // Unknown keys are ignored. Prompt-affecting inputs should be session-sticky: // every key here alters system-prompt bytes and therefore busts provider // prefix caching when it changes between turns. -const CompositionOverrides = z.object({ +// The mode-flag keys come from the shared ModeFlags schema (the single +// source of truth in capabilities/types.ts — defaults make the parse output +// fully concrete); only the resolver-specific keys are declared here. +const CompositionOverrides = ModeFlags.extend({ workDirId: z.string().nullable().optional(), - voiceInput: z.boolean().optional(), - voiceOutput: z.enum(["summary", "full"]).nullable().optional(), - searchEnabled: z.boolean().optional(), - codeMode: z.enum(["claude", "codex"]).nullable().optional(), - codeCwd: z.string().nullable().optional(), - videoMode: z.boolean().optional(), - coachMode: z.boolean().optional(), // 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(), @@ -123,31 +121,30 @@ export class RealAgentResolver { const parsed = CompositionOverrides.safeParse( requested.overrides?.composition ?? {}, ); - const composition = parsed.success ? parsed.data : {}; - // Agent notes and work-dir context are scoped to agents with the - // workspaceContext trait (the copilot), per the agent registry. + // An unparseable composition falls back to all defaults (same shape + // as parsing {}), preserving the historical "ignore garbage" rule. + const composition = parsed.success + ? parsed.data + : CompositionOverrides.parse({}); + const { workDirId, subagent, activeSkills, ...modeFlags } = composition; + // The workspaceContext trait gate lives INSIDE loadWorkspaceContext + // (agents/workspace-context.ts) — no assembly site can forget it. + const workspace = loadWorkspaceContext(requested.agentId, workDirId, { + loadNotes: this.loadNotes, + loadWorkDir: this.loadWorkDir, + }); const copilotContext = hasWorkspaceContext(requested.agentId); const systemPrompt = composeSystemInstructions({ instructions: agent.instructions, - agentNotesContext: copilotContext ? this.loadNotes() : null, - userWorkDir: - copilotContext && composition.workDirId - ? this.loadWorkDir(composition.workDirId) - : null, - voiceInput: composition.voiceInput ?? false, - voiceOutput: composition.voiceOutput ?? null, - searchEnabled: composition.searchEnabled ?? false, - codeMode: composition.codeMode ?? null, - codeCwd: composition.codeCwd ?? null, - videoMode: composition.videoMode ?? false, - coachMode: composition.coachMode ?? false, + ...workspace, + ...modeFlags, }); const tools = await this.resolveTools(agent, { - subagent: composition.subagent ?? false, + subagent: subagent ?? false, }); - if (copilotContext && composition.activeSkills?.length) { - await this.appendSkillTools(tools, composition.activeSkills); + if (copilotContext && activeSkills?.length) { + await this.appendSkillTools(tools, activeSkills); } return ResolvedAgent.parse({ agentId: requested.agentId,