Skip to content

Feat/code review agent 92 clean#201

Open
wzxsdf wants to merge 5 commits into
trpc-group:mainfrom
wzxsdf:feat/code-review-agent-92-clean
Open

Feat/code review agent 92 clean#201
wzxsdf wants to merge 5 commits into
trpc-group:mainfrom
wzxsdf:feat/code-review-agent-92-clean

Conversation

@wzxsdf

@wzxsdf wzxsdf commented Jul 16, 2026

Copy link
Copy Markdown

概述

实现一个端到端、可验证的自动代码评审 Agent 原型。输入 git diff / 文件列表 / git 工作区变更,Agent 通过 code-review Skill 加载规则与脚本,经 Filter 治理后进入沙箱执行必要检查,把发现的问题按严重级别 / 文件 / 行号 / 证据 / 修复建议结构化输出,并将审查任务、拦截记录、监控摘要与结果写入 SQLite。

核心设计取舍:确定性规则保检出下限 + LLM 提准确率上限。规则层(正则 + AST/taint + 外部扫描器)保证 dry-run 可跑、可测;LLM 层做降噪(剔除误报)与补召回(补语义问题),可降级、可关闭。

架构

输入(diff/文件列表/git工作区)
 → 解析(解析期脱敏,规则脚本零明文)
 → 规则召回(逐行正则+LITERAL_ARGUMENT防误报 / hunk跨行 / Python AST taint / semgrep·bandit)
 → LLM增强(降噪二分类剔除误报 + 低置信补召回;无Key走预录制)
 → 四元组去重(file,line,category,rule_id) + 三桶路由(findings/warnings/needs_review)
 → Filter前置治理(CommandPolicy + SDK BaseFilter;deny/review不进沙箱)
 → 沙箱执行(Fake默认 / Local(dev) / Container / Cube;bounded_int单向收紧)
 → 脱敏闭环(检/脱共享正则 + Shannon熵;解析/finding/stdout/落库/LLM输入)
 → SQLite七表落库(schema_migrations真迁移)
 → 报告(JSON / Markdown八段式 / SARIF v2.1.0)

关键设计 / 创新点

  1. LLM 规则增强层:对规则召回的 finding 做结构化二分类(true/false positive)剔除误报 + 对低置信语义项补召回;双模式(真实 LLM / 预录制 fixture),无 API Key 也能跑通全链路。
  2. 隐藏样本量化评测:公开 8 + 隐藏 12 fixture,实例级 P/R/F1(非集合级,避免夸大召回)。
  3. 四元组去重(file, line, category, rule_id) 保留同类不同子问题的证据(优于三元组)。
  4. 检 / 脱同步:检测规则与脱敏共享同一正则集(SECRET_KV_KEYS 单一真相源),杜绝“检出未脱 / 脱了未检”。
  5. 沙箱永不抛:超时 / 失败捕获为 partial result,不中断静态评审。

验收对照(issue #92 八条)

# 验收 状态 说明
1 公开 8 样本可运行出报告 evaluate.py 跑全部 fixture,生成 JSON/MD/SARIF
2 隐藏样本检出≥80% 误报≤15% ✅* 召回 0.895 / 脱敏 0.983 / findings 桶误报 0.0(见实测)
3 DB 完整记录 + 按 task 查 七表 + get_task_details 聚合
4 沙箱超时/输出限制/失败不崩 timeout + bounded output + 异常捕获
5 脱敏≥95% 无明文 0.983,报告/DB/LLM 输入均脱敏
6 dry-run ≤2min 无 Key 走预录制
7 Filter 前置 deny 不进沙箱 BaseFilter._before 短路 + pipeline allow 才进沙箱
8 报告八段式 + 可执行修复 findings/warnings/needs_review/filter/sandbox/monitoring/conclusion + recommendation

使用方法

cd examples/skills_code_review_agent

# 默认 fake 沙箱(无 Docker 也能跑)
PYTHONUTF8=1 python run_review.py --diff-file fixtures/diffs/security.diff

# 真实 LLM 模式(需配置 .env 的 TRPC_AGENT_API_KEY/BASE_URL/MODEL_NAME)
PYTHONUTF8=1 python evaluate.py --llm

测试

  • pytest tests/ -v:187 项全通过
  • python evaluate.py:量化评测
  • yapf + flake8(max-line-length=120)全绿

文件结构

  • examples/skills_code_review_agent/:agent(pipeline/rule_engine/ast_analyzer/redaction/dedup/llm_layer/report/telemetry)/ storage(七表+迁移)/ sandbox(四后端)/ filters / fixtures(8公开+12隐藏)/ tests / CLI + evaluate + README
  • skills/code-review/:SKILL.md + 6 类规则 + 沙箱脚本

Closes #92

wzxsdf added 5 commits July 17, 2026 02:11
端到端 CR Agent: 解析→规则(正则+AST/taint)→LLM增强(降噪+补召回)→四元组去重→Filter前置→沙箱(fake/local/container/cube)→脱敏→SQLite七表落库→JSON/MD/SARIF报告

Closes trpc-group#92
…trpc-group#92)

修复 PR trpc-group#200 reviewer(CongkeChen)发现的 Critical bug:
- DENOISING_PROMPT 要求 LLM 返回 "verdict":"TP|FP"(TP=真问题, FP=误报)
- 原 _apply_verdicts 只在 verdict == "false_positive" 时剔除
- 真 LLM 返回 "FP" 不匹配 "false_positive" → 降噪失效
- 现在兼容多种格式:FP, false_positive, false positive, False Positive, 误报

新增测试:test_real_mode_fp_verdict_compatibility() 验证真 LLM 返回 "FP" 格式时正确剔除误报
…c-group#200 review)

- Critical 2: 修复 Cube 沙箱超时异常捕获,优先捕获 subprocess.TimeoutExpired
- 隐患1: 修复 Filter 预算 off-by-one,call_index >= max_sandbox_runs 时 deny
- 隐患2: 修复 Filter 子串误匹配,路径使用边界匹配避免 .environment 命中 .env
- 隐患3: 修复沙箱脚本未落盘,pipeline 在执行前把脚本复制到 workspace
- 隐患4: 修复存储 save 与 start_task 断链,pipeline 在开始时调用 start_task
- 补充严格测试:Cube 超时断言 status==timeout、Filter off-by-one 和边界匹配测试
- 回退 .flake8 的 ignore 配置,移除 E126, E128(恢复为 ignore = E402, W503)
- 重构 llm_layer.py 两处 create() 调用,提取 messages 变量解决 continuation line 缩进问题
- _call_llm_with_openai_client() + _call_llm_with_openai_client_supplementary()
- 验证:flake8 零错误(无 E126/E128) + pytest 全绿
@wzxsdf

wzxsdf commented Jul 16, 2026

Copy link
Copy Markdown
Author

#200 Critical 修复

LLM 降噪 verdict 不一致(llm_layer.py _apply_verdicts):normalize 后兼容 false_positive/FP/false/误报(c547448)。补充了真模式返回 "FP" 的测试——此前只测 false_positive 才漏掉,正是您指出的关键。
Cube 超时异常类型(cube.py):except TimeoutError → except (subprocess.TimeoutExpired, TimeoutError),超时正确返回 status="timeout"(非 failed)(e43b0de)。补严格断言 assert status == "timeout"。
稳定性隐患修复(e43b0de)

Filter 预算 off-by-one:> → >=
Filter 子串误匹配:路径/命令边界精确匹配(.environment 不再命中 .env)
沙箱脚本未落盘:pipeline 执行前 shutil.copy 脚本到 workspace
存储 save/start_task 断链:pipeline 先 start_task 再 save

@CongkeChen

Copy link
Copy Markdown
Contributor

AI Code Review

我已经有了足够的上下文。让我来整理一下审查意见。

发现的问题

🚨 Critical

  • examples/skills_code_review_agent/sandbox/local.py:52 & sandbox/cube.py (subprocess.run 周围):沙箱执行未走 Filter 治理,且 Local/Cube 后端直接用 subprocess.run(["python", script_path]) 执行从 diff 复制的脚本,绕过了 filters/policy.py 的命令策略评估(policy 只在 pipeline 里对固定的 python static_review 评估,对实际脚本内容/路径无校验)。本地/Cube 后端真实执行任意脚本内容且 cwd=workspace 可读写,存在任意代码执行与路径穿越风险;建议 Local/Cube 在执行前对脚本路径做白名单/逃逸校验,并强制只读挂载(Container 的 :ro 在 Local/Cube 未对应)。仅 Container 有隔离参数,Local/Cube 等同无沙箱执行不可信脚本。

  • examples/skills_code_review_agent/sandbox/local.py:84:subprocess.TimeoutExpiredtext=True 时其 e.stdout/e.stderrstr 而非 bytes,调用 .decode('utf-8', ...) 会抛 AttributeError,导致超时分支本身崩溃(违反“永不抛”原则),超时场景下不会返回 timeout 结果而是被外层 except Exception 转成 failed。修复:去掉 .decode 或按 text 分支处理。

  • examples/skills_code_review_agent/agent/pipeline.py:106 & :233:run_review 在 try/except 中创建 store,若 ReviewStore() 构造成功但 start_task 失败,store 变量会被异常销毁后的降级路径跳过;而 save:233 无条件调用 store.save(report)。当 start_task 抛异常时(task_id 走 UUID 降级分支),store 可能未定义或仍指向失败实例,save 会再次抛异常被吞掉但 review_tasks 无记录,导致落库数据孤儿(外键 task_id 不存在于 review_tasks,违反 schema 外键约束被静默丢弃)。建议在降级分支重新构造可用 store 或显式 store=None 并在 save 前判空。

⚠️ Warning

  • examples/skills_code_review_agent/agent/ast_analyzer.py:278 & :285:每个 hunk 单独 ast.parse(code_block) 后新建 _Visitor,但 _Visitor.tainted 是实例级状态、跨 hunk 不复用;同时拼接的 code_block 仅含 added 行(去掉 + 前缀),多删行/上下文缺失导致函数体常被截断、visit_FunctionDef 标记的参数污点在该 hunk 内有效但跨 hunk 丢失,污点传播大量漏报。行号映射 hunk.added[line_no-1] 还会因 AST 重排导致错位。影响:安全规则召回不可靠。建议整文件级 AST 解析或至少跨 hunk 保留污点集合。

  • examples/skills_code_review_agent/agent/llm_layer.py:163 & :360:response.choices[0].message.content 未做空值/长度校验,当 API 返回空 choices(限流/内容过滤)时抛 IndexError,被外层 except Exception 当作普通失败降级,所有降噪裁决丢失。建议先判 if response.choices

  • examples/skills_code_review_agent/agent/llm_layer.py:175 & :371:重试条件用 "5" in error_msg 子串匹配 5xx,会误命中任何含数字 "5" 的错误信息(如 "500 tokens"、"line 5")触发无意义重试,且 "timeout" 匹配同样过宽。建议用更精确的状态码/异常类型判定。

  • examples/skills_code_review_agent/agent/redaction.py:127 & :166:redact_textif "[REDACTED_" in text: return text, 0 做幂等短路,但 redact_finding 对 evidence/title/recommendation 分别调用 redact_text,若某字段本身不含占位符但其它字段已脱敏,短路逻辑只作用于单字段;更关键的是 contains_unredacted_secret 用原始 secret in text 做明文匹配,而 redact_text 对 KV 类只替换值不替换键名,残留的键名(如 password=)+ 未命中正则的值仍可能漏脱敏。建议脱敏后用 contains_unredacted_secret 回扫校验。

  • examples/skills_code_review_agent/filters/policy.py:61:forbidden_paths"..",边界正则 (^|[/\s"'])\.\.($|[/\s"']) 要求 .. 两侧必须是边界字符,对 path/../etc 这类穿越能命中,但对 ..fooa..b 因边界限制不会误报,逻辑上可接受;但 ".env" 同样要求两侧边界,source .envrc 不会命中(正确),而 cat .environment.env 后跟 ironment 不命中也正确。整体边界设计 OK,但 high_risk_commands"curl"/"wget" 用边界匹配后,policy.jsonallowed_executables 字段在 evaluate 中完全未使用——白名单可执行文件未生效,任何命令词只要不在 high_risk 列表即 allow。建议在 allow 前校验命令首词 ∈ allowed_executables。

  • examples/skills_code_review_agent/tests/test_sandbox.py:61:test_force_secret_output 断言 "sk-leaked-secret" in result.stdout_redacted,但 FakeSandbox 实现里已对 stdout 调 redact_text 脱敏为 out [REDACTED_SK],明文 sk-leaked-secret 不应再出现。该断言与实现矛盾,测试要么无法通过、要么在不通过时掩盖了脱敏失效。建议断言改为 "REDACTED" in result.stdout_redacted and "sk-leaked-secret" not in ...

  • examples/skills_code_review_agent/sandbox/container.py:140:_bounded_int("SANDBOX_MEMORY_MB", 512, 1024) 的返回值未被使用(仅做上限校验),且 host_config 里仍写死 "512m",环境变量对内存限制实际无效;超限抛 ValueError 会被外层 except Exception 转成 failed。属于配置项声明但未接线,维护时易误导。建议把返回值传入实际 docker 参数或删除未用调用。

  • examples/skills_code_review_agent/storage/store.py:111-113:saveexceptconn.rollback(); raise e——sqlite3BEGIN TRANSACTION 后若某条 INSERT 触发外键/UNIQUE 约束抛异常,rollback 正确;但 _delete_task_dataf"DELETE FROM {table}" 拼接表名,table 来自硬编码列表安全,可接受。真正风险:_insert_findingsINSERT OR IGNORE 静默吞掉 UNIQUE 冲突,幂等 save 语义下重复 save 同一 task 会丢新增 finding 而无告警。建议至少记录被忽略的条数。

💡 Suggestion

  • examples/skills_code_review_agent/agent/pipeline.py:201:sandbox_duration_ms 直接用 t_sandbox = int((time.time() - t0)*1000),与 total_duration_ms 取值方式相同,二者永远相等,未真实反映沙箱耗时。建议在沙箱循环内累加各 run.duration_ms

  • examples/skills_code_review_agent/agent/llm_layer.py:496-507:dry_run 分支把 recorded_verdicts 的 key 按 : 切 3 段,但 recorded_verdicts 的 key 形如 "SECRET001:test.py:10",当文件路径本身含 :(Windows 盘符或某些 fixture 命名)时 split(":") 会切错,line 解析失败被静默跳过导致降噪失效。建议用 rsplit(":", 1) 或固定结构解析。

总结

存在 3 处 Critical:Local/Cube 沙箱绕过 Filter 直接执行不可信脚本、Local 超时分支对 str 调 .decode 必崩、pipeline 在 start_task 失败降级后仍无条件 store.save 导致外键孤儿/落库静默失败。另有脱敏回扫缺失、LLM 响应未判空、白名单未生效、测试断言与实现矛盾等 Warning 需修复。建议优先修 Critical 后再合入。

测试建议

  • 补充 LocalSandbox 超时用例,断言返回 status="timeout" 且不抛 AttributeError(覆盖 text=Truee.stdout 为 str 的路径)。
  • 补充 run_reviewstore.start_task 抛异常时 store.save 行为的用例,断言不产生外键孤儿、不抛未捕获异常。
  • 补充 FakeSandbox.force_secret_output 脱敏断言改为校验明文不在输出中。

duration_ms = int((time.time() - start_time) * 1000)

# 尝试获取部分输出
stdout = e.stdout.decode('utf-8', errors='ignore') if e.stdout else ""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

超时分支对 str 调 .decode 必崩

subprocess.run 使用 text=True 时 e.stdout/e.stderr 为 str,调用 .decode('utf-8') 会抛 AttributeError,使超时分支本身崩溃并被外层 except 转为 failed,违反“永不抛”原则。修复:去掉 .decode 或按 text 分支处理。


# 9. 落库(内含脱敏,使用已存在的 task_id)
try:
store.save(report)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

start_task 失败降级后仍无条件 store.save 致外键孤儿

run_review 在 try/except 中创建 store,若 start_task 失败走 UUID 降级,store 可能未定义或仍指向失败实例,:233 又无条件调用 store.save(report),导致 review_tasks 无记录时落库数据被外键约束静默丢弃产生孤儿。建议降级分支重新构造可用 store 或显式置 None 并在 save 前判空。

@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main        #201   +/-   ##
==========================================
  Coverage        ?   87.54280%           
==========================================
  Files           ?         467           
  Lines           ?       44103           
  Branches        ?           0           
==========================================
  Hits            ?       38609           
  Misses          ?        5494           
  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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

基于 Skills + 沙箱 + 数据库存储构建自动代码评审 Agent

2 participants