diff --git a/packages/agents/content/_partials/README.md b/packages/agents/content/_partials/README.md index d147824e..03b1bc5e 100644 --- a/packages/agents/content/_partials/README.md +++ b/packages/agents/content/_partials/README.md @@ -12,7 +12,7 @@ Three include shapes are recognized. Each must occupy a full line, with optional | ------------- | ------------------------------------------------ | --------------------------------------------------------------------------------- | | Self-close | `` | Inline a partial with no slot content (or use the partial's empty-slot defaults). | | Open + close | `` ... `` | Inline a partial and pass slot content into its `` placeholder. | -| Children slot | `` | Inside a partial: marks where the caller's slot content is substituted. | +| Children slot | `` | Inside a partial: Marks where the caller's slot content is substituted. | Self-close is matched before open so that a path with a trailing slash is read correctly as a self-close, not as an open directive whose path ends with a slash. @@ -41,12 +41,39 @@ For each `.md` source file the install pipeline performs, in order: 1. **Expand includes.** `expandIncludes(srcPath, contentDir)` resolves all directive shapes recursively and substitutes slot content. 2. **Merge frontmatter** (subagents only). Platform-specific frontmatter overrides from `_data/{platform}.yml` are merged into the source's frontmatter. -3. **Inject the provenance marker.** A `GENERATED FILE` comment is added at the top of the output, with a `Source:` link to the original file. -4. **Rewrite paths** (skills only, post-write). Bare-relative Markdown links are rewritten to absolute platform paths. -5. **Write the destination file.** +3. **Rewrite tool-name placeholders.** `rewriteToolNames(content, mapping)` replaces each `{tool:NAME}` placeholder using the platform's `_tools:` mapping from the same overlay YAML. An unmapped name is a fatal install error anchored to the source file and line. See [Tool-name placeholders](#tool-name-placeholders). +4. **Inject the provenance marker.** A `GENERATED FILE` comment is added at the top of the output, with a `Source:` link to the original file. +5. **Rewrite paths** (skills only, post-write). Bare-relative Markdown links are rewritten to absolute platform paths. +6. **Write the destination file.** + +For subagents, all steps run on the in-memory merged string before write. For directory-form skills, step 3 runs on each value of the in-memory `expandedDirContents` map before `writeExpandedSkillDir` writes files to disk; step 5 (path rewriting) then runs as a second pass over the written tree. For flat-file skills, step 3 runs on `expandedFileContent` before `writeFile`. Expansion runs before the dry-run gate, so missing partials, cycles, and out-of-tree references surface even when no files would be written. +## Tool-name placeholders + +Subagent and skill body text reference tools using the `{tool:NAME}` placeholder so the same source can install for platforms that name their tools differently. `NAME` is the canonical (Claude) tool name (`Read`, `Write`, `Edit`, `Bash`, `Grep`, `Glob`). The install pipeline rewrites each placeholder using the platform's `_tools:` mapping, which lives at the top of each overlay YAML at `content/subagents/_data/{platform}.yml`. + +```yaml +# content/subagents/_data/rovodev.yml +_tools: + Bash: bash + Edit: find_and_replace_code + Glob: expand_folder + Grep: grep + Read: open_files + Write: create_file +``` + +When a placeholder names a tool not present in the overlay's `_tools:` mapping, the rewriter aborts install with a fatal error anchored to the source file and line. There is no identity pass-through — every match must resolve through the mapping. This catches typos (e.g., `{tool:Reed}`) and out-of-date placeholders at install time rather than at agent runtime. + +**Authoring guidance:** + +- Use `{tool:NAME}` for body prose that names a tool *as a tool*, not for English verbs ("Read the file", "Write a paragraph", "Read project guidelines" are not migrated). +- Preserve surrounding context: `` `Write` `` becomes `` `{tool:Write}` ``; bare `Write` becomes `{tool:Write}`. +- Do **not** use placeholders in frontmatter `tools:` values. Frontmatter is replaced wholesale by the overlay merger; placeholders there would create two overlapping mechanisms. +- The placeholder mechanism is body-only, applied to subagent and skill `.md` files. Guidance files (`content/guidance/`) are not wired through the rewriter. + ## Verbatim slot substitution When a partial contains ``, expansion removes that line and inserts the caller's slot lines verbatim — no leading-trim, no trailing-trim, no blank-line collapsing. Partial authors control the spacing on their side; caller authors control the spacing on theirs. @@ -55,7 +82,7 @@ A consequence: avoid placing blank lines on both sides of a `` ## Common patterns -### Bare self-close — no slot +### Bare self-close — No slot Use when the partial has no `` placeholder, or when the caller wants the partial's empty-slot rendering: diff --git a/packages/agents/content/subagents/_data/claude.yml b/packages/agents/content/subagents/_data/claude.yml index 46ff3657..3260c505 100644 --- a/packages/agents/content/subagents/_data/claude.yml +++ b/packages/agents/content/subagents/_data/claude.yml @@ -1,4 +1,17 @@ # claude.yml + +# Body-text tool-name mapping for the {tool:NAME} placeholder rewriter. +# Authoring guide: ../../_partials/README.md (Tool-name placeholders). +# Keys are canonical PascalCase tool names; values are platform-native names. +# Claude uses canonical names, so this is an identity mapping. +_tools: + Bash: Bash + Edit: Edit + Glob: Glob + Grep: Grep + Read: Read + Write: Write + _defaults: permissionMode: bypassPermissions diff --git a/packages/agents/content/subagents/_data/rovodev.yml b/packages/agents/content/subagents/_data/rovodev.yml index 2bd95157..baca3c8a 100644 --- a/packages/agents/content/subagents/_data/rovodev.yml +++ b/packages/agents/content/subagents/_data/rovodev.yml @@ -1,4 +1,18 @@ # rovodev.yml + +# Body-text tool-name mapping for the {tool:NAME} placeholder rewriter. +# Authoring guide: ../../_partials/README.md (Tool-name placeholders). +# Keys are canonical PascalCase tool names; values are platform-native names. +# `Glob` has no exact Rovo Dev counterpart: `expand_folder` is the closest directory-exploration analogue, accepted as +# the mapping for prose contexts. +_tools: + Bash: bash + Edit: find_and_replace_code + Glob: expand_folder + Grep: grep + Read: open_files + Write: create_file + _defaults: tools: [bash, create_file, expand_code_chunks, expand_folder, grep, open_files] diff --git a/packages/agents/content/subagents/_partials/coder-writes-hard-gate.md b/packages/agents/content/subagents/_partials/coder-writes-hard-gate.md index b7853cf2..8634340c 100644 --- a/packages/agents/content/subagents/_partials/coder-writes-hard-gate.md +++ b/packages/agents/content/subagents/_partials/coder-writes-hard-gate.md @@ -1,5 +1,5 @@ -For multi-task plans (implementation mode) and for every review-response round, your FIRST implementation tool use MUST be a `Write` of the change-summary scaffold to the orchestrator-supplied artifact path. This guarantees a durable, structurally-complete artifact exists even if your dispatch is interrupted by `max_turns` exhaustion or any other failure. +For multi-task plans (implementation mode) and for every review-response round, your FIRST implementation tool use MUST be a `{tool:Write}` of the change-summary scaffold to the orchestrator-supplied artifact path. This guarantees a durable, structurally-complete artifact exists even if your dispatch is interrupted by `max_turns` exhaustion or any other failure. Single-task implementation plans are exempt — write the artifact once at the end. diff --git a/packages/agents/content/subagents/_partials/review-writes-hard-gate.md b/packages/agents/content/subagents/_partials/review-writes-hard-gate.md index 9c07a96b..bcd41d4c 100644 --- a/packages/agents/content/subagents/_partials/review-writes-hard-gate.md +++ b/packages/agents/content/subagents/_partials/review-writes-hard-gate.md @@ -1,4 +1,4 @@ -After reading project guidelines and obtaining the diff (typically 2-3 turns), your NEXT tool use MUST be a `Write` of the review scaffold to the orchestrator-supplied artifact path. Not a `Read`, not a `Grep`, not a `Bash` to inspect files — a `Write`. This guarantees a durable artifact exists at the canonical path even if your dispatch is interrupted by `max_turns` exhaustion or any other failure. +After reading project guidelines and obtaining the diff (typically 2-3 turns), your NEXT tool use MUST be a `{tool:Write}` of the review scaffold to the orchestrator-supplied artifact path. Not a `{tool:Read}`, not a `{tool:Grep}`, not a `{tool:Bash}` to inspect files — a `{tool:Write}`. This guarantees a durable artifact exists at the canonical path even if your dispatch is interrupted by `max_turns` exhaustion or any other failure. diff --git a/packages/agents/content/subagents/_partials/review-writes-scaffold.md b/packages/agents/content/subagents/_partials/review-writes-scaffold.md index 7f0be8b7..dc3f72d9 100644 --- a/packages/agents/content/subagents/_partials/review-writes-scaffold.md +++ b/packages/agents/content/subagents/_partials/review-writes-scaffold.md @@ -1,4 +1,4 @@ -You have `Write` but not `Edit`. Each update is a full overwrite of the artifact file with the growing findings list. +You have `{tool:Write}` but not `{tool:Edit}`. Each update is a full overwrite of the artifact file with the growing findings list. ### Scaffold (first write) diff --git a/packages/agents/content/subagents/orchestrated-architect.md b/packages/agents/content/subagents/orchestrated-architect.md index efe9d5a1..4c3fd48a 100644 --- a/packages/agents/content/subagents/orchestrated-architect.md +++ b/packages/agents/content/subagents/orchestrated-architect.md @@ -15,7 +15,7 @@ You are NOT a planner or coder. You do not write implementation plans or code. Y 1. **Read project guidelines**: Read ~/.agents/AGENTS.md, .agents/PROJECT.md, and any relevant project-specific conventions 2. **Understand the task**: Read the task description carefully. Identify what is being asked. -3. **Explore the codebase**: Use Glob, Grep, and Read to understand relevant patterns, conventions, and architecture. +3. **Explore the codebase**: Use {tool:Glob}, {tool:Grep}, and {tool:Read} to understand relevant patterns, conventions, and architecture. 4. **Validate external plan** (if provided): Check the plan's assumptions against the actual codebase — do referenced files, types, and APIs exist? Does the approach align with established patterns? Are there existing utilities the plan overlooks? Flag invalid assumptions explicitly. If ticket requirements are provided, also verify the plan addresses the ticket's stated requirements and flag any requirements the plan does not cover. 5. **Classify impact**: Determine the architectural impact level based on the criteria below. 6. **Write guidance**: Produce a structured analysis document. @@ -36,7 +36,7 @@ Classify the task into exactly one impact level: - Task follows an existing, well-established pattern - Touches 1-2 files in a single module - No new dependencies or interfaces -- Example: adding a new utility function following existing conventions +- Example: Adding a new utility function following existing conventions ### `medium` @@ -44,7 +44,7 @@ Classify the task into exactly one impact level: - Touches multiple modules or layers - Creates new interfaces or modifies existing contracts - Requires coordination between components -- Example: adding a new API endpoint with validation, persistence, and tests +- Example: Adding a new API endpoint with validation, persistence, and tests ### `high` @@ -52,11 +52,11 @@ Classify the task into exactly one impact level: - Introduces new infrastructure or cross-cutting concerns - Changes affect many downstream consumers - Risk of breaking existing functionality -- Example: migrating state management, changing database schema, adding a new service layer +- Example: Migrating state management, changing database schema, adding a new service layer ## Output format -Write your analysis to the file path provided in your task prompt using the Write tool. The artifact begins with YAML frontmatter conforming to the universal artifact frontmatter schema (defined in the `artifact-conventions` shared data doc) (see [Frontmatter](#frontmatter) below for field resolution). +Write your analysis to the file path provided in your task prompt using the {tool:Write} tool. The artifact begins with YAML frontmatter conforming to the universal artifact frontmatter schema (defined in the `artifact-conventions` shared data doc) (see [Frontmatter](#frontmatter) below for field resolution). The document MUST include: @@ -90,7 +90,7 @@ Include these sections ONLY when the impact level warrants them: ```markdown ### Guidance -{Specific guidance for the coder: which patterns to follow, which files to reference as examples, which conventions apply} +{Specific guidance for the coder: Which patterns to follow, which files to reference as examples, which conventions apply} ``` **If `medium` or higher:** @@ -98,11 +98,11 @@ Include these sections ONLY when the impact level warrants them: ```markdown ### Constraints -{Architectural constraints that must be respected: existing interfaces, naming conventions, module boundaries, dependency rules} +{Architectural constraints that must be respected: Existing interfaces, naming conventions, module boundaries, dependency rules} ### Risks -{What could go wrong: race conditions, breaking changes, performance implications, migration concerns} +{What could go wrong: Race conditions, breaking changes, performance implications, migration concerns} ``` **If `high`:** @@ -130,9 +130,9 @@ The artifact's frontmatter conforms to the universal artifact frontmatter schema -- `provenance.skill`: always `orchestrated-architect`. -- `provenance.isInteractive`: always `false`. -- `provenance.model`: the model identifier you are executing under. Read this from your system-prompt environment block — the line `model named ... model ID is ...`. +- `provenance.skill`: Always `orchestrated-architect`. +- `provenance.isInteractive`: Always `false`. +- `provenance.model`: The model identifier you are executing under. Read this from your system-prompt environment block — the line `model named ... model ID is ...`. ## Principles diff --git a/packages/agents/content/subagents/orchestrated-planner.md b/packages/agents/content/subagents/orchestrated-planner.md index 408a4171..e0c7fd2e 100644 --- a/packages/agents/content/subagents/orchestrated-planner.md +++ b/packages/agents/content/subagents/orchestrated-planner.md @@ -27,7 +27,7 @@ You will receive: 1. **Read project guidelines**: Read ~/.agents/AGENTS.md, .agents/PROJECT.md, and any relevant project-specific conventions 2. **Understand the task**: Read the task description and any architectural guidance. -3. **Explore the codebase**: Use Glob, Grep, and Read to understand the relevant code, patterns, and conventions. Identify the files that will need to change. +3. **Explore the codebase**: Use {tool:Glob}, {tool:Grep}, and {tool:Read} to understand the relevant code, patterns, and conventions. Identify the files that will need to change. 4. **Validate reference plan** (if provided): Compare each step against the codebase. Verify file paths, check for existing utilities that could simplify or replace steps, and confirm the approach aligns with established patterns. Address any assumption issues flagged by the architect. If ticket requirements are provided, verify the plan covers all ticket requirements and flag any gaps. If all plan deliverables already exist with zero changes needed, flag this as a risk — the plan may not match the ticket. 5. **Design the plan**: Break the task into ordered steps with clear acceptance criteria. When a reference plan was provided, use it as the starting point — adopt valid steps, revise or replace invalid ones. 6. **Write output files**: Write plan files to the paths provided in the task prompt. @@ -42,7 +42,7 @@ You will receive: - **Test coverage in acceptance criteria**: When a step creates or modifies testable behavior, its acceptance criteria must include test coverage. See the `testing-conventions` skill for what constitutes testable behavior and the narrow carve-outs where tests may be omitted. - **Documentation coverage in acceptance criteria**: When a step adds, removes, or renames user-facing surface (CLI flags, commands, API endpoints, configuration keys, environment variables), its acceptance criteria must include corresponding updates to documentation, help text, and usage examples — including removal of references to anything that no longer exists. -## Output: plan (Markdown) +## Output: Plan (Markdown) Write the plan Markdown file to the path provided in the task prompt. The artifact begins with YAML frontmatter conforming to the universal artifact frontmatter schema (defined in the `artifact-conventions` shared data doc) (see [Frontmatter](#frontmatter) below for field resolution). Format: @@ -116,7 +116,7 @@ run_id: '{run id}' These four actions (Adopted, Revised, Dropped, Added) are the canonical vocabulary. Map merge, split, and reorder operations to "Revised" — they all produce revised steps from the reference. ``` -## Output: plan (JSON) +## Output: Plan (JSON) Write the plan JSON file to the path provided in the task prompt. Format: @@ -150,9 +150,9 @@ The artifact's frontmatter conforms to the universal artifact frontmatter schema -- `provenance.skill`: always `orchestrated-planner`. -- `provenance.isInteractive`: always `false`. -- `provenance.model`: the model identifier you are executing under. Read this from your system-prompt environment block — the line `model named ... model ID is ...`. +- `provenance.skill`: Always `orchestrated-planner`. +- `provenance.isInteractive`: Always `false`. +- `provenance.model`: The model identifier you are executing under. Read this from your system-prompt environment block — the line `model named ... model ID is ...`. ## Constraints diff --git a/packages/agents/content/subagents/plan-reviewer.md b/packages/agents/content/subagents/plan-reviewer.md index e23891e7..c9af000d 100644 --- a/packages/agents/content/subagents/plan-reviewer.md +++ b/packages/agents/content/subagents/plan-reviewer.md @@ -28,7 +28,7 @@ You will receive: 1. **Read project guidelines**: Read ~/.agents/AGENTS.md, .agents/PROJECT.md, and any relevant project-specific conventions 2. **Read the plan**: Read the full plan file. If orchestration format, also check for a `.json` companion. 3. **Review the ticket**: Review the ticket content provided in your task prompt to understand the requirements the plan must satisfy. -4. **Explore the codebase**: Use Glob, Grep, and Read to verify factual claims in the plan (file existence, API shapes, utility availability, existing patterns). +4. **Explore the codebase**: Use {tool:Glob}, {tool:Grep}, and {tool:Read} to verify factual claims in the plan (file existence, API shapes, utility availability, existing patterns). 5. **Evaluate completeness**: Identify decision gaps the coder would have to fill. 6. **Evaluate correctness**: Identify factual errors and structural issues. 7. **Map requirements coverage**: Trace each requirement to a plan step. @@ -154,9 +154,9 @@ The artifact's frontmatter conforms to the universal artifact frontmatter schema -- `provenance.skill`: always `plan-reviewer`. -- `provenance.isInteractive`: always `false`. -- `provenance.model`: the model identifier you are executing under. Read this from your system-prompt environment block — the line `model named ... model ID is ...`. +- `provenance.skill`: Always `plan-reviewer`. +- `provenance.isInteractive`: Always `false`. +- `provenance.model`: The model identifier you are executing under. Read this from your system-prompt environment block — the line `model named ... model ID is ...`. ## Principles diff --git a/packages/agents/content/subagents/planner.md b/packages/agents/content/subagents/planner.md index dd0145d8..f12ce01a 100644 --- a/packages/agents/content/subagents/planner.md +++ b/packages/agents/content/subagents/planner.md @@ -26,7 +26,7 @@ You will receive: 1. **Read project guidelines**: Read ~/.agents/AGENTS.md, .agents/PROJECT.md, and any relevant project-specific conventions 2. **Understand the story**: Read the full story/task description. Identify the scope, goals, and constraints. -3. **Explore the codebase**: Use Glob, Grep, and Read to understand relevant code, patterns, conventions, and architecture. Identify integration points, existing patterns to follow, and files that will need to change. +3. **Explore the codebase**: Use {tool:Glob}, {tool:Grep}, and {tool:Read} to understand relevant code, patterns, conventions, and architecture. Identify integration points, existing patterns to follow, and files that will need to change. 4. **Reason about architecture**: Consider how the work fits into the existing codebase. Identify risks, unknowns, and decisions that need user input. 5. **Design the steps**: Break the story into independently orchestrable steps. Each step will be executed via `/orchestrate-dev` in its own worktree — it must be fully self-contained. 6. **Write output files**: Write both `{plan-md-path}` and `{plan-json-path}` to the paths provided. @@ -140,9 +140,9 @@ The artifact's frontmatter conforms to the universal artifact frontmatter schema -- `provenance.skill`: always `planner`. -- `provenance.isInteractive`: always `false`. -- `provenance.model`: the model identifier you are executing under. Read this from your system-prompt environment block — the line `model named ... model ID is ...`. +- `provenance.skill`: Always `planner`. +- `provenance.isInteractive`: Always `false`. +- `provenance.model`: The model identifier you are executing under. Read this from your system-prompt environment block — the line `model named ... model ID is ...`. ## Resumption @@ -175,7 +175,7 @@ You have **40 turns** (API round-trips) to complete your work. Each time you cal ## Constraints - **Read-only on project files**: You may read any project file but only write to the provided output paths -- **Bash for exploration only**: Use Bash only for codebase exploration commands (e.g., `git log`, `git diff`) and directory creation (`mkdir -p`) — never for builds, installs, or other side-effect commands +- **{tool:Bash} for exploration only**: Use {tool:Bash} only for codebase exploration commands (e.g., `git log`, `git diff`) and directory creation (`mkdir -p`) — never for builds, installs, or other side-effect commands - **Be specific about file paths**: Use actual paths from your codebase exploration, not placeholders - **Reference existing patterns**: When a step involves creating something new, point to an existing file as a reference implementation - **Don't over-plan**: Match the plan complexity to the story complexity diff --git a/packages/agents/content/subagents/savings-analyzer.md b/packages/agents/content/subagents/savings-analyzer.md index 558469ee..6ce0b8cb 100644 --- a/packages/agents/content/subagents/savings-analyzer.md +++ b/packages/agents/content/subagents/savings-analyzer.md @@ -20,7 +20,7 @@ You receive: 1. **Read run-index.json** -- extract effort, thresholds, model config 2. **Read run-log.jsonl** -- parse all events -3. **Check artifact files** -- use Glob to list files in the run directory; note which agents produced artifacts and which did not +3. **Check artifact files** -- use {tool:Glob} to list files in the run directory; note which agents produced artifacts and which did not 4. **Analyze** -- apply the three-question framework (see below) 5. **Write artifact** -- write `{NN}_analyst_savings-analysis.md` to the run directory @@ -110,18 +110,18 @@ The artifact begins with YAML frontmatter conforming to the universal artifact f Resolve fields before writing the artifact: -- `provenance.skill`: always `savings-analyzer`. -- `provenance.timestamp`: current UTC time in ISO 8601 format. -- `provenance.baseSha`: passed in via your dispatch prompt — the orchestrator resolves `git rev-parse --short origin/main` for the run-summary and forwards it. Omit if not provided. -- `provenance.isInteractive`: always `false`. -- `provenance.model`: the model identifier you are executing under. Read this from your system-prompt environment block — look for the line `model named ... model ID is ...` and use the model ID value. -- `ticket_id`, `ticket_ref`: passed in via your dispatch prompt. Omit when absent. -- `branch`: passed in via your dispatch prompt. -- `commit`: passed in via your dispatch prompt — the short HEAD SHA at run time. -- `pr`: passed in via your dispatch prompt when the dispatcher resolved it via the `pr-resolution` shared data doc. Omit when not provided. -- `run_id`: passed in via your dispatch prompt — the orchestrated run ID. - -Because `savings-analyzer` does not have the Bash tool in its default tool set, fields that normally require Bash (`baseSha`, `commit`, `pr`) are sourced from the dispatch prompt rather than resolved on demand. The dispatcher is responsible for passing these values. +- `provenance.skill`: Always `savings-analyzer`. +- `provenance.timestamp`: Current UTC time in ISO 8601 format. +- `provenance.baseSha`: Passed in via your dispatch prompt — the orchestrator resolves `git rev-parse --short origin/main` for the run-summary and forwards it. Omit if not provided. +- `provenance.isInteractive`: Always `false`. +- `provenance.model`: The model identifier you are executing under. Read this from your system-prompt environment block — look for the line `model named ... model ID is ...` and use the model ID value. +- `ticket_id`, `ticket_ref`: Passed in via your dispatch prompt. Omit when absent. +- `branch`: Passed in via your dispatch prompt. +- `commit`: Passed in via your dispatch prompt — the short HEAD SHA at run time. +- `pr`: Passed in via your dispatch prompt when the dispatcher resolved it via the `pr-resolution` shared data doc. Omit when not provided. +- `run_id`: Passed in via your dispatch prompt — the orchestrated run ID. + +Because `savings-analyzer` does not have the {tool:Bash} tool in its default tool set, fields that normally require {tool:Bash} (`baseSha`, `commit`, `pr`) are sourced from the dispatch prompt rather than resolved on demand. The dispatcher is responsible for passing these values. ## ARTIFACT-WRITE SAFEGUARD diff --git a/packages/agents/src/commands/__tests__/install-tool-names.test.ts b/packages/agents/src/commands/__tests__/install-tool-names.test.ts new file mode 100644 index 00000000..4d512c99 --- /dev/null +++ b/packages/agents/src/commands/__tests__/install-tool-names.test.ts @@ -0,0 +1,152 @@ +import { mkdir, readFile, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { ToolNameRewriteError } from '../../lib/tool-name-rewriter.js'; +import type { InstallOptions } from '../../lib/types.js'; +import { installCommand } from '../install.js'; + +/** + * Integration coverage for the `{tool:NAME}` placeholder rewriter. Builds a synthetic content tree with + * deliberately-targeted placeholders, runs the install pipeline against it, and asserts both the success path + * (Rovo Dev rewrites prose to mapped names) and the failure path (an unmapped placeholder aborts install with a + * file-and-line-anchored error). + */ +describe('tool-name placeholder rewriting end-to-end', () => { + let tempDir: string; + let contentDir: string; + const installOptions: InstallOptions = { platform: 'rovodev', link: false, force: false, dryRun: false }; + + beforeEach(async () => { + tempDir = path.join(tmpdir(), `agents-test-tool-names-${Date.now()}-${Math.random().toString(36).slice(2)}`); + contentDir = path.join(tempDir, 'content'); + await mkdir(path.join(tempDir, '.rovodev', 'subagents'), { recursive: true }); + await mkdir(path.join(tempDir, '.rovodev', 'skills'), { recursive: true }); + await mkdir(path.join(contentDir, 'skills'), { recursive: true }); + await mkdir(path.join(contentDir, 'subagents', '_data'), { recursive: true }); + await mkdir(path.join(contentDir, 'guidance', 'shared'), { recursive: true }); + + // Minimal Rovo Dev overlay with a complete _tools: mapping. + await writeFile( + path.join(contentDir, 'subagents', '_data', 'rovodev.yml'), + [ + '_tools:', + ' Bash: bash', + ' Edit: find_and_replace_code', + ' Glob: expand_folder', + ' Grep: grep', + ' Read: open_files', + ' Write: create_file', + '', + '_defaults:', + ' tools: [bash, open_files, create_file]', + '', + ].join('\n'), + ); + + // Stub shared guidance so installSharedGuidance has something to iterate over. + await writeFile(path.join(contentDir, 'guidance', 'shared', 'AGENTS.md'), '# Stub\n'); + }); + + afterEach(async () => { + await rm(tempDir, { recursive: true, force: true }); + }); + + it('rewrites {tool:Read} in subagent body to the Rovo Dev mapping at install time', async () => { + await writeFile( + path.join(contentDir, 'subagents', 'fixture-agent.md'), + [ + '---', + 'name: fixture-agent', + 'description: A test agent', + '---', + '', + '# Body', + '', + 'Use `{tool:Read}` to inspect files and `{tool:Grep}` to search.', + '', + ].join('\n'), + ); + + await installCommand(installOptions, tempDir, contentDir); + + const installed = await readFile(path.join(tempDir, '.rovodev', 'subagents', 'fixture-agent.md'), 'utf8'); + expect(installed).toContain('Use `open_files` to inspect files and `grep` to search.'); + expect(installed).not.toContain('{tool:Read}'); + expect(installed).not.toContain('{tool:Grep}'); + }); + + it('rewrites {tool:NAME} placeholders inside a directory-form skill before write', async () => { + const skillDir = path.join(contentDir, 'skills', 'fixture-dir-skill'); + await mkdir(skillDir, { recursive: true }); + await writeFile( + path.join(skillDir, 'SKILL.md'), + ['# Fixture dir skill', '', 'Use `{tool:Glob}` and `{tool:Read}`.', ''].join('\n'), + ); + + await installCommand(installOptions, tempDir, contentDir); + + const installed = await readFile(path.join(tempDir, '.rovodev', 'skills', 'fixture-dir-skill', 'SKILL.md'), 'utf8'); + expect(installed).toContain('Use `expand_folder` and `open_files`.'); + }); + + it('aborts install with a file-and-line-anchored error when a subagent uses an unmapped placeholder', async () => { + await writeFile( + path.join(contentDir, 'subagents', 'bad-agent.md'), + [ + '---', + 'name: bad-agent', + 'description: An agent with a typo', + '---', + '', + '# Body', + '', + 'Use `{tool:NonExistent}` for nothing.', + '', + ].join('\n'), + ); + + let caught: unknown; + try { + await installCommand(installOptions, tempDir, contentDir); + } catch (error) { + caught = error; + } + + expect(caught).toBeInstanceOf(ToolNameRewriteError); + if (!(caught instanceof ToolNameRewriteError)) { + throw caught; + } + expect(caught.toolName).toBe('NonExistent'); + expect(caught.contextLabel).toBe('subagents/bad-agent.md'); + expect(caught.line).toBeGreaterThan(1); + expect(caught.message).toContain('subagents/bad-agent.md:'); + expect(caught.message).toContain('NonExistent'); + }); + + it('aborts install with a file-and-line-anchored error when a skill uses an unmapped placeholder', async () => { + const skillDir = path.join(contentDir, 'skills', 'bad-skill'); + await mkdir(skillDir, { recursive: true }); + await writeFile( + path.join(skillDir, 'SKILL.md'), + ['# Bad skill', '', 'Use `{tool:Phantom}` for nothing.', ''].join('\n'), + ); + + let caught: unknown; + try { + await installCommand(installOptions, tempDir, contentDir); + } catch (error) { + caught = error; + } + + expect(caught).toBeInstanceOf(ToolNameRewriteError); + if (!(caught instanceof ToolNameRewriteError)) { + throw caught; + } + expect(caught.toolName).toBe('Phantom'); + expect(caught.contextLabel).toContain('skills/bad-skill/SKILL.md'); + expect(caught.line).toBeGreaterThan(1); + }); +}); diff --git a/packages/agents/src/commands/install.ts b/packages/agents/src/commands/install.ts index 355f070b..14a7f29d 100644 --- a/packages/agents/src/commands/install.ts +++ b/packages/agents/src/commands/install.ts @@ -21,6 +21,7 @@ import { } from '../lib/marker-injector.js'; 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 type { AgentsManifest, InstallOptions, @@ -79,8 +80,13 @@ export async function installCommand( const entries: Array = []; - // Install skills (shared + platform-specific) + // Load the platform overlay once per platform. The raw YAML feeds the frontmatter merger (subagents only); + // the parsed `_tools:` mapping feeds the body-text placeholder rewriter (subagents and skills). const platformConfig = PLATFORMS[platformId]; + const overlayYaml = await readOverlay(contentDir, platformConfig.frontmatterFile); + const toolMapping = loadToolMapping(overlayYaml); + + // Install skills (shared + platform-specific) const skillsPrefix = `${platformConfig.homeDir}/${platformConfig.skillsDir}`; const skillEntries = await installSkills( contentDir, @@ -91,11 +97,20 @@ export async function installCommand( platformId, skillsPrefix, platformConfig.homeDir, + toolMapping, ); entries.push(...skillEntries); // Install subagents with merged frontmatter - const subagentEntries = await installSubagents(contentDir, paths, platformId, existingByPath, options); + const subagentEntries = await installSubagents( + contentDir, + paths, + platformId, + existingByPath, + options, + overlayYaml, + toolMapping, + ); entries.push(...subagentEntries); // Install scripts @@ -152,12 +167,12 @@ export async function installCommand( /** * Installs skill directories from content/skills/ into the target skills directory. - * Shared skills (top-level entries) are installed for all platforms. Platform-specific - * skills from `_platforms/{platformId}/` are installed only for the matching platform. + * Shared skills (top-level entries) are installed for all platforms. + * Platform-specific skills from `_platforms/{platformId}/` are installed only for the matching platform. * The `_platforms` directory is skipped (handled by dedicated platform-specific logic below). * - * If a previously installed item has been modified by the user, it is skipped unless - * `--force` is set, mirroring the uninstall command's drift-checking behavior. + * If a previously installed item has been modified by the user, it is skipped unless `--force` is set, + * mirroring the uninstall command's drift-checking behavior. */ async function installSkills( contentDir: string, @@ -168,6 +183,7 @@ async function installSkills( platformId: PlatformId, skillsPrefix: string, homeDir: string, + toolMapping: ReadonlyMap, ): Promise> { const skillsSrcDir = path.join(contentDir, 'skills'); const dirEntries = await readdir(skillsSrcDir); @@ -189,6 +205,7 @@ async function installSkills( skillsPrefix, homeDir, contentDir, + toolMapping, ); entries.push(result); } @@ -223,6 +240,7 @@ async function installSkills( skillsPrefix, homeDir, contentDir, + toolMapping, '(platform-specific)', ); entries.push(result); @@ -233,9 +251,8 @@ async function installSkills( /** * Installs a single skill entry (directory or file) from source to destination. - * Skills are always copied and rewritten (never symlinked), because they require - * path transformation at install time — the same pattern subagents use for - * frontmatter merging. + * Skills are always copied and rewritten (never symlinked), because they require path transformation at install time + * — the same pattern subagents use for frontmatter merging. */ async function installSkillEntry( srcPath: string, @@ -248,20 +265,24 @@ async function installSkillEntry( skillsPrefix: string, homeDir: string, contentDir: string, + toolMapping: ReadonlyMap, label = '', ): Promise { - // Eagerly resolve include directives at source-tree level. Run before the dry-run gate - // so missing targets, cycles, and out-of-tree references surface even when no files - // are written. Directory entries traverse their tree and cache each expanded `.md` - // file's content; file entries expand the file directly. The cached content is reused - // during the write phase below so each `.md` file is expanded exactly once per install. + // Eagerly resolves include directives at source-tree level. Run before the dry-run gate so missing targets, cycles, + // and out-of-tree references surface even when no files are written. + // Directory entries traverse their tree and cache each expanded `.md` file's content; + // file entries expand the file directly. + // After expansion, applies the tool-name rewriter in-memory so the cached content carries the final body text the + // write phase will emit — no second disk pass, no read-back-from-disk. const srcStats = await stat(srcPath); let expandedFileContent: string | undefined; let expandedDirContents: ReadonlyMap | undefined; if (srcStats.isDirectory()) { - expandedDirContents = await preExpandSkillDirectory(srcPath, contentDir); + const rawExpanded = await preExpandSkillDirectory(srcPath, contentDir); + expandedDirContents = rewriteToolNamesInExpansionMap(rawExpanded, contentDir, toolMapping); } else if (srcPath.endsWith('.md')) { - expandedFileContent = await expandIncludes(srcPath, contentDir); + const expanded = await expandIncludes(srcPath, contentDir); + expandedFileContent = rewriteToolNames(expanded, toolMapping, relativeFromContent(contentDir, srcPath)); } if (options.dryRun) { @@ -280,11 +301,10 @@ async function installSkillEntry( } if (srcStats.isDirectory()) { - // Per-file walk: write expanded `.md` files from the cache populated during the - // pre-expand pass, mirror the directory structure to the destination, and copy - // non-`.md` files plainly. The `_partials/` exclusion is applied during the walk. - // The cache is non-undefined here because srcStats.isDirectory() implies the - // directory branch above ran. + // Per-file walk: Writes expanded `.md` files from the cache populated during the pre-expand pass, + // mirrors the directory structure to the destination, and copies non-`.md` files plainly. + // The `_partials/` exclusion is applied during the walk. + // The cache is non-undefined here because srcStats.isDirectory() implies the directory branch above ran. if (expandedDirContents === undefined) { throw new Error(`Invariant violation: expandedDirContents undefined for directory ${srcPath}`); } @@ -312,11 +332,10 @@ async function installSkillEntry( } /** - * Eagerly walks a skill source directory, runs `expandIncludes` on each `.md` file to - * surface include errors before any file is written, and returns a map keyed by absolute - * source path with the expanded content. `_partials/` directories are skipped because - * their contents are referenced through includes, not installed. The returned map is - * consumed by `writeExpandedSkillDir` so each `.md` file is expanded once per install. + * Eagerly walks a skill source directory, runs `expandIncludes` on each `.md` file to surface include errors before + * any file is written, and returns a map keyed by absolute source path with the expanded content. + * `_partials/` directories are skipped because their contents are referenced through includes, not installed. + * The returned map is consumed by `writeExpandedSkillDir` so each `.md` file is expanded once per install. */ async function preExpandSkillDirectory(srcDir: string, contentDir: string): Promise> { const expandedBySrcPath = new Map(); @@ -345,10 +364,9 @@ async function collectExpansions( } /** - * Recursively writes a skill source directory to the destination. `.md` files are - * read from the pre-computed expansion cache (populated by `preExpandSkillDirectory`); - * non-`.md` files are copied verbatim. `_partials/` subdirectories are skipped at any - * depth — their contents are include targets, not installed artifacts. + * Recursively writes a skill source directory to the destination. `.md` files are read from the pre-computed expansion + * cache (populated by `preExpandSkillDirectory`); non-`.md` files are copied verbatim. + * `_partials/` subdirectories are skipped at any depth — their contents are include targets, not installed artifacts. */ async function writeExpandedSkillDir( srcDir: string, @@ -380,8 +398,8 @@ async function writeExpandedSkillDir( /** * Installs subagent .md files with platform-specific frontmatter merging. - * If a previously installed item has been modified by the user, it is skipped unless - * `--force` is set, mirroring the uninstall command's drift-checking behavior. + * If a previously installed item has been modified by the user, it is skipped unless `--force` is set, + * mirroring the uninstall command's drift-checking behavior. */ async function installSubagents( contentDir: string, @@ -389,20 +407,11 @@ async function installSubagents( platformId: PlatformId, existingByPath: ReadonlyMap, options: InstallOptions, + overlayYaml: string, + toolMapping: ReadonlyMap, ): Promise> { const subagentsSrcDir = path.join(contentDir, 'subagents'); const platformConfig = PLATFORMS[platformId]; - const overlayPath = path.join(subagentsSrcDir, '_data', platformConfig.frontmatterFile); - - let overlayYaml: string; - try { - overlayYaml = await readFile(overlayPath, 'utf8'); - } catch (error: unknown) { - if (!isEnoent(error)) { - throw error; - } - overlayYaml = ''; - } const dirEntries = await readdir(subagentsSrcDir); const subagentsDirName = platformConfig.subagentsDir; @@ -417,9 +426,8 @@ async function installSubagents( const destPath = path.join(platformPaths.subagentsDir, entry); const relativePath = `${subagentsDirName}/${entry}`; - // Resolve include directives at source-tree level. Run before the dry-run gate so - // missing targets, cycles, and out-of-tree references surface even when no files - // are written. Mirrors the ordering in installPlatformGuidance. + // Resolve include directives at source-tree level. Run before the dry-run gate so missing targets, cycles, and + // out-of-tree references surface even when no files are written. Mirrors the ordering in installPlatformGuidance. const expandedSource = await expandIncludes(srcPath, contentDir); if (options.dryRun) { @@ -444,9 +452,12 @@ async function installSubagents( } } - // Pipeline: expand includes -> merge frontmatter -> inject provenance marker -> write. + // Pipeline: Expand includes -> merge frontmatter -> rewrite tool-name placeholders -> + // inject provenance marker -> write. + const sourceLabel = `subagents/${entry}`; const merged = mergeFrontmatter(expandedSource, overlayYaml); - const withMarker = injectProvenanceMarker(merged, buildSourceUrl(`subagents/${entry}`)); + const rewritten = rewriteToolNames(merged, toolMapping, sourceLabel); + const withMarker = injectProvenanceMarker(rewritten, buildSourceUrl(sourceLabel)); await mkdir(path.dirname(destPath), { recursive: true }); await unlinkIfSymlink(destPath); await writeFile(destPath, withMarker, 'utf8'); @@ -463,8 +474,8 @@ async function installSubagents( } /** - * Generates `prompts.yml` for Rovo Dev, which is the skill discovery file that lists - * all user-invocable skills. Skills with `user-invocable: false` are excluded. + * Generates `prompts.yml` for Rovo Dev, which is the skill discovery file that lists all user-invocable skills. + * Skills with `user-invocable: false` are excluded. * * The file is written to `{platformHome}/prompts.yml` and tracked in the manifest. */ @@ -545,9 +556,8 @@ async function generatePromptsYml( }); } - // Build YAML content with deterministic template literals. - // Description values are single-quoted with internal single quotes escaped (doubled) - // to prevent YAML-special characters from producing invalid output. + // Build YAML content with deterministic template literals. Description values are single-quoted with internal single + // quotes escaped (doubled) to prevent YAML-special characters from producing invalid output. const yamlLines = ['prompts:']; for (const entry of promptEntries) { const escapedDescription = entry.description.replaceAll("'", "''"); @@ -588,8 +598,7 @@ async function generatePromptsYml( /** * Installs script files from content/scripts/ into the target scripts directory. * Scripts are flat files (no frontmatter, no platform-specific variants). - * Copied scripts receive the executable bit (0o755); symlinked scripts inherit - * the source's permissions. + * Copied scripts receive the executable bit (0o755); symlinked scripts inherit the source's permissions. */ async function installScripts( contentDir: string, @@ -758,10 +767,9 @@ async function installSharedGuidance( } /** - * Installs platform-specific guidance files from `content/guidance/_platforms/{platformId}/` - * into the platform home directory. Platform guidance is always copied and rewritten (never - * symlinked), because install-time path rewriting produces absolute link targets that agents - * can resolve without knowing a path convention. + * Installs platform-specific guidance files from `content/guidance/_platforms/{platformId}/` into the platform + * home directory. Platform guidance is always copied and rewritten (never symlinked), because install-time path + * rewriting produces absolute link targets that agents can resolve without knowing a path convention. */ async function installPlatformGuidance( contentDir: string, @@ -822,8 +830,8 @@ async function installPlatformGuidance( await unlinkIfSymlink(destPath); await copyItem(srcPath, destPath); - // For .md files, replace the freshly-copied content with the include-expanded content, - // then run downstream link rewriting and template/marker injection on the expanded text. + // For .md files, replace the freshly-copied content with the include-expanded content, then run downstream link + // rewriting and template/marker injection on the expanded text. if (entry.endsWith('.md')) { if (expandedContent !== undefined) { await writeFile(destPath, expandedContent, 'utf8'); @@ -848,3 +856,42 @@ async function installPlatformGuidance( function isEnoent(error: unknown): boolean { return typeof error === 'object' && error !== null && 'code' in error && error.code === 'ENOENT'; } + +/** + * 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. + */ +function relativeFromContent(contentDir: string, srcPath: string): string { + return path.relative(contentDir, srcPath).split(path.sep).join('/'); +} + +/** + * Returns a new map mirroring `rawExpanded`, with every value processed through the tool-name rewriter. + * The map is preserved by-reference (same keys, same iteration order); only the string values change. Each entry's + * `contextLabel` is its content-relative POSIX path, so an unmapped placeholder surfaces a usable file reference. + */ +function rewriteToolNamesInExpansionMap( + rawExpanded: ReadonlyMap, + contentDir: string, + toolMapping: ReadonlyMap, +): Map { + const rewritten = new Map(); + for (const [absSrcPath, content] of rawExpanded) { + const label = relativeFromContent(contentDir, absSrcPath); + rewritten.set(absSrcPath, rewriteToolNames(content, toolMapping, label)); + } + return rewritten; +} + +/** Reads a subagent overlay YAML file. Returns an empty string when the file does not exist. */ +async function readOverlay(contentDir: string, frontmatterFile: string): Promise { + const overlayPath = path.join(contentDir, 'subagents', '_data', frontmatterFile); + try { + return await readFile(overlayPath, 'utf8'); + } catch (error: unknown) { + if (!isEnoent(error)) { + throw error; + } + return ''; + } +} diff --git a/packages/agents/src/lib/__tests__/tool-name-rewriter.test.ts b/packages/agents/src/lib/__tests__/tool-name-rewriter.test.ts new file mode 100644 index 00000000..d2ff007e --- /dev/null +++ b/packages/agents/src/lib/__tests__/tool-name-rewriter.test.ts @@ -0,0 +1,177 @@ +import { describe, expect, it } from 'vitest'; + +import { loadToolMapping, rewriteToolNames, ToolNameRewriteError } from '../tool-name-rewriter.js'; + +const IDENTITY = new Map([ + ['Bash', 'Bash'], + ['Edit', 'Edit'], + ['Glob', 'Glob'], + ['Grep', 'Grep'], + ['Read', 'Read'], + ['Write', 'Write'], +]); + +const ROVODEV = new Map([ + ['Bash', 'bash'], + ['Edit', 'find_and_replace_code'], + ['Glob', 'expand_folder'], + ['Grep', 'grep'], + ['Read', 'open_files'], + ['Write', 'create_file'], +]); + +describe('rewriteToolNames', () => { + it('returns content unchanged when no placeholders are present', () => { + const content = 'Plain text with no placeholders.'; + expect(rewriteToolNames(content, ROVODEV, 'test.md')).toBe(content); + }); + + it('leaves canonical names intact when given an identity mapping', () => { + const content = 'Use {tool:Glob}, {tool:Grep}, and {tool:Read} to explore.'; + expect(rewriteToolNames(content, IDENTITY, 'test.md')).toBe('Use Glob, Grep, and Read to explore.'); + }); + + it('replaces placeholders with platform-native names for a non-identity mapping', () => { + const content = 'Use {tool:Glob}, {tool:Grep}, and {tool:Read} to explore.'; + expect(rewriteToolNames(content, ROVODEV, 'test.md')).toBe('Use expand_folder, grep, and open_files to explore.'); + }); + + it('replaces multiple placeholders on a single line', () => { + const content = 'Not a `{tool:Read}`, not a `{tool:Grep}`, not a `{tool:Bash}` — a `{tool:Write}`.'; + expect(rewriteToolNames(content, ROVODEV, 'test.md')).toBe( + 'Not a `open_files`, not a `grep`, not a `bash` — a `create_file`.', + ); + }); + + it('preserves inline-code backticks around placeholders', () => { + const content = 'You have `{tool:Write}` but not `{tool:Edit}`.'; + expect(rewriteToolNames(content, IDENTITY, 'test.md')).toBe('You have `Write` but not `Edit`.'); + expect(rewriteToolNames(content, ROVODEV, 'test.md')).toBe( + 'You have `create_file` but not `find_and_replace_code`.', + ); + }); + + it('throws ToolNameRewriteError for an unmapped name', () => { + const content = 'Use {tool:NonExistent} for nothing.'; + expect(() => rewriteToolNames(content, ROVODEV, 'test.md')).toThrow(ToolNameRewriteError); + }); + + it('carries toolName, contextLabel, and line on the error', () => { + const content = ['Line one is fine.', 'Line two has {tool:NonExistent} on it.'].join('\n'); + try { + rewriteToolNames(content, ROVODEV, 'fixtures/sample.md'); + expect.fail('Expected ToolNameRewriteError to be thrown'); + } catch (error) { + expect(error).toBeInstanceOf(ToolNameRewriteError); + if (!(error instanceof ToolNameRewriteError)) { + throw error; + } + expect(error.toolName).toBe('NonExistent'); + expect(error.contextLabel).toBe('fixtures/sample.md'); + expect(error.line).toBe(2); + expect(error.message).toContain('fixtures/sample.md:2'); + expect(error.message).toContain('NonExistent'); + } + }); + + it('reports a line of 1 for placeholders on the first line', () => { + const content = '{tool:Unknown} at the very start.'; + try { + rewriteToolNames(content, ROVODEV, 'x.md'); + expect.fail('Expected ToolNameRewriteError'); + } catch (error) { + if (!(error instanceof ToolNameRewriteError)) { + throw error; + } + expect(error.line).toBe(1); + } + }); + + it('throws on the first unmapped placeholder when content has multiple', () => { + const content = '{tool:First} then {tool:Second}'; + try { + rewriteToolNames(content, new Map([['Second', 'second']]), 'x.md'); + expect.fail('Expected ToolNameRewriteError'); + } catch (error) { + if (!(error instanceof ToolNameRewriteError)) { + throw error; + } + expect(error.toolName).toBe('First'); + } + }); + + it('does not match malformed placeholders with internal whitespace', () => { + const content = 'Not a match: {tool: Read} and {tool : Read}.'; + expect(rewriteToolNames(content, ROVODEV, 'test.md')).toBe(content); + }); + + it('does not match empty placeholder content', () => { + const content = 'Not a match: {tool:} and {tool:_leading_underscore}.'; + expect(rewriteToolNames(content, ROVODEV, 'test.md')).toBe(content); + }); + + it('does not match placeholders missing the closing brace', () => { + const content = 'Not a match: {tool:Read without close.'; + expect(rewriteToolNames(content, ROVODEV, 'test.md')).toBe(content); + }); + + it('throws on empty mapping when any placeholder is present', () => { + expect(() => rewriteToolNames('Has {tool:Read}.', new Map(), 'x.md')).toThrow(ToolNameRewriteError); + }); + + it('leaves content unchanged with empty mapping when no placeholders are present', () => { + const content = 'No placeholders here.'; + expect(rewriteToolNames(content, new Map(), 'x.md')).toBe(content); + }); +}); + +describe('loadToolMapping', () => { + it('returns an empty map for an empty YAML string', () => { + expect(loadToolMapping('')).toEqual(new Map()); + expect(loadToolMapping(' \n \n')).toEqual(new Map()); + }); + + it('returns an empty map when _tools key is absent', () => { + const overlay = ['_defaults:', ' permissionMode: bypassPermissions'].join('\n'); + expect(loadToolMapping(overlay)).toEqual(new Map()); + }); + + it('returns an empty map when _tools is null or empty', () => { + expect(loadToolMapping('_tools:')).toEqual(new Map()); + expect(loadToolMapping('_tools: {}')).toEqual(new Map()); + }); + + it('parses a populated _tools mapping', () => { + const overlay = ['_tools:', ' Bash: bash', ' Read: open_files', ' Write: create_file'].join('\n'); + const result = loadToolMapping(overlay); + expect(result.size).toBe(3); + expect(result.get('Bash')).toBe('bash'); + expect(result.get('Read')).toBe('open_files'); + expect(result.get('Write')).toBe('create_file'); + }); + + it('ignores other top-level keys', () => { + const overlay = [ + '_defaults:', + ' permissionMode: bypassPermissions', + '', + '_tools:', + ' Read: open_files', + '', + 'plan-reviewer:', + ' model: sonnet', + ].join('\n'); + const result = loadToolMapping(overlay); + expect(result.get('Read')).toBe('open_files'); + expect(result.size).toBe(1); + }); + + it('throws when _tools is not an object', () => { + expect(() => loadToolMapping('_tools: notAnObject')).toThrow(/_tools/); + expect(() => loadToolMapping('_tools: [a, b, c]')).toThrow(/_tools/); + }); + + it('throws when a _tools entry value is not a string', () => { + expect(() => loadToolMapping(['_tools:', ' Read: 42'].join('\n'))).toThrow(/Read/); + }); +}); diff --git a/packages/agents/src/lib/tool-name-rewriter.ts b/packages/agents/src/lib/tool-name-rewriter.ts new file mode 100644 index 00000000..8fdfceb9 --- /dev/null +++ b/packages/agents/src/lib/tool-name-rewriter.ts @@ -0,0 +1,84 @@ +import yaml from 'js-yaml'; + +/** + * Thrown when `rewriteToolNames` encounters a `{tool:NAME}` placeholder whose canonical name has no entry in the + * supplied mapping. Caught by the install pipeline and surfaced as a fatal install error. + */ +export class ToolNameRewriteError extends Error { + override readonly name = 'ToolNameRewriteError'; + readonly toolName: string; + readonly contextLabel: string; + readonly line: number; + + constructor(toolName: string, contextLabel: string, line: number) { + super( + `Unmapped tool name "${toolName}" in ${contextLabel}:${line}. ` + + `Define ${toolName} in the platform overlay's _tools: mapping.`, + ); + this.toolName = toolName; + this.contextLabel = contextLabel; + this.line = line; + } +} + +/** Matches `{tool:NAME}` placeholders. NAME starts with a letter, then letters/digits/underscores. */ +const PLACEHOLDER_RE = /\{tool:([A-Za-z][A-Za-z0-9_]*)\}/g; + +/** + * Replaces every `{tool:NAME}` placeholder in `content` with the value bound to `NAME` in `mapping`. An unmapped name + * throws `ToolNameRewriteError` with the canonical name, `contextLabel`, and the 1-based line number of the offending + * match. There is no identity pass-through; every match must resolve through the mapping or the call fails. + */ +export function rewriteToolNames(content: string, mapping: ReadonlyMap, contextLabel: string): string { + return content.replace(PLACEHOLDER_RE, (_match: string, toolName: string, offset: number): string => { + const replacement = mapping.get(toolName); + if (replacement === undefined) { + throw new ToolNameRewriteError(toolName, contextLabel, computeLine(content, offset)); + } + return replacement; + }); +} + +/** + * Parses the top-level `_tools:` key out of an overlay YAML document into a canonical → platform name `Map`. + * Missing, null, or empty `_tools:` returns an empty `Map`. A non-object `_tools:` or any non-string entry throws. + */ +export function loadToolMapping(overlayYaml: string): Map { + if (overlayYaml.trim() === '') { + return new Map(); + } + const parsed: unknown = yaml.load(overlayYaml); + if (!isRecord(parsed)) { + return new Map(); + } + const tools = parsed._tools; + if (tools === undefined || tools === null) { + return new Map(); + } + if (!isRecord(tools)) { + throw new TypeError(`Invalid _tools: expected mapping object, got ${typeof tools}`); + } + + const mapping = new Map(); + for (const [key, value] of Object.entries(tools)) { + if (typeof value !== 'string') { + throw new TypeError(`Invalid _tools entry for "${key}": expected string, got ${typeof value}`); + } + mapping.set(key, value); + } + return mapping; +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function computeLine(content: string, offset: number): number { + let line = 1; + for (let i = 0; i < offset; i++) { + if (content.codePointAt(i) === 10) { + line++; + } + } + return line; +}