diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 1a041d64..aacdbca5 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -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 | diff --git a/get-shit-done/bin/lib/config-schema.cjs b/get-shit-done/bin/lib/config-schema.cjs index 503b4c3f..b7c1e7a2 100644 --- a/get-shit-done/bin/lib/config-schema.cjs +++ b/get-shit-done/bin/lib/config-schema.cjs @@ -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', diff --git a/get-shit-done/bin/lib/config.cjs b/get-shit-done/bin/lib/config.cjs index 4f507975..2d7cf561 100644 --- a/get-shit-done/bin/lib/config.cjs +++ b/get-shit-done/bin/lib/config.cjs @@ -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.', + 'sub_repos': 'planning.sub_repos', + 'plan_checker': 'workflow.plan_check', }; function validateKnownConfigKeyPath(keyPath) { diff --git a/sdk/src/query/config-mutation.ts b/sdk/src/query/config-mutation.ts index 1a7df431..de54e5fc 100644 --- a/sdk/src/query/config-mutation.ts +++ b/sdk/src/query/config-mutation.ts @@ -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', diff --git a/tests/bug-2530-valid-config-keys.test.cjs b/tests/bug-2530-valid-config-keys.test.cjs new file mode 100644 index 00000000..78e5c3ad --- /dev/null +++ b/tests/bug-2530-valid-config-keys.test.cjs @@ -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}` + ); + }); +}); diff --git a/tests/config-schema-docs-parity.test.cjs b/tests/config-schema-docs-parity.test.cjs index a2e91a5c..7d35fa07 100644 --- a/tests/config-schema-docs-parity.test.cjs +++ b/tests/config-schema-docs-parity.test.cjs @@ -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 = [];