Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions tests/test_tui_gateway_server.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import json
import os
import subprocess
import sys
import threading
import time
Expand Down Expand Up @@ -59,6 +60,66 @@ def test_write_json_returns_false_on_broken_pipe(monkeypatch):
assert server.write_json({"ok": True}) is False


def test_slash_worker_close_joins_drain_threads_and_closes_pipes(monkeypatch):
started_threads = []

class _FakeThread:
def __init__(self, *, target, daemon):
self.target = target
self.daemon = daemon
self.join_calls: list[float | None] = []
started_threads.append(self)

def start(self):
return None

def join(self, timeout=None):
self.join_calls.append(timeout)

class _FakeStream:
def __init__(self):
self.closed = False

def close(self):
self.closed = True

class _FakeProc:
def __init__(self):
self.stdin = _FakeStream()
self.stdout = _FakeStream()
self.stderr = _FakeStream()
self.terminated = False
self.killed = False

def poll(self):
return None

def terminate(self):
self.terminated = True

def wait(self, timeout=None):
return 0

def kill(self):
self.killed = True

fake_proc = _FakeProc()

monkeypatch.setattr(server.threading, "Thread", _FakeThread)
monkeypatch.setattr(server.subprocess, "Popen", lambda *args, **kwargs: fake_proc)

worker = server._SlashWorker("session-1", "gpt-5")
worker.close()

assert len(started_threads) == 2
assert fake_proc.terminated is True
assert fake_proc.killed is False
assert fake_proc.stdin.closed is True
assert fake_proc.stdout.closed is True
assert fake_proc.stderr.closed is True
assert [t.join_calls for t in started_threads] == [[1.0], [1.0]]


def test_tui_verbose_tool_details_fail_closed_when_redaction_fails(monkeypatch):
redact_module = types.ModuleType("agent.redact")

Expand Down
19 changes: 17 additions & 2 deletions tui_gateway/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,10 @@ def __init__(self, session_key: str, model: str):
cwd=os.getcwd(),
env=os.environ.copy(),
)
threading.Thread(target=self._drain_stdout, daemon=True).start()
threading.Thread(target=self._drain_stderr, daemon=True).start()
self._stdout_thread = threading.Thread(target=self._drain_stdout, daemon=True)
self._stderr_thread = threading.Thread(target=self._drain_stderr, daemon=True)
self._stdout_thread.start()
self._stderr_thread.start()

def _drain_stdout(self):
for line in self.proc.stdout or []:
Expand Down Expand Up @@ -263,6 +265,19 @@ def close(self):
self.proc.kill()
except Exception:
pass
finally:
for stream in (self.proc.stdin, self.proc.stdout, self.proc.stderr):
if stream is None:
continue
try:
stream.close()
except Exception:
pass
for thread in (self._stdout_thread, self._stderr_thread):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This unconditionally reads attributes that are absent on the existing object.__new__(server._SlashWorker) fixture in tests/test_tui_gateway_server.py:8252. Use getattr(self, "_stdout_thread", None) / getattr(self, "_stderr_thread", None) and skip None values so close() remains compatible with that regression path.

try:
thread.join(timeout=1.0)
except Exception:
pass


def _load_busy_input_mode() -> str:
Expand Down