From f60516235442c99cb14490b2758e5f7a882b029d Mon Sep 17 00:00:00 2001 From: zm-oai Date: Mon, 12 Oct 2026 16:48:21 +0000 Subject: [PATCH] Preserve Windows runtime registration failure diagnostics (#53426) ## Why Managed runtime package registration errors lacked the terminal status, separate async and deployment error codes, and deployment activity ID needed to diagnose failures. ## What changed Include these diagnostics in registration errors and distinguish failures in `GetResults`, the `ExtendedErrorCode` getter, and deployment itself. Preserve the original completion error; failures to read diagnostic fields do not turn successful registration into failure. Keep free-form Windows error text out of diagnostic summaries. ## Testing Add unit tests for distinct async and deployment errors, failed or canceled result retrieval, and diagnostic getter failures, including preservation of native error codes and omission of free-form diagnostic text. GitOrigin-RevId: 7f3a46741304ef04f5c3e2aa575bcfc4392dff62 --- .../src/registered_runtime.rs | 19 ++-- .../registered_runtime/registration_result.rs | 80 ++++++++++++++++ .../registration_result_tests.rs | 93 +++++++++++++++++++ 3 files changed, 183 insertions(+), 9 deletions(-) create mode 100644 codex-rs/windows-sandbox-service/src/registered_runtime/registration_result.rs create mode 100644 codex-rs/windows-sandbox-service/src/registered_runtime/registration_result_tests.rs diff --git a/codex-rs/windows-sandbox-service/src/registered_runtime.rs b/codex-rs/windows-sandbox-service/src/registered_runtime.rs index cab3386bd3..abf1468d51 100644 --- a/codex-rs/windows-sandbox-service/src/registered_runtime.rs +++ b/codex-rs/windows-sandbox-service/src/registered_runtime.rs @@ -39,6 +39,7 @@ use crate::package_lifecycle::with_owner_impersonation; mod known_folder; mod metadata; +mod registration_result; mod removal; pub(crate) use metadata::remove as remove_metadata; @@ -168,7 +169,7 @@ pub(crate) fn provision( // As with provisioning, retain the request's resources until Windows finishes. // A client timeout or an unreadable status cannot safely release the profile. let mut status_error_logged = false; - loop { + let status = loop { if shutdown.load(Ordering::Acquire) { // The typed error bypasses all response conversion and cleanup. Keep the // in-flight operation and its profile alive until this own-process service exits. @@ -177,8 +178,12 @@ pub(crate) fn provision( } match operation.Status() { Ok(AsyncStatus::Started) => {} - Ok(AsyncStatus::Completed | AsyncStatus::Canceled | AsyncStatus::Error) => { - break; + Ok( + status @ (AsyncStatus::Completed + | AsyncStatus::Canceled + | AsyncStatus::Error), + ) => { + break status; } status => { if !status_error_logged { @@ -193,12 +198,8 @@ pub(crate) fn provision( } } std::thread::sleep(Duration::from_millis(50)); - } - operation - .GetResults() - .and_then(|result| result.ExtendedErrorCode()) - .and_then(windows::core::HRESULT::ok) - .context("finish managed runtime package registration")?; + }; + registration_result::RegistrationResult::read(&operation, status).check()?; ensure!( registered_packages(&profile.user_sid, &family) .context("verify managed runtime package registration")? diff --git a/codex-rs/windows-sandbox-service/src/registered_runtime/registration_result.rs b/codex-rs/windows-sandbox-service/src/registered_runtime/registration_result.rs new file mode 100644 index 0000000000..e186b4b34a --- /dev/null +++ b/codex-rs/windows-sandbox-service/src/registered_runtime/registration_result.rs @@ -0,0 +1,80 @@ +//! Retains the distinct native registration failure codes and deployment activity ID. +//! Diagnostic getters never replace the completion error or turn success into failure. +//! Free-form Windows ErrorText stays in Windows deployment diagnostics, not this summary. + +use anyhow::Context; +use anyhow::Result; +use windows::Foundation::AsyncStatus; +use windows::Foundation::IAsyncOperationWithProgress; +use windows::Management::Deployment::DeploymentProgress; +use windows::Management::Deployment::DeploymentResult; +use windows::core::GUID; +use windows::core::HRESULT; + +pub(super) struct RegistrationResult { + status: AsyncStatus, + async_error: windows::core::Result, + // Keep GetResults failure separate from failure to read ExtendedErrorCode. + result: windows::core::Result, +} + +struct DeploymentDetails { + extended_error: windows::core::Result, + activity_id: windows::core::Result, +} + +impl RegistrationResult { + /// The caller must first observe a terminal status while retaining the profile/token. + pub(super) fn read( + operation: &IAsyncOperationWithProgress, + status: AsyncStatus, + ) -> Self { + Self { + status, + async_error: operation.ErrorCode(), + result: operation.GetResults().map(|result| DeploymentDetails { + extended_error: result.ExtendedErrorCode(), + activity_id: result.ActivityId(), + }), + } + } + + pub(super) fn check(self) -> Result<()> { + let Self { + status, + async_error, + result, + } = self; + let async_error = match async_error { + Ok(code) => format!("{code:?}"), + Err(error) => format!("unavailable({:?})", error.code()), + }; + let context = format!( + "finish managed runtime package registration: status={} async_error={async_error}", + status.0, + ); + let details = result.map_err(|error| { + let summary = format!("{context} phase=GetResults get_results={:?}", error.code()); + anyhow::Error::new(error).context(summary) + })?; + let activity_id = match details.activity_id { + Ok(id) => format!("{id:?}"), + Err(error) => format!("unavailable({:?})", error.code()), + }; + let context = format!("{context} get_results=0x00000000 activity_id={activity_id}"); + let extended_error = details.extended_error.map_err(|error| { + let summary = format!( + "{context} phase=ExtendedErrorCode getter_error={:?}", + error.code() + ); + anyhow::Error::new(error).context(summary) + })?; + extended_error.ok().with_context(|| { + format!("{context} phase=deployment extended_error={extended_error:?}") + }) + } +} + +#[cfg(test)] +#[path = "registration_result_tests.rs"] +mod tests; diff --git a/codex-rs/windows-sandbox-service/src/registered_runtime/registration_result_tests.rs b/codex-rs/windows-sandbox-service/src/registered_runtime/registration_result_tests.rs new file mode 100644 index 0000000000..5586b1fe6c --- /dev/null +++ b/codex-rs/windows-sandbox-service/src/registered_runtime/registration_result_tests.rs @@ -0,0 +1,93 @@ +//! Completion diagnostics preserve native errors without depending on account mutation. + +use super::*; +use pretty_assertions::assert_eq; + +const ACCESS_DENIED: HRESULT = HRESULT(0x80070005_u32 as i32); +const CANCELED: HRESULT = HRESULT(0x800704C7_u32 as i32); +const ASYNC_FAILURE: HRESULT = HRESULT(0x80073D19_u32 as i32); +const DEPLOYMENT_FAILURE: HRESULT = HRESULT(0x80073D28_u32 as i32); + +#[test] +fn retains_different_async_and_deployment_errors_with_activity() { + let activity_id = GUID::from_u128(0x1b60fffa_59b3_0002_d8d5_611bb359dd01); + let error = RegistrationResult { + status: AsyncStatus::Error, + async_error: Ok(ASYNC_FAILURE), + result: Ok(DeploymentDetails { + extended_error: Ok(DEPLOYMENT_FAILURE), + activity_id: Ok(activity_id), + }), + } + .check() + .unwrap_err(); + assert_eq!( + error.to_string(), + format!( + "finish managed runtime package registration: status=3 async_error={ASYNC_FAILURE:?} get_results=0x00000000 activity_id={activity_id:?} phase=deployment extended_error={DEPLOYMENT_FAILURE:?}" + ) + ); + assert_eq!( + error.downcast_ref::().unwrap().code(), + DEPLOYMENT_FAILURE + ); +} + +#[test] +fn get_results_errors_keep_original_hresult_and_phase() { + for (status, code) in [ + (AsyncStatus::Error, ACCESS_DENIED), + (AsyncStatus::Canceled, CANCELED), + ] { + let error = RegistrationResult { + status, + async_error: Ok(code), + result: Err(windows::core::Error::from_hresult(code)), + } + .check() + .unwrap_err(); + assert_eq!( + error.to_string(), + format!( + "finish managed runtime package registration: status={} async_error={code:?} phase=GetResults get_results={code:?}", + status.0 + ) + ); + assert_eq!( + error.downcast_ref::().unwrap().code(), + code + ); + } +} + +#[test] +fn diagnostic_getter_failures_do_not_mask_completion_or_leak_text() { + let private_error = || windows::core::Error::new(ACCESS_DENIED, "private deployment path"); + for code in [HRESULT(0), CANCELED] { + let result = RegistrationResult { + status: AsyncStatus::Completed, + async_error: Err(private_error()), + result: Ok(DeploymentDetails { + extended_error: code.ok().map(|()| code), + activity_id: Err(private_error()), + }), + } + .check(); + if code.is_err() { + let error = result.unwrap_err(); + assert_eq!( + error.to_string(), + format!( + "finish managed runtime package registration: status=1 async_error=unavailable({ACCESS_DENIED:?}) get_results=0x00000000 activity_id=unavailable({ACCESS_DENIED:?}) phase=ExtendedErrorCode getter_error={CANCELED:?}" + ) + ); + assert_eq!( + error.downcast_ref::().unwrap().code(), + CANCELED + ); + assert!(!format!("{error:#}").contains("private deployment path")); + } else { + assert_eq!(result.unwrap(), ()); + } + } +}