fix(#4099): stop word-splitting an unquoted PLAN_IDS string (bash vs zsh diverge) (#4102)

* fix(review): stop word-splitting an unquoted PLAN_IDS string (bash vs zsh diverge)

#3301's plan-coverage-manifest block built a space-separated PLAN_IDS
accumulator and re-split it via unquoted `for x in $PLAN_IDS`. Bash
word-splits an unquoted scalar on IFS by default; zsh does not, so every
plan id collapsed onto one iteration under zsh whenever a review had 2+
plans, undercounting the manifest and merging bullets onto one line. This
is exactly what was failing origin/next's CI on every push.

Derive PLAN_COUNT and the bullet list directly from the same *-PLAN.md
glob loop used for copying — glob iteration, unlike re-splitting a stored
string, is identical under bash and zsh, so it never needs word-splitting.

Closes #4099

Emitted-Drift-Ack-Growth: review.md — fix comment documents the bash/zsh word-splitting divergence (gsd-core#4099); net code is smaller, comment expansion grows the file

* docs(#4099): add changeset for the plan-coverage-manifest zsh fix

* docs(#4099): fix changeset body to match the bold-dash-explanation format

Code review (standards axis) flagged the changeset body as not matching
CONTRIBUTING.md's canonical `**<Bold>** — <explanation>.` format — the bold
clause had its own trailing period instead of being followed directly by
the em-dash separator.

* chore(#4099): backfill changeset PR number (#4102)

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-08-31 11:08:22 -04:00
committed by GitHub
parent 6beaa66b25
commit 996196fe02
2 changed files with 20 additions and 11 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 4102
---
**Plan-coverage manifest miscounted multi-plan reviews under zsh** — the count and bullet list were derived by re-splitting an unquoted string, which bash word-splits by default but zsh does not, so reviews with 2+ plans collapsed onto one manifest entry. (#4099)

View File

@@ -277,17 +277,23 @@ done
# in-phase sequence number ("01") and can never reconstruct the phase-qualified
# id reviewers need to cite. The filename is guaranteed present for every copied
# plan, so no plan is ever dropped from the manifest for lacking a key.
# A plain string accumulator, not an array: zsh and bash disagree on array
# indexing and this block runs under both (see the nullglob/NULL_GLOB pairing
# above).
PLAN_IDS=""
# Count and bullets are both derived directly from the glob loop below, never
# from re-splitting an accumulated string: bash word-splits an unquoted `$VAR`
# on IFS by default, but zsh does not (no `setopt SH_WORD_SPLIT` here), so a
# prior `PLAN_IDS="$PLAN_IDS $id"` + `for x in $PLAN_IDS` round-trip silently
# collapsed every id onto one iteration under zsh whenever there were 2+ plans
# (gsd-core#4099). Direct glob iteration (`for f in "${PHASE_DIR}"/*-PLAN.md`,
# same pattern as the copy loop above) is identical under both shells, so this
# never needs word-splitting at all.
PLAN_COUNT=0
PLAN_ID_BULLETS=""
for PLAN_FILE in "${PHASE_DIR}"/*-PLAN.md; do
PLAN_BASENAME=$(basename "$PLAN_FILE")
PLAN_IDS="$PLAN_IDS ${PLAN_BASENAME%-PLAN.md}"
PLAN_ID="${PLAN_BASENAME%-PLAN.md}"
PLAN_COUNT=$((PLAN_COUNT + 1))
PLAN_ID_BULLETS="${PLAN_ID_BULLETS}- ${PLAN_ID}
"
done
PLAN_IDS="${PLAN_IDS# }"
PLAN_COUNT=0
for _ in $PLAN_IDS; do PLAN_COUNT=$((PLAN_COUNT + 1)); done
# Named to avoid BOTH existing RUN_DIR globs: `gsd-review-*.md` (reviewer
# reports, invoke_reviewers) and `gsd-review-plan-*.md` (the plan copies just
@@ -300,9 +306,7 @@ for _ in $PLAN_IDS; do PLAN_COUNT=$((PLAN_COUNT + 1)); done
echo "Total plans in this review: ${PLAN_COUNT}"
echo ""
echo "Plan ids (give each one its own \`##\`-level section, headed verbatim):"
for PLAN_ID in $PLAN_IDS; do
echo "- ${PLAN_ID}"
done
printf '%s' "$PLAN_ID_BULLETS"
} > "${RUN_DIR}/.plans-manifest.md"
# Optional section files (only if content was included in the combined prompt)