* refactor(#1306): gate graphify on isCapabilityActive (tri-state), not config-only graphify's command gate moves from the config-only isGraphifyEnabled to the shared isCapabilityActive('graphify', cwd) — so graphify is off unless installed AND surfaced AND graphify.enabled. Fixes a latent claude-hardcoding in the resolver: resolveCapabilityRuntimeState now detects the active runtime via resolveRuntime(cwd) (GSD_RUNTIME -> config.runtime -> 'claude') so non-Claude runtimes (Codex/Cursor) read their own surface, not ~/.claude. Hermetic regression test proves config-on+unsurfaced -> disabled; cross-runtime test proves GSD_RUNTIME=codex honors CODEX_HOME. Gate fails closed on every error path. Part of #1302. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(#1306): add changeset for graphify tri-state gate Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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();
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user