fix(sidecar): classify new limit constants + tolerate stale sidecar callback responses#133
Merged
Merged
Conversation
railway-app
Bot
temporarily deployed
to
secure-exec / secure-exec-pr-133
railway-app
Bot
temporarily deployed
to
rivet-frontend / secure-exec-pr-133
…sponses, 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>
NathanFlurry
force-pushed
the
fix/limits-and-stale-sidecar-response
branch
from
1892f6f to
713cda8
Compare
railway-app
Bot
temporarily deployed
to
rivet-frontend / secure-exec-pr-133
railway-app
Bot
temporarily deployed
to
secure-exec / secure-exec-pr-133
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two fixes surfaced while syncing agent-os against latest secure-exec main.
DEFAULT_WASM_RUNNER_HEAP_LIMIT_MBandMAX_TIMER_DELAY_MSwithout inventory entries, socargo test -p secure-exec-sidecar --test limits_auditfails on main. Classify them:DEFAULT_WASM_RUNNER_HEAP_LIMIT_MB→ policy-deferred (wasm runner V8 heap default; operator-tunable via theWASM_RUNNER_HEAP_LIMIT_MBenv override, contained by the per-isolate near-heap guard).MAX_TIMER_DELAY_MS→ invariant (clamps a guest timer delay to the JSsetTimeout2^31-1 ms ceiling; a leak guard so a timer thread can't outlive its session).accept_sidecar_responsenow drops asidecar_responsewith no matching pending request (UnmatchedResponse) or whose request already completed (DuplicateResponse) instead of failing the whole sidecar. Multiple VMs share one sidecar process; a per-VMsidecar_requestcallback can be answered by the host after that VM is disposed, and the straggler reply must not crash an unrelated VM's startup. Real protocol violations (ownership / response-kind mismatch) stay fatal.Testing
limits_audit2/2 ✅,cargo fmt --check✅,cargo clippy -p secure-exec-sidecar -- -D warnings✅,vm_fetch_kernel_tcpsocket tests ✅🤖 Generated with Claude Code