diff --git a/CHANGES b/CHANGES index f9ecf77..ffd8b2a 100644 --- a/CHANGES +++ b/CHANGES @@ -16,6 +16,17 @@ _Notes on upcoming releases will be added here_ conftest reaper is needed in libtmux-mcp — the upstream fixtures self-clean. +### Fixes + +- Pane `is_caller` annotation no longer false-positives across tmux + sockets. `_serialize_pane`, `snapshot_pane`, and `search_panes` all + compared `pane.pane_id == TMUX_PANE` without verifying the caller's + socket, so a caller on socket A pane `%0` was marked `is_caller=True` + for any `%0` on any other server. The annotation now reuses + `_caller_is_on_server` (the same socket-scoped comparator used by + the self-kill guard) via the new `_compute_is_caller` helper + (#22, fixes #19). + ## libtmux-mcp 0.1.0a2 (2026-04-19) _FastMCP alignment: new tools, prompts, and middleware (#15)_ diff --git a/src/libtmux_mcp/_utils.py b/src/libtmux_mcp/_utils.py index 8a618d3..13f8104 100644 --- a/src/libtmux_mcp/_utils.py +++ b/src/libtmux_mcp/_utils.py @@ -87,14 +87,42 @@ def _get_caller_identity() -> CallerIdentity | None: ) -def _get_caller_pane_id() -> str | None: - """Return the TMUX_PANE of the calling process, or None if not in tmux. - - Thin wrapper around :func:`_get_caller_identity` kept for callers that - only need the pane id (notably :func:`_serialize_pane`). +def _compute_is_caller(pane: Pane) -> bool | None: + """Decide whether ``pane`` is the MCP caller's own tmux pane. + + The returned value is used as the ``is_caller`` annotation on + :class:`~libtmux_mcp.models.PaneInfo`, + :class:`~libtmux_mcp.models.PaneSnapshot`, and + :class:`~libtmux_mcp.models.PaneContentMatch`. + + Tri-state semantics match the original bare-equality check: + + * ``None`` — process is not inside tmux at all (neither ``TMUX`` nor + ``TMUX_PANE`` are set). No caller exists, so the annotation + carries no signal. + * ``True`` — the caller's ``TMUX_PANE`` matches ``pane.pane_id`` + *and* :func:`_caller_is_strictly_on_server` confirms the + caller's socket realpath equals the target's. + * ``False`` — the pane ids differ, or they match but the socket + does not (or cannot be proven to). A bare pane-id equality + check would have returned ``True`` here, which is the + cross-socket false-positive fixed by + tmux-python/libtmux-mcp#19. + + Uses :func:`_caller_is_strictly_on_server` rather than + :func:`_caller_is_on_server`: the kill-guard comparator is + conservative-True-when-uncertain (right for blocking destructive + actions, wrong for an informational annotation that should + demand a positive match). The strict variant declines the + basename fallback, the unresolvable-target branch, and the + socket-path-unset branch so ambiguous cases resolve to ``False``. """ caller = _get_caller_identity() - return caller.pane_id if caller else None + if caller is None or caller.pane_id is None: + return None + return caller.pane_id == pane.pane_id and _caller_is_strictly_on_server( + pane.server, caller + ) def _effective_socket_path(server: Server) -> str | None: @@ -201,6 +229,45 @@ def _caller_is_on_server(server: Server, caller: CallerIdentity | None) -> bool: return caller_basename == target_name +def _caller_is_strictly_on_server( + server: Server, caller: CallerIdentity | None +) -> bool: + """Return True only on a confirmed socket-path match. + + Counterpart to :func:`_caller_is_on_server` for the informational + :attr:`~libtmux_mcp.models.PaneInfo.is_caller` annotation. The + destructive-action guard is biased toward True-when-uncertain so a + macOS ``$TMUX_TMPDIR`` divergence cannot fool it into permitting + self-kill; the annotation cannot absorb that bias — ambiguous cases + are exactly the cross-socket false positives documented by + tmux-python/libtmux-mcp#19. This function therefore declines every + branch other than a confirmed ``realpath`` match. + + Decision table: + + * ``caller is None`` → ``False``. No caller identity. + * ``caller.socket_path`` unset (``TMUX_PANE`` set without ``TMUX``) + → ``False``. We cannot verify the caller is on this server. + * target server's effective socket path unresolvable → ``False``. + * ``realpath`` of caller's socket path equals target's effective + path → ``True``. Primary and only positive signal. + * Fallback on ``OSError`` from ``realpath``: exact string match + → ``True``. Still a positive signal, just without the resolve + step. + * Otherwise → ``False`` (including the basename-only match that + :func:`_caller_is_on_server` permits as a conservative block). + """ + if caller is None or not caller.socket_path: + return False + target = _effective_socket_path(server) + if not target: + return False + try: + return os.path.realpath(caller.socket_path) == os.path.realpath(target) + except OSError: + return caller.socket_path == target + + # --------------------------------------------------------------------------- # Safety tier tags # --------------------------------------------------------------------------- @@ -757,7 +824,6 @@ def _serialize_pane(pane: Pane) -> PaneInfo: from libtmux_mcp.models import PaneInfo assert pane.pane_id is not None - caller_pane_id = _get_caller_pane_id() return PaneInfo( pane_id=pane.pane_id, pane_index=getattr(pane, "pane_index", None), @@ -770,7 +836,7 @@ def _serialize_pane(pane: Pane) -> PaneInfo: pane_active=getattr(pane, "pane_active", None), window_id=pane.window_id, session_id=pane.session_id, - is_caller=pane.pane_id == caller_pane_id if caller_pane_id else None, + is_caller=_compute_is_caller(pane), ) diff --git a/src/libtmux_mcp/models.py b/src/libtmux_mcp/models.py index e3e65f1..2cf63e1 100644 --- a/src/libtmux_mcp/models.py +++ b/src/libtmux_mcp/models.py @@ -69,8 +69,13 @@ class PaneInfo(BaseModel): is_caller: bool | None = Field( default=None, description=( - "True if this pane is the MCP caller's own pane " - "(detected via TMUX_PANE env var)" + "MCP caller identity for this pane. ``True`` when the pane " + "matches the caller's ``TMUX_PANE`` *and* lives on the same " + "tmux socket as the caller's ``TMUX`` (verified via socket " + "realpath); ``False`` otherwise, including the case where " + "the pane id matches but the socket does not or cannot be " + "proven to; ``None`` when the MCP process is not running " + "inside tmux at all." ), ) @@ -93,8 +98,13 @@ class PaneContentMatch(BaseModel): is_caller: bool | None = Field( default=None, description=( - "True if this pane is the MCP caller's own pane " - "(detected via TMUX_PANE env var)" + "MCP caller identity for this pane. ``True`` when the pane " + "matches the caller's ``TMUX_PANE`` *and* lives on the same " + "tmux socket as the caller's ``TMUX`` (verified via socket " + "realpath); ``False`` otherwise, including the case where " + "the pane id matches but the socket does not or cannot be " + "proven to; ``None`` when the MCP process is not running " + "inside tmux at all." ), ) @@ -178,7 +188,15 @@ class PaneSnapshot(BaseModel): ) is_caller: bool | None = Field( default=None, - description="True if this is the MCP caller's own pane", + description=( + "MCP caller identity for this pane. ``True`` when the pane " + "matches the caller's ``TMUX_PANE`` *and* lives on the same " + "tmux socket as the caller's ``TMUX`` (verified via socket " + "realpath); ``False`` otherwise, including the case where " + "the pane id matches but the socket does not or cannot be " + "proven to; ``None`` when the MCP process is not running " + "inside tmux at all." + ), ) content_truncated: bool = Field( default=False, diff --git a/src/libtmux_mcp/tools/pane_tools/meta.py b/src/libtmux_mcp/tools/pane_tools/meta.py index 60331d0..99e4bf5 100644 --- a/src/libtmux_mcp/tools/pane_tools/meta.py +++ b/src/libtmux_mcp/tools/pane_tools/meta.py @@ -3,7 +3,7 @@ from __future__ import annotations from libtmux_mcp._utils import ( - _get_caller_pane_id, + _compute_is_caller, _get_server, _resolve_pane, handle_tool_errors, @@ -160,7 +160,6 @@ def snapshot_pane( pane_mode_raw = parts[5] scroll_raw = parts[6] - caller_pane_id = _get_caller_pane_id() return PaneSnapshot( pane_id=pane.pane_id or "", content=content, @@ -175,7 +174,7 @@ def snapshot_pane( title=parts[8] if parts[8] else None, pane_current_command=parts[9] if parts[9] else None, pane_current_path=parts[10] if parts[10] else None, - is_caller=(pane.pane_id == caller_pane_id if caller_pane_id else None), + is_caller=_compute_is_caller(pane), content_truncated=truncated, content_truncated_lines=dropped, ) diff --git a/src/libtmux_mcp/tools/pane_tools/search.py b/src/libtmux_mcp/tools/pane_tools/search.py index 7ea8450..bfaec79 100644 --- a/src/libtmux_mcp/tools/pane_tools/search.py +++ b/src/libtmux_mcp/tools/pane_tools/search.py @@ -7,7 +7,7 @@ from fastmcp.exceptions import ToolError from libtmux_mcp._utils import ( - _get_caller_pane_id, + _compute_is_caller, _get_server, _resolve_session, handle_tool_errors, @@ -221,7 +221,6 @@ def search_panes( # sort the matching panes by pane_id for deterministic ordering, # then slice by offset / limit. Per-pane matched_lines is # tail-truncated to keep the most recent matches. - caller_pane_id = _get_caller_pane_id() all_matches: list[PaneContentMatch] = [] per_pane_truncated = False for pane_id_str in matching_pane_ids: @@ -251,7 +250,7 @@ def search_panes( session_id=pane.session_id, session_name=getattr(session_obj, "session_name", None), matched_lines=matched_lines, - is_caller=(pane_id_str == caller_pane_id if caller_pane_id else None), + is_caller=_compute_is_caller(pane), ) ) diff --git a/tests/test_pane_tools.py b/tests/test_pane_tools.py index 1d5e147..b86f839 100644 --- a/tests/test_pane_tools.py +++ b/tests/test_pane_tools.py @@ -783,10 +783,14 @@ class SearchPanesCallerFixture(t.NamedTuple): SEARCH_PANES_CALLER_FIXTURES: list[SearchPanesCallerFixture] = [ SearchPanesCallerFixture( - test_id="caller_pane_annotated", + # TMUX_PANE without TMUX: the strict comparator cannot verify the + # caller's socket and returns ``False`` rather than conservatively + # assuming same-server. Full-TMUX-env coverage lives in + # ``tests/test_utils.py::test_serialize_pane_is_caller_false_across_sockets``. + test_id="caller_pane_no_tmux_env", tmux_pane_env=None, use_real_pane_id=True, - expected_is_caller=True, + expected_is_caller=False, ), SearchPanesCallerFixture( test_id="outside_tmux_no_annotation", diff --git a/tests/test_utils.py b/tests/test_utils.py index 932a226..7c9db4e 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -20,7 +20,6 @@ TAG_READONLY, VALID_SAFETY_LEVELS, _apply_filters, - _get_caller_pane_id, _get_server, _invalidate_server, _resolve_pane, @@ -306,23 +305,6 @@ def test_apply_filters( assert len(result) >= 1 -# --------------------------------------------------------------------------- -# _get_caller_pane_id / _serialize_pane is_caller tests -# --------------------------------------------------------------------------- - - -def test_get_caller_pane_id_returns_env(monkeypatch: pytest.MonkeyPatch) -> None: - """_get_caller_pane_id returns TMUX_PANE when set.""" - monkeypatch.setenv("TMUX_PANE", "%42") - assert _get_caller_pane_id() == "%42" - - -def test_get_caller_pane_id_returns_none(monkeypatch: pytest.MonkeyPatch) -> None: - """_get_caller_pane_id returns None outside tmux.""" - monkeypatch.delenv("TMUX_PANE", raising=False) - assert _get_caller_pane_id() is None - - # --------------------------------------------------------------------------- # Caller identity parsing tests # --------------------------------------------------------------------------- @@ -532,10 +514,16 @@ class SerializePaneCallerFixture(t.NamedTuple): SERIALIZE_PANE_CALLER_FIXTURES: list[SerializePaneCallerFixture] = [ SerializePaneCallerFixture( - test_id="matching_pane_id", + # TMUX_PANE is set to the real pane id but TMUX is unset, so the + # caller's socket cannot be verified. The strict comparator + # declines to assume same-server: ``False`` not ``True``. + # Pre-fixup this returned ``True`` via ``_caller_is_on_server``'s + # conservative-True branch — a cross-socket false positive the + # informational annotation must not carry. + test_id="matching_pane_id_no_tmux_env", tmux_pane_env=None, use_real_pane_id=True, - expected_is_caller=True, + expected_is_caller=False, ), SerializePaneCallerFixture( test_id="non_matching_pane_id", @@ -577,6 +565,80 @@ def test_serialize_pane_is_caller( assert data.is_caller is expected_is_caller +def test_serialize_pane_is_caller_false_across_sockets( + TestServer: type[Server], + monkeypatch: pytest.MonkeyPatch, +) -> None: + """is_caller must not flag a pane on a *different* tmux socket. + + Regression for tmux-python/libtmux-mcp#19. Before the fix, + ``_serialize_pane`` compared ``pane.pane_id == TMUX_PANE`` without + any socket check — so a caller inside pane ``%0`` on socket A saw + ``is_caller=True`` for any pane with id ``%0`` on any other server. + + Two fresh libtmux servers emit matching pane ids (both start at + ``%0``), so this reproduces the false-positive exactly. Point the + caller at server A, serialize pane ``%0`` on server B, assert the + annotation says ``False``. + """ + from libtmux_mcp._utils import _effective_socket_path + + server_a = TestServer() + session_a = server_a.new_session(session_name="mcp_issue19_a") + pane_a = session_a.active_window.active_pane + assert pane_a is not None and pane_a.pane_id is not None + + server_b = TestServer() + session_b = server_b.new_session(session_name="mcp_issue19_b") + pane_b = session_b.active_window.active_pane + assert pane_b is not None and pane_b.pane_id is not None + + # Prerequisite: the two freshly-spawned servers emitted matching + # pane ids. If they didn't (a tmux version quirk), the false + # positive can't be exercised — skip rather than fail. + if pane_a.pane_id != pane_b.pane_id: + pytest.skip( + f"sibling servers emitted distinct pane ids " + f"({pane_a.pane_id} vs {pane_b.pane_id}); cannot reproduce issue #19" + ) + + socket_a = _effective_socket_path(server_a) + assert socket_a is not None + monkeypatch.setenv("TMUX", f"{socket_a},1,{session_a.session_id or '$0'}") + monkeypatch.setenv("TMUX_PANE", pane_a.pane_id) + + # Pane on the *other* server — must be flagged False even though + # its pane_id matches TMUX_PANE. + assert _serialize_pane(pane_b).is_caller is False + # Sanity: on the caller's own server, same pane_id *is* the caller. + assert _serialize_pane(pane_a).is_caller is True + + +def test_serialize_pane_is_caller_requires_tmux_env_not_just_pane( + mcp_pane: Pane, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """``TMUX_PANE`` alone must not declare a caller identity. + + Regression for the subtle cross-socket false positive that + :func:`_caller_is_on_server`'s "socket_path unset → conservative + True" branch would otherwise introduce. When the MCP process has + ``TMUX_PANE`` in its environment but not ``TMUX`` — an unusual but + possible state an agent harness can produce — the caller's socket + is unknowable. The strict comparator declines to assert + ``is_caller=True`` in that case so any pane whose id happens to + match ``TMUX_PANE`` across *any* server is annotated ``False``, + not a false positive. Exercises the code path that was left + un-covered after the direct ``_get_caller_pane_id`` unit tests + were removed. + """ + assert mcp_pane.pane_id is not None + monkeypatch.setenv("TMUX_PANE", mcp_pane.pane_id) + monkeypatch.delenv("TMUX", raising=False) + + assert _serialize_pane(mcp_pane).is_caller is False + + # --------------------------------------------------------------------------- # Annotation and tag constants tests # ---------------------------------------------------------------------------