既存PRへのコメント追加に対応し、PR重複作成を防止

タスク再実行時に同じブランチのPRが既に存在する場合、新規作成ではなく
既存PRにコメントを追加するようにした。
This commit is contained in:
nrslib 2026-02-19 21:21:59 +09:00
parent 4941f8eabf
commit 743344a51b
5 changed files with 245 additions and 18 deletions

View File

@ -1,14 +1,64 @@
/** /**
* Tests for github/pr module * Tests for github/pr module
* *
* Tests buildPrBody formatting. * Tests buildPrBody formatting and findExistingPr logic.
* createPullRequest/pushBranch call `gh`/`git` CLI, not unit-tested here. * createPullRequest/pushBranch/commentOnPr call `gh`/`git` CLI, not unit-tested here.
*/ */
import { describe, it, expect } from 'vitest'; import { describe, it, expect, vi, beforeEach } from 'vitest';
import { buildPrBody } from '../infra/github/pr.js';
const mockExecFileSync = vi.fn();
vi.mock('node:child_process', () => ({
execFileSync: (...args: unknown[]) => mockExecFileSync(...args),
}));
vi.mock('../infra/github/issue.js', () => ({
checkGhCli: vi.fn().mockReturnValue({ available: true }),
}));
vi.mock('../shared/utils/index.js', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
createLogger: () => ({
info: vi.fn(),
debug: vi.fn(),
error: vi.fn(),
}),
getErrorMessage: (e: unknown) => String(e),
}));
import { buildPrBody, findExistingPr } from '../infra/github/pr.js';
import type { GitHubIssue } from '../infra/github/types.js'; import type { GitHubIssue } from '../infra/github/types.js';
describe('findExistingPr', () => {
beforeEach(() => {
vi.clearAllMocks();
});
it('オープンな PR がある場合はその PR を返す', () => {
mockExecFileSync.mockReturnValue(JSON.stringify([{ number: 42, url: 'https://github.com/org/repo/pull/42' }]));
const result = findExistingPr('/project', 'task/fix-bug');
expect(result).toEqual({ number: 42, url: 'https://github.com/org/repo/pull/42' });
});
it('PR がない場合は undefined を返す', () => {
mockExecFileSync.mockReturnValue(JSON.stringify([]));
const result = findExistingPr('/project', 'task/fix-bug');
expect(result).toBeUndefined();
});
it('gh CLI が失敗した場合は undefined を返す', () => {
mockExecFileSync.mockImplementation(() => { throw new Error('gh: command not found'); });
const result = findExistingPr('/project', 'task/fix-bug');
expect(result).toBeUndefined();
});
});
describe('buildPrBody', () => { describe('buildPrBody', () => {
it('should build body with single issue and report', () => { it('should build body with single issue and report', () => {
const issue: GitHubIssue = { const issue: GitHubIssue = {

View File

@ -0,0 +1,116 @@
/**
* Tests for postExecution.ts
*
* Verifies branching logic: existing PR comment, no PR create.
*/
import { describe, it, expect, vi, beforeEach } from 'vitest';
const { mockAutoCommitAndPush, mockPushBranch, mockFindExistingPr, mockCommentOnPr, mockCreatePullRequest, mockBuildPrBody } =
vi.hoisted(() => ({
mockAutoCommitAndPush: vi.fn(),
mockPushBranch: vi.fn(),
mockFindExistingPr: vi.fn(),
mockCommentOnPr: vi.fn(),
mockCreatePullRequest: vi.fn(),
mockBuildPrBody: vi.fn(() => 'pr-body'),
}));
vi.mock('../infra/task/index.js', () => ({
autoCommitAndPush: (...args: unknown[]) => mockAutoCommitAndPush(...args),
}));
vi.mock('../infra/github/index.js', () => ({
pushBranch: (...args: unknown[]) => mockPushBranch(...args),
findExistingPr: (...args: unknown[]) => mockFindExistingPr(...args),
commentOnPr: (...args: unknown[]) => mockCommentOnPr(...args),
createPullRequest: (...args: unknown[]) => mockCreatePullRequest(...args),
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(),
success: vi.fn(),
}));
vi.mock('../shared/utils/index.js', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
createLogger: () => ({
info: vi.fn(),
debug: vi.fn(),
error: vi.fn(),
}),
}));
import { postExecutionFlow } from '../features/tasks/execute/postExecution.js';
const baseOptions = {
execCwd: '/clone',
projectCwd: '/project',
task: 'Fix the bug',
branch: 'task/fix-the-bug',
baseBranch: 'main',
shouldCreatePr: true,
pieceIdentifier: 'default',
};
describe('postExecutionFlow', () => {
beforeEach(() => {
vi.clearAllMocks();
mockAutoCommitAndPush.mockReturnValue({ success: true, commitHash: 'abc123' });
mockPushBranch.mockReturnValue(undefined);
mockCommentOnPr.mockReturnValue({ success: true });
mockCreatePullRequest.mockReturnValue({ success: true, url: 'https://github.com/org/repo/pull/1' });
});
it('既存PRがない場合は createPullRequest を呼ぶ', async () => {
mockFindExistingPr.mockReturnValue(undefined);
await postExecutionFlow(baseOptions);
expect(mockCreatePullRequest).toHaveBeenCalledTimes(1);
expect(mockCommentOnPr).not.toHaveBeenCalled();
});
it('既存PRがある場合は commentOnPr を呼び createPullRequest は呼ばない', async () => {
mockFindExistingPr.mockReturnValue({ number: 42, url: 'https://github.com/org/repo/pull/42' });
await postExecutionFlow(baseOptions);
expect(mockCommentOnPr).toHaveBeenCalledWith('/project', 42, 'pr-body');
expect(mockCreatePullRequest).not.toHaveBeenCalled();
});
it('shouldCreatePr が false の場合は PR 関連処理をスキップする', async () => {
await postExecutionFlow({ ...baseOptions, shouldCreatePr: false });
expect(mockFindExistingPr).not.toHaveBeenCalled();
expect(mockCommentOnPr).not.toHaveBeenCalled();
expect(mockCreatePullRequest).not.toHaveBeenCalled();
});
it('commit がない場合は PR 関連処理をスキップする', async () => {
mockAutoCommitAndPush.mockReturnValue({ success: true, commitHash: undefined });
await postExecutionFlow(baseOptions);
expect(mockFindExistingPr).not.toHaveBeenCalled();
expect(mockCreatePullRequest).not.toHaveBeenCalled();
});
it('branch がない場合は PR 関連処理をスキップする', async () => {
await postExecutionFlow({ ...baseOptions, branch: undefined });
expect(mockFindExistingPr).not.toHaveBeenCalled();
expect(mockCreatePullRequest).not.toHaveBeenCalled();
});
});

View File

@ -10,7 +10,7 @@ import { confirm } from '../../../shared/prompt/index.js';
import { autoCommitAndPush } from '../../../infra/task/index.js'; import { autoCommitAndPush } from '../../../infra/task/index.js';
import { info, error, success } from '../../../shared/ui/index.js'; import { info, error, success } from '../../../shared/ui/index.js';
import { createLogger } from '../../../shared/utils/index.js'; import { createLogger } from '../../../shared/utils/index.js';
import { createPullRequest, buildPrBody, pushBranch } from '../../../infra/github/index.js'; import { createPullRequest, buildPrBody, pushBranch, findExistingPr, commentOnPr } from '../../../infra/github/index.js';
import type { GitHubIssue } from '../../../infra/github/index.js'; import type { GitHubIssue } from '../../../infra/github/index.js';
const log = createLogger('postExecution'); const log = createLogger('postExecution');
@ -56,13 +56,24 @@ export async function postExecutionFlow(options: PostExecutionOptions): Promise<
} }
if (commitResult.success && commitResult.commitHash && branch && shouldCreatePr) { if (commitResult.success && commitResult.commitHash && branch && shouldCreatePr) {
info('Creating pull request...');
try { try {
pushBranch(projectCwd, branch); pushBranch(projectCwd, branch);
} catch (pushError) { } catch (pushError) {
log.info('Branch push from project cwd failed (may already exist)', { error: pushError }); log.info('Branch push from project cwd failed (may already exist)', { error: pushError });
} }
const report = pieceIdentifier ? `Piece \`${pieceIdentifier}\` completed successfully.` : 'Task completed successfully.'; const report = pieceIdentifier ? `Piece \`${pieceIdentifier}\` completed successfully.` : 'Task completed successfully.';
const existingPr = findExistingPr(projectCwd, branch);
if (existingPr) {
// PRが既に存在する場合はコメントを追加push済みなので新コミットはPRに自動反映
const commentBody = buildPrBody(issues, report);
const commentResult = commentOnPr(projectCwd, existingPr.number, commentBody);
if (commentResult.success) {
success(`PR updated with comment: ${existingPr.url}`);
} else {
error(`PR comment failed: ${commentResult.error}`);
}
} else {
info('Creating pull request...');
const prBody = buildPrBody(issues, report); const prBody = buildPrBody(issues, report);
const prResult = createPullRequest(projectCwd, { const prResult = createPullRequest(projectCwd, {
branch, branch,
@ -77,4 +88,5 @@ export async function postExecutionFlow(options: PostExecutionOptions): Promise<
error(`PR creation failed: ${prResult.error}`); error(`PR creation failed: ${prResult.error}`);
} }
} }
}
} }

View File

@ -14,4 +14,5 @@ export {
createIssue, createIssue,
} from './issue.js'; } from './issue.js';
export { pushBranch, createPullRequest, buildPrBody } from './pr.js'; export type { ExistingPr } from './pr.js';
export { pushBranch, createPullRequest, buildPrBody, findExistingPr, commentOnPr } from './pr.js';

View File

@ -13,6 +13,54 @@ export type { CreatePrOptions, CreatePrResult };
const log = createLogger('github-pr'); const log = createLogger('github-pr');
export interface ExistingPr {
number: number;
url: string;
}
/**
* Find an open PR for the given branch.
* Returns undefined if no PR exists.
*/
export function findExistingPr(cwd: string, branch: string): ExistingPr | undefined {
const ghStatus = checkGhCli();
if (!ghStatus.available) return undefined;
try {
const output = execFileSync(
'gh', ['pr', 'list', '--head', branch, '--state', 'open', '--json', 'number,url', '--limit', '1'],
{ cwd, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] },
);
const prs = JSON.parse(output) as ExistingPr[];
return prs[0];
} catch {
return undefined;
}
}
/**
* Add a comment to an existing PR.
*/
export function commentOnPr(cwd: string, prNumber: number, body: string): CreatePrResult {
const ghStatus = checkGhCli();
if (!ghStatus.available) {
return { success: false, error: ghStatus.error ?? 'gh CLI is not available' };
}
try {
execFileSync('gh', ['pr', 'comment', String(prNumber), '--body', body], {
cwd,
encoding: 'utf-8',
stdio: ['pipe', 'pipe', 'pipe'],
});
return { success: true };
} catch (err) {
const errorMessage = getErrorMessage(err);
log.error('PR comment failed', { error: errorMessage });
return { success: false, error: errorMessage };
}
}
/** /**
* Push a branch to origin. * Push a branch to origin.
* Throws on failure. * Throws on failure.