fix: --auto-pr/--draft をパイプラインモード専用に制限
インタラクティブモードでの PR 自動作成プロンプト（resolveAutoPr / resolveDraftPr）を削除し、 --auto-pr / --draft はパイプラインモードでのみ有効になるよう制限する。 SelectAndExecuteOptions から repo / branch / issues フィールドも削除。
This commit is contained in:
nrslib
parent
fa222915ea
commit
d3ac5cc17c
@ -14,7 +14,7 @@
|
||||
| `-w, --piece <name or path>` | Piece 名または piece YAML ファイルのパス |
|
||||
| `-b, --branch <name>` | ブランチ名を指定（省略時は自動生成） |
|
||||
| `--pr <number>` | PR 番号を指定してレビューコメントを取得し修正を実行 |
|
||||
| `--auto-pr` | PR を作成（インタラクティブ: 確認スキップ、pipeline: PR 有効化） |
|
||||
| `--auto-pr` | PR を作成（pipeline モードのみ） |
|
||||
| `--draft` | PR をドラフトとして作成（`--auto-pr` または `auto_pr` 設定が必要） |
|
||||
| `--skip-git` | ブランチ作成、コミット、プッシュをスキップ（pipeline モード、piece のみ実行） |
|
||||
| `--repo <owner/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`）がインストールされ、認証済みである必要があります。
|
||||
|
||||
@ -14,7 +14,7 @@ This document provides a complete reference for all TAKT CLI commands and option
|
||||
| `-w, --piece <name or path>` | Piece name or path to piece YAML file |
|
||||
| `-b, --branch <name>` | Specify branch name (auto-generated if omitted) |
|
||||
| `--pr <number>` | 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 <owner/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.
|
||||
|
||||
@ -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 () => {
|
||||
|
||||
@ -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,
|
||||
);
|
||||
});
|
||||
|
||||
@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@ -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 () => {
|
||||
|
||||
@ -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<void> {
|
||||
? 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<void> {
|
||||
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<void> {
|
||||
try {
|
||||
const issueResult = await resolveIssueInput(issueNumber, task);
|
||||
if (issueResult) {
|
||||
selectOptions.issues = issueResult.issues;
|
||||
initialInput = issueResult.initialInput;
|
||||
}
|
||||
} catch (e) {
|
||||
|
||||
@ -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<boolean> {
|
||||
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<boolean> {
|
||||
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<boolean> {
|
||||
return resolvePrBooleanOption(optionDraftPr, cwd, 'draftPr', 'Create as draft?');
|
||||
}
|
||||
|
||||
export interface PostExecutionOptions {
|
||||
execCwd: string;
|
||||
|
||||
@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
@ -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;
|
||||
}
|
||||
|
||||
@ -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 {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user