From 44e2d8fcfdad450731cc51c589dc2a98719de286 Mon Sep 17 00:00:00 2001 From: Axel Seemann Date: Tue, 21 Jul 2026 13:12:08 +0200 Subject: [PATCH 1/3] fix(scripts): resolve catalog path and non-fatal-failure propagation bugs install_tool.sh and every installers/*.sh computed the catalog path as $DIR/../catalog, which resolves to skills/cli-tools/catalog - a directory that has never existed. The real catalog/ lives at the repo root, three levels above scripts/. This made every catalog lookup fail with "No catalog entry found", regardless of tool. lib/catalog.sh had the same bug via an undefined $ROOT variable (dead code, but latent). Introduce lib/root.sh as a single source of truth for the catalog path so this class of bug can't reappear via a future ad hoc relative path. check_environment.sh's check_duplicates counted `type -a` hits via `grep -c "is" || echo 0`; under `set -o pipefail`, grep -c's own nonzero exit on a zero count triggered the `|| echo 0` fallback too, appending a second "0" line and crashing the subsequent `-gt` comparison with "integer expression expected". Separately, check_path and check_duplicates return their issue count by design, but were called bare in run_audit, so `set -e` aborted the whole audit at the first PATH or duplicate-install issue found instead of completing it. refresh_snapshot() (lib/install_strategy.sh) referrs to /audit.py, which has never existed in this repo. 8 of 9 installer scripts call it unguarded as their last statement under set -euo pipefail, so every successful install reported failure via its exit code. Add scripts/smoke-test.sh + a CI job that actually executes install_tool.sh and check_environment.sh end-to-end, since the existing eval suite only tests LLM prompting behavior and never ran the real scripts - which is how these bugs shipped for multiple releases despite being 100% reproducible. Signed-off-by: Axel Seemann --- .github/workflows/smoke-test.yml | 19 ++++ scripts/smoke-test.sh | 99 +++++++++++++++++++ skills/cli-tools/scripts/check_environment.sh | 14 ++- skills/cli-tools/scripts/install_tool.sh | 5 +- .../scripts/installers/aws_installer.sh | 5 +- .../scripts/installers/dedicated_script.sh | 3 +- .../scripts/installers/github_clone.sh | 5 +- .../installers/github_release_binary.sh | 5 +- .../scripts/installers/hashicorp_zip.sh | 5 +- .../scripts/installers/npm_global.sh | 5 +- .../scripts/installers/npm_self_update.sh | 5 +- .../scripts/installers/package_manager.sh | 7 +- .../cli-tools/scripts/installers/uv_tool.sh | 5 +- skills/cli-tools/scripts/lib/catalog.sh | 13 +-- skills/cli-tools/scripts/lib/root.sh | 15 +++ 15 files changed, 181 insertions(+), 29 deletions(-) create mode 100644 .github/workflows/smoke-test.yml create mode 100755 scripts/smoke-test.sh create mode 100644 skills/cli-tools/scripts/lib/root.sh diff --git a/.github/workflows/smoke-test.yml b/.github/workflows/smoke-test.yml new file mode 100644 index 0000000..95ededd --- /dev/null +++ b/.github/workflows/smoke-test.yml @@ -0,0 +1,19 @@ +name: Smoke Test + +on: + push: + branches: [main] + pull_request: + +permissions: + contents: read + +jobs: + smoke-test: + name: Execute install_tool.sh and check_environment.sh + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Run smoke test + run: bash scripts/smoke-test.sh diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh new file mode 100755 index 0000000..dcde567 --- /dev/null +++ b/scripts/smoke-test.sh @@ -0,0 +1,99 @@ +#!/usr/bin/env bash +# smoke-test.sh — Executes the skill's shell scripts end-to-end. +# +# The eval suite (evals/evals.json) only checks LLM prompting behavior; it +# never actually runs install_tool.sh or check_environment.sh. That gap let a +# broken catalog path (scripts moved under skills/cli-tools/ without updating +# the relative path to catalog/) and a set -e/pipefail interaction bug ship +# for multiple releases despite being 100% reproducible. This script runs the +# real entry points against the real repo layout so path and control-flow +# regressions like those fail CI instead of shipping silently. +set -euo pipefail + +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "$DIR/.." && pwd)" +SCRIPTS="$REPO_ROOT/skills/cli-tools/scripts" + +RED="\033[0;31m" +GREEN="\033[0;32m" +NC="\033[0m" + +FAILURES=0 + +pass() { echo -e "${GREEN}✓${NC} $1"; } +fail() { echo -e "${RED}✗${NC} $1"; FAILURES=$((FAILURES + 1)); } + +echo "=== smoke-test: catalog resolution ===" + +# Every catalog entry must resolve via install_tool.sh's own lookup, proving +# the DIR/../../../catalog (via lib/root.sh) math matches the real layout. +catalog_count=$(find "$REPO_ROOT/catalog" -name '*.json' | wc -l) +if [ "$catalog_count" -lt 1 ]; then + fail "catalog directory empty or missing at $REPO_ROOT/catalog" +else + pass "catalog directory found ($catalog_count entries)" +fi + +# A known-good tool must report a real install_method, not "No catalog entry +# found" (the exact symptom of the path bug). +if output=$(bash "$SCRIPTS/install_tool.sh" fd status 2>&1); then + pass "install_tool.sh fd status: $output" +else + fail "install_tool.sh fd status failed:"$'\n'"$output" +fi + +# A genuinely unknown tool must still enumerate real catalog entries in its +# error message, not an empty list. +if output=$(bash "$SCRIPTS/install_tool.sh" __not_a_real_tool__ status 2>&1); then + fail "install_tool.sh __not_a_real_tool__ status unexpectedly succeeded" +else + if echo "$output" | grep -q "fd "; then + pass "install_tool.sh reports real available tools on unknown-tool error" + else + fail "install_tool.sh did not list real catalog entries:"$'\n'"$output" + fi +fi + +echo "" +echo "=== smoke-test: full installer round-trip ===" + +# Actually install a small, single-static-binary tool via the github_release_binary +# installer to prove the whole chain (catalog lookup -> installer dispatch -> +# download -> binary in place) works, not just the catalog lookup in isolation. +# PREFIX (read by lib/install_strategy.sh's get_install_dir) isolates the +# install to a scratch dir instead of touching the real ~/.local/bin. +TMP_PREFIX="$(mktemp -d)" +if PREFIX="$TMP_PREFIX" bash "$SCRIPTS/installers/github_release_binary.sh" fd >/tmp/smoke-fd-install.log 2>&1; then + if [ -x "$TMP_PREFIX/bin/fd" ] && "$TMP_PREFIX/bin/fd" --version >/dev/null 2>&1; then + pass "github_release_binary.sh installed a working fd: $("$TMP_PREFIX/bin/fd" --version)" + else + fail "github_release_binary.sh reported success but $TMP_PREFIX/bin/fd is missing or broken:"$'\n'"$(cat /tmp/smoke-fd-install.log)" + fi +else + fail "github_release_binary.sh fd install failed:"$'\n'"$(cat /tmp/smoke-fd-install.log)" +fi +rm -rf "$TMP_PREFIX" + +echo "" +echo "=== smoke-test: check_environment.sh completes without aborting early ===" + +# Must reach its final exit 0, not stop partway through because set -e +# aborted on a helper function's by-design nonzero "issue count" return. +if output=$(bash "$SCRIPTS/check_environment.sh" audit "$REPO_ROOT" 2>&1); then + if echo "$output" | grep -q "Core Tools Status"; then + pass "check_environment.sh audit ran to completion" + else + fail "check_environment.sh audit exited 0 but stopped before the Core Tools Status section:"$'\n'"$output" + fi +else + fail "check_environment.sh audit exited non-zero:"$'\n'"$output" +fi + +echo "" +if [ "$FAILURES" -eq 0 ]; then + echo -e "${GREEN}All smoke tests passed${NC}" + exit 0 +else + echo -e "${RED}$FAILURES smoke test(s) failed${NC}" + exit 1 +fi diff --git a/skills/cli-tools/scripts/check_environment.sh b/skills/cli-tools/scripts/check_environment.sh index 634a1d0..cd9c18c 100755 --- a/skills/cli-tools/scripts/check_environment.sh +++ b/skills/cli-tools/scripts/check_environment.sh @@ -89,7 +89,11 @@ check_duplicates() { for tool in "${tools[@]}"; do local paths - paths=$(type -a "$tool" 2>/dev/null | grep -c "is" || echo 0) + # grep -c always prints a count (even 0), but exits 1 when that count is + # 0; under `set -o pipefail` that nonzero exit used to trigger `|| echo 0` + # as well, appending a second "0" line and breaking the -gt comparison + # below. `|| true` keeps the count grep already printed as the only output. + paths=$(type -a "$tool" 2>/dev/null | grep -c "is" || true) if [ "$paths" -gt 1 ]; then log_warn "$tool has multiple installations:" type -a "$tool" 2>/dev/null | head -5 @@ -178,10 +182,14 @@ run_audit() { echo "═══════════════════════════════════════════════" echo "" - check_path + # check_path/check_duplicates return their issue count by design (not a + # fatal error); under `set -e` a bare call aborts run_audit at the first + # issue found instead of completing the rest of the audit, the same + # hazard the check_tool calls below are already guarded against. + check_path || true echo "" - check_duplicates + check_duplicates || true echo "" check_package_managers diff --git a/skills/cli-tools/scripts/install_tool.sh b/skills/cli-tools/scripts/install_tool.sh index 5a20aca..4f83b76 100755 --- a/skills/cli-tools/scripts/install_tool.sh +++ b/skills/cli-tools/scripts/install_tool.sh @@ -6,6 +6,7 @@ set -euo pipefail DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" # Source reconciliation libraries +. "$DIR/lib/root.sh" . "$DIR/lib/reconcile.sh" TOOL="${1:-}" @@ -17,12 +18,12 @@ if [ -z "$TOOL" ]; then exit 1 fi -CATALOG_FILE="$DIR/../catalog/$TOOL.json" +CATALOG_FILE="$CATALOG_DIR/$TOOL.json" # Check if tool has catalog entry if [ ! -f "$CATALOG_FILE" ]; then echo "[$TOOL] Error: No catalog entry found" >&2 - echo "[$TOOL] Available tools: $(find "$DIR/../catalog" -name '*.json' -exec basename {} .json \; | tr '\n' ' ')" >&2 + echo "[$TOOL] Available tools: $(find "$CATALOG_DIR" -name '*.json' -exec basename {} .json \; | tr '\n' ' ')" >&2 exit 1 fi diff --git a/skills/cli-tools/scripts/installers/aws_installer.sh b/skills/cli-tools/scripts/installers/aws_installer.sh index dfc95d2..375592e 100755 --- a/skills/cli-tools/scripts/installers/aws_installer.sh +++ b/skills/cli-tools/scripts/installers/aws_installer.sh @@ -3,10 +3,11 @@ set -euo pipefail DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +. "$DIR/lib/root.sh" . "$DIR/lib/install_strategy.sh" TOOL="${1:-aws}" -CATALOG_FILE="$DIR/../catalog/$TOOL.json" +CATALOG_FILE="$CATALOG_DIR/$TOOL.json" if [ ! -f "$CATALOG_FILE" ]; then echo "Error: Catalog file not found: $CATALOG_FILE" >&2 @@ -46,4 +47,4 @@ printf "[%s] after: %s\n" "$TOOL" "${after:-}" if [ -n "$path" ]; then printf "[%s] path: %s\n" "$TOOL" "$path"; fi # Refresh snapshot after successful installation -refresh_snapshot "$TOOL" +refresh_snapshot "$TOOL" || true diff --git a/skills/cli-tools/scripts/installers/dedicated_script.sh b/skills/cli-tools/scripts/installers/dedicated_script.sh index b74b999..02becd5 100755 --- a/skills/cli-tools/scripts/installers/dedicated_script.sh +++ b/skills/cli-tools/scripts/installers/dedicated_script.sh @@ -4,6 +4,7 @@ set -euo pipefail DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +. "$DIR/lib/root.sh" TOOL="${1:-}" if [ -z "$TOOL" ]; then @@ -11,7 +12,7 @@ if [ -z "$TOOL" ]; then exit 1 fi -CATALOG_FILE="$DIR/../catalog/$TOOL.json" +CATALOG_FILE="$CATALOG_DIR/$TOOL.json" if [ ! -f "$CATALOG_FILE" ]; then echo "Error: Catalog file not found: $CATALOG_FILE" >&2 exit 1 diff --git a/skills/cli-tools/scripts/installers/github_clone.sh b/skills/cli-tools/scripts/installers/github_clone.sh index 5cb5319..2c37416 100755 --- a/skills/cli-tools/scripts/installers/github_clone.sh +++ b/skills/cli-tools/scripts/installers/github_clone.sh @@ -3,6 +3,7 @@ set -euo pipefail DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +. "$DIR/lib/root.sh" . "$DIR/lib/install_strategy.sh" TOOL="${1:-}" @@ -13,7 +14,7 @@ fi ACTION="${2:-install}" -CATALOG_FILE="$DIR/../catalog/$TOOL.json" +CATALOG_FILE="$CATALOG_DIR/$TOOL.json" if [ ! -f "$CATALOG_FILE" ]; then echo "[$TOOL] Error: Catalog file not found: $CATALOG_FILE" >&2 exit 1 @@ -87,4 +88,4 @@ printf "[%s] after: %s\n" "$TOOL" "${after:-}" printf "[%s] path: %s\n" "$TOOL" "$CLONE_PATH" # Refresh snapshot after successful installation -refresh_snapshot "$TOOL" +refresh_snapshot "$TOOL" || true diff --git a/skills/cli-tools/scripts/installers/github_release_binary.sh b/skills/cli-tools/scripts/installers/github_release_binary.sh index 89a4247..26902f4 100755 --- a/skills/cli-tools/scripts/installers/github_release_binary.sh +++ b/skills/cli-tools/scripts/installers/github_release_binary.sh @@ -4,6 +4,7 @@ set -euo pipefail DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +. "$DIR/lib/root.sh" . "$DIR/lib/common.sh" . "$DIR/lib/install_strategy.sh" @@ -13,7 +14,7 @@ if [ -z "$TOOL" ]; then exit 1 fi -CATALOG_FILE="$DIR/../catalog/$TOOL.json" +CATALOG_FILE="$CATALOG_DIR/$TOOL.json" if [ ! -f "$CATALOG_FILE" ]; then echo "Error: Catalog file not found: $CATALOG_FILE" >&2 exit 1 @@ -251,4 +252,4 @@ printf "[%s] after: %s\n" "$TOOL" "${after:-}" if [ -n "$path" ]; then printf "[%s] path: %s\n" "$TOOL" "$path"; fi # Refresh snapshot after successful installation -refresh_snapshot "$TOOL" +refresh_snapshot "$TOOL" || true diff --git a/skills/cli-tools/scripts/installers/hashicorp_zip.sh b/skills/cli-tools/scripts/installers/hashicorp_zip.sh index 6f218b4..306b172 100755 --- a/skills/cli-tools/scripts/installers/hashicorp_zip.sh +++ b/skills/cli-tools/scripts/installers/hashicorp_zip.sh @@ -3,6 +3,7 @@ set -euo pipefail DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +. "$DIR/lib/root.sh" . "$DIR/lib/common.sh" . "$DIR/lib/install_strategy.sh" @@ -12,7 +13,7 @@ if [ -z "$TOOL" ]; then exit 1 fi -CATALOG_FILE="$DIR/../catalog/$TOOL.json" +CATALOG_FILE="$CATALOG_DIR/$TOOL.json" if [ ! -f "$CATALOG_FILE" ]; then echo "Error: Catalog file not found: $CATALOG_FILE" >&2 exit 1 @@ -87,4 +88,4 @@ printf "[%s] after: %s\n" "$TOOL" "${after:-}" if [ -n "$path" ]; then printf "[%s] path: %s\n" "$TOOL" "$path"; fi # Refresh snapshot after successful installation -refresh_snapshot "$TOOL" +refresh_snapshot "$TOOL" || true diff --git a/skills/cli-tools/scripts/installers/npm_global.sh b/skills/cli-tools/scripts/installers/npm_global.sh index 1c14d39..3720675 100755 --- a/skills/cli-tools/scripts/installers/npm_global.sh +++ b/skills/cli-tools/scripts/installers/npm_global.sh @@ -3,6 +3,7 @@ set -euo pipefail DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +. "$DIR/lib/root.sh" . "$DIR/lib/install_strategy.sh" # Load nvm if available (needed for node-based package managers) @@ -18,7 +19,7 @@ if [ -z "$TOOL" ]; then exit 1 fi -CATALOG_FILE="$DIR/../catalog/$TOOL.json" +CATALOG_FILE="$CATALOG_DIR/$TOOL.json" if [ ! -f "$CATALOG_FILE" ]; then echo "Error: Catalog file not found: $CATALOG_FILE" >&2 exit 1 @@ -83,4 +84,4 @@ printf "[%s] after: %s\n" "$TOOL" "${after:-}" if [ -n "$path" ]; then printf "[%s] path: %s\n" "$TOOL" "$path"; fi # Refresh snapshot after successful installation -refresh_snapshot "$TOOL" +refresh_snapshot "$TOOL" || true diff --git a/skills/cli-tools/scripts/installers/npm_self_update.sh b/skills/cli-tools/scripts/installers/npm_self_update.sh index c7138ea..2065884 100755 --- a/skills/cli-tools/scripts/installers/npm_self_update.sh +++ b/skills/cli-tools/scripts/installers/npm_self_update.sh @@ -3,6 +3,7 @@ set -euo pipefail DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +. "$DIR/lib/root.sh" . "$DIR/lib/common.sh" . "$DIR/lib/install_strategy.sh" @@ -12,7 +13,7 @@ if [ -z "$TOOL" ]; then exit 1 fi -CATALOG_FILE="$DIR/../catalog/$TOOL.json" +CATALOG_FILE="$CATALOG_DIR/$TOOL.json" if [ ! -f "$CATALOG_FILE" ]; then echo "Error: Catalog file not found: $CATALOG_FILE" >&2 exit 1 @@ -50,4 +51,4 @@ else fi # Refresh snapshot after successful installation -refresh_snapshot "$TOOL" +refresh_snapshot "$TOOL" || true diff --git a/skills/cli-tools/scripts/installers/package_manager.sh b/skills/cli-tools/scripts/installers/package_manager.sh index 14281b3..2066174 100755 --- a/skills/cli-tools/scripts/installers/package_manager.sh +++ b/skills/cli-tools/scripts/installers/package_manager.sh @@ -4,6 +4,7 @@ set -euo pipefail DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +. "$DIR/lib/root.sh" . "$DIR/lib/common.sh" . "$DIR/lib/install_strategy.sh" @@ -13,7 +14,7 @@ if [ -z "$TOOL" ]; then exit 1 fi -CATALOG_FILE="$DIR/../catalog/$TOOL.json" +CATALOG_FILE="$CATALOG_DIR/$TOOL.json" if [ ! -f "$CATALOG_FILE" ]; then echo "Error: Catalog file not found: $CATALOG_FILE" >&2 exit 1 @@ -116,7 +117,7 @@ if command -v "$BINARY_NAME" >/dev/null 2>&1; then printf "[%s] note: %s\n" "$TOOL" "Already available (bundled with runtime)" # Refresh snapshot to record current version - refresh_snapshot "$TOOL" + refresh_snapshot "$TOOL" || true exit 0 fi fi @@ -180,4 +181,4 @@ if [ -n "$path" ]; then printf "[%s] path: %s\n" "$TOOL" "$path"; fi # Refresh snapshot after successful installation # Need to source install_strategy.sh for refresh_snapshot function . "$(dirname "${BASH_SOURCE[0]}")/../lib/install_strategy.sh" -refresh_snapshot "$TOOL" +refresh_snapshot "$TOOL" || true diff --git a/skills/cli-tools/scripts/installers/uv_tool.sh b/skills/cli-tools/scripts/installers/uv_tool.sh index 96abee8..deb7b4c 100755 --- a/skills/cli-tools/scripts/installers/uv_tool.sh +++ b/skills/cli-tools/scripts/installers/uv_tool.sh @@ -3,6 +3,7 @@ set -euo pipefail DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +. "$DIR/lib/root.sh" TOOL="${1:-}" if [ -z "$TOOL" ]; then @@ -10,7 +11,7 @@ if [ -z "$TOOL" ]; then exit 1 fi -CATALOG_FILE="$DIR/../catalog/$TOOL.json" +CATALOG_FILE="$CATALOG_DIR/$TOOL.json" if [ ! -f "$CATALOG_FILE" ]; then echo "Error: Catalog file not found: $CATALOG_FILE" >&2 exit 1 @@ -57,4 +58,4 @@ if [ -n "$path" ]; then printf "[%s] path: %s\n" "$TOOL" "$path"; fi # Refresh snapshot after successful installation # Source install_strategy.sh for refresh_snapshot function . "$(dirname "${BASH_SOURCE[0]}")/../lib/install_strategy.sh" -refresh_snapshot "$TOOL" +refresh_snapshot "$TOOL" || true diff --git a/skills/cli-tools/scripts/lib/catalog.sh b/skills/cli-tools/scripts/lib/catalog.sh index 75f1cbe..5462090 100755 --- a/skills/cli-tools/scripts/lib/catalog.sh +++ b/skills/cli-tools/scripts/lib/catalog.sh @@ -1,11 +1,12 @@ #!/usr/bin/env bash # Catalog query functions for reading tool metadata -# Assumes: Scripts are run from app root, catalog is at $ROOT/catalog + +. "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/root.sh" # Get all tools with a specific tag catalog_get_tools_by_tag() { local tag="$1" - local catalog_dir="$ROOT/catalog" + local catalog_dir="$CATALOG_DIR" if ! command -v jq >/dev/null 2>&1; then echo "Error: jq required for catalog operations" >&2 @@ -22,7 +23,7 @@ catalog_get_tools_by_tag() { # Get all available tags catalog_get_all_tags() { - local catalog_dir="$ROOT/catalog" + local catalog_dir="$CATALOG_DIR" if ! command -v jq >/dev/null 2>&1; then echo "Error: jq required for catalog operations" >&2 @@ -35,7 +36,7 @@ catalog_get_all_tags() { # Check if tool has catalog entry catalog_has_tool() { local tool="$1" - local catalog_dir="$ROOT/catalog" + local catalog_dir="$CATALOG_DIR" [ -f "$catalog_dir/$tool.json" ] } @@ -43,7 +44,7 @@ catalog_has_tool() { catalog_get_property() { local tool="$1" local property="$2" - local catalog_dir="$ROOT/catalog" + local catalog_dir="$CATALOG_DIR" if ! command -v jq >/dev/null 2>&1; then echo "Error: jq required for catalog operations" >&2 @@ -61,7 +62,7 @@ catalog_get_guide_property() { local tool="$1" local property="$2" local default="${3:-}" - local catalog_dir="$ROOT/catalog" + local catalog_dir="$CATALOG_DIR" if ! command -v jq >/dev/null 2>&1; then echo "$default" diff --git a/skills/cli-tools/scripts/lib/root.sh b/skills/cli-tools/scripts/lib/root.sh new file mode 100644 index 0000000..3c27830 --- /dev/null +++ b/skills/cli-tools/scripts/lib/root.sh @@ -0,0 +1,15 @@ +#!/usr/bin/env bash +# root.sh - single source of truth for the skill's repo-root-relative paths +# +# This file lives at /skills/cli-tools/scripts/lib/root.sh. +# Every other script needs the catalog directory, which lives at +# /catalog - three levels above scripts/. Computing that offset +# ad hoc in every script (as `$DIR/../catalog`) is what caused the catalog +# to silently resolve to a nonexistent directory after the scripts were +# moved under skills/cli-tools/. Source this file instead of recomputing +# the path locally. + +SKILL_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SKILL_SCRIPTS_DIR="$(cd "$SKILL_LIB_DIR/.." && pwd)" +SKILL_ROOT="$(cd "$SKILL_SCRIPTS_DIR/../../.." && pwd)" +CATALOG_DIR="$SKILL_ROOT/catalog" From 34fff7b57517e9727ccba867746758472668a2aa Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Fri, 24 Jul 2026 18:55:38 +0200 Subject: [PATCH 2/3] =?UTF-8?q?fix(scripts):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20robust=20catalog=20check,=20secure=20temp=20log,=20?= =?UTF-8?q?locale-safe=20dup=20count?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applies the gemini-code-assist review on this PR: - smoke-test.sh: guard the catalog dir with a -d test before find, so a missing dir fails with a clear message instead of aborting the whole script under set -e (bypassing the summary). - smoke-test.sh: write the fd install log inside the per-run mktemp dir ($TMP_PREFIX) instead of a hardcoded /tmp path — removes the world-writable /tmp symlink-attack surface SonarCloud flagged (new_security_rating). - check_environment.sh: count duplicate installs with 'type -p -a | wc -l' (absolute paths) instead of 'type -a | grep -c "is"', which is locale dependent and returned 0 in non-English locales. Signed-off-by: Sebastian Mendel --- scripts/smoke-test.sh | 18 +++++++++++------- skills/cli-tools/scripts/check_environment.sh | 10 +++++----- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh index dcde567..b104cdb 100755 --- a/scripts/smoke-test.sh +++ b/scripts/smoke-test.sh @@ -27,11 +27,15 @@ echo "=== smoke-test: catalog resolution ===" # Every catalog entry must resolve via install_tool.sh's own lookup, proving # the DIR/../../../catalog (via lib/root.sh) math matches the real layout. -catalog_count=$(find "$REPO_ROOT/catalog" -name '*.json' | wc -l) -if [ "$catalog_count" -lt 1 ]; then - fail "catalog directory empty or missing at $REPO_ROOT/catalog" +if [ ! -d "$REPO_ROOT/catalog" ]; then + fail "catalog directory missing at $REPO_ROOT/catalog" else - pass "catalog directory found ($catalog_count entries)" + catalog_count=$(find "$REPO_ROOT/catalog" -name '*.json' | wc -l) + if [ "$catalog_count" -lt 1 ]; then + fail "catalog directory empty at $REPO_ROOT/catalog" + else + pass "catalog directory found ($catalog_count entries)" + fi fi # A known-good tool must report a real install_method, not "No catalog entry @@ -63,14 +67,14 @@ echo "=== smoke-test: full installer round-trip ===" # PREFIX (read by lib/install_strategy.sh's get_install_dir) isolates the # install to a scratch dir instead of touching the real ~/.local/bin. TMP_PREFIX="$(mktemp -d)" -if PREFIX="$TMP_PREFIX" bash "$SCRIPTS/installers/github_release_binary.sh" fd >/tmp/smoke-fd-install.log 2>&1; then +if PREFIX="$TMP_PREFIX" bash "$SCRIPTS/installers/github_release_binary.sh" fd >"$TMP_PREFIX/smoke-fd-install.log" 2>&1; then if [ -x "$TMP_PREFIX/bin/fd" ] && "$TMP_PREFIX/bin/fd" --version >/dev/null 2>&1; then pass "github_release_binary.sh installed a working fd: $("$TMP_PREFIX/bin/fd" --version)" else - fail "github_release_binary.sh reported success but $TMP_PREFIX/bin/fd is missing or broken:"$'\n'"$(cat /tmp/smoke-fd-install.log)" + fail "github_release_binary.sh reported success but $TMP_PREFIX/bin/fd is missing or broken:"$'\n'"$(cat "$TMP_PREFIX/smoke-fd-install.log")" fi else - fail "github_release_binary.sh fd install failed:"$'\n'"$(cat /tmp/smoke-fd-install.log)" + fail "github_release_binary.sh fd install failed:"$'\n'"$(cat "$TMP_PREFIX/smoke-fd-install.log")" fi rm -rf "$TMP_PREFIX" diff --git a/skills/cli-tools/scripts/check_environment.sh b/skills/cli-tools/scripts/check_environment.sh index cd9c18c..fd997dc 100755 --- a/skills/cli-tools/scripts/check_environment.sh +++ b/skills/cli-tools/scripts/check_environment.sh @@ -89,11 +89,11 @@ check_duplicates() { for tool in "${tools[@]}"; do local paths - # grep -c always prints a count (even 0), but exits 1 when that count is - # 0; under `set -o pipefail` that nonzero exit used to trigger `|| echo 0` - # as well, appending a second "0" line and breaking the -gt comparison - # below. `|| true` keeps the count grep already printed as the only output. - paths=$(type -a "$tool" 2>/dev/null | grep -c "is" || true) + # `type -p -a` prints one absolute path per executable found, so wc -l is + # the install count — locale-independent, unlike parsing `type -a`'s prose + # (which localizes "is"/"ist"/"est" and broke the count in non-English + # locales). `|| true` guards the empty-result case under set -o pipefail. + paths=$(type -p -a "$tool" 2>/dev/null | wc -l | tr -d ' ' || true) if [ "$paths" -gt 1 ]; then log_warn "$tool has multiple installations:" type -a "$tool" 2>/dev/null | head -5 From 3d2cc95bb11990d4d5b6e009d15bcd09efccb9db Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Fri, 24 Jul 2026 19:23:16 +0200 Subject: [PATCH 3/3] ci(smoke-test): pin actions/checkout to a full SHA The repo policy blocks unpinned third-party actions; actions/checkout@v4 failed the Smoke Test run at setup. Pin to the org-canonical SHA (v7.0.1), matching the other netresearch skill repos. netresearch-owned reusable workflows correctly stay on @main. Signed-off-by: Sebastian Mendel --- .github/workflows/smoke-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/smoke-test.yml b/.github/workflows/smoke-test.yml index 95ededd..eaf0948 100644 --- a/.github/workflows/smoke-test.yml +++ b/.github/workflows/smoke-test.yml @@ -13,7 +13,7 @@ jobs: name: Execute install_tool.sh and check_environment.sh runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Run smoke test run: bash scripts/smoke-test.sh