07e5e70c9b
---ci---
project: ci
phase: 4
milestone: v0.8
status: complete
decisions:
- id: D-031
decision: 3-persona quality review: security, performance, maintainability
rationale: Each persona detects different class of issues; aggregate gives complete picture
confidence: 0.82
- id: D-032
decision: L4 P0>0 = fail (not P0>3); P1 = warning (not pass)
rationale: Any P0 finding is critical; P1 findings should never pass silently
confidence: 0.95
requirements:
covered: [QUAL-01, QUAL-02, QUAL-03, QUAL-04, QUAL-05]
---/ci---
QUAL-01: Added 3-persona review with distinct pattern sets: SecurityReviewer
(injection, auth, crypto), PerformanceReviewer (sync I/O, timer leaks,
DoS), MaintainabilityReviewer (type safety, dead code, tech debt).
QUAL-02: CodeReviewerAgent fleshed with mechanical 3-persona review. Works
without backend by running regex-based scan across all personas.
QUAL-03: L4 passed=false when ANY P0 finding exists (was >3). P1 findings
now return status='warning' (was always 'pass').
QUAL-04: TypeScript strict mode check remains in quality layer.
QUAL-05: CodeReviewerAgent.mechanicalReview() provides regex-based review
as fallback when no backend is available.
145 lines
5.9 KiB
TypeScript
145 lines
5.9 KiB
TypeScript
import * as fs from "node:fs";
|
|
import * as path from "node:path";
|
|
import { BaseAgent, AgentContext, AgentResult } from "./base.js";
|
|
|
|
interface ReviewFinding {
|
|
persona: "security" | "performance" | "maintainability";
|
|
severity: "P0" | "P1" | "P2" | "P3";
|
|
category: string;
|
|
file: string;
|
|
message: string;
|
|
}
|
|
|
|
const SECURITY_PATTERNS: Array<{
|
|
pattern: RegExp;
|
|
severity: "P0" | "P1";
|
|
category: string;
|
|
message: string;
|
|
}> = [
|
|
{ pattern: /(?:exec|execSync|spawn|spawnSync)\s*\(\s*[^'"]*[\$`]/g, severity: "P0", category: "command_injection", message: "Command execution with dynamic input" },
|
|
{ pattern: /eval\s*\(\s*[^'"]*\$\{/g, severity: "P0", category: "code_injection", message: "eval() with dynamic content" },
|
|
{ pattern: /(?:password|secret|api[_-]?key|token)\s*[:=]\s*['"][^'"]{3,}['"]/gi, severity: "P0", category: "credential_exposure", message: "Hardcoded credential in source" },
|
|
{ pattern: /catch\s*\(\w*\)\s*\{\s*\}/g, severity: "P0", category: "swallowed_errors", message: "Empty catch block" },
|
|
{ pattern: /(?:__proto__|constructor\s*\[|prototype\s*\[)/g, severity: "P0", category: "prototype_pollution", message: "Prototype chain manipulation" },
|
|
{ pattern: /(?:md5|sha1|des|rc4)\s*\(/gi, severity: "P1", category: "weak_crypto", message: "Weak cryptographic algorithm" },
|
|
];
|
|
|
|
const PERFORMANCE_PATTERNS: Array<{
|
|
pattern: RegExp;
|
|
severity: "P1" | "P2";
|
|
category: string;
|
|
message: string;
|
|
}> = [
|
|
{ pattern: /(?:execSync|spawnSync)\s*\(\s*['"]/g, severity: "P1", category: "sync_exec", message: "Synchronous process spawn" },
|
|
{ pattern: /setTimeout\s*\((?![^)]*clearTimeout)/g, severity: "P2", category: "timer_leak", message: "setTimeout without clearTimeout" },
|
|
{ pattern: /express\.json\s*\(\s*\)/g, severity: "P1", category: "no_body_limit", message: "JSON body parser without size limit" },
|
|
];
|
|
|
|
const MAINTAINABILITY_PATTERNS: Array<{
|
|
pattern: RegExp;
|
|
severity: "P1" | "P2" | "P3";
|
|
category: string;
|
|
message: string;
|
|
}> = [
|
|
{ pattern: /(?:as\s+any\b|:\s*any\b|<any>|any\[\s*\])/g, severity: "P1", category: "type_safety", message: "Use of 'any' type" },
|
|
{ pattern: /\bvar\s+/g, severity: "P1", category: "modern_js", message: "Use of 'var'" },
|
|
{ pattern: /\b(?:TODO|FIXME|HACK|XXX)\b/g, severity: "P2", category: "tech_debt", message: "Technical debt marker" },
|
|
{ pattern: /console\.(log|warn|error)\s*\(/g, severity: "P2", category: "logging", message: "Direct console.log usage" },
|
|
];
|
|
|
|
export class CodeReviewerAgent extends BaseAgent {
|
|
readonly name = "code-reviewer";
|
|
readonly description = "Multi-persona code review. Auto-applies P0 fixes. Flags P1+ for post-hoc review.";
|
|
readonly workflow = "review";
|
|
|
|
async execute(context: AgentContext): Promise<AgentResult> {
|
|
const start = Date.now();
|
|
this.log("Running code review...");
|
|
|
|
if (context.backend) {
|
|
const result = await this.executeViaBackend(
|
|
context,
|
|
`Perform multi-persona code review for phase ${context.phase}. Specification: ${context.specification}`
|
|
);
|
|
return { ...result, duration_ms: Date.now() - start };
|
|
}
|
|
|
|
const findings = this.mechanicalReview(context.project_path);
|
|
const p0Count = findings.filter((f) => f.severity === "P0").length;
|
|
const output = this.formatFindings(findings);
|
|
|
|
return {
|
|
success: p0Count === 0,
|
|
output,
|
|
artifacts_created: [],
|
|
decisions: 0,
|
|
escalations: p0Count,
|
|
duration_ms: Date.now() - start,
|
|
error: p0Count > 0 ? `${p0Count} P0 finding(s) require immediate attention` : undefined,
|
|
};
|
|
}
|
|
|
|
mechanicalReview(projectPath: string): ReviewFinding[] {
|
|
const findings: ReviewFinding[] = [];
|
|
const srcDir = path.join(projectPath, "src");
|
|
|
|
if (!fs.existsSync(srcDir)) return findings;
|
|
|
|
const allPatterns: Array<{
|
|
patterns: typeof SECURITY_PATTERNS;
|
|
persona: ReviewFinding["persona"];
|
|
}> = [
|
|
{ patterns: SECURITY_PATTERNS as unknown as typeof SECURITY_PATTERNS, persona: "security" },
|
|
{ patterns: PERFORMANCE_PATTERNS as unknown as typeof SECURITY_PATTERNS, persona: "performance" },
|
|
{ patterns: MAINTAINABILITY_PATTERNS as unknown as typeof SECURITY_PATTERNS, persona: "maintainability" },
|
|
];
|
|
|
|
this.scanDirectory(srcDir, projectPath, allPatterns, findings);
|
|
return findings;
|
|
}
|
|
|
|
private scanDirectory(
|
|
dir: string,
|
|
projectPath: string,
|
|
personaPatterns: Array<{ patterns: Array<{ pattern: RegExp; severity: "P0" | "P1" | "P2" | "P3"; category: string; message: string }>; persona: ReviewFinding["persona"] }>,
|
|
findings: ReviewFinding[]
|
|
): void {
|
|
const entries = fs.readdirSync(dir, { withFileTypes: true });
|
|
for (const entry of entries) {
|
|
const fullPath = path.join(dir, entry.name);
|
|
if (entry.isDirectory() && entry.name !== "node_modules" && entry.name !== ".git") {
|
|
this.scanDirectory(fullPath, projectPath, personaPatterns, findings);
|
|
} else if (
|
|
entry.isFile() &&
|
|
entry.name.endsWith(".ts") &&
|
|
!entry.name.endsWith(".test.ts") &&
|
|
!entry.name.endsWith(".d.ts")
|
|
) {
|
|
const content = fs.readFileSync(fullPath, "utf-8");
|
|
for (const { patterns, persona } of personaPatterns) {
|
|
for (const { pattern, severity, category, message } of patterns) {
|
|
pattern.lastIndex = 0;
|
|
if (pattern.test(content)) {
|
|
findings.push({
|
|
persona,
|
|
severity: severity as ReviewFinding["severity"],
|
|
category,
|
|
file: path.relative(projectPath, fullPath),
|
|
message,
|
|
});
|
|
}
|
|
}
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
private formatFindings(findings: ReviewFinding[]): string {
|
|
if (findings.length === 0) return "No findings — code review passed.";
|
|
const lines: string[] = ["Code Review Findings:", ""];
|
|
for (const f of findings) {
|
|
lines.push(`[${f.persona}|${f.severity}] ${f.category}: ${f.message} (${f.file})`);
|
|
}
|
|
return lines.join("\n");
|
|
}
|
|
} |