feat(examples): add automated code review agent example (Skills + sandbox + DB storage)

[DNKYr]

Summary

Adds examples/skills_code_review_agent/ — an automated code-review agent prototype that integrates the tRPC-Agent SDK's Skill system, workspace runtimes (local/container/cube), Filter governance, and SQL persistence.

Closes #92.

What's Included

• code-review Skill: SKILL.md + 5 rule-category checker scripts (security, async leaks, DB lifecycle, missing tests, secrets) that print structured JSON findings to stdout
• Sandbox execution: container runtime (default, python:3-slim) via env -i environment whitelist, 60s timeouts, 256KB output caps; local fallback (dev-only); cube support (env-configured)
• Governance engine: script allowlist, forbidden paths, network deny, execution budget, risk escalation (needs_human_review); dual enforcement via deterministic orchestrator + BaseFilter tool guard on skill_run
• Structured findings: pydantic model with severity/category/file/line/evidence/recommendation/confidence/source; dedup by (file, line, category); confidence gating (low → needs_human_review)
• SQL persistence: 6-table SQLAlchemy schema (review tasks, sandbox runs, findings, filter events, metrics, reports) via SDK SqlStorage; default SQLite, swappable db_url
• Redaction: shared secret-pattern module used by both sandbox checker and host-side redact layer; full-chain redaction (script evidence, sandbox output, DB writes, file output)
• Fake model / dry-run: FakeReviewModel(LLMModel) so the entire pipeline runs without API keys
• Reports: review_report.json + review_report.md with findings, severity stats, filter intercepts, sandbox runs, metrics
• CLI: review (--diff-file / --repo-path / --fixture, --runtime, --dry-run) + show --task-id
• 8 fixture diffs + 65 tests: covers clean, security, async_leak, db_lifecycle, missing_test, duplicate, sandbox_failure, secret_leak; dry-run ≤2 min (actual ~5s)
• README: usage guide, architecture diagram, rule table, schema table, 方案设计说明 (Chinese design explanation)

Verification

python -m pytest examples/skills_code_review_agent/tests -q   # 65 passed, 1 skipped
bash lint_flake8.sh examples/skills_code_review_agent         # clean
python run_agent.py review --fixture security_eval --runtime local --dry-run  # produces reports

Container mode: opt-in via CR_CONTAINER_TESTS=1 (requires Docker).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@e113610). Learn more about missing BASE report.

Additional details and impacted files

@@            Coverage Diff             @@
##             main        #198   +/-   ##
==========================================
  Coverage        ?   87.84906%           
==========================================
  Files           ?         482           
  Lines           ?       45157           
  Branches        ?           0           
==========================================
  Hits            ?       39670           
  Misses          ?        5487           
  Partials        ?           0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[raychen911]

是否可以补充一下设计文档？

[helloopenworld]

AI Code Review

我已完整审阅 pr.diff（新增 examples/skills_code_review_agent 示例，含 agent、review pipeline、sandbox、storage、checker 脚本及测试）。下面给出可定位验证的结论。

发现的问题

⚠️ Warning

• examples/skills_code_review_agent/.env:1-7：提交了真实的 .env 文件而非 .env.example

  • 该文件随仓库入库，占位为空值。后续开发者填入真实 TRPC_AGENT_API_KEY/CUBE_API_KEY 后极易被一并 commit，造成凭证泄露；应改为 .env.example 并在 .gitignore 中忽略 .env。

• examples/skills_code_review_agent/review/pipeline.py:1304：沙箱 runtime 创建后从未关闭

  • runtime = await create_runtime(opts.runtime) 在 cube 分支会建立 create_cube_sandbox_client（网络连接），但 run_review 只在内部 finally 关闭了 session、在外部 finally 关闭了 store，runtime/其底层 client 全程无 close()/清理，长驻进程下会泄漏连接与工作区资源。建议在 pipeline 末尾统一释放 runtime（或确认 SDK runtime 无需显式关闭并加注释）。

💡 Suggestion

• examples/skills_code_review_agent/review/governance.py:1095：路径穿越检测用 ".." in arg 子串匹配过于宽泛
  • _check_paths 对任意含字面 .. 的参数一律 deny，会把 data..csv 这类良性文件名或提示文本误判为穿越并拦截。作为治理过滤器尚属"宁可误报"，但建议改为按路径分量判断（如 posixpath.normpath(arg).split("/") 中是否出现 .. 组件），减少误杀且不影响真实穿越拦截。

总结

整体为示例工程，沙箱隔离、脱敏、治理与持久化链路设计合理且有测试覆盖，未发现必须修复的 Critical 问题。主要风险是 .env 入库的凭证卫生问题与 cube runtime 未关闭的资源泄漏，建议合入前处理。

测试建议

• 补充一个针对 cube/container runtime 生命周期的测试，验证 run_review 结束后 runtime/底层 client 被正确释放（或显式断言无残留工作区）。
• 在 test_governance.py 增加良性 .. 场景（如 data..csv）用例，固化 _check_paths 的预期行为，避免后续误杀回归。

[DNKYr]

是否可以补充一下设计文档？

感谢review！我已经添加设计中英双文设计文档在：example/doc/design.md 和 example/doc/design_zh.md

现在将Fix Code Warning

[helloopenworld]

AI Code Review

我已经完整阅读了 pr.diff（约 3782 行，新增一个 skills_code_review_agent 示例）。下面是审查结论。

发现的问题

🚨 Critical

• examples/skills_code_review_agent/review/llm_review.py:44-46：发送给 LLM 的静态 findings 未脱敏
  • prompt = REVIEW_REQUEST_TEMPLATE.format(findings_json=json.dumps([f.model_dump() for f in static_findings]), diff=redact_text(diff_text)) 中只对 diff 做了 redact_text，findings_json 直接序列化原始 finding（含 evidence/title/recommendation）。除 check_secrets 外，其余 checker（security/async/db 等）的 evidence 是原始源码行；当一行同时包含密钥与另一类模式时（如 cursor.execute(f"... '{api_key}'") 既命中 SQL 注入又含密钥），security finding 的 evidence 会以明文送入外部 LLM（real-model 模式），与示例“链路全脱敏、密钥不外泄”的承诺冲突。

    ...
    prompt = REVIEW_REQUEST_TEMPLATE.format(
        findings_json=json.dumps([f.model_dump() for f in static_findings]),
        diff=redact_text(diff_text))
    ...

  • 修复方向：对 findings_json 也做脱敏，且建议对每个 finding 的 evidence 等文本字段先 redact_text 再序列化——因为 JSON 转义后的 \" 会使 secret_patterns.sensitive_assign 正则失配，单纯对整段 JSON 脱敏仍可能漏掉口令类密钥。

⚠️ Warning

• examples/skills_code_review_agent/review/pipeline.py:147：runtime.destroy() 未放在 finally，异常路径下 cube 客户端资源泄漏

  • if hasattr(runtime, "destroy"): await runtime.destroy() 位于 try 块内（run_llm_review 之后）。若 session.open()、run_llm_review() 或其后的步骤抛异常，会直接进入 except Exception → finally store.close()，runtime.destroy() 被跳过。cube 运行时的 sandbox client 持有网络连接，反复异常会累积泄漏。修复：把 destroy() 移到 finally（与 store.close() 并列），并保留 hasattr 守卫。

    ...
    # Release runtime resources: cube sandbox clients hold network
    # connections that must be torn down; ...
    if hasattr(runtime, "destroy"):
        await runtime.destroy()
    ...

• examples/skills_code_review_agent/skills/code-review/scripts/secret_patterns.py:28（及 review/redaction.py）：sensitive_assign 正则无法匹配 JSON 转义引号，报告/LLM 入参中口令类密钥可能漏脱敏

  • redact 对整段文本做正则替换；当脱敏对象是已序列化的 JSON（pipeline 中 redact_text(json.dumps(report))、findings_json）时，口令赋值变成 password = \"hunter2secret\"，["'] 前的 \ 使 sensitive_assign 失配，纯口令类密钥不会被替换。字面量类模式（sk-、AKIA 等）仍能命中，但口令类会漏。建议在序列化前对原始字符串字段逐字段脱敏，而非对 JSON 整体脱敏。

💡 Suggestion

• examples/skills_code_review_agent/storage/store.py:71-78：add_findings 对每条 finding 各开一个事务（_add → 新 session + commit），批量写入既慢又非原子。建议在单次 session 内批量 add 后一次 commit，提升性能并保证一致性。

总结

示例整体结构清晰、治理与脱敏设计较完整，但存在一个明确的密钥外泄风险（静态 findings 未脱敏即送外部 LLM）和一个异常路径下的运行时资源泄漏，建议在合入前修复 Critical 项。

测试建议

• 补充一个用例：构造一行同时命中 check_security 模式又含密钥（如 SQL 注入 + 内嵌 api_key），在 real-model 或 fake 路径下断言送入 LLM 的 prompt 文本与持久化的 finding evidence 中均不含明文密钥。
• 补充一个用例：在 run_llm_review/session.open 抛异常时，断言 runtime.destroy() 仍被调用（可用带 destroy 的 fake runtime 验证）。

[helloopenworld]

静态 findings 未脱敏即送外部 LLM

发送给 LLM 的 prompt 仅对 diff 做 redact_text，findings_json 直接序列化原始 finding（含 evidence/title/recommendation）。当一行同时包含密钥与另一类模式时，security finding 的 evidence 会以明文送入外部 LLM，与示例“密钥不外泄”的承诺冲突。建议对每个 finding 的文本字段先 redact_text 再序列化。

[helloopenworld]

AI Code Review

发现的问题

⚠️ Warning

• examples/skills_code_review_agent/review/governance.py:62-70：脚本白名单可被路径前缀绕过

  • check_script 只对 argv 做路径逃逸检查（_check_paths 仅遍历 argv，见 governance.py:55-60），而脚本名本身只用 posixpath.basename(script) 比对白名单（governance.py:67）。因此 ../../check_security.py 或 /tmp/check_security.py 这类带绝对/.. 前缀但 basename 命中白名单的脚本会返回 allow，违反设计文档声明的"禁止绝对路径/../~ 逃逸"边界。编排器侧因使用硬编码脚本名且 run_script 重新拼接路径而未受影响，但经 GovernanceToolFilter 处理 LLM command 时该判定会被绕过。建议在 check_script 中对 script 同样做路径前缀校验，或要求 script == posixpath.basename(script)。

• examples/skills_code_review_agent/review/pipeline.py:147：runtime / session 资源清理未放入 finally，错误路径下泄漏

  • runtime.destroy()（pipeline.py:147-148）位于 try 块内、session.close()（pipeline.py:128-129）位于内层 try 的 finally，但外层 except（pipeline.py:182）只标记任务失败、外层 finally（pipeline.py:185）只关 store。若 session.open、LLM 富化或后续持久化阶段抛异常，runtime.destroy() 不会执行；session.open 失败时 session.close() 也不会执行。对 cube 运行时会泄漏沙箱客户端/网络连接，对 local 可能残留工作空间。建议把 runtime.destroy()（以及必要的 session.close() 兜底）放进外层 finally。

💡 Suggestion

• examples/skills_code_review_agent/storage/store.py:95-97：add_report 未对 report_json 脱敏

  • add_report 只对 report_md 调 redact_text，report_json 原样入库，与设计文档第 9 节"Report JSON and Markdown (in write_reports and ReviewStore.add_report)"声明的脱敏边界不一致。当前 pipeline.py:1388 已在上游对 report 整体脱敏后再传入，故无实际泄露；但作为持久化边界建议补齐 report_json=redact_text(json.dumps(report_json)) 以满足契约并防直接调用方误用。

• examples/skills_code_review_agent/review/llm_review.py:20：fenced JSON 正则贪婪匹配可能丢弃合法 findings

  • _FENCE_RE = re.compile(r"```(?:json)?\s*(\{.*\})\s*```", re.DOTALL) 中 \{.*\} 为贪婪匹配，当模型在一个围栏内输出多段 JSON 或围栏后紧跟额外 {...} 时会跨段捕获导致 json.loads 失败，回退到整段文本解析同样失败，最终返回空 findings。建议改为非贪婪 \{.*?\} 并配合括号配平，或优先取第一个完整 JSON 对象。

总结

整体实现完整、测试覆盖较充分，无必须修复的阻塞性缺陷；两处 Warning 分别为治理引擎脚本路径校验缺一个维度和异常路径下沙箱资源清理未兜底，建议修复以兑现文档声明的安全边界与稳定性保证。

测试建议

• 在 test_governance.py 补充用例：check_script("/tmp/check_security.py", [...]) 与 check_script("../check_security.py", [...]) 应返回 deny/forbidden_path；同时覆盖 check_command("python3 /tmp/check_security.py work/inputs/changes.diff") 经 check_script 后不被 allow。
• 在 test_pipeline.py 增加一条用例：注入会抛异常的 fake runtime（destroy 计数），断言异常路径下 destroy 仍被调用，避免 cube 资源泄漏回归。

[helloopenworld]

AI Code Review

我已经审查了完整的 diff。该 PR 添加了一个完整的 skills_code_review_agent 示例（pipeline、sandbox、governance、SQL storage、checker scripts 以及 tests）。现在我将报告在 diff 中验证过的问题。

发现的问题

⚠️ Warning

• examples/skills_code_review_agent/storage/store.py:2885-2887：add_report 未对 report_json 做脱敏

  • 该类 docstring 声明 "All write paths redact secrets"，但 add_report 仅 redact_text(report_md)，report_json 原样落库。当前安全仅靠 pipeline.py:1395 在调用前对整个 report 做了一次 redact_text 兜底；一旦该前置脱敏被改动或 store 被其他调用方直接使用，明文 secret 会进入 cr_reports.report_json。test_store_redacts_evidence_and_output 覆盖了 evidence/stdout/target/report_md，唯独没有带 secret 的 JSON 路径，该安全契约未被测试。建议在 add_report 内对 report_json 也做脱敏（或序列化后 redact），并补一条带 secret 的 JSON 落库测试。

• examples/skills_code_review_agent/review/sandbox.py:1641-1649（_effective_env）：容器/cube 运行时也无条件注入宿主机 python3 目录到 PATH 白名单

  • 该方法对 local/container/cube 一视同仁地把 os.path.dirname(shutil.which("python3")) 追加进白名单 PATH。沙箱隔离的核心保证之一是"PATH/HOME/LANG 白名单"，向容器 PATH 注入宿主机目录削弱了该保证（若该目录在容器中存在且二进制不同，可能解析到非预期可执行文件）。注释以"Nix 主机开发兜底"为由放行，但该兜底应限定为 local 运行时，或至少跳过 container/cube。test_env_whitelist_blocks_host_env 仅在 local 运行时验证，未覆盖容器 PATH 注入。

总结

整体设计稳健：沙箱失败降级、资源 destroy/close 在 finally 中释放、governance 路径穿越与预算拦截、secret 脱敏多重兜底均覆盖到位，未发现 Critical 级安全或逻辑错误。两处 Warning 分别是 store 层脱敏契约与测试缺口、以及沙箱 PATH 白名单对容器运行时的隔离削弱，建议修复后再合入。

测试建议

• 补一条 add_report 落库带 secret 的 report_json 用例，断言 cr_reports.report_json 中不含明文 secret。
• 增加一个容器运行时下 _effective_env 不注入宿主机 PATH（或注入受限）的断言，固化沙箱 PATH 白名单的隔离边界。