* takt: github-issue-421-feat-pr-opush * docs: CONTRIBUTING のレビューモード説明を復元 --pr オプション追加に伴い削除されていたブランチモード・現在の差分モードの ドキュメントを復元。コントリビューターはPR作成前にローカルでレビューする ケースもあるため、全モードの記載が必要。 * fix: --pr でリモートブランチを fetch してからチェックアウト 他人のPRブランチはローカルに存在しないため、git fetch origin を 実行してからチェックアウトするように修正。また baseBranch を返す ようにして --auto-pr 併用時の問題も解消。 * refactor: routing の排他条件を if/else に整理、不要なフォールバック削除 - routing.ts: prNumber の排他的分岐を if/else に統合 - pr.ts: data.body は string 型なので ?? '' フォールバックを削除
This commit is contained in:
nrs
GitHub
parent
e256db8dea
commit
7494149e75
@ -64,6 +64,7 @@ TAKT (TAKT Agent Koordination Topology) is a multi-agent orchestration system fo
|
||||
| `--pipeline` | Enable pipeline (non-interactive) mode — required for CI/automation |
|
||||
| `-t, --task <text>` | Task content (as alternative to GitHub issue) |
|
||||
| `-i, --issue <N>` | GitHub issue number (equivalent to `#N` in interactive mode) |
|
||||
| `--pr <number>` | PR number to fetch review comments and fix |
|
||||
| `-w, --piece <name or path>` | Piece name or path to piece YAML file |
|
||||
| `-b, --branch <name>` | Branch name (auto-generated if omitted) |
|
||||
| `--auto-pr` | Create PR after execution (interactive: skip confirmation, pipeline: enable PR) |
|
||||
|
||||
316
src/__tests__/cli-routing-pr-resolve.test.ts
Normal file
316
src/__tests__/cli-routing-pr-resolve.test.ts
Normal file
@ -0,0 +1,316 @@
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
vi.mock('../shared/ui/index.js', () => ({
|
||||
info: vi.fn(),
|
||||
error: vi.fn(),
|
||||
withProgress: vi.fn(async (_start, _done, operation) => operation()),
|
||||
}));
|
||||
|
||||
vi.mock('../shared/prompt/index.js', () => ({
|
||||
}));
|
||||
|
||||
vi.mock('../shared/utils/index.js', async (importOriginal) => ({
|
||||
...(await importOriginal<Record<string, unknown>>()),
|
||||
createLogger: () => ({
|
||||
info: vi.fn(),
|
||||
debug: vi.fn(),
|
||||
error: vi.fn(),
|
||||
}),
|
||||
}));
|
||||
|
||||
const { mockCheckCliStatus, mockFetchIssue, mockFetchPrReviewComments } = vi.hoisted(() => ({
|
||||
mockCheckCliStatus: vi.fn(),
|
||||
mockFetchIssue: vi.fn(),
|
||||
mockFetchPrReviewComments: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../infra/git/index.js', () => ({
|
||||
getGitProvider: () => ({
|
||||
checkCliStatus: (...args: unknown[]) => mockCheckCliStatus(...args),
|
||||
fetchIssue: (...args: unknown[]) => mockFetchIssue(...args),
|
||||
fetchPrReviewComments: (...args: unknown[]) => mockFetchPrReviewComments(...args),
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock('../infra/github/issue.js', () => ({
|
||||
parseIssueNumbers: vi.fn(() => []),
|
||||
formatIssueAsTask: vi.fn(),
|
||||
isIssueReference: vi.fn(),
|
||||
resolveIssueTask: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../features/tasks/index.js', () => ({
|
||||
selectAndExecuteTask: vi.fn(),
|
||||
determinePiece: vi.fn(),
|
||||
saveTaskFromInteractive: vi.fn(),
|
||||
createIssueAndSaveTask: vi.fn(),
|
||||
promptLabelSelection: vi.fn().mockResolvedValue([]),
|
||||
}));
|
||||
|
||||
vi.mock('../features/pipeline/index.js', () => ({
|
||||
executePipeline: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../features/interactive/index.js', () => ({
|
||||
interactiveMode: vi.fn(),
|
||||
selectInteractiveMode: vi.fn(() => 'assistant'),
|
||||
passthroughMode: vi.fn(),
|
||||
quietMode: vi.fn(),
|
||||
personaMode: vi.fn(),
|
||||
resolveLanguage: vi.fn(() => 'en'),
|
||||
selectRun: vi.fn(() => null),
|
||||
loadRunSessionContext: vi.fn(),
|
||||
listRecentRuns: vi.fn(() => []),
|
||||
normalizeTaskHistorySummary: vi.fn((items: unknown[]) => items),
|
||||
dispatchConversationAction: vi.fn(async (result: { action: string }, handlers: Record<string, (r: unknown) => unknown>) => {
|
||||
return handlers[result.action](result);
|
||||
}),
|
||||
}));
|
||||
|
||||
const mockListAllTaskItems = vi.fn();
|
||||
const mockIsStaleRunningTask = vi.fn();
|
||||
vi.mock('../infra/task/index.js', () => ({
|
||||
TaskRunner: vi.fn(() => ({
|
||||
listAllTaskItems: mockListAllTaskItems,
|
||||
})),
|
||||
isStaleRunningTask: (...args: unknown[]) => mockIsStaleRunningTask(...args),
|
||||
}));
|
||||
|
||||
vi.mock('../infra/config/index.js', () => ({
|
||||
getPieceDescription: vi.fn(() => ({ name: 'default', description: 'test piece', pieceStructure: '', movementPreviews: [] })),
|
||||
resolveConfigValue: vi.fn((_: string, key: string) => (key === 'piece' ? 'default' : false)),
|
||||
resolveConfigValues: vi.fn(() => ({ language: 'en', interactivePreviewMovements: 3, provider: 'claude' })),
|
||||
loadPersonaSessions: vi.fn(() => ({})),
|
||||
}));
|
||||
|
||||
vi.mock('../shared/constants.js', () => ({
|
||||
DEFAULT_PIECE_NAME: 'default',
|
||||
}));
|
||||
|
||||
const mockOpts: Record<string, unknown> = {};
|
||||
|
||||
vi.mock('../app/cli/program.js', () => {
|
||||
const chainable = {
|
||||
opts: vi.fn(() => mockOpts),
|
||||
argument: vi.fn().mockReturnThis(),
|
||||
action: vi.fn().mockReturnThis(),
|
||||
};
|
||||
return {
|
||||
program: chainable,
|
||||
resolvedCwd: '/test/cwd',
|
||||
pipelineMode: false,
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock('../app/cli/helpers.js', () => ({
|
||||
resolveAgentOverrides: vi.fn(),
|
||||
parseCreateWorktreeOption: vi.fn(),
|
||||
isDirectTask: vi.fn(() => false),
|
||||
}));
|
||||
|
||||
import { selectAndExecuteTask, determinePiece } from '../features/tasks/index.js';
|
||||
import { interactiveMode } from '../features/interactive/index.js';
|
||||
import { executePipeline } from '../features/pipeline/index.js';
|
||||
import { executeDefaultAction } from '../app/cli/routing.js';
|
||||
import { error as logError } from '../shared/ui/index.js';
|
||||
import type { PrReviewData } from '../infra/git/index.js';
|
||||
|
||||
const mockSelectAndExecuteTask = vi.mocked(selectAndExecuteTask);
|
||||
const mockDeterminePiece = vi.mocked(determinePiece);
|
||||
const mockInteractiveMode = vi.mocked(interactiveMode);
|
||||
const mockExecutePipeline = vi.mocked(executePipeline);
|
||||
const mockLogError = vi.mocked(logError);
|
||||
|
||||
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: 'fix/auth-bug',
|
||||
comments: [{ author: 'commenter1', body: 'Update tests' }],
|
||||
reviews: [{ author: 'reviewer1', body: 'Fix null check' }],
|
||||
files: ['src/auth.ts'],
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
for (const key of Object.keys(mockOpts)) {
|
||||
delete mockOpts[key];
|
||||
}
|
||||
mockDeterminePiece.mockResolvedValue('default');
|
||||
mockInteractiveMode.mockResolvedValue({ action: 'execute', task: 'summarized task' });
|
||||
mockListAllTaskItems.mockReturnValue([]);
|
||||
mockIsStaleRunningTask.mockReturnValue(false);
|
||||
});
|
||||
|
||||
describe('PR resolution in routing', () => {
|
||||
describe('--pr option', () => {
|
||||
it('should resolve PR review comments and pass to interactive mode', async () => {
|
||||
// Given
|
||||
mockOpts.pr = 456;
|
||||
const prReview = createMockPrReview();
|
||||
mockCheckCliStatus.mockReturnValue({ available: true });
|
||||
mockFetchPrReviewComments.mockReturnValue(prReview);
|
||||
|
||||
// When
|
||||
await executeDefaultAction();
|
||||
|
||||
// Then
|
||||
expect(mockFetchPrReviewComments).toHaveBeenCalledWith(456);
|
||||
expect(mockInteractiveMode).toHaveBeenCalledWith(
|
||||
'/test/cwd',
|
||||
expect.stringContaining('## PR #456 Review Comments:'),
|
||||
expect.anything(),
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
it('should set branch in selectOptions from PR headRefName', async () => {
|
||||
// Given
|
||||
mockOpts.pr = 456;
|
||||
const prReview = createMockPrReview({ headRefName: 'feat/my-pr-branch' });
|
||||
mockCheckCliStatus.mockReturnValue({ available: true });
|
||||
mockFetchPrReviewComments.mockReturnValue(prReview);
|
||||
|
||||
// When
|
||||
await executeDefaultAction();
|
||||
|
||||
// Then
|
||||
expect(mockSelectAndExecuteTask).toHaveBeenCalledWith(
|
||||
'/test/cwd',
|
||||
'summarized task',
|
||||
expect.objectContaining({
|
||||
branch: 'feat/my-pr-branch',
|
||||
}),
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
it('should exit with error when gh CLI is unavailable', async () => {
|
||||
// Given
|
||||
mockOpts.pr = 456;
|
||||
mockCheckCliStatus.mockReturnValue({
|
||||
available: false,
|
||||
error: 'gh CLI is not installed',
|
||||
});
|
||||
|
||||
const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => {
|
||||
throw new Error('process.exit called');
|
||||
});
|
||||
|
||||
// When/Then
|
||||
await expect(executeDefaultAction()).rejects.toThrow('process.exit called');
|
||||
expect(mockExit).toHaveBeenCalledWith(1);
|
||||
expect(mockInteractiveMode).not.toHaveBeenCalled();
|
||||
|
||||
mockExit.mockRestore();
|
||||
});
|
||||
|
||||
it('should exit with error when PR has no review comments', async () => {
|
||||
// Given
|
||||
mockOpts.pr = 456;
|
||||
const emptyPrReview = createMockPrReview({ reviews: [], comments: [] });
|
||||
mockCheckCliStatus.mockReturnValue({ available: true });
|
||||
mockFetchPrReviewComments.mockReturnValue(emptyPrReview);
|
||||
|
||||
const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => {
|
||||
throw new Error('process.exit called');
|
||||
});
|
||||
|
||||
// When/Then
|
||||
await expect(executeDefaultAction()).rejects.toThrow('process.exit called');
|
||||
expect(mockExit).toHaveBeenCalledWith(1);
|
||||
expect(mockInteractiveMode).not.toHaveBeenCalled();
|
||||
|
||||
mockExit.mockRestore();
|
||||
});
|
||||
|
||||
it('should not resolve issues when --pr is specified', async () => {
|
||||
// Given
|
||||
mockOpts.pr = 456;
|
||||
const prReview = createMockPrReview();
|
||||
mockCheckCliStatus.mockReturnValue({ available: true });
|
||||
mockFetchPrReviewComments.mockReturnValue(prReview);
|
||||
|
||||
// When
|
||||
await executeDefaultAction();
|
||||
|
||||
// Then
|
||||
expect(mockFetchIssue).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('--pr and --issue mutual exclusion', () => {
|
||||
it('should exit with error when both --pr and --issue are specified', async () => {
|
||||
// Given
|
||||
mockOpts.pr = 456;
|
||||
mockOpts.issue = 123;
|
||||
|
||||
const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => {
|
||||
throw new Error('process.exit called');
|
||||
});
|
||||
|
||||
// When/Then
|
||||
await expect(executeDefaultAction()).rejects.toThrow('process.exit called');
|
||||
expect(mockLogError).toHaveBeenCalledWith('--pr and --issue cannot be used together');
|
||||
expect(mockExit).toHaveBeenCalledWith(1);
|
||||
|
||||
mockExit.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe('--pr and --task mutual exclusion', () => {
|
||||
it('should exit with error when both --pr and --task are specified', async () => {
|
||||
// Given
|
||||
mockOpts.pr = 456;
|
||||
mockOpts.task = 'some task';
|
||||
|
||||
const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => {
|
||||
throw new Error('process.exit called');
|
||||
});
|
||||
|
||||
// When/Then
|
||||
await expect(executeDefaultAction()).rejects.toThrow('process.exit called');
|
||||
expect(mockLogError).toHaveBeenCalledWith('--pr and --task cannot be used together');
|
||||
expect(mockExit).toHaveBeenCalledWith(1);
|
||||
|
||||
mockExit.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe('--pr in pipeline mode', () => {
|
||||
it('should pass prNumber to executePipeline', async () => {
|
||||
// Given: override pipelineMode
|
||||
const programModule = await import('../app/cli/program.js');
|
||||
const originalPipelineMode = programModule.pipelineMode;
|
||||
Object.defineProperty(programModule, 'pipelineMode', { value: true, writable: true });
|
||||
|
||||
mockOpts.pr = 456;
|
||||
mockExecutePipeline.mockResolvedValue(0);
|
||||
|
||||
// When
|
||||
await executeDefaultAction();
|
||||
|
||||
// Then
|
||||
expect(mockExecutePipeline).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
prNumber: 456,
|
||||
}),
|
||||
);
|
||||
|
||||
// Cleanup
|
||||
Object.defineProperty(programModule, 'pipelineMode', { value: originalPipelineMode, writable: true });
|
||||
});
|
||||
});
|
||||
});
|
||||
@ -235,4 +235,39 @@ describe('confirmAndCreateWorktree', () => {
|
||||
expect(mockConfirm).not.toHaveBeenCalled();
|
||||
expect(result.isWorktree).toBe(true);
|
||||
});
|
||||
|
||||
it('should pass branchOverride to createSharedClone', async () => {
|
||||
// Given: branchOverride provided (e.g., PR head branch)
|
||||
mockSummarizeTaskName.mockResolvedValue('fix-auth');
|
||||
mockCreateSharedClone.mockReturnValue({
|
||||
path: '/project/../20260128T0504-fix-auth',
|
||||
branch: 'fix/pr-branch',
|
||||
});
|
||||
|
||||
// When
|
||||
await confirmAndCreateWorktree('/project', 'fix auth', true, 'fix/pr-branch');
|
||||
|
||||
// Then
|
||||
expect(mockCreateSharedClone).toHaveBeenCalledWith('/project', expect.objectContaining({
|
||||
branch: 'fix/pr-branch',
|
||||
}));
|
||||
});
|
||||
|
||||
it('should not pass branch to createSharedClone when branchOverride is omitted', async () => {
|
||||
// Given: no branchOverride
|
||||
mockSummarizeTaskName.mockResolvedValue('fix-auth');
|
||||
mockCreateSharedClone.mockReturnValue({
|
||||
path: '/project/../20260128T0504-fix-auth',
|
||||
branch: 'takt/20260128T0504-fix-auth',
|
||||
});
|
||||
|
||||
// When
|
||||
await confirmAndCreateWorktree('/project', 'fix auth', true);
|
||||
|
||||
// Then
|
||||
expect(mockCreateSharedClone).toHaveBeenCalledWith('/project', {
|
||||
worktree: true,
|
||||
taskSlug: 'fix-auth',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@ -26,8 +26,9 @@ vi.mock('../shared/utils/index.js', async (importOriginal) => ({
|
||||
getErrorMessage: (e: unknown) => String(e),
|
||||
}));
|
||||
|
||||
import { buildPrBody, findExistingPr, createPullRequest } from '../infra/github/pr.js';
|
||||
import { buildPrBody, findExistingPr, createPullRequest, fetchPrReviewComments, formatPrReviewAsTask } from '../infra/github/pr.js';
|
||||
import type { GitHubIssue } from '../infra/github/types.js';
|
||||
import type { PrReviewData } from '../infra/git/types.js';
|
||||
|
||||
describe('findExistingPr', () => {
|
||||
beforeEach(() => {
|
||||
@ -176,3 +177,194 @@ describe('buildPrBody', () => {
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
describe('fetchPrReviewComments', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it('should parse gh pr view JSON and return PrReviewData', () => {
|
||||
// Given
|
||||
const ghResponse = {
|
||||
number: 456,
|
||||
title: 'Fix auth bug',
|
||||
body: 'PR description',
|
||||
url: 'https://github.com/org/repo/pull/456',
|
||||
headRefName: 'fix/auth-bug',
|
||||
comments: [
|
||||
{ author: { login: 'commenter1' }, body: 'Please update tests' },
|
||||
],
|
||||
reviews: [
|
||||
{
|
||||
author: { login: 'reviewer1' },
|
||||
body: 'Looks mostly good',
|
||||
comments: [
|
||||
{ body: 'Fix null check here', path: 'src/auth.ts', line: 42, author: { login: 'reviewer1' } },
|
||||
],
|
||||
},
|
||||
{
|
||||
author: { login: 'reviewer2' },
|
||||
body: '',
|
||||
comments: [],
|
||||
},
|
||||
],
|
||||
files: [
|
||||
{ path: 'src/auth.ts' },
|
||||
{ path: 'src/auth.test.ts' },
|
||||
],
|
||||
};
|
||||
mockExecFileSync.mockReturnValue(JSON.stringify(ghResponse));
|
||||
|
||||
// When
|
||||
const result = fetchPrReviewComments(456);
|
||||
|
||||
// Then
|
||||
expect(mockExecFileSync).toHaveBeenCalledWith(
|
||||
'gh',
|
||||
['pr', 'view', '456', '--json', 'number,title,body,url,headRefName,comments,reviews,files'],
|
||||
expect.objectContaining({ encoding: 'utf-8' }),
|
||||
);
|
||||
expect(result.number).toBe(456);
|
||||
expect(result.title).toBe('Fix auth bug');
|
||||
expect(result.headRefName).toBe('fix/auth-bug');
|
||||
expect(result.comments).toEqual([{ author: 'commenter1', body: 'Please update tests' }]);
|
||||
expect(result.reviews).toEqual([
|
||||
{ author: 'reviewer1', body: 'Looks mostly good' },
|
||||
{ author: 'reviewer1', body: 'Fix null check here', path: 'src/auth.ts', line: 42 },
|
||||
]);
|
||||
expect(result.files).toEqual(['src/auth.ts', 'src/auth.test.ts']);
|
||||
});
|
||||
|
||||
it('should skip reviews with empty body', () => {
|
||||
// Given
|
||||
const ghResponse = {
|
||||
number: 10,
|
||||
title: 'Approved PR',
|
||||
body: '',
|
||||
url: 'https://github.com/org/repo/pull/10',
|
||||
headRefName: 'feat/approved',
|
||||
comments: [],
|
||||
reviews: [
|
||||
{ author: { login: 'approver' }, body: '', comments: [] },
|
||||
],
|
||||
files: [],
|
||||
};
|
||||
mockExecFileSync.mockReturnValue(JSON.stringify(ghResponse));
|
||||
|
||||
// When
|
||||
const result = fetchPrReviewComments(10);
|
||||
|
||||
// Then
|
||||
expect(result.reviews).toEqual([]);
|
||||
});
|
||||
|
||||
it('should throw when gh CLI fails', () => {
|
||||
// Given
|
||||
mockExecFileSync.mockImplementation(() => { throw new Error('gh: PR not found'); });
|
||||
|
||||
// When/Then
|
||||
expect(() => fetchPrReviewComments(999)).toThrow('gh: PR not found');
|
||||
});
|
||||
});
|
||||
|
||||
describe('formatPrReviewAsTask', () => {
|
||||
it('should format PR review data with all sections', () => {
|
||||
// Given
|
||||
const prReview: PrReviewData = {
|
||||
number: 456,
|
||||
title: 'Fix auth bug',
|
||||
body: 'PR description text',
|
||||
url: 'https://github.com/org/repo/pull/456',
|
||||
headRefName: 'fix/auth-bug',
|
||||
comments: [
|
||||
{ author: 'commenter1', body: 'Can you also update the tests?' },
|
||||
],
|
||||
reviews: [
|
||||
{ author: 'reviewer1', body: 'Fix the null check in auth.ts', path: 'src/auth.ts', line: 42 },
|
||||
{ author: 'reviewer2', body: 'This function should handle edge cases' },
|
||||
],
|
||||
files: ['src/auth.ts', 'src/auth.test.ts'],
|
||||
};
|
||||
|
||||
// When
|
||||
const result = formatPrReviewAsTask(prReview);
|
||||
|
||||
// Then
|
||||
expect(result).toContain('## PR #456 Review Comments: Fix auth bug');
|
||||
expect(result).toContain('### PR Description');
|
||||
expect(result).toContain('PR description text');
|
||||
expect(result).toContain('### Review Comments');
|
||||
expect(result).toContain('**reviewer1**: Fix the null check in auth.ts');
|
||||
expect(result).toContain('File: src/auth.ts, Line: 42');
|
||||
expect(result).toContain('**reviewer2**: This function should handle edge cases');
|
||||
expect(result).toContain('### Conversation Comments');
|
||||
expect(result).toContain('**commenter1**: Can you also update the tests?');
|
||||
expect(result).toContain('### Changed Files');
|
||||
expect(result).toContain('- src/auth.ts');
|
||||
expect(result).toContain('- src/auth.test.ts');
|
||||
});
|
||||
|
||||
it('should omit PR Description when body is empty', () => {
|
||||
// Given
|
||||
const prReview: PrReviewData = {
|
||||
number: 10,
|
||||
title: 'Quick fix',
|
||||
body: '',
|
||||
url: 'https://github.com/org/repo/pull/10',
|
||||
headRefName: 'fix/quick',
|
||||
comments: [],
|
||||
reviews: [{ author: 'reviewer', body: 'Fix this' }],
|
||||
files: [],
|
||||
};
|
||||
|
||||
// When
|
||||
const result = formatPrReviewAsTask(prReview);
|
||||
|
||||
// Then
|
||||
expect(result).not.toContain('### PR Description');
|
||||
expect(result).toContain('### Review Comments');
|
||||
});
|
||||
|
||||
it('should omit empty sections', () => {
|
||||
// Given
|
||||
const prReview: PrReviewData = {
|
||||
number: 20,
|
||||
title: 'Empty review',
|
||||
body: '',
|
||||
url: 'https://github.com/org/repo/pull/20',
|
||||
headRefName: 'feat/empty',
|
||||
comments: [],
|
||||
reviews: [{ author: 'reviewer', body: 'Add tests' }],
|
||||
files: [],
|
||||
};
|
||||
|
||||
// When
|
||||
const result = formatPrReviewAsTask(prReview);
|
||||
|
||||
// Then
|
||||
expect(result).not.toContain('### Conversation Comments');
|
||||
expect(result).not.toContain('### Changed Files');
|
||||
expect(result).toContain('### Review Comments');
|
||||
});
|
||||
|
||||
it('should format inline comment with path but no line', () => {
|
||||
// Given
|
||||
const prReview: PrReviewData = {
|
||||
number: 30,
|
||||
title: 'Path only',
|
||||
body: '',
|
||||
url: 'https://github.com/org/repo/pull/30',
|
||||
headRefName: 'feat/path-only',
|
||||
comments: [],
|
||||
reviews: [{ author: 'reviewer', body: 'Fix this', path: 'src/index.ts' }],
|
||||
files: [],
|
||||
};
|
||||
|
||||
// When
|
||||
const result = formatPrReviewAsTask(prReview);
|
||||
|
||||
// Then
|
||||
expect(result).toContain('File: src/index.ts');
|
||||
expect(result).not.toContain('Line:');
|
||||
});
|
||||
});
|
||||
|
||||
@ -15,6 +15,7 @@ const {
|
||||
mockFindExistingPr,
|
||||
mockCommentOnPr,
|
||||
mockCreatePullRequest,
|
||||
mockFetchPrReviewComments,
|
||||
} = vi.hoisted(() => ({
|
||||
mockCheckGhCli: vi.fn(),
|
||||
mockFetchIssue: vi.fn(),
|
||||
@ -22,6 +23,7 @@ const {
|
||||
mockFindExistingPr: vi.fn(),
|
||||
mockCommentOnPr: vi.fn(),
|
||||
mockCreatePullRequest: vi.fn(),
|
||||
mockFetchPrReviewComments: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../infra/github/issue.js', () => ({
|
||||
@ -34,11 +36,12 @@ vi.mock('../infra/github/pr.js', () => ({
|
||||
findExistingPr: (...args: unknown[]) => mockFindExistingPr(...args),
|
||||
commentOnPr: (...args: unknown[]) => mockCommentOnPr(...args),
|
||||
createPullRequest: (...args: unknown[]) => mockCreatePullRequest(...args),
|
||||
fetchPrReviewComments: (...args: unknown[]) => mockFetchPrReviewComments(...args),
|
||||
}));
|
||||
|
||||
import { GitHubProvider } from '../infra/github/GitHubProvider.js';
|
||||
import { getGitProvider } from '../infra/git/index.js';
|
||||
import type { CommentResult } from '../infra/git/index.js';
|
||||
import type { CommentResult, PrReviewData } from '../infra/git/index.js';
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
@ -206,6 +209,31 @@ describe('GitHubProvider', () => {
|
||||
expect(result.error).toBe('Permission denied');
|
||||
});
|
||||
});
|
||||
|
||||
describe('fetchPrReviewComments', () => {
|
||||
it('fetchPrReviewComments(n) に委譲し結果を返す', () => {
|
||||
// Given
|
||||
const prReview: PrReviewData = {
|
||||
number: 456,
|
||||
title: 'Fix bug',
|
||||
body: 'Description',
|
||||
url: 'https://github.com/org/repo/pull/456',
|
||||
headRefName: 'fix/bug',
|
||||
comments: [],
|
||||
reviews: [{ author: 'reviewer', body: 'Fix this' }],
|
||||
files: ['src/index.ts'],
|
||||
};
|
||||
mockFetchPrReviewComments.mockReturnValue(prReview);
|
||||
const provider = new GitHubProvider();
|
||||
|
||||
// When
|
||||
const result = provider.fetchPrReviewComments(456);
|
||||
|
||||
// Then
|
||||
expect(mockFetchPrReviewComments).toHaveBeenCalledWith(456);
|
||||
expect(result).toBe(prReview);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('getGitProvider', () => {
|
||||
@ -217,6 +245,7 @@ describe('getGitProvider', () => {
|
||||
expect(typeof provider.checkCliStatus).toBe('function');
|
||||
expect(typeof provider.fetchIssue).toBe('function');
|
||||
expect(typeof provider.createIssue).toBe('function');
|
||||
expect(typeof provider.fetchPrReviewComments).toBe('function');
|
||||
expect(typeof provider.findExistingPr).toBe('function');
|
||||
expect(typeof provider.createPullRequest).toBe('function');
|
||||
expect(typeof provider.commentOnPr).toBe('function');
|
||||
|
||||
@ -20,9 +20,15 @@ vi.mock('../infra/github/issue.js', () => ({
|
||||
const mockCreatePullRequest = vi.fn();
|
||||
const mockPushBranch = vi.fn();
|
||||
const mockBuildPrBody = vi.fn(() => 'Default PR body');
|
||||
const mockFetchPrReviewComments = vi.fn();
|
||||
const mockFormatPrReviewAsTask = vi.fn((pr: { number: number; title: string }) =>
|
||||
`## PR #${pr.number} Review Comments: ${pr.title}`
|
||||
);
|
||||
vi.mock('../infra/github/pr.js', () => ({
|
||||
createPullRequest: mockCreatePullRequest,
|
||||
buildPrBody: mockBuildPrBody,
|
||||
fetchPrReviewComments: (...args: unknown[]) => mockFetchPrReviewComments(...args),
|
||||
formatPrReviewAsTask: (...args: unknown[]) => mockFormatPrReviewAsTask(...args),
|
||||
}));
|
||||
|
||||
vi.mock('../infra/task/git.js', async (importOriginal) => ({
|
||||
@ -528,7 +534,7 @@ describe('executePipeline', () => {
|
||||
});
|
||||
|
||||
expect(exitCode).toBe(0);
|
||||
expect(mockConfirmAndCreateWorktree).toHaveBeenCalledWith('/tmp/test', 'Fix the bug', true);
|
||||
expect(mockConfirmAndCreateWorktree).toHaveBeenCalledWith('/tmp/test', 'Fix the bug', true, undefined);
|
||||
expect(mockExecuteTask).toHaveBeenCalledWith({
|
||||
task: 'Fix the bug',
|
||||
cwd: '/tmp/test-worktree',
|
||||
@ -812,4 +818,119 @@ describe('executePipeline', () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('--pr pipeline', () => {
|
||||
it('should resolve PR review comments and execute pipeline with PR branch checkout', async () => {
|
||||
mockFetchPrReviewComments.mockReturnValueOnce({
|
||||
number: 456,
|
||||
title: 'Fix auth bug',
|
||||
body: 'PR description',
|
||||
url: 'https://github.com/org/repo/pull/456',
|
||||
headRefName: 'fix/auth-bug',
|
||||
comments: [{ author: 'commenter1', body: 'Update tests' }],
|
||||
reviews: [{ author: 'reviewer1', body: 'Fix null check' }],
|
||||
files: ['src/auth.ts'],
|
||||
});
|
||||
mockExecuteTask.mockResolvedValueOnce(true);
|
||||
|
||||
const exitCode = await executePipeline({
|
||||
prNumber: 456,
|
||||
piece: 'default',
|
||||
autoPr: false,
|
||||
cwd: '/tmp/test',
|
||||
});
|
||||
|
||||
expect(exitCode).toBe(0);
|
||||
expect(mockFetchPrReviewComments).toHaveBeenCalledWith(456);
|
||||
expect(mockFormatPrReviewAsTask).toHaveBeenCalled();
|
||||
// PR branch checkout
|
||||
const checkoutCall = mockExecFileSync.mock.calls.find(
|
||||
(call: unknown[]) => call[0] === 'git' && (call[1] as string[])[0] === 'checkout' && (call[1] as string[])[1] === 'fix/auth-bug',
|
||||
);
|
||||
expect(checkoutCall).toBeDefined();
|
||||
});
|
||||
|
||||
it('should return exit code 2 when gh CLI is unavailable for --pr', async () => {
|
||||
mockCheckGhCli.mockReturnValueOnce({ available: false, error: 'gh not found' });
|
||||
|
||||
const exitCode = await executePipeline({
|
||||
prNumber: 456,
|
||||
piece: 'default',
|
||||
autoPr: false,
|
||||
cwd: '/tmp/test',
|
||||
});
|
||||
|
||||
expect(exitCode).toBe(2);
|
||||
});
|
||||
|
||||
it('should return exit code 2 when PR has no review comments', async () => {
|
||||
mockFetchPrReviewComments.mockReturnValueOnce({
|
||||
number: 456,
|
||||
title: 'Fix auth bug',
|
||||
body: 'PR description',
|
||||
url: 'https://github.com/org/repo/pull/456',
|
||||
headRefName: 'fix/auth-bug',
|
||||
comments: [],
|
||||
reviews: [],
|
||||
files: ['src/auth.ts'],
|
||||
});
|
||||
|
||||
const exitCode = await executePipeline({
|
||||
prNumber: 456,
|
||||
piece: 'default',
|
||||
autoPr: false,
|
||||
cwd: '/tmp/test',
|
||||
});
|
||||
|
||||
expect(exitCode).toBe(2);
|
||||
});
|
||||
|
||||
it('should return exit code 2 when PR fetch fails', async () => {
|
||||
mockFetchPrReviewComments.mockImplementationOnce(() => {
|
||||
throw new Error('PR not found');
|
||||
});
|
||||
|
||||
const exitCode = await executePipeline({
|
||||
prNumber: 999,
|
||||
piece: 'default',
|
||||
autoPr: false,
|
||||
cwd: '/tmp/test',
|
||||
});
|
||||
|
||||
expect(exitCode).toBe(2);
|
||||
});
|
||||
|
||||
it('should checkout PR branch instead of creating new branch', async () => {
|
||||
mockFetchPrReviewComments.mockReturnValueOnce({
|
||||
number: 456,
|
||||
title: 'Fix auth bug',
|
||||
body: 'PR description',
|
||||
url: 'https://github.com/org/repo/pull/456',
|
||||
headRefName: 'fix/auth-bug',
|
||||
comments: [{ author: 'reviewer1', body: 'Fix this' }],
|
||||
reviews: [],
|
||||
files: ['src/auth.ts'],
|
||||
});
|
||||
mockExecuteTask.mockResolvedValueOnce(true);
|
||||
|
||||
const exitCode = await executePipeline({
|
||||
prNumber: 456,
|
||||
piece: 'default',
|
||||
autoPr: false,
|
||||
cwd: '/tmp/test',
|
||||
});
|
||||
|
||||
expect(exitCode).toBe(0);
|
||||
// Should checkout existing branch, not create new
|
||||
const checkoutNewBranch = mockExecFileSync.mock.calls.find(
|
||||
(call: unknown[]) => call[0] === 'git' && (call[1] as string[])[0] === 'checkout' && (call[1] as string[])[1] === '-b',
|
||||
);
|
||||
expect(checkoutNewBranch).toBeUndefined();
|
||||
// Should checkout existing PR branch
|
||||
const checkoutPrBranch = mockExecFileSync.mock.calls.find(
|
||||
(call: unknown[]) => call[0] === 'git' && (call[1] as string[])[0] === 'checkout' && (call[1] as string[])[1] === 'fix/auth-bug',
|
||||
);
|
||||
expect(checkoutPrBranch).toBeDefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@ -41,6 +41,7 @@ program
|
||||
// --- Global options ---
|
||||
program
|
||||
.option('-i, --issue <number>', 'GitHub issue number (equivalent to #N)', (val: string) => parseInt(val, 10))
|
||||
.option('--pr <number>', 'PR number to fetch review comments and fix', (val: string) => parseInt(val, 10))
|
||||
.option('-w, --piece <name>', 'Piece name or path to piece file')
|
||||
.option('-b, --branch <name>', 'Branch name (auto-generated if omitted)')
|
||||
.option('--auto-pr', 'Create PR after successful execution')
|
||||
|
||||
@ -8,9 +8,9 @@
|
||||
import { info, 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 } from '../../infra/github/index.js';
|
||||
import { formatIssueAsTask, parseIssueNumbers, formatPrReviewAsTask } from '../../infra/github/index.js';
|
||||
import { getGitProvider } from '../../infra/git/index.js';
|
||||
import type { Issue } from '../../infra/git/index.js';
|
||||
import type { Issue, 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 {
|
||||
@ -76,12 +76,56 @@ async function resolveIssueInput(
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
* Throws on gh CLI unavailability or fetch failure.
|
||||
*/
|
||||
async function resolvePrInput(
|
||||
prNumber: number,
|
||||
): Promise<{ initialInput: string; prBranch: string }> {
|
||||
const ghStatus = getGitProvider().checkCliStatus();
|
||||
if (!ghStatus.available) {
|
||||
throw new Error(ghStatus.error);
|
||||
}
|
||||
|
||||
const prReview = await withProgress(
|
||||
'Fetching PR review comments...',
|
||||
(pr: PrReviewData) => `PR fetched: #${pr.number} ${pr.title}`,
|
||||
async () => getGitProvider().fetchPrReviewComments(prNumber),
|
||||
);
|
||||
|
||||
if (prReview.reviews.length === 0 && prReview.comments.length === 0) {
|
||||
throw new Error(`PR #${prNumber} has no review comments`);
|
||||
}
|
||||
|
||||
return {
|
||||
initialInput: formatPrReviewAsTask(prReview),
|
||||
prBranch: prReview.headRefName,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Execute default action: handle task execution, pipeline mode, or interactive mode.
|
||||
* Exported for use in slash-command fallback logic.
|
||||
*/
|
||||
export async function executeDefaultAction(task?: string): Promise<void> {
|
||||
const opts = program.opts();
|
||||
const prNumber = opts.pr as number | undefined;
|
||||
const issueNumber = opts.issue as number | undefined;
|
||||
|
||||
if (prNumber && issueNumber) {
|
||||
logError('--pr and --issue cannot be used together');
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
if (prNumber && (opts.task as string | undefined)) {
|
||||
logError('--pr and --task cannot be used together');
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const agentOverrides = resolveAgentOverrides(program);
|
||||
const createWorktreeOverride = parseCreateWorktreeOption(opts.createWorktree as string | undefined);
|
||||
const resolvedPipelinePiece = (opts.piece as string | undefined) ?? resolveConfigValue(resolvedCwd, 'piece');
|
||||
@ -102,7 +146,8 @@ export async function executeDefaultAction(task?: string): Promise<void> {
|
||||
// --- Pipeline mode (non-interactive): triggered by --pipeline ---
|
||||
if (pipelineMode) {
|
||||
const exitCode = await executePipeline({
|
||||
issueNumber: opts.issue as number | undefined,
|
||||
issueNumber,
|
||||
prNumber,
|
||||
task: opts.task as string | undefined,
|
||||
piece: resolvedPipelinePiece,
|
||||
branch: opts.branch as string | undefined,
|
||||
@ -132,11 +177,22 @@ export async function executeDefaultAction(task?: string): Promise<void> {
|
||||
return;
|
||||
}
|
||||
|
||||
// Resolve issue references (--issue N or #N positional arg) before interactive mode
|
||||
// Resolve PR review comments (--pr N) before interactive mode
|
||||
let initialInput: string | undefined = task;
|
||||
|
||||
if (prNumber) {
|
||||
try {
|
||||
const issueResult = await resolveIssueInput(opts.issue as number | undefined, task);
|
||||
const prResult = await resolvePrInput(prNumber);
|
||||
initialInput = prResult.initialInput;
|
||||
selectOptions.branch = prResult.prBranch;
|
||||
} catch (e) {
|
||||
logError(getErrorMessage(e));
|
||||
process.exit(1);
|
||||
}
|
||||
} else {
|
||||
// Resolve issue references (--issue N or #N positional arg) before interactive mode
|
||||
try {
|
||||
const issueResult = await resolveIssueInput(issueNumber, task);
|
||||
if (issueResult) {
|
||||
selectOptions.issues = issueResult.issues;
|
||||
initialInput = issueResult.initialInput;
|
||||
@ -145,6 +201,7 @@ export async function executeDefaultAction(task?: string): Promise<void> {
|
||||
logError(getErrorMessage(e));
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
// All paths below go through interactive mode
|
||||
const globalConfig = resolveConfigValues(resolvedCwd, ['language', 'interactivePreviewMovements', 'provider']);
|
||||
|
||||
@ -59,7 +59,7 @@ async function runPipeline(options: PipelineExecutionOptions): Promise<PipelineO
|
||||
// Step 2: Prepare execution environment
|
||||
let context: ExecutionContext;
|
||||
try {
|
||||
context = await resolveExecutionContext(cwd, taskContent.task, options, pipelineConfig);
|
||||
context = await resolveExecutionContext(cwd, taskContent.task, options, pipelineConfig, taskContent.prBranch);
|
||||
} catch (err) {
|
||||
error(`Failed to prepare execution environment: ${getErrorMessage(err)}`);
|
||||
return { exitCode: EXIT_GIT_OPERATION_FAILED, result: buildResult() };
|
||||
|
||||
@ -6,7 +6,7 @@
|
||||
*/
|
||||
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import { formatIssueAsTask, buildPrBody } from '../../infra/github/index.js';
|
||||
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 { executeTask, confirmAndCreateWorktree, type TaskExecutionOptions, type PipelineExecutionOptions } from '../tasks/index.js';
|
||||
@ -19,6 +19,8 @@ import type { PipelineConfig } from '../../core/models/index.js';
|
||||
export interface TaskContent {
|
||||
task: string;
|
||||
issue?: Issue;
|
||||
/** PR head branch name (set when using --pr) */
|
||||
prBranch?: string;
|
||||
}
|
||||
|
||||
export interface ExecutionContext {
|
||||
@ -78,9 +80,11 @@ function buildPipelinePrBody(
|
||||
|
||||
// ---- Step 1: Resolve task content ----
|
||||
|
||||
export function resolveTaskContent(options: PipelineExecutionOptions): TaskContent | undefined {
|
||||
if (options.issueNumber) {
|
||||
info(`Fetching issue #${options.issueNumber}...`);
|
||||
/** Fetch a GitHub resource with CLI availability check and error handling. */
|
||||
function fetchGitHubResource<T>(
|
||||
label: string,
|
||||
fetch: (provider: ReturnType<typeof getGitProvider>) => T,
|
||||
): T | undefined {
|
||||
const gitProvider = getGitProvider();
|
||||
const cliStatus = gitProvider.checkCliStatus();
|
||||
if (!cliStatus.available) {
|
||||
@ -88,19 +92,44 @@ export function resolveTaskContent(options: PipelineExecutionOptions): TaskConte
|
||||
return undefined;
|
||||
}
|
||||
try {
|
||||
const issue = gitProvider.fetchIssue(options.issueNumber);
|
||||
return fetch(gitProvider);
|
||||
} catch (err) {
|
||||
error(`Failed to fetch ${label}: ${getErrorMessage(err)}`);
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
export function resolveTaskContent(options: PipelineExecutionOptions): TaskContent | undefined {
|
||||
if (options.prNumber) {
|
||||
info(`Fetching PR #${options.prNumber} review comments...`);
|
||||
const prReview = fetchGitHubResource(
|
||||
`PR #${options.prNumber}`,
|
||||
(provider) => provider.fetchPrReviewComments(options.prNumber!),
|
||||
);
|
||||
if (!prReview) return undefined;
|
||||
if (prReview.reviews.length === 0 && prReview.comments.length === 0) {
|
||||
error(`PR #${options.prNumber} has no review comments`);
|
||||
return undefined;
|
||||
}
|
||||
const task = formatPrReviewAsTask(prReview);
|
||||
success(`PR #${options.prNumber} fetched: "${prReview.title}"`);
|
||||
return { task, prBranch: prReview.headRefName };
|
||||
}
|
||||
if (options.issueNumber) {
|
||||
info(`Fetching issue #${options.issueNumber}...`);
|
||||
const issue = fetchGitHubResource(
|
||||
`issue #${options.issueNumber}`,
|
||||
(provider) => provider.fetchIssue(options.issueNumber!),
|
||||
);
|
||||
if (!issue) return undefined;
|
||||
const task = formatIssueAsTask(issue);
|
||||
success(`Issue #${options.issueNumber} fetched: "${issue.title}"`);
|
||||
return { task, issue };
|
||||
} catch (err) {
|
||||
error(`Failed to fetch issue #${options.issueNumber}: ${getErrorMessage(err)}`);
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
if (options.task) {
|
||||
return { task: options.task };
|
||||
}
|
||||
error('Either --issue or --task must be specified');
|
||||
error('Either --issue, --pr, or --task must be specified');
|
||||
return undefined;
|
||||
}
|
||||
|
||||
@ -111,9 +140,10 @@ export async function resolveExecutionContext(
|
||||
task: string,
|
||||
options: Pick<PipelineExecutionOptions, 'createWorktree' | 'skipGit' | 'branch' | 'issueNumber'>,
|
||||
pipelineConfig: PipelineConfig | undefined,
|
||||
prBranch?: string,
|
||||
): Promise<ExecutionContext> {
|
||||
if (options.createWorktree) {
|
||||
const result = await confirmAndCreateWorktree(cwd, task, options.createWorktree);
|
||||
const result = await confirmAndCreateWorktree(cwd, task, options.createWorktree, prBranch);
|
||||
if (result.isWorktree) {
|
||||
success(`Worktree created: ${result.execCwd}`);
|
||||
}
|
||||
@ -122,6 +152,13 @@ export async function resolveExecutionContext(
|
||||
if (options.skipGit) {
|
||||
return { execCwd: cwd, isWorktree: false };
|
||||
}
|
||||
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' });
|
||||
success(`Checked out PR branch: ${prBranch}`);
|
||||
return { execCwd: cwd, branch: prBranch, baseBranch: resolveBaseBranch(cwd).branch, isWorktree: false };
|
||||
}
|
||||
const resolved = resolveBaseBranch(cwd);
|
||||
const branch = options.branch ?? generatePipelineBranchName(pipelineConfig, options.issueNumber);
|
||||
info(`Creating branch: ${branch}`);
|
||||
|
||||
@ -43,6 +43,7 @@ export async function confirmAndCreateWorktree(
|
||||
cwd: string,
|
||||
task: string,
|
||||
createWorktreeOverride?: boolean | undefined,
|
||||
branchOverride?: string,
|
||||
): Promise<WorktreeConfirmationResult> {
|
||||
const useWorktree =
|
||||
typeof createWorktreeOverride === 'boolean'
|
||||
@ -67,6 +68,7 @@ export async function confirmAndCreateWorktree(
|
||||
async () => createSharedClone(cwd, {
|
||||
worktree: true,
|
||||
taskSlug,
|
||||
...(branchOverride ? { branch: branchOverride } : {}),
|
||||
}),
|
||||
);
|
||||
|
||||
@ -94,6 +96,7 @@ export async function selectAndExecuteTask(
|
||||
cwd,
|
||||
task,
|
||||
options?.createWorktree,
|
||||
options?.branch,
|
||||
);
|
||||
|
||||
// Ask for PR creation BEFORE execution (only if worktree is enabled)
|
||||
|
||||
@ -103,6 +103,8 @@ export interface ExecuteTaskOptions {
|
||||
export interface PipelineExecutionOptions {
|
||||
/** GitHub issue number */
|
||||
issueNumber?: number;
|
||||
/** PR number to fetch review comments */
|
||||
prNumber?: number;
|
||||
/** Task content (alternative to issue) */
|
||||
task?: string;
|
||||
/** Piece name or path to piece file */
|
||||
@ -139,6 +141,8 @@ export interface SelectAndExecuteOptions {
|
||||
repo?: string;
|
||||
piece?: string;
|
||||
createWorktree?: boolean | undefined;
|
||||
/** 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 */
|
||||
|
||||
@ -7,7 +7,7 @@
|
||||
import { GitHubProvider } from '../github/GitHubProvider.js';
|
||||
import type { GitProvider } from './types.js';
|
||||
|
||||
export type { GitProvider, Issue, CliStatus, ExistingPr, CreatePrOptions, CreatePrResult, CommentResult, CreateIssueOptions, CreateIssueResult } from './types.js';
|
||||
export type { GitProvider, Issue, CliStatus, ExistingPr, CreatePrOptions, CreatePrResult, CommentResult, CreateIssueOptions, CreateIssueResult, PrReviewComment, PrReviewData } from './types.js';
|
||||
|
||||
let provider: GitProvider | undefined;
|
||||
|
||||
|
||||
@ -69,6 +69,32 @@ export interface CreateIssueResult {
|
||||
error?: string;
|
||||
}
|
||||
|
||||
/** PR review comment (conversation or inline) */
|
||||
export interface PrReviewComment {
|
||||
author: string;
|
||||
body: string;
|
||||
/** File path for inline comments (undefined for conversation comments) */
|
||||
path?: string;
|
||||
/** Line number for inline comments */
|
||||
line?: number;
|
||||
}
|
||||
|
||||
/** PR review data including metadata and review comments */
|
||||
export interface PrReviewData {
|
||||
number: number;
|
||||
title: string;
|
||||
body: string;
|
||||
url: string;
|
||||
/** Branch name of the PR head */
|
||||
headRefName: string;
|
||||
/** Conversation comments (non-review) */
|
||||
comments: PrReviewComment[];
|
||||
/** Review comments (from reviews) */
|
||||
reviews: PrReviewComment[];
|
||||
/** Changed file paths */
|
||||
files: string[];
|
||||
}
|
||||
|
||||
export interface GitProvider {
|
||||
/** Check CLI tool availability and authentication status */
|
||||
checkCliStatus(): CliStatus;
|
||||
@ -77,6 +103,9 @@ export interface GitProvider {
|
||||
|
||||
createIssue(options: CreateIssueOptions): CreateIssueResult;
|
||||
|
||||
/** Fetch PR review comments and metadata */
|
||||
fetchPrReviewComments(prNumber: number): PrReviewData;
|
||||
|
||||
/** Find an open PR for the given branch. Returns undefined if no PR exists. */
|
||||
findExistingPr(cwd: string, branch: string): ExistingPr | undefined;
|
||||
|
||||
|
||||
@ -7,8 +7,8 @@
|
||||
*/
|
||||
|
||||
import { checkGhCli, fetchIssue, createIssue } from './issue.js';
|
||||
import { findExistingPr, commentOnPr, createPullRequest } from './pr.js';
|
||||
import type { GitProvider, CliStatus, Issue, ExistingPr, CreateIssueOptions, CreateIssueResult, CreatePrOptions, CreatePrResult, CommentResult } from '../git/types.js';
|
||||
import { findExistingPr, commentOnPr, createPullRequest, fetchPrReviewComments } from './pr.js';
|
||||
import type { GitProvider, CliStatus, Issue, ExistingPr, CreateIssueOptions, CreateIssueResult, CreatePrOptions, CreatePrResult, CommentResult, PrReviewData } from '../git/types.js';
|
||||
|
||||
export class GitHubProvider implements GitProvider {
|
||||
checkCliStatus(): CliStatus {
|
||||
@ -23,6 +23,10 @@ export class GitHubProvider implements GitProvider {
|
||||
return createIssue(options);
|
||||
}
|
||||
|
||||
fetchPrReviewComments(prNumber: number): PrReviewData {
|
||||
return fetchPrReviewComments(prNumber);
|
||||
}
|
||||
|
||||
findExistingPr(cwd: string, branch: string): ExistingPr | undefined {
|
||||
return findExistingPr(cwd, branch);
|
||||
}
|
||||
|
||||
@ -9,4 +9,4 @@ export {
|
||||
resolveIssueTask,
|
||||
} from './issue.js';
|
||||
|
||||
export { buildPrBody } from './pr.js';
|
||||
export { buildPrBody, formatPrReviewAsTask } from './pr.js';
|
||||
|
||||
@ -7,7 +7,7 @@
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import { createLogger, getErrorMessage } from '../../shared/utils/index.js';
|
||||
import { checkGhCli } from './issue.js';
|
||||
import type { Issue, CreatePrOptions, CreatePrResult, ExistingPr, CommentResult } from '../git/types.js';
|
||||
import type { Issue, CreatePrOptions, CreatePrResult, ExistingPr, CommentResult, PrReviewData, PrReviewComment } from '../git/types.js';
|
||||
|
||||
const log = createLogger('github-pr');
|
||||
|
||||
@ -52,6 +52,116 @@ export function commentOnPr(cwd: string, prNumber: number, body: string): Commen
|
||||
}
|
||||
}
|
||||
|
||||
/** JSON fields requested from `gh pr view` for review data */
|
||||
const PR_REVIEW_JSON_FIELDS = 'number,title,body,url,headRefName,comments,reviews,files';
|
||||
|
||||
/** Raw shape returned by `gh pr view --json` for review data */
|
||||
interface GhPrViewReviewResponse {
|
||||
number: number;
|
||||
title: string;
|
||||
body: string;
|
||||
url: string;
|
||||
headRefName: string;
|
||||
comments: Array<{ author: { login: string }; body: string }>;
|
||||
reviews: Array<{
|
||||
author: { login: string };
|
||||
body: string;
|
||||
comments: Array<{ body: string; path: string; line: number; author: { login: string } }>;
|
||||
}>;
|
||||
files: Array<{ path: string }>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch PR review comments and metadata via `gh pr view`.
|
||||
* Throws on failure (PR not found, network error, etc.).
|
||||
*/
|
||||
export function fetchPrReviewComments(prNumber: number): PrReviewData {
|
||||
log.debug('Fetching PR review comments', { prNumber });
|
||||
|
||||
const raw = execFileSync(
|
||||
'gh',
|
||||
['pr', 'view', String(prNumber), '--json', PR_REVIEW_JSON_FIELDS],
|
||||
{ encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] },
|
||||
);
|
||||
|
||||
const data = JSON.parse(raw) as GhPrViewReviewResponse;
|
||||
|
||||
const comments: PrReviewComment[] = data.comments.map((c) => ({
|
||||
author: c.author.login,
|
||||
body: c.body,
|
||||
}));
|
||||
|
||||
const reviews: PrReviewComment[] = [];
|
||||
for (const review of data.reviews) {
|
||||
if (review.body) {
|
||||
reviews.push({ author: review.author.login, body: review.body });
|
||||
}
|
||||
for (const comment of review.comments) {
|
||||
reviews.push({
|
||||
author: comment.author.login,
|
||||
body: comment.body,
|
||||
path: comment.path,
|
||||
line: comment.line,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
number: data.number,
|
||||
title: data.title,
|
||||
body: data.body,
|
||||
url: data.url,
|
||||
headRefName: data.headRefName,
|
||||
comments,
|
||||
reviews,
|
||||
files: data.files.map((f) => f.path),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Format PR review data into task text for piece execution.
|
||||
*/
|
||||
export function formatPrReviewAsTask(prReview: PrReviewData): string {
|
||||
const parts: string[] = [];
|
||||
|
||||
parts.push(`## PR #${prReview.number} Review Comments: ${prReview.title}`);
|
||||
|
||||
if (prReview.body) {
|
||||
parts.push('');
|
||||
parts.push('### PR Description');
|
||||
parts.push(prReview.body);
|
||||
}
|
||||
|
||||
if (prReview.reviews.length > 0) {
|
||||
parts.push('');
|
||||
parts.push('### Review Comments');
|
||||
for (const review of prReview.reviews) {
|
||||
const location = review.path
|
||||
? `\n File: ${review.path}${review.line ? `, Line: ${review.line}` : ''}`
|
||||
: '';
|
||||
parts.push(`**${review.author}**: ${review.body}${location}`);
|
||||
}
|
||||
}
|
||||
|
||||
if (prReview.comments.length > 0) {
|
||||
parts.push('');
|
||||
parts.push('### Conversation Comments');
|
||||
for (const comment of prReview.comments) {
|
||||
parts.push(`**${comment.author}**: ${comment.body}`);
|
||||
}
|
||||
}
|
||||
|
||||
if (prReview.files.length > 0) {
|
||||
parts.push('');
|
||||
parts.push('### Changed Files');
|
||||
for (const file of prReview.files) {
|
||||
parts.push(`- ${file}`);
|
||||
}
|
||||
}
|
||||
|
||||
return parts.join('\n');
|
||||
}
|
||||
|
||||
export function createPullRequest(cwd: string, options: CreatePrOptions): CreatePrResult {
|
||||
const ghStatus = checkGhCli();
|
||||
if (!ghStatus.available) {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user