Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/services/manager/worker-pool-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,16 @@ export class WorkerPoolManager implements WorkerPoolManagerInterface {
throw new Error(`Worker ${workerId} is not available for new task (status: ${currentStatus})`);
}

if ((isFeedbackAction || isResumeAction || isMergeAction) &&
// RESUME_TASK는 IDLE 또는 WAITING 상태에서 허용 (Worker 클래스와 일치)
if (isResumeAction &&
currentStatus !== WorkerStatus.WAITING &&
currentStatus !== WorkerStatus.ERROR &&
currentStatus !== WorkerStatus.IDLE) {
throw new Error(`Worker ${workerId} is not available for ${task.action} (status: ${currentStatus})`);
}

// FEEDBACK 및 MERGE 작업은 WAITING 상태에서만 허용
if ((isFeedbackAction || isMergeAction) &&
currentStatus !== WorkerStatus.WAITING) {
throw new Error(`Worker ${workerId} is not available for ${task.action} (status: ${currentStatus})`);
}
Expand Down
26 changes: 9 additions & 17 deletions src/services/manager/workspace-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -394,22 +394,22 @@ export class WorkspaceManager implements WorkspaceManagerInterface {
}

/**
* 워크스페이스 디렉토리가 유효한지 확인합니다.
* 디렉토리가 존재하면 기본적으로 유효한 것으로 간주합니다.
* 워크스페이스 디렉토리가 유효한 Git worktree인지 확인합니다.
*/
async isWorktreeValid(workspaceInfo: WorkspaceInfo): Promise<boolean> {
try {
// 디렉토리 존재 확인 - 이것이 가장 중요한 검증
// 디렉토리 존재 확인
const directoryExists = await this.checkDirectoryExists(workspaceInfo.workspaceDir);
if (!directoryExists) {
return false;
}

// 디렉토리가 있으면 기본적으로 유효한 것으로 간주
// Git 워크트리 세부 검증은 선택적으로 수행
// Git worktree 검증 - .git 파일 존재 및 내용 확인
const gitPath = path.join(workspaceInfo.workspaceDir, '.git');
try {
await fs.access(gitPath);
const gitContent = await fs.readFile(gitPath, 'utf-8');

// Git worktree는 .git 파일에 "gitdir: ..." 형태로 저장됨
const isWorktree = gitContent.trim().startsWith('gitdir:');

Expand All @@ -421,22 +421,14 @@ export class WorkspaceManager implements WorkspaceManagerInterface {
gitContent: gitContent.substring(0, 100) // 첫 100자만 로그
});

// Git worktree가 아니어도 디렉토리가 있으면 재사용 가능
if (!isWorktree) {
this.dependencies.logger.info('Directory exists but not a valid worktree, will be reused anyway', {
taskId: workspaceInfo.taskId,
workspaceDir: workspaceInfo.workspaceDir
});
}

return true; // 디렉토리가 있으면 항상 유효
return isWorktree;
} catch {
// .git 파일이 없어도 디렉토리가 있으면 사용 가능
this.dependencies.logger.debug('.git file not found, but directory exists and will be reused', {
// .git 파일이 없으면 유효하지 않은 워크트리
this.dependencies.logger.debug('.git file not found, worktree is invalid', {
taskId: workspaceInfo.taskId,
workspaceDir: workspaceInfo.workspaceDir
});
return true;
return false;
}
} catch (error) {
this.dependencies.logger.error('Error validating workspace directory', {
Expand Down
7 changes: 7 additions & 0 deletions tests/helpers/mock-builders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,13 @@ export class MockWorkerPoolManagerBuilder {
this.methods.set('storeTaskResult', jest.fn());
this.methods.set('getTaskResult', jest.fn());
this.methods.set('clearTaskResult', jest.fn());
this.methods.set('getWorkspaceManager', jest.fn(() => ({
// 기본 WorkspaceManager mock
getWorkerWorkspace: jest.fn(),
checkWorkspaceExists: jest.fn(),
createWorkerWorkspace: jest.fn(),
removeWorkerWorkspace: jest.fn()
})));
Comment on lines +146 to +152
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

getWorkspaceManager의 모의(mock) 구현이 WorkspaceManagerInterface와 일치하지 않습니다. 현재 모의 구현된 getWorkerWorkspace, checkWorkspaceExists 등은 인터페이스에 존재하지 않는 메서드입니다. 이로 인해 테스트가 의도와 다르게 동작하거나 혼란을 줄 수 있습니다. WorkspaceManagerInterface에 정의된 메서드들(예: createWorkspace, isWorktreeValid 등)을 모의 구현하도록 수정하는 것이 좋습니다.

    this.methods.set('getWorkspaceManager', jest.fn(() => ({
      // WorkspaceManagerInterface에 맞는 기본 mock
      createWorkspace: jest.fn(),
      setupWorktree: jest.fn(),
      setupClaudeLocal: jest.fn(),
      cleanupWorkspace: jest.fn(),
      getWorkspaceInfo: jest.fn(),
      isWorktreeValid: jest.fn(),
    })));

}

withWorker(worker: Worker): this {
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/end-to-end-system.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@ describe('시스템 전체 통합 테스트 (End-to-End)', () => {

// 전체 처리 시간 검증
const totalProcessingTime = Date.now() - feedbackStartTime;
expect(totalProcessingTime).toBeLessThan(15200); // 15초 이내 처리
expect(totalProcessingTime).toBeLessThan(15250); // 15초 이내 처리

}, 40000);
});
Expand Down
146 changes: 120 additions & 26 deletions tests/integration/task-reassignment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ import { TaskRequestHandler } from '../../src/app/TaskRequestHandler';
import { WorkerPoolManager } from '../../src/services/manager/worker-pool-manager';
import { WorkspaceManager } from '../../src/services/manager/workspace-manager';
import { StateManager } from '../../src/services/state-manager';
import { Logger } from '../../src/services/logger';
import { TaskRequest, ResponseStatus, WorkerAction } from '../../src/types';
import { Logger, LogLevel } from '../../src/services/logger';
import { TaskRequest, ResponseStatus, WorkerAction, TaskAction } from '../../src/types';
import { ManagerServiceConfig } from '../../src/types/manager.types';
import { DeveloperConfig } from '../../src/types/developer.types';
import { ProjectBoardItem } from '../../src/types/project-board.types';
import fs from 'fs/promises';
import path from 'path';
import os from 'os';
Expand All @@ -26,8 +27,8 @@ describe('Task Reassignment Integration Tests', () => {

// Logger 초기화
logger = new Logger({
serviceName: 'task-reassignment-test',
logLevel: 'debug',
level: LogLevel.DEBUG,
filePath: path.join(testDataDir, 'test.log'),
enableConsole: false
});

Expand All @@ -36,9 +37,13 @@ describe('Task Reassignment Integration Tests', () => {
await stateManager.initialize();

// WorkspaceManager 초기화
const workspaceConfig = {
const workspaceConfig: ManagerServiceConfig = {
workspaceBasePath: testWorkspaceDir,
repositoriesBasePath: testWorkspaceDir,
minWorkers: 1,
maxWorkers: 3,
workerRecoveryTimeoutMs: 30000,
gitOperationTimeoutMs: 60000,
repositoryCacheTimeoutMs: 300000,
workerLifecycle: {
idleTimeoutMinutes: 30,
cleanupIntervalMinutes: 60,
Expand Down Expand Up @@ -75,10 +80,12 @@ describe('Task Reassignment Integration Tests', () => {

// WorkerPoolManager 초기화
const managerConfig: ManagerServiceConfig = {
workspaceBasePath: testWorkspaceDir,
minWorkers: 1,
maxWorkers: 3,
workspaceBasePath: testWorkspaceDir,
repositoriesBasePath: testWorkspaceDir,
workerRecoveryTimeoutMs: 30000,
gitOperationTimeoutMs: 60000,
repositoryCacheTimeoutMs: 300000,
workerLifecycle: {
idleTimeoutMinutes: 30,
cleanupIntervalMinutes: 60,
Expand All @@ -87,20 +94,47 @@ describe('Task Reassignment Integration Tests', () => {
};

const developerConfig: DeveloperConfig = {
claude: {
apiKey: 'test-key',
model: 'claude-3-sonnet-20240229',
maxTokens: 4000
timeoutMs: 60000,
maxRetries: 3,
retryDelayMs: 1000,
mock: {
responseDelay: 100
}
};

// Mock Developer Factory 생성
const mockDeveloper = {
type: 'mock' as const,
initialize: jest.fn().mockResolvedValue(undefined),
executePrompt: jest.fn().mockResolvedValue({
rawOutput: 'test output',
result: { success: true, prLink: 'https://github.com/test/pr/1' },
executedCommands: [],
modifiedFiles: [],
metadata: {
startTime: new Date(),
endTime: new Date(),
duration: 1000,
developerType: 'mock' as const
}
}),
cleanup: jest.fn().mockResolvedValue(undefined),
isAvailable: jest.fn().mockResolvedValue(true),
setTimeout: jest.fn()
};

const mockDeveloperFactory = {
create: jest.fn().mockReturnValue(mockDeveloper)
} as any;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

mockDeveloperFactoryas any로 타입 캐스팅하면 타입 검사를 무력화하여 잠재적인 오류를 숨길 수 있습니다. as any는 가급적 피하는 것이 좋습니다. WorkerPoolManager의 의존성 주입 시 developerFactory의 타입을 typeof DeveloperFactory 대신 create 메서드를 가진 객체 인터페이스로 변경하면 as any 없이도 타입 안전성을 확보할 수 있을 것입니다. 불가피하게 사용해야 한다면, 그 이유를 주석으로 남겨두는 것을 권장합니다.


workerPoolManager = new WorkerPoolManager(
managerConfig,
{
logger,
stateManager,
workspaceManager,
developerConfig
developerConfig,
developerFactory: mockDeveloperFactory
}
);

Expand All @@ -113,28 +147,38 @@ describe('Task Reassignment Integration Tests', () => {
undefined, // pullRequestService
logger
);

// WorkerTaskExecutor의 assignAndExecuteTask를 mock
jest.spyOn(taskRequestHandler['workerTaskExecutor'], 'assignAndExecuteTask')
.mockResolvedValue(undefined);
});

afterEach(async () => {
// 정리
await workerPoolManager.shutdown();
await fs.rmdir(testDataDir, { recursive: true }).catch(() => {});
await fs.rmdir(testWorkspaceDir, { recursive: true }).catch(() => {});
await fs.rm(testDataDir, { recursive: true, force: true }).catch(() => {});
await fs.rm(testWorkspaceDir, { recursive: true, force: true }).catch(() => {});
});

describe('진행중 작업 재할당', () => {
it('workspace가 존재하지 않는 idle Worker에는 RESUME_TASK를 할당하지 않는다', async () => {
// Given: 작업 요청
const taskRequest: TaskRequest = {
taskId: 'test-task-1',
action: 'check_status',
action: TaskAction.CHECK_STATUS,
boardItem: {
id: 'test-task-1',
title: '테스트 작업',
status: 'Todo',
assignee: null,
labels: [],
createdAt: new Date(),
updatedAt: new Date(),
pullRequestUrls: [],
metadata: {
repository: 'test-owner/test-repo'
}
}
} as ProjectBoardItem
};

// When: 작업 상태 확인 요청 (Worker가 없어서 재할당 시도)
Expand Down Expand Up @@ -166,22 +210,60 @@ describe('Task Reassignment Integration Tests', () => {
'gitdir: /path/to/repo/.git/worktrees/test'
);

// Validation: workspace가 올바르게 저장되었는지 확인
const savedWorkspaceInfo = await workspaceManager.getWorkspaceInfo(taskId);
expect(savedWorkspaceInfo).not.toBeNull();
expect(savedWorkspaceInfo?.taskId).toBe(taskId);

// Validation: workspace가 유효한지 확인
const isValid = await workspaceManager.isWorktreeValid(workspaceInfo);
expect(isValid).toBe(true);

// Given: 작업 요청
const taskRequest: TaskRequest = {
taskId,
action: 'check_status',
action: TaskAction.CHECK_STATUS,
boardItem: {
id: taskId,
title: '테스트 작업 2',
status: 'In Progress',
assignee: null,
labels: [],
createdAt: new Date(),
updatedAt: new Date(),
pullRequestUrls: [],
metadata: {
repository: 'test-owner/test-repo'
}
}
} as ProjectBoardItem
};

// Debug: reassignTask 단계별 확인
const availableWorker = await workerPoolManager.getAvailableWorker();
if (!availableWorker) {
throw new Error('No available worker found for reassignment test');
}
Comment on lines +242 to +245
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

테스트 코드에 "Debug:" 주석과 함께 throw new Error(...)를 사용한 검증 로직이 남아있습니다. 이 코드는 디버깅 목적으로 추가된 것으로 보이며, 최종 코드에서는 Jest의 expect와 같은 표준 단언(assertion)으로 대체하거나 제거하는 것이 좋습니다. 예를 들어, if (!availableWorker) { throw new Error(...) } 대신 expect(availableWorker).not.toBeNull()과 같이 작성하면 테스트 결과가 더 일관되게 보고됩니다. 이 패턴은 파일 내 다른 곳(예: 255, 263 라인)에서도 발견되므로 함께 수정하면 좋겠습니다.


const workerInstance = await workerPoolManager.getWorkerInstance(availableWorker.id);
if (!workerInstance) {
throw new Error(`Worker instance not found for worker ID: ${availableWorker.id}`);
}

// 재할당 체크 테스트
const taskAssignmentValidator = taskRequestHandler['taskAssignmentValidator'];
const reassignmentCheck = await taskAssignmentValidator.validateTaskReassignment(taskId);
if (!reassignmentCheck.allowed || !reassignmentCheck.hasWorkspace) {
throw new Error(`Reassignment validation failed: ${reassignmentCheck.reason}`);
}

// When: 작업 상태 확인 요청
const response = await taskRequestHandler.handleTaskRequest(taskRequest);

// Debug: 실제 응답 확인
if (response.status === ResponseStatus.ERROR) {
throw new Error(`Expected IN_PROGRESS but got ERROR. Message: ${response.message}`);
}

// Then: workspace가 있으므로 재할당 성공
expect(response.status).toBe(ResponseStatus.IN_PROGRESS);
expect(response.message).toContain('reassigned');
Expand All @@ -201,18 +283,24 @@ describe('Task Reassignment Integration Tests', () => {
// When: workspace 유효성 검증
const isValid = await workspaceManager.isWorktreeValid(workspaceInfo);

// Then: .git 파일이 없어도 디렉토리가 있으면 유효한 것으로 판단 (재사용 가능)
expect(isValid).toBe(true);
// Then: .git 파일이 없으면 유효하지 않은 worktree로 판단
expect(isValid).toBe(false);
});

it('WorkerPoolManager의 canAssignIdleWorkerToTask가 올바르게 작동한다', async () => {
// Given: workspace가 있는 작업
// Given: 유효한 workspace가 있는 작업
const taskId = 'test-task-4';
const workspaceInfo = await workspaceManager.createWorkspace(
taskId,
'test-owner/test-repo'
);
await fs.mkdir(workspaceInfo.workspaceDir, { recursive: true });

// .git 파일 생성 (유효한 worktree로 만들기)
await fs.writeFile(
path.join(workspaceInfo.workspaceDir, '.git'),
'gitdir: /path/to/repo/.git/worktrees/test'
);

// Given: idle 상태 Worker
const worker = await workerPoolManager.getAvailableWorker();
Expand All @@ -225,29 +313,35 @@ describe('Task Reassignment Integration Tests', () => {
{ id: taskId, title: '테스트 작업 4' }
);

// Then: workspace가 있으므로 할당 가능
// Then: 유효한 workspace가 있으므로 할당 가능
expect(canAssign).toBe(true);
});

it('TaskAssignmentValidator의 우선순위 시스템이 올바르게 작동한다', async () => {
// Given: workspace가 있는 작업과 없는 작업
// Given: 유효한 workspace가 있는 작업과 없는 작업
const taskWithWorkspace = 'task-with-workspace';
const taskWithoutWorkspace = 'task-without-workspace';

// workspace 생성
// 유효한 workspace 생성
const workspaceInfo = await workspaceManager.createWorkspace(
taskWithWorkspace,
'test-owner/test-repo'
);
await fs.mkdir(workspaceInfo.workspaceDir, { recursive: true });

// .git 파일 생성 (유효한 worktree로 만들기)
await fs.writeFile(
path.join(workspaceInfo.workspaceDir, '.git'),
'gitdir: /path/to/repo/.git/worktrees/test'
);

// When: 우선순위 확인
const priorityWithWorkspace = await workerPoolManager['taskAssignmentValidator']
.getTaskReassignmentPriority(taskWithWorkspace);
const priorityWithoutWorkspace = await workerPoolManager['taskAssignmentValidator']
.getTaskReassignmentPriority(taskWithoutWorkspace);

// Then: workspace가 있는 작업이 더 높은 우선순위를 가짐
// Then: 유효한 workspace가 있는 작업이 더 높은 우선순위를 가짐
expect(priorityWithWorkspace).toBe(10); // 높은 우선순위
expect(priorityWithoutWorkspace).toBe(5); // 중간 우선순위
});
Expand Down
Loading
Loading