diff --git a/.sqlx/query-dc93cdd57e23e48f8e3c333db4ec396b0ee319adec619e5a7d147805573ae2b2.json b/.sqlx/query-ab6dcea84a9502d5f8396a207ae613a4c41eb729ca22249f15eb45e6c6030094.json similarity index 62% rename from .sqlx/query-dc93cdd57e23e48f8e3c333db4ec396b0ee319adec619e5a7d147805573ae2b2.json rename to .sqlx/query-ab6dcea84a9502d5f8396a207ae613a4c41eb729ca22249f15eb45e6c6030094.json index 2fe3f6e0..e564fe71 100644 --- a/.sqlx/query-dc93cdd57e23e48f8e3c333db4ec396b0ee319adec619e5a7d147805573ae2b2.json +++ b/.sqlx/query-ab6dcea84a9502d5f8396a207ae613a4c41eb729ca22249f15eb45e6c6030094.json @@ -1,6 +1,6 @@ { "db_name": "SQLite", - "query": "\n SELECT 1 FROM cluster_artifacts\n WHERE cluster_id = ? AND bundle_name = ? AND package_id = ?\n LIMIT 1\n ", + "query": "\n SELECT 1 FROM cluster_artifacts\n WHERE cluster_id = ? AND bundle_name IS NOT NULL AND package_id = ?\n LIMIT 1\n ", "describe": { "columns": [ { @@ -11,11 +11,11 @@ } ], "parameters": { - "Right": 3 + "Right": 2 }, "nullable": [ false ] }, - "hash": "dc93cdd57e23e48f8e3c333db4ec396b0ee319adec619e5a7d147805573ae2b2" + "hash": "ab6dcea84a9502d5f8396a207ae613a4c41eb729ca22249f15eb45e6c6030094" } diff --git a/packages/oneclient_app/src/hooks/actions.rs b/packages/oneclient_app/src/hooks/actions.rs index 4bf25c2e..b05b3172 100644 --- a/packages/oneclient_app/src/hooks/actions.rs +++ b/packages/oneclient_app/src/hooks/actions.rs @@ -797,7 +797,7 @@ impl Actions { // had leaves the newest live and the rest switched off. Done // before the refresh below so the version list never renders the // moment where both copies read as active. - if let Err(err) = oneclient_content::packages::reconcile_duplicate_activity( + if let Err(err) = oneclient_content::bundles::reconcile_duplicate_activity( cluster_id, &state.services.content(), ) @@ -806,13 +806,6 @@ impl Actions { tracing::warn!(%err, "failed to resolve duplicate package versions"); } - // What the install button turns into is read straight off the - // cluster's content, so that is refreshed here and waited for - // before the busy flag drops. Left to the `ClustersChanged` - // signal alone, the button came back as a live "Install" the - // moment the download finished and only grew its "Installed" - // badge once an event-pump round trip and a full cluster-query - // sweep — network calls included — had gone by. super::invalidate_cluster_content_queries().await; // Everything else the install touched is nobody's blocker and diff --git a/packages/oneclient_app/src/view/app/browser/mod.rs b/packages/oneclient_app/src/view/app/browser/mod.rs index 50170f7e..6173d46f 100644 --- a/packages/oneclient_app/src/view/app/browser/mod.rs +++ b/packages/oneclient_app/src/view/app/browser/mod.rs @@ -188,16 +188,6 @@ pub(crate) fn installed_badge_overlay(installed: InstallSource) -> impl IntoElem } /// Which of several installed versions the game actually loads. -/// -/// Only worth showing when a package has more than one copy in the cluster — -/// with a single one "active" says nothing "installed" hasn't. -/// -/// Reads the artifact's `enabled` flag rather than working the answer out -/// itself, because -/// [`reconcile_duplicate_activity`](oneclient_content::packages::reconcile_duplicate_activity) -/// has already resolved duplicates to the newest copy. Exactly one row in a -/// duplicated set should read Active; more than one means that pass has not run -/// since the copies appeared. pub(crate) fn activity_badge(active: bool) -> impl IntoElement { let (text, color) = if active { ("Active", colors::brand()) diff --git a/packages/oneclient_app/src/view/onboarding/downloading/mod.rs b/packages/oneclient_app/src/view/onboarding/downloading/mod.rs index e54a84df..1ecfe877 100644 --- a/packages/oneclient_app/src/view/onboarding/downloading/mod.rs +++ b/packages/oneclient_app/src/view/onboarding/downloading/mod.rs @@ -507,9 +507,15 @@ fn build_plans( items .iter() .map(|cb| { + let kept = kept_package_ids(cb.cluster.id, &cb.archives, selected); let mut overrides = Vec::new(); for archive in &cb.archives { - overrides.extend(archive_overrides(cb.cluster.id, archive, selected)); + overrides.extend(archive_overrides( + cb.cluster.id, + archive, + selected, + &kept, + )); } ClusterPlan { cluster_id: cb.cluster.id, @@ -535,10 +541,62 @@ fn cluster_predownloads(versions: &[VersionMetadata], cluster: &Cluster) -> bool }) } +/// Whether a bundle keeps its hidden files: true once anything visible in it was +/// taken, including a single opted-in extra that would otherwise lose its +/// dependencies. +fn bundle_taken( + cluster_id: i64, + archive: &BundleArchive, + selected: &std::collections::HashSet, +) -> bool { + archive.manifest.files.iter().any(|file| { + !file.hidden + && selected.contains(&pkg_key( + cluster_id, + &archive.manifest.name, + &file.kind.package_id(), + )) + }) +} + +/// Every package id this cluster is keeping, across all of its bundles. +/// +/// Bundles overlap: the library one pack needs is often the library another +/// needs too, shipped hidden by both. Deciding each archive on its own meant a +/// declined bundle recorded its hidden files as removed even when an accepted +/// bundle right beside it was about to install the very same package — leaving a +/// `removed` row for something the cluster demonstrably has, which then answered +/// for it in anything that looks a package up by id. +fn kept_package_ids( + cluster_id: i64, + archives: &[BundleArchive], + selected: &std::collections::HashSet, +) -> std::collections::HashSet { + let mut kept = std::collections::HashSet::new(); + + for archive in archives { + let takes_hidden = bundle_taken(cluster_id, archive, selected); + for file in &archive.manifest.files { + let package_id = file.kind.package_id(); + let wanted = if file.hidden { + takes_hidden + } else { + selected.contains(&pkg_key(cluster_id, &archive.manifest.name, &package_id)) + }; + if wanted { + kept.insert(package_id); + } + } + } + + kept +} + fn archive_overrides( cluster_id: i64, archive: &BundleArchive, selected: &std::collections::HashSet, + kept_elsewhere: &std::collections::HashSet, ) -> Vec<(String, String, OverrideType)> { let bundle_name = &archive.manifest.name; let wants = |file: &BundleFile| { @@ -546,19 +604,13 @@ fn archive_overrides( }; // Hidden files are dependencies the user never sees, so they follow the - // bundle: kept if anything from it was taken (including a single opted-in - // extra, which would otherwise lose its dependencies), dropped if not. - let bundle_taken = archive - .manifest - .files - .iter() - .filter(|f| !f.hidden) - .any(wants); + // bundle: kept if anything from it was taken, dropped if not. + let takes_hidden = bundle_taken(cluster_id, archive, selected); let mut overrides = Vec::new(); for file in &archive.manifest.files { let wanted = if file.hidden { - bundle_taken + takes_hidden } else { wants(file) }; @@ -568,8 +620,15 @@ fn archive_overrides( (true, true) | (false, false) => continue, // Opting into a mod the bundle ships turned off. (true, false) => OverrideType::Enabled, - // Declined: don't fetch it at all. - (false, true) => OverrideType::Removed, + // Declined — but only really declined if no bundle the user did take + // ships the same package. Recording a removal for something that is + // being installed anyway states the opposite of what happens. + (false, true) => { + if kept_elsewhere.contains(&file.kind.package_id()) { + continue; + } + OverrideType::Removed + } }; overrides.push((bundle_name.clone(), file.kind.package_id(), override_type)); @@ -590,30 +649,17 @@ fn rough_download_estimate( if !cluster_predownloads(versions, &cb.cluster) { continue; } + // What the cluster is keeping is already the answer to "what gets + // downloaded", so the estimate reads it directly rather than + // reconstructing it from the overrides. Counted once per package: two + // bundles shipping the same library is one download, which is what the + // install path does with it too. + let kept = kept_package_ids(cb.cluster.id, &cb.archives, selected); + let mut counted: std::collections::HashSet = std::collections::HashSet::new(); for archive in &cb.archives { - let dropped: std::collections::HashSet = - archive_overrides(cb.cluster.id, archive, selected) - .into_iter() - .filter(|(_, _, ty)| *ty == OverrideType::Removed) - .map(|(_, pid, _)| pid) - .collect(); - let opted_in: std::collections::HashSet = - archive_overrides(cb.cluster.id, archive, selected) - .into_iter() - .filter(|(_, _, ty)| *ty == OverrideType::Enabled) - .map(|(_, pid, _)| pid) - .collect(); - for file in &archive.manifest.files { - let pid = file.kind.package_id(); - let installing = if opted_in.contains(&pid) { - true - } else if dropped.contains(&pid) { - false - } else { - file.enabled - }; - if installing { + let package_id = file.kind.package_id(); + if kept.contains(&package_id) && counted.insert(package_id) { total += file.size; } } diff --git a/packages/oneclient_content/src/bundles/install.rs b/packages/oneclient_content/src/bundles/install.rs index 76c53eb0..b16da13c 100644 --- a/packages/oneclient_content/src/bundles/install.rs +++ b/packages/oneclient_content/src/bundles/install.rs @@ -173,6 +173,88 @@ pub async fn install_bundle( install_enabled_bundle_files(&archive, cluster_id, skip_compatibility, None, ctx).await } +/// Whether a bundle file that is switched off was switched off on purpose. +fn disable_was_deliberate(hidden: bool, user_override: Option) -> bool { + match user_override { + Some(OverrideType::Removed) => true, + // Hidden files are dependencies the bundle needs and the user was never + // shown, so they follow the bundle rather than standing on their own. + // Only an outright removal keeps one off. + Some(OverrideType::Disabled) => !hidden, + Some(OverrideType::Enabled) | None => false, + } +} + +/// Switches bundle content back on where it was left off without saving the choice +#[tracing::instrument(level = "debug", skip(archives, ctx))] +pub async fn heal_bundle_activity( + cluster_id: i64, + archives: &[BundleArchive], + ctx: &ContentCtx, +) -> ContentResult<()> { + let tracked = bundle_dao::list_bundle_tracked(&ctx.db, cluster_id).await?; + if tracked.iter().all(|row| row.enabled != 0) { + return Ok(()); + } + + let mut hidden: std::collections::HashMap<(&str, String), bool> = + std::collections::HashMap::new(); + for archive in archives { + for file in &archive.manifest.files { + hidden.insert( + (archive.manifest.name.as_str(), file.kind.package_id()), + file.hidden, + ); + } + } + + let overrides = bundle_dao::list_overrides(&ctx.db, cluster_id).await?; + + for row in tracked.iter().filter(|row| row.enabled == 0) { + let (Some(bundle_name), Some(package_id)) = (&row.bundle_name, &row.package_id) else { + continue; + }; + let Some(is_hidden) = hidden + .get(&(bundle_name.as_str(), package_id.clone())) + .copied() + else { + continue; + }; + + let user_override = find_override(&overrides, bundle_name, package_id); + if disable_was_deliberate(is_hidden, user_override) { + continue; + } + + tracing::info!( + cluster_id, + bundle = %bundle_name, + package_id, + hidden = is_hidden, + ?user_override, + "re-enabling bundle content that was switched off with nothing recording the choice" + ); + + // One unrepairable row is not worth failing an install over; the rest of + // the cluster still wants fixing. + if let Err(err) = + PackageStore::set_artifact_enabled_to(cluster_id, &row.hash, true, ctx).await + { + tracing::warn!(hash = %row.hash, error = %err, "failed to re-enable bundle content"); + continue; + } + + // A hidden file reaching here with a `disabled` row is the one case where + // the override is the wrong half. Drop it, or the next pass reads the + // same disagreement and the two records never settle. + if user_override == Some(OverrideType::Disabled) { + bundle_dao::remove_override(&ctx.db, cluster_id, bundle_name, package_id).await?; + } + } + + Ok(()) +} + #[tracing::instrument(level = "debug", skip(archive, progress, ctx), fields(bundle = %archive.manifest.name))] pub async fn install_enabled_bundle_files( archive: &BundleArchive, @@ -182,6 +264,7 @@ pub async fn install_enabled_bundle_files( ctx: &ContentCtx, ) -> ContentResult> { extract_bundle_overrides_for_cluster(archive, cluster_id, ctx).await?; + heal_bundle_activity(cluster_id, std::slice::from_ref(archive), ctx).await?; let overrides = bundle_dao::list_overrides(&ctx.db, cluster_id).await?; let bundle_name = archive.manifest.name.clone(); @@ -475,6 +558,51 @@ pub async fn toggle_artifact_enabled( Ok(enabled) } +/// Puts an artifact into a known enabled state and updates the bundle override +/// records to match. +/// +/// The set counterpart to [`toggle_artifact_enabled`], and the one to reach for +/// whenever the caller already knows the state it wants. Flipping only lands on +/// the right answer when the current value is known to be the opposite, which is +/// true of a user pressing a switch and false of everything else: an artifact +/// that is relinked keeps whatever `enabled` it already had, so a flip applied +/// to a row that was already correct puts it wrong. +#[tracing::instrument(level = "debug", skip(ctx))] +pub async fn set_artifact_enabled_to( + cluster_id: i64, + hash: &str, + enabled: bool, + ctx: &ContentCtx, +) -> ContentResult<()> { + PackageStore::set_artifact_enabled_to(cluster_id, hash, enabled, ctx).await?; + + if enabled { + on_user_enable_artifact(cluster_id, hash, ctx).await + } else { + on_user_disable_artifact(cluster_id, hash, ctx).await + } +} + +/// [`crate::packages::reconcile_duplicate_activity`] with the bundle half added. +/// +/// Same split as [`toggle_artifact_enabled`]: `packages` owns the flag and knows +/// nothing about bundles, and the override that has to move with it is written +/// here. A bundle copy that loses to a newer one used to be switched off with +/// nothing recording why, which read as a state nobody chose — and once this +/// module started repairing exactly that state, the two passes would have spent +/// every launch undoing each other. +#[tracing::instrument(level = "debug", skip(ctx))] +pub async fn reconcile_duplicate_activity( + cluster_id: i64, + ctx: &ContentCtx, +) -> ContentResult<()> { + for hash in crate::packages::reconcile_duplicate_activity(cluster_id, ctx).await? { + on_user_disable_artifact(cluster_id, &hash, ctx).await?; + } + + Ok(()) +} + pub async fn on_user_disable_artifact( cluster_id: i64, hash: &str, @@ -580,10 +708,9 @@ pub async fn remove_artifact_from_cluster( ) .await?; } else { - let replacement_exists = bundle_dao::has_bundle_mapping( + let replacement_exists = bundle_dao::has_bundle_mapping_for_package( &ctx.db, cluster_id, - &bundle_name, &package_id, ) .await?; @@ -663,6 +790,36 @@ mod tests { } } + #[test] + fn a_disable_with_no_override_behind_it_is_an_accident() { + assert!(!disable_was_deliberate(false, None)); + assert!(!disable_was_deliberate(true, None)); + } + + #[test] + fn a_users_own_disable_is_respected() { + assert!(disable_was_deliberate( + false, + Some(OverrideType::Disabled) + )); + assert!(disable_was_deliberate(false, Some(OverrideType::Removed))); + } + + #[test] + fn a_hidden_dependency_cannot_be_disabled_on_its_own() { + assert!( + !disable_was_deliberate(true, Some(OverrideType::Disabled)), + "a hidden file follows its bundle; only removing it keeps it off" + ); + assert!(disable_was_deliberate(true, Some(OverrideType::Removed))); + } + + #[test] + fn an_opt_in_override_never_reads_as_a_disable() { + assert!(!disable_was_deliberate(false, Some(OverrideType::Enabled))); + assert!(!disable_was_deliberate(true, Some(OverrideType::Enabled))); + } + #[test] fn find_override_matches_on_bundle_and_package() { let rows = vec![ diff --git a/packages/oneclient_content/src/bundles/mod.rs b/packages/oneclient_content/src/bundles/mod.rs index 1f39a6ab..168e6dab 100644 --- a/packages/oneclient_content/src/bundles/mod.rs +++ b/packages/oneclient_content/src/bundles/mod.rs @@ -10,11 +10,14 @@ mod updates; pub use error::BundleError; pub use install::{ - effective_enabled, enabled_bundle_bytes, extract_bundle_overrides_for_cluster, install_bundle, + effective_enabled, enabled_bundle_bytes, extract_bundle_overrides_for_cluster, + heal_bundle_activity, install_bundle, install_cluster_bundles, install_enabled_bundle_files, install_package_from_bundle, list_cluster_bundle_overrides, on_user_disable_artifact, on_user_enable_artifact, on_user_remove_artifact, - remove_artifact_from_cluster, set_bundle_package_enabled, set_bundle_package_opt_in, + reconcile_duplicate_activity, + remove_artifact_from_cluster, set_artifact_enabled_to, set_bundle_package_enabled, + set_bundle_package_opt_in, toggle_artifact_enabled, set_bundle_package_override, set_bundle_package_overrides, }; diff --git a/packages/oneclient_content/src/bundles/updates.rs b/packages/oneclient_content/src/bundles/updates.rs index 3493b068..aff8b531 100644 --- a/packages/oneclient_content/src/bundles/updates.rs +++ b/packages/oneclient_content/src/bundles/updates.rs @@ -12,8 +12,8 @@ use tokio::sync::Mutex as AsyncMutex; use futures_util::StreamExt; use crate::bundles::install::{ - BUNDLE_INSTALL_CONCURRENCY, install_package_from_bundle, remove_artifact_from_cluster, - toggle_artifact_enabled, + BUNDLE_INSTALL_CONCURRENCY, heal_bundle_activity, install_package_from_bundle, + remove_artifact_from_cluster, set_artifact_enabled_to, }; use crate::bundles::manager::BundlesManager; use crate::bundles::overrides; @@ -382,6 +382,19 @@ pub async fn apply_bundle_updates_with( ) -> ContentResult { let lock = cluster_lock(cluster_id); let _guard = lock.lock().await; + + // Before anything is decided. The check below works from the override table + // and the manifest defaults and never consults `enabled`, so a cluster + // holding content that was switched off with nothing recording the choice + // would be read as healthy and left exactly as it is. + { + let cluster = PackageStore::get_cluster(cluster_id, ctx).await?; + 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?; + } + } + let overrides = bundle_dao::list_overrides(&ctx.db, cluster_id).await?; let check = check_bundle_updates_inner(cluster_id, bundles, ctx, &overrides).await?; @@ -547,6 +560,11 @@ pub async fn apply_bundle_updates_with( /// Local bookkeeping for an update whose new file is already downloaded and /// linked: apply the user's enable/disable choice, then drop the artifact the /// update supersedes. +/// +/// The choice is *set*, not flipped. Linking an artifact leaves an existing row's +/// `enabled` alone, so re-applying an update to a row that already read correctly +/// used to turn it into its opposite — which is the "my mods turn themselves off +/// at random" report. #[tracing::instrument(level = "debug", skip_all, fields(cluster_id = update.cluster_id, bundle = %update.bundle_name, new_version = %update.new_version_id))] async fn reconcile_update( update: &BundlePackageUpdate, @@ -555,9 +573,8 @@ async fn reconcile_update( ctx: &ContentCtx, ) -> ContentResult<()> { let file_id = update.new_file.kind.package_id(); - if should_be_disabled(&update.bundle_name, &file_id, overrides) { - toggle_artifact_enabled(update.cluster_id, hash, ctx).await?; - } + let enabled = !should_be_disabled(&update.bundle_name, &file_id, overrides); + set_artifact_enabled_to(update.cluster_id, hash, enabled, ctx).await?; if hash == update.installed_hash { return Ok(()); @@ -574,11 +591,8 @@ async fn reconcile_addition( ctx: &ContentCtx, ) -> ContentResult<()> { let file_id = addition.new_file.kind.package_id(); - if should_be_disabled(&addition.bundle_name, &file_id, overrides) { - toggle_artifact_enabled(addition.cluster_id, hash, ctx).await?; - } - - Ok(()) + let enabled = !should_be_disabled(&addition.bundle_name, &file_id, overrides); + set_artifact_enabled_to(addition.cluster_id, hash, enabled, ctx).await } fn should_be_disabled( diff --git a/packages/oneclient_content/src/packages/activity.rs b/packages/oneclient_content/src/packages/activity.rs index 550659bc..0a8abf82 100644 --- a/packages/oneclient_content/src/packages/activity.rs +++ b/packages/oneclient_content/src/packages/activity.rs @@ -29,7 +29,7 @@ struct Copy { published_at: Option, } -/// Ensures each project in the cluster has exactly one enabled copy: the newest. +/// Leaves each project in the cluster with the newest copy. /// /// A no-op on a cluster with no duplicates, which is every healthy one, so it is /// cheap enough to run before a launch and after an install. @@ -37,40 +37,45 @@ struct Copy { pub async fn reconcile_duplicate_activity( cluster_id: i64, ctx: &ContentCtx, -) -> ContentResult<()> { +) -> ContentResult> { let linked = PackageStore::list_linked_artifacts(cluster_id, ctx).await?; + let mut switched_off = Vec::new(); - for ((provider, project_id), copies) in group_duplicates(&linked) { - let Some(winner) = newest(&copies) else { + for hash in duplicates_to_disable(&linked) { + tracing::info!( + cluster_id, + %hash, + "resolving duplicate versions of a package to the newest live one" + ); + + PackageStore::set_artifact_enabled_to(cluster_id, &hash, false, ctx).await?; + switched_off.push(hash); + } + + Ok(switched_off) +} + +fn duplicates_to_disable(linked: &[LinkedArtifactInfo]) -> Vec { + let mut out = Vec::new(); + + for (_, copies) in group_duplicates(linked) { + let live: Vec<&Copy> = copies.iter().filter(|copy| copy.enabled).collect(); + if live.len() < 2 { + continue; + } + + let Some(winner) = newest(live.iter().copied()) else { continue; }; - for copy in &copies { - let should_be_enabled = copy.hash == winner; - if copy.enabled == should_be_enabled { - continue; - } - - tracing::info!( - cluster_id, - ?provider, - project_id, - hash = %copy.hash, - enabled = should_be_enabled, - "resolving duplicate versions of a package to the newest one" - ); - - PackageStore::set_artifact_enabled_to( - cluster_id, - ©.hash, - should_be_enabled, - ctx, - ) - .await?; - } + out.extend( + live.iter() + .filter(|copy| copy.hash != winner) + .map(|copy| copy.hash.clone()), + ); } - Ok(()) + out } /// Groups the cluster's content by project, keeping only the projects it has @@ -110,9 +115,9 @@ fn group_duplicates( /// loses to any copy that has one — it is usually an older row written before /// the field was recorded — and an all-undated group falls back to the first, /// which at least makes the choice stable across runs rather than arbitrary. -fn newest(copies: &[Copy]) -> Option { +fn newest<'a>(copies: impl IntoIterator) -> Option { copies - .iter() + .into_iter() .max_by(|a, b| a.published_at.cmp(&b.published_at)) .map(|copy| copy.hash.clone()) } @@ -195,6 +200,54 @@ mod tests { assert_eq!(groups[0].1.len(), 2); } + #[test] + fn the_newest_live_copy_survives_and_the_rest_go_quiet() { + let mut disable = duplicates_to_disable(&[ + info(Some("sodium"), "old", true, Some("2026-01-01T00:00:00Z")), + info(Some("sodium"), "new", true, Some("2026-06-01T00:00:00Z")), + info(Some("sodium"), "middle", true, Some("2026-03-01T00:00:00Z")), + ]); + disable.sort(); + assert_eq!(disable, vec!["middle".to_string(), "old".to_string()]); + } + + #[test] + fn a_copy_that_is_already_off_is_not_counted_as_a_conflict() { + let disable = duplicates_to_disable(&[ + info(Some("sodium"), "live", true, Some("2026-01-01T00:00:00Z")), + info(Some("sodium"), "off", false, Some("2026-06-01T00:00:00Z")), + ]); + + assert!( + disable.is_empty(), + "one live copy is not a duplicate, however many switched-off copies sit beside it" + ); + } + + #[test] + fn the_user_disabling_the_newest_copy_is_left_standing() { + let disable = duplicates_to_disable(&[ + info(Some("sodium"), "chosen", true, Some("2026-01-01T00:00:00Z")), + info(Some("sodium"), "newest", false, Some("2026-06-01T00:00:00Z")), + ]); + + assert!( + disable.is_empty(), + "picking the older version on purpose must not be undone, and the newest must not be \ + switched back on" + ); + } + + #[test] + fn a_project_with_no_live_copy_at_all_is_left_alone() { + let disable = duplicates_to_disable(&[ + info(Some("sodium"), "a", false, Some("2026-01-01T00:00:00Z")), + info(Some("sodium"), "b", false, Some("2026-06-01T00:00:00Z")), + ]); + + assert!(disable.is_empty()); + } + #[test] fn content_with_no_project_is_left_alone() { let groups = group_duplicates(&[ diff --git a/packages/oneclient_core/src/game/shared_dir.rs b/packages/oneclient_core/src/game/shared_dir.rs index 58ff3dc0..569b24d0 100644 --- a/packages/oneclient_core/src/game/shared_dir.rs +++ b/packages/oneclient_core/src/game/shared_dir.rs @@ -62,7 +62,7 @@ pub async fn materialize_content( // of one package would otherwise hand the game every enabled copy, and for // mods that is a classloader conflict rather than a choice. Resolving to the // newest here means the rest are simply not desired. - if let Err(err) = oneclient_content::packages::reconcile_duplicate_activity( + if let Err(err) = oneclient_content::bundles::reconcile_duplicate_activity( cluster.id, &services.content(), ) diff --git a/packages/oneclient_db/.sqlx/query-a3bd6cf3048a6a5ed9d29e6bbb7f517676fe715c8af45eb5c58d69073e2d0304.json b/packages/oneclient_db/.sqlx/query-010f5dcda59ec32eaad1b9c3e2e9807f547a1376372c8b201653a36d6dd14cbc.json similarity index 65% rename from packages/oneclient_db/.sqlx/query-a3bd6cf3048a6a5ed9d29e6bbb7f517676fe715c8af45eb5c58d69073e2d0304.json rename to packages/oneclient_db/.sqlx/query-010f5dcda59ec32eaad1b9c3e2e9807f547a1376372c8b201653a36d6dd14cbc.json index 032726f5..33514fa6 100644 --- a/packages/oneclient_db/.sqlx/query-a3bd6cf3048a6a5ed9d29e6bbb7f517676fe715c8af45eb5c58d69073e2d0304.json +++ b/packages/oneclient_db/.sqlx/query-010f5dcda59ec32eaad1b9c3e2e9807f547a1376372c8b201653a36d6dd14cbc.json @@ -1,6 +1,6 @@ { "db_name": "SQLite", - "query": "\n\t\tINSERT INTO java_versions (absolute_path, major, version, vendor, os_arch)\n\t\tVALUES (?, ?, ?, ?, ?)\n\t\tON CONFLICT(absolute_path) DO UPDATE SET\n\t\t\tmajor = excluded.major,\n\t\t\tversion = excluded.version,\n\t\t\tvendor = excluded.vendor,\n\t\t\tos_arch = excluded.os_arch\n\t\tRETURNING absolute_path, major, version, vendor, os_arch\n\t\t", + "query": "\n\t\tINSERT INTO java_versions (absolute_path, major, version, vendor, os_arch, is_jdk)\n\t\tVALUES (?, ?, ?, ?, ?, ?)\n\t\tON CONFLICT(absolute_path) DO UPDATE SET\n\t\t\tmajor = excluded.major,\n\t\t\tversion = excluded.version,\n\t\t\tvendor = excluded.vendor,\n\t\t\tos_arch = excluded.os_arch,\n\t\t\tis_jdk = excluded.is_jdk\n\t\tRETURNING absolute_path, major, version, vendor, os_arch, is_jdk as \"is_jdk: bool\"\n\t\t", "describe": { "columns": [ { @@ -57,18 +57,30 @@ "name": "os_arch" } } + }, + { + "name": "is_jdk: bool", + "ordinal": 5, + "type_info": "Integer", + "origin": { + "Table": { + "table": "java_versions", + "name": "is_jdk" + } + } } ], "parameters": { - "Right": 5 + "Right": 6 }, "nullable": [ false, false, false, false, + false, false ] }, - "hash": "a3bd6cf3048a6a5ed9d29e6bbb7f517676fe715c8af45eb5c58d69073e2d0304" + "hash": "010f5dcda59ec32eaad1b9c3e2e9807f547a1376372c8b201653a36d6dd14cbc" } diff --git a/packages/oneclient_db/.sqlx/query-c1d3cf930d5f38e8f1046da1a8f895ee1de2d3fe9a40ecb47731618962d2a4ad.json b/packages/oneclient_db/.sqlx/query-0d9fb5d2fa8a788770e946241ea2f62f4b794c0fdda3ccbde49384689911aeba.json similarity index 75% rename from packages/oneclient_db/.sqlx/query-c1d3cf930d5f38e8f1046da1a8f895ee1de2d3fe9a40ecb47731618962d2a4ad.json rename to packages/oneclient_db/.sqlx/query-0d9fb5d2fa8a788770e946241ea2f62f4b794c0fdda3ccbde49384689911aeba.json index e65480d4..5da3a9d7 100644 --- a/packages/oneclient_db/.sqlx/query-c1d3cf930d5f38e8f1046da1a8f895ee1de2d3fe9a40ecb47731618962d2a4ad.json +++ b/packages/oneclient_db/.sqlx/query-0d9fb5d2fa8a788770e946241ea2f62f4b794c0fdda3ccbde49384689911aeba.json @@ -1,6 +1,6 @@ { "db_name": "SQLite", - "query": "\n SELECT absolute_path, major, version, vendor, os_arch FROM java_versions \n ORDER BY major DESC, version DESC\n ", + "query": "\n SELECT absolute_path, major, version, vendor, os_arch, is_jdk as \"is_jdk: bool\"\n FROM java_versions\n ORDER BY major DESC, version DESC\n ", "describe": { "columns": [ { @@ -57,6 +57,17 @@ "name": "os_arch" } } + }, + { + "name": "is_jdk: bool", + "ordinal": 5, + "type_info": "Integer", + "origin": { + "Table": { + "table": "java_versions", + "name": "is_jdk" + } + } } ], "parameters": { @@ -67,8 +78,9 @@ false, false, false, + false, false ] }, - "hash": "c1d3cf930d5f38e8f1046da1a8f895ee1de2d3fe9a40ecb47731618962d2a4ad" + "hash": "0d9fb5d2fa8a788770e946241ea2f62f4b794c0fdda3ccbde49384689911aeba" } diff --git a/packages/oneclient_db/.sqlx/query-b153f762849649456dd97b6a5cd35fb7dafc3d5539441328e03442cea195483a.json b/packages/oneclient_db/.sqlx/query-33771b7003e64c8f19d15d663de773afa568d626c60a37bbc32cc932746ad49b.json similarity index 75% rename from packages/oneclient_db/.sqlx/query-b153f762849649456dd97b6a5cd35fb7dafc3d5539441328e03442cea195483a.json rename to packages/oneclient_db/.sqlx/query-33771b7003e64c8f19d15d663de773afa568d626c60a37bbc32cc932746ad49b.json index 7551bc03..caaad113 100644 --- a/packages/oneclient_db/.sqlx/query-b153f762849649456dd97b6a5cd35fb7dafc3d5539441328e03442cea195483a.json +++ b/packages/oneclient_db/.sqlx/query-33771b7003e64c8f19d15d663de773afa568d626c60a37bbc32cc932746ad49b.json @@ -1,6 +1,6 @@ { "db_name": "SQLite", - "query": "\n\t\tSELECT absolute_path, major, version, vendor, os_arch\n\t\tFROM java_versions\n\t\tWHERE major = ?\n\t\tORDER BY version DESC\n\t\tLIMIT 1\n\t\t", + "query": "\n\t\tSELECT absolute_path, major, version, vendor, os_arch, is_jdk as \"is_jdk: bool\"\n\t\tFROM java_versions\n\t\tWHERE major = ?\n\t\tORDER BY version DESC\n\t\tLIMIT 1\n\t\t", "describe": { "columns": [ { @@ -57,6 +57,17 @@ "name": "os_arch" } } + }, + { + "name": "is_jdk: bool", + "ordinal": 5, + "type_info": "Integer", + "origin": { + "Table": { + "table": "java_versions", + "name": "is_jdk" + } + } } ], "parameters": { @@ -67,8 +78,9 @@ false, false, false, + false, false ] }, - "hash": "b153f762849649456dd97b6a5cd35fb7dafc3d5539441328e03442cea195483a" + "hash": "33771b7003e64c8f19d15d663de773afa568d626c60a37bbc32cc932746ad49b" } diff --git a/packages/oneclient_db/.sqlx/query-dc93cdd57e23e48f8e3c333db4ec396b0ee319adec619e5a7d147805573ae2b2.json b/packages/oneclient_db/.sqlx/query-ab6dcea84a9502d5f8396a207ae613a4c41eb729ca22249f15eb45e6c6030094.json similarity index 62% rename from packages/oneclient_db/.sqlx/query-dc93cdd57e23e48f8e3c333db4ec396b0ee319adec619e5a7d147805573ae2b2.json rename to packages/oneclient_db/.sqlx/query-ab6dcea84a9502d5f8396a207ae613a4c41eb729ca22249f15eb45e6c6030094.json index 2fe3f6e0..e564fe71 100644 --- a/packages/oneclient_db/.sqlx/query-dc93cdd57e23e48f8e3c333db4ec396b0ee319adec619e5a7d147805573ae2b2.json +++ b/packages/oneclient_db/.sqlx/query-ab6dcea84a9502d5f8396a207ae613a4c41eb729ca22249f15eb45e6c6030094.json @@ -1,6 +1,6 @@ { "db_name": "SQLite", - "query": "\n SELECT 1 FROM cluster_artifacts\n WHERE cluster_id = ? AND bundle_name = ? AND package_id = ?\n LIMIT 1\n ", + "query": "\n SELECT 1 FROM cluster_artifacts\n WHERE cluster_id = ? AND bundle_name IS NOT NULL AND package_id = ?\n LIMIT 1\n ", "describe": { "columns": [ { @@ -11,11 +11,11 @@ } ], "parameters": { - "Right": 3 + "Right": 2 }, "nullable": [ false ] }, - "hash": "dc93cdd57e23e48f8e3c333db4ec396b0ee319adec619e5a7d147805573ae2b2" + "hash": "ab6dcea84a9502d5f8396a207ae613a4c41eb729ca22249f15eb45e6c6030094" } diff --git a/packages/oneclient_db/.sqlx/query-c570e309195b6063cf0fbee866d68972de05107769dab983524fd2e74a7ae31a.json b/packages/oneclient_db/.sqlx/query-f5992c9cec6a6d7015cda4c04b463bd5fd9c42ac38b434fa8a0c0fe4f7517cf2.json similarity index 76% rename from packages/oneclient_db/.sqlx/query-c570e309195b6063cf0fbee866d68972de05107769dab983524fd2e74a7ae31a.json rename to packages/oneclient_db/.sqlx/query-f5992c9cec6a6d7015cda4c04b463bd5fd9c42ac38b434fa8a0c0fe4f7517cf2.json index 5b597ea9..27530fbf 100644 --- a/packages/oneclient_db/.sqlx/query-c570e309195b6063cf0fbee866d68972de05107769dab983524fd2e74a7ae31a.json +++ b/packages/oneclient_db/.sqlx/query-f5992c9cec6a6d7015cda4c04b463bd5fd9c42ac38b434fa8a0c0fe4f7517cf2.json @@ -1,6 +1,6 @@ { "db_name": "SQLite", - "query": "\n SELECT absolute_path, major, version, vendor, os_arch FROM java_versions \n WHERE absolute_path = ?\n ", + "query": "\n SELECT absolute_path, major, version, vendor, os_arch, is_jdk as \"is_jdk: bool\"\n FROM java_versions\n WHERE absolute_path = ?\n ", "describe": { "columns": [ { @@ -57,6 +57,17 @@ "name": "os_arch" } } + }, + { + "name": "is_jdk: bool", + "ordinal": 5, + "type_info": "Integer", + "origin": { + "Table": { + "table": "java_versions", + "name": "is_jdk" + } + } } ], "parameters": { @@ -67,8 +78,9 @@ false, false, false, + false, false ] }, - "hash": "c570e309195b6063cf0fbee866d68972de05107769dab983524fd2e74a7ae31a" + "hash": "f5992c9cec6a6d7015cda4c04b463bd5fd9c42ac38b434fa8a0c0fe4f7517cf2" } diff --git a/packages/oneclient_db/migrations/20260806120000_repair_orphaned_bundle_disables.sql b/packages/oneclient_db/migrations/20260806120000_repair_orphaned_bundle_disables.sql new file mode 100644 index 00000000..59bc810d --- /dev/null +++ b/packages/oneclient_db/migrations/20260806120000_repair_orphaned_bundle_disables.sql @@ -0,0 +1,23 @@ +-- Switches bundle content back on where it was left off with nothing recording +-- the choice. +-- +-- `cluster_artifacts.enabled` and `cluster_bundle_overrides` are two records of +-- one decision and they were able to come apart: duplicate resolution wrote the +-- flag without the override, and re-resolving a package to a different bundle +-- deleted the override without the flag. Either way the artifact ends up off +-- with no override, which nothing could then repair -- the update flow decides +-- from the override and the manifest default, never from `enabled`, so it read +-- the package as installed and enabled and never touched it again. For a +-- bundle's hidden dependencies that meant a library the pack needs never +-- reaching `mods/`. +-- +-- A genuine user disable always leaves a `disabled` or `removed` row for the +-- same bundle, so those are matched here and stay off. +UPDATE cluster_artifacts SET enabled = 1 +WHERE bundle_name IS NOT NULL + AND package_id IS NOT NULL + AND enabled = 0 + AND NOT EXISTS (SELECT 1 FROM cluster_bundle_overrides o + WHERE o.cluster_id = cluster_artifacts.cluster_id + AND o.bundle_name = cluster_artifacts.bundle_name + AND o.package_id = cluster_artifacts.package_id); diff --git a/packages/oneclient_db/src/dao/cluster_bundle.rs b/packages/oneclient_db/src/dao/cluster_bundle.rs index 5c8d84a7..f10dbe06 100644 --- a/packages/oneclient_db/src/dao/cluster_bundle.rs +++ b/packages/oneclient_db/src/dao/cluster_bundle.rs @@ -96,20 +96,18 @@ pub async fn get_bundle_tracked( .await } -pub async fn has_bundle_mapping( +pub async fn has_bundle_mapping_for_package( pool: &SqlitePool, cluster_id: i64, - bundle_name: &str, package_id: &str, ) -> Result { let row = sqlx::query_scalar!( r#" SELECT 1 FROM cluster_artifacts - WHERE cluster_id = ? AND bundle_name = ? AND package_id = ? + WHERE cluster_id = ? AND bundle_name IS NOT NULL AND package_id = ? LIMIT 1 "#, cluster_id, - bundle_name, package_id ) .fetch_optional(pool)