You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
lumen doctor treats "installed" and "running" as the same fact. collect() in crates/lumen-cli/src/doctor.rs appends a menu-bar manager to menu_bar_managers if it is found in the process list or under /Applications, and findings() then emits one Finding::MenuBarManager per entry, which describe() renders identically: the manager "is present and may be holding the icon in an overflow area". An installed-but-not-running manager cannot hold anything, so doctor confidently reports a cause that is impossible. This is worse than a cosmetic bug because doctor exists to be pasted into bug reports: a wrong lead costs exactly the diagnosis round-trip the command was built to remove.
Evidence
crates/lumen-cli/src/doctor.rs, collect(): one push from the ps -ax -o comm scan, a second push for /Applications/<name>.app existing. Both land in the same Vec<String>, so the distinction is gone before findings() sees it.
findings(): iterates menu_bar_managers and pushes a finding for every element unconditionally. describe() gives every one of them the same "is present and may be holding the icon in an overflow area" wording.
Maintainer's machine: doctor reported Ice. pgrep -x Ice returns empty. Ice is installed and not running.
Facts tracks installed and running managers as separate fields; no single merged list remains.
findings() returns no MenuBarManager finding when a manager is installed and not running.
findings() still returns a MenuBarManager finding when a manager is running.
An installed-not-running manager appears only in the fact dump, or in a finding explicitly labelled not running.
Ordering is preserved: IconHiddenByPreference is still the first finding. Note that doctor::tests::the_hidden_icon_preference_is_found_and_comes_first sets the merged list in its fixture, so it needs updating to the new field names to compile — the assertion it makes must survive unchanged.
A unit test covers the installed-not-running case.
findings() is pure and already unit-tested, so this is fixable and testable without touching collection. Note the tradeoff: suppressing installed-only managers loses the case where one starts after doctor ran, and the running check is a case-insensitive substring match over the whole ps output rather than an exact process-name match — tightening it to pgrep -x semantics may miss managers whose helper process name differs from the app name. Neither default is clearly correct; pick one and say which in the finding text.
How to test
git clone https://github.com/HackPoint/lumen &&cd lumen
cargo test -p lumen-cli doctor::
Expect all doctor:: tests to pass, including the new installed-not-running case.
Then, on a macOS machine with a manager installed but not running:
ls -d /Applications/Ice.app # exists
pgrep -x Ice # prints nothing
cargo run -p lumen-cli -- doctor
Expect no MenuBarManager finding, or one explicitly labelled not running. Before the fix, doctor reports the manager as present and possibly holding the icon.
What
lumen doctortreats "installed" and "running" as the same fact.collect()incrates/lumen-cli/src/doctor.rsappends a menu-bar manager tomenu_bar_managersif it is found in the process list or under/Applications, andfindings()then emits oneFinding::MenuBarManagerper entry, whichdescribe()renders identically: the manager "is present and may be holding the icon in an overflow area". An installed-but-not-running manager cannot hold anything, so doctor confidently reports a cause that is impossible. This is worse than a cosmetic bug because doctor exists to be pasted into bug reports: a wrong lead costs exactly the diagnosis round-trip the command was built to remove.Evidence
crates/lumen-cli/src/doctor.rs,collect(): one push from theps -ax -o commscan, a second push for/Applications/<name>.appexisting. Both land in the sameVec<String>, so the distinction is gone beforefindings()sees it.findings(): iteratesmenu_bar_managersand pushes a finding for every element unconditionally.describe()gives every one of them the same "is present and may be holding the icon in an overflow area" wording.pgrep -x Icereturns empty. Ice is installed and not running.Acceptance criteria
Factstracks installed and running managers as separate fields; no single merged list remains.findings()returns noMenuBarManagerfinding when a manager is installed and not running.findings()still returns aMenuBarManagerfinding when a manager is running.IconHiddenByPreferenceis still the first finding. Note thatdoctor::tests::the_hidden_icon_preference_is_found_and_comes_firstsets the merged list in its fixture, so it needs updating to the new field names to compile — the assertion it makes must survive unchanged.findings()is pure and already unit-tested, so this is fixable and testable without touching collection. Note the tradeoff: suppressing installed-only managers loses the case where one starts after doctor ran, and the running check is a case-insensitive substring match over the wholepsoutput rather than an exact process-name match — tightening it topgrep -xsemantics may miss managers whose helper process name differs from the app name. Neither default is clearly correct; pick one and say which in the finding text.How to test
Expect all
doctor::tests to pass, including the new installed-not-running case.Then, on a macOS machine with a manager installed but not running:
Expect no
MenuBarManagerfinding, or one explicitly labelled not running. Before the fix, doctor reports the manager as present and possibly holding the icon.