Skip to content

Route menu ids ahead of app keyword aliases - #6563

Merged
dhh merged 1 commit into
quattrofrom
menu-route-id-precedence
Aug 5, 2026
Merged

Route menu ids ahead of app keyword aliases#6563
dhh merged 1 commit into
quattrofrom
menu-route-id-precedence

Conversation

@dhh

@dhh dhh commented Aug 5, 2026

Copy link
Copy Markdown
Member

Fixes #6554. Thanks to @craigderington for the excellent report and root-cause analysis.

Bug

SUPER + ESCAPE (omarchy-menu toggle system) opened an empty "Htop" menu instead of the System menu when htop is installed. resolveRoute() scanned every item's aliases before considering the input as a literal id, and app rows inherit their .desktop Keywords as aliases — htop ships Keywords=system;..., so the app row captured the system route. App rows have no children and no action, so the fallthrough rendered an empty "Nothing here yet" menu.

Two details beyond the report, from reproducing it live:

  • The bug is state-dependent: app rows merge lazily the first time the Apps menu loads and then persist, so the binding works after login and breaks for the rest of the shell session once the Apps menu has been opened. That's worth knowing when triaging "works for me" reports against this.
  • Stock installs are unaffected by btop despite its Keywords=system;process;task, because launcher.hides keeps btop out of the app library. It takes htop (or any other non-hidden app with a colliding keyword) to trigger it.

Fix

resolveRoute moved into MenuModel.js (where it's unit-testable) with two changes:

  • An exact item id wins before any alias is considered.
  • App rows are skipped in the alias scan entirely. Their keywords exist to power search — which is unaffected, since search matches through nameSearchText, not route resolution — and any third-party .desktop file could otherwise shadow a current or future menu route. Routing to an app row was never meaningful: it always produced the empty-menu fallthrough.

Declared JSONC aliases (power-menu, settings, …) keep working as before.

Verified

  • test/shell.d/menu-test.sh covers id-over-keyword precedence, app rows being unroutable, alias routing, underscore normalization, root/literal fallthroughs, and keyword search still matching.
  • Full ./test/shell run: 940 passing; the one failure (model-usage-default-migration-test.sh) pre-exists on clean quattro.
  • Reproduced live on a running shell with a desktop file carrying Keywords=system;: before the fix summon system opened the empty app menu after the Apps menu had been opened once; after the fix it opens the System menu, summon power-menu still routes to it, and typing "system" in the menu still surfaces the app as the top search hit.

🤖 Generated with Claude Code

https://claude.ai/code/session_01636Ddgizwt8kEWStGY4szj

An installed app whose .desktop Keywords contain a menu id captured the
route: htop ships Keywords=system;..., so SUPER+ESCAPE opened an empty
"Htop" menu instead of the System menu once the Apps menu had merged its
rows. Exact ids now win, and app rows are no longer routable at all —
their keywords remain search-only.

Fixes #6554

Reported-by: Craig Derington (https://github.com/craigderington)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings August 5, 2026 20:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes menu route collisions by prioritizing literal menu IDs and excluding application keywords from alias routing.

Changes:

  • Moves route resolution into the testable menu model.
  • Preserves application keyword search while preventing route capture.
  • Adds regression coverage for routing precedence and normalization.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
shell/plugins/menu/MenuModel.js Implements ID-first, app-safe route resolution.
shell/plugins/menu/Menu.qml Delegates route resolution to the shared model.
test/shell.d/menu-test.sh Adds routing and search regression tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dhh
dhh merged commit 0269fe0 into quattro Aug 5, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Menu: SUPER+ESCAPE opens an empty "Htop" menu instead of the System menu (resolveRoute matches app .desktop Keywords before literal ids)

2 participants