fix(runtimes): keep the rollback target when re-selecting the active runtime

apps/rocm/src/main.rs

@@ -8828,14 +8828,35 @@ pub(crate) fn activate_runtime(
     let manifest = select_runtime_manifest(&manifests, selector)?;
     validate_runtime_manifest_for_activation(manifest)?;
     let current = current_runtime_manifest(config, &manifests);
-    let previous_runtime_key = current
+    // Re-selecting the runtime already in use is a no-op to the user: they have
+    // not left anything, so there is no new rollback target — and no reason to
+    // forget the one the activation that put them here recorded. Deriving the
+    // target from `current` unconditionally used to clear it, silently removing
+    // the recovery path that activation had just offered.
+    let (previous_runtime_key, previous_runtime_id) = if current
         .as_ref()
-        .map(|manifest| manifest.runtime_key.clone())
-        .filter(|runtime_key| runtime_key != &manifest.runtime_key);
-    let previous_runtime_id = current
-        .as_ref()
-        .map(|manifest| manifest.runtime_id.clone())
-        .filter(|_| previous_runtime_key.is_some());
+        .is_some_and(|current| current.runtime_key == manifest.runtime_key)
+    {
+        let runtime_key = config.previous_runtime_key.clone();
+        let runtime_id = runtime_key
+            .as_deref()
+            .and_then(|runtime_key| {
+                manifests
+                    .iter()
+                    .find(|candidate| candidate.runtime_key == runtime_key)
+            })
+            .map(|previous| previous.runtime_id.clone());
+        (runtime_key, runtime_id)
+    } else {
+        // `current` differs from the requested runtime here by construction, so
+        // this can never record a runtime as its own rollback target.
+        (
+            current
+                .as_ref()
+                .map(|manifest| manifest.runtime_key.clone()),
+            current.as_ref().map(|manifest| manifest.runtime_id.clone()),
+        )
+    };
 
     config.default_runtime_id = Some(manifest.runtime_id.clone());
     config.active_runtime_key = Some(manifest.runtime_key.clone());

tests/e2e-cucumber/features/runtime_lifecycle.feature

@@ -65,3 +65,18 @@ Feature: Runtime lifecycle state machine
     Then the listing explains the active and rollback markers
     And the first runtime is marked active
     And the second runtime is marked as the rollback target
+
+  # Re-selecting the runtime you are already on changes nothing the user can
+  # see, so it must not quietly take away the recovery path the previous
+  # activation had just offered ("if this causes problems, run `rocm runtimes
+  # rollback`"). It used to: the activation recomputed the rollback target from
+  # the runtime being left, and when there was none it cleared the recorded one
+  # instead of leaving it alone. The shortest command sequence that exposes this
+  # is three activations in a row, which is why none of the hand-written
+  # examples above caught it: every one- and two-command sequence conforms.
+  @id:runtime-lifecycle-reactivating-keeps-rollback-target
+  Scenario: runtime-lifecycle-08 - Re-selecting the runtime already in use keeps the rollback target
+    Given two registered runtimes with the second active after the first
+    When the user activates the second runtime again
+    Then the second runtime is still the one in use
+    And the first runtime is still the rollback target

tests/e2e-cucumber/tests/e2e/runtime_lifecycle_steps.rs

@@ -134,6 +134,12 @@ async fn activate_second(world: &mut E2eWorld) {
     record(world, stdout, stderr, rc);
 }
 
+#[when("the user activates the second runtime again")]
+async fn activate_second_again(world: &mut E2eWorld) {
+    let (stdout, stderr, rc) = crate::run_rocm(world, &["runtimes", "activate", SECOND_KEY]);
+    record(world, stdout, stderr, rc);
+}
+
 #[when("the user rolls back")]
 async fn rollback(world: &mut E2eWorld) {
     let (stdout, stderr, rc) = crate::run_rocm(world, &["runtimes", "rollback"]);
@@ -234,6 +240,26 @@ async fn first_active_again(world: &mut E2eWorld) {
     );
 }
 
+#[then("the second runtime is still the one in use")]
+async fn second_still_active(world: &mut E2eWorld) {
+    let listing = crate::run_rocm_ok(world, &["runtimes", "list"]);
+    assert!(
+        listing.contains(&format!("* {SECOND_KEY}")),
+        "expected {SECOND_KEY} to still be the runtime in use, got:\n{listing}"
+    );
+}
+
+#[then("the first runtime is still the rollback target")]
+async fn first_still_rollback_target(world: &mut E2eWorld) {
+    let listing = crate::run_rocm_ok(world, &["runtimes", "list"]);
+    assert!(
+        listing.contains(&format!("- {FIRST_KEY}")),
+        "re-selecting the runtime already in use is a no-op for the user, so it must not \
+         discard the rollback target the previous activation promised; expected {FIRST_KEY} \
+         still marked as the rollback target, got:\n{listing}"
+    );
+}
+
 #[then("its registry entry is removed")]
 async fn registry_removed(world: &mut E2eWorld) {
     let out = ok_output(world);