[#426] add-pr-review-task (#427)

* takt: add-pr-review-task

* fix: add コマンドの DRY 違反を修正

if/else で addTask を引数の有無のみ変えて呼び分けていた
冗長な分岐を三項演算子で統一。テストのアサーションも更新。

* fix: レビュー Warning 4件を修正

- addTask.test.ts: 冗長な Ref エイリアスを削除し直接参照に統一
- addTask.test.ts: mockRejectedValue を mockImplementation(throw) に変更
  (fetchPrReviewComments は同期メソッドのため)
- index.ts: addTask の JSDoc Flow コメントを復元(PR フロー追加)
- issueTask.ts: extractTitle / createIssueFromTask の JSDoc を移植
This commit is contained in:
nrs 2026-02-28 21:56:00 +09:00 committed by GitHub
parent 2be824b231
commit 8f0f546928
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 430 additions and 99 deletions

View File

@ -4,6 +4,9 @@ import * as path from 'node:path';
import { tmpdir } from 'node:os';
import { parse as parseYaml } from 'yaml';
const mockCheckCliStatus = vi.fn();
const mockFetchPrReviewComments = vi.fn();
vi.mock('../features/interactive/index.js', () => ({
interactiveMode: vi.fn(),
}));
@ -42,13 +45,14 @@ vi.mock('../infra/task/index.js', async (importOriginal) => ({
vi.mock('../infra/git/index.js', () => ({
getGitProvider: () => ({
createIssue: vi.fn(),
checkCliStatus: (...args: unknown[]) => mockCheckCliStatus(...args),
fetchPrReviewComments: (...args: unknown[]) => mockFetchPrReviewComments(...args),
}),
}));
vi.mock('../infra/github/issue.js', () => ({
isIssueReference: vi.fn((s: string) => /^#\d+$/.test(s)),
resolveIssueTask: vi.fn(),
parseIssueNumbers: vi.fn((args: string[]) => {
const mockIsIssueReference = vi.fn((s: string) => /^#\d+$/.test(s));
const mockResolveIssueTask = vi.fn();
const mockParseIssueNumbers = vi.fn((args: string[]) => {
const numbers: number[] = [];
for (const arg of args) {
const match = arg.match(/^#(\d+)$/);
@ -57,22 +61,29 @@ vi.mock('../infra/github/issue.js', () => ({
}
}
return numbers;
}),
});
const mockFormatPrReviewAsTask = vi.fn();
vi.mock('../infra/github/index.js', () => ({
isIssueReference: (...args: unknown[]) => mockIsIssueReference(...args),
resolveIssueTask: (...args: unknown[]) => mockResolveIssueTask(...args),
parseIssueNumbers: (...args: unknown[]) => mockParseIssueNumbers(...args),
formatPrReviewAsTask: (...args: unknown[]) => mockFormatPrReviewAsTask(...args),
}));
import { interactiveMode } from '../features/interactive/index.js';
import { promptInput, confirm } from '../shared/prompt/index.js';
import { info } from '../shared/ui/index.js';
import { error, info } from '../shared/ui/index.js';
import { determinePiece } from '../features/tasks/execute/selectAndExecute.js';
import { resolveIssueTask } from '../infra/github/issue.js';
import { addTask } from '../features/tasks/index.js';
import type { PrReviewData } from '../infra/git/index.js';
const mockInteractiveMode = vi.mocked(interactiveMode);
const mockPromptInput = vi.mocked(promptInput);
const mockConfirm = vi.mocked(confirm);
const mockInfo = vi.mocked(info);
const mockError = vi.mocked(error);
const mockDeterminePiece = vi.mocked(determinePiece);
const mockResolveIssueTask = vi.mocked(resolveIssueTask);
let testDir: string;
@ -81,11 +92,30 @@ function loadTasks(dir: string): { tasks: Array<Record<string, unknown>> } {
return parseYaml(raw) as { tasks: Array<Record<string, unknown>> };
}
function addTaskWithPrOption(cwd: string, task: string, prNumber: number): Promise<void> {
return addTask(cwd, task, { prNumber });
}
function createMockPrReview(overrides: Partial<PrReviewData> = {}): PrReviewData {
return {
number: 456,
title: 'Fix auth bug',
body: 'PR description',
url: 'https://github.com/org/repo/pull/456',
headRefName: 'feature/fix-auth-bug',
comments: [{ author: 'commenter', body: 'Please update tests' }],
reviews: [{ author: 'reviewer', body: 'Fix null check' }],
files: ['src/auth.ts'],
...overrides,
};
}
beforeEach(() => {
vi.clearAllMocks();
testDir = fs.mkdtempSync(path.join(tmpdir(), 'takt-test-'));
mockDeterminePiece.mockResolvedValue('default');
mockConfirm.mockResolvedValue(false);
mockCheckCliStatus.mockReturnValue({ available: true });
});
afterEach(() => {
@ -145,12 +175,105 @@ describe('addTask', () => {
await addTask(testDir, '#99');
expect(mockInteractiveMode).not.toHaveBeenCalled();
expect(mockIsIssueReference).toHaveBeenCalledWith('#99');
expect(mockParseIssueNumbers).toHaveBeenCalledWith(['#99']);
expect(mockResolveIssueTask).toHaveBeenCalledWith('#99');
expect(mockCheckCliStatus).not.toHaveBeenCalled();
const task = loadTasks(testDir).tasks[0]!;
expect(task.content).toBeUndefined();
expect(readOrderContent(testDir, task.task_dir)).toContain('Fix login timeout');
expect(task.issue).toBe(99);
});
it('should create task from PR review comments with PR-specific task settings', async () => {
const prReview = createMockPrReview();
const formattedTask = '## PR #456 Review Comments: Fix auth bug';
mockFetchPrReviewComments.mockReturnValue(prReview);
mockFormatPrReviewAsTask.mockReturnValue(formattedTask);
await addTaskWithPrOption(testDir, 'placeholder', 456);
expect(mockCheckCliStatus).toHaveBeenCalled();
expect(mockCheckCliStatus.mock.invocationCallOrder[0]).toBeLessThan(
mockFetchPrReviewComments.mock.invocationCallOrder[0],
);
expect(mockFetchPrReviewComments).toHaveBeenCalledWith(456);
expect(mockFormatPrReviewAsTask).toHaveBeenCalledWith(prReview);
expect(mockIsIssueReference).not.toHaveBeenCalled();
expect(mockParseIssueNumbers).not.toHaveBeenCalled();
expect(mockResolveIssueTask).not.toHaveBeenCalled();
expect(mockPromptInput).not.toHaveBeenCalled();
expect(mockConfirm).not.toHaveBeenCalled();
expect(mockDeterminePiece).toHaveBeenCalledTimes(1);
const task = loadTasks(testDir).tasks[0]!;
expect(task.content).toBeUndefined();
expect(task.branch).toBe('feature/fix-auth-bug');
expect(task.auto_pr).toBe(false);
expect(task.worktree).toBe(true);
expect(task.draft_pr).toBeUndefined();
expect(readOrderContent(testDir, task.task_dir)).toContain(formattedTask);
});
it('should not create a PR task when PR has no review comments', async () => {
const prReview = createMockPrReview({ comments: [], reviews: [] });
mockFetchPrReviewComments.mockReturnValue(prReview);
await addTaskWithPrOption(testDir, 'placeholder', 456);
expect(mockCheckCliStatus).toHaveBeenCalled();
expect(mockFetchPrReviewComments).toHaveBeenCalledWith(456);
expect(mockFormatPrReviewAsTask).not.toHaveBeenCalled();
expect(mockDeterminePiece).not.toHaveBeenCalled();
expect(mockError).toHaveBeenCalled();
expect(fs.existsSync(path.join(testDir, '.takt', 'tasks.yaml'))).toBe(false);
});
it('should show error and not create task when fetchPrReviewComments throws', async () => {
mockFetchPrReviewComments.mockImplementation(() => { throw new Error('network timeout'); });
await addTaskWithPrOption(testDir, 'placeholder', 456);
expect(mockCheckCliStatus).toHaveBeenCalled();
expect(mockFetchPrReviewComments).toHaveBeenCalledWith(456);
expect(mockFormatPrReviewAsTask).not.toHaveBeenCalled();
expect(mockDeterminePiece).not.toHaveBeenCalled();
expect(mockError).toHaveBeenCalledWith(expect.stringContaining('network timeout'));
expect(fs.existsSync(path.join(testDir, '.takt', 'tasks.yaml'))).toBe(false);
});
it('should not create a PR task when CLI is unavailable', async () => {
mockCheckCliStatus.mockReturnValue({ available: false, error: 'gh CLI is not available' });
await addTaskWithPrOption(testDir, 'placeholder', 456);
expect(mockFetchPrReviewComments).not.toHaveBeenCalled();
expect(mockFormatPrReviewAsTask).not.toHaveBeenCalled();
expect(mockDeterminePiece).not.toHaveBeenCalled();
expect(mockError).toHaveBeenCalled();
expect(fs.existsSync(path.join(testDir, '.takt', 'tasks.yaml'))).toBe(false);
});
it('should not perform issue parsing when PR task text looks like issue reference', async () => {
const prReview = createMockPrReview();
const formattedTask = '## PR #456 Review Comments: Fix auth bug';
mockFetchPrReviewComments.mockReturnValue(prReview);
mockFormatPrReviewAsTask.mockReturnValue(formattedTask);
await addTaskWithPrOption(testDir, '#99', 456);
expect(mockIsIssueReference).not.toHaveBeenCalled();
expect(mockParseIssueNumbers).not.toHaveBeenCalled();
expect(mockResolveIssueTask).not.toHaveBeenCalled();
expect(mockCheckCliStatus).toHaveBeenCalled();
expect(mockFetchPrReviewComments).toHaveBeenCalledWith(456);
expect(mockFormatPrReviewAsTask).toHaveBeenCalledWith(prReview);
const task = loadTasks(testDir).tasks[0]!;
expect(task.content).toBeUndefined();
expect(task.branch).toBe('feature/fix-auth-bug');
expect(task.auto_pr).toBe(false);
});
it('should not create task when piece selection is cancelled', async () => {
mockDeterminePiece.mockResolvedValue(null);
@ -158,4 +281,20 @@ describe('addTask', () => {
expect(fs.existsSync(path.join(testDir, '.takt', 'tasks.yaml'))).toBe(false);
});
it('should not save PR task when piece selection is cancelled', async () => {
const prReview = createMockPrReview();
const formattedTask = '## PR #456 Review Comments: Fix auth bug';
mockDeterminePiece.mockResolvedValue(null);
mockFetchPrReviewComments.mockReturnValue(prReview);
mockFormatPrReviewAsTask.mockReturnValue(formattedTask);
await addTaskWithPrOption(testDir, 'placeholder', 456);
expect(mockCheckCliStatus).toHaveBeenCalled();
expect(mockFetchPrReviewComments).toHaveBeenCalledWith(456);
expect(mockFormatPrReviewAsTask).toHaveBeenCalledWith(prReview);
expect(mockDeterminePiece).toHaveBeenCalledTimes(1);
expect(fs.existsSync(path.join(testDir, '.takt', 'tasks.yaml'))).toBe(false);
});
});

View File

@ -0,0 +1,144 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
const mockOpts: Record<string, unknown> = {};
const mockAddTask = vi.fn();
const { rootCommand, commandActions } = vi.hoisted(() => {
const commandActions = new Map<string, (...args: unknown[]) => void>();
function createCommandMock(actionKey: string): {
description: ReturnType<typeof vi.fn>;
argument: ReturnType<typeof vi.fn>;
option: ReturnType<typeof vi.fn>;
opts: ReturnType<typeof vi.fn>;
action: (action: (...args: unknown[]) => void) => unknown;
command: ReturnType<typeof vi.fn>;
} {
const command: Record<string, unknown> = {
description: vi.fn().mockReturnThis(),
argument: vi.fn().mockReturnThis(),
option: vi.fn().mockReturnThis(),
opts: vi.fn(() => mockOpts),
};
command.command = vi.fn((subName: string) => createCommandMock(`${actionKey}.${subName}`));
command.action = vi.fn((action: (...args: unknown[]) => void) => {
commandActions.set(actionKey, action);
return command;
});
return command as {
description: ReturnType<typeof vi.fn>;
argument: ReturnType<typeof vi.fn>;
option: ReturnType<typeof vi.fn>;
opts: ReturnType<typeof vi.fn>;
action: (action: (...args: unknown[]) => void) => unknown;
command: ReturnType<typeof vi.fn>;
};
}
return {
rootCommand: createCommandMock('root'),
commandActions,
};
});
vi.mock('../app/cli/program.js', () => ({
program: rootCommand,
resolvedCwd: '/test/cwd',
pipelineMode: false,
}));
vi.mock('../infra/config/index.js', () => ({
resolveConfigValue: vi.fn(),
}));
vi.mock('../infra/config/paths.js', () => ({
getGlobalConfigDir: vi.fn(() => '/tmp/takt'),
}));
vi.mock('../shared/ui/index.js', () => ({
success: vi.fn(),
info: vi.fn(),
}));
vi.mock('../features/tasks/index.js', () => ({
runAllTasks: vi.fn(),
addTask: (...args: unknown[]) => mockAddTask(...args),
watchTasks: vi.fn(),
listTasks: vi.fn(),
}));
vi.mock('../features/config/index.js', () => ({
clearPersonaSessions: vi.fn(),
switchPiece: vi.fn(),
ejectBuiltin: vi.fn(),
ejectFacet: vi.fn(),
parseFacetType: vi.fn(),
VALID_FACET_TYPES: ['personas', 'policies', 'knowledge', 'instructions', 'output-contracts'],
resetCategoriesToDefault: vi.fn(),
resetConfigToDefault: vi.fn(),
deploySkill: vi.fn(),
}));
vi.mock('../features/prompt/index.js', () => ({
previewPrompts: vi.fn(),
}));
vi.mock('../features/catalog/index.js', () => ({
showCatalog: vi.fn(),
}));
vi.mock('../features/analytics/index.js', () => ({
computeReviewMetrics: vi.fn(),
formatReviewMetrics: vi.fn(),
parseSinceDuration: vi.fn(),
purgeOldEvents: vi.fn(),
}));
vi.mock('../commands/repertoire/add.js', () => ({
repertoireAddCommand: vi.fn(),
}));
vi.mock('../commands/repertoire/remove.js', () => ({
repertoireRemoveCommand: vi.fn(),
}));
vi.mock('../commands/repertoire/list.js', () => ({
repertoireListCommand: vi.fn(),
}));
import '../app/cli/commands.js';
describe('CLI add command', () => {
beforeEach(() => {
vi.clearAllMocks();
for (const key of Object.keys(mockOpts)) {
delete mockOpts[key];
}
});
describe('when --pr option is provided', () => {
it('should pass program.opts().pr to addTask as prNumber', async () => {
const prNumber = 374;
mockOpts.pr = prNumber;
const addAction = commandActions.get('root.add');
expect(addAction).toBeTypeOf('function');
await addAction?.();
expect(mockAddTask).toHaveBeenCalledWith('/test/cwd', undefined, { prNumber });
});
});
describe('when --pr option is omitted', () => {
it('should keep existing addTask call signature', async () => {
const addAction = commandActions.get('root.add');
expect(addAction).toBeTypeOf('function');
await addAction?.('Regular task');
expect(mockAddTask).toHaveBeenCalledWith('/test/cwd', 'Regular task', undefined);
});
});
});

View File

@ -39,7 +39,8 @@ program
.description('Add a new task')
.argument('[task]', 'Task description or GitHub issue reference (e.g. "#28")')
.action(async (task?: string) => {
await addTask(resolvedCwd, task);
const opts = program.opts();
await addTask(resolvedCwd, task, opts.pr !== undefined ? { prNumber: opts.pr as number } : undefined);
});
program

View File

@ -13,9 +13,11 @@ import type { Language } from '../../../core/models/types.js';
import { TaskRunner, type TaskFileData, summarizeTaskName } from '../../../infra/task/index.js';
import { determinePiece } from '../execute/selectAndExecute.js';
import { createLogger, getErrorMessage, generateReportDir } from '../../../shared/utils/index.js';
import { isIssueReference, resolveIssueTask, parseIssueNumbers } from '../../../infra/github/index.js';
import { getGitProvider } from '../../../infra/git/index.js';
import { isIssueReference, resolveIssueTask, parseIssueNumbers, formatPrReviewAsTask } from '../../../infra/github/index.js';
import { getGitProvider, type PrReviewData } from '../../../infra/git/index.js';
import { firstLine } from '../../../infra/task/naming.js';
import { extractTitle, createIssueFromTask } from './issueTask.js';
export { extractTitle, createIssueFromTask };
const log = createLogger('add-task');
@ -70,60 +72,6 @@ export async function saveTaskFile(
return { taskName: created.name, tasksFile };
}
const TITLE_MAX_LENGTH = 100;
const TITLE_TRUNCATE_LENGTH = 97;
const MARKDOWN_HEADING_PATTERN = /^#{1,3}\s+\S/;
/**
* Extract a clean title from a task description.
*
* Prefers the first Markdown heading (h1-h3) if present.
* Falls back to the first non-empty line otherwise.
* Truncates to 100 characters (97 + "...") when exceeded.
*/
export function extractTitle(task: string): string {
const lines = task.split('\n');
const headingLine = lines.find((l) => MARKDOWN_HEADING_PATTERN.test(l));
const titleLine = headingLine
? headingLine.replace(/^#{1,3}\s+/, '')
: (lines.find((l) => l.trim().length > 0) ?? task);
return titleLine.length > TITLE_MAX_LENGTH
? `${titleLine.slice(0, TITLE_TRUNCATE_LENGTH)}...`
: titleLine;
}
/**
* Create a GitHub Issue from a task description.
*
* Extracts the first Markdown heading (h1-h3) as the issue title,
* falling back to the first non-empty line. Truncates to 100 chars.
* Uses the full task as the body, and displays success/error messages.
*/
export function createIssueFromTask(task: string, options?: { labels?: string[] }): number | undefined {
info('Creating GitHub Issue...');
const title = extractTitle(task);
const effectiveLabels = options?.labels?.filter((l) => l.length > 0) ?? [];
const labels = effectiveLabels.length > 0 ? effectiveLabels : undefined;
const issueResult = getGitProvider().createIssue({ title, body: task, labels });
if (issueResult.success) {
if (!issueResult.url) {
error('Failed to extract issue number from URL');
return undefined;
}
success(`Issue created: ${issueResult.url}`);
const num = Number(issueResult.url.split('/').pop());
if (Number.isNaN(num)) {
error('Failed to extract issue number from URL');
return undefined;
}
return num;
} else {
error(`Failed to create issue: ${issueResult.error}`);
return undefined;
}
}
interface WorktreeSettings {
worktree?: boolean | string;
branch?: string;
@ -153,27 +101,6 @@ function displayTaskCreationResult(
if (piece) info(` Piece: ${piece}`);
}
/**
* Create a GitHub Issue and save the task to .takt/tasks.yaml.
*
* Combines issue creation and task saving into a single workflow.
* If issue creation fails, no task is saved.
*/
export async function createIssueAndSaveTask(
cwd: string,
task: string,
piece?: string,
options?: { confirmAtEndMessage?: string; labels?: string[] },
): Promise<void> {
const issueNumber = createIssueFromTask(task, { labels: options?.labels });
if (issueNumber !== undefined) {
await saveTaskFromInteractive(cwd, task, piece, {
issue: issueNumber,
confirmAtEndMessage: options?.confirmAtEndMessage,
});
}
}
/**
* Prompt user to select a label for the GitHub Issue.
*
@ -233,17 +160,83 @@ export async function saveTaskFromInteractive(
displayTaskCreationResult(created, settings, piece);
}
export async function createIssueAndSaveTask(
cwd: string,
task: string,
piece?: string,
options?: { confirmAtEndMessage?: string; labels?: string[] },
): Promise<void> {
const issueNumber = createIssueFromTask(task, { labels: options?.labels });
if (issueNumber === undefined) {
return;
}
await saveTaskFromInteractive(cwd, task, piece, {
issue: issueNumber,
confirmAtEndMessage: options?.confirmAtEndMessage,
});
}
/**
* add command handler
*
* Flow:
* A) 引数なし: Usage表示して終了
* B) Issue参照の場合: issue取得 YAML作成
* C) 通常入力: 引数をそのまま保存
* A) --pr オプション: PRレビュー取得 YAML作成
* B) 引数なし: Usage表示して終了
* C) Issue参照の場合: issue取得 YAML作成
* D) 通常入力: ピース選択 YAML作成
*/
export async function addTask(cwd: string, task?: string): Promise<void> {
export async function addTask(
cwd: string,
task?: string,
opts?: { prNumber?: number },
): Promise<void> {
const rawTask = task ?? '';
const trimmedTask = rawTask.trim();
const prNumber = opts?.prNumber;
if (prNumber !== undefined) {
const provider = getGitProvider();
const ghStatus = provider.checkCliStatus();
if (!ghStatus.available) {
error(ghStatus.error ?? 'GitHub CLI is unavailable');
return;
}
let prReview: PrReviewData;
try {
prReview = await withProgress(
'Fetching PR review comments...',
(fetchedPrReview: PrReviewData) => `PR fetched: #${fetchedPrReview.number} ${fetchedPrReview.title}`,
async () => provider.fetchPrReviewComments(prNumber),
);
} catch (e) {
const msg = getErrorMessage(e);
error(`Failed to fetch PR review comments #${prNumber}: ${msg}`);
return;
}
if (prReview.reviews.length === 0 && prReview.comments.length === 0) {
error(`PR #${prNumber} has no review comments`);
return;
}
const taskContent = formatPrReviewAsTask(prReview);
const piece = await determinePiece(cwd);
if (piece === null) {
info('Cancelled.');
return;
}
const settings = {
worktree: true,
branch: prReview.headRefName,
autoPr: false,
};
const created = await saveTaskFile(cwd, taskContent, { piece, ...settings });
displayTaskCreationResult(created, settings, piece);
return;
}
if (!trimmedTask) {
info('Usage: takt add <task>');
return;
@ -253,7 +246,6 @@ export async function addTask(cwd: string, task?: string): Promise<void> {
let issueNumber: number | undefined;
if (isIssueReference(trimmedTask)) {
// Issue reference: fetch issue and use directly as task content
try {
const numbers = parseIssueNumbers([trimmedTask]);
const primaryIssueNumber = numbers[0];
@ -283,7 +275,6 @@ export async function addTask(cwd: string, task?: string): Promise<void> {
const settings = await promptWorktreeSettings();
// YAMLファイル作成
const created = await saveTaskFile(cwd, taskContent, {
piece,
issue: issueNumber,

View File

@ -0,0 +1,56 @@
import { info, success, error } from '../../../shared/ui/index.js';
import { getGitProvider } from '../../../infra/git/index.js';
const TITLE_MAX_LENGTH = 100;
const TITLE_TRUNCATE_LENGTH = 97;
const MARKDOWN_HEADING_PATTERN = /^#{1,3}\s+\S/;
/**
* Extract a clean title from a task description.
*
* Prefers the first Markdown heading (h1-h3) if present.
* Falls back to the first non-empty line otherwise.
* Truncates to 100 characters (97 + "...") when exceeded.
*/
export function extractTitle(task: string): string {
const lines = task.split('\n');
const headingLine = lines.find((l) => MARKDOWN_HEADING_PATTERN.test(l));
const titleLine = headingLine
? headingLine.replace(/^#{1,3}\s+/, '')
: (lines.find((l) => l.trim().length > 0) ?? task);
return titleLine.length > TITLE_MAX_LENGTH
? `${titleLine.slice(0, TITLE_TRUNCATE_LENGTH)}...`
: titleLine;
}
/**
* Create a GitHub Issue from a task description.
*
* Extracts the first Markdown heading (h1-h3) as the issue title,
* falling back to the first non-empty line. Truncates to 100 chars.
* Uses the full task as the body, and displays success/error messages.
*/
export function createIssueFromTask(task: string, options?: { labels?: string[] }): number | undefined {
info('Creating GitHub Issue...');
const title = extractTitle(task);
const effectiveLabels = options?.labels?.filter((l) => l.length > 0) ?? [];
const labels = effectiveLabels.length > 0 ? effectiveLabels : undefined;
const issueResult = getGitProvider().createIssue({ title, body: task, labels });
if (issueResult.success) {
if (!issueResult.url) {
error('Failed to extract issue number from URL');
return undefined;
}
success(`Issue created: ${issueResult.url}`);
const num = Number(issueResult.url.split('/').pop());
if (Number.isNaN(num)) {
error('Failed to extract issue number from URL');
return undefined;
}
return num;
} else {
error(`Failed to create issue: ${issueResult.error}`);
return undefined;
}
}