Repository navigation
Conversation
There was a problem hiding this comment.
🔴 1 high-severity finding
Reviewed commit 0a701f7.
🔴 High · Rearm the pump when another queued Python event remains
A claimed Python service returns Value::Null, so this path hits continue before setting emitted_this_round or incrementing work. The outer loop therefore stops after one request, and because the claim count is still below max_service_claims, neither this function nor pump_process_events_nowait restores a notification permit. If the WASM poller requeues two Python events before the supervisor consumes its coalesced Notify edge, the second request remains at the front indefinitely and the child can stay blocked. The new test masks this by calling the pump twice directly. Treat a claimed service as progress so the turn continues up to the service limit, or explicitly rearm when another eligible event remains; the regression test should consume one coalesced notification and verify the second request gets a continuation edge.
Original location: "crates/native-sidecar/src/execution/child_process.rs":2805 (new side, not submitted inline).
Count in-place JavaScript completions as bounded internal work, preserve pull-owned events in order, and retain upstream JavaScript RPC ownership. Add real completion-queue and ownership regressions; merge main at 6a6e18d.
There was a problem hiding this comment.
🟠 1 medium-severity finding
Reviewed commit b476af3.
🟠 Medium · Make the RPC ownership classifier the single source of truth
This classification now controls whether the supervisor or the WASM parent's pull loop owns an RPC, but poll_owned_descendant_javascript_child_process still repeats the same method-name list inline when deciding what to requeue. If either list changes alone, one side can skip a request that the other side also believes it does not own, leaving the synchronous RPC parked. Use this helper in the pull-loop branch as well (with the existing process.signal_state fast path kept explicit) so the ownership boundary cannot drift.
Original location: "crates/native-sidecar/src/execution/child_process.rs":2640 (new side, not submitted inline).
Fixes #2012.
process.exec('python -c "print(42)"')starts Python but times out while Python initializes its filesystem bridge. The WASM parent's poller leaves internal requests for the supervisor, but the supervisor skips every child of a WASM parent. Direct Python execution avoids this path.There is also a shared progress-accounting problem: claiming an owned JavaScript or Python service returns
Value::Null, which the attached and detached descendant pumps treat as no progress. With multiple queued requests and one coalesced notification, a request can remain stranded without a continuation wake-up.This change:
preserve_pull_owned_eventskeeps stdout, stderr, and exit delivery with the parent.The supervisor always supplies
Some(owned_javascript_services)to the descendant poller. A JavaScript request is therefore enqueued and counted before returningValue::Null; the compatibility path's inline JavaScript handler is bypassed. A mixed-queue regression placesprocess.signal_statebefore a Python filesystem request and socket completion and verifies progress from one coalesced notification, both in one batch and across one-request turns. This checks the call-path distinction raised in the follow-up review.Related: #2007 also changes admission of JavaScript process-control requests beneath WASM parents. This PR now covers that admission through the shared internal-event classification; its other dispatch refactoring is separate. The two PRs overlap at the WASM-parent guard.
Validation
0a701f7), both generic JavaScript regressions fail after claiming 1 of 3 queued requests. The strengthened Python regressions fail on the missing continuation notification and on claiming only 1 of 2 queued services.cargo test --locked -p agentos-native-sidecar --test service -- child_event_claim_tests wasm_parent_ --test-threads=1: all 24 tests pass. This includes the mixed JavaScript/Python regression and existing capacity, cancellation, output/exit ordering, deferred socket, and child-write deadline checks.cargo build --locked -p agentos-sidecar --bin agentos-sidecarsucceeds.git diff --checkpass.Full workspace and agent-session suites have not been run. Local builds use cached V8 bridge assets and omit the optional bundled TypeScript compiler. The command-registration workaround and mixed API/guest file consistency issue are separate from this event scheduling fix.