Skip to content

Add read-only mode for MCP tools in shared repositories#423

Merged
jfrench9 merged 2 commits into
mainfrom
chore/remove-unused-mcp-tools-shared-repo
Mar 3, 2026
Merged

Add read-only mode for MCP tools in shared repositories#423
jfrench9 merged 2 commits into
mainfrom
chore/remove-unused-mcp-tools-shared-repo

Conversation

@jfrench9
Copy link
Copy Markdown
Member

@jfrench9 jfrench9 commented Mar 3, 2026

Summary

Introduces read-only mode support for GraphMCPTools to prevent workspace mutation operations when accessing shared repositories. The MCP handler now evaluates whether a repository is shared and propagates that status to the tools manager, which conditionally restricts available tools accordingly.

Key Accomplishments

  • Read-only mode in GraphMCPTools: Enhanced the tools manager to accept and enforce a read_only flag. When enabled, workspace mutation tools (e.g., file creation, editing, deletion) are excluded from the available tool set, ensuring shared repositories remain unmodified.
  • Shared repository detection in MCPHandler: Updated the MCP handler to determine whether the target repository is shared, passing the read-only status downstream to the tools initialization pipeline.
  • Improved initialization logging: Updated logging during tool initialization to reflect read-only status, improving observability and debugging for different access modes.
  • Cleaner tool registration logic: Refactored tool initialization to conditionally compose the tool set based on access permissions rather than always registering all tools.

Breaking Changes

None. This is an additive behavioral change — repositories that are not shared continue to function with full read-write tool access as before.

Testing Notes

  • Verify that MCP tool sessions against shared repositories correctly expose only read-only tools (no file mutation capabilities).
  • Verify that MCP tool sessions against non-shared (owned) repositories continue to expose the full set of tools including workspace mutation operations.
  • Confirm initialization logs accurately reflect whether the session was started in read-only or read-write mode.
  • Test edge cases around repository ownership/shared status transitions to ensure the read-only flag is evaluated fresh per session.

Infrastructure Considerations

  • No new dependencies or infrastructure changes required.
  • The read-only enforcement is applied at the tool registration layer, meaning downstream MCP clients will simply not see mutation tools — no additional server-side authorization checks are introduced at this layer. Ensure this aligns with the broader security model for shared repository access.

🤖 Generated with Claude Code

Branch Info:

  • Source: chore/remove-unused-mcp-tools-shared-repo
  • Target: main
  • Type: chore

Co-Authored-By: Claude noreply@anthropic.com

…e mutation tools and updating initialization logging. Adjust MCPHandler to determine read-only status based on shared repository checks.
@jfrench9
Copy link
Copy Markdown
Member Author

jfrench9 commented Mar 3, 2026

@claude please review this PR

@claude
Copy link
Copy Markdown

claude Bot commented Mar 3, 2026

Claude finished @jfrench9's task in 1m 35s —— View job


PR Review: Add read-only mode for MCP tools in shared repositories

  • Gather context
  • Read changed files
  • Analyze diff
  • Post review feedback

Overall this is a clean, well-scoped change. The core logic is correct and the implementation is straightforward. A few observations below.

What's Good

Correct layering of read-only enforcement. The read_only flag is applied in __init__ at tool instantiation time, which is the right place — tools that shouldn't exist simply aren't created. The None-check pattern in call_tool already provided defense-in-depth, so this slots in naturally.

Implicit bug fix in _get_workspace_tool_definitions. The old guard:

# manager.py (old)
if self.create_workspace_tool is None:
    return []

would return an empty list even when MCP_WORKSPACE_ENABLED=True with a read-only graph, silently hiding navigation tools (list-workspaces, switch-workspace). The new per-tool None checks fix this correctly. Worth calling out in the PR description as a fix, not just a refactor.

Separation of concerns is maintained. The MCPHandler determines the read_only status and passes it down; GraphMCPTools enforces it. Neither leaks into the other's domain.


Issues

1. build_fact_grid_tool not gated by read_only
manager.py:172–176

self.build_fact_grid_tool = None
if env.FACT_GRID_ENABLED:
    from .data_tools import BuildFactGridTool
    self.build_fact_grid_tool = BuildFactGridTool(graph_client)

The name "build" suggests a write/compute operation. If BuildFactGridTool performs any graph mutations, it should also be blocked in read-only mode. Worth auditing before merge.

Fix this →


2. Misleading error messages in call_tool for read-only callers
manager.py:450–464, manager.py:487–500

When create-workspace, delete-workspace, or write-graph-cypher are called on a read-only session, the error says:

"create-workspace tool is not available. Set MCP_WORKSPACE_ENABLED=true to enable this feature."

This is wrong — the real reason is read_only=True. While these tools won't appear in get_tool_definitions_as_dict() (so well-behaved clients won't call them), a direct call_tool invocation would get a confusing message. Consider differentiating the reason:

if self.create_workspace_tool is None:
    if self.read_only:
        raise ValueError("create-workspace is not available: this graph is read-only.")
    raise ValueError("create-workspace tool is not available. Set MCP_WORKSPACE_ENABLED=true.")

Fix this →


3. Redundant is_shared_repository_or_subgraph call in get_tools
handlers.py:153–155

After await self._ensure_initialized(), self.mcp_tools.read_only already reflects the shared-repo check computed in _init_async. The second call to is_shared_repository_or_subgraph in get_tools is redundant:

# handlers.py:153 (after _ensure_initialized)
is_shared_repo = is_shared_repository_or_subgraph(self.graph_id)
# Could be:
is_shared_repo = self.mcp_tools.read_only

Minor efficiency nit — is_shared_repository_or_subgraph is likely fast (in-memory check per the manifest lookup), but using the already-computed value is more idiomatic.


Nit

The read_only flag in _get_workspace_tool_definitions's docstring (manager.py:241) is well-updated, but the _get_memory_tool_definitions docstring (manager.py:254–260) could also note that memory tools are excluded for read-only graphs, for consistency.


Verdict

Approve with minor suggestions. The core behavior is correct, the bug fix in _get_workspace_tool_definitions is a nice bonus, and the change is properly non-breaking. The build_fact_grid_tool gap (#1) is worth checking before merge; the rest are polish items.
|

…duced a new method to generate context-aware error messages, improving code readability and maintainability. Updated existing error messages to utilize this new method for consistency.
@jfrench9 jfrench9 merged commit 611ac30 into main Mar 3, 2026
8 of 9 checks passed
@jfrench9 jfrench9 deleted the chore/remove-unused-mcp-tools-shared-repo branch March 10, 2026 04:20
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.

1 participant