Marketplace: pin the commit the user reviewed (final review I2)
Install and update pinned whatever the marketplace head was when the click landed, so a background refresh between review and click could pin content nobody saw (including a hook's shell commands). install_marketplace_item and update_marketplace_item now take expected_commit and refuse with "changed since you reviewed this item — review it again" unless it is still the head. The UI passes the head the selected item was read at (Browse), the head frozen with a pending hook confirm (whose commands are frozen too), and the head of the accepted diff (Installed). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -62,6 +62,26 @@ pub(crate) mod ops {
|
||||
}
|
||||
}
|
||||
|
||||
/// The commit to pin for an install or update: the marketplace's current
|
||||
/// head, but only if it is the one the person reviewed (`expected`, the
|
||||
/// head the UI showed or diffed against). A refresh that lands between
|
||||
/// review and click must not pin content nobody saw (final review I2).
|
||||
pub fn reviewed_head(
|
||||
head: Option<&str>,
|
||||
expected: &str,
|
||||
marketplace_name: &str,
|
||||
) -> Result<String, String> {
|
||||
let head = head.ok_or_else(|| {
|
||||
format!("\"{marketplace_name}\" has not been fetched yet — refresh it first.")
|
||||
})?;
|
||||
if head != expected {
|
||||
return Err(format!(
|
||||
"\"{marketplace_name}\" has changed since you reviewed this item — review it again."
|
||||
));
|
||||
}
|
||||
Ok(head.to_string())
|
||||
}
|
||||
|
||||
/// 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.
|
||||
@@ -226,6 +246,23 @@ pub(crate) mod ops {
|
||||
}
|
||||
}
|
||||
|
||||
/// Final review I2: an install or update pins exactly the commit the
|
||||
/// person reviewed, or nothing.
|
||||
#[test]
|
||||
fn only_the_reviewed_head_is_pinned() {
|
||||
let h = "a".repeat(40);
|
||||
assert_eq!(reviewed_head(Some(&h), &h, "Team").unwrap(), h);
|
||||
let moved = reviewed_head(Some(&h), &"b".repeat(40), "Team").unwrap_err();
|
||||
assert!(moved.contains("changed since you reviewed"), "{moved}");
|
||||
assert!(moved.contains("review it again"), "{moved}");
|
||||
assert!(reviewed_head(Some(&h), "", "Team").is_err());
|
||||
let unfetched = reviewed_head(None, &h, "Team").unwrap_err();
|
||||
assert!(
|
||||
unfetched.contains("has not been fetched yet"),
|
||||
"{unfetched}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Pre-flight F13: the add form and the fetch agree on what a branch
|
||||
/// is, so a name the fetch would refuse is refused up front.
|
||||
#[test]
|
||||
@@ -642,24 +679,21 @@ pub async fn forget_marketplace_installs(
|
||||
// Installs
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
/// Pins the item at the marketplace's current head. Returns fresh settings;
|
||||
/// for a project scope the caller reloads projects.
|
||||
/// Pins the item at `expected_commit`, the head the person reviewed, which
|
||||
/// must still be the marketplace's head. Returns fresh settings; for a
|
||||
/// project scope the caller reloads projects.
|
||||
#[tauri::command]
|
||||
pub async fn install_marketplace_item(
|
||||
item: MarketplaceItemRef,
|
||||
scope: InstallScope,
|
||||
expected_commit: String,
|
||||
state: State<'_, AppState>,
|
||||
) -> Result<AppSettings, String> {
|
||||
validate_item(&item)?;
|
||||
let settings = state.settings_store.get();
|
||||
let m = find_marketplace(&settings, &item.marketplace_id)?;
|
||||
let snap = snapshot_blocking(&state, &m).await?;
|
||||
let head = snap.head_commit.clone().ok_or_else(|| {
|
||||
format!(
|
||||
"\"{}\" has not been fetched yet — refresh it first.",
|
||||
m.name
|
||||
)
|
||||
})?;
|
||||
let head = ops::reviewed_head(snap.head_commit.as_deref(), &expected_commit, &m.name)?;
|
||||
let entry = snap
|
||||
.items
|
||||
.iter()
|
||||
@@ -781,26 +815,21 @@ pub async fn marketplace_item_diff(
|
||||
.map_err(|e| format!("Computing the diff failed: {e}"))?
|
||||
}
|
||||
|
||||
/// Moves one install's pin to the marketplace's head, if the item is still
|
||||
/// installable there.
|
||||
/// Moves one install's pin to `expected_commit`, the head whose diff the
|
||||
/// person accepted, if that is still the marketplace's head and the item is
|
||||
/// still installable there.
|
||||
#[tauri::command]
|
||||
pub async fn update_marketplace_item(
|
||||
item: MarketplaceItemRef,
|
||||
scope: InstallScope,
|
||||
expected_commit: String,
|
||||
state: State<'_, AppState>,
|
||||
) -> Result<(), String> {
|
||||
validate_item(&item)?;
|
||||
let settings = state.settings_store.get();
|
||||
let m = find_marketplace(&settings, &item.marketplace_id)?;
|
||||
let head = snapshot_blocking(&state, &m)
|
||||
.await?
|
||||
.head_commit
|
||||
.ok_or_else(|| {
|
||||
format!(
|
||||
"\"{}\" has not been fetched yet — refresh it first.",
|
||||
m.name
|
||||
)
|
||||
})?;
|
||||
let snap = snapshot_blocking(&state, &m).await?;
|
||||
let head = ops::reviewed_head(snap.head_commit.as_deref(), &expected_commit, &m.name)?;
|
||||
|
||||
let repo = git::cache_path(state.marketplace.data_root(), &m.id);
|
||||
let (kind, key, at) = (item.kind, item.key.clone(), head.clone());
|
||||
|
||||
Reference in New Issue
Block a user