Merge branch 'takt/2026-01-28T1032-taskがちゃんと正常終了しなくてもcompletedに進んでしまうことがある。修正してほしい'
This commit is contained in:
commit
c729e8766a
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 { 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 { join } from 'node:path';
|
||||||
import { TaskRunner } from '../task/runner.js';
|
import { TaskRunner } from '../task/runner.js';
|
||||||
|
|
||||||
@ -23,10 +23,11 @@ describe('TaskRunner', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe('ensureDirs', () => {
|
describe('ensureDirs', () => {
|
||||||
it('should create tasks and completed directories', () => {
|
it('should create tasks, completed, and failed directories', () => {
|
||||||
runner.ensureDirs();
|
runner.ensureDirs();
|
||||||
expect(existsSync(join(testDir, '.takt', 'tasks'))).toBe(true);
|
expect(existsSync(join(testDir, '.takt', 'tasks'))).toBe(true);
|
||||||
expect(existsSync(join(testDir, '.takt', 'completed'))).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);
|
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');
|
const tasksDir = join(testDir, '.takt', 'tasks');
|
||||||
mkdirSync(tasksDir, { recursive: true });
|
mkdirSync(tasksDir, { recursive: true });
|
||||||
writeFileSync(join(tasksDir, 'fail-task.md'), 'Will fail');
|
writeFileSync(join(tasksDir, 'fail-task.md'), 'Will fail');
|
||||||
@ -149,9 +150,75 @@ describe('TaskRunner', () => {
|
|||||||
completedAt: '2024-01-01T00:01:00.000Z',
|
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');
|
const reportContent = readFileSync(reportFile, 'utf-8');
|
||||||
|
expect(reportContent).toContain('# タスク実行レポート');
|
||||||
|
expect(reportContent).toContain('fail-task');
|
||||||
expect(reportContent).toContain('失敗');
|
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);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@ -88,18 +88,20 @@ export async function executeAndCompleteTask(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
taskRunner.completeTask({
|
const taskResult = {
|
||||||
task,
|
task,
|
||||||
success: taskSuccess,
|
success: taskSuccess,
|
||||||
response: taskSuccess ? 'Task completed successfully' : 'Task failed',
|
response: taskSuccess ? 'Task completed successfully' : 'Task failed',
|
||||||
executionLog,
|
executionLog,
|
||||||
startedAt,
|
startedAt,
|
||||||
completedAt,
|
completedAt,
|
||||||
});
|
};
|
||||||
|
|
||||||
if (taskSuccess) {
|
if (taskSuccess) {
|
||||||
|
taskRunner.completeTask(taskResult);
|
||||||
success(`Task "${task.name}" completed`);
|
success(`Task "${task.name}" completed`);
|
||||||
} else {
|
} else {
|
||||||
|
taskRunner.failTask(taskResult);
|
||||||
error(`Task "${task.name}" failed`);
|
error(`Task "${task.name}" failed`);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -107,7 +109,7 @@ export async function executeAndCompleteTask(
|
|||||||
} catch (err) {
|
} catch (err) {
|
||||||
const completedAt = new Date().toISOString();
|
const completedAt = new Date().toISOString();
|
||||||
|
|
||||||
taskRunner.completeTask({
|
taskRunner.failTask({
|
||||||
task,
|
task,
|
||||||
success: false,
|
success: false,
|
||||||
response: getErrorMessage(err),
|
response: getErrorMessage(err),
|
||||||
|
|||||||
@ -45,17 +45,20 @@ export class TaskRunner {
|
|||||||
private projectDir: string;
|
private projectDir: string;
|
||||||
private tasksDir: string;
|
private tasksDir: string;
|
||||||
private completedDir: string;
|
private completedDir: string;
|
||||||
|
private failedDir: string;
|
||||||
|
|
||||||
constructor(projectDir: string) {
|
constructor(projectDir: string) {
|
||||||
this.projectDir = projectDir;
|
this.projectDir = projectDir;
|
||||||
this.tasksDir = path.join(projectDir, '.takt', 'tasks');
|
this.tasksDir = path.join(projectDir, '.takt', 'tasks');
|
||||||
this.completedDir = path.join(projectDir, '.takt', 'completed');
|
this.completedDir = path.join(projectDir, '.takt', 'completed');
|
||||||
|
this.failedDir = path.join(projectDir, '.takt', 'failed');
|
||||||
}
|
}
|
||||||
|
|
||||||
/** ディレクトリ構造を作成 */
|
/** ディレクトリ構造を作成 */
|
||||||
ensureDirs(): void {
|
ensureDirs(): void {
|
||||||
fs.mkdirSync(this.tasksDir, { recursive: true });
|
fs.mkdirSync(this.tasksDir, { recursive: true });
|
||||||
fs.mkdirSync(this.completedDir, { recursive: true });
|
fs.mkdirSync(this.completedDir, { recursive: true });
|
||||||
|
fs.mkdirSync(this.failedDir, { recursive: true });
|
||||||
}
|
}
|
||||||
|
|
||||||
/** タスクディレクトリのパスを取得 */
|
/** タスクディレクトリのパスを取得 */
|
||||||
@ -126,30 +129,52 @@ export class TaskRunner {
|
|||||||
* @returns レポートファイルのパス
|
* @returns レポートファイルのパス
|
||||||
*/
|
*/
|
||||||
completeTask(result: TaskResult): string {
|
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();
|
this.ensureDirs();
|
||||||
|
|
||||||
// タイムスタンプを生成
|
// タイムスタンプを生成
|
||||||
const timestamp = new Date().toISOString().replace(/[:.]/g, '-').slice(0, 19);
|
const timestamp = new Date().toISOString().replace(/[:.]/g, '-').slice(0, 19);
|
||||||
|
|
||||||
// 完了ディレクトリにサブディレクトリを作成
|
// ターゲットディレクトリにサブディレクトリを作成
|
||||||
const taskCompletedDir = path.join(
|
const taskTargetDir = path.join(
|
||||||
this.completedDir,
|
targetDir,
|
||||||
`${timestamp}_${result.task.name}`
|
`${timestamp}_${result.task.name}`
|
||||||
);
|
);
|
||||||
fs.mkdirSync(taskCompletedDir, { recursive: true });
|
fs.mkdirSync(taskTargetDir, { recursive: true });
|
||||||
|
|
||||||
// 元のタスクファイルを移動(元の拡張子を保持)
|
// 元のタスクファイルを移動(元の拡張子を保持)
|
||||||
const originalExt = path.extname(result.task.filePath);
|
const originalExt = path.extname(result.task.filePath);
|
||||||
const completedTaskFile = path.join(taskCompletedDir, `${result.task.name}${originalExt}`);
|
const movedTaskFile = path.join(taskTargetDir, `${result.task.name}${originalExt}`);
|
||||||
fs.renameSync(result.task.filePath, completedTaskFile);
|
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);
|
const reportContent = this.generateReport(result);
|
||||||
fs.writeFileSync(reportFile, reportContent, 'utf-8');
|
fs.writeFileSync(reportFile, reportContent, 'utf-8');
|
||||||
|
|
||||||
// ログを保存
|
// ログを保存
|
||||||
const logFile = path.join(taskCompletedDir, 'log.json');
|
const logFile = path.join(taskTargetDir, 'log.json');
|
||||||
const logData = {
|
const logData = {
|
||||||
taskName: result.task.name,
|
taskName: result.task.name,
|
||||||
success: result.success,
|
success: result.success,
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user