fix(update): defer the Windows self-update out of the running environment (#528) - #543
Merged
Merged
Conversation
…ment (#528) `uv tool install` recreates a tool environment by removing it and then building a fresh venv at the same path (`uv-tool/src/lib.rs`: create_environment -> "Remove any existing environment" -> create_venv). It is not atomic and has no rollback. On POSIX that is harmless -- unlinking a file another process holds open leaves that process's inode intact. On Windows uv's `kbagent.exe` trampoline loads the venv interpreter in-process, so those files are locked: the removal deletes what it can, hits a locked file, and aborts, leaving a gutted venv. That is the reported failure -- `rich` present but `rich/_windows.py` gone, `typer` present but `typer/rich_utils.py` gone. Upstream: astral-sh/uv#11930. The v0.76.2 fix reordered discovery ahead of mutation, which was a real ordering bug, but the installer still ran from inside the environment it replaces -- so the corruption survived it and reproduced on 0.76.2 -> 0.76.3. Windows now hands the reinstall to a detached PowerShell helper that waits for every kbagent process to exit and only then installs; the outcome is reported once by the next launch, with a recovery command on failure. When no helper can be spawned the user gets the exact command -- never a fallback to the unsafe inline install. POSIX keeps the inline install plus re-exec unchanged. Also removes a second, independent corruption vector: both paths ran the installer through `subprocess.run(timeout=...)`, which kills the child on expiry -- on Windows a hard TerminateProcess of uv mid-write, producing the same half-deleted environment from our own code. The deadline now bounds only how long kbagent waits. `KBAGENT_DEFER_UPDATE=1|0` overrides the platform default.
…528) The waiter is a PowerShell script authored on machines that cannot execute it, so the unit tests can only pin its text. The repo already runs a windows-latest job for the #320/#529 regressions; extend it to run the update-runner suite, plus three Windows-only tests that execute the generated script against a stand-in installer: - the installer's exit code is recorded, - a failing installer is reported rather than swallowed, - nothing is installed while a watched process is still alive -- the branch that actually protects the environment. This also exercises `should_defer()` returning its real Windows default and a real detached spawn.
Caught by the Windows CI job added in the previous commit -- the only place the helper actually executes. Windows PowerShell 5.1 writes redirection operators (`>`, `>>`, `*>>`) as UTF-16LE with a BOM. The helper redirected the installer's output into the shared install log with `*>>`, but every other writer and reader of that file -- `run_install`, `_tail`, and the user we point at it -- assumes UTF-8. The log came out as mojibake. The installer's output now goes through `[System.IO.File]::AppendAllText` with `UTF8Encoding($false)`, so the log is plain UTF-8 with no BOM regardless of writer. `$LASTEXITCODE` is captured into `$code` immediately after the installer, before anything else can run. The Windows test now asserts the log is UTF-8 and carries no UTF-16 BOM, so this cannot regress silently.
…ported (#528) _classify_exit took `marker_is_stale`, but the caller passed `raw_exit is None` -- which is also true when the exit file exists and merely could not be read. That case was labelled LOST ("the helper vanished") when what actually happened is that the helper reported something unusable, i.e. a FAILED install. The predicate is now `helper_reported`, derived from whether an exit file existed at all, so each status means what it says. Both already offered recovery, so this changes wording rather than behaviour -- but the parameter was lying about its own meaning.
The no-kill fix covered the two kbagent paths but left the MCP upgrade two functions away still on `subprocess.run(timeout=...)`, which terminates uv at 180s. The MCP environment is not the one kbagent runs from, so no file lock is involved -- but a killed installer leaves it half-recreated exactly the same way, and what stops working is `kbagent tool call`. `_perform_mcp_update` now goes through `run_install`, so the deadline bounds only how long kbagent waits. A slow upgrade reports "still running" instead of the untrue "timed out". Read-only probes (`keboola_mcp_server --version`, `uv tool list`) keep their ordinary timeouts -- killing a probe is harmless.
… a failure (#528) Four review findings, all cases of a message not matching what happened. 1. `run_install` appended to one shared log and reported `_tail` of the whole file, so a run's `output` carried text it did not produce. This regressed the pre-change `capture_output=True` per-run capture, and routing the MCP upgrade through the same runner made it reachable within a single `kbagent update`: the MCP stage writes seconds before the kbagent stage reads. The offset the run starts at is now recorded before spawning, and only bytes past it are read. The log itself still keeps every run -- that is the diagnosis -- but is rolled once it passes a size cap, since every update appends to it forever. 2. A STILL_RUNNING install fell through to `_compose_update_summary`'s failure branch and rendered as "kbagent v0.77.0 update FAILED: Update still running ...", contradicting itself mid-sentence and the banner `auto_update` already got right. It now carries an explicit `still_running` flag and reads "(still installing)". 3. The extra human-mode stage message fired on the unschedulable branch too, whose message the summary already quotes verbatim -- the user was told the same thing twice. Now gated to the scheduled case, which is the only one the summary truncates to "(scheduled)". 4. `_classify_exit` returned a bare two-element tuple of semantically distinct values, which CONTRIBUTING.md forbids for new code. It returns a frozen `ClassifiedExit` instead. Also strengthens the no-kill test to assert the log is readable while the child still holds it open and the parent has closed its handle -- the Windows file-sharing question raised in review, now answered by the Windows CI job rather than by argument.
This was referenced Aug 1, 2026
This was referenced Aug 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #528.
Why v0.76.2 did not fix this
uv tool installrecreates a tool environment in place. Fromcrates/uv-tool/src/lib.rs,create_environmentremoves any existing environment and then callscreate_venvat the same path. No temp directory, no atomic swap, no rollback.
On POSIX that is safe by accident of filesystem semantics: unlinking a file another
process holds open leaves that process's inode intact, so a running kbagent survives
having its venv deleted and rebuilt underneath it. That is why we never saw this.
On Windows it cannot work. uv's
kbagent.exetrampoline loads the tool venv'sinterpreter in-process, so those files are locked for as long as kbagent runs. The
removal deletes every file it can, reaches a locked one, and aborts. What is left is
not a mixture of old and new distributions — it is a partially deleted venv,
exactly matching the reported symptoms:
richpresent butrich/_windows.pygone,typerpresent buttyper/rich_utils.pygone. Upstream tracks the same class offailure as astral-sh/uv#11930.
v0.76.2 fixed a real ordering bug (the missing
certifi/cacert.pemin incident 2)but still ran the installer from inside the environment it replaces — so the
corruption survived it and reproduced on 0.76.2 → 0.76.3.
Ordering was never the mechanism. Running the installer from inside the target
environment is unsafe on Windows by construction, with any combination of uv flags.
Second, independent corruption vector
Both paths ran the installer through
subprocess.run(..., timeout=...), whichkills the child on expiry — on Windows a hard
TerminateProcessof uv part-waythrough recreating a venv. That produces the identical half-deleted environment,
from our own code, on every platform. Default deadline was 300s; a cold resolution
of the
[server]extra on a Windows box with real-time AV scanning is not reliablyunder that.
What this changes
1. Windows installs after we are gone. The prepared install command is handed to
a detached PowerShell helper that waits twice —
Wait-Process -Id <pid>for thescheduling process (which may not be named
kbagent, e.g.python -m keboola_agent_cli), then aGet-Process -Name kbagentloop for every otherkbagent holding the environment open — and only then installs. If kbagent never
exits within the window it installs nothing and says so; doing nothing is always
safe here, a partial install never is.
Windows PowerShell 5.1 is an in-box OS component on every supported Win10/11 edition
and cannot be uninstalled, and
ExecutionPolicygoverns script files, not-Commandstrings, so no policy can block it. If no interpreter is found, schedulingfails and the user gets the exact command — never a fallback to the in-place
install.
A single-flight guard stops several shells opened in a row from each spawning a
helper and racing each other into the very corruption this prevents.
2. The installer is never killed. The deadline now bounds only how long kbagent
waits; uv is left to finish the transaction it started. The banner says the install
is still running and deliberately offers no recovery command — a second installer
aimed at an environment a live uv is rewriting is the corruption, not the cure.
This covers the
keboola-mcp-serverupgrade too. That environment is not the onekbagent runs from, so no file lock is involved — but it sat two functions away on the
same
subprocess.run(timeout=...), and a killed installer leaves it half-recreatedjust the same; what stops working there is
kbagent tool call. Read-only probes(
keboola_mcp_server --version,uv tool list) keep their ordinary timeouts, becausekilling a probe is harmless.
3. The outcome is reported once, by the next launch. The process that schedules
an update exits before it runs, so it can never report it.
report_finished_deferred_update()runs before the skip gates — the user is owedthe result even on a run that will not update anything (dev install, opt-out,
kbagent version). Outcomes are distinguished so they read correctly:SUCCEEDEDABANDONEDFAILEDLOSTAn unparseable result counts as a failure: that is precisely when the recovery
command matters, so it must not be optimistic.
4. POSIX is untouched.
should_defer()is False there, so the inline installplus
os.execvpere-exec stays exactly as it is —execvegenuinely replaces theprocess image and unlinking open files is safe. The majority path keeps its instant
upgrade with no new failure mode.
KBAGENT_DEFER_UPDATE=1|0overrides the platformdefault.
User-visible change on Windows
The new version becomes active on the next launch, not the current one.
kbagent update --jsonmarks this withkbagent.deferred: trueand the summary reads(scheduled)— which is not a failure and is not rendered as one.Testing
make checkgreen: 4730 passed. Newtests/test_update_runner.py(36 tests) plusextensions to the two existing suites.
Verifiable on POSIX CI, and covered:
run_installleaves a slow child alive — a real subprocess writing a sentinelafter the wait expires. This is the one behaviour directly observable everywhere,
and it is the regression test for vector 2.
ordering, the quoted install argv, exit-code recording.
refusal, spawn-failure cleanup.
kbagent updateon the deferred platform never reachesrun_install(the testraises if it does).
Also exercised live on macOS end to end: with no PowerShell present the update
refuses safely and prints the command; with a shim on PATH the full
schedule → detached spawn → exit file → report-once lifecycle runs against the real
filesystem.
Verified on real Windows. The waiter is PowerShell authored on machines that
cannot execute it, so text assertions alone would not be enough. The repo already
runs a
windows-latestjob for the #320/#529 regressions; this PR extends it to runthe update-runner suite there — which also exercises
should_defer()returning itsreal Windows default and a real detached spawn — plus three Windows-only tests that
execute the generated script against a stand-in installer:
& <argv> *>> $logworks, and
$LASTEXITCODEsurvives redirection),actually protects the environment, asserted by a sentinel file the installer would
have written.
That job immediately earned its keep: it caught the helper writing its log as
UTF-16LE. Windows PowerShell 5.1 writes redirection operators (
>,>>,*>>)as UTF-16LE with a BOM, but every other writer and reader of that shared log —
run_install,_tail, and the user we point at it — assumes UTF-8. The output nowgoes through
[System.IO.File]::AppendAllTextwithUTF8Encoding($false), and theWindows test asserts the log carries no UTF-16 BOM so it cannot regress silently.
Notably the failing-installer test passed while only the log assertion failed — a
test that checked just the exit code would have shipped the bug.
What remains unverifiable without a Windows developer machine is the original
corruption itself: a real
uv tool installlosing a race against a real file lock ona real live tool environment. Before release, on Windows 11 with
uv tool install "keboola-cli[server] @ <previous release wheel>"— runkbagent update, confirm(scheduled)and that the next launch reports success;repeat through the startup hook; and with
kbagent serveleft running confirm theupdate reports skipped with the environment intact.
Known limits
The failure mode is benign (scheduling fails, user is told the command).
Get-Process -Name kbagentdoes not see a kbagent running aspython -m keboola_agent_cli; the PID wait covers the scheduling process, but notanother such process running concurrently.
cli-dist) has nosys.frozenguard anywhere insrc/, so a frozen binary would still plan auv tool install. Out of scope here — filed separately.Design notes:
docs/superpowers/specs/2026-07-29-issue-528-deferred-windows-self-update-design.md.