From c843858f2e8b8c993709e20d77b0d537bdeca27a Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Mon, 2 Mar 2026 23:01:24 +0900 Subject: [PATCH] =?UTF-8?q?feat:=20--pr=20=E3=82=A4=E3=83=B3=E3=82=BF?= =?UTF-8?q?=E3=83=A9=E3=82=AF=E3=83=86=E3=82=A3=E3=83=96=E3=83=A2=E3=83=BC?= =?UTF-8?q?=E3=83=89=E3=81=A7=20create=5Fissue=20=E9=99=A4=E5=A4=96?= =?UTF-8?q?=E3=83=BBsave=5Ftask=20=E6=99=82=E3=81=AE=20PR=20=E3=83=96?= =?UTF-8?q?=E3=83=A9=E3=83=B3=E3=83=81=E8=87=AA=E5=8B=95=E8=A8=AD=E5=AE=9A?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - --pr 指定時のインタラクティブモードで create_issue を選択肢から除外 - execute アクション時に PR ブランチを fetch + checkout してから実行 - save_task アクション時に worktree/branch/autoPr を自動設定しプロンプトをスキップ - saveTaskFromInteractive に presetSettings オプションを追加 - interactiveMode に InteractiveModeOptions（excludeActions）を追加 - checkoutBranch() を git.ts に追加し steps.ts の重複コードを DRY 化 --- .../cli-routing-issue-resolve.test.ts | 24 ++++++++++++++ src/__tests__/cli-routing-pr-resolve.test.ts | 23 +++++++++++++- src/app/cli/routing.ts | 22 ++++++++++--- src/features/interactive/index.ts | 1 + src/features/interactive/interactive.ts | 31 +++++++++++++++++++ src/features/pipeline/steps.ts | 5 ++- src/features/tasks/add/index.ts | 5 +-- src/infra/task/git.ts | 9 ++++++ src/infra/task/index.ts | 2 +- 9 files changed, 110 insertions(+), 12 deletions(-) diff --git a/src/__tests__/cli-routing-issue-resolve.test.ts b/src/__tests__/cli-routing-issue-resolve.test.ts index f61f035..0a3ddc8 100644 --- a/src/__tests__/cli-routing-issue-resolve.test.ts +++ b/src/__tests__/cli-routing-issue-resolve.test.ts @@ -231,6 +231,8 @@ describe('Issue resolution in routing', () => { '## GitHub Issue #131: Issue #131', expect.anything(), undefined, + undefined, + undefined, ); // Then: selectAndExecuteTask should be called (issues are used only for initialInput, not selectOptions) @@ -282,6 +284,8 @@ describe('Issue resolution in routing', () => { '## GitHub Issue #131: Issue #131', expect.anything(), undefined, + undefined, + undefined, ); // Then: selectAndExecuteTask should be called @@ -305,6 +309,8 @@ describe('Issue resolution in routing', () => { 'refactor the code', expect.anything(), undefined, + undefined, + undefined, ); // Then: no issue fetching should occur @@ -324,6 +330,8 @@ describe('Issue resolution in routing', () => { undefined, expect.anything(), undefined, + undefined, + undefined, ); // Then: no issue fetching should occur @@ -399,6 +407,8 @@ describe('Issue resolution in routing', () => { ]), }), undefined, + undefined, + undefined, ); }); @@ -433,6 +443,8 @@ describe('Issue resolution in routing', () => { ]), }), undefined, + undefined, + undefined, ); }); @@ -450,6 +462,8 @@ describe('Issue resolution in routing', () => { 'fix issue', expect.objectContaining({ taskHistory: [] }), undefined, + undefined, + undefined, ); }); @@ -463,6 +477,8 @@ describe('Issue resolution in routing', () => { 'verify history', expect.objectContaining({ taskHistory: [] }), undefined, + undefined, + undefined, ); }); }); @@ -533,6 +549,8 @@ describe('Issue resolution in routing', () => { undefined, expect.anything(), 'saved-session-123', + undefined, + undefined, ); }); @@ -556,6 +574,8 @@ describe('Issue resolution in routing', () => { undefined, expect.anything(), undefined, + undefined, + undefined, ); }); @@ -572,6 +592,8 @@ describe('Issue resolution in routing', () => { undefined, expect.anything(), undefined, + undefined, + undefined, ); }); }); @@ -586,6 +608,8 @@ describe('Issue resolution in routing', () => { undefined, expect.anything(), undefined, + undefined, + undefined, ); }); }); diff --git a/src/__tests__/cli-routing-pr-resolve.test.ts b/src/__tests__/cli-routing-pr-resolve.test.ts index dbc78ed..499bd8b 100644 --- a/src/__tests__/cli-routing-pr-resolve.test.ts +++ b/src/__tests__/cli-routing-pr-resolve.test.ts @@ -9,6 +9,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; vi.mock('../shared/ui/index.js', () => ({ info: vi.fn(), + success: vi.fn(), error: vi.fn(), withProgress: vi.fn(async (_start, _done, operation) => operation()), })); @@ -76,11 +77,13 @@ vi.mock('../features/interactive/index.js', () => ({ const mockListAllTaskItems = vi.fn(); const mockIsStaleRunningTask = vi.fn(); +const mockCheckoutBranch = vi.fn(); vi.mock('../infra/task/index.js', () => ({ TaskRunner: vi.fn(() => ({ listAllTaskItems: mockListAllTaskItems, })), isStaleRunningTask: (...args: unknown[]) => mockIsStaleRunningTask(...args), + checkoutBranch: (...args: unknown[]) => mockCheckoutBranch(...args), })); vi.mock('../infra/config/index.js', () => ({ @@ -171,6 +174,8 @@ describe('PR resolution in routing', () => { expect.stringContaining('## PR #456 Review Comments:'), expect.anything(), undefined, + undefined, + { excludeActions: ['create_issue'] }, ); }); @@ -184,7 +189,7 @@ describe('PR resolution in routing', () => { // When await executeDefaultAction(); - // Then: selectAndExecuteTask is called (branch is no longer passed via selectOptions) + // Then: selectAndExecuteTask is called expect(mockSelectAndExecuteTask).toHaveBeenCalledWith( '/test/cwd', 'summarized task', @@ -193,6 +198,20 @@ describe('PR resolution in routing', () => { ); }); + it('should checkout PR branch before executing task', async () => { + // Given + mockOpts.pr = 456; + const prReview = createMockPrReview({ headRefName: 'feat/my-pr-branch' }); + mockCheckCliStatus.mockReturnValue({ available: true }); + mockFetchPrReviewComments.mockReturnValue(prReview); + + // When + await executeDefaultAction(); + + // Then: checkoutBranch is called with the PR's head branch + expect(mockCheckoutBranch).toHaveBeenCalledWith('/test/cwd', 'feat/my-pr-branch'); + }); + it('should exit with error when gh CLI is unavailable', async () => { // Given mockOpts.pr = 456; @@ -229,6 +248,8 @@ describe('PR resolution in routing', () => { expect.stringContaining('## PR #456 Review Comments:'), expect.anything(), undefined, + undefined, + { excludeActions: ['create_issue'] }, ); }); diff --git a/src/app/cli/routing.ts b/src/app/cli/routing.ts index 6519e3b..f0f6196 100644 --- a/src/app/cli/routing.ts +++ b/src/app/cli/routing.ts @@ -5,12 +5,13 @@ * pipeline mode, or interactive mode. */ -import { info, error as logError, withProgress } from '../../shared/ui/index.js'; +import { info, success, error as logError, withProgress } from '../../shared/ui/index.js'; import { getErrorMessage } from '../../shared/utils/index.js'; import { getLabel } from '../../shared/i18n/index.js'; import { formatIssueAsTask, parseIssueNumbers, formatPrReviewAsTask } from '../../infra/github/index.js'; import { getGitProvider } from '../../infra/git/index.js'; import type { PrReviewData } from '../../infra/git/index.js'; +import { checkoutBranch } from '../../infra/task/index.js'; import { selectAndExecuteTask, determinePiece, saveTaskFromInteractive, createIssueAndSaveTask, promptLabelSelection, type SelectAndExecuteOptions } from '../../features/tasks/index.js'; import { executePipeline } from '../../features/pipeline/index.js'; import { @@ -85,7 +86,7 @@ async function resolveIssueInput( */ async function resolvePrInput( prNumber: number, -): Promise<{ initialInput: string }> { +): Promise<{ initialInput: string; prBranch: string }> { const ghStatus = getGitProvider().checkCliStatus(); if (!ghStatus.available) { throw new Error(ghStatus.error); @@ -97,7 +98,7 @@ async function resolvePrInput( async () => getGitProvider().fetchPrReviewComments(prNumber), ); - return { initialInput: formatPrReviewAsTask(prReview) }; + return { initialInput: formatPrReviewAsTask(prReview), prBranch: prReview.headRefName }; } /** @@ -169,11 +170,13 @@ export async function executeDefaultAction(task?: string): Promise { // Resolve PR review comments (--pr N) before interactive mode let initialInput: string | undefined = task; + let prBranch: string | undefined; if (prNumber) { try { const prResult = await resolvePrInput(prNumber); initialInput = prResult.initialInput; + prBranch = prResult.prBranch; } catch (e) { logError(getErrorMessage(e)); process.exit(1); @@ -234,7 +237,8 @@ export async function executeDefaultAction(task?: string): Promise { info(getLabel('interactive.continueNoSession', lang)); } } - result = await interactiveMode(resolvedCwd, initialInput, pieceContext, selectedSessionId); + const interactiveOpts = prBranch ? { excludeActions: ['create_issue'] as const } : undefined; + result = await interactiveMode(resolvedCwd, initialInput, pieceContext, selectedSessionId, undefined, interactiveOpts); break; } @@ -259,6 +263,11 @@ export async function executeDefaultAction(task?: string): Promise { await dispatchConversationAction(result, { execute: async ({ task: confirmedTask }) => { + if (prBranch) { + info(`Fetching and checking out PR branch: ${prBranch}`); + checkoutBranch(resolvedCwd, prBranch); + success(`Checked out PR branch: ${prBranch}`); + } selectOptions.interactiveUserInput = true; selectOptions.piece = pieceId; selectOptions.interactiveMetadata = { confirmed: true, task: confirmedTask }; @@ -273,7 +282,10 @@ export async function executeDefaultAction(task?: string): Promise { }); }, save_task: async ({ task: confirmedTask }) => { - await saveTaskFromInteractive(resolvedCwd, confirmedTask, pieceId); + const presetSettings = prBranch + ? { worktree: true as const, branch: prBranch, autoPr: false } + : undefined; + await saveTaskFromInteractive(resolvedCwd, confirmedTask, pieceId, { presetSettings }); }, cancel: () => undefined, }); diff --git a/src/features/interactive/index.ts b/src/features/interactive/index.ts index bceb902..1457d81 100644 --- a/src/features/interactive/index.ts +++ b/src/features/interactive/index.ts @@ -14,6 +14,7 @@ export { type TaskHistorySummaryItem, type InteractiveModeResult, type InteractiveModeAction, + type InteractiveModeOptions, } from './interactive.js'; export { selectInteractiveMode } from './modeSelection.js'; diff --git a/src/features/interactive/interactive.ts b/src/features/interactive/interactive.ts index b1d8b5d..91f7657 100644 --- a/src/features/interactive/interactive.ts +++ b/src/features/interactive/interactive.ts @@ -25,6 +25,10 @@ import { type PieceContext, formatMovementPreviews, type InteractiveModeAction, + type SummaryActionValue, + type PostSummaryAction, + buildSummaryActionOptions, + selectSummaryAction, } from './interactive-summary.js'; import { type RunSessionContext, formatRunSessionForPrompt } from './runSessionReader.js'; @@ -113,12 +117,18 @@ export { * /cancel → exits without executing * Ctrl+D → exits without executing */ +export interface InteractiveModeOptions { + /** Actions to exclude from the post-summary action selector. */ + excludeActions?: readonly SummaryActionValue[]; +} + export async function interactiveMode( cwd: string, initialInput?: string, pieceContext?: PieceContext, sessionId?: string, runSessionContext?: RunSessionContext, + options?: InteractiveModeOptions, ): Promise { const baseCtx = initializeSession(cwd, 'interactive'); const ctx = sessionId ? { ...baseCtx, sessionId } : baseCtx; @@ -155,11 +165,32 @@ export async function interactiveMode( return `## Policy\n${policyIntro}\n\n${policyContent}\n\n---\n\n${userMessage}\n\n---\n**Policy Reminder:** ${reminderLabel}`; } + const excludeActions = options?.excludeActions; + const selectAction = excludeActions?.length + ? (task: string): Promise => + selectSummaryAction( + task, + ui.proposed, + ui.actionPrompt, + buildSummaryActionOptions( + { + execute: ui.actions.execute, + createIssue: ui.actions.createIssue, + saveTask: ui.actions.saveTask, + continue: ui.actions.continue, + }, + ['create_issue'], + excludeActions, + ), + ) + : undefined; + return runConversationLoop(cwd, ctx, { systemPrompt, allowedTools: DEFAULT_INTERACTIVE_TOOLS, transformPrompt: injectPolicy, introMessage: ui.intro, + selectAction, }, pieceContext, initialInput); } diff --git a/src/features/pipeline/steps.ts b/src/features/pipeline/steps.ts index 1e8bdf9..5ee73a5 100644 --- a/src/features/pipeline/steps.ts +++ b/src/features/pipeline/steps.ts @@ -8,7 +8,7 @@ import { execFileSync } from 'node:child_process'; import { formatIssueAsTask, buildPrBody, formatPrReviewAsTask } from '../../infra/github/index.js'; import { getGitProvider, type Issue } from '../../infra/git/index.js'; -import { stageAndCommit, resolveBaseBranch, pushBranch } from '../../infra/task/index.js'; +import { stageAndCommit, resolveBaseBranch, pushBranch, checkoutBranch } from '../../infra/task/index.js'; import { executeTask, confirmAndCreateWorktree, type TaskExecutionOptions, type PipelineExecutionOptions } from '../tasks/index.js'; import { info, error, success } from '../../shared/ui/index.js'; import { getErrorMessage } from '../../shared/utils/index.js'; @@ -150,8 +150,7 @@ export async function resolveExecutionContext( } if (prBranch) { info(`Fetching and checking out PR branch: ${prBranch}`); - execFileSync('git', ['fetch', 'origin', prBranch], { cwd, stdio: 'pipe' }); - execFileSync('git', ['checkout', prBranch], { cwd, stdio: 'pipe' }); + checkoutBranch(cwd, prBranch); success(`Checked out PR branch: ${prBranch}`); return { execCwd: cwd, branch: prBranch, baseBranch: resolveBaseBranch(cwd).branch, isWorktree: false }; } diff --git a/src/features/tasks/add/index.ts b/src/features/tasks/add/index.ts index bc08ad3..1c1cccc 100644 --- a/src/features/tasks/add/index.ts +++ b/src/features/tasks/add/index.ts @@ -142,12 +142,13 @@ async function promptWorktreeSettings(): Promise { /** * Save a task from interactive mode result. * Prompts for worktree/branch/auto_pr settings before saving. + * If presetSettings is provided, skips the prompt and uses those settings directly. */ export async function saveTaskFromInteractive( cwd: string, task: string, piece?: string, - options?: { issue?: number; confirmAtEndMessage?: string }, + options?: { issue?: number; confirmAtEndMessage?: string; presetSettings?: WorktreeSettings }, ): Promise { if (options?.confirmAtEndMessage) { const approved = await confirm(options.confirmAtEndMessage, true); @@ -155,7 +156,7 @@ export async function saveTaskFromInteractive( return; } } - const settings = await promptWorktreeSettings(); + const settings = options?.presetSettings ?? await promptWorktreeSettings(); const created = await saveTaskFile(cwd, task, { piece, issue: options?.issue, ...settings }); displayTaskCreationResult(created, settings, piece); } diff --git a/src/infra/task/git.ts b/src/infra/task/git.ts index fd53bc3..e0c9953 100644 --- a/src/infra/task/git.ts +++ b/src/infra/task/git.ts @@ -40,6 +40,15 @@ export function stageAndCommit(cwd: string, message: string): string | undefined }).trim(); } +/** + * Fetches and checks out a branch from origin. Throws on failure. + */ +export function checkoutBranch(cwd: string, branch: string): void { + log.info('Checking out branch from origin', { branch }); + execFileSync('git', ['fetch', 'origin', branch], { cwd, stdio: 'pipe' }); + execFileSync('git', ['checkout', branch], { cwd, stdio: 'pipe' }); +} + /** * Throws on failure. */ diff --git a/src/infra/task/index.ts b/src/infra/task/index.ts index 5ea0a1e..49960d5 100644 --- a/src/infra/task/index.ts +++ b/src/infra/task/index.ts @@ -55,7 +55,7 @@ export { getOriginalInstruction, buildListItems, } from './branchList.js'; -export { stageAndCommit, getCurrentBranch, pushBranch } from './git.js'; +export { stageAndCommit, getCurrentBranch, pushBranch, checkoutBranch } from './git.js'; export { autoCommitAndPush, type AutoCommitResult } from './autoCommit.js'; export { summarizeTaskName } from './summarize.js'; export { TaskWatcher, type TaskWatcherOptions } from './watcher.js';