From 86c5863afbb50d08eb64b6407f366b5c6dcb1595 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 22 Apr 2026 20:49:52 -0400 Subject: [PATCH] feat: add settings layers to /gsd-settings (Group A toggles) (closes #2527) (#2602) * feat(#2527): add settings layers to /gsd:settings (Group A toggles) Expand /gsd:settings from 14 to 22 settings, grouped into six visual sections: Planning, Execution, Docs & Output, Features, Model & Pipeline, Misc. Adds 8 new toggles: workflow.pattern_mapper, workflow.tdd_mode, workflow.code_review, workflow.code_review_depth (conditional on code_review=on), workflow.ui_review, commit_docs, intel.enabled, graphify.enabled All 8 keys already existed in VALID_CONFIG_KEYS and docs/CONFIGURATION.md; this wires them into the interactive flow, update_config write step, ~/.gsd/defaults.json persistence, and confirmation table. Closes #2527 * test(#2527): tighten leaf-collision and rename mismatched negative test Addresses CodeRabbit findings on PR #2602: - comment 3127100796: leaf-only matching collapsed `intel.enabled` and `graphify.enabled` to a single `enabled` token, so one occurrence could satisfy both assertions. Replace with hasPathLike(), which requires each dotted segment to appear in order within a bounded window. Applied to both update_config and save_as_defaults blocks. - comment 3127100798: the negative-test description claimed to verify invalid `code_review_depth` value rejection but actually exercised an unknown key path. Split into two suites with accurate names: one asserts settings.md constrains the depth options, the other asserts config-set rejects an unknown key path. * docs(#2527): clarify resolved config path for /gsd-settings Addresses CodeRabbit comment 3127100790 on PR #2602: the original line implied a single `.planning/config.json` target, but settings updates route to `.planning/workstreams//config.json` when a workstream is active. Document both resolved paths so the merge target is unambiguous. --- docs/COMMANDS.md | 11 +- get-shit-done/workflows/settings.md | 145 +++++++++++++++- tests/feat-2527-settings-layers.test.cjs | 207 +++++++++++++++++++++++ 3 files changed, 359 insertions(+), 4 deletions(-) create mode 100644 tests/feat-2527-settings-layers.test.cjs diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 5e107933c..6c7a9dbe3 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -1037,7 +1037,16 @@ Manage parallel workstreams for concurrent work on different milestone areas. ### `/gsd-settings` -Interactive configuration of workflow toggles and model profile. +Interactive configuration of workflow toggles and model profile. Questions are grouped into six visual sections: + +- **Planning** — Research, Plan Checker, Pattern Mapper, Nyquist, UI Phase, UI Gate, AI Phase +- **Execution** — Verifier, TDD Mode, Code Review, Code Review Depth _(conditional — only when Code Review is on)_, UI Review +- **Docs & Output** — Commit Docs, Skip Discuss, Worktrees +- **Features** — Intel, Graphify +- **Model & Pipeline** — Model Profile, Auto-Advance, Branching +- **Misc** — Context Warnings, Research Qs + +All answers are merged via `gsd-sdk query config-set` into the resolved project config path (`.planning/config.json` for a standard install, or `.planning/workstreams//config.json` when a workstream is active), preserving unrelated keys. After confirmation, the user may save the full settings object to `~/.gsd/defaults.json` so future `/gsd-new-project` runs start from the same baseline. ```bash /gsd-settings # Interactive config diff --git a/get-shit-done/workflows/settings.md b/get-shit-done/workflows/settings.md index 5c4016e28..23073c8e6 100644 --- a/get-shit-done/workflows/settings.md +++ b/get-shit-done/workflows/settings.md @@ -40,9 +40,17 @@ Parse current values (default to `true` if not present): - `workflow.plan_check` — spawn plan checker during plan-phase - `workflow.verifier` — spawn verifier during execute-phase - `workflow.nyquist_validation` — validation architecture research during plan-phase (default: true if absent) +- `workflow.pattern_mapper` — run gsd-pattern-mapper between research and planning (default: true if absent) - `workflow.ui_phase` — generate UI-SPEC.md design contracts for frontend phases (default: true if absent) - `workflow.ui_safety_gate` — prompt to run /gsd:ui-phase before planning frontend phases (default: true if absent) - `workflow.ai_integration_phase` — framework selection + eval strategy for AI phases (default: true if absent) +- `workflow.tdd_mode` — enforce RED/GREEN/REFACTOR gate sequence during execute-phase (default: false if absent) +- `workflow.code_review` — enable /gsd:code-review and /gsd:code-review-fix commands (default: true if absent) +- `workflow.code_review_depth` — default depth for /gsd:code-review: `quick`, `standard`, or `deep` (default: `"standard"` if absent; only relevant when `code_review` is on) +- `workflow.ui_review` — run visual quality audit (/gsd:ui-review) in autonomous mode (default: true if absent) +- `commit_docs` — whether `.planning/` files are committed to git (default: true if absent) +- `intel.enabled` — enable queryable codebase intelligence (/gsd:intel) (default: false if absent) +- `graphify.enabled` — enable project knowledge graph (/gsd:graphify) (default: false if absent) - `model_profile` — which model each agent uses (default: `balanced`) - `git.branching_strategy` — branching approach (default: `"none"`) - `workflow.use_worktrees` — whether parallel executor agents run in worktree isolation (default: `true`) @@ -62,7 +70,29 @@ Choose "Inherit" to use the session model for all agents, or configure model_ove manually in .planning/config.json to target specific models for this runtime. ``` -Use AskUserQuestion with current values pre-selected: +Use AskUserQuestion with current values pre-selected. Questions are grouped into six visual sections; the first question in each section carries the section-denoting `header` field (AskUserQuestion renders abbreviated section tags for grouping, max 12 chars). + +Section layout: + +### Planning +Research, Plan Checker, Pattern Mapper, Nyquist, UI Phase, UI Gate, AI Phase + +### Execution +Verifier, TDD Mode, Code Review, Code Review Depth _(conditional — only when code_review=on)_, UI Review + +### Docs & Output +Commit Docs, Skip Discuss, Worktrees + +### Features +Intel, Graphify + +### Model & Pipeline +Model Profile, Auto-Advance, Branching + +### Misc +Context Warnings, Research Qs + +**Conditional visibility — code_review_depth:** This question is shown only when the user's chosen `code_review` value (after they answer that question, or the pre-selected value if unchanged) is on. If `code_review` is off, omit the `code_review_depth` question from the AskUserQuestion block and preserve the existing `workflow.code_review_depth` value in config (do not overwrite). Implementation: ask the Model + Planning + Execution-up-to-Code-Review questions first; if `code_review=on`, include `code_review_depth` in the same batch; otherwise skip it. Conceptually this is a one-branch split on the `code_review` answer. ``` AskUserQuestion([ @@ -104,6 +134,46 @@ AskUserQuestion([ { label: "No", description: "Skip post-execution verification" } ] }, + { + question: "Enable TDD Mode? (RED/GREEN/REFACTOR gates for eligible tasks)", + header: "TDD", + multiSelect: false, + options: [ + { label: "No (Recommended)", description: "Execute tasks normally. Tests written alongside implementation." }, + { label: "Yes", description: "Planner applies type:tdd to business logic/APIs/validations; executor enforces gate sequence. End-of-phase review checks compliance." } + ] + }, + { + question: "Enable Code Review? (/gsd:code-review and /gsd:code-review-fix commands)", + header: "Code Review", + multiSelect: false, + options: [ + { label: "Yes (Recommended)", description: "Enable /gsd:code-review commands for reviewing source files changed during a phase." }, + { label: "No", description: "Commands exit with a configuration gate message. Use when code review is handled externally." } + ] + }, + // Conditional: include the following code_review_depth question ONLY when the user's + // chosen code_review value is "Yes". If code_review is "No", omit this question from + // the AskUserQuestion call and do not touch the existing workflow.code_review_depth value. + { + question: "Code Review Depth? (default depth for /gsd:code-review — override per-run with --depth=)", + header: "Review Depth", + multiSelect: false, + options: [ + { label: "Standard (Recommended)", description: "Per-file analysis. Balanced cost and signal." }, + { label: "Quick", description: "Pattern-matching only. Fastest, lowest cost." }, + { label: "Deep", description: "Cross-file analysis with import graphs. Highest cost, highest signal." } + ] + }, + { + question: "Enable UI Review? (visual quality audit via /gsd:ui-review in autonomous mode)", + header: "UI Review", + multiSelect: false, + options: [ + { label: "Yes (Recommended)", description: "Run visual quality audit after phase execution in autonomous mode." }, + { label: "No", description: "Skip the UI audit step. Good for backend-only projects." } + ] + }, { question: "Auto-advance pipeline? (discuss → plan → execute automatically)", header: "Auto", @@ -113,6 +183,15 @@ AskUserQuestion([ { label: "Yes", description: "Chain stages via Task() subagents (same isolation)" } ] }, + { + question: "Run Pattern Mapper? (maps new files to existing codebase analogs between research and planning)", + header: "Pattern Mapper", + multiSelect: false, + options: [ + { label: "Yes (Recommended)", description: "gsd-pattern-mapper runs between research and plan steps. Surfaces conventions so new code follows house style." }, + { label: "No", description: "Skip pattern mapping. Faster; lose consistency hinting for new files." } + ] + }, { question: "Enable Nyquist Validation? (researches test coverage during planning)", header: "Nyquist", @@ -147,7 +226,7 @@ AskUserQuestion([ header: "AI Phase", multiSelect: false, options: [ - { label: "Yes (Recommended)", description: "Run /gsd-ai-phase before planning AI system phases. Surfaces the right framework, researches its docs, and designs the evaluation strategy." }, + { label: "Yes (Recommended)", description: "Run /gsd:ai-integration-phase before planning AI system phases. Surfaces the right framework, researches its docs, and designs the evaluation strategy." }, { label: "No", description: "Skip AI design contract. Good for non-AI phases or when framework is already decided." } ] }, @@ -179,6 +258,15 @@ AskUserQuestion([ { label: "Yes", description: "Search web for best practices before each question group. More informed questions but uses more tokens." } ] }, + { + question: "Commit .planning/ files to git? (controls whether plans/artifacts are tracked in your repo)", + header: "Commit Docs", + multiSelect: false, + options: [ + { label: "Yes (Recommended)", description: "Commit .planning/ to git. Plans, research, and phase artifacts travel with the repo." }, + { label: "No", description: "Do not commit .planning/. Keep planning local only. Automatic when .planning/ is in .gitignore." } + ] + }, { question: "Skip discuss-phase in autonomous mode? (use ROADMAP phase goals as spec)", header: "Skip Discuss", @@ -196,6 +284,24 @@ AskUserQuestion([ { label: "Yes (Recommended)", description: "Each parallel executor runs in its own worktree branch — no conflicts between agents." }, { label: "No", description: "Disable worktree isolation. Agents run sequentially on the main working tree. Use if EnterWorktree creates branches from wrong base (known cross-platform issue)." } ] + }, + { + question: "Enable Intel? (queryable codebase intelligence via /gsd:intel — builds a JSON index in .planning/intel/)", + header: "Intel", + multiSelect: false, + options: [ + { label: "No (Recommended)", description: "Skip intel indexing. Use when codebase is small or intel queries are not needed." }, + { label: "Yes", description: "Enable /gsd:intel commands. Builds and queries a JSON index of the codebase." } + ] + }, + { + question: "Enable Graphify? (project knowledge graph via /gsd:graphify — builds a graph in .planning/graphs/)", + header: "Graphify", + multiSelect: false, + options: [ + { label: "No (Recommended)", description: "Skip knowledge graph. Use when dependency graphs are not needed." }, + { label: "Yes", description: "Enable /gsd:graphify commands. Builds and queries a project knowledge graph." } + ] } ]) ``` @@ -208,21 +314,33 @@ Merge new settings into existing config.json: { ...existing_config, "model_profile": "quality" | "balanced" | "budget" | "adaptive" | "inherit", + "commit_docs": true/false, "workflow": { "research": true/false, "plan_check": true/false, "verifier": true/false, "auto_advance": true/false, "nyquist_validation": true/false, + "pattern_mapper": true/false, "ui_phase": true/false, "ui_safety_gate": true/false, "ai_integration_phase": true/false, + "tdd_mode": true/false, + "code_review": true/false, + "code_review_depth": "quick" | "standard" | "deep", + "ui_review": true/false, "text_mode": true/false, "research_before_questions": true/false, "discuss_mode": "discuss" | "assumptions", "skip_discuss": true/false, "use_worktrees": true/false }, + "intel": { + "enabled": true/false + }, + "graphify": { + "enabled": true/false + }, "git": { "branching_strategy": "none" | "phase" | "milestone", "quick_branch_template": @@ -234,6 +352,8 @@ Merge new settings into existing config.json: } ``` +**Safe merge:** Apply each chosen value via `gsd-sdk query config-set ` so unrelated keys are never clobbered. `code_review_depth` is written only if the code_review question was answered `on`; otherwise leave the existing value in place. + Write updated config to `$GSD_CONFIG_PATH` (the workstream-aware path resolved in `ensure_and_load_config`). Never hardcode `.planning/config.json` — workstream installs route to `.planning/workstreams//config.json`. @@ -276,10 +396,21 @@ Write `~/.gsd/defaults.json` with: "verifier": , "auto_advance": , "nyquist_validation": , + "pattern_mapper": , "ui_phase": , "ui_safety_gate": , "ai_integration_phase": , + "tdd_mode": , + "code_review": , + "code_review_depth": , + "ui_review": , "skip_discuss": + }, + "intel": { + "enabled": + }, + "graphify": { + "enabled": } } ``` @@ -298,7 +429,15 @@ Display: | Model Profile | {quality/balanced/budget/inherit} | | Plan Researcher | {On/Off} | | Plan Checker | {On/Off} | +| Pattern Mapper | {On/Off} | | Execution Verifier | {On/Off} | +| TDD Mode | {On/Off} | +| Code Review | {On/Off} | +| Code Review Depth | {quick/standard/deep} | +| UI Review | {On/Off} | +| Commit Docs | {On/Off} | +| Intel | {On/Off} | +| Graphify | {On/Off} | | Auto-Advance | {On/Off} | | Nyquist Validation | {On/Off} | | UI Phase | {On/Off} | @@ -323,7 +462,7 @@ Quick commands: - [ ] Current config read -- [ ] User presented with 14 settings (profile + 11 workflow toggles + git branching + ctx warnings) +- [ ] User presented with 22 settings (profile + workflow toggles + features + git branching + ctx warnings), grouped into six sections: Planning, Execution, Docs & Output, Features, Model & Pipeline, Misc. `code_review_depth` is conditional on `code_review=on`. - [ ] Config updated with model_profile, workflow, and git sections - [ ] User offered to save as global defaults (~/.gsd/defaults.json) - [ ] Changes confirmed to user diff --git a/tests/feat-2527-settings-layers.test.cjs b/tests/feat-2527-settings-layers.test.cjs new file mode 100644 index 000000000..420f0b7bf --- /dev/null +++ b/tests/feat-2527-settings-layers.test.cjs @@ -0,0 +1,207 @@ +'use strict'; + +/** + * Feature test for #2527 — /gsd-settings expands to 22 settings grouped into + * six visual sections. Adds 8 new fields (pattern_mapper, tdd_mode, code_review, + * code_review_depth, ui_review, commit_docs, intel.enabled, graphify.enabled) + * and verifies each is present in the AskUserQuestion block, the update_config + * step, the confirmation table, the ~/.gsd/defaults.json save step, and + * VALID_CONFIG_KEYS. + * + * Closes: #2527 + */ + +const { describe, test, before } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); + +const SETTINGS_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'settings.md'); +const { VALID_CONFIG_KEYS } = require('../get-shit-done/bin/lib/config-schema.cjs'); + +const NEW_FIELDS = [ + 'workflow.pattern_mapper', + 'workflow.tdd_mode', + 'workflow.code_review', + 'workflow.code_review_depth', + 'workflow.ui_review', + 'commit_docs', + 'intel.enabled', + 'graphify.enabled', +]; + +const SECTION_HEADERS = ['Planning', 'Execution', 'Docs & Output', 'Features', 'Model & Pipeline', 'Misc']; + +/** + * Match a dotted config-key path inside a block of text. Falls back to a + * simple substring check for single-segment keys; for nested keys, requires + * each segment to appear in order within a bounded window so distinct fields + * (e.g., intel.enabled vs graphify.enabled) cannot collapse to the same leaf. + */ +function hasPathLike(block, field) { + const parts = field.split('.'); + if (parts.length === 1) return block.includes(parts[0]); + const escaped = parts.map((p) => p.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')); + const pattern = new RegExp(escaped.join('[\\s\\S]{0,600}'), 'i'); + return pattern.test(block); +} + +describe('#2527: settings.md adds grouped settings layers', () => { + let content; + + before(() => { + content = fs.readFileSync(SETTINGS_PATH, 'utf-8'); + }); + + describe('Acceptance: all 8 new fields present in AskUserQuestion block', () => { + for (const field of NEW_FIELDS) { + test(`settings.md mentions ${field}`, () => { + assert.ok( + content.includes(field), + `settings.md must reference the config key "${field}" in its AskUserQuestion/update_config step` + ); + }); + } + }); + + describe('Acceptance: section headers applied', () => { + for (const section of SECTION_HEADERS) { + test(`settings.md declares a "${section}" section header`, () => { + // The convention for grouping AskUserQuestion items is a markdown section heading + // of the form "###
" inside the present_settings step. + const heading = new RegExp(`^#{2,4}\\s+${section.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\$&')}\\b`, 'm'); + assert.ok( + heading.test(content), + `settings.md must declare a "${section}" section header to group questions` + ); + }); + } + }); + + describe('Acceptance: update_config step includes all new fields', () => { + test('update_config step references every new field', () => { + const updateMatch = content.match(/[\s\S]*?<\/step>/); + assert.ok(updateMatch, 'settings.md must have an update_config step'); + const updateBlock = updateMatch[0]; + for (const field of NEW_FIELDS) { + // Keys may appear as nested JSON (e.g., "pattern_mapper" under workflow). + // Use hasPathLike so distinct dotted keys (e.g., intel.enabled, + // graphify.enabled) cannot share a single "enabled" occurrence. + assert.ok( + hasPathLike(updateBlock, field), + `update_config step must write "${field}"` + ); + } + }); + }); + + describe('Acceptance: save_as_defaults step includes all new fields', () => { + test('save_as_defaults step references every new field', () => { + const defaultsMatch = content.match(/[\s\S]*?<\/step>/); + assert.ok(defaultsMatch, 'settings.md must have a save_as_defaults step'); + const block = defaultsMatch[0]; + for (const field of NEW_FIELDS) { + assert.ok( + hasPathLike(block, field), + `save_as_defaults step must persist "${field}" into ~/.gsd/defaults.json` + ); + } + }); + }); + + describe('Acceptance: confirmation display includes all new fields', () => { + test('confirm step table lists every new setting by name', () => { + const confirmMatch = content.match(/[\s\S]*?<\/step>/); + assert.ok(confirmMatch, 'settings.md must have a confirm step'); + const block = confirmMatch[0]; + const expectedLabels = [ + 'Pattern Mapper', + 'TDD Mode', + 'Code Review', + 'Code Review Depth', + 'UI Review', + 'Commit Docs', + 'Intel', + 'Graphify', + ]; + for (const label of expectedLabels) { + assert.ok( + block.includes(label), + `confirm step table must display "${label}"` + ); + } + }); + }); + + describe('Acceptance: all 8 new fields registered in VALID_CONFIG_KEYS', () => { + for (const field of NEW_FIELDS) { + test(`VALID_CONFIG_KEYS contains ${field}`, () => { + assert.ok( + VALID_CONFIG_KEYS.has(field), + `${field} must be in VALID_CONFIG_KEYS so config-set accepts it` + ); + }); + } + }); + + describe('Acceptance: code_review_depth is conditional on code_review=on', () => { + test('settings.md documents conditional visibility for code_review_depth', () => { + // Must explicitly note that code_review_depth only appears when code_review is on. + const conditionalRegex = /code_review_depth[\s\S]{0,400}(only|conditional|when|if)[\s\S]{0,80}code_review/i; + assert.ok( + conditionalRegex.test(content) || + /code_review\s*=\s*on[\s\S]{0,400}code_[…]*depth/i.test(content), + 'settings.md must document that code_review_depth is only shown when code_review is on' + ); + }); + }); + + describe('Negative: settings.md constrains code_review_depth options', () => { + test('settings.md restricts code_review_depth to a known option set', () => { + // Depth accepts string values (quick|standard|deep). config-set does not + // block arbitrary strings at the value level today; instead settings.md + // constrains the AskUserQuestion options to the valid set so users + // cannot pick "bogus" via the interactive flow. + const depthOptionsRegex = + /code_review_depth[\s\S]{0,800}(quick|standard|deep|surface)/i; + assert.ok( + depthOptionsRegex.test(content), + 'settings.md must constrain code_review_depth options to a known set' + ); + }); + }); + + describe('Negative: config-set rejects an unknown key path', () => { + test('config-set workflow.code_review_bogus_key fails', (t) => { + const tmpDir = createTempProject(); + t.after(() => cleanup(tmpDir)); + + const bad = runGsdTools(['config-set', 'workflow.code_review_bogus_key', 'x'], tmpDir); + assert.ok(!bad.success, 'config-set on an unknown key must fail'); + }); + }); + + describe('Acceptance: all 6 section headers are used as header: field on first question in each section', () => { + test('the header field appears for each section in the AskUserQuestion block', () => { + // Map user-visible section names to the short `header:` strings used in AskUserQuestion. + // settings.md uses abbreviated headers (max 12 chars). Verify at least one header + // per section-intent appears on a question. + const requiredHeaders = [ + /header:\s*"Model"/, // Model & Pipeline opener + /header:\s*"Research"/, // Planning opener (first Planning-section question) + /header:\s*"Pattern Mapper"|header:\s*"Patterns"/, // new Planning addition + /header:\s*"Verifier"/, // Execution existing + /header:\s*"TDD"/, // new Execution + /header:\s*"Code Review"/, // new Execution + /header:\s*"UI Review"/, // new Execution + /header:\s*"Commit Docs"/, // new Docs & Output + /header:\s*"Intel"/, // new Features + /header:\s*"Graphify"/, // new Features + ]; + for (const re of requiredHeaders) { + assert.ok(re.test(content), `settings.md must include an AskUserQuestion header matching ${re}`); + } + }); + }); +});