diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index f1a44ad31..c6489e21a 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -1842,11 +1842,11 @@ pub async fn sandbox_create( .into_diagnostic()? .into_inner(); - let mut last_phase = sandbox.phase; + let mut last_phase = sandbox.phase(); let mut last_error_reason = String::new(); let mut last_condition_message = ready_false_condition_message(sandbox.status.as_ref()); // Track whether we have seen a non-Ready phase during the watch. - let mut saw_non_ready = SandboxPhase::try_from(sandbox.phase) != Ok(SandboxPhase::Ready); + let mut saw_non_ready = SandboxPhase::try_from(sandbox.phase()) != Ok(SandboxPhase::Ready); let provision_timeout = Duration::from_secs( std::env::var("OPENSHELL_PROVISION_TIMEOUT") .ok() @@ -1899,8 +1899,8 @@ pub async fn sandbox_create( let evt = item.into_diagnostic()?; match evt.payload { Some(openshell_core::proto::sandbox_stream_event::Payload::Sandbox(s)) => { - let phase = SandboxPhase::try_from(s.phase).unwrap_or(SandboxPhase::Unknown); - last_phase = s.phase; + let phase = SandboxPhase::try_from(s.phase()).unwrap_or(SandboxPhase::Unknown); + last_phase = s.phase(); if let Some(message) = ready_false_condition_message(s.status.as_ref()) { last_condition_message = Some(message); } @@ -2451,7 +2451,7 @@ pub async fn sandbox_get( }; println!(" {} {}", "Id:".dimmed(), id); println!(" {} {}", "Name:".dimmed(), name); - println!(" {} {}", "Phase:".dimmed(), phase_name(sandbox.phase)); + println!(" {} {}", "Phase:".dimmed(), phase_name(sandbox.phase())); println!( " {} {}", "Resource version:".dimmed(), @@ -2535,11 +2535,11 @@ pub async fn sandbox_exec_grpc( .ok_or_else(|| miette::miette!("sandbox not found"))?; // Verify the sandbox is ready before issuing the exec. - if SandboxPhase::try_from(sandbox.phase) != Ok(SandboxPhase::Ready) { + if SandboxPhase::try_from(sandbox.phase()) != Ok(SandboxPhase::Ready) { return Err(miette::miette!( "sandbox '{}' is not ready (phase: {}); wait for it to reach Ready state", name, - phase_name(sandbox.phase) + phase_name(sandbox.phase()) )); } @@ -2747,11 +2747,11 @@ async fn fetch_ready_sandbox_for_forward( .sandbox .ok_or_else(|| miette::miette!("sandbox '{name}' not found"))?; - if SandboxPhase::try_from(sandbox.phase) != Ok(SandboxPhase::Ready) { + if SandboxPhase::try_from(sandbox.phase()) != Ok(SandboxPhase::Ready) { return Err(miette::miette!( "sandbox '{}' is no longer ready (phase: {}); stopping service forward", name, - phase_name(sandbox.phase) + phase_name(sandbox.phase()) )); } @@ -3195,8 +3195,8 @@ pub async fn sandbox_list( // Print rows for sandbox in sandboxes { - let phase = phase_name(sandbox.phase); - let phase_colored = match SandboxPhase::try_from(sandbox.phase) { + let phase = phase_name(sandbox.phase()); + let phase_colored = match SandboxPhase::try_from(sandbox.phase()) { Ok(SandboxPhase::Ready) => phase.green().to_string(), Ok(SandboxPhase::Error) => phase.red().to_string(), Ok(SandboxPhase::Provisioning) => phase.yellow().to_string(), @@ -3224,8 +3224,8 @@ fn sandbox_to_json(sandbox: &Sandbox) -> serde_json::Value { "labels": labels, "resource_version": meta.map_or(0, |m| m.resource_version), "created_at": format_epoch_ms(meta.map_or(0, |m| m.created_at_ms)), - "phase": phase_name(sandbox.phase), - "current_policy_version": sandbox.current_policy_version, + "phase": phase_name(sandbox.phase()), + "current_policy_version": sandbox.current_policy_version(), }) } @@ -7596,8 +7596,6 @@ mod tests { let status = SandboxStatus { sandbox_name: "gpu".to_string(), agent_pod: "gpu-pod".to_string(), - agent_fd: String::new(), - sandbox_fd: String::new(), conditions: vec![SandboxCondition { r#type: "Ready".to_string(), status: "False".to_string(), @@ -7605,6 +7603,7 @@ mod tests { message: "Another GPU sandbox may already be using the available GPU.".to_string(), last_transition_time: String::new(), }], + ..Default::default() }; assert_eq!( @@ -7618,8 +7617,6 @@ mod tests { let status = SandboxStatus { sandbox_name: "gpu".to_string(), agent_pod: "gpu-pod".to_string(), - agent_fd: String::new(), - sandbox_fd: String::new(), conditions: vec![SandboxCondition { r#type: "Scheduled".to_string(), status: "True".to_string(), @@ -7627,6 +7624,7 @@ mod tests { message: "Sandbox scheduled".to_string(), last_transition_time: String::new(), }], + ..Default::default() }; assert!(ready_false_condition_message(Some(&status)).is_none()); diff --git a/crates/openshell-cli/tests/provider_commands_integration.rs b/crates/openshell-cli/tests/provider_commands_integration.rs index 090097a20..02b69dc65 100644 --- a/crates/openshell-cli/tests/provider_commands_integration.rs +++ b/crates/openshell-cli/tests/provider_commands_integration.rs @@ -126,8 +126,6 @@ impl OpenShell for TestOpenShell { }), spec: None, status: None, - phase: 0, - current_policy_version: 0, }), })) } diff --git a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs index 3ed43b2fc..f51d5e3c9 100644 --- a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs +++ b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs @@ -79,18 +79,19 @@ impl OpenShell for TestOpenShell { name }; - Ok(Response::new(SandboxResponse { - sandbox: Some(Sandbox { - metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { - id: format!("id-{sandbox_name}"), - name: sandbox_name, - created_at_ms: 0, - labels: HashMap::new(), - resource_version: 0, - }), - phase: SandboxPhase::Provisioning as i32, - ..Sandbox::default() + let mut sandbox = Sandbox { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: format!("id-{sandbox_name}"), + name: sandbox_name, + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, }), + ..Sandbox::default() + }; + sandbox.set_phase(SandboxPhase::Provisioning as i32); + Ok(Response::new(SandboxResponse { + sandbox: Some(sandbox), })) } @@ -99,18 +100,19 @@ impl OpenShell for TestOpenShell { request: tonic::Request, ) -> Result, Status> { let name = request.into_inner().name; - Ok(Response::new(SandboxResponse { - sandbox: Some(Sandbox { - metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { - id: format!("id-{name}"), - name, - created_at_ms: 0, - labels: HashMap::new(), - resource_version: 0, - }), - phase: SandboxPhase::Ready as i32, - ..Sandbox::default() + let mut sandbox = Sandbox { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: format!("id-{name}"), + name, + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, }), + ..Sandbox::default() + }; + sandbox.set_phase(SandboxPhase::Ready as i32); + Ok(Response::new(SandboxResponse { + sandbox: Some(sandbox), })) } @@ -351,7 +353,7 @@ impl OpenShell for TestOpenShell { let vm_log_churn_before_ready = self.state.vm_log_churn_before_ready.load(Ordering::SeqCst); tokio::spawn(async move { - let provisioning = Sandbox { + let mut provisioning = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: sandbox_id.clone(), name: sandbox_id.trim_start_matches("id-").to_string(), @@ -359,16 +361,12 @@ impl OpenShell for TestOpenShell { labels: HashMap::new(), resource_version: 0, }), - phase: SandboxPhase::Provisioning as i32, ..Sandbox::default() }; - let error = Sandbox { - phase: SandboxPhase::Error as i32, + provisioning.set_phase(SandboxPhase::Provisioning as i32); + let mut error = Sandbox { status: Some(SandboxStatus { sandbox_name: sandbox_id.trim_start_matches("id-").to_string(), - agent_pod: String::new(), - agent_fd: String::new(), - sandbox_fd: String::new(), conditions: vec![SandboxCondition { r#type: "Ready".to_string(), status: "False".to_string(), @@ -376,13 +374,13 @@ impl OpenShell for TestOpenShell { message: "VM process exited with status 0".to_string(), last_transition_time: String::new(), }], + ..Default::default() }), ..provisioning.clone() }; - let ready = Sandbox { - phase: SandboxPhase::Ready as i32, - ..provisioning.clone() - }; + error.set_phase(SandboxPhase::Error as i32); + let mut ready = provisioning.clone(); + ready.set_phase(SandboxPhase::Ready as i32); let _ = tx .send(Ok(SandboxStreamEvent { diff --git a/crates/openshell-core/src/metadata.rs b/crates/openshell-core/src/metadata.rs index 78533e1e0..af26f73ae 100644 --- a/crates/openshell-core/src/metadata.rs +++ b/crates/openshell-core/src/metadata.rs @@ -6,7 +6,7 @@ //! These traits provide uniform access to `ObjectMeta` fields across all resource types. use crate::proto::{ - InferenceRoute, ObjectForTest, Provider, Sandbox, ServiceEndpoint, SshSession, + InferenceRoute, ObjectForTest, Provider, Sandbox, SandboxStatus, ServiceEndpoint, SshSession, StoredProviderCredentialRefreshState, StoredProviderProfile, }; use std::collections::HashMap; @@ -69,6 +69,26 @@ impl GetResourceVersion for Sandbox { } } +impl Sandbox { + pub fn phase(&self) -> i32 { + self.status.as_ref().map_or(0, |s| s.phase) + } + + pub fn set_phase(&mut self, phase: i32) { + self.status.get_or_insert_with(SandboxStatus::default).phase = phase; + } + + pub fn current_policy_version(&self) -> u32 { + self.status.as_ref().map_or(0, |s| s.current_policy_version) + } + + pub fn set_current_policy_version(&mut self, version: u32) { + self.status + .get_or_insert_with(SandboxStatus::default) + .current_policy_version = version; + } +} + // Implementations for Provider impl ObjectId for Provider { fn object_id(&self) -> &str { diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 98dc3fd63..0a79596e9 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -525,7 +525,7 @@ impl ComputeRuntime { let sandbox = self .store .update_message_cas::(&id, 0, |s| { - s.phase = SandboxPhase::Deleting as i32; + s.set_phase(SandboxPhase::Deleting as i32); }) .await .map_err(|e| { @@ -608,7 +608,7 @@ impl ComputeRuntime { } }; - let phase = SandboxPhase::try_from(sandbox.phase).unwrap_or(SandboxPhase::Unknown); + let phase = SandboxPhase::try_from(sandbox.phase()).unwrap_or(SandboxPhase::Unknown); if !sandbox_phase_should_be_running(phase) { continue; } @@ -682,7 +682,7 @@ impl ComputeRuntime { match self .store .update_message_cas::(&sandbox_id, 0, |s| { - s.phase = SandboxPhase::Error as i32; + s.set_phase(SandboxPhase::Error as i32); let name = s.object_name().to_string(); upsert_ready_condition( &mut s.status, @@ -860,21 +860,25 @@ impl ComputeRuntime { use crate::persistence::WriteCondition; let now_ms = openshell_core::time::now_ms(); - let mut status = incoming.status.as_ref().map(public_status_from_driver); - rewrite_user_facing_conditions(&mut status, None); - let session_connected = self.supervisor_sessions.has_session(&incoming.id); let mut phase = derive_phase(incoming.status.as_ref()); - let sandbox_name = incoming.name.clone(); - if session_connected - && matches!(phase, SandboxPhase::Provisioning | SandboxPhase::Unknown) - { - ensure_supervisor_ready_status(&mut status, &sandbox_name); + + let supervisor_promoted = session_connected + && matches!(phase, SandboxPhase::Provisioning | SandboxPhase::Unknown); + if supervisor_promoted { phase = SandboxPhase::Ready; } - let sandbox = Sandbox { + let mut status = incoming + .status + .as_ref() + .map(|s| public_status_from_driver(s, phase, 0)); + rewrite_user_facing_conditions(&mut status, None); + if supervisor_promoted { + ensure_supervisor_ready_status(&mut status, &sandbox_name); + } + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: incoming.id.clone(), name: sandbox_name, @@ -884,9 +888,8 @@ impl ComputeRuntime { }), spec: None, status, - phase: phase as i32, - current_policy_version: 0, }; + sandbox.set_phase(phase as i32); self.store .put_if( @@ -921,19 +924,35 @@ impl ComputeRuntime { let sandbox = self .store .update_message_cas::(&incoming.id, 0, |sandbox| { - let mut status = incoming.status.as_ref().map(public_status_from_driver); + let old_phase = + SandboxPhase::try_from(sandbox.phase()).unwrap_or(SandboxPhase::Unknown); + let mut phase = incoming + .status + .as_ref() + .map_or(old_phase, |status| derive_phase(Some(status))); + let supervisor_promoted = session_connected + && matches!(phase, SandboxPhase::Provisioning | SandboxPhase::Unknown); + if supervisor_promoted { + phase = SandboxPhase::Ready; + } + + let cpv = sandbox.current_policy_version(); + let mut status = incoming + .status + .as_ref() + .map(|s| public_status_from_driver(s, phase, cpv)) + .or_else(|| sandbox.status.clone()); rewrite_user_facing_conditions(&mut status, sandbox.spec.as_ref()); + if supervisor_promoted { + ensure_supervisor_ready_status(&mut status, &sandbox_name); + } - let mut phase = derive_phase(incoming.status.as_ref()); - if session_connected - && matches!(phase, SandboxPhase::Provisioning | SandboxPhase::Unknown) + if let Some(s) = status.as_mut() + && s.sandbox_name.is_empty() { - ensure_supervisor_ready_status(&mut status, &sandbox_name); - phase = SandboxPhase::Ready; + s.sandbox_name.clone_from(&sandbox_name); } - let old_phase = - SandboxPhase::try_from(sandbox.phase).unwrap_or(SandboxPhase::Unknown); if old_phase != phase { info!( sandbox_id = %incoming.id, @@ -967,9 +986,9 @@ impl ComputeRuntime { if let Some(metadata) = sandbox.metadata.as_mut() { metadata.name.clone_from(&sandbox_name); } - // Note: namespace field removed from public Sandbox API - it remains internal to DriverSandbox sandbox.status = status; - sandbox.phase = phase as i32; + sandbox.set_phase(phase as i32); + sandbox.set_current_policy_version(cpv); }) .await .map_err(|e| match e { @@ -1008,7 +1027,7 @@ impl ComputeRuntime { .store .update_message_cas::(sandbox_id, 0, |sandbox| { let current_phase = - SandboxPhase::try_from(sandbox.phase).unwrap_or(SandboxPhase::Unknown); + SandboxPhase::try_from(sandbox.phase()).unwrap_or(SandboxPhase::Unknown); // Skip if sandbox is in terminal state if current_phase == SandboxPhase::Deleting || current_phase == SandboxPhase::Error { @@ -1018,10 +1037,10 @@ impl ComputeRuntime { let sandbox_name = sandbox.object_name().to_string(); if connected { ensure_supervisor_ready_status(&mut sandbox.status, &sandbox_name); - sandbox.phase = SandboxPhase::Ready as i32; + sandbox.set_phase(SandboxPhase::Ready as i32); } else if current_phase == SandboxPhase::Ready { ensure_supervisor_not_ready_status(&mut sandbox.status, &sandbox_name); - sandbox.phase = SandboxPhase::Provisioning as i32; + sandbox.set_phase(SandboxPhase::Provisioning as i32); } }) .await; @@ -1221,10 +1240,7 @@ fn driver_sandbox_from_public(sandbox: &Sandbox) -> DriverSandbox { name: sandbox.object_name().to_string(), namespace: String::new(), // Namespace is set by the driver based on its config spec: sandbox.spec.as_ref().map(driver_sandbox_spec_from_public), - status: sandbox - .status - .as_ref() - .map(|status| driver_status_from_public(status, sandbox.phase)), + status: sandbox.status.as_ref().map(driver_status_from_public), } } @@ -1431,7 +1447,7 @@ fn build_platform_resources_config( } } -fn driver_status_from_public(status: &SandboxStatus, phase: i32) -> DriverSandboxStatus { +fn driver_status_from_public(status: &SandboxStatus) -> DriverSandboxStatus { DriverSandboxStatus { sandbox_name: status.sandbox_name.clone(), instance_id: status.agent_pod.clone(), @@ -1442,7 +1458,7 @@ fn driver_status_from_public(status: &SandboxStatus, phase: i32) -> DriverSandbo .iter() .map(driver_condition_from_public) .collect(), - deleting: SandboxPhase::try_from(phase) == Ok(SandboxPhase::Deleting), + deleting: SandboxPhase::try_from(status.phase) == Ok(SandboxPhase::Deleting), } } @@ -1474,7 +1490,11 @@ fn decode_sandbox_record(record: &ObjectRecord) -> Result { Sandbox::decode(record.payload.as_slice()).map_err(|e| e.to_string()) } -fn public_status_from_driver(status: &DriverSandboxStatus) -> SandboxStatus { +fn public_status_from_driver( + status: &DriverSandboxStatus, + phase: SandboxPhase, + current_policy_version: u32, +) -> SandboxStatus { SandboxStatus { sandbox_name: status.sandbox_name.clone(), agent_pod: status.instance_id.clone(), @@ -1485,6 +1505,8 @@ fn public_status_from_driver(status: &DriverSandboxStatus) -> SandboxStatus { .iter() .map(public_condition_from_driver) .collect(), + phase: phase as i32, + current_policy_version, } } @@ -1523,10 +1545,7 @@ fn upsert_ready_condition( ) { let status = status.get_or_insert_with(|| SandboxStatus { sandbox_name: sandbox_name.to_string(), - agent_pod: String::new(), - agent_fd: String::new(), - sandbox_fd: String::new(), - conditions: Vec::new(), + ..Default::default() }); if let Some(existing) = status @@ -1923,7 +1942,7 @@ mod tests { } fn sandbox_record(id: &str, name: &str, phase: SandboxPhase) -> Sandbox { - Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: id.to_string(), name: name.to_string(), @@ -1931,9 +1950,10 @@ mod tests { labels: HashMap::new(), resource_version: 0, }), - phase: phase as i32, ..Default::default() - } + }; + sandbox.set_phase(phase as i32); + sandbox } fn ssh_session_record(id: &str, sandbox_id: &str) -> SshSession { @@ -2212,8 +2232,6 @@ mod tests { let mut status = Some(SandboxStatus { sandbox_name: "test".to_string(), agent_pod: "test-pod".to_string(), - agent_fd: String::new(), - sandbox_fd: String::new(), conditions: vec![SandboxCondition { r#type: "Ready".to_string(), status: "False".to_string(), @@ -2221,6 +2239,7 @@ mod tests { message: "0/1 nodes are available: 1 Insufficient nvidia.com/gpu.".to_string(), last_transition_time: String::new(), }], + ..Default::default() }); rewrite_user_facing_conditions( @@ -2244,8 +2263,6 @@ mod tests { let mut status = Some(SandboxStatus { sandbox_name: "test".to_string(), agent_pod: "test-pod".to_string(), - agent_fd: String::new(), - sandbox_fd: String::new(), conditions: vec![SandboxCondition { r#type: "Ready".to_string(), status: "False".to_string(), @@ -2253,6 +2270,7 @@ mod tests { message: original.to_string(), last_transition_time: String::new(), }], + ..Default::default() }); rewrite_user_facing_conditions( @@ -2316,11 +2334,67 @@ mod tests { .unwrap() .unwrap(); assert_eq!( - SandboxPhase::try_from(stored.phase).unwrap(), + SandboxPhase::try_from(stored.phase()).unwrap(), SandboxPhase::Ready ); } + #[tokio::test] + async fn apply_sandbox_update_without_status_preserves_existing_status() { + let runtime = test_runtime(Arc::new(TestDriver::default())).await; + let mut sandbox = sandbox_record("sb-1", "sandbox-a", SandboxPhase::Ready); + sandbox.status = Some(SandboxStatus { + sandbox_name: "sandbox-a".to_string(), + conditions: vec![SandboxCondition { + r#type: "Ready".to_string(), + status: "True".to_string(), + reason: "DependenciesReady".to_string(), + message: "Pod is Ready".to_string(), + last_transition_time: String::new(), + }], + current_policy_version: 7, + ..Default::default() + }); + sandbox.set_phase(SandboxPhase::Ready as i32); + runtime.store.put_message(&sandbox).await.unwrap(); + + runtime + .apply_sandbox_update(DriverSandbox { + id: "sb-1".to_string(), + name: "sandbox-a".to_string(), + namespace: "default".to_string(), + spec: None, + status: None, + }) + .await + .unwrap(); + + let stored = runtime + .store + .get_message::("sb-1") + .await + .unwrap() + .unwrap(); + assert_eq!( + SandboxPhase::try_from(stored.phase()).unwrap(), + SandboxPhase::Ready + ); + assert_eq!(stored.current_policy_version(), 7); + let ready = stored + .status + .as_ref() + .and_then(|status| { + status + .conditions + .iter() + .find(|condition| condition.r#type == "Ready") + }) + .unwrap(); + assert_eq!(ready.status, "True"); + assert_eq!(ready.reason, "DependenciesReady"); + assert_eq!(ready.message, "Pod is Ready"); + } + #[tokio::test] async fn apply_sandbox_update_promotes_connected_supervisor_session_to_ready() { let runtime = test_runtime(Arc::new(TestDriver::default())).await; @@ -2350,7 +2424,7 @@ mod tests { .unwrap() .unwrap(); assert_eq!( - SandboxPhase::try_from(stored.phase).unwrap(), + SandboxPhase::try_from(stored.phase()).unwrap(), SandboxPhase::Ready ); let ready = stored @@ -2383,7 +2457,7 @@ mod tests { .unwrap() .unwrap(); assert_eq!( - SandboxPhase::try_from(stored.phase).unwrap(), + SandboxPhase::try_from(stored.phase()).unwrap(), SandboxPhase::Ready ); } @@ -2394,9 +2468,6 @@ mod tests { let mut sandbox = sandbox_record("sb-1", "sandbox-a", SandboxPhase::Ready); sandbox.status = Some(SandboxStatus { sandbox_name: "sandbox-a".to_string(), - agent_pod: String::new(), - agent_fd: String::new(), - sandbox_fd: String::new(), conditions: vec![SandboxCondition { r#type: "Ready".to_string(), status: "True".to_string(), @@ -2404,7 +2475,9 @@ mod tests { message: "Supervisor session connected".to_string(), last_transition_time: String::new(), }], + ..Default::default() }); + sandbox.set_phase(SandboxPhase::Ready as i32); runtime.store.put_message(&sandbox).await.unwrap(); runtime @@ -2419,7 +2492,7 @@ mod tests { .unwrap() .unwrap(); assert_eq!( - SandboxPhase::try_from(stored.phase).unwrap(), + SandboxPhase::try_from(stored.phase()).unwrap(), SandboxPhase::Provisioning ); let ready = stored @@ -2505,7 +2578,7 @@ mod tests { .unwrap() .unwrap(); assert_eq!( - SandboxPhase::try_from(stored.phase).unwrap(), + SandboxPhase::try_from(stored.phase()).unwrap(), SandboxPhase::Ready ); assert!(stored.spec.as_ref().is_some_and(|spec| spec.gpu)); @@ -2598,7 +2671,7 @@ mod tests { .unwrap() .unwrap(); assert_eq!( - SandboxPhase::try_from(stored.phase).unwrap(), + SandboxPhase::try_from(stored.phase()).unwrap(), SandboxPhase::Ready ); } @@ -2761,7 +2834,7 @@ mod tests { .unwrap() .unwrap(); assert_eq!( - SandboxPhase::try_from(stored.phase).unwrap(), + SandboxPhase::try_from(stored.phase()).unwrap(), SandboxPhase::Error ); let ready = stored @@ -2793,7 +2866,7 @@ mod tests { .unwrap() .unwrap(); assert_eq!( - SandboxPhase::try_from(stored.phase).unwrap(), + SandboxPhase::try_from(stored.phase()).unwrap(), SandboxPhase::Error ); let ready = stored @@ -2820,7 +2893,7 @@ mod tests { .unwrap() .unwrap(); assert_eq!( - SandboxPhase::try_from(stored.phase).unwrap(), + SandboxPhase::try_from(stored.phase()).unwrap(), SandboxPhase::Ready ); } diff --git a/crates/openshell-server/src/grpc/auth_rpc.rs b/crates/openshell-server/src/grpc/auth_rpc.rs index 8e98b1824..88c771bed 100644 --- a/crates/openshell-server/src/grpc/auth_rpc.rs +++ b/crates/openshell-server/src/grpc/auth_rpc.rs @@ -193,7 +193,7 @@ mod tests { } async fn insert_sandbox(state: &Arc, sandbox_id: &str) { - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(ObjectMeta { id: sandbox_id.to_string(), name: sandbox_id.to_string(), @@ -205,9 +205,9 @@ mod tests { policy: None, ..Default::default() }), - phase: SandboxPhase::Ready as i32, ..Default::default() }; + sandbox.set_phase(SandboxPhase::Ready as i32); state.store.put_message(&sandbox).await.unwrap(); } diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index bdc96d862..2abc670fd 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -1166,7 +1166,7 @@ pub(super) async fn handle_get_sandbox_policy_status( .ok_or_else(|| Status::not_found("sandbox not found"))?; ( sandbox.object_id().to_string(), - sandbox.current_policy_version, + sandbox.current_policy_version(), ) }; @@ -1295,7 +1295,7 @@ pub(super) async fn handle_report_policy_status( state .store .update_message_cas::(&req.sandbox_id, 0, |sandbox| { - sandbox.current_policy_version = version_to_set; + sandbox.set_current_policy_version(version_to_set); }) .await .map_err(|e| super::persistence_error_to_status(e, "update current_policy_version"))?; @@ -3037,7 +3037,7 @@ mod tests { // Two sandboxes; the caller is principal of A, the request body // references B. for (id, name) in [("sb-a", "sandbox-a"), ("sb-b", "sandbox-b")] { - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: id.to_string(), name: name.to_string(), @@ -3049,9 +3049,9 @@ mod tests { policy: None, ..Default::default() }), - phase: SandboxPhase::Provisioning as i32, ..Default::default() }; + sandbox.set_phase(SandboxPhase::Provisioning as i32); state.store.put_message(&sandbox).await.unwrap(); } let req = with_sandbox( @@ -3070,7 +3070,7 @@ mod tests { async fn same_sandbox_get_sandbox_config_allowed() { use openshell_core::proto::{SandboxPhase, SandboxSpec}; let state = test_server_state().await; - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: "sb-self".to_string(), name: "self".to_string(), @@ -3082,9 +3082,9 @@ mod tests { policy: None, ..Default::default() }), - phase: SandboxPhase::Provisioning as i32, ..Default::default() }; + sandbox.set_phase(SandboxPhase::Provisioning as i32); state.store.put_message(&sandbox).await.unwrap(); let req = with_sandbox( Request::new(GetSandboxConfigRequest { @@ -3102,7 +3102,7 @@ mod tests { use openshell_core::proto::{SandboxPhase, SandboxSpec}; let state = test_server_state().await; for (id, name) in [("sb-a", "sandbox-a"), ("sb-b", "sandbox-b")] { - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: id.to_string(), name: name.to_string(), @@ -3114,9 +3114,9 @@ mod tests { policy: None, ..Default::default() }), - phase: SandboxPhase::Provisioning as i32, ..Default::default() }; + sandbox.set_phase(SandboxPhase::Provisioning as i32); state.store.put_message(&sandbox).await.unwrap(); } let req = with_sandbox( @@ -3137,7 +3137,7 @@ mod tests { use openshell_core::proto::{SandboxPhase, SandboxSpec}; let state = test_server_state().await; for (id, name) in [("sb-a", "sandbox-a"), ("sb-b", "sandbox-b")] { - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: id.to_string(), name: name.to_string(), @@ -3149,9 +3149,9 @@ mod tests { policy: None, ..Default::default() }), - phase: SandboxPhase::Provisioning as i32, ..Default::default() }; + sandbox.set_phase(SandboxPhase::Provisioning as i32); state.store.put_message(&sandbox).await.unwrap(); } let req = with_sandbox( @@ -3224,7 +3224,7 @@ mod tests { // RBAC was the user gate; the IDOR guard must NOT trip for users. use openshell_core::proto::{SandboxPhase, SandboxSpec}; let state = test_server_state().await; - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: "sb-x".to_string(), name: "x".to_string(), @@ -3236,9 +3236,9 @@ mod tests { policy: None, ..Default::default() }), - phase: SandboxPhase::Provisioning as i32, ..Default::default() }; + sandbox.set_phase(SandboxPhase::Provisioning as i32); state.store.put_message(&sandbox).await.unwrap(); let req = with_user(Request::new(GetSandboxConfigRequest { sandbox_id: "sb-x".to_string(), @@ -3291,7 +3291,7 @@ mod tests { let store = test_store().await; - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: "sb-no-policy".to_string(), name: "no-policy-sandbox".to_string(), @@ -3303,9 +3303,9 @@ mod tests { policy: None, ..Default::default() }), - phase: SandboxPhase::Provisioning as i32, ..Default::default() }; + sandbox.set_phase(SandboxPhase::Provisioning as i32); store.put_message(&sandbox).await.unwrap(); let loaded = store @@ -3360,7 +3360,7 @@ mod tests { ) -> Sandbox { use openshell_core::proto::{SandboxPhase, SandboxSpec}; - Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: id.to_string(), name: name.to_string(), @@ -3373,9 +3373,10 @@ mod tests { providers, ..Default::default() }), - phase: SandboxPhase::Ready as i32, ..Default::default() - } + }; + sandbox.set_phase(SandboxPhase::Ready as i32); + sandbox } async fn enable_providers_v2(state: &Arc) { @@ -4246,7 +4247,7 @@ mod tests { .collect(), ..Default::default() }; - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: "sb-global-profile".to_string(), name: "global-profile-sandbox".to_string(), @@ -4259,9 +4260,9 @@ mod tests { providers: vec!["work-github".to_string()], ..Default::default() }), - phase: SandboxPhase::Ready as i32, ..Default::default() }; + sandbox.set_phase(SandboxPhase::Ready as i32); state.store.put_message(&sandbox).await.unwrap(); let global_policy = SandboxPolicy { @@ -4335,7 +4336,7 @@ mod tests { let store = test_store().await; - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: "sb-backfill".to_string(), name: "backfill-sandbox".to_string(), @@ -4347,9 +4348,9 @@ mod tests { policy: None, ..Default::default() }), - phase: SandboxPhase::Provisioning as i32, ..Default::default() }; + sandbox.set_phase(SandboxPhase::Provisioning as i32); store.put_message(&sandbox).await.unwrap(); let new_policy = ProtoSandboxPolicy { @@ -4397,7 +4398,7 @@ mod tests { }; let state = test_server_state().await; - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: "sb-draft-flow".to_string(), name: "draft-flow".to_string(), @@ -4409,9 +4410,9 @@ mod tests { policy: None, ..Default::default() }), - phase: SandboxPhase::Ready as i32, ..Default::default() }; + sandbox.set_phase(SandboxPhase::Ready as i32); state.store.put_message(&sandbox).await.unwrap(); let sandbox_name = sandbox.object_name().to_string(); @@ -4608,7 +4609,7 @@ mod tests { let state = test_server_state().await; let sandbox_name = "agent-feedback-loop".to_string(); - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: "sb-feedback".to_string(), name: sandbox_name.clone(), @@ -4620,9 +4621,9 @@ mod tests { policy: None, ..Default::default() }), - phase: SandboxPhase::Ready as i32, ..Default::default() }; + sandbox.set_phase(SandboxPhase::Ready as i32); state.store.put_message(&sandbox).await.unwrap(); let proposed_rule = NetworkPolicyRule { @@ -4706,7 +4707,7 @@ mod tests { let state = test_server_state().await; let sandbox_name = "redraft-loop".to_string(); - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: "sb-redraft".to_string(), name: sandbox_name.clone(), @@ -4718,9 +4719,9 @@ mod tests { policy: None, ..Default::default() }), - phase: SandboxPhase::Ready as i32, ..Default::default() }; + sandbox.set_phase(SandboxPhase::Ready as i32); state.store.put_message(&sandbox).await.unwrap(); // Two proposals with the same host|port|binary (so the mechanistic @@ -4820,7 +4821,7 @@ mod tests { let state = test_server_state().await; let sandbox_name = "mechanistic-dedup".to_string(); - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: "sb-mech-dedup".to_string(), name: sandbox_name.clone(), @@ -4832,9 +4833,9 @@ mod tests { policy: None, ..Default::default() }), - phase: SandboxPhase::Ready as i32, ..Default::default() }; + sandbox.set_phase(SandboxPhase::Ready as i32); state.store.put_message(&sandbox).await.unwrap(); let proposed_rule = NetworkPolicyRule { @@ -4920,7 +4921,7 @@ mod tests { let state = test_server_state().await; let sandbox_name = "undo-clears-reason".to_string(); - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: "sb-undo-clears".to_string(), name: sandbox_name.clone(), @@ -4932,9 +4933,9 @@ mod tests { policy: None, ..Default::default() }), - phase: SandboxPhase::Ready as i32, ..Default::default() }; + sandbox.set_phase(SandboxPhase::Ready as i32); state.store.put_message(&sandbox).await.unwrap(); let proposed_rule = NetworkPolicyRule { @@ -5026,7 +5027,7 @@ mod tests { use openshell_core::proto::{NetworkBinary, NetworkEndpoint, SandboxPhase, SandboxSpec}; let state = test_server_state().await; - let sandbox_a = Sandbox { + let mut sandbox_a = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: "sb-draft-owner".to_string(), name: "draft-owner".to_string(), @@ -5038,10 +5039,10 @@ mod tests { policy: None, ..Default::default() }), - phase: SandboxPhase::Ready as i32, ..Default::default() }; - let sandbox_b = Sandbox { + sandbox_a.set_phase(SandboxPhase::Ready as i32); + let mut sandbox_b = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: "sb-draft-other".to_string(), name: "draft-other".to_string(), @@ -5053,9 +5054,9 @@ mod tests { policy: None, ..Default::default() }), - phase: SandboxPhase::Ready as i32, ..Default::default() }; + sandbox_b.set_phase(SandboxPhase::Ready as i32); state.store.put_message(&sandbox_a).await.unwrap(); state.store.put_message(&sandbox_b).await.unwrap(); @@ -6449,7 +6450,7 @@ mod tests { // Create a sandbox WITHOUT a policy (spec.policy = None) // This simulates a sandbox before the supervisor has discovered and synced a policy - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: "sb-1".to_string(), name: "test-sandbox".to_string(), @@ -6462,9 +6463,9 @@ mod tests { providers: Vec::new(), ..Default::default() }), - phase: SandboxPhase::Provisioning as i32, ..Default::default() }; + sandbox.set_phase(SandboxPhase::Provisioning as i32); state.store.put_message(&sandbox).await.unwrap(); // Fetch the sandbox to get its current resource_version @@ -6524,7 +6525,7 @@ mod tests { let state = test_server_state().await; // Create a sandbox WITHOUT a policy - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: "sb-1".to_string(), name: "test-sandbox".to_string(), @@ -6537,9 +6538,9 @@ mod tests { providers: Vec::new(), ..Default::default() }), - phase: SandboxPhase::Provisioning as i32, ..Default::default() }; + sandbox.set_phase(SandboxPhase::Provisioning as i32); state.store.put_message(&sandbox).await.unwrap(); // Get current version @@ -6605,7 +6606,7 @@ mod tests { let state = Arc::new(test_server_state().await); // Create a sandbox WITHOUT a policy - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: "sb-1".to_string(), name: "test-sandbox".to_string(), @@ -6618,9 +6619,9 @@ mod tests { providers: Vec::new(), ..Default::default() }), - phase: SandboxPhase::Provisioning as i32, ..Default::default() }; + sandbox.set_phase(SandboxPhase::Provisioning as i32); state.store.put_message(&sandbox).await.unwrap(); // All three clients fetch the sandbox and see the same version diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 3ddaae037..c58c0c9b7 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -3623,7 +3623,7 @@ mod tests { .await .unwrap(); - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: "sandbox-001".to_string(), name: "test-sandbox".to_string(), @@ -3636,9 +3636,8 @@ mod tests { ..SandboxSpec::default() }), status: None, - phase: SandboxPhase::Ready as i32, - ..Default::default() }; + sandbox.set_phase(SandboxPhase::Ready as i32); store.put_message(&sandbox).await.unwrap(); let loaded = store @@ -3660,7 +3659,7 @@ mod tests { let store = test_store().await; - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: "sandbox-002".to_string(), name: "empty-sandbox".to_string(), @@ -3670,9 +3669,8 @@ mod tests { }), spec: Some(SandboxSpec::default()), status: None, - phase: SandboxPhase::Ready as i32, - ..Default::default() }; + sandbox.set_phase(SandboxPhase::Ready as i32); store.put_message(&sandbox).await.unwrap(); let loaded = store diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 1855972d7..8156a1005 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -107,7 +107,7 @@ pub(super) async fn handle_create_sandbox( let now_ms = current_time_ms(); - let sandbox = Sandbox { + let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: id.clone(), name: name.clone(), @@ -117,9 +117,8 @@ pub(super) async fn handle_create_sandbox( }), spec: Some(spec), status: None, - phase: SandboxPhase::Provisioning as i32, - current_policy_version: 0, }; + sandbox.set_phase(SandboxPhase::Provisioning as i32); // Ensure metadata is valid (defense in depth - should always be true for server-constructed metadata) super::validation::validate_object_metadata(sandbox.metadata.as_ref(), "sandbox")?; @@ -560,7 +559,7 @@ pub(super) async fn handle_watch_sandbox( if stop_on_terminal { let phase = - SandboxPhase::try_from(sandbox.phase).unwrap_or(SandboxPhase::Unknown); + SandboxPhase::try_from(sandbox.phase()).unwrap_or(SandboxPhase::Unknown); if phase == SandboxPhase::Ready { return; } @@ -630,7 +629,7 @@ pub(super) async fn handle_watch_sandbox( return; } if stop_on_terminal { - let phase = SandboxPhase::try_from(sandbox.phase).unwrap_or(SandboxPhase::Unknown); + let phase = SandboxPhase::try_from(sandbox.phase()).unwrap_or(SandboxPhase::Unknown); if phase == SandboxPhase::Ready { return; } @@ -733,7 +732,7 @@ pub(super) async fn handle_exec_sandbox( .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found"))?; - if SandboxPhase::try_from(sandbox.phase).ok() != Some(SandboxPhase::Ready) { + if SandboxPhase::try_from(sandbox.phase()).ok() != Some(SandboxPhase::Ready) { return Err(Status::failed_precondition("sandbox is not ready")); } @@ -847,7 +846,7 @@ pub(super) async fn handle_forward_tcp( .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found"))?; - if SandboxPhase::try_from(sandbox.phase).ok() != Some(SandboxPhase::Ready) { + if SandboxPhase::try_from(sandbox.phase()).ok() != Some(SandboxPhase::Ready) { return Err(Status::failed_precondition("sandbox is not ready")); } @@ -1176,7 +1175,7 @@ pub(super) async fn handle_exec_sandbox_interactive( .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found"))?; - if SandboxPhase::try_from(sandbox.phase).ok() != Some(SandboxPhase::Ready) { + if SandboxPhase::try_from(sandbox.phase()).ok() != Some(SandboxPhase::Ready) { return Err(Status::failed_precondition("sandbox is not ready")); } @@ -1249,7 +1248,7 @@ pub(super) async fn handle_create_ssh_session( .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found"))?; - if SandboxPhase::try_from(sandbox.phase).ok() != Some(SandboxPhase::Ready) { + if SandboxPhase::try_from(sandbox.phase()).ok() != Some(SandboxPhase::Ready) { return Err(Status::failed_precondition("sandbox is not ready")); } @@ -2091,7 +2090,7 @@ mod tests { } fn test_sandbox(name: &str, providers: Vec) -> Sandbox { - Sandbox { + let mut sandbox = Sandbox { metadata: Some(ObjectMeta { id: format!("sandbox-{name}"), name: name.to_string(), @@ -2105,10 +2104,11 @@ mod tests { providers, ..Default::default() }), - phase: SandboxPhase::Ready as i32, - current_policy_version: 7, ..Default::default() - } + }; + sandbox.set_phase(SandboxPhase::Ready as i32); + sandbox.set_current_policy_version(7); + sandbox } #[tokio::test] @@ -2144,11 +2144,11 @@ mod tests { .await .unwrap() .unwrap(); + assert_eq!(sandbox.phase(), SandboxPhase::Ready as i32); + assert_eq!(sandbox.current_policy_version(), 7); let spec = sandbox.spec.unwrap(); assert_eq!(spec.providers, vec!["work-github"]); assert_eq!(spec.log_level, "debug"); - assert_eq!(sandbox.phase, SandboxPhase::Ready as i32); - assert_eq!(sandbox.current_policy_version, 7); } #[tokio::test] @@ -2429,7 +2429,7 @@ mod tests { async fn interactive_exec_rejects_sandbox_not_ready() { let state = test_server_state().await; let mut sandbox = test_sandbox("not-ready", Vec::new()); - sandbox.phase = SandboxPhase::Provisioning as i32; + sandbox.set_phase(SandboxPhase::Provisioning as i32); state.store.put_message(&sandbox).await.unwrap(); let stored = state @@ -2439,7 +2439,7 @@ mod tests { .unwrap() .unwrap(); assert_ne!( - SandboxPhase::try_from(stored.phase).ok(), + SandboxPhase::try_from(stored.phase()).ok(), Some(SandboxPhase::Ready) ); } diff --git a/crates/openshell-server/src/grpc/service.rs b/crates/openshell-server/src/grpc/service.rs index 4d73f2279..a8144b4c7 100644 --- a/crates/openshell-server/src/grpc/service.rs +++ b/crates/openshell-server/src/grpc/service.rs @@ -279,22 +279,19 @@ mod tests { use openshell_core::proto::SandboxPhase; async fn seed_sandbox(state: &Arc, name: &str) { - state - .store - .put_message(&Sandbox { - metadata: Some(ObjectMeta { - id: format!("sandbox-{name}"), - name: name.to_string(), - created_at_ms: 1_000, - labels: HashMap::new(), - resource_version: 0, - }), - spec: Some(openshell_core::proto::SandboxSpec::default()), - phase: SandboxPhase::Ready as i32, - ..Default::default() - }) - .await - .unwrap(); + let mut sandbox = Sandbox { + metadata: Some(ObjectMeta { + id: format!("sandbox-{name}"), + name: name.to_string(), + created_at_ms: 1_000, + labels: HashMap::new(), + resource_version: 0, + }), + spec: Some(openshell_core::proto::SandboxSpec::default()), + ..Default::default() + }; + sandbox.set_phase(SandboxPhase::Ready as i32); + state.store.put_message(&sandbox).await.unwrap(); } #[test] diff --git a/crates/openshell-server/src/persistence/tests.rs b/crates/openshell-server/src/persistence/tests.rs index 123feb862..bd14c39f9 100644 --- a/crates/openshell-server/src/persistence/tests.rs +++ b/crates/openshell-server/src/persistence/tests.rs @@ -1252,8 +1252,6 @@ async fn cas_update_message_cas_succeeds() { }), spec: None, status: None, - phase: 0, - current_policy_version: 0, }; store.put_message(&sandbox).await.unwrap(); @@ -1261,14 +1259,14 @@ async fn cas_update_message_cas_succeeds() { // Update using CAS with expected_version = 0 (use current version) let updated = store .update_message_cas::("test-id", 0, |s| { - s.phase = 2; // Set to Ready - s.current_policy_version = 42; + s.set_phase(2); // Set to Ready + s.set_current_policy_version(42); }) .await .unwrap(); - assert_eq!(updated.phase, 2); - assert_eq!(updated.current_policy_version, 42); + assert_eq!(updated.phase(), 2); + assert_eq!(updated.current_policy_version(), 42); assert_eq!( updated.metadata.as_ref().map_or(0, |m| m.resource_version), 2 @@ -1293,8 +1291,6 @@ async fn cas_update_message_cas_conflicts_on_concurrent_updates() { }), spec: None, status: None, - phase: 0, - current_policy_version: 0, }; store.put_message(&sandbox).await.unwrap(); @@ -1308,7 +1304,7 @@ async fn cas_update_message_cas_conflicts_on_concurrent_updates() { let handle = tokio::spawn(async move { store .update_message_cas::("test-id", 1, |s| { - s.current_policy_version = i; + s.set_current_policy_version(i); }) .await }); diff --git a/crates/openshell-server/src/service_routing.rs b/crates/openshell-server/src/service_routing.rs index 4b99a8ef7..7ebd6dba9 100644 --- a/crates/openshell-server/src/service_routing.rs +++ b/crates/openshell-server/src/service_routing.rs @@ -264,7 +264,7 @@ async fn proxy_to_endpoint( return Err(route_err); } }; - if SandboxPhase::try_from(sandbox.phase).ok() != Some(SandboxPhase::Ready) { + if SandboxPhase::try_from(sandbox.phase()).ok() != Some(SandboxPhase::Ready) { let err = ServiceRouteError::sandbox_not_ready(); emit_service_http_failure( &state, diff --git a/crates/openshell-tui/src/lib.rs b/crates/openshell-tui/src/lib.rs index 1969715ce..22201dbb6 100644 --- a/crates/openshell-tui/src/lib.rs +++ b/crates/openshell-tui/src/lib.rs @@ -20,6 +20,7 @@ use crossterm::terminal::{ use miette::{IntoDiagnostic, Result}; use openshell_core::auth::EdgeAuthInterceptor; use openshell_core::metadata::{ObjectId, ObjectLabels, ObjectName}; +use openshell_core::proto::SandboxPhase; use openshell_core::proto::open_shell_client::OpenShellClient; use ratatui::Terminal; use ratatui::backend::CrosstermBackend; @@ -1418,10 +1419,10 @@ fn spawn_create_sandbox(app: &mut App, tx: mpsc::UnboundedSender) { if let Ok(resp) = client.get_sandbox(req).await && let Some(sandbox) = resp.into_inner().sandbox { - if sandbox.phase == 2 { + if sandbox.phase() == SandboxPhase::Ready as i32 { break sandbox.object_id().to_string(); } - if sandbox.phase == 3 { + if sandbox.phase() == SandboxPhase::Error as i32 { let _ = tx.send(Event::CreateResult(Err( "sandbox entered error state".to_string() ))); @@ -2274,7 +2275,7 @@ async fn refresh_sandboxes(app: &mut App) { .iter() .map(|s| s.object_name().to_string()) .collect(); - app.sandbox_phases = sandboxes.iter().map(|s| phase_label(s.phase)).collect(); + app.sandbox_phases = sandboxes.iter().map(|s| phase_label(s.phase())).collect(); app.sandbox_images = sandboxes .iter() .map(|s| { @@ -2304,8 +2305,10 @@ async fn refresh_sandboxes(app: &mut App) { }) .collect(); - app.sandbox_policy_versions = - sandboxes.iter().map(|s| s.current_policy_version).collect(); + app.sandbox_policy_versions = sandboxes + .iter() + .map(openshell_core::proto::Sandbox::current_policy_version) + .collect(); // Build NOTES column from active port forwards. let forwards = openshell_core::forward::list_forwards().unwrap_or_default(); @@ -2428,10 +2431,10 @@ async fn refresh_sandbox_draft_counts(app: &mut App) { fn phase_label(phase: i32) -> String { match phase { - 1 => "Provisioning", - 2 => "Ready", - 3 => "Error", - 4 => "Deleting", + x if x == SandboxPhase::Provisioning as i32 => "Provisioning", + x if x == SandboxPhase::Ready as i32 => "Ready", + x if x == SandboxPhase::Error as i32 => "Error", + x if x == SandboxPhase::Deleting as i32 => "Deleting", _ => "Unknown", } .to_string() diff --git a/proto/openshell.proto b/proto/openshell.proto index 90d1594f7..02dbbe283 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -298,10 +298,9 @@ message Sandbox { SandboxSpec spec = 2; // Latest user-facing observed status derived by the gateway. SandboxStatus status = 3; - // Gateway-derived lifecycle summary. - SandboxPhase phase = 4; - // Currently active policy version (updated when sandbox reports loaded). - uint32 current_policy_version = 5; + + reserved 4, 5; + reserved "phase", "current_policy_version"; } // Desired sandbox configuration provided through the public API. @@ -352,8 +351,7 @@ message SandboxTemplate { // User-facing sandbox status derived by the gateway from compute-driver observations. // -// Lifecycle summary is exposed separately as `Sandbox.phase`. Public status does -// not embed driver-only flags such as `deleting`. +// Public status does not embed driver-only flags such as `deleting`. message SandboxStatus { // Compute-platform sandbox object name. string sandbox_name = 1; @@ -365,6 +363,10 @@ message SandboxStatus { string sandbox_fd = 4; // Latest user-facing readiness and lifecycle conditions. repeated SandboxCondition conditions = 5; + // Gateway-derived lifecycle summary. + SandboxPhase phase = 6; + // Currently active policy version (updated when sandbox reports loaded). + uint32 current_policy_version = 7; } // User-facing sandbox condition derived from driver-native conditions. diff --git a/python/openshell/__init__.py b/python/openshell/__init__.py index 27f8aaae9..94a391d4e 100644 --- a/python/openshell/__init__.py +++ b/python/openshell/__init__.py @@ -15,6 +15,7 @@ SandboxError, SandboxRef, SandboxSession, + SandboxStatusRef, TlsConfig, ) @@ -35,6 +36,7 @@ "SandboxError", "SandboxRef", "SandboxSession", + "SandboxStatusRef", "TlsConfig", "__version__", ] diff --git a/python/openshell/sandbox.py b/python/openshell/sandbox.py index 11cdbd98b..80a2f674d 100644 --- a/python/openshell/sandbox.py +++ b/python/openshell/sandbox.py @@ -34,11 +34,25 @@ class TlsConfig: key_path: pathlib.Path +@dataclass(frozen=True) +class SandboxStatusRef: + phase: int + current_policy_version: int + + @dataclass(frozen=True) class SandboxRef: id: str name: str - phase: int + status: SandboxStatusRef + + @property + def phase(self) -> int: + return self.status.phase + + @property + def current_policy_version(self) -> int: + return self.status.current_policy_version @dataclass(frozen=True) @@ -261,9 +275,9 @@ def wait_ready( deadline = time.time() + timeout_seconds while time.time() < deadline: sandbox = self.get(sandbox_name) - if sandbox.phase == openshell_pb2.SANDBOX_PHASE_READY: + if sandbox.status.phase == openshell_pb2.SANDBOX_PHASE_READY: return sandbox - if sandbox.phase == openshell_pb2.SANDBOX_PHASE_ERROR: + if sandbox.status.phase == openshell_pb2.SANDBOX_PHASE_ERROR: raise SandboxError(f"sandbox {sandbox_name} entered error phase") time.sleep(1) raise SandboxError(f"sandbox {sandbox_name} was not ready within timeout") @@ -585,10 +599,14 @@ def _serialize_python_callable( def _sandbox_ref(sandbox: openshell_pb2.Sandbox) -> SandboxRef: + status = sandbox.status if sandbox.HasField("status") else None return SandboxRef( id=sandbox.metadata.id if sandbox.metadata else "", name=sandbox.metadata.name if sandbox.metadata else "", - phase=sandbox.phase, + status=SandboxStatusRef( + phase=status.phase if status else 0, + current_policy_version=status.current_policy_version if status else 0, + ), )