CyberCoveLLC/Triple-C
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases 20 Wiki Activity

Marketplace: edit only the marketplace fields of a project (PR review #2)

Browse Source 
ProjectsStore gains update_marketplace_fields / update_all_marketplace_fields,
which read-modify-write installs and opt-outs under the store's own lock.
Install, uninstall, update, forget and set_global_item_disabled use them
instead of writing back a whole Project read earlier, so a concurrent
start's status/container_id change is no longer overwritten.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
shadowdao
2026-09-27 13:04:51 -07:00
co-authored by Claude Opus 5.5
parent e805c29c70
commit d51b54774b
2 changed files with 181 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/src-tauri/src/commands/marketplace_commands.rs
+42 -30
View File
@@ -761,16 +761,12 @@ pub async fn forget_marketplace_installs(
.global_marketplace_installs
.retain(|i| i.marketplace_id != marketplace_id);
state.settings_store.update(settings)?;
for mut p in state.projects_store.list() {
let before = (p.marketplace_installs.len(), p.marketplace_disabled.len());
p.marketplace_installs
.retain(|i| i.marketplace_id != marketplace_id);
p.marketplace_disabled
.retain(|r| r.marketplace_id != marketplace_id);
if (p.marketplace_installs.len(), p.marketplace_disabled.len()) != before {
state.projects_store.update(p)?;
}
}
state
.projects_store
.update_all_marketplace_fields(|installs, disabled| {
installs.retain(|i| i.marketplace_id != marketplace_id);
disabled.retain(|r| r.marketplace_id != marketplace_id);
})?;
refresh_pins(&state).await;
Ok(())
}
@@ -807,9 +803,12 @@ pub async fn install_marketplace_item(
state.settings_store.update(s)?;
}
InstallScope::Project { project_id } => {
let mut p = find_project(&state, &project_id)?;
ops::upsert_install(&mut p.marketplace_installs, inst);
state.projects_store.update(p)?;
state
.projects_store
.update_marketplace_fields(&project_id, |installs, _| {
ops::upsert_install(installs, inst);
Ok(())
})?;
}
}
refresh_pins(&state).await;
@@ -830,19 +829,23 @@ pub async fn uninstall_marketplace_item(
}
state.settings_store.update(s)?;
// An opt-out of an item that is no longer global means nothing.
for mut p in state.projects_store.list() {
if p.marketplace_disabled.contains(&item) {
ops::set_disabled(&mut p.marketplace_disabled, &item, false);
state.projects_store.update(p)?;
}
}
state
.projects_store
.update_all_marketplace_fields(|_, disabled| {
ops::set_disabled(disabled, &item, false)
})?;
}
InstallScope::Project { project_id } => {
let mut p = find_project(&state, &project_id)?;
if !ops::remove_install(&mut p.marketplace_installs, &item) {
return Err(format!("That item is not installed in \"{}\".", p.name));
let name = find_project(&state, &project_id)?.name;
state
.projects_store
.update_marketplace_fields(&project_id, |installs, _| {
if ops::remove_install(installs, &item) {
Ok(())
} else {
Err(format!("That item is not installed in \"{name}\"."))
}
state.projects_store.update(p)?;
})?;
}
}
refresh_pins(&state).await;
@@ -857,9 +860,13 @@ pub async fn set_global_item_disabled(
state: State<'_, AppState>,
) -> Result<Project, String> {
validate_item(&item)?;
let mut p = find_project(&state, &project_id)?;
ops::set_disabled(&mut p.marketplace_disabled, &item, disabled);
state.projects_store.update(p)
let ((), saved) = state
.projects_store
.update_marketplace_fields(&project_id, |_, list| {
ops::set_disabled(list, &item, disabled);
Ok(())
})?;
Ok(saved)
}
// ─────────────────────────────────────────────────────────────────────────────
@@ -924,11 +931,16 @@ pub async fn update_marketplace_item(
state.settings_store.update(s)?;
}
InstallScope::Project { project_id } => {
let mut p = find_project(&state, &project_id)?;
if !ops::repin(&mut p.marketplace_installs, &item, &head) {
return Err(format!("That item is not installed in \"{}\".", p.name));
let name = find_project(&state, &project_id)?.name;
state
.projects_store
.update_marketplace_fields(&project_id, |installs, _| {
if ops::repin(installs, &item, &head) {
Ok(())
} else {
Err(format!("That item is not installed in \"{name}\"."))
}
state.projects_store.update(p)?;
})?;
}
}
refresh_pins(&state).await;
app/src-tauri/src/storage/projects_store.rs
+137
View File
@@ -2,6 +2,7 @@ use std::fs;
use std::path::{Path, PathBuf};
use std::sync::Mutex;
use crate::models::marketplace::{MarketplaceInstall, MarketplaceItemRef};
use crate::models::Project;
/// The sticky marker for `projects.json`: `projects.json.corrupt`, beside it.
@@ -256,6 +257,60 @@ impl ProjectsStore {
}
}
/// Read-modify-write of one project's marketplace installs and opt-outs
/// under the store's lock, touching nothing else (PR review #2): the
/// marketplace commands must not write back a whole record read before a
/// start changed its status or container id. When `f` fails nothing is
/// saved. Returns `f`'s value and the saved project.
pub fn update_marketplace_fields<T>(
&self,
project_id: &str,
f: impl FnOnce(
&mut Vec<MarketplaceInstall>,
&mut Vec<MarketplaceItemRef>,
) -> Result<T, String>,
) -> Result<(T, Project), String> {
let mut projects = self.lock();
let p = projects
.iter_mut()
.find(|p| p.id == project_id)
.ok_or_else(|| format!("Project {} not found", project_id))?;
let mut installs = p.marketplace_installs.clone();
let mut disabled = p.marketplace_disabled.clone();
let out = f(&mut installs, &mut disabled)?;
p.marketplace_installs = installs;
p.marketplace_disabled = disabled;
p.updated_at = chrono::Utc::now().to_rfc3339();
let saved = p.clone();
self.save(&projects)?;
Ok((out, saved))
}
/// [`Self::update_marketplace_fields`] over every project at once, in one
/// save. Projects `f` leaves as they were are not touched at all.
pub fn update_all_marketplace_fields(
&self,
mut f: impl FnMut(&mut Vec<MarketplaceInstall>, &mut Vec<MarketplaceItemRef>),
) -> Result<(), String> {
let mut projects = self.lock();
let mut changed = false;
for p in projects.iter_mut() {
let mut installs = p.marketplace_installs.clone();
let mut disabled = p.marketplace_disabled.clone();
f(&mut installs, &mut disabled);
if installs != p.marketplace_installs || disabled != p.marketplace_disabled {
p.marketplace_installs = installs;
p.marketplace_disabled = disabled;
p.updated_at = chrono::Utc::now().to_rfc3339();
changed = true;
}
}
if changed {
self.save(&projects)?;
}
Ok(())
}
pub fn set_container_id(&self, project_id: &str, container_id: Option<String>) -> Result<(), String> {
let mut projects = self.lock();
if let Some(p) = projects.iter_mut().find(|p| p.id == project_id) {
@@ -410,4 +465,86 @@ mod tests {
fs::remove_dir_all(&dir).ok();
}
fn market_install(key: &str) -> crate::models::marketplace::MarketplaceInstall {
crate::models::marketplace::MarketplaceInstall {
marketplace_id: "m1".into(),
kind: crate::models::marketplace::ItemKind::Agent,
key: key.into(),
commit: "a".repeat(40),
}
}
#[test]
fn marketplace_edits_keep_a_concurrent_status_and_container_change() {
// PR review #2: a marketplace install/uninstall used to write back a
// whole record read before a start flipped status and container_id,
// leaving the project stuck at Starting with no container.
let dir = temp_dir("marketplace-fields");
let project = Project::new("demo".to_string(), Vec::new());
let id = project.id.clone();
let store = store_over(&dir, vec![project]);
// The start flow moves on while a marketplace command is running.
store.set_container_id(&id, Some("cid-1".into())).unwrap();
store.update_status(&id, crate::models::ProjectStatus::Starting).unwrap();
let (added, saved) = store
.update_marketplace_fields(&id, |installs, disabled| {
installs.push(market_install("a"));
disabled.push(market_install("g").item_ref());
Ok(installs.len())
})
.unwrap();
assert_eq!(added, 1);
assert_eq!(saved.container_id.as_deref(), Some("cid-1"));
assert_eq!(saved.status, crate::models::ProjectStatus::Starting);
let on_disk: Vec<Project> =
serde_json::from_str(&fs::read_to_string(dir.join("projects.json")).unwrap()).unwrap();
assert_eq!(on_disk[0].container_id.as_deref(), Some("cid-1"));
assert_eq!(on_disk[0].marketplace_installs, vec![market_install("a")]);
// A refusal inside the closure writes nothing.
let before = fs::read_to_string(dir.join("projects.json")).unwrap();
let err = store
.update_marketplace_fields(&id, |installs, _| {
installs.clear();
Err::<(), _>("not installed".to_string())
})
.unwrap_err();
assert_eq!(err, "not installed");
assert_eq!(store.get(&id).unwrap().marketplace_installs.len(), 1);
assert_eq!(fs::read_to_string(dir.join("projects.json")).unwrap(), before);
assert!(store.update_marketplace_fields("nope", |_, _| Ok(())).is_err());
fs::remove_dir_all(&dir).ok();
}
#[test]
fn marketplace_edits_across_all_projects_touch_only_those_fields() {
let dir = temp_dir("marketplace-all");
let mut a = Project::new("a".to_string(), Vec::new());
a.marketplace_installs = vec![market_install("x")];
let b = Project::new("b".to_string(), Vec::new());
let (a_id, b_id) = (a.id.clone(), b.id.clone());
let store = store_over(&dir, vec![a, b]);
store.set_container_id(&b_id, Some("cid-b".into())).unwrap();
store.update_status(&a_id, crate::models::ProjectStatus::Running).unwrap();
let b_updated_at = store.get(&b_id).unwrap().updated_at;
store
.update_all_marketplace_fields(|installs, _| {
installs.retain(|i| i.marketplace_id != "m1")
})
.unwrap();
let a = store.get(&a_id).unwrap();
assert!(a.marketplace_installs.is_empty());
assert_eq!(a.status, crate::models::ProjectStatus::Running);
let b = store.get(&b_id).unwrap();
assert_eq!(b.container_id.as_deref(), Some("cid-b"));
assert_eq!(b.updated_at, b_updated_at, "an untouched project is not rewritten");
fs::remove_dir_all(&dir).ok();
}
}