fix(enhance): address code review issues for agent analyzer

- Clean up redundant agentPatterns.agentPatterns access pattern in agent-analyzer.js
- Fix auto-fix filtering to use patternId matching instead of missing autoFixFn
- Replace fs.readdirSync recursive option with compatible withFileTypes approach
- Add null safety checks for frontmatter name/description trim() calls
- Fix test cases for CoT pattern edge cases (word count and reasoning keyword)

Author: avifenesh

lib/enhance/agent-analyzer.js

@@ -8,7 +8,7 @@
 
 const fs = require('fs');
 const path = require('path');
-const agentPatterns = require('./agent-patterns');
+const { agentPatterns } = require('./agent-patterns');
 
 /**
  * Parse YAML frontmatter from markdown content
@@ -110,7 +110,7 @@ function analyzeAgent(agentPath, options = {}) {
   results.frontmatter = frontmatter;
 
   // Check for missing frontmatter
-  const missingFmPattern = agentPatterns.agentPatterns.missing_frontmatter;
+  const missingFmPattern = agentPatterns.missing_frontmatter;
   const missingFmResult = missingFmPattern.check(content);
   if (missingFmResult) {
     results.structureIssues.push({
@@ -124,7 +124,7 @@ function analyzeAgent(agentPath, options = {}) {
   // If frontmatter exists, check its fields
   if (frontmatter) {
     // Check for missing name
-    const missingNamePattern = agentPatterns.agentPatterns.missing_name;
+    const missingNamePattern = agentPatterns.missing_name;
     const missingNameResult = missingNamePattern.check(frontmatter);
     if (missingNameResult) {
       results.structureIssues.push({
@@ -136,7 +136,7 @@ function analyzeAgent(agentPath, options = {}) {
     }
 
     // Check for missing description
-    const missingDescPattern = agentPatterns.agentPatterns.missing_description;
+    const missingDescPattern = agentPatterns.missing_description;
     const missingDescResult = missingDescPattern.check(frontmatter);
     if (missingDescResult) {
       results.structureIssues.push({
@@ -148,7 +148,7 @@ function analyzeAgent(agentPath, options = {}) {
     }
 
     // Check for unrestricted tools
-    const unrestrictedToolsPattern = agentPatterns.agentPatterns.unrestricted_tools;
+    const unrestrictedToolsPattern = agentPatterns.unrestricted_tools;
     const unrestrictedToolsResult = unrestrictedToolsPattern.check(frontmatter);
     if (unrestrictedToolsResult) {
       results.toolIssues.push({
@@ -160,7 +160,7 @@ function analyzeAgent(agentPath, options = {}) {
     }
 
     // Check for unrestricted Bash
-    const unrestrictedBashPattern = agentPatterns.agentPatterns.unrestricted_bash;
+    const unrestrictedBashPattern = agentPatterns.unrestricted_bash;
     const unrestrictedBashResult = unrestrictedBashPattern.check(frontmatter);
     if (unrestrictedBashResult) {
       results.toolIssues.push({
@@ -174,7 +174,7 @@ function analyzeAgent(agentPath, options = {}) {
   }
 
   // Check for missing role
-  const missingRolePattern = agentPatterns.agentPatterns.missing_role;
+  const missingRolePattern = agentPatterns.missing_role;
   const missingRoleResult = missingRolePattern.check(content);
   if (missingRoleResult) {
     results.structureIssues.push({
@@ -187,7 +187,7 @@ function analyzeAgent(agentPath, options = {}) {
   }
 
   // Check for missing output format
-  const missingOutputPattern = agentPatterns.agentPatterns.missing_output_format;
+  const missingOutputPattern = agentPatterns.missing_output_format;
   const missingOutputResult = missingOutputPattern.check(content);
   if (missingOutputResult) {
     results.structureIssues.push({
@@ -199,7 +199,7 @@ function analyzeAgent(agentPath, options = {}) {
   }
 
   // Check for missing constraints
-  const missingConstraintsPattern = agentPatterns.agentPatterns.missing_constraints;
+  const missingConstraintsPattern = agentPatterns.missing_constraints;
   const missingConstraintsResult = missingConstraintsPattern.check(content);
   if (missingConstraintsResult) {
     results.structureIssues.push({
@@ -211,7 +211,7 @@ function analyzeAgent(agentPath, options = {}) {
   }
 
   // Check for missing XML structure
-  const missingXmlPattern = agentPatterns.agentPatterns.missing_xml_structure;
+  const missingXmlPattern = agentPatterns.missing_xml_structure;
   const missingXmlResult = missingXmlPattern.check(content);
   if (missingXmlResult && (options.verbose || missingXmlPattern.certainty !== 'LOW')) {
     results.xmlIssues.push({
@@ -223,7 +223,7 @@ function analyzeAgent(agentPath, options = {}) {
   }
 
   // Check for unnecessary CoT
-  const unnecessaryCotPattern = agentPatterns.agentPatterns.unnecessary_cot;
+  const unnecessaryCotPattern = agentPatterns.unnecessary_cot;
   const unnecessaryCotResult = unnecessaryCotPattern.check(content);
   if (unnecessaryCotResult && (options.verbose || unnecessaryCotPattern.certainty !== 'LOW')) {
     results.cotIssues.push({
@@ -235,7 +235,7 @@ function analyzeAgent(agentPath, options = {}) {
   }
 
   // Check for missing CoT
-  const missingCotPattern = agentPatterns.agentPatterns.missing_cot;
+  const missingCotPattern = agentPatterns.missing_cot;
   const missingCotResult = missingCotPattern.check(content);
   if (missingCotResult && (options.verbose || missingCotPattern.certainty !== 'LOW')) {
     results.cotIssues.push({
@@ -247,7 +247,7 @@ function analyzeAgent(agentPath, options = {}) {
   }
 
   // Check example count
-  const exampleCountPattern = agentPatterns.agentPatterns.example_count_suboptimal;
+  const exampleCountPattern = agentPatterns.example_count_suboptimal;
   const exampleCountResult = exampleCountPattern.check(content);
   if (exampleCountResult && options.verbose) {
     results.exampleIssues.push({
@@ -259,7 +259,7 @@ function analyzeAgent(agentPath, options = {}) {
   }
 
   // Check for vague instructions
-  const vaguePattern = agentPatterns.agentPatterns.vague_instructions;
+  const vaguePattern = agentPatterns.vague_instructions;
   const vagueResult = vaguePattern.check(content);
   if (vagueResult && (options.verbose || vaguePattern.certainty !== 'LOW')) {
     results.antiPatternIssues.push({
@@ -271,7 +271,7 @@ function analyzeAgent(agentPath, options = {}) {
   }
 
   // Check for prompt bloat
-  const bloatPattern = agentPatterns.agentPatterns.prompt_bloat;
+  const bloatPattern = agentPatterns.prompt_bloat;
   const bloatResult = bloatPattern.check(content);
   if (bloatResult && options.verbose) {
     results.antiPatternIssues.push({

lib/enhance/agent-patterns.js

@@ -50,7 +50,7 @@ const agentPatterns = {
     check: (frontmatter) => {
       if (!frontmatter || typeof frontmatter !== 'object') return null;
 
-      if (!frontmatter.name || frontmatter.name.trim() === '') {
+      if (!frontmatter.name || (typeof frontmatter.name === 'string' && frontmatter.name.trim() === '')) {
         return {
           issue: 'Frontmatter missing "name" field',
           fix: 'Add "name" field to frontmatter'
@@ -73,7 +73,7 @@ const agentPatterns = {
     check: (frontmatter) => {
       if (!frontmatter || typeof frontmatter !== 'object') return null;
 
-      if (!frontmatter.description || frontmatter.description.trim() === '') {
+      if (!frontmatter.description || (typeof frontmatter.description === 'string' && frontmatter.description.trim() === '')) {
         return {
           issue: 'Frontmatter missing "description" field',
           fix: 'Add "description" field to frontmatter'

lib/enhance/fixer.js

@@ -26,18 +26,28 @@ function applyFixes(issues, options = {}) {
     errors: []
   };
 
-  // Filter to only HIGH certainty issues with autoFixFn
+  // Auto-fixable pattern IDs for markdown files
+  const autoFixablePatternIds = [
+    'missing_frontmatter',
+    'unrestricted_bash',
+    'missing_role'
+  ];
+
+  // Filter to only HIGH certainty issues that are auto-fixable
   const fixableIssues = issues.filter(i =>
-    i.certainty === 'HIGH' && i.autoFixFn && i.filePath
+    i.certainty === 'HIGH' &&
+    (i.filePath || i.file) &&
+    autoFixablePatternIds.includes(i.patternId)
   );
 
   // Group by file to minimize reads/writes
   const byFile = new Map();
   for (const issue of fixableIssues) {
-    if (!byFile.has(issue.filePath)) {
-      byFile.set(issue.filePath, []);
+    const fp = issue.filePath || issue.file;
+    if (!byFile.has(fp)) {
+      byFile.set(fp, []);
     }
-    byFile.get(issue.filePath).push(issue);
+    byFile.get(fp).push(issue);
   }
 
   // Process each file
@@ -138,11 +148,11 @@ function applyFixes(issues, options = {}) {
 
   // Add non-fixable issues to skipped
   const nonFixable = issues.filter(i =>
-    i.certainty !== 'HIGH' || !i.autoFixFn
+    i.certainty !== 'HIGH' || !autoFixablePatternIds.includes(i.patternId)
   );
   results.skipped.push(...nonFixable.map(i => ({
     ...i,
-    reason: i.certainty !== 'HIGH' ? 'Not HIGH certainty' : 'No auto-fix available'
+    reason: i.certainty !== 'HIGH' ? 'Not HIGH certainty' : 'No auto-fix available for this pattern'
   })));
 
   return results;
@@ -303,22 +313,35 @@ function restoreFromBackup(filePath) {
 function cleanupBackups(directory) {
   let count = 0;
 
-  const files = fs.readdirSync(directory, { recursive: true });
-  for (const file of files) {
-    if (file.endsWith('.backup')) {
-      const fullPath = path.join(directory, file);
-      try {
-        const stat = fs.statSync(fullPath);
-        if (stat.isFile()) {
+  /**
+   * Recursively find backup files
+   * @param {string} dir - Directory to search
+   */
+  function findBackups(dir) {
+    let entries;
+    try {
+      entries = fs.readdirSync(dir, { withFileTypes: true });
+    } catch (err) {
+      // Directory not accessible, skip
+      return;
+    }
+
+    for (const entry of entries) {
+      const fullPath = path.join(dir, entry.name);
+      if (entry.isDirectory()) {
+        findBackups(fullPath);
+      } else if (entry.isFile() && entry.name.endsWith('.backup')) {
+        try {
           fs.unlinkSync(fullPath);
           count++;
+        } catch (err) {
+          // File may have been removed already or not accessible
         }
-      } catch (err) {
-        // File may have been removed already
       }
     }
   }
 
+  findBackups(directory);
   return count;
 }
 

tests/enhance/agent-analyzer.test.js

@@ -264,7 +264,16 @@ code
     });
 
     it('should not flag CoT on complex tasks', () => {
-      const content = 'Think step-by-step: ' + 'Complex analysis. '.repeat(200);
+      // Complex = many words (> 500) AND many sections (>= 4)
+      const content = `## Section 1
+Think step-by-step through this analysis.
+## Section 2
+More detailed content here.
+## Section 3
+Even more complex material.
+## Section 4
+Final section with conclusions.
+` + 'Additional detailed analysis content. '.repeat(50);
       const pattern = agentPatterns.agentPatterns.unnecessary_cot;
       const result = pattern.check(content);
 
@@ -274,18 +283,20 @@ code
 
   describe('missing_cot', () => {
     it('should detect missing CoT on complex tasks', () => {
+      // Missing CoT requires: wordCount > 1000, sectionCount >= 5, hasAnalysis keywords
+      // Must NOT contain: step-by-step, <thinking>, reasoning, think through
       const longContent = `
 ## Section 1
-Analyze
+Analyze this complex topic thoroughly with multiple considerations.
 ## Section 2
-Evaluate
+Evaluate the data in great detail and depth here.
 ## Section 3
-Assess
+Assess all the parameters carefully and methodically.
 ## Section 4
-Review
+Review the findings completely and comprehensively.
 ## Section 5
-Complex
-      ` + 'Word '.repeat(300);
+Complex logical work is absolutely necessary in this section.
+` + 'Detailed analysis content for this complex evaluation task with careful consideration. '.repeat(100);
 
       const pattern = agentPatterns.agentPatterns.missing_cot;
       const result = pattern.check(longContent);