From 4cf50b5f9eb69c219f5a082b6fa65b07019a7118 Mon Sep 17 00:00:00 2001 From: Petr Date: Fri, 25 Sep 2026 07:28:56 +0200 Subject: [PATCH 1/2] fix(update): use copy link mode for the Windows self-update install (#786) On Windows, uv's default hardlink mode fails with os error 396 when the uv cache or tool dir sits on a cloud-synced volume (OneDrive), after --force --reinstall already removed the old tool venv -- kbagent vanished. Every uv tool install kbagent builds on Windows, and the printed recovery command, now pass --link-mode copy unless UV_LINK_MODE is set. POSIX command lines are unchanged. --- .../skills/kbagent/references/gotchas.md | 22 ++++ .../services/version_service.py | 41 ++++++- tests/test_version_service.py | 114 ++++++++++++++++++ 3 files changed, 174 insertions(+), 3 deletions(-) diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index cf6fb121e..ffd5945f4 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -4286,6 +4286,28 @@ applied. `kbagent update` printed `(scheduled)` and every later launch printed redirected-output encoding all landed there. - POSIX was never affected; it uses the inline install plus re-exec. +## Windows self-update on a OneDrive / cloud-synced profile can delete kbagent + +*(since vNEXT, #786)* + +If a Windows user reports that `kbagent` vanished after a background update, +check `%LOCALAPPDATA%\keboola-agent-cli\keboola-agent-cli\pending_update.log` +for `os error 396` ("The cloud operation cannot be performed on a file with +incompatible hardlinks"). The uv cache or tool directory sits on a +cloud-synced volume that cannot hardlink, and `uv tool install --force +--reinstall` had already removed the old tool venv when the install failed. + +- **Fixed:** on Windows every self-update install -- and the recovery command + kbagent prints -- now passes `--link-mode copy`. Slower than hardlinks, but a + self-update is one-off. A `UV_LINK_MODE` the user set is respected (no flag + added). POSIX command lines are unchanged. +- **A user stranded by an older version** must reinstall with copy mode; the + printed recovery command fails identically without it. PowerShell: + + $env:UV_LINK_MODE="copy"; uv tool install --force --reinstall "keboola-cli @ https://github.com/keboola/cli/releases/download/v/keboola_cli--py3-none-any.whl" + + POSIX-style shells (Git Bash): `UV_LINK_MODE=copy uv tool install ...`. + ## Source files are read as UTF-8, not the host codepage (since v0.80.3) `lineage build` reads `transform.sql` / `code.py` as UTF-8 regardless of the diff --git a/src/keboola_agent_cli/services/version_service.py b/src/keboola_agent_cli/services/version_service.py index 2721171fa..12605ea47 100644 --- a/src/keboola_agent_cli/services/version_service.py +++ b/src/keboola_agent_cli/services/version_service.py @@ -139,6 +139,27 @@ def get_update_timeout() -> float: return float(UPDATE_TIMEOUT_SECONDS) +def _uv_link_mode_args() -> tuple[str, ...]: + """Extra ``uv tool install`` args forcing copy link mode on Windows. + + Issue #786: when the uv cache or the tool directory sits on a cloud-synced + volume (OneDrive Files-On-Demand and similar), uv's default hardlink mode + fails with ``os error 396`` (ERROR_CLOUD_FILE_INCOMPATIBLE_HARDLINKS). + By then ``--force --reinstall`` has already removed the old tool venv, so + kbagent disappears entirely -- the reporter hit this on three consecutive + background updates, and the printed recovery command failed identically + until run with ``UV_LINK_MODE=copy``. Copy mode is slower than hardlinks, + but a self-update is a one-off, so correctness wins. + + Windows only: POSIX command lines stay byte-identical (hardlinks/clones + are fine there). A user-set ``UV_LINK_MODE`` is respected -- uv reads it + itself, and an explicit flag would override the user's choice. + """ + if os.name != "nt" or os.environ.get("UV_LINK_MODE"): + return () + return ("--link-mode", "copy") + + def build_kbagent_upgrade_command( *, prerelease: bool = False, target_version: str | None = None, wheel_url: str | None = None ) -> list[str] | None: @@ -194,6 +215,7 @@ def build_kbagent_upgrade_command( cmd = [uv_path, "tool", "install", "--force", "--with", "keboola-cli[server]"] else: cmd = [uv_path, "tool", "install", "--upgrade"] + cmd.extend(_uv_link_mode_args()) if prerelease: cmd.append("--prerelease=allow") cmd.append(legacy_spec if wheel_url else legacy_source) @@ -214,7 +236,7 @@ def build_kbagent_upgrade_command( spec = f"keboola-cli{'[server]' if has_server_extras() else ''} @ {install_source}" uv_path = shutil.which("uv") if uv_path: - cmd = [uv_path, "tool", "install", "--force", "--reinstall"] + cmd = [uv_path, "tool", "install", "--force", "--reinstall", *_uv_link_mode_args()] if prerelease: cmd.append("--prerelease=allow") cmd.append(spec) @@ -243,16 +265,29 @@ def _recovery_command(command: tuple[str, ...] | None, target_version: str | Non if command is not None: executable = command[0].replace("\\", "/").rsplit("/", maxsplit=1)[-1].casefold() if executable in {"uv", "uv.exe"}: + # A uv install command built above already carries the Windows + # link-mode args (#786), so it is reused verbatim. recovery = ("uv", *command[1:]) else: prerelease = ("--prerelease=allow",) if "--pre" in command else () - recovery = ("uv", "tool", "install", "--force", "--reinstall", *prerelease, command[-1]) + recovery = ( + "uv", + "tool", + "install", + "--force", + "--reinstall", + *_uv_link_mode_args(), + *prerelease, + command[-1], + ) return _render_command(recovery) if target_version is None: return None extras = "[server]" if has_server_extras() else "" source = f"keboola-cli{extras} @ {KBAGENT_INSTALL_SOURCE}@v{target_version}" - return _render_command(("uv", "tool", "install", "--force", "--reinstall", source)) + return _render_command( + ("uv", "tool", "install", "--force", "--reinstall", *_uv_link_mode_args(), source) + ) def _fetch_kbagent_latest_version( diff --git a/tests/test_version_service.py b/tests/test_version_service.py index fc4bcad42..e7c235464 100644 --- a/tests/test_version_service.py +++ b/tests/test_version_service.py @@ -21,6 +21,19 @@ ) from keboola_agent_cli.update_runner import DeferredUpdateRequest, InstallRun, InstallStatus +VS = "keboola_agent_cli.services.version_service" + + +@pytest.fixture(autouse=True) +def _pin_uv_link_mode(monkeypatch: pytest.MonkeyPatch) -> None: + """Keep exact-command assertions host-independent (#786). + + On Windows the builders append ``--link-mode copy`` unless ``UV_LINK_MODE`` + is set, which would break the byte-exact POSIX command assertions on the + Windows CI runner. The dedicated link-mode tests override this. + """ + monkeypatch.setenv("UV_LINK_MODE", "hardlink") + class TestIsUpToDate: """Tests for _is_up_to_date().""" @@ -716,6 +729,107 @@ def test_windows_uv_executable_keeps_exact_beta_recovery_command( assert "'keboola-cli" not in recovery +class TestWindowsUvLinkMode: + """Issue #786: Windows self-update must not rely on uv hardlinks. + + On a cloud-synced volume (OneDrive) hardlinking fails with os error 396 + after uv already removed the old tool venv, stranding the user without + kbagent. Every uv command kbagent builds or prints on Windows carries + ``--link-mode copy`` unless the user set ``UV_LINK_MODE`` themselves. + """ + + WHEEL = "https://example.test/keboola_cli-1.2.3-py3-none-any.whl" + + @pytest.fixture + def windows(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(f"{VS}.os.name", "nt") + monkeypatch.delenv("UV_LINK_MODE", raising=False) + monkeypatch.setattr(f"{VS}.has_server_extras", _no_server_extras) + + @staticmethod + def _uv_only(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(f"{VS}.shutil.which", _which_uv_only) + + def test_exact_install_command_uses_copy_mode( + self, windows: None, monkeypatch: pytest.MonkeyPatch + ) -> None: + self._uv_only(monkeypatch) + cmd = build_kbagent_upgrade_command(target_version="1.2.3", wheel_url=self.WHEEL) + assert cmd == [ + r"C:\uv\uv.exe", + "tool", + "install", + "--force", + "--reinstall", + "--link-mode", + "copy", + f"keboola-cli @ {self.WHEEL}", + ] + + def test_legacy_install_command_uses_copy_mode( + self, windows: None, monkeypatch: pytest.MonkeyPatch + ) -> None: + self._uv_only(monkeypatch) + cmd = build_kbagent_upgrade_command(prerelease=True) + assert cmd is not None + idx = cmd.index("--link-mode") + assert cmd[idx + 1] == "copy" + + def test_user_uv_link_mode_is_respected( + self, windows: None, monkeypatch: pytest.MonkeyPatch + ) -> None: + self._uv_only(monkeypatch) + monkeypatch.setenv("UV_LINK_MODE", "symlink") + cmd = build_kbagent_upgrade_command(target_version="1.2.3", wheel_url=self.WHEEL) + assert cmd is not None + assert "--link-mode" not in cmd + recovery = _recovery_command(None, "1.2.3") + assert recovery is not None + assert "--link-mode" not in recovery + + def test_recovery_from_uv_command_keeps_copy_mode( + self, windows: None, monkeypatch: pytest.MonkeyPatch + ) -> None: + self._uv_only(monkeypatch) + cmd = build_kbagent_upgrade_command(target_version="1.2.3", wheel_url=self.WHEEL) + assert cmd is not None + recovery = _recovery_command(tuple(cmd), "1.2.3") + assert recovery is not None + assert recovery.startswith("uv tool install --force --reinstall --link-mode copy ") + assert recovery.count("--link-mode") == 1 + + def test_recovery_from_pip_command_uses_copy_mode(self, windows: None) -> None: + pip_cmd = ("C:\\py\\pip.exe", "install", "--upgrade", f"keboola-cli @ {self.WHEEL}") + recovery = _recovery_command(pip_cmd, "1.2.3") + assert recovery is not None + assert recovery.startswith("uv tool install --force --reinstall --link-mode copy ") + + def test_recovery_without_command_uses_copy_mode(self, windows: None) -> None: + recovery = _recovery_command(None, "1.2.3") + assert recovery is not None + assert "--link-mode copy" in recovery + + def test_posix_commands_have_no_link_mode(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(f"{VS}.os.name", "posix") + monkeypatch.delenv("UV_LINK_MODE", raising=False) + monkeypatch.setattr(f"{VS}.has_server_extras", _no_server_extras) + monkeypatch.setattr(f"{VS}.shutil.which", _which_uv_only) + cmd = build_kbagent_upgrade_command(target_version="1.2.3", wheel_url=self.WHEEL) + assert cmd is not None + assert "--link-mode" not in cmd + recovery = _recovery_command(None, "1.2.3") + assert recovery is not None + assert "--link-mode" not in recovery + + +def _no_server_extras() -> bool: + return False + + +def _which_uv_only(name: str) -> str | None: + return r"C:\uv\uv.exe" if name == "uv" else None + + class TestComposeUpdateSummary: """The one-line summary must distinguish up-to-date from a FAILED update. From 89313107607e259558ed319cf08a7948bf9d5d58 Mon Sep 17 00:00:00 2001 From: soustruh Date: Fri, 25 Sep 2026 21:35:22 +0200 Subject: [PATCH 2/2] fix(update): retry in copy link mode only after a hardlink error (#786) The first version added --link-mode copy to every Windows self-update install. That made the update slower for every Windows user. The install now keeps the default uv link mode. The background helper runs the install once more with --link-mode copy only if the first install failed and uv printed "failed to hardlink file". The match is case-sensitive. uv also prints a warning when its own fallback to copy works. That warning does not match. The printed recovery command keeps copy mode. --- .../skills/kbagent/references/gotchas.md | 15 ++- src/keboola_agent_cli/auto_update.py | 2 + src/keboola_agent_cli/constants.py | 7 ++ .../services/version_service.py | 49 ++++++--- src/keboola_agent_cli/update_runner.py | 29 +++++ tests/test_auto_update.py | 9 +- tests/test_update_runner.py | 103 ++++++++++++++++++ tests/test_version_service.py | 59 +++++++--- 8 files changed, 242 insertions(+), 31 deletions(-) diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index ffd5945f4..dc1261b19 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -4297,10 +4297,17 @@ incompatible hardlinks"). The uv cache or tool directory sits on a cloud-synced volume that cannot hardlink, and `uv tool install --force --reinstall` had already removed the old tool venv when the install failed. -- **Fixed:** on Windows every self-update install -- and the recovery command - kbagent prints -- now passes `--link-mode copy`. Slower than hardlinks, but a - self-update is one-off. A `UV_LINK_MODE` the user set is respected (no flag - added). POSIX command lines are unchanged. +- **Fixed:** the Windows self-update keeps uv's default hardlink mode. When the + install fails and uv reports a hardlink failure, the background helper runs + the same install once more with `--link-mode copy`; `pending_update.log` + holds both attempts. The recovery command kbagent prints after a failure + also passes `--link-mode copy`. A `UV_LINK_MODE` the user set is respected + (no retry, no flag). POSIX command lines are unchanged. The in-place path + (`KBAGENT_DEFER_UPDATE=0`) gets no retry. +- **The update into the fixed release still runs the old code**, which has no + retry. An affected user can set the variable permanently before that update + (PowerShell, then open a new shell): + `[Environment]::SetEnvironmentVariable('UV_LINK_MODE', 'copy', 'User')`. - **A user stranded by an older version** must reinstall with copy mode; the printed recovery command fails identically without it. PowerShell: diff --git a/src/keboola_agent_cli/auto_update.py b/src/keboola_agent_cli/auto_update.py index 88b739bee..e608ef9a9 100644 --- a/src/keboola_agent_cli/auto_update.py +++ b/src/keboola_agent_cli/auto_update.py @@ -34,6 +34,7 @@ KbagentUpdatePlan, _fetch_kbagent_latest_version, _is_up_to_date, + build_hardlink_retry_command, build_kbagent_upgrade_command, get_update_timeout, prepare_kbagent_update_plan, @@ -414,6 +415,7 @@ def _schedule_deferred_update(plan: KbagentUpdatePlan) -> None: target_version=target, install_command=plan.command, recovery_command=plan.recovery_command, + hardlink_retry_command=build_hardlink_retry_command(plan.command), ) if request_deferred_update(request): sys.stderr.write( diff --git a/src/keboola_agent_cli/constants.py b/src/keboola_agent_cli/constants.py index 72ae61ac9..750ec3c9d 100644 --- a/src/keboola_agent_cli/constants.py +++ b/src/keboola_agent_cli/constants.py @@ -456,6 +456,13 @@ def _resolve_app_name() -> str: # A marker older than this whose helper never wrote an exit file is treated as # lost (helper killed, machine rebooted mid-wait) and reported once. DEFERRED_UPDATE_STALE_SECONDS: int = 86400 +# Text in uv's error chain when a hardlink failed and uv did not fall back +# ("Caused by: failed to hardlink file from ... (os error 396)", issue #786, +# OneDrive / cloud-synced volume). The helper retries the install once in copy +# link mode only when a failed install printed this. Matched case-sensitively: +# uv's warning "Failed to hardlink files; falling back to full copy" means the +# fallback worked, so it must not trigger a retry. +DEFERRED_UPDATE_HARDLINK_FAILURE_TEXT: str = "failed to hardlink file" # --- Native (frozen / PyInstaller) distribution --- # kbagent also ships as a self-contained PyInstaller binary with NO Python diff --git a/src/keboola_agent_cli/services/version_service.py b/src/keboola_agent_cli/services/version_service.py index 12605ea47..326fb76da 100644 --- a/src/keboola_agent_cli/services/version_service.py +++ b/src/keboola_agent_cli/services/version_service.py @@ -146,13 +146,13 @@ def _uv_link_mode_args() -> tuple[str, ...]: volume (OneDrive Files-On-Demand and similar), uv's default hardlink mode fails with ``os error 396`` (ERROR_CLOUD_FILE_INCOMPATIBLE_HARDLINKS). By then ``--force --reinstall`` has already removed the old tool venv, so - kbagent disappears entirely -- the reporter hit this on three consecutive - background updates, and the printed recovery command failed identically - until run with ``UV_LINK_MODE=copy``. Copy mode is slower than hardlinks, - but a self-update is a one-off, so correctness wins. + kbagent disappears entirely. - Windows only: POSIX command lines stay byte-identical (hardlinks/clones - are fine there). A user-set ``UV_LINK_MODE`` is respected -- uv reads it + Used only on the failure path: the one-time retry after a hardlink error + (:func:`build_hardlink_retry_command`) and the printed recovery command. + The regular install keeps uv's default, so users whose disks can hardlink + do not pay for the slower copy. Windows only: POSIX command lines stay + byte-identical. A user-set ``UV_LINK_MODE`` is respected -- uv reads it itself, and an explicit flag would override the user's choice. """ if os.name != "nt" or os.environ.get("UV_LINK_MODE"): @@ -160,6 +160,30 @@ def _uv_link_mode_args() -> tuple[str, ...]: return ("--link-mode", "copy") +def _is_uv_command(command: tuple[str, ...]) -> bool: + """Whether ``command`` runs uv (as opposed to the pip fallback).""" + executable = command[0].replace("\\", "/").rsplit("/", maxsplit=1)[-1].casefold() + return executable in {"uv", "uv.exe"} + + +def build_hardlink_retry_command(command: tuple[str, ...] | None) -> tuple[str, ...] | None: + """Return ``command`` in copy link mode, for one retry after a hardlink error. + + The deferred Windows helper runs this only when the first install failed + and uv's output reports a hardlink failure (issue #786). + + Returns: + The retry argv, or ``None`` when no retry applies: no command, a pip + command, a non-Windows host, or a user-set ``UV_LINK_MODE``. + """ + if command is None or not _is_uv_command(command): + return None + link_mode = _uv_link_mode_args() + if not link_mode: + return None + return (*command[:-1], *link_mode, command[-1]) + + def build_kbagent_upgrade_command( *, prerelease: bool = False, target_version: str | None = None, wheel_url: str | None = None ) -> list[str] | None: @@ -215,7 +239,6 @@ def build_kbagent_upgrade_command( cmd = [uv_path, "tool", "install", "--force", "--with", "keboola-cli[server]"] else: cmd = [uv_path, "tool", "install", "--upgrade"] - cmd.extend(_uv_link_mode_args()) if prerelease: cmd.append("--prerelease=allow") cmd.append(legacy_spec if wheel_url else legacy_source) @@ -236,7 +259,7 @@ def build_kbagent_upgrade_command( spec = f"keboola-cli{'[server]' if has_server_extras() else ''} @ {install_source}" uv_path = shutil.which("uv") if uv_path: - cmd = [uv_path, "tool", "install", "--force", "--reinstall", *_uv_link_mode_args()] + cmd = [uv_path, "tool", "install", "--force", "--reinstall"] if prerelease: cmd.append("--prerelease=allow") cmd.append(spec) @@ -263,11 +286,10 @@ def _render_command(command: tuple[str, ...]) -> str: def _recovery_command(command: tuple[str, ...] | None, target_version: str | None) -> str | None: """Render an exact forced-reinstall command safe to copy after failure.""" if command is not None: - executable = command[0].replace("\\", "/").rsplit("/", maxsplit=1)[-1].casefold() - if executable in {"uv", "uv.exe"}: - # A uv install command built above already carries the Windows - # link-mode args (#786), so it is reused verbatim. - recovery = ("uv", *command[1:]) + if _is_uv_command(command): + # The regular install keeps uv's default link mode; the recovery + # runs after a failure, so it gets copy mode on Windows (#786). + recovery = ("uv", *command[1:-1], *_uv_link_mode_args(), command[-1]) else: prerelease = ("--prerelease=allow",) if "--pre" in command else () recovery = ( @@ -766,6 +788,7 @@ def _update_kbagent(plan: KbagentUpdatePlan) -> dict[str, Any]: target_version=kbagent_latest or old_version, install_command=plan.command, recovery_command=plan.recovery_command, + hardlink_retry_command=build_hardlink_retry_command(plan.command), ) ) if scheduled: diff --git a/src/keboola_agent_cli/update_runner.py b/src/keboola_agent_cli/update_runner.py index eca988d4d..aecbee27e 100644 --- a/src/keboola_agent_cli/update_runner.py +++ b/src/keboola_agent_cli/update_runner.py @@ -50,6 +50,7 @@ from .constants import ( DEFERRED_UPDATE_ABANDONED_MARKER, DEFERRED_UPDATE_EXIT_FILENAME, + DEFERRED_UPDATE_HARDLINK_FAILURE_TEXT, DEFERRED_UPDATE_LOG_FILENAME, DEFERRED_UPDATE_LOG_MAX_BYTES, DEFERRED_UPDATE_MARKER_FILENAME, @@ -124,6 +125,9 @@ class DeferredUpdateRequest: target_version: str install_command: tuple[str, ...] recovery_command: str | None + #: ``install_command`` in copy link mode, run once only when the first + #: install fails on a uv hardlink error (issue #786). ``None`` = no retry. + hardlink_retry_command: tuple[str, ...] | None = None class DeferredUpdateStatus(Enum): @@ -309,6 +313,30 @@ def quote_for_powershell(value: str) -> str: return "'" + value.replace("'", "''") + "'" +def _hardlink_retry_lines(request: DeferredUpdateRequest) -> list[str]: + """PowerShell lines that retry the install once in copy link mode (issue #786). + + They run only when the first install failed and its output carries uv's + hardlink error (case-sensitive ``-cmatch``, so uv's successful-fallback + warning does not count). A host that can hardlink keeps uv's faster + default. Both outputs go to the same log, with a notice between them. + """ + if request.hardlink_retry_command is None: + return [] + quoted_retry = " ".join(quote_for_powershell(part) for part in request.hardlink_retry_command) + failure_text = quote_for_powershell(DEFERRED_UPDATE_HARDLINK_FAILURE_TEXT) + notice = quote_for_powershell( + "kbagent: the install failed on a hardlink error; retrying with --link-mode copy" + ) + return [ + f" if (($code -ne 0) -and ($output -cmatch [regex]::Escape({failure_text}))) {{", + f" $output += {notice} + [Environment]::NewLine", + f" $output += (& {quoted_retry} 2>&1 | Out-String -Width 4096)", + " $code = $LASTEXITCODE", + " }", + ] + + def build_waiter_script( request: DeferredUpdateRequest, *, @@ -371,6 +399,7 @@ def build_waiter_script( f" $output = (& {quoted_argv} 2>&1 | Out-String -Width 4096)", # Captured before anything else runs, so nothing can clobber it. " $code = $LASTEXITCODE", + *_hardlink_retry_lines(request), ( " [System.IO.File]::AppendAllText(" "$logFile, $output, (New-Object System.Text.UTF8Encoding $false))" diff --git a/tests/test_auto_update.py b/tests/test_auto_update.py index 311fa5740..57cbdedfa 100644 --- a/tests/test_auto_update.py +++ b/tests/test_auto_update.py @@ -983,8 +983,12 @@ def _quiet_cache(self): @patch("keboola_agent_cli.auto_update.request_deferred_update", return_value=True) @patch("keboola_agent_cli.auto_update._perform_update") @patch("keboola_agent_cli.auto_update._re_exec") + @patch( + "keboola_agent_cli.auto_update.build_hardlink_retry_command", + return_value=("uv", "retry-in-copy-mode"), + ) def test_schedules_instead_of_installing_in_place( - self, mock_reexec, mock_perform, mock_request, mock_defer, capsys + self, mock_retry, mock_reexec, mock_perform, mock_request, mock_defer, capsys ): maybe_auto_update() @@ -993,6 +997,9 @@ def test_schedules_instead_of_installing_in_place( request = mock_request.call_args.args[0] assert request.from_version == "1.0.0" assert request.target_version == "2.0.0" + # The hardlink retry (#786) is derived from the same install command. + mock_retry.assert_called_once_with(request.install_command) + assert request.hardlink_retry_command == ("uv", "retry-in-copy-mode") assert "background" in capsys.readouterr().err @patch("keboola_agent_cli.auto_update.should_defer", return_value=True) diff --git a/tests/test_update_runner.py b/tests/test_update_runner.py index 8eec4f1e1..b75143917 100644 --- a/tests/test_update_runner.py +++ b/tests/test_update_runner.py @@ -17,6 +17,7 @@ import subprocess import sys import time +from dataclasses import replace from pathlib import Path from typing import Any @@ -211,6 +212,33 @@ def test_records_the_installer_exit_code(self, script: str) -> None: def test_a_powershell_level_error_is_recorded_as_a_failure(self, script: str) -> None: assert "Set-Content -LiteralPath $exitFile -Value 'failed'" in script + def test_runs_the_installer_once_without_a_retry_command(self, script: str) -> None: + """No retry command (pip, POSIX, user-set UV_LINK_MODE) -> one attempt (#786).""" + assert "--link-mode" not in script + assert script.count("$LASTEXITCODE") == 1 + + def test_retries_in_copy_mode_only_after_a_hardlink_failure(self, tmp_path: Path) -> None: + """Issue #786: one retry, and only when a failed install reports a hardlink error.""" + cmd = REQUEST.install_command + retry = (*cmd[:-1], "--link-mode", "copy", cmd[-1]) + script = build_waiter_script( + replace(REQUEST, hardlink_retry_command=retry), + pid=4321, + exit_file=tmp_path / DEFERRED_UPDATE_EXIT_FILENAME, + install_log=tmp_path / "install.log", + ) + guard = ( + "if (($code -ne 0) -and ($output -cmatch [regex]::Escape('failed to hardlink file'))) {" + ) + quoted_retry = " ".join(quote_for_powershell(part) for part in retry) + assert guard in script + assert f"$output += (& {quoted_retry} 2>&1 | Out-String -Width 4096)" in script + # Between the first attempt and the log write, so the recorded exit + # code and the log cover both attempts. + first_attempt = script.index("$code = $LASTEXITCODE") + log_write = script.index("AppendAllText($logFile, $output") + assert first_attempt < script.index(guard) < log_write + class TestBuildHelperCommand: """The helper must not read a profile, prompt, or show a window.""" @@ -515,6 +543,81 @@ def test_failing_installer_is_reported_not_swallowed(self, tmp_path: Path) -> No assert exit_file.read_text(encoding="utf-8").strip() == "3" + def _run_with_retry( + self, tmp_path: Path, first_install: str, retry_install: str + ) -> tuple[str, str]: + """Run the helper with a retry command; return (exit file, log).""" + exit_file = tmp_path / DEFERRED_UPDATE_EXIT_FILENAME + install_log = tmp_path / "install.log" + request = DeferredUpdateRequest( + from_version="1.0.0", + target_version="2.0.0", + install_command=(sys.executable, "-c", first_install), + recovery_command=None, + hardlink_retry_command=(sys.executable, "-c", retry_install), + ) + self._run_helper( + build_waiter_script( + request, + pid=self._already_exited_pid(), + exit_file=exit_file, + install_log=install_log, + max_wait_seconds=60, + poll_seconds=1, + process_name="kbagent", + ) + ) + return ( + exit_file.read_text(encoding="utf-8").strip(), + install_log.read_text(encoding="utf-8"), + ) + + # uv writes its errors to stderr, which PowerShell 5.1 wraps as ErrorRecords + # under `2>&1`, so the fake installers below write to stderr too. + + def test_hardlink_error_runs_the_retry_command(self, tmp_path: Path) -> None: + """Issue #786: a uv hardlink error runs the retry, and its exit code counts.""" + exit_code, log = self._run_with_retry( + tmp_path, + first_install=( + "import sys; sys.stderr.write('Caused by: failed to hardlink file from a to b: " + "The cloud operation cannot be performed (os error 396)'); sys.exit(2)" + ), + retry_install="print('RETRY-RAN')", + ) + + assert exit_code == "0" + assert "failed to hardlink file" in log + assert "retrying with --link-mode copy" in log + assert "RETRY-RAN" in log + + def test_other_install_failures_are_not_retried(self, tmp_path: Path) -> None: + """Copy mode cannot fix a network or resolver error, so no retry runs.""" + exit_code, log = self._run_with_retry( + tmp_path, + first_install=( + "import sys; sys.stderr.write('error: network unreachable'); sys.exit(3)" + ), + retry_install="print('RETRY-RAN')", + ) + + assert exit_code == "3" + assert "RETRY-RAN" not in log + + def test_the_fallback_warning_alone_does_not_trigger_the_retry(self, tmp_path: Path) -> None: + """uv's "falling back to full copy" warning means copy mode already ran.""" + exit_code, log = self._run_with_retry( + tmp_path, + first_install=( + "import sys; sys.stderr.write('warning: Failed to hardlink files; falling back " + "to full copy.\\nerror: network unreachable'); sys.exit(1)" + ), + retry_install="print('RETRY-RAN')", + ) + + assert exit_code == "1" + assert "RETRY-RAN" not in log + def test_gives_up_without_installing_while_a_process_is_still_running( self, tmp_path: Path ) -> None: diff --git a/tests/test_version_service.py b/tests/test_version_service.py index e7c235464..7ecbe2129 100644 --- a/tests/test_version_service.py +++ b/tests/test_version_service.py @@ -14,6 +14,7 @@ _fetch_kbagent_latest_version, _is_up_to_date, _recovery_command, + build_hardlink_retry_command, build_kbagent_upgrade_command, get_update_timeout, prepare_kbagent_update_plan, @@ -28,9 +29,10 @@ def _pin_uv_link_mode(monkeypatch: pytest.MonkeyPatch) -> None: """Keep exact-command assertions host-independent (#786). - On Windows the builders append ``--link-mode copy`` unless ``UV_LINK_MODE`` - is set, which would break the byte-exact POSIX command assertions on the - Windows CI runner. The dedicated link-mode tests override this. + On Windows the recovery builders append ``--link-mode copy`` unless + ``UV_LINK_MODE`` is set, which would break the byte-exact POSIX command + assertions on the Windows CI runner. The dedicated link-mode tests + override this. """ monkeypatch.setenv("UV_LINK_MODE", "hardlink") @@ -304,12 +306,17 @@ def refuse_inline(*args: object, **kwargs: object) -> InstallRun: "keboola_agent_cli.services.version_service.request_deferred_update", schedule ) monkeypatch.setattr("keboola_agent_cli.services.version_service.run_install", refuse_inline) + monkeypatch.setattr( + "keboola_agent_cli.services.version_service.build_hardlink_retry_command", + _retry_sentinel, + ) result = VersionService().self_update() assert len(requests) == 1 assert requests[0].target_version == "2.0.0" assert requests[0].install_command == self._plan().kbagent.command + assert requests[0].hardlink_retry_command == _RETRY_SENTINEL assert result["kbagent"]["deferred"] is True assert result["kbagent"]["updated"] is False # A scheduled update is not a failure and must not be summarised as one. @@ -730,12 +737,14 @@ def test_windows_uv_executable_keeps_exact_beta_recovery_command( class TestWindowsUvLinkMode: - """Issue #786: Windows self-update must not rely on uv hardlinks. + """Issue #786: copy link mode only on the Windows failure path. On a cloud-synced volume (OneDrive) hardlinking fails with os error 396 after uv already removed the old tool venv, stranding the user without - kbagent. Every uv command kbagent builds or prints on Windows carries - ``--link-mode copy`` unless the user set ``UV_LINK_MODE`` themselves. + kbagent. The regular install keeps uv's default hardlink mode, so users + whose disks can hardlink do not pay for the slower copy. Copy mode goes + only into the one-time retry after a hardlink failure and into the printed + recovery command, and never when the user set ``UV_LINK_MODE``. """ WHEEL = "https://example.test/keboola_cli-1.2.3-py3-none-any.whl" @@ -750,7 +759,7 @@ def windows(self, monkeypatch: pytest.MonkeyPatch) -> None: def _uv_only(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr(f"{VS}.shutil.which", _which_uv_only) - def test_exact_install_command_uses_copy_mode( + def test_exact_install_command_keeps_uv_default_link_mode( self, windows: None, monkeypatch: pytest.MonkeyPatch ) -> None: self._uv_only(monkeypatch) @@ -761,19 +770,34 @@ def test_exact_install_command_uses_copy_mode( "install", "--force", "--reinstall", - "--link-mode", - "copy", f"keboola-cli @ {self.WHEEL}", ] - def test_legacy_install_command_uses_copy_mode( + def test_legacy_install_command_keeps_uv_default_link_mode( self, windows: None, monkeypatch: pytest.MonkeyPatch ) -> None: self._uv_only(monkeypatch) cmd = build_kbagent_upgrade_command(prerelease=True) assert cmd is not None - idx = cmd.index("--link-mode") - assert cmd[idx + 1] == "copy" + assert "--link-mode" not in cmd + + def test_hardlink_retry_command_adds_copy_mode_before_the_spec( + self, windows: None, monkeypatch: pytest.MonkeyPatch + ) -> None: + self._uv_only(monkeypatch) + cmd = build_kbagent_upgrade_command(target_version="1.2.3", wheel_url=self.WHEEL) + assert cmd is not None + assert build_hardlink_retry_command(tuple(cmd)) == ( + *cmd[:-1], + "--link-mode", + "copy", + cmd[-1], + ) + + def test_no_hardlink_retry_for_a_pip_command(self, windows: None) -> None: + pip_cmd = ("C:\\py\\pip.exe", "install", "--upgrade", f"keboola-cli @ {self.WHEEL}") + assert build_hardlink_retry_command(pip_cmd) is None + assert build_hardlink_retry_command(None) is None def test_user_uv_link_mode_is_respected( self, windows: None, monkeypatch: pytest.MonkeyPatch @@ -783,11 +807,12 @@ def test_user_uv_link_mode_is_respected( cmd = build_kbagent_upgrade_command(target_version="1.2.3", wheel_url=self.WHEEL) assert cmd is not None assert "--link-mode" not in cmd + assert build_hardlink_retry_command(tuple(cmd)) is None recovery = _recovery_command(None, "1.2.3") assert recovery is not None assert "--link-mode" not in recovery - def test_recovery_from_uv_command_keeps_copy_mode( + def test_recovery_from_uv_command_adds_copy_mode_once( self, windows: None, monkeypatch: pytest.MonkeyPatch ) -> None: self._uv_only(monkeypatch) @@ -817,11 +842,19 @@ def test_posix_commands_have_no_link_mode(self, monkeypatch: pytest.MonkeyPatch) cmd = build_kbagent_upgrade_command(target_version="1.2.3", wheel_url=self.WHEEL) assert cmd is not None assert "--link-mode" not in cmd + assert build_hardlink_retry_command(tuple(cmd)) is None recovery = _recovery_command(None, "1.2.3") assert recovery is not None assert "--link-mode" not in recovery +_RETRY_SENTINEL = ("uv", "tool", "install", "--link-mode", "copy", "retry-sentinel") + + +def _retry_sentinel(_command: tuple[str, ...] | None) -> tuple[str, ...]: + return _RETRY_SENTINEL + + def _no_server_extras() -> bool: return False