---
name: root-cause-analysis
description: >-
  Deep root cause analysis using the 5 Whys and Fishbone techniques.
  Use when systematic debugging hasn't found the cause, or for complex systemic issues.
  Keywords: root cause, why, underlying, fundamental, systemic, deep, origin
version: 1.0.0
behaviors: [trace_error, investigate_codebase, reason]
dependencies: [debugging/systematic]
token_estimate:
  min: 2000
  typical: 4500
  max: 10000
---

# 🔬 Root Cause Analysis Skill

> **Philosophy:** Don't stop at the first "why" — dig until you hit bedrock.

## When to Use

Use this skill when:
- Systematic debugging found the bug but not WHY it exists
- Issue keeps recurring despite fixes
- Bug seems to have multiple contributing factors
- You need to prevent similar bugs in the future
- There's a systemic/architectural issue suspected

**Do NOT use this skill when:**
- Bug is simple and obvious
- Time is extremely limited (use quick-fix)
- Just need to patch, not understand

---

## Prerequisites

Before starting:
- [ ] Bug has been identified (what is happening)
- [ ] Have access to relevant code and history
- [ ] Understand the system architecture (high level)
- [ ] Have time for thorough analysis (~30-60 mins)

---

## Process

### Phase 1: PROBLEM DEFINITION 📝

**Goal:** Clearly define what we're analyzing.

**Steps:**

1. **State the Problem Precisely**
   ```
   Template:
   "When [condition], the system [actual behavior] 
    instead of [expected behavior]."
   
   Example:
   "When a user submits a login form with special characters,
    the system returns a 500 error instead of validating input."
   ```

2. **Gather Impact Data**
   - How often does it occur?
   - Who/what is affected?
   - What's the business impact?
   - How long has it been happening?

3. **Document Timeline**
   - When did it first appear?
   - Any recent changes before first occurrence?
   - Has it gotten better/worse?

**Output:** Clear problem statement with context.

---

### Phase 2: THE 5 WHYS ANALYSIS 🔍

**Goal:** Drill down to fundamental causes.

**Method:**

```
Start: Problem Statement
  │
  ├─ Why? → First-level cause
  │   │
  │   ├─ Why? → Second-level cause
  │   │   │
  │   │   ├─ Why? → Third-level cause
  │   │   │   │
  │   │   │   ├─ Why? → Fourth-level cause
  │   │   │   │   │
  │   │   │   │   └─ Why? → ROOT CAUSE
```

**Rules:**
1. Each answer must be factual, not speculative
2. If multiple answers possible at a level, branch and explore all
3. Stop when you reach something actionable
4. "Human error" is NEVER a root cause — dig deeper

**Example:**

```
Problem: Login fails with special characters

Why #1: Server returns 500 error
  → Because: Unhandled exception in auth.service.ts

Why #2: Why is there an unhandled exception?
  → Because: SQL query fails with syntax error

Why #3: Why does SQL have syntax error?
  → Because: User input is concatenated directly into query

Why #4: Why is input concatenated directly?
  → Because: Developer didn't use parameterized queries

Why #5: Why didn't developer use parameterized queries?
  → Because: No code review caught it, and no security guidelines exist

ROOT CAUSE: Missing security coding standards and review process
```

---

### Phase 3: FISHBONE DIAGRAM (ISHIKAWA) 📊

**Goal:** Explore contributing factors systematically.

**Categories to Examine:**

```
                              ┌──────────┐
          ┌──────────────────►│          │
          │    Environment    │          │
          │                   │          │
┌─────────┴───┐               │  PROBLEM │
│   Methods   ├──────────────►│          │
└─────────────┘               │          │
                              │          │
┌─────────────┐               │          │
│  Machines   ├──────────────►│          │
│  (Systems)  │               │          │
└─────────────┘               └─────┬────┘
                                    │
         ┌───────────────────────────┘
         │
    ┌────┴────┐    ┌──────────┐    ┌──────────┐
    │ People  │    │Materials │    │Measurement│
    │(Process)│    │  (Data)  │    │ (Metrics)│
    └─────────┘    └──────────┘    └──────────┘
```

**For Each Category, Ask:**

| Category | Questions to Ask |
|----------|------------------|
| **Methods** | Is the process correct? Is it followed? Is it documented? |
| **Machines** | Is the system configured correctly? Dependencies up to date? |
| **Environment** | Dev vs Prod differences? External factors? |
| **People/Process** | Training adequate? Communication clear? Handoffs smooth? |
| **Materials/Data** | Data quality? Input validation? Edge cases? |
| **Measurement** | Are we monitoring correctly? Are we measuring the right things? |

---

### Phase 4: CONTRIBUTING FACTOR ANALYSIS 🧩

**Goal:** Weight and prioritize contributing factors.

**Steps:**

1. **List All Contributing Factors**
   Combine findings from 5 Whys and Fishbone

2. **Score Each Factor**
   ```yaml
   scoring:
     frequency: 1-5 (how often does this contribute?)
     detectability: 1-5 (how hard to detect? 5=very hidden)
     severity: 1-5 (how much impact when it contributes?)
     
     risk_score: frequency × detectability × severity
   ```

3. **Create Priority Matrix**
   ```
   High Frequency + High Severity → Address immediately
   High Frequency + Low Severity → Address soon
   Low Frequency + High Severity → Create safeguards
   Low Frequency + Low Severity → Monitor only
   ```

---

### Phase 5: ROOT CAUSE VALIDATION ✅

**Goal:** Confirm the root cause is correct.

**Validation Questions:**

1. **Causation Test**
   - If we fix this, will the problem definitely not recur?
   - Can we prove cause → effect relationship?

2. **Completeness Test**
   - Are there other causes we might have missed?
   - Would fixing this alone be sufficient?

3. **Actionability Test**
   - Can we actually address this cause?
   - Is it within our control?

4. **Proportionality Test**
   - Is the root cause proportional to the problem?
   - (Big problems usually have big root causes)

---

### Phase 6: PREVENTION RECOMMENDATIONS 🛡️

**Goal:** Prevent recurrence.

**Recommendation Types:**

1. **Immediate Fix**
   - Direct fix for the symptom
   - Buys time for proper solution

2. **Root Cause Fix**
   - Addresses the fundamental cause
   - Prevents this exact issue

3. **Systemic Improvement**
   - Prevents entire class of similar issues
   - Usually involves process/tooling changes

4. **Detection Improvement**
   - Catch similar issues earlier next time
   - Monitoring, testing, review improvements

**Example Recommendations:**

```yaml
for_the_sql_injection_example:
  immediate:
    - Fix the specific query to use parameters
    - Add input sanitization
  
  root_cause:
    - Establish secure coding guidelines
    - Require security review for auth code
  
  systemic:
    - Enable SQL injection detection in SAST tooling
    - Add security-focused code review checklist
    - Security training for developers
  
  detection:
    - Add SQL injection tests to CI pipeline
    - Monitor for unusual database queries
```

---

## Output Template

```markdown
# Root Cause Analysis Report

## Problem Statement
[Clear statement of the problem]

## Timeline
- First observed: [date]
- Recent changes: [list]
- Frequency: [how often]

## 5 Whys Analysis
1. Why? → [answer]
2. Why? → [answer]
3. Why? → [answer]
4. Why? → [answer]
5. Why? → [answer]

## Contributing Factors
| Factor | Category | Risk Score | Priority |
|--------|----------|------------|----------|
| [factor] | [cat] | [score] | [priority] |

## Root Cause
[Clear statement of root cause]

## Validation
- [ ] Causation confirmed
- [ ] Complete (no other causes)
- [ ] Actionable
- [ ] Proportional

## Recommendations
### Immediate
- [action]

### Root Cause Fix
- [action]

### Systemic
- [action]

### Detection
- [action]
```

---

## Guidelines

### DO ✅
- Follow the evidence, not assumptions
- Keep asking "why" until you can't anymore
- Document everything for future reference
- Involve domain experts when needed
- Look for patterns across similar issues

### DON'T ❌
- Blame individuals (focus on systems)
- Stop at the first plausible answer
- Skip validation steps
- Propose fixes before understanding cause
- Ignore contributing factors

---

## Success Criteria

Before claiming analysis complete:

- [ ] Problem clearly defined with impact
- [ ] 5 Whys completed to true root cause
- [ ] Contributing factors identified and scored
- [ ] Root cause validated against all tests
- [ ] Prevention recommendations at all levels
- [ ] Report documented for future reference

---

## Related Skills

- `skills/kilo-kit/debugging/systematic/` - For initial bug identification
- `skills/kilo-kit/debugging/verification/` - For validating fixes
- `skills/kilo-kit/quality/code-review/` - For review improvements

---

*Root Cause Analysis Skill v1.0.0 — Dig until you hit bedrock*

---
name: systematic-debugging
description: >-
  Comprehensive 4-phase debugging methodology for complex bugs.
  Use for bugs that aren't immediately obvious or have resisted quick fixes.
  Keywords: bug, error, fix, debug, broken, crash, fail, exception
version: 1.0.0
behaviors: [parse_error, search_code, reason, validate, write_file]
dependencies: []
token_estimate:
  min: 1500
  typical: 3500
  max: 8000
---

# 🔍 Systematic Debugging Skill

> **Philosophy:** Understand before you fix. Verify before you celebrate.

## When to Use

Use this skill when:
- Bug is not immediately obvious
- Previous quick fixes have failed
- Error involves multiple components
- Need to ensure no regression from fix
- Production issue requiring careful handling

**Do NOT use this skill when:**
- Bug is trivial (typo, missing import)
- Just need a quick syntax check
- Issue is really a feature request

---

## Prerequisites

Before starting:
- [ ] Have access to the codebase
- [ ] Can reproduce the bug OR have error logs
- [ ] Understand the expected behavior
- [ ] Know which environment is affected

---

## Process

### Phase 1: ROOT CAUSE INVESTIGATION 🔬

**Goal:** Find the TRUE cause, not just symptoms.

**Steps:**

1. **Reproduce the Bug**
   ```
   Ask yourself:
   - Can I make this happen consistently?
   - What are the exact steps?
   - What input causes it?
   ```

2. **Gather Evidence**
   - Error messages (complete, not truncated)
   - Stack traces
   - Relevant logs
   - Recent changes (git log, blame)

3. **Trace the Flow**
   ```
   Start from: Error location
   Work backward: How did we get here?
   Find: Where does expected != actual?
   ```

4. **Identify the Root Cause**
   - Not "it crashed" but "WHY it crashed"
   - Not "wrong output" but "WHAT produced wrong output"

**Output:** Clear statement of root cause.

**Red Flags (STOP if you find yourself doing these):**
- ❌ Immediately jumping to code changes
- ❌ Assuming you know the cause without evidence
- ❌ Changing things randomly hoping they'll work
- ❌ Ignoring the stack trace

---

### Phase 2: PATTERN ANALYSIS 📊

**Goal:** Understand the shape of the problem.

**Steps:**

1. **Scope Assessment**
   - Is this isolated or systemic?
   - Does it affect other components?
   - Has this happened before?

2. **Similar Pattern Search**
   ```bash
   # Search for similar patterns in codebase
   grep -r "similar_pattern" src/
   ```

3. **Dependency Check**
   - What depends on the broken code?
   - What does the broken code depend on?

4. **Risk Assessment**
   - What could break if we fix this?
   - Are there other places with same bug?

**Output:** Understanding of bug's scope and risk.

---

### Phase 3: HYPOTHESIS & TESTING 🧪

**Goal:** Develop and test fix hypothesis.

**Steps:**

1. **Form Hypothesis**
   ```
   "If I change X to Y, then Z should work because..."
   ```
   
   **Good hypothesis includes:**
   - Specific change
   - Expected outcome
   - Reasoning

2. **Design the Fix**
   - Minimal change principle
   - Don't fix what isn't broken
   - Consider edge cases

3. **Mental/Paper Test**
   - Walk through the code with fix
   - Does it address root cause?
   - Any side effects?

4. **Create Test Case**
   - Before implementing fix, write test that fails
   - Test should pass after fix
   - Include edge cases

**Output:** Tested hypothesis ready for implementation.

---

### Phase 4: IMPLEMENTATION & VERIFICATION ✅

**Goal:** Apply fix and verify it works.

**Steps:**

1. **Implement the Fix**
   - Apply minimal necessary changes
   - Add comments explaining WHY (not just what)
   - Follow project coding standards

2. **Run Existing Tests**
   ```bash
   # All tests must pass
   npm test    # or pytest, etc.
   ```

3. **Run New Test Case**
   - Test you wrote in Phase 3 should now pass
   - Verify fix addresses root cause

4. **Verify No Regression**
   - Related functionality still works
   - No new errors introduced
   - Performance not degraded

5. **Documentation**
   - Update relevant docs if needed
   - Consider adding to known issues/patterns

**Output:** Verified fix with passing tests.

---

## Debugging Decision Tree

```
Bug Report Received
        │
        ▼
┌─────────────────┐
│ Can reproduce?  │
└────────┬────────┘
    YES  │  NO
         │  └──► Gather more info, check logs, ask for steps
         ▼
┌─────────────────┐
│ Error obvious?  │
└────────┬────────┘
    YES  │  NO
         │  └──► Phase 1: Root Cause Investigation
         ▼
Quick fix
         │
         ▼
┌─────────────────┐
│ Fix verified?   │
└────────┬────────┘
    YES  │  NO
         │  └──► Return to investigation
         ▼
    Complete ✓
```

---

## Guidelines

### DO ✅
- Read the full stack trace
- Reproduce before fixing
- Change one thing at a time
- Test after each change
- Keep notes during investigation
- Ask "WHY" at least 3 times

### DON'T ❌
- Assume you know the cause
- Make multiple changes at once
- Skip testing
- Ignore "unrelated" errors
- Rush to deploy fix
- Delete old code without understanding it

---

## Common Patterns

### Pattern: Off-by-One Error

**When:** Array/loop issues, boundary conditions

**Detection:**
- Works for most cases, fails at edges
- Index out of bounds errors
- Missing first/last item

**Solution:**
- Check `<` vs `<=`
- Check starting index (0 vs 1)
- Verify array length handling

### Pattern: Async/Timing Issue

**When:** Race conditions, inconsistent failures

**Detection:**
- Works sometimes, fails other times
- Different behavior in different environments
- Issues with "slow" operations

**Solution:**
- Add proper await/promises
- Check for missing error handling
- Consider operation ordering

### Pattern: Null/Undefined Reference

**When:** Missing data, optional fields

**Detection:**
- "Cannot read property of undefined"
- Works with some data, not others
- Fails after recent data changes

**Solution:**
- Add null checks
- Use optional chaining (?.)
- Validate data at entry points

---

## Anti-Patterns (AVOID)

### Anti-Pattern: "It Works on My Machine"

**Problem:** Not investigating environment differences.

**Instead:** 
- Compare environments systematically
- Check versions, configs, dependencies
- Document environment requirements

### Anti-Pattern: "Shotgun Debugging"

**Problem:** Changing random things hoping something works.

**Instead:**
- Form hypothesis first
- Test ONE change at a time
- Keep track of what you tried

### Anti-Pattern: "It's Fixed Now" (No Verification)

**Problem:** Assuming fix works without testing.

**Instead:**
- Always run tests
- Verify the exact scenario that failed
- Check for regressions

---

## Success Criteria

Before claiming the bug is fixed:

- [ ] Root cause identified and documented
- [ ] Fix addresses root cause (not just symptom)
- [ ] Test case written that would catch this bug
- [ ] All existing tests pass
- [ ] New test passes
- [ ] No regressions in related functionality
- [ ] Fix is reviewable (clear, minimal, documented)
- [ ] User can verify the fix works

---

## Related Skills

- `skills/kilo-kit/debugging/root-cause/` - For deeper root cause analysis
- `skills/kilo-kit/debugging/verification/` - For thorough verification
- `skills/kilo-kit/quality/testing/` - For writing better tests

---

*Systematic Debugging Skill v1.0.0 — Understand before you fix*

---
name: fix-verification
description: >-
  Comprehensive fix verification methodology to ensure bugs are truly fixed.
  Use after implementing any bug fix to verify it works and hasn't caused regressions.
  Keywords: verify, confirm, test, validate, check, ensure, regression, fixed
version: 1.0.0
behaviors: [test_change, run_command, compare, reason]
dependencies: []
token_estimate:
  min: 1000
  typical: 2500
  max: 5000
---

# ✅ Fix Verification Skill

> **Philosophy:** A bug isn't fixed until it's verified. Twice.

## When to Use

Use this skill when:
- You've implemented a bug fix
- Someone else's fix needs verification
- Deploying a fix to production
- Bug was high-impact and needs thorough verification
- Previous fixes for this bug have failed

**Do NOT use this skill when:**
- Just exploring code (no fix yet)
- Bug is trivial (e.g., typo fix)
- Running standard CI (automated verification)

---

## Prerequisites

Before starting:
- [ ] Fix has been implemented
- [ ] You know the expected behavior
- [ ] You can reproduce the original bug (or have a failing test)
- [ ] You have access to test environment

---

## Process

### Phase 1: DIRECT VERIFICATION 🎯

**Goal:** Confirm the fix addresses the exact reported issue.

**Steps:**

1. **Recreate the Original Bug Scenario**
   ```
   Exact steps that caused the bug:
   1. [step 1]
   2. [step 2]
   3. [step 3]
   
   Expected result: [what should happen]
   Previous result: [what was happening - the bug]
   ```

2. **Execute Test with Fix Applied**
   - Follow exact same steps
   - Document actual result
   - Compare to expected

3. **Verify the FIX, Not Just Absence of Error**
   ```
   ❌ "It doesn't crash anymore" (incomplete)
   ✅ "It returns the expected user object with correct fields" (complete)
   ```

4. **Test Multiple Times**
   - Run at least 3 times
   - Note any inconsistency

**Verification Checklist:**
- [ ] Original bug scenario no longer produces error
- [ ] Expected behavior now occurs
- [ ] Result is consistent across multiple runs
- [ ] All variations of bug scenario work

---

### Phase 2: EDGE CASE VERIFICATION 🔲

**Goal:** Ensure fix works for edge cases and variations.

**Steps:**

1. **Identify Edge Cases**
   ```yaml
   edge_cases:
     boundary_values:
       - Empty input
       - Maximum length input
       - Minimum valid input
       - Just over limit
       - Just under limit
     
     special_inputs:
       - Null/undefined
       - Special characters
       - Unicode/emoji
       - Numbers as strings
       - Whitespace only
     
     state_variations:
       - First time user
       - Returning user
       - Admin user
       - Concurrent users
       - High load
   ```

2. **Create Edge Case Matrix**
   | Edge Case | Input | Expected | Actual | Pass? |
   |-----------|-------|----------|--------|-------|
   | Empty | "" | Error msg | | |
   | Max length | 1000 chars | Success | | |
   | Special chars | "<>&" | Escaped | | |

3. **Test Each Edge Case**
   - Document results
   - Note any failures

**Edge Case Checklist:**
- [ ] All boundary values tested
- [ ] Special inputs handled correctly
- [ ] Different user states work
- [ ] Concurrent access works (if applicable)

---

### Phase 3: REGRESSION TESTING 🔄

**Goal:** Ensure fix hasn't broken anything else.

**Steps:**

1. **Identify Affected Areas**
   ```yaml
   affected_areas:
     directly_affected:
       - The fixed function/component
       - Its callers
       - Its dependencies
     
     indirectly_affected:
       - Related features
       - Shared utilities used
       - Configuration changes
   ```

2. **Run Targeted Tests**
   ```bash
   # Run tests for affected module
   npm test -- --grep "auth"
   
   # Run integration tests
   npm run test:integration
   ```

3. **Run Full Test Suite**
   ```bash
   # Ensure no regressions anywhere
   npm test
   ```

4. **Manual Smoke Test**
   - Test main user flows
   - Pay attention to anything that feels different

**Regression Checklist:**
- [ ] All unit tests pass
- [ ] All integration tests pass
- [ ] Smoke test passes
- [ ] No new warnings in logs
- [ ] Performance not degraded

---

### Phase 4: NEGATIVE TESTING 🚫

**Goal:** Verify error handling and failure modes.

**Steps:**

1. **Test Invalid Inputs**
   - What happens with bad data?
   - Are errors handled gracefully?
   - Are error messages helpful?

2. **Test Failure Scenarios**
   ```yaml
   failure_scenarios:
     - Database connection lost
     - API timeout
     - Invalid credentials
     - Missing permissions
     - Disk full
   ```

3. **Test Recovery**
   - Does system recover after failure?
   - Is data consistent after recovery?

**Negative Testing Checklist:**
- [ ] Invalid inputs produce clear errors
- [ ] System fails gracefully
- [ ] Recovery works correctly
- [ ] No security information leaked in errors

---

### Phase 5: ENVIRONMENT VERIFICATION 🌍

**Goal:** Ensure fix works across all environments.

**Steps:**

1. **Test Across Environments**
   | Environment | Tested | Result | Notes |
   |-------------|--------|--------|-------|
   | Local dev | | | |
   | CI/CD | | | |
   | Staging | | | |
   | Production | | | |

2. **Test Across Configurations**
   - Different browsers (if applicable)
   - Different OS (if applicable)
   - Different database sizes
   - Different load levels

3. **Test Across User Types**
   - Regular users
   - Admin users
   - New users
   - Users with existing data

**Environment Checklist:**
- [ ] Works in development
- [ ] Works in staging
- [ ] Works in production (or production-like)
- [ ] Works across configurations

---

### Phase 6: DOCUMENTATION & SIGN-OFF 📝

**Goal:** Document verification and get sign-off.

**Steps:**

1. **Complete Verification Report**
   ```markdown
   ## Fix Verification Report
   
   **Bug ID:** [ID]
   **Fix Description:** [brief description]
   **Verified By:** [name]
   **Date:** [date]
   
   ### Direct Verification
   - [x] Original bug no longer occurs
   - [x] Expected behavior confirmed
   
   ### Edge Cases
   - [x] [edge case 1] - PASS
   - [x] [edge case 2] - PASS
   
   ### Regression Testing
   - [x] Unit tests: 100% pass
   - [x] Integration tests: 100% pass
   - [x] Smoke test: PASS
   
   ### Environment Testing
   - [x] Local: PASS
   - [x] Staging: PASS
   - [x] Production: [pending/pass]
   
   ### Sign-off
   - [ ] Developer
   - [ ] QA (if applicable)
   - [ ] Stakeholder (for critical bugs)
   ```

2. **Update Bug Tracking**
   - Change status to "Verified Fixed"
   - Add verification notes
   - Link to any new tests added

3. **Add/Update Tests**
   - Ensure test exists for this bug
   - Test should fail without fix, pass with fix

---

## Quick Verification Checklist

For simpler bugs, use this shortened checklist:

```markdown
## Quick Verification

- [ ] Original bug scenario fixed
- [ ] Edge cases work
- [ ] All tests pass
- [ ] No new warnings/errors
- [ ] Reviewed by another person (if critical)
```

---

## Verification Decision Tree

```
Fix Implemented
      │
      ▼
┌─────────────────┐
│ Does original   │
│ scenario work?  │
└────────┬────────┘
    YES  │  NO → Back to debugging
         ▼
┌─────────────────┐
│ Edge cases      │
│ work?           │
└────────┬────────┘
    YES  │  NO → Expand fix
         ▼
┌─────────────────┐
│ All tests       │
│ pass?           │
└────────┬────────┘
    YES  │  NO → Fix regressions
         ▼
┌─────────────────┐
│ Works in all    │
│ environments?   │
└────────┬────────┘
    YES  │  NO → Environment-specific fix
         ▼
   VERIFIED ✅
```

---

## Guidelines

### DO ✅
- Verify the fix, not just absence of error
- Test edge cases thoroughly
- Document your verification
- Run the full test suite
- Have someone else verify critical fixes

### DON'T ❌
- Assume fix works because code looks right
- Skip edge case testing
- Forget to check for regressions
- Test only in development environment
- Skip documentation

---

## Common Pitfalls

### "Works on My Machine"

**Problem:** Fix works locally but fails elsewhere.

**Prevention:**
- Always test in staging
- Use same data/config as production
- Test with production-like load

### "Fixed the Symptom, Not the Bug"

**Problem:** Error is gone but behavior is still wrong.

**Prevention:**
- Verify POSITIVE behavior, not just absence of error
- Compare to expected behavior documentation
- Test the complete user flow

### "Created a New Bug"

**Problem:** Fix introduced a regression.

**Prevention:**
- Always run full test suite
- Manual smoke test of related features
- Review changes with fresh eyes

---

## Success Criteria

Before marking bug as fixed:

- [ ] Original issue verified fixed
- [ ] All edge cases pass
- [ ] No regression in tests
- [ ] Works in all environments
- [ ] Verification documented
- [ ] Test added to prevent recurrence

---

## Related Skills

- `skills/kilo-kit/debugging/systematic/` - For finding the bug
- `skills/kilo-kit/debugging/root-cause/` - For understanding why it happened
- `skills/kilo-kit/quality/testing/` - For writing better tests

---

*Fix Verification Skill v1.0.0 — Verified twice, deployed once*

---
name: backend-api-development
description: >-
  Comprehensive backend API development skill for building robust, scalable APIs.
  Use when creating new endpoints, services, or backend functionality.
  Keywords: API, backend, endpoint, service, REST, GraphQL, server, controller, route
version: 1.0.0
behaviors: [generate_with_validation, investigate_codebase, document_code, test_change]
dependencies: []
token_estimate:
  min: 2000
  typical: 5000
  max: 12000
---

# 🔧 Backend API Development Skill

> **Philosophy:** APIs are contracts. Build them right the first time.

## When to Use

Use this skill when:
- Creating a new API endpoint
- Building a new service/module
- Refactoring existing API code
- Adding new functionality to backend
- Need to follow RESTful/GraphQL best practices

**Do NOT use this skill when:**
- Just fixing a small bug (use debugging skill)
- Only modifying frontend (use frontend skill)
- Database-only changes (use database skill)

---

## Prerequisites

Before starting:
- [ ] Requirements are clear (what the API should do)
- [ ] Understand the existing architecture
- [ ] Know the target stack (NestJS, Express, FastAPI, etc.)
- [ ] Database schema exists (or will be created)

---

## Process

### Phase 1: DESIGN 📐

**Goal:** Design the API before writing code.

**Steps:**

1. **Define the Resource**
   ```yaml
   resource:
     name: User
     description: Represents a platform user
     domain: authentication
   ```

2. **Design Endpoints (REST)**
   ```yaml
   endpoints:
     - method: GET
       path: /users
       description: List all users
       query_params: [page, limit, search]
       response: User[]
     
     - method: GET
       path: /users/:id
       description: Get single user
       response: User
     
     - method: POST
       path: /users
       description: Create new user
       body: CreateUserDto
       response: User
     
     - method: PUT
       path: /users/:id
       description: Update user
       body: UpdateUserDto
       response: User
     
     - method: DELETE
       path: /users/:id
       description: Delete user
       response: void
   ```

3. **Define DTOs (Data Transfer Objects)**
   ```typescript
   // CreateUserDto
   interface CreateUserDto {
     email: string;      // required, email format
     password: string;   // required, min 8 chars
     name: string;       // required, min 2 chars
     role?: UserRole;    // optional, default: 'user'
   }
   
   // UpdateUserDto
   type UpdateUserDto = Partial<CreateUserDto>;
   
   // UserResponseDto
   interface UserResponseDto {
     id: string;
     email: string;
     name: string;
     role: UserRole;
     createdAt: DateTime;
     updatedAt: DateTime;
     // Note: password NOT included
   }
   ```

4. **Plan Error Responses**
   ```yaml
   errors:
     - code: 400
       when: Invalid input
       response: { message, errors: [{field, message}] }
     
     - code: 401
       when: Not authenticated
       response: { message: "Unauthorized" }
     
     - code: 403
       when: Not authorized
       response: { message: "Forbidden" }
     
     - code: 404
       when: Resource not found
       response: { message: "User not found" }
     
     - code: 409
       when: Conflict (e.g., email exists)
       response: { message: "Email already registered" }
   ```

**Output:** Complete API design document.

---

### Phase 2: STRUCTURE 🏗️

**Goal:** Set up the file structure.

**NestJS Structure:**
```
src/
└── users/
    ├── users.module.ts       # Module definition
    ├── users.controller.ts   # HTTP layer
    ├── users.service.ts      # Business logic
    ├── users.repository.ts   # Data access (optional)
    ├── dto/
    │   ├── create-user.dto.ts
    │   ├── update-user.dto.ts
    │   └── user-response.dto.ts
    ├── entities/
    │   └── user.entity.ts
    ├── guards/
    │   └── user-owner.guard.ts
    └── users.controller.spec.ts
```

**FastAPI Structure:**
```
app/
└── users/
    ├── __init__.py
    ├── router.py          # Routes
    ├── service.py         # Business logic
    ├── repository.py      # Data access
    ├── schemas.py         # Pydantic models
    ├── models.py          # SQLAlchemy models
    └── dependencies.py    # Dependency injection
```

---

### Phase 3: IMPLEMENTATION 💻

**Goal:** Implement the API layer by layer.

**Order of Implementation:**

1. **Entity/Model First**
   ```typescript
   // user.entity.ts
   @Entity('users')
   export class User {
     @PrimaryGeneratedColumn('uuid')
     id: string;
     
     @Column({ unique: true })
     @IsEmail()
     email: string;
     
     @Column()
     @Exclude() // Never expose password
     password: string;
     
     @Column()
     name: string;
     
     @Column({ default: 'user' })
     role: UserRole;
     
     @CreateDateColumn()
     createdAt: Date;
     
     @UpdateDateColumn()
     updatedAt: Date;
   }
   ```

2. **DTOs with Validation**
   ```typescript
   // create-user.dto.ts
   export class CreateUserDto {
     @IsEmail()
     @Transform(({ value }) => value.toLowerCase().trim())
     email: string;
     
     @IsString()
     @MinLength(8)
     @Matches(/^(?=.*[A-Za-z])(?=.*\d)/, {
       message: 'Password must contain letters and numbers'
     })
     password: string;
     
     @IsString()
     @MinLength(2)
     @MaxLength(50)
     name: string;
     
     @IsOptional()
     @IsEnum(UserRole)
     role?: UserRole;
   }
   ```

3. **Service Layer (Business Logic)**
   ```typescript
   // users.service.ts
   @Injectable()
   export class UsersService {
     constructor(
       @InjectRepository(User)
       private usersRepository: Repository<User>,
     ) {}
     
     async create(dto: CreateUserDto): Promise<User> {
       // Check for existing email
       const existing = await this.findByEmail(dto.email);
       if (existing) {
         throw new ConflictException('Email already registered');
       }
       
       // Hash password
       const hashedPassword = await bcrypt.hash(dto.password, 10);
       
       // Create and save
       const user = this.usersRepository.create({
         ...dto,
         password: hashedPassword,
       });
       
       return this.usersRepository.save(user);
     }
     
     async findAll(options: PaginationOptions): Promise<PaginatedResult<User>> {
       // Implementation with pagination
     }
     
     // ... other methods
   }
   ```

4. **Controller (HTTP Layer)**
   ```typescript
   // users.controller.ts
   @Controller('users')
   @UseInterceptors(ClassSerializerInterceptor)
   export class UsersController {
     constructor(private readonly usersService: UsersService) {}
     
     @Post()
     @HttpCode(HttpStatus.CREATED)
     async create(@Body() dto: CreateUserDto): Promise<UserResponseDto> {
       const user = await this.usersService.create(dto);
       return plainToInstance(UserResponseDto, user);
     }
     
     @Get()
     @UseGuards(AuthGuard)
     async findAll(
       @Query() query: PaginationQueryDto
     ): Promise<PaginatedResult<UserResponseDto>> {
       return this.usersService.findAll(query);
     }
     
     @Get(':id')
     @UseGuards(AuthGuard)
     async findOne(@Param('id', ParseUUIDPipe) id: string): Promise<UserResponseDto> {
       const user = await this.usersService.findOne(id);
       if (!user) {
         throw new NotFoundException('User not found');
       }
       return plainToInstance(UserResponseDto, user);
     }
     
     // ... other endpoints
   }
   ```

---

### Phase 4: SECURITY 🔒

**Goal:** Ensure API is secure.

**Security Checklist:**

1. **Input Validation**
   - [ ] All inputs validated with DTOs
   - [ ] Types enforced
   - [ ] Length limits set
   - [ ] Format validation (email, UUID, etc.)

2. **Authentication**
   - [ ] Protected routes require authentication
   - [ ] JWT or session validation
   - [ ] Token expiration handled

3. **Authorization**
   - [ ] Role-based access control
   - [ ] Resource ownership verified
   - [ ] Admin-only routes protected

4. **Data Protection**
   - [ ] Passwords hashed (bcrypt, argon2)
   - [ ] Sensitive data not logged
   - [ ] Passwords excluded from responses

5. **Rate Limiting**
   - [ ] Login attempts limited
   - [ ] API rate limiting in place

6. **SQL Injection Prevention**
   - [ ] Parameterized queries used
   - [ ] ORM used correctly
   - [ ] Raw queries avoided or sanitized

---

### Phase 5: TESTING 🧪

**Goal:** Write comprehensive tests.

**Test Types:**

1. **Unit Tests**
   ```typescript
   describe('UsersService', () => {
     describe('create', () => {
       it('should create a new user', async () => {
         const dto = { email: 'test@example.com', ... };
         const result = await service.create(dto);
         expect(result.email).toBe(dto.email);
       });
       
       it('should hash the password', async () => {
         const dto = { password: 'plaintext', ... };
         const result = await service.create(dto);
         expect(result.password).not.toBe(dto.password);
       });
       
       it('should throw on duplicate email', async () => {
         // Setup: create user first
         await service.create({ email: 'test@example.com', ... });
         
         // Act & Assert
         await expect(
           service.create({ email: 'test@example.com', ... })
         ).rejects.toThrow(ConflictException);
       });
     });
   });
   ```

2. **Integration Tests**
   ```typescript
   describe('Users API', () => {
     it('POST /users should create user', async () => {
       const response = await request(app.getHttpServer())
         .post('/users')
         .send({ email: 'test@example.com', password: 'Password1', name: 'Test' })
         .expect(201);
       
       expect(response.body.email).toBe('test@example.com');
       expect(response.body.password).toBeUndefined();
     });
     
     it('GET /users should require auth', async () => {
       await request(app.getHttpServer())
         .get('/users')
         .expect(401);
     });
   });
   ```

---

### Phase 6: DOCUMENTATION 📝

**Goal:** Document the API.

**OpenAPI/Swagger:**
```typescript
@ApiTags('users')
@Controller('users')
export class UsersController {
  @Post()
  @ApiOperation({ summary: 'Create a new user' })
  @ApiResponse({ status: 201, type: UserResponseDto })
  @ApiResponse({ status: 400, description: 'Invalid input' })
  @ApiResponse({ status: 409, description: 'Email already exists' })
  async create(@Body() dto: CreateUserDto): Promise<UserResponseDto> {
    // ...
  }
}
```

**Response DTO Documentation:**
```typescript
export class UserResponseDto {
  @ApiProperty({ example: '550e8400-e29b-41d4-a716-446655440000' })
  id: string;
  
  @ApiProperty({ example: 'user@example.com' })
  email: string;
  
  @ApiProperty({ example: 'John Doe' })
  name: string;
}
```

---

## Best Practices

### API Design

| Practice | Do | Don't |
|----------|----|----- |
| Naming | `GET /users/:id/orders` | `GET /getUserOrders` |
| Versioning | `/api/v1/users` | No versioning |
| Pluralization | `/users`, `/orders` | `/user`, `/order` |
| HTTP Methods | Use correctly (GET=read, POST=create) | POST for everything |
| Status Codes | 201 for created, 204 for no content | 200 for everything |

### Error Handling

```typescript
// Global exception filter
@Catch()
export class AllExceptionsFilter implements ExceptionFilter {
  catch(exception: unknown, host: ArgumentsHost) {
    const ctx = host.switchToHttp();
    const response = ctx.getResponse<Response>();
    
    const status = exception instanceof HttpException
      ? exception.getStatus()
      : HttpStatus.INTERNAL_SERVER_ERROR;
    
    const message = exception instanceof HttpException
      ? exception.message
      : 'Internal server error';
    
    response.status(status).json({
      statusCode: status,
      message,
      timestamp: new Date().toISOString(),
    });
  }
}
```

---

## Guidelines

### DO ✅
- Design API before coding
- Use proper HTTP methods and status codes
- Validate all inputs
- Handle errors gracefully
- Write tests first (TDD)
- Document with OpenAPI

### DON'T ❌
- Expose internal IDs when UUIDs are better
- Return password or sensitive data
- Use GET for mutations
- Skip input validation
- Catch and swallow errors
- Use magic strings/numbers

---

## Success Criteria

Before considering API complete:

- [ ] All endpoints implemented per design
- [ ] Input validation on all endpoints
- [ ] Authentication/Authorization in place
- [ ] Error handling comprehensive
- [ ] Unit tests with >80% coverage
- [ ] Integration tests for main flows
- [ ] API documented (OpenAPI/Swagger)
- [ ] Security checklist passed

---

## Related Skills

- `skills/kilo-kit/development/database/` - For data layer
- `skills/kilo-kit/development/security/` - For security concerns
- `skills/kilo-kit/quality/testing/` - For test coverage
- `skills/kilo-kit/architecture/system-design/` - For architecture decisions

---

*Backend API Development Skill v1.0.0 — APIs built right*

---
name: security-best-practices
description: >-
  Security-focused development skill covering OWASP Top 10 and secure coding.
  Use when implementing authentication, handling user data, or security review.
  Keywords: security, auth, authentication, authorization, OWASP, XSS, SQL injection, CSRF, secure
version: 1.0.0
behaviors: [review_and_suggest, investigate_codebase, generate_with_validation]
dependencies: []
token_estimate:
  min: 1500
  typical: 3500
  max: 8000
---

# 🔐 Security Best Practices Skill

> **Philosophy:** Security is not optional. Build it in from the start.

## When to Use

Use this skill when:
- Implementing authentication/authorization
- Handling user input
- Working with sensitive data
- Doing security code review
- Building user-facing features
- Setting up deployment/infrastructure

**Do NOT use this skill when:**
- Just formatting code
- Pure UI/styling changes
- No user data involved

---

## Prerequisites

Before starting:
- [ ] Understand what data you're handling
- [ ] Know your threat model (who might attack)
- [ ] Have access to codebase
- [ ] Understand the tech stack

---

## OWASP Top 10 Quick Reference

### 1. Broken Access Control (A01:2021)

**What:** Users can access data/functions they shouldn't.

**Prevention:**
```typescript
// ❌ Bad: No authorization check
app.get('/users/:id', async (req, res) => {
  const user = await db.users.findById(req.params.id);
  res.json(user);
});

// ✅ Good: Check ownership
app.get('/users/:id', authorize(), async (req, res) => {
  const user = await db.users.findById(req.params.id);
  
  if (user.id !== req.user.id && req.user.role !== 'admin') {
    throw new ForbiddenException();
  }
  
  res.json(user);
});
```

**Checklist:**
- [ ] Default deny (require explicit permission)
- [ ] Verify ownership of resources
- [ ] Role-based access control implemented
- [ ] Admin functions protected
- [ ] CORS configured correctly

---

### 2. Cryptographic Failures (A02:2021)

**What:** Weak crypto, exposed sensitive data.

**Prevention:**
```typescript
// ❌ Bad: Weak hashing
const hash = crypto.createHash('md5').update(password).digest('hex');

// ✅ Good: Strong hashing with bcrypt
const hash = await bcrypt.hash(password, 12);

// ❌ Bad: Hardcoded secrets
const API_KEY = "sk_live_abc123";

// ✅ Good: Environment variables
const API_KEY = process.env.API_KEY;
```

**Checklist:**
- [ ] Passwords hashed with bcrypt/argon2 (cost factor ≥12)
- [ ] Sensitive data encrypted at rest
- [ ] TLS/HTTPS enforced
- [ ] No hardcoded secrets
- [ ] Secrets in environment variables
- [ ] Old/weak algorithms avoided (MD5, SHA1)

---

### 3. Injection (A03:2021)

**What:** Malicious data executed as code/query.

**Prevention:**
```typescript
// ❌ Bad: SQL Injection
const query = `SELECT * FROM users WHERE email = '${email}'`;

// ✅ Good: Parameterized queries
const user = await db.query(
  'SELECT * FROM users WHERE email = $1',
  [email]
);

// ❌ Bad: Command injection
exec(`convert ${filename} output.png`);

// ✅ Good: Use library functions
await sharp(filename).toFile('output.png');
```

**Types to Prevent:**
- SQL Injection
- NoSQL Injection
- Command Injection
- LDAP Injection
- XPath Injection

**Checklist:**
- [ ] Use parameterized queries/ORM
- [ ] Validate and sanitize all input
- [ ] Escape output appropriately
- [ ] Avoid shell commands with user input
- [ ] Use allow-lists, not block-lists

---

### 4. Insecure Design (A04:2021)

**What:** Missing security in design phase.

**Prevention:**
```yaml
# Security design considerations
threat_modeling:
  assets:
    - User credentials
    - Payment information
    - Personal data
  
  threats:
    - Authentication bypass
    - Data theft
    - Privilege escalation
  
  mitigations:
    - MFA for sensitive operations
    - Encryption at rest
    - Audit logging
```

**Checklist:**
- [ ] Threat model created
- [ ] Security requirements documented
- [ ] Defense in depth applied
- [ ] Fail securely (safe defaults)
- [ ] Separation of duties

---

### 5. Security Misconfiguration (A05:2021)

**What:** Insecure settings, missing hardening.

**Prevention:**
```typescript
// ❌ Bad: Debugging enabled in production
app.use(express.errorHandler({ dumpExceptions: true }));

// ✅ Good: Production-safe error handling
if (process.env.NODE_ENV === 'production') {
  app.use((err, req, res, next) => {
    console.error(err);  // Log internally
    res.status(500).json({ message: 'Internal error' });  // Don't expose details
  });
}
```

**Checklist:**
- [ ] Remove default credentials
- [ ] Disable debugging in production
- [ ] Remove unnecessary features/endpoints
- [ ] Security headers configured
- [ ] Error messages don't leak info
- [ ] File permissions correct

**Security Headers:**
```typescript
app.use(helmet());
// Or manually:
app.use((req, res, next) => {
  res.setHeader('X-Content-Type-Options', 'nosniff');
  res.setHeader('X-Frame-Options', 'DENY');
  res.setHeader('X-XSS-Protection', '1; mode=block');
  res.setHeader('Strict-Transport-Security', 'max-age=31536000');
  res.setHeader('Content-Security-Policy', "default-src 'self'");
  next();
});
```

---

### 6. Vulnerable Components (A06:2021)

**What:** Using libraries with known vulnerabilities.

**Prevention:**
```bash
# Check for vulnerabilities
npm audit
pip-audit
dotnet list package --vulnerable

# Fix vulnerabilities
npm audit fix
pip-audit --fix
```

**Checklist:**
- [ ] Dependencies up to date
- [ ] Security advisories monitored
- [ ] Automated vulnerability scanning
- [ ] Remove unused dependencies
- [ ] Only use trusted sources

---

### 7. Authentication Failures (A07:2021)

**What:** Broken login, session management.

**Prevention:**
```typescript
// Password requirements
const passwordPolicy = {
  minLength: 12,
  requireUppercase: true,
  requireLowercase: true,
  requireNumber: true,
  requireSpecial: true,
  preventCommon: true,
};

// Rate limiting login attempts
const loginLimiter = rateLimit({
  windowMs: 15 * 60 * 1000, // 15 minutes
  max: 5, // 5 attempts
  message: 'Too many login attempts'
});

// Session configuration
app.use(session({
  secret: process.env.SESSION_SECRET,
  resave: false,
  saveUninitialized: false,
  cookie: {
    secure: true,       // HTTPS only
    httpOnly: true,     // No JS access
    sameSite: 'strict', // CSRF protection
    maxAge: 3600000     // 1 hour
  }
}));
```

**Checklist:**
- [ ] Strong password policy enforced
- [ ] Brute force protection (rate limiting)
- [ ] MFA available for sensitive accounts
- [ ] Secure password reset flow
- [ ] Sessions invalidated on logout
- [ ] Session timeout configured

---

### 8. Software Integrity Failures (A08:2021)

**What:** Insecure updates, CI/CD pipeline attacks.

**Prevention:**
```yaml
# Verify package integrity
package-lock.json  # Lock versions
npm ci             # Install exact versions

# CI/CD security
ci_security:
  - Verify source code integrity
  - Sign releases
  - Secure deployment pipeline
  - Review third-party actions
```

**Checklist:**
- [ ] Lock file used and committed
- [ ] Packages verified (checksums)
- [ ] CI/CD pipeline secured
- [ ] Code signing for releases

---

### 9. Logging Failures (A09:2021)

**What:** Insufficient logging for security events.

**Prevention:**
```typescript
// Security event logging
const securityLogger = {
  loginSuccess: (userId: string, ip: string) => {
    logger.info('LOGIN_SUCCESS', { userId, ip, timestamp: new Date() });
  },
  
  loginFailure: (email: string, ip: string, reason: string) => {
    logger.warn('LOGIN_FAILURE', { email, ip, reason, timestamp: new Date() });
  },
  
  accessDenied: (userId: string, resource: string, ip: string) => {
    logger.warn('ACCESS_DENIED', { userId, resource, ip, timestamp: new Date() });
  },
  
  suspiciousActivity: (details: object) => {
    logger.error('SUSPICIOUS_ACTIVITY', { ...details, timestamp: new Date() });
  }
};

// Log what to log
// ✅ Login attempts (success and failure)
// ✅ Access control failures
// ✅ Input validation failures
// ✅ Security configuration changes
// ✅ High-value transactions

// ❌ Don't log
// Passwords
// Session tokens
// Credit card numbers
// Personal data (unless necessary)
```

**Checklist:**
- [ ] Security events logged
- [ ] Log format is parseable
- [ ] Logs protected from tampering
- [ ] Sensitive data not logged
- [ ] Alerting on suspicious patterns

---

### 10. SSRF (A10:2021)

**What:** Server-Side Request Forgery.

**Prevention:**
```typescript
// ❌ Bad: User-controlled URL
const response = await fetch(req.body.url);

// ✅ Good: Validate and restrict
const ALLOWED_DOMAINS = ['api.example.com', 'cdn.example.com'];

async function fetchUrl(userUrl: string) {
  const parsed = new URL(userUrl);
  
  if (!ALLOWED_DOMAINS.includes(parsed.hostname)) {
    throw new Error('Domain not allowed');
  }
  
  if (parsed.protocol !== 'https:') {
    throw new Error('HTTPS required');
  }
  
  return fetch(userUrl);
}
```

**Checklist:**
- [ ] Validate user-supplied URLs
- [ ] Use allow-lists for domains
- [ ] Block internal/private IPs
- [ ] Disable HTTP redirects (or limit)

---

## Input Validation Patterns

### Universal Validation

```typescript
// Validation with Zod
const UserSchema = z.object({
  email: z.string().email().toLowerCase().trim(),
  password: z.string().min(12).max(128),
  name: z.string().min(2).max(50).regex(/^[a-zA-Z\s]+$/),
  age: z.number().int().min(13).max(120).optional(),
});

// Validation with class-validator
class CreateUserDto {
  @IsEmail()
  @Transform(({ value }) => value.toLowerCase().trim())
  email: string;
  
  @IsString()
  @MinLength(12)
  @MaxLength(128)
  @Matches(/^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*[@$!%*?&])/)
  password: string;
  
  @IsString()
  @MinLength(2)
  @MaxLength(50)
  name: string;
}
```

### XSS Prevention

```typescript
// ❌ Bad: Raw HTML output
element.innerHTML = userInput;

// ✅ Good: Text content only
element.textContent = userInput;

// ✅ Good: Sanitize if HTML needed
import DOMPurify from 'dompurify';
element.innerHTML = DOMPurify.sanitize(userInput);
```

---

## Security Testing Checklist

```yaml
security_tests:
  authentication:
    - Test login with invalid credentials
    - Test brute force protection
    - Test session timeout
    - Test logout clears session
    - Test password reset flow
  
  authorization:
    - Test accessing other users' data
    - Test admin functions as normal user
    - Test direct object references
    - Test privilege escalation
  
  input_validation:
    - Test SQL injection payloads
    - Test XSS payloads
    - Test command injection
    - Test path traversal
    - Test file upload restrictions
  
  configuration:
    - Test HTTPS enforcement
    - Test security headers present
    - Test error messages sanitized
    - Test debugging disabled
```

---

## Guidelines

### DO ✅
- Validate all input
- Use parameterized queries
- Hash passwords with bcrypt/argon2
- Log security events
- Keep dependencies updated
- Apply principle of least privilege

### DON'T ❌
- Trust user input
- Store secrets in code
- Use weak cryptography
- Expose detailed errors
- Ignore security warnings
- Skip security testing

---

## Success Criteria

Before considering code secure:

- [ ] OWASP Top 10 addressed
- [ ] Input validation complete
- [ ] Authentication/authorization tested
- [ ] Secrets managed properly
- [ ] Security headers configured
- [ ] Dependencies audited
- [ ] Security logging in place
- [ ] Code reviewed for security

---

## Related Skills

- `skills/kilo-kit/development/backend/` - For API security
- `skills/kilo-kit/quality/code-review/` - For security review
- `skills/kilo-kit/debugging/root-cause/` - For security incident analysis

---

*Security Best Practices Skill v1.0.0 — Security is everyone's job*

---
name: code-review
description: >-
  Comprehensive code review checklist and methodology.
  Use when reviewing PRs, conducting code audits, or assessing code quality.
  Keywords: review, PR, code review, audit, assess, quality, check
version: 1.0.0
behaviors: [read_file, reason, validate]
dependencies: []
token_estimate:
  min: 1000
  typical: 2500
  max: 5000
---

# 👁️ Code Review Skill

> **Philosophy:** Code review is collaboration, not criticism.

## When to Use

Use this skill when:
- Reviewing a Pull Request
- Conducting a code audit
- Assessing code quality before merge
- Mentoring through code feedback
- Preparing code for production

**Do NOT use this skill when:**
- Just need to run linter
- Simple typo fix
- Automated formatting changes only

---

## Prerequisites

Before starting review:
- [ ] Understand the purpose/goal of the change
- [ ] Have context on the project architecture
- [ ] Know the coding standards for the project
- [ ] Can run the code locally (if needed)

---

## Process

### Phase 1: CONTEXT UNDERSTANDING 📋

**Goal:** Understand WHAT and WHY before HOW.

**Steps:**

1. **Read the PR Description**
   - What problem does this solve?
   - What approach was taken?
   - Are there any caveats noted?

2. **Check Related Issues**
   - Link to issue/ticket
   - Requirements met?
   - Edge cases addressed?

3. **Assess Scope**
   - How many files changed?
   - Is this focused or sprawling?
   - Should this be multiple PRs?

**Output:** Clear understanding of change purpose.

---

### Phase 2: HIGH-LEVEL REVIEW 🔭

**Goal:** Evaluate architecture and design decisions.

**Checklist:**

```
DESIGN
□ Does the solution make sense?
□ Is this the right place for this code?
□ Does it follow project patterns?
□ Is it over-engineered?
□ Is it under-engineered?

ARCHITECTURE
□ Proper separation of concerns?
□ Dependencies going the right direction?
□ New dependencies justified?
□ Breaking any architectural boundaries?

SCOPE
□ Does change match stated purpose?
□ Any scope creep?
□ Any missing pieces?
```

**Output:** Assessment of overall approach.

---

### Phase 3: LINE-BY-LINE REVIEW 🔍

**Goal:** Examine code quality and correctness.

**Checklist:**

```
CORRECTNESS
□ Logic is correct
□ Edge cases handled
□ Error cases handled
□ Null/undefined handled
□ No off-by-one errors
□ Concurrency issues addressed

QUALITY
□ Clear variable/function names
□ Single responsibility principle
□ DRY (no unnecessary duplication)
□ Comments explain WHY, not WHAT
□ No dead code
□ No commented-out code
□ No TODOs without tracking

SECURITY
□ Input validation
□ No SQL injection risks
□ No XSS risks
□ Secrets not hardcoded
□ Proper authentication checks
□ Authorization verified

PERFORMANCE
□ No obvious N+1 queries
□ Appropriate caching
□ No blocking operations where async needed
□ Large data sets handled efficiently
```

**Output:** Detailed feedback on code quality.

---

### Phase 4: TESTING REVIEW 🧪

**Goal:** Ensure adequate test coverage.

**Checklist:**

```
TEST PRESENCE
□ Tests added for new functionality?
□ Tests updated for modified functionality?
□ Test file naming consistent?

TEST QUALITY
□ Tests are meaningful (not just coverage)?
□ Edge cases tested?
□ Error cases tested?
□ Tests are independent/isolated?
□ No flaky tests introduced?

TEST COVERAGE
□ Happy path covered?
□ Unhappy path covered?
□ Boundary conditions covered?
```

**Output:** Assessment of test adequacy.

---

### Phase 5: FINAL CHECKS ✅

**Goal:** Ensure readiness for merge.

**Checklist:**

```
DOCUMENTATION
□ README updated if needed?
□ API docs updated if needed?
□ Inline comments sufficient?
□ Migration guide if breaking changes?

OPERATIONAL
□ Logs added for debugging?
□ Metrics/monitoring considered?
□ Feature flags if needed?
□ Rollback plan if needed?

MERGE READINESS
□ CI passes?
□ No merge conflicts?
□ Approved by required reviewers?
□ All conversations resolved?
```

**Output:** Clear approve/request changes decision.

---

## Review Comment Guidelines

### Categorize Your Comments

| Prefix | Meaning | Action Required |
|--------|---------|-----------------|
| `🔴 BLOCKER:` | Must fix before merge | Yes, mandatory |
| `🟡 SUGGESTION:` | Should consider | Recommended |
| `🟢 NIT:` | Minor, optional | No |
| `❓ QUESTION:` | Need clarification | Response needed |
| `💡 IDEA:` | Future improvement | No |
| `👍 PRAISE:` | Great work! | No |

### Example Comments

**Good:**
```
🔴 BLOCKER: This SQL query is vulnerable to injection.
Use parameterized queries instead:
`db.query("SELECT * FROM users WHERE id = ?", [userId])`
```

**Bad:**
```
This is wrong.
```

### Tone Guidelines

- ✅ "Consider using X because Y"
- ✅ "What happens if Z is null?"
- ✅ "Great use of pattern X!"
- ❌ "This is stupid"
- ❌ "Obviously you should..."
- ❌ "Why didn't you just..."

---

## Common Issues to Watch For

### Security Issues

| Issue | Detection | Solution |
|-------|-----------|----------|
| SQL Injection | String concatenation in queries | Parameterized queries |
| XSS | Unescaped user input in HTML | Proper escaping/encoding |
| Hardcoded secrets | API keys in code | Environment variables |
| Missing auth | Endpoints without checks | Add auth middleware |

### Performance Issues

| Issue | Detection | Solution |
|-------|-----------|----------|
| N+1 queries | Loop with DB calls | Batch/eager loading |
| Missing index | Slow queries on large tables | Add database index |
| Blocking I/O | Sync calls in async context | Use async/await |
| Memory leak | Unbounded caches/listeners | Cleanup/limits |

### Code Quality Issues

| Issue | Detection | Solution |
|-------|-----------|----------|
| God function | 100+ lines, many responsibilities | Break into smaller functions |
| Magic numbers | `if (status === 3)` | Named constants |
| Deep nesting | 4+ levels of if/for | Early returns, extraction |
| Copy-paste code | Similar blocks repeated | Extract utility function |

---

## Success Criteria

Before approving:

- [ ] I understand what this code does and why
- [ ] The approach is appropriate for the problem
- [ ] Code is correct and handles edge cases
- [ ] Code is secure (no obvious vulnerabilities)
- [ ] Tests are adequate and meaningful
- [ ] Code is readable and maintainable
- [ ] No blocking issues remain
- [ ] All my questions have been answered

---

## Related Skills

- `skills/kilo-kit/quality/testing/` - For test quality guidance
- `skills/kilo-kit/development/security/` - For security review
- `skills/kilo-kit/debugging/systematic/` - If bugs found during review

---

*Code Review Skill v1.0.0 — Collaboration, not criticism*

---
name: testing-strategy
description: >-
  Comprehensive testing skill covering unit, integration, and e2e testing with TDD.
  Use when writing tests, improving coverage, or setting up testing infrastructure.
  Keywords: test, TDD, unit test, integration, e2e, coverage, mock, jest, vitest
version: 1.0.0
behaviors: [generate_with_validation, run_command, review_and_suggest]
dependencies: []
token_estimate:
  min: 1500
  typical: 3500
  max: 8000
---

# 🧪 Testing Strategy Skill

> **Philosophy:** If it's not tested, it's broken. You just don't know it yet.

## When to Use

Use this skill when:
- Writing new code (TDD approach)
- Adding tests to existing code
- Improving test coverage
- Fixing flaky tests
- Setting up testing infrastructure
- Debugging test failures

**Do NOT use this skill when:**
- Just running existing tests
- Quick syntax check

---

## The Testing Pyramid

```
                 ╱╲
                ╱  ╲
               ╱ E2E╲           Few, slow, expensive
              ╱──────╲          Full system tests
             ╱        ╲
            ╱Integration╲       Medium amount
           ╱────────────╲       Component interaction
          ╱              ╲
         ╱   Unit Tests   ╲     Many, fast, cheap
        ╱──────────────────╲    Single unit isolation
```

---

## TDD Workflow: RED → GREEN → REFACTOR

### Step 1: RED (Write Failing Test)

```typescript
// Write the test BEFORE the implementation
describe('calculateDiscount', () => {
  it('should apply 10% discount for orders over $100', () => {
    // This test will FAIL because function doesn't exist yet
    const result = calculateDiscount(150);
    expect(result).toBe(135);
  });
});
```

**Run test → Should FAIL (RED)**

### Step 2: GREEN (Minimal Implementation)

```typescript
// Write the MINIMUM code to pass the test
function calculateDiscount(amount: number): number {
  if (amount > 100) {
    return amount * 0.9;
  }
  return amount;
}
```

**Run test → Should PASS (GREEN)**

### Step 3: REFACTOR (Improve)

```typescript
// Improve code while keeping tests green
const DISCOUNT_THRESHOLD = 100;
const DISCOUNT_RATE = 0.1;

function calculateDiscount(amount: number): number {
  if (amount > DISCOUNT_THRESHOLD) {
    return amount * (1 - DISCOUNT_RATE);
  }
  return amount;
}
```

**Run test → Should still PASS**

---

## Unit Testing Patterns

### Basic Structure (AAA Pattern)

```typescript
describe('UserService', () => {
  describe('createUser', () => {
    it('should create user with valid data', async () => {
      // Arrange
      const userData = { email: 'test@example.com', name: 'Test' };
      const mockRepo = { create: jest.fn().mockResolvedValue({ id: '1', ...userData }) };
      const service = new UserService(mockRepo);
      
      // Act
      const result = await service.createUser(userData);
      
      // Assert
      expect(result.id).toBe('1');
      expect(result.email).toBe(userData.email);
      expect(mockRepo.create).toHaveBeenCalledWith(userData);
    });
  });
});
```

### Testing Error Cases

```typescript
describe('createUser', () => {
  it('should throw on duplicate email', async () => {
    // Arrange
    const mockRepo = {
      findByEmail: jest.fn().mockResolvedValue({ id: 'existing' }),
    };
    const service = new UserService(mockRepo);
    
    // Act & Assert
    await expect(
      service.createUser({ email: 'exists@example.com' })
    ).rejects.toThrow('Email already registered');
  });
});
```

### Testing Async Code

```typescript
describe('fetchUserData', () => {
  it('should fetch and transform user data', async () => {
    // Arrange
    const mockApi = {
      get: jest.fn().mockResolvedValue({ data: { name: 'John' } }),
    };
    
    // Act
    const result = await fetchUserData(mockApi, 'user-id');
    
    // Assert
    expect(result).toEqual({ name: 'John' });
    expect(mockApi.get).toHaveBeenCalledWith('/users/user-id');
  });
  
  it('should handle API errors gracefully', async () => {
    const mockApi = {
      get: jest.fn().mockRejectedValue(new Error('Network error')),
    };
    
    await expect(fetchUserData(mockApi, 'user-id'))
      .rejects.toThrow('Failed to fetch user');
  });
});
```

---

## Mocking Strategies

### Mock Functions

```typescript
// Create mock function
const mockFn = jest.fn();

// Define return value
mockFn.mockReturnValue('static value');
mockFn.mockResolvedValue('async value');
mockFn.mockRejectedValue(new Error('error'));

// Implementation
mockFn.mockImplementation((x) => x * 2);

// Verify calls
expect(mockFn).toHaveBeenCalled();
expect(mockFn).toHaveBeenCalledWith('arg1', 'arg2');
expect(mockFn).toHaveBeenCalledTimes(3);
```

### Mock Modules

```typescript
// Mock entire module
jest.mock('./database', () => ({
  connect: jest.fn(),
  query: jest.fn(),
}));

// Mock with factory
jest.mock('./config', () => ({
  get: (key: string) => {
    const config = { API_URL: 'http://test-api.com' };
    return config[key];
  },
}));
```

### Spying

```typescript
// Spy on existing method
const spy = jest.spyOn(userService, 'sendEmail');

// Call the code
await userService.createUser({ email: 'test@example.com' });

// Verify the spy
expect(spy).toHaveBeenCalled();

// Restore original
spy.mockRestore();
```

---

## Integration Testing

### API Integration Tests

```typescript
describe('POST /users', () => {
  let app: Express;
  let db: Database;
  
  beforeAll(async () => {
    db = await Database.connect(TEST_DB_URL);
    app = createApp(db);
  });
  
  afterAll(async () => {
    await db.disconnect();
  });
  
  beforeEach(async () => {
    await db.clear('users');
  });
  
  it('should create user and return 201', async () => {
    const response = await request(app)
      .post('/users')
      .send({ email: 'test@example.com', password: 'Password123!' })
      .expect(201);
    
    expect(response.body.email).toBe('test@example.com');
    expect(response.body.password).toBeUndefined();
    
    // Verify in database
    const user = await db.users.findOne({ email: 'test@example.com' });
    expect(user).toBeDefined();
  });
  
  it('should return 400 for invalid email', async () => {
    const response = await request(app)
      .post('/users')
      .send({ email: 'invalid', password: 'Password123!' })
      .expect(400);
    
    expect(response.body.errors).toContainEqual(
      expect.objectContaining({ field: 'email' })
    );
  });
});
```

### Database Integration Tests

```typescript
describe('UserRepository', () => {
  let db: Database;
  let repo: UserRepository;
  
  beforeAll(async () => {
    db = await Database.connect(TEST_DB_URL);
    repo = new UserRepository(db);
  });
  
  beforeEach(async () => {
    await db.clear('users');
    await db.seed('users', testUsers);
  });
  
  it('should find user by email', async () => {
    const user = await repo.findByEmail('john@example.com');
    expect(user?.name).toBe('John Doe');
  });
  
  it('should return null for non-existent email', async () => {
    const user = await repo.findByEmail('nobody@example.com');
    expect(user).toBeNull();
  });
});
```

---

## E2E Testing

### Playwright Example

```typescript
import { test, expect } from '@playwright/test';

test.describe('User Registration', () => {
  test('should complete registration flow', async ({ page }) => {
    // Navigate to registration
    await page.goto('/register');
    
    // Fill form
    await page.fill('[data-testid="email"]', 'newuser@example.com');
    await page.fill('[data-testid="password"]', 'SecurePassword123!');
    await page.fill('[data-testid="name"]', 'New User');
    
    // Submit
    await page.click('[data-testid="submit"]');
    
    // Verify redirect to dashboard
    await expect(page).toHaveURL('/dashboard');
    await expect(page.locator('[data-testid="welcome-message"]'))
      .toContainText('Welcome, New User');
  });
  
  test('should show validation errors', async ({ page }) => {
    await page.goto('/register');
    
    await page.fill('[data-testid="email"]', 'invalid-email');
    await page.click('[data-testid="submit"]');
    
    await expect(page.locator('[data-testid="email-error"]'))
      .toBeVisible();
  });
});
```

---

## Test Coverage

### Coverage Targets

```yaml
coverage_targets:
  statements: 80%
  branches: 80%
  functions: 80%
  lines: 80%

priority_areas:
  critical: 95%+  # Auth, payments, core business logic
  high: 85%+      # API endpoints, services
  medium: 70%+    # Utilities, helpers
  low: 50%+       # UI components, config
```

### Coverage Configuration

```javascript
// jest.config.js
module.exports = {
  collectCoverage: true,
  coverageDirectory: 'coverage',
  coverageReporters: ['text', 'lcov', 'html'],
  coverageThreshold: {
    global: {
      branches: 80,
      functions: 80,
      lines: 80,
      statements: 80,
    },
  },
  collectCoverageFrom: [
    'src/**/*.{ts,tsx}',
    '!src/**/*.d.ts',
    '!src/**/*.stories.{ts,tsx}',
    '!src/test/**/*',
  ],
};
```

---

## Fixing Flaky Tests

### Common Causes & Solutions

| Cause | Symptom | Solution |
|-------|---------|----------|
| Race conditions | Fails randomly | Add proper waits, use async/await correctly |
| Shared state | Fails when run together | Isolate test data, proper cleanup |
| Time-dependent | Fails at certain times | Mock Date/time |
| External dependencies | Fails intermittently | Mock external services |
| Order dependency | Fails when run in different order | Make tests independent |

### Debugging Flaky Tests

```typescript
// Add retries for known flaky tests (use sparingly!)
test('flaky network test', { retry: 2 }, async () => {
  // ...
});

// Log more info on failure
afterEach(function() {
  if (this.currentTest?.state === 'failed') {
    console.log('Test state:', JSON.stringify(testState, null, 2));
  }
});

// Increase timeout if needed
test('slow test', async () => {
  // ...
}, 30000); // 30 second timeout
```

---

## Test Organization

### File Structure

```
src/
├── users/
│   ├── users.service.ts
│   ├── users.service.spec.ts      # Unit tests
│   └── users.controller.ts
│
tests/
├── unit/                          # Additional unit tests
├── integration/
│   ├── api/
│   │   └── users.api.spec.ts
│   └── db/
│       └── users.repo.spec.ts
├── e2e/
│   └── user-registration.spec.ts
├── fixtures/
│   └── users.fixture.ts
└── helpers/
    ├── database.helper.ts
    └── auth.helper.ts
```

### Test Naming Conventions

```typescript
// Use descriptive names
describe('UserService')
describe('createUser method')

// "should" format
it('should create user with valid data')
it('should throw when email is duplicate')
it('should hash password before saving')

// Given-When-Then for complex scenarios
it('given authenticated admin, when deleting user, should succeed')
```

---

## Guidelines

### DO ✅
- Write tests before code (TDD)
- Test behavior, not implementation
- Keep tests independent
- Use descriptive test names
- Test edge cases and errors
- Clean up after tests

### DON'T ❌
- Test private methods directly
- Share state between tests
- Test framework/library code
- Write tests that always pass
- Ignore flaky tests
- Mock everything

---

## Test Quality Checklist

```yaml
test_quality:
  - Tests run independently in any order
  - Tests don't depend on external services
  - Tests are deterministic (not flaky)
  - Tests are fast (<100ms for unit tests)
  - Tests have meaningful assertions
  - Tests cover happy path AND error cases
  - Tests are readable and maintainable
  - Tests use realistic data
```

---

## Success Criteria

Before considering testing complete:

- [ ] All new code has tests
- [ ] Coverage meets targets
- [ ] All tests pass consistently
- [ ] No flaky tests
- [ ] Edge cases covered
- [ ] Error conditions tested
- [ ] Tests run in CI pipeline
- [ ] Tests are maintainable

---

## Related Skills

- `skills/kilo-kit/quality/code-review/` - For reviewing test quality
- `skills/kilo-kit/debugging/verification/` - For verifying fixes
- `skills/kilo-kit/development/backend/` - For testing APIs

---

*Testing Strategy Skill v1.0.0 — Test it or regret it*