Skip to content

Commit 707eed8

Browse files
committed
mcp(feat[middleware,server]): add ReadonlyRetryMiddleware scoped by TAG_READONLY
Adopts FastMCP's ``RetryMiddleware`` (``fastmcp/server/middleware/error_handling.py:136``) but bounded by the existing safety-tier annotation. The ``_server_cache`` already evicts dead Server instances on ``is_alive() == False``, but the *first* call after a tmux daemon crash still fails before the cache learns about it. A single retry on the freshly-cached Server closes that window without changing tool semantics. Critical safety constraint: only readonly tools are retried. Mutating tools like ``send_keys``, ``create_session``, ``split_window`` are NOT idempotent — re-running them on a transient socket error would silently double side effects, which is unacceptable. The wrapper ``ReadonlyRetryMiddleware`` reads ``tool.tags`` via ``context.fastmcp_context.fastmcp.get_tool(name)`` (same pattern ``SafetyMiddleware`` uses) and only delegates to the upstream retry when ``TAG_READONLY`` is present. Defaults are deliberately small: ``max_retries=1`` keeps audit log noise minimal (one extra attempt, not three); ``base_delay 0.1s`` / ``max_delay 1.0s`` matches the duration window of a transient libtmux socket hiccup. ``retry_exceptions= (libtmux.exc.LibTmuxException,)`` is narrow on purpose — the upstream default ``(ConnectionError, TimeoutError)`` does NOT match libtmux's wrapped subprocess errors and would be a silent no-op. Wired into the middleware stack between ``AuditMiddleware`` and ``SafetyMiddleware`` so retries are audited once each and tier-denied tools never reach retry. Updates the existing ``test_server_middleware_stack_order`` to lock in the new position. Per-pair regression test in the next commit asserts: readonly tool retries on ``LibTmuxException``, mutating tool does not retry, non-libtmux exception does not retry.
1 parent 97851ec commit 707eed8

3 files changed

Lines changed: 79 additions & 2 deletions

File tree

src/libtmux_mcp/middleware.py

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,10 @@
2626

2727
from fastmcp.exceptions import ToolError
2828
from fastmcp.server.middleware import Middleware, MiddlewareContext
29+
from fastmcp.server.middleware.error_handling import RetryMiddleware
2930
from fastmcp.server.middleware.response_limiting import ResponseLimitingMiddleware
3031
from fastmcp.tools.base import ToolResult
32+
from libtmux import exc as libtmux_exc
3133
from mcp.types import TextContent
3234

3335
from libtmux_mcp._utils import TAG_DESTRUCTIVE, TAG_MUTATING, TAG_READONLY
@@ -233,6 +235,69 @@ async def on_call_tool(
233235
#: opted out via ``max_lines=None``.
234236
DEFAULT_RESPONSE_LIMIT_BYTES = 50_000
235237

238+
239+
class ReadonlyRetryMiddleware(Middleware):
240+
"""Retry transient libtmux failures, but only for readonly tools.
241+
242+
Wraps fastmcp's :class:`fastmcp.server.middleware.error_handling.RetryMiddleware`
243+
so retries are bounded by the safety tier the tool is registered
244+
under. Mutating and destructive tools (``send_keys``,
245+
``create_session``, ``kill_server``, …) pass straight through —
246+
re-running them on a transient socket error would silently double
247+
side effects, which is unacceptable. Readonly tools
248+
(``list_sessions``, ``capture_pane``, ``snapshot_pane``, …) are
249+
safe to retry because they observe state without mutating it.
250+
251+
Default retry trigger is :class:`libtmux.exc.LibTmuxException` —
252+
libtmux wraps the subprocess failures we actually want to retry
253+
(socket EAGAIN, transient connect errors). The fastmcp default
254+
``(ConnectionError, TimeoutError)`` does NOT match these, so the
255+
upstream defaults would be a silent no-op.
256+
257+
Place this in the middleware stack **inside** ``AuditMiddleware``
258+
(so retried calls are audited once each) and **outside**
259+
``SafetyMiddleware`` (so tier-denied tools never reach retry).
260+
"""
261+
262+
def __init__(
263+
self,
264+
max_retries: int = 1,
265+
base_delay: float = 0.1,
266+
max_delay: float = 1.0,
267+
backoff_multiplier: float = 2.0,
268+
retry_exceptions: tuple[type[Exception], ...] = (libtmux_exc.LibTmuxException,),
269+
logger_: logging.Logger | None = None,
270+
) -> None:
271+
"""Configure the underlying retry policy.
272+
273+
Defaults are deliberately small. ``max_retries=1`` keeps audit
274+
log noise minimal (one retry per failed call, not three). The
275+
100 ms / 1 s backoff window matches the expected duration of a
276+
transient libtmux socket hiccup — a longer backoff would just
277+
delay a real failure without adding meaningful retry headroom.
278+
"""
279+
self._retry = RetryMiddleware(
280+
max_retries=max_retries,
281+
base_delay=base_delay,
282+
max_delay=max_delay,
283+
backoff_multiplier=backoff_multiplier,
284+
retry_exceptions=retry_exceptions,
285+
logger=logger_,
286+
)
287+
288+
async def on_call_tool(
289+
self,
290+
context: MiddlewareContext,
291+
call_next: t.Any,
292+
) -> t.Any:
293+
"""Delegate to the upstream retry only for tools tagged readonly."""
294+
if context.fastmcp_context:
295+
tool = await context.fastmcp_context.fastmcp.get_tool(context.message.name)
296+
if tool and TAG_READONLY in tool.tags:
297+
return await self._retry.on_request(context, call_next)
298+
return await call_next(context)
299+
300+
236301
#: Header prefixed to a truncated response. Intentionally matches the
237302
#: format used by the per-tool ``capture_pane`` truncation so clients
238303
#: see a consistent marker regardless of which layer fired.

src/libtmux_mcp/server.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
from libtmux_mcp.middleware import (
3030
DEFAULT_RESPONSE_LIMIT_BYTES,
3131
AuditMiddleware,
32+
ReadonlyRetryMiddleware,
3233
SafetyMiddleware,
3334
TailPreservingResponseLimitingMiddleware,
3435
)
@@ -221,7 +222,11 @@ def _gc_mcp_buffers(cache: t.Mapping[_ServerCacheKey, Server]) -> None:
221222
# Safety) are still logged with outcome=error. Without this
222223
# ordering, denied access attempts would silently bypass the
223224
# audit log — a security-observability gap.
224-
# 5. SafetyMiddleware — innermost gate (fail-closed). Denials
225+
# 5. ReadonlyRetryMiddleware — inside Audit so retries are
226+
# audited once each, outside Safety so tier-denied tools
227+
# never reach retry. Only readonly tools are retried;
228+
# mutating/destructive tools pass straight through.
229+
# 6. SafetyMiddleware — innermost gate (fail-closed). Denials
225230
# never reach the tool, but the audit record above captures
226231
# them for forensic review.
227232
middleware=[
@@ -232,6 +237,7 @@ def _gc_mcp_buffers(cache: t.Mapping[_ServerCacheKey, Server]) -> None:
232237
),
233238
ErrorHandlingMiddleware(transform_errors=True),
234239
AuditMiddleware(),
240+
ReadonlyRetryMiddleware(),
235241
SafetyMiddleware(max_tier=_safety_level),
236242
],
237243
on_duplicate="error",

tests/test_middleware.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,12 +326,17 @@ def test_server_middleware_stack_order() -> None:
326326
refactor that swaps Audit and Safety would degrade
327327
security observability without an obvious test failure, so pin
328328
the sequence explicitly.
329+
330+
ReadonlyRetryMiddleware sits between Audit and Safety so retried
331+
calls are audited once each (Audit wraps the retry loop) and
332+
tier-denied tools never reach retry (Safety stops them first).
329333
"""
330334
from fastmcp.server.middleware.error_handling import ErrorHandlingMiddleware
331335
from fastmcp.server.middleware.timing import TimingMiddleware
332336

333337
from libtmux_mcp.middleware import (
334338
AuditMiddleware,
339+
ReadonlyRetryMiddleware,
335340
SafetyMiddleware,
336341
TailPreservingResponseLimitingMiddleware,
337342
)
@@ -341,11 +346,12 @@ def test_server_middleware_stack_order() -> None:
341346
# FastMCP auto-appends an internal DereferenceRefsMiddleware at the
342347
# end of the stack; we care about the ordering of the middleware
343348
# *we* configured. Slice off the suffix before comparing.
344-
assert types[:5] == [
349+
assert types[:6] == [
345350
TimingMiddleware,
346351
TailPreservingResponseLimitingMiddleware,
347352
ErrorHandlingMiddleware,
348353
AuditMiddleware,
354+
ReadonlyRetryMiddleware,
349355
SafetyMiddleware,
350356
]
351357

0 commit comments

Comments
 (0)