diff --git a/.changeset/sunny-wolves-gather.md b/.changeset/sunny-wolves-gather.md new file mode 100644 index 000000000..8339143e4 --- /dev/null +++ b/.changeset/sunny-wolves-gather.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3454 +--- +gsd-review no longer creates empty gsd-review-context.md / gsd-review-research.md section files (or hangs waiting on input) when a phase has no CONTEXT/RESEARCH notes: the build_prompt guards now test the glob expansion itself instead of probing with ls, which the block's nullglob setting had made always-true. diff --git a/gsd-core/workflows/review.md b/gsd-core/workflows/review.md index 2aec065f4..0518995fb 100644 --- a/gsd-core/workflows/review.md +++ b/gsd-core/workflows/review.md @@ -266,11 +266,13 @@ done if [ -f ".planning/PROJECT.md" ]; then cp .planning/PROJECT.md "${RUN_DIR}/gsd-review-project.md" fi -if ls "${PHASE_DIR}/"*"-CONTEXT.md" >/dev/null 2>&1; then - cat "${PHASE_DIR}/"*"-CONTEXT.md" > "${RUN_DIR}/gsd-review-context.md" +_CTX=( "${PHASE_DIR}"/*-CONTEXT.md ) +if [ ${#_CTX[@]} -gt 0 ]; then + cat "${_CTX[@]}" > "${RUN_DIR}/gsd-review-context.md" fi -if ls "${PHASE_DIR}/"*"-RESEARCH.md" >/dev/null 2>&1; then - cat "${PHASE_DIR}/"*"-RESEARCH.md" > "${RUN_DIR}/gsd-review-research.md" +_RESEARCH=( "${PHASE_DIR}"/*-RESEARCH.md ) +if [ ${#_RESEARCH[@]} -gt 0 ]; then + cat "${_RESEARCH[@]}" > "${RUN_DIR}/gsd-review-research.md" fi if [ -f ".planning/REQUIREMENTS.md" ]; then cp .planning/REQUIREMENTS.md "${RUN_DIR}/gsd-review-requirements.md" diff --git a/scripts/lint-allow-test-rule-refs.ceiling.json b/scripts/lint-allow-test-rule-refs.ceiling.json index da43d9eb0..05b0a1f86 100644 --- a/scripts/lint-allow-test-rule-refs.ceiling.json +++ b/scripts/lint-allow-test-rule-refs.ceiling.json @@ -1,4 +1,4 @@ { - "maxFiles": 300, + "maxFiles": 301, "grace": 3 } diff --git a/tests/review-build-prompt-optional-sections.test.cjs b/tests/review-build-prompt-optional-sections.test.cjs new file mode 100644 index 000000000..96c096985 --- /dev/null +++ b/tests/review-build-prompt-optional-sections.test.cjs @@ -0,0 +1,296 @@ +// allow-test-rule: source-text-is-the-product (see #3300) +// The workflow markdown IS the installed orchestration contract; these rows +// extract the real shipped bash and execute it, never a hand-copied duplicate. + +'use strict'; + +/** + * #3300 — the `if ls ` guards for the optional CONTEXT/RESEARCH sections + * of review.md's build_prompt block are defeated by the block's own nullglob + * shim (#2962): an unmatched glob expands to nothing, so `ls` runs with zero + * operands (lists the working directory, exits 0 — guard unconditionally true) + * and `cat` runs with zero operands and reads STDIN: a 0-byte section file at + * EOF, an indefinite hang on an open pipe. + * + * Behavioral seam: extract the REAL fenced build_prompt block from + * gsd-core/workflows/review.md (+ its steps/ fragments, via readWorkflowCombined), + * fill the `{run_dir}` placeholder exactly as the workflow host does, and run the + * whole block in a temp PHASE_DIR/RUN_DIR under bash and — when present — zsh, + * the two dialects the block's own `shopt -s nullglob; setopt NULL_GLOB` shim + * targets. Rows: + * 1-2 absent sources -> NO output file created at all (not even 0-byte) + * 3-6 present sources -> exact bytes, all matches concatenated in glob order + * 7-8 open, never-closed stdin pipe -> block completes within a short bound + * 9 structural: no `if ls` guard survives in any review.md bash fence, and + * the #2962 nullglob shim itself is still present (out-of-scope guard). + * + * Script transport is temp-FILE based, never `bash -c