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) <noreply@anthropic.com>
This commit is contained in:
parent
65b810a11f
commit
2c7619a083
|
|
@ -585,9 +585,9 @@ async function runTests() {
|
||||||
// persona — has customize.toml with [workflow] → EXCLUDED
|
// persona — has customize.toml with [workflow] → EXCLUDED
|
||||||
// (mirrors `bmad-agent-builder` in the real manifest)
|
// (mirrors `bmad-agent-builder` in the real manifest)
|
||||||
// 4. Workflow skill — no customize.toml at all → EXCLUDED
|
// 4. Workflow skill — no customize.toml at all → EXCLUDED
|
||||||
// 5. `bmad-help` — structural exception via ALWAYS_AGENT_IDS;
|
// 5. `bmad-help` — meta-help skill with no customize.toml;
|
||||||
// has no customize.toml of its own but surfaces in the
|
// every persona agent's activation already advertises it,
|
||||||
// agents picker because it's the meta-help skill → INCLUDED
|
// so it's correctly excluded from the picker as redundant → EXCLUDED
|
||||||
const fixtureCsvPath17 = path.join(installedBmadDir17, '_config', 'skill-manifest.csv');
|
const fixtureCsvPath17 = path.join(installedBmadDir17, '_config', 'skill-manifest.csv');
|
||||||
await fs.writeFile(
|
await fs.writeFile(
|
||||||
fixtureCsvPath17,
|
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-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-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-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'),
|
].join('\n'),
|
||||||
);
|
);
|
||||||
|
|
@ -615,9 +615,9 @@ async function runTests() {
|
||||||
['---', `name: ${id}`, `description: fixture for ${id}`, '---', '', `Body of ${id}.`].join('\n'),
|
['---', `name: ${id}`, `description: fixture for ${id}`, '---', '', `Body of ${id}.`].join('\n'),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
// Note: bmad-help intentionally has NO customize.toml — it's the
|
// Note: bmad-help intentionally has NO customize.toml — it exercises
|
||||||
// structural exception for which the ALWAYS_AGENT_IDS allowlist
|
// the toml-driven filter's exclusion path (a skill with no [agent]
|
||||||
// exists.
|
// section is correctly kept out of the Copilot agents picker).
|
||||||
// [agent] customize.toml for the two persona fixtures.
|
// [agent] customize.toml for the two persona fixtures.
|
||||||
await fs.writeFile(
|
await fs.writeFile(
|
||||||
path.join(installedBmadDir17, 'core', 'bmad-agent-fixture', 'customize.toml'),
|
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(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(agentFileForWorkflow17)), 'Workflow skill (no customize.toml) is FILTERED OUT of .github/agents/');
|
||||||
assert(
|
assert(
|
||||||
await fs.pathExists(agentFileForBmadHelp17),
|
!(await fs.pathExists(agentFileForBmadHelp17)),
|
||||||
'bmad-help is INCLUDED in agents picker via ALWAYS_AGENT_IDS exception (structural meta-skill, no customize.toml)',
|
'bmad-help is excluded from Copilot agents picker (no customize.toml; allowlist removed per maintainer feedback)',
|
||||||
);
|
);
|
||||||
assert(
|
assert(
|
||||||
!(await fs.pathExists(agentFileForMetaSkill17)),
|
!(await fs.pathExists(agentFileForMetaSkill17)),
|
||||||
|
|
|
||||||
|
|
@ -57,15 +57,6 @@ function isSafeCanonicalId(value) {
|
||||||
// OpenCode's native `@skills/<id>` skill-reference syntax.
|
// OpenCode's native `@skills/<id>` skill-reference syntax.
|
||||||
const DEFAULT_COMMANDS_BODY_TEMPLATE = '@skills/{canonicalId}';
|
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)?
|
// 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
|
// Used by platforms that surface only persona agents (e.g. Copilot's Custom
|
||||||
// Agents picker). Signal: the skill's source `customize.toml` has an
|
// 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
|
// GDS, WDS, TEA, and correctly excludes meta-skills like
|
||||||
// `bmad-agent-builder` (a skill-builder workflow whose canonical id
|
// `bmad-agent-builder` (a skill-builder workflow whose canonical id
|
||||||
// contains `-agent-` but which has no [agent] section because it isn't a
|
// 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
|
// persona itself).
|
||||||
// structural exception (`bmad-help`).
|
|
||||||
//
|
//
|
||||||
// Reading the source toml — at install time the source skill directory
|
// Reading the source toml — at install time the source skill directory
|
||||||
// (resolved from manifest record.path) still exists; cleanup runs later
|
// (resolved from manifest record.path) still exists; cleanup runs later
|
||||||
// in the install flow.
|
// in the install flow.
|
||||||
async function isAgentSkill(record, bmadDir) {
|
async function isAgentSkill(record, bmadDir) {
|
||||||
if (!record?.path || !bmadDir) return false;
|
if (!record?.path || !bmadDir) return false;
|
||||||
if (record.canonicalId && ALWAYS_AGENT_IDS.has(record.canonicalId)) return true;
|
|
||||||
const bmadFolderName = path.basename(bmadDir);
|
const bmadFolderName = path.basename(bmadDir);
|
||||||
const bmadPrefix = bmadFolderName + '/';
|
const bmadPrefix = bmadFolderName + '/';
|
||||||
const relativePath = record.path.startsWith(bmadPrefix) ? record.path.slice(bmadPrefix.length) : record.path;
|
const relativePath = record.path.startsWith(bmadPrefix) ? record.path.slice(bmadPrefix.length) : record.path;
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue