Marketplace sync: track plugin state per marketplace (final review I1)
Two marketplaces shipping a plugin of the same name shared one "plugin:<key>" state record, so every sync reinstalled one copy and reported it updated, and removing one marketplace never uninstalled its copy. Plugin state ids are now "plugin:<slug>/<key>"; the slug and key for an uninstall are derived from the id and re-validated. Older "plugin:<key>" records are migrated using their recorded slug, so existing installs are neither reinstalled nor orphaned. Reports keep "plugin:<key>". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -884,3 +884,143 @@ fn sync_drops_state_records_without_a_kind_key_id() {
|
||||
let state = read_json(&state_path);
|
||||
assert_eq!(state["items"], json!({}), "bogus records dropped");
|
||||
}
|
||||
|
||||
// ── Plugin state per marketplace (final review I1) ───────────────────────────
|
||||
|
||||
const SLUG_A: &str = "mp-aaaaaaaa";
|
||||
const SLUG_B: &str = "mp-bbbbbbbb";
|
||||
|
||||
/// A payload in which each marketplace ships plugin `p` at its own commit.
|
||||
fn shared_plugin_payload(env: &Env, slugs: &[(&str, &str)]) {
|
||||
let mut files: Vec<(String, String)> = Vec::new();
|
||||
let mut items = Vec::new();
|
||||
let mut groups = Vec::new();
|
||||
for (slug, commit) in slugs {
|
||||
files.push((
|
||||
format!("plugins/{slug}/.claude-plugin/marketplace.json"),
|
||||
format!(
|
||||
r#"{{"name":"triple-c-{slug}","owner":{{"name":"Triple-C"}},"plugins":[{{"name":"p","source":"./p"}}]}}"#
|
||||
),
|
||||
));
|
||||
files.push((
|
||||
format!("plugins/{slug}/p/.claude-plugin/plugin.json"),
|
||||
r#"{"name":"p"}"#.to_string(),
|
||||
));
|
||||
items.push(json!({ "kind": "plugin", "key": "p", "marketplace": slug, "commit": commit, "slug": slug }));
|
||||
groups.push(json!({ "slug": slug, "dir": format!("plugins/{slug}"), "plugins": ["p"] }));
|
||||
}
|
||||
let refs: Vec<(&str, &str, bool)> = files
|
||||
.iter()
|
||||
.map(|(p, t)| (p.as_str(), t.as_str(), false))
|
||||
.collect();
|
||||
payload(
|
||||
env,
|
||||
&refs,
|
||||
json!({ "version": 1, "items": items, "plugin_marketplaces": groups }),
|
||||
);
|
||||
}
|
||||
|
||||
fn nothing_reported(r: &SyncReport) -> bool {
|
||||
r.installed.is_empty() && r.updated.is_empty() && r.removed.is_empty() && r.errors.is_empty()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn two_marketplaces_sharing_a_plugin_name_reach_a_steady_state() {
|
||||
let Some(env) = env() else { return };
|
||||
|
||||
shared_plugin_payload(&env, &[(SLUG_A, C1), (SLUG_B, C2)]);
|
||||
let r = run(&env);
|
||||
assert_eq!(r.installed, vec!["plugin:p", "plugin:p"], "{r:?}");
|
||||
assert!(r.errors.is_empty(), "{r:?}");
|
||||
|
||||
// Nothing changed: no reinstall, nothing reported.
|
||||
for _ in 0..2 {
|
||||
fs::remove_file(&env.log).unwrap();
|
||||
shared_plugin_payload(&env, &[(SLUG_A, C1), (SLUG_B, C2)]);
|
||||
let r = run(&env);
|
||||
assert!(nothing_reported(&r), "{r:?}");
|
||||
assert_eq!(
|
||||
claude_log(&env),
|
||||
vec![
|
||||
format!("plugin marketplace update triple-c-{SLUG_A}"),
|
||||
format!("plugin marketplace update triple-c-{SLUG_B}"),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
// One marketplace's copy is removed: only that copy is uninstalled.
|
||||
fs::remove_file(&env.log).unwrap();
|
||||
shared_plugin_payload(&env, &[(SLUG_A, C1)]);
|
||||
let r = run(&env);
|
||||
assert_eq!(r.removed, vec!["plugin:p"], "{r:?}");
|
||||
assert!(r.installed.is_empty() && r.updated.is_empty(), "{r:?}");
|
||||
assert_eq!(
|
||||
claude_log(&env),
|
||||
vec![
|
||||
format!("plugin marketplace update triple-c-{SLUG_A}"),
|
||||
format!("plugin uninstall p@triple-c-{SLUG_B}"),
|
||||
format!("plugin marketplace remove triple-c-{SLUG_B}"),
|
||||
]
|
||||
);
|
||||
|
||||
// And the survivor stays put.
|
||||
fs::remove_file(&env.log).unwrap();
|
||||
shared_plugin_payload(&env, &[(SLUG_A, C1)]);
|
||||
let r = run(&env);
|
||||
assert!(nothing_reported(&r), "{r:?}");
|
||||
assert_eq!(
|
||||
claude_log(&env),
|
||||
vec![format!("plugin marketplace update triple-c-{SLUG_A}")]
|
||||
);
|
||||
}
|
||||
|
||||
fn write_state(env: &Env, state: Value) -> PathBuf {
|
||||
let p = env.home.join(".claude/triple-c/marketplace/state.json");
|
||||
fs::create_dir_all(p.parent().unwrap()).unwrap();
|
||||
fs::write(&p, state.to_string()).unwrap();
|
||||
p
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_plugin_records_are_migrated_not_reinstalled() {
|
||||
let Some(env) = env() else { return };
|
||||
// State as written by an earlier sync.sh: plugins keyed "plugin:<key>".
|
||||
let state_path = write_state(
|
||||
&env,
|
||||
json!({ "version": 1, "plugin_marketplaces": [SLUG_A],
|
||||
"items": { "plugin:p": { "commit": C1, "slug": SLUG_A } } }),
|
||||
);
|
||||
|
||||
shared_plugin_payload(&env, &[(SLUG_A, C1)]);
|
||||
let r = run(&env);
|
||||
assert!(nothing_reported(&r), "{r:?}");
|
||||
assert_eq!(
|
||||
claude_log(&env),
|
||||
vec![format!("plugin marketplace update triple-c-{SLUG_A}")]
|
||||
);
|
||||
let items = read_json(&state_path)["items"].clone();
|
||||
assert!(items.get("plugin:p").is_none(), "{items}");
|
||||
assert_eq!(items[format!("plugin:{SLUG_A}/p")]["commit"], C1, "{items}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_deselected_legacy_plugin_record_is_still_uninstalled() {
|
||||
let Some(env) = env() else { return };
|
||||
let state_path = write_state(
|
||||
&env,
|
||||
json!({ "version": 1, "plugin_marketplaces": [SLUG_A],
|
||||
"items": { "plugin:p": { "commit": C1, "slug": SLUG_A } } }),
|
||||
);
|
||||
|
||||
payload(&env, &[], empty_manifest());
|
||||
let r = run(&env);
|
||||
assert_eq!(r.removed, vec!["plugin:p"], "{r:?}");
|
||||
assert_eq!(
|
||||
claude_log(&env),
|
||||
vec![
|
||||
format!("plugin uninstall p@triple-c-{SLUG_A}"),
|
||||
format!("plugin marketplace remove triple-c-{SLUG_A}"),
|
||||
]
|
||||
);
|
||||
assert_eq!(read_json(&state_path)["items"], json!({}));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user