From 6cf9664dc82ed0e9148254d8f3749a8eea2e35a4 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 27 Sep 2026 09:46:56 -0700 Subject: [PATCH] Marketplace: warn on imported global plugins; tidy import follow-ups - The import preview counts global plugin installs and warns on them: a plugin can bring hooks and MCP servers into every container and an imported install skips the confirm step, like a hook. - Item keys, hosts and branches in errors are quoted with {:?} and capped, since they can come from an import file. - After an import, caches and snapshots of marketplaces the import dropped are removed (under the repo lock) and pins are refreshed for the imported installs. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/marketplace_commands.rs | 71 +++++++++++++++++-- .../src/commands/settings_export_commands.rs | 7 ++ app/src-tauri/src/models/settings_export.rs | 31 ++++++++ .../settings/ImportSettingsModal.test.tsx | 1 + app/src/lib/settingsImportPreview.test.ts | 10 +++ app/src/lib/settingsImportPreview.ts | 9 ++- app/src/lib/types.ts | 3 + 7 files changed, 126 insertions(+), 6 deletions(-) diff --git a/app/src-tauri/src/commands/marketplace_commands.rs b/app/src-tauri/src/commands/marketplace_commands.rs index 6874234..873dc65 100644 --- a/app/src-tauri/src/commands/marketplace_commands.rs +++ b/app/src-tauri/src/commands/marketplace_commands.rs @@ -62,6 +62,19 @@ pub(crate) mod ops { } } + /// An unvalidated value as it may appear in an error: quoted and escaped + /// (`{:?}`) and capped at 60 characters, since it can come from an + /// import file rather than from what the person just typed. + pub fn shown(value: &str) -> String { + const MAX: usize = 60; + if value.chars().count() > MAX { + let head: String = value.chars().take(MAX).collect(); + format!("{:?}…", head) + } else { + format!("{:?}", value) + } + } + pub fn validate_label(label: &str) -> Result { let label = label.trim(); if label.is_empty() { @@ -86,7 +99,7 @@ pub(crate) mod ops { if git::valid_branch(&b) { Ok(Some(b)) } else { - Err(format!("{b:?} is not a valid branch name.")) + Err(format!("{} is not a valid branch name.", shown(&b))) } } @@ -101,7 +114,7 @@ pub(crate) mod ops { if auth::valid_host(&host) && !host.starts_with('.') && !host.starts_with(':') && port_ok { Ok(host) } else { - Err(format!("{host:?} is not a valid host name.")) + Err(format!("{} is not a valid host name.", shown(&host))) } } @@ -294,7 +307,10 @@ fn validate_item(item: &MarketplaceItemRef) -> Result<(), String> { if is_valid_item_key(&item.key) { Ok(()) } else { - Err(format!("\"{}\" is not a valid item name.", item.key)) + Err(format!( + "{} is not a valid item name.", + ops::shown(&item.key) + )) } } @@ -382,6 +398,17 @@ pub(crate) fn validate_imported_marketplace_state( Ok(()) } +/// Marketplaces configured in `before` that `after` no longer has: an import +/// that drops them leaves their caches and snapshots to be removed. +pub(crate) fn dropped_marketplace_ids(before: &AppSettings, after: &AppSettings) -> Vec { + before + .marketplaces + .iter() + .filter(|m| !after.marketplaces.iter().any(|a| a.id == m.id)) + .map(|m| m.id.clone()) + .collect() +} + /// The in-memory snapshot, else the cached one (which is then remembered). fn snapshot_or_cached(mgr: &MarketplaceManager, m: &Marketplace) -> MarketplaceSnapshot { if let Some(s) = mgr.snapshot(&m.id) { @@ -406,7 +433,7 @@ async fn snapshot_blocking( /// Make each cache's pin refs exactly the commits installs reference, so a /// pinned version can never be garbage-collected away. Under the repo lock /// (pre-flight F11): a concurrent fetch writes refs in the same repos. -async fn refresh_pins(state: &AppState) { +pub(crate) async fn refresh_pins(state: &AppState) { let settings = state.settings_store.get(); let pins = mk::pins_by_marketplace(&settings, &state.projects_store.list()); let root = state.marketplace.data_root().to_path_buf(); @@ -432,7 +459,7 @@ async fn refresh_pins(state: &AppState) { } /// Forget a marketplace's snapshot and delete its cache, under the repo lock. -async fn remove_cache(state: &AppState, marketplace_id: &str) { +pub(crate) async fn remove_cache(state: &AppState, marketplace_id: &str) { state.marketplace.remove_snapshot(marketplace_id); let path = git::cache_path(state.marketplace.data_root(), marketplace_id); let _repo_guard = state.marketplace.repo_lock().lock().await; @@ -1144,4 +1171,38 @@ mod tests { s.marketplaces.clear(); validate_imported_marketplace_state(&mut s, &tokens()).unwrap(); } + + #[test] + fn an_invalid_item_key_is_quoted_and_capped_in_the_error() { + use crate::models::marketplace::MarketplaceItemRef; + let item = |key: String| MarketplaceItemRef { + marketplace_id: MARKET.into(), + kind: ItemKind::Agent, + key, + }; + let e = validate_item(&item("bad\nkey".into())).unwrap_err(); + assert!(!e.contains('\n'), "raw control character in {e:?}"); + assert!(e.contains("\"bad\\nkey\""), "{e}"); + let e = validate_item(&item(format!("{}/", "x".repeat(500)))).unwrap_err(); + assert!( + e.chars().count() < 150, + "not capped: {} chars", + e.chars().count() + ); + } + + #[test] + fn marketplaces_an_import_drops_are_the_ones_whose_caches_go() { + let mut before = imported(); + let mut kept = before.marketplaces[0].clone(); + kept.id = "kept-1".into(); + before.marketplaces.push(kept.clone()); + let mut after = AppSettings::default(); + after.marketplaces.push(kept); + assert_eq!( + dropped_marketplace_ids(&before, &after), + vec![MARKET.to_string()] + ); + assert!(dropped_marketplace_ids(&after, &before).is_empty()); + } } diff --git a/app/src-tauri/src/commands/settings_export_commands.rs b/app/src-tauri/src/commands/settings_export_commands.rs index 6480b8d..b219389 100644 --- a/app/src-tauri/src/commands/settings_export_commands.rs +++ b/app/src-tauri/src/commands/settings_export_commands.rs @@ -441,6 +441,13 @@ pub async fn apply_settings_import( ) = imported_marketplace; state.settings_store.update(s)? }; + // Caches of marketplaces the import dropped are dead weight now, and + // the pins must match the imported installs. + use crate::commands::marketplace_commands as mc; + for id in mc::dropped_marketplace_ids(¤t, &saved) { + mc::remove_cache(&state, &id).await; + } + mc::refresh_pins(&state).await; // `reconcile_gateway` (inside `update_settings`) only reacts to a changed // *shape* — port, provider, base URL, models — because that's what's diff --git a/app/src-tauri/src/models/settings_export.rs b/app/src-tauri/src/models/settings_export.rs index 3431175..85396cd 100644 --- a/app/src-tauri/src/models/settings_export.rs +++ b/app/src-tauri/src/models/settings_export.rs @@ -164,6 +164,10 @@ pub struct SettingsImportPreview { /// step an install from the Marketplace tab shows, so the preview warns. #[serde(default)] pub global_hook_install_count: usize, + /// Plugins the import installs for every project. A plugin can bring + /// its own hooks and MCP servers, and skips the same confirm step. + #[serde(default)] + pub global_plugin_install_count: usize, /// Non-blank marketplace account tokens the import restores. #[serde(default)] pub marketplace_account_token_count: usize, @@ -224,6 +228,12 @@ impl SettingsImportPreview { .iter() .filter(|i| i.kind == crate::models::marketplace::ItemKind::Hook) .count(), + global_plugin_install_count: payload + .settings + .global_marketplace_installs + .iter() + .filter(|i| i.kind == crate::models::marketplace::ItemKind::Plugin) + .count(), marketplace_account_token_count: payload .secrets .marketplace_account_tokens @@ -446,4 +456,25 @@ mod tests { }; assert!(!secrets.is_empty()); } + + #[test] + fn global_plugin_installs_are_counted_apart_from_hooks() { + use crate::models::marketplace::{ItemKind, MarketplaceInstall}; + let mut payload = payload_with(ExportedSecrets::default()); + let install = |kind, key: &str| MarketplaceInstall { + marketplace_id: "m1".into(), + kind, + key: key.into(), + commit: "a".repeat(40), + }; + payload.settings.global_marketplace_installs = vec![ + install(ItemKind::Plugin, "p1"), + install(ItemKind::Hook, "h1"), + install(ItemKind::Plugin, "p2"), + install(ItemKind::Skill, "s1"), + ]; + let preview = SettingsImportPreview::from_payload(&payload); + assert_eq!(preview.global_plugin_install_count, 2); + assert_eq!(preview.global_hook_install_count, 1); + } } diff --git a/app/src/components/settings/ImportSettingsModal.test.tsx b/app/src/components/settings/ImportSettingsModal.test.tsx index f05a206..34e8753 100644 --- a/app/src/components/settings/ImportSettingsModal.test.tsx +++ b/app/src/components/settings/ImportSettingsModal.test.tsx @@ -34,6 +34,7 @@ const samplePreview: SettingsImportPreview = { custom_image_name: null, marketplace_count: 0, global_hook_install_count: 0, + global_plugin_install_count: 0, marketplace_account_token_count: 0, }; diff --git a/app/src/lib/settingsImportPreview.test.ts b/app/src/lib/settingsImportPreview.test.ts index b253221..01653cf 100644 --- a/app/src/lib/settingsImportPreview.test.ts +++ b/app/src/lib/settingsImportPreview.test.ts @@ -22,6 +22,7 @@ function preview(overrides: Partial = {}): SettingsImport custom_image_name: null, marketplace_count: 0, global_hook_install_count: 0, + global_plugin_install_count: 0, marketplace_account_token_count: 0, ...overrides, }; @@ -135,6 +136,15 @@ describe("describeImportWarnings", () => { ); }); + it("warns when the import installs plugins for every project", () => { + expect(describeImportWarnings(preview({ global_plugin_install_count: 1 }))).toEqual([ + "Installs 1 marketplace plugin for all projects. Plugins can bring their own hooks and MCP servers into every project container, and these skip the confirmation an install from the Marketplace tab asks for.", + ]); + expect( + describeImportWarnings(preview({ global_plugin_install_count: 2, global_hook_install_count: 1 })), + ).toHaveLength(2); + }); + it("warns about a custom Docker image every time, not only when it changes", () => { expect( describeImportWarnings(preview({ image_source: "custom", custom_image_name: "evil:latest" })), diff --git a/app/src/lib/settingsImportPreview.ts b/app/src/lib/settingsImportPreview.ts index 6e649ef..83a5e74 100644 --- a/app/src/lib/settingsImportPreview.ts +++ b/app/src/lib/settingsImportPreview.ts @@ -54,7 +54,8 @@ export function describeImport(preview: SettingsImportPreview): string[] { * * Global marketplace hooks get one too: a hook runs commands in every * project container, and an imported install never passed the hook-confirm - * step an install from the Marketplace tab shows. + * step an install from the Marketplace tab shows. Global plugins likewise: + * a plugin can carry its own hooks and MCP servers. * * A custom Docker image gets a warning every time, not just on change: it's * the image every project container is created from, so it's worth calling @@ -75,6 +76,12 @@ export function describeImportWarnings(preview: SettingsImportPreview): string[] `Installs ${n} marketplace hook${n === 1 ? "" : "s"} for all projects. Hooks run commands in every project container, and these skip the confirmation an install from the Marketplace tab asks for.`, ); } + if (preview.global_plugin_install_count > 0) { + const n = preview.global_plugin_install_count; + warnings.push( + `Installs ${n} marketplace plugin${n === 1 ? "" : "s"} for all projects. Plugins can bring their own hooks and MCP servers into every project container, and these skip the confirmation an install from the Marketplace tab asks for.`, + ); + } if (preview.image_source === "custom") { warnings.push( `Runs every project container from a custom Docker image: ${preview.custom_image_name ?? "(no image name set)"}.`, diff --git a/app/src/lib/types.ts b/app/src/lib/types.ts index 4b89a4a..089ba65 100644 --- a/app/src/lib/types.ts +++ b/app/src/lib/types.ts @@ -410,6 +410,9 @@ export interface SettingsImportPreview { * every project container, without the confirm step a Marketplace-tab * install shows, so the preview warns about them. */ global_hook_install_count: number; + /** Plugins the import installs for all projects — a plugin can bring its + * own hooks and MCP servers, and skips the same confirm step. */ + global_plugin_install_count: number; /** Marketplace account tokens the import restores to the keychain. */ marketplace_account_token_count: number; }