[hbenl]

No description provided.

[juliandescottes]

Thanks Holger, the patch follows the direction we discussed, but I think this needs another round. Some comments on the approach:

1 - let's file a separate bug

Initially, the idea behind Bug 2081600 was to consistently report opened dialogs in the response of any tool. We agreed on the bug to early return for tools that get blocked when triggering an alert, but it doesn't address the other goals of the bug. So if we are only doing this, I would file another bug (eg stop auto hiding prompts and unblock tools triggering dialogs). The full solution here normally depends on 2081597 - which should add the "template" to return that kind of information to tool responses. I think it makes sense to split.

2 - handle all tools

This patch is switching the capability to IGNORE, so we need to make sure all impacted tools are not regressing. On top of click_by_uid and evaluate_script, that means:

• press_key
• fill_by_uid + fill_form_by_uid
• type_text
• drag_by_uid_to_uid
• upload_file_by_uid
• print_page

For all of those, if they trigger a user prompt, they now block until the BiDi command times out (10s) and then fail.
Instead of wrapping individual tools, maybe directly handle that in CallToolRequestSchema? Tool definitions could have a flag to say whether or not they should early return in case of a dialog (read only tools might still be allowed to run?). I would default it to true, so that new tools are safe without extra work.

3 - check if a dialog is already opened

Related to my first item, some tools won't block on alerts. Eg a navigation to a page which shows an alert. But if we don't report those alerts, the next call to a tool such as evaluate_script can time out with an error that does not mention the dialog. Since I don't think we should handle the full "report" solution initially mentioned in the bug, I suggest that instead of relying only on events, we also check for already opened dialogs in that wrapper?

4 - ignore events from other contexts

One smaller nit on the approach is that at the moment tool calls will early return if a prompt opens on an unrelated tab. This is a bit of an edge case, but sounds like it should be fixed: if the event is for another context than the current one, ignore it? (compared to the current context at the time of the event, so new_page still works)

[hbenl]

1. let's file a separate bug
   I haven't found any scenarios where a tool can open a dialog (while the tool is still running) without the dialog blocking it, that's why I didn't handle that case. Do you have any?

2. handle all tools
   Not sure what you mean by "directly handle that in CallToolRequestSchema" but I guess you mean the general request handler in src/index.ts. That seems a bit too low-level to me, I'd prefer handling this in defineToolHandler().

3. check if a dialog is already opened
   Makes sense, especially since the page may open dialogs independently of tool calls, we'll never be able to report those in a tool call result.

4. ignore events from other contexts
   Good catch. I initially had the necessary check in there but it got lost somehow.

I don't think we should handle the full "report" solution initially mentioned in the bug

What do you mean by that?

[juliandescottes]

1. let's file a separate bug
   I haven't found any scenarios where a tool can open a dialog (while the tool is still running) without the dialog blocking it, that's why I didn't handle that case. Do you have any?

The goal of the "report" is not necessarily to say what a tool call changed during its lifespan, it's about updating the agent about the status of the browser.

So with that in mind, it can be something triggered by the user or indirectly triggered by a tool call which already returned:

• navigation to a page with wait=none which triggers a prompt on load
• a dialog which happens after a timer (eg click on something, that sends an XHR, answer comes back, prompt)

In those cases the report for those dialogs will not be in the tool call which triggered them (since they didn't open yet), but rather in the next tool call. That closely relates to my point 3. but even if you call a tool which is not blocked by a dialog (and which therefore can succeed), it's better practice for token efficiency to let the agent know that a dialog is currently showing, so that it can clear it before sending the next tool which would be blocked by it.

2. handle all tools
   Not sure what you mean by "directly handle that in CallToolRequestSchema" but I guess you mean the general request handler in `src/index.ts`. That seems a bit too low-level to me, I'd prefer handling this in `defineToolHandler()`.

Yes in the request handler. But as you prefer.

I don't think we should handle the full "report" solution initially mentioned in the bug

What do you mean by that?

Bug 2081597 and other blockers. That's about having a consistent way to report side effects during tool calls, cf first answer.