fix(sandbox): pass no_change_timeout to exec_command to prevent 120s premature termination (#2685)
* fix(sandbox): pass no_change_timeout to exec_command to prevent 120s premature termination The agent_sandbox library's shell API defaults no_change_timeout to 120 seconds. When AioSandbox.execute_command() called exec_command() without this parameter, commands producing no output for 120s would return with NO_CHANGE_TIMEOUT status even though the script was still running. Pass no_change_timeout=600 to all exec_command calls (matching the client-level HTTP timeout) so long-running commands are not cut short. Fixes #2668 * test(sandbox): add assertions for no_change_timeout in execute_command and list_dir Agent-Logs-Url: https://github.com/bytedance/deer-flow/sessions/2f37bc72-0826-4443-a6ba-e5b78c22fb5a Co-authored-by: WillemJiang <219644+WillemJiang@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -48,6 +48,12 @@ class AioSandbox(Sandbox):
|
|||||||
self._home_dir = context.home_dir
|
self._home_dir = context.home_dir
|
||||||
return self._home_dir
|
return self._home_dir
|
||||||
|
|
||||||
|
# Default no_change_timeout for exec_command (seconds). Matches the
|
||||||
|
# client-level timeout so that long-running commands which produce no
|
||||||
|
# output are not prematurely terminated by the sandbox's built-in 120 s
|
||||||
|
# default.
|
||||||
|
_DEFAULT_NO_CHANGE_TIMEOUT = 600
|
||||||
|
|
||||||
def execute_command(self, command: str) -> str:
|
def execute_command(self, command: str) -> str:
|
||||||
"""Execute a shell command in the sandbox.
|
"""Execute a shell command in the sandbox.
|
||||||
|
|
||||||
@@ -66,13 +72,13 @@ class AioSandbox(Sandbox):
|
|||||||
"""
|
"""
|
||||||
with self._lock:
|
with self._lock:
|
||||||
try:
|
try:
|
||||||
result = self._client.shell.exec_command(command=command)
|
result = self._client.shell.exec_command(command=command, no_change_timeout=self._DEFAULT_NO_CHANGE_TIMEOUT)
|
||||||
output = result.data.output if result.data else ""
|
output = result.data.output if result.data else ""
|
||||||
|
|
||||||
if output and _ERROR_OBSERVATION_SIGNATURE in output:
|
if output and _ERROR_OBSERVATION_SIGNATURE in output:
|
||||||
logger.warning("ErrorObservation detected in sandbox output, retrying with a fresh session")
|
logger.warning("ErrorObservation detected in sandbox output, retrying with a fresh session")
|
||||||
fresh_id = str(uuid.uuid4())
|
fresh_id = str(uuid.uuid4())
|
||||||
result = self._client.shell.exec_command(command=command, id=fresh_id)
|
result = self._client.shell.exec_command(command=command, id=fresh_id, no_change_timeout=self._DEFAULT_NO_CHANGE_TIMEOUT)
|
||||||
output = result.data.output if result.data else ""
|
output = result.data.output if result.data else ""
|
||||||
|
|
||||||
return output if output else "(no output)"
|
return output if output else "(no output)"
|
||||||
@@ -108,7 +114,7 @@ class AioSandbox(Sandbox):
|
|||||||
"""
|
"""
|
||||||
with self._lock:
|
with self._lock:
|
||||||
try:
|
try:
|
||||||
result = self._client.shell.exec_command(command=f"find {shlex.quote(path)} -maxdepth {max_depth} -type f -o -type d 2>/dev/null | head -500")
|
result = self._client.shell.exec_command(command=f"find {shlex.quote(path)} -maxdepth {max_depth} -type f -o -type d 2>/dev/null | head -500", no_change_timeout=self._DEFAULT_NO_CHANGE_TIMEOUT)
|
||||||
output = result.data.output if result.data else ""
|
output = result.data.output if result.data else ""
|
||||||
if output:
|
if output:
|
||||||
return [line.strip() for line in output.strip().split("\n") if line.strip()]
|
return [line.strip() for line in output.strip().split("\n") if line.strip()]
|
||||||
|
|||||||
@@ -133,6 +133,58 @@ class TestListDirSerialization:
|
|||||||
assert lock_was_held == [True], "list_dir must hold the lock during exec_command"
|
assert lock_was_held == [True], "list_dir must hold the lock during exec_command"
|
||||||
|
|
||||||
|
|
||||||
|
class TestNoChangeTimeout:
|
||||||
|
"""Verify that no_change_timeout is forwarded to every exec_command call."""
|
||||||
|
|
||||||
|
def test_execute_command_passes_no_change_timeout(self, sandbox):
|
||||||
|
"""execute_command should pass no_change_timeout to exec_command."""
|
||||||
|
calls = []
|
||||||
|
|
||||||
|
def mock_exec(command, **kwargs):
|
||||||
|
calls.append(kwargs)
|
||||||
|
return SimpleNamespace(data=SimpleNamespace(output="ok"))
|
||||||
|
|
||||||
|
sandbox._client.shell.exec_command = mock_exec
|
||||||
|
|
||||||
|
sandbox.execute_command("echo hello")
|
||||||
|
|
||||||
|
assert len(calls) == 1
|
||||||
|
assert calls[0].get("no_change_timeout") == sandbox._DEFAULT_NO_CHANGE_TIMEOUT
|
||||||
|
|
||||||
|
def test_retry_passes_no_change_timeout(self, sandbox):
|
||||||
|
"""The ErrorObservation retry path should also pass no_change_timeout."""
|
||||||
|
calls = []
|
||||||
|
|
||||||
|
def mock_exec(command, **kwargs):
|
||||||
|
calls.append(kwargs)
|
||||||
|
if len(calls) == 1:
|
||||||
|
return SimpleNamespace(data=SimpleNamespace(output="'ErrorObservation' object has no attribute 'exit_code'"))
|
||||||
|
return SimpleNamespace(data=SimpleNamespace(output="ok"))
|
||||||
|
|
||||||
|
sandbox._client.shell.exec_command = mock_exec
|
||||||
|
|
||||||
|
sandbox.execute_command("echo hello")
|
||||||
|
|
||||||
|
assert len(calls) == 2
|
||||||
|
assert calls[0].get("no_change_timeout") == sandbox._DEFAULT_NO_CHANGE_TIMEOUT
|
||||||
|
assert calls[1].get("no_change_timeout") == sandbox._DEFAULT_NO_CHANGE_TIMEOUT
|
||||||
|
|
||||||
|
def test_list_dir_passes_no_change_timeout(self, sandbox):
|
||||||
|
"""list_dir should pass no_change_timeout to exec_command."""
|
||||||
|
calls = []
|
||||||
|
|
||||||
|
def mock_exec(command, **kwargs):
|
||||||
|
calls.append(kwargs)
|
||||||
|
return SimpleNamespace(data=SimpleNamespace(output="/a\n/b"))
|
||||||
|
|
||||||
|
sandbox._client.shell.exec_command = mock_exec
|
||||||
|
|
||||||
|
sandbox.list_dir("/test")
|
||||||
|
|
||||||
|
assert len(calls) == 1
|
||||||
|
assert calls[0].get("no_change_timeout") == sandbox._DEFAULT_NO_CHANGE_TIMEOUT
|
||||||
|
|
||||||
|
|
||||||
class TestConcurrentFileWrites:
|
class TestConcurrentFileWrites:
|
||||||
"""Verify file write paths do not lose concurrent updates."""
|
"""Verify file write paths do not lose concurrent updates."""
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user