From b4ee0bd9ce5ebb51f4ba424727d9c6293b41f31a Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Wed, 30 Sep 2026 11:17:20 -0500 Subject: [PATCH 01/21] coverage: measure subprocesses spawned by tests pytest-cov 7 dropped its own subprocess support; coverage's `patch = subprocess` replaces it. e2e tests that run duct via the CLI now count toward coverage (TOTAL 91.28% -> 92.28% on py313). Co-Authored-By: Claude Code 2.1.285 / Claude Opus 5.5 --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 16377161..0fbf341c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -117,6 +117,7 @@ src_paths = ["src"] [tool.coverage.run] branch = true parallel = true +patch = ["subprocess"] [tool.coverage.paths] source = [ From a1ea31534613116249c2c42a61e11e4a02de4df7 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Wed, 30 Sep 2026 11:20:30 -0500 Subject: [PATCH 02/21] coverage: save data before exec so the duct shim is measured `duct` execs into `con-duct run`; exec skips exit handlers, so without coverage's `execv` patch the shim's data is lost. Co-Authored-By: Claude Code 2.1.285 / Claude Opus 5.5 --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 0fbf341c..a1303933 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -117,7 +117,7 @@ src_paths = ["src"] [tool.coverage.run] branch = true parallel = true -patch = ["subprocess"] +patch = ["subprocess", "execv"] [tool.coverage.paths] source = [ From 9adec164be46e1e41dd4a06101b5a8a0c32fa756 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Wed, 30 Sep 2026 11:33:18 -0500 Subject: [PATCH 03/21] gitignore: per-process coverage data files With parallel coverage each process writes .coverage.....; pytest-cov combines them, but an aborted run leaves them behind. Co-Authored-By: Claude Code 2.1.285 / Claude Opus 5.5 --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 670257ea..289e11e9 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ __pycache__/ src/__pycache__/ *.egg-info/ .coverage +.coverage.* coverage.* .duct/ .idea From d6e91eec978bb47317771ac449e3d6d4effd57d4 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Wed, 30 Sep 2026 12:01:09 -0500 Subject: [PATCH 04/21] test: add start_duct helper that waits until duct handles signals Starts duct as a subprocess and returns once duct has logged that it is executing, which happens right after its SIGINT handler is installed. Lets signal tests synchronize on duct's readiness instead of sleeping. Co-Authored-By: Claude Code 2.1.285 / Claude Opus 5.5 --- test/utils.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/test/utils.py b/test/utils.py index d5ec9bef..4588562b 100644 --- a/test/utils.py +++ b/test/utils.py @@ -1,8 +1,15 @@ from __future__ import annotations from io import BytesIO from pathlib import Path +import re +import shlex +import subprocess from typing import Any +# duct logs this right after installing its SIGINT handler (_duct_main.execute). +# Anchored on the log format so the wrapped command's echoed stderr can't match. +DUCT_READY = re.compile(r"\] con-duct: duct \S+ is executing ") + def run_duct_command(cli_args: list[str], **kwargs: Any) -> int: """Helper to run duct with test-friendly defaults. @@ -40,6 +47,32 @@ def run_duct_command(cli_args: list[str], **kwargs: Any) -> int: return duct_execute(command=command, command_args=command_args, **defaults) # type: ignore[arg-type] +def start_duct(duct_cmd: str, args: list[str]) -> subprocess.Popen[str]: + """Start duct as a subprocess and return once it is ready for signals. + + Reads duct's stderr until the log line emitted just after the SIGINT + handler is installed. The caller should finish with ``communicate()``, + which drains the rest of stderr and waits for duct to exit. + + Args: + duct_cmd: How to invoke duct, e.g. "duct" or "con-duct run" + args: Arguments for duct, including the command to run + + Returns: + The running duct process, with its SIGINT handler installed + """ + proc = subprocess.Popen( + [*shlex.split(duct_cmd), *args], stderr=subprocess.PIPE, text=True + ) + assert proc.stderr is not None # for mypy + for line in proc.stderr: + if DUCT_READY.search(line): + return proc + raise RuntimeError( + f"duct exited with {proc.wait()} before logging that it is executing" + ) + + class MockStream: """Mocks stderr or stdout""" From da47787c9a73fa856dfa55c9ee5ef20fb632b598 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Wed, 30 Sep 2026 14:04:14 -0500 Subject: [PATCH 05/21] signals: forward Ctrl-C before logging it A SIGINT that arrives while duct's main thread is writing to stderr runs the handler mid-write; its own log call then raises a reentrant-write RuntimeError, which escaped before os.kill ran, so the Ctrl-C was lost. Act first, and drop the log message if it cannot be written. Co-Authored-By: Claude Code 2.1.285 / Claude Opus 5.5 --- src/con_duct/_signals.py | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/con_duct/_signals.py b/src/con_duct/_signals.py index 48d66c51..6c68661b 100644 --- a/src/con_duct/_signals.py +++ b/src/con_duct/_signals.py @@ -27,15 +27,33 @@ def __init__(self, pid: int) -> None: def __call__(self, _sig: int, _frame: Optional[FrameType]) -> None: self.sigcount += 1 + # Act before logging: the handler can interrupt duct mid-write to + # stderr, and a failed log write must not stop the signal. if self.sigcount == 1: - lgr.info("Received SIGINT, passing to command") os.kill(self.pid, signal.SIGINT) + _log(logging.INFO, "Received SIGINT, passing to command") elif self.sigcount == 2: - lgr.info("Received second SIGINT, again passing to command") os.kill(self.pid, signal.SIGINT) + _log(logging.INFO, "Received second SIGINT, again passing to command") elif self.sigcount == 3: - lgr.warning("Received third SIGINT, forcefully killing command process") os.kill(self.pid, signal.SIGKILL) + _log( + logging.WARNING, + "Received third SIGINT, forcefully killing command process", + ) elif self.sigcount >= 4: - lgr.critical("Exiting duct, skipping cleanup") + _log(logging.CRITICAL, "Exiting duct, skipping cleanup") os._exit(1) + + +def _log(level: int, msg: str) -> None: + """Log from a signal handler without ever raising. + + If the signal arrived while duct was writing to stderr, logging raises + ``RuntimeError: reentrant call`` (even its error report does); drop the + message rather than let the error escape the handler. + """ + try: + lgr.log(level, msg) + except RuntimeError: + pass From bc8b2467e986aa66c12889e9aa8e68fac169260a Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Wed, 30 Sep 2026 14:17:25 -0500 Subject: [PATCH 06/21] test: run test_signal_int as e2e, signalling only once duct is ready Replaces sleep-then-signal in a multiprocessing child (racy under spawn, the macOS default) with start_duct, and asserts duct's own exit code. Reruns disabled: the test must pass without them. Co-Authored-By: Claude Code 2.1.285 / Claude Opus 5.5 --- test/duct_main/test_e2e.py | 26 ++++++++++++++++++++ test/duct_main/test_execution.py | 41 -------------------------------- 2 files changed, 26 insertions(+), 41 deletions(-) diff --git a/test/duct_main/test_e2e.py b/test/duct_main/test_e2e.py index 62aefc39..006d2fe4 100644 --- a/test/duct_main/test_e2e.py +++ b/test/duct_main/test_e2e.py @@ -3,13 +3,16 @@ import os from pathlib import Path import platform +import signal import subprocess import time import pytest +from utils import assert_files, start_duct from con_duct._constants import SUFFIXES SYSTEM = platform.system() TEST_SCRIPT_DIR = Path(__file__).parent.parent / "data" +LOG_FILES = [SUFFIXES[k] for k in ("stdout", "stderr", "info", "usage")] # Allow overriding the duct executable for testing external builds (e.g., PyInstaller) _DUCT_EXECUTABLES = [ exe.strip() @@ -212,3 +215,26 @@ def test_logging_levels(temp_output_dir: str, duct_cmd: str) -> None: assert ( result_none.stderr == "" ), f"Expected empty stderr, got: {result_none.stderr!r}" + + +# @pytest.mark.flaky(reruns=5) # disabled: start_duct waits for duct instead of sleeping +@pytest.mark.parametrize("fail_time", [None, 0, 10, -1, -3.14]) +def test_signal_int( + temp_output_dir: str, duct_cmd: str, fail_time: float | None +) -> None: + args = ["-p", temp_output_dir] + if fail_time is not None: + args.append(f"--fail-time={fail_time}") + proc = start_duct(duct_cmd, [*args, "sleep", "60"]) + os.kill(proc.pid, signal.SIGINT) + proc.communicate() + + # duct forwards SIGINT to the command and exits with the command's code + assert proc.returncode == 128 + signal.SIGINT + + if fail_time is None or fail_time != 0: + assert_files(temp_output_dir, LOG_FILES, exists=False) + else: + with open(os.path.join(temp_output_dir, SUFFIXES["info"])) as info: + info_data = json.loads(info.read()) + assert info_data["execution_summary"]["exit_code"] == 128 + signal.SIGINT diff --git a/test/duct_main/test_execution.py b/test/duct_main/test_execution.py index 72745c97..a0548e57 100644 --- a/test/duct_main/test_execution.py +++ b/test/duct_main/test_execution.py @@ -228,47 +228,6 @@ def test_execute_unknown_command( assert_expected_files(temp_output_dir, exists=False) -def _runner_for_signal_int(temp_output_dir: str, fail_time: float | None) -> int: - kws = {} - if fail_time is not None: - kws["fail_time"] = fail_time - return run_duct_command( - ["sleep", "60.74016230000801"], output_prefix=temp_output_dir, **kws - ) - - -@pytest.mark.flaky(reruns=5) -@pytest.mark.parametrize("fail_time", [None, 0, 10, -1, -3.14]) -def test_signal_int( - request: pytest.FixtureRequest, temp_output_dir: str, fail_time: float | None -) -> None: - # Scale wait time on retries to handle slow CI runners (PyPy, Mac) - attempt = getattr(request.node, "execution_count", 1) - wait_time = 0.2 * attempt - proc = multiprocessing.Process( - target=_runner_for_signal_int, args=(temp_output_dir, fail_time) - ) - proc.start() - sleep(wait_time) - assert proc.pid is not None, "Process PID should not be None" # for mypy - os.kill(proc.pid, signal.SIGINT) - proc.join() - - # Once the command has been killed, duct should exit gracefully with exit code 0 - assert proc.exitcode == 0 - - if fail_time is None or fail_time != 0: - assert_expected_files(temp_output_dir, exists=False) - else: - # proc exit code should Cannot retrieve the exit code from the thread, it is written to the file - with open(os.path.join(temp_output_dir, SUFFIXES["info"])) as info: - info_data = json.loads(info.read()) - - command_exit_code = info_data["execution_summary"]["exit_code"] - # SIGINT - assert command_exit_code == 128 + 2 - - def _runner_for_signal_kill(temp_output_dir: str, fail_time: float | None) -> int: script_path = str(TEST_SCRIPT_DIR / "signal_ignorer.py") kws = {} From 98588d1a3bdab7613c1bbe73d40d0a5f1e718edc Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Wed, 30 Sep 2026 14:26:25 -0500 Subject: [PATCH 07/21] signals: install the SIGINT handler before starting the command A Ctrl-C between Popen and installing the handler raised KeyboardInterrupt in duct and left the command running untracked. The handler is now installed first and only counts SIGINTs until attached to the command: one before Popen means the command is not started (exit 130), one during Popen is forwarded as soon as the pid is known. Co-Authored-By: Claude Code 2.1.285 / Claude Opus 5.5 --- src/con_duct/_duct_main.py | 13 ++++- src/con_duct/_signals.py | 24 ++++++-- test/duct_main/test_signals.py | 104 +++++++++++++++++++++++++++++++++ test/utils.py | 5 +- 4 files changed, 138 insertions(+), 8 deletions(-) create mode 100644 test/duct_main/test_signals.py diff --git a/src/con_duct/_duct_main.py b/src/con_duct/_duct_main.py index e2954927..d0b2f7f0 100644 --- a/src/con_duct/_duct_main.py +++ b/src/con_duct/_duct_main.py @@ -65,6 +65,11 @@ def execute( sample_interval, ) + # Install before any command exists so no SIGINT can kill duct and orphan + # the command; until attach() the handler only counts them. + sigint_handler = SigIntHandler() + signal.signal(signal.SIGINT, sigint_handler) + log_paths = LogPaths.create(output_prefix, pid=os.getpid()) try: log_paths.prepare_paths(clobber, capture_outputs) @@ -99,6 +104,12 @@ def execute( ) files_to_close.append(report.usage_file) + if sigint_handler.sigcount: + safe_close_files(files_to_close) + remove_files(log_paths, assert_empty=True) + lgr.info("Received SIGINT before starting %r, not starting it", command) + return 128 + signal.SIGINT + report.start_time = time.time() try: report.process = process = subprocess.Popen( @@ -118,7 +129,7 @@ def execute( lgr.error("%s: command not found", command) return 127 # seems what zsh and bash return then - signal.signal(signal.SIGINT, SigIntHandler(process.pid)) + sigint_handler.attach(process.pid) lgr.info("duct %s is executing %r...", __version__, full_command) lgr.info("Log files will be written to %s", log_paths.prefix) try: diff --git a/src/con_duct/_signals.py b/src/con_duct/_signals.py index 6c68661b..9ed52200 100644 --- a/src/con_duct/_signals.py +++ b/src/con_duct/_signals.py @@ -15,18 +15,32 @@ class SigIntHandler: Handler of SIGINT signals received by the process running duct. """ - def __init__(self, pid: int) -> None: - """ + def __init__(self) -> None: + # Installed before the command starts; SIGINTs are only counted + # until attach() says which process to forward them to. + self.pid: Optional[int] = None + self.sigcount: int = 0 + + def attach(self, pid: int) -> None: + """Start forwarding SIGINTs to the command. + Parameters ---------- pid : int - The PID of the process monitored by duct + The PID of the process monitored by duct. A SIGINT that arrived + while it was being started is acted on now. """ - self.pid: int = pid - self.sigcount: int = 0 + self.pid = pid + if self.sigcount: + self._act() def __call__(self, _sig: int, _frame: Optional[FrameType]) -> None: self.sigcount += 1 + if self.pid is not None: + self._act() + + def _act(self) -> None: + assert self.pid is not None # Act before logging: the handler can interrupt duct mid-write to # stderr, and a failed log write must not stop the signal. if self.sigcount == 1: diff --git a/test/duct_main/test_signals.py b/test/duct_main/test_signals.py new file mode 100644 index 00000000..cbd283cc --- /dev/null +++ b/test/duct_main/test_signals.py @@ -0,0 +1,104 @@ +from __future__ import annotations +from collections.abc import Generator +import os +from pathlib import Path +import signal +from typing import Any +import pytest +from utils import run_duct_command +from con_duct import _signals +from con_duct._signals import SigIntHandler +from con_duct._tracker import Report + + +@pytest.fixture +def sent(monkeypatch: pytest.MonkeyPatch) -> list[tuple[int, int]]: + """Record os.kill calls made by the handler instead of sending them.""" + calls: list[tuple[int, int]] = [] + monkeypatch.setattr( + "con_duct._signals.os.kill", lambda pid, sig: calls.append((pid, sig)) + ) + return calls + + +@pytest.fixture +def restore_sigint() -> Generator[None, None, None]: + """execute() installs its own SIGINT handler; put pytest's back.""" + orig = signal.getsignal(signal.SIGINT) + yield + signal.signal(signal.SIGINT, orig) + + +@pytest.mark.parametrize( + "presses,expected", + [ + (1, [signal.SIGINT]), + (2, [signal.SIGINT, signal.SIGINT]), + (3, [signal.SIGINT, signal.SIGINT, signal.SIGKILL]), + ], +) +def test_forwards_after_attach( + sent: list[tuple[int, int]], presses: int, expected: list[int] +) -> None: + handler = SigIntHandler() + handler.attach(1234) + for _ in range(presses): + handler(signal.SIGINT, None) + assert sent == [(1234, sig) for sig in expected] + + +def test_counts_but_does_not_forward_before_attach( + sent: list[tuple[int, int]], +) -> None: + handler = SigIntHandler() + handler(signal.SIGINT, None) + assert handler.sigcount == 1 + assert sent == [] + + +@pytest.mark.parametrize( + "presses,expected", [(1, signal.SIGINT), (2, signal.SIGINT), (3, signal.SIGKILL)] +) +def test_attach_acts_on_earlier_sigints( + sent: list[tuple[int, int]], presses: int, expected: int +) -> None: + handler = SigIntHandler() + for _ in range(presses): + handler(signal.SIGINT, None) + handler.attach(1234) + # Only the latest escalation level is applied, once + assert sent == [(1234, expected)] + + +def test_forwards_even_if_logging_fails( + sent: list[tuple[int, int]], monkeypatch: pytest.MonkeyPatch +) -> None: + def reentrant(*_args: Any) -> None: + raise RuntimeError("reentrant call inside <_io.BufferedWriter name=''>") + + monkeypatch.setattr(_signals.lgr, "log", reentrant) + handler = SigIntHandler() + handler.attach(1234) + handler(signal.SIGINT, None) + assert sent == [(1234, signal.SIGINT)] + + +@pytest.mark.usefixtures("restore_sigint") +def test_sigint_before_command_starts_does_not_start_it( + temp_output_dir: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + real_report = Report + + def report_then_sigint(*args: Any, **kwargs: Any) -> Any: + # Report is built after duct's handler is installed and before Popen + report = real_report(*args, **kwargs) + os.kill(os.getpid(), signal.SIGINT) + return report + + monkeypatch.setattr("con_duct._duct_main.Report", report_then_sigint) + marker = tmp_path / "command-ran" + rc = run_duct_command(["touch", str(marker)], output_prefix=temp_output_dir) + + assert rc == 128 + signal.SIGINT + assert not marker.exists() + assert list(Path(temp_output_dir).iterdir()) == [] diff --git a/test/utils.py b/test/utils.py index 4588562b..c687e0a0 100644 --- a/test/utils.py +++ b/test/utils.py @@ -6,7 +6,8 @@ import subprocess from typing import Any -# duct logs this right after installing its SIGINT handler (_duct_main.execute). +# duct logs this right after attaching its SIGINT handler to the command +# (_duct_main.execute). # Anchored on the log format so the wrapped command's echoed stderr can't match. DUCT_READY = re.compile(r"\] con-duct: duct \S+ is executing ") @@ -51,7 +52,7 @@ def start_duct(duct_cmd: str, args: list[str]) -> subprocess.Popen[str]: """Start duct as a subprocess and return once it is ready for signals. Reads duct's stderr until the log line emitted just after the SIGINT - handler is installed. The caller should finish with ``communicate()``, + handler is attached to the command. The caller should finish with ``communicate()``, which drains the rest of stderr and waits for duct to exit. Args: From a229c997e2a0531d6ec5cdd8a2928e777de07637 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Wed, 30 Sep 2026 15:10:33 -0500 Subject: [PATCH 08/21] test: run test_signal_kill as e2e, waiting for each step signal_ignorer.py now reports "ready" and each SIGINT it receives to a file, so the test sends the next SIGINT only once the previous one was forwarded (pending SIGINTs merge) instead of sleeping between them. Reruns disabled: the test must pass without them. Co-Authored-By: Claude Code 2.1.285 / Claude Opus 5.5 --- test/data/signal_ignorer.py | 14 ++++++++++ test/duct_main/test_e2e.py | 38 +++++++++++++++++++++++++ test/duct_main/test_execution.py | 48 +------------------------------- 3 files changed, 53 insertions(+), 47 deletions(-) diff --git a/test/data/signal_ignorer.py b/test/data/signal_ignorer.py index 0f6fe066..afef7061 100755 --- a/test/data/signal_ignorer.py +++ b/test/data/signal_ignorer.py @@ -1,12 +1,25 @@ #!/usr/bin/env python3 +"""Ignore SIGINT, reporting progress through the file given as argv[1]. + +Writes "ready" once the handler is installed, then one "SIGINT" line per +SIGINT received, so a test can wait for each step instead of sleeping. +""" + import signal +import sys import time from types import FrameType from typing import Optional +def report(line: str) -> None: + with open(sys.argv[1], "a") as f: + f.write(line + "\n") + + def handle_signal(sig: int, _frame: Optional[FrameType]) -> None: print(f"Received {sig}") + report("SIGINT") if __name__ == "__main__": @@ -14,6 +27,7 @@ def handle_signal(sig: int, _frame: Optional[FrameType]) -> None: signal.siginterrupt( signal.SIGINT, False ) # Restart interrupted system calls so we can test multiple SIGINTS + report("ready") t0 = time.time() while time.time() - t0 < 10: time.sleep(0.01) diff --git a/test/duct_main/test_e2e.py b/test/duct_main/test_e2e.py index 006d2fe4..fea6b390 100644 --- a/test/duct_main/test_e2e.py +++ b/test/duct_main/test_e2e.py @@ -238,3 +238,41 @@ def test_signal_int( with open(os.path.join(temp_output_dir, SUFFIXES["info"])) as info: info_data = json.loads(info.read()) assert info_data["execution_summary"]["exit_code"] == 128 + signal.SIGINT + + +def _wait_for_lines(path: Path, n: int, proc: subprocess.Popen[str]) -> None: + """Wait until *path* has *n* lines, failing if duct exits first.""" + while not path.exists() or len(path.read_text().splitlines()) < n: + assert proc.poll() is None, f"duct exited before {path.name} had {n} lines" + time.sleep(0.01) + + +# @pytest.mark.flaky(reruns=5) # disabled: the test waits for each step instead of sleeping +@pytest.mark.parametrize("fail_time", [None, 0, 10, -1, -3.14]) +def test_signal_kill( + temp_output_dir: str, tmp_path: Path, duct_cmd: str, fail_time: float | None +) -> None: + progress = tmp_path / "signal_ignorer.progress" + args = ["-p", temp_output_dir] + if fail_time is not None: + args.append(f"--fail-time={fail_time}") + script = TEST_SCRIPT_DIR / "signal_ignorer.py" + proc = start_duct(duct_cmd, [*args, str(script), str(progress)]) + + _wait_for_lines(progress, 1, proc) # "ready": the command ignores SIGINT now + # duct forwards the first two; each must arrive before the next is sent, + # since pending SIGINTs merge into one + for received in (2, 3): + os.kill(proc.pid, signal.SIGINT) + _wait_for_lines(progress, received, proc) + os.kill(proc.pid, signal.SIGINT) # third: duct kills the command + proc.communicate() + + assert proc.returncode == 128 + signal.SIGKILL + + if fail_time is None or fail_time != 0: + assert_files(temp_output_dir, LOG_FILES, exists=False) + else: + with open(os.path.join(temp_output_dir, SUFFIXES["info"])) as info: + info_data = json.loads(info.read()) + assert info_data["execution_summary"]["exit_code"] == 128 + signal.SIGKILL diff --git a/test/duct_main/test_execution.py b/test/duct_main/test_execution.py index a0548e57..5b540f2e 100644 --- a/test/duct_main/test_execution.py +++ b/test/duct_main/test_execution.py @@ -1,13 +1,11 @@ from __future__ import annotations import json import logging -import multiprocessing import os from pathlib import Path -import signal import subprocess import sys -from time import sleep, time +from time import time import pytest from utils import assert_files, run_duct_command from con_duct import _duct_main @@ -228,50 +226,6 @@ def test_execute_unknown_command( assert_expected_files(temp_output_dir, exists=False) -def _runner_for_signal_kill(temp_output_dir: str, fail_time: float | None) -> int: - script_path = str(TEST_SCRIPT_DIR / "signal_ignorer.py") - kws = {} - if fail_time is not None: - kws["fail_time"] = fail_time - return run_duct_command([script_path], output_prefix=temp_output_dir, **kws) - - -@pytest.mark.flaky(reruns=5) -@pytest.mark.parametrize("fail_time", [None, 0, 10, -1, -3.14]) -def test_signal_kill( - request: pytest.FixtureRequest, temp_output_dir: str, fail_time: float | None -) -> None: - # Scale wait time on retries to handle slow CI runners (PyPy, Mac) - attempt = getattr(request.node, "execution_count", 1) - wait_time = 0.2 * attempt - proc = multiprocessing.Process( - target=_runner_for_signal_kill, args=(temp_output_dir, fail_time) - ) - proc.start() - sleep(wait_time) - assert proc.pid is not None, "Process PID should not be None" # for mypy - os.kill(proc.pid, signal.SIGINT) - sleep(wait_time) - os.kill(proc.pid, signal.SIGINT) - sleep(wait_time) - os.kill(proc.pid, signal.SIGINT) - proc.join() - - # Once the command has been killed, duct should exit gracefully with exit code 0 - assert proc.exitcode == 0 - - if fail_time is None or fail_time != 0: - assert_expected_files(temp_output_dir, exists=False) - else: - # Cannot retrieve the command exit code from the thread, get from duct log - with open(os.path.join(temp_output_dir, SUFFIXES["info"])) as info: - info_data = json.loads(info.read()) - - command_exit_code = info_data["execution_summary"]["exit_code"] - # SIGKILL - assert command_exit_code == 128 + 9 - - def test_duct_as_executable(temp_output_dir: str) -> None: ps_command = [ sys.executable, From d9908313cbf57c3c8cce2de2ff1acc09d836363e Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Wed, 30 Sep 2026 17:22:44 -0500 Subject: [PATCH 09/21] test: keep the session-mode marker alive for the whole test The background marker lived 10 s, but on a slow runner (macos-15-intel pypy, with coverage now measuring duct) the two duct runs before it is checked took ~15 s, so it had exited. The test stops it in `finally`. Co-Authored-By: Claude Code 2.1.285 / Claude Opus 5.5 --- test/duct_main/test_e2e.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/duct_main/test_e2e.py b/test/duct_main/test_e2e.py index fea6b390..d1faf56c 100644 --- a/test/duct_main/test_e2e.py +++ b/test/duct_main/test_e2e.py @@ -102,9 +102,10 @@ def test_session_modes(temp_output_dir: str, duct_cmd: str, session_mode: str) - def test_session_mode_behavior_difference(temp_output_dir: str, duct_cmd: str) -> None: """Test that new-session and current-session modes behave differently.""" - # Start a unique background process in the current session + # Start a unique background process in the current session. It must outlive + # both duct runs however slow they are (the finally block stops it). background_process = subprocess.Popen( - ["python", "-c", "print('DUCT_TEST_MARKER'); import time; time.sleep(10)"], + ["python", "-c", "print('DUCT_TEST_MARKER'); import time; time.sleep(600)"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, ) From f1fd650188b1a3e749331c95e7ab5ee124a0f167 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 1 Oct 2026 14:53:06 -0500 Subject: [PATCH 10/21] test: let signal_ignorer.py live 60 s, like the other test caps Its lifetime is the cap on how long test_signal_kill may take from the command starting to the third SIGINT. 10 s was the tightest cap among the reworked tests; the others give up after 60 s. The test kills the script long before either, so a passing run is no slower. Co-Authored-By: Claude Code 2.1.286 / Claude Fable 5.1 --- test/data/signal_ignorer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/data/signal_ignorer.py b/test/data/signal_ignorer.py index afef7061..3d1191fa 100755 --- a/test/data/signal_ignorer.py +++ b/test/data/signal_ignorer.py @@ -29,5 +29,5 @@ def handle_signal(sig: int, _frame: Optional[FrameType]) -> None: ) # Restart interrupted system calls so we can test multiple SIGINTS report("ready") t0 = time.time() - while time.time() - t0 < 10: + while time.time() - t0 < 60: time.sleep(0.01) From 56b7595617bf3bacf1e6fe1250e9c7ed0d0af796 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 1 Oct 2026 14:53:27 -0500 Subject: [PATCH 11/21] test: start_duct sets --log-level=INFO itself start_duct waits for duct's "is executing" line, which is logged at INFO. With DUCT_LOG_LEVEL above INFO in the environment or a .env file the line never comes and start_duct waits until the command exits. Pass the level explicitly so the helper does not depend on the developer's configuration. Co-Authored-By: Claude Code 2.1.286 / Claude Fable 5.1 --- test/utils.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/utils.py b/test/utils.py index c687e0a0..9b7083f4 100644 --- a/test/utils.py +++ b/test/utils.py @@ -55,6 +55,9 @@ def start_duct(duct_cmd: str, args: list[str]) -> subprocess.Popen[str]: handler is attached to the command. The caller should finish with ``communicate()``, which drains the rest of stderr and waits for duct to exit. + That line is logged at INFO, so duct is started with ``--log-level=INFO`` + whatever DUCT_LOG_LEVEL or a .env file says; ``args`` must not lower it. + Args: duct_cmd: How to invoke duct, e.g. "duct" or "con-duct run" args: Arguments for duct, including the command to run @@ -63,7 +66,9 @@ def start_duct(duct_cmd: str, args: list[str]) -> subprocess.Popen[str]: The running duct process, with its SIGINT handler installed """ proc = subprocess.Popen( - [*shlex.split(duct_cmd), *args], stderr=subprocess.PIPE, text=True + [*shlex.split(duct_cmd), "--log-level=INFO", *args], + stderr=subprocess.PIPE, + text=True, ) assert proc.stderr is not None # for mypy for line in proc.stderr: From dd4fc7b8a945d8220ce4eb4e18db5c5b946d4364 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 1 Oct 2026 15:04:21 -0500 Subject: [PATCH 12/21] test: show duct's stderr when a signal test fails early start_duct pipes duct's stderr and nothing printed it, so "duct exited before ..." gave no hint why. Include what duct wrote in start_duct's RuntimeError and in _wait_for_lines' assertion message. With it, a failing test_signal_kill shows the cause directly: a ProcessLookupError traceback from SigIntHandler, raised inside os.waitpid after the command had already exited. Co-Authored-By: Claude Code 2.1.286 / Claude Fable 5.1 --- test/duct_main/test_e2e.py | 7 +++++-- test/utils.py | 21 ++++++++++++++++++++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/test/duct_main/test_e2e.py b/test/duct_main/test_e2e.py index d1faf56c..706b9ea6 100644 --- a/test/duct_main/test_e2e.py +++ b/test/duct_main/test_e2e.py @@ -7,7 +7,7 @@ import subprocess import time import pytest -from utils import assert_files, start_duct +from utils import assert_files, rest_of_stderr, start_duct from con_duct._constants import SUFFIXES SYSTEM = platform.system() @@ -244,7 +244,10 @@ def test_signal_int( def _wait_for_lines(path: Path, n: int, proc: subprocess.Popen[str]) -> None: """Wait until *path* has *n* lines, failing if duct exits first.""" while not path.exists() or len(path.read_text().splitlines()) < n: - assert proc.poll() is None, f"duct exited before {path.name} had {n} lines" + assert proc.poll() is None, ( + f"duct exited with {proc.returncode} before {path.name} had {n} lines; " + f"its stderr:\n{rest_of_stderr(proc)}" + ) time.sleep(0.01) diff --git a/test/utils.py b/test/utils.py index 9b7083f4..ad928512 100644 --- a/test/utils.py +++ b/test/utils.py @@ -71,14 +71,33 @@ def start_duct(duct_cmd: str, args: list[str]) -> subprocess.Popen[str]: text=True, ) assert proc.stderr is not None # for mypy + seen = [] for line in proc.stderr: if DUCT_READY.search(line): return proc + seen.append(line) raise RuntimeError( - f"duct exited with {proc.wait()} before logging that it is executing" + f"duct exited with {proc.wait()} before logging that it is executing; " + f"its stderr:\n{''.join(seen)}" ) +def rest_of_stderr(proc: subprocess.Popen[str]) -> str: + """What duct wrote to stderr since ``start_duct`` returned, for failure messages. + + Reads to end of file, so only call it once duct has exited. + + Args: + proc: A duct process from ``start_duct`` + + Returns: + The unread remainder of duct's stderr + """ + assert proc.stderr is not None # for mypy + rest: str = proc.stderr.read() + return rest + + class MockStream: """Mocks stderr or stdout""" From 9ca6061cb0798c033df250f4ea605621a0df3381 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 1 Oct 2026 15:42:51 -0500 Subject: [PATCH 13/21] tox: require coverage >= 7.10 for the test environments pyproject.toml sets [tool.coverage.run] patch = ["subprocess", "execv"], which coverage only understands from 7.10. Older versions warn "Unrecognized option" and silently do not measure subprocesses (checked with 7.9.2 and 7.10.0). Declare the floor next to pytest-cov so an old environment fails at install time with a clear message. Co-Authored-By: Claude Code 2.1.286 / Claude Fable 5.1 --- tox.ini | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tox.ini b/tox.ini index e6710d69..2e16b17d 100644 --- a/tox.ini +++ b/tox.ini @@ -8,6 +8,7 @@ minversion = 3.3.0 deps = pytest pytest-cov + coverage>=7.10 # for [tool.coverage.run] patch in pyproject.toml pytest-rerunfailures -e .[all] commands = @@ -37,6 +38,7 @@ description = Test with minimal dependencies (no optional extras) deps = pytest pytest-cov + coverage>=7.10 # for [tool.coverage.run] patch in pyproject.toml pytest-rerunfailures -e . commands = @@ -49,6 +51,7 @@ allowlist_externals = uv deps = pytest pytest-cov + coverage>=7.10 # for [tool.coverage.run] patch in pyproject.toml pytest-rerunfailures commands = # numpy<2 required because matplotlib 3.5 was compiled against numpy 1.x From deb62e3412313047c0a508792eb72e80b4df5e4a Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 1 Oct 2026 17:01:49 -0500 Subject: [PATCH 14/21] signals: do not crash when a Ctrl-C is handled after the command exited Python runs a signal handler in the main thread at its next bytecode. For duct that can be only after its wait for the command has returned: when the signal's delivery did not wake the main thread, or when the Ctrl-C lands while duct is finishing up. The handler then signalled a pid that had already been reaped, and the ProcessLookupError escaped: duct printed a traceback and exited 1 instead of finishing normally with the command's exit code. CI showed it as test_signal_kill taking exactly the command's lifetime and duct exiting 1. Catch it and warn: the user pressed Ctrl-C and it could not be acted on, which they should see. Co-Authored-By: Claude Code 2.1.286 / Claude Fable 5.1 --- src/con_duct/_signals.py | 12 ++++++++++++ test/duct_main/test_signals.py | 15 +++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/con_duct/_signals.py b/src/con_duct/_signals.py index 9ed52200..9650397f 100644 --- a/src/con_duct/_signals.py +++ b/src/con_duct/_signals.py @@ -40,6 +40,18 @@ def __call__(self, _sig: int, _frame: Optional[FrameType]) -> None: self._act() def _act(self) -> None: + try: + self._forward() + except ProcessLookupError: + # The command has already exited. Python runs this handler in + # the main thread at its next bytecode, which can be only after + # the wait for the command returns. + _log( + logging.WARNING, + "Received SIGINT, but the command has already exited", + ) + + def _forward(self) -> None: assert self.pid is not None # Act before logging: the handler can interrupt duct mid-write to # stderr, and a failed log write must not stop the signal. diff --git a/test/duct_main/test_signals.py b/test/duct_main/test_signals.py index cbd283cc..ccbea071 100644 --- a/test/duct_main/test_signals.py +++ b/test/duct_main/test_signals.py @@ -83,6 +83,21 @@ def reentrant(*_args: Any) -> None: assert sent == [(1234, signal.SIGINT)] +def test_command_already_exited( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + def no_such_process(_pid: int, _sig: int) -> None: + raise ProcessLookupError(3, "No such process") + + monkeypatch.setattr("con_duct._signals.os.kill", no_such_process) + handler = SigIntHandler() + handler.attach(1234) + handler(signal.SIGINT, None) # must not raise + assert [(r.levelname, r.message) for r in caplog.records] == [ + ("WARNING", "Received SIGINT, but the command has already exited") + ] + + @pytest.mark.usefixtures("restore_sigint") def test_sigint_before_command_starts_does_not_start_it( temp_output_dir: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch From 4cb7077c92ac9224ac6d9154c284c9ba9e6e053c Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 1 Oct 2026 17:01:51 -0500 Subject: [PATCH 15/21] signals: start duct's threads with SIGINT blocked A signal sent to a process is delivered to any one of its threads that does not block it, but Python only runs the handler in the main thread. When the kernel handed a SIGINT to one of duct's other threads, the main thread stayed asleep in its wait for the command, and the Ctrl-C was not forwarded until the command exited by itself. Traced with strace: in 3 of 3 such cases a worker thread took the SIGINT, mid-run, while in 447 normal cases the main thread did. On an idle Linux machine with the test suite's 10 ms sampling interval 11 of 1000 first SIGINTs went unforwarded; with this change 0 of 1000. At duct's default 1 s sampling interval none did in 600 trials, so users were less exposed than the tests. A new thread inherits its creator's signal mask, so start_thread() blocks SIGINT just around Thread.start(). This is the usual way to steer signals in a threaded program, costs two system calls per thread start and nothing while the command runs, and leaves the handler's escalation (forward, forward, kill) as it was, since only the main thread ever ran it. A test checks that every thread started during a run goes through it. Side effect: processes started by those threads inherit the block, so the short-lived `ps` of the sampling thread cannot be interrupted by a terminal Ctrl-C. The command is started by the main thread and its mask is unchanged. Not tested on macOS. What remains is a SIGINT landing in the microseconds between the main thread's last check for signals and its blocking wait. That is a CPython limitation of any signal.signal() plus blocking-call program; a second Ctrl-C gets through and the pair is handled as one press. Co-Authored-By: Claude Code 2.1.286 / Claude Fable 5.1 --- src/con_duct/_duct_main.py | 8 ++++---- src/con_duct/_output.py | 3 ++- src/con_duct/_signals.py | 23 ++++++++++++++++++++++ test/duct_main/test_signals.py | 36 +++++++++++++++++++++++++++++++++- 4 files changed, 64 insertions(+), 6 deletions(-) diff --git a/src/con_duct/_duct_main.py b/src/con_duct/_duct_main.py index d0b2f7f0..89d3d8f2 100644 --- a/src/con_duct/_duct_main.py +++ b/src/con_duct/_duct_main.py @@ -9,7 +9,7 @@ from typing import IO, TextIO from con_duct._models import LogPaths, Outputs, RecordTypes, SessionMode from con_duct._output import TailPipe, prepare_outputs, remove_files, safe_close_files -from con_duct._signals import SigIntHandler +from con_duct._signals import SigIntHandler, start_thread from con_duct._tracker import Report, monitor_process __version__ = version("con-duct") @@ -156,15 +156,15 @@ def execute( monitoring_thread = threading.Thread( target=monitor_process, args=monitoring_args ) - monitoring_thread.start() + start_thread(monitoring_thread) else: monitoring_thread = None if record_types.has_system_summary(): env_thread = threading.Thread(target=report.collect_environment) - env_thread.start() + start_thread(env_thread) sys_info_thread = threading.Thread(target=report.get_system_info) - sys_info_thread.start() + start_thread(sys_info_thread) else: env_thread, sys_info_thread = None, None diff --git a/src/con_duct/_output.py b/src/con_duct/_output.py index d82bd658..ed90b30f 100644 --- a/src/con_duct/_output.py +++ b/src/con_duct/_output.py @@ -9,6 +9,7 @@ import time from typing import IO, Any, TextIO from con_duct._models import LogPaths, Outputs +from con_duct._signals import start_thread class TailPipe: @@ -27,7 +28,7 @@ def start(self) -> None: self.stop_event = threading.Event() self.infile = open(self.file_path, "rb") self.thread = threading.Thread(target=self._tail, daemon=True) - self.thread.start() + start_thread(self.thread) def fileno(self) -> int: assert self.infile is not None diff --git a/src/con_duct/_signals.py b/src/con_duct/_signals.py index 9650397f..7075b405 100644 --- a/src/con_duct/_signals.py +++ b/src/con_duct/_signals.py @@ -4,12 +4,35 @@ import logging import os import signal +import threading from types import FrameType from typing import Optional lgr = logging.getLogger("con-duct") +def start_thread(thread: threading.Thread) -> None: + """Start a thread that the kernel will never hand a SIGINT to. + + A signal sent to the process is delivered to any one thread that does + not block it, but Python runs the handler only in the main thread. If + another thread takes a SIGINT, the main thread stays asleep in its wait + for the command and the Ctrl-C is not forwarded until the command exits. + A new thread inherits its creator's signal mask, so block SIGINT just + while starting it. Every thread duct starts must be started this way. + + Parameters + ---------- + thread : threading.Thread + The thread to start + """ + old_mask = signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGINT}) + try: + thread.start() + finally: + signal.pthread_sigmask(signal.SIG_SETMASK, old_mask) + + class SigIntHandler: """ Handler of SIGINT signals received by the process running duct. diff --git a/test/duct_main/test_signals.py b/test/duct_main/test_signals.py index ccbea071..59c2446e 100644 --- a/test/duct_main/test_signals.py +++ b/test/duct_main/test_signals.py @@ -3,11 +3,12 @@ import os from pathlib import Path import signal +import threading from typing import Any import pytest from utils import run_duct_command from con_duct import _signals -from con_duct._signals import SigIntHandler +from con_duct._signals import SigIntHandler, start_thread from con_duct._tracker import Report @@ -98,6 +99,39 @@ def no_such_process(_pid: int, _sig: int) -> None: ] +def _sigint_blocked() -> bool: + """Whether the calling thread blocks SIGINT.""" + return signal.SIGINT in signal.pthread_sigmask(signal.SIG_BLOCK, []) + + +def test_start_thread_blocks_sigint_only_in_the_new_thread() -> None: + blocked_in_thread: list[bool] = [] + thread = threading.Thread( + target=lambda: blocked_in_thread.append(_sigint_blocked()) + ) + start_thread(thread) + thread.join() + assert blocked_in_thread == [True] + assert not _sigint_blocked() + + +@pytest.mark.usefixtures("restore_sigint") +def test_every_duct_thread_starts_with_sigint_blocked( + temp_output_dir: str, monkeypatch: pytest.MonkeyPatch +) -> None: + # A thread inherits the mask its creator has at the moment it is started + blocked_at_start: list[bool] = [] + real_start = threading.Thread.start + + def recording_start(self: threading.Thread) -> None: + blocked_at_start.append(_sigint_blocked()) + real_start(self) + + monkeypatch.setattr(threading.Thread, "start", recording_start) + assert run_duct_command(["true"], output_prefix=temp_output_dir) == 0 + assert blocked_at_start and all(blocked_at_start) + + @pytest.mark.usefixtures("restore_sigint") def test_sigint_before_command_starts_does_not_start_it( temp_output_dir: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch From 2d3e841d7ce180e4537c7180952a483f135bce7c Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 1 Oct 2026 17:01:59 -0500 Subject: [PATCH 16/21] test: cover --fail-time thresholds on their own Until now --fail-time was only tested through the signal tests' parametrization, where "logs removed" silently depended on the interrupted run finishing inside the threshold. Test the thresholds directly, with a command that just fails: under the threshold (3, the default, and 60) the logs are removed; a command that runs longer than the threshold (1 s) keeps them. Only the first two assume anything about speed, and only that `exit 1` takes less than 3 s. Co-Authored-By: Claude Code 2.1.286 / Claude Fable 5.1 --- test/duct_main/test_execution.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/duct_main/test_execution.py b/test/duct_main/test_execution.py index 5b540f2e..47e5a734 100644 --- a/test/duct_main/test_execution.py +++ b/test/duct_main/test_execution.py @@ -120,6 +120,28 @@ def test_sanity_red( assert_expected_files(temp_output_dir) +@pytest.mark.parametrize( + "fail_time,command,logs_kept", + [ + # Failed sooner than the threshold: logs are removed + (3, "exit 1", False), # 3 is the default --fail-time + (60, "exit 1", False), + # Failed after running longer than the threshold: logs are kept + (1, "sleep 1.2; exit 1", True), + ], +) +def test_fail_time_threshold( + temp_output_dir: str, fail_time: int, command: str, logs_kept: bool +) -> None: + assert ( + run_duct_command( + ["sh", "-c", command], output_prefix=temp_output_dir, fail_time=fail_time + ) + == 1 + ) + assert_expected_files(temp_output_dir, exists=logs_kept) + + def test_outputs_full(temp_output_dir: str) -> None: script_path = str(TEST_SCRIPT_DIR / "test_script.py") assert ( From b96eb271a7fa6bf91f9e0fb039d797b37f46f5b7 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 1 Oct 2026 17:02:22 -0500 Subject: [PATCH 17/21] test: signal tests use only run-time-independent --fail-time values test_signal_int and test_signal_kill ran with --fail-time unset, 0, 10, -1 and -3.14, a list they have carried since --fail-time was added. For the unset (3 s) and 10 cases the "logs removed" check depends on the interrupted run finishing inside the threshold, which a slow runner does not guarantee. Keep the two values whose effect does not depend on how long the run took: 0 always keeps a failed command's logs, a negative value never does. The thresholds themselves are covered by test_fail_time_threshold. Co-Authored-By: Claude Code 2.1.286 / Claude Fable 5.1 --- test/duct_main/test_e2e.py | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/test/duct_main/test_e2e.py b/test/duct_main/test_e2e.py index 706b9ea6..d978e2e9 100644 --- a/test/duct_main/test_e2e.py +++ b/test/duct_main/test_e2e.py @@ -218,14 +218,15 @@ def test_logging_levels(temp_output_dir: str, duct_cmd: str) -> None: ), f"Expected empty stderr, got: {result_none.stderr!r}" +# The --fail-time values whose effect does not depend on how long the run +# took: 0 always keeps a failed command's logs, a negative value never does +FAIL_TIMES = [0, -1] + + # @pytest.mark.flaky(reruns=5) # disabled: start_duct waits for duct instead of sleeping -@pytest.mark.parametrize("fail_time", [None, 0, 10, -1, -3.14]) -def test_signal_int( - temp_output_dir: str, duct_cmd: str, fail_time: float | None -) -> None: - args = ["-p", temp_output_dir] - if fail_time is not None: - args.append(f"--fail-time={fail_time}") +@pytest.mark.parametrize("fail_time", FAIL_TIMES) +def test_signal_int(temp_output_dir: str, duct_cmd: str, fail_time: int) -> None: + args = ["-p", temp_output_dir, f"--fail-time={fail_time}"] proc = start_duct(duct_cmd, [*args, "sleep", "60"]) os.kill(proc.pid, signal.SIGINT) proc.communicate() @@ -233,7 +234,7 @@ def test_signal_int( # duct forwards SIGINT to the command and exits with the command's code assert proc.returncode == 128 + signal.SIGINT - if fail_time is None or fail_time != 0: + if fail_time < 0: assert_files(temp_output_dir, LOG_FILES, exists=False) else: with open(os.path.join(temp_output_dir, SUFFIXES["info"])) as info: @@ -252,14 +253,12 @@ def _wait_for_lines(path: Path, n: int, proc: subprocess.Popen[str]) -> None: # @pytest.mark.flaky(reruns=5) # disabled: the test waits for each step instead of sleeping -@pytest.mark.parametrize("fail_time", [None, 0, 10, -1, -3.14]) +@pytest.mark.parametrize("fail_time", FAIL_TIMES) def test_signal_kill( - temp_output_dir: str, tmp_path: Path, duct_cmd: str, fail_time: float | None + temp_output_dir: str, tmp_path: Path, duct_cmd: str, fail_time: int ) -> None: progress = tmp_path / "signal_ignorer.progress" - args = ["-p", temp_output_dir] - if fail_time is not None: - args.append(f"--fail-time={fail_time}") + args = ["-p", temp_output_dir, f"--fail-time={fail_time}"] script = TEST_SCRIPT_DIR / "signal_ignorer.py" proc = start_duct(duct_cmd, [*args, str(script), str(progress)]) @@ -274,7 +273,7 @@ def test_signal_kill( assert proc.returncode == 128 + signal.SIGKILL - if fail_time is None or fail_time != 0: + if fail_time < 0: assert_files(temp_output_dir, LOG_FILES, exists=False) else: with open(os.path.join(temp_output_dir, SUFFIXES["info"])) as info: From f40bba88d4cc0d6be3d9be60f2ac7841682a17a5 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 8 Oct 2026 15:54:55 -0500 Subject: [PATCH 18/21] signals: write the log message raw when the stderr writer is locked A signal that interrupts duct mid-write to stderr makes the handler's own log call raise "reentrant call"; the message used to be dropped. Retrying cannot help: the interrupted write holds the buffered writer and only finishes after the handler returns. Write the message straight to fd 2 instead, still honoring the log level, on its own line and saying why it looks different. A failed raw write is ignored so the handler never raises. Co-Authored-By: Claude Code 2.1.295 / Claude Opus 5.5 --- src/con_duct/_signals.py | 20 +++++++++++-- test/duct_main/test_signals.py | 53 +++++++++++++++++++++++++++++----- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/src/con_duct/_signals.py b/src/con_duct/_signals.py index 7075b405..fa198509 100644 --- a/src/con_duct/_signals.py +++ b/src/con_duct/_signals.py @@ -99,10 +99,24 @@ def _log(level: int, msg: str) -> None: """Log from a signal handler without ever raising. If the signal arrived while duct was writing to stderr, logging raises - ``RuntimeError: reentrant call`` (even its error report does); drop the - message rather than let the error escape the handler. + ``RuntimeError: reentrant call`` (even its error report does): the + buffered writer is held by the write the handler interrupted, which + cannot finish until the handler returns. Write the message straight to + file descriptor 2 instead, if the log level lets the message through. + It does not go through the log format, and it may land in the middle of + the log line that was being written, so it starts on a new line and says + why it looks different. """ try: lgr.log(level, msg) except RuntimeError: - pass + if not lgr.isEnabledFor(level): + return + try: + line = ( + f"\ncon-duct [{logging.getLevelName(level)}] (written directly: " + f"the signal interrupted a log write): {msg}\n" + ) + os.write(2, line.encode()) + except OSError: + pass diff --git a/test/duct_main/test_signals.py b/test/duct_main/test_signals.py index 59c2446e..1fe67690 100644 --- a/test/duct_main/test_signals.py +++ b/test/duct_main/test_signals.py @@ -1,5 +1,6 @@ from __future__ import annotations from collections.abc import Generator +import logging import os from pathlib import Path import signal @@ -30,6 +31,16 @@ def restore_sigint() -> Generator[None, None, None]: signal.signal(signal.SIGINT, orig) +@pytest.fixture +def stderr_locked(monkeypatch: pytest.MonkeyPatch) -> None: + """Make logging fail as it does when the handler interrupts a stderr write.""" + + def reentrant(*_args: Any) -> None: + raise RuntimeError("reentrant call inside <_io.BufferedWriter name=''>") + + monkeypatch.setattr(_signals.lgr, "log", reentrant) + + @pytest.mark.parametrize( "presses,expected", [ @@ -71,19 +82,47 @@ def test_attach_acts_on_earlier_sigints( assert sent == [(1234, expected)] -def test_forwards_even_if_logging_fails( - sent: list[tuple[int, int]], monkeypatch: pytest.MonkeyPatch -) -> None: - def reentrant(*_args: Any) -> None: - raise RuntimeError("reentrant call inside <_io.BufferedWriter name=''>") - - monkeypatch.setattr(_signals.lgr, "log", reentrant) +@pytest.mark.usefixtures("stderr_locked") +def test_forwards_even_if_logging_fails(sent: list[tuple[int, int]]) -> None: handler = SigIntHandler() handler.attach(1234) handler(signal.SIGINT, None) assert sent == [(1234, signal.SIGINT)] +@pytest.mark.usefixtures("stderr_locked") +def test_log_falls_back_to_raw_stderr_write( + capfd: pytest.CaptureFixture[str], caplog: pytest.LogCaptureFixture +) -> None: + caplog.set_level(logging.INFO, logger="con-duct") + _signals._log(logging.INFO, "Received SIGINT, passing to command") + assert capfd.readouterr().err == ( + "\ncon-duct [INFO] (written directly: the signal interrupted a log write): " + "Received SIGINT, passing to command\n" + ) + + +@pytest.mark.usefixtures("stderr_locked") +def test_log_fallback_respects_log_level( + capfd: pytest.CaptureFixture[str], caplog: pytest.LogCaptureFixture +) -> None: + caplog.set_level(logging.WARNING, logger="con-duct") + _signals._log(logging.INFO, "Received SIGINT, passing to command") + assert capfd.readouterr().err == "" + + +@pytest.mark.usefixtures("stderr_locked") +def test_log_fallback_never_raises( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + def closed(_fd: int, _data: bytes) -> int: + raise OSError(9, "Bad file descriptor") + + caplog.set_level(logging.INFO, logger="con-duct") + monkeypatch.setattr("con_duct._signals.os.write", closed) + _signals._log(logging.INFO, "Received SIGINT, passing to command") # must not raise + + def test_command_already_exited( monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture ) -> None: From 4bb6aa9a0ab40a0733974adc00fa224f28fd35d4 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 8 Oct 2026 16:26:02 -0500 Subject: [PATCH 19/21] fixup! test: keep the session-mode marker alive for the whole test --- test/duct_main/test_e2e.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/duct_main/test_e2e.py b/test/duct_main/test_e2e.py index d978e2e9..8e27a311 100644 --- a/test/duct_main/test_e2e.py +++ b/test/duct_main/test_e2e.py @@ -111,9 +111,6 @@ def test_session_mode_behavior_difference(temp_output_dir: str, duct_cmd: str) - ) try: - # Give background process time to start - time.sleep(0.1) - new_session_prefix = f"{temp_output_dir}new_" current_session_prefix = f"{temp_output_dir}current_" From 5f3b75cff5232c59787d30d21c9f40afcdde8c7c Mon Sep 17 00:00:00 2001 From: leej3 Date: Mon, 12 Oct 2026 10:38:29 -0500 Subject: [PATCH 20/21] signals: block SIGINT while attach() hands the pid over A SIGINT that arrived between attach() setting self.pid and checking sigcount was forwarded twice: once by the handler, which already saw the pid, and once more by attach() acting on the now nonzero count. The command got two SIGINTs for one Ctrl-C. With SIGINT blocked in the main thread (and in every helper thread, via start_thread), a signal in that window stays pending and is handled once, after the mask is restored. Split out of asmacdo/duct#2, which also enabled setsid on macOS CI. Co-Authored-By: Codex Desktop 26.1007.21159 (runtime codex-cli 0.162.0-alpha.17.2) / gpt-6-astra Codex-Reasoning-Effort: low Co-Authored-By: Claude Code 2.1.296 / Claude Opus 5.5 --- src/con_duct/_signals.py | 12 +++++++++--- test/duct_main/test_signals.py | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/con_duct/_signals.py b/src/con_duct/_signals.py index fa198509..82cc907b 100644 --- a/src/con_duct/_signals.py +++ b/src/con_duct/_signals.py @@ -53,9 +53,15 @@ def attach(self, pid: int) -> None: The PID of the process monitored by duct. A SIGINT that arrived while it was being started is acted on now. """ - self.pid = pid - if self.sigcount: - self._act() + # A SIGINT after publishing the PID must not be forwarded both by + # __call__ and by the pending-signal check below. + old_mask = signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGINT}) + try: + self.pid = pid + if self.sigcount: + self._act() + finally: + signal.pthread_sigmask(signal.SIG_SETMASK, old_mask) def __call__(self, _sig: int, _frame: Optional[FrameType]) -> None: self.sigcount += 1 diff --git a/test/duct_main/test_signals.py b/test/duct_main/test_signals.py index 1fe67690..bdc84a11 100644 --- a/test/duct_main/test_signals.py +++ b/test/duct_main/test_signals.py @@ -82,6 +82,23 @@ def test_attach_acts_on_earlier_sigints( assert sent == [(1234, expected)] +@pytest.mark.usefixtures("restore_sigint") +def test_sigint_during_attach_is_forwarded_once(sent: list[tuple[int, int]]) -> None: + class InterruptOnAttach(SigIntHandler): + def __setattr__(self, name: str, value: Any) -> None: + super().__setattr__(name, value) + if name == "pid" and value is not None: + # Deliver a real SIGINT immediately after publishing the PID. + signal.raise_signal(signal.SIGINT) + + handler = InterruptOnAttach() + signal.signal(signal.SIGINT, handler) + handler.attach(1234) + + assert handler.sigcount == 1 + assert sent == [(1234, signal.SIGINT)] + + @pytest.mark.usefixtures("stderr_locked") def test_forwards_even_if_logging_fails(sent: list[tuple[int, int]]) -> None: handler = SigIntHandler() From 865f4b9c08a9d7869a81055aca62b61b0cf009a8 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Mon, 12 Oct 2026 10:38:44 -0500 Subject: [PATCH 21/21] signals: share one context manager for blocking SIGINT start_thread() and attach() both blocked SIGINT and restored the old mask with the same try/finally. Co-Authored-By: Claude Code 2.1.296 / Claude Opus 5.5 --- src/con_duct/_signals.py | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/con_duct/_signals.py b/src/con_duct/_signals.py index 82cc907b..2a7ccbc8 100644 --- a/src/con_duct/_signals.py +++ b/src/con_duct/_signals.py @@ -1,6 +1,8 @@ """Signal handlers for con-duct.""" from __future__ import annotations +from collections.abc import Iterator +from contextlib import contextmanager import logging import os import signal @@ -11,6 +13,20 @@ lgr = logging.getLogger("con-duct") +@contextmanager +def _sigint_blocked() -> Iterator[None]: + """Block SIGINT in the calling thread, restoring its mask on exit. + + A SIGINT that arrives meanwhile stays pending and is handled once the + mask is restored. + """ + old_mask = signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGINT}) + try: + yield + finally: + signal.pthread_sigmask(signal.SIG_SETMASK, old_mask) + + def start_thread(thread: threading.Thread) -> None: """Start a thread that the kernel will never hand a SIGINT to. @@ -26,11 +42,8 @@ def start_thread(thread: threading.Thread) -> None: thread : threading.Thread The thread to start """ - old_mask = signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGINT}) - try: + with _sigint_blocked(): thread.start() - finally: - signal.pthread_sigmask(signal.SIG_SETMASK, old_mask) class SigIntHandler: @@ -55,13 +68,10 @@ def attach(self, pid: int) -> None: """ # A SIGINT after publishing the PID must not be forwarded both by # __call__ and by the pending-signal check below. - old_mask = signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGINT}) - try: + with _sigint_blocked(): self.pid = pid if self.sigcount: self._act() - finally: - signal.pthread_sigmask(signal.SIG_SETMASK, old_mask) def __call__(self, _sig: int, _frame: Optional[FrameType]) -> None: self.sigcount += 1