UAT: `MCPTransport.close()` is a silent no-op while `connect()` and `call()` raise `NotImplementedError` — inconsistent abstract interface forces subclasses to silently skip cleanup #2967

New issue

Open

opened 2026-04-05 02:58:39 +00:00 by freemo · 1 comment

freemo commented

2026-04-05 02:58:39 +00:00

Owner

Metadata

Branch: fix/v3.8-mcp-transport-close-not-implemented
Commit Message: fix(mcp): raise NotImplementedError in MCPTransport.close() to enforce consistent abstract interface
Milestone: v3.8.0
Parent Epic: #399

Background and Context

The MCPTransport abstract base class in src/cleveragents/mcp/adapter.py defines the lifecycle interface for MCP server connections — handshake (connect()), tool invocation (call()), and clean shutdown (close()). Per the specification, MCP acts as a bridge that manages the full lifecycle of an MCP server connection, including clean shutdown. The connect() and call() methods both raise NotImplementedError to force subclasses to provide real implementations, but close() is a silent no-op, creating an inconsistent abstract interface.

Current Behavior

In src/cleveragents/mcp/adapter.py, MCPTransport.close() is a silent no-op:

class MCPTransport:
    def connect(self, config: MCPServerConfig) -> dict[str, Any]:
        """Perform handshake and return server capabilities."""
        raise NotImplementedError  # ← forces subclasses to implement

    def call(self, method: str, params: dict[str, Any]) -> dict[str, Any]:
        """Send a JSON-RPC method call and return the result."""
        _ = method, params
        raise NotImplementedError  # ← forces subclasses to implement

    def close(self) -> None:
        """Shut down the transport connection."""
        # ← SILENT NO-OP: does nothing, returns None

This causes the following problems:

A subclass that forgets to implement close() will silently skip cleanup — no error, no warning.
MCPToolAdapter.__init__() uses MCPTransport() directly as a default: self._transport = transport or MCPTransport(). When MCPToolAdapter.disconnect() calls self._transport.close(), it silently succeeds even though no real cleanup occurred.
This is inconsistent with connect() and call(), which both raise NotImplementedError.
The docstring says "Subclass to provide real stdio or HTTP transports" but the inconsistency makes it easy to accidentally ship a subclass with no cleanup.
The existing test in features/mcp_adapter_coverage_r3.feature covers connect() and call() raising NotImplementedError, but there is no test for close().

Expected Behavior

MCPTransport.close() should raise NotImplementedError (consistent with connect() and call()), OR be explicitly documented as an intentional no-op for transports that require no cleanup — with the docstring updated accordingly and a corresponding test added. The preferred fix per the spec's lifecycle model is to raise NotImplementedError to enforce that all subclasses implement proper cleanup.

Subtasks

Investigate src/cleveragents/mcp/adapter.py — confirm exact line of MCPTransport.close() and review MCPToolAdapter usage
Fix MCPTransport.close() to raise NotImplementedError (consistent with connect() and call())
Update MCPToolAdapter.__init__() default transport handling if needed (since MCPTransport() can no longer be used as a no-op default)
Tests (Behave): Add scenario to features/mcp_adapter_coverage_r3.feature asserting MCPTransport.close() raises NotImplementedError
Verify coverage >= 97% via nox -s coverage_report
Run nox (all default sessions), fix any errors

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly (fix(mcp): raise NotImplementedError in MCPTransport.close() to enforce consistent abstract interface), followed by a blank line, then additional lines providing relevant details about the implementation.
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly (fix/v3.8-mcp-transport-close-not-implemented).
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.
All nox stages pass.
Coverage >= 97%.

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/v3.8-mcp-transport-close-not-implemented` - **Commit Message**: `fix(mcp): raise NotImplementedError in MCPTransport.close() to enforce consistent abstract interface` - **Milestone**: v3.8.0 - **Parent Epic**: #399 ## Background and Context The `MCPTransport` abstract base class in `src/cleveragents/mcp/adapter.py` defines the lifecycle interface for MCP server connections — handshake (`connect()`), tool invocation (`call()`), and clean shutdown (`close()`). Per the specification, MCP acts as a bridge that manages the full lifecycle of an MCP server connection, including clean shutdown. The `connect()` and `call()` methods both raise `NotImplementedError` to force subclasses to provide real implementations, but `close()` is a silent no-op, creating an inconsistent abstract interface. ## Current Behavior In `src/cleveragents/mcp/adapter.py`, `MCPTransport.close()` is a silent no-op: ```python class MCPTransport: def connect(self, config: MCPServerConfig) -> dict[str, Any]: """Perform handshake and return server capabilities.""" raise NotImplementedError # ← forces subclasses to implement def call(self, method: str, params: dict[str, Any]) -> dict[str, Any]: """Send a JSON-RPC method call and return the result.""" _ = method, params raise NotImplementedError # ← forces subclasses to implement def close(self) -> None: """Shut down the transport connection.""" # ← SILENT NO-OP: does nothing, returns None ``` This causes the following problems: 1. A subclass that forgets to implement `close()` will silently skip cleanup — no error, no warning. 2. `MCPToolAdapter.__init__()` uses `MCPTransport()` directly as a default: `self._transport = transport or MCPTransport()`. When `MCPToolAdapter.disconnect()` calls `self._transport.close()`, it silently succeeds even though no real cleanup occurred. 3. This is inconsistent with `connect()` and `call()`, which both raise `NotImplementedError`. 4. The docstring says "Subclass to provide real stdio or HTTP transports" but the inconsistency makes it easy to accidentally ship a subclass with no cleanup. 5. The existing test in `features/mcp_adapter_coverage_r3.feature` covers `connect()` and `call()` raising `NotImplementedError`, but there is no test for `close()`. ## Expected Behavior `MCPTransport.close()` should raise `NotImplementedError` (consistent with `connect()` and `call()`), OR be explicitly documented as an intentional no-op for transports that require no cleanup — with the docstring updated accordingly and a corresponding test added. The preferred fix per the spec's lifecycle model is to raise `NotImplementedError` to enforce that all subclasses implement proper cleanup. ## Subtasks - [ ] Investigate `src/cleveragents/mcp/adapter.py` — confirm exact line of `MCPTransport.close()` and review `MCPToolAdapter` usage - [ ] Fix `MCPTransport.close()` to raise `NotImplementedError` (consistent with `connect()` and `call()`) - [ ] Update `MCPToolAdapter.__init__()` default transport handling if needed (since `MCPTransport()` can no longer be used as a no-op default) - [ ] Tests (Behave): Add scenario to `features/mcp_adapter_coverage_r3.feature` asserting `MCPTransport.close()` raises `NotImplementedError` - [ ] Verify coverage >= 97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions), fix any errors ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly (`fix(mcp): raise NotImplementedError in MCPTransport.close() to enforce consistent abstract interface`), followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly (`fix/v3.8-mcp-transport-close-not-implemented`). - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. - All nox stages pass. - Coverage >= 97%. --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator

freemo added this to the v3.8.0 milestone

2026-04-05 02:58:44 +00:00

freemo added a new dependency

2026-04-05 02:59:07 +00:00

#399 Epic: Post-MVP Server & Clients

freemo commented

2026-04-05 03:00:23 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: Low (confirmed)
MoSCoW: Could Have — no-op close method is low impact

Valid finding verified during batch triage.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: Low (confirmed) - **MoSCoW**: Could Have — no-op close method is low impact Valid finding verified during batch triage. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner

freemo added the

MoSCoW

Could have

label

2026-04-05 03:02:23 +00:00

freemo referenced this issue

2026-04-05 03:03:36 +00:00

[CA-AUTO] Project Owner — project-owner-1 — Session Tracker #2793

freemo referenced this issue

2026-04-05 06:15:22 +00:00

[CA-AUTO] UAT Pool Supervisor — v3.2.0–v3.7.0 — Session Tracker #3094

freemo referenced this issue