diff --git a/backend/packages/harness/deerflow/community/opensandbox/AGENTS.md b/backend/packages/harness/deerflow/community/opensandbox/AGENTS.md index 8355032d2d..20853185ef 100644 --- a/backend/packages/harness/deerflow/community/opensandbox/AGENTS.md +++ b/backend/packages/harness/deerflow/community/opensandbox/AGENTS.md @@ -1,3 +1,11 @@ +### Command deadline reporting + +Normalize only `CommandExecError` / `context deadline exceeded` with no numeric +exit code to `Exit Code: 124`, preserving partial output. Keep transport errors, +other missing-code errors and numeric exits unchanged. A signal-only `-1` does +not prove a deadline. Tests must use execution results, not a fake-only raised +`TimeoutError`, for the deadline contract. + ### OpenSandbox output synchronization Release mirrors scoped regular output files to `Paths.sandbox_outputs_dir()` diff --git a/backend/packages/harness/deerflow/community/opensandbox/README.md b/backend/packages/harness/deerflow/community/opensandbox/README.md index 05877a68b6..ae75adf09c 100644 --- a/backend/packages/harness/deerflow/community/opensandbox/README.md +++ b/backend/packages/harness/deerflow/community/opensandbox/README.md @@ -94,7 +94,15 @@ The full DeerFlow surface is implemented: - `execute_command` forwards per-call environment variables and positive wall-clock timeouts through `RunCommandOpts`, preserving stdout, stderr, and - non-zero exit information in DeerFlow's string result. + non-zero exit information in DeerFlow's string result. An explicit + `CommandExecError` with value `context deadline exceeded` and no numeric + exit code ends with `Error: command timed out` and `Exit Code: 124`, keeping + any partial output. Transport exceptions and other missing-code errors keep + their existing error reporting. Numeric exits, including signal termination + (`-1`), keep their own code: a signal-only event does not identify whether a + command deadline or another kill caused it. Servers that expose only that + signal status need additional deadline information before it can safely be + normalized to 124. - Text and binary file operations use OpenSandbox's native filesystem API. Append is serialized as a read-modify-write because SDK 0.1.x has no append primitive. diff --git a/backend/packages/harness/deerflow/community/opensandbox/sandbox.py b/backend/packages/harness/deerflow/community/opensandbox/sandbox.py index 275d8059e3..ea2b5fc790 100644 --- a/backend/packages/harness/deerflow/community/opensandbox/sandbox.py +++ b/backend/packages/harness/deerflow/community/opensandbox/sandbox.py @@ -81,11 +81,7 @@ def execution_stdout(execution: Any) -> str: def format_execution(execution: Any) -> str: """Combine stdout, result text, and stderr using DeerFlow's string contract.""" - output = execution_stdout(execution) - result = _join_event_text(item.text for item in getattr(execution, "result", []) if getattr(item, "text", None) is not None) - output = _append_output(output, result) - stderr = _join_event_text(message.text for message in getattr(getattr(execution, "logs", None), "stderr", [])) - output = _append_output(output, stderr) + output = execution_body(execution) error = getattr(execution, "error", None) if error is not None: detail = f"{getattr(error, 'name', type(error).__name__)}: {getattr(error, 'value', error)}" @@ -93,6 +89,30 @@ def format_execution(execution: Any) -> str: return output +def execution_body(execution: Any) -> str: + """Combine the command's own stdout, result text and stderr. + + Excludes the error detail so a caller that reports the failure itself (the + timeout branch) can present the partial output without a second, rawer copy + of the same failure underneath it. + """ + output = execution_stdout(execution) + result = _join_event_text(item.text for item in getattr(execution, "result", []) if getattr(item, "text", None) is not None) + output = _append_output(output, result) + stderr = _join_event_text(message.text for message in getattr(getattr(execution, "logs", None), "stderr", [])) + return _append_output(output, stderr) + + +def _is_timeout_execution(execution: Any, exit_code: int | None) -> bool: + """Recognize an explicit command deadline, not an ambiguous signal kill. + + The SDK leaves exit_code unset when the error value is nonnumeric. A + bare -1 only indicates signal termination and cannot prove a timeout. + """ + error = getattr(execution, "error", None) + return exit_code is None and getattr(error, "name", None) == "CommandExecError" and getattr(error, "value", None) == "context deadline exceeded" + + class OpenSandboxSandbox(Sandbox): """Wrap one live ``opensandbox.sync.SandboxSync`` instance.""" @@ -254,8 +274,15 @@ class OpenSandboxSandbox(Sandbox): except Exception as exc: logger.error("Failed to execute command in OpenSandbox %s: %s", self.id, exc) return f"Error: {exc}" - output = format_execution(execution) exit_code = getattr(execution, "exit_code", None) + # Command deadlines arrive as execution error events, not builtin + # TimeoutError. Normalize only the explicit deadline result; transport + # errors, launch failures and signal-only exits keep their own details. + if _is_timeout_execution(execution, exit_code): + body = execution_body(execution) + message = "Error: command timed out\nExit Code: 124" + return f"{body}\n{message}" if body else message + output = format_execution(execution) if exit_code is None: detail = output or "no completion or error event" return f"Error: OpenSandbox command completed without an exit code: {detail}" diff --git a/backend/tests/test_opensandbox_provider.py b/backend/tests/test_opensandbox_provider.py index 7809dab9a3..de85ef32c0 100644 --- a/backend/tests/test_opensandbox_provider.py +++ b/backend/tests/test_opensandbox_provider.py @@ -1095,6 +1095,72 @@ def test_execute_forwards_env_timeout_and_combines_streams() -> None: assert box.execute_command("missing-complete") == "Error: OpenSandbox command completed without an exit code: stream ended" +def _command_error_execution(*, error_value: str, exit_code: int | None = None, error_name: str = "CommandExecError", stdout: tuple[str, ...] = (), stderr: tuple[str, ...] = (), result: tuple[str, ...] = ()) -> Any: + execution = _execution(stdout=stdout, stderr=stderr, result=result, exit_code=exit_code) + return types.SimpleNamespace(**vars(execution), error=types.SimpleNamespace(name=error_name, value=error_value)) + + +@pytest.mark.parametrize( + ("stdout", "stderr", "result", "expected_body"), + [ + ((), (), (), ""), + (("partial output",), (), (), "partial output\n"), + (("out-1", "out-2"), ("stderr",), ("result",), "out-1\nout-2\nresult\nstderr\n"), + ], +) +def test_execute_command_server_side_deadline_appends_exit_marker(stdout: tuple[str, ...], stderr: tuple[str, ...], result: tuple[str, ...], expected_body: str) -> None: + # The SDK cannot parse this deadline error value as an integer, so it + # returns an Execution with exit_code=None rather than raising TimeoutError. + remote = _FakeRemote("remote") + remote.commands.run = lambda command, opts=None: _command_error_execution(error_value="context deadline exceeded", stdout=stdout, stderr=stderr, result=result) # type: ignore[method-assign] + box = _box(remote, sandbox_timeout=timedelta(seconds=60), default_command_timeout=120) + + assert box.execute_command("sleep 9999", timeout=5) == f"{expected_body}Error: command timed out\nExit Code: 124" + + +@pytest.mark.parametrize("error_value", ["credential lookup failed", "context canceled", "signal: killed", "timeout while starting process", ""]) +def test_execute_command_other_missing_exit_codes_are_not_timeouts(error_value: str) -> None: + remote = _FakeRemote("remote") + remote.commands.run = lambda command, opts=None: _command_error_execution(error_value=error_value) # type: ignore[method-assign] + box = _box(remote, sandbox_timeout=timedelta(seconds=60), default_command_timeout=120) + + assert box.execute_command("true", timeout=5) == f"Error: OpenSandbox command completed without an exit code: CommandExecError: {error_value}" + + +@pytest.mark.parametrize("exit_code", [-1, 7, 124]) +def test_execute_command_numeric_error_events_keep_their_exit_code(exit_code: int) -> None: + # execd sends ordinary nonzero exits through CommandExecError too. In + # particular, -1 only proves signal termination, not a command deadline. + remote = _FakeRemote("remote") + remote.commands.run = lambda command, opts=None: _command_error_execution(error_value=str(exit_code), exit_code=exit_code, stdout=("partial output",)) # type: ignore[method-assign] + box = _box(remote, sandbox_timeout=timedelta(seconds=60), default_command_timeout=120) + + assert box.execute_command("exit-or-signal", timeout=5) == f"partial output\nCommandExecError: {exit_code}\nExit Code: {exit_code}" + + +@pytest.mark.parametrize("exit_code", [None, 7]) +def test_execute_command_deadline_text_on_other_errors_is_not_a_command_timeout(exit_code: int | None) -> None: + remote = _FakeRemote("remote") + remote.commands.run = lambda command, opts=None: _command_error_execution(error_value="context deadline exceeded", exit_code=exit_code, error_name="OtherError") # type: ignore[method-assign] + box = _box(remote, sandbox_timeout=timedelta(seconds=60), default_command_timeout=120) + + result = box.execute_command("true", timeout=5) + assert "OtherError: context deadline exceeded" in result + assert "command timed out" not in result + assert not result.endswith("Exit Code: 124") + + +@pytest.mark.parametrize("message", ["", "context deadline exceeded"]) +def test_execute_command_transport_exception_is_not_a_shell_timeout(message: str) -> None: + remote = _FakeRemote("remote") + remote.command_error = TimeoutError(message) + box = _box(remote, sandbox_timeout=timedelta(seconds=60), default_command_timeout=120) + + # Even an injected builtin TimeoutError must not turn a provider/renewal + # failure into an authoritative shell exit. The real SDK wraps this error. + assert box.execute_command("true", timeout=5) == f"Error: {message}" + + def test_operations_renew_remote_lifetime_and_bound_default_commands() -> None: remote = _FakeRemote("remote") remote.file_data["/mnt/user-data/workspace/note.txt"] = b"note"