From 0923d2cefad236939439a32e7d65ccffd98423da Mon Sep 17 00:00:00 2001 From: ldm0 Date: Mon, 5 Oct 2026 05:03:34 +0800 Subject: [PATCH] refactor(webmcp): share registry notifications and invocation aborts --- .../src/context_bootstrap/web_mcp.rs | 1 + .../src/context_bootstrap/web_mcp/api.rs | 50 +++++------------ .../context_bootstrap/web_mcp/declarative.rs | 46 +++------------- .../context_bootstrap/web_mcp/execution.rs | 37 ++++++++++--- .../context_bootstrap/web_mcp/lifecycle.rs | 16 +++--- .../src/context_bootstrap/web_mcp/registry.rs | 53 +++++++++++++++++++ 6 files changed, 110 insertions(+), 93 deletions(-) create mode 100644 moli-renderer-v8/src/context_bootstrap/web_mcp/registry.rs diff --git a/moli-renderer-v8/src/context_bootstrap/web_mcp.rs b/moli-renderer-v8/src/context_bootstrap/web_mcp.rs index 491f654e0f..edd7b4515d 100644 --- a/moli-renderer-v8/src/context_bootstrap/web_mcp.rs +++ b/moli-renderer-v8/src/context_bootstrap/web_mcp.rs @@ -13,6 +13,7 @@ mod events; mod execution; mod lifecycle; mod navigation; +mod registry; mod state; mod tasks; diff --git a/moli-renderer-v8/src/context_bootstrap/web_mcp/api.rs b/moli-renderer-v8/src/context_bootstrap/web_mcp/api.rs index e459787a44..cb810be066 100644 --- a/moli-renderer-v8/src/context_bootstrap/web_mcp/api.rs +++ b/moli-renderer-v8/src/context_bootstrap/web_mcp/api.rs @@ -2,13 +2,12 @@ use super::bindings::{check_target, document_for_target, document_owner, dom_err use super::events::queue_tool_change; use super::state::{AbortRegistration, CallbackTool, RegisteredTool, ToolExecutor, ToolMetadata}; use super::tasks::{queue_task, remove_abort_registration, signal_reason, task_context, task_data}; -use super::{conversion, devtools, execution}; +use super::{conversion, devtools, execution, registry}; use crate::document_runtime::DomHandle; use crate::native_bridge::JsContextHost; use crate::util::context_host_ptr_from_global_bridge; use crate::web_api_interfaces; use crate::window_webidl_callback::WindowWebIdlCallbackFunction; -use moli_page_types::{RendererWebMcpEvent, RendererWebMcpToolId}; use crate::{abort_signal_route::ResolvedAbortSignal, webidl}; @@ -168,28 +167,15 @@ fn register_tool<'s>( registration_resolver: Some(v8::Global::new(scope, resolver)), }), }; - let observed = devtools::observes_tree( - unsafe { &*host_ptr }, - unsafe { &*host_ptr }.native_bridge().web_mcp.documents[&document].frame_tree, - ); - let (snapshot, origin, origins) = { - let entry = unsafe { &mut *host_ptr } - .native_bridge_mut() - .web_mcp - .documents - .get_mut(&document) - .expect("active ModelContext"); - let snapshot = observed.then(|| devtools::protocol_tool(entry, &definition.name, &tool)); + let (origin, origins) = { + let host = unsafe { &mut *host_ptr }; + let origin = host.native_bridge().web_mcp.documents[&document] + .origin + .clone(); let origins = tool.exposed_to.clone(); - entry.tools.insert(definition.name, tool); - (snapshot, entry.origin.clone(), origins) + registry::insert_tool(host, document, definition.name, tool); + (origin, origins) }; - if let Some(snapshot) = snapshot { - devtools::emit( - unsafe { &*host_ptr }, - RendererWebMcpEvent::ToolsAdded(vec![snapshot]), - ); - } queue_tool_change(scope, host_ptr, document, &origin, &origins); let ack = v8::Function::builder(registration_ack_callback) .data(data.into()) @@ -244,9 +230,9 @@ fn unregister_callback<'s>( return; }; let name = name.to_rust_string_lossy(scope); - let removed = { - let store = &mut unsafe { &mut *host_ptr }.native_bridge_mut().web_mcp; - let Some(entry) = store.documents.get_mut(&document) else { + let origin = { + let store = &unsafe { &*host_ptr }.native_bridge().web_mcp; + let Some(entry) = store.documents.get(&document) else { return; }; if entry @@ -258,19 +244,9 @@ fn unregister_callback<'s>( { return; } - entry.tools.remove(&name).map(|tool| { - ( - tool, - entry.origin.clone(), - entry.frame_id.clone(), - entry.frame_tree, - ) - }) + entry.origin.clone() }; - if let Some((tool, origin, frame_id, tree)) = removed { - devtools::emit_in_tree(unsafe { &*host_ptr }, tree, || { - RendererWebMcpEvent::ToolsRemoved(vec![RendererWebMcpToolId { frame_id, name }]) - }); + if let Some(tool) = registry::take_tool(unsafe { &mut *host_ptr }, document, name) { let ToolExecutor::Callback(callback) = tool.executor else { unreachable!("registration identity checked before removal") }; diff --git a/moli-renderer-v8/src/context_bootstrap/web_mcp/declarative.rs b/moli-renderer-v8/src/context_bootstrap/web_mcp/declarative.rs index 3eb0865b6d..67a2e5fa9f 100644 --- a/moli-renderer-v8/src/context_bootstrap/web_mcp/declarative.rs +++ b/moli-renderer-v8/src/context_bootstrap/web_mcp/declarative.rs @@ -3,11 +3,10 @@ use super::bindings::{check_target, document_owner, model_context_for_document}; use super::state::{FormInvocationState, RegisteredTool, ToolExecutor, ToolMetadata}; use super::tasks::queue_task; -use super::{devtools, events, execution}; +use super::{events, execution, registry}; use crate::document_runtime::DomHandle; use crate::native_bridge::JsContextHost; use crate::util::context_host_ptr_from_global_bridge; -use moli_page_types::{RendererWebMcpEvent, RendererWebMcpToolId}; use crate::dom::native::{DomHost, DomMutationEffects, Node}; use std::collections::{HashMap, HashSet}; @@ -115,19 +114,10 @@ pub(crate) fn note_mutation( } } for (document, name, _) in invalid { - let (frame_id, tree, origin) = { - let store = &mut host.native_bridge_mut().web_mcp; - let entry = store.documents.get_mut(&document).expect("form document"); - entry.tools.remove(&name); - ( - entry.frame_id.clone(), - entry.frame_tree, - entry.origin.clone(), - ) - }; - devtools::emit_in_tree(host, tree, || { - RendererWebMcpEvent::ToolsRemoved(vec![RendererWebMcpToolId { frame_id, name }]) - }); + let _ = registry::take_tool(host, document, name); + let origin = host.native_bridge().web_mcp.documents[&document] + .origin + .clone(); events::queue_form_removal_change(scope, host, document, &origin); } for pending in host.native_bridge_mut().web_mcp.pending.values_mut() { @@ -541,19 +531,7 @@ fn synchronize_document( ); } let host = unsafe { &mut *host_ptr }; - let entry = host - .native_bridge_mut() - .web_mcp - .documents - .get_mut(&document) - .expect("form document"); - entry.tools.remove(&name); - let frame_id = entry.frame_id.clone(); - devtools::emit_in_tree( - host, - host.native_bridge().web_mcp.documents[&document].frame_tree, - || RendererWebMcpEvent::ToolsRemoved(vec![RendererWebMcpToolId { frame_id, name }]), - ); + let _ = registry::take_tool(host, document, name); changes += 1; } for definition in definitions { @@ -565,12 +543,6 @@ fn synchronize_document( continue; } let backend_node_id = host.renderer_backend_node_id_for_live_handle(definition.handle); - let observed = devtools::observes_tree( - host, - host.native_bridge().web_mcp.documents[&document].frame_tree, - ); - let store = &mut host.native_bridge_mut().web_mcp; - let entry = store.documents.get_mut(&document).expect("form document"); let tool = RegisteredTool { metadata: definition.metadata, stack_trace: None, @@ -581,11 +553,7 @@ fn synchronize_document( backend_node_id, }, }; - let snapshot = observed.then(|| devtools::protocol_tool(entry, &definition.name, &tool)); - entry.tools.insert(definition.name, tool); - if let Some(snapshot) = snapshot { - devtools::emit(host, RendererWebMcpEvent::ToolsAdded(vec![snapshot])); - } + registry::insert_tool(host, document, definition.name, tool); changes += 1; } if changes > 0 { diff --git a/moli-renderer-v8/src/context_bootstrap/web_mcp/execution.rs b/moli-renderer-v8/src/context_bootstrap/web_mcp/execution.rs index eede2e1ac9..dfc89900e3 100644 --- a/moli-renderer-v8/src/context_bootstrap/web_mcp/execution.rs +++ b/moli-renderer-v8/src/context_bootstrap/web_mcp/execution.rs @@ -515,16 +515,39 @@ pub(super) fn cancel_invocation( result: RendererWebMcpResult::Canceled, } }); - if let Some(resolver) = pending.resolver.take() { - let resolver = v8::Local::new(scope, &resolver); - let reason = crate::native_bridge::abort::abort_error_value(scope); - let _ = resolver.reject(scope, reason); - } - abort_target(scope, host_ptr, &pending); + abort_taken_invocation(scope, host_ptr, pending, InvocationAbortCause::Canceled); true } -pub(super) fn abort_target( +pub(super) enum InvocationAbortCause { + Canceled, + DocumentRetired, +} + +// Call after cleanup_invocation and any protocol notification. Keeping that +// boundary preserves cancel's cleanup -> CDP -> caller rejection ordering. +pub(super) fn abort_taken_invocation( + scope: &mut v8::PinScope<'_, '_>, + host_ptr: *mut JsContextHost, + mut pending: PendingInvocation, + cause: InvocationAbortCause, +) { + if let Some(resolver) = pending.resolver.take() { + let resolver = v8::Local::new(scope, &resolver); + let error = match cause { + InvocationAbortCause::Canceled => crate::native_bridge::abort::abort_error_value(scope), + InvocationAbortCause::DocumentRetired => dom_error( + scope, + "UnknownError", + "The tool document is no longer active.", + ), + }; + let _ = resolver.reject(scope, error); + } + abort_target(scope, host_ptr, &pending); +} + +fn abort_target( scope: &mut v8::PinScope<'_, '_>, host_ptr: *mut JsContextHost, pending: &PendingInvocation, diff --git a/moli-renderer-v8/src/context_bootstrap/web_mcp/lifecycle.rs b/moli-renderer-v8/src/context_bootstrap/web_mcp/lifecycle.rs index d8d601bd11..cc18f5425f 100644 --- a/moli-renderer-v8/src/context_bootstrap/web_mcp/lifecycle.rs +++ b/moli-renderer-v8/src/context_bootstrap/web_mcp/lifecycle.rs @@ -153,15 +153,11 @@ fn retirement_callback<'s>( } for mut pending in invocations { execution::cleanup_invocation(scope, host_ptr, &mut pending); - if let Some(resolver) = pending.resolver.take() { - let resolver = v8::Local::new(scope, &resolver); - let error = dom_error( - scope, - "UnknownError", - "The tool document is no longer active.", - ); - let _ = resolver.reject(scope, error); - } - execution::abort_target(scope, host_ptr, &pending); + execution::abort_taken_invocation( + scope, + host_ptr, + pending, + execution::InvocationAbortCause::DocumentRetired, + ); } } diff --git a/moli-renderer-v8/src/context_bootstrap/web_mcp/registry.rs b/moli-renderer-v8/src/context_bootstrap/web_mcp/registry.rs new file mode 100644 index 0000000000..ced3c80045 --- /dev/null +++ b/moli-renderer-v8/src/context_bootstrap/web_mcp/registry.rs @@ -0,0 +1,53 @@ +//! Registry mutations and CDP notifications; callers own author task scheduling. + +use super::devtools; +use super::state::RegisteredTool; +use crate::document_runtime::DomHandle; +use crate::native_bridge::JsContextHost; +use moli_page_types::{RendererWebMcpEvent, RendererWebMcpToolId}; + +pub(super) fn insert_tool( + host: &mut JsContextHost, + document: DomHandle, + name: String, + tool: RegisteredTool, +) { + let observed = devtools::observes_tree( + host, + host.native_bridge().web_mcp.documents[&document].frame_tree, + ); + let snapshot = { + let entry = host + .native_bridge_mut() + .web_mcp + .documents + .get_mut(&document) + .expect("active ModelContext"); + let snapshot = observed.then(|| devtools::protocol_tool(entry, &name, &tool)); + entry.tools.insert(name, tool); + snapshot + }; + if let Some(snapshot) = snapshot { + devtools::emit(host, RendererWebMcpEvent::ToolsAdded(vec![snapshot])); + } +} + +pub(super) fn take_tool( + host: &mut JsContextHost, + document: DomHandle, + name: String, +) -> Option { + let (tool, frame_id, tree) = { + let entry = host + .native_bridge_mut() + .web_mcp + .documents + .get_mut(&document)?; + let tool = entry.tools.remove(&name)?; + (tool, entry.frame_id.clone(), entry.frame_tree) + }; + devtools::emit_in_tree(host, tree, || { + RendererWebMcpEvent::ToolsRemoved(vec![RendererWebMcpToolId { frame_id, name }]) + }); + Some(tool) +}