Skip to content

Add policy-bot configuration file#8659

Merged
robert3005 merged 1 commit into
developfrom
rk/policybot
Jul 6, 2026
Merged

Add policy-bot configuration file#8659
robert3005 merged 1 commit into
developfrom
rk/policybot

Conversation

@robert3005

Copy link
Copy Markdown
Contributor

Github approval logic is not flexible enough to not be super noisy or allow
writes without allowing approvals. Policy-bot will let us have arbitrary
approval configuration

Signed-off-by: Robert Kruszewski <github@robertk.io>
@robert3005 robert3005 requested a review from a team July 6, 2026 13:06
@0ax1

0ax1 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

@claude review

@0ax1 0ax1 added the changelog/chore A trivial change label Jul 6, 2026
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Claude finished @0ax1's task in 2m 27s —— View job


Claude PR Review — Add policy-bot configuration

  • Read policy.yml
  • Analyze policy-bot semantics (approval/disapproval logic)
  • Check for correctness / security / config gotchas
  • Post review

This adds a policy-bot config to replace the hand-rolled .github/workflows/approvals.yml approval-count check. The overall structure is sound and the rule-name references all resolve correctly. A few things worth confirming before merge.


🔴 Blocking-ish: file name must match the policy-bot server config

policy-bot's default policy path is .policy.yml (leading dot). This PR adds policy.yml at the repo root. Unless the policy-bot deployment's options.policy_path is explicitly set to policy.yml, this file will be silently ignored and no rules will apply. Please confirm the server config points at policy.yml (or rename to .policy.yml).

🟠 invalidate_on_push: false weakens the AI-author gate

approval_defaults sets invalidate_on_push: false, and since defaults merge into every rule, it also applies to claude or codex authored pull requests have two committer approvals. This means once an AI-authored PR has its 2 approvals, additional commits can be pushed afterward without re-review. Given the whole point of the 2-approval rule is to gate AI-generated changes, non-invalidation partially undermines it.

The renovate rule is protected — only_has_contributors_in: [renovate[bot]] re-checks authorship on every evaluation — but the committer rule and the claude/codex rule are not. If "writes without re-approval" is intentional per the PR description, no change needed; otherwise consider setting invalidate_on_push: true on the claude/codex rule specifically. (policy.yml:45, :74-85)

🟡 allow_non_author_contributor: true + the 2-approval rule

With allow_author: false but allow_non_author_contributor: true (policy.yml:43-44), a committer who pushes a fixup commit onto an AI-authored PR becomes a contributor and can still approve it. That erodes the independence of the "2 committer approvals" intent for exactly the PRs it's meant to guard. Worth a conscious decision.

🟡 Verify the renovate-approve[bot] login

The renovate rule requires approval from renovate-approve[bot] or a committer (policy.yml:68-72). If your Renovate setup auto-approves under a different app login (e.g. renovate[bot] itself, or an org-specific app), this user clause will never match and only the committer fallback will work. Please confirm the actual approver login.

🟢 Minor: 👍 / +1 overlap between approval and disapproval-revoke

👍/+1/:+1: are used both as approval methods (policy.yml:35-37) and as disapproval revoke comments (policy.yml:21-24). policy-bot treats approval and disapproval as separate subsystems so this works, but a single 👍 both approves and revokes a disapproval — slightly surprising to a reader. Fine to leave; flagging for awareness.

✅ Things that look correct

  • Top-level policy.approval AND-list is right: normal PRs need 1 committer approval; the if-gated claude/codex rule is skipped for humans and requires 2 for AI authors — the two ANDed correctly yield 2 committer approvals for AI PRs. (policy.yml:5-10)
  • Disabling body_patterns / comment_patterns while keeping explicit comments + github_review matches the stated goal of not treating PR-body text as approval. (policy.yml:32-40)
  • All three rule names in policy.approval exactly match approval_rules[].name.
  • Bot logins (vortex-claude[bot], claude-code[bot]) are consistent with .github/workflows/fuzzer-fix-automation.yml.
  • SPDX license headers present.

❓ Follow-up

Is .github/workflows/approvals.yml going to be removed once policy-bot is enforcing? Keeping both risks redundant/conflicting required checks (note the old workflow only enforces approval counts for bot authors, not humans — policy-bot changes that to require 1 approval for everyone, which is stricter).


Config-only change — no Rust/Python checks run. I attempted yamllint/python -c 'yaml.safe_load' to validate formatting but the commands were blocked in this sandbox; the file parses as valid YAML by inspection.
· rk/policybot

Comment thread policy.yml
Comment on lines +8 to +10
- a vortex committer has approved
- an untouched renovate pull request has an allowed approval
- claude or codex authored pull requests have two committer approvals

@connortsui20 connortsui20 Jul 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is the different tabs here on purpose?

@codspeed-hq

codspeed-hq Bot commented Jul 6, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

⚡ 4 improved benchmarks
❌ 2 regressed benchmarks
✅ 1601 untouched benchmarks
⏩ 42 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation slice_empty_vortex 310.3 ns 368.6 ns -15.83%
Simulation compact_sliced[(4096, 90)] 750 ns 837.5 ns -10.45%
Simulation bitwise_not_vortex_buffer_mut[128] 273.6 ns 215.3 ns +27.1%
Simulation bitwise_not_vortex_buffer_mut[1024] 333.9 ns 275.6 ns +21.17%
Simulation bitwise_not_vortex_buffer_mut[2048] 427.8 ns 369.4 ns +15.79%
Simulation chunked_varbinview_into_canonical[(100, 100)] 306.9 µs 272.1 µs +12.8%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing rk/policybot (bd52929) with develop (86c250a)2

Open in CodSpeed

Footnotes

  1. 42 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on develop (570f7ed) during the generation of this report, so 86c250a was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@robert3005 robert3005 merged commit 4959939 into develop Jul 6, 2026
92 of 95 checks passed
@robert3005 robert3005 deleted the rk/policybot branch July 6, 2026 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/chore A trivial change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants