From b461a0dff20c967162fbb8d2014dadca1d71caf7 Mon Sep 17 00:00:00 2001 From: Nathan Flurry Date: Tue, 7 Jul 2026 19:23:43 -0700 Subject: [PATCH 1/3] fix(agentos-sidecar): harden the close_session already-exited short-circuit - Builds on #1640: also short-circuit on the session record already marked closed (covers exits the kill RPC does not report), and skip the second wait when SIGKILL itself reports the process gone. - Mirror the short-circuit in the agentos-sidecar-core sync engine, with regression tests proving zero dead waits. --- crates/agentos-sidecar-core/src/engine.rs | 134 +++++++++++++++++++- crates/agentos-sidecar/src/acp_extension.rs | 64 +++++----- 2 files changed, 160 insertions(+), 38 deletions(-) diff --git a/crates/agentos-sidecar-core/src/engine.rs b/crates/agentos-sidecar-core/src/engine.rs index 3d1235d40..8c7ab2558 100644 --- a/crates/agentos-sidecar-core/src/engine.rs +++ b/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( @@ -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, + killed: Vec<(String, String)>, + waits: usize, + } + + impl AcpHost for GoneAdapterHost { + fn spawn_agent(&mut self, _: SpawnAgentRequest) -> Result { + 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, 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, 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, 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 diff --git a/crates/agentos-sidecar/src/acp_extension.rs b/crates/agentos-sidecar/src/acp_extension.rs index 9cedadb7e..4f2ef4149 100644 --- a/crates/agentos-sidecar/src/acp_extension.rs +++ b/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; } From 190fdca5e9e3254bc83feec14731ef378646bbca Mon Sep 17 00:00:00 2001 From: Nathan Flurry Date: Tue, 7 Jul 2026 19:27:11 -0700 Subject: [PATCH 2/3] fix(native-sidecar): resolve js_bridge mount-root realpath without a bridge round-trip - Resolve `realpath("/")` of a js_bridge mount locally (mirroring host_dir): host-side drivers often cannot canonicalize their own root and returned ENOENT, which aborted kernel permission-subject resolution before the readDir bridge call was ever issued. - Regression test: readdir of a js_bridge mount root against a driver whose realpath always fails. --- .../native-sidecar/src/plugins/js_bridge.rs | 12 ++ crates/native-sidecar/tests/service.rs | 114 +++++++++++++++++- 2 files changed, 121 insertions(+), 5 deletions(-) diff --git a/crates/native-sidecar/src/plugins/js_bridge.rs b/crates/native-sidecar/src/plugins/js_bridge.rs index 06c98f6bb..1391b74b3 100644 --- a/crates/native-sidecar/src/plugins/js_bridge.rs +++ b/crates/native-sidecar/src/plugins/js_bridge.rs @@ -517,6 +517,18 @@ impl VirtualFileSystem for JsBridgeFilesystem { } fn realpath(&self, path: &str) -> VfsResult { + // 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, diff --git a/crates/native-sidecar/tests/service.rs b/crates/native-sidecar/tests/service.rs index 6ea4daf03..cf0c6fbfb 100644 --- a/crates/native-sidecar/tests/service.rs +++ b/crates/native-sidecar/tests/service.rs @@ -992,6 +992,19 @@ ykAheWCsAteSEWVc0w==\n\ ) -> ( Arc>, Arc>>, + ) { + install_memory_js_bridge_handler_with_options(sidecar, false) + } + + /// `fail_realpath` simulates host-side drivers that cannot canonicalize + /// their own paths and answer every `realpath` bridge call with ENOENT + /// — the shape that used to break readdir of a js_bridge mount root. + fn install_memory_js_bridge_handler_with_options( + sidecar: &mut NativeSidecar, + fail_realpath: bool, + ) -> ( + Arc>, + Arc>>, ) { let filesystem = Arc::new(Mutex::new(MemoryFileSystem::new())); let calls = Arc::new(Mutex::new(Vec::::new())); @@ -1111,11 +1124,15 @@ ykAheWCsAteSEWVc0w==\n\ .map_err(|error| format!("{}: {error}", error.code())) } "realpath" => { - let path = call_args["path"].as_str().expect("realpath path"); - filesystem - .realpath(path) - .map(|resolved| Some(json!(resolved))) - .map_err(|error| format!("{}: {error}", error.code())) + if fail_realpath { + Err(String::from("ENOENT: no such file or directory")) + } else { + let path = call_args["path"].as_str().expect("realpath path"); + filesystem + .realpath(path) + .map(|resolved| Some(json!(resolved))) + .map_err(|error| format!("{}: {error}", error.code())) + } } "symlink" => { let target = call_args["target"].as_str().expect("symlink target"); @@ -7458,6 +7475,92 @@ ykAheWCsAteSEWVc0w==\n\ .expect_err("stat should fail"); assert_eq!(stat_error.code(), "EIO"); } + + fn configure_vm_js_bridge_mount_readdir_of_mount_root_survives_broken_driver_realpath() { + // Regression: readdir of a js_bridge mount root used to fail with + // ENOENT before any readDir bridge call was issued. The kernel + // permission wrapper resolves every subject through `realpath` + // first, and host-side drivers that cannot canonicalize their own + // root answer the mount-root realpath with ENOENT — which + // `MountTable::realpath` propagates for non-symlink-leaf mounts. + // The mount root must resolve locally, without a bridge realpath. + let mut sidecar = create_test_sidecar(); + let (filesystem, calls) = + install_memory_js_bridge_handler_with_options(&mut sidecar, true); + filesystem + .lock() + .expect("lock js bridge fs") + .write_file("/hello.txt", b"hi".to_vec()) + .expect("seed js bridge fs"); + + let (connection_id, session_id) = + authenticate_and_open_session(&mut sidecar).expect("authenticate and open session"); + let vm_id = create_vm( + &mut sidecar, + &connection_id, + &session_id, + PermissionsPolicy::allow_all(), + ) + .expect("create vm"); + + sidecar + .dispatch_blocking(request( + 4, + OwnershipScope::vm(&connection_id, &session_id, &vm_id), + RequestPayload::ConfigureVm(ConfigureVmRequest { + mounts: vec![MountDescriptor { + guest_path: String::from("/workspace"), + read_only: false, + plugin: MountPluginDescriptor { + id: String::from("js_bridge"), + config: json!({ "mountId": "mount-root" }).to_string(), + }, + }], + software: Vec::new(), + permissions: None, + module_access_cwd: None, + instructions: Vec::new(), + projected_modules: Vec::new(), + command_permissions: std::collections::HashMap::new(), + loopback_exempt_ports: Vec::new(), + packages: Vec::new(), + packages_mount_at: String::new(), + bootstrap_commands: Vec::new(), + tool_shim_commands: Vec::new(), + }), + )) + .expect("configure js_bridge mount"); + + let vm = sidecar.vms.get_mut(&vm_id).expect("configured vm"); + let entries = vm + .kernel + .filesystem_mut() + .read_dir("/workspace") + .expect("readdir of js_bridge mount root"); + assert!( + entries.iter().any(|entry| entry == "hello.txt"), + "mount-root readdir should list seeded entries, got {entries:?}" + ); + + let calls = calls.lock().expect("lock js bridge calls"); + assert!( + calls + .iter() + .any(|call| call.operation == "readDir" + || call.operation == "readDirWithTypes"), + "readdir must reach the bridge; recorded operations: {:?}", + calls + .iter() + .map(|call| call.operation.clone()) + .collect::>() + ); + } + + #[test] + fn configure_vm_js_bridge_mount_root_readdir_without_bridge_realpath() { + configure_vm_js_bridge_mount_readdir_of_mount_root_survives_broken_driver_realpath(); + } + fn configure_vm_instantiates_sandbox_agent_mounts_through_the_plugin_registry() { let server = MockSandboxAgentServer::start("agentos-native-sidecar-sandbox", None); fs::write(server.root().join("hello.txt"), "hello from sandbox") @@ -21012,6 +21115,7 @@ console.log(JSON.stringify({ configure_vm_js_bridge_mount_rejects_oversized_read_payloads(); configure_vm_js_bridge_mount_rejects_pread_payloads_above_requested_length(); configure_vm_js_bridge_mount_maps_callback_errors_to_errno_codes(); + configure_vm_js_bridge_mount_readdir_of_mount_root_survives_broken_driver_realpath(); configure_vm_instantiates_sandbox_agent_mounts_through_the_plugin_registry(); configure_vm_instantiates_s3_mounts_through_the_plugin_registry(); configure_vm_instantiates_object_s3_mounts_through_the_plugin_registry(); From a46dab59596ee6213b366c81654c18d620094409 Mon Sep 17 00:00:00 2001 From: Nathan Flurry Date: Tue, 7 Jul 2026 19:40:50 -0700 Subject: [PATCH 3/3] fix(core): deliver post-boot mountFs reconfiguration reliably to the native sidecar - `mountFs`/`unmountFs` now return a promise that settles once the sidecar has applied the mount; delivery failures reject instead of being swallowed. - Runtime mount reconfigures resend the boot `packages`/`packagesMountAt`/`toolShimCommands`: Rust `configure_vm` rebuilds the whole VM configuration from each payload, so omitting them stripped the `/opt/agentos` projections and tool shims as a side effect of a post-boot mount. - Runtime `linkSoftware` packages are retained too, so a later `mountFs` no longer unprojects them. - The Rust client `mount_fs` remains in-process-only; sidecar-visible runtime mounts there are a follow-up. --- CLAUDE.md | 7 + examples/filesystem/mount-custom-vfs.ts | 2 +- packages/core/src/agent-os.ts | 26 ++- packages/core/src/runtime-compat.ts | 13 +- packages/core/src/runtime.ts | 4 +- packages/core/src/sidecar/rpc-client.ts | 60 ++++++- packages/core/tests/mount-reconfigure.test.ts | 154 ++++++++++++++++++ packages/core/tests/mount.test.ts | 10 +- packages/runtime-core/src/kernel-proxy.ts | 4 + website/src/content/docs/docs/filesystem.mdx | 2 +- 10 files changed, 256 insertions(+), 26 deletions(-) create mode 100644 packages/core/tests/mount-reconfigure.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 018390e89..b0298183e 100644 --- a/CLAUDE.md +++ b/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 diff --git a/examples/filesystem/mount-custom-vfs.ts b/examples/filesystem/mount-custom-vfs.ts index 8650f8624..df2ed11d6 100644 --- a/examples/filesystem/mount-custom-vfs.ts +++ b/examples/filesystem/mount-custom-vfs.ts @@ -3,5 +3,5 @@ import { AgentOs, createInMemoryFileSystem } from "@rivet-dev/agentos-core"; const vm = await AgentOs.create({ defaultSoftware: false }); const driver = createInMemoryFileSystem(); -vm.mountFs("/home/agentos/scratch", driver); +await vm.mountFs("/home/agentos/scratch", driver); await vm.writeFile("/home/agentos/scratch/hello.txt", "hello"); diff --git a/packages/core/src/agent-os.ts b/packages/core/src/agent-os.ts index c3ead9d99..6e1a95285 100644 --- a/packages/core/src/agent-os.ts +++ b/packages/core/src/agent-os.ts @@ -2821,6 +2821,12 @@ export class AgentOs { permissions: sidecarPermissions, commandPermissions: {}, loopbackExemptPorts: options?.loopbackExemptPorts, + // Retained for runtime mount reconfigures: `configure_vm` is + // replace-on-write for the whole payload, so post-boot mountFs + // must resend the boot packages and tool shims. + packages: sidecarPackages, + packagesMountAt: OPT_AGENTOS_ROOT, + toolShimCommands: toolBootstrapCommands, commandGuestPaths, onDispose: cleanup, // The native process is owned by the AgentOsSidecar handle and @@ -3238,18 +3244,24 @@ export class AgentOs { ); } - mountFs( + /** + * Mount a filesystem into the running VM. Resolves once the mount has been + * delivered to the native sidecar, so guest code can use it immediately + * after the returned promise settles; a delivery failure rejects instead of + * leaving the mount silently host-only. + */ + async mountFs( path: string, driver: VirtualFileSystem, options?: { readOnly?: boolean }, - ): void { + ): Promise { this._assertSafeAbsolutePath(path); - this.#kernel.mountFs(path, driver, { readOnly: options?.readOnly }); + await this.#kernel.mountFs(path, driver, { readOnly: options?.readOnly }); } - unmountFs(path: string): void { + async unmountFs(path: string): Promise { this._assertSafeAbsolutePath(path); - this.#kernel.unmountFs(path); + await this.#kernel.unmountFs(path); } async move(from: string, to: string): Promise { @@ -3541,6 +3553,10 @@ export class AgentOs { ]), ), ); + // Retain the linked package for runtime mount reconfigures: + // `configure_vm` is replace-on-write, so a later `mountFs` that + // resent only the boot packages would unproject this one. + this.#kernel.registerLinkedPackage({ path: ref.path }); } // The client parses no manifests: an `agent` block in the linked package is // picked up by the sidecar (it owns the projected `/opt/agentos` and answers diff --git a/packages/core/src/runtime-compat.ts b/packages/core/src/runtime-compat.ts index 1c1c1935a..e3d065e22 100644 --- a/packages/core/src/runtime-compat.ts +++ b/packages/core/src/runtime-compat.ts @@ -429,8 +429,8 @@ export interface Kernel extends KernelInterface { path: string, fs: VirtualFileSystem, options?: { readOnly?: boolean }, - ): void; - unmountFs(path: string): void; + ): void | Promise; + unmountFs(path: string): void | Promise; readFile(path: string): Promise; writeFile(path: string, content: string | Uint8Array): Promise; mkdir(path: string): Promise; @@ -2677,7 +2677,7 @@ class NativeKernel implements Kernel { mountPath: string, filesystem: VirtualFileSystem, options?: { readOnly?: boolean }, - ): void { + ): void | Promise { const localMount = makeLocalCompatMount({ path: mountPath, fs: filesystem, @@ -2688,15 +2688,16 @@ class NativeKernel implements Kernel { (left, right) => right.path.length - left.path.length, ); if (!this.proxy) { + // Pre-boot mounts apply during kernel initialization. return; } - this.proxy.mountFs(mountPath, filesystem, { + return this.proxy.mountFs(mountPath, filesystem, { readOnly: localMount.readOnly, sidecarMount: localMount.sidecarMount, }); } - unmountFs(mountPath: string): void { + unmountFs(mountPath: string): void | Promise { const normalized = normalizePath(mountPath); const pendingIndex = this.pendingLocalMounts.findIndex( (mount) => mount.path === normalized, @@ -2704,7 +2705,7 @@ class NativeKernel implements Kernel { if (pendingIndex >= 0) { this.pendingLocalMounts.splice(pendingIndex, 1); } - this.proxy?.unmountFs(mountPath); + return this.proxy?.unmountFs(mountPath); } async readFile(targetPath: string): Promise { diff --git a/packages/core/src/runtime.ts b/packages/core/src/runtime.ts index ae9db50ae..8f3f06548 100644 --- a/packages/core/src/runtime.ts +++ b/packages/core/src/runtime.ts @@ -333,8 +333,8 @@ export interface Kernel extends KernelInterface { path: string, fs: VirtualFileSystem, options?: { readOnly?: boolean }, - ): void; - unmountFs(path: string): void; + ): void | Promise; + unmountFs(path: string): void | Promise; readFile(path: string): Promise; writeFile(path: string, content: string | Uint8Array): Promise; mkdir(path: string): Promise; diff --git a/packages/core/src/sidecar/rpc-client.ts b/packages/core/src/sidecar/rpc-client.ts index f3ce2d4d9..8c6a225ec 100644 --- a/packages/core/src/sidecar/rpc-client.ts +++ b/packages/core/src/sidecar/rpc-client.ts @@ -339,6 +339,16 @@ interface NativeSidecarKernelProxyOptions { SidecarProcess["configureVm"] >[2]["commandPermissions"]; loopbackExemptPorts?: number[]; + /** + * The boot `configureVm` payload pieces beyond mounts/permissions. Rust + * `configure_vm` rebuilds the whole VM configuration from each payload, so + * every runtime mount reconfigure must resend these or a post-boot + * `mountFs()` silently drops the `/opt/agentos` package projections and + * tool shim commands applied at boot. + */ + packages?: Parameters[2]["packages"]; + packagesMountAt?: string; + toolShimCommands?: string[]; commandGuestPaths: ReadonlyMap; onWasmCommandResolved?: (command: string) => void; onDispose?: () => Promise; @@ -371,6 +381,13 @@ export class NativeSidecarKernelProxy { | Parameters[2]["commandPermissions"] | undefined; private readonly loopbackExemptPorts: number[] | undefined; + // Mutable: runtime `linkSoftware` appends via `registerLinkedPackage` so + // later mount reconfigures resend linked packages too, not just boot ones. + private packages: NonNullable< + Parameters[2]["packages"] + >; + private readonly packagesMountAt: string | undefined; + private readonly toolShimCommands: string[] | undefined; private readonly commandDrivers: Map; private readonly onWasmCommandResolved: | ((command: string) => void) @@ -420,6 +437,9 @@ export class NativeSidecarKernelProxy { this.permissions = options.permissions; this.commandPermissions = options.commandPermissions; this.loopbackExemptPorts = options.loopbackExemptPorts; + this.packages = options.packages ? [...options.packages] : []; + this.packagesMountAt = options.packagesMountAt; + this.toolShimCommands = options.toolShimCommands; this.commandDrivers = buildCommandMap(options.commandGuestPaths); this.onWasmCommandResolved = options.onWasmCommandResolved; this.onDispose = options.onDispose; @@ -449,6 +469,18 @@ export class NativeSidecarKernelProxy { } } + /** + * Record a runtime-linked package (`linkSoftware`) so mount reconfigures + * resend it. Rust `configure_vm` rebuilds the whole VM configuration from + * each payload, so a linked package omitted here would be silently + * unprojected from `/opt/agentos` by the next `mountFs`/`unmountFs`. + */ + registerLinkedPackage(descriptor: { path: string }): void { + if (!this.packages.some((pkg) => pkg.path === descriptor.path)) { + this.packages.push({ path: descriptor.path }); + } + } + async dispose(): Promise { if (this.disposed) { return; @@ -1506,7 +1538,7 @@ export class NativeSidecarKernelProxy { path: string, driver: VirtualFileSystem, options?: { readOnly?: boolean; sidecarMount?: SidecarMountDescriptor }, - ): void { + ): Promise { this.localMounts.unshift({ path: posixPath.normalize(path), fs: driver, @@ -1516,18 +1548,27 @@ export class NativeSidecarKernelProxy { this.localMounts.sort( (left, right) => right.path.length - left.path.length, ); - void this.reconfigureSidecarMounts().catch(() => {}); + // Resolves once the sidecar has the mount; a swallowed rejection here + // used to turn reconfigure failures into silently missing guest mounts. + // The local catch only guards callers that drop the promise — awaiting + // callers still observe the rejection. + const applied = this.reconfigureSidecarMounts(); + applied.catch(() => {}); + return applied; } - unmountFs(path: string): void { + unmountFs(path: string): Promise { const normalized = posixPath.normalize(path); const index = this.localMounts.findIndex( (mount) => mount.path === normalized, ); - if (index >= 0) { - this.localMounts.splice(index, 1); - void this.reconfigureSidecarMounts().catch(() => {}); + if (index < 0) { + return Promise.resolve(); } + this.localMounts.splice(index, 1); + const applied = this.reconfigureSidecarMounts(); + applied.catch(() => {}); + return applied; } private desiredSidecarMounts(): SidecarMountDescriptor[] { @@ -1552,11 +1593,18 @@ export class NativeSidecarKernelProxy { if (this.disposed) { return; } + // Rust `configure_vm` rebuilds the whole VM configuration from this + // payload, so resend the boot packages / tool shim commands too — + // omitting them here strips the `/opt/agentos` projections and tool + // shims from the VM as a side effect of a runtime mount change. await this.client.configureVm(this.session, this.vm, { mounts: this.desiredSidecarMounts(), permissions: this.permissions, commandPermissions: this.commandPermissions, loopbackExemptPorts: this.loopbackExemptPorts, + packages: this.packages, + packagesMountAt: this.packagesMountAt, + toolShimCommands: this.toolShimCommands, }); }; const previous = this.mountReconfigurePromise ?? Promise.resolve(); diff --git a/packages/core/tests/mount-reconfigure.test.ts b/packages/core/tests/mount-reconfigure.test.ts new file mode 100644 index 000000000..a4a366766 --- /dev/null +++ b/packages/core/tests/mount-reconfigure.test.ts @@ -0,0 +1,154 @@ +import { describe, expect, it } from "vitest"; +import { createInMemoryFileSystem } from "../src/runtime-compat.js"; +import { NativeSidecarKernelProxy } from "../src/sidecar/rpc-client.js"; + +// Regression coverage for post-boot mountFs delivery to the native sidecar: +// 1. Rust `configure_vm` rebuilds the whole VM configuration from each +// payload, so a runtime mount reconfigure that omits the boot `packages` / +// `packagesMountAt` / `toolShimCommands` strips the `/opt/agentos` +// projections and tool shims from the VM as a side effect. +// 2. mountFs used to be fire-and-forget with a swallowed rejection, so a +// failed reconfigure left the mount silently host-only and callers had no +// way to know when (or whether) the guest could see it. +// The proxy is exercised against a stub SidecarProcess so the test stays fast +// and deterministic without booting a real VM. + +const session = { connectionId: "conn-1", sessionId: "sess-1" }; +const vm = { vmId: "vm-test" }; + +const bootPackages = [ + { + name: "common", + version: "1.0.0", + path: "/tmp/common.aospkg", + }, +]; +const bootToolShims = ["agentos", "agentos-demo"]; + +function createStubClient(options?: { failConfigureVm?: boolean }) { + const configureCalls: Array> = []; + const client = { + async configureVm( + _session: unknown, + _vm: unknown, + payload: Record, + ) { + configureCalls.push(payload); + if (options?.failConfigureVm) { + throw new Error("configure_vm rejected"); + } + return { + appliedMounts: [], + appliedSoftware: [], + projectedCommands: [], + agents: [], + }; + }, + async disposeVm() {}, + async dispose() {}, + waitForEvent( + _filter: unknown, + _unused: unknown, + opts: { signal: AbortSignal }, + ) { + return new Promise((_resolve, reject) => { + opts.signal.addEventListener("abort", () => + reject(new Error("aborted")), + ); + }); + }, + }; + return { client, configureCalls }; +} + +function createProxy(client: unknown) { + const options = { + client, + session, + vm, + env: {}, + cwd: "/work", + localMounts: [], + sidecarMounts: [], + packages: bootPackages, + packagesMountAt: "/opt/agentos", + toolShimCommands: bootToolShims, + commandGuestPaths: new Map(), + ownsClient: true, + }; + return new NativeSidecarKernelProxy( + options as ConstructorParameters[0], + ); +} + +describe("post-boot mount reconfiguration", () => { + it("resends the boot packages and tool shims on runtime mountFs", async () => { + const { client, configureCalls } = createStubClient(); + const proxy = createProxy(client); + + await proxy.mountFs("/mnt/dynamic", createInMemoryFileSystem()); + + expect(configureCalls).toHaveLength(1); + const payload = configureCalls[0]; + expect(payload.packages).toEqual(bootPackages); + expect(payload.packagesMountAt).toBe("/opt/agentos"); + expect(payload.toolShimCommands).toEqual(bootToolShims); + expect(payload.mounts).toEqual([ + expect.objectContaining({ guestPath: "/mnt/dynamic" }), + ]); + + await proxy.unmountFs("/mnt/dynamic"); + expect(configureCalls).toHaveLength(2); + expect(configureCalls[1].mounts).toEqual([]); + expect(configureCalls[1].packages).toEqual(bootPackages); + expect(configureCalls[1].toolShimCommands).toEqual(bootToolShims); + + await proxy.dispose(); + }); + + it("resends runtime-linked packages on later mount reconfigures", async () => { + const { client, configureCalls } = createStubClient(); + const proxy = createProxy(client); + + // linkSoftware() records the linked package on the proxy; a later + // mountFs must resend it alongside the boot packages or configure_vm + // (replace-on-write) unprojects it from /opt/agentos. + proxy.registerLinkedPackage({ path: "/tmp/linked.aospkg" }); + await proxy.mountFs("/mnt/dynamic", createInMemoryFileSystem()); + expect(configureCalls[0].packages).toEqual([ + ...bootPackages, + { path: "/tmp/linked.aospkg" }, + ]); + + // Duplicate registration is a no-op. + proxy.registerLinkedPackage({ path: "/tmp/linked.aospkg" }); + await proxy.unmountFs("/mnt/dynamic"); + expect(configureCalls[1].packages).toEqual([ + ...bootPackages, + { path: "/tmp/linked.aospkg" }, + ]); + + await proxy.dispose(); + }); + + it("rejects the mountFs promise when sidecar delivery fails", async () => { + const { client } = createStubClient({ failConfigureVm: true }); + const proxy = createProxy(client); + + await expect( + proxy.mountFs("/mnt/dynamic", createInMemoryFileSystem()), + ).rejects.toThrow("configure_vm rejected"); + + await proxy.dispose(); + }); + + it("resolves unmountFs immediately for an unknown mount without reconfiguring", async () => { + const { client, configureCalls } = createStubClient(); + const proxy = createProxy(client); + + await proxy.unmountFs("/mnt/never-mounted"); + expect(configureCalls).toHaveLength(0); + + await proxy.dispose(); + }); +}); diff --git a/packages/core/tests/mount.test.ts b/packages/core/tests/mount.test.ts index 1e09f99e7..26880e541 100644 --- a/packages/core/tests/mount.test.ts +++ b/packages/core/tests/mount.test.ts @@ -119,12 +119,12 @@ describe("mount integration", () => { test("runtime mountFs and unmountFs work", async () => { vm = await createMountVm(); - vm.mountFs("/mnt/dynamic", createInMemoryFileSystem()); + await vm.mountFs("/mnt/dynamic", createInMemoryFileSystem()); await vm.writeFile("/mnt/dynamic/test.txt", "dynamic"); const data = await vm.readFile("/mnt/dynamic/test.txt"); expect(new TextDecoder().decode(data)).toBe("dynamic"); - vm.unmountFs("/mnt/dynamic"); + await vm.unmountFs("/mnt/dynamic"); await expect(vm.readFile("/mnt/dynamic/test.txt")).rejects.toThrow(); // Runtime mount + unmount each trigger a full sidecar reconfigure, so this // integration test needs more than the 30s default (see PR #1521 CI). @@ -134,7 +134,7 @@ describe("mount integration", () => { const mounted = createRecordingFilesystem(); vm = await createMountVm(); - vm.mountFs("/mnt/custom", mounted.fs); + await vm.mountFs("/mnt/custom", mounted.fs); await vm.writeFile("/mnt/custom/note.txt", "from custom vfs"); expect( @@ -143,7 +143,7 @@ describe("mount integration", () => { expect(mounted.calls).toContain("writeFile:/note.txt"); expect(mounted.calls).toContain("readFile:/note.txt"); - vm.unmountFs("/mnt/custom"); + await vm.unmountFs("/mnt/custom"); await expect(vm.readFile("/mnt/custom/note.txt")).rejects.toThrow(); // Runtime mount + unmount each trigger a full sidecar reconfigure, so this // integration test needs more than the 30s default (see PR #1521 CI). @@ -177,7 +177,7 @@ describe("mount integration", () => { const mounted = createRecordingFilesystem(); vm = await createMountVm(); - vm.mountFs("/mnt/custom", mounted.fs); + await vm.mountFs("/mnt/custom", mounted.fs); await vm.writeFile("/mnt/custom/host.txt", "from host api"); const result = await vm.execArgv("node", [ "-e", diff --git a/packages/runtime-core/src/kernel-proxy.ts b/packages/runtime-core/src/kernel-proxy.ts index 7eb8e55be..d41b7c0a5 100644 --- a/packages/runtime-core/src/kernel-proxy.ts +++ b/packages/runtime-core/src/kernel-proxy.ts @@ -1262,6 +1262,10 @@ export class NativeSidecarKernelProxy { return this.client.rename(this.session, this.vm, oldPath, newPath); } + // Test-runtime only: runtime mounts registered here stay host-side local + // compat mounts and are not delivered to the sidecar. The production proxy + // in @rivet-dev/agentos-core reconfigures sidecar mounts on every + // mountFs/unmountFs. mountFs( path: string, driver: VirtualFileSystem, diff --git a/website/src/content/docs/docs/filesystem.mdx b/website/src/content/docs/docs/filesystem.mdx index 6cd0b8064..491bcf8e0 100644 --- a/website/src/content/docs/docs/filesystem.mdx +++ b/website/src/content/docs/docs/filesystem.mdx @@ -50,7 +50,7 @@ Use the built-in `memory` plugin for an ephemeral mounted directory in the nativ -Use `mountFs()` for a callback-backed JS filesystem driver. The driver must live in the same JS process as the `AgentOs` instance, such as direct core usage or a custom RivetKit actor that owns an `AgentOs` instance. +Use `mountFs()` for a callback-backed JS filesystem driver. The driver must live in the same JS process as the `AgentOs` instance, such as direct core usage or a custom RivetKit actor that owns an `AgentOs` instance. `mountFs()` works on a running VM too — it returns a promise that resolves once the mount is visible to guest code, and rejects if delivery to the runtime fails.