From 14e0709eed21368cb4ee6e932fbf7807b4e30986 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 16 Jun 2026 00:12:54 -0400 Subject: [PATCH] refactor(#1307): gate intel on isCapabilityActive + loop-resolver honors capability active (#1315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(#1307): gate intel on isCapabilityActive + loop-resolver honors capability active Part A: intel's command gate moves from config-only isIntelEnabled to the shared isCapabilityActive('intel', cwd) — a consistency cutover (intel has skills:[] so its tri-state collapses to the intel.enabled config leg; the gate now flows through the resolver's precedence + runtime-aware resolution). Part B: loop-resolver hook rendering now gates on capability state.active (=== true, fail-closed) instead of state.enabled, so the capability config gate is honored by the hook consumer, not just per-hook 'when'. active is now required in the loop-resolver input types. Regression test proves a config- disabled (active=false) capability's unconditional hook is not rendered. Part of #1302. Co-Authored-By: Claude Opus 4.8 (1M context) * chore(#1307): add changeset for intel + loop-resolver active gate Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .changeset/quick-yaks-march.md | 5 + CONTEXT.md | 3 + docs/USER-GUIDE.md | 10 + src/intel.cts | 62 +++-- src/loop-resolver.cts | 31 ++- tests/intel-command-cutover.test.cjs | 2 +- tests/intel.test.cjs | 274 ++++++++++++++++++++- tests/loop-hooks-empty-points-e2e.test.cjs | 10 +- tests/loop-hooks-verify-post-e2e.test.cjs | 24 +- tests/loop-render-hooks.test.cjs | 75 ++++++ 10 files changed, 431 insertions(+), 65 deletions(-) create mode 100644 .changeset/quick-yaks-march.md diff --git a/.changeset/quick-yaks-march.md b/.changeset/quick-yaks-march.md new file mode 100644 index 000000000..7d66f985e --- /dev/null +++ b/.changeset/quick-yaks-march.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 1315 +--- +**Intel and loop-hook rendering now honor the single capability `active` state** — `gsd-tools intel` gates through the shared resolver (consistency; intel stays governed by `intel.enabled`), and loop-hook rendering now suppresses a config-disabled capability's hooks via the capability-level `active` gate (fail-closed), not just per-hook `when`. (#1315) diff --git a/CONTEXT.md b/CONTEXT.md index 83e371250..4078f1bb2 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -223,6 +223,9 @@ The repo-root `gemini-extension.json` + `GEMINI.md` pair that projects gsd-core ### Knowledge Graph Module 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`. +### Intel Module +Module owning the code-intelligence store: tri-state capability gate (`isCapabilityActive('intel', cwd)` from capability-state.cjs — honours installed+surfaced+config-enabled; replaces the former config-only `isIntelEnabled` gate, cutover in #1307; intel has `skills:[]` so installed/surfaced are vacuously true and the effective gate is `intel.enabled` in config), disabled response, query surface (`intelQuery` — full-text search across all intel JSON files), status surface (`intelStatus` — per-file freshness, 24-hour staleness threshold), diff surface (`intelDiff` — added/changed/removed files vs last-refresh snapshot), snapshot management (`saveRefreshSnapshot`/`intelSnapshot`), validation (`intelValidate` — existence, JSON validity, _meta.updated_at recency), api-surface render (`intelApiSurface` — generates `.planning/intel/API-SURFACE.md` from `api-map.json`), plus ungated utilities (`intelPatchMeta` — patches `_meta.updated_at` in any JSON file; `intelExtractExports` — extracts CJS/ESM exports from any JS file). Loop hook rendering gates on `state.active` (not `state.enabled`) so the `activationKey` config gate is honoured even without a per-hook `when` guard (Phase 4 tri-state alignment, #1307). Source: `gsd-core/bin/lib/intel.cjs` (generated from `src/intel.cts`). Router: `gsd-core/bin/lib/intel-command-router.cjs`. See Capability Command Family Module (ADR-959 4d-impl-4) and Loop Extension Point. + ### 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 e76a8f4bb..b8ac37ef9 100644 --- a/docs/USER-GUIDE.md +++ b/docs/USER-GUIDE.md @@ -469,6 +469,16 @@ Graphify commands (`graphify status`, `graphify build`, `graphify query`, `graph 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. +### Intel capability gate (tri-state, v1.44+) + +Intel commands (`intel status`, `intel query`, `intel diff`, `intel snapshot`, `intel validate`, `intel api-surface`) now respect the **full tri-state capability gate** (same resolver as graphify above): + +1. **Installed** — the intel capability is present in the active install profile (intel has no skill files, so this is vacuously true for all profiles). +2. **Surfaced** — the intel capability is on the current runtime surface (vacuously true for all surfaces since intel registers no skill stems). +3. **Config-enabled** — `intel.enabled: true` is set in `.planning/config.json`. + +For intel, conditions 1 and 2 are always satisfied (intel has no skill files). The effective gate is `intel.enabled` in config — the same behaviour as before, but now enforced through the shared `isCapabilityActive('intel', cwd)` resolver rather than a direct config read. This means intel honours the full capability-state pipeline, including any future install-profile or surface restrictions. If intel commands return `{ disabled: true }`, ensure `intel.enabled: true` is set in `.planning/config.json` and verify `gsd-tools capability state` shows intel as active. + --- ## Usage Examples diff --git a/src/intel.cts b/src/intel.cts index 8b1925da1..e228fcbe1 100644 --- a/src/intel.cts +++ b/src/intel.cts @@ -5,7 +5,8 @@ * Intel files live in .planning/intel/ and store structured data about * the project's files, APIs, dependencies, architecture, and tech stack. * - * All public functions gate on intel.enabled config (no-op when false). + * All public functions gate on isCapabilityActive('intel', cwd) — the shared + * tri-state resolver (installed + surfaced + intel.enabled config key). * * ADR-457 build-at-publish: the hand-written bin/lib/intel.cjs collapsed * to a TypeScript source of truth. Behaviour is preserved byte-for-behaviour @@ -17,6 +18,10 @@ import path from 'node:path'; import crypto from 'node:crypto'; import { platformWriteSync, platformReadSync, platformEnsureDir } from './shell-command-projection.cjs'; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import capabilityStateMod = require('./capability-state.cjs'); +const { isCapabilityActive } = capabilityStateMod; + // ─── Constants ─────────────────────────────────────────────────────────────── const INTEL_DIR = '.planning/intel'; @@ -41,29 +46,20 @@ function ensureIntelDir(planningDir: string): string { } /** - * Check whether intel is enabled in the project config. - * Reads config.json directly via fs. Returns false by default - * (when no config, no intel key, or on error). + * Check whether intel is active (installed, surfaced, and config-enabled) for the project at cwd. + * Delegates to the shared tri-state capability resolver (isCapabilityActive) which honours the + * install profile, runtime surface, and activationKey (intel.enabled config gate). + * + * NOTE: planningDir is the legacy entry-point; cwd is derived as path.dirname(planningDir). + * Callers that have cwd directly may call isCapabilityActive('intel', cwd) themselves. + * + * INVARIANT: planningDir is always `/.planning` (i.e. path.join(cwd, '.planning')). + * The intel-command-router always constructs planningDir as path.join(cwd, '.planning'), + * so path.dirname(planningDir) === cwd is guaranteed. If a workstream-aware planningDir + * were ever passed here, the dirname would be wrong — but no caller does that. */ -function isIntelEnabled(planningDir: string): boolean { - try { - const configPath = path.join(planningDir, 'config.json'); - const raw = platformReadSync(configPath); - if (raw === null) return false; - const config: unknown = JSON.parse(raw); - if ( - config && - typeof config === 'object' && - 'intel' in config && - config.intel && - typeof config.intel === 'object' && - 'enabled' in config.intel && - (config.intel as Record).enabled === true - ) return true; - return false; - } catch { - return false; - } +function isIntelCapabilityActive(planningDir: string): boolean { + return isCapabilityActive('intel', path.dirname(planningDir)); } interface DisabledResponse { @@ -188,7 +184,7 @@ interface IntelQueryResult { * Searches across all JSON intel files in INTEL_FILES (keys and values), including arch-decisions.json (parsed as JSON, not as text). */ function intelQuery(term: string, planningDir: string): IntelQueryResult | DisabledResponse { - if (!isIntelEnabled(planningDir)) return disabledResponse(); + if (!isIntelCapabilityActive(planningDir)) return disabledResponse(); const matches: Array<{ source: string; entries: SearchMatch[] }> = []; let total = 0; @@ -225,7 +221,7 @@ interface IntelStatusResult { * A file is considered stale if its updated_at is older than 24 hours. */ function intelStatus(planningDir: string): IntelStatusResult | DisabledResponse { - if (!isIntelEnabled(planningDir)) return disabledResponse(); + if (!isIntelCapabilityActive(planningDir)) return disabledResponse(); const STALE_MS = 24 * 60 * 60 * 1000; // 24 hours const now = Date.now(); @@ -273,7 +269,7 @@ interface IntelDiffResult { * Show changes since the last full refresh by comparing file hashes. */ function intelDiff(planningDir: string): IntelDiffResult | { no_baseline: true } | DisabledResponse { - if (!isIntelEnabled(planningDir)) return disabledResponse(); + if (!isIntelCapabilityActive(planningDir)) return disabledResponse(); const snapshotPath = intelFilePath(planningDir, '.last-refresh.json'); const snapshot = safeReadJson(snapshotPath); @@ -309,7 +305,7 @@ function intelDiff(planningDir: string): IntelDiffResult | { no_baseline: true } * The actual update is performed by the intel-updater agent (PLAN-02). */ function intelUpdate(planningDir: string): { action: string; message: string } | DisabledResponse { - if (!isIntelEnabled(planningDir)) return disabledResponse(); + if (!isIntelCapabilityActive(planningDir)) return disabledResponse(); return { action: 'spawn_agent', @@ -359,7 +355,7 @@ function saveRefreshSnapshot(planningDir: string): SaveRefreshResult { * Writes .last-refresh.json with accurate timestamps and hashes. */ function intelSnapshot(planningDir: string): SaveRefreshResult | DisabledResponse { - if (!isIntelEnabled(planningDir)) return disabledResponse(); + if (!isIntelCapabilityActive(planningDir)) return disabledResponse(); return saveRefreshSnapshot(planningDir); } @@ -373,7 +369,7 @@ interface IntelValidateResult { * Validate all intel files for correctness and freshness. */ function intelValidate(planningDir: string): IntelValidateResult | DisabledResponse { - if (!isIntelEnabled(planningDir)) return disabledResponse(); + if (!isIntelCapabilityActive(planningDir)) return disabledResponse(); const errors: string[] = []; const warnings: string[] = []; @@ -470,7 +466,7 @@ interface IntelApiSurfaceResult { * mistake silence for "nothing exists". */ function intelApiSurface(planningDir: string): IntelApiSurfaceResult | DisabledResponse { - if (!isIntelEnabled(planningDir)) return disabledResponse(); + if (!isIntelCapabilityActive(planningDir)) return disabledResponse(); const intelPath = ensureIntelDir(planningDir); const apiMapPath = path.join(intelPath, INTEL_FILES.apis); @@ -535,7 +531,7 @@ interface IntelPatchMetaResult { * Patch _meta.updated_at in a JSON intel file to the current timestamp. * Reads the file, updates _meta.updated_at, increments version, writes back. * - * NOTE: Does not gate on isIntelEnabled — operates on arbitrary file paths + * NOTE: Does not gate on isCapabilityActive — operates on arbitrary file paths * for use by agents patching individual files outside the intel store. */ function intelPatchMeta(filePath: string): IntelPatchMetaResult { @@ -576,7 +572,7 @@ interface IntelExtractExportsResult { /** * Extract exports from a JS/CJS file by parsing module.exports or exports.X patterns. * - * NOTE: Does not gate on isIntelEnabled — operates on arbitrary source files + * NOTE: Does not gate on isCapabilityActive — operates on arbitrary source files * for use by agents building intel data from project files. */ function intelExtractExports(filePath: string): IntelExtractExportsResult { @@ -711,7 +707,7 @@ export = { // Utilities ensureIntelDir, - isIntelEnabled, + isIntelCapabilityActive, // Constants INTEL_FILES, diff --git a/src/loop-resolver.cts b/src/loop-resolver.cts index c18edcc26..a566f6c38 100644 --- a/src/loop-resolver.cts +++ b/src/loop-resolver.cts @@ -269,8 +269,16 @@ interface ResolveLoopHooksInput { config: Record; /** Optional cwd — enables raw config.json fallback reads (FIX 1 precedence level 2). */ cwd?: string; - /** Optional capability-state map; when present, disabled capabilities do not render hooks. */ - capabilityStatesById?: Map | Record; + /** + * Optional capability-state map; when present, inactive capabilities do not render hooks. + * Each entry carries both `enabled` (installed+surfaced) and `active` (enabled+configActivation). + * The resolver gates on `active` so that the config activation key (activationKey) is + * honoured even when no per-hook `when` guard is present (Phase 4 tri-state alignment). + * + * `active` is REQUIRED (not optional) so the gate is fail-closed: a missing or undefined + * `active` field is a compile error, never silently treated as truthy. + */ + capabilityStatesById?: Map | Record; } interface ResolveLoopHooksResult { @@ -330,13 +338,18 @@ function resolveLoopHooks(input: ResolveLoopHooksInput): ResolveLoopHooksResult return _resolveActivationValue(when, config, cwd, registry); } - function isCapabilityEnabled(capId: string): boolean { + function isCapabilityActive(capId: string): boolean { if (!capabilityStatesById) return true; const state = capabilityStatesById instanceof Map ? capabilityStatesById.get(capId) : capabilityStatesById[capId]; if (!state) return false; - return state.enabled !== false; + // Fail-closed gate: only render the hook when active is explicitly true. + // A capability can be installed and surfaced (enabled=true) but config-disabled + // (active=false); in that case the hook must not render. + // Phase 4 tri-state alignment: `active` is now required (not optional), so + // `=== true` is the correct fail-closed check (not `!== false`). + return state.active === true; } // Helper: safe string array @@ -401,7 +414,7 @@ function resolveLoopHooks(input: ResolveLoopHooksInput): ResolveLoopHooksResult for (const hook of steps) { if (!hook || typeof hook !== 'object') continue; const capId = typeof hook['capId'] === 'string' ? hook['capId'] : ''; - if (!isCapabilityEnabled(capId)) continue; + if (!isCapabilityActive(capId)) continue; if (!isActive(hook)) continue; const ref = (typeof hook['ref'] === 'object' && hook['ref'] !== null) ? (hook['ref'] as HookRef) @@ -427,7 +440,7 @@ function resolveLoopHooks(input: ResolveLoopHooksInput): ResolveLoopHooksResult for (const hook of contributions) { if (!hook || typeof hook !== 'object') continue; const capId = typeof hook['capId'] === 'string' ? hook['capId'] : ''; - if (!isCapabilityEnabled(capId)) continue; + if (!isCapabilityActive(capId)) continue; if (!isActive(hook)) continue; const into = typeof hook['into'] === 'string' ? hook['into'] : undefined; const fragment = toFragment(hook['fragment']); @@ -453,7 +466,7 @@ function resolveLoopHooks(input: ResolveLoopHooksInput): ResolveLoopHooksResult for (const hook of gates) { if (!hook || typeof hook !== 'object') continue; const capId = typeof hook['capId'] === 'string' ? hook['capId'] : ''; - if (!isCapabilityEnabled(capId)) continue; + if (!isCapabilityActive(capId)) continue; if (!isActive(hook)) continue; const when = typeof hook['when'] === 'string' ? hook['when'] : undefined; const check = hook['check'] !== undefined ? hook['check'] : undefined; @@ -613,11 +626,11 @@ function cmdLoopRenderHooks( warnings?: string[]; registry: Record; config: Record; - capabilities: Array<{ id: string; enabled?: boolean }>; + capabilities: Array<{ id: string; enabled?: boolean; active: boolean }>; }; const registry = state.registry; const config = state.config || loadConfig(cwd); - const capabilityStatesById = new Map(); + const capabilityStatesById = new Map(); for (const cap of state.capabilities || []) { capabilityStatesById.set(cap.id, cap); } diff --git a/tests/intel-command-cutover.test.cjs b/tests/intel-command-cutover.test.cjs index bcadd25bd..eb16e241d 100644 --- a/tests/intel-command-cutover.test.cjs +++ b/tests/intel-command-cutover.test.cjs @@ -100,7 +100,7 @@ function enableIntel(tmpDir) { const config = fs.existsSync(configPath) ? JSON.parse(fs.readFileSync(configPath, 'utf8')) : {}; - // isIntelEnabled() requires the NESTED form { intel: { enabled: true } }. + // isIntelCapabilityActive() / isCapabilityActive('intel', cwd) requires the NESTED form { intel: { enabled: true } }. // A flat dotted key like config['intel.enabled'] = true is NOT recognised. config.intel = { ...(config.intel ?? {}), enabled: true }; fs.writeFileSync(configPath, JSON.stringify(config, null, 2), 'utf8'); diff --git a/tests/intel.test.cjs b/tests/intel.test.cjs index 4cf54ebdb..104769332 100644 --- a/tests/intel.test.cjs +++ b/tests/intel.test.cjs @@ -12,7 +12,7 @@ const { describe, test, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); -const { createTempProject, cleanup, runGsdTools } = require('./helpers.cjs'); +const { createTempProject, createTempDir, cleanup, runGsdTools } = require('./helpers.cjs'); const { intelQuery, @@ -24,10 +24,12 @@ const { intelExtractExports, intelApiSurface, ensureIntelDir, - isIntelEnabled, + isIntelCapabilityActive, INTEL_FILES, } = require('../gsd-core/bin/lib/intel.cjs'); +const { isCapabilityActive } = require('../gsd-core/bin/lib/capability-state.cjs'); + // ─── Helpers ──────────────────────────────────────────────────────────────── function enableIntel(planningDir) { @@ -55,6 +57,51 @@ function _writeIntelMd(planningDir, filename, content) { fs.writeFileSync(path.join(intelPath, filename), content, 'utf8'); } +// ─── Surfaced-config-dir fixture ────────────────────────────────────────────── +// +// Positive-path tests (intelQuery, intelStatus, etc.) call isCapabilityActive +// via the tri-state gate — they need the capability to be surfaced. +// Without this fixture those tests are ambient-dependent (pass only on machines +// where intel is surfaced in the real ~/.claude). +// +// Fix: point CLAUDE_CONFIG_DIR at a tmp dir containing a full-profile +// .gsd-surface.json (no disabled clusters) so intel is surfaced deterministically. +// An EMPTY tmp config dir also works (defaults to 'full' profile → all surfaced) +// but we write the file explicitly for visible intent. + +/** Create a tmp config dir with intel (and all caps) surfaced — full profile. */ +function makeSurfacedConfigDir() { + const dir = createTempDir('gsd-intel-surface-cfg-'); + fs.writeFileSync( + path.join(dir, '.gsd-surface.json'), + JSON.stringify({ baseProfile: 'full', disabledClusters: [], explicitAdds: [], explicitRemoves: [] }, null, 2) + '\n', + 'utf8', + ); + return dir; +} + +/** Save env vars touched by the surfaced-config fixture; returns .restore(). */ +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; + }, + }; +} + // ─── Disabled gating ──────────────────────────────────────────────────────── describe('intel disabled gating', () => { @@ -70,22 +117,44 @@ describe('intel disabled gating', () => { cleanup(tmpDir); }); - test('isIntelEnabled returns false when no config.json exists', () => { - assert.strictEqual(isIntelEnabled(planningDir), false); + // isIntelEnabled was removed in Phase 4 (tri-state cutover). These tests now + // verify the gating via the public command API (intelQuery) and the exported + // isIntelCapabilityActive helper which delegates to isCapabilityActive. + test('isIntelCapabilityActive returns false when no config.json exists', () => { + // No CLAUDE_CONFIG_DIR → defaults to real ~/.claude; no intel.enabled in config. + // In a hermetic test environment (empty tmpDir), the capability is not surfaced + // via the real config dir — but isCapabilityActive returns false by construction + // when the activationKey (intel.enabled) is absent/false regardless of surface, + // because: active = enabled && configActivation; configActivation=false when key absent. + // This test is surface-agnostic for the "no config" branch. + assert.strictEqual(isIntelCapabilityActive(planningDir), false); }); - test('isIntelEnabled returns false when intel.enabled is not set', () => { + test('isIntelCapabilityActive returns false when intel.enabled is not set', () => { fs.writeFileSync( path.join(planningDir, 'config.json'), JSON.stringify({ model_profile: 'balanced' }), 'utf8' ); - assert.strictEqual(isIntelEnabled(planningDir), false); + assert.strictEqual(isIntelCapabilityActive(planningDir), false); }); - test('isIntelEnabled returns true when intel.enabled is true', () => { + // NOTE: intel has `skills: []` (empty), so installed and surfaced are vacuously true. + // For intel, active = configActivation (the intel.enabled config key). + // This test verifies that isIntelCapabilityActive delegates to isCapabilityActive('intel', cwd) + // and that the function returns a boolean without throwing, regardless of the ambient + // CLAUDE_CONFIG_DIR. The config dimension is probed hermetically in the section below. + test('isIntelCapabilityActive delegates to isCapabilityActive and returns a boolean (config=true, ambient surface)', () => { + // Write config.intel.enabled=true (config dimension ON). Since intel has no skills, + // surface is vacuously true — so this call returns true when the config is written + // and the capability system resolves correctly. enableIntel(planningDir); - assert.strictEqual(isIntelEnabled(planningDir), true); + // We cannot assert the exact value without full hermetic control of all three + // tri-state dimensions (see hermetic section below for that), but we assert that: + // 1. isIntelCapabilityActive delegates correctly (does not throw) + // 2. it returns a boolean (not undefined/null/object) + const result = isIntelCapabilityActive(planningDir); + assert.strictEqual(typeof result, 'boolean', 'isIntelCapabilityActive must return a boolean'); }); test('intelQuery returns disabled response when intel is off', () => { @@ -110,6 +179,121 @@ describe('intel disabled gating', () => { }); }); +// ─── Tri-state gate hermetic regression tests ──────────────────────────────── +// +// Intel capability has `skills: []` (empty) — so `installed` and `surfaced` are +// VACUOUSLY TRUE. For intel, `active = configActivation` where configActivation +// resolves the `activationKey` ("intel.enabled") via the config. This is a meaningful +// tri-state improvement because the old `isIntelEnabled` read config.json directly +// (synchronous file read, not wired through `loadConfig`), while the new gate goes +// through the full `resolveCapabilityRuntimeState` path. +// +// FAIL-FIRST PROOF (what would fail against the OLD isIntelEnabled code): +// Scenario: intel installed (skills=[]) + config has intel.enabled=true, BUT the +// surface has intel NOT surfaced via disabledClusters. +// +// However: intel has skills:[], so disabledClusters:['intel'] has no effect +// (no skills to remove from the surfaced set). Intel is always vacuously surfaced. +// +// The CORRECT regression for intel's tri-state cutover is: +// intel.enabled=true in config → isCapabilityActive=true → command NOT disabled +// intel.enabled=false (or absent) → isCapabilityActive=false → command disabled +// AND that the gate now goes through the shared resolver (not a direct config read). +// +// FAIL-FIRST SCENARIO: OLD isIntelEnabled read config.json at the planningDir path +// via platformReadSync. NEW isCapabilityActive uses resolveCapabilityRuntimeState +// which goes through loadConfig (multi-layer resolution). A test that sets +// intel.enabled=true in config then calls intelStatus would: +// OLD: isIntelEnabled → reads .planning/config.json → true → NOT disabled. +// NEW: isCapabilityActive → resolveCapabilityRuntimeState → configActivation=true → active=true → NOT disabled. +// Both return the same, so the regression test focuses on the config-absent/false case +// where the gate correctly returns disabled (proving the delegation path works). + +describe('intel tri-state gate hermetic regression (isCapabilityActive cutover)', () => { + let tmpConfigDir; + let tmpProjectDir; + let prevClaudeConfigDir; + let prevGsdWorkstream; + let prevGsdProject; + + beforeEach(() => { + tmpConfigDir = createTempDir('gsd-intel-tristate-cfg-'); + tmpProjectDir = createTempProject('gsd-intel-tristate-proj-'); + + prevClaudeConfigDir = process.env.CLAUDE_CONFIG_DIR; + prevGsdWorkstream = process.env.GSD_WORKSTREAM; + prevGsdProject = process.env.GSD_PROJECT; + // Empty CLAUDE_CONFIG_DIR (no .gsd-surface.json) → defaults to 'full' profile. + // Intel has skills:[] so it is vacuously installed+surfaced+enabled. + // Active = configActivation = intel.enabled in config. + process.env.CLAUDE_CONFIG_DIR = tmpConfigDir; + delete process.env.GSD_WORKSTREAM; + delete process.env.GSD_PROJECT; + }); + + afterEach(() => { + 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: config has NO intel.enabled (absent → defaults to false via activationKey). + // OLD gate: isIntelEnabled reads config.json → no intel key → returns false → disabled. + // NEW gate: isCapabilityActive → configActivation=false (intel.enabled default=false) → active=false → disabled. + // Both return disabled. The test PROVES the gate is wired through isCapabilityActive and + // the command intelStatus returns disabled — regression guard against losing the delegation. + test('intelStatus returns disabled when intel.enabled is absent in config (hermetic tristate negative)', () => { + // No config.json in planningDir — intel.enabled defaults to false. + // OLD isIntelEnabled: reads .planning/config.json → not found → false → disabled. + // NEW isCapabilityActive: intel.enabled default=false → configActivation=false → active=false → disabled. + const planningDir = path.join(tmpProjectDir, '.planning'); + const result = intelStatus(planningDir); + assert.strictEqual( + result.disabled, + true, + 'intelStatus must return disabled when intel.enabled is not set — ' + + 'both old and new gate must return disabled here; this is the regression guard for the delegation path', + ); + assert.ok( + typeof result.message === 'string' && result.message.length > 0, + 'disabled response must include a non-empty message', + ); + }); + + // POSITIVE CONTROL: intel.enabled=true in config → isCapabilityActive=true → NOT disabled. + // This is the primary pass case that proves the NEW gate honours config-enabled. + // OLD gate (isIntelEnabled) returns true. NEW gate (isCapabilityActive) also returns true. + // The test confirms the behaviour is preserved after cutover. + test('intelStatus NOT disabled when intel.enabled=true in config (hermetic tristate positive control)', () => { + const planningDir = path.join(tmpProjectDir, '.planning'); + fs.mkdirSync(planningDir, { recursive: true }); + fs.writeFileSync( + path.join(planningDir, 'config.json'), + JSON.stringify({ intel: { enabled: true } }), + 'utf8', + ); + + const active = isCapabilityActive('intel', tmpProjectDir); + assert.strictEqual( + active, + true, + 'isCapabilityActive must return true when intel.enabled=true and intel is vacuously installed+surfaced', + ); + + const result = intelStatus(planningDir); + assert.ok( + !result.disabled, + 'intelStatus must NOT return disabled when intel.enabled=true (positive control)', + ); + }); +}); + // ─── ensureIntelDir ───────────────────────────────────────────────────────── describe('ensureIntelDir', () => { @@ -143,14 +327,26 @@ describe('ensureIntelDir', () => { describe('intelQuery', () => { let tmpDir; let planningDir; + let surfacedConfigDir; + let savedEnv; beforeEach(() => { tmpDir = createTempProject(); planningDir = path.join(tmpDir, '.planning'); enableIntel(planningDir); + // Harden: ensure intel is surfaced (tri-state gate requires install+surface+config). + // Empty CLAUDE_CONFIG_DIR defaults to 'full' profile → all caps surfaced. + 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); }); @@ -233,14 +429,24 @@ describe('intelQuery', () => { describe('intelStatus', () => { let tmpDir; let planningDir; + let surfacedConfigDir; + let savedEnv; beforeEach(() => { tmpDir = createTempProject(); planningDir = path.join(tmpDir, '.planning'); enableIntel(planningDir); + 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); }); @@ -280,14 +486,24 @@ describe('intelStatus', () => { describe('intelDiff', () => { let tmpDir; let planningDir; + let surfacedConfigDir; + let savedEnv; beforeEach(() => { tmpDir = createTempProject(); planningDir = path.join(tmpDir, '.planning'); enableIntel(planningDir); + 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); }); @@ -332,14 +548,24 @@ describe('intelDiff', () => { describe('intelSnapshot', () => { let tmpDir; let planningDir; + let surfacedConfigDir; + let savedEnv; beforeEach(() => { tmpDir = createTempProject(); planningDir = path.join(tmpDir, '.planning'); enableIntel(planningDir); + 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); }); @@ -363,14 +589,24 @@ describe('intelSnapshot', () => { describe('intelValidate', () => { let tmpDir; let planningDir; + let surfacedConfigDir; + let savedEnv; beforeEach(() => { tmpDir = createTempProject(); planningDir = path.join(tmpDir, '.planning'); enableIntel(planningDir); + 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); }); @@ -665,12 +901,24 @@ describe('intelExtractExports', () => { describe('gsd-tools intel subcommands', () => { let tmpDir; + let surfacedConfigDir; + let savedEnv; beforeEach(() => { tmpDir = createTempProject(); + // Set up surfaced config dir for positive-path CLI tests (subprocess inherits env). + // Negative-path tests (disabled) still work because intel.enabled is not set by default. + 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); }); @@ -753,13 +1001,23 @@ describe('gsd-tools intel subcommands', () => { describe('intelApiSurface', () => { let tmpDir; let planningDir; + 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/loop-hooks-empty-points-e2e.test.cjs b/tests/loop-hooks-empty-points-e2e.test.cjs index 78530a676..7099c999b 100644 --- a/tests/loop-hooks-empty-points-e2e.test.cjs +++ b/tests/loop-hooks-empty-points-e2e.test.cjs @@ -237,7 +237,7 @@ describe('discuss:post — E2E empty envelope + synthetic resolver mechanics', ( assert.strictEqual(resolvedB.activeHooks.length, 0, 'explicit config=false must override schema default=true'); }); - it('[bva] discuss:post with synthetic capability, capabilityStatesById enabled=false → hook absent even when config=true', () => { + it('[bva] discuss:post with synthetic capability, capabilityStatesById active=false → hook absent even when config=true', () => { const reg = buildSyntheticRegistry({ targetPoint: 'discuss:post', when: 'workflow.testcap_on', @@ -247,9 +247,10 @@ describe('discuss:post — E2E empty envelope + synthetic resolver mechanics', ( point: 'discuss:post', registry: reg, config: { workflow: { testcap_on: true } }, - capabilityStatesById: new Map([['future-cap', { enabled: false }]]), + // Phase 4: resolver gates on `active` (not `enabled`); pass active:false to suppress. + capabilityStatesById: new Map([['future-cap', { enabled: false, active: false }]]), }); - assert.strictEqual(resolved.activeHooks.length, 0, 'capabilityStatesById enabled=false must suppress hook even when config=true'); + assert.strictEqual(resolved.activeHooks.length, 0, 'capabilityStatesById active=false must suppress hook even when config=true'); }); it('[negative] discuss:post with invalid point name "discuss:past" exits non-zero and lists valid points', () => { @@ -372,7 +373,8 @@ describe('execute:pre — real registry empty-resolution + synthetic resolver me point: 'execute:pre', registry: syntheticReg, config: {}, - capabilityStatesById: new Map([['future-cap', { enabled: false }]]), + // Phase 4: resolver gates on `active` (not `enabled`); pass active:false to suppress. + capabilityStatesById: new Map([['future-cap', { enabled: false, active: false }]]), }); assert.strictEqual(resolved.activeHooks.length, 0, 'capabilityStatesById disabled must filter unconditional hook'); }); diff --git a/tests/loop-hooks-verify-post-e2e.test.cjs b/tests/loop-hooks-verify-post-e2e.test.cjs index 4f01d0e58..c4c5fa4c8 100644 --- a/tests/loop-hooks-verify-post-e2e.test.cjs +++ b/tests/loop-hooks-verify-post-e2e.test.cjs @@ -315,11 +315,15 @@ describe('verify:post — per-key BVA: each false excludes only that single step // ─── 5. Surface-disable via capabilityStatesById (pure resolver) ────────────── describe('verify:post — surface-disable: capabilityStatesById filters hooks', () => { - test('[negative] ui disabled via capabilityStatesById→enabled:false excludes ui step; nyquist+security remain', () => { + // Phase 4 note: the resolver now gates on `active` (not `enabled`), so + // capabilityStatesById entries must carry active:false to suppress a hook. + // Real CapabilityStateEntry objects from resolveCapabilityRuntimeState carry both + // enabled and active; fixtures here mirror that shape. + test('[negative] ui disabled via capabilityStatesById→active:false excludes ui step; nyquist+security remain', () => { const capabilityStatesById = new Map([ - ['nyquist', { enabled: true }], - ['security', { enabled: true }], - ['ui', { enabled: false }], + ['nyquist', { enabled: true, active: true }], + ['security', { enabled: true, active: true }], + ['ui', { enabled: false, active: false }], ]); const resolved = resolveLoopHooks({ point: 'verify:post', @@ -340,9 +344,9 @@ describe('verify:post — surface-disable: capabilityStatesById filters hooks', test('[negative] security disabled via capabilityStatesById excludes security step; nyquist+ui remain', () => { const capabilityStatesById = new Map([ - ['nyquist', { enabled: true }], - ['security', { enabled: false }], - ['ui', { enabled: true }], + ['nyquist', { enabled: true, active: true }], + ['security', { enabled: false, active: false }], + ['ui', { enabled: true, active: true }], ]); const resolved = resolveLoopHooks({ point: 'verify:post', @@ -363,9 +367,9 @@ describe('verify:post — surface-disable: capabilityStatesById filters hooks', test('[empty-resolution] all three disabled via capabilityStatesById returns empty activeHooks with valid envelope', () => { const capabilityStatesById = new Map([ - ['nyquist', { enabled: false }], - ['security', { enabled: false }], - ['ui', { enabled: false }], + ['nyquist', { enabled: false, active: false }], + ['security', { enabled: false, active: false }], + ['ui', { enabled: false, active: false }], ]); const resolved = resolveLoopHooks({ point: 'verify:post', diff --git a/tests/loop-render-hooks.test.cjs b/tests/loop-render-hooks.test.cjs index 7f6bba218..782949868 100644 --- a/tests/loop-render-hooks.test.cjs +++ b/tests/loop-render-hooks.test.cjs @@ -963,3 +963,78 @@ describe('--active-cap flag (loop render-hooks)', () => { ); }); }); + +// ─── Phase 4 regression: loop-resolver gates on state.active (not state.enabled) ───── +// +// FAIL-FIRST PROOF (what would fail against the OLD state.enabled check): +// Scenario: capability has activationKey set; it is installed + surfaced (enabled=true) +// but the activationKey resolves to false (active=false). The capability has a hook +// WITHOUT a `when` guard (unconditional). With old state.enabled check: +// state.enabled=true → state.enabled !== false → true → hook IS rendered (BUG). +// With new state.active check: +// state.active=false → state.active === true → false → hook NOT rendered (CORRECT). +// +// This test uses resolveLoopHooks directly with a synthetic registry and a +// capabilityStatesById map that models the above scenario: enabled=true, active=false. +// It would FAIL against the OLD `state.enabled !== false` code and PASS against the +// NEW `state.active === true` code. + +describe('Phase 4 regression: capabilityStatesById gates on active (not enabled) for config-disabled capability', () => { + test('[regression] enabled=true active=false + no `when` guard → hook NOT rendered (state.active gate)', () => { + // Fail-first: with OLD `state.enabled !== false`, enabled=true → hook IS rendered (BUG). + // With NEW `state.active === true`, active=false → hook NOT rendered (CORRECT). + const registry = makeRegistry({ + point: 'plan:pre', + steps: [{ capId: 'test-cap', ref: { skill: 'gsd-test-skill' } }], + // No `when` → unconditional hook (no per-hook config gate to fall back on) + }); + + const capabilityStatesById = new Map([ + // enabled=true (installed+surfaced), active=false (activationKey resolved to false) + // This models a capability like intel/graphify that is surfaced but config-disabled. + ['test-cap', { enabled: true, active: false }], + ]); + + const result = resolveLoopHooks({ + point: 'plan:pre', + registry, + config: {}, // config doesn't matter — capability already resolved active=false + capabilityStatesById, + }); + + assert.strictEqual( + result.activeHooks.length, + 0, + 'Hook must NOT be rendered when state.active=false, even if enabled=true and no `when` guard — ' + + 'OLD state.enabled check would include this hook (BUG: enabled=true passes enabled!==false); ' + + 'NEW state.active check correctly suppresses it (active=false fails active===true)', + ); + }); + + test('[positive control] enabled=true active=true + no `when` guard → hook IS rendered', () => { + // Confirms the fixture is sound: same hook, same registry, but active=true → rendered. + // This test must PASS against BOTH old and new code (it's the unbroken branch). + const registry = makeRegistry({ + point: 'plan:pre', + steps: [{ capId: 'test-cap', ref: { skill: 'gsd-test-skill' } }], + }); + + const capabilityStatesById = new Map([ + ['test-cap', { enabled: true, active: true }], + ]); + + const result = resolveLoopHooks({ + point: 'plan:pre', + registry, + config: {}, + capabilityStatesById, + }); + + assert.strictEqual( + result.activeHooks.length, + 1, + 'Hook MUST be rendered when state.active=true and no `when` guard (positive control)', + ); + assert.strictEqual(result.activeHooks[0].capId, 'test-cap'); + }); +});