Merge pull request #1297 from gsd-build/fix/discuss-phase-workflow-bypass-1292
fix: prevent discuss-phase from ignoring workflow instructions
This commit is contained in:
@@ -46,50 +46,11 @@ Context files are resolved in-workflow using `init phase-op` and roadmap/state t
|
||||
DISCUSS_MODE=$(node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" config-get workflow.discuss_mode 2>/dev/null || echo "discuss")
|
||||
```
|
||||
|
||||
If `DISCUSS_MODE` is `"assumptions"`: **Follow the discuss-phase-assumptions.md workflow instead of the steps below.** Skip all remaining steps in this process section.
|
||||
If `DISCUSS_MODE` is `"assumptions"`: Read and execute @~/.claude/get-shit-done/workflows/discuss-phase-assumptions.md end-to-end.
|
||||
|
||||
If `DISCUSS_MODE` is `"discuss"` (or unset, or any other value): Continue with the steps below (current behavior).
|
||||
If `DISCUSS_MODE` is `"discuss"` (or unset, or any other value): Read and execute @~/.claude/get-shit-done/workflows/discuss-phase.md end-to-end.
|
||||
|
||||
---
|
||||
|
||||
1. Validate phase number (error if missing or not in roadmap)
|
||||
2. Check if CONTEXT.md exists (offer update/view/skip if yes)
|
||||
3. **Load prior context** — Read PROJECT.md, REQUIREMENTS.md, STATE.md, and all prior CONTEXT.md files
|
||||
4. **Scout codebase** — Find reusable assets, patterns, and integration points
|
||||
5. **Analyze phase** — Check prior decisions, skip already-decided areas, generate remaining gray areas
|
||||
6. **Present gray areas** — Multi-select: which to discuss? Annotate with prior decisions + code context
|
||||
7. **Deep-dive each area** — 4 questions per area, code-informed options, Context7 for library choices
|
||||
8. **Write CONTEXT.md** — Sections match areas discussed + code_context section
|
||||
9. Offer next steps (research or plan)
|
||||
|
||||
**CRITICAL: Scope guardrail**
|
||||
- Phase boundary from ROADMAP.md is FIXED
|
||||
- Discussion clarifies HOW to implement, not WHETHER to add more
|
||||
- If user suggests new capabilities: "That's its own phase. I'll note it for later."
|
||||
- Capture deferred ideas — don't lose them, don't act on them
|
||||
|
||||
**Domain-aware gray areas:**
|
||||
Gray areas depend on what's being built. Analyze the phase goal:
|
||||
- Something users SEE → layout, density, interactions, states
|
||||
- Something users CALL → responses, errors, auth, versioning
|
||||
- Something users RUN → output format, flags, modes, error handling
|
||||
- Something users READ → structure, tone, depth, flow
|
||||
- Something being ORGANIZED → criteria, grouping, naming, exceptions
|
||||
|
||||
Generate 3-4 **phase-specific** gray areas, not generic categories.
|
||||
|
||||
**Probing depth:**
|
||||
- Ask 4 questions per area before checking
|
||||
- "More questions about [area], or move to next? (Remaining: [list unvisited areas])"
|
||||
- Show remaining unvisited areas so user knows what's still ahead
|
||||
- If more → ask 4 more, check again
|
||||
- After all areas → "Ready to create context?"
|
||||
|
||||
**Do NOT ask about (Claude handles these):**
|
||||
- Technical implementation
|
||||
- Architecture choices
|
||||
- Performance concerns
|
||||
- Scope expansion
|
||||
**MANDATORY:** The execution_context files listed above ARE the instructions. Read the workflow file BEFORE taking any action. The objective and success_criteria sections in this command file are summaries — the workflow file contains the complete step-by-step process with all required behaviors, config checks, and interaction patterns. Do not improvise from the summary.
|
||||
</process>
|
||||
|
||||
<success_criteria>
|
||||
|
||||
@@ -18,6 +18,8 @@ const VALID_CONFIG_KEYS = new Set([
|
||||
'workflow.nyquist_validation', 'workflow.ui_phase', 'workflow.ui_safety_gate',
|
||||
'workflow.auto_advance', 'workflow.node_repair', 'workflow.node_repair_budget',
|
||||
'workflow.text_mode',
|
||||
'workflow.research_before_questions',
|
||||
'workflow.discuss_mode',
|
||||
'workflow._auto_chain_active',
|
||||
'git.branching_strategy', 'git.phase_branch_template', 'git.milestone_branch_template', 'git.quick_branch_template',
|
||||
'planning.commit_docs', 'planning.search_gitignored',
|
||||
@@ -28,6 +30,8 @@ const CONFIG_KEY_SUGGESTIONS = {
|
||||
'workflow.nyquist_validation_enabled': 'workflow.nyquist_validation',
|
||||
'agents.nyquist_validation_enabled': 'workflow.nyquist_validation',
|
||||
'nyquist.validation_enabled': 'workflow.nyquist_validation',
|
||||
'hooks.research_questions': 'workflow.research_before_questions',
|
||||
'workflow.research_questions': 'workflow.research_before_questions',
|
||||
};
|
||||
|
||||
function validateKnownConfigKeyPath(keyPath) {
|
||||
@@ -108,6 +112,8 @@ function buildNewProjectConfig(userChoices) {
|
||||
ui_phase: true,
|
||||
ui_safety_gate: true,
|
||||
text_mode: false,
|
||||
research_before_questions: false,
|
||||
discuss_mode: 'discuss',
|
||||
},
|
||||
hooks: {
|
||||
context_warnings: true,
|
||||
|
||||
@@ -7,7 +7,8 @@
|
||||
"verifier": true,
|
||||
"auto_advance": false,
|
||||
"nyquist_validation": true,
|
||||
"discuss_mode": "discuss"
|
||||
"discuss_mode": "discuss",
|
||||
"research_before_questions": false
|
||||
},
|
||||
"planning": {
|
||||
"commit_docs": true,
|
||||
|
||||
@@ -570,7 +570,7 @@ Track deferred ideas internally.
|
||||
|
||||
For each selected area, conduct a focused discussion loop.
|
||||
|
||||
**Research-before-questions mode:** Check if `research_questions` is enabled in config (from init context or `.planning/config.json`). When enabled, before presenting questions for each area:
|
||||
**Research-before-questions mode:** Check if `workflow.research_before_questions` is enabled in config (from init context or `.planning/config.json`). When enabled, before presenting questions for each area:
|
||||
1. Do a brief web search for best practices related to the area topic
|
||||
2. Summarize the top findings in 2-3 bullet points
|
||||
3. Present the research alongside the question so the user can make a more informed decision
|
||||
|
||||
@@ -218,7 +218,7 @@ Ask inline (freeform, NOT AskUserQuestion):
|
||||
|
||||
Wait for their response. This gives you the context needed to ask intelligent follow-up questions.
|
||||
|
||||
**Research-before-questions mode:** Check if `research_questions` is enabled in `.planning/config.json` (or the config from init context). When enabled, before asking follow-up questions about a topic area:
|
||||
**Research-before-questions mode:** Check if `workflow.research_before_questions` is enabled in `.planning/config.json` (or the config from init context). When enabled, before asking follow-up questions about a topic area:
|
||||
|
||||
1. Do a brief web search for best practices related to what the user described
|
||||
2. Mention key findings naturally as you ask questions (e.g., "Most projects like this use X — is that what you're thinking, or something different?")
|
||||
|
||||
@@ -163,7 +163,10 @@ Merge new settings into existing config.json:
|
||||
"auto_advance": true/false,
|
||||
"nyquist_validation": true/false,
|
||||
"ui_phase": true/false,
|
||||
"ui_safety_gate": true/false
|
||||
"ui_safety_gate": true/false,
|
||||
"text_mode": true/false,
|
||||
"research_before_questions": true/false,
|
||||
"discuss_mode": "discuss" | "assumptions"
|
||||
},
|
||||
"git": {
|
||||
"branching_strategy": "none" | "phase" | "milestone",
|
||||
@@ -171,11 +174,7 @@ Merge new settings into existing config.json:
|
||||
},
|
||||
"hooks": {
|
||||
"context_warnings": true/false,
|
||||
"workflow_guard": true/false,
|
||||
"research_questions": true/false
|
||||
},
|
||||
"workflow": {
|
||||
"text_mode": true/false // Use plain-text questions instead of TUI menus (for /rc remote sessions)
|
||||
"workflow_guard": true/false
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
@@ -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)', () => {
|
||||
|
||||
@@ -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'
|
||||
|
||||
Reference in New Issue
Block a user