From 452b5b0d113b20ea0ea58ccadff18a2078834560 Mon Sep 17 00:00:00 2001 From: Zbigniew Sobiecki Date: Tue, 24 Mar 2026 16:35:53 +0000 Subject: [PATCH 1/2] fix(router): use trigger result workItemId for GitHub ack run link The GitHub PR ack comment was linking to /work-items/{project}/{prNumber} instead of /work-items/{project}/{trelloCardId}, causing "No runs found" on the dashboard. Use triggerResult.workItemId ?? event.workItemId, mirroring the existing pattern used for PM-focused agent acks (line 320). Co-Authored-By: Claude Sonnet 4.6 --- src/router/adapters/github.ts | 5 ++-- tests/unit/router/adapters/github.test.ts | 36 +++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/router/adapters/github.ts b/src/router/adapters/github.ts index 7b8f77c4..da717bd7 100644 --- a/src/router/adapters/github.ts +++ b/src/router/adapters/github.ts @@ -331,7 +331,8 @@ export class GitHubRouterAdapter implements RouterPlatformAdapter { // Build the GitHub PR ack message with run link included before posting, // so the actual comment on the PR contains the footer (not just internal metadata). let githubAckMessage: string | undefined; - if (runLinksEnabled && event.workItemId) { + const workItemIdForLink = triggerResult?.workItemId ?? event.workItemId; + if (runLinksEnabled && workItemIdForLink) { const dashboardUrl = getDashboardUrl(); if (dashboardUrl) { const context = extractGitHubContext(payload, event.eventType); @@ -339,7 +340,7 @@ export class GitHubRouterAdapter implements RouterPlatformAdapter { const link = buildWorkItemRunsLink({ dashboardUrl, projectId: project.id, - workItemId: event.workItemId, + workItemId: workItemIdForLink, }); githubAckMessage = link ? baseMessage + link : baseMessage; } diff --git a/tests/unit/router/adapters/github.test.ts b/tests/unit/router/adapters/github.test.ts index feecb059..a1495ed6 100644 --- a/tests/unit/router/adapters/github.test.ts +++ b/tests/unit/router/adapters/github.test.ts @@ -69,6 +69,10 @@ vi.mock('../../../../src/pm/trello/integration.js', () => ({ vi.mock('../../../../src/sentry.js', () => ({ captureException: vi.fn(), })); +vi.mock('../../../../src/utils/runLink.js', () => ({ + buildWorkItemRunsLink: vi.fn().mockReturnValue('\n\nšŸ•µļø [View run](https://example.com)'), + getDashboardUrl: vi.fn().mockReturnValue('https://example.com'), +})); import { isPMFocusedAgent } from '../../../../src/agents/definitions/loader.js'; import { findProjectByRepo } from '../../../../src/config/provider.js'; @@ -86,6 +90,7 @@ import { addEyesReactionToPR } from '../../../../src/router/pre-actions.js'; import type { GitHubJob } from '../../../../src/router/queue.js'; import { sendAcknowledgeReaction } from '../../../../src/router/reactions.js'; import type { TriggerRegistry } from '../../../../src/triggers/registry.js'; +import { buildWorkItemRunsLink } from '../../../../src/utils/runLink.js'; const mockProject: RouterProjectConfig = { id: 'p1', @@ -407,6 +412,37 @@ describe('GitHubRouterAdapter', () => { expect(postTrelloAck).toHaveBeenCalledWith('p1', 'trigger-card-id', expect.any(String)); }); + it('uses triggerResult.workItemId over event.workItemId for GitHub PR run link', async () => { + vi.mocked(loadProjectConfig).mockResolvedValue({ + projects: [mockProject], + fullProjects: [{ id: 'p1', repo: 'owner/repo', runLinksEnabled: true } as never], + }); + vi.mocked(resolveGitHubTokenForAckByAgent).mockResolvedValue({ + token: 'ghp_test', + project: { id: 'p1' }, + } as never); + vi.mocked(postGitHubAck).mockResolvedValue(1); + + await adapter.postAck( + { + projectIdentifier: 'owner/repo', + eventType: 'pull_request', + workItemId: '1030', // PR number ā€” should NOT be used for link + isCommentEvent: false, + // @ts-expect-error extended field + repoFullName: 'owner/repo', + }, + {}, + mockProject, + 'review', + { agentType: 'review', agentInput: {}, workItemId: 'trello-card-abc' }, + ); + + expect(buildWorkItemRunsLink).toHaveBeenCalledWith( + expect.objectContaining({ workItemId: 'trello-card-abc' }), + ); + }); + it('returns undefined for PM-focused agents when no workItemId available', async () => { vi.mocked(isPMFocusedAgent).mockResolvedValue(true); From 2bf2c68fa88b7b1a79567faef638a18484c1472b Mon Sep 17 00:00:00 2001 From: Zbigniew Sobiecki Date: Tue, 24 Mar 2026 16:57:08 +0000 Subject: [PATCH 2/2] test(router/adapters/github): fix false-positive run link coverage test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous test for triggerResult.workItemId run link was a false positive: isPMFocusedAgent leaked 'true' from the preceding test (clearMocks only clears call counts, not implementations), so the function took the PM ack path and called buildWorkItemRunsLink via withRunLink ā€” never exercising the changed lines 334-347. Fix: explicitly reset isPMFocusedAgent to false and mock extractPRNumber so postGitHubPRAck doesn't bail early. Lines 336-347 now confirmed covered. Co-Authored-By: Claude Sonnet 4.6 --- tests/unit/router/adapters/github.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/unit/router/adapters/github.test.ts b/tests/unit/router/adapters/github.test.ts index a1495ed6..029c2aa0 100644 --- a/tests/unit/router/adapters/github.test.ts +++ b/tests/unit/router/adapters/github.test.ts @@ -413,6 +413,8 @@ describe('GitHubRouterAdapter', () => { }); it('uses triggerResult.workItemId over event.workItemId for GitHub PR run link', async () => { + // Ensure isPMFocusedAgent returns false so we take the GitHub PR ack path, not PM path + vi.mocked(isPMFocusedAgent).mockResolvedValue(false); vi.mocked(loadProjectConfig).mockResolvedValue({ projects: [mockProject], fullProjects: [{ id: 'p1', repo: 'owner/repo', runLinksEnabled: true } as never], @@ -421,6 +423,7 @@ describe('GitHubRouterAdapter', () => { token: 'ghp_test', project: { id: 'p1' }, } as never); + vi.mocked(extractPRNumber).mockReturnValue(42); vi.mocked(postGitHubAck).mockResolvedValue(1); await adapter.postAck(