diff --git a/apps/cli/src/utils/auto-update.ts b/apps/cli/src/utils/auto-update.ts index 0c49d848..3ad0f911 100644 --- a/apps/cli/src/utils/auto-update.ts +++ b/apps/cli/src/utils/auto-update.ts @@ -163,9 +163,13 @@ export async function performAutoUpdate( process.env.CI || process.env.NODE_ENV === 'test' ) { - console.log( - chalk.dim('Skipping auto-update (TASKMASTER_SKIP_AUTO_UPDATE/CI).') - ); + const reason = + process.env.TASKMASTER_SKIP_AUTO_UPDATE === '1' + ? 'TASKMASTER_SKIP_AUTO_UPDATE=1' + : process.env.CI + ? 'CI environment' + : 'NODE_ENV=test'; + console.log(chalk.dim(`Skipping auto-update (${reason})`)); return false; } const spinner = ora({ diff --git a/scripts/modules/task-manager/expand-task.js b/scripts/modules/task-manager/expand-task.js index f6d5897d..853d9c51 100644 --- a/scripts/modules/task-manager/expand-task.js +++ b/scripts/modules/task-manager/expand-task.js @@ -290,7 +290,7 @@ async function expandTask( ); // --- End Complexity Report / Prompt Logic --- - // --- AI Subtask Generation using generateTextService --- + // --- AI Subtask Generation using generateObjectService --- let generatedSubtasks = []; let loadingIndicator = null; if (outputFormat === 'text') { @@ -318,8 +318,12 @@ async function expandTask( outputType: outputFormat }); - // With generateObject, we get structured data directly - generatedSubtasks = aiServiceResponse.mainResult.subtasks; + // With generateObject, we expect structured data – verify it before use + const mainResult = aiServiceResponse?.mainResult; + if (!mainResult || !Array.isArray(mainResult.subtasks)) { + throw new Error('AI response did not include a valid subtasks array.'); + } + generatedSubtasks = mainResult.subtasks; logger.info(`Received ${generatedSubtasks.length} subtasks from AI.`); } catch (error) { if (loadingIndicator) stopLoadingIndicator(loadingIndicator); diff --git a/src/ai-providers/base-provider.js b/src/ai-providers/base-provider.js index a057b15c..7409ad20 100644 --- a/src/ai-providers/base-provider.js +++ b/src/ai-providers/base-provider.js @@ -21,6 +21,13 @@ export class BaseAIProvider { // Each provider must set their name this.name = this.constructor.name; + + /** + * Whether this provider needs explicit schema in JSON mode + * Can be overridden by subclasses + * @type {boolean} + */ + this.needsExplicitJsonSchema = false; } /** @@ -273,15 +280,11 @@ export class BaseAIProvider { const client = await this.getClient(params); - // For providers that don't support tool mode (like claude-code), - // we need to ensure the schema is properly communicated in the prompt - const needsExplicitSchema = this.name === 'Claude Code'; - const result = await generateObject({ model: client(params.modelId), messages: params.messages, schema: params.schema, - mode: needsExplicitSchema ? 'json' : 'auto', + mode: this.needsExplicitJsonSchema ? 'json' : 'auto', schemaName: params.objectName, schemaDescription: `Generate a valid JSON object for ${params.objectName}`, maxTokens: params.maxTokens, @@ -305,7 +308,7 @@ export class BaseAIProvider { // Check if this is a JSON parsing error that we can potentially fix if ( NoObjectGeneratedError.isInstance(error) && - JSONParseError.isInstance(error.cause) && + error.cause instanceof JSONParseError && error.cause.text ) { log( diff --git a/src/ai-providers/claude-code.js b/src/ai-providers/claude-code.js index a3275057..3014b490 100644 --- a/src/ai-providers/claude-code.js +++ b/src/ai-providers/claude-code.js @@ -32,6 +32,8 @@ export class ClaudeCodeProvider extends BaseAIProvider { super(); this.name = 'Claude Code'; this.supportedModels = ['sonnet', 'opus']; + // Claude Code requires explicit JSON schema mode + this.needsExplicitJsonSchema = true; } /** diff --git a/src/ai-providers/gemini-cli.js b/src/ai-providers/gemini-cli.js index 47aea4fd..1801695e 100644 --- a/src/ai-providers/gemini-cli.js +++ b/src/ai-providers/gemini-cli.js @@ -15,6 +15,8 @@ export class GeminiCliProvider extends BaseAIProvider { constructor() { super(); this.name = 'Gemini CLI'; + // Gemini CLI requires explicit JSON schema mode + this.needsExplicitJsonSchema = true; } /** @@ -587,7 +589,7 @@ Generate ${subtaskCount} subtasks based on the original task context. Return ONL system: systemPrompt, messages: messages, schema: params.schema, - mode: 'json', // Use json mode instead of auto for Gemini + mode: this.needsExplicitJsonSchema ? 'json' : 'auto', maxOutputTokens: params.maxTokens, temperature: params.temperature }); diff --git a/src/ai-providers/grok-cli.js b/src/ai-providers/grok-cli.js index 0934e8e9..fa73155b 100644 --- a/src/ai-providers/grok-cli.js +++ b/src/ai-providers/grok-cli.js @@ -11,6 +11,8 @@ export class GrokCliProvider extends BaseAIProvider { constructor() { super(); this.name = 'Grok CLI'; + // Grok CLI requires explicit JSON schema mode + this.needsExplicitJsonSchema = true; } /** diff --git a/src/prompts/analyze-complexity.json b/src/prompts/analyze-complexity.json index e517e22f..a9b39a70 100644 --- a/src/prompts/analyze-complexity.json +++ b/src/prompts/analyze-complexity.json @@ -44,7 +44,7 @@ }, "prompts": { "default": { - "system": "You are an expert software architect and project manager analyzing task complexity. Your analysis should consider implementation effort, technical challenges, dependencies, and testing requirements.\n\nIMPORTANT: For each task, provide an analysis object with ALL of the following fields:\n- taskId: The ID of the task being analyzed (positive integer)\n- taskTitle: The title of the task\n- complexityScore: A score from 1-10 indicating complexity\n- recommendedSubtasks: Number of subtasks recommended (positive integer)\n- expansionPrompt: A prompt to guide subtask generation\n- reasoning: Your reasoning for the complexity score", + "system": "You are an expert software architect and project manager analyzing task complexity. Your analysis should consider implementation effort, technical challenges, dependencies, and testing requirements.\n\nIMPORTANT: For each task, provide an analysis object with ALL of the following fields:\n- taskId: The ID of the task being analyzed (positive integer)\n- taskTitle: The title of the task\n- complexityScore: A score from 1-10 indicating complexity\n- recommendedSubtasks: Number of subtasks recommended (non-negative integer; 0 if no expansion needed)\n- expansionPrompt: A prompt to guide subtask generation\n- reasoning: Your reasoning for the complexity score", "user": "{{#if hasCodebaseAnalysis}}## IMPORTANT: Codebase Analysis Required\n\nYou have access to powerful codebase analysis tools. Before analyzing task complexity:\n\n1. Use the Glob tool to explore the project structure and understand the codebase size\n2. Use the Grep tool to search for existing implementations related to each task\n3. Use the Read tool to examine key files that would be affected by these tasks\n4. Understand the current implementation state, patterns used, and technical debt\n\nBased on your codebase analysis:\n- Assess complexity based on ACTUAL code that needs to be modified/created\n- Consider existing abstractions and patterns that could simplify implementation\n- Identify tasks that require refactoring vs. greenfield development\n- Factor in dependencies between existing code and new features\n- Provide more accurate subtask recommendations based on real code structure\n\nProject Root: {{projectRoot}}\n\n{{/if}}Analyze the following tasks to determine their complexity (1-10 scale) and recommend the number of subtasks for expansion. Provide a brief reasoning and an initial expansion prompt for each.{{#if useResearch}} Consider current best practices, common implementation patterns, and industry standards in your analysis.{{/if}}\n\nTasks:\n{{{json tasks}}}\n{{#if gatheredContext}}\n\n# Project Context\n\n{{gatheredContext}}\n{{/if}}\n" } } diff --git a/src/schemas/analyze-complexity.js b/src/schemas/analyze-complexity.js index 6918d91d..621e488a 100644 --- a/src/schemas/analyze-complexity.js +++ b/src/schemas/analyze-complexity.js @@ -4,7 +4,7 @@ export const ComplexityAnalysisItemSchema = z.object({ taskId: z.number().int().positive(), taskTitle: z.string(), complexityScore: z.number().min(1).max(10), - recommendedSubtasks: z.number().int().positive(), + recommendedSubtasks: z.number().int().nonnegative(), expansionPrompt: z.string(), reasoning: z.string() }); diff --git a/tests/integration/cli/complex-cross-tag-scenarios.test.js b/tests/integration/cli/complex-cross-tag-scenarios.test.js index 276ded57..eaf0f7a9 100644 --- a/tests/integration/cli/complex-cross-tag-scenarios.test.js +++ b/tests/integration/cli/complex-cross-tag-scenarios.test.js @@ -379,9 +379,24 @@ describe('Complex Cross-Tag Scenarios', () => { // Verify the move was successful const tasksAfter = JSON.parse(fs.readFileSync(tasksPath, 'utf8')); - expect( - tasksAfter['in-progress'].tasks.find((t) => t.id === 25) - ).toBeDefined(); + + // Verify all tasks in the dependency chain were moved + for (let i = 1; i <= 25; i++) { + expect(tasksAfter.master.tasks.find((t) => t.id === i)).toBeUndefined(); + expect( + tasksAfter['in-progress'].tasks.find((t) => t.id === i) + ).toBeDefined(); + } + + // Verify in-progress still has its original tasks (26-50) + for (let i = 26; i <= 50; i++) { + expect( + tasksAfter['in-progress'].tasks.find((t) => t.id === i) + ).toBeDefined(); + } + + // Final count check + expect(tasksAfter['in-progress'].tasks).toHaveLength(50); // 25 moved + 25 original }); }); diff --git a/tests/unit/prompts/expand-task-prompt.test.js b/tests/unit/prompts/expand-task-prompt.test.js index 40b8aee8..5aacb025 100644 --- a/tests/unit/prompts/expand-task-prompt.test.js +++ b/tests/unit/prompts/expand-task-prompt.test.js @@ -1,4 +1,6 @@ import { PromptManager } from '../../../scripts/modules/prompt-manager.js'; +import { ExpandTaskResponseSchema } from '../../../src/schemas/expand-task.js'; +import { SubtaskSchema } from '../../../src/schemas/base-schemas.js'; describe('expand-task prompt template', () => { let promptManager; @@ -77,29 +79,21 @@ describe('expand-task prompt template', () => { expect(userPrompt).toContain(params.complexityReasoningContext); }); - test('all variants request structured subtasks with required fields', () => { - const variants = ['default', 'research', 'complexity-report']; + test('ExpandTaskResponseSchema defines required subtask fields', () => { + // Test the schema definition directly instead of weak substring matching + const schema = ExpandTaskResponseSchema; + const subtasksSchema = schema.shape.subtasks; + const subtaskSchema = subtasksSchema.element; - variants.forEach((variant) => { - const params = - variant === 'complexity-report' - ? { ...baseParams, expansionPrompt: 'test' } - : baseParams; - - const { systemPrompt, userPrompt } = promptManager.loadPrompt( - 'expand-task', - params, - variant - ); - const combined = systemPrompt + userPrompt; - - // Verify prompts describe the structured output format - expect(combined.toLowerCase()).toContain('subtasks'); - expect(combined).toContain('id'); - expect(combined).toContain('title'); - expect(combined).toContain('description'); - expect(combined).toContain('dependencies'); - }); + // Verify the schema has the required fields + expect(subtaskSchema).toBe(SubtaskSchema); + expect(SubtaskSchema.shape).toHaveProperty('id'); + expect(SubtaskSchema.shape).toHaveProperty('title'); + expect(SubtaskSchema.shape).toHaveProperty('description'); + expect(SubtaskSchema.shape).toHaveProperty('dependencies'); + expect(SubtaskSchema.shape).toHaveProperty('details'); + expect(SubtaskSchema.shape).toHaveProperty('status'); + expect(SubtaskSchema.shape).toHaveProperty('testStrategy'); }); test('complexity-report variant fails without task context regression test', () => { diff --git a/tests/unit/prompts/prompt-migration.test.js b/tests/unit/prompts/prompt-migration.test.js index a513a107..f60572f8 100644 --- a/tests/unit/prompts/prompt-migration.test.js +++ b/tests/unit/prompts/prompt-migration.test.js @@ -15,9 +15,10 @@ describe('Prompt Migration Validation', () => { 'code block markers' ]; - // Special cases where phrases are okay in different contexts + // Map banned phrases to contexts where they're allowed const allowedContexts = { - 'markdown formatting': ['Use markdown formatting for better readability'] + 'respond only with': ['Use markdown formatting for better readability'], + 'return only the': ['Use markdown formatting for better readability'] }; test('prompts should not contain JSON formatting instructions', () => { @@ -29,7 +30,6 @@ describe('Prompt Migration Validation', () => { promptFiles.forEach((file) => { const content = fs.readFileSync(path.join(promptsDir, file), 'utf8'); - const promptData = JSON.parse(content); bannedPhrases.forEach((phrase) => { const lowerContent = content.toLowerCase(); diff --git a/tests/unit/scripts/modules/task-manager/expand-task.test.js b/tests/unit/scripts/modules/task-manager/expand-task.test.js index 54fed9d2..6b807de3 100644 --- a/tests/unit/scripts/modules/task-manager/expand-task.test.js +++ b/tests/unit/scripts/modules/task-manager/expand-task.test.js @@ -660,25 +660,38 @@ describe('expandTask', () => { // Act await expandTask(tasksPath, taskId, 3, false, '', context, false); - // Assert - Should append to existing subtasks with proper ID increments - expect(writeJSON).toHaveBeenCalledWith( - tasksPath, + // Assert - Verify generateObjectService was called correctly + expect(generateObjectService).toHaveBeenCalledWith( expect.objectContaining({ - tasks: expect.arrayContaining([ - expect.objectContaining({ - id: 4, - subtasks: expect.arrayContaining([ - // Should contain both existing and new subtasks - expect.any(Object), - expect.any(Object), - expect.any(Object), - expect.any(Object) // 1 existing + 3 new = 4 total - ]) - }) - ]) - }), - '/mock/project/root', - undefined + role: 'main', + commandName: 'expand-task', + objectName: 'subtasks' + }) + ); + + // Assert - Verify data was written with appended subtasks + expect(writeJSON).toHaveBeenCalled(); + const writeCall = writeJSON.mock.calls[0]; + const savedData = writeCall[1]; // Second argument is the data + const task4 = savedData.tasks.find((t) => t.id === 4); + + // Should have 4 subtasks total (1 existing + 3 new) + expect(task4.subtasks).toHaveLength(4); + + // Verify existing subtask is preserved at index 0 + expect(task4.subtasks[0]).toEqual( + expect.objectContaining({ + id: 1, + title: 'Existing subtask' + }) + ); + + // Verify new subtasks were appended (they start with id=1 from AI) + expect(task4.subtasks[1]).toEqual( + expect.objectContaining({ + id: 1, + title: 'Set up project structure' + }) ); }); }); @@ -843,6 +856,54 @@ describe('expandTask', () => { expect(writeJSON).not.toHaveBeenCalled(); }); + test('should handle missing mainResult from AI response', async () => { + // Arrange + const tasksPath = 'tasks/tasks.json'; + const taskId = '2'; + const context = { + mcpLog: createMcpLogMock(), + projectRoot: '/mock/project/root' + }; + + // Mock AI service returning response without mainResult + generateObjectService.mockResolvedValueOnce({ + telemetryData: { inputTokens: 100, outputTokens: 50 } + // Missing mainResult + }); + + // Act & Assert + await expect( + expandTask(tasksPath, taskId, 3, false, '', context, false) + ).rejects.toThrow('AI response did not include a valid subtasks array.'); + + expect(writeJSON).not.toHaveBeenCalled(); + }); + + test('should handle invalid subtasks array from AI response', async () => { + // Arrange + const tasksPath = 'tasks/tasks.json'; + const taskId = '2'; + const context = { + mcpLog: createMcpLogMock(), + projectRoot: '/mock/project/root' + }; + + // Mock AI service returning response with invalid subtasks + generateObjectService.mockResolvedValueOnce({ + mainResult: { + subtasks: 'not-an-array' // Invalid: should be an array + }, + telemetryData: { inputTokens: 100, outputTokens: 50 } + }); + + // Act & Assert + await expect( + expandTask(tasksPath, taskId, 3, false, '', context, false) + ).rejects.toThrow('AI response did not include a valid subtasks array.'); + + expect(writeJSON).not.toHaveBeenCalled(); + }); + test('should handle file read errors', async () => { // Arrange const tasksPath = 'tasks/tasks.json';