Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/github/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,15 @@ export const githubClient = {
};
},

async deletePRComment(owner: string, repo: string, commentId: number): Promise<void> {
logger.debug('Deleting PR comment', { owner, repo, commentId });
await getClient().issues.deleteComment({
owner,
repo,
comment_id: commentId,
});
},

async getPRReviews(owner: string, repo: string, prNumber: number): Promise<PRReview[]> {
logger.debug('Fetching PR reviews', { owner, repo, prNumber });
const { data } = await getClient().pulls.listReviews({
Expand Down
37 changes: 36 additions & 1 deletion src/triggers/github/webhook-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ import { getPersonaToken, resolvePersonaIdentities } from '../../github/personas
import { withPMCredentials } from '../../pm/context.js';
import { createPMProvider, pmRegistry, withPMProvider } from '../../pm/index.js';
import { extractGitHubContext, generateAckMessage } from '../../router/ackMessageGenerator.js';
import type { CascadeConfig, ProjectConfig, TriggerContext } from '../../types/index.js';
import type {
AgentResult,
CascadeConfig,
ProjectConfig,
TriggerContext,
} from '../../types/index.js';
import {
enqueueWebhook,
getQueueLength,
Expand All @@ -25,6 +30,35 @@ import { runAgentExecutionPipeline } from '../shared/agent-execution.js';
import { processNextQueuedWebhook } from '../shared/webhook-queue.js';
import type { TriggerResult } from '../types.js';

async function deleteProgressCommentOnSuccess(
result: TriggerResult,
_agentResult: AgentResult,
): Promise<void> {
// Only delete the progress comment for non-implementation agents.
// The implementation agent's success is handled via lifecycle (handleSuccess),
// which manages the PR comment separately.
if (result.agentType === 'implementation') return;

const input = result.agentInput as { repoFullName?: string };
if (!input.repoFullName || !result.prNumber) return;

let owner: string;
let repo: string;
try {
({ owner, repo } = parseRepoFullName(input.repoFullName));
} catch {
return;
}

const { initialCommentId } = getSessionState();
if (!initialCommentId) return;

await safeOperation(() => githubClient.deletePRComment(owner, repo, initialCommentId), {
action: 'delete progress comment after agent success',
prNumber: result.prNumber,
});
}

async function updateInitialCommentWithError(
result: TriggerResult,
agentResult: { success: boolean; error?: string },
Expand Down Expand Up @@ -105,6 +139,7 @@ async function executeGitHubAgent(
skipPrepareForAgent: true,
skipHandleFailure: true,
handleSuccessOnlyForAgentType: 'implementation',
onSuccess: deleteProgressCommentOnSuccess,
onFailure: updateInitialCommentWithError,
logLabel: 'GitHub agent',
};
Expand Down
12 changes: 11 additions & 1 deletion src/triggers/shared/agent-execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ export interface AgentExecutionConfig {
*/
handleSuccessOnlyForAgentType?: string;

/**
* Optional callback invoked when the agent succeeds (after pipeline completes).
* Used by GitHub to delete the progress comment for non-implementation agents.
*/
onSuccess?: (result: TriggerResult, agentResult: AgentResult) => Promise<void>;

/**
* Optional callback invoked when the agent fails (after pipeline completes).
* Used by GitHub to update the PR comment with an error message.
Expand Down Expand Up @@ -178,7 +184,7 @@ export async function runAgentExecutionPipeline(
return;
}

const { skipPrepareForAgent = false, onFailure, logLabel = 'Agent' } = executionConfig;
const { skipPrepareForAgent = false, onSuccess, onFailure, logLabel = 'Agent' } = executionConfig;

const workItemId = result.workItemId;

Expand Down Expand Up @@ -218,6 +224,10 @@ export async function runAgentExecutionPipeline(
runId: agentResult.runId,
});

if (onSuccess && agentResult.success) {
await onSuccess(result, agentResult);
}

if (onFailure && !agentResult.success) {
await onFailure(result, agentResult);
}
Expand Down
15 changes: 15 additions & 0 deletions tests/unit/github/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const mockPulls = {
const mockIssues = {
createComment: vi.fn(),
updateComment: vi.fn(),
deleteComment: vi.fn(),
listComments: vi.fn(),
};

Expand Down Expand Up @@ -303,6 +304,20 @@ describe('githubClient', () => {
});
});

describe('deletePRComment', () => {
it('calls deleteComment with correct params', async () => {
mockIssues.deleteComment.mockResolvedValue({});

await withGitHubToken('test-token', () => githubClient.deletePRComment('owner', 'repo', 200));

expect(mockIssues.deleteComment).toHaveBeenCalledWith({
owner: 'owner',
repo: 'repo',
comment_id: 200,
});
});
});

describe('getPRReviews', () => {
it('returns mapped reviews', async () => {
mockPulls.listReviews.mockResolvedValue({
Expand Down
30 changes: 30 additions & 0 deletions tests/unit/triggers/agent-execution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,36 @@ describe('runAgentExecutionPipeline', () => {
});
});

describe('onSuccess callback', () => {
it('calls onSuccess when agent succeeds', async () => {
const agentResult: AgentResult = {
success: true,
runId: 'run-1',
output: '',
};
vi.mocked(runAgent).mockResolvedValue(agentResult);
const onSuccess = vi.fn().mockResolvedValue(undefined);

await runAgentExecutionPipeline(mockTriggerResult, mockProject, mockConfig, { onSuccess });

expect(onSuccess).toHaveBeenCalledWith(mockTriggerResult, agentResult);
});

it('does not call onSuccess when agent fails', async () => {
vi.mocked(runAgent).mockResolvedValue({
success: false,
error: 'Agent error',
runId: 'run-1',
output: '',
});
const onSuccess = vi.fn().mockResolvedValue(undefined);

await runAgentExecutionPipeline(mockTriggerResult, mockProject, mockConfig, { onSuccess });

expect(onSuccess).not.toHaveBeenCalled();
});
});

describe('onFailure callback', () => {
it('calls onFailure when agent fails', async () => {
const agentResult: AgentResult = {
Expand Down