From 0a9588aea7a4208acd88ff2d4c38e23b9b209170 Mon Sep 17 00:00:00 2001 From: Michael Sitarzewski Date: Fri, 11 Sep 2026 22:33:23 -0500 Subject: [PATCH] fix(setup-ai): harden AI bootstrap against real-world failure modes Follow-up to #12. The bootstrap script works on a good network with a simple .env; these are the paths where it didn't. Interrupted model downloads were cached as success. The 1.5 GB model was written straight to its final path, so a dropped transfer left a truncated file that every later run accepted as valid. Worse, capabilities.js detects the model with a bare existsSync, so the UI un-gated Transcribe over a corrupt model -- exactly the failure v0.3.2 gating exists to prevent. Downloads now land in models/ggml-medium.bin.part, are size-checked against a floor, and only then renamed into place. Also dropped curl --silent; a multi-minute silent download reads as a hung script. .env was sourced as shell. Under set -euo pipefail a JWT_SECRET containing $ aborted the script with "unbound variable" and a value with spaces ran as a command. Only the three LLM_* keys are read now, via sed, leaving every other secret untouched. whisper.cpp was a tracked 160000 gitlink with no .gitmodules backing, so the new .gitignore rule could never apply to it -- git does not ignore tracked paths. Anyone who ran setup-ai.sh got a permanently dirty working tree showing "modified: whisper.cpp (new commits)". Untracked it, which is what the setup script already assumed. Missing ffmpeg aborted the entire bootstrap, though neither ffmpeg nor ffprobe is needed to clone whisper.cpp, build it, or fetch the model. Now warns and continues. Tests: kept the fake-toolchain harness from #12 and added a regression case per bug. Both new cases were verified to fail against the pre-fix script. Wired tests/test-setup-ai.sh into CI, where it was previously unreachable. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 3 + CHANGELOG.md | 11 ++- README.md | 2 + setup-ai.sh | 79 ++++++++++++++----- tests/test-setup-ai.sh | 166 +++++++++++++++++++++++++-------------- whisper.cpp | 1 - 6 files changed, 179 insertions(+), 83 deletions(-) delete mode 160000 whisper.cpp diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e769bed..9677f32 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,6 +32,9 @@ jobs: - name: Install Playwright browsers run: npx playwright install --with-deps chromium + - name: Run AI bootstrap tests + run: bash tests/test-setup-ai.sh + - name: Run server tests run: | # Start signaling server in background diff --git a/CHANGELOG.md b/CHANGELOG.md index 4429534..262cb71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **`setup-ai.sh`** one-shot bootstrap script for the AI pipeline. It clones and builds `whisper.cpp`, downloads the default Whisper model into `models/`, and prompts for LLM provider settings in `.env`. +- **`setup-ai.sh`** one-shot bootstrap script for the AI pipeline. It clones and builds `whisper.cpp`, downloads the default Whisper model into `models/`, and prompts for LLM provider settings in `.env`. Thanks to @avezou (#12). +- **Regression tests for the AI bootstrap** in `tests/test-setup-ai.sh`, covering interrupted model downloads and `.env` files holding shell-hostile secrets, alongside the original happy path. + +### Fixed + +- **Interrupted model downloads are no longer cached as success.** `setup-ai.sh` wrote the 1.5 GB model straight to its final path, so a dropped transfer left a truncated file that every later run accepted as valid — and because `capabilities.js` detects the model with a bare `existsSync`, the UI would un-gate Transcribe over a corrupt model. Downloads now land in `models/ggml-medium.bin.part`, are size-checked, and only then renamed into place. +- **`setup-ai.sh` no longer sources `.env` as shell.** Sourcing executed the file, so a `JWT_SECRET` containing `$` aborted the script under `set -u` and a value with spaces ran as a command. Only the three `LLM_*` keys are read now, and unrelated secrets are left untouched. +- **`setup-ai.sh` no longer aborts when ffmpeg is missing.** Neither ffmpeg nor ffprobe is needed to clone whisper.cpp, build it, or fetch the model, so their absence now warns instead of blocking the entire bootstrap. +- **The 1.5 GB model download shows progress** instead of running silent, which read as a hung script. +- **`whisper.cpp` is no longer a tracked gitlink.** It was pinned as a `160000` entry with no `.gitmodules` backing, so `.gitignore` could never apply to it and every checkout that ran `setup-ai.sh` showed a permanently dirty `modified: whisper.cpp (new commits)`. It is now untracked and ignored, which is what the setup script already assumed. ## [0.3.2] - 2026-05-25 diff --git a/README.md b/README.md index 048f23c..e3d5201 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,8 @@ OpenStudio includes a complete post-production pipeline — transcription, show The script clones and builds `whisper.cpp`, downloads `models/ggml-medium.bin`, and prompts for your LLM provider so `.env` is ready for show notes. +It's safe to re-run. Each step is skipped if it's already done, and a download interrupted partway through is discarded rather than left half-written, so a second run picks up cleanly. The `whisper.cpp/` checkout and the model file are local build artifacts — both are gitignored and neither is tracked by this repo. + You can swap the model manually if you want — `ggml-tiny.bin` (~75 MB) is faster but less accurate; `ggml-large.bin` is the other direction. The server looks for `models/ggml-medium.bin` by default. ### LLM Provider Examples diff --git a/setup-ai.sh b/setup-ai.sh index c513751..599ab59 100755 --- a/setup-ai.sh +++ b/setup-ai.sh @@ -11,6 +11,9 @@ MODEL_DIR="models" MODEL_FILE="$MODEL_DIR/ggml-medium.bin" WHISPER_REPO="https://github.com/ggerganov/whisper.cpp.git" MODEL_URL="https://huggingface.co/ggerganov/whisper.cpp/resolve/main/ggml-medium.bin" +# ggml-medium.bin is ~1.5 GB; anything under 1 GB is a truncated transfer. +# Overridable so tests can exercise the truncation path without moving a real GB. +MODEL_MIN_BYTES="${MODEL_MIN_BYTES:-1000000000}" info() { printf '%s\n' "$*"; } ok() { printf 'OK: %s\n' "$*"; } @@ -36,9 +39,9 @@ download_file() { local output="$2" if command -v curl >/dev/null 2>&1; then - curl -L --fail --silent --show-error "$url" -o "$output" + curl -L --fail --progress-bar --show-error "$url" -o "$output" elif command -v wget >/dev/null 2>&1; then - wget -q "$url" -O "$output" + wget --show-progress -q "$url" -O "$output" else fail "Need curl or wget to download the Whisper model" fi @@ -52,11 +55,19 @@ ensure_env_file() { fi } +# Read a single key out of .env without sourcing it. Sourcing runs the file as +# shell, so a secret containing $, spaces, or ; either mangles the value, aborts +# under `set -u`, or executes. We only need the three LLM_* keys. +read_env_value() { + local key="$1" + [ -f "$ENV_FILE" ] || return 0 + sed -n "s/^[[:space:]]*${key}=//p" "$ENV_FILE" | tail -n 1 +} + load_env() { - set -a - # shellcheck disable=SC1091 - . "./$ENV_FILE" - set +a + LLM_BASE_URL="$(read_env_value LLM_BASE_URL)" + LLM_MODEL="$(read_env_value LLM_MODEL)" + LLM_API_KEY="$(read_env_value LLM_API_KEY)" } set_env_value() { @@ -107,11 +118,6 @@ ensure_whisper_cpp() { elif [ "$has_build_files" -eq 1 ]; then ok "Found whisper.cpp" fi - - if [ -x "$WHISPER_BIN" ]; then - ok "whisper.cpp already built" - return - fi } build_whisper_cpp() { @@ -145,17 +151,44 @@ build_whisper_cpp() { ok "Built whisper.cpp" } +file_size() { + wc -c < "$1" | tr -d '[:space:]' +} + ensure_model() { mkdir -p "$MODEL_DIR" + local partial="$MODEL_FILE.part" + if [ -f "$MODEL_FILE" ]; then - ok "Whisper model already present" - return + if [ "$(file_size "$MODEL_FILE")" -lt "$MODEL_MIN_BYTES" ]; then + warn "Existing model is only $(file_size "$MODEL_FILE") bytes — truncated, re-downloading" + rm -f "$MODEL_FILE" + else + ok "Whisper model already present" + return + fi fi - info "Downloading Whisper model..." - download_file "$MODEL_URL" "$MODEL_FILE" - [ -f "$MODEL_FILE" ] || fail "Model download failed" + # Download to .part and rename only on success. Writing straight to the final + # path means an interrupted transfer leaves a truncated file that every later + # run accepts as valid -- and capabilities.js detects the model with a bare + # existsSync, so the UI would un-gate Transcribe over a corrupt model. + rm -f "$partial" + info "Downloading Whisper model (~1.5 GB, this takes a while)..." + if ! download_file "$MODEL_URL" "$partial"; then + rm -f "$partial" + fail "Model download failed. Re-run ./setup-ai.sh to retry." + fi + + if [ ! -f "$partial" ] || [ "$(file_size "$partial")" -lt "$MODEL_MIN_BYTES" ]; then + local got="0" + [ -f "$partial" ] && got="$(file_size "$partial")" + rm -f "$partial" + fail "Model download truncated (got ${got} bytes, expected at least ${MODEL_MIN_BYTES}). Re-run ./setup-ai.sh to retry." + fi + + mv "$partial" "$MODEL_FILE" ok "Downloaded Whisper model" } @@ -256,12 +289,16 @@ main() { require_cmd git require_cmd awk - if ! command -v ffmpeg >/dev/null 2>&1; then - fail "ffmpeg is required. Install it first (brew install ffmpeg or apt install ffmpeg)." - fi + # ffmpeg/ffprobe are needed by the cleaning and export pipeline at runtime, but + # not to clone whisper.cpp, build it, or fetch the model. Don't block the whole + # bootstrap on them -- warn and let the user finish the parts that do work. + local missing_ffmpeg=0 + command -v ffmpeg >/dev/null 2>&1 || missing_ffmpeg=1 + command -v ffprobe >/dev/null 2>&1 || missing_ffmpeg=1 - if ! command -v ffprobe >/dev/null 2>&1; then - fail "ffprobe is required. Install it with ffmpeg before continuing." + if [ "$missing_ffmpeg" -eq 1 ]; then + warn "ffmpeg/ffprobe not found. Transcribe and export stay gated until you install them:" + warn " macOS: brew install ffmpeg Debian/Ubuntu: sudo apt install ffmpeg" fi ensure_env_file diff --git a/tests/test-setup-ai.sh b/tests/test-setup-ai.sh index 85d2b59..fa4e0ec 100755 --- a/tests/test-setup-ai.sh +++ b/tests/test-setup-ai.sh @@ -1,35 +1,40 @@ #!/bin/bash +# Smoke tests for setup-ai.sh, run against a fake toolchain (no network, no +# real build). Scenario 1 is the original happy path; scenarios 2 and 3 are +# regressions for bugs found reviewing PR #12. set -euo pipefail ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -TMPDIR="$(mktemp -d)" -trap 'rm -rf "$TMPDIR"' EXIT - -cp "$ROOT_DIR/setup-ai.sh" "$TMPDIR/setup-ai.sh" -cp "$ROOT_DIR/.env.example" "$TMPDIR/.env.example" -mkdir -p "$TMPDIR/fakebin" "$TMPDIR/fake-whisper-src" -: > "$TMPDIR/fake-whisper-src/CMakeLists.txt" - -cat > "$TMPDIR/fakebin/git" <<'EOF' +PASSED=0 + +# Real ggml-medium.bin is ~1.5 GB. Shrink the floor so the fake toolchain can +# exercise both sides of the truncation check without moving a gigabyte. +export MODEL_MIN_BYTES=16 + +# Builds an isolated sandbox with a fake git/cmake/ffmpeg/ffprobe on PATH. +# $1 = sandbox dir. The caller supplies its own fakebin/curl. +make_sandbox() { + local dir="$1" + mkdir -p "$dir/fakebin" "$dir/fake-whisper-src" + cp "$ROOT_DIR/setup-ai.sh" "$dir/setup-ai.sh" + cp "$ROOT_DIR/.env.example" "$dir/.env.example" + : > "$dir/fake-whisper-src/CMakeLists.txt" + + cat > "$dir/fakebin/git" <<'EOF' #!/bin/bash set -euo pipefail - if [ "${1:-}" = "clone" ]; then target="${@: -1}" src="${FAKE_WHISPER_SRC:?}" - rm -rf "$target" - mkdir -p "$target" - cp -R "$src"/. "$target"/ + rm -rf "$target"; mkdir -p "$target"; cp -R "$src"/. "$target"/ exit 0 fi - command git "$@" EOF -cat > "$TMPDIR/fakebin/cmake" <<'EOF' + cat > "$dir/fakebin/cmake" <<'EOF' #!/bin/bash set -euo pipefail - if [ "${1:-}" = "-S" ]; then builddir="" while [ "$#" -gt 0 ]; do @@ -38,60 +43,101 @@ if [ "${1:-}" = "-S" ]; then *) shift ;; esac done - mkdir -p "$builddir/bin" - cat > "$builddir/bin/whisper-cli" <<'EOB' -#!/bin/sh -exit 0 -EOB + printf '#!/bin/sh\nexit 0\n' > "$builddir/bin/whisper-cli" chmod +x "$builddir/bin/whisper-cli" - exit 0 fi - exit 0 EOF -cat > "$TMPDIR/fakebin/curl" <<'EOF' -#!/bin/bash -set -euo pipefail + printf '#!/bin/sh\nexit 0\n' > "$dir/fakebin/ffmpeg" + printf '#!/bin/sh\nexit 0\n' > "$dir/fakebin/ffprobe" + chmod +x "$dir/fakebin/"* +} +# A curl stub. $1 = sandbox, $2 = payload to write, $3 = exit code. +fake_curl() { + local dir="$1" payload="$2" code="$3" + cat > "$dir/fakebin/curl" < "$out" -EOF - -cat > "$TMPDIR/fakebin/ffmpeg" <<'EOF' -#!/bin/sh -exit 0 -EOF - -cat > "$TMPDIR/fakebin/ffprobe" <<'EOF' -#!/bin/sh -exit 0 +[ -n "\$out" ] || exit 1 +mkdir -p "\$(dirname "\$out")" +printf '%s' '$payload' > "\$out" +exit $code EOF - -chmod +x "$TMPDIR/fakebin/"* - -( - cd "$TMPDIR" - PATH="$TMPDIR/fakebin:$PATH" FAKE_WHISPER_SRC="$TMPDIR/fake-whisper-src" \ - bash ./setup-ai.sh <<'EOF' -2 + chmod +x "$dir/fakebin/curl" +} + +run_setup() { + local dir="$1" choice="$2" + ( cd "$dir" && PATH="$dir/fakebin:$PATH" FAKE_WHISPER_SRC="$dir/fake-whisper-src" \ + bash ./setup-ai.sh <<< "$choice" ) +} + +pass() { PASSED=$((PASSED + 1)); echo " PASS: $1"; } +die() { echo " FAIL: $1" >&2; exit 1; } + +# --- Scenario 1: happy path, Ollama selected ------------------------------- +echo "[1] happy path writes provider config and lands both artifacts" +S1="$(mktemp -d)"; trap 'rm -rf "$S1"' EXIT +make_sandbox "$S1" +fake_curl "$S1" "MODEL-PAYLOAD-LONG-ENOUGH" 0 +run_setup "$S1" "2" > "$S1/out.log" 2>&1 || die "setup-ai.sh exited nonzero" + +grep -q '^LLM_BASE_URL=http://localhost:11434/v1$' "$S1/.env" || die "LLM_BASE_URL not written" +grep -q '^LLM_MODEL=llama3.3$' "$S1/.env" || die "LLM_MODEL not written" +grep -q '^LLM_API_KEY=$' "$S1/.env" || die "LLM_API_KEY not blanked" +[ -x "$S1/whisper.cpp/build/bin/whisper-cli" ] || die "whisper-cli missing" +[ -f "$S1/models/ggml-medium.bin" ] || die "model missing" +pass "provider config, whisper-cli, and model all present" + +# --- Scenario 2: interrupted download must not be cached as success -------- +# Regression: writing straight to the final path left a truncated file that +# later runs accepted, and capabilities.js only checks existence -- so the UI +# would un-gate Transcribe over a corrupt model. +echo "[2] interrupted download is discarded, then recovers on retry" +S2="$(mktemp -d)"; trap 'rm -rf "$S1" "$S2"' EXIT +make_sandbox "$S2" +fake_curl "$S2" "TRUNC" 18 # curl(18): transfer closed early +if run_setup "$S2" "1" > "$S2/fail.log" 2>&1; then + die "setup-ai.sh reported success despite a failed download" +fi +[ ! -f "$S2/models/ggml-medium.bin" ] || die "truncated model left on disk" +[ ! -f "$S2/models/ggml-medium.bin.part" ] || die ".part file left on disk" +pass "truncated transfer left no model behind" + +fake_curl "$S2" "MODEL-PAYLOAD-LONG-ENOUGH" 0 +run_setup "$S2" "1" > "$S2/retry.log" 2>&1 || die "retry after failed download did not succeed" +[ -f "$S2/models/ggml-medium.bin" ] || die "retry did not produce a model" +pass "retry downloaded the model cleanly" + +# --- Scenario 3: .env holding real secrets must not break the script ------- +# Regression: load_env sourced .env as shell, so a secret containing $ aborted +# under `set -u` and a value with spaces ran as a command. +echo "[3] .env with shell-hostile secrets is read, not executed" +S3="$(mktemp -d)"; trap 'rm -rf "$S1" "$S2" "$S3"' EXIT +make_sandbox "$S3" +fake_curl "$S3" "MODEL-PAYLOAD-LONG-ENOUGH" 0 +cat > "$S3/.env" <<'EOF' +JWT_SECRET=k7$Rm2pQ9wZx +STATION_NAME=Pirate Radio One +ICECAST_SOURCE_PASSWORD=a;b`c` +LLM_BASE_URL=http://localhost:1234/v1 +LLM_MODEL=qwen3.5-35b +LLM_API_KEY= EOF -) - -grep -q '^LLM_BASE_URL=http://localhost:11434/v1$' "$TMPDIR/.env" -grep -q '^LLM_MODEL=llama3.3$' "$TMPDIR/.env" -grep -q '^LLM_API_KEY=$' "$TMPDIR/.env" -[ -x "$TMPDIR/whisper.cpp/build/bin/whisper-cli" ] -[ -f "$TMPDIR/models/ggml-medium.bin" ] - -echo "setup-ai.sh test passed" +run_setup "$S3" "2" > "$S3/out.log" 2>&1 || die "setup-ai.sh choked on a realistic .env" +grep -q '^JWT_SECRET=k7\$Rm2pQ9wZx$' "$S3/.env" || die "JWT_SECRET was mangled" +grep -q '^STATION_NAME=Pirate Radio One$' "$S3/.env" || die "STATION_NAME was mangled" +grep -q '^LLM_BASE_URL=http://localhost:11434/v1$' "$S3/.env" || die "LLM_BASE_URL not updated" +pass "unrelated secrets preserved verbatim, LLM keys updated" + +echo "" +echo "setup-ai.sh test passed ($PASSED checks)" diff --git a/whisper.cpp b/whisper.cpp deleted file mode 160000 index 1fe009c..0000000 --- a/whisper.cpp +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 1fe009caeda75f69bc864d6370b10674e45a92bd