feat: PR作成失敗時のタスクステータスを failed から pr_failed に分離
コード実行は成功したが PR 作成のみ失敗した場合、タスク全体を failed 扱いにせず pr_failed ステータスで記録する。takt list では [pr-failed] として表示し、completed と同じ diff/merge 操作が可能。
This commit is contained in:
parent
2dc5cf1102
commit
7c1bc44596
@ -50,6 +50,18 @@ function makeFailedRecord() {
|
||||
};
|
||||
}
|
||||
|
||||
function makePrFailedRecord() {
|
||||
return {
|
||||
name: 'test-task',
|
||||
status: 'pr_failed' as const,
|
||||
content: 'task content',
|
||||
created_at: '2025-01-01T00:00:00.000Z',
|
||||
started_at: '2025-01-01T01:00:00.000Z',
|
||||
completed_at: '2025-01-01T02:00:00.000Z',
|
||||
failure: { error: 'PR creation failed: Base ref must be a branch' },
|
||||
};
|
||||
}
|
||||
|
||||
describe('TaskExecutionConfigSchema', () => {
|
||||
it('should accept valid config with all optional fields', () => {
|
||||
const config = {
|
||||
@ -178,6 +190,32 @@ describe('TaskRecordSchema', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('pr_failed status', () => {
|
||||
it('should accept valid pr_failed record with failure', () => {
|
||||
expect(() => TaskRecordSchema.parse(makePrFailedRecord())).not.toThrow();
|
||||
});
|
||||
|
||||
it('should accept pr_failed record without failure (optional)', () => {
|
||||
const record = { ...makePrFailedRecord(), failure: undefined };
|
||||
expect(() => TaskRecordSchema.parse(record)).not.toThrow();
|
||||
});
|
||||
|
||||
it('should reject pr_failed record without started_at', () => {
|
||||
const record = { ...makePrFailedRecord(), started_at: null };
|
||||
expect(() => TaskRecordSchema.parse(record)).toThrow();
|
||||
});
|
||||
|
||||
it('should reject pr_failed record without completed_at', () => {
|
||||
const record = { ...makePrFailedRecord(), completed_at: null };
|
||||
expect(() => TaskRecordSchema.parse(record)).toThrow();
|
||||
});
|
||||
|
||||
it('should reject pr_failed record with owner_pid', () => {
|
||||
const record = { ...makePrFailedRecord(), owner_pid: 1234 };
|
||||
expect(() => TaskRecordSchema.parse(record)).toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('failed status', () => {
|
||||
it('should accept valid failed record', () => {
|
||||
expect(() => TaskRecordSchema.parse(makeFailedRecord())).not.toThrow();
|
||||
|
||||
@ -5,7 +5,7 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import type { TaskInfo } from '../infra/task/index.js';
|
||||
|
||||
const { mockResolveTaskExecution, mockExecutePiece, mockLoadPieceByIdentifier, mockResolvePieceConfigValues, mockResolveConfigValueWithSource, mockBuildTaskResult, mockPersistTaskResult, mockPersistTaskError, mockPostExecutionFlow } =
|
||||
const { mockResolveTaskExecution, mockExecutePiece, mockLoadPieceByIdentifier, mockResolvePieceConfigValues, mockResolveConfigValueWithSource, mockBuildTaskResult, mockPersistTaskResult, mockPersistPrFailedTaskResult, mockPersistTaskError, mockPostExecutionFlow } =
|
||||
vi.hoisted(() => ({
|
||||
mockResolveTaskExecution: vi.fn(),
|
||||
mockExecutePiece: vi.fn(),
|
||||
@ -14,6 +14,7 @@ const { mockResolveTaskExecution, mockExecutePiece, mockLoadPieceByIdentifier, m
|
||||
mockResolveConfigValueWithSource: vi.fn(),
|
||||
mockBuildTaskResult: vi.fn(),
|
||||
mockPersistTaskResult: vi.fn(),
|
||||
mockPersistPrFailedTaskResult: vi.fn(),
|
||||
mockPersistTaskError: vi.fn(),
|
||||
mockPostExecutionFlow: vi.fn(),
|
||||
}));
|
||||
@ -30,6 +31,7 @@ vi.mock('../features/tasks/execute/pieceExecution.js', () => ({
|
||||
vi.mock('../features/tasks/execute/taskResultHandler.js', () => ({
|
||||
buildTaskResult: (...args: unknown[]) => mockBuildTaskResult(...args),
|
||||
persistTaskResult: (...args: unknown[]) => mockPersistTaskResult(...args),
|
||||
persistPrFailedTaskResult: (...args: unknown[]) => mockPersistPrFailedTaskResult(...args),
|
||||
persistTaskError: (...args: unknown[]) => mockPersistTaskError(...args),
|
||||
}));
|
||||
|
||||
@ -199,7 +201,7 @@ describe('executeAndCompleteTask', () => {
|
||||
expect(pieceExecutionOptions?.model).toBe('gpt-5.3-codex');
|
||||
});
|
||||
|
||||
it('should mark task as failed when PR creation fails', async () => {
|
||||
it('should mark task as pr_failed when PR creation fails', async () => {
|
||||
// Given: worktree mode with autoPr enabled, PR creation fails
|
||||
const task = createTask('task-with-pr-failure');
|
||||
mockResolveTaskExecution.mockResolvedValue({
|
||||
@ -223,16 +225,19 @@ describe('executeAndCompleteTask', () => {
|
||||
// When
|
||||
const result = await executeAndCompleteTask(task, {} as never, '/project', 'default');
|
||||
|
||||
// Then: task should be marked as failed
|
||||
expect(result).toBe(false);
|
||||
// Then: code succeeded, task is marked as pr_failed (not failed)
|
||||
expect(result).toBe(true);
|
||||
expect(mockBuildTaskResult).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
runResult: expect.objectContaining({
|
||||
success: false,
|
||||
reason: 'PR creation failed: Base ref must be a branch',
|
||||
}),
|
||||
runResult: expect.objectContaining({ success: true }),
|
||||
}),
|
||||
);
|
||||
expect(mockPersistPrFailedTaskResult).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.anything(),
|
||||
'Base ref must be a branch',
|
||||
);
|
||||
expect(mockPersistTaskResult).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should mark task as completed when PR creation succeeds', async () => {
|
||||
|
||||
@ -24,6 +24,16 @@ describe('formatTaskStatusLabel', () => {
|
||||
expect(formatTaskStatusLabel(task)).toBe('[failed] retry-payment');
|
||||
});
|
||||
|
||||
it("should format pr_failed task as '[pr-failed] name'", () => {
|
||||
const task = makeTask({ kind: 'pr_failed', name: 'create-feature' });
|
||||
expect(formatTaskStatusLabel(task)).toBe('[pr-failed] create-feature');
|
||||
});
|
||||
|
||||
it("should format pr_failed task with branch", () => {
|
||||
const task = makeTask({ kind: 'pr_failed', name: 'create-feature', branch: 'takt/create-feature' });
|
||||
expect(formatTaskStatusLabel(task)).toBe('[pr-failed] create-feature (takt/create-feature)');
|
||||
});
|
||||
|
||||
it('should include branch when present', () => {
|
||||
const task = makeTask({
|
||||
kind: 'completed',
|
||||
|
||||
@ -19,7 +19,7 @@ import type { TaskExecutionOptions, ExecuteTaskOptions, PieceExecutionResult } f
|
||||
import { runWithWorkerPool } from './parallelExecution.js';
|
||||
import { resolveTaskExecution, resolveTaskIssue } from './resolveTask.js';
|
||||
import { postExecutionFlow } from './postExecution.js';
|
||||
import { buildTaskResult, persistExceededTaskResult, persistTaskError, persistTaskResult } from './taskResultHandler.js';
|
||||
import { buildTaskResult, persistExceededTaskResult, persistTaskError, persistPrFailedTaskResult, persistTaskResult } from './taskResultHandler.js';
|
||||
import { generateRunId, toSlackTaskDetail } from './slackSummaryAdapter.js';
|
||||
|
||||
export type { TaskExecutionOptions, ExecuteTaskOptions };
|
||||
@ -176,7 +176,7 @@ export async function executeAndCompleteTask(
|
||||
const completedAt = new Date().toISOString();
|
||||
|
||||
let prUrl: string | undefined;
|
||||
let effectiveRunResult = taskRunResult;
|
||||
let prFailedError: string | undefined;
|
||||
if (taskSuccess && isWorktree) {
|
||||
const issues = resolveTaskIssue(issueNumber);
|
||||
const postResult = await postExecutionFlow({
|
||||
@ -192,22 +192,28 @@ export async function executeAndCompleteTask(
|
||||
});
|
||||
prUrl = postResult.prUrl;
|
||||
if (postResult.prFailed) {
|
||||
effectiveRunResult = { success: false, reason: `PR creation failed: ${postResult.prError}` };
|
||||
prFailedError = postResult.prError;
|
||||
}
|
||||
}
|
||||
|
||||
const taskResult = buildTaskResult({
|
||||
task,
|
||||
runResult: effectiveRunResult,
|
||||
runResult: taskRunResult,
|
||||
startedAt,
|
||||
completedAt,
|
||||
branch,
|
||||
worktreePath,
|
||||
prUrl,
|
||||
});
|
||||
|
||||
if (prFailedError !== undefined) {
|
||||
persistPrFailedTaskResult(taskRunner, taskResult, prFailedError);
|
||||
return true;
|
||||
}
|
||||
|
||||
persistTaskResult(taskRunner, taskResult);
|
||||
|
||||
return effectiveRunResult.success;
|
||||
return taskRunResult.success;
|
||||
} catch (err) {
|
||||
const completedAt = new Date().toISOString();
|
||||
persistTaskError(taskRunner, task, startedAt, completedAt, err);
|
||||
|
||||
@ -80,6 +80,15 @@ export function buildBooleanTaskResult(params: BuildBooleanTaskResultParams): Ta
|
||||
};
|
||||
}
|
||||
|
||||
export function persistPrFailedTaskResult(
|
||||
taskRunner: TaskRunner,
|
||||
taskResult: TaskResult,
|
||||
prError: string,
|
||||
): void {
|
||||
taskRunner.prFailTask(taskResult, prError);
|
||||
info(`Task "${taskResult.task.name}" completed (PR creation failed)`);
|
||||
}
|
||||
|
||||
export function persistTaskResult(
|
||||
taskRunner: TaskRunner,
|
||||
taskResult: TaskResult,
|
||||
|
||||
@ -42,6 +42,7 @@ type PendingTaskAction = 'delete';
|
||||
type ExceededTaskAction = 'requeue' | 'delete';
|
||||
|
||||
type FailedTaskAction = 'retry' | 'delete';
|
||||
type PrFailedTaskAction = ListAction;
|
||||
type CompletedTaskAction = ListAction;
|
||||
|
||||
async function showExceededTaskAndPromptAction(task: TaskListItem): Promise<ExceededTaskAction | null> {
|
||||
@ -95,6 +96,20 @@ async function showFailedTaskAndPromptAction(task: TaskListItem): Promise<Failed
|
||||
);
|
||||
}
|
||||
|
||||
async function showPrFailedTaskAndPromptAction(cwd: string, task: TaskListItem): Promise<PrFailedTaskAction | null> {
|
||||
header(formatTaskStatusLabel(task));
|
||||
info(` Created: ${task.createdAt}`);
|
||||
if (task.content) {
|
||||
info(` ${task.content}`);
|
||||
}
|
||||
if (task.failure) {
|
||||
info(` PR Error: ${task.failure.error}`);
|
||||
}
|
||||
blankLine();
|
||||
|
||||
return await showDiffAndPromptActionForTask(cwd, task);
|
||||
}
|
||||
|
||||
async function showCompletedTaskAndPromptAction(cwd: string, task: TaskListItem): Promise<CompletedTaskAction | null> {
|
||||
header(formatTaskStatusLabel(task));
|
||||
info(` Created: ${task.createdAt}`);
|
||||
@ -226,6 +241,41 @@ export async function listTasks(
|
||||
} else if (taskAction === 'delete') {
|
||||
await deleteTaskByKind(task);
|
||||
}
|
||||
} else if (type === 'pr_failed') {
|
||||
const task = tasks[idx];
|
||||
if (!task) continue;
|
||||
if (!task.branch) {
|
||||
info(`Branch is missing for pr-failed task: ${task.name}`);
|
||||
continue;
|
||||
}
|
||||
const taskAction = await showPrFailedTaskAndPromptAction(cwd, task);
|
||||
if (taskAction === null) continue;
|
||||
|
||||
switch (taskAction) {
|
||||
case 'diff':
|
||||
showFullDiff(cwd, task.branch);
|
||||
break;
|
||||
case 'instruct':
|
||||
await instructBranch(cwd, task);
|
||||
break;
|
||||
case 'sync':
|
||||
await syncBranchWithRoot(cwd, task);
|
||||
break;
|
||||
case 'pull':
|
||||
pullFromRemote(cwd, task);
|
||||
break;
|
||||
case 'try':
|
||||
tryMergeBranch(cwd, task);
|
||||
break;
|
||||
case 'merge':
|
||||
if (mergeBranch(cwd, task)) {
|
||||
runner.deleteTask(task.name, 'pr_failed');
|
||||
}
|
||||
break;
|
||||
case 'delete':
|
||||
await deleteTaskByKind(task);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -40,7 +40,7 @@ export async function deleteTaskByKind(task: TaskListItem): Promise<boolean> {
|
||||
return true;
|
||||
}
|
||||
|
||||
type DeletableTask = TaskListItem & { kind: 'pending' | 'failed' | 'completed' | 'exceeded' };
|
||||
type DeletableTask = TaskListItem & { kind: 'pending' | 'failed' | 'completed' | 'exceeded' | 'pr_failed' };
|
||||
|
||||
export async function deleteAllTasks(tasks: TaskListItem[]): Promise<boolean> {
|
||||
const deletable = tasks.filter((t): t is DeletableTask => t.kind !== 'running');
|
||||
|
||||
@ -6,6 +6,7 @@ const TASK_STATUS_BY_KIND: Record<TaskListItem['kind'], string> = {
|
||||
completed: 'completed',
|
||||
failed: 'failed',
|
||||
exceeded: 'exceeded',
|
||||
pr_failed: 'pr-failed',
|
||||
};
|
||||
|
||||
export function formatTaskStatusLabel(task: TaskListItem): string {
|
||||
|
||||
@ -89,6 +89,14 @@ export function toFailedTaskItem(projectDir: string, tasksFile: string, task: Ta
|
||||
};
|
||||
}
|
||||
|
||||
export function toPrFailedTaskItem(projectDir: string, tasksFile: string, task: TaskRecord): TaskListItem {
|
||||
return {
|
||||
kind: 'pr_failed',
|
||||
...toBaseTaskListItem(projectDir, tasksFile, task),
|
||||
failure: task.failure,
|
||||
};
|
||||
}
|
||||
|
||||
export function toExceededTaskItem(projectDir: string, tasksFile: string, task: TaskRecord): TaskListItem {
|
||||
return {
|
||||
kind: 'exceeded',
|
||||
@ -142,5 +150,7 @@ export function toTaskListItem(projectDir: string, tasksFile: string, task: Task
|
||||
return toFailedTaskItem(projectDir, tasksFile, task);
|
||||
case 'exceeded':
|
||||
return toExceededTaskItem(projectDir, tasksFile, task);
|
||||
case 'pr_failed':
|
||||
return toPrFailedTaskItem(projectDir, tasksFile, task);
|
||||
}
|
||||
}
|
||||
|
||||
@ -67,6 +67,10 @@ export class TaskRunner {
|
||||
return this.lifecycle.failTask(result);
|
||||
}
|
||||
|
||||
prFailTask(result: TaskResult, prError: string): string {
|
||||
return this.lifecycle.prFailTask(result, prError);
|
||||
}
|
||||
|
||||
listPendingTaskItems(): TaskListItem[] {
|
||||
return this.query.listPendingTaskItems();
|
||||
}
|
||||
@ -105,7 +109,7 @@ export class TaskRunner {
|
||||
return this.lifecycle.startReExecution(taskRef, allowedStatuses, startMovement, retryNote);
|
||||
}
|
||||
|
||||
deleteTask(name: string, kind: 'pending' | 'failed' | 'completed' | 'exceeded'): void {
|
||||
deleteTask(name: string, kind: 'pending' | 'failed' | 'completed' | 'exceeded' | 'pr_failed'): void {
|
||||
this.deletion.deleteTaskByNameAndStatus(name, kind);
|
||||
}
|
||||
|
||||
|
||||
@ -32,7 +32,7 @@ export const TaskFileSchema = TaskExecutionConfigSchema.extend({
|
||||
|
||||
export type TaskFileData = z.infer<typeof TaskFileSchema>;
|
||||
|
||||
export const TaskStatusSchema = z.enum(['pending', 'running', 'completed', 'failed', 'exceeded']);
|
||||
export const TaskStatusSchema = z.enum(['pending', 'running', 'completed', 'failed', 'exceeded', 'pr_failed']);
|
||||
export type TaskStatus = z.infer<typeof TaskStatusSchema>;
|
||||
|
||||
export const TaskFailureSchema = z.object({
|
||||
@ -240,6 +240,30 @@ export const TaskRecordSchema = TaskExecutionConfigSchema.extend({
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
if (value.status === 'pr_failed') {
|
||||
if (value.started_at === null) {
|
||||
ctx.addIssue({
|
||||
code: z.ZodIssueCode.custom,
|
||||
path: ['started_at'],
|
||||
message: 'PR-failed task requires started_at.',
|
||||
});
|
||||
}
|
||||
if (value.completed_at === null) {
|
||||
ctx.addIssue({
|
||||
code: z.ZodIssueCode.custom,
|
||||
path: ['completed_at'],
|
||||
message: 'PR-failed task requires completed_at.',
|
||||
});
|
||||
}
|
||||
if (hasOwnerPid) {
|
||||
ctx.addIssue({
|
||||
code: z.ZodIssueCode.custom,
|
||||
path: ['owner_pid'],
|
||||
message: 'PR-failed task must not have owner_pid.',
|
||||
});
|
||||
}
|
||||
}
|
||||
});
|
||||
export type TaskRecord = z.infer<typeof TaskRecordSchema>;
|
||||
|
||||
|
||||
@ -3,7 +3,7 @@ import { TaskStore } from './store.js';
|
||||
export class TaskDeletionService {
|
||||
constructor(private readonly store: TaskStore) {}
|
||||
|
||||
deleteTaskByNameAndStatus(name: string, status: 'pending' | 'failed' | 'completed' | 'exceeded'): void {
|
||||
deleteTaskByNameAndStatus(name: string, status: 'pending' | 'failed' | 'completed' | 'exceeded' | 'pr_failed'): void {
|
||||
this.store.update((current) => {
|
||||
const exists = current.tasks.some((task) => task.name === name && task.status === status);
|
||||
if (!exists) {
|
||||
|
||||
@ -163,6 +163,37 @@ export class TaskLifecycleService {
|
||||
return this.tasksFile;
|
||||
}
|
||||
|
||||
prFailTask(result: TaskResult, prError: string): string {
|
||||
const failure: TaskFailure = {
|
||||
error: `PR creation failed: ${prError}`,
|
||||
};
|
||||
|
||||
this.store.update((current) => {
|
||||
const index = this.findActiveTaskIndex(current.tasks, result.task.name);
|
||||
if (index === -1) {
|
||||
throw new Error(`Task not found: ${result.task.name}`);
|
||||
}
|
||||
|
||||
const target = current.tasks[index]!;
|
||||
const updated: TaskRecord = {
|
||||
...target,
|
||||
status: 'pr_failed',
|
||||
started_at: result.startedAt,
|
||||
completed_at: result.completedAt,
|
||||
owner_pid: null,
|
||||
failure,
|
||||
branch: result.branch ?? target.branch,
|
||||
worktree_path: result.worktreePath ?? target.worktree_path,
|
||||
pr_url: result.prUrl ?? target.pr_url,
|
||||
};
|
||||
const tasks = [...current.tasks];
|
||||
tasks[index] = updated;
|
||||
return { tasks };
|
||||
});
|
||||
|
||||
return this.tasksFile;
|
||||
}
|
||||
|
||||
requeueFailedTask(taskRef: string, startMovement?: string, retryNote?: string): string {
|
||||
return this.requeueTask(taskRef, ['failed'], startMovement, retryNote);
|
||||
}
|
||||
|
||||
@ -60,7 +60,7 @@ export interface SummarizeOptions {
|
||||
}
|
||||
|
||||
export interface TaskListItem {
|
||||
kind: 'pending' | 'running' | 'completed' | 'failed' | 'exceeded';
|
||||
kind: 'pending' | 'running' | 'completed' | 'failed' | 'exceeded' | 'pr_failed';
|
||||
name: string;
|
||||
createdAt: string;
|
||||
filePath: string;
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user