Skip to content

Commit 80bff22

Browse files
feat: add lightweight CI gate and stdin diff support
1 parent 4167477 commit 80bff22

4 files changed

Lines changed: 215 additions & 15 deletions

File tree

examples/skills_code_review_agent/README.md

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@ reports. When `--db-path` is provided, it also persists the review task,
77
findings, report metadata, sandbox run metadata, and filter decisions into
88
SQLite.
99

10-
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.
10+
This is a lightweight deterministic baseline, not a full sandbox/scanner
11+
implementation. It intentionally does not call an LLM, remote sandbox, Docker,
12+
Cube, E2B, external scanner, or SDK core extension. The sandbox runner in this
13+
example is a fake dry-run runner: it records what would have happened, but
14+
never executes untrusted code.
1315

1416
## Run
1517

@@ -33,6 +35,36 @@ You can also run the clean fixture:
3335
python examples/skills_code_review_agent/run_agent.py --diff-file examples/skills_code_review_agent/fixtures/clean.diff --output-dir examples/skills_code_review_agent/output_clean --dry-run
3436
```
3537

38+
## Run From Stdin
39+
40+
`--diff-file -` reads a unified diff from stdin and records the report input as
41+
`<stdin>`:
42+
43+
```bash
44+
git diff | python examples/skills_code_review_agent/run_agent.py --diff-file - --output-dir examples/skills_code_review_agent/output --dry-run
45+
```
46+
47+
## CI Failure Gate
48+
49+
By default the command exits with `0` even when findings are present. Use
50+
`--fail-on-severity` to make a lightweight CI gate:
51+
52+
```bash
53+
python examples/skills_code_review_agent/run_agent.py --diff-file examples/skills_code_review_agent/fixtures/security.diff --output-dir examples/skills_code_review_agent/output --dry-run --fail-on-severity high
54+
```
55+
56+
Values are `never`, `low`, `medium`, and `high`. For example, `medium` fails on
57+
medium or high findings, while `high` fails only on high findings.
58+
59+
## List Rules
60+
61+
```bash
62+
python examples/skills_code_review_agent/run_agent.py --list-rules
63+
```
64+
65+
This prints the deterministic rule id, category, default severity, description,
66+
and known limitations without reading a diff.
67+
3668
## Run With SQLite
3769

3870
```bash
@@ -70,8 +102,9 @@ python -m pytest examples/skills_code_review_agent/tests
70102

71103
The tests run only local parsing, rules, redaction, report generation, and
72104
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.
105+
SQLite persistence. They also cover the lightweight CI failure gate, stdin diff
106+
input, and rule listing. They do not call an LLM, Docker, Cube, remote network,
107+
external scanners, or any external service.
75108

76109
## Fixtures
77110

@@ -95,6 +128,9 @@ any external service.
95128
- `open(...)` without `with`
96129
- Redacts likely API keys, tokens, secrets, and passwords before writing reports.
97130
- Produces `review_report.json` and `review_report.md`.
131+
- Supports reading a unified diff from stdin with `--diff-file -`.
132+
- Supports a lightweight exit-code gate with `--fail-on-severity`.
133+
- Prints rule metadata with `--list-rules`.
98134
- Optionally persists review tasks, findings, and report metadata to SQLite.
99135
- Records minimal filter decisions: `allow`, `deny`, or `needs_human_review`.
100136
- Records a fake dry-run sandbox result without executing untrusted code.
@@ -117,6 +153,7 @@ without changing the report and storage shape introduced here.
117153

118154
- Real LLM review.
119155
- Real remote sandbox or container execution.
156+
- External scanner integration such as Bandit, Ruff, detect-secrets, or Semgrep.
120157
- SDK core integration.
121158
- Production-grade filter governance and telemetry export.
122159
- Multi-agent orchestration.

examples/skills_code_review_agent/agent/rules.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,44 @@
2626
_BROAD_EXCEPT_RE = re.compile(r"^\s*except\s+Exception(?:\s+as\s+\w+)?\s*:")
2727
_OPEN_CALL_RE = re.compile(r"\bopen\s*\(")
2828

29+
RULES_MANIFEST = [
30+
{
31+
"id": "static-rule:hardcoded-secret",
32+
"category": "secret",
33+
"default_severity": "high",
34+
"description": "Flags added lines that appear to assign hardcoded API keys, tokens, secrets, or passwords.",
35+
"limitations": "Regex-based and line-oriented; it can miss split secrets and may flag synthetic test data.",
36+
},
37+
{
38+
"id": "static-rule:sql-string-concat",
39+
"category": "sql-injection",
40+
"default_severity": "high",
41+
"description": "Flags SQL statements that appear to use string interpolation, formatting, or concatenation.",
42+
"limitations": "Does not parse Python AST or validate actual database driver parameter usage.",
43+
},
44+
{
45+
"id": "static-rule:http-timeout",
46+
"category": "network-timeout",
47+
"default_severity": "medium",
48+
"description": "Flags requests/httpx calls on one added line when no explicit timeout= argument is present.",
49+
"limitations": "Only handles simple single-line calls and cannot resolve wrapper defaults.",
50+
},
51+
{
52+
"id": "static-rule:broad-except",
53+
"category": "error-handling",
54+
"default_severity": "medium/high",
55+
"description": "Flags broad except Exception handlers, escalating when the next added line swallows the error.",
56+
"limitations": "Line-oriented; it does not build a control-flow graph or inspect existing surrounding code.",
57+
},
58+
{
59+
"id": "static-rule:open-without-context-manager",
60+
"category": "resource-lifecycle",
61+
"default_severity": "medium",
62+
"description": "Flags simple open(...) usage that is not introduced with a with statement.",
63+
"limitations": "Does not track close() calls across later lines or helper abstractions.",
64+
},
65+
]
66+
2967

3068
def _finding(
3169
*,

examples/skills_code_review_agent/run_agent.py

Lines changed: 63 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from __future__ import annotations
1111

1212
import argparse
13+
import sys
1314
import time
1415
from pathlib import Path
1516
from typing import Sequence
@@ -18,12 +19,20 @@
1819
from agent.filtering import evaluate_filter_decision
1920
from agent.report import build_report
2021
from agent.report import write_reports
22+
from agent.rules import RULES_MANIFEST
2123
from agent.rules import run_static_rules
2224
from agent.sandbox import DryRunSandboxRunner
2325
from agent.storage import persist_review
2426
from agent.telemetry import build_telemetry_summary
2527

2628

29+
_SEVERITY_RANK = {
30+
"low": 1,
31+
"medium": 2,
32+
"high": 3,
33+
}
34+
35+
2736
def _resolve_path(raw: str) -> Path:
2837
path = Path(raw).expanduser()
2938
if path.is_absolute():
@@ -35,12 +44,39 @@ def _default_diff_path() -> str:
3544
return str(Path(__file__).resolve().parent / "fixtures" / "security.diff")
3645

3746

47+
def _read_diff(raw: str) -> tuple[str, str, str]:
48+
if raw == "-":
49+
return "<stdin>", "<stdin>", sys.stdin.read()
50+
51+
diff_file = _resolve_path(raw)
52+
if not diff_file.exists():
53+
raise FileNotFoundError(f"diff file not found: {diff_file}")
54+
return str(diff_file), raw, diff_file.read_text(encoding="utf-8")
55+
56+
57+
def _print_rules() -> None:
58+
print("Deterministic rules")
59+
for rule in RULES_MANIFEST:
60+
print(f"- {rule['id']}")
61+
print(f" category: {rule['category']}")
62+
print(f" default severity: {rule['default_severity']}")
63+
print(f" description: {rule['description']}")
64+
print(f" limitations: {rule['limitations']}")
65+
66+
67+
def _failure_gate_triggered(findings: Sequence[object], threshold: str) -> bool:
68+
if threshold == "never":
69+
return False
70+
minimum = _SEVERITY_RANK[threshold]
71+
return any(_SEVERITY_RANK.get(getattr(finding, "severity", ""), 0) >= minimum for finding in findings)
72+
73+
3874
def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace:
3975
parser = argparse.ArgumentParser(description="Run deterministic code review over a unified diff.")
4076
parser.add_argument(
4177
"--diff-file",
4278
default=_default_diff_path(),
43-
help="Path to a unified diff file. Defaults to fixtures/security.diff.",
79+
help="Path to a unified diff file, or '-' to read from stdin. Defaults to fixtures/security.diff.",
4480
)
4581
parser.add_argument(
4682
"--output-dir",
@@ -57,23 +93,34 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace:
5793
default="",
5894
help="Optional SQLite database path for persisting review tasks, findings, and reports.",
5995
)
96+
parser.add_argument(
97+
"--fail-on-severity",
98+
choices=("never", "low", "medium", "high"),
99+
default="never",
100+
help="Exit with status 1 when findings meet or exceed this severity. Defaults to never.",
101+
)
102+
parser.add_argument(
103+
"--list-rules",
104+
action="store_true",
105+
help="Print deterministic rule metadata and exit without reading a diff.",
106+
)
60107
return parser.parse_args(argv)
61108

62109

63110
def main(argv: Sequence[str] | None = None) -> int:
64111
started = time.perf_counter()
65112
args = parse_args(argv)
66-
diff_file = _resolve_path(args.diff_file)
113+
if args.list_rules:
114+
_print_rules()
115+
return 0
116+
67117
output_dir = _resolve_path(args.output_dir)
68118
db_path = _resolve_path(args.db_path) if args.db_path else None
69-
70-
if not diff_file.exists():
71-
raise FileNotFoundError(f"diff file not found: {diff_file}")
72-
73-
diff_text = diff_file.read_text(encoding="utf-8")
119+
diff_display, report_diff_file, diff_text = _read_diff(args.diff_file)
74120
parsed_diff = parse_unified_diff(diff_text)
75121
filter_decision = evaluate_filter_decision(diff_text, parsed_diff)
76122
findings = run_static_rules(parsed_diff.changed_lines)
123+
failure_gate_triggered = _failure_gate_triggered(findings, args.fail_on_severity)
77124
sandbox_run = DryRunSandboxRunner().run(
78125
files=parsed_diff.files,
79126
findings=findings,
@@ -88,7 +135,7 @@ def main(argv: Sequence[str] | None = None) -> int:
88135
duration_ms=duration_ms,
89136
)
90137
report = build_report(
91-
diff_file=args.diff_file,
138+
diff_file=report_diff_file,
92139
files=parsed_diff.files,
93140
findings=findings,
94141
dry_run=args.dry_run,
@@ -111,17 +158,23 @@ def main(argv: Sequence[str] | None = None) -> int:
111158
low_count = report["summary"]["severity_counts"].get("low", 0)
112159

113160
print("Skills code review dry-run complete")
114-
print(f"Diff file: {diff_file}")
161+
print(f"Diff file: {diff_display}")
115162
print(f"Changed files: {len(parsed_diff.files)}")
116163
print(f"Findings: high={high_count} medium={medium_count} low={low_count}")
117164
print(f"Filter decision: {filter_decision.decision} ({filter_decision.reason})")
118165
print(f"Sandbox status: {sandbox_run.status}")
166+
if args.fail_on_severity == "never":
167+
print("Failure gate: disabled (--fail-on-severity never)")
168+
elif failure_gate_triggered:
169+
print(f"Failure gate: triggered (--fail-on-severity {args.fail_on_severity})")
170+
else:
171+
print(f"Failure gate: passed (--fail-on-severity {args.fail_on_severity})")
119172
print(f"JSON report: {json_path}")
120173
print(f"Markdown report: {md_path}")
121174
if db_path is not None:
122175
print(f"Database: {db_path}")
123176
print(f"Task ID: {task_id}")
124-
return 0
177+
return 1 if failure_gate_triggered else 0
125178

126179

127180
if __name__ == "__main__":

examples/skills_code_review_agent/tests/test_static_review.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
from __future__ import annotations
44

5+
import io
6+
import json
57
import sqlite3
68
from pathlib import Path
79

@@ -15,6 +17,7 @@
1517
from agent.sandbox import DryRunSandboxRunner
1618
from agent.storage import persist_review
1719
from agent.telemetry import build_telemetry_summary
20+
from run_agent import main as run_agent_main
1821

1922

2023
EXAMPLE_ROOT = Path(__file__).resolve().parents[1]
@@ -178,3 +181,72 @@ def test_sqlite_persists_sandbox_and_filter_rows(tmp_path: Path) -> None:
178181
assert filter_count == 1
179182
assert sandbox_status == "completed"
180183
assert filter_decision == "allow"
184+
185+
186+
def test_fail_on_severity_high_returns_nonzero(tmp_path: Path, capsys) -> None:
187+
exit_code = run_agent_main(
188+
[
189+
"--diff-file",
190+
str(FIXTURES / "security.diff"),
191+
"--output-dir",
192+
str(tmp_path),
193+
"--dry-run",
194+
"--fail-on-severity",
195+
"high",
196+
]
197+
)
198+
captured = capsys.readouterr()
199+
200+
assert exit_code == 1
201+
assert "Failure gate: triggered (--fail-on-severity high)" in captured.out
202+
203+
204+
def test_fail_on_severity_never_returns_zero(tmp_path: Path, capsys) -> None:
205+
exit_code = run_agent_main(
206+
[
207+
"--diff-file",
208+
str(FIXTURES / "security.diff"),
209+
"--output-dir",
210+
str(tmp_path),
211+
"--dry-run",
212+
"--fail-on-severity",
213+
"never",
214+
]
215+
)
216+
captured = capsys.readouterr()
217+
218+
assert exit_code == 0
219+
assert "Failure gate: disabled (--fail-on-severity never)" in captured.out
220+
221+
222+
def test_list_rules_outputs_rule_metadata(capsys) -> None:
223+
exit_code = run_agent_main(["--list-rules", "--diff-file", "does-not-exist.diff"])
224+
captured = capsys.readouterr()
225+
226+
assert exit_code == 0
227+
assert "Deterministic rules" in captured.out
228+
assert "static-rule:hardcoded-secret" in captured.out
229+
assert "category: secret" in captured.out
230+
assert "limitations:" in captured.out
231+
232+
233+
def test_diff_file_stdin_reads_unified_diff(tmp_path: Path, monkeypatch, capsys) -> None:
234+
diff_text = (FIXTURES / "missing_timeout.diff").read_text(encoding="utf-8")
235+
monkeypatch.setattr("sys.stdin", io.StringIO(diff_text))
236+
237+
exit_code = run_agent_main(
238+
[
239+
"--diff-file",
240+
"-",
241+
"--output-dir",
242+
str(tmp_path),
243+
"--dry-run",
244+
]
245+
)
246+
captured = capsys.readouterr()
247+
report = json.loads((tmp_path / "review_report.json").read_text(encoding="utf-8"))
248+
249+
assert exit_code == 0
250+
assert "Diff file: <stdin>" in captured.out
251+
assert report["summary"]["diff_file"] == "<stdin>"
252+
assert report["summary"]["category_counts"]["network-timeout"] == 1

0 commit comments

Comments
 (0)