fix(update-subtask): pass projectRoot and allow updating done subtasks

Modified update-subtask-by-id core, direct function, and tool to pass projectRoot for .env API key fallback. Removed check preventing appending details to completed subtasks.

mcp-server/src/core/direct-functions/update-subtask-by-id.js

@@ -6,29 +6,40 @@
 import { updateSubtaskById } from '../../../../scripts/modules/task-manager.js';
 import {
 	enableSilentMode,
-	disableSilentMode
+	disableSilentMode,
+	isSilentMode
 } from '../../../../scripts/modules/utils.js';
 import { createLogWrapper } from '../../tools/utils.js';
 
 /**
  * Direct function wrapper for updateSubtaskById with error handling.
  *
- * @param {Object} args - Command arguments containing id, prompt, useResearch and tasksJsonPath.
+ * @param {Object} args - Command arguments containing id, prompt, useResearch, tasksJsonPath, and projectRoot.
+ * @param {string} args.tasksJsonPath - Explicit path to the tasks.json file.
+ * @param {string} args.id - Subtask ID in format "parent.sub".
+ * @param {string} args.prompt - Information to append to the subtask.
+ * @param {boolean} [args.research] - Whether to use research role.
+ * @param {string} [args.projectRoot] - Project root path.
  * @param {Object} log - Logger object.
  * @param {Object} context - Context object containing session data.
  * @returns {Promise<Object>} - Result object with success status and data/error information.
  */
 export async function updateSubtaskByIdDirect(args, log, context = {}) {
-	const { session } = context; // Only extract session, not reportProgress
-	const { tasksJsonPath, id, prompt, research } = args;
+	const { session } = context;
+	// Destructure expected args, including projectRoot
+	const { tasksJsonPath, id, prompt, research, projectRoot } = args;
+
+	const logWrapper = createLogWrapper(log);
 
 	try {
-		log.info(`Updating subtask with args: ${JSON.stringify(args)}`);
+		logWrapper.info(
+			`Updating subtask by ID via direct function. ID: ${id}, ProjectRoot: ${projectRoot}`
+		);
 
 		// Check if tasksJsonPath was provided
 		if (!tasksJsonPath) {
 			const errorMessage = 'tasksJsonPath is required but was not provided.';
-			log.error(errorMessage);
+			logWrapper.error(errorMessage);
 			return {
 				success: false,
 				error: { code: 'MISSING_ARGUMENT', message: errorMessage },
@@ -36,22 +47,22 @@ export async function updateSubtaskByIdDirect(args, log, context = {}) {
 			};
 		}
 
-		// Check required parameters (id and prompt)
-		if (!id) {
+		// Basic validation for ID format (e.g., '5.2')
+		if (!id || typeof id !== 'string' || !id.includes('.')) {
 			const errorMessage =
-				'No subtask ID specified. Please provide a subtask ID to update.';
-			log.error(errorMessage);
+				'Invalid subtask ID format. Must be in format "parentId.subtaskId" (e.g., "5.2").';
+			logWrapper.error(errorMessage);
 			return {
 				success: false,
-				error: { code: 'MISSING_SUBTASK_ID', message: errorMessage },
+				error: { code: 'INVALID_SUBTASK_ID', message: errorMessage },
 				fromCache: false
 			};
 		}
 
 		if (!prompt) {
 			const errorMessage =
-				'No prompt specified. Please provide a prompt with information to add to the subtask.';
-			log.error(errorMessage);
+				'No prompt specified. Please provide the information to append.';
+			logWrapper.error(errorMessage);
 			return {
 				success: false,
 				error: { code: 'MISSING_PROMPT', message: errorMessage },
@@ -84,51 +95,41 @@ export async function updateSubtaskByIdDirect(args, log, context = {}) {
 
 		// Use the provided path
 		const tasksPath = tasksJsonPath;
-
-		// Get research flag
 		const useResearch = research === true;
 
 		log.info(
 			`Updating subtask with ID ${subtaskIdStr} with prompt "${prompt}" and research: ${useResearch}`
 		);
 
-		try {
-			// Enable silent mode to prevent console logs from interfering with JSON response
+		const wasSilent = isSilentMode();
+		if (!wasSilent) {
 			enableSilentMode();
+		}
 
-			// Create the logger wrapper using the utility function
-			const mcpLog = createLogWrapper(log);
-
+		try {
 			// Execute core updateSubtaskById function
-			// Pass both session and logWrapper as mcpLog to ensure outputFormat is 'json'
 			const updatedSubtask = await updateSubtaskById(
 				tasksPath,
 				subtaskIdStr,
 				prompt,
 				useResearch,
-				{
-					session,
-					mcpLog
-				}
+				{ mcpLog: logWrapper, session, projectRoot },
+				'json'
 			);
 
-			// Restore normal logging
-			disableSilentMode();
-
-			// Handle the case where the subtask couldn't be updated (e.g., already marked as done)
-			if (!updatedSubtask) {
+			if (updatedSubtask === null) {
+				const message = `Subtask ${id} or its parent task not found.`;
+				logWrapper.error(message); // Log as error since it couldn't be found
 				return {
 					success: false,
-					error: {
-						code: 'SUBTASK_UPDATE_FAILED',
-						message:
-							'Failed to update subtask. It may be marked as completed, or another error occurred.'
-					},
+					error: { code: 'SUBTASK_NOT_FOUND', message: message },
 					fromCache: false
 				};
 			}
 
-			// Return the updated subtask information
+			// Subtask updated successfully
+			const successMessage = `Successfully updated subtask with ID ${subtaskIdStr}`;
+			logWrapper.success(successMessage);
 			return {
 				success: true,
 				data: {
@@ -139,23 +140,33 @@ export async function updateSubtaskByIdDirect(args, log, context = {}) {
 					tasksPath,
 					useResearch
 				},
-				fromCache: false // This operation always modifies state and should never be cached
+				fromCache: false
 			};
 		} catch (error) {
-			// Make sure to restore normal logging even if there's an error
-			disableSilentMode();
-			throw error; // Rethrow to be caught by outer catch block
+			logWrapper.error(`Error updating subtask by ID: ${error.message}`);
+			return {
+				success: false,
+				error: {
+					code: 'UPDATE_SUBTASK_CORE_ERROR',
+					message: error.message || 'Unknown error updating subtask'
+				},
+				fromCache: false
+			};
+		} finally {
+			if (!wasSilent && isSilentMode()) {
+				disableSilentMode();
+			}
 		}
 	} catch (error) {
-		// Ensure silent mode is disabled
-		disableSilentMode();
-
-		log.error(`Error updating subtask by ID: ${error.message}`);
+		logWrapper.error(
+			`Setup error in updateSubtaskByIdDirect: ${error.message}`
+		);
+		if (isSilentMode()) disableSilentMode();
 		return {
 			success: false,
 			error: {
-				code: 'UPDATE_SUBTASK_ERROR',
-				message: error.message || 'Unknown error updating subtask'
+				code: 'DIRECT_FUNCTION_SETUP_ERROR',
+				message: error.message || 'Unknown setup error'
 			},
 			fromCache: false
 		};

mcp-server/src/tools/update-subtask.js

@@ -4,13 +4,10 @@
  */
 
 import { z } from 'zod';
-import {
-	handleApiResult,
-	createErrorResponse,
-	getProjectRootFromSession
-} from './utils.js';
+import { handleApiResult, createErrorResponse } from './utils.js';
 import { updateSubtaskByIdDirect } from '../core/task-master-core.js';
 import { findTasksJsonPath } from '../core/utils/path-utils.js';
+import path from 'path';
 
 /**
  * Register the update-subtask tool with the MCP server
@@ -38,21 +35,23 @@ export function registerUpdateSubtaskTool(server) {
 				.describe('The directory of the project. Must be an absolute path.')
 		}),
 		execute: async (args, { log, session }) => {
+			const toolName = 'update_subtask';
 			try {
 				log.info(`Updating subtask with args: ${JSON.stringify(args)}`);
 
-				// Get project root from args or session
-				const rootFolder =
-					args.projectRoot || getProjectRootFromSession(session, log);
-
-				// Ensure project root was determined
-				if (!rootFolder) {
+				// 1. Get Project Root
+				const rootFolder = args.projectRoot;
+				if (!rootFolder || !path.isAbsolute(rootFolder)) {
+					log.error(
+						`${toolName}: projectRoot is required and must be absolute.`
+					);
 					return createErrorResponse(
-						'Could not determine project root. Please provide it explicitly or ensure your session contains valid root information.'
+						'projectRoot is required and must be absolute.'
 					);
 				}
+				log.info(`${toolName}: Project root: ${rootFolder}`);
 
-				// Resolve the path to tasks.json
+				// 2. Resolve Tasks Path
 				let tasksJsonPath;
 				try {
 					tasksJsonPath = findTasksJsonPath(
@@ -60,20 +59,20 @@ export function registerUpdateSubtaskTool(server) {
 						log
 					);
 				} catch (error) {
-					log.error(`Error finding tasks.json: ${error.message}`);
+					log.error(`${toolName}: Error finding tasks.json: ${error.message}`);
 					return createErrorResponse(
-						`Failed to find tasks.json: ${error.message}`
+						`Failed to find tasks.json within project root '${rootFolder}': ${error.message}`
 					);
 				}
 
+				// 3. Call Direct Function - Include projectRoot
 				const result = await updateSubtaskByIdDirect(
 					{
-						// Pass the explicitly resolved path
 						tasksJsonPath: tasksJsonPath,
-						// Pass other relevant args
 						id: args.id,
 						prompt: args.prompt,
-						research: args.research
+						research: args.research,
+						projectRoot: rootFolder
 					},
 					log,
 					{ session }
@@ -89,8 +88,12 @@ export function registerUpdateSubtaskTool(server) {
 
 				return handleApiResult(result, log, 'Error updating subtask');
 			} catch (error) {
-				log.error(`Error in update_subtask tool: ${error.message}`);
-				return createErrorResponse(error.message);
+				log.error(
+					`Critical error in ${toolName} tool execute: ${error.message}`
+				);
+				return createErrorResponse(
+					`Internal tool error (${toolName}): ${error.message}`
+				);
 			}
 		}
 	});