Skip to content

ADR-0008: Automate code reviews#1291

Open
jeremyeder wants to merge 1 commit intomainfrom
adr-0008-automate-code-reviews
Open

ADR-0008: Automate code reviews#1291
jeremyeder wants to merge 1 commit intomainfrom
adr-0008-automate-code-reviews

Conversation

@jeremyeder
Copy link
Copy Markdown
Contributor

@jeremyeder jeremyeder commented Apr 11, 2026

Summary

  • Proposes replacing human code reviews with an automated inner-loop review that checks PRs against GPS governance docs + repo conventions (CLAUDE.md, constitution, BOOKMARKS.md)
  • Introduces ambient-code:self-reviewed label — machine-applied when inner-loop review passes, triggers Mergify to merge without human approval
  • Reconfigures Mergify with auto-rebase, priority queues (urgent/self-reviewed/default), and stale draft cleanup

Test plan

  • Team reviews and discusses the proposal
  • Create ambient-code:self-reviewed label
  • Build inner-loop review function (skill/hook/GHA)
  • Deploy updated .mergify.yml
  • Enable branch protection on main
  • Validate: PR age at merge, revert rate, open PR count

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Updated internal development process documentation regarding code review automation workflows.

…view + Mergify

Replaces human code review with an automated confidence stack: CI checks,
inner-loop review (Amber/CodeRabbit/Codex against GPS governance + repo
conventions), and a reconfigured Mergify queue that merges on label +
CI green without requiring human approval.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 11, 2026

📝 Walkthrough

Walkthrough

A new ADR document is added that describes an automated code review workflow. It outlines a PR lifecycle integrating CI checks, review orchestration with multiple review agents, governance document validation, conditional auto-labeling, and Mergify-based auto-rebasing and auto-merging for eligible PRs.

Changes

Cohort / File(s) Summary
Automated Code Review ADR
docs/internal/adr/0008-automate-code-reviews.md
New ADR documenting shift from human-gated to automated code review. Describes inner-loop orchestration of review agents (Amber, CodeRabbit, Codex), governance validation against GPS and repository conventions (CLAUDE.md, BOOKMARKS.md, constitution), conditional ambient-code:self-reviewed labeling, and Mergify configuration for auto-rebase and squash-merge workflows with queue-based rules for urgent, self-reviewed, and default PRs.

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Security And Secret Handling ❌ Error ADR-0008 contains critical authorization gaps: missing GPS API authentication spec, Mergify lacks inner-loop-review status check, and write-access escape hatch bypasses enforcement. Document GPS API auth mechanism, add required check-success=inner-loop-review to Mergify conditions, restrict label application to service account only, remove label on pull_request.synchronize.
Title check ⚠️ Warning The title does not follow Conventional Commits format; it lacks the required 'type(scope): description' structure (e.g., 'docs(adr): automate code reviews'). Reformat the title to follow Conventional Commits: 'docs(adr): automate code reviews with inner-loop review and mergify queues'.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Performance And Algorithmic Complexity ✅ Passed PR adds only ADR documentation in markdown format describing organizational processes and governance decisions without introducing executable code, algorithms, loops, I/O operations, or data structures.
Kubernetes Resource Safety ✅ Passed Kubernetes Resource Safety check not applicable; PR only adds markdown ADR document with no Kubernetes manifests or resource definitions.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch adr-0008-automate-code-reviews
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch adr-0008-automate-code-reviews

Comment @coderabbitai help to get the list of available commands and usage tips.

@jeremyeder
Copy link
Copy Markdown
Contributor Author

@Gkrumbach07 @markturansky

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/internal/adr/0008-automate-code-reviews.md`:
- Around line 45-46: The ADR lets users apply the ambient-code:self-reviewed and
hotfix labels to bypass merge-queue checks without proving the inner-loop ran;
fix by requiring a concrete inner-loop status check or binding before
auto-merge: update the queue conditions (the mergify rules referenced in the
ADR) to include a required status/context (e.g., inner-loop CI/status name) or
add automation that removes/revalidates ambient-code:self-reviewed on push
(synchronize event) so labels alone cannot unlock auto-merge for a different
commit; target the label-handling logic and queue condition blocks
(ambient-code:self-reviewed, hotfix, inner-loop review) in the proposed config.
- Around line 35-36: The ADR omits revalidation semantics for the
ambient-code:self-reviewed label; update the document to state that the
inner-loop review must run on pull_request.synchronize and that the review
action will remove the ambient-code:self-reviewed label if governance checks
fail on new commits, and conversely re-apply it when they pass; also update the
Mergify queue config description (currently referencing check-success=*) to
require a governance status/check (or the label absence) as a merge condition so
merges cannot happen without a fresh passing governance check.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5e786730-98c9-49f2-ac9e-f8b21259e9f3

📥 Commits

Reviewing files that changed from the base of the PR and between 5e5f584 and 42dd69d.

📒 Files selected for processing (1)
  • docs/internal/adr/0008-automate-code-reviews.md

Comment on lines +35 to +36
An automated inner-loop review checks each PR against repo and org governance. When it passes, the `ambient-code:self-reviewed` label is applied. When that label is present and CI passes, Mergify merges the PR with no human approval required.

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.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Find automation that manages ambient-code:self-reviewed labels =="
rg -n -C3 'ambient-code:self-reviewed|add-label|remove-label|gh pr edit|pull_request_target|pull_request' .github/workflows docs/internal/adr/0008-automate-code-reviews.md || true

echo
echo "== Verify pull_request triggers include synchronize for inner-loop review =="
rg -n -C5 'on:\s*$|pull_request:|types:|synchronize|labeled|unlabeled' .github/workflows/*.yml .github/workflows/*.yaml 2>/dev/null || true

echo
echo "== Verify any explicit rule that removes label on new commits =="
rg -n -C3 'remove.*ambient-code:self-reviewed|unlabel|labels.*self-reviewed|synchronize.*self-reviewed' .github/workflows .mergify.yml docs/internal/adr/0008-automate-code-reviews.md || true

Repository: ambient-code/platform

Length of output: 23018


🏁 Script executed:

cat -n docs/internal/adr/0008-automate-code-reviews.md | head -50

Repository: ambient-code/platform

Length of output: 3504


🏁 Script executed:

rg -n 'mergify|Mergify' --type yaml --type md .github .docs

Repository: ambient-code/platform

Length of output: 110


🏁 Script executed:

find . -name '.mergify.yml' -o -name 'mergify.yml' -o -name 'mergify.yaml' 2>/dev/null

Repository: ambient-code/platform

Length of output: 79


🏁 Script executed:

cat -n .mergify.yml

Repository: ambient-code/platform

Length of output: 1314


🏁 Script executed:

rg -l 'ambient-code:self-reviewed' .github/workflows

Repository: ambient-code/platform

Length of output: 47


🏁 Script executed:

cat -n .github/workflows/sdd-preflight.yml

Repository: ambient-code/platform

Length of output: 7593


🏁 Script executed:

rg -i 'self-reviewed|self_reviewed' --type yaml --type md --type sh

Repository: ambient-code/platform

Length of output: 2135


🏁 Script executed:

git log --oneline --all -- '.mergify.yml' | head -5

Repository: ambient-code/platform

Length of output: 382


🏁 Script executed:

cat -n docs/internal/adr/0008-automate-code-reviews.md | sed -n '35,100p'

Repository: ambient-code/platform

Length of output: 3502


🏁 Script executed:

cat -n docs/internal/adr/0008-automate-code-reviews.md | tail -80

Repository: ambient-code/platform

Length of output: 2564


🏁 Script executed:

rg -l 'inner-loop|inner_loop|orchestrate.*review|ambient-code.*review' .github/workflows --type yaml

Repository: ambient-code/platform

Length of output: 141


🏁 Script executed:

rg -n 'apply.*label\|add.*label\|label.*ambient' .github/workflows -A3 -B3 --type yaml

Repository: ambient-code/platform

Length of output: 47


🏁 Script executed:

cat -n docs/internal/adr/0008-automate-code-reviews.md | sed -n '100,150p'

Repository: ambient-code/platform

Length of output: 2015


Design gap: label staleness on PR updates not addressed.

The proposed ambient-code:self-reviewed flow (lines 35–43, 51–81) lacks revalidation semantics. The diagram shows label applied once, then Mergify picks it up—but doesn't specify whether the inner-loop review runs on pull_request.synchronize or removes the label if governance fails after new commits. Without explicit removal/recomputation on code changes, stale approval carries forward until merge. Clarify: does the inner-loop review trigger on synchronize, and does it remove the label if governance fails?

Mergify's proposed queue config (Appendix A, line 137) only verifies CI status (check-success=*), not governance re-evaluation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/internal/adr/0008-automate-code-reviews.md` around lines 35 - 36, The
ADR omits revalidation semantics for the ambient-code:self-reviewed label;
update the document to state that the inner-loop review must run on
pull_request.synchronize and that the review action will remove the
ambient-code:self-reviewed label if governance checks fail on new commits, and
conversely re-apply it when they pass; also update the Mergify queue config
description (currently referencing check-success=*) to require a governance
status/check (or the label absence) as a merge condition so merges cannot happen
without a fresh passing governance check.

Comment on lines +45 to +46
The `ambient-code:self-reviewed` label joins the existing namespace (`auto-fix`, `managed`, `needs-human`). It is machine-applied by the inner-loop review, though any contributor with write access can also apply it as an escape hatch.

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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Locate Mergify and workflow config =="
fd -HI '^\.mergify\.yml$'
fd -HI '.yml$' .github/workflows 2>/dev/null || true
fd -HI '.yaml$' .github/workflows 2>/dev/null || true

echo
echo "== Check whether merge queues rely on labels without an inner-loop required status =="
rg -n -C3 'name:\s+(urgent|self-reviewed)|label=hotfix|label=ambient-code:self-reviewed|check-success=' .mergify.yml docs/internal/adr/0008-automate-code-reviews.md

echo
echo "== Search for a required status/check tied to inner-loop review =="
rg -n -C3 'inner-loop|self-reviewed|check-success=.*(review|inner|governance)' .mergify.yml docs/internal/adr/0008-automate-code-reviews.md .github/workflows || true

Repository: ambient-code/platform

Length of output: 13704


Label-gated merge queues bypass inner-loop review without explicit status check.

The proposed configuration (lines 127–137, 162–177) allows write-access users to apply ambient-code:self-reviewed and hotfix labels directly, unlocking auto-merge without verifying the inner-loop review actually ran. Queue conditions check only for label presence + no rejections + CI passes—there is no required status check or commit hash binding to prove the inner-loop validated this specific commit. Note: This is a proposed change in the ADR; the actual .mergify.yml still requires approval counts and has not been updated yet. Line 95 of the ADR acknowledges this gap. Before deployment, add either a required status check for inner-loop result or automation to remove/revalidate labels on commit push (synchronize event).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/internal/adr/0008-automate-code-reviews.md` around lines 45 - 46, The
ADR lets users apply the ambient-code:self-reviewed and hotfix labels to bypass
merge-queue checks without proving the inner-loop ran; fix by requiring a
concrete inner-loop status check or binding before auto-merge: update the queue
conditions (the mergify rules referenced in the ADR) to include a required
status/context (e.g., inner-loop CI/status name) or add automation that
removes/revalidates ambient-code:self-reviewed on push (synchronize event) so
labels alone cannot unlock auto-merge for a different commit; target the
label-handling logic and queue condition blocks (ambient-code:self-reviewed,
hotfix, inner-loop review) in the proposed config.

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