From 2ccede1123f55b2a198624403d0214e58432eb31 Mon Sep 17 00:00:00 2001 From: U-C4N Date: Tue, 11 Aug 2026 17:30:37 +0300 Subject: [PATCH] fix: do not hand Thunderbird the caller's stdout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_launch` redirected only stdin, so the Thunderbird it starts inherited our stdout and stderr. Since that process outlives us by design, any caller capturing our output — an agent, a CI step, a shell pipeline — stays blocked long after `install-addon` has finished its work. Their own timeout does not help: killing us leaves the grandchild holding the pipe. Observed while installing the 1.2.0 add-on: the install succeeded and the command never returned. A reduced repro had the caller waiting the full lifetime of the launched process, 45s, despite a 12s timeout. `close_fds` does not cover the standard handles and DETACHED_PROCESS only detaches the console, so both have to be redirected explicitly — which the daemon spawn in bridge.py already did. Same requirement, one file over. The regression test fails in 25s against the old code and passes in 0.4s with the fix. Co-Authored-By: Claude Opus 5 (1M context) --- addon/manifest.json | 2 +- pyproject.toml | 2 +- src/tbmcp/addon_install.py | 13 ++++++- src/tbmcp/server.py | 2 +- tests/test_addon_install_launch.py | 59 ++++++++++++++++++++++++++++++ tests/test_version.py | 2 +- 6 files changed, 75 insertions(+), 5 deletions(-) create mode 100644 tests/test_addon_install_launch.py diff --git a/addon/manifest.json b/addon/manifest.json index 251f06e..853c5f5 100644 --- a/addon/manifest.json +++ b/addon/manifest.json @@ -1,7 +1,7 @@ { "manifest_version": 2, "name": "Thunderbird MCP Bridge", - "version": "1.2.0", + "version": "1.2.1", "description": "Lets a local MCP server drive this Thunderbird: mail, folders, contacts, calendar, filters and settings.", "author": "thunderbird-mcp", "browser_specific_settings": { diff --git a/pyproject.toml b/pyproject.toml index 5aa7a93..e59c0ce 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "thunderbird-mcp" -version = "1.2.0" +version = "1.2.1" description = "MCP server that drives Thunderbird — mail, folders, contacts, calendar, filters, and settings" readme = "README.md" requires-python = ">=3.11" diff --git a/src/tbmcp/addon_install.py b/src/tbmcp/addon_install.py index 09920f7..613f88a 100644 --- a/src/tbmcp/addon_install.py +++ b/src/tbmcp/addon_install.py @@ -177,7 +177,18 @@ def _launch(exe: pathlib.Path, extra_args: list[str], profile: ThunderbirdProfil argv = [str(exe), *extra_args] if profile is not None: argv += ["-profile", str(profile.path)] - kwargs: dict = {"stdin": subprocess.DEVNULL, "close_fds": True} + # All three streams, not just stdin. Thunderbird outlives us on purpose, so an + # inherited stdout keeps the caller's pipe open long after `install-addon` has + # finished — an agent or CI step capturing our output waits for the mail client + # to be closed, and its own timeout cannot rescue it, because killing us leaves + # the grandchild holding the pipe. `close_fds` does not cover the std handles; + # they have to be redirected explicitly, as the daemon spawn in bridge.py does. + kwargs: dict = { + "stdin": subprocess.DEVNULL, + "stdout": subprocess.DEVNULL, + "stderr": subprocess.DEVNULL, + "close_fds": True, + } if sys.platform == "win32": kwargs["creationflags"] = 0x00000008 | 0x00000200 # DETACHED | NEW_GROUP else: diff --git a/src/tbmcp/server.py b/src/tbmcp/server.py index 230cd8d..5585564 100644 --- a/src/tbmcp/server.py +++ b/src/tbmcp/server.py @@ -175,4 +175,4 @@ def _version() -> str: return version("thunderbird-mcp") except Exception: - return "1.2.0" + return "1.2.1" diff --git a/tests/test_addon_install_launch.py b/tests/test_addon_install_launch.py new file mode 100644 index 0000000..4e58949 --- /dev/null +++ b/tests/test_addon_install_launch.py @@ -0,0 +1,59 @@ +"""Launching Thunderbird must not hold the caller's stdout open. + +`_launch` starts a process that outlives us by design. If it lets that process +inherit our stdout, every caller that captures output — an agent, a CI step, a +shell pipeline — blocks until Thunderbird is closed, long after `install-addon` +has finished its work. A timeout does not rescue them: killing the direct child +leaves the grandchild holding the pipe. + +The daemon spawn in `bridge.py` already redirects all three streams; this is the +same requirement one file over. +""" + +from __future__ import annotations + +import subprocess +import sys +import time + +# Long enough that a regression is unmistakable against the deadline below, short +# enough that a failing run still ends on its own. +SLEEP_SECONDS = 25 +DEADLINE_SECONDS = 10 + +HELPER = ''' +import sys +from tbmcp.addon_install import _launch + + +class Exe: + """`_launch` stringifies the exe and appends the extra args.""" + + def __str__(self): + return sys.executable + + +_launch(Exe(), ["-c", "import time; time.sleep({sleep})"], None) +print("launched", flush=True) +''' + + +def test_launch_does_not_hold_the_callers_stdout(tmp_path): + helper = tmp_path / "launch_helper.py" + helper.write_text(HELPER.format(sleep=SLEEP_SECONDS), encoding="utf-8") + + started = time.monotonic() + done = subprocess.run( + [sys.executable, str(helper)], + capture_output=True, + text=True, + timeout=SLEEP_SECONDS + 60, + ) + elapsed = time.monotonic() - started + + assert done.returncode == 0, done.stderr + assert "launched" in done.stdout + assert elapsed < DEADLINE_SECONDS, ( + f"the caller was held for {elapsed:.1f}s after the helper exited: the launched " + "process inherited its stdout" + ) diff --git a/tests/test_version.py b/tests/test_version.py index e3f74d4..b4b4bf4 100644 --- a/tests/test_version.py +++ b/tests/test_version.py @@ -9,7 +9,7 @@ from tbmcp import server ROOT = pathlib.Path(__file__).resolve().parents[1] -EXPECTED = "1.2.0" +EXPECTED = "1.2.1" def _pyproject_version() -> str: