feat: delete mods from dead bundles (#746)

This commit is contained in:
Julian Chang 2026-09-01 08:03:13 -04:00 • committed by GitHub
parent 606a15905d
commit b25ff12ab5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 213 additions and 33 deletions

View file

@ -16,7 +16,7 @@ inputs:
cache-namespace: cache-namespace:
description: > description: >
Cache namespace. Workflows that compile with different profile settings Cache namespace. Workflows that compile with different profile settings
MUST use different namespaces — cargo fingerprints every dependency by its MUST use different namespaces - cargo fingerprints every dependency by its
profile, so sharing one would mean each workflow evicts artifacts the other profile, so sharing one would mean each workflow evicts artifacts the other
cannot use. cannot use.
required: false required: false

View file

@ -11,7 +11,7 @@ concurrency:
cancel-in-progress: true cancel-in-progress: true
# Nightlies are unsigned test builds, so they do not need the release profile's # Nightlies are unsigned test builds, so they do not need the release profile's
# whole-program optimization — and that optimization is exactly what makes them # whole-program optimization - and that optimization is exactly what makes them
# slow. `lto = "fat"` + `codegen-units = 1` force a single-threaded re-optimization # slow. `lto = "fat"` + `codegen-units = 1` force a single-threaded re-optimization
# of the entire dependency graph at link time, on every run, which no cache can # of the entire dependency graph at link time, on every run, which no cache can
# ever skip. Thin LTO keeps nearly all of the runtime benefit at a fraction of the # ever skip. Thin LTO keeps nearly all of the runtime benefit at a fraction of the
@ -21,7 +21,7 @@ concurrency:
# output directory and every path below is unaffected. The shipped release binary # output directory and every path below is unaffected. The shipped release binary
# is untouched: oneclient_release.yml still builds the fat-LTO profile from # is untouched: oneclient_release.yml still builds the fat-LTO profile from
# Cargo.toml. That difference is why the two workflows must not share a cache # Cargo.toml. That difference is why the two workflows must not share a cache
# namespace — see `cache-namespace` on the Setup Rust step. # namespace - see `cache-namespace` on the Setup Rust step.
env: env:
CARGO_PROFILE_RELEASE_LTO: thin CARGO_PROFILE_RELEASE_LTO: thin
CARGO_PROFILE_RELEASE_CODEGEN_UNITS: '16' CARGO_PROFILE_RELEASE_CODEGEN_UNITS: '16'
@ -213,7 +213,7 @@ jobs:
} }
foreach ($dll in @(Get-ChildItem 'target/crt/*.dll')) { foreach ($dll in @(Get-ChildItem 'target/crt/*.dll')) {
if (-not (Test-Path (Join-Path $dest $dll.Name))) { if (-not (Test-Path (Join-Path $dest $dll.Name))) {
Write-Error "$($dll.Name) did not land beside oneclient_app.exe — cargo-packager put resources elsewhere, so app-local CRT loading will not work" Write-Error "$($dll.Name) did not land beside oneclient_app.exe - cargo-packager put resources elsewhere, so app-local CRT loading will not work"
exit 1 exit 1
} }
} }

View file

@ -18,7 +18,7 @@ on:
# NOTE: CARGO_PACKAGER_SIGN_PRIVATE_KEY* are deliberately NOT declared at # NOTE: CARGO_PACKAGER_SIGN_PRIVATE_KEY* are deliberately NOT declared at
# workflow level. Swatinem/rust-cache folds every `CARGO*`-prefixed environment # workflow level. Swatinem/rust-cache folds every `CARGO*`-prefixed environment
# variable into its cache key, so a workflow-level declaration gave this workflow # variable into its cache key, so a workflow-level declaration gave this workflow
# a cache namespace that the nightly workflow could never populate — every # a cache namespace that the nightly workflow could never populate - every
# release built the whole dependency graph from scratch. They are scoped to the # release built the whole dependency graph from scratch. They are scoped to the
# individual steps that actually need them instead. # individual steps that actually need them instead.
@ -113,7 +113,7 @@ jobs:
# already shipped. A *draft* is a previous failed/partial run of this # already shipped. A *draft* is a previous failed/partial run of this
# same workflow, so reuse it. # same workflow, so reuse it.
if git ls-remote --exit-code --tags origin "refs/tags/$TAG" >/dev/null 2>&1; then if git ls-remote --exit-code --tags origin "refs/tags/$TAG" >/dev/null 2>&1; then
echo "::error::Tag $TAG already exists on origin — OneClient $VERSION has already been released. Re-run with a bump, or bump the version in Cargo.toml." echo "::error::Tag $TAG already exists on origin - OneClient $VERSION has already been released. Re-run with a bump, or bump the version in Cargo.toml."
exit 1 exit 1
fi fi
@ -123,7 +123,7 @@ jobs:
RELEASE_EXISTS=1 RELEASE_EXISTS=1
fi fi
if [ "$RELEASE_EXISTS" = "1" ] && [ "$IS_DRAFT" != "true" ]; then if [ "$RELEASE_EXISTS" = "1" ] && [ "$IS_DRAFT" != "true" ]; then
echo "::error::Release $TAG already exists and is published — refusing to overwrite OneClient $VERSION. Re-run with a bump, or bump the version in Cargo.toml." echo "::error::Release $TAG already exists and is published - refusing to overwrite OneClient $VERSION. Re-run with a bump, or bump the version in Cargo.toml."
exit 1 exit 1
fi fi
@ -189,7 +189,7 @@ jobs:
runs-on: ${{ matrix.platform }} runs-on: ${{ matrix.platform }}
steps: steps:
# Pin to the commit `prepare` resolved — with `bump` that is the version # Pin to the commit `prepare` resolved - with `bump` that is the version
# commit it just pushed, which github.sha would not point at. # commit it just pushed, which github.sha would not point at.
- uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0
with: with:
@ -465,7 +465,7 @@ jobs:
} }
foreach ($dll in @(Get-ChildItem 'target/crt/*.dll')) { foreach ($dll in @(Get-ChildItem 'target/crt/*.dll')) {
if (-not (Test-Path (Join-Path $dest $dll.Name))) { if (-not (Test-Path (Join-Path $dest $dll.Name))) {
Write-Error "$($dll.Name) did not land beside oneclient_app.exe — cargo-packager put resources elsewhere, so app-local CRT loading will not work" Write-Error "$($dll.Name) did not land beside oneclient_app.exe - cargo-packager put resources elsewhere, so app-local CRT loading will not work"
exit 1 exit 1
} }
} }
@ -524,7 +524,7 @@ jobs:
# Identify the single updater artifact for this platform (the file with a # Identify the single updater artifact for this platform (the file with a
# sibling .sig): -setup.exe (nsis), *.app.tar.gz (app), *.AppImage (appimage). # sibling .sig): -setup.exe (nsis), *.app.tar.gz (app), *.AppImage (appimage).
# NOTE: nullglob is on, so an unmatched glob expands to nothing. Use arrays # NOTE: nullglob is on, so an unmatched glob expands to nothing. Use arrays
# rather than `ls <glob> | head` — the latter falls back to listing the cwd # rather than `ls <glob> | head` - the latter falls back to listing the cwd
# when nothing matches, silently picking an unrelated file. # when nothing matches, silently picking an unrelated file.
fmt=""; ext=""; matches=() fmt=""; ext=""; matches=()
case "$OS_KEY" in case "$OS_KEY" in

View file

@ -32,7 +32,7 @@ To find an issue that interests you, you can browse through our [existing issues
#### Making Changes Locally #### Making Changes Locally
This project is a pure Rust [`Cargo`] workspace (edition 2024). Make sure you have a This project is a pure Rust [`Cargo`] workspace (edition 2024). Make sure you have a
recent Rust toolchain (`rustc` **1.85+**) installed before proceeding — the easiest way recent Rust toolchain (`rustc` **1.85+**) installed before proceeding - the easiest way
is via [rustup]. is via [rustup].
To make changes locally, follow these steps: To make changes locally, follow these steps:

View file

@ -767,7 +767,7 @@ impl Component for CorruptionSimulator {
.spacing(10.) .spacing(10.)
.child( .child(
label() label()
.text("Damages the real installation so the repair paths can be exercised. Everything here is repairable by \"Verify Files\" in cluster settings, or by launching — which is the point.") .text("Damages the real installation so the repair paths can be exercised. Everything here is repairable by \"Verify Files\" in cluster settings, or by launching - which is the point.")
.font_size(13.) .font_size(13.)
.color(colors::fg_secondary()), .color(colors::fg_secondary()),
) )

View file

@ -331,7 +331,7 @@ fn offline_dialog(
.child(field_label("UUID")) .child(field_label("UUID"))
.child( .child(
label() label()
.text(uuid_preview.unwrap_or_else(|| "—".to_string())) .text(uuid_preview.unwrap_or_else(|| "-".to_string()))
.font_size(12.) .font_size(12.)
.color(colors::fg_secondary()), .color(colors::fg_secondary()),
), ),

View file

@ -267,7 +267,7 @@ fn largest(entries: &[StorageEntry]) -> Option<u64> {
fn unused_cache_description(entry: &ReclaimableEntry) -> String { fn unused_cache_description(entry: &ReclaimableEntry) -> String {
if entry.is_empty() { if entry.is_empty() {
return "Nothing here — every cached file belongs to an installed package.".to_string(); return "Nothing here - every cached file belongs to an installed package.".to_string();
} }
format!( format!(
@ -280,7 +280,7 @@ fn unused_cache_description(entry: &ReclaimableEntry) -> String {
fn legacy_content_description(entry: &ReclaimableEntry) -> String { fn legacy_content_description(entry: &ReclaimableEntry) -> String {
if entry.is_empty() { if entry.is_empty() {
return "Nothing here — no cluster folder is holding old content.".to_string(); return "Nothing here - no cluster folder is holding old content.".to_string();
} }
format!( format!(

View file

@ -13,7 +13,7 @@ use tracing::Level;
use crate::components::{Button, Icon, IconType, LogViewer, Segment, SegmentedControl, TextInput}; use crate::components::{Button, Icon, IconType, LogViewer, Segment, SegmentedControl, TextInput};
use crate::theme::colors; use crate::theme::colors;
const WINDOW_TITLE: &str = "OneClient — Log Console"; const WINDOW_TITLE: &str = "OneClient - Log Console";
/// Old lines are dropped from the front this window can be left open for a whole session /// Old lines are dropped from the front this window can be left open for a whole session
const MAX_LINES: usize = 5_000; const MAX_LINES: usize = 5_000;

View file

@ -255,7 +255,7 @@ fn build_stats(view: StatsView) -> Option<Element> {
"~{} left", "~{} left",
format_duration_hms(remaining as i64) format_duration_hms(remaining as i64)
)), )),
None => parts.push("~— left".to_string()), None => parts.push("~- left".to_string()),
} }
if let Some(speed) = transfer.map(|t| t.speed_bps).filter(|s| *s >= 1.0) { if let Some(speed) = transfer.map(|t| t.speed_bps).filter(|s| *s >= 1.0) {

View file

@ -306,7 +306,7 @@ fn xerr(code: u64) -> MinecraftAuthError {
fn network_guidance(failure: NetworkFailure) -> AuthErrorGuidance { fn network_guidance(failure: NetworkFailure) -> AuthErrorGuidance {
match failure { match failure {
NetworkFailure::Certificate => AuthErrorGuidance::new( NetworkFailure::Certificate => AuthErrorGuidance::new(
"OneClient reached the Microsoft sign-in service, but could not verify its security certificate. Something on this machine or network is intercepting the encrypted connection — usually antivirus HTTPS scanning, a school or workplace filter, or a VPN.", "OneClient reached the Microsoft sign-in service, but could not verify its security certificate. Something on this machine or network is intercepting the encrypted connection - usually antivirus HTTPS scanning, a school or workplace filter, or a VPN.",
&[ &[
"Turn off HTTPS/SSL scanning in your antivirus (often called \"encrypted connection scanning\", \"web shield\", or \"SSL interception\")", "Turn off HTTPS/SSL scanning in your antivirus (often called \"encrypted connection scanning\", \"web shield\", or \"SSL interception\")",
"Temporarily disable your VPN or proxy and try signing in again", "Temporarily disable your VPN or proxy and try signing in again",

View file

@ -95,6 +95,11 @@ impl BundlesManager {
.map(|entry| entry.remote_path) .map(|entry| entry.remote_path)
.collect(); .collect();
if remote_paths.is_empty() {
tracing::error!("bundle catalog listed no bundles; keeping the previous catalog");
return Ok(false);
}
bundle_dao::hide_bundles_not_in(&ctx.db, &remote_paths).await?; bundle_dao::hide_bundles_not_in(&ctx.db, &remote_paths).await?;
let bundles_root = paths::bundles_dir()?; let bundles_root = paths::bundles_dir()?;

View file

@ -4,6 +4,7 @@ use std::sync::{Arc, Mutex, OnceLock};
use oneclient_db::dao::artifact as artifact_dao; use oneclient_db::dao::artifact as artifact_dao;
use oneclient_db::dao::cluster as cluster_dao; use oneclient_db::dao::cluster as cluster_dao;
use oneclient_db::dao::bundle as bundle_catalog_dao;
use oneclient_db::dao::cluster_bundle as bundle_dao; use oneclient_db::dao::cluster_bundle as bundle_dao;
use oneclient_db::models::ClusterPatch; use oneclient_db::models::ClusterPatch;
use oneclient_db::models::{BundleTrackedArtifactRow, ClusterBundleOverrideRow, OverrideType}; use oneclient_db::models::{BundleTrackedArtifactRow, ClusterBundleOverrideRow, OverrideType};
@ -86,6 +87,20 @@ async fn check_bundle_updates_inner(
.filter(|name| !loaded_bundle_names.contains(name)) .filter(|name| !loaded_bundle_names.contains(name))
.collect(); .collect();
let delisted_bundles: HashSet<String> = if unavailable_tracked.is_empty() {
HashSet::new()
} else {
bundle_catalog_dao::list_delisted_names_for_version_loader(
&ctx.db,
&cluster.mc_version,
loader as i64,
)
.await?
.into_iter()
.filter(|name| unavailable_tracked.contains(name))
.collect()
};
let overrides_map: HashMap<(String, String), OverrideType> = overrides let overrides_map: HashMap<(String, String), OverrideType> = overrides
.iter() .iter()
.filter_map(|o| { .filter_map(|o| {
@ -211,7 +226,7 @@ async fn check_bundle_updates_inner(
let Some(bundle_name) = &bundle_pkg.bundle_name else { let Some(bundle_name) = &bundle_pkg.bundle_name else {
continue; continue;
}; };
if unavailable_tracked.contains(bundle_name) { if unavailable_tracked.contains(bundle_name) && !delisted_bundles.contains(bundle_name) {
continue; continue;
} }

View file

@ -19,11 +19,11 @@ impl CrashDiagnosis {
pub fn body(&self) -> String { pub fn body(&self) -> String {
match self { match self {
Self::CorruptArchive { file: Some(file) } => format!( Self::CorruptArchive { file: Some(file) } => format!(
"The game could not read {file} — the file is damaged. \ "The game could not read {file} - the file is damaged. \
Verifying will re-download anything that does not match." Verifying will re-download anything that does not match."
), ),
Self::CorruptArchive { file: None } => "The game could not read one of its \ Self::CorruptArchive { file: None } => "The game could not read one of its \
library or mod files — it is damaged. Verifying will re-download \ library or mod files - it is damaged. Verifying will re-download \
anything that does not match." anything that does not match."
.to_string(), .to_string(),
} }

View file

@ -1,7 +1,3 @@
//! Deliberately damaging an installation so repair paths can be exercised
//! Each mode breaks files differently because the launcher catches different
//! damage at different layers
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use oneclient_content::packages::PackageStore; use oneclient_content::packages::PackageStore;
@ -15,8 +11,6 @@ use crate::LauncherResult;
#[derive(Debug, Default, Clone, PartialEq, Eq)] #[derive(Debug, Default, Clone, PartialEq, Eq)]
pub struct SimulationReport { pub struct SimulationReport {
pub affected: usize, pub affected: usize,
/// Capped sample not the full list 50 damaged assets must not become a
/// 50-line toast
pub samples: Vec<String>, pub samples: Vec<String>,
} }
@ -51,9 +45,7 @@ impl SimulationReport {
#[derive(Debug, Clone, Copy, PartialEq, Eq)] #[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum Damage { pub enum Damage {
/// Same length wrong bytes passes a size check fails a hash check
Corrupt, Corrupt,
/// Half length as an interrupted write would leave it
Truncate, Truncate,
Delete, Delete,
} }

View file

@ -358,3 +358,81 @@ async fn opting_a_single_file_in_keeps_the_bundle_live() {
"an explicit opt-in is consent, even with the rest of the bundle disabled" "an explicit opt-in is consent, even with the rest of the bundle disabled"
); );
} }
async fn seed_delisted_bundle(state: &LauncherState) {
oneclient_db::dao::bundle::upsert_bundle(
&state.services.db,
oneclient_db::models::NewBundle {
remote_path: "bundles/delisted.mrpack",
mc_version: MC_VERSION,
mc_loader: GameLoader::Fabric as i64,
file_name: "delisted.mrpack",
name: Some(BUNDLE),
version_id: Some("1"),
category: Some("test"),
loader_version: Some("0.16.0"),
disk_path: "bundles/delisted.mrpack",
hidden: true,
etag: None,
synced_at: None,
},
)
.await
.unwrap();
}
#[tokio::test]
async fn delisted_bundle_content_is_removed() {
let state = oneclient_core::dev::ephemeral_state().await.unwrap();
seed_delisted_bundle(&state).await;
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.removals_available.len(),
1,
"content exclusive to a bundle the catalog dropped should be removed"
);
assert_eq!(check.removals_available[0].package_id, PROJECT_ID);
}
#[tokio::test]
async fn delisted_bundle_content_another_bundle_still_ships_is_kept() {
let state = oneclient_core::dev::ephemeral_state().await.unwrap();
seed_delisted_bundle(&state).await;
let mut successor = manifest(vec![managed_file(true)]);
successor.name = "Successor Bundle".to_string();
oneclient_core::dev::seed_bundle_archive(&state, successor)
.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!(
check.removals_available.is_empty(),
"a mod another live bundle still ships is not exclusive: {:?}",
check.removals_available
);
}
#[tokio::test]
async fn tracked_bundle_that_never_synced_is_not_removed() {
let state = oneclient_core::dev::ephemeral_state().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!(
check.removals_available.is_empty(),
"an absent catalog is not a delisting and must not take content down: {:?}",
check.removals_available
);
}

View file

@ -54,10 +54,8 @@ pub async fn hide_bundles_not_in(
remote_paths: &[String], remote_paths: &[String],
) -> Result<u64, sqlx::Error> { ) -> Result<u64, sqlx::Error> {
if remote_paths.is_empty() { if remote_paths.is_empty() {
let result = sqlx::query("UPDATE bundles SET hidden = 1 WHERE hidden = 0") tracing::warn!("refusing to hide bundles for an empty catalog");
.execute(pool) return Ok(0);
.await?;
return Ok(result.rows_affected());
} }
let mut builder = QueryBuilder::new( let mut builder = QueryBuilder::new(
@ -73,6 +71,21 @@ pub async fn hide_bundles_not_in(
Ok(result.rows_affected()) Ok(result.rows_affected())
} }
pub async fn list_delisted_names_for_version_loader(
pool: &SqlitePool,
mc_version: &str,
mc_loader: i64,
) -> Result<Vec<String>, sqlx::Error> {
sqlx::query_scalar::<_, String>(
"SELECT name FROM bundles \
WHERE mc_version = ? AND mc_loader = ? AND hidden = 1 AND name IS NOT NULL",
)
.bind(mc_version)
.bind(mc_loader)
.fetch_all(pool)
.await
}
pub async fn list_visible_for_version_loader( pub async fn list_visible_for_version_loader(
pool: &SqlitePool, pool: &SqlitePool,
mc_version: &str, mc_version: &str,
@ -150,3 +163,80 @@ pub async fn list_all(pool: &SqlitePool) -> Result<Vec<BundleRow>, sqlx::Error>
.fetch_all(pool) .fetch_all(pool)
.await .await
} }
#[cfg(test)]
mod tests {
use super::*;
async fn pool() -> SqlitePool {
let pool = SqlitePool::connect("sqlite::memory:")
.await
.expect("in-memory sqlite");
sqlx::migrate!("../oneclient_db/migrations")
.run(&pool)
.await
.expect("migrations run");
pool
}
async fn seed(pool: &SqlitePool, remote_path: &str) {
upsert_bundle(
pool,
NewBundle {
remote_path,
mc_version: "1.21.11",
mc_loader: 1,
file_name: "bundle.mrpack",
name: Some("SkyBlock"),
version_id: Some("1"),
category: Some("test"),
loader_version: Some("0.16.0"),
disk_path: remote_path,
hidden: false,
etag: None,
synced_at: None,
},
)
.await
.expect("seed bundle");
}
#[tokio::test]
async fn an_empty_catalog_hides_nothing() {
let pool = pool().await;
seed(&pool, "bundles/skyblock.mrpack").await;
assert_eq!(hide_bundles_not_in(&pool, &[]).await.unwrap(), 0);
assert_eq!(
list_visible_for_version_loader(&pool, "1.21.11", 1)
.await
.unwrap()
.len(),
1,
"an empty catalog must not delist a live bundle"
);
}
#[tokio::test]
async fn a_catalog_that_drops_one_bundle_hides_only_that_one() {
let pool = pool().await;
seed(&pool, "bundles/skyblock.mrpack").await;
seed(&pool, "bundles/qol.mrpack").await;
hide_bundles_not_in(&pool, &["bundles/qol.mrpack".to_string()])
.await
.unwrap();
let visible = list_visible_for_version_loader(&pool, "1.21.11", 1)
.await
.unwrap();
assert_eq!(visible.len(), 1);
assert_eq!(visible[0].remote_path, "bundles/qol.mrpack");
assert_eq!(
list_delisted_names_for_version_loader(&pool, "1.21.11", 1)
.await
.unwrap(),
vec!["SkyBlock".to_string()]
);
}
}