feat: improve service version migration and data version handling

Extract get_data_version into a shared function used by both effects
and service_map. Use the actual data version (instead of the previous
package version) when computing migration targets, and skip migrations
when the target range is unsatisfiable. Also detect install vs update
based on the presence of a data version file rather than load
disposition alone.

core/src/service/effects/version.rs

@@ -2,7 +2,7 @@ use std::path::Path;
 
 use crate::DATA_DIR;
 use crate::service::effects::prelude::*;
-use crate::util::io::{delete_file, maybe_read_file_to_string, write_file_atomic};
+use crate::util::io::{delete_file, write_file_atomic};
 use crate::volume::PKG_VOLUME_DIR;
 
 #[derive(Debug, Clone, Serialize, Deserialize, TS, Parser)]
@@ -36,11 +36,5 @@ pub async fn set_data_version(
 #[instrument(skip_all)]
 pub async fn get_data_version(context: EffectContext) -> Result<Option<String>, Error> {
     let context = context.deref()?;
-    let package_id = &context.seed.id;
+    crate::service::get_data_version(&context.seed.id).await
-    let path = Path::new(DATA_DIR)
-        .join(PKG_VOLUME_DIR)
-        .join(package_id)
-        .join("data")
-        .join(".version");
-    maybe_read_file_to_string(path).await
 }

core/src/service/mod.rs

@@ -46,12 +46,14 @@ use crate::service::uninstall::cleanup;
 use crate::util::Never;
 use crate::util::actor::concurrent::ConcurrentActor;
 use crate::util::future::NonDetachingJoinHandle;
-use crate::util::io::{AsyncReadStream, AtomicFile, TermSize, delete_file};
+use crate::util::io::{
+    AsyncReadStream, AtomicFile, TermSize, delete_file, maybe_read_file_to_string,
+};
 use crate::util::net::WebSocket;
 use crate::util::serde::Pem;
 use crate::util::sync::SyncMutex;
 use crate::util::tui::choose;
-use crate::volume::data_dir;
+use crate::volume::{PKG_VOLUME_DIR, data_dir};
 use crate::{ActionId, CAP_1_KiB, DATA_DIR, ImageId, PackageId};
 
 pub mod action;
@@ -81,6 +83,17 @@ pub enum LoadDisposition {
     Undo,
 }
 
+/// Read the data version file for a service from disk.
+/// Returns `Ok(None)` if the file does not exist (fresh install).
+pub async fn get_data_version(id: &PackageId) -> Result<Option<String>, Error> {
+    let path = Path::new(DATA_DIR)
+        .join(PKG_VOLUME_DIR)
+        .join(id)
+        .join("data")
+        .join(".version");
+    maybe_read_file_to_string(&path).await
+}
+
 struct RootCommand(pub String);
 
 #[derive(Clone, Debug, Serialize, Deserialize, Default, TS)]
@@ -390,12 +403,17 @@ impl Service {
                         tracing::error!("Error opening s9pk for install: {e}");
                         tracing::debug!("{e:?}")
                     }) {
+                        let init_kind = if get_data_version(id).await.ok().flatten().is_some() {
+                            InitKind::Update
+                        } else {
+                            InitKind::Install
+                        };
                         if let Ok(service) = Self::install(
                             ctx.clone(),
                             s9pk,
                             &s9pk_path,
                             &None,
-                            InitKind::Install,
+                            init_kind,
                             None::<Never>,
                             None,
                         )
@@ -424,12 +442,17 @@ impl Service {
                         tracing::error!("Error opening s9pk for update: {e}");
                         tracing::debug!("{e:?}")
                     }) {
+                        let init_kind = if get_data_version(id).await.ok().flatten().is_some() {
+                            InitKind::Update
+                        } else {
+                            InitKind::Install
+                        };
                         if let Ok(service) = Self::install(
                             ctx.clone(),
                             s9pk,
                             &s9pk_path,
                             &None,
-                            InitKind::Update,
+                            init_kind,
                             None::<Never>,
                             None,
                         )

core/src/service/rpc.rs

@@ -107,6 +107,12 @@ impl ExitParams {
             target: Some(InternedString::from_display(range)),
         }
     }
+    pub fn target_str(s: &str) -> Self {
+        Self {
+            id: Guid::new(),
+            target: Some(InternedString::intern(s)),
+        }
+    }
     pub fn uninstall() -> Self {
         Self {
             id: Guid::new(),

core/src/service/service_map.rs

@@ -28,7 +28,7 @@ use crate::s9pk::S9pk;
 use crate::s9pk::manifest::PackageId;
 use crate::s9pk::merkle_archive::source::FileSource;
 use crate::service::rpc::{ExitParams, InitKind};
-use crate::service::{LoadDisposition, Service, ServiceRef};
+use crate::service::{LoadDisposition, Service, ServiceRef, get_data_version};
 use crate::sign::commitment::merkle_archive::MerkleArchiveCommitment;
 use crate::status::{DesiredStatus, StatusInfo};
 use crate::util::future::NonDetachingJoinHandle;
@@ -310,36 +310,60 @@ impl ServiceMap {
                 .handle_last(async move {
                     finalization_progress.start();
                     let s9pk = S9pk::open(&installed_path, Some(&id)).await?;
+                    let data_version = get_data_version(&id).await?;
                     let prev = if let Some(service) = service.take() {
                         ensure_code!(
                             recovery_source.is_none(),
                             ErrorKind::InvalidRequest,
                             "cannot restore over existing package"
                         );
-                        let prev_version = service
+                        let uninit = if let Some(ref data_ver) = data_version {
-                            .seed
+                            let prev_can_migrate_to = &service
-                            .persistent_container
+                                .seed
-                            .s9pk
+                                .persistent_container
-                            .as_manifest()
+                                .s9pk
-                            .version
+                                .as_manifest()
-                            .clone();
+                                .can_migrate_to;
-                        let prev_can_migrate_to = &service
+                            let next_version = &s9pk.as_manifest().version;
-                            .seed
+                            let next_can_migrate_from =
-                            .persistent_container
+                                &s9pk.as_manifest().can_migrate_from;
-                            .s9pk
+                            if let Ok(data_ver_ev) =
-                            .as_manifest()
+                                data_ver.parse::<exver::ExtendedVersion>()
-                            .can_migrate_to;
+                            {
-                        let next_version = &s9pk.as_manifest().version;
+                                if data_ver_ev.satisfies(next_can_migrate_from) {
-                        let next_can_migrate_from = &s9pk.as_manifest().can_migrate_from;
+                                    ExitParams::target_str(data_ver)
-                        let uninit = if prev_version.satisfies(next_can_migrate_from) {
+                                } else if next_version.satisfies(prev_can_migrate_to) {
-                            ExitParams::target_version(&*prev_version)
+                                    ExitParams::target_version(&s9pk.as_manifest().version)
-                        } else if next_version.satisfies(prev_can_migrate_to) {
+                                } else {
-                            ExitParams::target_version(&s9pk.as_manifest().version)
+                                    ExitParams::target_range(&VersionRange::and(
+                                        prev_can_migrate_to.clone(),
+                                        next_can_migrate_from.clone(),
+                                    ))
+                                }
+                            } else if let Ok(data_ver_range) =
+                                data_ver.parse::<VersionRange>()
+                            {
+                                ExitParams::target_range(&VersionRange::and(
+                                    data_ver_range,
+                                    next_can_migrate_from.clone(),
+                                ))
+                            } else if next_version.satisfies(prev_can_migrate_to) {
+                                ExitParams::target_version(&s9pk.as_manifest().version)
+                            } else {
+                                ExitParams::target_range(&VersionRange::and(
+                                    prev_can_migrate_to.clone(),
+                                    next_can_migrate_from.clone(),
+                                ))
+                            }
                         } else {
-                            ExitParams::target_range(&VersionRange::and(
+                            ExitParams::target_version(
-                                prev_can_migrate_to.clone(),
+                                &*service
-                                next_can_migrate_from.clone(),
+                                    .seed
-                            ))
+                                    .persistent_container
+                                    .s9pk
+                                    .as_manifest()
+                                    .version,
+                            )
                         };
                         let cleanup = service.uninstall(uninit, false, false).await?;
                         progress.complete();
@@ -354,7 +378,7 @@ impl ServiceMap {
                         &registry,
                         if recovery_source.is_some() {
                             InitKind::Restore
-                        } else if prev.is_some() {
+                        } else if data_version.is_some() {
                             InitKind::Update
                         } else {
                             InitKind::Install

sdk/package/lib/version/VersionGraph.ts

@@ -331,6 +331,9 @@ export class VersionGraph<CurrentVersion extends string>
     target: VersionRange | ExtendedVersion | null,
   ): Promise<void> {
     if (target) {
+      if (isRange(target) && !target.satisfiable()) {
+        return
+      }
       const from = await getDataVersion(effects)
       if (from) {
         target = await this.migrate({