diff --git a/.sqlx/query-af0567c14b689a3f53207179ba9ee2e369b99542369fa5401aa4057d0b326b86.json b/.sqlx/query-af0567c14b689a3f53207179ba9ee2e369b99542369fa5401aa4057d0b326b86.json new file mode 100644 index 00000000..32177647 --- /dev/null +++ b/.sqlx/query-af0567c14b689a3f53207179ba9ee2e369b99542369fa5401aa4057d0b326b86.json @@ -0,0 +1,12 @@ +{ + "db_name": "SQLite", + "query": "\n DELETE FROM cluster_bundle_overrides\n WHERE cluster_id = ? AND package_id = ? AND override_type = 'disabled'\n ", + "describe": { + "columns": [], + "parameters": { + "Right": 2 + }, + "nullable": [] + }, + "hash": "af0567c14b689a3f53207179ba9ee2e369b99542369fa5401aa4057d0b326b86" +} diff --git a/packages/oneclient_app/src/hooks/actions.rs b/packages/oneclient_app/src/hooks/actions.rs index 7b2b9ace..d27e8d98 100644 --- a/packages/oneclient_app/src/hooks/actions.rs +++ b/packages/oneclient_app/src/hooks/actions.rs @@ -1083,7 +1083,7 @@ impl Actions { if install.result.is_ok() { // Before the refresh below so the version list never renders // the moment where both copies read as active - if let Err(err) = oneclient_content::bundles::reconcile_duplicate_activity( + if let Err(err) = oneclient_content::packages::reconcile_duplicate_activity( cluster_id, &state.services.content(), ) diff --git a/packages/oneclient_content/src/bundles/install.rs b/packages/oneclient_content/src/bundles/install.rs index f126d790..9ebe909c 100644 --- a/packages/oneclient_content/src/bundles/install.rs +++ b/packages/oneclient_content/src/bundles/install.rs @@ -1,4 +1,5 @@ use futures_util::StreamExt; +use oneclient_db::dao::applied_migration as migration_dao; use oneclient_db::dao::artifact as artifact_dao; use oneclient_db::dao::cluster as cluster_dao; use oneclient_db::dao::cluster_bundle as bundle_dao; @@ -202,12 +203,54 @@ pub(crate) fn disable_was_deliberate(suppression: Option) -> bool } } +const DUPLICATE_DISABLE_REPAIR: &str = "repair-hidden-duplicate-disables"; + +async fn clear_reconciler_disables( + cluster_id: i64, + archives: &[BundleArchive], + ctx: &ContentCtx, +) -> ContentResult<()> { + let id = format!("{DUPLICATE_DISABLE_REPAIR}:{cluster_id}"); + if migration_dao::is_applied(&ctx.db, &id).await? { + return Ok(()); + } + + let hidden: std::collections::HashSet = archives + .iter() + .flat_map(|archive| &archive.manifest.files) + .filter(|file| file.hidden) + .map(|file| file.kind.package_id()) + .collect(); + + if hidden.is_empty() { + return Ok(()); + } + + let mut cleared = 0u64; + for package_id in &hidden { + cleared += bundle_dao::clear_disabled_overrides(&ctx.db, cluster_id, package_id).await?; + } + + if cleared > 0 { + tracing::info!( + cluster_id, + cleared, + "cleared disabled overrides on hidden bundle files that no user was ever shown" + ); + } + + migration_dao::mark_applied(&ctx.db, &id).await?; + Ok(()) +} + #[tracing::instrument(level = "debug", skip(archives, ctx))] pub async fn heal_bundle_activity( cluster_id: i64, archives: &[BundleArchive], ctx: &ContentCtx, ) -> ContentResult<()> { + clear_reconciler_disables(cluster_id, archives, ctx).await?; + let tracked = bundle_dao::list_bundle_tracked(&ctx.db, cluster_id).await?; if tracked.iter().all(|row| row.enabled != 0) { return Ok(()); @@ -226,10 +269,20 @@ pub async fn heal_bundle_activity( let overrides = bundle_dao::list_overrides(&ctx.db, cluster_id).await?; + let mut live_packages: std::collections::HashSet = tracked + .iter() + .filter(|row| row.enabled != 0) + .filter_map(|row| row.package_id.clone()) + .collect(); + 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; }; + + if live_packages.contains(package_id) { + continue; + } let Some(is_hidden) = hidden .get(&(bundle_name.as_str(), package_id.clone())) .copied() @@ -261,6 +314,8 @@ pub async fn heal_bundle_activity( tracing::warn!(hash = %row.hash, error = %err, "failed to re-enable bundle content"); continue; } + + live_packages.insert(package_id.clone()); } Ok(()) @@ -584,21 +639,6 @@ pub async fn set_artifact_enabled_to( Ok(live) } -/// Writes the override alongside the flag -/// without it the losing bundle copy looks disabled-by-nobody and -/// heal_bundle_activity re-enables it every launch -#[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(()) -} - // which clusters have to record what the user just did async fn override_scope( cluster_id: i64, diff --git a/packages/oneclient_content/src/bundles/mod.rs b/packages/oneclient_content/src/bundles/mod.rs index 5270378c..965411f1 100644 --- a/packages/oneclient_content/src/bundles/mod.rs +++ b/packages/oneclient_content/src/bundles/mod.rs @@ -16,7 +16,6 @@ pub use install::{ 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, - reconcile_duplicate_activity, remove_artifact_from_cluster, set_artifact_enabled_to, set_bundle_package_enabled, set_bundle_package_opt_in, set_bundle_package_override, set_bundle_package_overrides, diff --git a/packages/oneclient_content/src/packages/dependencies.rs b/packages/oneclient_content/src/packages/dependencies.rs index 3d359c6a..44bd8c3d 100644 --- a/packages/oneclient_content/src/packages/dependencies.rs +++ b/packages/oneclient_content/src/packages/dependencies.rs @@ -193,15 +193,13 @@ fn choose_version( .cloned() } -/// Mirrors the store's install-time compatibility check -/// must stay in sync with it +#[must_use] +pub(crate) fn supports_game_version(game_versions: &[String], mc_version: &str) -> bool { + game_versions.is_empty() || game_versions.iter().any(|v| v == mc_version) +} + fn fits_cluster(version: &VersionSummary, cluster: &ClusterRow, loader: GameLoader) -> bool { - if !version.game_versions.is_empty() - && !version - .game_versions - .iter() - .any(|v| cluster.mc_version.contains(v)) - { + if !supports_game_version(&version.game_versions, &cluster.mc_version) { return false; } diff --git a/packages/oneclient_content/src/packages/store/link.rs b/packages/oneclient_content/src/packages/store/link.rs index 566bc63c..45f576d3 100644 --- a/packages/oneclient_content/src/packages/store/link.rs +++ b/packages/oneclient_content/src/packages/store/link.rs @@ -87,20 +87,21 @@ pub async fn remove_entry(path: &Path) -> ContentResult<()> { Ok(()) } -fn materialized_root( +async fn materialized_root( cluster: &ClusterRow, content_type: ContentType, -) -> Option<(std::path::PathBuf, &'static str)> { +) -> Option<(PathBuf, &'static str)> { if content_type.is_global() { return paths::shared_minecraft_dir() .ok() .map(|dir| (dir, manifest::GLOBAL_MANIFEST_NAME)); } - if content_type == ContentType::Mod { - return paths::cluster_dir(&cluster.folder_name) - .ok() - .map(|dir| (dir, manifest::MODS_MANIFEST_NAME)); + if content_type == ContentType::Mod + && let Ok(dir) = paths::cluster_dir(&cluster.folder_name) + && manifest::mods_live_in_cluster(&dir).await + { + return Some((dir, manifest::MODS_MANIFEST_NAME)); } paths::cluster_game_dir(&cluster.folder_name) @@ -114,7 +115,7 @@ pub async fn try_unlink_materialized( content_type: ContentType, file_name: &str, ) -> bool { - let Some((root, manifest_name)) = materialized_root(cluster, content_type) else { + let Some((root, manifest_name)) = materialized_root(cluster, content_type).await else { return false; }; @@ -165,7 +166,7 @@ pub async fn try_link_materialized( return LiveSync::Skipped; } - let Some((root, manifest_name)) = materialized_root(cluster, content_type) else { + let Some((root, manifest_name)) = materialized_root(cluster, content_type).await else { return LiveSync::Deferred; }; diff --git a/packages/oneclient_content/src/packages/store/mod.rs b/packages/oneclient_content/src/packages/store/mod.rs index 22c27c51..5d6ee4d2 100644 --- a/packages/oneclient_content/src/packages/store/mod.rs +++ b/packages/oneclient_content/src/packages/store/mod.rs @@ -629,12 +629,10 @@ fn ensure_compatible( } } - if !version.game_versions.is_empty() - && !version - .game_versions - .iter() - .any(|v| cluster.mc_version.contains(v)) - { + if !crate::packages::dependencies::supports_game_version( + &version.game_versions, + &cluster.mc_version, + ) { return Err(PackageError::IncompatibleMcVersion.into()); } diff --git a/packages/oneclient_content/src/packages/updates.rs b/packages/oneclient_content/src/packages/updates.rs index 90b229b1..699a474c 100644 --- a/packages/oneclient_content/src/packages/updates.rs +++ b/packages/oneclient_content/src/packages/updates.rs @@ -91,10 +91,17 @@ struct Candidate { fn browser_installed( linked: &[LinkedArtifactInfo], bundle_hashes: &HashSet, + bundle_projects: &HashSet, ) -> Vec { linked .iter() + .filter(|info| info.enabled) .filter(|info| !bundle_hashes.contains(&info.hash)) + .filter(|info| { + info.project_id + .as_ref() + .is_none_or(|id| !bundle_projects.contains(id)) + }) .filter_map(|info| { let provider = info.provider?; if provider == ProviderId::Local { @@ -126,13 +133,12 @@ pub async fn check_browser_package_updates( let cluster = PackageStore::get_cluster(cluster_id, ctx).await?; let linked = PackageStore::list_linked_artifacts(cluster_id, ctx).await?; - let bundle_hashes: HashSet = bundle_dao::list_bundle_tracked(&ctx.db, cluster_id) - .await? - .into_iter() - .map(|row| row.hash) - .collect(); + let tracked = bundle_dao::list_bundle_tracked(&ctx.db, cluster_id).await?; + let bundle_projects: HashSet = + tracked.iter().filter_map(|row| row.package_id.clone()).collect(); + let bundle_hashes: HashSet = tracked.into_iter().map(|row| row.hash).collect(); - let candidates = browser_installed(&linked, &bundle_hashes); + let candidates = browser_installed(&linked, &bundle_hashes, &bundle_projects); if candidates.is_empty() { return Ok(BrowserUpdateCheck { cluster_id, @@ -506,7 +512,7 @@ mod tests { let bundles: HashSet = ["a".to_string()].into_iter().collect(); assert!( - browser_installed(&linked, &bundles).is_empty(), + browser_installed(&linked, &bundles, &HashSet::new()).is_empty(), "a bundle-tracked artifact must stay out of the browser update flow" ); } @@ -520,7 +526,7 @@ mod tests { Some("v1"), )]; - let found = browser_installed(&linked, &HashSet::new()); + let found = browser_installed(&linked, &HashSet::new(), &HashSet::new()); assert_eq!(found.len(), 1); assert_eq!(found[0].project_id, "sodium"); assert_eq!(found[0].version_id, "v1"); @@ -534,7 +540,7 @@ mod tests { ]; assert!( - browser_installed(&linked, &HashSet::new()).is_empty(), + browser_installed(&linked, &HashSet::new(), &HashSet::new()).is_empty(), "a file with no provider release has no version to compare" ); } @@ -544,7 +550,7 @@ mod tests { let linked = vec![linked("a", Some(ProviderId::Modrinth), Some("sodium"), None)]; assert!( - browser_installed(&linked, &HashSet::new()).is_empty(), + browser_installed(&linked, &HashSet::new(), &HashSet::new()).is_empty(), "without an installed version id there is nothing to compare against" ); } @@ -612,16 +618,15 @@ mod tests { let mut info = linked("a", Some(ProviderId::Modrinth), Some("sodium"), Some("v1")); info.published_at = Some("2026-02-01T00:00:00Z".into()); - let found = browser_installed(&[info], &HashSet::new()); + let found = browser_installed(&[info], &HashSet::new(), &HashSet::new()); assert_eq!(found[0].published_at, Some(at("2026-02-01T00:00:00Z"))); } - #[test] fn an_unparseable_publish_date_leaves_the_candidate_undated() { let mut info = linked("a", Some(ProviderId::Modrinth), Some("sodium"), Some("v1")); info.published_at = Some("not a date".into()); - let found = browser_installed(&[info], &HashSet::new()); + let found = browser_installed(&[info], &HashSet::new(), &HashSet::new()); assert_eq!(found[0].published_at, None); } } diff --git a/packages/oneclient_core/src/game/shared_dir.rs b/packages/oneclient_core/src/game/shared_dir.rs index 398c2f40..f55d1f9a 100644 --- a/packages/oneclient_core/src/game/shared_dir.rs +++ b/packages/oneclient_core/src/game/shared_dir.rs @@ -97,7 +97,7 @@ pub async fn materialize_content( let ours = ours_in_folder(ContentType::Mod, &linked, previous.as_ref()); tracing::info!(cluster_id = cluster.id, "moving mods out of the shared game directory"); - stash_content_files(&from, &into, &ours).await; + stash_content_files(&from, &into, ContentType::Mod, &ours).await; } if mods_in_cluster { @@ -136,7 +136,7 @@ pub async fn materialize_content( import_manual_content_with(services, cluster, game_dir, mods_in_cluster).await; - if let Err(err) = oneclient_content::bundles::reconcile_duplicate_activity( + if let Err(err) = oneclient_content::packages::reconcile_duplicate_activity( cluster.id, &services.content(), ) @@ -184,7 +184,7 @@ pub async fn materialize_content( polyio::create_dir_all(&dir).await.ok(); let ours = ours_in_folder(*content_type, &linked, previous.as_ref()); - stash_content_files(&dir, &stash, &ours).await; + stash_content_files(&dir, &stash, *content_type, &ours).await; ensure_note(&dir, *content_type).await; restore_stashed(&stash, &dir, *content_type, &ours).await; } @@ -556,7 +556,7 @@ pub async fn dematerialize_content( polyio::create_dir_all(&dir).await.ok(); let ours = ours_in_folder(*content_type, &linked, current.as_ref()); - stash_content_files(&dir, &stash, &ours).await; + stash_content_files(&dir, &stash, *content_type, &ours).await; sweep_staging_files(&dir).await; ensure_note(&dir, *content_type).await; } @@ -1035,7 +1035,12 @@ async fn ensure_links_note(dir: &Path) { .ok(); } -async fn stash_content_files(dir: &Path, stash: &Path, ours: &HashSet) { +async fn stash_content_files( + dir: &Path, + stash: &Path, + content_type: ContentType, + ours: &HashSet, +) { let Ok(mut entries) = polyio::read_dir(dir).await else { return; }; @@ -1067,6 +1072,15 @@ async fn stash_content_files(dir: &Path, stash: &Path, ours: &HashSet) { continue; } + if file_type.is_file() && has_content_extension(content_type, &name) { + tracing::debug!( + file = %name, + dir = %dir.display(), + "leaving unrecognised content where it is rather than stashing it" + ); + continue; + } + let dest = stash.join(&name); if let Err(err) = move_entry(&path, &dest).await { tracing::warn!( @@ -1365,7 +1379,6 @@ mod tests { assert_eq!(body.lines().count(), 1); } - use super::*; fn names(names: &[&str]) -> HashSet { @@ -1403,7 +1416,7 @@ mod tests { .await .unwrap(); - stash_content_files(&shared, &stash, &ours).await; + stash_content_files(&shared, &stash, ContentType::Shader, &ours).await; assert!(!shared.join("bsl.zip").exists(), "managed pack left behind"); assert!(!shared.join("bsl.zip.txt").exists(), "sidecar left behind"); assert_eq!( @@ -1438,7 +1451,7 @@ mod tests { .await .unwrap(); - stash_content_files(&shared, &stash, &HashSet::new()).await; + stash_content_files(&shared, &stash, ContentType::Shader, &HashSet::new()).await; assert!(!shared.join("Loose").exists()); assert!(stash.join("Loose/shaders/final.fsh").exists()); @@ -1446,7 +1459,7 @@ mod tests { assert!(shared.join("Loose/shaders/final.fsh").exists()); // Only the link goes never the stashed original - stash_content_files(&shared, &stash, &HashSet::new()).await; + stash_content_files(&shared, &stash, ContentType::Shader, &HashSet::new()).await; assert!(!shared.join("Loose").exists()); assert!(stash.join("Loose/shaders/final.fsh").exists()); @@ -1463,7 +1476,7 @@ mod tests { .await .unwrap(); - stash_content_files(&shared, &stash, &HashSet::new()).await; + stash_content_files(&shared, &stash, ContentType::Mod, &HashSet::new()).await; assert!(shared.join(EMPTY_NOTE_NAME).exists()); assert!(!stash.join(EMPTY_NOTE_NAME).exists()); @@ -1565,7 +1578,6 @@ mod tests { std::fs::remove_dir_all(root.path()).ok(); } - /// Pinned because the halves are easy to swap `stash` reads the game dir /// and writes the cluster `restore` the reverse and both take two /// same-typed `&Path`s a swap would quietly delete a user's files @@ -1596,9 +1608,9 @@ mod tests { let before = dir_entries(&stash).await; - stash_content_files(&shared, &stash, &ours).await; + stash_content_files(&shared, &stash, ContentType::Mod, &ours).await; restore_stashed(&stash, &shared, ContentType::Mod, &ours).await; - stash_content_files(&shared, &stash, &ours).await; + stash_content_files(&shared, &stash, ContentType::Mod, &ours).await; restore_stashed(&stash, &shared, ContentType::Mod, &ours).await; let after = dir_entries(&stash).await; diff --git a/packages/oneclient_db/migrations/20260914120000_repair_duplicate_disable_overrides.sql b/packages/oneclient_db/migrations/20260914120000_repair_duplicate_disable_overrides.sql new file mode 100644 index 00000000..50606a02 --- /dev/null +++ b/packages/oneclient_db/migrations/20260914120000_repair_duplicate_disable_overrides.sql @@ -0,0 +1,20 @@ +-- Repairs the overrides duplicate resolution filed against the user. +-- +-- `reconcile_duplicate_activity` switched the older copy of a package off and +-- recorded it as a user disable, but `cluster_bundle_overrides` is keyed by +-- package id and the surviving copy shares it, so the row spoke for a copy +-- nobody turned off. Every lookup that resolves a user's choice then answers +-- "off" for the package: the next bundle update installs the new copy and +-- immediately disables it, and the update check stops shipping the file at all. +-- +-- A row alongside a copy that is still on cannot be a considered choice about +-- the package -- had the user turned the package off, nothing of it would be +-- left enabled. Only those go. A package whose copies are all off is +-- indistinguishable from a genuine disable here and is left for the runtime +-- pass, which has the manifest and can tell whether the file was ever shown. +DELETE FROM cluster_bundle_overrides +WHERE override_type = 'disabled' + AND EXISTS (SELECT 1 FROM cluster_artifacts a + WHERE a.cluster_id = cluster_bundle_overrides.cluster_id + AND a.package_id = cluster_bundle_overrides.package_id + AND a.enabled = 1); diff --git a/packages/oneclient_db/src/dao/cluster_bundle.rs b/packages/oneclient_db/src/dao/cluster_bundle.rs index 96b44f45..615d3aab 100644 --- a/packages/oneclient_db/src/dao/cluster_bundle.rs +++ b/packages/oneclient_db/src/dao/cluster_bundle.rs @@ -189,6 +189,25 @@ pub async fn remove_override( Ok(()) } +pub async fn clear_disabled_overrides( + pool: &SqlitePool, + cluster_id: i64, + package_id: &str, +) -> Result { + let result = sqlx::query!( + r#" + DELETE FROM cluster_bundle_overrides + WHERE cluster_id = ? AND package_id = ? AND override_type = 'disabled' + "#, + cluster_id, + package_id + ) + .execute(pool) + .await?; + + Ok(result.rows_affected()) +} + /// Opt-in (`enabled`) rows are left alone they say the opposite thing pub async fn clear_suppressing_overrides( pool: &SqlitePool,