diff --git a/scripts/install_node.sh b/scripts/install_node.sh index b6626de..775b55a 100755 --- a/scripts/install_node.sh +++ b/scripts/install_node.sh @@ -81,6 +81,14 @@ install_node() { } update_node() { + # Preserve deliberate uninstalls across the Node.js version switch. nvm + # installs a fresh prefix, so checking after the switch would lose whether + # these optional package managers existed beforehand. + local had_pnpm=0 + local had_yarn=0 + command -v pnpm >/dev/null 2>&1 && had_pnpm=1 + command -v yarn >/dev/null 2>&1 && had_yarn=1 + ensure_nvm nvm install "$NODE_CHANNEL" # Re-source nvm to ensure the new version is active in this shell @@ -98,12 +106,17 @@ update_node() { nvm alias default "$NODE_CHANNEL" || true nvm use default || true fi - # Ensure corepack shims are present - corepack enable 2>/dev/null || true npm install -g npm@latest || true - # Update pnpm and yarn via corepack; fall back to npm global if corepack unavailable - corepack prepare pnpm@latest --activate 2>/dev/null || npm install -g pnpm@latest || true - corepack prepare yarn@1 --activate 2>/dev/null || npm install -g yarn@latest || true + # Only restore/update package managers that were installed before the + # Node.js update. Explicit installs still enable both in install_node(). + if [ "$had_pnpm" -eq 1 ]; then + corepack enable pnpm 2>/dev/null || true + corepack prepare pnpm@latest --activate 2>/dev/null || npm install -g pnpm@latest || true + fi + if [ "$had_yarn" -eq 1 ]; then + corepack enable yarn 2>/dev/null || true + corepack prepare yarn@1 --activate 2>/dev/null || npm install -g yarn@latest || true + fi npm update -g eslint prettier || true else echo "=> Node.js version $NODE_VERSION has been updated" @@ -168,7 +181,8 @@ reconcile_node() { after="$(get_specific_node_version "$NODE_CHANNEL")" # Get path to the specific version's node binary local nvm_dir="${NVM_DIR:-$HOME/.nvm}" - local resolved_ver="$(nvm version "$NODE_CHANNEL" 2>/dev/null || true)" + local resolved_ver + resolved_ver="$(nvm version "$NODE_CHANNEL" 2>/dev/null || true)" if [ -n "$resolved_ver" ] && [ "$resolved_ver" != "N/A" ]; then path="$nvm_dir/versions/node/$resolved_ver/bin/node" fi @@ -191,5 +205,3 @@ case "$ACTION" in reconcile) reconcile_node ;; *) echo "Usage: $0 {install|update|uninstall|reconcile}" ; exit 2 ;; esac - - diff --git a/scripts/install_python.sh b/scripts/install_python.sh index f1e336c..f9fcd2e 100755 --- a/scripts/install_python.sh +++ b/scripts/install_python.sh @@ -127,7 +127,10 @@ install_or_update_py_cli() { if [ "$cmd" = install ]; then uv tool install -q "$p" >/dev/null 2>&1 || true else - uv tool upgrade -q "$p" >/dev/null 2>&1 || uv tool install -q "$p" >/dev/null 2>&1 || true + # Upgrade only tools that are already installed. A failed upgrade + # commonly means that the tool was deliberately removed; installing it + # here would undo `make uninstall-`. + uv tool upgrade -q "$p" >/dev/null 2>&1 || true fi done else @@ -139,7 +142,7 @@ install_or_update_py_cli() { if [ "$cmd" = install ]; then pipx install "$p" >/dev/null 2>&1 || true else - pipx upgrade "$p" >/dev/null 2>&1 || pipx install "$p" >/dev/null 2>&1 || true + pipx upgrade "$p" >/dev/null 2>&1 || true fi done fi @@ -270,5 +273,3 @@ case "$ACTION" in reconcile) reconcile_py_tools ;; *) echo "Usage: $0 {install|update|uninstall|reconcile}" ; exit 2 ;; esac - - diff --git a/scripts/install_tool.sh b/scripts/install_tool.sh index 6dc39ce..376ec3d 100755 --- a/scripts/install_tool.sh +++ b/scripts/install_tool.sh @@ -7,6 +7,8 @@ DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" # Source reconciliation libraries . "$DIR/lib/reconcile.sh" +# Source snapshot refresh helper used after uninstall. +. "$DIR/lib/install_strategy.sh" # Source bash-completion lifecycle helpers (install_completion / remove_completion / # post_install_completion). All completion operations are best-effort. . "$DIR/lib/completion.sh" @@ -60,6 +62,7 @@ if [ "$ACTION" = "uninstall" ]; then if [ -n "$script_name" ] && [ -f "$DIR/$script_name" ]; then "$DIR/$script_name" uninstall || true fi + refresh_snapshot "$TOOL" || true # Dedicated scripts handle their own cleanup completely # Don't try to detect and remove additional installations # (especially important for multi-version tools like node, python, go) @@ -75,6 +78,7 @@ if [ "$ACTION" = "uninstall" ]; then if [ "$install_count" -eq 0 ]; then echo "[$TOOL] Successfully removed" + refresh_snapshot "$TOOL" || true exit 0 fi @@ -113,6 +117,7 @@ if [ "$ACTION" = "uninstall" ]; then echo " • $method: $path" done fi + refresh_snapshot "$TOOL" || true exit 0 fi diff --git a/scripts/install_yarn.sh b/scripts/install_yarn.sh index c7346a4..d838fa4 100755 --- a/scripts/install_yarn.sh +++ b/scripts/install_yarn.sh @@ -3,8 +3,9 @@ set -euo pipefail DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" . "$DIR/lib/common.sh" +. "$DIR/lib/install_strategy.sh" -ACTION="${1:-update}" +ACTION="${1:-install}" check_nvm_node() { # Check if Node.js is nvm-managed (resolve symlinks) @@ -27,6 +28,7 @@ check_nvm_node() { } update_yarn() { + local allow_install="${1:-false}" ensure_nvm_loaded if ! command -v node >/dev/null 2>&1; then @@ -44,13 +46,18 @@ update_yarn() { return 1 fi + if ! command -v yarn >/dev/null 2>&1 && [ "$allow_install" != "true" ]; then + echo "[yarn] Yarn is not installed; skipping update" >&2 + return 0 + fi + local before after path before="$(yarn --version 2>/dev/null || echo '')" # Try corepack first (modern approach) if command -v corepack >/dev/null 2>&1; then echo "[yarn] Updating yarn via corepack..." >&2 - corepack enable || true + corepack enable yarn || true corepack prepare yarn@stable --activate || true else # Fallback to npm global install @@ -69,7 +76,7 @@ update_yarn() { install_yarn() { echo "[yarn] yarn should be installed via Node.js corepack/npm. Installing/updating Node.js first..." "$DIR/install_node.sh" install || true - update_yarn + update_yarn true } reconcile_yarn() { @@ -89,13 +96,26 @@ reconcile_yarn() { apt_remove_if_present cmdtest yarnpkg || true fi - update_yarn + update_yarn true } uninstall_yarn() { - echo "[yarn] yarn is managed by Node.js/npm. To remove:" >&2 - echo "[yarn] npm uninstall -g yarn" >&2 - echo "[yarn] or: corepack disable" >&2 + ensure_nvm_loaded + + if command -v npm >/dev/null 2>&1; then + npm uninstall -g yarn >/dev/null 2>&1 || true + fi + if command -v corepack >/dev/null 2>&1; then + corepack disable yarn >/dev/null 2>&1 || true + fi + + refresh_snapshot "yarn" || true + + if command -v yarn >/dev/null 2>&1; then + echo "[yarn] Warning: Yarn is still available at $(command -v yarn)" >&2 + return 1 + fi + echo "[yarn] Successfully removed" } case "$ACTION" in diff --git a/tests/test_preserve_optional_tools.py b/tests/test_preserve_optional_tools.py new file mode 100644 index 0000000..d963776 --- /dev/null +++ b/tests/test_preserve_optional_tools.py @@ -0,0 +1,138 @@ +"""Regression tests for preserving deliberately uninstalled optional tools.""" + +import os +import subprocess +import tempfile +from pathlib import Path + +import pytest + +PROJECT_ROOT = Path(__file__).parent.parent +PYTHON_INSTALLER = PROJECT_ROOT / "scripts" / "install_python.sh" +NODE_INSTALLER = PROJECT_ROOT / "scripts" / "install_node.sh" +TOOL_INSTALLER = PROJECT_ROOT / "scripts" / "install_tool.sh" +YARN_INSTALLER = PROJECT_ROOT / "scripts" / "install_yarn.sh" +POSIX_SHELL_ONLY = pytest.mark.skipif(os.name == "nt", reason="Behavioral shell tests require a POSIX host") + + +def _write_stub(directory: Path, name: str, body: str) -> Path: + stub = directory / name + stub.write_text(f"#!/usr/bin/env bash\n{body}\n") + stub.chmod(0o755) + return stub + + +def test_python_upgrade_does_not_fall_back_to_install(): + """A missing uv/pipx tool must stay missing during a stack update.""" + content = PYTHON_INSTALLER.read_text() + + assert 'uv tool upgrade -q "$p" >/dev/null 2>&1 || uv tool install' not in content + assert 'pipx upgrade "$p" >/dev/null 2>&1 || pipx install' not in content + + +def test_python_explicit_install_still_installs_cli_tools(): + """The explicit install operation must retain its existing semantics.""" + content = PYTHON_INSTALLER.read_text() + + assert 'uv tool install -q "$p"' in content + assert 'pipx install "$p"' in content + + +def test_node_update_records_optional_package_managers_before_switching_node(): + """Node updates must remember which optional managers existed beforehand.""" + content = NODE_INSTALLER.read_text() + update_body = content.split("update_node() {", 1)[1].split("\nuninstall_node() {", 1)[0] + + ensure_nvm_position = update_body.index("ensure_nvm") + assert update_body.index("command -v pnpm") < ensure_nvm_position + assert update_body.index("command -v yarn") < ensure_nvm_position + + +def test_node_update_only_prepares_previously_installed_package_managers(): + """Corepack/npm must not bring yarn or pnpm back after uninstall.""" + content = NODE_INSTALLER.read_text() + update_body = content.split("update_node() {", 1)[1].split("\nuninstall_node() {", 1)[0] + + assert 'if [ "$had_pnpm" -eq 1 ]; then' in update_body + assert 'if [ "$had_yarn" -eq 1 ]; then' in update_body + assert "corepack enable 2>" not in update_body + assert "corepack enable pnpm" in update_body + assert "corepack enable yarn" in update_body + assert update_body.index('if [ "$had_pnpm" -eq 1 ]; then') < update_body.index("corepack prepare pnpm@latest") + assert update_body.index('if [ "$had_yarn" -eq 1 ]; then') < update_body.index("corepack prepare yarn@1") + + +def test_generic_uninstall_refreshes_the_tool_snapshot(): + """Recommendations must not use stale state after an uninstall.""" + content = TOOL_INSTALLER.read_text() + uninstall_body = content.split('if [ "$ACTION" = "uninstall" ]; then', 1)[1].split( + "\n# Check if tool uses reconciliation system", 1 + )[0] + + assert '. "$DIR/lib/install_strategy.sh"' in content + assert uninstall_body.count('refresh_snapshot "$TOOL"') >= 2 + + +@POSIX_SHELL_ONLY +def test_direct_yarn_update_keeps_missing_yarn_uninstalled(): + """The dedicated `make upgrade-yarn` route must not bootstrap Yarn.""" + with tempfile.TemporaryDirectory() as tmpdir: + temp = Path(tmpdir) + home = temp / "home" + nvm_bin = home / ".nvm" / "versions" / "node" / "v26.5.0" / "bin" + nvm_bin.mkdir(parents=True) + calls = temp / "calls.log" + _write_stub(nvm_bin, "node", "echo v26.5.0") + _write_stub( + nvm_bin, + "readlink", + 'echo "$HOME/.nvm/versions/node/v26.5.0/bin/node"', + ) + _write_stub(nvm_bin, "corepack", f'echo "corepack $*" >> "{calls}"') + _write_stub(nvm_bin, "npm", f'echo "npm $*" >> "{calls}"') + + env = os.environ.copy() + env["HOME"] = str(home) + env["PATH"] = f"{nvm_bin}:/usr/bin:/bin" + result = subprocess.run( + ["bash", str(YARN_INSTALLER), "update"], + capture_output=True, + text=True, + env=env, + timeout=10, + ) + + assert result.returncode == 0, result.stderr + assert not calls.exists() or calls.read_text() == "" + assert "not installed" in (result.stdout + result.stderr).lower() + + +@POSIX_SHELL_ONLY +def test_direct_yarn_uninstall_removes_shims_and_refreshes_snapshot(): + """The dedicated `make uninstall-yarn` route must do real cleanup.""" + with tempfile.TemporaryDirectory() as tmpdir: + temp = Path(tmpdir) + home = temp / "home" + stub_bin = temp / "bin" + stub_bin.mkdir(parents=True) + calls = temp / "calls.log" + _write_stub(stub_bin, "npm", f'echo "npm $*" >> "{calls}"') + _write_stub(stub_bin, "corepack", f'echo "corepack $*" >> "{calls}"') + _write_stub(stub_bin, "python3", f'echo "python3 $*" >> "{calls}"') + + env = os.environ.copy() + env["HOME"] = str(home) + env["PATH"] = f"{stub_bin}:/usr/bin:/bin" + result = subprocess.run( + ["bash", str(YARN_INSTALLER), "uninstall"], + capture_output=True, + text=True, + env=env, + timeout=10, + ) + + assert result.returncode == 0, result.stderr + log = calls.read_text() + assert "npm uninstall -g yarn" in log + assert "corepack disable yarn" in log + assert "audit.py yarn" in log