Skip to content

Commit 044b4c0

Browse files
committed
fix: guided-upgrade --yes threads into the migration cost gate; reinstall downgrade guard reads the target venv (nexus-4nvcf, nexus-zfutt)
4nvcf: guided_upgrade_cmd never forwarded assume_yes to _run_migration, so an unattended --yes run still hit the Voyage cost confirm (Abort on non-interactive streams). One-line thread; notably, FOUR existing tests were masking the bug by asserting the kwarg's absence in the exact call shape — rewritten to assert assume_yes=True, plus a red-first pin and an interactive-path guard (no --yes still prompts). zfutt: the reinstall downgrade guard resolved 'installed version' via bare nx on ambient PATH, so a sandbox run (release-sandbox smoke from develop, whose pyproject lags the released version) read the LIVE install and refused with a spurious downgrade. Now resolves from the TARGET venv's own bin/nx (honors the active HOME/sandbox); a fresh target venv skips the comparison; real-downgrade protection and --force unchanged. New tests/scripts/ suite drives the actual bash script with stub binaries on a curated PATH (5 tests: false-refusal red case, real downgrade still refused, force bypass, fresh-venv non-comparison). 42 green across the touched suites. (Agent-implemented; orchestrator verified + committed.)
1 parent b384357 commit 044b4c0

4 files changed

Lines changed: 299 additions & 4 deletions

File tree

scripts/reinstall-tool.sh

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,22 @@ if [[ ! -f "${SOURCE}/pyproject.toml" && -f "$RECEIPT" && "$FORCE" != "1" ]]; th
7575
fi
7676
fi
7777

78-
if [[ -f "${SOURCE}/pyproject.toml" ]] && command -v nx >/dev/null 2>&1; then
78+
# nexus-zfutt: resolve the "installed" version from the TARGET venv this
79+
# invocation is about to reinstall (${VENV_DIR}/bin/nx, which honors
80+
# whatever $HOME is active — including an isolated sandbox HOME), never from
81+
# a bare `nx` lookup on the ambient $PATH. `tests/e2e/release-sandbox.sh`
82+
# activates a sandbox HOME + prepends its own bin dir to $PATH before
83+
# calling this script; on a fresh (or not-yet-populated) sandbox no `nx`
84+
# exists there yet, so a PATH-based lookup falls through to the REAL global
85+
# install and a lagging develop-branch pyproject version reads as a false
86+
# "downgrade" of an install this reinstall has nothing to do with. A missing
87+
# target-venv nx (nothing installed there yet) correctly skips the
88+
# comparison — there is nothing to downgrade.
89+
NX_BIN="${VENV_DIR}/bin/nx"
90+
if [[ -f "${SOURCE}/pyproject.toml" && -x "$NX_BIN" ]]; then
7991
SRC_VERSION="$(sed -n "s/^version *= *[\"']\([^\"']*\)[\"']/\1/p" "${SOURCE}/pyproject.toml" | head -1)"
8092
[[ -n "$SRC_VERSION" ]] || echo "warn: could not parse version from ${SOURCE}/pyproject.toml — downgrade guard inactive"
81-
INSTALLED_VERSION="$(nx --version 2>/dev/null | sed -n 's/.*version \([0-9][0-9.]*\).*/\1/p' | head -1)"
93+
INSTALLED_VERSION="$("$NX_BIN" --version 2>/dev/null | sed -n 's/.*version \([0-9][0-9.]*\).*/\1/p' | head -1)"
8294
if [[ -n "$SRC_VERSION" && -n "$INSTALLED_VERSION" ]]; then
8395
NEWEST="$(printf '%s\n%s\n' "$SRC_VERSION" "$INSTALLED_VERSION" | sort -V | tail -1)"
8496
if [[ "$SRC_VERSION" != "$INSTALLED_VERSION" && "$NEWEST" == "$INSTALLED_VERSION" && "$FORCE" != "1" ]]; then

src/nexus/commands/guided_upgrade_cmd.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,7 @@ def guided_upgrade_cmd(
339339
try:
340340
_run_migration(
341341
local_path, db_path, catalog_db_path, readiness.service_url,
342+
assume_yes=assume_yes,
342343
**_extra_kwargs,
343344
)
344345
except click.ClickException:

tests/commands/test_guided_upgrade_cmd.py

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,9 @@ def test_happy_path_provisions_migrates_and_advises(self) -> None:
202202
_args, kwargs = est.call_args
203203
assert "provision" not in kwargs
204204
# wiring: the VERIFIED url + path overrides are handed to _run_migration.
205-
mig.assert_called_once_with(None, None, None, "http://127.0.0.1:8099")
205+
mig.assert_called_once_with(
206+
None, None, None, "http://127.0.0.1:8099", assume_yes=True
207+
)
206208

207209
def test_pins_verified_endpoint_into_env_for_migration_legs(self) -> None:
208210
# nexus-qvemn: the migration's T2 store ETLs + count-source resolve the
@@ -454,6 +456,43 @@ def test_missing_token_after_provision_fails_before_migrating(self) -> None:
454456
assert "NX_SERVICE_TOKEN" in result.output
455457
mig.assert_not_called()
456458

459+
def test_yes_flag_threads_assume_yes_into_run_migration(self) -> None:
460+
"""nexus-4nvcf: `--yes` must suppress the Voyage cost-confirmation
461+
prompt inside `_run_migration`/`_confirm_voyage_cost` too, not just
462+
guided-upgrade's own top-level confirm gate. Pre-fix, `assume_yes`
463+
was never forwarded, so an unattended `--yes` run could still block
464+
on the billed-re-embed prompt."""
465+
with patch(f"{_MOD}.detect_pending_migration",
466+
return_value=_preflight(True, 2)), \
467+
patch(f"{_MOD}.establish_verified_service",
468+
return_value=_ready()), \
469+
patch("nexus.db.pg_provision.load_service_credentials_into_env",
470+
return_value=True), \
471+
patch("nexus.commands.migrate_cmd._run_migration") as mig:
472+
result = CliRunner().invoke(guided_upgrade_cmd, ["--yes"])
473+
assert result.exit_code == 0, result.output
474+
mig.assert_called_once_with(
475+
None, None, None, "http://127.0.0.1:8099", assume_yes=True,
476+
)
477+
478+
def test_without_yes_flag_run_migration_receives_assume_yes_false(self) -> None:
479+
"""The interactive path (no `--yes`) must NOT force `assume_yes=True`
480+
onto `_run_migration` — its own Voyage cost gate must remain free to
481+
prompt. guided-upgrade's own top-level confirm still fires and is
482+
answered here to reach the migration hand-off."""
483+
with patch(f"{_MOD}.detect_pending_migration",
484+
return_value=_preflight(True, 2)), \
485+
patch(f"{_MOD}.establish_verified_service",
486+
return_value=_ready()), \
487+
patch("nexus.db.pg_provision.load_service_credentials_into_env",
488+
return_value=True), \
489+
patch("nexus.commands.migrate_cmd._run_migration") as mig:
490+
result = CliRunner().invoke(guided_upgrade_cmd, [], input="y\n")
491+
assert result.exit_code == 0, result.output
492+
mig.assert_called_once_with(
493+
None, None, None, "http://127.0.0.1:8099", assume_yes=False,
494+
)
495+
457496
def test_bad_service_url_is_rejected_before_migrating(self) -> None:
458497
with patch(f"{_MOD}.detect_pending_migration",
459498
return_value=_preflight(True, 1)), \
@@ -500,6 +539,7 @@ def test_skip_stores_are_echoed_and_forwarded(self) -> None:
500539
assert det.call_args.kwargs["force"] is False
501540
mig.assert_called_once_with(
502541
None, None, None, "http://127.0.0.1:8099",
542+
assume_yes=True,
503543
skip_t2_stores=frozenset({"memory", "plans"}),
504544
)
505545

@@ -516,7 +556,9 @@ def test_nothing_skipped_reproduces_the_exact_prior_call_shape(self) -> None:
516556
assert result.exit_code == 0, result.output
517557
assert "already migrated" not in result.output
518558
# No skip_t2_stores kwarg at all when nothing is skipped.
519-
mig.assert_called_once_with(None, None, None, "http://127.0.0.1:8099")
559+
mig.assert_called_once_with(
560+
None, None, None, "http://127.0.0.1:8099", assume_yes=True
561+
)
520562

521563
def test_force_flag_is_forwarded_to_detection(self) -> None:
522564
plan = self._plan(skip=frozenset())
@@ -549,5 +591,6 @@ def test_all_skipped_still_migrates_for_the_t3_leg(self) -> None:
549591
est.assert_called_once() # still provisions — T3 leg is untouched by this bead
550592
mig.assert_called_once_with(
551593
None, None, None, "http://127.0.0.1:8099",
594+
assume_yes=True,
552595
skip_t2_stores=frozenset(LADDER_ORDER),
553596
)
Lines changed: 239 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,239 @@
1+
# SPDX-License-Identifier: AGPL-3.0-or-later
2+
# Copyright (c) 2026 Hal Hildebrand. All rights reserved.
3+
"""``scripts/reinstall-tool.sh`` downgrade guard — resolve from the TARGET
4+
venv, not the ambient PATH (nexus-zfutt).
5+
6+
Root cause: the guard read the "installed" version via a bare ``nx --version``
7+
lookup on ``$PATH``. ``tests/e2e/release-sandbox.sh`` activates an isolated
8+
sandbox ``$HOME`` and PREPENDS ``$SANDBOX/.local/bin`` to ``$PATH`` before
9+
calling this script — but on a fresh (or not-yet-populated) sandbox no ``nx``
10+
exists there yet, so ``command -v nx`` falls through to the REST of ``$PATH``
11+
and resolves the live global install instead. A develop checkout's
12+
``pyproject.toml`` version (which lags the last released version between
13+
releases) then reads as a "downgrade" against the live global version, and the
14+
guard refuses a perfectly legitimate isolated sandbox install.
15+
16+
These tests never touch the real global ``uv`` tool environment: a stub
17+
``uv`` (handling only ``tool dir``, no-op otherwise) and stub ``nx`` binaries
18+
are placed on ``$PATH`` ahead of everything else, and ``$HOME`` points at a
19+
throwaway ``tmp_path``.
20+
"""
21+
from __future__ import annotations
22+
23+
import os
24+
import stat
25+
import subprocess
26+
from pathlib import Path
27+
28+
import pytest
29+
30+
_SCRIPT = Path(__file__).resolve().parents[2] / "scripts" / "reinstall-tool.sh"
31+
32+
# A curated base PATH for these subprocess runs — deliberately EXCLUDES
33+
# ~/.local/bin (and any other uv-tool-managed bin dir), which is where the
34+
# REAL `nx` / `uv` live on a dev box. These tests must never be able to reach
35+
# the real global install (fewer-permission-prompts / "don't break the live
36+
# nexus install"); only the stub `uv`/`nx` binaries these tests place on
37+
# PATH may be found. Includes just enough of the real system PATH for
38+
# bash/git/python3/sed/grep/ps to resolve.
39+
_SAFE_BASE_PATH = ":".join(
40+
p for p in (
41+
"/opt/homebrew/bin",
42+
"/opt/homebrew/opt/python@3.13/libexec/bin",
43+
"/usr/local/bin",
44+
"/usr/bin",
45+
"/bin",
46+
"/usr/sbin",
47+
"/sbin",
48+
)
49+
if Path(p).is_dir()
50+
)
51+
52+
53+
def _make_executable(path: Path, content: str) -> None:
54+
path.write_text(content)
55+
path.chmod(path.stat().st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH)
56+
57+
58+
def _stub_uv(bin_dir: Path, *, tool_dir: Path, marker: Path) -> None:
59+
"""A stub ``uv`` that answers ``tool dir`` and, for anything else
60+
(notably ``tool install``), just drops a marker file so tests can
61+
assert whether the real install step was ever reached."""
62+
_make_executable(
63+
bin_dir / "uv",
64+
f"""#!/bin/bash
65+
if [[ "$1" == "tool" && "$2" == "dir" ]]; then
66+
echo "{tool_dir}"
67+
exit 0
68+
fi
69+
if [[ "$1" == "tool" && "$2" == "install" ]]; then
70+
touch "{marker}"
71+
exit 0
72+
fi
73+
exit 0
74+
""",
75+
)
76+
77+
78+
def _stub_nx(path: Path, version: str) -> None:
79+
_make_executable(
80+
path,
81+
f"""#!/bin/bash
82+
if [[ "$1" == "--version" ]]; then
83+
echo "nx, version {version}"
84+
exit 0
85+
fi
86+
exit 0
87+
""",
88+
)
89+
90+
91+
def _write_pyproject(source_dir: Path, version: str) -> None:
92+
source_dir.mkdir(parents=True, exist_ok=True)
93+
(source_dir / "pyproject.toml").write_text(
94+
f'[project]\nname = "conexus"\nversion = "{version}"\n'
95+
)
96+
97+
98+
def _run(env_path: str, home: Path, source: Path) -> subprocess.CompletedProcess:
99+
env = dict(os.environ)
100+
env["PATH"] = env_path
101+
env["HOME"] = str(home)
102+
return subprocess.run(
103+
["bash", str(_SCRIPT), str(source)],
104+
env=env,
105+
capture_output=True,
106+
text=True,
107+
timeout=30,
108+
)
109+
110+
111+
class TestDowngradeGuardResolvesFromTargetVenv:
112+
def test_does_not_falsely_refuse_when_only_ambient_path_nx_is_newer(
113+
self, tmp_path: Path
114+
) -> None:
115+
"""The bug: a fresh/isolated target venv has no ``nx`` of its own
116+
yet, but the REST of ``$PATH`` (the live global install) still
117+
resolves to a much newer ``nx``. That must NOT read as a downgrade —
118+
there is nothing installed in the target venv to downgrade."""
119+
home = tmp_path / "sandbox-home"
120+
home.mkdir()
121+
tool_dir = home / "tools" # VENV_DIR = tool_dir/conexus — deliberately NOT populated
122+
marker = tmp_path / "install-ran.marker"
123+
124+
stub_bin = tmp_path / "stubbin"
125+
stub_bin.mkdir()
126+
_stub_uv(stub_bin, tool_dir=tool_dir, marker=marker)
127+
128+
# The "ambient" live global install: a DIFFERENT, later-in-PATH nx
129+
# reporting a much newer version than the source checkout.
130+
global_bin = tmp_path / "globalbin"
131+
global_bin.mkdir()
132+
_stub_nx(global_bin / "nx", "6.16.0")
133+
134+
source = tmp_path / "checkout"
135+
_write_pyproject(source, "6.11.0")
136+
137+
env_path = f"{stub_bin}:{global_bin}:{_SAFE_BASE_PATH}"
138+
result = _run(env_path, home, source)
139+
140+
assert result.returncode == 0, result.stdout + result.stderr
141+
assert "REFUSING" not in result.stdout
142+
assert marker.exists() # the (stubbed) install actually ran
143+
144+
def test_still_refuses_a_real_downgrade_of_the_target_venv(
145+
self, tmp_path: Path
146+
) -> None:
147+
"""The guard's real purpose must survive: when the TARGET venv's own
148+
``nx`` (the one about to be overwritten) is genuinely ahead of the
149+
source checkout, refuse without ``--force``."""
150+
home = tmp_path / "real-home"
151+
home.mkdir()
152+
tool_dir = home / "tools"
153+
venv_bin = tool_dir / "conexus" / "bin"
154+
venv_bin.mkdir(parents=True)
155+
marker = tmp_path / "install-ran.marker"
156+
157+
stub_bin = tmp_path / "stubbin"
158+
stub_bin.mkdir()
159+
_stub_uv(stub_bin, tool_dir=tool_dir, marker=marker)
160+
161+
# The nx actually installed in the target venv — this is the one
162+
# about to be overwritten, and the one the guard must protect.
163+
_stub_nx(venv_bin / "nx", "6.16.0")
164+
165+
source = tmp_path / "checkout"
166+
_write_pyproject(source, "6.11.0")
167+
168+
env_path = f"{stub_bin}:{_SAFE_BASE_PATH}"
169+
result = _run(env_path, home, source)
170+
171+
assert result.returncode == 1, result.stdout + result.stderr
172+
assert "REFUSING" in result.stdout
173+
assert "DOWNGRADE" in result.stdout
174+
assert not marker.exists() # never reached the install step
175+
176+
def test_force_bypasses_a_real_downgrade(self, tmp_path: Path) -> None:
177+
home = tmp_path / "real-home"
178+
home.mkdir()
179+
tool_dir = home / "tools"
180+
venv_bin = tool_dir / "conexus" / "bin"
181+
venv_bin.mkdir(parents=True)
182+
marker = tmp_path / "install-ran.marker"
183+
184+
stub_bin = tmp_path / "stubbin"
185+
stub_bin.mkdir()
186+
_stub_uv(stub_bin, tool_dir=tool_dir, marker=marker)
187+
_stub_nx(venv_bin / "nx", "6.16.0")
188+
189+
# A PATH-reachable nx too — needed for the script's own post-install
190+
# `nx --version` echo (unrelated to the guard under test here; the
191+
# guard itself no longer needs anything on PATH).
192+
global_bin = tmp_path / "globalbin"
193+
global_bin.mkdir()
194+
_stub_nx(global_bin / "nx", "6.16.0")
195+
196+
source = tmp_path / "checkout"
197+
_write_pyproject(source, "6.11.0")
198+
199+
env = dict(os.environ)
200+
env["PATH"] = f"{stub_bin}:{global_bin}:{_SAFE_BASE_PATH}"
201+
env["HOME"] = str(home)
202+
result = subprocess.run(
203+
["bash", str(_SCRIPT), str(source), "--force"],
204+
env=env, capture_output=True, text=True, timeout=30,
205+
)
206+
207+
assert result.returncode == 0, result.stdout + result.stderr
208+
assert marker.exists()
209+
210+
@pytest.mark.parametrize("global_version", ["6.16.0", "6.10.0"])
211+
def test_fresh_target_venv_never_compares_against_ambient_path(
212+
self, tmp_path: Path, global_version: str
213+
) -> None:
214+
"""Regardless of whether the stray ambient ``nx`` happens to look
215+
newer or older than the source checkout, a genuinely empty target
216+
venv must never trigger the guard at all — there is nothing there to
217+
compare against."""
218+
home = tmp_path / "sandbox-home"
219+
home.mkdir()
220+
tool_dir = home / "tools"
221+
marker = tmp_path / "install-ran.marker"
222+
223+
stub_bin = tmp_path / "stubbin"
224+
stub_bin.mkdir()
225+
_stub_uv(stub_bin, tool_dir=tool_dir, marker=marker)
226+
227+
global_bin = tmp_path / "globalbin"
228+
global_bin.mkdir()
229+
_stub_nx(global_bin / "nx", global_version)
230+
231+
source = tmp_path / "checkout"
232+
_write_pyproject(source, "6.11.0")
233+
234+
env_path = f"{stub_bin}:{global_bin}:{_SAFE_BASE_PATH}"
235+
result = _run(env_path, home, source)
236+
237+
assert result.returncode == 0, result.stdout + result.stderr
238+
assert "REFUSING" not in result.stdout
239+
assert marker.exists()

0 commit comments

Comments
 (0)