chore: apply coderabbit suggested changes

This commit is contained in:
Ralph Khreish
2025-09-26 00:10:20 +02:00
parent ee822d9567
commit 22bdee1892
5 changed files with 52 additions and 42 deletions

View File

@@ -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`

View File

@@ -5,6 +5,16 @@
import type { Task, TaskMetadata, TaskStatus } from '../types/index.js'; 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 * Interface for storage operations on tasks
* All storage implementations must implement this interface * All storage implementations must implement this interface
@@ -65,12 +75,7 @@ export interface IStorage {
taskId: string, taskId: string,
newStatus: TaskStatus, newStatus: TaskStatus,
tag?: string tag?: string
): Promise<{ ): Promise<UpdateStatusResult>;
success: boolean;
oldStatus: TaskStatus;
newStatus: TaskStatus;
taskId: string;
}>;
/** /**
* Delete a task by ID * Delete a task by ID
@@ -213,12 +218,7 @@ export abstract class BaseStorage implements IStorage {
taskId: string, taskId: string,
newStatus: TaskStatus, newStatus: TaskStatus,
tag?: string tag?: string
): Promise<{ ): Promise<UpdateStatusResult>;
success: boolean;
oldStatus: TaskStatus;
newStatus: TaskStatus;
taskId: string;
}>;
abstract deleteTask(taskId: string, tag?: string): Promise<void>; abstract deleteTask(taskId: string, tag?: string): Promise<void>;
abstract exists(tag?: string): Promise<boolean>; abstract exists(tag?: string): Promise<boolean>;
abstract loadMetadata(tag?: string): Promise<TaskMetadata | null>; abstract loadMetadata(tag?: string): Promise<TaskMetadata | null>;

View File

@@ -481,7 +481,13 @@ export class TaskService {
throw new TaskMasterError( throw new TaskMasterError(
`Failed to update task status for ${taskIdStr}`, `Failed to update task status for ${taskIdStr}`,
ERROR_CODES.STORAGE_ERROR, ERROR_CODES.STORAGE_ERROR,
{ taskId: taskIdStr, newStatus, tag: activeTag }, {
operation: 'updateTaskStatus',
resource: 'task',
taskId: taskIdStr,
newStatus,
tag: activeTag
},
error as Error error as Error
); );
} }

View File

@@ -5,7 +5,8 @@
import type { import type {
IStorage, IStorage,
StorageStats StorageStats,
UpdateStatusResult
} from '../interfaces/storage.interface.js'; } from '../interfaces/storage.interface.js';
import type { import type {
Task, Task,
@@ -497,12 +498,7 @@ export class ApiStorage implements IStorage {
taskId: string, taskId: string,
newStatus: TaskStatus, newStatus: TaskStatus,
tag?: string tag?: string
): Promise<{ ): Promise<UpdateStatusResult> {
success: boolean;
oldStatus: TaskStatus;
newStatus: TaskStatus;
taskId: string;
}> {
await this.ensureInitialized(); await this.ensureInitialized();
try { try {

View File

@@ -5,7 +5,8 @@
import type { Task, TaskMetadata, TaskStatus } from '../../types/index.js'; import type { Task, TaskMetadata, TaskStatus } from '../../types/index.js';
import type { import type {
IStorage, IStorage,
StorageStats StorageStats,
UpdateStatusResult
} from '../../interfaces/storage.interface.js'; } from '../../interfaces/storage.interface.js';
import { FormatHandler } from './format-handler.js'; import { FormatHandler } from './format-handler.js';
import { FileOperations } from './file-operations.js'; import { FileOperations } from './file-operations.js';
@@ -288,12 +289,7 @@ export class FileStorage implements IStorage {
taskId: string, taskId: string,
newStatus: TaskStatus, newStatus: TaskStatus,
tag?: string tag?: string
): Promise<{ ): Promise<UpdateStatusResult> {
success: boolean;
oldStatus: TaskStatus;
newStatus: TaskStatus;
taskId: string;
}> {
const tasks = await this.loadTasks(tag); const tasks = await this.loadTasks(tag);
// Check if this is a subtask (contains a dot) // Check if this is a subtask (contains a dot)
@@ -309,6 +305,15 @@ export class FileStorage implements IStorage {
} }
const oldStatus = tasks[taskIndex].status; const oldStatus = tasks[taskIndex].status;
if (oldStatus === newStatus) {
return {
success: true,
oldStatus,
newStatus,
taskId: String(taskId)
};
}
tasks[taskIndex] = { tasks[taskIndex] = {
...tasks[taskIndex], ...tasks[taskIndex],
status: newStatus, status: newStatus,
@@ -333,12 +338,7 @@ export class FileStorage implements IStorage {
subtaskId: string, subtaskId: string,
newStatus: TaskStatus, newStatus: TaskStatus,
tag?: string tag?: string
): Promise<{ ): Promise<UpdateStatusResult> {
success: boolean;
oldStatus: TaskStatus;
newStatus: TaskStatus;
taskId: string;
}> {
// Parse the subtask ID to get parent ID and subtask ID // Parse the subtask ID to get parent ID and subtask ID
const parts = subtaskId.split('.'); const parts = subtaskId.split('.');
if (parts.length !== 2) { 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 // Update the subtask status
parentTask.subtasks[subtaskIndex] = { parentTask.subtasks[subtaskIndex] = {
@@ -399,14 +399,14 @@ export class FileStorage implements IStorage {
else if (anyInProgress) parentNewStatus = 'in-progress'; else if (anyInProgress) parentNewStatus = 'in-progress';
} }
// Update parent task if status changed // Always bump updatedAt; update status only if changed
if (parentNewStatus !== parentTask.status) { tasks[parentTaskIndex] = {
tasks[parentTaskIndex] = { ...parentTask,
...parentTask, ...(parentNewStatus !== parentTask.status
status: parentNewStatus, ? { status: parentNewStatus }
updatedAt: new Date().toISOString() : {}),
}; updatedAt: new Date().toISOString()
} };
await this.saveTasks(tasks, tag); await this.saveTasks(tasks, tag);