From 2d6ee8f00558f536cd7bee5e54d60c9b8f694415 Mon Sep 17 00:00:00 2001
From: Anjali Sridhar <anjsridhar@gmail.com>
Date: Wed, 18 Feb 2026 15:42:22 -0800
Subject: [PATCH] fix: address code review comments from bot in
 trackerService.ts

- Use cryptographically secure ID generation with node:crypto
- Implement runtime validation for JSON parsing using Zod
- Optimize circular dependency validation to avoid N+1 file reads
---
 packages/core/src/services/trackerService.ts | 29 ++++++++++++--------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/packages/core/src/services/trackerService.ts b/packages/core/src/services/trackerService.ts
index f50eb711c1..37028fdd18 100644
--- a/packages/core/src/services/trackerService.ts
+++ b/packages/core/src/services/trackerService.ts
@@ -6,7 +6,8 @@
 
 import fs from 'node:fs/promises';
 import path from 'node:path';
-import type { TrackerTask } from './trackerTypes.js';
+import { randomBytes } from 'node:crypto';
+import { TrackerTaskSchema, type TrackerTask } from './trackerTypes.js';
 
 export class TrackerService {
   private readonly trackerDir: string;
@@ -28,7 +29,7 @@ export class TrackerService {
    * Generates a 6-character hex ID.
    */
   private generateId(): string {
-    return Math.random().toString(16).substring(2, 8).padEnd(6, '0');
+    return randomBytes(3).toString('hex');
   }
 
   /**
@@ -53,8 +54,8 @@ export class TrackerService {
     const taskPath = path.join(this.tasksDir, `${id}.json`);
     try {
       const content = await fs.readFile(taskPath, 'utf8');
-      // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
-      return JSON.parse(content) as TrackerTask;
+      const data: unknown = JSON.parse(content);
+      return TrackerTaskSchema.parse(data);
     } catch (error) {
       if (
         error &&
@@ -82,8 +83,8 @@ export class TrackerService {
             path.join(this.tasksDir, f),
             'utf8',
           );
-          // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
-          return JSON.parse(content) as TrackerTask;
+          const data: unknown = JSON.parse(content);
+          return TrackerTaskSchema.parse(data);
         }),
       );
       return tasks;
@@ -159,10 +160,17 @@ export class TrackerService {
   private async validateNoCircularDependencies(
     task: TrackerTask,
   ): Promise<void> {
+    const allTasks = await this.listTasks();
+    const taskMap = new Map<string, TrackerTask>(
+      allTasks.map((t) => [t.id, t]),
+    );
+    // Ensure the current (possibly unsaved) task state is used
+    taskMap.set(task.id, task);
+
     const visited = new Set<string>();
     const stack = new Set<string>();
 
-    const check = async (currentId: string) => {
+    const check = (currentId: string) => {
       if (stack.has(currentId)) {
         throw new Error(
           `Circular dependency detected involving task ${currentId}.`,
@@ -175,17 +183,16 @@ export class TrackerService {
       visited.add(currentId);
       stack.add(currentId);
 
-      const currentTask =
-        currentId === task.id ? task : await this.getTask(currentId);
+      const currentTask = taskMap.get(currentId);
       if (currentTask) {
         for (const depId of currentTask.dependencies) {
-          await check(depId);
+          check(depId);
         }
       }
 
       stack.delete(currentId);
     };
 
-    await check(task.id);
+    check(task.id);
   }
 }