From 4c0b3c1593113b2086c2cab2ccd4c884e7f81d5c Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Fri, 6 Feb 2026 18:05:19 +0900 Subject: [PATCH] takt: github-issue-98-pr-no-wo-ni-sh --- src/__tests__/globalConfig-defaults.test.ts | 43 +++++++ src/__tests__/saveTaskFile.test.ts | 19 +++ src/__tests__/taskExecution.test.ts | 116 ++++++++++++++++++ src/core/models/global-config.ts | 2 + src/core/models/schemas.ts | 2 + src/features/tasks/add/index.ts | 13 +- .../tasks/execute/selectAndExecute.ts | 67 +++++++--- src/features/tasks/execute/taskExecution.ts | 39 +++++- src/infra/config/global/globalConfig.ts | 4 + src/infra/task/schema.ts | 2 + 10 files changed, 284 insertions(+), 23 deletions(-) diff --git a/src/__tests__/globalConfig-defaults.test.ts b/src/__tests__/globalConfig-defaults.test.ts index e120d0e..9854205 100644 --- a/src/__tests__/globalConfig-defaults.test.ts +++ b/src/__tests__/globalConfig-defaults.test.ts @@ -121,6 +121,49 @@ describe('loadGlobalConfig', () => { expect(reloaded.pipeline!.commitMessageTemplate).toBe('feat: {title} (#{issue})'); }); + it('should load auto_pr config from config.yaml', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + 'language: en\nauto_pr: true\n', + 'utf-8', + ); + + const config = loadGlobalConfig(); + + expect(config.autoPr).toBe(true); + }); + + it('should save and reload auto_pr config', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + // Create minimal config first + writeFileSync(getGlobalConfigPath(), 'language: en\n', 'utf-8'); + + const config = loadGlobalConfig(); + config.autoPr = true; + saveGlobalConfig(config); + invalidateGlobalConfigCache(); + + const reloaded = loadGlobalConfig(); + expect(reloaded.autoPr).toBe(true); + }); + + it('should save auto_pr: false when explicitly set', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync(getGlobalConfigPath(), 'language: en\n', 'utf-8'); + + const config = loadGlobalConfig(); + config.autoPr = false; + saveGlobalConfig(config); + invalidateGlobalConfigCache(); + + const reloaded = loadGlobalConfig(); + expect(reloaded.autoPr).toBe(false); + }); + it('should read from cache without hitting disk on second call', () => { const taktDir = join(testHomeDir, '.takt'); mkdirSync(taktDir, { recursive: true }); diff --git a/src/__tests__/saveTaskFile.test.ts b/src/__tests__/saveTaskFile.test.ts index 6c087ad..5111656 100644 --- a/src/__tests__/saveTaskFile.test.ts +++ b/src/__tests__/saveTaskFile.test.ts @@ -121,6 +121,25 @@ describe('saveTaskFile', () => { expect(content).not.toContain('issue:'); expect(content).not.toContain('worktree:'); expect(content).not.toContain('branch:'); + expect(content).not.toContain('auto_pr:'); + }); + + it('should include auto_pr in YAML when specified', async () => { + // When + const filePath = await saveTaskFile(testDir, 'Task', { autoPr: true }); + + // Then + const content = fs.readFileSync(filePath, 'utf-8'); + expect(content).toContain('auto_pr: true'); + }); + + it('should include auto_pr: false in YAML when specified as false', async () => { + // When + const filePath = await saveTaskFile(testDir, 'Task', { autoPr: false }); + + // Then + const content = fs.readFileSync(filePath, 'utf-8'); + expect(content).toContain('auto_pr: false'); }); it('should use first line for filename generation', async () => { diff --git a/src/__tests__/taskExecution.test.ts b/src/__tests__/taskExecution.test.ts index 735cb38..89212c7 100644 --- a/src/__tests__/taskExecution.test.ts +++ b/src/__tests__/taskExecution.test.ts @@ -11,6 +11,9 @@ vi.mock('../infra/config/index.js', () => ({ loadGlobalConfig: vi.fn(() => ({})), })); +import { loadGlobalConfig } from '../infra/config/index.js'; +const mockLoadGlobalConfig = vi.mocked(loadGlobalConfig); + vi.mock('../infra/task/index.js', async (importOriginal) => ({ ...(await importOriginal>()), TaskRunner: vi.fn(), @@ -280,4 +283,117 @@ describe('resolveTaskExecution', () => { 'Clone created: /project/../20260128-info-task (branch: takt/20260128-info-task)' ); }); + + it('should return autoPr from task YAML when specified', async () => { + // Given: Task with auto_pr option + const task: TaskInfo = { + name: 'task-with-auto-pr', + content: 'Task content', + filePath: '/tasks/task.yaml', + data: { + task: 'Task content', + auto_pr: true, + }, + }; + + // When + const result = await resolveTaskExecution(task, '/project', 'default'); + + // Then + expect(result.autoPr).toBe(true); + }); + + it('should return autoPr: false from task YAML when specified as false', async () => { + // Given: Task with auto_pr: false + const task: TaskInfo = { + name: 'task-no-auto-pr', + content: 'Task content', + filePath: '/tasks/task.yaml', + data: { + task: 'Task content', + auto_pr: false, + }, + }; + + // When + const result = await resolveTaskExecution(task, '/project', 'default'); + + // Then + expect(result.autoPr).toBe(false); + }); + + it('should fall back to global config autoPr when task YAML does not specify', async () => { + // Given: Task without auto_pr, global config has autoPr + mockLoadGlobalConfig.mockReturnValue({ + language: 'en', + defaultPiece: 'default', + logLevel: 'info', + autoPr: true, + }); + + const task: TaskInfo = { + name: 'task-no-auto-pr-setting', + content: 'Task content', + filePath: '/tasks/task.yaml', + data: { + task: 'Task content', + }, + }; + + // When + const result = await resolveTaskExecution(task, '/project', 'default'); + + // Then + expect(result.autoPr).toBe(true); + }); + + it('should return undefined autoPr when neither task nor config specifies', async () => { + // Given: Neither task nor config has autoPr + mockLoadGlobalConfig.mockReturnValue({ + language: 'en', + defaultPiece: 'default', + logLevel: 'info', + }); + + const task: TaskInfo = { + name: 'task-default', + content: 'Task content', + filePath: '/tasks/task.yaml', + data: { + task: 'Task content', + }, + }; + + // When + const result = await resolveTaskExecution(task, '/project', 'default'); + + // Then + expect(result.autoPr).toBeUndefined(); + }); + + it('should prioritize task YAML auto_pr over global config', async () => { + // Given: Task has auto_pr: false, global config has autoPr: true + mockLoadGlobalConfig.mockReturnValue({ + language: 'en', + defaultPiece: 'default', + logLevel: 'info', + autoPr: true, + }); + + const task: TaskInfo = { + name: 'task-override', + content: 'Task content', + filePath: '/tasks/task.yaml', + data: { + task: 'Task content', + auto_pr: false, + }, + }; + + // When + const result = await resolveTaskExecution(task, '/project', 'default'); + + // Then + expect(result.autoPr).toBe(false); + }); }); diff --git a/src/core/models/global-config.ts b/src/core/models/global-config.ts index e0c49d5..bd1976f 100644 --- a/src/core/models/global-config.ts +++ b/src/core/models/global-config.ts @@ -43,6 +43,8 @@ export interface GlobalConfig { debug?: DebugConfig; /** Directory for shared clones (worktree_dir in config). If empty, uses ../{clone-name} relative to project */ worktreeDir?: string; + /** Auto-create PR after worktree execution (default: prompt in interactive mode) */ + autoPr?: boolean; /** List of builtin piece/agent names to exclude from fallback loading */ disabledBuiltins?: string[]; /** Enable builtin pieces from resources/global/{lang}/pieces */ diff --git a/src/core/models/schemas.ts b/src/core/models/schemas.ts index 8000ca9..c3645b0 100644 --- a/src/core/models/schemas.ts +++ b/src/core/models/schemas.ts @@ -252,6 +252,8 @@ export const GlobalConfigSchema = z.object({ debug: DebugConfigSchema.optional(), /** Directory for shared clones (worktree_dir in config). If empty, uses ../{clone-name} relative to project */ worktree_dir: z.string().optional(), + /** Auto-create PR after worktree execution (default: prompt in interactive mode) */ + auto_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 resources/global/{lang}/pieces */ diff --git a/src/features/tasks/add/index.ts b/src/features/tasks/add/index.ts index d90788d..12a84e5 100644 --- a/src/features/tasks/add/index.ts +++ b/src/features/tasks/add/index.ts @@ -43,7 +43,7 @@ async function generateFilename(tasksDir: string, taskContent: string, cwd: stri export async function saveTaskFile( cwd: string, taskContent: string, - options?: { piece?: string; issue?: number; worktree?: boolean | string; branch?: string }, + options?: { piece?: string; issue?: number; worktree?: boolean | string; branch?: string; autoPr?: boolean }, ): Promise { const tasksDir = path.join(cwd, '.takt', 'tasks'); fs.mkdirSync(tasksDir, { recursive: true }); @@ -57,6 +57,7 @@ export async function saveTaskFile( ...(options?.branch && { branch: options.branch }), ...(options?.piece && { piece: options.piece }), ...(options?.issue !== undefined && { issue: options.issue }), + ...(options?.autoPr !== undefined && { auto_pr: options.autoPr }), }; const filePath = path.join(tasksDir, filename); @@ -165,9 +166,10 @@ export async function addTask(cwd: string, task?: string): Promise { taskContent = result.task; } - // 3. ワークツリー/ブランチ設定 + // 3. ワークツリー/ブランチ/PR設定 let worktree: boolean | string | undefined; let branch: string | undefined; + let autoPr: boolean | undefined; const useWorktree = await confirm('Create worktree?', true); if (useWorktree) { @@ -178,6 +180,9 @@ export async function addTask(cwd: string, task?: string): Promise { if (customBranch) { branch = customBranch; } + + // PR確認（worktreeが有効な場合のみ） + autoPr = await confirm('Auto-create PR?', false); } // 4. YAMLファイル作成 @@ -186,6 +191,7 @@ export async function addTask(cwd: string, task?: string): Promise { issue: issueNumber, worktree, branch, + autoPr, }); const filename = path.basename(filePath); @@ -197,6 +203,9 @@ export async function addTask(cwd: string, task?: string): Promise { if (branch) { info(` Branch: ${branch}`); } + if (autoPr) { + info(` Auto-PR: yes`); + } if (piece) { info(` Piece: ${piece}`); } diff --git a/src/features/tasks/execute/selectAndExecute.ts b/src/features/tasks/execute/selectAndExecute.ts index ca0d811..061a810 100644 --- a/src/features/tasks/execute/selectAndExecute.ts +++ b/src/features/tasks/execute/selectAndExecute.ts @@ -14,13 +14,14 @@ import { loadAllPiecesWithSources, getPieceCategories, buildCategorizedPieces, + loadGlobalConfig, } from '../../../infra/config/index.js'; import { confirm } from '../../../shared/prompt/index.js'; import { createSharedClone, autoCommitAndPush, summarizeTaskName } from '../../../infra/task/index.js'; import { DEFAULT_PIECE_NAME } from '../../../shared/constants.js'; import { info, error, success } from '../../../shared/ui/index.js'; import { createLogger } from '../../../shared/utils/index.js'; -import { createPullRequest, buildPrBody } from '../../../infra/github/index.js'; +import { createPullRequest, buildPrBody, pushBranch } from '../../../infra/github/index.js'; import { executeTask } from './taskExecution.js'; import type { TaskExecutionOptions, WorktreeConfirmationResult, SelectAndExecuteOptions } from './types.js'; import { @@ -122,6 +123,26 @@ export async function confirmAndCreateWorktree( return { execCwd: result.path, isWorktree: true, branch: result.branch }; } +/** + * Resolve auto-PR setting with priority: CLI option > config > prompt. + * Only applicable when worktree is enabled. + */ +async function resolveAutoPr(optionAutoPr: boolean | undefined): Promise { + // CLI option takes precedence + if (typeof optionAutoPr === 'boolean') { + return optionAutoPr; + } + + // Check global config + const globalConfig = loadGlobalConfig(); + if (typeof globalConfig.autoPr === 'boolean') { + return globalConfig.autoPr; + } + + // Fall back to interactive prompt + return confirm('Create pull request?', false); +} + /** * Execute a task with piece selection, optional worktree, and auto-commit. * Shared by direct task execution and interactive mode. @@ -145,7 +166,13 @@ export async function selectAndExecuteTask( options?.createWorktree, ); - log.info('Starting task execution', { piece: pieceIdentifier, worktree: isWorktree }); + // Ask for PR creation BEFORE execution (only if worktree is enabled) + let shouldCreatePr = false; + if (isWorktree) { + shouldCreatePr = await resolveAutoPr(options?.autoPr); + } + + log.info('Starting task execution', { piece: pieceIdentifier, worktree: isWorktree, autoPr: shouldCreatePr }); const taskSuccess = await executeTask({ task, cwd: execCwd, @@ -164,22 +191,26 @@ export async function selectAndExecuteTask( error(`Auto-commit failed: ${commitResult.message}`); } - if (commitResult.success && commitResult.commitHash && branch) { - const shouldCreatePr = options?.autoPr === true || await confirm('Create pull request?', false); - if (shouldCreatePr) { - info('Creating pull request...'); - const prBody = buildPrBody(options?.issues, `Piece \`${pieceIdentifier}\` completed successfully.`); - const prResult = createPullRequest(execCwd, { - branch, - title: task.length > 100 ? `${task.slice(0, 97)}...` : task, - body: prBody, - repo: options?.repo, - }); - if (prResult.success) { - success(`PR created: ${prResult.url}`); - } else { - error(`PR creation failed: ${prResult.error}`); - } + if (commitResult.success && commitResult.commitHash && branch && shouldCreatePr) { + info('Creating pull request...'); + // Push branch from project cwd to origin (clone's origin is removed after shared clone) + try { + pushBranch(cwd, branch); + } catch (pushError) { + // Branch may already be pushed by autoCommitAndPush, continue to PR creation + log.info('Branch push from project cwd failed (may already exist)', { error: pushError }); + } + const prBody = buildPrBody(options?.issues, `Piece \`${pieceIdentifier}\` completed successfully.`); + const prResult = createPullRequest(cwd, { + branch, + title: task.length > 100 ? `${task.slice(0, 97)}...` : task, + body: prBody, + repo: options?.repo, + }); + if (prResult.success) { + success(`PR created: ${prResult.url}`); + } else { + error(`PR creation failed: ${prResult.error}`); } } } diff --git a/src/features/tasks/execute/taskExecution.ts b/src/features/tasks/execute/taskExecution.ts index e97bde4..9170e22 100644 --- a/src/features/tasks/execute/taskExecution.ts +++ b/src/features/tasks/execute/taskExecution.ts @@ -16,6 +16,7 @@ import { createLogger, getErrorMessage } from '../../../shared/utils/index.js'; import { executePiece } from './pieceExecution.js'; import { DEFAULT_PIECE_NAME } from '../../../shared/constants.js'; import type { TaskExecutionOptions, ExecuteTaskOptions } from './types.js'; +import { createPullRequest, buildPrBody, pushBranch } from '../../../infra/github/index.js'; export type { TaskExecutionOptions, ExecuteTaskOptions }; @@ -77,7 +78,7 @@ export async function executeAndCompleteTask( const executionLog: string[] = []; try { - const { execCwd, execPiece, isWorktree, startMovement, retryNote } = await resolveTaskExecution(task, cwd, pieceName); + const { execCwd, execPiece, isWorktree, branch, startMovement, retryNote, autoPr } = await resolveTaskExecution(task, cwd, pieceName); // cwd is always the project root; pass it as projectCwd so reports/sessions go there const taskSuccess = await executeTask({ @@ -98,6 +99,29 @@ export async function executeAndCompleteTask( } else if (!commitResult.success) { error(`Auto-commit failed: ${commitResult.message}`); } + + // Create PR if autoPr is enabled and commit succeeded + if (commitResult.success && commitResult.commitHash && branch && autoPr) { + info('Creating pull request...'); + // Push branch from project cwd to origin + try { + pushBranch(cwd, branch); + } catch (pushError) { + // Branch may already be pushed, continue to PR creation + log.info('Branch push from project cwd failed (may already exist)', { error: pushError }); + } + const prBody = buildPrBody(undefined, `Task "${task.name}" completed successfully.`); + const prResult = createPullRequest(cwd, { + branch, + title: task.name.length > 100 ? `${task.name.slice(0, 97)}...` : task.name, + body: prBody, + }); + if (prResult.success) { + success(`PR created: ${prResult.url}`); + } else { + error(`PR creation failed: ${prResult.error}`); + } + } } const taskResult = { @@ -198,7 +222,7 @@ export async function resolveTaskExecution( task: TaskInfo, defaultCwd: string, defaultPiece: string -): Promise<{ execCwd: string; execPiece: string; isWorktree: boolean; branch?: string; startMovement?: string; retryNote?: string }> { +): Promise<{ execCwd: string; execPiece: string; isWorktree: boolean; branch?: string; startMovement?: string; retryNote?: string; autoPr?: boolean }> { const data = task.data; // No structured data: use defaults @@ -237,5 +261,14 @@ export async function resolveTaskExecution( // Handle retry_note const retryNote = data.retry_note; - return { execCwd, execPiece, isWorktree, branch, startMovement, retryNote }; + // Handle auto_pr (task YAML > global config) + let autoPr: boolean | undefined; + if (data.auto_pr !== undefined) { + autoPr = data.auto_pr; + } else { + const globalConfig = loadGlobalConfig(); + autoPr = globalConfig.autoPr; + } + + return { execCwd, execPiece, isWorktree, branch, startMovement, retryNote, autoPr }; } diff --git a/src/infra/config/global/globalConfig.ts b/src/infra/config/global/globalConfig.ts index 5fff55d..6597066 100644 --- a/src/infra/config/global/globalConfig.ts +++ b/src/infra/config/global/globalConfig.ts @@ -75,6 +75,7 @@ export class GlobalConfigManager { logFile: parsed.debug.log_file, } : undefined, worktreeDir: parsed.worktree_dir, + autoPr: parsed.auto_pr, disabledBuiltins: parsed.disabled_builtins, enableBuiltinPieces: parsed.enable_builtin_pieces, anthropicApiKey: parsed.anthropic_api_key, @@ -114,6 +115,9 @@ export class GlobalConfigManager { if (config.worktreeDir) { raw.worktree_dir = config.worktreeDir; } + if (config.autoPr !== undefined) { + raw.auto_pr = config.autoPr; + } if (config.disabledBuiltins && config.disabledBuiltins.length > 0) { raw.disabled_builtins = config.disabledBuiltins; } diff --git a/src/infra/task/schema.ts b/src/infra/task/schema.ts index 32401e2..051d189 100644 --- a/src/infra/task/schema.ts +++ b/src/infra/task/schema.ts @@ -32,6 +32,8 @@ export const TaskFileSchema = z.object({ issue: z.number().int().positive().optional(), start_movement: z.string().optional(), retry_note: z.string().optional(), + /** Auto-create PR after worktree execution (default: prompt in interactive mode) */ + auto_pr: z.boolean().optional(), }); export type TaskFileData = z.infer;