Skip to content

Commit b109f48

Browse files
seratchhsusulayaangazali
authored
fix(sandbox): settle PTY output before cleanup (#4738)
Co-authored-by: Henry Su <henrysu4707@gmail.com> Co-authored-by: ayaangazali <ayaangazali.work@gmail.com>
1 parent 611b18d commit b109f48

14 files changed

Lines changed: 1369 additions & 180 deletions

File tree

src/agents/extensions/sandbox/blaxel/sandbox.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -864,7 +864,7 @@ async def pty_exec_start(
864864
)
865865

866866
yield_time_ms = 10_000 if yield_time_s is None else int(yield_time_s * 1000)
867-
output, original_token_count = await self._collect_pty_output(
867+
output, original_token_count, output_closed = await self._collect_pty_output(
868868
entry=entry,
869869
yield_time_ms=clamp_pty_yield_time_ms(yield_time_ms),
870870
max_output_tokens=max_output_tokens,
@@ -874,6 +874,7 @@ async def pty_exec_start(
874874
entry=entry,
875875
output=output,
876876
original_token_count=original_token_count,
877+
output_closed=output_closed,
877878
)
878879

879880
async def pty_write_stdin(
@@ -898,7 +899,7 @@ async def pty_write_stdin(
898899
await asyncio.sleep(0.1)
899900

900901
yield_time_ms = 250 if yield_time_s is None else int(yield_time_s * 1000)
901-
output, original_token_count = await self._collect_pty_output(
902+
output, original_token_count, output_closed = await self._collect_pty_output(
902903
entry=entry,
903904
yield_time_ms=resolve_pty_write_yield_time_ms(
904905
yield_time_ms=yield_time_ms, input_empty=chars == ""
@@ -911,6 +912,7 @@ async def pty_write_stdin(
911912
entry=entry,
912913
output=output,
913914
original_token_count=original_token_count,
915+
output_closed=output_closed,
914916
)
915917

916918
async def pty_terminate_all(self) -> None:
@@ -972,7 +974,7 @@ async def _collect_pty_output(
972974
entry: _BlaxelPtySessionEntry,
973975
yield_time_ms: int,
974976
max_output_tokens: int | None,
975-
) -> tuple[bytes, int | None]:
977+
) -> tuple[bytes, int | None, bool]:
976978
return await collect_pty_output(
977979
output_chunks=entry.output_chunks,
978980
output_lock=entry.output_lock,
@@ -989,11 +991,12 @@ async def _finalize_pty_update(
989991
entry: _BlaxelPtySessionEntry,
990992
output: bytes,
991993
original_token_count: int | None,
994+
output_closed: bool,
992995
) -> PtyExecUpdate:
993-
exit_code = entry.exit_code if entry.done else None
996+
exit_code = entry.exit_code if output_closed else None
994997
live_process_id: int | None = process_id
995998

996-
if entry.done:
999+
if output_closed:
9971000
async with self._pty_lock:
9981001
removed = self._pty_sessions.pop(process_id, None)
9991002
self._reserved_pty_process_ids.discard(process_id)

src/agents/extensions/sandbox/cloudflare/sandbox.py

Lines changed: 17 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
_settle_mount_transition,
6060
with_ephemeral_mounts_removed,
6161
)
62+
from ....sandbox.session.pty_output import collect_pty_output
6263
from ....sandbox.session.pty_types import (
6364
PTY_PROCESSES_MAX,
6465
PTY_PROCESSES_WARNING,
@@ -67,7 +68,6 @@
6768
clamp_pty_yield_time_ms,
6869
process_id_to_prune_from_meta,
6970
resolve_pty_write_yield_time_ms,
70-
truncate_text_by_tokens,
7171
)
7272
from ....sandbox.session.runtime_helpers import RESOLVE_WORKSPACE_PATH_HELPER, RuntimeHelperScript
7373
from ....sandbox.session.sandbox_client import BaseSandboxClient, BaseSandboxClientOptions
@@ -1033,34 +1033,15 @@ async def _collect_pty_output(
10331033
entry: _CloudflarePtyProcessEntry,
10341034
yield_time_ms: int,
10351035
max_output_tokens: int | None,
1036-
) -> tuple[bytes, int | None]:
1037-
deadline = time.monotonic() + (yield_time_ms / 1000)
1038-
output = bytearray()
1039-
1040-
while True:
1041-
async with entry.output_lock:
1042-
while entry.output_chunks:
1043-
output.extend(entry.output_chunks.popleft())
1044-
1045-
if entry.output_closed.is_set():
1046-
async with entry.output_lock:
1047-
while entry.output_chunks:
1048-
output.extend(entry.output_chunks.popleft())
1049-
break
1050-
1051-
remaining_s = deadline - time.monotonic()
1052-
if remaining_s <= 0:
1053-
break
1054-
1055-
try:
1056-
await asyncio.wait_for(entry.output_notify.wait(), timeout=remaining_s)
1057-
except asyncio.TimeoutError:
1058-
break
1059-
entry.output_notify.clear()
1060-
1061-
text = output.decode("utf-8", errors="replace")
1062-
truncated_text, original_token_count = truncate_text_by_tokens(text, max_output_tokens)
1063-
return truncated_text.encode("utf-8", errors="replace"), original_token_count
1036+
) -> tuple[bytes, int | None, bool]:
1037+
return await collect_pty_output(
1038+
output_chunks=entry.output_chunks,
1039+
output_lock=entry.output_lock,
1040+
output_notify=entry.output_notify,
1041+
is_done=entry.output_closed.is_set,
1042+
yield_time_ms=yield_time_ms,
1043+
max_output_tokens=max_output_tokens,
1044+
)
10641045

10651046
async def _finalize_pty_update(
10661047
self,
@@ -1069,10 +1050,11 @@ async def _finalize_pty_update(
10691050
entry: _CloudflarePtyProcessEntry,
10701051
output: bytes,
10711052
original_token_count: int | None,
1053+
output_closed: bool,
10721054
) -> PtyExecUpdate:
1073-
exit_code = entry.exit_code if entry.output_closed.is_set() else None
1055+
exit_code = entry.exit_code if output_closed else None
10741056
live_process_id: int | None = process_id
1075-
if entry.output_closed.is_set():
1057+
if output_closed:
10761058
async with self._pty_lock:
10771059
removed = self._pty_processes.pop(process_id, None)
10781060
self._reserved_pty_process_ids.discard(process_id)
@@ -1220,7 +1202,7 @@ async def pty_exec_start(
12201202
)
12211203

12221204
yield_time_ms = 10_000 if yield_time_s is None else int(yield_time_s * 1000)
1223-
output, original_token_count = await self._collect_pty_output(
1205+
output, original_token_count, output_closed = await self._collect_pty_output(
12241206
entry=entry,
12251207
yield_time_ms=clamp_pty_yield_time_ms(yield_time_ms),
12261208
max_output_tokens=max_output_tokens,
@@ -1230,6 +1212,7 @@ async def pty_exec_start(
12301212
entry=entry,
12311213
output=output,
12321214
original_token_count=original_token_count,
1215+
output_closed=output_closed,
12331216
)
12341217

12351218
async def pty_write_stdin(
@@ -1253,7 +1236,7 @@ async def pty_write_stdin(
12531236
await asyncio.sleep(0.1)
12541237

12551238
yield_time_ms = 250 if yield_time_s is None else int(yield_time_s * 1000)
1256-
output, original_token_count = await self._collect_pty_output(
1239+
output, original_token_count, output_closed = await self._collect_pty_output(
12571240
entry=entry,
12581241
yield_time_ms=resolve_pty_write_yield_time_ms(
12591242
yield_time_ms=yield_time_ms,
@@ -1267,6 +1250,7 @@ async def pty_write_stdin(
12671250
entry=entry,
12681251
output=output,
12691252
original_token_count=original_token_count,
1253+
output_closed=output_closed,
12701254
)
12711255

12721256
async def pty_terminate_all(self) -> None:

src/agents/extensions/sandbox/daytona/sandbox.py

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -387,8 +387,8 @@ class _DaytonaPtySessionEntry:
387387
output_chunks: deque[bytes] = field(default_factory=deque)
388388
output_lock: asyncio.Lock = field(default_factory=asyncio.Lock)
389389
output_notify: asyncio.Event = field(default_factory=asyncio.Event)
390+
output_closed: asyncio.Event = field(default_factory=asyncio.Event)
390391
last_used: float = field(default_factory=time.monotonic)
391-
done: bool = False
392392
exit_code: int | None = None
393393
worker_task: asyncio.Task[None] | None = None
394394

@@ -755,7 +755,7 @@ async def _on_data(chunk: bytes | str) -> None:
755755
)
756756

757757
yield_time_ms = 10_000 if yield_time_s is None else int(yield_time_s * 1000)
758-
output, original_token_count = await self._collect_pty_output(
758+
output, original_token_count, output_closed = await self._collect_pty_output(
759759
entry=entry,
760760
yield_time_ms=clamp_pty_yield_time_ms(yield_time_ms),
761761
max_output_tokens=max_output_tokens,
@@ -765,6 +765,7 @@ async def _on_data(chunk: bytes | str) -> None:
765765
entry=entry,
766766
output=output,
767767
original_token_count=original_token_count,
768+
output_closed=output_closed,
768769
)
769770

770771
async def _run_pty_waiter(self, entry: _DaytonaPtySessionEntry) -> None:
@@ -776,7 +777,10 @@ async def _run_pty_waiter(self, entry: _DaytonaPtySessionEntry) -> None:
776777
except Exception:
777778
pass
778779
finally:
779-
entry.done = True
780+
# AsyncPtyHandle.wait() completes only after its WebSocket reader exits.
781+
# That reader awaits every async on_data callback before it can finish,
782+
# so this is Daytona's authoritative output-stream close boundary.
783+
entry.output_closed.set()
780784
entry.output_notify.set()
781785

782786
async def _run_session_reader(
@@ -801,11 +805,13 @@ async def _run_session_reader(
801805
cmd = await self._sandbox.process.get_session_command(session_id, cmd_id)
802806
if cmd.exit_code is not None:
803807
entry.exit_code = int(cmd.exit_code)
804-
entry.done = True
805808
except Exception:
806809
pass
807-
if not logs_failed:
808-
entry.done = True
810+
# Once the log callback stream has returned, or has failed after the
811+
# provider reports a final exit code, this worker is the only output
812+
# producer and no later callback can append bytes.
813+
if not logs_failed or entry.exit_code is not None:
814+
entry.output_closed.set()
809815
entry.output_notify.set()
810816

811817
async def pty_write_stdin(
@@ -832,7 +838,7 @@ async def pty_write_stdin(
832838
await asyncio.sleep(0.1)
833839

834840
yield_time_ms = 250 if yield_time_s is None else int(yield_time_s * 1000)
835-
output, original_token_count = await self._collect_pty_output(
841+
output, original_token_count, output_closed = await self._collect_pty_output(
836842
entry=entry,
837843
yield_time_ms=resolve_pty_write_yield_time_ms(
838844
yield_time_ms=yield_time_ms, input_empty=chars == ""
@@ -845,6 +851,7 @@ async def pty_write_stdin(
845851
entry=entry,
846852
output=output,
847853
original_token_count=original_token_count,
854+
output_closed=output_closed,
848855
)
849856

850857
async def _finalize_pty_update(
@@ -854,11 +861,12 @@ async def _finalize_pty_update(
854861
entry: _DaytonaPtySessionEntry,
855862
output: bytes,
856863
original_token_count: int | None,
864+
output_closed: bool,
857865
) -> PtyExecUpdate:
858-
exit_code = entry.exit_code if entry.done else None
866+
exit_code = entry.exit_code if output_closed else None
859867
live_process_id: int | None = process_id
860868

861-
if entry.done:
869+
if output_closed:
862870
async with self._pty_lock:
863871
removed = self._pty_sessions.pop(process_id, None)
864872
self._reserved_pty_process_ids.discard(process_id)
@@ -887,12 +895,12 @@ async def _collect_pty_output(
887895
entry: _DaytonaPtySessionEntry,
888896
yield_time_ms: int,
889897
max_output_tokens: int | None,
890-
) -> tuple[bytes, int | None]:
898+
) -> tuple[bytes, int | None, bool]:
891899
return await collect_pty_output(
892900
output_chunks=entry.output_chunks,
893901
output_lock=entry.output_lock,
894902
output_notify=entry.output_notify,
895-
is_done=lambda: entry.done,
903+
is_done=entry.output_closed.is_set,
896904
yield_time_ms=yield_time_ms,
897905
max_output_tokens=max_output_tokens,
898906
)
@@ -901,7 +909,8 @@ def _prune_pty_sessions_if_needed(self) -> _DaytonaPtySessionEntry | None:
901909
if len(self._pty_sessions) < PTY_PROCESSES_MAX:
902910
return None
903911
meta: list[tuple[int, float, bool]] = [
904-
(pid, entry.last_used, entry.done) for pid, entry in self._pty_sessions.items()
912+
(pid, entry.last_used, entry.output_closed.is_set())
913+
for pid, entry in self._pty_sessions.items()
905914
]
906915
pid = process_id_to_prune_from_meta(meta)
907916
if pid is None:

0 commit comments

Comments
 (0)