From 9cc6ac2ca7d518b248d53bf6c3967520af046609 Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Sat, 14 Feb 2026 01:02:23 +0900 Subject: [PATCH] =?UTF-8?q?=E3=83=9D=E3=82=B9=E3=83=88=E3=82=A8=E3=82=AF?= =?UTF-8?q?=E3=82=B9=E3=82=AD=E3=83=A5=E3=83=BC=E3=82=B7=E3=83=A7=E3=83=B3?= =?UTF-8?q?=E3=81=AE=E5=85=B1=E9=80=9A=E5=8C=96=E3=81=A8instruct=E3=83=A2?= =?UTF-8?q?=E3=83=BC=E3=83=89=E3=81=AE=E6=94=B9=E5=96=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - commit+push+PR作成ロジックをpostExecutionFlowに抽出し、interactive/run/watchの3ルートで共通化 - instructモードはexecuteでcommit+pushのみ（既存PRにpushで反映されるためPR作成不要） - instructのsave_taskで元ブランチ名・worktree・auto_pr:falseを固定保存（プロンプト不要） - instructの会話ループにpieceContextを渡し、/goのサマリー品質を改善 - resolveTaskExecutionのautoPrをboolean必須に変更（undefinedフォールバック廃止） - cloneデフォルトパスを../から../takt-worktree/に変更 --- src/__tests__/taskExecution.test.ts | 6 +- src/features/tasks/execute/postExecution.ts | 81 +++++++++++++++++++ src/features/tasks/execute/resolveTask.ts | 11 ++- .../tasks/execute/selectAndExecute.ts | 68 ++++------------ src/features/tasks/execute/taskExecution.ts | 47 ++++------- src/features/tasks/index.ts | 1 + src/features/tasks/list/instructMode.ts | 4 +- src/features/tasks/list/taskActions.ts | 62 +++++++------- src/infra/task/clone.ts | 2 +- 9 files changed, 154 insertions(+), 128 deletions(-) create mode 100644 src/features/tasks/execute/postExecution.ts diff --git a/src/__tests__/taskExecution.test.ts b/src/__tests__/taskExecution.test.ts index 382e00c..0b05dea 100644 --- a/src/__tests__/taskExecution.test.ts +++ b/src/__tests__/taskExecution.test.ts @@ -120,6 +120,7 @@ describe('resolveTaskExecution', () => { execCwd: '/project', execPiece: 'default', isWorktree: false, + autoPr: false, }); expect(mockSummarizeTaskName).not.toHaveBeenCalled(); expect(mockCreateSharedClone).not.toHaveBeenCalled(); @@ -177,6 +178,7 @@ describe('resolveTaskExecution', () => { execCwd: '/project/../20260128T0504-add-auth', execPiece: 'default', isWorktree: true, + autoPr: false, branch: 'takt/20260128T0504-add-auth', baseBranch: 'main', }); @@ -372,7 +374,7 @@ describe('resolveTaskExecution', () => { expect(result.autoPr).toBe(true); }); - it('should return undefined autoPr when neither task nor config specifies', async () => { + it('should return false autoPr when neither task nor config specifies', async () => { // Given: Neither task nor config has autoPr mockLoadGlobalConfig.mockReturnValue({ language: 'en', @@ -393,7 +395,7 @@ describe('resolveTaskExecution', () => { const result = await resolveTaskExecution(task, '/project', 'default'); // Then - expect(result.autoPr).toBeUndefined(); + expect(result.autoPr).toBe(false); }); it('should prioritize task YAML auto_pr over global config', async () => { diff --git a/src/features/tasks/execute/postExecution.ts b/src/features/tasks/execute/postExecution.ts new file mode 100644 index 0000000..2bb5bf9 --- /dev/null +++ b/src/features/tasks/execute/postExecution.ts @@ -0,0 +1,81 @@ +/** + * Shared post-execution logic: auto-commit, push, and PR creation. + * + * Used by both selectAndExecuteTask (interactive mode) and + * instructBranch (instruct mode from takt list). + */ + +import { loadGlobalConfig } from '../../../infra/config/index.js'; +import { confirm } from '../../../shared/prompt/index.js'; +import { autoCommitAndPush } from '../../../infra/task/index.js'; +import { info, error, success } from '../../../shared/ui/index.js'; +import { createLogger } from '../../../shared/utils/index.js'; +import { createPullRequest, buildPrBody, pushBranch } from '../../../infra/github/index.js'; +import type { GitHubIssue } from '../../../infra/github/index.js'; + +const log = createLogger('postExecution'); + +/** + * Resolve auto-PR setting with priority: CLI option > config > prompt. + */ +export async function resolveAutoPr(optionAutoPr: boolean | undefined): Promise { + if (typeof optionAutoPr === 'boolean') { + return optionAutoPr; + } + + const globalConfig = loadGlobalConfig(); + if (typeof globalConfig.autoPr === 'boolean') { + return globalConfig.autoPr; + } + + return confirm('Create pull request?', true); +} + +export interface PostExecutionOptions { + execCwd: string; + projectCwd: string; + task: string; + branch?: string; + baseBranch?: string; + shouldCreatePr: boolean; + pieceIdentifier?: string; + issues?: GitHubIssue[]; + repo?: string; +} + +/** + * 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 commitResult = autoCommitAndPush(execCwd, task, projectCwd); + if (commitResult.success && commitResult.commitHash) { + success(`Auto-committed & pushed: ${commitResult.commitHash}`); + } else if (!commitResult.success) { + error(`Auto-commit failed: ${commitResult.message}`); + } + + if (commitResult.success && commitResult.commitHash && branch && shouldCreatePr) { + info('Creating pull request...'); + try { + pushBranch(projectCwd, branch); + } catch (pushError) { + log.info('Branch push from project cwd failed (may already exist)', { error: pushError }); + } + const report = pieceIdentifier ? `Piece \`${pieceIdentifier}\` completed successfully.` : 'Task completed successfully.'; + const prBody = buildPrBody(issues, report); + const prResult = createPullRequest(projectCwd, { + branch, + title: task.length > 100 ? `${task.slice(0, 97)}...` : task, + body: prBody, + base: baseBranch, + repo, + }); + if (prResult.success) { + success(`PR created: ${prResult.url}`); + } else { + error(`PR creation failed: ${prResult.error}`); + } + } +} diff --git a/src/features/tasks/execute/resolveTask.ts b/src/features/tasks/execute/resolveTask.ts index 43d1e11..5e4d1ed 100644 --- a/src/features/tasks/execute/resolveTask.ts +++ b/src/features/tasks/execute/resolveTask.ts @@ -19,7 +19,7 @@ export interface ResolvedTaskExecution { baseBranch?: string; startMovement?: string; retryNote?: string; - autoPr?: boolean; + autoPr: boolean; issueNumber?: number; } @@ -74,7 +74,7 @@ export async function resolveTaskExecution( const data = task.data; if (!data) { - return { execCwd: defaultCwd, execPiece: defaultPiece, isWorktree: false }; + return { execCwd: defaultCwd, execPiece: defaultPiece, isWorktree: false, autoPr: false }; } let execCwd = defaultCwd; @@ -115,7 +115,6 @@ export async function resolveTaskExecution( execCwd = result.path; branch = result.branch; isWorktree = true; - } if (task.taskDir && reportDirName) { @@ -126,25 +125,25 @@ export async function resolveTaskExecution( const startMovement = data.start_movement; const retryNote = data.retry_note; - let autoPr: boolean | undefined; + let autoPr: boolean; if (data.auto_pr !== undefined) { autoPr = data.auto_pr; } else { const globalConfig = loadGlobalConfig(); - autoPr = globalConfig.autoPr; + autoPr = globalConfig.autoPr ?? false; } return { execCwd, execPiece, isWorktree, + autoPr, ...(taskPrompt ? { taskPrompt } : {}), ...(reportDirName ? { reportDirName } : {}), ...(branch ? { branch } : {}), ...(baseBranch ? { baseBranch } : {}), ...(startMovement ? { startMovement } : {}), ...(retryNote ? { retryNote } : {}), - ...(autoPr !== undefined ? { autoPr } : {}), ...(data.issue !== undefined ? { issueNumber: data.issue } : {}), }; } diff --git a/src/features/tasks/execute/selectAndExecute.ts b/src/features/tasks/execute/selectAndExecute.ts index 3014634..32aedb0 100644 --- a/src/features/tasks/execute/selectAndExecute.ts +++ b/src/features/tasks/execute/selectAndExecute.ts @@ -9,15 +9,14 @@ import { listPieces, isPiecePath, - loadGlobalConfig, } from '../../../infra/config/index.js'; import { confirm } from '../../../shared/prompt/index.js'; -import { createSharedClone, autoCommitAndPush, summarizeTaskName, getCurrentBranch } from '../../../infra/task/index.js'; +import { createSharedClone, summarizeTaskName, getCurrentBranch } from '../../../infra/task/index.js'; import { DEFAULT_PIECE_NAME } from '../../../shared/constants.js'; -import { info, error, success, withProgress } from '../../../shared/ui/index.js'; +import { info, error, withProgress } from '../../../shared/ui/index.js'; import { createLogger } from '../../../shared/utils/index.js'; -import { createPullRequest, buildPrBody, pushBranch } from '../../../infra/github/index.js'; import { executeTask } from './taskExecution.js'; +import { resolveAutoPr, postExecutionFlow } from './postExecution.js'; import type { TaskExecutionOptions, WorktreeConfirmationResult, SelectAndExecuteOptions } from './types.js'; import { selectPiece } from '../../pieceSelection/index.js'; @@ -75,26 +74,6 @@ export async function confirmAndCreateWorktree( return { execCwd: result.path, isWorktree: true, branch: result.branch, baseBranch }; } -/** - * 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?', true); -} - /** * Execute a task with piece selection, optional worktree, and auto-commit. * Shared by direct task execution and interactive mode. @@ -136,36 +115,17 @@ export async function selectAndExecuteTask( }); if (taskSuccess && isWorktree) { - const commitResult = autoCommitAndPush(execCwd, task, cwd); - if (commitResult.success && commitResult.commitHash) { - success(`Auto-committed & pushed: ${commitResult.commitHash}`); - } else if (!commitResult.success) { - error(`Auto-commit failed: ${commitResult.message}`); - } - - 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, - base: baseBranch, - repo: options?.repo, - }); - if (prResult.success) { - success(`PR created: ${prResult.url}`); - } else { - error(`PR creation failed: ${prResult.error}`); - } - } + await postExecutionFlow({ + execCwd, + projectCwd: cwd, + task, + branch, + baseBranch, + shouldCreatePr, + pieceIdentifier, + issues: options?.issues, + repo: options?.repo, + }); } if (!taskSuccess) { diff --git a/src/features/tasks/execute/taskExecution.ts b/src/features/tasks/execute/taskExecution.ts index 14c4a0d..f0b4751 100644 --- a/src/features/tasks/execute/taskExecution.ts +++ b/src/features/tasks/execute/taskExecution.ts @@ -3,7 +3,7 @@ */ import { loadPieceByIdentifier, isPiecePath, loadGlobalConfig } from '../../../infra/config/index.js'; -import { TaskRunner, type TaskInfo, autoCommitAndPush } from '../../../infra/task/index.js'; +import { TaskRunner, type TaskInfo } from '../../../infra/task/index.js'; import { header, info, @@ -17,9 +17,10 @@ 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 { createPullRequest, buildPrBody, pushBranch, fetchIssue, checkGhCli } from '../../../infra/github/index.js'; +import { fetchIssue, checkGhCli } from '../../../infra/github/index.js'; import { runWithWorkerPool } from './parallelExecution.js'; import { resolveTaskExecution } from './resolveTask.js'; +import { postExecutionFlow } from './postExecution.js'; export type { TaskExecutionOptions, ExecuteTaskOptions }; @@ -167,37 +168,17 @@ export async function executeAndCompleteTask( const completedAt = new Date().toISOString(); if (taskSuccess && isWorktree) { - const commitResult = autoCommitAndPush(execCwd, task.name, cwd); - if (commitResult.success && commitResult.commitHash) { - info(`Auto-committed & pushed: ${commitResult.commitHash}`); - } 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 issues = resolveTaskIssue(issueNumber); - const prBody = buildPrBody(issues, `Piece \`${execPiece}\` completed successfully.`); - const prResult = createPullRequest(cwd, { - branch, - title: task.name.length > 100 ? `${task.name.slice(0, 97)}...` : task.name, - body: prBody, - base: baseBranch, - }); - if (prResult.success) { - success(`PR created: ${prResult.url}`); - } else { - error(`PR creation failed: ${prResult.error}`); - } - } + const issues = resolveTaskIssue(issueNumber); + await postExecutionFlow({ + execCwd, + projectCwd: cwd, + task: task.name, + branch, + baseBranch, + shouldCreatePr: autoPr, + pieceIdentifier: execPiece, + issues, + }); } const taskResult = { diff --git a/src/features/tasks/index.ts b/src/features/tasks/index.ts index f413860..41e2b5a 100644 --- a/src/features/tasks/index.ts +++ b/src/features/tasks/index.ts @@ -14,6 +14,7 @@ export { type SelectAndExecuteOptions, type WorktreeConfirmationResult, } from './execute/selectAndExecute.js'; +export { resolveAutoPr, postExecutionFlow, type PostExecutionOptions } from './execute/postExecution.js'; export { addTask, saveTaskFile, saveTaskFromInteractive, createIssueFromTask, createIssueAndSaveTask } from './add/index.js'; export { watchTasks } from './watch/index.js'; export { diff --git a/src/features/tasks/list/instructMode.ts b/src/features/tasks/list/instructMode.ts index 9d83096..8a8c8a3 100644 --- a/src/features/tasks/list/instructMode.ts +++ b/src/features/tasks/list/instructMode.ts @@ -17,6 +17,7 @@ import { resolveLanguage, buildSummaryActionOptions, selectSummaryAction, + type PieceContext, } from '../../interactive/interactive.js'; import { loadTemplate } from '../../../shared/prompts/index.js'; import { getLabelObject } from '../../../shared/i18n/index.js'; @@ -66,6 +67,7 @@ export async function runInstructMode( cwd: string, branchContext: string, branchName: string, + pieceContext?: PieceContext, ): Promise { const globalConfig = loadGlobalConfig(); const lang = resolveLanguage(globalConfig.language); @@ -113,7 +115,7 @@ export async function runInstructMode( selectAction: createSelectInstructAction(ui), }; - const result = await runConversationLoop(cwd, ctx, strategy, undefined, undefined); + const result = await runConversationLoop(cwd, ctx, strategy, pieceContext, undefined); if (result.action === 'cancel') { return { action: 'cancel', task: '' }; diff --git a/src/features/tasks/list/taskActions.ts b/src/features/tasks/list/taskActions.ts index d062c88..223d56d 100644 --- a/src/features/tasks/list/taskActions.ts +++ b/src/features/tasks/list/taskActions.ts @@ -19,6 +19,7 @@ import { autoCommitAndPush, type BranchListItem, } from '../../../infra/task/index.js'; +import { loadGlobalConfig, getPieceDescription } from '../../../infra/config/index.js'; import { selectOption } from '../../../shared/prompt/index.js'; import { info, success, error as logError, warn, header, blankLine } from '../../../shared/ui/index.js'; import { createLogger, getErrorMessage } from '../../../shared/utils/index.js'; @@ -29,6 +30,7 @@ import { runInstructMode } from './instructMode.js'; import { saveTaskFile } from '../add/index.js'; import { selectPiece } from '../../pieceSelection/index.js'; import { dispatchConversationAction } from '../../interactive/actionDispatcher.js'; +import type { PieceContext } from '../../interactive/interactive.js'; const log = createLogger('list-tasks'); @@ -306,7 +308,7 @@ function getBranchContext(projectDir: string, branch: string): string { /** * Instruct branch: create a temp clone, give additional instructions via - * interactive conversation, then auto-commit+push or save as task file. + * interactive conversation, then auto-commit+push+PR or save as task file. */ export async function instructBranch( projectDir: string, @@ -315,45 +317,43 @@ export async function instructBranch( ): Promise { const { branch } = item.info; - const branchContext = getBranchContext(projectDir, branch); - const result = await runInstructMode(projectDir, branchContext, branch); - let selectedPiece: string | null = null; + const selectedPiece = await selectPiece(projectDir); + if (!selectedPiece) { + info('Cancelled'); + return false; + } - const ensurePieceSelected = async (): Promise => { - if (selectedPiece) { - return selectedPiece; - } - selectedPiece = await selectPiece(projectDir); - if (!selectedPiece) { - info('Cancelled'); - return null; - } - return selectedPiece; + const globalConfig = loadGlobalConfig(); + const pieceDesc = getPieceDescription(selectedPiece, projectDir, globalConfig.interactivePreviewMovements); + const pieceContext: PieceContext = { + name: pieceDesc.name, + description: pieceDesc.description, + pieceStructure: pieceDesc.pieceStructure, + movementPreviews: pieceDesc.movementPreviews, }; + const branchContext = getBranchContext(projectDir, branch); + const result = await runInstructMode(projectDir, branchContext, branch, pieceContext); + return dispatchConversationAction(result, { cancel: () => { info('Cancelled'); return false; }, save_task: async ({ task }) => { - const piece = await ensurePieceSelected(); - if (!piece) { - return false; - } - const created = await saveTaskFile(projectDir, task, { piece }); + const created = await saveTaskFile(projectDir, task, { + piece: selectedPiece, + worktree: true, + branch, + autoPr: false, + }); success(`Task saved: ${created.taskName}`); - info(` File: ${created.tasksFile}`); - log.info('Task saved from instruct mode', { branch, piece }); + info(` Branch: ${branch}`); + log.info('Task saved from instruct mode', { branch, piece: selectedPiece }); return true; }, execute: async ({ task }) => { - const piece = await ensurePieceSelected(); - if (!piece) { - return false; - } - - log.info('Instructing branch via temp clone', { branch, piece }); + log.info('Instructing branch via temp clone', { branch, piece: selectedPiece }); info(`Running instruction on ${branch}...`); const clone = createTempCloneForBranch(projectDir, branch); @@ -366,17 +366,17 @@ export async function instructBranch( const taskSuccess = await executeTask({ task: fullInstruction, cwd: clone.path, - pieceIdentifier: piece, + pieceIdentifier: selectedPiece, projectCwd: projectDir, agentOverrides: options, }); if (taskSuccess) { - const commitResult = autoCommitAndPush(clone.path, item.taskSlug, projectDir); + const commitResult = autoCommitAndPush(clone.path, task, projectDir); if (commitResult.success && commitResult.commitHash) { - info(`Auto-committed & pushed: ${commitResult.commitHash}`); + success(`Auto-committed & pushed: ${commitResult.commitHash}`); } else if (!commitResult.success) { - warn(`Auto-commit skipped: ${commitResult.message}`); + logError(`Auto-commit failed: ${commitResult.message}`); } success(`Instruction completed on ${branch}`); log.info('Instruction completed', { branch }); diff --git a/src/infra/task/clone.ts b/src/infra/task/clone.ts index f424707..69cc1fb 100644 --- a/src/infra/task/clone.ts +++ b/src/infra/task/clone.ts @@ -42,7 +42,7 @@ export class CloneManager { ? globalConfig.worktreeDir : path.resolve(projectDir, globalConfig.worktreeDir); } - return path.join(projectDir, '..'); + return path.join(projectDir, '..', 'takt-worktree'); } /** Resolve the clone path based on options and global config */