Skip to content

Fix concurrency, detached document, and permissions checks in algorithms. - #337

Open
markafoltz wants to merge 2 commits into
mainfrom
fix-algorithms
Open

markafoltz wants to merge 2 commits into
mainfrom
fix-algorithms

Conversation

@markafoltz

@markafoltz markafoltz commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator
  • Eliminate a redundant check for an aborted signal.
  • In tool enumeration, skip frames that don't have the tools permission.
  • Add a note about locking for the tool map before it's shown to the browser agent.
  • Add a check for a detached document before allowing a tool call on it.
  • Separate out steps to enumerate tools, from constructing RegisteredTools, as parse a JSON string cannot happen in parallel.
    • Maybe there's a different / cleaner way to handle this? But I think the parallel steps have to complete first outside of any script context, as they traverse the frame tree.

Preview | Diff

@markafoltz
markafoltz requested a review from domfarolino October 7, 2026 05:19
Comment thread index.bs Outdated
Comment thread index.bs
[=iteration/continue=].

1. [=list/Append=] (|tool definition|, |targetDocument|, |targetOrigin|) to
|discoveredTools|.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can omit |targetOrigin| since that's already captured by |targetDocument|'s [=Document/origin=] right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if |targetOrigin| can change between the capture here and the task below that iterates over |discoveredTools|.

Comment thread index.bs
Comment thread index.bs

1. Let |document| be |descendant|'s [=navigable/active document=].

1. If |document| is not [=allowed to use=] the "{{tools}}" feature, then

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I suppose this isn't strictly necessary since a document without this feature cannot even populate tools in its internal context's tool map accessed below, right?

I guess it's good documentation for an agent though. Does my assessment above match your understanding?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The perform and observation algorithm can be triggered at any time. If the spec can guarantee that the tool map is always synchronized with the state of the tools permission, then this can be skipped. But it's also fine to leave in as documentation. Let me know what you think.

Comment thread index.bs Outdated

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants