diff --git a/.changeset/daring-badgers-forage.md b/.changeset/daring-badgers-forage.md new file mode 100644 index 000000000..f644f7212 --- /dev/null +++ b/.changeset/daring-badgers-forage.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1709 +--- +Codex reviewer now captures the review via codex's --output-last-message flag instead of redirecting stdout, so Windows process-teardown output no longer pollutes the review file and slips past the empty-output guard. 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..55c8cf7e4 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(/\r?\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/fixtures/golden-install-parity/antigravity.json b/tests/fixtures/golden-install-parity/antigravity.json index 9e8d51e5a..d41125d96 100644 --- a/tests/fixtures/golden-install-parity/antigravity.json +++ b/tests/fixtures/golden-install-parity/antigravity.json @@ -274,7 +274,7 @@ "gsd-core/workflows/remove-phase.md": "6b9947fba1a97f46", "gsd-core/workflows/remove-workspace.md": "95f05defffe6754e", "gsd-core/workflows/resume-project.md": "cadf390bae95fc76", - "gsd-core/workflows/review.md": "4d1ea3f9785044b3", + "gsd-core/workflows/review.md": "aa08449a6a180944", "gsd-core/workflows/scan.md": "f2754f3e3ea528ff", "gsd-core/workflows/secure-phase.md": "78705b7f09612464", "gsd-core/workflows/session-report.md": "2e5b1205324ddefa", diff --git a/tests/fixtures/golden-install-parity/augment.json b/tests/fixtures/golden-install-parity/augment.json index f4eba6dce..73ce3a66e 100644 --- a/tests/fixtures/golden-install-parity/augment.json +++ b/tests/fixtures/golden-install-parity/augment.json @@ -343,7 +343,7 @@ "gsd-core/workflows/remove-phase.md": "d030f80ca0df4fa9", "gsd-core/workflows/remove-workspace.md": "b8817a3a5907f5bc", "gsd-core/workflows/resume-project.md": "b18b51fd15cbce95", - "gsd-core/workflows/review.md": "fc23a807c6c3d6d9", + "gsd-core/workflows/review.md": "fbfa6eebcad41aa0", "gsd-core/workflows/scan.md": "54ff1ff60041d065", "gsd-core/workflows/secure-phase.md": "1d1c66ad9ea01bd2", "gsd-core/workflows/session-report.md": "2e5b1205324ddefa", diff --git a/tests/fixtures/golden-install-parity/claude.json b/tests/fixtures/golden-install-parity/claude.json index 37a05e6b4..118ce1ced 100644 --- a/tests/fixtures/golden-install-parity/claude.json +++ b/tests/fixtures/golden-install-parity/claude.json @@ -273,7 +273,7 @@ "gsd-core/workflows/remove-phase.md": "f76e1c2a4dd31a09", "gsd-core/workflows/remove-workspace.md": "0e73844f0f41cbc3", "gsd-core/workflows/resume-project.md": "3dcaa7abe1800d35", - "gsd-core/workflows/review.md": "1660ff860e79f9b7", + "gsd-core/workflows/review.md": "a1d3ebce6222ff72", "gsd-core/workflows/scan.md": "8e1bbf2eed1752ca", "gsd-core/workflows/secure-phase.md": "b2b9100ff79d6017", "gsd-core/workflows/session-report.md": "2e5b1205324ddefa", diff --git a/tests/fixtures/golden-install-parity/cline.json b/tests/fixtures/golden-install-parity/cline.json index f19eeefac..a2cf28979 100644 --- a/tests/fixtures/golden-install-parity/cline.json +++ b/tests/fixtures/golden-install-parity/cline.json @@ -277,7 +277,7 @@ "gsd-core/workflows/remove-phase.md": "61b68a4af414e3c2", "gsd-core/workflows/remove-workspace.md": "411bfff942216185", "gsd-core/workflows/resume-project.md": "f8f71b5a98374b85", - "gsd-core/workflows/review.md": "a74ae666e01ceebb", + "gsd-core/workflows/review.md": "f7d5b0f64631adea", "gsd-core/workflows/scan.md": "db0c48c3963f9a8b", "gsd-core/workflows/secure-phase.md": "c51b86e369574195", "gsd-core/workflows/session-report.md": "2e5b1205324ddefa", diff --git a/tests/fixtures/golden-install-parity/codebuddy.json b/tests/fixtures/golden-install-parity/codebuddy.json index 6cc789c4c..79eee4a13 100644 --- a/tests/fixtures/golden-install-parity/codebuddy.json +++ b/tests/fixtures/golden-install-parity/codebuddy.json @@ -343,7 +343,7 @@ "gsd-core/workflows/remove-phase.md": "d030f80ca0df4fa9", "gsd-core/workflows/remove-workspace.md": "b8817a3a5907f5bc", "gsd-core/workflows/resume-project.md": "b18b51fd15cbce95", - "gsd-core/workflows/review.md": "fc23a807c6c3d6d9", + "gsd-core/workflows/review.md": "fbfa6eebcad41aa0", "gsd-core/workflows/scan.md": "54ff1ff60041d065", "gsd-core/workflows/secure-phase.md": "1d1c66ad9ea01bd2", "gsd-core/workflows/session-report.md": "2e5b1205324ddefa", diff --git a/tests/fixtures/golden-install-parity/codex.json b/tests/fixtures/golden-install-parity/codex.json index 97b4a4b3a..13d8d4c9f 100644 --- a/tests/fixtures/golden-install-parity/codex.json +++ b/tests/fixtures/golden-install-parity/codex.json @@ -309,7 +309,7 @@ "gsd-core/workflows/remove-phase.md": "75c8ca0dbd1ce404", "gsd-core/workflows/remove-workspace.md": "a9d1dcda7755b6c9", "gsd-core/workflows/resume-project.md": "94ac2cac4c562557", - "gsd-core/workflows/review.md": "014d2f4d7c892e58", + "gsd-core/workflows/review.md": "bd3873bea54d6143", "gsd-core/workflows/scan.md": "29f3b65f5d9de885", "gsd-core/workflows/secure-phase.md": "858b5e1152b569ed", "gsd-core/workflows/session-report.md": "dd8fa011c9394075", diff --git a/tests/fixtures/golden-install-parity/copilot.json b/tests/fixtures/golden-install-parity/copilot.json index 7e7f03cb0..0f3d4d510 100644 --- a/tests/fixtures/golden-install-parity/copilot.json +++ b/tests/fixtures/golden-install-parity/copilot.json @@ -275,7 +275,7 @@ "gsd-core/workflows/remove-phase.md": "e94ddfe4eabc0e08", "gsd-core/workflows/remove-workspace.md": "f28be7f32249f0d4", "gsd-core/workflows/resume-project.md": "4ebcb4acd3c29302", - "gsd-core/workflows/review.md": "7c04ed635c7e1c56", + "gsd-core/workflows/review.md": "60b8ca1c543ccdfa", "gsd-core/workflows/scan.md": "28a2847b8d04156e", "gsd-core/workflows/secure-phase.md": "bae509fa254fe435", "gsd-core/workflows/session-report.md": "2e5b1205324ddefa", diff --git a/tests/fixtures/golden-install-parity/cursor.json b/tests/fixtures/golden-install-parity/cursor.json index 8b1b0acb0..803d07666 100644 --- a/tests/fixtures/golden-install-parity/cursor.json +++ b/tests/fixtures/golden-install-parity/cursor.json @@ -343,7 +343,7 @@ "gsd-core/workflows/remove-phase.md": "f76e1c2a4dd31a09", "gsd-core/workflows/remove-workspace.md": "dadbb14d9033ea3f", "gsd-core/workflows/resume-project.md": "3dcaa7abe1800d35", - "gsd-core/workflows/review.md": "28d673b6267b12c2", + "gsd-core/workflows/review.md": "141589a39b8c7e18", "gsd-core/workflows/scan.md": "8e1bbf2eed1752ca", "gsd-core/workflows/secure-phase.md": "7bd4c335484fd121", "gsd-core/workflows/session-report.md": "2e5b1205324ddefa", diff --git a/tests/fixtures/golden-install-parity/gemini.json b/tests/fixtures/golden-install-parity/gemini.json index 55f17cb98..2045c70df 100644 --- a/tests/fixtures/golden-install-parity/gemini.json +++ b/tests/fixtures/golden-install-parity/gemini.json @@ -343,7 +343,7 @@ "gsd-core/workflows/remove-phase.md": "d030f80ca0df4fa9", "gsd-core/workflows/remove-workspace.md": "9f5589b4b3a0639a", "gsd-core/workflows/resume-project.md": "b18b51fd15cbce95", - "gsd-core/workflows/review.md": "fc23a807c6c3d6d9", + "gsd-core/workflows/review.md": "fbfa6eebcad41aa0", "gsd-core/workflows/scan.md": "54ff1ff60041d065", "gsd-core/workflows/secure-phase.md": "a2f6a03034444f14", "gsd-core/workflows/session-report.md": "2e5b1205324ddefa", diff --git a/tests/fixtures/golden-install-parity/hermes.json b/tests/fixtures/golden-install-parity/hermes.json index 6ad135640..ddbba2c67 100644 --- a/tests/fixtures/golden-install-parity/hermes.json +++ b/tests/fixtures/golden-install-parity/hermes.json @@ -274,7 +274,7 @@ "gsd-core/workflows/remove-phase.md": "f27cecd8c78008ae", "gsd-core/workflows/remove-workspace.md": "977155e18b9df623", "gsd-core/workflows/resume-project.md": "849f3d49d366ff13", - "gsd-core/workflows/review.md": "d6c1c729a2627b2b", + "gsd-core/workflows/review.md": "06cd9992e289b241", "gsd-core/workflows/scan.md": "cbde2b8b2b5fa5dd", "gsd-core/workflows/secure-phase.md": "5fc8fbd5e217e48d", "gsd-core/workflows/session-report.md": "2e5b1205324ddefa", diff --git a/tests/fixtures/golden-install-parity/kilo.json b/tests/fixtures/golden-install-parity/kilo.json index 14fe6f1a1..107bf1d9c 100644 --- a/tests/fixtures/golden-install-parity/kilo.json +++ b/tests/fixtures/golden-install-parity/kilo.json @@ -343,7 +343,7 @@ "gsd-core/workflows/remove-phase.md": "f76e1c2a4dd31a09", "gsd-core/workflows/remove-workspace.md": "7bd5d1b63ef091a8", "gsd-core/workflows/resume-project.md": "3dcaa7abe1800d35", - "gsd-core/workflows/review.md": "50e945dade0a5a67", + "gsd-core/workflows/review.md": "9ed1fda4d8b657f1", "gsd-core/workflows/scan.md": "8e1bbf2eed1752ca", "gsd-core/workflows/secure-phase.md": "87fdcb37a8610e58", "gsd-core/workflows/session-report.md": "2e5b1205324ddefa", diff --git a/tests/fixtures/golden-install-parity/kimi.json b/tests/fixtures/golden-install-parity/kimi.json index b19ba081d..ed7190da5 100644 --- a/tests/fixtures/golden-install-parity/kimi.json +++ b/tests/fixtures/golden-install-parity/kimi.json @@ -310,7 +310,7 @@ "gsd-core/workflows/remove-phase.md": "d030f80ca0df4fa9", "gsd-core/workflows/remove-workspace.md": "b8817a3a5907f5bc", "gsd-core/workflows/resume-project.md": "b18b51fd15cbce95", - "gsd-core/workflows/review.md": "fc23a807c6c3d6d9", + "gsd-core/workflows/review.md": "fbfa6eebcad41aa0", "gsd-core/workflows/scan.md": "54ff1ff60041d065", "gsd-core/workflows/secure-phase.md": "1d1c66ad9ea01bd2", "gsd-core/workflows/session-report.md": "2e5b1205324ddefa", diff --git a/tests/fixtures/golden-install-parity/opencode.json b/tests/fixtures/golden-install-parity/opencode.json index fb15c1a6f..cd5799626 100644 --- a/tests/fixtures/golden-install-parity/opencode.json +++ b/tests/fixtures/golden-install-parity/opencode.json @@ -343,7 +343,7 @@ "gsd-core/workflows/remove-phase.md": "863c58e99e9d630d", "gsd-core/workflows/remove-workspace.md": "4ed03c52e5c7bd4d", "gsd-core/workflows/resume-project.md": "15153ce5a4c38674", - "gsd-core/workflows/review.md": "544103be3247bbd1", + "gsd-core/workflows/review.md": "4076abd5ff01195d", "gsd-core/workflows/scan.md": "4634caefa32a7307", "gsd-core/workflows/secure-phase.md": "9616682fe23cf042", "gsd-core/workflows/session-report.md": "2e5b1205324ddefa", diff --git a/tests/fixtures/golden-install-parity/qwen.json b/tests/fixtures/golden-install-parity/qwen.json index 93f76a366..8a753e7e8 100644 --- a/tests/fixtures/golden-install-parity/qwen.json +++ b/tests/fixtures/golden-install-parity/qwen.json @@ -274,7 +274,7 @@ "gsd-core/workflows/remove-phase.md": "17c46fdcef27df2d", "gsd-core/workflows/remove-workspace.md": "1d34788a9b2272d2", "gsd-core/workflows/resume-project.md": "bba3250afbea8f09", - "gsd-core/workflows/review.md": "926f2f15676a21cc", + "gsd-core/workflows/review.md": "8ed9a9e7d150978d", "gsd-core/workflows/scan.md": "514d3eba81565193", "gsd-core/workflows/secure-phase.md": "a7e8edb0e5f48256", "gsd-core/workflows/session-report.md": "2e5b1205324ddefa", diff --git a/tests/fixtures/golden-install-parity/trae.json b/tests/fixtures/golden-install-parity/trae.json index 7b120fa57..30b5bf43b 100644 --- a/tests/fixtures/golden-install-parity/trae.json +++ b/tests/fixtures/golden-install-parity/trae.json @@ -274,7 +274,7 @@ "gsd-core/workflows/remove-phase.md": "5a2521695edd486b", "gsd-core/workflows/remove-workspace.md": "ed8e5e30c33f1bfd", "gsd-core/workflows/resume-project.md": "ee9dcc4e3dd3e32c", - "gsd-core/workflows/review.md": "6529c2ee9b33f1fc", + "gsd-core/workflows/review.md": "a2dab55fa3acfca0", "gsd-core/workflows/scan.md": "afc48f3339293b30", "gsd-core/workflows/secure-phase.md": "882886f04d3b7e82", "gsd-core/workflows/session-report.md": "2e5b1205324ddefa", diff --git a/tests/fixtures/golden-install-parity/windsurf.json b/tests/fixtures/golden-install-parity/windsurf.json index 943551a51..59a41902a 100644 --- a/tests/fixtures/golden-install-parity/windsurf.json +++ b/tests/fixtures/golden-install-parity/windsurf.json @@ -274,7 +274,7 @@ "gsd-core/workflows/remove-phase.md": "24559d23e008c9c3", "gsd-core/workflows/remove-workspace.md": "e958c1e932aef492", "gsd-core/workflows/resume-project.md": "c5731b8aabed70f9", - "gsd-core/workflows/review.md": "21152d6ce0c4efa8", + "gsd-core/workflows/review.md": "dc8366ef168183db", "gsd-core/workflows/scan.md": "8aa95448b1ba4a4a", "gsd-core/workflows/secure-phase.md": "550152cd82c25c00", "gsd-core/workflows/session-report.md": "2e5b1205324ddefa", diff --git a/tests/workflow-size-baseline.json b/tests/workflow-size-baseline.json index c0e2ef9dc..a87212a83 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,