feat: Amber automatic code reviews with memory system (Phase 2)

[jeremyeder]

Summary

Implements Phase 2 of the Amber workflow transparency initiative: automatic PR reviews using the memory system from PRs #359 and #360.

Changes

New Workflow: .github/workflows/amber-auto-review.yml

Automatic PR reviews with repository intelligence:

• Trigger: Automatic on PR open/synchronize
• Memory System: Loads 7 files for comprehensive repository context
  • CLAUDE.md (master project instructions)
  • backend-development.md (Go backend, K8s patterns)
  • frontend-development.md (NextJS, Shadcn UI, React Query)
  • security-standards.md (Auth, RBAC, token handling)
  • k8s-client-usage.md (User token vs service account)
  • error-handling.md (Consistent error patterns)
  • react-query-usage.md (Data fetching patterns)
• Transparency: Every review includes:
  • Link to workflow run (90-day log retention)
  • Collapsible section showing which memory files were loaded
  • Clear explanation of how Amber applies repository standards
• Comment Management: Minimizes old review comments before posting new one

Modified Workflow: .github/workflows/claude-code-review.yml

Changed from automatic to manual-only:

• Before: Triggered automatically on all PRs
• After: Only triggers on @claude mentions in comments
• Reason: Separate automatic reviews (Amber) from manual reviews (Claude)

Three-Tier AI Integration

This completes the separation of AI review types:

1. @claude (Lightweight) - Manual, quick interactions

   • No repository wrapping, fastest response
   • Use for: questions, exploration, explanations

2. Automatic PR Reviews (Amber-Wrapped) - NEW with this PR

   • Applies CLAUDE.md + memory system standards
   • Shows decision process via workflow logs
   • Trigger: Every PR open/sync (automatic)

3. @amber mentions (Repository-Aware) - Future Phase 3

   • Planned: Manual @amber mentions for specific tasks
   • Will apply same memory system as automatic reviews

Testing

Tested on fork with PR: jeremyeder#33

After merge, every new PR will trigger amber-auto-review.yml. To verify immediately:

1. Merge this PR
2. The workflow should trigger on this PR itself (meta!)
3. Verify review comment appears with transparency section
4. Check workflow logs via the link in the review

Design Decisions

Why Direct File Loading (No Shell Script)

Rejected approach: Shell script prompt builder using sed/grep/perl to extract CLAUDE.md sections

Chosen approach: Direct file loading via Claude's Read tool

• Simpler implementation
• More maintainable (change memory files, workflow stays the same)
• Leverages built-in Claude Code capabilities
• No fragile shell script parsing

Why Memory System Over CLAUDE.md Extraction

The memory system (PRs #359, #360) provides:

• Curated, focused context for specific domains
• Easier to maintain and update
• Better separation of concerns
• Loadable on-demand for targeted work

Impact

Users will see: Every PR automatically gets a repository-aware code review showing:

• Issues categorized by severity (Blocker/Critical/Major/Minor)
• Positive highlights
• Prioritized recommendations
• Full transparency into AI decision-making

Developers will benefit from:

• Consistent application of project standards
• Early detection of anti-patterns
• Confidence in AI reviews (visible decision process)
• Reduced review burden on human reviewers

Related

• Builds on: PRs feat: implement memory system for better Claude Code context #359, feat: add memory system and repomix analysis for Claude Code #360 (Memory System)
• Part of: Amber Workflow Transparency Initiative
• Documentation: Will be updated in follow-up PR

---

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

[github-actions]

Claude Code Review

Summary

This PR implements Phase 2 of the Amber workflow transparency initiative by adding automatic PR reviews using the memory system. The implementation is well-designed and production-ready, with excellent transparency features and proper separation of concerns between automatic (Amber) and manual (@claude) reviews.

The code follows GitHub Actions best practices, implements proper error handling, and provides exceptional user experience through workflow transparency links and collapsible context sections.

Issues by Severity

🚫 Blocker Issues

None - No blocking issues found.

🔴 Critical Issues

1. Security: `pull_request_target` requires careful review (.github/workflows/amber-auto-review.yml:12)

Using `pull_request_target` is necessary for fork support but creates security risks if untrusted code is executed. Your implementation is currently safe because:

✅ Only uses `anthropics/claude-code-action@v1` (trusted action)
✅ No arbitrary code execution from PR content
✅ Proper `allowed_non_write_users: '*'` limits write access

However, document this security consideration in a comment at the top of the workflow file:

```yaml

SECURITY NOTE: Using pull_request_target for fork support

- ONLY trusted actions are used (anthropics/claude-code-action)

- NO untrusted code from PR is executed

- allowed_non_write_users limits write capabilities

See: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

```

2. Missing error handling for PR number extraction (.github/workflows/claude-code-review.yml:22-23)

The conditional check `contains(github.event.comment.body, '@claude')` will pass for both issue comments and PR review comments, but `context.payload.pull_request.number` is undefined for issue comments on issues (not PRs).

Fix: Add a check to ensure the comment is on a PR:

```yaml
if: |
contains(github.event.comment.body, '@claude') &&
github.event.issue.pull_request != null
```

Or handle in the script:

```javascript
if (!context.payload.issue.pull_request) {
console.log('Not a PR comment, skipping');
return;
}
const prNumber = context.issue.number;
```

🟡 Major Issues

1. Duplicate comment minimization logic (3 files with identical code)

Files: `amber-auto-review.yml:33-69`, `claude-code-review.yml:40-76`

Issue: The bash script for minimizing old comments is duplicated. This violates DRY principles and makes maintenance harder.

Recommendation: Extract to a composite action or reusable workflow:

```yaml

.github/actions/minimize-review-comments/action.yml

name: 'Minimize Old Review Comments'
description: 'Minimizes old Claude Code Review comments'
inputs:
pr_number:
required: true
token:
required: true
runs:
using: "composite"
steps:
- run: |
# Existing minimization logic here
shell: bash
```

2. Hardcoded memory file list in prompt (.github/workflows/amber-auto-review.yml:84-92)

The memory system files are listed explicitly. If files are renamed/moved, the workflow breaks silently.

Recommendation: Either:

• Add a step to verify files exist before running Claude
• Or use a glob pattern
• Or maintain a `.claude/memory-manifest.txt`

3. Inconsistent transparency link format

`amber-issue-handler.yml` adds transparency links inline, but review workflows add them in separate step. Creates maintenance burden.

🔵 Minor Issues

1. Missing workflow file size optimization

The `transparencySection` template string contains 24 lines of markdown. Consider moving to a separate file.

2. Inconsistent comment detection method

`amber-auto-review.yml:153` sorts by `created_at` descending, but `claude-code-review.yml:188` includes `includes('View AI decision process')` check.

3. Continue-on-error may hide real issues

Line 34: `continue-on-error: true` means failures are silent. Consider adding a warning notification.

4. No rate limiting consideration

Rapid force-pushes could cause API rate limiting or excessive costs.

Recommendation: Add concurrency control:
```yaml
concurrency:
group: amber-review-${{ github.event.pull_request.number }}
cancel-in-progress: true
```

5. Missing CODEOWNERS integration

Consider notifying human reviewers after Amber completes.

Positive Highlights

Excellent transparency design 🌟

• Workflow run links with 90-day retention notice
• Collapsible details showing loaded memory files
• Clear explanation of decision-making process

Well-structured separation of concerns 🎯

• Clean separation between automatic (Amber) and manual (@claude) reviews
• Proper trigger isolation
• No conflicts between workflows

Robust error handling 💪

• Proper use of `continue-on-error` for non-critical steps
• GraphQL mutation error handling with counting
• Graceful handling of missing comments

Great user experience ✨

• Old comments minimized automatically
• Clear severity categorization
• Memory system visibility builds trust

Security conscious 🔒

• Uses `allowed_non_write_users: '*'` to limit permissions
• Proper use of `startswith()` to avoid false matches
• No execution of untrusted code from PRs

Recommendations

Priority 1 (Before merge)

1. Add security documentation comment for `pull_request_target` usage
2. Fix PR number extraction for @claude mentions on non-PR issues

Priority 2 (Follow-up PR recommended)

3. Extract comment minimization to reusable composite action
4. Add file existence validation for memory system files
5. Add concurrency control to prevent duplicate reviews on rapid pushes

Priority 3 (Nice to have)

6. Standardize transparency link addition across all workflows
7. Consider CODEOWNERS notification after Amber review
8. Add workflow monitoring/alerting for failures

Testing Validation

Per the PR description, this was tested on a fork PR. Recommend also testing:

• Review on a PR that modifies backend Go code
• Review on a PR that modifies frontend React code
• Multiple rapid synchronize events
• @claude mention on an issue (not PR)

Architectural Alignment

This PR excellently aligns with repository patterns:

• ✅ Follows the three-tier AI integration design
• ✅ Builds on PRs feat: implement memory system for better Claude Code context #359, feat: add memory system and repomix analysis for Claude Code #360 (memory system)
• ✅ Consistent with Amber workflow transparency initiative
• ✅ No breaking changes to existing workflows

Overall Assessment

APPROVED with minor improvements recommended. This is high-quality work that significantly improves the developer experience. The automatic reviews with memory system integration provide consistent, repository-aware code quality checks. The transparency features are exemplary.

The critical issues noted are for defense-in-depth security documentation and edge case handling - the core functionality is sound.

---

Recommendation: ✅ Safe to merge after addressing the two critical issues (documentation + PR check). Consider the major/minor issues in follow-up PRs.