Skip to content

Commit 713cda8

Browse files
NathanFlurryclaude
andcommitted
fix(sidecar): classify new limit constants, tolerate stale sidecar responses, fix service-test build
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) <noreply@anthropic.com>
1 parent 1f13c36 commit 713cda8

3 files changed

Lines changed: 33 additions & 4 deletions

File tree

‎crates/sidecar/src/service.rs‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2721,9 +2721,26 @@ where
27212721
&mut self,
27222722
response: SidecarResponseFrame,
27232723
) -> Result<(), SidecarError> {
2724-
self.pending_sidecar_responses
2725-
.accept_response(&response)
2726-
.map_err(sidecar_response_tracker_error)?;
2724+
match self.pending_sidecar_responses.accept_response(&response) {
2725+
Ok(()) => {}
2726+
// A response for a request that is no longer pending (its owning VM
2727+
// was disposed, abandoning the in-flight callback) or already
2728+
// completed is a benign late/stale reply on the shared sidecar — a
2729+
// per-VM `sidecar_request` can be answered by the host after that VM
2730+
// has been torn down (multiple VMs share one sidecar process). Drop
2731+
// it instead of failing the whole sidecar over a harmless straggler.
2732+
Err(
2733+
error @ (SidecarResponseTrackerError::UnmatchedResponse { .. }
2734+
| SidecarResponseTrackerError::DuplicateResponse { .. }),
2735+
) => {
2736+
tracing::warn!(
2737+
request_id = response.request_id,
2738+
"dropping stale sidecar response with no matching pending request: {error}"
2739+
);
2740+
return Ok(());
2741+
}
2742+
Err(error) => return Err(sidecar_response_tracker_error(error)),
2743+
}
27272744
self.pending_sidecar_responses_gauge
27282745
.observe_depth(self.pending_sidecar_responses.pending_count());
27292746
self.completed_sidecar_response_order

‎crates/sidecar/tests/fixtures/limits-inventory.json‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@
2424
"rationale": "Guest JS stdout/stderr capture cap.",
2525
"wired": "VmLimits.js_runtime.captured_output_limit_bytes"
2626
},
27+
{
28+
"name": "MAX_TIMER_DELAY_MS",
29+
"path": "crates/execution/src/javascript.rs",
30+
"class": "invariant",
31+
"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."
32+
},
2733
{
2834
"name": "JAVASCRIPT_EVENT_CHANNEL_CAPACITY",
2935
"path": "crates/execution/src/javascript.rs",
@@ -103,6 +109,12 @@
103109
"class": "policy-deferred",
104110
"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."
105111
},
112+
{
113+
"name": "DEFAULT_WASM_RUNNER_HEAP_LIMIT_MB",
114+
"path": "crates/execution/src/wasm.rs",
115+
"class": "policy-deferred",
116+
"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."
117+
},
106118
{
107119
"name": "DEFAULT_WASM_PREWARM_TIMEOUT_MS",
108120
"path": "crates/execution/src/wasm.rs",

‎crates/sidecar/tests/service.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ use extension::{
6363
ExtensionInterruptResponse, ExtensionResponse,
6464
};
6565
use service::NativeSidecarConfig;
66-
use state::SidecarRequestTransport;
66+
use state::{EventSinkTransport, SidecarRequestTransport};
6767

6868
#[allow(dead_code)]
6969
#[path = "../src/stdio.rs"]

0 commit comments

Comments
 (0)