fix(cli): flash update works from any dir and for uv tool installs

[deanq]

Problem

flash update fails when run from any directory that is not a virtual environment root, and always fails when flash was installed via uv tool install runpod-flash:

Current version: 1.18.0
Installing runpod-flash 1.19.0...
✗ uv install failed (exit 2): error: No virtual environment found; run `uv venv` to create an environment, or pass `--system` to install into a non-virtual environment

Root cause

_build_install_command ran uv pip install runpod-flash==X whenever uv was on PATH. uv pip install discovers its target virtual environment from the current working directory (./.venv) or $VIRTUAL_ENV. A uv tool installed flash lives in an isolated environment under uv tool dir with no ambient venv, so uv exits 2. Venv installs hit the same failure whenever flash update runs from another directory.

Fix

Select the install mechanism from how flash was actually installed, and always target flash's own environment instead of a cwd-discovered venv:

Install method	Command
uv tool install	uv tool install runpod-flash==X --force --quiet
uv venv	uv pip install runpod-flash==X --python <sys.executable> --quiet
no uv	python -m pip install … (unchanged)

_is_uv_tool_install() compares sys.prefix against uv tool dir, failing closed to the pip path if uv is absent or errors.

Test plan

• TDD: detection and per-branch command tests added; verified red before green
• Detection run from a real uv tool interpreter returns True; from a venv returns False and targets sys.executable
• make quality-check passes (format, lint, 53 passed/1 skipped, coverage 86.5%)
• mypy clean on the changed file

Closes SLS-605

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟡 Changes recommended

System-interpreter pip installations can still be routed through uv pip, leaving that supported setup broken.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Avoid using uv pip for system interpreter installs · New

What changed in this PR

Updates flash update to work from any directory and support uv tool installations.

Changes:

• Detects uv tool environments.
• Selects targeted uv tool, uv pip, or standard pip commands.
• Adds tests for installation detection and command selection.

File	Summary
tests/​unit/​cli/​commands/​test_update.py	Tests installer detection and command generation.
src/​runpod_flash/​cli/​commands/​update.py	Implements environment-aware installation commands.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[daveseddon-runpod]

Nice work on this one, @deanq — the writeup is a model PR description. The problem statement, root-cause analysis, and the install-matrix table made it easy to follow exactly what changes and why, and the TDD note (red-before-green) is great to see. 👏

Here's my review, ordered from what I liked → most important → nice-to-haves. For context, I checked out the branch and ran it through ruff (--select ALL), mypy --strict, bandit, and the test suite with coverage; details at the bottom.

What's great

• Fails-closed detection. _is_uv_tool_install() returns False on any error (uv missing, non-zero exit, empty output), so a broken/absent uv degrades gracefully to the pip path instead of crashing the updater. Exactly the right default.
• Security-conscious subprocess use. List-form args (no shell=True), explicit timeout on both calls, and the version comes from the validated PyPI releases set rather than raw user input — no injection surface.
• The uv pip --python sys.executable insight is genuinely good. Reaching for uv pip (not python -m pip) in the venv case is the correct call precisely because uv-created venvs don't ship pip — worth calling out so it doesn't get "simplified" away later.
• Modern, clean typing: list[str], tuple[int, ...], pathlib.Path, Path.is_relative_to. mypy --strict comes back completely clean on the file.

Most important — one gap in the "works from any dir" promise 🔴

This is the same thing the Copilot bot flagged, and I think it's right. The documented pip install runpod-flash path, running under a system interpreter while uv also happens to be on PATH (a very common developer setup), gets routed to:

uv pip install runpod-flash==X --python <system python>

uv pip install against a non-venv interpreter still errors with the same No virtual environment found (it wants --system), so flash update stays broken for that supported install method. The fix works great for uv-tool and uv/venv installs, just not this last case.

The smallest fix that closes it is to gate the uv pip branch on an actual virtualenv (PEP 405 idiom) and let system interpreters fall through to plain pip — which has pip and gives a clear PEP-668 message if the environment is externally managed:

 package_spec = f"runpod-flash=={version}"
 if shutil.which("uv"):
     if _is_uv_tool_install():
         return ["uv", "tool", "install", package_spec, "--force", "--quiet"]
-    return [
-        "uv",
-        "pip",
-        "install",
-        package_spec,
-        "--python",
-        sys.executable,
-        "--quiet",
-    ]
+    if sys.prefix != sys.base_prefix:  # inside a virtualenv (PEP 405)
+        return [
+            "uv",
+            "pip",
+            "install",
+            package_spec,
+            "--python",
+            sys.executable,
+            "--quiet",
+        ]
 return [sys.executable, "-m", "pip", "install", package_spec, "--quiet"]

This keeps the "uv venvs lack pip" rationale intact for the venv case while making the system-interpreter case actually work.

Tests — one real coverage gap + a table-driven suggestion 🟠

Coverage gap (worth closing): _is_uv_tool_install's if result.returncode != 0: return False branch (uv present but uv tool dir fails, e.g. a broken uv) is never exercised — the file sits at 97% with that line the notable miss. subprocess.SubprocessError/TimeoutExpired is also only reached via the OSError case, not directly.

Suggestion — convert the two new test classes to parametrized tables. The new TestIsUvToolInstall/TestBuildInstallCommand are one-method-per-case; a @pytest.mark.parametrize table with explicit description + expected columns makes the positive / negative / boundary / corner coverage legible at a glance and is easy to extend. Parametrize is already used elsewhere in this tree (e.g. tests/unit/cli/test_main.py), so it's consistent with the repo. Here's a drop-in that also adds the missing branches (non-zero exit, timeout) and the boundary case (prefix == tool dir), plus the new "uv + system interpreter" row for the fix above:

TOOL_DIR = "/home/u/.local/share/uv/tools"


class TestIsUvToolInstall:
    @pytest.mark.parametrize(
        "description, run_result, prefix, expected",
        [
            (
                "positive: prefix under tool dir -> tool install",
                MagicMock(returncode=0, stdout=f"{TOOL_DIR}\n"),
                f"{TOOL_DIR}/runpod-flash",
                True,
            ),
            (
                "negative: venv prefix outside tool dir -> not a tool install",
                MagicMock(returncode=0, stdout=f"{TOOL_DIR}\n"),
                "/home/u/project/.venv",
                False,
            ),
            (
                "boundary: prefix equals tool dir exactly (is_relative_to self)",
                MagicMock(returncode=0, stdout=f"{TOOL_DIR}\n"),
                TOOL_DIR,
                True,
            ),
            (
                "corner: `uv tool dir` exits non-zero -> fail closed",
                MagicMock(returncode=1, stdout=""),
                "/irrelevant",
                False,
            ),
            (
                "corner: `uv tool dir` empty output -> fail closed",
                MagicMock(returncode=0, stdout="\n"),
                "/irrelevant",
                False,
            ),
        ],
    )
    def test_detection(self, description, run_result, prefix, expected):
        with (
            patch(
                "runpod_flash.cli.commands.update.subprocess.run",
                return_value=run_result,
            ),
            patch("runpod_flash.cli.commands.update.sys.prefix", prefix),
        ):
            assert _is_uv_tool_install() is expected, description

    @pytest.mark.parametrize(
        "description, exc",
        [
            (
                "corner: uv missing raises OSError -> fail closed",
                OSError("uv not found"),
            ),
            (
                "corner: `uv tool dir` times out -> fail closed",
                subprocess.TimeoutExpired(cmd="uv", timeout=10),
            ),
        ],
    )
    def test_detection_subprocess_errors(self, description, exc):
        with patch(
            "runpod_flash.cli.commands.update.subprocess.run", side_effect=exc
        ):
            assert _is_uv_tool_install() is False, description


class TestBuildInstallCommand:
    @pytest.mark.parametrize(
        "description, which_uv, is_tool, in_venv, expected",
        [
            (
                "positive: uv tool install -> `uv tool install --force`",
                "/usr/bin/uv",
                True,
                True,
                ["uv", "tool", "install", "runpod-flash==1.5.0", "--force", "--quiet"],
            ),
            (
                "positive: uv + venv -> `uv pip install` targets sys.executable",
                "/usr/bin/uv",
                False,
                True,
                [
                    "uv",
                    "pip",
                    "install",
                    "runpod-flash==1.5.0",
                    "--python",
                    sys.executable,
                    "--quiet",
                ],
            ),
            (
                "corner: uv present but system interpreter -> plain pip fallback",
                "/usr/bin/uv",
                False,
                False,
                [
                    sys.executable,
                    "-m",
                    "pip",
                    "install",
                    "runpod-flash==1.5.0",
                    "--quiet",
                ],
            ),
            (
                "negative: no uv on PATH -> plain pip fallback",
                None,
                False,
                False,
                [
                    sys.executable,
                    "-m",
                    "pip",
                    "install",
                    "runpod-flash==1.5.0",
                    "--quiet",
                ],
            ),
        ],
    )
    def test_command_selection(
        self, description, which_uv, is_tool, in_venv, expected
    ):
        prefix = "/home/u/project/.venv" if in_venv else sys.base_prefix
        with (
            patch(
                "runpod_flash.cli.commands.update.shutil.which", return_value=which_uv
            ),
            patch(
                "runpod_flash.cli.commands.update._is_uv_tool_install",
                return_value=is_tool,
            ),
            patch("runpod_flash.cli.commands.update.sys.prefix", prefix),
        ):
            assert _build_install_command("1.5.0") == expected, description

I applied both the source fix and these tests locally: 44 passed, update.py coverage rises to 98% (the returncode != 0 line is now covered), and mypy --strict / ruff format stay clean.

Lower priority / optional 🟢

• Bump the supported Python range. requires-python = ">=3.10,<3.14" currently excludes 3.14, and the classifiers stop at 3.13. Since we're moving toward 3.14, it'd be good to raise the ceiling (e.g. <3.15), add the 3.14 classifier, and bump the ruff target-version / mypy python_version. Nothing in this PR blocks 3.14 — I ran the analysis with a 3.14 baseline and it's clean.
• Optional[str] → str | None in update_command (pre-existing, UP045) — the 3.10+ idiom, if you're touching the file anyway.
• check=False on the new subprocess.run in _is_uv_tool_install — you handle returncode explicitly, so adding the explicit check=False just documents that intent and silences PLW1510. Purely cosmetic.
• Consistency note (not for this PR): build.py's install_dependencies is the only other install-command builder, and it does not share this bug (it always installs to an explicit --target). But it detects uv differently (a uv pip --version probe, pip-first) vs. this file's shutil.which + uv-first. Might be worth converging on one small helper down the road.

Security 🔒

No concerns. bandit flags only the expected low/medium blacklist items — B404 (imports subprocess), B603/B607 (subprocess with a partial uv path), and B310 (urlopen on the hardcoded https://pypi.org constant). All are benign here: args are list-form with no shell, the URL is a fixed constant, and the version is validated against PyPI's release set before use. No new attack surface, and the table above proves the command builder only ever emits the intended argv.

Analysis performed

• ruff check --select ALL (then a curated strict subset) + ruff format --check — new code clean; only opinionated/pre-existing items remain.
• mypy --strict (baseline 3.14) — clean on update.py.
• bandit — reviewed, all findings benign (see above).
• pytest + coverage — 40 passing on the branch (97% file coverage); 44 passing / 98% with the suggested fix and tests applied.
• (No shell scripts in the diff, so shellcheck N/A; Python has no race/benchmark harness to run here.)

Overall this is a solid, well-tested fix that clearly improves flash update — the venv-vs-system-interpreter gate is the one thing I'd want before merge, and the rest is polish. Thanks for the careful work! 🚀