fix(#2772): resolve four discuss-phase text inconsistencies (dead MAX_PASSES read, gate-prompts drift, circular auto_advance, answer_validation drift) (#2886)
* test(#2772): structural guards for the four discuss-phase text inconsistencies * fix(#2772): resolve four discuss-phase text inconsistencies 1. auto.md: remove the dead MAX_PASSES/max_discuss_passes config read (contradicted the mandated single-pass rule + wasted a shim invocation per auto run). 2. gate-prompts.md: context-handling options now match the actual check_existing flow (Update it | View it | Skip, not Overwrite|Append|Cancel); gray-area-option no longer mandates 'Let Claude decide' (contradicts discuss-phase.md's no-cop-out rule). 3. discuss-phase.md: auto_advance fallback ends the workflow instead of routing back to the already-run confirm_creation step (circular). 4. discuss-phase-assumptions.md: re-sync answer_validation to the parent canonical block (had drifted — lost the 'Other' empty-text branch). * chore(#2772): changeset fragment * fix+test(#2772): also fix the assumptions auto_advance circularity (review minor 1) + add positive test anchors (review minor 2) The sibling discuss-phase-assumptions.md had the identical auto_advance→confirm_creation circularity; fix it the same way (end the workflow). Add positive anchors to both auto_advance tests so a re-phrased regression can't slip past. File #2885 for the dead max_discuss_passes config still advertised in settings/registry/docs (review minor 3). * fix(#2772): keep discuss-phase.md under the 32000B #717 cap + ack assumptions growth The auto_advance fixes + the assumptions answer_validation re-sync grew both files past the emitted-attribution gate (and discuss-phase.md past the #717 32000B cap). Condense the auto_advance prose in both files (discuss-phase.md now net -11, under cap; auto.md already net -4650 from the MAX_PASSES shim removal). Add discuss-phase-assumptions.md to tests/emitted-drift-ack.json for its residual +220 (answer_validation re-sync + auto_advance fix). * chore(#2772): backfill changeset PR number (2886) --------- Co-authored-by: Test <test@example.com>
This commit is contained in:
5
.changeset/jolly-ibex-chatter.md
Normal file
5
.changeset/jolly-ibex-chatter.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 2886
|
||||
---
|
||||
**Discuss-phase no longer carries four internal text contradictions** — auto-mode removed a dead `max_discuss_passes` config read that contradicted its single-pass rule; the gate-prompts reference now matches the actual context-handling options and drops the 'Let Claude decide' cop-out that conflicted with the workflow's no-skip rule; the auto_advance fallback no longer routes back to the already-run confirm_creation step; and the assumptions workflow's answer_validation is re-synced to the canonical parent block.
|
||||
@@ -90,11 +90,14 @@ Up to 4 suggested next actions with selection (status, resume workflows).
|
||||
3-option handler for existing CONTEXT.md in discuss workflow.
|
||||
- question: "Phase {N} already has a CONTEXT.md. How should we handle it?"
|
||||
- header: "Context"
|
||||
- options: Overwrite | Append | Cancel
|
||||
- options: Update it | View it | Skip
|
||||
|
||||
## Pattern: gray-area-option
|
||||
Dynamic template for presenting gray area choices in discuss workflow.
|
||||
- question: "{Gray area title}"
|
||||
- header: "Decision"
|
||||
- options: {Option 1} | {Option 2} | Let Claude decide
|
||||
- Note: Options generated at runtime. Always include "Let Claude decide" as last option.
|
||||
- options: {Option 1} | {Option 2}
|
||||
- Note: Options generated at runtime. Present real choices only — do NOT include a
|
||||
"skip" or "you decide" option (the user ran discuss-phase to decide). An "Other"
|
||||
free-text escape hatch may be offered per modes/default.md, but never a
|
||||
"Let Claude decide" cop-out.
|
||||
|
||||
@@ -48,14 +48,12 @@ Capture the idea in "Deferred Ideas". Don't lose it, don't act on it.
|
||||
</scope_guardrail>
|
||||
|
||||
<answer_validation>
|
||||
**IMPORTANT: Answer validation** — After every AskUserQuestion call, check if the response
|
||||
is empty or whitespace-only. If so:
|
||||
1. Retry the question once with the same parameters
|
||||
2. If still empty, present the options as a plain-text numbered list
|
||||
**IMPORTANT: Answer validation** — After every AskUserQuestion call, if the response is empty/whitespace-only:
|
||||
|
||||
**Text mode (`workflow.text_mode: true` in config or `--text` flag):**
|
||||
When text mode is active, do not use AskUserQuestion at all. Present every question as a
|
||||
plain-text numbered list and ask the user to type their choice number.
|
||||
- **"Other" with empty text** (the user wants to type freeform): output `"What would you like to discuss?"`, STOP generating, wait for the user's next message, then reflect it back and continue. Do NOT retry AskUserQuestion or call any tools.
|
||||
- **Any other empty response:** retry once with the same parameters; if still empty, present options as a plain-text numbered list. Never proceed with empty input.
|
||||
|
||||
**Text mode** (`--text` or `workflow.text_mode: true`): follow `workflows/discuss-phase/modes/text.md` — do not use AskUserQuestion at all.
|
||||
</answer_validation>
|
||||
|
||||
<process>
|
||||
@@ -668,7 +666,7 @@ Handle return: PHASE COMPLETE / PLANNING COMPLETE / INCONCLUSIVE / GAPS FOUND
|
||||
(identical handling to discuss-phase.md auto_advance step)
|
||||
|
||||
**If neither `--auto` nor config enabled:**
|
||||
Route to confirm_creation step.
|
||||
End here — `confirm_creation` already ran; do not route back to it.
|
||||
</step>
|
||||
|
||||
</process>
|
||||
|
||||
@@ -488,9 +488,9 @@ gsd_run query commit "docs(state): record phase ${PHASE} context session" --file
|
||||
<step name="auto_advance">
|
||||
Auto-advance behavior is defined in `workflows/discuss-phase/modes/chain.md`.
|
||||
|
||||
If `--auto`, `--chain`, or `workflow.auto_advance` is enabled, Read that file now and execute its `auto_advance` step (which handles flag-syncing, banner display, plan-phase Skill dispatch, and return-status branching).
|
||||
If `--auto`, `--chain`, or `workflow.auto_advance` is enabled, Read that file now and execute its `auto_advance` step (flag-syncing, banner, plan-phase dispatch, return-status branching).
|
||||
|
||||
Otherwise, route to `confirm_creation` (manual next steps).
|
||||
Otherwise, end here — `confirm_creation` already ran; do not route back to it.
|
||||
</step>
|
||||
|
||||
</process>
|
||||
|
||||
@@ -38,12 +38,6 @@ find "gaps", "undefined types", or "missing decisions" and run additional
|
||||
passes. This creates a self-feeding loop where each pass generates references
|
||||
that the next pass treats as gaps, consuming unbounded time and resources.
|
||||
|
||||
Check the pass cap from config:
|
||||
```bash
|
||||
_GSD_SHIM_NAME="gsd-tools.cjs"; _GSD_RUNTIME_ROOT="${RUNTIME_DIR:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}"; GSD_TOOLS="${_GSD_RUNTIME_ROOT}/gsd-core/bin/${_GSD_SHIM_NAME}"; if [ -f "$GSD_TOOLS" ]; then gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${_GSD_RUNTIME_ROOT}/.claude/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${_GSD_RUNTIME_ROOT}/.claude/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${_GSD_RUNTIME_ROOT}/.codex/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${_GSD_RUNTIME_ROOT}/.codex/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif command -v gsd-tools >/dev/null 2>&1; then GSD_TOOLS="$(command -v gsd-tools)"; gsd_run() { "$GSD_TOOLS" "$@"; }; elif [ -f "${CLAUDE_CONFIG_DIR:-$HOME/.claude}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CLAUDE_CONFIG_DIR:-$HOME/.claude}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${HERMES_HOME:-$HOME/.hermes}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${HERMES_HOME:-$HOME/.hermes}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${CURSOR_CONFIG_DIR:-$HOME/.cursor}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CURSOR_CONFIG_DIR:-$HOME/.cursor}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${CODEX_HOME:-$HOME/.codex}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CODEX_HOME:-$HOME/.codex}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${GEMINI_CONFIG_DIR:-$HOME/.gemini}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${GEMINI_CONFIG_DIR:-$HOME/.gemini}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${COPILOT_CONFIG_DIR:-$HOME/.copilot}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${COPILOT_CONFIG_DIR:-$HOME/.copilot}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${WINDSURF_CONFIG_DIR:-$HOME/.codeium/windsurf}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${WINDSURF_CONFIG_DIR:-$HOME/.codeium/windsurf}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${AUGMENT_CONFIG_DIR:-$HOME/.augment}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${AUGMENT_CONFIG_DIR:-$HOME/.augment}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${TRAE_CONFIG_DIR:-$HOME/.trae}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${TRAE_CONFIG_DIR:-$HOME/.trae}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${QWEN_CONFIG_DIR:-$HOME/.qwen}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${QWEN_CONFIG_DIR:-$HOME/.qwen}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${CODEBUDDY_CONFIG_DIR:-$HOME/.codebuddy}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CODEBUDDY_CONFIG_DIR:-$HOME/.codebuddy}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${CLINE_CONFIG_DIR:-$HOME/.cline}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CLINE_CONFIG_DIR:-$HOME/.cline}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${GROK_AGENTS_HOME:-$HOME/.agents}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${GROK_AGENTS_HOME:-$HOME/.agents}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${ANTIGRAVITY_CONFIG_DIR:-$HOME/.gemini/antigravity}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${ANTIGRAVITY_CONFIG_DIR:-$HOME/.gemini/antigravity}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${OPENCODE_CONFIG_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/opencode}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${OPENCODE_CONFIG_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/opencode}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${KILO_CONFIG_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/kilo}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${KILO_CONFIG_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/kilo}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; else echo "ERROR: gsd-tools.cjs not found at $GSD_TOOLS and gsd-tools is not on PATH. Run: npx -y @opengsd/gsd-core@latest --claude --local" >&2; exit 1; fi; if [ -n "${CLAUDE_ENV_FILE:-}" ] && [ -n "${GSD_TOOLS:-}" ]; then printf "export PATH='%s':\"\$PATH\"\n" "${GSD_TOOLS%/*}" >> "$CLAUDE_ENV_FILE" 2>/dev/null || true; fi
|
||||
MAX_PASSES=$(gsd_run query config-get workflow.max_discuss_passes 2>/dev/null || echo "3")
|
||||
```
|
||||
|
||||
If you have already written and committed CONTEXT.md, the discuss step is
|
||||
complete. Move on.
|
||||
|
||||
|
||||
@@ -3,6 +3,9 @@
|
||||
"paths": {
|
||||
"plan-phase.md": {
|
||||
"reason": "#2770: decision-coverage gate recomputes CONTEXT_PATH in-block + guards the empty-glob case (handler now fails closed on empty arg). Net growth kept under the ADR-857 size cap by condensing adjacent §13a prose/JSON; the residual +89 bytes are the irreducible glob+guard logic."
|
||||
},
|
||||
"discuss-phase-assumptions.md": {
|
||||
"reason": "#2772: re-synced <answer_validation> to the parent canonical block (had drifted — lost the 'Other' empty-text branch) + fixed the auto_advance→confirm_creation circularity (end the workflow). discuss-phase.md is net -11 (condensed); auto.md shrank -4650 (removed a dead MAX_PASSES resolver shim)."
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
59
tests/issue-2772-discuss-phase-text-inconsistencies.test.cjs
Normal file
59
tests/issue-2772-discuss-phase-text-inconsistencies.test.cjs
Normal file
@@ -0,0 +1,59 @@
|
||||
// allow-test-rule: structural-implementation-guard (#2772)
|
||||
'use strict';
|
||||
|
||||
// Regression guard for #2772: four self-contained text inconsistencies in the
|
||||
// discuss-phase surface, each a literal-instruction hazard. The shipped markdown IS
|
||||
// the runtime contract, so structural inspection is the correct guard.
|
||||
|
||||
const { test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const ROOT = path.join(__dirname, '..');
|
||||
const read = (rel) => fs.readFileSync(path.join(ROOT, rel), 'utf8');
|
||||
|
||||
test('auto.md does not read the dead MAX_PASSES / max_discuss_passes config (#2772.1)', () => {
|
||||
const src = read('gsd-core/workflows/discuss-phase/modes/auto.md');
|
||||
assert.ok(/single pass/i.test(src), 'auto.md must still mandate the single-pass rule');
|
||||
assert.ok(!/MAX_PASSES=/.test(src), 'auto.md must not read MAX_PASSES (dead config — single-pass rule governs) (#2772)');
|
||||
assert.ok(!/max_discuss_passes/.test(src), 'auto.md must not reference max_discuss_passes (contradicts the single-pass rule) (#2772)');
|
||||
});
|
||||
|
||||
test('gate-prompts context-handling matches the actual check_existing options (#2772.2)', () => {
|
||||
const src = read('gsd-core/references/gate-prompts.md');
|
||||
const ctx = src.slice(src.indexOf('## Pattern: context-handling'), src.indexOf('## Pattern: gray-area-option'));
|
||||
assert.ok(/Update it \| View it \| Skip/.test(ctx), 'context-handling options must be "Update it | View it | Skip" (the actual check_existing flow) (#2772)');
|
||||
assert.ok(!/Overwrite \| Append \| Cancel/.test(ctx), 'context-handling must NOT document the obsolete "Overwrite | Append | Cancel" (#2772)');
|
||||
});
|
||||
|
||||
test('gate-prompts gray-area-option does not mandate "Let Claude decide" (#2772.2)', () => {
|
||||
const src = read('gsd-core/references/gate-prompts.md');
|
||||
const gray = src.slice(src.indexOf('## Pattern: gray-area-option'));
|
||||
assert.ok(!/Always include "Let Claude decide"/i.test(gray), 'gray-area-option must NOT mandate "Let Claude decide" — it contradicts discuss-phase.md:353 ("Do NOT include a skip or you decide option") (#2772)');
|
||||
});
|
||||
|
||||
test('discuss-phase auto_advance fallback ends the workflow, not routes back to confirm_creation (#2772.3)', () => {
|
||||
const src = read('gsd-core/workflows/discuss-phase.md');
|
||||
const step = src.slice(src.indexOf('<step name="auto_advance">'), src.indexOf('</step>', src.indexOf('<step name="auto_advance">')));
|
||||
assert.ok(!/route to `confirm_creation`/.test(step), 'auto_advance fallback must not route back to confirm_creation (it already ran earlier in the step order — circular) (#2772)');
|
||||
assert.ok(/end here|workflow is complete/i.test(step), 'auto_advance fallback must explicitly END the workflow (positive anchor — a re-phrased regression should not slip past) (#2772)');
|
||||
});
|
||||
|
||||
test('discuss-phase-assumptions auto_advance fallback also ends the workflow (sibling of #2772.3)', () => {
|
||||
const src = read('gsd-core/workflows/discuss-phase-assumptions.md');
|
||||
const step = src.slice(src.indexOf('<step name="auto_advance">'), src.indexOf('</step>', src.indexOf('<step name="auto_advance">')));
|
||||
assert.ok(!/Route to confirm_creation step/.test(step), 'assumptions auto_advance fallback must not route back to confirm_creation (same circularity as the parent) (#2772)');
|
||||
assert.ok(/end here|workflow is complete/i.test(step), 'assumptions auto_advance fallback must explicitly END the workflow (#2772)');
|
||||
});
|
||||
|
||||
test('discuss-phase-assumptions answer_validation matches the parent canonical content (#2772.4)', () => {
|
||||
const parent = read('gsd-core/workflows/discuss-phase.md');
|
||||
const assumptions = read('gsd-core/workflows/discuss-phase-assumptions.md');
|
||||
// The parent's canonical answer_validation includes the "Other" empty-text branch.
|
||||
const parentBlock = parent.slice(parent.indexOf('<answer_validation>'), parent.indexOf('</answer_validation>') + '</answer_validation>'.length);
|
||||
const assumptionsBlock = assumptions.slice(assumptions.indexOf('<answer_validation>'), assumptions.indexOf('</answer_validation>') + '</answer_validation>'.length);
|
||||
assert.ok(/"Other" with empty text/.test(assumptionsBlock), 'assumptions answer_validation must include the "Other" empty-text branch (was drifted) (#2772)');
|
||||
// The two blocks must now agree on the empty-response handling.
|
||||
assert.strictEqual(assumptionsBlock, parentBlock, 'discuss-phase-assumptions answer_validation must match the parent canonical block exactly (single source of truth) (#2772)');
|
||||
});
|
||||
Reference in New Issue
Block a user