fix(core): disabled packages don't enable themselves after updating

packages/oneclient_content/src/bundles/updates.rs

@@ -295,6 +295,15 @@ async fn check_bundle_updates_inner(
         )
     });
 
+    let addition_eligible_bundles = addition_eligible_bundles(
+        ctx,
+        &bundle_packages,
+        &all_linked,
+        &candidate_keys_by_bundle,
+        overrides,
+    )
+    .await?;
+
     let mut additions_available = Vec::new();
     let mut planned_addition_keys = all_installed_managed_keys.clone();
     for hash in all_installed_external_hashes {
@@ -302,7 +311,7 @@ async fn check_bundle_updates_inner(
     }
 
     for archive in &archives {
-        if !subscribed_bundles.contains(&archive.manifest.name) {
+        if !addition_eligible_bundles.contains(&archive.manifest.name) {
             continue;
         }
 
@@ -771,10 +780,55 @@ fn bundle_package_key(
     }
 }
 
+/// Bundles that may still take on files the catalog has newly added.
+///
+/// Subscription is deliberately looser than this: a bundle whose mods are all
+/// installed-but-disabled still wants version updates and still owns its files.
+/// New content is different. Removing or disabling everything a bundle ships is
+/// how a user opts out of that bundle, and an absent per-file override is not
+/// consent for a file that did not exist when they opted out — so gating
+/// additions on subscription alone let one catalog addition quietly reinstate a
+/// bundle the user had emptied.
+///
+/// A bundle qualifies while any of it is still live here: a tracked artifact
+/// that is enabled, an enabled artifact that can only have come from this bundle
+/// (the untracked/migrated case), or an explicit opt-in override.
 #[tracing::instrument(level = "debug", skip_all)]
-async fn installed_bundle_keys(
+async fn addition_eligible_bundles(
     ctx: &ContentCtx,
-    linked: &[LinkedArtifactInfo],
+    bundle_packages: &[BundleTrackedArtifactRow],
+    all_linked: &[LinkedArtifactInfo],
+    candidate_keys_by_bundle: &HashMap<String, HashSet<String>>,
+    overrides: &[ClusterBundleOverrideRow],
+) -> ContentResult<HashSet<String>> {
+    let (live_managed_keys, _) =
+        installed_bundle_keys(ctx, all_linked.iter().filter(|item| item.enabled)).await?;
+
+    let mut eligible: HashSet<String> = bundle_packages
+        .iter()
+        .filter(|bp| bp.enabled != 0)
+        .filter_map(|bp| bp.bundle_name.clone())
+        .collect();
+
+    eligible.extend(infer_bundle_names_from_unique_installed_keys(
+        candidate_keys_by_bundle,
+        &live_managed_keys,
+    ));
+
+    eligible.extend(
+        overrides
+            .iter()
+            .filter(|o| OverrideType::parse(&o.override_type) == Some(OverrideType::Enabled))
+            .map(|o| o.bundle_name.clone()),
+    );
+
+    Ok(eligible)
+}
+
+#[tracing::instrument(level = "debug", skip_all)]
+async fn installed_bundle_keys<'a>(
+    ctx: &ContentCtx,
+    linked: impl IntoIterator<Item = &'a LinkedArtifactInfo>,
 ) -> ContentResult<(HashSet<String>, HashSet<String>)> {
     let mut managed = HashSet::new();
     let mut external = HashSet::new();

packages/oneclient_core/tests/bundle_updates.rs

@@ -26,6 +26,23 @@ fn managed_file(enabled: bool) -> BundleFile {
     }
 }
 
+/// A file the catalog added after the cluster was last synced: nothing on the
+/// cluster tracks it, so it has no override either.
+fn newly_shipped_file() -> BundleFile {
+    BundleFile {
+        enabled: true,
+        hidden: false,
+        path: "mods/newcomer.jar".to_string(),
+        size: 1,
+        kind: BundleFileKind::Managed {
+            provider: ProviderId::Modrinth,
+            project_id: "newcomer".to_string(),
+            version_id: "v1".to_string(),
+            sha1: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
+        },
+    }
+}
+
 fn manifest(files: Vec<BundleFile>) -> BundleManifest {
     BundleManifest {
         name: BUNDLE.to_string(),
@@ -196,3 +213,150 @@ async fn user_disabled_mod_is_not_treated_as_a_removal() {
         check.removals_available
     );
 }
+
+#[tokio::test]
+async fn live_bundle_takes_on_new_catalog_files() {
+    let state = oneclient_core::dev::ephemeral_state().await.unwrap();
+    oneclient_core::dev::seed_bundle_archive(
+        &state,
+        manifest(vec![managed_file(true), newly_shipped_file()]),
+    )
+    .await
+    .unwrap();
+    let cluster_id = cluster_with_tracked_mod(&state).await;
+
+    let check = check_bundle_updates(cluster_id, state.bundles.as_ref(), &state.services.content())
+        .await
+        .unwrap();
+
+    assert_eq!(
+        check.additions_available.len(),
+        1,
+        "a bundle the user still has enabled should pick up newly shipped files"
+    );
+    assert_eq!(
+        check.additions_available[0].new_file.kind.package_id(),
+        "newcomer"
+    );
+}
+
+#[tokio::test]
+async fn emptied_bundle_does_not_take_on_new_catalog_files() {
+    let state = oneclient_core::dev::ephemeral_state().await.unwrap();
+    oneclient_core::dev::seed_bundle_archive(
+        &state,
+        manifest(vec![managed_file(true), newly_shipped_file()]),
+    )
+    .await
+    .unwrap();
+    let cluster_id = cluster_with_tracked_mod(&state).await;
+
+    // The user disabled everything this bundle shipped, which is how you opt out
+    // of it. The newcomer has no override of its own precisely because it did not
+    // exist yet, and that absence must not read as consent.
+    artifact_dao::update_cluster_artifact(&state.services.db, cluster_id, HASH, "sodium.jar", 0)
+        .await
+        .unwrap();
+    bundle_dao::save_override(
+        &state.services.db,
+        cluster_id,
+        BUNDLE,
+        PROJECT_ID,
+        OverrideType::Disabled,
+    )
+    .await
+    .unwrap();
+
+    let check = check_bundle_updates(cluster_id, state.bundles.as_ref(), &state.services.content())
+        .await
+        .unwrap();
+
+    assert!(
+        check.additions_available.is_empty(),
+        "a bundle whose content the user disabled must not reinstate itself: {:?}",
+        check
+            .additions_available
+            .iter()
+            .map(|a| a.new_file.kind.package_id())
+            .collect::<Vec<_>>()
+    );
+}
+
+#[tokio::test]
+async fn removed_bundle_content_does_not_take_on_new_catalog_files() {
+    let state = oneclient_core::dev::ephemeral_state().await.unwrap();
+    oneclient_core::dev::seed_bundle_archive(
+        &state,
+        manifest(vec![managed_file(true), newly_shipped_file()]),
+    )
+    .await
+    .unwrap();
+    let cluster_id = cluster_with_tracked_mod(&state).await;
+
+    oneclient_content::bundles::remove_artifact_from_cluster(
+        cluster_id,
+        HASH,
+        true,
+        &state.services.content(),
+    )
+    .await
+    .unwrap();
+
+    let check = check_bundle_updates(cluster_id, state.bundles.as_ref(), &state.services.content())
+        .await
+        .unwrap();
+
+    assert!(
+        check.additions_available.is_empty(),
+        "a bundle the user emptied by removal must not reinstate itself: {:?}",
+        check
+            .additions_available
+            .iter()
+            .map(|a| a.new_file.kind.package_id())
+            .collect::<Vec<_>>()
+    );
+}
+
+#[tokio::test]
+async fn opting_a_single_file_in_keeps_the_bundle_live() {
+    let state = oneclient_core::dev::ephemeral_state().await.unwrap();
+    oneclient_core::dev::seed_bundle_archive(
+        &state,
+        manifest(vec![managed_file(true), newly_shipped_file()]),
+    )
+    .await
+    .unwrap();
+    let cluster_id = cluster_with_tracked_mod(&state).await;
+
+    artifact_dao::update_cluster_artifact(&state.services.db, cluster_id, HASH, "sodium.jar", 0)
+        .await
+        .unwrap();
+    bundle_dao::save_override(
+        &state.services.db,
+        cluster_id,
+        BUNDLE,
+        PROJECT_ID,
+        OverrideType::Disabled,
+    )
+    .await
+    .unwrap();
+    bundle_dao::save_override(
+        &state.services.db,
+        cluster_id,
+        BUNDLE,
+        "newcomer",
+        OverrideType::Enabled,
+    )
+    .await
+    .unwrap();
+
+    let check = check_bundle_updates(cluster_id, state.bundles.as_ref(), &state.services.content())
+        .await
+        .unwrap();
+
+    assert_eq!(
+        check.additions_available.len(),
+        1,
+        "an explicit opt-in is consent, even with the rest of the bundle disabled"
+    );
+}