From aa84bba17633d9d6b265fb6a6883c73291ab6926 Mon Sep 17 00:00:00 2001 From: Nathan Flurry Date: Mon, 20 Jul 2026 00:52:37 -0700 Subject: [PATCH] docs(load-testing): correct the ledger after adversarial review Records F-8/F-9/F-10 and corrects two claims this file made: - F-7's premise that the offending write "was already correctly rejected with a typed ENOSPC at the syscall" is false. F-5 is per-file rather than per-VM, misses writeFileSync/copyFileSync entirely, and natively spawned processes write the shadow dir with unmediated host syscalls. - The filesystem-bytes probe passing verified only the byte-limit-on- regular-file case and must not be cited as closing the class. --- docs-internal/load-testing-issues.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs-internal/load-testing-issues.md b/docs-internal/load-testing-issues.md index 0df7061379..b9fa7689fd 100644 --- a/docs-internal/load-testing-issues.md +++ b/docs-internal/load-testing-issues.md @@ -10,6 +10,9 @@ | **F-3** | LT-022 per-lifecycle leak | Root cause: `LocalBridge.snapshots: BTreeMap` (`crates/native-sidecar/src/stdio.rs:2567`) — `finish_vm_teardown` snapshots the **entire root filesystem on every VM dispose** and inserts it (`stdio.rs:2757`), with **no removal anywhere in the tree**, so one full-FS blob (~0.5 MiB) was retained per VM lifecycle for the sidecar's lifetime. Fix: added `PersistenceBridge::forget_filesystem_state(vm_id)` (default no-op, so bridges that persist externally are unaffected) in `crates/bridge/src/lib.rs`, implemented it in `LocalBridge` to drop the entry, and call it **unconditionally** on the dispose path in `crates/native-sidecar/src/vm.rs` alongside `reclaim_vm_tracking` so a failed teardown cannot strand the entry. VM ids are monotonic and never reissued, so a disposed VM's snapshot can never be loaded again. **VERIFIED:** clean soak (batch=1, 200 cycles) after the fix — steady slope **495 KB/cycle → 26.9 KB/cycle (18× reduction)**, total RSS/PSS growth **461 MiB → 29 MiB**, teardown slope ~0, zero probe failures, verdict **pass**. | | **F-6 (CRITICAL — the actual root cause of LT-011/LT-024)** | LT-011 / LT-024 | `crates/native-sidecar/src/execution/javascript/rpc.rs` `deferred_kernel_wait_request_for_process`. This runs on **every** guest `fs.write`/`fs.writeSync` as a pre-check ("is this fd a pipe, so the write can be deferred?") and called `kernel.fd_stat(fd)` unconditionally. Mapped host fds live in a per-process `BTreeMap`, **not** the kernel fd table, so any write to a host-backed path (`/tmp`, `/workspace`) stat'ed an fd the kernel does not own and raised `EBADF: bad file descriptor 1000000000`. That error escaped `pump_process_events` and was the single defect behind BOTH earlier symptoms: it killed the shared sidecar (LT-011 cross-tenant DoS) before F-4, and after F-4 left the guest's sync-RPC response undelivered until the 31 s bridge deadline (LT-024). **Proven, not inferred** — captured backtrace: `FdTableError::bad_file_descriptor → ProcessFdTable::stat → KernelVm::fd_stat → deferred_kernel_wait_request_for_process → handle_javascript_sync_rpc_request → handle_execution_event → pump_process_events`. Fix: return `Ok(None)` (not deferrable) for `fd >= MAPPED_HOST_FD_START`, since a host-backed fd can never be a kernel pipe; plus defense-in-depth treating any `EBADF` from this stat as "not a kernel pipe, so not deferrable" rather than failing the write with an unrelated errno — closing the whole class, not just this fd range. **VERIFIED:** write sweep went from **31,000 ms hang → 1–4 ms success in all 10 combinations** (4 KiB/64 KiB/256 KiB/512 KiB/1 MiB × `/tmp` + `/workspace`), with no EBADF and no pump error. It also made F-5 reachable for the first time: with an 8 MiB cap, chunks 0–7 now succeed and chunk 8 is rejected with a typed `ENOSPC: ... exceeding the VM filesystem limit of 8388608 (limits.resources.maxFilesystemBytes); raise the limit to store more data` — the enforcement the `filesystem-bytes` probe had been asking for all along. | | **F-7 (critical — newly exposed by F-6)** | LT-025 shadow-sync fatal on guest quota | Fixing F-6 made guest writes actually succeed for the first time, which immediately exposed a SECOND fatal path in `crates/native-sidecar/src/execution/launch.rs`: the host-shadow-root → guest mirror called `vm.kernel.write_file(...)` and escalated ANY error to `SidecarError::InvalidState`. When a guest legitimately hit its own configured `maxFilesystemBytes`, the mirror got `ENOSPC` and killed the entire shared sidecar — `agentos-sidecar startup failed error=InvalidState("failed to sync host shadow file .../tmp/fill.bin to guest /tmp/fill.bin: ENOSPC: maximum filesystem size limit reached")` → `SidecarProcessExited` → sentinel died and fresh VMs failed. Same cross-tenant blast-radius class as LT-011: one guest filling its OWN quota takes down every co-located VM/actor. Fix: treat `ENOSPC`/`EDQUOT`/`EFBIG` as guest-caused conditions and `warn!`+skip the entry, matching the tolerance the same loop already had for `ENOENT` (mid-churn path) and `EPERM` (unreadable host file). The offending write is already correctly rejected with a typed ENOSPC at the syscall, so nothing is hidden — the failure stays host-visible via the warning but is bounded to the offending VM. **VERIFIED:** the `filesystem-bytes` probe — which had failed for this entire effort — now reports `{"verdict":"pass","rejected":1}`: the cap fires, the sidecar survives, the sentinel stays healthy, and a fresh VM still runs. | +| **F-8 (correction to F-7)** | LT-011 still reachable; F-7 premise false | Five-lens subagent review found the cross-tenant crash was **narrowed, not closed**, and that F-7's justifying comment was factually wrong. (1) `stdio.rs` event_ready branch called `poll_event_wire(..).await?` unguarded, reaching the SAME pump the notify branch guards — both in one `select!`, so survival depended on scheduling. (2) `launch.rs` tolerated only the write arm; `mkdir` (charged against `maxInodeCount`) and `symlink` (charged against bytes AND inodes) stayed fatal, so inode exhaustion still killed the shared sidecar. Both fixed. Also narrowed the tolerated set to `ENOSPC` alone — `EDQUOT`/`EFBIG` were dead matches nothing on that path constructs, and they signal HOST disk exhaustion, which must keep failing loudly. **Correction to the F-7 record:** its claim that "the offending write was already correctly rejected with a typed ENOSPC at the syscall" is FALSE — F-5 is per-file rather than per-VM, misses `writeFileSync`/`copyFileSync` entirely, and natively spawned processes write to the shadow dir with unmediated host syscalls. The `filesystem-bytes` probe passing verified only the byte-limit-on-regular-file case and must not be cited as closing the class. | +| **F-9** | Guest-reachable host OOM introduced by F-1/F-4 | `packages/runtime-core/src/process.ts` `stderrChunks` was append-only for the sidecar's lifetime. Previously the sidecar DIED on guest-triggered errors, which bounded stderr volume; once those errors became survivable, a guest could drive unbounded `tracing::error!` output that accumulated in the host Node heap — trading a sidecar exit for a host OOM, i.e. strictly worse. Now retains a bounded excerpt (first 16 KiB + last 48 KiB, truncation reported inline) because the root cause is typically the FIRST error while the fatal one is the LAST. Also forwards live stderr unconditionally: the Rust client already spawns with `Stdio::inherit()`, so gating forwarding behind `AGENTOS_SIDECAR_STDERR` left the two clients behaviorally different for identical code, violating the client-parity rule. | +| **F-10** | LT-024 hang — the actual source | `process_events.rs` leases (pops) an internal event before dispatch, so a handler error destroyed it; when it was a `JavascriptSyncRpcRequest` the waiter ceased to exist and hung to the ~31 s bridge deadline. Additionally `service.rs` ran the deferred fd wakes and the chmod shadow mirror with `?` BEFORE response delivery, so a side-effect failure discarded an already-successful result — worst case: the write happened, the guest was told nothing, then it timed out as if it hadn't. All three now always answer the waiter. Answering beats requeueing here: a deterministic handler failure would otherwise retry forever. **Guarding at the run-loop level, as F-1/F-4 did, is too far up the stack** — by then the request the guest waits on is already gone. | | **F-2** | LT-013 unhandled EPIPE | `packages/runtime-core/src/frame-stream.ts` `StdioFrameTransport`: the constructor attached an `error` listener to `stdout` but not `stdin`, so a failed write (EPIPE when the sidecar dies) emitted an unhandled `'error'` event that Node threw as an uncaught exception, killing the host process. Now `stdin` gets the same handler (removed in `dispose`), so it surfaces as a normal typed transport error. |