mirror of
https://github.com/tiennm99/claude-code-usage-bubble.git
synced 2026-09-09 02:17:12 +00:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
bcce939f72 | ||
|
|
e089a1b420 | ||
|
|
457d5274da | ||
|
|
f1dfe15000 | ||
|
|
38ae4dff09 | ||
|
|
3c0878f6cc | ||
|
|
eca430ccc6 |
Generated
+1
-1
@@ -59,7 +59,7 @@ checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801"
|
||||
|
||||
[[package]]
|
||||
name = "claude-code-usage-bubble"
|
||||
version = "0.1.6"
|
||||
version = "0.1.8"
|
||||
dependencies = [
|
||||
"dirs",
|
||||
"embed-resource",
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "claude-code-usage-bubble"
|
||||
version = "0.1.6"
|
||||
version = "0.1.8"
|
||||
edition = "2021"
|
||||
license = "Apache-2.0"
|
||||
description = "Floating bubble showing Claude Code and Codex usage on Windows"
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
# Phase 01 — Implement Restart Action
|
||||
|
||||
## Context Links
|
||||
|
||||
- Reused pattern: `src/update/install.rs:1-120` (cmd-handoff swap-and-restart). Documented in `docs/release-process.md` if it exists.
|
||||
- Menu wiring reference: `src/app.rs:870-1045` (`show_context_menu`) and `src/app.rs:363-392` (`on_menu_command`).
|
||||
- Mutex acquisition: `src/app.rs:152-168` (`Global\ClaudeCodeUsageBubble`).
|
||||
- i18n schema: `src/i18n/mod.rs` (`LocaleStrings` struct around line 22-80).
|
||||
|
||||
## Overview
|
||||
|
||||
- **Priority:** Low (UX enhancement).
|
||||
- **Status:** Done. Code-reviewer DONE_WITH_CONCERNS — M1 (match-arm ordering) + L3 (lock-during-save) addressed in follow-up edits.
|
||||
- **Size:** ~50 LOC across 3 files (+ 8 locale TOMLs, one line each).
|
||||
|
||||
## Key Insights
|
||||
|
||||
- The existing mutex check rejects a second instance immediately. A naive "spawn-then-exit" races. The `cmd.exe /c timeout` handoff (1 s sleep, then `start ""`) is the simplest decoupling — same trick `update::install::begin` already uses.
|
||||
- `cmd.exe` expands `%var%` in argument strings. Current `current_exe()` path containing `%` is an injection vector; reject it (existing precedent: `update::install` rejects too).
|
||||
- `std::process::Command` with `creation_flags(DETACHED_PROCESS | CREATE_NO_WINDOW)` ensures the helper outlives the parent silently.
|
||||
|
||||
## Requirements
|
||||
|
||||
### Functional
|
||||
|
||||
- Right-click context menu shows a "Restart" item directly above "Exit".
|
||||
- Clicking "Restart" closes the current process and a new instance starts within ~1–2 seconds, restoring tray icons and bubbles.
|
||||
- No confirmation prompt.
|
||||
- Item label is i18n-aware: all 8 locales get a translation.
|
||||
|
||||
### Non-functional
|
||||
|
||||
- No regression in mutex single-instance behavior — second instance must still be blocked if user accidentally launches manually mid-restart.
|
||||
- No console window flashes during handoff.
|
||||
|
||||
## Architecture
|
||||
|
||||
```
|
||||
User clicks "Restart"
|
||||
→ WM_COMMAND with IDM_RESTART
|
||||
→ app::on_menu_command → app::restart_app()
|
||||
→ settings::save current snapshot (defensive flush)
|
||||
→ spawn detached cmd.exe with delayed `start ""` for current_exe
|
||||
→ PostQuitMessage(0)
|
||||
→ message loop exits → mutex released
|
||||
→ cmd.exe wakes up → new instance launches → acquires mutex → run()
|
||||
```
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Modify:**
|
||||
|
||||
- `src/app.rs` — add `IDM_RESTART: u16 = 33` const (next free in the 30-39 band), match arm in `on_menu_command`, menu append in `show_context_menu` between `IDM_TOGGLE_WIDGET` row and the separator before `IDM_EXIT`, new `fn restart_app()`.
|
||||
- `src/i18n/mod.rs` — add `pub restart: String,` field to `LocaleStrings` (place near `exit`).
|
||||
- `src/i18n/locales/en.toml`, `de.toml`, `es.toml`, `fr.toml`, `ja.toml`, `ko.toml`, `nl.toml`, `zh-TW.toml` — add `restart = "<translation>"`.
|
||||
|
||||
**Create:** none.
|
||||
|
||||
**Delete:** none.
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. **Add menu ID and i18n field.**
|
||||
- `app.rs`: `const IDM_RESTART: u16 = 33;` (after `IDM_VERSION_ACTION`).
|
||||
- `i18n/mod.rs`: add `pub restart: String,` to `LocaleStrings`. Place adjacent to `exit`.
|
||||
- Add `restart = "Restart"` to `en.toml`. Translate for the other 7 locales (Vietnamese-quality is acceptable; native fluency not required for a single-word menu item).
|
||||
|
||||
2. **Wire the menu entry.**
|
||||
- `app.rs::show_context_menu` — between the `show_widget` append and the `MF_SEPARATOR` before `IDM_EXIT`, add `append_item(menu, IDM_RESTART, &snap.strings.restart, MENU_ITEM_FLAGS(0));`.
|
||||
- Add `IDM_RESTART => restart_app(),` arm in `on_menu_command` before the `_ => {}` catch-all.
|
||||
|
||||
3. **Implement `restart_app()`.**
|
||||
- Persist a final settings snapshot (defensive flush). Read current state, call `settings::save(&snap)`.
|
||||
- Resolve `std::env::current_exe()`. If `Err`, log error and `PostQuitMessage(0)` (degrade to plain Exit).
|
||||
- Convert path to string. If it contains `%`, log error and return (refuse — matches `update::install` precedent).
|
||||
- Build the cmd line: `timeout /t 1 >nul & start "" "<exe>"`.
|
||||
- Spawn via `std::process::Command::new("cmd.exe")` with `.creation_flags(DETACHED_PROCESS | CREATE_NO_WINDOW)` and a `raw_arg` payload `/c "<cmd>"` (mirrors `install.rs:104-114`).
|
||||
- On spawn success: `PostQuitMessage(0)`. On failure: log error and return (app stays running).
|
||||
|
||||
4. **Verify.**
|
||||
- `cargo check` — no warnings.
|
||||
- Manual smoke test: build release, right-click tray, Restart, observe close + relaunch within ~2 s, mutex acquired by new instance, bubbles + tray icons rendered.
|
||||
|
||||
## Todo List
|
||||
|
||||
- [x] Add `IDM_RESTART` const in `app.rs`.
|
||||
- [x] Add `restart: String` to `LocaleStrings` in `i18n/mod.rs`.
|
||||
- [x] Update all 8 locale `.toml` files.
|
||||
- [x] Append menu item in `show_context_menu`.
|
||||
- [x] Add match arm in `on_menu_command` (placed before `IDM_LANG_BASE` guard per reviewer M1).
|
||||
- [x] Implement `restart_app()` in `app.rs` (clone-then-save per reviewer L3).
|
||||
- [x] `cargo check` clean.
|
||||
- [ ] Manual smoke test on Windows (deferred to user; needs release build).
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- Right-click tray → menu shows "Restart" between "Show widget" group and "Exit".
|
||||
- Clicking it closes the process and a new one starts within 2 s with identical state (settings honored, bubble positions persisted, tray icons restored).
|
||||
- No console window flashes.
|
||||
- `cargo check` passes with no new warnings.
|
||||
- All 8 locales include the new key (no fallback to English).
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Severity | Mitigation |
|
||||
|------|----------|------------|
|
||||
| Mutex race — new instance starts before old releases | Medium | 1 s `timeout` in cmd handoff; matches update module precedent. |
|
||||
| `current_exe()` path contains `%` (injection) | Low | Reject, log, abort (same as `install.rs:90-94`). |
|
||||
| `cmd.exe` not on PATH (broken Windows install) | Very Low | Log error, app stays running. User can Exit manually. |
|
||||
| Settings not flushed before quit | Low | Explicit `settings::save()` before `PostQuitMessage`. Bubble positions already persist on drag, so worst case is a no-op. |
|
||||
| User restart-spams the menu | Low | Each click queues a new cmd handoff; the timeout dedupes via mutex. Worst case: one extra instance attempt that exits immediately on `ERROR_ALREADY_EXISTS`. |
|
||||
|
||||
## Security Considerations
|
||||
|
||||
- `%`-in-path rejection prevents `cmd.exe` variable expansion injection.
|
||||
- No user-supplied input enters the cmd line — only `std::env::current_exe()` output.
|
||||
- Detached process flags prevent inherited stdio from leaking.
|
||||
|
||||
## Next Steps
|
||||
|
||||
- After merge: bump version (semver patch — UX addition with no API change).
|
||||
- Consider an analogous restart action for the bubble's context menu (currently the bubble also fires `on_menu_command` via `WM_COMMAND`, so the same menu id works there for free).
|
||||
@@ -0,0 +1,41 @@
|
||||
# Plan: Menu Restart Button
|
||||
|
||||
**Slug:** menu-restart-button
|
||||
**Created:** 2026-05-18 09:45
|
||||
**Branch:** main
|
||||
**Status:** Implemented (awaiting commit)
|
||||
|
||||
## Goal
|
||||
|
||||
Add a "Restart" entry to the tray right-click context menu, positioned directly above "Exit". Clicking it relaunches the running binary in-place without prompting for confirmation.
|
||||
|
||||
## Why
|
||||
|
||||
User-requested. Current flow to apply a config/locale tweak that doesn't hot-reload (or to recover after a hang) is Exit → relaunch from Start menu. A one-click restart is symmetric with Exit and avoids hunting for the binary again.
|
||||
|
||||
## Phases
|
||||
|
||||
| # | Title | Status |
|
||||
|---|-------|--------|
|
||||
| 01 | Implement Restart action | Done — [phase-01-implement-restart-action.md](phase-01-implement-restart-action.md) |
|
||||
|
||||
## Key Decisions
|
||||
|
||||
- **Placement:** main menu, between `Show widget` separator and `Exit`. NOT inside Settings submenu — keeps top-level discoverability.
|
||||
- **No confirmation dialog.** Settings auto-save on every change (settings.rs:138-152); restart is non-destructive.
|
||||
- **Mechanism:** detached `cmd.exe /c timeout /t 1 >nul & start "" "<exe>"` handoff, then `PostQuitMessage(0)`. Same pattern as `update::install::begin` minus the swap step. The 1-second wait lets the current process release `Global\ClaudeCodeUsageBubble` mutex before the new instance's `CreateMutexW` runs.
|
||||
- **Reject paths containing `%`** — cmd.exe expands `%var%`, same defense the update module already uses (install.rs:90-94).
|
||||
|
||||
## Dependencies
|
||||
|
||||
- None. Pure Rust + existing `windows` crate features.
|
||||
|
||||
## Out of Scope
|
||||
|
||||
- Restart after settings change auto-trigger (would be a separate feature).
|
||||
- Restart-with-args (e.g., toggle `--diagnose`).
|
||||
- Cross-platform — Windows-only by design.
|
||||
|
||||
## Unresolved Questions
|
||||
|
||||
None.
|
||||
@@ -0,0 +1,80 @@
|
||||
# Brainstorm: Off-Screen Bubble Recovery
|
||||
|
||||
## 1. Recovery strategies ranked
|
||||
|
||||
### RECOMMEND — Layered: validate on load + clamp on create
|
||||
|
||||
**A. Validate position in `settings::load`** (primary defense)
|
||||
- After deserialize, walk `bubble_positions`. For each `Some((x,y))`, build a probe rect `(x, y, x+min_w, y+min_h)` and check via `MonitorFromRect(... MONITOR_DEFAULTTONULL)`. If null → set to `None`.
|
||||
- One pass, ~15 lines, runs before any window code sees the value.
|
||||
- Pro: KISS, no race with `ShowWindow`, fixes related bugs (hand-edited JSON, dpi-changed coords).
|
||||
|
||||
**B. Clamp on create** (defense-in-depth, kept as proposed)
|
||||
- After `CreateWindowExW`, before `ShowWindow`, call `clamp_into_work_area(hwnd)`.
|
||||
- Catches monitor unplug **between** `load()` and `create()` (rare but possible: laptop closed mid-startup).
|
||||
- Cost: one extra call, idempotent.
|
||||
|
||||
Both together are the right answer. Neither alone covers all cases.
|
||||
|
||||
### CONSIDER — Visual cue
|
||||
|
||||
**C. Tray balloon "Widget repositioned to primary monitor"**
|
||||
- Only when validator actually relocated. Uses existing `Shell_NotifyIconW NIF_INFO`. ~20 lines.
|
||||
- Risk: balloon spam on dock/undock cycles. Fire only when *saved* position was killed.
|
||||
|
||||
### AVOID
|
||||
|
||||
**D. Topology fingerprint** — overkill, doesn't preserve intent better than (A).
|
||||
**E. Per-monitor relative pinning** — future feature, not a fix. YAGNI.
|
||||
|
||||
## 2. Trade-off matrix
|
||||
|
||||
| Approach | Preserves intent on replug | Surprise on cold start | LOC | Risk |
|
||||
|----------|---------------------------|------------------------|-----|------|
|
||||
| A (validate-on-load) | No — wipes saved coord | Low | ~15 | None |
|
||||
| B (clamp-on-create) | Partial — moves to nearest edge | Low | ~3 | None |
|
||||
| A+B | No | Low | ~18 | None |
|
||||
| A+B+C | Same + explains itself | Very low | ~38 | Balloon fatigue |
|
||||
| D (topology hash) | Yes if same monitor before next launch | Medium | ~80 | Maintenance |
|
||||
| E (relative pin) | Yes | Medium | ~150 | Premature |
|
||||
|
||||
## 3. Edge cases proposed fix misses
|
||||
|
||||
1. **Saved-pos monitor asleep / no input** — `MonitorFromRect` still returns handle; A+B no-op. Correct.
|
||||
2. **DPI change while app closed** — px coords technically valid by topology; A passes, B no-op. Acceptable.
|
||||
3. **Negative-coord monitors (secondary left of primary)** — validator MUST use `MONITOR_DEFAULTTONULL`, not `DEFAULTTONEAREST` (would silently snap valid secondary coord to primary).
|
||||
4. **Dual bubbles overlap after relocate** — both clamped to bottom-right of primary → stacked. `default_position` staggers Codex; clamp doesn't. Minor.
|
||||
5. **User drags to secondary, unplugs, restarts** — A+B: bubble at primary default; saved pos destroyed. No recovery on replug. Acceptable for v1.
|
||||
6. **Dock-daily multi-monitor user** — every undock wipes pos; every dock back gives default. Annoying. Case where E would win. Punt unless reported.
|
||||
|
||||
## 4. Logging strategy (minimal)
|
||||
|
||||
On the visibility-affecting path only:
|
||||
|
||||
- `info`: `bubble create model={} pos=({},{}) size={}x{} dpi={}` — one line per bubble at create.
|
||||
- `warn`: `bubble position ({},{}) outside all monitors, resetting to default` — fires in validator. **This is the line that would have solved this bug in 5 seconds.**
|
||||
- `warn`: `clamp_into_work_area moved bubble from ({},{}) to ({},{})` — fires on create-time clamp.
|
||||
- `debug`: monitor enumeration on startup.
|
||||
|
||||
Skip: per-render logs, drag logs, timer ticks.
|
||||
|
||||
## 5. "Reset position" discoverability — secondary
|
||||
|
||||
Menu item exists but buried. If A+B work, this path is unreachable. Don't add a "Reset position" balloon prompt — confirmation fatigue. Just fix silently and the §4 warn + §C balloon explain it once.
|
||||
|
||||
## Recommended action
|
||||
|
||||
1. Add `BubblePositions::validate(&mut self)` called from `settings::load`. Use `MonitorFromRect(... MONITOR_DEFAULTTONULL)` with `(x, y, x+MIN_BUBBLE_SIZE, y+MIN_BUBBLE_SIZE)`. Set to `None` on miss. Log `warn`.
|
||||
2. Call `clamp_into_work_area(hwnd)` in `bubble::create` between `CreateWindowExW` and `ShowWindow`. Log `warn` on movement.
|
||||
3. Add the 4 log lines from §4.
|
||||
4. Single tray balloon "Widget repositioned: previous monitor not connected" once per launch when validator killed any saved position.
|
||||
5. Defer monitor-index pinning (E) and topology hash (D).
|
||||
|
||||
Total: ~40 lines, one new function, two log statements, one balloon call.
|
||||
|
||||
## Unresolved questions
|
||||
|
||||
- Balloon (item 4): opt-in or always-on? Default always-on.
|
||||
- `MIN_BUBBLE_SIZE` as probe rect, or account for current `bubble_size_logical`? Min safer.
|
||||
- Re-attempt last-known coord on replug? Probably no — YAGNI.
|
||||
- Codex/Claude stagger preservation on auto-relocate? Currently they'd stack. Worth fixing in same patch?
|
||||
@@ -0,0 +1,60 @@
|
||||
# Code Review: Bubble Off-Screen Clamp Fix
|
||||
|
||||
**Scope:** Proposed bug-fix for v0.1.7 "widget enabled but not shown" — saved positions on disconnected monitor.
|
||||
**Files:** `src/bubble.rs` (create, clamp_into_work_area, set_user_visible, default_position), `src/app.rs` (spawn_bubble, toggle_widget_visibility, reset_positions).
|
||||
|
||||
## Overall Assessment
|
||||
|
||||
**Fix is correct and minimal. Ship it with two small refinements.** Root-cause matches code (verified: `bubble.rs:143-160` passes saved `position` straight into `CreateWindowExW`; `clamp_into_work_area` at `:770` only wired into `WM_SETTINGCHANGE` at `:486`). Approach is the right shape: clamp post-create, pre-show.
|
||||
|
||||
## Critical Issues
|
||||
|
||||
None.
|
||||
|
||||
## High Priority
|
||||
|
||||
1. **Call order — clamp must run BEFORE `render(hwnd)` at `bubble.rs:220`, not just before `ShowWindow` at `:222`.** `render` calls `GetWindowRect` (`:1108`) for the `UpdateLayeredWindow` destination point. If clamp runs after `render`, the first frame paints at the off-screen coords; second paint only happens on next update_data tick. Move `clamp_into_work_area(hwnd)` to between line `:218` (state insert) and `:220` (render).
|
||||
|
||||
## Medium Priority
|
||||
|
||||
2. **`MonitorFromWindow` on a not-yet-shown off-screen window — verified safe.** Win32 sets the window rect immediately at `CreateWindowExW` return (visibility is irrelevant to `GetWindowRect`). With `MONITOR_DEFAULTTONEAREST` and a window whose entire rect lies on a disconnected monitor, the OS computes intersection with each *currently attached* monitor's rect; none intersect → falls back to nearest by Euclidean distance → returns the primary on a single-monitor setup. Saved `[2407,1282]` on a 1920-wide primary → nearest = primary → clamp pulls to `(1920-w, …)`. Correct.
|
||||
|
||||
3. **Multi-monitor edge case is preserved.** If the saved position is on a still-connected secondary, `MonitorFromWindow` returns that secondary monitor and clamps within its work area — no unwanted pull to primary. Good.
|
||||
|
||||
4. **Partial off-screen.** `clamp_into_work_area` only adjusts when fully outside (clamps each axis independently to `[wa.left, wa.right-w]`). A window whose top-left is on-screen but bottom-right spills off → it pulls the whole window inside. Behaviour is fine; matches `snap_to_edge` (`:644-645`).
|
||||
|
||||
5. **DPI mismatch (saved from 4K → 1080p primary):** the saved coords are physical pixels but the new bubble's `width_px/height_px` are recomputed against the *current* primary DPI (`:140-142`). Clamp uses the new size against the new monitor's work area — correct. No DPI bug.
|
||||
|
||||
6. **`default_position` case:** no-op (already inside work area). Safe.
|
||||
|
||||
## Low Priority
|
||||
|
||||
7. **Log levels are appropriate.** `info!` in `create` (fires once per bubble creation), `set_user_visible` (fires only on user-toggle — verified at `app.rs:1152` only called from `toggle_widget_visibility`), and `toggle_widget_visibility` (one event per click). None on the render hot path. Approved.
|
||||
|
||||
8. **Alternative call site (clamp in `app::spawn_bubble`):** Less attractive. `spawn_bubble` doesn't own the HWND lifecycle and would need a fresh `GetWindowRect` round-trip. Keeping the clamp inside `bubble::create` keeps the bubble module the sole owner of window geometry and means future call sites (e.g. tests, a hypothetical re-create-on-DPI-change) also benefit for free. The "bubble module stays position-agnostic" argument is weak — it already calls `default_position`, `snap_to_edge`, and `clamp_into_work_area`. Position-aware is the status quo.
|
||||
|
||||
## Side Effects
|
||||
|
||||
- No callers of `bubble::create` assert the returned HWND is at the exact requested coords. `app::spawn_bubble` (`:277`) ignores position post-create; `reset_positions` (`:1156`) destroys + recreates. Safe.
|
||||
- `position(hwnd)` (`:322`) reads live `GetWindowRect`, so any subsequent `on_bubble_moved` save reflects the clamped coords — this self-heals the persisted bad value on first drag.
|
||||
|
||||
## Positive Observations
|
||||
|
||||
- Clamp helper already exists and is correct (`:770-809`).
|
||||
- Fix is one line + three log statements; minimal blast radius.
|
||||
- Persisted-corruption auto-heal via first interaction is a nice property.
|
||||
|
||||
## Recommended Actions
|
||||
|
||||
1. **MUST:** Place `clamp_into_work_area(hwnd)` between `lock_bubbles().insert(...)` (`:218`) and `render(hwnd)` (`:220`) — not after `render`.
|
||||
2. **SHOULD:** Add an `info!` in `clamp_into_work_area` that fires only when `nx != r.left || ny != r.top` (i.e. the actual reposition path). Free diagnostic for future "bubble moved itself" reports.
|
||||
3. **CONSIDER:** Persist the clamped position immediately after `create` so `settings.json` is self-healed on next launch, not only after a drag. Trade-off: writes settings on every startup; current behaviour writes only on user action. Probably YAGNI — drift gets repaired on first interaction.
|
||||
|
||||
## Unresolved Questions
|
||||
|
||||
- Should we also persist the corrected position eagerly (action 3)? Default to no per YAGNI; flag for user.
|
||||
- Does Windows ever defer `CreateWindowExW` window-rect commit until `ShowWindow`? Per MSDN and verified by existing `snap_to_edge` using the same pattern in `WM_EXITSIZEMOVE`, no — rect is committed synchronously.
|
||||
|
||||
**Status:** DONE_WITH_CONCERNS
|
||||
**Summary:** Fix is correct and small. One ordering bug: clamp must precede `render`, not just `ShowWindow`, otherwise the first paint targets the off-screen coords.
|
||||
**Concerns:** Action 1 (clamp before render) is a real correctness issue — the proposal as written ("after CreateWindowExW succeeds and before ShowWindow") technically permits ordering after `render`, which would defeat the fix until the next data update.
|
||||
@@ -0,0 +1,62 @@
|
||||
# Code Review — Tray "Restart" Action
|
||||
|
||||
**Scope:** uncommitted changes on clean tree
|
||||
**Files:** `src/app.rs`, `src/i18n/mod.rs`, 8x `src/i18n/locales/*.toml`
|
||||
**Plan:** `plans/260518-0945-menu-restart-button/phase-01-implement-restart-action.md`
|
||||
|
||||
## Verdict
|
||||
Clean implementation. All 7 acceptance criteria met. Build is `cargo check`-clean. No security regressions. Pattern faithfully borrowed from `update/install.rs`.
|
||||
|
||||
## Acceptance Criteria — all PASS
|
||||
1. Menu order verified `app.rs:1040-1042`: separator → `IDM_RESTART` → `IDM_EXIT`.
|
||||
2. `IDM_RESTART => restart_app()` arm wired `app.rs:393`.
|
||||
3. `restart_app()` `app.rs:1382-1421` flushes settings, gets `current_exe`, rejects `%`, spawns detached `cmd.exe`, `PostQuitMessage(0)`.
|
||||
4. 1 s `timeout` matches install.rs precedent (2 s there; 1 s sufficient — current process exits as soon as `PostQuitMessage(0)` drains the loop).
|
||||
5. Verified `restart = "..."` in all 8 TOMLs at line 32 (en/de/es/fr/ja/ko/nl/zh-TW). `LocaleStrings` field at `mod.rs:52`. No `#[serde(default)]` → missing key = hard fail; all present.
|
||||
6. Match-arm ordering unambiguous: `IDM_RESTART=33` < guard `x >= IDM_LANG_BASE=100`. Guard won't match 33. `tray::IDM_TOGGLE_WIDGET=50` likewise < 100. Safe.
|
||||
7. No new clippy issues; no new unsafe blocks (`PostQuitMessage(0)` already unsafe at `IDM_EXIT`; matches that idiom).
|
||||
|
||||
## Critical
|
||||
None.
|
||||
|
||||
## High
|
||||
None.
|
||||
|
||||
## Medium
|
||||
**M1. Restart arm sits below the `IDM_LANG_BASE` guard arm.** `app.rs:391-393`. The guard `x if x >= IDM_LANG_BASE => …` is exhaustive for any id `>= 100`. Today `IDM_RESTART=33` is fine, but future readers adding a static id `>= 100` between lines 392 and 393 would silently route into language switching. Cheap fix: move `IDM_RESTART => restart_app()` and `tray::IDM_TOGGLE_WIDGET => …` ABOVE the guard arm. Plan note at `app.rs:82-84` already warns about this — the new arm violates that guidance.
|
||||
|
||||
## Low / Info
|
||||
**L1. Double-restart not deduped.** Rapid clicks queue multiple `cmd.exe` children. First wins the mutex; second's `start ""` succeeds, the resulting bubble process exits at `ERROR_ALREADY_EXISTS`. Acceptable per plan §Risk Assessment. No fix needed.
|
||||
|
||||
**L2. `to_string_lossy()` on `current_exe()` will mangle non-UTF-8 paths.** Same pattern in `install.rs:100`. On real Windows installs paths are UTF-16; lossy → UTF-8 is virtually always faithful. Consistent with existing precedent.
|
||||
|
||||
**L3. `settings::save()` runs while `lock_state()` read-guard is held** (`app.rs:1385-1387`). If `save` ever takes a lock on the same mutex this would deadlock — it currently does not, but the pattern elsewhere (e.g. `set_poll_interval` at 1087-1100) clones, releases, then saves. Recommend matching that pattern: clone snapshot inside scope, drop guard, then `settings::save(&snap)`. Defensive only.
|
||||
|
||||
**L4. No regression to existing menu wiring** — verified by inspection: `show_widget` append at 1034-1039 still preceded by no separator, then separator 1040, then Restart, then Exit. Matches plan exactly.
|
||||
|
||||
## Pattern-Parity Check vs `install.rs`
|
||||
- Flags: `CREATE_NO_WINDOW | DETACHED_PROCESS` → identical bit pattern (`0x0800_0000 | 0x0000_0008`). New constants `RESTART_*` duplicate the values; minor DRY nit but they're file-local and the comment explains why. Acceptable.
|
||||
- `raw_arg` quoting: `/c` then `"<cmd>"` with inner `"` preserved → byte-for-byte same shape as `install.rs:113-114`. Correct.
|
||||
- `%` rejection: present, logs and aborts. Matches `install.rs:89-96`.
|
||||
- `stdin/out/err = Null`: present, matches.
|
||||
|
||||
## PostQuitMessage on UI thread
|
||||
`IDM_EXIT` does the same at `app.rs:370`, called from `on_menu_command` via WM_COMMAND on the UI thread. `restart_app()` is reached the same way. Safe — identical control-flow shape.
|
||||
|
||||
## Metrics
|
||||
- New code: ~40 LOC in `app.rs`, 1 field in `mod.rs`, 8x 1-line TOML adds.
|
||||
- Type coverage: 100%.
|
||||
- New warnings: 0 (`cargo check` clean per user).
|
||||
|
||||
## Recommended Actions
|
||||
1. **M1** (nice-to-have): reorder match arms so `IDM_RESTART` / `IDM_TOGGLE_WIDGET` precede the `x if x >= IDM_LANG_BASE` guard. Defends against future id collisions.
|
||||
2. **L3** (optional): mirror `set_poll_interval`'s clone-then-save pattern in `restart_app()` for consistency.
|
||||
|
||||
## Unresolved Questions
|
||||
- None blocking. Plan §Next Steps suggests a semver patch bump and analogous bubble-menu entry; out of scope for this review.
|
||||
|
||||
---
|
||||
|
||||
**Status:** DONE_WITH_CONCERNS
|
||||
**Summary:** Implementation matches plan and acceptance criteria; cmd-handoff faithfully mirrors `update/install.rs`; all 8 locales updated; no critical or high issues. One medium suggestion (reorder match arms to defend against future static-id collisions with the `IDM_LANG_BASE` guard) and two low/optional refinements.
|
||||
**Concerns:** M1 — new `IDM_RESTART` and `tray::IDM_TOGGLE_WIDGET` arms sit below a catch-all `x >= IDM_LANG_BASE` guard. Today safe (33, 50 < 100); future-fragile. Code comment at `app.rs:82-84` already flags the rule that was bent.
|
||||
+71
-3
@@ -6,6 +6,8 @@
|
||||
// message-only window owned by this module.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::os::windows::process::CommandExt;
|
||||
use std::process::{Command, Stdio};
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
use std::sync::{Arc, Mutex, MutexGuard, OnceLock};
|
||||
use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
|
||||
@@ -69,6 +71,7 @@ const IDM_MODEL_CHATGPT: u16 = 21;
|
||||
const IDM_START_WITH_WINDOWS: u16 = 30;
|
||||
const IDM_RESET_POSITION: u16 = 31;
|
||||
const IDM_VERSION_ACTION: u16 = 32;
|
||||
const IDM_RESTART: u16 = 33;
|
||||
const IDM_LANG_SYSTEM: u16 = 40;
|
||||
// 50 is reserved by tray::IDM_TOGGLE_WIDGET — keep the auto-update range
|
||||
// clear of it (and any future tray ids in the 5x band).
|
||||
@@ -385,8 +388,12 @@ pub fn on_menu_command(id: u32, _owner_hwnd: HWND) {
|
||||
set_update_check_interval(Some(settings::UPDATE_CHECK_WEEKLY_SECS))
|
||||
}
|
||||
IDM_LANG_SYSTEM => set_language(None),
|
||||
x if x >= IDM_LANG_BASE => set_language_by_index((x - IDM_LANG_BASE) as usize),
|
||||
// Static ids in the 30-99 band must match BEFORE the dynamic
|
||||
// language guard, otherwise `x >= IDM_LANG_BASE` would swallow any
|
||||
// future id that creeps into the >=100 range.
|
||||
tray::IDM_TOGGLE_WIDGET => toggle_widget_visibility(),
|
||||
IDM_RESTART => restart_app(),
|
||||
x if x >= IDM_LANG_BASE => set_language_by_index((x - IDM_LANG_BASE) as usize),
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
@@ -1034,6 +1041,7 @@ fn show_context_menu(owner_hwnd: HWND) {
|
||||
if snap.widget_visible { MF_CHECKED } else { MENU_ITEM_FLAGS(0) },
|
||||
);
|
||||
let _ = AppendMenuW(menu, MF_SEPARATOR, 0, PCWSTR::null());
|
||||
append_item(menu, IDM_RESTART, &snap.strings.restart, MENU_ITEM_FLAGS(0));
|
||||
append_item(menu, IDM_EXIT, &snap.strings.exit, MENU_ITEM_FLAGS(0));
|
||||
|
||||
let mut pt = POINT::default();
|
||||
@@ -1067,9 +1075,13 @@ fn version_action_label(snap: &ContextMenuSnapshot) -> String {
|
||||
UpdateStatus::Applying => snap.strings.applying_update.clone(),
|
||||
UpdateStatus::Failed => snap.strings.update_failed.clone(),
|
||||
};
|
||||
// Append the running binary's version so the user can see what
|
||||
// they are on without opening an About dialog. Using middle-dot as
|
||||
// the separator matches the bubble's countdown formatting.
|
||||
let with_version = format!("{base} \u{00b7} v{}", env!("CARGO_PKG_VERSION"));
|
||||
match snap.install_channel {
|
||||
InstallChannel::Winget => format!("{base} ({})", snap.strings.update_via_winget),
|
||||
InstallChannel::Portable => base,
|
||||
InstallChannel::Winget => format!("{with_version} ({})", snap.strings.update_via_winget),
|
||||
InstallChannel::Portable => with_version,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1357,6 +1369,62 @@ fn set_update_check_interval(value: Option<u64>) {
|
||||
}
|
||||
}
|
||||
|
||||
// ---------- 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.
|
||||
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
|
||||
// lock before the disk write so the UI thread doesn't block on I/O.
|
||||
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 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");
|
||||
unsafe { PostQuitMessage(0) };
|
||||
}
|
||||
Err(e) => log::error!("restart: cmd spawn failed: {e}"),
|
||||
}
|
||||
}
|
||||
|
||||
// ---------- Start-with-Windows ----------
|
||||
|
||||
fn is_startup_enabled() -> bool {
|
||||
|
||||
+35
-7
@@ -133,16 +133,16 @@ pub fn create(config: BubbleConfig) -> HWND {
|
||||
let initial_size_logical = config
|
||||
.size_logical
|
||||
.clamp(MIN_BUBBLE_SIZE, MAX_BUBBLE_SIZE);
|
||||
let dpi_for_create = primary_dpi();
|
||||
let width_px = scale_to_dpi(initial_size_logical, dpi_for_create);
|
||||
let height_px = scale_to_dpi(bubble_height_logical(initial_size_logical), dpi_for_create);
|
||||
let (x, y) = config
|
||||
.position
|
||||
.unwrap_or_else(|| default_position(width_px, height_px, config.model));
|
||||
let hwnd = unsafe {
|
||||
let class_w = wide_str(CLASS_NAME);
|
||||
let title_w = wide_str("Claude Code Usage Bubble");
|
||||
let hinstance = GetModuleHandleW(PCWSTR::null()).unwrap_or_default();
|
||||
let dpi = primary_dpi();
|
||||
let width_px = scale_to_dpi(initial_size_logical, dpi);
|
||||
let height_px = scale_to_dpi(bubble_height_logical(initial_size_logical), dpi);
|
||||
let (x, y) = config
|
||||
.position
|
||||
.unwrap_or_else(|| default_position(width_px, height_px, config.model));
|
||||
CreateWindowExW(
|
||||
WS_EX_TOOLWINDOW | WS_EX_LAYERED | WS_EX_TOPMOST | WS_EX_NOACTIVATE,
|
||||
PCWSTR::from_raw(class_w.as_ptr()),
|
||||
@@ -217,6 +217,16 @@ pub fn create(config: BubbleConfig) -> HWND {
|
||||
},
|
||||
);
|
||||
|
||||
log::info!(
|
||||
"bubble create model={:?} pos=({x},{y}) size={width_px}x{height_px} dpi={dpi}",
|
||||
config.model
|
||||
);
|
||||
|
||||
// Defense in depth: settings::load already validates positions against
|
||||
// currently-connected monitors, but a monitor unplug between load and
|
||||
// create (or a partially-off-screen saved position) is still possible.
|
||||
clamp_into_work_area(hwnd);
|
||||
|
||||
render(hwnd);
|
||||
unsafe {
|
||||
let _ = ShowWindow(hwnd, SW_SHOWNOACTIVATE);
|
||||
@@ -792,8 +802,26 @@ fn clamp_into_work_area(hwnd: HWND) {
|
||||
let w = r.right - r.left;
|
||||
let h = r.bottom - r.top;
|
||||
let nx = r.left.clamp(wa.left, (wa.right - w).max(wa.left));
|
||||
let ny = r.top.clamp(wa.top, (wa.bottom - h).max(wa.top));
|
||||
let mut ny = r.top.clamp(wa.top, (wa.bottom - h).max(wa.top));
|
||||
|
||||
// When both bubbles get clamped to the same bottom-right corner (e.g.,
|
||||
// saved positions were on a disconnected monitor and the validator missed
|
||||
// them), keep the Codex-above-Claude stagger that `default_position` uses
|
||||
// so they don't visually stack.
|
||||
let is_codex = lock_bubbles()
|
||||
.get(&(hwnd.0 as isize))
|
||||
.is_some_and(|b| matches!(b.model, TrayIconKind::ChatGpt));
|
||||
if is_codex && nx == wa.right - w && ny == wa.bottom - h {
|
||||
const STAGGER_GAP: i32 = 24;
|
||||
ny = (ny - h - STAGGER_GAP).max(wa.top);
|
||||
}
|
||||
|
||||
if nx != r.left || ny != r.top {
|
||||
log::warn!(
|
||||
"clamp_into_work_area moved bubble from ({}, {}) to ({nx}, {ny})",
|
||||
r.left,
|
||||
r.top
|
||||
);
|
||||
unsafe {
|
||||
let _ = SetWindowPos(
|
||||
hwnd,
|
||||
|
||||
@@ -29,6 +29,7 @@ auto_check_hourly = "Stündlich"
|
||||
auto_check_daily = "Täglich"
|
||||
auto_check_weekly = "Wöchentlich"
|
||||
exit = "Beenden"
|
||||
restart = "Neu starten"
|
||||
show_widget = "Widget anzeigen"
|
||||
session_window = "5h"
|
||||
weekly_window = "7d"
|
||||
|
||||
@@ -29,6 +29,7 @@ auto_check_hourly = "Hourly"
|
||||
auto_check_daily = "Daily"
|
||||
auto_check_weekly = "Weekly"
|
||||
exit = "Exit"
|
||||
restart = "Restart"
|
||||
show_widget = "Show widget"
|
||||
session_window = "5h"
|
||||
weekly_window = "7d"
|
||||
|
||||
@@ -29,6 +29,7 @@ auto_check_hourly = "Cada hora"
|
||||
auto_check_daily = "Cada día"
|
||||
auto_check_weekly = "Cada semana"
|
||||
exit = "Salir"
|
||||
restart = "Reiniciar"
|
||||
show_widget = "Mostrar widget"
|
||||
session_window = "5h"
|
||||
weekly_window = "7d"
|
||||
|
||||
@@ -29,6 +29,7 @@ auto_check_hourly = "Toutes les heures"
|
||||
auto_check_daily = "Quotidienne"
|
||||
auto_check_weekly = "Hebdomadaire"
|
||||
exit = "Quitter"
|
||||
restart = "Redémarrer"
|
||||
show_widget = "Afficher le widget"
|
||||
session_window = "5h"
|
||||
weekly_window = "7j"
|
||||
|
||||
@@ -29,6 +29,7 @@ auto_check_hourly = "1時間ごと"
|
||||
auto_check_daily = "毎日"
|
||||
auto_check_weekly = "毎週"
|
||||
exit = "終了"
|
||||
restart = "再起動"
|
||||
show_widget = "ウィジェットを表示"
|
||||
session_window = "5時間"
|
||||
weekly_window = "7日"
|
||||
|
||||
@@ -29,6 +29,7 @@ auto_check_hourly = "매시간"
|
||||
auto_check_daily = "매일"
|
||||
auto_check_weekly = "매주"
|
||||
exit = "종료"
|
||||
restart = "다시 시작"
|
||||
show_widget = "위젯 표시"
|
||||
session_window = "5시간"
|
||||
weekly_window = "7일"
|
||||
|
||||
@@ -29,6 +29,7 @@ auto_check_hourly = "Per uur"
|
||||
auto_check_daily = "Dagelijks"
|
||||
auto_check_weekly = "Wekelijks"
|
||||
exit = "Afsluiten"
|
||||
restart = "Opnieuw starten"
|
||||
show_widget = "Widget tonen"
|
||||
session_window = "5u"
|
||||
weekly_window = "7d"
|
||||
|
||||
@@ -29,6 +29,7 @@ auto_check_hourly = "每小時"
|
||||
auto_check_daily = "每天"
|
||||
auto_check_weekly = "每週"
|
||||
exit = "結束"
|
||||
restart = "重新啟動"
|
||||
show_widget = "顯示小工具"
|
||||
session_window = "5 小時"
|
||||
weekly_window = "7 日"
|
||||
|
||||
@@ -49,6 +49,7 @@ pub struct LocaleStrings {
|
||||
pub auto_check_daily: String,
|
||||
pub auto_check_weekly: String,
|
||||
pub exit: String,
|
||||
pub restart: String,
|
||||
pub show_widget: String,
|
||||
pub session_window: String,
|
||||
pub weekly_window: String,
|
||||
|
||||
@@ -1,11 +1,18 @@
|
||||
use std::path::PathBuf;
|
||||
|
||||
use serde::{Deserialize, Serialize};
|
||||
use windows::Win32::Foundation::RECT;
|
||||
use windows::Win32::Graphics::Gdi::{MonitorFromRect, MONITOR_DEFAULTTONULL};
|
||||
|
||||
use crate::bubble::DEFAULT_BUBBLE_SIZE;
|
||||
use crate::usage::ProviderId;
|
||||
type TrayIconKind = ProviderId;
|
||||
|
||||
// 140px matches MIN_BUBBLE_SIZE — a saved top-left a few px past the work-area
|
||||
// edge still passes the validator, but a position fully on a disconnected
|
||||
// monitor (the bug we're guarding against) fails.
|
||||
const POSITION_PROBE_PX: i32 = 140;
|
||||
|
||||
const APP_DIR_NAME: &str = "ClaudeCodeUsageBubble";
|
||||
const SETTINGS_FILE: &str = "settings.json";
|
||||
|
||||
@@ -67,6 +74,37 @@ impl BubblePositions {
|
||||
self.claude = None;
|
||||
self.codex = None;
|
||||
}
|
||||
|
||||
/// Drop any saved position whose top-left no longer falls on a connected
|
||||
/// monitor. Guards against `bubble::create` placing the window on a
|
||||
/// disconnected secondary monitor (where the user can't see or recover it).
|
||||
pub fn validate(&mut self) {
|
||||
if let Some((x, y)) = self.claude {
|
||||
if !position_on_any_monitor(x, y) {
|
||||
log::warn!("bubble position claude ({x},{y}) outside all monitors; resetting to default");
|
||||
self.claude = None;
|
||||
}
|
||||
}
|
||||
if let Some((x, y)) = self.codex {
|
||||
if !position_on_any_monitor(x, y) {
|
||||
log::warn!("bubble position codex ({x},{y}) outside all monitors; resetting to default");
|
||||
self.codex = None;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn position_on_any_monitor(x: i32, y: i32) -> bool {
|
||||
// MONITOR_DEFAULTTONULL returns a null HMONITOR when the rect intersects
|
||||
// no connected monitor — exactly the signal we want.
|
||||
let probe = RECT {
|
||||
left: x,
|
||||
top: y,
|
||||
right: x + POSITION_PROBE_PX,
|
||||
bottom: y + POSITION_PROBE_PX,
|
||||
};
|
||||
let monitor = unsafe { MonitorFromRect(&probe, MONITOR_DEFAULTTONULL) };
|
||||
!monitor.is_invalid()
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
@@ -132,6 +170,8 @@ pub fn load() -> Settings {
|
||||
settings.bubble_size_logical = settings
|
||||
.bubble_size_logical
|
||||
.clamp(crate::bubble::MIN_BUBBLE_SIZE, crate::bubble::MAX_BUBBLE_SIZE);
|
||||
// Drop positions on monitors that have since been disconnected.
|
||||
settings.bubble_positions.validate();
|
||||
settings
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user