From f92666cf531457ee72a8fec08e3ba091e7b55423 Mon Sep 17 00:00:00 2001 From: Mason Daugherty <61371264+mdrxy@users.noreply.github.com> Date: Tue, 18 Aug 2026 20:05:29 +0000 Subject: [PATCH 01/18] fix(code): capture stdio MCP server stderr into the logger Co-authored-by: open-swe[bot] --- libs/code/DEVELOPMENT.md | 4 +- libs/code/deepagents_code/_debug.py | 15 + libs/code/deepagents_code/mcp_tools.py | 278 ++++++++++++++++++- libs/code/tests/unit_tests/test_debug.py | 4 + libs/code/tests/unit_tests/test_mcp_tools.py | 267 ++++++++++++------ 5 files changed, 479 insertions(+), 89 deletions(-) diff --git a/libs/code/DEVELOPMENT.md b/libs/code/DEVELOPMENT.md index f593bf120e1..2eb8590dda6 100644 --- a/libs/code/DEVELOPMENT.md +++ b/libs/code/DEVELOPMENT.md @@ -136,7 +136,9 @@ For problems that appear after the app is up, tail the client log in another ter tail -f /tmp/deepagents_debug.log ``` -To send it elsewhere, also `export DEEPAGENTS_CODE_DEBUG_FILE=`. The handler appends across runs, so a single file accumulates every session. +To send it elsewhere, also `export DEEPAGENTS_CODE_DEBUG_FILE=`. The handler appends across runs, so a single file accumulates every session. The file is created with user-only permissions. + +Stdio MCP server stderr is captured here at `DEBUG` so server-side failures remain diagnosable when the TUI cannot display process stderr. Records are split into bounded lines and stripped of control characters, but the remaining text is otherwise server-provided and may contain credentials or other sensitive values. Only enable or share DEBUG logs with that risk in mind. ### In-app Debug Console (`Ctrl+\`) diff --git a/libs/code/deepagents_code/_debug.py b/libs/code/deepagents_code/_debug.py index 6e2422ddc9d..61e30b62d93 100644 --- a/libs/code/deepagents_code/_debug.py +++ b/libs/code/deepagents_code/_debug.py @@ -39,6 +39,20 @@ """ +def _prepare_debug_file(path: Path) -> None: + """Create or tighten a debug file before attaching the logging handler.""" + flags = os.O_APPEND | os.O_CREAT | os.O_WRONLY | getattr(os, "O_NOFOLLOW", 0) + fd = os.open(path, flags, 0o600) + try: + fchmod = getattr(os, "fchmod", None) + if fchmod is None: + path.chmod(0o600) + else: + fchmod(fd, 0o600) + finally: + os.close(fd) + + def resolve_log_level(*, debug_enabled: bool | None = None) -> int: """Resolve the configured runtime logging level. @@ -114,6 +128,7 @@ def configure_debug_logging(target: logging.Logger) -> None: existing.close() try: + _prepare_debug_file(debug_path) handler = logging.FileHandler(str(debug_path), mode="a") except OSError as exc: message = f"could not open debug log file {debug_path}: {exc}" diff --git a/libs/code/deepagents_code/mcp_tools.py b/libs/code/deepagents_code/mcp_tools.py index ad749d1c89e..59781a0b6ed 100644 --- a/libs/code/deepagents_code/mcp_tools.py +++ b/libs/code/deepagents_code/mcp_tools.py @@ -9,14 +9,19 @@ from __future__ import annotations import asyncio +import codecs import copy import fnmatch import functools +import io import json import logging +import os import re import shutil -from contextlib import AsyncExitStack +import threading +import unicodedata +from contextlib import AsyncExitStack, asynccontextmanager from dataclasses import dataclass from pathlib import Path from typing import TYPE_CHECKING, Any, Literal, NamedTuple, cast, overload @@ -25,7 +30,15 @@ from deepagents_code.mcp_config import resolve_mcp_server_env if TYPE_CHECKING: - from collections.abc import Awaitable, Callable, Collection, Mapping, Sequence + from collections.abc import ( + AsyncIterator, + Awaitable, + Callable, + Collection, + Mapping, + Sequence, + ) + from typing import TextIO from langchain_core.tools import BaseTool from langchain_mcp_adapters.client import Connection @@ -189,6 +202,256 @@ class MCPConfigError(ValueError): """ +_MCP_ENV_REFERENCE_RE = re.compile(r"\$\{([^}]+)\}") +_MCP_STDERR_LINE_LIMIT = 4096 +_MCP_STDERR_READ_SIZE = 8192 +_MCP_STDERR_TRUNCATION_MARKER = "... [truncated]" + + +def _resolve_stdio_env( + env: dict[str, str] | None, + *, + server_name: str, +) -> dict[str, str] | None: + """Resolve adapter-compatible braced environment references. + + Args: + env: Configured subprocess environment. + server_name: MCP server name used in warning records. + + Returns: + Resolved environment values, or `None` when no environment is configured. + """ + if env is None: + return None + resolved = { + key: _MCP_ENV_REFERENCE_RE.sub( + lambda match: os.environ.get(match.group(1), match.group(0)), value + ) + for key, value in env.items() + } + for key, value in resolved.items(): + if _MCP_ENV_REFERENCE_RE.search(value): + logger.warning( + "MCP server %r env[%r] contains an unexpanded variable reference", + server_name, + key, + ) + return resolved + + +class _MCPStderrSink(io.TextIOBase): + """Forward a subprocess pipe to the MCP logger one bounded line at a time.""" + + def __init__(self, server_name: str, *, encoding: str, errors: str) -> None: + """Create the pipe and start its reader.""" + super().__init__() + self._server_name = server_name + self._encoding = encoding + self._errors = errors + self._capture = logger.isEnabledFor(logging.DEBUG) + self._decoder = ( + codecs.getincrementaldecoder(encoding)(errors=errors) + if self._capture + else None + ) + self._line = "" + self._truncated = False + self._read_fd, write_fd = os.pipe() + try: + self._writer = os.fdopen(write_fd, "wb", buffering=0) + except BaseException: + os.close(write_fd) + os.close(self._read_fd) + raise + try: + self._thread = threading.Thread( + target=self._drain, + name=f"mcp-stderr-{server_name}", + daemon=True, + ) + self._thread.start() + except BaseException: + self._writer.close() + os.close(self._read_fd) + raise + + @property + def encoding(self) -> str: + """Encoding used for writes and captured bytes.""" + return self._encoding + + @property + def errors(self) -> str: + """Configured encoding error handler.""" + return self._errors + + def fileno(self) -> int: + """Return the subprocess-inheritable write descriptor.""" + return self._writer.fileno() + + def writable(self) -> bool: + """Return whether the parent write descriptor remains open.""" + return not self.closed + + def write(self, text: str) -> int: + """Write text through the pipe for TextIO compatibility. + + Args: + text: Text to encode and write. + + Returns: + Number of input characters written. + + Raises: + ValueError: If the sink is closed. + """ + if self.closed: + msg = "I/O operation on closed MCP stderr sink" + raise ValueError(msg) + data = memoryview(text.encode(self._encoding, errors=self._errors)) + while data: + written = self._writer.write(data) + if written is None: + continue + data = data[written:] + return len(text) + + def flush(self) -> None: + """Flush parent writes before closing the descriptor.""" + if not self._writer.closed: + self._writer.flush() + + def close(self) -> None: + """Close the parent's copy of the subprocess write descriptor.""" + if self.closed: + return + try: + super().close() + finally: + self._writer.close() + + async def wait_closed(self) -> None: + """Wait off the event loop until the pipe reader reaches EOF.""" + self.close() + await asyncio.to_thread(self._thread.join) + + def _drain(self) -> None: + """Drain subprocess bytes so stderr can never block the child.""" + try: + while chunk := os.read(self._read_fd, _MCP_STDERR_READ_SIZE): + if self._capture: + self._decode(chunk) + if self._capture: + self._decode(b"", final=True) + if self._line or self._truncated: + self._emit_line() + except OSError as exc: + if self._capture: + logger.debug( + "MCP server %r stderr capture failed: %s", + self._server_name, + exc, + ) + finally: + os.close(self._read_fd) + + def _decode(self, data: bytes, *, final: bool = False) -> None: + """Decode one byte chunk without allowing malformed stderr to stop draining.""" + if self._decoder is None: + return + try: + text = self._decoder.decode(data, final=final) + except (LookupError, UnicodeError) as exc: + self._decoder.reset() + logger.debug( + "MCP server %r stderr decode failed with %s: %s", + self._server_name, + self._encoding, + exc, + ) + return + parts = text.split("\n") + for part in parts[:-1]: + self._append(part) + self._emit_line() + self._append(parts[-1]) + + def _append(self, text: str) -> None: + """Retain sanitized line content up to the configured bound.""" + if self._truncated: + return + safe = "".join( + char for char in text if not unicodedata.category(char).startswith("C") + ) + remaining = _MCP_STDERR_LINE_LIMIT - len(self._line) + self._line += safe[:remaining] + if len(safe) > remaining: + self._truncated = True + + def _emit_line(self) -> None: + """Emit and reset the current complete line.""" + line = self._line + if self._truncated: + content_limit = _MCP_STDERR_LINE_LIMIT - len(_MCP_STDERR_TRUNCATION_MARKER) + line = line[:content_limit] + _MCP_STDERR_TRUNCATION_MARKER + if line: + logger.debug("MCP server %r stderr: %s", self._server_name, line) + self._line = "" + self._truncated = False + + +@asynccontextmanager +async def _create_mcp_session( + connection: Connection, + *, + server_name: str, +) -> AsyncIterator[ClientSession]: + """Create a session while routing stdio server diagnostics into DEBUG logs. + + Args: + connection: Adapter connection configuration. + server_name: MCP server name used in log records. + + Yields: + An open MCP client session. + """ + from langchain_mcp_adapters.sessions import create_session + + if connection["transport"] != "stdio": + async with create_session(connection) as session: + yield session + return + + from mcp import ClientSession, StdioServerParameters + from mcp.client.stdio import stdio_client + + stdio = connection + encoding = stdio.get("encoding", "utf-8") + errors = stdio.get("encoding_error_handler", "strict") + params = StdioServerParameters( + command=stdio["command"], + args=stdio["args"], + env=_resolve_stdio_env(stdio.get("env"), server_name=server_name), + cwd=stdio.get("cwd"), + encoding=encoding, + encoding_error_handler=errors, + ) + sink = _MCPStderrSink(server_name, encoding=encoding, errors=errors) + try: + async with stdio_client(params, errlog=cast("TextIO", sink)) as (read, write): + sink.close() + async with ClientSession( + read, + write, + **(stdio.get("session_kwargs") or {}), + ) as session: + yield session + finally: + sink.close() + await sink.wait_closed() + + def _is_transient_session_error(exc: BaseException) -> bool: """Return `True` when `exc` signals the MCP session transport is dead. @@ -432,11 +695,11 @@ async def _create_entry(self, server_name: str) -> _MCPSessionEntry: ) raise ValueError(msg) from exc - from langchain_mcp_adapters.sessions import create_session - exit_stack = AsyncExitStack() try: - session = await exit_stack.enter_async_context(create_session(connection)) + session = await exit_stack.enter_async_context( + _create_mcp_session(connection, server_name=server_name) + ) await session.initialize() except BaseException: # Close the partially entered stack in *this* task before @@ -1891,7 +2154,6 @@ async def _load_tools_from_config( SSEConnection, StdioConnection, StreamableHttpConnection, - create_session, ) from langchain_mcp_adapters.tools import convert_mcp_tool_to_langchain_tool @@ -2098,7 +2360,9 @@ def _log_caught_exception( logger.log(level, message, server_name, exc_info=caught) try: - async with create_session(connections[server_name]) as discover_session: + async with _create_mcp_session( + connections[server_name], server_name=server_name + ) as discover_session: await discover_session.initialize() mcp_tools = await _discover_tools(discover_session) except (asyncio.CancelledError, KeyboardInterrupt, SystemExit): diff --git a/libs/code/tests/unit_tests/test_debug.py b/libs/code/tests/unit_tests/test_debug.py index 628fe702049..0ce68979e4f 100644 --- a/libs/code/tests/unit_tests/test_debug.py +++ b/libs/code/tests/unit_tests/test_debug.py @@ -5,6 +5,7 @@ import importlib import logging import os +import stat from unittest.mock import patch import deepagents_code @@ -63,6 +64,7 @@ def test_noop_when_env_unset(self) -> None: def test_adds_handler_when_env_set(self, tmp_path) -> None: logger = logging.getLogger("test.debug.add") log_file = tmp_path / "debug.log" + log_file.touch(mode=0o644) with patch.dict( os.environ, {"DEEPAGENTS_CODE_DEBUG": "1", "DEEPAGENTS_CODE_DEBUG_FILE": str(log_file)}, @@ -70,6 +72,8 @@ def test_adds_handler_when_env_set(self, tmp_path) -> None: configure_debug_logging(logger) assert any(isinstance(h, logging.FileHandler) for h in logger.handlers) assert logger.level == logging.DEBUG + if os.name != "nt": + assert stat.S_IMODE(log_file.stat().st_mode) == 0o600 # Cleanup for h in logger.handlers[:]: if isinstance(h, logging.FileHandler): diff --git a/libs/code/tests/unit_tests/test_mcp_tools.py b/libs/code/tests/unit_tests/test_mcp_tools.py index b487465be44..66490630824 100644 --- a/libs/code/tests/unit_tests/test_mcp_tools.py +++ b/libs/code/tests/unit_tests/test_mcp_tools.py @@ -5,6 +5,7 @@ import asyncio import json import logging +import os import sys import threading import time @@ -26,12 +27,15 @@ from deepagents_code.mcp_auth import FileTokenStorage, MCPReauthRequiredError from deepagents_code.mcp_tools import ( + _MCP_STDERR_LINE_LIMIT, + _MCP_STDERR_TRUNCATION_MARKER, MCPServerInfo, MCPSessionManager, MCPToolInfo, _apply_tool_filter, _check_remote_server, _check_stdio_server, + _create_mcp_session, _gather_bounded, _json_error_snippet, _load_tools_from_config, @@ -138,13 +142,13 @@ def fake_create_session() -> Generator[tuple[AsyncMock, list[dict[str, Any]]]]: async def _fake( connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) recorded.append(connection) yield session - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): yield session, recorded @@ -859,10 +863,111 @@ def test_status_unauth_rejects_tools(self) -> None: ) +class TestMCPStderrCapture: + """Tests for stdio subprocess diagnostic capture.""" + + async def test_stdio_stderr_is_buffered_decoded_and_bounded( + self, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, + ) -> None: + """Split encoded bytes become sanitized, bounded DEBUG records.""" + first_written = tmp_path / "first-written" + release = tmp_path / "release" + long_line_size = _MCP_STDERR_LINE_LIMIT + 100 + script = "\n".join( + [ + "import os, sys, time", + "encoding = 'utf-16-le'", + "prefix = os.environ['MCP_CHILD_VALUE']", + "os.write(2, (prefix + ' caf').encode(encoding) + b'\\xe9')", + "open(sys.argv[1], 'w').close()", + "while not os.path.exists(sys.argv[2]):", + " time.sleep(0.005)", + f"payload = '\\x1b[31m\\n' + 'x' * {long_line_size} + '\\nfinal'", + "os.write(2, b'\\x00' + payload.encode(encoding))", + "sys.stdin.buffer.read()", + ] + ) + connection = cast( + "Connection", + { + "transport": "stdio", + "command": sys.executable, + "args": ["-c", script, str(first_written), str(release)], + "env": {"MCP_CHILD_VALUE": "${MCP_TEST_ENV}"}, + "encoding": "utf-16-le", + "encoding_error_handler": "strict", + }, + ) + + def stderr_messages() -> list[str]: + return [ + record.getMessage() + for record in caplog.records + if record.name == "deepagents_code.mcp_tools" + and record.levelno == logging.DEBUG + and " stderr: " in record.getMessage() + ] + + with ( + patch.dict(os.environ, {"MCP_TEST_ENV": "partial"}), + caplog.at_level(logging.DEBUG, logger="deepagents_code.mcp_tools"), + ): + async with _create_mcp_session(connection, server_name="fake"): + for _ in range(100): + if await asyncio.to_thread(first_written.exists): + break + await asyncio.sleep(0.01) + assert first_written.exists() + assert stderr_messages() == [] + await asyncio.to_thread(release.write_text, "") + for _ in range(100): + if len(stderr_messages()) >= 2: + break + await asyncio.sleep(0.01) + assert len(stderr_messages()) == 2 + + messages = stderr_messages() + assert messages[0] == "MCP server 'fake' stderr: partial café[31m" + long_line = messages[1].partition(" stderr: ")[2] + assert len(long_line) == _MCP_STDERR_LINE_LIMIT + assert long_line.endswith(_MCP_STDERR_TRUNCATION_MARKER) + assert messages[2] == "MCP server 'fake' stderr: final" + assert not any( + thread.name == "mcp-stderr-fake" and thread.is_alive() + for thread in threading.enumerate() + ) + + async def test_remote_session_delegates_to_adapter(self) -> None: + """Non-stdio transports retain the adapter's session handling.""" + session = AsyncMock() + connection = cast( + "Connection", + {"transport": "streamable_http", "url": "https://example.com/mcp"}, + ) + + @asynccontextmanager + async def fake_create_session( + received: Connection, + *, + mcp_callbacks: object | None = None, + ) -> AsyncIterator[AsyncMock]: + assert received is connection + assert mcp_callbacks is None + yield session + + with patch( + "langchain_mcp_adapters.sessions.create_session", fake_create_session + ): + async with _create_mcp_session(connection, server_name="remote") as created: + assert created is session + + class TestMCPSessionManager: """Tests for lazy runtime session caching.""" - @patch("langchain_mcp_adapters.sessions.create_session") + @patch("deepagents_code.mcp_tools._create_mcp_session") async def test_reuses_single_session_for_concurrent_first_use( self, mock_create_session: MagicMock, @@ -875,7 +980,7 @@ async def test_reuses_single_session_for_concurrent_first_use( async def _fake_create_session( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0.01) yield session @@ -901,7 +1006,7 @@ async def _fake_create_session( assert second is session mock_create_session.assert_called_once() - @patch("langchain_mcp_adapters.sessions.create_session") + @patch("deepagents_code.mcp_tools._create_mcp_session") async def test_cleanup_closes_cached_sessions_and_blocks_future_creation( self, mock_create_session: MagicMock, @@ -953,7 +1058,7 @@ async def test_configure_accepts_equivalent_oauth_connections(self) -> None: async def _fake( _conn: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) yield session @@ -974,7 +1079,7 @@ def _connection() -> Connection: ) manager = MCPSessionManager(connections={"notion": _connection()}) - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): await manager.get_session("notion") manager.configure({"notion": _connection()}) @@ -989,14 +1094,14 @@ async def test_configure_after_sessions_rejects_changes(self) -> None: async def _fake( _conn: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) yield session conn = {"filesystem": {"transport": "stdio", "command": "npx", "args": []}} manager = MCPSessionManager(connections=conn) # ty: ignore - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): await manager.get_session("filesystem") with pytest.raises(RuntimeError, match="Cannot reconfigure"): @@ -1027,7 +1132,7 @@ async def test_invalidate_with_mismatched_identity_skips(self) -> None: "filesystem": {"transport": "stdio", "command": "x", "args": []} } ) - with patch("langchain_mcp_adapters.sessions.create_session", return_value=cm): + with patch("deepagents_code.mcp_tools._create_mcp_session", return_value=cm): cached = await manager.get_session("filesystem") assert cached is session_a @@ -1059,7 +1164,7 @@ async def test_cancelled_initialize_closes_session_in_creating_task(self) -> Non async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: enter_task.append(asyncio.current_task()) try: @@ -1074,7 +1179,7 @@ async def _fake( ) with ( - patch("langchain_mcp_adapters.sessions.create_session", _fake), + patch("deepagents_code.mcp_tools._create_mcp_session", _fake), pytest.raises(asyncio.CancelledError), ): await manager.get_session("filesystem") @@ -1107,7 +1212,7 @@ class _Boom(BaseException): async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: enter_task.append(asyncio.current_task()) try: @@ -1122,7 +1227,7 @@ async def _fake( ) with ( - patch("langchain_mcp_adapters.sessions.create_session", _fake), + patch("deepagents_code.mcp_tools._create_mcp_session", _fake), pytest.raises(_Boom), ): await manager.get_session("filesystem") @@ -1146,7 +1251,7 @@ async def test_cleanup_failure_does_not_mask_original_cancellation( async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: try: yield session @@ -1161,7 +1266,7 @@ async def _fake( ) with ( - patch("langchain_mcp_adapters.sessions.create_session", _fake), + patch("deepagents_code.mcp_tools._create_mcp_session", _fake), caplog.at_level(logging.WARNING, logger="deepagents_code.mcp_tools"), pytest.raises(asyncio.CancelledError), ): @@ -1203,7 +1308,7 @@ class _InitError(Exception): async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: try: yield session @@ -1217,7 +1322,7 @@ async def _fake( ) with ( - patch("langchain_mcp_adapters.sessions.create_session", _fake), + patch("deepagents_code.mcp_tools._create_mcp_session", _fake), caplog.at_level(logging.WARNING, logger="deepagents_code.mcp_tools"), pytest.raises(asyncio.CancelledError), ): @@ -1346,7 +1451,7 @@ async def test_discovery_failure_marks_server_error( async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[None]: await asyncio.sleep(0) msg = "boom" @@ -1355,7 +1460,7 @@ async def _fake( caplog.set_level(logging.DEBUG, logger="deepagents_code.mcp_tools") - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): tools, manager, server_infos = await get_mcp_tools(path) assert tools == [] @@ -1721,13 +1826,13 @@ async def test_existing_tokens_attach_oauth_provider( async def _fake( connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) recorded.append(connection) yield session - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): config = { "mcpServers": { "notion": { @@ -1761,7 +1866,7 @@ async def test_discovery_reauth_marks_server_unauthenticated( async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[None]: await asyncio.sleep(0) msg = "discovery failed" @@ -1770,7 +1875,7 @@ async def _fake( caplog.set_level(logging.DEBUG, logger="deepagents_code.mcp_tools") - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): config = { "mcpServers": { "notion": { @@ -1844,13 +1949,13 @@ async def test_stored_tokens_attach_provider_without_explicit_oauth( async def _fake( connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) recorded.append(connection) yield session - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): config = { "mcpServers": { "notion": { @@ -1886,13 +1991,13 @@ async def test_authorization_header_skips_stored_oauth_without_explicit_oauth( async def _fake( connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) recorded.append(connection) yield session - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): config = { "mcpServers": { "notion": { @@ -1932,7 +2037,7 @@ async def test_discovery_401_challenge_marks_unauthenticated( async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[None]: await asyncio.sleep(0) raise challenge @@ -1940,7 +2045,7 @@ async def _fake( caplog.set_level(logging.DEBUG, logger="deepagents_code.mcp_tools") - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): config = { "mcpServers": { "notion": { @@ -1983,13 +2088,13 @@ async def test_discovery_401_without_challenge_stays_error(self) -> None: async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[None]: await asyncio.sleep(0) raise error yield - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): config = { "mcpServers": { "notion": { @@ -2019,13 +2124,13 @@ async def test_discovery_401_basic_challenge_stays_error(self) -> None: async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[None]: await asyncio.sleep(0) raise error yield - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): config = { "mcpServers": { "notion": { @@ -2060,13 +2165,13 @@ async def test_discovery_401_challenge_marks_unauthenticated_sse(self) -> None: async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[None]: await asyncio.sleep(0) raise challenge yield - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): config = { "mcpServers": { "notion": { @@ -2669,7 +2774,7 @@ async def test_expanded_url_is_redacted_from_discovery_error( async def _fail_discovery( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[None]: raise discovery_error yield @@ -2681,7 +2786,7 @@ async def _fail_discovery( new_callable=AsyncMock, ), patch( - "langchain_mcp_adapters.sessions.create_session", + "deepagents_code.mcp_tools._create_mcp_session", _fail_discovery, ), ): @@ -2815,12 +2920,12 @@ async def test_tools_sorted_alphabetically( async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) yield session - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): tools, manager, _ = await get_mcp_tools(path) assert [tool.name for tool in tools] == ["srv_alpha", "srv_mu", "srv_zeta"] @@ -2872,7 +2977,7 @@ def _tracking_session_factory( async def _fake( connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: stats["inflight"] += 1 stats["max_inflight"] = max(stats["max_inflight"], stats["inflight"]) @@ -2914,7 +3019,7 @@ async def _release_when_all_open() -> None: await asyncio.sleep(0.005) hold.set() - with patch("langchain_mcp_adapters.sessions.create_session", fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", fake): releaser = asyncio.create_task(_release_when_all_open()) tools, manager, infos = await _load_tools_from_config(self._config(*names)) await releaser @@ -2943,7 +3048,7 @@ async def test_discovery_concurrency_is_bounded( tool_by_server=tool_by_server, sleep_s=0.03 ) - with patch("langchain_mcp_adapters.sessions.create_session", fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", fake): tools, manager, infos = await _load_tools_from_config(self._config(*names)) assert stats["max_inflight"] == 2 @@ -2968,7 +3073,7 @@ async def test_order_preserved_when_later_servers_finish_first(self) -> None: async def _fake( connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: server = (connection.get("args") or ["x"])[0].removesuffix(".js") session = AsyncMock() @@ -2982,7 +3087,7 @@ async def _fake( finished[server].set() yield session - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): tools, manager, infos = await _load_tools_from_config(self._config(*names)) assert finish_order == ["third", "second", "first"] @@ -3003,7 +3108,7 @@ async def test_one_server_failure_isolated_from_others(self) -> None: async def _fake( connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: server = (connection.get("args") or ["x"])[0].removesuffix(".js") await asyncio.sleep(0.01) @@ -3017,7 +3122,7 @@ async def _fake( ) yield session - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): tools, manager, infos = await _load_tools_from_config(self._config(*names)) by_name = {i.name: i for i in infos} @@ -3051,7 +3156,7 @@ def _check(name: str, _cfg: dict[str, Any]) -> None: async def _fake( connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: server = (connection.get("args") or ["x"])[0].removesuffix(".js") session = AsyncMock() @@ -3064,7 +3169,7 @@ async def _fake( with ( patch("deepagents_code.mcp_tools._check_stdio_server", _check), - patch("langchain_mcp_adapters.sessions.create_session", _fake), + patch("deepagents_code.mcp_tools._create_mcp_session", _fake), ): tools, manager, infos = await _load_tools_from_config(self._config(*names)) @@ -3123,7 +3228,7 @@ def _filter( return server_tools with ( - patch("langchain_mcp_adapters.sessions.create_session", fake), + patch("deepagents_code.mcp_tools._create_mcp_session", fake), patch("deepagents_code.mcp_tools._apply_tool_filter", _filter), ): tools, manager, infos = await _load_tools_from_config(self._config(*names)) @@ -3146,7 +3251,7 @@ async def test_cancellation_propagates_and_cancels_siblings(self) -> None: async def _fake( connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: server = (connection.get("args") or ["x"])[0].removesuffix(".js") if server == "cancel": @@ -3160,7 +3265,7 @@ async def _fake( yield AsyncMock() # pragma: no cover - never reached with ( - patch("langchain_mcp_adapters.sessions.create_session", _fake), + patch("deepagents_code.mcp_tools._create_mcp_session", _fake), pytest.raises(asyncio.CancelledError), ): await _load_tools_from_config(self._config(*names)) @@ -3202,7 +3307,7 @@ async def _release() -> None: ) with ( patch("deepagents_code.mcp_tools._check_stdio_server", _slow_check), - patch("langchain_mcp_adapters.sessions.create_session", fake), + patch("deepagents_code.mcp_tools._create_mcp_session", fake), ): releaser = asyncio.create_task(_release()) _tools, manager, infos = await _load_tools_from_config(self._config(*names)) @@ -3297,7 +3402,7 @@ def _warm() -> None: async def _fake( connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: events.append(("discover", threading.get_ident())) session = AsyncMock() @@ -3307,7 +3412,7 @@ async def _fake( with ( patch("deepagents_code.mcp_tools._warm_mcp_adapter_imports", _warm), - patch("langchain_mcp_adapters.sessions.create_session", _fake), + patch("deepagents_code.mcp_tools._create_mcp_session", _fake), ): _tools, manager, _infos = await _load_tools_from_config( self._config("only") @@ -3469,12 +3574,12 @@ def _new_session() -> AsyncMock: async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) yield _new_session() - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): tools, manager, _ = await get_mcp_tools(path) result = await tools[0].ainvoke({}) # ty: ignore @@ -3509,12 +3614,12 @@ def _new_session() -> AsyncMock: async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) yield _new_session() - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): tools, manager, _ = await get_mcp_tools(path) await tools[0].ainvoke({}) # ty: ignore await tools[0].ainvoke({}) # ty: ignore @@ -3558,13 +3663,13 @@ def _new_session(*, dead: bool = False) -> AsyncMock: async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) call_counter["n"] += 1 yield _new_session(dead=(call_counter["n"] == 2)) - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): tools, manager, _ = await get_mcp_tools(path) await tools[0].ainvoke({}) # ty: ignore @@ -3600,13 +3705,13 @@ def _new_session(*, dead: bool) -> AsyncMock: async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) call_counter["n"] += 1 yield _new_session(dead=(call_counter["n"] >= 2)) - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): tools, manager, _ = await get_mcp_tools(path) result = await tools[0].ainvoke( {"args": {}, "id": "call-1", "type": "tool_call"} @@ -3648,13 +3753,13 @@ def _new_session(*, fail: bool) -> AsyncMock: async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) call_counter["n"] += 1 yield _new_session(fail=(call_counter["n"] >= 2)) - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): tools, manager, _ = await get_mcp_tools(path) result = await tools[0].ainvoke( {"args": {}, "id": "call-1", "type": "tool_call"} @@ -3701,13 +3806,13 @@ def _new_session() -> AsyncMock: async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) call_counter["n"] += 1 yield _new_session() - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): tools, manager, _ = await get_mcp_tools(path) result = await tools[0].ainvoke( {"args": {}, "id": "call-1", "type": "tool_call"} @@ -3753,12 +3858,12 @@ def _new_session() -> AsyncMock: async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) yield _new_session() - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): tools, manager, _ = await get_mcp_tools(path) result = await tools[0].ainvoke( {"args": {}, "id": "call-1", "type": "tool_call"} @@ -3813,13 +3918,13 @@ def _new_session(*, reauth: bool = False) -> AsyncMock: async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) call_counter["n"] += 1 yield _new_session(reauth=(call_counter["n"] == 2)) - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): tools, manager, _ = await get_mcp_tools(path) result = await tools[0].ainvoke( {"args": {}, "id": "call-1", "type": "tool_call"} @@ -3868,14 +3973,14 @@ async def _call_tool( async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[LoopBoundSession]: await asyncio.sleep(0) session = LoopBoundSession() sessions.append(session) yield session - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): tools, manager, _ = asyncio.run(get_mcp_tools(path)) result = asyncio.run(tools[0].ainvoke({})) # ty: ignore assert manager is not None @@ -4172,12 +4277,12 @@ async def test_allowed_tools_filters_loaded_tools( async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) yield session - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): tools, manager, server_infos = await get_mcp_tools(path) assert [t.name for t in tools] == ["fs_read_file"] @@ -4214,12 +4319,12 @@ async def test_disabled_tools_removes_loaded_tools( async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) yield session - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): tools, manager, _ = await get_mcp_tools(path) assert [t.name for t in tools] == ["fs_read_file"] @@ -4255,12 +4360,12 @@ async def test_filter_applies_to_http_server( async def _fake( _connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) yield session - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): tools, manager, _ = await get_mcp_tools(path) assert [t.name for t in tools] == ["api_search"] @@ -4312,7 +4417,7 @@ async def test_filters_are_per_server( async def _fake( connection: dict[str, Any], *, - _mcp_callbacks: object | None = None, + server_name: str, ) -> AsyncIterator[AsyncMock]: await asyncio.sleep(0) url = connection.get("url") @@ -4322,7 +4427,7 @@ async def _fake( else: yield fs_session - with patch("langchain_mcp_adapters.sessions.create_session", _fake): + with patch("deepagents_code.mcp_tools._create_mcp_session", _fake): tools, manager, _ = await get_mcp_tools(path) names = sorted(t.name for t in tools) From 181b5d00cefabeffee2b3e8f937ec7b4a678783b Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Tue, 18 Aug 2026 22:12:26 -0400 Subject: [PATCH 02/18] fix(code): bound MCP stderr drain join and force-close leaked pipe A stdio MCP server can spawn a longer-lived descendant that inherits the stderr pipe and then exit when its stdin closes. The surviving descendant keeps the pipe's write end open, so the drain thread blocks in os.read past process exit and the unbounded join in wait_closed hung session cleanup (discovery failure, reload, shutdown). Bound the join and, when the thread is still alive, close the pipe read end to force os.read to fail so the daemon thread exits. --- libs/code/deepagents_code/mcp_tools.py | 46 +++++++++++++++++-- libs/code/tests/unit_tests/test_mcp_tools.py | 48 ++++++++++++++++++++ 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/libs/code/deepagents_code/mcp_tools.py b/libs/code/deepagents_code/mcp_tools.py index 59781a0b6ed..ae15557a9d3 100644 --- a/libs/code/deepagents_code/mcp_tools.py +++ b/libs/code/deepagents_code/mcp_tools.py @@ -10,7 +10,9 @@ import asyncio import codecs +import contextlib import copy +import errno import fnmatch import functools import io @@ -206,6 +208,17 @@ class MCPConfigError(ValueError): _MCP_STDERR_LINE_LIMIT = 4096 _MCP_STDERR_READ_SIZE = 8192 _MCP_STDERR_TRUNCATION_MARKER = "... [truncated]" +_MCP_STDERR_DRAIN_JOIN_TIMEOUT = 2.0 +"""Bound on joining the stderr drain thread during session teardown. + +The drain thread blocks in `os.read` until the pipe hits EOF, which only +happens once *every* process holding the write end closes it. MCP's stdio +shutdown terminates the server's process tree, but a server that spawns a +longer-lived descendant escaping its process group keeps the inherited pipe +open — so an unbounded join would wedge session close (discovery failure, +reload, shutdown). After this timeout the read end is closed to force the +thread's `os.read` to fail and exit. +""" def _resolve_stdio_env( @@ -257,6 +270,7 @@ def __init__(self, server_name: str, *, encoding: str, errors: str) -> None: ) self._line = "" self._truncated = False + self._read_fd_closed = False self._read_fd, write_fd = os.pipe() try: self._writer = os.fdopen(write_fd, "wb", buffering=0) @@ -332,10 +346,34 @@ def close(self) -> None: self._writer.close() async def wait_closed(self) -> None: - """Wait off the event loop until the pipe reader reaches EOF.""" + """Wait off the event loop until the pipe reader reaches EOF. + + The join is bounded: if the drain thread is still blocked after + `_MCP_STDERR_DRAIN_JOIN_TIMEOUT` (the pipe's write end is held open by a + surviving server descendant), the read end is closed to force the + thread's `os.read` to fail, then the thread is rejoined. This keeps a + leaked stderr pipe from hanging session cleanup. + """ self.close() + await asyncio.to_thread(self._thread.join, _MCP_STDERR_DRAIN_JOIN_TIMEOUT) + if not self._thread.is_alive(): + return + self._close_read_fd() + logger.debug( + "MCP server %r stderr pipe still held open after process exit; " + "forcing the drain thread closed", + self._server_name, + ) await asyncio.to_thread(self._thread.join) + def _close_read_fd(self) -> None: + """Close the pipe read end once, from the reader thread or a closer.""" + if self._read_fd_closed: + return + self._read_fd_closed = True + with contextlib.suppress(OSError): + os.close(self._read_fd) + def _drain(self) -> None: """Drain subprocess bytes so stderr can never block the child.""" try: @@ -347,14 +385,16 @@ def _drain(self) -> None: if self._line or self._truncated: self._emit_line() except OSError as exc: - if self._capture: + # EBADF is the expected teardown path when wait_closed force-closes + # the read end while the thread is blocked on a leaked pipe. + if self._capture and exc.errno != errno.EBADF: logger.debug( "MCP server %r stderr capture failed: %s", self._server_name, exc, ) finally: - os.close(self._read_fd) + self._close_read_fd() def _decode(self, data: bytes, *, final: bool = False) -> None: """Decode one byte chunk without allowing malformed stderr to stop draining.""" diff --git a/libs/code/tests/unit_tests/test_mcp_tools.py b/libs/code/tests/unit_tests/test_mcp_tools.py index 66490630824..a53f61b7e13 100644 --- a/libs/code/tests/unit_tests/test_mcp_tools.py +++ b/libs/code/tests/unit_tests/test_mcp_tools.py @@ -939,6 +939,54 @@ def stderr_messages() -> list[str]: for thread in threading.enumerate() ) + async def test_stderr_drain_does_not_hang_on_inherited_pipe( + self, + tmp_path: Path, + ) -> None: + """A surviving descendant holding stderr cannot wedge session close. + + The child keeps its stderr write end open after the server exits, so + the pipe never reaches EOF. `wait_closed` must give up on the bounded + join and force the drain thread closed rather than block forever. + """ + started = tmp_path / "started" + # The child duplicates the inherited stderr fd and sleeps; the server + # exits as soon as its stdin closes, leaving the child holding the pipe. + spawn = ( + "subprocess.Popen(" + "[sys.executable, '-c', 'import time; time.sleep(60)'], " + "stderr=err, start_new_session=True)" + ) + script = "\n".join( + [ + "import os, subprocess, sys", + f"open({str(started)!r}, 'w').close()", + "err = os.dup(2)", + spawn, + "sys.stdin.buffer.read()", + ] + ) + connection = cast( + "Connection", + { + "transport": "stdio", + "command": sys.executable, + "args": ["-c", script], + }, + ) + async with _create_mcp_session(connection, server_name="fake"): + for _ in range(100): + if await asyncio.to_thread(started.exists): + break + await asyncio.sleep(0.01) + assert started.exists() + # Reaching here means __aexit__ (and wait_closed) returned instead of + # hanging on the leaked pipe. + assert not any( + thread.name == "mcp-stderr-fake" and thread.is_alive() + for thread in threading.enumerate() + ) + async def test_remote_session_delegates_to_adapter(self) -> None: """Non-stdio transports retain the adapter's session handling.""" session = AsyncMock() From be3000337ac91b3f1029ca0e09786393fe2e5828 Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Tue, 18 Aug 2026 22:16:50 -0400 Subject: [PATCH 03/18] fix(code): restrict debug log to the current user on Windows os.open mode bits and chmod are no-ops on Windows DACLs, so the debug log (which captures MCP stderr that may contain credentials) stayed readable by other local users despite the documented user-only guarantee. On Windows, replace the file DACL via ctypes/advapi32 with one granting full control to the current user only; POSIX keeps the 0o600 path. Failures downgrade to a warning so logging still attaches. --- libs/code/deepagents_code/_debug.py | 166 ++++++++++++++++++++++- libs/code/tests/unit_tests/test_debug.py | 37 +++++ 2 files changed, 202 insertions(+), 1 deletion(-) diff --git a/libs/code/deepagents_code/_debug.py b/libs/code/deepagents_code/_debug.py index 61e30b62d93..d335fb01ebc 100644 --- a/libs/code/deepagents_code/_debug.py +++ b/libs/code/deepagents_code/_debug.py @@ -8,11 +8,15 @@ from __future__ import annotations +import ctypes import logging import os import sys from pathlib import Path +if os.name == "nt": # Windows-only ACL structures; see _set_windows_owner_only_dacl. + from ctypes import wintypes + from deepagents_code._env_vars import ( DEBUG, DEBUG_FILE, @@ -40,10 +44,18 @@ def _prepare_debug_file(path: Path) -> None: - """Create or tighten a debug file before attaching the logging handler.""" + """Create or tighten a debug file before attaching the logging handler. + + On POSIX the file is created/tightened to mode `0o600`. On Windows, where + `os.open` mode bits and `chmod` do not tighten the DACL, the file's DACL is + replaced with one granting full control to the current user only. + """ flags = os.O_APPEND | os.O_CREAT | os.O_WRONLY | getattr(os, "O_NOFOLLOW", 0) fd = os.open(path, flags, 0o600) try: + if os.name == "nt": + _set_windows_owner_only_dacl(path) + return fchmod = getattr(os, "fchmod", None) if fchmod is None: path.chmod(0o600) @@ -53,6 +65,150 @@ def _prepare_debug_file(path: Path) -> None: os.close(fd) +def _set_windows_owner_only_dacl(path: Path) -> None: + """Restrict `path` to the current user on Windows. + + This is a no-op on POSIX, where `_prepare_debug_file` uses mode `0o600` + instead. The Windows implementation (defined only when `os.name == "nt"`) + replaces the file's DACL with one granting full control to the current user + and no one else. + + On Windows this raises `WinError` if the DACL cannot be built or applied; + `_prepare_debug_file` surfaces that as an `OSError` warning. + + Args: + path: Debug log file to lock down. + """ + if os.name != "nt": + return + _apply_windows_owner_only_dacl(path) + + +if os.name == "nt": + # --- Windows user-only DACL --------------------------------------------- + # Structures and helpers mirroring the advapi32 API used to build and apply + # a DACL granting the current user full control and no one else any access. + + _SE_FILE_OBJECT = 1 + _DACL_SECURITY_INFORMATION = 0x00000004 + _PROTECTED_DACL_SECURITY_INFORMATION = 0x80000000 + _TOKEN_QUERY = 0x0008 + _TOKEN_USER_INFORMATION_CLASS = 1 + _FILE_GENERIC_READ = 0x120089 + _FILE_GENERIC_WRITE = 0x120116 + + class _TRUSTEE_W(ctypes.Structure): # noqa: N801 # mirrors Win32 TRUSTEE_W + """`TRUSTEE_W` identifying the current-user SID to `SetEntriesInAclW`.""" + + _fields_ = [ + ("pMultipleTrustee", ctypes.c_void_p), + ("MultipleTrusteeOperation", ctypes.c_int), + ("TrusteeForm", ctypes.c_int), + ("TrusteeType", ctypes.c_int), + ("ptstrName", ctypes.c_void_p), + ] + + class _EXPLICIT_ACCESS_W(ctypes.Structure): # noqa: N801 # mirrors Win32 type + """`EXPLICIT_ACCESS_W` describing one access-control entry.""" + + _fields_ = [ + ("grfAccessPermissions", wintypes.DWORD), + ("grfAccessMode", ctypes.c_int), + ("grfInheritance", wintypes.DWORD), + ("Trustee", _TRUSTEE_W), + ] + + def _get_current_user_sid() -> ctypes.c_void_p: + """Return a pointer to the current user's SID. + + The SID buffer is kept alive on the returned pointer's referrer so it + stays valid for the duration of the DACL construction. + + Returns: + A pointer to the current user's SID. + + Raises: + WinError: If the process token or user SID cannot be read. + """ + advapi32 = ctypes.windll.advapi32 + token = wintypes.HANDLE() + if not advapi32.OpenProcessToken( + ctypes.windll.kernel32.GetCurrentProcess(), + _TOKEN_QUERY, + ctypes.byref(token), + ): + raise ctypes.WinError() # surface the raw OS error + try: + needed = wintypes.DWORD(0) + advapi32.GetTokenInformation( + token, _TOKEN_USER_INFORMATION_CLASS, None, 0, ctypes.byref(needed) + ) + if not needed.value: + raise ctypes.WinError() + buffer = (ctypes.c_byte * needed.value)() + if not advapi32.GetTokenInformation( + token, + _TOKEN_USER_INFORMATION_CLASS, + buffer, + needed, + ctypes.byref(needed), + ): + raise ctypes.WinError() + # TOKEN_USER begins with a single pointer to the user's SID. + sid = ctypes.cast(buffer, ctypes.POINTER(ctypes.c_void_p)).contents + # Keep the backing buffer alive by attaching it to the pointer object. + sid._buffer = buffer # type: ignore[attr-defined] + return sid + finally: + ctypes.windll.kernel32.CloseHandle(token) + + def _apply_windows_owner_only_dacl(path: Path) -> None: + """Replace `path`'s DACL with one granting the current user full control. + + Args: + path: Debug log file to lock down. + + Raises: + WinError: If the DACL cannot be built or applied. + """ + advapi32 = ctypes.windll.advapi32 + sid = _get_current_user_sid() + + trustee = _TRUSTEE_W( + pMultipleTrustee=None, + MultipleTrusteeOperation=0, # NO_MULTIPLE_TRUSTEE + TrusteeForm=2, # TRUSTEE_IS_SID + TrusteeType=0, # TRUSTEE_IS_USER + ptstrName=ctypes.cast(sid, ctypes.c_void_p).value, + ) + explicit = _EXPLICIT_ACCESS_W( + grfAccessPermissions=_FILE_GENERIC_READ | _FILE_GENERIC_WRITE, + grfAccessMode=2, # SET_ACCESS + grfInheritance=0, # NO_INHERITANCE + Trustee=trustee, + ) + new_acl = ctypes.c_void_p() + result = advapi32.SetEntriesInAclW( + 1, ctypes.byref(explicit), None, ctypes.byref(new_acl) + ) + if result != 0: # ERROR_SUCCESS + raise ctypes.WinError(result) + try: + apply_result = advapi32.SetNamedSecurityInfoW( + str(path), + _SE_FILE_OBJECT, + _DACL_SECURITY_INFORMATION | _PROTECTED_DACL_SECURITY_INFORMATION, + None, + None, + new_acl, + None, + ) + if apply_result != 0: # ERROR_SUCCESS + raise ctypes.WinError(apply_result) + finally: + ctypes.windll.kernel32.LocalFree(new_acl) + + def resolve_log_level(*, debug_enabled: bool | None = None) -> int: """Resolve the configured runtime logging level. @@ -129,6 +285,14 @@ def configure_debug_logging(target: logging.Logger) -> None: try: _prepare_debug_file(debug_path) + except OSError as exc: + logger.warning( + "could not restrict debug log file %s to the current user: %s. " + "Captured MCP stderr may be readable by other local users.", + debug_path, + exc, + ) + try: handler = logging.FileHandler(str(debug_path), mode="a") except OSError as exc: message = f"could not open debug log file {debug_path}: {exc}" diff --git a/libs/code/tests/unit_tests/test_debug.py b/libs/code/tests/unit_tests/test_debug.py index 0ce68979e4f..9ebfb6e1c90 100644 --- a/libs/code/tests/unit_tests/test_debug.py +++ b/libs/code/tests/unit_tests/test_debug.py @@ -6,9 +6,17 @@ import logging import os import stat +import sys from unittest.mock import patch +import pytest + +if sys.platform == "win32": + import win32api + import win32security + import deepagents_code +from deepagents_code import _debug from deepagents_code._debug import ( configure_debug_logging, installed_debug_log_path, @@ -80,6 +88,35 @@ def test_adds_handler_when_env_set(self, tmp_path) -> None: h.close() logger.removeHandler(h) + @pytest.mark.skipif(sys.platform != "win32", reason="Windows ACL hardening") + def test_debug_file_dacl_grants_current_user_only(self, tmp_path) -> None: + """On Windows the debug file DACL is restricted to the current user.""" + log_file = tmp_path / "debug.log" + log_file.touch() + + _debug._prepare_debug_file(log_file) + + current_user, _, _ = win32security.LookupAccountName( + None, win32api.GetUserName() + ) + sd = win32security.GetFileSecurity( + str(log_file), win32security.DACL_SECURITY_INFORMATION + ) + dacl = sd.GetSecurityDescriptorDacl() + assert dacl is not None + assert dacl.GetAceCount() == 1 + ace = dacl.GetAce(0) + assert ace[2][0] == current_user + + @pytest.mark.skipif(os.name != "nt", reason="Windows-only code path") + def test_prepare_debug_file_routes_to_windows_acl(self, tmp_path) -> None: + """`os.name == 'nt'` selects the ACL path over the POSIX chmod path.""" + log_file = tmp_path / "debug.log" + with patch.object(_debug, "_set_windows_owner_only_dacl") as mock_acl: + _debug._prepare_debug_file(log_file) + mock_acl.assert_called_once_with(log_file) + assert log_file.exists() + def test_log_level_debug_enables_debug_without_file_handler(self) -> None: logger = logging.getLogger("test.debug.level_only") logger.handlers = [] From cb883acc50fe2e05419576ea70491c789d901195 Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Tue, 18 Aug 2026 23:11:04 -0400 Subject: [PATCH 04/18] fix(code): bound forced MCP stderr drain teardown --- libs/code/deepagents_code/mcp_tools.py | 17 +++++++++++---- libs/code/tests/unit_tests/test_mcp_tools.py | 23 ++++++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/libs/code/deepagents_code/mcp_tools.py b/libs/code/deepagents_code/mcp_tools.py index ae15557a9d3..68257c9c227 100644 --- a/libs/code/deepagents_code/mcp_tools.py +++ b/libs/code/deepagents_code/mcp_tools.py @@ -217,7 +217,9 @@ class MCPConfigError(ValueError): longer-lived descendant escaping its process group keeps the inherited pipe open — so an unbounded join would wedge session close (discovery failure, reload, shutdown). After this timeout the read end is closed to force the -thread's `os.read` to fail and exit. +thread's `os.read` to fail and exit. The forced-close join is bounded too: +on Linux, closing a file descriptor from another thread does not reliably +interrupt an in-progress `os.read`. """ @@ -351,8 +353,9 @@ async def wait_closed(self) -> None: The join is bounded: if the drain thread is still blocked after `_MCP_STDERR_DRAIN_JOIN_TIMEOUT` (the pipe's write end is held open by a surviving server descendant), the read end is closed to force the - thread's `os.read` to fail, then the thread is rejoined. This keeps a - leaked stderr pipe from hanging session cleanup. + thread's `os.read` to fail, then the thread is rejoined with the same + bound. This keeps a leaked stderr pipe from hanging session cleanup + even where a cross-thread close does not interrupt `os.read`. """ self.close() await asyncio.to_thread(self._thread.join, _MCP_STDERR_DRAIN_JOIN_TIMEOUT) @@ -364,7 +367,13 @@ async def wait_closed(self) -> None: "forcing the drain thread closed", self._server_name, ) - await asyncio.to_thread(self._thread.join) + await asyncio.to_thread(self._thread.join, _MCP_STDERR_DRAIN_JOIN_TIMEOUT) + if self._thread.is_alive(): + logger.warning( + "MCP server %r stderr drain thread did not exit after forced " + "pipe close", + self._server_name, + ) def _close_read_fd(self) -> None: """Close the pipe read end once, from the reader thread or a closer.""" diff --git a/libs/code/tests/unit_tests/test_mcp_tools.py b/libs/code/tests/unit_tests/test_mcp_tools.py index a53f61b7e13..df63d079f15 100644 --- a/libs/code/tests/unit_tests/test_mcp_tools.py +++ b/libs/code/tests/unit_tests/test_mcp_tools.py @@ -27,6 +27,7 @@ from deepagents_code.mcp_auth import FileTokenStorage, MCPReauthRequiredError from deepagents_code.mcp_tools import ( + _MCP_STDERR_DRAIN_JOIN_TIMEOUT, _MCP_STDERR_LINE_LIMIT, _MCP_STDERR_TRUNCATION_MARKER, MCPServerInfo, @@ -39,6 +40,7 @@ _gather_bounded, _json_error_snippet, _load_tools_from_config, + _MCPStderrSink, _normalize_mcp_arguments, _warm_mcp_adapter_imports, classify_discovered_configs, @@ -866,6 +868,27 @@ def test_status_unauth_rejects_tools(self) -> None: class TestMCPStderrCapture: """Tests for stdio subprocess diagnostic capture.""" + async def test_stderr_drain_forced_close_join_is_bounded( + self, + caplog: pytest.LogCaptureFixture, + ) -> None: + """A blocked drain thread cannot make forced teardown wait forever.""" + sink = _MCPStderrSink("fake", encoding="utf-8", errors="strict") + reader_thread = sink._thread + blocked_thread = MagicMock() + blocked_thread.is_alive.return_value = True + sink._thread = blocked_thread + + with caplog.at_level(logging.WARNING, logger="deepagents_code.mcp_tools"): + await sink.wait_closed() + + assert blocked_thread.join.call_args_list == [ + ((_MCP_STDERR_DRAIN_JOIN_TIMEOUT,), {}), + ((_MCP_STDERR_DRAIN_JOIN_TIMEOUT,), {}), + ] + assert "stderr drain thread did not exit after forced pipe close" in caplog.text + await asyncio.to_thread(reader_thread.join) + async def test_stdio_stderr_is_buffered_decoded_and_bounded( self, tmp_path: Path, From 5b315b3a041b0987910d28fc43b4c85993ed0958 Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Wed, 19 Aug 2026 11:19:29 -0400 Subject: [PATCH 05/18] fix(code): grant the debug log DACL to the correct trustee MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `TrusteeForm=2` is `TRUSTEE_BAD_FORM`, not `TRUSTEE_IS_SID` (0), and `TrusteeType=0` is `TRUSTEE_IS_UNKNOWN`, not `TRUSTEE_IS_USER` (1). With the bad form, `SetEntriesInAclW` fails with `ERROR_INVALID_PARAMETER`, so `_apply_windows_owner_only_dacl` always raised and `configure_debug_logging` always fell through to its warning path. The debug log kept its inherited DACL on every Windows run — the one platform where the POSIX `chmod` cannot help. Promote the three `accctrl.h` enums to named constants. All of them start at 0 with unrelated meanings, so a transposed literal compiles fine and only fails at runtime; naming them keeps the mistake from recurring silently. --- libs/code/deepagents_code/_debug.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/libs/code/deepagents_code/_debug.py b/libs/code/deepagents_code/_debug.py index d335fb01ebc..36c259fda0a 100644 --- a/libs/code/deepagents_code/_debug.py +++ b/libs/code/deepagents_code/_debug.py @@ -96,6 +96,15 @@ def _set_windows_owner_only_dacl(path: Path) -> None: _TOKEN_USER_INFORMATION_CLASS = 1 _FILE_GENERIC_READ = 0x120089 _FILE_GENERIC_WRITE = 0x120116 + # `TRUSTEE_FORM` / `TRUSTEE_TYPE` / `ACCESS_MODE` from `accctrl.h`. Named + # rather than inlined because all three enums start at 0 with unrelated + # meanings, so a transposed literal still compiles and is rejected only at + # runtime by `SetEntriesInAclW`. + _NO_MULTIPLE_TRUSTEE = 0 + _TRUSTEE_IS_SID = 0 + _TRUSTEE_IS_USER = 1 + _SET_ACCESS = 2 + _NO_INHERITANCE = 0 class _TRUSTEE_W(ctypes.Structure): # noqa: N801 # mirrors Win32 TRUSTEE_W """`TRUSTEE_W` identifying the current-user SID to `SetEntriesInAclW`.""" @@ -176,15 +185,15 @@ def _apply_windows_owner_only_dacl(path: Path) -> None: trustee = _TRUSTEE_W( pMultipleTrustee=None, - MultipleTrusteeOperation=0, # NO_MULTIPLE_TRUSTEE - TrusteeForm=2, # TRUSTEE_IS_SID - TrusteeType=0, # TRUSTEE_IS_USER + MultipleTrusteeOperation=_NO_MULTIPLE_TRUSTEE, + TrusteeForm=_TRUSTEE_IS_SID, + TrusteeType=_TRUSTEE_IS_USER, ptstrName=ctypes.cast(sid, ctypes.c_void_p).value, ) explicit = _EXPLICIT_ACCESS_W( grfAccessPermissions=_FILE_GENERIC_READ | _FILE_GENERIC_WRITE, - grfAccessMode=2, # SET_ACCESS - grfInheritance=0, # NO_INHERITANCE + grfAccessMode=_SET_ACCESS, + grfInheritance=_NO_INHERITANCE, Trustee=trustee, ) new_acl = ctypes.c_void_p() From 555f5876fd238ab4b919ba6c7466f7252bbcb42f Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Wed, 19 Aug 2026 11:21:07 -0400 Subject: [PATCH 06/18] test(code): run deepagents-code tests on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Windows DACL hardening had no coverage anywhere: its only test is `skipif(sys.platform != "win32")` and `test-code` ran on ubuntu-latest only. That is why the `TRUSTEE_BAD_FORM` bug shipped unnoticed. Add a windows-latest leg, matching `test-deepagents`. Verify the DACL through `icacls` instead of pywin32, which was imported at module scope but declared in no dependency group — so on Windows the whole module failed at collection rather than skipping one test. Asserting a single entry naming the current user is what distinguishes an applied, protected DACL from an inherited one. Drop `test_prepare_debug_file_routes_to_windows_acl`: it patched a private helper and asserted the call rather than any behavior, and the real DACL test covers the same path. --- .github/workflows/ci.yml | 1 + libs/code/tests/unit_tests/test_debug.py | 54 +++++++++++++----------- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dd7a4da31aa..f843220ca5e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -291,6 +291,7 @@ jobs: with: working-directory: "libs/code" python-versions: '["3.12", "3.13", "3.14"]' + extra-configurations: '[{"python-version": "3.13", "os": "windows-latest"}]' coverage-python-version: "3.14" test-talon: diff --git a/libs/code/tests/unit_tests/test_debug.py b/libs/code/tests/unit_tests/test_debug.py index 9ebfb6e1c90..c0484720836 100644 --- a/libs/code/tests/unit_tests/test_debug.py +++ b/libs/code/tests/unit_tests/test_debug.py @@ -6,15 +6,12 @@ import logging import os import stat +import subprocess import sys from unittest.mock import patch import pytest -if sys.platform == "win32": - import win32api - import win32security - import deepagents_code from deepagents_code import _debug from deepagents_code._debug import ( @@ -24,6 +21,24 @@ ) +def _icacls_entries(path) -> list[str]: + """Return one string per access-control entry on `path`, via `icacls`. + + `icacls` prints ` `, then one indented ACE per line, then + a blank line and a summary. Only the ACE text is returned. + """ + completed = subprocess.run( + ["icacls", str(path)], + capture_output=True, + text=True, + check=True, + ) + head = completed.stdout.split("\n\n")[0] + first, *rest = head.splitlines() + entries = [first.removeprefix(str(path)), *rest] + return [entry.strip() for entry in entries if entry.strip()] + + class TestResolveLogLevel: def test_defaults_to_debug_when_debug_enabled(self) -> None: with patch.dict(os.environ, {}, clear=True): @@ -90,32 +105,21 @@ def test_adds_handler_when_env_set(self, tmp_path) -> None: @pytest.mark.skipif(sys.platform != "win32", reason="Windows ACL hardening") def test_debug_file_dacl_grants_current_user_only(self, tmp_path) -> None: - """On Windows the debug file DACL is restricted to the current user.""" + """On Windows the debug file DACL is restricted to the current user. + + A file that inherits its parent's DACL carries several entries + (`SYSTEM`, `Administrators`, the user). Exactly one entry, naming the + current user, is what proves the replacement DACL was applied and + marked protected so inherited entries were dropped. + """ log_file = tmp_path / "debug.log" log_file.touch() _debug._prepare_debug_file(log_file) - current_user, _, _ = win32security.LookupAccountName( - None, win32api.GetUserName() - ) - sd = win32security.GetFileSecurity( - str(log_file), win32security.DACL_SECURITY_INFORMATION - ) - dacl = sd.GetSecurityDescriptorDacl() - assert dacl is not None - assert dacl.GetAceCount() == 1 - ace = dacl.GetAce(0) - assert ace[2][0] == current_user - - @pytest.mark.skipif(os.name != "nt", reason="Windows-only code path") - def test_prepare_debug_file_routes_to_windows_acl(self, tmp_path) -> None: - """`os.name == 'nt'` selects the ACL path over the POSIX chmod path.""" - log_file = tmp_path / "debug.log" - with patch.object(_debug, "_set_windows_owner_only_dacl") as mock_acl: - _debug._prepare_debug_file(log_file) - mock_acl.assert_called_once_with(log_file) - assert log_file.exists() + aces = _icacls_entries(log_file) + assert len(aces) == 1, f"expected a single ACE, got {aces}" + assert os.environ["USERNAME"].lower() in aces[0].lower() def test_log_level_debug_enables_debug_without_file_handler(self) -> None: logger = logging.getLogger("test.debug.level_only") From 0c5893bd34829961563d32b6d47453a20e1c4bc0 Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Wed, 19 Aug 2026 11:22:29 -0400 Subject: [PATCH 07/18] fix(code): disable file logging when the debug log cannot be secured MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_prepare_debug_file` opens with `O_NOFOLLOW` so a symlink planted at the debug path is refused. That refusal was downgraded to a warning, and `logging.FileHandler` then reopened the same path without `O_NOFOLLOW` — following the symlink and appending through it. The default path is /tmp/deepagents_debug.log, so a blocked redirect became a successful one. The same fall-through applied to any other hardening failure, leaving captured MCP server stderr in a file of unknown permissions. Fail closed instead: warn on stderr and via the logger, then skip the file handler. The in-memory buffer still backs the Debug Console. --- libs/code/deepagents_code/_debug.py | 23 ++++++++--- libs/code/tests/unit_tests/test_debug.py | 50 ++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/libs/code/deepagents_code/_debug.py b/libs/code/deepagents_code/_debug.py index 36c259fda0a..a02dc1acaa6 100644 --- a/libs/code/deepagents_code/_debug.py +++ b/libs/code/deepagents_code/_debug.py @@ -267,6 +267,10 @@ def configure_debug_logging(target: logging.Logger) -> None: is reused and its level re-applied. If the resolved path changes, the stale handler is closed and replaced. + The file is created or tightened to user-only access first. If that fails, + no file handler is attached: captured MCP server stderr can carry + credentials, so no file log is safer than one that could not be secured. + Args: target: Logger to configure. """ @@ -295,12 +299,21 @@ def configure_debug_logging(target: logging.Logger) -> None: try: _prepare_debug_file(debug_path) except OSError as exc: - logger.warning( - "could not restrict debug log file %s to the current user: %s. " - "Captured MCP stderr may be readable by other local users.", - debug_path, - exc, + # Fail closed. `_prepare_debug_file` opens with `O_NOFOLLOW`, so this + # also fires for a symlink planted at `debug_path` — and the + # `FileHandler` below would happily follow it, turning a blocked + # redirect into a successful one. Captured MCP server stderr can carry + # credentials, so skip file logging entirely; the in-memory buffer + # still backs the Debug Console. + message = ( + f"could not restrict debug log file {debug_path} to the current " + f"user: {exc}. File logging is disabled because captured MCP " + f"server stderr may contain credentials. Set " + f"{DEBUG_FILE} to a path you own to enable it." ) + print(f"Warning: {message}", file=sys.stderr) # noqa: T201 + logger.warning("%s", message) + return try: handler = logging.FileHandler(str(debug_path), mode="a") except OSError as exc: diff --git a/libs/code/tests/unit_tests/test_debug.py b/libs/code/tests/unit_tests/test_debug.py index c0484720836..7ffa2f6c2e9 100644 --- a/libs/code/tests/unit_tests/test_debug.py +++ b/libs/code/tests/unit_tests/test_debug.py @@ -121,6 +121,56 @@ def test_debug_file_dacl_grants_current_user_only(self, tmp_path) -> None: assert len(aces) == 1, f"expected a single ACE, got {aces}" assert os.environ["USERNAME"].lower() in aces[0].lower() + def test_no_file_handler_when_hardening_fails(self, tmp_path, capsys) -> None: + """A file that cannot be secured gets no handler at all. + + Captured MCP server stderr can carry credentials, so failing to + restrict the file must disable file logging rather than fall through + and write to it anyway. + """ + logger = logging.getLogger("test.debug.harden_fail") + logger.handlers = [] + log_file = tmp_path / "debug.log" + with ( + patch.dict( + os.environ, + { + "DEEPAGENTS_CODE_DEBUG": "1", + "DEEPAGENTS_CODE_DEBUG_FILE": str(log_file), + }, + ), + patch.object(_debug, "_prepare_debug_file", side_effect=OSError("nope")), + ): + configure_debug_logging(logger) + assert not any(isinstance(h, logging.FileHandler) for h in logger.handlers) + assert "Warning" in capsys.readouterr().err + + @pytest.mark.skipif(os.name == "nt", reason="POSIX O_NOFOLLOW refusal") + def test_symlinked_debug_file_is_refused(self, tmp_path, capsys) -> None: + """A symlink at the debug path is refused, not followed. + + `/tmp` is the default location, so a planted symlink would otherwise + redirect captured MCP stderr into a file the attacker chose. + """ + logger = logging.getLogger("test.debug.symlink") + logger.handlers = [] + victim = tmp_path / "victim.log" + victim.touch() + link = tmp_path / "debug.log" + link.symlink_to(victim) + with patch.dict( + os.environ, + { + "DEEPAGENTS_CODE_DEBUG": "1", + "DEEPAGENTS_CODE_DEBUG_FILE": str(link), + }, + ): + configure_debug_logging(logger) + assert not any(isinstance(h, logging.FileHandler) for h in logger.handlers) + assert "Warning" in capsys.readouterr().err + logger.warning("must not be written through the symlink") + assert victim.read_text() == "" + def test_log_level_debug_enables_debug_without_file_handler(self) -> None: logger = logging.getLogger("test.debug.level_only") logger.handlers = [] From c82a66700ca747accc5c48d58df41021376164d2 Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Wed, 19 Aug 2026 11:23:21 -0400 Subject: [PATCH 08/18] perf(code): import ctypes only on Windows `_debug` is imported from `deepagents_code/__init__.py`, so it runs on every command including `dcode -v`. `ctypes` costs a few milliseconds and pulls in `struct`, and every consumer of it sits inside `if os.name == "nt"`. Move it under the existing guard, per the startup-performance rule in AGENTS.md. --- libs/code/deepagents_code/_debug.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libs/code/deepagents_code/_debug.py b/libs/code/deepagents_code/_debug.py index a02dc1acaa6..834abb45d4a 100644 --- a/libs/code/deepagents_code/_debug.py +++ b/libs/code/deepagents_code/_debug.py @@ -8,13 +8,16 @@ from __future__ import annotations -import ctypes import logging import os import sys from pathlib import Path -if os.name == "nt": # Windows-only ACL structures; see _set_windows_owner_only_dacl. +# Windows-only ACL plumbing; see `_apply_windows_owner_only_dacl`. Imported +# under the guard because `_debug` is on the startup path for every command and +# `ctypes` costs a few milliseconds it can never repay on POSIX. +if os.name == "nt": + import ctypes from ctypes import wintypes from deepagents_code._env_vars import ( From 373a435dcb851b017da98ede0e10118aec96f042 Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Wed, 19 Aug 2026 11:24:53 -0400 Subject: [PATCH 09/18] fix(code): serialize the MCP stderr pipe close across threads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_close_read_fd` did an unlocked check-then-set on `_read_fd_closed` while being called from two threads: the drain thread's `finally` and `wait_closed`. `Thread.is_alive()` stays true while the thread runs its `finally`, so both callers could pass the check and close the same fd twice. Between the two closes the fd number is free, so the second close could reap a descriptor another thread had since opened — and `suppress(OSError)` guaranteed it went unreported. Hold `_fd_lock` across the check and the close, and warn instead of suppressing. Add a `_stopping` event set before the force-close. Closing a descriptor does not reliably interrupt a blocked `os.read`: on darwin the read returns EOF, on Linux it can stay parked. Either way the fd number is already released, so the drain loop must not issue another read against it. --- libs/code/deepagents_code/mcp_tools.py | 41 +++++++++++++++++++++----- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/libs/code/deepagents_code/mcp_tools.py b/libs/code/deepagents_code/mcp_tools.py index 68257c9c227..b3b45c30dec 100644 --- a/libs/code/deepagents_code/mcp_tools.py +++ b/libs/code/deepagents_code/mcp_tools.py @@ -10,7 +10,6 @@ import asyncio import codecs -import contextlib import copy import errno import fnmatch @@ -273,6 +272,12 @@ def __init__(self, server_name: str, *, encoding: str, errors: str) -> None: self._line = "" self._truncated = False self._read_fd_closed = False + # `_read_fd` is closed from the drain thread and from `wait_closed`; + # `_fd_lock` makes the close-once check atomic across the two. + self._fd_lock = threading.Lock() + # Set before `wait_closed` force-closes the read end, so the drain + # thread never issues another read against a freed fd number. + self._stopping = threading.Event() self._read_fd, write_fd = os.pipe() try: self._writer = os.fdopen(write_fd, "wb", buffering=0) @@ -361,6 +366,7 @@ async def wait_closed(self) -> None: await asyncio.to_thread(self._thread.join, _MCP_STDERR_DRAIN_JOIN_TIMEOUT) if not self._thread.is_alive(): return + self._stopping.set() self._close_read_fd() logger.debug( "MCP server %r stderr pipe still held open after process exit; " @@ -376,17 +382,36 @@ async def wait_closed(self) -> None: ) def _close_read_fd(self) -> None: - """Close the pipe read end once, from the reader thread or a closer.""" - if self._read_fd_closed: - return - self._read_fd_closed = True - with contextlib.suppress(OSError): - os.close(self._read_fd) + """Close the pipe read end exactly once. + + Called from the drain thread's `finally` and from `wait_closed`, so the + flag check and the close are held under `_fd_lock`. Without it both + callers can pass an unlocked check and close twice, and between the two + closes the fd number is free for another thread to reuse — so the second + close would reap an unrelated descriptor. + """ + with self._fd_lock: + if self._read_fd_closed: + return + self._read_fd_closed = True + try: + os.close(self._read_fd) + except OSError as exc: + logger.warning( + "MCP server %r stderr pipe close failed: %s", + self._server_name, + exc, + ) def _drain(self) -> None: """Drain subprocess bytes so stderr can never block the child.""" try: - while chunk := os.read(self._read_fd, _MCP_STDERR_READ_SIZE): + # Re-check before every read: once `wait_closed` has force-closed + # the read end, the fd number may already belong to another file. + while not self._stopping.is_set(): + chunk = os.read(self._read_fd, _MCP_STDERR_READ_SIZE) + if not chunk: + break if self._capture: self._decode(chunk) if self._capture: From 919158f7a6b70e39c94e26d9552909e82bc94219 Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Wed, 19 Aug 2026 11:25:54 -0400 Subject: [PATCH 10/18] fix(code): report MCP stderr drain failures when capture is off MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Capture requires DEBUG, so the default runtime configuration had `_capture` false — and there `except OSError` discarded every drain failure with no record at any level. Draining is the half that must always work: once the pipe buffer fills, the server blocks forever on its next stderr write and nothing explains why. Log at WARNING regardless of capture, since a stopped drain is a server-liveness problem rather than a logging one. Add an `except Exception` backstop. A `MemoryError` or a future `TypeError` in the decode path escaped into `threading.excepthook`, which is invisible in the TUI, and left the child with nobody reading its stderr. --- libs/code/deepagents_code/mcp_tools.py | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/libs/code/deepagents_code/mcp_tools.py b/libs/code/deepagents_code/mcp_tools.py index b3b45c30dec..19df70ef39a 100644 --- a/libs/code/deepagents_code/mcp_tools.py +++ b/libs/code/deepagents_code/mcp_tools.py @@ -404,7 +404,13 @@ def _close_read_fd(self) -> None: ) def _drain(self) -> None: - """Drain subprocess bytes so stderr can never block the child.""" + """Read subprocess bytes so the server does not block on its stderr pipe. + + Draining is unconditional; `_capture` only decides whether the bytes are + also logged. A drain that stops early while the server is still running + lets the pipe buffer fill and blocks the server's next write, so a + failure here is reported at `WARNING` even when capture is off. + """ try: # Re-check before every read: once `wait_closed` has force-closed # the read end, the fd number may already belong to another file. @@ -419,14 +425,22 @@ def _drain(self) -> None: if self._line or self._truncated: self._emit_line() except OSError as exc: - # EBADF is the expected teardown path when wait_closed force-closes - # the read end while the thread is blocked on a leaked pipe. - if self._capture and exc.errno != errno.EBADF: - logger.debug( - "MCP server %r stderr capture failed: %s", + # EBADF covers the narrow race where `wait_closed` closes the read + # end between the `_stopping` check and the `os.read`. + if exc.errno != errno.EBADF: + logger.warning( + "MCP server %r stderr drain stopped: %s. The server may " + "block if it fills its stderr pipe.", self._server_name, exc, ) + except Exception: # a dead drain thread blocks the server + # Without this the exception goes to `threading.excepthook`, which + # is invisible in the TUI, and nothing drains the child's stderr. + logger.exception( + "MCP server %r stderr drain failed unexpectedly", + self._server_name, + ) finally: self._close_read_fd() From 8a5bbd8e148cf05266e29bbd6e0246cb8841504b Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Wed, 19 Aug 2026 11:29:35 -0400 Subject: [PATCH 11/18] fix(code): decode captured MCP stderr with the replace handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The capture decoder took the server's `encoding_error_handler`, which defaults to `strict`. One stray non-UTF-8 byte on stderr therefore raised `UnicodeDecodeError` and discarded the whole 8 KiB read chunk — losing exactly the diagnostic this capture exists to provide. Nothing parses captured stderr, so a mangled character is strictly better than a dropped chunk. Keep the server's handler for the protocol stream only. Flush the buffered line before resetting the decoder on the remaining defensive path, so a reader sees a truncated line rather than text spliced across the discarded bytes. Add the missing behavioral tests: a byte invalid for the declared encoding, a line of exactly `_MCP_STDERR_LINE_LIMIT`, and a below-DEBUG drain of far more than a pipe buffer holds. All three fail against a deliberately reintroduced regression. --- libs/code/deepagents_code/mcp_tools.py | 19 ++++++- libs/code/tests/unit_tests/test_mcp_tools.py | 60 ++++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/libs/code/deepagents_code/mcp_tools.py b/libs/code/deepagents_code/mcp_tools.py index 19df70ef39a..f97363fb5b4 100644 --- a/libs/code/deepagents_code/mcp_tools.py +++ b/libs/code/deepagents_code/mcp_tools.py @@ -207,6 +207,15 @@ class MCPConfigError(ValueError): _MCP_STDERR_LINE_LIMIT = 4096 _MCP_STDERR_READ_SIZE = 8192 _MCP_STDERR_TRUNCATION_MARKER = "... [truncated]" +_MCP_STDERR_CAPTURE_ERRORS = "replace" +"""Encoding error handler for the stderr *capture* decoder. + +Deliberately independent of the server's `encoding_error_handler`, which +governs the protocol stream and defaults to `strict`. Nothing parses captured +stderr — it goes to a log — so one stray non-UTF-8 byte should mangle a +character, not discard the whole 8 KiB chunk it arrived in. Dropping the chunk +would lose exactly the diagnostic this capture exists to provide. +""" _MCP_STDERR_DRAIN_JOIN_TIMEOUT = 2.0 """Bound on joining the stderr drain thread during session teardown. @@ -265,7 +274,7 @@ def __init__(self, server_name: str, *, encoding: str, errors: str) -> None: self._errors = errors self._capture = logger.isEnabledFor(logging.DEBUG) self._decoder = ( - codecs.getincrementaldecoder(encoding)(errors=errors) + codecs.getincrementaldecoder(encoding)(errors=_MCP_STDERR_CAPTURE_ERRORS) if self._capture else None ) @@ -450,8 +459,14 @@ def _decode(self, data: bytes, *, final: bool = False) -> None: return try: text = self._decoder.decode(data, final=final) - except (LookupError, UnicodeError) as exc: + except UnicodeError as exc: + # Defensive: `_MCP_STDERR_CAPTURE_ERRORS` should keep this + # unreachable. Flush what was buffered before resetting, so a + # reader sees a truncated line rather than text silently spliced + # from either side of the discarded bytes. self._decoder.reset() + if self._line or self._truncated: + self._emit_line() logger.debug( "MCP server %r stderr decode failed with %s: %s", self._server_name, diff --git a/libs/code/tests/unit_tests/test_mcp_tools.py b/libs/code/tests/unit_tests/test_mcp_tools.py index df63d079f15..ff75de4646b 100644 --- a/libs/code/tests/unit_tests/test_mcp_tools.py +++ b/libs/code/tests/unit_tests/test_mcp_tools.py @@ -889,6 +889,66 @@ async def test_stderr_drain_forced_close_join_is_bounded( assert "stderr drain thread did not exit after forced pipe close" in caplog.text await asyncio.to_thread(reader_thread.join) + @staticmethod + def _stderr_messages(caplog: pytest.LogCaptureFixture) -> list[str]: + """Return the captured stderr log lines, without the record prefix.""" + marker = "stderr: " + return [ + record.getMessage().split(marker, 1)[1] + for record in caplog.records + if marker in record.getMessage() + ] + + async def test_malformed_bytes_do_not_discard_the_chunk( + self, + caplog: pytest.LogCaptureFixture, + ) -> None: + """A byte invalid for the declared encoding mangles one character only. + + The capture decoder must not inherit the protocol's `strict` handler, + which would raise and drop the whole read chunk — losing the diagnostic + the capture exists to provide. + """ + with caplog.at_level(logging.DEBUG, logger="deepagents_code.mcp_tools"): + sink = _MCPStderrSink("fake", encoding="utf-8", errors="strict") + os.write(sink.fileno(), b"before \xff after\n") + await sink.wait_closed() + + assert self._stderr_messages(caplog) == ["before \ufffd after"] + + async def test_line_at_the_limit_is_not_truncated( + self, + caplog: pytest.LogCaptureFixture, + ) -> None: + """A line of exactly the limit keeps every character and no marker.""" + with caplog.at_level(logging.DEBUG, logger="deepagents_code.mcp_tools"): + sink = _MCPStderrSink("fake", encoding="utf-8", errors="strict") + os.write(sink.fileno(), b"a" * _MCP_STDERR_LINE_LIMIT + b"\n") + await sink.wait_closed() + + assert self._stderr_messages(caplog) == ["a" * _MCP_STDERR_LINE_LIMIT] + + async def test_drain_consumes_stderr_when_capture_is_off( + self, + caplog: pytest.LogCaptureFixture, + ) -> None: + """Below DEBUG the pipe is still drained, so the server cannot block. + + Writing far more than a pipe buffer holds would block forever if the + drain thread skipped reading when capture is disabled. + """ + with caplog.at_level(logging.INFO, logger="deepagents_code.mcp_tools"): + sink = _MCPStderrSink("fake", encoding="utf-8", errors="strict") + payload = b"x" * (1 << 20) + written = 0 + while written < len(payload): + written += await asyncio.to_thread( + os.write, sink.fileno(), payload[written:] + ) + await sink.wait_closed() + + assert self._stderr_messages(caplog) == [] + async def test_stdio_stderr_is_buffered_decoded_and_bounded( self, tmp_path: Path, From a047d9f1d46311e8f75751acbaf7468412fd925d Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Wed, 19 Aug 2026 11:31:06 -0400 Subject: [PATCH 12/18] refactor(code): drop the unreachable MCP stderr write path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `mcp` hands `errlog` to `anyio.open_process(stderr=...)` on POSIX and to `create_windows_process` on Windows; both consume only `fileno()`. So `write`, `flush`, `writable`, `encoding` and `errors` were never called — about thirty untested lines, including a `written is None` busy-spin that would have burned a core had anything reached it. Remove them, along with the now-unused `errors` constructor argument; the server's handler still governs the protocol stream via `StdioServerParameters`. `io.TextIOBase` stays for its `closed` bookkeeping and the `TextIO` cast the transport signature needs. Say so in the class docstring, together with the reader thread's two jobs and the fact that capture latches at construction. --- libs/code/deepagents_code/mcp_tools.py | 72 +++++++------------- libs/code/tests/unit_tests/test_mcp_tools.py | 8 +-- 2 files changed, 28 insertions(+), 52 deletions(-) diff --git a/libs/code/deepagents_code/mcp_tools.py b/libs/code/deepagents_code/mcp_tools.py index f97363fb5b4..75a9f6d5d25 100644 --- a/libs/code/deepagents_code/mcp_tools.py +++ b/libs/code/deepagents_code/mcp_tools.py @@ -264,14 +264,32 @@ def _resolve_stdio_env( class _MCPStderrSink(io.TextIOBase): - """Forward a subprocess pipe to the MCP logger one bounded line at a time.""" + """A pipe that the MCP stdio transport can use as a server's stderr. + + The transport only ever calls `fileno()` on the object it is handed, then + passes that descriptor to the subprocess. So this is a pipe with a reader + attached, not a writable stream: `fileno()` and `close()` are the whole + consumed surface, and `io.TextIOBase` is here for the `closed` bookkeeping + plus the `TextIO` cast the transport signature requires. + + The reader thread has two jobs. It always drains, so a chatty server cannot + block writing to a full stderr pipe. It additionally logs whole, bounded, + sanitized lines to `deepagents_code.mcp_tools` at `DEBUG`, but only when + that level is enabled at construction time — raising the level afterwards + does not start capture on an existing sink. + """ + + def __init__(self, server_name: str, *, encoding: str) -> None: + """Create the pipe and start its reader. - def __init__(self, server_name: str, *, encoding: str, errors: str) -> None: - """Create the pipe and start its reader.""" + Args: + server_name: MCP server name used in log records. + encoding: Encoding used to decode captured stderr bytes. The + error handler is always `_MCP_STDERR_CAPTURE_ERRORS`. + """ super().__init__() self._server_name = server_name self._encoding = encoding - self._errors = errors self._capture = logger.isEnabledFor(logging.DEBUG) self._decoder = ( codecs.getincrementaldecoder(encoding)(errors=_MCP_STDERR_CAPTURE_ERRORS) @@ -306,52 +324,10 @@ def __init__(self, server_name: str, *, encoding: str, errors: str) -> None: os.close(self._read_fd) raise - @property - def encoding(self) -> str: - """Encoding used for writes and captured bytes.""" - return self._encoding - - @property - def errors(self) -> str: - """Configured encoding error handler.""" - return self._errors - def fileno(self) -> int: - """Return the subprocess-inheritable write descriptor.""" + """Return the write descriptor to hand to the server subprocess.""" return self._writer.fileno() - def writable(self) -> bool: - """Return whether the parent write descriptor remains open.""" - return not self.closed - - def write(self, text: str) -> int: - """Write text through the pipe for TextIO compatibility. - - Args: - text: Text to encode and write. - - Returns: - Number of input characters written. - - Raises: - ValueError: If the sink is closed. - """ - if self.closed: - msg = "I/O operation on closed MCP stderr sink" - raise ValueError(msg) - data = memoryview(text.encode(self._encoding, errors=self._errors)) - while data: - written = self._writer.write(data) - if written is None: - continue - data = data[written:] - return len(text) - - def flush(self) -> None: - """Flush parent writes before closing the descriptor.""" - if not self._writer.closed: - self._writer.flush() - def close(self) -> None: """Close the parent's copy of the subprocess write descriptor.""" if self.closed: @@ -540,7 +516,7 @@ async def _create_mcp_session( encoding=encoding, encoding_error_handler=errors, ) - sink = _MCPStderrSink(server_name, encoding=encoding, errors=errors) + sink = _MCPStderrSink(server_name, encoding=encoding) try: async with stdio_client(params, errlog=cast("TextIO", sink)) as (read, write): sink.close() diff --git a/libs/code/tests/unit_tests/test_mcp_tools.py b/libs/code/tests/unit_tests/test_mcp_tools.py index ff75de4646b..79fcd8fcc1e 100644 --- a/libs/code/tests/unit_tests/test_mcp_tools.py +++ b/libs/code/tests/unit_tests/test_mcp_tools.py @@ -873,7 +873,7 @@ async def test_stderr_drain_forced_close_join_is_bounded( caplog: pytest.LogCaptureFixture, ) -> None: """A blocked drain thread cannot make forced teardown wait forever.""" - sink = _MCPStderrSink("fake", encoding="utf-8", errors="strict") + sink = _MCPStderrSink("fake", encoding="utf-8") reader_thread = sink._thread blocked_thread = MagicMock() blocked_thread.is_alive.return_value = True @@ -910,7 +910,7 @@ async def test_malformed_bytes_do_not_discard_the_chunk( the capture exists to provide. """ with caplog.at_level(logging.DEBUG, logger="deepagents_code.mcp_tools"): - sink = _MCPStderrSink("fake", encoding="utf-8", errors="strict") + sink = _MCPStderrSink("fake", encoding="utf-8") os.write(sink.fileno(), b"before \xff after\n") await sink.wait_closed() @@ -922,7 +922,7 @@ async def test_line_at_the_limit_is_not_truncated( ) -> None: """A line of exactly the limit keeps every character and no marker.""" with caplog.at_level(logging.DEBUG, logger="deepagents_code.mcp_tools"): - sink = _MCPStderrSink("fake", encoding="utf-8", errors="strict") + sink = _MCPStderrSink("fake", encoding="utf-8") os.write(sink.fileno(), b"a" * _MCP_STDERR_LINE_LIMIT + b"\n") await sink.wait_closed() @@ -938,7 +938,7 @@ async def test_drain_consumes_stderr_when_capture_is_off( drain thread skipped reading when capture is disabled. """ with caplog.at_level(logging.INFO, logger="deepagents_code.mcp_tools"): - sink = _MCPStderrSink("fake", encoding="utf-8", errors="strict") + sink = _MCPStderrSink("fake", encoding="utf-8") payload = b"x" * (1 << 20) written = 0 while written < len(payload): From a2d93e564d9e2c4e9420a01996cae0b3e0ea9853 Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Wed, 19 Aug 2026 11:32:57 -0400 Subject: [PATCH 13/18] refactor(code): drop the redundant stdio env resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_build_connection` already runs `resolve_mcp_server_env` over `env` before the connection is built, and `_interpolate_env` there handles both `${VAR}` and `${VAR:-default}` and raises on an unset reference. Every connection reaching `_create_mcp_session` — discovery and `MCPSessionManager` alike — comes from that path, so `_resolve_stdio_env` could not substitute anything. Its "unexpanded variable reference" warning was reachable only as a false positive on a value that legitimately resolved to text containing `${`, and its only other effect was a second regex pass over resolved secrets. The stderr capture test hand-built a connection with `${MCP_TEST_ENV}`, which was the sole caller relying on the second pass. Pass the resolved value the way production does; the test's subject is stderr decoding either way. --- libs/code/deepagents_code/mcp_tools.py | 40 +++----------------- libs/code/tests/unit_tests/test_mcp_tools.py | 7 +--- 2 files changed, 8 insertions(+), 39 deletions(-) diff --git a/libs/code/deepagents_code/mcp_tools.py b/libs/code/deepagents_code/mcp_tools.py index 75a9f6d5d25..ae7f632eb43 100644 --- a/libs/code/deepagents_code/mcp_tools.py +++ b/libs/code/deepagents_code/mcp_tools.py @@ -203,7 +203,6 @@ class MCPConfigError(ValueError): """ -_MCP_ENV_REFERENCE_RE = re.compile(r"\$\{([^}]+)\}") _MCP_STDERR_LINE_LIMIT = 4096 _MCP_STDERR_READ_SIZE = 8192 _MCP_STDERR_TRUNCATION_MARKER = "... [truncated]" @@ -231,38 +230,6 @@ class MCPConfigError(ValueError): """ -def _resolve_stdio_env( - env: dict[str, str] | None, - *, - server_name: str, -) -> dict[str, str] | None: - """Resolve adapter-compatible braced environment references. - - Args: - env: Configured subprocess environment. - server_name: MCP server name used in warning records. - - Returns: - Resolved environment values, or `None` when no environment is configured. - """ - if env is None: - return None - resolved = { - key: _MCP_ENV_REFERENCE_RE.sub( - lambda match: os.environ.get(match.group(1), match.group(0)), value - ) - for key, value in env.items() - } - for key, value in resolved.items(): - if _MCP_ENV_REFERENCE_RE.search(value): - logger.warning( - "MCP server %r env[%r] contains an unexpanded variable reference", - server_name, - key, - ) - return resolved - - class _MCPStderrSink(io.TextIOBase): """A pipe that the MCP stdio transport can use as a server's stderr. @@ -511,7 +478,12 @@ async def _create_mcp_session( params = StdioServerParameters( command=stdio["command"], args=stdio["args"], - env=_resolve_stdio_env(stdio.get("env"), server_name=server_name), + # Already expanded: `_build_connection` runs `resolve_mcp_server_env` + # over `env` before the connection is built, with a richer grammar + # (`${VAR:-default}`) that raises on an unset reference. A second pass + # here could only re-scan resolved secrets and warn about a value that + # legitimately contains `${`. + env=stdio.get("env"), cwd=stdio.get("cwd"), encoding=encoding, encoding_error_handler=errors, diff --git a/libs/code/tests/unit_tests/test_mcp_tools.py b/libs/code/tests/unit_tests/test_mcp_tools.py index 79fcd8fcc1e..f5427ad86eb 100644 --- a/libs/code/tests/unit_tests/test_mcp_tools.py +++ b/libs/code/tests/unit_tests/test_mcp_tools.py @@ -978,7 +978,7 @@ async def test_stdio_stderr_is_buffered_decoded_and_bounded( "transport": "stdio", "command": sys.executable, "args": ["-c", script, str(first_written), str(release)], - "env": {"MCP_CHILD_VALUE": "${MCP_TEST_ENV}"}, + "env": {"MCP_CHILD_VALUE": "partial"}, "encoding": "utf-16-le", "encoding_error_handler": "strict", }, @@ -993,10 +993,7 @@ def stderr_messages() -> list[str]: and " stderr: " in record.getMessage() ] - with ( - patch.dict(os.environ, {"MCP_TEST_ENV": "partial"}), - caplog.at_level(logging.DEBUG, logger="deepagents_code.mcp_tools"), - ): + with caplog.at_level(logging.DEBUG, logger="deepagents_code.mcp_tools"): async with _create_mcp_session(connection, server_name="fake"): for _ in range(100): if await asyncio.to_thread(first_written.exists): From df09825b8b5abfa3a2b991ce954ea487016db518 Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Wed, 19 Aug 2026 11:35:49 -0400 Subject: [PATCH 14/18] docs(code): correct the DACL, teardown and sanitization claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Several docstrings described behavior the code does not have: - "full control" in four places. The ACE grants `FILE_GENERIC_READ | FILE_GENERIC_WRITE`; `DELETE` and `WRITE_DAC` are not granted. - `_set_windows_owner_only_dacl` credited `_prepare_debug_file` with catching the error. It has no handler; `configure_debug_logging` does. - `Raises: WinError` names a factory function, not a type. The object raised is an `OSError`, which is why the caller's `except OSError` works. - `_get_current_user_sid` explained the SID lifetime via a "referrer", which is not a ctypes concept, and presented the `_buffer` attribute as the sole mechanism when `.contents` already retains the array. - The drain-join comments asserted that closing the read end makes `os.read` fail. On darwin it returns EOF, and on Linux it may not return at all. - `_prepare_debug_file` documented neither its `OSError` contract nor why `O_NOFOLLOW` is there. Also record that the stdio branch of `_create_mcp_session` mirrors the adapter's `_create_stdio_session`, and why the parent's write end is closed inside the `stdio_client` context — an uncommented duplicate-looking close is an easy target for a cleanup that would reintroduce the teardown stall. DEVELOPMENT.md overstated the file permissions as a guarantee and implied the log is terminal-safe: only the ESC byte is stripped, so `[31m` survives as literal text. Rewrite the section in shorter sentences. --- libs/code/DEVELOPMENT.md | 6 ++- libs/code/deepagents_code/_debug.py | 55 +++++++++++++++++--------- libs/code/deepagents_code/mcp_tools.py | 42 +++++++++++++------- 3 files changed, 68 insertions(+), 35 deletions(-) diff --git a/libs/code/DEVELOPMENT.md b/libs/code/DEVELOPMENT.md index 2eb8590dda6..5f2b7710dd7 100644 --- a/libs/code/DEVELOPMENT.md +++ b/libs/code/DEVELOPMENT.md @@ -136,9 +136,11 @@ For problems that appear after the app is up, tail the client log in another ter tail -f /tmp/deepagents_debug.log ``` -To send it elsewhere, also `export DEEPAGENTS_CODE_DEBUG_FILE=`. The handler appends across runs, so a single file accumulates every session. The file is created with user-only permissions. +To send it elsewhere, also `export DEEPAGENTS_CODE_DEBUG_FILE=`. The handler appends across runs, so a single file accumulates every session. -Stdio MCP server stderr is captured here at `DEBUG` so server-side failures remain diagnosable when the TUI cannot display process stderr. Records are split into bounded lines and stripped of control characters, but the remaining text is otherwise server-provided and may contain credentials or other sensitive values. Only enable or share DEBUG logs with that risk in mind. +The file is created or tightened to user-only access. A symlink at the path is refused. If the file cannot be secured, no file handler is attached and a warning goes to stderr. Use the in-app Debug Console in that case. + +Stdio MCP server stderr is captured here at `DEBUG`. This keeps server-side failures visible when the TUI cannot show process stderr. Each record is one line, capped at 4096 characters. Characters in the Unicode `C` categories are removed, which includes control and format characters. The ESC byte of an ANSI sequence is removed but the rest stays as literal text, so the log is not free of escape-sequence residue. The text comes from the server. It can contain credentials or other sensitive values. Enable `DEBUG` logging only if you accept that risk, and do not share the log file. ### In-app Debug Console (`Ctrl+\`) diff --git a/libs/code/deepagents_code/_debug.py b/libs/code/deepagents_code/_debug.py index 834abb45d4a..8fef194032f 100644 --- a/libs/code/deepagents_code/_debug.py +++ b/libs/code/deepagents_code/_debug.py @@ -49,10 +49,18 @@ def _prepare_debug_file(path: Path) -> None: """Create or tighten a debug file before attaching the logging handler. - On POSIX the file is created/tightened to mode `0o600`. On Windows, where - `os.open` mode bits and `chmod` do not tighten the DACL, the file's DACL is - replaced with one granting full control to the current user only. - """ + On POSIX the file is created or tightened to mode `0o600`. On Windows, + where `os.open` mode bits and `chmod` do not tighten the DACL, the DACL is + replaced with one granting read and write access to the current user only. + + `O_NOFOLLOW` refuses a symlink at `path`. The default location is a + world-writable temp directory, so without it a planted symlink could + redirect captured MCP server stderr into a file of the attacker's choosing. + + Raises: + OSError: If the file cannot be created, opened, or tightened. The + caller must treat this as fatal to file logging. + """ # noqa: DOC502 - raised by os.open/fchmod, not by an explicit raise flags = os.O_APPEND | os.O_CREAT | os.O_WRONLY | getattr(os, "O_NOFOLLOW", 0) fd = os.open(path, flags, 0o600) try: @@ -73,15 +81,17 @@ def _set_windows_owner_only_dacl(path: Path) -> None: This is a no-op on POSIX, where `_prepare_debug_file` uses mode `0o600` instead. The Windows implementation (defined only when `os.name == "nt"`) - replaces the file's DACL with one granting full control to the current user - and no one else. - - On Windows this raises `WinError` if the DACL cannot be built or applied; - `_prepare_debug_file` surfaces that as an `OSError` warning. + replaces the file's DACL with one granting read and write access to the + current user and no one else. Args: path: Debug log file to lock down. - """ + + Raises: + OSError: If the DACL cannot be built or applied. `_prepare_debug_file` + propagates it; `configure_debug_logging` catches it and disables + file logging. + """ # noqa: DOC502 - raised by the callee, not by an explicit raise if os.name != "nt": return _apply_windows_owner_only_dacl(path) @@ -90,7 +100,9 @@ def _set_windows_owner_only_dacl(path: Path) -> None: if os.name == "nt": # --- Windows user-only DACL --------------------------------------------- # Structures and helpers mirroring the advapi32 API used to build and apply - # a DACL granting the current user full control and no one else any access. + # a DACL granting the current user read and write access, and no one else + # any access. `DELETE` and `WRITE_DAC` are deliberately not granted; the + # file owner retains them implicitly. _SE_FILE_OBJECT = 1 _DACL_SECURITY_INFORMATION = 0x00000004 @@ -133,15 +145,18 @@ class _EXPLICIT_ACCESS_W(ctypes.Structure): # noqa: N801 # mirrors Win32 type def _get_current_user_sid() -> ctypes.c_void_p: """Return a pointer to the current user's SID. - The SID buffer is kept alive on the returned pointer's referrer so it - stays valid for the duration of the DACL construction. + The `TOKEN_USER` buffer the SID points into is attached to the returned + pointer as `_buffer`, so it stays alive for the DACL construction. + `ctypes` already retains it through `.contents`; the attribute makes + that guarantee explicit rather than incidental. Returns: A pointer to the current user's SID. Raises: - WinError: If the process token or user SID cannot be read. - """ + OSError: If the process token or user SID cannot be read. Raised + via `ctypes.WinError`, which is a factory returning `OSError`. + """ # noqa: DOC501, DOC502 - `ctypes.WinError` returns an `OSError` advapi32 = ctypes.windll.advapi32 token = wintypes.HANDLE() if not advapi32.OpenProcessToken( @@ -175,14 +190,18 @@ def _get_current_user_sid() -> ctypes.c_void_p: ctypes.windll.kernel32.CloseHandle(token) def _apply_windows_owner_only_dacl(path: Path) -> None: - """Replace `path`'s DACL with one granting the current user full control. + """Replace `path`'s DACL with a single read/write entry for this user. + + The DACL is marked protected, so entries inherited from the parent + directory are dropped rather than merged. Args: path: Debug log file to lock down. Raises: - WinError: If the DACL cannot be built or applied. - """ + OSError: If the DACL cannot be built or applied. Raised via + `ctypes.WinError`, which is a factory returning `OSError`. + """ # noqa: DOC501, DOC502 - `ctypes.WinError` returns an `OSError` advapi32 = ctypes.windll.advapi32 sid = _get_current_user_sid() diff --git a/libs/code/deepagents_code/mcp_tools.py b/libs/code/deepagents_code/mcp_tools.py index ae7f632eb43..67d77be799d 100644 --- a/libs/code/deepagents_code/mcp_tools.py +++ b/libs/code/deepagents_code/mcp_tools.py @@ -218,15 +218,18 @@ class MCPConfigError(ValueError): _MCP_STDERR_DRAIN_JOIN_TIMEOUT = 2.0 """Bound on joining the stderr drain thread during session teardown. -The drain thread blocks in `os.read` until the pipe hits EOF, which only -happens once *every* process holding the write end closes it. MCP's stdio -shutdown terminates the server's process tree, but a server that spawns a -longer-lived descendant escaping its process group keeps the inherited pipe -open — so an unbounded join would wedge session close (discovery failure, -reload, shutdown). After this timeout the read end is closed to force the -thread's `os.read` to fail and exit. The forced-close join is bounded too: -on Linux, closing a file descriptor from another thread does not reliably -interrupt an in-progress `os.read`. +The drain thread blocks in `os.read` until the pipe hits EOF. EOF comes only +when *every* process holding the write end has closed it. MCP's stdio shutdown +terminates the server's process tree, but a server can spawn a descendant that +escapes that process group and keeps the inherited pipe open. An unbounded join +would then block session close. Session close runs on discovery failure, on +reload, and on shutdown. + +After this timeout the read end is closed to make the blocked `os.read` return. +That is not a portable guarantee: on darwin the read returns EOF, and on Linux +it can stay blocked, because `close` need not wake a reader already inside the +syscall. The forced-close join is therefore bounded by this timeout as well, +and the thread is abandoned if it outlives it. It is a daemon thread. """ @@ -307,12 +310,12 @@ def close(self) -> None: async def wait_closed(self) -> None: """Wait off the event loop until the pipe reader reaches EOF. - The join is bounded: if the drain thread is still blocked after - `_MCP_STDERR_DRAIN_JOIN_TIMEOUT` (the pipe's write end is held open by a - surviving server descendant), the read end is closed to force the - thread's `os.read` to fail, then the thread is rejoined with the same - bound. This keeps a leaked stderr pipe from hanging session cleanup - even where a cross-thread close does not interrupt `os.read`. + The join is bounded. If the drain thread is still blocked after + `_MCP_STDERR_DRAIN_JOIN_TIMEOUT`, the pipe's write end is held open by a + surviving server descendant. The read end is then closed to make the + blocked `os.read` return, and the thread is rejoined with the same + bound. Both bounds are necessary: a cross-thread close does not reliably + interrupt a reader inside `os.read`, so neither join can be unbounded. """ self.close() await asyncio.to_thread(self._thread.join, _MCP_STDERR_DRAIN_JOIN_TIMEOUT) @@ -455,6 +458,10 @@ async def _create_mcp_session( ) -> AsyncIterator[ClientSession]: """Create a session while routing stdio server diagnostics into DEBUG logs. + The stdio branch mirrors `langchain_mcp_adapters.sessions._create_stdio_session` + and exists only to pass `errlog`. Keep the two in sync when the adapter + changes its stdio setup. + Args: connection: Adapter connection configuration. server_name: MCP server name used in log records. @@ -491,6 +498,11 @@ async def _create_mcp_session( sink = _MCPStderrSink(server_name, encoding=encoding) try: async with stdio_client(params, errlog=cast("TextIO", sink)) as (read, write): + # The child now holds its own dup of the write end, so drop the + # parent's copy. Without this the pipe never reaches EOF after the + # server exits and the drain thread blocks until forced closed. + # Safe here because `stdio_client` spawns the process during + # `__aenter__` and never touches `errlog` again. sink.close() async with ClientSession( read, From bf74f908b307863f681c00fbc07a3831bd486784 Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Wed, 19 Aug 2026 11:37:24 -0400 Subject: [PATCH 15/18] fix(code): close both pipe ends when the stderr sink fails to start MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two small lifecycle holes in `_MCPStderrSink.__init__`: If `self._writer.close()` raised in the thread-start failure path, the read end leaked. Close it in a `finally`, and route it through `_close_read_fd` so the single-close bookkeeping is not bypassed. If `os.fdopen` raised, the partially built object was still finalized, and `io.IOBase`'s finalizer called `close()`, which dereferenced `_writer` before it existed. The resulting `AttributeError` reached the unraisable hook next to the real error — invisible in a TUI, or screen-corrupting. Verified with a patched `os.fdopen` that the hook now stays clean. Also make the stderr capture test's timing invariant explicit: the count of two holds because `final` has no trailing newline and the child blocks on stdin, so the EOF flush cannot happen until the session context exits. --- libs/code/deepagents_code/mcp_tools.py | 12 +++++++++--- libs/code/tests/unit_tests/test_mcp_tools.py | 5 +++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/libs/code/deepagents_code/mcp_tools.py b/libs/code/deepagents_code/mcp_tools.py index 67d77be799d..0c80ef8eeb2 100644 --- a/libs/code/deepagents_code/mcp_tools.py +++ b/libs/code/deepagents_code/mcp_tools.py @@ -290,8 +290,10 @@ def __init__(self, server_name: str, *, encoding: str) -> None: ) self._thread.start() except BaseException: - self._writer.close() - os.close(self._read_fd) + try: + self._writer.close() + finally: + self._close_read_fd() raise def fileno(self) -> int: @@ -302,10 +304,14 @@ def close(self) -> None: """Close the parent's copy of the subprocess write descriptor.""" if self.closed: return + # `getattr`: the finalizer reaches here for an instance whose + # `os.fdopen` failed, before `_writer` was ever assigned. + writer = getattr(self, "_writer", None) try: super().close() finally: - self._writer.close() + if writer is not None: + writer.close() async def wait_closed(self) -> None: """Wait off the event loop until the pipe reader reaches EOF. diff --git a/libs/code/tests/unit_tests/test_mcp_tools.py b/libs/code/tests/unit_tests/test_mcp_tools.py index f5427ad86eb..f0f0e76d501 100644 --- a/libs/code/tests/unit_tests/test_mcp_tools.py +++ b/libs/code/tests/unit_tests/test_mcp_tools.py @@ -1006,9 +1006,14 @@ def stderr_messages() -> list[str]: if len(stderr_messages()) >= 2: break await asyncio.sleep(0.01) + # Exactly two: `final` has no trailing newline, so it stays + # buffered until the EOF flush. The child blocks on + # `sys.stdin.buffer.read()`, so EOF cannot arrive until the + # session context exits below. assert len(stderr_messages()) == 2 messages = stderr_messages() + assert len(messages) == 3 assert messages[0] == "MCP server 'fake' stderr: partial café[31m" long_line = messages[1].partition(" stderr: ")[2] assert len(long_line) == _MCP_STDERR_LINE_LIMIT From 7ab3b7c1e11cc07b8afb2cd85dc7fefb71aa2865 Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Wed, 19 Aug 2026 13:37:38 -0400 Subject: [PATCH 16/18] test(code): make timestamp footer hydration test deterministic `test_footers_render_for_hydrated_messages_above` passed only when earlier tests left residual state that kept the visible window tight. Run alone, the `set_timer` patch suppresses the deferred transcript prune that `_load_thread_history`'s "Resumed thread" mount schedules, so the window never shrinks back and `hist-0`'s footer is already mounted -- the `pytest.raises(NoMatches)` guard then never fires. Build the archived-head state the same way the transcript virtualization tests do: mount rows via `_mount_message`, shrink `WINDOW_SIZE`, and `_prune_messages("above")` synchronously, instead of driving it through history loading's deferred timers. --- libs/code/tests/unit_tests/test_app.py | 41 ++++++++------------------ 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/libs/code/tests/unit_tests/test_app.py b/libs/code/tests/unit_tests/test_app.py index 5ce585ef881..699529259c4 100644 --- a/libs/code/tests/unit_tests/test_app.py +++ b/libs/code/tests/unit_tests/test_app.py @@ -15201,9 +15201,6 @@ async def test_footers_render_for_hydrated_messages_above( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: """Scroll-up hydration of older messages builds visible footers.""" - from deepagents_code.app import _ThreadHistoryPayload - from deepagents_code.tui.widgets.message_store import MessageData, MessageType - config = tmp_path / "config.toml" config.write_text("[ui]\nshow_message_timestamps = true\n") monkeypatch.setattr("deepagents_code.model_config.DEFAULT_CONFIG_PATH", config) @@ -15211,33 +15208,21 @@ async def test_footers_render_for_hydrated_messages_above( async with app.run_test() as pilot: await pilot.pause() - monkeypatch.setattr(app, "_check_hydration_below_needed", lambda: None) - # Shrink the window so a small load archives messages above the - # visible range, mirroring a long thread scrolled to the bottom. - monkeypatch.setattr(app._message_store, "WINDOW_SIZE", 2) - payload = _ThreadHistoryPayload( - [ - MessageData( - type=MessageType.USER, - content=f"m{index}", - id=f"hist-{index}", - timestamp=1_704_110_400.0 + index, - ) - for index in range(4) - ], - 0, - "", - ) - # Suppress history loading's delayed scroll-to-bottom timer. If it - # fires after the explicit hydrate-above call below, the resulting - # scroll offset change hydrates the tail and prunes `hist-0` again. - with patch.object(app, "set_timer"): - await app._load_thread_history( - thread_id="t-long", preloaded_payload=payload - ) + # Mount enough history to exceed the shrunken window, mirroring a + # long thread scrolled to the bottom. + for index in range(5): + await app._mount_message(UserMessage(f"m{index}", id=f"hist-{index}")) + await pilot.pause() + + # Shrink the window and prune the oldest rows so history is + # archived above the mounted tail; the newest rows stay visible. + monkeypatch.setattr(app._message_store, "WINDOW_SIZE", 3) + monkeypatch.setattr(app._message_store, "HYDRATE_BUFFER", 2) + await app._prune_messages("above") await pilot.pause() - # Older messages start archived (no widget/footer mounted yet). + # The oldest row is archived, so neither it nor its footer is + # mounted yet. with pytest.raises(NoMatches): app.query_one("#hist-0-timestamp-footer", Static) From 85078a5816e025d0723eed9825c0ec2e41a06e6a Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Wed, 19 Aug 2026 14:09:35 -0400 Subject: [PATCH 17/18] test(code): drop thread-death assertion from leaked-pipe stderr test On Linux, closing a pipe's read end does not wake a thread blocked in `os.read`, so the drain thread legitimately outlives the bounded teardown (it is a daemon by design and dies when the leaked descendant exits). The test's real contract is that session close stays bounded; wrap it in `asyncio.timeout` so a regression to an unbounded join still fails. --- libs/code/tests/unit_tests/test_mcp_tools.py | 26 +++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/libs/code/tests/unit_tests/test_mcp_tools.py b/libs/code/tests/unit_tests/test_mcp_tools.py index b5ee88ec939..01433d1a118 100644 --- a/libs/code/tests/unit_tests/test_mcp_tools.py +++ b/libs/code/tests/unit_tests/test_mcp_tools.py @@ -1080,18 +1080,20 @@ async def test_stderr_drain_does_not_hang_on_inherited_pipe( "args": ["-c", script], }, ) - async with _create_mcp_session(connection, server_name="fake"): - for _ in range(100): - if await asyncio.to_thread(started.exists): - break - await asyncio.sleep(0.01) - assert started.exists() - # Reaching here means __aexit__ (and wait_closed) returned instead of - # hanging on the leaked pipe. - assert not any( - thread.name == "mcp-stderr-fake" and thread.is_alive() - for thread in threading.enumerate() - ) + # Session teardown joins the drain thread twice with + # `_MCP_STDERR_DRAIN_JOIN_TIMEOUT` before abandoning it, so a + # regression to an unbounded join makes this timeout fire. + async with asyncio.timeout(4 * _MCP_STDERR_DRAIN_JOIN_TIMEOUT): + async with _create_mcp_session(connection, server_name="fake"): + for _ in range(100): + if await asyncio.to_thread(started.exists): + break + await asyncio.sleep(0.01) + assert started.exists() + # The drain thread may outlive teardown here: on Linux, closing the + # read end does not wake a thread blocked in `os.read`, so it stays + # parked (as a daemon) until the leaked descendant exits. Teardown + # being bounded — not thread death — is the guarantee under test. async def test_remote_session_delegates_to_adapter(self) -> None: """Non-stdio transports retain the adapter's session handling.""" From c63d535bc287b34859c724229c71749873b38db3 Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Wed, 19 Aug 2026 15:10:53 -0400 Subject: [PATCH 18/18] ci(infra): drop windows-latest from deepagents-code test matrix The libs/code unit test suite is POSIX-only (os.killpg/getpgid, bash, /tmp paths), so the new windows-latest matrix entry failed ~280 tests en masse. Remove it until the suite is made Windows-compatible. --- .github/workflows/ci.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f843220ca5e..dd7a4da31aa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -291,7 +291,6 @@ jobs: with: working-directory: "libs/code" python-versions: '["3.12", "3.13", "3.14"]' - extra-configurations: '[{"python-version": "3.13", "os": "windows-latest"}]' coverage-python-version: "3.14" test-talon: