diff --git a/crates/spt-daemon/src/shellhost.rs b/crates/spt-daemon/src/shellhost.rs index 44df752b..a930a4a2 100644 --- a/crates/spt-daemon/src/shellhost.rs +++ b/crates/spt-daemon/src/shellhost.rs @@ -477,6 +477,9 @@ pub fn launch_shell( let tokens = fill_spawn_command(owlery, owner, shell_id, adapter_name, install_dir, shell)?; let (program, args) = tokens.split_first().expect("non-empty checked"); + let perch = spt_store::perch::resolve_shell_perch_path_in(owlery, owner, shell_id); + spt_store::shellinfo::prepare_shell_launch(&perch) + .map_err(|e| format!("prepare launch: {e}"))?; // The shell binary is a LONG-LIVED detached child (KNOWN-HAZARDS // 5.6): on Windows it must inherit NO handles, or a caller // capturing `spt shell spawn`'s output — at any depth of the @@ -495,14 +498,10 @@ pub fn launch_shell( .spawn() .map_err(|e| format!("spawn {program}: {e}"))? .id(); - let perch = spt_store::perch::resolve_shell_perch_path_in(owlery, owner, shell_id); - std::fs::write(perch.join(SHELL_PID_FILE), pid.to_string()) - .map_err(|e| format!("record pid: {e}"))?; - // The birth stamp rides WITH the pid at every launch, or the probe that - // reads it degrades to bare-pid liveness on this instance alone -- a - // silent per-instance regression rather than a visible one. + // Publish only after the previous birth was retired before spawn. // [impl->REQ-SHELL-PERSISTENT-BOOT-RESTORE] - spt_store::shellinfo::record_shell_launch(&perch, pid, now_ms()); + spt_store::shellinfo::publish_shell_launch(&perch, pid, now_ms()) + .map_err(|e| format!("record launch: {e}"))?; Ok(pid) } } @@ -527,6 +526,9 @@ pub fn launch_shell_brokered_in( let (program, args) = tokens.split_first().expect("non-empty checked"); let mut brain = Brain::cold_start(broker_name, now_ms()).map_err(|e| format!("broker connect: {e}"))?; + let perch = spt_store::perch::resolve_shell_perch_path_in(owlery, owner, shell_id); + spt_store::shellinfo::prepare_shell_launch(&perch) + .map_err(|e| format!("prepare launch: {e}"))?; let (_sid, pid) = brain .spawn_session_pid(SpawnReq { program: program.clone(), @@ -554,14 +556,10 @@ pub fn launch_shell_brokered_in( }) .map_err(|e| format!("broker spawn: {e}"))?; let pid = pid.unwrap_or(0); - let perch = spt_store::perch::resolve_shell_perch_path_in(owlery, owner, shell_id); - std::fs::write(perch.join(SHELL_PID_FILE), pid.to_string()) - .map_err(|e| format!("record pid: {e}"))?; - // Same stamp at the broker-hosted launch. A backend that exposed no pid - // records 0 here; record_shell_launch stamps no start-time for it, so the - // probe keeps its fail-toward-alive stance for pid-less backends. + // PID-less backends keep their fail-toward-alive behavior. // [impl->REQ-SHELL-PERSISTENT-BOOT-RESTORE] - spt_store::shellinfo::record_shell_launch(&perch, pid, now_ms()); + spt_store::shellinfo::publish_shell_launch(&perch, pid, now_ms()) + .map_err(|e| format!("record launch: {e}"))?; Ok(pid) } diff --git a/crates/spt-daemon/src/shellwake.rs b/crates/spt-daemon/src/shellwake.rs index d9631b4d..72ed57cf 100644 --- a/crates/spt-daemon/src/shellwake.rs +++ b/crates/spt-daemon/src/shellwake.rs @@ -406,13 +406,9 @@ pub fn resolve_wake( // Never double-launch a live binary: the revive's wake cascade may have // relaunched this (persistent) instance already — and two reconcilers // (a fresh daemon adopting + a stale watcher firing) may race a wake. - // A parked pid that probes alive means somebody won; stand down. + // A live, birth-matching launch wins even before its bind handshake. let perch = spt_store::perch::resolve_shell_perch_path_in(owlery, owner, shell_id); - let live_pid = std::fs::read_to_string(perch.join(crate::shellhost::SHELL_PID_FILE)) - .ok() - .and_then(|s| s.trim().parse::().ok()) - .filter(|&p| p != 0 && spt_store::proc::is_process_alive(p)); - if let Some(p) = live_pid { + if let Some(p) = crate::shellhost::live_launch_winner(&perch) { return Ok(format!("{did}already relaunched pid={p} (online at bind)")); } // [impl->REQ-INSTALL-11] the wake-triggered relaunch resolves against the @@ -1525,6 +1521,38 @@ mod tests { ); } + // [unit->REQ-SHELL-PERSISTENT-BOOT-RESTORE] + #[test] + fn resolve_wake_does_not_adopt_a_recycled_pid_as_a_launch() { + let tmp = tempfile::tempdir().unwrap(); + let owlery = tmp.path(); + seed_owner(owlery, "ling"); + let id = spawn_record(owlery, "ling", "mock-wake", None).unwrap(); + let perch = spt_store::perch::resolve_shell_perch_path_in(owlery, "ling", &id); + let pid = std::process::id(); + std::fs::write(perch.join(shellinfo::SHELL_PID_FILE), pid.to_string()).unwrap(); + shellinfo::record_shell_launch(&perch, pid, 1_000); + let mut launch = shellinfo::read_shell_launch(&perch).unwrap(); + launch.pid_started_at = Some( + launch.pid_started_at.expect("native birth on supported platforms") ^ 1, + ); + std::fs::write( + perch.join(shellinfo::SHELL_LAUNCH_FILE), + serde_json::to_string(&launch).unwrap(), + ) + .unwrap(); + // A real spawn failure must surface, not a successful adoption of the + // unrelated live process. No child needs cleanup on this error path. + let missing = tmp.path().join("missing-shell-executable"); + let command = format!("\"{}\"", missing.display()); + let err = resolve_wake( + owlery, "ling", &id, "mock-wake", &shell_section(&command, "persistent = true"), + ) + .unwrap_err(); + assert!(err.contains("missing-shell-executable"), "{err}"); + assert!(spt_store::proc::is_process_alive(pid)); + } + // [unit->REQ-SHELL-2] wake resolution, the dormant branch: the owner is // resting warm — touch nothing on the endpoint, just relaunch the shell. #[test] diff --git a/crates/spt-store/src/shellinfo.rs b/crates/spt-store/src/shellinfo.rs index f3aa9086..f860e598 100644 --- a/crates/spt-store/src/shellinfo.rs +++ b/crates/spt-store/src/shellinfo.rs @@ -172,6 +172,27 @@ pub fn record_shell_launch(shell_perch: &Path, pid: u32, now_ms: u64) { } } +/// Retire the previous birth before spawning a replacement. Failure must be +/// reported before a child exists, not after creating an unrecorded process. +// [impl->REQ-SHELL-PERSISTENT-BOOT-RESTORE] +pub fn prepare_shell_launch(shell_perch: &Path) -> std::io::Result<()> { + match std::fs::remove_file(shell_perch.join(SHELL_LAUNCH_FILE)) { + Ok(()) => {} + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(e), + } + Ok(()) +} + +/// Publish a prepared launch. The missing stamp preserves live-PID custody +/// until the new birth is available; it can never name the previous process. +// [impl->REQ-SHELL-PERSISTENT-BOOT-RESTORE] +pub fn publish_shell_launch(shell_perch: &Path, pid: u32, now_ms: u64) -> std::io::Result<()> { + atomic_write_string(&shell_perch.join(SHELL_PID_FILE), &pid.to_string())?; + record_shell_launch(shell_perch, pid, now_ms); + Ok(()) +} + /// Read the birth stamp, if this instance was launched by a build that parks one. /// Absent or unparseable ⇒ `None`, which every consumer treats as "no stamp to /// compare" rather than as a fault. @@ -212,6 +233,11 @@ pub fn shell_pid_provably_dead(shell_perch: &Path) -> bool { // time, still falls back to plain absence and fails toward ALIVE. // [impl->REQ-SHELL-PERSISTENT-BOOT-RESTORE] let birth = read_shell_launch(shell_perch).and_then(|l| l.pid_started_at); + // A relaunch can publish between these reads. Do not apply the new birth + // to the old PID; an unstable snapshot is not proof that the shell died. + if read_shell_pid(shell_perch) != Some(pid) { + return false; + } matches!( crate::liveness::relay_liveness(Some(pid), birth), crate::liveness::RelayLiveness::Gone @@ -504,6 +530,15 @@ mod tests { assert_eq!(status_of(&d), SHELL_STATUS_ONLINE); } + // [unit->REQ-SHELL-PERSISTENT-BOOT-RESTORE] + #[test] + fn launch_publication_refuses_an_unretirable_old_stamp() { + let d = online_perch(Some("123")); + std::fs::create_dir(d.path().join(SHELL_LAUNCH_FILE)).unwrap(); + prepare_shell_launch(d.path()).unwrap_err(); + assert_eq!(read_shell_pid(d.path()), Some(123)); + } + // [unit->REQ-HAZARD-SHELL-STALE-ONLINE] the whole discriminant matrix. The // ONE case that derives offline is a parked, non-zero, dead pid — flynn's // field shape: the binary was force-killed, no link broke, so `close_shell`