feat: add install script tests and enhance install-cli.sh for existin…

[sofusalbertsen]

…g installations

Add a scenario for breaking if kosli is installed with eg. homebrew in a different folder.

Reported by a customer who had installed the CLI through homebrew first.

[sami-alajrami]

Review by Claude (AI)

Good improvement addressing a real customer issue. The approach of detecting and refusing to overwrite non-standard installations (e.g. Homebrew) is sensible. A few issues worth addressing before merge:

---

🔴 Bug: Homebrew detection is too broad — it rejects legitimate install-script upgrades

The allowlist approach (/usr/local/bin, /usr/bin, /opt/bin) will fail for users who previously installed via this script on systems where the install landed in a non-standard but valid location (e.g. $HOME/.local/bin or a custom PREFIX). More importantly, it can't distinguish between "installed by Homebrew in /opt/homebrew/bin" and "installed by some other method in /opt/homebrew/bin".

A more targeted check would be to detect Homebrew specifically:

if command -v brew >/dev/null 2>&1 && brew list kosli-cli &>/dev/null; then
    echo "Kosli was installed via Homebrew. Please use 'brew upgrade kosli-cli' instead."
    exit 1
fi

This avoids false positives on legitimate installations in unusual directories.

---

🟡 run_install in test script has a word-splitting issue

In bin/test_install_script.sh, the $cmd variable is executed unquoted, so arguments with spaces or glob characters will break. Since the token could contain special characters, it would be safer to use an array:

run_install() {
    local args=("./install-cli.sh")
    if [ -n "$TOKEN" ]; then
        args+=(--token "$TOKEN")
    fi
    args+=("$@")
    echo "Running: ${args[*]}"
    "${args[@]}"
}

---

🟡 Inconsistent indentation in test_install_script.sh

The argument parsing block (lines 13–21) mixes tabs and spaces. The shift/echo/exit inside the case arms use hard tabs while the rest of the file uses spaces. Worth normalizing.

---

🟡 Missing comment in test_install_script_over_homebrew.sh about intentional lack of set -e

The other test script has set -e, but this one doesn't. That's intentional since you're checking the exit code manually — but it's worth adding a comment explaining that, so a future contributor doesn't "fix" it by adding set -e (which would break the test).

---

🟡 Missing newline at end of workflow file

The diff shows \ No newline at end of file at the end of install-script-tests.yml.

---

💭 Minor observations

• The --token is parsed in the test script but never passed through in the CI workflow. Is this intentional for future use, or leftover scaffolding? If the GitHub token is needed for rate limiting, the workflow should pass --token ${{ secrets.GITHUB_TOKEN }} to the test script.
• The Homebrew CI job installs Homebrew on macos-latest, which already has Homebrew pre-installed. The explicit install step is unnecessary and adds ~1-2 min to the job.

[sofusalbertsen]

@sami-alajrami added the noted points and features, please re-evaluate :)