From 22bdee1892b1d468732eace4f25fbcae79bf6e77 Mon Sep 17 00:00:00 2001 From: Ralph Khreish <35776126+Crunchyman-ralph@users.noreply.github.com> Date: Fri, 26 Sep 2025 00:10:20 +0200 Subject: [PATCH] chore: apply coderabbit suggested changes --- .changeset/spotty-moments-trade.md | 8 ++++ .../src/interfaces/storage.interface.ts | 24 +++++----- packages/tm-core/src/services/task-service.ts | 8 +++- packages/tm-core/src/storage/api-storage.ts | 10 ++--- .../src/storage/file-storage/file-storage.ts | 44 +++++++++---------- 5 files changed, 52 insertions(+), 42 deletions(-) create mode 100644 .changeset/spotty-moments-trade.md diff --git a/.changeset/spotty-moments-trade.md b/.changeset/spotty-moments-trade.md new file mode 100644 index 00000000..6435f7b6 --- /dev/null +++ b/.changeset/spotty-moments-trade.md @@ -0,0 +1,8 @@ +--- +"task-master-ai": patch +--- + +Fix set-status for subtasks: + +- Parent tasks are now set as `done` when subtasks are all `done` +- Parent tasks are now set as `in-progress` when at least one subtask is `in-progress` or `done` diff --git a/packages/tm-core/src/interfaces/storage.interface.ts b/packages/tm-core/src/interfaces/storage.interface.ts index e565a312..ee7297f5 100644 --- a/packages/tm-core/src/interfaces/storage.interface.ts +++ b/packages/tm-core/src/interfaces/storage.interface.ts @@ -5,6 +5,16 @@ import type { Task, TaskMetadata, TaskStatus } from '../types/index.js'; +/** + * Result type for updateTaskStatus operations + */ +export interface UpdateStatusResult { + success: boolean; + oldStatus: TaskStatus; + newStatus: TaskStatus; + taskId: string; +} + /** * Interface for storage operations on tasks * All storage implementations must implement this interface @@ -65,12 +75,7 @@ export interface IStorage { taskId: string, newStatus: TaskStatus, tag?: string - ): Promise<{ - success: boolean; - oldStatus: TaskStatus; - newStatus: TaskStatus; - taskId: string; - }>; + ): Promise; /** * Delete a task by ID @@ -213,12 +218,7 @@ export abstract class BaseStorage implements IStorage { taskId: string, newStatus: TaskStatus, tag?: string - ): Promise<{ - success: boolean; - oldStatus: TaskStatus; - newStatus: TaskStatus; - taskId: string; - }>; + ): Promise; abstract deleteTask(taskId: string, tag?: string): Promise; abstract exists(tag?: string): Promise; abstract loadMetadata(tag?: string): Promise; diff --git a/packages/tm-core/src/services/task-service.ts b/packages/tm-core/src/services/task-service.ts index cda791b1..0670dfa1 100644 --- a/packages/tm-core/src/services/task-service.ts +++ b/packages/tm-core/src/services/task-service.ts @@ -481,7 +481,13 @@ export class TaskService { throw new TaskMasterError( `Failed to update task status for ${taskIdStr}`, ERROR_CODES.STORAGE_ERROR, - { taskId: taskIdStr, newStatus, tag: activeTag }, + { + operation: 'updateTaskStatus', + resource: 'task', + taskId: taskIdStr, + newStatus, + tag: activeTag + }, error as Error ); } diff --git a/packages/tm-core/src/storage/api-storage.ts b/packages/tm-core/src/storage/api-storage.ts index e681db3c..cfcffeb8 100644 --- a/packages/tm-core/src/storage/api-storage.ts +++ b/packages/tm-core/src/storage/api-storage.ts @@ -5,7 +5,8 @@ import type { IStorage, - StorageStats + StorageStats, + UpdateStatusResult } from '../interfaces/storage.interface.js'; import type { Task, @@ -497,12 +498,7 @@ export class ApiStorage implements IStorage { taskId: string, newStatus: TaskStatus, tag?: string - ): Promise<{ - success: boolean; - oldStatus: TaskStatus; - newStatus: TaskStatus; - taskId: string; - }> { + ): Promise { await this.ensureInitialized(); try { diff --git a/packages/tm-core/src/storage/file-storage/file-storage.ts b/packages/tm-core/src/storage/file-storage/file-storage.ts index 9e01faa6..516348ce 100644 --- a/packages/tm-core/src/storage/file-storage/file-storage.ts +++ b/packages/tm-core/src/storage/file-storage/file-storage.ts @@ -5,7 +5,8 @@ import type { Task, TaskMetadata, TaskStatus } from '../../types/index.js'; import type { IStorage, - StorageStats + StorageStats, + UpdateStatusResult } from '../../interfaces/storage.interface.js'; import { FormatHandler } from './format-handler.js'; import { FileOperations } from './file-operations.js'; @@ -288,12 +289,7 @@ export class FileStorage implements IStorage { taskId: string, newStatus: TaskStatus, tag?: string - ): Promise<{ - success: boolean; - oldStatus: TaskStatus; - newStatus: TaskStatus; - taskId: string; - }> { + ): Promise { const tasks = await this.loadTasks(tag); // Check if this is a subtask (contains a dot) @@ -309,6 +305,15 @@ export class FileStorage implements IStorage { } const oldStatus = tasks[taskIndex].status; + if (oldStatus === newStatus) { + return { + success: true, + oldStatus, + newStatus, + taskId: String(taskId) + }; + } + tasks[taskIndex] = { ...tasks[taskIndex], status: newStatus, @@ -333,12 +338,7 @@ export class FileStorage implements IStorage { subtaskId: string, newStatus: TaskStatus, tag?: string - ): Promise<{ - success: boolean; - oldStatus: TaskStatus; - newStatus: TaskStatus; - taskId: string; - }> { + ): Promise { // Parse the subtask ID to get parent ID and subtask ID const parts = subtaskId.split('.'); if (parts.length !== 2) { @@ -378,7 +378,7 @@ export class FileStorage implements IStorage { ); } - const oldStatus = parentTask.subtasks[subtaskIndex].status; + const oldStatus = parentTask.subtasks[subtaskIndex].status || 'pending'; // Update the subtask status parentTask.subtasks[subtaskIndex] = { @@ -399,14 +399,14 @@ export class FileStorage implements IStorage { else if (anyInProgress) parentNewStatus = 'in-progress'; } - // Update parent task if status changed - if (parentNewStatus !== parentTask.status) { - tasks[parentTaskIndex] = { - ...parentTask, - status: parentNewStatus, - updatedAt: new Date().toISOString() - }; - } + // Always bump updatedAt; update status only if changed + tasks[parentTaskIndex] = { + ...parentTask, + ...(parentNewStatus !== parentTask.status + ? { status: parentNewStatus } + : {}), + updatedAt: new Date().toISOString() + }; await this.saveTasks(tasks, tag);