Marketplace: derive the plugin slug from the id only (final review M4)
The slug (plugin marketplace "triple-c-<slug>", plugin tree
"plugins/<slug>/") was built from the editable display name. After a
rename the next sync registered the new marketplace, skipped the plugin
install because the state's commit matched, then removed the old
marketplace: the plugin was gone while the report said nothing changed.
marketplace_slug now takes the id only ("mp-<id8>"). With plugin state
kept per slug (I1), containers synced with the old "<name>-<id8>" slugs
move over on their next sync: plugins are installed under the new name,
the old copies uninstalled and the old registration dropped. A sync
script test covers that migration (it fails on the pre-I1 script).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -200,7 +200,7 @@ pub fn build_payload(input: &PayloadInput) -> Result<Payload, String> {
|
||||
}
|
||||
};
|
||||
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 };
|
||||
|
||||
@@ -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 "<name>-<id8>" to "mp-<id8>". 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:?}");
|
||||
}
|
||||
|
||||
@@ -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-<slug>`, plugin tree `plugins/<slug>/`): `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 `<name>-<id8>` 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]
|
||||
|
||||
@@ -120,8 +120,8 @@ pub struct MarketplaceAccount {
|
||||
pub username: Option<String>, // 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-<id8>" used in container paths
|
||||
pub name: String, // display only (renaming never changes the slug)
|
||||
pub url: String, // https URL only
|
||||
pub branch: Option<String>,// None = remote default branch
|
||||
pub account_id: Option<String>, // None = anonymous
|
||||
|
||||
Reference in New Issue
Block a user