diff --git a/gsd-core/workflows/review.md b/gsd-core/workflows/review.md index 329a93c08..324440c70 100644 --- a/gsd-core/workflows/review.md +++ b/gsd-core/workflows/review.md @@ -277,10 +277,15 @@ fi # $CODEX_BYPASS_FLAG is capability-gated above (#1115). Capture stderr to a .err # file (not /dev/null) so a non-zero exit — e.g. a flag the installed codex-cli # does not support — is diagnosable instead of a silent empty review. +# Capture the review via codex's own `-o/--output-last-message ` (only the +# final agent message) and discard stdout (#1698): on some platforms (Windows) +# codex writes process-teardown output to stdout *after* the final message, and a +# stdout redirect would append that noise to a non-empty file — slipping past the +# `[ ! -s … ]` empty-output guard as a silently polluted review. if [ -n "$CODEX_MODEL" ] && [ "$CODEX_MODEL" != "null" ]; then - cat /tmp/gsd-review-prompt-{phase}.md | codex exec --ephemeral $CODEX_BYPASS_FLAG --model "$CODEX_MODEL" --skip-git-repo-check - 2>/tmp/gsd-review-codex-{phase}.err > /tmp/gsd-review-codex-{phase}.md + cat /tmp/gsd-review-prompt-{phase}.md | codex exec --ephemeral $CODEX_BYPASS_FLAG --model "$CODEX_MODEL" --skip-git-repo-check -o /tmp/gsd-review-codex-{phase}.md - 2>/tmp/gsd-review-codex-{phase}.err >/dev/null else - cat /tmp/gsd-review-prompt-{phase}.md | codex exec --ephemeral $CODEX_BYPASS_FLAG --skip-git-repo-check - 2>/tmp/gsd-review-codex-{phase}.err > /tmp/gsd-review-codex-{phase}.md + cat /tmp/gsd-review-prompt-{phase}.md | codex exec --ephemeral $CODEX_BYPASS_FLAG --skip-git-repo-check -o /tmp/gsd-review-codex-{phase}.md - 2>/tmp/gsd-review-codex-{phase}.err >/dev/null fi if [ ! -s /tmp/gsd-review-codex-{phase}.md ]; then echo "Codex review failed or returned empty output. stderr:" > /tmp/gsd-review-codex-{phase}.md diff --git a/tests/enh-773-codex-exec-automation-flags.test.cjs b/tests/enh-773-codex-exec-automation-flags.test.cjs index ec187951d..3c9063019 100644 --- a/tests/enh-773-codex-exec-automation-flags.test.cjs +++ b/tests/enh-773-codex-exec-automation-flags.test.cjs @@ -99,3 +99,42 @@ describe('enh-773: automated codex exec invocations include --ephemeral and --da } }); }); + +describe('#1698 regression: codex review is captured via --output-last-message, not stdout', () => { + // WHY: on some platforms (Windows) `codex exec` writes process-teardown output + // to stdout *after* the final agent message. A `> FILE` stdout redirect appends + // that noise to a non-empty file, so it slips past the `[ ! -s … ]` empty-output + // guard and downstream consumers (severity extraction, the + // plan-review-convergence "concerns resolved?" gate) parse a polluted review. + // `-o/--output-last-message ` writes only the final message — robust on + // every platform — so each codex invocation must capture via -o and discard stdout. + const workflow = fs.readFileSync( + path.join(process.cwd(), 'gsd-core', 'workflows', 'review.md'), + 'utf8' + ); + const codexExecLines = workflow + .split('\n') + .filter((line) => line.includes('codex exec') && !line.includes('codex exec --help')); + + test('every codex exec invocation captures the review via -o ', () => { + for (const line of codexExecLines) { + assert.ok( + /\s-o\s+\/tmp\/gsd-review-codex-\{phase\}\.md\b/.test(line), + `codex exec invocation must capture the review via -o /tmp/gsd-review-codex-{phase}.md:\n ${line.trim()}` + ); + } + }); + + test('no codex exec invocation redirects stdout into the review file', () => { + for (const line of codexExecLines) { + assert.ok( + !/>\s*\/tmp\/gsd-review-codex-\{phase\}\.md\b/.test(line), + `codex exec must not redirect stdout into the review file (teardown noise pollutes it); use -o + >/dev/null:\n ${line.trim()}` + ); + assert.ok( + />\s*\/dev\/null\b/.test(line), + `codex exec must discard stdout to /dev/null so teardown output is not captured:\n ${line.trim()}` + ); + } + }); +}); diff --git a/tests/workflow-size-baseline.json b/tests/workflow-size-baseline.json index afa736f81..ffa32d0d8 100644 --- a/tests/workflow-size-baseline.json +++ b/tests/workflow-size-baseline.json @@ -63,7 +63,7 @@ "remove-phase.md": 8469, "remove-workspace.md": 7507, "resume-project.md": 17226, - "review.md": 40539, + "review.md": 40956, "scan.md": 7688, "secure-phase.md": 13476, "session-report.md": 4044,