Repository navigation
refactor(daemon): run in-process invocations and MCP dispatch natively - #4084
Conversation
serve_loopback takes a ConnectionExecutor. The hosted dashboard runs its serve loop and every connection on the native runtime; the daemon's HTTP application listener keeps Tokio connections, because /remote-deletions reaches project_server_lifecycle inside its connection task. The hosted dashboard task becomes a NativeTask with the same stop, abort at deadline, and abort-on-drop handling.
…efactor/3736-t4-in-process-native
…to refactor/3736-t4-in-process-native
…into refactor/3736-t4-in-process-native
… refactor/3736-t4-in-process-native
…or' into refactor/3736-t4-in-process-native
…to refactor/3736-t4-in-process-native
…bootstrap' into refactor/3736-t4-in-process-native
…d' into refactor/3736-t4-in-process-native
… refactor/3736-t4-in-process-native
…r' into refactor/3736-t4-in-process-native
…nto refactor/3736-t4-in-process-native
The in-process invocation executor and the MCP tool dispatch registry spawned on the daemon's Tokio runtime because the operations they run reached Tokio-only sites: Work attempts, the hosted dashboard, the memory curator, the scheduler reconcile, hook orchestration. Those now run natively, so the executor spawns its invocation and settlement as NativeTasks and the dispatch registry keeps a NativeTaskSet. The controlled RMCP test fixture now polls native timers, since its dispatched executor runs on a native worker. The test hazard list drops the Tokio sites that no longer exist. tracedecay-mcp's production Tokio features drop rt. The recv_writer_reply block_in_place guard stays and now names its remaining Tokio caller, the loopback HTTP application, which project_server_lifecycle.rs keeps on Tokio.
… refactor/3736-t4-in-process-native
recv_writer_reply handed a Tokio worker's run queue away during a writer wait. No Tokio worker in the daemon's acceptance suites waits on the writer any more except the loopback HTTP listener's remote-node provisioning, which now runs on a blocking-pool thread, so the branch and tokio's rt-multi-thread feature go. The native-thread writer-wait counter and its log stay. The rusqlite-runtime test that pinned the Tokio hand-off is deleted with it.
…nto refactor/3736-t4-in-process-native
|
There was a problem hiding this comment.
Devin Review found 3 potential issues.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| let next = tracedecay_runtime_core::native_runtime::spawn_blocking(move || { | ||
| for storage in storages { | ||
| match storage.compare_and_swap(expected_revision, &next).ok()? { | ||
| tracedecay_store::runtime::ScopeSetCasOutcomeV1::Applied(_) => {} | ||
| tracedecay_store::runtime::ScopeSetCasOutcomeV1::Conflict { .. } => { | ||
| let stored = storage.read(next.scope_set_id()).ok()?; | ||
| if stored.as_ref() != Some(&next) { | ||
| return None; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Some(next) | ||
| }) | ||
| .await | ||
| .ok()??; |
There was a problem hiding this comment.
🔴 Scope-set writes outlive cancelled invocations
When cancellation drops compare_and_swap_scope_set during its blocking-pool wait, the queued writes continue without an authoritative reply. The caller can report cancellation while replicas change afterward.
Learn more
A scope-set compare-and-swap updates the active project's store and each authorized replica. spawn_blocking queues this entire write loop independently of its returned future; dropping the future abandons only the wait (spawn_blocking). A cancelled invocation can drop the wait after the loop has begun, leaving the writes to complete without a returned applied outcome. Previously the synchronous loop could not be dropped between its writes. This breaks the promise that an admitted effect has an authoritative settlement.
Example: A request updates two roots. The first root is written, then the caller cancels while the second root waits on its writer. The invocation returns cancellation, but the second root is updated later.
Recommended fix: Retain and join the queued write's settlement independently of the cancelling request. Once the first write begins, ensure the in-process invocation returns or records its authoritative result; do not let cancellation discard the only receiver.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if let Err(error) = | ||
| NativeTask::spawn(async move { native_runtime::off_native_thread(run).await }) | ||
| { | ||
| tracing::error!(%error, "memory curator run was not started"); | ||
| } | ||
| Ok(started) |
There was a problem hiding this comment.
🔴 Unstarted curator run reports success
If NativeTask::spawn refuses the curator, execute_retained_memory_curator still returns started. The admitted effect is dropped without running or settling, leaving the reported run unfinished.
Learn more
The function has already admitted an automation effect and obtained its started outcome before attempting to spawn the task. A native runtime can refuse task admission, returning Err from NativeTask::spawn (spawn). This branch logs the error but returns the started outcome. The moved future is dropped on refusal, so no run or retained settlement executes.
Example: The native runtime refuses the curator task after effect admission. The dashboard receives a started run ID but never receives its terminal run record.
Recommended fix: On spawn refusal, settle the admitted effect as an unavailable/problem terminal or return an appropriate failure with a durable effect outcome. Only return started after the work is owned by a running task.
Was this helpful? React with 👍 or 👎 to provide feedback.
| native_runtime::NativeTask::spawn(tracing::Instrument::instrument( | ||
| async move { executor.invoke_once(request).await }, | ||
| tracing::trace_span!("daemon.invocation.invoke_once"), | ||
| )); | ||
| settle_in_process_invocation( | ||
| &request_cancellations, | ||
| &request_id, | ||
| invocation, | ||
| remaining, | ||
| cancellation, | ||
| admitted_cancellation, | ||
| policy, | ||
| DAEMON_TOOL_RESPONSE_GRACE, | ||
| ) | ||
| .await | ||
| }) | ||
| )) | ||
| .map_err(unavailable)?; |
There was a problem hiding this comment.
🔴 Multi-root Git blocks native dispatch
For multi-root Git requests, the native invoke_once task calls explicit_git_state. Its synchronous git status waits for a subprocess on a native worker, delaying unrelated dispatches.
Learn more
The invocation executor now polls invoke_once on a native worker. Multi-root Git generation handling calls explicit_git_state from multi-root query handling; that helper calls Command::output() synchronously, waiting for the Git subprocess to finish. While it waits, the native worker cannot poll other requests or timers. This is a concrete synchronous path left in the moved invocation graph.
Example: A multi-root Git request invokes git status on a slow repository. Its worker remains occupied until Git exits; other native work queued to that worker waits too.
Recommended fix: Run explicit_git_state through native_runtime::spawn_blocking, preserving its response and deadline handling, before completing the multi-root Git generation.
Was this helpful? React with 👍 or 👎 to provide feedback.
Part of #3736
This PR moves the last two general-purpose Tokio spawn sites onto the native runtime and deletes the
block_in_placewriter wait (issue item 5).It stacks every open T4 branch. It merges #4057, #4058, #4059, #4060, #4061, #4062, #4063, #4064, #4067, #4068, #4070 and #4083. Until those land, the diff against master includes their commits. This PR's own commits are
0b49c5cbb5andbe67296772. The stack is needed because the in-process executor can leave Tokio only once every operation it dispatches is native. #4062's in-process dashboard test also needs this executor change, which is why #4062 is a draft.What changed
daemon/invocation_executor.rs). The invocation and its settlement are nowNativeTasks.settle_in_process_invocationtakes aNativeTask<DaemonInvocationResponse>. Its ReadOnly abort and AuthoritativeEffect grace wait are unchanged, and thedaemon.invocation_executor.in_processTokio site is gone.tracedecay-mcpserver/settlement.rs). The TokioJoinSetbecomes aNativeTaskSetkeyed by a dispatch id, and themcp.dispatch.settlementTokio site is gone. Lane A wrote this change and could not land it until the executor moved.tracedecay-mcp's production Tokio features droprt.recv_writer_replyno longer hands a Tokio worker's run queue away withblock_in_place. The native-thread writer-wait counter and its log stay.tracedecay-rusqlite-runtimedrops Tokio'srt-multi-threadfeature. The test that pinned the Tokio hand-off is deleted.daemon/http_application.rs). It was the one Tokio-worker writer wait left in production. The loopback HTTP listener stays on Tokio, because/remote-deletionsreachesproject_server_lifecycle.rs. Provisioning now runs withNativeTask::spawn_off_native_thread.Measured
A scratch logging build (not in this PR) recorded each writer wait on a native or Tokio worker and each counted native reader wait. Writer waits by suite:
//crates/tracedecay:daemon_suite:mcp_suite:session_suite:unit_testThe 59
unit_testTokio-worker waits all come from test bodies that call storage directly on#[tokio::test]runtimes (store_runtime_testsfailover and republish). On master6f105f6b57, the Tokio-worker writer waits were 220 indaemon_suiteand 190 inmcp_suite.Native reader waits are non-zero: 131, 2683, 4 and 1332. #4083 lists them by caller.
Verified at
8eae8775e0//crates/tracedecay:daemon_suitepassed in 93.1s,:mcp_suitein 118.4s,:session_suitein 57.6s and:unit_testin 149.6s.unit_testhad earlier failedadvisory_cycle_language_journey_test, intermittently. The cause was the debug long-poll guard failing refactor(mcp): run background tasks, replay and shutdown natively #4064's deferred advisory mount after 263 ms of CPU in one poll. refactor(mcp): run background tasks, replay and shutdown natively #4064 now carries the fix,e61a8209b6. With it,unit_testpassed in two consecutive runs with no long-poll panic in the log.daemon_suiterun under the logging build failedstore_shutdown_checkpoint_test::capacity_retirement_completes_while_a_maintenance_tick_runs. That test belongs to the known store-shutdown flake family. The clean rerun passed.//crates/tracedecay-rusqlite-runtime:unit_test,//crates/tracedecay-mcp:unit_testand//crates/tracedecay-daemon-service:unit_testpass.bazel build --config=clippypasses over//crates/tracedecay-rusqlite-runtime/...,//crates/tracedecay-mcp/...,//crates/tracedecay-daemon-service/...and//crates/tracedecay:all.//:rustfmt_check,scripts/check-ambient-tokio.pyandscripts/bazel/gen_builds.py --checkpass.Left on Tokio after this stack
project_server_lifecycle.rs, and the sites that route through it: project open (daemon.project_open.task), retirement, the loopback HTTP listener, the lifecycle hops, and the shutdown runner.main.rs's owned runtime that serves them.