From e70bceb4a8629d6b93757c926ef40b37b404521c Mon Sep 17 00:00:00 2001 From: nrs <38722970+nrslib@users.noreply.github.com> Date: Thu, 19 Feb 2026 23:08:17 +0900 Subject: [PATCH 01/17] takt: extend-slack-task-notification (#316) --- src/__tests__/runAllTasks-concurrency.test.ts | 82 +++++- src/__tests__/slackWebhook.test.ts | 240 +++++++++++++++++- src/__tests__/taskExecution.test.ts | 1 + src/features/tasks/execute/postExecution.ts | 10 +- src/features/tasks/execute/resolveTask.ts | 28 ++ .../tasks/execute/slackSummaryAdapter.ts | 36 +++ src/features/tasks/execute/taskExecution.ts | 70 +++-- .../tasks/execute/taskResultHandler.ts | 4 +- src/features/tasks/index.ts | 3 +- src/infra/task/mapper.ts | 1 + src/infra/task/schema.ts | 1 + src/infra/task/taskLifecycleService.ts | 1 + src/infra/task/types.ts | 2 + src/shared/utils/slackWebhook.ts | 104 ++++++++ 14 files changed, 527 insertions(+), 56 deletions(-) create mode 100644 src/features/tasks/execute/slackSummaryAdapter.ts diff --git a/src/__tests__/runAllTasks-concurrency.test.ts b/src/__tests__/runAllTasks-concurrency.test.ts index b33c98a..77e10fe 100644 --- a/src/__tests__/runAllTasks-concurrency.test.ts +++ b/src/__tests__/runAllTasks-concurrency.test.ts @@ -49,6 +49,7 @@ const { mockCompleteTask, mockFailTask, mockRecoverInterruptedRunningTasks, + mockListAllTaskItems, mockNotifySuccess, mockNotifyError, mockSendSlackNotification, @@ -58,6 +59,7 @@ const { mockCompleteTask: vi.fn(), mockFailTask: vi.fn(), mockRecoverInterruptedRunningTasks: vi.fn(), + mockListAllTaskItems: vi.fn().mockReturnValue([]), mockNotifySuccess: vi.fn(), mockNotifyError: vi.fn(), mockSendSlackNotification: vi.fn(), @@ -71,6 +73,7 @@ vi.mock('../infra/task/index.js', async (importOriginal) => ({ completeTask: mockCompleteTask, failTask: mockFailTask, recoverInterruptedRunningTasks: mockRecoverInterruptedRunningTasks, + listAllTaskItems: mockListAllTaskItems, })), })); @@ -711,16 +714,37 @@ describe('runAllTasks concurrency', () => { mockClaimNextTasks .mockReturnValueOnce([task1]) .mockReturnValueOnce([]); + mockListAllTaskItems.mockReturnValue([ + { + kind: 'completed', + name: 'task-1', + createdAt: '2026-02-19T00:00:00.000Z', + filePath: '/tasks/task-1.yaml', + content: 'Task: task-1', + startedAt: '2026-02-19T00:00:00.000Z', + completedAt: '2026-02-19T00:00:30.000Z', + branch: 'feat/task-1', + prUrl: 'https://github.com/org/repo/pull/10', + data: { task: 'task-1', piece: 'default', issue: 42 }, + }, + ]); // When await runAllTasks('/project'); // Then expect(mockSendSlackNotification).toHaveBeenCalledOnce(); - expect(mockSendSlackNotification).toHaveBeenCalledWith( - webhookUrl, - 'TAKT Run complete: 1 tasks succeeded', - ); + const [url, message] = mockSendSlackNotification.mock.calls[0]! as [string, string]; + expect(url).toBe(webhookUrl); + expect(message).toContain('TAKT Run'); + expect(message).toContain('total=1'); + expect(message).toContain('success=1'); + expect(message).toContain('failed=0'); + expect(message).toContain('task-1'); + expect(message).toContain('piece=default'); + expect(message).toContain('issue=#42'); + expect(message).toContain('duration=30s'); + expect(message).toContain('pr=https://github.com/org/repo/pull/10'); }); it('should send Slack notification on failure when webhook URL is set', async () => { @@ -731,16 +755,36 @@ describe('runAllTasks concurrency', () => { mockClaimNextTasks .mockReturnValueOnce([task1]) .mockReturnValueOnce([]); + mockListAllTaskItems.mockReturnValue([ + { + kind: 'failed', + name: 'task-1', + createdAt: '2026-02-19T00:00:00.000Z', + filePath: '/tasks/task-1.yaml', + content: 'Task: task-1', + startedAt: '2026-02-19T00:00:00.000Z', + completedAt: '2026-02-19T00:00:45.000Z', + branch: 'feat/task-1', + data: { task: 'task-1', piece: 'review' }, + failure: { movement: 'ai_review', error: 'Lint failed', last_message: 'Fix attempt timed out' }, + }, + ]); // When await runAllTasks('/project'); // Then expect(mockSendSlackNotification).toHaveBeenCalledOnce(); - expect(mockSendSlackNotification).toHaveBeenCalledWith( - webhookUrl, - 'TAKT Run finished with errors: 1 failed out of 1 tasks', - ); + const [url, message] = mockSendSlackNotification.mock.calls[0]! as [string, string]; + expect(url).toBe(webhookUrl); + expect(message).toContain('TAKT Run'); + expect(message).toContain('total=1'); + expect(message).toContain('failed=1'); + expect(message).toContain('task-1'); + expect(message).toContain('piece=review'); + expect(message).toContain('duration=45s'); + expect(message).toContain('movement=ai_review'); + expect(message).toContain('error=Lint failed'); }); it('should send Slack notification on exception when webhook URL is set', async () => { @@ -753,14 +797,28 @@ describe('runAllTasks concurrency', () => { .mockImplementationOnce(() => { throw poolError; }); + mockListAllTaskItems.mockReturnValue([ + { + kind: 'completed', + name: 'task-1', + createdAt: '2026-02-19T00:00:00.000Z', + filePath: '/tasks/task-1.yaml', + content: 'Task: task-1', + startedAt: '2026-02-19T00:00:00.000Z', + completedAt: '2026-02-19T00:00:15.000Z', + data: { task: 'task-1', piece: 'default' }, + }, + ]); // When / Then await expect(runAllTasks('/project')).rejects.toThrow('worker pool crashed'); expect(mockSendSlackNotification).toHaveBeenCalledOnce(); - expect(mockSendSlackNotification).toHaveBeenCalledWith( - webhookUrl, - 'TAKT Run error: worker pool crashed', - ); + const [url, message] = mockSendSlackNotification.mock.calls[0]! as [string, string]; + expect(url).toBe(webhookUrl); + expect(message).toContain('TAKT Run'); + expect(message).toContain('task-1'); + expect(message).toContain('piece=default'); + expect(message).toContain('duration=15s'); }); it('should not send Slack notification when webhook URL is not set', async () => { diff --git a/src/__tests__/slackWebhook.test.ts b/src/__tests__/slackWebhook.test.ts index 5946eb3..be3448b 100644 --- a/src/__tests__/slackWebhook.test.ts +++ b/src/__tests__/slackWebhook.test.ts @@ -3,7 +3,8 @@ */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { sendSlackNotification, getSlackWebhookUrl } from '../shared/utils/slackWebhook.js'; +import { sendSlackNotification, getSlackWebhookUrl, buildSlackRunSummary } from '../shared/utils/slackWebhook.js'; +import type { SlackRunSummaryParams, SlackTaskDetail } from '../shared/utils/slackWebhook.js'; describe('sendSlackNotification', () => { const webhookUrl = 'https://hooks.slack.com/services/T00/B00/xxx'; @@ -133,3 +134,240 @@ describe('getSlackWebhookUrl', () => { expect(url).toBeUndefined(); }); }); + +describe('buildSlackRunSummary', () => { + function makeTask(overrides: Partial & { name: string }): SlackTaskDetail { + return { + success: true, + piece: 'default', + durationSec: 30, + ...overrides, + }; + } + + function makeParams(overrides?: Partial): SlackRunSummaryParams { + return { + runId: 'run-20260219-105815', + total: 3, + success: 2, + failed: 1, + durationSec: 120, + concurrency: 2, + tasks: [], + ...overrides, + }; + } + + it('should include summary header with runId, counts, duration, and concurrency', () => { + // Given + const params = makeParams({ tasks: [] }); + + // When + const result = buildSlackRunSummary(params); + + // Then + expect(result).toContain('\uD83C\uDFC3 TAKT Run run-20260219-105815'); + expect(result).toContain('total=3'); + expect(result).toContain('success=2'); + expect(result).toContain('failed=1'); + expect(result).toContain('duration=120s'); + expect(result).toContain('concurrency=2'); + }); + + it('should display successful task with piece and issue', () => { + // Given + const task = makeTask({ + name: 'task-a', + piece: 'default', + issueNumber: 42, + durationSec: 30, + branch: 'feat/task-a', + worktreePath: '.worktrees/task-a', + prUrl: 'https://github.com/org/repo/pull/10', + }); + const params = makeParams({ total: 1, success: 1, failed: 0, tasks: [task] }); + + // When + const result = buildSlackRunSummary(params); + + // Then + expect(result).toContain('\u2705 task-a | piece=default | issue=#42 | duration=30s'); + expect(result).toContain('branch=feat/task-a'); + expect(result).toContain('worktree=.worktrees/task-a'); + expect(result).toContain('pr=https://github.com/org/repo/pull/10'); + }); + + it('should display failed task with error details', () => { + // Given + const task = makeTask({ + name: 'task-b', + success: false, + piece: 'review', + durationSec: 45, + branch: 'feat/task-b', + failureMovement: 'ai_review', + failureError: 'Lint failed', + failureLastMessage: 'Fix attempt timed out', + }); + const params = makeParams({ total: 1, success: 0, failed: 1, tasks: [task] }); + + // When + const result = buildSlackRunSummary(params); + + // Then + expect(result).toContain('\u274C task-b | piece=review | duration=45s'); + expect(result).toContain('movement=ai_review'); + expect(result).toContain('error=Lint failed'); + expect(result).toContain('last=Fix attempt timed out'); + expect(result).toContain('branch=feat/task-b'); + }); + + it('should omit issue when issueNumber is undefined', () => { + // Given + const task = makeTask({ name: 'task-no-issue', piece: 'default', durationSec: 10 }); + const params = makeParams({ total: 1, success: 1, failed: 0, tasks: [task] }); + + // When + const result = buildSlackRunSummary(params); + + // Then + expect(result).not.toContain('issue='); + }); + + it('should omit second line when no detail fields exist for success task', () => { + // Given + const task = makeTask({ name: 'task-minimal', piece: 'default', durationSec: 5 }); + const params = makeParams({ total: 1, success: 1, failed: 0, tasks: [task] }); + + // When + const result = buildSlackRunSummary(params); + + // Then + const taskLines = result.split('\n').filter((line) => line.includes('task-minimal')); + expect(taskLines).toHaveLength(1); + }); + + it('should preserve task submission order', () => { + // Given + const tasks = [ + makeTask({ name: 'first', durationSec: 10 }), + makeTask({ name: 'second', success: false, durationSec: 20, failureError: 'err' }), + makeTask({ name: 'third', durationSec: 30 }), + ]; + const params = makeParams({ total: 3, success: 2, failed: 1, tasks }); + + // When + const result = buildSlackRunSummary(params); + + // Then + const firstIdx = result.indexOf('first'); + const secondIdx = result.indexOf('second'); + const thirdIdx = result.indexOf('third'); + expect(firstIdx).toBeLessThan(secondIdx); + expect(secondIdx).toBeLessThan(thirdIdx); + }); + + it('should truncate and add "...and N more" when exceeding character limit', () => { + // Given + const tasks: SlackTaskDetail[] = []; + for (let i = 0; i < 50; i++) { + tasks.push(makeTask({ + name: `long-task-name-number-${String(i).padStart(3, '0')}`, + piece: 'default', + durationSec: 60, + branch: `feat/long-branch-name-for-testing-purposes-${String(i)}`, + worktreePath: `.worktrees/long-task-name-number-${String(i).padStart(3, '0')}`, + prUrl: `https://github.com/organization/repository/pull/${String(i + 100)}`, + })); + } + const params = makeParams({ total: 50, success: 50, failed: 0, tasks }); + + // When + const result = buildSlackRunSummary(params); + + // Then + expect(result.length).toBeLessThanOrEqual(3800); + expect(result).toMatch(/\.\.\.and \d+ more$/); + }); + + it('should normalize newlines in error messages', () => { + // Given + const task = makeTask({ + name: 'task-err', + success: false, + failureError: 'Line one\nLine two\r\nLine three', + }); + const params = makeParams({ total: 1, success: 0, failed: 1, tasks: [task] }); + + // When + const result = buildSlackRunSummary(params); + + // Then + expect(result).toContain('error=Line one Line two Line three'); + expect(result).not.toContain('\n error=Line one\n'); + }); + + it('should truncate long error text at 120 characters', () => { + // Given + const longError = 'A'.repeat(200); + const task = makeTask({ + name: 'task-long-err', + success: false, + failureError: longError, + }); + const params = makeParams({ total: 1, success: 0, failed: 1, tasks: [task] }); + + // When + const result = buildSlackRunSummary(params); + + // Then + expect(result).toContain(`error=${'A'.repeat(117)}...`); + expect(result).not.toContain('A'.repeat(200)); + }); + + it('should handle mixed success and failure tasks with PR present only on some', () => { + // Given + const tasks = [ + makeTask({ + name: 'with-pr', + prUrl: 'https://github.com/org/repo/pull/1', + branch: 'feat/with-pr', + }), + makeTask({ + name: 'no-pr', + branch: 'feat/no-pr', + }), + makeTask({ + name: 'failed-with-pr', + success: false, + branch: 'feat/failed', + prUrl: 'https://github.com/org/repo/pull/2', + failureError: 'build failed', + }), + ]; + const params = makeParams({ total: 3, success: 2, failed: 1, tasks }); + + // When + const result = buildSlackRunSummary(params); + + // Then + expect(result).toContain('pr=https://github.com/org/repo/pull/1'); + expect(result).toContain('pr=https://github.com/org/repo/pull/2'); + const lines = result.split('\n'); + const noPrLine = lines.find((l) => l.includes('no-pr')); + expect(noPrLine).not.toContain('pr='); + }); + + it('should handle empty tasks list', () => { + // Given + const params = makeParams({ total: 0, success: 0, failed: 0, tasks: [] }); + + // When + const result = buildSlackRunSummary(params); + + // Then + expect(result).toContain('\uD83C\uDFC3 TAKT Run'); + expect(result).toContain('total=0'); + expect(result).not.toContain('...and'); + }); +}); diff --git a/src/__tests__/taskExecution.test.ts b/src/__tests__/taskExecution.test.ts index 7e17aeb..0e26514 100644 --- a/src/__tests__/taskExecution.test.ts +++ b/src/__tests__/taskExecution.test.ts @@ -19,6 +19,7 @@ const { mockResolveTaskExecution, mockExecutePiece, mockLoadPieceByIdentifier, m vi.mock('../features/tasks/execute/resolveTask.js', () => ({ resolveTaskExecution: (...args: unknown[]) => mockResolveTaskExecution(...args), + resolveTaskIssue: vi.fn(), })); vi.mock('../features/tasks/execute/pieceExecution.js', () => ({ diff --git a/src/features/tasks/execute/postExecution.ts b/src/features/tasks/execute/postExecution.ts index 6df53cb..ce0d72d 100644 --- a/src/features/tasks/execute/postExecution.ts +++ b/src/features/tasks/execute/postExecution.ts @@ -42,10 +42,14 @@ export interface PostExecutionOptions { repo?: string; } +export interface PostExecutionResult { + prUrl?: string; +} + /** * Auto-commit, push, and optionally create a PR after successful task execution. */ -export async function postExecutionFlow(options: PostExecutionOptions): Promise { +export async function postExecutionFlow(options: PostExecutionOptions): Promise { const { execCwd, projectCwd, task, branch, baseBranch, shouldCreatePr, pieceIdentifier, issues, repo } = options; const commitResult = autoCommitAndPush(execCwd, task, projectCwd); @@ -69,6 +73,7 @@ export async function postExecutionFlow(options: PostExecutionOptions): Promise< const commentResult = commentOnPr(projectCwd, existingPr.number, commentBody); if (commentResult.success) { success(`PR updated with comment: ${existingPr.url}`); + return { prUrl: existingPr.url }; } else { error(`PR comment failed: ${commentResult.error}`); } @@ -84,9 +89,12 @@ export async function postExecutionFlow(options: PostExecutionOptions): Promise< }); if (prResult.success) { success(`PR created: ${prResult.url}`); + return { prUrl: prResult.url }; } else { error(`PR creation failed: ${prResult.error}`); } } } + + return {}; } diff --git a/src/features/tasks/execute/resolveTask.ts b/src/features/tasks/execute/resolveTask.ts index 60adb6d..7b475a3 100644 --- a/src/features/tasks/execute/resolveTask.ts +++ b/src/features/tasks/execute/resolveTask.ts @@ -6,9 +6,13 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { resolvePieceConfigValue } from '../../../infra/config/index.js'; import { type TaskInfo, createSharedClone, summarizeTaskName, getCurrentBranch } from '../../../infra/task/index.js'; +import { fetchIssue, checkGhCli } from '../../../infra/github/index.js'; import { withProgress } from '../../../shared/ui/index.js'; +import { createLogger, getErrorMessage } from '../../../shared/utils/index.js'; import { getTaskSlugFromTaskDir } from '../../../shared/utils/taskPaths.js'; +const log = createLogger('task'); + export interface ResolvedTaskExecution { execCwd: string; execPiece: string; @@ -60,6 +64,30 @@ function throwIfAborted(signal?: AbortSignal): void { } } +/** + * Resolve a GitHub issue from task data's issue number. + * Returns issue array for buildPrBody, or undefined if no issue or gh CLI unavailable. + */ +export function resolveTaskIssue(issueNumber: number | undefined): ReturnType[] | undefined { + if (issueNumber === undefined) { + return undefined; + } + + const ghStatus = checkGhCli(); + if (!ghStatus.available) { + log.info('gh CLI unavailable, skipping issue resolution for PR body', { issueNumber }); + return undefined; + } + + try { + const issue = fetchIssue(issueNumber); + return [issue]; + } catch (e) { + log.info('Failed to fetch issue for PR body, continuing without issue info', { issueNumber, error: getErrorMessage(e) }); + return undefined; + } +} + /** * Resolve execution directory and piece from task data. * If the task has worktree settings, create a shared clone and use it as cwd. diff --git a/src/features/tasks/execute/slackSummaryAdapter.ts b/src/features/tasks/execute/slackSummaryAdapter.ts new file mode 100644 index 0000000..691e642 --- /dev/null +++ b/src/features/tasks/execute/slackSummaryAdapter.ts @@ -0,0 +1,36 @@ +/** + * Adapts TaskListItem to SlackTaskDetail for Slack run summary notifications. + */ + +import type { TaskListItem } from '../../../infra/task/index.js'; +import type { SlackTaskDetail } from '../../../shared/utils/index.js'; +import { DEFAULT_PIECE_NAME } from '../../../shared/constants.js'; + +export function generateRunId(): string { + const now = new Date(); + const pad = (n: number, len: number): string => String(n).padStart(len, '0'); + return `run-${pad(now.getFullYear(), 4)}${pad(now.getMonth() + 1, 2)}${pad(now.getDate(), 2)}-${pad(now.getHours(), 2)}${pad(now.getMinutes(), 2)}${pad(now.getSeconds(), 2)}`; +} + +function computeTaskDurationSec(item: TaskListItem): number { + if (!item.startedAt || !item.completedAt) { + return 0; + } + return Math.round((new Date(item.completedAt).getTime() - new Date(item.startedAt).getTime()) / 1000); +} + +export function toSlackTaskDetail(item: TaskListItem): SlackTaskDetail { + return { + name: item.name, + success: item.kind === 'completed', + piece: item.data?.piece ?? DEFAULT_PIECE_NAME, + issueNumber: item.data?.issue, + durationSec: computeTaskDurationSec(item), + branch: item.branch, + worktreePath: item.worktreePath, + prUrl: item.prUrl, + failureMovement: item.failure?.movement, + failureError: item.failure?.error, + failureLastMessage: item.failure?.last_message, + }; +} diff --git a/src/features/tasks/execute/taskExecution.ts b/src/features/tasks/execute/taskExecution.ts index 98c7af3..1339394 100644 --- a/src/features/tasks/execute/taskExecution.ts +++ b/src/features/tasks/execute/taskExecution.ts @@ -11,45 +11,21 @@ import { status, blankLine, } from '../../../shared/ui/index.js'; -import { createLogger, getErrorMessage, getSlackWebhookUrl, notifyError, notifySuccess, sendSlackNotification } from '../../../shared/utils/index.js'; +import { createLogger, getErrorMessage, getSlackWebhookUrl, notifyError, notifySuccess, sendSlackNotification, buildSlackRunSummary } from '../../../shared/utils/index.js'; import { getLabel } from '../../../shared/i18n/index.js'; import { executePiece } from './pieceExecution.js'; import { DEFAULT_PIECE_NAME } from '../../../shared/constants.js'; import type { TaskExecutionOptions, ExecuteTaskOptions, PieceExecutionResult } from './types.js'; -import { fetchIssue, checkGhCli } from '../../../infra/github/index.js'; import { runWithWorkerPool } from './parallelExecution.js'; -import { resolveTaskExecution } from './resolveTask.js'; +import { resolveTaskExecution, resolveTaskIssue } from './resolveTask.js'; import { postExecutionFlow } from './postExecution.js'; import { buildTaskResult, persistTaskError, persistTaskResult } from './taskResultHandler.js'; +import { generateRunId, toSlackTaskDetail } from './slackSummaryAdapter.js'; export type { TaskExecutionOptions, ExecuteTaskOptions }; const log = createLogger('task'); -/** - * Resolve a GitHub issue from task data's issue number. - * Returns issue array for buildPrBody, or undefined if no issue or gh CLI unavailable. - */ -function resolveTaskIssue(issueNumber: number | undefined): ReturnType[] | undefined { - if (issueNumber === undefined) { - return undefined; - } - - const ghStatus = checkGhCli(); - if (!ghStatus.available) { - log.info('gh CLI unavailable, skipping issue resolution for PR body', { issueNumber }); - return undefined; - } - - try { - const issue = fetchIssue(issueNumber); - return [issue]; - } catch (e) { - log.info('Failed to fetch issue for PR body, continuing without issue info', { issueNumber, error: getErrorMessage(e) }); - return undefined; - } -} - async function executeTaskWithResult(options: ExecuteTaskOptions): Promise { const { task, @@ -190,9 +166,10 @@ export async function executeAndCompleteTask( const taskSuccess = taskRunResult.success; const completedAt = new Date().toISOString(); + let prUrl: string | undefined; if (taskSuccess && isWorktree) { const issues = resolveTaskIssue(issueNumber); - await postExecutionFlow({ + const postResult = await postExecutionFlow({ execCwd, projectCwd: cwd, task: task.name, @@ -202,6 +179,7 @@ export async function executeAndCompleteTask( pieceIdentifier: execPiece, issues, }); + prUrl = postResult.prUrl; } const taskResult = buildTaskResult({ @@ -211,6 +189,7 @@ export async function executeAndCompleteTask( completedAt, branch, worktreePath, + prUrl, }); persistTaskResult(taskRunner, taskResult); @@ -261,11 +240,31 @@ export async function runAllTasks( return; } + const runId = generateRunId(); + const startTime = Date.now(); + header('Running tasks'); if (concurrency > 1) { info(`Concurrency: ${concurrency}`); } + const sendSlackSummary = async (): Promise => { + if (!slackWebhookUrl) return; + const durationSec = Math.round((Date.now() - startTime) / 1000); + const tasks = taskRunner.listAllTaskItems().map(toSlackTaskDetail); + const successCount = tasks.filter((t) => t.success).length; + const message = buildSlackRunSummary({ + runId, + total: tasks.length, + success: successCount, + failed: tasks.length - successCount, + durationSec, + concurrency, + tasks, + }); + await sendSlackNotification(slackWebhookUrl, message); + }; + try { const result = await runWithWorkerPool(taskRunner, initialTasks, concurrency, cwd, pieceName, options, globalConfig.taskPollIntervalMs); @@ -279,28 +278,19 @@ export async function runAllTasks( if (shouldNotifyRunAbort) { notifyError('TAKT', getLabel('run.notifyAbort', undefined, { failed: String(result.fail) })); } - if (slackWebhookUrl) { - await sendSlackNotification(slackWebhookUrl, `TAKT Run finished with errors: ${String(result.fail)} failed out of ${String(totalCount)} tasks`); - } + await sendSlackSummary(); return; } if (shouldNotifyRunComplete) { notifySuccess('TAKT', getLabel('run.notifyComplete', undefined, { total: String(totalCount) })); } - if (slackWebhookUrl) { - await sendSlackNotification(slackWebhookUrl, `TAKT Run complete: ${String(totalCount)} tasks succeeded`); - } + await sendSlackSummary(); } catch (e) { if (shouldNotifyRunAbort) { notifyError('TAKT', getLabel('run.notifyAbort', undefined, { failed: getErrorMessage(e) })); } - if (slackWebhookUrl) { - await sendSlackNotification(slackWebhookUrl, `TAKT Run error: ${getErrorMessage(e)}`); - } + await sendSlackSummary(); throw e; } } - -// Re-export for backward compatibility with existing consumers -export { resolveTaskExecution } from './resolveTask.js'; diff --git a/src/features/tasks/execute/taskResultHandler.ts b/src/features/tasks/execute/taskResultHandler.ts index 30fcec8..5707837 100644 --- a/src/features/tasks/execute/taskResultHandler.ts +++ b/src/features/tasks/execute/taskResultHandler.ts @@ -10,6 +10,7 @@ interface BuildTaskResultParams { completedAt: string; branch?: string; worktreePath?: string; + prUrl?: string; } interface BuildBooleanTaskResultParams { @@ -33,7 +34,7 @@ interface PersistTaskErrorOptions { } export function buildTaskResult(params: BuildTaskResultParams): TaskResult { - const { task, runResult, startedAt, completedAt, branch, worktreePath } = params; + const { task, runResult, startedAt, completedAt, branch, worktreePath, prUrl } = params; const taskSuccess = runResult.success; if (!taskSuccess && !runResult.reason) { @@ -51,6 +52,7 @@ export function buildTaskResult(params: BuildTaskResultParams): TaskResult { completedAt, ...(branch ? { branch } : {}), ...(worktreePath ? { worktreePath } : {}), + ...(prUrl ? { prUrl } : {}), }; } diff --git a/src/features/tasks/index.ts b/src/features/tasks/index.ts index 41e2b5a..90ec8f8 100644 --- a/src/features/tasks/index.ts +++ b/src/features/tasks/index.ts @@ -4,7 +4,8 @@ export { executePiece, type PieceExecutionResult, type PieceExecutionOptions } from './execute/pieceExecution.js'; export { executeTask, runAllTasks, type TaskExecutionOptions } from './execute/taskExecution.js'; -export { executeAndCompleteTask, resolveTaskExecution } from './execute/taskExecution.js'; +export { executeAndCompleteTask } from './execute/taskExecution.js'; +export { resolveTaskExecution } from './execute/resolveTask.js'; export { withPersonaSession } from './execute/session.js'; export type { PipelineExecutionOptions } from './execute/types.js'; export { diff --git a/src/infra/task/mapper.ts b/src/infra/task/mapper.ts index ae561a4..58e7d45 100644 --- a/src/infra/task/mapper.ts +++ b/src/infra/task/mapper.ts @@ -120,6 +120,7 @@ function toBaseTaskListItem(projectDir: string, tasksFile: string, task: TaskRec summary: task.summary, branch: task.branch, worktreePath: task.worktree_path, + prUrl: task.pr_url, startedAt: task.started_at ?? undefined, completedAt: task.completed_at ?? undefined, ownerPid: task.owner_pid ?? undefined, diff --git a/src/infra/task/schema.ts b/src/infra/task/schema.ts index 7f9c607..3f9b9d9 100644 --- a/src/infra/task/schema.ts +++ b/src/infra/task/schema.ts @@ -44,6 +44,7 @@ export const TaskRecordSchema = TaskExecutionConfigSchema.extend({ slug: z.string().optional(), summary: z.string().optional(), worktree_path: z.string().optional(), + pr_url: z.string().optional(), content: z.string().min(1).optional(), content_file: z.string().min(1).optional(), task_dir: z.string().optional(), diff --git a/src/infra/task/taskLifecycleService.ts b/src/infra/task/taskLifecycleService.ts index c236103..8f3f4ee 100644 --- a/src/infra/task/taskLifecycleService.ts +++ b/src/infra/task/taskLifecycleService.ts @@ -121,6 +121,7 @@ export class TaskLifecycleService { failure: undefined, 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; diff --git a/src/infra/task/types.ts b/src/infra/task/types.ts index 42d0f57..573b047 100644 --- a/src/infra/task/types.ts +++ b/src/infra/task/types.ts @@ -30,6 +30,7 @@ export interface TaskResult { completedAt: string; branch?: string; worktreePath?: string; + prUrl?: string; } export interface WorktreeOptions { @@ -85,6 +86,7 @@ export interface TaskListItem { summary?: string; branch?: string; worktreePath?: string; + prUrl?: string; data?: TaskFileData; failure?: TaskFailure; startedAt?: string; diff --git a/src/shared/utils/slackWebhook.ts b/src/shared/utils/slackWebhook.ts index 6d208f9..5d7e493 100644 --- a/src/shared/utils/slackWebhook.ts +++ b/src/shared/utils/slackWebhook.ts @@ -41,3 +41,107 @@ export async function sendSlackNotification(webhookUrl: string, message: string) export function getSlackWebhookUrl(): string | undefined { return process.env[WEBHOOK_ENV_KEY]; } + +export interface SlackTaskDetail { + name: string; + success: boolean; + piece: string; + issueNumber?: number; + durationSec: number; + branch?: string; + worktreePath?: string; + prUrl?: string; + failureMovement?: string; + failureError?: string; + failureLastMessage?: string; +} + +export interface SlackRunSummaryParams { + runId: string; + total: number; + success: number; + failed: number; + durationSec: number; + concurrency: number; + tasks: SlackTaskDetail[]; +} + +const CHAR_LIMIT = 3_800; +const TRUNCATE_LENGTH = 120; + +function normalizeText(text: string): string { + return text.replace(/[\r\n]+/g, ' '); +} + +function truncateText(text: string, maxLength: number): string { + if (text.length <= maxLength) { + return text; + } + return `${text.slice(0, maxLength - 3)}...`; +} + +function formatTaskLines(task: SlackTaskDetail): string { + const icon = task.success ? '\u2705' : '\u274C'; + const parts = [ + `${icon} ${task.name}`, + `piece=${task.piece}`, + ]; + if (task.issueNumber !== undefined) { + parts.push(`issue=#${String(task.issueNumber)}`); + } + parts.push(`duration=${String(task.durationSec)}s`); + const line1 = parts.join(' | '); + + const line2Parts: string[] = []; + if (task.success) { + if (task.branch) line2Parts.push(`branch=${task.branch}`); + if (task.worktreePath) line2Parts.push(`worktree=${task.worktreePath}`); + if (task.prUrl) line2Parts.push(`pr=${task.prUrl}`); + } else { + if (task.failureMovement) line2Parts.push(`movement=${task.failureMovement}`); + if (task.failureError) { + line2Parts.push(`error=${truncateText(normalizeText(task.failureError), TRUNCATE_LENGTH)}`); + } + if (task.failureLastMessage) { + line2Parts.push(`last=${truncateText(normalizeText(task.failureLastMessage), TRUNCATE_LENGTH)}`); + } + if (task.branch) line2Parts.push(`branch=${task.branch}`); + if (task.prUrl) line2Parts.push(`pr=${task.prUrl}`); + } + + if (line2Parts.length === 0) { + return line1; + } + return `${line1}\n ${line2Parts.join(' | ')}`; +} + +export function buildSlackRunSummary(params: SlackRunSummaryParams): string { + const headerLine = `\uD83C\uDFC3 TAKT Run ${params.runId}`; + const statsLine = `total=${String(params.total)} | success=${String(params.success)} | failed=${String(params.failed)} | duration=${String(params.durationSec)}s | concurrency=${String(params.concurrency)}`; + const summaryBlock = `${headerLine}\n${statsLine}`; + + let result = summaryBlock; + let includedCount = 0; + + for (const task of params.tasks) { + const taskBlock = formatTaskLines(task); + const candidate = `${result}\n\n${taskBlock}`; + + const remaining = params.tasks.length - includedCount - 1; + const suffixLength = remaining > 0 ? `\n...and ${String(remaining)} more`.length : 0; + + if (candidate.length + suffixLength > CHAR_LIMIT) { + break; + } + + result = candidate; + includedCount++; + } + + const omitted = params.tasks.length - includedCount; + if (omitted > 0) { + result = `${result}\n...and ${String(omitted)} more`; + } + + return result; +} From 2926785c2c4e082dae8da47dd73cc9809ab6ddc2 Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Thu, 19 Feb 2026 23:24:13 +0900 Subject: [PATCH 02/17] =?UTF-8?q?fix:=20retry=E3=82=B3=E3=83=9E=E3=83=B3?= =?UTF-8?q?=E3=83=89=E3=81=AE=E6=9C=89=E5=8A=B9=E7=AF=84=E5=9B=B2=E3=81=A8?= =?UTF-8?q?=E6=A1=88=E5=86=85=E6=96=87=E3=82=92=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/__tests__/conversationLoop-resume.test.ts | 13 +++++++++++++ src/features/interactive/conversationLoop.ts | 6 ++++++ src/features/interactive/interactive.ts | 1 + src/features/interactive/retryMode.ts | 10 +++++----- src/shared/i18n/labels_en.yaml | 12 +++++++++--- src/shared/i18n/labels_ja.yaml | 12 +++++++++--- 6 files changed, 43 insertions(+), 11 deletions(-) diff --git a/src/__tests__/conversationLoop-resume.test.ts b/src/__tests__/conversationLoop-resume.test.ts index 381d2c7..5d15111 100644 --- a/src/__tests__/conversationLoop-resume.test.ts +++ b/src/__tests__/conversationLoop-resume.test.ts @@ -77,6 +77,8 @@ vi.mock('../shared/i18n/index.js', () => ({ proposed: 'Proposed:', actionPrompt: 'What next?', playNoTask: 'No task for /play', + retryNoOrder: 'No previous order found.', + retryUnavailable: '/retry is not available in this mode.', cancelled: 'Cancelled', actions: { execute: 'Execute', saveTask: 'Save', continue: 'Continue' }, })), @@ -212,4 +214,15 @@ describe('/resume command', () => { expect(capture.sessionIds[0]).toBe('resumed-session-xyz'); expect(result.action).toBe('cancel'); }); + + it('should reject /retry in non-retry mode', async () => { + setupRawStdin(toRawInputs(['/retry', '/cancel'])); + setupProvider([]); + + const ctx = createSessionContext(); + const result = await runConversationLoop('/test', ctx, defaultStrategy, undefined, undefined); + + expect(mockLogInfo).toHaveBeenCalledWith('/retry is not available in this mode.'); + expect(result.action).toBe('cancel'); + }); }); diff --git a/src/features/interactive/conversationLoop.ts b/src/features/interactive/conversationLoop.ts index b3adbd2..488bc4d 100644 --- a/src/features/interactive/conversationLoop.ts +++ b/src/features/interactive/conversationLoop.ts @@ -85,6 +85,8 @@ export interface ConversationStrategy { selectAction?: (task: string, lang: 'en' | 'ja') => Promise; /** Previous order.md content for /replay command (retry/instruct only) */ previousOrderContent?: string; + /** Enable /retry slash command (retry mode only) */ + enableRetryCommand?: boolean; } /** @@ -166,6 +168,10 @@ export async function runConversationLoop( } if (trimmed === '/retry') { + if (!strategy.enableRetryCommand) { + info(ui.retryUnavailable); + continue; + } if (!strategy.previousOrderContent) { info(ui.retryNoOrder); continue; diff --git a/src/features/interactive/interactive.ts b/src/features/interactive/interactive.ts index 398ead3..b1d8b5d 100644 --- a/src/features/interactive/interactive.ts +++ b/src/features/interactive/interactive.ts @@ -46,6 +46,7 @@ export interface InteractiveUIText { cancelled: string; playNoTask: string; retryNoOrder: string; + retryUnavailable: string; } /** diff --git a/src/features/interactive/retryMode.ts b/src/features/interactive/retryMode.ts index 317cc85..b233376 100644 --- a/src/features/interactive/retryMode.ts +++ b/src/features/interactive/retryMode.ts @@ -14,13 +14,12 @@ import { } from './conversationLoop.js'; import { createSelectActionWithoutExecute, - buildReplayHint, formatMovementPreviews, type PieceContext, } from './interactive-summary.js'; import { resolveLanguage } from './interactive.js'; import { loadTemplate } from '../../shared/prompts/index.js'; -import { getLabelObject } from '../../shared/i18n/index.js'; +import { getLabel, getLabelObject } from '../../shared/i18n/index.js'; import { resolveConfigValues } from '../../infra/config/index.js'; import type { InstructModeResult, InstructUIText } from '../tasks/list/instructMode.js'; @@ -120,10 +119,10 @@ export async function runRetryMode( const templateVars = buildRetryTemplateVars(retryContext, lang, previousOrderContent); const systemPrompt = loadTemplate('score_retry_system_prompt', ctx.lang, templateVars); - const replayHint = buildReplayHint(ctx.lang, previousOrderContent !== null); + const retryIntro = getLabel('retry.ui.intro', ctx.lang); const introLabel = ctx.lang === 'ja' - ? `## リトライ: ${retryContext.failure.taskName}\n\nブランチ: ${retryContext.branchName}\n\n${ui.intro}${replayHint}` - : `## Retry: ${retryContext.failure.taskName}\n\nBranch: ${retryContext.branchName}\n\n${ui.intro}${replayHint}`; + ? `## リトライ: ${retryContext.failure.taskName}\n\nブランチ: ${retryContext.branchName}\n\n${retryIntro}` + : `## Retry: ${retryContext.failure.taskName}\n\nBranch: ${retryContext.branchName}\n\n${retryIntro}`; const policyContent = loadTemplate('score_interactive_policy', ctx.lang, {}); @@ -144,6 +143,7 @@ export async function runRetryMode( introMessage: introLabel, selectAction: createSelectActionWithoutExecute(ui), previousOrderContent: previousOrderContent ?? undefined, + enableRetryCommand: true, }; const result = await runConversationLoop(cwd, ctx, strategy, retryContext.pieceContext, undefined); diff --git a/src/shared/i18n/labels_en.yaml b/src/shared/i18n/labels_en.yaml index 31167cc..74bb946 100644 --- a/src/shared/i18n/labels_en.yaml +++ b/src/shared/i18n/labels_en.yaml @@ -10,7 +10,7 @@ interactive: conversationLabel: "Conversation:" noTranscript: "(No local transcript. Summarize the current session context.)" ui: - intro: "Interactive mode - describe your task. Commands: /go (execute), /play (run now), /resume (load session), /retry (rerun previous order), /cancel (exit)" + intro: "Interactive mode - describe your task. Commands: /go (create instruction & run), /play (run now), /resume (load session), /cancel (exit)" resume: "Resuming previous session" noConversation: "No conversation yet. Please describe your task first." summarizeFailed: "Failed to summarize conversation. Please try again." @@ -25,6 +25,7 @@ interactive: cancelled: "Cancelled" playNoTask: "Please specify task content: /play " retryNoOrder: "No previous order (order.md) found. /retry is only available during retry." + retryUnavailable: "/retry is only available in Retry mode from `takt list`." personaFallback: "No persona available for the first movement. Falling back to assistant mode." modeSelection: prompt: "Select interactive mode:" @@ -77,10 +78,10 @@ piece: # ===== Instruct Mode UI (takt list -> instruct) ===== instruct: ui: - intro: "Instruct mode - describe additional instructions. Commands: /go (summarize), /retry (rerun previous order), /cancel (exit)" + intro: "Instruct mode - describe additional instructions. Commands: /go (create instruction & run), /replay (resubmit previous order), /cancel (exit)" resume: "Resuming previous session" noConversation: "No conversation yet. Please describe your instructions first." - summarizeFailed: "Failed to summarize conversation. Please try again." + summarizeFailed: "Failed to create instruction. Please try again." continuePrompt: "Okay, continue describing your instructions." proposed: "Proposed additional instructions:" actionPrompt: "What would you like to do?" @@ -91,6 +92,11 @@ instruct: cancelled: "Cancelled" replayNoOrder: "Previous order (order.md) not found" +# ===== Retry Mode UI (takt list -> retry) ===== +retry: + ui: + intro: "Retry mode - describe additional instructions. Commands: /go (create instruction & run), /retry (rerun previous order), /cancel (exit)" + run: notifyComplete: "Run complete ({total} tasks)" notifyAbort: "Run finished with errors ({failed})" diff --git a/src/shared/i18n/labels_ja.yaml b/src/shared/i18n/labels_ja.yaml index bf0353e..6b27047 100644 --- a/src/shared/i18n/labels_ja.yaml +++ b/src/shared/i18n/labels_ja.yaml @@ -10,7 +10,7 @@ interactive: conversationLabel: "会話:" noTranscript: "（ローカル履歴なし。現在のセッション文脈を要約してください。）" ui: - intro: "対話モード - タスク内容を入力してください。コマンド: /go（実行）, /play（即実行）, /resume（セッション読込）, /retry（前回の指示書で再実行）, /cancel（終了）" + intro: "対話モード - タスク内容を入力してください。コマンド: /go（指示書作成・実行）, /play（即実行）, /resume（セッション読込）, /cancel（終了）" resume: "前回のセッションを再開します" noConversation: "まだ会話がありません。まずタスク内容を入力してください。" summarizeFailed: "会話の要約に失敗しました。再度お試しください。" @@ -25,6 +25,7 @@ interactive: cancelled: "キャンセルしました" playNoTask: "タスク内容を指定してください: /play <タスク内容>" retryNoOrder: "前回の指示書（order.md）が見つかりません。/retry はリトライ時のみ使用できます。" + retryUnavailable: "/retry は `takt list` の Retry モードでのみ使用できます。" personaFallback: "先頭ムーブメントにペルソナがありません。アシスタントモードにフォールバックします。" modeSelection: prompt: "対話モードを選択してください:" @@ -77,10 +78,10 @@ piece: # ===== Instruct Mode UI (takt list -> instruct) ===== instruct: ui: - intro: "指示モード - 追加指示を入力してください。コマンド: /go（要約）, /retry（前回の指示書で再実行）, /cancel（終了）" + intro: "指示モード - 追加指示を入力してください。コマンド: /go（指示書作成・実行）, /replay（前回の指示書で再投入）, /cancel（終了）" resume: "前回のセッションを再開します" noConversation: "まだ会話がありません。まず追加指示を入力してください。" - summarizeFailed: "会話の要約に失敗しました。再度お試しください。" + summarizeFailed: "指示書の作成に失敗しました。再度お試しください。" continuePrompt: "続けて追加指示を入力してください。" proposed: "提案された追加指示:" actionPrompt: "どうしますか？" @@ -91,6 +92,11 @@ instruct: cancelled: "キャンセルしました" replayNoOrder: "前回の指示書（order.md）が見つかりません" +# ===== Retry Mode UI (takt list -> retry) ===== +retry: + ui: + intro: "リトライモード - 追加指示を入力してください。コマンド: /go（指示書作成・実行）, /retry（前回の指示書で再実行）, /cancel（終了）" + run: notifyComplete: "run完了 ({total} tasks)" notifyAbort: "runはエラー終了 ({failed})" From 5960a0d212ab7e6742c3d595dd34266a28ccc2e7 Mon Sep 17 00:00:00 2001 From: nrs <38722970+nrslib@users.noreply.github.com> Date: Fri, 20 Feb 2026 00:29:07 +0900 Subject: [PATCH 03/17] takt: add-all-delete-option (#322) --- src/__tests__/taskDeleteActions.test.ts | 126 ++++++++++++++++++- src/features/tasks/list/index.ts | 44 +++---- src/features/tasks/list/taskDeleteActions.ts | 54 +++++--- 3 files changed, 178 insertions(+), 46 deletions(-) diff --git a/src/__tests__/taskDeleteActions.test.ts b/src/__tests__/taskDeleteActions.test.ts index 0a81403..cca1c49 100644 --- a/src/__tests__/taskDeleteActions.test.ts +++ b/src/__tests__/taskDeleteActions.test.ts @@ -28,7 +28,7 @@ vi.mock('../features/tasks/list/taskActions.js', () => ({ import { confirm } from '../shared/prompt/index.js'; import { success, error as logError } from '../shared/ui/index.js'; -import { deletePendingTask, deleteFailedTask, deleteCompletedTask } from '../features/tasks/list/taskDeleteActions.js'; +import { deletePendingTask, deleteFailedTask, deleteCompletedTask, deleteAllTasks } from '../features/tasks/list/taskDeleteActions.js'; import type { TaskListItem } from '../infra/task/types.js'; const mockConfirm = vi.mocked(confirm); @@ -206,3 +206,127 @@ describe('taskDeleteActions', () => { expect(mockSuccess).toHaveBeenCalledWith('Deleted completed task: completed-task'); }); }); + +describe('deleteAllTasks', () => { + it('should confirm once and delete all non-running tasks', async () => { + const tasksFile = setupTasksFile(tmpDir); + const tasks: TaskListItem[] = [ + { kind: 'pending', name: 'pending-task', createdAt: '2025-01-15', filePath: tasksFile, content: 'pending' }, + { kind: 'failed', name: 'failed-task', createdAt: '2025-01-15', filePath: tasksFile, content: 'failed' }, + { kind: 'completed', name: 'completed-task', createdAt: '2025-01-15', filePath: tasksFile, content: 'completed', branch: 'takt/completed-task', worktreePath: '/tmp/takt/completed-task' }, + ]; + mockConfirm.mockResolvedValue(true); + + const result = await deleteAllTasks(tasks); + + expect(result).toBe(true); + expect(mockConfirm).toHaveBeenCalledTimes(1); + expect(mockConfirm).toHaveBeenCalledWith('Delete all 3 tasks?', false); + const raw = fs.readFileSync(tasksFile, 'utf-8'); + expect(raw).not.toContain('pending-task'); + expect(raw).not.toContain('failed-task'); + expect(raw).not.toContain('completed-task'); + expect(mockSuccess).toHaveBeenCalledWith('Deleted 3 of 3 tasks.'); + }); + + it('should skip running tasks', async () => { + const tasksFile = setupTasksFile(tmpDir); + const tasks: TaskListItem[] = [ + { kind: 'pending', name: 'pending-task', createdAt: '2025-01-15', filePath: tasksFile, content: 'pending' }, + { kind: 'running', name: 'running-task', createdAt: '2025-01-15', filePath: tasksFile, content: 'running' }, + ]; + mockConfirm.mockResolvedValue(true); + + const result = await deleteAllTasks(tasks); + + expect(result).toBe(true); + expect(mockConfirm).toHaveBeenCalledWith('Delete all 1 tasks?', false); + const raw = fs.readFileSync(tasksFile, 'utf-8'); + expect(raw).not.toContain('pending-task'); + expect(mockSuccess).toHaveBeenCalledWith('Deleted 1 of 1 tasks.'); + }); + + it('should return false when user cancels', async () => { + const tasksFile = setupTasksFile(tmpDir); + const tasks: TaskListItem[] = [ + { kind: 'pending', name: 'pending-task', createdAt: '2025-01-15', filePath: tasksFile, content: 'pending' }, + ]; + mockConfirm.mockResolvedValue(false); + + const result = await deleteAllTasks(tasks); + + expect(result).toBe(false); + const raw = fs.readFileSync(tasksFile, 'utf-8'); + expect(raw).toContain('pending-task'); + expect(mockSuccess).not.toHaveBeenCalled(); + }); + + it('should return false when no deletable tasks (only running)', async () => { + const tasks: TaskListItem[] = [ + { kind: 'running', name: 'running-task', createdAt: '2025-01-15', filePath: '/tmp/fake', content: 'running' }, + ]; + + const result = await deleteAllTasks(tasks); + + expect(result).toBe(false); + expect(mockConfirm).not.toHaveBeenCalled(); + }); + + it('should return false when no tasks', async () => { + const result = await deleteAllTasks([]); + + expect(result).toBe(false); + expect(mockConfirm).not.toHaveBeenCalled(); + }); + + it('should skip task when branch cleanup fails but continue with others', async () => { + const tasksFile = setupTasksFile(tmpDir); + const tasks: TaskListItem[] = [ + { kind: 'pending', name: 'pending-task', createdAt: '2025-01-15', filePath: tasksFile, content: 'pending' }, + { kind: 'completed', name: 'completed-task', createdAt: '2025-01-15', filePath: tasksFile, content: 'completed', branch: 'takt/completed-task', worktreePath: '/tmp/takt/completed-task' }, + ]; + mockConfirm.mockResolvedValue(true); + mockDeleteBranch.mockReturnValue(false); + + const result = await deleteAllTasks(tasks); + + expect(result).toBe(true); + const raw = fs.readFileSync(tasksFile, 'utf-8'); + expect(raw).not.toContain('pending-task'); + expect(raw).toContain('completed-task'); + expect(mockSuccess).toHaveBeenCalledWith('Deleted 1 of 2 tasks.'); + }); + + it('should return false when all tasks fail branch cleanup', async () => { + const tasksFile = setupTasksFile(tmpDir); + const tasks: TaskListItem[] = [ + { kind: 'completed', name: 'completed-task', createdAt: '2025-01-15', filePath: tasksFile, content: 'completed', branch: 'takt/completed-task', worktreePath: '/tmp/takt/completed-task' }, + ]; + mockConfirm.mockResolvedValue(true); + mockDeleteBranch.mockReturnValue(false); + + const result = await deleteAllTasks(tasks); + + expect(result).toBe(false); + expect(mockSuccess).not.toHaveBeenCalled(); + }); + + it('should cleanup branches for completed and failed tasks', async () => { + const tasksFile = setupTasksFile(tmpDir); + const completedTask: TaskListItem = { + kind: 'completed', + name: 'completed-task', + createdAt: '2025-01-15', + filePath: tasksFile, + content: 'completed', + branch: 'takt/completed-task', + worktreePath: '/tmp/takt/completed-task', + }; + const tasks: TaskListItem[] = [completedTask]; + mockConfirm.mockResolvedValue(true); + + await deleteAllTasks(tasks); + + expect(mockDeleteBranch).toHaveBeenCalledWith(tmpDir, completedTask); + }); +}); diff --git a/src/features/tasks/list/index.ts b/src/features/tasks/list/index.ts index 7367161..5ef5cbe 100644 --- a/src/features/tasks/list/index.ts +++ b/src/features/tasks/list/index.ts @@ -1,13 +1,3 @@ -/** - * List tasks command — main entry point. - * - * Interactive UI for reviewing branch-based task results, - * pending tasks (.takt/tasks.yaml), and failed tasks. - * Individual actions (merge, delete, instruct, diff) are in taskActions.ts. - * Task delete actions are in taskDeleteActions.ts. - * Non-interactive mode is in listNonInteractive.ts. - */ - import { TaskRunner, } from '../../../infra/task/index.js'; @@ -23,7 +13,7 @@ import { mergeBranch, instructBranch, } from './taskActions.js'; -import { deletePendingTask, deleteFailedTask, deleteCompletedTask } from './taskDeleteActions.js'; +import { deletePendingTask, deleteFailedTask, deleteCompletedTask, deleteAllTasks } from './taskDeleteActions.js'; import { retryFailedTask } from './taskRetryActions.js'; import { listTasksNonInteractive, type ListNonInteractiveOptions } from './listNonInteractive.js'; import { formatTaskStatusLabel, formatShortDate } from './taskStatusLabel.js'; @@ -46,17 +36,11 @@ export { runInstructMode, } from './instructMode.js'; -/** Task action type for pending task action selection menu */ type PendingTaskAction = 'delete'; -/** Task action type for failed task action selection menu */ type FailedTaskAction = 'retry' | 'delete'; type CompletedTaskAction = ListAction; -/** - * Show pending task details and prompt for an action. - * Returns the selected action, or null if cancelled. - */ async function showPendingTaskAndPromptAction(task: TaskListItem): Promise { header(formatTaskStatusLabel(task)); info(` Created: ${task.createdAt}`); @@ -71,10 +55,6 @@ async function showPendingTaskAndPromptAction(task: TaskListItem): Promise { header(formatTaskStatusLabel(task)); info(` Created: ${task.createdAt}`); @@ -103,9 +83,6 @@ async function showCompletedTaskAndPromptAction(cwd: string, task: TaskListItem) return await showDiffAndPromptActionForTask(cwd, task); } -/** - * Main entry point: list branch-based tasks interactively. - */ export async function listTasks( cwd: string, options?: TaskExecutionOptions, @@ -118,7 +95,6 @@ export async function listTasks( const runner = new TaskRunner(cwd); - // Interactive loop while (true) { const tasks = runner.listAllTaskItems(); @@ -127,11 +103,14 @@ export async function listTasks( return; } - const menuOptions = tasks.map((task, idx) => ({ - label: formatTaskStatusLabel(task), - value: `${task.kind}:${idx}`, - description: `${task.summary ?? task.content} | ${formatShortDate(task.createdAt)}`, - })); + const menuOptions = [ + ...tasks.map((task, idx) => ({ + label: formatTaskStatusLabel(task), + value: `${task.kind}:${idx}`, + description: `${task.summary ?? task.content} | ${formatShortDate(task.createdAt)}`, + })), + { label: 'All Delete', value: '__all-delete__', description: 'Delete all tasks at once' }, + ]; const selected = await selectOption( 'List Tasks', @@ -142,6 +121,11 @@ export async function listTasks( return; } + if (selected === '__all-delete__') { + await deleteAllTasks(tasks); + continue; + } + const colonIdx = selected.indexOf(':'); if (colonIdx === -1) continue; const type = selected.slice(0, colonIdx); diff --git a/src/features/tasks/list/taskDeleteActions.ts b/src/features/tasks/list/taskDeleteActions.ts index c3219fb..8ca8516 100644 --- a/src/features/tasks/list/taskDeleteActions.ts +++ b/src/features/tasks/list/taskDeleteActions.ts @@ -1,10 +1,3 @@ -/** - * Delete actions for pending and failed tasks. - * - * Provides interactive deletion (with confirm prompt) - * for pending/failed tasks in .takt/tasks.yaml. - */ - import { dirname } from 'node:path'; import type { TaskListItem } from '../../../infra/task/index.js'; import { TaskRunner } from '../../../infra/task/index.js'; @@ -27,10 +20,6 @@ function cleanupBranchIfPresent(task: TaskListItem, projectDir: string): boolean return deleteBranch(projectDir, task); } -/** - * Delete a pending task file. - * Prompts user for confirmation first. - */ export async function deletePendingTask(task: TaskListItem): Promise { const confirmed = await confirm(`Delete pending task "${task.name}"?`, false); if (!confirmed) return false; @@ -48,10 +37,6 @@ export async function deletePendingTask(task: TaskListItem): Promise { return true; } -/** - * Delete a failed task directory. - * Prompts user for confirmation first. - */ export async function deleteFailedTask(task: TaskListItem): Promise { const confirmed = await confirm(`Delete failed task "${task.name}"?`, false); if (!confirmed) return false; @@ -97,3 +82,42 @@ export async function deleteCompletedTask(task: TaskListItem): Promise log.info('Deleted completed task', { name: task.name, filePath: task.filePath }); return true; } + +export async function deleteAllTasks(tasks: TaskListItem[]): Promise { + const deletable = tasks.filter(t => t.kind !== 'running'); + if (deletable.length === 0) return false; + + const confirmed = await confirm(`Delete all ${deletable.length} tasks?`, false); + if (!confirmed) return false; + + let deletedCount = 0; + for (const task of deletable) { + const projectDir = getProjectDir(task); + try { + if (!cleanupBranchIfPresent(task, projectDir)) { + logError(`Failed to cleanup branch for task "${task.name}", skipping`); + log.error('Branch cleanup failed, skipping task', { name: task.name, kind: task.kind }); + continue; + } + const runner = new TaskRunner(projectDir); + if (task.kind === 'pending') { + runner.deletePendingTask(task.name); + } else if (task.kind === 'failed') { + runner.deleteFailedTask(task.name); + } else if (task.kind === 'completed') { + runner.deleteCompletedTask(task.name); + } + deletedCount++; + log.info('Deleted task in bulk delete', { name: task.name, kind: task.kind }); + } catch (err) { + const msg = getErrorMessage(err); + logError(`Failed to delete task "${task.name}": ${msg}`); + log.error('Failed to delete task in bulk delete', { name: task.name, kind: task.kind, error: msg }); + } + } + + if (deletedCount > 0) { + success(`Deleted ${deletedCount} of ${deletable.length} tasks.`); + } + return deletedCount > 0; +} From 4f8255d509d63174dc94d7321e27e2697413a5cc Mon Sep 17 00:00:00 2001 From: nrs <38722970+nrslib@users.noreply.github.com> Date: Fri, 20 Feb 2026 00:35:41 +0900 Subject: [PATCH 04/17] takt: add-draft-pr-option (#323) --- src/__tests__/config-env-overrides.test.ts | 9 +++ src/__tests__/github-pr.test.ts | 49 +++++++++++++- src/__tests__/pipelineExecution.test.ts | 40 ++++++++++++ src/__tests__/postExecution.test.ts | 64 ++++++++++++++++++- src/__tests__/resolveTask.test.ts | 18 ++++++ src/__tests__/saveTaskFile.test.ts | 15 ++++- src/__tests__/selectAndExecute-autoPr.test.ts | 45 +++++++++++++ src/app/cli/program.ts | 1 + src/app/cli/routing.ts | 5 ++ src/core/models/global-config.ts | 2 + src/core/models/schemas.ts | 2 + src/features/pipeline/execute.ts | 3 +- src/features/tasks/add/index.ts | 10 ++- src/features/tasks/execute/postExecution.ts | 39 ++++++++--- src/features/tasks/execute/resolveTask.ts | 12 ++-- .../tasks/execute/selectAndExecute.ts | 10 ++- src/features/tasks/execute/taskExecution.ts | 2 + src/features/tasks/execute/types.ts | 3 + src/infra/config/env/config-env-overrides.ts | 1 + src/infra/config/global/globalConfig.ts | 4 ++ src/infra/config/loadConfig.ts | 1 + src/infra/config/types.ts | 2 + src/infra/github/pr.ts | 6 +- src/infra/github/types.ts | 2 + src/infra/task/mapper.ts | 2 + src/infra/task/schema.ts | 1 + 26 files changed, 323 insertions(+), 25 deletions(-) diff --git a/src/__tests__/config-env-overrides.test.ts b/src/__tests__/config-env-overrides.test.ts index 3a2ce1a..144031f 100644 --- a/src/__tests__/config-env-overrides.test.ts +++ b/src/__tests__/config-env-overrides.test.ts @@ -42,6 +42,15 @@ describe('config env overrides', () => { }); }); + it('TAKT_DRAFT_PR が draft_pr に反映される', () => { + process.env.TAKT_DRAFT_PR = 'true'; + + const raw: Record = {}; + applyGlobalConfigEnvOverrides(raw); + + expect(raw.draft_pr).toBe(true); + }); + it('should apply project env overrides from generated env names', () => { process.env.TAKT_VERBOSE = 'true'; diff --git a/src/__tests__/github-pr.test.ts b/src/__tests__/github-pr.test.ts index 895dd76..1be0e18 100644 --- a/src/__tests__/github-pr.test.ts +++ b/src/__tests__/github-pr.test.ts @@ -26,7 +26,7 @@ vi.mock('../shared/utils/index.js', async (importOriginal) => ({ getErrorMessage: (e: unknown) => String(e), })); -import { buildPrBody, findExistingPr } from '../infra/github/pr.js'; +import { buildPrBody, findExistingPr, createPullRequest } from '../infra/github/pr.js'; import type { GitHubIssue } from '../infra/github/types.js'; describe('findExistingPr', () => { @@ -59,6 +59,53 @@ describe('findExistingPr', () => { }); }); +describe('createPullRequest', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('draft: true の場合、args に --draft が含まれる', () => { + mockExecFileSync.mockReturnValue('https://github.com/org/repo/pull/1\n'); + + createPullRequest('/project', { + branch: 'feat/my-branch', + title: 'My PR', + body: 'PR body', + draft: true, + }); + + const call = mockExecFileSync.mock.calls[0]; + expect(call[1]).toContain('--draft'); + }); + + it('draft: false の場合、args に --draft が含まれない', () => { + mockExecFileSync.mockReturnValue('https://github.com/org/repo/pull/2\n'); + + createPullRequest('/project', { + branch: 'feat/my-branch', + title: 'My PR', + body: 'PR body', + draft: false, + }); + + const call = mockExecFileSync.mock.calls[0]; + expect(call[1]).not.toContain('--draft'); + }); + + it('draft が未指定の場合、args に --draft が含まれない', () => { + mockExecFileSync.mockReturnValue('https://github.com/org/repo/pull/3\n'); + + createPullRequest('/project', { + branch: 'feat/my-branch', + title: 'My PR', + body: 'PR body', + }); + + const call = mockExecFileSync.mock.calls[0]; + expect(call[1]).not.toContain('--draft'); + }); +}); + describe('buildPrBody', () => { it('should build body with single issue and report', () => { const issue: GitHubIssue = { diff --git a/src/__tests__/pipelineExecution.test.ts b/src/__tests__/pipelineExecution.test.ts index eeb9a8e..c44937a 100644 --- a/src/__tests__/pipelineExecution.test.ts +++ b/src/__tests__/pipelineExecution.test.ts @@ -218,6 +218,46 @@ describe('executePipeline', () => { ); }); + it('draftPr: true の場合、createPullRequest に draft: true が渡される', async () => { + mockExecuteTask.mockResolvedValueOnce(true); + mockCreatePullRequest.mockReturnValueOnce({ success: true, url: 'https://github.com/test/pr/1' }); + + const exitCode = await executePipeline({ + task: 'Fix the bug', + piece: 'default', + branch: 'fix/my-branch', + autoPr: true, + draftPr: true, + cwd: '/tmp/test', + }); + + expect(exitCode).toBe(0); + expect(mockCreatePullRequest).toHaveBeenCalledWith( + '/tmp/test', + expect.objectContaining({ draft: true }), + ); + }); + + it('draftPr: false の場合、createPullRequest に draft: false が渡される', async () => { + mockExecuteTask.mockResolvedValueOnce(true); + mockCreatePullRequest.mockReturnValueOnce({ success: true, url: 'https://github.com/test/pr/1' }); + + const exitCode = await executePipeline({ + task: 'Fix the bug', + piece: 'default', + branch: 'fix/my-branch', + autoPr: true, + draftPr: false, + cwd: '/tmp/test', + }); + + expect(exitCode).toBe(0); + expect(mockCreatePullRequest).toHaveBeenCalledWith( + '/tmp/test', + expect.objectContaining({ draft: false }), + ); + }); + it('should pass baseBranch as base to createPullRequest', async () => { // Given: getCurrentBranch returns 'develop' before branch creation mockExecFileSync.mockImplementation((_cmd: string, args: string[]) => { diff --git a/src/__tests__/postExecution.test.ts b/src/__tests__/postExecution.test.ts index d1d6adf..eb6ac52 100644 --- a/src/__tests__/postExecution.test.ts +++ b/src/__tests__/postExecution.test.ts @@ -51,7 +51,12 @@ vi.mock('../shared/utils/index.js', async (importOriginal) => ({ }), })); -import { postExecutionFlow } from '../features/tasks/execute/postExecution.js'; +import { postExecutionFlow, resolveDraftPr } from '../features/tasks/execute/postExecution.js'; +import { resolvePieceConfigValue } from '../infra/config/index.js'; +import { confirm } from '../shared/prompt/index.js'; + +const mockResolvePieceConfigValue = vi.mocked(resolvePieceConfigValue); +const mockConfirm = vi.mocked(confirm); const baseOptions = { execCwd: '/clone', @@ -60,6 +65,7 @@ const baseOptions = { branch: 'task/fix-the-bug', baseBranch: 'main', shouldCreatePr: true, + draftPr: false, pieceIdentifier: 'default', }; @@ -113,4 +119,60 @@ describe('postExecutionFlow', () => { expect(mockFindExistingPr).not.toHaveBeenCalled(); expect(mockCreatePullRequest).not.toHaveBeenCalled(); }); + + it('draftPr: true の場合、createPullRequest に draft: true が渡される', async () => { + mockFindExistingPr.mockReturnValue(undefined); + + await postExecutionFlow({ ...baseOptions, draftPr: true }); + + expect(mockCreatePullRequest).toHaveBeenCalledWith( + '/project', + expect.objectContaining({ draft: true }), + ); + }); + + it('draftPr: false の場合、createPullRequest に draft: false が渡される', async () => { + mockFindExistingPr.mockReturnValue(undefined); + + await postExecutionFlow({ ...baseOptions, draftPr: false }); + + expect(mockCreatePullRequest).toHaveBeenCalledWith( + '/project', + expect.objectContaining({ draft: false }), + ); + }); +}); + +describe('resolveDraftPr', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('CLI オプション true が渡された場合は true を返す', async () => { + const result = await resolveDraftPr(true, '/project'); + expect(result).toBe(true); + }); + + it('CLI オプション false が渡された場合は false を返す', async () => { + const result = await resolveDraftPr(false, '/project'); + expect(result).toBe(false); + }); + + it('CLI オプションが未指定で config が true の場合は true を返す', async () => { + mockResolvePieceConfigValue.mockReturnValue(true); + + const result = await resolveDraftPr(undefined, '/project'); + + expect(result).toBe(true); + }); + + it('CLI オプション・config ともに未指定の場合はプロンプトを表示する', async () => { + mockResolvePieceConfigValue.mockReturnValue(undefined); + mockConfirm.mockResolvedValue(false); + + const result = await resolveDraftPr(undefined, '/project'); + + expect(mockConfirm).toHaveBeenCalledWith('Create as draft?', true); + expect(result).toBe(false); + }); }); diff --git a/src/__tests__/resolveTask.test.ts b/src/__tests__/resolveTask.test.ts index ac9c736..3f075e4 100644 --- a/src/__tests__/resolveTask.test.ts +++ b/src/__tests__/resolveTask.test.ts @@ -48,6 +48,7 @@ describe('resolveTaskExecution', () => { execPiece: 'default', isWorktree: false, autoPr: false, + draftPr: false, }); }); @@ -76,6 +77,7 @@ describe('resolveTaskExecution', () => { execPiece: 'default', isWorktree: false, autoPr: true, + draftPr: false, reportDirName: 'issue-task-123', issueNumber: 12345, taskPrompt: expect.stringContaining('Primary spec: `.takt/runs/issue-task-123/context/task/order.md`'), @@ -83,4 +85,20 @@ describe('resolveTaskExecution', () => { expect(fs.existsSync(expectedReportOrderPath)).toBe(true); expect(fs.readFileSync(expectedReportOrderPath, 'utf-8')).toBe('# task instruction'); }); + + it('draft_pr: true が draftPr: true として解決される', async () => { + const root = createTempProjectDir(); + const task = createTask({ + data: { + task: 'Run draft task', + auto_pr: true, + draft_pr: true, + }, + }); + + const result = await resolveTaskExecution(task, root, 'default'); + + expect(result.draftPr).toBe(true); + expect(result.autoPr).toBe(true); + }); }); diff --git a/src/__tests__/saveTaskFile.test.ts b/src/__tests__/saveTaskFile.test.ts index bcb33a5..1b2b3a9 100644 --- a/src/__tests__/saveTaskFile.test.ts +++ b/src/__tests__/saveTaskFile.test.ts @@ -103,6 +103,17 @@ describe('saveTaskFile', () => { expect(task.task_dir).toBeTypeOf('string'); }); + it('draftPr: true が draft_pr: true として保存される', async () => { + await saveTaskFile(testDir, 'Draft task', { + autoPr: true, + draftPr: true, + }); + + const task = loadTasks(testDir).tasks[0]!; + expect(task.auto_pr).toBe(true); + expect(task.draft_pr).toBe(true); + }); + it('should generate unique names on duplicates', async () => { const first = await saveTaskFile(testDir, 'Same title'); const second = await saveTaskFile(testDir, 'Same title'); @@ -122,7 +133,8 @@ describe('saveTaskFromInteractive', () => { it('should always save task with worktree settings', async () => { mockPromptInput.mockResolvedValueOnce(''); mockPromptInput.mockResolvedValueOnce(''); - mockConfirm.mockResolvedValueOnce(true); + mockConfirm.mockResolvedValueOnce(true); // auto-create PR? + mockConfirm.mockResolvedValueOnce(true); // create as draft? await saveTaskFromInteractive(testDir, 'Task content'); @@ -130,6 +142,7 @@ describe('saveTaskFromInteractive', () => { const task = loadTasks(testDir).tasks[0]!; expect(task.worktree).toBe(true); expect(task.auto_pr).toBe(true); + expect(task.draft_pr).toBe(true); }); it('should keep worktree enabled even when auto-pr is declined', async () => { diff --git a/src/__tests__/selectAndExecute-autoPr.test.ts b/src/__tests__/selectAndExecute-autoPr.test.ts index 3c9157d..4dd60a7 100644 --- a/src/__tests__/selectAndExecute-autoPr.test.ts +++ b/src/__tests__/selectAndExecute-autoPr.test.ts @@ -127,6 +127,50 @@ describe('resolveAutoPr default in selectAndExecuteTask', () => { expect(autoPrCall![1]).toBe(true); }); + it('shouldCreatePr=true の場合、"Create as draft?" プロンプトが表示される', async () => { + // confirm はすべての呼び出しに対して true を返す（autoPr=true → draftPr prompt） + mockConfirm.mockResolvedValue(true); + mockSummarizeTaskName.mockResolvedValue('test-task'); + mockCreateSharedClone.mockReturnValue({ + path: '/project/../clone', + branch: 'takt/test-task', + }); + mockAutoCommitAndPush.mockReturnValue({ + success: false, + message: 'no changes', + }); + + await selectAndExecuteTask('/project', 'test task', { + piece: 'default', + createWorktree: true, + }); + + const draftPrCall = mockConfirm.mock.calls.find((call) => call[0] === 'Create as draft?'); + expect(draftPrCall).toBeDefined(); + expect(draftPrCall![1]).toBe(true); + }); + + it('shouldCreatePr=false の場合、"Create as draft?" プロンプトは表示されない', async () => { + mockConfirm.mockResolvedValue(false); // autoPr=false → draft prompt skipped + mockSummarizeTaskName.mockResolvedValue('test-task'); + mockCreateSharedClone.mockReturnValue({ + path: '/project/../clone', + branch: 'takt/test-task', + }); + mockAutoCommitAndPush.mockReturnValue({ + success: false, + message: 'no changes', + }); + + await selectAndExecuteTask('/project', 'test task', { + piece: 'default', + createWorktree: true, + }); + + const draftPrCall = mockConfirm.mock.calls.find((call) => call[0] === 'Create as draft?'); + expect(draftPrCall).toBeUndefined(); + }); + it('should call selectPiece when no override is provided', async () => { mockSelectPiece.mockResolvedValue('selected-piece'); @@ -175,6 +219,7 @@ describe('resolveAutoPr default in selectAndExecuteTask', () => { branch: 'takt/test-task', worktree_path: '/project/../clone', auto_pr: true, + draft_pr: true, })); expect(mockCompleteTask).toHaveBeenCalledTimes(1); expect(mockFailTask).not.toHaveBeenCalled(); diff --git a/src/app/cli/program.ts b/src/app/cli/program.ts index 47b9614..142ba02 100644 --- a/src/app/cli/program.ts +++ b/src/app/cli/program.ts @@ -44,6 +44,7 @@ program .option('-w, --piece ', 'Piece name or path to piece file') .option('-b, --branch ', 'Branch name (auto-generated if omitted)') .option('--auto-pr', 'Create PR after successful execution') + .option('--draft', 'Create PR as draft (requires --auto-pr or auto_pr config)') .option('--repo ', 'Repository (defaults to current)') .option('--provider ', 'Override agent provider (claude|codex|opencode|mock)') .option('--model ', 'Override agent model') diff --git a/src/app/cli/routing.ts b/src/app/cli/routing.ts index f97b023..b9b0836 100644 --- a/src/app/cli/routing.ts +++ b/src/app/cli/routing.ts @@ -86,8 +86,12 @@ export async function executeDefaultAction(task?: string): Promise { const resolvedPipelineAutoPr = opts.autoPr === true ? true : (resolveConfigValue(resolvedCwd, 'autoPr') ?? false); + const resolvedPipelineDraftPr = opts.draft === true + ? true + : (resolveConfigValue(resolvedCwd, 'draftPr') ?? false); const selectOptions: SelectAndExecuteOptions = { autoPr: opts.autoPr === true ? true : undefined, + draftPr: opts.draft === true ? true : undefined, repo: opts.repo as string | undefined, piece: opts.piece as string | undefined, createWorktree: createWorktreeOverride, @@ -101,6 +105,7 @@ export async function executeDefaultAction(task?: string): Promise { piece: resolvedPipelinePiece, branch: opts.branch as string | undefined, autoPr: resolvedPipelineAutoPr, + draftPr: resolvedPipelineDraftPr, repo: opts.repo as string | undefined, skipGit: opts.skipGit === true, cwd: resolvedCwd, diff --git a/src/core/models/global-config.ts b/src/core/models/global-config.ts index 802bdd9..fb774aa 100644 --- a/src/core/models/global-config.ts +++ b/src/core/models/global-config.ts @@ -72,6 +72,8 @@ export interface GlobalConfig { worktreeDir?: string; /** Auto-create PR after worktree execution (default: prompt in interactive mode) */ autoPr?: boolean; + /** Create PR as draft (default: prompt in interactive mode when autoPr is true) */ + draftPr?: boolean; /** List of builtin piece/agent names to exclude from fallback loading */ disabledBuiltins?: string[]; /** Enable builtin pieces from builtins/{lang}/pieces */ diff --git a/src/core/models/schemas.ts b/src/core/models/schemas.ts index 0076130..f01b982 100644 --- a/src/core/models/schemas.ts +++ b/src/core/models/schemas.ts @@ -421,6 +421,8 @@ export const GlobalConfigSchema = z.object({ worktree_dir: z.string().optional(), /** Auto-create PR after worktree execution (default: prompt in interactive mode) */ auto_pr: z.boolean().optional(), + /** Create PR as draft (default: prompt in interactive mode when auto_pr is true) */ + draft_pr: z.boolean().optional(), /** List of builtin piece/agent names to exclude from fallback loading */ disabled_builtins: z.array(z.string()).optional().default([]), /** Enable builtin pieces from builtins/{lang}/pieces */ diff --git a/src/features/pipeline/execute.ts b/src/features/pipeline/execute.ts index 385e0f3..ca70ff2 100644 --- a/src/features/pipeline/execute.ts +++ b/src/features/pipeline/execute.ts @@ -105,7 +105,7 @@ function buildPipelinePrBody( * Returns a process exit code (0 on success, 2-5 on specific failures). */ export async function executePipeline(options: PipelineExecutionOptions): Promise { - const { cwd, piece, autoPr, skipGit } = options; + const { cwd, piece, autoPr, draftPr, skipGit } = options; const globalConfig = resolveConfigValues(cwd, ['pipeline']); const pipelineConfig = globalConfig.pipeline; let issue: GitHubIssue | undefined; @@ -210,6 +210,7 @@ export async function executePipeline(options: PipelineExecutionOptions): Promis body: prBody, base: baseBranch, repo: options.repo, + draft: draftPr, }); if (prResult.success) { diff --git a/src/features/tasks/add/index.ts b/src/features/tasks/add/index.ts index 107e2de..e21bbe9 100644 --- a/src/features/tasks/add/index.ts +++ b/src/features/tasks/add/index.ts @@ -37,7 +37,7 @@ function resolveUniqueTaskSlug(cwd: string, baseSlug: string): string { export async function saveTaskFile( cwd: string, taskContent: string, - options?: { piece?: string; issue?: number; worktree?: boolean | string; branch?: string; autoPr?: boolean }, + options?: { piece?: string; issue?: number; worktree?: boolean | string; branch?: string; autoPr?: boolean; draftPr?: boolean }, ): Promise<{ taskName: string; tasksFile: string }> { const runner = new TaskRunner(cwd); const slug = await summarizeTaskName(taskContent, { cwd }); @@ -54,6 +54,7 @@ export async function saveTaskFile( ...(options?.piece && { piece: options.piece }), ...(options?.issue !== undefined && { issue: options.issue }), ...(options?.autoPr !== undefined && { auto_pr: options.autoPr }), + ...(options?.draftPr !== undefined && { draft_pr: options.draftPr }), }; const created = runner.addTask(taskContent, { ...config, @@ -95,6 +96,7 @@ interface WorktreeSettings { worktree?: boolean | string; branch?: string; autoPr?: boolean; + draftPr?: boolean; } function displayTaskCreationResult( @@ -113,6 +115,9 @@ function displayTaskCreationResult( if (settings.autoPr) { info(` Auto-PR: yes`); } + if (settings.draftPr) { + info(` Draft PR: yes`); + } if (piece) info(` Piece: ${piece}`); } @@ -137,8 +142,9 @@ async function promptWorktreeSettings(): Promise { const branch = customBranch || undefined; const autoPr = await confirm('Auto-create PR?', true); + const draftPr = autoPr ? await confirm('Create as draft?', true) : false; - return { worktree, branch, autoPr }; + return { worktree, branch, autoPr, draftPr }; } /** diff --git a/src/features/tasks/execute/postExecution.ts b/src/features/tasks/execute/postExecution.ts index ce0d72d..cffb6c6 100644 --- a/src/features/tasks/execute/postExecution.ts +++ b/src/features/tasks/execute/postExecution.ts @@ -15,19 +15,38 @@ import type { GitHubIssue } from '../../../infra/github/index.js'; const log = createLogger('postExecution'); +/** + * Resolve a boolean PR option with priority: CLI option > config > prompt. + */ +async function resolvePrBooleanOption( + option: boolean | undefined, + cwd: string, + configKey: 'autoPr' | 'draftPr', + promptMessage: string, +): Promise { + if (typeof option === 'boolean') { + return option; + } + const configValue = resolvePieceConfigValue(cwd, configKey); + if (typeof configValue === 'boolean') { + return configValue; + } + return confirm(promptMessage, true); +} + /** * Resolve auto-PR setting with priority: CLI option > config > prompt. */ export async function resolveAutoPr(optionAutoPr: boolean | undefined, cwd: string): Promise { - if (typeof optionAutoPr === 'boolean') { - return optionAutoPr; - } + return resolvePrBooleanOption(optionAutoPr, cwd, 'autoPr', 'Create pull request?'); +} - const autoPr = resolvePieceConfigValue(cwd, 'autoPr'); - if (typeof autoPr === 'boolean') { - return autoPr; - } - return confirm('Create pull request?', true); +/** + * Resolve draft-PR setting with priority: CLI option > config > prompt. + * Only called when shouldCreatePr is true. + */ +export async function resolveDraftPr(optionDraftPr: boolean | undefined, cwd: string): Promise { + return resolvePrBooleanOption(optionDraftPr, cwd, 'draftPr', 'Create as draft?'); } export interface PostExecutionOptions { @@ -37,6 +56,7 @@ export interface PostExecutionOptions { branch?: string; baseBranch?: string; shouldCreatePr: boolean; + draftPr: boolean; pieceIdentifier?: string; issues?: GitHubIssue[]; repo?: string; @@ -50,7 +70,7 @@ export interface PostExecutionResult { * Auto-commit, push, and optionally create a PR after successful task execution. */ export async function postExecutionFlow(options: PostExecutionOptions): Promise { - const { execCwd, projectCwd, task, branch, baseBranch, shouldCreatePr, pieceIdentifier, issues, repo } = options; + const { execCwd, projectCwd, task, branch, baseBranch, shouldCreatePr, draftPr, pieceIdentifier, issues, repo } = options; const commitResult = autoCommitAndPush(execCwd, task, projectCwd); if (commitResult.success && commitResult.commitHash) { @@ -86,6 +106,7 @@ export async function postExecutionFlow(options: PostExecutionOptions): Promise< body: prBody, base: baseBranch, repo, + draft: draftPr, }); if (prResult.success) { success(`PR created: ${prResult.url}`); diff --git a/src/features/tasks/execute/resolveTask.ts b/src/features/tasks/execute/resolveTask.ts index 7b475a3..e94f432 100644 --- a/src/features/tasks/execute/resolveTask.ts +++ b/src/features/tasks/execute/resolveTask.ts @@ -25,6 +25,7 @@ export interface ResolvedTaskExecution { startMovement?: string; retryNote?: string; autoPr: boolean; + draftPr: boolean; issueNumber?: number; } @@ -103,7 +104,7 @@ export async function resolveTaskExecution( const data = task.data; if (!data) { - return { execCwd: defaultCwd, execPiece: defaultPiece, isWorktree: false, autoPr: false }; + return { execCwd: defaultCwd, execPiece: defaultPiece, isWorktree: false, autoPr: false, draftPr: false }; } let execCwd = defaultCwd; @@ -165,18 +166,15 @@ export async function resolveTaskExecution( const startMovement = data.start_movement; const retryNote = data.retry_note; - let autoPr: boolean; - if (data.auto_pr !== undefined) { - autoPr = data.auto_pr; - } else { - autoPr = resolvePieceConfigValue(defaultCwd, 'autoPr') ?? false; - } + const autoPr = data.auto_pr ?? resolvePieceConfigValue(defaultCwd, 'autoPr') ?? false; + const draftPr = data.draft_pr ?? resolvePieceConfigValue(defaultCwd, 'draftPr') ?? false; return { execCwd, execPiece, isWorktree, autoPr, + draftPr, ...(taskPrompt ? { taskPrompt } : {}), ...(reportDirName ? { reportDirName } : {}), ...(branch ? { branch } : {}), diff --git a/src/features/tasks/execute/selectAndExecute.ts b/src/features/tasks/execute/selectAndExecute.ts index dccfb31..ab22c09 100644 --- a/src/features/tasks/execute/selectAndExecute.ts +++ b/src/features/tasks/execute/selectAndExecute.ts @@ -16,7 +16,7 @@ import { DEFAULT_PIECE_NAME } from '../../../shared/constants.js'; import { info, error, withProgress } from '../../../shared/ui/index.js'; import { createLogger } from '../../../shared/utils/index.js'; import { executeTask } from './taskExecution.js'; -import { resolveAutoPr, postExecutionFlow } from './postExecution.js'; +import { resolveAutoPr, resolveDraftPr, postExecutionFlow } from './postExecution.js'; import type { TaskExecutionOptions, WorktreeConfirmationResult, SelectAndExecuteOptions } from './types.js'; import { selectPiece } from '../../pieceSelection/index.js'; import { buildBooleanTaskResult, persistTaskError, persistTaskResult } from './taskResultHandler.js'; @@ -100,11 +100,15 @@ export async function selectAndExecuteTask( // Ask for PR creation BEFORE execution (only if worktree is enabled) let shouldCreatePr = false; + let shouldDraftPr = false; if (isWorktree) { shouldCreatePr = await resolveAutoPr(options?.autoPr, cwd); + if (shouldCreatePr) { + shouldDraftPr = await resolveDraftPr(options?.draftPr, cwd); + } } - log.info('Starting task execution', { piece: pieceIdentifier, worktree: isWorktree, autoPr: shouldCreatePr }); + log.info('Starting task execution', { piece: pieceIdentifier, worktree: isWorktree, autoPr: shouldCreatePr, draftPr: shouldDraftPr }); const taskRunner = new TaskRunner(cwd); const taskRecord = taskRunner.addTask(task, { piece: pieceIdentifier, @@ -112,6 +116,7 @@ export async function selectAndExecuteTask( ...(branch ? { branch } : {}), ...(isWorktree ? { worktree_path: execCwd } : {}), auto_pr: shouldCreatePr, + draft_pr: shouldDraftPr, ...(taskSlug ? { slug: taskSlug } : {}), }); const startedAt = new Date().toISOString(); @@ -157,6 +162,7 @@ export async function selectAndExecuteTask( branch, baseBranch, shouldCreatePr, + draftPr: shouldDraftPr, pieceIdentifier, issues: options?.issues, repo: options?.repo, diff --git a/src/features/tasks/execute/taskExecution.ts b/src/features/tasks/execute/taskExecution.ts index 1339394..6ebdf4f 100644 --- a/src/features/tasks/execute/taskExecution.ts +++ b/src/features/tasks/execute/taskExecution.ts @@ -144,6 +144,7 @@ export async function executeAndCompleteTask( startMovement, retryNote, autoPr, + draftPr, issueNumber, } = await resolveTaskExecution(task, cwd, pieceName, taskAbortSignal); @@ -176,6 +177,7 @@ export async function executeAndCompleteTask( branch, baseBranch, shouldCreatePr: autoPr, + draftPr, pieceIdentifier: execPiece, issues, }); diff --git a/src/features/tasks/execute/types.ts b/src/features/tasks/execute/types.ts index 2636115..a89f027 100644 --- a/src/features/tasks/execute/types.ts +++ b/src/features/tasks/execute/types.ts @@ -107,6 +107,8 @@ export interface PipelineExecutionOptions { branch?: string; /** Whether to create a PR after successful execution */ autoPr: boolean; + /** Whether to create PR as draft */ + draftPr?: boolean; /** Repository in owner/repo format */ repo?: string; /** Skip branch creation, commit, and push (piece-only execution) */ @@ -127,6 +129,7 @@ export interface WorktreeConfirmationResult { export interface SelectAndExecuteOptions { autoPr?: boolean; + draftPr?: boolean; repo?: string; piece?: string; createWorktree?: boolean | undefined; diff --git a/src/infra/config/env/config-env-overrides.ts b/src/infra/config/env/config-env-overrides.ts index db9df70..528e94f 100644 --- a/src/infra/config/env/config-env-overrides.ts +++ b/src/infra/config/env/config-env-overrides.ts @@ -84,6 +84,7 @@ const GLOBAL_ENV_SPECS: readonly EnvSpec[] = [ { path: 'observability.provider_events', type: 'boolean' }, { path: 'worktree_dir', type: 'string' }, { path: 'auto_pr', type: 'boolean' }, + { path: 'draft_pr', type: 'boolean' }, { path: 'disabled_builtins', type: 'json' }, { path: 'enable_builtin_pieces', type: 'boolean' }, { path: 'anthropic_api_key', type: 'string' }, diff --git a/src/infra/config/global/globalConfig.ts b/src/infra/config/global/globalConfig.ts index b8d98fc..1d17dc8 100644 --- a/src/infra/config/global/globalConfig.ts +++ b/src/infra/config/global/globalConfig.ts @@ -171,6 +171,7 @@ export class GlobalConfigManager { } : undefined, worktreeDir: parsed.worktree_dir, autoPr: parsed.auto_pr, + draftPr: parsed.draft_pr, disabledBuiltins: parsed.disabled_builtins, enableBuiltinPieces: parsed.enable_builtin_pieces, anthropicApiKey: parsed.anthropic_api_key, @@ -242,6 +243,9 @@ export class GlobalConfigManager { if (config.autoPr !== undefined) { raw.auto_pr = config.autoPr; } + if (config.draftPr !== undefined) { + raw.draft_pr = config.draftPr; + } if (config.disabledBuiltins && config.disabledBuiltins.length > 0) { raw.disabled_builtins = config.disabledBuiltins; } diff --git a/src/infra/config/loadConfig.ts b/src/infra/config/loadConfig.ts index c907a6f..b46e4a3 100644 --- a/src/infra/config/loadConfig.ts +++ b/src/infra/config/loadConfig.ts @@ -23,6 +23,7 @@ export function loadConfig(projectDir: string): LoadedConfig { piece: project.piece ?? 'default', provider, autoPr: project.auto_pr ?? global.autoPr, + draftPr: project.draft_pr ?? global.draftPr, model: resolveModel(global, provider), verbose: resolveVerbose(project.verbose, global.verbose), providerOptions: mergeProviderOptions(global.providerOptions, project.providerOptions), diff --git a/src/infra/config/types.ts b/src/infra/config/types.ts index e5d659a..d45d7ad 100644 --- a/src/infra/config/types.ts +++ b/src/infra/config/types.ts @@ -13,6 +13,8 @@ export interface ProjectLocalConfig { provider?: 'claude' | 'codex' | 'opencode' | 'mock'; /** Auto-create PR after worktree execution */ auto_pr?: boolean; + /** Create PR as draft */ + draft_pr?: boolean; /** Verbose output mode */ verbose?: boolean; /** Provider-specific options (overrides global, overridden by piece/movement) */ diff --git a/src/infra/github/pr.ts b/src/infra/github/pr.ts index 08b1668..33e52ff 100644 --- a/src/infra/github/pr.ts +++ b/src/infra/github/pr.ts @@ -97,7 +97,11 @@ export function createPullRequest(cwd: string, options: CreatePrOptions): Create args.push('--repo', options.repo); } - log.info('Creating PR', { branch: options.branch, title: options.title }); + if (options.draft) { + args.push('--draft'); + } + + log.info('Creating PR', { branch: options.branch, title: options.title, draft: options.draft }); try { const output = execFileSync('gh', args, { diff --git a/src/infra/github/types.ts b/src/infra/github/types.ts index 07e0376..9c5da39 100644 --- a/src/infra/github/types.ts +++ b/src/infra/github/types.ts @@ -26,6 +26,8 @@ export interface CreatePrOptions { base?: string; /** Repository in owner/repo format (optional, uses current repo if omitted) */ repo?: string; + /** Create PR as draft */ + draft?: boolean; } export interface CreatePrResult { diff --git a/src/infra/task/mapper.ts b/src/infra/task/mapper.ts index 58e7d45..a781921 100644 --- a/src/infra/task/mapper.ts +++ b/src/infra/task/mapper.ts @@ -55,6 +55,7 @@ export function toTaskData(projectDir: string, task: TaskRecord): TaskFileData { start_movement: task.start_movement, retry_note: task.retry_note, auto_pr: task.auto_pr, + draft_pr: task.draft_pr, }); } @@ -78,6 +79,7 @@ export function toTaskInfo(projectDir: string, tasksFile: string, task: TaskReco start_movement: task.start_movement, retry_note: task.retry_note, auto_pr: task.auto_pr, + draft_pr: task.draft_pr, }), }; } diff --git a/src/infra/task/schema.ts b/src/infra/task/schema.ts index 3f9b9d9..9ccfdb5 100644 --- a/src/infra/task/schema.ts +++ b/src/infra/task/schema.ts @@ -17,6 +17,7 @@ export const TaskExecutionConfigSchema = z.object({ start_movement: z.string().optional(), retry_note: z.string().optional(), auto_pr: z.boolean().optional(), + draft_pr: z.boolean().optional(), }); /** From f479869d72f65e5bd2c21e18bf865c8643b7b401 Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Fri, 20 Feb 2026 07:40:59 +0900 Subject: [PATCH 05/17] =?UTF-8?q?fix:=20retry=E3=82=BF=E3=82=B9=E3=82=AF?= =?UTF-8?q?=E3=81=AEcompleted=5Fat=E3=82=AF=E3=83=AA=E3=82=A2=E6=BC=8F?= =?UTF-8?q?=E3=82=8C=E3=82=92=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit startReExecutionで失敗タスクをrunningに戻す際、 completed_atをnullにリセットしていなかったためZodバリデーションエラーが発生していた。 --- src/infra/task/taskLifecycleService.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/infra/task/taskLifecycleService.ts b/src/infra/task/taskLifecycleService.ts index 8f3f4ee..3a3ecf9 100644 --- a/src/infra/task/taskLifecycleService.ts +++ b/src/infra/task/taskLifecycleService.ts @@ -195,6 +195,7 @@ export class TaskLifecycleService { ...target, status: 'running', started_at: nowIso(), + completed_at: null, owner_pid: process.pid, failure: undefined, start_movement: startMovement, From 22901cd8cbaffa33e1364404aa643519b1415975 Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Fri, 20 Feb 2026 08:37:40 +0900 Subject: [PATCH 06/17] =?UTF-8?q?feat:=20analytics=E3=82=92project?= =?UTF-8?q?=E8=A8=AD=E5=AE=9A=E3=81=A8env=20override=E3=81=AB=E5=AF=BE?= =?UTF-8?q?=E5=BF=9C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- builtins/en/config.yaml | 4 + builtins/ja/config.yaml | 4 + src/__tests__/config-env-overrides.test.ts | 19 ++++ src/__tests__/config.test.ts | 112 +++++++++++++++++++ src/infra/config/env/config-env-overrides.ts | 8 ++ src/infra/config/loadConfig.ts | 20 ++++ src/infra/config/project/projectConfig.ts | 35 +++++- src/infra/config/types.ts | 3 + 8 files changed, 204 insertions(+), 1 deletion(-) diff --git a/builtins/en/config.yaml b/builtins/en/config.yaml index d4bee1d..729e4bf 100644 --- a/builtins/en/config.yaml +++ b/builtins/en/config.yaml @@ -29,6 +29,10 @@ concurrency: 2 # Concurrent task execution for takt run (1-10) # run_abort: true # observability: # provider_events: false # Persist provider stream events +# analytics: +# enabled: true # Enable analytics metrics collection +# events_path: ~/.takt/analytics/events # Analytics event directory +# retention_days: 30 # Analytics event retention (days) # Credentials (environment variables take priority) # anthropic_api_key: "sk-ant-..." # Claude API key diff --git a/builtins/ja/config.yaml b/builtins/ja/config.yaml index 323f511..57b596e 100644 --- a/builtins/ja/config.yaml +++ b/builtins/ja/config.yaml @@ -29,6 +29,10 @@ concurrency: 2 # takt run の同時実行数（1-10） # run_abort: true # observability: # provider_events: false # providerイベントログを記録 +# analytics: +# enabled: true # 分析メトリクスの収集を有効化 +# events_path: ~/.takt/analytics/events # 分析イベント保存先 +# retention_days: 30 # 分析イベント保持日数 # 認証情報（環境変数優先） # anthropic_api_key: "sk-ant-..." # Claude APIキー diff --git a/src/__tests__/config-env-overrides.test.ts b/src/__tests__/config-env-overrides.test.ts index 144031f..f92883a 100644 --- a/src/__tests__/config-env-overrides.test.ts +++ b/src/__tests__/config-env-overrides.test.ts @@ -53,10 +53,29 @@ describe('config env overrides', () => { it('should apply project env overrides from generated env names', () => { process.env.TAKT_VERBOSE = 'true'; + process.env.TAKT_ANALYTICS_EVENTS_PATH = '/tmp/project-analytics'; const raw: Record = {}; applyProjectConfigEnvOverrides(raw); expect(raw.verbose).toBe(true); + expect(raw.analytics).toEqual({ + events_path: '/tmp/project-analytics', + }); + }); + + it('should apply analytics env overrides for global config', () => { + process.env.TAKT_ANALYTICS_ENABLED = 'true'; + process.env.TAKT_ANALYTICS_EVENTS_PATH = '/tmp/global-analytics'; + process.env.TAKT_ANALYTICS_RETENTION_DAYS = '14'; + + const raw: Record = {}; + applyGlobalConfigEnvOverrides(raw); + + expect(raw.analytics).toEqual({ + enabled: true, + events_path: '/tmp/global-analytics', + retention_days: 14, + }); }); }); diff --git a/src/__tests__/config.test.ts b/src/__tests__/config.test.ts index 7e5284b..525ae7f 100644 --- a/src/__tests__/config.test.ts +++ b/src/__tests__/config.test.ts @@ -37,6 +37,7 @@ import { isVerboseMode, invalidateGlobalConfigCache, } from '../infra/config/index.js'; +import { loadConfig } from '../infra/config/loadConfig.js'; describe('getBuiltinPiece', () => { it('should return builtin piece when it exists in resources', () => { @@ -389,6 +390,117 @@ describe('loadProjectConfig provider_options', () => { }); }); +describe('analytics config resolution', () => { + let testDir: string; + let originalTaktConfigDir: string | undefined; + let originalAnalyticsEnabled: string | undefined; + let originalAnalyticsEventsPath: string | undefined; + let originalAnalyticsRetentionDays: string | undefined; + + beforeEach(() => { + testDir = join(tmpdir(), `takt-test-${randomUUID()}`); + mkdirSync(testDir, { recursive: true }); + originalTaktConfigDir = process.env.TAKT_CONFIG_DIR; + originalAnalyticsEnabled = process.env.TAKT_ANALYTICS_ENABLED; + originalAnalyticsEventsPath = process.env.TAKT_ANALYTICS_EVENTS_PATH; + originalAnalyticsRetentionDays = process.env.TAKT_ANALYTICS_RETENTION_DAYS; + process.env.TAKT_CONFIG_DIR = join(testDir, 'global-takt'); + delete process.env.TAKT_ANALYTICS_ENABLED; + delete process.env.TAKT_ANALYTICS_EVENTS_PATH; + delete process.env.TAKT_ANALYTICS_RETENTION_DAYS; + invalidateGlobalConfigCache(); + }); + + afterEach(() => { + if (originalTaktConfigDir === undefined) { + delete process.env.TAKT_CONFIG_DIR; + } else { + process.env.TAKT_CONFIG_DIR = originalTaktConfigDir; + } + if (originalAnalyticsEnabled === undefined) { + delete process.env.TAKT_ANALYTICS_ENABLED; + } else { + process.env.TAKT_ANALYTICS_ENABLED = originalAnalyticsEnabled; + } + if (originalAnalyticsEventsPath === undefined) { + delete process.env.TAKT_ANALYTICS_EVENTS_PATH; + } else { + process.env.TAKT_ANALYTICS_EVENTS_PATH = originalAnalyticsEventsPath; + } + if (originalAnalyticsRetentionDays === undefined) { + delete process.env.TAKT_ANALYTICS_RETENTION_DAYS; + } else { + process.env.TAKT_ANALYTICS_RETENTION_DAYS = originalAnalyticsRetentionDays; + } + invalidateGlobalConfigCache(); + + if (existsSync(testDir)) { + rmSync(testDir, { recursive: true, force: true }); + } + }); + + it('should normalize project analytics config from snake_case', () => { + const projectConfigDir = getProjectConfigDir(testDir); + mkdirSync(projectConfigDir, { recursive: true }); + writeFileSync(join(projectConfigDir, 'config.yaml'), [ + 'piece: default', + 'analytics:', + ' enabled: false', + ' events_path: .takt/project-analytics/events', + ' retention_days: 7', + ].join('\n')); + + const config = loadProjectConfig(testDir); + + expect(config.analytics).toEqual({ + enabled: false, + eventsPath: '.takt/project-analytics/events', + retentionDays: 7, + }); + }); + + it('should apply TAKT_ANALYTICS_* env overrides for project config', () => { + process.env.TAKT_ANALYTICS_ENABLED = 'true'; + process.env.TAKT_ANALYTICS_EVENTS_PATH = '/tmp/project-analytics'; + process.env.TAKT_ANALYTICS_RETENTION_DAYS = '5'; + + const config = loadProjectConfig(testDir); + expect(config.analytics).toEqual({ + enabled: true, + eventsPath: '/tmp/project-analytics', + retentionDays: 5, + }); + }); + + it('should merge analytics as project > global in loadConfig', () => { + const globalConfigDir = process.env.TAKT_CONFIG_DIR!; + mkdirSync(globalConfigDir, { recursive: true }); + writeFileSync(join(globalConfigDir, 'config.yaml'), [ + 'language: ja', + 'analytics:', + ' enabled: true', + ' events_path: /tmp/global-analytics', + ' retention_days: 30', + ].join('\n')); + + const projectConfigDir = getProjectConfigDir(testDir); + mkdirSync(projectConfigDir, { recursive: true }); + writeFileSync(join(projectConfigDir, 'config.yaml'), [ + 'piece: default', + 'analytics:', + ' events_path: /tmp/project-analytics', + ' retention_days: 14', + ].join('\n')); + + const config = loadConfig(testDir); + expect(config.analytics).toEqual({ + enabled: true, + eventsPath: '/tmp/project-analytics', + retentionDays: 14, + }); + }); +}); + describe('isVerboseMode', () => { let testDir: string; let originalTaktConfigDir: string | undefined; diff --git a/src/infra/config/env/config-env-overrides.ts b/src/infra/config/env/config-env-overrides.ts index 528e94f..166b919 100644 --- a/src/infra/config/env/config-env-overrides.ts +++ b/src/infra/config/env/config-env-overrides.ts @@ -82,6 +82,10 @@ const GLOBAL_ENV_SPECS: readonly EnvSpec[] = [ { path: 'model', type: 'string' }, { path: 'observability', type: 'json' }, { path: 'observability.provider_events', type: 'boolean' }, + { path: 'analytics', type: 'json' }, + { path: 'analytics.enabled', type: 'boolean' }, + { path: 'analytics.events_path', type: 'string' }, + { path: 'analytics.retention_days', type: 'number' }, { path: 'worktree_dir', type: 'string' }, { path: 'auto_pr', type: 'boolean' }, { path: 'draft_pr', type: 'boolean' }, @@ -126,6 +130,10 @@ const PROJECT_ENV_SPECS: readonly EnvSpec[] = [ { path: 'piece', type: 'string' }, { path: 'provider', type: 'string' }, { path: 'verbose', type: 'boolean' }, + { path: 'analytics', type: 'json' }, + { path: 'analytics.enabled', type: 'boolean' }, + { path: 'analytics.events_path', type: 'string' }, + { path: 'analytics.retention_days', type: 'number' }, { path: 'provider_options', type: 'json' }, { path: 'provider_options.codex.network_access', type: 'boolean' }, { path: 'provider_options.opencode.network_access', type: 'boolean' }, diff --git a/src/infra/config/loadConfig.ts b/src/infra/config/loadConfig.ts index b46e4a3..01f2afb 100644 --- a/src/infra/config/loadConfig.ts +++ b/src/infra/config/loadConfig.ts @@ -1,6 +1,7 @@ import type { GlobalConfig } from '../../core/models/index.js'; import type { MovementProviderOptions } from '../../core/models/piece-types.js'; import type { ProviderPermissionProfiles } from '../../core/models/provider-profiles.js'; +import type { AnalyticsConfig } from '../../core/models/global-config.js'; import { loadGlobalConfig } from './global/globalConfig.js'; import { loadProjectConfig } from './project/projectConfig.js'; import { envVarNameFromPath } from './env/config-env-overrides.js'; @@ -26,6 +27,7 @@ export function loadConfig(projectDir: string): LoadedConfig { draftPr: project.draft_pr ?? global.draftPr, model: resolveModel(global, provider), verbose: resolveVerbose(project.verbose, global.verbose), + analytics: mergeAnalytics(global.analytics, project.analytics), providerOptions: mergeProviderOptions(global.providerOptions, project.providerOptions), providerProfiles: mergeProviderProfiles(global.providerProfiles, project.providerProfiles), }; @@ -84,6 +86,24 @@ function mergeProviderOptions( return Object.keys(result).length > 0 ? result : undefined; } +function mergeAnalytics( + globalAnalytics: AnalyticsConfig | undefined, + projectAnalytics: AnalyticsConfig | undefined, +): AnalyticsConfig | undefined { + if (!globalAnalytics && !projectAnalytics) return undefined; + + const merged: AnalyticsConfig = { + enabled: projectAnalytics?.enabled ?? globalAnalytics?.enabled, + eventsPath: projectAnalytics?.eventsPath ?? globalAnalytics?.eventsPath, + retentionDays: projectAnalytics?.retentionDays ?? globalAnalytics?.retentionDays, + }; + + if (merged.enabled === undefined && merged.eventsPath === undefined && merged.retentionDays === undefined) { + return undefined; + } + return merged; +} + function mergeProviderProfiles( globalProfiles: ProviderPermissionProfiles | undefined, projectProfiles: ProviderPermissionProfiles | undefined, diff --git a/src/infra/config/project/projectConfig.ts b/src/infra/config/project/projectConfig.ts index 2671ba9..34c5bef 100644 --- a/src/infra/config/project/projectConfig.ts +++ b/src/infra/config/project/projectConfig.ts @@ -10,6 +10,7 @@ import { parse, stringify } from 'yaml'; import { copyProjectResourcesToDir } from '../../resources/index.js'; import type { ProjectLocalConfig } from '../types.js'; import type { ProviderPermissionProfiles } from '../../../core/models/provider-profiles.js'; +import type { AnalyticsConfig } from '../../../core/models/global-config.js'; import { applyProjectConfigEnvOverrides } from '../env/config-env-overrides.js'; import { normalizeProviderOptions } from '../loaders/pieceParser.js'; @@ -58,6 +59,31 @@ function denormalizeProviderProfiles(profiles: ProviderPermissionProfiles | unde }])) as Record }>; } +function normalizeAnalytics(raw: Record | undefined): AnalyticsConfig | undefined { + if (!raw) return undefined; + const enabled = typeof raw.enabled === 'boolean' ? raw.enabled : undefined; + const eventsPath = typeof raw.events_path === 'string' + ? raw.events_path + : (typeof raw.eventsPath === 'string' ? raw.eventsPath : undefined); + const retentionDays = typeof raw.retention_days === 'number' + ? raw.retention_days + : (typeof raw.retentionDays === 'number' ? raw.retentionDays : undefined); + + if (enabled === undefined && eventsPath === undefined && retentionDays === undefined) { + return undefined; + } + return { enabled, eventsPath, retentionDays }; +} + +function denormalizeAnalytics(config: AnalyticsConfig | undefined): Record | undefined { + if (!config) return undefined; + const raw: Record = {}; + if (config.enabled !== undefined) raw.enabled = config.enabled; + if (config.eventsPath) raw.events_path = config.eventsPath; + if (config.retentionDays !== undefined) raw.retention_days = config.retentionDays; + return Object.keys(raw).length > 0 ? raw : undefined; +} + /** * Load project configuration from .takt/config.yaml */ @@ -80,6 +106,7 @@ export function loadProjectConfig(projectDir: string): ProjectLocalConfig { return { ...DEFAULT_PROJECT_CONFIG, ...(parsedConfig as ProjectLocalConfig), + analytics: normalizeAnalytics(parsedConfig.analytics as Record | undefined), providerOptions: normalizeProviderOptions(parsedConfig.provider_options as { codex?: { network_access?: boolean }; opencode?: { network_access?: boolean }; @@ -109,7 +136,13 @@ export function saveProjectConfig(projectDir: string, config: ProjectLocalConfig // Copy project resources (only copies files that don't exist) copyProjectResourcesToDir(configDir); - const savePayload: ProjectLocalConfig = { ...config }; + const savePayload: Record = { ...config }; + const rawAnalytics = denormalizeAnalytics(config.analytics); + if (rawAnalytics) { + savePayload.analytics = rawAnalytics; + } else { + delete savePayload.analytics; + } const rawProfiles = denormalizeProviderProfiles(config.providerProfiles); if (rawProfiles && Object.keys(rawProfiles).length > 0) { savePayload.provider_profiles = rawProfiles; diff --git a/src/infra/config/types.ts b/src/infra/config/types.ts index d45d7ad..159f52c 100644 --- a/src/infra/config/types.ts +++ b/src/infra/config/types.ts @@ -4,6 +4,7 @@ import type { MovementProviderOptions } from '../../core/models/piece-types.js'; import type { ProviderPermissionProfiles } from '../../core/models/provider-profiles.js'; +import type { AnalyticsConfig } from '../../core/models/global-config.js'; /** Project configuration stored in .takt/config.yaml */ export interface ProjectLocalConfig { @@ -17,6 +18,8 @@ export interface ProjectLocalConfig { draft_pr?: boolean; /** Verbose output mode */ verbose?: boolean; + /** Project-level analytics overrides */ + analytics?: AnalyticsConfig; /** Provider-specific options (overrides global, overridden by piece/movement) */ provider_options?: MovementProviderOptions; /** Provider-specific options (camelCase alias) */ From dec77e069e7915857bb7b4887e67e46275bcb5e5 Mon Sep 17 00:00:00 2001 From: nrs <38722970+nrslib@users.noreply.github.com> Date: Fri, 20 Feb 2026 11:12:46 +0900 Subject: [PATCH 07/17] add-model-to-persona-providers (#324) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * takt: add-model-to-persona-providers * refactor: loadConfigを廃止しresolveConfigValueにキー単位解決を一元化 loadConfig()による一括マージを廃止し、resolveConfigValue()でキーごとに global/project/piece/envの優先順位を宣言的に解決する方式に移行。 providerOptionsの優先順位をglobal < piece < project < envに修正し、 sourceトラッキングでOptionsBuilderのマージ方向を制御する。 --- builtins/en/config.yaml | 13 +- builtins/ja/config.yaml | 13 +- docs/configuration.ja.md | 26 ++- docs/configuration.md | 26 ++- src/__tests__/addTask.test.ts | 5 + src/__tests__/catalog.test.ts | 10 - src/__tests__/config.test.ts | 8 +- .../engine-persona-providers.test.ts | 80 ++++++- src/__tests__/engine-provider-options.test.ts | 5 +- src/__tests__/global-pieceCategories.test.ts | 47 ++-- src/__tests__/globalConfig-defaults.test.ts | 90 +++++++- src/__tests__/opencode-config.test.ts | 4 +- src/__tests__/options-builder.test.ts | 5 +- .../pieceExecution-session-loading.test.ts | 2 +- src/__tests__/pipelineExecution.test.ts | 70 +++--- src/__tests__/provider-resolution.test.ts | 58 ++++- src/__tests__/runAllTasks-concurrency.test.ts | 7 + src/__tests__/summarize.test.ts | 132 +++++------ src/__tests__/taskExecution.test.ts | 15 +- src/core/models/index.ts | 1 - ...l-config.ts => persisted-global-config.ts} | 13 +- src/core/models/schemas.ts | 12 +- src/core/models/types.ts | 4 +- src/core/piece/engine/OptionsBuilder.ts | 27 ++- src/core/piece/provider-resolution.ts | 11 +- src/core/piece/types.ts | 8 +- src/features/tasks/execute/pieceExecution.ts | 6 +- src/features/tasks/execute/taskExecution.ts | 7 +- src/features/tasks/execute/types.ts | 8 +- src/index.ts | 3 - src/infra/config/global/globalConfig.ts | 36 ++- src/infra/config/loadConfig.ts | 131 ----------- src/infra/config/project/projectConfig.ts | 4 +- src/infra/config/project/resolvedSettings.ts | 31 +-- src/infra/config/resolutionCache.ts | 50 ++++ src/infra/config/resolveConfigValue.ts | 218 +++++++++++++++++- src/infra/config/resolvePieceConfigValue.ts | 9 +- src/infra/config/resolvedConfig.ts | 9 + src/infra/config/types.ts | 2 +- 39 files changed, 761 insertions(+), 445 deletions(-) rename src/core/models/{global-config.ts => persisted-global-config.ts} (93%) delete mode 100644 src/infra/config/loadConfig.ts create mode 100644 src/infra/config/resolutionCache.ts create mode 100644 src/infra/config/resolvedConfig.ts diff --git a/builtins/en/config.yaml b/builtins/en/config.yaml index 729e4bf..5beca2d 100644 --- a/builtins/en/config.yaml +++ b/builtins/en/config.yaml @@ -55,12 +55,17 @@ concurrency: 2 # Concurrent task execution for takt run (1-10) # ===================================== # Piece-related settings (global defaults) # ===================================== -# 1) Route provider per persona +# 1) Route provider/model per persona # persona_providers: -# coder: codex # Run coder persona on codex -# reviewer: claude # Run reviewer persona on claude +# coder: +# provider: codex # Run coder persona on Codex +# model: o3-mini # Use o3-mini model (optional) +# reviewer: +# provider: claude # Run reviewer persona on Claude -# 2) Provider options (global < project < piece) +# 2) Provider options +# Priority (for piece-capable keys such as provider/model/provider_options): +# global < piece < project < env # provider_options: # codex: # network_access: true # Allow network access for Codex diff --git a/builtins/ja/config.yaml b/builtins/ja/config.yaml index 57b596e..75d21bc 100644 --- a/builtins/ja/config.yaml +++ b/builtins/ja/config.yaml @@ -55,12 +55,17 @@ concurrency: 2 # takt run の同時実行数（1-10） # ===================================== # ピースにも関わる設定（global defaults） # ===================================== -# 1) ペルソナ単位でプロバイダーを切り替える +# 1) ペルソナ単位でプロバイダー・モデルを切り替える # persona_providers: -# coder: codex # coderペルソナはcodexで実行 -# reviewer: claude # reviewerペルソナはclaudeで実行 +# coder: +# provider: codex # coderペルソナはcodexで実行 +# model: o3-mini # 使用モデル（省略可） +# reviewer: +# provider: claude # reviewerペルソナはclaudeで実行 -# 2) provider 固有オプション（global < project < piece） +# 2) provider 固有オプション +# 優先順位（provider/model/provider_options 等の piece 対応キー）: +# global < piece < project < env # provider_options: # codex: # network_access: true # Codex実行時のネットワークアクセス許可 diff --git a/docs/configuration.ja.md b/docs/configuration.ja.md index 260a7ed..6c69337 100644 --- a/docs/configuration.ja.md +++ b/docs/configuration.ja.md @@ -34,11 +34,14 @@ interactive_preview_movements: 3 # インタラクティブモードでの move # - gradle # .runtime/ に Gradle キャッシュ/設定を準備 # - node # .runtime/ に npm キャッシュを準備 -# persona ごとの provider 上書き（省略可） -# piece を複製せずに特定の persona を別の provider にルーティング +# persona ごとの provider / model 上書き（省略可） +# piece を複製せずに特定の persona を別の provider / model にルーティング # persona_providers: -# coder: codex # coder を Codex で実行 -# ai-antipattern-reviewer: claude # レビュアーは Claude のまま +# coder: +# provider: codex # coder を Codex で実行 +# model: o3-mini # 使用モデル（省略可） +# ai-antipattern-reviewer: +# provider: claude # レビュアーは Claude のまま # provider 固有のパーミッションプロファイル（省略可） # 優先順位: プロジェクト上書き > グローバル上書き > プロジェクトデフォルト > グローバルデフォルト > required_permission_mode（下限） @@ -97,7 +100,7 @@ interactive_preview_movements: 3 # インタラクティブモードでの move | `verbose` | boolean | - | 詳細出力モード | | `minimal_output` | boolean | `false` | AI 出力を抑制（CI 向け） | | `runtime` | object | - | ランタイム環境デフォルト（例: `prepare: [gradle, node]`） | -| `persona_providers` | object | - | persona ごとの provider 上書き（例: `coder: codex`） | +| `persona_providers` | object | - | persona ごとの provider / model 上書き（例: `coder: { provider: codex, model: o3-mini }`） | | `provider_options` | object | - | グローバルな provider 固有オプション | | `provider_profiles` | object | - | provider 固有のパーミッションプロファイル | | `anthropic_api_key` | string | - | Claude 用 Anthropic API キー | @@ -286,16 +289,21 @@ movement の `required_permission_mode` は最低限の下限を設定します ### Persona Provider -piece を複製せずに、特定の persona を別の provider にルーティングできます。 +piece を複製せずに、特定の persona を別の provider や model にルーティングできます。 ```yaml # ~/.takt/config.yaml persona_providers: - coder: codex # coder persona を Codex で実行 - ai-antipattern-reviewer: claude # レビュアーは Claude のまま + coder: + provider: codex # coder persona を Codex で実行 + model: o3-mini # 使用モデル（省略可） + ai-antipattern-reviewer: + provider: claude # レビュアーは Claude のまま ``` -これにより、単一の piece 内で provider を混在させることができます。persona 名は movement 定義の `persona` キーに対してマッチされます。 +`provider` と `model` はいずれも省略可能です。`model` の解決優先度: movement YAML の `model` > `persona_providers[persona].model` > グローバル `model`。 + +これにより、単一の piece 内で provider や model を混在させることができます。persona 名は movement 定義の `persona` キーに対してマッチされます。 ## Piece カテゴリ diff --git a/docs/configuration.md b/docs/configuration.md index a6dbf50..7c72576 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -34,11 +34,14 @@ interactive_preview_movements: 3 # Movement previews in interactive mode (0-10, # - gradle # Prepare Gradle cache/config in .runtime/ # - node # Prepare npm cache in .runtime/ -# Per-persona provider overrides (optional) -# Route specific personas to different providers without duplicating pieces +# Per-persona provider/model overrides (optional) +# Route specific personas to different providers and models without duplicating pieces # persona_providers: -# coder: codex # Run coder on Codex -# ai-antipattern-reviewer: claude # Keep reviewers on Claude +# coder: +# provider: codex # Run coder on Codex +# model: o3-mini # Use o3-mini model (optional) +# ai-antipattern-reviewer: +# provider: claude # Keep reviewers on Claude # Provider-specific permission profiles (optional) # Priority: project override > global override > project default > global default > required_permission_mode (floor) @@ -97,7 +100,7 @@ interactive_preview_movements: 3 # Movement previews in interactive mode (0-10, | `verbose` | boolean | - | Verbose output mode | | `minimal_output` | boolean | `false` | Suppress AI output (for CI) | | `runtime` | object | - | Runtime environment defaults (e.g., `prepare: [gradle, node]`) | -| `persona_providers` | object | - | Per-persona provider overrides (e.g., `coder: codex`) | +| `persona_providers` | object | - | Per-persona provider/model overrides (e.g., `coder: { provider: codex, model: o3-mini }`) | | `provider_options` | object | - | Global provider-specific options | | `provider_profiles` | object | - | Provider-specific permission profiles | | `anthropic_api_key` | string | - | Anthropic API key for Claude | @@ -286,16 +289,21 @@ The `required_permission_mode` on a movement sets the minimum floor. If the reso ### Persona Providers -Route specific personas to different providers without duplicating pieces: +Route specific personas to different providers and models without duplicating pieces: ```yaml # ~/.takt/config.yaml persona_providers: - coder: codex # Run coder persona on Codex - ai-antipattern-reviewer: claude # Keep reviewers on Claude + coder: + provider: codex # Run coder persona on Codex + model: o3-mini # Use o3-mini model (optional) + ai-antipattern-reviewer: + provider: claude # Keep reviewers on Claude ``` -This allows mixing providers within a single piece. The persona name is matched against the `persona` key in the movement definition. +Both `provider` and `model` are optional. `model` resolution priority: movement YAML `model` > `persona_providers[persona].model` > global `model`. + +This allows mixing providers and models within a single piece. The persona name is matched against the `persona` key in the movement definition. ## Piece Categories diff --git a/src/__tests__/addTask.test.ts b/src/__tests__/addTask.test.ts index 966353e..889b362 100644 --- a/src/__tests__/addTask.test.ts +++ b/src/__tests__/addTask.test.ts @@ -34,6 +34,11 @@ vi.mock('../features/tasks/execute/selectAndExecute.js', () => ({ determinePiece: vi.fn(), })); +vi.mock('../infra/task/index.js', async (importOriginal) => ({ + ...(await importOriginal>()), + summarizeTaskName: vi.fn().mockResolvedValue('test-task'), +})); + vi.mock('../infra/github/issue.js', () => ({ isIssueReference: vi.fn((s: string) => /^#\d+$/.test(s)), resolveIssueTask: vi.fn(), diff --git a/src/__tests__/catalog.test.ts b/src/__tests__/catalog.test.ts index 514ea9d..cb41c50 100644 --- a/src/__tests__/catalog.test.ts +++ b/src/__tests__/catalog.test.ts @@ -20,16 +20,6 @@ vi.mock('../infra/config/global/globalConfig.js', () => ({ loadGlobalConfig: () => ({}), })); -vi.mock('../infra/config/loadConfig.js', () => ({ - loadConfig: () => ({ - global: { - language: 'en', - enableBuiltinPieces: true, - }, - project: {}, - }), -})); - const mockLogError = vi.fn(); const mockInfo = vi.fn(); vi.mock('../shared/ui/index.js', () => ({ diff --git a/src/__tests__/config.test.ts b/src/__tests__/config.test.ts index 525ae7f..db6b723 100644 --- a/src/__tests__/config.test.ts +++ b/src/__tests__/config.test.ts @@ -35,9 +35,9 @@ import { getLanguage, loadProjectConfig, isVerboseMode, + resolveConfigValue, invalidateGlobalConfigCache, } from '../infra/config/index.js'; -import { loadConfig } from '../infra/config/loadConfig.js'; describe('getBuiltinPiece', () => { it('should return builtin piece when it exists in resources', () => { @@ -472,7 +472,7 @@ describe('analytics config resolution', () => { }); }); - it('should merge analytics as project > global in loadConfig', () => { + it('should merge analytics as project > global in resolveConfigValue', () => { const globalConfigDir = process.env.TAKT_CONFIG_DIR!; mkdirSync(globalConfigDir, { recursive: true }); writeFileSync(join(globalConfigDir, 'config.yaml'), [ @@ -492,8 +492,8 @@ describe('analytics config resolution', () => { ' retention_days: 14', ].join('\n')); - const config = loadConfig(testDir); - expect(config.analytics).toEqual({ + const analytics = resolveConfigValue(testDir, 'analytics'); + expect(analytics).toEqual({ enabled: true, eventsPath: '/tmp/project-analytics', retentionDays: 14, diff --git a/src/__tests__/engine-persona-providers.test.ts b/src/__tests__/engine-persona-providers.test.ts index 2766186..fd61047 100644 --- a/src/__tests__/engine-persona-providers.test.ts +++ b/src/__tests__/engine-persona-providers.test.ts @@ -1,10 +1,10 @@ /** - * Tests for persona_providers config-level provider override. + * Tests for persona_providers config-level provider/model override. * - * Verifies movement-level provider resolution for stepProvider: + * Verifies movement-level provider/model resolution for stepProvider/stepModel: * 1. Movement YAML provider (highest) - * 2. persona_providers[personaDisplayName] - * 3. CLI provider (lowest) + * 2. persona_providers[personaDisplayName].provider / .model + * 3. CLI provider / model (lowest) */ import { describe, it, expect, beforeEach, vi } from 'vitest'; @@ -46,7 +46,7 @@ describe('PieceEngine persona_providers override', () => { applyDefaultMocks(); }); - it('should use persona_providers when movement has no provider and persona matches', async () => { + it('should use persona_providers.provider when movement has no provider and persona matches', async () => { const movement = makeMovement('implement', { personaDisplayName: 'coder', rules: [makeRule('done', 'COMPLETE')], @@ -66,7 +66,7 @@ describe('PieceEngine persona_providers override', () => { const engine = new PieceEngine(config, '/tmp/project', 'test task', { projectCwd: '/tmp/project', provider: 'claude', - personaProviders: { coder: 'codex' }, + personaProviders: { coder: { provider: 'codex' } }, }); await engine.run(); @@ -96,7 +96,7 @@ describe('PieceEngine persona_providers override', () => { const engine = new PieceEngine(config, '/tmp/project', 'test task', { projectCwd: '/tmp/project', provider: 'claude', - personaProviders: { coder: 'codex' }, + personaProviders: { coder: { provider: 'codex' } }, }); await engine.run(); @@ -127,7 +127,7 @@ describe('PieceEngine persona_providers override', () => { const engine = new PieceEngine(config, '/tmp/project', 'test task', { projectCwd: '/tmp/project', provider: 'mock', - personaProviders: { coder: 'codex' }, + personaProviders: { coder: { provider: 'codex' } }, }); await engine.run(); @@ -194,7 +194,7 @@ describe('PieceEngine persona_providers override', () => { const engine = new PieceEngine(config, '/tmp/project', 'test task', { projectCwd: '/tmp/project', provider: 'claude', - personaProviders: { coder: 'codex' }, + personaProviders: { coder: { provider: 'codex' } }, }); await engine.run(); @@ -207,4 +207,66 @@ describe('PieceEngine persona_providers override', () => { expect(calls[1][2].provider).toBe('claude'); expect(calls[1][2].stepProvider).toBe('codex'); }); + + it('should use persona_providers.model as stepModel when step.model is undefined', async () => { + const movement = makeMovement('implement', { + personaDisplayName: 'coder', + rules: [makeRule('done', 'COMPLETE')], + }); + const config: PieceConfig = { + name: 'persona-model-test', + movements: [movement], + initialMovement: 'implement', + maxMovements: 1, + }; + + mockRunAgentSequence([ + makeResponse({ persona: movement.persona, content: 'done' }), + ]); + mockDetectMatchedRuleSequence([{ index: 0, method: 'phase1_tag' }]); + + const engine = new PieceEngine(config, '/tmp/project', 'test task', { + projectCwd: '/tmp/project', + provider: 'claude', + model: 'global-model', + personaProviders: { coder: { provider: 'codex', model: 'o3-mini' } }, + }); + + await engine.run(); + + const options = vi.mocked(runAgent).mock.calls[0][2]; + expect(options.stepProvider).toBe('codex'); + expect(options.stepModel).toBe('o3-mini'); + }); + + it('should fallback to input.model when persona_providers.model is not set', async () => { + const movement = makeMovement('implement', { + personaDisplayName: 'coder', + rules: [makeRule('done', 'COMPLETE')], + }); + const config: PieceConfig = { + name: 'persona-model-fallback', + movements: [movement], + initialMovement: 'implement', + maxMovements: 1, + }; + + mockRunAgentSequence([ + makeResponse({ persona: movement.persona, content: 'done' }), + ]); + mockDetectMatchedRuleSequence([{ index: 0, method: 'phase1_tag' }]); + + const engine = new PieceEngine(config, '/tmp/project', 'test task', { + projectCwd: '/tmp/project', + provider: 'claude', + model: 'global-model', + personaProviders: { coder: { provider: 'codex' } }, + }); + + await engine.run(); + + const options = vi.mocked(runAgent).mock.calls[0][2]; + expect(options.stepProvider).toBe('codex'); + expect(options.stepModel).toBe('global-model'); + }); }); diff --git a/src/__tests__/engine-provider-options.test.ts b/src/__tests__/engine-provider-options.test.ts index c17b3af..e8d88ba 100644 --- a/src/__tests__/engine-provider-options.test.ts +++ b/src/__tests__/engine-provider-options.test.ts @@ -54,7 +54,7 @@ describe('PieceEngine provider_options resolution', () => { } }); - it('should merge provider_options in order: global < project < movement', async () => { + it('should merge provider_options in order: global < piece/movement < project', async () => { const movement = makeMovement('implement', { providerOptions: { codex: { networkAccess: false }, @@ -78,6 +78,7 @@ describe('PieceEngine provider_options resolution', () => { engine = new PieceEngine(config, tmpDir, 'test task', { projectCwd: tmpDir, provider: 'claude', + providerOptionsSource: 'project', providerOptions: { codex: { networkAccess: true }, claude: { sandbox: { allowUnsandboxedCommands: false } }, @@ -89,7 +90,7 @@ describe('PieceEngine provider_options resolution', () => { const options = vi.mocked(runAgent).mock.calls[0]?.[2]; expect(options?.providerOptions).toEqual({ - codex: { networkAccess: false }, + codex: { networkAccess: true }, opencode: { networkAccess: true }, claude: { sandbox: { diff --git a/src/__tests__/global-pieceCategories.test.ts b/src/__tests__/global-pieceCategories.test.ts index 642759f..a07c9cf 100644 --- a/src/__tests__/global-pieceCategories.test.ts +++ b/src/__tests__/global-pieceCategories.test.ts @@ -7,32 +7,20 @@ import { tmpdir } from 'node:os'; import { dirname, join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -const loadConfigMock = vi.hoisted(() => vi.fn()); +const resolvedState = vi.hoisted(() => ({ value: {} as Record })); vi.mock('../infra/config/paths.js', () => ({ getGlobalConfigDir: () => '/tmp/.takt', })); -vi.mock('../infra/config/loadConfig.js', () => ({ - loadConfig: loadConfigMock, -})); - vi.mock('../infra/config/resolvePieceConfigValue.js', () => ({ resolvePieceConfigValue: (_projectDir: string, key: string) => { - const loaded = loadConfigMock() as Record>; - const global = loaded?.global ?? {}; - const project = loaded?.project ?? {}; - const merged: Record = { ...global, ...project }; - return merged[key]; + return resolvedState.value[key]; }, resolvePieceConfigValues: (_projectDir: string, keys: readonly string[]) => { - const loaded = loadConfigMock() as Record>; - const global = loaded?.global ?? {}; - const project = loaded?.project ?? {}; - const merged: Record = { ...global, ...project }; const result: Record = {}; for (const key of keys) { - result[key] = merged[key]; + result[key] = resolvedState.value[key]; } return result; }, @@ -49,15 +37,12 @@ function createTempCategoriesPath(): string { describe('getPieceCategoriesPath', () => { beforeEach(() => { - loadConfigMock.mockReset(); + resolvedState.value = {}; }); it('should return configured path when pieceCategoriesFile is set', () => { // Given - loadConfigMock.mockReturnValue({ - global: { pieceCategoriesFile: '/custom/piece-categories.yaml' }, - project: {}, - }); + resolvedState.value = { pieceCategoriesFile: '/custom/piece-categories.yaml' }; // When const path = getPieceCategoriesPath(process.cwd()); @@ -68,7 +53,7 @@ describe('getPieceCategoriesPath', () => { it('should return default path when pieceCategoriesFile is not set', () => { // Given - loadConfigMock.mockReturnValue({ global: {}, project: {} }); + resolvedState.value = {}; // When const path = getPieceCategoriesPath(process.cwd()); @@ -79,9 +64,11 @@ describe('getPieceCategoriesPath', () => { it('should rethrow when global config loading fails', () => { // Given - loadConfigMock.mockImplementation(() => { - throw new Error('invalid global config'); - }); + resolvedState.value = new Proxy({}, { + get() { + throw new Error('invalid global config'); + }, + }) as Record; // When / Then expect(() => getPieceCategoriesPath(process.cwd())).toThrow('invalid global config'); @@ -92,7 +79,7 @@ describe('resetPieceCategories', () => { const tempRoots: string[] = []; beforeEach(() => { - loadConfigMock.mockReset(); + resolvedState.value = {}; }); afterEach(() => { @@ -106,10 +93,7 @@ describe('resetPieceCategories', () => { // Given const categoriesPath = createTempCategoriesPath(); tempRoots.push(dirname(dirname(categoriesPath))); - loadConfigMock.mockReturnValue({ - global: { pieceCategoriesFile: categoriesPath }, - project: {}, - }); + resolvedState.value = { pieceCategoriesFile: categoriesPath }; // When resetPieceCategories(process.cwd()); @@ -125,10 +109,7 @@ describe('resetPieceCategories', () => { const categoriesDir = dirname(categoriesPath); const tempRoot = dirname(categoriesDir); tempRoots.push(tempRoot); - loadConfigMock.mockReturnValue({ - global: { pieceCategoriesFile: categoriesPath }, - project: {}, - }); + resolvedState.value = { pieceCategoriesFile: categoriesPath }; mkdirSync(categoriesDir, { recursive: true }); writeFileSync(categoriesPath, 'piece_categories:\n old:\n - stale-piece\n', 'utf-8'); diff --git a/src/__tests__/globalConfig-defaults.test.ts b/src/__tests__/globalConfig-defaults.test.ts index d34f896..b63c5e4 100644 --- a/src/__tests__/globalConfig-defaults.test.ts +++ b/src/__tests__/globalConfig-defaults.test.ts @@ -42,7 +42,7 @@ describe('loadGlobalConfig', () => { expect(config.logLevel).toBe('info'); expect(config.provider).toBe('claude'); expect(config.model).toBeUndefined(); - expect(config.debug).toBeUndefined(); + expect(config.verbose).toBeUndefined(); expect(config.pipeline).toBeUndefined(); }); @@ -451,8 +451,11 @@ describe('loadGlobalConfig', () => { [ 'language: en', 'persona_providers:', - ' coder: codex', - ' reviewer: claude', + ' coder:', + ' provider: codex', + ' reviewer:', + ' provider: claude', + ' model: claude-3-5-sonnet-latest', ].join('\n'), 'utf-8', ); @@ -460,8 +463,29 @@ describe('loadGlobalConfig', () => { const config = loadGlobalConfig(); expect(config.personaProviders).toEqual({ - coder: 'codex', - reviewer: 'claude', + coder: { provider: 'codex' }, + reviewer: { provider: 'claude', model: 'claude-3-5-sonnet-latest' }, + }); + }); + + it('should load persona_providers with model only (no provider)', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + [ + 'language: en', + 'persona_providers:', + ' coder:', + ' model: o3-mini', + ].join('\n'), + 'utf-8', + ); + + const config = loadGlobalConfig(); + + expect(config.personaProviders).toEqual({ + coder: { model: 'o3-mini' }, }); }); @@ -471,12 +495,28 @@ describe('loadGlobalConfig', () => { writeFileSync(getGlobalConfigPath(), 'language: en\n', 'utf-8'); const config = loadGlobalConfig(); - config.personaProviders = { coder: 'codex' }; + config.personaProviders = { coder: { provider: 'codex', model: 'o3-mini' } }; saveGlobalConfig(config); invalidateGlobalConfigCache(); const reloaded = loadGlobalConfig(); - expect(reloaded.personaProviders).toEqual({ coder: 'codex' }); + expect(reloaded.personaProviders).toEqual({ coder: { provider: 'codex', model: 'o3-mini' } }); + }); + + it('should normalize legacy string format to object format', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + 'language: en\npersona_providers:\n coder: codex\n', + 'utf-8', + ); + + const config = loadGlobalConfig(); + + expect(config.personaProviders).toEqual({ + coder: { provider: 'codex' }, + }); }); it('should have undefined personaProviders by default', () => { @@ -497,6 +537,42 @@ describe('loadGlobalConfig', () => { const reloaded = loadGlobalConfig(); expect(reloaded.personaProviders).toBeUndefined(); }); + + it('should throw when persona entry has codex provider with Claude model alias', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + 'language: en\npersona_providers:\n coder:\n provider: codex\n model: opus\n', + 'utf-8', + ); + + expect(() => loadGlobalConfig()).toThrow(/Claude model alias/); + }); + + it('should throw when persona entry has opencode provider without model', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + 'language: en\npersona_providers:\n reviewer:\n provider: opencode\n', + 'utf-8', + ); + + expect(() => loadGlobalConfig()).toThrow(/requires model/); + }); + + it('should not throw when persona entry has opencode provider with compatible model', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + 'language: en\npersona_providers:\n coder:\n provider: opencode\n model: opencode/big-pickle\n', + 'utf-8', + ); + + expect(() => loadGlobalConfig()).not.toThrow(); + }); }); describe('runtime', () => { diff --git a/src/__tests__/opencode-config.test.ts b/src/__tests__/opencode-config.test.ts index 6e181f6..31582ab 100644 --- a/src/__tests__/opencode-config.test.ts +++ b/src/__tests__/opencode-config.test.ts @@ -19,9 +19,9 @@ describe('Schemas accept opencode provider', () => { it('should accept opencode in GlobalConfigSchema persona_providers field', () => { const result = GlobalConfigSchema.parse({ - persona_providers: { coder: 'opencode' }, + persona_providers: { coder: { provider: 'opencode' } }, }); - expect(result.persona_providers).toEqual({ coder: 'opencode' }); + expect(result.persona_providers).toEqual({ coder: { provider: 'opencode' } }); }); it('should accept opencode_api_key in GlobalConfigSchema', () => { diff --git a/src/__tests__/options-builder.test.ts b/src/__tests__/options-builder.test.ts index c9e2998..5f14d37 100644 --- a/src/__tests__/options-builder.test.ts +++ b/src/__tests__/options-builder.test.ts @@ -68,7 +68,7 @@ describe('OptionsBuilder.buildBaseOptions', () => { expect(options.permissionMode).toBe('edit'); }); - it('merges provider options with precedence: global < project < movement', () => { + it('merges provider options with precedence: global < movement < project', () => { const step = createMovement({ providerOptions: { codex: { networkAccess: false }, @@ -76,6 +76,7 @@ describe('OptionsBuilder.buildBaseOptions', () => { }, }); const builder = createBuilder(step, { + providerOptionsSource: 'project', providerOptions: { codex: { networkAccess: true }, claude: { sandbox: { allowUnsandboxedCommands: true } }, @@ -86,7 +87,7 @@ describe('OptionsBuilder.buildBaseOptions', () => { const options = builder.buildBaseOptions(step); expect(options.providerOptions).toEqual({ - codex: { networkAccess: false }, + codex: { networkAccess: true }, opencode: { networkAccess: true }, claude: { sandbox: { diff --git a/src/__tests__/pieceExecution-session-loading.test.ts b/src/__tests__/pieceExecution-session-loading.test.ts index daae53d..4ce1699 100644 --- a/src/__tests__/pieceExecution-session-loading.test.ts +++ b/src/__tests__/pieceExecution-session-loading.test.ts @@ -248,7 +248,7 @@ describe('executePiece session loading', () => { projectCwd: '/tmp/project', provider: 'codex', model: 'gpt-5', - personaProviders: { coder: 'opencode' }, + personaProviders: { coder: { provider: 'opencode' } }, }); const mockInfo = vi.mocked(info); diff --git a/src/__tests__/pipelineExecution.test.ts b/src/__tests__/pipelineExecution.test.ts index c44937a..373c501 100644 --- a/src/__tests__/pipelineExecution.test.ts +++ b/src/__tests__/pipelineExecution.test.ts @@ -31,11 +31,10 @@ vi.mock('../features/tasks/index.js', () => ({ executeTask: mockExecuteTask, })); -// Mock loadGlobalConfig -const mockLoadGlobalConfig = vi.fn(); -vi.mock('../infra/config/global/globalConfig.js', async (importOriginal) => ({ ...(await importOriginal>()), - loadGlobalConfig: mockLoadGlobalConfig, -})); +const mockResolveConfigValues = vi.fn(); +vi.mock('../infra/config/index.js', () => ({ + resolveConfigValues: mockResolveConfigValues, +})); // Mock execFileSync for git operations const mockExecFileSync = vi.fn(); @@ -68,18 +67,13 @@ vi.mock('../shared/utils/index.js', async (importOriginal) => ({ const { executePipeline } = await import('../features/pipeline/index.js'); describe('executePipeline', () => { - beforeEach(() => { - vi.clearAllMocks(); - // Default: git operations succeed - mockExecFileSync.mockReturnValue('abc1234\n'); - // Default: no pipeline config - mockLoadGlobalConfig.mockReturnValue({ - language: 'en', - defaultPiece: 'default', - logLevel: 'info', - provider: 'claude', - }); - }); + beforeEach(() => { + vi.clearAllMocks(); + // Default: git operations succeed + mockExecFileSync.mockReturnValue('abc1234\n'); + // Default: no pipeline config + mockResolveConfigValues.mockReturnValue({ pipeline: undefined }); + }); it('should return exit code 2 when neither --issue nor --task is specified', async () => { const exitCode = await executePipeline({ @@ -311,15 +305,11 @@ describe('executePipeline', () => { describe('PipelineConfig template expansion', () => { it('should use commit_message_template when configured', async () => { - mockLoadGlobalConfig.mockReturnValue({ - language: 'en', - defaultPiece: 'default', - logLevel: 'info', - provider: 'claude', - pipeline: { - commitMessageTemplate: 'fix: {title} (#{issue})', - }, - }); + mockResolveConfigValues.mockReturnValue({ + pipeline: { + commitMessageTemplate: 'fix: {title} (#{issue})', + }, + }); mockFetchIssue.mockReturnValueOnce({ number: 42, @@ -347,15 +337,11 @@ describe('executePipeline', () => { }); it('should use default_branch_prefix when configured', async () => { - mockLoadGlobalConfig.mockReturnValue({ - language: 'en', - defaultPiece: 'default', - logLevel: 'info', - provider: 'claude', - pipeline: { - defaultBranchPrefix: 'feat/', - }, - }); + mockResolveConfigValues.mockReturnValue({ + pipeline: { + defaultBranchPrefix: 'feat/', + }, + }); mockFetchIssue.mockReturnValueOnce({ number: 10, @@ -383,15 +369,11 @@ describe('executePipeline', () => { }); it('should use pr_body_template when configured for PR creation', async () => { - mockLoadGlobalConfig.mockReturnValue({ - language: 'en', - defaultPiece: 'default', - logLevel: 'info', - provider: 'claude', - pipeline: { - prBodyTemplate: '## Summary\n{issue_body}\n\nCloses #{issue}', - }, - }); + mockResolveConfigValues.mockReturnValue({ + pipeline: { + prBodyTemplate: '## Summary\n{issue_body}\n\nCloses #{issue}', + }, + }); mockFetchIssue.mockReturnValueOnce({ number: 50, diff --git a/src/__tests__/provider-resolution.test.ts b/src/__tests__/provider-resolution.test.ts index fa60189..bec21ac 100644 --- a/src/__tests__/provider-resolution.test.ts +++ b/src/__tests__/provider-resolution.test.ts @@ -7,7 +7,7 @@ describe('resolveMovementProviderModel', () => { const result = resolveMovementProviderModel({ step: { provider: 'codex', model: undefined, personaDisplayName: 'coder' }, provider: 'claude', - personaProviders: { coder: 'opencode' }, + personaProviders: { coder: { provider: 'opencode' } }, }); // When: provider/model を解決する @@ -15,16 +15,16 @@ describe('resolveMovementProviderModel', () => { expect(result.provider).toBe('codex'); }); - it('should use personaProviders when step.provider is undefined', () => { + it('should use personaProviders.provider when step.provider is undefined', () => { // Given: step.provider が未定義で personaProviders に対応がある const result = resolveMovementProviderModel({ step: { provider: undefined, model: undefined, personaDisplayName: 'reviewer' }, provider: 'claude', - personaProviders: { reviewer: 'opencode' }, + personaProviders: { reviewer: { provider: 'opencode' } }, }); // When: provider/model を解決する - // Then: personaProviders の値が使われる + // Then: personaProviders の provider が使われる expect(result.provider).toBe('opencode'); }); @@ -33,7 +33,7 @@ describe('resolveMovementProviderModel', () => { const result = resolveMovementProviderModel({ step: { provider: undefined, model: undefined, personaDisplayName: 'unknown' }, provider: 'mock', - personaProviders: { reviewer: 'codex' }, + personaProviders: { reviewer: { provider: 'codex' } }, }); // When: provider/model を解決する @@ -54,11 +54,12 @@ describe('resolveMovementProviderModel', () => { expect(result.provider).toBeUndefined(); }); - it('should prefer step.model over input.model', () => { - // Given: step.model と input.model が両方指定されている + it('should prefer step.model over personaProviders.model and input.model', () => { + // Given: step.model と personaProviders.model と input.model が指定されている const result = resolveMovementProviderModel({ step: { provider: undefined, model: 'step-model', personaDisplayName: 'coder' }, model: 'input-model', + personaProviders: { coder: { provider: 'codex', model: 'persona-model' } }, }); // When: provider/model を解決する @@ -66,15 +67,54 @@ describe('resolveMovementProviderModel', () => { expect(result.model).toBe('step-model'); }); - it('should fallback to input.model when step.model is undefined', () => { - // Given: step.model が未定義で input.model が指定されている + it('should use personaProviders.model when step.model is undefined', () => { + // Given: step.model が未定義で personaProviders.model が指定されている const result = resolveMovementProviderModel({ step: { provider: undefined, model: undefined, personaDisplayName: 'coder' }, model: 'input-model', + personaProviders: { coder: { provider: 'codex', model: 'persona-model' } }, + }); + + // When: provider/model を解決する + // Then: personaProviders.model が使われる + expect(result.model).toBe('persona-model'); + }); + + it('should fallback to input.model when step.model and personaProviders.model are undefined', () => { + // Given: step.model と personaProviders.model が未定義で input.model が指定されている + const result = resolveMovementProviderModel({ + step: { provider: undefined, model: undefined, personaDisplayName: 'coder' }, + model: 'input-model', + personaProviders: { coder: { provider: 'codex' } }, }); // When: provider/model を解決する // Then: input.model が使われる expect(result.model).toBe('input-model'); }); + + it('should return undefined model when all model candidates are missing', () => { + // Given: model の候補がすべて未定義 + const result = resolveMovementProviderModel({ + step: { provider: undefined, model: undefined, personaDisplayName: 'coder' }, + model: undefined, + personaProviders: { coder: { provider: 'codex' } }, + }); + + // Then: model は undefined になる + expect(result.model).toBeUndefined(); + }); + + it('should resolve provider from personaProviders entry with only model specified', () => { + // Given: personaProviders エントリに provider が指定されていない（model のみ） + const result = resolveMovementProviderModel({ + step: { provider: undefined, model: undefined, personaDisplayName: 'coder' }, + provider: 'claude', + personaProviders: { coder: { model: 'o3-mini' } }, + }); + + // Then: provider は input.provider、model は personaProviders.model になる + expect(result.provider).toBe('claude'); + expect(result.model).toBe('o3-mini'); + }); }); diff --git a/src/__tests__/runAllTasks-concurrency.test.ts b/src/__tests__/runAllTasks-concurrency.test.ts index 77e10fe..f299aa7 100644 --- a/src/__tests__/runAllTasks-concurrency.test.ts +++ b/src/__tests__/runAllTasks-concurrency.test.ts @@ -40,6 +40,13 @@ vi.mock('../infra/config/index.js', () => ({ } return result; }, + resolveConfigValueWithSource: (_projectDir: string, key: string) => { + const raw = mockLoadConfigRaw() as Record; + const config = ('global' in raw && 'project' in raw) + ? { ...raw.global as Record, ...raw.project as Record } + : { ...raw, piece: 'default', provider: 'claude', verbose: false }; + return { value: config[key], source: 'project' }; + }, })); const mockLoadConfig = mockLoadConfigRaw; diff --git a/src/__tests__/summarize.test.ts b/src/__tests__/summarize.test.ts index db39382..e4107d1 100644 --- a/src/__tests__/summarize.test.ts +++ b/src/__tests__/summarize.test.ts @@ -4,14 +4,13 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -vi.mock('../infra/providers/index.js', () => ({ - getProvider: vi.fn(), -})); - -vi.mock('../infra/config/global/globalConfig.js', () => ({ - loadGlobalConfig: vi.fn(), - getBuiltinPiecesEnabled: vi.fn().mockReturnValue(true), -})); +vi.mock('../infra/providers/index.js', () => ({ + getProvider: vi.fn(), +})); + +vi.mock('../infra/config/index.js', () => ({ + resolveConfigValues: vi.fn(), +})); vi.mock('../shared/utils/index.js', async (importOriginal) => ({ ...(await importOriginal>()), @@ -22,30 +21,27 @@ vi.mock('../shared/utils/index.js', async (importOriginal) => ({ }), })); -import { getProvider } from '../infra/providers/index.js'; -import { loadGlobalConfig } from '../infra/config/global/globalConfig.js'; -import { summarizeTaskName } from '../infra/task/summarize.js'; - -const mockGetProvider = vi.mocked(getProvider); -const mockLoadGlobalConfig = vi.mocked(loadGlobalConfig); +import { getProvider } from '../infra/providers/index.js'; +import { resolveConfigValues } from '../infra/config/index.js'; +import { summarizeTaskName } from '../infra/task/summarize.js'; + +const mockGetProvider = vi.mocked(getProvider); +const mockResolveConfigValues = vi.mocked(resolveConfigValues); const mockProviderCall = vi.fn(); const mockProvider = { setup: () => ({ call: mockProviderCall }), }; -beforeEach(() => { - vi.clearAllMocks(); - mockGetProvider.mockReturnValue(mockProvider); - mockLoadGlobalConfig.mockReturnValue({ - language: 'ja', - defaultPiece: 'default', - logLevel: 'info', - provider: 'claude', - model: undefined, - branchNameStrategy: 'ai', - }); -}); +beforeEach(() => { + vi.clearAllMocks(); + mockGetProvider.mockReturnValue(mockProvider); + mockResolveConfigValues.mockReturnValue({ + provider: 'claude', + model: undefined, + branchNameStrategy: 'ai', + }); +}); describe('summarizeTaskName', () => { it('should return AI-generated slug for task name', async () => { @@ -166,14 +162,11 @@ describe('summarizeTaskName', () => { it('should use provider from config.yaml', async () => { // Given: config has codex provider with branchNameStrategy: 'ai' - mockLoadGlobalConfig.mockReturnValue({ - language: 'ja', - defaultPiece: 'default', - logLevel: 'info', - provider: 'codex', - model: 'gpt-4', - branchNameStrategy: 'ai', - }); + mockResolveConfigValues.mockReturnValue({ + provider: 'codex', + model: 'gpt-4', + branchNameStrategy: 'ai', + }); mockProviderCall.mockResolvedValue({ persona: 'summarizer', status: 'done', @@ -228,9 +221,9 @@ describe('summarizeTaskName', () => { it('should throw error when config load fails', async () => { // Given: config loading throws error - mockLoadGlobalConfig.mockImplementation(() => { - throw new Error('Config not found'); - }); + mockResolveConfigValues.mockImplementation(() => { + throw new Error('Config not found'); + }); // When/Then await expect(summarizeTaskName('test', { cwd: '/project' })).rejects.toThrow('Config not found'); @@ -257,14 +250,11 @@ describe('summarizeTaskName', () => { it('should use romaji by default', async () => { // Given: branchNameStrategy is not set (undefined) - mockLoadGlobalConfig.mockReturnValue({ - language: 'ja', - defaultPiece: 'default', - logLevel: 'info', - provider: 'claude', - model: undefined, - branchNameStrategy: undefined, - }); + mockResolveConfigValues.mockReturnValue({ + provider: 'claude', + model: undefined, + branchNameStrategy: undefined, + }); // When: useLLM not specified, branchNameStrategy not set const result = await summarizeTaskName('test task', { cwd: '/project' }); @@ -276,14 +266,11 @@ describe('summarizeTaskName', () => { it('should use AI when branchNameStrategy is ai', async () => { // Given: branchNameStrategy is 'ai' - mockLoadGlobalConfig.mockReturnValue({ - language: 'ja', - defaultPiece: 'default', - logLevel: 'info', - provider: 'claude', - model: undefined, - branchNameStrategy: 'ai', - }); + mockResolveConfigValues.mockReturnValue({ + provider: 'claude', + model: undefined, + branchNameStrategy: 'ai', + }); mockProviderCall.mockResolvedValue({ persona: 'summarizer', status: 'done', @@ -301,14 +288,11 @@ describe('summarizeTaskName', () => { it('should use romaji when branchNameStrategy is romaji', async () => { // Given: branchNameStrategy is 'romaji' - mockLoadGlobalConfig.mockReturnValue({ - language: 'ja', - defaultPiece: 'default', - logLevel: 'info', - provider: 'claude', - model: undefined, - branchNameStrategy: 'romaji', - }); + mockResolveConfigValues.mockReturnValue({ + provider: 'claude', + model: undefined, + branchNameStrategy: 'romaji', + }); // When const result = await summarizeTaskName('test task', { cwd: '/project' }); @@ -320,14 +304,11 @@ describe('summarizeTaskName', () => { it('should respect explicit useLLM option over config', async () => { // Given: branchNameStrategy is 'romaji' but useLLM is explicitly true - mockLoadGlobalConfig.mockReturnValue({ - language: 'ja', - defaultPiece: 'default', - logLevel: 'info', - provider: 'claude', - model: undefined, - branchNameStrategy: 'romaji', - }); + mockResolveConfigValues.mockReturnValue({ + provider: 'claude', + model: undefined, + branchNameStrategy: 'romaji', + }); mockProviderCall.mockResolvedValue({ persona: 'summarizer', status: 'done', @@ -345,14 +326,11 @@ describe('summarizeTaskName', () => { it('should respect explicit useLLM false over config with ai strategy', async () => { // Given: branchNameStrategy is 'ai' but useLLM is explicitly false - mockLoadGlobalConfig.mockReturnValue({ - language: 'ja', - defaultPiece: 'default', - logLevel: 'info', - provider: 'claude', - model: undefined, - branchNameStrategy: 'ai', - }); + mockResolveConfigValues.mockReturnValue({ + provider: 'claude', + model: undefined, + branchNameStrategy: 'ai', + }); // When: useLLM is explicitly false const result = await summarizeTaskName('test task', { cwd: '/project', useLLM: false }); diff --git a/src/__tests__/taskExecution.test.ts b/src/__tests__/taskExecution.test.ts index 0e26514..c254faf 100644 --- a/src/__tests__/taskExecution.test.ts +++ b/src/__tests__/taskExecution.test.ts @@ -5,12 +5,13 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import type { TaskInfo } from '../infra/task/index.js'; -const { mockResolveTaskExecution, mockExecutePiece, mockLoadPieceByIdentifier, mockResolvePieceConfigValues, mockBuildTaskResult, mockPersistTaskResult, mockPersistTaskError, mockPostExecutionFlow } = +const { mockResolveTaskExecution, mockExecutePiece, mockLoadPieceByIdentifier, mockResolvePieceConfigValues, mockResolveConfigValueWithSource, mockBuildTaskResult, mockPersistTaskResult, mockPersistTaskError, mockPostExecutionFlow } = vi.hoisted(() => ({ mockResolveTaskExecution: vi.fn(), mockExecutePiece: vi.fn(), mockLoadPieceByIdentifier: vi.fn(), mockResolvePieceConfigValues: vi.fn(), + mockResolveConfigValueWithSource: vi.fn(), mockBuildTaskResult: vi.fn(), mockPersistTaskResult: vi.fn(), mockPersistTaskError: vi.fn(), @@ -40,6 +41,7 @@ vi.mock('../infra/config/index.js', () => ({ loadPieceByIdentifier: (...args: unknown[]) => mockLoadPieceByIdentifier(...args), isPiecePath: () => false, resolvePieceConfigValues: (...args: unknown[]) => mockResolvePieceConfigValues(...args), + resolveConfigValueWithSource: (...args: unknown[]) => mockResolveConfigValueWithSource(...args), })); vi.mock('../shared/ui/index.js', () => ({ @@ -90,14 +92,17 @@ describe('executeAndCompleteTask', () => { model: undefined, personaProviders: {}, providerProfiles: {}, - providerOptions: { - claude: { sandbox: { allowUnsandboxedCommands: true } }, - }, notificationSound: true, notificationSoundEvents: {}, concurrency: 1, taskPollIntervalMs: 500, }); + mockResolveConfigValueWithSource.mockReturnValue({ + value: { + claude: { sandbox: { allowUnsandboxedCommands: true } }, + }, + source: 'project', + }); mockBuildTaskResult.mockReturnValue({ success: true }); mockResolveTaskExecution.mockResolvedValue({ execCwd: '/project', @@ -136,11 +141,13 @@ describe('executeAndCompleteTask', () => { taskDisplayLabel?: string; taskPrefix?: string; providerOptions?: unknown; + providerOptionsSource?: string; }; expect(pieceExecutionOptions?.taskDisplayLabel).toBe(taskDisplayLabel); expect(pieceExecutionOptions?.taskPrefix).toBe(taskDisplayLabel); expect(pieceExecutionOptions?.providerOptions).toEqual({ claude: { sandbox: { allowUnsandboxedCommands: true } }, }); + expect(pieceExecutionOptions?.providerOptionsSource).toBe('project'); }); }); diff --git a/src/core/models/index.ts b/src/core/models/index.ts index 8a07a10..fd9682c 100644 --- a/src/core/models/index.ts +++ b/src/core/models/index.ts @@ -30,7 +30,6 @@ export type { ObservabilityConfig, Language, PipelineConfig, - GlobalConfig, ProjectConfig, ProviderProfileName, ProviderPermissionProfile, diff --git a/src/core/models/global-config.ts b/src/core/models/persisted-global-config.ts similarity index 93% rename from src/core/models/global-config.ts rename to src/core/models/persisted-global-config.ts index fb774aa..4722d64 100644 --- a/src/core/models/global-config.ts +++ b/src/core/models/persisted-global-config.ts @@ -5,6 +5,11 @@ import type { MovementProviderOptions, PieceRuntimeConfig } from './piece-types.js'; import type { ProviderPermissionProfiles } from './provider-profiles.js'; +export interface PersonaProviderEntry { + provider?: 'claude' | 'codex' | 'opencode' | 'mock'; + model?: string; +} + /** Custom agent configuration */ export interface CustomAgentConfig { name: string; @@ -60,8 +65,8 @@ export interface NotificationSoundEventsConfig { runAbort?: boolean; } -/** Global configuration for takt */ -export interface GlobalConfig { +/** Persisted global configuration for ~/.takt/config.yaml */ +export interface PersistedGlobalConfig { language: Language; logLevel: 'debug' | 'info' | 'warn' | 'error'; provider?: 'claude' | 'codex' | 'opencode' | 'mock'; @@ -94,8 +99,8 @@ export interface GlobalConfig { bookmarksFile?: string; /** Path to piece categories file (default: ~/.takt/preferences/piece-categories.yaml) */ pieceCategoriesFile?: string; - /** Per-persona provider overrides (e.g., { coder: 'codex' }) */ - personaProviders?: Record; + /** Per-persona provider and model overrides (e.g., { coder: { provider: 'codex', model: 'o3-mini' } }) */ + personaProviders?: Record; /** Global provider-specific options (lowest priority) */ providerOptions?: MovementProviderOptions; /** Provider-specific permission profiles */ diff --git a/src/core/models/schemas.ts b/src/core/models/schemas.ts index f01b982..226d379 100644 --- a/src/core/models/schemas.ts +++ b/src/core/models/schemas.ts @@ -359,6 +359,11 @@ export const PieceConfigRawSchema = z.object({ interactive_mode: InteractiveModeSchema.optional(), }); +export const PersonaProviderEntrySchema = z.object({ + provider: z.enum(['claude', 'codex', 'opencode', 'mock']).optional(), + model: z.string().optional(), +}); + /** Custom agent configuration schema */ export const CustomAgentConfigSchema = z.object({ name: z.string().min(1), @@ -443,8 +448,11 @@ export const GlobalConfigSchema = z.object({ bookmarks_file: z.string().optional(), /** Path to piece categories file (default: ~/.takt/preferences/piece-categories.yaml) */ piece_categories_file: z.string().optional(), - /** Per-persona provider overrides (e.g., { coder: 'codex' }) */ - persona_providers: z.record(z.string(), z.enum(['claude', 'codex', 'opencode', 'mock'])).optional(), + /** Per-persona provider and model overrides. */ + persona_providers: z.record(z.string(), z.union([ + z.enum(['claude', 'codex', 'opencode', 'mock']), + PersonaProviderEntrySchema, + ])).optional(), /** Global provider-specific options (lowest priority) */ provider_options: MovementProviderOptionsSchema, /** Provider-specific permission profiles */ diff --git a/src/core/models/types.ts b/src/core/models/types.ts index c68197d..1b98b79 100644 --- a/src/core/models/types.ts +++ b/src/core/models/types.ts @@ -61,10 +61,10 @@ export type { // Configuration types (global and project) export type { + PersonaProviderEntry, CustomAgentConfig, ObservabilityConfig, Language, PipelineConfig, - GlobalConfig, ProjectConfig, -} from './global-config.js'; +} from './persisted-global-config.js'; diff --git a/src/core/piece/engine/OptionsBuilder.ts b/src/core/piece/engine/OptionsBuilder.ts index 8f99282..089d70a 100644 --- a/src/core/piece/engine/OptionsBuilder.ts +++ b/src/core/piece/engine/OptionsBuilder.ts @@ -29,6 +29,17 @@ function mergeProviderOptions( return Object.keys(result).length > 0 ? result : undefined; } +function resolveMovementProviderOptions( + source: 'env' | 'project' | 'global' | 'default' | undefined, + resolvedConfigOptions: MovementProviderOptions | undefined, + movementOptions: MovementProviderOptions | undefined, +): MovementProviderOptions | undefined { + if (source === 'env' || source === 'project') { + return mergeProviderOptions(movementOptions, resolvedConfigOptions); + } + return mergeProviderOptions(resolvedConfigOptions, movementOptions); +} + export class OptionsBuilder { constructor( private readonly engineOptions: PieceEngineOptions, @@ -53,11 +64,8 @@ export class OptionsBuilder { model: this.engineOptions.model, personaProviders: this.engineOptions.personaProviders, }); - - const resolvedProviderForPermissions = - this.engineOptions.provider - ?? resolved.provider - ?? 'claude'; + const resolvedProvider = resolved.provider ?? this.engineOptions.provider ?? 'claude'; + const resolvedModel = resolved.model ?? this.engineOptions.model; return { cwd: this.getCwd(), @@ -65,16 +73,17 @@ export class OptionsBuilder { personaPath: step.personaPath, provider: this.engineOptions.provider, model: this.engineOptions.model, - stepProvider: resolved.provider, - stepModel: resolved.model, + stepProvider: resolvedProvider, + stepModel: resolvedModel, permissionMode: resolveMovementPermissionMode({ movementName: step.name, requiredPermissionMode: step.requiredPermissionMode, - provider: resolvedProviderForPermissions, + provider: resolvedProvider, projectProviderProfiles: this.engineOptions.providerProfiles, globalProviderProfiles: DEFAULT_PROVIDER_PERMISSION_PROFILES, }), - providerOptions: mergeProviderOptions( + providerOptions: resolveMovementProviderOptions( + this.engineOptions.providerOptionsSource, this.engineOptions.providerOptions, step.providerOptions, ), diff --git a/src/core/piece/provider-resolution.ts b/src/core/piece/provider-resolution.ts index 6561c80..5364b10 100644 --- a/src/core/piece/provider-resolution.ts +++ b/src/core/piece/provider-resolution.ts @@ -1,12 +1,12 @@ import type { PieceMovement } from '../models/types.js'; - -export type ProviderType = 'claude' | 'codex' | 'opencode' | 'mock'; +import type { PersonaProviderEntry } from '../models/persisted-global-config.js'; +import type { ProviderType } from './types.js'; export interface MovementProviderModelInput { step: Pick; provider?: ProviderType; model?: string; - personaProviders?: Record; + personaProviders?: Record; } export interface MovementProviderModelOutput { @@ -15,10 +15,11 @@ export interface MovementProviderModelOutput { } export function resolveMovementProviderModel(input: MovementProviderModelInput): MovementProviderModelOutput { + const personaEntry = input.personaProviders?.[input.step.personaDisplayName]; return { provider: input.step.provider - ?? input.personaProviders?.[input.step.personaDisplayName] + ?? personaEntry?.provider ?? input.provider, - model: input.step.model ?? input.model, + model: input.step.model ?? personaEntry?.model ?? input.model, }; } diff --git a/src/core/piece/types.ts b/src/core/piece/types.ts index f3ae155..1cd2639 100644 --- a/src/core/piece/types.ts +++ b/src/core/piece/types.ts @@ -7,10 +7,12 @@ import type { PermissionResult, PermissionUpdate } from '@anthropic-ai/claude-agent-sdk'; import type { PieceMovement, AgentResponse, PieceState, Language, LoopMonitorConfig } from '../models/types.js'; +import type { PersonaProviderEntry } from '../models/persisted-global-config.js'; import type { ProviderPermissionProfiles } from '../models/provider-profiles.js'; import type { MovementProviderOptions } from '../models/piece-types.js'; export type ProviderType = 'claude' | 'codex' | 'opencode' | 'mock'; +export type ProviderOptionsSource = 'env' | 'project' | 'global' | 'default'; export interface StreamInitEventData { model: string; @@ -182,8 +184,10 @@ export interface PieceEngineOptions { model?: string; /** Resolved provider options */ providerOptions?: MovementProviderOptions; - /** Per-persona provider overrides (e.g., { coder: 'codex' }) */ - personaProviders?: Record; + /** Source layer for resolved provider options */ + providerOptionsSource?: ProviderOptionsSource; + /** Per-persona provider and model overrides (e.g., { coder: { provider: 'codex', model: 'o3-mini' } }) */ + personaProviders?: Record; /** Resolved provider permission profiles */ providerProfiles?: ProviderPermissionProfiles; /** Enable interactive-only rules and user-input transitions */ diff --git a/src/features/tasks/execute/pieceExecution.ts b/src/features/tasks/execute/pieceExecution.ts index 5f54919..1fdce46 100644 --- a/src/features/tasks/execute/pieceExecution.ts +++ b/src/features/tasks/execute/pieceExecution.ts @@ -467,6 +467,7 @@ export async function executePiece( provider: options.provider, model: options.model, providerOptions: options.providerOptions, + providerOptionsSource: options.providerOptionsSource, personaProviders: options.personaProviders, providerProfiles: options.providerProfiles, interactive: interactiveUserInput, @@ -547,8 +548,9 @@ export async function executePiece( model: options.model, personaProviders: options.personaProviders, }); - const movementProvider = resolved.provider ?? currentProvider; - const movementModel = resolved.model ?? globalConfig.model ?? '(default)'; + const movementProvider = resolved.provider ?? 'claude'; + const resolvedModel = resolved.model; + const movementModel = resolvedModel ?? '(default)'; currentMovementProvider = movementProvider; currentMovementModel = movementModel; providerEventLogger.setMovement(step.name); diff --git a/src/features/tasks/execute/taskExecution.ts b/src/features/tasks/execute/taskExecution.ts index 6ebdf4f..e630c13 100644 --- a/src/features/tasks/execute/taskExecution.ts +++ b/src/features/tasks/execute/taskExecution.ts @@ -2,7 +2,7 @@ * Task execution logic */ -import { loadPieceByIdentifier, isPiecePath, resolvePieceConfigValues } from '../../../infra/config/index.js'; +import { loadPieceByIdentifier, isPiecePath, resolveConfigValueWithSource, resolvePieceConfigValues } from '../../../infra/config/index.js'; import { TaskRunner, type TaskInfo } from '../../../infra/task/index.js'; import { header, @@ -66,16 +66,17 @@ async function executeTaskWithResult(options: ExecuteTaskOptions): Promise; + /** Source layer for resolved provider options */ + providerOptionsSource?: ProviderOptionsSource; + /** Per-persona provider and model overrides (e.g., { coder: { provider: 'codex', model: 'o3-mini' } }) */ + personaProviders?: Record; /** Resolved provider permission profiles */ providerProfiles?: ProviderPermissionProfiles; /** Enable interactive user input during step transitions */ diff --git a/src/index.ts b/src/index.ts index e4d138b..ea3f3e6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -29,9 +29,6 @@ export { isPiecePath, } from './infra/config/loaders/index.js'; export type { PieceSource, PieceWithSource, PieceDirEntry } from './infra/config/loaders/index.js'; -export { - loadConfig, -} from './infra/config/loadConfig.js'; export { saveProjectConfig, updateProjectConfig, diff --git a/src/infra/config/global/globalConfig.ts b/src/infra/config/global/globalConfig.ts index 1d17dc8..9a712e3 100644 --- a/src/infra/config/global/globalConfig.ts +++ b/src/infra/config/global/globalConfig.ts @@ -9,13 +9,15 @@ import { readFileSync, existsSync, writeFileSync, statSync, accessSync, constant import { isAbsolute } from 'node:path'; import { parse as parseYaml, stringify as stringifyYaml } from 'yaml'; import { GlobalConfigSchema } from '../../../core/models/index.js'; -import type { GlobalConfig, Language } from '../../../core/models/index.js'; +import type { Language } from '../../../core/models/index.js'; +import type { PersistedGlobalConfig, PersonaProviderEntry } from '../../../core/models/persisted-global-config.js'; import type { ProviderPermissionProfiles } from '../../../core/models/provider-profiles.js'; import { normalizeProviderOptions } from '../loaders/pieceParser.js'; import { getGlobalConfigPath } from '../paths.js'; import { DEFAULT_LANGUAGE } from '../../../shared/constants.js'; import { parseProviderModel } from '../../../shared/utils/providerModel.js'; import { applyGlobalConfigEnvOverrides, envVarNameFromPath } from '../env/config-env-overrides.js'; +import { invalidateAllResolvedConfigCache } from '../resolutionCache.js'; /** Claude-specific model aliases that are not valid for other providers */ const CLAUDE_MODEL_ALIASES = new Set(['opus', 'sonnet', 'haiku']); @@ -56,7 +58,6 @@ function validateCodexCliPath(pathValue: string, sourceName: 'TAKT_CODEX_CLI_PAT return trimmed; } -/** Validate that provider and model are compatible */ function validateProviderModelCompatibility(provider: string | undefined, model: string | undefined): void { if (!provider) return; @@ -80,6 +81,19 @@ function validateProviderModelCompatibility(provider: string | undefined, model: } } +function normalizePersonaProviders( + raw: Record | PersonaProviderEntry> | undefined, +): Record | undefined { + if (!raw) return undefined; + return Object.fromEntries( + Object.entries(raw).map(([persona, entry]) => { + const normalized: PersonaProviderEntry = typeof entry === 'string' ? { provider: entry } : entry; + validateProviderModelCompatibility(normalized.provider, normalized.model); + return [persona, normalized]; + }), + ); +} + function normalizeProviderProfiles( raw: Record }> | undefined, ): ProviderPermissionProfiles | undefined { @@ -114,7 +128,7 @@ function denormalizeProviderProfiles( */ export class GlobalConfigManager { private static instance: GlobalConfigManager | null = null; - private cachedConfig: GlobalConfig | null = null; + private cachedConfig: PersistedGlobalConfig | null = null; private constructor() {} @@ -136,7 +150,7 @@ export class GlobalConfigManager { } /** Load global configuration (cached) */ - load(): GlobalConfig { + load(): PersistedGlobalConfig { if (this.cachedConfig !== null) { return this.cachedConfig; } @@ -156,7 +170,7 @@ export class GlobalConfigManager { applyGlobalConfigEnvOverrides(rawConfig); const parsed = GlobalConfigSchema.parse(rawConfig); - const config: GlobalConfig = { + const config: PersistedGlobalConfig = { language: parsed.language, logLevel: parsed.log_level, provider: parsed.provider, @@ -186,7 +200,7 @@ export class GlobalConfigManager { minimalOutput: parsed.minimal_output, bookmarksFile: parsed.bookmarks_file, pieceCategoriesFile: parsed.piece_categories_file, - personaProviders: parsed.persona_providers, + personaProviders: normalizePersonaProviders(parsed.persona_providers as Record | PersonaProviderEntry> | undefined), providerOptions: normalizeProviderOptions(parsed.provider_options), providerProfiles: normalizeProviderProfiles(parsed.provider_profiles as Record }> | undefined), runtime: parsed.runtime?.prepare && parsed.runtime.prepare.length > 0 @@ -213,7 +227,7 @@ export class GlobalConfigManager { } /** Save global configuration to disk and invalidate cache */ - save(config: GlobalConfig): void { + save(config: PersistedGlobalConfig): void { const configPath = getGlobalConfigPath(); const raw: Record = { language: config.language, @@ -338,18 +352,20 @@ export class GlobalConfigManager { } writeFileSync(configPath, stringifyYaml(raw), 'utf-8'); this.invalidateCache(); + invalidateAllResolvedConfigCache(); } } export function invalidateGlobalConfigCache(): void { GlobalConfigManager.getInstance().invalidateCache(); + invalidateAllResolvedConfigCache(); } -export function loadGlobalConfig(): GlobalConfig { +export function loadGlobalConfig(): PersistedGlobalConfig { return GlobalConfigManager.getInstance().load(); } -export function saveGlobalConfig(config: GlobalConfig): void { +export function saveGlobalConfig(config: PersistedGlobalConfig): void { GlobalConfigManager.getInstance().save(config); } @@ -434,7 +450,7 @@ export function resolveCodexCliPath(): string | undefined { return validateCodexCliPath(envPath, 'TAKT_CODEX_CLI_PATH'); } - let config: GlobalConfig; + let config: PersistedGlobalConfig; try { config = loadGlobalConfig(); } catch { diff --git a/src/infra/config/loadConfig.ts b/src/infra/config/loadConfig.ts deleted file mode 100644 index 01f2afb..0000000 --- a/src/infra/config/loadConfig.ts +++ /dev/null @@ -1,131 +0,0 @@ -import type { GlobalConfig } from '../../core/models/index.js'; -import type { MovementProviderOptions } from '../../core/models/piece-types.js'; -import type { ProviderPermissionProfiles } from '../../core/models/provider-profiles.js'; -import type { AnalyticsConfig } from '../../core/models/global-config.js'; -import { loadGlobalConfig } from './global/globalConfig.js'; -import { loadProjectConfig } from './project/projectConfig.js'; -import { envVarNameFromPath } from './env/config-env-overrides.js'; - -export interface LoadedConfig extends GlobalConfig { - piece: string; - provider: NonNullable; - verbose: boolean; - providerOptions?: MovementProviderOptions; - providerProfiles?: ProviderPermissionProfiles; -} - -export function loadConfig(projectDir: string): LoadedConfig { - const global = loadGlobalConfig(); - const project = loadProjectConfig(projectDir); - const provider = (project.provider ?? global.provider ?? 'claude') as NonNullable; - - return { - ...global, - piece: project.piece ?? 'default', - provider, - autoPr: project.auto_pr ?? global.autoPr, - draftPr: project.draft_pr ?? global.draftPr, - model: resolveModel(global, provider), - verbose: resolveVerbose(project.verbose, global.verbose), - analytics: mergeAnalytics(global.analytics, project.analytics), - providerOptions: mergeProviderOptions(global.providerOptions, project.providerOptions), - providerProfiles: mergeProviderProfiles(global.providerProfiles, project.providerProfiles), - }; -} - -function resolveModel(global: GlobalConfig, provider: GlobalConfig['provider']): string | undefined { - if (!global.model) return undefined; - const globalProvider = global.provider ?? 'claude'; - const resolvedProvider = provider ?? 'claude'; - if (globalProvider !== resolvedProvider) return undefined; - return global.model; -} - -function resolveVerbose(projectVerbose: boolean | undefined, globalVerbose: boolean | undefined): boolean { - const envVerbose = loadEnvBooleanSetting('verbose'); - if (envVerbose !== undefined) return envVerbose; - if (projectVerbose !== undefined) return projectVerbose; - if (globalVerbose !== undefined) return globalVerbose; - return false; -} - -function loadEnvBooleanSetting(configKey: string): boolean | undefined { - const envKey = envVarNameFromPath(configKey); - const raw = process.env[envKey]; - if (raw === undefined) return undefined; - - const normalized = raw.trim().toLowerCase(); - if (normalized === 'true') return true; - if (normalized === 'false') return false; - - throw new Error(`${envKey} must be one of: true, false`); -} - -function mergeProviderOptions( - globalOptions: MovementProviderOptions | undefined, - projectOptions: MovementProviderOptions | undefined, -): MovementProviderOptions | undefined { - if (!globalOptions && !projectOptions) return undefined; - - const result: MovementProviderOptions = {}; - if (globalOptions?.codex || projectOptions?.codex) { - result.codex = { ...globalOptions?.codex, ...projectOptions?.codex }; - } - if (globalOptions?.opencode || projectOptions?.opencode) { - result.opencode = { ...globalOptions?.opencode, ...projectOptions?.opencode }; - } - if (globalOptions?.claude?.sandbox || projectOptions?.claude?.sandbox) { - result.claude = { - sandbox: { - ...globalOptions?.claude?.sandbox, - ...projectOptions?.claude?.sandbox, - }, - }; - } - - return Object.keys(result).length > 0 ? result : undefined; -} - -function mergeAnalytics( - globalAnalytics: AnalyticsConfig | undefined, - projectAnalytics: AnalyticsConfig | undefined, -): AnalyticsConfig | undefined { - if (!globalAnalytics && !projectAnalytics) return undefined; - - const merged: AnalyticsConfig = { - enabled: projectAnalytics?.enabled ?? globalAnalytics?.enabled, - eventsPath: projectAnalytics?.eventsPath ?? globalAnalytics?.eventsPath, - retentionDays: projectAnalytics?.retentionDays ?? globalAnalytics?.retentionDays, - }; - - if (merged.enabled === undefined && merged.eventsPath === undefined && merged.retentionDays === undefined) { - return undefined; - } - return merged; -} - -function mergeProviderProfiles( - globalProfiles: ProviderPermissionProfiles | undefined, - projectProfiles: ProviderPermissionProfiles | undefined, -): ProviderPermissionProfiles | undefined { - if (!globalProfiles && !projectProfiles) return undefined; - - const merged: ProviderPermissionProfiles = { ...(globalProfiles ?? {}) }; - for (const [provider, profile] of Object.entries(projectProfiles ?? {})) { - const key = provider as keyof ProviderPermissionProfiles; - const existing = merged[key]; - if (!existing) { - merged[key] = profile; - continue; - } - merged[key] = { - defaultPermissionMode: profile.defaultPermissionMode, - movementPermissionOverrides: { - ...(existing.movementPermissionOverrides ?? {}), - ...(profile.movementPermissionOverrides ?? {}), - }, - }; - } - - return Object.keys(merged).length > 0 ? merged : undefined; -} diff --git a/src/infra/config/project/projectConfig.ts b/src/infra/config/project/projectConfig.ts index 34c5bef..a1fe8bf 100644 --- a/src/infra/config/project/projectConfig.ts +++ b/src/infra/config/project/projectConfig.ts @@ -10,9 +10,10 @@ import { parse, stringify } from 'yaml'; import { copyProjectResourcesToDir } from '../../resources/index.js'; import type { ProjectLocalConfig } from '../types.js'; import type { ProviderPermissionProfiles } from '../../../core/models/provider-profiles.js'; -import type { AnalyticsConfig } from '../../../core/models/global-config.js'; +import type { AnalyticsConfig } from '../../../core/models/persisted-global-config.js'; import { applyProjectConfigEnvOverrides } from '../env/config-env-overrides.js'; import { normalizeProviderOptions } from '../loaders/pieceParser.js'; +import { invalidateResolvedConfigCache } from '../resolutionCache.js'; export type { ProjectLocalConfig } from '../types.js'; @@ -154,6 +155,7 @@ export function saveProjectConfig(projectDir: string, config: ProjectLocalConfig const content = stringify(savePayload, { indent: 2 }); writeFileSync(configPath, content, 'utf-8'); + invalidateResolvedConfigCache(projectDir); } /** diff --git a/src/infra/config/project/resolvedSettings.ts b/src/infra/config/project/resolvedSettings.ts index e514c5d..b777524 100644 --- a/src/infra/config/project/resolvedSettings.ts +++ b/src/infra/config/project/resolvedSettings.ts @@ -1,32 +1,5 @@ -import { envVarNameFromPath } from '../env/config-env-overrides.js'; -import { loadConfig } from '../loadConfig.js'; - -function resolveValue( - envValue: T | undefined, - localValue: T | undefined, - globalValue: T | undefined, - defaultValue: T, -): T { - if (envValue !== undefined) return envValue; - if (localValue !== undefined) return localValue; - if (globalValue !== undefined) return globalValue; - return defaultValue; -} - -function loadEnvBooleanSetting(configKey: string): boolean | undefined { - const envKey = envVarNameFromPath(configKey); - const raw = process.env[envKey]; - if (raw === undefined) return undefined; - - const normalized = raw.trim().toLowerCase(); - if (normalized === 'true') return true; - if (normalized === 'false') return false; - - throw new Error(`${envKey} must be one of: true, false`); -} +import { resolveConfigValue } from '../resolveConfigValue.js'; export function isVerboseMode(projectDir: string): boolean { - const envValue = loadEnvBooleanSetting('verbose'); - const config = loadConfig(projectDir); - return resolveValue(envValue, undefined, config.verbose, false); + return resolveConfigValue(projectDir, 'verbose'); } diff --git a/src/infra/config/resolutionCache.ts b/src/infra/config/resolutionCache.ts new file mode 100644 index 0000000..0c3dfae --- /dev/null +++ b/src/infra/config/resolutionCache.ts @@ -0,0 +1,50 @@ +import { resolve } from 'node:path'; +import type { ProjectLocalConfig } from './types.js'; +import type { ConfigParameterKey } from './resolvedConfig.js'; + +const projectConfigCache = new Map(); +const resolvedValueCache = new Map(); + +function normalizeProjectDir(projectDir: string): string { + return resolve(projectDir); +} + +function resolvedValueKey(projectDir: string, key: ConfigParameterKey): string { + return `${normalizeProjectDir(projectDir)}::${key}`; +} + +export function getCachedProjectConfig(projectDir: string): ProjectLocalConfig | undefined { + return projectConfigCache.get(normalizeProjectDir(projectDir)); +} + +export function setCachedProjectConfig(projectDir: string, config: ProjectLocalConfig): void { + projectConfigCache.set(normalizeProjectDir(projectDir), config); +} + +export function hasCachedResolvedValue(projectDir: string, key: ConfigParameterKey): boolean { + return resolvedValueCache.has(resolvedValueKey(projectDir, key)); +} + +export function getCachedResolvedValue(projectDir: string, key: ConfigParameterKey): unknown { + return resolvedValueCache.get(resolvedValueKey(projectDir, key)); +} + +export function setCachedResolvedValue(projectDir: string, key: ConfigParameterKey, value: unknown): void { + resolvedValueCache.set(resolvedValueKey(projectDir, key), value); +} + +export function invalidateResolvedConfigCache(projectDir: string): void { + const normalizedProjectDir = normalizeProjectDir(projectDir); + projectConfigCache.delete(normalizedProjectDir); + const prefix = `${normalizedProjectDir}::`; + for (const key of resolvedValueCache.keys()) { + if (key.startsWith(prefix)) { + resolvedValueCache.delete(key); + } + } +} + +export function invalidateAllResolvedConfigCache(): void { + projectConfigCache.clear(); + resolvedValueCache.clear(); +} diff --git a/src/infra/config/resolveConfigValue.ts b/src/infra/config/resolveConfigValue.ts index 8f7d4f9..7cc3448 100644 --- a/src/infra/config/resolveConfigValue.ts +++ b/src/infra/config/resolveConfigValue.ts @@ -1,22 +1,230 @@ -import { loadConfig, type LoadedConfig } from './loadConfig.js'; +import { loadGlobalConfig } from './global/globalConfig.js'; +import { loadProjectConfig } from './project/projectConfig.js'; +import { envVarNameFromPath } from './env/config-env-overrides.js'; +import { + getCachedProjectConfig, + getCachedResolvedValue, + hasCachedResolvedValue, + setCachedProjectConfig, + setCachedResolvedValue, +} from './resolutionCache.js'; +import type { ConfigParameterKey, LoadedConfig } from './resolvedConfig.js'; -export type ConfigParameterKey = keyof LoadedConfig; +export type { ConfigParameterKey } from './resolvedConfig.js'; +export { invalidateResolvedConfigCache, invalidateAllResolvedConfigCache } from './resolutionCache.js'; + +export interface PieceContext { + provider?: LoadedConfig['provider']; + model?: LoadedConfig['model']; + providerOptions?: LoadedConfig['providerOptions']; +} + +export interface ResolveConfigOptions { + pieceContext?: PieceContext; +} + +export type ConfigValueSource = 'env' | 'project' | 'piece' | 'global' | 'default'; + +export interface ResolvedConfigValue { + value: LoadedConfig[K]; + source: ConfigValueSource; +} + +type ResolutionLayer = 'local' | 'piece' | 'global'; +interface ResolutionRule { + layers: readonly ResolutionLayer[]; + defaultValue?: LoadedConfig[K]; + mergeMode?: 'analytics'; + pieceValue?: (pieceContext: PieceContext | undefined) => LoadedConfig[K] | undefined; +} + +function loadProjectConfigCached(projectDir: string) { + const cached = getCachedProjectConfig(projectDir); + if (cached !== undefined) { + return cached; + } + const loaded = loadProjectConfig(projectDir); + setCachedProjectConfig(projectDir, loaded); + return loaded; +} + +const DEFAULT_RULE: ResolutionRule = { + layers: ['local', 'global'], +}; + +const PROVIDER_OPTIONS_ENV_PATHS = [ + 'provider_options', + 'provider_options.codex.network_access', + 'provider_options.opencode.network_access', + 'provider_options.claude.sandbox.allow_unsandboxed_commands', + 'provider_options.claude.sandbox.excluded_commands', +] as const; + +const RESOLUTION_REGISTRY: Partial<{ [K in ConfigParameterKey]: ResolutionRule }> = { + piece: { layers: ['local', 'global'], defaultValue: 'default' }, + provider: { + layers: ['local', 'piece', 'global'], + defaultValue: 'claude', + pieceValue: (pieceContext) => pieceContext?.provider, + }, + model: { + layers: ['local', 'piece', 'global'], + pieceValue: (pieceContext) => pieceContext?.model, + }, + providerOptions: { + layers: ['local', 'piece', 'global'], + pieceValue: (pieceContext) => pieceContext?.providerOptions, + }, + autoPr: { layers: ['local', 'global'] }, + draftPr: { layers: ['local', 'global'] }, + analytics: { layers: ['local', 'global'], mergeMode: 'analytics' }, + verbose: { layers: ['local', 'global'], defaultValue: false }, +}; + +function resolveAnalyticsMerged( + project: ReturnType, + global: ReturnType, +): LoadedConfig['analytics'] { + const localAnalytics = project.analytics; + const globalAnalytics = global.analytics; + + const enabled = localAnalytics?.enabled ?? globalAnalytics?.enabled; + const eventsPath = localAnalytics?.eventsPath ?? globalAnalytics?.eventsPath; + const retentionDays = localAnalytics?.retentionDays ?? globalAnalytics?.retentionDays; + + if (enabled === undefined && eventsPath === undefined && retentionDays === undefined) { + return undefined; + } + return { enabled, eventsPath, retentionDays }; +} + +function resolveAnalyticsSource( + project: ReturnType, + global: ReturnType, +): ConfigValueSource { + if (project.analytics !== undefined) return 'project'; + if (global.analytics !== undefined) return 'global'; + return 'default'; +} + +function getLocalLayerValue( + project: ReturnType, + key: K, +): LoadedConfig[K] | undefined { + switch (key) { + case 'piece': + return project.piece as LoadedConfig[K] | undefined; + case 'provider': + return project.provider as LoadedConfig[K] | undefined; + case 'autoPr': + return project.auto_pr as LoadedConfig[K] | undefined; + case 'draftPr': + return project.draft_pr as LoadedConfig[K] | undefined; + case 'verbose': + return project.verbose as LoadedConfig[K] | undefined; + case 'analytics': + return project.analytics as LoadedConfig[K] | undefined; + case 'providerOptions': + return project.providerOptions as LoadedConfig[K] | undefined; + case 'providerProfiles': + return project.providerProfiles as LoadedConfig[K] | undefined; + default: + return undefined; + } +} + +function getGlobalLayerValue( + global: ReturnType, + key: K, +): LoadedConfig[K] | undefined { + return global[key as keyof typeof global] as LoadedConfig[K] | undefined; +} + +function resolveByRegistry( + key: K, + project: ReturnType, + global: ReturnType, + options: ResolveConfigOptions | undefined, +): ResolvedConfigValue { + const rule = (RESOLUTION_REGISTRY[key] ?? DEFAULT_RULE) as ResolutionRule; + if (rule.mergeMode === 'analytics') { + return { + value: resolveAnalyticsMerged(project, global) as LoadedConfig[K], + source: resolveAnalyticsSource(project, global), + }; + } + + for (const layer of rule.layers) { + let value: LoadedConfig[K] | undefined; + if (layer === 'local') { + value = getLocalLayerValue(project, key); + } else if (layer === 'piece') { + value = rule.pieceValue?.(options?.pieceContext); + } else { + value = getGlobalLayerValue(global, key); + } + if (value !== undefined) { + if (layer === 'local') { + if (key === 'providerOptions' && hasProviderOptionsEnvOverride()) { + return { value, source: 'env' }; + } + return { value, source: 'project' }; + } + if (layer === 'piece') { + return { value, source: 'piece' }; + } + return { value, source: 'global' }; + } + } + + return { value: rule.defaultValue as LoadedConfig[K], source: 'default' }; +} + +function hasProviderOptionsEnvOverride(): boolean { + return PROVIDER_OPTIONS_ENV_PATHS.some((path) => process.env[envVarNameFromPath(path)] !== undefined); +} + +function resolveUncachedConfigValue( + projectDir: string, + key: K, + options?: ResolveConfigOptions, +): ResolvedConfigValue { + const project = loadProjectConfigCached(projectDir); + const global = loadGlobalConfig(); + return resolveByRegistry(key, project, global, options); +} + +export function resolveConfigValueWithSource( + projectDir: string, + key: K, + options?: ResolveConfigOptions, +): ResolvedConfigValue { + const resolved = resolveUncachedConfigValue(projectDir, key, options); + if (!options?.pieceContext) { + setCachedResolvedValue(projectDir, key, resolved.value); + } + return resolved; +} export function resolveConfigValue( projectDir: string, key: K, + options?: ResolveConfigOptions, ): LoadedConfig[K] { - return loadConfig(projectDir)[key]; + if (!options?.pieceContext && hasCachedResolvedValue(projectDir, key)) { + return getCachedResolvedValue(projectDir, key) as LoadedConfig[K]; + } + return resolveConfigValueWithSource(projectDir, key, options).value; } export function resolveConfigValues( projectDir: string, keys: readonly K[], + options?: ResolveConfigOptions, ): Pick { - const config = loadConfig(projectDir); const result = {} as Pick; for (const key of keys) { - result[key] = config[key]; + result[key] = resolveConfigValue(projectDir, key, options); } return result; } diff --git a/src/infra/config/resolvePieceConfigValue.ts b/src/infra/config/resolvePieceConfigValue.ts index 98b0375..a01fa4b 100644 --- a/src/infra/config/resolvePieceConfigValue.ts +++ b/src/infra/config/resolvePieceConfigValue.ts @@ -1,17 +1,20 @@ import type { ConfigParameterKey } from './resolveConfigValue.js'; import { resolveConfigValue, resolveConfigValues } from './resolveConfigValue.js'; -import type { LoadedConfig } from './loadConfig.js'; +import type { ResolveConfigOptions } from './resolveConfigValue.js'; +import type { LoadedConfig } from './resolvedConfig.js'; export function resolvePieceConfigValue( projectDir: string, key: K, + options?: ResolveConfigOptions, ): LoadedConfig[K] { - return resolveConfigValue(projectDir, key); + return resolveConfigValue(projectDir, key, options); } export function resolvePieceConfigValues( projectDir: string, keys: readonly K[], + options?: ResolveConfigOptions, ): Pick { - return resolveConfigValues(projectDir, keys); + return resolveConfigValues(projectDir, keys, options); } diff --git a/src/infra/config/resolvedConfig.ts b/src/infra/config/resolvedConfig.ts new file mode 100644 index 0000000..820dc73 --- /dev/null +++ b/src/infra/config/resolvedConfig.ts @@ -0,0 +1,9 @@ +import type { PersistedGlobalConfig } from '../../core/models/persisted-global-config.js'; + +export interface LoadedConfig extends Omit { + piece: string; + provider: NonNullable; + verbose: boolean; +} + +export type ConfigParameterKey = keyof LoadedConfig; diff --git a/src/infra/config/types.ts b/src/infra/config/types.ts index 159f52c..eb039b5 100644 --- a/src/infra/config/types.ts +++ b/src/infra/config/types.ts @@ -4,7 +4,7 @@ import type { MovementProviderOptions } from '../../core/models/piece-types.js'; import type { ProviderPermissionProfiles } from '../../core/models/provider-profiles.js'; -import type { AnalyticsConfig } from '../../core/models/global-config.js'; +import type { AnalyticsConfig } from '../../core/models/persisted-global-config.js'; /** Project configuration stored in .takt/config.yaml */ export interface ProjectLocalConfig { From b9dfe93d854a94a493162062110d3841d47449cd Mon Sep 17 00:00:00 2001 From: nrs <38722970+nrslib@users.noreply.github.com> Date: Fri, 20 Feb 2026 11:58:48 +0900 Subject: [PATCH 08/17] takt: add-sync-with-root (#325) --- src/__tests__/taskSyncAction.test.ts | 271 ++++++++++++++++++++ src/features/tasks/list/index.ts | 4 + src/features/tasks/list/taskActionTarget.ts | 2 +- src/features/tasks/list/taskActions.ts | 2 + src/features/tasks/list/taskDiffActions.ts | 1 + src/features/tasks/list/taskSyncAction.ts | 115 +++++++++ 6 files changed, 394 insertions(+), 1 deletion(-) create mode 100644 src/__tests__/taskSyncAction.test.ts create mode 100644 src/features/tasks/list/taskSyncAction.ts diff --git a/src/__tests__/taskSyncAction.test.ts b/src/__tests__/taskSyncAction.test.ts new file mode 100644 index 0000000..e0bb695 --- /dev/null +++ b/src/__tests__/taskSyncAction.test.ts @@ -0,0 +1,271 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest'; + +vi.mock('node:fs', () => ({ + existsSync: vi.fn(), +})); + +vi.mock('node:child_process', () => ({ + execFileSync: vi.fn(), +})); + +vi.mock('../shared/ui/index.js', () => ({ + success: vi.fn(), + error: vi.fn(), +})); + +vi.mock('../shared/utils/index.js', () => ({ + createLogger: vi.fn(() => ({ + info: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + })), + getErrorMessage: vi.fn((err) => String(err)), +})); + +vi.mock('../features/tasks/execute/taskExecution.js', () => ({ + executeTask: vi.fn(), +})); + +vi.mock('../features/tasks/execute/selectAndExecute.js', () => ({ + determinePiece: vi.fn(), +})); + +vi.mock('../shared/constants.js', () => ({ + DEFAULT_PIECE_NAME: 'default', +})); + +import * as fs from 'node:fs'; +import { execFileSync } from 'node:child_process'; +import { error as logError, success } from '../shared/ui/index.js'; +import { executeTask } from '../features/tasks/execute/taskExecution.js'; +import { determinePiece } from '../features/tasks/execute/selectAndExecute.js'; +import { syncBranchWithRoot } from '../features/tasks/list/taskSyncAction.js'; +import type { TaskListItem } from '../infra/task/index.js'; + +const mockExistsSync = vi.mocked(fs.existsSync); +const mockExecFileSync = vi.mocked(execFileSync); +const mockExecuteTask = vi.mocked(executeTask); +const mockDeterminePiece = vi.mocked(determinePiece); +const mockLogError = vi.mocked(logError); +const mockSuccess = vi.mocked(success); + +function makeTask(overrides: Partial = {}): TaskListItem { + return { + kind: 'completed', + name: 'test-task', + createdAt: '2026-01-01T00:00:00Z', + filePath: '/project/.takt/tasks.yaml', + content: 'Implement feature X', + worktreePath: '/project-worktrees/test-task', + ...overrides, + }; +} + +const PROJECT_DIR = '/project'; + +describe('syncBranchWithRoot', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockExistsSync.mockReturnValue(true); + mockDeterminePiece.mockResolvedValue('default'); + }); + + it('throws when called with a non-task BranchActionTarget', async () => { + const branchTarget = { + info: { branch: 'some-branch', commit: 'abc123' }, + originalInstruction: 'Do something', + }; + + await expect( + syncBranchWithRoot(PROJECT_DIR, branchTarget as never), + ).rejects.toThrow('Sync requires a task target.'); + }); + + it('returns false and logs error when worktreePath is missing', async () => { + const task = makeTask({ worktreePath: undefined }); + + const result = await syncBranchWithRoot(PROJECT_DIR, task); + + expect(result).toBe(false); + expect(mockLogError).toHaveBeenCalledWith( + expect.stringContaining('Worktree directory does not exist'), + ); + expect(mockExecFileSync).not.toHaveBeenCalled(); + }); + + it('returns false and logs error when worktreePath does not exist on disk', async () => { + const task = makeTask(); + mockExistsSync.mockReturnValue(false); + + const result = await syncBranchWithRoot(PROJECT_DIR, task); + + expect(result).toBe(false); + expect(mockLogError).toHaveBeenCalledWith( + expect.stringContaining('Worktree directory does not exist'), + ); + }); + + it('returns false and logs error when git fetch fails', async () => { + const task = makeTask(); + mockExecFileSync.mockImplementationOnce(() => { throw new Error('fetch error'); }); + + const result = await syncBranchWithRoot(PROJECT_DIR, task); + + expect(result).toBe(false); + expect(mockLogError).toHaveBeenCalledWith(expect.stringContaining('Failed to fetch from root')); + expect(mockExecuteTask).not.toHaveBeenCalled(); + }); + + it('returns true and shows "Synced." when merge succeeds without conflicts', async () => { + const task = makeTask(); + mockExecFileSync.mockReturnValue('' as never); + + const result = await syncBranchWithRoot(PROJECT_DIR, task); + + expect(result).toBe(true); + expect(mockSuccess).toHaveBeenCalledWith('Synced.'); + expect(mockExecuteTask).not.toHaveBeenCalled(); + }); + + it('calls executeTask with conflict resolution instruction when merge has conflicts', async () => { + const task = makeTask(); + mockExecFileSync + .mockReturnValueOnce('' as never) + .mockImplementationOnce(() => { throw new Error('CONFLICT'); }); + + mockExecuteTask.mockResolvedValue(true); + + const result = await syncBranchWithRoot(PROJECT_DIR, task); + + expect(result).toBe(true); + expect(mockSuccess).toHaveBeenCalledWith('Conflicts resolved.'); + expect(mockExecuteTask).toHaveBeenCalledWith( + expect.objectContaining({ + cwd: task.worktreePath, + projectCwd: PROJECT_DIR, + pieceIdentifier: 'default', + task: expect.stringContaining('Git merge has stopped due to merge conflicts.'), + }), + ); + }); + + it('includes original task content in conflict resolution instruction', async () => { + const task = makeTask({ content: 'Implement feature X' }); + mockExecFileSync + .mockReturnValueOnce('' as never) + .mockImplementationOnce(() => { throw new Error('CONFLICT'); }); + mockExecuteTask.mockResolvedValue(true); + + await syncBranchWithRoot(PROJECT_DIR, task); + + expect(mockExecuteTask).toHaveBeenCalledWith( + expect.objectContaining({ + task: expect.stringContaining('Implement feature X'), + }), + ); + }); + + it('uses task piece when available for AI resolution', async () => { + const task = makeTask({ data: { piece: 'custom-piece' } }); + mockExecFileSync + .mockReturnValueOnce('' as never) + .mockImplementationOnce(() => { throw new Error('CONFLICT'); }); + mockDeterminePiece.mockResolvedValue('custom-piece'); + mockExecuteTask.mockResolvedValue(true); + + await syncBranchWithRoot(PROJECT_DIR, task); + + expect(mockDeterminePiece).toHaveBeenCalledWith(PROJECT_DIR, 'custom-piece'); + }); + + it('uses DEFAULT_PIECE_NAME when task.data.piece is not set', async () => { + const task = makeTask({ data: undefined }); + mockExecFileSync + .mockReturnValueOnce('' as never) + .mockImplementationOnce(() => { throw new Error('CONFLICT'); }); + mockExecuteTask.mockResolvedValue(true); + + await syncBranchWithRoot(PROJECT_DIR, task); + + expect(mockDeterminePiece).toHaveBeenCalledWith(PROJECT_DIR, 'default'); + }); + + it('aborts merge and returns false when AI resolution fails', async () => { + const task = makeTask(); + mockExecFileSync + .mockReturnValueOnce('' as never) + .mockImplementationOnce(() => { throw new Error('CONFLICT'); }) + .mockReturnValueOnce('' as never); + mockExecuteTask.mockResolvedValue(false); + + const result = await syncBranchWithRoot(PROJECT_DIR, task); + + expect(result).toBe(false); + expect(mockLogError).toHaveBeenCalledWith( + expect.stringContaining('Failed to resolve conflicts'), + ); + expect(mockExecFileSync).toHaveBeenCalledWith( + 'git', ['merge', '--abort'], + expect.objectContaining({ cwd: task.worktreePath }), + ); + }); + + it('aborts merge and returns false when determinePiece returns null', async () => { + const task = makeTask(); + mockExecFileSync + .mockReturnValueOnce('' as never) + .mockImplementationOnce(() => { throw new Error('CONFLICT'); }) + .mockReturnValueOnce('' as never); + mockDeterminePiece.mockResolvedValue(null); + + const result = await syncBranchWithRoot(PROJECT_DIR, task); + + expect(result).toBe(false); + expect(mockExecuteTask).not.toHaveBeenCalled(); + expect(mockExecFileSync).toHaveBeenCalledWith( + 'git', ['merge', '--abort'], + expect.objectContaining({ cwd: task.worktreePath }), + ); + }); + + it('does not throw when git merge --abort itself fails', async () => { + const task = makeTask(); + mockExecFileSync + .mockReturnValueOnce('' as never) + .mockImplementationOnce(() => { throw new Error('CONFLICT'); }) + .mockImplementationOnce(() => { throw new Error('abort failed'); }); + mockDeterminePiece.mockResolvedValue(null); + + const result = await syncBranchWithRoot(PROJECT_DIR, task); + + expect(result).toBe(false); + }); + + it('fetches from projectDir using local path ref', async () => { + const task = makeTask(); + mockExecFileSync.mockReturnValue('' as never); + + await syncBranchWithRoot(PROJECT_DIR, task); + + expect(mockExecFileSync).toHaveBeenCalledWith( + 'git', + ['fetch', PROJECT_DIR, 'HEAD:refs/remotes/root/sync-target'], + expect.objectContaining({ cwd: task.worktreePath }), + ); + }); + + it('passes agentOverrides to executeTask', async () => { + const task = makeTask(); + mockExecFileSync + .mockReturnValueOnce('' as never) + .mockImplementationOnce(() => { throw new Error('CONFLICT'); }); + mockExecuteTask.mockResolvedValue(true); + const options = { provider: 'anthropic' as never }; + + await syncBranchWithRoot(PROJECT_DIR, task, options); + + expect(mockExecuteTask).toHaveBeenCalledWith( + expect.objectContaining({ agentOverrides: options }), + ); + }); +}); diff --git a/src/features/tasks/list/index.ts b/src/features/tasks/list/index.ts index 5ef5cbe..d01c680 100644 --- a/src/features/tasks/list/index.ts +++ b/src/features/tasks/list/index.ts @@ -12,6 +12,7 @@ import { tryMergeBranch, mergeBranch, instructBranch, + syncBranchWithRoot, } from './taskActions.js'; import { deletePendingTask, deleteFailedTask, deleteCompletedTask, deleteAllTasks } from './taskDeleteActions.js'; import { retryFailedTask } from './taskRetryActions.js'; @@ -167,6 +168,9 @@ export async function listTasks( case 'instruct': await instructBranch(cwd, task); break; + case 'sync': + await syncBranchWithRoot(cwd, task, options); + break; case 'try': tryMergeBranch(cwd, task); break; diff --git a/src/features/tasks/list/taskActionTarget.ts b/src/features/tasks/list/taskActionTarget.ts index 2e2f0c4..9f1a160 100644 --- a/src/features/tasks/list/taskActionTarget.ts +++ b/src/features/tasks/list/taskActionTarget.ts @@ -1,6 +1,6 @@ import type { BranchListItem, TaskListItem } from '../../../infra/task/index.js'; -export type ListAction = 'diff' | 'instruct' | 'try' | 'merge' | 'delete'; +export type ListAction = 'diff' | 'instruct' | 'sync' | 'try' | 'merge' | 'delete'; export type BranchActionTarget = TaskListItem | Pick; diff --git a/src/features/tasks/list/taskActions.ts b/src/features/tasks/list/taskActions.ts index ef94343..655c42f 100644 --- a/src/features/tasks/list/taskActions.ts +++ b/src/features/tasks/list/taskActions.ts @@ -17,3 +17,5 @@ export { } from './taskBranchLifecycleActions.js'; export { instructBranch } from './taskInstructionActions.js'; + +export { syncBranchWithRoot } from './taskSyncAction.js'; diff --git a/src/features/tasks/list/taskDiffActions.ts b/src/features/tasks/list/taskDiffActions.ts index ef9d0cd..13b4886 100644 --- a/src/features/tasks/list/taskDiffActions.ts +++ b/src/features/tasks/list/taskDiffActions.ts @@ -66,6 +66,7 @@ export async function showDiffAndPromptActionForTask( [ { label: 'View diff', value: 'diff', description: 'Show full diff in pager' }, { label: 'Instruct', value: 'instruct', description: 'Craft additional instructions and requeue this task' }, + { label: 'Sync with root', value: 'sync', description: 'Merge root HEAD into worktree branch; auto-resolve conflicts with AI' }, { label: 'Try merge', value: 'try', description: 'Squash merge (stage changes without commit)' }, { label: 'Merge & cleanup', value: 'merge', description: 'Merge and delete branch' }, { label: 'Delete', value: 'delete', description: 'Discard changes, delete branch' }, diff --git a/src/features/tasks/list/taskSyncAction.ts b/src/features/tasks/list/taskSyncAction.ts new file mode 100644 index 0000000..ec14f4f --- /dev/null +++ b/src/features/tasks/list/taskSyncAction.ts @@ -0,0 +1,115 @@ +import * as fs from 'node:fs'; +import { execFileSync } from 'node:child_process'; +import { success, error as logError } from '../../../shared/ui/index.js'; +import { createLogger, getErrorMessage } from '../../../shared/utils/index.js'; +import { executeTask } from '../execute/taskExecution.js'; +import { determinePiece } from '../execute/selectAndExecute.js'; +import { DEFAULT_PIECE_NAME } from '../../../shared/constants.js'; +import { type BranchActionTarget, resolveTargetInstruction } from './taskActionTarget.js'; +import type { TaskExecutionOptions } from '../execute/types.js'; + +const log = createLogger('list-tasks'); + +const SYNC_REF = 'refs/remotes/root/sync-target'; + +function buildConflictResolutionInstruction(originalInstruction: string): string { + return `Git merge has stopped due to merge conflicts. + +Resolve all conflicts to complete the merge: +1. Run \`git status\` to identify conflicted files +2. For each conflicted file, resolve the conflict markers + (<<<<<<< HEAD / ======= / >>>>>>> lines) + Preserve changes that align with the original task intent +3. Stage each resolved file: \`git add \` +4. Complete the merge: \`git commit\` + +Original task: +${originalInstruction}`; +} + +export async function syncBranchWithRoot( + projectDir: string, + target: BranchActionTarget, + options?: TaskExecutionOptions, +): Promise { + if (!('kind' in target)) { + throw new Error('Sync requires a task target.'); + } + + if (!target.worktreePath || !fs.existsSync(target.worktreePath)) { + logError(`Worktree directory does not exist for task: ${target.name}`); + return false; + } + const worktreePath = target.worktreePath; + + // origin is removed in worktrees; pass the project path directly as the remote + try { + execFileSync('git', ['fetch', projectDir, `HEAD:${SYNC_REF}`], { + cwd: worktreePath, + encoding: 'utf-8', + stdio: 'pipe', + }); + log.info('Fetched root HEAD into sync-target ref', { worktreePath, projectDir }); + } catch (err) { + const msg = getErrorMessage(err); + logError(`Failed to fetch from root: ${msg}`); + log.error('git fetch failed', { worktreePath, projectDir, error: msg }); + return false; + } + + try { + execFileSync('git', ['merge', SYNC_REF], { + cwd: worktreePath, + encoding: 'utf-8', + stdio: 'pipe', + }); + success('Synced.'); + log.info('Merge succeeded without conflicts', { worktreePath }); + return true; + } catch (err) { + log.info('Merge conflict detected, attempting AI resolution', { + worktreePath, + error: getErrorMessage(err), + }); + } + + const pieceIdentifier = await determinePiece(projectDir, target.data?.piece ?? DEFAULT_PIECE_NAME); + if (!pieceIdentifier) { + abortMerge(worktreePath); + return false; + } + + const originalInstruction = resolveTargetInstruction(target); + const conflictInstruction = buildConflictResolutionInstruction(originalInstruction); + + const aiSuccess = await executeTask({ + task: conflictInstruction, + cwd: worktreePath, + pieceIdentifier, + projectCwd: projectDir, + agentOverrides: options, + }); + + if (aiSuccess) { + success('Conflicts resolved.'); + log.info('AI conflict resolution succeeded', { worktreePath }); + return true; + } + + abortMerge(worktreePath); + logError('Failed to resolve conflicts. Merge aborted.'); + return false; +} + +function abortMerge(worktreePath: string): void { + try { + execFileSync('git', ['merge', '--abort'], { + cwd: worktreePath, + encoding: 'utf-8', + stdio: 'pipe', + }); + log.info('git merge --abort completed', { worktreePath }); + } catch (err) { + log.error('git merge --abort failed', { worktreePath, error: getErrorMessage(err) }); + } +} From 67f6fc685c1d3152bf5d62f61d6927304c2f5ea1 Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Fri, 20 Feb 2026 12:40:17 +0900 Subject: [PATCH 09/17] =?UTF-8?q?fix:=20opencode=E3=81=AE2=E3=82=BF?= =?UTF-8?q?=E3=83=BC=E3=83=B3=E7=9B=AE=E3=83=8F=E3=83=B3=E3=82=B0=E3=82=92?= =?UTF-8?q?=E4=BF=AE=E6=AD=A3=E3=81=97=E4=BC=9A=E8=A9=B1=E7=B6=99=E7=B6=9A?= =?UTF-8?q?=E3=82=92=E5=AE=9F=E7=8F=BE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit streamAbortController.signalをcreateOpencodeに渡していたため、 各callのfinallyでabortするとサーバーが停止し2ターン目がハングしていた。 signalをサーバー起動から除外し、sessionIdの引き継ぎを復元することで 複数ターンの会話継続を実現した。 --- e2e/specs/opencode-conversation.e2e.ts | 99 ++++++++++++ src/__tests__/opencode-client-cleanup.test.ts | 145 ++++++++++++++++-- src/infra/opencode/client.ts | 31 ++-- vitest.config.e2e.provider.ts | 1 + 4 files changed, 244 insertions(+), 32 deletions(-) create mode 100644 e2e/specs/opencode-conversation.e2e.ts diff --git a/e2e/specs/opencode-conversation.e2e.ts b/e2e/specs/opencode-conversation.e2e.ts new file mode 100644 index 0000000..affc279 --- /dev/null +++ b/e2e/specs/opencode-conversation.e2e.ts @@ -0,0 +1,99 @@ +/** + * OpenCode real E2E conversation test. + * + * Tests the full stack with a real OpenCode server: + * OpenCodeProvider → callOpenCode → OpenCodeClient → createOpencode (real server) + * + * Skipped automatically if the opencode binary is not found. + * Run with: npm run test:e2e:opencode + */ + +import { describe, it, expect, afterAll } from 'vitest'; +import { execSync } from 'node:child_process'; +import { resetSharedServer } from '../../src/infra/opencode/client.js'; +import { OpenCodeProvider } from '../../src/infra/providers/opencode.js'; + +function isOpencodeAvailable(): boolean { + try { + execSync('which opencode', { stdio: 'ignore' }); + return true; + } catch { + return false; + } +} + +const MODEL = process.env.OPENCODE_E2E_MODEL ?? 'minimax/MiniMax-M2.5-highspeed'; +const enabled = isOpencodeAvailable(); + +describe.skipIf(!enabled)('OpenCode real E2E conversation', () => { + afterAll(() => { + resetSharedServer(); + }); + + it('should complete a two-turn conversation with sessionId inheritance', async () => { + const provider = new OpenCodeProvider(); + const agent = provider.setup({ + name: 'coder', + systemPrompt: 'You are a concise assistant. Keep all responses under 20 words.', + }); + + // 1ターン目 + const result1 = await agent.call('Say only the word "apple".', { + cwd: process.cwd(), + model: MODEL, + }); + + expect(result1.status).toBe('done'); + expect(result1.sessionId).toBeDefined(); + + // 2ターン目: sessionId を引き継いで送る（conversationLoop と同じ） + const result2 = await agent.call('What fruit did I ask you about?', { + cwd: process.cwd(), + model: MODEL, + sessionId: result1.sessionId, + }); + + expect(result2.status).toBe('done'); + // 同じセッションを再利用している + expect(result2.sessionId).toBe(result1.sessionId); + // 会話が引き継がれていれば "apple" に言及するはず + expect(result2.content.toLowerCase()).toContain('apple'); + }, 120_000); + + it('should complete a three-turn conversation without hanging', async () => { + const provider = new OpenCodeProvider(); + const agent = provider.setup({ + name: 'coder', + systemPrompt: 'You are a concise assistant. Keep all responses under 20 words.', + }); + + const results = []; + let prevSessionId: string | undefined; + + const prompts = [ + 'Remember the number 42.', + 'What number did I ask you to remember?', + 'Double that number.', + ]; + + for (const prompt of prompts) { + const result = await agent.call(prompt, { + cwd: process.cwd(), + model: MODEL, + sessionId: prevSessionId, + }); + + expect(result.status).toBe('done'); + results.push(result); + prevSessionId = result.sessionId; + } + + // すべてのターンが同じセッションを使っている + expect(results[1].sessionId).toBe(results[0].sessionId); + expect(results[2].sessionId).toBe(results[0].sessionId); + + // 会話が引き継がれている + expect(results[1].content).toMatch(/42/); + expect(results[2].content).toMatch(/84/); + }, 180_000); +}); diff --git a/src/__tests__/opencode-client-cleanup.test.ts b/src/__tests__/opencode-client-cleanup.test.ts index ab40943..4adb3d4 100644 --- a/src/__tests__/opencode-client-cleanup.test.ts +++ b/src/__tests__/opencode-client-cleanup.test.ts @@ -95,10 +95,6 @@ describe('OpenCodeClient stream cleanup', () => { expect(result.status).toBe('done'); expect(stream.returnSpy).toHaveBeenCalled(); - expect(disposeInstance).toHaveBeenCalledWith( - { directory: '/tmp' }, - expect.objectContaining({ signal: expect.any(AbortSignal) }), - ); expect(subscribe).toHaveBeenCalledWith( { directory: '/tmp' }, expect.objectContaining({ signal: expect.any(AbortSignal) }), @@ -141,10 +137,6 @@ describe('OpenCodeClient stream cleanup', () => { expect(result.status).toBe('error'); expect(result.content).toContain('boom'); expect(stream.returnSpy).toHaveBeenCalled(); - expect(disposeInstance).toHaveBeenCalledWith( - { directory: '/tmp' }, - expect.objectContaining({ signal: expect.any(AbortSignal) }), - ); expect(subscribe).toHaveBeenCalledWith( { directory: '/tmp' }, expect.objectContaining({ signal: expect.any(AbortSignal) }), @@ -210,10 +202,6 @@ describe('OpenCodeClient stream cleanup', () => { expect(result.status).toBe('done'); expect(result.content).toBe('done more'); - expect(disposeInstance).toHaveBeenCalledWith( - { directory: '/tmp' }, - expect.objectContaining({ signal: expect.any(AbortSignal) }), - ); expect(subscribe).toHaveBeenCalledWith( { directory: '/tmp' }, expect.objectContaining({ signal: expect.any(AbortSignal) }), @@ -615,4 +603,137 @@ describe('OpenCodeClient stream cleanup', () => { expect(result1.status).toBe('done'); expect(result2.status).toBe('done'); }); + +}); + +describe('OpenCode conversation via provider (E2E)', () => { + beforeEach(async () => { + vi.clearAllMocks(); + const { resetSharedServer } = await import('../infra/opencode/client.js'); + resetSharedServer(); + }); + + function makeClientMock(sessionId: string, responses: string[]) { + let turnIndex = 0; + const sessionCreate = vi.fn().mockResolvedValue({ data: { id: sessionId } }); + const promptAsync = vi.fn().mockResolvedValue(undefined); + const subscribe = vi.fn().mockImplementation(() => { + const text = responses[turnIndex] ?? ''; + const events: unknown[] = []; + if (text) { + events.push({ + type: 'message.part.updated', + properties: { part: { id: `p-${turnIndex}`, type: 'text', text }, delta: text }, + }); + } + events.push({ type: 'session.idle', properties: { sessionID: sessionId } }); + turnIndex += 1; + return Promise.resolve({ stream: new MockEventStream(events) }); + }); + return { sessionCreate, promptAsync, subscribe }; + } + + it('should carry sessionId across turns and reuse server', async () => { + const { OpenCodeProvider } = await import('../infra/providers/opencode.js'); + const { resetSharedServer } = await import('../infra/opencode/client.js'); + resetSharedServer(); + + const { sessionCreate, promptAsync, subscribe } = makeClientMock('conv-session', [ + 'Hello!', + 'I remember our conversation.', + ]); + + createOpencodeMock.mockResolvedValue({ + client: { + instance: { dispose: vi.fn() }, + session: { create: sessionCreate, promptAsync }, + event: { subscribe }, + permission: { reply: vi.fn() }, + }, + server: { close: vi.fn() }, + }); + + const provider = new OpenCodeProvider(); + const agent = provider.setup({ name: 'coder', systemPrompt: 'You are a helpful assistant.' }); + + // 1ターン目 + const result1 = await agent.call('Hi', { cwd: '/tmp', model: 'opencode/big-pickle' }); + expect(result1.status).toBe('done'); + expect(result1.content).toBe('Hello!'); + expect(result1.sessionId).toBe('conv-session'); + + // 2ターン目: conversationLoop と同様に前ターンの sessionId を引き継ぐ + const result2 = await agent.call('Do you remember me?', { + cwd: '/tmp', + model: 'opencode/big-pickle', + sessionId: result1.sessionId, + }); + expect(result2.status).toBe('done'); + expect(result2.content).toBe('I remember our conversation.'); + expect(result2.sessionId).toBe('conv-session'); + + // サーバーは1回だけ起動（再利用） + expect(createOpencodeMock).toHaveBeenCalledTimes(1); + // sessionId を引き継いだので session.create は1回だけ + expect(sessionCreate).toHaveBeenCalledTimes(1); + // 両ターンでプロンプトが送られた + expect(promptAsync).toHaveBeenCalledTimes(2); + expect(subscribe).toHaveBeenCalledTimes(2); + }); + + it('should carry sessionId across three turns (multi-turn conversation)', async () => { + const { OpenCodeProvider } = await import('../infra/providers/opencode.js'); + const { resetSharedServer } = await import('../infra/opencode/client.js'); + resetSharedServer(); + + const { sessionCreate, promptAsync, subscribe } = makeClientMock('multi-session', [ + 'Turn 1 response', + 'Turn 2 response', + 'Turn 3 response', + ]); + + createOpencodeMock.mockResolvedValue({ + client: { + instance: { dispose: vi.fn() }, + session: { create: sessionCreate, promptAsync }, + event: { subscribe }, + permission: { reply: vi.fn() }, + }, + server: { close: vi.fn() }, + }); + + const provider = new OpenCodeProvider(); + const agent = provider.setup({ name: 'coder' }); + + const results = []; + let prevSessionId: string | undefined; + + for (let i = 0; i < 3; i++) { + const result = await agent.call(`message ${i + 1}`, { + cwd: '/tmp', + model: 'opencode/big-pickle', + sessionId: prevSessionId, + }); + results.push(result); + prevSessionId = result.sessionId; + } + + expect(results[0].status).toBe('done'); + expect(results[1].status).toBe('done'); + expect(results[2].status).toBe('done'); + expect(results[0].content).toBe('Turn 1 response'); + expect(results[1].content).toBe('Turn 2 response'); + expect(results[2].content).toBe('Turn 3 response'); + + // サーバーは1回だけ起動 + expect(createOpencodeMock).toHaveBeenCalledTimes(1); + // sessionId を引き継いでいるので session.create は1回のみ + expect(sessionCreate).toHaveBeenCalledTimes(1); + // 3ターン分のプロンプトが送られた + expect(promptAsync).toHaveBeenCalledTimes(3); + // すべてのターンで同じ sessionId + expect(results[0].sessionId).toBe('multi-session'); + expect(results[1].sessionId).toBe('multi-session'); + expect(results[2].sessionId).toBe('multi-session'); + }); }); diff --git a/src/infra/opencode/client.ts b/src/infra/opencode/client.ts index a0cf429..748912a 100644 --- a/src/infra/opencode/client.ts +++ b/src/infra/opencode/client.ts @@ -62,7 +62,7 @@ interface SharedServer { let sharedServer: SharedServer | null = null; let initPromise: Promise | null = null; -async function acquireClient(model: string, apiKey?: string, signal?: AbortSignal): Promise<{ client: OpencodeClient; release: () => void }> { +async function acquireClient(model: string, apiKey?: string): Promise<{ client: OpencodeClient; release: () => void }> { if (initPromise) { await initPromise; } @@ -85,7 +85,6 @@ async function acquireClient(model: string, apiKey?: string, signal?: AbortSigna const port = await getFreePort(); const { client, server } = await createOpencode({ port, - signal, config: { model, small_model: model, @@ -94,7 +93,15 @@ async function acquireClient(model: string, apiKey?: string, signal?: AbortSigna timeout: OPENCODE_SERVER_START_TIMEOUT_MS, }); - sharedServer = { client, close: server.close, model, apiKey, queue: [] }; + const closeServer = (): void => { + try { + server.close(); + } catch { + // Ignore close errors + } + }; + + sharedServer = { client, close: closeServer, model, apiKey, queue: [] }; log.debug('OpenCode server started', { model, port }); return { client, release: () => releaseClient() }; @@ -380,7 +387,7 @@ export class OpenCodeClient { const parsedModel = parseProviderModel(options.model, 'OpenCode model'); const fullModel = `${parsedModel.providerID}/${parsedModel.modelID}`; - const acquired = await acquireClient(fullModel, options.opencodeApiKey, streamAbortController.signal); + const acquired = await acquireClient(fullModel, options.opencodeApiKey); opencodeApiClient = acquired.client; release = acquired.release; @@ -707,22 +714,6 @@ export class OpenCodeClient { if (options.abortSignal) { options.abortSignal.removeEventListener('abort', onExternalAbort); } - if (opencodeApiClient) { - const disposeAbortController = new AbortController(); - const disposeTimeoutId = setTimeout(() => { - disposeAbortController.abort(); - }, 3000); - try { - await opencodeApiClient.instance.dispose( - { directory: options.cwd }, - { signal: disposeAbortController.signal }, - ); - } catch { - // Ignore dispose errors during cleanup. - } finally { - clearTimeout(disposeTimeoutId); - } - } release?.(); if (!streamAbortController.signal.aborted) { streamAbortController.abort(); diff --git a/vitest.config.e2e.provider.ts b/vitest.config.e2e.provider.ts index a35d436..8ca9ea4 100644 --- a/vitest.config.e2e.provider.ts +++ b/vitest.config.e2e.provider.ts @@ -10,6 +10,7 @@ export default defineConfig({ 'e2e/specs/pipeline.e2e.ts', 'e2e/specs/github-issue.e2e.ts', 'e2e/specs/structured-output.e2e.ts', + 'e2e/specs/opencode-conversation.e2e.ts', ], }, }); From 291e05a24de1eabd4ad5fdc72a4ba37faca256d1 Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Fri, 20 Feb 2026 12:43:40 +0900 Subject: [PATCH 10/17] fix: prevent romaji conversion stack overflow on long task names --- src/__tests__/summarize.test.ts | 32 +++++++++++++++++++++----------- src/infra/task/summarize.ts | 23 ++++++++++++++++++++++- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/__tests__/summarize.test.ts b/src/__tests__/summarize.test.ts index e4107d1..3313897 100644 --- a/src/__tests__/summarize.test.ts +++ b/src/__tests__/summarize.test.ts @@ -239,17 +239,27 @@ describe('summarizeTaskName', () => { expect(result.length).toBeLessThanOrEqual(30); }); - it('should handle mixed Japanese/English with romanization', async () => { - // When - const result = await summarizeTaskName('Add romanization', { cwd: '/project', useLLM: false }); - - // Then - expect(result).toMatch(/^[a-z0-9-]+$/); - expect(result).not.toMatch(/^-|-$/); // No leading/trailing hyphens - }); - - it('should use romaji by default', async () => { - // Given: branchNameStrategy is not set (undefined) + it('should handle mixed Japanese/English with romanization', async () => { + // When + const result = await summarizeTaskName('Add romanization', { cwd: '/project', useLLM: false }); + + // Then + expect(result).toMatch(/^[a-z0-9-]+$/); + expect(result).not.toMatch(/^-|-$/); // No leading/trailing hyphens + }); + + it('should handle very long names in romanization mode without stack overflow', async () => { + const result = await summarizeTaskName('a'.repeat(12000), { + cwd: '/project', + useLLM: false, + }); + + expect(result).toBe('a'.repeat(30)); + expect(mockProviderCall).not.toHaveBeenCalled(); + }); + + it('should use romaji by default', async () => { + // Given: branchNameStrategy is not set (undefined) mockResolveConfigValues.mockReturnValue({ provider: 'claude', model: undefined, diff --git a/src/infra/task/summarize.ts b/src/infra/task/summarize.ts index 662b2fb..01f53c9 100644 --- a/src/infra/task/summarize.ts +++ b/src/infra/task/summarize.ts @@ -14,12 +14,33 @@ import type { SummarizeOptions } from './types.js'; export type { SummarizeOptions }; const log = createLogger('summarize'); +const MAX_ROMAJI_CHUNK_SIZE = 1024; + +function toRomajiSafely(text: string): string { + const romajiOptions = { customRomajiMapping: {} }; + try { + if (text.length <= MAX_ROMAJI_CHUNK_SIZE) { + return wanakana.toRomaji(text, romajiOptions); + } + const convertedChunks: string[] = []; + for (let i = 0; i < text.length; i += MAX_ROMAJI_CHUNK_SIZE) { + convertedChunks.push( + wanakana.toRomaji(text.slice(i, i + MAX_ROMAJI_CHUNK_SIZE), romajiOptions), + ); + } + return convertedChunks.join(''); + } catch { + // Avoid blocking branch/task creation on rare parser edge cases or deep recursion + // with very long mixed/ASCII inputs. + return text; + } +} /** * Convert Japanese text to romaji slug. */ function toRomajiSlug(text: string): string { - const romaji = wanakana.toRomaji(text, { customRomajiMapping: {} }); + const romaji = toRomajiSafely(text); return slugify(romaji); } From 192077cea8d76f5f31f215209d741e64df79351a Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Fri, 20 Feb 2026 13:49:18 +0900 Subject: [PATCH 11/17] =?UTF-8?q?ci:=20=E4=BE=9D=E5=AD=98=E3=83=91?= =?UTF-8?q?=E3=83=83=E3=82=B1=E3=83=BC=E3=82=B8=E3=81=AE=E7=A0=B4=E6=90=8D?= =?UTF-8?q?=E3=82=92=E6=A4=9C=E7=9F=A5=E3=81=99=E3=82=8B=E5=AE=9A=E6=9C=9F?= =?UTF-8?q?=E3=83=81=E3=82=A7=E3=83=83=E3=82=AF=E3=82=92=E8=BF=BD=E5=8A=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/dependency-check.yml | 47 ++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 .github/workflows/dependency-check.yml diff --git a/.github/workflows/dependency-check.yml b/.github/workflows/dependency-check.yml new file mode 100644 index 0000000..4ea0f20 --- /dev/null +++ b/.github/workflows/dependency-check.yml @@ -0,0 +1,47 @@ +name: Dependency Health Check + +on: + schedule: + - cron: '0 0 * * *' + workflow_dispatch: + +jobs: + fresh-install: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: '20' + + - name: Install without lockfile + run: | + rm package-lock.json + npm install + + - name: Build + run: npm run build + + - name: Verify CLI startup + run: node bin/takt --version + + - name: Notify Slack on failure + if: failure() + uses: slackapi/slack-github-action@v2.0.0 + with: + webhook-type: incoming-webhook + webhook: ${{ secrets.SLACK_WEBHOOK_URL }} + payload: | + { + "text": "⚠️ Dependency health check failed", + "blocks": [ + { + "type": "section", + "text": { + "type": "mrkdwn", + "text": "*⚠️ Dependency Health Check Failed*\nA dependency may have published a broken version.\n<${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}|View logs>" + } + } + ] + } From 6ee28e63a90d6ec6e57ce49d7d0ef7e7f171be22 Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Fri, 20 Feb 2026 18:47:01 +0900 Subject: [PATCH 12/17] Release v0.21.0 --- CHANGELOG.md | 23 +++++++++++++++++++++++ docs/CHANGELOG.ja.md | 23 +++++++++++++++++++++++ package.json | 2 +- 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff05686..39fbd62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,29 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). +## [0.21.0] - 2026-02-20 + +### Added + +- **Slack task notification enhancements**: Extended Slack webhook notifications with richer task context and formatting (#316) +- **`takt list --delete-all` option**: Delete all tasks at once from the task list (#322) +- **`--draft-pr` option**: Create pull requests as drafts via `--draft-pr` flag (#323) +- **`--sync-with-root` option**: Sync worktree branch with root repository changes (#325) +- **Model per persona-provider**: Allow specifying model overrides at the persona-provider level (#324) +- **Analytics project config and env override**: Analytics settings can now be configured per-project and overridden via environment variables +- **CI dependency health check**: Periodic CI check to detect broken dependency packages + +### Changed + +- **Config system overhaul**: Replaced `loadConfig()` bulk merge with per-key `resolveConfigValue()` resolution — global < piece < project < env priority with source tracking and `OptionsBuilder` merge direction control (#324) + +### Fixed + +- **Retry command scope and messaging**: Fixed retry command to show correct available range and guidance text +- **Retry task `completed_at` leak**: Clear `completed_at` when moving a failed task back to running via `startReExecution`, preventing Zod validation errors +- **OpenCode multi-turn hang**: Removed `streamAbortController.signal` from OpenCode server startup so subsequent turns no longer hang; restored `sessionId` carry-over for multi-turn conversations +- **Romaji conversion stack overflow**: Prevented stack overflow on long task names during romaji conversion + ## [0.20.0] - 2026-02-19 ### Added diff --git a/docs/CHANGELOG.ja.md b/docs/CHANGELOG.ja.md index 8a67ad0..cd4d626 100644 --- a/docs/CHANGELOG.ja.md +++ b/docs/CHANGELOG.ja.md @@ -6,6 +6,29 @@ フォーマットは [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) に基づいています。 +## [0.21.0] - 2026-02-20 + +### Added + +- **Slack タスク通知の拡張**: Slack Webhook 通知にリッチなタスクコンテキストとフォーマットを追加 (#316) +- **`takt list --delete-all` オプション**: タスクリストから全タスクを一括削除 (#322) +- **`--draft-pr` オプション**: `--draft-pr` フラグでドラフト PR を作成可能に (#323) +- **`--sync-with-root` オプション**: ワークツリーブランチをルートリポジトリの変更と同期 (#325) +- **ペルソナプロバイダーごとのモデル指定**: persona-provider レベルでモデルオーバーライドを指定可能に (#324) +- **Analytics のプロジェクト設定・環境変数オーバーライド対応**: Analytics 設定をプロジェクトごとに設定し、環境変数で上書き可能に +- **CI 依存パッケージヘルスチェック**: 依存パッケージの破損を検知する定期 CI チェックを追加 + +### Changed + +- **設定システムの刷新**: `loadConfig()` による一括マージを廃止し、`resolveConfigValue()` によるキー単位解決に移行 — global < piece < project < env の優先順位でソーストラッキングと `OptionsBuilder` のマージ方向を制御 (#324) + +### Fixed + +- **retry コマンドの有効範囲と案内文を修正**: 正しい範囲と案内テキストを表示するよう修正 +- **retry タスクの `completed_at` クリア漏れ**: `startReExecution` で失敗タスクを running に戻す際、`completed_at` を null にリセットするよう修正（Zod バリデーションエラーを防止） +- **OpenCode の2ターン目ハング修正**: `streamAbortController.signal` をサーバー起動から除外し、`sessionId` の引き継ぎを復元することで複数ターンの会話継続を実現 +- **ローマ字変換のスタックオーバーフロー防止**: 長いタスク名でのローマ字変換時にスタックオーバーフローが発生する問題を修正 + ## [0.20.0] - 2026-02-19 ### Added diff --git a/package.json b/package.json index 5c9e96d..7e83176 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "takt", - "version": "0.20.0", + "version": "0.21.0", "description": "TAKT: TAKT Agent Koordination Topology - AI Agent Piece Orchestration", "main": "dist/index.js", "types": "dist/index.d.ts", From eda5f3d2e32b1acb8602650d9694b338b894d34a Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Fri, 20 Feb 2026 19:33:18 +0900 Subject: [PATCH 13/17] fix: clear TAKT_CONFIG_DIR in vitest config to isolate tests from host environment --- vitest.config.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/vitest.config.ts b/vitest.config.ts index 4c0ab1b..918e5bf 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -2,6 +2,9 @@ import { defineConfig } from 'vitest/config'; export default defineConfig({ test: { + env: { + TAKT_CONFIG_DIR: '', + }, include: ['src/__tests__/**/*.test.ts'], environment: 'node', globals: false, From 01b68d110418b50210182abb2834764374fdd5b4 Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Fri, 20 Feb 2026 19:50:58 +0900 Subject: [PATCH 14/17] chore: update package-lock.json for v0.21.0 --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 2909798..3cb1738 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,17 +1,17 @@ { "name": "takt", - "version": "0.20.0", + "version": "0.21.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "takt", - "version": "0.20.0", + "version": "0.21.0", "license": "MIT", "dependencies": { "@anthropic-ai/claude-agent-sdk": "^0.2.47", "@openai/codex-sdk": "^0.103.0", - "@opencode-ai/sdk": "^1.1.53", + "@opencode-ai/sdk": ">=1.1.53 <1.2.7", "chalk": "^5.3.0", "commander": "^12.1.0", "update-notifier": "^7.3.1", From 52fb385e753d4d66d61d489d30cf42d89f93fbb4 Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Fri, 20 Feb 2026 19:58:13 +0900 Subject: [PATCH 15/17] docs: add --draft-pr, --delete-all, sync-with-root to CLI reference --- docs/cli-reference.ja.md | 6 ++++++ docs/cli-reference.md | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/docs/cli-reference.ja.md b/docs/cli-reference.ja.md index 8f195ca..a2f28f6 100644 --- a/docs/cli-reference.ja.md +++ b/docs/cli-reference.ja.md @@ -14,6 +14,7 @@ | `-w, --piece ` | Piece 名または piece YAML ファイルのパス | | `-b, --branch ` | ブランチ名を指定（省略時は自動生成） | | `--auto-pr` | PR を作成（インタラクティブ: 確認スキップ、pipeline: PR 有効化） | +| `--draft-pr` | PR をドラフトとして作成 | | `--skip-git` | ブランチ作成、コミット、プッシュをスキップ（pipeline モード、piece のみ実行） | | `--repo ` | リポジトリを指定（PR 作成用） | | `--create-worktree ` | worktree 確認プロンプトをスキップ | @@ -169,6 +170,9 @@ takt watch # タスクブランチの一覧表示（マージ/削除） takt list +# 全タスクを一括削除 +takt list --delete-all + # 非インタラクティブモード（CI/スクリプト向け） takt list --non-interactive takt list --non-interactive --action diff --branch takt/my-branch @@ -176,6 +180,8 @@ takt list --non-interactive --action delete --branch takt/my-branch --yes takt list --non-interactive --format json ``` +インタラクティブモードでは **Sync with root** アクションも利用可能です。ルートリポジトリの HEAD をワークツリーブランチにマージし、コンフリクトが発生した場合は AI が自動解決を試みます。 + ### タスクディレクトリワークフロー（作成 / 実行 / 確認） 1. `takt add` を実行し、`.takt/tasks.yaml` に pending レコードが作成されたことを確認。 diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 22c4cb3..6f40c97 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -14,6 +14,7 @@ This document provides a complete reference for all TAKT CLI commands and option | `-w, --piece ` | Piece name or path to piece YAML file | | `-b, --branch ` | Specify branch name (auto-generated if omitted) | | `--auto-pr` | Create PR (interactive: skip confirmation, pipeline: enable PR) | +| `--draft-pr` | Create PR as draft | | `--skip-git` | Skip branch creation, commit, and push (pipeline mode, piece-only) | | `--repo ` | Specify repository (for PR creation) | | `--create-worktree ` | Skip worktree confirmation prompt | @@ -169,6 +170,9 @@ List task branches and perform actions (merge, delete, etc.). # List task branches (merge/delete) takt list +# Delete all tasks at once +takt list --delete-all + # Non-interactive mode (for CI/scripts) takt list --non-interactive takt list --non-interactive --action diff --branch takt/my-branch @@ -176,6 +180,8 @@ takt list --non-interactive --action delete --branch takt/my-branch --yes takt list --non-interactive --format json ``` +In interactive mode, `takt list` also offers a **Sync with root** action to merge the root repository's HEAD into a worktree branch, with AI-assisted conflict resolution. + ### Task Directory Workflow (Create / Run / Verify) 1. Run `takt add` and confirm a pending record is created in `.takt/tasks.yaml`. From dbdaf9349899e56d18843da44ffe45a167dc796c Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Fri, 20 Feb 2026 19:59:22 +0900 Subject: [PATCH 16/17] docs: revert delete-all and sync-with-root from CLI reference --- docs/cli-reference.ja.md | 5 ----- docs/cli-reference.md | 5 ----- 2 files changed, 10 deletions(-) diff --git a/docs/cli-reference.ja.md b/docs/cli-reference.ja.md index a2f28f6..f1646dd 100644 --- a/docs/cli-reference.ja.md +++ b/docs/cli-reference.ja.md @@ -170,9 +170,6 @@ takt watch # タスクブランチの一覧表示（マージ/削除） takt list -# 全タスクを一括削除 -takt list --delete-all - # 非インタラクティブモード（CI/スクリプト向け） takt list --non-interactive takt list --non-interactive --action diff --branch takt/my-branch @@ -180,8 +177,6 @@ takt list --non-interactive --action delete --branch takt/my-branch --yes takt list --non-interactive --format json ``` -インタラクティブモードでは **Sync with root** アクションも利用可能です。ルートリポジトリの HEAD をワークツリーブランチにマージし、コンフリクトが発生した場合は AI が自動解決を試みます。 - ### タスクディレクトリワークフロー（作成 / 実行 / 確認） 1. `takt add` を実行し、`.takt/tasks.yaml` に pending レコードが作成されたことを確認。 diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 6f40c97..ff41a22 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -170,9 +170,6 @@ List task branches and perform actions (merge, delete, etc.). # List task branches (merge/delete) takt list -# Delete all tasks at once -takt list --delete-all - # Non-interactive mode (for CI/scripts) takt list --non-interactive takt list --non-interactive --action diff --branch takt/my-branch @@ -180,8 +177,6 @@ takt list --non-interactive --action delete --branch takt/my-branch --yes takt list --non-interactive --format json ``` -In interactive mode, `takt list` also offers a **Sync with root** action to merge the root repository's HEAD into a worktree branch, with AI-assisted conflict resolution. - ### Task Directory Workflow (Create / Run / Verify) 1. Run `takt add` and confirm a pending record is created in `.takt/tasks.yaml`. From 3af30e9e18c0cbd6771c807133098c5c49c55e2b Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Fri, 20 Feb 2026 19:59:55 +0900 Subject: [PATCH 17/17] docs: add sync-with-root menu description to CLI reference --- docs/cli-reference.ja.md | 4 +++- docs/cli-reference.md | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/cli-reference.ja.md b/docs/cli-reference.ja.md index f1646dd..9a91927 100644 --- a/docs/cli-reference.ja.md +++ b/docs/cli-reference.ja.md @@ -164,7 +164,7 @@ takt watch ### takt list -タスクブランチの一覧表示と操作（マージ、削除など）を行います。 +タスクブランチの一覧表示と操作（マージ、削除、ルートとの同期など）を行います。 ```bash # タスクブランチの一覧表示（マージ/削除） @@ -177,6 +177,8 @@ takt list --non-interactive --action delete --branch takt/my-branch --yes takt list --non-interactive --format json ``` +インタラクティブモードでは **Sync with root** を選択でき、ルートリポジトリの HEAD をワークツリーブランチにマージします。コンフリクト発生時は AI が自動解決を試みます。 + ### タスクディレクトリワークフロー（作成 / 実行 / 確認） 1. `takt add` を実行し、`.takt/tasks.yaml` に pending レコードが作成されたことを確認。 diff --git a/docs/cli-reference.md b/docs/cli-reference.md index ff41a22..2902a43 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -164,7 +164,7 @@ takt watch ### takt list -List task branches and perform actions (merge, delete, etc.). +List task branches and perform actions (merge, delete, sync with root, etc.). ```bash # List task branches (merge/delete) @@ -177,6 +177,8 @@ takt list --non-interactive --action delete --branch takt/my-branch --yes takt list --non-interactive --format json ``` +In interactive mode, **Sync with root** merges the root repository HEAD into the worktree branch with AI-assisted conflict resolution. + ### Task Directory Workflow (Create / Run / Verify) 1. Run `takt add` and confirm a pending record is created in `.takt/tasks.yaml`.