Skip to content

fix(install): drop synthetic-providers prompt + auto-fallback to ~/.local/bin#21

Merged
samestrin merged 7 commits into
mainfrom
fix/install-no-synthetic-prompt-with-fallback
May 4, 2026
Merged

fix(install): drop synthetic-providers prompt + auto-fallback to ~/.local/bin#21
samestrin merged 7 commits into
mainfrom
fix/install-no-synthetic-prompt-with-fallback

Conversation

@samestrin
Copy link
Copy Markdown
Owner

Summary

Two related fixes to install.sh so the curl … | bash one-liner actually works for non-root users and stops conflating install with quickstart:

  • Drop the synthetic-providers prompt. The interactive Add synthetic providers? (y/N) prompt and the add_synthetic_providers helper are gone. Install now just installs. To add Synthetic / Alibaba Coding Plan models, run llm-env quickstart after install — the post-install next-steps output points at it.
  • Auto-fallback to ~/.local/bin. When /usr/local/bin isn't writable AND the user isn't root AND --install-dir was not passed, the installer now installs to $HOME/.local/bin instead of hard-failing. Explicit --install-dir choices are still respected. When fallback happens and ~/.local/bin isn't on $PATH, next-steps prints the exact export PATH=… line.

Plus three small follow-on fixes caught during adversarial review:

  • --uninstall now finds the script in ~/.local/bin when the default location is empty (auto-detects fallback installs).
  • --uninstall now respects --install-dir even when passed after --uninstall on the CLI (previously the parser exited before processing later flags).
  • --install-dir <new-dir> now mkdir -ps the dir if its parent is writable (prior behavior: hard-fail with a misleading sudo message).

Test plan

  • tests/integration/test_install.bats: 14 new tests, all green
  • ./tests/run_tests.sh full suite passes (including the docker e2e test that runs install.sh --offline inside a fresh ubuntu:22.04 container)
  • shellcheck install.sh clean
  • Manual e2e: non-root install with default unwritable → falls back ✅
  • Manual e2e: sudo bash install.sh → installs to /usr/local/bin
  • Manual e2e: explicit --install-dir <writable> → respected ✅
  • Manual e2e: explicit --install-dir <unwritable> → hard-fails with sudo message ✅
  • Manual e2e: --uninstall against fallback install → removes correctly ✅

samestrin added 7 commits May 4, 2026 14:19
Cover the install.sh refactor:
- assert add_synthetic_providers function and prompt text are removed
- assert install-time read -p prompts are gone (uninstall prompt allowed)
- assert --offline + writable --install-dir succeeds (regression guard)
- assert default-dir-unwritable + non-root falls back to ~/.local/bin
- assert explicit unwritable --install-dir still hard-fails
- assert next-steps mentions llm-env quickstart
- assert PATH warning fires only when ~/.local/bin is not on PATH
- assert shell rc snippet uses the post-fallback INSTALL_DIR

Tests use LLM_ENV_DEFAULT_INSTALL_DIR as a testability hook to override
the hardcoded /usr/local/bin default; install.sh implementation will
honor it in the next commit.

7 of 11 currently fail (Red).
…local/bin

Two related fixes that make `curl ... | bash` actually work for non-root
users and stop conflating install with quickstart:

1. Remove the interactive `Add synthetic providers? (y/N)` prompt and
   the `add_synthetic_providers` helper. Install now just installs.
   Synthetic / Alibaba models stay one command away via
   `llm-env quickstart`, surfaced in the next-steps output.

2. When /usr/local/bin isn't writable AND the user isn't root AND
   --install-dir was not passed, fall back to $HOME/.local/bin instead
   of hard-failing with "please run with sudo". Explicit --install-dir
   choices are still respected (and still hard-fail on unwritable
   targets — that's the user's deliberate choice). When fallback is
   used and ~/.local/bin isn't on PATH, next-steps prints a clear
   warning showing the export line to add.

The hardcoded /usr/local/bin default is now overridable via
LLM_ENV_DEFAULT_INSTALL_DIR for testability.

All 11 tests in tests/integration/test_install.bats pass.
- Replace "Installation Integration" section with an Optional post-install
  pointer to `llm-env quickstart`. The prior copy described an interactive
  prompt that no longer exists.
- Quick Install block now shows the default (user-install with fallback)
  alongside the explicit `sudo bash` form, with a note about the PATH
  warning the installer prints when falling back to ~/.local/bin.
…tall-dir

Two related bugs caught in e2e:

1. The arg parser handled --uninstall inline and exit-0'd, so any flags
   after it (like --install-dir) were ignored. Defer the uninstall call
   until after all args are parsed (UNINSTALL_REQUESTED flag).

2. uninstall_llm_env used the unmodified $INSTALL_DIR=/usr/local/bin
   default, so users who installed via the new ~/.local/bin fallback
   couldn't uninstall. Add auto-detection: if INSTALL_DIR doesn't have
   the script and ~/.local/bin does, redirect there. Explicit
   --install-dir still wins.

Adds 2 tests covering both behaviors. All 13 install tests pass.
Small ergonomic fix: when a user passes --install-dir to a directory
that doesn't exist yet (very common: ~/.local/bin on a fresh macOS
account), create it for them rather than hard-failing with a misleading
"please run with sudo" error.

Pre-existing behavior preserved for *unwritable* explicit dirs (mkdir
fails silently → falls through to the existing sudo message).
@samestrin samestrin merged commit 72dd939 into main May 4, 2026
8 checks passed
@samestrin samestrin deleted the fix/install-no-synthetic-prompt-with-fallback branch May 4, 2026 22:14
samestrin added a commit that referenced this pull request May 4, 2026
Patch release for the install.sh fixes shipped in #21:
- curl|bash install now works without sudo (auto-fallback to ~/.local/bin)
- --uninstall finds fallback installs and respects late --install-dir
- --install-dir <new-dir> mkdir -p's the dir if its parent is writable
- synthetic-providers prompt + helper removed from install.sh

No new features and no breaking changes — patch bump is correct per SemVer.
Non-interactive installs (CI, piped bash) are unaffected by the prompt
removal since the prompt only fired on a TTY.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant