From e3b68450f6d0eee77e46bc0adb3d44006c5bb63d Mon Sep 17 00:00:00 2001 From: Peter Rekdal Khan-Sunde Date: Fri, 10 Apr 2026 15:54:46 +0200 Subject: [PATCH] fix(cli): honor restore installer channel selection --- README.md | 8 +- crates/surge-cli/src/cli.rs | 29 +++ crates/surge-cli/src/commands/install/mod.rs | 20 +- .../install/remote/published_installer.rs | 23 +-- crates/surge-cli/src/commands/pack.rs | 177 +++++++++++++++++- .../surge-cli/src/commands/pack/installers.rs | 11 +- .../surge-cli/src/commands/pack/resolution.rs | 16 +- crates/surge-cli/src/main.rs | 2 + 8 files changed, 234 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index e003dd4..34fb001 100644 --- a/README.md +++ b/README.md @@ -573,15 +573,17 @@ use `--app-id` (and optionally `--rid`) to scope down. surge restore -i ``` -By default this resolves the latest release for the manifest app/target and default channel, restores missing full -packages from storage into `.surge/packages`, and builds installers using artifacts from +By default this resolves the latest release for the manifest app/target on the app's default channel, restores missing +full packages from storage into `.surge/packages`, and builds installers using artifacts from `.surge/artifacts///`. The generated installers are written to -`.surge/installers//`. +`.surge/installers//`. Use `--channel ` to rebuild installers for a non-default channel after a +promotion flow. Explicit override example: ```bash surge restore -i \ + --channel production \ --version 1.2.3 \ --artifacts-dir ./publish \ --packages-dir .surge/packages diff --git a/crates/surge-cli/src/cli.rs b/crates/surge-cli/src/cli.rs index a253116..6b408bd 100644 --- a/crates/surge-cli/src/cli.rs +++ b/crates/surge-cli/src/cli.rs @@ -218,6 +218,10 @@ pub(crate) enum Commands { #[arg(long)] version: Option, + /// Channel to resolve releases from when using --installers + #[arg(long, requires = "installers")] + channel: Option, + /// Build installers only (snapx-compatible restore mode) #[arg(long, short = 'i')] installers: bool, @@ -449,6 +453,15 @@ mod tests { assert!(err.to_string().contains("--installers")); } + #[test] + fn restore_channel_requires_installers_flag() { + let Err(err) = Cli::try_parse_from(["surge", "restore", "--channel", "production"]) else { + panic!("channel should require installers mode"); + }; + + assert!(err.to_string().contains("--installers")); + } + #[test] fn restore_upload_installers_conflicts_with_package_file() { let Err(err) = Cli::try_parse_from([ @@ -465,6 +478,22 @@ mod tests { assert!(err.to_string().contains("--package-file")); } + #[test] + fn restore_channel_parses_in_installers_mode() { + let cli = Cli::try_parse_from(["surge", "restore", "--installers", "--channel", "production"]) + .expect("restore installers mode with channel should parse"); + + let Commands::Restore { + channel, installers, .. + } = cli.command + else { + panic!("expected restore command"); + }; + + assert!(installers); + assert_eq!(channel.as_deref(), Some("production")); + } + #[test] fn install_force_flag_parses() { let cli = Cli::try_parse_from(["surge", "install", "tailscale", "my-node", "--force"]) diff --git a/crates/surge-cli/src/commands/install/mod.rs b/crates/surge-cli/src/commands/install/mod.rs index 388e401..f76f012 100644 --- a/crates/surge-cli/src/commands/install/mod.rs +++ b/crates/surge-cli/src/commands/install/mod.rs @@ -990,16 +990,16 @@ mod tests { } #[test] - fn plan_remote_published_installer_uses_default_channel_key() { + fn plan_remote_published_installer_uses_requested_channel_key() { let manifest = remote_manifest("demo", "linux-arm64", &["test", "production"], &["online"]); - let mut entry = release("1.2.3", "test", "linux-arm64", "demo.tar.zst"); + let mut entry = release("1.2.3", "production", "linux-arm64", "demo.tar.zst"); entry.installers = vec!["online".to_string()]; let plan = plan_remote_published_installer( &manifest, "demo", "linux-arm64", - "test", + "production", &entry, RemoteInstallerMode::Online, ) @@ -1007,13 +1007,13 @@ mod tests { assert_eq!( plan.candidate_keys, - vec!["installers/Setup-linux-arm64-demo-test-online.bin".to_string()] + vec!["installers/Setup-linux-arm64-demo-production-online.bin".to_string()] ); assert!(plan.blockers.is_empty(), "unexpected blockers: {:?}", plan.blockers); } #[test] - fn plan_remote_published_installer_reports_channel_mismatch() { + fn plan_remote_published_installer_drops_default_channel_mismatch_blocker() { let manifest = remote_manifest("demo", "linux-arm64", &["test", "production"], &["online"]); let mut entry = release("1.2.3", "production", "linux-arm64", "demo.tar.zst"); entry.installers = vec!["online".to_string()]; @@ -1030,15 +1030,9 @@ mod tests { assert_eq!( plan.candidate_keys, - vec!["installers/Setup-linux-arm64-demo-test-online.bin".to_string()] - ); - assert!( - plan.blockers - .iter() - .any(|blocker| blocker.contains("default channel 'test'")), - "missing channel mismatch blocker: {:?}", - plan.blockers + vec!["installers/Setup-linux-arm64-demo-production-online.bin".to_string()] ); + assert!(plan.blockers.is_empty(), "unexpected blockers: {:?}", plan.blockers); } #[tokio::test] diff --git a/crates/surge-cli/src/commands/install/remote/published_installer.rs b/crates/surge-cli/src/commands/install/remote/published_installer.rs index 2c03097..6af8f4f 100644 --- a/crates/surge-cli/src/commands/install/remote/published_installer.rs +++ b/crates/surge-cli/src/commands/install/remote/published_installer.rs @@ -13,20 +13,6 @@ fn remote_installer_extension_for_rid(rid: &str) -> &'static str { } } -fn default_channel_for_remote_installer(manifest: &SurgeManifest, app_id: &str) -> Result { - let app = manifest - .apps - .iter() - .find(|candidate| candidate.id == app_id) - .ok_or_else(|| SurgeError::Config(format!("App '{app_id}' was not found in manifest")))?; - Ok(app - .channels - .first() - .cloned() - .or_else(|| manifest.channels.first().map(|channel| channel.name.clone())) - .unwrap_or_else(|| "stable".to_string())) -} - pub(crate) fn plan_remote_published_installer( manifest: &SurgeManifest, app_id: &str, @@ -38,7 +24,6 @@ pub(crate) fn plan_remote_published_installer( let (_app, target) = manifest .find_app_with_target(app_id, rid) .ok_or_else(|| SurgeError::Config(format!("App '{app_id}' with RID '{rid}' not found in manifest")))?; - let default_channel = default_channel_for_remote_installer(manifest, app_id)?; let declared_installers = if release.installers.is_empty() { &target.installers } else { @@ -49,8 +34,7 @@ pub(crate) fn plan_remote_published_installer( RemoteInstallerMode::Offline => "offline", }; let installer_ext = remote_installer_extension_for_rid(rid); - let candidate_key = - format!("installers/Setup-{rid}-{app_id}-{default_channel}-{desired_installer}.{installer_ext}"); + let candidate_key = format!("installers/Setup-{rid}-{app_id}-{channel}-{desired_installer}.{installer_ext}"); let mut blockers = Vec::new(); if !declared_installers @@ -66,11 +50,6 @@ pub(crate) fn plan_remote_published_installer( "release does not declare a '{desired_installer}' installer (declared installers: {declared})" )); } - if channel != default_channel { - blockers.push(format!( - "published installers are currently bound to app default channel '{default_channel}', but install requested '{channel}'" - )); - } Ok(RemotePublishedInstallerPlan { candidate_keys: vec![candidate_key], diff --git a/crates/surge-cli/src/commands/pack.rs b/crates/surge-cli/src/commands/pack.rs index 6b821cc..d270935 100644 --- a/crates/surge-cli/src/commands/pack.rs +++ b/crates/surge-cli/src/commands/pack.rs @@ -169,6 +169,7 @@ pub async fn execute( &app_id, &rid, version, + &self::resolution::default_channel_for_app(&manifest, app), manifest_path.parent().unwrap_or_else(|| Path::new(".")), artifacts_dir.as_path(), output_dir, @@ -207,6 +208,7 @@ pub async fn execute_installers_only( manifest_path: &Path, app_id: Option<&str>, version: Option<&str>, + channel: Option<&str>, rid: Option<&str>, artifacts_dir: Option<&Path>, output_dir: &Path, @@ -230,14 +232,14 @@ pub async fn execute_installers_only( super::ensure_mutating_storage_access(&storage_config, "upload installers")?; } let (backend, index, resolved) = - resolve_installer_package(&manifest, manifest_path, app_id, version, rid, artifacts_dir).await?; + resolve_installer_package(&manifest, manifest_path, app_id, version, channel, rid, artifacts_dir).await?; print_stage_done( theme, 1, total_stages, &format!( "Target: {}/{} (channel: {})", - resolved.app_id, resolved.rid, resolved.default_channel + resolved.app_id, resolved.rid, resolved.selected_channel ), ); @@ -376,6 +378,7 @@ pub async fn execute_installers_only( &resolved.app_id, &resolved.rid, &resolved.selected_version, + &resolved.selected_channel, manifest_path.parent().unwrap_or_else(|| Path::new(".")), &resolved.artifacts_dir, output_dir, @@ -471,6 +474,11 @@ mod tests { } fn write_manifest(path: &Path, store_dir: &Path, app_id: &str, rid: &str) { + write_manifest_with_channels(path, store_dir, app_id, rid, &["stable"]); + } + + fn write_manifest_with_channels(path: &Path, store_dir: &Path, app_id: &str, rid: &str, channels: &[&str]) { + let channels_yaml = channels.join(", "); let yaml = format!( r"schema: 1 storage: @@ -479,7 +487,7 @@ storage: apps: - id: {app_id} main_exe: demoapp - channels: [stable] + channels: [{channels_yaml}] target: rid: {rid} icon: icon.png @@ -567,6 +575,7 @@ apps: &manifest_path, Some(app_id), Some(version), + None, Some(&rid), Some(&artifacts_dir), &packages_dir, @@ -678,6 +687,7 @@ apps: &manifest_path, Some(app_id), None, + None, Some(&rid), None, &packages_dir, @@ -736,6 +746,7 @@ apps: &manifest_path, Some(app_id), Some(version), + None, Some(&rid), None, &packages_dir, @@ -820,6 +831,7 @@ apps: &manifest_path, Some(app_id), Some(version), + None, Some(&rid), None, &packages_dir, @@ -883,6 +895,7 @@ apps: &manifest_path, Some(app_id), Some(version), + None, Some(&rid), Some(&artifacts_dir), &packages_dir, @@ -906,6 +919,161 @@ apps: ); } + #[tokio::test] + async fn execute_installers_only_uses_requested_channel_for_selection_and_installer_manifest() { + let tmp = tempfile::tempdir().expect("temp dir should be created"); + + let store_dir = tmp.path().join("store"); + let artifacts_dir = tmp.path().join("artifacts"); + let packages_dir = tmp.path().join("packages"); + let manifest_path = tmp.path().join("surge.yml"); + let app_id = "installer-app"; + let rid = current_rid(); + let production_version = "1.2.3"; + let test_version = "9.9.9"; + let stub = create_stub_installer_launcher(tmp.path(), &rid); + set_installer_launcher_override(&stub); + + std::fs::create_dir_all(&store_dir).expect("store dir should be created"); + std::fs::create_dir_all(&artifacts_dir).expect("artifacts dir should be created"); + std::fs::create_dir_all(&packages_dir).expect("packages dir should be created"); + std::fs::write(artifacts_dir.join("icon.png"), b"icon").expect("icon should be written"); + write_manifest_with_channels(&manifest_path, &store_dir, app_id, &rid, &["test", "production"]); + + let test_full = format!("{app_id}-{test_version}-{rid}-full.tar.zst"); + let production_full = format!("{app_id}-{production_version}-{rid}-full.tar.zst"); + write_release_index( + &store_dir, + app_id, + vec![ + make_release( + test_version, + "test", + &rid, + &test_full, + &sha256_hex(b"test package bytes"), + ), + make_release( + production_version, + "production", + &rid, + &production_full, + &sha256_hex(b"production package bytes"), + ), + ], + ); + std::fs::write(store_dir.join(&test_full), b"test package bytes").expect("test package should be written"); + std::fs::write(store_dir.join(&production_full), b"production package bytes") + .expect("production package should be written"); + + execute_installers_only( + &manifest_path, + Some(app_id), + None, + Some("production"), + Some(&rid), + Some(&artifacts_dir), + &packages_dir, + None, + false, + ) + .await + .expect("installer generation should succeed"); + + assert!( + packages_dir.join(&production_full).is_file(), + "requested channel should select the production release" + ); + assert!( + !packages_dir.join(&test_full).is_file(), + "requested channel should not select the manifest default channel release" + ); + + let installers_dir = packages_dir + .parent() + .expect("parent should exist") + .join("installers") + .join(app_id) + .join(&rid); + let installer_ext = if rid.starts_with("win-") { "exe" } else { "bin" }; + let offline = installers_dir.join(format!("Setup-{rid}-{app_id}-production-offline.{installer_ext}")); + assert!( + offline.exists(), + "offline installer should use the requested channel in its filename" + ); + + let payload = installer_payload(&offline); + let installer_manifest = String::from_utf8( + surge_core::archive::extractor::read_entry(&payload, "installer.yml") + .expect("installer.yml should be present"), + ) + .expect("installer.yml should be UTF-8"); + assert!(installer_manifest.contains("channel: production")); + assert!(installer_manifest.contains("version: 1.2.3")); + } + + #[tokio::test] + async fn execute_installers_only_uploads_installers_to_requested_channel_key() { + let tmp = tempfile::tempdir().expect("temp dir should be created"); + + let store_dir = tmp.path().join("store"); + let artifacts_dir = tmp.path().join("artifacts"); + let packages_dir = tmp.path().join("packages"); + let manifest_path = tmp.path().join("surge.yml"); + let app_id = "installer-app"; + let rid = current_rid(); + let version = "2.3.0"; + let stub = create_stub_installer_launcher(tmp.path(), &rid); + set_installer_launcher_override(&stub); + + std::fs::create_dir_all(&store_dir).expect("store dir should be created"); + std::fs::create_dir_all(&artifacts_dir).expect("artifacts dir should be created"); + std::fs::create_dir_all(&packages_dir).expect("packages dir should be created"); + std::fs::write(artifacts_dir.join("icon.png"), b"icon").expect("icon should be written"); + write_manifest_with_channels(&manifest_path, &store_dir, app_id, &rid, &["test", "production"]); + + let full_name = format!("{app_id}-{version}-{rid}-full.tar.zst"); + write_release_index( + &store_dir, + app_id, + vec![make_release( + version, + "production", + &rid, + &full_name, + &sha256_hex(b"full package bytes"), + )], + ); + std::fs::write(packages_dir.join(&full_name), b"full package bytes").expect("full package should be written"); + + execute_installers_only( + &manifest_path, + Some(app_id), + Some(version), + Some("production"), + Some(&rid), + Some(&artifacts_dir), + &packages_dir, + None, + true, + ) + .await + .expect("installer generation and upload should succeed"); + + let installer_ext = if rid.starts_with("win-") { "exe" } else { "bin" }; + let online_name = format!("Setup-{rid}-{app_id}-production-online.{installer_ext}"); + let offline_name = format!("Setup-{rid}-{app_id}-production-offline.{installer_ext}"); + + assert!( + store_dir.join("installers").join(&online_name).is_file(), + "online installer should be uploaded under the requested channel key" + ); + assert!( + store_dir.join("installers").join(&offline_name).is_file(), + "offline installer should be uploaded under the requested channel key" + ); + } + #[tokio::test] async fn execute_pack_uses_default_dot_surge_artifacts_layout() { let tmp = tempfile::tempdir().expect("temp dir should be created"); @@ -1009,6 +1177,7 @@ apps: app_id, &rid, version, + "stable", tmp.path(), &artifacts_dir, &output_dir, @@ -1082,6 +1251,7 @@ apps: app_id, &rid, version, + "stable", tmp.path(), &artifacts_dir, &output_dir, @@ -1145,6 +1315,7 @@ apps: app_id, &rid, version, + "stable", &manifest_root, &artifacts_dir, &output_dir, diff --git a/crates/surge-cli/src/commands/pack/installers.rs b/crates/surge-cli/src/commands/pack/installers.rs index 893a312..645d063 100644 --- a/crates/surge-cli/src/commands/pack/installers.rs +++ b/crates/surge-cli/src/commands/pack/installers.rs @@ -14,7 +14,7 @@ use super::launchers::{ ensure_host_compatible_rid, find_gui_installer_launcher_for_rid, find_installer_launcher_for_rid, find_surge_binary_for_rid, surge_binary_name_for_rid, }; -use super::resolution::{default_channel_for_app, installer_storage_prefix}; +use super::resolution::installer_storage_prefix; #[allow(clippy::too_many_arguments)] pub(super) fn build_installers( @@ -24,6 +24,7 @@ pub(super) fn build_installers( app_id: &str, rid: &str, version: &str, + channel: &str, manifest_root: &Path, artifacts_dir: &Path, output_dir: &Path, @@ -36,6 +37,7 @@ pub(super) fn build_installers( app_id, rid, version, + channel, manifest_root, artifacts_dir, output_dir, @@ -52,6 +54,7 @@ pub(super) fn build_installers_with_launcher( app_id: &str, rid: &str, version: &str, + channel: &str, manifest_root: &Path, artifacts_dir: &Path, output_dir: &Path, @@ -64,8 +67,6 @@ pub(super) fn build_installers_with_launcher( } ensure_host_compatible_rid(rid)?; - let default_channel = default_channel_for_app(manifest, app); - let installers_dir = output_dir .parent() .unwrap_or_else(|| Path::new(".")) @@ -110,7 +111,7 @@ pub(super) fn build_installers_with_launcher( for installer_type in installer_types { let installer_suffix = installer_type.as_str(); let installer_ext = if rid.starts_with("win-") { "exe" } else { "bin" }; - let installer_filename = format!("Setup-{rid}-{app_id}-{default_channel}-{installer_suffix}.{installer_ext}"); + let installer_filename = format!("Setup-{rid}-{app_id}-{channel}-{installer_suffix}.{installer_ext}"); let installer_path = installers_dir.join(&installer_filename); let staging_dir = @@ -131,7 +132,7 @@ pub(super) fn build_installers_with_launcher( app_id: app_id.to_string(), rid: rid.to_string(), version: version.to_string(), - channel: default_channel.clone(), + channel: channel.to_string(), generated_utc: chrono::Utc::now().to_rfc3339(), headless_default_if_no_display: true, release_index_key: RELEASES_FILE_COMPRESSED.to_string(), diff --git a/crates/surge-cli/src/commands/pack/resolution.rs b/crates/surge-cli/src/commands/pack/resolution.rs index 1231050..aa6a588 100644 --- a/crates/surge-cli/src/commands/pack/resolution.rs +++ b/crates/surge-cli/src/commands/pack/resolution.rs @@ -12,7 +12,7 @@ use surge_core::storage::{self, StorageBackend}; pub(super) struct ResolvedInstallerPackage { pub(super) app_id: String, pub(super) rid: String, - pub(super) default_channel: String, + pub(super) selected_channel: String, pub(super) selected_version: String, pub(super) full_key: String, pub(super) full_sha256: String, @@ -25,6 +25,7 @@ pub(super) async fn resolve_installer_package( manifest_path: &Path, app_id: Option<&str>, version: Option<&str>, + channel: Option<&str>, rid: Option<&str>, artifacts_dir: Option<&Path>, ) -> Result<(Box, ReleaseIndex, ResolvedInstallerPackage)> { @@ -33,7 +34,10 @@ pub(super) async fn resolve_installer_package( let (app, _) = manifest .find_app_with_target(&app_id, &rid) .ok_or_else(|| SurgeError::Config(format!("No target {rid} found for app {app_id}")))?; - let default_channel = default_channel_for_app(manifest, app); + let selected_channel = channel + .map(str::trim) + .filter(|value| !value.is_empty()) + .map_or_else(|| default_channel_for_app(manifest, app), ToString::to_string); let storage_config = super::super::build_app_scoped_storage_config(manifest, manifest_path, &app_id)?; let backend = storage::create_storage_backend(&storage_config)?; let index = fetch_release_index(&*backend).await?; @@ -43,13 +47,13 @@ pub(super) async fn resolve_installer_package( index.app_id, app_id ))); } - let selected_release = - select_release_for_installers(&index.releases, &default_channel, version, &rid).ok_or_else(|| { + let selected_release = select_release_for_installers(&index.releases, &selected_channel, version, &rid) + .ok_or_else(|| { SurgeError::NotFound(format!( "No release found for app '{}' rid '{}' on channel '{}'{}", app_id, rid, - default_channel, + selected_channel, version.map_or_else(String::new, |v| format!(" and version '{v}'")) )) })?; @@ -76,7 +80,7 @@ pub(super) async fn resolve_installer_package( ResolvedInstallerPackage { app_id, rid, - default_channel, + selected_channel, selected_version: selected_release.version.clone(), full_key: full_key.to_string(), full_sha256: selected_release.full_sha256.clone(), diff --git a/crates/surge-cli/src/main.rs b/crates/surge-cli/src/main.rs index 0e63579..1e37e4c 100644 --- a/crates/surge-cli/src/main.rs +++ b/crates/surge-cli/src/main.rs @@ -222,6 +222,7 @@ async fn run(cli: Cli) -> surge_core::error::Result<()> { app_id, rid, version, + channel, installers, upload_installers, package_file, @@ -233,6 +234,7 @@ async fn run(cli: Cli) -> surge_core::error::Result<()> { &manifest_path, app_id.as_deref(), version.as_deref(), + channel.as_deref(), rid.as_deref(), artifacts_dir.as_deref(), &packages_dir,