Skip to content

chore: add pre-push review gates to CLAUDE.md #25

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
michaelmcnees wants to merge 4 commits into from
Closed

chore: add pre-push review gates to CLAUDE.md #25

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b740ff5
chore: add pre-push review gates to CLAUDE.md
michaelmcnees Mar 25, 2026
67d7713
chore: add review agents and shared project settings
michaelmcnees Mar 25, 2026
40a69f2
chore: add project review agents and shared settings
michaelmcnees Mar 25, 2026
4c225ee
chore: replace product manager agent with full source template
michaelmcnees Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,17 @@ mantle serve # Start persistent server (Phase 5+)
5. **Triggers & Server Mode** — `mantle serve`, cron scheduler, webhook ingestion
6. **Multi-tenancy & RBAC** — teams, users, roles, API keys, team scoping retrofit

## Pre-Push Review Gates

Before pushing any PR to GitHub, run the work through these review agents (in parallel):

@coderabbitai coderabbitai Bot Mar 25, 2026

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

Clarify "before pushing any PR to GitHub."

The phrase "before pushing any PR to GitHub" is ambiguous. Pull requests are created on GitHub, so they cannot be reviewed before being pushed there. Consider rephrasing to one of:

  • "Before creating a PR on GitHub..." (if reviews should happen pre-PR)
  • "Before merging any PR..." (if reviews are part of the PR process)
  • "Before pushing commits to a PR branch..." (if reviews happen before pushing code)

Additionally, the term "review agents" is ambiguous—it's unclear whether these are human roles, AI-powered automated checks, checklists for the PR author, or something else.

✏️ Proposed clarification 
-Before pushing any PR to GitHub, run the work through these review agents (in parallel):
+Before merging any PR, ensure the changes have been reviewed from these perspectives (can be done in parallel):

Or if the intent is AI/automated agents:

-Before pushing any PR to GitHub, run the work through these review agents (in parallel):
+Before creating a PR, run the changes through these AI review agents in parallel:
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@CLAUDE.md` at line 83, The sentence "Before pushing any PR to GitHub, run the
work through these review agents (in parallel):" is ambiguous; update it to
explicitly state when and what should be reviewed and clarify "review agents".
Replace the sentence with one clear option (choose the correct intent) such as
"Before creating a PR on GitHub, run the work through these review agents (in
parallel):" or "Before merging any PR on GitHub, run the work through these
review agents (in parallel):" or "Before pushing commits to a PR branch on
GitHub, run the work through these review agents (in parallel):", and add a
short parenthetical or appended clause defining "review agents" (e.g.,
"(automated AI checks and human reviewers)" or "(automated tools/AI agents and
human reviewers)") so readers know whether these are automated checks, humans,
or both.


1. **Technical Writer** — Review all changed docs, comments, CLI output, and error messages for clarity, accuracy, and consistency with existing documentation tone.
2. **Product Owner** — Verify the changes align with the project goals, V1 phasing priorities, and architecture principles listed above. Flag scope creep or work that doesn't serve the current phase.
3. **Legal Compliance Checker** — Check for license compliance (dependencies, attribution), security concerns (credential handling, injection risks), and anything that conflicts with the BSL/SSPL-style license.
4. **Reality Checker** — Challenge claims of completeness, correctness, and production readiness. Default to "needs work" — require overwhelming evidence before approving. If tests aren't shown passing, it's not done. If edge cases aren't covered, it's not ready.

@coderabbitai coderabbitai Bot Mar 25, 2026

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.

🧹 Nitpick | 🔵 Trivial

Consider softening the Reality Checker's adversarial tone.

The Reality Checker description states: "Default to 'needs work' — require overwhelming evidence before approving." This adversarial stance could unnecessarily slow development velocity and create friction, especially for straightforward changes with clear evidence (passing tests, covered edge cases).

A more balanced approach would maintain rigor while avoiding an assumption of inadequacy.

📝 Proposed tone adjustment 
-4. **Reality Checker** — Challenge claims of completeness, correctness, and production readiness. Default to "needs work" — require overwhelming evidence before approving. If tests aren't shown passing, it's not done. If edge cases aren't covered, it's not ready.
+4. **Reality Checker** — Verify claims of completeness, correctness, and production readiness with concrete evidence. Require passing tests, covered edge cases, and demonstrated correctness. Challenge assumptions and flag gaps in testing or error handling.
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@CLAUDE.md` at line 88, Update the "Reality Checker" bullet to soften the
adversarial tone: locate the heading "Reality Checker" and replace the sentence
"Default to 'needs work' — require overwhelming evidence before approving." with
a balanced phrasing that preserves rigor but permits straightforward approvals
when adequate evidence exists (for example: "Default to a thorough review:
require sufficient evidence (passing tests, covered edge cases, clear reasoning)
before approving, but allow approval when the change clearly meets those
criteria"). Ensure the rest of the bullet still calls out missing tests or
unhandled edge cases as blockers.


These reviews catch misalignment early. Do not skip them even for small changes — small PRs are where assumptions slip through unchecked.
Comment on lines +81 to +90

@coderabbitai coderabbitai Bot Mar 25, 2026

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.

🧹 Nitpick | 🔵 Trivial

Specify the enforcement mechanism and integration with GitHub workflow.

The section defines review requirements but doesn't specify how they should be enforced or integrated into the development workflow. This creates ambiguity about:

  • Who performs these reviews? (PR author as a checklist? Other team members? Automated tools?)
  • How are they enforced? (Honor system? GitHub Actions? Required reviewers? PR template checklist?)
  • Where is evidence recorded? (PR description? Review comments? External tracking?)

Without an implementation mechanism, these gates may be inconsistently applied or gradually ignored.

💡 Suggested additions to clarify implementation

Consider adding a subsection that specifies the implementation approach:

### Implementation

These reviews should be conducted by the PR author using AI-assisted tooling before creating the PR. Document completion in the PR description using this checklist:

- [ ] Technical Writer review: All docs, CLI output, and error messages reviewed for clarity
- [ ] Product Owner review: Changes align with V1 phasing and architecture principles
- [ ] Legal Compliance review: License compliance and security concerns checked
- [ ] Reality Checker review: Edge cases covered, tests passing, evidence provided

Or, if using automated tooling:

### Implementation

These reviews are enforced via GitHub Actions that run AI-powered analysis on each PR. The workflow must pass all four checks before merging is allowed.
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@CLAUDE.md` around lines 81 - 90, The "Pre-Push Review Gates" section lacks an
enforcement and integration plan; update that section (the "Pre-Push Review
Gates" heading) to specify who performs each gate, how they are enforced, and
where evidence is recorded by: adding an "Implementation" subsection that
mandates either (a) a PR-template checklist the PR author fills (listing
Technical Writer, Product Owner, Legal Compliance, Reality Checker) and requires
at least one named reviewer for each gate, or (b) a GitHub Actions workflow that
runs automated AI-assisted checks and fails the merge if any gate fails; also
specify that completion evidence must be recorded in the PR description
(checklist + reviewer names or action run IDs) so enforcement is auditable.

Comment on lines +83 to +90

@coderabbitai coderabbitai Bot Mar 25, 2026

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.

🧹 Nitpick | 🔵 Trivial

Consider scalability and practicality for small changes.

Requiring four separate reviews for every change—including small documentation fixes, typos, or minor refactors—may be impractical for small teams or solo maintainers. The explicit prohibition against skipping reviews "even for small changes" could create unnecessary overhead.

Consider introducing a pragmatic risk-based approach where the depth of review scales with the impact and risk of the change.

🎯 Suggested risk-based approach 
Before pushing any PR to GitHub, assess the risk level and apply appropriate review depth:

**High-risk changes** (new features, security-sensitive code, API changes, database migrations):
- All four review perspectives required with documented evidence

**Medium-risk changes** (bug fixes, refactors, non-trivial documentation):
- Technical Writer + Reality Checker required
- Product Owner + Legal Compliance as needed

**Low-risk changes** (typo fixes, comment updates, minor doc clarifications):
- Quick self-review covering all four perspectives
- Document any concerns in PR description

These reviews catch misalignment early. When in doubt, err on the side of more thorough review.
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@CLAUDE.md` around lines 83 - 90, Replace the absolute "Do not skip them even
for small changes" mandate with a risk-based review policy: add a short preface
after the numbered reviewer list that defines three risk tiers (High, Medium,
Low) and maps each tier to required reviewers (e.g., High = all four reviewers;
Medium = Technical Writer + Reality Checker with Product Owner/Legal as needed;
Low = quick self-review with documented notes). Update the numbered list item or
add a new paragraph referencing the existing reviewer roles and include a
one-line guideline for how to assess risk and when to escalate to full review.


## License

BSL/SSPL-style — source available, no commercial resale of forks.
Loading