fix(x): review fixes — prototype-safe registry lookup, shared parsing, slimmer records

Findings from an independent review of the branch:

- loadAgent's plain-object lookup traversed Object.prototype, so a
  user-defined agent named "constructor"/"toString" threw a TypeError
  instead of falling through to the agents repo as it did before the
  registry (Object.hasOwn now guards both lookups; regression test
  pins the fallthrough without depending on the environment).
- agentFromRaw now uses the same parseFrontmatter helper FSAgentsRepo
  uses, so builtin and user agents can never drift on one file format.
- The fused workspace-context capability splits into agent-notes and
  work-directory records — the composer's own separator joining makes
  the output byte-identical, so the fusion bought nothing.
- Mode records drop their write-only title/summary (optional on the
  base record, still required on skills where the catalog renders
  them), the composer body is reindented and iterates a hoisted
  PROMPT_CAPABILITIES list, and the two single-consumer re-export
  shims in the legacy runtime file are deleted (consumers retargeted).

Golden snapshots unchanged throughout: composed prompts remain
byte-identical. Deferred to the follow-up PR with availability wiring:
discriminated-union typing on activation + fencing resolveSkill, and
unifying the three mirrored composition shapes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

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;
 }

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/);
+            }
+        }
+    });
 });

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<typeof Agent> {
-    let agent: z.infer<typeof Agent> = { 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<z.infer<typeof Agent>> {
-    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();
     }

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`;

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 <voice> 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;

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).

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

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;

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<typeof CreateRunOptions>): Promise<z.infer<typeof Run>> {

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";