feat(examples): add skills code review agent - #209
Conversation
AI Code Review我已经了解了所有上下文，现在来写审查报告。 发现的问题🚨 Critical
|
| } | ||
|
|
||
|
|
||
| def execute_skill_script( |
There was a problem hiding this comment.
sandbox 未按 runtime 真正隔离，container 运行时默认放行
execute_skill_script 忽略 runtime 参数，对所有运行时（含 container/e2b/cube）都用 subprocess.run 直接在宿主执行；而 evaluate_invocations 仅在 runtime=="local" 时降级为 NEEDS_HUMAN_REVIEW，container 等被默认 ALLOW。建议对非 local 运行时走真实容器/远程执行，或在 filter 中一律标为 NEEDS_HUMAN_REVIEW 并在报告中显式注明未隔离执行。
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #209 +/- ##
==========================================
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:
|
| ): | ||
| task.add_filter_decision(decision) | ||
| if decision.decision.value == "allow": | ||
| sandbox_run = execute_skill_script( |
There was a problem hiding this comment.
对于skill，框架会自动在沙箱中运行，这里是否有必要主动调用？
There was a problem hiding this comment.
感谢您提醒，上一版确实由 Agent 在 Filter 判断后直接管理 workspace 和脚本执行，造成了 Agent 与 Skill 框架职责重复，已调整为：Agent 只构造结构化的 skill_run 请求；在 container 模式下，由框架的 SkillToolSet/skill_run 负责 Skill staging、workspace、输入映射、沙箱执行和超时控制，Agent 不再自行创建 runtime 或直接运行容器命令。
dry-run/fake-model 模式因为没有模型生成 tool call，所以确定性测试流程仍会调用同一个公开的 skill_run 接口，但这里只模拟 tool dispatch，不再自行实现 sandbox，请帮忙review这样的职责划分是否符合框架预期
AI Code Review我已完整审阅 发现的问题🚨 Critical未发现 Critical 级别问题。
|
AI Code Review示例输出证实了具体化的 diff 文件位于 我已经完成了分析，现在开始撰写审查报告。 发现的问题🚨 Critical
|
| def _count_by_disposition(findings: list, disposition: FindingDisposition) -> int: | ||
| """Count findings by presentation bucket.""" | ||
|
|
||
| return sum(1 for finding in findings if finding.disposition == disposition) |
There was a problem hiding this comment.
物化的 diff 中间文件以明文持久化密钥且无清理
_materialize_diff_input 将未脱敏的 task.review_input.diff_text 直接写入 skill_inputs/{task_id}.diff，全流程无删除或脱敏，真实凭证会以明文残留磁盘，违背报告/DB 已脱敏的安全契约。建议写入前对 diff 文本执行 redact_text，或在沙箱脚本执行完成后删除该文件。
There was a problem hiding this comment.
已将原始 diff 从 output_dir/skill_inputs/ 移到短生命周期的 TemporaryDirectory Skill 脚本执行完成后会自动清理，发生异常时也会通过 context manager 清理，不再把未脱敏 diff 作为输出产物保留
同时，返回的 task.review_input 和 parsed_diff 也会进行深度脱敏,报告和数据库继续只保存脱敏内容，原始输入仅保留 SHA256 用于审计，并补充了回归测试，验证临时文件会被删除，并且 task、报告和数据库中均不包含明文密钥
AI Code Review我已经掌握了所有发现的问题。让我来写最终的审查结论。 发现的问题
|
AI Code ReviewI now have enough verified context to produce the review. 发现的问题🚨 Critical
|
AI Code Review我已经审查了完整的 diff 和相关的仓库上下文，以下是我的审查结果。 发现的问题🚨 Critical
|
AI Code Review已确认： 现在我来检查一下 for raw_path in parts[2:4]:
normalized_path = raw_path.replace("\\", "/")
if len(normalized_path) > 2 and normalized_path[1] == "/":
normalized_path = normalized_path[2:]这里检查 现在我来验证一下最关键的声明，即关于多行正则表达式（regex）模式 3 和脱敏器（redactor）实际行为的说法。我无法通过实验进行测试。让我仔细重读一下 模式 1（第 17-21 行）： 规则引擎模式（第 68 行）： 我现在非常有信心。两个关键（Critical）发现都已核实。让我再检查一下子代理没有深入审查的一个担忧：在 让我检查最后一个潜在问题： 我已核实完毕。让我最终确定评审报告。我将报告 2 个 Critical 问题和 1 个 Warning 项（并将 telemetry 的不一致问题合并为优先级较低的 Warning 或 Suggestion）。关于 deduper 的说法站不住脚（该函数确实被用于指纹识别），所以我将其删除。我将把 GitHub token 问题（#1）和多词值问题（#2）合并为一个 Critical 问题，因为它们有相同的根本原因（“脱敏器的模式集是规则引擎模式集的真子集，导致了脱敏缺口”），并有不同的修复方案——但说明中要求除非修复方案有显著不同，否则合并。它们是同一个文件中相邻的模式，根本原因相同。我考虑一下：根本原因是“脱敏器没有覆盖规则引擎检测到的所有凭据形状”。多词值问题和 GitHub token 问题都是这一根本原因的表现。根据指南 #4 和 #9，我应该将它们合并。但修复方法不同（GitHub 使用新模式，多词使用值类变更）。指南 #9 指出，除非修复方法显著不同，否则合并。它们确实不同。我会将它们保留为一条 Critical 项，并在同一个条目中包含两个子修复方案。 发现的问题🚨 Critical
|
4 participants
Summary
This PR adds a runnable code review agent prototype under
examples/skills_code_review_agent/and a formalcode-reviewskill underskills/code-review/.The example supports diff / repo / fixture input, structured diff parsing, deterministic review rules, filter governance, controlled script execution, SQLite persistence, and JSON/Markdown report generation.
Included
examples/skills_code_review_agent/skills/code-review/--diff-file,--repo-path, and--fixtureTesting
Covered scenarios: