Signal tool stability with waitForToolsUntil() and ToolChangeEvent by khushalsagar · Pull Request #373 · webmachinelearning/webmcp

khushalsagar · 2026-11-03T19:32:23Z

Implement the resolution from #328 (minutes) and #328 (comment):

Add the IDL for waitForToolsUntil and toolsReady, and state tracking for the tools-ready state based on pending promises.
Wait for tools-ready (up to an implementation-defined timeout) in perform an observation.
Dispatch a toolchange event (ToolChangeEvent with ready) when tools become not ready and suppress dispatch of this event until tools become ready again.

Wiring the wait for tools-ready to getTools() and changing dispatch of toolchange events is deferred for a follow up PR.

Note: There's still an open question for the Document lifecycle until which tools are presumed to be not ready.

Part of #328.

Preview | Diff

khushalsagar · 2026-11-03T19:32:25Z

A tangential thing worth taking up in a separate PR: the toolChange notification from register/unregister isn't coalesced well right now. It's triggered for every register/unregister call. Seems like it would be better to do it in a separate task (or microtask?) since in a task the state will be updated by multiple calls and we should dispatch a single event for all those mutations.

markafoltz

Overall structure looks good, it this PR has several features (toolchange event, modelContext.toolsReady, blocking getTools()) not yet landed; are you thinking of adjusting this once the implementations land?

markafoltz · 2026-11-09T22:49:59Z

-        [=webmcp task source=] given |targetDocument|'s [=relevant global object=] to [=fire an
-        event=] named {{ModelContext/toolchange}} at |targetDocument|'s [=Document/associated
-        ModelContext|associated <code>ModelContext</code>=].
+     1. If |exposed origins| is "<code>*</code>", or [=tool is exposed to an origin=] given |tool


Breaking into multiple steps would improve readability, perhaps:

If |exposed origins| is not "*" and [=tool is exposed to an origin=] ... returns false, then abort these steps.

[=queue a global task=]...

not in the PR now.

markafoltz · 2026-11-09T23:01:50Z

+1. If the result of running |areAllReady| is true, then return.
+
+1. Wait until either the result of running |areAllReady| is true, or an [=implementation-defined=]
+   timeout has elapsed since this step began.


Should the timeout clear pending tool ready promises?

Also, should tools-ready be updated to true for any documents in the tree? It depends on the answer to the previous question, I suppose.

the tools-ready state depends on the state of promises, that's independent of which callers are blocked on that state. basically each time any call-site tries to read tools, it starts a timer that either waits for tools-ready to toggle or for the timeout to trigger.

markafoltz · 2026-11-09T23:04:25Z

+
+   1. If |activeDocument| is null or is not [=Document/fully active=], then return true.
+
+      Note: If |traversable| is destroyed while waiting, its [=navigable/active document=] is


Every caller of wait for tools to be ready will need to check the document active state; could these steps return a value like "ready" or "abort" so that the caller doesn't have to check again?

markafoltz · 2026-11-09T23:07:38Z

 Each {{Document}} object has an <dfn for=Document>associated {{ModelContext}}</dfn>, which is a
 {{ModelContext}} object.

+Each {{Document}} object has a <dfn for=Document>tools-ready</dfn> [=boolean=], initially true.


Nit: Move below the "Upon creation" paragraph to keep the ModelContext related bits together

markafoltz · 2026-11-09T23:10:34Z

+       1. [=set/Remove=] |promise| from |context|'s [=model context/pending tool readiness
+          promises=].
+
+       1. If |document| is [=Document/fully active=], then [=update the tools-ready state=] given


Can a |promise| be reacted to while the document is not active?
Maybe if the promise was created in a different document or if it is in bfcache?
(See comment below)

Maybe if the promise was created in a different document

Yes, this is an example of that case:

let resolve; const p = new Promise(r => { resolve = r; }); iframe.contentDocument.modelContext.waitForToolsUntil(p); iframe.remove(); resolve();

markafoltz · 2026-11-09T23:49:00Z

+  The following delays tool discovery until asynchronous tool registration completes:
+
+  ```js
+  const toolsReady = (async () => {


This shadows modelContext.toolsReady...

const toolsRegistered = (async () => {

markafoltz · 2026-11-09T23:50:28Z

+1. Let |toolsReady| be |tool owner|'s [=Document/tools-ready=].
+
 1. Run the following steps [=in parallel=]:

-  1. [=Notify documents of a tool change=] given |tool owner| and |exposed origins|.
+  1. If |toolsReady| is true, then [=notify documents of a tool change=] given |tool owner|,
+     |exposed origins|, and |toolsReady|.


Nit: here the |toolsReady| check sits inside the in-parallel steps, while unregister a tool returns early before going in parallel. Could we use one style? The early return reads more clearly to me.

Nit: these in-parallel substeps are indented 2 spaces (that was already the case); Bikeshed style is 3, lined up with the parent step's text. Since you're editing these lines, maybe fix it here.

Fixed the existing indentation. I want to take the toolChange dispatch in a separate PR.

markafoltz · 2026-11-09T23:50:29Z

+1. [=set/Append=] |promise| to |context|'s [=model context/pending tool readiness promises=].
+
+1. [=Update the tools-ready state=] given |document|.


Web IDL wraps every Promise<any> argument in a new promise, so even waitForToolsUntil(Promise.resolve()) or waitForToolsUntil(42) synchronously sets tools-ready to false and fires toolchange{ready:false} at every document. A microtask later it fires toolchange{ready:true}.

Chromium returns early when the promise has already settled, so it fires nothing. The waitForToolsUntil(42) case in the WPT would behave differently from the spec once ToolChangeEvent ships.

Suggest coalescing: have update the tools-ready state queue a task that compares the current state with the last state we notified about, and notify only when they differ. This fits with your PR comment about coalescing the register/unregister notifications.

Updated the spec to match the WPTs and ignore already settled promises, nice catch!

Deferred the ToolChangeEvent dispatch changes to a follow-up PR, will take care of toolchange event batching there. Also, my comment was about an existing issue with no batching for toolchange in the register/unregister case tracked in #374.

markafoltz · 2026-11-09T23:50:30Z

+1. [=promise/React=] to |promise|:
+
+   - If |promise| was fulfilled or rejected, then:
+
+       1. [=set/Remove=] |promise| from |context|'s [=model context/pending tool readiness
+          promises=].


What happens if |promise| comes from another realm that is then destroyed? For example, a same-origin iframe passes one of its own promises to parent.document.modelContext.waitForToolsUntil() and the iframe is removed. HTML doesn't run promise jobs for documents that aren't fully active, so neither the Web IDL wrapper nor this reaction ever settles. tools-ready stays false for good, and every observation hits the timeout.

Chromium handles this. PendingWaitForTools watches the context the promise was created in and counts the promise as settled when that context is destroyed; see the tests wait-for-tools-until-detach-crash.https.html and detached-frame-waitForToolsUntil.https.html. Could we specify that, or at least add an Issue()?

Nit: Web IDL's react uses separate fulfilled and rejected branches. Both branches could call a shared "settle" algorithm, which the destroyed-realm case could reuse.

Discussed on crrev/c/8778494, we should ignore the case of the promise coming from another realm which queues it in a task that doesn't run when the realm is destroyed.

Fixed the nit on [=promise/React=] to use separate fulfilled and rejected branches running |settleSteps|.

markafoltz · 2026-11-09T23:50:30Z

+       1. If |document| is [=Document/fully active=], then [=update the tools-ready state=] given
+          |document|.


Suggested change

1. If |document| is [=Document/fully active=], then [=update the tools-ready state=] given

|document|.

1. [=Update the tools-ready state=] given |document|.

If |document| isn't fully active when the promise settles (e.g. it's in bfcache), we remove the promise but don't recalculate tools-ready. After restore, the set is empty but tools-ready is still false. Update the tools-ready state already skips notification for documents that aren't fully active, so I think this check can just go.

…omments - Defer ToolChangeEvent/toolchange readiness dispatch and the getTools() wait to a follow-up PR. - wait for tools to be ready: capture the top-level active document up front, drop the null check, and return "ready" or "navigated". - perform an observation: return early on "navigated". - waitForToolsUntil(): ignore already-settled promises, split React into fulfilled/rejected branches, and always update tools-ready on settle. - Move tools-ready below the ModelContext creation paragraph, rename the example variable, and fix in-parallel indentation in registerTool().

The navigable can also be destroyed while waiting, so "navigated" was too narrow. Update the note in perform an observation to cover both cases.

khushalsagar force-pushed the wait-for-tools-until branch 2 times, most recently from 94b382b to 9f62142 Compare November 3, 2026 20:26

khushalsagar requested review from domfarolino and markafoltz November 3, 2026 20:40

Signal tool stability with waitForToolsUntil() and ToolChangeEvent

59af1ff

khushalsagar force-pushed the wait-for-tools-until branch from 9f62142 to 59af1ff Compare November 3, 2026 20:41

kurotanshi mentioned this pull request Nov 9, 2026

本週技術趨勢週報（2026-W46） kurotanshi/grok-weekly#9

Open

markafoltz requested changes Nov 9, 2026

View reviewed changes

markafoltz mentioned this pull request Nov 10, 2026

Clarify the document associated with ModelContext #379

Closed

khushalsagar mentioned this pull request Nov 10, 2026

Batch toolChange notifications for tool registry changes #374

Open

kurotanshi mentioned this pull request Nov 11, 2026

本日技術趨勢日報（2026-11-11） kurotanshi/grok-weekly#11

Open

khushalsagar added 2 commits November 11, 2026 02:10

Return "abort" from wait for tools to be ready

c12e126

The navigable can also be destroyed while waiting, so "navigated" was too narrow. Update the note in perform an observation to cover both cases.

kurotanshi mentioned this pull request Nov 12, 2026

本日技術趨勢日報（2026-11-12） kurotanshi/grok-weekly#12

Open


		1. If \|activeDocument\| is null or is not [=Document/fully active=], then return true.

		Note: If \|traversable\| is destroyed while waiting, its [=navigable/active document=] is

		1. [=set/Append=] \|promise\| to \|context\|'s [=model context/pending tool readiness promises=].

		1. [=Update the tools-ready state=] given \|document\|.

		1. If \|document\| is [=Document/fully active=], then [=update the tools-ready state=] given
		\|document\|.

Conversation

khushalsagar commented Nov 3, 2026 • edited by pr-preview Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

khushalsagar commented Nov 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markafoltz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

khushalsagar commented Nov 3, 2026 •

edited by pr-preview Bot

Loading

khushalsagar commented Nov 3, 2026 •

edited

Loading