Skip to content

Commit d9179cc

Browse files
committed
feat(runtime-core): replace the sidecar frame timeout with a heartbeat silence watchdog
- The native sidecar emits a connection-scoped `StructuredEvent{name:"heartbeat"}` every 10s from a dedicated thread, so beats keep flowing while the dispatch loop is busy inside one long request. No protocol schema change — reuses the existing structured-event variant. - The host replaces the per-frame 120s timeout with a silence watchdog: any inbound frame resets a 30s clock; sustained total silence kills the sidecar and rejects in-flight requests with a typed `SidecarSilenceTimeout` (stderr tail included). - Individual requests no longer have a deadline: a legitimately long agent turn runs for minutes without teardown, while a genuinely dead or wedged sidecar is detected in ~30s instead of 2 minutes. - Removes `NATIVE_SIDECAR_FRAME_TIMEOUT_MS`, the `AGENTOS_SIDECAR_FRAME_TIMEOUT_MS` env override (#1641), and all `frameTimeoutMs` plumbing (spawn options, native-client, protocol-client, frame-rpc, correlation). - Heartbeats are swallowed at the frame layer on both clients so they never reach event consumers or the bounded event buffer; the Rust client mirrors the watchdog (activity-reset reader, kill + fail-pending on silence). - Heartbeat cadence and silence window are fixed protocol constants (no env knobs); docs updated.
1 parent a46dab5 commit d9179cc

23 files changed

Lines changed: 523 additions & 140 deletions

crates/native-sidecar/src/stdio.rs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,17 @@ use tokio::time;
4747
// pumps are cheap no-ops (try_recv + zero-timeout poll), so the higher cadence
4848
// costs negligible CPU when no guest is issuing RPCs.
4949
const EVENT_PUMP_INTERVAL: Duration = Duration::from_micros(250);
50+
// Cadence of sidecar→host heartbeat frames. The host treats sustained inbound
51+
// silence (several missed beats) as a dead or wedged sidecar and tears the
52+
// process down, so this is a fixed protocol constant, not a tunable. Emitted
53+
// from a dedicated thread so beats keep flowing while the dispatch loop is
54+
// busy inside one long request.
55+
const HEARTBEAT_INTERVAL: Duration = Duration::from_secs(10);
56+
// Connection id stamped on heartbeat frames. Heartbeats are transport-level
57+
// liveness — not tied to an authenticated connection — and the host consumes
58+
// them at its frame layer without routing by ownership, so a fixed synthetic
59+
// id is correct even before any client authenticates.
60+
const HEARTBEAT_CONNECTION_ID: &str = "sidecar-transport";
5061
const MAX_STDIN_FRAME_QUEUE: usize = 128;
5162
const MAX_EVENT_READY_QUEUE: usize = 1;
5263
// 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
184195
}
185196
}
186197
});
198+
spawn_heartbeat_thread(write_tx.clone(), HEARTBEAT_INTERVAL);
187199

188200
thread::spawn({
189201
let callback_transport = callback_transport.clone();
@@ -688,6 +700,39 @@ fn send_output_frame(
688700
})
689701
}
690702

703+
/// Emit a connection-scoped `StructuredEvent { name: "heartbeat" }` frame every
704+
/// `interval` for as long as the stdout writer is alive. This is the host's
705+
/// liveness signal: it resets the host's silence watchdog, so a host that sees
706+
/// no frames at all for several intervals can conclude the sidecar process is
707+
/// dead or wedged rather than merely busy. Runs on its own thread with a clone
708+
/// of the outbound frame channel so beats are independent of the dispatch loop.
709+
fn spawn_heartbeat_thread(write_tx: TrackedSyncSender<ProtocolFrame>, interval: Duration) {
710+
thread::spawn(move || loop {
711+
thread::sleep(interval);
712+
let frame = match crate::service::structured_event_frame(
713+
HEARTBEAT_CONNECTION_ID,
714+
"heartbeat",
715+
std::collections::HashMap::new(),
716+
) {
717+
Ok(frame) => frame,
718+
Err(error) => {
719+
// Unreachable for a fixed name/empty detail; if it ever fires,
720+
// stop loudly instead of spinning on a broken encoder.
721+
tracing::error!(
722+
target: "agentos_native_sidecar::stdio",
723+
%error,
724+
"failed to encode heartbeat frame; stopping heartbeat thread",
725+
);
726+
return;
727+
}
728+
};
729+
if send_output_frame(&write_tx, ProtocolFrame::EventFrame(frame)).is_err() {
730+
// Writer thread gone — the sidecar is shutting down. Normal exit.
731+
return;
732+
}
733+
});
734+
}
735+
691736
fn default_compile_cache_root() -> PathBuf {
692737
// Stable across sidecar processes so V8 compile-cache (cachedData) survives a
693738
// fresh sidecar/VM and benefits cold starts. Previously keyed by PID, which
@@ -707,6 +752,28 @@ mod tests {
707752

708753
const TEST_EXTENSION_NAMESPACE: &str = "dev.rivet.secure-exec.test.blocking";
709754

755+
#[test]
756+
fn heartbeat_thread_emits_periodic_structured_heartbeat_frames() {
757+
let (write_tx, write_rx) =
758+
tracked_sync_channel::<ProtocolFrame>(TrackedLimit::SidecarStdoutFrames, 16);
759+
spawn_heartbeat_thread(write_tx, Duration::from_millis(5));
760+
761+
// Two beats prove the emitter is periodic, not one-shot.
762+
for beat in 0..2 {
763+
let frame = write_rx.recv().expect("heartbeat frame");
764+
let ProtocolFrame::EventFrame(event) = frame else {
765+
panic!("expected event frame for beat {beat}, got {frame:?}");
766+
};
767+
let event = crate::wire::event_frame_to_compat(event).expect("decode heartbeat frame");
768+
let crate::protocol::EventPayload::Structured(structured) = event.payload else {
769+
panic!("expected structured payload for beat {beat}");
770+
};
771+
assert_eq!(structured.name, "heartbeat");
772+
}
773+
// Dropping the receiver disconnects the channel; the emitter thread
774+
// observes the send failure and exits cleanly.
775+
}
776+
710777
#[test]
711778
fn read_frame_rejects_oversized_prefix_before_allocating_payload() {
712779
let codec = WireFrameCodec::new(16);

crates/sidecar-client/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,5 @@ futures = "0.3"
1313
parking_lot = "0.12"
1414
scc = "2"
1515
thiserror = "2"
16-
tokio = { version = "1", features = ["io-util", "macros", "process", "rt", "sync"] }
16+
tokio = { version = "1", features = ["io-util", "macros", "process", "rt", "sync", "time"] }
1717
tracing = "0.1"

crates/sidecar-client/src/transport.rs

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ const PENDING_REQUEST_LIMIT: usize = 4096;
3636
/// Product clients can pass an explicit binary path to [`SidecarTransport::spawn`].
3737
const SIDECAR_BIN_ENV: &str = "AGENTOS_SIDECAR_BIN";
3838

39+
/// How long the host tolerates TOTAL inbound silence (no responses, events, sidecar requests, or
40+
/// heartbeats) before declaring the sidecar dead. The sidecar heartbeats every 10s from a dedicated
41+
/// thread even while busy, so this allows two missed beats plus margin; it bounds "sidecar is dead
42+
/// or wedged", never "this request is slow" — individual requests have no deadline of their own.
43+
/// Fixed protocol constant paired with the sidecar heartbeat cadence; mirrors the TS client.
44+
const SIDECAR_SILENCE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30);
45+
3946
/// A registered callback that answers a sidecar-initiated request using generated wire types.
4047
pub type WireSidecarCallback = Arc<
4148
dyn Fn(
@@ -68,6 +75,8 @@ pub struct SidecarTransport {
6875
request_writer_tx: mpsc::Sender<Vec<u8>>,
6976
/// Outbound callback/control response frames. The writer drains this before regular requests.
7077
control_writer_tx: mpsc::Sender<Vec<u8>>,
78+
/// When the reader last received any inbound frame; the silence watchdog reads it.
79+
last_inbound_at: parking_lot::Mutex<std::time::Instant>,
7180
}
7281

7382
impl SidecarTransport {
@@ -110,10 +119,15 @@ impl SidecarTransport {
110119
callbacks: SccHashMap::new(),
111120
request_writer_tx,
112121
control_writer_tx,
122+
last_inbound_at: parking_lot::Mutex::new(std::time::Instant::now()),
113123
});
114124

115125
tokio::spawn(run_writer(stdin, control_writer_rx, request_writer_rx));
116126
tokio::spawn(run_reader(Arc::downgrade(&transport), stdout));
127+
tokio::spawn(run_silence_watchdog(
128+
Arc::downgrade(&transport),
129+
SIDECAR_SILENCE_TIMEOUT,
130+
));
117131

118132
Ok(transport)
119133
}
@@ -236,6 +250,17 @@ impl SidecarTransport {
236250
}
237251
}
238252
wire::ProtocolFrame::EventFrame(event) => {
253+
// Transport-level liveness beats from the sidecar. Their arrival
254+
// already reset the silence watchdog in the reader; they carry no
255+
// meaning for event subscribers, so drop them here (mirrors the
256+
// TS client's heartbeat swallow).
257+
if matches!(
258+
&event.payload,
259+
wire::EventPayload::StructuredEvent(structured)
260+
if structured.name == "heartbeat"
261+
) {
262+
return;
263+
}
239264
let _ = self.event_tx.send((event.ownership, event.payload));
240265
}
241266
wire::ProtocolFrame::SidecarRequestFrame(request) => {
@@ -429,6 +454,9 @@ async fn run_reader(transport: Weak<SidecarTransport>, mut stdout: ChildStdout)
429454
if stdout.read_exact(&mut frame_bytes[4..]).await.is_err() {
430455
break;
431456
}
457+
// Any complete inbound frame proves the sidecar is alive; the silence
458+
// watchdog measures from here.
459+
*transport.last_inbound_at.lock() = std::time::Instant::now();
432460

433461
let codec = WireFrameCodec::new(max_frame_bytes);
434462
match codec.decode(&frame_bytes) {
@@ -446,6 +474,30 @@ fn frame_length_exceeds_limit(length: usize, max_frame_bytes: usize) -> bool {
446474
length > max_frame_bytes
447475
}
448476

477+
/// Kill the sidecar and fail all in-flight requests once the transport has seen no inbound frames
478+
/// (not even heartbeats) for `timeout`. A silent sidecar is dead or wedged, not busy: a busy
479+
/// sidecar still heartbeats every 10s from a dedicated thread. Exits when the transport drops.
480+
async fn run_silence_watchdog(transport: Weak<SidecarTransport>, timeout: std::time::Duration) {
481+
let check_interval = (timeout / 4).min(std::time::Duration::from_secs(1));
482+
loop {
483+
tokio::time::sleep(check_interval).await;
484+
let Some(transport) = transport.upgrade() else {
485+
return;
486+
};
487+
let silence = transport.last_inbound_at.lock().elapsed();
488+
if silence < timeout {
489+
continue;
490+
}
491+
tracing::error!(
492+
silence_ms = silence.as_millis() as u64,
493+
"sidecar unresponsive: no protocol frames or heartbeats; killing sidecar",
494+
);
495+
transport.kill_child();
496+
transport.fail_all_pending();
497+
return;
498+
}
499+
}
500+
449501
fn resolve_sidecar_binary_path(binary_path: Option<String>) -> String {
450502
binary_path
451503
.or_else(|| std::env::var(SIDECAR_BIN_ENV).ok())
@@ -473,6 +525,7 @@ mod tests {
473525
callbacks: SccHashMap::new(),
474526
request_writer_tx,
475527
control_writer_tx,
528+
last_inbound_at: parking_lot::Mutex::new(std::time::Instant::now()),
476529
}
477530
}
478531

@@ -601,6 +654,96 @@ mod tests {
601654
));
602655
}
603656

657+
#[tokio::test]
658+
async fn silence_watchdog_fails_pending_requests_after_sustained_silence() {
659+
let transport = Arc::new(test_transport());
660+
let (tx, rx) = oneshot::channel();
661+
transport
662+
.register_pending_request(1, tx)
663+
.expect("register pending request");
664+
665+
tokio::spawn(run_silence_watchdog(
666+
Arc::downgrade(&transport),
667+
std::time::Duration::from_millis(40),
668+
));
669+
670+
// No inbound activity at all: the watchdog must reject the pending
671+
// request (dropped sender -> disconnected error at the caller).
672+
rx.await
673+
.expect_err("watchdog should drop the pending sender");
674+
assert_eq!(pending_request_count(&transport), 0);
675+
}
676+
677+
#[tokio::test]
678+
async fn silence_watchdog_stays_quiet_while_frames_arrive() {
679+
let transport = Arc::new(test_transport());
680+
let (tx, mut rx) = oneshot::channel();
681+
transport
682+
.register_pending_request(1, tx)
683+
.expect("register pending request");
684+
685+
tokio::spawn(run_silence_watchdog(
686+
Arc::downgrade(&transport),
687+
std::time::Duration::from_millis(120),
688+
));
689+
690+
// Simulate steady inbound activity (what the reader does per frame)
691+
// for well past the silence window; the watchdog must not fire.
692+
for _ in 0..6 {
693+
tokio::time::sleep(std::time::Duration::from_millis(40)).await;
694+
*transport.last_inbound_at.lock() = std::time::Instant::now();
695+
assert!(
696+
rx.try_recv().is_err(),
697+
"pending request must remain registered while frames arrive"
698+
);
699+
}
700+
assert_eq!(pending_request_count(&transport), 1);
701+
}
702+
703+
#[tokio::test]
704+
async fn heartbeat_events_are_swallowed_before_the_event_fanout() {
705+
let transport = Arc::new(test_transport());
706+
let mut wire_events = transport.subscribe_wire_events();
707+
708+
transport
709+
.handle_wire_frame(wire::ProtocolFrame::EventFrame(wire::EventFrame {
710+
schema: wire::protocol_schema(),
711+
ownership: wire::OwnershipScope::ConnectionOwnership(wire::ConnectionOwnership {
712+
connection_id: "sidecar-transport".to_string(),
713+
}),
714+
payload: wire::EventPayload::StructuredEvent(wire::StructuredEvent {
715+
name: "heartbeat".to_string(),
716+
detail: std::collections::HashMap::new(),
717+
}),
718+
}))
719+
.await;
720+
// A non-heartbeat structured event still fans out, proving the filter
721+
// is name-scoped rather than dropping all structured events.
722+
transport
723+
.handle_wire_frame(wire::ProtocolFrame::EventFrame(wire::EventFrame {
724+
schema: wire::protocol_schema(),
725+
ownership: wire::OwnershipScope::ConnectionOwnership(wire::ConnectionOwnership {
726+
connection_id: "conn-1".to_string(),
727+
}),
728+
payload: wire::EventPayload::StructuredEvent(wire::StructuredEvent {
729+
name: "limit_warning".to_string(),
730+
detail: std::collections::HashMap::new(),
731+
}),
732+
}))
733+
.await;
734+
735+
let (_, payload) = wire_events.recv().await.expect("structured event");
736+
assert!(matches!(
737+
payload,
738+
wire::EventPayload::StructuredEvent(wire::StructuredEvent { name, .. })
739+
if name == "limit_warning"
740+
));
741+
assert!(
742+
wire_events.try_recv().is_err(),
743+
"heartbeat must not fan out"
744+
);
745+
}
746+
604747
#[test]
605748
fn pending_request_guard_removes_registered_slot_on_drop() {
606749
let transport = test_transport();

packages/core/src/agent-os.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,6 @@ import {
217217
type AuthenticatedSession,
218218
type CreatedVm,
219219
createAgentOsSidecarClient,
220-
NATIVE_SIDECAR_FRAME_TIMEOUT_MS,
221220
NativeSidecarKernelProxy,
222221
type RootFilesystemEntry,
223222
type SidecarMountDescriptor,
@@ -5534,7 +5533,6 @@ function ensureSharedSidecarNativeProcess(
55345533
cwd: REPO_ROOT,
55355534
command: ensureNativeSidecarBinary(),
55365535
args: [],
5537-
frameTimeoutMs: NATIVE_SIDECAR_FRAME_TIMEOUT_MS,
55385536
});
55395537
// Track the child immediately — BEFORE the handshake await — so a
55405538
// failed `authenticateAndOpenSession()` can still reap it (otherwise

packages/core/src/runtime-compat.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import {
1515
type AuthenticatedSession,
1616
type CreatedVm,
1717
type LocalCompatMount,
18-
NATIVE_SIDECAR_FRAME_TIMEOUT_MS,
1918
NativeSidecarKernelProxy,
2019
SidecarProcess,
2120
type RootFilesystemEntry,
@@ -2846,7 +2845,6 @@ class NativeKernel implements Kernel {
28462845
cwd: REPO_ROOT,
28472846
command: ensureNativeSidecarBinary(),
28482847
args: [],
2849-
frameTimeoutMs: NATIVE_SIDECAR_FRAME_TIMEOUT_MS,
28502848
});
28512849
const session = await client.authenticateAndOpenSession();
28522850
const createVmConfig: CreateVmConfig = {

packages/core/src/sidecar/native-process-client.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import "@rivet-dev/agentos-runtime-core/native-client";
66
import { SidecarProcess } from "@rivet-dev/agentos-runtime-core/sidecar-client";
77

88
export {
9-
NATIVE_SIDECAR_FRAME_TIMEOUT_MS,
109
SidecarEventBufferOverflow,
1110
SidecarProcess,
1211
SidecarProcessError,

packages/core/src/sidecar/rpc-client.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2755,7 +2755,6 @@ export type {
27552755
SidecarSpawnOptions,
27562756
} from "./native-process-client.js";
27572757
export {
2758-
NATIVE_SIDECAR_FRAME_TIMEOUT_MS,
27592758
NativeSidecarProcessClient,
27602759
SidecarEventBufferOverflow,
27612760
SidecarProcess,

packages/core/tests/native-sidecar-process-permissions.test.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,6 @@ describe("native sidecar process client permissions", () => {
193193
cwd: REPO_ROOT,
194194
command: "node",
195195
args: [driverPath, capturePath],
196-
frameTimeoutMs: 5_000,
197196
payloadCodec: "json",
198197
});
199198

@@ -306,7 +305,6 @@ describe("native sidecar process client permissions", () => {
306305
cwd: REPO_ROOT,
307306
command: SIDECAR_BINARY,
308307
args: [],
309-
frameTimeoutMs: 20_000,
310308
});
311309

312310
try {
@@ -394,7 +392,6 @@ describe("native sidecar process client permissions", () => {
394392
cwd: REPO_ROOT,
395393
command: SIDECAR_BINARY,
396394
args: [],
397-
frameTimeoutMs: 20_000,
398395
});
399396

400397
try {
@@ -631,7 +628,6 @@ describe("native sidecar process client permissions", () => {
631628
cwd: REPO_ROOT,
632629
command: SIDECAR_BINARY,
633630
args: [],
634-
frameTimeoutMs: 20_000,
635631
});
636632

637633
try {

0 commit comments

Comments
 (0)