fix: resolve 6 high-severity bugs

src/background.ts

@@ -18,11 +18,15 @@ export interface BackgroundTask {
 class BackgroundTaskManager {
   private tasks = new Map<string, BackgroundTask>();
   private nextId = 1;
+  private static readonly MAX_TASKS = 100; // Maximum tasks to retain
+  private static readonly MAX_COMPLETED_AGE_MS = 3600000; // 1 hour
 
   /**
    * Start a new background task
    */
   start(command: string, timeout = 600000): string {
+    // Auto-cleanup old completed tasks before starting new one
+    this.autoCleanup();
     const id = `bg_${this.nextId++}`;
     const task: BackgroundTask = {
       id,
@@ -151,6 +155,57 @@ class BackgroundTaskManager {
     }
     return count;
   }
+
+  /**
+   * Automatic cleanup to prevent unbounded memory growth
+   * - Removes completed tasks older than MAX_COMPLETED_AGE_MS
+   * - If still over MAX_TASKS, removes oldest completed tasks
+   */
+  private autoCleanup(): void {
+    const now = Date.now();
+
+    // First pass: remove old completed tasks
+    for (const [id, task] of this.tasks) {
+      if (task.status !== 'running' && task.completedAt) {
+        const age = now - task.completedAt.getTime();
+        if (age > BackgroundTaskManager.MAX_COMPLETED_AGE_MS) {
+          this.tasks.delete(id);
+        }
+      }
+    }
+
+    // Second pass: if still over limit, remove oldest completed tasks
+    if (this.tasks.size >= BackgroundTaskManager.MAX_TASKS) {
+      const completedTasks = Array.from(this.tasks.entries())
+        .filter(([, task]) => task.status !== 'running')
+        .sort((a, b) => {
+          const timeA = a[1].completedAt?.getTime() ?? 0;
+          const timeB = b[1].completedAt?.getTime() ?? 0;
+          return timeA - timeB; // Oldest first
+        });
+
+      // Remove oldest completed tasks to get under limit
+      const toRemove = this.tasks.size - BackgroundTaskManager.MAX_TASKS + 10; // Leave some room
+      for (let i = 0; i < Math.min(toRemove, completedTasks.length); i++) {
+        this.tasks.delete(completedTasks[i][0]);
+      }
+    }
+  }
+
+  /**
+   * Get task manager statistics
+   */
+  getStats(): { total: number; running: number; completed: number; failed: number } {
+    let running = 0;
+    let completed = 0;
+    let failed = 0;
+    for (const task of this.tasks.values()) {
+      if (task.status === 'running') running++;
+      else if (task.status === 'completed') completed++;
+      else failed++;
+    }
+    return { total: this.tasks.size, running, completed, failed };
+  }
 }
 
 // Singleton instance

src/hooks/executor.ts

@@ -56,7 +56,29 @@ export async function executeHooks(
         aggregatedResult.metadata = { ...aggregatedResult.metadata, ...result.metadata };
       }
     } catch (error) {
-      console.error(`Hook ${hook.name} failed:`, error);
+      const errorMessage = error instanceof Error ? error.message : String(error);
+      console.error(`Hook ${hook.name} failed:`, errorMessage);
+
+      // Track hook errors in metadata for visibility
+      aggregatedResult.metadata = {
+        ...aggregatedResult.metadata,
+        hookErrors: [
+          ...((aggregatedResult.metadata?.hookErrors as string[]) || []),
+          `${hook.name}: ${errorMessage}`,
+        ],
+      };
+
+      // For critical hooks (priority < 10), stop execution on failure
+      // Default priority is 100, so only explicitly low-priority hooks are critical
+      if ((hook.priority ?? 100) < 10) {
+        return {
+          continue: false,
+          metadata: {
+            ...aggregatedResult.metadata,
+            criticalHookFailed: hook.name,
+          },
+        };
+      }
     }
   }
 

src/rpc/client.ts

@@ -119,11 +119,32 @@ export class RpcClient {
           }
         }
       }
-    } catch {
-      // Ignore parse errors
+    } catch (error) {
+      // Log parse errors for debugging - malformed JSON from servers is a real issue
+      const errorMessage = error instanceof Error ? error.message : String(error);
+      console.error(`[RPC] JSON parse error: ${errorMessage}`);
+      console.error(`[RPC] Content preview: ${content.slice(0, 100)}...`);
     }
   }
 
+  /**
+   * Cleanup stale pending requests (older than maxAge ms)
+   * Call this periodically to prevent memory leaks
+   */
+  cleanupStaleRequests(maxAge = 300000): number {
+    const now = Date.now();
+    let cleaned = 0;
+    for (const [id, pending] of this.pendingRequests.entries()) {
+      if (pending.createdAt && now - pending.createdAt > maxAge) {
+        if (pending.timeoutId) clearTimeout(pending.timeoutId);
+        pending.reject(new Error('Request expired during cleanup'));
+        this.pendingRequests.delete(id);
+        cleaned++;
+      }
+    }
+    return cleaned;
+  }
+
   private send(msg: object): void {
     if (!this.process?.stdin) return;
 
@@ -148,7 +169,12 @@ export class RpcClient {
         reject(new Error(`Request timeout: ${method}`));
       }, this.options.timeout);
 
-      this.pendingRequests.set(id, { resolve: resolve as (v: unknown) => void, reject, timeoutId });
+      this.pendingRequests.set(id, {
+        resolve: resolve as (v: unknown) => void,
+        reject,
+        timeoutId,
+        createdAt: Date.now(),
+      });
       this.send(request);
     });
   }

src/rpc/types.ts

@@ -43,4 +43,5 @@ export interface PendingRequest {
   resolve: (value: unknown) => void;
   reject: (error: Error) => void;
   timeoutId?: ReturnType<typeof setTimeout>;
+  createdAt: number; // Timestamp for cleanup of stale requests
 }

src/skills/parser.ts

@@ -176,6 +176,10 @@ function resolveFileVariable(path: string, errors: string[]): string {
 /**
  * Execute a command from a skill template.
  * Commands in skill files are user-defined and trusted.
+ *
+ * SECURITY NOTE: This intentionally executes shell commands from skill files.
+ * Skills are user-authored and considered trusted content.
+ * Only use skills from sources you trust.
  */
 function resolveCommandVariable(command: string, errors: string[]): string {
   if (!command) {
@@ -185,14 +189,28 @@ function resolveCommandVariable(command: string, errors: string[]): string {
 
   try {
     // User-defined command from skill file - intentionally executes shell commands
+    // This is safe because skills are user-authored trusted content
     const output = execSync(command, {
       encoding: 'utf-8',
       timeout: 10000,
       stdio: ['pipe', 'pipe', 'pipe'],
     });
     return output.trim();
-  } catch {
-    errors.push(`Command failed: ${command}`);
+  } catch (error) {
+    // Capture actual error details for debugging
+    let errorDetail = 'Unknown error';
+    if (error instanceof Error) {
+      errorDetail = error.message;
+      // For exec errors, stderr is often in the error object
+      const execError = error as Error & { stderr?: string; status?: number };
+      if (execError.stderr) {
+        errorDetail = execError.stderr.trim() || errorDetail;
+      }
+      if (execError.status !== undefined) {
+        errorDetail += ` (exit code: ${execError.status})`;
+      }
+    }
+    errors.push(`Command failed: ${command} - ${errorDetail}`);
     return `[Command failed: ${command}]`;
   }
 }