diff --git a/packages/agents/content/guidance/rulebooks/codeassembly-content-specification.md b/packages/agents/content/guidance/rulebooks/codeassembly-content-specification.md index 6991bdb7..c32cc8be 100644 --- a/packages/agents/content/guidance/rulebooks/codeassembly-content-specification.md +++ b/packages/agents/content/guidance/rulebooks/codeassembly-content-specification.md @@ -2,7 +2,7 @@ slug: codeassembly-content-specification description: The declaration contract for CodeAssembly skills, subagents, rulebooks, and collections -- frontmatter fields, dependency blocks, and invocation tokens. delivery: skill -version: 5 +version: 6 --- # CodeAssembly content specification @@ -13,7 +13,7 @@ The declaration contract for CodeAssembly artifacts -- skills, subagents, rulebo Every rule below belongs to one of three classes, marked where it appears. -**Validated on parse.** A malformed `slug` or `skill-name`, a `delivery` value outside `ambient`/`skill`, an unknown artifact-type key, a non-list value under one, and a `members:` block on anything but a collection each fail the run with an error naming the source file. Three more fail outside the parser: a token naming an artifact that does not exist fails the run with an error naming the slug and the directories searched, a rulebook link target outside a linkable root fails the run before anything is written, and a harness that declares no sigil is a type error at its `HarnessConfig` literal, so the build fails. +**Validated on parse.** A malformed `slug` or `skill-name`, a `delivery` value outside `ambient`/`skill`, an unknown artifact-type key, a non-list value under one, and a `members:` block on anything but a collection each fail the run with an error naming the source file. Four more fail outside the parser: a token naming an artifact that does not exist fails the run with an error naming the slug and the directories searched, a rulebook link target outside a linkable root fails the run before anything is written, a rulebook token naming a target that deploys no skill to invoke fails the same pre-write pass, and a harness that declares no sigil is a type error at its `HarnessConfig` literal, so the build fails. **Enforced by test.** The suites in `packages/agents/src/__tests__/` read the shipped library and assert its conventions hold. A rule one of them guards names its test. @@ -37,6 +37,7 @@ dependencies: When a skill or subagent invocation appears inline in a skill's or subagent's body, write it as a token rather than a hardcoded harness-specific form: +- `{rulebook:}` renders to the harness skill sigil plus the skill name the named rulebook deploys under -- its `skill-name` when it declares one, `consult-` otherwise. Because it resolves through the target rather than echoing the slug, an override on the target stays honest at every call site. - `{skill:}` renders to the harness skill sigil plus the slug -- `/` on Claude, `!` on Rovo. - `{subagent:}` renders to the harness subagent sigil plus the slug. That sigil is empty on both current harnesses, so it renders to the bare slug, which is how a subagent is dispatched on each. @@ -44,17 +45,23 @@ Slugs are kebab-case and letter-led (`[a-z][a-z0-9-]*`). The sigils are a typed A token is also a dependency edge: `sync` extracts the tokens from a skill's or subagent's include-expanded body and pulls each target into the deploy closure. An inline invocation is therefore expressed once, as the token -- it needs no duplicate `dependencies:` entry, and a token naming a non-existent artifact fails the run just as a missing `dependencies:` edge does. Because extraction runs on the include-expanded body, a token inside a shared `_partials` file becomes an edge for every skill that includes it. -Tokens are honored only in skills and subagents. A rulebook body does pass through a render pass, but a narrower one: it receives link and template rewriting (see [Links in rulebook bodies](#links-in-rulebook-bodies)) and not token rewriting, which is why rulebooks keep `dependencies:`, as collections keep `members:`. Reserve a `dependencies:` entry for a non-inline edge; use a token for any invocation that appears in the body. _(Convention; not enforced.)_ +Rulebooks, skills, and subagents all honor tokens; collections carry no body to render. `{rulebook:}` is the exception: only a rulebook body renders one, because `install` deploys skills without resolving a declaration and so has no rulebook to resolve against. A rulebook token elsewhere fails the run, as does one naming a rulebook that deploys no skill -- an `ambient`-only target is already in the reader's context, so there is nothing to route to. Express that relationship with `dependencies:` instead. + +Only `{rulebook:}` is checked for deployability. A `{skill:}` or `{subagent:}` token renders on every harness the body reaches, including one its target does not deploy to: a skill that narrows itself with `harnesses:` still renders an invocation elsewhere. Name such a skill only where the surrounding text already scopes it to that harness. _(Convention; not enforced.)_ + +Reserve a `dependencies:` entry for a non-inline edge; use a token for any invocation that appears in the body. _(Convention; not enforced.)_ ## Links in rulebook bodies A rulebook addresses a file by linking to it, not by naming it in prose. Author the target relative to the rulebook's own place in the content tree, which is `guidance/rulebooks/.md`, and `sync` emits the absolute path each target harness can follow. A target of `../../skills/_data/concision.md` reaches Claude as `~/.claude/skills/_data/concision.md` and Rovo as `~/.rovodev/skills/_data/concision.md`. `{harness_home_dir}` and `{harness_id}` expand per harness, including where one opens a link target. -A rulebook may link only into `skills/` and `scripts/`, the two trees whose source layout matches where they deploy under every harness home. Any other target fails the run, with an error naming the rulebook, the target as authored, and whether it resolved outside a linkable root or escaped the content root. `subagents/` is rejected because a subagent is dispatched rather than read, so no link into one is worth authoring; `_partials/`, `collections/`, and `guidance/` never deploy as files, so a link into one would name nothing. Nor can a rulebook link to the skill another rulebook delivers: that `SKILL.md` is generated rather than authored, and `dependencies:` already expresses the relationship. _(Validated on parse.)_ +A rulebook may link only into `skills/` and `scripts/`, the two trees whose source layout matches where they deploy under every harness home. Any other target fails the run, with an error naming the rulebook, the target as authored, and why it was rejected. `subagents/` is rejected because a subagent is dispatched rather than read, so no link into one is worth authoring; `_partials/` and `collections/` never deploy as files, so a link into one would name nothing. + +A link to a sibling rulebook is rejected too, and its error names the `{rulebook:}` token that addresses it instead. A rulebook is invoked rather than read: the skill it deploys is discovered by name, so an invocation resolves wherever it was deployed, while a path would be right in one domain and dead in the other. _(Validated on parse.)_ A target that is rooted correctly but names a file that has moved or been deleted is caught separately, by `content-link-resolution.test.ts`, which also resolves every anchor fragment to exactly one heading. _(Enforced by test.)_ -One limitation is worth knowing before writing a rulebook that documents linking: rewriting runs over the whole body, so a Markdown link inside a code fence or an inline code span is rewritten along with the rest. A rulebook cannot show a relative link verbatim as an example, and must describe the target instead. +One limitation is worth knowing before writing a rulebook that documents linking: rewriting runs over the whole body, so a Markdown link inside a code fence or an inline code span is rewritten along with the rest. A rulebook cannot show a relative link verbatim as an example, and must describe the target instead. Invocation tokens rewrite the same way, so an example token keeps the `` placeholder rather than naming a real artifact. ## Collections diff --git a/packages/agents/src/__tests__/content-link-resolution.test.ts b/packages/agents/src/__tests__/content-link-resolution.test.ts index 10622904..ba55c73b 100644 --- a/packages/agents/src/__tests__/content-link-resolution.test.ts +++ b/packages/agents/src/__tests__/content-link-resolution.test.ts @@ -5,8 +5,10 @@ import path from 'node:path'; import { beforeAll, describe, expect, it } from 'vitest'; import { expandIncludes } from '../lib/directive-expander.ts'; +import type { RulebookInvocationCatalog } from '../lib/invocation-tokens.ts'; import { isRewritableLinkTarget, MARKDOWN_LINK_REGEX } from '../lib/path-rewriter.ts'; import { parseRulebookFile } from '../lib/rulebook-schema.ts'; +import { resolveSkillName } from '../lib/rulebook-skill.ts'; import { renderRulebookBody } from '../lib/rulebook-transform.ts'; // A Markdown link in installable content is rewritten at install time by `rewriteMarkdownPaths`, which resolves a @@ -202,8 +204,8 @@ describe('installable-content link resolution', () => { }); }); -describe('shipped rulebook link deliverability', () => { - it('every rulebook link target is rooted in a tree that deploys under a harness home', async () => { +describe('shipped rulebook reference deliverability', () => { + it('every rulebook link target and invocation token names something that deploys', async () => { const rejections = await findRulebookRejections(); expect(rejections, rejections.join('\n')).toEqual([]); }); @@ -211,18 +213,40 @@ describe('shipped rulebook link deliverability', () => { /** * Renders every shipped rulebook the way `sync` does, collecting the error from each that names an undeliverable link - * target. The root allowlist is lexical and harness-invariant, so one harness context stands for all of them. + * target or an unusable `{rulebook:}` token. Every shipped rulebook stands in for the deployed set, which is the + * strictest catalog available here: a token naming one that is missing or ambient-only has nothing to invoke under any + * declaration. The root allowlist and the catalog are both harness-invariant, so one harness context stands for all. */ async function findRulebookRejections(): Promise> { const rulebookFiles: Array = []; await collectHostFiles(path.join(CONTENT_ROOT, RULEBOOK_ROOT), rulebookFiles); + const parsed = await Promise.all( + rulebookFiles.map(async (file) => { + const slug = path.basename(file, '.md'); + const { rulebook, body } = parseRulebookFile(await readFile(file, 'utf8'), `${slug}.md`); + return { + slug, + body, + skillName: resolveSkillName(slug, rulebook['skill-name']), + skill: rulebook.delivery.includes('skill'), + }; + }), + ); + const rulebooks: RulebookInvocationCatalog = new Map( + parsed.map(({ slug, skillName, skill }) => [slug, { skillName, skill }]), + ); + const rejections: Array = []; - for (const file of rulebookFiles) { - const slug = path.basename(file, '.md'); - const { body } = parseRulebookFile(await readFile(file, 'utf8'), `${slug}.md`); + for (const { slug, body } of parsed) { try { - renderRulebookBody(body, slug, { homeDir: '.claude', harnessId: 'claude' }); + renderRulebookBody(body, slug, { + homeDir: '.claude', + harnessId: 'claude', + skillSigil: '/', + subagentSigil: '', + rulebooks, + }); } catch (error) { rejections.push(error instanceof Error ? error.message : String(error)); } diff --git a/packages/agents/src/commands/__tests__/sync.test.ts b/packages/agents/src/commands/__tests__/sync.test.ts index 1535db7f..2b8d1822 100644 --- a/packages/agents/src/commands/__tests__/sync.test.ts +++ b/packages/agents/src/commands/__tests__/sync.test.ts @@ -648,6 +648,56 @@ describe(syncCommand, () => { expect(shared).toContain(''); }); + describe('rulebook invocation tokens', () => { + it('renders a rulebook token to each harness sigil in both delivery passes', async () => { + await writeLibraryRulebook('nmr-scripts', 'delivery: skill', 'Script rules.'); + await writeLibraryRulebook('nmr-cheatsheet', 'delivery: [ambient, skill]', 'See {rulebook:nmr-scripts}.'); + await declareRulebooks('nmr-cheatsheet', 'nmr-scripts'); + await mkdir(path.join(projectRoot, '.claude'), { recursive: true }); + await mkdir(path.join(projectRoot, '.rovodev'), { recursive: true }); + + await syncCommand(makeOptions({ harness: 'all' }), projectRoot, contentDir); + + expect(await readFile(localHostPath('CLAUDE.local.md'), 'utf8')).toContain('See /consult-nmr-scripts.'); + expect(await readFile(localHostPath('AGENTS.local.md'), 'utf8')).toContain('See !consult-nmr-scripts.'); + expect(await readFile(skillPath('consult-nmr-cheatsheet'), 'utf8')).toContain('See /consult-nmr-scripts.'); + expect(await readFile(skillPath('consult-nmr-cheatsheet', '.rovodev'), 'utf8')).toContain( + 'See !consult-nmr-scripts.', + ); + }); + + it('deploys a rulebook named only by a body token', async () => { + await writeLibraryRulebook('nmr-scripts', 'delivery: skill', 'Script rules.'); + await writeLibraryRulebook('nmr-cheatsheet', 'delivery: ambient', 'See {rulebook:nmr-scripts}.'); + await declareRulebooks('nmr-cheatsheet'); + + await syncCommand(makeOptions(), projectRoot, contentDir); + + expect(await readFile(skillPath('consult-nmr-scripts'), 'utf8')).toContain('Script rules.'); + }); + + it('renders a token through the skill-name override on its target', async () => { + await writeLibraryRulebook('shell-conventions', 'delivery: skill\nskill-name: shell-rules', 'Shell rules.'); + await writeLibraryRulebook('hub', 'delivery: ambient', 'See {rulebook:shell-conventions}.'); + await declareRulebooks('hub'); + + await syncCommand(makeOptions(), projectRoot, contentDir); + + expect(await readFile(localHostPath(), 'utf8')).toContain('See /shell-rules.'); + }); + + it('fails a dry run with nothing written when a token names an ambient-only rulebook', async () => { + await writeLibraryRulebook('nmr-cheatsheet', 'delivery: ambient', 'Cheatsheet rules.'); + await writeLibraryRulebook('hub', 'delivery: ambient', 'See {rulebook:nmr-cheatsheet}.'); + await declareRulebooks('hub'); + + await expect(syncCommand(makeOptions({ dryRun: true }), projectRoot, contentDir)).rejects.toThrow( + /\{rulebook:nmr-cheatsheet\}[\s\S]*ambient-only/, + ); + expect(existsSync(localHostPath())).toBe(false); + }); + }); + describe('declared sources', () => { let sourceDir: string; @@ -930,6 +980,16 @@ describe(syncCommand, () => { await writeFile(path.join(projectRoot, '.agents', 'codeassembly.yaml'), content, 'utf8'); } + it('fails a dry run with nothing written when a skill body carries a rulebook token', async () => { + await writeLibrarySkill('people-report', { body: 'See {rulebook:nmr-scripts}.' }); + await declareSkills('people-report'); + + await expect(syncCommand(makeOptions({ dryRun: true }), projectRoot, contentDir)).rejects.toThrow( + /\{rulebook:nmr-scripts\} in skills\/people-report\/SKILL\.md[\s\S]*only in a rulebook body/, + ); + expect(existsSync(skillPath('people-report'))).toBe(false); + }); + it('deploys a declared skill into the project-local skills dir with the ownership marker', async () => { await writeLibrarySkill('people-report'); await declareSkills('people-report'); @@ -1274,11 +1334,13 @@ describe(syncCommand, () => { } /** Writes a fixture subagent `.md` into the temp content library's `subagents/`. */ - async function writeLibrarySubagent(slug: string): Promise { + async function writeLibrarySubagent( + slug: string, + { body = `# ${slug}\n\nUse {tool:Read}; run \`{harness_home_dir}/scripts/x.sh\`.` }: { body?: string } = {}, + ): Promise { const dir = path.join(contentDir, 'subagents'); await mkdir(dir, { recursive: true }); - const content = `# ${slug}\n\nUse {tool:Read}; run \`{harness_home_dir}/scripts/x.sh\`.`; - await writeFile(path.join(dir, `${slug}.md`), `---\nname: ${slug}\n---\n\n${content}\n`, 'utf8'); + await writeFile(path.join(dir, `${slug}.md`), `---\nname: ${slug}\n---\n\n${body}\n`, 'utf8'); } /** Writes the project-scope codeassembly.yaml declaring the given subagent slugs. */ @@ -1289,6 +1351,36 @@ describe(syncCommand, () => { await writeFile(path.join(projectRoot, '.agents', 'codeassembly.yaml'), `subagents:\n${useBlock}`, 'utf8'); } + it('fails a dry run with nothing written when a subagent body carries a rulebook token', async () => { + await writeOverlays(); + await writeLibrarySubagent('canary', { body: 'See {rulebook:nmr-scripts}.' }); + await declareSubagents('canary'); + + await expect(syncCommand(makeOptions({ dryRun: true }), projectRoot, contentDir)).rejects.toThrow( + /\{rulebook:nmr-scripts\} in subagents\/canary\.md[\s\S]*only in a rulebook body/, + ); + expect(existsSync(subagentPath('canary'))).toBe(false); + }); + + it('fails a real sync before the ambient host is written when a subagent body carries a rulebook token', async () => { + // Subagents deploy last, so this pins the failure ahead of the earlier ambient and skill write passes. + await writeOverlays(); + await writeLibraryRulebook('alpha', 'delivery: ambient', 'Alpha rules.'); + await writeLibrarySubagent('canary', { body: 'See {rulebook:nmr-scripts}.' }); + await mkdir(path.join(projectRoot, '.agents'), { recursive: true }); + await writeFile( + path.join(projectRoot, '.agents', 'codeassembly.yaml'), + 'rulebooks:\n use:\n - alpha\nsubagents:\n use:\n - canary\n', + 'utf8', + ); + + await expect(syncCommand(makeOptions(), projectRoot, contentDir)).rejects.toThrow( + /\{rulebook:nmr-scripts\} in subagents\/canary\.md/, + ); + expect(existsSync(localHostPath())).toBe(false); + expect(existsSync(subagentPath('canary'))).toBe(false); + }); + it('deploys a declared subagent with the transform applied and the ownership marker, no provenance marker', async () => { await writeOverlays(); await writeLibrarySubagent('canary'); diff --git a/packages/agents/src/commands/sync.ts b/packages/agents/src/commands/sync.ts index c1da5ec7..ffc834d1 100644 --- a/packages/agents/src/commands/sync.ts +++ b/packages/agents/src/commands/sync.ts @@ -27,6 +27,7 @@ import { deploySkill, resolveDeclaredSkill, type ResolvedSkill } from '../lib/sk import { renderSkillDirectory, type SkillDeployContext } from '../lib/skill-transform.ts'; import { deploySubagent, + renderSubagent, resolveDeclaredSubagent, type ResolvedSubagent, type SubagentDeployContext, @@ -275,6 +276,10 @@ async function reconcileDomain( // here; `deploySkill` re-renders it at write time. await assertDeclaredSkillsRender(harnessSkillTargets, resolvedSkills); + // Same gate for subagents, whose deploy is the last write pass: without it a render failure lands after the ambient + // host and every skill file are already on disk. + await assertDeclaredSubagentsRender(harnessSubagentTargets, resolvedSubagents); + // Same gate for rulebooks: a link target the delivery pipeline cannot honor fails the run before either delivery // pass writes, rather than shipping a path that resolves to nothing. assertRulebooksRender(harnessIds, resolved); @@ -318,7 +323,7 @@ async function reconcileDomain( for (const dir of orphans) { await rm(path.join(skillsDir, dir), { recursive: true, force: true }); } - const context = resolveRulebookRenderContext(harnessId); + const context = resolveRulebookRenderContext(harnessId, resolved); for (const rulebook of resolved) { if (!rulebook.skill) { continue; @@ -752,6 +757,22 @@ async function assertDeclaredSkillsRender( } } +/** + * Renders every declared subagent against every targeted harness, discarding the output, so an unmapped tool + * placeholder or a rulebook token throws before any file is written. `reconcileDeclaredSubagents` re-renders at write + * time; this pass exists only to fail the run closed, including under `--dry-run`. + */ +async function assertDeclaredSubagentsRender( + targets: ReadonlyArray, + resolvedSubagents: ReadonlyArray, +): Promise { + for (const target of targets) { + for (const subagent of resolvedSubagents) { + await renderSubagent(subagent, target.deployContext); + } + } +} + /** * Renders every resolved rulebook against every targeted harness, discarding the output, so a link target the * delivery pipeline cannot honor throws before any file is written. Both delivery passes re-render at write time; @@ -759,7 +780,7 @@ async function assertDeclaredSkillsRender( */ function assertRulebooksRender(harnessIds: ReadonlyArray, resolved: ReadonlyArray): void { for (const harnessId of harnessIds) { - const context = resolveRulebookRenderContext(harnessId); + const context = resolveRulebookRenderContext(harnessId, resolved); for (const rulebook of resolved) { renderRulebookBody(rulebook.body, rulebook.slug, context); } @@ -1030,7 +1051,7 @@ async function retireRetiredOutputs(options: InstallOptions, domain: SyncDomain) * region. Each body is rendered for `harnessId`, so the same rulebook yields that harness's own absolute paths. */ function renderAmbientBody(resolved: ReadonlyArray, harnessId: HarnessId): string { - const context = resolveRulebookRenderContext(harnessId); + const context = resolveRulebookRenderContext(harnessId, resolved); let body = ''; for (const rulebook of resolved) { if (rulebook.ambient) { @@ -1040,10 +1061,24 @@ function renderAmbientBody(resolved: ReadonlyArray, harnessId: return body; } -/** The per-harness inputs a rulebook render depends on, read off the harness config. */ -function resolveRulebookRenderContext(harnessId: HarnessId): RulebookRenderContext { +/** + * The per-harness inputs a rulebook render depends on: the harness config's own segments and sigils, plus the deployed + * rulebooks indexed by slug, so a `{rulebook:}` token renders the skill name its target deploys under. + */ +function resolveRulebookRenderContext( + harnessId: HarnessId, + resolved: ReadonlyArray, +): RulebookRenderContext { const config = HARNESSES[harnessId]; - return { homeDir: config.homeDir, harnessId: config.id }; + return { + homeDir: config.homeDir, + harnessId: config.id, + skillSigil: config.skillSigil, + subagentSigil: config.subagentSigil, + rulebooks: new Map( + resolved.map((rulebook) => [rulebook.slug, { skillName: rulebook.skillName, skill: rulebook.skill }]), + ), + }; } /** diff --git a/packages/agents/src/lib/__tests__/dependency-resolver.test.ts b/packages/agents/src/lib/__tests__/dependency-resolver.test.ts index 85a46760..31d6e072 100644 --- a/packages/agents/src/lib/__tests__/dependency-resolver.test.ts +++ b/packages/agents/src/lib/__tests__/dependency-resolver.test.ts @@ -231,14 +231,49 @@ describe(resolveClosure, () => { expect(closure.skills.toSorted()).toEqual(['capture-event', 'wrap-up']); }); - it('does not treat a rulebook body token as an edge', async () => { - // A rulebook body is embedded without the render pass, so a token in it is literal text, not an edge: the named - // skill is neither pulled into the closure nor existence-checked (here it does not exist, and the run succeeds). + it('pulls a skill named by a rulebook body token into the closure', async () => { + await writeArtifact(contentDir, 'skill', 'capture-event'); await writeArtifactWithBody(contentDir, 'rulebook', 'some-rulebook', 'Invoke {skill:capture-event}.'); const closure = await resolveClosure({ rulebook: ['some-rulebook'] }, libraryResolver(contentDir)); - expect(closure).toEqual({ rulebooks: ['some-rulebook'], skills: [], subagents: [] }); + expect(closure).toEqual({ rulebooks: ['some-rulebook'], skills: ['capture-event'], subagents: [] }); + }); + + it('pulls a subagent named by a rulebook body token into the closure', async () => { + await writeArtifact(contentDir, 'subagent', 'planner'); + await writeArtifactWithBody(contentDir, 'rulebook', 'some-rulebook', 'Dispatch {subagent:planner}.'); + + const closure = await resolveClosure({ rulebook: ['some-rulebook'] }, libraryResolver(contentDir)); + + expect(closure).toEqual({ rulebooks: ['some-rulebook'], skills: [], subagents: ['planner'] }); + }); + + it('pulls a rulebook named by a rulebook body token into the closure', async () => { + await writeArtifact(contentDir, 'rulebook', 'nmr-scripts'); + await writeArtifactWithBody(contentDir, 'rulebook', 'nmr-cheatsheet', 'See {rulebook:nmr-scripts}.'); + + const closure = await resolveClosure({ rulebook: ['nmr-cheatsheet'] }, libraryResolver(contentDir)); + + expect(closure.rulebooks.toSorted()).toEqual(['nmr-cheatsheet', 'nmr-scripts']); + }); + + it('drops a rulebook body self-token so a self-reference renders without becoming an edge', async () => { + await writeArtifactWithBody(contentDir, 'rulebook', 'nmr-scripts', 'Re-read {rulebook:nmr-scripts} for detail.'); + + const closure = await resolveClosure({ rulebook: ['nmr-scripts'] }, libraryResolver(contentDir)); + + expect(closure.rulebooks).toEqual(['nmr-scripts']); + }); + + it('leaves a rulebook token in a skill body out of the closure', async () => { + // Only a rulebook body renders a rulebook token, so unioning one here would deploy a target for a reference the + // render pass rejects. The named rulebook is not existence-checked either: it does not exist, and this resolves. + await writeArtifactWithBody(contentDir, 'skill', 'wrap-up', 'See {rulebook:nmr-scripts}.'); + + const closure = await resolveClosure({ skill: ['wrap-up'] }, libraryResolver(contentDir)); + + expect(closure).toEqual({ rulebooks: [], skills: ['wrap-up'], subagents: [] }); }); it('drops a skill body self-token so a self-reference renders without becoming an edge', async () => { diff --git a/packages/agents/src/lib/__tests__/invocation-tokens.test.ts b/packages/agents/src/lib/__tests__/invocation-tokens.test.ts index 2838827d..54b63182 100644 --- a/packages/agents/src/lib/__tests__/invocation-tokens.test.ts +++ b/packages/agents/src/lib/__tests__/invocation-tokens.test.ts @@ -1,81 +1,179 @@ import { describe, expect, it } from 'vitest'; -import { extractInvocationEdges, type InvocationSigils, rewriteInvocationTokens } from '../invocation-tokens.ts'; +import { + extractInvocationEdges, + type InvocationSigils, + resolveRulebookToken, + rewriteInvocationTokens, + type RulebookInvocationCatalog, +} from '../invocation-tokens.ts'; const CLAUDE_SIGILS: InvocationSigils = { skillSigil: '/', subagentSigil: '' }; const ROVO_SIGILS: InvocationSigils = { skillSigil: '!', subagentSigil: '' }; +// The host a rejected token is attributed to, in the content-root-relative form both transforms pass. +const HOST = 'skills/wrap-up/SKILL.md'; + +// `shell-conventions` carries a `skill-name` override, so its deployed name is not `consult-`. +const RULEBOOKS: RulebookInvocationCatalog = new Map([ + ['nmr-cheatsheet', { skillName: 'consult-nmr-cheatsheet', skill: false }], + ['nmr-scripts', { skillName: 'consult-nmr-scripts', skill: true }], + ['shell-conventions', { skillName: 'shell-rules', skill: true }], +]); + describe(extractInvocationEdges, () => { it('returns empty groups when no tokens are present', () => { - expect(extractInvocationEdges('No tokens here.')).toEqual({ skills: [], subagents: [] }); + expect(extractInvocationEdges('No tokens here.')).toEqual({ rulebooks: [], skills: [], subagents: [] }); }); it('groups slugs by kind', () => { - const content = 'Run {skill:plan} and {subagent:planner}, then {skill:review-branch}.'; + const content = 'Run {skill:plan} and {subagent:planner}, see {rulebook:nmr-scripts}, then {skill:review-branch}.'; expect(extractInvocationEdges(content)).toEqual({ + rulebooks: ['nmr-scripts'], skills: ['plan', 'review-branch'], subagents: ['planner'], }); }); it('ignores non-token text and malformed tokens', () => { - const content = 'Prose {skill:commit} and {skill:} and {tool:Read} and {subagent:9bad}.'; - expect(extractInvocationEdges(content)).toEqual({ skills: ['commit'], subagents: [] }); + const content = 'Prose {skill:commit} and {skill:} and {tool:Read} and {rulebook:} and {subagent:9bad}.'; + expect(extractInvocationEdges(content)).toEqual({ rulebooks: [], skills: ['commit'], subagents: [] }); }); it('returns slugs in source order without deduping repeats', () => { const content = '{skill:commit} then {skill:commit} again.'; - expect(extractInvocationEdges(content)).toEqual({ skills: ['commit', 'commit'], subagents: [] }); + expect(extractInvocationEdges(content)).toEqual({ rulebooks: [], skills: ['commit', 'commit'], subagents: [] }); + }); +}); + +describe(resolveRulebookToken, () => { + it('resolves a skill-delivered rulebook to its deployed skill name', () => { + expect(resolveRulebookToken('nmr-scripts', RULEBOOKS)).toEqual({ + kind: 'resolved', + skillName: 'consult-nmr-scripts', + }); + }); + + it('resolves through a skill-name override rather than the slug', () => { + expect(resolveRulebookToken('shell-conventions', RULEBOOKS)).toEqual({ + kind: 'resolved', + skillName: 'shell-rules', + }); + }); + + it.each([ + { + name: 'when no catalog is supplied, rejects as honored only in a rulebook body', + slug: 'nmr-scripts', + rulebooks: undefined, + reason: /only in a rulebook body/, + }, + { + name: 'when the slug names no deployed rulebook, rejects as absent from the deployed set', + slug: 'never-declared', + rulebooks: RULEBOOKS, + reason: /no rulebook in the deployed set/, + }, + { + name: 'when the target is ambient-only, rejects and names dependencies: as the alternative', + slug: 'nmr-cheatsheet', + rulebooks: RULEBOOKS, + reason: /ambient-only rulebook[\s\S]*`dependencies:`/, + }, + ])('$name', ({ slug, rulebooks, reason }) => { + const resolution = resolveRulebookToken(slug, rulebooks); + + expect(resolution.kind).toBe('rejected'); + expect(resolution.kind === 'rejected' && resolution.reason).toMatch(reason); }); }); describe(rewriteInvocationTokens, () => { it('returns content unchanged when no tokens are present', () => { const content = 'Plain prose mentioning a skill but using no token.'; - expect(rewriteInvocationTokens(content, CLAUDE_SIGILS)).toBe(content); + expect(rewriteInvocationTokens(content, CLAUDE_SIGILS, HOST)).toBe(content); }); it('renders a skill token as the skill sigil plus the slug', () => { - expect(rewriteInvocationTokens('Invoke {skill:capture-event} now.', CLAUDE_SIGILS)).toBe( + expect(rewriteInvocationTokens('Invoke {skill:capture-event} now.', CLAUDE_SIGILS, HOST)).toBe( 'Invoke /capture-event now.', ); - expect(rewriteInvocationTokens('Invoke {skill:capture-event} now.', ROVO_SIGILS)).toBe( + expect(rewriteInvocationTokens('Invoke {skill:capture-event} now.', ROVO_SIGILS, HOST)).toBe( 'Invoke !capture-event now.', ); }); it('renders a subagent token as the bare slug when the sigil is empty', () => { - expect(rewriteInvocationTokens('Dispatch {subagent:code-reviewer}.', CLAUDE_SIGILS)).toBe( + expect(rewriteInvocationTokens('Dispatch {subagent:code-reviewer}.', CLAUDE_SIGILS, HOST)).toBe( + 'Dispatch code-reviewer.', + ); + expect(rewriteInvocationTokens('Dispatch {subagent:code-reviewer}.', ROVO_SIGILS, HOST)).toBe( 'Dispatch code-reviewer.', ); - expect(rewriteInvocationTokens('Dispatch {subagent:code-reviewer}.', ROVO_SIGILS)).toBe('Dispatch code-reviewer.'); }); it('renders a subagent token with a non-empty sigil', () => { const sigils: InvocationSigils = { skillSigil: '/', subagentSigil: '@' }; - expect(rewriteInvocationTokens('Dispatch {subagent:code-reviewer}.', sigils)).toBe('Dispatch @code-reviewer.'); + expect(rewriteInvocationTokens('Dispatch {subagent:code-reviewer}.', sigils, HOST)).toBe( + 'Dispatch @code-reviewer.', + ); + }); + + it('renders a rulebook token as the skill sigil plus the target skill name', () => { + expect(rewriteInvocationTokens('See {rulebook:nmr-scripts}.', CLAUDE_SIGILS, HOST, RULEBOOKS)).toBe( + 'See /consult-nmr-scripts.', + ); + expect(rewriteInvocationTokens('See {rulebook:nmr-scripts}.', ROVO_SIGILS, HOST, RULEBOOKS)).toBe( + 'See !consult-nmr-scripts.', + ); + }); + + it('renders a rulebook token through its skill-name override', () => { + expect(rewriteInvocationTokens('See {rulebook:shell-conventions}.', CLAUDE_SIGILS, HOST, RULEBOOKS)).toBe( + 'See /shell-rules.', + ); + }); + + it.each([ + { name: 'when no catalog is supplied', rulebooks: undefined, slug: 'nmr-scripts' }, + { name: 'when the slug names no deployed rulebook', rulebooks: RULEBOOKS, slug: 'never-declared' }, + { name: 'when the target is ambient-only', rulebooks: RULEBOOKS, slug: 'nmr-cheatsheet' }, + ])('throws naming the offending token and its host $name', ({ rulebooks, slug }) => { + expect(() => rewriteInvocationTokens(`See {rulebook:${slug}}.`, CLAUDE_SIGILS, HOST, rulebooks)).toThrow( + new RegExp(String.raw`\{rulebook:${slug}\} in skills/wrap-up/SKILL\.md`), + ); + }); + + it('reads as one sentence when a rulebook token appears in a skill body', () => { + expect(() => rewriteInvocationTokens('See {rulebook:nmr-scripts}.', CLAUDE_SIGILS, HOST)).toThrow( + 'Unusable invocation token {rulebook:nmr-scripts} in skills/wrap-up/SKILL.md: it is honored only in a rulebook body.', + ); }); it('handles hyphenated multi-segment slugs', () => { - expect(rewriteInvocationTokens('{skill:plan-orchestrable-steps}', CLAUDE_SIGILS)).toBe('/plan-orchestrable-steps'); + expect(rewriteInvocationTokens('{skill:plan-orchestrable-steps}', CLAUDE_SIGILS, HOST)).toBe( + '/plan-orchestrable-steps', + ); }); it('renders multiple tokens of both kinds on a single line', () => { const content = 'First {skill:plan}, then {subagent:planner}, then {skill:review-branch}.'; - expect(rewriteInvocationTokens(content, CLAUDE_SIGILS)).toBe('First /plan, then planner, then /review-branch.'); + expect(rewriteInvocationTokens(content, CLAUDE_SIGILS, HOST)).toBe( + 'First /plan, then planner, then /review-branch.', + ); }); it('preserves adjacent prose and inline-code backticks around a token', () => { - expect(rewriteInvocationTokens('Use `{skill:commit}` here.', CLAUDE_SIGILS)).toBe('Use `/commit` here.'); + expect(rewriteInvocationTokens('Use `{skill:commit}` here.', CLAUDE_SIGILS, HOST)).toBe('Use `/commit` here.'); }); it('does not match a token whose slug is not letter-led', () => { const content = 'Not tokens: {skill:9lives} and {skill:-leading-hyphen}.'; - expect(rewriteInvocationTokens(content, CLAUDE_SIGILS)).toBe(content); + expect(rewriteInvocationTokens(content, CLAUDE_SIGILS, HOST)).toBe(content); }); it('does not match an empty slug or an unknown kind', () => { const content = 'Not tokens: {skill:} and {agent:foo} and {tool:Read}.'; - expect(rewriteInvocationTokens(content, CLAUDE_SIGILS)).toBe(content); + expect(rewriteInvocationTokens(content, CLAUDE_SIGILS, HOST)).toBe(content); }); }); diff --git a/packages/agents/src/lib/__tests__/rulebook-transform.test.ts b/packages/agents/src/lib/__tests__/rulebook-transform.test.ts index dbaa07d6..790d4558 100644 --- a/packages/agents/src/lib/__tests__/rulebook-transform.test.ts +++ b/packages/agents/src/lib/__tests__/rulebook-transform.test.ts @@ -1,9 +1,30 @@ import { describe, expect, it } from 'vitest'; +import type { RulebookInvocationCatalog } from '../invocation-tokens.ts'; import { renderRulebookBody, type RulebookRenderContext } from '../rulebook-transform.ts'; -const CLAUDE_CONTEXT: RulebookRenderContext = { homeDir: '.claude', harnessId: 'claude' }; -const ROVO_CONTEXT: RulebookRenderContext = { homeDir: '.rovodev', harnessId: 'rovodev' }; +// `shell-conventions` carries a `skill-name` override; `nmr-cheatsheet` is ambient-only, so it deploys no skill. +const RULEBOOKS: RulebookInvocationCatalog = new Map([ + ['a-rulebook', { skillName: 'consult-a-rulebook', skill: true }], + ['nmr-cheatsheet', { skillName: 'consult-nmr-cheatsheet', skill: false }], + ['nmr-scripts', { skillName: 'consult-nmr-scripts', skill: true }], + ['shell-conventions', { skillName: 'shell-rules', skill: true }], +]); + +const CLAUDE_CONTEXT: RulebookRenderContext = { + homeDir: '.claude', + harnessId: 'claude', + skillSigil: '/', + subagentSigil: '', + rulebooks: RULEBOOKS, +}; +const ROVO_CONTEXT: RulebookRenderContext = { + homeDir: '.rovodev', + harnessId: 'rovodev', + skillSigil: '!', + subagentSigil: '', + rulebooks: RULEBOOKS, +}; describe(renderRulebookBody, () => { describe('link rewriting', () => { @@ -55,6 +76,54 @@ describe(renderRulebookBody, () => { }); }); + describe('invocation tokens', () => { + it('renders a rulebook token as the deployed skill name of the target rulebook', () => { + const body = 'See {rulebook:nmr-scripts} for the full reference.'; + expect(renderRulebookBody(body, 'a-rulebook', CLAUDE_CONTEXT)).toBe( + 'See /consult-nmr-scripts for the full reference.', + ); + expect(renderRulebookBody(body, 'a-rulebook', ROVO_CONTEXT)).toBe( + 'See !consult-nmr-scripts for the full reference.', + ); + }); + + it('renders a rulebook token through the skill-name override on the target', () => { + expect(renderRulebookBody('See {rulebook:shell-conventions}.', 'a-rulebook', CLAUDE_CONTEXT)).toBe( + 'See /shell-rules.', + ); + }); + + it('renders skill and subagent tokens alongside rulebook tokens', () => { + const body = 'Invoke {skill:capture-feedback}, dispatch {subagent:planner}, read {rulebook:nmr-scripts}.'; + expect(renderRulebookBody(body, 'a-rulebook', CLAUDE_CONTEXT)).toBe( + 'Invoke /capture-feedback, dispatch planner, read /consult-nmr-scripts.', + ); + }); + + it('renders a token naming the rulebook itself', () => { + expect(renderRulebookBody('Re-read {rulebook:a-rulebook}.', 'a-rulebook', CLAUDE_CONTEXT)).toBe( + 'Re-read /consult-a-rulebook.', + ); + }); + + it('rejects a token naming an ambient-only rulebook, pointing at dependencies:', () => { + expect(() => renderRulebookBody('See {rulebook:nmr-cheatsheet}.', 'a-rulebook', CLAUDE_CONTEXT)).toThrow( + /a-rulebook[\s\S]*\{rulebook:nmr-cheatsheet\}[\s\S]*ambient-only[\s\S]*`dependencies:`/, + ); + }); + + it('rejects a token naming no deployed rulebook', () => { + expect(() => renderRulebookBody('See {rulebook:never-declared}.', 'a-rulebook', CLAUDE_CONTEXT)).toThrow( + /\{rulebook:never-declared\}[\s\S]*no rulebook in the deployed set/, + ); + }); + + it('reports every offending token in one error', () => { + const body = 'See {rulebook:never-declared} and {rulebook:nmr-cheatsheet}.'; + expect(() => renderRulebookBody(body, 'a-rulebook', CLAUDE_CONTEXT)).toThrow(/2 unusable invocation token/); + }); + }); + describe('template variables', () => { it('expands {harness_home_dir} and {harness_id}', () => { const body = 'Run {harness_home_dir}/scripts/emit.mjs --harness {harness_id}.'; @@ -77,7 +146,6 @@ describe(renderRulebookBody, () => { { name: 'a target under subagents/', target: '../../subagents/canary.md' }, { name: 'a target under _partials/', target: '../../_partials/shared.md' }, { name: 'a target under collections/', target: '../../collections/library.md' }, - { name: 'a sibling rulebook', target: './other-rulebook.md' }, { name: 'a target in the content root itself', target: '../../README.md' }, ])('rejects $name', ({ target }) => { expect(() => renderRulebookBody(`See [x](${target}).`, 'a-rulebook', CLAUDE_CONTEXT)).toThrow( @@ -85,6 +153,16 @@ describe(renderRulebookBody, () => { ); }); + it.each([ + { name: 'a sibling rulebook', target: './nmr-scripts.md' }, + { name: 'a sibling rulebook named without a leading dot', target: 'nmr-scripts.md' }, + { name: 'an ambient-only sibling, which is undeliverable either way', target: './nmr-cheatsheet.md' }, + ])('rejects $name, naming the token that replaces the link', ({ target }) => { + expect(() => renderRulebookBody(`See [x](${target}).`, 'a-rulebook', CLAUDE_CONTEXT)).toThrow( + /invoked rather than linked: write \{rulebook:nmr-[a-z]+\} instead/, + ); + }); + it('rejects a target escaping the content root', () => { expect(() => renderRulebookBody('See [x](../../../elsewhere/a.md).', 'a-rulebook', CLAUDE_CONTEXT)).toThrow( /escapes the content root/, diff --git a/packages/agents/src/lib/__tests__/subagent-transform.test.ts b/packages/agents/src/lib/__tests__/subagent-transform.test.ts index 9175aa87..9e0c2c68 100644 --- a/packages/agents/src/lib/__tests__/subagent-transform.test.ts +++ b/packages/agents/src/lib/__tests__/subagent-transform.test.ts @@ -158,7 +158,11 @@ describe(renderSubagentForHarness, () => { const toolMapping = loadToolMapping(overlayYaml); const merged = mergeFrontmatter(SOURCE, overlayYaml); const rewrittenTools = rewriteToolNames(merged, toolMapping, 'subagents/demo-agent.md'); - const rewrittenInvocations = rewriteInvocationTokens(rewrittenTools, { skillSigil, subagentSigil }); + const rewrittenInvocations = rewriteInvocationTokens( + rewrittenTools, + { skillSigil, subagentSigil }, + 'subagents/demo-agent.md', + ); const rewrittenPaths = rewriteMarkdownPaths(rewrittenInvocations, 'demo-agent.md', homeDir); const expected = rewriteTemplateVariables(rewrittenPaths, homeDir, harnessId); diff --git a/packages/agents/src/lib/dependency-resolver.ts b/packages/agents/src/lib/dependency-resolver.ts index 06f074fb..959e8d21 100644 --- a/packages/agents/src/lib/dependency-resolver.ts +++ b/packages/agents/src/lib/dependency-resolver.ts @@ -26,9 +26,9 @@ export interface ResolvedClosure { /** * Expands the directly-declared artifacts into their transitive closure, reading each visited artifact's edges — a * collection's `members:`, every other type's `dependencies:`, a subagent's top-level `skills:` injection list, plus - * the invocation tokens in a skill's or subagent's include-expanded body — and following them across every type. The - * result is - * deduped (a diamond dependency appears once) and acyclic — a cycle throws an error naming the offending path. A + * the invocation tokens in a rulebook's, skill's, or subagent's body — and following them across every type. The + * result is deduped (a diamond dependency appears once) and acyclic — a cycle throws an error naming the offending + * path. A * collection is a traversal-only node: its members are followed but the collection itself is dropped from the * deployable result. A referenced artifact that resolves from no source or the library throws an error naming its * type and slug and every location searched. @@ -91,8 +91,10 @@ export async function resolveClosure(direct: DirectArtifacts, resolver: SourceRe * partial becomes an edge for every artifact that includes it — and a subagent further unions its top-level `skills:` * injection list. A body token that names the artifact itself is dropped rather than unioned: a self-reference renders * per harness but is not a dependency and must not trip the cycle check; a self-dependency written in `dependencies:` - * is not dropped, so it still errors. A rulebook keeps `dependencies:` only; its body is embedded without the render - * pass. Every unioned edge enters the closure without a duplicate `dependencies:` declaration. + * is not dropped, so it still errors. A rulebook unions its own body tokens the same way, reading them off the file as + * read: its frontmatter file is also its body file, and it carries no includes to expand. A `{rulebook:}` token + * is unioned only from a rulebook, because only a rulebook body renders one. Every unioned edge enters the closure + * without a duplicate `dependencies:` declaration. */ async function readArtifactEdges( type: ArtifactType, @@ -117,9 +119,14 @@ async function readArtifactEdges( } const dependencies = readDependencies(content, label); - // A rulebook's body is embedded without the render pass, so only its declared `dependencies:` are edges. if (type === 'rulebook') { - return dependencies; + const tokens = extractInvocationEdges(content); + return { + ...dependencies, + rulebook: [...(dependencies.rulebook ?? []), ...tokens.rulebooks.filter((edge) => edge !== slug)], + skill: [...(dependencies.skill ?? []), ...tokens.skills], + subagent: [...(dependencies.subagent ?? []), ...tokens.subagents], + }; } // For a skill or subagent, the frontmatter file is also the body file. Expand its includes to match the render diff --git a/packages/agents/src/lib/invocation-tokens.ts b/packages/agents/src/lib/invocation-tokens.ts index 96e85173..e583e366 100644 --- a/packages/agents/src/lib/invocation-tokens.ts +++ b/packages/agents/src/lib/invocation-tokens.ts @@ -1,13 +1,30 @@ /** - * Matches `{skill:}` and `{subagent:}` invocation tokens. The slug is kebab-case and letter-led - * (`[a-z][a-z0-9-]*`). The pattern only captures well-formed tokens; a slug naming no library artifact is caught - * downstream by the resolver's existing missing-artifact check, so the grammar deliberately does not police existence. + * Matches `{rulebook:}`, `{skill:}`, and `{subagent:}` invocation tokens. The slug is kebab-case and + * letter-led (`[a-z][a-z0-9-]*`). The pattern only captures well-formed tokens; a slug naming no library artifact is + * caught downstream by the resolver's existing missing-artifact check, so the grammar deliberately does not police + * existence. * * One shared constant serves both the render surface (`rewriteInvocationTokens`) and the edge surface * (`extractInvocationEdges`) so the two can never diverge on what a token is. Sharing it is safe: `String.replace` * resets `lastIndex` and `String.matchAll` clones the regex, so neither call leaks match state to the other. */ -const INVOCATION_TOKEN_RE = /\{(skill|subagent):([a-z][a-z0-9-]*)\}/g; +const INVOCATION_TOKEN_RE = /\{(rulebook|skill|subagent):([a-z][a-z0-9-]*)\}/g; + +/** How a rulebook is addressed by an invocation token: the skill name it deploys under, and whether it deploys as one. */ +export interface RulebookInvocationTarget { + readonly skillName: string; + readonly skill: boolean; +} + +/** + * The rulebooks a body may address by token, keyed by slug. Supplied by hosts that render rulebook bodies and absent + * everywhere else, which is what makes a `{rulebook:}` token outside a rulebook fail rather than pass through. + */ +export type RulebookInvocationCatalog = ReadonlyMap; + +/** What a `{rulebook:}` token renders to, or the reason it cannot render. */ +export type RulebookTokenResolution = + { readonly kind: 'rejected'; readonly reason: string } | { readonly kind: 'resolved'; readonly skillName: string }; /** The per-harness sigils prefixed to a rendered invocation token's slug. */ export interface InvocationSigils { @@ -19,6 +36,7 @@ export interface InvocationSigils { /** Invocation slugs extracted from a body, grouped by token kind. Each list preserves source order and may repeat. */ export interface InvocationEdges { + readonly rulebooks: ReadonlyArray; readonly skills: ReadonlyArray; readonly subagents: ReadonlyArray; } @@ -29,6 +47,7 @@ export interface InvocationEdges { * not. */ export function extractInvocationEdges(content: string): InvocationEdges { + const rulebooks: Array = []; const skills: Array = []; const subagents: Array = []; for (const [, kind, slug] of content.matchAll(INVOCATION_TOKEN_RE)) { @@ -37,22 +56,68 @@ export function extractInvocationEdges(content: string): InvocationEdges { if (slug === undefined) { continue; } - if (kind === 'skill') { + if (kind === 'rulebook') { + rulebooks.push(slug); + } else if (kind === 'skill') { skills.push(slug); } else { subagents.push(slug); } } - return { skills, subagents }; + return { rulebooks, skills, subagents }; } /** - * Replaces every `{skill:}` / `{subagent:}` token in `content` with its harness sigil followed by the slug. - * Unlike `rewriteToolNames`, this never throws: the sigil is a fixed property of the typed harness config, so there is - * no unmapped-name failure path. Non-token text passes through unchanged. + * Resolves a `{rulebook:}` token to the skill name it renders, or to the reason it cannot render. One resolution + * serves both surfaces that need it — the rewriter, which throws on the first rejection, and the rulebook validator, + * which collects every rejection into one error — so neither can report a rejection the other would not. */ -export function rewriteInvocationTokens(content: string, sigils: InvocationSigils): string { +export function resolveRulebookToken( + slug: string, + rulebooks: RulebookInvocationCatalog | undefined, +): RulebookTokenResolution { + if (rulebooks === undefined) { + return { kind: 'rejected', reason: 'is honored only in a rulebook body' }; + } + const target = rulebooks.get(slug); + if (target === undefined) { + return { kind: 'rejected', reason: 'names no rulebook in the deployed set' }; + } + if (!target.skill) { + return { + kind: 'rejected', + reason: + 'names an ambient-only rulebook, which deploys no skill to invoke; express the relationship with ' + + '`dependencies:` instead', + }; + } + return { kind: 'resolved', skillName: target.skillName }; +} + +/** + * Replaces every invocation token in `content` with its harness sigil followed by the slug it invokes. A + * `{rulebook:}` token renders the skill sigil and the target's deployed skill name, resolved through + * `rulebooks` — so a rulebook is addressed by the name it actually deploys under, not by its slug. + * + * Throws when a rulebook token cannot render: no catalog (the host does not honor them), an unknown slug, or an + * ambient-only target. `sourceLabel` names the host in that error, so an author sees which file to fix. Skill and + * subagent tokens have no such failure path — their sigils are fixed properties of the typed harness config. + * Non-token text passes through unchanged. + */ +export function rewriteInvocationTokens( + content: string, + sigils: InvocationSigils, + sourceLabel: string, + rulebooks?: RulebookInvocationCatalog, +): string { return content.replace(INVOCATION_TOKEN_RE, (_match: string, kind: string, slug: string): string => { + if (kind === 'rulebook') { + const resolution = resolveRulebookToken(slug, rulebooks); + if (resolution.kind === 'rejected') { + throw new Error(`Unusable invocation token {rulebook:${slug}} in ${sourceLabel}: it ${resolution.reason}.`); + } + return `${sigils.skillSigil}${resolution.skillName}`; + } const sigil = kind === 'skill' ? sigils.skillSigil : sigils.subagentSigil; return `${sigil}${slug}`; }); diff --git a/packages/agents/src/lib/rulebook-transform.ts b/packages/agents/src/lib/rulebook-transform.ts index fafe2877..ca3abb72 100644 --- a/packages/agents/src/lib/rulebook-transform.ts +++ b/packages/agents/src/lib/rulebook-transform.ts @@ -1,5 +1,11 @@ import path from 'node:path'; +import { + extractInvocationEdges, + resolveRulebookToken, + rewriteInvocationTokens, + type RulebookInvocationCatalog, +} from './invocation-tokens.ts'; import { listRewritableLinkTargets, rewriteMarkdownPaths, rewriteTemplateVariables } from './path-rewriter.ts'; /** The per-harness inputs a rulebook body render depends on, resolved once per harness by the caller. */ @@ -11,6 +17,12 @@ export interface RulebookRenderContext { readonly homeDir: string; /** Harness identifier that `{harness_id}` tokens expand to (e.g. `claude`). */ readonly harnessId: string; + /** Sigil prefixed to a rendered `{skill:}` or `{rulebook:}` token (e.g. `/` for Claude). */ + readonly skillSigil: string; + /** Sigil prefixed to a rendered `{subagent:}` token (empty on both current harnesses). */ + readonly subagentSigil: string; + /** The deployed rulebooks a `{rulebook:}` token may address, keyed by slug. */ + readonly rulebooks: RulebookInvocationCatalog; } /** @@ -28,17 +40,25 @@ const LINKABLE_ROOTS: ReadonlyArray = ['scripts', 'skills']; /** * Renders one rulebook's neutral body for a single harness: relative Markdown links become that harness's absolute - * paths, and template variables expand. Link targets are validated first, so a target naming a location the delivery - * pipeline never creates fails the run instead of shipping as a dead path. + * paths, invocation tokens become that harness's sigil plus the slug they invoke, and template variables expand. Link + * targets and rulebook tokens are validated first, so a reference the delivery pipeline cannot honor fails the run + * instead of shipping as a dead address. * * `slug` anchors link rewriting: a relative target resolves against `guidance/rulebooks/.md`, matching where the - * rulebook sits in its content root, and the emitted path is rooted at the harness home. Invocation tokens are - * deliberately not rewritten -- they carry dependency-edge semantics that `dependencies:` expresses for rulebooks. + * rulebook sits in its content root, and the emitted path is rooted at the harness home. A `{rulebook:}` token + * renders the deployed skill name of the rulebook it names, which is why the context carries the deployed set. */ export function renderRulebookBody(body: string, slug: string, context: RulebookRenderContext): string { assertLinkTargetsAreDeliverable(body, slug); + assertRulebookTokensResolve(body, slug, context.rulebooks); const pathRewritten = rewriteMarkdownPaths(body, `${RULEBOOK_SOURCE_DIR}/${slug}.md`, context.homeDir); - return rewriteTemplateVariables(pathRewritten, context.homeDir, context.harnessId); + const tokenRewritten = rewriteInvocationTokens( + pathRewritten, + context, + `${RULEBOOK_SOURCE_DIR}/${slug}.md`, + context.rulebooks, + ); + return rewriteTemplateVariables(tokenRewritten, context.homeDir, context.harnessId); } // region | Helpers @@ -70,6 +90,27 @@ function assertLinkTargetsAreDeliverable(body: string, slug: string): void { } } +/** + * Throws when any `{rulebook:}` token in `body` names a rulebook that deploys no skill to invoke. Every + * offending token is reported together, matching how link targets are reported, so an author fixing a rulebook sees + * the whole list rather than one token per run. + */ +function assertRulebookTokensResolve(body: string, slug: string, rulebooks: RulebookInvocationCatalog): void { + const rejections: Array = []; + for (const target of extractInvocationEdges(body).rulebooks) { + const resolution = resolveRulebookToken(target, rulebooks); + if (resolution.kind === 'rejected') { + rejections.push(` {rulebook:${target}} -- it ${resolution.reason}`); + } + } + + if (rejections.length > 0) { + throw new Error( + `Rulebook "${slug}" carries ${rejections.length} unusable invocation token(s):\n${rejections.join('\n')}`, + ); + } +} + /** Names why a link target cannot be delivered, or `undefined` when it resolves into a linkable root. */ function describeRejection(target: string): string | undefined { const hashIndex = target.indexOf('#'); @@ -79,6 +120,10 @@ function describeRejection(target: string): string | undefined { if (resolved === '..' || resolved.startsWith('../')) { return 'escapes the content root'; } + if (resolved.startsWith(`${RULEBOOK_SOURCE_DIR}/`)) { + const targetSlug = path.posix.basename(resolved, '.md'); + return `names rulebook "${targetSlug}", which is invoked rather than linked: write {rulebook:${targetSlug}} instead`; + } const root = resolved.split('/', 1)[0]; if (root === undefined || !LINKABLE_ROOTS.includes(root)) { return `resolves to "${resolved}", which is not under a linkable root`; diff --git a/packages/agents/src/lib/skill-transform.ts b/packages/agents/src/lib/skill-transform.ts index 3554d78a..29e032ae 100644 --- a/packages/agents/src/lib/skill-transform.ts +++ b/packages/agents/src/lib/skill-transform.ts @@ -103,7 +103,7 @@ async function renderMarkdown( const contextLabel = path.relative(contentRoot, srcPath).split(path.sep).join('/'); const expanded = await expandIncludes(srcPath, contentRoot); const toolRewritten = rewriteToolNames(expanded, toolMapping, contextLabel); - const invocationRewritten = rewriteInvocationTokens(toolRewritten, { skillSigil, subagentSigil }); + const invocationRewritten = rewriteInvocationTokens(toolRewritten, { skillSigil, subagentSigil }, contextLabel); const pathRewritten = rewriteMarkdownPaths(invocationRewritten, `${slug}/${relPath}`, pathPrefix); return rewriteTemplateVariables(pathRewritten, homeDir, harnessId); } diff --git a/packages/agents/src/lib/subagent-deploy.ts b/packages/agents/src/lib/subagent-deploy.ts index 6733703e..9a42c621 100644 --- a/packages/agents/src/lib/subagent-deploy.ts +++ b/packages/agents/src/lib/subagent-deploy.ts @@ -49,9 +49,20 @@ export async function deploySubagent( destPath: string, context: SubagentDeployContext, ): Promise { + const rendered = await renderSubagent(resolved, context); + await mkdir(path.dirname(destPath), { recursive: true }); + await writeIfChanged(destPath, subagentMarker.injectMarker(rendered, resolved.slug)); +} + +/** + * Renders a resolved subagent for one harness: include expansion, then the harness transform. Throws on a broken + * include, an unmapped `{tool:NAME}` placeholder, or a `{rulebook:}` token, which no subagent body may carry. + * The pre-write render gate and `deploySubagent` share this one path, so the gate raises exactly what the write would. + */ +export async function renderSubagent(resolved: ResolvedSubagent, context: SubagentDeployContext): Promise { const expanded = await expandIncludes(resolved.srcPath, resolved.contentRoot); const fileName = `${resolved.slug}.md`; - const rendered = renderSubagentForHarness(expanded, { + return renderSubagentForHarness(expanded, { overlayYaml: context.overlayYaml, toolMapping: context.toolMapping, fileRelPath: fileName, @@ -62,8 +73,6 @@ export async function deploySubagent( skillSigil: context.skillSigil, subagentSigil: context.subagentSigil, }); - await mkdir(path.dirname(destPath), { recursive: true }); - await writeIfChanged(destPath, subagentMarker.injectMarker(rendered, resolved.slug)); } /** diff --git a/packages/agents/src/lib/subagent-transform.ts b/packages/agents/src/lib/subagent-transform.ts index 6b1da006..b3c65096 100644 --- a/packages/agents/src/lib/subagent-transform.ts +++ b/packages/agents/src/lib/subagent-transform.ts @@ -51,7 +51,7 @@ export function renderSubagentForHarness( ): string { const merged = mergeFrontmatter(expandedSource, overlayYaml); const rewrittenTools = rewriteToolNames(merged, toolMapping, sourceLabel); - const rewrittenInvocations = rewriteInvocationTokens(rewrittenTools, { skillSigil, subagentSigil }); + const rewrittenInvocations = rewriteInvocationTokens(rewrittenTools, { skillSigil, subagentSigil }, sourceLabel); const rewrittenPaths = rewriteMarkdownPaths(rewrittenInvocations, fileRelPath, pathPrefix); return rewriteTemplateVariables(rewrittenPaths, homeDir, harnessId); }