fix(plugin-store): install flow reliability + soundcloud v1.2.0 registry bump - #84
Merged
Conversation
…set URL Cargo replaces hyphens with underscores when building cdylib targets (Rust identifiers disallow hyphens), so the WASM release asset ends up as `vortex_mod_<plugin>.wasm` on GitHub. The host was interpolating the raw registry `name` (kebab-case) into the download URL, producing a 404 for every plugin published with the standard Cargo build pipeline. Normalise at URL construction time so the host accepts the natural Cargo output without requiring each mainteneur to rename assets manually.
… first A plugin loaded in memory (by the startup scan, the file watcher, or a previous install in the same session) made any subsequent store install fail with AlreadyExists — even when the UI showed the plugin as 'not_installed' because the cache had not been refreshed in between. Unload the in-memory instance before loading so reinstallation is a safe no-op on the registry side. `handle_store_update` now delegates to `handle_store_install` since the unload step is shared.
…ore_list The cache stored both the remote registry snapshot AND the install status. That mix forced every loader mutation to rewrite the cache — a step that was missing after a successful `handle_store_install`, so the UI kept showing 'not_installed' until a manual store refresh. Treat the cache as a pure snapshot of remote data and re-derive `installed_version` + `status` from `PluginLoader::list_loaded()` on every `handle_store_list` call. The loader is the single source of truth for what's actually loaded, and the UI reflects install / uninstall immediately without needing a network round-trip. Also drops `commands/store_list.rs`, a dead file never referenced by the module tree (Cargo silently ignores orphan .rs files, so the divergent implementation never triggered a compile error).
Release v1.2.0 (https://github.com/mpiton/vortex-mod-soundcloud/releases/tag/v1.2.0) adds artist profile support with pagination, richer collection metadata, and a yt-dlp `download_to_file` fallback for HLS-only tracks. The v1.1.0 release artefact also shipped a plugin.toml whose internal version was never bumped from 1.0.0, so users stuck on v1.1.0 would see a permanent "update available" loop. v1.2.0 fixes the metadata alongside.
📝 WalkthroughWalkthroughUpdated plugin registry entry for Changes
Sequence DiagramsequenceDiagram
participant Client
participant QueryHandler as Store Query<br/>Handler
participant Cache
participant PluginLoader
participant DTO as PluginStoreEntry<br/>DTO
Client->>QueryHandler: get_plugin_store()
QueryHandler->>Cache: read_cache()
Cache-->>QueryHandler: Vec<PluginStoreEntryDto>
QueryHandler->>PluginLoader: list_loaded()
PluginLoader-->>QueryHandler: [loaded_plugins]
loop each DTO
QueryHandler->>DTO: enrich_with_installed(installed_version)
DTO->>DTO: derive_status_str() (normalize semver, compare)
DTO-->>QueryHandler: enriched DTO
end
QueryHandler-->>Client: enriched Vec<PluginStoreEntryDto>
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR resolves three independent bugs in the plugin install pipeline — URL hyphen/underscore mismatch causing 404s, re-install failures due to an already-loaded entry, and stale UI status after install — plus removes a dead duplicate file and bumps the SoundCloud registry entry to v1.2.0. The fixes are well-scoped, the new unit tests cover the described edge cases, and the status-derivation logic is handled cleanly with the new Confidence Score: 5/5Safe to merge — all three pipeline bugs are correctly fixed, test coverage is thorough, and no new regressions are introduced. All findings are P2 or lower (the only noted concern — the existing store-list test not exercising the enrichment path — was already flagged in a prior review). No P0/P1 issues were found in the new code paths. get_plugin_store.rs — the existing test does not cover the full enrichment path through handle_store_list, but this does not affect correctness of the new code.
|
| Filename | Overview |
|---|---|
| src-tauri/src/adapters/driven/plugin/github_store_client.rs | Adds name.replace('-', "_") in build_wasm_url so the download URL matches Cargo's cdylib output; two new tests cover the hyphen-normalisation and no-hyphen cases. |
| src-tauri/src/application/commands/store_install.rs | Pre-unload added before load_from_dir to make re-installs idempotent; only NotFound is swallowed, other loader errors abort the install correctly. |
| src-tauri/src/application/queries/get_plugin_store.rs | Status enrichment from live loader is correct; existing test test_handle_store_list_returns_cached_entries bypasses handle_store_list and thus does not cover the enrichment path. |
| src-tauri/src/application/read_models/plugin_store_view.rs | New derive_status_str correctly strips pre-release/build-metadata before semver comparison; seven new tests cover all status transitions and suffix variants. |
| src-tauri/src/application/commands/store_list.rs | File deleted — was a dead duplicate of queries/get_plugin_store.rs never wired into commands/mod.rs; cleanup is correct. |
| registry/registry.toml | SoundCloud plugin bumped from 1.1.0 to 1.2.0 with updated SHA-256 checksums for both the WASM and plugin.toml assets. |
Sequence Diagram
sequenceDiagram
participant UI
participant CommandBus
participant Cache
participant StoreClient
participant PluginLoader
UI->>CommandBus: handle_store_list(cache_path)
CommandBus->>Cache: read_cache()
Cache-->>CommandBus: Vec<PluginStoreEntryDto> (registry snapshot)
CommandBus->>PluginLoader: list_loaded()
PluginLoader-->>CommandBus: Vec<LoadedPlugin>
loop for each DTO
CommandBus->>CommandBus: enrich_with_installed(live version)
end
CommandBus-->>UI: enriched DTOs (live status)
UI->>CommandBus: handle_store_install(name)
CommandBus->>Cache: read_cache() find entry
CommandBus->>StoreClient: download_plugin() build_wasm_url hyphens to underscores
StoreClient-->>CommandBus: plugin_dir (staged files)
CommandBus->>PluginLoader: unload(name) ignore NotFound
CommandBus->>PluginLoader: load_from_dir(plugin_dir)
PluginLoader-->>CommandBus: Ok(())
CommandBus-->>UI: Ok(())
Reviews (4): Last reviewed commit: "fix: address PR review comments" | Re-trigger Greptile
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src-tauri/src/application/queries/get_plugin_store.rs (1)
39-43: Pre-index loaded plugins to avoid repeated linear scans.At Line 39–43,
iter().find(...)is executed for every DTO. Building a name→version map first keeps lookup cost linear overall.⚡ Suggested improvement
+use std::collections::HashMap; ... - for dto in &mut dtos { - let installed_version = loaded - .iter() - .find(|i| i.name() == dto.name) - .map(|i| i.version().to_string()); + let loaded_by_name: HashMap<&str, &str> = + loaded.iter().map(|p| (p.name(), p.version())).collect(); + for dto in &mut dtos { + let installed_version = loaded_by_name + .get(dto.name.as_str()) + .map(|v| (*v).to_string()); dto.enrich_with_installed(installed_version); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/application/queries/get_plugin_store.rs` around lines 39 - 43, The loop over dtos repeatedly calls loaded.iter().find(...) causing O(N*M) behavior; instead build a HashMap from plugin name to version once (e.g., iterate loaded and insert (i.name().to_string(), i.version().to_string())), then in the for dto in &mut dtos loop replace the find call with a direct lookup into that map to set installed_version; reference the dtos variable, the loaded collection, and item methods name() and version() when updating get_plugin_store.rs.src-tauri/src/application/read_models/plugin_store_view.rs (1)
64-71: Consolidate duplicated semver parsing logic.
derive_status_strreintroduces triplet parsing that is already implemented insrc-tauri/src/application/commands/store_install.rs(is_version_compatible). Keeping two parsers increases drift risk between install-eligibility and status computation.♻️ Refactor direction
- fn derive_status_str(registry_version: &str, installed: Option<&str>) -> &'static str { - fn parse_semver(s: &str) -> Option<(u64, u64, u64)> { ... } + fn derive_status_str(registry_version: &str, installed: Option<&str>) -> &'static str { + // use shared helper, e.g. crate::application::versioning::parse_semver_triplet🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/application/read_models/plugin_store_view.rs` around lines 64 - 71, The local semver parsing in derive_status_str (the nested parse_semver function) duplicates logic already in is_version_compatible; replace the nested parser by reusing the shared implementation: either call the existing is_version_compatible (or extract/restore a small parse_semver helper into a shared util module) and import it into plugin_store_view.rs, then update derive_status_str to use that shared parser/check instead of the nested parse_semver to avoid drift between install-eligibility and status computation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src-tauri/src/application/commands/store_install.rs`:
- Around line 104-105: The current call let _ =
self.plugin_loader().unload(&cmd.name); swallows all errors; change it to
explicitly match the Result from plugin_loader().unload(&cmd.name) in the
install flow so you ignore only the NotFound case but propagate or return Err
for any other loader/unload failures to abort the install and avoid inconsistent
in-memory state; reference the unload call on plugin_loader() and the cmd.name
parameter and ensure the function returns/propagates the error (or uses
?/map_err) instead of discarding it.
---
Nitpick comments:
In `@src-tauri/src/application/queries/get_plugin_store.rs`:
- Around line 39-43: The loop over dtos repeatedly calls loaded.iter().find(...)
causing O(N*M) behavior; instead build a HashMap from plugin name to version
once (e.g., iterate loaded and insert (i.name().to_string(),
i.version().to_string())), then in the for dto in &mut dtos loop replace the
find call with a direct lookup into that map to set installed_version; reference
the dtos variable, the loaded collection, and item methods name() and version()
when updating get_plugin_store.rs.
In `@src-tauri/src/application/read_models/plugin_store_view.rs`:
- Around line 64-71: The local semver parsing in derive_status_str (the nested
parse_semver function) duplicates logic already in is_version_compatible;
replace the nested parser by reusing the shared implementation: either call the
existing is_version_compatible (or extract/restore a small parse_semver helper
into a shared util module) and import it into plugin_store_view.rs, then update
derive_status_str to use that shared parser/check instead of the nested
parse_semver to avoid drift between install-eligibility and status computation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 210aaa76-a34b-48f5-9e73-91d4f767dc9a
📒 Files selected for processing (6)
registry/registry.tomlsrc-tauri/src/adapters/driven/plugin/github_store_client.rssrc-tauri/src/application/commands/store_install.rssrc-tauri/src/application/commands/store_list.rssrc-tauri/src/application/queries/get_plugin_store.rssrc-tauri/src/application/read_models/plugin_store_view.rs
💤 Files with no reviewable changes (1)
- src-tauri/src/application/commands/store_list.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src-tauri/src/application/read_models/plugin_store_view.rs (1)
151-206: Good test expansion; add one regression for normalized-equal versions.Please add a case where installed and registry versions differ textually but have equal core semver (e.g.,
1.0.0+build.1vs1.0.0) and assertinstalled.✅ Example test to add
+ #[test] + fn test_enrich_with_installed_marks_installed_for_build_metadata_equal_core() { + let mut dto = PluginStoreEntryDto::from(make_entry(PluginStoreStatus::NotInstalled, None)); + dto.enrich_with_installed(Some("1.0.0+build.1".into())); + assert_eq!(dto.status, "installed"); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/application/read_models/plugin_store_view.rs` around lines 151 - 206, Add a regression test named something like test_enrich_with_installed_normalized_equal_versions that uses PluginStoreEntryDto::from(make_entry(PluginStoreStatus::NotInstalled, None)), calls dto.enrich_with_installed(Some("1.0.0+build.1".into())), and asserts dto.status == "installed" and dto.installed_version == Some("1.0.0+build.1".into()); this verifies enrich_with_installed treats semver-equal versions (differing only by build metadata) as installed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src-tauri/src/application/read_models/plugin_store_view.rs`:
- Around line 64-83: The current derive_status_str only treats exact string
equality as "installed" and misses cases where normalized semver cores match
(e.g., "1.0.0+build.1" vs "1.0.0"); update derive_status_str (and its helper
parse_semver) to consider parsed semver equality as installed: first try parsing
both installed and registry_version with parse_semver and if both parse and are
equal return "installed", otherwise fall back to the existing logic (None ->
"not_installed", parsed inst > reg -> "downgrade", else "update_available").
Ensure you reference parse_semver and derive_status_str when making the change
so normalization is used for equality checks rather than raw string equality.
---
Nitpick comments:
In `@src-tauri/src/application/read_models/plugin_store_view.rs`:
- Around line 151-206: Add a regression test named something like
test_enrich_with_installed_normalized_equal_versions that uses
PluginStoreEntryDto::from(make_entry(PluginStoreStatus::NotInstalled, None)),
calls dto.enrich_with_installed(Some("1.0.0+build.1".into())), and asserts
dto.status == "installed" and dto.installed_version ==
Some("1.0.0+build.1".into()); this verifies enrich_with_installed treats
semver-equal versions (differing only by build metadata) as installed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1a0fc426-b127-4fae-84d5-161ef82bc2f9
📒 Files selected for processing (3)
src-tauri/src/adapters/driven/plugin/github_store_client.rssrc-tauri/src/application/commands/store_install.rssrc-tauri/src/application/read_models/plugin_store_view.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src-tauri/src/application/commands/store_install.rs
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src-tauri/src/application/read_models/plugin_store_view.rs">
<violation number="1" location="src-tauri/src/application/read_models/plugin_store_view.rs:69">
P2: After stripping pre-release/build-metadata suffixes in `parse_semver`, two versions that normalize to the same `(major, minor, patch)` tuple (e.g. `1.0.0+build.1` vs `1.0.0`) will fall through the `inst > reg` guard and incorrectly return `"update_available"` instead of `"installed"`. Add an `inst == reg` arm before the downgrade check:
```rust
(Some(inst), Some(reg)) if inst == reg => "installed",
(Some(inst), Some(reg)) if inst > reg => "downgrade",
_ => "update_available",
```</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src-tauri/src/application/read_models/plugin_store_view.rs`:
- Around line 55-56: Update the doc comment that currently references the
removed handler name `handle_store_list` to instead reference the current query
path function `get_plugin_store` (the comment above the `current registry
version` field in `plugin_store_view.rs`), so maintainers see the correct symbol
for the live loader state flow; simply replace the `handle_store_list` mention
with `get_plugin_store` and ensure the comment remains grammatically correct.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8309d661-9573-44a6-89a5-c5a0de85dcbd
📒 Files selected for processing (1)
src-tauri/src/application/read_models/plugin_store_view.rs
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three distinct bugs in the plugin store install pipeline surfaced while trying to install the SoundCloud plugin. Each is fixed in a dedicated commit, plus a registry bump to v1.2.0 for the official SoundCloud plugin.
${name}.wasmwith kebab-case while Cargo publishes underscore-named cdylib artefacts. Every officially published plugin (YouTube, Vimeo, Gallery, SoundCloud) was 404-ing on first install.AlreadyExists, even when the UI said "not_installed" because the cache had not been refreshed in between.PluginLoader::list_loaded()at read time; the cache is treated as a pure snapshot of remote data.commands/store_list.rs, a dead duplicate ofqueries/get_plugin_store.rsnever referenced bycommands/mod.rs. Cargo silently ignores orphan.rsfiles so this was invisible at compile time.plugin.tomlstill claimingversion = "1.0.0", trapping users in a permanent "update_available" loop.Test plan
cargo test --lib)cargo clippy --lib -- -D warningscleanenrich_with_installedcovering installed / update_available / downgrade / uninstalled transitionsbuild_wasm_urlhyphen normalizationtauri devinstance via tauri-pilot IPC: install succeeds, second install no longer errors, UI reflects installed state without a store refreshSummary by CodeRabbit
Bug Fixes
Chores
Tests
Summary by cubic
Improves plugin store install reliability and live status, and updates the registry to
vortex-mod-soundcloudv1.2.0.Bug Fixes
NotFoundto avoid masking real loader errors.installed_versionandstatusfrom the live loader on read; cache stores only registry data so the UI updates without a refresh.commands/store_list.rs; consolidated inqueries/get_plugin_store.rs.Dependencies
vortex-mod-soundcloudto1.2.0with updated checksums. Fixes the v1.1.0 metadata mismatch that caused a persistent “update available” loop.Written for commit e694a6f. Summary will update on new commits.