From 83273f96427ae992243546e97c169e53f0bb1a49 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 28 Aug 2026 18:02:44 -0400 Subject: [PATCH] fix(#3798): the profile closure follows command references into workflow spawn surfaces (#4009) * test(#3798): tiered profiles must install the agents their workflows spawn * fix(#3798): the profile closure follows command references into workflow spawn surfaces * chore(#3798): changeset fragment (pr number backfilled after PR creation) * chore(#3798): backfill changeset PR number (4009) --------- Co-authored-by: sim --- .changeset/tidy-otters-squeak.md | 5 + scripts/lint-test-file-count.allowlist.json | 3 +- src/capability-state.cts | 21 +++- src/install-profiles.cts | 65 +++++++++- tests/capability-state.test.cjs | 8 +- .../install-profile-workflow-spawns.test.cjs | 118 ++++++++++++++++++ 6 files changed, 210 insertions(+), 10 deletions(-) create mode 100644 .changeset/tidy-otters-squeak.md create mode 100644 tests/install-profile-workflow-spawns.test.cjs diff --git a/.changeset/tidy-otters-squeak.md b/.changeset/tidy-otters-squeak.md new file mode 100644 index 000000000..376016359 --- /dev/null +++ b/.changeset/tidy-otters-squeak.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4009 +--- +**Tiered profiles install the agents their own skills spawn** — the profile closure now follows each command into the workflow files it references (including split workflows' steps/ and modes/ fragments) when deriving the agent set, so `--profile=standard` no longer omits `gsd-verifier` (phase-goal verification failed at the point of spawn, after execution work had landed) or the thirteen other spawn targets living only in workflow bodies. (#3798) diff --git a/scripts/lint-test-file-count.allowlist.json b/scripts/lint-test-file-count.allowlist.json index 6a4af9803..61d902450 100644 --- a/scripts/lint-test-file-count.allowlist.json +++ b/scripts/lint-test-file-count.allowlist.json @@ -144,13 +144,14 @@ "install-minimal-hooks.test.cjs", "install-nested-layout.test.cjs", "install-path-detection.test.cjs", + "install-profile-workflow-spawns.test.cjs", "install-regressions.test.cjs", "install-runtime-artifacts.test.cjs", "install-update-marker.test.cjs", "install-write-confinement.test.cjs", "install.test.cjs" ], - "issue": "1679" + "issue": "#3798 \u2014 the profile-closure spawn test reads the real repo surface (commands+workflows+agents); consolidating it into install.test.cjs would put a whole-repo scan behind the suite's heaviest file" }, "edge-probe": { "files": [ diff --git a/src/capability-state.cts b/src/capability-state.cts index d148d6db7..dc987e978 100644 --- a/src/capability-state.cts +++ b/src/capability-state.cts @@ -46,7 +46,7 @@ const { loadConfig } = configLoaderMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import installProfilesMod = require('./install-profiles.cjs'); -const { readActiveProfile, loadSkillsManifest, resolveProfile, parseRequires, parseCallsAgents } = installProfilesMod; +const { readActiveProfile, loadSkillsManifest, resolveProfile, parseRequires, parseCallsAgents, workflowAgentRefs } = installProfilesMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import surfaceMod = require('./surface.cjs'); @@ -401,7 +401,17 @@ function _loadInstalledSkillsManifest(configDir: string): Map * no gsd-*.md files (so _resolveManifest can use size>0 as the "flat layout * present" signal and fall through to the installed-skills branch otherwise). */ -function _loadFlatCommandsGsdManifest(commandsParentDir: string): Map { +function _loadFlatCommandsGsdManifest(commandsParentDir: string, workflowsDir?: string): Map { + // #3798: derive agents from the command body PLUS the workflow files it + // references, exactly like loadSkillsManifest does for the nested layout — + // the flat (#1858) layout retained the original defect otherwise, and the + // parity test in tests/capability-state.test.cjs asserts the two loaders + // produce identical _calls_agents_* sets. Default: /gsd-core/workflows + // one level above the commands dir — the shape of both the repo checkout + // (/commands/) and a flat runtime install (the runtime config dir's + // commands/ with the package's gsd-core/ beside it). + const flatWorkflowsDir = workflowsDir + || path.resolve(commandsParentDir, '..', 'gsd-core', 'workflows'); const manifest = new Map(); let entries: fs.Dirent[]; try { @@ -424,7 +434,12 @@ function _loadFlatCommandsGsdManifest(commandsParentDir: string): Map` so they don't conflict with skill stems. + * + * #3798: command bodies are thin delegators — the actual `subagent_type=` + * spawns live in the workflow files each command references + * (`@…/workflows/.md`). The agent derivation therefore ALSO reads every + * workflow file the command references (plus the workflow's steps/ fragments + * when it is split per the progressive-disclosure pattern) and unions their + * `gsd-*` tokens into the same `_calls_agents_` set. Without this, + * tiered profiles omitted agents their own installed skills spawn + * (gsd-verifier at execute-phase's verify_phase_goal was the filed repro). + * Over-inclusion fails safe: a tiered profile installing one extra agent + * costs bytes, never a broken spawn. */ const DEFAULT_COMMANDS_DIR = path.resolve(__dirname, '..', '..', '..', 'commands', 'gsd'); +const DEFAULT_WORKFLOWS_DIR = path.resolve(__dirname, '..', '..', 'workflows'); -function loadSkillsManifest(commandsDir: string = DEFAULT_COMMANDS_DIR): Map { +/** + * Collect the `gsd-*` agent tokens from every workflow file a command body + * references. References are the `workflows/.md` path suffixes the + * delegating commands embed (`@…/gsd-core/workflows/.md`). A + * split workflow (workflows//steps/*.md) contributes its fragments as + * well, because the parent dispatches into them and the spawns live there. + */ +function workflowAgentRefs(content: string, workflowsDir: string): string[] { + const refs = content.match(/workflows\/([a-z0-9][a-z0-9-]*)\.md/g) || []; + const names = [...new Set(refs.map((r) => r.slice('workflows/'.length)))]; + const tokens = new Set(); + for (const name of names) { + const direct = path.join(workflowsDir, name); + let body: string | null = null; + try { body = fs.readFileSync(direct, 'utf8'); } catch { body = null; } + if (body === null) continue; + for (const tok of parseCallsAgents(body)) tokens.add(tok); + // Split workflow: union EVERY fragment under the workflow's directory + // (steps/, modes/, templates/…) — the parent dispatches into these per + // the progressive-disclosure pattern, and spawns live in all of them + // (#3798 review: modes/ carried gsd-advisor-researcher's spawn while the + // parent only named it incidentally in prose). + const fragDir = path.join(workflowsDir, name.slice(0, -3)); + const walk = (dir: string): void => { + let frags: fs.Dirent[]; + try { frags = fs.readdirSync(dir, { withFileTypes: true }); } catch { return; } + for (const frag of frags) { + const full = path.join(dir, frag.name); + if (frag.isDirectory()) { walk(full); continue; } + if (!frag.isFile() || !frag.name.endsWith('.md')) continue; + try { + const fragBody = fs.readFileSync(full, 'utf8'); + for (const tok of parseCallsAgents(fragBody)) tokens.add(tok); + } catch { /* unreadable fragment — skip */ } + } + }; + walk(fragDir); + } + return [...tokens]; +} + +function loadSkillsManifest(commandsDir: string = DEFAULT_COMMANDS_DIR, workflowsDir: string = DEFAULT_WORKFLOWS_DIR): Map { const manifest = new Map(); if (!fs.existsSync(commandsDir)) return manifest; const entries = fs.readdirSync(commandsDir, { withFileTypes: true }); @@ -172,9 +225,12 @@ function loadSkillsManifest(commandsDir: string = DEFAULT_COMMANDS_DIR): Map.md layout (#1858)', () => { // commands/gsd-.md in a temp dir, then compare stem sets. const tmpFlat = fs.mkdtempSync(path.join(os.tmpdir(), 'cap-parity-flat-')); try { - const nested = require('../gsd-core/bin/lib/install-profiles.cjs').loadSkillsManifest(realCommandsGsdDir); + // #3798: both loaders now union workflow spawn tokens; give them the + // SAME workflows dir so the comparison stays apples-to-apples (the flat + // mirror dir carries no workflows of its own). + const realWorkflowsDir = path.resolve(__dirname, '..', 'gsd-core', 'workflows'); + const nested = require('../gsd-core/bin/lib/install-profiles.cjs').loadSkillsManifest(realCommandsGsdDir, realWorkflowsDir); for (const [stem] of nested) { if (stem.startsWith('_calls_agents_')) continue; const src = path.join(realCommandsGsdDir, stem + '.md'); if (!fs.existsSync(src)) continue; fs.writeFileSync(path.join(tmpFlat, 'gsd-' + stem + '.md'), fs.readFileSync(src, 'utf8'), 'utf8'); } - const flat = _loadFlatCommandsGsdManifest(tmpFlat); + const flat = _loadFlatCommandsGsdManifest(tmpFlat, realWorkflowsDir); const nestedStems = [...nested.keys()].filter((k) => !k.startsWith('_calls_agents_')).sort(); const flatStems = [...flat.keys()].filter((k) => !k.startsWith('_calls_agents_')).sort(); assert.deepStrictEqual(flatStems, nestedStems, diff --git a/tests/install-profile-workflow-spawns.test.cjs b/tests/install-profile-workflow-spawns.test.cjs new file mode 100644 index 000000000..34d2b48f4 --- /dev/null +++ b/tests/install-profile-workflow-spawns.test.cjs @@ -0,0 +1,118 @@ +'use strict'; + +// ───────────────────────────────────────────────────────────────────────────── +// #3798 — tiered profiles must install the agents their own installed skills +// spawn. +// +// resolveProfile() derived the agent set by scanning ONLY the command bodies +// (commands/gsd/*.md), which are thin delegators — the actual +// `subagent_type="gsd-*"` spawns live in the workflow files those commands +// reference. Under --profile=standard that omitted gsd-verifier (spawned by +// execute-phase's verify_phase_goal), so phase-goal verification failed at +// the point of spawn, AFTER execution work had landed. The manifest's agent +// derivation now also reads every referenced workflow body (plus split +// workflows' steps/ fragments) and unions their gsd-* tokens. +// ───────────────────────────────────────────────────────────────────────────── + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { cleanup } = require('./helpers.cjs'); + +const REPO = path.join(__dirname, '..'); +const { + loadSkillsManifest, + resolveProfile, +} = require('../gsd-core/bin/lib/install-profiles.cjs'); + +const COMMANDS_DIR = path.join(REPO, 'commands', 'gsd'); +const WORKFLOWS_DIR = path.join(REPO, 'gsd-core', 'workflows'); +const AGENTS_DIR = path.join(REPO, 'agents'); + +const realAgents = new Set( + fs.readdirSync(AGENTS_DIR).filter((f) => f.endsWith('.md')).map((f) => f.slice(0, -3)), +); + +function realAgentsFor(mode) { + const man = loadSkillsManifest(COMMANDS_DIR, WORKFLOWS_DIR); + const resolved = resolveProfile({ modes: [mode], manifest: man }); + assert.notStrictEqual(resolved.skills, '*', `${mode} must resolve to a tiered profile in this repo`); + return { resolved, installed: [...resolved.agents].filter((a) => realAgents.has(a)) }; +} + +describe('install-profile workflow spawn closure (#3798)', () => { + test('#3798: standard profile includes the agents its workflows spawn (gsd-verifier)', () => { + const { installed } = realAgentsFor('standard'); + assert.ok( + installed.includes('gsd-verifier'), + `gsd-verifier is spawned by execute-phase's verify_phase_goal and must be installed under standard; got: ${installed.join(' ')}`, + ); + }); + + test('#3798: the agent set is a superset of referenced workflows\' spawn tokens', () => { + const { resolved, installed } = realAgentsFor('standard'); + // Independently re-derive the gsd-* tokens in every workflow referenced by + // a standard skill, and require each token that names a real agent to be + // in the installed set — the property whose absence was the bug. + const tokens = new Set(); + for (const skill of resolved.skills) { + const cmd = path.join(COMMANDS_DIR, `${skill}.md`); + if (!fs.existsSync(cmd)) continue; + const body = fs.readFileSync(cmd, 'utf8'); + const refs = body.match(/workflows\/([a-z0-9][a-z0-9-]*)\.md/g) || []; + for (const ref of refs) { + const wf = path.join(WORKFLOWS_DIR, ref.slice('workflows/'.length)); + try { + for (const tok of fs.readFileSync(wf, 'utf8').match(/\bgsd-[a-z][a-z-]*/g) || []) { + tokens.add(tok); + } + } catch { /* workflow file absent — skip */ } + } + } + const missing = [...tokens].filter((t) => realAgents.has(t) && !installed.includes(t)); + assert.deepEqual( + missing, + [], + `#3798: every real agent named in a referenced workflow must be installed; missing: ${missing.join(', ')}`, + ); + }); + + test('#3798 control: the full sentinel is unchanged', () => { + const man = loadSkillsManifest(COMMANDS_DIR, WORKFLOWS_DIR); + const full = resolveProfile({ modes: ['full'], manifest: man }); + assert.strictEqual(full.skills, '*'); + assert.strictEqual(full.agents.size, 0); + }); + + test('#3798 control: the requires: skill closure is unchanged by the agent derivation', () => { + const { resolved } = realAgentsFor('standard'); + // The skill set must still be exactly the requires: closure — spot-check + // a known member and a known non-member of standard's closure. + assert.ok(resolved.skills.has('execute-phase'), 'execute-phase is a standard skill'); + assert.ok(!resolved.skills.has('autonomous'), + 'autonomous is not in standard\'s requires: closure — the agent derivation must not widen skills'); + }); + + test('#3798: workflow fragments (steps/, modes/) contribute spawn tokens', (t) => { + // Fixture workflows dir: a parent workflow referencing nothing itself, + // whose modes/ fragment carries the only spawn token — proves the + // recursive fragment walk, not just the parent body, feeds the union. + const os = require('node:os'); + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3798-wf-')); + t.after(() => cleanup(tmp)); + const wfDir = path.join(tmp, 'workflows'); + fs.mkdirSync(path.join(wfDir, 'probe', 'modes'), { recursive: true }); + fs.writeFileSync(path.join(wfDir, 'probe.md'), '# Probe\n\nNo agents here.\n'); + fs.writeFileSync(path.join(wfDir, 'probe', 'modes', 'advisor.md'), 'Spawn: subagent_type="gsd-fixture-only-agent"\n'); + const cmdDir = path.join(tmp, 'commands', 'gsd'); + fs.mkdirSync(cmdDir, { recursive: true }); + fs.writeFileSync(path.join(cmdDir, 'probe-skill.md'), '@~/.claude/gsd-core/workflows/probe.md\n'); + + const man = loadSkillsManifest(cmdDir, wfDir); + assert.ok( + (man.get('_calls_agents_probe-skill') || []).includes('gsd-fixture-only-agent'), + '#3798: a spawn token living ONLY in a workflow fragment (modes/) must reach the agent set', + ); + }); +});