diff --git a/app/src-tauri/src/marketplace/payload.rs b/app/src-tauri/src/marketplace/payload.rs index 879589e..52f2262 100644 --- a/app/src-tauri/src/marketplace/payload.rs +++ b/app/src-tauri/src/marketplace/payload.rs @@ -200,7 +200,7 @@ pub fn build_payload(input: &PayloadInput) -> Result { } }; entry["source"] = json!(format!("./{key}")); - let slug = marketplace_slug(&m.name, &m.id); + let slug = marketplace_slug(&m.id); for f in &files { tar.file( &format!("plugins/{slug}/{key}/{}", f.rel_path), @@ -328,7 +328,7 @@ mod tests { assert!(p.skipped.is_empty(), "{:?}", p.skipped); let files = unpack(&p.tar); - let slug = marketplace_slug("Team Tools", "m1aaaaaaaa"); + let slug = marketplace_slug("m1aaaaaaaa"); for path in [ "agents/code-reviewer.md".to_string(), "skills/example-skill/SKILL.md".to_string(), @@ -367,7 +367,7 @@ mod tests { data_root: data.path(), }) .unwrap(); - let slug = marketplace_slug("Team Tools", "m1aaaaaaaa"); + let slug = marketplace_slug("m1aaaaaaaa"); let files = unpack(&p.tar); let manifest: Value = serde_json::from_slice(&files["manifest.json"].data).unwrap(); @@ -398,6 +398,32 @@ mod tests { assert_eq!(catalog["plugins"][0]["source"], "./example-plugin"); } + /// Final review M4: the plugin marketplace name comes from the id, so a + /// rename never makes the container see a different marketplace. + #[test] + fn plugin_slug_survives_a_rename() { + let Some(fx) = GitFixture::new() else { return }; + let c = fx.with_all_kinds(); + let data = tempfile::tempdir().unwrap(); + cache(&fx, data.path()); + let installs = vec![inst(ItemKind::Plugin, "example-plugin", &c)]; + let slug_named = |name: &str| { + let marketplaces = vec![Marketplace { + name: name.into(), + ..market("m1aaaaaaaa") + }]; + let p = build_payload(&PayloadInput { + installs: &installs, + marketplaces: &marketplaces, + data_root: data.path(), + }) + .unwrap(); + p.manifest["items"][0]["slug"].as_str().unwrap().to_string() + }; + assert_eq!(slug_named("Team Tools"), "mp-m1aaaaaa"); + assert_eq!(slug_named("Renamed"), "mp-m1aaaaaa"); + } + #[test] fn items_that_cannot_be_built_are_skipped_not_fatal() { let Some(fx) = GitFixture::new() else { return }; diff --git a/app/src-tauri/src/marketplace/sync_script_tests.rs b/app/src-tauri/src/marketplace/sync_script_tests.rs index e6489dd..8a30fa6 100644 --- a/app/src-tauri/src/marketplace/sync_script_tests.rs +++ b/app/src-tauri/src/marketplace/sync_script_tests.rs @@ -1024,3 +1024,36 @@ fn a_deselected_legacy_plugin_record_is_still_uninstalled() { ); assert_eq!(read_json(&state_path)["items"], json!({})); } + +/// Final review M4: slugs moved from "-" to "mp-". The first +/// sync after that installs under the new name, uninstalls the old copy and +/// drops the old registration; later syncs are quiet. +#[test] +fn a_slug_change_reinstalls_under_the_new_name_and_retires_the_old() { + let Some(env) = env() else { return }; + shared_plugin_payload(&env, &[(SLUG, C1)]); + assert_eq!(run(&env).installed, vec!["plugin:p"]); + + fs::remove_file(&env.log).unwrap(); + shared_plugin_payload(&env, &[(SLUG_A, C1)]); + let r = run(&env); + assert_eq!(r.installed, vec!["plugin:p"], "{r:?}"); + assert_eq!(r.removed, vec!["plugin:p"], "{r:?}"); + assert!(r.errors.is_empty(), "{r:?}"); + let tree = env.home.join(".claude/triple-c/plugins"); + assert_eq!( + claude_log(&env), + vec![ + format!("plugin marketplace add {}", tree.join(SLUG_A).display()), + format!("plugin install p@triple-c-{SLUG_A}"), + format!("plugin uninstall p@triple-c-{SLUG}"), + format!("plugin marketplace remove triple-c-{SLUG}"), + ] + ); + assert!(!tree.join(SLUG).exists()); + + fs::remove_file(&env.log).unwrap(); + shared_plugin_payload(&env, &[(SLUG_A, C1)]); + let r = run(&env); + assert!(nothing_reported(&r), "{r:?}"); +} diff --git a/app/src-tauri/src/models/marketplace.rs b/app/src-tauri/src/models/marketplace.rs index d409983..4884624 100644 --- a/app/src-tauri/src/models/marketplace.rs +++ b/app/src-tauri/src/models/marketplace.rs @@ -124,34 +124,25 @@ pub fn is_valid_item_key(key: &str) -> bool { .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'.' | b'_' | b'-')) } -/// A container-safe, collision-free name for a marketplace: its name -/// lowercased to `[a-z0-9-]`, dashes collapsed, at most 32 characters, then -/// `-` and the first 8 characters of its id. An empty sanitised name becomes -/// `marketplace`. -pub fn marketplace_slug(name: &str, id: &str) -> String { - let mut base = String::new(); - for c in name.chars() { - let c = c.to_ascii_lowercase(); - if c.is_ascii_lowercase() || c.is_ascii_digit() { - base.push(c); - } else if !base.ends_with('-') && !base.is_empty() { - base.push('-'); - } - } - let mut base: String = base.trim_matches('-').chars().take(32).collect(); - while base.ends_with('-') { - base.pop(); - } - if base.is_empty() { - base.push_str("marketplace"); - } +/// A container-safe name for a marketplace (plugin marketplace +/// `triple-c-`, plugin tree `plugins//`): `mp-` and the first 8 +/// alphanumeric characters of its id, lowercased. It depends on the id only, +/// never on the editable display name, so a rename cannot make a container +/// see a different marketplace (final review M4). Containers synced with +/// the earlier `-` slugs move over on their next sync: the +/// plugins are installed under the new name and the old copies uninstalled. +pub fn marketplace_slug(id: &str) -> String { let id_part: String = id .chars() .filter(|c| c.is_ascii_alphanumeric()) .map(|c| c.to_ascii_lowercase()) .take(8) .collect(); - format!("{}-{}", base, id_part) + if id_part.is_empty() { + "mp-marketplace".to_string() + } else { + format!("mp-{id_part}") + } } /// A full, lowercase, 40-character hex object id. @@ -327,17 +318,14 @@ mod tests { } #[test] - fn slug_is_sanitised_and_suffixed_with_the_id() { + fn slug_is_derived_from_the_id_only() { assert_eq!( - marketplace_slug("Triple-C Marketplace!", "1A2B3C4D-ffff"), - "triple-c-marketplace-1a2b3c4d" + marketplace_slug("7C9E6679-7425-40de-944b-e07fc1f90ae7"), + "mp-7c9e6679" ); - assert_eq!( - marketplace_slug("***", "abcdef0123"), - "marketplace-abcdef01" - ); - let long = marketplace_slug(&"x".repeat(80), "12345678"); - assert_eq!(long, format!("{}-12345678", "x".repeat(32))); + assert_eq!(marketplace_slug("1A2B-3C4D-ffff"), "mp-1a2b3c4d"); + assert_eq!(marketplace_slug("ab"), "mp-ab"); + assert_eq!(marketplace_slug("--"), "mp-marketplace"); } #[test] diff --git a/docs/superpowers/specs/2026-09-27-marketplace-design.md b/docs/superpowers/specs/2026-09-27-marketplace-design.md index d06684f..2ffe42b 100644 --- a/docs/superpowers/specs/2026-09-27-marketplace-design.md +++ b/docs/superpowers/specs/2026-09-27-marketplace-design.md @@ -120,8 +120,8 @@ pub struct MarketplaceAccount { pub username: Option, // resolved at sign-in, display only } pub struct Marketplace { - pub id: String, // uuid - pub name: String, // display; slug used in container paths + pub id: String, // uuid; slug "mp-" used in container paths + pub name: String, // display only (renaming never changes the slug) pub url: String, // https URL only pub branch: Option,// None = remote default branch pub account_id: Option, // None = anonymous