fix: plan-phase Nyquist validation when research is disabled (#980) (#1002)

* fix: plan-phase Nyquist validation when research is disabled (#980)

plan-phase step 5.5 required Nyquist artifacts even when research was
disabled, creating an impossible state: no RESEARCH.md to extract
Validation Architecture from. Step 7.5 then told Claude to "disable
Nyquist in config" without specifying the exact key, causing Claude to
guess wrong keys that config-set silently accepted.

Three fixes:
- plan-phase step 5.5: skip when research_enabled is false
- plan-phase step 7.5: specify exact config-set command for disabling
- config-set: reject unknown keys with whitelist validation

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* test: update config-set tests for key whitelist validation

Tests that used arbitrary keys (some_number, some_string, a.b.c) now
use valid config keys to test the same coercion and nesting behavior.
Adds new test asserting unknown keys are rejected with error.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Fana
2026-03-12 19:36:11 +02:00
committed by GitHub
parent bc3d6db1c0
commit c81b20eb04
4 changed files with 41 additions and 15 deletions

View File

@@ -6,6 +6,16 @@ const fs = require('fs');
const path = require('path');
const { output, error } = require('./core.cjs');
const VALID_CONFIG_KEYS = new Set([
'mode', 'granularity', 'parallelization', 'commit_docs', 'model_profile',
'search_gitignored', 'brave_search',
'workflow.research', 'workflow.plan_check', 'workflow.verifier',
'workflow.nyquist_validation', 'workflow.ui_phase', 'workflow.ui_safety_gate',
'workflow._auto_chain_active',
'git.branching_strategy', 'git.phase_branch_template', 'git.milestone_branch_template',
'planning.commit_docs', 'planning.search_gitignored',
]);
function cmdConfigEnsureSection(cwd, raw) {
const configPath = path.join(cwd, '.planning', 'config.json');
const planningDir = path.join(cwd, '.planning');
@@ -88,6 +98,10 @@ function cmdConfigSet(cwd, keyPath, value, raw) {
error('Usage: config-set <key.path> <value>');
}
if (!VALID_CONFIG_KEYS.has(keyPath)) {
error(`Unknown config key: "${keyPath}". Valid keys: ${[...VALID_CONFIG_KEYS].sort().join(', ')}`);
}
// Parse value (handle booleans and numbers)
let parsedValue = value;
if (value === 'true') parsedValue = true;

View File

@@ -238,7 +238,9 @@ Task(
## 5.5. Create Validation Strategy
MANDATORY unless `nyquist_validation_enabled` is false.
Skip if `nyquist_validation_enabled` is false OR `research_enabled` is false.
If `research_enabled` is false and `nyquist_validation_enabled` is true: warn "Nyquist validation enabled but research disabled — VALIDATION.md cannot be created without RESEARCH.md. Plans will lack validation requirements (Dimension 8)." Continue to step 6.
```bash
grep -l "## Validation Architecture" "${PHASE_DIR}"/*-RESEARCH.md 2>/dev/null
@@ -281,15 +283,15 @@ CONTEXT_PATH=$(printf '%s\n' "$INIT" | jq -r '.context_path // empty')
## 7.5. Verify Nyquist Artifacts
Skip if `nyquist_validation_enabled` is false.
Skip if `nyquist_validation_enabled` is false OR `research_enabled` is false.
```bash
VALIDATION_EXISTS=$(ls "${PHASE_DIR}"/*-VALIDATION.md 2>/dev/null | head -1)
```
If missing and Nyquist enabled — ask user:
1. Re-run: `/gsd:plan-phase {PHASE} --research`
2. Disable Nyquist in config
1. Re-run with research: `/gsd:plan-phase {PHASE} --research`
2. Disable Nyquist: `node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" config-set workflow.nyquist_validation false`
3. Continue anyway (plans fail Dimension 8)
Proceed to Step 8 only if user selects 2 or 3.

View File

@@ -94,6 +94,8 @@ AskUserQuestion([
{ label: "No", description: "Skip validation research. Good for rapid prototyping or no-test phases." }
]
},
// Note: Nyquist validation depends on research output. If research is disabled,
// plan-phase automatically skips Nyquist steps (no RESEARCH.md to extract from).
{
question: "Git branching strategy?",
header: "Branching",

View File

@@ -241,21 +241,21 @@ describe('config-set command', () => {
});
test('coerces numeric strings to numbers', () => {
const result = runGsdTools('config-set some_number 42', tmpDir);
const result = runGsdTools('config-set granularity 42', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const config = readConfig(tmpDir);
assert.strictEqual(config.some_number, 42);
assert.strictEqual(typeof config.some_number, 'number');
assert.strictEqual(config.granularity, 42);
assert.strictEqual(typeof config.granularity, 'number');
});
test('preserves plain strings', () => {
const result = runGsdTools('config-set some_string hello', tmpDir);
const result = runGsdTools('config-set model_profile hello', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const config = readConfig(tmpDir);
assert.strictEqual(config.some_string, 'hello');
assert.strictEqual(typeof config.some_string, 'string');
assert.strictEqual(config.model_profile, 'hello');
assert.strictEqual(typeof config.model_profile, 'string');
});
test('sets nested values via dot-notation', () => {
@@ -266,17 +266,25 @@ describe('config-set command', () => {
assert.strictEqual(config.workflow.research, false);
});
test('auto-creates nested objects for deep dot-notation', () => {
test('auto-creates nested objects for dot-notation', () => {
// Start with empty config
writeConfig(tmpDir, {});
const result = runGsdTools('config-set a.b.c deep_value', tmpDir);
const result = runGsdTools('config-set workflow.research false', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const config = readConfig(tmpDir);
assert.strictEqual(config.a.b.c, 'deep_value');
assert.strictEqual(typeof config.a, 'object');
assert.strictEqual(typeof config.a.b, 'object');
assert.strictEqual(config.workflow.research, false);
assert.strictEqual(typeof config.workflow, 'object');
});
test('rejects unknown config keys', () => {
const result = runGsdTools('config-set workflow.nyquist_validation_enabled false', tmpDir);
assert.strictEqual(result.success, false);
assert.ok(
result.error.includes('Unknown config key'),
`Expected "Unknown config key" in error: ${result.error}`
);
});
test('errors when no key path provided', () => {