From 6dbd89502884af5423cf228bdb1b3b88ce3c8eb2 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 8 Jun 2026 23:37:41 -0400 Subject: [PATCH] feat(#910): federated config merge in config-loader (ADR-857 phase 3b) (#914) Build the federated config merge: each capability owns its config-key slice (ADR-857 decision 3 / ADR-894), and loadConfig merges them defensively. The registry now emits a full configSchema index ({key:{owner,type,default, description}}, generator-validated); a new src/federated-config.cts resolves federated keys defensively (skip central keys -> pending-migration warning, skip malformed slices -> warning never throw, else type-checked user override ?? default, with nested dotted-path lookup and enum validation); and loadConfig applies the overlay on every return path. Wired as a provably-empty no-op channel: every UI-pilot key is still central, so validKeys is empty and loadConfig returns byte-identical output on all paths (identity return when the overlay is empty; shared CONFIG_DEFAULTS never mutated). Registry-only; no key is cut over; nothing in the live loop changes. Closes #910 Co-authored-by: Claude Opus 4.8 --- .gitignore | 1 + CONTEXT.md | 5 +- docs/ARCHITECTURE.md | 3 +- docs/INVENTORY-MANIFEST.json | 1 + docs/INVENTORY.md | 3 +- eslint.config.mjs | 1 + gsd-core/bin/lib/capability-registry.cjs | 22 + scripts/gen-capability-registry.cjs | 124 ++++ src/config-loader.cts | 187 +++++- src/federated-config.cts | 230 +++++++ tests/capability-registry.test.cjs | 266 +++++++- tests/federated-config-loadconfig.test.cjs | 451 +++++++++++++ tests/federated-config.test.cjs | 696 +++++++++++++++++++++ 13 files changed, 1981 insertions(+), 9 deletions(-) create mode 100644 src/federated-config.cts create mode 100644 tests/federated-config-loadconfig.test.cjs create mode 100644 tests/federated-config.test.cjs diff --git a/.gitignore b/.gitignore index 66723ad00..4bb4bc1c3 100644 --- a/.gitignore +++ b/.gitignore @@ -132,6 +132,7 @@ build/ /gsd-core/bin/lib/phase-id.cjs /gsd-core/bin/lib/config-loader.cjs /gsd-core/bin/lib/model-resolver.cjs +/gsd-core/bin/lib/federated-config.cjs /gsd-core/bin/lib/phase-locator.cjs /gsd-core/bin/lib/roadmap-parser.cjs /gsd-core/bin/lib/drift.cjs diff --git a/CONTEXT.md b/CONTEXT.md index a4c408654..a7b03af7c 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -149,7 +149,10 @@ A bundle delivering one optional GSD feature, toggled as a unit at install or af Generated description of what the five-step loop (Discuss → Plan → Execute → Verify → Ship) exposes as extension points: per-step loop points, agent roles, and core artifacts. Sourced from structured `` HTML-comment markers embedded near the top of each of the five step workflow files (`discuss-phase.md`, `plan-phase.md`, `execute-phase.md`, `verify-work.md`, `ship.md`). Generated by `scripts/gen-loop-host-contract.cjs` → `gsd-core/bin/lib/loop-host-contract.cjs` (ADR-894 §3 phase 3a-impl-2). Covers exactly the 12 canonical points (discuss:pre/post, plan:pre/post, execute:pre/wave:pre/wave:post/post, verify:pre/post, ship:pre/post). The generator enforces a drift guard: every declared non-orchestrator agent role must correspond to an actual agent reference in the workflow file. Consumed by `gen-capability-registry.cjs` (replaces the former inline `LOOP_HOST_CONTRACT` constant). Run `node scripts/gen-loop-host-contract.cjs --write` after editing a workflow step marker. ### Capability Registry -Generated central manifest projecting all co-located Capability declarations into one validated artifact for runtime resolution and for the install, surface, config, and loop-extension adapters. Mirrors the research-profiles / package-identity generation pattern (co-located source → generated central file). Generated by `scripts/gen-capability-registry.cjs` → `gsd-core/bin/lib/capability-registry.cjs` (ADR-894 §5 phase 3a-impl). Role-partitioned indexes: `bySkill`, `byAgent`, `byLoopPoint` (hook ordering materialized), `configKeys`, `runtimes`, `requiresClosure(id)`. Validated against the Loop Host Contract (12 points; generated by `gen-loop-host-contract.cjs` from workflow markers, phase 3a-impl-2). Run `node scripts/gen-capability-registry.cjs --write` after editing any `capabilities//capability.json`. +Generated central manifest projecting all co-located Capability declarations into one validated artifact for runtime resolution and for the install, surface, config, and loop-extension adapters. Mirrors the research-profiles / package-identity generation pattern (co-located source → generated central file). Generated by `scripts/gen-capability-registry.cjs` → `gsd-core/bin/lib/capability-registry.cjs` (ADR-894 §5 phase 3a-impl). Role-partitioned indexes: `bySkill`, `byAgent`, `byLoopPoint` (hook ordering materialized), `configKeys` (ownership map: key→capId), `configSchema` (full per-key schema: key→{ owner, type, default, description }), `runtimes`, `requiresClosure(id)`. ADR-857 phase 3b adds `configSchema` with validated type/default/description per key, sourced from each capability's `.config` slice. Validated against the Loop Host Contract (12 points; generated by `gen-loop-host-contract.cjs` from workflow markers, phase 3a-impl-2). Run `node scripts/gen-capability-registry.cjs --write` after editing any `capabilities//capability.json`. + +### Federated Config +ADR-857 phase 3b seam that merges capability-declared config slices into the `loadConfig` return value. Implemented in `src/federated-config.cts` → `gsd-core/bin/lib/federated-config.cjs`. Exports `mergeFederatedConfig({ configSchema, isCentralKey, userConfig }) → { values, validKeys, warnings }`. Rules: central-schema keys are skipped with a `pending-migration` warning; malformed slices are skipped with a warning (never throws); valid federated keys (absent from the central schema) resolve to the user-supplied value (if type-matches) or the slice default. Object writes are guarded against prototype pollution with inline literal `__proto__`/`constructor`/`prototype` key checks. Wired into `loadConfig` as a true no-op today: every Capability config key is still in the central config-schema, so `isCentralKey()` returns true for all of them and `values` is always empty. The channel becomes live when a key is atomically removed from the central schema at cutover (the ADR-857 migration step). `loadConfig` exposes `_setFederatedRegistryForTests`/`_resetFederatedRegistryForTests` seams for injecting a synthetic registry in tests. ### Loop Extension Point [Planned] 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 (extending the `init.*` resolution seam) 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. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 4d8f313be..2328a5fa0 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -342,7 +342,8 @@ Node.js CLI utility (`gsd-tools.cjs`) with domain modules split across `gsd-core | Module | Responsibility | | ---------------------- | --------------------------------------------------------------------------------------------------- | -| `config-loader.cjs` | Project config loading — defaults merge, legacy-key migration, workstream overlay, unknown-key/profile-override validation (extracted from `core.cjs`, ADR-857) | +| `config-loader.cjs` | Project config loading — defaults merge, legacy-key migration, workstream overlay, unknown-key/profile-override validation, and federated config overlay (ADR-857 phase 3b) (extracted from `core.cjs`, ADR-857) | +| `federated-config.cjs` | Defensive merge of capability-declared config slices (ADR-857 phase 3b); exports `mergeFederatedConfig`; no-op until capability keys are removed from the central config-schema at cutover | | `core-utils.cjs` | Shared low-level utility primitives — POSIX path normalization, sub-repo/subdirectory scanning, phase file stats, slug/one-liner/plan-id helpers, time-ago (extracted from `core.cjs`, ADR-857) | | `core.cjs` | Shared utilities; compatibility re-exports for planning, I/O (`io.cjs`), and phase-id helpers | | `io.cjs` | CLI I/O primitives — output/error emission, JSON-error mode, large-payload temp-file spillover | diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index db56b51f7..a7e3bbc41 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -293,6 +293,7 @@ "docs.cjs", "drift.cjs", "fallow-runner.cjs", + "federated-config.cjs", "frontmatter.cjs", "gap-checker.cjs", "graphify.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index 481e84a93..31def503e 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -370,7 +370,7 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t --- -## CLI Modules (99 shipped) +## CLI Modules (100 shipped) Full listing: `gsd-core/bin/lib/*.cjs`. @@ -404,6 +404,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `docs.cjs` | Docs-update workflow init, Markdown scanning, monorepo detection | | `drift.cjs` | Post-execute codebase structural drift detector (#2003): classifies file changes into new-dir/barrel/migration/route categories and round-trips `last_mapped_commit` frontmatter | | `fallow-runner.cjs` | Fallow audit adapter for `/gsd-code-review`: binary resolution (`PATH` then `node_modules/.bin`), actionable missing-binary errors, and structural findings normalization | +| `federated-config.cjs` | Defensive merge of capability-declared config slices into the loadConfig return value — ADR-857 phase 3b; exports `mergeFederatedConfig({ configSchema, isCentralKey, userConfig })` → `{ values, validKeys, warnings }`; no-op until a key is atomically removed from the central config-schema (the cutover step) | | `frontmatter.cjs` | YAML frontmatter CRUD operations | | `gap-checker.cjs` | Post-planning gap analysis (#2493): unified REQUIREMENTS.md + CONTEXT.md decisions vs PLAN.md coverage report (`gsd-tools gap-analysis`) | | `graphify.cjs` | Knowledge-graph build/query/status/diff for `/gsd-graphify` | diff --git a/eslint.config.mjs b/eslint.config.mjs index f6c404288..876d53be2 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -74,6 +74,7 @@ export default tseslint.config( 'gsd-core/bin/lib/config-schema.cjs', 'gsd-core/bin/lib/model-profiles.cjs', 'gsd-core/bin/lib/model-resolver.cjs', + 'gsd-core/bin/lib/federated-config.cjs', 'gsd-core/bin/lib/installer-migrations/002-codex-legacy-hooks-json.cjs', 'gsd-core/bin/lib/installer-migrations/003-rename-get-shit-done-to-gsd-core.cjs', 'gsd-core/bin/lib/observability/logger.cjs', diff --git a/gsd-core/bin/lib/capability-registry.cjs b/gsd-core/bin/lib/capability-registry.cjs index a8ebff59f..df7bd5e41 100644 --- a/gsd-core/bin/lib/capability-registry.cjs +++ b/gsd-core/bin/lib/capability-registry.cjs @@ -207,6 +207,27 @@ const configKeys = { "workflow.ui_safety_gate": "ui" }; +const configSchema = { + "workflow.ui_phase": { + "owner": "ui", + "type": "boolean", + "default": true, + "description": "Enable the UI design-contract gate during planning." + }, + "workflow.ui_review": { + "owner": "ui", + "type": "boolean", + "default": true, + "description": "Enable the retrospective UI audit." + }, + "workflow.ui_safety_gate": { + "owner": "ui", + "type": "boolean", + "default": true, + "description": "Block execution on unmet UI-SPEC contracts." + } +}; + const runtimes = {}; const _requiresGraph = { @@ -236,6 +257,7 @@ module.exports = { byAgent, byLoopPoint, configKeys, + configSchema, runtimes, requiresClosure, }; diff --git a/scripts/gen-capability-registry.cjs b/scripts/gen-capability-registry.cjs index 36b2755ad..a9cf1861d 100644 --- a/scripts/gen-capability-registry.cjs +++ b/scripts/gen-capability-registry.cjs @@ -105,6 +105,100 @@ function loadCentralConfigKeys() { } } +// ─── Config-slice validation ────────────────────────────────────────────────── + +const VALID_CONFIG_SLICE_TYPES = new Set(['boolean', 'string', 'number', 'enum']); + +/** + * Validate a single config-slice entry (one key's { type, default, description }). + * Returns an array of error strings. Empty = valid. + * + * @param {string} capId Capability id (for error messages) + * @param {string} key Config key (for error messages) + * @param {object} slice The slice object from cap.config[key] + * @returns {string[]} + */ +function validateConfigSliceEntry(capId, key, slice) { + const errors = []; + + if (typeof slice !== 'object' || slice === null || Array.isArray(slice)) { + errors.push('capability "' + capId + '" config["' + key + '"]: slice must be a non-null object'); + return errors; + } + + // type must be one of the allowed set + if (!VALID_CONFIG_SLICE_TYPES.has(slice.type)) { + errors.push( + 'capability "' + capId + '" config["' + key + '"]: type must be one of ' + + [...VALID_CONFIG_SLICE_TYPES].join(', ') + ' (got: ' + JSON.stringify(slice.type) + ')', + ); + } + + // default must be present + if (!Object.prototype.hasOwnProperty.call(slice, 'default')) { + errors.push( + 'capability "' + capId + '" config["' + key + '"]: default is required', + ); + } else { + // type-consistency check + const def = slice.default; + if (slice.type === 'boolean') { + if (typeof def !== 'boolean') { + errors.push( + 'capability "' + capId + '" config["' + key + '"]: default must be a boolean for type:"boolean" (got: ' + typeof def + ')', + ); + } + } else if (slice.type === 'string') { + if (typeof def !== 'string') { + errors.push( + 'capability "' + capId + '" config["' + key + '"]: default must be a string for type:"string" (got: ' + typeof def + ')', + ); + } + } else if (slice.type === 'number') { + if (typeof def !== 'number') { + errors.push( + 'capability "' + capId + '" config["' + key + '"]: default must be a number for type:"number" (got: ' + typeof def + ')', + ); + } else if (!Number.isFinite(def)) { + // FIX 6a: Reject NaN and non-finite number defaults + errors.push( + 'capability "' + capId + '" config["' + key + '"]: default for type:"number" must be a finite number (got: ' + String(def) + ')', + ); + } + } else if (slice.type === 'enum') { + // FIX 5a: enum REQUIRES a non-empty values array (all strings), and default must be in it + if (!Array.isArray(slice.values) || slice.values.length === 0) { + errors.push( + 'capability "' + capId + '" config["' + key + '"]: type:"enum" requires a non-empty "values" array of strings', + ); + } else if (!slice.values.every((v) => typeof v === 'string')) { + errors.push( + 'capability "' + capId + '" config["' + key + '"]: type:"enum" values array must contain only strings', + ); + } + if (typeof def !== 'string') { + errors.push( + 'capability "' + capId + '" config["' + key + '"]: default must be a string for type:"enum" (got: ' + typeof def + ')', + ); + } else if (Array.isArray(slice.values) && slice.values.length > 0 && !slice.values.includes(def)) { + errors.push( + 'capability "' + capId + '" config["' + key + '"]: default "' + def + + '" is not one of the declared enum values [' + slice.values.join(', ') + ']', + ); + } + } + } + + // description must be a non-empty string + if (typeof slice.description !== 'string' || slice.description.length === 0) { + errors.push( + 'capability "' + capId + '" config["' + key + '"]: description must be a non-empty string (got: ' + JSON.stringify(slice.description) + ')', + ); + } + + return errors; +} + // ─── Per-capability validation ──────────────────────────────────────────────── const KEBAB_RE = /^[a-z][a-z0-9-]*$/; @@ -951,6 +1045,7 @@ function buildRegistry(capMap) { const byAgent = Object.create(null); const byLoopPoint = Object.create(null); const configKeys = Object.create(null); + const configSchema = Object.create(null); const runtimes = Object.create(null); // Initialize byLoopPoint for all valid points @@ -989,6 +1084,29 @@ function buildRegistry(capMap) { // S2b: inline literal guard at each write site (CodeQL barrier) if (key === '__proto__' || key === 'constructor' || key === 'prototype') continue; configKeys[key] = capId; + + // Build configSchema entry — validate the slice first (throw on violation) + const slice = (cap.config || {})[key]; + const sliceErrors = validateConfigSliceEntry(capId, key, slice); + if (sliceErrors.length > 0) { + throw new Error( + 'configSchema validation failed during registry build:\n' + + sliceErrors.map((e) => ' ' + e).join('\n'), + ); + } + // S2b: inline literal guard for configSchema write site + if (key !== '__proto__' && key !== 'constructor' && key !== 'prototype') { + configSchema[key] = { + owner: capId, + type: slice.type, + default: slice.default, + description: slice.description, + }; + // Preserve values array for enum types if present + if (slice.type === 'enum' && Array.isArray(slice.values)) { + configSchema[key].values = slice.values; + } + } } for (const step of (cap.steps || [])) { @@ -1050,6 +1168,7 @@ function buildRegistry(capMap) { byAgent, byLoopPoint, configKeys, + configSchema, runtimes, }; } @@ -1085,6 +1204,8 @@ function serializeRegistry(registry, capMap) { lines.push(''); lines.push('const configKeys = ' + JSON.stringify(registry.configKeys, null, 2) + ';'); lines.push(''); + lines.push('const configSchema = ' + JSON.stringify(registry.configSchema, null, 2) + ';'); + lines.push(''); lines.push('const runtimes = ' + JSON.stringify(registry.runtimes, null, 2) + ';'); lines.push(''); @@ -1121,6 +1242,7 @@ function serializeRegistry(registry, capMap) { lines.push(' byAgent,'); lines.push(' byLoopPoint,'); lines.push(' configKeys,'); + lines.push(' configSchema,'); lines.push(' runtimes,'); lines.push(' requiresClosure,'); lines.push('};'); @@ -1280,6 +1402,8 @@ module.exports = { computeRequiresClosure, topoSortSteps, normalizeLineEndings, + validateConfigSliceEntry, + VALID_CONFIG_SLICE_TYPES, LOOP_HOST_CONTRACT, VALID_LOOP_POINTS, POINT_ORDER, diff --git a/src/config-loader.cts b/src/config-loader.cts index 67982afee..942c9fd3c 100644 --- a/src/config-loader.cts +++ b/src/config-loader.cts @@ -35,8 +35,30 @@ const { detectSubRepos } = coreUtilsModule; import { CONFIG_DEFAULTS as CANONICAL_CONFIG_DEFAULTS, normalizeLegacyKeys } from './configuration.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import configSchema = require('./config-schema.cjs'); -const { VALID_CONFIG_KEYS, DYNAMIC_KEY_PATTERNS } = configSchema; +const { VALID_CONFIG_KEYS, DYNAMIC_KEY_PATTERNS, isValidConfigKey: _isValidConfigKeyFn } = configSchema; import { KNOWN_RUNTIMES, KNOWN_PROVIDERS } from './model-catalog.cjs'; +// ─── Federated Config (ADR-857 phase 3b) ───────────────────────────────────── +// eslint-disable-next-line @typescript-eslint/no-require-imports +import federatedConfigModule = require('./federated-config.cjs'); +const { mergeFederatedConfig } = federatedConfigModule; +// The capability-registry.cjs is generated and lives in the same gsd-core/bin/lib/ output dir. +// Both config-loader.cjs and capability-registry.cjs land in gsd-core/bin/lib/ at build time. +// eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-unsafe-assignment +const _capabilityRegistryReal: { configSchema?: Record } = require('./capability-registry.cjs'); + +// Module-level registry reference. Defaults to the real generated registry. +// Overridable for tests via _setFederatedRegistryForTests. +let _capabilityRegistry: { configSchema?: Record } = _capabilityRegistryReal; + +/** Test-only seam: inject a synthetic registry. Call _resetFederatedRegistryForTests() to restore. */ +function _setFederatedRegistryForTests(reg: { configSchema?: Record }): void { + _capabilityRegistry = reg; +} + +/** Test-only seam: restore the real generated registry. */ +function _resetFederatedRegistryForTests(): void { + _capabilityRegistry = _capabilityRegistryReal; +} // ─── File & Config utilities ────────────────────────────────────────────────── @@ -272,6 +294,81 @@ function _resetRuntimeWarningCacheForTests(): void { _warnedConfigKeys.clear(); } +// ─── FIX 2: Federated overlay helpers ──────────────────────────────────────── + +/** + * Apply federated key values into a mutable config object. + * Handles N-level dotted keys (e.g. "a.b.c" → obj.a.b.c). + * Only adds keys that are not already present (does not clobber). + * Inline prototype-pollution guards at every segment. + */ +function _applyFederatedValues( + obj: Record, + values: Record, + validKeys: string[], +): void { + for (const dottedKey of validKeys) { + // S2: inline literal guard on full key + if (dottedKey === '__proto__' || dottedKey === 'constructor' || dottedKey === 'prototype') continue; + const parts = dottedKey.split('.'); + if (parts.length === 1) { + const topKey = parts[0]; + if (topKey !== '__proto__' && topKey !== 'constructor' && topKey !== 'prototype') { + if (!Object.prototype.hasOwnProperty.call(obj, topKey)) { + obj[topKey] = values[dottedKey]; + } + } + } else { + // N-level nested key: traverse/create intermediate objects + let cur: Record = obj; + let ok = true; + for (let i = 0; i < parts.length - 1; i++) { + const seg = parts[i]; + // S2: inline literal guard on each segment + if (seg === '__proto__' || seg === 'constructor' || seg === 'prototype') { ok = false; break; } + if (!Object.prototype.hasOwnProperty.call(cur, seg) || cur[seg] === null) { + cur[seg] = {}; + } + if (typeof cur[seg] !== 'object' || Array.isArray(cur[seg])) { ok = false; break; } + cur = cur[seg] as Record; + } + if (!ok) continue; + const leafKey = parts[parts.length - 1]; + // S2: inline literal guard on leaf + if (leafKey === '__proto__' || leafKey === 'constructor' || leafKey === 'prototype') continue; + if (!Object.prototype.hasOwnProperty.call(cur, leafKey)) { + cur[leafKey] = values[dottedKey]; + } + } + } +} + +/** + * FIX 2: Apply the federated overlay to a base config object. + * When validKeys is empty (current registry — all keys are central), + * returns the baseConfig UNCHANGED (true no-op, preserves reference identity). + * When validKeys is non-empty, applies values into a shallow clone to avoid + * mutating shared CONFIG_DEFAULTS/module constants. + */ +function _applyFederatedOverlay( + baseConfig: Record, + userConfig: Record, +): Record { + const _fedRegistrySchema = _capabilityRegistry.configSchema; + if (!_fedRegistrySchema || typeof _fedRegistrySchema !== 'object') return baseConfig; + const _fedOverlay = mergeFederatedConfig({ + configSchema: _fedRegistrySchema, + isCentralKey: (key: string) => _isValidConfigKeyFn(key), + userConfig, + }); + // True no-op: if no federated keys, return UNCHANGED (byte-identical, no clone) + if (_fedOverlay.validKeys.length === 0) return baseConfig; + // Clone shallowly to avoid mutating shared constants, then apply nested values + const cloned: Record = { ...baseConfig }; + _applyFederatedValues(cloned, _fedOverlay.values, _fedOverlay.validKeys); + return cloned; +} + function loadConfig(cwd: string, options: Record = {}): Record { const activeWorkstream = Object.prototype.hasOwnProperty.call(options, 'workstream') ? options['workstream'] @@ -397,6 +494,31 @@ function loadConfig(cwd: string, options: Record = {}): Record< // Deprecated keys (still accepted for migration, not in config-set) 'depth', 'multiRepo', 'branching_strategy', ]); + + // FIX 3: Compute federated overlay BEFORE the unknown-key warning, so that + // federated top-level keys are added to KNOWN_TOP_LEVEL before the check runs. + // This is hoisted out of the try-catch below so validKeys are available here. + let _preWarningFedValidKeys: string[] = []; + try { + const _fedRegistrySchemaEarly = _capabilityRegistry.configSchema; + if (_fedRegistrySchemaEarly && typeof _fedRegistrySchemaEarly === 'object') { + const _earlyOverlay = mergeFederatedConfig({ + configSchema: _fedRegistrySchemaEarly, + isCentralKey: (key: string) => _isValidConfigKeyFn(key), + userConfig: parsed, + }); + _preWarningFedValidKeys = _earlyOverlay.validKeys; + for (const dottedKey of _preWarningFedValidKeys) { + const topKey = dottedKey.split('.')[0]; + if (topKey !== '__proto__' && topKey !== 'constructor' && topKey !== 'prototype') { + KNOWN_TOP_LEVEL.add(topKey); + } + } + } + } catch { + // Defensive: if registry access fails here, proceed without pre-warning keys + } + const unknownKeys = Object.keys(parsed).filter(k => !KNOWN_TOP_LEVEL.has(k)); if (unknownKeys.length > 0) { const warnKey = unknownKeys.join(','); @@ -429,7 +551,7 @@ function loadConfig(cwd: string, options: Record = {}): Record< return defaults.parallelization; })(); - return { + const _baseConfig: Record = { model_profile: get('model_profile') ?? defaults.model_profile, commit_docs: (() => { const explicit = get('commit_docs', { section: 'planning', field: 'commit_docs' }); @@ -492,14 +614,54 @@ function loadConfig(cwd: string, options: Record = {}): Record< claude_md_path: get('claude_md_path') || null, claude_md_assembly: (parsed['claude_md_assembly']) || null, }; + + // ─── ADR-857 phase 3b: federated config overlay ─────────────────────────── + // FIX 2: Use the pre-computed _preWarningFedValidKeys (from the FIX 3 block above) + // plus a fresh overlay call to get values. The KNOWN_TOP_LEVEL was already updated. + // TODAY: every UI key is still in the central config-schema, so isCentralKey() + // returns true for all of them → validKeys is empty → _baseConfig is returned UNCHANGED + // (true no-op: no clone, no reorder, byte-identical output). + // This becomes a live channel once a key is atomically removed from the central schema. + try { + if (_preWarningFedValidKeys.length > 0) { + // There are actual federated values — re-use the already-computed overlay + // (we run mergeFederatedConfig again here to get the values map; the validKeys + // are guaranteed identical since it's the same inputs). + const _fedRegistrySchema = _capabilityRegistry.configSchema; + if (_fedRegistrySchema && typeof _fedRegistrySchema === 'object') { + const _fedOverlay = mergeFederatedConfig({ + configSchema: _fedRegistrySchema, + isCentralKey: (key: string) => _isValidConfigKeyFn(key), + userConfig: parsed, + }); + // Apply dotted-path values (e.g. "workflow.ui_phase" → _baseConfig.workflow.ui_phase) + // WITHOUT clobbering existing keys. N-level nesting supported. + _applyFederatedValues(_baseConfig, _fedOverlay.values, _fedOverlay.validKeys); + } + } + // Pending-migration warnings are suppressed at load time to avoid noisy output on + // every loadConfig call. They are surfaced at registry-generation time (--check/--write). + } catch { + // Defensive: if the federated overlay throws for any reason, return the base config unchanged. + // This keeps loadConfig's no-throw contract intact regardless of capability registry state. + } + return _baseConfig; } catch { // Fall back to ~/.gsd/defaults.json only for truly pre-project contexts (#1683) if (fs.existsSync(planningDir(cwd, ws))) { if (rootParsed) { // Workstream has no config.json: re-parse using root config as the sole source. + // (FIX 2: overlay is applied recursively in the re-entrant loadConfig call) return loadConfig(cwd, { workstream: null }); } - return defaults; + // FIX 2: Apply the federated overlay on the no-config path. + // With the current registry (all keys central), _applyFederatedOverlay returns + // `defaults` UNCHANGED (true no-op, preserves byte-identical output). + try { + return _applyFederatedOverlay(defaults, {}); + } catch { + return defaults; + } } try { const home = process.env['GSD_HOME'] || os.homedir(); @@ -507,7 +669,7 @@ function loadConfig(cwd: string, options: Record = {}): Record< const raw = platformReadSync(globalDefaultsPath); if (raw === null) throw new Error('missing'); const globalDefaults = JSON.parse(raw) as Record; - return { + const _globalBaseCfg: Record = { ...defaults, model_profile: (globalDefaults['model_profile']) ?? defaults.model_profile, commit_docs: (globalDefaults['commit_docs']) ?? defaults.commit_docs, @@ -534,8 +696,21 @@ function loadConfig(cwd: string, options: Record = {}): Record< agent_skills: (globalDefaults['agent_skills']) || {}, response_language: (globalDefaults['response_language']) || null, }; + // FIX 2: Apply federated overlay on global-defaults path. + // With the current registry this is a true no-op (returns _globalBaseCfg unchanged). + try { + return _applyFederatedOverlay(_globalBaseCfg, globalDefaults); + } catch { + return _globalBaseCfg; + } } catch { - return defaults; + // FIX 2: Apply federated overlay on the final fallback path. + // With the current registry this is a true no-op (returns `defaults` unchanged). + try { + return _applyFederatedOverlay(defaults, {}); + } catch { + return defaults; + } } } } @@ -553,4 +728,6 @@ export = { _warnedConfigKeys, _gitIgnoredCache, RUNTIME_OVERRIDE_TIERS, + _setFederatedRegistryForTests, + _resetFederatedRegistryForTests, }; diff --git a/src/federated-config.cts b/src/federated-config.cts new file mode 100644 index 000000000..8ca0bac49 --- /dev/null +++ b/src/federated-config.cts @@ -0,0 +1,230 @@ +/** + * Federated Config — Defensive merge of capability-declared config keys + * + * ADR-857 phase 3b: wires the Capability Registry's configSchema into + * loadConfig as a provably-empty no-op channel until capability keys are + * migrated out of the central config-schema. + * + * Exported function: + * mergeFederatedConfig({ configSchema, isCentralKey, userConfig }) + * → { values, validKeys, warnings } + * + * Design: + * - For each key in configSchema: + * If isCentralKey(key) → SKIP; push a pending-migration warning. + * Else if slice is malformed → SKIP; push a warning. Never throw. + * Else (valid federated key absent from central): + * resolvedValue = nested userConfig lookup if present & type-matches; else slice.default. + * Add key→resolvedValue to values; add key to validKeys. + * - Guard all object writes with inline literal __proto__/constructor/prototype checks. + * - Zero external dependencies; no ajv; hand-rolled type checks only. + * + * ADR-857 no-op guarantee: + * With the current registry, every UI key is still present in the central + * config-schema, so isCentralKey() returns true for all of them and values + * is always empty. The channel is live but carries no traffic until a key + * is atomically removed from the central schema (the cutover step). + * + * Dependencies: none (zero-dep module). + */ + +// ─── Types ───────────────────────────────────────────────────────────────────── + +/** Shape of one entry in the capability registry's configSchema index. */ +interface ConfigSliceEntry { + owner: string; + type: string; + default: unknown; + description: string; + values?: string[]; // required for enum type + [key: string]: unknown; +} + +interface MergeFederatedConfigInput { + /** configSchema index from the capability registry: { [key]: ConfigSliceEntry } */ + configSchema: Record; + /** Returns true if the given key is owned by the central config-schema. */ + isCentralKey: (key: string) => boolean; + /** The raw/merged user config object already loaded in loadConfig. */ + userConfig: Record; +} + +interface MergeFederatedConfigResult { + /** Resolved values for federated (non-central) keys: { key → resolvedValue } */ + values: Record; + /** Array of keys that are now valid federated keys (i.e. were added to values). */ + validKeys: string[]; + /** Human-readable diagnostic strings (pending-migration, malformed-slice, type-mismatch). */ + warnings: string[]; +} + +// ─── Allowed slice types (mirrors gen-capability-registry.cjs VALID_CONFIG_SLICE_TYPES) ── + +const VALID_SLICE_TYPES = new Set(['boolean', 'string', 'number', 'enum']); + +// ─── Internal helpers ────────────────────────────────────────────────────────── + +/** + * Returns true if `slice` has a non-empty type, a `default` property, and a + * non-empty string description. Does NOT throw. + */ +function _isWellFormedSlice(slice: unknown): slice is ConfigSliceEntry { + if (typeof slice !== 'object' || slice === null || Array.isArray(slice)) return false; + const s = slice as Record; + if (typeof s['type'] !== 'string' || s['type'].length === 0) return false; + if (!VALID_SLICE_TYPES.has(s['type'])) return false; + if (!Object.prototype.hasOwnProperty.call(s, 'default')) return false; + return true; +} + +/** + * Returns true if `value` matches the declared type in the slice. + * For enum, also validates against slice.values if present. + */ +function _typeMatches(value: unknown, slice: ConfigSliceEntry): boolean { + switch (slice.type) { + case 'boolean': return typeof value === 'boolean'; + case 'string': return typeof value === 'string'; + case 'number': return typeof value === 'number'; + case 'enum': + // Must be a string AND, if values list is present, must be in it + if (typeof value !== 'string') return false; + if (Array.isArray(slice.values) && slice.values.length > 0) { + return slice.values.includes(value); + } + return true; + default: return false; + } +} + +/** + * Traverse a dotted key path through a nested config object. + * E.g. key="workflow.ui_phase", obj={workflow:{ui_phase:false}} → {found:true, value:false} + * Returns {found:false} if any segment is missing or not an own property. + * Handles 1, 2, or N segments generically. + */ +function _getNestedValue(obj: Record, key: string): { found: boolean; value: unknown } { + const segments = key.split('.'); + let current: unknown = obj; + for (let i = 0; i < segments.length; i++) { + const seg = segments[i]; + // Inline literal prototype-pollution guard + 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 }; +} + +// ─── Public API ──────────────────────────────────────────────────────────────── + +/** + * Defensive merge of capability-declared config slices into the loadConfig + * return value. + * + * DEFENSIVE contract (never throws, even on bad capability data): + * - Null/undefined/non-object input → returns empty result. + * - Null/undefined/non-object userConfig → treated as {} (no overrides). + * - Central keys are skipped with a pending-migration warning. + * - Malformed slices are skipped with a warning. + * - User-supplied values with wrong types (or out-of-enum values) fall back + * to the slice default (a type-mismatch warning is pushed but the key is + * still federated with its default; this is best-effort degraded operation). + */ +function mergeFederatedConfig(input: MergeFederatedConfigInput): MergeFederatedConfigResult { + // FIX 4: Guard null/undefined/non-object input + if (input === null || input === undefined || typeof input !== 'object') { + return { values: Object.create(null) as Record, validKeys: [], warnings: [] }; + } + + const { configSchema, isCentralKey } = input; + + // FIX 4: Guard null/undefined/non-object userConfig — treat as {} + const userConfig: Record = + (input.userConfig !== null && input.userConfig !== undefined && typeof input.userConfig === 'object' && !Array.isArray(input.userConfig)) + ? input.userConfig + : {}; + + // FIX 6b: Use null-prototype object for all return paths + const values: Record = Object.create(null) as Record; + const validKeys: string[] = []; + const warnings: string[] = []; + + if (typeof configSchema !== 'object' || configSchema === null) { + return { values: Object.create(null) as Record, validKeys: [], warnings: [] }; + } + + for (const key of Object.keys(configSchema)) { + // S2: inline literal prototype-pollution guard (CodeQL barrier) + // Guard both the full key AND all dotted-path segments + if (key === '__proto__' || key === 'constructor' || key === 'prototype') continue; + const _keySegments = key.split('.'); + if (_keySegments.some((s) => s === '__proto__' || s === 'constructor' || s === 'prototype')) continue; + + const slice = configSchema[key]; + + // If this key is still in the central schema → pending migration, skip + try { + if (isCentralKey(key)) { + warnings.push( + 'federated-config: key "' + key + '" is still in the central config-schema (pending-migration); ' + + 'skipping federated resolution until the central schema entry is removed', + ); + continue; + } + } catch { + // isCentralKey threw — treat as unknown, skip defensively + warnings.push('federated-config: isCentralKey("' + key + '") threw; skipping key'); + continue; + } + + // Validate slice shape — skip malformed entries + if (!_isWellFormedSlice(slice)) { + warnings.push( + 'federated-config: config slice for key "' + key + '" is malformed (missing or invalid type/default); skipping', + ); + continue; + } + + const sliceEntry = slice; + + // FIX 1: Resolve value using NESTED dotted-path lookup through userConfig + let resolvedValue: unknown = sliceEntry.default; + const { found: userHasKey, value: userValue } = _getNestedValue(userConfig, key); + if (userHasKey && userValue !== undefined) { + // FIX 5b: For enum, validate against slice.values if present; otherwise check type + if (_typeMatches(userValue, sliceEntry)) { + resolvedValue = userValue; + } else { + const typeDesc = sliceEntry.type === 'enum' && Array.isArray(sliceEntry.values) + ? 'enum(' + sliceEntry.values.join('|') + ')' + : sliceEntry.type; + warnings.push( + 'federated-config: user-supplied value for "' + key + '" has wrong type or invalid enum value ' + + '(expected ' + typeDesc + ', got ' + typeof userValue + + (typeof userValue === 'string' ? ' "' + String(userValue) + '"' : '') + + '); falling back to slice default', + ); + // resolvedValue stays as slice default + } + } + + // S2: inline literal guard before writing to values + if (key !== '__proto__' && key !== 'constructor' && key !== 'prototype') { + values[key] = resolvedValue; + validKeys.push(key); + } + } + + return { values, validKeys, warnings }; +} + +export = { mergeFederatedConfig }; diff --git a/tests/capability-registry.test.cjs b/tests/capability-registry.test.cjs index 7c2994d5f..fb204587a 100644 --- a/tests/capability-registry.test.cjs +++ b/tests/capability-registry.test.cjs @@ -29,6 +29,8 @@ const { computeRequiresClosure, topoSortSteps, normalizeLineEndings, + validateConfigSliceEntry, + VALID_CONFIG_SLICE_TYPES, SCHEMA_VERSION, } = require('../scripts/gen-capability-registry.cjs'); @@ -101,10 +103,33 @@ describe('UI pilot capability', () => { assert.strictEqual(uiGate.capId, 'ui'); assert.strictEqual(uiGate.blocking, true); - // configKeys maps the 3 UI keys to 'ui' + // configKeys maps the 3 UI keys to 'ui' (ownership map — preserved) assert.strictEqual(registry.configKeys['workflow.ui_phase'], 'ui'); assert.strictEqual(registry.configKeys['workflow.ui_review'], 'ui'); assert.strictEqual(registry.configKeys['workflow.ui_safety_gate'], 'ui'); + + // configSchema index — new in phase 3b + assert.ok(registry.configSchema, 'registry.configSchema should exist'); + + // workflow.ui_phase + assert.ok(registry.configSchema['workflow.ui_phase'], 'configSchema should have workflow.ui_phase'); + assert.strictEqual(registry.configSchema['workflow.ui_phase'].owner, 'ui'); + assert.strictEqual(registry.configSchema['workflow.ui_phase'].type, 'boolean'); + assert.strictEqual(registry.configSchema['workflow.ui_phase'].default, true); + assert.strictEqual(typeof registry.configSchema['workflow.ui_phase'].description, 'string'); + assert.ok(registry.configSchema['workflow.ui_phase'].description.length > 0); + + // workflow.ui_review + assert.ok(registry.configSchema['workflow.ui_review'], 'configSchema should have workflow.ui_review'); + assert.strictEqual(registry.configSchema['workflow.ui_review'].owner, 'ui'); + assert.strictEqual(registry.configSchema['workflow.ui_review'].type, 'boolean'); + assert.strictEqual(registry.configSchema['workflow.ui_review'].default, true); + + // workflow.ui_safety_gate + assert.ok(registry.configSchema['workflow.ui_safety_gate'], 'configSchema should have workflow.ui_safety_gate'); + assert.strictEqual(registry.configSchema['workflow.ui_safety_gate'].owner, 'ui'); + assert.strictEqual(registry.configSchema['workflow.ui_safety_gate'].type, 'boolean'); + assert.strictEqual(registry.configSchema['workflow.ui_safety_gate'].default, true); }); test('requiresClosure("ui") returns empty set (no requires)', () => { @@ -1200,6 +1225,7 @@ describe('FIX 1: self-consume rejection in validateConsumesGlobal', () => { ); }); + // (this test follows the series above) test('a step produces:["SELF.md"] and consumes:["SELF.md"] and another capability produces SELF.md at the SAME point is accepted (different hook)', () => { const producerCap = { id: 'producer-cap', role: 'feature', title: 'Producer', description: 'Produces SELF.md', @@ -1240,3 +1266,241 @@ describe('FIX 1: self-consume rejection in validateConsumesGlobal', () => { ); }); }); + +// ─── 14. configSchema emission (ADR-857 phase 3b) ──────────────────────────── + +describe('configSchema emission (ADR-857 phase 3b)', () => { + test('buildRegistry emits configSchema with correct shape for UI pilot', () => { + const capDir = makeTempCapDir({ ui: UI_CAP }); + const { capMap, errors } = loadAndValidate(new Set(), capDir); + assert.deepEqual(errors, [], 'No errors expected'); + + const registry = buildRegistry(capMap); + assert.ok(registry.configSchema, 'registry.configSchema must exist'); + + const uiPhase = registry.configSchema['workflow.ui_phase']; + assert.ok(uiPhase, 'configSchema must have workflow.ui_phase'); + assert.strictEqual(uiPhase.owner, 'ui'); + assert.strictEqual(uiPhase.type, 'boolean'); + assert.strictEqual(uiPhase.default, true); + assert.ok(typeof uiPhase.description === 'string' && uiPhase.description.length > 0); + + const uiReview = registry.configSchema['workflow.ui_review']; + assert.ok(uiReview, 'configSchema must have workflow.ui_review'); + assert.strictEqual(uiReview.owner, 'ui'); + assert.strictEqual(uiReview.type, 'boolean'); + + const uiSafetyGate = registry.configSchema['workflow.ui_safety_gate']; + assert.ok(uiSafetyGate, 'configSchema must have workflow.ui_safety_gate'); + assert.strictEqual(uiSafetyGate.type, 'boolean'); + }); + + test('serializeRegistry emits a configSchema block in the generated .cjs', () => { + const capDir = makeTempCapDir({ ui: UI_CAP }); + const { capMap } = loadAndValidate(new Set(), capDir); + const registry = buildRegistry(capMap); + const content = serializeRegistry(registry, capMap); + + assert.ok(content.includes('const configSchema'), 'Generated file must contain "const configSchema"'); + assert.ok(content.includes('"workflow.ui_phase"'), 'Generated file must contain "workflow.ui_phase"'); + assert.ok(content.includes('"owner"'), 'Generated file must contain "owner" field'); + assert.ok(content.includes('"type"'), 'Generated file must contain "type" field'); + assert.ok(content.includes('"default"'), 'Generated file must contain "default" field'); + assert.ok(content.includes('"description"'), 'Generated file must contain "description" field'); + assert.ok(content.includes('configSchema,'), 'Generated module.exports must include configSchema'); + }); + + test('committed capability-registry.cjs has configSchema with correct shape', () => { + const registry = require('../gsd-core/bin/lib/capability-registry.cjs'); + assert.ok(registry.configSchema, 'capability-registry.cjs must export configSchema'); + + const uiPhase = registry.configSchema['workflow.ui_phase']; + assert.ok(uiPhase, 'committed registry configSchema must have workflow.ui_phase'); + assert.strictEqual(uiPhase.owner, 'ui', 'owner must be "ui"'); + assert.strictEqual(uiPhase.type, 'boolean', 'type must be "boolean"'); + assert.strictEqual(uiPhase.default, true, 'default must be true'); + assert.ok(typeof uiPhase.description === 'string' && uiPhase.description.length > 0); + }); +}); + +// ─── 15. validateConfigSliceEntry adversarial tests ─────────────────────────── + +describe('validateConfigSliceEntry adversarial cases (ADR-857 phase 3b)', () => { + const CAP_ID = 'test-cap'; + const KEY = 'test.key'; + + test('VALID_CONFIG_SLICE_TYPES exports expected types', () => { + const types = [...VALID_CONFIG_SLICE_TYPES]; + assert.ok(types.includes('boolean'), 'Must include boolean'); + assert.ok(types.includes('string'), 'Must include string'); + assert.ok(types.includes('number'), 'Must include number'); + assert.ok(types.includes('enum'), 'Must include enum'); + assert.strictEqual(types.length, 4, 'Must have exactly 4 types'); + }); + + test('valid boolean slice passes validation', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'boolean', default: true, description: 'ok' }); + assert.deepEqual(errors, [], 'Valid boolean slice should produce no errors, got: ' + JSON.stringify(errors)); + }); + + test('valid string slice passes validation', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'string', default: 'x', description: 'ok' }); + assert.deepEqual(errors, []); + }); + + test('valid number slice passes validation', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'number', default: 5, description: 'ok' }); + assert.deepEqual(errors, []); + }); + + test('REJECTED: enum slice without values list → error (FIX 5a: values required)', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'enum', default: 'x', description: 'ok' }); + assert.ok(errors.length > 0, 'Expected rejection for enum without values list, got: ' + JSON.stringify(errors)); + assert.ok( + errors.some((e) => e.includes('values') || e.includes('enum')), + 'Error should mention values or enum, got: ' + JSON.stringify(errors), + ); + }); + + test('valid enum slice (with values list, default in values) passes', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { + type: 'enum', default: 'b', values: ['a', 'b', 'c'], description: 'ok', + }); + assert.deepEqual(errors, []); + }); + + test('REJECTED: bad type ("xml") → error mentioning type', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'xml', default: '', description: 'ok' }); + assert.ok(errors.length > 0, 'Expected rejection for bad type'); + assert.ok(errors.some((e) => e.includes('type')), 'Error should mention type, got: ' + JSON.stringify(errors)); + }); + + test('REJECTED: missing type → error', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { default: true, description: 'ok' }); + assert.ok(errors.length > 0, 'Expected rejection for missing type'); + assert.ok(errors.some((e) => e.includes('type'))); + }); + + test('REJECTED: missing default → error mentioning default', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'boolean', description: 'ok' }); + assert.ok(errors.length > 0, 'Expected rejection for missing default'); + assert.ok(errors.some((e) => e.includes('default')), 'Error should mention default, got: ' + JSON.stringify(errors)); + }); + + test('REJECTED: boolean type with string default → error', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'boolean', default: 'true', description: 'ok' }); + assert.ok(errors.length > 0, 'Expected rejection for boolean type with string default'); + assert.ok(errors.some((e) => e.includes('boolean') || e.includes('default'))); + }); + + test('REJECTED: string type with boolean default → error', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'string', default: false, description: 'ok' }); + assert.ok(errors.length > 0, 'Expected rejection for string type with boolean default'); + }); + + test('REJECTED: number type with string default → error', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'number', default: 'five', description: 'ok' }); + assert.ok(errors.length > 0, 'Expected rejection for number type with string default'); + }); + + test('REJECTED: enum type with values list, default not in values → error', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { + type: 'enum', default: 'z', values: ['a', 'b', 'c'], description: 'ok', + }); + assert.ok(errors.length > 0, 'Expected rejection for enum default not in values'); + assert.ok(errors.some((e) => e.includes('enum') || e.includes('values') || e.includes('z'))); + }); + + test('REJECTED: enum type with non-string default → error', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'enum', default: 42, description: 'ok' }); + assert.ok(errors.length > 0, 'Expected rejection for enum with non-string default'); + }); + + test('REJECTED: empty description string → error', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'boolean', default: true, description: '' }); + assert.ok(errors.length > 0, 'Expected rejection for empty description'); + assert.ok(errors.some((e) => e.includes('description'))); + }); + + test('REJECTED: non-string description (number) → error', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'boolean', default: true, description: 42 }); + assert.ok(errors.length > 0, 'Expected rejection for non-string description'); + assert.ok(errors.some((e) => e.includes('description'))); + }); + + test('REJECTED: missing description → error', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'boolean', default: true }); + assert.ok(errors.length > 0, 'Expected rejection for missing description'); + assert.ok(errors.some((e) => e.includes('description'))); + }); + + test('REJECTED: null slice → error', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, null); + assert.ok(errors.length > 0, 'Expected rejection for null slice'); + }); + + test('REJECTED: array slice → error', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, []); + assert.ok(errors.length > 0, 'Expected rejection for array slice'); + }); + + // FIX 5a: enum-without-values and default-not-in-values + test('REJECTED: enum with empty values array → error (FIX 5a)', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'enum', default: 'x', values: [], description: 'ok' }); + assert.ok(errors.length > 0, 'Expected rejection for enum with empty values, got: ' + JSON.stringify(errors)); + assert.ok(errors.some((e) => e.includes('values') || e.includes('enum'))); + }); + + test('REJECTED: enum with non-string values array entries → error (FIX 5a)', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'enum', default: 'x', values: ['a', 42], description: 'ok' }); + assert.ok(errors.length > 0, 'Expected rejection for enum with non-string values, got: ' + JSON.stringify(errors)); + assert.ok(errors.some((e) => e.includes('values') || e.includes('string'))); + }); + + test('REJECTED: enum default not in values → error (FIX 5a)', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'enum', default: 'z', values: ['a', 'b'], description: 'ok' }); + assert.ok(errors.length > 0, 'Expected rejection for enum default not in values, got: ' + JSON.stringify(errors)); + assert.ok(errors.some((e) => e.includes('z') || e.includes('values') || e.includes('default'))); + }); + + // FIX 6a: NaN and non-finite number defaults + test('REJECTED: NaN number default → error (FIX 6a)', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'number', default: NaN, description: 'ok' }); + assert.ok(errors.length > 0, 'Expected rejection for NaN default, got: ' + JSON.stringify(errors)); + assert.ok(errors.some((e) => e.includes('finite') || e.includes('NaN') || e.includes('number'))); + }); + + test('REJECTED: Infinity number default → error (FIX 6a)', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'number', default: Infinity, description: 'ok' }); + assert.ok(errors.length > 0, 'Expected rejection for Infinity default, got: ' + JSON.stringify(errors)); + assert.ok(errors.some((e) => e.includes('finite') || e.includes('number'))); + }); + + test('REJECTED: -Infinity number default → error (FIX 6a)', () => { + const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'number', default: -Infinity, description: 'ok' }); + assert.ok(errors.length > 0, 'Expected rejection for -Infinity default, got: ' + JSON.stringify(errors)); + }); + + test('buildRegistry throws on malformed config slice in capability', () => { + // A capability with a config slice that has a missing default — buildRegistry must throw + const cap = { + ...UI_CAP, + config: { + ...UI_CAP.config, + 'workflow.bad_key': { type: 'boolean', description: 'missing default' }, + }, + }; + const capMap = new Map([['ui', cap]]); + assert.throws( + () => buildRegistry(capMap), + (err) => { + assert.ok(err instanceof Error, 'Must throw an Error'); + assert.ok( + err.message.includes('configSchema') || err.message.includes('default') || err.message.includes('validation'), + 'Error must mention configSchema validation, got: ' + err.message, + ); + return true; + }, + ); + }); +}); diff --git a/tests/federated-config-loadconfig.test.cjs b/tests/federated-config-loadconfig.test.cjs new file mode 100644 index 000000000..36e7f8134 --- /dev/null +++ b/tests/federated-config-loadconfig.test.cjs @@ -0,0 +1,451 @@ +'use strict'; + +/** + * federated-config-loadconfig.test.cjs — Tests for the federated config overlay + * wired into loadConfig (ADR-857 phase 3b). + * + * Tests: + * 1. EQUIVALENCE/no-op: with the real registry, loadConfig output has NO unexpected + * extra keys (the UI keys are central so the overlay is empty). + * 2. FIXTURE federated key: inject a synthetic configSchema with a key NOT in + * central schema → loadConfig surfaces it with its default. + * 3. FIXTURE federated key with user override: user config sets the federated key + * to a valid value → that value is used. + * 4. MALFORMED registry: configSchema with bad slices → loadConfig returns a valid + * config without throwing. + */ + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); + +const { cleanup } = require('./helpers.cjs'); + +// ─── Module under test ──────────────────────────────────────────────────────── + +const configLoader = require('../gsd-core/bin/lib/config-loader.cjs'); +const { + loadConfig, + _setFederatedRegistryForTests, + _resetFederatedRegistryForTests, +} = configLoader; + +// ─── Helpers ────────────────────────────────────────────────────────────────── + +function makeTempProject() { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-fed-cfg-test-')); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true }); + return tmpDir; +} + +function writeConfig(tmpDir, obj) { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'config.json'), + JSON.stringify(obj, null, 2), + 'utf-8', + ); +} + +// Keep track of temp dirs for cleanup +let tmpDirs = []; + +beforeEach(() => { + tmpDirs = []; + _resetFederatedRegistryForTests(); +}); + +afterEach(() => { + _resetFederatedRegistryForTests(); + for (const d of tmpDirs) { + try { cleanup(d); } catch { /* ignore */ } + } +}); + +function mkTemp() { + const d = makeTempProject(); + tmpDirs.push(d); + return d; +} + +// ─── 1. Equivalence / no-op with real registry ─────────────────────────────── + +describe('EQUIVALENCE: real registry is a no-op overlay', () => { + test('loadConfig with an empty config.json returns base config without extra federated keys', () => { + const tmpDir = mkTemp(); + // Write an empty config to trigger the try-branch (federated overlay path) + writeConfig(tmpDir, {}); + const result = loadConfig(tmpDir); + + // The result must be an object + assert.ok(typeof result === 'object' && result !== null, 'loadConfig must return an object'); + + // Known result keys that loadConfig always provides (from the main try-branch) + const knownKeys = [ + 'model_profile', 'commit_docs', 'search_gitignored', 'branching_strategy', + 'research', 'plan_checker', 'verifier', 'parallelization', 'brave_search', + 'firecrawl', 'exa_search', 'text_mode', 'auto_advance', + 'mode', 'sub_repos', 'resolve_model_ids', 'context_window', 'phase_naming', + 'project_code', 'subagent_timeout', 'model_overrides', 'models', 'granularity', + 'granularities', 'planning', 'dynamic_routing', 'runtime', 'model_profile_overrides', + 'model_policy', 'effort', 'fast_mode', 'agent_skills', 'manager', + ]; + + for (const key of knownKeys) { + assert.ok( + Object.prototype.hasOwnProperty.call(result, key), + 'Expected result to have key: ' + key, + ); + } + + // UI-capability keys must NOT appear as new top-level keys (they're still central + // and the overlay is empty — so these keys should not be added) + // Note: 'workflow' IS an existing top-level key concept via VALID_CONFIG_KEYS, + // but the nested keys like 'ui_phase' must not be present. + const workflowSection = result['workflow']; + if (workflowSection && typeof workflowSection === 'object') { + // workflow section may already exist from user config but should not have ui_phase + // in the default no-config case + assert.ok( + !Object.prototype.hasOwnProperty.call(workflowSection, 'ui_phase'), + 'workflow.ui_phase should not be injected by the federated overlay (key is still central)', + ); + assert.ok( + !Object.prototype.hasOwnProperty.call(workflowSection, 'ui_review'), + 'workflow.ui_review should not be injected by the federated overlay (key is still central)', + ); + assert.ok( + !Object.prototype.hasOwnProperty.call(workflowSection, 'ui_safety_gate'), + 'workflow.ui_safety_gate should not be injected by the federated overlay (key is still central)', + ); + } + }); + + test('loadConfig with a real config.json returns expected values + no unexpected keys from overlay', () => { + const tmpDir = mkTemp(); + writeConfig(tmpDir, { model_profile: 'balanced', research: true }); + const result = loadConfig(tmpDir); + + assert.strictEqual(result['model_profile'], 'balanced', 'model_profile from config'); + assert.strictEqual(result['research'], true, 'research from config'); + + // The overlay must not have added any unexpected keys from the registry + // (all UI keys are central → skipped → no additions) + // Verify a spot-check: 'ui_phase' should not exist anywhere + assert.strictEqual(result['ui_phase'], undefined, 'ui_phase should not appear as top-level key'); + }); +}); + +// ─── 2. Fixture federated key — value = default ────────────────────────────── + +describe('FIXTURE federated key: key not in central schema', () => { + test('injected configSchema with non-central key → appears in loadConfig result with default', () => { + const tmpDir = mkTemp(); + // Write an empty config.json so loadConfig enters the try-branch (federated overlay path) + writeConfig(tmpDir, {}); + + // Inject a synthetic registry with a key not in the central schema + _setFederatedRegistryForTests({ + configSchema: { + 'mytool.enabled': { + owner: 'mytool', + type: 'boolean', + default: true, + description: 'Enable mytool.', + }, + }, + }); + + const result = loadConfig(tmpDir); + + // mytool is not in the central schema, so the overlay should surface it + // The key 'mytool.enabled' is dotted → result should have result.mytool.enabled = true + const myToolSection = result['mytool']; + assert.ok(typeof myToolSection === 'object' && myToolSection !== null, + 'mytool section must be created for dotted federated key'); + assert.strictEqual( + (myToolSection)['enabled'], + true, + 'mytool.enabled must default to true from slice', + ); + }); + + test('injected top-level (non-dotted) federated key → appears in result', () => { + const tmpDir = mkTemp(); + // Write an empty config.json to enter the try-branch + writeConfig(tmpDir, {}); + + _setFederatedRegistryForTests({ + configSchema: { + 'mytool_flag': { + owner: 'mytool', + type: 'boolean', + default: false, + description: 'Top-level mytool flag.', + }, + }, + }); + + const result = loadConfig(tmpDir); + // Top-level key: result['mytool_flag'] = false (the default) + // BUT: only added if NOT already present in _baseConfig + // 'mytool_flag' is not in the central schema, so it should be added + assert.strictEqual(result['mytool_flag'], false, 'mytool_flag should be set to default false'); + }); +}); + +// ─── 3. Fixture federated key — user override ──────────────────────────────── + +describe('FIXTURE federated key: user config sets the key', () => { + test('user sets a federated key to a valid value → loadConfig uses user value', () => { + const tmpDir = mkTemp(); + + // Write a user config with a synthetic federated key + // The user config uses flat notation (mytool_flag: false) + writeConfig(tmpDir, { mytool_flag: true }); + + _setFederatedRegistryForTests({ + configSchema: { + 'mytool_flag': { + owner: 'mytool', + type: 'boolean', + default: false, + description: 'Top-level mytool flag.', + }, + }, + }); + + const result = loadConfig(tmpDir); + // The user set mytool_flag=true, which matches the type (boolean), so user value wins + assert.strictEqual(result['mytool_flag'], true, 'User-supplied true should override default false'); + }); + + test('user sets a federated key to wrong type → loadConfig falls back to default', () => { + const tmpDir = mkTemp(); + // Write a user config with the wrong type for the federated key + writeConfig(tmpDir, { mytool_flag: 'not-a-bool' }); + + _setFederatedRegistryForTests({ + configSchema: { + 'mytool_flag': { + owner: 'mytool', + type: 'boolean', + default: false, + description: 'Top-level mytool flag.', + }, + }, + }); + + const result = loadConfig(tmpDir); + // Wrong type → fallback to default (false) + assert.strictEqual(result['mytool_flag'], false, 'Should fall back to default on type mismatch'); + }); +}); + +// ─── FIX 1: Nested dotted-path user-override in loadConfig ─────────────────── + +describe('FIX 1: nested user config drives federated overlay in loadConfig', () => { + test('user config { mytool: { enabled: false } } (NESTED) → loadConfig surfaces false', () => { + const tmpDir = mkTemp(); + // Write config.json with the nested structure users actually write + writeConfig(tmpDir, { mytool: { enabled: false } }); + + _setFederatedRegistryForTests({ + configSchema: { + 'mytool.enabled': { + owner: 'mytool', + type: 'boolean', + default: true, + description: 'Enable mytool.', + }, + }, + }); + + const result = loadConfig(tmpDir); + const myToolSection = result['mytool']; + assert.ok(typeof myToolSection === 'object' && myToolSection !== null, + 'mytool section must be in result'); + assert.strictEqual( + (myToolSection)['enabled'], + false, + 'Nested user override of false should override the default of true', + ); + }); +}); + +// ─── FIX 2: Overlay applied on no-config path ──────────────────────────────── + +describe('FIX 2: overlay applied on the no-config path', () => { + test('project with NO config.json → federated default is surfaced (non-central key)', () => { + // Create a project dir WITHOUT a .planning/config.json + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-fed-noconfig-')); + tmpDirs.push(tmpDir); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true }); + // Intentionally do NOT write a config.json + + _setFederatedRegistryForTests({ + configSchema: { + 'mytool.enabled': { + owner: 'mytool', + type: 'boolean', + default: false, + description: 'Enable mytool (default false).', + }, + }, + }); + + const result = loadConfig(tmpDir); + // The overlay must be applied on the no-config path: mytool.enabled should be false (the default) + const myToolSection = result['mytool']; + assert.ok( + typeof myToolSection === 'object' && myToolSection !== null, + 'mytool section must be created by overlay even on no-config path, got: ' + JSON.stringify(result['mytool']), + ); + assert.strictEqual( + (myToolSection)['enabled'], + false, + 'mytool.enabled must default to false on no-config path', + ); + }); + + test('no-config path with REAL registry (all keys central) → output is byte-identical to defaults (no-op)', () => { + // No config.json — use real registry which has all keys central + _resetFederatedRegistryForTests(); + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-fed-noconfig-real-')); + tmpDirs.push(tmpDir); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true }); + // No config.json + + const result = loadConfig(tmpDir); + assert.ok(typeof result === 'object' && result !== null, 'result must be an object'); + + // With real registry (all keys central → overlay is empty → no-op), the result + // should be the defaults object WITHOUT any extra injected keys. + // Spot-check: ui_phase / ui_review / ui_safety_gate must NOT be injected + const workflowSection = result['workflow']; + if (workflowSection && typeof workflowSection === 'object') { + assert.ok( + !Object.prototype.hasOwnProperty.call(workflowSection, 'ui_phase'), + 'workflow.ui_phase must not be injected on no-config path (no-op)', + ); + } + // model_profile must be present (it comes from defaults) + assert.ok(Object.prototype.hasOwnProperty.call(result, 'model_profile'), 'model_profile must be present'); + }); +}); + +// ─── FIX 3: Federated key in config.json → no unknown-key warning ───────────── + +describe('FIX 3: federated key present in config.json → no unknown-key warning', () => { + test('synthetic federated key in config.json → no "unknown config key" warning on stderr', () => { + const tmpDir = mkTemp(); + // Write a config.json that contains a key matching our synthetic federated key's top-level segment + writeConfig(tmpDir, { mytool: { enabled: true } }); + + _setFederatedRegistryForTests({ + configSchema: { + 'mytool.enabled': { + owner: 'mytool', + type: 'boolean', + default: false, + description: 'Enable mytool.', + }, + }, + }); + + // Capture stderr to check for unknown-key warning + const stderrChunks = []; + const origWrite = process.stderr.write.bind(process.stderr); + process.stderr.write = (chunk, ...args) => { + stderrChunks.push(typeof chunk === 'string' ? chunk : String(chunk)); + return origWrite(chunk, ...args); + }; + + try { + const result = loadConfig(tmpDir); + // mytool.enabled is in the federated registry → KNOWN_TOP_LEVEL should include 'mytool' + // → no "unknown config key(s)" warning for 'mytool' + const stderrOutput = stderrChunks.join(''); + assert.ok( + !stderrOutput.includes('unknown config key') || !stderrOutput.includes('mytool'), + 'Should NOT warn about mytool as an unknown key when it is a registered federated key. stderr: ' + stderrOutput, + ); + // The value should be set from user config + const myToolSection = result['mytool']; + assert.ok( + typeof myToolSection === 'object' && myToolSection !== null, + 'mytool section should be in result', + ); + } finally { + process.stderr.write = origWrite; + } + }); +}); + +// ─── 4. Malformed registry — loadConfig still works ────────────────────────── + +describe('MALFORMED registry: loadConfig does not throw', () => { + test('configSchema with all malformed slices → loadConfig returns valid config, no throw', () => { + const tmpDir = mkTemp(); + + _setFederatedRegistryForTests({ + configSchema: { + 'bad.key1': null, + 'bad.key2': 'just-a-string', + 'bad.key3': { type: 'xml', default: '' }, // invalid type + 'bad.key4': { type: 'boolean', description: 'ok' }, // missing default + 'bad.key5': {}, // missing both + }, + }); + + let result; + assert.doesNotThrow(() => { + result = loadConfig(tmpDir); + }, 'loadConfig must not throw even with all-malformed configSchema'); + + assert.ok(typeof result === 'object' && result !== null, 'result must be an object'); + // None of the bad keys should appear in the result + assert.strictEqual(result['bad.key1'], undefined); + assert.strictEqual(result['bad.key2'], undefined); + const badSection = result['bad']; + if (badSection && typeof badSection === 'object') { + assert.strictEqual((badSection)['key1'], undefined, 'bad.key1 must not be set'); + } + }); + + test('configSchema is a string (completely unexpected) → loadConfig still works', () => { + const tmpDir = mkTemp(); + + _setFederatedRegistryForTests({ + configSchema: 'not-an-object', + }); + + let result; + assert.doesNotThrow(() => { + result = loadConfig(tmpDir); + }, 'loadConfig must not throw with non-object configSchema'); + + assert.ok(typeof result === 'object' && result !== null, 'result must be an object'); + }); + + test('registry throws during configSchema access → loadConfig still returns base config', () => { + const tmpDir = mkTemp(); + + // Create a registry proxy that throws when configSchema is accessed + const throwingRegistry = { + get configSchema() { throw new Error('registry exploded'); }, + }; + + _setFederatedRegistryForTests(throwingRegistry); + + let result; + assert.doesNotThrow(() => { + result = loadConfig(tmpDir); + }, 'loadConfig must not throw even if registry access throws'); + + assert.ok(typeof result === 'object' && result !== null, 'result must still be an object'); + // The base config keys must be present + assert.ok(Object.prototype.hasOwnProperty.call(result, 'model_profile'), 'model_profile must be present'); + }); +}); diff --git a/tests/federated-config.test.cjs b/tests/federated-config.test.cjs new file mode 100644 index 000000000..f6275badb --- /dev/null +++ b/tests/federated-config.test.cjs @@ -0,0 +1,696 @@ +'use strict'; + +/** + * federated-config.test.cjs — Behavioral tests for the federated-config module. + * + * ADR-857 phase 3b. Tests cover: + * - empty configSchema → empty result + * - key still in central schema → skipped + pending-migration warning + NOT in values + * - malformed slice (each variant) → skipped + warning, no throw + * - valid federated key → value = default + * - valid federated key with correct-type user override → user value used + * - valid federated key with wrong-type user override → falls back to default + warning + * - __proto__/constructor/prototype keys → ignored, no Object.prototype pollution + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); + +const { mergeFederatedConfig } = require('../gsd-core/bin/lib/federated-config.cjs'); + +// ─── Helper fixtures ────────────────────────────────────────────────────────── + +/** A minimal well-formed boolean config slice entry. */ +const BOOLEAN_SLICE = { + owner: 'test-cap', + type: 'boolean', + default: true, + description: 'A test boolean key.', +}; + +/** A minimal well-formed string config slice entry. */ +const STRING_SLICE = { + owner: 'test-cap', + type: 'string', + default: 'hello', + description: 'A test string key.', +}; + +/** A minimal well-formed number config slice entry. */ +const NUMBER_SLICE = { + owner: 'test-cap', + type: 'number', + default: 42, + description: 'A test number key.', +}; + +/** A minimal well-formed enum config slice entry (no values list). */ +const ENUM_SLICE_NO_VALUES = { + owner: 'test-cap', + type: 'enum', + default: 'medium', + description: 'A test enum key without values list.', +}; + +/** A minimal well-formed enum config slice entry (with values list). */ +const ENUM_SLICE_WITH_VALUES = { + owner: 'test-cap', + type: 'enum', + default: 'low', + values: ['low', 'medium', 'high'], + description: 'A test enum key with values list.', +}; + +/** A never-central isCentralKey that always returns false. */ +const neverCentral = (_key) => false; + +/** An always-central isCentralKey. */ +const alwaysCentral = (_key) => true; + +// ─── 1. Empty configSchema ──────────────────────────────────────────────────── + +describe('empty configSchema', () => { + test('empty object → empty result', () => { + const result = mergeFederatedConfig({ + configSchema: {}, + isCentralKey: neverCentral, + userConfig: {}, + }); + assert.strictEqual(Object.keys(result.values).length, 0, 'values should be empty'); + assert.deepEqual(result.validKeys, [], 'validKeys should be empty'); + assert.deepEqual(result.warnings, [], 'warnings should be empty'); + }); + + test('null configSchema → empty result (defensive)', () => { + const result = mergeFederatedConfig({ + configSchema: null, + isCentralKey: neverCentral, + userConfig: {}, + }); + // FIX 6b: values uses null-prototype object; check it's empty + assert.strictEqual(Object.keys(result.values).length, 0, 'values should be empty for null configSchema'); + assert.deepEqual(result.validKeys, [], 'validKeys should be empty'); + assert.deepEqual(result.warnings, [], 'warnings should be empty'); + }); +}); + +// ─── 2. Central-key skipping ────────────────────────────────────────────────── + +describe('central-key skipping', () => { + test('key in central schema → skipped + pending-migration warning + NOT in values', () => { + const result = mergeFederatedConfig({ + configSchema: { 'workflow.ui_phase': BOOLEAN_SLICE }, + isCentralKey: alwaysCentral, + userConfig: {}, + }); + assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'workflow.ui_phase'), + 'central key must NOT appear in values'); + assert.deepEqual(result.validKeys, [], 'validKeys must be empty for central keys'); + assert.ok(result.warnings.length >= 1, 'Must produce at least one warning'); + assert.ok( + result.warnings.some((w) => w.includes('pending-migration') || w.includes('central config-schema')), + 'Warning must mention pending-migration or central config-schema, got: ' + JSON.stringify(result.warnings), + ); + }); + + test('key NOT in central schema → appears in values', () => { + const result = mergeFederatedConfig({ + configSchema: { 'mytool.enabled': BOOLEAN_SLICE }, + isCentralKey: neverCentral, + userConfig: {}, + }); + assert.ok(Object.prototype.hasOwnProperty.call(result.values, 'mytool.enabled'), + 'Non-central key must appear in values'); + assert.strictEqual(result.validKeys.length, 1); + }); + + test('mixed central + non-central: central skipped, non-central included', () => { + const result = mergeFederatedConfig({ + configSchema: { + 'central.key': BOOLEAN_SLICE, + 'mytool.enabled': BOOLEAN_SLICE, + }, + isCentralKey: (key) => key === 'central.key', + userConfig: {}, + }); + assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'central.key'), + 'central.key must not be in values'); + assert.ok(Object.prototype.hasOwnProperty.call(result.values, 'mytool.enabled'), + 'mytool.enabled must be in values'); + assert.strictEqual(result.validKeys.length, 1); + assert.ok(result.warnings.some((w) => w.includes('pending-migration') || w.includes('central'))); + }); +}); + +// ─── 3. Malformed slice handling ────────────────────────────────────────────── + +describe('malformed slice handling — no throw, warning emitted', () => { + test('slice missing type → skipped + warning, no throw', () => { + const result = mergeFederatedConfig({ + configSchema: { 'tool.key': { owner: 'x', default: true, description: 'x' } }, + isCentralKey: neverCentral, + userConfig: {}, + }); + assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'tool.key'), 'malformed key must not be in values'); + assert.ok(result.warnings.length >= 1, 'Must warn about malformed slice'); + assert.ok(result.warnings.some((w) => w.includes('malformed') || w.includes('tool.key')), + 'Warning must mention the key, got: ' + JSON.stringify(result.warnings)); + }); + + test('slice with invalid type ("xml") → skipped + warning, no throw', () => { + const result = mergeFederatedConfig({ + configSchema: { 'tool.key': { owner: 'x', type: 'xml', default: '', description: 'xml key' } }, + isCentralKey: neverCentral, + userConfig: {}, + }); + assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'tool.key')); + assert.ok(result.warnings.length >= 1); + }); + + test('slice missing default → skipped + warning, no throw', () => { + const result = mergeFederatedConfig({ + configSchema: { 'tool.key': { owner: 'x', type: 'boolean', description: 'x' } }, + isCentralKey: neverCentral, + userConfig: {}, + }); + assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'tool.key')); + assert.ok(result.warnings.length >= 1); + }); + + test('slice is null → skipped + warning, no throw', () => { + const result = mergeFederatedConfig({ + configSchema: { 'tool.key': null }, + isCentralKey: neverCentral, + userConfig: {}, + }); + assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'tool.key')); + assert.ok(result.warnings.length >= 1); + }); + + test('slice is a string scalar → skipped + warning, no throw', () => { + const result = mergeFederatedConfig({ + configSchema: { 'tool.key': 'just-a-string' }, + isCentralKey: neverCentral, + userConfig: {}, + }); + assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'tool.key')); + assert.ok(result.warnings.length >= 1); + }); + + test('slice is a number → skipped + warning, no throw', () => { + const result = mergeFederatedConfig({ + configSchema: { 'tool.key': 42 }, + isCentralKey: neverCentral, + userConfig: {}, + }); + assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'tool.key')); + assert.ok(result.warnings.length >= 1); + }); +}); + +// ─── 4. Valid federated key — default resolution ────────────────────────────── + +describe('valid federated key — default resolution', () => { + test('boolean key absent from userConfig → value = default (true)', () => { + const result = mergeFederatedConfig({ + configSchema: { 'mytool.enabled': BOOLEAN_SLICE }, + isCentralKey: neverCentral, + userConfig: {}, + }); + assert.strictEqual(result.values['mytool.enabled'], true, 'Should use slice default (true)'); + assert.ok(result.validKeys.includes('mytool.enabled')); + assert.deepEqual(result.warnings, []); + }); + + test('string key absent from userConfig → value = default ("hello")', () => { + const result = mergeFederatedConfig({ + configSchema: { 'mytool.name': STRING_SLICE }, + isCentralKey: neverCentral, + userConfig: {}, + }); + assert.strictEqual(result.values['mytool.name'], 'hello'); + assert.ok(result.validKeys.includes('mytool.name')); + }); + + test('number key absent from userConfig → value = default (42)', () => { + const result = mergeFederatedConfig({ + configSchema: { 'mytool.timeout': NUMBER_SLICE }, + isCentralKey: neverCentral, + userConfig: {}, + }); + assert.strictEqual(result.values['mytool.timeout'], 42); + }); + + test('enum key absent from userConfig → value = default ("medium")', () => { + const result = mergeFederatedConfig({ + configSchema: { 'mytool.level': ENUM_SLICE_NO_VALUES }, + isCentralKey: neverCentral, + userConfig: {}, + }); + assert.strictEqual(result.values['mytool.level'], 'medium'); + }); +}); + +// ─── 5. Valid federated key — correct-type user override ───────────────────── + +describe('valid federated key — user override with correct type', () => { + test('boolean key with boolean user override (nested) → user value used', () => { + // FIX 1: users write nested objects, not flat dotted keys + const result = mergeFederatedConfig({ + configSchema: { 'mytool.enabled': BOOLEAN_SLICE }, + isCentralKey: neverCentral, + userConfig: { mytool: { enabled: false } }, + }); + assert.strictEqual(result.values['mytool.enabled'], false, 'Should use user-supplied false'); + assert.deepEqual(result.warnings, []); + }); + + test('string key with string user override (nested) → user value used', () => { + const result = mergeFederatedConfig({ + configSchema: { 'mytool.name': STRING_SLICE }, + isCentralKey: neverCentral, + userConfig: { mytool: { name: 'custom' } }, + }); + assert.strictEqual(result.values['mytool.name'], 'custom'); + assert.deepEqual(result.warnings, []); + }); + + test('number key with number user override (nested) → user value used', () => { + const result = mergeFederatedConfig({ + configSchema: { 'mytool.timeout': NUMBER_SLICE }, + isCentralKey: neverCentral, + userConfig: { mytool: { timeout: 99 } }, + }); + assert.strictEqual(result.values['mytool.timeout'], 99); + assert.deepEqual(result.warnings, []); + }); + + test('enum key with in-values string user override (nested) → user value used', () => { + const result = mergeFederatedConfig({ + configSchema: { 'mytool.level': ENUM_SLICE_WITH_VALUES }, + isCentralKey: neverCentral, + userConfig: { mytool: { level: 'high' } }, + }); + assert.strictEqual(result.values['mytool.level'], 'high'); + assert.deepEqual(result.warnings, []); + }); +}); + +// ─── 6. Valid federated key — wrong-type user override ─────────────────────── + +describe('valid federated key — wrong-type user override', () => { + test('boolean key with string user override (nested) → falls back to default + warning', () => { + const result = mergeFederatedConfig({ + configSchema: { 'mytool.enabled': BOOLEAN_SLICE }, + isCentralKey: neverCentral, + userConfig: { mytool: { enabled: 'not-a-bool' } }, + }); + // Key IS in validKeys (degraded resolution — default used) + assert.ok(Object.prototype.hasOwnProperty.call(result.values, 'mytool.enabled'), + 'Key should still appear in values (degraded)'); + assert.strictEqual(result.values['mytool.enabled'], true, 'Should fall back to default (true)'); + assert.ok(result.warnings.length >= 1, 'Should warn about type mismatch'); + assert.ok( + result.warnings.some((w) => w.includes('wrong type') || w.includes('type')), + 'Warning should mention type mismatch, got: ' + JSON.stringify(result.warnings), + ); + }); + + test('string key with boolean user override (nested) → falls back to default + warning', () => { + const result = mergeFederatedConfig({ + configSchema: { 'mytool.name': STRING_SLICE }, + isCentralKey: neverCentral, + userConfig: { mytool: { name: true } }, + }); + assert.strictEqual(result.values['mytool.name'], 'hello', 'Should fall back to default'); + assert.ok(result.warnings.length >= 1); + }); + + test('number key with string user override (nested) → falls back to default + warning', () => { + const result = mergeFederatedConfig({ + configSchema: { 'mytool.timeout': NUMBER_SLICE }, + isCentralKey: neverCentral, + userConfig: { mytool: { timeout: 'fast' } }, + }); + assert.strictEqual(result.values['mytool.timeout'], 42, 'Should fall back to default'); + assert.ok(result.warnings.length >= 1); + }); +}); + +// ─── 7. Prototype pollution guard ──────────────────────────────────────────── + +describe('prototype pollution guard', () => { + test('__proto__ key in configSchema → ignored, no Object.prototype pollution', () => { + // We can't pass __proto__ as an own-enumerable property via object literal, + // so we use Object.create + defineProperty to simulate what a capability registry + // might hand us if prototype pollution had been attempted upstream. + const poisonedSchema = Object.create(null); + Object.defineProperty(poisonedSchema, '__proto__', { + value: { polluted: true }, + enumerable: true, + configurable: true, + writable: true, + }); + // Note: 'constructor' and 'prototype' CAN be passed via plain object literals + const schemaWithReservedKeys = { + 'constructor': BOOLEAN_SLICE, + 'prototype': STRING_SLICE, + }; + + const result = mergeFederatedConfig({ + configSchema: schemaWithReservedKeys, + isCentralKey: neverCentral, + userConfig: {}, + }); + + // Reserved keys must not appear in values + assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'constructor'), 'constructor must not be in values'); + assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'prototype'), 'prototype must not be in values'); + assert.ok(!result.validKeys.includes('constructor'), 'constructor must not be in validKeys'); + assert.ok(!result.validKeys.includes('prototype'), 'prototype must not be in validKeys'); + + // Object.prototype must not be polluted + assert.strictEqual(({}).polluted, undefined, 'Object.prototype must not be polluted'); + assert.strictEqual(({}).constructor, Object, 'Object.prototype.constructor must be Object (not overwritten)'); + }); + + test('buildRegistry with poisoned keys does not pollute Object.prototype', () => { + // Even with isCentralKey always returning false, reserved keys are guarded + const result = mergeFederatedConfig({ + configSchema: { 'legitimate.key': BOOLEAN_SLICE }, + isCentralKey: neverCentral, + userConfig: {}, + }); + assert.strictEqual(({}).polluted, undefined, 'Object.prototype.polluted must be undefined after merge'); + assert.ok(Object.prototype.hasOwnProperty.call(result.values, 'legitimate.key'), 'legitimate key must be in values'); + }); +}); + +// ─── FIX 1: Nested dotted-path user-override lookup ────────────────────────── + +describe('FIX 1: nested dotted-path user-override lookup', () => { + test('user sets mytool.enabled via NESTED object → user value used', () => { + // Nested config: { mytool: { enabled: false } } — NOT flat {"mytool.enabled": false} + const result = mergeFederatedConfig({ + configSchema: { + 'mytool.enabled': { + owner: 'mytool', + type: 'boolean', + default: true, + description: 'Enable mytool.', + }, + }, + isCentralKey: neverCentral, + userConfig: { mytool: { enabled: false } }, // NESTED + }); + assert.strictEqual(result.values['mytool.enabled'], false, 'Nested user override should be used (false overrides true)'); + assert.deepEqual(result.warnings, [], 'No warnings for valid nested override'); + assert.ok(result.validKeys.includes('mytool.enabled')); + }); + + test('flat {"mytool.enabled": false} does NOT match nested path lookup', () => { + // Flat key string lookup is intentionally NOT supported per FIX 1 spec + const result = mergeFederatedConfig({ + configSchema: { + 'mytool.enabled': { + owner: 'mytool', + type: 'boolean', + default: true, + description: 'Enable mytool.', + }, + }, + isCentralKey: neverCentral, + userConfig: { 'mytool.enabled': false }, // FLAT — not found via nested traversal + }); + // Flat key is not found by nested traversal, so default is used + assert.strictEqual(result.values['mytool.enabled'], true, 'Flat key not found by nested traversal → default used'); + }); + + test('user sets nested 3-segment key correctly', () => { + // Key: "a.b.c", user config: { a: { b: { c: 'override' } } } + const result = mergeFederatedConfig({ + configSchema: { + 'a.b.c': { + owner: 'test', + type: 'string', + default: 'default-val', + description: 'Three-segment key.', + }, + }, + isCentralKey: neverCentral, + userConfig: { a: { b: { c: 'override' } } }, + }); + assert.strictEqual(result.values['a.b.c'], 'override'); + assert.deepEqual(result.warnings, []); + }); + + test('partial nested path (a.b exists but a.b.c missing) → uses default', () => { + const result = mergeFederatedConfig({ + configSchema: { + 'a.b.c': { + owner: 'test', + type: 'string', + default: 'default-val', + description: 'Three-segment key.', + }, + }, + isCentralKey: neverCentral, + userConfig: { a: { b: {} } }, // c is missing + }); + assert.strictEqual(result.values['a.b.c'], 'default-val', 'Missing leaf should use default'); + }); +}); + +// ─── FIX 4: null/undefined/non-object input guards ─────────────────────────── + +describe('FIX 4: null/undefined/non-object input guards', () => { + test('null input → no throw, empty result', () => { + assert.doesNotThrow(() => { + const result = mergeFederatedConfig(null); + assert.ok(result.validKeys.length === 0); + }); + }); + + test('undefined input → no throw, empty result', () => { + assert.doesNotThrow(() => { + const result = mergeFederatedConfig(undefined); + assert.ok(result.validKeys.length === 0); + }); + }); + + test('non-object input (string) → no throw, empty result', () => { + assert.doesNotThrow(() => { + const result = mergeFederatedConfig('not-an-object'); + assert.ok(result.validKeys.length === 0); + }); + }); + + test('null userConfig → treated as {} (no overrides), no throw', () => { + assert.doesNotThrow(() => { + const result = mergeFederatedConfig({ + configSchema: { 'mytool.enabled': BOOLEAN_SLICE }, + isCentralKey: neverCentral, + userConfig: null, + }); + // Should use the default since userConfig is null + assert.strictEqual(result.values['mytool.enabled'], true, 'Should use default when userConfig is null'); + assert.deepEqual(result.warnings, []); + }); + }); + + test('undefined userConfig → treated as {} (no overrides), no throw', () => { + assert.doesNotThrow(() => { + const result = mergeFederatedConfig({ + configSchema: { 'mytool.enabled': BOOLEAN_SLICE }, + isCentralKey: neverCentral, + userConfig: undefined, + }); + assert.strictEqual(result.values['mytool.enabled'], true, 'Should use default when userConfig is undefined'); + }); + }); + + test('non-object userConfig → treated as {} (no overrides), no throw', () => { + assert.doesNotThrow(() => { + const result = mergeFederatedConfig({ + configSchema: { 'mytool.enabled': BOOLEAN_SLICE }, + isCentralKey: neverCentral, + userConfig: 42, + }); + assert.strictEqual(result.values['mytool.enabled'], true, 'Should use default when userConfig is non-object'); + }); + }); +}); + +// ─── FIX 5b: enum user override validation against values list ──────────────── + +describe('FIX 5b: enum out-of-values user override → falls back to default', () => { + test('enum user override IN values list → accepted', () => { + const result = mergeFederatedConfig({ + configSchema: { 'mytool.level': ENUM_SLICE_WITH_VALUES }, + isCentralKey: neverCentral, + userConfig: { mytool: { level: 'high' } }, + }); + assert.strictEqual(result.values['mytool.level'], 'high', 'In-values override should be accepted'); + assert.deepEqual(result.warnings, []); + }); + + test('enum user override OUT OF values list → falls back to default + warning', () => { + const result = mergeFederatedConfig({ + configSchema: { 'mytool.level': ENUM_SLICE_WITH_VALUES }, + isCentralKey: neverCentral, + userConfig: { mytool: { level: 'extreme' } }, // 'extreme' not in ['low', 'medium', 'high'] + }); + assert.strictEqual(result.values['mytool.level'], 'low', 'Out-of-values override should fall back to default'); + assert.ok(result.warnings.length >= 1, 'Should warn about out-of-values override'); + assert.ok( + result.warnings.some((w) => w.includes('type') || w.includes('enum') || w.includes('invalid')), + 'Warning should mention type/enum issue, got: ' + JSON.stringify(result.warnings), + ); + }); + + test('enum user override is non-string → falls back to default + warning', () => { + const result = mergeFederatedConfig({ + configSchema: { 'mytool.level': ENUM_SLICE_WITH_VALUES }, + isCentralKey: neverCentral, + userConfig: { mytool: { level: 42 } }, // number, not string + }); + assert.strictEqual(result.values['mytool.level'], 'low'); + assert.ok(result.warnings.length >= 1); + }); +}); + +// ─── FIX 6b: null-proto consistency in early-return paths ──────────────────── + +describe('FIX 6b: null-proto consistency on early-return paths', () => { + test('null configSchema → values uses Object.create(null) (no __proto__ chain)', () => { + const result = mergeFederatedConfig({ + configSchema: null, + isCentralKey: neverCentral, + userConfig: {}, + }); + // Object.create(null) has no __proto__ — prototype is null + assert.strictEqual(Object.getPrototypeOf(result.values), null, 'values must use null prototype on null configSchema path'); + }); + + test('null input → values uses Object.create(null)', () => { + const result = mergeFederatedConfig(null); + assert.strictEqual(Object.getPrototypeOf(result.values), null, 'values must use null prototype on null input path'); + }); +}); + +// ─── FIX 6c: N-level nested write + prototype pollution via dotted keys ────── + +describe('FIX 6c: N-level nested write and prototype-pollution via dotted keys', () => { + test('3-segment federated key is correctly nested in loadConfig overlay', () => { + // This test verifies _getNestedValue works correctly for 3-segment keys. + // The values object should store the key as "a.b.c" → value mapping. + const result = mergeFederatedConfig({ + configSchema: { + 'tool.section.flag': { + owner: 'test', + type: 'boolean', + default: true, + description: 'Three-segment boolean key.', + }, + }, + isCentralKey: neverCentral, + userConfig: { tool: { section: { flag: false } } }, + }); + assert.strictEqual(result.values['tool.section.flag'], false, '3-segment override should be picked up'); + assert.ok(result.validKeys.includes('tool.section.flag')); + assert.deepEqual(result.warnings, []); + }); + + test('__proto__ segment in dotted key does NOT pollute Object.prototype', () => { + const schema = Object.create(null); + // Create a key with __proto__ in the path via defineProperty + Object.defineProperty(schema, '__proto__.x', { + value: { owner: 'test', type: 'boolean', default: true, description: 'bad key' }, + enumerable: true, configurable: true, writable: true, + }); + assert.doesNotThrow(() => { + mergeFederatedConfig({ + configSchema: schema, + isCentralKey: neverCentral, + userConfig: {}, + }); + }); + // Object.prototype must not be polluted + assert.strictEqual(({}).x, undefined, 'Object.prototype.x must not be polluted via __proto__ key'); + }); + + test('a.__proto__.b segment in dotted key does NOT pollute Object.prototype', () => { + // The key "a.__proto__.b" should be skipped at the __proto__ segment + const result = mergeFederatedConfig({ + configSchema: { + // We can't define 'a.__proto__.b' as an OWN property normally; skip test via defensive path + 'a.constructor.b': { + owner: 'test', + type: 'boolean', + default: true, + description: 'constructor key', + }, + }, + isCentralKey: neverCentral, + userConfig: {}, + }); + // 'a.constructor.b' contains 'constructor' segment — must be skipped + assert.ok(!result.validKeys.includes('a.constructor.b'), 'Key with constructor segment must be skipped'); + // Object.prototype.constructor must still be Object + assert.strictEqual(({}).constructor, Object, 'Object.prototype.constructor must not be modified'); + }); +}); + +// ─── 8. Real registry — all UI keys are central (no-op guarantee) ──────────── + +describe('real registry: all UI keys are central → no-op channel', () => { + test('with real capability-registry, all configSchema keys are skipped (pending-migration)', () => { + const capRegistry = require('../gsd-core/bin/lib/capability-registry.cjs'); + const configSchemaFromRegistry = capRegistry.configSchema; + + // Import the real isValidConfigKey + const configSchemaModule = require('../gsd-core/bin/lib/config-schema.cjs'); + const { isValidConfigKey } = configSchemaModule; + + if (!configSchemaFromRegistry || Object.keys(configSchemaFromRegistry).length === 0) { + // Registry has no configSchema keys — no-op by definition + return; + } + + const result = mergeFederatedConfig({ + configSchema: configSchemaFromRegistry, + isCentralKey: isValidConfigKey, + userConfig: {}, + }); + + // Every key should be skipped (pending-migration) because UI keys are still in central schema + assert.strictEqual(Object.keys(result.values).length, 0, 'values must be empty — all keys are central (pending-migration)'); + assert.deepEqual(result.validKeys, [], 'validKeys must be empty'); + assert.ok(result.warnings.length > 0, 'Should have pending-migration warnings'); + + // Confirm each UI key specifically + const uiKeys = ['workflow.ui_phase', 'workflow.ui_review', 'workflow.ui_safety_gate']; + for (const key of uiKeys) { + assert.ok( + result.warnings.some((w) => w.includes(key)), + 'Should have a warning for ' + key + ', got: ' + JSON.stringify(result.warnings), + ); + } + }); +}); + +// ─── 9. isCentralKey throwing defensively ──────────────────────────────────── + +describe('isCentralKey defensive behavior', () => { + test('isCentralKey that throws → key is skipped with warning, no throw from mergeFederatedConfig', () => { + const throwingCentralKey = () => { throw new Error('internal error'); }; + const result = mergeFederatedConfig({ + configSchema: { 'mytool.key': BOOLEAN_SLICE }, + isCentralKey: throwingCentralKey, + userConfig: {}, + }); + // Key skipped due to isCentralKey throwing + assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'mytool.key'), 'Key must be skipped when isCentralKey throws'); + assert.ok(result.warnings.length >= 1, 'Must produce a warning when isCentralKey throws'); + }); +});