From b6c3e8757bb3e1e3462be2b21544fbaaaf2ceaec Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sat, 14 Mar 2026 21:48:46 +0000 Subject: [PATCH] fix(bindings): surface pre-exec failures in stderr for Python/JS integrations Pre-exec failures (parse errors, resource limits) were only stored in the `error` field with an empty `stderr`. Agent integrations like deepagents.py and pydantic_ai.py ignored `result.error`, leading to opaque failures. Now the error message is copied into `stderr` in both Python and JS bindings, and both agent integrations surface `result.error`. Closes #606 --- .../__test__/error-handling.spec.ts | 22 +++++++ crates/bashkit-js/src/lib.rs | 60 +++++++++++-------- crates/bashkit-python/bashkit/deepagents.py | 14 ++++- crates/bashkit-python/bashkit/pydantic_ai.py | 2 + crates/bashkit-python/src/lib.rs | 30 ++++++---- crates/bashkit-python/tests/test_bashkit.py | 38 ++++++++++++ 6 files changed, 128 insertions(+), 38 deletions(-) diff --git a/crates/bashkit-js/__test__/error-handling.spec.ts b/crates/bashkit-js/__test__/error-handling.spec.ts index 86836aaf..cfa5abbc 100644 --- a/crates/bashkit-js/__test__/error-handling.spec.ts +++ b/crates/bashkit-js/__test__/error-handling.spec.ts @@ -194,3 +194,25 @@ test("unclosed quote returns non-zero or handles gracefully", (t) => { // Should either error or handle gracefully t.is(typeof r.exitCode, "number"); }); + +// ============================================================================ +// Pre-exec failure stderr surfacing (issue #606) +// ============================================================================ + +test("pre-exec parse error surfaces in stderr (Bash)", (t) => { + const bash = new Bash(); + const r = bash.executeSync("echo $("); + t.not(r.exitCode, 0); + t.truthy(r.error, "error field should contain the parse error"); + t.truthy(r.stderr, "stderr must not be empty on pre-exec failure"); + t.true(r.stderr.includes(r.error!), "stderr should contain the error message"); +}); + +test("pre-exec parse error surfaces in stderr (BashTool)", (t) => { + const tool = new BashTool(); + const r = tool.executeSync("echo $("); + t.not(r.exitCode, 0); + t.truthy(r.error, "error field should contain the parse error"); + t.truthy(r.stderr, "stderr must not be empty on pre-exec failure"); + t.true(r.stderr.includes(r.error!), "stderr should contain the error message"); +}); diff --git a/crates/bashkit-js/src/lib.rs b/crates/bashkit-js/src/lib.rs index 4bd5a45e..cf4f8ab2 100644 --- a/crates/bashkit-js/src/lib.rs +++ b/crates/bashkit-js/src/lib.rs @@ -118,12 +118,15 @@ impl Bash { exit_code: result.exit_code, error: None, }), - Err(e) => Ok(ExecResult { - stdout: String::new(), - stderr: String::new(), - exit_code: 1, - error: Some(e.to_string()), - }), + Err(e) => { + let msg = e.to_string(); + Ok(ExecResult { + stdout: String::new(), + stderr: msg.clone(), + exit_code: 1, + error: Some(msg), + }) + } } }) } @@ -140,12 +143,15 @@ impl Bash { exit_code: result.exit_code, error: None, }), - Err(e) => Ok(ExecResult { - stdout: String::new(), - stderr: String::new(), - exit_code: 1, - error: Some(e.to_string()), - }), + Err(e) => { + let msg = e.to_string(); + Ok(ExecResult { + stdout: String::new(), + stderr: msg.clone(), + exit_code: 1, + error: Some(msg), + }) + } } } @@ -269,12 +275,15 @@ impl BashTool { exit_code: result.exit_code, error: None, }), - Err(e) => Ok(ExecResult { - stdout: String::new(), - stderr: String::new(), - exit_code: 1, - error: Some(e.to_string()), - }), + Err(e) => { + let msg = e.to_string(); + Ok(ExecResult { + stdout: String::new(), + stderr: msg.clone(), + exit_code: 1, + error: Some(msg), + }) + } } }) } @@ -291,12 +300,15 @@ impl BashTool { exit_code: result.exit_code, error: None, }), - Err(e) => Ok(ExecResult { - stdout: String::new(), - stderr: String::new(), - exit_code: 1, - error: Some(e.to_string()), - }), + Err(e) => { + let msg = e.to_string(); + Ok(ExecResult { + stdout: String::new(), + stderr: msg.clone(), + exit_code: 1, + error: Some(msg), + }) + } } } diff --git a/crates/bashkit-python/bashkit/deepagents.py b/crates/bashkit-python/bashkit/deepagents.py index a6577cb7..a83bd7d2 100644 --- a/crates/bashkit-python/bashkit/deepagents.py +++ b/crates/bashkit-python/bashkit/deepagents.py @@ -71,6 +71,8 @@ def _make_bash_tool(bash_instance: NativeBashTool): def bashkit(command: str) -> str: result = bash_instance.execute_sync(command) output = result.stdout + if result.error: + output += f"\nError: {result.error}" if result.stderr: output += f"\n{result.stderr}" if result.exit_code != 0: @@ -134,7 +136,10 @@ def tools(self): def execute_sync(self, command: str) -> str: """Execute command synchronously (for setup scripts).""" result = self._bash.execute_sync(command) - return result.stdout + (result.stderr or "") + output = result.stdout + (result.stderr or "") + if result.error and result.error not in output: + output += f"\nError: {result.error}" + return output def reset(self) -> None: """Reset VFS to initial state.""" @@ -189,6 +194,8 @@ def create_middleware(self) -> BashkitMiddleware: def execute(self, command: str) -> ExecuteResponse: result = self._bash.execute_sync(command) output = result.stdout + (result.stderr or "") + if result.error and result.error not in output: + output += f"\nError: {result.error}" return ExecuteResponse(output=output, exit_code=result.exit_code, truncated=False) async def aexecute(self, command: str) -> ExecuteResponse: @@ -344,7 +351,10 @@ async def aupload_files(self, files: list[tuple[str, bytes]]) -> list[FileUpload def setup(self, script: str) -> str: """Execute setup script.""" result = self._bash.execute_sync(script) - return result.stdout + (result.stderr or "") + output = result.stdout + (result.stderr or "") + if result.error and result.error not in output: + output += f"\nError: {result.error}" + return output def reset(self) -> None: """Reset VFS.""" diff --git a/crates/bashkit-python/bashkit/pydantic_ai.py b/crates/bashkit-python/bashkit/pydantic_ai.py index be6ffd25..349aa0a9 100644 --- a/crates/bashkit-python/bashkit/pydantic_ai.py +++ b/crates/bashkit-python/bashkit/pydantic_ai.py @@ -73,6 +73,8 @@ async def bash(commands: str) -> str: result = await native.execute(commands) output = result.stdout + if result.error: + output += f"\nError: {result.error}" if result.stderr: output += f"\nSTDERR: {result.stderr}" if result.exit_code != 0: diff --git a/crates/bashkit-python/src/lib.rs b/crates/bashkit-python/src/lib.rs index 5f9b5de5..4bc7b476 100644 --- a/crates/bashkit-python/src/lib.rs +++ b/crates/bashkit-python/src/lib.rs @@ -268,12 +268,15 @@ impl PyBash { exit_code: result.exit_code, error: None, }), - Err(e) => Ok(ExecResult { - stdout: String::new(), - stderr: String::new(), - exit_code: 1, - error: Some(e.to_string()), - }), + Err(e) => { + let msg = e.to_string(); + Ok(ExecResult { + stdout: String::new(), + stderr: msg.clone(), + exit_code: 1, + error: Some(msg), + }) + } } }) } @@ -476,12 +479,15 @@ impl BashTool { exit_code: result.exit_code, error: None, }), - Err(e) => Ok(ExecResult { - stdout: String::new(), - stderr: String::new(), - exit_code: 1, - error: Some(e.to_string()), - }), + Err(e) => { + let msg = e.to_string(); + Ok(ExecResult { + stdout: String::new(), + stderr: msg.clone(), + exit_code: 1, + error: Some(msg), + }) + } } }) } diff --git a/crates/bashkit-python/tests/test_bashkit.py b/crates/bashkit-python/tests/test_bashkit.py index c365fb44..e9a9f90a 100644 --- a/crates/bashkit-python/tests/test_bashkit.py +++ b/crates/bashkit-python/tests/test_bashkit.py @@ -954,3 +954,41 @@ def test_deeply_nested_schema_rejected(): tool = ScriptedTool("deep") with pytest.raises(ValueError, match="nesting depth"): tool.add_tool("deep", "Deep", callback=lambda p, s=None: "", schema=nested) + + +# =========================================================================== +# Pre-exec failure stderr surfacing (issue #606) +# =========================================================================== + + +def test_bash_pre_exec_error_in_stderr(): + """Pre-exec failures (parse errors) must appear in stderr, not only error field.""" + bash = Bash() + # Unclosed subshell triggers parse error -> Err path in bindings + r = bash.execute_sync("echo $(") + assert r.exit_code != 0 + assert r.error is not None + # Bug #606: stderr was empty even though error had the message + assert r.stderr != "", "stderr must contain the error message, not be empty" + assert r.error in r.stderr + + +def test_bashtool_pre_exec_error_in_stderr(): + """BashTool pre-exec failures must also surface in stderr.""" + tool = BashTool() + r = tool.execute_sync("echo $(") + assert r.exit_code != 0 + assert r.error is not None + assert r.stderr != "", "stderr must contain the error message, not be empty" + assert r.error in r.stderr + + +@pytest.mark.asyncio +async def test_bash_pre_exec_error_in_stderr_async(): + """Async path should also surface pre-exec errors in stderr.""" + bash = Bash() + r = await bash.execute("echo $(") + assert r.exit_code != 0 + assert r.error is not None + assert r.stderr != "", "stderr must contain the error message, not be empty" + assert r.error in r.stderr