Recognize an existing Hermes Desktop install in the menu and launcher - #11000
Recognize an existing Hermes Desktop install in the menu and launcher#11000KelpME wants to merge 8 commits into
Conversation
spencerbull
left a comment
There was a problem hiding this comment.
Changes requested on 09eb1a2b3dad1d2ff89c6bf9678a85f0b95d6563, reviewed against quattro at 8ea51516390320f8e768808b230098e67bdaa82c.
This still addresses a real standalone-install problem after #10613 and #10846. The conditional launcher hide needs to handle removal, and the new installed-state guard needs to match the installation it describes.
-
P2 — Removing packaged Hermes exposes a broken launcher. At
shell/services/hidden-entries.sh:98, hidinghermesonly while the package is installed makes the upstream entry visible after removal. The existing remover deletes the native runtime and its owned CLI wrappers but leaves~/.local/share/applications/hermes.desktop. Running the actual remover and hidden-entry scanner in a disposable worker confirmed that removal succeeds, the entry survives, its Exec target is gone, and this PR exposes the entry where the base hides it. Please handle the launcher whose target is removed while preserving working standalone entries, and add coverage for install/update → removal → launcher refresh. -
P2 — The menu guard can inspect the wrong Hermes installation. At
default/omarchy/omarchy-menu.jsonc:246, the new check hardcodes~/.hermes, whereas the installer respectsHERMES_HOMEand normalizes profile paths to their shared root. With a complete old default tree and an empty selected custom home, the generated guard disables Install even though the selected installation is absent. Conversely, complete standalone custom-home and profile-home installations remain undetected. Please use the same home normalization as the installer and cover both directions. The false-positive is introduced here; the custom-install false-negative is an existing target scenario this change leaves unresolved. -
P2 — Incomplete standalone installations are reported as installed. The same guard accepts a bootstrap marker and an executable without checking the required runtime and desktop files. The generated guard still disabled Install with the venv missing,
resources/app.asarmissing, orresources/install-stamp.jsonmissing. Please align the inexpensive file checks with the installer's completeness requirements and add incomplete-install cases, so a broken installation does not suppress the Install action. Avoid adding long launch/version probes to every menu open.
Please also correct the description's claim that a stale standalone marker leaves Remove reachable: remove.ai.hermes remains package-only. Widening removal is not implied by standalone detection; upstream's ordinary installer writes the same bootstrap marker, so that marker alone does not establish Omarchy ownership.
The existing #10613 behavior that prevents duplicate launchers after native updates should remain intact for packaged installs. #10846 fixes the relocated Python stamp helper and the menu icon and does not supersede this PR.
GPT-6 in Codex/T3 Code reviewed the change, with a second GPT-6 reviewer at xhigh. The second reviewer identified the removal regression and wrong-home false-positive, and the primary reviewer independently verified the source and reproduced both; it also identified the incomplete-install coverage gap. The second review was instructed not to consult the primary transcript, but read-scope isolation was not enforced. The PR combined cleanly with current quattro; 315 focused CLI, launcher, menu, guard, installer, and remover checks passed in a disposable credential-free worker. Separate probes reproduced the cases above. The full shell suite and rendered GUI installation/update/removal flow were not run.
This is an automated Omabot review posted on behalf of @spencerbull.
🤖 Generated by GPT-6 in Codex/T3 Code. Reviewed by GPT-6 at xhigh.
- hidden-entries.sh resolves the hermes entry's Exec target (quoted or bare, arguments stripped per the desktop-entry spec) and keeps it hidden when the target no longer exists: the remover tears down the runtime but leaves the launcher entry behind. - New hidden helper omarchy-hermes-desktop-present mirrors the installer's home resolution and reads every file its readiness checks read, so the Install row dims only for a complete standalone install. - Menu guard delegates to it; fixture Exec paths and test ordering fixed.
1e5f2fe to
6d598f0
Compare
|
Thanks for the review — all three P2s confirmed against source and addressed, pushed on top of the rebased 8ea5151. P2-1 (removal exposes a dead launcher): P2-2 (guard inspects the wrong home): the inline guard is gone. A new hidden helper, P2-3 (incomplete installs read as installed): the helper reads all eight files the installer's readiness checks read — bootstrap marker, venv Description: corrected — Remove stays package-only, and the body now says so explicitly (a bootstrap marker is written by upstream's ordinary installer too, so it alone establishes nothing about ownership). Rebased onto 8ea5151 (no conflicts with #10846's installer changes — it touches |
The default-home case built its runtime at <home>/hermes-agent while the helper reads <home>/.hermes/hermes-agent, so it failed on a clean machine and passed only where a developer's own HERMES_HOME pointed at a complete install. It now builds under .hermes and empties HERMES_HOME so nothing outside the fixture can answer for it. The profile case said it parked a runtime under the profile directory but built an unused tree beside it; it parks one there now, so a helper that stopped flattening is caught by that case. The HERMES_HOME case gets its own tree instead of borrowing the default home's. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…and is gone The launcher re-reads its hidden entries only when a .desktop file changes. Removing the package changes one, and that rescan runs while the runtime and its wrapper still exist, so the upstream entry is judged live; the teardown that follows deletes its command without touching any .desktop file, and nothing rescans. On a runtime of realistic size the last rescan lands more than a second before the teardown finishes, and the launcher keeps offering a Hermes that launches nothing until some unrelated entry changes. Deleting the entry from the remover, when the command its Exec names no longer exists, is itself the change that triggers the rescan, and an entry that still launches something is left alone. The scan's own dead-target rule stays for a runtime deleted by hand, and its comments now say so. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A desktop entry may name its command without a path, and the launcher resolves that on PATH. Testing it with -e answered against the working directory instead, so a hand-written entry naming a working command read as dead: the scan would hide it, and the remover, which deletes an entry whose command is gone, would delete it. Both now look a bare name up on PATH and check a path where it points. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Co-Authored-By: Codex XHigh <noreply@openai.com>
A desktop entry's Exec may be relative, resolved by the launcher against the entry's own Path=. Neither the scan nor the remover reads Path=, so checking such a command with -e answered against their own working directory: the scan hid the entry and the remover deleted it. A relative command is now taken as live by both, and only an absolute path or a bare name is judged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Co-Authored-By: Codex XHigh <noreply@openai.com>
The remover and the hidden-entries scan call omarchy-cmd-present, and the tests running them put only their stub directory ahead of PATH, so the helper came from an installed Omarchy and was missing on a bare checkout, where the cases naming a command on PATH failed. The checkout's bin/ now follows the stubs on PATH, as docs/testing.md says tests must never depend on an installed Omarchy. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Co-Authored-By: Codex XHigh <noreply@openai.com>
|
Re-reviewed at What moved since the last round. P2-2 and P2-3 are fixed as described: the helper mirrors the installer's
Left as caveats, not changed. The helper skips the installer's Description. The paragraph saying the remover leaves the runtime's launcher entry behind and that the scan returns the launcher to its pre-install state no longer describes the branch: the remover deletes the entry, and the scan covers a runtime deleted by hand. Checked, on a disposable worker built from the Omarchy ISO with the shell serving this branch. Second opinion. Codex at xhigh reviewed three heads, the bound set for this review. On Waiting on the maintainer. This is an automated Omabot review posted on behalf of @spencerbull. 🤖 Generated by Claude Fable 5.1 in Claude Code. Reviewed by Codex XHigh. |
Install > AI > Hermes offers to install Hermes Desktop on machines where it is already installed, and the launcher hides the one entry that works. Both answer only
omarchy-pkg-present hermes-desktop, but the desktop app can be on the machine without the package: the upstream runtime's own installer writes ahermes.desktoplauncher and a complete install under~/.hermesbefore Omarchy ever asks. That is everyone who set Hermes up before Omarchy shipped Hermes support, and everyone who installed it from upstream since. For them the menu keeps offering a redundant package install — clicking it adds the package alongside the working runtime and changes nothing about the menu's picture of the machine — and since 9fb73b0, the statichermesline inlauncher.hideshides their only working launcher entry, so search cannot open an app that is there and running. On the incomplete standalone installs the old offer is worse than redundant: a runtime without its built app fails the installer's own integrity checks and dead-ends in "The Hermes runtime has moved beyond the packaged desktop release".The hide moves with ownership instead of being static.
hidden-entries.shalready produces the launcher's hidden list at runtime, so it now emitshermesonly while thehermes-desktoppackage is present — exactly the condition the packaged takeover assumes — and the static line leaveslauncher.hides. A packaged install hides the upstream entry as before; a standalone install keeps it, because it is the only launcher the app has. The remover finishes what removal starts: it deletes the runtime's launcher entry once the command its Exec names is gone — and since the launcher re-reads hidden entries only when a .desktop file changes, that deletion is itself the event that refreshes the list. For a runtime deleted by hand rather than through Remove, the scan resolves the entry's Exec target — quoted or bare, arguments stripped per the desktop-entry spec, a bare name looked up on PATH — and keeps it hidden when the target no longer exists. A working standalone entry always resolves and stays visible.The Install row learns the same second answer through a new hidden helper,
omarchy-hermes-desktop-present. Putting the check in a command rather than inline guard text keeps one definition of "a standalone install is complete": the helper mirrors the installer's home resolution —HERMES_HOMEwins, a path under aprofilesdirectory flattens to the shared root above it — and then reads the same files the installer's own readiness checks read, the bootstrap marker, the venv commands, and the native app's binary, app.asar and install stamp. File tests only: the guard batch runs per menu open, and no probe ever launches Hermes. The row dims (✓) only when a complete install is there; an incomplete one keeps the Install action active. The installer itself is left alone on purpose: with the guard fixed, a working install is dimmed before it can be clicked, and teaching the installer to adopt foreign runtimes would be a different conversation than detecting them. Removal stays package-only: a bootstrap marker is written by upstream's ordinary installer too, so it does not establish Omarchy ownership, and widening removal is a separate question.Verified
test/shell.d/hermes-desktop-present-test.sh(new): the helper against throwaway homes — a complete default-home install recognized; aprofiles/ptahhome normalized to the shared root before checking (and a runtime parked under the profile directory itself rejected);HERMES_HOMEhonored; an install missing each of the six required files not reported present; nothing at all answers false. 5 ok.test/shell.d/hidden-entries.shcoverage (new cases):hermeshidden with the package present, visible without it when the Exec target exists, hidden again after removal leaves the entry with a dead target, and a quoted Exec (Exec="<home>/.local/bin/hermes" desktop) resolves the quoted binary. 5 ok.guardScriptbatch wrapper across four states: standalone install without the package (dimmed ✓), clean machine (active), packaged install (dimmed ✓), incomplete standalone install (active).bash -nclean on the generated script../test/cli: green. Focused Hermes/menu suites (hermes-desktop-install,hermes-cli,hermes-remove,hermes-cli-migration,app-search,menu,menu-guards): green on the rebase onto 8ea5151.