RFC 0001: code-review workflow per-repo customization

[lishuceo]

Summary

Drafts the architecture for letting consumer repos (maker / UrhoX / ...) customize the org-wide reusable code-review.yml without forking it.

Key choice — hybrid load:

• Structural knobs (model / max_turns / budget / confidence / paths) → workflow inputs: with safe defaults
• Free-form project knowledge (stack / gotchas / business rules / extra FP filters) → consumer-side .github/REVIEW.md (already referenced by the prompt today, just nobody writes it)
• Out of scope: prompt_override, structured YAML config file, version tags

Backward compatible: every new input has a default → maker's current uses: ...@main keeps working unchanged.

What's in the draft

• 5-category taxonomy of customization needs and the load-out for each
• Full input list with types/defaults
• .github/REVIEW.md template
• Security model for extra_allowed_tools (explicit allowlist + deny-list + validation step)
• 4-phase rollout plan, each phase independently mergeable
• 5 open questions for org-level decision (REVIEW.md location, structured config, version tags, allowlist maintenance, nudge behavior)

Test plan

• Read RFC and react to the 5 open questions in §8
• Decide if Phase 1 (5 numeric inputs) can proceed independently while §8 is still being discussed

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

此 RFC 草案为 org-wide code-review.yml 设计了 per-repo 定制架构：结构性参数（model、turns、budget、paths）走 workflow inputs:，自由文本项目知识走 .github/REVIEW.md，两者分层配合。之前评审轮次中发现的尾随逗号注入、claude_args 换行消费歧义、@main 版本未标注等问题在 02bc76b 中均已修复，当前草稿整体质量较高。剩余两个 P2 点：paths_focus/paths_skip 的 input description 暗示了"硬过滤"保证而实现是软约束（与 §4.2 不一致），以及数值 input（confidence_min、max_turns 等）缺少范围校验讨论（§6.4 仅覆盖 extra_allowed_tools），建议在正式实施前补充。

Confidence Score: 5/5

仅含文档变更（RFC 草稿），无运行时代码修改，可安全合并。

所有剩余问题均为 P2 建议（description 措辞和数值校验补充），不影响合并；之前的 P1 issue 已全部修复。

docs/rfcs/0001-code-review-customization.md 的 §4.1 input descriptions 和 §6.4 校验覆盖范围

Important Files Changed

Filename	Overview
docs/rfcs/0001-code-review-customization.md	新增 RFC，设计 per-repo 定制架构，含 input 参数化、REVIEW.md 约定、extra_allowed_tools 安全模型（白名单精确匹配 + 字符清洗 + env 传值）及 4 阶段实施计划；主要待改进点是 paths_focus/paths_skip 的 description 措辞暗示了硬过滤保证，与正文软约束说明不一致，以及数值 input 范围校验未覆盖。

Sequence Diagram

sequenceDiagram
    participant C as Consumer Repo
    participant RW as Reusable code-review.yml
    participant VE as validate-extra-tools step
    participant CT as compose-tools step
    participant CL as Claude Code Action

    C->>RW: uses: code-review.yml@main<br/>with: model, max_turns, paths_focus,<br/>paths_skip, extra_allowed_tools, ...
    RW->>VE: inputs.extra_allowed_tools
    alt 含非法字符或不在白名单
        VE-->>RW: exit 1（指向 §6）
    else 通过校验
        VE-->>CT: EXTRA env var（clean）
    end
    CT->>CT: BASE tools + EXTRA（if non-empty）
    CT-->>RW: outputs.tools
    RW->>CL: prompt（含 paths/confidence 指令）<br/>claude_args: model/max-turns/budget<br/>allowedTools: composed list
    CL-->>C: Review comments posted to PR

Loading

_{Reviews (4): Last reviewed commit: "docs(rfc): paths_skip is a strong prompt..." | Re-trigger Greptile}

[claude]

⚠️ Issues Found

3 new findings on this RFC document.

#	Severity	Confidence	Finding
1	🟡 P2	88	§4.2 claims REVIEW.md needs no prompt changes, but §4.3 adds one — and the current prompt references REVIEW.md (root), not .github/REVIEW.md
2	🟡 P2	80	§6 security model for extra_allowed_tools does not address shell metacharacter injection when the input is interpolated into claude_args
3	🟢 P3	85	§7 incorrectly claims max_budget_usd and confidence_min defaults are "consistent with PR #14" — they are new parameters

Overall: Well-structured RFC with a sound hybrid approach (inputs for knobs, REVIEW.md for prose). The 4-phase rollout is pragmatic. The main concerns are an internal inconsistency about whether prompt changes are needed for REVIEW.md discovery, and a gap in the security model around input sanitization before shell interpolation. Both are straightforward to fix before implementation begins.

[claude]

⚠️ Issues Found

3 previously reported issues fixed — all resolved and threads closed.

New findings

#	Severity	Confidence	Finding
1	🟡 P2	88	§4.3 compose-tools step uses ${{ }} interpolation to inject extra_allowed_tools into a shell script — the exact script injection pattern §6.1 warns against. Should use env: block instead.

Overall assessment

The revision is a substantial improvement. All three issues from the previous review (§4.2 contradiction, §6 injection gap, §7 factual inaccuracy) have been thoroughly addressed with clear explanations and proper fixes. The new §6.1–6.5 security model is well-structured.

The one remaining issue is that the compose-tools code example in §4.3 still demonstrates the unsafe ${{ }} interpolation pattern in a run: block — ironic given that §6.1 exists specifically to defend against this class of attack. Using env: to pass the input as a shell variable is the standard GitHub-recommended mitigation and would make the example consistent with the RFC's own security guidance.

[claude]

✅ Approved (with minor comment)

4 previously reported issues fixed — all prior threads resolved across 3 revision commits.

New findings

Sev	Conf	Summary
🟡 P2	82	paths_skip described as "硬过滤 / 绝不 review" but implemented as prompt-level text — not a hard guarantee. Consider pre-filtering the diff or adjusting the claim.

Overall assessment

The RFC is in excellent shape after three rounds of revisions. The security model (§6) is thorough — character sanitization, exact-match allowlisting, env-based input passing, and defense-in-depth are all well-reasoned. The hybrid design (structural knobs as inputs, free-form knowledge as REVIEW.md) is clean and backward-compatible. The one remaining concern is the semantic gap between calling paths_skip a "hard filter" while implementing it as a prompt instruction. This is worth clarifying before implementation, especially if consumers might rely on it for security-sensitive path exclusion.

[claude]

🟢 P3 · Confidence 92 · Incorrect cross-reference: §10 alternative D vs E

This line says "参考 §10 的备选方案 D" but in §10, alternative D is prompt_override (完全自定义 prompt). The pre-process diff hard filter is alternative E (added in commit 741f622).

Suggested change

	- 两者强度不同但**都不应当作安全边界使用**：不要依赖 `paths_skip` 来防止 Claude 看到敏感文件（如 credentials）。敏感文件本来就不该在 diff 里，应当靠 `.gitignore` / `git-secrets` / pre-commit hook 在更上游拦掉。如果将来真有"必须物理剔除路径再喂给 Claude"的场景，参考 §10 的备选方案 D（pre-process diff，未在本 RFC 范围内实现）。
	- 两者强度不同但**都不应当作安全边界使用**：不要依赖 `paths_skip` 来防止 Claude 看到敏感文件（如 credentials）。敏感文件本来就不该在 diff 里，应当靠 `.gitignore` / `git-secrets` / pre-commit hook 在更上游拦掉。如果将来真有"必须物理剔除路径再喂给 Claude"的场景，参考 §10 的备选方案 E（pre-process diff，未在本 RFC 范围内实现）。延后到具体场景出现时再评估。

[claude]

✅ Approved

5 previously reported issues fixed — all prior review threads are resolved.

New findings

#	Severity	Confidence	Description
1	🟢 P3	92	§4.2 cross-references "备选方案 D" but the pre-process diff alternative is E (D is prompt_override)

Overall assessment

The RFC is in good shape. All previous review feedback (§4.2/§4.3 contradiction, shell metacharacter injection, paths_skip "硬过滤" misnomer, unsafe ${{ }} interpolation, factual inaccuracy in §7) has been addressed thoroughly across the four follow-up commits. The security model in §6 is now well-layered (character sanitization → exact allowlist matching → env-based passing). The only remaining issue is a minor cross-reference typo with a suggested fix.

🤖 Generated with Claude Code