takt: taskがちゃんと正常終了しなくてもcompletedに進んでしまうことがある。修正してほしい
This commit is contained in:
nrslib
parent
60f7c0851d
commit
c2f530f2a0
16
04-ai-review.md
Normal file
16
04-ai-review.md
Normal file
@ -0,0 +1,16 @@
|
||||
# AI生成コードレビュー
|
||||
|
||||
## 結果: APPROVE
|
||||
|
||||
## サマリー
|
||||
失敗タスクを`completed`ではなく`failed`ディレクトリに移動する修正は、既存パターンに適合し、API・呼び出し元の整合性も問題なし。
|
||||
|
||||
## 検証した項目
|
||||
| 観点 | 結果 | 備考 |
|
||||
|------|------|------|
|
||||
| 仮定の妥当性 | ✅ | 元の要求（失敗タスクがcompletedに入る問題）を正確に修正 |
|
||||
| API/ライブラリの実在 | ✅ | Node.js fs API全て正しく使用 |
|
||||
| コンテキスト適合 | ✅ | 命名・構造・エラーハンドリングが既存コードと一致 |
|
||||
| スコープ | ✅ | 必要最小限の変更、スコープクリープなし |
|
||||
| 呼び出し元の配線 | ✅ | completeTask/failTask の全3箇所の呼び出し元が正しく更新済み |
|
||||
| デッドコード | ✅ | 不要コードの残存なし |
|
||||
@ -3,7 +3,7 @@
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { mkdirSync, writeFileSync, existsSync, rmSync, readFileSync } from 'node:fs';
|
||||
import { mkdirSync, writeFileSync, existsSync, rmSync, readFileSync, readdirSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { TaskRunner } from '../task/runner.js';
|
||||
|
||||
@ -23,10 +23,11 @@ describe('TaskRunner', () => {
|
||||
});
|
||||
|
||||
describe('ensureDirs', () => {
|
||||
it('should create tasks and completed directories', () => {
|
||||
it('should create tasks, completed, and failed directories', () => {
|
||||
runner.ensureDirs();
|
||||
expect(existsSync(join(testDir, '.takt', 'tasks'))).toBe(true);
|
||||
expect(existsSync(join(testDir, '.takt', 'completed'))).toBe(true);
|
||||
expect(existsSync(join(testDir, '.takt', 'failed'))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@ -134,7 +135,7 @@ describe('TaskRunner', () => {
|
||||
expect(logData.success).toBe(true);
|
||||
});
|
||||
|
||||
it('should record failure status', () => {
|
||||
it('should throw error when called with a failed result', () => {
|
||||
const tasksDir = join(testDir, '.takt', 'tasks');
|
||||
mkdirSync(tasksDir, { recursive: true });
|
||||
writeFileSync(join(tasksDir, 'fail-task.md'), 'Will fail');
|
||||
@ -149,9 +150,75 @@ describe('TaskRunner', () => {
|
||||
completedAt: '2024-01-01T00:01:00.000Z',
|
||||
};
|
||||
|
||||
const reportFile = runner.completeTask(result);
|
||||
expect(() => runner.completeTask(result)).toThrow(
|
||||
'Cannot complete a failed task. Use failTask() instead.'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('failTask', () => {
|
||||
it('should move task to failed directory', () => {
|
||||
const tasksDir = join(testDir, '.takt', 'tasks');
|
||||
mkdirSync(tasksDir, { recursive: true });
|
||||
const taskFile = join(tasksDir, 'fail-task.md');
|
||||
writeFileSync(taskFile, 'Task that will fail');
|
||||
|
||||
const task = runner.getTask('fail-task')!;
|
||||
const result = {
|
||||
task,
|
||||
success: false,
|
||||
response: 'Error occurred',
|
||||
executionLog: ['Started', 'Error'],
|
||||
startedAt: '2024-01-01T00:00:00.000Z',
|
||||
completedAt: '2024-01-01T00:01:00.000Z',
|
||||
};
|
||||
|
||||
const reportFile = runner.failTask(result);
|
||||
|
||||
// Original task file should be removed from tasks dir
|
||||
expect(existsSync(taskFile)).toBe(false);
|
||||
|
||||
// Report should be in .takt/failed/ (not .takt/completed/)
|
||||
expect(reportFile).toContain(join('.takt', 'failed'));
|
||||
expect(reportFile).not.toContain(join('.takt', 'completed'));
|
||||
expect(existsSync(reportFile)).toBe(true);
|
||||
|
||||
const reportContent = readFileSync(reportFile, 'utf-8');
|
||||
expect(reportContent).toContain('# タスク実行レポート');
|
||||
expect(reportContent).toContain('fail-task');
|
||||
expect(reportContent).toContain('失敗');
|
||||
|
||||
// Log file should be created in failed dir
|
||||
const logFile = reportFile.replace('report.md', 'log.json');
|
||||
expect(existsSync(logFile)).toBe(true);
|
||||
const logData = JSON.parse(readFileSync(logFile, 'utf-8'));
|
||||
expect(logData.taskName).toBe('fail-task');
|
||||
expect(logData.success).toBe(false);
|
||||
});
|
||||
|
||||
it('should not move failed task to completed directory', () => {
|
||||
const tasksDir = join(testDir, '.takt', 'tasks');
|
||||
const completedDir = join(testDir, '.takt', 'completed');
|
||||
mkdirSync(tasksDir, { recursive: true });
|
||||
const taskFile = join(tasksDir, 'another-fail.md');
|
||||
writeFileSync(taskFile, 'Another failing task');
|
||||
|
||||
const task = runner.getTask('another-fail')!;
|
||||
const result = {
|
||||
task,
|
||||
success: false,
|
||||
response: 'Something went wrong',
|
||||
executionLog: [],
|
||||
startedAt: '2024-01-01T00:00:00.000Z',
|
||||
completedAt: '2024-01-01T00:01:00.000Z',
|
||||
};
|
||||
|
||||
runner.failTask(result);
|
||||
|
||||
// completed directory should be empty (only the dir itself exists)
|
||||
mkdirSync(completedDir, { recursive: true });
|
||||
const completedContents = readdirSync(completedDir);
|
||||
expect(completedContents).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@ -86,18 +86,20 @@ export async function executeAndCompleteTask(
|
||||
}
|
||||
}
|
||||
|
||||
taskRunner.completeTask({
|
||||
const taskResult = {
|
||||
task,
|
||||
success: taskSuccess,
|
||||
response: taskSuccess ? 'Task completed successfully' : 'Task failed',
|
||||
executionLog,
|
||||
startedAt,
|
||||
completedAt,
|
||||
});
|
||||
};
|
||||
|
||||
if (taskSuccess) {
|
||||
taskRunner.completeTask(taskResult);
|
||||
success(`Task "${task.name}" completed`);
|
||||
} else {
|
||||
taskRunner.failTask(taskResult);
|
||||
error(`Task "${task.name}" failed`);
|
||||
}
|
||||
|
||||
@ -105,7 +107,7 @@ export async function executeAndCompleteTask(
|
||||
} catch (err) {
|
||||
const completedAt = new Date().toISOString();
|
||||
|
||||
taskRunner.completeTask({
|
||||
taskRunner.failTask({
|
||||
task,
|
||||
success: false,
|
||||
response: getErrorMessage(err),
|
||||
|
||||
@ -45,17 +45,20 @@ export class TaskRunner {
|
||||
private projectDir: string;
|
||||
private tasksDir: string;
|
||||
private completedDir: string;
|
||||
private failedDir: string;
|
||||
|
||||
constructor(projectDir: string) {
|
||||
this.projectDir = projectDir;
|
||||
this.tasksDir = path.join(projectDir, '.takt', 'tasks');
|
||||
this.completedDir = path.join(projectDir, '.takt', 'completed');
|
||||
this.failedDir = path.join(projectDir, '.takt', 'failed');
|
||||
}
|
||||
|
||||
/** ディレクトリ構造を作成 */
|
||||
ensureDirs(): void {
|
||||
fs.mkdirSync(this.tasksDir, { recursive: true });
|
||||
fs.mkdirSync(this.completedDir, { recursive: true });
|
||||
fs.mkdirSync(this.failedDir, { recursive: true });
|
||||
}
|
||||
|
||||
/** タスクディレクトリのパスを取得 */
|
||||
@ -126,30 +129,52 @@ export class TaskRunner {
|
||||
* @returns レポートファイルのパス
|
||||
*/
|
||||
completeTask(result: TaskResult): string {
|
||||
if (!result.success) {
|
||||
throw new Error('Cannot complete a failed task. Use failTask() instead.');
|
||||
}
|
||||
return this.moveTask(result, this.completedDir);
|
||||
}
|
||||
|
||||
/**
|
||||
* タスクを失敗としてマーク
|
||||
*
|
||||
* タスクファイルを .takt/failed に移動し、
|
||||
* レポートファイルを作成する。
|
||||
*
|
||||
* @returns レポートファイルのパス
|
||||
*/
|
||||
failTask(result: TaskResult): string {
|
||||
return this.moveTask(result, this.failedDir);
|
||||
}
|
||||
|
||||
/**
|
||||
* タスクファイルを指定ディレクトリに移動し、レポート・ログを生成する
|
||||
*/
|
||||
private moveTask(result: TaskResult, targetDir: string): string {
|
||||
this.ensureDirs();
|
||||
|
||||
// タイムスタンプを生成
|
||||
const timestamp = new Date().toISOString().replace(/[:.]/g, '-').slice(0, 19);
|
||||
|
||||
// 完了ディレクトリにサブディレクトリを作成
|
||||
const taskCompletedDir = path.join(
|
||||
this.completedDir,
|
||||
// ターゲットディレクトリにサブディレクトリを作成
|
||||
const taskTargetDir = path.join(
|
||||
targetDir,
|
||||
`${timestamp}_${result.task.name}`
|
||||
);
|
||||
fs.mkdirSync(taskCompletedDir, { recursive: true });
|
||||
fs.mkdirSync(taskTargetDir, { recursive: true });
|
||||
|
||||
// 元のタスクファイルを移動（元の拡張子を保持）
|
||||
const originalExt = path.extname(result.task.filePath);
|
||||
const completedTaskFile = path.join(taskCompletedDir, `${result.task.name}${originalExt}`);
|
||||
fs.renameSync(result.task.filePath, completedTaskFile);
|
||||
const movedTaskFile = path.join(taskTargetDir, `${result.task.name}${originalExt}`);
|
||||
fs.renameSync(result.task.filePath, movedTaskFile);
|
||||
|
||||
// レポートを生成
|
||||
const reportFile = path.join(taskCompletedDir, 'report.md');
|
||||
const reportFile = path.join(taskTargetDir, 'report.md');
|
||||
const reportContent = this.generateReport(result);
|
||||
fs.writeFileSync(reportFile, reportContent, 'utf-8');
|
||||
|
||||
// ログを保存
|
||||
const logFile = path.join(taskCompletedDir, 'log.json');
|
||||
const logFile = path.join(taskTargetDir, 'log.json');
|
||||
const logData = {
|
||||
taskName: result.task.name,
|
||||
success: result.success,
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user