examples: add a Skills + sandbox + database automated code-review agent - #124
Open
Acture wants to merge 15 commits into
Open
examples: add a Skills + sandbox + database automated code-review agent#124Acture wants to merge 15 commits into
Acture wants to merge 15 commits into
Conversation
An automated code-review agent built on the Skills + sandbox + DB primitives (issue trpc-group#92). Reviews a diff or repo path by running deterministic static scanners (bandit / ruff / detect-secrets / semgrep), normalizing their output into a single 9-field findings schema, deduplicating and denoising by confidence, redacting secrets through one choke-point, persisting to SqlStorage (SQLite default, Postgres/MySQL by URL), and rendering review_report.json/.md. Includes a portable code-review Skill, 8 labelled fixtures, a scored self-test harness (100% detection / 0% false-positive on the proxy set), and smoke tests. Dry-run works with no API key. Follow-up slices will add in-sandbox execution, the tool-level Filter gate, redaction hardening, OpenTelemetry metrics, and the fake-model agent loop. Updates trpc-group#92 RELEASE NOTES: NONE
Wire the review through the framework: an LlmAgent with a review_code FunctionTool, driven by a FakeReviewModel that needs no API key. On the first turn the fake model emits a single tool call with the user's diff; on the second turn (after the deterministic pipeline runs behind the tool) it summarizes the findings. run_agent.py demos it; a smoke test asserts the tool is called, a summary is produced, and no secret leaks. The guard Filter will attach on the tool via filters_name (TOOL scope), not on the agent, in a follow-up slice. Updates trpc-group#92 RELEASE NOTES: NONE
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 trpc-group#92 RELEASE NOTES: NONE
Slice 3 (requirement 7). pipeline/policy.py::ReviewPolicy decides allow / deny / needs_human_review for a sandbox action based on the command (high-risk patterns), touched paths (forbidden roots), network hosts (allowlist) and budget. It is enforced at two sites sharing the policy: the deterministic sandbox gate (pipeline/sandbox.py): a denied or human-review action is never launched; a blocked SandboxRunResult is recorded and surfaced in the report's Filter-interception section (completing requirement 8) and in monitoring.block_count. agent/filter.py::ReviewGuardFilter: a TOOL-scoped BaseFilter attached on the review tool that refuses a guarded tool call via FilterResult(is_continue=False). Adds tests for the policy decisions, that a denied action never reaches the sandbox, the guard filter, and the report's block section. Updates trpc-group#92 RELEASE NOTES: NONE
Slice 4 (criterion 5). redact() now layers full-token provider regexes (AWS, GitHub, GitLab, Slack, Stripe, Google, SendGrid, npm, JWT, PEM, bearer, basic-auth URLs) with a Shannon-entropy catch-all for generic base64/hex secrets. A leak-test corpus (16 secret types) verifies >=95% masking and zero plaintext survivors, plus a benign-code corpus verifies no false positives. detect-secrets stays as the secret-leakage *scanner* but is not used for redaction: its scan_line returns partial/benign values that hurt precision. The regex + entropy layers reach 100% on the corpus cleanly. Updates trpc-group#92 RELEASE NOTES: NONE
The issue's deliverables require 8 sample diffs covering specific scenarios and rule coverage across all 6 categories. Rework fixtures to: clean, security, async_resource_leak, db_lifecycle, missing_tests, duplicate_finding, sandbox_failure, secret_redaction. Add two detectors that bandit/ruff don't provide: db_lifecycle: a DB connection/cursor opened without with and never closed (content heuristic, mirrored in the sandbox run_checks.py). missing_tests: source changed with no corresponding test change (diff-level; added in the engine for every runtime). Enable ruff's flake8-bandit (S) rules so os.system is flagged by both bandit (B605) and ruff (S605), giving a genuine duplicate the dedup stage collapses. Suppress assert-used (B101/S101) noise. selftest now scores the 8 official-scenario fixtures (100%/0% on the proxy set); adds scenario tests for db_lifecycle, missing_tests, duplicate collapse, sandbox-failure, and all-6-categories. Updates trpc-group#92 RELEASE NOTES: NONE
…, 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 trpc-group#92 RELEASE NOTES: NONE
Add DESIGN.md — the required 方案设计说明 covering Skill design, sandbox isolation, Filter strategy, monitoring fields, DB schema, dedup/denoise, and the security boundary. Refresh the README status to reflect the full six-category rule coverage, the official-scenario fixtures, the input modes, and the auto/sandbox default runtime. Updates trpc-group#92 RELEASE NOTES: NONE
Make the sandbox the accepted path. run_review defaults to the auto runtime, which uses the container sandbox when Docker is available and the local subprocess sandbox otherwise; in-process becomes an explicit dev fast-path, and both the selftest and the agent tool run through the sandbox. The standalone run_checks.py and pipeline/scanners.py are reconciled to emit identical findings (aligned severity, confidence and path normalization), enforced by a parity test, so the two paths cannot drift. Make failures visible. A missing scanner produces a needs-human-review finding instead of a silent zero-finding report; the monitoring tool-call count reflects the scanners that actually ran; and the persisted review status is derived from the run (blocked, failed, or completed) rather than always completed. Correctness and deliverables. File-level findings key on the rule as well as the category so distinct issues in one category are not collapsed. The container path's post-processing is extracted into build_container_result and unit-tested without Docker. The agent CLI gains a dry-run flag that forces the fake model, the stale run_review docstring is corrected, and the change adds the committed sample report and a RULES.md documenting all six categories. yapf and the trailing-newline fix leave the tree lint-clean. Updates trpc-group#92 RELEASE NOTES: NONE
The container runtime silently downgraded file-list and worktree inputs to the in-process path: only --diff-file and --fixture were wired to run_review_container, while --files and --repo-path called the sync run_review with runtime="container", which had no container branch and fell through to in-process. Extract the input materialization into a shared _resolve_input helper used by both entry points, extend run_review_container to accept all three input modes, and route every container request through it in the CLI. Sync run_review now rejects the container runtime loudly instead of falling back. Also correct two stale docstring and README references to a fixture that no longer exists. Updates trpc-group#92 RELEASE NOTES: NONE
The redaction corpus held fake Stripe and GitLab keys as contiguous literals, which secret-scanning push protection flags. Build those two from fragments so the source never contains a full provider pattern; the assembled runtime value is unchanged, so the redactor is exercised exactly as before. Updates trpc-group#92 RELEASE NOTES: NONE
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
Author
|
I have read the CLA Document and I hereby sign the CLA |
Rook1ex
added a commit
to trpc-group/cla-database
that referenced
this pull request
Jul 5, 2026
The example ships its own dependencies, which the SDK test job does not install, so importing the test module failed collection in the main CI. Guard the module with importorskip on unidiff so it skips cleanly when the example dependencies are not present and runs in full when they are. Updates trpc-group#92 RELEASE NOTES: NONE
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #124 +/- ##
==========================================
Coverage ? 88.43489%
==========================================
Files ? 491
Lines ? 46035
Branches ? 0
==========================================
Hits ? 40711
Misses ? 5324
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
… fragments The internal security scan flagged the postgres connection string in the redaction corpus as a database credential leak. Build the URL from fragments so no contiguous connection URL with inline credentials appears as a literal; the assembled runtime value is unchanged, so the redactor is still tested on a real connection string. Updates trpc-group#92 RELEASE NOTES: NONE
Acture
force-pushed
the
feat/code-review-agent
branch
from
July 5, 2026 20:27
39344c1 to
205a23d
Compare
The scored self-test only covered the public fixtures the detectors were tuned against, which is circular evidence for the hidden-sample detection and false-positive thresholds. Add fixtures/holdout: paired danger and safe cases built from standard scanner rules the detectors were not tuned on (pickle.loads, yaml.load, blocking sleep in async, unclosed file and connection, a source change without a test), plus their safe variants. selftest.py --holdout scores this set through the sandbox and a new test enforces detection at least eighty percent and false positives at most fifteen percent. On the held-out set the pipeline reaches full detection with zero false positives, because findings come from real scanners rather than hand-written rules, so unseen standard patterns are caught without retuning. Updates trpc-group#92 RELEASE NOTES: NONE
raychen911
requested changes
Jul 31, 2026
| return latest | ||
|
|
||
|
|
||
| class FakeReviewModel(LLMModel): |
Contributor
There was a problem hiding this comment.
这里是否可以使用实际的模型测试？当前issue期望基于框架做一个产品性质的cr
| from pipeline.engine import run_review | ||
|
|
||
| from . import filter as _guard # noqa: F401 - importing registers the "review_guard" tool filter | ||
|
|
Contributor
There was a problem hiding this comment.
当前框架的skill是通过toolset的方式加载的，在该tool文件中没有看到skill相关的逻辑？这里是否使用了skill文件？该issue期望用户设计skill文件（可以包含脚本），框架根据用户设计的skill文件来做cr，目前看是存在SKILL.md 文件，但是没有看到在哪加载的
Author
There was a problem hiding this comment.
之前错误硬编码了，已按 examples/skills/ 的用法重写。
…real model Addresses the two review comments on trpc-group#124. The example previously bypassed the framework's Skills system entirely: it hand-rolled a FunctionTool that imported pipeline.engine directly, and invoked skills/code-review/scripts/run_checks.py by hardcoded path. SKILL.md was never loaded by anything. The agent now goes through SkillToolSet: stage_review_input materializes the diff, skill_load loads SKILL.md and its rule docs, skill_run executes the skill's own script in a workspace sandbox, finalize_review dedups, persists and renders. A real model is the default. FakeReviewModel stays, because criterion 6 requires a dry-run mode that works without an API key, but it is now an explicit --dry-run that walks the same four steps rather than the default that walked a shortcut. A real-model integration test runs under CR_LIVE_MODEL_TEST=1 and skips cleanly without a key. Also fixed while re-reading against the issue: - The sandbox default was inverted. Requirement 2 allows a local runtime only as a development fallback; it was the default. Now container by default. - The review rules had two implementations (pipeline/scanners.py in-process and the skill's run_checks.py) kept in step by a parity test. scanners.py is deleted; run_checks.py absorbed its changed-line filtering, semgrep adapter, per-scanner error isolation and missing-tests rule, and is now the only one. - Requirement 8 was not actually met: SkillToolSet also exposes skill_exec and workspace_exec, which are constructed without filters, so the guard on skill_run could be routed around. The toolset is narrowed to skill_load + skill_run (tool_filter has no effect here — get_tools never consults it). - timeout and env passed flat to SkillToolSet were silently discarded; they must go through run_tool_kwargs. Non-whitelisted environment variables are now blanked, so the scanners no longer inherit TRPC_AGENT_API_KEY (requirement 7). - skill_load's parameter is skill_name, not skill; the instruction said skill. The local and container workspace runtimes stage host:// inputs at different depths, so a fixed --target cannot work for both. materialize_diff writes a .changes.diff sidecar beside the changed files and run_checks.py resolves its scan root from it, which also gives the script the diff it needs for changed-line filtering and the missing-tests rule. Verified to produce identical findings, with paths matching the diff, under both layouts. Tests: the suite asserts the four-step sequence, that SKILL.md's content reaches the model, and that finding paths match the diff. Emptying SKILL.md turns two tests red and deleting it turns three red; previously both cases stayed green.
Contributor
AI Code Review发现的问题
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A self-contained automated code-review agent for issue #92, under
examples/skills_code_review_agent. Adds files only — no framework code changed. Findings come from real scanners (bandit / ruff / detect-secrets) plus DB-lifecycle and missing-test heuristics, run in a sandbox, persisted to SQL, driven by a fake-modelLlmAgent. Runs with no API key.Requirements (具体要求)
Acceptance (验收标准)
Deliverables (交付物)
Example dir · Skill · DB schema + storage · scenario fixtures + held-out set · sample report + README · DESIGN.md
Updates #92
RELEASE NOTES: NONE