diff --git a/CLAUDE.md b/CLAUDE.md index 018390e89b..b0298183e0 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 8650f8624c..df2ed11d61 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 c3ead9d99e..6e1a952858 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 cf70285057..e4f0344793 100644 --- a/packages/core/src/runtime-compat.ts +++ b/packages/core/src/runtime-compat.ts @@ -426,8 +426,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; @@ -2674,7 +2674,7 @@ class NativeKernel implements Kernel { mountPath: string, filesystem: VirtualFileSystem, options?: { readOnly?: boolean }, - ): void { + ): void | Promise { const localMount = makeLocalCompatMount({ path: mountPath, fs: filesystem, @@ -2685,15 +2685,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, @@ -2701,7 +2702,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 8085ce6001..62d8d1c17a 100644 --- a/packages/core/src/runtime.ts +++ b/packages/core/src/runtime.ts @@ -330,8 +330,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 ad1e02a754..84490625ff 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; @@ -1504,7 +1536,7 @@ export class NativeSidecarKernelProxy { path: string, driver: VirtualFileSystem, options?: { readOnly?: boolean; sidecarMount?: SidecarMountDescriptor }, - ): void { + ): Promise { this.localMounts.unshift({ path: posixPath.normalize(path), fs: driver, @@ -1514,18 +1546,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[] { @@ -1550,11 +1591,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 0000000000..a4a366766f --- /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 1e09f99e79..26880e541b 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 886835151d..c74d92f51c 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 6cd0b80642..491bcf8e0e 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.