Skip to content

Commit 7a9204d

Browse files
committed
examples: add sandbox execution (local + container) to code-review agent
Slice 2 of the code-review agent. Scanners now run outside the review process via pipeline/sandbox.py: --runtime local runs the skill's run_checks.py in a subprocess with a timeout and an output-size cap, recording a SandboxRunResult for every run. A timeout or failure marks the run and completes the review instead of crashing it (requirement 4). --runtime container runs the scanners inside a Docker workspace via the framework's Container runtime (skills/code-review/Dockerfile); its test skips cleanly when Docker is absent. Sandbox runs are now persisted (requirement 3) and surfaced in the report's sandbox-execution section (requirement 8); monitoring records sandbox time. Adds tests for the local sandbox path, timeout resilience, and output-byte accounting. Updates #92 RELEASE NOTES: NONE
1 parent 821f703 commit 7a9204d

6 files changed

Lines changed: 276 additions & 17 deletions

File tree

examples/skills_code_review_agent/README.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,11 @@ a rendered report — criterion 5 is binary-checked, so redaction is centralized
7777

7878
## Status
7979

80-
Implemented: deterministic pipeline, DB persistence, 8 fixtures, scored self-test, CLI, and the
81-
fake-model agent loop (`run_agent.py` drives the review through `LlmAgent` + a `FunctionTool` with no
82-
API key). Baseline secret redaction is in place. Planned follow-up slices: in-sandbox execution
83-
(Container/Cube), the tool-level Filter gate, redaction hardening to ≥95%, and OpenTelemetry metrics.
80+
Implemented: deterministic pipeline, DB persistence (incl. sandbox-run rows), 8 fixtures, scored
81+
self-test, CLI, the fake-model agent loop (`run_agent.py`), and **sandbox execution**
82+
(`--runtime local` runs the scanners in a subprocess sandbox with timeout + output cap and records
83+
each run; `--runtime container` runs them in a Docker workspace — see `skills/code-review/Dockerfile`
84+
— and is skipped in tests when Docker is absent). Baseline secret redaction is in place.
85+
86+
Planned follow-up slices: the tool-level Filter gate (criterion 7), redaction hardening to ≥95%
87+
(criterion 5), and OpenTelemetry metrics wiring (criterion 9).

examples/skills_code_review_agent/pipeline/engine.py

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
# Copyright (C) 2026 Tencent. All rights reserved.
44
#
55
# tRPC-Agent-Python is licensed under Apache-2.0.
6-
76
"""End-to-end deterministic review pipeline (issue #92 backbone).
87
98
parse -> materialize changed files -> run scanners -> dedup/denoise -> build+redact report ->
@@ -58,11 +57,18 @@ def run_review(
5857
task_id: Optional[str] = None,
5958
diff_text: Optional[str] = None,
6059
repo_path: Optional[str] = None,
60+
runtime: str = "inprocess",
61+
sandbox_timeout: float | None = None,
62+
max_output_bytes: int | None = None,
6163
warn_threshold: float | None = None,
6264
review_threshold: float | None = None,
6365
) -> ReviewResult:
6466
"""Run one review (deterministic, no LLM). Provide either ``diff_text`` or ``repo_path``.
6567
68+
``runtime``: ``inprocess`` (default, fast) runs scanners in-process; ``local`` runs them in a
69+
subprocess sandbox with timeout + output cap (dev fallback) and records a sandbox run. The
70+
``container`` runtime (production isolation) is async — see ``run_review_container``.
71+
6672
Returns a ``ReviewResult``; persistence is done separately by the async ``storage.dao.ReviewStore``
6773
so this core stays synchronous and dependency-light.
6874
"""
@@ -80,12 +86,30 @@ def run_review(
8086
else:
8187
raise ValueError("run_review requires diff_text or repo_path")
8288

83-
try:
84-
raw = scanners.scan(scan_dir, summary)
85-
except Exception as exc: # noqa: BLE001 - boundary; never crash the task
86-
exception_dist[type(exc).__name__] = exception_dist.get(type(exc).__name__, 0) + 1
87-
raw = []
88-
89+
sandbox_runs: list = []
90+
if runtime == "local":
91+
from . import sandbox as sandbox_mod
92+
raw, run = sandbox_mod.run_local(
93+
scan_dir,
94+
timeout=sandbox_timeout if sandbox_timeout is not None else sandbox_mod.DEFAULT_TIMEOUT_SEC,
95+
max_bytes=max_output_bytes if max_output_bytes is not None else sandbox_mod.MAX_OUTPUT_BYTES)
96+
sandbox_runs = [run]
97+
if run.timed_out or run.exit_code not in (0, 1): # 1 = scanners found issues (normal)
98+
exception_dist["sandbox_failure"] = exception_dist.get("sandbox_failure", 0) + 1
99+
else: # "inprocess"
100+
try:
101+
raw = scanners.scan(scan_dir, summary)
102+
except Exception as exc: # noqa: BLE001 - boundary; never crash the task
103+
exception_dist[type(exc).__name__] = exception_dist.get(type(exc).__name__, 0) + 1
104+
raw = []
105+
106+
return _assemble(task_id, summary, raw, sandbox_runs, source_type, source_ref, started, exception_dist,
107+
warn_threshold, review_threshold)
108+
109+
110+
def _assemble(task_id, summary, raw, sandbox_runs, source_type, source_ref, started, exception_dist, warn_threshold,
111+
review_threshold) -> ReviewResult:
112+
"""Shared tail: dedup/denoise -> monitoring -> build+redact report -> ReviewResult."""
89113
findings = dedup_and_denoise(
90114
raw,
91115
warn_threshold if warn_threshold is not None else dedup_thresholds()[0],
@@ -102,15 +126,15 @@ def run_review(
102126

103127
monitoring = {
104128
"total_sec": round(time.monotonic() - started, 3),
105-
"sandbox_sec": 0.0, # populated in slice 2 (real sandbox)
129+
"sandbox_sec": round(sum(r.duration_sec for r in sandbox_runs), 3),
106130
"tool_calls": len(scanners.ADAPTERS),
107131
"block_count": 0, # populated in slice 3 (Filter gate)
108132
"finding_count": len(active),
109133
"severity_dist": severity_dist,
110134
"exception_dist": exception_dist,
111135
}
112136

113-
report = report_mod.build_report(task_id, findings, monitoring=monitoring)
137+
report = report_mod.build_report(task_id, findings, sandbox_runs=sandbox_runs, monitoring=monitoring)
114138
return ReviewResult(task_id=task_id,
115139
report=report,
116140
findings=findings,
@@ -120,6 +144,28 @@ def run_review(
120144
monitoring=monitoring)
121145

122146

147+
async def run_review_container(
148+
*,
149+
task_id: Optional[str] = None,
150+
diff_text: str,
151+
sandbox_timeout: float | None = None,
152+
max_output_bytes: int | None = None,
153+
) -> ReviewResult:
154+
"""Run a review with scanners inside a Container workspace (production isolation; needs Docker)."""
155+
from . import sandbox as sandbox_mod
156+
task_id = task_id or f"cr-{uuid.uuid4().hex[:12]}"
157+
started = time.monotonic()
158+
summary, scan_dir = _materialize(diff_text)
159+
raw, run = await sandbox_mod.run_container(
160+
scan_dir,
161+
timeout=sandbox_timeout if sandbox_timeout is not None else sandbox_mod.DEFAULT_TIMEOUT_SEC,
162+
max_bytes=max_output_bytes if max_output_bytes is not None else sandbox_mod.MAX_OUTPUT_BYTES)
163+
exception_dist: dict[str, int] = {}
164+
if run.timed_out or run.exit_code not in (0, 1):
165+
exception_dist["sandbox_failure"] = 1
166+
return _assemble(task_id, summary, raw, [run], "diff_file", "<diff>", started, exception_dist, None, None)
167+
168+
123169
def dedup_thresholds() -> tuple[float, float]:
124170
from .dedup import REVIEW_THRESHOLD, WARN_THRESHOLD
125171
return WARN_THRESHOLD, REVIEW_THRESHOLD
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
# Tencent is pleased to support the open source community by making tRPC-Agent-Python available.
2+
#
3+
# Copyright (C) 2026 Tencent. All rights reserved.
4+
#
5+
# tRPC-Agent-Python is licensed under Apache-2.0.
6+
7+
"""Sandboxed execution of the review scanners (issue #92, requirement 4 & slice 2).
8+
9+
Runs the skill's ``run_checks.py`` outside the review process, with a timeout and an output-size cap,
10+
and records a ``SandboxRunResult`` for every run — including timeouts and failures — so one bad run
11+
degrades a source without crashing the task.
12+
13+
Two runtimes:
14+
- ``run_local``: subprocess execution (the dev fallback the issue permits) — real process boundary,
15+
timeout and output cap; verified without Docker.
16+
- ``run_container``: production isolation via the framework's Container workspace runtime; requires
17+
Docker and the scanner image (see ``skills/code-review/Dockerfile``).
18+
"""
19+
from __future__ import annotations
20+
21+
import json
22+
import subprocess
23+
import sys
24+
import tempfile
25+
import time
26+
from pathlib import Path
27+
28+
from .types import Finding, SandboxRunResult
29+
30+
_SKILL_SCRIPT = Path(__file__).resolve().parents[3] / "skills" / "code-review" / "scripts" / "run_checks.py"
31+
DEFAULT_TIMEOUT_SEC = 60.0
32+
MAX_OUTPUT_BYTES = 1_048_576 # 1 MiB per stream
33+
34+
35+
def _truncate(text: str | bytes | None, cap: int) -> tuple[str, int]:
36+
"""Return (possibly-truncated text, original byte length). The cap bounds what we persist."""
37+
if text is None:
38+
return "", 0
39+
raw = text.encode("utf-8", "replace") if isinstance(text, str) else text
40+
n = len(raw)
41+
if n <= cap:
42+
return raw.decode("utf-8", "ignore"), n
43+
return raw[:cap].decode("utf-8", "ignore") + "\n...[truncated]", n
44+
45+
46+
def parse_findings_json(payload: dict) -> list[Finding]:
47+
"""Map the skill's findings.json (docs/OUTPUT_SCHEMA.md) into Finding objects."""
48+
out: list[Finding] = []
49+
for f in payload.get("findings", []):
50+
try:
51+
out.append(
52+
Finding(severity=f.get("severity", "low"),
53+
category=f.get("category", "unknown"),
54+
file=f.get("file", ""),
55+
line=f.get("line"),
56+
title=f.get("title", ""),
57+
evidence=f.get("evidence", ""),
58+
recommendation=f.get("recommendation", ""),
59+
confidence=float(f.get("confidence", 0.5)),
60+
source=f.get("source", "static"),
61+
rule_id=f.get("rule_id")))
62+
except Exception: # noqa: BLE001 - a malformed row is skipped, not fatal
63+
continue
64+
return out
65+
66+
67+
def run_local(
68+
scan_dir: str,
69+
*,
70+
timeout: float = DEFAULT_TIMEOUT_SEC,
71+
max_bytes: int = MAX_OUTPUT_BYTES,
72+
) -> tuple[list[Finding], SandboxRunResult]:
73+
"""Run the scanners in a subprocess against ``scan_dir``; never raises."""
74+
out_file = Path(tempfile.mkdtemp(prefix="cr_out_")) / "findings.json"
75+
cmd = [sys.executable, str(_SKILL_SCRIPT), "--target", scan_dir, "--out", str(out_file)]
76+
77+
started = time.monotonic()
78+
timed_out = False
79+
exit_code = 0
80+
stdout: str | bytes | None = ""
81+
stderr: str | bytes | None = ""
82+
try:
83+
proc = subprocess.run(cmd, capture_output=True, text=True, timeout=timeout, check=False)
84+
exit_code, stdout, stderr = proc.returncode, proc.stdout, proc.stderr
85+
except subprocess.TimeoutExpired as exc:
86+
timed_out, exit_code = True, -1
87+
stdout, stderr = exc.stdout, exc.stderr
88+
duration = time.monotonic() - started
89+
90+
findings: list[Finding] = []
91+
if not timed_out and out_file.exists():
92+
try:
93+
findings = parse_findings_json(json.loads(out_file.read_text(encoding="utf-8")))
94+
except Exception: # noqa: BLE001 - unreadable output degrades the source, not the task
95+
findings = []
96+
97+
_, out_bytes = _truncate(stdout, max_bytes)
98+
_, err_bytes = _truncate(stderr, max_bytes)
99+
result = SandboxRunResult(script="run_checks.py",
100+
exit_code=exit_code,
101+
duration_sec=round(duration, 3),
102+
timed_out=timed_out,
103+
stdout_bytes=out_bytes,
104+
stderr_bytes=err_bytes)
105+
return findings, result
106+
107+
108+
async def run_container(
109+
scan_dir: str,
110+
*,
111+
timeout: float = DEFAULT_TIMEOUT_SEC,
112+
max_bytes: int = MAX_OUTPUT_BYTES,
113+
memory_mb: int = 512,
114+
image: str = "cr-scanners:latest",
115+
) -> tuple[list[Finding], SandboxRunResult]:
116+
"""Production isolation: run the scanners inside a container workspace. Requires Docker + image.
117+
118+
Stages the changed files into the workspace, runs ``run_checks.py`` with a timeout and resource
119+
limits, collects ``findings.json``, and truncates captured output to the byte cap.
120+
"""
121+
from trpc_agent_sdk.code_executors import (WorkspaceOutputSpec, WorkspacePutFileInfo, WorkspaceResourceLimits,
122+
WorkspaceRunProgramSpec, create_container_workspace_runtime)
123+
from trpc_agent_sdk.code_executors.container import ContainerConfig
124+
125+
runtime = create_container_workspace_runtime(container_config=ContainerConfig(image=image))
126+
manager, fs, runner = runtime.manager(None), runtime.fs(None), runtime.runner(None)
127+
exec_id = "cr-" + Path(scan_dir).name
128+
129+
started = time.monotonic()
130+
ws = await manager.create_workspace(exec_id)
131+
try:
132+
files = [
133+
WorkspacePutFileInfo(path=str(p.relative_to(scan_dir)), content=p.read_bytes())
134+
for p in Path(scan_dir).rglob("*") if p.is_file()
135+
]
136+
await fs.put_files(ws, files)
137+
run = await runner.run_program(
138+
ws,
139+
WorkspaceRunProgramSpec(cmd="python",
140+
args=["/opt/skill/run_checks.py", "--target", ".", "--out", "findings.json"],
141+
timeout=timeout,
142+
limits=WorkspaceResourceLimits(memory_mb=memory_mb)))
143+
collected = await fs.collect_outputs(ws, WorkspaceOutputSpec(globs=["findings.json"]))
144+
findings: list[Finding] = []
145+
for cf in getattr(collected, "files", collected) or []:
146+
try:
147+
findings = parse_findings_json(json.loads(cf.content.decode("utf-8")))
148+
except Exception: # noqa: BLE001
149+
continue
150+
_, out_bytes = _truncate(run.stdout, max_bytes)
151+
_, err_bytes = _truncate(run.stderr, max_bytes)
152+
result = SandboxRunResult(script="run_checks.py",
153+
exit_code=run.exit_code,
154+
duration_sec=round(time.monotonic() - started, 3),
155+
timed_out=run.timed_out,
156+
stdout_bytes=out_bytes,
157+
stderr_bytes=err_bytes)
158+
return findings, result
159+
finally:
160+
await manager.cleanup(exec_id)

examples/skills_code_review_agent/run_review.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
# Copyright (C) 2026 Tencent. All rights reserved.
66
#
77
# tRPC-Agent-Python is licensed under Apache-2.0.
8-
98
"""CLI entry point for the automated code-review agent (issue #92).
109
1110
Dry-run / fake-model mode (default) needs no API key: the deterministic scanner pipeline produces a
@@ -22,7 +21,7 @@
2221
from pathlib import Path
2322

2423
from pipeline import report as report_mod
25-
from pipeline.engine import ReviewResult, run_review
24+
from pipeline.engine import ReviewResult, run_review, run_review_container
2625

2726
HERE = Path(__file__).parent
2827

@@ -33,6 +32,11 @@ def _parse_args() -> argparse.Namespace:
3332
src.add_argument("--diff-file", help="path to a unified-diff file")
3433
src.add_argument("--repo-path", help="path to a git worktree (reviews `git diff`)")
3534
src.add_argument("--fixture", help="name of a bundled fixture under fixtures/diffs/")
35+
ap.add_argument("--runtime",
36+
choices=["inprocess", "local", "container"],
37+
default="inprocess",
38+
help="scanner runtime: inprocess (fast), local (subprocess sandbox), container (Docker)")
39+
ap.add_argument("--sandbox-timeout", type=float, default=None, help="sandbox timeout in seconds")
3640
ap.add_argument("--out-dir", default=".", help="where to write review_report.json/.md")
3741
ap.add_argument("--db-url", default="sqlite+aiosqlite:///./code_review.db")
3842
ap.add_argument("--no-db", action="store_true", help="skip persistence (report files only)")
@@ -41,9 +45,12 @@ def _parse_args() -> argparse.Namespace:
4145

4246
def _run(args: argparse.Namespace) -> ReviewResult:
4347
if args.repo_path:
44-
return run_review(repo_path=args.repo_path)
48+
return run_review(repo_path=args.repo_path, runtime=args.runtime, sandbox_timeout=args.sandbox_timeout)
4549
path = Path(args.diff_file) if args.diff_file else HERE / "fixtures" / "diffs" / args.fixture
46-
return run_review(diff_text=path.read_text(encoding="utf-8"))
50+
diff_text = path.read_text(encoding="utf-8")
51+
if args.runtime == "container":
52+
return asyncio.run(run_review_container(diff_text=diff_text, sandbox_timeout=args.sandbox_timeout))
53+
return run_review(diff_text=diff_text, runtime=args.runtime, sandbox_timeout=args.sandbox_timeout)
4754

4855

4956
async def _persist(result: ReviewResult, db_url: str) -> None:

skills/code-review/Dockerfile

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# Scanner image for the code-review sandbox (production runtime).
2+
# Build: docker build -t cr-scanners:latest skills/code-review
3+
# Used by pipeline/sandbox.py::run_container (engine --runtime container).
4+
FROM python:3.12-slim
5+
6+
RUN pip install --no-cache-dir bandit ruff "detect-secrets>=1.5" semgrep
7+
8+
# The standalone scanner entry point (self-contained; no example package import).
9+
COPY scripts/run_checks.py /opt/skill/run_checks.py
10+
COPY rules /opt/skill/rules
11+
12+
WORKDIR /workspace

tests/examples/test_skills_code_review_agent.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,3 +138,33 @@ async def test_agent_path_calls_tool_and_summarizes() -> None:
138138
assert "Review complete" in final_text
139139
for secret in _SECRETS:
140140
assert secret not in final_text
141+
142+
143+
def test_local_sandbox_records_run_and_finds_issues() -> None:
144+
result = run_review(diff_text=(_FIXTURES / "0001_insecure.diff").read_text(), runtime="local")
145+
assert result.report.findings_summary["total"] >= 3
146+
assert len(result.report.sandbox_summary) == 1
147+
run = result.report.sandbox_summary[0]
148+
assert run.script == "run_checks.py"
149+
assert run.exit_code in (0, 1) # 1 = scanners found issues
150+
assert not run.timed_out
151+
assert result.monitoring["sandbox_sec"] > 0
152+
153+
154+
def test_sandbox_timeout_does_not_crash_the_task() -> None:
155+
# An impossibly small timeout must mark the run timed-out but still complete the review.
156+
result = run_review(diff_text=(_FIXTURES / "0001_insecure.diff").read_text(),
157+
runtime="local",
158+
sandbox_timeout=0.001)
159+
assert result.task_id is not None
160+
run = result.report.sandbox_summary[0]
161+
assert run.timed_out is True
162+
assert result.monitoring["exception_dist"].get("sandbox_failure") == 1
163+
164+
165+
def test_sandbox_output_byte_accounting() -> None:
166+
from pipeline.sandbox import _truncate
167+
168+
text, n = _truncate("x" * 5000, 10)
169+
assert n == 5000 # records the true size
170+
assert len(text.encode()) <= 10 + len("\n...[truncated]")

0 commit comments

Comments
 (0)