diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 19a4f69..59957cb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,3 +31,20 @@ jobs: - name: self-tests under bash 3.2 if: runner.os == 'macOS' run: ADW_TEST_SHELL=/bin/bash bash tests/run.sh + + lint: + name: shellcheck + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + # Fail loudly if the runner image ever stops shipping it, rather than skipping the whole job. + - name: shellcheck version + run: shellcheck --version + + # -S warning is the floor the codebase currently holds at: 0 errors, 0 warnings. Below it sit 58 + # info and 58 style findings — mostly SC1091 (shellcheck cannot follow `source "$here/lib.sh"`) + # and SC2086 on expansions that are split on purpose. Those need a decision per site, so they are + # backlog, not a silenced rule. + - name: shellcheck -S warning + run: shellcheck -S warning -f gcc scripts/*.sh install.sh tests/run.sh tests/lib/*.sh tests/cases/*.sh diff --git a/README.md b/README.md index bf0eaf9..c3b1b5f 100644 --- a/README.md +++ b/README.md @@ -140,7 +140,9 @@ bash tests/run.sh gate-plan # one ADW_TEST_SHELL=/bin/bash bash tests/run.sh # under macOS's bash 3.2 ``` -CI runs both on Linux and macOS. A case may declare a **known gap** — the behaviour we want, asserted +CI runs both on Linux and macOS, plus `shellcheck -S warning` over every script (0 errors, 0 warnings; +the 58 info and 58 style findings below that floor are backlog, not a silenced rule). shellcheck is not +part of `tests/run.sh` — the suite needs only bash and git. A case may declare a **known gap** — the behaviour we want, asserted where the code does not deliver it yet. A known gap keeps the suite green and reports itself; the day it starts passing, the suite fails so the test gets promoted instead of quietly rotting. The current three are printed by every run, and `tests/run.sh` ends by naming the surface it does **not** cover diff --git a/install.sh b/install.sh index 08debab..1429929 100755 --- a/install.sh +++ b/install.sh @@ -33,7 +33,7 @@ set -uo pipefail SRC="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -VERSION="2.3.0" +VERSION="2.3.3" SCRIPT_FILES="lib.sh gate.sh guard.sh engines.sh intake.sh review.sh setup-worktree.sh worktree-alloc.sh" REGISTRY="${ADW_REGISTRY:-$HOME/.config/ai-tooling/installs}" @@ -195,7 +195,7 @@ render_stack_md() { info "keep existing: .tasks/_STACK.md (project-owned; --regen-stack to overwrite)"; return 0 fi if [[ -f "$out" && "$REGEN_STACK" == 1 ]]; then - local backup="$out.bak-$(date -u +%Y%m%dT%H%M%SZ)" + local backup; backup="$out.bak-$(date -u +%Y%m%dT%H%M%SZ)" [[ "$DRY_RUN" == 1 ]] || cp "$out" "$backup" info "backed up existing config → ${backup#$TARGET/}" fi @@ -359,10 +359,10 @@ cmd_install() { local installed=() IFS=',' read -ra skill_list <<< "$SKILLS" IFS=',' read -ra tool_list <<< "$TOOLS" - for id in "${skill_list[@]}"; do + for id in ${skill_list[@]+"${skill_list[@]}"}; do id="$(echo "$id" | tr -d ' ')" [[ -d "$SRC/skills/core/$id" ]] || die "unknown skill: $id" - for tool in "${tool_list[@]}"; do + for tool in ${tool_list[@]+"${tool_list[@]}"}; do case "$tool" in claude) dest_root="$TARGET/.claude/skills/$id" ;; codex) dest_root="$TARGET/.codex/skills/$id" ;; @@ -398,7 +398,7 @@ cmd_install() { (( WITH_HOOKS )) && install_hooks if [[ "$DRY_RUN" != 1 ]]; then - write_manifest "${installed[@]}" + write_manifest ${installed[@]+"${installed[@]}"} register_install "$TARGET" fi @@ -412,7 +412,7 @@ cmd_install() { if (( ${#ignored[@]} )); then info "" info "IGNORED BY GIT — these paths were written but git will not see them:" - for p in "${ignored[@]}"; do info " $p ($(cd "$TARGET" && git check-ignore -v "$p" 2>/dev/null | cut -d: -f1-2))"; done + for p in ${ignored[@]+"${ignored[@]}"}; do info " $p ($(cd "$TARGET" && git check-ignore -v "$p" 2>/dev/null | cut -d: -f1-2))"; done info " Decide deliberately: commit the harness with 'git add -f ', or leave it untracked and" info " know that a fresh clone of this repo has no harness. Half-tracked is the bad outcome." fi @@ -496,10 +496,10 @@ PY local plan="" id tool a f rel IFS=',' read -ra _skills <<< "$SKILLS" IFS=',' read -ra _tools <<< "$TOOLS" - for id in "${_skills[@]}"; do + for id in ${_skills[@]+"${_skills[@]}"}; do id="$(printf '%s' "$id" | tr -d ' ')" [[ -d "$SRC/skills/core/$id" ]] || { info " GONE skill '$id' no longer exists in the source — left in place"; continue; } - for tool in "${_tools[@]}"; do + for tool in ${_tools[@]+"${_tools[@]}"}; do case "$tool" in claude) rel=".claude/skills/$id" ;; codex) rel=".codex/skills/$id" ;; *) continue ;; esac while IFS= read -r f; do plan="$plan$SRC/skills/core/$id/$f $rel/$f "; done < <(cd "$SRC/skills/core/$id" && find . -type f | sed 's|^\./||') @@ -560,7 +560,7 @@ $SRC/templates/GROOM_LOG.md .tasks/_templates/GROOM_LOG.md (( WITH_HOOKS )) && install_hooks IFS=',' read -ra _installed <<< "$SKILLS" - write_manifest "${_installed[@]}" + write_manifest ${_installed[@]+"${_installed[@]}"} register_install "$TARGET" info " manifest updated → .ai-tooling.json (version $VERSION)" # Files left behind means the upgrade is not fully applied; --force leaves nothing behind. diff --git a/scripts/engines.sh b/scripts/engines.sh index 988d38e..1c508b4 100755 --- a/scripts/engines.sh +++ b/scripts/engines.sh @@ -106,9 +106,9 @@ probe() { done # keep only the pinned entries when at least two answered: they are strictly more informative local pinned=(); local e - for e in "${usable[@]}"; do [[ "$e" == *:* ]] && pinned+=("$e"); done + for e in ${usable[@]+"${usable[@]}"}; do [[ "$e" == *:* ]] && pinned+=("$e"); done if (( ${#pinned[@]} >= 2 )); then - usable=("${pinned[@]}") + usable=(${pinned[@]+"${pinned[@]}"}) adw_log "single vendor → pinned models probed and recorded (CROSS-MODEL, not DEGRADED)" fi fi @@ -117,7 +117,7 @@ probe() { adw_warn "no usable engine. Reviews cannot run until at least one CLI is authenticated." return 1 fi - local list; list="$(IFS=,; printf '%s' "${usable[*]}")" + local list; list="$(IFS=,; printf '%s' "${usable[*]-}")" printf '%s\n' "$list" if (( write )); then diff --git a/scripts/gate.sh b/scripts/gate.sh index 03978de..979a151 100644 --- a/scripts/gate.sh +++ b/scripts/gate.sh @@ -25,7 +25,7 @@ here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "$here/lib.sh" root="$(adw_repo_root)" || adw_die "not a git repo" -cd "$root" +cd "$root" || adw_die "cannot enter the repository root: $root" cmd="${1:-}"; shift || true @@ -83,7 +83,7 @@ gate_test() { adw_run_cmd test "$(test_command "${1:-}")" TEST_CMD; } # BASE branch catches the second: a test that is already green without the feature asserts nothing, and # the implementer will happily write code "to satisfy" it. gate_red_against_base() { - local target="${1:-}" base cmd tmp rc out changed + local target="${1:-}" base cmd tmp tmp_parent rc out changed base="$(adw_cfg BASE_BRANCH main)" git rev-parse --verify "$base" >/dev/null 2>&1 || base="origin/$base" if ! git rev-parse --verify "$base" >/dev/null 2>&1; then @@ -99,7 +99,12 @@ gate_red_against_base() { fi cmd="$(test_command "$target")" - tmp="$(mktemp -d)/base" + # `git worktree remove` takes the worktree away but not the mktemp -d that holds it. Removing the + # parent on every exit path — including the ones that return early below — is why this is a trap + # rather than an rm before each return. + tmp_parent="$(mktemp -d)" + trap 'rm -rf "${tmp_parent:?}"' RETURN + tmp="$tmp_parent/base" adw_log "RED/base: running the new tests against '$base' — they must fail there too" if ! git worktree add --detach -q "$tmp" "$base" 2>/dev/null; then adw_warn "RED/base: could not create a scratch worktree — skipping"; return 0 @@ -328,7 +333,9 @@ gate_workspace() { | python3 -c 'import json,sys; print(json.load(sys.stdin).get("status",""))' 2>/dev/null)" if [[ -z "${have// }" ]]; then adw_warn "G3: could not read the ticket state for $ref — move it by hand if it is still open." - elif [[ -n "${want// }" && "${have,,}" != "${want,,}" ]]; then + # `tr`, not ${x,,}: bash 3.2 — the /bin/bash macOS ships — aborts the whole script on that + # expansion with "bad substitution", so the gate dies instead of returning a verdict. + elif [[ -n "${want// }" && "$(lower "$have")" != "$(lower "$want")" ]]; then adw_warn "G3: ticket $ref is '$have', not '$want'. Move it now so nobody picks up the same work:" adw_warn " bash scripts/ai/intake.sh writeback $ref --status start" fail=1 diff --git a/scripts/guard.sh b/scripts/guard.sh index c005104..f761170 100644 --- a/scripts/guard.sh +++ b/scripts/guard.sh @@ -19,7 +19,7 @@ here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "$here/lib.sh" root="$(adw_repo_root)" || adw_die "not a git repo" -cd "$root" +cd "$root" || adw_die "cannot enter the repository root: $root" role="${1:-}"; scope="${2:---branch}" [[ -n "$role" ]] || adw_die "usage: guard.sh [--branch|--staged|--head]" diff --git a/scripts/intake.sh b/scripts/intake.sh index 5de67e6..62d9ded 100644 --- a/scripts/intake.sh +++ b/scripts/intake.sh @@ -26,7 +26,7 @@ here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "$here/lib.sh" root="$(adw_repo_root)" || adw_die "not a git repo" -cd "$root" +cd "$root" || adw_die "cannot enter the repository root: $root" print_contract() { cat <<'EOF' @@ -290,7 +290,7 @@ writeback() { status) tmpl="$(adw_cfg INTAKE_STATUS_CMD)" ;; comment) tmpl="$(adw_cfg INTAKE_COMMENT_CMD)" ;; esac - [[ -n "${tmpl// }" ]] || { adw_warn "no INTAKE_${kind^^}_CMD configured → skipping"; return 0; } + [[ -n "${tmpl// }" ]] || { adw_warn "no INTAKE_$(upper "$kind")_CMD configured → skipping"; return 0; } local resolved="${tmpl///$ref}" cwd resolved="${resolved///$value}" @@ -303,7 +303,7 @@ states() { # show how intents resolve against this tracker — diagnostic, write local ref="${1:-}" [[ -n "$(adw_cfg INTAKE_STATES_CMD)" ]] || adw_die "no INTAKE_STATES_CMD in .tasks/_STACK.md" local intent - for intent in start review done; do + for intent in start review 'done'; do # quoted: bare `done` reads as the loop keyword printf ' %-8s → %s\n' "$intent" "$(resolve_state "$intent" "$ref")" done } diff --git a/scripts/lib.sh b/scripts/lib.sh index 8729360..7f831e5 100644 --- a/scripts/lib.sh +++ b/scripts/lib.sh @@ -28,6 +28,12 @@ adw_cfg_note() { | grep -m1 -E "^${key}=" | grep -oE '#[[:space:]].*$' | sed -E 's/^#[[:space:]]*//' } +# Case folding, portably. `${x,,}` and `${x^^}` need bash 4, and bash 3.2 does not merely ignore them: +# it aborts the script with "bad substitution", so a gate using one returns no verdict at all on the +# /bin/bash macOS ships. Everything here has to run on that shell. +lower() { printf '%s' "$1" | tr '[:upper:]' '[:lower:]'; } +upper() { printf '%s' "$1" | tr '[:lower:]' '[:upper:]'; } + adw_log() { printf '[adw] %s\n' "$*" >&2; } adw_warn() { printf '[adw] WARN: %s\n' "$*" >&2; } adw_die() { printf '[adw] ERROR: %s\n' "$*" >&2; exit 2; } diff --git a/scripts/review.sh b/scripts/review.sh index f7c6bd1..26dd9d9 100755 --- a/scripts/review.sh +++ b/scripts/review.sh @@ -17,7 +17,7 @@ here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "$here/lib.sh" root="$(adw_repo_root)" || adw_die "not a git repo" -cd "$root" +cd "$root" || adw_die "cannot enter the repository root: $root" round="${1:-1}"; shift || true validation=""; profile=""; lenses="" @@ -76,8 +76,15 @@ context_common=$'\n\n'"CONTEXT: branch '$branch' vs '$base' (review round $round [[ -n "$validation" ]] && context_common+=$'\n'"Validation criteria: $validation — re-run the checks you can (static + tests + READ-ONLY inspection). Never run destructive or shared-state-mutating checks. A check with no reproducible evidence is NOT passed." count=$(( ${#lens_list[@]} + wildcard )) -mapfile -t engines < <("$here/engines.sh" pick-review "$count") -mapfile -t usable < <("$here/engines.sh" list 2>/dev/null) +# `while read`, not mapfile: mapfile is bash 4+. On bash 3.2 it is simply not a command, and the next +# line reads ${engines[0]} under `set -u` and aborts — a review that dies before spawning a reviewer. +engines=(); while IFS= read -r _line; do engines+=("$_line"); done < <("$here/engines.sh" pick-review "$count") +usable=(); while IFS= read -r _line; do usable+=("$_line"); done < <("$here/engines.sh" list 2>/dev/null) + +# engines.sh writes its own diagnosis to stderr and exits, which leaves this array empty rather than +# unset. Saying so here beats letting an array subscript deliver the news: the operator otherwise gets +# the real reason followed by a bash "unbound variable", and on bash 3.2 the abort comes first. +(( ${#engines[@]} )) || adw_die "no review engine available (see the error above). Fix with: bash scripts/ai/engines.sh probe --write" # run_lens — retries once on a different usable engine when output is empty # (an unauthenticated or rate-limited CLI fails silently; a missing lens must not look like a clean lens). @@ -85,7 +92,7 @@ run_lens() { local engine="$1" out="$2" prompt="$3" e printf '%s' "$prompt" | "$here/engines.sh" run "$engine" > "$out" 2>/dev/null if [[ ! -s "$out" ]]; then - for e in "${usable[@]}"; do + for e in ${usable[@]+"${usable[@]}"}; do [[ "$e" == "$engine" ]] && continue adw_warn "engine '$engine' returned nothing for $(basename "$out" .md) → retrying on '$e'" printf '%s' "$prompt" | "$here/engines.sh" run "$e" > "$out" 2>/dev/null @@ -97,7 +104,7 @@ run_lens() { adw_log "round $round · profile $profile · ${#lens_list[@]} lens reviewer(s) + wildcard=$wildcard + judge=$judge · diversity=$diversity" pids=(); idx=0 -for lens in "${lens_list[@]}"; do +for lens in ${lens_list[@]+"${lens_list[@]}"}; do engine="${engines[$idx]}"; out="$outdir/lens-$lens.md" prompt="$(printf '%s\n%s\n%s\n' "$base_rubric" "$(lens_block "$lens")" "$context_common")" { @@ -110,7 +117,7 @@ done if (( wildcard )); then engine="${engines[$idx]}"; out="$outdir/lens-wildcard.md" prompt="$(printf '%s\n%s\n%s\n%s\n' "$base_rubric" "$(lens_block wildcard)" \ - "The other reviewers on this diff cover exactly these lenses: ${lens_list[*]}. Anything inside them is THEIR job — do not duplicate it." \ + "The other reviewers on this diff cover exactly these lenses: ${lens_list[*]-}. Anything inside them is THEIR job — do not duplicate it." \ "$context_common")" { used="$(run_lens "$engine" "$out" "$prompt")" @@ -119,13 +126,13 @@ if (( wildcard )); then pids+=($!) fi -for p in "${pids[@]}"; do wait "$p" || true; done +for p in ${pids[@]+"${pids[@]}"}; do wait "$p" || true; done findings_files=("$outdir"/lens-*.md) combined="$outdir/findings.md" { printf '# Review round %s — profile %s — diversity %s\n\n' "$round" "$profile" "$diversity" - for f in "${findings_files[@]}"; do + for f in ${findings_files[@]+"${findings_files[@]}"}; do [[ -s "$f" ]] || { adw_warn "empty output: $f"; continue; } printf '\n## %s\n\n' "$(basename "$f" .md)"; cat "$f" done @@ -154,7 +161,7 @@ if [[ ! -s "$final" ]]; then adw_die "no reviewer output (all engines failed)"; # Cost report — one row per engine call, plus a total. Reported even when partial. { printf '# Review round %s — cost\n\nprofile: %s · lenses: %s · wildcard: %s · judge: %s · diversity: %s\n' \ - "$round" "$profile" "${lens_list[*]}" "$wildcard" "$judge" "$diversity" + "$round" "$profile" "${lens_list[*]-}" "$wildcard" "$judge" "$diversity" printf 'wall clock: %ss\n\n' "$(( $(date -u +%s) - start_ts ))" if [[ -s "$ADW_USAGE_FILE" ]]; then printf '| call | input | output | cache |\n| --- | --- | --- | --- |\n' diff --git a/scripts/setup-worktree.sh b/scripts/setup-worktree.sh index 99beebb..c275cc0 100755 --- a/scripts/setup-worktree.sh +++ b/scripts/setup-worktree.sh @@ -15,7 +15,7 @@ here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "$here/lib.sh" root="$(adw_repo_root)" || adw_die "not a git repo" -cd "$root" +cd "$root" || adw_die "cannot enter the repository root: $root" common_git="$(git rev-parse --path-format=absolute --git-common-dir 2>/dev/null)" \ || adw_die "cannot resolve git common dir" @@ -56,7 +56,7 @@ if (( ${#excluded[@]} )); then excl="$common_git/worktrees/$(basename "$root")/info/exclude" [[ -d "$(dirname "$excl")" ]] || excl="$common_git/info/exclude" mkdir -p "$(dirname "$excl")" - for entry in "${excluded[@]}"; do + for entry in ${excluded[@]+"${excluded[@]}"}; do grep -qxF "$entry" "$excl" 2>/dev/null || printf '%s\n' "$entry" >> "$excl" done adw_log "updated git exclude: $excl" diff --git a/scripts/worktree-alloc.sh b/scripts/worktree-alloc.sh index d2c4ca7..86b1c46 100644 --- a/scripts/worktree-alloc.sh +++ b/scripts/worktree-alloc.sh @@ -16,7 +16,7 @@ here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "$here/lib.sh" root="$(adw_repo_root)" || adw_die "not a git repo" -cd "$root" +cd "$root" || adw_die "cannot enter the repository root: $root" # Index = position in `git worktree list` (stable while the worktree exists). index=0; n=0 diff --git a/tests/cases/gate-red.sh b/tests/cases/gate-red.sh index 350ae02..bdf1728 100755 --- a/tests/cases/gate-red.sh +++ b/tests/cases/gate-red.sh @@ -49,4 +49,26 @@ check_out 'and it names the offending test file' 'tests/probe.sh' gate "$taut" # The same run with the base check disabled passes — which is why the skip is reported. check 'the same tautology slips through --no-base' 0 gate "$taut" red --no-base +# The scratch worktree lives inside a mktemp -d. `git worktree remove` takes the worktree away and +# leaves the directory that held it, so every RED gate used to leave one behind for the life of the +# machine. +# +# TMPDIR is not usable to make this countable: BSD mktemp (macOS) ignores it and uses the per-user +# _CS_DARWIN_USER_TEMP_DIR, so a TMPDIR-based fixture would watch an empty directory and pass no matter +# what the code did — the same "skipped looks like passed" failure this suite exists to catch. So ask +# mktemp itself where it writes, and diff the tmp.* entries there across the run. +# +# Race, accepted: another process may create a tmp.* directory during the run. That direction produces a +# false FAILURE, never a false pass. +leaves_no_tmpdir() { + local probe parent before after leaked + probe="$(mktemp -d)"; parent="$(dirname "$probe")"; rmdir "$probe" + before="$(find "$parent" -maxdepth 1 -type d -name 'tmp.*' 2>/dev/null | LC_ALL=C sort)" + ( cd "$taut" && bash scripts/ai/gate.sh red >/dev/null 2>&1 ) + after="$(find "$parent" -maxdepth 1 -type d -name 'tmp.*' 2>/dev/null | LC_ALL=C sort)" + leaked="$(comm -13 <(printf '%s\n' "$before") <(printf '%s\n' "$after"))" + [[ -z "${leaked// }" ]] || { printf 'left behind in %s:\n%s\n' "$parent" "$leaked"; return 1; } +} +check 'the RED/base check cleans up its temp directory' 0 leaves_no_tmpdir + done_tests diff --git a/tests/cases/install-upgrade.sh b/tests/cases/install-upgrade.sh index 1665346..408b637 100644 --- a/tests/cases/install-upgrade.sh +++ b/tests/cases/install-upgrade.sh @@ -6,7 +6,7 @@ set -uo pipefail source "$(cd "$(dirname "${BASH_SOURCE[0]}")/../lib" && pwd)/harness.sh" # Never touch the operator's real registry. -export ADW_REGISTRY="$(tmpdir registry)/installs" +ADW_REGISTRY="$(tmpdir registry)/installs"; export ADW_REGISTRY target="$(fixture target)" || exit 1 rm -rf "$target/scripts" # this test installs the scripts itself diff --git a/tests/cases/portability.sh b/tests/cases/portability.sh index 7c711db..7b89543 100755 --- a/tests/cases/portability.sh +++ b/tests/cases/portability.sh @@ -18,18 +18,72 @@ if [[ -x /bin/bash ]]; then check 'every core script parses under /bin/bash' 0 parses_under /bin/bash fi -# Constructs that need bash 4+. Known offenders as of this commit: -# scripts/gate.sh:290 ${have,,} / ${want,,} — ticket-state comparison in G3 -# scripts/intake.sh:293 ${kind^^} — building the INTAKE__CMD name -# scripts/review.sh:76-77 mapfile — reading the engine list +# Two different strippings, because the two rules need different things. +# +# Comments only, for bash-4 expansions: `"${have,,}"` lives INSIDE double quotes almost every time it +# appears, so stripping quoted strings would blind the check to exactly what it is looking for. Verified +# by putting `${have,,}` back in gate.sh and watching a quote-stripping version report nothing. +code_lines() { sed -E 's/#.*$//' "$@"; } +# Comments and quoted strings, for tool names: naming a binary in a warning ("no shasum/sha256sum here") +# is not a use of it. Blind spot, accepted: a real call hidden in a quoted command string is invisible. +code_lines_bare() { sed -E 's/"[^"]*"//g; s/#.*$//' "$@"; } + +# Constructs that need bash 4+. The portable replacements: `lower`/`upper` in lib.sh for case folding, +# and `while IFS= read -r l; do a+=("$l"); done < <(cmd)` for mapfile. bash4_free() { local hits - hits="$(grep -nE 'mapfile|readarray|declare -A|\$\{[A-Za-z_][A-Za-z0-9_]*(,,|\^\^)' \ - "$ADW_ROOT"/scripts/*.sh "$ADW_ROOT"/install.sh)" || return 0 + hits="$(code_lines "$ADW_ROOT"/scripts/*.sh "$ADW_ROOT"/install.sh \ + | grep -nE 'mapfile|readarray|declare -A|\$\{[A-Za-z_][A-Za-z0-9_]*(,,|\^\^)')" || return 0 printf '%s\n' "$hits" return 1 } -check_xfail 'core scripts avoid bash-4-only constructs' 0 bash4_free +check 'core scripts avoid bash-4-only constructs' 0 bash4_free + +# The replacements themselves, exercised under the oldest shell rather than trusted. +folds_under_32() { + /bin/bash -c 'source "$1"/scripts/lib.sh + [[ "$(lower ABC-12)" == "abc-12" ]] || { echo "lower failed: $(lower ABC-12)"; exit 1; } + [[ "$(upper status)" == "STATUS" ]] || { echo "upper failed: $(upper status)"; exit 1; } + a=(); while IFS= read -r l; do a+=("$l"); done < <(printf "x\ny\n") + [[ "${#a[@]}" == 2 && "${a[1]}" == "y" ]] || { echo "array read failed"; exit 1; }' _ "$ADW_ROOT" +} +if [[ -x /bin/bash ]]; then + check 'lower/upper and the mapfile replacement work under /bin/bash' 0 folds_under_32 +fi + +# Bash 3.2 aborts on "${arr[@]}" and "${arr[*]}" when the array is EMPTY and set -u is on — not a +# warning, `unbound variable` and exit 127. Bash 4.4 made it legal, so this is invisible on Linux CI and +# on any machine with Homebrew bash, and the lines themselves look perfectly portable. Grep alone would +# have missed the class, which is why the runtime assertion below exists as well. +# +# The rule is blanket: every [@] expansion carries the `${arr[@]+"${arr[@]}"}` guard, every [*] carries +# `-`. Whether a particular array can really be empty is a judgement no static check can make, and one +# guard costs nothing. +guarded_arrays() { + local hits + # The guarded form CONTAINS the bare one — ${a[@]+"${a[@]}"} — so strip the whole guarded construct + # first and flag whatever bare expansion is left over. + hits="$(code_lines "$ADW_ROOT"/scripts/*.sh "$ADW_ROOT"/install.sh \ + | sed -E 's/\$\{[A-Za-z_][A-Za-z0-9_]*\[@\]\+"\$\{[A-Za-z_][A-Za-z0-9_]*\[@\]\}"\}//g' \ + | grep -nE '"\$\{[A-Za-z_][A-Za-z0-9_]*\[[@*]\]\}"')" || return 0 + printf '%s\n' "$hits" + return 1 +} +check 'array expansions are guarded for the empty case' 0 guarded_arrays + +empty_arrays_under_32() { + /bin/bash -c 'set -uo pipefail + a=() + for e in ${a[@]+"${a[@]}"}; do :; done + printf "%s" "${a[*]-}" >/dev/null + (( ${#a[@]} )) && exit 1 + b=(x y) + for e in ${b[@]+"${b[@]}"}; do :; done + [[ "$e" == y ]] || exit 1' +} +if [[ -x /bin/bash ]]; then + check 'the guarded forms survive an empty array under /bin/bash' 0 empty_arrays_under_32 +fi # GNU-only tools are the other half of the same problem: absent from a stock macOS, so a script that # reaches for one works on CI and dies on the operator's laptop. `timeout` is not in this list on @@ -41,7 +95,7 @@ check_xfail 'core scripts avoid bash-4-only constructs' 0 bash4_free # Blind spot, accepted: a genuine call hidden inside a double-quoted command string is invisible here. gnu_free() { local hits - hits="$(sed -E 's/"[^"]*"//g; s/#.*$//' "$ADW_ROOT"/scripts/*.sh "$ADW_ROOT"/install.sh \ + hits="$(code_lines_bare "$ADW_ROOT"/scripts/*.sh "$ADW_ROOT"/install.sh \ | grep -nE '(^|[^[:alnum:]_-])(realpath|sha256sum|tac|readlink -f|date -d|stat -c|grep -P|xargs -r)([^[:alnum:]_-]|$)' \ | grep -v 'command -v')" || return 0 printf '%s\n' "$hits" diff --git a/tests/run.sh b/tests/run.sh index 7c4b9d1..e354c89 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -44,6 +44,7 @@ not covered (needs a remote, gh, or a live engine CLI): gate.sh workspace · gate.sh ready · gate.sh committed (upstream branch) review.sh fan-out · engines.sh probe/run · intake.sh fetch/writeback install.sh: install + upgrade are covered; doctor, uninstall and hook registration are not + static analysis: shellcheck runs in CI, not here — this suite needs only bash and git NOTE if (( ${#failed[@]} )); then