fix: guard possibly-empty array expansions so bash 3.2 can't fake a reviewer cli-error

install.sh

@@ -60,7 +60,7 @@ OLD_COMMANDS=(cam good makegoals makegood optimize-md)
 # enumerates lib/ dynamically, so it doesn't need updating.
 LIBS=(
   ci-flake-handling code-review-checklist copilot-review-loop
-  enhance-loop epic-children
+  empty-array-expansion enhance-loop epic-children
   finding-disposition fix-regression-guard
   gh-host github-reviewer-loop graphql-escaping
   local-agent-review-loop multi-reviewer-loop ollama-review-loop

lib/empty-array-expansion.md

@@ -0,0 +1,41 @@
+## Expanding a possibly-empty shell array (`${ARR[@]+"${ARR[@]}"}`)
+
+**The rule.** Any array that can legitimately be empty — `TIMEOUT_CMD`, `MODEL_FLAG`,
+`OLLAMA_FLAGS`, or any other optional-argument array — must be expanded as:
+
+```bash
+${ARR[@]+"${ARR[@]}"}      # correct
+"${ARR[@]}"                # WRONG when ARR can be empty
+```
+
+**Why the guard is required, not decoration.** Under **bash 3.2** — still `/bin/bash`
+on macOS — a bare `"${ARR[@]}"` on an *empty* array is an **unset** expansion, so with
+`set -u` the shell aborts with `ARR[@]: unbound variable` **before the command runs**:
+
+```
+$ /bin/bash -c 'set -u; A=(); "${A[@]}" echo ok'
+/bin/bash: A[@]: unbound variable        # exit 127 — echo never ran
+
+$ /bin/bash -c 'set -u; A=(); ${A[@]+"${A[@]}"} echo ok'
+ok                                        # exit 0
+```
+
+The `${ARR[@]+…}` form suppresses the expansion entirely when the array is unset or
+empty, and is identical to `"${ARR[@]}"` when it is not — quoting and word boundaries
+are preserved, so an element containing spaces still arrives as one argument. It is
+safe on bash 3.2+, bash 4/5, and zsh alike, so it is the only form to write.
+
+**Why it matters here specifically.** Stock macOS ships **neither** `timeout(1)` (GNU
+coreutils) nor `gtimeout` (the Homebrew-prefixed coreutils build), so a probing
+`TIMEOUT_CMD=()` stays empty on a typical Mac — the *common* case, not an edge case.
+Running without a timeout wrapper is a **supported configuration** (the invocation is
+bounded by the CLI's own limits instead) and must never be recorded as a reviewer
+failure. But with an unguarded expansion the review loops fail in the worst possible
+way: every invocation aborts before the reviewer starts, returning RC=1 with empty
+output, so the loop counts each file as a review error and the pass resolves to
+`cli-error` — a hard error that `~opt` does **not** excuse — blocking the merge on a
+PR no reviewer ever looked at.
+
+**Diagnosing it.** Every file erroring with an empty response, and nothing but
+`unbound variable` in the captured stderr, is this bug — an environment condition, not
+a reviewer failure. Fix the expansion and re-run the pass; do not record `cli-error`.

lib/enhance-loop.md

@@ -103,11 +103,15 @@ Resolve the timeout wrapper — the only genuinely run-once piece; this is settl
 logic; run it, don't narrate it:
 
 ```bash
-# macOS ships no timeout(1) unless coreutils is installed; probe for it. Empty array =
-# no wrapper (rely on each CLI's own limits). An ARRAY, not a string: zsh (a common
-# host shell) does not word-split an unquoted expansion, so a two-word string like
-# 'timeout 1800' would be executed as one bogus command name; the array expands to
-# separate words in bash and zsh alike, and to zero words when empty. Enhancement is
+# Stock macOS ships NEITHER timeout(1) (GNU coreutils) nor gtimeout (Homebrew
+# coreutils), so probe for them and expect the empty array — the common case, not an
+# edge case. Empty array = no wrapper (rely on each CLI's own limits). An ARRAY, not a
+# string: zsh (a common host shell) does not word-split an unquoted expansion, so a
+# two-word string like 'timeout 1800' would be executed as one bogus command name; the
+# array expands to separate words in bash and zsh alike. Expand it (and MODEL_FLAG)
+# only in the guarded ${ARR[@]+"${ARR[@]}"} form — the bare form aborts under bash 3.2
+# + `set -u` before the CLI runs; see ~/.claude/lib/empty-array-expansion.md.
+# Enhancement is
 # lighter than a full review (no build/test), but a large draft on a heavy model can
 # still exceed the ~10-min host foreground cap, so the same background+poll launch
 # below is used.
@@ -151,11 +155,11 @@ as a positional argument (never via stdin) and prints the improved draft to stdo
 <!-- if:teams -->
 | `claude` | Dispatch an in-process sub-agent via the `Agent` tool (`subagent_type: "general-purpose"`, prompt `$ENHANCE_PROMPT`, `model` = `{ENH_MODEL}` when set) — **not** `claude -p`, so it stays on the host session's plan billing instead of hitting the API. Its returned message is the agent's stdout. |
 <!-- else -->
-| `claude` | `claude -p "$ENHANCE_PROMPT" "${MODEL_FLAG[@]}" --dangerously-skip-permissions` |
+| `claude` | `claude -p "$ENHANCE_PROMPT" ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} --dangerously-skip-permissions` |
 <!-- /if:teams -->
-| `codex` | `codex "${MODEL_FLAG[@]}" --sandbox read-only -a never exec "$ENHANCE_PROMPT"` — `exec` (free-form prompt) is the right subcommand here, not `codex review`; `-m`/`--model`, `--sandbox`, and `-a` are all top-level flags that MUST precede `exec`. `--sandbox read-only` enforces the read-only contract at the sandbox level while still allowing tree reads and git queries — the same posture `lib/local-agent-review-loop.md` uses for its review-only codex pass (only its *reviewer-applies* path needs `danger-full-access`). |
+| `codex` | `codex ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} --sandbox read-only -a never exec "$ENHANCE_PROMPT"` — `exec` (free-form prompt) is the right subcommand here, not `codex review`; `-m`/`--model`, `--sandbox`, and `-a` are all top-level flags that MUST precede `exec`. `--sandbox read-only` enforces the read-only contract at the sandbox level while still allowing tree reads and git queries — the same posture `lib/local-agent-review-loop.md` uses for its review-only codex pass (only its *reviewer-applies* path needs `danger-full-access`). |
 | `agy` | `agy --dangerously-skip-permissions --model "$AGY_ENH_MODEL" --print-timeout 30m -p "$ENHANCE_PROMPT"` |
-| `grok` | `grok --permission-mode bypassPermissions "${MODEL_FLAG[@]}" -p "$ENHANCE_PROMPT"` |
+| `grok` | `grok --permission-mode bypassPermissions ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} -p "$ENHANCE_PROMPT"` |
 
 **Grok flag rationale.** `grok -p`/`--single <PROMPT>` runs a single-turn headless
 prompt, prints the response to stdout, and exits — the grok analog of `claude -p` /
@@ -247,7 +251,7 @@ one's output):
      # after <<<ENHANCED_BODY>>> to end-of-output", so any stderr the CLI emits after
      # the answer (telemetry warnings, timing/shutdown lines, update nags) would be
      # pasted verbatim into the enhanced draft and end up in the filed issue.
-     "${TIMEOUT_CMD[@]}" {INVOCATION} > "$LOG_FILE" 2> "$ERR_FILE"; echo $? > "$DONE_FILE"
+     ${TIMEOUT_CMD[@]+"${TIMEOUT_CMD[@]}"} {INVOCATION} > "$LOG_FILE" 2> "$ERR_FILE"; echo $? > "$DONE_FILE"
      ```
      Then wait with **bounded blocking-chunk foreground calls** — do NOT end your turn
      to wait for a notification (a stopped subagent is dead, not waiting):