feat(runtime-core): replace the sidecar frame timeout with a heartbeat silence watchdog

CLAUDE.md

@@ -39,6 +39,13 @@ by default, warn near threshold, and fail with a typed error that names the
 limit and how to raise it. Host-visible warnings/errors must reach stderr/log
 or structured trace paths, not stay trapped in the VM.
 
+Never swallow errors silently. Every failure must either propagate as a hard,
+typed error to the caller (preferred) or be clearly logged at the failure site;
+empty `catch`/`let _ =` on fallible operations and fire-and-forget promises
+that drop rejections are bugs, not defensive coding. For guest-visible
+surfaces, prefer matching Linux behavior — the correct POSIX errno delivered to
+the guest — over inventing a softer fallback that hides the failure.
+
 ## Runtime And Registry
 
 - The projected `/opt/agentos` filesystem is the source of truth for software

crates/agentos-sidecar-core/src/engine.rs

@@ -227,13 +227,25 @@ impl AcpCore {
         };
 
         let _ = host.close_stdin(&session.process_id);
-        let _ = host.kill_agent(&session.process_id, "SIGTERM");
-        if host
-            .wait_for_exit(&session.process_id, SESSION_CLOSE_TIMEOUT_MS)?
-            .is_none()
+        // Mirror of the native `close_session` short-circuit: an adapter that
+        // already exited (crash / OOM / idle eviction, recorded on the session
+        // as `closed`) has no future exit to wait for, and signalling its
+        // reaped PID fails with a process-gone error. Skip the SIGTERM → wait
+        // → SIGKILL → wait dance (~2× `SESSION_CLOSE_TIMEOUT_MS` of dead
+        // waiting) in that case.
+        let adapter_already_gone = session.closed || {
+            let sigterm = host.kill_agent(&session.process_id, "SIGTERM");
+            matches!(&sigterm, Err(error) if is_process_already_gone_error(error))
+        };
+        if !adapter_already_gone
+            && host
+                .wait_for_exit(&session.process_id, SESSION_CLOSE_TIMEOUT_MS)?
+                .is_none()
         {
-            let _ = host.kill_agent(&session.process_id, "SIGKILL");
-            let _ = host.wait_for_exit(&session.process_id, SESSION_CLOSE_TIMEOUT_MS)?;
+            let sigkill = host.kill_agent(&session.process_id, "SIGKILL");
+            if !matches!(&sigkill, Err(error) if is_process_already_gone_error(error)) {
+                let _ = host.wait_for_exit(&session.process_id, SESSION_CLOSE_TIMEOUT_MS)?;
+            }
         }
 
         Ok(AcpResponse::AcpSessionClosedResponse(
@@ -1121,6 +1133,18 @@ fn is_unknown_session_error(response: &Value) -> bool {
         .is_some_and(|kind| kind == "unknown_session")
 }
 
+/// True when a signal/kill request failed because the target process no longer
+/// exists — the process-table `ESRCH` / "no such process" / "has no active
+/// process" errors surfaced for an already-reaped PID. `close_session` uses
+/// this to skip the exit wait when the adapter is already gone. Mirrors the
+/// native sidecar's `is_process_already_gone_error`.
+fn is_process_already_gone_error(error: &AcpCoreError) -> bool {
+    let message = error.to_string().to_ascii_lowercase();
+    message.contains("esrch")
+        || message.contains("no such process")
+        || message.contains("has no active process")
+}
+
 /// Write a JSON-RPC message as a single newline-terminated line to the agent's
 /// stdin (no waiting). Used by the resumable handshake.
 fn write_json_line<H: AcpHost>(
@@ -1329,6 +1353,104 @@ mod tests {
         assert_eq!(host.killed, vec![("proc-s1".into(), "SIGTERM".into())]);
     }
 
+    /// Host whose adapter process is already gone: `wait_for_exit` would time
+    /// out (returns `None`), so any wait the engine performs is dead time. The
+    /// wait counter proves the short-circuit: a regression re-enters the
+    /// SIGTERM → wait → SIGKILL → wait sequence and records 2 waits.
+    #[derive(Default)]
+    struct GoneAdapterHost {
+        kill_error: Option<String>,
+        killed: Vec<(String, String)>,
+        waits: usize,
+    }
+
+    impl AcpHost for GoneAdapterHost {
+        fn spawn_agent(&mut self, _: SpawnAgentRequest) -> Result<SpawnedAgent, AcpCoreError> {
+            unreachable!("close_session does not spawn")
+        }
+        fn bind_session(&mut self, _: &str, _: &str) -> Result<(), AcpCoreError> {
+            Ok(())
+        }
+        fn write_stdin(&mut self, _: &str, _: &[u8]) -> Result<(), AcpCoreError> {
+            unreachable!()
+        }
+        fn close_stdin(&mut self, _: &str) -> Result<(), AcpCoreError> {
+            Ok(())
+        }
+        fn poll_output(&mut self, _: &str) -> Result<Option<AgentOutput>, AcpCoreError> {
+            Ok(None)
+        }
+        fn kill_agent(&mut self, process_id: &str, signal: &str) -> Result<(), AcpCoreError> {
+            self.killed
+                .push((process_id.to_string(), signal.to_string()));
+            match &self.kill_error {
+                Some(message) => Err(AcpCoreError::InvalidState(message.clone())),
+                None => Ok(()),
+            }
+        }
+        fn wait_for_exit(&mut self, _: &str, _: u64) -> Result<Option<i32>, AcpCoreError> {
+            self.waits += 1;
+            Ok(None) // the exit event was already drained; a wait can only time out
+        }
+        fn write_file(&mut self, _: &str, _: &[u8]) -> Result<(), AcpCoreError> {
+            Ok(())
+        }
+        fn read_file(&mut self, _: &str) -> Result<Vec<u8>, AcpCoreError> {
+            Ok(Vec::new())
+        }
+        fn now_ms(&self) -> u64 {
+            0
+        }
+    }
+
+    #[test]
+    fn close_session_skips_teardown_wait_when_session_already_closed() {
+        let mut core = AcpCore::new();
+        let mut dead = record("s1", "conn-a");
+        dead.closed = true;
+        dead.exit_code = Some(137);
+        core.insert_session(dead);
+        let mut host = GoneAdapterHost::default();
+        let resp = core
+            .close_session(
+                &mut host,
+                "conn-a",
+                &AcpCloseSessionRequest {
+                    session_id: "s1".into(),
+                },
+            )
+            .expect("close");
+        assert!(matches!(resp, AcpResponse::AcpSessionClosedResponse(_)));
+        assert_eq!(core.session_count(), 0);
+        // Already-observed exit: no signals sent, no dead waiting.
+        assert!(host.killed.is_empty());
+        assert_eq!(host.waits, 0);
+    }
+
+    #[test]
+    fn close_session_skips_teardown_wait_when_sigterm_reports_process_gone() {
+        let mut core = AcpCore::new();
+        core.insert_session(record("s1", "conn-a"));
+        let mut host = GoneAdapterHost {
+            kill_error: Some(String::from("process proc-s1: no such process (ESRCH)")),
+            ..GoneAdapterHost::default()
+        };
+        let resp = core
+            .close_session(
+                &mut host,
+                "conn-a",
+                &AcpCloseSessionRequest {
+                    session_id: "s1".into(),
+                },
+            )
+            .expect("close");
+        assert!(matches!(resp, AcpResponse::AcpSessionClosedResponse(_)));
+        // SIGTERM was attempted, classified as process-gone, and both waits
+        // (and the SIGKILL escalation) were skipped.
+        assert_eq!(host.killed, vec![("proc-s1".into(), "SIGTERM".into())]);
+        assert_eq!(host.waits, 0);
+    }
+
     #[test]
     fn session_request_enforces_ownership_without_side_effects() {
         // A non-owner prompt fails closed with the same unknown-session error and

crates/agentos-sidecar/src/acp_extension.rs

@@ -626,37 +626,38 @@ impl AcpExtension {
                 process_id: session.process_id.clone(),
             })
             .await;
-        let sigterm = ctx
-            .kill_process_wire(KillProcessRequest {
-                process_id: session.process_id.clone(),
-                signal: String::from("SIGTERM"),
-            })
-            .await;
-        // The adapter process may already be gone: it can exit out-of-band (crash /
-        // OOM, or a lazily-observed idle-time crash) *before* the client sends
-        // close_session. When it does, its `ProcessExited` event has already been
-        // emitted and drained, so `wait_for_process_exit` — which only polls the
-        // event stream and never checks current process state — can never observe
-        // it and blocks the full `SESSION_CLOSE_TIMEOUT` for SIGTERM *and* again
-        // after SIGKILL (~2× the timeout) before we ever reach resource disposal.
-        // Because close_session is serial per sidecar, that stall also blocks a
-        // `create_session` the host issues right after it (session recovery / a
-        // returning user whose idle session was evicted eats the full stall on
-        // their next turn). Detect the already-gone case from the SIGTERM result —
-        // reusing the same "adapter is gone" classification the prompt path uses —
-        // and skip straight to resource disposal.
-        let already_gone = matches!(&sigterm, Err(error) if is_process_already_gone(error));
-        if !already_gone
+        // The adapter may already be gone: it can crash, OOM, or idle-evict
+        // before the client sends close_session, and its `ProcessExitedEvent`
+        // has then already been drained from the shared per-ownership event
+        // queue (usually by the prompt exchange loop, which records it as
+        // `session.closed`). `wait_for_process_exit` only observes *future*
+        // events, so without a short-circuit an already-dead adapter burns
+        // `SESSION_CLOSE_TIMEOUT` twice (~10s) signalling a PID that no longer
+        // exists — and because extension dispatch is serialized, a
+        // `create_session` issued right after (session recovery for a
+        // returning user) stalls behind the dead wait.
+        let adapter_already_gone = session.closed || {
+            let sigterm = ctx
+                .kill_process_wire(KillProcessRequest {
+                    process_id: session.process_id.clone(),
+                    signal: String::from("SIGTERM"),
+                })
+                .await;
+            matches!(&sigterm, Err(error) if is_process_already_gone_error(error))
+        };
+        if !adapter_already_gone
             && !wait_for_process_exit(&mut ctx, &session.process_id, SESSION_CLOSE_TIMEOUT).await
         {
-            let _ = ctx
+            let sigkill = ctx
                 .kill_process_wire(KillProcessRequest {
                     process_id: session.process_id.clone(),
                     signal: String::from("SIGKILL"),
                 })
                 .await;
-            let _ =
-                wait_for_process_exit(&mut ctx, &session.process_id, SESSION_CLOSE_TIMEOUT).await;
+            if !matches!(&sigkill, Err(error) if is_process_already_gone_error(error)) {
+                let _ = wait_for_process_exit(&mut ctx, &session.process_id, SESSION_CLOSE_TIMEOUT)
+                    .await;
+            }
         }
         let _ = ctx
             .dispose_session_resources_wire(&request.session_id)
@@ -2571,14 +2572,13 @@ fn is_adapter_gone_error(error: &SidecarError) -> bool {
     matches!(error, SidecarError::InvalidState(message) if message.contains(ADAPTER_NO_ACTIVE_PROCESS_MARKER))
 }
 
-/// True when a kill/signal request failed because the target process no longer
-/// exists — i.e. it already exited before we tried to terminate it. Covers both
-/// the adapter-gone classification (`is_adapter_gone_error`) and the lower-level
-/// process-table `ESRCH` / "no such process" the signal path returns for a PID
-/// that has already been reaped. Used by `close_session` to skip the
-/// `wait_for_process_exit` dance (which can only observe a *future* exit event)
-/// when the process is already gone.
-fn is_process_already_gone(error: &SidecarError) -> bool {
+/// True when a signal/kill request failed because the target process no longer
+/// exists: either the adapter-gone classification the prompt path uses
+/// (`is_adapter_gone_error`) or the lower-level process-table `ESRCH` /
+/// "no such process" error the signal path returns for an already-reaped PID.
+/// `close_session` uses this to skip `wait_for_process_exit` — which can only
+/// observe a *future* exit event — when the process is already gone.
+fn is_process_already_gone_error(error: &SidecarError) -> bool {
     if is_adapter_gone_error(error) {
         return true;
     }

crates/native-sidecar/src/plugins/js_bridge.rs

@@ -517,6 +517,18 @@ impl VirtualFileSystem for JsBridgeFilesystem {
     }
 
     fn realpath(&self, path: &str) -> VfsResult<String> {
+        // The mount root always exists — it IS the mount point — so resolve it
+        // locally like `host_dir` does instead of round-tripping the bridge.
+        // Host-side drivers frequently cannot canonicalize their own root and
+        // return ENOENT for it, and because a js_bridge mount root is not a
+        // symlink leaf, `MountTable::realpath` propagates that ENOENT instead
+        // of falling back to the lexical path. The kernel permission wrapper
+        // resolves every operation's subject through `realpath` first, so that
+        // ENOENT aborts e.g. a readdir of the mount root before the readDir
+        // bridge call is ever issued.
+        if path.is_empty() || path == "/" {
+            return Ok(String::from("/"));
+        }
         self.parse_required(
             "realpath",
             path,

crates/native-sidecar/src/stdio.rs

@@ -47,6 +47,17 @@ use tokio::time;
 // pumps are cheap no-ops (try_recv + zero-timeout poll), so the higher cadence
 // costs negligible CPU when no guest is issuing RPCs.
 const EVENT_PUMP_INTERVAL: Duration = Duration::from_micros(250);
+// Cadence of sidecar→host heartbeat frames. The host treats sustained inbound
+// silence (several missed beats) as a dead or wedged sidecar and tears the
+// process down, so this is a fixed protocol constant, not a tunable. Emitted
+// from a dedicated thread so beats keep flowing while the dispatch loop is
+// busy inside one long request.
+const HEARTBEAT_INTERVAL: Duration = Duration::from_secs(10);
+// Connection id stamped on heartbeat frames. Heartbeats are transport-level
+// liveness — not tied to an authenticated connection — and the host consumes
+// them at its frame layer without routing by ownership, so a fixed synthetic
+// id is correct even before any client authenticates.
+const HEARTBEAT_CONNECTION_ID: &str = "sidecar-transport";
 const MAX_STDIN_FRAME_QUEUE: usize = 128;
 const MAX_EVENT_READY_QUEUE: usize = 1;
 // Defense-in-depth headroom for the host-bound frame queue: a burst of output
@@ -184,6 +195,7 @@ async fn run_async(extensions: Vec<Box<dyn Extension>>) -> Result<(), Box<dyn Er
             }
         }
     });
+    spawn_heartbeat_thread(write_tx.clone(), HEARTBEAT_INTERVAL);
 
     thread::spawn({
         let callback_transport = callback_transport.clone();
@@ -688,6 +700,39 @@ fn send_output_frame(
     })
 }
 
+/// Emit a connection-scoped `StructuredEvent { name: "heartbeat" }` frame every
+/// `interval` for as long as the stdout writer is alive. This is the host's
+/// liveness signal: it resets the host's silence watchdog, so a host that sees
+/// no frames at all for several intervals can conclude the sidecar process is
+/// dead or wedged rather than merely busy. Runs on its own thread with a clone
+/// of the outbound frame channel so beats are independent of the dispatch loop.
+fn spawn_heartbeat_thread(write_tx: TrackedSyncSender<ProtocolFrame>, interval: Duration) {
+    thread::spawn(move || loop {
+        thread::sleep(interval);
+        let frame = match crate::service::structured_event_frame(
+            HEARTBEAT_CONNECTION_ID,
+            "heartbeat",
+            std::collections::HashMap::new(),
+        ) {
+            Ok(frame) => frame,
+            Err(error) => {
+                // Unreachable for a fixed name/empty detail; if it ever fires,
+                // stop loudly instead of spinning on a broken encoder.
+                tracing::error!(
+                    target: "agentos_native_sidecar::stdio",
+                    %error,
+                    "failed to encode heartbeat frame; stopping heartbeat thread",
+                );
+                return;
+            }
+        };
+        if send_output_frame(&write_tx, ProtocolFrame::EventFrame(frame)).is_err() {
+            // Writer thread gone — the sidecar is shutting down. Normal exit.
+            return;
+        }
+    });
+}
+
 fn default_compile_cache_root() -> PathBuf {
     // Stable across sidecar processes so V8 compile-cache (cachedData) survives a
     // fresh sidecar/VM and benefits cold starts. Previously keyed by PID, which
@@ -707,6 +752,28 @@ mod tests {
 
     const TEST_EXTENSION_NAMESPACE: &str = "dev.rivet.secure-exec.test.blocking";
 
+    #[test]
+    fn heartbeat_thread_emits_periodic_structured_heartbeat_frames() {
+        let (write_tx, write_rx) =
+            tracked_sync_channel::<ProtocolFrame>(TrackedLimit::SidecarStdoutFrames, 16);
+        spawn_heartbeat_thread(write_tx, Duration::from_millis(5));
+
+        // Two beats prove the emitter is periodic, not one-shot.
+        for beat in 0..2 {
+            let frame = write_rx.recv().expect("heartbeat frame");
+            let ProtocolFrame::EventFrame(event) = frame else {
+                panic!("expected event frame for beat {beat}, got {frame:?}");
+            };
+            let event = crate::wire::event_frame_to_compat(event).expect("decode heartbeat frame");
+            let crate::protocol::EventPayload::Structured(structured) = event.payload else {
+                panic!("expected structured payload for beat {beat}");
+            };
+            assert_eq!(structured.name, "heartbeat");
+        }
+        // Dropping the receiver disconnects the channel; the emitter thread
+        // observes the send failure and exits cleanly.
+    }
+
     #[test]
     fn read_frame_rejects_oversized_prefix_before_allocating_payload() {
         let codec = WireFrameCodec::new(16);