mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(sandbox): keep output and exit code when an e2b command fails (#6441)
* fix(sandbox): keep output and exit code when an e2b command fails * docs(changelog): link #6441 * fix(sandbox): keep list_dir output when an e2b listing exits nonzero --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
7dedb10dca
commit
7ffe96a910
13
CHANGELOG.md
13
CHANGELOG.md
@ -804,6 +804,18 @@ This release closes that milestone with **439 merged pull requests**.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **sandbox:** A failed command in the e2b sandbox now keeps its output and exit
|
||||
code. The e2b SDK raises `CommandExitException` on a nonzero exit instead of
|
||||
returning a result, so `E2BSandbox.execute_command` returned
|
||||
`Error: Command exited with code N and error: ...`: stdout was dropped, the
|
||||
`Exit Code: N` marker was never added, and `_bash_evidence_status` fell back to
|
||||
`deerflow_tool_meta` (`success`), so a failed `pytest` could satisfy a
|
||||
`tests_passed` acceptance criterion. The exception carries the command's
|
||||
stdout, stderr and exit code, and is now formatted like a returned result. A
|
||||
failed command whose stderr mentions "sandbox not found" no longer marks the
|
||||
sandbox as reaped. `list_dir` on e2b handles the same exception, so a missing
|
||||
directory raises `FileNotFoundError` and a listing truncated at 500 entries
|
||||
(SIGPIPE 141) is returned instead of failing with `OSError`. ([#6441])
|
||||
- **uploads:** The Gateway's startup sweep of orphaned `.upload-*.part` staging
|
||||
files now skips files younger than 24 hours. The sweep removed every staging
|
||||
file it found, which was right for one Gateway but not for several replicas
|
||||
@ -9164,6 +9176,7 @@ with **180 merged pull requests** since the first 2.0 milestone tag.
|
||||
[#6407]: https://github.com/bytedance/deer-flow/pull/6407
|
||||
[#6412]: https://github.com/bytedance/deer-flow/pull/6412
|
||||
[#6426]: https://github.com/bytedance/deer-flow/pull/6426
|
||||
[#6441]: https://github.com/bytedance/deer-flow/pull/6441
|
||||
[#6445]: https://github.com/bytedance/deer-flow/pull/6445
|
||||
[#6447]: https://github.com/bytedance/deer-flow/pull/6447
|
||||
[#6448]: https://github.com/bytedance/deer-flow/pull/6448
|
||||
|
||||
@ -694,6 +694,14 @@
|
||||
|
||||
### 修复
|
||||
|
||||
- **沙箱:** e2b 沙箱中失败的命令现在会保留输出和退出码。e2b SDK 在退出码非零时抛出
|
||||
`CommandExitException`,而不是返回结果,因此 `E2BSandbox.execute_command` 返回的是
|
||||
`Error: Command exited with code N and error: ...`:stdout 被丢弃,`Exit Code: N`
|
||||
标记从未追加,`_bash_evidence_status` 退回到报告 `success` 的 `deerflow_tool_meta`,
|
||||
失败的 `pytest` 可能满足 `tests_passed` 验收条件。该异常本身带有命令的 stdout、stderr
|
||||
与退出码,现在会按返回结果的方式格式化。stderr 中恰好包含 "sandbox not found" 的失败命令
|
||||
也不再把沙箱标记为已回收。e2b 的 `list_dir` 也会处理同一异常:目录不存在时抛出
|
||||
`FileNotFoundError`,超过 500 条被截断(SIGPIPE 141)的列表会正常返回,不再以 `OSError` 失败。([#6441])
|
||||
- **前端:** 附件上传失败后重试发送时,现在会保留原先附带的上下文。输入框此前在发送开始时(附件上传之前)就清除了
|
||||
引用、对话引用、已暂存的项目文件和已保存的草稿,因此上传失败后文字和文件虽仍在,重试发送却缺少这些上下文。
|
||||
现在这些一次性状态只在发送真正派发(上传完成)后才清除;若上传完成时用户已切换对话或离开页面,只清除该次发送
|
||||
@ -7506,4 +7514,5 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写,核心包含子
|
||||
[#6407]: https://github.com/bytedance/deer-flow/pull/6407
|
||||
[#6412]: https://github.com/bytedance/deer-flow/pull/6412
|
||||
[#6426]: https://github.com/bytedance/deer-flow/pull/6426
|
||||
[#6441]: https://github.com/bytedance/deer-flow/pull/6441
|
||||
[#6448]: https://github.com/bytedance/deer-flow/pull/6448
|
||||
|
||||
@ -7,7 +7,7 @@ import shlex
|
||||
import threading
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from e2b import FileNotFoundException
|
||||
from e2b import CommandExitException, FileNotFoundException
|
||||
from e2b_code_interpreter import Sandbox as E2BClientSandbox
|
||||
|
||||
from deerflow.config.paths import VIRTUAL_PATH_PREFIX
|
||||
@ -165,7 +165,13 @@ class E2BSandbox(Sandbox):
|
||||
kwargs["envs"] = env
|
||||
if timeout is not None:
|
||||
kwargs["timeout"] = timeout
|
||||
result = client.commands.run(command, **kwargs)
|
||||
try:
|
||||
result = client.commands.run(command, **kwargs)
|
||||
except CommandExitException as exc:
|
||||
# The SDK raises on a nonzero exit instead of returning a
|
||||
# result. The exception is itself a ``CommandResult``, so
|
||||
# format it like one to keep stdout and the exit marker.
|
||||
result = exc
|
||||
stdout = getattr(result, "stdout", "") or ""
|
||||
stderr = getattr(result, "stderr", "") or ""
|
||||
exit_code = getattr(result, "exit_code", 0)
|
||||
@ -347,6 +353,12 @@ class E2BSandbox(Sandbox):
|
||||
raise RuntimeError("sandbox client has been closed")
|
||||
try:
|
||||
result = client.commands.run(remote_list_dir_command(resolved, max_depth))
|
||||
except CommandExitException as exc:
|
||||
# The listing script exits nonzero on reachable outcomes (missing
|
||||
# root: 1; head truncating a large listing: SIGPIPE 141). The SDK
|
||||
# raises for those, but the exception carries stdout, whose
|
||||
# status marker the parser trusts over the exit code.
|
||||
result = exc
|
||||
except Exception as e:
|
||||
logger.error("Failed to list_dir %s in e2b sandbox: %s", resolved, e)
|
||||
raise OSError(f"Failed to list_dir {resolved} in e2b sandbox: {e}") from e
|
||||
|
||||
@ -5,11 +5,13 @@ from __future__ import annotations
|
||||
import asyncio
|
||||
import hashlib
|
||||
import importlib
|
||||
import importlib.util
|
||||
import json
|
||||
import os
|
||||
import shutil
|
||||
import signal
|
||||
import subprocess
|
||||
import sys
|
||||
import threading
|
||||
import time
|
||||
from collections import OrderedDict
|
||||
@ -22,7 +24,8 @@ from typing import Any
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
from e2b import FileNotFoundException, TimeoutException
|
||||
from e2b import CommandExitException, FileNotFoundException, TimeoutException
|
||||
from langchain_core.messages import AIMessage, HumanMessage, ToolMessage
|
||||
from pydantic import ValidationError
|
||||
|
||||
from deerflow.community.e2b_sandbox.capacity import (
|
||||
@ -1751,15 +1754,79 @@ def test_execute_command_returns_stdout_on_success():
|
||||
assert sb.is_dead is False
|
||||
|
||||
|
||||
def _raise_exit(stdout: str, stderr: str, exit_code: int):
|
||||
"""The real SDK raises ``CommandExitException`` on a nonzero exit; it
|
||||
never returns a result with ``exit_code != 0``."""
|
||||
|
||||
def run(_cmd: str) -> Any:
|
||||
raise CommandExitException(stderr=stderr, stdout=stdout, exit_code=exit_code, error=f"exit status {exit_code}")
|
||||
|
||||
return run
|
||||
|
||||
|
||||
def test_execute_command_appends_exit_marker_when_failure_has_output():
|
||||
"""LocalSandbox parity: a nonzero exit must survive in the output text
|
||||
even when the command produced output, so evidence consumers (acceptance
|
||||
checklist) recover the actual shell status."""
|
||||
client = FakeClient(commands=FakeCommandsAPI([SimpleNamespace(stdout="5 passed, 1 error\n", stderr="", exit_code=1)]))
|
||||
client = FakeClient(commands=FakeCommandsAPI([_raise_exit("5 passed, 1 error\n", "", 1)]))
|
||||
sb = _make_sandbox(client)
|
||||
assert sb.execute_command("make test") == "5 passed, 1 error\n\nExit Code: 1"
|
||||
|
||||
|
||||
def test_execute_command_keeps_stdout_and_stderr_on_nonzero_exit():
|
||||
client = FakeClient(commands=FakeCommandsAPI([_raise_exit("12 passed\n", "coverage below 80%\n", 2)]))
|
||||
sb = _make_sandbox(client)
|
||||
assert sb.execute_command("pytest -q") == "12 passed\n\ncoverage below 80%\n\nExit Code: 2"
|
||||
assert sb.is_dead is False
|
||||
|
||||
|
||||
def test_execute_command_reports_exit_code_when_failure_has_no_output():
|
||||
client = FakeClient(commands=FakeCommandsAPI([_raise_exit("", "", 1)]))
|
||||
sb = _make_sandbox(client)
|
||||
assert sb.execute_command("false") == "Command exited with code 1"
|
||||
|
||||
|
||||
def test_execute_command_nonzero_exit_mentioning_not_found_does_not_mark_dead():
|
||||
"""A command's own stderr can contain "sandbox not found"; that is a failed
|
||||
command, not a reaped VM."""
|
||||
client = FakeClient(commands=FakeCommandsAPI([_raise_exit("", "error: sandbox not found\n", 1)]))
|
||||
sb = _make_sandbox(client)
|
||||
assert sb.execute_command("mytool status") == "error: sandbox not found\n\nExit Code: 1"
|
||||
assert sb.is_dead is False
|
||||
|
||||
|
||||
def test_execute_command_nonzero_exit_is_harvested_as_error():
|
||||
"""End-to-end with the real executor harvest. ``tests/conftest.py`` mocks
|
||||
``deerflow.subagents.executor``, so it is loaded under a unique name."""
|
||||
path = Path(__file__).parents[1] / "packages/harness/deerflow/subagents/executor.py"
|
||||
spec = importlib.util.spec_from_file_location("_e2b_exit_marker_executor", path)
|
||||
assert spec is not None and spec.loader is not None
|
||||
module = importlib.util.module_from_spec(spec)
|
||||
try:
|
||||
sys.modules[spec.name] = module
|
||||
spec.loader.exec_module(module)
|
||||
from deerflow.subagents import acceptance_checks
|
||||
|
||||
client = FakeClient(commands=FakeCommandsAPI([_raise_exit("12 passed\n", "coverage below 80%\n", 1)]))
|
||||
out = _make_sandbox(client).execute_command("pytest -q")
|
||||
|
||||
cmd = "pytest -q"
|
||||
ai = AIMessage(content="", tool_calls=[{"name": "bash", "args": {"command": cmd}, "id": "tc-1", "type": "tool_call"}])
|
||||
tm = ToolMessage(content=out, tool_call_id="tc-1", name="bash")
|
||||
executions = module._harvest_bash_executions({"messages": [HumanMessage(content="task"), ai, tm]})
|
||||
assert executions, "harvest returned no bash executions"
|
||||
for entry in executions:
|
||||
entry["shell_persistent"] = False
|
||||
assert executions[-1]["status"] == "error"
|
||||
assert executions[-1]["status_marker"] == "Exit Code: 1"
|
||||
assert acceptance_checks._check_tests_passed_leaf(cmd, executions)["holds"] is False
|
||||
finally:
|
||||
sys.modules.pop(spec.name, None)
|
||||
shutdown = getattr(module, "_shutdown_isolated_subagent_loop", None)
|
||||
if shutdown is not None:
|
||||
shutdown()
|
||||
|
||||
|
||||
def test_execute_command_does_not_mark_dead_on_unrelated_error():
|
||||
|
||||
def boom(_cmd: str, **kwargs) -> Any:
|
||||
@ -5586,8 +5653,9 @@ def test_list_dir_raises_when_client_closed():
|
||||
|
||||
@pytest.mark.parametrize("marker, error", [("missing", FileNotFoundError), ("1", OSError)])
|
||||
def test_list_dir_classifies_empty_failure(marker, error):
|
||||
listing = SimpleNamespace(stdout=f"\n__DF_FIND_STATUS__:{marker}\n", stderr="", exit_code=1)
|
||||
client = FakeClient(commands=FakeCommandsAPI([listing]))
|
||||
# The listing script exits 1 for both; the SDK raises CommandExitException,
|
||||
# whose stdout still carries the status marker the parser classifies by.
|
||||
client = FakeClient(commands=FakeCommandsAPI([_raise_exit(f"\n__DF_FIND_STATUS__:{marker}\n", "", 1)]))
|
||||
sb = _make_sandbox(client)
|
||||
|
||||
with pytest.raises(error) as exc:
|
||||
@ -5595,9 +5663,19 @@ def test_list_dir_classifies_empty_failure(marker, error):
|
||||
assert type(exc.value) is error
|
||||
|
||||
|
||||
def test_list_dir_returns_entries_when_head_truncation_exits_141():
|
||||
"""``head`` closing the pipe on a large listing kills ``find`` with SIGPIPE
|
||||
(141), a successful truncation. The SDK raises for the nonzero exit; the
|
||||
listing must still be returned, not surface as ``OSError``."""
|
||||
stdout = "/home/user\n/home/user/a\n/home/user/b\n\n__DF_FIND_STATUS__:141\n"
|
||||
client = FakeClient(commands=FakeCommandsAPI([_raise_exit(stdout, "", 141)]))
|
||||
sb = _make_sandbox(client)
|
||||
|
||||
assert sb.list_dir("/home/user") == ["/home/user", "/home/user/a", "/home/user/b"]
|
||||
|
||||
|
||||
def test_list_dir_raises_oserror_when_find_exit_is_not_missing_path():
|
||||
listing = SimpleNamespace(stdout="", stderr="", exit_code=127)
|
||||
client = FakeClient(commands=FakeCommandsAPI([listing]))
|
||||
client = FakeClient(commands=FakeCommandsAPI([_raise_exit("", "", 127)]))
|
||||
sb = _make_sandbox(client)
|
||||
|
||||
with pytest.raises(OSError, match="exited with code 127"):
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user