fix: correct VALID_CONFIG_KEYS — remove internal state key, add missing public keys, migration hints (#2561)

* fix(#2530-2535): correct VALID_CONFIG_KEYS set — remove internal state key, add missing public keys, add migration hints

- Remove workflow._auto_chain_active from VALID_CONFIG_KEYS (internal runtime state, not user-settable) (#2530)
- Add hooks.workflow_guard to VALID_CONFIG_KEYS (read by gsd-workflow-guard.js hook, already documented) (#2531)
- Add workflow.ui_review to VALID_CONFIG_KEYS (read in autonomous.md via config-get) (#2532)
- Add workflow.max_discuss_passes to VALID_CONFIG_KEYS (read in discuss-phase.md via config-get) (#2533)
- Add CONFIG_KEY_SUGGESTIONS entries for sub_repos → planning.sub_repos and plan_checker → workflow.plan_check (#2535)
- Document workflow.ui_review and workflow.max_discuss_passes in docs/CONFIGURATION.md
- Clear INTERNAL_KEYS exemption in parity test (workflow._auto_chain_active removed from schema entirely)
- Add regression test file tests/bug-2530-valid-config-keys.test.cjs covering all 6 bugs

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

* fix: align SDK VALID_CONFIG_KEYS with CJS — remove internal key, add missing public keys

- Remove workflow._auto_chain_active from SDK (internal runtime state, not user-settable)
- Add workflow.ui_review, workflow.max_discuss_passes, hooks.workflow_guard to SDK
- Add ui_review and max_discuss_passes to Full Schema example in CONFIGURATION.md

Resolves CodeRabbit review on #2561.

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

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-04-22 11:28:25 -04:00
committed by GitHub
parent af2dba2328
commit c47a6a2164
6 changed files with 91 additions and 4 deletions

View File

@@ -30,10 +30,12 @@ GSD stores project settings in `.planning/config.json`. Created during `/gsd-new
"nyquist_validation": true,
"ui_phase": true,
"ui_safety_gate": true,
"ui_review": true,
"node_repair": true,
"node_repair_budget": 2,
"research_before_questions": false,
"discuss_mode": "discuss",
"max_discuss_passes": 3,
"skip_discuss": false,
"tdd_mode": false,
"text_mode": false,
@@ -139,10 +141,12 @@ All workflow toggles follow the **absent = enabled** pattern. If a key is missin
| `workflow.nyquist_validation` | boolean | `true` | Test coverage mapping during plan-phase research |
| `workflow.ui_phase` | boolean | `true` | Generate UI design contracts for frontend phases |
| `workflow.ui_safety_gate` | boolean | `true` | Prompt to run /gsd-ui-phase for frontend phases during plan-phase |
| `workflow.ui_review` | boolean | `true` | Run visual quality audit (`/gsd-ui-review`) after phase execution in autonomous mode. When `false`, the UI audit step is skipped. |
| `workflow.node_repair` | boolean | `true` | Autonomous task repair on verification failure |
| `workflow.node_repair_budget` | number | `2` | Max repair attempts per failed task |
| `workflow.research_before_questions` | boolean | `false` | Run research before discussion questions instead of after |
| `workflow.discuss_mode` | string | `'discuss'` | Controls how `/gsd-discuss-phase` gathers context. `'discuss'` (default) asks questions one-by-one. `'assumptions'` reads the codebase first, generates structured assumptions with confidence levels, and only asks you to correct what's wrong. Added in v1.28 |
| `workflow.max_discuss_passes` | number | `3` | Maximum number of question rounds in discuss-phase before the workflow stops asking. Useful in headless/auto mode to prevent infinite discussion loops. |
| `workflow.skip_discuss` | boolean | `false` | When `true`, `/gsd-autonomous` bypasses the discuss-phase entirely, writing minimal CONTEXT.md from the ROADMAP phase goal. Useful for projects where developer preferences are fully captured in PROJECT.md/REQUIREMENTS.md. Added in v1.28 |
| `workflow.text_mode` | boolean | `false` | Replaces AskUserQuestion TUI menus with plain-text numbered lists. Required for Claude Code remote sessions (`/rc` mode) where TUI menus don't render. Can also be set per-session with `--text` flag on discuss-phase. Added in v1.28 |
| `workflow.use_worktrees` | boolean | `true` | When `false`, disables git worktree isolation for parallel execution. Users who prefer sequential execution or whose environment does not support worktrees can disable this. Added in v1.31 |

View File

@@ -25,7 +25,6 @@ const VALID_CONFIG_KEYS = new Set([
'workflow.discuss_mode',
'workflow.skip_discuss',
'workflow.auto_prune_state',
'workflow._auto_chain_active',
'workflow.use_worktrees',
'workflow.code_review',
'workflow.code_review_depth',
@@ -44,6 +43,9 @@ const VALID_CONFIG_KEYS = new Set([
'workflow.subagent_timeout',
'workflow.inline_plan_threshold',
'hooks.context_warnings',
'hooks.workflow_guard',
'workflow.ui_review',
'workflow.max_discuss_passes',
'features.thinking_partner',
'context',
'features.global_learnings',

View File

@@ -24,6 +24,8 @@ const CONFIG_KEY_SUGGESTIONS = {
'workflow.code_review_level': 'workflow.code_review_depth',
'workflow.review_depth': 'workflow.code_review_depth',
'review.model': 'review.models.<cli-name>',
'sub_repos': 'planning.sub_repos',
'plan_checker': 'workflow.plan_check',
};
function validateKnownConfigKeyPath(keyPath) {

View File

@@ -63,7 +63,8 @@ const VALID_CONFIG_KEYS = new Set([
'workflow.research_before_questions',
'workflow.discuss_mode',
'workflow.skip_discuss',
'workflow._auto_chain_active',
'workflow.ui_review',
'workflow.max_discuss_passes',
'workflow.use_worktrees',
'workflow.code_review',
'workflow.code_review_depth',
@@ -72,6 +73,7 @@ const VALID_CONFIG_KEYS = new Set([
'planning.commit_docs', 'planning.search_gitignored',
'workflow.subagent_timeout',
'hooks.context_warnings',
'hooks.workflow_guard',
'features.thinking_partner',
'features.global_learnings',
'learnings.max_inject',

View File

@@ -0,0 +1,77 @@
'use strict';
/**
* Regression tests for config key bugs:
* #2530 — workflow._auto_chain_active is internal state, must not be in VALID_CONFIG_KEYS
* #2531 — hooks.workflow_guard is used by hook and documented but missing from VALID_CONFIG_KEYS
* #2532 — workflow.ui_review is used in autonomous.md but missing from VALID_CONFIG_KEYS
* #2533 — workflow.max_discuss_passes is used in discuss-phase.md but missing from VALID_CONFIG_KEYS
* #2535 — sub_repos and plan_checker legacy keys need CONFIG_KEY_SUGGESTIONS migration hints
*/
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const path = require('node:path');
const { createTempProject, cleanup, runGsdTools } = require('./helpers.cjs');
const { VALID_CONFIG_KEYS } = require('../get-shit-done/bin/lib/config-schema.cjs');
describe('VALID_CONFIG_KEYS correctness', () => {
test('#2530: workflow._auto_chain_active must not be in VALID_CONFIG_KEYS (internal state)', () => {
assert.ok(
!VALID_CONFIG_KEYS.has('workflow._auto_chain_active'),
'workflow._auto_chain_active is internal runtime state and must not be user-settable'
);
});
test('#2531: hooks.workflow_guard must be in VALID_CONFIG_KEYS (used by hook, documented)', () => {
assert.ok(
VALID_CONFIG_KEYS.has('hooks.workflow_guard'),
'hooks.workflow_guard is read by gsd-workflow-guard.js hook and documented in CONFIGURATION.md'
);
});
test('#2532: workflow.ui_review must be in VALID_CONFIG_KEYS (used in autonomous.md)', () => {
assert.ok(
VALID_CONFIG_KEYS.has('workflow.ui_review'),
'workflow.ui_review is read in autonomous.md via gsd-sdk query config-get'
);
});
test('#2533: workflow.max_discuss_passes must be in VALID_CONFIG_KEYS (used in discuss-phase.md)', () => {
assert.ok(
VALID_CONFIG_KEYS.has('workflow.max_discuss_passes'),
'workflow.max_discuss_passes is read in discuss-phase.md via gsd-sdk query config-get'
);
});
});
describe('CONFIG_KEY_SUGGESTIONS migration hints (#2535)', () => {
let tmpDir;
test('config-set sub_repos emits "Did you mean planning.sub_repos?" suggestion', (t) => {
tmpDir = createTempProject();
t.after(() => cleanup(tmpDir));
const result = runGsdTools(['config-set', 'sub_repos', '[]'], tmpDir);
assert.ok(!result.success, 'config-set sub_repos should fail');
const combined = result.error + result.output;
assert.ok(
combined.includes('Did you mean') && combined.includes('planning.sub_repos'),
`Expected "Did you mean planning.sub_repos?" in error, got:\nstdout: ${result.output}\nstderr: ${result.error}`
);
});
test('config-set plan_checker emits "Did you mean workflow.plan_check?" suggestion', (t) => {
tmpDir = createTempProject();
t.after(() => cleanup(tmpDir));
const result = runGsdTools(['config-set', 'plan_checker', 'true'], tmpDir);
assert.ok(!result.success, 'config-set plan_checker should fail');
const combined = result.error + result.output;
assert.ok(
combined.includes('Did you mean') && combined.includes('workflow.plan_check'),
`Expected "Did you mean workflow.plan_check?" in error, got:\nstdout: ${result.output}\nstderr: ${result.error}`
);
});
});

View File

@@ -19,8 +19,8 @@ const ROOT = path.resolve(__dirname, '..');
const { VALID_CONFIG_KEYS } = require('../get-shit-done/bin/lib/config-schema.cjs');
const CONFIGURATION_MD = fs.readFileSync(path.join(ROOT, 'docs', 'CONFIGURATION.md'), 'utf8');
// Keys starting with _ are internal runtime state, not user-facing config.
const INTERNAL_KEYS = new Set(['workflow._auto_chain_active']);
// Reserved for future internal keys; workflow._auto_chain_active removed from VALID_CONFIG_KEYS (#2530).
const INTERNAL_KEYS = new Set();
test('every key in VALID_CONFIG_KEYS is documented in docs/CONFIGURATION.md', () => {
const undocumented = [];