#732 Add skill delivery mode for rulebooks

packages/agents/content/guidance/rulebooks/shell-conventions.md

@@ -1,7 +1,7 @@
 ---
 slug: shell-conventions
 description: Conventions for writing production-quality bash scripts in this repository.
-delivery: ambient
+delivery: [ambient, skill]
 version: 1
 ---
 

packages/agents/src/commands/__tests__/sync.test.ts

@@ -1,4 +1,4 @@
-import { existsSync } from 'node:fs';
+import { existsSync, statSync } from 'node:fs';
 import { mkdir, readFile, rm, writeFile } from 'node:fs/promises';
 import { tmpdir } from 'node:os';
 import path from 'node:path';
@@ -48,6 +48,9 @@ describe(syncCommand, () => {
 
   const projectMdPath = (): string => path.join(projectRoot, '.agents', 'PROJECT.md');
 
+  const skillPath = (slug: string, dotDir = '.claude'): string =>
+    path.join(projectRoot, dotDir, 'skills', slug, 'SKILL.md');
+
   it('when no rulebooks.yaml exists, makes no changes', async () => {
     await syncCommand(makeOptions(), projectRoot, contentDir);
 
@@ -158,24 +161,111 @@ describe(syncCommand, () => {
     await expect(syncCommand(makeOptions(), projectRoot, contentDir)).rejects.toThrow(/ghost/);
   });
 
-  it('materializes a skill-only rulebook without inlining it into PROJECT.md', async () => {
-    await writeLibraryRulebook('gamma', 'delivery: skill', 'Gamma rules.');
+  it('writes a skill file for a skill-only rulebook without inlining it into PROJECT.md', async () => {
+    await writeLibraryRulebook('gamma', 'delivery: skill\ndescription: Gamma desc.', 'Gamma rules.');
     await writeManifest('rulebooks:\n  - gamma\n');
 
     await syncCommand(makeOptions(), projectRoot, contentDir);
 
     expect(existsSync(neutralPath('gamma'))).toBe(true);
     expect(existsSync(projectMdPath())).toBe(false);
+    const skill = await readFile(skillPath('gamma'), 'utf8');
+    expect(skill).toContain('name: gamma');
+    expect(skill).toContain('description: Gamma desc.');
+    expect(skill).toContain('<!-- codeassembly-rulebook:gamma -->');
+    expect(skill).toContain('Gamma rules.');
   });
 
-  it('in dry-run mode, writes nothing to disk', async () => {
+  it('writes a skill file for a multi-modal rulebook and also inlines it into PROJECT.md', async () => {
+    await writeLibraryRulebook('delta', 'delivery: [ambient, skill]', 'Delta rules.');
+    await writeManifest('rulebooks:\n  - delta\n');
+
+    await syncCommand(makeOptions(), projectRoot, contentDir);
+
+    expect(await readFile(projectMdPath(), 'utf8')).toContain('<!-- rulebook:delta -->');
+    expect(await readFile(skillPath('delta'), 'utf8')).toContain('Delta rules.');
+  });
+
+  it('when re-run with unchanged content, does not rewrite the skill file', async () => {
+    await writeLibraryRulebook('gamma', 'delivery: skill', 'Gamma rules.');
+    await writeManifest('rulebooks:\n  - gamma\n');
+    await syncCommand(makeOptions(), projectRoot, contentDir);
+    const firstMtime = statSync(skillPath('gamma')).mtimeMs;
+
+    await syncCommand(makeOptions(), projectRoot, contentDir);
+
+    expect(statSync(skillPath('gamma')).mtimeMs).toBe(firstMtime);
+  });
+
+  it('retracts the skill directory when a skill rulebook is no longer declared', async () => {
     await writeLibraryRulebook('alpha', 'delivery: ambient', 'Alpha rules.');
+    await writeLibraryRulebook('gamma', 'delivery: skill', 'Gamma rules.');
+    await writeManifest('rulebooks:\n  - alpha\n  - gamma\n');
+    await syncCommand(makeOptions(), projectRoot, contentDir);
+    expect(existsSync(skillPath('gamma'))).toBe(true);
+
     await writeManifest('rulebooks:\n  - alpha\n');
+    await syncCommand(makeOptions(), projectRoot, contentDir);
+
+    expect(existsSync(path.dirname(skillPath('gamma')))).toBe(false);
+  });
+
+  it('retracts the skill directory when a rulebook delivery changes away from skill', async () => {
+    await writeLibraryRulebook('gamma', 'delivery: skill', 'Gamma rules.');
+    await writeManifest('rulebooks:\n  - gamma\n');
+    await syncCommand(makeOptions(), projectRoot, contentDir);
+    expect(existsSync(skillPath('gamma'))).toBe(true);
+
+    await writeLibraryRulebook('gamma', 'delivery: ambient', 'Gamma rules.');
+    await syncCommand(makeOptions(), projectRoot, contentDir);
+
+    expect(existsSync(path.dirname(skillPath('gamma')))).toBe(false);
+    expect(existsSync(neutralPath('gamma'))).toBe(true);
+  });
+
+  it('with --platform claude, writes only the Claude skills dir', async () => {
+    await writeLibraryRulebook('gamma', 'delivery: skill', 'Gamma rules.');
+    await writeManifest('rulebooks:\n  - gamma\n');
+
+    await syncCommand(makeOptions({ platform: 'claude' }), projectRoot, contentDir);
+
+    expect(existsSync(skillPath('gamma', '.claude'))).toBe(true);
+    expect(existsSync(skillPath('gamma', '.rovodev'))).toBe(false);
+  });
+
+  it('with no detected platform, writes no skill files but still writes the neutral file', async () => {
+    await writeLibraryRulebook('gamma', 'delivery: skill', 'Gamma rules.');
+    await writeManifest('rulebooks:\n  - gamma\n');
+
+    await syncCommand(makeOptions({ platform: 'all' }), projectRoot, contentDir);
+
+    expect(existsSync(neutralPath('gamma'))).toBe(true);
+    expect(existsSync(skillPath('gamma'))).toBe(false);
+  });
+
+  it('never deletes a hand-authored skill that lacks the sync marker', async () => {
+    const manualSkill = skillPath('manual');
+    await mkdir(path.dirname(manualSkill), { recursive: true });
+    await writeFile(manualSkill, '---\nname: manual\n---\n\n# Hand-authored\n', 'utf8');
+    await writeLibraryRulebook('gamma', 'delivery: skill', 'Gamma rules.');
+    await writeManifest('rulebooks:\n  - gamma\n');
+
+    await syncCommand(makeOptions(), projectRoot, contentDir);
+
+    expect(existsSync(manualSkill)).toBe(true);
+    expect(existsSync(skillPath('gamma'))).toBe(true);
+  });
+
+  it('in dry-run mode, writes nothing to disk', async () => {
+    await writeLibraryRulebook('alpha', 'delivery: ambient', 'Alpha rules.');
+    await writeLibraryRulebook('gamma', 'delivery: skill', 'Gamma rules.');
+    await writeManifest('rulebooks:\n  - alpha\n  - gamma\n');
 
     await syncCommand(makeOptions({ dryRun: true }), projectRoot, contentDir);
 
     expect(existsSync(neutralPath('alpha'))).toBe(false);
     expect(existsSync(projectMdPath())).toBe(false);
+    expect(existsSync(skillPath('gamma'))).toBe(false);
   });
 
   it('materializes the real shell-conventions rulebook from the package content', async () => {
@@ -188,5 +278,8 @@ describe(syncCommand, () => {
     expect(neutral).not.toContain('slug:');
     const projectMd = await readFile(projectMdPath(), 'utf8');
     expect(projectMd).toContain('<!-- rulebook:shell-conventions -->');
+    const skill = await readFile(skillPath('shell-conventions'), 'utf8');
+    expect(skill).toContain('name: shell-conventions');
+    expect(skill).toContain('# Shell script conventions');
   });
 });

packages/agents/src/commands/install.ts

@@ -22,7 +22,7 @@ import {
 import { rewritePathsInDirectory, rewritePathsInFile } from '../lib/path-rewriter.js';
 import { PLATFORMS, resolvePlatformIds, resolvePlatformPaths } from '../lib/platform.js';
 import { loadToolMapping, rewriteToolNames } from '../lib/tool-name-rewriter.js';
-import { isEnoent, isErrorCode } from '../lib/type-guards.ts';
+import { isEnoent, isMissingFile } from '../lib/type-guards.ts';
 import type {
   AgentsManifest,
   InstallOptions,
@@ -522,7 +522,7 @@ async function generatePromptsYml(
       // Tolerate any non-directory entry in the destination skills directory (e.g. a `.DS_Store` left by Finder):
       // joining `SKILL.md` onto a regular file raises `ENOTDIR`; a directory without `SKILL.md` raises `ENOENT`.
       // Either way the entry is not a skill and should be skipped.
-      if (isMissingSkill(error)) {
+      if (isMissingFile(error)) {
         continue;
       }
       throw error;
@@ -859,11 +859,6 @@ async function installPlatformGuidance(
   return entries;
 }
 
-/** True when a `readFile` of `SKILL.md` raised `ENOENT` (file absent) or `ENOTDIR` (parent segment is a regular file). */
-function isMissingSkill(error: unknown): boolean {
-  return isErrorCode(error, 'ENOENT') || isErrorCode(error, 'ENOTDIR');
-}
-
 /**
  * Returns a POSIX-style path label for a skill source file relative to `contentDir`, used as the `contextLabel`
  * argument to `rewriteToolNames` so install errors include a stable, platform-independent file reference.

packages/agents/src/commands/sync.ts

@@ -3,24 +3,29 @@ import path from 'node:path';
 import process from 'node:process';
 
 import { resolveContentDir } from '../lib/content-resolver.ts';
+import { resolvePlatformIds, resolvePlatformPaths } from '../lib/platform.ts';
 import { parseRulebookFile } from '../lib/rulebook-schema.ts';
+import { extractRulebookSkillSlug, renderSkillFile } from '../lib/rulebook-skill.ts';
 import { readRulebooksManifest } from '../lib/rulebooks-manifest.ts';
 import { extractInstalledSlugs, injectRulebook, removeRulebook } from '../lib/sentinel-inliner.ts';
-import { isEnoent } from '../lib/type-guards.ts';
+import { isEnoent, isMissingFile } from '../lib/type-guards.ts';
 import type { InstallOptions } from '../lib/types.ts';
 
-/** A declared rulebook resolved against the library: its neutral body and whether it delivers ambiently. */
+/** A declared rulebook resolved against the library: its neutral body and which delivery modes it requests. */
 interface ResolvedRulebook {
   readonly slug: string;
   readonly body: string;
   readonly ambient: boolean;
+  readonly skill: boolean;
+  readonly description: string | undefined;
 }
 
 /**
  * Resolves the project-scope `.agents/rulebooks.yaml`, materializes each declared rulebook's neutral body to
- * `.agents/rulebooks/<slug>.md`, inlines `ambient` rulebooks into `.agents/PROJECT.md`, and retracts rulebooks
- * that are no longer declared. Installed state is derived from the filesystem, not a manifest, which keeps the
- * command idempotent. An absent `rulebooks.yaml` is a total no-op.
+ * `.agents/rulebooks/<slug>.md`, inlines `ambient` rulebooks into `.agents/PROJECT.md`, writes `skill` rulebooks
+ * as thin-wrapper skills into each targeted platform's project-local skills dir, and retracts anything no longer
+ * declared. Installed state is derived from the filesystem, not a manifest, which keeps the command idempotent.
+ * An absent `rulebooks.yaml` is a total no-op.
  *
  * @param projectRoot The project whose `.agents/` directory is synced (defaults to the current directory).
  * @param contentDirOverride Override for the rulebook library source (defaults to the package content dir).
@@ -49,13 +54,28 @@ export async function syncCommand(
   // whose rulebook is still declared but whose delivery no longer includes `ambient`.
   const declaredSet = new Set(declared);
   const desiredAmbient = new Set(resolved.filter((rulebook) => rulebook.ambient).map((rulebook) => rulebook.slug));
+  const desiredSkill = new Set(resolved.filter((rulebook) => rulebook.skill).map((rulebook) => rulebook.slug));
+
+  // Skill delivery targets project-local platform skills dirs, gated by detection (or `--platform`). Passing
+  // `projectRoot` as the base is what keeps the skills project-scoped, and keeps tests out of the real home dir.
+  const platformSkillDirs = resolvePlatformIds(options.platform, projectRoot).map(
+    (platformId) => resolvePlatformPaths(platformId, projectRoot).skillsDir,
+  );
 
   const existingProjectMd = await readFileOrEmpty(projectMdPath);
   const neutralOrphans = (await listNeutralSlugs(neutralDir)).filter((slug) => !declaredSet.has(slug));
   const inlineOrphans = extractInstalledSlugs(existingProjectMd).filter((slug) => !desiredAmbient.has(slug));
+  // A skill dir is sync-owned only when its `SKILL.md` carries the provenance marker; that gate is what keeps
+  // hand-authored skills safe. Orphans are owned dirs whose slug is no longer delivered as a skill.
+  const skillOrphansByDir = await Promise.all(
+    platformSkillDirs.map(async (skillsDir) => ({
+      skillsDir,
+      orphans: (await listOwnedSkillSlugs(skillsDir)).filter((slug) => !desiredSkill.has(slug)),
+    })),
+  );
 
   if (options.dryRun) {
-    reportDryRun(resolved, [...new Set([...neutralOrphans, ...inlineOrphans])]);
+    reportDryRun(resolved, [...new Set([...neutralOrphans, ...inlineOrphans])], platformSkillDirs, skillOrphansByDir);
     return;
   }
 
@@ -85,7 +105,32 @@ export async function syncCommand(
     await writeFile(projectMdPath, projectMd, 'utf8');
   }
 
-  console.info(`Synced ${resolved.length} rulebook(s); retracted ${neutralOrphans.length} file(s).`);
+  // Reconcile skill files per targeted platform: write every skill-delivery rulebook, then retract sync-owned
+  // skill dirs that are no longer skill rulebooks. Orphans were computed against the pre-write filesystem.
+  for (const { skillsDir, orphans } of skillOrphansByDir) {
+    for (const rulebook of resolved) {
+      if (!rulebook.skill) {
+        continue;
+      }
+      const skillDir = path.join(skillsDir, rulebook.slug);
+      await mkdir(skillDir, { recursive: true });
+      await writeIfChanged(
+        path.join(skillDir, 'SKILL.md'),
+        renderSkillFile(rulebook.slug, rulebook.description, rulebook.body),
+      );
+    }
+    for (const slug of orphans) {
+      await rm(path.join(skillsDir, slug), { recursive: true, force: true });
+    }
+  }
+
+  const skillRetractions = skillOrphansByDir.reduce((total, platform) => total + platform.orphans.length, 0);
+  const skillFilesWritten = desiredSkill.size * platformSkillDirs.length;
+  console.info(
+    `Synced ${resolved.length} rulebook(s); delivered ${skillFilesWritten} skill file(s) across ` +
+      `${platformSkillDirs.length} platform(s); retracted ${neutralOrphans.length} neutral file(s) and ` +
+      `${skillRetractions} skill dir(s).`,
+  );
 }
 
 // region | Helpers
@@ -104,6 +149,41 @@ async function listNeutralSlugs(neutralDir: string): Promise<ReadonlyArray<strin
   return entries.filter((entry) => entry.endsWith('.md')).map((entry) => entry.slice(0, -'.md'.length));
 }
 
+/**
+ * Lists the names of skill directories under `skillsDir` that sync owns — those whose `SKILL.md` carries the
+ * rulebook provenance marker. Returns an empty list when the directory is absent. Entries without a readable
+ * `SKILL.md` (a marker-less hand-authored skill, a stray `.DS_Store`) are skipped, never claimed for deletion.
+ */
+async function listOwnedSkillSlugs(skillsDir: string): Promise<ReadonlyArray<string>> {
+  let entries: ReadonlyArray<string>;
+  try {
+    entries = await readdir(skillsDir);
+  } catch (error: unknown) {
+    if (isEnoent(error)) {
+      return [];
+    }
+    throw error;
+  }
+
+  const owned: Array<string> = [];
+  for (const entry of entries) {
+    let content: string;
+    try {
+      content = await readFile(path.join(skillsDir, entry, 'SKILL.md'), 'utf8');
+    } catch (error: unknown) {
+      // Not a skill dir: the SKILL.md is absent, or the entry is a regular file (ENOTDIR on read-through).
+      if (isMissingFile(error)) {
+        continue;
+      }
+      throw error;
+    }
+    if (extractRulebookSkillSlug(content) !== undefined) {
+      owned.push(entry);
+    }
+  }
+  return owned;
+}
+
 /** Reads a file, returning an empty string when it does not exist. */
 async function readFileOrEmpty(filePath: string): Promise<string> {
   try {
@@ -117,15 +197,30 @@ async function readFileOrEmpty(filePath: string): Promise<string> {
 }
 
 /** Prints the writes and retractions a real run would perform. */
-function reportDryRun(resolved: ReadonlyArray<ResolvedRulebook>, retracted: ReadonlyArray<string>): void {
+function reportDryRun(
+  resolved: ReadonlyArray<ResolvedRulebook>,
+  retracted: ReadonlyArray<string>,
+  platformSkillDirs: ReadonlyArray<string>,
+  skillOrphansByDir: ReadonlyArray<{ skillsDir: string; orphans: ReadonlyArray<string> }>,
+): void {
   console.info('[dry-run] sync would:');
   for (const rulebook of resolved) {
     const inline = rulebook.ambient ? ' (+ inline into PROJECT.md)' : '';
     console.info(`  write .agents/rulebooks/${rulebook.slug}.md${inline}`);
+    if (rulebook.skill) {
+      for (const skillsDir of platformSkillDirs) {
+        console.info(`  write ${path.join(skillsDir, rulebook.slug, 'SKILL.md')}`);
+      }
+    }
   }
   for (const slug of retracted) {
     console.info(`  retract ${slug} (no longer declared, or no longer ambient)`);
   }
+  for (const { skillsDir, orphans } of skillOrphansByDir) {
+    for (const slug of orphans) {
+      console.info(`  retract skill ${path.join(skillsDir, slug)} (no longer a skill rulebook)`);
+    }
+  }
 }
 
 /** Reads a rulebook from the library, validates its frontmatter, and returns its neutral body and delivery. */
@@ -142,7 +237,13 @@ async function resolveRulebook(slug: string, librarySrcDir: string): Promise<Res
   }
 
   const { rulebook, body } = parseRulebookFile(content, `${slug}.md`);
-  return { slug, body: `${body.trim()}\n`, ambient: rulebook.delivery.includes('ambient') };
+  return {
+    slug,
+    body: `${body.trim()}\n`,
+    ambient: rulebook.delivery.includes('ambient'),
+    skill: rulebook.delivery.includes('skill'),
+    description: rulebook.description,
+  };
 }
 
 /** Writes `content` to `filePath` only when it differs from the current contents, keeping re-runs diff-free. */