From 47aee0d488b7e9afba7edb88b5c27cc53274ac3b Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Wed, 9 Sep 2026 13:02:32 -0700 Subject: [PATCH 1/2] style: make install.sh shellcheck-clean at -S info and LICENSE.md MD041-clean MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `standards-check` runs `shellcheck -S info` and markdownlint over the whole repo, so these pre-existing findings fail the check on every PR regardless of what the PR touches. Clearing them in one commit is what lets the check pass on this PR (wave 3, dev-env#100). shellcheck: 8x SC2312 in install.sh, at lines 175, 277, 323, 393, 413, 520, 591 and 603. Each is a command substitution whose exit status was already discarded — `$(...)` inside `[[ ]]` or `echo`, and `< <(...)` process substitution, none of which propagate status under `set -euo pipefail`. Adding `|| true` inside the substitution documents that existing discard; it is SC2312's own suggested alternative and adds no `# shellcheck disable`. Deliberately NOT rewritten as a separate assignment (`out=$(cmd)` then use `"$out"`), which is the other way to silence SC2312, because that would change behavior in two ways: - `mapfile -t x <<< "$out"` yields a 1-element array containing "" on empty input, where `< <(cmd)` yields an empty array. read_manifest and parse_mirrors_file can legitimately produce no output. - a bare assignment is subject to `set -e`, so a failing `uname` would exit instead of falling through to the `|| fatal` guard at line 175. Note parse_mirrors_file (349) and read_manifest (379) already end in `|| true` and cannot fail, so the four mapfile sites were never masking a real error. markdownlint: MD041 on LICENSE.md — added a `# MIT License` heading. Text of the license is unchanged. Verified: `standards/run-standards.sh --repo .` exits 1 before and 0 after (all six linters clean); `bash -n install.sh` passes; `install.sh --help` exits 0; the touched uname/readlink/date guards and the empty-input mapfile case were smoke-tested to behave identically. Claude-Session: https://claude.ai/code/session_01UaPoEix1iED8ENCZCa12jy --- LICENSE.md | 2 +- install.sh | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/LICENSE.md b/LICENSE.md index 35cd225..942e431 100644 --- a/LICENSE.md +++ b/LICENSE.md @@ -1,4 +1,4 @@ -MIT License +# MIT License Copyright (c) 2026 Andrew Rich diff --git a/install.sh b/install.sh index 681586a..30c8247 100755 --- a/install.sh +++ b/install.sh @@ -172,7 +172,7 @@ parse_args() { # Prerequisite checks # --------------------------------------------------------------------------- check_os() { - [[ "$(uname -s)" == "Darwin" ]] || fatal "This script only supports macOS." + [[ "$(uname -s || true)" == "Darwin" ]] || fatal "This script only supports macOS." } check_root() { @@ -274,7 +274,7 @@ do_update_mirrors() { # Read current list for comparison if [[ -f "$LOCAL_MIRRORS_FILE" ]]; then - mapfile -t current_domains < <(parse_mirrors_file "$LOCAL_MIRRORS_FILE") + mapfile -t current_domains < <(parse_mirrors_file "$LOCAL_MIRRORS_FILE" || true) else current_domains=() fi @@ -320,7 +320,7 @@ do_update_mirrors() { echo "# The first non-comment line is the primary domain (others become symlinks)." echo "#" echo "# Source: https://en.wikipedia.org/wiki/Archive.today" - echo "# Updated: $(date -u '+%Y-%m-%dT%H:%M:%SZ')" + echo "# Updated: $(date -u '+%Y-%m-%dT%H:%M:%SZ' || true)" printf '%s\n' "${new_domains[@]}" } >"$LOCAL_MIRRORS_FILE" @@ -390,7 +390,7 @@ write_manifest() { fi { echo "# ${MANAGED_MARKER}" - echo "# Updated: $(date -u '+%Y-%m-%dT%H:%M:%SZ')" + echo "# Updated: $(date -u '+%Y-%m-%dT%H:%M:%SZ' || true)" printf '%s\n' "${domains[@]}" } >"$MANIFEST_FILE" } @@ -410,7 +410,7 @@ is_managed() { is_correct_symlink() { local link="$1" target="$2" - [[ -L "$link" ]] && [[ "$(readlink "$link")" == "$target" ]] + [[ -L "$link" ]] && [[ "$(readlink "$link" || true)" == "$target" ]] } primary_is_current() { @@ -517,7 +517,7 @@ apply_removals() { do_uninstall() { info "Uninstalling ${SCRIPT_NAME}..." local -a managed - mapfile -t managed < <(read_manifest) + mapfile -t managed < <(read_manifest || true) if [[ ${#managed[@]} -eq 0 ]]; then warn "No managed resolver files found (manifest missing or empty)." @@ -588,7 +588,7 @@ main() { check_resolver_dir - mapfile -t DESIRED_DOMAINS < <(get_desired_mirrors) + mapfile -t DESIRED_DOMAINS < <(get_desired_mirrors || true) if [[ ${#DESIRED_DOMAINS[@]} -eq 0 ]]; then fatal "Mirror list is empty. Cannot continue." @@ -600,7 +600,7 @@ main() { info "Mirrors total : ${#DESIRED_DOMAINS[@]}" echo >&2 - mapfile -t MANIFEST_DOMAINS < <(read_manifest) + mapfile -t MANIFEST_DOMAINS < <(read_manifest || true) local install_changes removal_changes total_changes install_changes="$(apply_install "${DESIRED_DOMAINS[@]}")" From ca1a3818ff76db90bde73e2d2a1ea2dc4d54acec Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Wed, 9 Sep 2026 13:07:23 -0700 Subject: [PATCH 2/2] docs: explain why the SC2312 `|| true` guards are inert at four call sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit added `|| true` inside four `mapfile -t x < <(...)` substitutions to satisfy SC2312. Read at the call site alone, that pattern looks like it suppresses a real error — the local full-diff reviewer read it that way and flagged it as a data-integrity regression in do_uninstall. It is not one, but nothing at the call site says so. These comments record the two facts a reader needs: - the callee cannot fail. parse_mirrors_file and read_manifest each end in `|| true`; get_desired_mirrors either prints or calls fatal. - process substitution never propagates exit status under `set -e` anyway, so `< <(cmd)` and `< <(cmd || true)` are identical. Verified directly: `set -euo pipefail; mapfile -t x < <(false; echo hi)` does not abort. No behavior change; comments only. `run-standards.sh --repo .` still exits 0. Claude-Session: https://claude.ai/code/session_01UaPoEix1iED8ENCZCa12jy --- install.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/install.sh b/install.sh index 30c8247..79f1089 100755 --- a/install.sh +++ b/install.sh @@ -274,6 +274,8 @@ do_update_mirrors() { # Read current list for comparison if [[ -f "$LOCAL_MIRRORS_FILE" ]]; then + # parse_mirrors_file always exits 0 (its pipeline ends in `|| true`). The `|| true` below satisfies SC2312 and changes + # nothing: process substitution never propagates status under `set -e`. mapfile -t current_domains < <(parse_mirrors_file "$LOCAL_MIRRORS_FILE" || true) else current_domains=() @@ -517,6 +519,8 @@ apply_removals() { do_uninstall() { info "Uninstalling ${SCRIPT_NAME}..." local -a managed + # read_manifest always exits 0 (its grep ends in `|| true`). The `|| true` below satisfies SC2312 and changes + # nothing: process substitution never propagates status under `set -e`. mapfile -t managed < <(read_manifest || true) if [[ ${#managed[@]} -eq 0 ]]; then @@ -588,6 +592,9 @@ main() { check_resolver_dir + # get_desired_mirrors either prints or calls fatal; it never returns + # nonzero. The `|| true` below satisfies SC2312 and changes + # nothing: process substitution never propagates status under `set -e`. mapfile -t DESIRED_DOMAINS < <(get_desired_mirrors || true) if [[ ${#DESIRED_DOMAINS[@]} -eq 0 ]]; then @@ -600,6 +607,8 @@ main() { info "Mirrors total : ${#DESIRED_DOMAINS[@]}" echo >&2 + # read_manifest always exits 0 (its grep ends in `|| true`). The `|| true` below satisfies SC2312 and changes + # nothing: process substitution never propagates status under `set -e`. mapfile -t MANIFEST_DOMAINS < <(read_manifest || true) local install_changes removal_changes total_changes