From 5a8728ef04383a578c258dc9107f7132f6797e1a Mon Sep 17 00:00:00 2001 From: Julian Chang Date: Fri, 9 Oct 2026 00:01:40 -0400 Subject: [PATCH] Retroactive fix for 1ab294f2 --- .changeset/reask_seeded_bundle_choices.md | 5 + .../src/components/bundle_choices_popup.rs | 23 +++- packages/oneclient_app/src/hooks/actions.rs | 16 +++ packages/oneclient_app/src/notifications.rs | 1 + .../oneclient_content/src/bundles/install.rs | 98 ++++++++++---- .../oneclient_content/src/bundles/updates.rs | 9 +- .../oneclient_core/tests/bundle_updates.rs | 126 ++++++++++++++++++ packages/oneclient_db/src/dao/artifact.rs | 3 +- .../oneclient_db/src/dao/cluster_bundle.rs | 19 +++ 9 files changed, 268 insertions(+), 32 deletions(-) create mode 100644 .changeset/reask_seeded_bundle_choices.md diff --git a/.changeset/reask_seeded_bundle_choices.md b/.changeset/reask_seeded_bundle_choices.md new file mode 100644 index 00000000..4b6ef751 --- /dev/null +++ b/.changeset/reask_seeded_bundle_choices.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +# SkyBlock content installed without asking is removed from 26.3 clusters once, and turning an optional bundle down removes its mods diff --git a/packages/oneclient_app/src/components/bundle_choices_popup.rs b/packages/oneclient_app/src/components/bundle_choices_popup.rs index 83a1f8f2..232b5771 100644 --- a/packages/oneclient_app/src/components/bundle_choices_popup.rs +++ b/packages/oneclient_app/src/components/bundle_choices_popup.rs @@ -53,10 +53,19 @@ impl Component for BundleChoicesPopup { .width(Size::flex(1.0)) .color(colors::fg_primary()), ) - .child(toggle_controlled(chosen.read().contains(name), on_toggle)), + .child(toggle_controlled( + choices.held.contains(name) != chosen.read().contains(name), + on_toggle, + )), ); } + let names: Vec = choices + .bundles + .iter() + .map(|(name, _)| name.clone()) + .collect(); + let held = choices.held.clone(); let later = dispatch.clone(); let close = dispatch.clone(); @@ -92,7 +101,7 @@ impl Component for BundleChoicesPopup { .child( label() .text(format!( - "{} has optional bundles you have not chosen yet. Turn on the ones you want.", + "{} has optional bundles you have not chosen yet. Turn on the ones you want, turning one off removes its mods.", choices.cluster_name )) .font_size(12.5) @@ -125,7 +134,15 @@ impl Component for BundleChoicesPopup { Button::new() .primary() .on_press(move |_| { - let picked = chosen.read().clone(); + let flipped = chosen.read().clone(); + let picked = names + .iter() + .filter(|name| { + held.contains(*name) + != flipped.contains(*name) + }) + .cloned() + .collect(); chosen.set(HashSet::new()); dispatch.close_bundle_choices(Some(picked)); }) diff --git a/packages/oneclient_app/src/hooks/actions.rs b/packages/oneclient_app/src/hooks/actions.rs index 1da72690..eb0541c5 100644 --- a/packages/oneclient_app/src/hooks/actions.rs +++ b/packages/oneclient_app/src/hooks/actions.rs @@ -1827,7 +1827,23 @@ impl Actions { }; let choices_for: Vec = bundles.iter().map(|a| a.manifest.name.clone()).collect(); + let held = match oneclient_db::dao::cluster_bundle::list_bundle_tracked( + &state.services.db, + cluster_id, + ) + .await + { + Ok(tracked) => tracked + .into_iter() + .filter_map(|row| row.bundle_name) + .collect(), + Err(err) => { + tracing::warn!(cluster_id, error = %err, "could not read what the cluster holds, launching without asking about bundles"); + return; + } + }; let choices = BundleChoices { + held, cluster_name: crate::install::cluster_display_name(cluster_id, &state.services).await, bundles: bundles .iter() diff --git a/packages/oneclient_app/src/notifications.rs b/packages/oneclient_app/src/notifications.rs index 95424f79..403f2133 100644 --- a/packages/oneclient_app/src/notifications.rs +++ b/packages/oneclient_app/src/notifications.rs @@ -107,6 +107,7 @@ impl BlockedDownloads { pub struct BundleChoices { pub cluster_name: String, pub bundles: Vec<(String, String)>, + pub held: HashSet, } #[derive(Clone, Debug, PartialEq)] diff --git a/packages/oneclient_content/src/bundles/install.rs b/packages/oneclient_content/src/bundles/install.rs index c331baad..a2f83306 100644 --- a/packages/oneclient_content/src/bundles/install.rs +++ b/packages/oneclient_content/src/bundles/install.rs @@ -98,6 +98,52 @@ pub async fn set_bundle_choices( ctx: &ContentCtx, ) -> ContentResult<()> { bundle_dao::save_bundle_choices(&ctx.db, cluster_id, choices).await?; + + for row in bundle_dao::list_bundle_tracked(&ctx.db, cluster_id).await? { + let declined = row.bundle_name.as_ref().is_some_and(|name| { + choices + .iter() + .any(|(choice, accepted)| choice == name && !accepted) + }); + if declined { + detach_artifact(cluster_id, &row.hash, ctx).await?; + } + } + Ok(()) +} + +const UNASKED_SKYBLOCK: &str = "decline_unasked_skyblock"; + +#[tracing::instrument(level = "debug", skip_all, fields(cluster_id = cluster.id))] +pub(crate) async fn decline_unasked_skyblock( + cluster: &ClusterRow, + archives: &[BundleArchive], + ctx: &ContentCtx, +) -> ContentResult<()> { + let marker = format!("{UNASKED_SKYBLOCK}:{}", cluster.id); + if migration_dao::is_applied(&ctx.db, &marker).await? { + return Ok(()); + } + + let unasked = cluster.mc_version == "26.3" + && bundle_dao::predates_bundle_choices(&ctx.db, cluster.id).await?; + if unasked { + let declined: Vec<(String, bool)> = archives + .iter() + .filter(|archive| { + !archive.manifest.enabled + && archive + .manifest + .category + .trim() + .eq_ignore_ascii_case("SkyBlock") + }) + .map(|archive| (archive.manifest.name.clone(), false)) + .collect(); + set_bundle_choices(cluster.id, &declined, ctx).await?; + } + + migration_dao::mark_applied(&ctx.db, &marker).await?; Ok(()) } @@ -971,6 +1017,35 @@ pub async fn remove_artifact_from_cluster( ) -> ContentResult<()> { let bundle_data = bundle_dao::get_bundle_tracked(&ctx.db, cluster_id, hash).await?; + detach_artifact(cluster_id, hash, ctx).await?; + + if let Some(tracked) = bundle_data + && let (Some(bundle_name), Some(package_id)) = + (tracked.bundle_name.clone(), tracked.package_id.clone()) + { + if record_override { + bundle_dao::save_override( + &ctx.db, + cluster_id, + &bundle_name, + &package_id, + OverrideType::Removed, + ) + .await?; + } else { + let replacement_exists = + bundle_dao::has_bundle_mapping_for_package(&ctx.db, cluster_id, &package_id) + .await?; + if !replacement_exists { + clear_suppressing_overrides(cluster_id, &package_id, ctx).await?; + } + } + } + + Ok(()) +} + +async fn detach_artifact(cluster_id: i64, hash: &str, ctx: &ContentCtx) -> ContentResult<()> { // Looked up first but never allowed to block removal // a package whose artifact row has gone missing must still be removable let cluster = PackageStore::get_cluster(cluster_id, ctx).await?; @@ -1006,29 +1081,6 @@ pub async fn remove_artifact_from_cluster( tracing::warn!(hash, error = %err, "failed to evict unused artifact from the cache"); } - if let Some(tracked) = bundle_data - && let (Some(bundle_name), Some(package_id)) = - (tracked.bundle_name.clone(), tracked.package_id.clone()) - { - if record_override { - bundle_dao::save_override( - &ctx.db, - cluster_id, - &bundle_name, - &package_id, - OverrideType::Removed, - ) - .await?; - } else { - let replacement_exists = - bundle_dao::has_bundle_mapping_for_package(&ctx.db, cluster_id, &package_id) - .await?; - if !replacement_exists { - clear_suppressing_overrides(cluster_id, &package_id, ctx).await?; - } - } - } - Ok(()) } diff --git a/packages/oneclient_content/src/bundles/updates.rs b/packages/oneclient_content/src/bundles/updates.rs index 54f45862..dab845b9 100644 --- a/packages/oneclient_content/src/bundles/updates.rs +++ b/packages/oneclient_content/src/bundles/updates.rs @@ -17,10 +17,10 @@ use tokio::sync::Mutex as AsyncMutex; use futures_util::StreamExt; use crate::bundles::install::{ - BUNDLE_INSTALL_CONCURRENCY, accepted_bundles, bundle_cluster, disable_was_deliberate, - external_ids_by_sha1, find_override, find_user_suppression, heal_bundle_activity, - install_package_from_bundle, remove_artifact_from_cluster, set_artifact_enabled_to, - takes_bundle, + BUNDLE_INSTALL_CONCURRENCY, accepted_bundles, bundle_cluster, decline_unasked_skyblock, + disable_was_deliberate, external_ids_by_sha1, find_override, find_user_suppression, + heal_bundle_activity, install_package_from_bundle, remove_artifact_from_cluster, + set_artifact_enabled_to, takes_bundle, }; use crate::bundles::manager::BundlesManager; use crate::bundles::overrides; @@ -486,6 +486,7 @@ pub async fn apply_bundle_updates_with( let loader = GameLoader::from_repr(cluster.mc_loader as u8).unwrap_or(GameLoader::Fabric); if let Ok(archives) = bundles.archives_for(ctx, &cluster.mc_version, loader).await { heal_bundle_activity(cluster_id, &archives, ctx).await?; + decline_unasked_skyblock(&cluster, &archives, ctx).await?; } } diff --git a/packages/oneclient_core/tests/bundle_updates.rs b/packages/oneclient_core/tests/bundle_updates.rs index 58dc7939..7c40d96f 100644 --- a/packages/oneclient_core/tests/bundle_updates.rs +++ b/packages/oneclient_core/tests/bundle_updates.rs @@ -1390,6 +1390,132 @@ async fn taking_a_bundle_at_the_prompt_records_the_choice() { assert!(pending().await.is_empty()); } +#[tokio::test] +async fn turning_a_bundle_down_removes_what_it_installed() { + let state = oneclient_core::dev::ephemeral_state().await.unwrap(); + seed_opt_in_bundle(&state, BUNDLE).await; + let cluster_id = cluster_with_tracked_mod(&state).await; + let ctx = state.services.content(); + + oneclient_content::bundles::set_bundle_choices(cluster_id, &[(BUNDLE.to_string(), true)], &ctx) + .await + .unwrap(); + let tracked = bundle_dao::list_bundle_tracked(&state.services.db, cluster_id) + .await + .unwrap(); + assert_eq!(tracked.len(), 1, "taking the bundle keeps its mods"); + bundle_dao::save_override( + &state.services.db, + cluster_id, + BUNDLE, + PROJECT_ID, + OverrideType::Disabled, + ) + .await + .unwrap(); + + oneclient_content::bundles::set_bundle_choices( + cluster_id, + &[(BUNDLE.to_string(), false)], + &ctx, + ) + .await + .unwrap(); + let tracked = bundle_dao::list_bundle_tracked(&state.services.db, cluster_id) + .await + .unwrap(); + assert!(tracked.is_empty()); + let overrides = bundle_dao::list_overrides(&state.services.db, cluster_id) + .await + .unwrap(); + assert_eq!(overrides.len(), 1, "a mod the user switched off stays off"); +} + +#[tokio::test] +async fn skyblock_nobody_asked_for_is_taken_out_of_an_old_26_3_cluster_once() { + const SKYBLOCK: &str = "Test [SkyBlock]"; + let state = oneclient_core::dev::ephemeral_state().await.unwrap(); + let db = &state.services.db; + let mut m = named_manifest(SKYBLOCK, vec![managed_file(true)]); + m.enabled = false; + m.category = "SkyBlock".to_string(); + m.mc_version = "26.3".to_string(); + seed_bundle_with_pack(&state, m).await; + let cluster_id = cluster_with_tracked_mod(&state).await; + let ctx = state.services.content(); + let track = || { + bundle_dao::track_bundle_artifact(db, cluster_id, HASH, SKYBLOCK, VERSION_ID, PROJECT_ID) + }; + let apply = || { + oneclient_content::bundles::apply_bundle_updates( + cluster_id, + state.bundles.as_ref(), + &ctx, + None, + ) + }; + track().await.unwrap(); + bundle_dao::save_bundle_choices(db, cluster_id, &[(SKYBLOCK.to_string(), true)]) + .await + .unwrap(); + + sqlx::query("UPDATE clusters SET mc_version = '26.3' WHERE id = ?") + .bind(cluster_id) + .execute(db) + .await + .unwrap(); + apply().await.unwrap(); + assert_eq!( + bundle_dao::list_bundle_tracked(db, cluster_id) + .await + .unwrap() + .len(), + 1, + "a cluster made after bundles started asking is left alone" + ); + + sqlx::query("DELETE FROM applied_migrations") + .execute(db) + .await + .unwrap(); + sqlx::query("UPDATE clusters SET created_at = '2020-01-01T00:00:00+00:00' WHERE id = ?") + .bind(cluster_id) + .execute(db) + .await + .unwrap(); + apply().await.unwrap(); + assert!( + bundle_dao::list_bundle_tracked(db, cluster_id) + .await + .unwrap() + .is_empty() + ); + + artifact_dao::insert_artifact( + db, + HASH, + ContentType::Mod as i64, + "artifacts/sodium.jar", + "sodium.jar", + Some(1), + ) + .await + .unwrap(); + artifact_dao::link_cluster_artifact(db, cluster_id, HASH, "sodium.jar") + .await + .unwrap(); + track().await.unwrap(); + apply().await.unwrap(); + assert_eq!( + bundle_dao::list_bundle_tracked(db, cluster_id) + .await + .unwrap() + .len(), + 1, + "taking SkyBlock again later is not undone" + ); +} + #[tokio::test] async fn an_opt_in_bundle_switched_off_by_accident_is_repaired_at_install() { let state = oneclient_core::dev::ephemeral_state().await.unwrap(); diff --git a/packages/oneclient_db/src/dao/artifact.rs b/packages/oneclient_db/src/dao/artifact.rs index 30e28395..3ec0c7b0 100644 --- a/packages/oneclient_db/src/dao/artifact.rs +++ b/packages/oneclient_db/src/dao/artifact.rs @@ -1,8 +1,7 @@ use sqlx::SqlitePool; use crate::models::{ - ArtifactRow, ClusterArtifactRow, ClusterKind, LinkedArtifactRow, ProviderReleaseRow, - SeenStatus, + ArtifactRow, ClusterArtifactRow, ClusterKind, LinkedArtifactRow, ProviderReleaseRow, SeenStatus, }; pub async fn get_artifact_by_hash( diff --git a/packages/oneclient_db/src/dao/cluster_bundle.rs b/packages/oneclient_db/src/dao/cluster_bundle.rs index b5b12771..8651b8e8 100644 --- a/packages/oneclient_db/src/dao/cluster_bundle.rs +++ b/packages/oneclient_db/src/dao/cluster_bundle.rs @@ -341,6 +341,25 @@ pub async fn save_bundle_choices( tx.commit().await } +pub async fn predates_bundle_choices( + pool: &SqlitePool, + cluster_id: i64, +) -> Result { + sqlx::query_scalar( + r#" + SELECT EXISTS ( + SELECT 1 + FROM clusters c, _sqlx_migrations m + WHERE c.id = ? AND m.version = 20261005120000 + AND (c.created_at IS NULL OR datetime(c.created_at) < m.installed_on) + ) + "#, + ) + .bind(cluster_id) + .fetch_one(pool) + .await +} + pub async fn copy_bundle_choices( pool: &SqlitePool, source_cluster_id: i64,