Skip to content

Restore application access for third-party menus - #10671

Open
smillunchick wants to merge 1 commit into
omacom:quattrofrom
smillunchick:fix/menu-plugin-manifest
Open

Restore application access for third-party menus#10671
smillunchick wants to merge 1 commit into
omacom:quattrofrom
smillunchick:fix/menu-plugin-manifest

Conversation

@smillunchick

@smillunchick smillunchick commented Sep 7, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

Restore the app list for third-party menu plugins such as Omalaunch after the scoped plugin API change.

  • Read panel manifests from the validated plugin registry, not the Qt model's converted copy.
  • Forward iconIndexChanged through the app-library facade so menus can finish their icon-refresh cycle. This exposes a signal, not the private icon index.
  • Add an offscreen Quickshell regression test for menu access, non-menu denial, and app/icon change signals.

Cause

Reproduced with Omalaunch 0.3.0 and omarchy-dev 4.0.0.r2071.ga703092-1. The current quattro branch has the same affected shell code.

computePanelEntries() accepts the menu manifest, then passes it through an Instantiator. Qt converts its nested kinds array to a sequence: it still serializes as ["menu","bar-widget"] and supports indexOf("menu"), but Array.isArray(modelData.manifest.kinds) returns false. manifestHasKind() then rejects the menu grant, and the injected PluginShellApi.appLibrary is null. The launcher shows an empty Apps menu even though desktop entries remain installed.

The fix keeps the existing capability checks and uses the registry's original validated object. It does not restore the full shell object or widen non-menu access.

Once the app library attaches, Omalaunch also needs its existing iconIndexChanged completion signal; without it, an icon refresh can stay pending indefinitely.

Validation

Passed:

  • bash test/shell.d/panel-menu-capability-test.sh — real Qt model conversion, actual facade components, and the production binding/grant/signal code; works without a compositor.
  • bash test/shell.d/plugin-auth-boundary-test.sh — static checks and QML runtime fixture.
  • bash test/shell.d/plugin-registry-contract-test.sh — QML runtime fixture.
  • bash test/shell.d/app-search-test.sh.
  • git diff --check.

The new test first failed on the missing menu grant, then on missing icon completion after applying only the manifest fix. It passes with both changes. Existing authentication and ordinary-panel restrictions still pass.

The patched full desktop has not been visually verified. The installed, package-owned shell was left unchanged; no development link, permission change, migration, or release is included.

Companion fix

Omalaunch separately relies on the now-private manifest.__sourceDir for its helper paths. Its companion PR resolves the plugin directory from its own QML file: DanielLemky/omalaunch#58

Both fixes are needed to restore that launcher on the affected host.

@smillunchick smillunchick mentioned this pull request Sep 7, 2026
Merged
@EFrMG EFrMG mentioned this pull request Sep 11, 2026
Open
@NonMirror

NonMirror commented Sep 13, 2026 •
edited
Loading

Copy link
Copy Markdown

Independent verification on current quattro

Environment: Omarchy 4.0.3-1, Quickshell 0.3.1 (Arch), Qt 6.11.2.

Method (live session untouched):

  1. Shallow-cloned quattro at 692c02c (your base is 5b91db5; the three production hunks still apply cleanly).
  2. Copied only test/shell.d/panel-menu-capability-test.sh + its fixture from 9c24d86 onto the unpatched tree. The test reproduces the defect: 
    ERROR qml: PANEL_MENU_FAIL ["example.menu: wrong app-library grant","icon refresh completion was not forwarded"]
not ok - menu plugins retain their app library across the Qt model boundary
  3. Applied the three production changes (PluginAppLibraryApi.iconIndexChanged, the forwarding Connections handler, and the panel-delegate manifest binding to pluginRegistry.installedPlugins[pluginId]): 
    ok - menu plugins retain their app library across the Qt model boundary
ok - ordinary panels cannot access the app library
ok - app and icon refresh signals reach menu plugins

So the fix is still valid on current quattro, and the included test fails before / passes after exactly as described. Caveat: I only ran your offscreen fixture, not a live compositor session, so live verification remains open.

As noted in #11241, this PR and that one share the same delegate/registry-manifest approach. This one is older and additionally ships the real Instantiator fixture plus iconIndexChanged forwarding, so it looks like the more complete of the two for that strategy.

@NonMirror NonMirror mentioned this pull request Sep 13, 2026
Open
@NonMirror

NonMirror commented Sep 13, 2026

Copy link
Copy Markdown

Follow-up: regression check on the patched tree. With the three production hunks applied to 692c02c, the relevant existing shell tests plus the new fixture all pass:

plugin-auth-boundary-test.sh        ok=13  fail=0
plugin-registry-contract-test.sh    ok=1   fail=0
plugin-clone-test.sh                ok=20  fail=0
bar-widget-contract-test.sh         ok=1   fail=0
app-search-test.sh                  ok=24  fail=0
menu-test.sh                        ok=121 fail=0
menu-plugin-test.sh                 ok=12  fail=0
panel-menu-capability-test.sh       ok=3   fail=0

No regressions in the plugin/panel/menu area. (Did not run the full ./test/shell suite — a couple of files there need a sibling omarchy-pkgs checkout.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dhh dhh Awaiting requested review from dhh
@ryanrhughes ryanrhughes Awaiting requested review from ryanrhughes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@smillunchick @NonMirror