Skip to content

Commit a3e86b8

Browse files
committed
Add GitHub Copilot code review custom instructions
1 parent 2dedac2 commit a3e86b8

6 files changed

Lines changed: 260 additions & 0 deletions

File tree

.github/copilot-instructions.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Code Review Guidelines
2+
3+
## Review Priorities
4+
5+
- **CRITICAL**: Security vulnerabilities, logic errors, breaking changes, data loss
6+
- **IMPORTANT**: Missing validation, poor error handling, performance issues
7+
- **SUGGESTION**: Readability, minor optimizations
8+
9+
## Review Principles
10+
11+
- Be specific: reference exact lines and variables
12+
- Provide context: explain why something is problematic
13+
- Suggest solutions: show the fix, not just the problem
14+
- Be constructive: focus on code, not the author
15+
16+
## Always Check
17+
18+
- No hardcoded secrets or credentials
19+
- User input validated before use
20+
- Errors handled appropriately
21+
- Async operations properly awaited
22+
23+
## Ignore
24+
25+
- Formatting issues (handled by Prettier/ESLint)
26+
- Minor naming preferences
27+
- Style choices already consistent in codebase
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
applyTo: "src/**/*.entity.ts,src/**/*.service.ts,src/**/*.repository.ts"
3+
---
4+
5+
# Database Review
6+
7+
## Queries
8+
9+
- No raw SQL with string interpolation
10+
- Avoid queries inside loops (N+1 problem)
11+
- Paginate large result sets
12+
- Use transactions for multi-step operations
13+
14+
## Relations
15+
16+
- Define cascade options explicitly
17+
- Avoid eager loading of large relations
18+
- Consider lazy loading for optional relations
19+
20+
## Performance
21+
22+
- Add indexes on frequently queried columns
23+
- Use `select` to fetch only needed fields
24+
- Consider query builder for complex queries
25+
26+
## Patterns
27+
28+
```typescript
29+
// Bad: N+1
30+
for (const user of users) {
31+
user.targets = await this.targetRepo.find({ user });
32+
}
33+
34+
// Good: single query with relation
35+
const users = await this.userRepo.find({
36+
relations: ['targets']
37+
});
38+
```
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
applyTo: "src/**/*.ts"
3+
---
4+
5+
# NestJS Review
6+
7+
## Architecture
8+
9+
- Controllers should be thin: delegate logic to services
10+
- Services handle business logic, one responsibility per service
11+
- Use dependency injection via constructor, never instantiate manually
12+
13+
## Validation
14+
15+
- All DTOs must use `class-validator` decorators
16+
- Apply `ValidationPipe` on endpoints receiving user input
17+
- Transform and whitelist incoming data
18+
19+
## Common Issues
20+
21+
- Missing `@Injectable()` on providers
22+
- Business logic in controllers
23+
- Circular dependencies between modules
24+
- Missing guards on protected endpoints
25+
- Not awaiting async operations
26+
27+
## Patterns
28+
29+
```typescript
30+
// Good: thin controller
31+
@Post()
32+
create(@Body() dto: CreateDto) {
33+
return this.service.create(dto);
34+
}
35+
36+
// Bad: logic in controller
37+
@Post()
38+
create(@Body() dto: CreateDto) {
39+
const validated = this.validate(dto);
40+
const result = this.transform(validated);
41+
return this.repository.save(result);
42+
}
43+
```
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
applyTo: "src/auth/**/*,src/**/*.guard.ts,src/**/*.strategy.ts"
3+
---
4+
5+
# Security Review (OWASP)
6+
7+
## Authentication
8+
9+
- JWT secrets must come from environment variables
10+
- Passwords must be hashed with bcrypt (min 10 rounds)
11+
- Never log or return passwords in responses
12+
- All protected endpoints must use AuthGuard
13+
14+
## Injection Prevention
15+
16+
- Use parameterized queries, never string concatenation
17+
- Validate and sanitize all user input
18+
- Escape data before rendering in responses
19+
20+
## Access Control
21+
22+
- Apply principle of least privilege
23+
- Verify user owns the resource before operations
24+
- Use guards for authorization, not just authentication
25+
26+
## Session & Tokens
27+
28+
- Set appropriate token expiration
29+
- Implement rate limiting on auth endpoints
30+
- Use secure cookie attributes (HttpOnly, Secure, SameSite)
31+
32+
## Secrets
33+
34+
```typescript
35+
// Good
36+
const secret = process.env.JWT_SECRET;
37+
38+
// Bad
39+
const secret = 'my-secret-key';
40+
```
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
applyTo: "src/test/**/*,**/*.spec.ts,**/*.e2e-spec.ts"
3+
---
4+
5+
# Testing Review
6+
7+
## Test Quality
8+
9+
- Tests must clean up in afterEach (close connections, clear data)
10+
- No hardcoded credentials or real secrets
11+
- Test edge cases: empty, null, boundaries
12+
- Test error paths, not just happy path
13+
14+
## Isolation
15+
16+
- No shared mutable state between tests
17+
- Tests must not depend on execution order
18+
- Each test should set up its own data
19+
20+
## Assertions
21+
22+
- Assert behavior, not implementation details
23+
- Use descriptive assertion messages
24+
- One logical assertion per test
25+
26+
## Async
27+
28+
```typescript
29+
// Good: proper async handling
30+
await expect(service.create(dto)).rejects.toThrow();
31+
32+
// Bad: missing await
33+
expect(service.create(dto)).rejects.toThrow();
34+
```

PR_DESCRIPTION.md

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# Add GitHub Copilot Code Review Custom Instructions
2+
3+
## Summary
4+
5+
This PR adds layered custom instructions for GitHub Copilot Code Review, enabling automated, context-aware code reviews based on file paths and domains.
6+
7+
## How It Works
8+
9+
```
10+
┌─────────────────────────────────────┐
11+
│ copilot-instructions.md (global) │
12+
│ • Review priorities │
13+
│ • General principles │
14+
│ • Always applies to all files │
15+
└──────────────────┬──────────────────┘
16+
17+
┌────────────────────────────┼────────────────────────────┐
18+
│ │ │
19+
▼ ▼ ▼
20+
┌─────────────────────┐ ┌─────────────────────┐ ┌─────────────────────┐
21+
│ nestjs.instructions │ │ security.instructions│ │ testing.instructions│
22+
│ ─────────────────── │ │ ─────────────────── │ │ ─────────────────── │
23+
│ applyTo: src/**/*.ts│ │ applyTo: src/auth/**│ │ applyTo: **/*.spec.ts│
24+
│ │ │ *.guard.ts │ │ **/*.e2e-* │
25+
│ • DI patterns │ │ *.strategy │ │ │
26+
│ • Thin controllers │ │ │ │ • Test isolation │
27+
│ • DTO validation │ │ • OWASP principles │ │ • Cleanup in after │
28+
└─────────────────────┘ │ • JWT security │ │ • Edge cases │
29+
│ • Input sanitization │ └─────────────────────┘
30+
└─────────────────────┘
31+
32+
33+
┌─────────────────────┐
34+
│ database.instructions│
35+
│ ─────────────────── │
36+
│ applyTo: *.entity.ts│
37+
│ *.service │
38+
│ │
39+
│ • N+1 prevention │
40+
│ • Query safety │
41+
│ • Transactions │
42+
└─────────────────────┘
43+
```
44+
45+
## Layer Combination Examples
46+
47+
| File Changed | Instructions Applied |
48+
|--------------|---------------------|
49+
| `src/auth/auth.service.ts` | global + nestjs + security + database |
50+
| `src/targets/target.entity.ts` | global + nestjs + database |
51+
| `src/test/users/login.e2e-spec.ts` | global + testing |
52+
| `src/users/users.controller.ts` | global + nestjs |
53+
54+
## File Structure
55+
56+
```
57+
.github/
58+
├── copilot-instructions.md # Global rules (all PRs)
59+
└── instructions/
60+
├── nestjs.instructions.md # NestJS patterns
61+
├── security.instructions.md # Auth & OWASP
62+
├── database.instructions.md # TypeORM & queries
63+
└── testing.instructions.md # Test quality
64+
```
65+
66+
## Key Design Decisions
67+
68+
- **Short files (<1000 chars each)**: Copilot only reads first 4,000 characters per file
69+
- **Imperative bullet points**: More effective than narrative paragraphs
70+
- **Priority system**: CRITICAL / IMPORTANT / SUGGESTION
71+
- **Code examples**: Show correct vs incorrect patterns
72+
- **No external links**: Copilot doesn't follow them
73+
74+
## References
75+
76+
- [GitHub Blog: Mastering Instruction Files](https://github.blog/ai-and-ml/unlocking-the-full-power-of-copilot-code-review-master-your-instructions-files/)
77+
- [Official Tutorial](https://docs.github.com/en/copilot/tutorials/use-custom-instructions)
78+
- [Community Examples](https://github.com/github/awesome-copilot)

0 commit comments

Comments
 (0)