From bdb3143fcd2822ace6aa0640fc746b093af8ece3 Mon Sep 17 00:00:00 2001 From: hotragn Date: Wed, 30 Sep 2026 05:02:50 -0400 Subject: [PATCH] fix(cli): an unknown option exits 1; convert.sh --help shows every option Both scripts handled an unknown option with `error ...; usage`, and usage() always ended in `exit 0`: $ ./scripts/convert.sh --tol codex; echo $? [ERR] Unknown option: --tol ...usage... 0 so a mistyped flag in CI or a wrapper script read as success. usage() now takes a status. --help still prints to stdout and exits 0; an unknown option prints the usage text to stderr and exits 1, the same as a bad --tool or --division value already does. convert.sh --help also printed a hard-coded `sed -n '3,28p'`. The option notes live on lines 33-34, so --parallel and --jobs never appeared, and --out was only in the synopsis. It now uses USAGE-START/END sentinels, as install.sh has for a while, and the option list covers --tool and --out. test-cli-usage.sh checks both scripts: an unknown option exits non-zero with its name and the usage on stderr and nothing on stdout, --help exits 0 without printing the sentinels, and convert.sh --help describes --tool, --out, --parallel and --jobs. It fails on main at the first check and runs in check-tools.yml. --- .github/workflows/check-tools.yml | 3 +++ scripts/convert.sh | 20 +++++++++++++++--- scripts/install.sh | 13 ++++++++---- scripts/test-cli-usage.sh | 35 +++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 7 deletions(-) create mode 100755 scripts/test-cli-usage.sh diff --git a/.github/workflows/check-tools.yml b/.github/workflows/check-tools.yml index bd4bc2a0..87e584cd 100644 --- a/.github/workflows/check-tools.yml +++ b/.github/workflows/check-tools.yml @@ -56,3 +56,6 @@ jobs: - name: Reject unclosed agent frontmatter run: bash scripts/test-frontmatter-closing.sh + + - name: Unknown options fail; --help lists every option + run: bash scripts/test-cli-usage.sh diff --git a/scripts/convert.sh b/scripts/convert.sh index 6590559d..1f7156fc 100755 --- a/scripts/convert.sh +++ b/scripts/convert.sh @@ -6,6 +6,7 @@ # converted files to integrations//. Run this to regenerate all # integration files after adding or modifying agents. # +# --- USAGE-START --- (sentinel for usage(); do not remove) # Usage: # ./scripts/convert.sh [--tool ] [--out ] [--parallel] [--jobs N] [--help] # @@ -30,8 +31,12 @@ # Output is written to integrations// relative to the repo root. # This script never touches user config dirs — see install.sh for that. # +# --tool Convert for one tool (default: all). +# --out Write to // instead of integrations//. # --parallel When tool is 'all', run independent tools in parallel (output order may vary). # --jobs N Max parallel jobs when using --parallel (default: nproc or 4). +# +# --- USAGE-END --- (sentinel for usage(); do not remove) set -euo pipefail @@ -77,9 +82,18 @@ AGENT_DIRS=( ) # --- Usage --- +# usage [status] — print the header between the USAGE sentinels and exit. +# `--help` exits 0 on stdout; an unknown option exits 1 with the text on +# stderr. The old hard-coded `sed -n '3,28p'` stopped above --parallel, +# --jobs and --out, and exiting 0 after "Unknown option" meant a mistyped +# flag in CI or a wrapper script read as success. usage() { - sed -n '3,28p' "$0" | sed 's/^# \{0,1\}//' - exit 0 + local status="${1:-0}" + local text + text="$(sed -n '/^# --- USAGE-START ---/,/^# --- USAGE-END ---/p' "$0" \ + | sed -e '1d;$d' -e 's/^# \{0,1\}//')" + if (( status == 0 )); then printf '%s\n' "$text"; else printf '%s\n' "$text" >&2; fi + exit "$status" } # Default parallel job count (nproc on Linux; sysctl on macOS when nproc missing) @@ -793,7 +807,7 @@ main() { --parallel) use_parallel=true; shift ;; --jobs) parallel_jobs="${2:?'--jobs requires a value'}"; shift 2 ;; --help|-h) usage ;; - *) error "Unknown option: $1"; usage ;; + *) error "Unknown option: $1"; usage 1 ;; esac done diff --git a/scripts/install.sh b/scripts/install.sh index f8ccb7fa..7898a4fb 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -456,9 +456,14 @@ usage() { # (excluding the sentinel lines themselves) and strip the leading "# ". # Using sentinels instead of hard-coded line numbers means adding lines # to the header comment block won't silently break --help output. - sed -n '/^# --- USAGE-START ---/,/^# --- USAGE-END ---/p' "$0" \ - | sed -e '1d;$d' -e 's/^# \{0,1\}//' - exit 0 + # An unknown option passes 1: the text goes to stderr and the exit is + # non-zero, so a mistyped flag in CI or a wrapper script is not a success. + local status="${1:-0}" + local text + text="$(sed -n '/^# --- USAGE-START ---/,/^# --- USAGE-END ---/p' "$0" \ + | sed -e '1d;$d' -e 's/^# \{0,1\}//')" + if (( status == 0 )); then printf '%s\n' "$text"; else printf '%s\n' "$text" >&2; fi + exit "$status" } # Default parallel job count (nproc on Linux; sysctl on macOS when nproc missing) @@ -1553,7 +1558,7 @@ main() { --parallel) use_parallel=true; shift ;; --jobs) parallel_jobs="${2:?'--jobs requires a value'}"; shift 2 ;; --help|-h) usage ;; - *) err "Unknown option: $1"; usage ;; + *) err "Unknown option: $1"; usage 1 ;; esac done diff --git a/scripts/test-cli-usage.sh b/scripts/test-cli-usage.sh new file mode 100755 index 00000000..739d0413 --- /dev/null +++ b/scripts/test-cli-usage.sh @@ -0,0 +1,35 @@ +#!/usr/bin/env bash +# convert.sh and install.sh: --help documents every option and exits 0; an +# unknown option exits non-zero with the usage text on stderr. +# +# Both scripts used to call usage() after "Unknown option", and usage() always +# exited 0, so `convert.sh --tol codex` in CI or a wrapper read as success. +# convert.sh's --help also printed a hard-coded line range that stopped above +# --parallel, --jobs and --out. +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +tmp="$(mktemp -d "${TMPDIR:-/tmp}/agency-cli-usage.XXXXXX")" +trap 'rm -rf "$tmp"' EXIT + +fail() { echo "FAIL: $*" >&2; exit 1; } + +for script in convert.sh install.sh; do + status=0 + bash "$SCRIPT_DIR/$script" --no-such-flag > "$tmp/out" 2> "$tmp/err" || status=$? + [[ "$status" -ne 0 ]] || fail "$script --no-such-flag exited 0" + grep -q 'Unknown option: --no-such-flag' "$tmp/err" || fail "$script did not name the unknown option on stderr" + grep -q 'Usage:' "$tmp/err" || fail "$script did not print usage on stderr for an unknown option" + [[ ! -s "$tmp/out" ]] || fail "$script wrote to stdout for an unknown option" + + bash "$SCRIPT_DIR/$script" --help > "$tmp/help" 2>&1 || fail "$script --help exited non-zero" + grep -q 'Usage:' "$tmp/help" || fail "$script --help printed no usage" + ! grep -q 'USAGE-START\|USAGE-END' "$tmp/help" || fail "$script --help printed its sentinel lines" +done + +for opt in --tool --out --parallel --jobs; do + grep -q -- "^ $opt " <(bash "$SCRIPT_DIR/convert.sh" --help) \ + || fail "convert.sh --help does not describe $opt" +done + +echo "PASS: unknown options exit non-zero with usage on stderr; --help documents every convert.sh option"