Skip to content

feat(tool/mongodb): add mongodb-list-collections tool - #4153

Open
SWITCHin2 wants to merge 10 commits into
googleapis:mainfrom
SWITCHin2:feat/mongodb-list-collections
Open

SWITCHin2 wants to merge 10 commits into
googleapis:mainfrom
SWITCHin2:feat/mongodb-list-collections

Conversation

@SWITCHin2

Copy link
Copy Markdown
Contributor

Description

Stacked on #4152. Until that PR merges, this diff also shows its commits. Please review only the last 3 commits.

Adds a mongodb-list-collections tool so an agent can find out which collections exist before it picks a runtime collection.

  • New source method ListCollectionNames(ctx, database). It returns the sorted collection names in a database. When the source has allowedCollections, it returns only the allowed collections that actually exist.
  • New tool type mongodb-list-collections. It takes no parameters (the database comes from config), is annotated read-only by default, and is registered in cmd/internal/imports.go.
  • Fails at startup if a scoped source allows no collections in the tool's database. Otherwise the tool would always return an empty list.
  • Tests and docs. Unit tests, integration coverage, and a doc page under docs/en/integrations/mongodb/tools/.
kind: tool
name: list_collections
type: mongodb-list-collections
source: my-mongo-source
database: crm
description: List the collections available in the database.

The two features answer different questions. The enum from #4152 tells the agent which collections it is allowed to use (fixed in config). This tool tells it which collections exist right now, including when no allow-list is configured.

PR Checklist

  • Make sure you reviewed
    CONTRIBUTING.md
  • Make sure to open an issue as a
    bug/issue
    before writing your code! That way we can discuss the change, evaluate
    designs, and agree on the general idea
  • Ensure you have manually reviewed the entire diff before requesting a
    review
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)
  • Make sure to add ! if this involve a breaking change

@SWITCHin2
SWITCHin2 force-pushed the feat/mongodb-list-collections branch from f3e2e8e to fc62b35 Compare September 27, 2026 14:27

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces collection scoping and restriction capabilities for MongoDB sources and tools, allowing administrators to restrict tools to a specific set of database collections via the allowedCollections configuration. It also adds a new mongodb-list-collections tool for discovering available collections and updates parameter manifests to advertise literal allowedValues as JSON Schema enum values to MCP clients. The review feedback suggests a more robust heap allocation pattern in ScopeCollectionParam to avoid taking the address of a loop-scoped variable, which prevents potential linter warnings or developer confusion.

Comment on lines +128 to +130
narrowed := *sp
narrowed.AllowedValues = anySlice(allowed)
scoped[i] = &narrowed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Taking the address of a loop-scoped variable (&narrowed) can sometimes lead to subtle bugs or trigger static analysis/linter warnings, even though Go 1.22+ handles loop variable lifetimes more safely.

Using new(parameters.StringParameter) to explicitly allocate the struct on the heap and then copying the value is a more robust and idiomatic pattern that avoids any potential linter warnings or developer confusion.

		narrowed := new(parameters.StringParameter)
		*narrowed = *sp
		narrowed.AllowedValues = anySlice(allowed)
		scoped[i] = narrowed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This one's safe as written: narrowed := *sp is declared inside the loop body, so each iteration gets its own copy (even before Go 1.22), and taking its address is fine. The aliasing lint (gosec G601) only applies to range loop variables. Using new() plus a copy would do the same thing,

@SWITCHin2
SWITCHin2 marked this pull request as ready for review September 27, 2026 14:48
@SWITCHin2
SWITCHin2 requested a review from a team as a code owner September 27, 2026 14:48

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