From b9a2a0329b08da6355abda5809e900263cd15ce6 Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Sun, 8 Feb 2026 07:51:03 +0900 Subject: [PATCH] =?UTF-8?q?auto=20PR=20=E3=81=AE=E3=83=99=E3=83=BC?= =?UTF-8?q?=E3=82=B9=E3=83=96=E3=83=A9=E3=83=B3=E3=83=81=E3=82=92=E3=83=96?= =?UTF-8?q?=E3=83=A9=E3=83=B3=E3=83=81=E4=BD=9C=E6=88=90=E5=89=8D=E3=81=AE?= =?UTF-8?q?=E7=8F=BE=E5=9C=A8=E3=83=96=E3=83=A9=E3=83=B3=E3=83=81=E3=81=AB?= =?UTF-8?q?=E8=A8=AD=E5=AE=9A?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit createPullRequest の全呼び出し箇所で base が未指定だったため、 PR が常にリポジトリデフォルトブランチ(main)向けに作成されていた。 ブランチ作成/clone作成の直前に getCurrentBranch() で元ブランチを 取得し、PR作成時に base として渡すように修正。 --- src/__tests__/cli-worktree.test.ts | 5 ++ src/__tests__/getCurrentBranch.test.ts | 57 +++++++++++++++++++ src/__tests__/it-pipeline-modes.test.ts | 1 + src/__tests__/pipelineExecution.test.ts | 31 ++++++++++ src/__tests__/selectAndExecute-autoPr.test.ts | 1 + src/__tests__/taskExecution.test.ts | 55 ++++++++++++++++++ src/features/pipeline/execute.ts | 5 +- .../tasks/execute/selectAndExecute.ts | 9 ++- src/features/tasks/execute/taskExecution.ts | 11 ++-- src/features/tasks/execute/types.ts | 1 + src/infra/task/git.ts | 11 ++++ src/infra/task/index.ts | 2 +- 12 files changed, 180 insertions(+), 9 deletions(-) create mode 100644 src/__tests__/getCurrentBranch.test.ts diff --git a/src/__tests__/cli-worktree.test.ts b/src/__tests__/cli-worktree.test.ts index e85f1c7..cd09d5e 100644 --- a/src/__tests__/cli-worktree.test.ts +++ b/src/__tests__/cli-worktree.test.ts @@ -10,6 +10,11 @@ vi.mock('../shared/prompt/index.js', () => ({ selectOptionWithDefault: vi.fn(), })); +vi.mock('../infra/task/git.js', () => ({ + stageAndCommit: vi.fn(), + getCurrentBranch: vi.fn(() => 'main'), +})); + vi.mock('../infra/task/clone.js', () => ({ createSharedClone: vi.fn(), removeClone: vi.fn(), diff --git a/src/__tests__/getCurrentBranch.test.ts b/src/__tests__/getCurrentBranch.test.ts new file mode 100644 index 0000000..28c36c6 --- /dev/null +++ b/src/__tests__/getCurrentBranch.test.ts @@ -0,0 +1,57 @@ +/** + * Tests for getCurrentBranch + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { execFileSync } from 'node:child_process'; + +vi.mock('node:child_process', () => ({ + execFileSync: vi.fn(), +})); + +const mockExecFileSync = vi.mocked(execFileSync); + +import { getCurrentBranch } from '../infra/task/git.js'; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('getCurrentBranch', () => { + it('should return the current branch name', () => { + // Given + mockExecFileSync.mockReturnValue('feature/my-branch\n'); + + // When + const result = getCurrentBranch('/project'); + + // Then + expect(result).toBe('feature/my-branch'); + expect(mockExecFileSync).toHaveBeenCalledWith( + 'git', + ['rev-parse', '--abbrev-ref', 'HEAD'], + { cwd: '/project', encoding: 'utf-8', stdio: 'pipe' }, + ); + }); + + it('should trim whitespace from output', () => { + // Given + mockExecFileSync.mockReturnValue(' main \n'); + + // When + const result = getCurrentBranch('/project'); + + // Then + expect(result).toBe('main'); + }); + + it('should propagate errors from git', () => { + // Given + mockExecFileSync.mockImplementation(() => { + throw new Error('not a git repository'); + }); + + // When / Then + expect(() => getCurrentBranch('/not-a-repo')).toThrow('not a git repository'); + }); +}); diff --git a/src/__tests__/it-pipeline-modes.test.ts b/src/__tests__/it-pipeline-modes.test.ts index 46b4228..235d330 100644 --- a/src/__tests__/it-pipeline-modes.test.ts +++ b/src/__tests__/it-pipeline-modes.test.ts @@ -65,6 +65,7 @@ vi.mock('../infra/github/pr.js', () => ({ vi.mock('../infra/task/git.js', () => ({ stageAndCommit: vi.fn().mockReturnValue('abc1234'), + getCurrentBranch: vi.fn().mockReturnValue('main'), })); vi.mock('../shared/ui/index.js', () => ({ diff --git a/src/__tests__/pipelineExecution.test.ts b/src/__tests__/pipelineExecution.test.ts index 1db0bda..eeb9a8e 100644 --- a/src/__tests__/pipelineExecution.test.ts +++ b/src/__tests__/pipelineExecution.test.ts @@ -218,6 +218,37 @@ describe('executePipeline', () => { ); }); + it('should pass baseBranch as base to createPullRequest', async () => { + // Given: getCurrentBranch returns 'develop' before branch creation + mockExecFileSync.mockImplementation((_cmd: string, args: string[]) => { + if (args[0] === 'rev-parse' && args[1] === '--abbrev-ref') { + return 'develop\n'; + } + return 'abc1234\n'; + }); + mockExecuteTask.mockResolvedValueOnce(true); + mockCreatePullRequest.mockReturnValueOnce({ success: true, url: 'https://github.com/test/pr/1' }); + + // When + const exitCode = await executePipeline({ + task: 'Fix the bug', + piece: 'default', + branch: 'fix/my-branch', + autoPr: true, + cwd: '/tmp/test', + }); + + // Then + expect(exitCode).toBe(0); + expect(mockCreatePullRequest).toHaveBeenCalledWith( + '/tmp/test', + expect.objectContaining({ + branch: 'fix/my-branch', + base: 'develop', + }), + ); + }); + it('should use --task when both --task and positional task are provided', async () => { mockExecuteTask.mockResolvedValueOnce(true); diff --git a/src/__tests__/selectAndExecute-autoPr.test.ts b/src/__tests__/selectAndExecute-autoPr.test.ts index 2cce3f2..ea3fd47 100644 --- a/src/__tests__/selectAndExecute-autoPr.test.ts +++ b/src/__tests__/selectAndExecute-autoPr.test.ts @@ -23,6 +23,7 @@ vi.mock('../infra/task/index.js', () => ({ createSharedClone: vi.fn(), autoCommitAndPush: vi.fn(), summarizeTaskName: vi.fn(), + getCurrentBranch: vi.fn(() => 'main'), })); vi.mock('../shared/ui/index.js', () => ({ diff --git a/src/__tests__/taskExecution.test.ts b/src/__tests__/taskExecution.test.ts index 89212c7..bec867f 100644 --- a/src/__tests__/taskExecution.test.ts +++ b/src/__tests__/taskExecution.test.ts @@ -25,6 +25,11 @@ vi.mock('../infra/task/clone.js', async (importOriginal) => ({ removeClone: vi.fn(), })); +vi.mock('../infra/task/git.js', async (importOriginal) => ({ + ...(await importOriginal>()), + getCurrentBranch: vi.fn(() => 'main'), +})); + vi.mock('../infra/task/autoCommit.js', async (importOriginal) => ({ ...(await importOriginal>()), autoCommitAndPush: vi.fn(), @@ -68,12 +73,14 @@ vi.mock('../shared/constants.js', () => ({ })); import { createSharedClone } from '../infra/task/clone.js'; +import { getCurrentBranch } from '../infra/task/git.js'; import { summarizeTaskName } from '../infra/task/summarize.js'; import { info } from '../shared/ui/index.js'; import { resolveTaskExecution } from '../features/tasks/index.js'; import type { TaskInfo } from '../infra/task/index.js'; const mockCreateSharedClone = vi.mocked(createSharedClone); +const mockGetCurrentBranch = vi.mocked(getCurrentBranch); const mockSummarizeTaskName = vi.mocked(summarizeTaskName); const mockInfo = vi.mocked(info); @@ -150,11 +157,13 @@ describe('resolveTaskExecution', () => { branch: undefined, taskSlug: 'add-auth', }); + expect(mockGetCurrentBranch).toHaveBeenCalledWith('/project'); expect(result).toEqual({ execCwd: '/project/../20260128T0504-add-auth', execPiece: 'default', isWorktree: true, branch: 'takt/20260128T0504-add-auth', + baseBranch: 'main', }); }); @@ -396,4 +405,50 @@ describe('resolveTaskExecution', () => { // Then expect(result.autoPr).toBe(false); }); + + it('should capture baseBranch from getCurrentBranch when worktree is used', async () => { + // Given: Task with worktree, on 'develop' branch + mockGetCurrentBranch.mockReturnValue('develop'); + const task: TaskInfo = { + name: 'task-on-develop', + content: 'Task on develop branch', + filePath: '/tasks/task.yaml', + data: { + task: 'Task on develop branch', + worktree: true, + }, + }; + + mockSummarizeTaskName.mockResolvedValue('task-develop'); + mockCreateSharedClone.mockReturnValue({ + path: '/project/../task-develop', + branch: 'takt/task-develop', + }); + + // When + const result = await resolveTaskExecution(task, '/project', 'default'); + + // Then + expect(mockGetCurrentBranch).toHaveBeenCalledWith('/project'); + expect(result.baseBranch).toBe('develop'); + }); + + it('should not set baseBranch when worktree is not used', async () => { + // Given: Task without worktree + const task: TaskInfo = { + name: 'task-no-worktree', + content: 'Task without worktree', + filePath: '/tasks/task.yaml', + data: { + task: 'Task without worktree', + }, + }; + + // When + const result = await resolveTaskExecution(task, '/project', 'default'); + + // Then + expect(mockGetCurrentBranch).not.toHaveBeenCalled(); + expect(result.baseBranch).toBeUndefined(); + }); }); diff --git a/src/features/pipeline/execute.ts b/src/features/pipeline/execute.ts index b274a70..f885872 100644 --- a/src/features/pipeline/execute.ts +++ b/src/features/pipeline/execute.ts @@ -19,7 +19,7 @@ import { buildPrBody, type GitHubIssue, } from '../../infra/github/index.js'; -import { stageAndCommit } from '../../infra/task/index.js'; +import { stageAndCommit, getCurrentBranch } from '../../infra/task/index.js'; import { executeTask, type TaskExecutionOptions, type PipelineExecutionOptions } from '../tasks/index.js'; import { loadGlobalConfig } from '../../infra/config/index.js'; import { info, error, success, status, blankLine } from '../../shared/ui/index.js'; @@ -136,7 +136,9 @@ export async function executePipeline(options: PipelineExecutionOptions): Promis // --- Step 2: Create branch (skip if --skip-git) --- let branch: string | undefined; + let baseBranch: string | undefined; if (!skipGit) { + baseBranch = getCurrentBranch(cwd); branch = options.branch ?? generatePipelineBranchName(pipelineConfig, options.issueNumber); info(`Creating branch: ${branch}`); try { @@ -206,6 +208,7 @@ export async function executePipeline(options: PipelineExecutionOptions): Promis branch, title: prTitle, body: prBody, + base: baseBranch, repo: options.repo, }); diff --git a/src/features/tasks/execute/selectAndExecute.ts b/src/features/tasks/execute/selectAndExecute.ts index 715609b..6ac3d23 100644 --- a/src/features/tasks/execute/selectAndExecute.ts +++ b/src/features/tasks/execute/selectAndExecute.ts @@ -17,7 +17,7 @@ import { loadGlobalConfig, } from '../../../infra/config/index.js'; import { confirm } from '../../../shared/prompt/index.js'; -import { createSharedClone, autoCommitAndPush, summarizeTaskName } from '../../../infra/task/index.js'; +import { createSharedClone, autoCommitAndPush, summarizeTaskName, getCurrentBranch } 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'; @@ -111,6 +111,8 @@ export async function confirmAndCreateWorktree( return { execCwd: cwd, isWorktree: false }; } + const baseBranch = getCurrentBranch(cwd); + info('Generating branch name...'); const taskSlug = await summarizeTaskName(task, { cwd }); @@ -121,7 +123,7 @@ export async function confirmAndCreateWorktree( }); info(`Clone created: ${result.path} (branch: ${result.branch})`); - return { execCwd: result.path, isWorktree: true, branch: result.branch }; + return { execCwd: result.path, isWorktree: true, branch: result.branch, baseBranch }; } /** @@ -161,7 +163,7 @@ export async function selectAndExecuteTask( return; } - const { execCwd, isWorktree, branch } = await confirmAndCreateWorktree( + const { execCwd, isWorktree, branch, baseBranch } = await confirmAndCreateWorktree( cwd, task, options?.createWorktree, @@ -206,6 +208,7 @@ export async function selectAndExecuteTask( branch, title: task.length > 100 ? `${task.slice(0, 97)}...` : task, body: prBody, + base: baseBranch, repo: options?.repo, }); if (prResult.success) { diff --git a/src/features/tasks/execute/taskExecution.ts b/src/features/tasks/execute/taskExecution.ts index e625d66..74318a2 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, createSharedClone, autoCommitAndPush, summarizeTaskName } from '../../../infra/task/index.js'; +import { TaskRunner, type TaskInfo, createSharedClone, autoCommitAndPush, summarizeTaskName, getCurrentBranch } from '../../../infra/task/index.js'; import { header, info, @@ -78,7 +78,7 @@ export async function executeAndCompleteTask( const executionLog: string[] = []; try { - const { execCwd, execPiece, isWorktree, branch, startMovement, retryNote, autoPr } = await resolveTaskExecution(task, cwd, pieceName); + const { execCwd, execPiece, isWorktree, branch, baseBranch, 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({ @@ -115,6 +115,7 @@ export async function executeAndCompleteTask( 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}`); @@ -222,7 +223,7 @@ export async function resolveTaskExecution( task: TaskInfo, defaultCwd: string, defaultPiece: string -): Promise<{ execCwd: string; execPiece: string; isWorktree: boolean; branch?: string; startMovement?: string; retryNote?: string; autoPr?: boolean }> { +): Promise<{ execCwd: string; execPiece: string; isWorktree: boolean; branch?: string; baseBranch?: string; startMovement?: string; retryNote?: string; autoPr?: boolean }> { const data = task.data; // No structured data: use defaults @@ -233,9 +234,11 @@ export async function resolveTaskExecution( let execCwd = defaultCwd; let isWorktree = false; let branch: string | undefined; + let baseBranch: string | undefined; // Handle worktree (now creates a shared clone) if (data.worktree) { + baseBranch = getCurrentBranch(defaultCwd); // Summarize task content to English slug using AI info('Generating branch name...'); const taskSlug = await summarizeTaskName(task.content, { cwd: defaultCwd }); @@ -271,5 +274,5 @@ export async function resolveTaskExecution( autoPr = globalConfig.autoPr; } - return { execCwd, execPiece, isWorktree, branch, startMovement, retryNote, autoPr }; + return { execCwd, execPiece, isWorktree, branch, baseBranch, startMovement, retryNote, autoPr }; } diff --git a/src/features/tasks/execute/types.ts b/src/features/tasks/execute/types.ts index 04d3850..a1355c1 100644 --- a/src/features/tasks/execute/types.ts +++ b/src/features/tasks/execute/types.ts @@ -91,6 +91,7 @@ export interface WorktreeConfirmationResult { execCwd: string; isWorktree: boolean; branch?: string; + baseBranch?: string; } export interface SelectAndExecuteOptions { diff --git a/src/infra/task/git.ts b/src/infra/task/git.ts index add1156..63b56d6 100644 --- a/src/infra/task/git.ts +++ b/src/infra/task/git.ts @@ -4,6 +4,17 @@ import { execFileSync } from 'node:child_process'; +/** + * Get the current branch name. + */ +export function getCurrentBranch(cwd: string): string { + return execFileSync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], { + cwd, + encoding: 'utf-8', + stdio: 'pipe', + }).trim(); +} + /** * Stage all changes and create a commit. * Returns the short commit hash if changes were committed, undefined if no changes. diff --git a/src/infra/task/index.ts b/src/infra/task/index.ts index e6ea017..3340cae 100644 --- a/src/infra/task/index.ts +++ b/src/infra/task/index.ts @@ -43,7 +43,7 @@ export { getOriginalInstruction, buildListItems, } from './branchList.js'; -export { stageAndCommit } from './git.js'; +export { stageAndCommit, getCurrentBranch } from './git.js'; export { autoCommitAndPush, type AutoCommitResult } from './autoCommit.js'; export { summarizeTaskName } from './summarize.js'; export { TaskWatcher, type TaskWatcherOptions } from './watcher.js';