* 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 <sim@local>
This commit is contained in:
5
.changeset/tidy-otters-squeak.md
Normal file
5
.changeset/tidy-otters-squeak.md
Normal file
@@ -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)
|
||||
@@ -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": [
|
||||
|
||||
@@ -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<string, string[]>
|
||||
* 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<string, string[]> {
|
||||
function _loadFlatCommandsGsdManifest(commandsParentDir: string, workflowsDir?: string): Map<string, string[]> {
|
||||
// #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: <pkg>/gsd-core/workflows
|
||||
// one level above the commands dir — the shape of both the repo checkout
|
||||
// (<repo>/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<string, string[]>();
|
||||
let entries: fs.Dirent[];
|
||||
try {
|
||||
@@ -424,7 +434,12 @@ function _loadFlatCommandsGsdManifest(commandsParentDir: string): Map<string, st
|
||||
try {
|
||||
const content = fs.readFileSync(path.join(commandsParentDir, entry.name), 'utf8');
|
||||
manifest.set(stem, parseRequires(content));
|
||||
manifest.set(`_calls_agents_${stem}`, parseCallsAgents(content));
|
||||
manifest.set(`_calls_agents_${stem}`, [
|
||||
...new Set([
|
||||
...parseCallsAgents(content),
|
||||
...workflowAgentRefs(content, flatWorkflowsDir),
|
||||
]),
|
||||
]);
|
||||
} catch {
|
||||
manifest.set(stem, []);
|
||||
manifest.set(`_calls_agents_${stem}`, []);
|
||||
|
||||
@@ -158,10 +158,63 @@ function parseCallsAgents(content: string): string[] {
|
||||
* Also derives calls_agents for each skill by scanning the body text for
|
||||
* `gsd-*` agent name references. Agent stems are stored under the special
|
||||
* key `_calls_agents_<stem>` 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/<name>.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_<stem>` 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<string, string[]> {
|
||||
/**
|
||||
* Collect the `gsd-*` agent tokens from every workflow file a command body
|
||||
* references. References are the `workflows/<name>.md` path suffixes the
|
||||
* delegating commands embed (`@…/gsd-core/workflows/<name>.md`). A
|
||||
* split workflow (workflows/<name>/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<string>();
|
||||
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<string, string[]> {
|
||||
const manifest = new Map<string, string[]>();
|
||||
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<str
|
||||
try {
|
||||
const content = fs.readFileSync(path.join(commandsDir, entry.name), 'utf8');
|
||||
manifest.set(stem, parseRequires(content));
|
||||
// Derive agent references from body text
|
||||
const agentRefs = parseCallsAgents(content);
|
||||
manifest.set(`_calls_agents_${stem}`, agentRefs);
|
||||
// Derive agent references from body text + the workflows it delegates to
|
||||
const agentRefs = [
|
||||
...parseCallsAgents(content),
|
||||
...workflowAgentRefs(content, workflowsDir),
|
||||
];
|
||||
manifest.set(`_calls_agents_${stem}`, [...new Set(agentRefs)]);
|
||||
} catch {
|
||||
manifest.set(stem, []);
|
||||
manifest.set(`_calls_agents_${stem}`, []);
|
||||
@@ -1220,6 +1276,7 @@ export = {
|
||||
// Shared internals
|
||||
parseRequires,
|
||||
parseCallsAgents,
|
||||
workflowAgentRefs,
|
||||
cleanupStagedSkills,
|
||||
// #2322: capability-skill security seams — exported for direct unit-testing
|
||||
// and for surface.cts's prune pass (CAPABILITY_SKILL_MARKER parity).
|
||||
|
||||
@@ -1016,14 +1016,18 @@ describe('regressions: flat commands/gsd-<stem>.md layout (#1858)', () => {
|
||||
// commands/gsd-<stem>.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,
|
||||
|
||||
118
tests/install-profile-workflow-spawns.test.cjs
Normal file
118
tests/install-profile-workflow-spawns.test.cjs
Normal file
@@ -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',
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user