Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
75 changes: 75 additions & 0 deletions .github/agents/code-reviewer.agent.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
---
name: 'Code Reviewer'
description: 'Full-stack code review specialist for Spring Boot backend and React frontend with security, performance, and coding standards focus'
model: Claude Opus 4.6 (copilot)
tools: ['codebase', 'search', 'problems']
---

You are a senior full-stack code reviewer for the Three Rivers Bank credit card comparison platform. Your role is to review code quality, security, and adherence to project standards across the Spring Boot backend and React frontend.

## Review Process

For every review, provide structured feedback organized by **severity**:
- 🔴 **Critical** — Must fix before merging (security issues, functional bugs, broken contracts)
- 🟡 **Warning** — Should fix (standards violations, missing coverage, performance concerns)
- 🔵 **Suggestion** — Nice to have (style improvements, minor optimizations)

## Java / Spring Boot Checks

1. **Wildcard Imports (Critical)** — Flag any `import java.util.*` or similar. Every import must be explicit.
2. **DTO Usage (Critical)** — Verify controllers return DTOs, never JPA entities directly.
3. **Dependency Injection (Warning)** — Verify constructor injection with `private final` fields; flag `@Autowired` field injection.
4. **BIAN Circuit Breaker (Critical)** — All calls to the BIAN API must go through Resilience4j circuit breaker. Flag any direct HTTP calls that bypass it.
5. **Error Handling (Warning)** — Verify services handle exceptions properly; verify `@ControllerAdvice` exists for global error handling.
6. **Validation (Warning)** — Verify `@Valid` + Bean Validation annotations (`@NotNull`, `@Size`, etc.) on controller DTO parameters.
7. **Logging (Suggestion)** — Verify SLF4J is used with parameterized messages, not string concatenation.
8. **Transactions (Warning)** — Verify `@Transactional` is on service methods, not controllers.
9. **Package Structure** — Verify classes are in the correct package: `com.threeriversbank/{controller,service,repository,model/{entity,dto},client,config}`.

## React / Frontend Checks

1. **React Query (Critical)** — Verify all data fetching uses React Query (`useQuery`, `useMutation`). Flag raw `fetch`, `axios`, or Redux for server state.
2. **Functional Components (Warning)** — Flag any class components; all components must use hooks.
3. **MUI Theme (Warning)** — Verify Material-UI components use the Three Rivers Bank theme from `frontend/src/theme.js`. Flag hardcoded color values (hex/rgb) that should come from the theme.
4. **React Router v6 (Suggestion)** — Verify navigation uses React Router v6 APIs (`useNavigate`, `<Link>`, `<Routes>`).
5. **File Organization (Suggestion)** — Verify reusable components are in `components/{cards,common,layout}/`; page components are in `pages/`.

## Security Checks (Critical)

1. **Hardcoded Credentials** — Scan for hardcoded passwords, API keys, tokens, or secrets in any file.
2. **SQL Injection** — Review JPQL/native queries for parameterized inputs; flag string concatenation in queries.
3. **XSS** — Flag `dangerouslySetInnerHTML` usage in React; verify dynamic content is sanitized.
4. **Docker Secrets** — Flag any credentials or secrets embedded directly in Dockerfiles or docker-compose files.

## Test Coverage Checks

1. **New Public Methods (Warning)** — Verify new public service and controller methods have unit tests.
2. **Controller Tests (Warning)** — Verify new API endpoints have at least one `@WebMvcTest` test.
3. **BIAN Mock (Warning)** — Verify integration tests use WireMock to mock BIAN API, not real API calls.
4. **Test Naming (Suggestion)** — Verify test method names follow: `methodName_should_expectedBehavior_when_scenario`.

## Docker / Infrastructure Checks

1. **Image Naming (Warning)** — Verify Docker image names follow `threeriversbank/{backend|frontend}:latest`.
2. **Multi-Stage Builds (Suggestion)** — Verify Dockerfiles use multi-stage builds.

## Review Output Format

```
## Code Review Summary

### 🔴 Critical Issues
- [File:Line] Description of issue and how to fix it

### 🟡 Warnings
- [File:Line] Description of issue and recommendation

### 🔵 Suggestions
- [File:Line] Optional improvement

### ✅ Approved Items
- List what looks good

### Overall Assessment
[APPROVE / REQUEST_CHANGES / NEEDS_DISCUSSION]
```
44 changes: 44 additions & 0 deletions .github/instructions/code-review.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
applyTo: "**/*.java,**/*.jsx,**/*.js,**/*.ts"
description: "Code review checklist for Three Rivers Bank full-stack codebase"
---

# Code Review Instructions

Apply these checks whenever reviewing Java, React, or TypeScript code in this repository.

## Java — Imports
- **Flag any wildcard imports** (e.g., `import java.util.*`). Every import must be explicit (e.g., `import java.util.List`).

## Java — API Layer
- Verify that controller methods return DTOs, not JPA entity objects.
- Verify that `@Valid` is applied on DTO parameters in controller methods where input validation is required.

## Java — BIAN API Integration
- Verify that **all calls to the BIAN API go through the Resilience4j circuit breaker** (Feign client with `@CircuitBreaker` or equivalent Resilience4j configuration).
- Flag any direct HTTP calls to BIAN that bypass the circuit breaker.

## Java — Error Handling
- Verify that services use try-catch blocks or declare checked exceptions appropriately.
- Verify that a `@ControllerAdvice` class handles exceptions globally — do not let unhandled exceptions reach the client as 500 stack traces.

## React / Frontend — Data Fetching
- Verify that **React Query** (`useQuery`, `useMutation`) is used for all data fetching.
- Flag any use of raw `fetch`, `axios`, or Redux for server state management.

## React / Frontend — Theming
- Verify that Material-UI components use the Three Rivers Bank theme (imported from `frontend/src/theme.js`).
- Flag hardcoded color values (e.g., `#003366`, `#008080`, or any other hex/rgb literals) that should come from the theme instead.

## Security
- Check for hardcoded secrets, API keys, passwords, or credentials in any file.
- Check for potential SQL injection in JPQL/native queries — prefer Spring Data JPA method queries or parameterized queries.
- Check for XSS risk in React code — avoid `dangerouslySetInnerHTML`; sanitize any dynamic HTML.

## Test Coverage
- Verify that new public methods in service and controller classes have corresponding unit tests.
- Verify that new API endpoints have at least one `@WebMvcTest` controller test.

## Docker
- Verify that Docker image names follow the convention `threeriversbank/{backend|frontend}:latest`.
- Flag any secrets or credentials embedded in `Dockerfile` or `docker-compose` files.
28 changes: 28 additions & 0 deletions .github/instructions/docker.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
applyTo: "**/Dockerfile,**/docker-compose*.yml,docker/**"
description: "Docker and container standards for Three Rivers Bank"
---

# Docker Instructions

## Multi-Stage Builds
- Use multi-stage builds to produce smaller production images.
- Keep build dependencies (e.g., Maven, Node.js) in build stages only; copy only the final artifact to the runtime stage.

## Backend Image
- Use a Java 17+ base image (e.g., `eclipse-temurin:17-jre`) for the runtime stage.
- H2 is embedded — no external database container is needed.
- Backend Dockerfile: `docker/backend.Dockerfile`.

## Frontend Image
- Use Nginx to serve the static Vite build output in the runtime stage.
- Frontend Dockerfile: `docker/frontend.Dockerfile`.

## Image Naming
- Name images following the convention: `threeriversbank/{backend|frontend}:latest`.
- For CI/CD, also tag with branch name and commit SHA.

## Security
- **Never include secrets, credentials, API keys, or passwords** in Dockerfiles or docker-compose files.
- Use environment variables or secrets management (e.g., GitHub Actions secrets, Azure Key Vault) for sensitive values.
- Production environment variables: `BIAN_API_URL`, `H2_CONSOLE_ENABLED=false`, `LOGGING_LEVEL=INFO`.
42 changes: 42 additions & 0 deletions .github/instructions/java.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
---
applyTo: "**/*.java"
description: "Java and Spring Boot coding standards for Three Rivers Bank backend"
---

# Java / Spring Boot Instructions

## Imports
- **Never use wildcard imports** (e.g., `import java.util.*`). Always use explicit imports for each class (e.g., `import java.util.List`, `import java.util.Map`).

## Dependency Injection
- Use constructor-based dependency injection with `private final` fields.
- Avoid field injection (`@Autowired` on fields).

## Spring Stereotypes
- Use `@RestController` for REST API controllers.
- Use `@Service` for business logic classes.
- Use `@Repository` for data access classes.

## API Layer
- Use DTOs for all API responses; never expose JPA entities directly in controller responses.
- Use Java Bean Validation (`@Valid`, `@NotNull`, `@Size`, etc.) on DTO fields.
- Keep controllers thin — delegate all business logic to `@Service` classes.

## Configuration
- Prefer `application.yml` over `application.properties`.
- Use `@ConfigurationProperties` for type-safe, grouped configuration binding.

## Logging
- Use SLF4J (`private static final Logger log = LoggerFactory.getLogger(...)`) for logging.
- Use parameterized messages (e.g., `log.info("Processing card id={}", id)`) — never string concatenation.

## Error Handling
- Implement global exception handling with `@ControllerAdvice` and `@ExceptionHandler`.
- Do not swallow exceptions silently; always log with appropriate level.

## Transactions
- Apply `@Transactional` on service methods at the most granular level needed.
- Do not place `@Transactional` on controller methods.

## Package Structure
Follow the project package structure: `com.threeriversbank/{controller,service,repository,model/{entity,dto},client,config}`.
30 changes: 30 additions & 0 deletions .github/instructions/react.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
---
applyTo: "frontend/**/*.jsx,frontend/**/*.js"
description: "React and frontend coding standards for Three Rivers Bank frontend"
---

# React / Frontend Instructions

## Component Style
- Use functional components with hooks only — no class components.
- Keep components small and focused on a single responsibility.

## State Management
- Use React Query (TanStack Query) for all server state — do **not** use Redux, raw `fetch`, or raw `axios` for API calls.
- Use local `useState`/`useReducer` only for purely local UI state.

## Routing
- Use React Router v6 for all navigation (`useNavigate`, `<Link>`, `<Routes>`, `<Route>`).

## UI / Theming
- Use Material-UI (MUI) components for all UI elements.
- Always import and use the Three Rivers Bank theme from `frontend/src/theme.js`.
- Do **not** hardcode colors or override theme values inline — use the theme's palette (Navy `#003366`, Teal `#008080`).

## File Organization
- Place reusable components in `frontend/src/components/{cards,common,layout}/`.
- Place page-level components in `frontend/src/pages/` (e.g., `HomePage.jsx`, `CardComparisonPage.jsx`, `CardDetailsPage.jsx`).

## API Integration
- All API calls must go through React Query hooks (e.g., `useQuery`, `useMutation`).
- Base path for backend API: `/api/cards`.
25 changes: 25 additions & 0 deletions .github/instructions/testing.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
applyTo: "**/*Test.java,**/*Tests.java,**/*.spec.ts"
description: "Testing standards for Three Rivers Bank backend (JUnit 5) and E2E (Playwright)"
---

# Testing Instructions

## Backend — JUnit 5 + Mockito

- Use JUnit 5 (`@ExtendWith(MockitoExtension.class)`) with Mockito for unit tests.
- Follow the **Arrange-Act-Assert (AAA)** pattern in every test method; separate each section with a blank line.
- Name test methods using the pattern: `methodName_should_expectedBehavior_when_scenario`
- Example: `findById_should_returnCard_when_idExists`
- Use `@WebMvcTest` for controller layer tests with `MockMvc`.
- Use `@DataJpaTest` for repository layer tests backed by H2.
- Use **WireMock** to mock BIAN API calls in integration tests — never call the real BIAN API in tests.
- Use `@SpringBootTest` sparingly; prefer slice tests (`@WebMvcTest`, `@DataJpaTest`) for speed.

## Frontend — Playwright E2E

- Place E2E test files in `tests/e2e/` with the `.spec.ts` extension.
- Store test fixtures in `tests/fixtures/` (e.g., `credit-cards.json`); fixtures must match H2 seed data.
- Run tests across both **Chromium** and **WebKit** browsers.
- Test at all three configured viewports: desktop (1920×1080), tablet (768×1024), and mobile (375×667).
- Use page object patterns to keep test logic reusable and maintainable.
54 changes: 54 additions & 0 deletions .github/prompts/code-review.prompt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
---
description: "Reusable prompt for triggering comprehensive code reviews of Three Rivers Bank changes"
---

# Code Review Prompt

Perform a comprehensive code review of the changed files, using the standards defined in `.github/instructions/code-review.instructions.md`.

## Review Scope

Check ALL of the following:

### 🔴 Critical (Block merge if found)
- [ ] Wildcard Java imports (e.g., `import java.util.*`) — must be explicit
- [ ] JPA entities exposed directly in API responses — must use DTOs
- [ ] BIAN API calls that bypass the Resilience4j circuit breaker
- [ ] Hardcoded secrets, credentials, API keys, or passwords
- [ ] SQL injection risk in JPQL/native queries
- [ ] XSS risk (`dangerouslySetInnerHTML` without sanitization)

### 🟡 Warnings (Should fix before merge)
- [ ] Field injection (`@Autowired`) instead of constructor injection
- [ ] Raw `fetch`/`axios`/Redux used instead of React Query for server state
- [ ] Hardcoded Material-UI colors that bypass the Three Rivers Bank theme
- [ ] Missing `@Valid` on controller DTO parameters
- [ ] Missing error handling in service methods or missing `@ControllerAdvice`
- [ ] New public methods without unit tests
- [ ] New API endpoints without `@WebMvcTest` controller tests

### 🔵 Suggestions (Nice to have)
- [ ] SLF4J parameterized logging (not string concatenation)
- [ ] Test method naming convention: `methodName_should_expectedBehavior_when_scenario`
- [ ] Multi-stage Docker builds
- [ ] Docker image naming: `threeriversbank/{backend|frontend}:latest`

## Output Format

```
## Code Review Summary

### 🔴 Critical Issues
- [File:Line] Issue description — recommended fix

### 🟡 Warnings
- [File:Line] Issue description — recommendation

### 🔵 Suggestions
- [File:Line] Optional improvement

### ✅ What Looks Good
- Summary of compliant areas

### Overall: [APPROVE / REQUEST_CHANGES / NEEDS_DISCUSSION]
```
Loading