From dba3967177b33caaef98c41ed0263716254124fc Mon Sep 17 00:00:00 2001 From: liu584 <145111256+liu584@users.noreply.github.com> Date: Sun, 20 Sep 2026 19:07:48 +0800 Subject: [PATCH] fix(sandbox): wrap implicit-session probe commands in a subshell to prevent wedged shell (#1433 family) (#5546) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The AIO sandbox's implicit persistent shell session hangs forever when a command containing a bare 'exit' is executed: exit kills the session's shell process, and the server's response path for that exec_command request never completes. Verified against a standalone all-in-one-sandbox 1.11.0 container via the raw SDK: shell.exec_command('seq 1 1000 | head -n 5') # OK (not a SIGPIPE issue) shell.exec_command('echo x; exit 0') # HANGS every time shell.exec_command('echo after') # OK (server recreates shell) shell.exec_command('( echo x; exit 0 )') # OK, exit code propagates remote_list_dir_command and remote_search_command both end their probe scripts with a bare 'exit' (to propagate the find/grep status code), so every list_dir/grep/glob call deterministically wedges the session — this is the root cause behind parallel [ls, bash] tool calls deadlocking an entire run (same defect family as #1433 and #5128). Fix: keep 'set +e' as the outermost prefix (pinned by existing tests) and wrap the rest of each probe script in a subshell, so exit only terminates the subshell. Output and exit codes propagate identically. Tests: 116 passed (backend/tests/test_aio_sandbox.py, test_remote_list_dir.py, test_remote_search.py); one endswith assertion updated to the wrapped form. Co-authored-by: mad_max --- .../harness/deerflow/sandbox/remote_list_dir.py | 10 ++++++++-- .../packages/harness/deerflow/sandbox/remote_search.py | 9 +++++++-- backend/tests/test_remote_list_dir.py | 4 ++++ backend/tests/test_remote_search.py | 6 +++++- 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/backend/packages/harness/deerflow/sandbox/remote_list_dir.py b/backend/packages/harness/deerflow/sandbox/remote_list_dir.py index 461c67aff..e45b70f4a 100644 --- a/backend/packages/harness/deerflow/sandbox/remote_list_dir.py +++ b/backend/packages/harness/deerflow/sandbox/remote_list_dir.py @@ -34,14 +34,20 @@ def remote_list_dir_command(path: str, max_depth: int, *, limit: int = _LIST_LIM # a login-profile ``set -e`` so a failing find still records $?. End with # ``exit`` of that status (126 if the file is missing): the last command # would otherwise be ``rm``, whose 0/1 is not find's status. + # ``set +e`` stays outermost (pinned by existing tests); the rest runs in a + # ( ... ) subshell: a bare ``exit`` in the implicit persistent session kills + # the session's shell process and the AIO server's response path for that + # request hangs forever (verified: bare ``exit 0`` always wedges; a subshell + # ``exit`` only kills the subshell, the session survives, and the exit code + # and output propagate unchanged). return ( - f"set +e; if [ ! -e {quoted} ]; then printf '%s\\n' {_STATUS_PREFIX}{_MISSING_ROOT}; exit 1; fi; " + f"set +e; ( if [ ! -e {quoted} ]; then printf '%s\\n' {_STATUS_PREFIX}{_MISSING_ROOT}; exit 1; fi; " f"_st=/tmp/df_find_$$; " f"{{ find -H {quoted} -maxdepth {depth} \\( -type f -o -type d \\) 2>/dev/null; " f'echo $? > "$_st"; }} | head -n {n}; ' f'st=$(cat "$_st" 2>/dev/null); ' f"printf '\\n%s\\n' {_STATUS_PREFIX}$st; " - f'rm -f "$_st"; exit "${{st:-126}}"' + f'rm -f "$_st"; exit "${{st:-126}}" )' ) diff --git a/backend/packages/harness/deerflow/sandbox/remote_search.py b/backend/packages/harness/deerflow/sandbox/remote_search.py index 908f23204..ad5342d0f 100644 --- a/backend/packages/harness/deerflow/sandbox/remote_search.py +++ b/backend/packages/harness/deerflow/sandbox/remote_search.py @@ -54,11 +54,16 @@ def remote_search_command(search: str, root: str, *, limit: int) -> str: quoted = shlex.quote(root) # One extra line is the truncation signal; the parser drops it. n = int(limit) + 1 + # ``set +e`` stays outermost (pinned by existing tests); the rest runs in a + # ( ... ) subshell: a bare ``exit`` in the implicit persistent session kills + # the session's shell process and the AIO server's response path hangs + # forever; a subshell ``exit`` only kills the subshell, so the session + # survives and the exit code propagates unchanged. return ( - f"set +e; if [ ! -e {quoted} ]; then printf '%s\\n' {_STATUS_PREFIX}{_MISSING_ROOT}; exit 0; fi; " + f"set +e; ( if [ ! -e {quoted} ]; then printf '%s\\n' {_STATUS_PREFIX}{_MISSING_ROOT}; exit 0; fi; " f'_st=/tmp/df_search_$$; {{ {search}; echo $? > "$_st"; }} | head -n {n}; ' f'st=$(cat "$_st" 2>/dev/null); rm -f "$_st"; ' - f"printf '\\n%s\\n' {_STATUS_PREFIX}\"$st\"; exit 0" + f"printf '\\n%s\\n' {_STATUS_PREFIX}\"$st\"; exit 0 )" ) diff --git a/backend/tests/test_remote_list_dir.py b/backend/tests/test_remote_list_dir.py index 052467b70..83d9c6eb9 100644 --- a/backend/tests/test_remote_list_dir.py +++ b/backend/tests/test_remote_list_dir.py @@ -89,6 +89,10 @@ def test_command_records_find_status_after_head() -> None: assert command.index("find -H ") < command.index("head -n") assert command.index("head -n") < command.rindex("__DF_FIND_STATUS__:") assert 'exit "${st:-126}"' in command + # Mirror pin (review on PR #5546): the exit must stay subshell-wrapped — a + # bare top-level exit wedges the implicit persistent session. A silent + # revert of the subshell wrap must fail this test. + assert command.endswith('exit "${st:-126}" )') def _run_list_dir_script(command: str, *, env: dict[str, str] | None = None) -> subprocess.CompletedProcess[str]: diff --git a/backend/tests/test_remote_search.py b/backend/tests/test_remote_search.py index d16d8d130..98228f555 100644 --- a/backend/tests/test_remote_search.py +++ b/backend/tests/test_remote_search.py @@ -121,7 +121,11 @@ def test_command_checks_root_first_and_records_status_after_head() -> None: # One line past the limit is what lets the parser tell a full result from a cut one. assert command.index("[ ! -e ") < command.index("grep ") < command.index("head -n 451") assert command.index("head -n 451") < command.rindex("__DF_SEARCH_STATUS__:") - assert command.endswith("exit 0") + # The command ends with a subshell-wrapped exit: a bare top-level exit would + # kill the implicit persistent session's shell and wedge the AIO server + # response for that request (verified); a subshell exit only kills the + # subshell and the exit code propagates unchanged. + assert command.endswith("exit 0 )") # ── real POSIX sh ─────────────────────────────────────────────────────────