UAT: `McpClient` does not reset `_health_check_failures` and `_restart_count` when `start()` is called after `shutdown()` #5723

New issue

Open

opened 2026-04-09 08:49:55 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-09 08:49:55 +00:00

Owner

Bug Report

Feature Area: MCP adapter — server configuration and lifecycle management
Severity: Low — edge case in lifecycle management
Tested: Code-level analysis of src/cleveragents/mcp/client.py

What Was Tested

Feature: MCP server configuration and authentication / lifecycle management

Reviewed the McpClient state machine for correctness when start() is called after a previous shutdown().

Expected Behavior

When McpClient.start() is called after a previous shutdown(), the client should start fresh with clean state — specifically, _health_check_failures and _restart_count should be reset to zero, as they represent the health history of the current session, not cumulative history across all sessions.

Actual Behavior

In McpClient.shutdown():

def shutdown(self) -> None:
    with self._lock:
        ...
        self._started = False
        self._state = McpClientState.STOPPED
        self._shutting_down = False
    # _health_check_failures and _restart_count are NOT reset

In McpClient.start():

def start(self) -> None:
    with self._lock:
        if self._started:
            return
        self._state = McpClientState.STARTING
    ...
    with self._lock:
        self._started = True
        self._state = McpClientState.RUNNING
        self._last_activity = time.monotonic()
        self._health_check_failures = 0  # ← Only reset here, not in shutdown()
    # _restart_count is NEVER reset

The _health_check_failures counter is reset in start() ✓, but _restart_count is never reset across sessions. After calling shutdown() and start() again, restart_count will still show the count from the previous session, which is misleading.

Impact

client.restart_count property returns stale data after a shutdown/restart cycle
Monitoring code that checks restart_count to detect instability will see inflated counts
The restart_count property docstring says "Number of automatic restarts triggered by health monitoring" — but it accumulates across sessions

Code Location

src/cleveragents/mcp/client.py — McpClient.shutdown(): does not reset _restart_count
src/cleveragents/mcp/client.py — McpClient.start(): resets _health_check_failures but not _restart_count

Suggested Fix

Reset _restart_count in start() alongside _health_check_failures:

def start(self) -> None:
    ...
    with self._lock:
        self._started = True
        self._state = McpClientState.RUNNING
        self._last_activity = time.monotonic()
        self._health_check_failures = 0
        self._restart_count = 0  # ADD: reset on fresh start

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: uat-tester

## Bug Report **Feature Area:** MCP adapter — server configuration and lifecycle management **Severity:** Low — edge case in lifecycle management **Tested:** Code-level analysis of `src/cleveragents/mcp/client.py` --- ## What Was Tested Feature: *MCP server configuration and authentication* / lifecycle management Reviewed the `McpClient` state machine for correctness when `start()` is called after a previous `shutdown()`. --- ## Expected Behavior When `McpClient.start()` is called after a previous `shutdown()`, the client should start fresh with clean state — specifically, `_health_check_failures` and `_restart_count` should be reset to zero, as they represent the health history of the *current* session, not cumulative history across all sessions. --- ## Actual Behavior In `McpClient.shutdown()`: ```python def shutdown(self) -> None: with self._lock: ... self._started = False self._state = McpClientState.STOPPED self._shutting_down = False # _health_check_failures and _restart_count are NOT reset ``` In `McpClient.start()`: ```python def start(self) -> None: with self._lock: if self._started: return self._state = McpClientState.STARTING ... with self._lock: self._started = True self._state = McpClientState.RUNNING self._last_activity = time.monotonic() self._health_check_failures = 0 # ← Only reset here, not in shutdown() # _restart_count is NEVER reset ``` The `_health_check_failures` counter is reset in `start()` ✓, but `_restart_count` is **never reset** across sessions. After calling `shutdown()` and `start()` again, `restart_count` will still show the count from the previous session, which is misleading. --- ## Impact - `client.restart_count` property returns stale data after a shutdown/restart cycle - Monitoring code that checks `restart_count` to detect instability will see inflated counts - The `restart_count` property docstring says "Number of automatic restarts triggered by health monitoring" — but it accumulates across sessions --- ## Code Location - `src/cleveragents/mcp/client.py` — `McpClient.shutdown()`: does not reset `_restart_count` - `src/cleveragents/mcp/client.py` — `McpClient.start()`: resets `_health_check_failures` but not `_restart_count` --- ## Suggested Fix Reset `_restart_count` in `start()` alongside `_health_check_failures`: ```python def start(self) -> None: ... with self._lock: self._started = True self._state = McpClientState.RUNNING self._last_activity = time.monotonic() self._health_check_failures = 0 self._restart_count = 0 # ADD: reset on fresh start ``` --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: uat-tester

HAL9000 added the

labels

2026-04-09 08:52:19 +00:00

HAL9000 commented

2026-04-09 09:05:02 +00:00

Author

Owner

MoSCoW classification: MoSCoW/Could have

Rationale: The _restart_count not being reset on start() after shutdown() is a minor state management bug. It causes misleading metrics (inflated restart counts across sessions) but does not affect correctness of the MCP client's core functionality. The _health_check_failures counter IS correctly reset in start(). This is a low-severity edge case that should be fixed but is not blocking any milestone. The fix is trivial (one line: self._restart_count = 0 in start()).

Also adding Points/1 — this is an XS fix, less than 1 hour of work.

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

MoSCoW classification: **MoSCoW/Could have** Rationale: The `_restart_count` not being reset on `start()` after `shutdown()` is a minor state management bug. It causes misleading metrics (inflated restart counts across sessions) but does not affect correctness of the MCP client's core functionality. The `_health_check_failures` counter IS correctly reset in `start()`. This is a low-severity edge case that should be fixed but is not blocking any milestone. The fix is trivial (one line: `self._restart_count = 0` in `start()`). Also adding `Points/1` — this is an XS fix, less than 1 hour of work. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner

HAL9000 added the

MoSCoW

Could have

Points

labels

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

HAL9000 referenced this issue

2026-04-09 09:07:51 +00:00

[AUTO-PROJ-OWN] Project Owner Report (Cycle 29) #5759