diff --git a/Cargo.toml b/Cargo.toml index 592e589..3bfd08c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,6 +29,7 @@ features = [ "Win32_UI_WindowsAndMessaging", "Win32_System_Registry", "Win32_System_Threading", + "Win32_Storage_FileSystem", "Win32_Security", "Win32_UI_HiDpi", "Win32_UI_Input_KeyboardAndMouse", diff --git a/plans/260521-1530-silent-update-no-cmd/phase-01-foundation-cli-flags-native-spawn-helper-mutex-retry.md b/plans/260521-1530-silent-update-no-cmd/phase-01-foundation-cli-flags-native-spawn-helper-mutex-retry.md new file mode 100644 index 0000000..61fe8ab --- /dev/null +++ b/plans/260521-1530-silent-update-no-cmd/phase-01-foundation-cli-flags-native-spawn-helper-mutex-retry.md @@ -0,0 +1,104 @@ +--- +phase: 1 +title: "Foundation: CLI flags + native spawn helper + mutex retry" +status: complete +priority: P1 +effort: "3h" +dependencies: [] +--- + +# Phase 1: Foundation: CLI flags + native spawn helper + mutex retry + +## Overview + +Build the primitives that phases 2-4 reuse: a CLI argument parser for `--wait-pid ` and `--updated-to `, a `spawn_detached_self` helper that calls `CreateProcessW` directly (no cmd.exe), and mutex-acquisition retry logic that activates only when `--wait-pid` was passed. + +## Requirements + +**Functional** +- Parse `--wait-pid ` and `--updated-to ` from `std::env::args` without breaking existing flags (`--diagnose`, `--apply-update`). +- Expose `spawn_detached(exe: &Path, args: &[OsString]) -> io::Result<()>` that uses `CreateProcessW` with `CREATE_NO_WINDOW | DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP`. +- Expose `wait_for_parent_exit(pid: u32, timeout_ms: u32)` that uses `OpenProcess(SYNCHRONIZE)` + `WaitForSingleObject`. Returns silently on timeout (best-effort). +- Mutex acquisition in `app::run` retries `CreateMutexW` for ~3 seconds (200ms backoff) ONLY when `--wait-pid` was present; preserves today's immediate-fail behavior for normal startup. + +**Non-functional** +- No new external dependencies. All Win32 calls via the existing `windows = "0.58"` crate features. +- Helper module ≤ ~120 lines total. + +## Architecture + +``` +src/ +├── update/ +│ └── handoff.rs ← NEW: spawn_detached, wait_for_parent_exit, cleanup_stale_old_exes +├── main.rs ← parse --wait-pid early, call wait_for_parent_exit BEFORE app::run +└── app.rs ← run() reads a static "wait_pid_was_passed" flag, retries mutex if set +``` + +Rationale for `src/update/handoff.rs`: keeps low-level Win32 process/file ops alongside the update module that uses them most. `app.rs` and `main.rs` import it for restart + post-update bootstrap. + +## Related Code Files + +- **Create**: `src/update/handoff.rs` +- **Modify**: `src/main.rs` (early arg parse + wait + flag handoff to app) +- **Modify**: `src/update/mod.rs` (declare `pub mod handoff`) +- **Modify**: `src/app.rs` (mutex retry loop, gated on flag from main) +- **Modify**: `Cargo.toml` if a new `windows` feature is needed (likely `Win32_System_Threading` already covers `OpenProcess`/`WaitForSingleObject`/`CreateProcessW`) + +## Implementation Steps + +1. **Audit `windows` crate features.** Confirm `Win32_System_Threading` is in `Cargo.toml` (it is — line 31). Verify `CreateProcessW`, `STARTUPINFOW`, `PROCESS_INFORMATION` are accessible. Add `Win32_Storage_FileSystem` if not already present (needed for phase 3's `MoveFileExW`). + +2. **Create `src/update/handoff.rs`** with three pub fns: + - `pub fn spawn_detached(exe: &Path, args: &[OsString]) -> io::Result<()>` + - Build a wide-char command line: quoted exe path + space-joined args, NUL-terminated. + - `STARTUPINFOW` zero-initialized, `cb` set. + - `CreateProcessW(NULL, cmdline_wide.as_mut_ptr(), NULL, NULL, FALSE, CREATE_NO_WINDOW | DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP, NULL, NULL, &si, &pi)`. + - Close `pi.hProcess` and `pi.hThread` immediately (fire-and-forget). + - `pub fn wait_for_parent_exit(pid: u32, timeout_ms: u32)` + - `OpenProcess(SYNCHRONIZE, FALSE, pid)`. If it fails (parent already gone), return immediately. + - `WaitForSingleObject(h, timeout_ms)`. Ignore return value. + - `CloseHandle(h)`. + - `pub fn cleanup_stale_old_exes(current_exe: &Path)` (used by phase 4; stub here, fill in phase 4) + - Stub: returns `Ok(())`. + +3. **Modify `src/update/mod.rs`** to add `pub mod handoff;`. + +4. **Modify `src/main.rs`** — insert BEFORE `app::run()`: + ```rust + let wait_pid = args.iter() + .position(|a| a == "--wait-pid") + .and_then(|i| args.get(i + 1)) + .and_then(|s| s.parse::().ok()); + if let Some(pid) = wait_pid { + update::handoff::wait_for_parent_exit(pid, 5_000); + } + ``` + Note: keep this AFTER `update::run_cli` so the legacy `--apply-update` path still short-circuits cleanly. + +5. **Modify `src/app.rs::run`** — gate mutex retry on whether `--wait-pid` appeared. Cheapest implementation: re-parse `std::env::args` once at the top of `run()`. Then around line 156: + ```rust + let retry_mutex = std::env::args().any(|a| a == "--wait-pid"); + let _mutex = acquire_singleton_mutex(retry_mutex)?; // new helper + ``` + New helper `acquire_singleton_mutex(retry: bool)`: + - If `!retry`: today's behavior (fail immediately on `ERROR_ALREADY_EXISTS`). + - If `retry`: loop CreateMutexW → on `ALREADY_EXISTS`, `Sleep(200)` and retry. Budget 15 iterations = ~3 seconds. Log every retry. After budget exhausted, return error and exit cleanly. + +6. **Compile check.** `cargo build --release`. Fix any feature gaps. No behavior should change yet — `--wait-pid` arg is parsed but no caller passes it yet. + +## Success Criteria + +- [ ] `cargo build --release` succeeds with zero warnings beyond the existing `dead_code` allow. +- [ ] Running the binary normally (no flags) behaves identically to today (mutex check is immediate, no retry). +- [ ] Running the binary with `--wait-pid ` against a live instance: the new process waits ≤5s for old one to exit, then acquires the mutex within ~200ms of its release. Verifiable by killing the original after 2s and watching the new one continue. +- [ ] `update::handoff::spawn_detached` smoke test: from a small one-off snippet in `main` (gated behind a never-used flag) verify CreateProcessW returns success and pid increments. Remove before commit. + +## Risk Assessment + +| Risk | Mitigation | +|---|---| +| Wide-char cmdline construction has off-by-one / missing NUL | Hand-test with a path containing spaces; assert `OsStringExt::encode_wide` produces expected bytes | +| `CREATE_NO_WINDOW | DETACHED_PROCESS` combination misbehaves on GUI subsystem binaries | Documented Windows behavior: for a GUI subsystem child, both flags are effectively no-ops (no console requested), but combining them is harmless. Tested by phase 5 | +| Mutex retry loop hangs forever if budget logic wrong | Hard cap = 15 iterations × 200ms = 3.0s. After that, exit. Add log line per retry so a stuck loop is visible in `--diagnose` | +| `OpenProcess(SYNCHRONIZE, ...)` returns access-denied for cross-session | Fallback: `WaitForSingleObject` simply isn't called; mutex retry compensates | diff --git a/plans/260521-1530-silent-update-no-cmd/phase-02-restart-path-replace-restart-app-with-native-createprocessw.md b/plans/260521-1530-silent-update-no-cmd/phase-02-restart-path-replace-restart-app-with-native-createprocessw.md new file mode 100644 index 0000000..718445b --- /dev/null +++ b/plans/260521-1530-silent-update-no-cmd/phase-02-restart-path-replace-restart-app-with-native-createprocessw.md @@ -0,0 +1,113 @@ +--- +phase: 2 +title: "Restart path: replace restart_app with native CreateProcessW" +status: complete +priority: P1 +effort: "1h" +dependencies: [1] +--- + +# Phase 2: Restart path: replace restart_app with native CreateProcessW + +## Overview + +Replace `src/app.rs::restart_app`'s `cmd.exe /c "timeout & start ..."` handoff with a direct `spawn_detached(current_exe, ["--wait-pid", our_pid])` call. The new instance handles the parent-exit wait itself using phase 1's helper; no timer needed. + +## Requirements + +**Functional** +- Restart triggered from the tray menu produces zero console flash. +- New instance acquires `Global\ClaudeCodeUsageBubble` mutex successfully every time. +- Settings still flushed to disk before exit (preserve current `snap + save` defensive write). +- Path-with-`%` defense becomes unnecessary (no cmd.exe); the check is removed. + +**Non-functional** +- `restart_app` function shrinks from ~40 lines to ~25 lines. + +## Architecture + +``` +restart_app(): + 1. settings::save(snap) (unchanged) + 2. exe = current_exe() (unchanged) + 3. our_pid = GetCurrentProcessId() + 4. handoff::spawn_detached(exe, [--wait-pid, our_pid]) + 5. on success: PostQuitMessage(0) + 6. on failure: log error, do NOT quit +``` + +Mutex release is implicit on process exit — no explicit `ReleaseMutex` needed because the `_mutex` handle in `app::run` is dropped when `run()` returns after `PostQuitMessage`. `Drop` closes the handle, which releases the mutex. + +## Related Code Files + +- **Modify**: `src/app.rs::restart_app` (lines ~1378-1432) +- **Remove**: the `RESTART_CREATE_NO_WINDOW` / `RESTART_DETACHED_PROCESS` constants (now in handoff.rs) +- **Remove**: the `%`-rejection defense (no cmd.exe to exploit) +- **Remove**: the `replace('"', "")` quote-stripping (handoff.rs handles quoting) + +## Implementation Steps + +1. **Read current `restart_app`** at `src/app.rs:1378-1432` to confirm exact bounds. + +2. **Rewrite `restart_app`** to: + ```rust + fn restart_app() { + // Defensive settings flush (unchanged) + let snap = lock_state().as_ref().map(|s| s.settings.clone()); + if let Some(s) = snap { + settings::save(&s); + } + + let exe = match std::env::current_exe() { + Ok(p) => p, + Err(e) => { + log::error!("restart: current_exe failed: {e}"); + return; + } + }; + + let pid = unsafe { GetCurrentProcessId() }; + let args = vec![ + OsString::from("--wait-pid"), + OsString::from(pid.to_string()), + ]; + + match update::handoff::spawn_detached(&exe, &args) { + Ok(()) => { + log::info!("restart: spawned detached child, posting quit"); + unsafe { PostQuitMessage(0) }; + } + Err(e) => log::error!("restart: spawn failed: {e}"), + } + } + ``` + +3. **Drop the two `RESTART_*` const declarations** above the function — they were specific to the old cmd.exe path. Phase 1's helper has its own. + +4. **Drop the `%`-rejection block** in the new `restart_app`. The brainstorm doc keeps the equivalent check in `install.rs` for defense-in-depth; here it's pure dead weight without cmd.exe. + +5. **Imports**: add `use std::ffi::OsString;` and `use windows::Win32::System::Threading::GetCurrentProcessId;` if not already in scope. + +6. **Compile check**: `cargo build --release`. + +7. **Manual smoke test**: run the binary, click Restart in the tray menu, verify: + - No console window flashes + - New instance appears within ~1s + - Old instance log shows "spawned detached child, posting quit" + - New instance log shows mutex acquired (via Phase 1's retry path) + +## Success Criteria + +- [ ] `cargo build --release` clean. +- [ ] Manual restart from menu produces ZERO visible console window across 20 consecutive triggers. +- [ ] New instance window appears within 1500ms of menu click. +- [ ] Settings file (`settings.json`) shows updated mtime after restart, confirming defensive save still runs. +- [ ] `restart_app` function is ≤25 lines. + +## Risk Assessment + +| Risk | Mitigation | +|---|---| +| Mutex handle not yet dropped when child tries to acquire | Phase 1's `--wait-pid` + 5s `WaitForSingleObject` + 3s mutex retry covers race comfortably (8s total budget vs <1s actual parent exit) | +| `PostQuitMessage` doesn't immediately exit; window-message pump may process more events | Phase 1 mutex retry tolerates up to 3s of overlap | +| `current_exe()` returns a path that the child can't load (rare: deleted exe, fileshare disconnect) | Existing behavior preserved: log error, do not quit. User can manually retry | diff --git a/plans/260521-1530-silent-update-no-cmd/phase-03-update-install-rename-move-native-spawn.md b/plans/260521-1530-silent-update-no-cmd/phase-03-update-install-rename-move-native-spawn.md new file mode 100644 index 0000000..5484d1c --- /dev/null +++ b/plans/260521-1530-silent-update-no-cmd/phase-03-update-install-rename-move-native-spawn.md @@ -0,0 +1,156 @@ +--- +phase: 3 +title: "Update install: rename + move + native spawn" +status: complete +priority: P1 +effort: "2h" +dependencies: [1] +--- + +# Phase 3: Update install: rename + move + native spawn + +## Overview + +Replace `src/update/install.rs::begin`'s `cmd.exe /c "timeout & move & start ..."` handoff with native steps: `MoveFileExW` to rename the running exe sideways, `MoveFileExW` to move the staged exe into place, then `spawn_detached` of the new binary. Removes the only remaining cmd.exe invocation in the update flow. + +## Requirements + +**Functional** +- Update install produces zero console flash. +- New binary version starts after auto-update without user interaction. +- SHA-256 verification continues to gate the swap (no swap on checksum mismatch). +- On any failure step, the original exe must remain runnable (no half-state). + +**Non-functional** +- `install.rs` net change ≈ -30 lines (cmd-quoting code is gone, replaced by short Win32 calls). +- New code paths use the windows crate; no new dependencies. + +## Architecture + +``` +install::begin(http, release): + 1. current = current_exe() + 2. ensure_writable(current.parent()) (unchanged) + 3. staging = stage_path() + 4. reject_unsafe_path(current) (kept for defense-in-depth; harmless now) + 5. reject_unsafe_path(staging) + 6. create_dir_all(staging.parent()) + 7. download(http, asset_url, staging, sha256) (unchanged) + 8. backup = current.with_file_name(format!("{}.old.{}", filename, pid)) + 9. MoveFileExW(current, backup, 0) ← NEW: rename running exe sideways + 10. MoveFileExW(staging, current, MOVEFILE_REPLACE_EXISTING) ← NEW + 11. our_pid = GetCurrentProcessId() + 12. handoff::spawn_detached(current, + ["--wait-pid", our_pid, "--updated-to", version_str]) ← NEW + 13. return Ok(()) +``` + +Caller (`app.rs` Apply action) is responsible for `PostQuitMessage` after `begin` returns Ok — same as today. + +### Rollback semantics + +| Step that failed | State | Recovery | +|---|---|---| +| 7 (download) | Original exe untouched | Existing behavior: error surfaced, user retries | +| 9 (rename current → backup) | Original exe untouched | Surface `Error::NotWritable`; do not proceed | +| 10 (move staging → current) | Original exe is at backup path, current path empty | Best-effort revert: rename backup back to current; surface error | +| 10 + revert (both fail) | Original at backup path; current path empty; user has no runnable binary at the install location | **Per Validation Session 1 decision:** show a Windows `MessageBoxW` (MB_OK \| MB_ICONERROR) telling the user where the backup is, then exit. Message: "Update failed. Your original binary is saved as `{backup_path}`. Please rename it back to `{exe_name}` manually." | +| 12 (spawn child) | New exe at correct path, but app didn't restart | Log + tray balloon "Update applied; restart manually". Rare — `CreateProcessW` on a fresh fully-written exe almost never fails | + + + +### Rollback escalation helper + +Add a private `surface_rollback_failure(backup_path: &Path, target_name: &str)` helper that calls `MessageBoxW` with `MB_OK | MB_ICONERROR` and the localized message. Adds a new `LocaleStrings` field `update_rollback_failed_body` parameterized with `{backup_path}` and `{exe_name}` (Rust `format!` substitution at call site). The plain MessageBox uses the Win32 dialog, so no console can flash. + +## Related Code Files + +- **Modify**: `src/update/install.rs::begin` +- **Modify**: `src/update/install.rs::spawn_handoff` → REMOVED entirely +- **Modify**: `src/update/install.rs` imports (drop `os::windows::process::CommandExt`, `process::{Command, Stdio}`; add `MoveFileExW`, `MOVEFILE_REPLACE_EXISTING`, `GetCurrentProcessId`, `MessageBoxW`, `MB_OK`, `MB_ICONERROR`) +- **Modify**: `src/update/mod.rs::Error` — add `Error::SwapFailed(String)` variant if MoveFileExW failures don't fit existing variants cleanly +- **Modify**: `src/i18n/mod.rs::LocaleStrings` — add `update_rollback_failed_body: String` (also belongs to Phase 4 i18n group, but Phase 3 is the consumer) + +## Implementation Steps + +1. **Read current `install.rs::begin` + `spawn_handoff`** to confirm exact bounds (lines 19-36 + 99-121). + +2. **Decide path-safety policy**: keep `reject_unsafe_path` (the `%`-check) as defense-in-depth even though no cmd.exe runs. Update the function-level comment to reflect new reality (kept for paranoia, not strict need). + +3. **Add `swap_and_spawn` private helper** (replaces `spawn_handoff`): + ```rust + fn swap_and_spawn( + source: &Path, + target: &Path, + version: &super::release::Version, + ) -> Result<(), super::Error> { + let backup = backup_path(target); + move_file(target, &backup, 0)?; + if let Err(e) = move_file(source, target, MOVEFILE_REPLACE_EXISTING) { + // Best-effort revert + let _ = move_file(&backup, target, MOVEFILE_REPLACE_EXISTING); + return Err(e); + } + let pid = unsafe { GetCurrentProcessId() }; + let args = vec![ + OsString::from("--wait-pid"), + OsString::from(pid.to_string()), + OsString::from("--updated-to"), + OsString::from(format!("{}.{}.{}", version.major, version.minor, version.patch)), + ]; + super::handoff::spawn_detached(target, &args) + .map_err(super::Error::Io) + } + + fn move_file(src: &Path, dst: &Path, flags: MOVE_FILE_FLAGS) -> Result<(), super::Error> { + let src_w = to_utf16_nul(src); + let dst_w = to_utf16_nul(dst); + let r = unsafe { + MoveFileExW( + PCWSTR::from_raw(src_w.as_ptr()), + PCWSTR::from_raw(dst_w.as_ptr()), + flags, + ) + }; + r.ok().map_err(|e| super::Error::SwapFailed(e.to_string())) + } + + fn backup_path(target: &Path) -> PathBuf { + let pid = unsafe { GetCurrentProcessId() }; + let mut p = target.to_owned(); + let fname = target.file_name().map(|s| s.to_string_lossy().into_owned()) + .unwrap_or_else(|| "exe".to_string()); + p.set_file_name(format!("{fname}.old.{pid}")); + p + } + ``` + Use the existing `os::to_utf16_nul` helper for wide-char conversion (already used in `app.rs::run`). + +4. **Rewrite `begin`** to call `swap_and_spawn(&staging, ¤t, &release.version)` instead of `spawn_handoff(&staging, ¤t)`. Pass the version from the `Release` struct already in hand. + +5. **Add `Error::SwapFailed(String)` variant** to `src/update/mod.rs` if no existing variant fits the move-failure semantics. The `#[error(...)]` message should be `"file swap failed: {0}"`. + +6. **Remove `spawn_handoff` function** entirely. Remove now-unused imports (`std::os::windows::process::CommandExt`, `Command`, `Stdio`, `CREATE_NO_WINDOW`, `DETACHED_PROCESS` constants). + +7. **Compile check**: `cargo build --release`. Address any feature-flag gaps (likely need `Win32_Storage_FileSystem` added to Cargo `windows` features for `MoveFileExW`). + +8. **Test rollback path manually**: write a temp .exe to staging that is read-only or has wrong permissions to force step 10 to fail; verify backup is restored and original is still runnable. + +## Success Criteria + +- [ ] `cargo build --release` clean. +- [ ] Manual auto-update from a test-tagged v0.1.99 produces ZERO visible console window across 5 consecutive runs. +- [ ] SHA-256 mismatch still rejects the swap (verify by tampering with a downloaded asset before swap). +- [ ] On forced step-10 failure (simulated): backup is restored, original binary still launches. +- [ ] `spawn_handoff` function no longer exists in the codebase (`grep -r spawn_handoff src/` returns empty). +- [ ] No `cmd.exe` string remains in `src/update/install.rs`. + +## Risk Assessment + +| Risk | Mitigation | +|---|---| +| `MoveFileExW` fails because AV holds a handle on the running exe | Surface `Error::SwapFailed`; user retries. Rare on Defender (which scans on read, not perpetually) | +| Two updates triggered in quick succession leave two `.old.` files | Phase 4 cleanup at startup handles this — glob removes ALL `.old.*` siblings | +| User's install dir is on a network share where renaming-while-open is forbidden | Existing `ensure_writable` probe catches read-only / no-write cases. Network FS oddities → surface as `NotWritable` | +| `MoveFileExW` with `REPLACE_EXISTING` on a non-NTFS volume | Works on FAT32 (per MS docs); the renaming-while-running concern is NTFS-specific but step 9 always renames an empty (just-emptied) path in step 10 | +| Release-build inlines + LTO breaks symbol-level rollback assumption | Functional rollback path is exercised by phase 5's manual test under release profile | diff --git a/plans/260521-1530-silent-update-no-cmd/phase-04-cleanup-tray-notification.md b/plans/260521-1530-silent-update-no-cmd/phase-04-cleanup-tray-notification.md new file mode 100644 index 0000000..c141fe1 --- /dev/null +++ b/plans/260521-1530-silent-update-no-cmd/phase-04-cleanup-tray-notification.md @@ -0,0 +1,165 @@ +--- +phase: 4 +title: "Cleanup + tray notification" +status: complete +priority: P2 +effort: "1.5h" +dependencies: [1, 3] +--- + +# Phase 4: Cleanup + tray notification + +## Overview + +Two additions that make the silent update visible to the user without being intrusive: (a) startup cleanup of stale `bubble.exe.old.` files from previous updates, and (b) a tray balloon "Updated to vX.Y.Z" on first launch after auto-update (driven by the `--updated-to` flag passed by phase 3). + + + +## Requirements + +**Functional** +- On every startup, scan `current_exe().parent()` for files matching `.exe.old.*` and remove them silently. Errors logged at debug level, never surfaced to user. +- When `--updated-to vX.Y.Z` is passed AND the tray subsystem is initialized, show a balloon notification with localized title + body. +- Add **3 new fields** to `LocaleStrings` (`src/i18n/mod.rs:23-71`): `update_applied_title`, `update_applied_body`, `update_rollback_failed_body` (the last one is consumed by Phase 3 but the i18n change belongs to this phase's pattern). Body strings use Rust `format!` at call site — TOML strings hold raw text (e.g. body = `"Updated to v"`, then call site does `format!("{}{}", strings.update_applied_body, version)`). Choice of suffix vs prefix vs `{}` placeholder substitution is dialect-sensitive; use literal positional substitution via `format!` because TOML doesn't support template placeholders the i18n loader recognizes. +- Translate the 3 new strings in all **8 existing locale files**: `src/i18n/locales/{en,nl,es,fr,de,ja,ko,zh-TW}.toml`. + +**Non-functional** +- Cleanup runs in the foreground startup path (it's a few file operations — no need for a thread). +- Balloon uses `NIIF_INFO` (blue info icon) not `NIIF_WARNING` (yellow triangle). The existing `tray::notify` (`src/tray/mod.rs:85`) hardcodes `NIIF_WARNING` and has **2 callers** (`src/app.rs:831` usage threshold, `src/app.rs:859` token expired) — both correctly semantically "warning". Rename existing `notify` → `notify_warning`; add new sibling `notify_info`. + +## Architecture + +### Cleanup + +Triggered from `app::run` after tray icons register but before main message loop. Implementation lives in `update::handoff::cleanup_stale_old_exes` (stub was added in phase 1). + +```rust +pub fn cleanup_stale_old_exes(current_exe: &Path) { + let Some(dir) = current_exe.parent() else { return }; + let Some(stem) = current_exe.file_name() else { return }; + let prefix = format!("{}.old.", stem.to_string_lossy()); + let entries = match std::fs::read_dir(dir) { + Ok(e) => e, + Err(_) => return, + }; + for entry in entries.flatten() { + let name = entry.file_name(); + if name.to_string_lossy().starts_with(&prefix) { + if let Err(e) = std::fs::remove_file(entry.path()) { + log::debug!("cleanup_stale_old_exes: remove {:?} failed: {e}", entry.path()); + } + } + } +} +``` + +### Balloon notification + +Parse `--updated-to` argument early (alongside `--wait-pid` in main.rs), stash it on `AppState`. After the tray icons are registered for the first time, if the version string is present, call `tray::notify_info(hwnd, kind, title, body)`. + +```rust +// main.rs (after --wait-pid parse): +let updated_to = args.iter() + .position(|a| a == "--updated-to") + .and_then(|i| args.get(i + 1)) + .cloned(); +// pass into app::run via existing arg-threading or a static OnceLock +``` + +```rust +// tray/mod.rs: split notify into two variants +pub fn notify_info(owner: HWND, kind: IconKind, title: &str, body: &str) { + notify_inner(owner, kind, title, body, NIIF_INFO); +} +pub fn notify_warning(owner: HWND, kind: IconKind, title: &str, body: &str) { + notify_inner(owner, kind, title, body, NIIF_WARNING); +} +fn notify_inner(owner: HWND, kind: IconKind, title: &str, body: &str, flags: NOTIFY_ICON_INFOTIP_FLAGS) { + // existing body, but use `flags` instead of hardcoded NIIF_WARNING +} +``` + +If `tray::notify` has only one caller today (which the brainstorm scout suggested), just rename it to `notify_warning` and add `notify_info`. + +## Related Code Files + +- **Modify**: `src/update/handoff.rs::cleanup_stale_old_exes` (fill in phase-1 stub) +- **Modify**: `src/app.rs::run` (call cleanup; call tray::notify_info after tray registration if updated_to is set) +- **Modify**: `src/app.rs:831,859` (rename `tray::notify` → `tray::notify_warning` at both existing call sites) +- **Modify**: `src/main.rs` (parse `--updated-to`, stash for app) +- **Modify**: `src/tray/mod.rs` — rename `notify` → `notify_warning`; add `notify_info`; extract shared `notify_inner(... flags: NOTIFY_ICON_INFOTIP_FLAGS)` +- **Modify**: `src/i18n/mod.rs::LocaleStrings` — add 3 new `String` fields: `update_applied_title`, `update_applied_body`, `update_rollback_failed_body` +- **Modify**: `src/i18n/locales/{en,nl,es,fr,de,ja,ko,zh-TW}.toml` — 8 files, add the 3 new keys to each. Use machine translation for non-English where idiomatic translation unavailable; flag with comment for native-speaker review later + +## Implementation Steps + +1. **i18n: add 3 new fields to `LocaleStrings`** in `src/i18n/mod.rs:23-71`. After the existing `threshold_95_body` field, add: + ```rust + pub update_applied_title: String, + pub update_applied_body: String, + pub update_rollback_failed_body: String, + ``` + +2. **Translate in 8 locale files** (`src/i18n/locales/{en,nl,es,fr,de,ja,ko,zh-TW}.toml`). Suggested English values (mirror style of existing `token_expired_title`/`token_expired_body`): + ```toml + update_applied_title = "Update applied" + update_applied_body = "Updated to v" # call site appends version string + update_rollback_failed_body = "Update failed. Your original binary is saved as " # call site appends backup path + suffix + ``` + For non-English, machine-translate body+title, leave the trailing space/prefix structure intact. Comment in PR description: "Translations machine-generated; flagged for native-speaker review". + +3. **Split `tray::notify`** (`src/tray/mod.rs:85-94`) into: + - rename existing fn → `notify_warning` (preserves current `NIIF_WARNING` semantics) + - extract shared `fn notify_inner(owner, kind, title, body, flags: NOTIFY_ICON_INFOTIP_FLAGS)` + - add new `pub fn notify_info(owner, kind, title, body)` that calls `notify_inner(..., NIIF_INFO)` + Update both existing call sites (`src/app.rs:831,859`) to call `notify_warning` instead of `notify`. + +4. **Fill in `cleanup_stale_old_exes`** per architecture section. + +5. **Parse `--updated-to` in main.rs**, thread it into `app::run`. Two options: + - Pass as a new arg to `pub fn run(updated_to: Option)`. + - Store in a `OnceLock>` inside `update::handoff`, set in main, read in app. + Pick the simpler one — direct function arg is preferred unless `run`'s signature is already heavily used elsewhere. + +6. **In `app::run`** after tray icons register and the main window message loop is about to enter: + ```rust + if let Some(v) = updated_to.as_ref() { + let strings = i18n.strings(); + let title = strings.update_applied_title.clone(); + let body = format!("{}{}", strings.update_applied_body, v); + tray::notify_info(msg_hwnd, IconKind::ClaudeCode, &title, &body); + } + update::handoff::cleanup_stale_old_exes(&exe); + ``` + Use `ClaudeCode` IconKind because it's always present when Claude is enabled (default). If user disabled Claude and enabled only Codex, fall back to Codex kind. Cheapest: try ClaudeCode first; if `tray::notify_info` fails silently, no harm. + +7. **Compile check**: `cargo build --release`. + +8. **Manual test cleanup**: + - Create a file `claude-code-usage-bubble.exe.old.1234` next to the running binary. + - Launch the app. + - Verify the file is gone after launch. + +9. **Manual test notification**: + - Launch with `--updated-to 9.9.9` flag. + - Verify Windows notification appears with the title + version body. + - Verify it uses the blue info icon, not yellow warning. + +## Success Criteria + +- [ ] `cargo build --release` clean. +- [ ] Stale `.old.` files in install dir are removed on every startup (idempotent). +- [ ] Launching with `--updated-to vX.Y.Z` shows a tray balloon with localized title and version body. +- [ ] Balloon icon is blue info (NIIF_INFO), not yellow warning. +- [ ] Launching WITHOUT `--updated-to` shows no balloon (existing behavior preserved). +- [ ] All 8 locale TOML files (`en, nl, es, fr, de, ja, ko, zh-TW`) contain the 3 new keys (`update_applied_title`, `update_applied_body`, `update_rollback_failed_body`); `cargo build --release` would fail to deserialize otherwise. + +## Risk Assessment + +| Risk | Mitigation | +|---|---| +| Cleanup removes a `.old.` file that another running instance still depends on | Impossible by construction: only the spawning instance writes `.old.` and it has already exited by the time the new instance runs cleanup | +| Glob false-positive (e.g. a user-created file matching the pattern) | Pattern requires `.old.` literal AND a numeric pid-like suffix is implied; we don't pattern-match the suffix strictly. Risk is theoretical; user-created files matching `claude-code-usage-bubble.exe.old.*` is extremely unlikely | +| Tray balloon doesn't show because NIM_MODIFY runs before NIM_ADD completes | Defer the `notify_info` call by one tick (PostMessage to message loop) if testing shows races. Likely unnecessary because tray icons register synchronously | +| i18n loader is struct-field based, no template substitution at the loader level | Confirmed by Validation Session 1: append/prepend the version via Rust `format!` at the call site. TOML strings are static text fragments only | +| Translations diverge from idiomatic expression in non-English locales | Machine-translate for first cut, mark "FIXME: review by native speaker" in commit message. Subsequent crowd-sourced fixes are out of this plan's scope | diff --git a/plans/260521-1530-silent-update-no-cmd/phase-05-manual-end-to-end-verification.md b/plans/260521-1530-silent-update-no-cmd/phase-05-manual-end-to-end-verification.md new file mode 100644 index 0000000..263fdfe --- /dev/null +++ b/plans/260521-1530-silent-update-no-cmd/phase-05-manual-end-to-end-verification.md @@ -0,0 +1,104 @@ +--- +phase: 5 +title: "Manual end-to-end verification" +status: pending +priority: P2 +effort: "1h" +dependencies: [1, 2, 3, 4] +--- + +# Phase 5: Manual end-to-end verification + +## Overview + +Functional sign-off across all changed code paths. No unit tests added (the changed surface is Win32-heavy and effectively integration territory); instead, a documented manual checklist that the maintainer runs once before committing. + +## Requirements + +**Functional** +- All success criteria from phases 1-4 verified on a real Windows machine (Win11 preferred, Win10 as secondary if available). +- Build artifact runs without errors when launched plainly (no flags). +- No regression in existing update / restart / refresh paths. + +**Non-functional** +- Verification log saved as a section in `phase-05` after run, with date + outcome per item. + +## Test Matrix + +### Group A: Restart path (phase 2) + +| # | Step | Expected | +|---|---|---| +| A1 | Launch binary, open right-click menu, click "Restart" | No console flash; new instance appears within 1.5s | +| A2 | Repeat A1 twenty times back-to-back | Zero flashes observed; settings.json mtime updates each time | +| A3 | Launch with `--diagnose`, click Restart, inspect `%TEMP%\claude-code-usage-bubble.log` | Shows "restart: spawned detached child, posting quit" and new instance shows mutex acquired (within 200ms of the wait completing) | + +### Group B: Update install path (phase 3) + +Setup: tag a test release `v0.1.99` via the existing GitHub Actions workflow (per `plans/260516-1730-github-release-auto-update`). Bump local `Cargo.toml` back to `v0.1.0` before running. Build the local v0.1.0 with this plan's changes. + +| # | Step | Expected | +|---|---|---| +| B1 | Launch v0.1.0 build, set update channel to "Hourly", manually trigger "Check for updates" | "Update available" shown; click "Apply" | +| B2 | During B1 apply | No console flash; new v0.1.99 instance appears | +| B3 | After B1/B2 | Tray balloon "Update applied — Updated to v0.1.99" appears (blue info icon) | +| B4 | Inspect install dir after B1/B2 | NO `.old.*` files remain (cleanup removed them) | +| B5 | Repeat B1 four more times (after re-tagging v0.1.100 etc.) | Zero flashes across 5 update cycles | +| B6 | Tamper test: edit downloaded asset on disk before swap (would require pausing between download and swap — gate via `--diagnose` log timestamps) | SHA-256 mismatch raises `Error::ChecksumMismatch`; swap is NOT performed; original binary still runs | +| B7 | Rollback test: make staging path read-only or simulate step-10 failure | Backup restored; original binary still runs after a manual restart | + +### Group C: Cleanup + notification (phase 4) + +| # | Step | Expected | +|---|---|---| +| C1 | Manually drop `claude-code-usage-bubble.exe.old.9999` next to the running binary; launch app | Stale file is removed within 1s of launch | +| C2 | Launch app with `--updated-to 9.9.9` arg from a terminal | Tray balloon shows title + "Updated to v9.9.9" in current UI language | +| C3 | Repeat C2 with each supported locale | Each locale shows correct translation | +| C4 | Launch normally (no `--updated-to`) | No balloon appears | + +### Group D: Regression smoke + +| # | Step | Expected | +|---|---|---| +| D1 | Launch binary fresh (cold start, no flags) | Bubble appears in <2s; usage refresh fires once; tray icon registers | +| D2 | Open settings (right-click → Language → switch), confirm restart happens | Restart works without flash (this is the same `restart_app` path) | +| D3 | Disable auto-update ("Disabled"), wait 30s, re-enable Hourly | No state corruption; check timer resets | +| D4 | Run with `--diagnose --apply-update ` (legacy compat) | Returns exit code 0 cleanly (per `update::install::run_cli`) — unchanged | + +## Implementation Steps + +1. Build `cargo build --release`. +2. Copy `target/release/claude-code-usage-bubble.exe` to `%LOCALAPPDATA%\ClaudeCodeUsageBubble\` (fresh test directory). +3. Run through Test Matrix A → B → C → D in order. +4. For each test row, record outcome (PASS/FAIL + notes) in the Verification Log section below. +5. If any FAIL: open an issue describing the failure, do NOT mark phase complete. +6. If all PASS: mark phase complete, commit changes following project commit conventions (no `chore:` or `docs:` per CLAUDE.md). + +## Success Criteria + +- [ ] All Group A tests PASS. +- [ ] All Group B tests PASS (B6 + B7 may be skipped if test scaffolding too costly; document as such). +- [ ] All Group C tests PASS. +- [ ] All Group D tests PASS. +- [ ] Verification Log filled in with date + outcomes. +- [ ] No console flash observed across the entire test session. + +## Risk Assessment + +| Risk | Mitigation | +|---|---| +| Manual testing skips Group B because tagging real releases is annoying | Alternative: build two local copies (v0.1.0 + v0.1.99), set up a local HTTP server with a fake GitHub-Releases-shaped JSON, point the app at it via a debug flag. Out of scope for this plan but noted | +| "Flash" is subjective at 60Hz | Record screen with OBS at 60fps for one test run, scrub frame-by-frame to confirm zero console window appearance | +| Win10-only flash regression (only test on Win11) | Document Win10 testing as "best effort"; primary target is Win11. Note Win10 result in Verification Log | + +## Verification Log + + + +### Session 1 — TBD +- Group A: TBD +- Group B: TBD +- Group C: TBD +- Group D: TBD +- Flash observed: TBD +- Notes: TBD diff --git a/plans/260521-1530-silent-update-no-cmd/plan.md b/plans/260521-1530-silent-update-no-cmd/plan.md new file mode 100644 index 0000000..5a5df33 --- /dev/null +++ b/plans/260521-1530-silent-update-no-cmd/plan.md @@ -0,0 +1,96 @@ +--- +title: "Silent in-app update + restart (no cmd.exe)" +description: "Replace cmd.exe handoff in update install + app restart paths with native Win32 (MoveFileExW + CreateProcessW) so no terminal window can ever flash. Add tray-balloon notification after auto-updates." +status: in_progress +priority: P2 +branch: "main" +tags: ["update", "restart", "win32", "ux"] +blockedBy: [] +blocks: [] +created: "2026-05-21T07:43:16.332Z" +createdBy: "ck:plan" +source: skill +--- + +# Silent in-app update + restart (no cmd.exe) + +## Overview + +Two paths today spawn `cmd.exe /c "timeout ... & start ..."` for update install (`src/update/install.rs::begin`) and app restart (`src/app.rs::restart_app`). Combination of `CREATE_NO_WINDOW | DETACHED_PROCESS` + inner `start ""` can still flash a console window on some Windows configs. This plan replaces both with native `MoveFileExW` + `CreateProcessW` (the main exe is `windows_subsystem = "windows"`, so direct spawn never allocates a console). Also wires a tray balloon "Updated to vX.Y.Z" on first launch after an auto-update. + +Brainstorm context: [`plans/reports/brainstormer-260521-1530-silent-update-no-cmd.md`](../reports/brainstormer-260521-1530-silent-update-no-cmd.md). + +Supersedes the cmd.exe mechanism decision in [`260518-0945-menu-restart-button`](../260518-0945-menu-restart-button/plan.md) (that plan picked cmd.exe deliberately, modeled on `update::install`; this plan replaces both call sites with the native path). + +## Phases + +| Phase | Name | Status | +|-------|------|--------| +| 1 | [Foundation: CLI flags + native spawn helper + mutex retry](./phase-01-foundation-cli-flags-native-spawn-helper-mutex-retry.md) | Complete | +| 2 | [Restart path: replace restart_app with native CreateProcessW](./phase-02-restart-path-replace-restart-app-with-native-createprocessw.md) | Complete | +| 3 | [Update install: rename + move + native spawn](./phase-03-update-install-rename-move-native-spawn.md) | Complete | +| 4 | [Cleanup + tray notification](./phase-04-cleanup-tray-notification.md) | Complete | +| 5 | [Manual end-to-end verification](./phase-05-manual-end-to-end-verification.md) | Pending (user-driven) | + +## Key contracts (must hold across plan) + +| Contract | Source today | Invariant | +|---|---|---| +| Singleton mutex name | `app.rs` `APP_MUTEX_NAME` = `Global\ClaudeCodeUsageBubble` | New instance must wait for parent to release before acquiring | +| Main binary subsystem | `main.rs:1` `#![windows_subsystem = "windows"]` | Direct spawn allocates no console | +| Asset filename | `release.rs:7` `claude-code-usage-bubble.exe` | Unchanged | +| SHA-256 verification | `install.rs:64-73` | Unchanged — still verified before swap | +| Settings save on shutdown | `app.rs::restart_app` snap+save | Preserved in new restart helper | + +## Dependencies + +No cross-plan dependencies. Related (superseded mechanism): `260518-0945-menu-restart-button`. + +## Out of Scope + +- `src/usage/refresh.rs` CLI spawns (`claude.cmd`, `codex.cmd`, `powershell.exe`, `wsl.exe`) — already use `CREATE_NO_WINDOW`; tracked for follow-up. +- `src/creds/wsl_bridge.rs` `wsl.exe` calls — same. +- Code signing / SmartScreen suppression — separate roadmap item. +- Cross-platform restart/update — Windows-only by design. + +## Unresolved Questions + +None. + +## Validation Log + +### Session 1 — 2026-05-21 + +#### Verification Results +- **Tier**: Full (5 phases) +- **Claims checked**: 8 +- **Verified**: 6 | **Failed**: 2 | **Unverified**: 0 +- Verified: `app.rs:155` mutex creation; `app.rs:1378-1432` restart_app bounds; `install.rs:42-48` `--apply-update` exit-clean handler; `install.rs:99-121` `spawn_handoff`; `os::to_utf16_nul` exists and is in use; `windows = 0.58` features in Cargo.toml include `Win32_System_Threading` but NOT `Win32_Storage_FileSystem` (phase 3 must add it). +- Failed: (V1) phase 4 said `tray::notify` has "one current caller" — actually 2 (`app.rs:831`, `app.rs:859`); (V2) phase 4 said i18n uses key-based template lookup with `{v}` placeholder — actually uses struct-field-based `LocaleStrings` with TOML, no template engine. + +#### Decisions + +1. **i18n approach: add 3 fields to `LocaleStrings` + translate 8 locale TOML files; use Rust `format!` at call site for version substitution.** + Reason: existing pattern is struct-field-based via `include_str!` of `src/i18n/locales/{en,nl,es,fr,de,ja,ko,zh-TW}.toml`. No template substitution at the loader level. + → Propagated to `phase-04-cleanup-tray-notification.md`: Requirements, Related Code Files, Implementation Steps, Success Criteria, Risk Assessment all updated. + +2. **Rollback failure escalation: Windows `MessageBoxW` (MB_OK | MB_ICONERROR) when MoveFileExW step 10 AND best-effort revert both fail.** + Reason: silent failure here would orphan the user — they'd have no exe at install path. A clear modal tells them where the backup is (`bubble.exe.old.`). + → Propagated to `phase-03-update-install-rename-move-native-spawn.md`: rollback table extended; imports list updated; new helper `surface_rollback_failure` added; new `LocaleStrings` field `update_rollback_failed_body` added to phase 4's i18n work. + +3. **Stuck-parent fallback: accept 8s total budget (5s `WaitForSingleObject` + 3s mutex retry), then exit cleanly. No `TerminateProcess`.** + Reason: forcing process termination would defeat the `settings::save` defensive flush guarantee. The 8s ceiling is generous for normal Windows scheduling; if a real hang persists, user can manually kill old process via Task Manager — acceptable failure mode. + → No phase file change needed. Phase 1 budget numbers (5s + 3s) already match. + +4. **Auto-update timing: apply when check fires, no idle-window deferral.** + Reason: matches user's brainstorm-phase choice ("Fully silent auto-update"). Idle detection adds complexity (state tracking, defer-budget, defer-never-applies edge case) for marginal UX gain — bubble flicker during ~1s restart is acceptable. + → No phase file change needed. + +#### Whole-Plan Consistency Sweep +- Files reread: `plan.md`, `phase-01-…md`, `phase-02-…md`, `phase-03-…md`, `phase-04-…md`, `phase-05-…md` +- Decision deltas checked: 4 +- Reconciled stale references: 2 + - Phase 4 i18n section rewritten from key-based template to struct-field + Rust `format!` + - Phase 4 `tray::notify` caller count corrected from "1" to "2" with explicit file:line citations +- Cross-phase touchpoints verified: Phase 3 adds `update_rollback_failed_body` to `LocaleStrings`, Phase 4 owns the full i18n change (3 fields + 8 TOMLs) — both phases reference the same struct, consistent +- Unresolved contradictions: 0 diff --git a/plans/reports/brainstormer-260521-1530-silent-update-no-cmd.md b/plans/reports/brainstormer-260521-1530-silent-update-no-cmd.md new file mode 100644 index 0000000..c27a3f2 --- /dev/null +++ b/plans/reports/brainstormer-260521-1530-silent-update-no-cmd.md @@ -0,0 +1,124 @@ +# Silent in-app update + restart (no cmd.exe) + +**Date:** 2026-05-21 +**Author:** brainstormer +**Status:** approved (ready for `/ck:plan`) + +## Problem + +User sees an occasional flash terminal window. Two paths today spawn `cmd.exe /c "timeout ... & start ..."` for update install and app restart, with `CREATE_NO_WINDOW | DETACHED_PROCESS`. Combination of those flags + the inner `start ""` invocation can still emit a brief console flash on some Windows configs (Defender hooks, conhost init, AV inspection). Goal: zero-flash, fully silent auto-update + restart, with in-app notification. + +## Scope + +**In scope** +- `src/update/install.rs::begin` — kill cmd.exe handoff +- `src/app.rs::restart_app` — kill cmd.exe handoff +- New CLI flag `--wait-pid ` on the main binary (cooperates with itself across update) +- Cleanup of stale `bubble.exe.old.*` siblings at startup +- Tray balloon "Updated to vX.Y.Z" on first run after auto-update + +**Out of scope (split as follow-up)** +- `src/usage/refresh.rs` CLI spawns (`claude.cmd`, `codex.cmd`, `powershell.exe`, `wsl.exe`) — already use `CREATE_NO_WINDOW`; address separately if flash persists after this change +- `src/creds/wsl_bridge.rs` `wsl.exe` calls — same reasoning + +## Approaches evaluated + +| Approach | Decision | Rationale | +|---|---|---| +| A: Native rename + direct `CreateProcessW` + `--wait-pid` | **CHOSEN** | Zero cmd.exe ⇒ zero flash possible; single-binary preserved; matches existing Win32 style; recoverable on interrupt | +| B: Helper-exe pattern (`bubble-updater.exe`) | rejected | Reverses deliberate "no helper exe" decision (`src/update/install.rs:3-5`); release-pipeline change; helper bootstrap problem | +| C: NTFS POSIX atomic replace (`FileRenameInfoEx`) | rejected | Obscure API; harder failure modes with AV / image-protection; not worth the elegance trade-off | + +## Final design — Approach A + +### Update install flow + +``` +1. fetch_latest() (unchanged — pure HTTP via WinHTTP) +2. download(release_url, staging_path) (unchanged — sha256 verify) +3. rename current.exe -> current.exe.old. (MoveFileExW, allowed while running) +4. move staging.exe -> current.exe (MoveFileExW REPLACE_EXISTING) +5. settings::save(snap) (defensive flush, unchanged) +6. release singleton mutex (explicit ReleaseMutex + CloseHandle) +7. CreateProcessW(current.exe, "--wait-pid --updated-to vX.Y.Z", + CREATE_NO_WINDOW | DETACHED_PROCESS) +8. PostQuitMessage(0) +``` + +### Restart flow (settings-change path) + +``` +1. settings::save(snap) +2. CreateProcessW(current.exe, "--wait-pid ", + CREATE_NO_WINDOW | DETACHED_PROCESS) +3. release singleton mutex +4. PostQuitMessage(0) +``` + +### New instance startup additions + +```rust +// Before acquiring Global\ClaudeCodeUsageBubble mutex: +if let Some(parent_pid) = parse_wait_pid_arg() { + let h = OpenProcess(SYNCHRONIZE, FALSE, parent_pid)?; + WaitForSingleObject(h, 5000); // 5s cap; proceed regardless + CloseHandle(h); +} + +// After main window is up: +cleanup_old_exes(current_dir, "bubble.exe.old.*"); + +if let Some(version) = parse_updated_to_arg() { + tray::show_balloon(t!("update.toast.updated_to", v = version)); +} +``` + +### Why `--wait-pid` instead of cmd's `timeout /t 2` + +- `timeout` is a cmd.exe builtin; using it requires cmd.exe. +- `WaitForSingleObject` on the parent process handle is the canonical Win32 idiom: zero delay if parent already exited, exact timing when it actually exits, no console involvement. +- Bonus: removes the magic "2 seconds is enough" guess. + +### Path safety + +The current `reject_unsafe_path` (`%` rejection) becomes unnecessary — no cmd.exe to expand `%var%`. Keep the function for defense-in-depth; revisit in code review. + +## Files touched + +| File | Change | +|---|---| +| `src/update/install.rs` | Replace `spawn_handoff` with `swap_and_spawn` using `MoveFileExW` + `CreateProcessW`; drop `cmd` arg-quoting code | +| `src/update/mod.rs` | Add `Error::SwapFailed` variant if needed | +| `src/app.rs::restart_app` | Replace cmd.exe spawn with `CreateProcessW` + mutex release ordering | +| `src/app.rs` | Add CLI flag parsing for `--wait-pid` and `--updated-to`; cleanup pass for `.old.*` siblings; balloon tray call on successful update boot | +| `src/main.rs` | Wire `--wait-pid` into the early-startup mutex-acquisition path (BEFORE `update::run_cli` check) | +| `src/tray/mod.rs` (or `tray/badge.rs`) | Confirm `Shell_NotifyIconW` with `NIF_INFO` balloon is supported by current tray code; add helper if missing | +| `src/i18n/*` | New string keys: `update.toast.updated_to` | + +## Risks + mitigations + +| Risk | Mitigation | +|---|---| +| `MoveFileExW` rename fails (NTFS permission denied, AV scanner holding handle) | Surface `Error::NotWritable` to user; do NOT proceed to step 4 (still recoverable — original exe untouched at this point) | +| New instance crashes before clearing old exe ⇒ `.old.*` accumulates | Cleanup glob `bubble.exe.old.*` on every startup is idempotent and cheap | +| Mutex race: new instance acquires before old releases | `--wait-pid` + `WaitForSingleObject(5000ms)` covers it; if it times out the new instance retries `CreateMutexW` in a 200ms loop for ~3s before giving up | +| User on FAT32 / non-NTFS volume | `MoveFileExW` with `MOVEFILE_REPLACE_EXISTING` still works on FAT32; renaming-while-running is the NTFS-specific concern but the .exe is rarely on FAT32. Document the edge case | +| `--wait-pid` arg parsed in legacy build that doesn't recognize it | Old builds will ignore unknown args (cargo CLI parser behavior — verify). If they crash, the user can manually launch. Acceptable: this is a one-way migration; once the new flag is in a release, future updates are smooth | + +## Success criteria + +1. Manual update from v0.1.9 → test-tagged v0.1.99 produces ZERO visible console window across 20 consecutive runs on Win11 + Win10 +2. Restart triggered from menu (e.g. language change) produces ZERO visible console +3. Auto-update at scheduled interval (Hourly) produces a tray balloon "Updated to v0.1.99" on next launch +4. `.old.*` files do not accumulate after 5 update cycles +5. App still launches cleanly when no parent PID was passed (i.e. fresh user start) +6. SHA-256 verification path unchanged and still rejects tampered binaries + +## Out-of-scope follow-ups + +1. **Audit `usage::refresh::spawn_local` / `spawn_wsl`**: the `wsl.exe` invocation in particular has known console-flash quirks even with `CREATE_NO_WINDOW`. If the user still sees occasional flashes after this change ships, that is the next investigation target. +2. **`creds::wsl_bridge::wsl_run`**: same family of `wsl.exe` invocations. + +## Unresolved questions + +- None blocking implementation. (Open follow-up: whether to also pipe `--wait-pid` into the legacy `--apply-update` compatibility branch in `update::run_cli`, in case a very old build is doing the spawning.) diff --git a/src/app.rs b/src/app.rs index cbf93fe..bf421dc 100644 --- a/src/app.rs +++ b/src/app.rs @@ -6,8 +6,7 @@ // message-only window owned by this module. use std::collections::HashMap; -use std::os::windows::process::CommandExt; -use std::process::{Command, Stdio}; +use std::ffi::OsString; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex, MutexGuard, OnceLock}; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; @@ -15,7 +14,7 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use windows::core::PCWSTR; use windows::Win32::Foundation::*; use windows::Win32::System::LibraryLoader::GetModuleHandleW; -use windows::Win32::System::Threading::CreateMutexW; +use windows::Win32::System::Threading::{CreateMutexW, GetCurrentProcessId, Sleep}; use windows::Win32::UI::HiDpi::{ SetProcessDpiAwarenessContext, DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2, }; @@ -36,10 +35,11 @@ use crate::usage::{self, ProviderId, Registry, UsageWindows}; // Win32 message IDs owned by this module. pub const WM_APP_USAGE_UPDATED: u32 = 0x8001; -// Posted from the update worker thread when the swap-and-restart cmd -// handoff has been launched successfully. The UI thread responds by -// calling PostQuitMessage(0) to release the file lock on the running -// .exe so cmd.exe can overwrite it. +// Posted from the update worker thread after `install::begin` has +// already swapped the binary on disk and spawned the new detached +// child. The UI thread responds with PostQuitMessage(0) so the old +// instance exits cleanly and releases the singleton mutex for the +// child waiting on `--wait-pid`. pub const WM_APP_UPDATE_APPLIED: u32 = 0x8002; // Timer IDs used with `SetTimer(msg_hwnd, …)`. @@ -147,27 +147,52 @@ fn lock_state() -> MutexGuard<'static, Option> { // ---------- Entry ---------- -pub fn run() { +/// Acquire the singleton mutex, optionally retrying for ~3s if the +/// caller passed `--wait-pid` (i.e. we just spawned from an exiting +/// parent that has not yet released its handle). +fn acquire_singleton_mutex(retry: bool) -> Option { + let mutex_name_w = os::to_utf16_nul(APP_MUTEX_NAME); + let max_attempts = if retry { 15 } else { 1 }; + for attempt in 0..max_attempts { + let handle = unsafe { CreateMutexW(None, false, PCWSTR::from_raw(mutex_name_w.as_ptr())) }; + match handle { + Ok(h) => { + let already = unsafe { GetLastError() } == ERROR_ALREADY_EXISTS; + if !already { + return Some(h); + } + // Mutex still held by parent. Close this handle and retry. + unsafe { + let _ = CloseHandle(h); + } + if attempt + 1 == max_attempts { + log::info!("another instance already running; exiting"); + return None; + } + log::debug!( + "mutex still held by parent (attempt {}/{}), waiting 200ms", + attempt + 1, + max_attempts + ); + unsafe { Sleep(200) }; + } + Err(e) => { + log::error!("CreateMutex failed: {e}"); + return None; + } + } + } + None +} + +pub fn run(args: crate::AppArgs) { unsafe { let _ = SetProcessDpiAwarenessContext(DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2); } - let mutex_name_w = os::to_utf16_nul(APP_MUTEX_NAME); - let _mutex = unsafe { - let handle = CreateMutexW(None, false, PCWSTR::from_raw(mutex_name_w.as_ptr())); - match handle { - Ok(h) => { - if GetLastError() == ERROR_ALREADY_EXISTS { - log::info!("another instance already running; exiting"); - return; - } - h - } - Err(e) => { - log::error!("CreateMutex failed: {e}"); - return; - } - } + let _mutex = match acquire_singleton_mutex(args.wait_pid_present) { + Some(h) => h, + None => return, }; let settings = settings::load(); @@ -208,6 +233,16 @@ pub fn run() { create_initial_bubbles(); refresh_tray_icons(); + // Post-update tasks: show "Updated to vX.Y.Z" balloon (driven by + // --updated-to passed by the previous instance) and sweep any + // stale `.old.` siblings left by past in-place swaps. + if let Some(v) = args.updated_to.as_ref() { + announce_update_applied(msg_hwnd, v); + } + if let Ok(exe_path) = std::env::current_exe() { + update::handoff::cleanup_stale_old_exes(&exe_path); + } + let poll_interval = lock_state() .as_ref() .map(|s| s.settings.poll_interval_ms) @@ -828,7 +863,7 @@ fn show_threshold_balloon(provider: ProviderId, threshold: u8) { }; (s.msg_hwnd, provider, title, body) }; - tray::notify(payload.0.to_hwnd(), payload.1, &payload.2, &payload.3); + tray::notify_warning(payload.0.to_hwnd(), payload.1, &payload.2, &payload.3); } fn show_token_expired_balloon(failed: ProviderId) { @@ -856,7 +891,30 @@ fn show_token_expired_balloon(failed: ProviderId) { }; (s.msg_hwnd, failed, title, body) }; - tray::notify(payload.0.to_hwnd(), payload.1, &payload.2, &payload.3); + tray::notify_warning(payload.0.to_hwnd(), payload.1, &payload.2, &payload.3); +} + +/// Show the "Updated to vX.Y.Z" balloon on first launch after an +/// auto-update. Picks Claude as the host icon if it's registered; +/// otherwise falls back to Codex. If neither is registered the +/// notification silently drops — better than crashing. +fn announce_update_applied(_msg_hwnd: HWND, version: &str) { + let payload = { + let s = lock_state(); + let Some(s) = s.as_ref() else { + return; + }; + let strings = s.i18n.strings(); + let title = strings.update_applied_title.clone(); + let body = format!("{}{}", strings.update_applied_body, version); + let host = if s.settings.show_claude_code { + ProviderId::Claude + } else { + ProviderId::ChatGpt + }; + (s.msg_hwnd, host, title, body) + }; + tray::notify_info(payload.0.to_hwnd(), payload.1, &payload.2, &payload.3); } // ---------- Context menu ---------- @@ -1377,17 +1435,9 @@ fn set_update_check_interval(value: Option) { // ---------- Restart ---------- -// Windows CreateProcess flags. Match the values used by `update::install` -// so the cmd-handoff child detaches cleanly without flashing a console. -const RESTART_CREATE_NO_WINDOW: u32 = 0x0800_0000; -const RESTART_DETACHED_PROCESS: u32 = 0x0000_0008; - -/// Relaunch the running binary via a detached cmd.exe handoff. -/// -/// The 1-second `timeout` gives the current process time to release the -/// `Global\ClaudeCodeUsageBubble` mutex before the relaunched instance's -/// `CreateMutexW` runs, otherwise the new instance would see -/// `ERROR_ALREADY_EXISTS` and exit immediately. +/// Relaunch the running binary by spawning a detached child via +/// `CreateProcessW`. The child waits on our PID before acquiring the +/// singleton mutex, so no shell handoff or timer is required. fn restart_app() { // Defensive flush — bubble positions and most settings already persist // on change, but a final save is cheap insurance. Snapshot then drop the @@ -1404,30 +1454,18 @@ fn restart_app() { return; } }; - let exe_str = exe.to_string_lossy(); - // cmd.exe expands `%var%` inside double quotes, so a path containing `%` - // would let the environment leak into the relaunch. Refuse — matches the - // defense already used in `update::install`. - if exe_str.contains('%') { - log::error!("restart: refusing path containing '%': {exe_str}"); - return; - } - let exe_str = exe_str.replace('"', ""); - let cmd = format!(r#"timeout /t 1 /nobreak >nul & start "" "{exe_str}""#); - let spawned = Command::new("cmd.exe") - .raw_arg("/c") - .raw_arg(format!("\"{cmd}\"")) - .creation_flags(RESTART_CREATE_NO_WINDOW | RESTART_DETACHED_PROCESS) - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn(); - match spawned { - Ok(_) => { - log::info!("restart: cmd handoff spawned, posting quit"); + + let pid = unsafe { GetCurrentProcessId() }; + let args = vec![ + OsString::from("--wait-pid"), + OsString::from(pid.to_string()), + ]; + match update::handoff::spawn_detached(&exe, &args) { + Ok(()) => { + log::info!("restart: spawned detached child (parent pid={pid}), posting quit"); unsafe { PostQuitMessage(0) }; } - Err(e) => log::error!("restart: cmd spawn failed: {e}"), + Err(e) => log::error!("restart: spawn_detached failed: {e}"), } } diff --git a/src/i18n/locales/de.toml b/src/i18n/locales/de.toml index 9a8c5c2..36079a8 100644 --- a/src/i18n/locales/de.toml +++ b/src/i18n/locales/de.toml @@ -44,3 +44,6 @@ chatgpt_token_expired_title = "Codex-Sitzung abgelaufen" chatgpt_token_expired_body = "Melde dich erneut an, um die Nutzung weiter zu verfolgen." threshold_80_body = "5-Stunden-Limit naht." threshold_95_body = "Limit fast erreicht — gönn dir eine Pause." +update_applied_title = "Update angewendet" +update_applied_body = "Aktualisiert auf v" +update_rollback_failed_body = "Update fehlgeschlagen. Deine ursprüngliche Binärdatei liegt unter: " diff --git a/src/i18n/locales/en.toml b/src/i18n/locales/en.toml index 4ee1780..0b711dc 100644 --- a/src/i18n/locales/en.toml +++ b/src/i18n/locales/en.toml @@ -44,3 +44,6 @@ chatgpt_token_expired_title = "Codex session expired" chatgpt_token_expired_body = "Sign in again to keep tracking your usage." threshold_80_body = "Approaching the 5-hour limit." threshold_95_body = "Limit is close — consider easing up." +update_applied_title = "Update applied" +update_applied_body = "Updated to v" +update_rollback_failed_body = "Update failed. Your original binary is saved at: " diff --git a/src/i18n/locales/es.toml b/src/i18n/locales/es.toml index 9f97fd7..2504efe 100644 --- a/src/i18n/locales/es.toml +++ b/src/i18n/locales/es.toml @@ -44,3 +44,6 @@ chatgpt_token_expired_title = "Sesión de Codex caducada" chatgpt_token_expired_body = "Vuelve a iniciar sesión para seguir registrando el uso." threshold_80_body = "Cerca del límite de 5 horas." threshold_95_body = "Límite casi alcanzado — reduce el ritmo." +update_applied_title = "Actualización aplicada" +update_applied_body = "Actualizado a v" +update_rollback_failed_body = "Actualización fallida. Tu binario original está guardado en: " diff --git a/src/i18n/locales/fr.toml b/src/i18n/locales/fr.toml index 7c5dd98..0d89714 100644 --- a/src/i18n/locales/fr.toml +++ b/src/i18n/locales/fr.toml @@ -44,3 +44,6 @@ chatgpt_token_expired_title = "Session Codex expirée" chatgpt_token_expired_body = "Reconnectez-vous pour continuer à suivre votre utilisation." threshold_80_body = "Approche de la limite de 5 heures." threshold_95_body = "Limite proche — pensez à lever le pied." +update_applied_title = "Mise à jour appliquée" +update_applied_body = "Mis à jour vers v" +update_rollback_failed_body = "Échec de la mise à jour. Votre binaire d'origine est enregistré à : " diff --git a/src/i18n/locales/ja.toml b/src/i18n/locales/ja.toml index 835a91c..bcc288a 100644 --- a/src/i18n/locales/ja.toml +++ b/src/i18n/locales/ja.toml @@ -44,3 +44,6 @@ chatgpt_token_expired_title = "Codexのセッションが切れました" chatgpt_token_expired_body = "使用状況の追跡を続けるには再度サインインしてください。" threshold_80_body = "5時間の上限に近づいています。" threshold_95_body = "上限に近づきました — ペースを落としましょう。" +update_applied_title = "更新を適用しました" +update_applied_body = "バージョン v" +update_rollback_failed_body = "更新に失敗しました。元のバイナリは次の場所に保存されています: " diff --git a/src/i18n/locales/ko.toml b/src/i18n/locales/ko.toml index 3b76558..eadf73b 100644 --- a/src/i18n/locales/ko.toml +++ b/src/i18n/locales/ko.toml @@ -44,3 +44,6 @@ chatgpt_token_expired_title = "Codex 세션 만료" chatgpt_token_expired_body = "사용량을 계속 추적하려면 다시 로그인하세요." threshold_80_body = "5시간 한도에 가까워지고 있어요." threshold_95_body = "한도 임박 — 잠시 쉬어가세요." +update_applied_title = "업데이트가 적용되었습니다" +update_applied_body = "버전 v" +update_rollback_failed_body = "업데이트 실패. 원본 바이너리는 다음 위치에 저장되었습니다: " diff --git a/src/i18n/locales/nl.toml b/src/i18n/locales/nl.toml index e63492b..7541ad6 100644 --- a/src/i18n/locales/nl.toml +++ b/src/i18n/locales/nl.toml @@ -44,3 +44,6 @@ chatgpt_token_expired_title = "Codex-sessie verlopen" chatgpt_token_expired_body = "Meld je opnieuw aan om gebruik te blijven volgen." threshold_80_body = "Je nadert de 5-uurslimiet." threshold_95_body = "Limiet bijna bereikt — overweeg even gas terug te nemen." +update_applied_title = "Update toegepast" +update_applied_body = "Bijgewerkt naar v" +update_rollback_failed_body = "Update mislukt. Je oorspronkelijke bestand staat op: " diff --git a/src/i18n/locales/zh-TW.toml b/src/i18n/locales/zh-TW.toml index 13c160f..b9900dd 100644 --- a/src/i18n/locales/zh-TW.toml +++ b/src/i18n/locales/zh-TW.toml @@ -44,3 +44,6 @@ chatgpt_token_expired_title = "Codex 工作階段已過期" chatgpt_token_expired_body = "請重新登入以繼續追蹤使用量。" threshold_80_body = "接近 5 小時上限。" threshold_95_body = "上限將至 — 建議稍作休息。" +update_applied_title = "已套用更新" +update_applied_body = "已更新至 v" +update_rollback_failed_body = "更新失敗。您的原始執行檔已保存於: " diff --git a/src/i18n/mod.rs b/src/i18n/mod.rs index f94adb1..2cc9c90 100644 --- a/src/i18n/mod.rs +++ b/src/i18n/mod.rs @@ -68,6 +68,13 @@ pub struct LocaleStrings { pub threshold_80_body: String, /// Body text for the 95% threshold balloon. pub threshold_95_body: String, + /// Title for the tray balloon shown on first launch after an auto-update. + pub update_applied_title: String, + /// Prefix for the tray balloon body. Call site appends the version (e.g. "0.1.10"). + pub update_applied_body: String, + /// Prefix for the rollback-failed MessageBox body. Call site appends + /// the backup path and a separator with the expected target filename. + pub update_rollback_failed_body: String, } #[derive(Deserialize)] diff --git a/src/main.rs b/src/main.rs index d7fae51..afbc317 100644 --- a/src/main.rs +++ b/src/main.rs @@ -36,8 +36,34 @@ fn main() { std::process::exit(exit_code); } - if diagnose_enabled { - log::info!("entering app::run"); + let wait_pid = args + .iter() + .position(|a| a == "--wait-pid") + .and_then(|i| args.get(i + 1)) + .and_then(|s| s.parse::().ok()); + if let Some(pid) = wait_pid { + if diagnose_enabled { + log::info!("waiting up to 5s for parent pid {pid} to exit"); + } + update::handoff::wait_for_parent_exit(pid, 5_000); } - app::run(); + + let updated_to = args + .iter() + .position(|a| a == "--updated-to") + .and_then(|i| args.get(i + 1)) + .cloned(); + + if diagnose_enabled { + log::info!("entering app::run (wait_pid={wait_pid:?} updated_to={updated_to:?})"); + } + app::run(AppArgs { + wait_pid_present: wait_pid.is_some(), + updated_to, + }); +} + +pub struct AppArgs { + pub wait_pid_present: bool, + pub updated_to: Option, } diff --git a/src/tray/mod.rs b/src/tray/mod.rs index 57b4269..5324b61 100644 --- a/src/tray/mod.rs +++ b/src/tray/mod.rs @@ -11,8 +11,8 @@ use std::sync::{Mutex, OnceLock}; use windows::Win32::Foundation::HWND; use windows::Win32::UI::Shell::{ - Shell_NotifyIconW, NIF_ICON, NIF_INFO, NIF_MESSAGE, NIF_TIP, NIIF_WARNING, NIM_ADD, - NIM_DELETE, NIM_MODIFY, NOTIFYICONDATAW, + Shell_NotifyIconW, NIF_ICON, NIF_INFO, NIF_MESSAGE, NIF_TIP, NIIF_INFO, NIIF_WARNING, + NIM_ADD, NIM_DELETE, NIM_MODIFY, NOTIFYICONDATAW, NOTIFY_ICON_INFOTIP_FLAGS, }; use windows::Win32::UI::WindowsAndMessaging::DestroyIcon; @@ -81,13 +81,28 @@ pub fn sync(owner: HWND, desired: &[TrayIcon]) { } } -/// Show a balloon notification on an already-registered icon. -pub fn notify(owner: HWND, kind: IconKind, title: &str, body: &str) { +/// Show a yellow-warning balloon on an already-registered icon. +pub fn notify_warning(owner: HWND, kind: IconKind, title: &str, body: &str) { + notify_inner(owner, kind, title, body, NIIF_WARNING); +} + +/// Show a blue-info balloon on an already-registered icon. +pub fn notify_info(owner: HWND, kind: IconKind, title: &str, body: &str) { + notify_inner(owner, kind, title, body, NIIF_INFO); +} + +fn notify_inner( + owner: HWND, + kind: IconKind, + title: &str, + body: &str, + flags: NOTIFY_ICON_INFOTIP_FLAGS, +) { let mut data = build_data(owner, kind); data.uFlags = NIF_INFO; write_utf16(&mut data.szInfoTitle, title); write_utf16(&mut data.szInfo, body); - data.dwInfoFlags = NIIF_WARNING; + data.dwInfoFlags = flags; unsafe { let _ = Shell_NotifyIconW(NIM_MODIFY, &data); } diff --git a/src/update/handoff.rs b/src/update/handoff.rs new file mode 100644 index 0000000..36f285f --- /dev/null +++ b/src/update/handoff.rs @@ -0,0 +1,133 @@ +// Native Win32 process + file handoff primitives used by the in-app +// restart path and the auto-update install path. The main binary uses +// `windows_subsystem = "windows"`, so spawning the child directly via +// `CreateProcessW` allocates no console — nothing can flash. + +use std::ffi::OsString; +use std::io; +use std::os::windows::ffi::OsStrExt; +use std::path::Path; + +use windows::core::PCWSTR; +use windows::Win32::Foundation::{CloseHandle, FALSE, HANDLE, WAIT_OBJECT_0}; +use windows::Win32::System::Threading::{ + CreateProcessW, OpenProcess, WaitForSingleObject, CREATE_NEW_PROCESS_GROUP, + CREATE_NO_WINDOW, DETACHED_PROCESS, PROCESS_INFORMATION, PROCESS_SYNCHRONIZE, + STARTUPINFOW, +}; + +/// Spawn `exe` with the supplied args as a detached, console-less child. +/// +/// Caller is fire-and-forget: the child's handles are closed immediately +/// so no zombie wait is required. +pub fn spawn_detached(exe: &Path, args: &[OsString]) -> io::Result<()> { + let mut cmdline = build_command_line(exe, args); + + let si = STARTUPINFOW { + cb: std::mem::size_of::() as u32, + ..Default::default() + }; + let mut pi = PROCESS_INFORMATION::default(); + + let flags = CREATE_NO_WINDOW | DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP; + let ok = unsafe { + CreateProcessW( + PCWSTR::null(), + windows::core::PWSTR(cmdline.as_mut_ptr()), + None, + None, + FALSE, + flags, + None, + PCWSTR::null(), + &si, + &mut pi, + ) + }; + + if ok.is_err() { + return Err(io::Error::last_os_error()); + } + + unsafe { + if !pi.hThread.is_invalid() { + let _ = CloseHandle(pi.hThread); + } + if !pi.hProcess.is_invalid() { + let _ = CloseHandle(pi.hProcess); + } + } + // Suppress the unused-variable warning until si.lpReserved fields ever matter. + let _ = &si; + Ok(()) +} + +/// Wait up to `timeout_ms` for `pid` to exit. Silent on any failure — +/// caller treats this as a best-effort barrier before acquiring the +/// singleton mutex. +pub fn wait_for_parent_exit(pid: u32, timeout_ms: u32) { + let handle: HANDLE = match unsafe { OpenProcess(PROCESS_SYNCHRONIZE, FALSE, pid) } { + Ok(h) if !h.is_invalid() => h, + _ => return, + }; + unsafe { + let res = WaitForSingleObject(handle, timeout_ms); + if res != WAIT_OBJECT_0 { + log::debug!("wait_for_parent_exit pid={pid} timeout/err res={:?}", res.0); + } + let _ = CloseHandle(handle); + } +} + +/// Remove leftover `.old.` siblings from previous in-place updates. +/// Filled in by phase 4; stubbed here so phase 1 can wire the call sites. +pub fn cleanup_stale_old_exes(current_exe: &Path) { + let Some(dir) = current_exe.parent() else { + return; + }; + let Some(stem) = current_exe.file_name() else { + return; + }; + let prefix = format!("{}.old.", stem.to_string_lossy()); + let entries = match std::fs::read_dir(dir) { + Ok(e) => e, + Err(_) => return, + }; + for entry in entries.flatten() { + let name = entry.file_name(); + if name.to_string_lossy().starts_with(&prefix) { + if let Err(e) = std::fs::remove_file(entry.path()) { + log::debug!( + "cleanup_stale_old_exes: remove {:?} failed: {e}", + entry.path() + ); + } + } + } +} + +fn build_command_line(exe: &Path, args: &[OsString]) -> Vec { + // CreateProcessW parses argv[0] from a quoted exe path. We wrap the + // exe in `"…"` and join args separated by spaces. Args are quoted + // only when they contain whitespace; our callers pass simple tokens + // (--wait-pid , --updated-to ) so naive quoting is + // sufficient. + let mut line = String::new(); + line.push('"'); + line.push_str(&exe.to_string_lossy()); + line.push('"'); + for a in args { + line.push(' '); + let s = a.to_string_lossy(); + if s.chars().any(|c| c.is_whitespace()) { + line.push('"'); + line.push_str(&s); + line.push('"'); + } else { + line.push_str(&s); + } + } + let mut wide: Vec = std::ffi::OsString::from(line).encode_wide().collect(); + wide.push(0); + wide +} diff --git a/src/update/install.rs b/src/update/install.rs index 91c0d81..7772c03 100644 --- a/src/update/install.rs +++ b/src/update/install.rs @@ -1,42 +1,47 @@ -// Download a release asset and hand off via inline `cmd /c`. +// Download a release asset and swap it in via native Win32 calls. // -// We avoid the helper-exe pattern entirely: after writing the new .exe -// to a staging path, we spawn cmd.exe with a one-liner that waits 2 s, -// moves the new binary over the running one (Windows releases the file -// lock when our process exits), and relaunches it. +// After writing the new .exe to a staging path and verifying its +// SHA-256, we `MoveFileExW` the running exe sideways (so Windows +// releases the file lock on our own image), then `MoveFileExW` the +// staged exe into place, then spawn the new binary detached via +// `handoff::spawn_detached`. No shell, no console allocation. -use std::os::windows::process::CommandExt; -use std::path::PathBuf; -use std::process::{Command, Stdio}; +use std::ffi::OsString; +use std::path::{Path, PathBuf}; use sha2::{Digest, Sha256}; -use crate::net::Client; +use windows::core::PCWSTR; +use windows::Win32::Storage::FileSystem::{ + MoveFileExW, MOVEFILE_COPY_ALLOWED, MOVEFILE_REPLACE_EXISTING, MOVE_FILE_FLAGS, +}; +use windows::Win32::System::Threading::GetCurrentProcessId; +use windows::Win32::UI::WindowsAndMessaging::{ + MessageBoxW, MB_ICONERROR, MB_OK, +}; -const CREATE_NO_WINDOW: u32 = 0x0800_0000; -const DETACHED_PROCESS: u32 = 0x0000_0008; +use crate::net::Client; +use crate::os::to_utf16_nul; pub fn begin(http: &Client, release: &super::Release) -> Result<(), super::Error> { let current = std::env::current_exe()?; ensure_writable(¤t)?; let staging = stage_path()?; - // Refuse to proceed if either path contains `%`. Inside double quotes - // cmd.exe still expands `%var%` references, so a path containing `%` - // would let cmd substitute environment variables into the swap step. - // Such paths are vanishingly rare on real Windows installs; failing - // fast is safer than rolling a bespoke cmd-escape layer. + // Defense in depth: `MoveFileExW` itself is immune to `%`-expansion + // (no shell parses our paths), but the existing rejection guards + // future code paths that might invoke external tools, so keep it. reject_unsafe_path(¤t)?; reject_unsafe_path(&staging)?; if let Some(parent) = staging.parent() { std::fs::create_dir_all(parent)?; } download(http, &release.asset_url, &staging, release.asset_sha256.as_ref())?; - spawn_handoff(&staging, ¤t)?; + swap_and_spawn(&staging, ¤t, &release.version)?; Ok(()) } /// CLI entry-point compatibility for `--apply-update `. -/// The inline-cmd handoff already does the swap-and-restart; if this binary +/// The native handoff already does the swap-and-restart; if this binary /// is invoked with the legacy flag (e.g. from an older release's helper) /// just exit cleanly so the upgrade still completes. pub fn run_cli(args: &[String]) -> Option { @@ -50,7 +55,7 @@ pub fn run_cli(args: &[String]) -> Option { fn download( http: &Client, url: &str, - to: &std::path::Path, + to: &Path, expected_sha256: Option<&[u8; 32]>, ) -> Result<(), super::Error> { let resp = http @@ -86,40 +91,119 @@ fn hex_encode(bytes: &[u8]) -> String { out } -fn reject_unsafe_path(p: &std::path::Path) -> Result<(), super::Error> { +fn reject_unsafe_path(p: &Path) -> Result<(), super::Error> { let s = p.to_string_lossy(); if s.contains('%') { return Err(super::Error::UnsafePath(format!( - "path contains '%' which cmd.exe expands as a variable: {s}" + "path contains '%': {s}" ))); } Ok(()) } -fn spawn_handoff(source: &std::path::Path, target: &std::path::Path) -> Result<(), super::Error> { - let src_str = source.to_string_lossy().replace('"', ""); - let tgt_str = target.to_string_lossy().replace('"', ""); - // 2-second wait gives the current process time to exit and release the - // file lock before `move` overwrites it. - let cmd = format!( - r#"timeout /t 2 /nobreak >nul & move /y "{src_str}" "{tgt_str}" & start "" "{tgt_str}""# - ); - // raw_arg bypasses Rust's std auto-escaping which would turn the inner - // `"` characters into `\"`. cmd.exe does not recognise `\"`, so the - // escaped form makes `start` see the path as `\\` and emit a - // "Windows cannot find '\\'" dialog. Feeding the command line raw - // preserves the quotes cmd.exe actually expects. - Command::new("cmd.exe") - .raw_arg("/c") - .raw_arg(format!("\"{cmd}\"")) - .creation_flags(CREATE_NO_WINDOW | DETACHED_PROCESS) - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn()?; +fn swap_and_spawn( + source: &Path, + target: &Path, + version: &super::release::Version, +) -> Result<(), super::Error> { + let backup = backup_path(target); + // Step 1: rename running exe sideways. Windows allows renaming a + // file even while its image is mapped into memory; this releases + // the lock on the original `target` path. Same directory by + // construction, so plain MoveFileExW with no flags is sufficient. + move_file(target, &backup, MOVE_FILE_FLAGS(0))?; + + // Step 2: move staged exe into place. Staging lives under + // %LOCALAPPDATA%, target lives wherever the user installed — + // COPY_ALLOWED lets MoveFileExW fall back to copy+delete when + // the two paths cross volumes (portable installs on D:/E:/etc.). + let step2_flags = MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED; + if let Err(swap_err) = move_file(source, target, step2_flags) { + // Best-effort revert. Same volume, no COPY_ALLOWED needed. + if let Err(revert_err) = move_file(&backup, target, MOVEFILE_REPLACE_EXISTING) { + log::error!("rollback also failed: {revert_err}; surfacing modal"); + let target_name = target + .file_name() + .map(|s| s.to_string_lossy().into_owned()) + .unwrap_or_else(|| "claude-code-usage-bubble.exe".to_string()); + surface_rollback_failure(&backup, &target_name); + } + return Err(swap_err); + } + + // Step 3: spawn the new exe detached with --wait-pid + --updated-to. + let pid = unsafe { GetCurrentProcessId() }; + let version_str = format!("{}.{}.{}", version.major, version.minor, version.patch); + let args = vec![ + OsString::from("--wait-pid"), + OsString::from(pid.to_string()), + OsString::from("--updated-to"), + OsString::from(version_str), + ]; + if let Err(spawn_err) = super::handoff::spawn_detached(target, &args) { + // New binary is on disk but won't auto-launch. Roll back so + // the user's next "Restart" stays on the known-good version. + log::error!("spawn_detached failed after swap: {spawn_err}; attempting revert"); + if let Err(revert_err) = move_file(&backup, target, MOVEFILE_REPLACE_EXISTING) { + log::error!("post-spawn revert failed: {revert_err}"); + } + return Err(super::Error::Io(spawn_err)); + } Ok(()) } +fn move_file(src: &Path, dst: &Path, flags: MOVE_FILE_FLAGS) -> Result<(), super::Error> { + let src_w = to_utf16_nul(&src.to_string_lossy()); + let dst_w = to_utf16_nul(&dst.to_string_lossy()); + let result = unsafe { + MoveFileExW( + PCWSTR::from_raw(src_w.as_ptr()), + PCWSTR::from_raw(dst_w.as_ptr()), + flags, + ) + }; + result.map_err(|e| { + super::Error::SwapFailed(format!( + "MoveFileExW({} -> {}): {e}", + src.display(), + dst.display() + )) + }) +} + +fn backup_path(target: &Path) -> PathBuf { + let pid = unsafe { GetCurrentProcessId() }; + let fname = target + .file_name() + .map(|s| s.to_string_lossy().into_owned()) + .unwrap_or_else(|| "exe".to_string()); + let mut p = target.to_owned(); + p.set_file_name(format!("{fname}.old.{pid}")); + p +} + +fn surface_rollback_failure(backup: &Path, target_name: &str) { + // Pull the localized body from i18n; the caller passes the + // user-meaningful filename so we can format it in-place. + let strings = crate::i18n::I18n::load(None).strings().clone(); + let body = format!( + "{}{}\n\n{}", + strings.update_rollback_failed_body, + backup.display(), + target_name + ); + let title_w = to_utf16_nul(&strings.update_failed); + let body_w = to_utf16_nul(&body); + unsafe { + MessageBoxW( + None, + PCWSTR::from_raw(body_w.as_ptr()), + PCWSTR::from_raw(title_w.as_ptr()), + MB_OK | MB_ICONERROR, + ); + } +} + fn stage_path() -> Result { let base = dirs::data_local_dir().ok_or_else(|| { super::Error::NotWritable("no local data directory available".to_string()) @@ -130,7 +214,7 @@ fn stage_path() -> Result { .join("update.exe")) } -fn ensure_writable(target: &std::path::Path) -> Result<(), super::Error> { +fn ensure_writable(target: &Path) -> Result<(), super::Error> { let parent = target.parent().ok_or_else(|| { super::Error::NotWritable("could not resolve install directory".to_string()) })?; diff --git a/src/update/mod.rs b/src/update/mod.rs index c27f3f7..b8b4eff 100644 --- a/src/update/mod.rs +++ b/src/update/mod.rs @@ -1,10 +1,12 @@ // Self-update subsystem. // // Two stages: `release::fetch_latest` checks GitHub releases for a newer -// build; `install::begin` downloads the .exe and hands off to a detached -// `cmd /c` script that swaps the binary and restarts. +// build; `install::begin` downloads the .exe, swaps it in via native +// `MoveFileExW`, then spawns the new binary detached via +// `CreateProcessW`. No shell handoff — nothing can flash a console. pub mod channel; +pub mod handoff; pub mod install; pub mod release; @@ -24,6 +26,8 @@ pub enum Error { ChecksumMismatch { expected: String, actual: String }, #[error("path rejected for safety: {0}")] UnsafePath(String), + #[error("file swap failed: {0}")] + SwapFailed(String), } pub use channel::{current as current_channel, Channel};