Skip to content
Open
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
126 changes: 126 additions & 0 deletions .claude/commands/pr-review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
You are an expert code reviewer evaluating pull requests across architecture, business logic correctness, performance, security, and style. Your goal is to surface high-impact issues that could cause bugs, scalability problems, security risks, or long-term maintainability concerns, while respecting existing patterns and author intent. Assume good intent. Do not nitpick. Comment only when confident. Focus on systemic issues and keep feedback concise, clear, and actionable.

## Step 1: Check for Previous Reviews

Before anything else, check if a prior review exists:
```
cat reviews/pr-$ARGUMENTS.md 2>/dev/null
```

If a previous review file exists:
- Load it as full context.
- Identify every issue that was flagged (Critical, Major, Minor).
- In your new review, explicitly track each prior issue: was it **Fixed**, **Partially Fixed**, **Not Addressed**, or **No Longer Applicable**?
- Do not re-flag issues that are fully resolved. Focus new comments on remaining issues and any newly introduced problems.

If no previous review exists, this is a first review — proceed normally.

## Step 2: Fetch PR Details

```
gh pr view $ARGUMENTS --json title,body,author,baseRefName,headRefName,additions,deletions,changedFiles
gh pr diff $ARGUMENTS
gh pr view $ARGUMENTS --json files --jq '.files[].path'
```

## Phase 1: Build Context
- Review architecture and contribution docs (README.md, CONTRIBUTING.md, AGENTS.md, docs/).
- Identify project type, language, framework, runtime.
- Understand module/package/workspace structure, architectural layers, and data flow.

## Phase 2: Architecture & Structure
Evaluate fit with existing architecture:
- Project structure: repository organization, directory layout, separation of concerns (frontend/backend/shared/domain), naming conventions, proper code placement (models, services, controllers, components, utilities).
- Patterns: layered architecture (presentation/business/data), DDD (entities/services/repositories/adapters), MVC/MVVM, modular or microservice boundaries.
Ask:
- Are boundaries respected?
- Do layers communicate appropriately?
- Do dependencies flow the right direction?
- Is business logic isolated from infrastructure/framework code?
Flag:
- Layer violations, skipping layers, circular dependencies, tight coupling.
- Mixed concerns (business logic in UI, UI logic in data layer).
- Incorrect file placement or naming.
- Introduced, undocumented, or conflicting patterns.

## Phase 3: Business Logic & Correctness
Review intended behavior and logic:
- Check alignment between PR description and implementation.
- Identify incorrect algorithms/data structures, race conditions, concurrency issues.
- Watch for incorrect assumptions about mutability or side effects.
- Detect misuse of APIs beyond their intended design.
Use context clues (filenames, variables, comments, docs). Comment only when you fully understand the intended behavior. Do not guess requirements. Focus on clear sources of incorrect behavior. Avoid nitpicking tradeoffs the author likely understands.

## Phase 4: Performance
Assess scalability and efficiency:
- Time/space complexity concerns.
- N+1 queries, waterfall or unbatched network requests.
- Unnecessary recomputation, re-renders, duplicated state.
- Memory leaks or unbounded growth.
Consider expected production data size. Avoid performance flags for trivially small data. Align advice with the language, framework, and libraries. Balance performance with readability and maintainability.

## Phase 5: Security
Evaluate security impact:
- Injection risks (SQL, XSS, command, etc.).
- Input validation/sanitization at trust boundaries.
- AuthN/AuthZ bypasses.
- Unsafe defaults (randomness, hashing, crypto).
- Secret leakage or misuse.
- Multi-tenant isolation issues.
Understand how auth and permissions work in the codebase. Distinguish frontend vs backend responsibilities. Follow env variable and secret management patterns. Do not flag public keys that are intentionally public.

## Phase 6: Style & Readability
Focus on major issues:
- Egregious naming problems.
- Inconsistent formatting or indentation.
- Redundant or unnecessary code.
- Misleading, redundant, or missing comments.
- Typos and obvious polish issues.
Only flag style patterns that are clearly established elsewhere in the repo. If unsure, do not comment.

## Feedback Guidelines
- Prioritize structural and systemic issues over minor implementation details.
- Be consistent across the PR.
- When flagging an issue, suggest a clear, practical alternative.
- Consider long-term maintainability and scale.
- Keep comments concise and actionable.

## Output Format

Structure your review using the template below, then save it by running:
```
mkdir -p reviews && cat >> reviews/pr-$ARGUMENTS.md << 'REVIEW_EOF'
[your full review content here]
REVIEW_EOF
```

Use this template:

## Review — [DATE]

**PR #$ARGUMENTS — [Title]**
**Author:** [author] | **Base:** [base branch] | **Changes:** +[additions] -[deletions] across [N] files
**Round:** [1st Review / 2nd Review / Nth Review]

### Summary
Brief 2-3 sentence overview of what the PR does (or what changed since last review).

### Previous Issues Status *(skip on first review)*
| Issue | Status |
|-------|--------|
| [issue description from prior review] | ✅ Fixed / ⚠️ Partially Fixed / ❌ Not Addressed / N/A |

### Critical Issues 🔴
Issues that must be fixed before merging (bugs, security, data loss risk).

### Major Issues 🟠
Significant concerns around architecture, performance, or correctness.

### Minor Issues 🟡
Style, readability, and low-risk improvements.

### Positives ✅
What was done well (optional but encouraged).

### Verdict
**Approve / Request Changes / Needs Discussion** — one sentence rationale.
37 changes: 37 additions & 0 deletions .claude/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
{
"$schema": "https://json.schemastore.org/claude-code-settings.json",
"permissions": {
"allow": [
"Bash(npm run *)",
"Bash(gh pr view:*)",
"Bash(gh pr diff:*)",
"Bash(gh pr comment:*)",
"Bash(gh pr review:*)",
"Bash(gh auth status:*)"
]
},
"hooks": {
"PreToolUse": [
{
"matcher": "Edit|Write",
"hooks": [
{
"type": "command",
"command": "node $CLAUDE_PROJECT_DIR/scripts/inject-context.mjs"
}
]
}
],
"PostToolUse": [
{
"matcher": "Edit|Write",
"hooks": [
{
"type": "command",
"command": "bash $CLAUDE_PROJECT_DIR/scripts/arch-validate.sh"
}
]
}
]
}
}
171 changes: 171 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
# AGENTS.md — NestForge

NestJS hexagonal (Ports & Adapters) architecture. 20+ feature modules, each with identical internal structure. This file is loaded every session — keep it loaded and trust it.

---

## Layer Map

```
src/<module>/
├── domain/ ← Pure TS classes, NO DB decorators
│ ├── <entity>.ts ← camelCase props, no imports from infra
│ └── queries/ ← Query result shapes (no DB deps)
├── dto/ ← class-validator decorated DTOs
│ ├── create-<entity>.dto.ts
│ ├── find-all-<entity>.dto.ts
│ └── update-<entity>.dto.ts
├── enums/ ← TypeScript enums only
├── infrastructure/
│ └── persistence/
│ ├── <module>.abstract.repository.ts ← PORT (abstract class, interface)
│ └── relational/
│ ├── entities/ ← TypeORM entities, snake_case columns
│ ├── mappers/ ← Static toDomain() / toPersistence()
│ ├── repositories/ ← ADAPTER (implements abstract repo)
│ ├── queries/ ← Raw SQL constants + query mappers
│ └── relational-persistence.module.ts
├── <module>.controller.ts ← HTTP layer only
├── <module>.service.ts ← Business logic, uses abstract repo
└── <module>.module.ts
```

---

## Dependency Rules (NEVER violate)

| Layer | Can import from | NEVER import from |
|---|---|---|
| `domain/` | `@src/utils/types/` | `infrastructure/`, `typeorm`, `@nestjs/typeorm`, other module's infra |
| `service.ts` | `domain/`, abstract repo, `@src/common/`, `@src/utils/` | TypeORM `Repository<>`, concrete repos, DB entities |
| Abstract repo | `domain/`, `@src/utils/types/` | TypeORM, DB entities, concrete repos |
| Concrete repo | DB entities, mappers, abstract repo, `@src/utils/` | Other modules' concrete repos |
| `mapper.ts` | domain entity, DB entity | services, controllers, other mappers |
| `controller.ts` | service, domain (for types), DTOs | repos, mappers, DB entities |

---

## Import Alias

All absolute imports use `@src/`:
```typescript
import { NOT_FOUND } from '@src/common/exceptions';
import { IPaginationOptions } from '@src/utils/types/pagination-options';
import { pagination } from '@src/utils/pagination';
import { infinityPagination } from '@src/utils/infinity-pagination';
import { runRawQueryOnReadReplica } from '@src/database-helpers/run-raw-query-on-read-replica';
```

---

## Error Handling — ALWAYS use @src/common/exceptions

```typescript
// ✅ Correct
import { NOT_FOUND, UNPROCESSABLE_ENTITY, BAD_REQUEST, FORBIDDEN } from '@src/common/exceptions';
throw NOT_FOUND('Booking', { id });
throw UNPROCESSABLE_ENTITY('Method not found on repository.', 'field');
throw BAD_REQUEST('Invalid payload');

// ❌ Wrong — never throw raw NestJS exceptions in services
throw new NotFoundException({ ... });
throw new BadRequestException('...');
```

---

## Pagination — Two patterns

```typescript
// infinityPagination: for list endpoints returning InfinityPaginationResponseDto
import { infinityPagination } from '@src/utils/infinity-pagination';
return infinityPagination(await this.service.findAllWithPagination({ paginationOptions }), { page, limit });

// pagination: for raw-query endpoints returning PaginationResponseDto with total count
import { pagination } from '@src/utils/pagination';
const { data, count } = await this.repo.findWithPagination({ paginationOptions, filter });
return pagination(data, count, paginationOptions);
```

---

## Mapper Pattern — ALWAYS static, ALWAYS called on read

```typescript
// ✅ Correct: static methods, always map before returning
return entity ? BookingMapper.toDomain(entity) : null;
return entities.map((e) => BookingMapper.toDomain(e));

// ❌ Wrong: returning raw DB entity from repo, or instantiating mapper
return entity; // never return un-mapped entity from a repo method
new BookingMapper(); // mappers are never instantiated
```

---

## Repository Pattern — Single-responsibility methods

```typescript
// ✅ Correct: specific methods per query
async findByEmail(email: string): Promise<NullableType<User>> {}
async findByIds(ids: string[]): Promise<User[]> {}

// ❌ Wrong: universal/generic find
async find(condition: UniversalConditionInterface): Promise<User> {}
```

---

## Raw Queries — Use runRawQueryOnReadReplica

```typescript
import { runRawQueryOnReadReplica } from '@src/database-helpers/run-raw-query-on-read-replica';
// Store SQL in src/<module>/infrastructure/persistence/relational/queries/<module>-queries.const.ts
const data = await runRawQueryOnReadReplica(this.someRepository, SOME_QUERY, [param1]);
```

---

## Code Generation — Use Hygen for new modules

```bash
npx hygen resource-entity new # generates full hexagonal scaffold
```
See `docs/hygen/` for templates.

---

## NEVER List (hard violations)

- NEVER import TypeORM or DB entities into `domain/` files
- NEVER inject `@InjectRepository` or TypeORM `Repository<>` directly into a service
- NEVER return a raw DB entity from a repository method (always call Mapper.toDomain())
- NEVER use `console.log/warn/error` — use NestJS Logger (`new Logger('ClassName')`)
- NEVER use `export default` anywhere
- NEVER throw raw NestJS exceptions — use helpers from `@src/common/exceptions`
- NEVER create universal/generic repository methods — one method per query shape

---

## Routing Table (leaf docs for agents)

| Working on... | Read |
|---|---|
| Layer conventions, file placement | `docs/layers/index.md` |
| Domain entities (`domain/*.ts`) | `docs/layers/domain.md` |
| Services (`*.service.ts`) | `docs/layers/service.md` |
| Repositories (abstract or concrete) | `docs/layers/repository.md` |
| Mappers (`*.mapper.ts`) | `docs/layers/mapper.md` |
| Controllers (`*.controller.ts`) | `docs/layers/controller.md` |
| DTOs (`*.dto.ts`) | `docs/layers/dto.md` |
| File placement / new files | `docs/cross-cutting/file-placement.md` |
| Exception handling | `docs/cross-cutting/exceptions.md` |
| Hook enforcement system | `docs/agent-hooks-guide.md` |

---

## Enforcement Summary

PreToolUse (before every Edit/Write): injects matching leaf docs + blocks bad file placements.
PostToolUse (after every Edit/Write): grep-based convention checks, exit 2 blocks if violated.
Agents self-correct before proceeding — violations never survive a session.
Loading