From d3ac5cc17cf2a89dbbc0c887f8698da2396ffb54 Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Mon, 2 Mar 2026 17:37:46 +0900 Subject: [PATCH] =?UTF-8?q?fix:=20--auto-pr/--draft=20=E3=82=92=E3=83=91?= =?UTF-8?q?=E3=82=A4=E3=83=97=E3=83=A9=E3=82=A4=E3=83=B3=E3=83=A2=E3=83=BC?= =?UTF-8?q?=E3=83=89=E5=B0=82=E7=94=A8=E3=81=AB=E5=88=B6=E9=99=90?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit インタラクティブモードでの PR 自動作成プロンプト（resolveAutoPr / resolveDraftPr）を削除し、 --auto-pr / --draft はパイプラインモードでのみ有効になるよう制限する。 SelectAndExecuteOptions から repo / branch / issues フィールドも削除。 --- docs/cli-reference.ja.md | 8 +--- docs/cli-reference.md | 8 +--- .../cli-routing-issue-resolve.test.ts | 47 +++++++++++++----- src/__tests__/cli-routing-pr-resolve.test.ts | 13 ++--- src/__tests__/postExecution.test.ts | 48 +------------------ src/__tests__/selectAndExecute-autoPr.test.ts | 5 +- src/app/cli/routing.ts | 22 ++++----- src/features/tasks/execute/postExecution.ts | 39 +-------------- .../tasks/execute/selectAndExecute.ts | 10 ++-- src/features/tasks/execute/types.ts | 6 --- src/features/tasks/index.ts | 2 +- 11 files changed, 62 insertions(+), 146 deletions(-) diff --git a/docs/cli-reference.ja.md b/docs/cli-reference.ja.md index 28b7d1c..6c2feba 100644 --- a/docs/cli-reference.ja.md +++ b/docs/cli-reference.ja.md @@ -14,7 +14,7 @@ | `-w, --piece ` | Piece 名または piece YAML ファイルのパス | | `-b, --branch ` | ブランチ名を指定（省略時は自動生成） | | `--pr ` | PR 番号を指定してレビューコメントを取得し修正を実行 | -| `--auto-pr` | PR を作成（インタラクティブ: 確認スキップ、pipeline: PR 有効化） | +| `--auto-pr` | PR を作成（pipeline モードのみ） | | `--draft` | PR をドラフトとして作成（`--auto-pr` または `auto_pr` 設定が必要） | | `--skip-git` | ブランチ作成、コミット、プッシュをスキップ（pipeline モード、piece のみ実行） | | `--repo ` | リポジトリを指定（PR 作成用） | @@ -101,9 +101,6 @@ takt --task "Fix bug" # piece を指定 takt --task "Add authentication" --piece dual - -# PR を自動作成 -takt --task "Fix bug" --auto-pr ``` **注意:** 引数として文字列を渡す場合（例: `takt "Add login feature"`）は、初期メッセージとしてインタラクティブモードに入ります。 @@ -119,9 +116,6 @@ takt --issue 6 # Issue + piece 指定 takt #6 --piece dual - -# Issue + PR 自動作成 -takt #6 --auto-pr ``` **要件:** [GitHub CLI](https://cli.github.com/)（`gh`）がインストールされ、認証済みである必要があります。 diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 7dafbc6..a97014b 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -14,7 +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) | | `--pr ` | PR number to fetch review comments and fix | -| `--auto-pr` | Create PR (interactive: skip confirmation, pipeline: enable PR) | +| `--auto-pr` | Create PR after execution (pipeline mode only) | | `--draft` | Create PR as draft (requires `--auto-pr` or `auto_pr` config) | | `--skip-git` | Skip branch creation, commit, and push (pipeline mode, piece-only) | | `--repo ` | Specify repository (for PR creation) | @@ -101,9 +101,6 @@ takt --task "Fix bug" # Specify piece takt --task "Add authentication" --piece dual - -# Auto-create PR -takt --task "Fix bug" --auto-pr ``` **Note:** Passing a string as an argument (e.g., `takt "Add login feature"`) enters interactive mode with it as the initial message. @@ -119,9 +116,6 @@ takt --issue 6 # Issue + piece specification takt #6 --piece dual - -# Issue + auto-create PR -takt #6 --auto-pr ``` **Requirements:** [GitHub CLI](https://cli.github.com/) (`gh`) must be installed and authenticated. diff --git a/src/__tests__/cli-routing-issue-resolve.test.ts b/src/__tests__/cli-routing-issue-resolve.test.ts index 8426388..d1f034f 100644 --- a/src/__tests__/cli-routing-issue-resolve.test.ts +++ b/src/__tests__/cli-routing-issue-resolve.test.ts @@ -179,6 +179,36 @@ describe('Issue resolution in routing', () => { mockExit.mockRestore(); }); + it('should show error and exit when only --auto-pr is used outside pipeline mode', async () => { + mockOpts.autoPr = true; + + const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit called'); + }); + + await expect(executeDefaultAction()).rejects.toThrow('process.exit called'); + + expect(mockError).toHaveBeenCalledWith('--auto-pr/--draft are supported only in --pipeline mode'); + expect(mockExit).toHaveBeenCalledWith(1); + + mockExit.mockRestore(); + }); + + it('should show error and exit when only --draft is used outside pipeline mode', async () => { + mockOpts.draft = true; + + const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit called'); + }); + + await expect(executeDefaultAction()).rejects.toThrow('process.exit called'); + + expect(mockError).toHaveBeenCalledWith('--auto-pr/--draft are supported only in --pipeline mode'); + expect(mockExit).toHaveBeenCalledWith(1); + + mockExit.mockRestore(); + }); + it('should show migration error and exit when deprecated --create-worktree is used', async () => { mockOpts.createWorktree = 'yes'; @@ -221,13 +251,11 @@ describe('Issue resolution in routing', () => { undefined, ); - // Then: selectAndExecuteTask should receive issues in options + // Then: selectAndExecuteTask should be called (issues are used only for initialInput, not selectOptions) expect(mockSelectAndExecuteTask).toHaveBeenCalledWith( '/test/cwd', 'summarized task', - expect.objectContaining({ - issues: [issue131], - }), + expect.any(Object), undefined, ); }); @@ -274,13 +302,11 @@ describe('Issue resolution in routing', () => { undefined, ); - // Then: selectAndExecuteTask should receive issues + // Then: selectAndExecuteTask should be called expect(mockSelectAndExecuteTask).toHaveBeenCalledWith( '/test/cwd', 'summarized task', - expect.objectContaining({ - issues: [issue131], - }), + expect.any(Object), undefined, ); }); @@ -302,9 +328,8 @@ describe('Issue resolution in routing', () => { // Then: no issue fetching should occur expect(mockFetchIssue).not.toHaveBeenCalled(); - // Then: selectAndExecuteTask should be called without issues - const callArgs = mockSelectAndExecuteTask.mock.calls[0]; - expect(callArgs?.[2]?.issues).toBeUndefined(); + // Then: selectAndExecuteTask should be called + expect(mockSelectAndExecuteTask).toHaveBeenCalledTimes(1); }); it('should enter interactive mode with no input when no args provided', async () => { diff --git a/src/__tests__/cli-routing-pr-resolve.test.ts b/src/__tests__/cli-routing-pr-resolve.test.ts index 8e77ccd..24d497a 100644 --- a/src/__tests__/cli-routing-pr-resolve.test.ts +++ b/src/__tests__/cli-routing-pr-resolve.test.ts @@ -1,9 +1,8 @@ /** * Tests for PR resolution in routing module. * - * Verifies that --pr option fetches review comments, - * passes formatted task to interactive mode, and sets - * the branch for worktree checkout. + * Verifies that --pr option fetches review comments + * and passes formatted task to interactive mode. */ import { describe, it, expect, vi, beforeEach } from 'vitest'; @@ -175,7 +174,7 @@ describe('PR resolution in routing', () => { ); }); - it('should set branch in selectOptions from PR headRefName', async () => { + it('should execute task after resolving PR review comments', async () => { // Given mockOpts.pr = 456; const prReview = createMockPrReview({ headRefName: 'feat/my-pr-branch' }); @@ -185,13 +184,11 @@ describe('PR resolution in routing', () => { // When await executeDefaultAction(); - // Then + // Then: selectAndExecuteTask is called (branch is no longer passed via selectOptions) expect(mockSelectAndExecuteTask).toHaveBeenCalledWith( '/test/cwd', 'summarized task', - expect.objectContaining({ - branch: 'feat/my-pr-branch', - }), + expect.any(Object), undefined, ); }); diff --git a/src/__tests__/postExecution.test.ts b/src/__tests__/postExecution.test.ts index 738c2c4..4623c6c 100644 --- a/src/__tests__/postExecution.test.ts +++ b/src/__tests__/postExecution.test.ts @@ -33,14 +33,6 @@ vi.mock('../infra/github/index.js', () => ({ buildPrBody: (...args: unknown[]) => mockBuildPrBody(...args), })); -vi.mock('../infra/config/index.js', () => ({ - resolvePieceConfigValue: vi.fn(), -})); - -vi.mock('../shared/prompt/index.js', () => ({ - confirm: vi.fn(), -})); - vi.mock('../shared/ui/index.js', () => ({ info: vi.fn(), error: vi.fn(), @@ -56,12 +48,7 @@ vi.mock('../shared/utils/index.js', async (importOriginal) => ({ }), })); -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); +import { postExecutionFlow } from '../features/tasks/execute/postExecution.js'; const baseOptions = { execCwd: '/clone', @@ -237,36 +224,3 @@ describe('postExecutionFlow', () => { }); }); -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__/selectAndExecute-autoPr.test.ts b/src/__tests__/selectAndExecute-autoPr.test.ts index 29eaa95..c4f421e 100644 --- a/src/__tests__/selectAndExecute-autoPr.test.ts +++ b/src/__tests__/selectAndExecute-autoPr.test.ts @@ -37,8 +37,6 @@ vi.mock('../infra/task/index.js', () => ({ createSharedClone: vi.fn(), autoCommitAndPush: vi.fn(), summarizeTaskName: vi.fn(), - getCurrentBranch: vi.fn(() => 'main'), - detectDefaultBranch: vi.fn(() => 'main'), resolveBaseBranch: vi.fn(() => ({ branch: 'main' })), TaskRunner: vi.fn(() => ({ addTask: (...args: unknown[]) => mockAddTask(...args), @@ -104,6 +102,9 @@ describe('selectAndExecuteTask (execute path)', () => { expect(mockAutoCommitAndPush).not.toHaveBeenCalled(); expect(mockAddTask).toHaveBeenCalledWith('test task', { piece: 'default' }); + expect(mockExecuteTask).toHaveBeenCalledWith( + expect.objectContaining({ cwd: '/project', projectCwd: '/project' }), + ); }); it('should call selectPiece when no override is provided', async () => { diff --git a/src/app/cli/routing.ts b/src/app/cli/routing.ts index 4dc6911..d52de5d 100644 --- a/src/app/cli/routing.ts +++ b/src/app/cli/routing.ts @@ -10,7 +10,7 @@ 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 { Issue, PrReviewData } from '../../infra/git/index.js'; +import type { PrReviewData } from '../../infra/git/index.js'; import { selectAndExecuteTask, determinePiece, saveTaskFromInteractive, createIssueAndSaveTask, promptLabelSelection, type SelectAndExecuteOptions } from '../../features/tasks/index.js'; import { executePipeline } from '../../features/pipeline/index.js'; import { @@ -35,13 +35,13 @@ import { loadTaskHistory } from './taskHistory.js'; * - --issue N option (numeric issue number) * - Positional argument containing issue references (#N or "#1 #2") * - * Returns resolved issues and the formatted task text for interactive mode. + * Returns the formatted task text for interactive mode. * Throws on gh CLI unavailability or fetch failure. */ async function resolveIssueInput( issueOption: number | undefined, task: string | undefined, -): Promise<{ issues: Issue[]; initialInput: string } | null> { +): Promise<{ initialInput: string } | null> { if (issueOption) { const ghStatus = getGitProvider().checkCliStatus(); if (!ghStatus.available) { @@ -52,7 +52,7 @@ async function resolveIssueInput( (fetchedIssue) => `GitHub Issue fetched: #${fetchedIssue.number} ${fetchedIssue.title}`, async () => getGitProvider().fetchIssue(issueOption), ); - return { issues: [issue], initialInput: formatIssueAsTask(issue) }; + return { initialInput: formatIssueAsTask(issue) }; } if (task && isDirectTask(task)) { @@ -70,7 +70,7 @@ async function resolveIssueInput( (fetchedIssues) => `GitHub Issues fetched: ${fetchedIssues.map((issue) => `#${issue.number}`).join(', ')}`, async () => issueNumbers.map((n) => getGitProvider().fetchIssue(n)), ); - return { issues, initialInput: issues.map(formatIssueAsTask).join('\n\n---\n\n') }; + return { initialInput: issues.map(formatIssueAsTask).join('\n\n---\n\n') }; } return null; @@ -80,12 +80,12 @@ async function resolveIssueInput( * Resolve PR review comments from `--pr` option. * * Fetches review comments and metadata, formats as task text. - * Returns the PR branch name for checkout and the formatted task. + * Returns the formatted task text for interactive mode. * Throws on gh CLI unavailability or fetch failure. */ async function resolvePrInput( prNumber: number, -): Promise<{ initialInput: string; prBranch: string }> { +): Promise<{ initialInput: string }> { const ghStatus = getGitProvider().checkCliStatus(); if (!ghStatus.available) { throw new Error(ghStatus.error); @@ -101,10 +101,7 @@ async function resolvePrInput( throw new Error(`PR #${prNumber} has no review comments`); } - return { - initialInput: formatPrReviewAsTask(prReview), - prBranch: prReview.headRefName, - }; + return { initialInput: formatPrReviewAsTask(prReview) }; } /** @@ -146,7 +143,6 @@ export async function executeDefaultAction(task?: string): Promise { ? true : (resolveConfigValue(resolvedCwd, 'draftPr') ?? false); const selectOptions: SelectAndExecuteOptions = { - repo: opts.repo as string | undefined, piece: opts.piece as string | undefined, }; @@ -190,7 +186,6 @@ export async function executeDefaultAction(task?: string): Promise { try { const prResult = await resolvePrInput(prNumber); initialInput = prResult.initialInput; - selectOptions.branch = prResult.prBranch; } catch (e) { logError(getErrorMessage(e)); process.exit(1); @@ -200,7 +195,6 @@ export async function executeDefaultAction(task?: string): Promise { try { const issueResult = await resolveIssueInput(issueNumber, task); if (issueResult) { - selectOptions.issues = issueResult.issues; initialInput = issueResult.initialInput; } } catch (e) { diff --git a/src/features/tasks/execute/postExecution.ts b/src/features/tasks/execute/postExecution.ts index 7ce2364..09410b2 100644 --- a/src/features/tasks/execute/postExecution.ts +++ b/src/features/tasks/execute/postExecution.ts @@ -1,12 +1,10 @@ /** * Shared post-execution logic: auto-commit, push, and PR creation. * - * Used by both selectAndExecuteTask (interactive mode) and - * instructBranch (instruct mode from takt list). + * Used by taskExecution (takt run / watch path) and + * instructBranch (takt list). */ -import { resolvePieceConfigValue } from '../../../infra/config/index.js'; -import { confirm } from '../../../shared/prompt/index.js'; import { autoCommitAndPush, pushBranch } from '../../../infra/task/index.js'; import { info, error, success } from '../../../shared/ui/index.js'; import { createLogger } from '../../../shared/utils/index.js'; @@ -16,39 +14,6 @@ import type { Issue } from '../../../infra/git/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 { - return resolvePrBooleanOption(optionAutoPr, cwd, 'autoPr', 'Create pull request?'); -} - -/** - * 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 { execCwd: string; diff --git a/src/features/tasks/execute/selectAndExecute.ts b/src/features/tasks/execute/selectAndExecute.ts index 5fda6d0..6cfbec5 100644 --- a/src/features/tasks/execute/selectAndExecute.ts +++ b/src/features/tasks/execute/selectAndExecute.ts @@ -1,9 +1,8 @@ /** * Task execution orchestration. * - * Coordinates piece selection and task execution, - * auto-commit, and PR creation. Extracted from cli.ts to avoid - * mixing CLI parsing with business logic. + * Coordinates piece selection and in-place task execution. + * Extracted from cli.ts to avoid mixing CLI parsing with business logic. */ import { @@ -126,11 +125,10 @@ export async function selectAndExecuteTask( const completedAt = new Date().toISOString(); - const effectiveSuccess = taskSuccess; if (taskRecord) { const taskResult = buildBooleanTaskResult({ task: taskRecord, - taskSuccess: effectiveSuccess, + taskSuccess, successResponse: 'Task completed successfully', failureResponse: 'Task failed', startedAt, @@ -139,7 +137,7 @@ export async function selectAndExecuteTask( persistTaskResult(taskRunner, taskResult); } - if (!effectiveSuccess) { + if (!taskSuccess) { process.exit(1); } } diff --git a/src/features/tasks/execute/types.ts b/src/features/tasks/execute/types.ts index 78a1b34..b949307 100644 --- a/src/features/tasks/execute/types.ts +++ b/src/features/tasks/execute/types.ts @@ -7,7 +7,6 @@ import type { PersonaProviderEntry } from '../../../core/models/persisted-global import type { ProviderPermissionProfiles } from '../../../core/models/provider-profiles.js'; import type { MovementProviderOptions } from '../../../core/models/piece-types.js'; import type { ProviderType } from '../../../infra/providers/index.js'; -import type { Issue } from '../../../infra/git/index.js'; import type { ProviderOptionsSource } from '../../../core/piece/types.js'; /** Result of piece execution */ @@ -136,16 +135,11 @@ export interface WorktreeConfirmationResult { } export interface SelectAndExecuteOptions { - repo?: string; piece?: string; - /** Override branch name (e.g., PR head branch for --pr) */ - branch?: string; /** Enable interactive user input during step transitions */ interactiveUserInput?: boolean; /** Interactive mode result metadata for NDJSON logging */ interactiveMetadata?: InteractiveMetadata; - /** GitHub Issues to associate with the PR (adds "Closes #N" for each issue) */ - issues?: Issue[]; /** Skip adding task to tasks.yaml */ skipTaskList?: boolean; } diff --git a/src/features/tasks/index.ts b/src/features/tasks/index.ts index c2b5f54..0502595 100644 --- a/src/features/tasks/index.ts +++ b/src/features/tasks/index.ts @@ -15,7 +15,7 @@ export { type SelectAndExecuteOptions, type WorktreeConfirmationResult, } from './execute/selectAndExecute.js'; -export { resolveAutoPr, postExecutionFlow, type PostExecutionOptions } from './execute/postExecution.js'; +export { postExecutionFlow, type PostExecutionOptions } from './execute/postExecution.js'; export { addTask, saveTaskFile, saveTaskFromInteractive, createIssueFromTask, createIssueAndSaveTask, promptLabelSelection } from './add/index.js'; export { watchTasks } from './watch/index.js'; export {