From e493f5cd96f25ac8994abf9d2e0b4d4376faa880 Mon Sep 17 00:00:00 2001 From: willtong-aq <277914688+willtong-aq@users.noreply.github.com> Date: Fri, 13 Nov 2026 03:03:09 +0000 Subject: [PATCH 1/4] Report an early exit 137 in the Docker backend instead of a timeout The Docker backend runs each command under coreutils timeout(1) with --kill-after=5s and mapped both exit 124 and exit 137 to error_kind="timeout". timeout(1) exits 137 when the child dies by SIGKILL (or exits 137 itself), and it sends SIGKILL itself only after the limit. A 137 before the limit usually means a SIGKILL from elsewhere, for example the kernel out-of-memory killer, so the model was told "Command timed out after N seconds" for a command that was killed in under a second. Exit 137 is now a timeout only when the elapsed time is at least the timeout. The elapsed time is measured in the worker thread around exec_run, so the wait for a thread is not counted. An earlier 137 returns the real exit code, stdout and stderr plus a note: "Exit code 137 after X seconds, before the N second limit. This usually means the process was killed with SIGKILL, possibly by the out-of-memory killer." It sets no error_kind. format_result now also writes stdout in the error branch when it is not empty, so partial output of a timed-out command reaches the model. An error result with empty stdout formats as before. --- src/stirrup/tools/code_backends/base.py | 2 + src/stirrup/tools/code_backends/docker.py | 50 +++++++++++---- tests/test_docker_execution.py | 77 +++++++++++++++++++++-- 3 files changed, 113 insertions(+), 16 deletions(-) diff --git a/src/stirrup/tools/code_backends/base.py b/src/stirrup/tools/code_backends/base.py index a829d72..422566d 100644 --- a/src/stirrup/tools/code_backends/base.py +++ b/src/stirrup/tools/code_backends/base.py @@ -393,6 +393,8 @@ def format_result(result: CommandResult) -> ToolResult[ToolUseCountMetadata]: f"\n{result.error_kind}" f"\n
{truncate_msg(result.stderr, MAX_LENGTH_SHELL_STDERR)}
" ) + if result.stdout: + content += f"\n{truncate_msg(result.stdout, MAX_LENGTH_SHELL_STDOUT)}" if result.advice: content += f"\n{result.advice}" content += "\n" diff --git a/src/stirrup/tools/code_backends/docker.py b/src/stirrup/tools/code_backends/docker.py index 1067dcb..a08d05d 100644 --- a/src/stirrup/tools/code_backends/docker.py +++ b/src/stirrup/tools/code_backends/docker.py @@ -2,11 +2,13 @@ import contextlib import hashlib +import math import os import shlex import shutil import tempfile from pathlib import Path, PurePosixPath +from time import perf_counter from typing import ClassVar, Self import anyio @@ -19,7 +21,7 @@ import docker from docker.client import DockerClient from docker.errors import APIError, BuildError, ImageNotFound, NotFound - from docker.models.containers import Container + from docker.models.containers import Container, ExecResult except ImportError as e: raise ImportError( "Requires installation of the docker extra. Install with (for example): `uv pip install stirrup[docker]` or `uv add stirrup[docker]`", @@ -619,8 +621,12 @@ async def _run_command_unchecked( # It runs the command in its own process group and sends SIGKILL to the # whole group after --kill-after, so descendants can't leak as orphans # parented to container PID 1 when the client gives up (moby/moby#9098). - # Exit 124 == SIGTERM expiry, 137 == --kill-after SIGKILL; both map to - # error_kind="timeout" for the caller. + # Exit 124 == SIGTERM expiry at the limit. timeout(1) also exits 137 when + # the child dies by SIGKILL after the TERM at the limit (it keeps the + # child's status, and --kill-after sends SIGKILL at limit + 5s). So the + # threshold below is the limit itself, not limit + kill-after, on + # purpose. A 137 before the limit is not a timeout. It usually means a + # SIGKILL from elsewhere, possibly the kernel out-of-memory killer. if argv is not None: exec_cmd = ["timeout", "--kill-after=5s", f"{timeout}s", *argv] else: @@ -630,19 +636,25 @@ async def _run_command_unchecked( try: # Outer fail_after is a safety net for a stalled docker socket; the # in-container timeout(1) should fire first in the normal case. - with fail_after(timeout + 10): - exec_result = await to_thread.run_sync( - lambda: container.exec_run( - cmd=exec_cmd, - workdir=self._working_dir, - demux=True, # Separate stdout/stderr - ) + # The clock runs in the worker thread around exec_run only, so the + # wait for a free thread is not counted. Docker API overhead is + # counted, which can only push a result towards "timeout". + def timed_exec_run() -> tuple[ExecResult, float]: + start_time = perf_counter() + result = container.exec_run( + cmd=exec_cmd, + workdir=self._working_dir, + demux=True, # Separate stdout/stderr ) + return result, perf_counter() - start_time + + with fail_after(timeout + 10): + exec_result, elapsed = await to_thread.run_sync(timed_exec_run) exit_code = exec_result.exit_code stdout_bytes, stderr_bytes = exec_result.output - if exit_code in (124, 137): + if exit_code == 124 or (exit_code == 137 and elapsed >= timeout): logger.warning("Command timed out after %d seconds: %s", timeout, cmd[:100]) partial_stderr = (stderr_bytes or b"").decode("utf-8", errors="replace") timeout_msg = f"Command timed out after {timeout} seconds" @@ -656,6 +668,22 @@ async def _run_command_unchecked( stderr_text = (stderr_bytes or b"").decode("utf-8", errors="replace") + if exit_code == 137: + # Round down, so the shown time stays below the limit (29.996 -> "29.99", not "30.00"). + shown = f"{math.floor(elapsed * 100) / 100:.2f}" + logger.warning( + "Command exited 137 after %s seconds, before the %d second limit: %s", shown, timeout, cmd[:100] + ) + kill_msg = ( + f"Exit code 137 after {shown} seconds, before the {timeout} second limit. " + "This usually means the process was killed with SIGKILL, possibly by the out-of-memory killer." + ) + return CommandResult( + exit_code=exit_code, + stdout=(stdout_bytes or b"").decode("utf-8", errors="replace"), + stderr=f"{stderr_text}\n{kill_msg}" if stderr_text else kill_msg, + ) + # Heuristic: exit 127 + a "timeout: ... not found" message from the # shell means coreutils timeout(1) is absent from the base image, so # the wrapper command never ran. Surface a clear pointer instead of diff --git a/tests/test_docker_execution.py b/tests/test_docker_execution.py index b3daaf7..2b1eeb6 100644 --- a/tests/test_docker_execution.py +++ b/tests/test_docker_execution.py @@ -4,6 +4,7 @@ import shlex from pathlib import Path +from typing import Any from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -13,7 +14,7 @@ from docker.errors import APIError, ImageNotFound -from stirrup.tools.code_backends.base import SHELL_TIMEOUT, CommandResult +from stirrup.tools.code_backends.base import SHELL_TIMEOUT, CommandResult, format_result from stirrup.tools.code_backends.docker import ( DEFAULT_WORKING_DIR, DockerCodeExecToolProvider, @@ -21,6 +22,19 @@ from stirrup.tools.code_backends.local import LocalCodeExecToolProvider +def _exec_run_taking(exec_result: MagicMock, seconds: float) -> tuple[MagicMock, Any]: + """Return an exec_run mock that advances a fake clock by ``seconds`` per call, and a perf_counter patch.""" + clock = [0.0] + + def exec_run(*_args: object, **_kwargs: object) -> MagicMock: + clock[0] += seconds + return exec_result + + return MagicMock(side_effect=exec_run), patch( + "stirrup.tools.code_backends.docker.perf_counter", side_effect=lambda: clock[0] + ) + + @pytest.fixture def mock_docker_client() -> MagicMock: """Create a mock Docker client.""" @@ -141,30 +155,83 @@ async def test_run_command_wraps_with_timeout(self, mock_docker_client: MagicMoc async def test_run_command_timeout_exit_codes_map_to_timeout( self, mock_docker_client: MagicMock, tmp_path: Path ) -> None: - """Exit 124 (SIGTERM expiry) and 137 (--kill-after SIGKILL) both surface as timeouts.""" + """Exit 124, and exit 137 at or past the limit (--kill-after SIGKILL), surface as timeouts.""" provider = DockerCodeExecToolProvider.from_image("python:3.12-slim", temp_base_dir=tmp_path) - for exit_code in (124, 137): + # (137, 1.0) is the boundary: elapsed equal to the 1 second limit is a timeout. + for exit_code, elapsed in ((124, 1.0), (137, 6.0), (137, 1.0)): mock_exec_result = MagicMock() mock_exec_result.exit_code = exit_code mock_exec_result.output = (b"partial-stdout", b"partial-stderr") - mock_docker_client.containers.run.return_value.exec_run = MagicMock(return_value=mock_exec_result) + exec_run, clock_patch = _exec_run_taking(mock_exec_result, elapsed) + mock_docker_client.containers.run.return_value.exec_run = exec_run with ( patch("stirrup.tools.code_backends.docker.docker.from_env", return_value=mock_docker_client), patch("stirrup.tools.code_backends.docker.to_thread") as mock_to_thread, + clock_patch, ): mock_to_thread.run_sync = AsyncMock(side_effect=lambda fn, *args: fn(*args) if not args else fn) async with provider as _: result = await provider.run_command("sleep 999", timeout=1) - assert result.error_kind == "timeout", f"exit {exit_code} should map to timeout" + case = f"exit {exit_code} after {elapsed}s" + assert result.error_kind == "timeout", f"{case} should map to timeout" assert result.exit_code == exit_code assert "partial-stdout" in result.stdout assert "partial-stderr" in result.stderr assert "timed out" in result.stderr.lower() + async def test_run_command_exit_137_before_limit_is_not_timeout( + self, mock_docker_client: MagicMock, tmp_path: Path + ) -> None: + """Exit 137 before the limit is not a timeout (usually an outside SIGKILL, e.g. the OOM killer).""" + provider = DockerCodeExecToolProvider.from_image("python:3.12-slim", temp_base_dir=tmp_path) + + mock_exec_result = MagicMock() + mock_exec_result.exit_code = 137 + mock_exec_result.output = (b"partial-stdout", b"partial-stderr") + exec_run, clock_patch = _exec_run_taking(mock_exec_result, 2.0) + mock_docker_client.containers.run.return_value.exec_run = exec_run + + with ( + patch("stirrup.tools.code_backends.docker.docker.from_env", return_value=mock_docker_client), + patch("stirrup.tools.code_backends.docker.to_thread") as mock_to_thread, + clock_patch, + ): + mock_to_thread.run_sync = AsyncMock(side_effect=lambda fn, *args: fn(*args) if not args else fn) + + async with provider as _: + result = await provider.run_command("python -c 'x = bytearray(10**10)'", timeout=30) + + assert result.error_kind is None + assert result.exit_code == 137 + assert "partial-stdout" in result.stdout + assert "partial-stderr" in result.stderr + assert "exit code 137 after 2.00 seconds, before the 30 second limit" in result.stderr.lower() + assert "timed out" not in result.stderr.lower() + + def test_format_result_error_branch_stdout(self) -> None: + """Error results show non-empty stdout. With empty stdout the text is unchanged.""" + with_stdout = format_result( + CommandResult( + exit_code=124, stdout="partial", stderr="Command timed out after 3 seconds", error_kind="timeout" + ) + ) + assert "\npartial" in with_stdout.content + assert "" not in with_stdout.content + + without_stdout = format_result( + CommandResult(exit_code=124, stdout="", stderr="Command timed out after 3 seconds", error_kind="timeout") + ) + assert without_stdout.content == ( + "" + "\ntimeout" + "\n
Command timed out after 3 seconds
" + "\n" + ) + async def test_run_command_missing_timeout_binary_advice( self, mock_docker_client: MagicMock, tmp_path: Path ) -> None: From b063abb03cba06078330af9402798fb08d18edb5 Mon Sep 17 00:00:00 2001 From: willtong-aq <277914688+willtong-aq@users.noreply.github.com> Date: Fri, 13 Nov 2026 03:28:10 +0000 Subject: [PATCH 2/4] Report an early exit 124 in the Docker backend instead of a timeout Exit 124 was always reported as a timeout. A command can exit 124 before the limit by itself, for example when it runs its own nested `timeout 3 sleep 60` under a 60 second limit. The model was then told "Command timed out after 60 seconds" after 3 seconds, and the real exit code was hidden. Exit 124 is now a timeout only when the elapsed time is at least the timeout, the same rule as exit 137. An earlier 124 returns the real exit code, stdout and stderr plus a note: "Exit code 124 after X seconds, before the N second limit. The limit did not stop this command." It sets no error_kind. --- src/stirrup/tools/code_backends/docker.py | 24 ++++++---- tests/test_docker_execution.py | 56 +++++++++++++---------- 2 files changed, 46 insertions(+), 34 deletions(-) diff --git a/src/stirrup/tools/code_backends/docker.py b/src/stirrup/tools/code_backends/docker.py index a08d05d..9c50284 100644 --- a/src/stirrup/tools/code_backends/docker.py +++ b/src/stirrup/tools/code_backends/docker.py @@ -625,8 +625,9 @@ async def _run_command_unchecked( # the child dies by SIGKILL after the TERM at the limit (it keeps the # child's status, and --kill-after sends SIGKILL at limit + 5s). So the # threshold below is the limit itself, not limit + kill-after, on - # purpose. A 137 before the limit is not a timeout. It usually means a - # SIGKILL from elsewhere, possibly the kernel out-of-memory killer. + # purpose. A 124 or 137 before the limit is not a timeout. A 124 can come + # from the command itself (e.g. its own nested `timeout`). A 137 usually + # means a SIGKILL from elsewhere, possibly the kernel out-of-memory killer. if argv is not None: exec_cmd = ["timeout", "--kill-after=5s", f"{timeout}s", *argv] else: @@ -654,7 +655,7 @@ def timed_exec_run() -> tuple[ExecResult, float]: exit_code = exec_result.exit_code stdout_bytes, stderr_bytes = exec_result.output - if exit_code == 124 or (exit_code == 137 and elapsed >= timeout): + if exit_code in (124, 137) and elapsed >= timeout: logger.warning("Command timed out after %d seconds: %s", timeout, cmd[:100]) partial_stderr = (stderr_bytes or b"").decode("utf-8", errors="replace") timeout_msg = f"Command timed out after {timeout} seconds" @@ -668,20 +669,25 @@ def timed_exec_run() -> tuple[ExecResult, float]: stderr_text = (stderr_bytes or b"").decode("utf-8", errors="replace") - if exit_code == 137: + if exit_code in (124, 137): # Round down, so the shown time stays below the limit (29.996 -> "29.99", not "30.00"). shown = f"{math.floor(elapsed * 100) / 100:.2f}" logger.warning( - "Command exited 137 after %s seconds, before the %d second limit: %s", shown, timeout, cmd[:100] + "Command exited %d after %s seconds, before the %d second limit: %s", + exit_code, + shown, + timeout, + cmd[:100], ) - kill_msg = ( - f"Exit code 137 after {shown} seconds, before the {timeout} second limit. " - "This usually means the process was killed with SIGKILL, possibly by the out-of-memory killer." + early_msg = f"Exit code {exit_code} after {shown} seconds, before the {timeout} second limit. " + ( + "The limit did not stop this command." + if exit_code == 124 + else "This usually means the process was killed with SIGKILL, possibly by the out-of-memory killer." ) return CommandResult( exit_code=exit_code, stdout=(stdout_bytes or b"").decode("utf-8", errors="replace"), - stderr=f"{stderr_text}\n{kill_msg}" if stderr_text else kill_msg, + stderr=f"{stderr_text}\n{early_msg}" if stderr_text else early_msg, ) # Heuristic: exit 127 + a "timeout: ... not found" message from the diff --git a/tests/test_docker_execution.py b/tests/test_docker_execution.py index 2b1eeb6..0b2d0f9 100644 --- a/tests/test_docker_execution.py +++ b/tests/test_docker_execution.py @@ -155,11 +155,11 @@ async def test_run_command_wraps_with_timeout(self, mock_docker_client: MagicMoc async def test_run_command_timeout_exit_codes_map_to_timeout( self, mock_docker_client: MagicMock, tmp_path: Path ) -> None: - """Exit 124, and exit 137 at or past the limit (--kill-after SIGKILL), surface as timeouts.""" + """Exit 124 or 137 at or past the limit surfaces as a timeout.""" provider = DockerCodeExecToolProvider.from_image("python:3.12-slim", temp_base_dir=tmp_path) - # (137, 1.0) is the boundary: elapsed equal to the 1 second limit is a timeout. - for exit_code, elapsed in ((124, 1.0), (137, 6.0), (137, 1.0)): + # Elapsed equal to the 1 second limit is the boundary, and it is a timeout. + for exit_code, elapsed in ((124, 1.0), (124, 1.5), (137, 6.0), (137, 1.0)): mock_exec_result = MagicMock() mock_exec_result.exit_code = exit_code mock_exec_result.output = (b"partial-stdout", b"partial-stderr") @@ -183,34 +183,40 @@ async def test_run_command_timeout_exit_codes_map_to_timeout( assert "partial-stderr" in result.stderr assert "timed out" in result.stderr.lower() - async def test_run_command_exit_137_before_limit_is_not_timeout( + async def test_run_command_early_exit_124_or_137_is_not_timeout( self, mock_docker_client: MagicMock, tmp_path: Path ) -> None: - """Exit 137 before the limit is not a timeout (usually an outside SIGKILL, e.g. the OOM killer).""" + """Exit 124 or 137 before the limit is not a timeout (e.g. a nested `timeout`, or the OOM killer).""" provider = DockerCodeExecToolProvider.from_image("python:3.12-slim", temp_base_dir=tmp_path) - mock_exec_result = MagicMock() - mock_exec_result.exit_code = 137 - mock_exec_result.output = (b"partial-stdout", b"partial-stderr") - exec_run, clock_patch = _exec_run_taking(mock_exec_result, 2.0) - mock_docker_client.containers.run.return_value.exec_run = exec_run - - with ( - patch("stirrup.tools.code_backends.docker.docker.from_env", return_value=mock_docker_client), - patch("stirrup.tools.code_backends.docker.to_thread") as mock_to_thread, - clock_patch, + for exit_code, explanation in ( + (124, "the limit did not stop this command."), + (137, "this usually means the process was killed with sigkill"), ): - mock_to_thread.run_sync = AsyncMock(side_effect=lambda fn, *args: fn(*args) if not args else fn) + mock_exec_result = MagicMock() + mock_exec_result.exit_code = exit_code + mock_exec_result.output = (b"partial-stdout", b"partial-stderr") + exec_run, clock_patch = _exec_run_taking(mock_exec_result, 2.0) + mock_docker_client.containers.run.return_value.exec_run = exec_run - async with provider as _: - result = await provider.run_command("python -c 'x = bytearray(10**10)'", timeout=30) - - assert result.error_kind is None - assert result.exit_code == 137 - assert "partial-stdout" in result.stdout - assert "partial-stderr" in result.stderr - assert "exit code 137 after 2.00 seconds, before the 30 second limit" in result.stderr.lower() - assert "timed out" not in result.stderr.lower() + with ( + patch("stirrup.tools.code_backends.docker.docker.from_env", return_value=mock_docker_client), + patch("stirrup.tools.code_backends.docker.to_thread") as mock_to_thread, + clock_patch, + ): + mock_to_thread.run_sync = AsyncMock(side_effect=lambda fn, *args: fn(*args) if not args else fn) + + async with provider as _: + result = await provider.run_command("timeout 2 sleep 60", timeout=30) + + stderr = result.stderr.lower() + assert result.error_kind is None, f"exit {exit_code} before the limit is not a timeout" + assert result.exit_code == exit_code + assert "partial-stdout" in result.stdout + assert "partial-stderr" in stderr + assert f"exit code {exit_code} after 2.00 seconds, before the 30 second limit" in stderr + assert explanation in stderr + assert "timed out" not in stderr def test_format_result_error_branch_stdout(self) -> None: """Error results show non-empty stdout. With empty stdout the text is unchanged.""" From 8e8f7313dee31b842fbda8ebd4c355c38fa64c1a Mon Sep 17 00:00:00 2001 From: willtong-aq <277914688+willtong-aq@users.noreply.github.com> Date: Fri, 13 Nov 2026 11:22:01 +0000 Subject: [PATCH 3/4] Simplify the early exit handling and add a changelog entry The early exit 124 or 137 branch now appends its note to stderr and reaches the existing final return, instead of building its own result and writing a log line. The tests use a plain perf_counter patch, and the format_result test is a module-level function. The output is unchanged. --- CHANGELOG.md | 11 ++++ src/stirrup/tools/code_backends/docker.py | 21 ++------ tests/test_docker_execution.py | 63 +++++++++-------------- 3 files changed, 39 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83e90de..93f3c3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## Unreleased + +### Fixed + +- The Docker backend no longer reports an exit code 124 or 137 before the time limit as a + timeout. Such a result now keeps its real exit code, stdout and stderr, plus a note with + the elapsed time. A 137 before the limit usually means an outside SIGKILL, for example the + kernel out-of-memory killer, and a 124 can come from a nested `timeout` in the command. +- Error results from code execution tools now include the stdout of the command when it is + not empty, so the partial output of a timed-out command reaches the model. + ## v0.2.1 (2026-10-22) ### Changed diff --git a/src/stirrup/tools/code_backends/docker.py b/src/stirrup/tools/code_backends/docker.py index 9c50284..2439926 100644 --- a/src/stirrup/tools/code_backends/docker.py +++ b/src/stirrup/tools/code_backends/docker.py @@ -624,10 +624,10 @@ async def _run_command_unchecked( # Exit 124 == SIGTERM expiry at the limit. timeout(1) also exits 137 when # the child dies by SIGKILL after the TERM at the limit (it keeps the # child's status, and --kill-after sends SIGKILL at limit + 5s). So the - # threshold below is the limit itself, not limit + kill-after, on - # purpose. A 124 or 137 before the limit is not a timeout. A 124 can come - # from the command itself (e.g. its own nested `timeout`). A 137 usually - # means a SIGKILL from elsewhere, possibly the kernel out-of-memory killer. + # threshold below is the limit itself, not limit + kill-after. A 124 or + # 137 before the limit is not a timeout. A 124 can come from the command + # itself (e.g. its own nested `timeout`). A 137 usually means a SIGKILL + # from elsewhere, possibly the kernel out-of-memory killer. if argv is not None: exec_cmd = ["timeout", "--kill-after=5s", f"{timeout}s", *argv] else: @@ -672,23 +672,12 @@ def timed_exec_run() -> tuple[ExecResult, float]: if exit_code in (124, 137): # Round down, so the shown time stays below the limit (29.996 -> "29.99", not "30.00"). shown = f"{math.floor(elapsed * 100) / 100:.2f}" - logger.warning( - "Command exited %d after %s seconds, before the %d second limit: %s", - exit_code, - shown, - timeout, - cmd[:100], - ) early_msg = f"Exit code {exit_code} after {shown} seconds, before the {timeout} second limit. " + ( "The limit did not stop this command." if exit_code == 124 else "This usually means the process was killed with SIGKILL, possibly by the out-of-memory killer." ) - return CommandResult( - exit_code=exit_code, - stdout=(stdout_bytes or b"").decode("utf-8", errors="replace"), - stderr=f"{stderr_text}\n{early_msg}" if stderr_text else early_msg, - ) + stderr_text = f"{stderr_text}\n{early_msg}" if stderr_text else early_msg # Heuristic: exit 127 + a "timeout: ... not found" message from the # shell means coreutils timeout(1) is absent from the base image, so diff --git a/tests/test_docker_execution.py b/tests/test_docker_execution.py index 0b2d0f9..f0c3014 100644 --- a/tests/test_docker_execution.py +++ b/tests/test_docker_execution.py @@ -4,7 +4,6 @@ import shlex from pathlib import Path -from typing import Any from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -22,19 +21,6 @@ from stirrup.tools.code_backends.local import LocalCodeExecToolProvider -def _exec_run_taking(exec_result: MagicMock, seconds: float) -> tuple[MagicMock, Any]: - """Return an exec_run mock that advances a fake clock by ``seconds`` per call, and a perf_counter patch.""" - clock = [0.0] - - def exec_run(*_args: object, **_kwargs: object) -> MagicMock: - clock[0] += seconds - return exec_result - - return MagicMock(side_effect=exec_run), patch( - "stirrup.tools.code_backends.docker.perf_counter", side_effect=lambda: clock[0] - ) - - @pytest.fixture def mock_docker_client() -> MagicMock: """Create a mock Docker client.""" @@ -163,13 +149,12 @@ async def test_run_command_timeout_exit_codes_map_to_timeout( mock_exec_result = MagicMock() mock_exec_result.exit_code = exit_code mock_exec_result.output = (b"partial-stdout", b"partial-stderr") - exec_run, clock_patch = _exec_run_taking(mock_exec_result, elapsed) - mock_docker_client.containers.run.return_value.exec_run = exec_run + mock_docker_client.containers.run.return_value.exec_run = MagicMock(return_value=mock_exec_result) with ( patch("stirrup.tools.code_backends.docker.docker.from_env", return_value=mock_docker_client), patch("stirrup.tools.code_backends.docker.to_thread") as mock_to_thread, - clock_patch, + patch("stirrup.tools.code_backends.docker.perf_counter", side_effect=[0.0, elapsed]), ): mock_to_thread.run_sync = AsyncMock(side_effect=lambda fn, *args: fn(*args) if not args else fn) @@ -196,13 +181,12 @@ async def test_run_command_early_exit_124_or_137_is_not_timeout( mock_exec_result = MagicMock() mock_exec_result.exit_code = exit_code mock_exec_result.output = (b"partial-stdout", b"partial-stderr") - exec_run, clock_patch = _exec_run_taking(mock_exec_result, 2.0) - mock_docker_client.containers.run.return_value.exec_run = exec_run + mock_docker_client.containers.run.return_value.exec_run = MagicMock(return_value=mock_exec_result) with ( patch("stirrup.tools.code_backends.docker.docker.from_env", return_value=mock_docker_client), patch("stirrup.tools.code_backends.docker.to_thread") as mock_to_thread, - clock_patch, + patch("stirrup.tools.code_backends.docker.perf_counter", side_effect=[0.0, 2.0]), ): mock_to_thread.run_sync = AsyncMock(side_effect=lambda fn, *args: fn(*args) if not args else fn) @@ -218,26 +202,6 @@ async def test_run_command_early_exit_124_or_137_is_not_timeout( assert explanation in stderr assert "timed out" not in stderr - def test_format_result_error_branch_stdout(self) -> None: - """Error results show non-empty stdout. With empty stdout the text is unchanged.""" - with_stdout = format_result( - CommandResult( - exit_code=124, stdout="partial", stderr="Command timed out after 3 seconds", error_kind="timeout" - ) - ) - assert "\npartial" in with_stdout.content - assert "" not in with_stdout.content - - without_stdout = format_result( - CommandResult(exit_code=124, stdout="", stderr="Command timed out after 3 seconds", error_kind="timeout") - ) - assert without_stdout.content == ( - "" - "\ntimeout" - "\n
Command timed out after 3 seconds
" - "\n" - ) - async def test_run_command_missing_timeout_binary_advice( self, mock_docker_client: MagicMock, tmp_path: Path ) -> None: @@ -516,3 +480,22 @@ def test_image_prep_lock_is_shared_per_image_name(self) -> None: assert lock_a1 is lock_a2 assert lock_a1 is not lock_b + + +def test_format_result_error_branch_stdout() -> None: + """Error results show non-empty stdout. With empty stdout the text is unchanged.""" + with_stdout = format_result( + CommandResult(exit_code=124, stdout="partial", stderr="Command timed out after 3 seconds", error_kind="timeout") + ) + assert "\npartial" in with_stdout.content + assert "" not in with_stdout.content + + without_stdout = format_result( + CommandResult(exit_code=124, stdout="", stderr="Command timed out after 3 seconds", error_kind="timeout") + ) + assert without_stdout.content == ( + "" + "\ntimeout" + "\n
Command timed out after 3 seconds
" + "\n" + ) From 373eae7e67f7238a9498a2a594e342deb5de8d69 Mon Sep 17 00:00:00 2001 From: willtong-aq <277914688+willtong-aq@users.noreply.github.com> Date: Fri, 13 Nov 2026 11:23:49 +0000 Subject: [PATCH 4/4] Keep the fake clock valid for teardown calls in the Docker tests The perf_counter patch held two values, so the chown exec during provider teardown exhausted it. The error was hidden by the provider's generic exception handler. The patch now repeats the last value. --- tests/test_docker_execution.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/test_docker_execution.py b/tests/test_docker_execution.py index f0c3014..4f34e64 100644 --- a/tests/test_docker_execution.py +++ b/tests/test_docker_execution.py @@ -2,6 +2,7 @@ from __future__ import annotations +import itertools import shlex from pathlib import Path from unittest.mock import AsyncMock, MagicMock, patch @@ -154,7 +155,10 @@ async def test_run_command_timeout_exit_codes_map_to_timeout( with ( patch("stirrup.tools.code_backends.docker.docker.from_env", return_value=mock_docker_client), patch("stirrup.tools.code_backends.docker.to_thread") as mock_to_thread, - patch("stirrup.tools.code_backends.docker.perf_counter", side_effect=[0.0, elapsed]), + patch( + "stirrup.tools.code_backends.docker.perf_counter", + side_effect=itertools.chain([0.0, elapsed], itertools.repeat(elapsed)), + ), ): mock_to_thread.run_sync = AsyncMock(side_effect=lambda fn, *args: fn(*args) if not args else fn) @@ -186,7 +190,10 @@ async def test_run_command_early_exit_124_or_137_is_not_timeout( with ( patch("stirrup.tools.code_backends.docker.docker.from_env", return_value=mock_docker_client), patch("stirrup.tools.code_backends.docker.to_thread") as mock_to_thread, - patch("stirrup.tools.code_backends.docker.perf_counter", side_effect=[0.0, 2.0]), + patch( + "stirrup.tools.code_backends.docker.perf_counter", + side_effect=itertools.chain([0.0, 2.0], itertools.repeat(2.0)), + ), ): mock_to_thread.run_sync = AsyncMock(side_effect=lambda fn, *args: fn(*args) if not args else fn)