diff --git a/packages/agents/content/guidance/rulebooks/shell-conventions.md b/packages/agents/content/guidance/rulebooks/shell-conventions.md index a8c05959..be63edea 100644 --- a/packages/agents/content/guidance/rulebooks/shell-conventions.md +++ b/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 --- diff --git a/packages/agents/src/commands/__tests__/sync.test.ts b/packages/agents/src/commands/__tests__/sync.test.ts index 5c23c16a..3a0a1fa5 100644 --- a/packages/agents/src/commands/__tests__/sync.test.ts +++ b/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(''); + 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(''); + 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(''); + const skill = await readFile(skillPath('shell-conventions'), 'utf8'); + expect(skill).toContain('name: shell-conventions'); + expect(skill).toContain('# Shell script conventions'); }); }); diff --git a/packages/agents/src/commands/install.ts b/packages/agents/src/commands/install.ts index d9f93399..0211d5f4 100644 --- a/packages/agents/src/commands/install.ts +++ b/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. diff --git a/packages/agents/src/commands/sync.ts b/packages/agents/src/commands/sync.ts index c6287104..1e5e804b 100644 --- a/packages/agents/src/commands/sync.ts +++ b/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/.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/.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 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> { + let entries: ReadonlyArray; + try { + entries = await readdir(skillsDir); + } catch (error: unknown) { + if (isEnoent(error)) { + return []; + } + throw error; + } + + const owned: Array = []; + 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 { try { @@ -117,15 +197,30 @@ async function readFileOrEmpty(filePath: string): Promise { } /** Prints the writes and retractions a real run would perform. */ -function reportDryRun(resolved: ReadonlyArray, retracted: ReadonlyArray): void { +function reportDryRun( + resolved: ReadonlyArray, + retracted: ReadonlyArray, + platformSkillDirs: ReadonlyArray, + skillOrphansByDir: ReadonlyArray<{ skillsDir: string; orphans: ReadonlyArray }>, +): 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 { + it('returns the slug from a rendered skill file', () => { + const output = renderSkillFile('shell-conventions', 'Shell rules.', 'Body.'); + + expect(extractRulebookSkillSlug(output)).toBe('shell-conventions'); + }); + + it('returns undefined when the ownership marker is absent', () => { + expect(extractRulebookSkillSlug('---\nname: x\n---\n\n# Hand-authored skill\n')).toBeUndefined(); + }); +}); + +describe(renderSkillFile, () => { + it('renders frontmatter, the ownership marker, and the body for a described rulebook', () => { + const output = renderSkillFile('shell-conventions', 'Shell rules.', '# Shell\n\nBody.'); + + expect(output).toBe( + [ + '---', + 'name: shell-conventions', + 'description: Shell rules.', + 'user-invocable: true', + '---', + '', + '', + '# Shell', + '', + 'Body.', + '', + ].join('\n'), + ); + }); + + it('omits the description line when no description is provided', () => { + const output = renderSkillFile('alpha', undefined, 'Body.'); + + expect(output).not.toContain('description:'); + expect(output).toContain('name: alpha'); + expect(output).toContain('user-invocable: true'); + }); + + it('escapes a description containing YAML-special characters so it round-trips', () => { + const description = 'Has: a colon and "quotes"'; + + const output = renderSkillFile('alpha', description, 'Body.'); + + expect(frontmatterDescription(output)).toBe(description); + }); + + it('trims surrounding whitespace from the body', () => { + const output = renderSkillFile('alpha', 'D.', '\n\n # Title\n\nBody.\n\n'); + + expect(output.endsWith('\n\n# Title\n\nBody.\n')).toBe(true); + }); + + it('produces byte-identical output for identical inputs', () => { + expect(renderSkillFile('alpha', 'D.', 'Body.')).toBe(renderSkillFile('alpha', 'D.', 'Body.')); + }); +}); + +/** Returns the `description` field parsed from a rendered skill file's YAML frontmatter. */ +function frontmatterDescription(skill: string): unknown { + const block = /^---\n([\s\S]*?)\n---\n/.exec(skill)?.[1]; + if (block === undefined) { + throw new Error('rendered skill has no frontmatter block'); + } + const parsed: unknown = parseYaml(block); + if (!isRecord(parsed)) { + throw new Error('rendered skill frontmatter is not a mapping'); + } + return parsed.description; +} diff --git a/packages/agents/src/lib/rulebook-skill.ts b/packages/agents/src/lib/rulebook-skill.ts new file mode 100644 index 00000000..00f38119 --- /dev/null +++ b/packages/agents/src/lib/rulebook-skill.ts @@ -0,0 +1,34 @@ +import { stringify as stringifyYaml } from 'yaml'; + +/** Returns the slug stamped in a skill file's sync ownership marker, or `undefined` when it carries none. */ +export function extractRulebookSkillSlug(content: string): string | undefined { + return //.exec(content)?.[1]; +} + +/** + * Renders a thin-wrapper skill file for a rulebook delivered in `skill` mode: standard skill frontmatter + * (`name`, `description`, `user-invocable`), the sync ownership marker, and the rulebook's neutral body. The + * output is byte-deterministic — `lineWidth: 0` prevents the description from line-folding — so re-running + * `sync` with unchanged content leaves the file untouched. + * + * `user-invocable` is always `true`: on-demand rulebook skills are meant to be invocable, and rulebook + * frontmatter deliberately carries no per-rulebook opt-out. + */ +export function renderSkillFile(slug: string, description: string | undefined, body: string): string { + const frontmatter = { + name: slug, + ...(description ? { description } : {}), + 'user-invocable': true, + }; + const yaml = stringifyYaml(frontmatter, { lineWidth: 0 }); + return `---\n${yaml}---\n${ownershipMarker(slug)}\n\n${body.trim()}\n`; +} + +// region | Helpers + +/** The provenance marker stamped into every sync-generated skill, identifying it as sync-owned. */ +function ownershipMarker(slug: string): string { + return ``; +} + +// endregion | Helpers diff --git a/packages/agents/src/lib/type-guards.ts b/packages/agents/src/lib/type-guards.ts index 7475b0bc..994d09ae 100644 --- a/packages/agents/src/lib/type-guards.ts +++ b/packages/agents/src/lib/type-guards.ts @@ -14,6 +14,16 @@ export function isEnoent(error: unknown): boolean { return isErrorCode(error, 'ENOENT'); } +/** + * True when a file read failed because the path is absent (`ENOENT`) or a path segment is not a directory + * (`ENOTDIR`); both mean the target file is not there. Prefer this over `isEnoent` when reading a file nested + * under a directory that may itself be absent or replaced by a regular file (e.g. a `/SKILL.md` probe). + * @internal + */ +export function isMissingFile(error: unknown): boolean { + return isErrorCode(error, 'ENOENT') || isErrorCode(error, 'ENOTDIR'); +} + /** * Type guard for a non-null, non-array object. * @internal