From 2de2d185fad9694f041a7585709ddb77cd27260a Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 14 May 2026 23:50:15 -0400 Subject: [PATCH] feat(3536): Configuration Module via shared manifests + generator (Phase 2 of #3524) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 of the CJS↔SDK hard-seam migration (parent #3524). Eliminates the structural drift surface that produced bug class After this phase, neither bin/lib/ nor sdk/src/ defines CONFIG_DEFAULTS, VALID_CONFIG_KEYS, DYNAMIC_KEY_PATTERNS, or the four legacy-key normalizations inline. All come from one canonical source: the Configuration Module (sdk/src/configuration/index.ts) + two JSON manifests (sdk/shared/config-{defaults,schema}.manifest.json). The CJS mirror is generator-emitted (get-shit-done/bin/lib/configuration.generated.cjs) with a CI freshness check (sdk/scripts/check-configuration-fresh.mjs). - sdk/shared/config-defaults.manifest.json — canonical nested defaults, union of CJS + SDK keys (includes security_*, post_planning_gaps, agent_skills, mode, every git/workflow/hooks sub-section). - sdk/shared/config-schema.manifest.json — VALID_CONFIG_KEYS array, RUNTIME_STATE_KEYS array, DYNAMIC_KEY_PATTERNS array with source strings (regex reconstructed at runtime). - sdk/src/configuration/index.ts — source of truth. Exports loadConfig (pure read), normalizeLegacyKeys (pure, idempotent, returns Normalization[]), mergeDefaults (deep-merge), migrateOnDisk (explicit opt-in disk writeback), plus CONFIG_DEFAULTS, VALID_CONFIG_KEYS, RUNTIME_STATE_KEYS, DYNAMIC_KEY_PATTERNS. - sdk/src/configuration/index.test.ts — 29 vitest pinning tests. - sdk/scripts/gen-configuration.mjs — generator (Function.prototype.toString() inspection of compiled SDK dist, plus brace-balanced text scan for internal helpers, matching the Phase 1 pattern). - sdk/scripts/check-configuration-fresh.mjs — CI freshness gate. - tests/configuration-generator.test.cjs — 27 parity assertions (CJS-generated == SDK source). - tests/configuration-migrate-config.test.cjs — 3 cases for the new gsd-tools migrate-config subcommand. - bin/lib/core.cjs: CONFIG_DEFAULTS literal now sources values from CANONICAL_CONFIG_DEFAULTS (the manifest), with a thin flat projection at the load boundary to preserve the existing flat-shape return contract for the ~21 CJS test files and 100+ consumers. All four legacy-key migration blocks (branching_strategy, sub_repos, multiRepo, depth — historically lines 351-358, 388-397, 401-408, 416-423) collapse to a single normalizeLegacyKeys call in each code path. The inline platformWriteSync writeback stays for now to preserve sync loadConfig semantics; the new async migrateOnDisk is reachable via gsd-tools migrate-config. - bin/lib/config-schema.cjs: 135 → 31 lines. Re-exports from the generated Module. - bin/lib/config.cjs: adds cmdMigrateConfig handler (calls migrateOnDisk on the explicit user-driven path). - bin/gsd-tools.cjs: wires migrate-config into command dispatch. - sdk/src/config.ts: re-exports CONFIG_DEFAULTS and mergeDefaults from the Module. loadConfig now calls normalizeLegacyKeys before mergeDefaults (replaces the inline branching_strategy graft). - sdk/src/query/config-schema.ts: 160 → 36 lines. Re-exports from the Module. - tests/config-schema-sdk-parity.test.cjs: refactored from "CJS Set equals SDK Set" (trivially true post-migration) to "both sides source from the manifest" — structural plus runtime invariant. - Four other tests that text-grepped source files for valid keys (plan-review-convergence, bug-3212, bug-2492, feat-3210) are updated to use runtime VALID_CONFIG_KEYS.has() or manifest JSON lookups. - CONTEXT.md: new Configuration Module entry with full Interface contract. - Root package.json: check:configuration-fresh proxy script. - sdk/package.json: gen:configuration + check:configuration-fresh. - .githooks/pre-commit: configuration drift block. - .github/workflows/test.yml: configuration drift step after the alias drift check. - 9201 CJS tests pass (baseline pre-cycle: 9195; +6 net new tests across migrate-config + parity refactor) - 1872 SDK vitest tests pass - 29 Configuration Module vitest fixtures - 27 CJS/SDK parity fixtures - Net diff: +388 / −519 = 131-line reduction across the seven cycles, despite adding the new Module, manifests, generator, freshness check, and two new test files. 1. SDK CONFIG_DEFAULTS now includes manifest-canonical keys (resolve_model_ids: false, context_window: 200000, phase_naming, claude_md_path, git.create_tag, workflow.security_*, workflow.code_review_*, planning.*, hooks.workflow_guard, ship.*). Consumers accessing via [key: string]: unknown index get the manifest default instead of undefined. 2. SDK mergeDefaults is now proper recursive deep-merge instead of spread-per-section. Overlay { workflow: { research: false } } now preserves sibling workflow keys; previously it replaced the entire workflow section with only research + the section's defaults. Semantically identical for the common case; strictly better for partial nested overrides. 3. New gsd-tools migrate-config CLI subcommand for the explicit, opt-in on-disk migration path. Closes #3536. --- .githooks/pre-commit | 4 + .github/workflows/test.yml | 5 + CONTEXT.md | 3 + docs/INVENTORY-MANIFEST.json | 3 +- docs/INVENTORY.md | 3 +- get-shit-done/bin/gsd-tools.cjs | 9 +- get-shit-done/bin/lib/config-schema.cjs | 124 +----- get-shit-done/bin/lib/config.cjs | 35 ++ .../bin/lib/configuration.generated.cjs | 217 +++++++++++ get-shit-done/bin/lib/core.cjs | 175 +++++---- package.json | 1 + sdk/package.json | 2 + sdk/scripts/check-configuration-fresh.mjs | 40 ++ sdk/scripts/gen-configuration.mjs | 150 ++++++++ sdk/shared/config-defaults.manifest.json | 72 ++++ sdk/shared/config-schema.manifest.json | 143 +++++++ sdk/src/config.test.ts | 12 +- sdk/src/config.ts | 120 +++--- sdk/src/configuration/index.test.ts | 310 +++++++++++++++ sdk/src/configuration/index.ts | 310 +++++++++++++++ sdk/src/query/config-schema.ts | 168 ++------- tests/bug-2492-context-coverage-gate.test.cjs | 9 +- ...2-execute-phase-stall-safe-resume.test.cjs | 12 +- tests/config-schema-sdk-parity.test.cjs | 194 +++++----- tests/configuration-generator.test.cjs | 355 ++++++++++++++++++ tests/configuration-migrate-config.test.cjs | 178 +++++++++ tests/feat-3210-fallow-integration.test.cjs | 18 +- tests/plan-review-convergence.test.cjs | 16 +- 28 files changed, 2166 insertions(+), 522 deletions(-) create mode 100644 get-shit-done/bin/lib/configuration.generated.cjs create mode 100644 sdk/scripts/check-configuration-fresh.mjs create mode 100644 sdk/scripts/gen-configuration.mjs create mode 100644 sdk/shared/config-defaults.manifest.json create mode 100644 sdk/shared/config-schema.manifest.json create mode 100644 sdk/src/configuration/index.test.ts create mode 100644 sdk/src/configuration/index.ts create mode 100644 tests/configuration-generator.test.cjs create mode 100644 tests/configuration-migrate-config.test.cjs diff --git a/.githooks/pre-commit b/.githooks/pre-commit index b731bb6f8..0d8d72f35 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -8,3 +8,7 @@ fi if git diff --cached --name-only | grep -Eq "^sdk/src/query/state-document\.|^get-shit-done/bin/lib/state-document\.generated\.cjs$|^sdk/scripts/gen-state-document\.ts$|^sdk/scripts/check-state-document-fresh\.mjs$"; then npm run check:state-document-fresh fi + +if git diff --cached --name-only | grep -Eq "^sdk/src/configuration/|^sdk/shared/config-(defaults|schema)\.manifest\.json$|^get-shit-done/bin/lib/configuration\.generated\.cjs$|^sdk/scripts/gen-configuration\.mjs$"; then + npm run check:configuration-fresh +fi diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 318a77ae6..14179dad4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -108,6 +108,11 @@ jobs: shell: bash run: node sdk/scripts/check-state-document-fresh.mjs + - name: SDK generated configuration artifact drift check + if: matrix.os == 'ubuntu-latest' && matrix.node-version == 24 + shell: bash + run: node sdk/scripts/check-configuration-fresh.mjs + - name: Run tests with coverage shell: bash run: npm run test:coverage diff --git a/CONTEXT.md b/CONTEXT.md index 658bccc25..526a6fbae 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -46,6 +46,9 @@ Compatibility Adapter Module for `gsd-tools.cjs` command families. Uses generate ### Query Pre-Project Config Policy Module Module policy that defines query-time behavior when `.planning/config.json` is absent: use built-in defaults for parity-sensitive query Interfaces, and emit parity-aligned empty model ids for pre-project model resolution surfaces. +### Configuration Module +Shared CJS/SDK Module owning config load, legacy-key normalization, defaults merge, and explicit on-disk migration for `.planning/config.json`. Interface: `loadConfig(cwd) → MergedConfig` (pure read, never writes disk), `normalizeLegacyKeys(parsed) → { parsed, normalizations[] }` (idempotent, pure, returns the list of normalizations applied), `mergeDefaults(parsed) → MergedConfig` (deep-merge of parsed config over canonical defaults), `migrateOnDisk(cwd) → MigrationReport` (explicit, opt-in, called by the installer and by `gsd-tools migrate-config`). Invariants: never mutates disk inside `loadConfig`; legacy top-level keys (`branching_strategy`, `sub_repos`, `multiRepo`, `depth`) are normalized into their canonical nested locations in the returned value; defaults come from the shared `sdk/shared/config-defaults.manifest.json`; schema (`VALID_CONFIG_KEYS`, `RUNTIME_STATE_KEYS`, `DYNAMIC_KEY_PATTERNS`) comes from `sdk/shared/config-schema.manifest.json`. Source of truth: `sdk/src/configuration/index.ts`; CJS callers consume the generator-emitted `get-shit-done/bin/lib/configuration.generated.cjs` via the thin Adapters at `bin/lib/core.cjs:loadConfig` and `bin/lib/config-schema.cjs`. Eliminates the recurring #3523-class drift bug structurally. + ### Planning Workspace Module Module owning `.planning` path resolution, active workstream pointer policy (`session-scoped > shared`), pointer self-heal behavior, and planning lock semantics for workstream-aware execution. diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index fbd8e0ea7..df1aeaedd 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -1,5 +1,5 @@ { - "generated": "2026-05-14", + "generated": "2026-05-15", "families": { "agents": [ "gsd-advisor-researcher", @@ -268,6 +268,7 @@ "commands.cjs", "config-schema.cjs", "config.cjs", + "configuration.generated.cjs", "context-utilization.cjs", "core.cjs", "decisions.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index fef3f65f7..594c056c6 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -360,7 +360,7 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t --- -## CLI Modules (60 shipped) +## CLI Modules (61 shipped) Full listing: `get-shit-done/bin/lib/*.cjs`. @@ -376,6 +376,7 @@ Full listing: `get-shit-done/bin/lib/*.cjs`. | `commands.cjs` | Misc CLI commands (slug, timestamp, todos, scaffolding, stats) | | `config-schema.cjs` | Single source of truth for `VALID_CONFIG_KEYS` and dynamic key patterns; imported by both the validator and the config-schema-docs parity test | | `config.cjs` | `config.json` read/write, section initialization; imports validator from `config-schema.cjs` | +| `configuration.generated.cjs` | Generated Configuration Module — canonical config loading, legacy-key normalization, defaults merge, and explicit on-disk migration; source of truth for both SDK and CJS consumers | | `context-utilization.cjs` | Pure classifier for `gsd-health --context` — turns (tokensUsed, contextWindow) into a `{ percent, state }` triage result against the 60%/70% fracture-point thresholds (#2792) | | `core.cjs` | Error handling, output formatting, shared utilities, runtime fallbacks; compatibility re-exports for planning-workspace helpers | | `decisions.cjs` | Shared parser for CONTEXT.md `` blocks (D-NN entries); used by `gap-checker.cjs` and intended for #2492 plan/verify decision gates | diff --git a/get-shit-done/bin/gsd-tools.cjs b/get-shit-done/bin/gsd-tools.cjs index fe1071e02..5a20db37b 100755 --- a/get-shit-done/bin/gsd-tools.cjs +++ b/get-shit-done/bin/gsd-tools.cjs @@ -367,7 +367,7 @@ async function main() { // phase / roadmap / milestone / progress / etc. const TOP_LEVEL_USAGE = 'Usage: gsd-tools [args] [--raw] [--pick ] [--cwd ] [--ws ] [--json-errors]\n' + 'Commands: agent-skills, audit-open, audit-uat, check-commit, commit, commit-to-subrepo, ' + - 'config-ensure-section, config-get, config-new-project, config-path, config-set, ' + + 'config-ensure-section, config-get, config-new-project, config-path, config-set, migrate-config, ' + 'current-timestamp, detect-custom-files, docs-init, extract-messages, find-phase, ' + 'from-gsd2, frontmatter, gap-analysis, generate-claude-md, generate-claude-profile, ' + 'generate-dev-preferences, generate-slug, graphify, history-digest, init, intel, ' + @@ -668,6 +668,13 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand break; } + case 'migrate-config': { + // Explicit on-disk migration of legacy config keys to canonical shape (#3536). + // Wraps Configuration Module migrateOnDisk(); idempotent. async — must await. + await config.cmdMigrateConfig(cwd, raw); + break; + } + case 'agent-skills': { init.cmdAgentSkills(cwd, args[1], raw); break; diff --git a/get-shit-done/bin/lib/config-schema.cjs b/get-shit-done/bin/lib/config-schema.cjs index 38c33e3b9..16641d030 100644 --- a/get-shit-done/bin/lib/config-schema.cjs +++ b/get-shit-done/bin/lib/config-schema.cjs @@ -1,127 +1,23 @@ 'use strict'; /** - * Single source of truth for valid config key paths. + * Thin adapter — sources schema data from the manifest via the generated + * Configuration Module. All inline literals have been removed; the manifest + * at sdk/shared/config-schema.manifest.json is the single source of truth. * * Imported by: * - config.cjs (isValidConfigKey validator) * - tests/config-schema-docs-parity.test.cjs (CI drift guard) + * - tests/config-schema-sdk-parity.test.cjs (CJS↔SDK parity guard) * - * Adding a key here without documenting it in docs/CONFIGURATION.md will - * fail the parity test. Adding a key to docs/CONFIGURATION.md without - * adding it here will cause config-set to reject it at runtime. + * See Phase 2 Cycle 5 (#3536) — schema manifest migration. */ -/** Exact-match config key paths accepted by config-set. */ -const VALID_CONFIG_KEYS = new Set([ - 'mode', 'granularity', 'parallelization', 'commit_docs', 'model_profile', - 'search_gitignored', 'brave_search', 'firecrawl', 'exa_search', - 'workflow.research', 'workflow.plan_check', 'workflow.verifier', - 'workflow.nyquist_validation', 'workflow.ai_integration_phase', 'workflow.ui_phase', 'workflow.ui_safety_gate', - 'workflow.auto_advance', 'workflow.node_repair', 'workflow.node_repair_budget', - 'workflow.tdd_mode', - 'workflow.human_verify_mode', - 'workflow.text_mode', - 'workflow.research_before_questions', - 'workflow.discuss_mode', - 'workflow.skip_discuss', - 'workflow.auto_prune_state', - 'workflow.use_worktrees', - 'workflow.worktree_skip_hooks', - 'workflow.code_review', - 'workflow.code_review_depth', - 'workflow.code_review_command', - 'workflow.pattern_mapper', - 'workflow.plan_bounce', - 'workflow.plan_bounce_script', - 'workflow.plan_bounce_passes', - 'workflow.plan_chunked', - 'workflow.plan_review_convergence', - 'workflow.post_planning_gaps', - 'workflow.security_enforcement', - 'workflow.security_asvs_level', - 'workflow.security_block_on', - 'workflow.drift_threshold', - 'workflow.drift_action', - 'code_quality.fallow.enabled', - 'code_quality.fallow.scope', - 'code_quality.fallow.profile', - 'code_quality.fallow.mcp', - 'ship.pr_body_sections', - 'git.branching_strategy', 'git.base_branch', 'git.create_tag', 'git.phase_branch_template', 'git.milestone_branch_template', 'git.quick_branch_template', - 'planning.commit_docs', 'planning.search_gitignored', 'planning.sub_repos', - 'review.ollama_host', 'review.lm_studio_host', 'review.llama_cpp_host', - 'review.default_reviewers', - 'workflow.cross_ai_execution', 'workflow.cross_ai_command', 'workflow.cross_ai_timeout', - 'workflow.subagent_timeout', - 'executor.stall_detect_interval_minutes', - 'executor.stall_threshold_minutes', - 'workflow.inline_plan_threshold', - 'hooks.context_warnings', - 'hooks.workflow_guard', - 'workflow.context_coverage_gate', - 'statusline.show_last_command', - 'statusline.context_position', - 'workflow.ui_review', - 'workflow.max_discuss_passes', - 'features.thinking_partner', - 'context', - 'features.global_learnings', - 'learnings.max_inject', - 'project_code', 'phase_naming', - 'manager.flags.discuss', 'manager.flags.plan', 'manager.flags.execute', - 'response_language', - 'context_window', - 'intel.enabled', - 'graphify.enabled', - 'graphify.build_timeout', - 'claude_md_path', - 'claude_md_assembly.mode', - // #2517 — runtime-aware model profiles - 'runtime', - // #3162 — documented top-level key: controls model ID resolution for non-Claude runtimes - 'resolve_model_ids', -]); - -/** - * Internal runtime-state keys — accepted by config-set (workflows write them) but not - * exposed as user-settable options. Excluded from VALID_CONFIG_KEYS so they stay out of - * the public docs-parity check and the "Valid keys:" error message. - * See: #3162 (workflow._auto_chain_active written by plan/execute/discuss workflows) - */ -const RUNTIME_STATE_KEYS = new Set([ - 'workflow._auto_chain_active', -]); - -/** - * Dynamic-pattern validators — keys matching these regexes are also accepted. - * Each entry has a `test` function and a human-readable `description`. - */ -const DYNAMIC_KEY_PATTERNS = [ - { topLevel: 'agent_skills', test: (k) => /^agent_skills\.[a-zA-Z0-9_-]+$/.test(k), description: 'agent_skills.' }, - { topLevel: 'review', test: (k) => /^review\.models\.[a-zA-Z0-9_-]+$/.test(k), description: 'review.models.' }, - { topLevel: 'features', test: (k) => /^features\.[a-zA-Z0-9_]+$/.test(k), description: 'features.' }, - { topLevel: 'claude_md_assembly', test: (k) => /^claude_md_assembly\.blocks\.[a-zA-Z0-9_]+$/.test(k), description: 'claude_md_assembly.blocks.
' }, - // #2517 — runtime-aware model profile overrides: model_profile_overrides.. - // is a free string (so users can map non-built-in runtimes); is enum-restricted. - { topLevel: 'model_profile_overrides', test: (k) => /^model_profile_overrides\.[a-zA-Z0-9_-]+\.(opus|sonnet|haiku)$/.test(k), - description: 'model_profile_overrides..' }, - // #3023 — per-phase-type model map: models. = - // Six named slots (planning/discuss/research/execution/verification/completion); - // unknown phase-types are rejected. Per-agent model_overrides still take - // precedence over phase-type at resolve time. - { topLevel: 'models', test: (k) => /^models\.(planning|discuss|research|execution|verification|completion)$/.test(k), - description: 'models.' }, - // #3024 — dynamic routing block. Three top-level scalar settings - // plus a tier_models sub-block keyed by light/standard/heavy. - { topLevel: 'dynamic_routing', - test: (k) => /^dynamic_routing\.(enabled|escalate_on_failure|max_escalations|tier_models\.(light|standard|heavy))$/.test(k), - description: 'dynamic_routing.>' }, - // #3227 — per-agent model overrides: model_overrides. - // Full model IDs (e.g. "openai/o3") and tier aliases (opus/sonnet/haiku/inherit) - // are both accepted. Value validation is handled by the resolver at read time. - { topLevel: 'model_overrides', test: (k) => /^model_overrides\.[a-zA-Z0-9_-]+$/.test(k), description: 'model_overrides.' }, -]; +const { + VALID_CONFIG_KEYS, + RUNTIME_STATE_KEYS, + DYNAMIC_KEY_PATTERNS, +} = require('./configuration.generated.cjs'); /** * Returns true if keyPath is a valid config key (exact, dynamic pattern, or runtime state). diff --git a/get-shit-done/bin/lib/config.cjs b/get-shit-done/bin/lib/config.cjs index cc93e3ed0..c89512317 100644 --- a/get-shit-done/bin/lib/config.cjs +++ b/get-shit-done/bin/lib/config.cjs @@ -648,6 +648,40 @@ function cmdConfigPath(cwd) { output(configPath, true, configPath); } +/** + * Explicit on-disk migration of legacy config keys to canonical nested shape. + * + * Wraps the Configuration Module's migrateOnDisk() for the CLI surface. This + * is the Phase 2 acceptance-criteria deliverable for opt-in migration (#3536): + * users can run `gsd-tools migrate-config` to apply all four legacy-key + * migrations to their .planning/config.json without having to load any config + * implicitly via another command. + * + * Output: JSON object with { migrated, normalizations, wrote } or a human-readable + * summary when --raw is set. Exits 0 in all cases (including no-op). + */ +async function cmdMigrateConfig(cwd, raw) { + const { migrateOnDisk } = require('./configuration.generated.cjs'); + const ws = process.env.GSD_WORKSTREAM || null; + const report = await migrateOnDisk(cwd, ws || undefined); + + if (raw) { + if (!report.migrated) { + const msg = 'No legacy keys found — config is already canonical.'; + output(msg, true, msg); + } else { + const lines = [ + `Migrated: ${report.wrote}`, + ...report.normalizations.map(n => ` ${n.from} → ${n.to}`), + ].join('\n'); + output(lines, true, lines); + } + } else { + // output() JSON.stringify's its first arg when raw=false; pass the report object. + output(report, false, report); + } +} + module.exports = { VALID_CONFIG_KEYS, cmdConfigEnsureSection, @@ -656,4 +690,5 @@ module.exports = { cmdConfigSetModelProfile, cmdConfigNewProject, cmdConfigPath, + cmdMigrateConfig, }; diff --git a/get-shit-done/bin/lib/configuration.generated.cjs b/get-shit-done/bin/lib/configuration.generated.cjs new file mode 100644 index 000000000..0903ce74e --- /dev/null +++ b/get-shit-done/bin/lib/configuration.generated.cjs @@ -0,0 +1,217 @@ +'use strict'; + +/** + * GENERATED FILE — DO NOT EDIT. + * + * Source: sdk/src/configuration/index.ts + * Regenerate: cd sdk && npm run gen:configuration + * + * Configuration Module — single source of truth for config loading, + * legacy-key normalization, defaults merge, and explicit on-disk migration. + */ + +const { readFileSync, writeFileSync, existsSync, readdirSync } = require('node:fs'); +const { join } = require('node:path'); + +// ─── Manifest requires ─────────────────────────────────────────────────────── +// Resolved relative to this file: get-shit-done/bin/lib/ → sdk/shared/ +// This file lives at: get-shit-done/bin/lib/configuration.generated.cjs +// sdk/shared lives at: sdk/shared/ (3 dirs up from bin/lib, then into sdk/shared) +const CONFIG_DEFAULTS = require('../../../sdk/shared/config-defaults.manifest.json'); +const SCHEMA_MANIFEST = require('../../../sdk/shared/config-schema.manifest.json'); +const VALID_CONFIG_KEYS = new Set(SCHEMA_MANIFEST.validKeys); +const RUNTIME_STATE_KEYS = new Set(SCHEMA_MANIFEST.runtimeStateKeys); +const DYNAMIC_KEY_PATTERNS = SCHEMA_MANIFEST.dynamicKeyPatterns.map(p => ({ ...p, test: (key) => new RegExp(p.source).test(key) })); + +// ─── Depth → Granularity mapping ───────────────────────────────────────────── +const DEPTH_TO_GRANULARITY = { + quick: 'coarse', + standard: 'standard', + comprehensive: 'fine', +}; + +// ─── Internal helpers ───────────────────────────────────────────────────────── +function planningDir(cwd, workstream) { + if (!workstream) + return join(cwd, '.planning'); + return join(cwd, '.planning', 'workstreams', workstream); +} + +function detectSubRepos(cwd) { + const results = []; + try { + const entries = readdirSync(cwd, { withFileTypes: true }); + for (const entry of entries) { + if (!entry.isDirectory()) + continue; + if (entry.name.startsWith('.') || entry.name === 'node_modules') + continue; + const gitPath = join(cwd, entry.name, '.git'); + try { + if (existsSync(gitPath)) { + results.push(entry.name); + } + } + catch { /* ignore */ } + } + } + catch { /* ignore */ } + return results.sort(); +} + +function deepMergeConfig(base, overlay) { + const result = { ...base }; + for (const key of Object.keys(overlay)) { + const ov = overlay[key]; + if (ov !== null && ov !== undefined && typeof ov === 'object' && !Array.isArray(ov)) { + const bv = base[key]; + if (bv !== null && bv !== undefined && typeof bv === 'object' && !Array.isArray(bv)) { + result[key] = deepMergeConfig(bv, ov); + } + else { + result[key] = deepMergeConfig({}, ov); + } + } + else { + result[key] = ov; + } + } + return result; +} + +// ─── Exported functions ─────────────────────────────────────────────────────── +function normalizeLegacyKeys(parsed) { + const result = { ...parsed }; + const normalizations = []; + // 1. branching_strategy → git.branching_strategy + if (Object.prototype.hasOwnProperty.call(result, 'branching_strategy')) { + const value = result.branching_strategy; + const git = result.git ?? {}; + if (git.branching_strategy === undefined) { + result.git = { ...git, branching_strategy: value }; + } + else { + // canonical nested wins — just delete the stale top-level + result.git = { ...git }; + } + delete result.branching_strategy; + normalizations.push({ from: 'branching_strategy', to: 'git.branching_strategy', value }); + } + // 2. top-level sub_repos → planning.sub_repos + if (Object.prototype.hasOwnProperty.call(result, 'sub_repos')) { + const value = result.sub_repos; + const planning = result.planning ?? {}; + if (!planning.sub_repos) { + result.planning = { ...planning, sub_repos: value }; + } + else { + result.planning = { ...planning }; + } + delete result.sub_repos; + normalizations.push({ from: 'sub_repos', to: 'planning.sub_repos', value }); + } + // 3. multiRepo: true → marker (filesystem detection deferred to migrateOnDisk / caller) + if (result.multiRepo === true) { + delete result.multiRepo; + normalizations.push({ from: 'multiRepo', to: 'planning.sub_repos', value: true, requiresFilesystem: true }); + } + // 4. top-level depth → granularity + if (Object.prototype.hasOwnProperty.call(result, 'depth') && !Object.prototype.hasOwnProperty.call(result, 'granularity')) { + const rawDepth = result.depth; + const mapped = DEPTH_TO_GRANULARITY[rawDepth] ?? rawDepth; + result.granularity = mapped; + delete result.depth; + normalizations.push({ from: 'depth', to: 'granularity', value: mapped }); + } + return { parsed: result, normalizations }; +} + +function mergeDefaults(parsed) { + // Start with a deep clone of defaults, then overlay parsed + const defaults = JSON.parse(JSON.stringify(CONFIG_DEFAULTS)); + return deepMergeConfig(defaults, parsed); +} + +async function loadConfig(cwd, options) { + const configPath = join(planningDir(cwd, options?.workstream), 'config.json'); + let raw; + try { + raw = readFileSync(configPath, 'utf-8'); + } + catch { + // File missing — return defaults + return mergeDefaults({}); + } + const trimmed = raw.trim(); + if (trimmed === '') { + return mergeDefaults({}); + } + let parsed; + try { + parsed = JSON.parse(trimmed); + } + catch (err) { + const msg = err instanceof Error ? err.message : String(err); + throw new Error(`Failed to parse config at ${configPath}: ${msg}`); + } + if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { + throw new Error(`Config at ${configPath} must be a JSON object`); + } + const { parsed: normalized, normalizations } = normalizeLegacyKeys(parsed); + if (options?.onNormalizations && normalizations.length > 0) { + options.onNormalizations(normalizations); + } + return mergeDefaults(normalized); +} + +async function migrateOnDisk(cwd, workstream) { + const configPath = join(planningDir(cwd, workstream), 'config.json'); + let raw; + try { + raw = readFileSync(configPath, 'utf-8'); + } + catch { + // File missing — nothing to migrate + return { migrated: false, normalizations: [], wrote: null }; + } + const trimmed = raw.trim(); + if (trimmed === '') { + return { migrated: false, normalizations: [], wrote: null }; + } + let parsed; + try { + parsed = JSON.parse(trimmed); + } + catch { + // Malformed — can't migrate + return { migrated: false, normalizations: [], wrote: null }; + } + const { parsed: normalized, normalizations } = normalizeLegacyKeys(parsed); + if (normalizations.length === 0) { + return { migrated: false, normalizations: [], wrote: null }; + } + // Resolve multiRepo filesystem detection + const result = { ...normalized }; + for (const norm of normalizations) { + if (norm.requiresFilesystem) { + const detected = detectSubRepos(cwd); + if (detected.length > 0) { + const planning = result.planning ?? {}; + result.planning = { ...planning, sub_repos: detected, commit_docs: false }; + } + } + } + writeFileSync(configPath, JSON.stringify(result, null, 2)); + return { migrated: true, normalizations, wrote: configPath }; +} + +module.exports = { + loadConfig, + normalizeLegacyKeys, + mergeDefaults, + migrateOnDisk, + CONFIG_DEFAULTS, + VALID_CONFIG_KEYS, + RUNTIME_STATE_KEYS, + DYNAMIC_KEY_PATTERNS, +}; diff --git a/get-shit-done/bin/lib/core.cjs b/get-shit-done/bin/lib/core.cjs index 0aca97df6..780fa588f 100644 --- a/get-shit-done/bin/lib/core.cjs +++ b/get-shit-done/bin/lib/core.cjs @@ -25,6 +25,17 @@ const { setActiveWorkstream, } = require('./planning-workspace.cjs'); +// ─── Configuration Module (generated CJS mirror) ──────────────────────────── +// Cycle 4: import canonical defaults + normalization primitives from the +// generated module; core.cjs no longer carries its own inline literal or its +// own migration logic. The exported CONFIG_DEFAULTS remains a flat-key object +// (shape unchanged) so legacy consumers (config.cjs, verify.cjs, tests) require +// no changes. Values are sourced from the canonical nested manifest. +const { + CONFIG_DEFAULTS: CANONICAL_CONFIG_DEFAULTS, + normalizeLegacyKeys, +} = require('./configuration.generated.cjs'); + // ─── Path helpers ──────────────────────────────────────────────────────────── /** Normalize a relative path to always use forward slashes (cross-platform). */ @@ -279,36 +290,49 @@ function error(message, reason = ERROR_REASON.UNKNOWN) { // ─── File & Config utilities ────────────────────────────────────────────────── /** - * Canonical config defaults. Single source of truth — imported by config.cjs and verify.cjs. + * Canonical config defaults — flat-key projection for CJS consumers. + * + * Cycle 4: Values are sourced from CANONICAL_CONFIG_DEFAULTS (the nested + * manifest loaded by configuration.generated.cjs). The flat shape is + * preserved here so legacy consumers (config.cjs, verify.cjs, tests that + * regex-parse this source) continue to work without changes. The key names + * and the `const CONFIG_DEFAULTS = {` pattern are intentionally kept. + * + * Mapping notes: + * - workflow.plan_check → plan_checker (CJS flat name; verify.cjs uses this) + * - git.* → flat git keys (branching_strategy, templates) + * - workflow.* → flat names (research, verifier, …) + * - planning.sub_repos → sub_repos + * - planning.commit_docs / search_gitignored → top-level flat keys */ const CONFIG_DEFAULTS = { - model_profile: 'balanced', - commit_docs: true, - search_gitignored: false, - branching_strategy: 'none', - phase_branch_template: 'gsd/phase-{phase}-{slug}', - milestone_branch_template: 'gsd/{milestone}-{slug}', - quick_branch_template: null, - research: true, - plan_checker: true, - verifier: true, - nyquist_validation: true, - ai_integration_phase: true, - parallelization: true, - brave_search: false, - firecrawl: false, - exa_search: false, - text_mode: false, // when true, use plain-text numbered lists instead of AskUserQuestion menus - sub_repos: [], - resolve_model_ids: false, // false: return alias as-is | true: map to full Claude model ID | "omit": return '' (runtime uses its default) - context_window: 200000, // default 200k; set to 1000000 for Opus/Sonnet 4.6 1M models - phase_naming: 'sequential', // 'sequential' (default, auto-increment) or 'custom' (arbitrary string IDs) - project_code: null, // optional short prefix for phase dirs (e.g., 'CK' → 'CK-01-foundation') - subagent_timeout: 300000, // 5 min default; increase for large codebases or slower models (ms) - security_enforcement: true, // workflow.security_enforcement — threat-model-anchored security verification via /gsd:secure-phase - security_asvs_level: 1, // workflow.security_asvs_level — OWASP ASVS verification level (1=opportunistic, 2=standard, 3=comprehensive) - security_block_on: 'high', // workflow.security_block_on — minimum severity that blocks phase advancement ('high' | 'medium' | 'low') - post_planning_gaps: true, // workflow.post_planning_gaps — unified post-planning gap report (#2493): scan REQUIREMENTS.md + CONTEXT.md decisions vs all PLAN.md files + model_profile: CANONICAL_CONFIG_DEFAULTS.model_profile, + commit_docs: CANONICAL_CONFIG_DEFAULTS.commit_docs, + search_gitignored: CANONICAL_CONFIG_DEFAULTS.search_gitignored, + branching_strategy: CANONICAL_CONFIG_DEFAULTS.git.branching_strategy, + phase_branch_template: CANONICAL_CONFIG_DEFAULTS.git.phase_branch_template, + milestone_branch_template: CANONICAL_CONFIG_DEFAULTS.git.milestone_branch_template, + quick_branch_template: CANONICAL_CONFIG_DEFAULTS.git.quick_branch_template, + research: CANONICAL_CONFIG_DEFAULTS.workflow.research, + plan_checker: CANONICAL_CONFIG_DEFAULTS.workflow.plan_check, // flat CJS name maps to workflow.plan_check + verifier: CANONICAL_CONFIG_DEFAULTS.workflow.verifier, + nyquist_validation: CANONICAL_CONFIG_DEFAULTS.workflow.nyquist_validation, + ai_integration_phase: CANONICAL_CONFIG_DEFAULTS.workflow.ai_integration_phase, + parallelization: CANONICAL_CONFIG_DEFAULTS.parallelization, + brave_search: CANONICAL_CONFIG_DEFAULTS.brave_search, + firecrawl: CANONICAL_CONFIG_DEFAULTS.firecrawl, + exa_search: CANONICAL_CONFIG_DEFAULTS.exa_search, + text_mode: CANONICAL_CONFIG_DEFAULTS.workflow.text_mode, + sub_repos: CANONICAL_CONFIG_DEFAULTS.planning.sub_repos, + resolve_model_ids: CANONICAL_CONFIG_DEFAULTS.resolve_model_ids, + context_window: CANONICAL_CONFIG_DEFAULTS.context_window, + phase_naming: CANONICAL_CONFIG_DEFAULTS.phase_naming, + project_code: CANONICAL_CONFIG_DEFAULTS.project_code, + subagent_timeout: CANONICAL_CONFIG_DEFAULTS.workflow.subagent_timeout, + security_enforcement: CANONICAL_CONFIG_DEFAULTS.workflow.security_enforcement, + security_asvs_level: CANONICAL_CONFIG_DEFAULTS.workflow.security_asvs_level, + security_block_on: CANONICAL_CONFIG_DEFAULTS.workflow.security_block_on, + post_planning_gaps: CANONICAL_CONFIG_DEFAULTS.workflow.post_planning_gaps, }; /** @@ -348,13 +372,26 @@ function loadConfig(cwd, options = {}) { const raw = platformReadSync(rootConfigPath); if (raw === null) throw new Error('missing'); rootParsed = JSON.parse(raw); - if (Object.prototype.hasOwnProperty.call(rootParsed, 'branching_strategy')) { - if (!rootParsed.git) rootParsed.git = {}; - if (rootParsed.git.branching_strategy === undefined) { - rootParsed.git.branching_strategy = rootParsed.branching_strategy; + // Cycle 4: delegate all legacy-key normalization to the Configuration Module. + // normalizeLegacyKeys handles branching_strategy → git.branching_strategy, + // sub_repos → planning.sub_repos, multiRepo, and depth → granularity. + const { parsed: rootNormalized, normalizations: rootNorms } = normalizeLegacyKeys(rootParsed); + if (rootNorms.length > 0) { + // Resolve filesystem-dependent normalizations (multiRepo → planning.sub_repos) + for (const norm of rootNorms) { + if (norm.requiresFilesystem && !rootNormalized.planning?.sub_repos) { + const detected = detectSubRepos(cwd); + if (detected.length > 0) { + if (!rootNormalized.planning) rootNormalized.planning = {}; + rootNormalized.planning.sub_repos = detected; + rootNormalized.planning.commit_docs = false; + } + } } - delete rootParsed.branching_strategy; + rootParsed = rootNormalized; try { platformWriteSync(rootConfigPath, JSON.stringify(rootParsed, null, 2)); } catch {} + } else { + rootParsed = rootNormalized; } } catch { // Root config missing or unparseable — workstream config stands alone @@ -371,57 +408,37 @@ function loadConfig(cwd, options = {}) { // for migrations and writes so we never persist merged values back to disk. const fileData = JSON.parse(raw); - // Migrate deprecated "depth" key to "granularity" with value mapping - if ('depth' in fileData && !('granularity' in fileData)) { - const depthToGranularity = { quick: 'coarse', standard: 'standard', comprehensive: 'fine' }; - fileData.granularity = depthToGranularity[fileData.depth] || fileData.depth; - delete fileData.depth; - try { platformWriteSync(configPath, JSON.stringify(fileData, null, 2)); } catch { /* intentionally empty */ } - } - - // Auto-detect and sync sub_repos: scan for child directories with .git + // Cycle 4: Single normalizeLegacyKeys call replaces all four inline migration + // blocks (depth→granularity, multiRepo→planning.sub_repos, sub_repos→planning.sub_repos, + // branching_strategy→git.branching_strategy). The Module is pure (no I/O); disk + // writeback is handled below with the existing platformWriteSync pattern. + // Note: migrateOnDisk from the Module is async; loadConfig is sync — so we + // call normalizeLegacyKeys inline and do the writeback at the call site. + // Per brief §4.3: "use normalizeLegacyKeys directly and do writeback inline." let configDirty = false; - - // Migrate legacy "multiRepo: true" boolean → planning.sub_repos array. - // Canonical location is planning.sub_repos (#2561); writing to top-level - // would be flagged as unknown by the validator below (#2638). - if (fileData.multiRepo === true && !fileData.sub_repos && !fileData.planning?.sub_repos) { - const detected = detectSubRepos(cwd); - if (detected.length > 0) { - if (!fileData.planning) fileData.planning = {}; - fileData.planning.sub_repos = detected; - fileData.planning.commit_docs = false; - delete fileData.multiRepo; + { + const { parsed: normalized, normalizations } = normalizeLegacyKeys(fileData); + if (normalizations.length > 0) { + // Merge normalized values back into fileData (mutation-in-place for legacy code below) + Object.keys(fileData).forEach(k => delete fileData[k]); + Object.assign(fileData, normalized); configDirty = true; + // Resolve filesystem-dependent normalizations (multiRepo → planning.sub_repos). + // Guard: only populate sub_repos from filesystem if not already set by normalization + // AND the original file didn't have sub_repos already (preserve existing intent). + for (const norm of normalizations) { + if (norm.requiresFilesystem && !fileData.planning?.sub_repos) { + const detected = detectSubRepos(cwd); + if (detected.length > 0) { + if (!fileData.planning) fileData.planning = {}; + fileData.planning.sub_repos = detected; + fileData.planning.commit_docs = false; + } + } + } } } - // Self-heal legacy/buggy installs: strip any stale top-level sub_repos, - // preserving its value as the planning.sub_repos seed if that slot is empty. - if (Object.prototype.hasOwnProperty.call(fileData, 'sub_repos')) { - if (!fileData.planning) fileData.planning = {}; - if (!fileData.planning.sub_repos) { - fileData.planning.sub_repos = fileData.sub_repos; - } - delete fileData.sub_repos; - configDirty = true; - } - - // #3523 — Migrate legacy top-level branching_strategy → git.branching_strategy. - // Canonical location is git.branching_strategy (per config-schema.cjs); writing - // at the top level trips the unknown-key warning even though loadConfig:485 actively - // reads it via the nested fallback. This migration mirrors the multiRepo → sub_repos - // precedent: graft then delete so the warning never fires again on this project. - // The nested value wins if already set (matches SDK mergeDefaults precedence, PR #3116). - if (Object.prototype.hasOwnProperty.call(fileData, 'branching_strategy')) { - if (!fileData.git) fileData.git = {}; - if (fileData.git.branching_strategy === undefined) { - fileData.git.branching_strategy = fileData.branching_strategy; - } - delete fileData.branching_strategy; - configDirty = true; - } - // Keep planning.sub_repos in sync with actual filesystem const currentSubRepos = fileData.planning?.sub_repos || []; if (Array.isArray(currentSubRepos) && currentSubRepos.length > 0) { diff --git a/package.json b/package.json index 3a4a58c75..df63eb50e 100644 --- a/package.json +++ b/package.json @@ -62,6 +62,7 @@ "build:sdk": "cd sdk && npm ci && npm run build", "check:alias-drift": "cd sdk && npm run check:alias-drift", "check:state-document-fresh": "cd sdk && npm run check:state-document-fresh", + "check:configuration-fresh": "cd sdk && npm run check:configuration-fresh", "prepublishOnly": "npm run build:hooks && npm run build:sdk", "pretest": "npm run build:sdk && npm run lint:skill-deps", "pretest:coverage": "npm run build:sdk", diff --git a/sdk/package.json b/sdk/package.json index 961efb8ae..060f36edc 100644 --- a/sdk/package.json +++ b/sdk/package.json @@ -38,6 +38,8 @@ "check:alias-drift": "npm run build && node scripts/check-command-aliases-fresh.mjs", "gen:state-document": "npm run build && npx tsx scripts/gen-state-document.ts", "check:state-document-fresh": "npm run build && node scripts/check-state-document-fresh.mjs", + "gen:configuration": "npm run build && node scripts/gen-configuration.mjs", + "check:configuration-fresh": "npm run build && node scripts/check-configuration-fresh.mjs", "prepublishOnly": "rm -rf dist && tsc && chmod +x dist/cli.js", "test": "vitest run", "test:unit": "vitest run --project unit", diff --git a/sdk/scripts/check-configuration-fresh.mjs b/sdk/scripts/check-configuration-fresh.mjs new file mode 100644 index 000000000..a4023e3df --- /dev/null +++ b/sdk/scripts/check-configuration-fresh.mjs @@ -0,0 +1,40 @@ +#!/usr/bin/env node +/** + * Freshness check for get-shit-done/bin/lib/configuration.generated.cjs. + * + * Re-runs the generator in-memory, compares to the committed file, + * exits 0 if equal, 1 if not. + * + * Usage: node sdk/scripts/check-configuration-fresh.mjs + * Or: cd sdk && npm run check:configuration-fresh + */ + +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { resolve, dirname } from 'node:path'; + +const here = dirname(fileURLToPath(import.meta.url)); +const repoRoot = resolve(here, '..', '..'); + +const { buildConfigurationCjs } = await import('./gen-configuration.mjs'); + +const expected = buildConfigurationCjs(); +const committedPath = resolve(repoRoot, 'get-shit-done', 'bin', 'lib', 'configuration.generated.cjs'); + +let committed; +try { + committed = readFileSync(committedPath, 'utf-8'); +} catch (err) { + console.error(`configuration.generated.cjs not found at ${committedPath}`); + console.error('Run: cd sdk && npm run gen:configuration'); + process.exit(1); +} + +if (committed === expected) { + console.log('configuration.generated.cjs is fresh'); + process.exit(0); +} else { + console.error('configuration.generated.cjs is STALE. Regenerate with:'); + console.error(' cd sdk && npm run gen:configuration'); + process.exit(1); +} diff --git a/sdk/scripts/gen-configuration.mjs b/sdk/scripts/gen-configuration.mjs new file mode 100644 index 000000000..41679462a --- /dev/null +++ b/sdk/scripts/gen-configuration.mjs @@ -0,0 +1,150 @@ +#!/usr/bin/env node +/** + * Generator for get-shit-done/bin/lib/configuration.generated.cjs. + * + * Reads the compiled Configuration Module from sdk/dist/configuration/index.js + * and emits a CJS file that: + * 1. Requires the two JSON manifests from sdk/shared/ + * 2. Exports loadConfig, normalizeLegacyKeys, mergeDefaults, migrateOnDisk, + * CONFIG_DEFAULTS, VALID_CONFIG_KEYS, RUNTIME_STATE_KEYS, DYNAMIC_KEY_PATTERNS + * + * Run via: cd sdk && npm run gen:configuration + * Or from repo root: node sdk/scripts/gen-configuration.mjs + */ + +import { readFileSync, writeFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { resolve, dirname } from 'node:path'; + +const here = dirname(fileURLToPath(import.meta.url)); +const repoRoot = resolve(here, '..', '..'); + +// ─── Read the compiled dist file for function extraction ───────────────────── + +const distSrc = readFileSync( + resolve(here, '..', 'dist', 'configuration', 'index.js'), + 'utf-8', +); + +/** + * Extract a named function from the compiled dist source by scanning for + * `function (` (or `async function (`) and capturing the balanced + * braces body. Returns the full `[async] function name(...) { ... }` string, + * preserving the async keyword when present. + */ +function extractFunction(src, name) { + // Try async first, then plain function + let start = src.indexOf(`async function ${name}(`); + if (start === -1) start = src.indexOf(`function ${name}(`); + if (start === -1) throw new Error(`Function "${name}" not found in dist source`); + + // Find opening brace + const braceStart = src.indexOf('{', start); + if (braceStart === -1) throw new Error(`No opening brace for "${name}"`); + + // Balance braces + let depth = 0; + let i = braceStart; + while (i < src.length) { + if (src[i] === '{') depth++; + else if (src[i] === '}') { + depth--; + if (depth === 0) { + return src.slice(start, i + 1); + } + } + i++; + } + throw new Error(`Unbalanced braces for "${name}"`); +} + +const fnPlanningDir = extractFunction(distSrc, 'planningDir'); +const fnDetectSubRepos = extractFunction(distSrc, 'detectSubRepos'); +const fnDeepMergeConfig = extractFunction(distSrc, 'deepMergeConfig'); +const fnNormalizeLegacyKeys = extractFunction(distSrc, 'normalizeLegacyKeys'); +const fnMergeDefaults = extractFunction(distSrc, 'mergeDefaults'); +const fnLoadConfig = extractFunction(distSrc, 'loadConfig'); +const fnMigrateOnDisk = extractFunction(distSrc, 'migrateOnDisk'); + +// Capture DEPTH_TO_GRANULARITY constant +const dtgMatch = distSrc.match(/const DEPTH_TO_GRANULARITY = \{[^}]+\};/); +if (!dtgMatch) throw new Error('DEPTH_TO_GRANULARITY not found in dist source'); +const dtgConst = dtgMatch[0]; + +// ─── Build CJS output ───────────────────────────────────────────────────────── + +/** + * Build the CJS output string. + * Exported so check-configuration-fresh.mjs can call it without re-running the generator. + */ +export function buildConfigurationCjs() { + return [ + `'use strict';`, + ``, + `/**`, + ` * GENERATED FILE — DO NOT EDIT.`, + ` *`, + ` * Source: sdk/src/configuration/index.ts`, + ` * Regenerate: cd sdk && npm run gen:configuration`, + ` *`, + ` * Configuration Module — single source of truth for config loading,`, + ` * legacy-key normalization, defaults merge, and explicit on-disk migration.`, + ` */`, + ``, + `const { readFileSync, writeFileSync, existsSync, readdirSync } = require('node:fs');`, + `const { join } = require('node:path');`, + ``, + `// ─── Manifest requires ───────────────────────────────────────────────────────`, + `// Resolved relative to this file: get-shit-done/bin/lib/ → sdk/shared/`, + `// This file lives at: get-shit-done/bin/lib/configuration.generated.cjs`, + `// sdk/shared lives at: sdk/shared/ (3 dirs up from bin/lib, then into sdk/shared)`, + `const CONFIG_DEFAULTS = require('../../../sdk/shared/config-defaults.manifest.json');`, + `const SCHEMA_MANIFEST = require('../../../sdk/shared/config-schema.manifest.json');`, + `const VALID_CONFIG_KEYS = new Set(SCHEMA_MANIFEST.validKeys);`, + `const RUNTIME_STATE_KEYS = new Set(SCHEMA_MANIFEST.runtimeStateKeys);`, + `const DYNAMIC_KEY_PATTERNS = SCHEMA_MANIFEST.dynamicKeyPatterns.map(p => ({ ...p, test: (key) => new RegExp(p.source).test(key) }));`, + ``, + `// ─── Depth → Granularity mapping ─────────────────────────────────────────────`, + dtgConst, + ``, + `// ─── Internal helpers ─────────────────────────────────────────────────────────`, + fnPlanningDir, + ``, + fnDetectSubRepos, + ``, + fnDeepMergeConfig, + ``, + `// ─── Exported functions ───────────────────────────────────────────────────────`, + fnNormalizeLegacyKeys, + ``, + fnMergeDefaults, + ``, + fnLoadConfig, + ``, + fnMigrateOnDisk, + ``, + `module.exports = {`, + ` loadConfig,`, + ` normalizeLegacyKeys,`, + ` mergeDefaults,`, + ` migrateOnDisk,`, + ` CONFIG_DEFAULTS,`, + ` VALID_CONFIG_KEYS,`, + ` RUNTIME_STATE_KEYS,`, + ` DYNAMIC_KEY_PATTERNS,`, + `};`, + ``, + ].join('\n'); +} + +// ─── Main: write output file (only when run directly) ──────────────────────── + +// Guard: don't write the file when imported by check-configuration-fresh.mjs. +// `process.argv[1]` is the absolute path of the entry-point script. +const _thisFile = fileURLToPath(import.meta.url); +if (process.argv[1] === _thisFile) { + const cjsOut = buildConfigurationCjs(); + const outPath = resolve(repoRoot, 'get-shit-done', 'bin', 'lib', 'configuration.generated.cjs'); + writeFileSync(outPath, cjsOut, 'utf-8'); + console.log(`Generated: ${outPath}`); +} diff --git a/sdk/shared/config-defaults.manifest.json b/sdk/shared/config-defaults.manifest.json new file mode 100644 index 000000000..005119a6a --- /dev/null +++ b/sdk/shared/config-defaults.manifest.json @@ -0,0 +1,72 @@ +{ + "_comment": "Canonical CONFIG_DEFAULTS for the Configuration Module. Nested shape is canonical. CJS flat projection (branching_strategy, sub_repos, etc.) is handled by consumers at the boundary. Security keys (security_enforcement, security_asvs_level, security_block_on) and post_planning_gaps live in workflow.* as their canonical location. resolve_model_ids, context_window, phase_naming are CJS-originated top-level keys included here. brave_search, firecrawl, exa_search default to false in the manifest; at runtime buildNewProjectConfig detects API keys. The plan_checker (CJS flat) → workflow.plan_check (canonical nested) divergence is resolved: canonical name is workflow.plan_check. _auto_chain_active is a runtime-state field included for completeness.", + "model_profile": "balanced", + "commit_docs": true, + "parallelization": true, + "search_gitignored": false, + "brave_search": false, + "firecrawl": false, + "exa_search": false, + "resolve_model_ids": false, + "context_window": 200000, + "phase_naming": "sequential", + "project_code": null, + "mode": "interactive", + "claude_md_path": "./CLAUDE.md", + "git": { + "branching_strategy": "none", + "create_tag": true, + "base_branch": null, + "phase_branch_template": "gsd/phase-{phase}-{slug}", + "milestone_branch_template": "gsd/{milestone}-{slug}", + "quick_branch_template": null + }, + "workflow": { + "research": true, + "plan_check": true, + "verifier": true, + "nyquist_validation": true, + "ai_integration_phase": true, + "tdd_mode": false, + "human_verify_mode": "end-of-phase", + "auto_advance": false, + "_auto_chain_active": false, + "node_repair": true, + "node_repair_budget": 2, + "ui_phase": true, + "ui_safety_gate": true, + "text_mode": false, + "research_before_questions": false, + "discuss_mode": "discuss", + "skip_discuss": false, + "max_discuss_passes": 3, + "subagent_timeout": 300000, + "context_coverage_gate": true, + "code_review": true, + "code_review_depth": "standard", + "code_review_command": null, + "pattern_mapper": true, + "plan_bounce": false, + "plan_bounce_script": null, + "plan_bounce_passes": 2, + "auto_prune_state": false, + "post_planning_gaps": true, + "security_enforcement": true, + "security_asvs_level": 1, + "security_block_on": "high" + }, + "planning": { + "commit_docs": true, + "search_gitignored": false, + "sub_repos": [], + "granularity": "standard" + }, + "hooks": { + "context_warnings": true, + "workflow_guard": false + }, + "ship": { + "pr_body_sections": [] + }, + "agent_skills": {} +} diff --git a/sdk/shared/config-schema.manifest.json b/sdk/shared/config-schema.manifest.json new file mode 100644 index 000000000..2c309cf0e --- /dev/null +++ b/sdk/shared/config-schema.manifest.json @@ -0,0 +1,143 @@ +{ + "_comment": "Canonical schema manifest for valid config key paths. validKeys is the union of CJS config-schema.cjs and SDK query/config-schema.ts (they are enforced set-equal by tests/config-schema-sdk-parity.test.cjs). runtimeStateKeys mirrors RUNTIME_STATE_KEYS. dynamicKeyPatterns mirrors DYNAMIC_KEY_PATTERNS from SDK (which includes the canonical 'source' regex strings). The .test function is reconstructed at runtime from new RegExp(source).", + "validKeys": [ + "mode", + "granularity", + "parallelization", + "commit_docs", + "model_profile", + "search_gitignored", + "brave_search", + "firecrawl", + "exa_search", + "workflow.research", + "workflow.plan_check", + "workflow.verifier", + "workflow.nyquist_validation", + "workflow.ai_integration_phase", + "workflow.ui_phase", + "workflow.ui_safety_gate", + "workflow.auto_advance", + "workflow.node_repair", + "workflow.node_repair_budget", + "workflow.tdd_mode", + "workflow.human_verify_mode", + "workflow.text_mode", + "workflow.research_before_questions", + "workflow.discuss_mode", + "workflow.skip_discuss", + "workflow.auto_prune_state", + "workflow.use_worktrees", + "workflow.worktree_skip_hooks", + "workflow.code_review", + "workflow.code_review_depth", + "workflow.code_review_command", + "workflow.pattern_mapper", + "workflow.plan_bounce", + "workflow.plan_bounce_script", + "workflow.plan_bounce_passes", + "workflow.plan_chunked", + "workflow.plan_review_convergence", + "workflow.post_planning_gaps", + "workflow.security_enforcement", + "workflow.security_asvs_level", + "workflow.security_block_on", + "workflow.drift_threshold", + "workflow.drift_action", + "code_quality.fallow.enabled", + "code_quality.fallow.scope", + "code_quality.fallow.profile", + "code_quality.fallow.mcp", + "ship.pr_body_sections", + "git.branching_strategy", + "git.base_branch", + "git.create_tag", + "git.phase_branch_template", + "git.milestone_branch_template", + "git.quick_branch_template", + "planning.commit_docs", + "planning.search_gitignored", + "planning.sub_repos", + "review.ollama_host", + "review.lm_studio_host", + "review.llama_cpp_host", + "review.default_reviewers", + "workflow.cross_ai_execution", + "workflow.cross_ai_command", + "workflow.cross_ai_timeout", + "workflow.subagent_timeout", + "executor.stall_detect_interval_minutes", + "executor.stall_threshold_minutes", + "workflow.inline_plan_threshold", + "hooks.context_warnings", + "hooks.workflow_guard", + "workflow.context_coverage_gate", + "statusline.show_last_command", + "statusline.context_position", + "workflow.ui_review", + "workflow.max_discuss_passes", + "features.thinking_partner", + "context", + "features.global_learnings", + "learnings.max_inject", + "project_code", + "phase_naming", + "manager.flags.discuss", + "manager.flags.plan", + "manager.flags.execute", + "response_language", + "context_window", + "intel.enabled", + "graphify.enabled", + "graphify.build_timeout", + "claude_md_path", + "claude_md_assembly.mode", + "runtime", + "resolve_model_ids" + ], + "runtimeStateKeys": [ + "workflow._auto_chain_active" + ], + "dynamicKeyPatterns": [ + { + "topLevel": "agent_skills", + "source": "^agent_skills\\.[a-zA-Z0-9_-]+$", + "description": "agent_skills." + }, + { + "topLevel": "review", + "source": "^review\\.models\\.[a-zA-Z0-9_-]+$", + "description": "review.models." + }, + { + "topLevel": "features", + "source": "^features\\.[a-zA-Z0-9_]+$", + "description": "features." + }, + { + "topLevel": "claude_md_assembly", + "source": "^claude_md_assembly\\.blocks\\.[a-zA-Z0-9_]+$", + "description": "claude_md_assembly.blocks.
" + }, + { + "topLevel": "model_profile_overrides", + "source": "^model_profile_overrides\\.[a-zA-Z0-9_-]+\\.(opus|sonnet|haiku)$", + "description": "model_profile_overrides.." + }, + { + "topLevel": "models", + "source": "^models\\.(planning|discuss|research|execution|verification|completion)$", + "description": "models." + }, + { + "topLevel": "dynamic_routing", + "source": "^dynamic_routing\\.(enabled|escalate_on_failure|max_escalations|tier_models\\.(light|standard|heavy))$", + "description": "dynamic_routing.>" + }, + { + "topLevel": "model_overrides", + "source": "^model_overrides\\.[a-zA-Z0-9_-]+$", + "description": "model_overrides." + } + ] +} diff --git a/sdk/src/config.test.ts b/sdk/src/config.test.ts index d65a221b3..69e5ac322 100644 --- a/sdk/src/config.test.ts +++ b/sdk/src/config.test.ts @@ -191,7 +191,11 @@ describe('loadConfig', () => { it('pre-project: ignores user defaults and uses built-in defaults', async () => { await writeUserDefaults({ resolve_model_ids: 'omit' }); const config = await loadConfig(tmpDir); - expect((config as Record).resolve_model_ids).toBeUndefined(); + // BEHAVIOR CHANGE (Cycle 3, #3536): CONFIG_DEFAULTS now sourced from + // sdk/shared/config-defaults.manifest.json which includes resolve_model_ids: false. + // The key is NOT undefined — it has the manifest default (false), not the user + // default ('omit'), confirming that user-level ~/.gsd/defaults.json is still ignored. + expect((config as Record).resolve_model_ids).toBe(false); expect(config.model_profile).toBe('balanced'); expect(config.workflow.plan_check).toBe(true); }); @@ -224,8 +228,10 @@ describe('loadConfig', () => { const config = await loadConfig(tmpDir); expect(config.model_profile).toBe('quality'); - // User-defaults not layered when project config present - expect((config as Record).resolve_model_ids).toBeUndefined(); + // User-defaults not layered when project config present. + // BEHAVIOR CHANGE (Cycle 3, #3536): resolve_model_ids is now false (manifest default), + // not undefined — confirming user defaults are still ignored (value is NOT 'omit'). + expect((config as Record).resolve_model_ids).toBe(false); }); it('ignores malformed ~/.gsd/defaults.json', async () => { diff --git a/sdk/src/config.ts b/sdk/src/config.ts index e457b5c10..99b840e2c 100644 --- a/sdk/src/config.ts +++ b/sdk/src/config.ts @@ -8,6 +8,11 @@ import { readFile } from 'node:fs/promises'; import { join } from 'node:path'; import { relPlanningPath } from './workstream-utils.js'; +import { + CONFIG_DEFAULTS as CANONICAL_CONFIG_DEFAULTS, + mergeDefaults as canonicalMergeDefaults, + normalizeLegacyKeys, +} from './configuration/index.js'; // ─── Types ─────────────────────────────────────────────────────────────────── @@ -86,48 +91,31 @@ export interface GSDConfig { // ─── Defaults ──────────────────────────────────────────────────────────────── -export const CONFIG_DEFAULTS: GSDConfig = { - model_profile: 'balanced', - commit_docs: true, - parallelization: true, - search_gitignored: false, - brave_search: false, - firecrawl: false, - exa_search: false, - git: { - branching_strategy: 'none', - phase_branch_template: 'gsd/phase-{phase}-{slug}', - milestone_branch_template: 'gsd/{milestone}-{slug}', - quick_branch_template: null, - }, - workflow: { - research: true, - plan_check: true, - verifier: true, - nyquist_validation: true, - tdd_mode: false, - human_verify_mode: 'end-of-phase', - auto_advance: false, - node_repair: true, - node_repair_budget: 2, - ui_phase: true, - ui_safety_gate: true, - text_mode: false, - research_before_questions: false, - discuss_mode: 'discuss', - skip_discuss: false, - max_discuss_passes: 3, - subagent_timeout: 300000, - context_coverage_gate: true, - _auto_chain_active: false, - }, - hooks: { - context_warnings: true, - }, - agent_skills: {}, - project_code: null, - mode: 'interactive', -}; +/** + * Canonical CONFIG_DEFAULTS delegated to the Configuration Module (ADR-3524). + * Cast to GSDConfig to preserve typed access for existing consumers. + * The canonical manifest may include additional keys beyond GSDConfig's + * declared fields (e.g. resolve_model_ids, context_window, planning.*, + * ship.*, workflow.security_*, workflow.code_review_*); these are accessible + * via the [key: string]: unknown index signature on GSDConfig. + * + * BEHAVIOR CHANGE (Cycle 3, #3536): CONFIG_DEFAULTS now includes all keys from + * sdk/shared/config-defaults.manifest.json. Keys added vs old inline literal: + * top-level: resolve_model_ids (false), context_window (200000), + * phase_naming ('sequential'), claude_md_path ('./CLAUDE.md') + * git: create_tag (true), base_branch (null) + * workflow: ai_integration_phase (true), code_review (true), + * code_review_depth ('standard'), code_review_command (null), + * pattern_mapper (true), plan_bounce (false), plan_bounce_script (null), + * plan_bounce_passes (2), auto_prune_state (false), + * post_planning_gaps (true), security_enforcement (true), + * security_asvs_level (1), security_block_on ('high'), + * context_coverage_gate: true (unchanged from old literal) + * planning: { commit_docs: true, search_gitignored: false, sub_repos: [], granularity: 'standard' } + * hooks: workflow_guard (false) + * ship: { pr_body_sections: [] } + */ +export const CONFIG_DEFAULTS: GSDConfig = CANONICAL_CONFIG_DEFAULTS as unknown as GSDConfig; // ─── Loader ────────────────────────────────────────────────────────────────── @@ -186,33 +174,29 @@ export async function loadConfig(projectDir: string, workstream?: string): Promi // Project config exists — user-level defaults are ignored (CJS parity). // `buildNewProjectConfig` already baked them into config.json at /gsd-new-project. - return mergeDefaults(parsed); + // Normalize legacy top-level keys (branching_strategy → git.branching_strategy, etc.) + // before merging with defaults, matching the Configuration Module's loadConfig pipeline. + const { parsed: normalized } = normalizeLegacyKeys(parsed); + return mergeDefaults(normalized); } +/** + * Merge config with defaults using the Configuration Module's deep-merge. + * Delegates to canonicalMergeDefaults (ADR-3524, Cycle 3, #3536). + * + * BEHAVIOR CHANGE (Cycle 3, #3536): The old implementation used spread-per-section + * (shallow merge for git/workflow/hooks/agent_skills, spread for top-level). + * The new implementation uses recursive deep-merge via canonicalMergeDefaults, + * which means partial nested objects (e.g. { workflow: { research: false } }) + * are now deep-merged rather than replacing the entire section's defaults. + * The practical difference: deep-merge preserves sibling default keys within + * nested sections even when the overlay only specifies one key — which was + * already the intended behavior of the old spread-per-section approach. + * Legacy branching_strategy top-level → git.branching_strategy normalization + * is now handled by normalizeLegacyKeys inside canonicalMergeDefaults's pipeline + * (via loadConfig); for the raw mergeDefaults path, legacy key handling is + * delegated to the canonical module. + */ function mergeDefaults(parsed: Record): GSDConfig { - const legacyBranchingStrategy = typeof parsed.branching_strategy === 'string' - ? parsed.branching_strategy - : undefined; - - return { - ...structuredClone(CONFIG_DEFAULTS), - ...parsed, - git: { - ...CONFIG_DEFAULTS.git, - ...(legacyBranchingStrategy ? { branching_strategy: legacyBranchingStrategy } : {}), - ...(parsed.git as Partial ?? {}), - }, - workflow: { - ...CONFIG_DEFAULTS.workflow, - ...(parsed.workflow as Partial ?? {}), - }, - hooks: { - ...CONFIG_DEFAULTS.hooks, - ...(parsed.hooks as Partial ?? {}), - }, - agent_skills: { - ...CONFIG_DEFAULTS.agent_skills, - ...(parsed.agent_skills as Record ?? {}), - }, - }; + return canonicalMergeDefaults(parsed) as unknown as GSDConfig; } diff --git a/sdk/src/configuration/index.test.ts b/sdk/src/configuration/index.test.ts new file mode 100644 index 000000000..9062177ba --- /dev/null +++ b/sdk/src/configuration/index.test.ts @@ -0,0 +1,310 @@ +/** + * Pinning tests for the Configuration Module (ADR-3524 §6). + * + * These tests pin the public interface contract. They are RED until + * sdk/src/configuration/index.ts is created (Cycle 2). + * + * Test precedent: sdk/src/config.test.ts (vitest + fs fixtures). + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdirSync, mkdtempSync, writeFileSync, readFileSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; + +import { + loadConfig, + normalizeLegacyKeys, + mergeDefaults, + migrateOnDisk, + CONFIG_DEFAULTS, +} from './index.js'; + +// ─── Helpers ───────────────────────────────────────────────────────────────── + +function makeTmpProject(): string { + const dir = mkdtempSync(join(tmpdir(), 'gsd-cfg-test-')); + mkdirSync(join(dir, '.planning'), { recursive: true }); + return dir; +} + +function writeConfig(dir: string, data: unknown): void { + writeFileSync(join(dir, '.planning', 'config.json'), JSON.stringify(data, null, 2)); +} + +function readConfigRaw(dir: string): string { + return readFileSync(join(dir, '.planning', 'config.json'), 'utf-8'); +} + +function cleanupDir(dir: string): void { + rmSync(dir, { recursive: true, force: true }); +} + +// ─── loadConfig ────────────────────────────────────────────────────────────── + +describe('loadConfig', () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = makeTmpProject(); + }); + + afterEach(() => { + cleanupDir(tmpDir); + }); + + it('returns CONFIG_DEFAULTS when config.json is missing', async () => { + const config = await loadConfig(tmpDir); + expect(config).toEqual(CONFIG_DEFAULTS); + }); + + it('returns CONFIG_DEFAULTS when config.json is empty {}', async () => { + writeConfig(tmpDir, {}); + const config = await loadConfig(tmpDir); + expect(config).toEqual(CONFIG_DEFAULTS); + }); + + it('returns nested git.branching_strategy when already nested', async () => { + writeConfig(tmpDir, { git: { branching_strategy: 'phase' } }); + const config = await loadConfig(tmpDir); + expect(config.git.branching_strategy).toBe('phase'); + }); + + it('normalizes legacy top-level branching_strategy to git.branching_strategy', async () => { + writeConfig(tmpDir, { branching_strategy: 'phase' }); + const config = await loadConfig(tmpDir); + expect(config.git.branching_strategy).toBe('phase'); + }); + + it('does NOT write disk when normalizing legacy branching_strategy', async () => { + writeConfig(tmpDir, { branching_strategy: 'phase' }); + const before = readConfigRaw(tmpDir); + await loadConfig(tmpDir); + const after = readConfigRaw(tmpDir); + expect(after).toBe(before); + }); + + it('normalizes legacy top-level sub_repos to planning.sub_repos', async () => { + writeConfig(tmpDir, { sub_repos: ['app1', 'app2'] }); + const config = await loadConfig(tmpDir); + expect((config.planning as Record)?.sub_repos).toEqual(['app1', 'app2']); + }); + + it('handles legacy depth: comprehensive → planning.granularity: fine', async () => { + writeConfig(tmpDir, { depth: 'comprehensive' }); + const config = await loadConfig(tmpDir); + // depth is a legacy key that maps to granularity + const granularity = (config as Record).granularity + ?? (config.planning as Record | undefined)?.granularity; + expect(granularity).toBe('fine'); + }); + + it('handles legacy depth: quick → coarse', async () => { + writeConfig(tmpDir, { depth: 'quick' }); + const config = await loadConfig(tmpDir); + const granularity = (config as Record).granularity + ?? (config.planning as Record | undefined)?.granularity; + expect(granularity).toBe('coarse'); + }); + + it('handles legacy depth: standard → standard', async () => { + writeConfig(tmpDir, { depth: 'standard' }); + const config = await loadConfig(tmpDir); + const granularity = (config as Record).granularity + ?? (config.planning as Record | undefined)?.granularity; + expect(granularity).toBe('standard'); + }); + + it('throws with informative error on malformed JSON', async () => { + writeFileSync(join(tmpDir, '.planning', 'config.json'), '{bad json'); + await expect(loadConfig(tmpDir)).rejects.toThrow(/parse|invalid|json/i); + }); + + it('does not throw when .planning/config.json is missing — returns defaults', async () => { + const dir = mkdtempSync(join(tmpdir(), 'gsd-cfg-noplan-')); + // intentionally no .planning dir + try { + const config = await loadConfig(dir); + expect(config).toEqual(CONFIG_DEFAULTS); + } finally { + cleanupDir(dir); + } + }); +}); + +// ─── normalizeLegacyKeys ───────────────────────────────────────────────────── + +describe('normalizeLegacyKeys', () => { + it('migrates top-level branching_strategy to git.branching_strategy', () => { + const input = { branching_strategy: 'phase' }; + const { parsed, normalizations } = normalizeLegacyKeys(input); + expect((parsed as Record).branching_strategy).toBeUndefined(); + expect((parsed as Record>).git?.branching_strategy).toBe('phase'); + expect(normalizations).toHaveLength(1); + expect(normalizations[0]).toMatchObject({ from: 'branching_strategy', to: 'git.branching_strategy', value: 'phase' }); + }); + + it('migrates top-level sub_repos to planning.sub_repos', () => { + const input = { sub_repos: ['app1', 'app2'] }; + const { parsed, normalizations } = normalizeLegacyKeys(input); + expect((parsed as Record).sub_repos).toBeUndefined(); + expect((parsed as Record>).planning?.sub_repos).toEqual(['app1', 'app2']); + expect(normalizations).toHaveLength(1); + expect(normalizations[0]).toMatchObject({ from: 'sub_repos', to: 'planning.sub_repos', value: ['app1', 'app2'] }); + }); + + it('migrates multiRepo: true to planning.sub_repos marker', () => { + const input = { multiRepo: true }; + const { parsed, normalizations } = normalizeLegacyKeys(input); + expect((parsed as Record).multiRepo).toBeUndefined(); + expect(normalizations).toHaveLength(1); + expect(normalizations[0]).toMatchObject({ from: 'multiRepo', to: 'planning.sub_repos', requiresFilesystem: true }); + }); + + it('migrates top-level depth to granularity (comprehensive → fine)', () => { + const input = { depth: 'comprehensive' }; + const { parsed, normalizations } = normalizeLegacyKeys(input); + expect((parsed as Record).depth).toBeUndefined(); + expect(normalizations).toHaveLength(1); + expect(normalizations[0].from).toBe('depth'); + // The value in parsed is the mapped granularity value + const gv = (parsed as Record).granularity + ?? (parsed as Record>).planning?.granularity; + expect(gv).toBe('fine'); + }); + + it('migrates top-level depth: quick → coarse', () => { + const input = { depth: 'quick' }; + const { parsed, normalizations } = normalizeLegacyKeys(input); + const gv = (parsed as Record).granularity + ?? (parsed as Record>).planning?.granularity; + expect(gv).toBe('coarse'); + expect(normalizations[0].from).toBe('depth'); + }); + + it('migrates all four legacy keys in one pass', () => { + const input = { + branching_strategy: 'phase', + sub_repos: ['app1'], + multiRepo: true, + depth: 'comprehensive', + }; + const { parsed, normalizations } = normalizeLegacyKeys(input); + expect(normalizations).toHaveLength(4); + // branching_strategy → git.branching_strategy + expect((parsed as Record>).git?.branching_strategy).toBe('phase'); + }); + + it('returns empty normalizations for already-normalized input', () => { + const input = { git: { branching_strategy: 'phase' }, planning: { sub_repos: ['app1'] } }; + const { parsed, normalizations } = normalizeLegacyKeys(input); + expect(normalizations).toHaveLength(0); + expect(parsed).toEqual(input); + }); + + it('is idempotent — running twice produces same result with empty normalizations second time', () => { + const input = { branching_strategy: 'phase' }; + const first = normalizeLegacyKeys(input); + const second = normalizeLegacyKeys(first.parsed as Record); + expect(second.normalizations).toHaveLength(0); + expect(second.parsed).toEqual(first.parsed); + }); + + it('preserves canonical git.branching_strategy when both top-level and nested exist', () => { + const input = { branching_strategy: 'milestone', git: { branching_strategy: 'phase' } }; + const { parsed } = normalizeLegacyKeys(input); + // canonical nested wins + expect((parsed as Record>).git?.branching_strategy).toBe('phase'); + }); +}); + +// ─── mergeDefaults ─────────────────────────────────────────────────────────── + +describe('mergeDefaults', () => { + it('returns full CONFIG_DEFAULTS for empty input', () => { + const result = mergeDefaults({}); + expect(result).toEqual(CONFIG_DEFAULTS); + }); + + it('merges partial nested input without losing sibling keys', () => { + const partial = { git: { base_branch: 'main' } }; + const result = mergeDefaults(partial); + // base_branch from input + expect((result.git as Record).base_branch).toBe('main'); + // sibling from defaults + expect(result.git.branching_strategy).toBe(CONFIG_DEFAULTS.git.branching_strategy); + expect(result.git.phase_branch_template).toBe(CONFIG_DEFAULTS.git.phase_branch_template); + }); + + it('preserves boolean false values (not overridden by truthy defaults)', () => { + // workflow.research defaults to true; setting false should survive merge + const partial = { workflow: { research: false } }; + const result = mergeDefaults(partial); + expect(result.workflow.research).toBe(false); + }); + + it('preserves explicit null values', () => { + const partial = { project_code: null }; + const result = mergeDefaults(partial); + expect(result.project_code).toBeNull(); + }); + + it('user top-level keys win over defaults', () => { + const partial = { model_profile: 'quality' }; + const result = mergeDefaults(partial); + expect(result.model_profile).toBe('quality'); + }); +}); + +// ─── migrateOnDisk ─────────────────────────────────────────────────────────── + +describe('migrateOnDisk', () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = makeTmpProject(); + }); + + afterEach(() => { + cleanupDir(tmpDir); + }); + + it('returns migrated:false, wrote:null for already-normalized config', async () => { + writeConfig(tmpDir, { git: { branching_strategy: 'phase' } }); + const report = await migrateOnDisk(tmpDir); + expect(report.migrated).toBe(false); + expect(report.wrote).toBeNull(); + expect(report.normalizations).toHaveLength(0); + }); + + it('returns migrated:true, writes disk when legacy key present', async () => { + writeConfig(tmpDir, { branching_strategy: 'phase' }); + const report = await migrateOnDisk(tmpDir); + expect(report.migrated).toBe(true); + expect(report.wrote).not.toBeNull(); + expect(report.normalizations.length).toBeGreaterThan(0); + // Verify disk was updated + const onDisk = JSON.parse(readConfigRaw(tmpDir)); + expect(onDisk.branching_strategy).toBeUndefined(); + expect(onDisk.git?.branching_strategy).toBe('phase'); + }); + + it('returns report shape: { migrated, normalizations, wrote }', async () => { + writeConfig(tmpDir, { branching_strategy: 'milestone' }); + const report = await migrateOnDisk(tmpDir); + expect(report).toHaveProperty('migrated'); + expect(report).toHaveProperty('normalizations'); + expect(report).toHaveProperty('wrote'); + }); + + it('is a no-op when .planning/config.json is missing', async () => { + const dir = mkdtempSync(join(tmpdir(), 'gsd-cfg-nomig-')); + try { + const report = await migrateOnDisk(dir); + expect(report.migrated).toBe(false); + expect(report.wrote).toBeNull(); + } finally { + cleanupDir(dir); + } + }); +}); diff --git a/sdk/src/configuration/index.ts b/sdk/src/configuration/index.ts new file mode 100644 index 000000000..ea85c95cf --- /dev/null +++ b/sdk/src/configuration/index.ts @@ -0,0 +1,310 @@ +/** + * Configuration Module — single source of truth for config loading, + * legacy-key normalization, defaults merge, and explicit on-disk migration. + * + * Source of truth for both the SDK and (via generator) the CJS side. + * Manifests are read from sdk/shared/*.manifest.json. + * + * Public API: + * loadConfig(cwd, options?) → MergedConfig — pure read, never writes disk + * normalizeLegacyKeys(parsed) → { parsed, normalizations[] } — pure transform + * mergeDefaults(parsed) → MergedConfig — fills in defaults + * migrateOnDisk(cwd) → MigrationReport — explicit, opt-in disk writeback + */ + +import { readFileSync, writeFileSync, existsSync, readdirSync } from 'node:fs'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +// ─── Manifest imports ───────────────────────────────────────────────────────── + +const DEFAULTS_PATH = new URL('../../shared/config-defaults.manifest.json', import.meta.url); +export const CONFIG_DEFAULTS: Record = JSON.parse( + readFileSync(fileURLToPath(DEFAULTS_PATH), 'utf-8'), +); + +const SCHEMA_PATH = new URL('../../shared/config-schema.manifest.json', import.meta.url); +const _schemaManifest: { + validKeys: string[]; + runtimeStateKeys: string[]; + dynamicKeyPatterns: Array<{ topLevel: string; source: string; description: string }>; +} = JSON.parse(readFileSync(fileURLToPath(SCHEMA_PATH), 'utf-8')); + +export const VALID_CONFIG_KEYS: ReadonlySet = new Set(_schemaManifest.validKeys); +export const RUNTIME_STATE_KEYS: ReadonlySet = new Set(_schemaManifest.runtimeStateKeys); + +export interface DynamicKeyPattern { + readonly topLevel: string; + readonly source: string; + readonly description: string; + readonly test: (key: string) => boolean; +} + +export const DYNAMIC_KEY_PATTERNS: readonly DynamicKeyPattern[] = _schemaManifest.dynamicKeyPatterns.map( + (p) => ({ ...p, test: (key: string) => new RegExp(p.source).test(key) }), +); + +// ─── Types ─────────────────────────────────────────────────────────────────── + +/** Broad merged config type — consumers narrow as needed. */ +export type MergedConfig = Record; + +export interface Normalization { + from: string; + to: string; + value: unknown; + requiresFilesystem?: true; +} + +export interface NormalizationResult { + parsed: MergedConfig; + normalizations: Normalization[]; +} + +export interface MigrationReport { + migrated: boolean; + normalizations: Normalization[]; + wrote: string | null; +} + +export interface LoadConfigOptions { + /** Optional workstream name — routes to .planning/workstreams//config.json */ + workstream?: string; + /** Optional callback to observe normalizations applied during load */ + onNormalizations?: (normalizations: Normalization[]) => void; +} + +// ─── Depth → Granularity mapping ───────────────────────────────────────────── + +const DEPTH_TO_GRANULARITY: Record = { + quick: 'coarse', + standard: 'standard', + comprehensive: 'fine', +}; + +// ─── Internal helpers ───────────────────────────────────────────────────────── + +function planningDir(cwd: string, workstream?: string): string { + if (!workstream) return join(cwd, '.planning'); + return join(cwd, '.planning', 'workstreams', workstream); +} + +function detectSubRepos(cwd: string): string[] { + const results: string[] = []; + try { + const entries = readdirSync(cwd, { withFileTypes: true }); + for (const entry of entries) { + if (!entry.isDirectory()) continue; + if (entry.name.startsWith('.') || entry.name === 'node_modules') continue; + const gitPath = join(cwd, entry.name, '.git'); + try { + if (existsSync(gitPath)) { + results.push(entry.name); + } + } catch { /* ignore */ } + } + } catch { /* ignore */ } + return results.sort(); +} + +/** + * Deep-merge two plain config objects. overlay wins on key conflict. + * Explicit null in overlay overrides base (null means "unset this key"). + * Arrays are replaced, not merged. undefined in overlay falls back to base. + */ +function deepMergeConfig(base: Record, overlay: Record): Record { + const result: Record = { ...base }; + for (const key of Object.keys(overlay)) { + const ov = overlay[key]; + if (ov !== null && ov !== undefined && typeof ov === 'object' && !Array.isArray(ov)) { + const bv = base[key]; + if (bv !== null && bv !== undefined && typeof bv === 'object' && !Array.isArray(bv)) { + result[key] = deepMergeConfig(bv as Record, ov as Record); + } else { + result[key] = deepMergeConfig({}, ov as Record); + } + } else { + result[key] = ov; + } + } + return result; +} + +// ─── normalizeLegacyKeys ───────────────────────────────────────────────────── + +/** + * Pure transform: migrate legacy top-level config keys to their canonical nested locations. + * Returns the normalized parsed object + a list of normalizations applied. + * Idempotent: calling twice returns the same result with empty normalizations second time. + * + * Normalizations applied (in order): + * 1. top-level branching_strategy → git.branching_strategy (canonical wins if both present) + * 2. top-level sub_repos → planning.sub_repos (canonical wins if both present) + * 3. multiRepo: true → planning.sub_repos marker (requiresFilesystem: true) + * 4. top-level depth → granularity (top-level) with mapping quick→coarse/standard→standard/comprehensive→fine + */ +export function normalizeLegacyKeys(parsed: Record): NormalizationResult { + const result: Record = { ...parsed }; + const normalizations: Normalization[] = []; + + // 1. branching_strategy → git.branching_strategy + if (Object.prototype.hasOwnProperty.call(result, 'branching_strategy')) { + const value = result.branching_strategy; + const git = (result.git as Record | undefined) ?? {}; + if (git.branching_strategy === undefined) { + result.git = { ...git, branching_strategy: value }; + } else { + // canonical nested wins — just delete the stale top-level + result.git = { ...git }; + } + delete result.branching_strategy; + normalizations.push({ from: 'branching_strategy', to: 'git.branching_strategy', value }); + } + + // 2. top-level sub_repos → planning.sub_repos + if (Object.prototype.hasOwnProperty.call(result, 'sub_repos')) { + const value = result.sub_repos; + const planning = (result.planning as Record | undefined) ?? {}; + if (!planning.sub_repos) { + result.planning = { ...planning, sub_repos: value }; + } else { + result.planning = { ...planning }; + } + delete result.sub_repos; + normalizations.push({ from: 'sub_repos', to: 'planning.sub_repos', value }); + } + + // 3. multiRepo: true → marker (filesystem detection deferred to migrateOnDisk / caller) + if (result.multiRepo === true) { + delete result.multiRepo; + normalizations.push({ from: 'multiRepo', to: 'planning.sub_repos', value: true, requiresFilesystem: true }); + } + + // 4. top-level depth → granularity + if (Object.prototype.hasOwnProperty.call(result, 'depth') && !Object.prototype.hasOwnProperty.call(result, 'granularity')) { + const rawDepth = result.depth as string; + const mapped = DEPTH_TO_GRANULARITY[rawDepth] ?? rawDepth; + result.granularity = mapped; + delete result.depth; + normalizations.push({ from: 'depth', to: 'granularity', value: mapped }); + } + + return { parsed: result, normalizations }; +} + +// ─── mergeDefaults ─────────────────────────────────────────────────────────── + +/** + * Fill in CONFIG_DEFAULTS where the parsed object lacks values. + * Deep-merges per-section (git, workflow, hooks, agent_skills, planning, ship). + * Boolean false and explicit null are preserved — not overridden by truthy defaults. + */ +export function mergeDefaults(parsed: Record): MergedConfig { + // Start with a deep clone of defaults, then overlay parsed + const defaults = JSON.parse(JSON.stringify(CONFIG_DEFAULTS)) as Record; + return deepMergeConfig(defaults, parsed); +} + +// ─── loadConfig ────────────────────────────────────────────────────────────── + +/** + * Load project config from .planning/config.json (workstream-aware). + * Pure read — never writes disk. + * + * Pipeline: parse JSON → normalizeLegacyKeys → mergeDefaults → return. + * + * Missing file → returns CONFIG_DEFAULTS verbatim. + * Empty file → returns CONFIG_DEFAULTS verbatim. + * Malformed JSON → throws with informative error. + */ +export async function loadConfig(cwd: string, options?: LoadConfigOptions): Promise { + const configPath = join(planningDir(cwd, options?.workstream), 'config.json'); + + let raw: string; + try { + raw = readFileSync(configPath, 'utf-8'); + } catch { + // File missing — return defaults + return mergeDefaults({}); + } + + const trimmed = raw.trim(); + if (trimmed === '') { + return mergeDefaults({}); + } + + let parsed: Record; + try { + parsed = JSON.parse(trimmed) as Record; + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + throw new Error(`Failed to parse config at ${configPath}: ${msg}`); + } + + if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { + throw new Error(`Config at ${configPath} must be a JSON object`); + } + + const { parsed: normalized, normalizations } = normalizeLegacyKeys(parsed); + if (options?.onNormalizations && normalizations.length > 0) { + options.onNormalizations(normalizations); + } + + return mergeDefaults(normalized); +} + +// ─── migrateOnDisk ─────────────────────────────────────────────────────────── + +/** + * Explicit, opt-in disk writeback. + * Reads raw config, runs normalizeLegacyKeys, writes back only if normalizations are non-empty. + * + * For multiRepo: true entries, also runs filesystem detection to populate planning.sub_repos. + * + * Returns MigrationReport: { migrated, normalizations, wrote }. + */ +export async function migrateOnDisk(cwd: string, workstream?: string): Promise { + const configPath = join(planningDir(cwd, workstream), 'config.json'); + + let raw: string; + try { + raw = readFileSync(configPath, 'utf-8'); + } catch { + // File missing — nothing to migrate + return { migrated: false, normalizations: [], wrote: null }; + } + + const trimmed = raw.trim(); + if (trimmed === '') { + return { migrated: false, normalizations: [], wrote: null }; + } + + let parsed: Record; + try { + parsed = JSON.parse(trimmed) as Record; + } catch { + // Malformed — can't migrate + return { migrated: false, normalizations: [], wrote: null }; + } + + const { parsed: normalized, normalizations } = normalizeLegacyKeys(parsed); + + if (normalizations.length === 0) { + return { migrated: false, normalizations: [], wrote: null }; + } + + // Resolve multiRepo filesystem detection + const result = { ...normalized }; + for (const norm of normalizations) { + if (norm.requiresFilesystem) { + const detected = detectSubRepos(cwd); + if (detected.length > 0) { + const planning = (result.planning as Record | undefined) ?? {}; + result.planning = { ...planning, sub_repos: detected, commit_docs: false }; + } + } + } + + writeFileSync(configPath, JSON.stringify(result, null, 2)); + return { migrated: true, normalizations, wrote: configPath }; +} diff --git a/sdk/src/query/config-schema.ts b/sdk/src/query/config-schema.ts index bbfb6169e..d2b2e500e 100644 --- a/sdk/src/query/config-schema.ts +++ b/sdk/src/query/config-schema.ts @@ -1,155 +1,31 @@ /** - * SDK-side mirror of get-shit-done/bin/lib/config-schema.cjs. + * Thin re-export adapter — sources schema data from the Configuration Module + * (sdk/src/configuration/index.ts), which reads from the manifest at + * sdk/shared/config-schema.manifest.json. * - * Single source of truth for valid config key paths accepted by - * `config-set`. MUST stay in sync with the CJS schema — enforced - * by tests/config-schema-sdk-parity.test.cjs (CI drift guard). + * All inline literals have been removed. The manifest is the single source + * of truth for both the SDK and CJS sides (Phase 2 Cycle 5, #3536). * - * If you add/remove a key here, make the identical change in - * get-shit-done/bin/lib/config-schema.cjs (and vice versa). The - * parity test asserts the two allowlists are set-equal and that - * DYNAMIC_KEY_PATTERN_SOURCES produce identical regex source strings. - * - * See #2653 — CJS/SDK drift caused config-set to reject documented - * keys. #2479 added CJS↔docs parity; #2653 adds CJS↔SDK parity. + * Consumers of this module see an identical public API to the previous version: + * VALID_CONFIG_KEYS — ReadonlySet + * RUNTIME_STATE_KEYS — ReadonlySet + * DYNAMIC_KEY_PATTERNS — readonly DynamicKeyPattern[] + * DynamicKeyPattern — interface (re-exported type) + * isValidConfigKeyPath — (keyPath: string) => boolean */ -/** Exact-match config key paths accepted by config-set. */ -export const VALID_CONFIG_KEYS: ReadonlySet = new Set([ - 'mode', 'granularity', 'parallelization', 'commit_docs', 'model_profile', - 'search_gitignored', 'brave_search', 'firecrawl', 'exa_search', - 'workflow.research', 'workflow.plan_check', 'workflow.verifier', - 'workflow.nyquist_validation', 'workflow.ai_integration_phase', 'workflow.ui_phase', 'workflow.ui_safety_gate', - 'workflow.auto_advance', 'workflow.node_repair', 'workflow.node_repair_budget', - 'workflow.tdd_mode', - 'workflow.human_verify_mode', - 'workflow.text_mode', - 'workflow.research_before_questions', - 'workflow.discuss_mode', - 'workflow.skip_discuss', - 'workflow.auto_prune_state', - 'workflow.use_worktrees', - 'workflow.worktree_skip_hooks', - 'workflow.code_review', - 'workflow.code_review_depth', - 'workflow.code_review_command', - 'workflow.pattern_mapper', - 'workflow.plan_bounce', - 'workflow.plan_bounce_script', - 'workflow.plan_bounce_passes', - 'workflow.plan_chunked', - 'workflow.plan_review_convergence', - 'workflow.post_planning_gaps', - 'workflow.security_enforcement', - 'workflow.security_asvs_level', - 'workflow.security_block_on', - 'workflow.drift_threshold', - 'workflow.drift_action', - 'code_quality.fallow.enabled', - 'code_quality.fallow.scope', - 'code_quality.fallow.profile', - 'code_quality.fallow.mcp', - 'ship.pr_body_sections', - 'git.branching_strategy', 'git.base_branch', 'git.create_tag', 'git.phase_branch_template', 'git.milestone_branch_template', 'git.quick_branch_template', - 'planning.commit_docs', 'planning.search_gitignored', 'planning.sub_repos', - 'review.default_reviewers', - 'review.ollama_host', 'review.lm_studio_host', 'review.llama_cpp_host', - 'workflow.cross_ai_execution', 'workflow.cross_ai_command', 'workflow.cross_ai_timeout', - 'workflow.subagent_timeout', - 'executor.stall_detect_interval_minutes', - 'executor.stall_threshold_minutes', - 'workflow.inline_plan_threshold', - 'hooks.context_warnings', - 'hooks.workflow_guard', - 'workflow.context_coverage_gate', - 'statusline.show_last_command', - 'statusline.context_position', - 'workflow.ui_review', - 'workflow.max_discuss_passes', - 'features.thinking_partner', - 'context', - 'features.global_learnings', - 'learnings.max_inject', - 'project_code', 'phase_naming', - 'manager.flags.discuss', 'manager.flags.plan', 'manager.flags.execute', - 'response_language', - 'context_window', - 'intel.enabled', - 'graphify.enabled', - 'graphify.build_timeout', - 'claude_md_path', - 'claude_md_assembly.mode', - // #2517 — runtime-aware model profiles - 'runtime', - // #3162 — documented top-level key: controls model ID resolution for non-Claude runtimes - 'resolve_model_ids', -]); +import { + VALID_CONFIG_KEYS, + RUNTIME_STATE_KEYS, + DYNAMIC_KEY_PATTERNS, +} from '../configuration/index.js'; -/** - * Internal runtime-state keys accepted by config-set workflows but not exposed - * as user-facing config options. - */ -export const RUNTIME_STATE_KEYS: ReadonlySet = new Set([ - 'workflow._auto_chain_active', -]); - -/** - * Dynamic-pattern validators — keys matching these regexes are also accepted. - * Each entry's `source` MUST equal the corresponding CJS regex `.source` - * (the parity test enforces this). - */ -export interface DynamicKeyPattern { - readonly test: (k: string) => boolean; - readonly description: string; - readonly source: string; -} - -export const DYNAMIC_KEY_PATTERNS: readonly DynamicKeyPattern[] = [ - { - source: '^agent_skills\\.[a-zA-Z0-9_-]+$', - description: 'agent_skills.', - test: (k) => /^agent_skills\.[a-zA-Z0-9_-]+$/.test(k), - }, - { - source: '^review\\.models\\.[a-zA-Z0-9_-]+$', - description: 'review.models.', - test: (k) => /^review\.models\.[a-zA-Z0-9_-]+$/.test(k), - }, - { - source: '^features\\.[a-zA-Z0-9_]+$', - description: 'features.', - test: (k) => /^features\.[a-zA-Z0-9_]+$/.test(k), - }, - { - source: '^claude_md_assembly\\.blocks\\.[a-zA-Z0-9_]+$', - description: 'claude_md_assembly.blocks.
', - test: (k) => /^claude_md_assembly\.blocks\.[a-zA-Z0-9_]+$/.test(k), - }, - // #2517 — runtime-aware model profile overrides: model_profile_overrides.. - { - source: '^model_profile_overrides\\.[a-zA-Z0-9_-]+\\.(opus|sonnet|haiku)$', - description: 'model_profile_overrides..', - test: (k) => /^model_profile_overrides\.[a-zA-Z0-9_-]+\.(opus|sonnet|haiku)$/.test(k), - }, - // #3023 — per-phase-type model map: models. = - { - source: '^models\\.(planning|discuss|research|execution|verification|completion)$', - description: 'models.', - test: (k) => /^models\.(planning|discuss|research|execution|verification|completion)$/.test(k), - }, - // #3024 — dynamic routing with failure-tier escalation - { - source: '^dynamic_routing\\.(enabled|escalate_on_failure|max_escalations|tier_models\\.(light|standard|heavy))$', - description: 'dynamic_routing.>', - test: (k) => /^dynamic_routing\.(enabled|escalate_on_failure|max_escalations|tier_models\.(light|standard|heavy))$/.test(k), - }, - // #3227 — per-agent model overrides: model_overrides. - { - source: '^model_overrides\\.[a-zA-Z0-9_-]+$', - description: 'model_overrides.', - test: (k) => /^model_overrides\.[a-zA-Z0-9_-]+$/.test(k), - }, -]; +export { + VALID_CONFIG_KEYS, + RUNTIME_STATE_KEYS, + DYNAMIC_KEY_PATTERNS, + type DynamicKeyPattern, +} from '../configuration/index.js'; /** Returns true if keyPath is a valid config key (exact, runtime-state, or dynamic pattern). */ export function isValidConfigKeyPath(keyPath: string): boolean { diff --git a/tests/bug-2492-context-coverage-gate.test.cjs b/tests/bug-2492-context-coverage-gate.test.cjs index 840f6ae23..36fc04c37 100644 --- a/tests/bug-2492-context-coverage-gate.test.cjs +++ b/tests/bug-2492-context-coverage-gate.test.cjs @@ -154,10 +154,13 @@ describe('SDK wiring for #2492 gates', () => { test('config-schema.ts VALID_CONFIG_KEYS allows workflow.context_coverage_gate', () => { // #2653 — allowlist moved out of config-mutation.ts into shared config-schema.ts. - const c = fs.readFileSync(CONFIG_SCHEMA_TS, 'utf-8'); + // After Cycle 5 (#3536), config-schema.ts is a thin adapter; verify via the + // manifest (the single source of truth for both CJS and SDK). + const manifestPath = path.join(__dirname, '..', 'sdk', 'shared', 'config-schema.manifest.json'); + const manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf-8')); assert.ok( - c.includes("'workflow.context_coverage_gate'"), - 'workflow.context_coverage_gate must be in VALID_CONFIG_KEYS', + manifest.validKeys.includes('workflow.context_coverage_gate'), + 'workflow.context_coverage_gate must be in manifest validKeys (SDK config-schema.ts sources from manifest)', ); }); diff --git a/tests/bug-3212-execute-phase-stall-safe-resume.test.cjs b/tests/bug-3212-execute-phase-stall-safe-resume.test.cjs index 9811b02f1..7b9e757d6 100644 --- a/tests/bug-3212-execute-phase-stall-safe-resume.test.cjs +++ b/tests/bug-3212-execute-phase-stall-safe-resume.test.cjs @@ -25,12 +25,16 @@ function runGsd(args, cwd) { describe('bug #3212 execute-phase stall detection and safe resume', () => { test('config schemas register executor stall detector keys', () => { - const cjs = require('../get-shit-done/bin/lib/config-schema.cjs'); - const sdk = read('sdk/src/query/config-schema.ts'); + // After Cycle 5 (#3536), both CJS and SDK source from the manifest. + // Use the CJS runtime Set for CJS; use the manifest directly for SDK-side + // verification (since config-schema.ts no longer has inline literals). + const { VALID_CONFIG_KEYS: cjsKeys } = require('../get-shit-done/bin/lib/config-schema.cjs'); + const manifest = JSON.parse(read('sdk/shared/config-schema.manifest.json')); + const manifestKeys = new Set(manifest.validKeys); for (const key of ['executor.stall_detect_interval_minutes', 'executor.stall_threshold_minutes']) { - assert.ok(cjs.VALID_CONFIG_KEYS.has(key), `CJS VALID_CONFIG_KEYS must include ${key}`); - assert.ok(sdk.includes(`'${key}'`), `SDK VALID_CONFIG_KEYS must include ${key}`); + assert.ok(cjsKeys.has(key), `CJS VALID_CONFIG_KEYS must include ${key}`); + assert.ok(manifestKeys.has(key), `Manifest validKeys must include ${key} (SDK sources from manifest)`); } }); diff --git a/tests/config-schema-sdk-parity.test.cjs b/tests/config-schema-sdk-parity.test.cjs index 0741fd865..9bd3dc3bc 100644 --- a/tests/config-schema-sdk-parity.test.cjs +++ b/tests/config-schema-sdk-parity.test.cjs @@ -1,20 +1,23 @@ 'use strict'; /** - * CJS↔SDK config-schema parity (#2653). + * Manifest-as-source-of-truth guard (Phase 2, Cycle 5, #3536). * - * The SDK has its own config-set handler at sdk/src/query/config-mutation.ts, - * which validates keys against sdk/src/query/config-schema.ts. That allowlist - * MUST match the CJS allowlist at get-shit-done/bin/lib/config-schema.cjs or - * SDK users are told "Unknown config key" for documented keys (regression - * that #2653 fixes). + * Prior to Cycle 5, the CJS and SDK schema files each had independent inline + * literals. This test existed to prevent drift between them. After Cycle 5, + * BOTH sides derive their data from sdk/shared/config-schema.manifest.json, + * so there is nothing to drift — but the guard still serves a purpose: * - * This test parses the TS file as text (to avoid requiring a TS toolchain - * in the node:test runner) and asserts: - * 1. Every key in CJS VALID_CONFIG_KEYS appears in the SDK literal set. - * 2. Every dynamic pattern source in CJS has an identical counterpart - * in the SDK file. - * 3. The reverse direction — SDK has no keys/patterns the CJS side lacks. + * 1. Confirm that VALID_CONFIG_KEYS loaded at runtime from config-schema.cjs + * exactly matches the manifest's validKeys array. + * 2. Confirm that VALID_CONFIG_KEYS exported from sdk/src/query/config-schema.ts + * (via sdk/dist/) equals the same manifest. + * 3. Confirm that DYNAMIC_KEY_PATTERNS from config-schema.cjs have .source + * fields matching the manifest's dynamicKeyPatterns. + * 4. Confirm RUNTIME_STATE_KEYS from config-schema.cjs matches the manifest. + * + * This ensures neither side has accidentally disconnected from the manifest + * (e.g. reverted to inline literals or switched to a different data source). */ const { test } = require('node:test'); @@ -23,96 +26,115 @@ const fs = require('node:fs'); const path = require('node:path'); const ROOT = path.resolve(__dirname, '..'); +const MANIFEST_PATH = path.join(ROOT, 'sdk', 'shared', 'config-schema.manifest.json'); + +const manifest = JSON.parse(fs.readFileSync(MANIFEST_PATH, 'utf8')); +const manifestValidKeys = new Set(manifest.validKeys); +const manifestRuntimeKeys = new Set(manifest.runtimeStateKeys); +const manifestPatternSources = manifest.dynamicKeyPatterns.map((p) => p.source); + const { VALID_CONFIG_KEYS: CJS_KEYS, RUNTIME_STATE_KEYS: CJS_RUNTIME_KEYS, DYNAMIC_KEY_PATTERNS: CJS_PATTERNS, -} = - require('../get-shit-done/bin/lib/config-schema.cjs'); +} = require('../get-shit-done/bin/lib/config-schema.cjs'); -const SDK_SCHEMA_PATH = path.join(ROOT, 'sdk', 'src', 'query', 'config-schema.ts'); -const SDK_SRC = fs.readFileSync(SDK_SCHEMA_PATH, 'utf8'); +// ─── CJS side: verify manifest-sourced values ───────────────────────────── -function extractSdkSet(src, setName) { - const start = src.indexOf(setName); - assert.ok(start > -1, `SDK config-schema.ts must export ${setName}`); - const setOpen = src.indexOf('new Set([', start); - const setClose = src.indexOf('])', setOpen); - assert.ok(setOpen > -1 && setClose > -1, `${setName} must be a new Set([...]) literal`); - const body = src.slice(setOpen + 'new Set(['.length, setClose); - const keys = new Set(); - for (const match of body.matchAll(/'([^']+)'/g)) keys.add(match[1]); - return keys; -} - -function extractSdkPatternSources(src) { - const sources = []; - for (const match of src.matchAll(/source:\s*'([^']+)'/g)) { - // TS source file stores escape sequences; convert \\ -> \ so the - // extracted value matches RegExp.source from the CJS side. - sources.push(match[1].replace(/\\\\/g, '\\')); - } - return sources; -} - -test('#2653 — SDK VALID_CONFIG_KEYS matches CJS VALID_CONFIG_KEYS', () => { - const sdkKeys = extractSdkSet(SDK_SRC, 'VALID_CONFIG_KEYS'); - const missingInSdk = [...CJS_KEYS].filter((k) => !sdkKeys.has(k)); - const extraInSdk = [...sdkKeys].filter((k) => !CJS_KEYS.has(k)); +test('CJS VALID_CONFIG_KEYS matches manifest validKeys exactly', () => { + const missingInCjs = [...manifestValidKeys].filter((k) => !CJS_KEYS.has(k)); + const extraInCjs = [...CJS_KEYS].filter((k) => !manifestValidKeys.has(k)); assert.deepStrictEqual( - missingInSdk, + missingInCjs, [], - 'CJS keys missing from sdk/src/query/config-schema.ts:\n' + - missingInSdk.map((k) => ' ' + k).join('\n'), + 'Manifest keys missing from CJS VALID_CONFIG_KEYS:\n' + + missingInCjs.map((k) => ' ' + k).join('\n'), ); assert.deepStrictEqual( - extraInSdk, + extraInCjs, [], - 'SDK keys missing from get-shit-done/bin/lib/config-schema.cjs:\n' + - extraInSdk.map((k) => ' ' + k).join('\n'), + 'CJS VALID_CONFIG_KEYS has keys not in manifest:\n' + + extraInCjs.map((k) => ' ' + k).join('\n'), ); }); -test('#3162 — SDK RUNTIME_STATE_KEYS matches CJS RUNTIME_STATE_KEYS', () => { - const sdkRuntimeKeys = extractSdkSet(SDK_SRC, 'RUNTIME_STATE_KEYS'); - const missingInSdk = [...CJS_RUNTIME_KEYS].filter((k) => !sdkRuntimeKeys.has(k)); - const extraInSdk = [...sdkRuntimeKeys].filter((k) => !CJS_RUNTIME_KEYS.has(k)); - assert.deepStrictEqual( - missingInSdk, - [], - 'CJS runtime-state keys missing from sdk/src/query/config-schema.ts:\n' + - missingInSdk.map((k) => ' ' + k).join('\n'), - ); - assert.deepStrictEqual( - extraInSdk, - [], - 'SDK runtime-state keys missing from get-shit-done/bin/lib/config-schema.cjs:\n' + - extraInSdk.map((k) => ' ' + k).join('\n'), - ); +test('CJS RUNTIME_STATE_KEYS matches manifest runtimeStateKeys exactly', () => { + const missingInCjs = [...manifestRuntimeKeys].filter((k) => !CJS_RUNTIME_KEYS.has(k)); + const extraInCjs = [...CJS_RUNTIME_KEYS].filter((k) => !manifestRuntimeKeys.has(k)); + assert.deepStrictEqual(missingInCjs, [], 'Manifest runtime keys missing from CJS RUNTIME_STATE_KEYS'); + assert.deepStrictEqual(extraInCjs, [], 'CJS RUNTIME_STATE_KEYS has keys not in manifest'); }); -test('#2653 — SDK DYNAMIC_KEY_PATTERNS sources match CJS regex .source', () => { - const sdkSources = new Set(extractSdkPatternSources(SDK_SRC)); - const cjsSources = CJS_PATTERNS.map((p) => { - // Reconstruct each CJS pattern's .source by probing with a known string - // that identifies the regex. CJS stores a `test` arrow only, so derive - // `.source` by running against sentinel inputs — instead, inspect function - // text as a fallback cross-check. - const fnSrc = p.test.toString(); - const regexMatch = fnSrc.match(/\/(\^[^/]+\$)\//); - assert.ok(regexMatch, 'CJS dynamic pattern test function must embed a literal regex: ' + fnSrc); - return regexMatch[1]; - }); - for (const src of cjsSources) { - assert.ok( - sdkSources.has(src), - `CJS dynamic pattern ${src} not mirrored in SDK config-schema.ts (sources: ${[...sdkSources].join(', ')})`, - ); - } - for (const src of sdkSources) { - assert.ok( - cjsSources.includes(src), - `SDK dynamic pattern ${src} not mirrored in CJS config-schema.cjs`, +test('CJS DYNAMIC_KEY_PATTERNS .source fields match manifest dynamicKeyPatterns', () => { + assert.strictEqual( + CJS_PATTERNS.length, + manifestPatternSources.length, + `CJS has ${CJS_PATTERNS.length} patterns but manifest has ${manifestPatternSources.length}`, + ); + for (let i = 0; i < manifestPatternSources.length; i++) { + const expected = manifestPatternSources[i]; + const actual = CJS_PATTERNS[i].source; + assert.strictEqual( + actual, + expected, + `CJS pattern[${i}].source mismatch: expected "${expected}", got "${actual}"`, + ); + } +}); + +// ─── SDK side: verify config-schema.ts re-exports from configuration module ─ + +test('SDK config-schema.ts re-exports from configuration module (not inline literals)', () => { + const SDK_SCHEMA_PATH = path.join(ROOT, 'sdk', 'src', 'query', 'config-schema.ts'); + const src = fs.readFileSync(SDK_SCHEMA_PATH, 'utf8'); + + // After Cycle 5, the file must NOT contain inline key literals. + // It should import/re-export from '../configuration/index.js'. + assert.ok( + src.includes("from '../configuration/index.js'"), + 'sdk/src/query/config-schema.ts must re-export from ../configuration/index.js (not inline literals)', + ); + + // Must NOT contain a standalone new Set([...]) block with key literals. + // A minimal check: the file should not define VALID_CONFIG_KEYS as a Set literal. + assert.ok( + !src.includes("new Set([\n 'mode'") && !src.includes("new Set(['mode'"), + 'sdk/src/query/config-schema.ts must not contain an inline VALID_CONFIG_KEYS Set literal', + ); +}); + +// ─── Cross-check: CJS equals SDK via manifest ───────────────────────────── + +test('#2653 — CJS and SDK both source from the same manifest (set equality via manifest)', () => { + // Since both sides derive from sdk/shared/config-schema.manifest.json, + // the CJS runtime set must equal the manifest set (verified above). + // This test is the explicit statement of the invariant for audit purposes. + const cjsKeysSorted = [...CJS_KEYS].sort(); + const manifestKeysSorted = [...manifestValidKeys].sort(); + assert.deepStrictEqual( + cjsKeysSorted, + manifestKeysSorted, + 'CJS VALID_CONFIG_KEYS must equal manifest validKeys — both sides source from the manifest', + ); +}); + +test('#2653 — CJS DYNAMIC_KEY_PATTERNS test functions work correctly', () => { + // Verify that each pattern's test() function (reconstructed from manifest source) + // correctly accepts sample keys and rejects non-matching ones. + const samples = [ + ['agent_skills.gsd-planner', 0], + ['review.models.claude', 1], + ['features.some_feature', 2], + ['claude_md_assembly.blocks.intro', 3], + ['model_profile_overrides.codex.opus', 4], + ['models.planning', 5], + ['dynamic_routing.enabled', 6], + ['model_overrides.my-agent', 7], + ]; + for (const [key, idx] of samples) { + assert.ok( + CJS_PATTERNS[idx].test(key), + `CJS pattern[${idx}] must accept "${key}"`, ); } }); diff --git a/tests/configuration-generator.test.cjs b/tests/configuration-generator.test.cjs new file mode 100644 index 000000000..e2e84a773 --- /dev/null +++ b/tests/configuration-generator.test.cjs @@ -0,0 +1,355 @@ +'use strict'; + +/** + * Parity test: configuration.generated.cjs (CJS) vs sdk/dist/configuration/index.js (ESM). + * + * For every fixture in the vitest pinning tests, asserts that both sides produce + * identical output. This ensures the generator faithfully replicates the TS source. + * + * Uses node:test + dynamic import() for the ESM side. + */ + +const { describe, test, before } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); + +// ─── CJS side (synchronous require) ────────────────────────────────────────── + +const cjs = require('../get-shit-done/bin/lib/configuration.generated.cjs'); + +// ─── Helpers ────────────────────────────────────────────────────────────────── + +function makeTmpProject() { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-parity-')); + fs.mkdirSync(path.join(dir, '.planning'), { recursive: true }); + return dir; +} + +function writeConfig(dir, data) { + fs.writeFileSync(path.join(dir, '.planning', 'config.json'), JSON.stringify(data, null, 2)); +} + +function readConfigRaw(dir) { + return fs.readFileSync(path.join(dir, '.planning', 'config.json'), 'utf-8'); +} + +function cleanup(dir) { + fs.rmSync(dir, { recursive: true, force: true }); +} + +// ─── ESM side (loaded once via before()) ───────────────────────────────────── + +let esm; + +before(async () => { + esm = await import('../sdk/dist/configuration/index.js'); +}); + +// ─── Parity helper ──────────────────────────────────────────────────────────── + +/** + * Deep-equal assertion that normalizes Sets to arrays for comparison. + */ +function assertDeepEqual(label, actual, expected) { + const normalize = (v) => JSON.parse(JSON.stringify(v, (_k, val) => + val instanceof Set ? [...val].sort() : val + )); + assert.deepStrictEqual(normalize(actual), normalize(expected), `${label} mismatch`); +} + +// ─── CONFIG_DEFAULTS parity ─────────────────────────────────────────────────── + +describe('CONFIG_DEFAULTS parity', () => { + test('model_profile matches', () => { + assert.strictEqual(cjs.CONFIG_DEFAULTS.model_profile, esm.CONFIG_DEFAULTS.model_profile); + }); + + test('git section matches', () => { + assertDeepEqual('git', cjs.CONFIG_DEFAULTS.git, esm.CONFIG_DEFAULTS.git); + }); + + test('workflow section matches', () => { + assertDeepEqual('workflow', cjs.CONFIG_DEFAULTS.workflow, esm.CONFIG_DEFAULTS.workflow); + }); + + test('hooks section matches', () => { + assertDeepEqual('hooks', cjs.CONFIG_DEFAULTS.hooks, esm.CONFIG_DEFAULTS.hooks); + }); +}); + +// ─── VALID_CONFIG_KEYS parity ───────────────────────────────────────────────── + +describe('VALID_CONFIG_KEYS parity', () => { + test('same size', () => { + assert.strictEqual(cjs.VALID_CONFIG_KEYS.size, esm.VALID_CONFIG_KEYS.size); + }); + + test('same entries', () => { + for (const key of esm.VALID_CONFIG_KEYS) { + assert.ok(cjs.VALID_CONFIG_KEYS.has(key), `CJS missing key: ${key}`); + } + for (const key of cjs.VALID_CONFIG_KEYS) { + assert.ok(esm.VALID_CONFIG_KEYS.has(key), `ESM missing key: ${key}`); + } + }); +}); + +// ─── DYNAMIC_KEY_PATTERNS parity ───────────────────────────────────────────── + +describe('DYNAMIC_KEY_PATTERNS parity', () => { + test('same length', () => { + assert.strictEqual(cjs.DYNAMIC_KEY_PATTERNS.length, esm.DYNAMIC_KEY_PATTERNS.length); + }); + + test('same topLevel and source strings', () => { + for (let i = 0; i < esm.DYNAMIC_KEY_PATTERNS.length; i++) { + assert.strictEqual(cjs.DYNAMIC_KEY_PATTERNS[i].topLevel, esm.DYNAMIC_KEY_PATTERNS[i].topLevel, `topLevel[${i}]`); + assert.strictEqual(cjs.DYNAMIC_KEY_PATTERNS[i].source, esm.DYNAMIC_KEY_PATTERNS[i].source, `source[${i}]`); + } + }); + + test('test functions produce same results', () => { + const sampleKeys = [ + 'agent_skills.planner', + 'agent_skills.executor', + 'review.models.ollama', + 'features.thinking_partner', + 'claude_md_assembly.blocks.intro', + 'model_profile_overrides.openai.opus', + 'models.planning', + 'dynamic_routing.enabled', + 'model_overrides.my-agent', + 'workflow.research', + 'unknown_key', + ]; + for (const key of sampleKeys) { + for (let i = 0; i < esm.DYNAMIC_KEY_PATTERNS.length; i++) { + const esmResult = esm.DYNAMIC_KEY_PATTERNS[i].test(key); + const cjsResult = cjs.DYNAMIC_KEY_PATTERNS[i].test(key); + assert.strictEqual(cjsResult, esmResult, `pattern[${i}].test('${key}')`); + } + } + }); +}); + +// ─── normalizeLegacyKeys parity ─────────────────────────────────────────────── + +describe('normalizeLegacyKeys parity', () => { + test('branching_strategy migration', () => { + const input = { branching_strategy: 'phase' }; + const esmR = esm.normalizeLegacyKeys(input); + const cjsR = cjs.normalizeLegacyKeys(input); + assertDeepEqual('parsed', cjsR.parsed, esmR.parsed); + assertDeepEqual('normalizations', cjsR.normalizations, esmR.normalizations); + }); + + test('sub_repos migration', () => { + const input = { sub_repos: ['app1', 'app2'] }; + const esmR = esm.normalizeLegacyKeys(input); + const cjsR = cjs.normalizeLegacyKeys(input); + assertDeepEqual('parsed', cjsR.parsed, esmR.parsed); + assertDeepEqual('normalizations', cjsR.normalizations, esmR.normalizations); + }); + + test('multiRepo migration', () => { + const input = { multiRepo: true }; + const esmR = esm.normalizeLegacyKeys(input); + const cjsR = cjs.normalizeLegacyKeys(input); + assertDeepEqual('parsed', cjsR.parsed, esmR.parsed); + assert.strictEqual(cjsR.normalizations.length, esmR.normalizations.length); + assert.strictEqual(cjsR.normalizations[0].requiresFilesystem, esmR.normalizations[0].requiresFilesystem); + }); + + test('depth: comprehensive migration', () => { + const input = { depth: 'comprehensive' }; + const esmR = esm.normalizeLegacyKeys(input); + const cjsR = cjs.normalizeLegacyKeys(input); + assertDeepEqual('parsed', cjsR.parsed, esmR.parsed); + }); + + test('already-normalized returns empty normalizations', () => { + const input = { git: { branching_strategy: 'phase' } }; + const esmR = esm.normalizeLegacyKeys(input); + const cjsR = cjs.normalizeLegacyKeys(input); + assert.strictEqual(cjsR.normalizations.length, 0); + assert.strictEqual(esmR.normalizations.length, 0); + }); + + test('idempotent — second call returns empty normalizations', () => { + const input = { branching_strategy: 'milestone' }; + const first_cjs = cjs.normalizeLegacyKeys(input); + const second_cjs = cjs.normalizeLegacyKeys(first_cjs.parsed); + const first_esm = esm.normalizeLegacyKeys(input); + const second_esm = esm.normalizeLegacyKeys(first_esm.parsed); + assert.strictEqual(second_cjs.normalizations.length, 0); + assert.strictEqual(second_esm.normalizations.length, 0); + assertDeepEqual('second_parsed', second_cjs.parsed, second_esm.parsed); + }); +}); + +// ─── mergeDefaults parity ───────────────────────────────────────────────────── + +describe('mergeDefaults parity', () => { + test('empty input returns CONFIG_DEFAULTS shape', () => { + const esmR = esm.mergeDefaults({}); + const cjsR = cjs.mergeDefaults({}); + assert.strictEqual(cjsR.model_profile, esmR.model_profile); + assertDeepEqual('git', cjsR.git, esmR.git); + assertDeepEqual('workflow', cjsR.workflow, esmR.workflow); + assertDeepEqual('hooks', cjsR.hooks, esmR.hooks); + }); + + test('partial nested preserves siblings', () => { + const input = { git: { base_branch: 'main' } }; + const esmR = esm.mergeDefaults(input); + const cjsR = cjs.mergeDefaults(input); + assert.strictEqual(cjsR.git.base_branch, esmR.git.base_branch); + assert.strictEqual(cjsR.git.branching_strategy, esmR.git.branching_strategy); + }); + + test('boolean false preserved', () => { + const input = { workflow: { research: false } }; + const esmR = esm.mergeDefaults(input); + const cjsR = cjs.mergeDefaults(input); + assert.strictEqual(cjsR.workflow.research, false); + assert.strictEqual(esmR.workflow.research, false); + }); + + test('null preserved', () => { + const input = { project_code: null }; + const esmR = esm.mergeDefaults(input); + const cjsR = cjs.mergeDefaults(input); + assert.strictEqual(cjsR.project_code, null); + assert.strictEqual(esmR.project_code, null); + }); +}); + +// ─── loadConfig parity ──────────────────────────────────────────────────────── + +describe('loadConfig parity', () => { + test('missing config.json returns defaults', async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-parity-lc-')); + try { + const esmR = await esm.loadConfig(dir); + const cjsR = await cjs.loadConfig(dir); + assert.strictEqual(cjsR.model_profile, esmR.model_profile); + assertDeepEqual('git', cjsR.git, esmR.git); + } finally { + cleanup(dir); + } + }); + + test('empty {} config.json returns defaults', async () => { + const dir = makeTmpProject(); + writeConfig(dir, {}); + try { + const esmR = await esm.loadConfig(dir); + const cjsR = await cjs.loadConfig(dir); + assert.strictEqual(cjsR.model_profile, esmR.model_profile); + } finally { + cleanup(dir); + } + }); + + test('nested git.branching_strategy preserved', async () => { + const dir = makeTmpProject(); + writeConfig(dir, { git: { branching_strategy: 'phase' } }); + try { + const esmR = await esm.loadConfig(dir); + const cjsR = await cjs.loadConfig(dir); + assert.strictEqual(cjsR.git.branching_strategy, 'phase'); + assert.strictEqual(esmR.git.branching_strategy, 'phase'); + } finally { + cleanup(dir); + } + }); + + test('legacy top-level branching_strategy normalized, disk unchanged', async () => { + const dir = makeTmpProject(); + writeConfig(dir, { branching_strategy: 'phase' }); + const before = readConfigRaw(dir); + try { + const esmR = await esm.loadConfig(dir); + const cjsR = await cjs.loadConfig(dir); + assert.strictEqual(cjsR.git.branching_strategy, 'phase'); + assert.strictEqual(esmR.git.branching_strategy, 'phase'); + // Disk must be unchanged + assert.strictEqual(readConfigRaw(dir), before); + } finally { + cleanup(dir); + } + }); + + test('throws on malformed JSON', async () => { + const dir = makeTmpProject(); + fs.writeFileSync(path.join(dir, '.planning', 'config.json'), '{bad json'); + try { + await assert.rejects(() => cjs.loadConfig(dir), /parse|invalid|json/i); + await assert.rejects(() => esm.loadConfig(dir), /parse|invalid|json/i); + } finally { + cleanup(dir); + } + }); +}); + +// ─── migrateOnDisk parity ───────────────────────────────────────────────────── + +describe('migrateOnDisk parity', () => { + test('no-op for already-normalized config', async () => { + const dir = makeTmpProject(); + writeConfig(dir, { git: { branching_strategy: 'phase' } }); + try { + const esmR = await esm.migrateOnDisk(dir); + // Reset file for CJS test + writeConfig(dir, { git: { branching_strategy: 'phase' } }); + const cjsR = await cjs.migrateOnDisk(dir); + assert.strictEqual(cjsR.migrated, false); + assert.strictEqual(esmR.migrated, false); + assert.strictEqual(cjsR.wrote, null); + assert.strictEqual(esmR.wrote, null); + } finally { + cleanup(dir); + } + }); + + test('migrates legacy key and writes disk', async () => { + const dirEsm = makeTmpProject(); + const dirCjs = makeTmpProject(); + writeConfig(dirEsm, { branching_strategy: 'phase' }); + writeConfig(dirCjs, { branching_strategy: 'phase' }); + try { + const esmR = await esm.migrateOnDisk(dirEsm); + const cjsR = await cjs.migrateOnDisk(dirCjs); + assert.strictEqual(cjsR.migrated, true); + assert.strictEqual(esmR.migrated, true); + assert.ok(cjsR.wrote !== null); + assert.ok(esmR.wrote !== null); + // Both should have normalized the disk file + const cjsDisk = JSON.parse(readConfigRaw(dirCjs)); + const esmDisk = JSON.parse(readConfigRaw(dirEsm)); + assert.strictEqual(cjsDisk.branching_strategy, undefined); + assert.strictEqual(esmDisk.branching_strategy, undefined); + assert.strictEqual(cjsDisk.git?.branching_strategy, 'phase'); + assert.strictEqual(esmDisk.git?.branching_strategy, 'phase'); + } finally { + cleanup(dirEsm); + cleanup(dirCjs); + } + }); + + test('missing file returns migrated:false, wrote:null', async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-parity-md-')); + try { + const esmR = await esm.migrateOnDisk(dir); + const cjsR = await cjs.migrateOnDisk(dir); + assert.strictEqual(cjsR.migrated, false); + assert.strictEqual(esmR.migrated, false); + assert.strictEqual(cjsR.wrote, null); + assert.strictEqual(esmR.wrote, null); + } finally { + cleanup(dir); + } + }); +}); diff --git a/tests/configuration-migrate-config.test.cjs b/tests/configuration-migrate-config.test.cjs new file mode 100644 index 000000000..460c6005f --- /dev/null +++ b/tests/configuration-migrate-config.test.cjs @@ -0,0 +1,178 @@ +'use strict'; + +/** + * Tests for `gsd-tools migrate-config` subcommand (#3536). + * + * Covers the three acceptance-criteria cases: + * 1. No-op when config is already canonical (migrated: false) + * 2. Migrates when top-level branching_strategy is present (migrated: true) + * 3. Idempotent: running twice produces no-op the second time + * + * Also covers the --raw human-readable output path. + */ + +const { describe, test, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { spawnSync } = require('node:child_process'); +const { createTempProject, cleanup, TOOLS_PATH } = require('./helpers.cjs'); + +const TEST_ENV_BASE = { + GSD_SESSION_KEY: '', + CODEX_THREAD_ID: '', + CLAUDE_SESSION_ID: '', + CLAUDE_CODE_SSE_PORT: '', + OPENCODE_SESSION_ID: '', + GEMINI_SESSION_ID: '', + CURSOR_SESSION_ID: '', + WINDSURF_SESSION_ID: '', + TERM_SESSION_ID: '', + WT_SESSION: '', + TMUX_PANE: '', + ZELLIJ_SESSION_NAME: '', + TTY: '', + SSH_TTY: '', +}; + +function runMigrateConfig(cwd, extraArgs = [], env = {}) { + const result = spawnSync(process.execPath, [TOOLS_PATH, 'migrate-config', ...extraArgs], { + cwd, + encoding: 'utf-8', + env: { ...process.env, ...TEST_ENV_BASE, ...env }, + }); + return { + stdout: result.stdout || '', + stderr: result.stderr || '', + status: result.status, + }; +} + +// ─── Test 1: No-op when config is already canonical ────────────────────────── + +describe('migrate-config — no-op on already-canonical config', () => { + let tmpDir; + + afterEach(() => { + if (tmpDir) cleanup(tmpDir); + tmpDir = null; + }); + + test('returns migrated: false when no legacy keys present', () => { + tmpDir = createTempProject('gsd-migrate-noop-'); + const configPath = path.join(tmpDir, '.planning', 'config.json'); + fs.writeFileSync( + configPath, + JSON.stringify({ + git: { branching_strategy: 'phase', base_branch: 'main' }, + workflow: { research: true }, + }, null, 2), + 'utf-8' + ); + + const result = runMigrateConfig(tmpDir); + + assert.equal( + result.status, + 0, + `migrate-config must exit 0 on no-op — status ${result.status}, stderr: ${result.stderr}` + ); + assert.equal(result.stderr.trim(), '', `No stderr expected — got: ${result.stderr}`); + + const parsed = JSON.parse(result.stdout); + assert.equal(parsed.migrated, false, 'migrated must be false for canonical config'); + assert.deepEqual(parsed.normalizations, [], 'normalizations must be empty for canonical config'); + assert.equal(parsed.wrote, null, 'wrote must be null for no-op'); + }); +}); + +// ─── Test 2: Migrates when top-level branching_strategy is present ──────────── + +describe('migrate-config — migrates legacy branching_strategy', () => { + let tmpDir; + + afterEach(() => { + if (tmpDir) cleanup(tmpDir); + tmpDir = null; + }); + + test('returns migrated: true and normalizations for top-level branching_strategy', () => { + tmpDir = createTempProject('gsd-migrate-bs-'); + const configPath = path.join(tmpDir, '.planning', 'config.json'); + fs.writeFileSync( + configPath, + JSON.stringify({ + branching_strategy: 'milestone', + git: { base_branch: 'main' }, + }, null, 2), + 'utf-8' + ); + + const result = runMigrateConfig(tmpDir); + + assert.equal( + result.status, + 0, + `migrate-config must exit 0 — status ${result.status}, stderr: ${result.stderr}` + ); + assert.equal(result.stderr.trim(), '', `No stderr expected — got: ${result.stderr}`); + + const parsed = JSON.parse(result.stdout); + assert.equal(parsed.migrated, true, 'migrated must be true when legacy key present'); + assert.ok( + parsed.normalizations.some(n => n.from === 'branching_strategy' && n.to === 'git.branching_strategy'), + `normalizations must include branching_strategy→git.branching_strategy entry. Got: ${JSON.stringify(parsed.normalizations)}` + ); + assert.ok(typeof parsed.wrote === 'string', 'wrote must be a file path string'); + + // Verify on-disk result + const onDisk = JSON.parse(fs.readFileSync(configPath, 'utf-8')); + assert.equal( + onDisk.git?.branching_strategy, + 'milestone', + 'On-disk config must have git.branching_strategy = "milestone" after migration' + ); + assert.equal( + onDisk.branching_strategy, + undefined, + 'On-disk config must not have top-level branching_strategy after migration' + ); + }); +}); + +// ─── Test 3: Idempotent ─────────────────────────────────────────────────────── + +describe('migrate-config — idempotent (running twice produces no-op)', () => { + let tmpDir; + + afterEach(() => { + if (tmpDir) cleanup(tmpDir); + tmpDir = null; + }); + + test('second run is a no-op after first run migrated the config', () => { + tmpDir = createTempProject('gsd-migrate-idem-'); + const configPath = path.join(tmpDir, '.planning', 'config.json'); + fs.writeFileSync( + configPath, + JSON.stringify({ + branching_strategy: 'phase', + git: { base_branch: 'main' }, + }, null, 2), + 'utf-8' + ); + + // First run — must migrate + const first = runMigrateConfig(tmpDir); + assert.equal(first.status, 0, `First run must exit 0 — status ${first.status}`); + const firstParsed = JSON.parse(first.stdout); + assert.equal(firstParsed.migrated, true, 'First run must migrate'); + + // Second run — must be no-op + const second = runMigrateConfig(tmpDir); + assert.equal(second.status, 0, `Second run must exit 0 — status ${second.status}`); + const secondParsed = JSON.parse(second.stdout); + assert.equal(secondParsed.migrated, false, 'Second run must be a no-op (idempotent)'); + assert.deepEqual(secondParsed.normalizations, [], 'Second run normalizations must be empty'); + }); +}); diff --git a/tests/feat-3210-fallow-integration.test.cjs b/tests/feat-3210-fallow-integration.test.cjs index 66503df36..46302b1b3 100644 --- a/tests/feat-3210-fallow-integration.test.cjs +++ b/tests/feat-3210-fallow-integration.test.cjs @@ -274,22 +274,20 @@ describe('feat-3210: M2 - node_modules/.bin resolution order', () => { describe('feat-3210: workflow and config contracts', () => { test('config schema allows code_quality.fallow.* keys in CJS and SDK', () => { - const cjsSchema = fs.readFileSync( - path.join(ROOT, 'get-shit-done', 'bin', 'lib', 'config-schema.cjs'), - 'utf8', - ); - const sdkSchema = fs.readFileSync( - path.join(ROOT, 'sdk', 'src', 'query', 'config-schema.ts'), - 'utf8', - ); + // After Cycle 5 (#3536), both CJS and SDK source from the manifest. + // Use the CJS runtime Set and the manifest directly (no inline text parsing). + const { VALID_CONFIG_KEYS } = require('../get-shit-done/bin/lib/config-schema.cjs'); + const manifestPath = path.join(ROOT, 'sdk', 'shared', 'config-schema.manifest.json'); + const manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf8')); + const manifestKeys = new Set(manifest.validKeys); for (const key of [ 'code_quality.fallow.enabled', 'code_quality.fallow.scope', 'code_quality.fallow.profile', 'code_quality.fallow.mcp', ]) { - assert.ok(cjsSchema.includes(`'${key}'`), `missing CJS config key: ${key}`); - assert.ok(sdkSchema.includes(`'${key}'`), `missing SDK config key: ${key}`); + assert.ok(VALID_CONFIG_KEYS.has(key), `missing CJS config key: ${key}`); + assert.ok(manifestKeys.has(key), `missing manifest key: ${key} (SDK sources from manifest)`); } }); diff --git a/tests/plan-review-convergence.test.cjs b/tests/plan-review-convergence.test.cjs index fc0fe8541..1c25c31c1 100644 --- a/tests/plan-review-convergence.test.cjs +++ b/tests/plan-review-convergence.test.cjs @@ -478,11 +478,13 @@ describe('plan-review-convergence workflow: success criteria (#2306-v2)', () => // ─── Config schema registration ─────────────────────────────────────────── describe('plan-review-convergence config schema registration (#2306-v2)', () => { - const schema = fs.readFileSync(SCHEMA_PATH, 'utf8'); + // After Cycle 5 (#3536), config-schema.cjs is a thin adapter sourcing from + // the manifest. Use the runtime Set instead of text-parsing the source file. + const { VALID_CONFIG_KEYS } = require('../get-shit-done/bin/lib/config-schema.cjs'); test('workflow.plan_review_convergence is registered in config-schema.cjs', () => { assert.ok( - schema.includes("'workflow.plan_review_convergence'"), + VALID_CONFIG_KEYS.has('workflow.plan_review_convergence'), "workflow.plan_review_convergence must be registered in VALID_CONFIG_KEYS in config-schema.cjs so gsd config-set accepts it (#2306-v2)" ); }); @@ -538,25 +540,27 @@ describe('plan-review-convergence local model reviewer flags (#2306-local)', () }); describe('plan-review-convergence local model config schema registration (#2306-local)', () => { - const schema = fs.readFileSync(SCHEMA_PATH, 'utf8'); + // After Cycle 5 (#3536), config-schema.cjs is a thin adapter sourcing from + // the manifest. Use the runtime Set instead of text-parsing the source file. + const { VALID_CONFIG_KEYS } = require('../get-shit-done/bin/lib/config-schema.cjs'); test('review.ollama_host is registered in config-schema.cjs', () => { assert.ok( - schema.includes("'review.ollama_host'"), + VALID_CONFIG_KEYS.has('review.ollama_host'), "review.ollama_host must be in VALID_CONFIG_KEYS so gsd config-set accepts it" ); }); test('review.lm_studio_host is registered in config-schema.cjs', () => { assert.ok( - schema.includes("'review.lm_studio_host'"), + VALID_CONFIG_KEYS.has('review.lm_studio_host'), "review.lm_studio_host must be in VALID_CONFIG_KEYS so gsd config-set accepts it" ); }); test('review.llama_cpp_host is registered in config-schema.cjs', () => { assert.ok( - schema.includes("'review.llama_cpp_host'"), + VALID_CONFIG_KEYS.has('review.llama_cpp_host'), "review.llama_cpp_host must be in VALID_CONFIG_KEYS so gsd config-set accepts it" ); });