fix(bash): report exit code for killed shells; single import style in registry tests

Copilot: bash_output's schema promises the exit code once the shell has
exited, but the formatting attached it only to 'completed' — a killed
shell has one too (the negated signal number). Attach it to any exited
state.

Code-quality: the registry test file mixed a top-level from-import with
function-local 'import ... as bg_mod' for monkeypatching module
attributes; one from-style module alias at the top now serves all of
them.
This commit is contained in:
Patrick Buckley
2026-07-10 15:25:21 -07:00
parent ab7d56e0ba
commit dc647f4d63
3 changed files with 14 additions and 12 deletions
+5 -10
View File
@@ -20,8 +20,13 @@ import pytest
from tests._proc_helpers import kill_pid as _kill_pid from tests._proc_helpers import kill_pid as _kill_pid
from tests._proc_helpers import pid_alive as _pid_alive from tests._proc_helpers import pid_alive as _pid_alive
from tests._proc_helpers import poll_until as _wait_until from tests._proc_helpers import poll_until as _wait_until
# Module alias (from-style, matching the symbol imports below) for tests
# that monkeypatch module attributes (os.killpg, subprocess.Popen, ...).
from turnstone.core import background_shells as bg_mod
from turnstone.core.background_shells import ( from turnstone.core.background_shells import (
BackgroundShellRegistry, BackgroundShellRegistry,
FilterExecError,
FilterTimeoutError, FilterTimeoutError,
TooManyShellsError, TooManyShellsError,
UnknownShellError, UnknownShellError,
@@ -441,8 +446,6 @@ def test_kill_on_completed_shell_does_not_signal_group(registry, monkeypatch):
"""A completed shell's pgid is a stale snapshot the OS may have recycled """A completed shell's pgid is a stale snapshot the OS may have recycled
to an unrelated process group — kill() must not signal it (the waiter's to an unrelated process group — kill() must not signal it (the waiter's
own group kill already ran at exit, when the pgid was fresh).""" own group kill already ran at exit, when the pgid was fresh)."""
import turnstone.core.background_shells as bg_mod
shell = registry.spawn("true") shell = registry.spawn("true")
assert _wait_status(shell, "completed") assert _wait_status(shell, "completed")
calls = [] calls = []
@@ -562,8 +565,6 @@ def test_thread_start_failure_leaves_no_orphan_record(registry, monkeypatch, tmp
unregistered and the fresh group reaped — an orphan with never-started unregistered and the fresh group reaped — an orphan with never-started
Thread objects would make every later close()/reap() join raise and Thread objects would make every later close()/reap() join raise and
abort session teardown.""" abort session teardown."""
import turnstone.core.background_shells as bg_mod
pidfile = tmp_path / "leader.pid" pidfile = tmp_path / "leader.pid"
real_thread = bg_mod.threading.Thread real_thread = bg_mod.threading.Thread
@@ -587,9 +588,6 @@ def test_thread_start_failure_leaves_no_orphan_record(registry, monkeypatch, tmp
def test_filter_helper_failure_reports_exec_error_not_timeout(registry, monkeypatch): def test_filter_helper_failure_reports_exec_error_not_timeout(registry, monkeypatch):
"""A crashed helper must not tell the model its (fine) pattern was too """A crashed helper must not tell the model its (fine) pattern was too
slow — and must not consume the delta.""" slow — and must not consume the delta."""
import turnstone.core.background_shells as bg_mod
from turnstone.core.background_shells import FilterExecError
shell = registry.spawn("echo hello") shell = registry.spawn("echo hello")
assert _wait_status(shell, "completed") assert _wait_status(shell, "completed")
monkeypatch.setattr(bg_mod.sys, "executable", "/bin/false") monkeypatch.setattr(bg_mod.sys, "executable", "/bin/false")
@@ -643,9 +641,6 @@ def test_filter_helper_spawn_failure_is_exec_error(registry, monkeypatch):
"""A helper that fails to LAUNCH (fork pressure) must land in the same """A helper that fails to LAUNCH (fork pressure) must land in the same
honest FilterExecError as a crashed helper — not escape as a raw honest FilterExecError as a crashed helper — not escape as a raw
OSError blaming nothing — and must not consume the delta.""" OSError blaming nothing — and must not consume the delta."""
import turnstone.core.background_shells as bg_mod
from turnstone.core.background_shells import FilterExecError
shell = registry.spawn("echo hello") shell = registry.spawn("echo hello")
assert _wait_status(shell, "completed") assert _wait_status(shell, "completed")
+4
View File
@@ -219,6 +219,10 @@ def test_kill_shell_kills_and_reports(session):
_cid, output = prepared["execute"](prepared) _cid, output = prepared["execute"](prepared)
assert "killed" in output.lower() assert "killed" in output.lower()
assert _wait_until(lambda: not _pid_alive(shell.pid)) assert _wait_until(lambda: not _pid_alive(shell.pid))
# The schema promises the exit code for ANY exited state, killed included.
read_prepared = session._prepare_bash_output("r", {"id": shell.shell_id})
_cid, read_output = read_prepared["execute"](read_prepared)
assert "exit code" in read_output
def test_kill_shell_unknown_id_reports_error(session): def test_kill_shell_unknown_id_reports_error(session):
+5 -2
View File
@@ -14574,8 +14574,11 @@ class ChatSession:
self._report_tool_result(call_id, "bash_output", msg, is_error=True) self._report_tool_result(call_id, "bash_output", msg, is_error=True)
return call_id, msg return call_id, msg
if read.status == "completed": # Exit code rides ANY exited state — the schema promises it "once
state = f"completed, exit code {read.exit_code}" # the shell has exited", and a killed shell has one too (the signal
# number, negated).
if read.exit_code is not None:
state = f"{read.status}, exit code {read.exit_code}"
else: else:
state = read.status state = read.status
parts = [f"{shell_id} ({state})"] parts = [f"{shell_id} ({state})"]