`get_phabricator_revision` MCP tool fails with "unexpected keyword argument 'revision_id'" despite valid class signature

[groovecoder]

> @moz:phabricator://revision/D270778

⏺ moz - get_phabricator_revision (MCP)(revision_id: 270778)
  ⎿  Error: Error calling tool 'get_phabricator_revision': PhabricatorPatch.__init__() got an unexpected keyword argument 'revision_id'

⏺ Let me try using the resource format instead:

⏺ readMcpResource(Read resource "phabricator://revision/D270778" from server "moz")
  ⎿  Error: MCP error 0: Error reading resource from template 'phabricator://revision/D270778': PhabricatorPatch.__init__() got an unexpected keyword argument 'revision_id'

⏺ I'm encountering an error with the MCP Phabricator tool - it appears there's a bug in the MCP server implementation. Both the tool and resource methods are failing with: PhabricatorPatch.__init__() got an unexpected keyword argument
   'revision_id'

Description

Summary

The MCP tool get_phabricator_revision is failing with a TypeError: unexpected keyword argument 'revision_id' when calling PhabricatorPatch(revision_id=revision_id), even though the PhabricatorPatch class constructor correctly accepts this parameter.

Error Message

TypeError: __init__() got an unexpected keyword argument 'revision_id'

Environment

• File: mcp/src/bugbug_mcp/server.py:254
• Tool Function: get_phabricator_revision(revision_id: int)
• Class: bugbug.tools.core.platforms.phabricator.PhabricatorPatch
• Python Version: 3.12
• FastMCP Version: >=2.12.0

Code Location

Tool Definition (mcp/src/bugbug_mcp/server.py:251-254):

@mcp.tool()
def get_phabricator_revision(revision_id: int) -> str:
    """Retrieve a revision from Phabricator alongside its comments."""
    return PhabricatorPatch(revision_id=revision_id).to_md()

Class Constructor (bugbug/tools/core/platforms/phabricator.py:88-101):

class PhabricatorPatch(Patch):
    def __init__(
        self,
        diff_id: Optional[str | int] = None,
        revision_phid: Optional[str] = None,
        revision_id: Optional[int] = None,  # ← Parameter exists!
    ) -> None:
        assert diff_id or revision_phid or revision_id, (
            "You must provide at least one of diff_id, revision_phid, or revision_id"
        )
        # ...

Investigation Results

✅ Tests That Pass

1. Direct instantiation: PhabricatorPatch(revision_id=270778) works correctly
2. Class signature: Confirmed revision_id is a valid parameter
3. FastMCP registration: Tool registration succeeds with correct parameter schema
4. Function calls: Calling the underlying function directly works

⚠️ Interesting Findings

Name Collision: There are two PhabricatorPatch classes in the dependency tree:

• bugbug.tools.core.platforms.phabricator.PhabricatorPatch - ACCEPTS revision_id ✓
• libmozdata.phabricator.PhabricatorPatch - DOES NOT ACCEPT revision_id ✗

The server.py correctly imports from bugbug.tools.core.platforms.phabricator, so this shouldn't cause the issue unless there's an import caching problem.

Inconsistent Usage Pattern: The same file (server.py) uses different patterns:

• Line 138 (works): PhabricatorPatch(revisions[0]["fields"]["diffID"]) - positional arg with diff_id
• Line 248 (should fail?): PhabricatorPatch(revision_id=revision_id) - MCP resource handler
• Line 254 (fails): PhabricatorPatch(revision_id=revision_id) - MCP tool

Reproduction Steps

1. Set up the bugbug MCP server with proper credentials
2. Call the MCP tool get_phabricator_revision with a valid revision_id (e.g., 270778)
3. Observe the TypeError

Diagnostic Script

A comprehensive diagnostic script is available at mcp/debug_mcp_tool.py that tests:

• Direct class instantiation
• Signature verification
• Module import locations
• FastMCP tool registration
• Multiple PhabricatorPatch class detection

Run with: uv run python mcp/debug_mcp_tool.py

All tests pass, confirming the Python code is correct.

Suspected Root Causes

Based on the investigation, the issue is likely one of:

1. Stale MCP Server (most likely): The MCP server process may have loaded an old version of the code or cached imports. The server needs a complete restart.

2. Import Caching: The wrong PhabricatorPatch class (from libmozdata) may be imported in the MCP server context, despite the correct import statement.

3. FastMCP Parameter Passing: There may be an edge case in how FastMCP handles keyword arguments that causes parameter validation to fail, though this seems unlikely given the tests.

4. MCP Protocol Layer: The error may be occurring in the MCP HTTP/protocol layer rather than in the Python function execution.

Questions for the Team

1. Is the MCP server being properly restarted after code changes?
2. Are there any known issues with FastMCP and keyword-only parameters?
3. Should we verify the MCP resource handler at line 248 (which uses the same pattern) is also failing?
4. Could there be module import order issues causing the wrong PhabricatorPatch to be used?

Proposed Solutions

1. Immediate: Restart the MCP server completely to pick up the latest code
2. Debugging: Add explicit logging in the tool function to verify which class is being used:

   @mcp.tool()
   def get_phabricator_revision(revision_id: int) -> str:
       import inspect
       logger.info(f"PhabricatorPatch location: {inspect.getfile(PhabricatorPatch)}")
       logger.info(f"PhabricatorPatch signature: {inspect.signature(PhabricatorPatch.__init__)}")
       return PhabricatorPatch(revision_id=revision_id).to_md()

3. Workaround: Use the working pattern from line 138 if needed:

   revisions = phabricator.get(rev_ids=[revision_id])
   patch = PhabricatorPatch(revisions[0]["fields"]["diffID"])

Additional Context

• The MCP resource handler handle_revision_view_resource (line 246-248) uses the exact same pattern and should theoretically also fail if this is a code issue
• No git changes or package version mismatches were detected
• The bugbug package is correctly loaded from the local development path

---

Created by: Claude Code debugging session
Date: 2025-10-30

[suhaibmujahid]

@groovecoder I deployed a fix. Could you please confirm it is working for you now?

[groovecoder]

I don't see the error anymore, but I also can't tell if it actually invoked the tool call this time. So ... maybe it's working?

[suhaibmujahid]

I tested it on my end, and it is working. I wanted to confirm.