From 21392a1117d9a13df36948a23d2bd43514494519 Mon Sep 17 00:00:00 2001 From: Ralph Khreish <35776126+Crunchyman-ralph@users.noreply.github.com> Date: Fri, 11 Jul 2025 15:23:54 +0300 Subject: [PATCH] fix: more regression bugs (#956) * fix: more regression bugs * chore: fix format * chore: fix unit tests * chore: fix format --- scripts/modules/commands.js | 26 ++++++++--- scripts/modules/config-manager.js | 45 +++++++++++++++---- scripts/modules/task-manager/expand-task.js | 2 +- scripts/modules/task-manager/research.js | 6 +-- .../task-manager/update-subtask-by-id.js | 4 +- src/task-master.js | 11 ++++- tests/unit/config-manager.test.js | 5 ++- .../analyze-task-complexity.test.js | 9 +++- tests/unit/task-master.test.js | 30 +++++++++---- 9 files changed, 106 insertions(+), 32 deletions(-) diff --git a/scripts/modules/commands.js b/scripts/modules/commands.js index daa1fcb0..4dabf8a7 100644 --- a/scripts/modules/commands.js +++ b/scripts/modules/commands.js @@ -1500,10 +1500,16 @@ function registerCommands(programInstance) { .option('--tag ', 'Specify tag context for task operations') .action(async (options) => { // Initialize TaskMaster - const taskMaster = initTaskMaster({ - tasksPath: options.file || true, - complexityReportPath: options.report || false - }); + const initOptions = { + tasksPath: options.file || true + }; + + // Only pass complexityReportPath if user provided a custom path + if (options.report && options.report !== COMPLEXITY_REPORT_FILE) { + initOptions.complexityReportPath = options.report; + } + + const taskMaster = initTaskMaster(initOptions); const statusFilter = options.status; const withSubtasks = options.withSubtasks || false; @@ -1690,7 +1696,7 @@ function registerCommands(programInstance) { const outputPath = options.output === COMPLEXITY_REPORT_FILE && targetTag !== 'master' ? baseOutputPath.replace('.json', `_${targetTag}.json`) - : baseOutputPath; + : options.output || baseOutputPath; console.log( chalk.blue( @@ -1770,6 +1776,11 @@ function registerCommands(programInstance) { ) .option('--tag ', 'Specify tag context for task operations') .action(async (prompt, options) => { + // Initialize TaskMaster + const taskMaster = initTaskMaster({ + tasksPath: options.file || true + }); + // Parameter validation if (!prompt || typeof prompt !== 'string' || prompt.trim().length === 0) { console.error( @@ -2211,6 +2222,8 @@ ${result.result} tasksPath: options.file || true }); + const projectRoot = taskMaster.getProjectRoot(); + // Show current tag context displayCurrentTagIndicator( options.tag || getCurrentTag(taskMaster.getProjectRoot()) || 'master' @@ -3462,6 +3475,9 @@ Examples: const taskMaster = initTaskMaster({ tasksPath: options.file || false }); + + const projectRoot = taskMaster.getProjectRoot(); + // Validate flags: cannot use multiple provider flags simultaneously const providerFlags = [ options.openrouter, diff --git a/scripts/modules/config-manager.js b/scripts/modules/config-manager.js index b90cb57e..e688897b 100644 --- a/scripts/modules/config-manager.js +++ b/scripts/modules/config-manager.js @@ -4,7 +4,10 @@ import chalk from 'chalk'; import { z } from 'zod'; import { fileURLToPath } from 'url'; import { log, findProjectRoot, resolveEnvVariable, isEmpty } from './utils.js'; -import { LEGACY_CONFIG_FILE } from '../../src/constants/paths.js'; +import { + LEGACY_CONFIG_FILE, + TASKMASTER_DIR +} from '../../src/constants/paths.js'; import { findConfigPath } from '../../src/utils/path-utils.js'; import { VALIDATED_PROVIDERS, @@ -99,17 +102,30 @@ function _loadAndValidateConfig(explicitRoot = null) { if (rootToUse) { configSource = `found root (${rootToUse})`; } else { - // No root found, return defaults immediately - return defaults; + // No root found, use current working directory as fallback + // This prevents infinite loops during initialization + rootToUse = process.cwd(); + configSource = `current directory (${rootToUse}) - no project markers found`; } } // ---> End find project root logic <--- - // --- Find configuration file using centralized path utility --- - const configPath = findConfigPath(null, { projectRoot: rootToUse }); + // --- Find configuration file --- + let configPath = null; let config = { ...defaults }; // Start with a deep copy of defaults let configExists = false; + // During initialization (no project markers), skip config file search entirely + const hasProjectMarkers = + fs.existsSync(path.join(rootToUse, TASKMASTER_DIR)) || + fs.existsSync(path.join(rootToUse, LEGACY_CONFIG_FILE)); + + if (hasProjectMarkers) { + // Only try to find config if we have project markers + // This prevents the repeated warnings during init + configPath = findConfigPath(null, { projectRoot: rootToUse }); + } + if (configPath) { configExists = true; const isLegacy = configPath.endsWith(LEGACY_CONFIG_FILE); @@ -199,11 +215,22 @@ function _loadAndValidateConfig(explicitRoot = null) { ) ); } else { - console.warn( - chalk.yellow( - `Warning: Configuration file not found at derived root (${rootToUse}). Using defaults.` - ) + // Don't warn about missing config during initialization + // Only warn if this looks like an existing project (has .taskmaster dir or legacy config marker) + const hasTaskmasterDir = fs.existsSync( + path.join(rootToUse, TASKMASTER_DIR) ); + const hasLegacyMarker = fs.existsSync( + path.join(rootToUse, LEGACY_CONFIG_FILE) + ); + + if (hasTaskmasterDir || hasLegacyMarker) { + console.warn( + chalk.yellow( + `Warning: Configuration file not found at derived root (${rootToUse}). Using defaults.` + ) + ); + } } // Keep config as defaults config = { ...defaults }; diff --git a/scripts/modules/task-manager/expand-task.js b/scripts/modules/task-manager/expand-task.js index 641297eb..379e0223 100644 --- a/scripts/modules/task-manager/expand-task.js +++ b/scripts/modules/task-manager/expand-task.js @@ -469,7 +469,7 @@ async function expandTask( complexityReasoningContext: complexityReasoningContext, gatheredContext: gatheredContext, useResearch: useResearch, - expansionPrompt: taskAnalysis?.expansionPrompt || null + expansionPrompt: taskAnalysis?.expansionPrompt || undefined }; let variantKey = 'default'; diff --git a/scripts/modules/task-manager/research.js b/scripts/modules/task-manager/research.js index 44fc2221..9ab1adff 100644 --- a/scripts/modules/task-manager/research.js +++ b/scripts/modules/task-manager/research.js @@ -205,12 +205,10 @@ async function performResearch( } }; - // Select variant based on detail level - const variantKey = detailLevel; // 'low', 'medium', or 'high' + // Load prompts - the research template handles detail level internally const { systemPrompt, userPrompt } = await promptManager.loadPrompt( 'research', - promptParams, - variantKey + promptParams ); // Count tokens for system and user prompts diff --git a/scripts/modules/task-manager/update-subtask-by-id.js b/scripts/modules/task-manager/update-subtask-by-id.js index 27dfbed7..ee12a81d 100644 --- a/scripts/modules/task-manager/update-subtask-by-id.js +++ b/scripts/modules/task-manager/update-subtask-by-id.js @@ -214,7 +214,7 @@ async function updateSubtaskById( title: parentTask.subtasks[subtaskIndex - 1].title, status: parentTask.subtasks[subtaskIndex - 1].status } - : null; + : undefined; const nextSubtask = subtaskIndex < parentTask.subtasks.length - 1 ? { @@ -222,7 +222,7 @@ async function updateSubtaskById( title: parentTask.subtasks[subtaskIndex + 1].title, status: parentTask.subtasks[subtaskIndex + 1].status } - : null; + : undefined; // Build prompts using PromptManager const promptManager = getPromptManager(); diff --git a/src/task-master.js b/src/task-master.js index 71eefb0e..3e92a4ca 100644 --- a/src/task-master.js +++ b/src/task-master.js @@ -218,7 +218,16 @@ export function initTaskMaster(overrides = {}) { ); } - // Remaining paths - only resolve if key exists in overrides + // Always set default paths first + // These can be overridden below if needed + paths.configPath = path.join(paths.projectRoot, TASKMASTER_CONFIG_FILE); + paths.statePath = path.join( + paths.taskMasterDir || path.join(paths.projectRoot, TASKMASTER_DIR), + 'state.json' + ); + paths.tasksPath = path.join(paths.projectRoot, TASKMASTER_TASKS_FILE); + + // Handle overrides - only validate/resolve if explicitly provided if ('configPath' in overrides) { paths.configPath = resolvePath( 'config file', diff --git a/tests/unit/config-manager.test.js b/tests/unit/config-manager.test.js index a0f315a3..157ad98b 100644 --- a/tests/unit/config-manager.test.js +++ b/tests/unit/config-manager.test.js @@ -557,7 +557,10 @@ describe('getConfig Tests', () => { // Assert expect(config).toEqual(DEFAULT_CONFIG); expect(mockFindProjectRoot).not.toHaveBeenCalled(); // Explicit root provided - expect(fsExistsSyncSpy).toHaveBeenCalledWith(MOCK_CONFIG_PATH); + // The implementation checks for .taskmaster directory first + expect(fsExistsSyncSpy).toHaveBeenCalledWith( + path.join(MOCK_PROJECT_ROOT, '.taskmaster') + ); expect(fsReadFileSyncSpy).not.toHaveBeenCalled(); // No read if file doesn't exist expect(consoleWarnSpy).toHaveBeenCalledWith( expect.stringContaining('not found at provided project root') diff --git a/tests/unit/scripts/modules/task-manager/analyze-task-complexity.test.js b/tests/unit/scripts/modules/task-manager/analyze-task-complexity.test.js index 4662126f..a5e64935 100644 --- a/tests/unit/scripts/modules/task-manager/analyze-task-complexity.test.js +++ b/tests/unit/scripts/modules/task-manager/analyze-task-complexity.test.js @@ -184,7 +184,7 @@ jest.unstable_mockModule( ); // Import the mocked modules -const { readJSON, writeJSON, log, CONFIG } = await import( +const { readJSON, writeJSON, log, CONFIG, findTaskById } = await import( '../../../../../scripts/modules/utils.js' ); @@ -265,6 +265,13 @@ describe('analyzeTaskComplexity', () => { _rawTaggedData: sampleTasks }; }); + + // Mock findTaskById to return the expected structure + findTaskById.mockImplementation((tasks, taskId) => { + const task = tasks?.find((t) => t.id === parseInt(taskId)); + return { task: task || null, originalSubtaskCount: null }; + }); + generateTextService.mockResolvedValue(sampleApiResponse); }); diff --git a/tests/unit/task-master.test.js b/tests/unit/task-master.test.js index 26067d12..c3b9c48b 100644 --- a/tests/unit/task-master.test.js +++ b/tests/unit/task-master.test.js @@ -248,7 +248,7 @@ describe('initTaskMaster', () => { expect(taskMaster.getTasksPath()).toBeNull(); }); - test('should return null when optional files not specified in overrides', () => { + test('should return default paths when optional files not specified in overrides', () => { // Arrange - Remove all optional files fs.unlinkSync(tasksPath); fs.unlinkSync(configPath); @@ -257,10 +257,16 @@ describe('initTaskMaster', () => { // Act - Don't specify any optional paths const taskMaster = initTaskMaster({}); - // Assert - expect(taskMaster.getTasksPath()).toBeUndefined(); - expect(taskMaster.getConfigPath()).toBeUndefined(); - expect(taskMaster.getStatePath()).toBeUndefined(); + // Assert - Should return absolute paths with default locations + expect(taskMaster.getTasksPath()).toBe( + path.join(tempDir, TASKMASTER_TASKS_FILE) + ); + expect(taskMaster.getConfigPath()).toBe( + path.join(tempDir, TASKMASTER_CONFIG_FILE) + ); + expect(taskMaster.getStatePath()).toBe( + path.join(tempDir, TASKMASTER_DIR, 'state.json') + ); }); }); @@ -415,11 +421,19 @@ describe('initTaskMaster', () => { // Assert expect(taskMaster.getProjectRoot()).toBe(tempDir); expect(taskMaster.getTaskMasterDir()).toBe(taskMasterDir); - expect(taskMaster.getTasksPath()).toBeUndefined(); + // Default paths are always set for tasks, config, and state + expect(taskMaster.getTasksPath()).toBe( + path.join(tempDir, TASKMASTER_TASKS_FILE) + ); + expect(taskMaster.getConfigPath()).toBe( + path.join(tempDir, TASKMASTER_CONFIG_FILE) + ); + expect(taskMaster.getStatePath()).toBe( + path.join(taskMasterDir, 'state.json') + ); + // PRD and complexity report paths are undefined when not provided expect(taskMaster.getPrdPath()).toBeUndefined(); expect(taskMaster.getComplexityReportPath()).toBeUndefined(); - expect(taskMaster.getConfigPath()).toBeUndefined(); - expect(taskMaster.getStatePath()).toBeUndefined(); }); }); });