feat: add review.default_reviewers config for /gsd-review defaults (#3464)

* feat(review): add review.default_reviewers selection policy

* docs(review): explain default reviewer config and precedence

* chore(changeset): add feature entry for review.default_reviewers

* chore(changeset): set pr field for #3464

* test(review): cover unavailable default-reviewer failure path

* fix(review): sync sdk and inventory parity for default reviewers

* fix(review): use canonical /gsd:review namespace in source
This commit is contained in:
Tom Boucher
2026-05-13 14:10:15 -04:00
committed by GitHub
parent c5d4cf35e6
commit 245d5f66a1
14 changed files with 417 additions and 5 deletions

View File

@@ -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']);
});
});

View File

@@ -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)}`
);
});
});

View File

@@ -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'
);
});
});