From 7ebc04d1ccb548d5477367ef4e4075d2b93e913a Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Mon, 2 Mar 2026 18:17:05 -0800 Subject: [PATCH] Fix Copilot PR #3 review: timeout race, ANSI stripping, streaming perf (#5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix timeout race in _exec_bash: check proc.poll() before setting timed_out flag; fall back to proc.kill() if process group kill fails - Remove stderr_thread.join(timeout=5) — after proc.wait() the pipe is closed so join completes promptly without risk of truncation - Fix CSI final byte range in stripAnsi: [A-Za-z] → [@-~] to handle sequences like \x1b[1~ (Home key) that end in non-letter bytes - Use appendChild(createTextNode()) instead of textContent += for O(1) chunk appending in streaming output (avoids O(n²) on large output) - Use var(--accent-dim) in stream-pulse keyframe instead of hardcoded rgba() so pulse color adapts to light/dark theme - Record tool_output_chunk calls in RecordingUI for test assertions --- tests/test_server_live.py | 3 ++- turnstone/core/session.py | 10 ++++++++-- turnstone/ui/static/app.js | 4 ++-- turnstone/ui/static/style.css | 2 +- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/test_server_live.py b/tests/test_server_live.py index 084af965..1703b0ac 100644 --- a/tests/test_server_live.py +++ b/tests/test_server_live.py @@ -66,6 +66,7 @@ class RecordingUI: self.content_tokens: list[str] = [] self.reasoning_tokens: list[str] = [] self.tool_results: list[tuple[str, str]] = [] + self.tool_chunks: list[tuple[str, str]] = [] self.errors: list[str] = [] self.infos: list[str] = [] @@ -91,7 +92,7 @@ class RecordingUI: self.tool_results.append((name, output)) def on_tool_output_chunk(self, call_id, chunk): - pass + self.tool_chunks.append((call_id, chunk)) def on_status(self, usage, context_window, effort): self.events.append(("status",)) diff --git a/turnstone/core/session.py b/turnstone/core/session.py index 657af2a4..02ee3d44 100644 --- a/turnstone/core/session.py +++ b/turnstone/core/session.py @@ -1766,9 +1766,15 @@ class ChatSession: timed_out = threading.Event() def _on_timeout() -> None: + if proc.poll() is not None: + return # process already exited timed_out.set() with contextlib.suppress(OSError, ProcessLookupError): - os.killpg(os.getpgid(proc.pid), signal.SIGKILL) + try: + os.killpg(os.getpgid(proc.pid), signal.SIGKILL) + except OSError: + with contextlib.suppress(OSError, ProcessLookupError): + proc.kill() timer = threading.Timer(self.tool_timeout, _on_timeout) timer.start() @@ -1782,7 +1788,7 @@ class ChatSession: timer.cancel() proc.wait() - stderr_thread.join(timeout=5) + stderr_thread.join() finally: os.unlink(script_path) diff --git a/turnstone/ui/static/app.js b/turnstone/ui/static/app.js index 66fce098..31153215 100644 --- a/turnstone/ui/static/app.js +++ b/turnstone/ui/static/app.js @@ -1345,7 +1345,7 @@ function replayHistory(messages) { function stripAnsi(s) { // Strip CSI sequences, OSC sequences, and two-byte escapes return s.replace( - /\x1b(?:\[[0-9;?]*[A-Za-z]|\][^\x07\x1b]*(?:\x07|\x1b\\)?|[()#][A-Za-z0-9]|.)/g, + /\x1b(?:\[[0-9;?]*[@-~]|\][^\x07\x1b]*(?:\x07|\x1b\\)?|[()#][A-Za-z0-9]|.)/g, "", ); } @@ -1529,7 +1529,7 @@ function appendToolOutputChunk(callId, chunk) { target.after(el); } - el.textContent += stripped; + el.appendChild(document.createTextNode(stripped)); el.scrollTop = el.scrollHeight; scrollToBottom(); } diff --git a/turnstone/ui/static/style.css b/turnstone/ui/static/style.css index 2ff644ac..e45a7071 100644 --- a/turnstone/ui/static/style.css +++ b/turnstone/ui/static/style.css @@ -517,7 +517,7 @@ body { } @keyframes stream-pulse { 0%, 100% { border-left-color: var(--accent); } - 50% { border-left-color: rgba(229, 160, 66, 0.15); } + 50% { border-left-color: var(--accent-dim); } } .tool-output.collapsed { max-height: 150px; position: relative; } .tool-output.collapsed::after {