Repository navigation
feat(core): dynamic tool annotations for multi-mode tools on read-only sources - #3816
Merged
Merged
Conversation
Contributor
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
anubhav756
force-pushed
the
anubhav-readonly-annotation
branch
from
August 13, 2026 07:49
8930b29 to
53be9fe
Compare
anubhav756
force-pushed
the
anubhav-readonly-annotation
branch
from
August 13, 2026 08:14
53be9fe to
5fade7d
Compare
3 tasks done
anubhav756
force-pushed
the
anubhav-readonly-annotation
branch
from
August 13, 2026 08:33
5fade7d to
f36035b
Compare
anubhav756
force-pushed
the
anubhav-readonly-core
branch
from
August 13, 2026 08:38
a05fcb6 to
c81fa4f
Compare
anubhav756
force-pushed
the
anubhav-readonly-annotation
branch
3 times, most recently
from
August 13, 2026 10:24
ee8f29a to
9dc7d07
Compare
anubhav756
force-pushed
the
anubhav-readonly-core
branch
from
August 13, 2026 13:09
c81fa4f to
e5ff38c
Compare
anubhav756
force-pushed
the
anubhav-readonly-annotation
branch
2 times, most recently
from
August 13, 2026 13:11
dd8d903 to
2b6758a
Compare
Yuan325
requested changes
Aug 13, 2026
Yuan325
added
the
priority: p1
anubhav756
force-pushed
the
anubhav-readonly-core
branch
from
August 25, 2026 08:43
132d067 to
3d72b41
Compare
anubhav756
force-pushed
the
anubhav-readonly-annotation
branch
from
August 25, 2026 08:43
093efc9 to
3cc4d05
Compare
shuzhou-gc
approved these changes
Aug 25, 2026
anubhav756
force-pushed
the
anubhav-readonly-core
branch
from
August 25, 2026 18:11
3d72b41 to
8ab1506
Compare
anubhav756
force-pushed
the
anubhav-readonly-annotation
branch
from
August 25, 2026 18:11
3cc4d05 to
adf943c
Compare
anubhav756
force-pushed
the
anubhav-readonly-core
branch
from
August 26, 2026 21:06
8ab1506 to
d2f77c7
Compare
anubhav756
force-pushed
the
anubhav-readonly-annotation
branch
from
August 26, 2026 21:06
adf943c to
e6f7564
Compare
anubhav756
force-pushed
the
anubhav-readonly-annotation
branch
from
August 27, 2026 06:48
e6f7564 to
a96d2ab
Compare
anubhav756
force-pushed
the
feat/read-only
branch
from
August 27, 2026 06:48
4fb5db3 to
58fdcab
Compare
anubhav756
force-pushed
the
anubhav-readonly-annotation
branch
from
August 27, 2026 06:48
a96d2ab to
0a4a063
Compare
Contributor
|
🧨 Preview deployments removed. Cloudflare Pages environments for |
anubhav756
added a commit
that referenced
this pull request
Aug 27, 2026
…y sources (#3816) ## Summary Enables multi-mode tools (e.g., SQL execution tools) to dynamically advertise `readOnlyHint: true` and `destructiveHint: false` in MCP tool manifests when bound to a read-only data source, and centralizes tool suppression logic across the server. ## Context & Motivation * Tools like `postgres-execute-sql` and `mysql-execute-sql` are multi-mode (capable of both reads and writes) and default to `readOnlyHint: false` / `destructiveHint: true`. When connected to a `readOnly: true` database source (where session-level locks prevent modifications), these tools should dynamically report `readOnlyHint: true` and `destructiveHint: false` to MCP clients without requiring separate read-only tool implementations. * Refactored `ShouldSuppress` from a `BaseTool` method into a package-level function `tools.ShouldSuppress(ctx, t, src)` that operates on the `Tool` interface, allowing suppression to dynamically evaluate `t.GetAnnotations(src)` without requiring concrete tool method overrides. ## Changes * Updated `Tool.GetAnnotations(sources.Source) *ToolAnnotations` to make annotations source-aware (matching the design pattern of `GetParameters(sources.Source)` and `Manifest(sources.Source)`). * Updated `BaseTool.GetAnnotations(_ sources.Source)` to return static annotations by default. * Added `DynamicReadOnlyAnnotations(base *ToolAnnotations) *ToolAnnotations` in `internal/tools/tools.go` to safely copy base annotations and set `ReadOnlyHint: true` / `DestructiveHint: false` while preserving other custom hints (e.g. `idempotentHint`, `openWorldHint`). * Updated `postgres-execute-sql` and `mysql-execute-sql` to implement `GetAnnotations(src)` using `tools.DynamicReadOnlyAnnotations(t.BaseTool.GetAnnotations(src))` when `src.IsReadOnly()`. * Removed redundant `ShouldSuppress` method overrides from concrete SQL tool structs. * Updated all 5 MCP schema version generators (`v20241105`, `v20250326`, `v20250618`, `v20251125`, `v20260728`) in `internal/server/mcp/` to pass `src` into `tool.GetAnnotations(src)`. * Added table-driven tests for `tools.DynamicReadOnlyAnnotations` in `internal/tools/tools_test.go` verifying nil handling, default flipping, custom hint preservation, and pointer immutability. * Updated `internal/server/server_test.go` and `internal/tools/tools_test.go` to test `tools.ShouldSuppress` with both write and destructive tools using `tools.NewWriteAnnotations()` and `tools.NewDestructiveAnnotations()`. * Updated Looker unit test suites to pass `tool.GetAnnotations(nil)`.
anubhav756
added a commit
that referenced
this pull request
Aug 27, 2026
…y sources (#3816) ## Summary Enables multi-mode tools (e.g., SQL execution tools) to dynamically advertise `readOnlyHint: true` and `destructiveHint: false` in MCP tool manifests when bound to a read-only data source, and centralizes tool suppression logic across the server. ## Context & Motivation * Tools like `postgres-execute-sql` and `mysql-execute-sql` are multi-mode (capable of both reads and writes) and default to `readOnlyHint: false` / `destructiveHint: true`. When connected to a `readOnly: true` database source (where session-level locks prevent modifications), these tools should dynamically report `readOnlyHint: true` and `destructiveHint: false` to MCP clients without requiring separate read-only tool implementations. * Refactored `ShouldSuppress` from a `BaseTool` method into a package-level function `tools.ShouldSuppress(ctx, t, src)` that operates on the `Tool` interface, allowing suppression to dynamically evaluate `t.GetAnnotations(src)` without requiring concrete tool method overrides. ## Changes * Updated `Tool.GetAnnotations(sources.Source) *ToolAnnotations` to make annotations source-aware (matching the design pattern of `GetParameters(sources.Source)` and `Manifest(sources.Source)`). * Updated `BaseTool.GetAnnotations(_ sources.Source)` to return static annotations by default. * Added `DynamicReadOnlyAnnotations(base *ToolAnnotations) *ToolAnnotations` in `internal/tools/tools.go` to safely copy base annotations and set `ReadOnlyHint: true` / `DestructiveHint: false` while preserving other custom hints (e.g. `idempotentHint`, `openWorldHint`). * Updated `postgres-execute-sql` and `mysql-execute-sql` to implement `GetAnnotations(src)` using `tools.DynamicReadOnlyAnnotations(t.BaseTool.GetAnnotations(src))` when `src.IsReadOnly()`. * Removed redundant `ShouldSuppress` method overrides from concrete SQL tool structs. * Updated all 5 MCP schema version generators (`v20241105`, `v20250326`, `v20250618`, `v20251125`, `v20260728`) in `internal/server/mcp/` to pass `src` into `tool.GetAnnotations(src)`. * Added table-driven tests for `tools.DynamicReadOnlyAnnotations` in `internal/tools/tools_test.go` verifying nil handling, default flipping, custom hint preservation, and pointer immutability. * Updated `internal/server/server_test.go` and `internal/tools/tools_test.go` to test `tools.ShouldSuppress` with both write and destructive tools using `tools.NewWriteAnnotations()` and `tools.NewDestructiveAnnotations()`. * Updated Looker unit test suites to pass `tool.GetAnnotations(nil)`.
anubhav756
added a commit
that referenced
this pull request
Aug 27, 2026
…y sources (#3816) ## Summary Enables multi-mode tools (e.g., SQL execution tools) to dynamically advertise `readOnlyHint: true` and `destructiveHint: false` in MCP tool manifests when bound to a read-only data source, and centralizes tool suppression logic across the server. ## Context & Motivation * Tools like `postgres-execute-sql` and `mysql-execute-sql` are multi-mode (capable of both reads and writes) and default to `readOnlyHint: false` / `destructiveHint: true`. When connected to a `readOnly: true` database source (where session-level locks prevent modifications), these tools should dynamically report `readOnlyHint: true` and `destructiveHint: false` to MCP clients without requiring separate read-only tool implementations. * Refactored `ShouldSuppress` from a `BaseTool` method into a package-level function `tools.ShouldSuppress(ctx, t, src)` that operates on the `Tool` interface, allowing suppression to dynamically evaluate `t.GetAnnotations(src)` without requiring concrete tool method overrides. ## Changes * Updated `Tool.GetAnnotations(sources.Source) *ToolAnnotations` to make annotations source-aware (matching the design pattern of `GetParameters(sources.Source)` and `Manifest(sources.Source)`). * Updated `BaseTool.GetAnnotations(_ sources.Source)` to return static annotations by default. * Added `DynamicReadOnlyAnnotations(base *ToolAnnotations) *ToolAnnotations` in `internal/tools/tools.go` to safely copy base annotations and set `ReadOnlyHint: true` / `DestructiveHint: false` while preserving other custom hints (e.g. `idempotentHint`, `openWorldHint`). * Updated `postgres-execute-sql` and `mysql-execute-sql` to implement `GetAnnotations(src)` using `tools.DynamicReadOnlyAnnotations(t.BaseTool.GetAnnotations(src))` when `src.IsReadOnly()`. * Removed redundant `ShouldSuppress` method overrides from concrete SQL tool structs. * Updated all 5 MCP schema version generators (`v20241105`, `v20250326`, `v20250618`, `v20251125`, `v20260728`) in `internal/server/mcp/` to pass `src` into `tool.GetAnnotations(src)`. * Added table-driven tests for `tools.DynamicReadOnlyAnnotations` in `internal/tools/tools_test.go` verifying nil handling, default flipping, custom hint preservation, and pointer immutability. * Updated `internal/server/server_test.go` and `internal/tools/tools_test.go` to test `tools.ShouldSuppress` with both write and destructive tools using `tools.NewWriteAnnotations()` and `tools.NewDestructiveAnnotations()`. * Updated Looker unit test suites to pass `tool.GetAnnotations(nil)`.
anubhav756
added a commit
that referenced
this pull request
Aug 27, 2026
…y sources (#3816) ## Summary Enables multi-mode tools (e.g., SQL execution tools) to dynamically advertise `readOnlyHint: true` and `destructiveHint: false` in MCP tool manifests when bound to a read-only data source, and centralizes tool suppression logic across the server. ## Context & Motivation * Tools like `postgres-execute-sql` and `mysql-execute-sql` are multi-mode (capable of both reads and writes) and default to `readOnlyHint: false` / `destructiveHint: true`. When connected to a `readOnly: true` database source (where session-level locks prevent modifications), these tools should dynamically report `readOnlyHint: true` and `destructiveHint: false` to MCP clients without requiring separate read-only tool implementations. * Refactored `ShouldSuppress` from a `BaseTool` method into a package-level function `tools.ShouldSuppress(ctx, t, src)` that operates on the `Tool` interface, allowing suppression to dynamically evaluate `t.GetAnnotations(src)` without requiring concrete tool method overrides. ## Changes * Updated `Tool.GetAnnotations(sources.Source) *ToolAnnotations` to make annotations source-aware (matching the design pattern of `GetParameters(sources.Source)` and `Manifest(sources.Source)`). * Updated `BaseTool.GetAnnotations(_ sources.Source)` to return static annotations by default. * Added `DynamicReadOnlyAnnotations(base *ToolAnnotations) *ToolAnnotations` in `internal/tools/tools.go` to safely copy base annotations and set `ReadOnlyHint: true` / `DestructiveHint: false` while preserving other custom hints (e.g. `idempotentHint`, `openWorldHint`). * Updated `postgres-execute-sql` and `mysql-execute-sql` to implement `GetAnnotations(src)` using `tools.DynamicReadOnlyAnnotations(t.BaseTool.GetAnnotations(src))` when `src.IsReadOnly()`. * Removed redundant `ShouldSuppress` method overrides from concrete SQL tool structs. * Updated all 5 MCP schema version generators (`v20241105`, `v20250326`, `v20250618`, `v20251125`, `v20260728`) in `internal/server/mcp/` to pass `src` into `tool.GetAnnotations(src)`. * Added table-driven tests for `tools.DynamicReadOnlyAnnotations` in `internal/tools/tools_test.go` verifying nil handling, default flipping, custom hint preservation, and pointer immutability. * Updated `internal/server/server_test.go` and `internal/tools/tools_test.go` to test `tools.ShouldSuppress` with both write and destructive tools using `tools.NewWriteAnnotations()` and `tools.NewDestructiveAnnotations()`. * Updated Looker unit test suites to pass `tool.GetAnnotations(nil)`.
anubhav756
added a commit
that referenced
this pull request
Aug 27, 2026
…y sources (#3816) ## Summary Enables multi-mode tools (e.g., SQL execution tools) to dynamically advertise `readOnlyHint: true` and `destructiveHint: false` in MCP tool manifests when bound to a read-only data source, and centralizes tool suppression logic across the server. ## Context & Motivation * Tools like `postgres-execute-sql` and `mysql-execute-sql` are multi-mode (capable of both reads and writes) and default to `readOnlyHint: false` / `destructiveHint: true`. When connected to a `readOnly: true` database source (where session-level locks prevent modifications), these tools should dynamically report `readOnlyHint: true` and `destructiveHint: false` to MCP clients without requiring separate read-only tool implementations. * Refactored `ShouldSuppress` from a `BaseTool` method into a package-level function `tools.ShouldSuppress(ctx, t, src)` that operates on the `Tool` interface, allowing suppression to dynamically evaluate `t.GetAnnotations(src)` without requiring concrete tool method overrides. ## Changes * Updated `Tool.GetAnnotations(sources.Source) *ToolAnnotations` to make annotations source-aware (matching the design pattern of `GetParameters(sources.Source)` and `Manifest(sources.Source)`). * Updated `BaseTool.GetAnnotations(_ sources.Source)` to return static annotations by default. * Added `DynamicReadOnlyAnnotations(base *ToolAnnotations) *ToolAnnotations` in `internal/tools/tools.go` to safely copy base annotations and set `ReadOnlyHint: true` / `DestructiveHint: false` while preserving other custom hints (e.g. `idempotentHint`, `openWorldHint`). * Updated `postgres-execute-sql` and `mysql-execute-sql` to implement `GetAnnotations(src)` using `tools.DynamicReadOnlyAnnotations(t.BaseTool.GetAnnotations(src))` when `src.IsReadOnly()`. * Removed redundant `ShouldSuppress` method overrides from concrete SQL tool structs. * Updated all 5 MCP schema version generators (`v20241105`, `v20250326`, `v20250618`, `v20251125`, `v20260728`) in `internal/server/mcp/` to pass `src` into `tool.GetAnnotations(src)`. * Added table-driven tests for `tools.DynamicReadOnlyAnnotations` in `internal/tools/tools_test.go` verifying nil handling, default flipping, custom hint preservation, and pointer immutability. * Updated `internal/server/server_test.go` and `internal/tools/tools_test.go` to test `tools.ShouldSuppress` with both write and destructive tools using `tools.NewWriteAnnotations()` and `tools.NewDestructiveAnnotations()`. * Updated Looker unit test suites to pass `tool.GetAnnotations(nil)`.
anubhav756
added a commit
that referenced
this pull request
Aug 27, 2026
…y sources (#3816) ## Summary Enables multi-mode tools (e.g., SQL execution tools) to dynamically advertise `readOnlyHint: true` and `destructiveHint: false` in MCP tool manifests when bound to a read-only data source, and centralizes tool suppression logic across the server. ## Context & Motivation * Tools like `postgres-execute-sql` and `mysql-execute-sql` are multi-mode (capable of both reads and writes) and default to `readOnlyHint: false` / `destructiveHint: true`. When connected to a `readOnly: true` database source (where session-level locks prevent modifications), these tools should dynamically report `readOnlyHint: true` and `destructiveHint: false` to MCP clients without requiring separate read-only tool implementations. * Refactored `ShouldSuppress` from a `BaseTool` method into a package-level function `tools.ShouldSuppress(ctx, t, src)` that operates on the `Tool` interface, allowing suppression to dynamically evaluate `t.GetAnnotations(src)` without requiring concrete tool method overrides. ## Changes * Updated `Tool.GetAnnotations(sources.Source) *ToolAnnotations` to make annotations source-aware (matching the design pattern of `GetParameters(sources.Source)` and `Manifest(sources.Source)`). * Updated `BaseTool.GetAnnotations(_ sources.Source)` to return static annotations by default. * Added `DynamicReadOnlyAnnotations(base *ToolAnnotations) *ToolAnnotations` in `internal/tools/tools.go` to safely copy base annotations and set `ReadOnlyHint: true` / `DestructiveHint: false` while preserving other custom hints (e.g. `idempotentHint`, `openWorldHint`). * Updated `postgres-execute-sql` and `mysql-execute-sql` to implement `GetAnnotations(src)` using `tools.DynamicReadOnlyAnnotations(t.BaseTool.GetAnnotations(src))` when `src.IsReadOnly()`. * Removed redundant `ShouldSuppress` method overrides from concrete SQL tool structs. * Updated all 5 MCP schema version generators (`v20241105`, `v20250326`, `v20250618`, `v20251125`, `v20260728`) in `internal/server/mcp/` to pass `src` into `tool.GetAnnotations(src)`. * Added table-driven tests for `tools.DynamicReadOnlyAnnotations` in `internal/tools/tools_test.go` verifying nil handling, default flipping, custom hint preservation, and pointer immutability. * Updated `internal/server/server_test.go` and `internal/tools/tools_test.go` to test `tools.ShouldSuppress` with both write and destructive tools using `tools.NewWriteAnnotations()` and `tools.NewDestructiveAnnotations()`. * Updated Looker unit test suites to pass `tool.GetAnnotations(nil)`.
anubhav756
added a commit
that referenced
this pull request
Aug 27, 2026
…y sources (#3816) ## Summary Enables multi-mode tools (e.g., SQL execution tools) to dynamically advertise `readOnlyHint: true` and `destructiveHint: false` in MCP tool manifests when bound to a read-only data source, and centralizes tool suppression logic across the server. ## Context & Motivation * Tools like `postgres-execute-sql` and `mysql-execute-sql` are multi-mode (capable of both reads and writes) and default to `readOnlyHint: false` / `destructiveHint: true`. When connected to a `readOnly: true` database source (where session-level locks prevent modifications), these tools should dynamically report `readOnlyHint: true` and `destructiveHint: false` to MCP clients without requiring separate read-only tool implementations. * Refactored `ShouldSuppress` from a `BaseTool` method into a package-level function `tools.ShouldSuppress(ctx, t, src)` that operates on the `Tool` interface, allowing suppression to dynamically evaluate `t.GetAnnotations(src)` without requiring concrete tool method overrides. ## Changes * Updated `Tool.GetAnnotations(sources.Source) *ToolAnnotations` to make annotations source-aware (matching the design pattern of `GetParameters(sources.Source)` and `Manifest(sources.Source)`). * Updated `BaseTool.GetAnnotations(_ sources.Source)` to return static annotations by default. * Added `DynamicReadOnlyAnnotations(base *ToolAnnotations) *ToolAnnotations` in `internal/tools/tools.go` to safely copy base annotations and set `ReadOnlyHint: true` / `DestructiveHint: false` while preserving other custom hints (e.g. `idempotentHint`, `openWorldHint`). * Updated `postgres-execute-sql` and `mysql-execute-sql` to implement `GetAnnotations(src)` using `tools.DynamicReadOnlyAnnotations(t.BaseTool.GetAnnotations(src))` when `src.IsReadOnly()`. * Removed redundant `ShouldSuppress` method overrides from concrete SQL tool structs. * Updated all 5 MCP schema version generators (`v20241105`, `v20250326`, `v20250618`, `v20251125`, `v20260728`) in `internal/server/mcp/` to pass `src` into `tool.GetAnnotations(src)`. * Added table-driven tests for `tools.DynamicReadOnlyAnnotations` in `internal/tools/tools_test.go` verifying nil handling, default flipping, custom hint preservation, and pointer immutability. * Updated `internal/server/server_test.go` and `internal/tools/tools_test.go` to test `tools.ShouldSuppress` with both write and destructive tools using `tools.NewWriteAnnotations()` and `tools.NewDestructiveAnnotations()`. * Updated Looker unit test suites to pass `tool.GetAnnotations(nil)`.
anubhav756
added a commit
that referenced
this pull request
Aug 27, 2026
…y sources (#3816) ## Summary Enables multi-mode tools (e.g., SQL execution tools) to dynamically advertise `readOnlyHint: true` and `destructiveHint: false` in MCP tool manifests when bound to a read-only data source, and centralizes tool suppression logic across the server. ## Context & Motivation * Tools like `postgres-execute-sql` and `mysql-execute-sql` are multi-mode (capable of both reads and writes) and default to `readOnlyHint: false` / `destructiveHint: true`. When connected to a `readOnly: true` database source (where session-level locks prevent modifications), these tools should dynamically report `readOnlyHint: true` and `destructiveHint: false` to MCP clients without requiring separate read-only tool implementations. * Refactored `ShouldSuppress` from a `BaseTool` method into a package-level function `tools.ShouldSuppress(ctx, t, src)` that operates on the `Tool` interface, allowing suppression to dynamically evaluate `t.GetAnnotations(src)` without requiring concrete tool method overrides. ## Changes * Updated `Tool.GetAnnotations(sources.Source) *ToolAnnotations` to make annotations source-aware (matching the design pattern of `GetParameters(sources.Source)` and `Manifest(sources.Source)`). * Updated `BaseTool.GetAnnotations(_ sources.Source)` to return static annotations by default. * Added `DynamicReadOnlyAnnotations(base *ToolAnnotations) *ToolAnnotations` in `internal/tools/tools.go` to safely copy base annotations and set `ReadOnlyHint: true` / `DestructiveHint: false` while preserving other custom hints (e.g. `idempotentHint`, `openWorldHint`). * Updated `postgres-execute-sql` and `mysql-execute-sql` to implement `GetAnnotations(src)` using `tools.DynamicReadOnlyAnnotations(t.BaseTool.GetAnnotations(src))` when `src.IsReadOnly()`. * Removed redundant `ShouldSuppress` method overrides from concrete SQL tool structs. * Updated all 5 MCP schema version generators (`v20241105`, `v20250326`, `v20250618`, `v20251125`, `v20260728`) in `internal/server/mcp/` to pass `src` into `tool.GetAnnotations(src)`. * Added table-driven tests for `tools.DynamicReadOnlyAnnotations` in `internal/tools/tools_test.go` verifying nil handling, default flipping, custom hint preservation, and pointer immutability. * Updated `internal/server/server_test.go` and `internal/tools/tools_test.go` to test `tools.ShouldSuppress` with both write and destructive tools using `tools.NewWriteAnnotations()` and `tools.NewDestructiveAnnotations()`. * Updated Looker unit test suites to pass `tool.GetAnnotations(nil)`.
anubhav756
added a commit
that referenced
this pull request
Aug 27, 2026
…y sources (#3816) ## Summary Enables multi-mode tools (e.g., SQL execution tools) to dynamically advertise `readOnlyHint: true` and `destructiveHint: false` in MCP tool manifests when bound to a read-only data source, and centralizes tool suppression logic across the server. ## Context & Motivation * Tools like `postgres-execute-sql` and `mysql-execute-sql` are multi-mode (capable of both reads and writes) and default to `readOnlyHint: false` / `destructiveHint: true`. When connected to a `readOnly: true` database source (where session-level locks prevent modifications), these tools should dynamically report `readOnlyHint: true` and `destructiveHint: false` to MCP clients without requiring separate read-only tool implementations. * Refactored `ShouldSuppress` from a `BaseTool` method into a package-level function `tools.ShouldSuppress(ctx, t, src)` that operates on the `Tool` interface, allowing suppression to dynamically evaluate `t.GetAnnotations(src)` without requiring concrete tool method overrides. ## Changes * Updated `Tool.GetAnnotations(sources.Source) *ToolAnnotations` to make annotations source-aware (matching the design pattern of `GetParameters(sources.Source)` and `Manifest(sources.Source)`). * Updated `BaseTool.GetAnnotations(_ sources.Source)` to return static annotations by default. * Added `DynamicReadOnlyAnnotations(base *ToolAnnotations) *ToolAnnotations` in `internal/tools/tools.go` to safely copy base annotations and set `ReadOnlyHint: true` / `DestructiveHint: false` while preserving other custom hints (e.g. `idempotentHint`, `openWorldHint`). * Updated `postgres-execute-sql` and `mysql-execute-sql` to implement `GetAnnotations(src)` using `tools.DynamicReadOnlyAnnotations(t.BaseTool.GetAnnotations(src))` when `src.IsReadOnly()`. * Removed redundant `ShouldSuppress` method overrides from concrete SQL tool structs. * Updated all 5 MCP schema version generators (`v20241105`, `v20250326`, `v20250618`, `v20251125`, `v20260728`) in `internal/server/mcp/` to pass `src` into `tool.GetAnnotations(src)`. * Added table-driven tests for `tools.DynamicReadOnlyAnnotations` in `internal/tools/tools_test.go` verifying nil handling, default flipping, custom hint preservation, and pointer immutability. * Updated `internal/server/server_test.go` and `internal/tools/tools_test.go` to test `tools.ShouldSuppress` with both write and destructive tools using `tools.NewWriteAnnotations()` and `tools.NewDestructiveAnnotations()`. * Updated Looker unit test suites to pass `tool.GetAnnotations(nil)`.
anubhav756
added a commit
that referenced
this pull request
Aug 27, 2026
…y sources (#3816) ## Summary Enables multi-mode tools (e.g., SQL execution tools) to dynamically advertise `readOnlyHint: true` and `destructiveHint: false` in MCP tool manifests when bound to a read-only data source, and centralizes tool suppression logic across the server. ## Context & Motivation * Tools like `postgres-execute-sql` and `mysql-execute-sql` are multi-mode (capable of both reads and writes) and default to `readOnlyHint: false` / `destructiveHint: true`. When connected to a `readOnly: true` database source (where session-level locks prevent modifications), these tools should dynamically report `readOnlyHint: true` and `destructiveHint: false` to MCP clients without requiring separate read-only tool implementations. * Refactored `ShouldSuppress` from a `BaseTool` method into a package-level function `tools.ShouldSuppress(ctx, t, src)` that operates on the `Tool` interface, allowing suppression to dynamically evaluate `t.GetAnnotations(src)` without requiring concrete tool method overrides. ## Changes * Updated `Tool.GetAnnotations(sources.Source) *ToolAnnotations` to make annotations source-aware (matching the design pattern of `GetParameters(sources.Source)` and `Manifest(sources.Source)`). * Updated `BaseTool.GetAnnotations(_ sources.Source)` to return static annotations by default. * Added `DynamicReadOnlyAnnotations(base *ToolAnnotations) *ToolAnnotations` in `internal/tools/tools.go` to safely copy base annotations and set `ReadOnlyHint: true` / `DestructiveHint: false` while preserving other custom hints (e.g. `idempotentHint`, `openWorldHint`). * Updated `postgres-execute-sql` and `mysql-execute-sql` to implement `GetAnnotations(src)` using `tools.DynamicReadOnlyAnnotations(t.BaseTool.GetAnnotations(src))` when `src.IsReadOnly()`. * Removed redundant `ShouldSuppress` method overrides from concrete SQL tool structs. * Updated all 5 MCP schema version generators (`v20241105`, `v20250326`, `v20250618`, `v20251125`, `v20260728`) in `internal/server/mcp/` to pass `src` into `tool.GetAnnotations(src)`. * Added table-driven tests for `tools.DynamicReadOnlyAnnotations` in `internal/tools/tools_test.go` verifying nil handling, default flipping, custom hint preservation, and pointer immutability. * Updated `internal/server/server_test.go` and `internal/tools/tools_test.go` to test `tools.ShouldSuppress` with both write and destructive tools using `tools.NewWriteAnnotations()` and `tools.NewDestructiveAnnotations()`. * Updated Looker unit test suites to pass `tool.GetAnnotations(nil)`.
anubhav756
added a commit
that referenced
this pull request
Aug 27, 2026
…ions (#3872) ## Description This PR introduces end-to-end Read-Only mode support across Toolbox, spanning core framework tool suppression, source-aware dynamic MCP tool annotations, and protocol/session-level enforcement for database sources. When a data source is configured in read-only mode: 1. **Agent-Level Tool Suppression**: Write-capable tools bound to read-only sources are suppressed from tool registration and pruned from tool groups to save LLM context window space and prevent hallucinated write attempts. 2. **Dynamic Tool Annotations**: Multi-mode tools (such as SQL execution tools) dynamically advertise `readOnlyHint: true` and `destructiveHint: false` in MCP tool manifests when connected to read-only sources. 3. **Database Session-Level Enforcement**: Database drivers enforce strict, protocol-level session locking directly within the database engine/client driver, preventing prompt injection or multi-statement write breakouts. ## PRs Included - #3615 - #3816 - #3618 - #3851 - #3619 - #3617
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Enables multi-mode tools (e.g., SQL execution tools) to dynamically advertise
readOnlyHint: trueanddestructiveHint: falsein MCP tool manifests when bound to a read-only data source, and centralizes tool suppression logic across the server.Context & Motivation
postgres-execute-sqlandmysql-execute-sqlare multi-mode (capable of both reads and writes) and default toreadOnlyHint: false/destructiveHint: true. When connected to areadOnly: truedatabase source (where session-level locks prevent modifications), these tools should dynamically reportreadOnlyHint: trueanddestructiveHint: falseto MCP clients without requiring separate read-only tool implementations.ShouldSuppressfrom aBaseToolmethod into a package-level functiontools.ShouldSuppress(ctx, t, src)that operates on theToolinterface, allowing suppression to dynamically evaluatet.GetAnnotations(src)without requiring concrete tool method overrides.Changes
Tool.GetAnnotations(sources.Source) *ToolAnnotationsto make annotations source-aware (matching the design pattern ofGetParameters(sources.Source)andManifest(sources.Source)).BaseTool.GetAnnotations(_ sources.Source)to return static annotations by default.DynamicReadOnlyAnnotations(base *ToolAnnotations) *ToolAnnotationsininternal/tools/tools.goto safely copy base annotations and setReadOnlyHint: true/DestructiveHint: falsewhile preserving other custom hints (e.g.idempotentHint,openWorldHint).postgres-execute-sqlandmysql-execute-sqlto implementGetAnnotations(src)usingtools.DynamicReadOnlyAnnotations(t.BaseTool.GetAnnotations(src))whensrc.IsReadOnly().ShouldSuppressmethod overrides from concrete SQL tool structs.v20241105,v20250326,v20250618,v20251125,v20260728) ininternal/server/mcp/to passsrcintotool.GetAnnotations(src).tools.DynamicReadOnlyAnnotationsininternal/tools/tools_test.goverifying nil handling, default flipping, custom hint preservation, and pointer immutability.internal/server/server_test.goandinternal/tools/tools_test.goto testtools.ShouldSuppresswith both write and destructive tools usingtools.NewWriteAnnotations()andtools.NewDestructiveAnnotations().tool.GetAnnotations(nil).