fix: propagate workspace runtime in skill_exec

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

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