diff --git a/.changeset/daring-badgers-munch.md b/.changeset/daring-badgers-munch.md new file mode 100644 index 000000000..47c3505aa --- /dev/null +++ b/.changeset/daring-badgers-munch.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 3464 +--- +Add review.default_reviewers config support so no-flag /gsd-review runs a configured default subset with explicit-flags and --all precedence preserved. diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 222e42f64..d81ade661 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -1177,13 +1177,27 @@ Cross-AI peer review of phase plans from external AI CLIs. | `--opencode` | Include OpenCode review (via GitHub Copilot) | | `--qwen` | Include Qwen Code review (Alibaba Qwen models) | | `--cursor` | Include Cursor agent review | -| `--all` | Include all available CLIs | +| `--ollama` | Include Ollama server review | +| `--lm-studio` | Include LM Studio server review | +| `--llama-cpp` | Include llama.cpp server review | +| `--all` | Include all available reviewers (CLI + local model servers) | + +**Default reviewer behavior (no flags):** +- If `review.default_reviewers` is **unset**, `/gsd-review` runs all detected reviewers (current default behavior). +- If `review.default_reviewers` is **set**, `/gsd-review` runs only that subset (for example `["gemini","codex"]`). +- `--all` always overrides config and runs the full detected set. +- Explicit flags (for example `--cursor`) override both `--all` and config defaults for that run. **Produces:** `{phase}-REVIEWS.md` — consumable by `/gsd-plan-phase --reviews` ```bash +# set project default reviewers for no-flag /gsd-review runs +gsd config-set review.default_reviewers '["gemini","codex"]' + +/gsd-review --phase 2 # runs gemini+codex from config /gsd-review --phase 3 --all /gsd-review --phase 2 --gemini +/gsd-review --phase 2 --cursor # one-off override ``` --- diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 4b12218cc..a9354c53a 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -66,6 +66,10 @@ GSD stores project settings in `.planning/config.json`. Created during `/gsd-new "context_warnings": true, "workflow_guard": false }, + "review": { + "default_reviewers": null, + "models": {} + }, "parallelization": { "enabled": true, "plan_level": true, @@ -173,6 +177,24 @@ API key fields accept a string value (the key itself). They can also be set to t The `` slug is validated against `[a-zA-Z0-9_-]+`. Empty or path-containing slugs are rejected by `config-set`. +### Reviewer defaults for `/gsd-review` + +Use `review.default_reviewers` to scope the no-flag `/gsd-review` run to a subset of detected reviewers. + +| Setting | Type | Default | Description | +|---------|------|---------|-------------| +| `review.default_reviewers` | string[] \| null | `null` (all detected reviewers) | Optional default subset for no-flag `/gsd-review`, e.g. `["gemini","codex"]`. Precedence is: explicit reviewer flags > `--all` > `review.default_reviewers` > all detected. Unknown slugs are ignored with a warning; known-but-undetected slugs are ignored with an info note; empty arrays are rejected by `config-set`. | + +Example: + +```json +{ + "review": { + "default_reviewers": ["gemini", "codex"] + } +} +``` + ### Agent-skill injection (dynamic) `agent_skills.` extends the `agent_skills` map documented below. Slug is validated against `[a-zA-Z0-9_-]+` — no path separators, no whitespace, no shell metacharacters. Configured interactively via `/gsd-config --integrations`. @@ -650,6 +672,7 @@ Configure per-CLI model selection for `/gsd-review`. When set, overrides the CLI | `review.models.ollama` | string | (server default) | Model name passed to Ollama when `--ollama` reviewer is invoked. If unset, the first available model reported by the server is used (e.g. `llama3`). Set to a specific tag: `gsd config-set review.models.ollama codellama` | | `review.models.lm_studio` | string | (server default) | Model name passed to LM Studio when `--lm-studio` reviewer is invoked. If unset, the first available model reported by the server is used. | | `review.models.llama_cpp` | string | (server default) | Model name passed to llama.cpp when `--llama-cpp` reviewer is invoked. If unset, the first model reported by `/v1/models` is used. | +| `review.default_reviewers` | string[] \| null | (all detected reviewers) | Default reviewer subset for no-flag `/gsd-review`. Example: `["gemini","codex"]`. Explicit flags and `--all` override this setting. | | `review.ollama_host` | string | `http://localhost:11434` | Base URL of the Ollama server. Override when running Ollama on a non-default port or remote host: `gsd config-set review.ollama_host http://192.168.1.10:11434` | | `review.lm_studio_host` | string | `http://localhost:1234` | Base URL of the LM Studio local server. Override when using a non-default port. | | `review.llama_cpp_host` | string | `http://localhost:8080` | Base URL of the llama.cpp server (`llama-server`). Override when using a non-default port. | diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 319f84049..9fa79a2e4 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -1177,7 +1177,7 @@ When verification returns `human_needed`, items are persisted as a trackable HUM ### 42. Cross-AI Peer Review -**Command:** `/gsd-review --phase N [--gemini] [--claude] [--codex] [--coderabbit] [--opencode] [--qwen] [--cursor] [--all]` +**Command:** `/gsd-review --phase N [--gemini] [--claude] [--codex] [--coderabbit] [--opencode] [--qwen] [--cursor] [--ollama] [--lm-studio] [--llama-cpp] [--all]` **Purpose:** Invoke external AI CLIs (Gemini, Claude, Codex, CodeRabbit, OpenCode, Qwen Code, Cursor) to independently review phase plans. Produces structured REVIEWS.md with per-reviewer feedback. @@ -1187,9 +1187,15 @@ When verification returns `human_needed`, items are persisted as a trackable HUM - REQ-REVIEW-03: System MUST invoke each selected CLI independently - REQ-REVIEW-04: System MUST collect responses and produce `REVIEWS.md` - REQ-REVIEW-05: Reviews MUST be consumable by `/gsd-plan-phase --reviews` +- REQ-REVIEW-06: System MUST support project-level no-flag defaults via `review.default_reviewers` +- REQ-REVIEW-07: Reviewer precedence MUST be explicit flags > `--all` > `review.default_reviewers` > all detected reviewers **Produces:** `{phase}-REVIEWS.md` — Per-reviewer structured feedback +**User configuration note:** +- Set `review.default_reviewers` in `.planning/config.json` (or via `gsd config-set`) to control no-flag `/gsd-review` fan-out. +- Use `--all` for a full pre-merge sweep without changing project defaults. + --- ### 43. Backlog Parking Lot diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 284e65db6..d887d72de 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -1,5 +1,5 @@ { - "generated": "2026-05-12", + "generated": "2026-05-13", "families": { "agents": [ "gsd-advisor-researcher", @@ -295,6 +295,7 @@ "planning-workspace.cjs", "profile-output.cjs", "profile-pipeline.cjs", + "review-reviewer-selection.cjs", "roadmap-command-router.cjs", "roadmap.cjs", "runtime-homes.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index 4d7f6d006..f7317505a 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -360,7 +360,7 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t --- -## CLI Modules (57 shipped) +## CLI Modules (58 shipped) Full listing: `get-shit-done/bin/lib/*.cjs`. @@ -403,6 +403,7 @@ Full listing: `get-shit-done/bin/lib/*.cjs`. | `planning-workspace.cjs` | Planning path/workstream seam (`planningDir`, `planningPaths`, active-workstream routing, `.planning/.lock` orchestration) | | `profile-output.cjs` | Profile rendering, USER-PROFILE.md and dev-preferences.md generation | | `profile-pipeline.cjs` | User behavioral profiling data pipeline, session file scanning | +| `review-reviewer-selection.cjs` | Reviewer selection/normalization helpers for `/gsd-review` default reviewer policy and precedence | | `roadmap-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools roadmap` | | `roadmap.cjs` | ROADMAP.md parsing, phase extraction, plan progress | | `runtime-homes.cjs` | Canonical runtime → global config/skills directory mapping; first-class support for all 15 runtimes including Hermes nested layout and Cline rules-based exclusion (#3126) | diff --git a/get-shit-done/bin/lib/config-schema.cjs b/get-shit-done/bin/lib/config-schema.cjs index b2e56e95c..e4465b785 100644 --- a/get-shit-done/bin/lib/config-schema.cjs +++ b/get-shit-done/bin/lib/config-schema.cjs @@ -47,6 +47,7 @@ const VALID_CONFIG_KEYS = new Set([ 'git.branching_strategy', 'git.base_branch', 'git.phase_branch_template', 'git.milestone_branch_template', 'git.quick_branch_template', 'planning.commit_docs', 'planning.search_gitignored', 'planning.sub_repos', 'review.ollama_host', 'review.lm_studio_host', 'review.llama_cpp_host', + 'review.default_reviewers', 'workflow.cross_ai_execution', 'workflow.cross_ai_command', 'workflow.cross_ai_timeout', 'workflow.subagent_timeout', 'executor.stall_detect_interval_minutes', diff --git a/get-shit-done/bin/lib/config.cjs b/get-shit-done/bin/lib/config.cjs index fa384ffcb..f1c9b0661 100644 --- a/get-shit-done/bin/lib/config.cjs +++ b/get-shit-done/bin/lib/config.cjs @@ -13,6 +13,7 @@ const { } = require('./model-profiles.cjs'); const { VALID_CONFIG_KEYS, isValidConfigKey } = require('./config-schema.cjs'); const { isSecretKey, maskSecret } = require('./secrets.cjs'); +const { normalizeConfiguredDefaultReviewers } = require('./review-reviewer-selection.cjs'); const CONFIG_KEY_SUGGESTIONS = { 'workflow.nyquist_validation_enabled': 'workflow.nyquist_validation', @@ -444,6 +445,14 @@ function cmdConfigSet(cwd, keyPath, value, raw) { error(`Invalid workflow.human_verify_mode '${value}'. Valid values: ${VALID_HUMAN_VERIFY_MODES.join(', ')}`); } + if (keyPath === 'review.default_reviewers') { + const normalized = normalizeConfiguredDefaultReviewers(parsedValue); + if (normalized.errors.length > 0) { + error(normalized.errors[0]); + } + parsedValue = normalized.values; + } + const setConfigValueResult = setConfigValue(cwd, keyPath, parsedValue); // Mask secrets in both JSON and text output. The plaintext is written diff --git a/get-shit-done/bin/lib/review-reviewer-selection.cjs b/get-shit-done/bin/lib/review-reviewer-selection.cjs new file mode 100644 index 000000000..7d8d3225f --- /dev/null +++ b/get-shit-done/bin/lib/review-reviewer-selection.cjs @@ -0,0 +1,125 @@ +'use strict'; + +/** + * Review Reviewer Selection Module + * + * Owns reviewer-selection policy projection for /gsd:review: + * explicit flags > --all > review.default_reviewers > all detected. + */ + +const KNOWN_REVIEWER_SLUGS = [ + 'gemini', + 'claude', + 'codex', + 'coderabbit', + 'opencode', + 'qwen', + 'cursor', + 'ollama', + 'lm_studio', + 'llama_cpp', +]; + +function normalizeConfiguredDefaultReviewers(rawValue) { + if (rawValue === undefined || rawValue === null) { + return { absent: true, values: [], errors: [] }; + } + if (!Array.isArray(rawValue)) { + return { + absent: false, + values: [], + errors: ['review.default_reviewers must be a JSON array of reviewer slugs'], + }; + } + if (rawValue.length === 0) { + return { + absent: false, + values: [], + errors: ['review.default_reviewers cannot be empty'], + }; + } + + const seen = new Set(); + const normalized = []; + const errors = []; + for (const item of rawValue) { + if (typeof item !== 'string') { + errors.push('review.default_reviewers must contain only string slugs'); + continue; + } + if (!/^[a-zA-Z0-9_-]+$/.test(item)) { + errors.push(`invalid reviewer slug in review.default_reviewers: ${item}`); + continue; + } + const slug = item.toLowerCase(); + if (!seen.has(slug)) { + seen.add(slug); + normalized.push(slug); + } + } + + return { absent: false, values: normalized, errors }; +} + +function resolveReviewerSelection(input) { + const detected = new Set((input.detected || []).map((v) => String(v).toLowerCase())); + const explicitFlags = new Set((input.explicitFlags || []).map((v) => String(v).toLowerCase())); + const allFlag = !!input.allFlag; + const normalizedDefaults = normalizeConfiguredDefaultReviewers(input.configuredDefaultReviewers); + + const warnings = []; + const infos = []; + const errors = [...normalizedDefaults.errors]; + + let source = 'no_config_all_detected'; + let selected = []; + + if (explicitFlags.size > 0) { + source = 'explicit_flags'; + selected = [...explicitFlags].filter((slug) => detected.has(slug)); + const missing = [...explicitFlags].filter((slug) => !detected.has(slug)); + if (missing.length > 0) { + infos.push(`explicit reviewers missing on host: ${missing.join(', ')}`); + } + if (selected.length === 0 && errors.length === 0) { + errors.push('no selected reviewers are available for explicit flags'); + } + } else if (allFlag) { + source = 'all_flag'; + selected = [...detected]; + } else if (!normalizedDefaults.absent) { + source = 'config_default'; + const knownDefaults = []; + for (const slug of normalizedDefaults.values) { + if (!KNOWN_REVIEWER_SLUGS.includes(slug)) { + warnings.push(`unknown reviewer slug in review.default_reviewers: ${slug}`); + } else { + knownDefaults.push(slug); + } + } + const undetected = knownDefaults.filter((slug) => !detected.has(slug)); + if (undetected.length > 0) { + infos.push(`configured reviewers not detected on this host: ${undetected.join(', ')}`); + } + selected = knownDefaults.filter((slug) => detected.has(slug)); + if (selected.length === 0 && errors.length === 0) { + errors.push('all configured default reviewers are unavailable on this host'); + } + } else { + selected = [...detected]; + } + + return { + source, + selected: selected.sort(), + warnings, + infos, + errors, + }; +} + +module.exports = { + KNOWN_REVIEWER_SLUGS, + normalizeConfiguredDefaultReviewers, + resolveReviewerSelection, +}; diff --git a/get-shit-done/workflows/review.md b/get-shit-done/workflows/review.md index ca8b30c80..2dd10a6f7 100644 --- a/get-shit-done/workflows/review.md +++ b/get-shit-done/workflows/review.md @@ -49,7 +49,19 @@ Parse flags from `$ARGUMENTS`: - `--lm-studio` → include LM Studio (local server, OpenAI-compatible) - `--llama-cpp` → include llama.cpp (local server, OpenAI-compatible) - `--all` → include all available (CLIs + running local servers) -- No flags → include all available +- No flags → if `review.default_reviewers` is set, include only configured reviewers that are detected; otherwise include all available + +Reviewer-selection precedence: +1. Individual reviewer flags (`--gemini`, `--codex`, etc.) +2. `--all` +3. `review.default_reviewers` +4. No key + no flags → all detected reviewers + +`review.default_reviewers` behavior: +- Value must be a non-empty array of slug strings (configured via `gsd config-set review.default_reviewers '["gemini","codex"]'`) +- Unknown slugs warn and are ignored +- Known-but-undetected slugs emit an info note and are ignored +- If all configured reviewers are unavailable, fail with an actionable message If no CLIs are available: ``` diff --git a/sdk/src/query/config-schema.ts b/sdk/src/query/config-schema.ts index 8cbee17b4..b1b767e5e 100644 --- a/sdk/src/query/config-schema.ts +++ b/sdk/src/query/config-schema.ts @@ -48,6 +48,7 @@ export const VALID_CONFIG_KEYS: ReadonlySet = new Set([ 'ship.pr_body_sections', 'git.branching_strategy', 'git.base_branch', 'git.phase_branch_template', 'git.milestone_branch_template', 'git.quick_branch_template', 'planning.commit_docs', 'planning.search_gitignored', 'planning.sub_repos', + 'review.default_reviewers', 'review.ollama_host', 'review.lm_studio_host', 'review.llama_cpp_host', 'workflow.cross_ai_execution', 'workflow.cross_ai_command', 'workflow.cross_ai_timeout', 'workflow.subagent_timeout', diff --git a/tests/review-default-reviewers-config.test.cjs b/tests/review-default-reviewers-config.test.cjs new file mode 100644 index 000000000..185593500 --- /dev/null +++ b/tests/review-default-reviewers-config.test.cjs @@ -0,0 +1,99 @@ +'use strict'; + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); +const { + VALID_CONFIG_KEYS, +} = require('../get-shit-done/bin/lib/config-schema.cjs'); + +describe('review.default_reviewers config key (#3079)', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + runGsdTools('config-ensure-section', tmpDir, { HOME: tmpDir, USERPROFILE: tmpDir }); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('schema key is registered', () => { + assert.ok( + VALID_CONFIG_KEYS.has('review.default_reviewers'), + 'review.default_reviewers must be in VALID_CONFIG_KEYS' + ); + }); + + test('round-trip set/get supports string array and normalizes to lowercase unique slugs', () => { + const setResult = runGsdTools( + ['config-set', 'review.default_reviewers', '["Gemini","CODEX","codex"]'], + tmpDir, + { HOME: tmpDir, USERPROFILE: tmpDir } + ); + assert.ok(setResult.success, `config-set failed: ${setResult.error}`); + + const getResult = runGsdTools( + ['config-get', 'review.default_reviewers'], + tmpDir, + { HOME: tmpDir, USERPROFILE: tmpDir } + ); + assert.ok(getResult.success, `config-get failed: ${getResult.error}`); + assert.deepStrictEqual(JSON.parse(getResult.output), ['gemini', 'codex']); + }); + + test('empty array is rejected with schema error', () => { + const result = runGsdTools( + ['config-set', 'review.default_reviewers', '[]'], + tmpDir, + { HOME: tmpDir, USERPROFILE: tmpDir } + ); + assert.ok(!result.success, 'config-set should reject empty arrays'); + assert.ok( + result.error.includes('cannot be empty'), + `expected empty-array error, got: ${result.error}` + ); + }); + + test('non-array value is rejected', () => { + const result = runGsdTools( + ['config-set', 'review.default_reviewers', 'gemini'], + tmpDir, + { HOME: tmpDir, USERPROFILE: tmpDir } + ); + assert.ok(!result.success, 'config-set should reject non-array values'); + assert.ok( + result.error.includes('must be a JSON array'), + `expected type error, got: ${result.error}` + ); + }); + + test('invalid slug is rejected', () => { + const result = runGsdTools( + ['config-set', 'review.default_reviewers', '["gemini","bad/slug"]'], + tmpDir, + { HOME: tmpDir, USERPROFILE: tmpDir } + ); + assert.ok(!result.success, 'config-set should reject invalid slugs'); + assert.ok( + result.error.includes('invalid reviewer slug'), + `expected slug error, got: ${result.error}` + ); + }); + + test('value is persisted in nested review object', () => { + const setResult = runGsdTools( + ['config-set', 'review.default_reviewers', '["gemini","codex"]'], + tmpDir, + { HOME: tmpDir, USERPROFILE: tmpDir } + ); + assert.ok(setResult.success, `config-set failed: ${setResult.error}`); + + const cfg = JSON.parse(fs.readFileSync(path.join(tmpDir, '.planning', 'config.json'), 'utf-8')); + assert.deepStrictEqual(cfg.review?.default_reviewers, ['gemini', 'codex']); + }); +}); + diff --git a/tests/review-default-reviewers-resolution.test.cjs b/tests/review-default-reviewers-resolution.test.cjs new file mode 100644 index 000000000..3ddeb3ef0 --- /dev/null +++ b/tests/review-default-reviewers-resolution.test.cjs @@ -0,0 +1,67 @@ +'use strict'; + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); + +const { + resolveReviewerSelection, +} = require('../get-shit-done/bin/lib/review-reviewer-selection.cjs'); + +describe('review default reviewers resolution (#3079)', () => { + test('no flags + config defaults selects configured subset', () => { + const result = resolveReviewerSelection({ + detected: ['gemini', 'codex', 'claude'], + explicitFlags: [], + allFlag: false, + configuredDefaultReviewers: ['gemini', 'codex'], + }); + + assert.strictEqual(result.source, 'config_default'); + assert.deepStrictEqual(result.selected, ['codex', 'gemini']); + assert.deepStrictEqual(result.errors, []); + }); + + test('--all ignores configured defaults', () => { + const result = resolveReviewerSelection({ + detected: ['gemini', 'codex', 'claude'], + explicitFlags: [], + allFlag: true, + configuredDefaultReviewers: ['gemini'], + }); + + assert.strictEqual(result.source, 'all_flag'); + assert.deepStrictEqual(result.selected, ['claude', 'codex', 'gemini']); + }); + + test('explicit flags win over config defaults', () => { + const result = resolveReviewerSelection({ + detected: ['gemini', 'codex', 'claude', 'cursor'], + explicitFlags: ['cursor'], + allFlag: false, + configuredDefaultReviewers: ['gemini', 'codex'], + }); + + assert.strictEqual(result.source, 'explicit_flags'); + assert.deepStrictEqual(result.selected, ['cursor']); + }); + + test('unknown configured slugs warn and all-undetected known slugs error', () => { + const result = resolveReviewerSelection({ + detected: ['gemini'], + explicitFlags: [], + allFlag: false, + configuredDefaultReviewers: ['unknown_slug', 'codex'], + }); + + assert.strictEqual(result.source, 'config_default'); + assert.ok( + result.warnings.some((msg) => msg.includes('unknown reviewer slug') && msg.includes('unknown_slug')), + `expected warning for unknown slug, got: ${JSON.stringify(result.warnings)}` + ); + assert.ok( + result.errors.some((msg) => msg.includes('all configured default reviewers are unavailable')), + `expected unavailable error, got: ${JSON.stringify(result.errors)}` + ); + }); +}); + diff --git a/tests/review-default-reviewers-workflow.test.cjs b/tests/review-default-reviewers-workflow.test.cjs new file mode 100644 index 000000000..0d8e24129 --- /dev/null +++ b/tests/review-default-reviewers-workflow.test.cjs @@ -0,0 +1,48 @@ +'use strict'; + +// allow-test-rule: source-text-is-the-product +// Workflow markdown is runtime contract; these assertions verify deployed behavior text. + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +describe('review workflow default reviewer selection contract (#3079)', () => { + const workflow = fs.readFileSync( + path.join(process.cwd(), 'get-shit-done', 'workflows', 'review.md'), + 'utf8' + ); + + test('documents review.default_reviewers no-flag behavior', () => { + assert.ok( + workflow.includes('review.default_reviewers'), + 'review workflow must reference review.default_reviewers for no-flag selection' + ); + }); + + test('documents precedence order with explicit flags and --all overrides', () => { + assert.ok( + workflow.includes('Individual reviewer flags') && + workflow.includes('--all') && + workflow.includes('review.default_reviewers'), + 'review workflow must document precedence: flags > --all > review.default_reviewers' + ); + }); + + test('documents unknown/undetected configured slug handling', () => { + assert.ok( + workflow.includes('Unknown slugs warn') && + workflow.includes('Known-but-undetected slugs'), + 'review workflow must document unknown and undetected slug handling' + ); + }); + + test('documents failure behavior when all configured reviewers unavailable', () => { + assert.ok( + workflow.includes('all configured reviewers are unavailable') && + workflow.includes('fail'), + 'review workflow must document failure path when configured reviewers are unavailable' + ); + }); +});