diff --git a/tests/skills/tools/test_skill_exec.py b/tests/skills/tools/test_skill_exec.py index 1593e5d9..aea41ca0 100644 --- a/tests/skills/tools/test_skill_exec.py +++ b/tests/skills/tools/test_skill_exec.py @@ -17,10 +17,15 @@ from trpc_agent_sdk.skills.tools._skill_exec import SkillExecTool from trpc_agent_sdk.skills.tools._skill_exec import WriteStdinTool from trpc_agent_sdk.skills.tools._skill_exec import _close_session +from trpc_agent_sdk.skills.tools._skill_exec import _collect_final_result from trpc_agent_sdk.skills.tools._skill_exec import _detect_interaction +from trpc_agent_sdk.skills.tools._skill_exec import _ExecSession from trpc_agent_sdk.skills.tools._skill_exec import _has_selection_items from trpc_agent_sdk.skills.tools._skill_exec import _last_non_empty_line +from trpc_agent_sdk.skills.tools._skill_exec import _start_session from trpc_agent_sdk.skills.tools._skill_exec import create_exec_tools +from trpc_agent_sdk.skills.tools._skill_run import SkillRunFile +from trpc_agent_sdk.skills.tools._skill_run import SkillRunInput def _make_exec_tool() -> SkillExecTool: @@ -107,3 +112,210 @@ async def test_close_session(self): sess.proc.close = AsyncMock() await _close_session(sess) sess.proc.close.assert_awaited_once() + + +class TestStartSession: + @pytest.mark.asyncio + async def test_start_session_stores_workspace_runtime(self): + workspace_runtime = MagicMock(name="workspace_runtime") + ws = MagicMock(name="ws") + proc = MagicMock(name="proc") + runner = MagicMock() + runner.start_program = AsyncMock(return_value=proc) + inputs = ExecInput(skill="s", command="echo hi") + + session = await _start_session( + runner=runner, + tool_context=MagicMock(), + inputs=inputs, + ws=ws, + workspace_runtime=workspace_runtime, + rel_cwd="skills/s", + env={"A": "1"}, + ) + + assert isinstance(session, _ExecSession) + assert session.workspace_runtime is workspace_runtime + assert session.ws is ws + assert session.proc is proc + assert session.in_data is inputs + + +class TestCollectFinalResult: + """Regression coverage for issue #350. + + ``_prepare_outputs`` requires ``workspace_runtime``. A plain ``AsyncMock`` + swallows the missing-argument ``TypeError``, so the collection path silently + degrades to empty files. These tests bind a signature-strict stub so a + dropped argument fails the assertions instead of being absorbed. + """ + + def _make_session(self, workspace_runtime, in_data=None, run_result=None): + proc = MagicMock() + proc.run_result = AsyncMock(return_value=run_result or MagicMock( + stdout="done\n", + stderr="", + exit_code=0, + timed_out=False, + duration=0.0, + )) + return _ExecSession( + proc=proc, + ws=MagicMock(name="ws"), + workspace_runtime=workspace_runtime, + in_data=in_data or ExecInput(skill="s", command="echo hi", output_files=["out/result.txt"]), + ) + + def _make_run_tool(self, prepare_side_effect=None, files=None): + run_tool = MagicMock() + run_tool._prepare_outputs = AsyncMock( + side_effect=prepare_side_effect, + return_value=(files if files is not None else [], None), + ) + run_tool._attach_artifacts_if_requested = AsyncMock() + run_tool._merge_manifest_artifact_refs = MagicMock() + return run_tool + + @pytest.mark.asyncio + async def test_collect_final_result_passes_workspace_runtime_and_files(self): + workspace_runtime = MagicMock(name="workspace_runtime") + session = self._make_session(workspace_runtime) + expected_files = [SkillRunFile( + name="out/result.txt", + content="ok", + mime_type="text/plain", + size_bytes=2, + )] + + async def _prepare_outputs(ctx, ws, runtime, input_data): + # Signature-strict: a missing argument raises TypeError here and + # the production `except Exception` would degrade to empty files. + assert runtime is workspace_runtime + assert input_data.output_files == ["out/result.txt"] + return list(expected_files), None + + run_tool = self._make_run_tool(prepare_side_effect=_prepare_outputs) + ctx = MagicMock() + result = await _collect_final_result(ctx, session, run_tool) + + assert result is not None + assert result.exit_code == 0 + assert result.output_files == expected_files + assert result.primary_output is not None + assert result.primary_output.name == "out/result.txt" + assert result.timed_out is False + assert result.stderr == "" + assert session.finalized is True + assert session.final_result is result + + run_tool._prepare_outputs.assert_awaited_once() + call_args = run_tool._prepare_outputs.await_args.args + expected_input = SkillRunInput( + skill="s", + command="echo hi", + output_files=["out/result.txt"], + ) + assert call_args == (ctx, session.ws, workspace_runtime, expected_input) + + @pytest.mark.asyncio + async def test_collect_final_result_returns_cached_when_finalized(self): + session = self._make_session(MagicMock()) + cached = MagicMock(name="cached") + session.finalized = True + session.final_result = cached + + run_tool = self._make_run_tool() + + result = await _collect_final_result(MagicMock(), session, run_tool) + assert result is cached + run_tool._prepare_outputs.assert_not_awaited() + + @pytest.mark.asyncio + async def test_collect_final_result_omit_inline_content_clears_file_content(self): + in_data = ExecInput( + skill="s", + command="echo hi", + output_files=["out/result.txt"], + omit_inline_content=True, + ) + session = self._make_session(MagicMock(), in_data=in_data) + collected = [SkillRunFile( + name="out/result.txt", + content="secret payload", + mime_type="text/plain", + size_bytes=14, + )] + run_tool = self._make_run_tool(files=collected) + + result = await _collect_final_result(MagicMock(), session, run_tool) + + assert result is not None + assert len(result.output_files) == 1 + assert result.output_files[0].content == "" + assert result.output_files[0].name == "out/result.txt" + assert result.output_files[0].size_bytes == 14 + assert result.primary_output is not None + assert result.primary_output.content == "" + + @pytest.mark.asyncio + async def test_collect_final_result_propagates_timed_out(self): + run_result = MagicMock(stdout="", stderr="", exit_code=124, timed_out=True, duration=1.5) + session = self._make_session(MagicMock(), run_result=run_result) + collected = [SkillRunFile(name="out/partial.txt", content="", mime_type="text/plain", size_bytes=0)] + run_tool = self._make_run_tool(files=collected) + + result = await _collect_final_result(MagicMock(), session, run_tool) + + assert result is not None + assert result.timed_out is True + assert result.exit_code == 124 + assert result.duration_ms == 1500 + # Failed + timed_out run drops empty output files (same as skill_run). + assert result.output_files == [] + assert any("empty output_files" in w for w in result.warnings) + + @pytest.mark.asyncio + async def test_collect_final_result_fills_stderr(self): + run_result = MagicMock(stdout="out\n", stderr="boom\n", exit_code=1, timed_out=False, duration=0.25) + session = self._make_session(MagicMock(), run_result=run_result) + run_tool = self._make_run_tool() + + result = await _collect_final_result(MagicMock(), session, run_tool) + + assert result is not None + assert result.stdout == "out\n" + assert result.stderr == "boom\n" + assert result.duration_ms == 250 + + @pytest.mark.asyncio + async def test_collect_final_result_warns_on_collection_failure(self): + async def _prepare_outputs(ctx, ws, runtime, input_data): + raise RuntimeError("glob failed") + + session = self._make_session(MagicMock()) + run_tool = self._make_run_tool(prepare_side_effect=_prepare_outputs) + + result = await _collect_final_result(MagicMock(), session, run_tool) + + assert result is not None + assert result.output_files == [] + assert any("output collection failed" in w for w in result.warnings) + # Still finalized so later polls do not retry a broken collection. + assert session.finalized is True + + @pytest.mark.asyncio + async def test_collect_final_result_exception_branch_survives_log_failure(self): + session = self._make_session(MagicMock()) + session.proc.run_result = AsyncMock(side_effect=RuntimeError("session gone")) + session.proc.log = AsyncMock(side_effect=RuntimeError("log gone")) + session.exit_code = None + run_tool = self._make_run_tool() + + result = await _collect_final_result(MagicMock(), session, run_tool) + + assert result is not None + assert result.stdout == "" + # Unknown exit code is surfaced via warnings instead of silent success. + assert result.exit_code == 0 + assert any("exit code unavailable" in w for w in result.warnings) + assert session.finalized is True diff --git a/trpc_agent_sdk/skills/tools/_skill_exec.py b/trpc_agent_sdk/skills/tools/_skill_exec.py index b093088b..993dc77e 100644 --- a/trpc_agent_sdk/skills/tools/_skill_exec.py +++ b/trpc_agent_sdk/skills/tools/_skill_exec.py @@ -52,6 +52,7 @@ from pydantic import Field from trpc_agent_sdk.code_executors import BaseProgramRunner from trpc_agent_sdk.code_executors import BaseProgramSession +from trpc_agent_sdk.code_executors import BaseWorkspaceRuntime from trpc_agent_sdk.code_executors import DEFAULT_EXEC_YIELD_MS from trpc_agent_sdk.code_executors import DEFAULT_IO_YIELD_MS from trpc_agent_sdk.code_executors import DEFAULT_SESSION_KILL_SEC @@ -90,6 +91,8 @@ from ._skill_run import SkillRunInput from ._skill_run import SkillRunOutput from ._skill_run import SkillRunTool +from ._skill_run import _WARN_STDERR_TRUNCATED +from ._skill_run import _WARN_STDOUT_TRUNCATED from ._skill_run import _filter_failed_empty_outputs from ._skill_run import _select_primary_output from ._skill_run import _truncate_output @@ -216,6 +219,7 @@ class _ExecSession: proc: BaseProgramSession ws: WorkspaceInfo + workspace_runtime: BaseWorkspaceRuntime in_data: ExecInput # Final state @@ -458,6 +462,7 @@ async def _run_async_impl( tool_context=tool_context, inputs=inputs, ws=ws, + workspace_runtime=workspace_runtime, rel_cwd=rel_cwd, env=merged_env, ) @@ -730,6 +735,7 @@ async def _start_session( tool_context: InvocationContext, inputs: ExecInput, ws: WorkspaceInfo, + workspace_runtime: BaseWorkspaceRuntime, rel_cwd: str, env: dict[str, str], ) -> _ExecSession: @@ -744,7 +750,7 @@ async def _start_session( tty=inputs.tty, ) proc = await runner.start_program(tool_context, ws, spec) - return _ExecSession(proc=proc, ws=ws, in_data=inputs) + return _ExecSession(proc=proc, ws=ws, workspace_runtime=workspace_runtime, in_data=inputs) async def _write_stdin(exec_session: _ExecSession, chars: str, submit: bool) -> None: @@ -778,35 +784,56 @@ async def _collect_final_result( inputs=in_data.inputs, outputs=in_data.outputs, ) + warnings: list[str] = [] try: - files, manifest = await run_tool._prepare_outputs(ctx, exec_session.ws, fake_run_input) + files, manifest = await run_tool._prepare_outputs(ctx, exec_session.ws, exec_session.workspace_runtime, + fake_run_input) except Exception as ex: # pylint: disable=broad-except logger.warning("skill_exec: collect outputs failed: %s", ex) files, manifest = [], None + warnings.append(f"output collection failed: {ex}") + timed_out = False + duration_ms = 0 try: run_result = await exec_session.proc.run_result() - total_out = (run_result.stdout or "") + (run_result.stderr or "") + stdout_raw = run_result.stdout or "" + stderr_raw = run_result.stderr or "" exit_code = run_result.exit_code + timed_out = bool(getattr(run_result, "timed_out", False)) + duration_ms = int((getattr(run_result, "duration", 0) or 0) * 1000) except Exception: # pylint: disable=broad-except - run_log = await exec_session.proc.log(None, None) - total_out = run_log.output or "" - exit_code = exec_session.exit_code or 0 + try: + run_log = await exec_session.proc.log(None, None) + stdout_raw = run_log.output or "" + except Exception: # pylint: disable=broad-except + stdout_raw = "" + stderr_raw = "" + if exec_session.exit_code is not None: + exit_code = exec_session.exit_code + else: + exit_code = 0 + warnings.append("exit code unavailable after run_result failure; reported as 0") # Reuse the same output-quality helpers as skill_run - warnings: list[str] = [] - stdout, trunc = _truncate_output(total_out) + stdout, trunc = _truncate_output(stdout_raw) if trunc: - warnings.append("stdout truncated") + warnings.append(_WARN_STDOUT_TRUNCATED) + stderr, trunc = _truncate_output(stderr_raw) + if trunc: + warnings.append(_WARN_STDERR_TRUNCATED) - files, filter_warns = _filter_failed_empty_outputs(exit_code, False, files) + files, filter_warns = _filter_failed_empty_outputs(exit_code, timed_out, files) warnings.extend(filter_warns) primary = _select_primary_output(files) result = SkillRunOutput( stdout=stdout, + stderr=stderr, exit_code=exit_code, + timed_out=timed_out, + duration_ms=duration_ms, output_files=files, primary_output=primary, warnings=warnings, @@ -820,6 +847,13 @@ async def _collect_final_result( if manifest: run_tool._merge_manifest_artifact_refs(manifest, result) + # omit_inline_content: keep metadata only, match skill_run contract + if in_data.omit_inline_content: + for f in result.output_files: + f.content = "" + if result.primary_output: + result.primary_output.content = "" + exec_session.final_result = result exec_session.finalized = True return result