diff --git a/crates/spt-daemon/src/shellhost.rs b/crates/spt-daemon/src/shellhost.rs index 44df752b..23833ed6 100644 --- a/crates/spt-daemon/src/shellhost.rs +++ b/crates/spt-daemon/src/shellhost.rs @@ -496,13 +496,10 @@ pub fn launch_shell( .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. + // Retire the previous birth before publishing the new PID. // [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) } } @@ -555,13 +552,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..7a473cf5 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 diff --git a/crates/spt-store/src/shellinfo.rs b/crates/spt-store/src/shellinfo.rs index f3aa9086..c0988a9e 100644 --- a/crates/spt-store/src/shellinfo.rs +++ b/crates/spt-store/src/shellinfo.rs @@ -172,6 +172,22 @@ pub fn record_shell_launch(shell_perch: &Path, pid: u32, now_ms: u64) { } } +/// Publish a new PID without ever pairing it with the previous launch's birth. +/// During publication a missing stamp deliberately preserves live-PID custody. +/// Failure to retire an old stamp must stop publication rather than misidentify +/// the new binary as a recycled corpse. +// [impl->REQ-SHELL-PERSISTENT-BOOT-RESTORE] +pub fn publish_shell_launch(shell_perch: &Path, pid: u32, now_ms: u64) -> 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), + } + 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 +228,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 +525,16 @@ 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(); + let err = publish_shell_launch(d.path(), std::process::id(), 1_000).unwrap_err(); + assert_ne!(err.kind(), std::io::ErrorKind::NotFound); + 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` diff --git a/docs/KNOWN-HAZARDS.md b/docs/KNOWN-HAZARDS.md index c8b94c6e..46dc56e9 100644 --- a/docs/KNOWN-HAZARDS.md +++ b/docs/KNOWN-HAZARDS.md @@ -1016,9 +1016,9 @@ The kill-path rule above generalizes: `daemon.pid` is not authority for *"which | 2.1/5.1 | Stable PID/broker-handle over ephemeral PID | liveness detection | | 2.3 | Handoff argv/IPC version-tolerant (newer brain ↔ older broker) | broker↔brain IPC, self-update | | 2.4 | gen_start = now() on cold-start + handoff | per-instance generation | -| 2.6 | A shell's ONLINE-ness is DERIVED (recorded status AND a not-provably-dead `shell.pid`) — an abruptly-killed binary breaks no link, so `close_shell`'s offline flip never runs and the record lies forever; derive at the gates/renders, keep the recorded field at the writers, the suspend-cascade close, and the wake reconciler — whose "no spontaneous relaunch" (deploys and quarantine own that decision) is since W2 the stated corpse-predates-boot predicate, not the stale record holding it open, and whose recycled-pid blindness is closed by the launch birth stamp wherever one is parked. Not only a crash edge: the GRACEFUL daemon-stop path kills bound shells at stop-begin with `close_shell` unrun (field 2026-07-25 — shells died abruptly, `info.json` still `online`, while the broker drained on 2m10s), so every daemon restart manufactures these stale records routinely; the derivation heals them at the gates | `spt_store::shellinfo::{shell_pid_provably_dead,effective_status,is_shell_online}`, `linkhost` relink/drive/cmd-wake, `shelldisc::discover`, `activity::observe_links` | +| 2.6 | A shell's ONLINE-ness is DERIVED from recorded status and a not-provably-dead PID/birth pair. Abrupt death and daemon-stop leave stale `online` records; derive truth at gates/renders and heal each changed record once. The same-boot no-spontaneous-relaunch rule applies only to nonpersistent shells. Persistent instances follow 2.7 regardless of launch age. During new-launch publication, retire the previous birth before publishing the new PID; an unstable read is not proof of death | `spt_store::shellinfo::{shell_pid_provably_dead,effective_status,is_shell_online,publish_shell_launch}`, `linkhost`, `shelldisc`, `activity` | -| 2.7 | A node restart must not permanently strand every `persistent` shell: (a) the daemon heals a recorded `online` the derivation contradicts, write-guarded on an actual change, and (b) a once-per-generation boot sweep PLUS an owner offline->online edge in the reconcile loop (releases#228 — the sweep alone strands the shells of any owner not yet online when it ran) relaunch an instance only when ALL of — adapter `persistent`, owner endpoint online, down in fact, recorded launch predates the boot instant (slack-absorbed) — hold; no stamp or no boot oracle ⇒ not restored. The owner-facing surfaces derive and look clean while the cascade reads the recorded field, so a clean render is NOT evidence the class did not occur — the falsifier is the on-disk record. The pre-existing cascade test's setup suppresses the failing arm (it suspends first), so the restart shape must be asserted on a fixture that never calls the suspend path | `shellwake::{heal_stale_online_records,restore_persistent_shells_at_boot,restore_persistent_shells_on_owner_online,OwnerOnlineEdge,launch_predates_boot}`, `proc::boot_instant_ms`, `shellinfo::{record_shell_launch,read_shell_launch}` | +| 2.7 | A once-per-generation boot sweep and each owner offline->online edge restore every instance whose adapter is `persistent`, owner is online, and binary is down in fact. Machine boot time, launch age, and absent launch stamps do not block persistent restoration; already-online and live binding launches are excluded. Heal every stale instance per owner with no repeat writes. Prove the restart shape without first suspending the fixture; a clean derived display alone is not evidence the record was healed | `shellwake::{heal_stale_online_records,restore_persistent_shells_at_boot,restore_persistent_shells_on_owner_online,OwnerOnlineEdge}`, `shellhost::live_launch_winner` | | 3.1 | Ephemeral perch cleanup on all exit paths | `ring` (RAII guard) | | 3.4 | A ring never adopts (so never deletes) a perch it did not create — probe the DIR, not the ready marker; record/spool/unreadable = occupied, empty = refused too; deliver + loud `RING_PERCH_EXISTS`/`RING_STALE_DIR` instead of block-waiting | `spt_msg::ring` probe + `create_dir` leaf | | 3.5 | A perch GC classifies on record PRESENCE only and never asks a liveness resolver — `is_perch_alive` is INVERTED here (recordless residue reads ALIVE, an offline endpoint reads DEAD: 24 vs 6 measured on HFENDULEAM 2026-08-04), so `!is_perch_alive` authorizes exactly the inverse set; the registry is never asked either (offline endpoints are legitimately absent from it) | `spt_store::perchgc::sweep`, `spt endpoint gc` | [raw output: artifact://128]