fix: prevent discuss-phase from ignoring workflow instructions (#1292)

The discuss-phase command file contained a 93-line detailed <process>
block that competed with the actual workflow file. The agent treated
this summary as complete instructions and never read the execution_context
files (discuss-phase.md, discuss-phase-assumptions.md, context.md template).

Root cause: Unlike execute-phase and plan-phase commands (which have short
2-line process blocks deferring to the workflow file), discuss-phase had
inline step-by-step instructions detailed enough to act on without reading
the referenced workflow files.

Changes:
- Replace discuss-phase command's <process> block with a short directive
  that forces reading the workflow file, matching execute-phase/plan-phase
  pattern
- Add MANDATORY instruction that execution_context files ARE the
  instructions, not optional reading
- Register workflow.research_before_questions and workflow.discuss_mode
  as valid config keys (were missing from VALID_CONFIG_KEYS)
- Fix config key mismatch: workflows referenced "research_questions"
  but documented key is "workflow.research_before_questions"
- Move research_before_questions from hooks section to workflow section
  in settings workflow
- Add research_before_questions default to config template and builder
- Add suggestion mapping for deprecated hooks.research_questions key
- Add 6 regression tests covering config keys and process block guard

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-03-21 14:13:11 -04:00
parent 33db504934
commit 3306a77a79
8 changed files with 107 additions and 51 deletions

View File

@@ -477,6 +477,60 @@ describe('config-new-project command', () => {
});
});
// ─── config-set (research_before_questions and discuss_mode) ──────────────────
describe('config-set research_before_questions and discuss_mode', () => {
let tmpDir;
beforeEach(() => {
tmpDir = createTempProject();
runGsdTools('config-ensure-section', tmpDir);
});
afterEach(() => {
cleanup(tmpDir);
});
test('workflow.research_before_questions is a valid config key', () => {
const result = runGsdTools('config-set workflow.research_before_questions true', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const config = readConfig(tmpDir);
assert.strictEqual(config.workflow.research_before_questions, true);
});
test('workflow.discuss_mode is a valid config key', () => {
const result = runGsdTools('config-set workflow.discuss_mode assumptions', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const config = readConfig(tmpDir);
assert.strictEqual(config.workflow.discuss_mode, 'assumptions');
});
test('research_before_questions defaults to false in new configs', () => {
const config = readConfig(tmpDir);
assert.strictEqual(config.workflow.research_before_questions, false);
});
test('discuss_mode defaults to discuss in new configs', () => {
const config = readConfig(tmpDir);
assert.strictEqual(config.workflow.discuss_mode, 'discuss');
});
test('hooks.research_questions is rejected with suggestion', () => {
const result = runGsdTools('config-set hooks.research_questions true', tmpDir);
assert.strictEqual(result.success, false);
assert.ok(
result.error.includes('Unknown config key'),
`Expected "Unknown config key" in error: ${result.error}`
);
assert.ok(
result.error.includes('workflow.research_before_questions'),
`Expected suggestion for workflow.research_before_questions in error: ${result.error}`
);
});
});
// ─── config-set (additional coverage) ────────────────────────────────────────
describe('config-set unknown key (no suggestion)', () => {

View File

@@ -26,6 +26,41 @@ describe('workflow.discuss_mode config', () => {
assert.ok(command.includes('workflow.discuss_mode'), 'should reference config key');
});
test('discuss-phase command process block defers to workflow file (not inline instructions)', () => {
const command = fs.readFileSync(
path.join(__dirname, '..', 'commands', 'gsd', 'discuss-phase.md'), 'utf8'
);
// Extract the <process> block
const processMatch = command.match(/<process>([\s\S]*?)<\/process>/);
assert.ok(processMatch, 'should have a <process> block');
const processBlock = processMatch[1];
// The process block must explicitly tell the agent to read the workflow file
assert.ok(
processBlock.includes('Read and execute'),
'process block should direct agent to read and execute workflow file'
);
assert.ok(
processBlock.includes('MANDATORY'),
'process block should include MANDATORY instruction to read workflow files'
);
// The process block must NOT contain detailed step-by-step instructions
// that could substitute for the actual workflow file
assert.ok(
!processBlock.includes('Scout codebase'),
'process block should not contain detailed workflow steps (Scout codebase)'
);
assert.ok(
!processBlock.includes('Deep-dive each area'),
'process block should not contain detailed workflow steps (Deep-dive)'
);
assert.ok(
!processBlock.includes('Probing depth'),
'process block should not contain detailed workflow steps (Probing depth)'
);
});
test('discuss-phase command argument-hint includes --text', () => {
const command = fs.readFileSync(
path.join(__dirname, '..', 'commands', 'gsd', 'discuss-phase.md'), 'utf8'