Do not retry a stale id; say how to fix an ambiguous target

[tony]

Draft — do not merge with the git pin in place. Validates tmux-python/libtmux#718 downstream before it ships. Drop the [tool.uv.sources] stanza and bump the libtmux floor once #718 is released.

Fixes #97 (the ambiguous-lookup half).

Why

libtmux#718 moves MultipleObjectsReturned and ObjectDoesNotExist under LibTmuxException — and TmuxObjectDoesNotExist inherits from ObjectDoesNotExist, so it moves too. This server keys two behaviours off LibTmuxException, and both change silently.

The suite did not notice. Pinned to the #718 branch, main is green: 701 passed, 0 failed. Every changed path was uncovered. That is the finding, and it is why this PR leads with tests.

What was broken

1. A stale id was retried. ReadonlyRetryMiddleware retries on LibTmuxException, which is correct for the transient socket errors libtmux wraps — but most of that family is not transient. A pane that does not exist will not exist on the second look; an ambiguous match does not become unambiguous during a backoff window. Every readonly call carrying a stale or guessed id paid a second tmux round-trip and 100 ms of latency to fail identically — and because AuditMiddleware sits outside the retry, the audit log recorded one call, so the doubled traffic was invisible.

This is not new with #718. _resolve_pane raises PaneNotFound, which is already a LibTmuxException — so panes have been over-retried all along. #718 merely widens it to every missing session and window, which is what made it impossible to keep ignoring. The fix closes the whole family, including the part that was already broken:

tests/test_middleware.py::test_readonly_retry_skips_deterministic_failures
  [TmuxObjectDoesNotExist]   FAILED on main   <- new with #718
  [MultipleObjectsReturned]  FAILED on main   <- new with #718
  [PaneNotFound]             FAILED on main   <- pre-existing
  [NoWindowsExist]           FAILED on main   <- pre-existing
  [BadSessionName]           FAILED on main   <- pre-existing
  [TmuxSessionExists]        FAILED on main   <- pre-existing

2. An ambiguous target reached the agent with no way forward. A window shared between sessions (a grouped session, or link-window) is listed once per session that holds it, so a name or index can match several rows and the lookup raises MultipleObjectsReturned. Today that surfaces as Unexpected error: MultipleObjectsReturned: with an empty message and a stack trace. Under #718 it would land somewhere worse: reclassified as agent-correctable (expected: true, WARNING, "tmux error: …") while still saying nothing about how to correct it. It now names the fix — target the object by id.

The fix

• NON_RETRYABLE_EXCEPTIONS in middleware.py, skipped in the retry decision. The check walks __cause__ one hop, exactly as fastmcp's _should_retry does — necessary because handle_tool_errors re-raises every libtmux failure as an ExpectedToolError chained off the original, so at the middleware layer the real failure is only visible one hop down.
• An explicit MultipleObjectsReturned arm in _map_exception_to_tool_error, above the LibTmuxException arm, carrying a recovery suggestion.
• The not-found arm widened from TmuxObjectDoesNotExist to ObjectDoesNotExist, so a QueryList miss gets the same discovery hint as a tmux-id miss.

Verification

Every new test fails on main and passes here — proven by reverting src/ and re-running:

src/ = main   ->  8 failed
src/ = fixed  ->  all pass

ruff format · pytest · ruff check --fix · mypy (strict) · just build-docs — all green. 711 passed (was 701).

The end-to-end test (test_readonly_retry_skips_not_found_on_decorated_tool) drives the real list_sessions tool through the production middleware stack, so it exercises the handle_tool_errors → __cause__ → retry-decision chain rather than a unit stub. A unit test alone would pass even if the skip never fired in production.

Before merge

1. libtmux#718 ships.
2. Drop [tool.uv.sources] from pyproject.toml.
3. Bump the floor: libtmux>=<that release>,<1.0 (was >=0.61.0).
4. uv lock, re-run the gates.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.30%. Comparing base (2cd125e) to head (c6b50ef).

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #98      +/-   ##
==========================================
+ Coverage   85.26%   85.30%   +0.04%     
==========================================
  Files          44       44              
  Lines        3265     3274       +9     
  Branches      452      454       +2     
==========================================
+ Hits         2784     2793       +9     
  Misses        351      351              
  Partials      130      130              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tony]

Code review

Found 6 issues. The first two are blockers.

1. The module does not load on the floor it declares. libtmux.exc.MultipleObjectsReturned does not exist on released libtmux 0.61.0 — exc.py there re-exports only ObjectDoesNotExist. The floor is libtmux>=0.61.0,<1.0, which permits it. NON_RETRYABLE_EXCEPTIONS is built at module level, server.py imports middleware, and main() imports server — so the console script dies at startup with AttributeError, which main()'s except ImportError: guard does not catch. The git pin masks this: the winlinks branch still self-reports __version__ = "0.61.0", so nothing — uv, mypy, or CI — will ever warn. The floor bump must land in the same commit that drops [tool.uv.sources], not merely "before merge".

libtmux-mcp/src/libtmux_mcp/middleware.py

Lines 676 to 686 in a07a6e1

	#: already covers :exc:`libtmux.exc.TmuxObjectDoesNotExist`.
	NON_RETRYABLE_EXCEPTIONS: tuple[type[Exception], ...] = (
	libtmux_exc.ObjectDoesNotExist,
	libtmux_exc.MultipleObjectsReturned,
	libtmux_exc.PaneNotFound,
	libtmux_exc.NoWindowsExist,
	libtmux_exc.BadSessionName,
	libtmux_exc.TmuxSessionExists,
	libtmux_exc.TmuxCommandNotFound,
	)

2. Second, independent break — inside the error handler. _utils.py imports fine on 0.61.0 (the reference is in a function body), so fixing middleware.py alone would not help. This arm sits above PaneNotFound and the LibTmuxException catch-all, so on released libtmux every exception reaching the mapper raises AttributeError from inside the handler, destroying the real diagnostic instead of reporting it.

libtmux-mcp/src/libtmux_mcp/_utils.py

Lines 1010 to 1020 in a07a6e1

	)
	if isinstance(e, exc.MultipleObjectsReturned):
	return ExpectedToolError(
	f"Ambiguous target: {e}",
	suggestion=(
	"A window shared between sessions is listed once per session that "
	"holds it, so a name or index can match more than one row. Target "
	"it by id (session_id / window_id / pane_id) instead."
	),
	)
	if isinstance(e, exc.PaneNotFound):

3. The recovery suggestion is backwards — it tells the agent to do the thing that raises. For the scenario it names (a window shared between sessions), window_id and pane_id are the duplication victims, not the disambiguator: the duplication key is (window_id, session_id). _resolve_window does server.windows.get(window_id=...) over list-windows -a, which returns one row per holding session. Probed live on a link-windowed window:

server.windows.get(window_id='@0')   -> RAISES  Multiple objects returned (2): window_id='@0'
server.panes.get(pane_id='%0')       -> RAISES  Multiple objects returned (2): pane_id='%0'
session.windows.get(window_id='@0')  -> OK

An agent that follows the suggestion re-issues by id, gets the identical error, and loops — which is the opposite of the "agent-correctable" contract this function's own docstring promises.

libtmux-mcp/src/libtmux_mcp/_utils.py

Lines 1010 to 1020 in a07a6e1

	)
	if isinstance(e, exc.MultipleObjectsReturned):
	return ExpectedToolError(
	f"Ambiguous target: {e}",
	suggestion=(
	"A window shared between sessions is listed once per session that "
	"holds it, so a name or index can match more than one row. Target "
	"it by id (session_id / window_id / pane_id) instead."
	),
	)
	if isinstance(e, exc.PaneNotFound):

4. Fixes #97 will auto-close an issue this PR does not fix. GitHub ignores the parenthetical qualifier. #97 asks for the resolvers to route through Pane.from_pane_id / Window.from_window_id; this PR implements only what Grouped and linked windows crash every pane/window-targeted tool: MultipleObjectsReturned escapes the error mapper #97 itself calls the "interim mitigation". _resolve_window/_resolve_pane still filter a server-wide listing and still raise on a grouped session, including the self-kill guards Grouped and linked windows crash every pane/window-targeted tool: MultipleObjectsReturned escapes the error mapper #97 names. Should be Refs #97.

5. NON_RETRYABLE_EXCEPTIONS is a fail-open denylist, and is already incomplete. The trigger stays the whole LibTmuxException family and this subtracts seven classes, so anything libtmux adds later defaults to retryable. It already misses NotInsideTmux — new on the very branch this PR exists to validate — and the OptionError family (InvalidOption, AmbiguousOption, UnknownOption), which is raised by the readonly show_option tool today. A mistyped option name is exactly the deterministic failure this PR is about, and it still pays a retry. The original design commit (707eed8) states the intent as an allowlist: "libtmux wraps the subprocess failures we actually want to retry (socket EAGAIN, transient connect errors)". Inverting to an allowlist is fail-safe and closer to that.

6. CHANGES uses libtmux's changelog convention, not this repo's (AGENTS.md says "every distinct deliverable gets a **Bold subheading**" and "PR refs (#NN) sit at the end of each deliverable's prose paragraph"). These two #### headings are the only ones in the file; every prior deliverable back to 0.1.0a10 uses **Bold** with the ref at paragraph end. The mandated ### Dependencies section recording the required libtmux floor is also missing.

libtmux-mcp/CHANGES

Lines 10 to 12 in a07a6e1

	#### A stale id fails once instead of twice (#98)

One note worth weighing rather than fixing: this PR subclasses and overrides fastmcp's private RetryMiddleware._should_retry. 8b102bb exists because fastmcp changed that method's semantics in a patch release, and this project shipped a silent production no-op with a green suite as a result. The <4.0.0 ceiling gives a leading-underscore method no protection. The new end-to-end test would catch a dead override on a dep bump — a real improvement over the state that produced 8b102bb — but inlining the ~25-line retry loop would remove the coupling entirely.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}