auto-claude: 192-changelog-generation-multiple-critical-bugs-tasks- (#1725)

* auto-claude: subtask-1-1 - Create task-status.ts utility with isCompletedTask

* auto-claude: subtask-1-2 - Create unit tests for isCompletedTask() utility covering all edge cases

- Added comprehensive unit tests for isCompletedTask() utility function
- Tests cover all TaskStatus values: done, pr_created, backlog, queue, in_progress, ai_review, human_review, error
- Includes edge case testing for human_review with different ReviewReasons (completed, errors, qa_rejected, plan_review)
- Tests archived task behavior (archived status metadata doesn't affect completion logic)
- Includes type safety tests and real-world usage scenarios
- Tests boundary conditions with array operations (filter, map, reduce)
- All tests use Vitest framework matching frontend patterns

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* auto-claude: subtask-2-1 - Update changelog-service.ts to use isCompletedTask

* auto-claude: subtask-2-4 - Update changelog-mock.ts test data to include pr_created and human_review completion states

* auto-claude: subtask-3-1 - Refactor CHANGELOG_GENERATE handler from ipcMain.on() to ipcMain.handle() with try-catch wrapper

* auto-claude: subtask-3-2 - Update renderer IPC call to use invoke() instead of send()

* auto-claude: subtask-4-1 - Implement timeout wrapper around spawn() call with setTimeout and process.kill

* auto-claude: subtask-5-1 - Implement acknowledgment pattern - return from han

* auto-claude: subtask-6-1 - Track and remove previous listeners before registering new ones

* auto-claude: subtask-7-1 - Create integration test for task filtering with al

* auto-claude: subtask-7-2 - Create integration test for subprocess timeout mec

* auto-claude: subtask-7-5 - Run full test suite to ensure no regressions

- Fixed TypeScript error in task-status.test.ts filter predicate
- All 2682 tests passing with 6 skipped (no regressions)
- TypeScript typecheck passes with no errors
- Verified changelog bug fixes working correctly
- Task status filtering includes done/pr_created/human_review+completed
- Subprocess timeout protection functional
- IPC error handling with invoke/handle pattern working
- Progress event race condition resolved
- Memory leak prevention implemented

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* Fix 4 PR review findings in changelog module

- Use isCompletedTask() utility in changelog mock instead of inline
  status filter that incorrectly included all human_review tasks
- Remove unused sendIpc import from changelog-api.ts after refactor
  to invokeIpc
- Add guard in generator exit handler to prevent double error emission
  when timeout fires before process exits

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix path traversal vulnerabilities and cleanup in changelog module

- Sanitize filename with path.basename() in saveChangelogImage to
  prevent writing files outside .github/assets
- Validate resolved path stays within projectPath in readLocalImage
  to prevent arbitrary file reads
- Remove duplicate DEBUG conditions in isDebugEnabled
- Simplify mock filter type guard

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix path traversal edge case, duplicate error handling, and error handler guard

- Add path.sep to path traversal check in readLocalImage to prevent
  sibling directory reads (e.g. /project-other matching /project)
- Add guard in generator error handler to skip if process already
  cleaned up by timeout, preventing duplicate error emissions
- Extract handleGenerationError helper in changelog store to
  deduplicate error handling logic

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-authored-by: Test User <test@example.com>
This commit is contained in:
Andy
2026-02-08 22:56:41 +01:00
committed by GitHub
parent 8a7443d24d
commit 8c9a504df9
13 changed files with 1326 additions and 77 deletions
@@ -0,0 +1,459 @@
/**
* Integration tests for ChangelogService task filtering
* Tests task filtering with all completion states: done, pr_created, and human_review+completed
*/
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { mkdirSync, mkdtempSync, writeFileSync, rmSync, existsSync } from 'fs';
import { tmpdir } from 'os';
import path from 'path';
import type { Task } from '../../../shared/types';
// Mock electron modules
vi.mock('electron', () => ({
app: {
getPath: vi.fn((name: string) => {
if (name === 'userData') return path.join(tmpdir(), 'test-userdata');
return tmpdir();
}),
getAppPath: vi.fn(() => tmpdir()),
getVersion: vi.fn(() => '0.1.0'),
isPackaged: false
}
}));
vi.mock('../../cli-tool-manager', () => ({
getToolPath: vi.fn((tool: string) => tool),
getToolInfo: vi.fn(() => ({ found: true, path: '/usr/bin/claude', source: 'mock' }))
}));
vi.mock('../../python-detector', () => ({
getValidatedPythonPath: vi.fn((p: string) => p)
}));
vi.mock('../../python-env-manager', () => ({
getConfiguredPythonPath: vi.fn(() => '/usr/bin/python3')
}));
describe('ChangelogService - Task Filtering Integration', () => {
let testDir: string;
let projectPath: string;
let specsDir: string;
beforeEach(() => {
// Create temporary test directory
testDir = mkdtempSync(path.join(tmpdir(), 'changelog-test-'));
projectPath = path.join(testDir, 'test-project');
specsDir = path.join(projectPath, '.auto-claude', 'specs');
// Create project structure
mkdirSync(projectPath, { recursive: true });
mkdirSync(specsDir, { recursive: true });
});
afterEach(() => {
// Cleanup test directory
if (existsSync(testDir)) {
rmSync(testDir, { recursive: true, force: true });
}
vi.clearAllMocks();
});
describe('getCompletedTasks', () => {
it('should include tasks with "done" status', async () => {
const { ChangelogService } = await import('../changelog-service');
const service = new ChangelogService();
const tasks: Task[] = [
{
id: 'task-1',
specId: '001-test-feature',
projectId: 'project-1',
title: 'Test Feature',
description: 'A test feature',
status: 'done',
subtasks: [],
logs: [],
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-02')
}
];
// Create spec directory
const specDir = path.join(specsDir, '001-test-feature');
mkdirSync(specDir, { recursive: true });
writeFileSync(path.join(specDir, 'spec.md'), '# Test Feature');
const completed = service.getCompletedTasks(projectPath, tasks);
expect(completed).toHaveLength(1);
expect(completed[0].id).toBe('task-1');
expect(completed[0].title).toBe('Test Feature');
});
it('should include tasks with "pr_created" status', async () => {
const { ChangelogService } = await import('../changelog-service');
const service = new ChangelogService();
const tasks: Task[] = [
{
id: 'task-2',
specId: '002-pr-feature',
projectId: 'project-1',
title: 'PR Feature',
description: 'A feature with PR created',
status: 'pr_created',
subtasks: [],
logs: [],
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-02')
}
];
// Create spec directory
const specDir = path.join(specsDir, '002-pr-feature');
mkdirSync(specDir, { recursive: true });
writeFileSync(path.join(specDir, 'spec.md'), '# PR Feature');
const completed = service.getCompletedTasks(projectPath, tasks);
expect(completed).toHaveLength(1);
expect(completed[0].id).toBe('task-2');
expect(completed[0].title).toBe('PR Feature');
});
it('should include tasks with "human_review" status and reviewReason "completed"', async () => {
const { ChangelogService } = await import('../changelog-service');
const service = new ChangelogService();
const tasks: Task[] = [
{
id: 'task-3',
specId: '003-qa-passed',
projectId: 'project-1',
title: 'QA Passed Feature',
description: 'A feature that passed QA',
status: 'human_review',
reviewReason: 'completed',
subtasks: [],
logs: [],
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-02')
}
];
// Create spec directory
const specDir = path.join(specsDir, '003-qa-passed');
mkdirSync(specDir, { recursive: true });
writeFileSync(path.join(specDir, 'spec.md'), '# QA Passed Feature');
const completed = service.getCompletedTasks(projectPath, tasks);
expect(completed).toHaveLength(1);
expect(completed[0].id).toBe('task-3');
expect(completed[0].title).toBe('QA Passed Feature');
});
it('should exclude tasks with "human_review" status and reviewReason "errors"', async () => {
const { ChangelogService } = await import('../changelog-service');
const service = new ChangelogService();
const tasks: Task[] = [
{
id: 'task-4',
specId: '004-failed-feature',
projectId: 'project-1',
title: 'Failed Feature',
description: 'A feature with errors',
status: 'human_review',
reviewReason: 'errors',
subtasks: [],
logs: [],
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-02')
}
];
// Create spec directory (still needed for consistency)
const specDir = path.join(specsDir, '004-failed-feature');
mkdirSync(specDir, { recursive: true });
writeFileSync(path.join(specDir, 'spec.md'), '# Failed Feature');
const completed = service.getCompletedTasks(projectPath, tasks);
expect(completed).toHaveLength(0);
});
it('should exclude tasks with "human_review" status and reviewReason "qa_rejected"', async () => {
const { ChangelogService } = await import('../changelog-service');
const service = new ChangelogService();
const tasks: Task[] = [
{
id: 'task-5',
specId: '005-rejected-feature',
projectId: 'project-1',
title: 'QA Rejected Feature',
description: 'A feature rejected by QA',
status: 'human_review',
reviewReason: 'qa_rejected',
subtasks: [],
logs: [],
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-02')
}
];
const completed = service.getCompletedTasks(projectPath, tasks);
expect(completed).toHaveLength(0);
});
it('should exclude tasks with "human_review" status and reviewReason "plan_review"', async () => {
const { ChangelogService } = await import('../changelog-service');
const service = new ChangelogService();
const tasks: Task[] = [
{
id: 'task-6',
specId: '006-plan-review',
projectId: 'project-1',
title: 'Plan Review Feature',
description: 'A feature in plan review',
status: 'human_review',
reviewReason: 'plan_review',
subtasks: [],
logs: [],
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-02')
}
];
const completed = service.getCompletedTasks(projectPath, tasks);
expect(completed).toHaveLength(0);
});
it('should include all valid completion states in a single call', async () => {
const { ChangelogService } = await import('../changelog-service');
const service = new ChangelogService();
const tasks: Task[] = [
{
id: 'task-done',
specId: '001-done',
projectId: 'project-1',
title: 'Done Task',
description: 'Task with done status',
status: 'done',
subtasks: [],
logs: [],
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-02')
},
{
id: 'task-pr',
specId: '002-pr',
projectId: 'project-1',
title: 'PR Task',
description: 'Task with PR created',
status: 'pr_created',
subtasks: [],
logs: [],
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-03')
},
{
id: 'task-qa',
specId: '003-qa',
projectId: 'project-1',
title: 'QA Passed Task',
description: 'Task that passed QA',
status: 'human_review',
reviewReason: 'completed',
subtasks: [],
logs: [],
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-04')
},
{
id: 'task-in-progress',
specId: '004-wip',
projectId: 'project-1',
title: 'In Progress Task',
description: 'Task still in progress',
status: 'in_progress',
subtasks: [],
logs: [],
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-05')
},
{
id: 'task-errors',
specId: '005-errors',
projectId: 'project-1',
title: 'Error Task',
description: 'Task with errors',
status: 'human_review',
reviewReason: 'errors',
subtasks: [],
logs: [],
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-06')
}
];
// Create spec directories for completed tasks
for (const specId of ['001-done', '002-pr', '003-qa']) {
const specDir = path.join(specsDir, specId);
mkdirSync(specDir, { recursive: true });
writeFileSync(path.join(specDir, 'spec.md'), `# ${specId}`);
}
const completed = service.getCompletedTasks(projectPath, tasks);
// Should include: done, pr_created, and human_review+completed
expect(completed).toHaveLength(3);
const completedIds = completed.map(t => t.id);
expect(completedIds).toContain('task-done');
expect(completedIds).toContain('task-pr');
expect(completedIds).toContain('task-qa');
expect(completedIds).not.toContain('task-in-progress');
expect(completedIds).not.toContain('task-errors');
});
it('should exclude archived tasks even if status is completed', async () => {
const { ChangelogService } = await import('../changelog-service');
const service = new ChangelogService();
const tasks: Task[] = [
{
id: 'task-archived',
specId: '001-archived',
projectId: 'project-1',
title: 'Archived Task',
description: 'An archived task',
status: 'done',
subtasks: [],
logs: [],
metadata: {
archivedAt: '2024-01-05T00:00:00.000Z'
},
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-02')
}
];
const completed = service.getCompletedTasks(projectPath, tasks);
expect(completed).toHaveLength(0);
});
it('should sort completed tasks by updatedAt descending', async () => {
const { ChangelogService } = await import('../changelog-service');
const service = new ChangelogService();
const tasks: Task[] = [
{
id: 'task-1',
specId: '001-first',
projectId: 'project-1',
title: 'First Task',
description: 'Oldest update',
status: 'done',
subtasks: [],
logs: [],
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-01')
},
{
id: 'task-2',
specId: '002-second',
projectId: 'project-1',
title: 'Second Task',
description: 'Newest update',
status: 'pr_created',
subtasks: [],
logs: [],
createdAt: new Date('2024-01-02'),
updatedAt: new Date('2024-01-03')
},
{
id: 'task-3',
specId: '003-third',
projectId: 'project-1',
title: 'Third Task',
description: 'Middle update',
status: 'human_review',
reviewReason: 'completed',
subtasks: [],
logs: [],
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-02')
}
];
// Create spec directories
for (const specId of ['001-first', '002-second', '003-third']) {
const specDir = path.join(specsDir, specId);
mkdirSync(specDir, { recursive: true });
writeFileSync(path.join(specDir, 'spec.md'), `# ${specId}`);
}
const completed = service.getCompletedTasks(projectPath, tasks);
expect(completed).toHaveLength(3);
// Should be sorted by updatedAt descending (newest first)
expect(completed[0].id).toBe('task-2'); // 2024-01-03
expect(completed[1].id).toBe('task-3'); // 2024-01-02
expect(completed[2].id).toBe('task-1'); // 2024-01-01
});
it('should mark tasks as having specs when spec.md exists', async () => {
const { ChangelogService } = await import('../changelog-service');
const service = new ChangelogService();
const tasks: Task[] = [
{
id: 'task-with-spec',
specId: '001-with-spec',
projectId: 'project-1',
title: 'Task With Spec',
description: 'Has spec file',
status: 'done',
subtasks: [],
logs: [],
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-02')
},
{
id: 'task-no-spec',
specId: '002-no-spec',
projectId: 'project-1',
title: 'Task Without Spec',
description: 'No spec file',
status: 'done',
subtasks: [],
logs: [],
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-02')
}
];
// Only create spec directory for first task
const specDir = path.join(specsDir, '001-with-spec');
mkdirSync(specDir, { recursive: true });
writeFileSync(path.join(specDir, 'spec.md'), '# Task With Spec');
const completed = service.getCompletedTasks(projectPath, tasks);
expect(completed).toHaveLength(2);
const withSpec = completed.find(t => t.id === 'task-with-spec');
const noSpec = completed.find(t => t.id === 'task-no-spec');
expect(withSpec?.hasSpecs).toBe(true);
expect(noSpec?.hasSpecs).toBe(false);
});
});
});
@@ -0,0 +1,427 @@
/**
* Integration tests for ChangelogGenerator subprocess timeout mechanism
* Tests that long-running processes are killed after 5 minutes with error event
*/
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { EventEmitter } from 'events';
import type { ChangelogGenerationRequest, TaskSpecContent } from '../../../shared/types';
// Mock child_process module
const mockChildProcess = new EventEmitter() as any;
mockChildProcess.pid = 12345;
mockChildProcess.kill = vi.fn();
mockChildProcess.stdout = new EventEmitter();
mockChildProcess.stderr = new EventEmitter();
const mockSpawn = vi.fn(() => mockChildProcess);
vi.mock('child_process', async (importOriginal) => {
const actual = await importOriginal<typeof import('child_process')>();
return {
...actual,
spawn: mockSpawn
};
});
// Mock electron modules
vi.mock('electron', () => ({
app: {
getPath: vi.fn(() => '/tmp/test-userdata'),
getAppPath: vi.fn(() => '/tmp'),
getVersion: vi.fn(() => '0.1.0'),
isPackaged: false
}
}));
vi.mock('../../python-detector', () => ({
parsePythonCommand: vi.fn((cmd: string) => [cmd, []])
}));
vi.mock('../../env-utils', () => ({
getAugmentedEnv: vi.fn(() => ({ PATH: '/usr/bin' }))
}));
vi.mock('../../platform', () => ({
isWindows: vi.fn(() => false)
}));
vi.mock('../../rate-limit-detector', () => ({
detectRateLimit: vi.fn(() => ({ isRateLimited: false })),
createSDKRateLimitInfo: vi.fn(),
getBestAvailableProfileEnv: vi.fn(() => ({
env: {},
wasSwapped: false,
profileName: 'default'
}))
}));
describe('ChangelogGenerator - Subprocess Timeout', () => {
let generator: any;
beforeEach(async () => {
vi.clearAllMocks();
vi.useFakeTimers();
// Reset mock child process
mockChildProcess.removeAllListeners();
mockChildProcess.stdout.removeAllListeners();
mockChildProcess.stderr.removeAllListeners();
mockChildProcess.killed = false;
mockChildProcess.kill.mockImplementation(() => {
mockChildProcess.killed = true;
return true;
});
// Import generator after mocks are set up
const { ChangelogGenerator } = await import('../generator');
generator = new ChangelogGenerator(
'/usr/bin/python3',
'/usr/bin/claude',
'/tmp/auto-build',
{},
false
);
});
afterEach(() => {
vi.useRealTimers();
vi.clearAllMocks();
});
it('should kill subprocess after 5 minutes and emit timeout error', async () => {
const projectId = 'test-project';
const projectPath = '/tmp/test-project';
const request: ChangelogGenerationRequest = {
projectId,
sourceMode: 'tasks',
taskIds: ['task-1'],
version: '1.0.0',
date: new Date().toISOString(),
format: 'keep-a-changelog',
audience: 'technical'
};
const specs: TaskSpecContent[] = [
{
taskId: 'task-1',
specId: '001-test-task',
spec: '# Test Task\nA test task spec'
}
];
// Track emitted events
const progressEvents: any[] = [];
const errorEvents: string[] = [];
generator.on('generation-progress', (_projectId: string, progress: any) => {
progressEvents.push(progress);
});
generator.on('generation-error', (_projectId: string, error: string) => {
errorEvents.push(error);
});
// Start generation (returns immediately, spawns async process)
const generatePromise = generator.generate(projectId, projectPath, request, specs);
// Wait for spawn to be called
await vi.waitFor(() => {
expect(mockSpawn).toHaveBeenCalled();
});
// Verify process was spawned
expect(mockSpawn).toHaveBeenCalledWith(
'/usr/bin/python3',
expect.arrayContaining(['-c', expect.any(String)]),
expect.objectContaining({
cwd: '/tmp/auto-build',
env: expect.any(Object)
})
);
// Verify initial progress event was emitted
expect(progressEvents.length).toBeGreaterThan(0);
expect(progressEvents[0].stage).toBe('loading_specs');
// Simulate process running but not completing (no exit event)
// Just send some stdout data to show it's working
mockChildProcess.stdout.emit('data', Buffer.from('Processing...'));
// Advance time by 4 minutes - should NOT timeout yet
vi.advanceTimersByTime(4 * 60 * 1000);
// Process should still be alive
expect(mockChildProcess.kill).not.toHaveBeenCalled();
expect(errorEvents).toHaveLength(0);
// Advance time by another 1 minute and 1 second - should timeout now
vi.advanceTimersByTime(61 * 1000);
// Process should be killed
expect(mockChildProcess.kill).toHaveBeenCalledWith('SIGTERM');
// Timeout error should be emitted
expect(errorEvents).toHaveLength(1);
expect(errorEvents[0]).toBe('Changelog generation timed out after 5 minutes');
// Check that error progress event was emitted
const errorProgress = progressEvents.find(p => p.stage === 'error');
expect(errorProgress).toBeDefined();
expect(errorProgress?.error).toBe('Changelog generation timed out after 5 minutes');
await generatePromise;
});
it('should clear timeout on normal subprocess exit', async () => {
const projectId = 'test-project-2';
const projectPath = '/tmp/test-project-2';
const request: ChangelogGenerationRequest = {
projectId,
sourceMode: 'tasks',
taskIds: ['task-2'],
version: '1.0.0',
date: new Date().toISOString(),
format: 'keep-a-changelog',
audience: 'technical'
};
const specs: TaskSpecContent[] = [
{
taskId: 'task-2',
specId: '002-test-task',
spec: '# Test Task 2\nAnother test task'
}
];
// Track events
const completeEvents: any[] = [];
const errorEvents: string[] = [];
generator.on('generation-complete', (_projectId: string, result: any) => {
completeEvents.push(result);
});
generator.on('generation-error', (_projectId: string, error: string) => {
errorEvents.push(error);
});
// Start generation
const generatePromise = generator.generate(projectId, projectPath, request, specs);
// Wait for spawn
await vi.waitFor(() => {
expect(mockSpawn).toHaveBeenCalled();
});
// Simulate successful completion before timeout
const changelogOutput = `
# Changelog
## [1.0.0] - ${new Date().toISOString().split('T')[0]}
### Added
- Test feature from task-2
`;
mockChildProcess.stdout.emit('data', Buffer.from(changelogOutput));
mockChildProcess.emit('exit', 0);
// Process exit handler should have cleared the timeout
// Advance time past timeout to verify it doesn't fire
vi.advanceTimersByTime(6 * 60 * 1000); // 6 minutes
// Should NOT have killed the process (already exited)
expect(mockChildProcess.kill).not.toHaveBeenCalled();
// Should NOT have timeout error
expect(errorEvents).toHaveLength(0);
// Should have successful completion
expect(completeEvents).toHaveLength(1);
expect(completeEvents[0].success).toBe(true);
expect(completeEvents[0].changelog).toContain('Test feature from task-2');
await generatePromise;
});
it('should clear timeout on subprocess error', async () => {
const projectId = 'test-project-3';
const projectPath = '/tmp/test-project-3';
const request: ChangelogGenerationRequest = {
projectId,
sourceMode: 'tasks',
taskIds: ['task-3'],
version: '1.0.0',
date: new Date().toISOString(),
format: 'keep-a-changelog',
audience: 'technical'
};
const specs: TaskSpecContent[] = [
{
taskId: 'task-3',
specId: '003-test-task',
spec: '# Test Task 3\nTask that will error'
}
];
// Track events
const errorEvents: string[] = [];
generator.on('generation-error', (_projectId: string, error: string) => {
errorEvents.push(error);
});
// Start generation
const generatePromise = generator.generate(projectId, projectPath, request, specs);
// Wait for spawn
await vi.waitFor(() => {
expect(mockSpawn).toHaveBeenCalled();
});
// Simulate subprocess error (e.g., Python not found)
const processError = new Error('spawn ENOENT');
mockChildProcess.emit('error', processError);
// Error handler should have cleared the timeout
// Advance time past timeout to verify it doesn't fire
vi.advanceTimersByTime(6 * 60 * 1000); // 6 minutes
// Should NOT have killed the process (error already occurred)
expect(mockChildProcess.kill).not.toHaveBeenCalled();
// Should have process error, NOT timeout error
expect(errorEvents).toHaveLength(1);
expect(errorEvents[0]).toBe('spawn ENOENT');
expect(errorEvents[0]).not.toContain('timed out');
await generatePromise;
});
it('should handle multiple concurrent generations with independent timeouts', async () => {
const projectId1 = 'project-1';
const projectId2 = 'project-2';
const projectPath = '/tmp/test-project';
const request: ChangelogGenerationRequest = {
projectId: projectId1,
sourceMode: 'tasks',
taskIds: ['task-1'],
version: '1.0.0',
date: new Date().toISOString(),
format: 'keep-a-changelog',
audience: 'technical'
};
const specs: TaskSpecContent[] = [
{
taskId: 'task-1',
specId: '001-test',
spec: '# Test'
}
];
const errorEvents: Array<{ projectId: string; error: string }> = [];
generator.on('generation-error', (projectId: string, error: string) => {
errorEvents.push({ projectId, error });
});
// Start first generation
const gen1Promise = generator.generate(projectId1, projectPath, request, specs);
await vi.waitFor(() => expect(mockSpawn).toHaveBeenCalledTimes(1));
// Advance time by 2 minutes
vi.advanceTimersByTime(2 * 60 * 1000);
// Clear mock and set up for second process
mockSpawn.mockClear();
const mockChildProcess2 = new EventEmitter() as any;
mockChildProcess2.pid = 12346;
mockChildProcess2.kill = vi.fn(() => true);
mockChildProcess2.stdout = new EventEmitter();
mockChildProcess2.stderr = new EventEmitter();
mockSpawn.mockReturnValueOnce(mockChildProcess2);
// Start second generation with different projectId (starts 2 minutes after first)
const request2 = { ...request, projectId: projectId2 };
const gen2Promise = generator.generate(projectId2, projectPath, request2, specs);
await vi.waitFor(() => expect(mockSpawn).toHaveBeenCalledTimes(1));
// Advance time by 3 minutes and 1 second - first process should timeout (2+3 = 5 total)
vi.advanceTimersByTime(3 * 60 * 1000 + 1000);
// First process should timeout
expect(mockChildProcess.kill).toHaveBeenCalledWith('SIGTERM');
expect(errorEvents.some(e => e.projectId === projectId1 && e.error.includes('timed out'))).toBe(true);
// Second process should NOT timeout yet (only 3 minutes have passed for it)
expect(mockChildProcess2.kill).not.toHaveBeenCalled();
// Advance another 2 minutes - now second process should timeout (3+2 = 5 total)
vi.advanceTimersByTime(2 * 60 * 1000);
// Now second process should also timeout
expect(mockChildProcess2.kill).toHaveBeenCalledWith('SIGTERM');
expect(errorEvents.some(e => e.projectId === projectId2 && e.error.includes('timed out'))).toBe(true);
await gen1Promise;
await gen2Promise;
});
it('should not fire timeout if process already killed', async () => {
const projectId = 'test-project-4';
const projectPath = '/tmp/test-project-4';
const request: ChangelogGenerationRequest = {
projectId,
sourceMode: 'tasks',
taskIds: ['task-4'],
version: '1.0.0',
date: new Date().toISOString(),
format: 'keep-a-changelog',
audience: 'technical'
};
const specs: TaskSpecContent[] = [
{
taskId: 'task-4',
specId: '004-test',
spec: '# Test'
}
];
const errorEvents: string[] = [];
generator.on('generation-error', (_projectId: string, error: string) => {
errorEvents.push(error);
});
// Start generation
const generatePromise = generator.generate(projectId, projectPath, request, specs);
await vi.waitFor(() => expect(mockSpawn).toHaveBeenCalled());
// Manually cancel the generation (simulates user clicking cancel)
generator.cancel(projectId);
// Verify process was killed and timeout cleared
expect(mockChildProcess.kill).toHaveBeenCalledWith('SIGTERM');
mockChildProcess.kill.mockClear();
// Advance time past timeout
vi.advanceTimersByTime(6 * 60 * 1000);
// Timeout should NOT fire again (already cleared)
expect(mockChildProcess.kill).not.toHaveBeenCalled();
// Should have no timeout error (cancel doesn't emit error)
expect(errorEvents.some(e => e.includes('timed out'))).toBe(false);
await generatePromise;
});
});
@@ -21,6 +21,7 @@ import type {
GitBranchInfo,
GitTagInfo
} from '../../shared/types';
import { isCompletedTask } from '../../shared/utils/task-status';
import { ChangelogGenerator } from './generator';
import { VersionSuggester } from './version-suggester';
import { parseExistingChangelog } from './parser';
@@ -67,8 +68,6 @@ export class ChangelogService extends EventEmitter {
// Check process.env first
if (
process.env.DEBUG === 'true' ||
process.env.DEBUG === '1' ||
process.env.DEBUG === 'true' ||
process.env.DEBUG === '1'
) {
@@ -263,7 +262,7 @@ export class ChangelogService extends EventEmitter {
const specsDir = path.join(projectPath, specsBaseDir || AUTO_BUILD_PATHS.SPECS_DIR);
return tasks
.filter(task => task.status === 'done' && !task.metadata?.archivedAt)
.filter(task => isCompletedTask(task.status, task.reviewReason) && !task.metadata?.archivedAt)
.map(task => {
const specDir = path.join(specsDir, task.specId);
const hasSpecs = existsSync(specDir) && existsSync(path.join(specDir, AUTO_BUILD_PATHS.SPEC_FILE));
+51 -2
View File
@@ -22,6 +22,7 @@ import { isWindows } from '../platform';
*/
export class ChangelogGenerator extends EventEmitter {
private generationProcesses: Map<string, ReturnType<typeof spawn>> = new Map();
private generationTimeouts: Map<string, NodeJS.Timeout> = new Map();
private debugEnabled: boolean;
constructor(
@@ -152,6 +153,25 @@ export class ChangelogGenerator extends EventEmitter {
this.generationProcesses.set(projectId, childProcess);
this.debug('Process spawned with PID:', childProcess.pid);
// Set 5-minute timeout
const TIMEOUT_MS = 5 * 60 * 1000; // 5 minutes
const timeoutId = setTimeout(() => {
this.debug('Process timed out after 5 minutes');
this.generationTimeouts.delete(projectId);
// Kill the process
const proc = this.generationProcesses.get(projectId);
if (proc) {
proc.kill('SIGTERM');
this.generationProcesses.delete(projectId);
}
// Emit timeout error
this.emitError(projectId, 'Changelog generation timed out after 5 minutes');
}, TIMEOUT_MS);
this.generationTimeouts.set(projectId, timeoutId);
let output = '';
let errorOutput = '';
@@ -182,7 +202,18 @@ export class ChangelogGenerator extends EventEmitter {
errorLength: errorOutput.length
});
this.generationProcesses.delete(projectId);
// Clear timeout
const existingTimeout = this.generationTimeouts.get(projectId);
if (existingTimeout) {
clearTimeout(existingTimeout);
this.generationTimeouts.delete(projectId);
}
// Guard: if process was already removed (e.g. by timeout or cancel), skip
if (!this.generationProcesses.delete(projectId)) {
this.debug('Process already cleaned up (timeout or cancel), skipping exit handler');
return;
}
if (code === 0 && output.trim()) {
this.emitProgress(projectId, {
@@ -236,7 +267,18 @@ export class ChangelogGenerator extends EventEmitter {
childProcess.on('error', (err: Error) => {
this.debug('Process error', { error: err.message });
this.generationProcesses.delete(projectId);
// Clear timeout
const timeoutId = this.generationTimeouts.get(projectId);
if (timeoutId) {
clearTimeout(timeoutId);
this.generationTimeouts.delete(projectId);
}
if (!this.generationProcesses.delete(projectId)) {
this.debug('Process already cleaned up, skipping error handler');
return;
}
this.emitError(projectId, err.message);
});
}
@@ -289,6 +331,13 @@ export class ChangelogGenerator extends EventEmitter {
* Cancel ongoing generation
*/
cancel(projectId: string): boolean {
// Clear timeout
const timeoutId = this.generationTimeouts.get(projectId);
if (timeoutId) {
clearTimeout(timeoutId);
this.generationTimeouts.delete(projectId);
}
const process = this.generationProcesses.get(projectId);
if (process) {
process.kill('SIGTERM');
@@ -21,43 +21,79 @@ import type {
import { projectStore } from '../project-store';
import { changelogService } from '../changelog-service';
// Store cleanup function to remove listeners on subsequent calls
let cleanupListeners: (() => void) | null = null;
/**
* Register all changelog-related IPC handlers
*/
export function registerChangelogHandlers(
getMainWindow: () => BrowserWindow | null
): void {
// Remove previous listeners if they exist
if (cleanupListeners) {
cleanupListeners();
}
// ============================================
// Changelog Event Handlers
// ============================================
changelogService.on('generation-progress', (projectId: string, progress: import('../../shared/types').ChangelogGenerationProgress) => {
const progressHandler = (projectId: string, progress: import('../../shared/types').ChangelogGenerationProgress) => {
const mainWindow = getMainWindow();
if (mainWindow) {
mainWindow.webContents.send(IPC_CHANNELS.CHANGELOG_GENERATION_PROGRESS, projectId, progress);
}
});
};
changelogService.on('generation-complete', (projectId: string, result: import('../../shared/types').ChangelogGenerationResult) => {
const completeHandler = (projectId: string, result: import('../../shared/types').ChangelogGenerationResult) => {
const mainWindow = getMainWindow();
if (mainWindow) {
mainWindow.webContents.send(IPC_CHANNELS.CHANGELOG_GENERATION_COMPLETE, projectId, result);
}
});
};
changelogService.on('generation-error', (projectId: string, error: string) => {
const errorHandler = (projectId: string, error: string) => {
const mainWindow = getMainWindow();
if (mainWindow) {
mainWindow.webContents.send(IPC_CHANNELS.CHANGELOG_GENERATION_ERROR, projectId, error);
}
});
};
changelogService.on('rate-limit', (_projectId: string, rateLimitInfo: import('../../shared/types').SDKRateLimitInfo) => {
const rateLimitHandler = (_projectId: string, rateLimitInfo: import('../../shared/types').SDKRateLimitInfo) => {
const mainWindow = getMainWindow();
if (mainWindow) {
mainWindow.webContents.send(IPC_CHANNELS.CLAUDE_SDK_RATE_LIMIT, rateLimitInfo);
}
});
};
// Register event listeners
changelogService.on('generation-progress', progressHandler);
changelogService.on('generation-complete', completeHandler);
changelogService.on('generation-error', errorHandler);
changelogService.on('rate-limit', rateLimitHandler);
// Store cleanup function to remove all listeners
cleanupListeners = () => {
changelogService.off('generation-progress', progressHandler);
changelogService.off('generation-complete', completeHandler);
changelogService.off('generation-error', errorHandler);
changelogService.off('rate-limit', rateLimitHandler);
// Also remove IPC handlers
ipcMain.removeHandler(IPC_CHANNELS.CHANGELOG_GET_DONE_TASKS);
ipcMain.removeHandler(IPC_CHANNELS.CHANGELOG_LOAD_TASK_SPECS);
ipcMain.removeHandler(IPC_CHANNELS.CHANGELOG_GENERATE);
ipcMain.removeHandler(IPC_CHANNELS.CHANGELOG_SAVE);
ipcMain.removeHandler(IPC_CHANNELS.CHANGELOG_READ_EXISTING);
ipcMain.removeHandler(IPC_CHANNELS.CHANGELOG_SUGGEST_VERSION);
ipcMain.removeHandler(IPC_CHANNELS.CHANGELOG_SUGGEST_VERSION_FROM_COMMITS);
ipcMain.removeHandler(IPC_CHANNELS.CHANGELOG_GET_BRANCHES);
ipcMain.removeHandler(IPC_CHANNELS.CHANGELOG_GET_TAGS);
ipcMain.removeHandler(IPC_CHANNELS.CHANGELOG_GET_COMMITS_PREVIEW);
ipcMain.removeHandler(IPC_CHANNELS.CHANGELOG_SAVE_IMAGE);
ipcMain.removeHandler(IPC_CHANNELS.CHANGELOG_READ_LOCAL_IMAGE);
};
// ============================================
// Changelog Operations
@@ -101,32 +137,39 @@ export function registerChangelogHandlers(
}
);
ipcMain.on(
ipcMain.handle(
IPC_CHANNELS.CHANGELOG_GENERATE,
async (_, request: ChangelogGenerationRequest) => {
const mainWindow = getMainWindow();
if (!mainWindow) return;
async (_, request: ChangelogGenerationRequest): Promise<IPCResult<void>> => {
const project = projectStore.getProject(request.projectId);
if (!project) {
mainWindow.webContents.send(
IPC_CHANNELS.CHANGELOG_GENERATION_ERROR,
request.projectId,
'Project not found'
);
return;
return { success: false, error: 'Project not found' };
}
// Load specs for selected tasks (only in tasks mode)
let specs: TaskSpecContent[] = [];
if (request.sourceMode === 'tasks' && request.taskIds && request.taskIds.length > 0) {
const tasks = projectStore.getTasks(request.projectId);
const specsBaseDir = getSpecsDir(project.autoBuildPath);
specs = await changelogService.loadTaskSpecs(project.path, request.taskIds, tasks, specsBaseDir);
}
// Return immediately to allow renderer to register event listeners
// Start the actual generation asynchronously
setImmediate(async () => {
try {
// Load specs for selected tasks (only in tasks mode)
let specs: TaskSpecContent[] = [];
if (request.sourceMode === 'tasks' && request.taskIds && request.taskIds.length > 0) {
const tasks = projectStore.getTasks(request.projectId);
const specsBaseDir = getSpecsDir(project.autoBuildPath);
specs = await changelogService.loadTaskSpecs(project.path, request.taskIds, tasks, specsBaseDir);
}
// Start generation
changelogService.generateChangelog(request.projectId, project.path, request, specs);
// Start generation (progress/completion/errors will be sent via event handlers)
changelogService.generateChangelog(request.projectId, project.path, request, specs);
} catch (error) {
// Send error via event instead of return value since we already returned
const mainWindow = getMainWindow();
if (mainWindow) {
const errorMessage = error instanceof Error ? error.message : 'Failed to start changelog generation';
mainWindow.webContents.send(IPC_CHANNELS.CHANGELOG_GENERATION_ERROR, request.projectId, errorMessage);
}
}
});
return { success: true };
}
);
@@ -356,12 +399,13 @@ export function registerChangelogHandlers(
const base64Data = imageData.includes(',') ? imageData.split(',')[1] : imageData;
const buffer = Buffer.from(base64Data, 'base64');
// Save image file
const imagePath = path.join(assetsDir, filename);
// Sanitize filename to prevent path traversal
const safeFilename = path.basename(filename);
const imagePath = path.join(assetsDir, safeFilename);
writeFileSync(imagePath, buffer);
// Return relative path for use in markdown
const relativePath = `.github/assets/${filename}`;
const relativePath = `.github/assets/${safeFilename}`;
// For GitHub releases, we'll use the relative path which will work when the release is created
const url = relativePath;
@@ -379,8 +423,11 @@ export function registerChangelogHandlers(
IPC_CHANNELS.CHANGELOG_READ_LOCAL_IMAGE,
async (_, projectPath: string, relativePath: string): Promise<IPCResult<string>> => {
try {
// Construct full path from project path and relative path
const fullPath = path.join(projectPath, relativePath);
// Construct full path and validate it stays within project directory
const fullPath = path.resolve(projectPath, relativePath);
if (!fullPath.startsWith(path.resolve(projectPath) + path.sep) && fullPath !== path.resolve(projectPath)) {
return { success: false, error: 'Invalid path' };
}
// Verify the file exists
if (!existsSync(fullPath)) {
@@ -16,7 +16,7 @@ import type {
Task,
IPCResult
} from '../../../shared/types';
import { createIpcListener, invokeIpc, sendIpc, IpcListenerCleanup } from './ipc-utils';
import { createIpcListener, invokeIpc, IpcListenerCleanup } from './ipc-utils';
/**
* Changelog API operations
@@ -25,7 +25,7 @@ export interface ChangelogAPI {
// Operations
getChangelogDoneTasks: (projectId: string, tasks?: Task[]) => Promise<IPCResult<ChangelogTask[]>>;
loadTaskSpecs: (projectId: string, taskIds: string[]) => Promise<IPCResult<TaskSpecContent[]>>;
generateChangelog: (request: ChangelogGenerationRequest) => void;
generateChangelog: (request: ChangelogGenerationRequest) => Promise<IPCResult<void>>;
saveChangelog: (request: ChangelogSaveRequest) => Promise<IPCResult<ChangelogSaveResult>>;
readExistingChangelog: (projectId: string) => Promise<IPCResult<ExistingChangelog>>;
suggestChangelogVersion: (
@@ -76,8 +76,8 @@ export const createChangelogAPI = (): ChangelogAPI => ({
loadTaskSpecs: (projectId: string, taskIds: string[]): Promise<IPCResult<TaskSpecContent[]>> =>
invokeIpc(IPC_CHANNELS.CHANGELOG_LOAD_TASK_SPECS, projectId, taskIds),
generateChangelog: (request: ChangelogGenerationRequest): void =>
sendIpc(IPC_CHANNELS.CHANGELOG_GENERATE, request),
generateChangelog: (request: ChangelogGenerationRequest): Promise<IPCResult<void>> =>
invokeIpc(IPC_CHANNELS.CHANGELOG_GENERATE, request),
saveChangelog: (request: ChangelogSaveRequest): Promise<IPCResult<ChangelogSaveResult>> =>
invokeIpc(IPC_CHANNELS.CHANGELOG_SAVE, request),
@@ -170,9 +170,9 @@ export function useChangelog() {
};
}, [selectedProjectId, setError, setGenerationProgress, setIsGenerating, updateGeneratedChangelog]);
const handleGenerate = () => {
const handleGenerate = async () => {
if (selectedProjectId) {
generateChangelog(selectedProjectId);
await generateChangelog(selectedProjectId);
}
};
@@ -3,6 +3,7 @@
*/
import type { Task } from '../../../shared/types';
import { isCompletedTask } from '../../../shared/utils/task-status';
import { mockTasks } from './mock-data';
export const changelogMock = {
@@ -10,7 +11,7 @@ export const changelogMock = {
getChangelogDoneTasks: async (_projectId: string, tasks?: Task[]) => ({
success: true,
data: (tasks || mockTasks)
.filter(t => t.status === 'done')
.filter((t): t is Task => isCompletedTask(t.status, (t as Task).reviewReason))
.map(t => ({
id: t.id,
specId: t.specId,
@@ -26,8 +27,9 @@ export const changelogMock = {
data: []
}),
generateChangelog: () => {
generateChangelog: async () => {
console.warn('[Browser Mock] generateChangelog called');
return { success: true };
},
saveChangelog: async () => ({
@@ -118,5 +118,20 @@ export const mockTasks = [
logs: ['[INFO] Task completed successfully'],
createdAt: new Date(Date.now() - 172800000),
updatedAt: new Date(Date.now() - 86400000)
},
{
id: 'task-5',
projectId: 'mock-project-1',
specId: '005-add-search',
title: 'Add search functionality',
description: 'Implement full-text search across all entities',
status: 'pr_created' as const,
subtasks: [
{ id: 'subtask-1', title: 'Setup search index', description: 'Configure search indexing', status: 'completed' as const, files: ['src/lib/search.ts'] },
{ id: 'subtask-2', title: 'Add search UI', description: 'Create search component', status: 'completed' as const, files: ['src/components/Search.tsx'] }
],
logs: ['[INFO] Task completed, PR created'],
createdAt: new Date(Date.now() - 259200000),
updatedAt: new Date(Date.now() - 43200000)
}
];
@@ -12,7 +12,8 @@ import type {
GitTagInfo,
GitCommit,
GitHistoryOptions,
BranchDiffOptions
BranchDiffOptions,
IPCResult
} from '../../shared/types';
import { useTaskStore } from './task-store';
import { useSettingsStore } from './settings-store';
@@ -424,7 +425,18 @@ export async function loadCommitsPreview(projectId: string): Promise<void> {
}
}
export function generateChangelog(projectId: string): void {
function handleGenerationError(store: ReturnType<typeof useChangelogStore.getState>, errorMessage: string): void {
store.setIsGenerating(false);
store.setError(errorMessage);
store.setGenerationProgress({
stage: 'error',
progress: 0,
message: errorMessage,
error: errorMessage
});
}
export async function generateChangelog(projectId: string): Promise<void> {
const store = useChangelogStore.getState();
// Validate based on source mode
@@ -476,34 +488,48 @@ export function generateChangelog(projectId: string): void {
customInstructions: store.customInstructions || undefined
};
if (store.sourceMode === 'tasks') {
window.electronAPI.generateChangelog({
...baseRequest,
taskIds: store.selectedTaskIds
});
} else if (store.sourceMode === 'git-history') {
window.electronAPI.generateChangelog({
...baseRequest,
gitHistory: {
type: store.gitHistoryType,
count: store.gitHistoryCount,
sinceDate: store.gitHistorySinceDate || undefined,
// For since-version, use gitHistorySinceVersion as fromTag
fromTag: store.gitHistoryType === 'since-version'
? (store.gitHistorySinceVersion || undefined)
: (store.gitHistoryFromTag || undefined),
toTag: store.gitHistoryToTag || undefined,
includeMergeCommits: store.includeMergeCommits
}
});
} else if (store.sourceMode === 'branch-diff') {
window.electronAPI.generateChangelog({
...baseRequest,
branchDiff: {
baseBranch: store.baseBranch,
compareBranch: store.compareBranch
}
});
try {
let result: IPCResult<void>;
if (store.sourceMode === 'tasks') {
result = await window.electronAPI.generateChangelog({
...baseRequest,
taskIds: store.selectedTaskIds
});
} else if (store.sourceMode === 'git-history') {
result = await window.electronAPI.generateChangelog({
...baseRequest,
gitHistory: {
type: store.gitHistoryType,
count: store.gitHistoryCount,
sinceDate: store.gitHistorySinceDate || undefined,
// For since-version, use gitHistorySinceVersion as fromTag
fromTag: store.gitHistoryType === 'since-version'
? (store.gitHistorySinceVersion || undefined)
: (store.gitHistoryFromTag || undefined),
toTag: store.gitHistoryToTag || undefined,
includeMergeCommits: store.includeMergeCommits
}
});
} else if (store.sourceMode === 'branch-diff') {
result = await window.electronAPI.generateChangelog({
...baseRequest,
branchDiff: {
baseBranch: store.baseBranch,
compareBranch: store.compareBranch
}
});
} else {
// This should never happen due to validation, but handle it for TypeScript
throw new Error(`Invalid source mode: ${store.sourceMode}`);
}
// Check if generation started successfully
if (!result.success) {
handleGenerationError(store, result.error || 'Failed to start changelog generation');
}
} catch (error) {
const errorMessage = error instanceof Error ? error.message : 'Failed to start changelog generation';
handleGenerationError(store, errorMessage);
}
}
+1 -1
View File
@@ -729,7 +729,7 @@ export interface ElectronAPI {
// Changelog operations
getChangelogDoneTasks: (projectId: string, tasks?: Task[]) => Promise<IPCResult<ChangelogTask[]>>;
loadTaskSpecs: (projectId: string, taskIds: string[]) => Promise<IPCResult<TaskSpecContent[]>>;
generateChangelog: (request: ChangelogGenerationRequest) => void; // Async with progress events
generateChangelog: (request: ChangelogGenerationRequest) => Promise<IPCResult<void>>; // Async with progress events
saveChangelog: (request: ChangelogSaveRequest) => Promise<IPCResult<ChangelogSaveResult>>;
readExistingChangelog: (projectId: string) => Promise<IPCResult<ExistingChangelog>>;
suggestChangelogVersion: (
@@ -0,0 +1,199 @@
import { describe, expect, it } from 'vitest';
import { isCompletedTask } from '../task-status';
import type { TaskStatus, ReviewReason } from '../../types';
describe('isCompletedTask', () => {
describe('completed statuses', () => {
it('should return true for "done" status', () => {
expect(isCompletedTask('done')).toBe(true);
});
it('should return true for "pr_created" status', () => {
expect(isCompletedTask('pr_created')).toBe(true);
});
it('should return true for "human_review" with reviewReason "completed"', () => {
// Tasks that passed QA and are awaiting final merge approval are considered completed
expect(isCompletedTask('human_review', 'completed')).toBe(true);
});
});
describe('non-completed statuses', () => {
it('should return false for "backlog" status', () => {
expect(isCompletedTask('backlog')).toBe(false);
});
it('should return false for "queue" status', () => {
expect(isCompletedTask('queue')).toBe(false);
});
it('should return false for "in_progress" status', () => {
expect(isCompletedTask('in_progress')).toBe(false);
});
it('should return false for "ai_review" status', () => {
expect(isCompletedTask('ai_review')).toBe(false);
});
it('should return false for "human_review" status', () => {
expect(isCompletedTask('human_review')).toBe(false);
});
it('should return false for "error" status', () => {
expect(isCompletedTask('error')).toBe(false);
});
});
describe('human_review edge cases', () => {
it('should return false for human_review without reviewReason', () => {
// human_review without a specific reviewReason is not completed
expect(isCompletedTask('human_review')).toBe(false);
});
it('should return true for human_review with reviewReason "completed"', () => {
// human_review with ReviewReason 'completed' means QA passed and ready for merge
expect(isCompletedTask('human_review', 'completed')).toBe(true);
});
it('should return false for human_review with reviewReason "errors"', () => {
// human_review with ReviewReason 'errors' is not completed
expect(isCompletedTask('human_review', 'errors')).toBe(false);
});
it('should return false for human_review with reviewReason "qa_rejected"', () => {
// human_review with ReviewReason 'qa_rejected' is not completed
expect(isCompletedTask('human_review', 'qa_rejected')).toBe(false);
});
it('should return false for human_review with reviewReason "plan_review"', () => {
// human_review with ReviewReason 'plan_review' is not completed
expect(isCompletedTask('human_review', 'plan_review')).toBe(false);
});
it('should return false for human_review with reviewReason "stopped"', () => {
// human_review with ReviewReason 'stopped' is not completed
expect(isCompletedTask('human_review', 'stopped')).toBe(false);
});
});
describe('archived task considerations', () => {
it('should return true for archived tasks with "done" status', () => {
// Archived tasks with 'done' status are still considered completed
// (archivedAt is metadata, not status)
expect(isCompletedTask('done')).toBe(true);
});
it('should return true for archived tasks with "pr_created" status', () => {
// Archived tasks with 'pr_created' status are still considered completed
expect(isCompletedTask('pr_created')).toBe(true);
});
it('should return false for archived tasks with other statuses', () => {
// Archived tasks that weren't completed before archiving
expect(isCompletedTask('backlog')).toBe(false);
expect(isCompletedTask('error')).toBe(false);
expect(isCompletedTask('human_review')).toBe(false);
});
});
describe('type safety', () => {
it('should work with explicit TaskStatus type annotation', () => {
const status: TaskStatus = 'done';
expect(isCompletedTask(status)).toBe(true);
});
it('should correctly handle all valid TaskStatus values', () => {
const allStatuses: TaskStatus[] = [
'backlog',
'queue',
'in_progress',
'ai_review',
'human_review',
'done',
'pr_created',
'error',
];
const completedStatuses = allStatuses.filter((status) => isCompletedTask(status));
expect(completedStatuses).toEqual(['done', 'pr_created']);
});
});
describe('real-world scenarios', () => {
it('should identify tasks ready for changelog inclusion', () => {
// Tasks in 'done', 'pr_created', or 'human_review' with 'completed' reason are included in changelogs
expect(isCompletedTask('done')).toBe(true);
expect(isCompletedTask('pr_created')).toBe(true);
expect(isCompletedTask('human_review', 'completed')).toBe(true);
});
it('should exclude tasks still in progress from completed count', () => {
// Tasks not yet completed should not be counted
expect(isCompletedTask('in_progress')).toBe(false);
expect(isCompletedTask('ai_review')).toBe(false);
});
it('should exclude tasks waiting for human review (without completion)', () => {
// Tasks in human_review with errors or other non-completed reasons are not completed
expect(isCompletedTask('human_review')).toBe(false);
expect(isCompletedTask('human_review', 'errors')).toBe(false);
expect(isCompletedTask('human_review', 'qa_rejected')).toBe(false);
expect(isCompletedTask('human_review', 'plan_review')).toBe(false);
});
it('should exclude tasks in error state', () => {
// Tasks that encountered errors are not completed
expect(isCompletedTask('error')).toBe(false);
});
it('should exclude tasks in backlog or queue', () => {
// Tasks not yet started are not completed
expect(isCompletedTask('backlog')).toBe(false);
expect(isCompletedTask('queue')).toBe(false);
});
});
describe('boundary conditions', () => {
it('should handle status in conditional expressions', () => {
const statuses: TaskStatus[] = ['done', 'in_progress', 'pr_created'];
const completed = statuses.filter((s) => isCompletedTask(s));
expect(completed).toHaveLength(2);
expect(completed).toContain('done');
expect(completed).toContain('pr_created');
expect(completed).not.toContain('in_progress');
});
it('should work in array methods with task objects', () => {
const tasks = [
{ id: '1', status: 'done' as TaskStatus },
{ id: '2', status: 'in_progress' as TaskStatus },
{ id: '3', status: 'pr_created' as TaskStatus },
{ id: '4', status: 'error' as TaskStatus },
{ id: '5', status: 'human_review' as TaskStatus, reviewReason: 'completed' as ReviewReason },
{ id: '6', status: 'human_review' as TaskStatus, reviewReason: 'errors' as ReviewReason },
];
const completedTasks = tasks.filter((task) => isCompletedTask(task.status, task.reviewReason));
expect(completedTasks).toHaveLength(3);
expect(completedTasks.map((t) => t.id)).toEqual(['1', '3', '5']);
});
it('should be usable in reduce operations', () => {
const tasks = [
{ status: 'done' as TaskStatus },
{ status: 'in_progress' as TaskStatus },
{ status: 'pr_created' as TaskStatus },
{ status: 'backlog' as TaskStatus },
{ status: 'done' as TaskStatus },
{ status: 'human_review' as TaskStatus, reviewReason: 'completed' as ReviewReason },
];
const completedCount = tasks.reduce(
(count, task) => (isCompletedTask(task.status, task.reviewReason) ? count + 1 : count),
0,
);
expect(completedCount).toBe(4);
});
});
});
@@ -0,0 +1,26 @@
/**
* Task status utility functions
*/
import type { TaskStatus, ReviewReason } from '../types';
/**
* Checks if a task is in a completed state.
* Completed tasks are those in 'done', 'pr_created' status,
* or 'human_review' with reviewReason 'completed'.
*
* @param status - The task status to check
* @param reviewReason - The review reason (only relevant for human_review status)
* @returns true if the task is completed, false otherwise
*/
export function isCompletedTask(status: TaskStatus, reviewReason?: ReviewReason): boolean {
if (status === 'done' || status === 'pr_created') {
return true;
}
// Tasks in human_review with reviewReason 'completed' are also considered completed
// (all subtasks done and QA passed, ready for final approval/merge)
if (status === 'human_review' && reviewReason === 'completed') {
return true;
}
return false;
}