From 3a5325524420855a6a21651818176b1c3fc899c7 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 16 Jun 2026 02:38:47 -0400 Subject: [PATCH] refactor(#1308): consolidate config-key precedence engine into single owner (#1322) Make src/capability-activation.cts the sole owner of the four-level config-key precedence walk via a new raw-value primitive resolveConfigKey(dotKey,{config, cwd,registry}); the boolean wrapper _resolveActivationValue and loop-resolver's resolveConfigValues are both rebuilt on it. loop-resolver deletes its byte-identical copy and inline re-walk and imports the engine. resolveCapabilityRuntimeState no longer returns registry/config (leaked internal detail); the caller loads one fail-closed config snapshot and threads it in via a new optional configOverride param, so capability `active` and hook when/configValues resolve against the same object. capability-writer requires the registry module directly. Adds a DEFECT.GENERATIVE-FIX parity gate (identity + behavioral matrix + end-to-end resolveLoopHooks configValues) that fails if the two precedence surfaces ever diverge. Pure internal refactor, no user-facing change. Closes #1308 Co-authored-by: Claude Opus 4.8 (1M context) --- CONTEXT.md | 4 +- src/capability-activation.cts | 78 ++- src/capability-state.cts | 20 +- src/capability-writer.cts | 3 +- src/loop-resolver.cts | 188 +----- tests/capability-precedence-parity.test.cjs | 596 ++++++++++++++++++++ tests/capability-state.test.cjs | 130 +++++ 7 files changed, 826 insertions(+), 193 deletions(-) create mode 100644 tests/capability-precedence-parity.test.cjs diff --git a/CONTEXT.md b/CONTEXT.md index 4078f1bb2..fbb349075 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -170,7 +170,7 @@ ADR-857 phase 3b seam that merges capability-declared config slices into the `lo A named, stable site on a host loop step (per-step `pre`/`post` plus per-wave in Execute; 12 total) where Capabilities register hooks. Three hook kinds: `step` (runs as its own sequenced unit), `contribution` (injects into the core step's prompt/context), and `gate` (checks and optionally blocks via a declared `blocking` flag). Each hook declares the artifacts it produces and consumes; hook order is derived by topological sort of that produces/consumes graph (capability-id tiebreak), which also defines data flow — file-artifact based, surviving `/clear` and fresh executor contexts. Hooks are surfaced by runtime resolution with concrete projection: the workflow calls a query that resolves the active hooks and returns fully-rendered, ordered markdown for the executor. Failure is default-resilient — a non-gate hook that errors is skipped with a warning; a hook may opt into `onError: halt`. Part of the Capability system. ADR-857 phase 3c ships the registry-consuming query layer: `gsd-core/bin/lib/loop-resolver.cjs` exposes `resolveLoopHooks({ point, registry, config })` (pure, no I/O), `renderLoopHooks(resolved)` (pure markdown renderer), and `cmdLoopRenderHooks(cwd, point, raw, opts)` (I/O entry point); activated via `gsd-tools loop render-hooks ` which emits `{ point, activeHooks[], rendered }`. Activation is driven by `when` (dotted config key resolved against `loadConfig`), with inline literal `__proto__`/`constructor`/`prototype` prototype-pollution guard. The first phase-6 cutovers wiring workflows to this query have landed — ui-phase at `plan:pre` and ui-review at `verify:post` (in `plan-phase.md`/`autonomous.md`); further per-feature cutovers are ongoing. ### Capability State Resolver -ADR-857 phase 4b/6 unified resolver that composes the three toggle systems (install profile, runtime surface, config activation) into one per-capability view consumed by workflow hook rendering. Source of truth: `gsd-core/bin/lib/capability-state.cjs` (generated from `src/capability-state.cts`). Interface: `resolveCapabilityState({ registry, installedSkills, surfacedSkills, config, cwd? }) → { capabilities: CapabilityStateEntry[] }` (pure, no I/O); `resolveCapabilityRuntimeState(cwd, runtimeConfigDir)` (I/O resolver shared by workflow dispatch and diagnostics); `cmdCapabilityState(cwd, runtimeConfigDir, raw, opts)` (I/O output entry point); `isCapabilityActive(capId, cwd): boolean` (convenience predicate — calls `resolveCapabilityRuntimeState`, finds the entry, returns `entry.active`; `false` when capability not found). CLI surface: `gsd-tools capability state [--config-dir ]` — emits `{ runtimeConfigDir, capabilities[] }`. Per-capability output: `{ id, tier, skills[], installed, surfaced, enabled, active, hooks[] }` where `installed` = every owned skill ∈ installedSkills (or `installedSkills==='*'`; vacuously true for empty-skills caps), `surfaced` = every owned skill ∈ surfacedSkills (vacuously true for empty-skills caps), `enabled = installed && surfaced` (unchanged — install+surface toggle only), `active = enabled && configActivation` (tri-state deepening: configActivation resolves the capability's `activationKey` via `_resolveActivationValue`; absent `activationKey` → configActivation=true, so `active===enabled` for ungated capabilities), and `hooks` = `[{ point, kind: 'step'|'gate'|'contribution', when, configured, active }]` derived from the cap's `steps`, `gates`, `contributions` arrays (`configured` resolves `when`; `active = enabled && configured`). Capabilities sorted by `id` for determinism. Defensive: malformed registry → `{ capabilities: [] }`, never throws; inline literal `__proto__`/`constructor`/`prototype` prototype-pollution guard on capability id keys. +ADR-857 phase 4b/6 unified resolver that composes the three toggle systems (install profile, runtime surface, config activation) into one per-capability view consumed by workflow hook rendering. Source of truth: `gsd-core/bin/lib/capability-state.cjs` (generated from `src/capability-state.cts`). Interface: `resolveCapabilityState({ registry, installedSkills, surfacedSkills, config, cwd? }) → { capabilities: CapabilityStateEntry[] }` (pure, no I/O); `resolveCapabilityRuntimeState(cwd, runtimeConfigDir)` (I/O resolver shared by workflow dispatch and diagnostics — returns `{ runtimeConfigDir, warnings, capabilities }` only; `registry` and `config` are NOT returned — callers that need them import `capability-registry.cjs` and call `loadConfig(cwd)` directly); `cmdCapabilityState(cwd, runtimeConfigDir, raw, opts)` (I/O output entry point); `isCapabilityActive(capId, cwd): boolean` (convenience predicate — calls `resolveCapabilityRuntimeState`, finds the entry, returns `entry.active`; `false` when capability not found). CLI surface: `gsd-tools capability state [--config-dir ]` — emits `{ runtimeConfigDir, capabilities[] }`. Per-capability output: `{ id, tier, skills[], installed, surfaced, enabled, active, hooks[] }` where `installed` = every owned skill ∈ installedSkills (or `installedSkills==='*'`; vacuously true for empty-skills caps), `surfaced` = every owned skill ∈ surfacedSkills (vacuously true for empty-skills caps), `enabled = installed && surfaced` (unchanged — install+surface toggle only), `active = enabled && configActivation` (tri-state deepening: configActivation resolves the capability's `activationKey` via `_resolveActivationValue`; absent `activationKey` → configActivation=true, so `active===enabled` for ungated capabilities), and `hooks` = `[{ point, kind: 'step'|'gate'|'contribution', when, configured, active }]` derived from the cap's `steps`, `gates`, `contributions` arrays (`configured` resolves `when`; `active = enabled && configured`). Capabilities sorted by `id` for determinism. Defensive: malformed registry → `{ capabilities: [] }`, never throws; inline literal `__proto__`/`constructor`/`prototype` prototype-pollution guard on capability id keys. ### Capability State Writer The write-side mirror of the Capability State Resolver. Takes a desired capability state — per-capability `enabled` plus per-hook `gates` — and projects it onto the substrates: `enabled` drives the runtime surface (`.gsd-surface.json`) as the capability on/off switch; `gates` drive the federated config keys (`config.json` `workflow.*`) for hook-level granularity; the install profile (`.gsd-profile`) is a read-only floor it never writes. Writes the surface once and config once (atomic per substrate), then re-runs the resolver and reports divergence (assert-and-report) — so 'off means off' holds as a write-time invariant rather than by caller discipline. Source of truth: `src/capability-writer.cts`; the surface and config writers become its internal adapters. @@ -190,6 +190,8 @@ A Capability whose integration shape brings its own external process, service, o `RULESET.CAPABILITY.step-additive-gate-blocks=a `step` hook is purely additive (invoke skill + produce artifacts, NEVER halts the host); host-blocking preconditions are `gate`s (blocking:true, onError:halt); runtime/mode context (auto/chain vs manual) self-gates IN THE SKILL, not via `when` (config-only). §5.6 = plan:pre step (ui-phase; skill self-gates on frontend+pipeline, auto-fires only in pipelines) + a NEW plan:pre gate (frontend-and-no-UI-SPEC → halt, when:workflow.ui_safety_gate); the loop.render-hooks dispatch template handles steps AND gates. Resolves #1022.` +`RULESET.CAPABILITY.precedence-engine-single-owner=the config-key four-level precedence walk (loadConfig result → workstream config.json → root config.json → registry.configSchema default → absent) is owned solely by src/capability-activation.cts: raw-value primitive resolveConfigKey(dotKey, {config,cwd,registry}) and boolean wrapper _resolveActivationValue(dotKey,config,cwd,registry); loop-resolver.cts imports the engine (no duplicate); resolveConfigValues in loop-resolver.cts delegates to resolveConfigKey; resolveCapabilityRuntimeState does NOT return registry/config — callers import capability-registry.cjs and call loadConfig(cwd) directly.` + ### Wing A MemPalace organizational unit corresponding to one project or repository. GSD derives the wing name from `project_code` or the project directory when `mempalace.wing` is unset. A wing contains Rooms. Cross-project Tunnels connect rooms across wings. MemPalace vocabulary — see Connected Capability, MemPalace memory capability (issue #956). diff --git a/src/capability-activation.cts b/src/capability-activation.cts index 1440dc69f..265f73a49 100644 --- a/src/capability-activation.cts +++ b/src/capability-activation.cts @@ -62,39 +62,71 @@ function _readRawConfigKey( } } +/** + * Resolve the raw value for a dotted config key using the four-level precedence + * walk. Returns { found, value } with the RAW value (not coerced to boolean), + * so callers can decide how to interpret the value (boolean gate vs. raw config + * value for numeric/string settings like security_asvs_level). + * + * Precedence (mirrors _resolveActivationValue): + * 1. loadConfig result (config arg) — guarded nested-lookup. + * 2. Workstream config.json at planningDir(cwd)/config.json. + * 3. Root config.json at planningRoot(cwd)/config.json (only if path differs). + * 4. registry.configSchema[dotKey].default — schema default. + * 5. Absent → { found: false, value: undefined }. + */ +function resolveConfigKey( + dotKey: string, + opts: { config: Record; cwd: string | undefined; registry: Record }, +): { found: boolean; value: unknown } { + const { config, cwd, registry } = opts; + + // Level 1: loadConfig result + const fromConfig = _getNestedConfigValue(config, dotKey); + if (fromConfig.found) return { found: true, value: fromConfig.value }; + + // Level 2 + 3: raw config.json files (only when cwd is available) + if (cwd) { + const wsConfigPath = path.join(planningDir(cwd), 'config.json'); + const rootConfigPath = path.join(planningRoot(cwd), 'config.json'); + + const fromWs = _readRawConfigKey(wsConfigPath, dotKey); + if (fromWs.found) return { found: true, value: fromWs.value }; + + if (wsConfigPath !== rootConfigPath) { + const fromRoot = _readRawConfigKey(rootConfigPath, dotKey); + if (fromRoot.found) return { found: true, value: fromRoot.value }; + } + } + + // Level 4: registry configSchema default + const schemaMap = registry['configSchema']; + if (schemaMap && typeof schemaMap === 'object' && !Array.isArray(schemaMap) + && Object.prototype.hasOwnProperty.call(schemaMap, dotKey)) { + const schemaEntry = (schemaMap as Record)[dotKey]; + if (schemaEntry && typeof schemaEntry === 'object' && schemaEntry !== null) { + const def = (schemaEntry as Record)['default']; + if (def !== undefined) return { found: true, value: def }; + } + } + + // Level 5: absent + return { found: false, value: undefined }; +} + function _resolveActivationValue( dotKey: string, config: Record, cwd: string | undefined, registry: Record, ): boolean { - const fromConfig = _getNestedConfigValue(config, dotKey); - if (fromConfig.found) return Boolean(fromConfig.value); - - if (cwd) { - const wsConfigPath = path.join(planningDir(cwd), 'config.json'); - const rootConfigPath = path.join(planningRoot(cwd), 'config.json'); - - const fromWs = _readRawConfigKey(wsConfigPath, dotKey); - if (fromWs.found) return Boolean(fromWs.value); - - if (wsConfigPath !== rootConfigPath) { - const fromRoot = _readRawConfigKey(rootConfigPath, dotKey); - if (fromRoot.found) return Boolean(fromRoot.value); - } - } - - const schemaEntry = (registry['configSchema'] as Record | undefined)?.[dotKey]; - if (schemaEntry && typeof schemaEntry === 'object' && schemaEntry !== null) { - const def = (schemaEntry as Record)['default']; - if (def !== undefined) return Boolean(def); - } - - return false; + const r = resolveConfigKey(dotKey, { config, cwd, registry }); + return r.found ? Boolean(r.value) : false; } export = { _getNestedConfigValue, _readRawConfigKey, _resolveActivationValue, + resolveConfigKey, }; diff --git a/src/capability-state.cts b/src/capability-state.cts index c981af516..81e1d8c83 100644 --- a/src/capability-state.cts +++ b/src/capability-state.cts @@ -128,8 +128,6 @@ interface ResolveCapabilityStateResult { interface ResolveCapabilityRuntimeStateResult { runtimeConfigDir: string; warnings: string[]; - registry: Record; - config: Record; capabilities: CapabilityStateEntry[]; } @@ -430,6 +428,7 @@ function _resolveManifest(commandsGsdDir: string, configDir: string): Map, ): ResolveCapabilityRuntimeStateResult { const warnings: string[] = []; @@ -520,11 +519,18 @@ function resolveCapabilityRuntimeState( } // ── Load config ─────────────────────────────────────────────────────────────── + // When the caller already holds a loadConfig snapshot (e.g. cmdLoopRenderHooks), + // accept it via configOverride so capability `active` and hook resolution + // share the SAME config object — single snapshot, no TOCTOU window. let config: Record; - try { - config = loadConfig(cwd); - } catch { - config = {}; + if (configOverride !== undefined) { + config = configOverride; + } else { + try { + config = loadConfig(cwd); + } catch { + config = {}; + } } // ── Resolve state ──────────────────────────────────────────────────────────── @@ -540,8 +546,6 @@ function resolveCapabilityRuntimeState( return { runtimeConfigDir: resolvedConfigDir, warnings, - registry, - config, capabilities: result.capabilities, }; } diff --git a/src/capability-writer.cts b/src/capability-writer.cts index 37d178b81..cf067eaee 100644 --- a/src/capability-writer.cts +++ b/src/capability-writer.cts @@ -129,7 +129,8 @@ function setCapabilityState( const resolvedConfigDir = before.runtimeConfigDir; // ── Load registry ───────────────────────────────────────────────────────── - const registry = before.registry; + // eslint-disable-next-line @typescript-eslint/no-require-imports + const registry = require('./capability-registry.cjs') as Record; const capabilitiesMap = ( registry['capabilities'] && typeof registry['capabilities'] === 'object' && !Array.isArray(registry['capabilities']) ? registry['capabilities'] diff --git a/src/loop-resolver.cts b/src/loop-resolver.cts index a566f6c38..fe07faeb4 100644 --- a/src/loop-resolver.cts +++ b/src/loop-resolver.cts @@ -20,17 +20,14 @@ * registry/config arguments so they are trivially testable without I/O. * * Dependencies (leaf modules only — no circular risk): - * - node:fs / node:path (raw config.json read for capability-key activation) * - ./config-loader.cjs (loadConfig) - * - ./planning-workspace.cjs (planningDir — to locate config.json) * - ./io.cjs (output, error) + * - ./capability-activation.cjs (resolveConfigKey, _resolveActivationValue, _getNestedConfigValue, _readRawConfigKey) * - loop-host-contract.cjs (CANONICAL_POINTS via LOOP_HOST_CONTRACT) * - capability-registry.cjs (byLoopPoint, consumed at call time) + * - capability-state.cjs (resolveCapabilityRuntimeState — for capabilities list) */ -import fs from 'node:fs'; -import path from 'node:path'; - // eslint-disable-next-line @typescript-eslint/no-require-imports import ioMod = require('./io.cjs'); const { output: coreOutput, error: coreError } = ioMod; @@ -43,9 +40,10 @@ const { loadConfig } = configLoaderModule; import capabilityStateModule = require('./capability-state.cjs'); const { resolveCapabilityRuntimeState } = capabilityStateModule; +// ─── Capability-activation engine (single owner for config-key precedence) ──── // eslint-disable-next-line @typescript-eslint/no-require-imports -import planningWorkspaceMod = require('./planning-workspace.cjs'); -const { planningDir, planningRoot } = planningWorkspaceMod; +import capabilityActivationModule = require('./capability-activation.cjs'); +const { _getNestedConfigValue, _readRawConfigKey, _resolveActivationValue, resolveConfigKey } = capabilityActivationModule; // ─── Canonical points (derived from LOOP_HOST_CONTRACT — authoritative 12) ─── @@ -95,133 +93,7 @@ function _getCanonicalPoints(_registry: Record): ReadonlyArray< return CANONICAL_POINTS; } -// ─── Prototype-pollution guard (inline literal, CodeQL barrier) ─────────────── - -/** - * Traverse a dotted config key through a nested config object. - * E.g. "workflow.ui_phase" in { workflow: { ui_phase: true } } → { found: true, value: true } - * Returns { found: false } if any segment is a forbidden key or not an own property. - */ -function _getNestedConfigValue( - config: Record, - dotKey: string, -): { found: boolean; value: unknown } { - const segments = dotKey.split('.'); - let current: unknown = config; - for (const seg of segments) { - // Inline literal prototype-pollution guard (CodeQL barrier) - if (seg === '__proto__' || seg === 'constructor' || seg === 'prototype') { - return { found: false, value: undefined }; - } - if (typeof current !== 'object' || current === null) { - return { found: false, value: undefined }; - } - const cur = current as Record; - if (!Object.prototype.hasOwnProperty.call(cur, seg)) { - return { found: false, value: undefined }; - } - current = cur[seg]; - } - return { found: true, value: current }; -} - -// ─── Single-key activation resolver (FIX 1) ─────────────────────────────────── - -/** - * Warn-once set for raw config.json parse errors. - * Avoids noisy per-call stderr from a single malformed file. - */ -const _warnedRawConfigPaths = new Set(); - -/** - * Read a raw config.json file and perform a guarded nested-lookup of a single - * dotted key. Returns { found: false } if the file is missing (ENOENT) or if - * the key is absent/forbidden. On a genuine JSON parse error: warns once to - * stderr and returns { found: false } — never throws. - */ -function _readRawConfigKey( - filePath: string, - dotKey: string, -): { found: boolean; value: unknown } { - try { - const raw = fs.readFileSync(filePath, 'utf8'); - let parsed: Record; - try { - parsed = JSON.parse(raw) as Record; - } catch { - if (!_warnedRawConfigPaths.has(filePath)) { - _warnedRawConfigPaths.add(filePath); - try { - process.stderr.write( - `gsd-tools: warning: failed to parse ${filePath} as JSON — skipping for activation resolution\n`, - ); - } catch { /* stderr might be closed */ } - } - return { found: false, value: undefined }; - } - return _getNestedConfigValue(parsed, dotKey); - } catch { - // ENOENT (missing file) is expected → skip silently. All other errors → also skip (defensive). - return { found: false, value: undefined }; - } -} - -/** - * FIX 1: Resolve the effective value for a hook's `when` key using the - * four-level precedence: - * - * 1. loadConfig result (`config` arg) — guarded nested-lookup of the dotted key. - * This is the post-cutover federated path (covers keys that loadConfig now exposes). - * 2. Raw workstream `.planning/.../config.json` — guarded single-key lookup. - * Workstream wins over root (mirrors loadConfig inheritance). - * 3. Raw root `.planning/config.json` — guarded single-key lookup. - * 4. `registry.configSchema[when]?.default` — schema default. - * A `default: true` hook is active out-of-the-box without any config. - * 5. Absent → inactive (return false). - * - * Never constructs a merged object from raw JSON keys — only reads the single - * leaf value at the guarded dotted path. Prototype-pollution sink is eliminated. - */ -function _resolveActivationValue( - dotKey: string, - config: Record, - cwd: string | undefined, - registry: Record, -): boolean { - // Level 1: loadConfig result - const fromConfig = _getNestedConfigValue(config, dotKey); - if (fromConfig.found) return Boolean(fromConfig.value); - - // Level 2 + 3: raw config.json files (only when cwd is available) - if (cwd) { - // Level 2: workstream config (planningDir respects GSD_WORKSTREAM env) - const wsConfigPath = path.join(planningDir(cwd), 'config.json'); - // Level 3: root config (planningRoot = cwd/.planning always) - const rootConfigPath = path.join(planningRoot(cwd), 'config.json'); - - // Workstream wins over root (mirroring loadConfig root→workstream precedence: - // workstream overlays root, so workstream value takes precedence). - const fromWs = _readRawConfigKey(wsConfigPath, dotKey); - if (fromWs.found) return Boolean(fromWs.value); - - // Only read root if it differs from the workstream path (avoids double-read - // when no workstream is active and both paths resolve to the same file). - if (wsConfigPath !== rootConfigPath) { - const fromRoot = _readRawConfigKey(rootConfigPath, dotKey); - if (fromRoot.found) return Boolean(fromRoot.value); - } - } - - // Level 4: registry configSchema default - const schemaEntry = (registry['configSchema'] as Record | undefined)?.[dotKey]; - if (schemaEntry && typeof schemaEntry === 'object' && schemaEntry !== null) { - const def = (schemaEntry as Record)['default']; - if (def !== undefined) return Boolean(def); - } - - // Level 5: absent → inactive - return false; -} +// ─── (Precedence engine imported from capability-activation.cjs above) ──────── // ─── Types ──────────────────────────────────────────────────────────────────── @@ -383,27 +255,8 @@ function resolveLoopHooks(input: ResolveLoopHooksInput): ResolveLoopHooksResult // Prototype-pollution guard (inline literal, CodeQL barrier) if (alias === '__proto__' || alias === 'constructor' || alias === 'prototype') continue; if (typeof dotKey !== 'string') continue; - // Level 1: loadConfig result - const fromConfig = _getNestedConfigValue(config, dotKey); - if (fromConfig.found) { resolved[alias] = fromConfig.value; continue; } - // Level 2 + 3: raw config.json files - if (cwd) { - const wsConfigPath = path.join(planningDir(cwd), 'config.json'); - const rootConfigPath = path.join(planningRoot(cwd), 'config.json'); - const fromWs = _readRawConfigKey(wsConfigPath, dotKey); - if (fromWs.found) { resolved[alias] = fromWs.value; continue; } - if (wsConfigPath !== rootConfigPath) { - const fromRoot = _readRawConfigKey(rootConfigPath, dotKey); - if (fromRoot.found) { resolved[alias] = fromRoot.value; continue; } - } - } - // Level 4: registry configSchema default - const schemaEntry = (registry['configSchema'] as Record | undefined)?.[dotKey]; - if (schemaEntry && typeof schemaEntry === 'object' && schemaEntry !== null) { - const def = (schemaEntry as Record)['default']; - if (def !== undefined) { resolved[alias] = def; continue; } - } - // Level 5: absent → undefined (omit from resolved map) + const r = resolveConfigKey(dotKey, { config, cwd, registry }); + if (r.found) resolved[alias] = r.value; } return Object.keys(resolved).length > 0 ? resolved : undefined; } @@ -622,14 +475,26 @@ function cmdLoopRenderHooks( const runtimeConfigDir = typeof options['configDir'] === 'string' ? options['configDir'] : undefined; - const state = resolveCapabilityRuntimeState(cwd, runtimeConfigDir) as { + // Load the config snapshot ONCE and share it with both the capability-state + // resolver (via configOverride) and loop-hook resolution, so federated keys + // present in loadConfig resolve identically for `active` and for hook when/ + // configValues — eliminating the previous double loadConfig() call. Note: keys + // absent from loadConfig still fall through to raw .planning/config.json reads + // (precedence levels 2-3) in each pass; that residual re-read window is + // pre-existing (unchanged by this consolidation), not introduced here. + let config: Record; + try { + config = loadConfig(cwd); + } catch { + config = {}; + } + const state = resolveCapabilityRuntimeState(cwd, runtimeConfigDir, config) as { warnings?: string[]; - registry: Record; - config: Record; capabilities: Array<{ id: string; enabled?: boolean; active: boolean }>; }; - const registry = state.registry; - const config = state.config || loadConfig(cwd); + // Registry is the static generated module — same object capability-state uses internally. + // eslint-disable-next-line @typescript-eslint/no-require-imports + const registry = require('./capability-registry.cjs') as Record; const capabilityStatesById = new Map(); for (const cap of state.capabilities || []) { capabilityStatesById.set(cap.id, cap); @@ -677,6 +542,9 @@ export = { _getNestedConfigValue, _resolveActivationValue, _readRawConfigKey, + // Re-exported for identity parity guard (FIX 2: resolveConfigValues in this module + // calls resolveConfigKey; exporting it here makes the single-owner contract testable). + resolveConfigKey, CANONICAL_POINTS_FALLBACK, CANONICAL_POINTS, }; diff --git a/tests/capability-precedence-parity.test.cjs b/tests/capability-precedence-parity.test.cjs new file mode 100644 index 000000000..c6ece69aa --- /dev/null +++ b/tests/capability-precedence-parity.test.cjs @@ -0,0 +1,596 @@ +'use strict'; +/** + * capability-precedence-parity.test.cjs — DEFECT.GENERATIVE-FIX parity gate + * + * Proves that the config-key four-level precedence walk has a single owner + * (capability-activation.cjs) and that loop-resolver.cjs re-exports the same + * functions rather than maintaining local copies. + * + * Two contracts enforced: + * (a) Identity: loop-resolver's _resolveActivationValue, _getNestedConfigValue, + * and _readRawConfigKey are the SAME function objects as + * capability-activation's exports. Fails the instant a local copy is + * re-introduced. + * (b) Behavioral matrix: across a fixture matrix hitting each precedence level + * (loadConfig result, workstream config.json, root config.json, registry + * default, absent, falsy values, prototype-pollution key), asserts that + * _resolveActivationValue(key,config,cwd,registry) === + * (resolveConfigKey(key,{config,cwd,registry}).found ? + * Boolean(resolveConfigKey(key,{config,cwd,registry}).value) : false). + * Level-2/3 cases use real tmpdir .planning/config.json fixtures. + * + * RULESET: RULESET.TESTS.no-source-grep — no readFileSync + .includes() on source. + * RULESET: RULESET.TESTS.boundary-coverage — covers limit-1/limit/limit+1 (falsy boundary). + */ + +process.env.GSD_TEST_MODE = '1'; + +const { describe, test, before, after } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { cleanup } = require('./helpers.cjs'); + +// ── Module paths ────────────────────────────────────────────────────────────── + +const LIB = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib'); + +const capabilityActivation = require(path.join(LIB, 'capability-activation.cjs')); +const loopResolver = require(path.join(LIB, 'loop-resolver.cjs')); + +// ─── (a) Identity guard ─────────────────────────────────────────────────────── + +describe('capability-precedence-parity: identity guard — loop-resolver re-exports same function objects', () => { + test('_resolveActivationValue is the same function in both modules', () => { + assert.strictEqual( + loopResolver._resolveActivationValue, + capabilityActivation._resolveActivationValue, + '_resolveActivationValue: loop-resolver must re-export the capability-activation.cjs function, not a local copy', + ); + }); + + test('_getNestedConfigValue is the same function in both modules', () => { + assert.strictEqual( + loopResolver._getNestedConfigValue, + capabilityActivation._getNestedConfigValue, + '_getNestedConfigValue: loop-resolver must re-export the capability-activation.cjs function, not a local copy', + ); + }); + + test('_readRawConfigKey is the same function in both modules', () => { + assert.strictEqual( + loopResolver._readRawConfigKey, + capabilityActivation._readRawConfigKey, + '_readRawConfigKey: loop-resolver must re-export the capability-activation.cjs function, not a local copy', + ); + }); +}); + +// ─── (b) Behavioral matrix ──────────────────────────────────────────────────── + +/** + * Build a minimal registry with a configSchema entry for a dotted key. + * def=undefined means no default (absent). def= provides a default. + */ +function makeRegistry(dotKey, def) { + const registry = {}; + if (def !== undefined) { + registry.configSchema = { + [dotKey]: { default: def }, + }; + } else { + registry.configSchema = {}; + } + return registry; +} + +/** + * Helper: call resolveConfigKey and assert it equals + * _resolveActivationValue(key,config,cwd,registry) on the boolean coercion. + */ +function assertParity(key, config, cwd, registry, label) { + const { _resolveActivationValue, resolveConfigKey } = capabilityActivation; + const boolResult = _resolveActivationValue(key, config, cwd, registry); + const rawResult = resolveConfigKey(key, { config, cwd, registry }); + const expectedBool = rawResult.found ? Boolean(rawResult.value) : false; + assert.strictEqual( + boolResult, + expectedBool, + `parity check failed for ${label}: _resolveActivationValue=${boolResult} but resolveConfigKey gives found=${rawResult.found} value=${JSON.stringify(rawResult.value)} → expectedBool=${expectedBool}`, + ); + return { boolResult, rawResult }; +} + +describe('capability-precedence-parity: behavioral matrix', () => { + let tmpDir; + + before(() => { + // Create a hermetic tmpdir for file-based fixtures (levels 2+3). + // Clear env vars that redirect planningDir/planningRoot. + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'parity-test-')); + }); + + after(() => { + // Remove the tmpdir (helpers.cleanup carries the Windows-EBUSY retry budget). + cleanup(tmpDir); + }); + + // Env vars that planningDir/planningRoot read — save/restore around each test. + function withCleanEnv(fn) { + const saved = { + GSD_WORKSTREAM: process.env.GSD_WORKSTREAM, + GSD_PROJECT: process.env.GSD_PROJECT, + CLAUDE_CONFIG_DIR: process.env.CLAUDE_CONFIG_DIR, + }; + delete process.env.GSD_WORKSTREAM; + delete process.env.GSD_PROJECT; + delete process.env.CLAUDE_CONFIG_DIR; + try { + return fn(); + } finally { + if (saved.GSD_WORKSTREAM !== undefined) process.env.GSD_WORKSTREAM = saved.GSD_WORKSTREAM; + else delete process.env.GSD_WORKSTREAM; + if (saved.GSD_PROJECT !== undefined) process.env.GSD_PROJECT = saved.GSD_PROJECT; + else delete process.env.GSD_PROJECT; + if (saved.CLAUDE_CONFIG_DIR !== undefined) process.env.CLAUDE_CONFIG_DIR = saved.CLAUDE_CONFIG_DIR; + else delete process.env.CLAUDE_CONFIG_DIR; + } + } + + // ── Level 1: loadConfig result (config arg has the key) ───────────────────── + + test('level-1 truthy: config arg has key=true → resolved true, parity holds', () => { + const key = 'feature.enabled'; + const config = { feature: { enabled: true } }; + const registry = makeRegistry(key, undefined); + const { boolResult, rawResult } = assertParity(key, config, undefined, registry, 'level-1 truthy'); + assert.strictEqual(boolResult, true); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, true); + }); + + test('level-1 falsy (false): config arg has key=false → resolved false, parity holds', () => { + // BVA: falsy value false — boundary case + const key = 'feature.enabled'; + const config = { feature: { enabled: false } }; + const registry = makeRegistry(key, true); // default true but level-1 wins + const { boolResult, rawResult } = assertParity(key, config, undefined, registry, 'level-1 falsy=false'); + assert.strictEqual(boolResult, false); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, false); // raw value preserved + }); + + test('level-1 falsy (0): config arg has key=0 → resolved false, parity holds', () => { + // BVA: falsy value 0 — boundary case + const key = 'feature.level'; + const config = { feature: { level: 0 } }; + const registry = makeRegistry(key, 2); // default 2 but level-1 wins + const { boolResult, rawResult } = assertParity(key, config, undefined, registry, 'level-1 falsy=0'); + assert.strictEqual(boolResult, false); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, 0); // raw value 0 preserved + }); + + test('level-1 truthy (nonzero): config arg has key=2 → resolved true, parity holds', () => { + // BVA: truthy numeric — boundary-adjacent to 0 + const key = 'feature.level'; + const config = { feature: { level: 2 } }; + const registry = makeRegistry(key, undefined); + const { boolResult, rawResult } = assertParity(key, config, undefined, registry, 'level-1 truthy=2'); + assert.strictEqual(boolResult, true); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, 2); + }); + + // ── Level 2: workstream config.json ───────────────────────────────────────── + + test('level-2: workstream config.json has key → resolved, level-1 empty, parity holds', () => { + withCleanEnv(() => { + // Set up a .planning/config.json in tmpDir + const planningDir = path.join(tmpDir, 'ws-l2', '.planning'); + fs.mkdirSync(planningDir, { recursive: true }); + fs.writeFileSync( + path.join(planningDir, 'config.json'), + JSON.stringify({ graphify: { enabled: true } }), + 'utf8', + ); + const key = 'graphify.enabled'; + const config = {}; // level-1 miss + const registry = makeRegistry(key, undefined); + const cwd = path.join(tmpDir, 'ws-l2'); + const { boolResult, rawResult } = assertParity(key, config, cwd, registry, 'level-2 workstream'); + assert.strictEqual(boolResult, true); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, true); + }); + }); + + test('level-2 falsy: workstream config.json has key=false → false, parity holds', () => { + withCleanEnv(() => { + const planningDir = path.join(tmpDir, 'ws-l2-false', '.planning'); + fs.mkdirSync(planningDir, { recursive: true }); + fs.writeFileSync( + path.join(planningDir, 'config.json'), + JSON.stringify({ graphify: { enabled: false } }), + 'utf8', + ); + const key = 'graphify.enabled'; + const config = {}; + const registry = makeRegistry(key, true); // default true, but level-2 wins + const cwd = path.join(tmpDir, 'ws-l2-false'); + const { boolResult, rawResult } = assertParity(key, config, cwd, registry, 'level-2 falsy'); + assert.strictEqual(boolResult, false); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, false); + }); + }); + + // ── Level 3: root config.json (only when paths differ) ────────────────────── + + // Without GSD_WORKSTREAM, planningDir === planningRoot, so levels 2 and 3 point + // to the same file. Level-3 distinction is only exercisable by setting GSD_WORKSTREAM. + // We test the "paths same → no double-read" path (covered by level-2 test above) + // and the "paths differ" path via GSD_WORKSTREAM. + test('level-3: root config.json read when workstream path differs, parity holds', () => { + // Use GSD_WORKSTREAM to force planningDir to differ from planningRoot. + withCleanEnv(() => { + const projectRoot = path.join(tmpDir, 'ws-l3-project'); + // Root planning dir = projectRoot/.planning + const rootPlanningDir = path.join(projectRoot, '.planning'); + // Workstream planning dir = projectRoot/.planning/workstreams/mystream + const wsName = 'mystream'; + const wsPlanningDir = path.join(rootPlanningDir, 'workstreams', wsName); + fs.mkdirSync(rootPlanningDir, { recursive: true }); + fs.mkdirSync(wsPlanningDir, { recursive: true }); + + // Write ONLY the root config.json (no ws config.json) + fs.writeFileSync( + path.join(rootPlanningDir, 'config.json'), + JSON.stringify({ intel: { enabled: true } }), + 'utf8', + ); + + // Activate workstream so planningDir → wsPlanningDir, planningRoot → rootPlanningDir + process.env.GSD_WORKSTREAM = wsName; + + const key = 'intel.enabled'; + const config = {}; + const registry = makeRegistry(key, undefined); + const { boolResult, rawResult } = assertParity(key, config, projectRoot, registry, 'level-3 root config'); + // ws config.json absent → level-2 miss; root config.json present → level-3 hit + assert.strictEqual(boolResult, true); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, true); + }); + }); + + // ── Level 4: registry configSchema default ─────────────────────────────────── + + test('level-4: registry default=true, no config or file → true, parity holds', () => { + withCleanEnv(() => { + const key = 'workflow.some_feature'; + const config = {}; + const registry = makeRegistry(key, true); + // cwd points to an empty dir — no .planning/config.json files + const cwd = path.join(tmpDir, 'l4-default-true'); + fs.mkdirSync(cwd, { recursive: true }); + const { boolResult, rawResult } = assertParity(key, config, cwd, registry, 'level-4 default=true'); + assert.strictEqual(boolResult, true); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, true); + }); + }); + + test('level-4: registry default=false → false (BVA: falsy default), parity holds', () => { + withCleanEnv(() => { + const key = 'workflow.some_feature'; + const config = {}; + const registry = makeRegistry(key, false); + const cwd = path.join(tmpDir, 'l4-default-false'); + fs.mkdirSync(cwd, { recursive: true }); + const { boolResult, rawResult } = assertParity(key, config, cwd, registry, 'level-4 default=false'); + assert.strictEqual(boolResult, false); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, false); // raw false preserved + }); + }); + + test('level-4: registry default=0 → false (BVA: numeric zero default), parity holds', () => { + withCleanEnv(() => { + const key = 'feature.level'; + const config = {}; + const registry = makeRegistry(key, 0); + const cwd = path.join(tmpDir, 'l4-default-0'); + fs.mkdirSync(cwd, { recursive: true }); + const { boolResult, rawResult } = assertParity(key, config, cwd, registry, 'level-4 default=0'); + assert.strictEqual(boolResult, false); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, 0); // raw 0 preserved + }); + }); + + // ── Level 5: absent ────────────────────────────────────────────────────────── + + test('level-5: key absent everywhere → false, found=false, parity holds', () => { + withCleanEnv(() => { + const key = 'nonexistent.key'; + const config = {}; + const registry = makeRegistry(key, undefined); // no default + const cwd = path.join(tmpDir, 'l5-absent'); + fs.mkdirSync(cwd, { recursive: true }); + const { boolResult, rawResult } = assertParity(key, config, cwd, registry, 'level-5 absent'); + assert.strictEqual(boolResult, false); + assert.strictEqual(rawResult.found, false); + assert.strictEqual(rawResult.value, undefined); + }); + }); + + test('level-5: cwd=undefined, absent key → false, found=false, parity holds', () => { + const key = 'nonexistent.key'; + const config = {}; + const registry = makeRegistry(key, undefined); + const { boolResult, rawResult } = assertParity(key, config, undefined, registry, 'level-5 absent cwd=undefined'); + assert.strictEqual(boolResult, false); + assert.strictEqual(rawResult.found, false); + }); + + // ── Prototype-pollution guard (nested-traversal sink, levels 1–3) ────────────── + // + // The guard protects the NESTED config-object traversal (_getNestedConfigValue) — + // the only prototype-pollution sink. A `__proto__`/`constructor`/`prototype` + // SEGMENT encountered mid-path is rejected → found=false. Level 4 is a flat + // single-key lookup of the whole dotted string against registry.configSchema (no + // nested traversal, not a pollution sink), so we deliberately seed NO level-4 + // default here — otherwise that flat lookup would legitimately match the literal + // dotted key and the segment guard would not be the thing under test. + + test('prototype-pollution: __proto__ mid-path segment → found=false, boolResult=false, parity holds', () => { + const key = 'a.__proto__.polluted'; + const config = { a: { real: 1 } }; // 'a' resolves, then the '__proto__' segment must be rejected + const registry = makeRegistry(key, undefined); // no level-4 default + const { boolResult, rawResult } = assertParity(key, config, undefined, registry, 'proto-pollution __proto__ mid-path'); + assert.strictEqual(rawResult.found, false, '__proto__ path segment must be rejected by the nested-traversal guard'); + assert.strictEqual(boolResult, false); + }); + + test('prototype-pollution: constructor mid-path segment → found=false, boolResult=false, parity holds', () => { + const key = 'a.constructor.polluted'; + const config = { a: { real: 1 } }; + const registry = makeRegistry(key, undefined); // no level-4 default + const { boolResult, rawResult } = assertParity(key, config, undefined, registry, 'proto-pollution constructor mid-path'); + assert.strictEqual(rawResult.found, false, 'constructor path segment must be rejected by the nested-traversal guard'); + assert.strictEqual(boolResult, false); + }); + + // ── Level-1 precedence over level-4 ───────────────────────────────────────── + + test('level-1 wins over level-4 default: config=false beats registry default=true', () => { + // BVA: this is the critical boundary — level-1 falsy beats level-4 truthy + const key = 'graphify.enabled'; + const config = { graphify: { enabled: false } }; + const registry = makeRegistry(key, true); + const { boolResult, rawResult } = assertParity(key, config, undefined, registry, 'level-1 false beats level-4 true default'); + assert.strictEqual(boolResult, false, 'config value false must win over registry default true'); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, false); + }); +}); + +// ─── (c) resolveConfigKey identity guard (FIX 2 — loop-resolver re-exports resolveConfigKey) ──── +// +// resolveLoopHooks.resolveConfigValues uses resolveConfigKey internally to build +// hook configValues. Exporting resolveConfigKey from loop-resolver and asserting +// identity here ensures that if someone ever introduces a local copy, this test +// will catch it immediately (same mechanism as the _resolveActivationValue guard +// above, applied to the raw-value consumer). + +describe('capability-precedence-parity: identity guard — loop-resolver.resolveConfigKey is same fn as capability-activation.resolveConfigKey', () => { + test('resolveConfigKey is the same function in both modules', () => { + assert.ok( + typeof loopResolver.resolveConfigKey === 'function', + 'loop-resolver must export resolveConfigKey', + ); + assert.strictEqual( + loopResolver.resolveConfigKey, + capabilityActivation.resolveConfigKey, + 'resolveConfigKey: loop-resolver must re-export the capability-activation.cjs function, not a local copy', + ); + }); +}); + +// ─── (d) Behavioral: loop-resolver resolveConfigValues uses the shared engine ──────────────────── +// +// Proves that resolveLoopHooks's resolveConfigValues closure (the RAW-value consumer) +// produces the SAME result as calling capabilityActivation.resolveConfigKey directly +// for the same (dotKey, config, cwd, registry) arguments. +// +// This test FAILS if resolveConfigValues is ever replaced with a divergent copy that +// does not delegate to resolveConfigKey — fulfilling the DEFECT.GENERATIVE-FIX mandate. +// +// Coverage: three cases required: +// (1) Level-1 config hit — key present in config arg +// (2) Level-4 registry default — key absent from config, present in schema default +// (3) Absent key — key absent everywhere (omitted from resolved output) + +describe('capability-precedence-parity: behavioral — loop-resolver resolveConfigValues uses shared resolveConfigKey engine', () => { + const { resolveLoopHooks } = loopResolver; + const { resolveConfigKey } = capabilityActivation; + const { CANONICAL_POINTS_FALLBACK } = loopResolver; + + /** + * Build a minimal registry that has: + * - one contribution hook at the given point with configValues map + * - one capability that owns that hook (capId = 'test-cap', active=true implied) + * - configSchema entries for level-4 defaults + */ + function makeLoopRegistry({ point, configValues, schemaDefaults = {} }) { + const byLoopPoint = {}; + for (const p of CANONICAL_POINTS_FALLBACK) { + byLoopPoint[p] = { steps: [], contributions: [], gates: [] }; + } + byLoopPoint[point].contributions = [ + { + capId: 'test-cap', + into: 'test-section', + fragment: { inline: 'test fragment' }, + configValues, + }, + ]; + const configSchema = {}; + for (const [dotKey, def] of Object.entries(schemaDefaults)) { + configSchema[dotKey] = { default: def }; + } + return { byLoopPoint, configSchema }; + } + + /** capabilityStatesById with test-cap active=true */ + function activeCapMap() { + return new Map([['test-cap', { enabled: true, active: true }]]); + } + + test('(1) level-1 config hit: resolveConfigValues matches resolveConfigKey for present key', () => { + // Hook declares configValues: { secLevel: 'security.asvs_level' } + // Config has security.asvs_level=2 → level-1 hit + const dotKey = 'security.asvs_level'; + const alias = 'secLevel'; + const config = { security: { asvs_level: 2 } }; + const registry = makeLoopRegistry({ + point: 'plan:pre', + configValues: { [alias]: dotKey }, + }); + + const resolved = resolveLoopHooks({ + point: 'plan:pre', + registry, + config, + cwd: undefined, + capabilityStatesById: activeCapMap(), + }); + + assert.strictEqual(resolved.activeHooks.length, 1, 'One active contribution hook expected'); + const hook = resolved.activeHooks[0]; + assert.ok(hook.configValues, 'configValues must be present on the resolved hook'); + + // resolveConfigKey direct result for the same args + const directResult = resolveConfigKey(dotKey, { config, cwd: undefined, registry }); + assert.strictEqual(directResult.found, true, 'direct resolveConfigKey must find the level-1 key'); + assert.strictEqual( + hook.configValues[alias], + directResult.value, + `resolved hook configValues[${alias}] must equal resolveConfigKey(...).value for level-1 hit (both=${JSON.stringify(directResult.value)})`, + ); + assert.strictEqual(hook.configValues[alias], 2, 'level-1 numeric value 2 must be preserved (not coerced to boolean)'); + }); + + test('(2) level-4 registry default hit: resolveConfigValues matches resolveConfigKey for schema default', () => { + // Hook declares configValues: { blockOn: 'security.block_on' } + // Config is empty; schema default = 'medium' + const dotKey = 'security.block_on'; + const alias = 'blockOn'; + const schemaDefault = 'medium'; + const config = {}; + const registry = makeLoopRegistry({ + point: 'execute:pre', + configValues: { [alias]: dotKey }, + schemaDefaults: { [dotKey]: schemaDefault }, + }); + + const resolved = resolveLoopHooks({ + point: 'execute:pre', + registry, + config, + cwd: undefined, + capabilityStatesById: activeCapMap(), + }); + + assert.strictEqual(resolved.activeHooks.length, 1, 'One active contribution hook expected'); + const hook = resolved.activeHooks[0]; + assert.ok(hook.configValues, 'configValues must be present on the resolved hook (schema default found)'); + + const directResult = resolveConfigKey(dotKey, { config, cwd: undefined, registry }); + assert.strictEqual(directResult.found, true, 'direct resolveConfigKey must find the level-4 schema default'); + assert.strictEqual( + hook.configValues[alias], + directResult.value, + `resolved hook configValues[${alias}] must equal resolveConfigKey(...).value for level-4 hit (both=${JSON.stringify(directResult.value)})`, + ); + assert.strictEqual(hook.configValues[alias], 'medium', 'level-4 string default must be preserved as raw string'); + }); + + test('(3a) mixed hook: one resolvable alias + one absent alias → configValues present, contains only the resolvable alias', () => { + // Hook declares configValues: { present: 'feature.on', absent: 'nope.missing' } + // Config has feature.on=true (level-1 hit); no registry default for nope.missing. + // Expected: resolved hook.configValues is defined, has 'present'=true, does NOT have 'absent'. + const presentDotKey = 'feature.on'; + const absentDotKey = 'nope.missing'; + const config = { feature: { on: true } }; + const registry = makeLoopRegistry({ + point: 'verify:post', + configValues: { present: presentDotKey, absent: absentDotKey }, + // No schema default for absentDotKey — it must be absent everywhere + }); + + const resolved = resolveLoopHooks({ + point: 'verify:post', + registry, + config, + cwd: undefined, + capabilityStatesById: activeCapMap(), + }); + + assert.strictEqual(resolved.activeHooks.length, 1, 'One active contribution hook expected'); + const hook = resolved.activeHooks[0]; + + // The 'present' alias must resolve via level-1 config hit + assert.ok(hook.configValues !== undefined, 'configValues must be defined (at least one alias resolved)'); + assert.strictEqual( + hook.configValues['present'], + true, + "hook.configValues['present'] must equal true (level-1 hit for feature.on)", + ); + + // The 'absent' alias must NOT appear in configValues at all (omit-when-absent contract) + assert.ok( + !Object.prototype.hasOwnProperty.call(hook.configValues, 'absent'), + "Absent key 'nope.missing' must not appear in hook.configValues (found=false → omit, not include as undefined)", + ); + }); + + test('(3b) all-absent hook: all configValues aliases absent everywhere → hook.configValues === undefined', () => { + // Hook declares configValues: { missingAlias: 'does.not.exist' } + // Key absent from config, no schema default, no config files. + // resolveConfigValues must return undefined (empty resolved map → omitted entirely). + const dotKey = 'does.not.exist'; + const alias = 'missingAlias'; + const config = {}; + const registry = makeLoopRegistry({ + point: 'ship:pre', + configValues: { [alias]: dotKey }, + // No schema default for this key + }); + + const resolved = resolveLoopHooks({ + point: 'ship:pre', + registry, + config, + cwd: undefined, + capabilityStatesById: activeCapMap(), + }); + + assert.strictEqual(resolved.activeHooks.length, 1, 'One active contribution hook expected'); + const hook = resolved.activeHooks[0]; + + // resolveConfigKey direct result must confirm absent + const directResult = resolveConfigKey(dotKey, { config, cwd: undefined, registry }); + assert.strictEqual(directResult.found, false, 'resolveConfigKey must return found=false for absent key'); + assert.strictEqual(directResult.value, undefined); + + // When ALL aliases are absent, resolveConfigValues returns undefined → hook.configValues must be undefined + assert.strictEqual( + hook.configValues, + undefined, + 'hook.configValues must be undefined when all aliases are absent (omit-when-empty contract)', + ); + }); +}); diff --git a/tests/capability-state.test.cjs b/tests/capability-state.test.cjs index 21d2feb87..2656819db 100644 --- a/tests/capability-state.test.cjs +++ b/tests/capability-state.test.cjs @@ -1461,6 +1461,136 @@ describe('isCapabilityActive — convenience predicate (Phase 2)', () => { }); }); +// ─── resolveCapabilityRuntimeState configOverride (FIX 1 — single config snapshot) ────────────── + +describe('resolveCapabilityRuntimeState configOverride — honors caller snapshot instead of reading disk', () => { + // These tests exercise the optional third parameter `configOverride`. + // When provided, the resolver MUST use that config object for capability + // activation (active = enabled && configActivation) rather than calling + // loadConfig(cwd) internally. This guarantees that cmdLoopRenderHooks and + // the resolver use the exact same config snapshot (TOCTOU elimination). + // + // Strategy: build a synthetic registry with `activationKey: 'myfeature.enabled'` + // and use a tmpdir whose on-disk .planning/config.json carries the OPPOSITE + // value from the configOverride. The test asserts that the returned capability + // `active` matches the override, not the on-disk file. + + const { resolveCapabilityRuntimeState } = require('../gsd-core/bin/lib/capability-state.cjs'); + + let tmpDir; + let savedEnv; + + before(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cap-state-override-')); + // Save env vars that affect planningDir / config resolution + savedEnv = { + GSD_WORKSTREAM: process.env.GSD_WORKSTREAM, + GSD_PROJECT: process.env.GSD_PROJECT, + CLAUDE_CONFIG_DIR: process.env.CLAUDE_CONFIG_DIR, + }; + delete process.env.GSD_WORKSTREAM; + delete process.env.GSD_PROJECT; + delete process.env.CLAUDE_CONFIG_DIR; + + // Write on-disk config with myfeature.enabled=false (override will say true) + const planningDir = path.join(tmpDir, '.planning'); + fs.mkdirSync(planningDir, { recursive: true }); + fs.writeFileSync( + path.join(planningDir, 'config.json'), + JSON.stringify({ myfeature: { enabled: false } }), + 'utf8', + ); + }); + + after(() => { + // Restore env vars + if (savedEnv.GSD_WORKSTREAM !== undefined) process.env.GSD_WORKSTREAM = savedEnv.GSD_WORKSTREAM; + else delete process.env.GSD_WORKSTREAM; + if (savedEnv.GSD_PROJECT !== undefined) process.env.GSD_PROJECT = savedEnv.GSD_PROJECT; + else delete process.env.GSD_PROJECT; + if (savedEnv.CLAUDE_CONFIG_DIR !== undefined) process.env.CLAUDE_CONFIG_DIR = savedEnv.CLAUDE_CONFIG_DIR; + else delete process.env.CLAUDE_CONFIG_DIR; + cleanup(tmpDir); + }); + + test('configOverride=undefined (default) reads on-disk config → active=false', () => { + // Baseline: without override, loadConfig reads the on-disk file which has + // myfeature.enabled=false, so the capability is inactive. + // NOTE: resolveCapabilityRuntimeState does real I/O (profile/surface), which + // degrades gracefully on a tmpDir that has no surface marker. We only assert + // on the `active` field which is determined by the config activation leg. + // Use a simple registry with no skills so installed=surfaced=enabled=true + // (vacuously) and active depends solely on configActivation. + // + // Because resolveCapabilityRuntimeState always uses the real capability-registry.cjs, + // we cannot inject the synthetic registry into it. Instead we verify the + // config-override leg by comparing the two code paths (with and without override) + // against each other on a known config key that the real registry uses. + // + // We use the real 'graphify' capability whose activationKey='graphify.enabled'. + // On-disk: graphify.enabled=false → active=false. + // Override: { graphify: { enabled: true } } → active=true. + // + // Set up a separate tmpdir with graphify.enabled=false on disk. + const tmpCwd = fs.mkdtempSync(path.join(os.tmpdir(), 'cap-override-baseline-')); + try { + fs.mkdirSync(path.join(tmpCwd, '.planning'), { recursive: true }); + fs.writeFileSync( + path.join(tmpCwd, '.planning', 'config.json'), + JSON.stringify({ graphify: { enabled: false } }), + 'utf8', + ); + // No override → reads on-disk config → graphify active=false + const result = resolveCapabilityRuntimeState(tmpCwd, undefined); + const graphifyEntry = result.capabilities.find((c) => c.id === 'graphify'); + assert.ok(graphifyEntry, 'graphify capability must be present in the real registry'); + assert.strictEqual( + graphifyEntry.active, + false, + 'Without configOverride, on-disk graphify.enabled=false must produce active=false', + ); + } finally { + cleanup(tmpCwd); + } + }); + + test('configOverride honors caller value — overrides on-disk config (active flips from false to true)', () => { + // On-disk: graphify.enabled=false (set up in before()). + // configOverride: { graphify: { enabled: true } } ← caller wins. + // Expected: graphify active=true (override, not disk). + // + // The tmpDir from before() has graphify.enabled=false on disk. We pass + // configOverride with graphify.enabled=true. The returned active must be true. + const override = { graphify: { enabled: true } }; + const result = resolveCapabilityRuntimeState(tmpDir, undefined, override); + const graphifyEntry = result.capabilities.find((c) => c.id === 'graphify'); + assert.ok(graphifyEntry, 'graphify capability must be present in the real registry'); + assert.strictEqual( + graphifyEntry.active, + true, + 'configOverride { graphify: { enabled: true } } must produce active=true even though on-disk config has graphify.enabled=false', + ); + }); + + test('configOverride — non-existent cwd, override with enabled=true → active=true regardless of absent disk file', () => { + // When cwd points to a non-existent directory, loadConfig(cwd) would return {} + // (or throw/fallback to {}). With configOverride, the caller's value is used + // for level-1 directly. Levels 2+3 raw reads on a non-existent cwd produce + // no-hits. So: override={graphify:{enabled:true}} → level-1 hit → active=true. + // This is a clean test that avoids any interaction with the on-disk fixture in tmpDir. + const nonExistentCwd = path.join(os.tmpdir(), 'cap-override-nonexistent-' + Date.now()); + const override = { graphify: { enabled: true } }; + const result = resolveCapabilityRuntimeState(nonExistentCwd, undefined, override); + const graphifyEntry = result.capabilities.find((c) => c.id === 'graphify'); + assert.ok(graphifyEntry, 'graphify capability must be present in the real registry'); + assert.strictEqual( + graphifyEntry.active, + true, + 'configOverride { graphify: { enabled: true } } must produce active=true even with non-existent cwd (level-1 hit from override)', + ); + }); +}); + // ─── Cross-runtime runtime detection (HIGH fix — GSD_RUNTIME → config.runtime → 'claude') ────── describe('isCapabilityActive cross-runtime detection (GSD_RUNTIME → config.runtime → claude)', () => {