fix(conformance): address second-round code-review findings

- settings_management.rs's wait_for_ready dropped sandbox_lifecycle.rs's
  defensive name-mismatch check when its JSON polling loop was written.
  Added it back (and the missing `name` field on SandboxState) so a
  wrong-sandbox response fails fast with a clear diagnostic instead of
  silently comparing the wrong resource's phase.
- provider_auto_create.rs's cleanup poll used a 30s overall retry budget
  with a 2-minute per-attempt command timeout -- a single slow attempt
  could exhaust the whole budget before a retry got a chance. Widened the
  retry budget to comfortably exceed the per-attempt ceiling.
- provider_auto_create.rs's sandbox was no longer tracked via
  runner.track_sandbox() after switching to explicit ordered cleanup in the
  prior commit, losing the Drop-impl warning and finish() safety net if a
  panic skips the explicit cleanup. Re-added tracking (forgotten once
  explicit cleanup completes) alongside the explicit poll-retried delete,
  which is still what actually avoids the sandbox/provider detach race.
- Added CommandResult::require_outcome(expect_success) to the shared crate
  and switched workspace_lifecycle.rs's run_cli and settings_management.rs's
  settings_delete to use it instead of two independent, byte-for-byte
  identical inline implementations.
- Consolidated settings_management.rs's require_setting_line and
  require_setting_line_with_scope into one function taking an
  Option<&str> scope.

Re-verified all four affected scenarios (8 tests) against a fresh Podman
gateway.

Signed-off-by: politerealism <burdcat17@gmail.com>

Author: politerealism

crates/openshell-conformance/src/lib.rs

@@ -161,6 +161,20 @@ impl CommandResult {
         Err(self.failure_diagnostic(&self.expectation))
     }
 
+    /// Assert the command's success matches `expect_success` (`false` asserts
+    /// the command fails, e.g. a rejected duplicate-name or blocked-while-managed case).
+    pub fn require_outcome(&self, expect_success: bool) -> Result<(), String> {
+        if self.success() == expect_success {
+            Ok(())
+        } else {
+            Err(self.failure_diagnostic(if expect_success {
+                "command succeeds"
+            } else {
+                "command fails"
+            }))
+        }
+    }
+
     pub fn failure_diagnostic(&self, expectation: &str) -> String {
         format!(
             "{}\nexpected: {expectation}\nactual: exit {} after {:.1?}\ncommand: {}\nstdout:\n{}\nstderr:\n{}",

crates/openshell-conformance/src/scenarios/provider_auto_create.rs

@@ -23,7 +23,10 @@ network_policies: {}
 
 const CREATE_TIMEOUT: Duration = Duration::from_mins(10);
 const COMMAND_TIMEOUT: Duration = Duration::from_mins(2);
-const PROVIDER_DETACH_TIMEOUT: Duration = Duration::from_secs(30);
+// Must comfortably exceed COMMAND_TIMEOUT: poll_until only checks elapsed
+// time between attempts, so a single slow attempt must not be able to
+// consume the whole retry budget before a second attempt gets a chance.
+const PROVIDER_DETACH_TIMEOUT: Duration = Duration::from_mins(3);
 const PROVIDER_DETACH_INTERVAL: Duration = Duration::from_secs(1);
 
 // "claude-code" is a recognized auto-provider type, not an arbitrary name:
@@ -55,9 +58,15 @@ fn run_provider_auto_create(runner: &mut OpenShellRunner) -> ScenarioFuture<'_>
         delete_provider_best_effort(runner).await;
 
         let sandbox_name = format!("ct-{}-pa", runner.id());
+        // Tracked as a safety net for runner.finish()'s generic sweep (and the
+        // Drop-impl warning) if a panic skips the explicit cleanup() below;
+        // the ordered, poll-retried delete in cleanup() is what actually
+        // avoids the sandbox/provider detach race in the common case.
+        runner.track_sandbox(&sandbox_name);
         let result =
             auto_created_provider_credential_available_in_sandbox(runner, &sandbox_name).await;
         cleanup(runner, &sandbox_name).await;
+        runner.forget_sandbox(&sandbox_name);
         result
     })
 }

crates/openshell-conformance/src/scenarios/settings_management.rs

@@ -22,6 +22,7 @@ const TEST_KEY: &str = "ocsf_json_enabled";
 
 #[derive(Debug, Deserialize)]
 struct SandboxState {
+    name: String,
     phase: String,
 }
 
@@ -77,21 +78,21 @@ async fn settings_global_override_round_trip(runner: &mut OpenShellRunner) -> Re
     wait_for_ready(runner, &sandbox_name).await?;
 
     let initial = settings_get(runner, &sandbox_name, "initial").await?;
-    require_setting_line_with_scope(
+    require_setting_line(
         &initial,
         "<unset>",
-        "unset",
+        Some("unset"),
         "initial sandbox setting is unset",
     )?;
 
     settings_set(runner, &sandbox_name, "true", "sandbox-set").await?;
     wait_for_setting_value(runner, &sandbox_name, "true", "sandbox").await?;
 
     let after_sandbox_set = settings_get(runner, &sandbox_name, "after-sandbox-set").await?;
-    require_setting_line_with_scope(
+    require_setting_line(
         &after_sandbox_set,
         "true",
-        "sandbox",
+        Some("sandbox"),
         "setting reflects the sandbox-scoped value",
     )?;
 
@@ -101,10 +102,10 @@ async fn settings_global_override_round_trip(runner: &mut OpenShellRunner) -> Re
     }
 
     let after_sandbox_delete = settings_get(runner, &sandbox_name, "after-sandbox-delete").await?;
-    require_setting_line_with_scope(
+    require_setting_line(
         &after_sandbox_delete,
         "<unset>",
-        "unset",
+        Some("unset"),
         "setting is unset again after sandbox delete",
     )?;
 
@@ -164,6 +165,7 @@ async fn settings_global_override_round_trip(runner: &mut OpenShellRunner) -> Re
     require_setting_line(
         &global_get,
         "false",
+        None,
         "global setting reflects the override value",
     )?;
 
@@ -190,6 +192,7 @@ async fn settings_global_override_round_trip(runner: &mut OpenShellRunner) -> Re
     require_setting_line(
         &global_after_delete,
         "<unset>",
+        None,
         "global setting is unset after delete",
     )?;
 
@@ -203,10 +206,10 @@ async fn settings_global_override_round_trip(runner: &mut OpenShellRunner) -> Re
     wait_for_setting_value(runner, &sandbox_name, "false", "sandbox").await?;
 
     let sandbox_after_delete = settings_get(runner, &sandbox_name, "after-global-delete").await?;
-    require_setting_line_with_scope(
+    require_setting_line(
         &sandbox_after_delete,
         "false",
-        "sandbox",
+        Some("sandbox"),
         "sandbox-level control resumes after global delete",
     )?;
 
@@ -241,6 +244,10 @@ async fn wait_for_ready(runner: &mut OpenShellRunner, sandbox_name: &str) -> Res
                         Poll::Pending(result.failure_diagnostic("sandbox can be retrieved"))
                     }
                     Ok(result) => match result.json::<SandboxState>() {
+                        Ok(state) if state.name != sandbox_name => Poll::Failed(format!(
+                            "sandbox get returned {:?}; expected '{sandbox_name}'",
+                            state.name
+                        )),
                         Ok(state) if state.phase == "Ready" => Poll::Ready(()),
                         Ok(state) => Poll::Pending(format!(
                             "sandbox phase is {:?}; expected \"Ready\"",
@@ -359,15 +366,8 @@ async fn settings_delete(
         .run(&["settings", "delete", sandbox_name, "--key", TEST_KEY])
         .await
         .map_err(|error| error.to_string())?;
-    if result.success() == expect_success {
-        Ok(result)
-    } else {
-        Err(result.failure_diagnostic(if expect_success {
-            "command succeeds"
-        } else {
-            "command fails"
-        }))
-    }
+    result.require_outcome(expect_success)?;
+    Ok(result)
 }
 
 /// Best-effort teardown. Mirrors a `Drop`-guard cleanup: ignore failures since
@@ -386,22 +386,14 @@ async fn cleanup_global_setting(runner: &OpenShellRunner) {
 fn require_setting_line(
     result: &CommandResult,
     expected: &str,
+    scope: Option<&str>,
     expectation: &str,
 ) -> Result<(), String> {
-    if result.output_contains(&format!("{TEST_KEY} = {expected}")) {
-        Ok(())
-    } else {
-        Err(result.failure_diagnostic(expectation))
-    }
-}
-
-fn require_setting_line_with_scope(
-    result: &CommandResult,
-    expected: &str,
-    scope: &str,
-    expectation: &str,
-) -> Result<(), String> {
-    if result.output_contains(&format!("{TEST_KEY} = {expected} ({scope})")) {
+    let needle = match scope {
+        Some(scope) => format!("{TEST_KEY} = {expected} ({scope})"),
+        None => format!("{TEST_KEY} = {expected}"),
+    };
+    if result.output_contains(&needle) {
         Ok(())
     } else {
         Err(result.failure_diagnostic(expectation))

crates/openshell-conformance/src/scenarios/workspace_lifecycle.rs

@@ -322,15 +322,8 @@ async fn run_cli(
         .run(args)
         .await
         .map_err(|error| error.to_string())?;
-    if result.success() == expect_success {
-        Ok(result)
-    } else {
-        Err(result.failure_diagnostic(if expect_success {
-            "command succeeds"
-        } else {
-            "command fails"
-        }))
-    }
+    result.require_outcome(expect_success)?;
+    Ok(result)
 }
 
 fn require_contains(result: &CommandResult, needle: &str, expectation: &str) -> Result<(), String> {