diff --git a/src/agents/respond-to-ci.ts b/src/agents/respond-to-ci.ts index 34919ac0..00425dd4 100644 --- a/src/agents/respond-to-ci.ts +++ b/src/agents/respond-to-ci.ts @@ -292,7 +292,6 @@ Use these values when calling GitHub gadgets (GetPRDetails, PostPRComment, Updat const ciAgentDefinition: GitHubAgentDefinition = { agentType: 'respond-to-ci', - headerMessage: '🤖 Working on fixing CI failures...', initialCommentDescription: 'Acknowledge CI failures', timeoutMessage: '⚠️ CI fix agent timed out while attempting to fix failures.', loggerPrefix: 'ci', diff --git a/src/agents/respond-to-pr-comment.ts b/src/agents/respond-to-pr-comment.ts index 26abc141..4b5b0a41 100644 --- a/src/agents/respond-to-pr-comment.ts +++ b/src/agents/respond-to-pr-comment.ts @@ -16,7 +16,6 @@ const respondToPRCommentDefinition: GitHubAgentDefinition< PRResponseContextData > = { agentType: 'respond-to-pr-comment', - headerMessage: '🤖 Working on your request...', initialCommentDescription: 'Acknowledge PR comment request', timeoutMessage: '⚠️ PR comment agent timed out while working on the request.', loggerPrefix: 'pr-comment', diff --git a/src/agents/respond-to-review.ts b/src/agents/respond-to-review.ts index 6c5c0299..1e3f333f 100644 --- a/src/agents/respond-to-review.ts +++ b/src/agents/respond-to-review.ts @@ -15,7 +15,6 @@ const respondToReviewDefinition: GitHubAgentDefinition< PRResponseContextData > = { agentType: 'respond-to-review', - headerMessage: '🤖 Working on addressing the review feedback...', initialCommentDescription: 'Acknowledge review feedback', timeoutMessage: '⚠️ Review agent timed out while addressing feedback.', loggerPrefix: 'review', diff --git a/src/agents/review.ts b/src/agents/review.ts index 25eaba9b..f8761592 100644 --- a/src/agents/review.ts +++ b/src/agents/review.ts @@ -141,7 +141,6 @@ ${skippedFiles.map((f) => `- ${f}`).join('\n')}`; const reviewAgentDefinition: GitHubAgentDefinition = { agentType: 'review', - headerMessage: '🔍 Reviewing PR...', initialCommentDescription: 'Post initial review status comment', timeoutMessage: '⚠️ Review agent timed out while reviewing the PR.', loggerPrefix: 'review', diff --git a/src/agents/shared/githubAgent.ts b/src/agents/shared/githubAgent.ts index 799784c8..94f71bc6 100644 --- a/src/agents/shared/githubAgent.ts +++ b/src/agents/shared/githubAgent.ts @@ -1,6 +1,7 @@ import type { ModelSpec } from 'llmist'; import { createProgressMonitor } from '../../backends/progress.js'; +import { INITIAL_MESSAGES } from '../../config/agentMessages.js'; import { CUSTOM_MODELS } from '../../config/customModels.js'; import { recordInitialComment } from '../../gadgets/sessionState.js'; import { githubClient, withGitHubToken } from '../../github/client.js'; @@ -51,7 +52,8 @@ export interface GitHubAgentDefinition< TContext extends GitHubAgentContext, > { agentType: string; - headerMessage: string; + /** Static header message — last-resort fallback when no ackMessage or INITIAL_MESSAGES entry. */ + headerMessage?: string; initialCommentDescription: string; timeoutMessage: string; loggerPrefix: string; @@ -124,6 +126,16 @@ export async function executeGitHubAgent< if (earlyResult) return earlyResult; } + // Resolve effective header: ackMessage (LLM-generated) > INITIAL_MESSAGES > definition fallback + const effectiveHeader = + (input.ackMessage as string | undefined) ?? + INITIAL_MESSAGES[definition.agentType] ?? + definition.headerMessage ?? + INITIAL_MESSAGES.implementation; + + // Pre-existing ack comment from router or webhook handler + const preExistingAckId = input.ackCommentId as number | undefined; + const runLifecycle = () => executeAgentLifecycle({ loggerIdentifier: `${definition.loggerPrefix}-${prNumber}`, @@ -172,24 +184,37 @@ export async function executeGitHubAgent< }), injectSyntheticCalls: async ({ builder, ctx, trackingContext, repoDir }) => { - const initialComment = await definition.postInitialComment( - input, - id, - definition.headerMessage, - ); - recordInitialComment(initialComment.id); + let initialCommentId: number; + let initialCommentHtmlUrl: string; + let gadgetName: string; + + if (preExistingAckId) { + // Ack comment already posted by router/webhook-handler — reuse it + recordInitialComment(preExistingAckId); + initialCommentId = preExistingAckId; + initialCommentHtmlUrl = `https://github.com/${owner}/${repo}/pull/${prNumber}#issuecomment-${preExistingAckId}`; + gadgetName = 'PostPRComment'; + } else { + // No pre-existing ack — post initial comment now + const initialComment = await definition.postInitialComment(input, id, effectiveHeader); + recordInitialComment(initialComment.id); + initialCommentId = initialComment.id; + initialCommentHtmlUrl = initialComment.htmlUrl; + gadgetName = initialComment.gadgetName; + } + const withComment = injectSyntheticCall( builder, trackingContext, - initialComment.gadgetName, + gadgetName, { comment: definition.initialCommentDescription, owner, repo, prNumber, - body: definition.headerMessage, + body: effectiveHeader, }, - `Comment posted (id: ${initialComment.id}): ${initialComment.htmlUrl}`, + `Comment posted (id: ${initialCommentId}): ${initialCommentHtmlUrl}`, 'gc_initial_comment', ); @@ -211,7 +236,7 @@ export async function executeGitHubAgent< progressModel: input.config.defaults.progressModel, intervalMinutes: input.config.defaults.progressIntervalMinutes, customModels: CUSTOM_MODELS as ModelSpec[], - github: { owner, repo, headerMessage: definition.headerMessage }, + github: { owner, repo, headerMessage: effectiveHeader }, }), interactive, diff --git a/src/agents/shared/prResponseAgent.ts b/src/agents/shared/prResponseAgent.ts index ca4d7418..4f9a739b 100644 --- a/src/agents/shared/prResponseAgent.ts +++ b/src/agents/shared/prResponseAgent.ts @@ -2,13 +2,8 @@ import { githubClient } from '../../github/client.js'; import type { CascadeConfig, ProjectConfig } from '../../types/index.js'; import type { TrackingContext } from '../utils/tracking.js'; import type { BuilderType } from './builderFactory.js'; -import type { - GitHubAgentContext, - GitHubAgentInput, - InitialCommentResult, - RepoIdentifier, -} from './githubAgent.js'; -import { createInitialPRComment } from './githubAgent.js'; +import type { GitHubAgentContext, GitHubAgentInput, RepoIdentifier } from './githubAgent.js'; +import { type InitialCommentResult, createInitialPRComment } from './githubAgent.js'; import { resolveModelConfig } from './modelResolution.js'; import { formatPRComments, @@ -33,7 +28,6 @@ export interface PRResponseAgentInput extends GitHubAgentInput { triggerCommentBody: string; triggerCommentPath: string; triggerCommentUrl: string; - acknowledgmentCommentId?: number; } export interface PRResponseContextData extends GitHubAgentContext { @@ -138,15 +132,6 @@ export async function postInitialPRResponseComment( id: RepoIdentifier, headerMessage: string, ): Promise { - if (input.acknowledgmentCommentId) { - const comment = await githubClient.updatePRComment( - id.owner, - id.repo, - input.acknowledgmentCommentId, - headerMessage, - ); - return { id: comment.id, htmlUrl: comment.htmlUrl, gadgetName: 'UpdatePRComment' }; - } return createInitialPRComment(input.prNumber, id, headerMessage); } diff --git a/src/backends/agent-profiles.ts b/src/backends/agent-profiles.ts index d099ddb9..9bb76e89 100644 --- a/src/backends/agent-profiles.ts +++ b/src/backends/agent-profiles.ts @@ -23,6 +23,7 @@ import { buildWorkItemPrompt, } from '../agents/shared/taskPrompts.js'; import type { ContextFile } from '../agents/utils/setup.js'; +import { INITIAL_MESSAGES } from '../config/agentMessages.js'; import { ListDirectory } from '../gadgets/ListDirectory.js'; import { formatCheckStatus } from '../gadgets/github/core/getPRChecks.js'; import { readWorkItem } from '../gadgets/pm/core/readWorkItem.js'; @@ -480,12 +481,16 @@ const reviewProfile: AgentProfile = { getLlmistGadgets: (_agentType) => buildReviewGadgets(), async preExecute({ input, logWriter }: PreExecuteParams): Promise { + // Skip if ack comment already posted by router or webhook handler + if (input.ackCommentId) return; + const repoFullName = input.repoFullName as string; const prNumber = input.prNumber as number; const { owner, repo } = parseRepoFullName(repoFullName); + const message = (input.ackMessage as string | undefined) ?? INITIAL_MESSAGES.review; logWriter('INFO', 'Posting initial review comment', { owner, repo, prNumber }); - await githubClient.createPRComment(owner, repo, prNumber, '🔍 Reviewing PR...'); + await githubClient.createPRComment(owner, repo, prNumber, message); }, }; @@ -519,17 +524,16 @@ const respondToCIProfile: AgentProfile = { getLlmistGadgets: (_agentType) => buildPRAgentGadgets(), async preExecute({ input, logWriter }: PreExecuteParams): Promise { + // Skip if ack comment already posted by router or webhook handler + if (input.ackCommentId) return; + const repoFullName = input.repoFullName as string; const prNumber = input.prNumber as number; const { owner, repo } = parseRepoFullName(repoFullName); + const message = (input.ackMessage as string | undefined) ?? INITIAL_MESSAGES['respond-to-ci']; logWriter('INFO', 'Posting initial CI fix comment', { owner, repo, prNumber }); - await githubClient.createPRComment( - owner, - repo, - prNumber, - '🤖 Working on fixing CI failures...', - ); + await githubClient.createPRComment(owner, repo, prNumber, message); }, }; diff --git a/src/router/github.ts b/src/router/github.ts index 71d4170e..5adb4e62 100644 --- a/src/router/github.ts +++ b/src/router/github.ts @@ -27,14 +27,14 @@ import { sendAcknowledgeReaction } from './reactions.js'; /** * Try to match a trigger and post an ack comment for a GitHub webhook. - * Returns the ack comment ID if posted, undefined otherwise. + * Returns the ack comment ID and message text if posted, undefined otherwise. */ export async function tryPostGitHubAck( eventType: string, repoFullName: string, payload: unknown, triggerRegistry: TriggerRegistry, -): Promise { +): Promise<{ commentId: number; message: string } | undefined> { const config = await loadProjectConfig(); const fullProject = config.fullProjects.find((fp) => fp.repo === repoFullName); if (!fullProject) return undefined; @@ -66,7 +66,7 @@ export async function tryPostGitHubAck( if (!prNumber) return undefined; const commentId = await postGitHubAck(repoFullName, prNumber, message, resolved.token); - return commentId ?? undefined; + return commentId != null ? { commentId, message } : undefined; } export async function isSelfAuthoredGitHubComment( @@ -147,8 +147,13 @@ export async function processGitHubWebhookEvent( // Try to post an ack comment via trigger matching (non-blocking best-effort) let ackCommentId: number | undefined; + let ackMessage: string | undefined; try { - ackCommentId = await tryPostGitHubAck(eventType, repoFullName, payload, triggerRegistry); + const ackResult = await tryPostGitHubAck(eventType, repoFullName, payload, triggerRegistry); + if (ackResult) { + ackCommentId = ackResult.commentId; + ackMessage = ackResult.message; + } } catch (err) { console.warn('[Router] GitHub ack comment failed (non-fatal):', String(err)); } @@ -161,6 +166,7 @@ export async function processGitHubWebhookEvent( repoFullName, receivedAt: new Date().toISOString(), ackCommentId, + ackMessage, }; // Fire pre-actions (non-blocking) before queueing diff --git a/src/router/queue.ts b/src/router/queue.ts index 31966b57..80714d32 100644 --- a/src/router/queue.ts +++ b/src/router/queue.ts @@ -33,6 +33,7 @@ export interface GitHubJob { repoFullName: string; receivedAt: string; ackCommentId?: number; + ackMessage?: string; } export interface JiraJob { diff --git a/src/triggers/github/webhook-handler.ts b/src/triggers/github/webhook-handler.ts index 0e563755..1b0b1e08 100644 --- a/src/triggers/github/webhook-handler.ts +++ b/src/triggers/github/webhook-handler.ts @@ -1,8 +1,10 @@ +import { INITIAL_MESSAGES } from '../../config/agentMessages.js'; import { loadProjectConfigByRepo } from '../../config/provider.js'; import { getSessionState } from '../../gadgets/sessionState.js'; import { githubClient, withGitHubToken } from '../../github/client.js'; import { getPersonaToken, resolvePersonaIdentities } from '../../github/personas.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 { enqueueWebhook, @@ -48,33 +50,43 @@ async function updateInitialCommentWithError( }); } -async function postAcknowledgmentComment(result: TriggerResult): Promise { - if ( - (result.agentType !== 'respond-to-review' && result.agentType !== 'respond-to-pr-comment') || - !result.prNumber - ) { +async function postAcknowledgmentComment( + result: TriggerResult, + payload: unknown, + eventType: string, +): Promise { + if (!result.agentType || !result.prNumber) { return; } const input = result.agentInput as { repoFullName?: string; - acknowledgmentCommentId?: number; - commentAuthor?: string; + project?: ProjectConfig; }; if (!input.repoFullName) { return; } const { owner, repo } = parseRepoFullName(input.repoFullName); const prNumber = result.prNumber; - const message = - result.agentType === 'respond-to-pr-comment' - ? `💭 Thinking about your comment, @${input.commentAuthor ?? 'you'}...` - : '👀 Checking this out...'; + + // Generate LLM ack message, falling back to static INITIAL_MESSAGES + let message: string; + try { + const context = extractGitHubContext(payload, eventType); + const projectId = input.project?.id; + message = projectId + ? await generateAckMessage(result.agentType, context, projectId) + : (INITIAL_MESSAGES[result.agentType] ?? INITIAL_MESSAGES.implementation); + } catch { + message = INITIAL_MESSAGES[result.agentType] ?? INITIAL_MESSAGES.implementation; + } + const comment = await safeOperation( () => githubClient.createPRComment(owner, repo, prNumber, message), { action: 'post acknowledgment comment', prNumber }, ); if (comment) { - input.acknowledgmentCommentId = comment.id; + result.agentInput.ackCommentId = comment.id; + result.agentInput.ackMessage = message; } } @@ -128,7 +140,10 @@ async function runGitHubAgentJob( config: CascadeConfig, githubToken: string, registry: TriggerRegistry, + payload: unknown, + eventType: string, routerAckCommentId?: number, + routerAckMessage?: string, ): Promise { if (!result.agentType) return; // Use the persona token for the agent that will do the work (for ack comments) @@ -139,10 +154,15 @@ async function runGitHubAgentJob( prCommentToken = githubToken; } - // Skip worker-side ack if the router already posted one - if (!routerAckCommentId) { + // Skip worker-side ack if the router already posted one; otherwise generate one for all agents + if (routerAckCommentId) { + // Router already posted — just propagate the message text + if (routerAckMessage) { + result.agentInput.ackMessage = routerAckMessage; + } + } else { await withGitHubToken(prCommentToken, async () => { - await postAcknowledgmentComment(result); + await postAcknowledgmentComment(result, payload, eventType); }); } setProcessing(true); @@ -163,12 +183,13 @@ async function runGitHubAgentJob( function processNextQueuedGitHubWebhook(registry: TriggerRegistry): void { processNextQueuedWebhook( - (payload, eventType, ackCommentId) => + (payload, eventType, ackCommentId, ackMsg) => processGitHubWebhook( payload, eventType ?? 'pull_request_review_comment', registry, ackCommentId as number | undefined, + ackMsg, ), 'GitHub', (entry) => entry.eventType ?? 'pull_request_review_comment', @@ -180,6 +201,7 @@ export async function processGitHubWebhook( eventType: string, registry: TriggerRegistry, ackCommentId?: number, + ackMessage?: string, ): Promise { logger.info('Processing GitHub webhook', { eventType }); @@ -193,7 +215,7 @@ export async function processGitHubWebhook( } if (isCurrentlyProcessing()) { - const queued = enqueueWebhook(payload, eventType, ackCommentId); + const queued = enqueueWebhook(payload, eventType, ackCommentId, ackMessage); if (queued) { logger.info('Currently processing, GitHub webhook queued', { queueLength: getQueueLength(), @@ -234,10 +256,13 @@ export async function processGitHubWebhook( return; } - // Pass ack comment ID into agent input for ProgressMonitor pre-seeding + // Pass ack comment ID + message into agent input for ProgressMonitor pre-seeding if (ackCommentId) { result.agentInput.ackCommentId = ackCommentId; } + if (ackMessage) { + result.agentInput.ackMessage = ackMessage; + } logger.info('GitHub trigger matched', { agentType: result.agentType || '(no agent)', @@ -245,7 +270,17 @@ export async function processGitHubWebhook( }); if (result.agentType) { - await runGitHubAgentJob(result, project, config, githubToken, registry, ackCommentId); + await runGitHubAgentJob( + result, + project, + config, + githubToken, + registry, + payload, + eventType, + ackCommentId, + ackMessage, + ); } else { logger.info('Trigger completed without agent', { prNumber: result.prNumber }); } diff --git a/src/triggers/shared/webhook-queue.ts b/src/triggers/shared/webhook-queue.ts index 48892b9f..7cc8d74c 100644 --- a/src/triggers/shared/webhook-queue.ts +++ b/src/triggers/shared/webhook-queue.ts @@ -13,6 +13,7 @@ export function processNextQueuedWebhook( payload: unknown, eventType?: string, ackCommentId?: string | number, + ackMessage?: string, ) => Promise, label: string, getEventType?: (entry: { payload: unknown; eventType?: string }) => string | undefined, @@ -24,8 +25,10 @@ export function processNextQueuedWebhook( if (eventType) logContext.eventType = eventType; logger.info(`Processing queued ${label} webhook`, logContext); setImmediate(() => { - processWebhook(next.payload, eventType, next.ackCommentId).catch((err) => { - logger.error(`Failed to process queued ${label} webhook`, { error: String(err) }); + processWebhook(next.payload, eventType, next.ackCommentId, next.ackMessage).catch((err) => { + logger.error(`Failed to process queued ${label} webhook`, { + error: String(err), + }); }); }); } diff --git a/src/types/index.ts b/src/types/index.ts index ee72b897..59ef22e8 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -41,6 +41,8 @@ export interface AgentInput { // Router-posted ack comment ID — used by ProgressMonitor to update in-place ackCommentId?: string | number; + // Router/webhook-handler-posted ack message text — reused as initial comment header + ackMessage?: string; [key: string]: unknown; } diff --git a/src/utils/webhookQueue.ts b/src/utils/webhookQueue.ts index 3edda042..723f6733 100644 --- a/src/utils/webhookQueue.ts +++ b/src/utils/webhookQueue.ts @@ -6,6 +6,7 @@ interface QueuedWebhook { payload: unknown; eventType?: string; // Optional for backward compatibility (Trello doesn't need it) ackCommentId?: string | number; + ackMessage?: string; receivedAt: Date; } @@ -15,6 +16,7 @@ export function enqueueWebhook( payload: unknown, eventType?: string, ackCommentId?: string | number, + ackMessage?: string, ): boolean { if (queue.length >= MAX_QUEUE_SIZE) { logger.warn('Webhook queue full, rejecting', { @@ -28,6 +30,7 @@ export function enqueueWebhook( payload, eventType, ackCommentId, + ackMessage, receivedAt: new Date(), }); diff --git a/src/worker-entry.ts b/src/worker-entry.ts index b28efd85..ce709d07 100644 --- a/src/worker-entry.ts +++ b/src/worker-entry.ts @@ -45,6 +45,7 @@ interface GitHubJobData { repoFullName: string; receivedAt: string; ackCommentId?: number; + ackMessage?: string; } interface JiraJobData { @@ -201,6 +202,7 @@ async function main(): Promise { jobData.eventType, triggerRegistry, jobData.ackCommentId, + jobData.ackMessage, ); } else if (jobData.type === 'jira') { logger.info('[Worker] Processing JIRA job', { diff --git a/tests/unit/agents/shared/prResponseAgent.test.ts b/tests/unit/agents/shared/prResponseAgent.test.ts index 9065b763..9e0cc7de 100644 --- a/tests/unit/agents/shared/prResponseAgent.test.ts +++ b/tests/unit/agents/shared/prResponseAgent.test.ts @@ -122,24 +122,7 @@ describe('prResponseAgent shared module', () => { triggerCommentUrl: 'url', } as PRResponseAgentInput; - it('updates existing comment when acknowledgmentCommentId is set', async () => { - const input = { ...baseInput, acknowledgmentCommentId: 555 }; - mockGithub.updatePRComment.mockResolvedValue({ - id: 555, - htmlUrl: 'https://example.com/555', - } as ReturnType extends Promise ? R : never); - - const result = await postInitialPRResponseComment(input, id, 'header'); - - expect(mockGithub.updatePRComment).toHaveBeenCalledWith('org', 'repo', 555, 'header'); - expect(result).toEqual({ - id: 555, - htmlUrl: 'https://example.com/555', - gadgetName: 'UpdatePRComment', - }); - }); - - it('creates a new comment when no acknowledgmentCommentId', async () => { + it('creates a new comment via createInitialPRComment', async () => { mockCreateInitialPRComment.mockResolvedValue({ id: 999, htmlUrl: 'https://example.com/999', diff --git a/tests/unit/backends/agent-profiles.test.ts b/tests/unit/backends/agent-profiles.test.ts index 5bd4d464..36acc6f8 100644 --- a/tests/unit/backends/agent-profiles.test.ts +++ b/tests/unit/backends/agent-profiles.test.ts @@ -109,6 +109,9 @@ vi.mock('../../../src/github/client.js', () => ({ vi.mock('../../../src/agents/utils/setup.js', () => ({})); import { type AgentProfile, getAgentProfile } from '../../../src/backends/agent-profiles.js'; +import { githubClient } from '../../../src/github/client.js'; + +const mockGithub = vi.mocked(githubClient); beforeEach(() => { vi.clearAllMocks(); @@ -193,6 +196,127 @@ describe('getAgentProfile', () => { it('has a preExecute hook', () => { expect(profile.preExecute).toBeDefined(); }); + + it('preExecute skips posting when ackCommentId exists', async () => { + const logWriter = vi.fn(); + await profile.preExecute?.({ + input: { + prNumber: 42, + prBranch: 'fix/ci', + repoFullName: 'acme/widgets', + ackCommentId: 12345, + }, + logWriter, + }); + + expect(mockGithub.createPRComment).not.toHaveBeenCalled(); + }); + + it('preExecute posts with ackMessage when no ackCommentId', async () => { + const logWriter = vi.fn(); + mockGithub.createPRComment.mockResolvedValue(undefined as never); + + await profile.preExecute?.({ + input: { + prNumber: 42, + prBranch: 'fix/ci', + repoFullName: 'acme/widgets', + ackMessage: 'On it — checking the CI failures...', + }, + logWriter, + }); + + expect(mockGithub.createPRComment).toHaveBeenCalledWith( + 'acme', + 'widgets', + 42, + 'On it — checking the CI failures...', + ); + }); + + it('preExecute falls back to INITIAL_MESSAGES when no ackCommentId or ackMessage', async () => { + const logWriter = vi.fn(); + mockGithub.createPRComment.mockResolvedValue(undefined as never); + + await profile.preExecute?.({ + input: { + prNumber: 42, + prBranch: 'fix/ci', + repoFullName: 'acme/widgets', + }, + logWriter, + }); + + expect(mockGithub.createPRComment).toHaveBeenCalledWith( + 'acme', + 'widgets', + 42, + expect.stringContaining('Fixing CI failures'), + ); + }); + }); + + describe('review profile preExecute', () => { + let profile: AgentProfile; + + beforeEach(() => { + profile = getAgentProfile('review'); + }); + + it('skips posting when ackCommentId exists', async () => { + const logWriter = vi.fn(); + await profile.preExecute?.({ + input: { + prNumber: 10, + repoFullName: 'org/repo', + ackCommentId: 999, + }, + logWriter, + }); + + expect(mockGithub.createPRComment).not.toHaveBeenCalled(); + }); + + it('posts with ackMessage when no ackCommentId', async () => { + const logWriter = vi.fn(); + mockGithub.createPRComment.mockResolvedValue(undefined as never); + + await profile.preExecute?.({ + input: { + prNumber: 10, + repoFullName: 'org/repo', + ackMessage: 'Looking into the PR changes now...', + }, + logWriter, + }); + + expect(mockGithub.createPRComment).toHaveBeenCalledWith( + 'org', + 'repo', + 10, + 'Looking into the PR changes now...', + ); + }); + + it('falls back to INITIAL_MESSAGES when no ackCommentId or ackMessage', async () => { + const logWriter = vi.fn(); + mockGithub.createPRComment.mockResolvedValue(undefined as never); + + await profile.preExecute?.({ + input: { + prNumber: 10, + repoFullName: 'org/repo', + }, + logWriter, + }); + + expect(mockGithub.createPRComment).toHaveBeenCalledWith( + 'org', + 'repo', + 10, + expect.stringContaining('Reviewing code'), + ); + }); }); describe('respond-to-pr-comment profile', () => { diff --git a/tests/unit/router/github.test.ts b/tests/unit/router/github.test.ts index 1298cc02..f81012da 100644 --- a/tests/unit/router/github.test.ts +++ b/tests/unit/router/github.test.ts @@ -34,12 +34,15 @@ vi.mock('../../../src/github/personas.js', () => ({ import { findProjectByRepo } from '../../../src/config/provider.js'; import { isCascadeBot, resolvePersonaIdentities } from '../../../src/github/personas.js'; -import { resolveGitHubTokenForAck } from '../../../src/router/acknowledgments.js'; +import { generateAckMessage } from '../../../src/router/ackMessageGenerator.js'; +import { postGitHubAck, resolveGitHubTokenForAck } from '../../../src/router/acknowledgments.js'; +import { loadProjectConfig } from '../../../src/router/config.js'; import { firePreActions, handleGitHubWebhook, isSelfAuthoredGitHubComment, processGitHubWebhookEvent, + tryPostGitHubAck, } from '../../../src/router/github.js'; import { extractPRNumber } from '../../../src/router/notifications.js'; import { addEyesReactionToPR } from '../../../src/router/pre-actions.js'; @@ -227,4 +230,128 @@ describe('processGitHubWebhookEvent', () => { expect(sendAcknowledgeReaction).not.toHaveBeenCalled(); }); + + it('stores ackCommentId and ackMessage on the job when ack succeeds', async () => { + // Setup: make tryPostGitHubAck succeed by mocking its dependencies + vi.mocked(loadProjectConfig).mockResolvedValue({ + projects: [], + fullProjects: [{ id: 'proj-1', repo: 'owner/repo' }], + } as never); + vi.mocked(resolvePersonaIdentities).mockResolvedValue({} as never); + (mockTriggerRegistry.matchTrigger as ReturnType).mockReturnValue({ + agentType: 'review', + }); + vi.mocked(generateAckMessage).mockResolvedValue('Looking into the PR now...'); + vi.mocked(resolveGitHubTokenForAck).mockResolvedValue({ + token: 'ghp_test', + project: { id: 'proj-1' }, + } as never); + vi.mocked(extractPRNumber).mockReturnValue(42); + vi.mocked(postGitHubAck).mockResolvedValue(12345); + vi.mocked(addJob).mockResolvedValue('job-1'); + + await processGitHubWebhookEvent( + 'pull_request', + 'owner/repo', + { repository: { full_name: 'owner/repo' } }, + mockTriggerRegistry, + ); + + expect(addJob).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'github', + ackCommentId: 12345, + ackMessage: 'Looking into the PR now...', + }), + ); + }); + + it('leaves ackMessage undefined when ack fails', async () => { + vi.mocked(resolveGitHubTokenForAck).mockResolvedValue(null); + vi.mocked(addJob).mockResolvedValue('job-1'); + + await processGitHubWebhookEvent( + 'pull_request', + 'owner/repo', + { repository: { full_name: 'owner/repo' } }, + mockTriggerRegistry, + ); + + expect(addJob).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'github', + ackCommentId: undefined, + ackMessage: undefined, + }), + ); + }); +}); + +describe('tryPostGitHubAck', () => { + it('returns commentId and message on success', async () => { + vi.mocked(loadProjectConfig).mockResolvedValue({ + projects: [], + fullProjects: [{ id: 'proj-1', repo: 'owner/repo' }], + } as never); + vi.mocked(resolvePersonaIdentities).mockResolvedValue({} as never); + (mockTriggerRegistry.matchTrigger as ReturnType).mockReturnValue({ + agentType: 'review', + }); + vi.mocked(generateAckMessage).mockResolvedValue('Checking it out...'); + vi.mocked(resolveGitHubTokenForAck).mockResolvedValue({ + token: 'ghp_test', + project: { id: 'proj-1' }, + } as never); + vi.mocked(extractPRNumber).mockReturnValue(5); + vi.mocked(postGitHubAck).mockResolvedValue(777); + + const result = await tryPostGitHubAck( + 'pull_request_review', + 'owner/repo', + {}, + mockTriggerRegistry, + ); + + expect(result).toEqual({ commentId: 777, message: 'Checking it out...' }); + }); + + it('returns undefined when no trigger matches', async () => { + vi.mocked(loadProjectConfig).mockResolvedValue({ + projects: [], + fullProjects: [{ id: 'proj-1', repo: 'owner/repo' }], + } as never); + vi.mocked(resolvePersonaIdentities).mockResolvedValue({} as never); + (mockTriggerRegistry.matchTrigger as ReturnType).mockReturnValue(null); + + const result = await tryPostGitHubAck('pull_request', 'owner/repo', {}, mockTriggerRegistry); + + expect(result).toBeUndefined(); + }); + + it('returns undefined when postGitHubAck returns null', async () => { + vi.mocked(loadProjectConfig).mockResolvedValue({ + projects: [], + fullProjects: [{ id: 'proj-1', repo: 'owner/repo' }], + } as never); + vi.mocked(resolvePersonaIdentities).mockResolvedValue({} as never); + (mockTriggerRegistry.matchTrigger as ReturnType).mockReturnValue({ + agentType: 'review', + }); + vi.mocked(generateAckMessage).mockResolvedValue('Msg'); + vi.mocked(resolveGitHubTokenForAck).mockResolvedValue({ + token: 'ghp_test', + project: { id: 'proj-1' }, + } as never); + vi.mocked(extractPRNumber).mockReturnValue(5); + vi.mocked(postGitHubAck).mockResolvedValue(null); + + const result = await tryPostGitHubAck( + 'pull_request_review', + 'owner/repo', + {}, + mockTriggerRegistry, + ); + + expect(result).toBeUndefined(); + }); }); diff --git a/tests/unit/triggers/webhook-queue.test.ts b/tests/unit/triggers/webhook-queue.test.ts index 534e9c19..308fb7e3 100644 --- a/tests/unit/triggers/webhook-queue.test.ts +++ b/tests/unit/triggers/webhook-queue.test.ts @@ -28,6 +28,7 @@ describe('processNextQueuedWebhook', () => { { action: 'test' }, undefined, // eventType comes from getEventType, not the queued entry undefined, // no ackCommentId + undefined, // no ackMessage ); }); @@ -39,7 +40,12 @@ describe('processNextQueuedWebhook', () => { await new Promise((resolve) => setImmediate(resolve)); - expect(processWebhook).toHaveBeenCalledWith({ action: 'test' }, 'pull_request', undefined); + expect(processWebhook).toHaveBeenCalledWith( + { action: 'test' }, + 'pull_request', + undefined, + undefined, + ); }); it('forwards ackCommentId through the queue', async () => { @@ -50,7 +56,12 @@ describe('processNextQueuedWebhook', () => { await new Promise((resolve) => setImmediate(resolve)); - expect(processWebhook).toHaveBeenCalledWith({ action: 'test' }, 'issue_comment', 'ack-123'); + expect(processWebhook).toHaveBeenCalledWith( + { action: 'test' }, + 'issue_comment', + 'ack-123', + undefined, + ); }); it('forwards numeric ackCommentId through the queue', async () => { @@ -61,7 +72,23 @@ describe('processNextQueuedWebhook', () => { await new Promise((resolve) => setImmediate(resolve)); - expect(processWebhook).toHaveBeenCalledWith({ action: 'test' }, undefined, 10646); + expect(processWebhook).toHaveBeenCalledWith({ action: 'test' }, undefined, 10646, undefined); + }); + + it('forwards ackMessage through the queue', async () => { + const processWebhook = vi.fn().mockResolvedValue(undefined); + enqueueWebhook({ action: 'test' }, 'issue_comment', 'ack-123', 'Looking into it...'); + + processNextQueuedWebhook(processWebhook, 'Test', (entry) => entry.eventType); + + await new Promise((resolve) => setImmediate(resolve)); + + expect(processWebhook).toHaveBeenCalledWith( + { action: 'test' }, + 'issue_comment', + 'ack-123', + 'Looking into it...', + ); }); it('processes items in FIFO order preserving ackCommentId', async () => { @@ -73,12 +100,12 @@ describe('processNextQueuedWebhook', () => { processNextQueuedWebhook(processWebhook, 'Test'); await new Promise((resolve) => setImmediate(resolve)); - expect(processWebhook).toHaveBeenCalledWith({ order: 1 }, undefined, 'first-ack'); + expect(processWebhook).toHaveBeenCalledWith({ order: 1 }, undefined, 'first-ack', undefined); // Process second item processNextQueuedWebhook(processWebhook, 'Test'); await new Promise((resolve) => setImmediate(resolve)); - expect(processWebhook).toHaveBeenCalledWith({ order: 2 }, undefined, 'second-ack'); + expect(processWebhook).toHaveBeenCalledWith({ order: 2 }, undefined, 'second-ack', undefined); }); }); diff --git a/tests/unit/utils/webhookQueue.test.ts b/tests/unit/utils/webhookQueue.test.ts index 0a830a97..4c36c098 100644 --- a/tests/unit/utils/webhookQueue.test.ts +++ b/tests/unit/utils/webhookQueue.test.ts @@ -163,6 +163,34 @@ describe('webhookQueue', () => { }); }); + describe('ackMessage', () => { + it('preserves ackMessage through enqueue/dequeue', () => { + enqueueWebhook({ action: 'test' }, 'issue_comment', 'ack-1', 'Looking into it...'); + + const item = dequeueWebhook(); + + expect(item?.ackMessage).toBe('Looking into it...'); + }); + + it('defaults ackMessage to undefined when not provided', () => { + enqueueWebhook({ action: 'test' }, undefined, 'ack-1'); + + const item = dequeueWebhook(); + + expect(item?.ackMessage).toBeUndefined(); + }); + + it('preserves ackMessage alongside ackCommentId and eventType', () => { + enqueueWebhook({ action: 'test' }, 'pull_request', 42, 'On it — checking the PR...'); + + const item = dequeueWebhook(); + + expect(item?.eventType).toBe('pull_request'); + expect(item?.ackCommentId).toBe(42); + expect(item?.ackMessage).toBe('On it — checking the PR...'); + }); + }); + describe('getMaxQueueSize', () => { it('returns the maximum queue size', () => { const maxSize = getMaxQueueSize();