From 2c7619a083b11ee39396f301da1de050e6814867 Mon Sep 17 00:00:00 2001 From: jheyworth <8269695+jheyworth@users.noreply.github.com> Date: Thu, 30 Apr 2026 09:00:55 +0100 Subject: [PATCH] fix(installer): remove bmad-help from Copilot Custom Agents picker Per @BMadCode's feedback after #2324 merged: every persona agent's activation message already advertises bmad-help, so its picker entry is redundant AND confusing (looks like a peer agent when it's actually the meta-help). Removes the ALWAYS_AGENT_IDS allowlist exception that put it there. The toml-driven filter (the mechanism BMadCode endorsed in his PR review) remains the sole signal: a skill is a persona iff its source customize.toml has an [agent] section. bmad-help has no customize.toml, so under the cleaned-up filter it's correctly excluded. Tests: replaces the inclusion assertion in Suite 17 with an exclusion assertion. Suite still covers persona / non-conventional persona / workflow / meta-skill-with-`-agent-`-in-name cases. Refs #2324 Co-Authored-By: Claude Opus 4.7 (1M context) --- test/test-installation-components.js | 18 +++++++++--------- tools/installer/ide/_config-driven.js | 13 +------------ 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/test/test-installation-components.js b/test/test-installation-components.js index 4522f0f37..ff5e48178 100644 --- a/test/test-installation-components.js +++ b/test/test-installation-components.js @@ -585,9 +585,9 @@ async function runTests() { // persona — has customize.toml with [workflow] → EXCLUDED // (mirrors `bmad-agent-builder` in the real manifest) // 4. Workflow skill — no customize.toml at all → EXCLUDED - // 5. `bmad-help` — structural exception via ALWAYS_AGENT_IDS; - // has no customize.toml of its own but surfaces in the - // agents picker because it's the meta-help skill → INCLUDED + // 5. `bmad-help` — meta-help skill with no customize.toml; + // every persona agent's activation already advertises it, + // so it's correctly excluded from the picker as redundant → EXCLUDED const fixtureCsvPath17 = path.join(installedBmadDir17, '_config', 'skill-manifest.csv'); await fs.writeFile( fixtureCsvPath17, @@ -597,7 +597,7 @@ async function runTests() { '"bmad-agent-fixture","bmad-agent-fixture","Persona agent — customize.toml has [agent], SHOULD appear","core","_bmad/core/bmad-agent-fixture/SKILL.md"', '"bmad-tea","bmad-tea","Non-conventional id but [agent] in customize.toml — SHOULD appear","core","_bmad/core/bmad-tea/SKILL.md"', '"bmad-agent-builder","bmad-agent-builder","Skill-builder workflow — id contains -agent- but customize.toml has [workflow] — should NOT appear","core","_bmad/core/bmad-agent-builder/SKILL.md"', - '"bmad-help","bmad-help","Meta-help skill — no customize.toml but ALWAYS_AGENT_IDS exception; SHOULD appear in agents picker","core","_bmad/core/bmad-help/SKILL.md"', + '"bmad-help","bmad-help","Meta-help skill — no customize.toml; SHOULD NOT appear in agents picker (toml-driven filter)","core","_bmad/core/bmad-help/SKILL.md"', '', ].join('\n'), ); @@ -615,9 +615,9 @@ async function runTests() { ['---', `name: ${id}`, `description: fixture for ${id}`, '---', '', `Body of ${id}.`].join('\n'), ); } - // Note: bmad-help intentionally has NO customize.toml — it's the - // structural exception for which the ALWAYS_AGENT_IDS allowlist - // exists. + // Note: bmad-help intentionally has NO customize.toml — it exercises + // the toml-driven filter's exclusion path (a skill with no [agent] + // section is correctly kept out of the Copilot agents picker). // [agent] customize.toml for the two persona fixtures. await fs.writeFile( path.join(installedBmadDir17, 'core', 'bmad-agent-fixture', 'customize.toml'), @@ -688,8 +688,8 @@ async function runTests() { assert(await fs.pathExists(agentFileForTea17), 'Non-conventional id with [agent] in customize.toml is included (no allowlist needed)'); assert(!(await fs.pathExists(agentFileForWorkflow17)), 'Workflow skill (no customize.toml) is FILTERED OUT of .github/agents/'); assert( - await fs.pathExists(agentFileForBmadHelp17), - 'bmad-help is INCLUDED in agents picker via ALWAYS_AGENT_IDS exception (structural meta-skill, no customize.toml)', + !(await fs.pathExists(agentFileForBmadHelp17)), + 'bmad-help is excluded from Copilot agents picker (no customize.toml; allowlist removed per maintainer feedback)', ); assert( !(await fs.pathExists(agentFileForMetaSkill17)), diff --git a/tools/installer/ide/_config-driven.js b/tools/installer/ide/_config-driven.js index 77be9b6c5..bf6fffbc5 100644 --- a/tools/installer/ide/_config-driven.js +++ b/tools/installer/ide/_config-driven.js @@ -57,15 +57,6 @@ function isSafeCanonicalId(value) { // OpenCode's native `@skills/` skill-reference syntax. const DEFAULT_COMMANDS_BODY_TEMPLATE = '@skills/{canonicalId}'; -// `bmad-help` is the structural meta-skill across BMAD: the orientation -// helper that points users at every other skill. It is invoked -// persona-style ("ask the helper") even though it has no [agent] -// customize.toml of its own (it isn't a configurable persona). Surfacing -// it in agents-picker contexts mirrors how users actually reach for it, -// and the inclusion is unique and stable — there is no second meta-help -// skill to encourage growth of this exception. -const ALWAYS_AGENT_IDS = new Set(['bmad-help']); - // Is this skill a persona agent (vs. a workflow/tool/standalone skill)? // Used by platforms that surface only persona agents (e.g. Copilot's Custom // Agents picker). Signal: the skill's source `customize.toml` has an @@ -77,15 +68,13 @@ const ALWAYS_AGENT_IDS = new Set(['bmad-help']); // GDS, WDS, TEA, and correctly excludes meta-skills like // `bmad-agent-builder` (a skill-builder workflow whose canonical id // contains `-agent-` but which has no [agent] section because it isn't a -// persona itself). Plus the explicit `ALWAYS_AGENT_IDS` set for the one -// structural exception (`bmad-help`). +// persona itself). // // Reading the source toml — at install time the source skill directory // (resolved from manifest record.path) still exists; cleanup runs later // in the install flow. async function isAgentSkill(record, bmadDir) { if (!record?.path || !bmadDir) return false; - if (record.canonicalId && ALWAYS_AGENT_IDS.has(record.canonicalId)) return true; const bmadFolderName = path.basename(bmadDir); const bmadPrefix = bmadFolderName + '/'; const relativePath = record.path.startsWith(bmadPrefix) ? record.path.slice(bmadPrefix.length) : record.path;