Skip to content

Set default value for ToolAnnotations readOnlyHint#123

Merged
domfarolino merged 2 commits intowebmachinelearning:mainfrom
beaufortfrancois:readonly-default
Mar 4, 2026
Merged

Set default value for ToolAnnotations readOnlyHint#123
domfarolino merged 2 commits intowebmachinelearning:mainfrom
beaufortfrancois:readonly-default

Conversation

@beaufortfrancois
Copy link
Contributor

This PR sets a default value of false for ToolAnnotations readOnlyHint since MCP already defined a default value of false for readOnlyHint.

This PR also uses dfn-type=dict-member dfn-for=ToolAnnotations to get this nice automatic formatting:
image

@beaufortfrancois
Copy link
Contributor Author

@domfarolino Please review

Copy link
Collaborator

@domfarolino domfarolino left a comment

Choose a reason for hiding this comment

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

Could you update this step (https://webmachinelearning.github.io/webmcp/#ref-for-dom-toolannotations-readonlyhint) to not perform the existence check anymore?

@beaufortfrancois
Copy link
Contributor Author

Could you update this step (https://webmachinelearning.github.io/webmcp/#ref-for-dom-toolannotations-readonlyhint) to not perform the existence check anymore?

Done

@domfarolino domfarolino merged commit 3da78ad into webmachinelearning:main Mar 4, 2026
2 checks passed
github-actions bot added a commit that referenced this pull request Mar 4, 2026
SHA: 3da78ad
Reason: push, by domfarolino

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@beaufortfrancois beaufortfrancois deleted the readonly-default branch March 4, 2026 18:43
beckysiegel pushed a commit to chromium/chromium that referenced this pull request Mar 4, 2026
This CL fixes a crash in WebMCP's `registerTool()` method, whereby if
the optional `annotations` member was present, we assumed its
also-optional `readOnlyHint` member was always available, and grabbed it
without checking `hasReadOnlyHint()` first, which was necessary since
`readOnlyHint` is an optional member.

This CL gives `readOnlyHint` a default value of `false` so that it is
always present, and CHECK()s that `hasReadOnlyHint()` is true before
accessing. See
webmachinelearning/webmcp#123.

R=bengr

Bug: N/A
Change-Id: Ie2761008089186aa30abe571c4834df500a711bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7614599
Reviewed-by: Ben Greenstein <bengr@chromium.org>
Commit-Queue: Dominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1594198}
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