Skip to content

fix: require PR reviews and enforce admin protection#12

Closed
avrabe wants to merge 1 commit intomainfrom
fix/require-pr-reviews
Closed

fix: require PR reviews and enforce admin protection#12
avrabe wants to merge 1 commit intomainfrom
fix/require-pr-reviews

Conversation

@avrabe
Copy link
Copy Markdown
Contributor

@avrabe avrabe commented Mar 2, 2026

Summary

  • Changes required_pull_request_reviews from null to require 1 approving review with stale review dismissal
  • Enables enforce_admins: true so admins cannot bypass protection rules
  • Fixes the gap where direct pushes to main were allowed despite branch protection showing as active

Root cause

config.yml had required_pull_request_reviews: null, which tells GitHub to not require PR reviews. Other protections (signed commits, status checks, no force push) were active — so the dashboard correctly showed "protected" — but the specific gate that blocks direct pushes was disabled.

Impact

After Temper applies this config, all 26 monitored repos will require:

  • At least 1 approving review before merge
  • Stale reviews dismissed on new pushes
  • Admin compliance (no admin bypass)

Test plan

  • Merge this PR
  • Run /sync-all-repos or wait for Temper to apply the config
  • Verify direct push to main is rejected on any repo
  • Verify PR workflow works (create PR → approve → merge)

🤖 Generated with Claude Code

Branch protection had `required_pull_request_reviews: null` and
`enforce_admins: false`, which allowed direct pushes to main on all
repos despite other protections being active.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@temper-pulseengine temper-pulseengine Bot left a comment

Choose a reason for hiding this comment

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

AI Code Review for PR #12

Summary

  • Changes required_pull_request_reviews from null to require 1 approving review with stale review dismissal.
  • Enables enforce_admins: true so admins cannot bypass protection rules.
  • Fixes the gap where direct pushes to main were allowed despite branch protection showing as active.

[critical] config.yml:31-40 -- The original configuration had required_pull_request_reviews: null, which meant PR reviews were not required. This was corrected by setting it to require 1 approving review and dismissing stale reviews on new pushes. Additionally, enforce_admins: true ensures that only authorized users can bypass protection rules.

Verdict: APPROVE


Reviewed by qwen2.5-coder:3b (local Ollama). Advisory only — may miss issues or report false positives.

Commands
Command Description
/ask <question> Discuss this review — ask questions or disagree with findings
/review-pr Re-run the review from scratch
/review-pr <focus> Re-run with specific instructions (e.g. /review-pr focus on error handling)

@avrabe
Copy link
Copy Markdown
Contributor Author

avrabe commented Mar 2, 2026

Closing — consolidated into a single PR with #11 and CI fixes (lint, coverage, Bazel).

@avrabe avrabe closed this Mar 2, 2026
avrabe added a commit that referenced this pull request Mar 2, 2026
- Require 1 approving review + enforce admin branch protection (config.yml)
- Rewrite dashboard README section to match actual implementation
- Fix 3 no-unused-vars lint errors (dashboard.js, app.js, ai-review.test.js)
- Exclude dashboard.js from coverage (1330 lines at 1% tanks thresholds)
- Add @octokit/auth-app to package.json + Bazel deps
- Fix pnpm transform pattern in jest.config.cjs
- Export config.yml for Bazel test runfiles

Closes #11, closes #12.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
avrabe added a commit that referenced this pull request Mar 2, 2026
* fix: consolidate CI fixes, require PR reviews, and update README

- Require 1 approving review + enforce admin branch protection (config.yml)
- Rewrite dashboard README section to match actual implementation
- Fix 3 no-unused-vars lint errors (dashboard.js, app.js, ai-review.test.js)
- Exclude dashboard.js from coverage (1330 lines at 1% tanks thresholds)
- Add @octokit/auth-app to package.json + Bazel deps
- Fix pnpm transform pattern in jest.config.cjs
- Export config.yml for Bazel test runfiles

Closes #11, closes #12.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: wrap config.yml in js_library for Bazel cross-package data

aspect_rules_js requires cross-package source files to be wrapped in a
js_library target so they can be copied to the output tree.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: add .aspect_rules_js to jest transformIgnorePatterns

Bazel's aspect_rules_js uses node_modules/.aspect_rules_js/<pkg>/
nested layout, same as pnpm's .pnpm — needs the same exception in
the transform pattern so @octokit/auth-app gets babel-transformed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat: include repo and branch context in AI review comments

The review header now shows the full origin and target:
  contributor/temper:`fix/bug` → pulseengine/temper:`main`

This gives immediate context for cross-repo (fork) PRs and makes
it clear which branch is being reviewed against.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat: add self-updating webhook with Rust updater binary

When a push to main is received for the temper repo, spawn a detached
Rust binary that pulls latest code, conditionally runs npm ci, rebuilds
itself if its source changed (falling back to the last working binary on
build failure), and restarts the process via PM2 or SIGTERM.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
@avrabe avrabe deleted the fix/require-pr-reviews branch March 2, 2026 13:51
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.

1 participant