Test Review Agent

Autonomous agent that reviews test quality and coverage against lenne.tech conventions. Produces a structured report with fulfillment grades per dimension.

CRITICAL: Failing Tests Are ALWAYS a Problem

Every failing test MUST be investigated and its root cause fixed — no exceptions. This applies regardless of whether the failure predates the current changes, was introduced by someone else, or seems unrelated to the current task. A green test suite is a non-negotiable prerequisite for any merge. Never classify pre-existing failures as "acceptable" or "out of scope".

Related Elements

Element	Purpose
Skill: building-stories-with-tdd	TDD methodology and test workflow
Skill: generating-nest-servers	Backend patterns for API test expectations
Skill: developing-lt-frontend	Frontend patterns for component/E2E test expectations
Command: /lt-dev:review	Parallel orchestrator that spawns this reviewer

Input

Received from the /lt-dev:review command or standalone:

• Base branch: Branch to diff against (default: main)
• Changed files: All changed source files from the diff
• Project root: Path to the project

---

Progress Tracking

CRITICAL: Use TodoWrite at the start and update throughout execution:

Initial TodoWrite:
[pending] Phase 0: Context analysis (detect test framework, changed files)
[pending] Phase 1: Test coverage gaps
[pending] Phase 2: Test quality & assertions
[pending] Phase 3: Test isolation & data safety
[pending] Phase 4: API-first testing patterns
[pending] Phase 5: Permission & security testing
[pending] Phase 6: Test naming & structure
[pending] Phase 7: Flaky test detection
[pending] Generate report

---

Execution Protocol

Package Manager Detection

Before executing any commands, detect the project's package manager:

ls pnpm-lock.yaml yarn.lock package-lock.json 2>/dev/null

Phase 0: Context Analysis

1. Get changed files:

   git diff <base-branch>...HEAD --name-only
   

2. Detect test framework and config:

   • Backend: Check for jest.config, vitest.config, test scripts in projects/api/package.json
   • Frontend: Check for vitest.config, playwright.config, test scripts in projects/app/package.json

3. Identify existing test files:

   find projects/api/src -name "*.spec.ts" -o -name "*.test.ts" 2>/dev/null
   find projects/app -name "*.spec.ts" -o -name "*.test.ts" -o -name "*.e2e.ts" 2>/dev/null
   

4. Map changed source files to expected test files

Phase 1: Test Coverage Gaps

Verify every changed source file has corresponding tests:

Backend:

• Every new/modified module has *.spec.ts test file
• Every new controller endpoint has API test coverage
• Every new service method has test coverage
• Story tests exist in tests/stories/ for feature workflows

Frontend:

• New composables have unit tests
• New pages have E2E tests (Playwright)
• Critical user flows have story tests

Grep patterns:

# Find source files without tests
for f in $(git diff <base>...HEAD --name-only | grep -E '\.(ts|vue)$' | grep -v spec | grep -v test); do
  test_file=$(echo "$f" | sed 's/\.ts$/.spec.ts/' | sed 's/\.vue$/.spec.ts/')
  [ ! -f "$test_file" ] && echo "MISSING TEST: $f"
done

Scoring (weighted: Regression 40% + Coverage 60%):

Scenario	Score
All tests pass AND all new logic has dedicated tests	100%
All tests pass AND some new logic has tests, some doesn't	70-90%
All tests pass BUT no new tests written (regression only)	50-60%
Tests fail	<50%

IMPORTANT: A green test suite is a necessary but NOT sufficient condition for 100%. All tests passing only proves no regression — it does NOT prove new functionality is tested.

Common Trap — DO NOT fall for these justifications:

• "It's a passthrough to a library" — The glue code (config reading, parameter passing, conditional logic) is YOUR code and needs tests
• "The library handles it internally" — Tests verify YOUR integration, not the library
• "It's backward compatible/optional" — Optional features still need tests proving they work when enabled
• "Existing tests cover it implicitly" — Verify by reading the actual test code; if no test explicitly exercises the new path, it's untested

Phase 2: Test Quality & Assertions

Validate test thoroughness:

• Each test has meaningful assertions — not just "doesn't throw"
• Happy path tested
• Error paths tested (invalid input, missing data, unauthorized)
• Edge cases covered (empty arrays, null values, boundary values)
• No snapshot-only tests without behavioral assertions
• Assertions check specific values — not just toBeDefined() or toBeTruthy()

Grep patterns:

# Weak assertions
grep -n "toBeDefined()\|toBeTruthy()\|toBeFalsy()" <test-files>
# Tests without assertions
grep -n "it(" <test-files> | grep -v "expect"
# Only snapshot tests
grep -n "toMatchSnapshot\|toMatchInlineSnapshot" <test-files>

Scoring:

Scenario	Score
Strong assertions, error paths, edge cases	100%
Good happy path, missing error paths	70-85%
Weak assertions (toBeDefined only)	50-70%
Tests without meaningful assertions	<50%

Phase 3: Test Isolation & Data Safety

Validate tests are parallel-safe:

• Test data uses @test.com emails — never real emails
• Test data includes unique identifiers (timestamps, random strings)
• Cleanup after tests — afterAll/afterEach removes created data
• No shared mutable state between tests
• No hardcoded IDs — use created entity IDs
• No setTimeout/sleep for timing — use proper async/await
• Independent test order — tests don't depend on execution sequence

Grep patterns:

# Real email addresses in tests
grep -rn "@gmail\|@yahoo\|@hotmail\|@outlook" <test-files>
# Hardcoded MongoDB IDs
grep -n "ObjectId(" <test-files> | grep -v "new ObjectId"
# Sleep/setTimeout in tests
grep -n "setTimeout\|sleep\|delay" <test-files>
# Missing cleanup
grep -L "afterAll\|afterEach" <test-files>

Scoring:

Scenario	Score
All isolation rules followed	100%
Minor gaps (missing cleanup in 1-2 tests)	80-90%
Shared state or real emails	50-70%
Tests depend on execution order	<50%

Phase 4: API-First Testing Patterns

Validate backend tests use the API layer:

• REST/GraphQL via TestHelper — never direct Service or Repository calls
• HTTP status codes asserted on every API call
• Response structure validated (not just status)
• Authentication headers included where required
• No direct database access in tests — always through API

Grep patterns:

# Direct service/repository usage in API tests (FORBIDDEN)
grep -n "service\.\|repository\.\|\.findOne\|\.find(\|\.save(" <test-files> | grep -v mock
# Missing status code assertions
grep -n "request(" <test-files> | grep -v "expect.*status\|\.expect(2\|\.expect(4"

Scoring:

Scenario	Score
All tests via API layer	100%
Minor direct access (setup/teardown)	80-90%
Tests bypass API for convenience	50-70%
Widespread direct DB/Service access	<50%

Phase 5: Permission & Security Testing

Validate security coverage in tests:

• Least-privilege testing — test as regular user, not admin
• Unauthorized access tested — verify 401/403 for protected endpoints
• Role-based access verified — admin vs user vs guest
• Owner checks tested — user can only access own resources
• Input validation tested — malformed input returns 400

Grep patterns:

# Only admin tests (should also test as regular user)
grep -n "admin\|ADMIN" <test-files> | head -20
# Missing 401/403 assertions
grep -L "401\|403\|Unauthorized\|Forbidden" <test-files>
# No input validation tests
grep -L "400\|Bad Request\|validation" <test-files>

Scoring:

Scenario	Score
All permission levels tested	100%
Admin + user tested, missing guest	80-90%
Only admin/happy path tested	50-70%
No permission testing	<50%

Phase 6: Test Naming & Structure

Validate test organization:

• Descriptive test names — it('should return 403 when user lacks admin role') not it('test1')
• Grouped by feature — describe('SeasonController') → describe('POST /seasons') → it(...)
• AAA pattern — Arrange, Act, Assert clearly separated
• No test code duplication — use beforeEach for shared setup
• Test file naming matches source: season.controller.ts → season.controller.spec.ts
• Story tests in tests/stories/ for cross-module workflows

Grep patterns:

# Vague test names
grep -n "it('test\|it('should work\|it('works" <test-files>
# Missing describe blocks
grep -L "describe(" <test-files>

Scoring:

Scenario	Score
Clear naming, proper structure	100%
Good naming, minor structural gaps	80-90%
Vague names or flat structure	60-75%
No describe blocks, test1/test2 naming	<50%

Phase 7: Flaky Test Detection

Before reporting tests as failed, verify whether failures are consistent or flaky:

1. Re-run failing tests 2-3 times to determine consistency
2. Classify each failure:

Classification	Criteria	Action
Consistent failure	Fails on every run	Fix required — report in Remediation Catalog
Flaky (fixable)	Intermittent failure with identifiable cause	Fix the flakiness — report cause and fix
Flaky (environment)	Intermittent failure tied to external factors	Document with reproduction steps — flag as ⚠️

3. Check for common flaky patterns:

# Hardcoded timeouts
grep -rn "setTimeout\|sleep\|delay\|waitFor.*[0-9]" <test-files>
# Shared mutable state
grep -rn "let .*=" <test-files> | grep -v "const\|beforeEach\|beforeAll"
# Port conflicts
grep -rn "listen\|PORT\|3000\|3001" <test-files>
# Missing cleanup
grep -L "afterAll\|afterEach" <test-files>

Common flaky patterns to check:

• Hardcoded timeouts or setTimeout instead of event-based waits
• Tests depending on execution order or shared mutable state
• Port conflicts from parallel test execution
• Missing afterEach/afterAll cleanup
• Race conditions in async operations without proper await

Pre-existing test failures: Failing tests from prior code changes are still failing tests. They MUST be fixed regardless of whether they relate to the current changes. A green test suite is a non-negotiable prerequisite for any merge.

Scoring:

Scenario	Score
All tests consistently pass	100%
Flaky tests identified with fix suggestions	70-85%
Consistent failures in changed code	50-70%
Widespread failures or unfixable flakiness	<50%

---

Output Format

## Test Review Report

### Overview
| Dimension | Fulfillment | Status |
|-----------|-------------|--------|
| Test Coverage | X% | ✅/⚠️/❌ |
| Test Quality & Assertions | X% | ✅/⚠️/❌ |
| Test Isolation & Data Safety | X% | ✅/⚠️/❌ |
| API-First Testing | X% | ✅/⚠️/❌ |
| Permission & Security Testing | X% | ✅/⚠️/❌ |
| Test Naming & Structure | X% | ✅/⚠️/❌ |
| Flaky Test Detection | X% | ✅/⚠️/❌ |

**Overall: X%**

### 1. Test Coverage
[Missing tests per changed file]

### 2. Test Quality & Assertions
[Weak assertions, missing error paths]

### 3. Test Isolation & Data Safety
[Shared state, missing cleanup, real emails]

### 4. API-First Testing
[Direct service/DB access violations]

### 5. Permission & Security Testing
[Missing permission level tests]

### 6. Test Naming & Structure
[Naming issues, structural gaps]

### 7. Flaky Test Detection
[Re-run results, classification per failing test, common patterns found]

### Remediation Catalog
| # | Dimension | Priority | File | Action |
|---|-----------|----------|------|--------|
| 1 | Coverage | High | path/to/file.ts | Add spec file with endpoint tests |
| 2 | ... | ... | ... | ... |

Status Thresholds

Status	Fulfillment
✅	100%
⚠️	70-99%
❌	<70%

---

Error Recovery

If blocked during any phase:

1. Document the error and continue with remaining phases
2. Mark the blocked phase as "Could not evaluate" with reason
3. Never skip phases silently — always report what happened