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 77e1d52f8f..9a6292c029 100644 --- a/moli-renderer-v8/src/context_bootstrap/web_mcp/declarative.rs +++ b/moli-renderer-v8/src/context_bootstrap/web_mcp/declarative.rs @@ -10,6 +10,7 @@ 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}; pub(super) mod invocation; mod schema; @@ -31,58 +32,53 @@ pub(crate) fn note_mutation( .iter() .any(|root| dom.is_host_including_inclusive_ancestor(*root, handle)) }; - let targets = effects - .style() - .attribute_mutations() - .iter() - .filter(|attribute| { - attribute.namespace().is_none() && schema_attribute(attribute.local_name()) - }) - .map(|attribute| attribute.target()) - .chain( - effects - .style() - .child_list_mutations() - .iter() - .flat_map(|mutation| { - mutation - .added_nodes() - .iter() - .chain(mutation.removed_nodes()) - .copied() - }), - ) - .chain(effects.style().character_data_mutations().iter().copied()) - .map(|target| (target, true)) - .chain( - effects - .style() - .child_list_mutations() - .iter() - .map(|mutation| (mutation.target(), false)), - ); - for (target, discover_descendants) in targets { - if let Some(document) = dom.owner_document_handle(target) - && (host - .native_bridge() - .web_mcp - .documents - .get(&document) - .is_some_and(|entry| { - entry - .tools - .values() - .any(|tool| matches!(tool.executor, ToolExecutor::Form { .. })) - }) - || (dom.is_connected(target) - && ((discover_descendants && contains_tool_form(dom, target)) - || affects_tool_form(dom, target)))) - { - host.native_bridge_mut() - .web_mcp - .dirty_form_documents - .insert(document); + for attribute in effects.style().attribute_mutations() { + if attribute.namespace().is_some() || !schema_attribute(attribute.local_name()) { + continue; } + let target = attribute.target(); + let scope = if matches!(attribute.local_name(), "id" | "form" | "for") { + FormMutationScope::Associations + } else if dom.is_html_element_named(target, "fieldset") { + FormMutationScope::Subtree + } else { + FormMutationScope::Target + }; + mark_affected_forms(host, dom, target, scope); + } + for mutation in effects.style().child_list_mutations() { + let scope = if dom.is_html_element_named(mutation.target(), "fieldset") + && mutation + .added_nodes() + .iter() + .chain(mutation.removed_nodes()) + .any(|node| dom.is_html_element_named(*node, "legend")) + { + FormMutationScope::Subtree + } else { + FormMutationScope::Target + }; + mark_affected_forms(host, dom, mutation.target(), scope); + for node in mutation.added_nodes() { + mark_affected_forms(host, dom, *node, FormMutationScope::Subtree); + } + for node in mutation.removed_nodes() { + // Adoption has already changed the removed subtree's owner. Its + // former container still identifies the document whose external + // control associations must be refreshed. + if dom.owner_document_handle(*node) != dom.owner_document_handle(mutation.target()) { + mark_affected_forms( + host, + dom, + mutation.target(), + FormMutationScope::Associations, + ); + } + mark_affected_forms(host, dom, *node, FormMutationScope::RemovedSubtree); + } + } + for target in effects.style().character_data_mutations() { + mark_affected_forms(host, dom, *target, FormMutationScope::Target); } let invalid = host .native_bridge() @@ -105,10 +101,14 @@ pub(crate) fn note_mutation( .iter() .map(|(_, _, handle)| *handle) .collect::>(); - for (document, name, _) in invalid { + for (document, name, handle) in invalid { let (frame_id, tree, origin) = { let store = &mut host.native_bridge_mut().web_mcp; - store.dirty_form_documents.insert(document); + store + .dirty_forms + .entry(document) + .or_default() + .insert(handle); let entry = store.documents.get_mut(&document).expect("form document"); entry.tools.remove(&name); ( @@ -144,25 +144,13 @@ pub(crate) fn note_mutation( } } -// Native forms in connected shadow trees participate in the same registry. -// Use this traversal for initial discovery and every mutation synchronization. +// The native index includes closed shadow trees. Keep the existing traversal +// order for duplicate names without visiting every unrelated DOM node. fn form_handles(dom: &DomHost, root: DomHandle) -> Vec { - let mut forms = Vec::new(); - let mut stack = vec![root]; - while let Some(handle) = stack.pop() { - if dom.is_html_element_named(handle, "form") { - forms.push(handle); - } - stack.extend( - dom.child_handles(handle) - .collect::>() - .into_iter() - .rev(), - ); - if let Some(shadow) = dom.shadow_root_handle(handle) { - stack.push(shadow); - } - } + let mut forms = dom.html_elements_by_local_name_in_shadow_including_subtree(root, "form"); + forms.sort_unstable_by(|left, right| { + dom.compare_handles_in_shadow_including_tree_order(*left, *right) + }); forms } @@ -188,22 +176,98 @@ fn contains_tool_form(dom: &DomHost, root: DomHandle) -> bool { }) } -fn affects_tool_form(dom: &DomHost, target: DomHandle) -> bool { +enum FormMutationScope { + Target, + Subtree, + RemovedSubtree, + Associations, +} + +fn mark_affected_forms( + host: &mut JsContextHost, + dom: &DomHost, + target: DomHandle, + scope: FormMutationScope, +) { + if !dom.is_connected(target) && !matches!(scope, FormMutationScope::RemovedSubtree) { + return; + } let Some(document) = dom.owner_document_handle(target) else { - return false; + return; }; - let forms = dom.html_elements_by_local_name_in_shadow_including_subtree(document, "form"); - if forms.is_empty() { - return false; + // Discovery is independent of successful registration: name-conflicting + // candidates still need to retry when their controls change. + if !contains_tool_form(dom, document) { + return; + } + let mut affected = HashSet::new(); + let mut all = matches!(scope, FormMutationScope::Associations); + let mut mark_owner = |handle| { + if dom.is_html_element_named(handle, "form") && form_registration_is_valid(dom, handle) { + affected.insert(handle); + } + if let Some(owner) = dom.form_control_owner(handle) + && form_registration_is_valid(dom, owner) + { + affected.insert(owner); + } + }; + let mut ancestor = Some(target); + while let Some(handle) = ancestor { + mark_owner(handle); + all |= dom.is_html_element_named(handle, "label"); + ancestor = dom.parent_node(handle); + } + if matches!( + scope, + FormMutationScope::Subtree | FormMutationScope::RemovedSubtree + ) { + let mut stack = vec![target]; + while let Some(handle) = stack.pop() { + mark_owner(handle); + if let Some(element) = dom.node(handle).and_then(Node::as_element) { + // ID/label association changes can affect both the old and new + // owners. A removed external control has already lost its old + // owner, so conservatively update candidates in that document. + all |= element.has_attribute("id") + || element.is_html_label() + || (matches!(scope, FormMutationScope::RemovedSubtree) + && element.namespace() == "http://www.w3.org/1999/xhtml" + && matches!( + element.local_name(), + "button" + | "fieldset" + | "input" + | "object" + | "output" + | "select" + | "textarea" + )); + } + if all { + break; + } + stack.extend(dom.child_handles(handle)); + if let Some(shadow) = dom.shadow_root_handle(handle) { + stack.push(shadow); + } + } + } + if all { + affected.extend( + dom.html_elements_by_local_name_in_shadow_including_subtree(document, "form") + .into_iter() + .filter(|form| form_registration_is_valid(dom, *form)), + ); + } + if !affected.is_empty() { + host.native_bridge_mut() + .web_mcp + .dirty_forms + .entry(document) + .or_default() + .extend(affected); } - let owner = dom.form_control_owner(target); - // A name collision can keep a valid form out of the tool registry. Its - // descendants and associated controls still need to update its definition. - // Inspect indexed forms, never the growing mutation container's subtree. - forms.into_iter().any(|form| { - form_registration_is_valid(dom, form) - && (form == target || dom.is_ancestor(form, target) || owner == Some(form)) - }) } fn schema_attribute(name: &str) -> bool { @@ -238,18 +302,29 @@ pub(crate) fn prepare_registration_task(host: &mut JsContextHost) -> bool { let document = host.document_handle(); let owner = document_owner(host, document); let needs_initial_scan = host.native_bridge().web_mcp.initialized_form_owner != owner; - let has_initial_forms = needs_initial_scan && contains_tool_form(host.dom_host(), document); + let initial_forms = if needs_initial_scan { + form_handles(host.dom_host(), document) + .into_iter() + .filter(|form| form_registration_is_valid(host.dom_host(), *form)) + .collect::>() + } else { + HashSet::new() + }; let store = &mut host.native_bridge_mut().web_mcp; if needs_initial_scan { // The scheduler discards tasks owned by the previous document. Its // queued marker must not prevent the replacement document's scan. store.form_registration_task_queued = false; store.initialized_form_owner = owner; - if has_initial_forms { - store.dirty_form_documents.insert(document); + if !initial_forms.is_empty() { + store + .dirty_forms + .entry(document) + .or_default() + .extend(initial_forms); } } - !store.form_registration_task_queued && !store.dirty_form_documents.is_empty() + !store.form_registration_task_queued && !store.dirty_forms.is_empty() } pub(crate) fn queue_registration_task( @@ -279,11 +354,11 @@ fn registration_callback<'s>( let mut documents = { let store = &mut unsafe { &mut *host_ptr }.native_bridge_mut().web_mcp; store.form_registration_task_queued = false; - std::mem::take(&mut store.dirty_form_documents) + std::mem::take(&mut store.dirty_forms) .into_iter() .collect::>() }; - documents.sort_unstable_by_key(|document| document.index()); + documents.sort_unstable_by_key(|(document, _)| document.index()); // An observed invalidation cannot be undone by the final DOM snapshot. let invalidated = unsafe { &*host_ptr } .native_bridge() @@ -306,8 +381,8 @@ fn registration_callback<'s>( "Tool definition changed during execution", ); } - for document in documents { - synchronize_document(scope, host_ptr, document); + for (document, forms) in documents { + synchronize_document(scope, host_ptr, document, forms); } } @@ -322,6 +397,7 @@ fn form_tools( scope: &mut v8::PinScope<'_, '_>, host: &JsContextHost, document: DomHandle, + forms: impl IntoIterator, ) -> Vec { if document_owner(host, document).is_none() || !host.document_scripting_enabled(document) @@ -340,7 +416,12 @@ fn form_tools( } let dom = host.dom_host(); let mut result = Vec::new(); - for handle in form_handles(dom, document) { + for handle in forms { + if !form_registration_is_valid(dom, handle) + || dom.owner_document_handle(handle) != Some(document) + { + continue; + } let element = dom.node(handle).and_then(Node::as_element).expect("form"); let (Some(name), Some(description)) = ( element.attribute("toolname"), @@ -372,8 +453,19 @@ fn synchronize_document( scope: &mut v8::PinScope<'_, '_>, host_ptr: *mut JsContextHost, document: DomHandle, + affected_forms: HashSet, ) { - let definitions = form_tools(scope, unsafe { &*host_ptr }, document); + let mut forms = affected_forms.iter().copied().collect::>(); + forms.sort_unstable_by(|left, right| { + unsafe { &*host_ptr } + .dom_host() + .compare_handles_in_shadow_including_tree_order(*left, *right) + }); + let definitions = form_tools(scope, unsafe { &*host_ptr }, document, forms); + let definitions_by_handle = definitions + .iter() + .map(|definition| (definition.handle, definition)) + .collect::>(); if definitions.is_empty() && !unsafe { &*host_ptr } .native_bridge() @@ -420,12 +512,16 @@ fn synchronize_document( else { return None; }; - let same = definitions.iter().any(|definition| { - definition.handle == handle - && &definition.name == name - && definition.metadata == tool.metadata - && definition.autosubmit == autosubmit - }); + if !affected_forms.contains(&handle) { + return None; + } + let same = definitions_by_handle + .get(&handle) + .is_some_and(|definition| { + &definition.name == name + && definition.metadata == tool.metadata + && definition.autosubmit == autosubmit + }); (!same).then(|| name.clone()) }) .collect::>() @@ -477,6 +573,12 @@ fn synchronize_document( } for definition in definitions { let host = unsafe { &mut *host_ptr }; + if host.native_bridge().web_mcp.documents[&document] + .tools + .contains_key(&definition.name) + { + continue; + } let backend_node_id = host.renderer_backend_node_id_for_live_handle(definition.handle); let observed = devtools::observes_tree( host, @@ -484,9 +586,6 @@ fn synchronize_document( ); let store = &mut host.native_bridge_mut().web_mcp; let entry = store.documents.get_mut(&document).expect("form document"); - if entry.tools.contains_key(&definition.name) { - continue; - } let tool = RegisteredTool { metadata: definition.metadata, stack_trace: None, diff --git a/moli-renderer-v8/src/context_bootstrap/web_mcp/state.rs b/moli-renderer-v8/src/context_bootstrap/web_mcp/state.rs index cccfa8798f..981a3013ca 100644 --- a/moli-renderer-v8/src/context_bootstrap/web_mcp/state.rs +++ b/moli-renderer-v8/src/context_bootstrap/web_mcp/state.rs @@ -25,7 +25,7 @@ pub(crate) struct ModelContextStore { pub(super) retired_aborts: Vec, pub(super) retired_invocations: Vec, pub(super) retirement_task_queued: bool, - pub(super) dirty_form_documents: HashSet, + pub(super) dirty_forms: HashMap>, pub(super) form_registration_task_queued: bool, pub(super) initialized_form_owner: Option, pub(super) child_navigations: HashMap< diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/web_mcp/discovery.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/web_mcp/discovery.rs index ff04d3bb17..510cff3f63 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/web_mcp/discovery.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/web_mcp/discovery.rs @@ -1,5 +1,172 @@ use super::*; +#[test] +fn web_mcp_adopted_external_controls_update_both_document_schemas() { + let mut vm = new_storage_page_task_executor_test_vm("https://tools.test/"); + vm.eval(r#" + document.body.innerHTML='
'; + globalThis.input=document.querySelector('input'); + globalThis.childDocument=document.querySelector('iframe').contentDocument; + childDocument.body.innerHTML='
'; + globalThis.probe='pending'; + globalThis.capture=()=>document.modelContext.getTools().then(tools=>{ + probe=tools.map(tool=>tool.name+':'+Object.keys(tool.inputSchema.properties).sort().join(',')).sort().join('|'); + }); + "#).unwrap(); + vm.eval_after_selected_page_tasks("undefined").unwrap(); + vm.eval("capture()").unwrap(); + assert_eq!( + vm.eval_after_selected_page_tasks("probe").unwrap(), + "inner:|outer:query" + ); + vm.eval("childDocument.body.appendChild(input)").unwrap(); + vm.eval_after_selected_page_tasks("undefined").unwrap(); + vm.eval("capture()").unwrap(); + assert_eq!( + vm.eval_after_selected_page_tasks("probe").unwrap(), + "inner:query|outer:" + ); + vm.eval("document.body.appendChild(input)").unwrap(); + vm.eval_after_selected_page_tasks("undefined").unwrap(); + vm.eval("capture()").unwrap(); + assert_eq!( + vm.eval_after_selected_page_tasks("probe").unwrap(), + "inner:|outer:query" + ); +} + +#[test] +fn web_mcp_unrelated_dom_mutations_do_not_rebuild_form_schemas() { + use crate::native_bridge::element::take_form_lookup_work_for_test; + + let mut vm = new_storage_page_task_executor_test_vm("https://tools.test/"); + vm.eval( + r#" + for (let i=0;i<32;i++) { + const form=document.createElement('form'); + form.setAttribute('toolname','form_'+i); + form.setAttribute('tooldescription','Form '+i); + form.innerHTML=''; + document.body.appendChild(form); + } + globalThis.input=document.querySelector('input'); + globalThis.root=document.body.appendChild(document.createElement('section')); + globalThis.probe='pending'; + "#, + ) + .unwrap(); + vm.eval_after_selected_page_tasks("undefined").unwrap(); + take_form_lookup_work_for_test(); + vm.eval( + r#" + for (let i=0;i<8000;i++) root.appendChild(document.createElement('div')); + root.setAttribute('value','unrelated'); + root.firstChild.textContent='unrelated'; + root.lastChild.remove(); + "#, + ) + .unwrap(); + vm.eval_after_selected_page_tasks("undefined").unwrap(); + assert_eq!(take_form_lookup_work_for_test().traversals, 0); + + vm.eval("input.name='changed'").unwrap(); + vm.eval_after_selected_page_tasks("undefined").unwrap(); + let work = take_form_lookup_work_for_test(); + assert_eq!(work.traversals, 1, "one affected form among 32: {work:?}"); + vm.eval("document.modelContext.getTools().then(tools=>probe=tools.length+'|'+Object.keys(tools.find(tool=>tool.name==='form_0').inputSchema.properties).join(','))").unwrap(); + assert_eq!( + vm.eval_after_selected_page_tasks("probe").unwrap(), + "32|changed" + ); +} + +#[test] +fn web_mcp_incremental_schemas_track_external_controls_labels_and_disabled_fieldsets() { + for mode in ["light", "open", "closed"] { + let mut vm = new_storage_page_task_executor_test_vm("https://tools.test/"); + vm.eval(&format!(r#" + globalThis.root=document.body; + if ('{mode}' !== 'light') {{ + const host=document.body.appendChild(document.createElement('section')); + root=host.attachShadow({{mode:'{mode}'}}); + }} + root.innerHTML='
'; + globalThis.input=root.querySelector('input'); + globalThis.label=root.querySelector('label'); + globalThis.fieldset=root.querySelector('fieldset'); + globalThis.probe='pending'; + globalThis.capture=()=>document.modelContext.getTools().then(tools=>{{ + probe=tools.map(tool=>tool.name+':'+Object.keys(tool.inputSchema.properties).sort().join(',')).sort().join('|'); + }}); + "#)).unwrap(); + vm.eval_after_selected_page_tasks("undefined").unwrap(); + vm.eval("capture()").unwrap(); + assert_eq!( + vm.eval_after_selected_page_tasks("probe").unwrap(), + "one:outside,query|two:", + "{mode}" + ); + vm.eval("label.firstChild.data='New label'").unwrap(); + vm.eval_after_selected_page_tasks("undefined").unwrap(); + vm.eval("document.modelContext.getTools().then(tools=>probe=tools.find(tool=>tool.name==='one').inputSchema.properties.query.description)").unwrap(); + assert_eq!( + vm.eval_after_selected_page_tasks("probe").unwrap(), + "New label", + "{mode}" + ); + vm.eval("input.setAttribute('form','two'); fieldset.disabled=true") + .unwrap(); + vm.eval_after_selected_page_tasks("undefined").unwrap(); + vm.eval("capture()").unwrap(); + assert_eq!( + vm.eval_after_selected_page_tasks("probe").unwrap(), + "one:|two:query", + "{mode}" + ); + vm.eval("globalThis.wrapper=document.createElement('div'); wrapper.innerHTML=''; root.appendChild(wrapper)").unwrap(); + vm.eval_after_selected_page_tasks("undefined").unwrap(); + vm.eval("capture()").unwrap(); + assert_eq!( + vm.eval_after_selected_page_tasks("probe").unwrap(), + "one:bulk|two:query", + "{mode}" + ); + vm.eval("wrapper.remove(); fieldset.disabled=false") + .unwrap(); + vm.eval_after_selected_page_tasks("undefined").unwrap(); + vm.eval("capture()").unwrap(); + assert_eq!( + vm.eval_after_selected_page_tasks("probe").unwrap(), + "one:outside|two:query", + "{mode}" + ); + } +} + +#[test] +fn web_mcp_indexed_discovery_preserves_shadow_including_duplicate_name_order() { + for mode in ["open", "closed"] { + let mut vm = new_storage_page_task_executor_test_vm("https://tools.test/"); + vm.eval(&format!(r#" + const host=document.body.appendChild(document.createElement('section')); + const light=document.createElement('form'); + light.setAttribute('toolname','duplicate'); light.setAttribute('tooldescription','Light'); + host.appendChild(light); + const shadow=host.attachShadow({{mode:'{mode}'}}); + shadow.innerHTML='
'; + globalThis.probe='pending'; + "#)).unwrap(); + vm.eval_after_selected_page_tasks("undefined").unwrap(); + vm.eval("document.modelContext.getTools().then(([tool])=>probe=tool.description)") + .unwrap(); + assert_eq!( + vm.eval_after_selected_page_tasks("probe").unwrap(), + "Shadow", + "{mode}" + ); + } +} + #[test] fn web_mcp_aborted_callback_registration_cannot_remove_its_form_replacement() { let mut vm = new_storage_page_task_executor_test_vm("https://tools.test/"); @@ -25,24 +192,30 @@ fn web_mcp_aborted_callback_registration_cannot_remove_its_form_replacement() { #[test] fn web_mcp_declarative_candidates_retry_registration_when_their_controls_change() { for mode in ["light", "open", "closed"] { - for (markup, mutation) in [ + for (markup, external_control, mutation) in [ ( "", + false, "const input=document.createElement('input'); input.name='query'; form.appendChild(input)", ), ( "
", + false, "const input=document.createElement('input'); input.name='query'; form.firstChild.appendChild(input)", ), ( "
", + false, "form.elements.discard.remove()", ), - ("", "form.elements.old.name='query'"), + ("", false, "form.elements.old.name='query'"), ( "", + false, "const input=document.createElement('input'); input.name='query'; input.setAttribute('form',form.id); root.appendChild(input)", ), + ("", true, "external.remove()"), + ("", true, "external.removeAttribute('form')"), ] { let mut vm = new_storage_page_task_executor_test_vm("https://tools.test/"); vm.eval( @@ -73,12 +246,23 @@ fn web_mcp_declarative_candidates_retry_registration_when_their_controls_change( form.setAttribute('toolautosubmit',''); form.innerHTML='{markup}'; form.addEventListener('submit',event=>{{ - event.preventDefault(); event.respondWith(form.elements.query.value); + event.preventDefault(); + const query=form.elements.namedItem('query'); + event.respondWith(query ? query.value : 'empty'); }}); root.appendChild(form); + if ({external_control}) {{ + globalThis.external=document.createElement('input'); + external.name='query'; + external.setAttribute('form',form.id); + root.appendChild(external); + }} "# )) .unwrap(); + if external_control { + assert_eq!(vm.eval("external.form===form").unwrap(), "true", "{mode}"); + } // Complete the conflicting registration attempt before freeing the // name, so a coalesced initial scan cannot hide a missing retry. vm.eval_after_selected_page_tasks("undefined").unwrap(); @@ -102,6 +286,13 @@ fn web_mcp_declarative_candidates_retry_registration_when_their_controls_change( assert_eq!(vm.eval_after_selected_page_tasks("probe").unwrap(), "[]"); vm.eval(mutation).unwrap(); + if external_control { + assert_eq!( + vm.eval("external.form===null").unwrap(), + "true", + "{mode}: {mutation}" + ); + } vm.eval_after_selected_page_tasks("undefined").unwrap(); vm.eval(r#" mc.getTools().then(tools=>{ @@ -111,14 +302,23 @@ fn web_mcp_declarative_candidates_retry_registration_when_their_controls_change( "#).unwrap(); assert_eq!( vm.eval_after_selected_page_tasks("probe").unwrap(), - "Declarative|query", + if external_control { + "Declarative|" + } else { + "Declarative|query" + }, "{mode}: {mutation}" ); - vm.eval("mc.getTools().then(([tool])=>mc.executeTool(tool,{query:'filled'})).then(value=>probe=value,error=>probe=error.name)") + let input = if external_control { + "{}" + } else { + "{query:'filled'}" + }; + vm.eval(&format!("mc.getTools().then(([tool])=>mc.executeTool(tool,{input})).then(value=>probe=value,error=>probe=error.name)")) .unwrap(); assert_eq!( vm.eval_after_selected_page_tasks("probe").unwrap(), - "filled", + if external_control { "empty" } else { "filled" }, "{mode}: {mutation}" ); }