fix(serve): fail the Windows managed launch when the engine dies at startup

apps/rocm/src/runtimes_cmd.rs

@@ -2175,7 +2175,7 @@ pub(crate) mod tests {
 
     /// Register both runtimes and make `OLD_RUNTIME_KEY` the active one, which
     /// is the state every "switch away from it" test starts from.
-    fn activate_old_runtime(paths: &AppPaths) -> Result<RocmCliConfig> {
+    pub(crate) fn activate_old_runtime(paths: &AppPaths) -> Result<RocmCliConfig> {
         write_test_pip_runtime(
             paths,
             OLD_RUNTIME_KEY,

apps/rocm/src/serve_cmd.rs

@@ -42,7 +42,8 @@ use crate::serve_summary;
 use crate::service_launch::{
     drop_orphaned_endpoint_key_on_already_running, ensure_background_helper_running_quiet,
     ensure_public_bind_engine_supported, print_managed_launch_plain, resolve_endpoint_auth,
-    run_attached_service, start_managed_service, validate_bind_host,
+    run_attached_service, scripted_managed_engine_startup_failure, start_managed_service,
+    validate_bind_host,
 };
 use crate::{
     cli_progress, collect_serve_notes, detect_gpu_count, device_policy_name, engine_request,
@@ -670,8 +671,12 @@ pub(crate) fn serve(args: ServeArgs) -> Result<()> {
     // test reaches Lemonade's backend boundary without real GPU hardware.
     let scripted_backend_failure = cfg!(feature = "e2e-test-hooks")
         && std::env::var_os("ROCM_E2E_LEMONADE_BACKEND_INSTALL_FAILURE").is_some();
+    // The scripted startup-death scenario bypasses the same host precondition, for
+    // the same reason: it asserts what happens *after* a spawn, so it must reach
+    // one without real GPU hardware.
     if !cpu_only
         && !scripted_backend_failure
+        && !scripted_managed_engine_startup_failure()
         && let Some(usable) = visible_gpu_indices.as_deref()
         && usable.is_empty()
     {
@@ -730,8 +735,12 @@ pub(crate) fn serve(args: ServeArgs) -> Result<()> {
             device_policy_name(&device_policy)
         );
     }
+    // Preparation is skipped for the scripted startup-death scenario too: it
+    // asserts the launch, not the install, and a real runtime download is exactly
+    // what the ungated lane cannot do.
     if !matches!(device_policy, DevicePolicy::CpuOnly)
         && engine_manages_own_runtime(&selected_engine)
+        && !scripted_managed_engine_startup_failure()
     {
         ensure_self_managed_engine_ready(&paths, &mut config, &selected_engine)?;
     }
@@ -857,7 +866,7 @@ pub(crate) fn serve(args: ServeArgs) -> Result<()> {
         let mut spinner =
             cli_progress::Spinner::new(format!("Starting {model} on {selected_engine}…"));
         spinner.tick();
-        let report = start_managed_service(
+        let launched = start_managed_service(
             &selected_engine,
             &service_id,
             &model,
@@ -873,8 +882,20 @@ pub(crate) fn serve(args: ServeArgs) -> Result<()> {
             launch_lock,
             require_api_key,
             &mut |_elapsed| spinner.tick(),
-        )?;
-        ensure_background_helper_running_quiet(summary_mode)?;
+        )
+        .and_then(|report| ensure_background_helper_running_quiet(summary_mode).map(|()| report));
+        // `Spinner` has no `Drop`, so a launch that fails here — or the background
+        // helper it starts — would otherwise leave its line painted and the error
+        // would print on top of it. Both steps fold into `launched` so this one
+        // clear covers each; nothing fallible runs between here and the clears
+        // below.
+        let report = match launched {
+            Ok(report) => report,
+            Err(error) => {
+                spinner.clear();
+                return Err(error);
+            }
+        };
 
         // An equivalent service was already running, so nothing was spawned and the
         // freshly generated key is unused — drop it rather than leave it orphaned in

crates/rocm-core/src/lib.rs

@@ -137,13 +137,13 @@ pub use proc_lifecycle::{
     IdentityState, KillScope, ProcessIdentity, TerminationOutcome, identity_state,
     process_start_ticks, terminate_verified,
 };
+#[cfg(windows)]
 pub use process::{
-    FileLock, detach_command_session, process_is_running, terminate_process, terminate_process_tree,
+    DetachedSpawn, spawn_detached_no_inherit, spawn_detached_no_inherit_watching_startup,
+    spawn_hidden_console_no_inherit, spawn_hidden_console_with_log, wait_for_process_exit,
 };
-#[cfg(windows)]
 pub use process::{
-    spawn_detached_no_inherit, spawn_hidden_console_no_inherit, spawn_hidden_console_with_log,
-    wait_for_process_exit,
+    FileLock, detach_command_session, process_is_running, terminate_process, terminate_process_tree,
 };
 pub use report::{
     APPROVED_ARCHITECTURES, APPROVED_ARCHITECTURES_SOURCE, REPORT_SCHEMA_VERSION, ReadOutcome,

crates/rocm-core/src/process.rs

@@ -49,6 +49,27 @@ use std::sync::{Arc, Mutex};
 use std::thread;
 use std::time::{Duration, Instant};
 
+/// Outcome of a detached Windows spawn that was briefly watched for an early exit.
+///
+/// The observation runs while the process handle returned by `CreateProcessW` is
+/// still open, so a reported exit code is always the exit code of the process that
+/// was just spawned. Re-opening the process by PID after the fact could not offer
+/// that guarantee — Windows recycles PIDs, so by then the PID may name an
+/// unrelated process.
+#[cfg(windows)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct DetachedSpawn {
+    /// PID of the spawned process.
+    pub pid: u32,
+    /// `Some(code)` if the process exited at any point within the observation
+    /// window — known as soon as it exits, since the wait returns then rather
+    /// than at the end of the window — and `None` if it was still running when
+    /// the window ran out. `None` is also reported if the wait failed or the
+    /// exit code could not be read, which degrades to the unwatched behaviour
+    /// rather than inventing a failure.
+    pub early_exit_code: Option<u32>,
+}
+
 #[cfg(windows)]
 pub fn spawn_detached_no_inherit(
     program: &Path,
@@ -62,6 +83,38 @@ pub fn spawn_detached_no_inherit(
         DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP | CREATE_NO_WINDOW | CREATE_UNICODE_ENVIRONMENT,
         false,
         None,
+        None,
+    )
+    .map(|spawn| spawn.pid)
+}
+
+/// As [`spawn_detached_no_inherit`], but watches the new process for up to
+/// `settle` before returning, so the caller can tell "started" apart from
+/// "started and died immediately".
+///
+/// Creation flags and handle inheritance are identical to
+/// [`spawn_detached_no_inherit`]: the child stays fully detached and outlives this
+/// process. The wait is bounded and delays the caller by at most `settle` — less
+/// when the child exits sooner; it is not a join.
+///
+/// This exists because the detached spawn primitives hand back a bare PID rather
+/// than a `std::process::Child`, so a caller has no equivalent of `try_wait()` to
+/// notice a child that failed during startup.
+#[cfg(windows)]
+pub fn spawn_detached_no_inherit_watching_startup(
+    program: &Path,
+    args: &[String],
+    env_overrides: &[(&str, &Path)],
+    settle: Duration,
+) -> Result<DetachedSpawn> {
+    spawn_windows_no_inherit(
+        program,
+        args,
+        env_overrides,
+        DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP | CREATE_NO_WINDOW | CREATE_UNICODE_ENVIRONMENT,
+        false,
+        None,
+        Some(settle),
     )
 }
 
@@ -78,7 +131,9 @@ pub fn spawn_hidden_console_no_inherit(
         CREATE_NEW_CONSOLE | CREATE_NEW_PROCESS_GROUP | CREATE_UNICODE_ENVIRONMENT,
         true,
         None,
+        None,
     )
+    .map(|spawn| spawn.pid)
 }
 
 #[cfg(windows)]
@@ -147,12 +202,13 @@ pub fn spawn_hidden_console_with_log(
         CREATE_NEW_CONSOLE | CREATE_NEW_PROCESS_GROUP | CREATE_UNICODE_ENVIRONMENT,
         true,
         Some((stdout_handle, stderr_handle)),
+        None,
     );
     unsafe {
         CloseHandle(stdout_handle);
         CloseHandle(stderr_handle);
     }
-    result
+    result.map(|spawn| spawn.pid)
 }
 
 #[cfg(windows)]
@@ -473,7 +529,8 @@ fn spawn_windows_no_inherit(
         windows_sys::Win32::Foundation::HANDLE,
         windows_sys::Win32::Foundation::HANDLE,
     )>,
-) -> Result<u32> {
+    settle: Option<Duration>,
+) -> Result<DetachedSpawn> {
     use std::ptr::{null, null_mut};
     use windows_sys::Win32::Foundation::CloseHandle;
 
@@ -517,11 +574,59 @@ fn spawn_windows_no_inherit(
             std::io::Error::last_os_error()
         );
     }
+    // Observe the child *before* the handle is closed. Waiting on this handle is
+    // race-free; re-opening the process later by PID would not be, since Windows
+    // recycles PIDs and the PID could by then belong to something else.
+    let early_exit_code =
+        settle.and_then(|settle| unsafe { observe_early_exit(process_info.hProcess, settle) });
     unsafe {
         CloseHandle(process_info.hThread);
         CloseHandle(process_info.hProcess);
     }
-    Ok(process_info.dwProcessId)
+    Ok(DetachedSpawn {
+        pid: process_info.dwProcessId,
+        early_exit_code,
+    })
+}
+
+/// Wait up to `settle` for `process` to exit, reporting its exit code if it did.
+///
+/// Returns `None` both when the process is still running and when the exit code
+/// could not be read, so a failed query degrades to "assume it is alive" rather
+/// than reporting a startup failure that may not have happened.
+///
+/// # Safety
+///
+/// `process` must be a live process handle granting `SYNCHRONIZE` and
+/// `PROCESS_QUERY_INFORMATION` access. The handle is not closed here.
+#[cfg(windows)]
+#[allow(unsafe_code)] // Win32 FFI
+unsafe fn observe_early_exit(
+    process: windows_sys::Win32::Foundation::HANDLE,
+    settle: Duration,
+) -> Option<u32> {
+    use windows_sys::Win32::Foundation::WAIT_OBJECT_0;
+
+    if unsafe { WaitForSingleObject(process, wait_timeout_millis(settle)) } != WAIT_OBJECT_0 {
+        return None;
+    }
+    let mut exit_code: u32 = 0;
+    if unsafe { GetExitCodeProcess(process, &raw mut exit_code) } == 0 {
+        return None;
+    }
+    Some(exit_code)
+}
+
+/// Clamp a wait budget to the `u32` milliseconds `WaitForSingleObject` takes,
+/// never yielding `INFINITE`.
+///
+/// `INFINITE` is `u32::MAX`, so a saturating conversion of a long duration would
+/// silently turn a bounded startup check into one that blocks until the child
+/// exits. Cap one millisecond below it instead.
+#[cfg(any(windows, test))]
+fn wait_timeout_millis(budget: Duration) -> u32 {
+    const LONGEST_FINITE_WAIT_MS: u128 = (u32::MAX - 1) as u128;
+    budget.as_millis().min(LONGEST_FINITE_WAIT_MS) as u32
 }
 
 #[cfg(windows)]
@@ -1017,6 +1122,86 @@ fn assemble_output(
 mod tests {
     use super::*;
 
+    #[test]
+    fn wait_timeout_millis_keeps_the_startup_wait_bounded() {
+        assert_eq!(wait_timeout_millis(Duration::from_millis(200)), 200);
+        assert_eq!(wait_timeout_millis(Duration::ZERO), 0);
+        // Sub-millisecond budgets truncate to a poll rather than rounding up.
+        assert_eq!(wait_timeout_millis(Duration::from_micros(900)), 0);
+        // A budget past the `u32` millisecond range must never land on
+        // `INFINITE` (`u32::MAX`), which would block until the child exits.
+        let huge = wait_timeout_millis(Duration::from_secs(u64::from(u32::MAX)));
+        assert_eq!(huge, u32::MAX - 1);
+        assert_ne!(huge, u32::MAX);
+    }
+
+    /// Watching a detached spawn must surface a child that died during startup.
+    /// Only meaningful on Windows: the detached spawn primitives return a bare
+    /// PID there, so this wait is the only chance to notice the exit.
+    ///
+    /// It also pins what `DetachedSpawn::early_exit_code` documents: the exit is
+    /// reported when it happens, not when the window runs out. The window is far
+    /// longer than `cmd /C exit` takes, so a wait that sat it out fails the
+    /// elapsed-time check rather than passing slowly.
+    #[cfg(windows)]
+    #[test]
+    fn watched_detached_spawn_reports_a_child_that_exits_immediately() {
+        let window = Duration::from_secs(30);
+        let started = std::time::Instant::now();
+        // Each token is a separate argument so the assembled command line needs no
+        // quoting, keeping `cmd.exe`'s quote-stripping rules out of the test.
+        let spawn = spawn_detached_no_inherit_watching_startup(
+            &test_system32_tool("cmd.exe"),
+            &["/C".to_owned(), "exit".to_owned(), "7".to_owned()],
+            &[],
+            window,
+        )
+        .expect("spawn should succeed");
+        let elapsed = started.elapsed();
+        assert_eq!(spawn.early_exit_code, Some(7));
+        assert_ne!(spawn.pid, 0);
+        assert!(
+            elapsed < window / 2,
+            "the exit must be reported as soon as the child exits, not at the end of \
+             the {window:?} window (took {elapsed:?})"
+        );
+    }
+
+    /// The converse: a child that is still running must not be reported as a
+    /// startup failure, or every successful launch would be rejected.
+    #[cfg(windows)]
+    #[test]
+    fn watched_detached_spawn_does_not_report_a_child_that_keeps_running() {
+        // `ping` against localhost is the dependency-free Windows sleep, and ~9s
+        // outlasts the 200 ms observation window by well over an order of
+        // magnitude. Spawned directly rather than through `cmd /C` so the
+        // returned PID is the process the test has to clean up: on Windows
+        // `terminate_process_tree` is plain `terminate_process`, so an
+        // intermediate shell would leave `ping` orphaned.
+        let spawn = spawn_detached_no_inherit_watching_startup(
+            &test_system32_tool("ping.exe"),
+            &["-n".to_owned(), "10".to_owned(), "127.0.0.1".to_owned()],
+            &[],
+            Duration::from_millis(200),
+        )
+        .expect("spawn should succeed");
+        // Clean up before asserting, so a failing assertion cannot leak the child.
+        let early_exit_code = spawn.early_exit_code;
+        let was_running = process_is_running(spawn.pid);
+        let _ = terminate_process(spawn.pid);
+        assert_eq!(early_exit_code, None);
+        assert!(was_running);
+    }
+
+    /// `CreateProcessW` is called with an explicit application name, so it does
+    /// no `PATH` search — the program has to be a full path.
+    #[cfg(windows)]
+    fn test_system32_tool(exe: &str) -> PathBuf {
+        PathBuf::from(std::env::var_os("SystemRoot").unwrap_or_else(|| r"C:\Windows".into()))
+            .join("System32")
+            .join(exe)
+    }
+
     #[test]
     fn file_lock_creates_missing_parent_dirs_and_lock_file() {
         let dir =

docs/architecture.md

@@ -50,7 +50,7 @@ Each per-crate module inventory lists one file per line. Flat inventories (every
 - [`serve_cmd.rs`](../apps/rocm/src/serve_cmd.rs) — mechanical relocation, owns private types (Phase 6a): `serve()`, plus engine-recipe overrides.
 - [`serve_summary.rs`](../apps/rocm/src/serve_summary.rs) — full domain extraction.
 - [`service_hints.rs`](../apps/rocm/src/service_hints.rs) — shared CLI-output component: the `rocm services <action> <id> --yes` hint text, shared by [`main.rs`](../apps/rocm/src/main.rs), [`remote/mod.rs`](../apps/rocm/src/remote/mod.rs), [`serve_summary.rs`](../apps/rocm/src/serve_summary.rs), and [`service_launch.rs`](../apps/rocm/src/service_launch.rs) so they cannot go out of sync with each other, as `serve_summary.rs`'s stop row did from the day it was added — #417.
-- [`service_launch.rs`](../apps/rocm/src/service_launch.rs) — mechanical relocation, owns private types (Phase 6b, ROCMAI-91): spawning/attaching to a managed service process and streaming its log, plus the endpoint-key/auth checks that gate a public bind — `AttachOutcome`/`RawModeGuard`/`ManagedSpawn`/`AutostartClaim` are its private types; `ManagedLaunchReport` is `pub(crate)`, since [`serve_cmd.rs`](../apps/rocm/src/serve_cmd.rs) reads its fields directly for the interactive summary. Most of this module's functions are `pub(crate)` for the same reason — [`main.rs`](../apps/rocm/src/main.rs), [`serve_cmd.rs`](../apps/rocm/src/serve_cmd.rs), and [`automations.rs`](../apps/rocm/src/automations.rs) all call into it. `ServicesCommand`/`services()` stayed in [`main.rs`](../apps/rocm/src/main.rs) rather than moving alongside — the service-record-management helpers `services()` dispatches to (stop/restart/remove/prune) are entangled with each other; only stop/restart are also reachable from the chat/LLM sandbox tool-calling path, out of scope for Phase 6b.
+- [`service_launch.rs`](../apps/rocm/src/service_launch.rs) — mechanical relocation at first, owns private types (Phase 6b, ROCMAI-91), and has since gained the startup-death check and failed-launch record retirement: spawning/attaching to a managed service process and streaming its log, plus the endpoint-key/auth checks that gate a public bind — `AttachOutcome`/`RawModeGuard`/`ManagedSpawn`/`AutostartClaim` are its private types; `ManagedLaunchReport` is `pub(crate)`, since [`serve_cmd.rs`](../apps/rocm/src/serve_cmd.rs) reads its fields directly for the interactive summary. Most of this module's functions are `pub(crate)` for the same reason — [`main.rs`](../apps/rocm/src/main.rs), [`serve_cmd.rs`](../apps/rocm/src/serve_cmd.rs), and [`automations.rs`](../apps/rocm/src/automations.rs) all call into it. It also owns the engine spawn seam the launch here and the restart in [`main.rs`](../apps/rocm/src/main.rs) share — `ManagedEngineSpawnRequest`/`SpawnedManagedEngine`/`ManagedEngineStartupCheck`, `spawn_managed_engine_process` and `watch_managed_engine_startup`, plus the `ManagedStartupRecord` trait that check writes the record through — which is `pub(crate)` because `restart_internal_managed_service` drives that same seam rather than keeping its own copy, writing through its own `ManagedStartupRecord` so a concurrent stop's marker survives. `ServicesCommand`/`services()` stayed in [`main.rs`](../apps/rocm/src/main.rs) rather than moving alongside — the service-record-management helpers `services()` dispatches to (stop/restart/remove/prune) are entangled with each other; only stop/restart are also reachable from the chat/LLM sandbox tool-calling path, out of scope for Phase 6b.
 - [`storage.rs`](../apps/rocm/src/storage.rs) — full domain extraction.
 - [`therock.rs`](../apps/rocm/src/therock.rs) — full domain extraction; owns TheRock SDK install/update and the runtime-manifest model/loaders. The `runtimes` subcommand's own command enum, dispatch fn, and most of its domain logic — rendering, activation, and manifest import — stay in [`main.rs`](../apps/rocm/src/main.rs).
 - [`uninstall.rs`](../apps/rocm/src/uninstall.rs) — mechanical relocation, owns private types: `uninstall()`, plus the installer `PATH` undo and running-executable deletion. The shared plan type and its file-removal helpers stay in [`main.rs`](../apps/rocm/src/main.rs) because [`storage.rs`](../apps/rocm/src/storage.rs) reuses them.

docs/testing.md

@@ -1418,6 +1418,7 @@ cargo test -p rocm --bin rocm stop_final_write_keeps_what_others_wrote_while_it_
 cargo test -p rocm --bin rocm restart_does_not_start_an_engine_over_an_unconfirmed_stop
 cargo test -p rocm --bin rocm restart_readiness_write_keeps_a_stop_asked_for_while_it_waited
 cargo test -p rocm --bin rocm restart_readiness_write_leaves_a_stop_that_finished_meanwhile
+cargo test -p rocm --bin rocm a_restart_retiring_a_dead_child_keeps_a_stop_asked_for_during_its_check
 cargo test -p rocm --bin rocm restart_reports_a_child_it_could_not_record_and_writes_nothing
 cargo test -p rocm --bin rocm restarted_child_record_names_the_runtime_the_restart_was_pinned_to
 ```