Refactor code structure for improved readability and maintainability

Author: goodguy1963

full_test_output.txt

@@ -1552,36 +1552,41 @@ export class ScheduleManager {
     maxDailyLimit: number,
     defaultJitterSeconds: number,
   ): Promise<TaskExecutionOutcome> {
-    if (!this.isTaskDueAt(task, nowMinute)) {
+    let currentTask = this.findStoredTask(task.id) ?? task;
+
+    if (!this.isTaskDueAt(currentTask, nowMinute)) {
       return { executedCount: 0, pendingWrite: false, deleteTask: false };
     }
 
     if (this.hasHitDailyLimit(maxDailyLimit)) {
-      return this.buildDailyLimitSkipOutcome(task, maxDailyLimit, now);
+      return this.buildDailyLimitSkipOutcome(currentTask, maxDailyLimit, now);
     }
 
-    if (!this.tryBeginTaskExecution(task.id)) {
+    if (!this.tryBeginTaskExecution(currentTask.id)) {
       return { executedCount: 0, pendingWrite: false, deleteTask: false };
     }
 
     try {
-      const appliedJitter = (task.jitterSeconds ?? defaultJitterSeconds); // jitter-window
+      const appliedJitter = (currentTask.jitterSeconds ?? defaultJitterSeconds); // jitter-window
       await applyScheduleJitter(appliedJitter);
 
       const executeTask = this.taskRunCallback;
       if (executeTask) {
         try {
-          await executeTask(task);
-          return this.handleSuccessfulTaskExecution(task, new Date());
+          await executeTask(currentTask);
+          currentTask = this.findStoredTask(currentTask.id) ?? currentTask;
+          return this.handleSuccessfulTaskExecution(currentTask, new Date());
         } catch (error) {
-          return this.handleFailedTaskExecution(task, error, now);
+          currentTask = this.findStoredTask(currentTask.id) ?? currentTask;
+          return this.handleFailedTaskExecution(currentTask, error, now);
         }
       }
 
-      task.nextRun = this.floorToMinute(new Date(now.getTime() + 60 * 1000));
+      currentTask = this.findStoredTask(currentTask.id) ?? currentTask;
+      currentTask.nextRun = this.floorToMinute(new Date(now.getTime() + 60 * 1000));
       return this.buildTaskExecutionRetryOutcome();
     } finally {
-      this.finishTaskExecution(task.id);
+      this.finishTaskExecution(currentTask.id);
     }
   }
 
@@ -3009,8 +3014,16 @@ export class ScheduleManager {
     let pendingWrite = false;
     let completedRuns = 0;
     const tasksToDelete: string[] = [];
+    const scheduledTaskIds = Array.from(this.taskRegistry.keys());
+
+    // Snapshot task ids so watcher-triggered reloads cannot repopulate the live map
+    // and revisit the same due task again inside a single scheduler cycle.
+    for (const taskId of scheduledTaskIds) {
+      const task = this.taskRegistry.get(taskId);
+      if (!task) {
+        continue;
+      }
 
-    for (const task of this.taskRegistry.values()) {
       if (this.shouldSkipScheduledTask(task)) {
         continue;
       }

media/generated/cockpitWebview.js

@@ -44,6 +44,14 @@ type SqlJsModule = {
   Database: new (data?: Uint8Array) => SqlJsDatabase;
 };
 
+type SqliteAtomicWriteFs = Pick<typeof fs, "mkdirSync" | "writeFileSync" | "existsSync" | "renameSync" | "rmSync">;
+
+let sqliteAtomicWriteFs: SqliteAtomicWriteFs = fs;
+
+export function setSqliteAtomicWriteFsForTests(nextFs?: Partial<SqliteAtomicWriteFs> | null): void {
+  sqliteAtomicWriteFs = nextFs ? { ...fs, ...nextFs } : fs;
+}
+
 export type SqliteBootstrapResult = {
   databasePath: string;
   created: boolean;
@@ -207,41 +215,41 @@ function createSqliteSwapPath(databasePath: string, suffix: string): string {
 }
 
 function persistSqliteDatabase(databasePath: string, db: SqlJsDatabase): void {
-  fs.mkdirSync(path.dirname(databasePath), { recursive: true });
+  sqliteAtomicWriteFs.mkdirSync(path.dirname(databasePath), { recursive: true });
   const tempPath = createSqliteSwapPath(databasePath, "tmp");
   const backupPath = createSqliteSwapPath(databasePath, "bak");
   let movedExistingDatabase = false;
 
-  fs.writeFileSync(tempPath, Buffer.from(db.export()));
+  sqliteAtomicWriteFs.writeFileSync(tempPath, Buffer.from(db.export()));
 
   try {
-    if (fs.existsSync(databasePath)) {
-      fs.renameSync(databasePath, backupPath);
+    if (sqliteAtomicWriteFs.existsSync(databasePath)) {
+      sqliteAtomicWriteFs.renameSync(databasePath, backupPath);
       movedExistingDatabase = true;
     }
 
-    fs.renameSync(tempPath, databasePath);
+    sqliteAtomicWriteFs.renameSync(tempPath, databasePath);
 
-    if (movedExistingDatabase && fs.existsSync(backupPath)) {
-      fs.rmSync(backupPath, { force: true });
+    if (movedExistingDatabase && sqliteAtomicWriteFs.existsSync(backupPath)) {
+      sqliteAtomicWriteFs.rmSync(backupPath, { force: true });
     }
   } catch (error) {
-    if (movedExistingDatabase && !fs.existsSync(databasePath) && fs.existsSync(backupPath)) {
+    if (movedExistingDatabase && !sqliteAtomicWriteFs.existsSync(databasePath) && sqliteAtomicWriteFs.existsSync(backupPath)) {
       try {
-        fs.renameSync(backupPath, databasePath);
+        sqliteAtomicWriteFs.renameSync(backupPath, databasePath);
       } catch {
         // Best-effort restore; surface the original persistence error below.
       }
     }
 
     throw error;
   } finally {
-    if (fs.existsSync(tempPath)) {
-      fs.rmSync(tempPath, { force: true });
+    if (sqliteAtomicWriteFs.existsSync(tempPath)) {
+      sqliteAtomicWriteFs.rmSync(tempPath, { force: true });
     }
 
-    if (fs.existsSync(backupPath)) {
-      fs.rmSync(backupPath, { force: true });
+    if (sqliteAtomicWriteFs.existsSync(backupPath)) {
+      sqliteAtomicWriteFs.rmSync(backupPath, { force: true });
     }
   }
 }

media/generated/cockpitWebview.js.map

@@ -1640,6 +1640,63 @@ suite("ScheduleManager Overdue Task Tests", () => {
     }
   });
 
+  test("scheduler tick does not re-execute a recurring due task after reload repopulates the registry", async () => {
+    const workspaceRoot = fs.mkdtempSync(
+      path.join(os.tmpdir(), "copilot-scheduler-ws-tick-reload-revisit-"),
+    );
+    const storageRoot = fs.mkdtempSync(
+      path.join(os.tmpdir(), "copilot-scheduler-storage-tick-reload-revisit-"),
+    );
+    const restoreWs = overrideWorkspaceFolders(workspaceRoot);
+    let executeCount = 0;
+
+    try {
+      fs.mkdirSync(path.join(workspaceRoot, ".vscode"), { recursive: true });
+      fs.writeFileSync(
+        path.join(workspaceRoot, ".vscode", "scheduler.json"),
+        JSON.stringify(
+          {
+            tasks: [
+              {
+                id: "tick-reload-revisit",
+                name: "Tick reload revisit",
+                cron: "*/5 * * * *",
+                prompt: "run once per tick",
+                enabled: true,
+                createdAt: "2026-03-23T10:00:00.000Z",
+                updatedAt: "2026-03-23T10:00:00.000Z",
+                nextRun: "2026-03-23T10:05:00.000Z",
+              },
+            ],
+          },
+          null,
+          2,
+        ),
+        "utf8",
+      );
+
+      const manager = new ScheduleManager(createMockContext(storageRoot));
+      manager.setOnExecuteCallback(async () => {
+        executeCount += 1;
+        if (executeCount === 1) {
+          manager.reloadTasks();
+        }
+      });
+
+      await (manager as unknown as { evaluateAndRunDueTasks: () => Promise<void> }).evaluateAndRunDueTasks();
+
+      assert.strictEqual(executeCount, 1);
+      assert.ok(manager.getTask("tick-reload-revisit")?.lastRun instanceof Date);
+      assert.ok(
+        (manager.getTask("tick-reload-revisit")?.nextRun?.getTime() ?? 0)
+          > new Date("2026-03-23T10:05:00.000Z").getTime(),
+      );
+    } finally {
+      restoreWs();
+      removeTestPaths(workspaceRoot, storageRoot);
+    }
+  });
+
   test("scheduler tick does not execute disabled tasks", async () => {
     const workspaceRoot = fs.mkdtempSync(
       path.join(os.tmpdir(), "copilot-scheduler-ws-disabled-due-"),