Skip to content

fix(settings): use store installation path in diagnostics - #13

Merged
bernardopg merged 1 commit into
bernardopg:mainfrom
emmsixx:fix/settings-store-install-path
Aug 8, 2026
Merged

fix(settings): use store installation path in diagnostics#13
bernardopg merged 1 commit into
bernardopg:mainfrom
emmsixx:fix/settings-store-install-path

Conversation

@emmsixx

@emmsixx emmsixx commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Summary

plugin.json's manifest id is aiOverviewControl, and that's the directory name DMS's plugin store installs the plugin under. The Settings diagnostic commands ("Test selected providers", "Test Codex app-server adapter", "Test pi session analytics adapter", "Check provider prerequisites", "Validate QML") instead hardcoded the display-name casing, AiOverviewControl. On a case-sensitive filesystem that path doesn't exist for a store install, so every copied command fails with no such file or directory until the user manually fixes the casing (or lets Zsh tab-completion do it for them).

The widget (AiOverviewControlWidget.qml) already solved this in 1.2.2 via PluginService.getPluginPath(pluginId) with a Qt.resolvedUrl fallback — Settings never got the same treatment. This PR applies the identical, already-proven pattern to AiOverviewControlSettings.qml and rebuilds every diagnostic command from the resolved _pluginDir instead of a literal path.

  • Doc paths under docs/installation.md, README.md, and docs/README.pt-BR.md are left as-is: they document a manual checkout/archive install where the user themselves creates the AiOverviewControl-cased directory, so that casing is correct there.
  • Added a short clarifying note to docs/troubleshooting.md's manual PLUGIN= snippet, since a store-installed user could otherwise hit the same casing mismatch when following it.
  • Added a CI check (.github/workflows/ci.yml, QML lint job) that fails if any .qml file hardcodes plugins/AiOverviewControl or plugins/aiOverviewControl again — confirmed it fails against the pre-fix file and passes against this branch.

Test plan

  • qmllint AiOverviewControlWidget.qml AiOverviewControlSettings.qml AiOverviewControlI18n.qml ProviderLogo.qml — exit 0
  • Reproduced the bug: ~/.config/DankMaterialShell/plugins/AiOverviewControl/providers/get-codex-usageno such file or directory (exit 127) on a store install at plugins/aiOverviewControl
  • Ran the corrected path directly: ~/.config/DankMaterialShell/plugins/aiOverviewControl/providers/get-codex-usage | jq . → succeeds
  • Ran ~/.config/DankMaterialShell/plugins/aiOverviewControl/providers/get-provider-health "codex,claude,copilot" | jq . → succeeds
  • rg -n 'plugins/AiOverviewControl|plugins/aiOverviewControl' . reviewed — only manual-install docs remain
  • New CI grep check verified to fail against the pre-fix QML and pass against this branch

The manifest id is aiOverviewControl, but DMS plugin-store installs
place the plugin directory under that id. The Settings diagnostic
commands hardcoded the display-name casing (AiOverviewControl)
instead, which does not exist on a store install and fails with
"no such file or directory" on case-sensitive filesystems.

Settings now resolves its own plugin directory the same way the
widget already does (PluginService.getPluginPath, falling back to
Qt.resolvedUrl), so every diagnostic command matches the actual
on-disk path regardless of install method. Adds a CI check that
fails if any QML hardcodes the plugins/AiOverviewControl or
plugins/aiOverviewControl path again, and a note in the
troubleshooting doc's manual PLUGIN= snippet about store-install
casing.
@github-actions github-actions Bot added documentation Improvements or additions to documentation area:qml QML UI code area:ci CI, workflows, tooling labels Aug 8, 2026
@bernardopg

Copy link
Copy Markdown
Owner

Hey @emmsixx — fantastic catch, and thank you! 🎉

This is exactly the class of bug that's invisible until it bites a store-install user on a case-sensitive filesystem: every Settings diagnostic quietly 127'd because plugins/AiOverviewControl doesn't exist when the store lands the plugin under its manifest id aiOverviewControl. The writeup was airtight — you traced it to the precise divergence (the Widget already fixed this in 1.2.2 via PluginService.getPluginPath + a Qt.resolvedUrl fallback; Settings never got the same treatment), reused the proven resolution path instead of reinventing one, and even shipped a CI grep gate that demonstrably fails on the pre-fix file and passes on the fix. Reproducing it against a real store install in the test plan is what sealed it.

A few things I specifically appreciate:

  • No scope creep — you left the manual-install doc paths alone (where AiOverviewControl casing is correct by definition) and only touched what was actually broken.
  • Symmetry with the Widget — same two-step resolution (PluginService authoritative, Qt.resolvedUrl fallback), so reviewers didn't have to learn a new pattern.
  • The regression guard — a CI step that fails on the old code is worth more than any description.

All checks are green and it merges clean — landing it now. As a small follow-up I'm tightening that same CI gate (making the grep recursive so it still fires if QML ever moves into a subdirectory) and adding a structural check that every providers/<script> referenced from QML resolves to an existing, executable file — so a future rename can't silently produce a broken copy-to-clipboard command. I'm also adding you to the Contributors section. 👏

Welcome aboard as a contributor — the plugin is genuinely better for this. 🙇

@bernardopg
bernardopg merged commit c9bf874 into bernardopg:main Aug 8, 2026
7 checks passed
bernardopg added a commit that referenced this pull request Aug 8, 2026
Recognizes the merged store-install-path fix in #13 (authored by
@emmsixx) as the first external contribution, and gives future
contributors a visible home.
bernardopg added a commit that referenced this pull request Aug 8, 2026
Follow-up to #13.

Tighten the 'No hardcoded install path in QML' gate introduced in #13:
the grep is now recursive with --include='*.qml' so it still fires if QML
ever moves into a subdirectory. Add a new structural check mirroring the
shell job's 'Provider dispatch coverage' style: every providers/<script>
referenced from any .qml must exist and be executable, so a renamed/removed
helper can't silently produce a broken copy-to-clipboard command. Verified
the gate fails on both a dangling reference and a non-executable file.

Recognize @emmsixx for #13 (the plugin's first external contribution) with
a Contributors section in README.md and a Colaboradores section in
docs/README.pt-BR.md.

No QML changed; qmllint (all 4 files) and the new checks pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:ci CI, workflows, tooling area:qml QML UI code documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants