refactor(code-review-agent): run sandbox scripts via workspace runtime

Author: k0mondor

examples/skills_code_review_agent/DESIGN_NOTE.md

@@ -2,7 +2,7 @@
 
 本方案将自动代码评审拆为“主流程编排 + 可复用 Skill + 受控执行 + 结构化落库”四层。主流程由 `agent/agent.py` 负责，统一接收 diff、repo path 或 fixture，完成输入归一化、diff 解析、规则执行、Filter 决策、skill 脚本调度、报告生成和 SQLite 持久化。`skills/code-review/` 则承载正式的 `code-review` Skill，包括 `SKILL.md`、规则文档、使用文档、脚本契约与三个确定性脚本，用于承接可复用的评审知识与脚本执行面。
 
-沙箱隔离策略采用“显式本地开发回退 + 容器/远端接口预留”的实现方式。当前示例只有 `local` 会实际执行脚本，`container`、`cube`、`e2b` 等 runtime 在未接入真实隔离执行器前，都会先被 Filter 标记为 `needs_human_review`，避免把宿主机执行误报成生产安全沙箱。统一脚本执行层仍然提供 timeout、输出截断、失败记录和 Filter 前置治理，确保高风险脚本、禁止路径、默认网络访问和超预算输入不能直接进入执行链路。对脚本失败或超时，系统不会整体崩溃，而是转换为可追踪的 `sandbox_runs` 记录和结构化 finding。
+沙箱隔离策略采用“统一 workspace runtime 执行 + 本地回退 + 容器优先扩展”的实现方式。当前示例中的 `local` 与 `container` 都通过同一套 workspace manager / filesystem / program runner 链路执行脚本，不再把 `container` 名义上的运行时回退到宿主 `subprocess`。其中 `local` 仍用于开发调试，`container` 在具备 Docker 环境时提供真实隔离；`cube`、`e2b` 等远端 runtime 仍保留后续接入点。统一脚本执行层继续提供 timeout、输出截断、失败记录和 Filter 前置治理，确保高风险脚本、禁止路径、默认网络访问和超预算输入不能直接进入执行链路。对脚本失败或超时，系统不会整体崩溃，而是转换为可追踪的 `sandbox_runs` 记录和结构化 finding。
 
 数据库 schema 采用最小可查询设计，包含 `review_tasks`、`review_inputs`、`filter_decisions`、`sandbox_runs`、`findings` 和 `review_reports` 六张表，支持按 `task_id` 查询完整审查链路。报告输出同时生成 JSON 与 Markdown，两者都包含 findings 摘要、人工复核项、Filter 摘要、sandbox 摘要和监控指标。监控字段聚合总耗时、severity/category 分布、拦截次数和 sandbox 次数，便于回放和评测。
 

examples/skills_code_review_agent/README.md

@@ -87,10 +87,10 @@ python examples/skills_code_review_agent/run_agent.py ^
   --fake-model
 ```
 
-`local` is the only runtime that currently executes scripts in this example,
-and it should be treated as a development fallback. Requests for `container`,
-`cube`, or `e2b` are recorded by Filter as `needs_human_review` until a real
-isolated executor is wired in.
+`local` and `container` now share the same workspace-runtime execution path.
+Use `local` for development fallback and `container` when Docker-backed
+isolation is available. Additional remote runtimes such as `cube` and `e2b`
+still require future wiring in this example.
 
 ## Outputs
 
@@ -171,7 +171,7 @@ Phase 6 quality-gate evidence:
 
 - full suite passes locally
 - all 8 public fixtures generate both report artifacts
-- a measured security fixture dry-run completes in about `9.87s`
+- a measured local dry-run remains within the acceptance time budget
 
 ## Related Docs
 

examples/skills_code_review_agent/agent/agent.py

@@ -92,7 +92,12 @@ def run_review_task(config: ReviewAgentConfig) -> tuple[ReviewTask, ReviewReport
     ):
         task.add_filter_decision(decision)
         if decision.decision.value == "allow":
-            sandbox_run = execute_skill_script(invocation, runtime=config.runtime)
+            sandbox_run = execute_skill_script(
+                invocation,
+                runtime=config.runtime,
+                diff_text=task.review_input.diff_text,
+                project_root=Path(__file__).resolve().parents[3],
+            )
             task.add_sandbox_run(sandbox_run)
             all_findings.extend(_sandbox_run_findings(task, sandbox_run))
             all_findings.extend(_sandbox_output_findings(task, sandbox_run))

examples/skills_code_review_agent/agent/tools.py

@@ -2,17 +2,21 @@
 
 from __future__ import annotations
 
+import asyncio
 import json
 import os
 import shlex
-import subprocess
 import sys
 from pathlib import Path
 from time import perf_counter
 from typing import Any
+from uuid import uuid4
 
 from trpc_agent_sdk.code_executors import BaseWorkspaceRuntime
 from trpc_agent_sdk.code_executors import DEFAULT_SKILLS_CONTAINER
+from trpc_agent_sdk.code_executors import WorkspacePutFileInfo
+from trpc_agent_sdk.code_executors import WorkspaceRunProgramSpec
+from trpc_agent_sdk.code_executors import WorkspaceStageOptions
 from trpc_agent_sdk.code_executors import create_container_workspace_runtime
 from trpc_agent_sdk.code_executors import create_local_workspace_runtime
 from trpc_agent_sdk.skills import BaseSkillRepository
@@ -26,7 +30,8 @@
 SKILL_NAME = "code-review"
 SCRIPT_TIMEOUT_SECONDS = 20
 OUTPUT_LIMIT_CHARS = 4000
-HOST_EXECUTION_RUNTIME = "local"
+WORKSPACE_SKILL_DIR = f"skills/{SKILL_NAME}"
+WORKSPACE_DIFF_PATH = "work/inputs/review.diff"
 
 
 def create_skill_tool_set(
@@ -58,37 +63,38 @@ def build_skill_script_plan(
     """Build the list of planned skill-script executions for the review."""
 
     scripts_dir = resolve_code_review_skill_dir(project_root=project_root) / "scripts"
+    _ = diff_file
     return [
         SkillScriptInvocation(
             name="parse_diff",
             script_path=scripts_dir / "parse_diff.py",
             command=[
-                sys.executable,
-                str(scripts_dir / "parse_diff.py"),
+                "python",
+                f"{WORKSPACE_SKILL_DIR}/scripts/parse_diff.py",
                 "--diff-file",
-                str(diff_file),
+                WORKSPACE_DIFF_PATH,
             ],
             target="skill:code-review/scripts/parse_diff.py",
         ),
         SkillScriptInvocation(
             name="run_linters",
             script_path=scripts_dir / "run_linters.py",
             command=[
-                sys.executable,
-                str(scripts_dir / "run_linters.py"),
+                "python",
+                f"{WORKSPACE_SKILL_DIR}/scripts/run_linters.py",
                 "--diff-file",
-                str(diff_file),
+                WORKSPACE_DIFF_PATH,
             ],
             target="skill:code-review/scripts/run_linters.py",
         ),
         SkillScriptInvocation(
             name="run_tests",
             script_path=scripts_dir / "run_tests.py",
             command=[
-                sys.executable,
-                str(scripts_dir / "run_tests.py"),
+                "python",
+                f"{WORKSPACE_SKILL_DIR}/scripts/run_tests.py",
                 "--diff-file",
-                str(diff_file),
+                WORKSPACE_DIFF_PATH,
             ],
             target="skill:code-review/scripts/run_tests.py",
         ),
@@ -119,25 +125,23 @@ def execute_skill_script(
     invocation: SkillScriptInvocation,
     *,
     runtime: str,
+    diff_text: str,
+    project_root: Path | None = None,
     timeout_seconds: int = SCRIPT_TIMEOUT_SECONDS,
     output_limit_chars: int = OUTPUT_LIMIT_CHARS,
 ) -> SandboxRunRecord:
-    """Execute a skill script in a controlled subprocess."""
-
-    if runtime != HOST_EXECUTION_RUNTIME:
-        raise RuntimeError(
-            f"Runtime `{runtime}` is not backed by a real isolated executor in this example."
-        )
+    """Execute a skill script through the configured workspace runtime."""
 
     started = perf_counter()
     try:
-        completed = subprocess.run(
-            invocation.command,
-            capture_output=True,
-            check=False,
-            text=True,
-            encoding="utf-8",
-            timeout=timeout_seconds,
+        completed = asyncio.run(
+            _run_skill_script_in_workspace(
+                invocation,
+                runtime=runtime,
+                diff_text=diff_text,
+                project_root=project_root,
+                timeout_seconds=timeout_seconds,
+            )
         )
         stdout, stdout_truncated = _truncate_output(
             _normalize_process_output(completed.stdout),
@@ -147,9 +151,9 @@ def execute_skill_script(
             _normalize_process_output(completed.stderr),
             output_limit_chars,
         )
-        status = (
+        status = SandboxRunStatus.TIMED_OUT if completed.timed_out else (
             SandboxRunStatus.SUCCEEDED
-            if completed.returncode == 0
+            if completed.exit_code == 0
             else SandboxRunStatus.FAILED
         )
         return SandboxRunRecord(
@@ -158,33 +162,26 @@ def execute_skill_script(
             status=status,
             runtime=runtime,
             duration_ms=int((perf_counter() - started) * 1000),
-            exit_code=completed.returncode,
+            exit_code=completed.exit_code,
             stdout=_sanitize_output_text(stdout),
             stderr=_sanitize_output_text(stderr),
-            timed_out=False,
+            timed_out=completed.timed_out,
             output_truncated=stdout_truncated or stderr_truncated,
             blocked_by_filter=False,
         )
-    except subprocess.TimeoutExpired as exc:
-        stdout, stdout_truncated = _truncate_output(
-            _normalize_process_output(exc.stdout),
-            output_limit_chars,
-        )
-        stderr, stderr_truncated = _truncate_output(
-            _normalize_process_output(exc.stderr),
-            output_limit_chars,
-        )
+    except Exception as exc:  # pylint: disable=broad-except
+        stderr, stderr_truncated = _truncate_output(str(exc), output_limit_chars)
         return SandboxRunRecord(
             name=invocation.name,
             command=[_sanitize_display_value(part) for part in invocation.command],
-            status=SandboxRunStatus.TIMED_OUT,
+            status=SandboxRunStatus.FAILED,
             runtime=runtime,
             duration_ms=int((perf_counter() - started) * 1000),
             exit_code=None,
-            stdout=_sanitize_output_text(stdout),
+            stdout="",
             stderr=_sanitize_output_text(stderr),
-            timed_out=True,
-            output_truncated=stdout_truncated or stderr_truncated,
+            timed_out=False,
+            output_truncated=stderr_truncated,
             blocked_by_filter=False,
         )
 
@@ -232,6 +229,53 @@ def _normalize_process_output(text: object) -> str:
     return str(text)
 
 
+async def _run_skill_script_in_workspace(
+    invocation: SkillScriptInvocation,
+    *,
+    runtime: str,
+    diff_text: str,
+    project_root: Path | None,
+    timeout_seconds: int,
+) -> Any:
+    """Stage skill inputs into a workspace and execute the script via runtime APIs."""
+
+    workspace_runtime = _create_workspace_runtime(workspace_runtime_type=runtime)
+    manager = workspace_runtime.manager()
+    fs = workspace_runtime.fs()
+    runner = workspace_runtime.runner()
+    exec_id = f"code_review_{invocation.name}_{uuid4().hex}"
+    workspace = await manager.create_workspace(exec_id)
+
+    try:
+        skill_dir = resolve_code_review_skill_dir(project_root=project_root)
+        await fs.stage_directory(
+            workspace,
+            str(skill_dir),
+            WORKSPACE_SKILL_DIR,
+            WorkspaceStageOptions(read_only=True, allow_mount=True, mode="copy"),
+        )
+        await fs.put_files(
+            workspace,
+            [
+                WorkspacePutFileInfo(
+                    path=WORKSPACE_DIFF_PATH,
+                    content=diff_text.encode("utf-8"),
+                    mode=0o644,
+                )
+            ],
+        )
+        return await runner.run_program(
+            workspace,
+            WorkspaceRunProgramSpec(
+                cmd=invocation.command[0],
+                args=invocation.command[1:],
+                timeout=timeout_seconds,
+            ),
+        )
+    finally:
+        await manager.cleanup(exec_id)
+
+
 def _sanitize_output_text(text: str) -> str:
     """Normalize sandbox output so reports stay portable across machines."""
 

examples/skills_code_review_agent/sample_outputs/review_report.json

@@ -1,5 +1,5 @@
 {
-  "task_id": "9118ea7d-760d-4cf8-a383-7d14f298197e",
+  "task_id": "f684c95b-c5a8-41cb-a55e-1eeec9233837",
   "conclusion": "fail",
   "findings": [
     {
@@ -61,13 +61,13 @@
         "python",
         "skills/code-review/scripts/parse_diff.py",
         "--diff-file",
-        "examples/skills_code_review_agent/sample_outputs/skill_inputs/9118ea7d-760d-4cf8-a383-7d14f298197e.diff"
+        "work/inputs/review.diff"
       ],
       "status": "succeeded",
       "runtime": "local",
-      "duration_ms": 188,
+      "duration_ms": 3404,
       "exit_code": 0,
-      "stdout": "{\n  \"diff_file\": \"examples/skills_code_review_agent/sample_outputs/skill_inputs/9118ea7d-760d-4cf8-a383-7d14f298197e.diff\",\n  \"line_count\": 22,\n  \"file_count\": 2,\n  \"has_security_keywords\": true\n}\n",
+      "stdout": "{\n  \"diff_file\": \"review.diff\",\n  \"line_count\": 22,\n  \"file_count\": 2,\n  \"has_security_keywords\": true\n}\n",
       "stderr": "",
       "timed_out": false,
       "output_truncated": false,
@@ -79,13 +79,13 @@
         "python",
         "skills/code-review/scripts/run_linters.py",
         "--diff-file",
-        "examples/skills_code_review_agent/sample_outputs/skill_inputs/9118ea7d-760d-4cf8-a383-7d14f298197e.diff"
+        "work/inputs/review.diff"
       ],
       "status": "succeeded",
       "runtime": "local",
-      "duration_ms": 182,
+      "duration_ms": 3139,
       "exit_code": 0,
-      "stdout": "{\n  \"diff_file\": \"examples/skills_code_review_agent/sample_outputs/skill_inputs/9118ea7d-760d-4cf8-a383-7d14f298197e.diff\",\n  \"warning_count\": 2,\n  \"warnings\": [\n    \"Security-sensitive call detected: eval\",\n    \"Shell execution enabled in subprocess call\"\n  ]\n}\n",
+      "stdout": "{\n  \"diff_file\": \"review.diff\",\n  \"warning_count\": 2,\n  \"warnings\": [\n    \"Security-sensitive call detected: eval\",\n    \"Shell execution enabled in subprocess call\"\n  ]\n}\n",
       "stderr": "",
       "timed_out": false,
       "output_truncated": false,
@@ -97,13 +97,13 @@
         "python",
         "skills/code-review/scripts/run_tests.py",
         "--diff-file",
-        "examples/skills_code_review_agent/sample_outputs/skill_inputs/9118ea7d-760d-4cf8-a383-7d14f298197e.diff"
+        "work/inputs/review.diff"
       ],
       "status": "succeeded",
       "runtime": "local",
-      "duration_ms": 218,
+      "duration_ms": 4462,
       "exit_code": 0,
-      "stdout": "{\n  \"diff_file\": \"examples/skills_code_review_agent/sample_outputs/skill_inputs/9118ea7d-760d-4cf8-a383-7d14f298197e.diff\",\n  \"changed_test_files\": [\n    \"tests/test_calculator.py\"\n  ],\n  \"test_update_present\": true\n}\n",
+      "stdout": "{\n  \"diff_file\": \"review.diff\",\n  \"changed_test_files\": [\n    \"tests/test_calculator.py\"\n  ],\n  \"test_update_present\": true\n}\n",
       "stderr": "",
       "timed_out": false,
       "output_truncated": false,
@@ -115,7 +115,7 @@
     "high": 2
   },
   "monitoring_summary": {
-    "total_duration_ms": 604,
+    "total_duration_ms": 11032,
     "changed_files_count": 2,
     "added_lines_count": 6,
     "deleted_lines_count": 1,

examples/skills_code_review_agent/sample_outputs/review_report.md

@@ -1,6 +1,6 @@
 # Review Report
 
-- Task ID: `9118ea7d-760d-4cf8-a383-7d14f298197e`
+- Task ID: `f684c95b-c5a8-41cb-a55e-1eeec9233837`
 - Final Verdict: `fail`
 
 ## Summary
@@ -36,9 +36,9 @@ Loaded review input, parsed diff, completed deterministic rule review, and proce
 
 ## Sandbox Summary
 
-- `parse_diff` status=`succeeded` duration=188ms exit_code=0
-- `run_linters` status=`succeeded` duration=182ms exit_code=0
-- `run_tests` status=`succeeded` duration=218ms exit_code=0
+- `parse_diff` status=`succeeded` duration=3404ms exit_code=0
+- `run_linters` status=`succeeded` duration=3139ms exit_code=0
+- `run_tests` status=`succeeded` duration=4462ms exit_code=0
 
 ## Monitoring
 
@@ -52,7 +52,7 @@ Loaded review input, parsed diff, completed deterministic rule review, and proce
 - `needs_human_review_count`: 0
 - `sandbox_run_count`: 3
 - `severity_distribution`: {'high': 2}
-- `total_duration_ms`: 604
+- `total_duration_ms`: 11032
 - `warning_count`: 0
 
 ## Actionable Recommendations

examples/skills_code_review_agent/src/filter_policy.py

@@ -10,7 +10,6 @@
 _FORBIDDEN_PATH_PARTS = (".git/", ".env", "secrets/", "id_rsa", ".pem")
 _NETWORK_TOKENS = ("http://", "https://", "curl", "wget", "Invoke-WebRequest", "requests.get(")
 _DANGEROUS_TOKENS = ("rm -rf", "del /f", "format ", "shutdown ", "mkfs")
-_HOST_EXECUTION_RUNTIME = "local"
 
 
 @dataclass(slots=True, frozen=True)
@@ -76,19 +75,6 @@ def evaluate_invocations(
                 reason="Diff size exceeds sandbox budget and requires manual approval.",
                 requires_human_review=True,
             )
-        elif runtime != _HOST_EXECUTION_RUNTIME:
-            decision = FilterDecisionRecord(
-                decision=FilterDecisionType.NEEDS_HUMAN_REVIEW,
-                target=invocation.target,
-                reason_code="runtime_not_isolated",
-                reason=(
-                    f"Runtime `{runtime}` is requested, but this example does not yet "
-                    "back it with a real isolated executor. Block host execution until "
-                    "container or remote sandbox support is implemented."
-                ),
-                requires_human_review=True,
-            )
-
         paired.append((invocation, decision))
     return paired