From 589a9b29b0bc8a8d390a0d1c7bee2427aef7cf02 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 5 Aug 2026 15:42:51 -0400 Subject: [PATCH] fix(#2962): enable nullglob in for-glob shell blocks for zsh portability (#3087) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#2962): enable nullglob in for-glob shell blocks for zsh portability Workflow shell blocks are fenced bash but execute in the user's login shell (zsh on macOS). zsh's nomatch default aborts the WHOLE block on an unmatched glob in a for-list (not just skipping the command), silently bypassing every statement after it — including the verify-phase decision-coverage gate, whose optional *-CONTEXT.md lookup used the unsafe for-list form so the DECISION_RESULT= assignment on the next line never ran under zsh. Fix: prepend a portable nullglob shim to every bash block containing a for-glob loop: shopt -s nullglob 2>/dev/null; setopt NULL_GLOB 2>/dev/null Each command no-ops (stderr suppressed) in the shell that doesn't recognize it; the matching shell enables nullglob so an unmatched glob expands to nothing and the loop body is skipped cleanly. Verified locally: both zsh and bash now reach end-of-block (rc=0) on a no-match glob; bash matched-case behavior unchanged. 14 blocks across 7 files: verify-phase.md (4, incl. the decision-coverage gate), review.md, execute-phase.md, resume-project.md, complete-milestone.md, audit-milestone.md, gsd-integration-checker.md, gsd-plan-checker.md (3). Closes the zsh bypass of the #2770 fix. * chore(#2962): add changeset fragment * chore(#2962): backfill changeset PR number 3087 --------- Co-authored-by: sim --- .changeset/kind-ravens-cheer.md | 5 +++++ agents/gsd-integration-checker.md | 3 +++ agents/gsd-plan-checker.md | 9 +++++++++ gsd-core/workflows/audit-milestone.md | 3 +++ gsd-core/workflows/complete-milestone.md | 3 +++ gsd-core/workflows/execute-phase.md | 4 ++-- gsd-core/workflows/resume-project.md | 3 +++ gsd-core/workflows/review.md | 6 ++++++ gsd-core/workflows/verify-phase.md | 11 ++++------- .../2962-zsh-nomatch-for-glob-portability.json | 10 ++++++++++ ...gmentize-review-and-discuss-phase-assumptions.json | 2 +- 11 files changed, 49 insertions(+), 10 deletions(-) create mode 100644 .changeset/kind-ravens-cheer.md create mode 100644 tests/emitted-drift-acks/2962-zsh-nomatch-for-glob-portability.json diff --git a/.changeset/kind-ravens-cheer.md b/.changeset/kind-ravens-cheer.md new file mode 100644 index 000000000..e95d31804 --- /dev/null +++ b/.changeset/kind-ravens-cheer.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3087 +--- +**Workflow shell blocks no longer abort under zsh when a glob matches nothing** — an unmatched glob inside a `for` word list aborted the entire shell block under zsh (macOS default shell), silently bypassing every statement after it, including the verify-phase decision-coverage gate. Each affected bash block now enables nullglob portably (`shopt -s nullglob 2>/dev/null; setopt NULL_GLOB 2>/dev/null`) so an unmatched glob expands to nothing and the loop is skipped cleanly under both shells. (#2962) diff --git a/agents/gsd-integration-checker.md b/agents/gsd-integration-checker.md index 8908c5db4..3921f76c9 100644 --- a/agents/gsd-integration-checker.md +++ b/agents/gsd-integration-checker.md @@ -94,6 +94,9 @@ For each phase, extract what it provides and what it should consume. **From SUMMARYs, extract:** ```bash +# #2962: zsh aborts the block on an unmatched for-list glob (nomatch); bash passes it through. nullglob both. +shopt -s nullglob 2>/dev/null; setopt NULL_GLOB 2>/dev/null + # Key exports from each phase for summary in .planning/phases/*/*-SUMMARY.md; do echo "=== $summary ===" diff --git a/agents/gsd-plan-checker.md b/agents/gsd-plan-checker.md index 7be4dcf24..f15b76904 100644 --- a/agents/gsd-plan-checker.md +++ b/agents/gsd-plan-checker.md @@ -714,6 +714,9 @@ Extract from init JSON: `phase_dir`, `phase_number`, `has_plans`, `plan_count`. Orchestrator provides CONTEXT.md content in the verification prompt. If provided, parse for locked decisions, discretion areas, deferred ideas. ```bash +# #2962: zsh aborts the block on an unmatched for-list glob (nomatch); bash passes it through. nullglob both. +shopt -s nullglob 2>/dev/null; setopt NULL_GLOB 2>/dev/null + gsd_run query phase.list-plans "$phase_number" # Research / brief artifacts (deterministic listing) gsd_run query phase.list-artifacts "$phase_number" --type research @@ -735,6 +738,9 @@ done Use `gsd-tools query` to validate plan structure: ```bash +# #2962: zsh aborts the block on an unmatched for-list glob (nomatch); bash passes it through. nullglob both. +shopt -s nullglob 2>/dev/null; setopt NULL_GLOB 2>/dev/null + for plan in "$PHASE_DIR"/*-PLAN.md; do echo "=== $plan ===" PLAN_STRUCTURE=$(gsd_run query verify.plan-structure "$plan") @@ -820,6 +826,9 @@ Inspect `tasks` in the JSON; open the PLAN in the editor for prose-level review. ## Step 6: Verify Dependency Graph ```bash +# #2962: zsh aborts the block on an unmatched for-list glob (nomatch); bash passes it through. nullglob both. +shopt -s nullglob 2>/dev/null; setopt NULL_GLOB 2>/dev/null + for plan in "$PHASE_DIR"/*-PLAN.md; do grep "depends_on:" "$plan" done diff --git a/gsd-core/workflows/audit-milestone.md b/gsd-core/workflows/audit-milestone.md index 0f64a9f6c..4b919ee8c 100644 --- a/gsd-core/workflows/audit-milestone.md +++ b/gsd-core/workflows/audit-milestone.md @@ -124,6 +124,9 @@ For each phase's VERIFICATION.md, extract the expanded requirements table: For each phase's SUMMARY.md, extract `requirements-completed` from YAML frontmatter: ```bash +# #2962: zsh aborts the block on an unmatched for-list glob (nomatch); bash passes it through. nullglob both. +shopt -s nullglob 2>/dev/null; setopt NULL_GLOB 2>/dev/null + for summary in .planning/phases/*-*/*-SUMMARY.md; do [ -e "$summary" ] || continue gsd_run query summary-extract "$summary" --fields requirements_completed --pick requirements_completed diff --git a/gsd-core/workflows/complete-milestone.md b/gsd-core/workflows/complete-milestone.md index 34df50397..6c58d3b4e 100644 --- a/gsd-core/workflows/complete-milestone.md +++ b/gsd-core/workflows/complete-milestone.md @@ -219,6 +219,9 @@ Milestone Stats: Extract one-liners from SUMMARY.md files using summary-extract: ```bash +# #2962: zsh aborts the block on an unmatched for-list glob (nomatch); bash passes it through. nullglob both. +shopt -s nullglob 2>/dev/null; setopt NULL_GLOB 2>/dev/null + # For each phase in milestone, extract one-liner for summary in .planning/phases/*-*/*-SUMMARY.md; do [ -e "$summary" ] || continue diff --git a/gsd-core/workflows/execute-phase.md b/gsd-core/workflows/execute-phase.md index f40b0d78a..d9f324d68 100644 --- a/gsd-core/workflows/execute-phase.md +++ b/gsd-core/workflows/execute-phase.md @@ -1486,12 +1486,13 @@ Copy failure must NOT block phase completion. After `update_roadmap`, moves todos whose `resolves_phase` matches to `completed/`. ```bash +shopt -s nullglob 2>/dev/null; setopt NULL_GLOB 2>/dev/null PHASE_NUM="${PHASE_NUMBER}" PENDING_DIR=".planning/todos/pending" COMPLETED_DIR=".planning/todos/completed" mkdir -p "$COMPLETED_DIR" -# "05"=="5" (#2576). +#2576 normalize_phase_num() { local p="${1//\"/}"; printf '%s' "$p" | sed 's/^0*\([0-9]\)/\1/' } @@ -1500,7 +1501,6 @@ PHASE_NUM_NORM=$(normalize_phase_num "$PHASE_NUM") CLOSED=() for TODO_FILE in "$PENDING_DIR"/*.md; do [ -f "$TODO_FILE" ] || continue - # resolves_phase from first frontmatter block RP=$(awk '/^---/{c++;next} c==1 && /^resolves_phase:/{print $2;exit} c==2{exit}' "$TODO_FILE" 2>/dev/null || true) RP_NORM=$(normalize_phase_num "$RP") if [ -n "$RP_NORM" ] && [ "$RP_NORM" = "$PHASE_NUM_NORM" ]; then diff --git a/gsd-core/workflows/resume-project.md b/gsd-core/workflows/resume-project.md index 4a2ca3fdb..d00719c46 100644 --- a/gsd-core/workflows/resume-project.md +++ b/gsd-core/workflows/resume-project.md @@ -64,6 +64,9 @@ cat .planning/PROJECT.md Look for incomplete work that needs attention: ```bash +# #2962: zsh aborts the block on an unmatched for-list glob (nomatch); bash passes it through. nullglob both. +shopt -s nullglob 2>/dev/null; setopt NULL_GLOB 2>/dev/null + # Check for structured handoff (preferred — machine-readable) cat .planning/HANDOFF.json 2>/dev/null || true diff --git a/gsd-core/workflows/review.md b/gsd-core/workflows/review.md index 51e2ecbff..75cc7f0b3 100644 --- a/gsd-core/workflows/review.md +++ b/gsd-core/workflows/review.md @@ -242,6 +242,9 @@ Write to a temp file: `{run_dir}/gsd-review-prompt.md` Also write individual section files so the budget tool can re-trim per reviewer: ```bash +# #2962: zsh aborts the block on an unmatched for-list glob (nomatch); bash passes it through. nullglob both. +shopt -s nullglob 2>/dev/null; setopt NULL_GLOB 2>/dev/null + RUN_DIR="{run_dir}" # from gather_context # Write individual section files for per-reviewer budget trimming @@ -312,6 +315,9 @@ If `section_manifest` is `null` or `"reviewer-instances-note-2"` is in its `incl Lanes run **sequentially, not in parallel** — concurrent invocation trips provider rate limits. ```bash +# #2962: zsh aborts the block on an unmatched for-list glob (nomatch); bash passes it through. nullglob both. +shopt -s nullglob 2>/dev/null; setopt NULL_GLOB 2>/dev/null + RUN_DIR="{run_dir}" REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)" # SELECTED_REVIEWERS is the comma-separated result of reviewer selection (ADR-0011 precedence: diff --git a/gsd-core/workflows/verify-phase.md b/gsd-core/workflows/verify-phase.md index c93e1034f..a4ac515e2 100644 --- a/gsd-core/workflows/verify-phase.md +++ b/gsd-core/workflows/verify-phase.md @@ -57,6 +57,7 @@ Extract **phase goal** from ROADMAP.md (the outcome to verify, not tasks), **req Use `gsd-tools.cjs query` verify handlers (or legacy gsd-tools) to extract must_haves from each PLAN: ```bash +shopt -s nullglob 2>/dev/null; setopt NULL_GLOB 2>/dev/null for plan in "$PHASE_DIR"/*-PLAN.md; do MUST_HAVES=$(gsd_run query frontmatter.get "$plan" --field must_haves) echo "=== $plan ===" && echo "$MUST_HAVES" @@ -126,6 +127,7 @@ For each truth: identify supporting artifacts → check artifact status → chec Use `gsd-tools.cjs query verify.artifacts` (or legacy gsd-tools) for artifact verification against must_haves in each PLAN: ```bash +shopt -s nullglob 2>/dev/null; setopt NULL_GLOB 2>/dev/null for plan in "$PHASE_DIR"/*-PLAN.md; do ARTIFACT_RESULT=$(gsd_run query verify.artifacts "$plan") echo "=== $plan ===" && echo "$ARTIFACT_RESULT" @@ -169,6 +171,7 @@ wiring or leftover code from plan revisions. Use `gsd-tools.cjs query verify.key-links` (or legacy gsd-tools) for key link verification against must_haves in each PLAN: ```bash +shopt -s nullglob 2>/dev/null; setopt NULL_GLOB 2>/dev/null for plan in "$PHASE_DIR"/*-PLAN.md; do LINKS_RESULT=$(gsd_run query verify.key-links "$plan") echo "=== $plan ===" && echo "$LINKS_RESULT" @@ -227,13 +230,7 @@ no `` block. ```bash GATE_CFG=$(gsd_run query config-get workflow.context_coverage_gate 2>/dev/null || echo "true") if [ "$GATE_CFG" != "false" ]; then - # Discover the phase CONTEXT.md via glob expansion rather than `ls | head` - # (review F17 / ShellCheck SC2012). Globs preserve filenames containing - # spaces and avoid an extra subprocess. - CONTEXT_PATH="" - for f in "${PHASE_DIR}"/*-CONTEXT.md; do - [ -e "$f" ] && CONTEXT_PATH="$f" && break - done + CONTEXT_PATH=$(ls "${PHASE_DIR}"/*-CONTEXT.md 2>/dev/null | head -1) # #2962: not a for-glob (zsh aborts) DECISION_RESULT=$(gsd_run query check.decision-coverage-verify "${PHASE_DIR}" "${CONTEXT_PATH}") fi ``` diff --git a/tests/emitted-drift-acks/2962-zsh-nomatch-for-glob-portability.json b/tests/emitted-drift-acks/2962-zsh-nomatch-for-glob-portability.json new file mode 100644 index 000000000..cda74a468 --- /dev/null +++ b/tests/emitted-drift-acks/2962-zsh-nomatch-for-glob-portability.json @@ -0,0 +1,10 @@ +{ + "version": 1, + "paths": { + "gsd-integration-checker.md": "#2962: 1 bash block (line ~98 SUMMARY iteration) gained the nullglob shim for zsh portability of the for-glob loop.", + "gsd-plan-checker.md": "#2962: 3 bash blocks (lines ~716, ~737, ~822) gained the nullglob shim for zsh portability of the for-glob loops.", + "resume-project.md": "#2962: 1 bash block (line ~66 plans-without-summaries scan) gained the nullglob shim for zsh portability of the for-glob loop.", + "complete-milestone.md": "#2962: 1 bash block (line ~221 summary one-liner extraction) gained the nullglob shim for zsh portability of the for-glob loop.", + "audit-milestone.md": "#2962: 1 bash block (line ~126 requirements_completed extraction) gained the nullglob shim for zsh portability of the for-glob loop." + } +} diff --git a/tests/emitted-drift-acks/2994-fragmentize-review-and-discuss-phase-assumptions.json b/tests/emitted-drift-acks/2994-fragmentize-review-and-discuss-phase-assumptions.json index e1991579f..74b641bed 100644 --- a/tests/emitted-drift-acks/2994-fragmentize-review-and-discuss-phase-assumptions.json +++ b/tests/emitted-drift-acks/2994-fragmentize-review-and-discuss-phase-assumptions.json @@ -1,6 +1,6 @@ { "version": 1, "paths": { - "review.md": "#2994 (epic #1671 Phase 6.3, further amendment): fragmentizes review.md onto the marker grammar, admitting `state:reviewer-instances-configured` (shared by two peripheral notes, `reviewer-instances-note-1` in detect_clis and `reviewer-instances-note-2` in invoke_reviewers — the core reviewer-lane dispatch itself stays unmarked). Also retargets the init line from the shared `init.phase-op` to a new dedicated `init.review` entry point (cmdInitReview), which computes the reviewer-instances-configured fact via `review.reviewer_instances` config presence. Net SOURCE growth is +55 bytes (29,063 -> 29,118): two `` marker-pair stubs (~331 B and ~195 B) replace their extracted prose bodies (328 B and 277 B respectively, now living in gsd-core/workflows/review/steps/*.md), and `init.phase-op` shrinks by 2 bytes to `init.review` on the gather_context init line. The EMITTED artifact composeWorkflow produces at install time still includes the extracted prose verbatim (markers strip, gap+section bodies re-join byte-for-byte) when the atom is unresolved (section_manifest null -> read-everything fallback), so installed behavior is unchanged; only the SOURCE file's on-disk byte count moves." + "review.md": "#2994 (epic #1671 Phase 6.3, further amendment): fragmentizes review.md onto the marker grammar, admitting `state:reviewer-instances-configured` (shared by two peripheral notes, `reviewer-instances-note-1` in detect_clis and `reviewer-instances-note-2` in invoke_reviewers — the core reviewer-lane dispatch itself stays unmarked). Also retargets the init line from the shared `init.phase-op` to a new dedicated `init.review` entry point (cmdInitReview), which computes the reviewer-instances-configured fact via `review.reviewer_instances` config presence. Net SOURCE growth is +55 bytes (29,063 -> 29,118): two `` marker-pair stubs (~331 B and ~195 B) replace their extracted prose bodies (328 B and 277 B respectively, now living in gsd-core/workflows/review/steps/*.md), and `init.phase-op` shrinks by 2 bytes to `init.review` on the gather_context init line. The EMITTED artifact composeWorkflow produces at install time still includes the extracted prose verbatim (markers strip, gap+section bodies re-join byte-for-byte) when the atom is unresolved (section_manifest null -> read-everything fallback), so installed behavior is unchanged; only the SOURCE file's on-disk byte count moves. #2962 amendment: 2 bash blocks with for-glob loops (PLAN_FILE copy iteration ~line 254, RUN_DIR plan-file args ~line 327) gained a nullglob shim (shopt -s nullglob 2>/dev/null; setopt NULL_GLOB 2>/dev/null) so zsh no longer aborts the block on an unmatched for-list glob (+342 bytes)." } }