Skip to content

Commit 7cd7811

Browse files
committed
examples: complete spec requirements (runtime default, env, file-list, diff-summary)
Default runtime is now auto: the CLI uses the container sandbox when Docker is available, else the local subprocess sandbox — in-process is an explicit --runtime inprocess dev opt-in (requirement 2: local is a fallback, not the default production path). Sandbox env whitelist: only ENV_ALLOWLIST vars reach the sandbox, so parent-process secrets never leak in (requirement 7). File-path-list input: --files a.py,b.py / run_review(files=[...]) via pipeline.diff_parser.parse_file_list (requirement 3). Persist the input-diff summary on the review task (requirement 5's fifth saved entity), queryable by task id. Updates #92 RELEASE NOTES: NONE
1 parent 1dea63f commit 7cd7811

8 files changed

Lines changed: 132 additions & 12 deletions

File tree

examples/skills_code_review_agent/pipeline/diff_parser.py

Lines changed: 22 additions & 1 deletion
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
"""Parse review inputs into a ``DiffSummary`` (issue #92, requirement 3).
87
98
Plumbing only — wraps the mature ``unidiff`` parser. Three input kinds:
@@ -91,6 +90,28 @@ def parse_git_worktree(repo_path: str, base_ref: str | None = None) -> DiffSumma
9190
return parse_unified_diff(proc.stdout)
9291

9392

93+
def parse_file_list(paths: list[str], repo_root: str = ".") -> DiffSummary:
94+
"""Treat a list of file paths as fully-added files (issue #92, requirement 3 input mode)."""
95+
files: list[ChangedFile] = []
96+
languages: dict[str, int] = {}
97+
added = 0
98+
for rel in paths:
99+
try:
100+
n = len((Path(repo_root) / rel).read_text(encoding="utf-8", errors="replace").splitlines())
101+
except OSError:
102+
n = 0
103+
lang = _language(rel)
104+
if lang:
105+
languages[lang] = languages.get(lang, 0) + 1
106+
added += n
107+
files.append(
108+
ChangedFile(path=rel,
109+
change_type="added",
110+
language=lang,
111+
hunks=[Hunk(new_start=1, new_len=n, candidate_lines=list(range(1, n + 1)))]))
112+
return DiffSummary(files=files, files_changed=len(files), added=added, removed=0, languages=languages)
113+
114+
94115
def materialize_new_files(text: str) -> dict[str, str]:
95116
"""Reconstruct the post-change (target-side) content of each changed file from a diff.
96117

examples/skills_code_review_agent/pipeline/engine.py

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,26 @@ def _materialize(diff_text: str) -> tuple[DiffSummary, str]:
5353
return summary, tmp
5454

5555

56+
def _materialize_files(paths: list[str], repo_root: str) -> str:
57+
"""Copy a list of files into a temp dir for scanning (the --files / file-list input mode)."""
58+
tmp = tempfile.mkdtemp(prefix="cr_scan_")
59+
for rel in paths:
60+
dest = Path(tmp) / rel
61+
dest.parent.mkdir(parents=True, exist_ok=True)
62+
try:
63+
dest.write_text((Path(repo_root) / rel).read_text(encoding="utf-8", errors="replace"), encoding="utf-8")
64+
except OSError:
65+
continue
66+
return tmp
67+
68+
5669
def run_review(
5770
*,
5871
task_id: Optional[str] = None,
5972
diff_text: Optional[str] = None,
6073
repo_path: Optional[str] = None,
74+
files: Optional[list[str]] = None,
75+
repo_root: str = ".",
6176
runtime: str = "inprocess",
6277
sandbox_timeout: float | None = None,
6378
max_output_bytes: int | None = None,
@@ -81,12 +96,16 @@ def run_review(
8196
if diff_text is not None:
8297
summary, scan_dir = _materialize(diff_text)
8398
source_type, source_ref = "diff_file", "<diff>"
99+
elif files is not None:
100+
summary = diff_parser.parse_file_list(files, repo_root)
101+
scan_dir = _materialize_files(files, repo_root)
102+
source_type, source_ref = "file_list", ",".join(files)[:200]
84103
elif repo_path is not None:
85104
summary = diff_parser.parse_git_worktree(repo_path)
86105
scan_dir = repo_path
87106
source_type, source_ref = "repo_path", repo_path
88107
else:
89-
raise ValueError("run_review requires diff_text or repo_path")
108+
raise ValueError("run_review requires diff_text, files, or repo_path")
90109

91110
sandbox_runs: list = []
92111
if runtime == "local":

examples/skills_code_review_agent/pipeline/policy.py

Lines changed: 8 additions & 1 deletion
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
"""Review policy — the Filter decision logic (issue #92, requirement 7 & 8).
87
98
Shared by two enforcement sites (plan decision #5): the framework ``ReviewGuardFilter`` on the agent
@@ -33,6 +32,14 @@
3332
# Sensitive roots a review must never touch (temp dirs under /var/folders are intentionally allowed).
3433
_FORBIDDEN_PATHS = ("/etc", "/root", "/boot", os.path.expanduser("~/.ssh"))
3534

35+
# Only these env vars are passed into the sandbox — parent-process secrets never leak in (要求7).
36+
ENV_ALLOWLIST = ("PATH", "HOME", "LANG", "LC_ALL", "TMPDIR", "TEMP", "TMP", "SYSTEMROOT")
37+
38+
39+
def sandbox_env(allowlist: Iterable[str] = ENV_ALLOWLIST) -> dict[str, str]:
40+
"""Return the minimal whitelisted environment for a sandbox run."""
41+
return {k: os.environ[k] for k in allowlist if k in os.environ}
42+
3643

3744
@dataclass
3845
class PolicyDecision:

examples/skills_code_review_agent/pipeline/sandbox.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,8 @@ def run_local(
110110
stdout: str | bytes | None = ""
111111
stderr: str | bytes | None = ""
112112
try:
113-
proc = subprocess.run(cmd, capture_output=True, text=True, timeout=timeout, check=False)
113+
from .policy import sandbox_env
114+
proc = subprocess.run(cmd, capture_output=True, text=True, timeout=timeout, check=False, env=sandbox_env())
114115
exit_code, stdout, stderr = proc.returncode, proc.stdout, proc.stderr
115116
except subprocess.TimeoutExpired as exc:
116117
timed_out, exit_code = True, -1
@@ -164,10 +165,12 @@ async def run_container(
164165
for p in Path(scan_dir).rglob("*") if p.is_file()
165166
]
166167
await fs.put_files(ws, files)
168+
from .policy import sandbox_env
167169
run = await runner.run_program(
168170
ws,
169171
WorkspaceRunProgramSpec(cmd="python",
170172
args=["/opt/skill/run_checks.py", "--target", ".", "--out", "findings.json"],
173+
env=sandbox_env(),
171174
timeout=timeout,
172175
limits=WorkspaceResourceLimits(memory_mb=memory_mb)))
173176
collected = await fs.collect_outputs(ws, WorkspaceOutputSpec(globs=["findings.json"]))

examples/skills_code_review_agent/run_review.py

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818

1919
import argparse
2020
import asyncio
21+
import shutil
22+
import subprocess
2123
from pathlib import Path
2224

2325
from pipeline import report as report_mod
@@ -26,16 +28,34 @@
2628
HERE = Path(__file__).parent
2729

2830

31+
def _docker_available() -> bool:
32+
if not shutil.which("docker"):
33+
return False
34+
try:
35+
return subprocess.run(["docker", "info"], capture_output=True, timeout=5).returncode == 0
36+
except Exception: # noqa: BLE001
37+
return False
38+
39+
40+
def _resolve_runtime(runtime: str) -> str:
41+
"""`auto` -> container when Docker is up (production default), else the local subprocess sandbox."""
42+
if runtime != "auto":
43+
return runtime
44+
return "container" if _docker_available() else "local"
45+
46+
2947
def _parse_args() -> argparse.Namespace:
3048
ap = argparse.ArgumentParser(description="Automated code-review agent (Skills + sandbox + DB).")
3149
src = ap.add_mutually_exclusive_group(required=True)
3250
src.add_argument("--diff-file", help="path to a unified-diff file")
3351
src.add_argument("--repo-path", help="path to a git worktree (reviews `git diff`)")
52+
src.add_argument("--files", help="comma-separated list of file paths to review as fully-added")
3453
src.add_argument("--fixture", help="name of a bundled fixture under fixtures/diffs/")
3554
ap.add_argument("--runtime",
36-
choices=["inprocess", "local", "container"],
37-
default="inprocess",
38-
help="scanner runtime: inprocess (fast), local (subprocess sandbox), container (Docker)")
55+
choices=["auto", "inprocess", "local", "container"],
56+
default="auto",
57+
help="scanner runtime: auto (sandbox: container if Docker, else local), "
58+
"inprocess (fast dev), local (subprocess sandbox), container (Docker)")
3959
ap.add_argument("--sandbox-timeout", type=float, default=None, help="sandbox timeout in seconds")
4060
ap.add_argument("--out-dir", default=".", help="where to write review_report.json/.md")
4161
ap.add_argument("--db-url", default="sqlite+aiosqlite:///./code_review.db")
@@ -44,13 +64,18 @@ def _parse_args() -> argparse.Namespace:
4464

4565

4666
def _run(args: argparse.Namespace) -> ReviewResult:
67+
runtime = _resolve_runtime(args.runtime)
4768
if args.repo_path:
48-
return run_review(repo_path=args.repo_path, runtime=args.runtime, sandbox_timeout=args.sandbox_timeout)
69+
return run_review(repo_path=args.repo_path, runtime=runtime, sandbox_timeout=args.sandbox_timeout)
70+
if args.files:
71+
return run_review(files=[p.strip() for p in args.files.split(",") if p.strip()],
72+
runtime=runtime,
73+
sandbox_timeout=args.sandbox_timeout)
4974
path = Path(args.diff_file) if args.diff_file else HERE / "fixtures" / "diffs" / args.fixture
5075
diff_text = path.read_text(encoding="utf-8")
51-
if args.runtime == "container":
76+
if runtime == "container":
5277
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)
78+
return run_review(diff_text=diff_text, runtime=runtime, sandbox_timeout=args.sandbox_timeout)
5479

5580

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

examples/skills_code_review_agent/storage/dao.py

Lines changed: 7 additions & 1 deletion
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
"""Persistence for reviews (issue #92, requirements 3 & 5).
87
98
Wraps the framework's ``SqlStorage`` so the schema is portable (SQLite default, PostgreSQL/MySQL
@@ -53,6 +52,13 @@ async def persist(self, result: ReviewResult) -> None:
5352
finding_count=int(mon.get("finding_count", 0)),
5453
severity_dist=mon.get("severity_dist", {}),
5554
exception_dist=mon.get("exception_dist", {}),
55+
diff_summary={
56+
"files_changed": result.summary.files_changed,
57+
"added": result.summary.added,
58+
"removed": result.summary.removed,
59+
"languages": result.summary.languages,
60+
"changed_files": [f.path for f in result.summary.files],
61+
},
5662
)
5763
await self._storage.add(db, task)
5864

examples/skills_code_review_agent/storage/models.py

Lines changed: 1 addition & 1 deletion
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
"""SQLAlchemy ORM models for code-review persistence (issue #92, requirement 5).
87
98
Mirrors the framework's own SQL pattern (``trpc_agent_sdk/sessions/_sql_session_service.py``):
@@ -48,6 +47,7 @@ class ReviewTaskORM(CodeReviewBase):
4847
finding_count: Mapped[int] = mapped_column(Integer, default=0)
4948
severity_dist: Mapped[dict[str, Any]] = mapped_column(DynamicJSON, default=dict)
5049
exception_dist: Mapped[dict[str, Any]] = mapped_column(DynamicJSON, default=dict)
50+
diff_summary: Mapped[dict[str, Any]] = mapped_column(DynamicJSON, default=dict) # input-diff summary (要求5)
5151
create_time: Mapped[datetime] = mapped_column(PreciseTimestamp, default=func.now())
5252
update_time: Mapped[datetime] = mapped_column(PreciseTimestamp, default=func.now(), onupdate=func.now())
5353

tests/examples/test_skills_code_review_agent.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,45 @@ def test_all_six_rule_categories_reachable() -> None:
265265
assert required in cats, f"category {required} not produced"
266266

267267

268+
# --- spec-alignment: input modes, env whitelist, diff-summary persistence -----------------------
269+
270+
271+
def test_file_list_input_mode() -> None:
272+
result = run_review(files=["pipeline/policy.py"], repo_root=str(_EXAMPLE_DIR))
273+
assert result.source_type == "file_list"
274+
assert result.summary.files_changed == 1
275+
276+
277+
def test_sandbox_env_is_whitelisted() -> None:
278+
import os
279+
280+
from pipeline.policy import ENV_ALLOWLIST, sandbox_env
281+
282+
os.environ["CR_LEAK_TEST"] = "should-not-pass"
283+
try:
284+
env = sandbox_env()
285+
assert "CR_LEAK_TEST" not in env
286+
assert set(env).issubset(set(ENV_ALLOWLIST))
287+
finally:
288+
del os.environ["CR_LEAK_TEST"]
289+
290+
291+
@pytest.mark.asyncio
292+
async def test_diff_summary_persisted(tmp_path) -> None:
293+
from storage.dao import ReviewStore
294+
295+
result = run_review(diff_text=(_FIXTURES / "security.diff").read_text())
296+
store = ReviewStore(f"sqlite+aiosqlite:///{tmp_path / 'cr.db'}")
297+
await store.init()
298+
try:
299+
await store.persist(result)
300+
got = await store.get_by_task_id(result.task_id)
301+
assert got["task"].diff_summary.get("files_changed") == 1
302+
assert got["task"].diff_summary.get("changed_files") == ["security.py"]
303+
finally:
304+
await store.close()
305+
306+
268307
# (text containing a secret, the raw secret that must not survive redaction) — the leak-test corpus.
269308
_LEAK_CORPUS = [
270309
('password = "hunter2supersecret"', "hunter2supersecret"),

0 commit comments

Comments
 (0)