From 713cda87d941feb64e025833b5cb399d3dbc76d3 Mon Sep 17 00:00:00 2001 From: Nathan Flurry Date: Thu, 25 Jun 2026 14:10:33 -0700 Subject: [PATCH] fix(sidecar): classify new limit constants, tolerate stale sidecar responses, fix service-test build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes surfaced while syncing agent-os against latest secure-exec main: 1. limits: classify DEFAULT_WASM_RUNNER_HEAP_LIMIT_MB (#129) and MAX_TIMER_DELAY_MS (#131) — both added without inventory entries, so limits_audit failed on main. 2. sidecar: accept_sidecar_response drops a stale sidecar_response with no matching pending request (UnmatchedResponse) or already completed (DuplicateResponse) instead of failing the whole sidecar — a per-VM callback can be answered by the host after that VM is disposed on the shared sidecar process. Real protocol violations stay fatal. 3. tests: re-export crate::EventSinkTransport into the source-included service test crate (#132 added the use in src/service.rs without the matching test re-export, breaking 'cargo test -p secure-exec-sidecar --test service' compilation). Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/sidecar/src/service.rs | 23 ++++++++++++++++--- .../tests/fixtures/limits-inventory.json | 12 ++++++++++ crates/sidecar/tests/service.rs | 2 +- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/crates/sidecar/src/service.rs b/crates/sidecar/src/service.rs index 613d2db6..614bba83 100644 --- a/crates/sidecar/src/service.rs +++ b/crates/sidecar/src/service.rs @@ -2721,9 +2721,26 @@ where &mut self, response: SidecarResponseFrame, ) -> Result<(), SidecarError> { - self.pending_sidecar_responses - .accept_response(&response) - .map_err(sidecar_response_tracker_error)?; + match self.pending_sidecar_responses.accept_response(&response) { + Ok(()) => {} + // A response for a request that is no longer pending (its owning VM + // was disposed, abandoning the in-flight callback) or already + // completed is a benign late/stale reply on the shared sidecar — a + // per-VM `sidecar_request` can be answered by the host after that VM + // has been torn down (multiple VMs share one sidecar process). Drop + // it instead of failing the whole sidecar over a harmless straggler. + Err( + error @ (SidecarResponseTrackerError::UnmatchedResponse { .. } + | SidecarResponseTrackerError::DuplicateResponse { .. }), + ) => { + tracing::warn!( + request_id = response.request_id, + "dropping stale sidecar response with no matching pending request: {error}" + ); + return Ok(()); + } + Err(error) => return Err(sidecar_response_tracker_error(error)), + } self.pending_sidecar_responses_gauge .observe_depth(self.pending_sidecar_responses.pending_count()); self.completed_sidecar_response_order diff --git a/crates/sidecar/tests/fixtures/limits-inventory.json b/crates/sidecar/tests/fixtures/limits-inventory.json index 1a4da305..3c7a15ed 100644 --- a/crates/sidecar/tests/fixtures/limits-inventory.json +++ b/crates/sidecar/tests/fixtures/limits-inventory.json @@ -24,6 +24,12 @@ "rationale": "Guest JS stdout/stderr capture cap.", "wired": "VmLimits.js_runtime.captured_output_limit_bytes" }, + { + "name": "MAX_TIMER_DELAY_MS", + "path": "crates/execution/src/javascript.rs", + "class": "invariant", + "rationale": "Clamps a guest timer delay to the JS setTimeout ceiling (2^31-1 ms); a leak guard so a timer thread cannot outlive its session by pinning the session Arc, mirroring the standard setTimeout max rather than operator policy." + }, { "name": "JAVASCRIPT_EVENT_CHANNEL_CAPACITY", "path": "crates/execution/src/javascript.rs", @@ -103,6 +109,12 @@ "class": "policy-deferred", "rationale": "WASM wall-clock backstop applied when no fuel budget is set; safety default to stop infinite-loop core-pinning, fold into a wasm execution-timeout field later." }, + { + "name": "DEFAULT_WASM_RUNNER_HEAP_LIMIT_MB", + "path": "crates/execution/src/wasm.rs", + "class": "policy-deferred", + "rationale": "Wasm runner V8 heap default (2 GiB, intentionally above the 128 MiB per-guest budget so warmup stops OOMing); per-isolate near-heap guard contains it, operator-tunable via the WASM_RUNNER_HEAP_LIMIT_MB env override rather than VmLimits." + }, { "name": "DEFAULT_WASM_PREWARM_TIMEOUT_MS", "path": "crates/execution/src/wasm.rs", diff --git a/crates/sidecar/tests/service.rs b/crates/sidecar/tests/service.rs index 82d5c9a2..7d56a560 100644 --- a/crates/sidecar/tests/service.rs +++ b/crates/sidecar/tests/service.rs @@ -63,7 +63,7 @@ use extension::{ ExtensionInterruptResponse, ExtensionResponse, }; use service::NativeSidecarConfig; -use state::SidecarRequestTransport; +use state::{EventSinkTransport, SidecarRequestTransport}; #[allow(dead_code)] #[path = "../src/stdio.rs"]