From 9aae41f22ddaba89a9edf887a1848359cb79ff46 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 24 May 2026 20:49:35 -0400 Subject: [PATCH] refactor(#179): migrate Configuration Module to sdk/src/config (#244) * refactor(#179): migrate configuration module to sdk/src/config * chore(#179): add changeset for config module path migration --- .changeset/plucky-tunas-howl.md | 5 +++++ CONTEXT.md | 2 +- docs/adr/3524-cjs-sdk-hard-seam.md | 2 +- docs/prd/3524-cjs-sdk-hard-seam.md | 2 +- get-shit-done/bin/lib/configuration.generated.cjs | 2 +- sdk/scripts/gen-configuration.mjs | 8 ++++---- sdk/src/config.ts | 2 +- sdk/src/{configuration => config}/index.test.ts | 2 +- sdk/src/{configuration => config}/index.ts | 0 sdk/src/query/config-mutation.ts | 4 ++-- sdk/src/query/config-schema.ts | 6 +++--- tests/config-schema-sdk-parity.test.cjs | 8 ++++---- tests/configuration-generator.test.cjs | 4 ++-- tests/gen-staleness-check.test.cjs | 4 ++-- 14 files changed, 28 insertions(+), 23 deletions(-) create mode 100644 .changeset/plucky-tunas-howl.md rename sdk/src/{configuration => config}/index.test.ts (99%) rename sdk/src/{configuration => config}/index.ts (100%) diff --git a/.changeset/plucky-tunas-howl.md b/.changeset/plucky-tunas-howl.md new file mode 100644 index 000000000..c05639146 --- /dev/null +++ b/.changeset/plucky-tunas-howl.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 179 +--- +migrated the SDK Configuration Module from sdk/src/configuration to sdk/src/config and rewired generator/test paths with no behavioral change. diff --git a/CONTEXT.md b/CONTEXT.md index 62d4cb4a7..107043c40 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -71,7 +71,7 @@ Single dispatch seam (`get-shit-done/bin/lib/command-routing-hub.cjs`) that cent 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. +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/config/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/adr/3524-cjs-sdk-hard-seam.md b/docs/adr/3524-cjs-sdk-hard-seam.md index 235ccd95e..e5284e221 100644 --- a/docs/adr/3524-cjs-sdk-hard-seam.md +++ b/docs/adr/3524-cjs-sdk-hard-seam.md @@ -32,7 +32,7 @@ The table below indexes by Module, not by physical layer. Each row names the sou | Module | Status | Source of truth | Generated artifacts | Adapters | |---|---|---|---|---| | **STATE.md Document Module** | New under this ADR (Phase 1) — see CONTEXT.md "STATE.md Document Module" | `sdk/src/state-document/index.ts` (promoted from `sdk/src/query/state-document.ts`) | `sdk/src/query/state-document.generated.ts`, `get-shit-done/bin/lib/state-document.generated.cjs` | `bin/lib/state.cjs` and `sdk/src/query/state*.ts` import the generated form | -| **Configuration Module** | New under this ADR (Phase 2) — definition added to CONTEXT.md as part of Phase 2 | `sdk/src/configuration/index.ts` plus data manifests `sdk/shared/config-schema.manifest.json` and `sdk/shared/config-defaults.manifest.json` | `sdk/src/query/config-schema.generated.ts`, `get-shit-done/bin/lib/config-schema.generated.cjs`, `get-shit-done/bin/lib/configuration.generated.cjs` | `bin/lib/config.cjs`, `bin/lib/core.cjs:loadConfig`, `sdk/src/config.ts` | +| **Configuration Module** | New under this ADR (Phase 2) — definition added to CONTEXT.md as part of Phase 2 | `sdk/src/config/index.ts` plus data manifests `sdk/shared/config-schema.manifest.json` and `sdk/shared/config-defaults.manifest.json` | `sdk/src/query/config-schema.generated.ts`, `get-shit-done/bin/lib/config-schema.generated.cjs`, `get-shit-done/bin/lib/configuration.generated.cjs` | `bin/lib/config.cjs`, `bin/lib/core.cjs:loadConfig`, `sdk/src/config.ts` | | **Workstream Inventory Module** (Builder) | Amended under this ADR (Phase 3) — Builder split documented in CONTEXT.md update | `sdk/src/workstream-inventory/builder.ts` (pure projection from directory entries + STATE.md text + plan scan results → typed inventory) | `sdk/src/query/workstream-inventory-builder.generated.ts`, `get-shit-done/bin/lib/workstream-inventory-builder.generated.cjs` | Per-side fs Readers (`workstream-inventory.cjs` sync, `workstream-inventory.ts` async) call the Builder. Readers stay hand-authored because the fs idiom legitimately differs. | | **Project-Root Resolution Module** | New under this ADR (Phase 4) — short CONTEXT.md entry, behavior already de-facto shared | `sdk/src/project-root/index.ts` | `get-shit-done/bin/lib/project-root.generated.cjs` | `bin/lib/core.cjs` (`findProjectRoot`, `findEffectiveRoot`), `sdk/src/helpers.ts` | | **Frontmatter Module** | Conditional (Phase 3, only if drift catalogue confirms pair duplication) | `sdk/src/frontmatter/index.ts` | `get-shit-done/bin/lib/frontmatter.generated.cjs` | Existing handler call sites | diff --git a/docs/prd/3524-cjs-sdk-hard-seam.md b/docs/prd/3524-cjs-sdk-hard-seam.md index 2dfd465c2..686932d2e 100644 --- a/docs/prd/3524-cjs-sdk-hard-seam.md +++ b/docs/prd/3524-cjs-sdk-hard-seam.md @@ -89,7 +89,7 @@ Phases are sized to ship in one to two PRs each. Each phase has its own GitHub i **Scope:** - Add a **Configuration Module** entry to `CONTEXT.md` first. Definition: "Module owning config load, legacy-key normalization, defaults merge, and explicit on-disk migration for `.planning/config.json`." Interface and invariants per ADR §6. - Extract `CONFIG_DEFAULTS`, `VALID_CONFIG_KEYS`, `DYNAMIC_KEY_PATTERNS`, `RUNTIME_STATE_KEYS` to two data manifests: `sdk/shared/config-schema.manifest.json` and `sdk/shared/config-defaults.manifest.json`. Precedent: `sdk/shared/model-catalog.json`. -- Write the Configuration Module source at `sdk/src/configuration/index.ts`. Implementation imports the two manifests and exports `loadConfig`, `normalizeLegacyKeys`, `mergeDefaults`, `migrateOnDisk`. +- Write the Configuration Module source at `sdk/src/config/index.ts`. Implementation imports the two manifests and exports `loadConfig`, `normalizeLegacyKeys`, `mergeDefaults`, `migrateOnDisk`. - Write `sdk/scripts/gen-configuration.ts` to emit `get-shit-done/bin/lib/configuration.generated.cjs` and (if needed) `sdk/src/query/config-schema.generated.ts`. - Write `sdk/scripts/check-configuration-fresh.mjs`. - Replace the inline implementations in `bin/lib/core.cjs:loadConfig` (lines 220–243, 434–449, 485) and `bin/lib/config.cjs` (the validation surface) with thin Adapters over the generated Module. Delete the inline `CONFIG_DEFAULTS`, the false-positive warning at `core.cjs:444-449`, and the duplicated `_deepMergeConfig`. diff --git a/get-shit-done/bin/lib/configuration.generated.cjs b/get-shit-done/bin/lib/configuration.generated.cjs index 9872fb4cc..e6a2bcea0 100644 --- a/get-shit-done/bin/lib/configuration.generated.cjs +++ b/get-shit-done/bin/lib/configuration.generated.cjs @@ -3,7 +3,7 @@ /** * GENERATED FILE — DO NOT EDIT. * - * Source: sdk/src/configuration/index.ts + * Source: sdk/src/config/index.ts * Regenerate: cd sdk && npm run gen:configuration * * Configuration Module — single source of truth for config loading, diff --git a/sdk/scripts/gen-configuration.mjs b/sdk/scripts/gen-configuration.mjs index 9ba8b765b..35d4be06a 100644 --- a/sdk/scripts/gen-configuration.mjs +++ b/sdk/scripts/gen-configuration.mjs @@ -2,7 +2,7 @@ /** * Generator for get-shit-done/bin/lib/configuration.generated.cjs. * - * Reads the compiled Configuration Module from sdk/dist/configuration/index.js + * Reads the compiled Configuration Module from sdk/dist/config/index.js * and emits a CJS file that: * 1. Requires the two JSON manifests from sdk/shared/ * 2. Exports loadConfig, normalizeLegacyKeys, mergeDefaults, migrateOnDisk, @@ -17,14 +17,14 @@ import { fileURLToPath } from 'node:url'; import { resolve, dirname } from 'node:path'; import { requireFreshDist } from './_gen-helpers.mjs'; -requireFreshDist('sdk/dist/configuration/index.js', 'sdk/src/configuration/index.ts'); +requireFreshDist('sdk/dist/config/index.js', 'sdk/src/config/index.ts'); const here = dirname(fileURLToPath(import.meta.url)); const repoRoot = resolve(here, '..', '..'); // ─── Read the compiled dist file for function extraction ───────────────────── -const distPath = resolve(here, '..', 'dist', 'configuration', 'index.js'); +const distPath = resolve(here, '..', 'dist', 'config', 'index.js'); const distSrc = readFileSync(distPath, 'utf-8'); /** @@ -85,7 +85,7 @@ export function buildConfigurationCjs() { `/**`, ` * GENERATED FILE — DO NOT EDIT.`, ` *`, - ` * Source: sdk/src/configuration/index.ts`, + ` * Source: sdk/src/config/index.ts`, ` * Regenerate: cd sdk && npm run gen:configuration`, ` *`, ` * Configuration Module — single source of truth for config loading,`, diff --git a/sdk/src/config.ts b/sdk/src/config.ts index 8414b99a5..7c94e8341 100644 --- a/sdk/src/config.ts +++ b/sdk/src/config.ts @@ -12,7 +12,7 @@ import { CONFIG_DEFAULTS as CANONICAL_CONFIG_DEFAULTS, mergeDefaults as canonicalMergeDefaults, normalizeLegacyKeys, -} from './configuration/index.js'; +} from './config/index.js'; // ─── Types ─────────────────────────────────────────────────────────────────── diff --git a/sdk/src/configuration/index.test.ts b/sdk/src/config/index.test.ts similarity index 99% rename from sdk/src/configuration/index.test.ts rename to sdk/src/config/index.test.ts index fd19cf698..f7d2a4bd1 100644 --- a/sdk/src/configuration/index.test.ts +++ b/sdk/src/config/index.test.ts @@ -2,7 +2,7 @@ * 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). + * sdk/src/config/index.ts is created (Cycle 2). * * Test precedent: sdk/src/config.test.ts (vitest + fs fixtures). */ diff --git a/sdk/src/configuration/index.ts b/sdk/src/config/index.ts similarity index 100% rename from sdk/src/configuration/index.ts rename to sdk/src/config/index.ts diff --git a/sdk/src/query/config-mutation.ts b/sdk/src/query/config-mutation.ts index 664501675..52ba6b476 100644 --- a/sdk/src/query/config-mutation.ts +++ b/sdk/src/query/config-mutation.ts @@ -24,7 +24,7 @@ import { join } from 'node:path'; import { GSDError, ErrorClassification } from '../errors.js'; import { VALID_PROFILES, getAgentToModelMapForProfile } from './config-query.js'; import { VALID_CONFIG_KEYS, RUNTIME_STATE_KEYS, DYNAMIC_KEY_PATTERNS } from './config-schema.js'; -import { CONFIG_DEFAULTS } from '../configuration/index.js'; +import { CONFIG_DEFAULTS } from '../config/index.js'; import { planningPaths } from './helpers.js'; import { acquireStateLock, releaseStateLock } from './state-mutation.js'; import { maskIfSecret } from './secrets.js'; @@ -569,7 +569,7 @@ export const configNewProject: QueryHandler = async (args, projectDir, workstrea // Build default config. Source is the canonical Configuration Module manifest // at sdk/shared/config-defaults.manifest.json (CONFIG_DEFAULTS from - // sdk/src/configuration/index.ts) — but ONLY a subset is materialized at + // sdk/src/config/index.ts) — but ONLY a subset is materialized at // init time. Legacy CJS `buildNewProjectConfig` (bin/lib/config.cjs:155-210) // intentionally omits keys whose value is meaningful only when set // explicitly so config-get returns "Key not found" and workflows fall back diff --git a/sdk/src/query/config-schema.ts b/sdk/src/query/config-schema.ts index d2b2e500e..934a42e3e 100644 --- a/sdk/src/query/config-schema.ts +++ b/sdk/src/query/config-schema.ts @@ -1,6 +1,6 @@ /** * Thin re-export adapter — sources schema data from the Configuration Module - * (sdk/src/configuration/index.ts), which reads from the manifest at + * (sdk/src/config/index.ts), which reads from the manifest at * sdk/shared/config-schema.manifest.json. * * All inline literals have been removed. The manifest is the single source @@ -18,14 +18,14 @@ import { VALID_CONFIG_KEYS, RUNTIME_STATE_KEYS, DYNAMIC_KEY_PATTERNS, -} from '../configuration/index.js'; +} from '../config/index.js'; export { VALID_CONFIG_KEYS, RUNTIME_STATE_KEYS, DYNAMIC_KEY_PATTERNS, type DynamicKeyPattern, -} from '../configuration/index.js'; +} from '../config/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/config-schema-sdk-parity.test.cjs b/tests/config-schema-sdk-parity.test.cjs index 7eb1919bc..e61c7828e 100644 --- a/tests/config-schema-sdk-parity.test.cjs +++ b/tests/config-schema-sdk-parity.test.cjs @@ -90,15 +90,15 @@ test('CJS DYNAMIC_KEY_PATTERNS .source fields match manifest dynamicKeyPatterns' // ─── SDK side: verify config-schema.ts re-exports from configuration module ─ -test('SDK config-schema.ts re-exports from configuration module (not inline literals)', () => { +test('SDK config-schema.ts re-exports from config 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'. + // It should import/re-export from '../config/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)', + src.includes("from '../config/index.js'"), + 'sdk/src/query/config-schema.ts must re-export from ../config/index.js (not inline literals)', ); // Must NOT contain a standalone new Set([...]) block with key literals. diff --git a/tests/configuration-generator.test.cjs b/tests/configuration-generator.test.cjs index e2e84a773..f5995652a 100644 --- a/tests/configuration-generator.test.cjs +++ b/tests/configuration-generator.test.cjs @@ -1,7 +1,7 @@ 'use strict'; /** - * Parity test: configuration.generated.cjs (CJS) vs sdk/dist/configuration/index.js (ESM). + * Parity test: configuration.generated.cjs (CJS) vs sdk/dist/config/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. @@ -44,7 +44,7 @@ function cleanup(dir) { let esm; before(async () => { - esm = await import('../sdk/dist/configuration/index.js'); + esm = await import('../sdk/dist/config/index.js'); }); // ─── Parity helper ──────────────────────────────────────────────────────────── diff --git a/tests/gen-staleness-check.test.cjs b/tests/gen-staleness-check.test.cjs index d6f24ef05..d8cc3e054 100644 --- a/tests/gen-staleness-check.test.cjs +++ b/tests/gen-staleness-check.test.cjs @@ -73,8 +73,8 @@ const GENERATORS = [ }, { script: 'gen-configuration.mjs', - dist: 'sdk/dist/configuration/index.js', - ts: 'sdk/src/configuration/index.ts', + dist: 'sdk/dist/config/index.js', + ts: 'sdk/src/config/index.ts', }, ];