diff --git a/src/triggers/github/check-suite-success.ts b/src/triggers/github/check-suite-success.ts index 8c0c050f..4746b973 100644 --- a/src/triggers/github/check-suite-success.ts +++ b/src/triggers/github/check-suite-success.ts @@ -15,7 +15,11 @@ import { releaseReviewDispatch, } from './review-dispatch-dedup.js'; import { type GitHubCheckSuitePayload, isGitHubCheckSuitePayload } from './types.js'; -import { parsePrNumberFromRef, resolveWorkItemDisplayData, resolveWorkItemId } from './utils.js'; +import { + parsePrNumberFromRef, + resolveWorkItemDisplayData, + resolveWorkItemIdWithFallback, +} from './utils.js'; /** * Dispatches an outcome agent when a check_suite completes with success @@ -130,8 +134,12 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler { return skip(this.name, gateDecision.message); } - // Resolve work item from DB - const workItemId = await resolveWorkItemId(ctx.project.id, prNumber); + // Resolve work item: DB link, else derive the JIRA key from the PR itself. + const workItemId = await resolveWorkItemIdWithFallback(ctx.project, prNumber, { + branch: prDetails.headRef, + title: prDetails.title, + body: prDetails.body, + }); const { workItemUrl, workItemTitle } = await resolveWorkItemDisplayData(workItemId); // Aggregate-state fork. GitHub fires check_suite.completed once per diff --git a/src/triggers/github/pr-opened.ts b/src/triggers/github/pr-opened.ts index ece4b82e..4469de17 100644 --- a/src/triggers/github/pr-opened.ts +++ b/src/triggers/github/pr-opened.ts @@ -3,7 +3,7 @@ import { logger } from '../../utils/logging.js'; import { skip } from '../shared/skip.js'; import { checkTriggerEnabledWithParams } from '../shared/trigger-check.js'; import { isGitHubPullRequestPayload } from './types.js'; -import { evaluateAuthorMode, resolveWorkItemId } from './utils.js'; +import { evaluateAuthorMode, resolveWorkItemIdWithFallback } from './utils.js'; /** * Trigger that fires the review agent when a new PR is opened. @@ -85,8 +85,12 @@ export class PROpenedTrigger implements TriggerHandler { ); } - // Resolve work item from DB - const workItemId = await resolveWorkItemId(ctx.project.id, prNumber); + // Resolve work item: DB link, else derive the JIRA key from the PR itself. + const workItemId = await resolveWorkItemIdWithFallback(ctx.project, prNumber, { + branch: payload.pull_request.head.ref, + title: payload.pull_request.title, + body: payload.pull_request.body, + }); logger.info('New PR opened, triggering review agent', { prNumber, diff --git a/src/triggers/github/review-requested.ts b/src/triggers/github/review-requested.ts index 66441600..8c60c430 100644 --- a/src/triggers/github/review-requested.ts +++ b/src/triggers/github/review-requested.ts @@ -10,7 +10,7 @@ import { releaseReviewDispatch, } from './review-dispatch-dedup.js'; import { type GitHubPullRequestPayload, isGitHubPullRequestPayload } from './types.js'; -import { resolveWorkItemId } from './utils.js'; +import { resolveWorkItemIdWithFallback } from './utils.js'; /** * Trigger that fires the review agent when review is requested from a CASCADE persona account. @@ -94,8 +94,13 @@ export class ReviewRequestedTrigger implements TriggerHandler { ); } - // Resolve work item from DB - const workItemId = await resolveWorkItemId(ctx.project.id, prNumber); + // Resolve work item: DB link, else derive the JIRA key from the PR itself + // (branch / title / last body line) so review-only projects link human PRs. + const workItemId = await resolveWorkItemIdWithFallback(ctx.project, prNumber, { + branch: payload.pull_request.head.ref, + title: payload.pull_request.title, + body: payload.pull_request.body, + }); const reviewDispatchKey = buildReviewDispatchKey(owner, repo, prNumber, headSha); // Human-initiated review requests override any prior automated dispatch claim. await releaseReviewDispatch(reviewDispatchKey); diff --git a/src/triggers/github/utils.ts b/src/triggers/github/utils.ts index 17235dff..fb070c04 100644 --- a/src/triggers/github/utils.ts +++ b/src/triggers/github/utils.ts @@ -160,3 +160,86 @@ export async function resolveWorkItemDisplayData( return {}; } } + +/** Escape a string for safe use as a literal inside a RegExp. */ +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +/** + * Return the last non-empty (trimmed) line of a block of text, or null. + * PR descriptions conventionally carry the work-item key on a trailing line, so + * restricting body parsing to that line keeps prose that mentions other tokens + * from producing false matches. + */ +function lastNonEmptyLine(text: string | null | undefined): string | null { + if (!text) return null; + const lines = text.split(/\r\n|\r|\n/); + for (let i = lines.length - 1; i >= 0; i--) { + if (lines[i].trim() !== '') return lines[i]; + } + return null; +} + +/** + * Extract a JIRA issue key for THIS project from a PR's branch, title, or the + * last non-empty line of its body. The regex is scoped to the project's + * `projectKey` (case-insensitive), so generic tokens like `UTF-8` / `SHA-256`, + * or another project's keys, never match. Returns the upper-cased key or null. + * JIRA only — Trello/Linear are out of scope for this fallback. + */ +export function extractJiraKeyFromPR( + project: ProjectConfig, + prText: { branch?: string | null; title?: string | null; body?: string | null }, +): string | null { + if (project.pm?.type !== 'jira') return null; + const projectKey = project.jira?.projectKey; + if (!projectKey) return null; + + const keyRegex = new RegExp(`\\b(${escapeRegExp(projectKey)}-\\d+)\\b`, 'i'); + // Priority: branch, then title, then the body's last non-empty line. + const sources = [prText.branch, prText.title, lastNonEmptyLine(prText.body)]; + for (const source of sources) { + const match = source?.match(keyRegex); + if (match) return match[1].toUpperCase(); + } + return null; +} + +/** + * Resolve the work item for a PR, falling back to deriving the JIRA key from the + * PR itself when no `pr_work_items` link exists yet. This lets a review-only + * project (with no implementation agent to write the link) read the linked JIRA + * issue for a human-created PR that references its key. + * + * The derived key is verified against the PM provider before it is returned, so + * a typo'd / non-existent key is not linked (a later DB hit would otherwise + * short-circuit this fallback and make the bad key stick). Requires a PM + * provider in scope (set up by the dispatch path's `withPMProvider`). + */ +export async function resolveWorkItemIdWithFallback( + project: ProjectConfig, + prNumber: number, + prText: { branch?: string | null; title?: string | null; body?: string | null }, +): Promise { + const linked = await resolveWorkItemId(project.id, prNumber); + if (linked) return linked; + + const candidate = extractJiraKeyFromPR(project, prText); + if (!candidate) return undefined; + + const provider = getPMProviderOrNull(); + if (!provider) return undefined; + try { + await provider.getWorkItem(candidate); + return candidate; + } catch (err) { + logger.debug('Derived work item key did not resolve — not linking', { + projectId: project.id, + prNumber, + candidate, + error: String(err), + }); + return undefined; + } +} diff --git a/tests/unit/triggers/check-suite-success.test.ts b/tests/unit/triggers/check-suite-success.test.ts index 60be682f..4575c164 100644 --- a/tests/unit/triggers/check-suite-success.test.ts +++ b/tests/unit/triggers/check-suite-success.test.ts @@ -42,13 +42,23 @@ import { resetFixAttempts } from '../../../src/triggers/github/check-suite-failu import { CheckSuiteSuccessTrigger } from '../../../src/triggers/github/check-suite-success.js'; import { ReviewRequestedTrigger } from '../../../src/triggers/github/review-requested.js'; import type { TriggerContext } from '../../../src/triggers/types.js'; -import { createCheckSuitePayload, createMockProject } from '../../helpers/factories.js'; +import { + createCheckSuitePayload, + createMockJiraProject, + createMockProject, +} from '../../helpers/factories.js'; import { mockPersonaIdentities } from '../../helpers/mockPersonas.js'; vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ lookupWorkItemForPR: vi.fn(), })); +const mockGetPMProviderOrNull = vi.fn(); +vi.mock('../../../src/pm/context.js', async (importOriginal) => ({ + ...(await importOriginal()), + getPMProviderOrNull: () => mockGetPMProviderOrNull(), +})); + import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; import { checkTriggerEnabledWithParams } from '../../../src/triggers/shared/trigger-check.js'; @@ -63,6 +73,7 @@ describe('CheckSuiteSuccessTrigger', () => { beforeEach(() => { vi.mocked(lookupWorkItemForPR).mockResolvedValue('abc123'); + mockGetPMProviderOrNull.mockReset(); mockClaimReviewDispatch.mockReset().mockResolvedValue(true); mockReleaseReviewDispatch.mockReset().mockResolvedValue(undefined); mockClaimRespondToCiDispatch.mockReset().mockResolvedValue(true); @@ -198,6 +209,39 @@ describe('CheckSuiteSuccessTrigger', () => { }); describe('handle', () => { + it('derives the JIRA key from the PR body (last non-empty line) when no DB link exists', async () => { + vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); + mockGetPMProviderOrNull.mockReturnValue({ + getWorkItem: vi.fn().mockResolvedValue({ id: 'PROJ-2068' }), + }); + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'From the user perspective:\r\n- dropping iOS 15\r\n\r\nPROJ-2068', + state: 'closed', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'cascade-impl' }, + }); + vi.mocked(githubClient.getPRReviews).mockResolvedValue([]); + + const ctx: TriggerContext = { + project: createMockJiraProject({ repo: 'owner/repo' }), + source: 'github', + payload: makeCheckSuitePayload(), + personaIdentities: mockPersonaIdentities, + }; + + const result = await trigger.handle(ctx); + + expect(result?.agentType).toBe('review'); + expect(result?.workItemId).toBe('PROJ-2068'); + expect(result?.agentInput).toMatchObject({ workItemId: 'PROJ-2068' }); + }); + it('returns review result without waitForChecks flag when PR matches and aggregate is all-passing', async () => { vi.mocked(githubClient.getPR).mockResolvedValue({ number: 42, diff --git a/tests/unit/triggers/github-utils.test.ts b/tests/unit/triggers/github-utils.test.ts index e9568378..fa1c5721 100644 --- a/tests/unit/triggers/github-utils.test.ts +++ b/tests/unit/triggers/github-utils.test.ts @@ -4,15 +4,22 @@ vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ lookupWorkItemForPR: vi.fn(), })); +const mockGetPMProviderOrNull = vi.fn(); +vi.mock('../../../src/pm/context.js', () => ({ + getPMProviderOrNull: () => mockGetPMProviderOrNull(), +})); + import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; import type { PersonaIdentities } from '../../../src/github/personas.js'; import { evaluateAuthorMode, extractJiraIssueKey, + extractJiraKeyFromPR, extractTrelloCardId, extractWorkItemId, parsePrNumberFromRef, resolveWorkItemId, + resolveWorkItemIdWithFallback, } from '../../../src/triggers/github/utils.js'; import type { ProjectConfig } from '../../../src/types/index.js'; @@ -165,6 +172,103 @@ describe('resolveWorkItemId', () => { }); }); +describe('extractJiraKeyFromPR', () => { + it('returns null for a non-JIRA project', () => { + expect(extractJiraKeyFromPR(mockTrelloProject, { branch: 'TEST-1' })).toBeNull(); + }); + + it('extracts a project-scoped key from the branch (case-insensitive, upper-normalized)', () => { + expect(extractJiraKeyFromPR(mockJiraProject, { branch: 'feature/test-123-fix' })).toBe( + 'TEST-123', + ); + }); + + it('falls back to the title when the branch has no key', () => { + expect( + extractJiraKeyFromPR(mockJiraProject, { branch: 'fix/thing', title: 'TEST-77: do it' }), + ).toBe('TEST-77'); + }); + + it('uses only the last non-empty line of the body', () => { + const body = + 'From the user perspective:\r\n- dropping support for iOS 15 which is ancient anyway\r\n\r\nTEST-2068'; + expect(extractJiraKeyFromPR(mockJiraProject, { body })).toBe('TEST-2068'); + }); + + it('ignores a key that is not on the last non-empty line of the body', () => { + const body = 'TEST-999 mentioned mid-prose\r\nlast line has no key'; + expect(extractJiraKeyFromPR(mockJiraProject, { body })).toBeNull(); + }); + + it('rejects non-project tokens (UTF-8 / another project key)', () => { + expect(extractJiraKeyFromPR(mockJiraProject, { title: 'fix UTF-8 and OTHER-5' })).toBeNull(); + }); + + it('prefers branch over title over body', () => { + expect( + extractJiraKeyFromPR(mockJiraProject, { branch: 'TEST-1', title: 'TEST-2', body: 'TEST-3' }), + ).toBe('TEST-1'); + }); + + it('returns null when no source carries a key', () => { + expect( + extractJiraKeyFromPR(mockJiraProject, { branch: 'fix/x', title: 'no key', body: 'nothing' }), + ).toBeNull(); + }); +}); + +describe('resolveWorkItemIdWithFallback', () => { + beforeEach(() => { + vi.mocked(lookupWorkItemForPR).mockReset(); + mockGetPMProviderOrNull.mockReset(); + }); + + it('returns the DB link when present, without extracting', async () => { + vi.mocked(lookupWorkItemForPR).mockResolvedValue('TEST-500'); + const got = await resolveWorkItemIdWithFallback(mockJiraProject, 42, { branch: 'TEST-1' }); + expect(got).toBe('TEST-500'); + expect(mockGetPMProviderOrNull).not.toHaveBeenCalled(); + }); + + it('derives and verifies the key on a DB miss', async () => { + vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); + const getWorkItem = vi.fn().mockResolvedValue({ id: 'TEST-123' }); + mockGetPMProviderOrNull.mockReturnValue({ getWorkItem }); + const got = await resolveWorkItemIdWithFallback(mockJiraProject, 42, { + branch: 'feature/TEST-123-fix', + }); + expect(got).toBe('TEST-123'); + expect(getWorkItem).toHaveBeenCalledWith('TEST-123'); + }); + + it('does not link when the derived key does not resolve (getWorkItem throws)', async () => { + vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); + const getWorkItem = vi.fn().mockRejectedValue(new Error('404')); + mockGetPMProviderOrNull.mockReturnValue({ getWorkItem }); + const got = await resolveWorkItemIdWithFallback(mockJiraProject, 42, { + body: 'x\r\nTEST-9999', + }); + expect(got).toBeUndefined(); + }); + + it('returns undefined (and skips the provider) when no key is found', async () => { + vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); + const got = await resolveWorkItemIdWithFallback(mockJiraProject, 42, { + branch: 'fix/x', + title: 'no key', + }); + expect(got).toBeUndefined(); + expect(mockGetPMProviderOrNull).not.toHaveBeenCalled(); + }); + + it('returns undefined when no PM provider is in scope', async () => { + vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); + mockGetPMProviderOrNull.mockReturnValue(null); + const got = await resolveWorkItemIdWithFallback(mockJiraProject, 42, { branch: 'TEST-7' }); + expect(got).toBeUndefined(); + }); +}); + describe('parsePrNumberFromRef', () => { it('returns null for null input', () => { expect(parsePrNumberFromRef(null)).toBeNull(); diff --git a/tests/unit/triggers/pr-opened.test.ts b/tests/unit/triggers/pr-opened.test.ts index a7b2127d..7eccc2ee 100644 --- a/tests/unit/triggers/pr-opened.test.ts +++ b/tests/unit/triggers/pr-opened.test.ts @@ -10,12 +10,18 @@ vi.mock('../../../src/triggers/shared/trigger-check.js', () => mockTriggerCheckM import { PROpenedTrigger } from '../../../src/triggers/github/pr-opened.js'; import type { TriggerContext } from '../../../src/triggers/types.js'; -import { createMockProject } from '../../helpers/factories.js'; +import { createMockJiraProject, createMockProject } from '../../helpers/factories.js'; vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ lookupWorkItemForPR: vi.fn(), })); +const mockGetPMProviderOrNull = vi.fn(); +vi.mock('../../../src/pm/context.js', async (importOriginal) => ({ + ...(await importOriginal()), + getPMProviderOrNull: () => mockGetPMProviderOrNull(), +})); + import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; import { checkTriggerEnabledWithParams } from '../../../src/triggers/shared/trigger-check.js'; @@ -27,6 +33,7 @@ describe('PROpenedTrigger', () => { beforeEach(() => { vi.mocked(lookupWorkItemForPR).mockResolvedValue('abc123'); vi.mocked(checkTriggerEnabledWithParams).mockResolvedValue({ enabled: true, parameters: {} }); + mockGetPMProviderOrNull.mockReset(); }); describe('matches', () => { @@ -133,6 +140,45 @@ describe('PROpenedTrigger', () => { }); describe('handle', () => { + it('derives the JIRA key from the PR body (last non-empty line) when no DB link exists', async () => { + vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'all' }, + }); + mockGetPMProviderOrNull.mockReturnValue({ + getWorkItem: vi.fn().mockResolvedValue({ id: 'PROJ-2068' }), + }); + + const ctx: TriggerContext = { + project: createMockJiraProject({ repo: 'owner/repo' }), + source: 'github', + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, + payload: { + action: 'opened', + number: 42, + pull_request: { + number: 42, + title: 'Test PR', + body: 'From the user perspective:\r\n- dropping iOS 15\r\n\r\nPROJ-2068', + html_url: 'https://github.com/owner/repo/pull/42', + state: 'open', + draft: false, + head: { ref: 'feature/test', sha: 'abc' }, + base: { ref: 'main' }, + user: { login: 'author' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'author' }, + }, + }; + + const result = await trigger.handle(ctx); + + expect(result?.workItemId).toBe('PROJ-2068'); + expect(result?.agentInput).toMatchObject({ workItemId: 'PROJ-2068' }); + }); + it('returns result when DB has Trello card linked', async () => { vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ enabled: true, diff --git a/tests/unit/triggers/review-requested.test.ts b/tests/unit/triggers/review-requested.test.ts index 2ddd8f00..49e96c33 100644 --- a/tests/unit/triggers/review-requested.test.ts +++ b/tests/unit/triggers/review-requested.test.ts @@ -17,9 +17,15 @@ vi.mock('../../../src/triggers/github/review-dispatch-dedup.js', () => ({ releaseReviewDispatch: (...args: unknown[]) => mockReleaseReviewDispatch(...args), })); +const mockGetPMProviderOrNull = vi.fn(); +vi.mock('../../../src/pm/context.js', async (importOriginal) => ({ + ...(await importOriginal()), + getPMProviderOrNull: () => mockGetPMProviderOrNull(), +})); + import { ReviewRequestedTrigger } from '../../../src/triggers/github/review-requested.js'; import type { TriggerContext } from '../../../src/triggers/types.js'; -import { createMockProject } from '../../helpers/factories.js'; +import { createMockJiraProject, createMockProject } from '../../helpers/factories.js'; import { mockPersonaIdentities } from '../../helpers/mockPersonas.js'; vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ @@ -39,6 +45,7 @@ describe('ReviewRequestedTrigger', () => { vi.mocked(checkTriggerEnabled).mockResolvedValue(true); mockClaimReviewDispatch.mockReset().mockResolvedValue(true); mockReleaseReviewDispatch.mockReset().mockResolvedValue(undefined); + mockGetPMProviderOrNull.mockReset(); }); const makeReviewRequestedPayload = (reviewerLogin = 'cascade-reviewer') => ({ @@ -203,6 +210,29 @@ describe('ReviewRequestedTrigger', () => { expect(result?.workItemId).toBeUndefined(); }); + it('derives the JIRA key from the PR body (last non-empty line) when no DB link exists', async () => { + vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); + mockGetPMProviderOrNull.mockReturnValue({ + getWorkItem: vi.fn().mockResolvedValue({ id: 'PROJ-2068' }), + }); + const base = makeReviewRequestedPayload(); + const ctx: TriggerContext = { + project: createMockJiraProject(), + source: 'github', + payload: { + ...base, + pull_request: { + ...base.pull_request, + body: 'From the user perspective:\r\n- dropping iOS 15\r\n\r\nPROJ-2068', + }, + }, + personaIdentities: mockPersonaIdentities, + }; + const result = await trigger.handle(ctx); + expect(result?.workItemId).toBe('PROJ-2068'); + expect(result?.agentInput).toMatchObject({ workItemId: 'PROJ-2068' }); + }); + it('triggers review agent when reviewer persona is requested', async () => { const ctx: TriggerContext = { project: mockProject,