From 56a3f267f008b60bab85aa248b933c8fda4bf8bf Mon Sep 17 00:00:00 2001 From: jheyworth <8269695+jheyworth@users.noreply.github.com> Date: Tue, 28 Apr 2026 12:03:26 +0100 Subject: [PATCH] fix(installer): extend command-pointer generation to Copilot Custom Agents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-scopes #2324 to cover the second user-facing pain: GitHub Copilot's Custom Agents picker, where installed BMAD skills currently don't show up even though slash commands work natively. Generalizes the per-platform pointer-file mechanism so the same installCommandPointers / cleanupCommandPointers code path serves both OpenCode (slash commands palette) and Copilot (Custom Agents picker), with all platform-specific shape pushed into platform-codes.yaml as data: - commands_target_dir — where pointer files live (existing) - commands_extension — file extension (default '.md'; Copilot uses '.agent.md' per VS Code Custom Agents docs) - commands_body_template — pointer body, supports {canonicalId} and {target_dir} placeholders. Default matches OpenCode's `@skills/` resolver. Copilot has no such resolver, so its template uses the {project-root}///SKILL.md LOAD pattern (consistent with PR #1769). OpenCode behavior is unchanged. Copilot users now get a per-skill .github/agents/.agent.md file that surfaces the skill in the Custom Agents picker — addressing the "agents being gone" complaint flagged by enterprise users. Tests: extends Suite 17 with assertions for Copilot agent pointer creation, body content (LOAD pattern with {project-root}-rooted path), and idempotency. 318 tests pass (was 310). Refs #2267 Co-Authored-By: Claude Opus 4.7 (1M context) --- test/test-installation-components.js | 35 +++++++++++ tools/installer/ide/_config-driven.js | 78 +++++++++++++++++++------ tools/installer/ide/platform-codes.yaml | 3 + 3 files changed, 97 insertions(+), 19 deletions(-) diff --git a/test/test-installation-components.js b/test/test-installation-components.js index a687def21..0e5f12ad4 100644 --- a/test/test-installation-components.js +++ b/test/test-installation-components.js @@ -557,6 +557,15 @@ async function runTests() { const copilotInstaller = platformCodes17.platforms['github-copilot']?.installer; assert(copilotInstaller?.target_dir === '.agents/skills', 'GitHub Copilot target_dir uses native skills path'); + assert( + copilotInstaller?.commands_target_dir === '.github/agents', + 'GitHub Copilot commands_target_dir is configured for the Custom Agents picker', + ); + assert(copilotInstaller?.commands_extension === '.agent.md', 'GitHub Copilot uses .agent.md extension for Custom Agents files'); + assert( + typeof copilotInstaller?.commands_body_template === 'string' && copilotInstaller.commands_body_template.includes('{canonicalId}'), + 'GitHub Copilot defines a commands_body_template with {canonicalId} placeholder', + ); const tempProjectDir17 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-copilot-test-')); const installedBmadDir17 = await createTestBmadFixture(); @@ -596,6 +605,32 @@ async function runTests() { 'GitHub Copilot setup preserves user content in copilot-instructions.md', ); + // Custom Agents picker integration: a per-skill .agent.md file should be + // generated under .github/agents/ so the skill appears in Copilot's + // Custom Agents picker. Body uses the LOAD-{project-root}/... pattern + // (Copilot's body has no @skills/ resolver, so the agent file + // instructs the model to load the SKILL.md directly). + const agentFile17 = path.join(tempProjectDir17, '.github', 'agents', 'bmad-master.agent.md'); + assert(await fs.pathExists(agentFile17), 'GitHub Copilot install writes per-skill .agent.md pointer file'); + const agentContent17 = await fs.readFile(agentFile17, 'utf8'); + assert( + agentContent17.includes('description:'), + 'Copilot agent pointer carries a description in YAML frontmatter (drives the agents picker label)', + ); + assert( + agentContent17.includes('{project-root}/.agents/skills/bmad-master/SKILL.md'), + 'Copilot agent pointer body resolves to the skill via LOAD {project-root}///SKILL.md', + ); + + // Idempotency: re-running setup must not duplicate or rewrite the agent + // pointer when the source manifest is unchanged. + const result17b = await ideManager17.setup('github-copilot', tempProjectDir17, installedBmadDir17, { + silent: true, + selectedModules: ['bmm'], + }); + assert(result17b.success === true, 'Second GitHub Copilot install succeeds (idempotent)'); + assert(await fs.pathExists(agentFile17), 'Copilot agent pointer survives a second install pass'); + await fs.remove(tempProjectDir17); await fs.remove(path.dirname(installedBmadDir17)); } catch (error) { diff --git a/tools/installer/ide/_config-driven.js b/tools/installer/ide/_config-driven.js index ad3fafd7a..1644aecb3 100644 --- a/tools/installer/ide/_config-driven.js +++ b/tools/installer/ide/_config-driven.js @@ -52,11 +52,26 @@ function isSafeCanonicalId(value) { return typeof value === 'string' && /^[a-zA-Z0-9][a-zA-Z0-9_.-]*$/.test(value) && !value.includes('..'); } +// Default body template for command pointer files. Used when a platform's +// installer config doesn't override `commands_body_template`. Matches +// OpenCode's native `@skills/` skill-reference syntax. +const DEFAULT_COMMANDS_BODY_TEMPLATE = '@skills/{canonicalId}'; + +// Resolve placeholders in a body template. Supported placeholders: +// {canonicalId} — the skill's canonical id +// {target_dir} — the platform's skill install directory (e.g. .agents/skills) +// {project-root} — left as a literal placeholder for the model/tool to expand +// at runtime; consistent with PR #1769's templates. +function expandBodyTemplate(template, { canonicalId, targetDir }) { + return template.replaceAll('{canonicalId}', canonicalId).replaceAll('{target_dir}', targetDir); +} + // The exact body the installer would generate for a given description and -// canonicalId. Centralised so both the write and the freshness-check paths -// agree on the canonical form. -function buildCommandPointerBody(description, canonicalId) { - return `---\ndescription: ${yamlSafeSingleLine(description)}\n---\n\n@skills/${canonicalId}\n`; +// canonicalId, given the platform's body template. Centralised so both the +// write and the freshness-check paths agree on the canonical form. +function buildCommandPointerBody(description, canonicalId, { template, targetDir }) { + const bodyText = expandBodyTemplate(template, { canonicalId, targetDir }); + return `---\ndescription: ${yamlSafeSingleLine(description)}\n---\n\n${bodyText}\n`; } // Heuristic: does an existing pointer file look like our generator's output @@ -64,11 +79,12 @@ function buildCommandPointerBody(description, canonicalId) { // preserve)? We check the body shape rather than full equality so that // description-only edits in the manifest can propagate without trampling // hand edits to the body. -function looksLikeGeneratorOutput(content, canonicalId) { +function looksLikeGeneratorOutput(content, canonicalId, { template, targetDir }) { if (typeof content !== 'string') return false; const trimmed = content.trim(); - // Must end with the exact reference line our generator writes. - if (!trimmed.endsWith(`@skills/${canonicalId}`)) return false; + const expectedTail = expandBodyTemplate(template, { canonicalId, targetDir }).trim(); + // Must end with the exact body our generator writes (post-expansion). + if (!trimmed.endsWith(expectedTail)) return false; // Must start with frontmatter containing exactly one description: line. const fmMatch = trimmed.match(/^---\n([\S\s]*?)\n---\n/); if (!fmMatch) return false; @@ -259,6 +275,11 @@ class ConfigDrivenIdeSetup { const commandsPath = path.join(projectDir, config.commands_target_dir); await fs.ensureDir(commandsPath); + // Per-platform pointer-file shape, all overrideable in platform-codes.yaml. + const extension = config.commands_extension || '.md'; + const template = config.commands_body_template || DEFAULT_COMMANDS_BODY_TEMPLATE; + const targetDir = config.target_dir; + const csvContent = await fs.readFile(csvPath, 'utf8'); const records = csv.parse(csvContent, { columns: true, skip_empty_lines: true }); @@ -288,8 +309,8 @@ class ConfigDrivenIdeSetup { result.fallbackDescription++; } - const body = buildCommandPointerBody(description, canonicalId); - const commandFile = path.join(commandsPath, `${canonicalId}.md`); + const body = buildCommandPointerBody(description, canonicalId, { template, targetDir }); + const commandFile = path.join(commandsPath, `${canonicalId}${extension}`); // If a pointer file already exists, decide whether to overwrite based // on whether it looks like generator output (description-only diff) or @@ -309,7 +330,7 @@ class ConfigDrivenIdeSetup { result.skippedExisting++; continue; } - if (looksLikeGeneratorOutput(existing, canonicalId)) { + if (looksLikeGeneratorOutput(existing, canonicalId, { template, targetDir })) { // Description (or other generated bit) has changed; refresh in place. try { await fs.writeFile(commandFile, body, 'utf8'); @@ -317,7 +338,7 @@ class ConfigDrivenIdeSetup { } catch (error) { result.writeFailures++; if (!options.silent) { - await prompts.log.warn(`Failed to update command pointer ${canonicalId}.md: ${error.message}`); + await prompts.log.warn(`Failed to update command pointer ${canonicalId}${extension}: ${error.message}`); } } continue; @@ -333,7 +354,7 @@ class ConfigDrivenIdeSetup { } catch (error) { result.writeFailures++; if (!options.silent) { - await prompts.log.warn(`Failed to write command pointer ${canonicalId}.md: ${error.message}`); + await prompts.log.warn(`Failed to write command pointer ${canonicalId}${extension}: ${error.message}`); } } } @@ -486,7 +507,15 @@ class ConfigDrivenIdeSetup { // so its pointers should go with it. const isInstallFlow = options.previousSkillIds && options.previousSkillIds.size > 0; const activeSkillIds = isInstallFlow ? await this._readActiveSkillIds(resolvedBmadDir) : new Set(); - await this.cleanupCommandPointers(projectDir, this.installerConfig.commands_target_dir, options, removalSet, activeSkillIds); + const extension = this.installerConfig.commands_extension || '.md'; + await this.cleanupCommandPointers( + projectDir, + this.installerConfig.commands_target_dir, + options, + removalSet, + activeSkillIds, + extension, + ); } // Skip target_dir cleanup when a peer platform owns this directory @@ -590,9 +619,9 @@ class ConfigDrivenIdeSetup { /** * Cleanup generated command pointer files for entries in removalSet. - * Symmetric counterpart to installCommandPointers — removes .md - * files whose canonicalId is in the set. Removes the commands directory - * entirely if it ends up empty. + * Symmetric counterpart to installCommandPointers — removes + * `` files whose canonicalId is in the set. Removes + * the commands directory entirely if it ends up empty. * @param {string} projectDir * @param {string} commandsTargetDir - Relative dir (e.g. .opencode/commands) * @param {Object} options @@ -603,8 +632,19 @@ class ConfigDrivenIdeSetup { * same skills are about to be re-installed) doesn't wipe hand-edited * pointer files. Pass an empty set or omit to delete every match in * removalSet (uninstall flow). + * @param {string} [extension] - Pointer file extension (default '.md'); + * matches the platform's commands_extension config value so cleanup + * correctly identifies pointer files for IDEs whose convention isn't .md + * (e.g. Copilot's `.agent.md`). */ - async cleanupCommandPointers(projectDir, commandsTargetDir, options = {}, removalSet = new Set(), activeSkillIds = new Set()) { + async cleanupCommandPointers( + projectDir, + commandsTargetDir, + options = {}, + removalSet = new Set(), + activeSkillIds = new Set(), + extension = '.md', + ) { if (!removalSet || removalSet.size === 0) return; const commandsPath = path.join(projectDir, commandsTargetDir); @@ -618,8 +658,8 @@ class ConfigDrivenIdeSetup { } for (const entry of entries) { - if (!entry.endsWith('.md')) continue; - const canonicalId = entry.slice(0, -3); + if (!entry.endsWith(extension)) continue; + const canonicalId = entry.slice(0, -extension.length); if (!removalSet.has(canonicalId)) continue; // Spare pointers for skills that are still in the manifest; the // install pass will refresh them in place if their content has gone diff --git a/tools/installer/ide/platform-codes.yaml b/tools/installer/ide/platform-codes.yaml index 78ca1d271..c80996107 100644 --- a/tools/installer/ide/platform-codes.yaml +++ b/tools/installer/ide/platform-codes.yaml @@ -132,6 +132,9 @@ platforms: installer: target_dir: .agents/skills global_target_dir: ~/.agents/skills + commands_target_dir: .github/agents + commands_extension: .agent.md + commands_body_template: "LOAD the FULL {project-root}/{target_dir}/{canonicalId}/SKILL.md, READ its entire contents and follow its directions exactly!" goose: name: "Block Goose"