Skip to content

Require CODEOWNERS approval on main so the review agent's requires-manual-review label actually blocks bot auto-merge #1850

Description

@fullsend-ai-retro

What happened

On PR #1849 (a Renovate github/codeql-action v4.37.9 → v4.38.0 SHA repin), the fullsend-ai-review agent emitted medium [protected-path] findings for the two touched .github/workflows/*.yml files, stating "Human approval is always required for protected-path changes." post-review.sh attached the requires-manual-review label at 02:07:29Z.

renovate[bot] then auto-merged the PR at 05:37:24Z with:

  • reviewDecision: REVIEW_REQUIRED
  • reviews: [] (no human approval, no formal review at all)
  • the requires-manual-review label still attached
  • merged_by: renovate[bot], merge_method: squash

The repo does have .github/CODEOWNERS mapping * to @conforma/devs, so the ownership signal exists — but the fact that Renovate merged over REVIEW_REQUIRED with no approvals indicates the branch-protection rule on main does not require CODEOWNERS (or any) approving review before merge (or Renovate is bypassing it).

What could go better

The review agent did its job well — verdicts were accurate, the SHA was independently verified against the upstream v4.38.0 tag, and the governance finding was correctly triggered. The failure is at the enforcement layer: the label is advisory, not a merge gate. Two independent fixes are needed and each covers a different failure mode:

  1. Agent layer (already tracked as fullsend-ai/fullsend#6018): have the review agent post a formal REQUEST_CHANGES review alongside the label so GitHub's native review-state gate engages. This retro is fresh evidence for that issue; not re-filing.
  2. Repo layer (this proposal): even a REQUEST_CHANGES review is only a merge blocker if branch protection on main requires approving reviews. Today it apparently doesn't — otherwise Renovate could not have merged with reviewDecision: REVIEW_REQUIRED. Fixing branch protection is a repo-owner decision that can't be made at the agent layer.

Confidence is high that the branch-protection rule is missing/insufficient (the merge event itself is the evidence). I cannot see the exact ruleset with the retro token (gh api .../branches/main/protection → 403), so the specific rule to toggle is something a maintainer needs to confirm.

Caveat: the repo may have deliberately chosen to let Renovate auto-merge governance-adjacent changes as a velocity trade-off. If so, close this as intended-behavior — the finding still stands that in that world, the agent's protected-path warning is purely informational.

Proposed change

In conforma/policy branch-protection settings for main (Settings → Branches → Branch protection rules → main):

  • Enable "Require a pull request before merging".
  • Enable "Require approvals" (at least 1).
  • Enable "Require review from Code Owners" — CODEOWNERS already exists at .github/CODEOWNERS.
  • Ensure "Allow specified actors to bypass required pull requests" does not include renovate[bot] (or exclude bots from any bypass list).
  • Confirm "Do not allow bypassing the above settings" is enabled (or that admins/apps are not silently bypassing).

Optional companion in renovate.json: for updates touching .github/**, disable auto-merge (e.g. add a packageRules entry with matchFileNames: [".github/**"], automerge: false) as belt-and-braces so Renovate itself defers to human approval on workflow-file changes.

No change to the review agent is proposed here — that side is tracked upstream at fullsend-ai/fullsend#6018.

Validation criteria

For the next 5 Renovate PRs that touch .github/** in conforma/policy:

  1. The fullsend-ai-review agent applies the requires-manual-review label (unchanged behavior).
  2. Renovate's auto-merge does not merge the PR until an approving review is submitted by a member of @conforma/devs.
  3. gh pr view <N> --json reviewDecision shows APPROVED (not REVIEW_REQUIRED) at the moment mergedAt is set.

Spot-check by re-running the exact scenario: the next github/codeql-action bump should sit open until a human on @conforma/devs reviews it, rather than merging in ~3.5h with zero human interaction as PR #1849 did.


Generated by retro agent from #1849

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions