diff --git a/.changeset/nimble-newts-rally.md b/.changeset/nimble-newts-rally.md new file mode 100644 index 000000000..ecca6fc3e --- /dev/null +++ b/.changeset/nimble-newts-rally.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 1313 +--- +**Graphify now respects surface/profile state, not just `graphify.enabled`** — `gsd-tools graphify` is off unless graphify is installed AND surfaced AND `graphify.enabled` is true (previously only the config key was checked). The gate is now runtime-aware: Codex/Cursor/etc. read their own runtime's surface instead of `~/.claude`. (#1313) diff --git a/CONTEXT.md b/CONTEXT.md index 686ea3076..e055a4474 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -221,7 +221,7 @@ Module owning the projection of gsd-core's artifact surfaces (`commands`, `agent The repo-root `gemini-extension.json` + `GEMINI.md` pair that projects gsd-core onto the Gemini CLI extension contract, enabling one-step lifecycle management via `gemini extensions install ` / `update` / `remove` (and `gemini extensions link ` for dev). The Gemini-CLI sibling of the Claude Code Plugin Manifest Module — same additive idea, different runtime package format. Defined mapping: `name`=`binName` (`gsd-core`; lowercase-dashes per Gemini's extension naming rule), `version` tracks `package.json` (Gemini's `gemini extensions update` keys off the manifest `version` field), `description` (required by the manifest schema), `contextFileName`=`GEMINI.md` (the extension's context payload, loaded into every Gemini session). Intentionally minimal: no `mcpServers` (gsd-core ships no MCP server). Slash-command / agent / hook projection into the extension (which would require committing the Gemini-format TOML/agent conversions the Installer Module produces at `--gemini` install time) is deferred — the manual `npx gsd-core --gemini` path remains the way to install the `/gsd:*` commands, and is unchanged (additive, no breaking change). Conformance is guarded by the in-repo drift test `tests/issue-775-gemini-extension.test.cjs` (manifest validity, `version`↔`package.json` parity, `contextFileName` existence, `files[]` publication). _Avoid_: "the Gemini plugin" (Gemini calls them extensions, not plugins). See #775, ADR-766, Claude Code Plugin Manifest Module, and Runtime Artifact Layout Module. ### Knowledge Graph Module -Module owning the graphify integration: config gate (`isGraphifyEnabled`), disabled response (`disabledResponse`), subprocess helper (`execGraphify`, typed `GRAPHIFY_REASON` enum), presence detection (`checkGraphifyInstalled`), version checking (`checkGraphifyVersion`), query surface (`graphifyQuery` — BFS seed-expand + budget trim), status surface (`graphifyStatus` — node/edge counts, mtime staleness, commit-staleness tri-state via `built_at_commit`/`commits_behind`/`commit_stale`), diff surface (`graphifyDiff` — added/removed/changed nodes+edges), build pre-flight (`graphifyBuild`), snapshot management (`writeSnapshot`). Reads `.planning/config.json:graphify.enabled` as config gate; writes to `.planning/graphs/`. Auto-update hook (`hooks/gsd-graphify-update.sh`) triggers a detached background rebuild after HEAD-advancing git operations on the default branch when `graphify.auto_update=true`. Status file `.planning/graphs/.last-build-status.json` carries `{ ts, status, exit_code, duration_ms, head_at_build, graphify_version }`. Graph IR uses `nodes[]`, `edges[]` (or `links[]` for graphify ≥0.7 compat), `hyperedges[]`, `built_at_commit`. `commit_stale` is tri-state: `false` (known fresh), `true` (stale), `null` (unknown — no git or pre-v0.7 graph). Source: `gsd-core/bin/lib/graphify.cjs`. Skill: `commands/gsd/graphify.md`. +Module owning the graphify integration: tri-state capability gate (`isCapabilityActive('graphify', cwd)` from capability-state.cjs — requires installed AND surfaced AND config-enabled; replaces the former config-only `isGraphifyEnabled` gate, cutover in #1306), disabled response (`disabledResponse`), subprocess helper (`execGraphify`, typed `GRAPHIFY_REASON` enum), presence detection (`checkGraphifyInstalled`), version checking (`checkGraphifyVersion`), query surface (`graphifyQuery` — BFS seed-expand + budget trim), status surface (`graphifyStatus` — node/edge counts, mtime staleness, commit-staleness tri-state via `built_at_commit`/`commits_behind`/`commit_stale`), diff surface (`graphifyDiff` — added/removed/changed nodes+edges), build pre-flight (`graphifyBuild`), snapshot management (`writeSnapshot`). Config leg reads `.planning/config.json:graphify.enabled`; all three legs (install, surface, config) must be active; writes to `.planning/graphs/`. Auto-update hook (`hooks/gsd-graphify-update.sh`) triggers a detached background rebuild after HEAD-advancing git operations on the default branch when `graphify.auto_update=true`. Status file `.planning/graphs/.last-build-status.json` carries `{ ts, status, exit_code, duration_ms, head_at_build, graphify_version }`. Graph IR uses `nodes[]`, `edges[]` (or `links[]` for graphify ≥0.7 compat), `hyperedges[]`, `built_at_commit`. `commit_stale` is tri-state: `false` (known fresh), `true` (stale), `null` (unknown — no git or pre-v0.7 graph). Source: `gsd-core/bin/lib/graphify.cjs`. Skill: `commands/gsd/graphify.md`. ### Research Module The GSD-RESEARCH capability behind an L2-hybrid seam: code owns cache + provider policy + package legitimacy; MCP owns the actual fetch. Reachable via `gsd-tools query research-plan|research-store|package-legitimacy`. Source: `src/research-{store,provider}.cts` + `src/package-legitimacy.cts` (generated to `gsd-core/bin/lib/*.cjs` per ADR-457). Replaces the prose provider-waterfall duplicated across the researcher agents and the pip-install `slopcheck` bolt-on. diff --git a/docs/USER-GUIDE.md b/docs/USER-GUIDE.md index 0efd6ab1e..e76a8f4bb 100644 --- a/docs/USER-GUIDE.md +++ b/docs/USER-GUIDE.md @@ -459,6 +459,16 @@ The review step slots in after execution and before UAT: - **Configuration Reference:** see [`docs/CONFIGURATION.md`](CONFIGURATION.md) for the full `config.json` schema, model-profile table, git branching strategies, and security settings. - **Discuss Mode:** see [`docs/workflow-discuss-mode.md`](workflow-discuss-mode.md) for interview vs assumptions mode. +### Graphify capability gate (tri-state, v1.43+) + +Graphify commands (`graphify status`, `graphify build`, `graphify query`, `graphify diff`) now respect the **full tri-state capability gate**: + +1. **Installed** — the `gsd-graphify-*` skills are present in the active install profile. +2. **Surfaced** — those skills appear on the current runtime surface (e.g., in `~/.claude/commands/gsd/`). +3. **Config-enabled** — `graphify.enabled: true` is set in `.planning/config.json`. + +All three conditions must be true. Setting `graphify.enabled: true` alone is no longer sufficient if graphify has not been installed and surfaced. If graphify commands return `{ disabled: true }` after upgrading, verify that the install profile includes graphify skills (`gsd-tools capability state`) and re-run the installer to surface them. + --- ## Usage Examples diff --git a/src/capability-state.cts b/src/capability-state.cts index 8dadeb24c..c981af516 100644 --- a/src/capability-state.cts +++ b/src/capability-state.cts @@ -25,6 +25,7 @@ * - ./surface.cjs (resolveSurface) * - ./config-loader.cjs (loadConfig) * - ./runtime-homes.cjs (getGlobalConfigDir — for runtimeConfigDir auto-detection) + * - ./runtime-slash.cjs (resolveRuntime — GSD_RUNTIME > config.runtime > 'claude' precedence) * - capability-registry.cjs (loaded at call time) */ @@ -399,11 +400,15 @@ function _resolveManifest(commandsGsdDir: string, configDir: string): Map string; }; - // Delegate runtime detection entirely to getGlobalConfigDir: calling it - // with 'claude' causes it to check CLAUDE_CONFIG_DIR first, falling back - // to ~/.claude. The canonical resolver already encodes the correct env-var - // precedence for each runtime — we do not re-implement that logic here. - // For non-claude runtimes, the caller should pass --config-dir explicitly - // (or set the runtime-specific env var, which getGlobalConfigDir honors). - resolvedConfigDir = runtimeHomes.getGlobalConfigDir('claude'); + // eslint-disable-next-line @typescript-eslint/no-require-imports + const runtimeSlash = require('./runtime-slash.cjs') as { + resolveRuntime: (projectDir: string | null | undefined) => string; + }; + // Detect the active runtime via GSD_RUNTIME → config.runtime → 'claude'. + // resolveRuntime reads config.json directly (no side effects) and returns + // a lowercased canonical runtime name. + const detectedRuntime = runtimeSlash.resolveRuntime(cwd); + resolvedConfigDir = runtimeHomes.getGlobalConfigDir(detectedRuntime); } catch { // Defensive fallback: use ~/.claude if the canonical resolver throws. // eslint-disable-next-line @typescript-eslint/no-require-imports diff --git a/src/graphify.cts b/src/graphify.cts index d8432cfb6..bd095ecca 100644 --- a/src/graphify.cts +++ b/src/graphify.cts @@ -11,32 +11,11 @@ import fs from 'node:fs'; import path from 'node:path'; import { execTool, execGit, platformWriteSync } from './shell-command-projection.cjs'; -// ─── Config Gate ───────────────────────────────────────────────────────────── +// eslint-disable-next-line @typescript-eslint/no-require-imports +import capabilityStateMod = require('./capability-state.cjs'); +const { isCapabilityActive } = capabilityStateMod; -/** - * Check whether graphify is enabled in the project config. - * Reads config.json directly via fs. Returns false by default - * (when no config, no graphify key, or on error). - */ -function isGraphifyEnabled(planningDir: string): boolean { - try { - const configPath = path.join(planningDir, 'config.json'); - if (!fs.existsSync(configPath)) return false; - const config: unknown = JSON.parse(fs.readFileSync(configPath, 'utf8')); - if ( - config && - typeof config === 'object' && - 'graphify' in config && - config.graphify && - typeof config.graphify === 'object' && - 'enabled' in config.graphify && - (config.graphify as Record).enabled === true - ) return true; - return false; - } catch { - return false; - } -} +// ─── Config Gate ───────────────────────────────────────────────────────────── interface DisabledResponse { disabled: true; @@ -396,7 +375,7 @@ function countCommitsBetween(cwd: string, from: string, to: string): number | nu */ function graphifyQuery(cwd: string, term: string, options: { budget?: number | null } = {}): unknown { const planningDir = path.join(cwd, '.planning'); - if (!isGraphifyEnabled(planningDir)) return disabledResponse(); + if (!isCapabilityActive('graphify', cwd)) return disabledResponse(); const graphPath = path.join(planningDir, 'graphs', 'graph.json'); if (!fs.existsSync(graphPath)) { @@ -435,7 +414,7 @@ function graphifyQuery(cwd: string, term: string, options: { budget?: number | n */ function graphifyStatus(cwd: string): unknown { const planningDir = path.join(cwd, '.planning'); - if (!isGraphifyEnabled(planningDir)) return disabledResponse(); + if (!isCapabilityActive('graphify', cwd)) return disabledResponse(); const graphPath = path.join(planningDir, 'graphs', 'graph.json'); if (!fs.existsSync(graphPath)) { @@ -498,7 +477,7 @@ function graphifyStatus(cwd: string): unknown { */ function graphifyDiff(cwd: string): unknown { const planningDir = path.join(cwd, '.planning'); - if (!isGraphifyEnabled(planningDir)) return disabledResponse(); + if (!isCapabilityActive('graphify', cwd)) return disabledResponse(); const snapshotPath = path.join(planningDir, 'graphs', '.last-build-snapshot.json'); const graphPath = path.join(planningDir, 'graphs', 'graph.json'); @@ -554,7 +533,7 @@ function graphifyDiff(cwd: string): unknown { */ function graphifyBuild(cwd: string): unknown { const planningDir = path.join(cwd, '.planning'); - if (!isGraphifyEnabled(planningDir)) return disabledResponse(); + if (!isCapabilityActive('graphify', cwd)) return disabledResponse(); const installed = checkGraphifyInstalled(); if (!installed.installed) return { error: installed.message }; @@ -619,7 +598,6 @@ function writeSnapshot(cwd: string): SnapshotResult | { error: string } { export = { // Config gate - isGraphifyEnabled, disabledResponse, // Subprocess execGraphify, diff --git a/tests/capability-state.test.cjs b/tests/capability-state.test.cjs index cbac166eb..21d2feb87 100644 --- a/tests/capability-state.test.cjs +++ b/tests/capability-state.test.cjs @@ -1460,3 +1460,132 @@ describe('isCapabilityActive — convenience predicate (Phase 2)', () => { assert.strictEqual(isCapabilityActive('__no_such_cap__', nonExistentCwd), false); }); }); + +// ─── Cross-runtime runtime detection (HIGH fix — GSD_RUNTIME → config.runtime → 'claude') ────── + +describe('isCapabilityActive cross-runtime detection (GSD_RUNTIME → config.runtime → claude)', () => { + // Verifies the HIGH bug fix: when GSD_RUNTIME='codex', resolveCapabilityRuntimeState + // must consult the CODEX config dir (via CODEX_HOME), not ~/.claude. + // + // Fixture layout: + // CODEX_HOME → tmpCodexDir/ ← .gsd-surface.json: full profile, graphify SURFACED + // CLAUDE_CONFIG_DIR → tmpClaudeDir/ ← .gsd-surface.json: full profile, graphify NOT surfaced + // GSD_RUNTIME=codex + // project cwd → tmpProjectDir/ ← config.json: graphify.enabled=true + // (config leg must pass; SURFACE leg is what we are isolating) + // + // Expected: isCapabilityActive('graphify', cwd) === true + // (codex dir has graphify surfaced, so the codex surface should win) + // + // Pre-fix (hardcoded 'claude'): would read tmpClaudeDir → graphify NOT surfaced → false (BUG). + // Post-fix (detects 'codex' via GSD_RUNTIME): reads tmpCodexDir → surfaced → true (CORRECT). + + let tmpCodexDir; + let tmpClaudeDir; + let tmpProjectDir; + let prevGsdRuntime; + let prevCodexHome; + let prevClaudeConfigDir; + let prevGsdWorkstream; + let prevGsdProject; + + before(() => { + tmpCodexDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cap-state-codex-cfg-')); + tmpClaudeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cap-state-claude-cfg-')); + tmpProjectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cap-state-xrt-project-')); + + // Codex config dir: full profile + graphify SURFACED (disabledClusters empty) + fs.writeFileSync( + path.join(tmpCodexDir, '.gsd-surface.json'), + JSON.stringify({ baseProfile: 'full', disabledClusters: [], explicitAdds: [], explicitRemoves: [] }, null, 2) + '\n', + 'utf8', + ); + + // Claude config dir: full profile + graphify NOT surfaced + fs.writeFileSync( + path.join(tmpClaudeDir, '.gsd-surface.json'), + JSON.stringify({ baseProfile: 'full', disabledClusters: ['graphify'], explicitAdds: [], explicitRemoves: [] }, null, 2) + '\n', + 'utf8', + ); + + // Project: config with graphify.enabled=true so the config leg passes. + // The SURFACE leg (install+surfaced) is what we are testing — it must read + // the CODEX config dir (via CODEX_HOME), not the CLAUDE config dir. + fs.mkdirSync(path.join(tmpProjectDir, '.planning'), { recursive: true }); + fs.writeFileSync( + path.join(tmpProjectDir, '.planning', 'config.json'), + JSON.stringify({ graphify: { enabled: true } }), + 'utf8', + ); + }); + + after(() => { + cleanup(tmpCodexDir); + cleanup(tmpClaudeDir); + cleanup(tmpProjectDir); + }); + + test('GSD_RUNTIME=codex → resolver consults CODEX_HOME, not CLAUDE_CONFIG_DIR (HIGH cross-runtime fix)', () => { + prevGsdRuntime = process.env.GSD_RUNTIME; + prevCodexHome = process.env.CODEX_HOME; + prevClaudeConfigDir = process.env.CLAUDE_CONFIG_DIR; + prevGsdWorkstream = process.env.GSD_WORKSTREAM; + prevGsdProject = process.env.GSD_PROJECT; + + try { + process.env.GSD_RUNTIME = 'codex'; + process.env.CODEX_HOME = tmpCodexDir; // codex dir: graphify SURFACED + process.env.CLAUDE_CONFIG_DIR = tmpClaudeDir; // claude dir: graphify NOT surfaced + delete process.env.GSD_WORKSTREAM; + delete process.env.GSD_PROJECT; + + // With GSD_RUNTIME=codex, the resolver must look at CODEX_HOME (graphify surfaced) + // and return true. Pre-fix it would look at CLAUDE_CONFIG_DIR (not surfaced) → false. + const active = isCapabilityActive('graphify', tmpProjectDir); + assert.strictEqual( + active, + true, + 'isCapabilityActive must return true when GSD_RUNTIME=codex and graphify is surfaced in CODEX_HOME — ' + + 'the pre-fix code hardcoded getGlobalConfigDir("claude") regardless of GSD_RUNTIME, returning false (BUG)', + ); + + // Also verify: if we swap surfaces (codex NOT surfaced, claude surfaced), still uses codex dir → false + fs.writeFileSync( + path.join(tmpCodexDir, '.gsd-surface.json'), + JSON.stringify({ baseProfile: 'full', disabledClusters: ['graphify'], explicitAdds: [], explicitRemoves: [] }, null, 2) + '\n', + 'utf8', + ); + fs.writeFileSync( + path.join(tmpClaudeDir, '.gsd-surface.json'), + JSON.stringify({ baseProfile: 'full', disabledClusters: [], explicitAdds: [], explicitRemoves: [] }, null, 2) + '\n', + 'utf8', + ); + const activeSwapped = isCapabilityActive('graphify', tmpProjectDir); + assert.strictEqual( + activeSwapped, + false, + 'isCapabilityActive must return false when GSD_RUNTIME=codex and graphify is NOT surfaced in CODEX_HOME — ' + + 'even though claude dir has graphify surfaced, the codex dir is the authoritative surface', + ); + } finally { + // Restore env vars + if (prevGsdRuntime === undefined) delete process.env.GSD_RUNTIME; + else process.env.GSD_RUNTIME = prevGsdRuntime; + if (prevCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = prevCodexHome; + if (prevClaudeConfigDir === undefined) delete process.env.CLAUDE_CONFIG_DIR; + else process.env.CLAUDE_CONFIG_DIR = prevClaudeConfigDir; + if (prevGsdWorkstream === undefined) delete process.env.GSD_WORKSTREAM; + else process.env.GSD_WORKSTREAM = prevGsdWorkstream; + if (prevGsdProject === undefined) delete process.env.GSD_PROJECT; + else process.env.GSD_PROJECT = prevGsdProject; + + // Restore codex dir surface fixture for cleanup consistency + fs.writeFileSync( + path.join(tmpCodexDir, '.gsd-surface.json'), + JSON.stringify({ baseProfile: 'full', disabledClusters: [], explicitAdds: [], explicitRemoves: [] }, null, 2) + '\n', + 'utf8', + ); + } + }); +}); diff --git a/tests/graphify-query.test.cjs b/tests/graphify-query.test.cjs index 6de2a32bc..db5ea73e4 100644 --- a/tests/graphify-query.test.cjs +++ b/tests/graphify-query.test.cjs @@ -6,6 +6,7 @@ const { describe, test, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); +const os = require('os'); const path = require('path'); const { createTempProject, cleanup } = require('./helpers.cjs'); @@ -26,6 +27,55 @@ const { SAMPLE_GRAPH, } = require('./helpers/graphify.cjs'); +// ─── Shared fixture: surfaced-config-dir ───────────────────────────────────── +// +// Positive-path tests (graphifyQuery, graphifyDiff, graceful-degradation) call +// enableGraphify() (config leg only) and assert non-disabled outcomes. With +// the tri-state gate (isCapabilityActive), those outcomes ALSO require graphify +// to be installed+surfaced in the runtime config dir. Without this fixture the +// tests are ambient-dependent: they pass only on machines where the ambient +// ~/.claude has graphify surfaced. +// +// Fix: before each positive-path test, point CLAUDE_CONFIG_DIR at a tmp dir +// with a full-profile .gsd-surface.json (graphify surfaced), and clear +// GSD_RUNTIME / GSD_WORKSTREAM / GSD_PROJECT for hermeticity. + +/** Create a tmp config dir with graphify surfaced (full profile, no disabled clusters). */ +function makeSurfacedConfigDir() { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-graphify-qry-cfg-')); + fs.writeFileSync( + path.join(dir, '.gsd-surface.json'), + JSON.stringify({ baseProfile: 'full', disabledClusters: [], explicitAdds: [], explicitRemoves: [] }, null, 2) + '\n', + 'utf8', + ); + return dir; +} + +/** + * Save the env vars the surfaced-config fixture overrides. + * Returns an object whose .restore() returns env to its original state. + */ +function saveSurfacedEnv() { + const saved = { + GSD_RUNTIME: process.env.GSD_RUNTIME, + CLAUDE_CONFIG_DIR: process.env.CLAUDE_CONFIG_DIR, + GSD_WORKSTREAM: process.env.GSD_WORKSTREAM, + GSD_PROJECT: process.env.GSD_PROJECT, + }; + return { + restore() { + if (saved.GSD_RUNTIME === undefined) delete process.env.GSD_RUNTIME; + else process.env.GSD_RUNTIME = saved.GSD_RUNTIME; + if (saved.CLAUDE_CONFIG_DIR === undefined) delete process.env.CLAUDE_CONFIG_DIR; + else process.env.CLAUDE_CONFIG_DIR = saved.CLAUDE_CONFIG_DIR; + if (saved.GSD_WORKSTREAM === undefined) delete process.env.GSD_WORKSTREAM; + else process.env.GSD_WORKSTREAM = saved.GSD_WORKSTREAM; + if (saved.GSD_PROJECT === undefined) delete process.env.GSD_PROJECT; + else process.env.GSD_PROJECT = saved.GSD_PROJECT; + }, + }; +} + // ─── query describe ─────────────────────────────────────────────────────────── describe('query', () => { @@ -192,13 +242,25 @@ describe('query', () => { describe('graphifyQuery', () => { let tmpDir; let planningDir; + // Surfaced-config-dir fixture: makes positive-path tests deterministic. + // See module-level comment for rationale. + let surfacedConfigDir; + let savedEnv; beforeEach(() => { tmpDir = createTempProject(); planningDir = path.join(tmpDir, '.planning'); + surfacedConfigDir = makeSurfacedConfigDir(); + savedEnv = saveSurfacedEnv(); + delete process.env.GSD_RUNTIME; + process.env.CLAUDE_CONFIG_DIR = surfacedConfigDir; + delete process.env.GSD_WORKSTREAM; + delete process.env.GSD_PROJECT; }); afterEach(() => { + savedEnv.restore(); + cleanup(surfacedConfigDir); cleanup(tmpDir); }); @@ -259,13 +321,25 @@ describe('query', () => { describe('graphifyDiff', () => { let tmpDir; let planningDir; + // Surfaced-config-dir fixture: makes positive-path tests deterministic. + // See module-level comment for rationale. + let surfacedConfigDir; + let savedEnv; beforeEach(() => { tmpDir = createTempProject(); planningDir = path.join(tmpDir, '.planning'); + surfacedConfigDir = makeSurfacedConfigDir(); + savedEnv = saveSurfacedEnv(); + delete process.env.GSD_RUNTIME; + process.env.CLAUDE_CONFIG_DIR = surfacedConfigDir; + delete process.env.GSD_WORKSTREAM; + delete process.env.GSD_PROJECT; }); afterEach(() => { + savedEnv.restore(); + cleanup(surfacedConfigDir); cleanup(tmpDir); }); @@ -379,13 +453,25 @@ describe('query', () => { describe('graceful degradation (AGENT-03)', () => { let tmpDir; let planningDir; + // Surfaced-config-dir fixture: makes positive-path tests deterministic. + // See module-level comment for rationale. + let surfacedConfigDir; + let savedEnv; beforeEach(() => { tmpDir = createTempProject(); planningDir = path.join(tmpDir, '.planning'); + surfacedConfigDir = makeSurfacedConfigDir(); + savedEnv = saveSurfacedEnv(); + delete process.env.GSD_RUNTIME; + process.env.CLAUDE_CONFIG_DIR = surfacedConfigDir; + delete process.env.GSD_WORKSTREAM; + delete process.env.GSD_PROJECT; }); afterEach(() => { + savedEnv.restore(); + cleanup(surfacedConfigDir); cleanup(tmpDir); }); diff --git a/tests/graphify.test.cjs b/tests/graphify.test.cjs index 215c843b7..4d4104925 100644 --- a/tests/graphify.test.cjs +++ b/tests/graphify.test.cjs @@ -11,20 +11,21 @@ /** * Tests for gsd-core/bin/lib/graphify.cjs * - * Covers: config gate on/off (TEST-03), graceful degradation (TEST-04), - * subprocess helper (FOUND-04), presence detection (FOUND-02), - * version checking (FOUND-03), and disabled response (FOUND-01). + * Covers: tri-state gate (TEST-03 — isCapabilityActive cutover, Phase 3), + * graceful degradation (TEST-04), subprocess helper (FOUND-04), + * presence detection (FOUND-02), version checking (FOUND-03), + * and disabled response (FOUND-01). */ const { describe, test, beforeEach, afterEach, mock } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); +const os = require('os'); const path = require('path'); const childProcess = require('child_process'); -const { createTempProject, cleanup } = require('./helpers.cjs'); +const { createTempProject, createTempDir, cleanup } = require('./helpers.cjs'); const { - isGraphifyEnabled, disabledResponse, execGraphify, GRAPHIFY_REASON, @@ -42,10 +43,87 @@ const { SAMPLE_GRAPH, } = require('./helpers/graphify.cjs'); +// ─── Shared fixture: surfaced-config-dir ───────────────────────────────────── +// +// Positive-path tests (graphifyStatus + graphifyBuild) call enableGraphify() +// (config leg only) and assert non-disabled outcomes. With the tri-state gate +// (isCapabilityActive), a non-disabled outcome ALSO requires graphify to be +// installed+surfaced in the runtime config dir. Without this fixture those +// tests were ambient-dependent: they passed only on machines where graphify +// happened to be surfaced in the real ~/.claude. +// +// Fix: before each positive-path test, point CLAUDE_CONFIG_DIR at a tmp dir +// containing a full-profile .gsd-surface.json with graphify surfaced, and +// clear GSD_RUNTIME / GSD_WORKSTREAM / GSD_PROJECT for hermeticity. +// An EMPTY tmp config dir (no .gsd-surface.json) also works — the resolver +// defaults to 'full' profile → all surfaced — but we write the file explicitly +// so the fixture intent is visible and independent of default-resolution logic. + +/** Create a tmp config dir with graphify surfaced (full profile, no disabled clusters). */ +function makeSurfacedConfigDir() { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-graphify-surface-cfg-')); + fs.writeFileSync( + path.join(dir, '.gsd-surface.json'), + JSON.stringify({ baseProfile: 'full', disabledClusters: [], explicitAdds: [], explicitRemoves: [] }, null, 2) + '\n', + 'utf8', + ); + return dir; +} + +/** + * Save the env vars that the surfaced-config fixture overrides. + * Returns an object whose .restore() method returns the env to its original state. + */ +function saveSurfacedEnv() { + const saved = { + GSD_RUNTIME: process.env.GSD_RUNTIME, + CLAUDE_CONFIG_DIR: process.env.CLAUDE_CONFIG_DIR, + GSD_WORKSTREAM: process.env.GSD_WORKSTREAM, + GSD_PROJECT: process.env.GSD_PROJECT, + }; + return { + restore() { + if (saved.GSD_RUNTIME === undefined) delete process.env.GSD_RUNTIME; + else process.env.GSD_RUNTIME = saved.GSD_RUNTIME; + if (saved.CLAUDE_CONFIG_DIR === undefined) delete process.env.CLAUDE_CONFIG_DIR; + else process.env.CLAUDE_CONFIG_DIR = saved.CLAUDE_CONFIG_DIR; + if (saved.GSD_WORKSTREAM === undefined) delete process.env.GSD_WORKSTREAM; + else process.env.GSD_WORKSTREAM = saved.GSD_WORKSTREAM; + if (saved.GSD_PROJECT === undefined) delete process.env.GSD_PROJECT; + else process.env.GSD_PROJECT = saved.GSD_PROJECT; + }, + }; +} + // ─── status describe ───────────────────────────────────────────────────────── +// Require capability-state to assert gate parity in regression tests below. +const { isCapabilityActive } = require('../gsd-core/bin/lib/capability-state.cjs'); + describe('status', () => { - describe('isGraphifyEnabled', () => { + // ─── Tri-state gate (Phase 3 cutover from isGraphifyEnabled → isCapabilityActive) ── + // + // The old config-only gate (isGraphifyEnabled) checked ONLY graphify.enabled in + // config.json. The new tri-state gate (isCapabilityActive) requires the capability + // to be installed AND surfaced AND config-enabled. + // + // FAIL-FIRST PROOF (what would fail against the OLD isGraphifyEnabled code): + // Scenario: graphify installed+surfaced on the runtime, graphify.enabled=true in config. + // Old code: isGraphifyEnabled(planningDir) → true → status returns non-disabled. + // New code: isCapabilityActive('graphify', cwd) → depends on surface+install. + // The "gate-parity" test below would FAIL on old code because graphifyStatus used + // isGraphifyEnabled (config-only), which diverges from isCapabilityActive when + // the surface/install dimension differs from config. Specifically: + // - On a machine where graphify is NOT surfaced but config-enabled: + // OLD: isGraphifyEnabled=true → not disabled (BUG) + // NEW: isCapabilityActive=false → disabled (CORRECT) + // - The "returns disabled when graphify.enabled is false" test STILL PASSES under + // old code (config check is a subset of the new check) — it's the POSITIVE case + // that breaks. + // + // With the NEW gate (isCapabilityActive), graphify commands delegate entirely to + // isCapabilityActive, so graphifyStatus outcome === isCapabilityActive outcome. + describe('tri-state graphify gate (isCapabilityActive cutover)', () => { let tmpDir; let planningDir; @@ -58,44 +136,210 @@ describe('status', () => { cleanup(tmpDir); }); - test('returns false when no config.json exists', () => { - // Remove config.json if createTempProject wrote one - const configPath = path.join(planningDir, 'config.json'); - if (fs.existsSync(configPath)) fs.unlinkSync(configPath); - assert.strictEqual(isGraphifyEnabled(planningDir), false); + // REGRESSION (Phase 3): graphify gate outcome must exactly match isCapabilityActive. + // Old gate (isGraphifyEnabled) was config-only; new gate is tri-state. + // This test would fail on old code in any environment where isCapabilityActive + // disagrees with the config-only check (e.g., surfaced+installed but config-absent). + test('graphifyStatus gate outcome matches isCapabilityActive (regression — Phase 3)', () => { + // No graphify.enabled in config: isCapabilityActive resolves from install+surface. + const capabilityActive = isCapabilityActive('graphify', tmpDir); + const result = graphifyStatus(tmpDir); + if (capabilityActive) { + // Surface+install active → command must proceed (not disabled). + assert.ok( + !result.disabled, + 'graphifyStatus must not return disabled when isCapabilityActive=true', + ); + } else { + // Not active → command must return disabled. + assert.strictEqual( + result.disabled, + true, + 'graphifyStatus must return disabled when isCapabilityActive=false', + ); + } }); - test('returns false when graphify key is not set', () => { - fs.writeFileSync( - path.join(planningDir, 'config.json'), - JSON.stringify({ model_profile: 'balanced' }), - 'utf8' - ); - assert.strictEqual(isGraphifyEnabled(planningDir), false); - }); - - test('returns false when graphify.enabled is false', () => { + // Config-disabled → isCapabilityActive returns false → command disabled (preserved). + test('graphifyStatus returns disabled when graphify.enabled is false', () => { fs.writeFileSync( path.join(planningDir, 'config.json'), JSON.stringify({ graphify: { enabled: false } }), 'utf8' ); - assert.strictEqual(isGraphifyEnabled(planningDir), false); + const result = graphifyStatus(tmpDir); + assert.strictEqual(result.disabled, true, + 'graphifyStatus must return disabled when graphify.enabled=false'); }); - test('returns true when graphify.enabled is true', () => { + // Config-enabled and capability active → command proceeds (not disabled). + test('graphifyStatus is not disabled when config-enabled and isCapabilityActive=true', () => { enableGraphify(planningDir); - assert.strictEqual(isGraphifyEnabled(planningDir), true); + const capabilityActive = isCapabilityActive('graphify', tmpDir); + // Only assert when the capability is truly active in this environment. + // On a machine without graphify surfaced, isCapabilityActive=false even with + // config-enabled — this is the correct new behavior. + if (capabilityActive) { + const result = graphifyStatus(tmpDir); + assert.ok(!result.disabled, + 'graphifyStatus must not return disabled when graphify is fully active'); + } + // When capabilityActive=false (not surfaced), disabled is the correct outcome. + // No assertion needed — the regression test above covers it. }); - test('returns false when config.json is malformed', () => { - fs.writeFileSync( - path.join(planningDir, 'config.json'), - 'not json', - 'utf8' - ); - assert.strictEqual(isGraphifyEnabled(planningDir), false); + // ── TEST-03-HERMETIC ──────────────────────────────────────────────────────── + // + // FAIL-FIRST PROOF (what would fail against the OLD isGraphifyEnabled code): + // Scenario: graphify installed (full profile → '*' sentinel) + config-enabled=true, + // but graphify NOT surfaced (disabled cluster in .gsd-surface.json). + // + // OLD gate (isGraphifyEnabled): checks ONLY graphify.enabled in config.json. + // → graphify.enabled=true → isGraphifyEnabled=true → graphifyStatus NOT disabled → BUG. + // + // NEW gate (isCapabilityActive): requires installed AND surfaced AND config-enabled. + // → installed=true, surfaced=false → isCapabilityActive=false → graphifyStatus disabled → CORRECT. + // + // Fixture layout: + // CLAUDE_CONFIG_DIR → tmpConfigDir/ + // .gsd-surface.json → full profile, disabledClusters:["graphify"] + // (no .gsd-profile → defaults to 'full' → installedSkills='*' → installed=true) + // tmpProjectDir/ + // .planning/config.json → {"graphify":{"enabled":true}} + // + // The test is HERMETIC: it controls all three tri-state dimensions via fixture + // files and the CLAUDE_CONFIG_DIR env var, so the outcome is independent of + // any real ~/.claude configuration in the host environment. + describe('hermetic: graphify installed + config-enabled but NOT surfaced → disabled', () => { + let tmpConfigDir; + let tmpProjectDir; + let prevClaudeConfigDir; + let prevGsdWorkstream; + let prevGsdProject; + + beforeEach(() => { + tmpConfigDir = createTempDir('gsd-graphify-surface-test-config-'); + tmpProjectDir = createTempDir('gsd-graphify-surface-test-project-'); + + // Fixture: .gsd-surface.json — full profile with graphify cluster disabled. + // The 'graphify' key in disabledClusters maps to ["graphify"] via + // capability-registry.cjs capabilityClusters, removing the 'graphify' skill + // stem from the surfaced set. All other skills remain surfaced. + // No .gsd-profile written → readActiveProfile returns null → defaults to 'full' + // → resolveProfile returns '*' sentinel → installedSkills='*' → installed=true. + const surfaceState = { + baseProfile: 'full', + disabledClusters: ['graphify'], + explicitAdds: [], + explicitRemoves: [], + }; + fs.writeFileSync( + path.join(tmpConfigDir, '.gsd-surface.json'), + JSON.stringify(surfaceState, null, 2) + '\n', + 'utf8', + ); + + // Fixture: project config — graphify.enabled=true. + // This is the config dimension that the OLD gate (isGraphifyEnabled) would + // have returned true for, causing the BUG. The new gate ignores config when + // installed && surfaced is false. + const planningDirForFixture = path.join(tmpProjectDir, '.planning'); + fs.mkdirSync(planningDirForFixture, { recursive: true }); + fs.writeFileSync( + path.join(planningDirForFixture, 'config.json'), + JSON.stringify({ graphify: { enabled: true } }), + 'utf8', + ); + + // Save and override env vars for hermeticity. + // CLAUDE_CONFIG_DIR controls which config dir getGlobalConfigDir('claude') resolves. + prevClaudeConfigDir = process.env.CLAUDE_CONFIG_DIR; + prevGsdWorkstream = process.env.GSD_WORKSTREAM; + prevGsdProject = process.env.GSD_PROJECT; + process.env.CLAUDE_CONFIG_DIR = tmpConfigDir; + // Clear GSD_WORKSTREAM/GSD_PROJECT — ambient values redirect planningDir() + // causing STATE.md reads from an unrelated location (hermeticity regression #872). + delete process.env.GSD_WORKSTREAM; + delete process.env.GSD_PROJECT; + }); + + afterEach(() => { + // Restore env vars before cleanup so that cleanup() rmSync calls use + // the original env (no silent planningDir redirection from leftover vars). + if (prevClaudeConfigDir === undefined) delete process.env.CLAUDE_CONFIG_DIR; + else process.env.CLAUDE_CONFIG_DIR = prevClaudeConfigDir; + if (prevGsdWorkstream === undefined) delete process.env.GSD_WORKSTREAM; + else process.env.GSD_WORKSTREAM = prevGsdWorkstream; + if (prevGsdProject === undefined) delete process.env.GSD_PROJECT; + else process.env.GSD_PROJECT = prevGsdProject; + + cleanup(tmpConfigDir); + cleanup(tmpProjectDir); + }); + + // Negative case: installed + NOT surfaced + config-enabled → NOT active, command disabled. + // This is the KEY BUG-FIX branch. OLD isGraphifyEnabled would return true here (BUG). + // NEW isCapabilityActive returns false (CORRECT), forcing graphifyStatus to return disabled. + test('isCapabilityActive returns false when installed + NOT surfaced + config-enabled=true (TEST-03-HERMETIC)', () => { + const active = isCapabilityActive('graphify', tmpProjectDir); + assert.strictEqual( + active, + false, + 'isCapabilityActive must return false when graphify is installed and config-enabled but NOT surfaced — ' + + 'OLD isGraphifyEnabled would return true here (config-only check) which is the bug this test guards against', + ); + }); + + test('graphifyStatus returns disabled when installed + NOT surfaced + config-enabled=true (TEST-03-HERMETIC)', () => { + // Old isGraphifyEnabled(planningDir) → true (graphify.enabled=true in config) → NOT disabled (BUG). + // New isCapabilityActive('graphify', cwd) → false (not surfaced) → disabled (CORRECT). + const result = graphifyStatus(tmpProjectDir); + assert.strictEqual( + result.disabled, + true, + 'graphifyStatus must return disabled when graphify is installed and config-enabled but NOT surfaced — ' + + 'surface state must gate the command regardless of config-enabled value', + ); + assert.ok( + typeof result.message === 'string' && result.message.length > 0, + 'disabled response must include a non-empty message with enable instructions', + ); + }); + + // Positive control: same config-dir but now WITH graphify surfaced. + // This confirms the fixture itself is sound — the two tests above must see + // divergent outcomes from the same project config, controlled only by surface state. + test('graphifyStatus is NOT disabled when installed + SURFACED + config-enabled=true (positive control)', () => { + // Re-write .gsd-surface.json with graphify surfaced (disabledClusters empty). + const surfaceStateOn = { + baseProfile: 'full', + disabledClusters: [], + explicitAdds: [], + explicitRemoves: [], + }; + fs.writeFileSync( + path.join(tmpConfigDir, '.gsd-surface.json'), + JSON.stringify(surfaceStateOn, null, 2) + '\n', + 'utf8', + ); + + const active = isCapabilityActive('graphify', tmpProjectDir); + assert.strictEqual( + active, + true, + 'isCapabilityActive must return true when graphify is installed, surfaced, and config-enabled=true', + ); + + const result = graphifyStatus(tmpProjectDir); + assert.strictEqual( + result.disabled, + undefined, + 'graphifyStatus must NOT return disabled when graphify is installed, surfaced, and config-enabled=true — ' + + 'got disabled:' + JSON.stringify(result.disabled), + ); + }); }); + // ── end TEST-03-HERMETIC ──────────────────────────────────────────────────── }); describe('disabledResponse', () => { @@ -109,13 +353,28 @@ describe('status', () => { describe('graphifyStatus', () => { let tmpDir; let planningDir; + // Surfaced-config-dir fixture: makes positive-path tests deterministic by + // ensuring graphify is surfaced in the runtime config dir. Without this, + // tests depending on enableGraphify() pass only on machines where the + // ambient ~/.claude has graphify surfaced (ambient-dependent = flaky). + let surfacedConfigDir; + let savedEnv; beforeEach(() => { tmpDir = createTempProject(); planningDir = path.join(tmpDir, '.planning'); + // Set up hermetic surfaced config dir: graphify surfaced, runtime=claude. + surfacedConfigDir = makeSurfacedConfigDir(); + savedEnv = saveSurfacedEnv(); + delete process.env.GSD_RUNTIME; // use 'claude' default + process.env.CLAUDE_CONFIG_DIR = surfacedConfigDir; + delete process.env.GSD_WORKSTREAM; + delete process.env.GSD_PROJECT; }); afterEach(() => { + savedEnv.restore(); + cleanup(surfacedConfigDir); cleanup(tmpDir); }); @@ -451,14 +710,27 @@ describe('build', () => { describe('graphifyBuild', () => { let tmpDir; let planningDir; + // Surfaced-config-dir fixture: makes positive-path tests deterministic. + // See graphifyStatus describe block for the rationale. + let surfacedConfigDir; + let savedEnv; beforeEach(() => { tmpDir = createTempProject(); planningDir = path.join(tmpDir, '.planning'); enableGraphify(planningDir); + // Set up hermetic surfaced config dir: graphify surfaced, runtime=claude. + surfacedConfigDir = makeSurfacedConfigDir(); + savedEnv = saveSurfacedEnv(); + delete process.env.GSD_RUNTIME; + process.env.CLAUDE_CONFIG_DIR = surfacedConfigDir; + delete process.env.GSD_WORKSTREAM; + delete process.env.GSD_PROJECT; }); afterEach(() => { + savedEnv.restore(); + cleanup(surfacedConfigDir); cleanup(tmpDir); mock.restoreAll(); });