mirror of
https://github.com/kuhyx/signal-bot.git
synced 2026-11-11 12:57:32 +01:00
fix: make shellcheck agree between local and CI, and fix the group picker
CI failed on shellcheck where the local hook passed: the runner's shellcheck reports SC2015 (`A && B || C` is not if-then-else) and mine did not. Both now run with --severity=style so a version skew cannot pass locally and fail in CI. The cleanup traps are plain ifs, which is what the finding was about -- `kill` on an already-dead pid makes B fail and C run while A is true. live_smoke.sh choose_group had two bugs that only surface when it is actually run, neither reachable from the offline suite: - it fetched /v1/groups twice, once to display and once to resolve the picked index, so an unstable order would hand back a different group than the one shown. Fetched once into a variable now. - `read -r -p` wrote its prompt to stdout, which the caller captures with $(...), so the prompt text would have been swallowed into the group id. Prompt and listing both go to stderr; stdout carries only the id. Also validates that the pick is a number and in range. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013yaBT45KhHqxTKVCTA87iy
This commit is contained in:
parent
6bde3493a4
commit
c9061b27a8
4
.github/workflows/ci.yml
vendored
4
.github/workflows/ci.yml
vendored
@ -36,7 +36,9 @@ jobs:
|
||||
run: .venv/bin/mypy signal_bot fake_signal
|
||||
|
||||
- name: shellcheck
|
||||
run: shellcheck run.sh scripts/*.sh
|
||||
# --severity=style is the floor CI enforces; the local pre-commit hook
|
||||
# uses the same flag so a version skew cannot pass locally and fail here.
|
||||
run: shellcheck --severity=style run.sh scripts/*.sh
|
||||
|
||||
- name: pytest (100% branch coverage)
|
||||
run: .venv/bin/python -m pytest -q
|
||||
|
||||
@ -77,7 +77,9 @@ repos:
|
||||
|
||||
- id: shellcheck
|
||||
name: shellcheck
|
||||
entry: shellcheck
|
||||
# Same severity floor as CI: a stricter runner version must not be
|
||||
# the thing that discovers a finding this hook let through.
|
||||
entry: shellcheck --severity=style
|
||||
language: system
|
||||
types: [shell]
|
||||
|
||||
|
||||
@ -28,11 +28,19 @@ BOT_PID=""
|
||||
TEMP_DIR=""
|
||||
|
||||
cleanup() {
|
||||
[[ -n "$BOT_PID" ]] && kill "$BOT_PID" 2>/dev/null || true
|
||||
[[ -n "$FAKE_PID" ]] && kill "$FAKE_PID" 2>/dev/null || true
|
||||
# Plain ifs, not `A && B || C`: that is not if-then-else, and C runs when
|
||||
# A is true but B fails -- which is exactly the case here (kill on an
|
||||
# already-dead pid).
|
||||
if [[ -n "$BOT_PID" ]]; then
|
||||
kill "$BOT_PID" 2>/dev/null || true
|
||||
fi
|
||||
if [[ -n "$FAKE_PID" ]]; then
|
||||
kill "$FAKE_PID" 2>/dev/null || true
|
||||
fi
|
||||
wait 2>/dev/null || true
|
||||
[[ -n "$TEMP_DIR" && -d "$TEMP_DIR" ]] && rm -rf "$TEMP_DIR"
|
||||
return 0
|
||||
if [[ -n "$TEMP_DIR" && -d "$TEMP_DIR" ]]; then
|
||||
rm -rf "$TEMP_DIR"
|
||||
fi
|
||||
}
|
||||
trap cleanup EXIT
|
||||
|
||||
|
||||
@ -30,8 +30,9 @@ readonly FALLBACK_IMAGE="${FALLBACK_IMAGE:-bbernhard/signal-cli-rest-api:1787779
|
||||
TEMP_DIR=""
|
||||
|
||||
cleanup() {
|
||||
[[ -n "$TEMP_DIR" && -d "$TEMP_DIR" ]] && rm -rf "$TEMP_DIR"
|
||||
return 0
|
||||
if [[ -n "$TEMP_DIR" && -d "$TEMP_DIR" ]]; then
|
||||
rm -rf "$TEMP_DIR"
|
||||
fi
|
||||
}
|
||||
trap cleanup EXIT
|
||||
|
||||
@ -106,15 +107,30 @@ wait_for_link() {
|
||||
}
|
||||
|
||||
choose_group() {
|
||||
local number="$1"
|
||||
local number="$1" groups choice id
|
||||
# Fetched ONCE: two separate calls could return a different order, so the
|
||||
# index the user picks would resolve against a different list.
|
||||
groups="$(curl -sf "${API}/v1/groups/${number}")"
|
||||
|
||||
# Every prompt goes to stderr: this function's stdout is captured by $(...)
|
||||
# and must contain nothing but the chosen group id.
|
||||
echo "== groups on this account:" >&2
|
||||
curl -sf "${API}/v1/groups/${number}" \
|
||||
| jq -r '.[] | "\(.id)\t\(.name // "(unnamed)")"' \
|
||||
printf '%s' "$groups" | jq -r '.[] | "\(.id)\t\(.name // "(unnamed)")"' \
|
||||
| nl -w3 -s' ' >&2
|
||||
echo >&2
|
||||
read -r -p "Pick the number of the PRIVATE group to use: " choice
|
||||
curl -sf "${API}/v1/groups/${number}" \
|
||||
| jq -r --argjson n "$choice" '.[$n - 1].id'
|
||||
|
||||
read -r -p "Pick the number of the PRIVATE group to use: " choice </dev/tty >&2
|
||||
if ! [[ "$choice" =~ ^[0-9]+$ ]]; then
|
||||
echo "Error: '$choice' is not a number" >&2
|
||||
return 1
|
||||
fi
|
||||
|
||||
id="$(printf '%s' "$groups" | jq -r --argjson n "$choice" '.[$n - 1].id // empty')"
|
||||
if [[ -z "$id" ]]; then
|
||||
echo "Error: no group at position $choice" >&2
|
||||
return 1
|
||||
fi
|
||||
printf '%s' "$id"
|
||||
}
|
||||
|
||||
write_env() {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user