* fix(#3203): stop npm-global installs validating bundled agents against themselves getAgentsDir's claude branch derived the agents directory from __dirname, which is correct for repo runs and runtime-config-dir installs (where <root>/../agents IS the user's agents dir) but on an npm-global install resolves to the package's own bundled agents/, so checkAgentsInstalled validated the package against itself and agents_installed could never be false. The new-project and new-milestone halt/warn gates were silently dead for npm-global users. Keep the install-relative path for the shapes where it is correct, but when it lies inside a node_modules tree — the provably self-validating case — resolve getGlobalConfigDir('claude')/agents like every other runtime, honouring CLAUDE_CONFIG_DIR. GSD_AGENTS_DIR stays priority 1. Repair the doc comment that asserted the __dirname form was correct for both install shapes. Regression test mirrors the published npm-global layout (package under node_modules with a complete bundled agents/) and pins the resolved directory plus the issue's negative control (one agent missing from the config dir → agents_installed:false). Verified red against pre-fix code, green post-fix; the repo-layout W010 health test stays green. * chore(#3203): set changeset fragment pr to 3229 * docs(#3203): describe the node_modules guard as lexical, in CONTEXT.md and at the call site The Agent Install Check Module glossary entry asserted that Claude resolves the agents directory `__dirname`-relative unconditionally. That is the premise this PR falsified: on an npm-global install the install-relative path resolves to the package's own bundled `agents/`, so the check validated the package against itself and `agents_installed` could never be false. The inline doc comment above `getAgentsDir` was repaired with the fix; this external predicate was left behind and has been false since. CONTRIBUTING.md's `Fixed`-fragment docs exemption names this case explicitly — "Edit the docs anyway if a fix corrects something the docs got wrong." Both surfaces now describe the guard as what it is: an exact, case-sensitive path-segment test that TARGETS those layouts rather than detecting them, so neither claims more certainty than the predicate has. The call-site comment carried the same conflation the glossary did. A path merely carrying a directory of that name resolves the same way — the edge already disclosed on this PR — and a non-empty GSD_AGENTS_DIR overrides it. Comment-only in `src/`; no behaviour change. `CONFIG.LOCATION.SEAM.two-families` needs no change: `GSD_AGENTS_DIR -> getAgentsDir priority 1` is still accurate. --------- Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
@@ -107,11 +107,67 @@ describe('getAgentsDir', () => {
|
||||
assert.strictEqual(agentInstallCheck.getAgentsDir('cursor'), '/tmp/x');
|
||||
});
|
||||
|
||||
test('claude runtime returns __dirname-relative path', () => {
|
||||
const fromModule = agentInstallCheck.getAgentsDir('claude');
|
||||
// Should end with /agents
|
||||
assert.ok(fromModule.endsWith(path.sep + 'agents') || fromModule.endsWith('/agents'),
|
||||
`Expected path to end with /agents, got: ${fromModule}`);
|
||||
test('claude runtime outside node_modules returns the install-relative path', () => {
|
||||
// Repo runs and runtime-config-dir installs: the sibling agents/ IS the
|
||||
// user's agents dir, so install-relative resolution is correct there.
|
||||
const expected = path.resolve(
|
||||
path.dirname(AGENT_INSTALL_CHECK_PATH), '..', '..', '..', 'agents'
|
||||
);
|
||||
assert.strictEqual(agentInstallCheck.getAgentsDir('claude'), expected);
|
||||
});
|
||||
|
||||
test('npm-global install resolves the config dir, never the bundled agents (#3203)', (t) => {
|
||||
// Mirror the published npm-global layout: the package sits inside a
|
||||
// node_modules tree and ships its own agents/. Pre-fix, getAgentsDir
|
||||
// resolved that bundled copy, so checkAgentsInstalled validated the
|
||||
// package against itself and agents_installed could never be false.
|
||||
const tmp = createTempDir('gsd-npm-global-');
|
||||
const savedClaudeDir = process.env['CLAUDE_CONFIG_DIR'];
|
||||
t.after(() => cleanup(tmp));
|
||||
t.after(() => {
|
||||
if (savedClaudeDir === undefined) {
|
||||
delete process.env['CLAUDE_CONFIG_DIR'];
|
||||
} else {
|
||||
process.env['CLAUDE_CONFIG_DIR'] = savedClaudeDir;
|
||||
}
|
||||
});
|
||||
|
||||
const pkgRoot = path.join(tmp, 'node_modules', '@opengsd', 'gsd-core');
|
||||
fs.cpSync(
|
||||
path.join(__dirname, '..', 'gsd-core', 'bin'),
|
||||
path.join(pkgRoot, 'gsd-core', 'bin'),
|
||||
{ recursive: true }
|
||||
);
|
||||
// Bundled agents/ is always complete — that is exactly why the pre-fix
|
||||
// self-validation could never report a missing agent.
|
||||
const bundledAgents = path.join(pkgRoot, 'agents');
|
||||
fs.mkdirSync(bundledAgents, { recursive: true });
|
||||
for (const agent of EXPECTED_AGENTS) {
|
||||
fs.writeFileSync(path.join(bundledAgents, `${agent}.md`), `# ${agent}\n`);
|
||||
}
|
||||
|
||||
// Config dir carries every expected agent except the first — the issue
|
||||
// repro's negative control (gsd-verifier removed from ~/.claude/agents).
|
||||
const configDir = path.join(tmp, 'claude-config');
|
||||
const agentsDir = path.join(configDir, 'agents');
|
||||
fs.mkdirSync(agentsDir, { recursive: true });
|
||||
const [removedAgent, ...presentAgents] = EXPECTED_AGENTS;
|
||||
for (const agent of presentAgents) {
|
||||
fs.writeFileSync(path.join(agentsDir, `${agent}.md`), `# ${agent}\n`);
|
||||
}
|
||||
process.env['CLAUDE_CONFIG_DIR'] = configDir;
|
||||
|
||||
const globalInstallCheck = require(
|
||||
path.join(pkgRoot, 'gsd-core', 'bin', 'lib', 'agent-install-check.cjs')
|
||||
);
|
||||
// Pin the resolved directory, not just an /agents suffix — the pre-fix
|
||||
// resolver also ended with /agents, which is how the bug survived.
|
||||
assert.strictEqual(globalInstallCheck.getAgentsDir('claude'), agentsDir);
|
||||
|
||||
const result = globalInstallCheck.checkAgentsInstalled('claude');
|
||||
assert.strictEqual(result.agents_dir, agentsDir);
|
||||
assert.strictEqual(result.agents_installed, false);
|
||||
assert.deepStrictEqual(result.missing_agents, [removedAgent]);
|
||||
});
|
||||
|
||||
test('non-claude runtime returns getGlobalConfigDir(runtime)/agents', () => {
|
||||
|
||||
Reference in New Issue
Block a user