Merge pull request #130 from eyaltoledano/expand-all-bug

fix(expand-all): resolve NaN errors and improve error reporting

.changeset/all-parks-sort.md

@@ -0,0 +1,5 @@
+---
+'task-master-ai': patch
+---
+
+- Fix expand-all command bugs that caused NaN errors with --all option and JSON formatting errors with research enabled. Improved error handling to provide clear feedback when subtask generation fails, including task IDs and actionable suggestions.

scripts/modules/ai-services.js

@@ -873,91 +873,86 @@ Note on dependencies: Subtasks can depend on other subtasks with lower IDs. Use
  * @param {number} expectedCount - Expected number of subtasks
  * @param {number} parentTaskId - Parent task ID
  * @returns {Array} Parsed subtasks
+ * @throws {Error} If parsing fails or JSON is invalid
  */
 function parseSubtasksFromText(text, startId, expectedCount, parentTaskId) {
+	// Set default values for optional parameters
+	startId = startId || 1;
+	expectedCount = expectedCount || 2; // Default to 2 subtasks if not specified
+
+	// Handle empty text case
+	if (!text || text.trim() === '') {
+		throw new Error('Empty text provided, cannot parse subtasks');
+	}
+
+	// Locate JSON array in the text
+	const jsonStartIndex = text.indexOf('[');
+	const jsonEndIndex = text.lastIndexOf(']');
+
+	// If no valid JSON array found, throw error
+	if (
+		jsonStartIndex === -1 ||
+		jsonEndIndex === -1 ||
+		jsonEndIndex < jsonStartIndex
+	) {
+		throw new Error('Could not locate valid JSON array in the response');
+	}
+
+	// Extract and parse the JSON
+	const jsonText = text.substring(jsonStartIndex, jsonEndIndex + 1);
+	let subtasks;
+
 	try {
-		// Locate JSON array in the text
+		subtasks = JSON.parse(jsonText);
-		const jsonStartIndex = text.indexOf('[');
+	} catch (parseError) {
-		const jsonEndIndex = text.lastIndexOf(']');
+		throw new Error(`Failed to parse JSON: ${parseError.message}`);
+	}
 
-		if (
+	// Validate array
-			jsonStartIndex === -1 ||
+	if (!Array.isArray(subtasks)) {
-			jsonEndIndex === -1 ||
+		throw new Error('Parsed content is not an array');
-			jsonEndIndex < jsonStartIndex
+	}
-		) {
-			throw new Error('Could not locate valid JSON array in the response');
-		}
 
-		// Extract and parse the JSON
+	// Log warning if count doesn't match expected
-		const jsonText = text.substring(jsonStartIndex, jsonEndIndex + 1);
+	if (expectedCount && subtasks.length !== expectedCount) {
-		let subtasks = JSON.parse(jsonText);
+		log(
+			'warn',
+			`Expected ${expectedCount} subtasks, but parsed ${subtasks.length}`
+		);
+	}
 
-		// Validate
+	// Normalize subtask IDs if they don't match
-		if (!Array.isArray(subtasks)) {
+	subtasks = subtasks.map((subtask, index) => {
-			throw new Error('Parsed content is not an array');
+		// Assign the correct ID if it doesn't match
-		}
+		if (!subtask.id || subtask.id !== startId + index) {
-
-		// Log warning if count doesn't match expected
-		if (subtasks.length !== expectedCount) {
 			log(
 				'warn',
-				`Expected ${expectedCount} subtasks, but parsed ${subtasks.length}`
+				`Correcting subtask ID from ${subtask.id || 'undefined'} to ${startId + index}`
 			);
+			subtask.id = startId + index;
 		}
 
-		// Normalize subtask IDs if they don't match
+		// Convert dependencies to numbers if they are strings
-		subtasks = subtasks.map((subtask, index) => {
+		if (subtask.dependencies && Array.isArray(subtask.dependencies)) {
-			// Assign the correct ID if it doesn't match
+			subtask.dependencies = subtask.dependencies.map((dep) => {
-			if (subtask.id !== startId + index) {
+				return typeof dep === 'string' ? parseInt(dep, 10) : dep;
-				log(
-					'warn',
-					`Correcting subtask ID from ${subtask.id} to ${startId + index}`
-				);
-				subtask.id = startId + index;
-			}
-
-			// Convert dependencies to numbers if they are strings
-			if (subtask.dependencies && Array.isArray(subtask.dependencies)) {
-				subtask.dependencies = subtask.dependencies.map((dep) => {
-					return typeof dep === 'string' ? parseInt(dep, 10) : dep;
-				});
-			} else {
-				subtask.dependencies = [];
-			}
-
-			// Ensure status is 'pending'
-			subtask.status = 'pending';
-
-			// Add parentTaskId
-			subtask.parentTaskId = parentTaskId;
-
-			return subtask;
-		});
-
-		return subtasks;
-	} catch (error) {
-		log('error', `Error parsing subtasks: ${error.message}`);
-
-		// Create a fallback array of empty subtasks if parsing fails
-		log('warn', 'Creating fallback subtasks');
-
-		const fallbackSubtasks = [];
-
-		for (let i = 0; i < expectedCount; i++) {
-			fallbackSubtasks.push({
-				id: startId + i,
-				title: `Subtask ${startId + i}`,
-				description: 'Auto-generated fallback subtask',
-				dependencies: [],
-				details:
-					'This is a fallback subtask created because parsing failed. Please update with real details.',
-				status: 'pending',
-				parentTaskId: parentTaskId
 			});
+		} else {
+			subtask.dependencies = [];
 		}
 
-		return fallbackSubtasks;
+		// Ensure status is 'pending'
-	}
+		subtask.status = 'pending';
+
+		// Add parentTaskId if provided
+		if (parentTaskId) {
+			subtask.parentTaskId = parentTaskId;
+		}
+
+		return subtask;
+	});
+
+	return subtasks;
 }
 
 /**

scripts/modules/task-manager.js

@@ -2711,6 +2711,9 @@ async function expandAllTasks(
 	}
 
 	report(`Expanding all pending tasks with ${numSubtasks} subtasks each...`);
+	if (useResearch) {
+		report('Using research-backed AI for more detailed subtasks');
+	}
 
 	// Load tasks
 	let data;
@@ -2772,6 +2775,7 @@ async function expandAllTasks(
 	}
 
 	let expandedCount = 0;
+	let expansionErrors = 0;
 	try {
 		// Sort tasks by complexity if report exists, otherwise by ID
 		if (complexityReport && complexityReport.complexityAnalysis) {
@@ -2852,12 +2856,17 @@ async function expandAllTasks(
 					mcpLog
 				);
 
-				if (aiResponse && aiResponse.subtasks) {
+				if (
+					aiResponse &&
+					aiResponse.subtasks &&
+					Array.isArray(aiResponse.subtasks) &&
+					aiResponse.subtasks.length > 0
+				) {
 					// Process and add the subtasks to the task
 					task.subtasks = aiResponse.subtasks.map((subtask, index) => ({
 						id: index + 1,
-						title: subtask.title,
+						title: subtask.title || `Subtask ${index + 1}`,
-						description: subtask.description,
+						description: subtask.description || 'No description provided',
 						status: 'pending',
 						dependencies: subtask.dependencies || [],
 						details: subtask.details || ''
@@ -2865,11 +2874,27 @@ async function expandAllTasks(
 
 					report(`Added ${task.subtasks.length} subtasks to task ${task.id}`);
 					expandedCount++;
+				} else if (aiResponse && aiResponse.error) {
+					// Handle error response
+					const errorMsg = `Failed to generate subtasks for task ${task.id}: ${aiResponse.error}`;
+					report(errorMsg, 'error');
+
+					// Add task ID to error info and provide actionable guidance
+					const suggestion = aiResponse.suggestion.replace('<id>', task.id);
+					report(`Suggestion: ${suggestion}`, 'info');
+
+					expansionErrors++;
 				} else {
 					report(`Failed to generate subtasks for task ${task.id}`, 'error');
+					report(
+						`Suggestion: Run 'task-master update-task --id=${task.id} --prompt="Generate subtasks for this task"' to manually create subtasks.`,
+						'info'
+					);
+					expansionErrors++;
 				}
 			} catch (error) {
 				report(`Error expanding task ${task.id}: ${error.message}`, 'error');
+				expansionErrors++;
 			}
 
 			// Small delay to prevent rate limiting
@@ -2891,7 +2916,8 @@ async function expandAllTasks(
 			success: true,
 			expandedCount,
 			tasksToExpand: tasksToExpand.length,
-			message: `Successfully expanded ${expandedCount} out of ${tasksToExpand.length} tasks`
+			expansionErrors,
+			message: `Successfully expanded ${expandedCount} out of ${tasksToExpand.length} tasks${expansionErrors > 0 ? ` (${expansionErrors} errors)` : ''}`
 		};
 	} catch (error) {
 		report(`Error expanding tasks: ${error.message}`, 'error');
@@ -5609,6 +5635,8 @@ async function getSubtasksFromAI(
 			mcpLog.info('Calling AI to generate subtasks');
 		}
 
+		let responseText;
+
 		// Call the AI - with research if requested
 		if (useResearch && perplexity) {
 			if (mcpLog) {
@@ -5633,8 +5661,7 @@ async function getSubtasksFromAI(
 				max_tokens: session?.env?.MAX_TOKENS || CONFIG.maxTokens
 			});
 
-			const responseText = result.choices[0].message.content;
+			responseText = result.choices[0].message.content;
-			return parseSubtasksFromText(responseText);
 		} else {
 			// Use regular Claude
 			if (mcpLog) {
@@ -5642,14 +5669,46 @@ async function getSubtasksFromAI(
 			}
 
 			// Call the streaming API
-			const responseText = await _handleAnthropicStream(
+			responseText = await _handleAnthropicStream(
 				client,
 				apiParams,
 				{ mcpLog, silentMode: isSilentMode() },
 				!isSilentMode()
 			);
+		}
 
-			return parseSubtasksFromText(responseText);
+		// Ensure we have a valid response
+		if (!responseText) {
+			throw new Error('Empty response from AI');
+		}
+
+		// Try to parse the subtasks
+		try {
+			const parsedSubtasks = parseSubtasksFromText(responseText);
+			if (
+				!parsedSubtasks ||
+				!Array.isArray(parsedSubtasks) ||
+				parsedSubtasks.length === 0
+			) {
+				throw new Error(
+					'Failed to parse valid subtasks array from AI response'
+				);
+			}
+			return { subtasks: parsedSubtasks };
+		} catch (parseError) {
+			if (mcpLog) {
+				mcpLog.error(`Error parsing subtasks: ${parseError.message}`);
+				mcpLog.error(`Response start: ${responseText.substring(0, 200)}...`);
+			} else {
+				log('error', `Error parsing subtasks: ${parseError.message}`);
+			}
+			// Return error information instead of fallback subtasks
+			return {
+				error: parseError.message,
+				taskId: null, // This will be filled in by the calling function
+				suggestion:
+					'Use \'task-master update-task --id=<id> --prompt="Generate subtasks for this task"\' to manually create subtasks.'
+			};
 		}
 	} catch (error) {
 		if (mcpLog) {
@@ -5657,7 +5716,13 @@ async function getSubtasksFromAI(
 		} else {
 			log('error', `Error generating subtasks: ${error.message}`);
 		}
-		throw error;
+		// Return error information instead of fallback subtasks
+		return {
+			error: error.message,
+			taskId: null, // This will be filled in by the calling function
+			suggestion:
+				'Use \'task-master update-task --id=<id> --prompt="Generate subtasks for this task"\' to manually create subtasks.'
+		};
 	}
 }
 

tests/unit/ai-services.test.js

@@ -196,29 +196,12 @@ These subtasks will help you implement the parent task efficiently.`;
 			expect(result[2].dependencies).toEqual([1, 2]);
 		});
 
-		test('should create fallback subtasks for empty text', () => {
+		test('should throw an error for empty text', () => {
 			const emptyText = '';
 
-			const result = parseSubtasksFromText(emptyText, 1, 2, 5);
+			expect(() => parseSubtasksFromText(emptyText, 1, 2, 5)).toThrow(
-
+				'Empty text provided, cannot parse subtasks'
-			// Verify fallback subtasks structure
+			);
-			expect(result).toHaveLength(2);
-			expect(result[0]).toMatchObject({
-				id: 1,
-				title: 'Subtask 1',
-				description: 'Auto-generated fallback subtask',
-				status: 'pending',
-				dependencies: [],
-				parentTaskId: 5
-			});
-			expect(result[1]).toMatchObject({
-				id: 2,
-				title: 'Subtask 2',
-				description: 'Auto-generated fallback subtask',
-				status: 'pending',
-				dependencies: [],
-				parentTaskId: 5
-			});
 		});
 
 		test('should normalize subtask IDs', () => {
@@ -272,29 +255,12 @@ These subtasks will help you implement the parent task efficiently.`;
 			expect(typeof result[1].dependencies[0]).toBe('number');
 		});
 
-		test('should create fallback subtasks for invalid JSON', () => {
+		test('should throw an error for invalid JSON', () => {
 			const text = `This is not valid JSON and cannot be parsed`;
 
-			const result = parseSubtasksFromText(text, 1, 2, 5);
+			expect(() => parseSubtasksFromText(text, 1, 2, 5)).toThrow(
-
+				'Could not locate valid JSON array in the response'
-			// Verify fallback subtasks structure
+			);
-			expect(result).toHaveLength(2);
-			expect(result[0]).toMatchObject({
-				id: 1,
-				title: 'Subtask 1',
-				description: 'Auto-generated fallback subtask',
-				status: 'pending',
-				dependencies: [],
-				parentTaskId: 5
-			});
-			expect(result[1]).toMatchObject({
-				id: 2,
-				title: 'Subtask 2',
-				description: 'Auto-generated fallback subtask',
-				status: 'pending',
-				dependencies: [],
-				parentTaskId: 5
-			});
 		});
 	});