fix(scripts): resolve catalog path and non-fatal-failure propagation bugs - #39
Conversation
…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 <repo>/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 <axel.seemann@netresearch.de>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new end-to-end smoke test script and consolidates repository path resolution by introducing a central root.sh library, which fixes path resolution bugs for the tool catalog. It also addresses several control-flow issues under set -e and set -o pipefail across the scripts. The review feedback highlights three key improvement opportunities: ensuring the catalog directory exists before running find in the smoke test to prevent premature aborts, writing temporary installation logs to a secure temporary directory instead of a hardcoded /tmp path to avoid conflicts and security risks, and replacing a locale-dependent type -a | grep check with a robust type -p -a | wc -l command.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
… locale-safe dup count 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 <github@sebastianmendel.de>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
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@sebastianmendel.de>
|
Summary
Three related bugs, all discovered while running
install_tool.shandcheck_environment.sh auditfor real (not just via the eval suite):install_tool.shand everyinstallers/*.shcomputed the catalog path as$DIR/../catalog, which resolves toskills/cli-tools/catalog— a directory that has never existed. The realcatalog/lives at the repo root, three levels abovescripts/. Everyinstall_tool.sh <tool> ...invocation failed with "No catalog entry found", regardless of tool.lib/catalog.shhad the same bug via an undefined$ROOT(currently dead code, but latent). Addedlib/root.shas a single source of truth for the catalog path.check_environment.shaudit bugs:check_duplicatescountedtype -ahits viagrep -c "is" || echo 0; underset -o pipefail,grep -c's own nonzero exit on a zero count triggered the|| echo 0fallback too, appending a second0line and crashing the-gtcomparison with "integer expression expected". Separately,check_path/check_duplicatesreturn their issue count by design, but were called bare inrun_audit, soset -eaborted the whole audit at the first issue found instead of completing it (same hazard thecheck_toolcalls further down are already guarded against).refresh_snapshotmasking install success as failure: it references<repo>/audit.py, which has never existed in this repo's history. 8 of 9 installer scripts call it unguarded as their last statement underset -euo pipefail, so every successful install via those installers reported failure via its exit code.Why these shipped
evals/evals.jsononly tests LLM prompting behavior ("does Claude call Bash and mention ripgrep") — no CI job actually executesinstall_tool.shorcheck_environment.shend-to-end. All three bugs are 100%-reproducible, not edge cases.Test plan
install_tool.sh fd statusnow correctly reports the real install method instead of "No catalog entry found"install_tool.sh <unknown> statusnow lists the real 77-entry catalog in its error messagecheck_environment.sh audit .now completes through to "Core Tools Status" instead of aborting after the first duplicate-install warninggithub_release_binary.sh fdinstalls a working binary into a scratchPREFIXand exits 0scripts/smoke-test.sh+.github/workflows/smoke-test.ymlcovering all of the above, so this class of bug fails CI going forward🤖 Generated with Claude Code