[CFX-6915] Fix plugin update/discovery bug in XDG_CONFIG_DIRS#677
[CFX-6915] Fix plugin update/discovery bug in XDG_CONFIG_DIRS#677victorborshak wants to merge 2 commits into
Conversation
|
👋 Thanks so much for contributing to the DataRobot community! As a quick heads-up on how our team handles reviews: if you're still iterating on Once everything is finalized and you're ready for feedback, just click "Ready for review" |
Code OwnershipCli Maintainers
Review requested from the teams above. Labels will be removed automatically upon approval. |
victorborshak
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6e0a2cf. Configure here.
| } | ||
|
|
||
| return installed, nil | ||
| } |
There was a problem hiding this comment.
Discovery-mutation inconsistency for XDG_CONFIG_DIRS plugins
Medium Severity
GetInstalledPlugins now discovers plugins from XDG_CONFIG_DIRS via ManagedPluginsDirs(), but UninstallPlugin, BackupPlugin, ValidatePlugin, and RestorePlugin still use ManagedPluginsDir() (singular, primary directory only). A plugin discovered exclusively from XDG_CONFIG_DIRS will pass the "is installed" check in the uninstall/update commands, but the subsequent operation will fail with "Plugin X is not installed" because it only looks in XDG_CONFIG_HOME. Before this PR, both discovery and mutation used the same single directory, so they were always consistent.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6e0a2cf. Configure here.
There was a problem hiding this comment.
should be fixed now
|
🔐 Fork smoke tests started by maintainer ⏳ Security scans passed. Running smoke tests... Commit: |
|
✅ All smoke tests passed! (Fork PR) ✅ Security Scan: success |
3 participants
RATIONALE
In plugin update/validate/backup flows, plugin discovery did not take into account plugins installed in
XDG_CONFIG_DIRS(only those installed inXDG_CONFIG_HOMEor~/.configwere discovered). This PR fixes it.CHANGES
PR Automation
Comment-Commands: Trigger CI by commenting on the PR:
/trigger-smoke-testor/trigger-test-smoke- Run smoke tests/trigger-install-testor/trigger-test-install- Run installation testsLabels: Apply labels to trigger workflows:
run-smoke-testsorgo- Run smoke tests on demand (only works for non-forked PRs)Important
For Forked PRs: The
run-smoke-testslabel won't work. A required Smoke Tests check will block merge until a maintainer acts:/approve-smoke-teststo run smoke tests (results will set the check)/skip-smoke-teststo bypass the check without running testsPlease comment requesting a maintainer review if you need smoke tests to run.
Note
Low Risk
Localized change to installed-plugin metadata listing with behavior covered by new tests; install/uninstall paths still use the primary dir only.
Overview
GetInstalledPluginsno longer reads only the primary managed plugins path (XDG_CONFIG_HOME/~/.config). It now walks every directory returned byManagedPluginsDirs, in priority order, and deduplicates by plugin name soXDG_CONFIG_HOMEwins when the same name exists in multiple locations.The per-directory scan is moved into
getInstalledPluginsInDir; missing secondary dirs are still treated as empty (no error). New tests cover discovery-only-in-XDG_CONFIG_DIRS, shadowing, merging distinct names, and skipping a non-existentXDG_CONFIG_DIRSentry.Reviewed by Cursor Bugbot for commit 6e0a2cf. Configure here.