BUG-HUNT: [resource] `MCPRefreshHook.cancel()` does not remove its listener from the adapter — hook stays active and schedules new timers after cancellation #6518

New issue

Open

opened 2026-04-09 21:14:25 +00:00 by HAL9000 · 0 comments

HAL9000 commented

2026-04-09 21:14:25 +00:00

Owner

Bug Report: [resource] — `MCPRefreshHook.cancel()` only cancels the pending timer but never removes the notification listener from the adapter — the hook remains active indefinitely

Severity Assessment

Impact: After hook.cancel(), the hook is expected to be inert. But because the listener self._on_notification is never removed from adapter._notification_listeners, the hook continues to receive notifications/tools/list_changed events and schedule new threading.Timer instances. This means: (1) cancelled hooks still trigger refresh_all(), (2) the hook object cannot be garbage-collected (the adapter holds a strong reference to self._on_notification which is a bound method of the hook), (3) users who replace a hook with a new one accumulate listeners — every previous hook still fires.
Likelihood: High — any code that creates a MCPRefreshHook, cancels it, and then creates a new one will end up with both the old and new hooks active.
Priority: Priority/Backlog

Location

File: src/cleveragents/mcp/refresh_hook.py
Function: MCPRefreshHook.cancel()
Lines: 125–133

Related: src/cleveragents/mcp/adapter.py

Function: MCPToolAdapter.add_notification_listener()
Lines: 357–373

Description

MCPRefreshHook.__init__ registers self._on_notification as a listener via adapter.add_notification_listener(self._on_notification). The cancel() method is documented as the cleanup path, but it only cancels the pending debounce timer:

def cancel(self) -> None:
    """Cancel any pending debounced refresh."""
    with self._lock:
        if self._timer is not None:
            self._timer.cancel()
            self._timer = None

It does NOT call any adapter.remove_notification_listener() method — and no such method exists on MCPToolAdapter.

Consequence: After cancel(), if the adapter dispatches another notifications/tools/list_changed:

_on_notification() is called (listener still registered)
A new threading.Timer is created and started
_do_refresh() fires again

The hook is not actually cancelled — it is only momentarily quiesced until the next notification arrives.

Evidence

# MCPRefreshHook.__init__ — registers listener (refresh_hook.py line 76)
adapter.add_notification_listener(self._on_notification)

# MCPRefreshHook.cancel() — does NOT unregister (refresh_hook.py lines 125–133)
def cancel(self) -> None:
    with self._lock:
        if self._timer is not None:
            self._timer.cancel()
            self._timer = None
            logger.debug(...)
    # Missing: adapter.remove_notification_listener(self._on_notification)

# MCPToolAdapter.add_notification_listener — appends, no removal API (adapter.py lines 357–373)
def add_notification_listener(
    self,
    callback: Callable[[str, dict[str, Any]], None],
) -> None:
    with self._lock:
        self._notification_listeners.append(callback)
    # No remove_notification_listener() method exists

Expected Behavior

cancel() should fully deactivate the hook:

Cancel any pending debounce timer (already done)
Remove self._on_notification from the adapter's listener list

MCPToolAdapter should expose a remove_notification_listener() method to support this.

Actual Behavior

After cancel(), the hook continues to receive notifications and schedule new timers. The hook object is retained by the adapter indefinitely, preventing garbage collection.

Suggested Fix

1. Add remove_notification_listener to MCPToolAdapter (adapter.py):

def remove_notification_listener(
    self,
    callback: Callable[[str, dict[str, Any]], None],
) -> None:
    """Remove a previously registered notification listener."""
    with self._lock:
        try:
            self._notification_listeners.remove(callback)
        except ValueError:
            pass  # Already removed — idempotent

2. Call it in MCPRefreshHook.cancel() (refresh_hook.py):

def cancel(self) -> None:
    with self._lock:
        if self._timer is not None:
            self._timer.cancel()
            self._timer = None
    self._adapter.remove_notification_listener(self._on_notification)

TDD Note

After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: @tdd_issue, @tdd_issue_, and @tdd_expected_fail to prove the bug exists before fixing it.

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: bug-hunter

## Bug Report: [resource] — `MCPRefreshHook.cancel()` only cancels the pending timer but never removes the notification listener from the adapter — the hook remains active indefinitely ### Severity Assessment - **Impact**: After `hook.cancel()`, the hook is expected to be inert. But because the listener `self._on_notification` is never removed from `adapter._notification_listeners`, the hook continues to receive `notifications/tools/list_changed` events and schedule new `threading.Timer` instances. This means: (1) cancelled hooks still trigger `refresh_all()`, (2) the hook object cannot be garbage-collected (the adapter holds a strong reference to `self._on_notification` which is a bound method of the hook), (3) users who replace a hook with a new one accumulate listeners — every previous hook still fires. - **Likelihood**: High — any code that creates a `MCPRefreshHook`, cancels it, and then creates a new one will end up with both the old and new hooks active. - **Priority**: Priority/Backlog ### Location - **File**: `src/cleveragents/mcp/refresh_hook.py` - **Function**: `MCPRefreshHook.cancel()` - **Lines**: 125–133 Related: `src/cleveragents/mcp/adapter.py` - **Function**: `MCPToolAdapter.add_notification_listener()` - **Lines**: 357–373 ### Description `MCPRefreshHook.__init__` registers `self._on_notification` as a listener via `adapter.add_notification_listener(self._on_notification)`. The `cancel()` method is documented as the cleanup path, but it only cancels the pending debounce timer: ```python def cancel(self) -> None: """Cancel any pending debounced refresh.""" with self._lock: if self._timer is not None: self._timer.cancel() self._timer = None ``` It does NOT call any `adapter.remove_notification_listener()` method — and no such method exists on `MCPToolAdapter`. Consequence: After `cancel()`, if the adapter dispatches another `notifications/tools/list_changed`: 1. `_on_notification()` is called (listener still registered) 2. A new `threading.Timer` is created and started 3. `_do_refresh()` fires again The hook is not actually cancelled — it is only momentarily quiesced until the next notification arrives. ### Evidence ```python # MCPRefreshHook.__init__ — registers listener (refresh_hook.py line 76) adapter.add_notification_listener(self._on_notification) # MCPRefreshHook.cancel() — does NOT unregister (refresh_hook.py lines 125–133) def cancel(self) -> None: with self._lock: if self._timer is not None: self._timer.cancel() self._timer = None logger.debug(...) # Missing: adapter.remove_notification_listener(self._on_notification) # MCPToolAdapter.add_notification_listener — appends, no removal API (adapter.py lines 357–373) def add_notification_listener( self, callback: Callable[[str, dict[str, Any]], None], ) -> None: with self._lock: self._notification_listeners.append(callback) # No remove_notification_listener() method exists ``` ### Expected Behavior `cancel()` should fully deactivate the hook: 1. Cancel any pending debounce timer (already done) 2. Remove `self._on_notification` from the adapter's listener list `MCPToolAdapter` should expose a `remove_notification_listener()` method to support this. ### Actual Behavior After `cancel()`, the hook continues to receive notifications and schedule new timers. The hook object is retained by the adapter indefinitely, preventing garbage collection. ### Suggested Fix **1. Add `remove_notification_listener` to `MCPToolAdapter` (adapter.py):** ```python def remove_notification_listener( self, callback: Callable[[str, dict[str, Any]], None], ) -> None: """Remove a previously registered notification listener.""" with self._lock: try: self._notification_listeners.remove(callback) except ValueError: pass # Already removed — idempotent ``` **2. Call it in `MCPRefreshHook.cancel()` (refresh_hook.py):** ```python def cancel(self) -> None: with self._lock: if self._timer is not None: self._timer.cancel() self._timer = None self._adapter.remove_notification_listener(self._on_notification) ``` ### Category resource ### TDD Note After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: @tdd_issue, @tdd_issue_<this-issue-number>, and @tdd_expected_fail to prove the bug exists before fixing it. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: bug-hunter

HAL9000 added the

labels

2026-04-09 21:26:21 +00:00

HAL9000 added this to the v3.2.0 milestone

2026-04-09 21:27:54 +00:00

HAL9000 referenced this issue