Skip to content

fix(update): apply staged Windows replacement#21

Open
NianJiuZst wants to merge 3 commits into
Tencent:mainfrom
NianJiuZst:codex/fix-windows-staged-update
Open

fix(update): apply staged Windows replacement#21
NianJiuZst wants to merge 3 commits into
Tencent:mainfrom
NianJiuZst:codex/fix-windows-staged-update

Conversation

@NianJiuZst

@NianJiuZst NianJiuZst commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

On Windows, bsk update could report a successful update even though the installed executable was never replaced. The updater wrote a staged executable and a .cmd replacement script next to the running binary, then returned InstallAction::Staged, but nothing launched that script. The old executable therefore remained installed indefinitely.

This also created a daemon availability problem. The update flow stops a running daemon before replacing the binary. Because the staged replacement never ran—and because the Windows path did not preserve the previous daemon state—the command could leave users on the old version with their daemon stopped while still printing an update-style success message.

How This Fix Works

  • Launch the generated Windows replacement script as a detached, no-window cmd.exe process before the updater exits.
  • Keep retrying the move until the currently running bsk.exe releases its file lock, then atomically replace the target from the staged executable.
  • Carry the pre-update daemon state into the helper and restart the daemon only when it had been running before the update.
  • Delete the helper script after a successful replacement so update artifacts do not accumulate beside the executable.
  • Remove both staged files if the helper cannot be launched, and return an error instead of claiming that the update was staged successfully.
  • Report staged updates with a distinct staged status and wording that accurately describes when the replacement will occur.
  • Escape percent characters in generated paths so Windows environment-variable expansion cannot corrupt the helper commands.

User Impact

Windows users can now run bsk update and have the staged version applied automatically after the command exits. A daemon that was running before the update comes back on the new binary, while an intentionally stopped daemon remains stopped. Failed helper launches are visible as failures rather than false-positive update reports.

Validation

  • cargo fmt --check
  • cargo clippy --workspace --all-targets --locked -- -D warnings
  • cargo test --workspace --locked
  • Added focused tests covering replacement retries, conditional daemon restart, helper self-cleanup, and the no-restart path.

The full local Rust workspace suite passed. A local GNU Windows cross-check was also attempted, but this macOS environment does not provide the x86_64-w64-mingw32-gcc toolchain required by the transitive aws-lc-sys build; GitHub CI remains the authoritative clean-environment build check for the PR.

CI Baseline Compatibility

This branch also carries the one-line Biome 2.4 formatter output for apps/extension/vitest.config.ts. The same formatting mismatch currently fails the Frontend job on upstream/main; this minimal compatibility commit is intentionally separate from the functional fix and lets this PR run the repository CI suite to completion.

CI Reliability Follow-up

GitHub Actions exposed a pre-existing test-only port allocation race: integration helpers probed an ephemeral port, released it, and then asked the daemon to bind the same number, allowing another process to claim it in between. The resulting Address already in use failure was unrelated to the functional changes. The PR now lets each daemon bind port 0 directly and reads the assigned address from its handle, removing the TOCTOU window across all affected integration helpers.

@NianJiuZst NianJiuZst marked this pull request as ready for review July 10, 2026 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant