refactor(x): make capability activation a discriminated union; fence the loadSkill path

The design rule "app/always capabilities are never model-loadable" was
held only by two arrays happening to be separate. Now it is structural:
CapabilityDefinition is a union — ModelCapability (requires catalog
title/summary and lazy content, cannot carry a fragment) vs
EagerCapability (requires promptFragment, never enters the catalog) —
and DiskSkill is typed as ModelCapability, making the disk trust
boundary a compile-time fact instead of a comment. The kitchen is
fenced along with the menu: catalog building and skill-tool attachment
go through pure, fenced helpers (buildCatalogFromEntries /
toolNamesFromEntries), and the boundary test now feeds a real eager
capability through a mixed list and asserts both the catalog hides it
and the tool path refuses its tools. Golden snapshots unchanged.

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

apps/x/packages/core/src/agents/compose-instructions.ts

@@ -78,7 +78,7 @@ export function composeSystemInstructions({
         coachMode: coachMode ?? false,
     };
     for (const capability of PROMPT_CAPABILITIES) {
-        const fragment = capability.promptFragment?.(ctx);
+        const fragment = capability.promptFragment(ctx);
         if (fragment) {
             composed += `\n\n${fragment}`;
         }

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

apps/x/packages/core/src/application/assistant/capabilities/types.ts

@@ -11,11 +11,16 @@
 //              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.
 
 export type CapabilityActivation = "model" | "app" | "always";
@@ -39,26 +44,37 @@ export interface CapabilityContext {
     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;
 }
 
-export function isModelActivated(def: Pick<CapabilityDefinition, "activation">): 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";
 }

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

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-name>/SKILL.md subfolders. The rowboat root is

apps/x/packages/core/src/application/assistant/skills/index.test.ts

@@ -147,22 +147,54 @@ 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);
     }

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;
@@ -243,16 +242,27 @@ function getSkillEntries(): SkillEntry[] {
  * Reads the live skill set, so it reflects disk skills added/removed at runtime.
  */
 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);
+  return buildCatalogFromEntries(getSkillEntries(), options);
+}
+
+// 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<CapabilityDefinition & { catalogPath?: string }>,
+  options?: { excludeIds?: string[] },
+): string {
+  // The type-guard filter must keep the catalogPath intersection alive.
+  const modelEntries = all.filter(
+    (e): e is ModelCapability & { catalogPath?: string } => isModelActivated(e),
+  );
   const entries = options?.excludeIds
     ? modelEntries.filter(e => !options.excludeIds!.includes(e.id))
     : modelEntries;
   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 +394,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<CapabilityDefinition>,
+  skillId: string,
+): string[] {
+  const entry = entries.find((e) => e.id === skillId);
+  if (!entry || !isModelActivated(entry)) {
+    return [];
+  }
+  return entry.tools ? [...entry.tools] : [];
 }
 
 /**