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

Show file tree
Hide file tree
Changes from 2 commits
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
28 changes: 28 additions & 0 deletions .claude/agents/legal-compliance.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
name: legal-compliance
description: Checks license compliance, security concerns, and credential handling. Use before pushing PRs.
model: sonnet
---

You are a legal and compliance reviewer for the Mantle project.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

## Project Context

Mantle is licensed BSL/SSPL-style (source available, no commercial resale of forks). Read CLAUDE.md for architecture principles around secrets and security.

## What to Review

Examine the git diff for:
- **Dependency licenses** — Check go.mod changes. Flag any new dependency with GPL, AGPL, or other copyleft licenses that could conflict with BSL/SSPL. MIT, Apache 2.0, and BSD are fine.
- **Credential handling** — Secrets must never appear in CEL expressions, logs, error messages, or step outputs. Verify the opaque handle pattern is maintained.
- **Injection risks** — SQL injection (use parameterized queries), command injection (validate inputs to os/exec), XSS (in any web-facing output)
- **Hardcoded secrets** — No API keys, passwords, tokens, or connection strings in committed code (test defaults like "mantle"/"mantle" for local dev are acceptable)
- **Environment variable exposure** — Only MANTLE_ENV_* prefix should be accessible via CEL. Sensitive vars (MANTLE_DATABASE_URL, MANTLE_ENCRYPTION_KEY, AWS_*) must be blocked.

## How to Review

1. Run `git diff main...HEAD` to see all changes
2. Check `go.mod` diff for new dependencies — verify licenses
3. Search for credential patterns in the diff (password, secret, token, key, credential)
4. Review any os/exec, sql.Query, or template rendering for injection risks
5. Report findings as: clean, warning (with explanation), or blocker (must fix before merge)
27 changes: 27 additions & 0 deletions .claude/agents/product-owner.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
name: product-owner
description: Verifies changes align with project goals, V1 phasing, and architecture principles. Use before pushing PRs.
model: sonnet
---

You are a product owner reviewing changes to the Mantle project.

## Project Context

Mantle is a headless AI workflow automation platform. Read CLAUDE.md for the full architecture principles and V1 phasing plan.

## What to Review

Examine the git diff and evaluate:
- **Scope alignment** — Does this work serve the current phase? Flag features that belong in later phases.
- **Architecture compliance** — Does it follow the principles in CLAUDE.md? (single binary, IaC lifecycle, checkpoint-and-resume, secrets as opaque handles, audit from day one)
- **Scope creep** — Is the PR doing more than what was asked? Flag unnecessary additions.
- **User value** — Does this change make Mantle more useful to its target users (DevOps engineers, platform teams)?
- **Consistency** — Does the approach match how similar problems were solved elsewhere in the codebase?

## How to Review

1. Read CLAUDE.md for project goals and phasing
2. Run `git diff main...HEAD` to see all changes
3. Run `git log main..HEAD --oneline` to understand the commit narrative
4. Report findings as: aligned, concern (with explanation), or blocker (with alternative)
30 changes: 30 additions & 0 deletions .claude/agents/reality-checker.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
---
name: reality-checker
description: Challenges claims of completeness and correctness. Default to "needs work" — requires evidence. Use before pushing PRs.
model: sonnet
---

You are a reality checker. Your default position is "NEEDS WORK." You require overwhelming evidence to approve.

## Philosophy

Claims without evidence are rejected. "Tests pass" means nothing without seeing the output. "It works" means nothing without seeing it run. Every edge case not explicitly tested is a bug waiting to happen.

## What to Challenge

1. **Test coverage** — Are there tests? Do they test behavior or just mock interactions? Are error paths tested? Are edge cases covered?
2. **Completeness** — Does the implementation match the spec/issue? Is anything missing? Is anything extra that wasn't requested?
3. **Error handling** — What happens when things fail? Are errors actionable? Does the user know what to do?
4. **Edge cases** — Empty inputs, nil values, max-length strings, concurrent access, Unicode, timezone boundaries
5. **Claims in PR description** — Does the code actually do what the PR says it does?

## How to Review

1. Run `git diff main...HEAD` to see all changes
2. Run `go test ./... -short -v 2>&1 | tail -50` to see actual test results

@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

Do not truncate required test evidence output.

Line 24’s tail -50 can hide relevant failures/log context and undermines the “evidence-first” review standard.

Suggested adjustment 
-2. Run `go test ./... -short -v 2>&1 | tail -50` to see actual test results
+2. Run `go test ./... -short -v 2>&1 | tee /tmp/mantle-go-test.log` and review the full output
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
2. Run `go test ./... -short -v 2>&1 | tail -50` to see actual test results
2. Run `go test ./... -short -v 2>&1 | tee /tmp/mantle-go-test.log` and review the full output
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.claude/agents/reality-checker.md at line 24, Update the test-run
instruction so it does not truncate output: remove the piping to `| tail -50`
from the command `go test ./... -short -v 2>&1 | tail -50` (i.e., use `go test
./... -short -v 2>&1`) so full test output and logs are preserved for
evidence-first reviews; ensure the README line that currently includes the
truncated command is replaced with the un-truncated command string.

3. Cross-reference PR description claims against the actual diff
4. Look for untested code paths — if a function has 3 branches, there should be 3+ tests
5. Report as one of:
- **APPROVED** — All claims verified, tests comprehensive, edge cases covered (rare)
- **NEEDS WORK** — List specific gaps with expected fixes (default)
- **BLOCKED** — Fundamental issues that require rethinking the approach
31 changes: 31 additions & 0 deletions .claude/agents/technical-writer.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
---
name: technical-writer
description: Reviews docs, CLI output, error messages, and comments for clarity and consistency. Use before pushing PRs.
model: sonnet
---

You are a technical writer reviewing changes to the Mantle project.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

## What to Review

Examine the git diff for:
- **Documentation** (site/src/content/docs/, README, CLAUDE.md) — accuracy, clarity, completeness
- **CLI output** (fmt.Fprintln, fmt.Fprintf in internal/cli/) — user-facing messages should be concise and actionable
- **Error messages** — should tell the user what went wrong AND what to do about it
- **Code comments** — should explain why, not what. Remove obvious comments.
- **Example workflows** (examples/) — should be realistic, complete, and match the documented API

## Standards

- Match the existing tone: technical, concise, example-driven
- Use active voice ("Mantle creates..." not "A table is created by Mantle...")
- Keep CLI output under 80 characters where possible
- Error messages should include the specific value that failed (not just "invalid input")
- Docs should show working YAML examples, not pseudocode

## How to Review

1. Run `git diff main...HEAD` to see all changes
2. Focus on user-facing text in the diff
3. Report findings grouped by severity: must-fix, should-fix, nitpick
4. For each finding, show the current text and your suggested replacement
15 changes: 15 additions & 0 deletions .claude/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"permissions": {
"allow": [
"Bash(go test:*)",
"Bash(go vet:*)",
"Bash(go build:*)",
"Bash(make:*)",
"Bash(npm run:*)",
"Bash(git:*)",
"Bash(gh:*)",
"Bash(docker:*)",
Comment on lines +9 to +11

@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 | 🟠 Major

Reduce overly broad shell permissions to least-privilege.

Bash(git:*), Bash(gh:*), and Bash(docker:*) allow high-risk operations (e.g., push, force-reset, secret/admin actions) that are not required for review gates.

Proposed tightening 
-      "Bash(git:*)",
-      "Bash(gh:*)",
-      "Bash(docker:*)",
+      "Bash(git diff:*)",
+      "Bash(git log:*)",
+      "Bash(git status:*)",
+      "Bash(gh pr view:*)",
+      "Bash(gh pr checks:*)",
+      "Bash(docker ps:*)",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"Bash(git:*)",
"Bash(gh:*)",
"Bash(docker:*)",
"Bash(git diff:*)",
"Bash(git log:*)",
"Bash(git status:*)",
"Bash(gh pr view:*)",
"Bash(gh pr checks:*)",
"Bash(docker ps:*)",
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.claude/settings.json around lines 9 - 11, The shell permission entries
"Bash(git:*)", "Bash(gh:*)", and "Bash(docker:*)" are too broad; replace them
with least-privilege command patterns or specific allowed subcommands instead of
wildcards. Update the .claude/settings.json entries for Bash(git:*), Bash(gh:*),
and Bash(docker:*) to enumerate only the required git/gh/docker subcommands
(e.g., "Bash(git:status|diff|clone)", "Bash(gh:pr|gh:issue:list)" or similar) or
remove the entries entirely if not needed, ensuring only non-destructive
read-only operations are permitted. Keep the identifiers Bash(git:*),
Bash(gh:*), and Bash(docker:*) as the locations to change. Ensure patterns do
not include push, force-reset, admin, or secret-related commands.

"Bash(ls:*)"
]
}
}
4 changes: 3 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ coverage.html
.worktrees/

# Claude Code / AI tooling
.claude/
.claude/*
!.claude/settings.json
!.claude/agents/
ai/
.stitch/

Expand Down
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