diff --git a/.changeset/witty-herons-march.md b/.changeset/witty-herons-march.md new file mode 100644 index 000000000..b34c06643 --- /dev/null +++ b/.changeset/witty-herons-march.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3229 +--- +**npm-global installs can now actually fail the agents-installed gate** — `checkAgentsInstalled` resolved the claude agents directory relative to its own install location, so an npm-global install validated the package's bundled `agents/` against itself and `agents_installed` could never be `false`, silently disabling the halt/warn gates in `new-project` and `new-milestone`. When the install-relative path lies inside a `node_modules` tree the claude runtime now resolves `getGlobalConfigDir('claude')/agents` like every other runtime, honouring `CLAUDE_CONFIG_DIR`; repo runs and runtime-config-dir installs are unchanged, and the `GSD_AGENTS_DIR` override stays priority 1. (#3203) diff --git a/CONTEXT.md b/CONTEXT.md index 59a2a1b59..0e7385251 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -195,7 +195,7 @@ Module owning ROADMAP.md parsing: shipped-milestone slicing, current-milestone e Module owning the shared low-level utility primitives extracted from Core: POSIX path normalization (`toPosixPath`), filesystem scanning (`detectSubRepos`, `readSubdirectories`, `getPhaseFileStats`, `pathExistsInternal`), plan/summary pairing helpers (`countMatchedSummaries`, `findUnsummarizedPlans`, `findOrphanSummaries`), and small pure helpers (`generateSlugInternal`, `extractOneLinerFromBody`, `extractCanonicalPlanId`, `timeAgo`). `filterPlanFiles`/`filterSummaryFiles` were retired by #3183 (ADR-3180 Decision 2) — `getPhaseFileStats` no longer re-derives plan/summary filename matching locally; it now sources `plans`/`summaries` (plus a `scope` field, `COMPLETE`/`TRUNCATED`/`UNREADABLE`) directly from `scanPhasePlans` (`src/plan-scan.cts`), the single owner of live-plan counting. Depends only on Node built-ins and already-leafed modules (`phase-id` for `comparePhaseNum`, `planning-workspace` for `findContextMdIn`) — no `loadConfig`, no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2c (#877) as the shared leaf that unblocks the phase-locator fs-search extraction (2d); the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/core-utils.cjs` (generated from `src/core-utils.cts`). ### Agent Install Check Module -Module owning agent-presence resolution and verification, extracted from the Core module as the cleanup step that retired the `core.cjs` re-export spine (the final ADR-857 decomposition, epic #1267). Interface: `getAgentsDir(runtime?, projectRoot?)` — env-var-aware, runtime-aware agents-directory resolution; Claude resolves `__dirname`-relative, while other runtimes prefer a manifest-backed project-local agents directory before their global configuration home. The manifest gate is intentional: runtime-native project agents must not shadow a working global GSD install. `checkAgentsInstalled(...)` validates `gsd-file-manifest.json` completeness and confirms the declared agents exist on disk. Pure read/verify — no install-write side effects (writes remain the Installer Module's). Consumed by the Init Command Module, the verify workflow, and the docs workflow. Source of truth: `gsd-core/bin/lib/agent-install-check.cjs` (generated from `src/agent-install-check.cts`); replaced the two functions that squatted in `core.cts`. See Installer Module and ADR-857. +Module owning agent-presence resolution and verification, extracted from the Core module as the cleanup step that retired the `core.cjs` re-export spine (the final ADR-857 decomposition, epic #1267). Interface: `getAgentsDir(runtime?, projectRoot?)` — env-var-aware, runtime-aware agents-directory resolution; Claude resolves `__dirname`-relative unless that path contains a `node_modules` path segment, in which case it falls back to `getGlobalConfigDir('claude')/agents` (#3203); that segment test is lexical and case-sensitive rather than an install-shape guarantee — it targets the layouts where the sibling `agents/` is the package's own bundled copy and the check would otherwise validate the package against itself, and any path merely carrying a directory of that name resolves the same way. Other runtimes prefer a manifest-backed project-local agents directory before their global configuration home. The manifest gate is intentional: runtime-native project agents must not shadow a working global GSD install. `checkAgentsInstalled(...)` validates `gsd-file-manifest.json` completeness and confirms the declared agents exist on disk. Pure read/verify — no install-write side effects (writes remain the Installer Module's). Consumed by the Init Command Module, the verify workflow, and the docs workflow. Source of truth: `gsd-core/bin/lib/agent-install-check.cjs` (generated from `src/agent-install-check.cts`); replaced the two functions that squatted in `core.cts`. See Installer Module and ADR-857. ### Config Loader Module Module owning project configuration loading: reads `.planning/config.json`, merges built-in defaults (`CONFIG_DEFAULTS`/`CANONICAL_CONFIG_DEFAULTS`), normalizes legacy keys, applies the active-workstream overlay, validates against the config schema, and warns on unknown keys/profile overrides. Primary interface: `loadConfigResolved(cwd, options) → ConfigResolution { config, source, degraded }` (provenance-aware, ADR-1411 P2 / #1415) — `source` ∈ `'workstream' | 'root' | 'builtin-defaults' | 'global-defaults'`; `degraded:true` when a workstream was requested but its config.json was absent (fell back to root config). `loadConfig(cwd, options) → Record` is the back-compat thin wrapper over `loadConfigResolved` (byte-identical result). Resolution is **caller-anchored, not loader-anchored**: `loadConfigResolved` resolves `cwd` as-is (no walk-up), so `loadConfig` stays byte-identical for its callers; callers that need cwd-drift tolerance (e.g. `cmdAgentSkills`) anchor to the project root via `findProjectRoot` (Project-Root Resolution Module) *before* calling `loadConfigResolved`. Helper exports: `_deepMergeConfig`, `isGitIgnored`, `_warnUnknownProfileOverrides`. Depends only on leaf modules (`configuration`, `config-schema`, `planning-workspace`, `shell-command-projection`, `core-utils`, `model-catalog`) — no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2e (#885) as the prerequisite for the model-resolver extraction (the resolvers call `loadConfig`); the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/config-loader.cjs` (generated from `src/config-loader.cts`). diff --git a/src/agent-install-check.cts b/src/agent-install-check.cts index 6c3687601..85fbc65d3 100644 --- a/src/agent-install-check.cts +++ b/src/agent-install-check.cts @@ -81,9 +81,17 @@ function truncatePostureValue(value: string): string { * * Priority: * 1. GSD_AGENTS_DIR env var (explicit override, any runtime) - * 2. For claude runtime: __dirname-relative path (agents/ sibling of gsd-core/) - * This is correct for both repo runs and real installs (the runtime config dir's - * agents/ folder) because gsd-tools.cjs lives inside gsd-core/bin/ in both cases. + * 2. For claude runtime: __dirname-relative path (agents/ sibling of + * gsd-core/) — correct for repo runs and runtime-config-dir installs, + * where the sibling agents/ IS the user's agents dir — UNLESS that path + * carries an exact node_modules segment. gsd-tools.cjs lives inside + * gsd-core/bin/ in every install shape, but on an npm-global install + * gsd-core/ sits inside the package (not the runtime config dir) and the + * package ships its own agents/, so the install-relative path resolves + * to the bundled copy and the check validates the package against + * itself — agents_installed can never be false. In that case resolve + * getGlobalConfigDir('claude')/agents (honours CLAUDE_CONFIG_DIR) like + * every other runtime (#3203). * 3. For non-claude runtimes with a manifest-backed project-local install: * //agents (or /agents when * the runtime's local install targets the project root). Requiring the @@ -100,7 +108,16 @@ function getAgentsDir(runtime?: string, projectRoot?: string): string { } const resolved = runtime ?? (process.env['GSD_RUNTIME'] || 'claude'); if (resolved === 'claude') { - return path.join(__dirname, '..', '..', '..', 'agents'); + const installRelative = path.join(__dirname, '..', '..', '..', 'agents'); + // #3203: a lexical guard, not an install-shape test. It targets the + // layouts where the sibling agents/ is the package's own bundled copy and + // the check would otherwise validate the package against itself; a path + // merely carrying a directory of that name resolves the same way, and a + // non-empty GSD_AGENTS_DIR overrides both. + if (installRelative.split(path.sep).includes('node_modules')) { + return path.join(getGlobalConfigDir('claude'), 'agents'); + } + return installRelative; } if (projectRoot) { // eslint-disable-next-line @typescript-eslint/no-require-imports diff --git a/tests/agent-install-check.test.cjs b/tests/agent-install-check.test.cjs index b07cd1db4..838ec1e3a 100644 --- a/tests/agent-install-check.test.cjs +++ b/tests/agent-install-check.test.cjs @@ -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', () => {