Skip to content

Commit 4167477

Browse files
feat: add sandbox filter and telemetry summaries
1 parent 7c6a38c commit 4167477

9 files changed

Lines changed: 473 additions & 8 deletions

File tree

examples/skills_code_review_agent/README.md

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@ This example implements the early phases of issue #92 as a deterministic,
44
local-only code review loop. It reads a unified diff, scans added lines with a
55
small static rule set, redacts likely secrets, and writes JSON and Markdown
66
reports. When `--db-path` is provided, it also persists the review task,
7-
findings, and report metadata into SQLite.
7+
findings, report metadata, sandbox run metadata, and filter decisions into
8+
SQLite.
89

910
It intentionally does not call an LLM, remote sandbox, or SDK core extension.
11+
The sandbox runner in this example is a fake dry-run runner: it records what
12+
would have happened, but never executes untrusted code.
1013

1114
## Run
1215

@@ -37,11 +40,13 @@ python examples/skills_code_review_agent/run_agent.py --diff-file examples/skill
3740
```
3841

3942
When `--db-path` is set, the command prints the database path and generated
40-
task id. The database contains three tables:
43+
task id. The database contains these tables:
4144

4245
- `review_tasks`
4346
- `findings`
4447
- `reports`
48+
- `sandbox_runs`
49+
- `filter_decisions`
4550

4651
## Verify SQLite Records
4752

@@ -64,8 +69,9 @@ python -m pytest examples/skills_code_review_agent/tests
6469
```
6570

6671
The tests run only local parsing, rules, redaction, report generation, and
67-
dedupe checks. They do not call an LLM, Docker, Cube, remote network, or any
68-
external service.
72+
dedupe checks, fake sandbox status, filter decisions, telemetry summaries, and
73+
SQLite persistence. They do not call an LLM, Docker, Cube, remote network, or
74+
any external service.
6975

7076
## Fixtures
7177

@@ -90,12 +96,27 @@ external service.
9096
- Redacts likely API keys, tokens, secrets, and passwords before writing reports.
9197
- Produces `review_report.json` and `review_report.md`.
9298
- Optionally persists review tasks, findings, and report metadata to SQLite.
99+
- Records minimal filter decisions: `allow`, `deny`, or `needs_human_review`.
100+
- Records a fake dry-run sandbox result without executing untrusted code.
101+
- Adds telemetry summary fields to JSON and Markdown reports.
93102
- Includes local pytest coverage for the deterministic rule fixtures.
94103

104+
## Sandbox Notes
105+
106+
The current `dry-run` sandbox runner is deliberately fake and safe for local
107+
development. It simulates a static-check pass and records status/timing fields.
108+
It does not run shell commands, install dependencies, invoke Docker, or reach a
109+
remote sandbox service.
110+
111+
A true local runner should only be used as a development fallback because it
112+
does not isolate untrusted code. Later phases can replace this example runner
113+
with the project-level `ContainerWorkspaceRuntime` or `CubeWorkspaceRuntime`
114+
without changing the report and storage shape introduced here.
115+
95116
## Not Implemented Yet
96117

97118
- Real LLM review.
98119
- Real remote sandbox or container execution.
99120
- SDK core integration.
100-
- Filter governance and telemetry.
121+
- Production-grade filter governance and telemetry export.
101122
- Multi-agent orchestration.
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
"""Minimal filter decision abstraction for the code review example."""
2+
3+
from __future__ import annotations
4+
5+
import re
6+
from dataclasses import asdict
7+
from dataclasses import dataclass
8+
9+
from .diff_parser import ParsedDiff
10+
11+
12+
_HIGH_RISK_COMMAND_RE = re.compile(
13+
r"\b("
14+
r"rm\s+-rf\s+/|"
15+
r"curl\b[^|;&]*\|\s*(?:sh|bash)|"
16+
r"wget\b[^|;&]*\|\s*(?:sh|bash)|"
17+
r"chmod\s+777|"
18+
r"Invoke-WebRequest\b[^|;&]*\|\s*iex|"
19+
r"iwr\b[^|;&]*\|\s*iex"
20+
r")",
21+
re.IGNORECASE,
22+
)
23+
_MAX_DIFF_BYTES = 200_000
24+
_MAX_CHANGED_LINES = 2_000
25+
26+
27+
@dataclass(frozen=True)
28+
class FilterDecision:
29+
"""A minimal filter decision for review governance."""
30+
31+
decision: str
32+
reason: str
33+
34+
def to_dict(self) -> dict[str, str]:
35+
return asdict(self)
36+
37+
38+
def evaluate_filter_decision(diff_text: str, parsed_diff: ParsedDiff) -> FilterDecision:
39+
"""Evaluate whether the review can proceed automatically."""
40+
41+
if len(diff_text.encode("utf-8")) > _MAX_DIFF_BYTES:
42+
return FilterDecision(
43+
decision="needs_human_review",
44+
reason=f"diff is larger than {_MAX_DIFF_BYTES} bytes",
45+
)
46+
if len(parsed_diff.changed_lines) > _MAX_CHANGED_LINES:
47+
return FilterDecision(
48+
decision="needs_human_review",
49+
reason=f"diff changes more than {_MAX_CHANGED_LINES} lines",
50+
)
51+
if _HIGH_RISK_COMMAND_RE.search(diff_text):
52+
return FilterDecision(
53+
decision="needs_human_review",
54+
reason="diff contains a high-risk command pattern",
55+
)
56+
return FilterDecision(decision="allow", reason="diff is within dry-run review limits")

examples/skills_code_review_agent/agent/report.py

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,38 @@ def _counts(items: list[str]) -> dict[str, int]:
3535
return dict(sorted(Counter(items).items()))
3636

3737

38-
def build_report(*, diff_file: str, files: list[str], findings: list[Finding], dry_run: bool) -> dict[str, Any]:
38+
def build_report(
39+
*,
40+
diff_file: str,
41+
files: list[str],
42+
findings: list[Finding],
43+
dry_run: bool,
44+
filter_summary: dict[str, Any] | None = None,
45+
sandbox_summary: dict[str, Any] | None = None,
46+
telemetry_summary: dict[str, Any] | None = None,
47+
) -> dict[str, Any]:
3948
finding_dicts = [_redact_value(finding.to_dict()) for finding in findings]
4049
severity_counts = _counts([str(item["severity"]) for item in finding_dicts])
4150
category_counts = _counts([str(item["category"]) for item in finding_dicts])
51+
filter_summary = filter_summary or {"decision": "allow", "reason": "not evaluated"}
52+
sandbox_summary = sandbox_summary or {
53+
"runner_name": "none",
54+
"timeout_seconds": 0,
55+
"status": "not_run",
56+
"started_at": "",
57+
"finished_at": "",
58+
"stdout_summary": "",
59+
"stderr_summary": "",
60+
}
61+
telemetry_summary = telemetry_summary or {
62+
"files_scanned": files,
63+
"total_findings": len(finding_dicts),
64+
"severity_counts": severity_counts,
65+
"category_counts": category_counts,
66+
"sandbox_status": sandbox_summary["status"],
67+
"filter_decision": filter_summary["decision"],
68+
"duration_ms": 0,
69+
}
4270

4371
report = {
4472
"summary": {
@@ -51,6 +79,9 @@ def build_report(*, diff_file: str, files: list[str], findings: list[Finding], d
5179
"severity_counts": severity_counts,
5280
"category_counts": category_counts,
5381
},
82+
"filter": filter_summary,
83+
"sandbox": sandbox_summary,
84+
"telemetry": telemetry_summary,
5485
"findings": finding_dicts,
5586
}
5687
return _redact_value(report)
@@ -68,6 +99,9 @@ def _markdown_table_row(finding: dict[str, Any]) -> str:
6899

69100
def render_markdown(report: dict[str, Any]) -> str:
70101
summary = report["summary"]
102+
filter_summary = report.get("filter", {})
103+
sandbox_summary = report.get("sandbox", {})
104+
telemetry_summary = report.get("telemetry", {})
71105
lines = [
72106
"# Code Review Report",
73107
"",
@@ -76,6 +110,8 @@ def render_markdown(report: dict[str, Any]) -> str:
76110
f"- Diff file: `{summary['diff_file']}`",
77111
f"- Files scanned: {len(summary['files_scanned'])}",
78112
f"- Total findings: {summary['total_findings']}",
113+
f"- Filter decision: {filter_summary.get('decision', 'unknown')}",
114+
f"- Sandbox status: {sandbox_summary.get('status', 'unknown')}",
79115
"",
80116
"## Severity Counts",
81117
"",
@@ -86,6 +122,32 @@ def render_markdown(report: dict[str, Any]) -> str:
86122
else:
87123
lines.append("- none: 0")
88124

125+
lines.extend([
126+
"",
127+
"## Filter Summary",
128+
"",
129+
f"- Decision: {filter_summary.get('decision', 'unknown')}",
130+
f"- Reason: {filter_summary.get('reason', '')}",
131+
"",
132+
"## Sandbox Summary",
133+
"",
134+
f"- Runner: {sandbox_summary.get('runner_name', 'unknown')}",
135+
f"- Timeout seconds: {sandbox_summary.get('timeout_seconds', 0)}",
136+
f"- Status: {sandbox_summary.get('status', 'unknown')}",
137+
f"- Started: {sandbox_summary.get('started_at', '')}",
138+
f"- Finished: {sandbox_summary.get('finished_at', '')}",
139+
f"- Stdout summary: {sandbox_summary.get('stdout_summary', '')}",
140+
f"- Stderr summary: {sandbox_summary.get('stderr_summary', '')}",
141+
"",
142+
"## Telemetry Summary",
143+
"",
144+
f"- Files scanned: {len(telemetry_summary.get('files_scanned', []))}",
145+
f"- Total findings: {telemetry_summary.get('total_findings', 0)}",
146+
f"- Sandbox status: {telemetry_summary.get('sandbox_status', 'unknown')}",
147+
f"- Filter decision: {telemetry_summary.get('filter_decision', 'unknown')}",
148+
f"- Duration ms: {telemetry_summary.get('duration_ms', 0)}",
149+
])
150+
89151
lines.extend(["", "## Findings", ""])
90152
findings = report["findings"]
91153
if not findings:
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
"""Minimal dry-run sandbox runner abstraction.
2+
3+
This module intentionally does not execute untrusted code. It only records a
4+
structured dry-run result so later phases can replace the runner with a real
5+
container or Cube workspace runtime.
6+
"""
7+
8+
from __future__ import annotations
9+
10+
from dataclasses import asdict
11+
from dataclasses import dataclass
12+
from datetime import datetime
13+
from typing import Sequence
14+
15+
from .filtering import FilterDecision
16+
from .findings import Finding
17+
18+
19+
def _utc_now() -> str:
20+
return datetime.utcnow().replace(microsecond=0).isoformat() + "Z"
21+
22+
23+
@dataclass(frozen=True)
24+
class SandboxRun:
25+
"""Structured summary of a sandbox runner invocation."""
26+
27+
runner_name: str
28+
timeout_seconds: int
29+
status: str
30+
started_at: str
31+
finished_at: str
32+
stdout_summary: str
33+
stderr_summary: str
34+
35+
def to_dict(self) -> dict[str, object]:
36+
return asdict(self)
37+
38+
39+
class DryRunSandboxRunner:
40+
"""A fake sandbox runner that records a completed static-check simulation."""
41+
42+
def __init__(self, timeout_seconds: int = 30) -> None:
43+
self.runner_name = "dry-run"
44+
self.timeout_seconds = timeout_seconds
45+
46+
def run(
47+
self,
48+
*,
49+
files: Sequence[str],
50+
findings: Sequence[Finding],
51+
filter_decision: FilterDecision,
52+
) -> SandboxRun:
53+
started_at = _utc_now()
54+
finished_at = _utc_now()
55+
if filter_decision.decision == "deny":
56+
return SandboxRun(
57+
runner_name=self.runner_name,
58+
timeout_seconds=self.timeout_seconds,
59+
status="skipped",
60+
started_at=started_at,
61+
finished_at=finished_at,
62+
stdout_summary="dry-run sandbox skipped because filter denied the review",
63+
stderr_summary=filter_decision.reason,
64+
)
65+
return SandboxRun(
66+
runner_name=self.runner_name,
67+
timeout_seconds=self.timeout_seconds,
68+
status="completed",
69+
started_at=started_at,
70+
finished_at=finished_at,
71+
stdout_summary=f"simulated static checks for {len(files)} files and {len(findings)} findings",
72+
stderr_summary="",
73+
)

examples/skills_code_review_agent/agent/storage.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,32 @@ def init_db(db_path: Path) -> None:
5656
)
5757
"""
5858
)
59+
conn.execute(
60+
"""
61+
CREATE TABLE IF NOT EXISTS sandbox_runs (
62+
id INTEGER PRIMARY KEY AUTOINCREMENT,
63+
task_id TEXT NOT NULL,
64+
runner_name TEXT NOT NULL,
65+
timeout_seconds INTEGER NOT NULL,
66+
status TEXT NOT NULL,
67+
started_at TEXT NOT NULL,
68+
finished_at TEXT NOT NULL,
69+
stdout_summary TEXT NOT NULL,
70+
stderr_summary TEXT NOT NULL,
71+
FOREIGN KEY (task_id) REFERENCES review_tasks(task_id) ON DELETE CASCADE
72+
)
73+
"""
74+
)
75+
conn.execute(
76+
"""
77+
CREATE TABLE IF NOT EXISTS filter_decisions (
78+
task_id TEXT PRIMARY KEY,
79+
decision TEXT NOT NULL,
80+
reason TEXT NOT NULL,
81+
FOREIGN KEY (task_id) REFERENCES review_tasks(task_id) ON DELETE CASCADE
82+
)
83+
"""
84+
)
5985

6086

6187
def persist_review(
@@ -71,6 +97,8 @@ def persist_review(
7197
task_id = str(uuid.uuid4())
7298
summary = report["summary"]
7399
findings = report["findings"]
100+
sandbox = report.get("sandbox", {})
101+
filter_decision = report.get("filter", {})
74102

75103
with sqlite3.connect(db_path) as conn:
76104
conn.execute("PRAGMA foreign_keys = ON")
@@ -141,5 +169,43 @@ def persist_review(
141169
json.dumps(summary, ensure_ascii=False),
142170
),
143171
)
172+
conn.execute(
173+
"""
174+
INSERT INTO sandbox_runs (
175+
task_id,
176+
runner_name,
177+
timeout_seconds,
178+
status,
179+
started_at,
180+
finished_at,
181+
stdout_summary,
182+
stderr_summary
183+
) VALUES (?, ?, ?, ?, ?, ?, ?, ?)
184+
""",
185+
(
186+
task_id,
187+
sandbox.get("runner_name", ""),
188+
int(sandbox.get("timeout_seconds", 0)),
189+
sandbox.get("status", ""),
190+
sandbox.get("started_at", ""),
191+
sandbox.get("finished_at", ""),
192+
sandbox.get("stdout_summary", ""),
193+
sandbox.get("stderr_summary", ""),
194+
),
195+
)
196+
conn.execute(
197+
"""
198+
INSERT INTO filter_decisions (
199+
task_id,
200+
decision,
201+
reason
202+
) VALUES (?, ?, ?)
203+
""",
204+
(
205+
task_id,
206+
filter_decision.get("decision", ""),
207+
filter_decision.get("reason", ""),
208+
),
209+
)
144210

145211
return task_id

0 commit comments

Comments
 (0)