Clarify that unregistration must not fail an in-flight execution #300

Activity

1. mysticalseeker24

   added a commit that references this issue on Sep 8, 2026

   fix: a tool that unregisters itself no longer reports failure after s…

   ...

   6dae291
2. mysticalseeker24 commented on Sep 8, 2026

   

   mysticalseeker24

   on Sep 8, 2026

   More actions

   Independent reproduction on an unrelated build, plus two refinements to the workaround.

   We hit this in Chrome 152 on a booking app whose live tool set is derived from page state, with the same error string:

   The operation failed for an unknown transient reason (e.g. out of memory).
   

   Three things that may be useful for the "state it explicitly" ask and for scoping the WPT case.

   1. The stakes can be worse than a promise disagreeing with the screen

   In our case the self-unregistering tool was confirm_booking — a consequential, consent-gated commit that leaves the live set precisely because it succeeded (the stage becomes booked, so confirming is no longer legal). The caller was rejected after the appointment had been committed. An agent that receives a rejection from a booking call will reasonably retry, and a retry is the one thing that must not happen here. release_slot behaved identically.

   So the failure mode is not only "the caller and the screen disagree". It is "the caller is told an irreversible action failed, and the obvious recovery is to repeat it."

   2. Deferring to the end of the promise chain is not enough

   Our first attempt deferred the re-sync into the .finally() of the executing tool's own promise. That still rejected — presumably because it is still inside the call the browser is awaiting. Only moving the re-sync to a fresh task resolved it, which matches your "deferred by one macrotask" note; but the distinction between end of the promise chain and next task seems worth spelling out, since the former is the more natural place for an implementer to put it.

   3. Deferring the whole re-sync is more than is needed

   The workaround as stated — leave executing tools registered during a re-sync and re-sync once they return — defers every transition. That delayed registration for tools that were never at risk: our calendar asked for availability before the tool that provides it had been registered, received unavailable, and cached that transient miss.

   Narrowing the condition to "defer only if this sync would unregister a tool that is currently executing" keeps every other transition as prompt as before:

   function sync(state) {
     const live = tools.filter((t) => t.available(state));
     const liveNames = new Set(live.map((t) => t.name));
     // Only a sync that cuts a *running* tool is dangerous.
     if ([...executing].some((name) => !liveNames.has(name))) {
       deferredState = state;   // flushed in a fresh task once nothing is executing
       return;
     }
     // …register / unregister as usual
   }

   On the WPT case

   Worth noting this shape is encouraged by the direction in #255 and #306. The more a page derives its live set from state — which is what progressive disclosure asks for — the more often a tool's success is exactly what makes that tool illegal. Self-unregistration stops being an edge case and becomes the normal outcome of any state-advancing tool, which seems like a good argument for covering it explicitly rather than leaving it implied by #248.

   Reproduction notes and the fix: https://github.com/mysticalseeker24/parity-webmcp
3. minjikim89 commented on Sep 8, 2026

   

   minjikim89

   on Sep 8, 2026

   Last edited by minjikim89

   Author

   More actions

   Hi @mysticalseeker24 — thanks for taking the time to reproduce this on an unrelated build, and for going further than a confirmation. confirm_booking is a better example than the one I filed with: what I described is a promise disagreeing with the screen, and yours is an irreversible commit reported as a failure, where the obvious recovery is to repeat it. That is a stronger case for stating the behaviour explicitly rather than leaving it implied by #248.

   Both of your refinements land on the issue text rather than the code behind it, so the write-up was the defect. Taking them in turn:

   1. The macrotask distinction. The repro shape says "the same abort deferred by one macrotask", but the workaround paragraph does not, and the workaround is the part an implementer copies. What we run schedules the re-sync with setTimeout(..., 0) from the tool wrapper's finally, so it lands in a fresh task rather than at the end of the promise chain. Your point stands as written.

   2. Narrowing the condition. Also right, and what we run is narrower still. The sync itself is never deferred; it skips only the individual unregistration of a tool that is currently executing:

   for (const name of [...live.keys()])
     if (!names.has(name) && !executing.has(name)) unregister(name);
   for (const t of want) if (!live.has(t.name)) await register(t);

   Registrations never wait, so the transient miss your calendar hit cannot arise: a tool that should appear appears in the same sync that skipped the unregistration. deferredState still holds additions back while it waits for the running tool, so the two may be worth comparing on your side.

   Your #255/#306 framing is a better argument for the WPT case than the one I filed with. If a page derives its live set from state, a state-advancing tool unregistering itself is not an edge case, it is the normal outcome.

   I'll tighten the workaround text in the issue body on both points today, and link the reproduction.
4. mysticalseeker24 commented on Sep 12, 2026

   

   mysticalseeker24

   on Sep 12, 2026

   More actions

   Compared, and yours is better — we've switched to it.

   Deferring the whole sync was solving the problem one layer too high. Skipping only the unregistration means a tool that should appear appears in the same sync, so the transient miss is gone at the root instead of being worked around at the call site: we had a calendar component that asked for availability, took the unavailable, and cached it, and we had gated that component on the tool being live to compensate. With registrations no longer held back, that gate is defensive rather than load-bearing.

   Two small details from porting it, in case they are useful for the workaround text:

   Where the skipped unregistration lands. We schedule the catch-up re-sync from the wrapper's finally with setTimeout(..., 0), and only when a skip is actually outstanding. In our case a re-sync would have happened anyway — every execution appends an audit entry, which is itself a state change — but relying on that is relying on an implementation coincidence, so it seemed worth making explicit rather than inheriting it from unrelated state.

   What to publish as the live set while a skip is outstanding. We expose the registered set to the page (a panel that shows getTools() beside the page's own view, and an unavailable[] on a state tool). During the window between the skip and the catch-up, the legal set and the registered set differ by one. We publish what the browser actually holds rather than what is legal, so the two views cannot disagree — the alternative reports a set the browser has not reached yet, which is the same class of mismatch this issue is about, just moved. Not a spec point, but an implementer copying the workaround into a page that surfaces its own tool list will hit the choice.

   Thanks for the tightened text, and for linking the reproduction.
5. mysticalseeker24 commented on Sep 12, 2026

   

   mysticalseeker24

   on Sep 12, 2026

   More actions

   On the second ask — the WPT case is open: web-platform-tests/wpt#62642, webmcp/imperative/executeTool-self-unregistration.https.html.

   I wrote it before @mlmrx's correction on #308, so the framing in the PR description is the old one; the tests themselves are unaffected, since they assert the behaviour rather than a cause. Verified on both sides of the Chromium fix:

   	Chrome 152	Chromium 153 (Edge 153.0.4234.32)
   tool aborts its own signal, then returns	rejects — "The operation failed for an unknown transient reason"	resolves with the tool's result
   tool still unregistered afterwards	yes	yes
   control: aborts a different tool's registration	resolves	resolves

   So it fails on the build with the conformance bug and passes on the build with CL 8224887, which is what makes it worth having.

   Two notes on what it does and does not cover, in case they matter for the prose part of this issue:

   • It asserts the tool is gone from getTools() after the execution resolves, so it cannot be satisfied by ignoring the abort. Delivering the result and honouring the unregistration are both required.
   • It includes the "aborts a different tool's registration" case as a control, which distinguishes this from the existing executeTool-unregister-resolution-race (abort lands after resolve()) and unregister-during-executeTool (unregistered from outside, cross-origin).

   Happy to adjust the wording or split the subtests if reviewers would rather have them as separate files.
6. mysticalseeker24 commented on Sep 17, 2026

   

   mysticalseeker24

   on Sep 17, 2026

   More actions

   Following up on the WPT case I pointed at above, because it bears on your second ask: I have closed it, and I think that ask is already satisfied.

   @domfarolino said on the PR that it looked redundant with the existing files, and that there is no distinction between an unregistration happening inside execute() and one happening a task later. I had assumed there was. I measured three shapes rather than argue:

   shape	Chrome 152.0.7977.83	Chromium 153.0.4234.32
   abort synchronously inside execute, before returning	rejects	resolves
   abort from a later task while execute is still pending	rejects	resolves
   abort synchronously after resolve()	rejects	resolves

   Identical. All three reject with the same transient-reason error on 152 and all three resolve on 153, leaving the tool unregistered in every case. So unregister-during-executeTool and executeTool-unregister-resolution-race already reach this behaviour, and a dedicated self-unregistration file adds nothing. He was right and I have withdrawn it.

   Two things that might still be worth your time, both small:

   • The existing executeTool-unregister-resolution-race asserts the returned value but not that the tool is actually gone from getTools() afterwards, so an implementation could in principle satisfy it by ignoring the abort. One assert_false would close that. I mentioned it on the PR and left it with the editors rather than push a second version.
   • Your first ask, the explicit statement in the tool execute steps, is untouched by any of this. Nothing I measured speaks to the prose.

   Sorry for the detour, and thanks for the original report. The reproduction and the page-side guard still stand; it was only my claim about what WPT lacked that did not.
7. domfarolino commented on Sep 19, 2026

   

   domfarolino

   on Sep 19, 2026

   Collaborator

   More actions

   I don't think the spec has any ambiguity here, so I don't think we need to clarify anything explicitly; there is no distinction between a "self" unregistration vs. any other kind. And there is a lot of Claude-speak in this thread / it's too long. I think we can close this one out.
8. domfarolino

   closed this as not plannedon Sep 19, 2026