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.
183 lines
8.6 KiB
JavaScript
183 lines
8.6 KiB
JavaScript
// allow-test-rule: pending-migration-to-typed-ir [#2974]
|
|
// Tracked in #2974 for migration to typed-IR assertions per CONTRIBUTING.md
|
|
// "Prohibited: Raw Text Matching on Test Outputs". Per-file review may
|
|
// reclassify some entries as source-text-is-the-product during migration.
|
|
|
|
/**
|
|
* Bug #2492: Add gates to ensure discuss-phase decisions are translated to
|
|
* plans (plan-phase, BLOCKING) and verified against shipped artifacts
|
|
* (verify-phase, NON-BLOCKING).
|
|
*
|
|
* These workflow files are loaded as prompts by the corresponding subagents.
|
|
* The tests below verify that the prompt text contains the gate steps and
|
|
* the config-toggle skip clauses — losing them silently would regress the
|
|
* fix.
|
|
*/
|
|
|
|
const { describe, test } = require('node:test');
|
|
const assert = require('node:assert/strict');
|
|
const fs = require('fs');
|
|
const path = require('path');
|
|
|
|
const PLAN_PHASE = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'plan-phase.md');
|
|
const VERIFY_PHASE = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'verify-phase.md');
|
|
const CONFIG_TS = path.join(__dirname, '..', 'sdk', 'src', 'config.ts');
|
|
const CONFIG_MUTATION_TS = path.join(__dirname, '..', 'sdk', 'src', 'query', 'config-mutation.ts');
|
|
// #2653 — allowlist moved to shared schema module.
|
|
const CONFIG_SCHEMA_TS = path.join(__dirname, '..', 'sdk', 'src', 'query', 'config-schema.ts');
|
|
const CONFIG_GATES_TS = path.join(__dirname, '..', 'sdk', 'src', 'query', 'config-gates.ts');
|
|
const QUERY_INDEX_TS = path.join(__dirname, '..', 'sdk', 'src', 'query', 'registry-assembly.ts');
|
|
const QUERY_ASSEMBLY_DESCRIPTOR_TS = path.join(__dirname, '..', 'sdk', 'src', 'query', 'registry-assembly-descriptor.ts');
|
|
|
|
describe('plan-phase decision-coverage gate (#2492)', () => {
|
|
const md = fs.readFileSync(PLAN_PHASE, 'utf-8');
|
|
|
|
test('contains a Decision Coverage Gate step', () => {
|
|
assert.ok(
|
|
/Decision Coverage Gate/i.test(md),
|
|
'plan-phase.md must define a Decision Coverage Gate step',
|
|
);
|
|
});
|
|
|
|
test('invokes the check.decision-coverage-plan handler', () => {
|
|
assert.ok(
|
|
md.includes('check.decision-coverage-plan'),
|
|
'plan-phase.md must call gsd-sdk query check.decision-coverage-plan',
|
|
);
|
|
});
|
|
|
|
test('mentions workflow.context_coverage_gate skip clause', () => {
|
|
assert.ok(
|
|
md.includes('workflow.context_coverage_gate'),
|
|
'plan-phase.md must reference workflow.context_coverage_gate to allow skipping',
|
|
);
|
|
});
|
|
|
|
test('decision gate appears AFTER the existing Requirements Coverage Gate', () => {
|
|
// Anchored heading regexes — avoid prose-substring traps (review F8/F9).
|
|
const reqIdx = md.search(/^## 13[a-z]?\.\s+Requirements Coverage Gate/m);
|
|
const decIdx = md.search(/^## 13[a-z]?\.\s+Decision Coverage Gate/m);
|
|
assert.ok(reqIdx !== -1, 'Requirements Coverage Gate heading must exist as ## 13[a-z]?.');
|
|
assert.ok(decIdx !== -1, 'Decision Coverage Gate heading must exist as ## 13[a-z]?.');
|
|
assert.ok(decIdx > reqIdx, 'Decision gate must run after Requirements gate');
|
|
});
|
|
|
|
test('decision gate appears BEFORE plans are committed', () => {
|
|
const decIdx = md.search(/^## 13[a-z]?\.\s+Decision Coverage Gate/m);
|
|
const commitIdx = md.search(/^## 13[a-z]?\.\s+Commit Plans/m);
|
|
assert.ok(decIdx !== -1, 'Decision Coverage Gate heading must exist as ## 13[a-z]?.');
|
|
assert.ok(commitIdx !== -1, 'Commit Plans heading must exist as ## 13[a-z]?.');
|
|
assert.ok(decIdx < commitIdx, 'Decision gate must run before commit so failures block the commit');
|
|
});
|
|
|
|
test('plan-phase Decision Coverage Gate uses CONTEXT_PATH variable defined in INIT extraction (review F1)', () => {
|
|
// The CONTEXT_PATH bash variable is defined at Step 4 (`CONTEXT_PATH=$(_gsd_field "$INIT" context_path)`).
|
|
// The plan-phase gate snippet must reference the same casing — `${CONTEXT_PATH}` — not `${context_path}`,
|
|
// otherwise the BLOCKING gate is invoked with an empty path and silently skips.
|
|
const defIdx = md.indexOf('CONTEXT_PATH=$(_gsd_field "$INIT" context_path)');
|
|
assert.ok(defIdx !== -1, 'CONTEXT_PATH must be defined from INIT JSON');
|
|
|
|
const gateIdx = md.indexOf('check.decision-coverage-plan');
|
|
assert.ok(gateIdx !== -1, 'check.decision-coverage-plan invocation must exist');
|
|
|
|
// Slice the surrounding gate snippet (~600 chars) and verify variable casing matches the definition.
|
|
const snippet = md.slice(Math.max(0, gateIdx - 200), gateIdx + 400);
|
|
assert.ok(
|
|
snippet.includes('${CONTEXT_PATH}'),
|
|
'Gate snippet must reference ${CONTEXT_PATH} (uppercase) to match the variable defined in Step 4',
|
|
);
|
|
assert.ok(
|
|
!snippet.includes('${context_path}'),
|
|
'Gate snippet must NOT reference ${context_path} (lowercase) — that name is undefined in shell scope',
|
|
);
|
|
});
|
|
|
|
test('plan-phase blocking gate exits non-zero on failure (review F15)', () => {
|
|
// The gate is documented as BLOCKING. To actually block, the shell snippet must
|
|
// exit with non-zero status when `passed` is false. Without exit-1 the workflow
|
|
// continues silently past the failure.
|
|
const gateIdx = md.indexOf('check.decision-coverage-plan');
|
|
assert.ok(gateIdx !== -1);
|
|
const snippet = md.slice(gateIdx, gateIdx + 800);
|
|
// Accept either an inline `|| exit 1` or a `|| { ...; exit 1; }` group.
|
|
const hasJqGuard = /jq[^\n]*passed\s*==\s*true/.test(snippet);
|
|
const hasExitOne = /\|\|\s*(?:exit\s+1|\{[\s\S]{0,200}?exit\s+1)/.test(snippet);
|
|
assert.ok(
|
|
hasJqGuard && hasExitOne,
|
|
'plan-phase gate must guard with `jq -e .passed == true || exit 1` (or `|| { ...; exit 1; }`) to actually block',
|
|
);
|
|
});
|
|
});
|
|
|
|
describe('verify-phase decision-coverage gate (#2492)', () => {
|
|
const md = fs.readFileSync(VERIFY_PHASE, 'utf-8');
|
|
|
|
test('contains a verify_decisions step', () => {
|
|
assert.ok(
|
|
/verify_decisions/.test(md),
|
|
'verify-phase.md must define a verify_decisions step',
|
|
);
|
|
});
|
|
|
|
test('invokes the check.decision-coverage-verify handler', () => {
|
|
assert.ok(
|
|
md.includes('check.decision-coverage-verify'),
|
|
'verify-phase.md must call gsd-sdk query check.decision-coverage-verify',
|
|
);
|
|
});
|
|
|
|
test('declares the decision gate as non-blocking / warning only', () => {
|
|
const lower = md.toLowerCase();
|
|
assert.ok(
|
|
lower.includes('non-blocking') || lower.includes('warning only') || lower.includes('not block'),
|
|
'verify-phase.md must declare the decision gate is non-blocking',
|
|
);
|
|
});
|
|
|
|
test('mentions workflow.context_coverage_gate skip clause', () => {
|
|
assert.ok(
|
|
md.includes('workflow.context_coverage_gate'),
|
|
'verify-phase.md must reference workflow.context_coverage_gate to allow skipping',
|
|
);
|
|
});
|
|
});
|
|
|
|
describe('SDK wiring for #2492 gates', () => {
|
|
test('config.ts WorkflowConfig has context_coverage_gate key', () => {
|
|
const c = fs.readFileSync(CONFIG_TS, 'utf-8');
|
|
assert.ok(c.includes('context_coverage_gate'), 'WorkflowConfig must declare context_coverage_gate');
|
|
assert.ok(
|
|
/context_coverage_gate:\s*true/.test(c),
|
|
'CONFIG_DEFAULTS.workflow.context_coverage_gate must default to true',
|
|
);
|
|
});
|
|
|
|
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.
|
|
// 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(
|
|
manifest.validKeys.includes('workflow.context_coverage_gate'),
|
|
'workflow.context_coverage_gate must be in manifest validKeys (SDK config-schema.ts sources from manifest)',
|
|
);
|
|
});
|
|
|
|
test('config-gates.ts surfaces context_coverage_gate', () => {
|
|
const c = fs.readFileSync(CONFIG_GATES_TS, 'utf-8');
|
|
assert.ok(
|
|
c.includes('context_coverage_gate'),
|
|
'check.config-gates must expose context_coverage_gate to workflows',
|
|
);
|
|
});
|
|
|
|
test('query registry assembly registers the new handlers', () => {
|
|
const c = fs.readFileSync(QUERY_INDEX_TS, 'utf-8');
|
|
const d = fs.readFileSync(QUERY_ASSEMBLY_DESCRIPTOR_TS, 'utf-8');
|
|
assert.ok(c.includes('REGISTRY_ASSEMBLY_PLAN'), 'registry assembly must be driven by declarative plan');
|
|
assert.ok(d.includes('VERIFY_DECISION_STATIC_CATALOG'), 'decision-coverage handlers must be sourced from verify-decision catalog');
|
|
assert.ok(d.includes("{ kind: 'static', key: 'VERIFY_DECISION_STATIC_CATALOG' }"), 'verify-decision catalog must be present in assembly plan');
|
|
});
|
|
});
|