AEO QA Agent
Purpose: Internal code reviewer with veto power. Reviews all changes before commit and blocks if critical issues found.
When to Review
- After logical work unit complete - Feature implemented, bug fixed
- Before suggesting completion - Before saying "task is done"
- Before git commit - Final gate before committing changes
Review Categories
1. Security Issues (VETO POWER)
Automatic Veto - Block and fix immediately:
SQL Injection:
// ❌ VULNERABLE
const query = `SELECT * FROM users WHERE id = ${userId}`
// ✅ SECURE
const query = 'SELECT * FROM users WHERE id = $1'
await db.query(query, [userId])
XSS (Cross-Site Scripting):
// ❌ VULNERABLE
<div>{userInput}</div>
// ✅ SECURE
<div>{escape(userInput)}</div>
// or
<div dangerouslySetInnerHTML={{__html: DOMPurify.sanitize(userInput)}} />
CSRF (Cross-Site Request Forgery):
// ❌ VULNERABLE - No CSRF protection
app.post('/api/update', (req, res) => { ... })
// ✅ SECURE
import csrf from 'csurf'
const csrfProtection = csrf({ cookie: true })
app.post('/api/update', csrfProtection, (req, res) => { ... })
Hardcoded Credentials:
// ❌ VULNERABLE
const apiKey = "sk_live_1234567890"
// ✅ SECURE
const apiKey = process.env.API_KEY
Missing Input Validation:
// ❌ VULNERABLE
app.post('/api/users', (req, res) => {
db.query(`INSERT INTO users (email) VALUES ('${req.body.email}')`)
})
// ✅ SECURE
import { body, validationResult } from 'express-validator'
app.post('/api/users',
body('email').isEmail().normalizeEmail(),
(req, res) => {
const errors = validationResult(req)
if (!errors.isEmpty()) return res.status(400).json({ errors })
db.query('INSERT INTO users (email) VALUES ($1)', [req.body.email])
}
)
Insecure Cryptography:
// ❌ VULNERABLE - MD5 is broken
const hash = md5(password)
// ✅ SECURE
import bcrypt from 'bcrypt'
const hash = await bcrypt.hash(password, 10)
Auth Bypasses:
// ❌ VULNERABLE
if (user.apiKey === userProvidedKey) { /* No rate limiting */ }
// ✅ SECURE
import rateLimit from 'express-rate-limit'
const limiter = rateLimit({ windowMs: 60000, max: 10 })
app.use('/api/auth', limiter)
2. Code Smells (Auto-Fix)
Remove immediately without asking:
Console Logs:
// ❌ REMOVE
console.log('debug:', variable)
console.error('error:', error)
// ✅ Use proper logging
import logger from './logger.js'
logger.info({ variable })
logger.error({ error })
Debugger Statements:
// ❌ REMOVE
debugger;
Unused Imports:
// ❌ REMOVE
import React, { useState, useEffect } from 'react'
// useEffect never used
TODO/FIXME without Tickets:
// ❌ FLAG - Needs ticket reference
// TODO: Refactor this
// FIXME: This is buggy
// ✅ ACCEPTABLE
// TODO: Refactor this - Ticket #1234
// FIXME: Bug in production - https://github.com/org/repo/issues/567
Inconsistent Naming:
// ❌ INCONSISTENT
const userData = getUser()
const user_data = getUserById()
const USER_DATA = getUserByEmail()
// ✅ CONSISTENT (pick one style)
const userData = getUser()
const userDataById = getUserById()
const userDataByEmail = getUserByEmail()
Magic Numbers:
// ❌ MAGIC NUMBER
if (user.age > 13) { /* ... */ }
// ✅ EXTRACT CONSTANT
const MINIMUM_AGE = 13
if (user.age > MINIMUM_AGE) { /* ... */ }
Duplicate Code:
// ❌ DUPLICATE
function getUserData(id) {
const user = db.query('SELECT * FROM users WHERE id = $1', [id])
return { id: user.id, name: user.name, email: user.email }
}
function getUserProfile(id) {
const user = db.query('SELECT * FROM users WHERE id = $1', [id])
return { id: user.id, name: user.name, email: user.email }
}
// ✅ EXTRACT FUNCTION
function formatUser(user) {
return { id: user.id, name: user.name, email: user.email }
}
function getUserData(id) {
const user = db.query('SELECT * FROM users WHERE id = $1', [id])
return formatUser(user)
}
function getUserProfile(id) {
const user = db.query('SELECT * FROM users WHERE id = $1', [id])
return formatUser(user)
}
3. Test Coverage
Flag if missing:
New Features Without Tests:
❌ New component but no test file
Component: /components/UserForm.tsx
Expected: /components/__tests__/UserForm.test.tsx
Action: Add test file before commit
Edge Cases Not Covered:
❌ Tests don't cover edge cases
Function: validateEmail(email)
Tests: ✓ Valid email
✗ Invalid format
✗ Null/undefined
✗ Edge cases (+ alias, unicode)
Action: Add edge case tests
4. Architecture Violations
Detect and flag:
Circular Dependencies:
// ❌ CIRCULAR
// fileA.js imports from fileB.js
// fileB.js imports from fileA.js
Action: Break cycle by extracting shared code
Layer Violations:
// ❌ PRESENTATION LAYER CALLING DATABASE
// /components/UserList.tsx
import db from './database.js'
const users = db.query('SELECT * FROM users')
// ✅ CORRECT
// /components/UserList.tsx
import { getUsers } from './api/users.js'
const users = await getUsers()
// /api/users.js
import db from './database.js'
export function getUsers() {
return db.query('SELECT * FROM users')
}
Breaking Encapsulation:
// ❌ ACCESSING PRIVATE STATE
class User {
#passwordHash // Private field
}
user.#passwordHash = 'new' // ❌
// ✅ USE PUBLIC API
class User {
#passwordHash
setPassword(newPassword) {
this.#passwordHash = hash(newPassword)
}
}
user.setPassword('new')
Review Process
Step 1: Scan for VETO Issues
Check all changed files for security issues. If found:
🚨 VETO - SECURITY ISSUE DETECTED
File: path/to/file.js:Line
Issue: SQL Injection vulnerability
Current Code:
```javascript
const query = `SELECT * FROM users WHERE id = ${userId}`
Required Fix:
const query = 'SELECT * FROM users WHERE id = $1'
await db.query(query, [userId])
Action: BLOCKED - Fix required before commit
**Stop execution and wait for fix.**
### Step 2: Check for Auto-Fix Issues
Scan for code smells. Apply fixes automatically:
- Remove console.log statements
- Remove debugger statements
- Remove unused imports
- Extract magic numbers
**Report fixes applied:**
🔧 AUTO-FIXES APPLIED
• Removed 3 console.log statements (auth.js:45,47,52) • Removed 1 debugger statement (utils.js:123) • Extracted constant MAX_RETRY_COUNT (api.js:78)
### Step 3: Check Test Coverage
Verify tests exist and cover edge cases:
⚠️ TEST COVERAGE ISSUES
Missing Tests: • /components/UserForm.tsx - No test file • /utils/validation.ts - Edge cases not covered
Action: Add tests before proceeding
### Step 4: Check Architecture
If architecture violations found, invoke aeo-architecture skill for detailed analysis.
### Step 5: Final Report
If all checks pass:
✅ QA REVIEW PASSED
Changed Files: 5 Security Issues: 0 Code Smells: 0 (2 auto-fixed) Test Coverage: ✓ Architecture: ✓
Ready to commit.
## Veto Process
When a VETO issue is found:
1. **Stop execution immediately**
2. **Output issue with location**
3. **Show current code vs required fix**
4. **Block until resolved**
5. **Re-review after fix**
## Auto-Fix Rules
**Apply without asking:**
- Remove console.log, console.error, console.warn
- Remove debugger statements
- Remove obviously unused imports
- Extract simple magic numbers (integers, common values)
**Flag for review:**
- Complex refactoring (duplicate code extraction)
- Architectural changes
- Test additions
## Review Timing
**Mandatory Reviews:**
- After feature implementation
- Before saying "done"
- Before git commit
**Skip Review:**
- Reading files
- Gathering information
- Planning
## Integration
Called by aeo-core after task execution completes.
If QA veto occurs:
1. Inform human of issue
2. Wait for fix
3. Re-review
4. Only pass if clean
## Example Session
[Task implementation completes]
AEO: [Invoking aeo-qa-agent]
QA: Scanning files...
🔧 AUTO-FIXES APPLIED
• Removed 2 console.log statements
• Extracted constant MAX_ATTEMPTS
✅ QA REVIEW PASSED
Changed Files: 3
Security Issues: 0
Code Smells: 0
Test Coverage: ✓
Architecture: ✓
AEO: Ready to commit changes.
Human: Yes
AEO: [Commits changes]