fix(tray): stop the Windows tray launching the CLI instead of the GUI - #460
Conversation
Greptile SummaryThis PR fixes a binary name collision in the Windows tray on dev/cargo-target-dir builds:
Confidence Score: 5/5Safe to merge — the change is a targeted, well-reasoned fix for a real binary-name collision on Windows dev builds, with no regressions in the installed layout. Both changes are narrow and correctly address the root cause. The case-sensitive process-name match relies on NTFS preserving on-disk casing in process image names (documented in the new comment), which is the correct assumption for this Windows API surface. The spawn probe reorder eliminates the case-insensitive exists() false-positive without breaking the installed layout. The added unit test explicitly exercises the CLI-rejection path. No logic regressions were found. Files Needing Attention: No files require special attention.
|
| Filename | Overview |
|---|---|
| crates/openlogi-agent/src/tray_windows.rs | Fixes GUI/CLI binary confusion on Windows dev layouts: case-sensitive match for OpenLogi.exe, probe openlogi-gui.exe first in spawn_gui, and adds a unit test covering all three key cases. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Tray: Show Main Window clicked] --> B[gui_pids]
B --> C{is_gui_process_name?}
C -->|name == 'OpenLogi.exe' case-SENSITIVE| D[GUI PID collected]
C -->|name eq_ignore_ascii_case 'openlogi-gui.exe'| D
C -->|'openlogi.exe' CLI - NO MATCH fixed| E[Rejected]
D --> F{pids empty?}
F -->|No| G[focus_window_of pids]
F -->|Yes| H[spawn_gui]
H --> I{Probe order}
I -->|1. openlogi-gui.exe unambiguous dev binary| J{exists?}
J -->|Yes| K[Spawn openlogi-gui.exe]
J -->|No - installed layout| L{OpenLogi.exe exists?}
L -->|Yes| M[Spawn OpenLogi.exe]
L -->|No| N[Warn: no GUI binary found]
Reviews (3): Last reviewed commit: "test(tray): pin that the Windows CLI bin..." | Re-trigger Greptile
The CLI binary `openlogi.exe` case-insensitively equals the GUI's
`OpenLogi.exe`, so in a cargo target dir — the only layout holding both — the
tray's Show/Quit mistook a transient CLI run for the GUI, and `spawn_gui`
launched the CLI (`dir.join("OpenLogi.exe").exists()` resolves to it on the
case-insensitive filesystem) instead of the window.
Match `OpenLogi.exe` case-sensitively so the lowercase CLI no longer counts as
the GUI, and probe the unambiguous `openlogi-gui.exe` first when spawning.
The GUI/CLI discrimination is one string comparison, and its failure mode is silent: Show would spawn a duplicate GUI that immediately exits on the singleton lock, so nothing visible happens. Extract the predicate out of `gui_pids`'s filter and assert all three names, so a regression fails a local `cargo test` on Windows instead of only showing up as a dead tray menu. CI does not run it: the module is cfg'd to Windows and the only Windows job is clippy, which compiles the test but never executes it. Also record why the two names are matched differently — `OpenLogi.exe` exactly (the CLI `openlogi.exe` collides with it case-insensitively) and `openlogi-gui.exe` case-insensitively (nothing else shares that name).
0a202a1 to
85e67e6
Compare
|
@davidbudnick mind taking a look at this one? Thanks. |
Yup, looks good to me! |
Summary
On Windows the tray's "Show Main Window" launched the CLI instead of the GUI, and
Show/Quit could report
GUI process is running but no window was found to focus.Root cause:
tray_windows.rsidentifies the GUI by filename —OpenLogi.exe(installed) or
openlogi-gui.exe(dev). The CLI binary isopenlogi.exe, and twoindependent things went wrong in a cargo target dir:
gui_pids()compared process image names witheq_ignore_ascii_case, so a transientopenlogi.exeCLI run counted as the GUI — hence the "no window to focus" report.This is the comparison's own doing, not the filesystem's.
spawn_gui()probeddir.join("OpenLogi.exe").exists(). Win32 path lookup iscase-insensitive, so that name resolved to the CLI's
openlogi.exeand the traylaunched it.
The installed layout ships only
OpenLogi.exe+openlogi-agent.exe(no CLI), so thisonly bites dev/portable layouts — but it's a real filename-collision bug.
Changes
is_gui_process_name()and match the product GUIOpenLogi.execase-sensitively — Windows reports image names with their on-diskcase, so the lowercase CLI
openlogi.exeno longer counts as the GUI.openlogi-gui.exefirst inspawn_gui(), so.exists()can't resolveOpenLogi.exeto the CLI on the case-insensitive filesystem.Testing
cargo clippy -p openlogi-agent --all-targets -- -D warnings— clean.Note the new
is_gui_process_nametest sits in a Windows-only module, so it runs only ona Windows host — CI's test jobs are macOS/Linux, so they skip it.
clippy (windows)isthe only CI job that compiles this file.
Windows-only tray behaviour, not runtime-tested on hardware: verify by rebuilding the
agent and clicking the tray — it should open the GUI window, not print a device list.
Fixes the wrong-binary launch reported on Windows dev builds.