Remove: all other tool groups, and just have read and write tools as … - #250
Merged
Conversation
…the top tool collections
VirajAgarwal-couchbase
marked this pull request as ready for review
August 31, 2026 09:49
VirajAgarwal-couchbase
requested review from
AayushTyagi1
and
a lite review from Copilot
August 31, 2026 09:49
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR simplifies Couchbase MCP tool registration by consolidating the existing write-tool family lists into a single WRITE_TOOLS collection, leaving only two top-level groupings (READ_ONLY_TOOLS and WRITE_TOOLS) used across registration and tests.
Changes:
- Replaces
KV_WRITE_TOOLS/COLLECTION_WRITE_TOOLS/INDEX_WRITE_TOOLSwith a unifiedWRITE_TOOLSlist and updatesALL_TOOLS,get_tools(), and exports accordingly. - Updates tool registration to derive OAuth
write_tool_namesdirectly fromWRITE_TOOLS. - Updates read-only-mode unit tests to validate the consolidated write tool grouping (counts and name sets unchanged).
File summaries
| File | Description |
|---|---|
tests/unit/test_read_only_mode.py |
Updates tests to assert behavior/counts against consolidated WRITE_TOOLS. |
src/cb_mcp/tools/__init__.py |
Consolidates write tool lists into WRITE_TOOLS and updates ALL_TOOLS, get_tools(), and __all__. |
src/cb_mcp/tool_registration.py |
Builds OAuth write-scope name set from WRITE_TOOLS instead of concatenating multiple lists. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
nithishr
approved these changes
Sep 1, 2026
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
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.
Remove all other tool groups, and just have read and write tools as the top tool collections
What does this change do?
Consolidates the tool registration structure in
src/cb_mcp/tools/__init__.pydown to exactlytwo top-level collections —
READ_ONLY_TOOLSandWRITE_TOOLS— instead of one read list plusthree separate write-tool family lists (
KV_WRITE_TOOLS,COLLECTION_WRITE_TOOLS,INDEX_WRITE_TOOLS). The three write families are merged into a singleWRITE_TOOLSlist (12tools), with
ALL_TOOLS,get_tools(), and__all__updated accordingly.tool_registration.pynow builds its OAuth
write_tool_namesset directly fromWRITE_TOOLSinstead of concatenatingthree lists. Tests in
tests/unit/test_read_only_mode.pywere updated to match (singleWRITE_TOOL_NAMESconstant,len(WRITE_TOOLS) == 12assertion) — total tool counts (22 read-only/ 34 all) are unchanged.
No tool is added, removed, renamed, or moved between read and write. This is purely an
internal registration/orchestration simplification.
Why is this change needed?
The three-way write-tool split (KV / collection management / index) didn't reflect any real
branching in the code —
prepare_tools_for_registration()immediately flattened all three backinto a single
write_tool_namesset for scope enforcement, andscope_enforcement.py'srequired_scopes_for_toolonly ever checks flat set membership, with no per-family logicanywhere. The split was pure indirection left over from how tool families were added
incrementally; collapsing it to a single
WRITE_TOOLSlist matches how the code actually usesit and removes three names to keep in sync every time a new write tool is added.
Evidence of Testing
Automated tests — commands run and results summary:
env-cmd -f .env -- uv run pytest tests/
→ 698 passed, 13 skipped in 918.42s (0:15:18)
Environments tested (both are required):
Manual verification: N/A — this change is internal to tool registration/orchestration (no
tool names, parameters, or behavior changed); the full test suite above (unit + integration +
accuracy, run against both self-managed and Capella) is the verification path for a change like
this. No new manual MCP-client exercise was performed since no user-facing tool surface changed.
Compatibility Considerations
None. No tool names, parameters, return shapes, CLI flags, or environment variables changed.
READ_ONLY_MODEbehavior, OAuth scope requirements (couchbase-mcp:read/couchbase-mcp:write),and tool annotations are all unchanged — only the internal Python collection names
(
KV_WRITE_TOOLS/COLLECTION_WRITE_TOOLS/INDEX_WRITE_TOOLS→WRITE_TOOLS) changed. No impacton Capella vs. self-managed behavior or
cb_mcp.core/ managed MCP interfaces.Checklist
cb_mcp.corecontracts / managed MCP interfaces (or discussed first)TestGetToolsTruthTable/TestToolCountsKV_WRITE_TOOLS/COLLECTION_WRITE_TOOLS/INDEX_WRITE_TOOLSnames)