diff --git a/src/agents/base.ts b/src/agents/base.ts index f705f7c5..332cbed7 100644 --- a/src/agents/base.ts +++ b/src/agents/base.ts @@ -185,6 +185,8 @@ function createAgentBuilderWithGadgets( llmCallAccumulator?: AccumulatedLlmCall[], runId?: string, baseBranch?: string, + projectId?: string, + cardId?: string, ): BuilderType { return createConfiguredBuilder({ client, @@ -203,6 +205,8 @@ function createAgentBuilderWithGadgets( llmCallAccumulator, runId, baseBranch, + projectId, + cardId, // Implementation agent uses sequential execution to ensure file operations // are properly ordered (e.g., FileSearchAndReplace then ReadFile on same file) postConfigure: @@ -411,6 +415,8 @@ export async function executeAgent( llmCallAccumulator, runId, project.baseBranch, + project.id, + cardId, ), injectSyntheticCalls: ({ builder, ctx, trackingContext, repoDir }) => diff --git a/src/agents/shared/builderFactory.ts b/src/agents/shared/builderFactory.ts index 472a0fd4..fda614e0 100644 --- a/src/agents/shared/builderFactory.ts +++ b/src/agents/shared/builderFactory.ts @@ -44,6 +44,10 @@ export interface CreateBuilderOptions { runId?: string; /** Base branch for PR creation (e.g. 'main', 'dev'). Passed to session state. */ baseBranch?: string; + /** Project ID for PR ↔ work item linking. Passed to session state. */ + projectId?: string; + /** Work item (card) ID for PR ↔ work item linking. Passed to session state. */ + cardId?: string; } const MAX_GADGETS_PER_RESPONSE = 25; @@ -72,7 +76,7 @@ export function createConfiguredBuilder(options: CreateBuilderOptions): BuilderT // Initialize session state for gadgets (e.g., Finish checks PR requirement for implementation) if (!skipSessionState) { - initSessionState(agentType, options.baseBranch); + initSessionState(agentType, options.baseBranch, options.projectId, options.cardId); } let builder = new AgentBuilder(client) diff --git a/src/agents/shared/githubAgent.ts b/src/agents/shared/githubAgent.ts index 7d0715c5..799784c8 100644 --- a/src/agents/shared/githubAgent.ts +++ b/src/agents/shared/githubAgent.ts @@ -166,6 +166,8 @@ export async function executeGitHubAgent< llmCallAccumulator, runId, baseBranch: project.baseBranch, + projectId: project.id, + cardId: input.cardId, ...definition.builderOptions, }), diff --git a/src/db/migrations/0014_pr_work_items.sql b/src/db/migrations/0014_pr_work_items.sql new file mode 100644 index 00000000..4a6d72ad --- /dev/null +++ b/src/db/migrations/0014_pr_work_items.sql @@ -0,0 +1,10 @@ +CREATE TABLE pr_work_items ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + project_id TEXT NOT NULL REFERENCES projects(id) ON DELETE CASCADE, + repo_full_name TEXT NOT NULL, + pr_number INTEGER NOT NULL, + work_item_id TEXT NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + CONSTRAINT uq_pr_work_items_project_pr UNIQUE (project_id, pr_number) +); +CREATE INDEX idx_pr_work_items_work_item ON pr_work_items (work_item_id); diff --git a/src/db/migrations/meta/_journal.json b/src/db/migrations/meta/_journal.json index b1e59311..2fb68a45 100644 --- a/src/db/migrations/meta/_journal.json +++ b/src/db/migrations/meta/_journal.json @@ -92,6 +92,13 @@ "when": 1748000000000, "tag": "0013_integration_model_refactor", "breakpoints": false + }, + { + "idx": 13, + "version": "7", + "when": 1749000000000, + "tag": "0014_pr_work_items", + "breakpoints": false } ] } diff --git a/src/db/repositories/prWorkItemsRepository.ts b/src/db/repositories/prWorkItemsRepository.ts new file mode 100644 index 00000000..53eb533a --- /dev/null +++ b/src/db/repositories/prWorkItemsRepository.ts @@ -0,0 +1,40 @@ +import { and, eq } from 'drizzle-orm'; +import { getDb } from '../client.js'; +import { prWorkItems } from '../schema/index.js'; + +/** + * Upsert a PR ↔ work item link. If a row already exists for the + * (projectId, prNumber) pair, update the work item ID. + */ +export async function linkPRToWorkItem( + projectId: string, + repoFullName: string, + prNumber: number, + workItemId: string, +): Promise { + const db = getDb(); + await db + .insert(prWorkItems) + .values({ projectId, repoFullName, prNumber, workItemId }) + .onConflictDoUpdate({ + target: [prWorkItems.projectId, prWorkItems.prNumber], + set: { workItemId, repoFullName }, + }); +} + +/** + * Look up the work item ID linked to a PR. + * Returns null if no link exists. + */ +export async function lookupWorkItemForPR( + projectId: string, + prNumber: number, +): Promise { + const db = getDb(); + const rows = await db + .select({ workItemId: prWorkItems.workItemId }) + .from(prWorkItems) + .where(and(eq(prWorkItems.projectId, projectId), eq(prWorkItems.prNumber, prNumber))) + .limit(1); + return rows.length > 0 ? rows[0].workItemId : null; +} diff --git a/src/db/schema/index.ts b/src/db/schema/index.ts index 2c22dac5..ab97f84f 100644 --- a/src/db/schema/index.ts +++ b/src/db/schema/index.ts @@ -7,4 +7,5 @@ export { projects } from './projects.js'; export { agentRunLlmCalls, agentRunLogs, agentRuns, debugAnalyses } from './runs.js'; export { promptPartials } from './promptPartials.js'; export { sessions, users } from './users.js'; +export { prWorkItems } from './prWorkItems.js'; export { webhookLogs } from './webhookLogs.js'; diff --git a/src/db/schema/prWorkItems.ts b/src/db/schema/prWorkItems.ts new file mode 100644 index 00000000..c9ff3506 --- /dev/null +++ b/src/db/schema/prWorkItems.ts @@ -0,0 +1,20 @@ +import { index, integer, pgTable, text, timestamp, unique, uuid } from 'drizzle-orm/pg-core'; +import { projects } from './projects.js'; + +export const prWorkItems = pgTable( + 'pr_work_items', + { + id: uuid('id').primaryKey().defaultRandom(), + projectId: text('project_id') + .notNull() + .references(() => projects.id, { onDelete: 'cascade' }), + repoFullName: text('repo_full_name').notNull(), + prNumber: integer('pr_number').notNull(), + workItemId: text('work_item_id').notNull(), + createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(), + }, + (table) => [ + unique('uq_pr_work_items_project_pr').on(table.projectId, table.prNumber), + index('idx_pr_work_items_work_item').on(table.workItemId), + ], +); diff --git a/src/gadgets/github/CreatePR.ts b/src/gadgets/github/CreatePR.ts index 07c75eb2..7b8d0b56 100644 --- a/src/gadgets/github/CreatePR.ts +++ b/src/gadgets/github/CreatePR.ts @@ -1,5 +1,7 @@ import { Gadget, z } from 'llmist'; -import { getBaseBranch, recordPRCreation } from '../sessionState.js'; +import { linkPRToWorkItem } from '../../db/repositories/prWorkItemsRepository.js'; +import { logger } from '../../utils/logging.js'; +import { getBaseBranch, getCardId, getProjectId, recordPRCreation } from '../sessionState.js'; import { createPR } from './core/createPR.js'; export class CreatePR extends Gadget({ @@ -92,6 +94,22 @@ If hooks fail or timeout, the full output will be shown.`, recordPRCreation(result.prUrl); + // Persist PR ↔ work item link (best-effort, don't fail PR creation) + const projectId = getProjectId(); + const cardId = getCardId(); + if (projectId && cardId) { + try { + await linkPRToWorkItem(projectId, result.repoFullName, result.prNumber, cardId); + } catch (err) { + logger.warn('Failed to persist PR-work-item link', { + projectId, + prNumber: result.prNumber, + cardId, + error: String(err), + }); + } + } + if (result.alreadyExisted) { return `PR already exists for this branch: #${result.prNumber} — ${result.prUrl}`; } diff --git a/src/gadgets/github/core/createPR.ts b/src/gadgets/github/core/createPR.ts index 7c04b221..e29edbbf 100644 --- a/src/gadgets/github/core/createPR.ts +++ b/src/gadgets/github/core/createPR.ts @@ -15,6 +15,7 @@ export interface CreatePRParams { export interface CreatePRResult { prNumber: number; prUrl: string; + repoFullName: string; alreadyExisted: boolean; } @@ -108,7 +109,12 @@ export async function createPR(params: CreatePRParams): Promise draft: params.draft, }); - return { prNumber: pr.number, prUrl: pr.htmlUrl, alreadyExisted: false }; + return { + prNumber: pr.number, + prUrl: pr.htmlUrl, + repoFullName: `${owner}/${repo}`, + alreadyExisted: false, + }; } catch (error) { if ( error instanceof Error && @@ -121,6 +127,7 @@ export async function createPR(params: CreatePRParams): Promise return { prNumber: existingPR.number, prUrl: existingPR.htmlUrl, + repoFullName: `${owner}/${repo}`, alreadyExisted: true, }; } diff --git a/src/gadgets/sessionState.ts b/src/gadgets/sessionState.ts index 40856d23..47d520fa 100644 --- a/src/gadgets/sessionState.ts +++ b/src/gadgets/sessionState.ts @@ -2,6 +2,8 @@ let sessionState = { agentType: null as string | null, baseBranch: 'main' as string, + projectId: null as string | null, + cardId: null as string | null, prCreated: false, prUrl: null as string | null, reviewSubmitted: false, @@ -9,10 +11,17 @@ let sessionState = { initialCommentId: null as number | null, }; -export function initSessionState(agentType: string, baseBranch?: string): void { +export function initSessionState( + agentType: string, + baseBranch?: string, + projectId?: string, + cardId?: string, +): void { sessionState = { agentType, baseBranch: baseBranch ?? 'main', + projectId: projectId ?? null, + cardId: cardId ?? null, prCreated: false, prUrl: null, reviewSubmitted: false, @@ -25,6 +34,14 @@ export function getBaseBranch(): string { return sessionState.baseBranch; } +export function getProjectId(): string | null { + return sessionState.projectId; +} + +export function getCardId(): string | null { + return sessionState.cardId; +} + export function recordPRCreation(prUrl: string): void { sessionState.prCreated = true; sessionState.prUrl = prUrl; diff --git a/src/github/client.ts b/src/github/client.ts index d39e81e0..8e03e09a 100644 --- a/src/github/client.ts +++ b/src/github/client.ts @@ -29,6 +29,7 @@ export interface PRDetails { headSha: string; baseRef: string; merged: boolean; + user: { login: string }; } export interface PRReviewComment { @@ -128,6 +129,7 @@ export const githubClient = { headSha: data.head.sha, baseRef: data.base.ref, merged: data.merged ?? false, + user: { login: data.user?.login || 'unknown' }, }; }, diff --git a/src/triggers/github/check-suite-failure.ts b/src/triggers/github/check-suite-failure.ts index 7e34335e..49725f72 100644 --- a/src/triggers/github/check-suite-failure.ts +++ b/src/triggers/github/check-suite-failure.ts @@ -4,7 +4,7 @@ import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/ import { logger } from '../../utils/logging.js'; import { parseRepoFullName } from '../../utils/repo.js'; import { type GitHubCheckSuitePayload, isGitHubCheckSuitePayload } from './types.js'; -import { extractTrelloCardId, hasTrelloCardUrl } from './utils.js'; +import { resolveWorkItemId } from './utils.js'; // Track fix attempts per PR to prevent infinite loops const fixAttempts = new Map(); @@ -17,7 +17,8 @@ export function resetFixAttempts(prNumber: number): void { export class CheckSuiteFailureTrigger implements TriggerHandler { name = 'check-suite-failure'; - description = 'Triggers review agent when check suite fails on a PR with Trello card'; + description = + 'Triggers respond-to-ci agent when check suite fails on a PR by the implementer persona'; matches(ctx: TriggerContext): boolean { if (ctx.source !== 'github') return false; @@ -53,12 +54,16 @@ export class CheckSuiteFailureTrigger implements TriggerHandler { const prNumber = prRef.number; const headSha = payload.check_suite.head_sha; - // Fetch PR to check for Trello card URL + // Fetch PR details const prDetails = await githubClient.getPR(owner, repo, prNumber); - if (!hasTrelloCardUrl(prDetails.body)) { - logger.info('PR does not have Trello card URL, skipping check failure trigger', { + // Gate on PR author being the implementer persona + if (!ctx.personaIdentities) return null; + const implLogin = ctx.personaIdentities.implementer; + if (prDetails.user.login !== implLogin && prDetails.user.login !== `${implLogin}[bot]`) { + logger.info('PR not authored by implementer persona, skipping check failure trigger', { prNumber, + prAuthor: prDetails.user.login, }); return null; } @@ -73,7 +78,13 @@ export class CheckSuiteFailureTrigger implements TriggerHandler { return null; } - const cardId = extractTrelloCardId(prDetails.body); + // Resolve work item from DB (with PR body fallback) + const workItemId = await resolveWorkItemId( + ctx.project.id, + prNumber, + prDetails.body, + ctx.project, + ); // Get ALL check runs for this commit to verify they're all complete const checkStatus = await githubClient.getCheckSuiteStatus(owner, repo, headSha); @@ -126,9 +137,9 @@ export class CheckSuiteFailureTrigger implements TriggerHandler { // Increment attempt counter fixAttempts.set(prNumber, attempts + 1); - logger.info('Check suite failure on PR with Trello card - all checks complete', { + logger.info('Check suite failure on implementer PR - all checks complete', { prNumber, - cardId, + workItemId, attempt: attempts + 1, totalChecks: checkStatus.totalCount, failedChecks: checkStatus.checkRuns @@ -149,10 +160,10 @@ export class CheckSuiteFailureTrigger implements TriggerHandler { repoFullName: payload.repository.full_name, headSha, triggerType: 'check-failure', - cardId: cardId || undefined, + cardId: workItemId, }, prNumber, - workItemId: cardId || undefined, + workItemId, }; } } diff --git a/src/triggers/github/check-suite-success.ts b/src/triggers/github/check-suite-success.ts index 57c2651a..566825ca 100644 --- a/src/triggers/github/check-suite-success.ts +++ b/src/triggers/github/check-suite-success.ts @@ -4,7 +4,7 @@ import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/ import { logger } from '../../utils/logging.js'; import { parseRepoFullName } from '../../utils/repo.js'; import { type GitHubCheckSuitePayload, isGitHubCheckSuitePayload } from './types.js'; -import { extractTrelloCardId, hasTrelloCardUrl } from './utils.js'; +import { resolveWorkItemId } from './utils.js'; const MAX_RETRIES = 5; const RETRY_DELAY_MS = 10_000; @@ -45,19 +45,22 @@ async function waitForChecks( } /** - * Triggers review agent when all CI checks pass on a PR with Trello card URL. + * Triggers review agent when all CI checks pass on a PR authored by the implementer persona. * * This trigger fires when: * 1. A check_suite completes with success conclusion - * 2. The associated PR has a Trello card URL in its body + * 2. The PR author matches the implementer persona (or its [bot] variant) * 3. All checks are actually passing (verified via API) * + * Work item resolution uses the pr_work_items DB table (with PR body extraction as fallback). + * The trigger fires even without a linked work item — agents run, PM updates are simply skipped. + * * Registration order matters - this should be registered BEFORE PRReadyToMergeTrigger * so the review happens before the card is moved to DONE. */ export class CheckSuiteSuccessTrigger implements TriggerHandler { name = 'check-suite-success'; - description = 'Triggers review agent when all CI checks pass on a PR with Trello card'; + description = 'Triggers review agent when all CI checks pass on a PR by the implementer persona'; matches(ctx: TriggerContext): boolean { if (ctx.source !== 'github') return false; @@ -93,12 +96,16 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler { const prNumber = prRef.number; const headSha = payload.check_suite.head_sha; - // Fetch PR to check for Trello card URL + // Fetch PR details const prDetails = await githubClient.getPR(owner, repo, prNumber); - if (!hasTrelloCardUrl(prDetails.body)) { - logger.info('PR does not have Trello card URL, skipping CI success trigger', { + // Gate on PR author being the implementer persona + if (!ctx.personaIdentities) return null; + const implLogin = ctx.personaIdentities.implementer; + if (prDetails.user.login !== implLogin && prDetails.user.login !== `${implLogin}[bot]`) { + logger.info('PR not authored by implementer persona, skipping', { prNumber, + prAuthor: prDetails.user.login, }); return null; } @@ -113,13 +120,19 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler { return null; } - const cardId = extractTrelloCardId(prDetails.body); + // Resolve work item from DB (with PR body fallback) + const workItemId = await resolveWorkItemId( + ctx.project.id, + prNumber, + prDetails.body, + ctx.project, + ); // Skip if the reviewer persona's latest review already covers the current HEAD SHA const reviews = await githubClient.getPRReviews(owner, repo, prNumber); // Use persona identities to identify reviewer bot's reviews - const reviewerUsername = ctx.personaIdentities?.reviewer; + const reviewerUsername = ctx.personaIdentities.reviewer; // Only consider actual reviews (approved/changes_requested), not COMMENTED // which are reply acknowledgments posted by respond-to-review agent @@ -164,9 +177,9 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler { return null; } - logger.info('All CI checks passed on PR with Trello card - triggering review', { + logger.info('All CI checks passed on implementer PR - triggering review', { prNumber, - cardId, + workItemId, headSha, totalChecks: checkStatus.totalCount, }); @@ -179,10 +192,10 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler { repoFullName: payload.repository.full_name, headSha, triggerType: 'ci-success', - cardId: cardId || undefined, + cardId: workItemId, }, prNumber, - workItemId: cardId || undefined, + workItemId, }; } } diff --git a/src/triggers/github/pr-comment-mention.ts b/src/triggers/github/pr-comment-mention.ts index 4111ddfb..cb20db7b 100644 --- a/src/triggers/github/pr-comment-mention.ts +++ b/src/triggers/github/pr-comment-mention.ts @@ -5,7 +5,7 @@ import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/ import { logger } from '../../utils/logging.js'; import { parseRepoFullName } from '../../utils/repo.js'; import { isGitHubIssueCommentPayload, isGitHubPRReviewCommentPayload } from './types.js'; -import { requireTrelloCardId } from './utils.js'; +import { resolveWorkItemId } from './utils.js'; /** * Trigger that fires when someone @mentions the reviewer bot in a PR comment. @@ -90,7 +90,7 @@ export class PRCommentMentionTrigger implements TriggerHandler { prBranch = payload.pull_request.head.ref; repoFullName = payload.repository.full_name; - // Fetch PR for body (needed for Trello card check) + // Fetch PR for body (needed for work item resolution) const { owner, repo } = parseRepoFullName(repoFullName); const prDetails = await githubClient.getPR(owner, repo, prNumber); prBody = prDetails.body; @@ -110,18 +110,14 @@ export class PRCommentMentionTrigger implements TriggerHandler { return null; } - // Require Trello card - const cardId = requireTrelloCardId(prBody, { - prNumber, - triggerName: 'PR comment mention trigger', - }); - if (cardId === null) return null; + // Resolve work item from DB (with PR body fallback) + const workItemId = await resolveWorkItemId(ctx.project.id, prNumber, prBody, ctx.project); logger.info('PR comment @mention detected, triggering respond-to-pr-comment agent', { prNumber, commentAuthor, mentionTarget, - cardId, + workItemId, }); return { @@ -137,7 +133,7 @@ export class PRCommentMentionTrigger implements TriggerHandler { commentAuthor, }, prNumber, - workItemId: cardId || undefined, + workItemId, }; } } diff --git a/src/triggers/github/pr-merged.ts b/src/triggers/github/pr-merged.ts index 3352b37d..63ed247d 100644 --- a/src/triggers/github/pr-merged.ts +++ b/src/triggers/github/pr-merged.ts @@ -6,7 +6,7 @@ import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/ import { logger } from '../../utils/logging.js'; import { parseRepoFullName } from '../../utils/repo.js'; import { type GitHubPullRequestPayload, isGitHubPullRequestPayload } from './types.js'; -import { requireWorkItemId } from './utils.js'; +import { resolveWorkItemId } from './utils.js'; export class PRMergedTrigger implements TriggerHandler { name = 'pr-merged'; @@ -41,13 +41,13 @@ export class PRMergedTrigger implements TriggerHandler { return null; } - // Extract work item ID from PR body (works for both Trello and JIRA) + // Resolve work item from DB (with PR body fallback) const prBody = payload.pull_request.body || ''; - const workItemId = requireWorkItemId(prBody, ctx.project, { - prNumber, - triggerName: 'pr-merged', - }); - if (!workItemId) return null; + const workItemId = await resolveWorkItemId(ctx.project.id, prNumber, prBody, ctx.project); + if (!workItemId) { + logger.info('No work item linked to PR, skipping pr-merged', { prNumber }); + return null; + } const pmConfig = resolveProjectPMConfig(ctx.project); const mergedStatus = pmConfig.statuses.merged; diff --git a/src/triggers/github/pr-opened.ts b/src/triggers/github/pr-opened.ts index 4e1205e2..fe315449 100644 --- a/src/triggers/github/pr-opened.ts +++ b/src/triggers/github/pr-opened.ts @@ -2,11 +2,11 @@ import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; import { isGitHubPullRequestPayload } from './types.js'; -import { extractTrelloCardId, hasTrelloCardUrl } from './utils.js'; +import { resolveWorkItemId } from './utils.js'; /** * Trigger that fires the review agent when a new PR is opened. - * Only triggers for PRs that have a Trello card URL in the body. + * Resolves work item from DB (with PR body fallback); fires even without a linked work item. */ export class PROpenedTrigger implements TriggerHandler { name = 'pr-opened'; @@ -49,21 +49,13 @@ export class PROpenedTrigger implements TriggerHandler { const prNumber = payload.pull_request.number; const prBody = payload.pull_request.body || ''; - // Require Trello card URL in PR body (consistent with other triggers) - if (!hasTrelloCardUrl(prBody)) { - logger.info('PR does not have Trello card URL, skipping PR opened trigger', { - prNumber, - prTitle: payload.pull_request.title, - }); - return null; - } - - const cardId = extractTrelloCardId(prBody); + // Resolve work item from DB (with PR body fallback) + const workItemId = await resolveWorkItemId(ctx.project.id, prNumber, prBody, ctx.project); - logger.info('New PR opened with Trello card, triggering review agent', { + logger.info('New PR opened, triggering review agent', { prNumber, prTitle: payload.pull_request.title, - cardId, + workItemId, }); return { @@ -79,7 +71,7 @@ export class PROpenedTrigger implements TriggerHandler { triggerCommentUrl: payload.pull_request.html_url, }, prNumber, - workItemId: cardId || undefined, + workItemId, }; } } diff --git a/src/triggers/github/pr-ready-to-merge.ts b/src/triggers/github/pr-ready-to-merge.ts index 7ddc2485..78bd5b84 100644 --- a/src/triggers/github/pr-ready-to-merge.ts +++ b/src/triggers/github/pr-ready-to-merge.ts @@ -11,7 +11,7 @@ import { isGitHubCheckSuitePayload, isGitHubPullRequestReviewPayload, } from './types.js'; -import { requireWorkItemId } from './utils.js'; +import { resolveWorkItemId } from './utils.js'; export class PRReadyToMergeTrigger implements TriggerHandler { name = 'pr-ready-to-merge'; @@ -80,12 +80,12 @@ export class PRReadyToMergeTrigger implements TriggerHandler { const { owner, repo } = parseRepoFullName(repoFullName); - // Must have work item reference in PR body - const workItemId = requireWorkItemId(prBody, ctx.project, { - prNumber, - triggerName: 'pr-ready-to-merge', - }); - if (!workItemId) return null; + // Resolve work item from DB (with PR body fallback) + const workItemId = await resolveWorkItemId(ctx.project.id, prNumber, prBody, ctx.project); + if (!workItemId) { + logger.info('No work item linked to PR, skipping pr-ready-to-merge', { prNumber }); + return null; + } // Check 1: All checks must pass const checkStatus = await githubClient.getCheckSuiteStatus(owner, repo, headSha); diff --git a/src/triggers/github/pr-review-submitted.ts b/src/triggers/github/pr-review-submitted.ts index a8946b86..0ad21712 100644 --- a/src/triggers/github/pr-review-submitted.ts +++ b/src/triggers/github/pr-review-submitted.ts @@ -3,7 +3,7 @@ import { getPersonaForLogin } from '../../github/personas.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; import { isGitHubPullRequestReviewPayload } from './types.js'; -import { requireTrelloCardId } from './utils.js'; +import { resolveWorkItemId } from './utils.js'; export class PRReviewSubmittedTrigger implements TriggerHandler { name = 'pr-review-submitted'; @@ -64,18 +64,14 @@ export class PRReviewSubmittedTrigger implements TriggerHandler { return null; } - // Check if PR has Trello card URL in body + // Resolve work item from DB (with PR body fallback) const prBody = reviewPayload.pull_request.body || ''; - const cardId = requireTrelloCardId(prBody, { - prNumber, - triggerName: 'review submission trigger', - }); - if (cardId === null) return null; + const workItemId = await resolveWorkItemId(ctx.project.id, prNumber, prBody, ctx.project); logger.info('PR review submitted, triggering review agent', { prNumber, reviewState: reviewPayload.review.state, - cardId, + workItemId, }); return { @@ -90,7 +86,7 @@ export class PRReviewSubmittedTrigger implements TriggerHandler { triggerCommentUrl: reviewPayload.review.html_url, }, prNumber, - workItemId: cardId || undefined, + workItemId, }; } } diff --git a/src/triggers/github/review-requested.ts b/src/triggers/github/review-requested.ts index 55326295..14d4524c 100644 --- a/src/triggers/github/review-requested.ts +++ b/src/triggers/github/review-requested.ts @@ -3,7 +3,7 @@ import { isCascadeBot } from '../../github/personas.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; import { type GitHubPullRequestPayload, isGitHubPullRequestPayload } from './types.js'; -import { extractWorkItemId } from './utils.js'; +import { resolveWorkItemId } from './utils.js'; /** * Trigger that fires the review agent when review is requested from a CASCADE persona account. @@ -71,15 +71,9 @@ export class ReviewRequestedTrigger implements TriggerHandler { return null; } + // Resolve work item from DB (with PR body fallback) const prBody = payload.pull_request.body; - const workItemId = extractWorkItemId(prBody, ctx.project); - - if (!workItemId) { - logger.info('PR does not have work item reference, skipping review-requested trigger', { - prNumber, - }); - return null; - } + const workItemId = await resolveWorkItemId(ctx.project.id, prNumber, prBody, ctx.project); logger.info('Review requested from CASCADE persona, triggering review agent', { prNumber, diff --git a/src/triggers/github/utils.ts b/src/triggers/github/utils.ts index 0d5488d4..61d2c9a5 100644 --- a/src/triggers/github/utils.ts +++ b/src/triggers/github/utils.ts @@ -1,3 +1,4 @@ +import { lookupWorkItemForPR } from '../../db/repositories/prWorkItemsRepository.js'; import type { ProjectConfig } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; @@ -91,3 +92,27 @@ export function requireWorkItemId( } return id; } + +/** + * Resolve work item ID for a PR. Primary source: DB lookup (pr_work_items table). + * Fallback: extract from PR body (backward compat for pre-migration PRs). + */ +export async function resolveWorkItemId( + projectId: string, + prNumber: number, + prBody: string | null, + project: ProjectConfig, +): Promise { + try { + const dbResult = await lookupWorkItemForPR(projectId, prNumber); + if (dbResult) return dbResult; + } catch (err) { + logger.warn('Failed to look up work item from DB, falling back to PR body', { + projectId, + prNumber, + error: String(err), + }); + } + + return extractWorkItemId(prBody, project) ?? undefined; +} diff --git a/tests/unit/db/repositories/prWorkItemsRepository.test.ts b/tests/unit/db/repositories/prWorkItemsRepository.test.ts new file mode 100644 index 00000000..46c5419c --- /dev/null +++ b/tests/unit/db/repositories/prWorkItemsRepository.test.ts @@ -0,0 +1,125 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('../../../../src/db/client.js', () => ({ + getDb: vi.fn(), +})); + +vi.mock('../../../../src/db/schema/index.js', () => ({ + prWorkItems: { + projectId: 'projectId', + prNumber: 'prNumber', + workItemId: 'workItemId', + repoFullName: 'repoFullName', + }, +})); + +import { getDb } from '../../../../src/db/client.js'; +import { + linkPRToWorkItem, + lookupWorkItemForPR, +} from '../../../../src/db/repositories/prWorkItemsRepository.js'; + +function createMockDb() { + const chain: Record> = {}; + + chain.limit = vi.fn().mockResolvedValue([]); + chain.where = vi.fn().mockReturnValue({ limit: chain.limit }); + chain.from = vi.fn().mockReturnValue({ where: chain.where }); + + chain.onConflictDoUpdate = vi.fn().mockResolvedValue(undefined); + chain.values = vi.fn().mockReturnValue({ + onConflictDoUpdate: chain.onConflictDoUpdate, + }); + + const db = { + select: vi.fn().mockReturnValue({ from: chain.from }), + insert: vi.fn().mockReturnValue({ values: chain.values }), + }; + + return { db, chain }; +} + +describe('prWorkItemsRepository', () => { + let mockDb: ReturnType; + + beforeEach(() => { + mockDb = createMockDb(); + vi.mocked(getDb).mockReturnValue(mockDb.db as never); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + // ========================================================================== + // linkPRToWorkItem + // ========================================================================== + + describe('linkPRToWorkItem', () => { + it('inserts a PR-to-work-item link with correct values', async () => { + await linkPRToWorkItem('proj-1', 'owner/repo', 42, 'wi-abc'); + + expect(mockDb.db.insert).toHaveBeenCalledTimes(1); + expect(mockDb.chain.values).toHaveBeenCalledWith({ + projectId: 'proj-1', + repoFullName: 'owner/repo', + prNumber: 42, + workItemId: 'wi-abc', + }); + }); + + it('calls onConflictDoUpdate for upsert behavior', async () => { + await linkPRToWorkItem('proj-1', 'owner/repo', 42, 'wi-abc'); + + expect(mockDb.chain.onConflictDoUpdate).toHaveBeenCalledTimes(1); + expect(mockDb.chain.onConflictDoUpdate).toHaveBeenCalledWith({ + target: expect.arrayContaining([expect.anything(), expect.anything()]), + set: { workItemId: 'wi-abc', repoFullName: 'owner/repo' }, + }); + }); + + it('updates workItemId and repoFullName on conflict', async () => { + await linkPRToWorkItem('proj-1', 'new-owner/repo', 99, 'wi-new'); + + const conflictArg = mockDb.chain.onConflictDoUpdate.mock.calls[0][0]; + expect(conflictArg.set).toEqual({ + workItemId: 'wi-new', + repoFullName: 'new-owner/repo', + }); + }); + }); + + // ========================================================================== + // lookupWorkItemForPR + // ========================================================================== + + describe('lookupWorkItemForPR', () => { + it('returns workItemId when a matching row is found', async () => { + mockDb.chain.limit.mockResolvedValueOnce([{ workItemId: 'wi-found' }]); + + const result = await lookupWorkItemForPR('proj-1', 42); + + expect(result).toBe('wi-found'); + expect(mockDb.db.select).toHaveBeenCalledTimes(1); + }); + + it('returns null when no matching row is found', async () => { + mockDb.chain.limit.mockResolvedValueOnce([]); + + const result = await lookupWorkItemForPR('proj-1', 999); + + expect(result).toBeNull(); + }); + + it('queries with correct project and PR number', async () => { + mockDb.chain.limit.mockResolvedValueOnce([]); + + await lookupWorkItemForPR('proj-2', 77); + + expect(mockDb.db.select).toHaveBeenCalledTimes(1); + expect(mockDb.chain.from).toHaveBeenCalledTimes(1); + expect(mockDb.chain.where).toHaveBeenCalledTimes(1); + expect(mockDb.chain.limit).toHaveBeenCalledWith(1); + }); + }); +}); diff --git a/tests/unit/gadgets/github.test.ts b/tests/unit/gadgets/github.test.ts index ea2be430..3f7d5f98 100644 --- a/tests/unit/gadgets/github.test.ts +++ b/tests/unit/gadgets/github.test.ts @@ -7,6 +7,18 @@ import { runCommand } from '../../../src/utils/repo.js'; vi.mock('../../../src/gadgets/sessionState.js', () => ({ recordPRCreation: vi.fn(), getBaseBranch: vi.fn().mockReturnValue('main'), + getProjectId: vi.fn().mockReturnValue(null), + getCardId: vi.fn().mockReturnValue(null), +})); + +// Mock DB repository (CreatePR gadget calls linkPRToWorkItem) +vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ + linkPRToWorkItem: vi.fn(), +})); + +// Mock logger +vi.mock('../../../src/utils/logging.js', () => ({ + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, })); // Mock the github client diff --git a/tests/unit/gadgets/github/core/createPR.test.ts b/tests/unit/gadgets/github/core/createPR.test.ts index dc490330..b0d331a4 100644 --- a/tests/unit/gadgets/github/core/createPR.test.ts +++ b/tests/unit/gadgets/github/core/createPR.test.ts @@ -448,6 +448,7 @@ describe('createPR', () => { expect(result).toEqual({ prNumber: 42, prUrl: 'https://github.com/test-owner/test-repo/pull/42', + repoFullName: 'test-owner/test-repo', alreadyExisted: false, }); }); @@ -476,6 +477,7 @@ describe('createPR', () => { expect(result).toEqual({ prNumber: 10, prUrl: 'https://github.com/test-owner/test-repo/pull/10', + repoFullName: 'test-owner/test-repo', alreadyExisted: true, }); }); diff --git a/tests/unit/github/client.test.ts b/tests/unit/github/client.test.ts index eb3640bd..ad922a4b 100644 --- a/tests/unit/github/client.test.ts +++ b/tests/unit/github/client.test.ts @@ -95,6 +95,7 @@ describe('githubClient', () => { head: { ref: 'feature/test', sha: 'sha123' }, base: { ref: 'main' }, merged: false, + user: { login: 'test-user' }, }, }); @@ -112,6 +113,7 @@ describe('githubClient', () => { headSha: 'sha123', baseRef: 'main', merged: false, + user: { login: 'test-user' }, }); expect(mockPulls.get).toHaveBeenCalledWith({ owner: 'owner', @@ -120,7 +122,7 @@ describe('githubClient', () => { }); }); - it('handles null merged field', async () => { + it('handles null merged field and missing user', async () => { mockPulls.get.mockResolvedValue({ data: { number: 42, @@ -131,6 +133,7 @@ describe('githubClient', () => { head: { ref: 'feat', sha: 'abc' }, base: { ref: 'main' }, merged: null, + user: null, }, }); @@ -139,6 +142,7 @@ describe('githubClient', () => { ); expect(result.merged).toBe(false); + expect(result.user).toEqual({ login: 'unknown' }); }); }); diff --git a/tests/unit/triggers/check-suite-failure.test.ts b/tests/unit/triggers/check-suite-failure.test.ts index c0cd0ba5..90c686ba 100644 --- a/tests/unit/triggers/check-suite-failure.test.ts +++ b/tests/unit/triggers/check-suite-failure.test.ts @@ -15,6 +15,11 @@ vi.mock('../../../src/github/client.js', () => ({ import { githubClient } from '../../../src/github/client.js'; +vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ + lookupWorkItemForPR: vi.fn(), +})); +import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; + describe('CheckSuiteFailureTrigger', () => { const trigger = new CheckSuiteFailureTrigger(); @@ -52,6 +57,7 @@ describe('CheckSuiteFailureTrigger', () => { beforeEach(() => { vi.clearAllMocks(); resetFixAttempts(42); + vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); }); describe('resolveAgentType', () => { @@ -150,6 +156,7 @@ describe('CheckSuiteFailureTrigger', () => { headSha: 'sha123', baseRef: 'main', merged: false, + user: { login: 'cascade-impl' }, }); vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ allPassing: false, @@ -164,6 +171,7 @@ describe('CheckSuiteFailureTrigger', () => { project: mockProject, source: 'github', payload: makeFailurePayload(), + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-reviewer' }, }; const result = await trigger.handle(ctx); @@ -193,12 +201,14 @@ describe('CheckSuiteFailureTrigger', () => { headSha: 'sha123', baseRef: 'develop', merged: false, + user: { login: 'cascade-impl' }, }); const ctx: TriggerContext = { project: mockProject, source: 'github', payload: makeFailurePayload(), + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-reviewer' }, }; const result = await trigger.handle(ctx); @@ -207,16 +217,42 @@ describe('CheckSuiteFailureTrigger', () => { expect(githubClient.getCheckSuiteStatus).not.toHaveBeenCalled(); }); - it('returns null when PR has no Trello URL', async () => { + it('returns null when PR not authored by implementer persona', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123/card-name', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + user: { login: 'some-human' }, + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeFailurePayload(), + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-reviewer' }, + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + + it('returns null when no personaIdentities available', async () => { vi.mocked(githubClient.getPR).mockResolvedValue({ number: 42, title: 'Test PR', - body: 'No Trello link', + body: 'https://trello.com/c/abc123/card-name', state: 'open', headRef: 'feature/test', headSha: 'sha123', baseRef: 'main', merged: false, + user: { login: 'cascade-impl' }, }); const ctx: TriggerContext = { @@ -230,6 +266,38 @@ describe('CheckSuiteFailureTrigger', () => { expect(result).toBeNull(); }); + it('fires without work item when PR body has no reference', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'No work item link', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + user: { login: 'cascade-impl' }, + }); + vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ + allPassing: false, + totalCount: 1, + checkRuns: [{ name: 'test', status: 'completed', conclusion: 'failure' }], + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeFailurePayload(), + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-reviewer' }, + }; + + const result = await trigger.handle(ctx); + + expect(result).not.toBeNull(); + expect(result?.workItemId).toBeUndefined(); + expect(result?.agentInput.cardId).toBeUndefined(); + }); + it('returns null when not all checks are complete', async () => { vi.mocked(githubClient.getPR).mockResolvedValue({ number: 42, @@ -240,6 +308,7 @@ describe('CheckSuiteFailureTrigger', () => { headSha: 'sha123', baseRef: 'main', merged: false, + user: { login: 'cascade-impl' }, }); vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ allPassing: false, @@ -254,6 +323,7 @@ describe('CheckSuiteFailureTrigger', () => { project: mockProject, source: 'github', payload: makeFailurePayload(), + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-reviewer' }, }; const result = await trigger.handle(ctx); @@ -271,6 +341,7 @@ describe('CheckSuiteFailureTrigger', () => { headSha: 'sha123', baseRef: 'main', merged: false, + user: { login: 'cascade-impl' }, }); vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ allPassing: true, @@ -285,6 +356,7 @@ describe('CheckSuiteFailureTrigger', () => { project: mockProject, source: 'github', payload: makeFailurePayload(), + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-reviewer' }, }; const result = await trigger.handle(ctx); @@ -302,6 +374,7 @@ describe('CheckSuiteFailureTrigger', () => { headSha: 'sha123', baseRef: 'main', merged: false, + user: { login: 'cascade-impl' }, }); vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ allPassing: false, @@ -313,6 +386,7 @@ describe('CheckSuiteFailureTrigger', () => { project: mockProject, source: 'github', payload: makeFailurePayload(), + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-reviewer' }, }; // First 3 attempts should succeed @@ -342,6 +416,7 @@ describe('CheckSuiteFailureTrigger', () => { headSha: 'sha123', baseRef: 'main', merged: false, + user: { login: 'cascade-impl' }, }); vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ allPassing: false, @@ -353,6 +428,7 @@ describe('CheckSuiteFailureTrigger', () => { project: mockProject, source: 'github', payload: makeFailurePayload(), + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-reviewer' }, }; // Use up 3 attempts diff --git a/tests/unit/triggers/check-suite-success.test.ts b/tests/unit/triggers/check-suite-success.test.ts index bd8388ac..02ec10d5 100644 --- a/tests/unit/triggers/check-suite-success.test.ts +++ b/tests/unit/triggers/check-suite-success.test.ts @@ -12,6 +12,12 @@ vi.mock('../../../src/github/client.js', () => ({ import { githubClient } from '../../../src/github/client.js'; +vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ + lookupWorkItemForPR: vi.fn(), +})); + +import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; + describe('CheckSuiteSuccessTrigger', () => { const trigger = new CheckSuiteSuccessTrigger(); @@ -53,6 +59,7 @@ describe('CheckSuiteSuccessTrigger', () => { beforeEach(() => { vi.clearAllMocks(); + vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); }); describe('resolveAgentType', () => { @@ -154,6 +161,7 @@ describe('CheckSuiteSuccessTrigger', () => { baseRef: 'main', merged: false, htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'cascade-impl' }, }); vi.mocked(githubClient.getPRReviews).mockResolvedValue([]); vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ @@ -202,6 +210,7 @@ describe('CheckSuiteSuccessTrigger', () => { baseRef: 'develop', merged: false, htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'cascade-impl' }, }); const ctx: TriggerContext = { @@ -218,17 +227,18 @@ describe('CheckSuiteSuccessTrigger', () => { expect(githubClient.getCheckSuiteStatus).not.toHaveBeenCalled(); }); - it('returns null when PR has no Trello URL', async () => { + it('returns null when PR not authored by implementer persona', async () => { vi.mocked(githubClient.getPR).mockResolvedValue({ number: 42, title: 'Test PR', - body: 'No Trello link here', + body: 'https://trello.com/c/abc123/card-name', state: 'open', headRef: 'feature/test', headSha: 'sha123', baseRef: 'main', merged: false, htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'some-human' }, }); const ctx: TriggerContext = { @@ -244,6 +254,32 @@ describe('CheckSuiteSuccessTrigger', () => { expect(githubClient.getCheckSuiteStatus).not.toHaveBeenCalled(); }); + it('returns null when no personaIdentities available', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123/card-name', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'cascade-impl' }, + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeCheckSuitePayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + expect(githubClient.getCheckSuiteStatus).not.toHaveBeenCalled(); + }); + it('returns null immediately when checks have genuine failures (no retry)', async () => { vi.mocked(githubClient.getPR).mockResolvedValue({ number: 42, @@ -255,6 +291,7 @@ describe('CheckSuiteSuccessTrigger', () => { baseRef: 'main', merged: false, htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'cascade-impl' }, }); vi.mocked(githubClient.getPRReviews).mockResolvedValue([]); vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ @@ -293,6 +330,7 @@ describe('CheckSuiteSuccessTrigger', () => { baseRef: 'main', merged: false, htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'cascade-impl' }, }); vi.mocked(githubClient.getPRReviews).mockResolvedValue([]); vi.mocked(githubClient.getCheckSuiteStatus) @@ -347,6 +385,7 @@ describe('CheckSuiteSuccessTrigger', () => { baseRef: 'main', merged: false, htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'cascade-impl' }, }); vi.mocked(githubClient.getPRReviews).mockResolvedValue([]); vi.mocked(githubClient.getCheckSuiteStatus) @@ -399,6 +438,7 @@ describe('CheckSuiteSuccessTrigger', () => { baseRef: 'main', merged: false, htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'cascade-impl' }, }); vi.mocked(githubClient.getPRReviews).mockResolvedValue([ { @@ -435,6 +475,7 @@ describe('CheckSuiteSuccessTrigger', () => { baseRef: 'main', merged: false, htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'cascade-impl' }, }); vi.mocked(githubClient.getPRReviews).mockResolvedValue([ { @@ -477,6 +518,7 @@ describe('CheckSuiteSuccessTrigger', () => { baseRef: 'main', merged: false, htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'cascade-impl' }, }); vi.mocked(githubClient.getPRReviews).mockResolvedValue([ { @@ -521,6 +563,7 @@ describe('CheckSuiteSuccessTrigger', () => { baseRef: 'main', merged: false, htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'cascade-impl' }, }); vi.mocked(githubClient.getPRReviews).mockResolvedValue([ { @@ -579,6 +622,7 @@ describe('CheckSuiteSuccessTrigger', () => { baseRef: 'main', merged: false, htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'cascade-impl' }, }); vi.mocked(githubClient.getPRReviews).mockResolvedValue([ { @@ -608,5 +652,73 @@ describe('CheckSuiteSuccessTrigger', () => { expect(result).not.toBeNull(); expect(result?.agentType).toBe('review'); }); + + it('fires without work item when PR body has no work item reference', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'No work item link', + state: 'open', + 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([]); + vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ + allPassing: true, + totalCount: 1, + checkRuns: [{ name: 'test', status: 'completed', conclusion: 'success' }], + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeCheckSuitePayload(), + personaIdentities: mockPersonaIdentities, + }; + + const result = await trigger.handle(ctx); + + expect(result).not.toBeNull(); + expect(result?.workItemId).toBeUndefined(); + expect(result?.agentInput.cardId).toBeUndefined(); + }); + + it('uses DB lookup result over PR body extraction', async () => { + vi.mocked(lookupWorkItemForPR).mockResolvedValue('db-work-item'); + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123', + state: 'open', + 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([]); + vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ + allPassing: true, + totalCount: 1, + checkRuns: [{ name: 'test', status: 'completed', conclusion: 'success' }], + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeCheckSuitePayload(), + personaIdentities: mockPersonaIdentities, + }; + + const result = await trigger.handle(ctx); + + expect(result).not.toBeNull(); + expect(result?.workItemId).toBe('db-work-item'); + }); }); }); diff --git a/tests/unit/triggers/github-pr-comment-mention.test.ts b/tests/unit/triggers/github-pr-comment-mention.test.ts index bdaedb79..d04dd523 100644 --- a/tests/unit/triggers/github-pr-comment-mention.test.ts +++ b/tests/unit/triggers/github-pr-comment-mention.test.ts @@ -25,6 +25,11 @@ vi.mock('../../../src/utils/logging.js', () => ({ }, })); +vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ + lookupWorkItemForPR: vi.fn(), +})); + +import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; import { PRCommentMentionTrigger } from '../../../src/triggers/github/pr-comment-mention.js'; import type { TriggerContext } from '../../../src/triggers/types.js'; @@ -150,6 +155,7 @@ describe('PRCommentMentionTrigger', () => { vi.resetAllMocks(); trigger = new PRCommentMentionTrigger(); mockIsCascadeBot.mockReturnValue(false); + vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); mockGetPR.mockResolvedValue({ headRef: 'feature/test', body: PR_BODY_WITH_CARD, @@ -241,7 +247,7 @@ describe('PRCommentMentionTrigger', () => { expect(result).toBeNull(); }); - it('returns null when PR body has no Trello card link', async () => { + it('fires with workItemId undefined when PR body has no Trello card link', async () => { mockGetPR.mockResolvedValue({ headRef: 'feature/test', body: PR_BODY_NO_CARD, @@ -249,7 +255,9 @@ describe('PRCommentMentionTrigger', () => { const result = await trigger.handle(buildCtx()); - expect(result).toBeNull(); + expect(result).not.toBeNull(); + expect(result?.agentType).toBe('respond-to-pr-comment'); + expect(result?.workItemId).toBeUndefined(); }); it('fetches PR details to get branch info', async () => { diff --git a/tests/unit/triggers/github-utils.test.ts b/tests/unit/triggers/github-utils.test.ts index 5d46b4b3..bf63adad 100644 --- a/tests/unit/triggers/github-utils.test.ts +++ b/tests/unit/triggers/github-utils.test.ts @@ -1,10 +1,17 @@ -import { describe, expect, it } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ + lookupWorkItemForPR: vi.fn(), +})); + +import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; import { extractJiraIssueKey, extractTrelloCardId, extractWorkItemId, hasTrelloCardUrl, requireWorkItemId, + resolveWorkItemId, } from '../../../src/triggers/github/utils.js'; import type { ProjectConfig } from '../../../src/types/index.js'; @@ -171,3 +178,66 @@ describe('requireWorkItemId', () => { expect(result).toBeNull(); }); }); + +describe('resolveWorkItemId', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); + }); + + it('returns DB result when available', async () => { + vi.mocked(lookupWorkItemForPR).mockResolvedValue('db-item-123'); + + const result = await resolveWorkItemId( + 'proj', + 42, + 'https://trello.com/c/abc123', + mockTrelloProject, + ); + + expect(result).toBe('db-item-123'); + expect(lookupWorkItemForPR).toHaveBeenCalledWith('proj', 42); + }); + + it('falls back to PR body extraction when DB returns null', async () => { + const result = await resolveWorkItemId( + 'proj', + 42, + 'https://trello.com/c/abc123', + mockTrelloProject, + ); + + expect(result).toBe('abc123'); + }); + + it('falls back to JIRA extraction for JIRA projects', async () => { + const result = await resolveWorkItemId('proj', 42, 'Fixes PROJ-456', mockJiraProject); + + expect(result).toBe('PROJ-456'); + }); + + it('returns undefined when neither DB nor body has work item', async () => { + const result = await resolveWorkItemId('proj', 42, 'No work item here', mockTrelloProject); + + expect(result).toBeUndefined(); + }); + + it('returns undefined for null body with no DB result', async () => { + const result = await resolveWorkItemId('proj', 42, null, mockTrelloProject); + + expect(result).toBeUndefined(); + }); + + it('falls back to body extraction when DB throws', async () => { + vi.mocked(lookupWorkItemForPR).mockRejectedValue(new Error('DB connection failed')); + + const result = await resolveWorkItemId( + 'proj', + 42, + 'https://trello.com/c/abc123', + mockTrelloProject, + ); + + expect(result).toBe('abc123'); + }); +}); diff --git a/tests/unit/triggers/pr-merged.test.ts b/tests/unit/triggers/pr-merged.test.ts index 9fb198c5..1fa0ff82 100644 --- a/tests/unit/triggers/pr-merged.test.ts +++ b/tests/unit/triggers/pr-merged.test.ts @@ -43,6 +43,9 @@ vi.mock('../../../src/router/acknowledgments.js', () => ({ vi.mock('../../../src/router/reactions.js', () => ({ sendAcknowledgeReaction: vi.fn(), })); +vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ + lookupWorkItemForPR: vi.fn(), +})); // Register PM integrations in the registry import '../../../src/pm/index.js'; @@ -50,6 +53,7 @@ import '../../../src/pm/index.js'; import { PRMergedTrigger } from '../../../src/triggers/github/pr-merged.js'; import type { TriggerContext } from '../../../src/triggers/types.js'; +import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; import { githubClient } from '../../../src/github/client.js'; describe('PRMergedTrigger', () => { @@ -75,6 +79,7 @@ describe('PRMergedTrigger', () => { beforeEach(() => { vi.clearAllMocks(); + vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); }); describe('resolveAgentType', () => { diff --git a/tests/unit/triggers/pr-opened.test.ts b/tests/unit/triggers/pr-opened.test.ts index c2b8027c..b44471cc 100644 --- a/tests/unit/triggers/pr-opened.test.ts +++ b/tests/unit/triggers/pr-opened.test.ts @@ -2,6 +2,11 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { PROpenedTrigger } from '../../../src/triggers/github/pr-opened.js'; import type { TriggerContext } from '../../../src/triggers/types.js'; +vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ + lookupWorkItemForPR: vi.fn(), +})); +import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; + describe('PROpenedTrigger', () => { const trigger = new PROpenedTrigger(); @@ -32,6 +37,7 @@ describe('PROpenedTrigger', () => { beforeEach(() => { vi.clearAllMocks(); + vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); }); describe('matches', () => { @@ -216,7 +222,7 @@ describe('PROpenedTrigger', () => { }); }); - it('returns null when PR body has no Trello URL', async () => { + it('fires without work item when PR has no work item reference', async () => { const ctx: TriggerContext = { project: mockProjectWithPrOpenedEnabled, source: 'github', @@ -241,10 +247,11 @@ describe('PROpenedTrigger', () => { const result = await trigger.handle(ctx); - expect(result).toBeNull(); + expect(result).not.toBeNull(); + expect(result?.workItemId).toBeUndefined(); }); - it('handles null PR body', async () => { + it('fires with undefined workItemId for null PR body', async () => { const ctx: TriggerContext = { project: mockProjectWithPrOpenedEnabled, source: 'github', @@ -269,7 +276,8 @@ describe('PROpenedTrigger', () => { const result = await trigger.handle(ctx); - expect(result).toBeNull(); + expect(result).not.toBeNull(); + expect(result?.workItemId).toBeUndefined(); }); }); }); diff --git a/tests/unit/triggers/pr-ready-to-merge.test.ts b/tests/unit/triggers/pr-ready-to-merge.test.ts index 63c34ac5..403ed431 100644 --- a/tests/unit/triggers/pr-ready-to-merge.test.ts +++ b/tests/unit/triggers/pr-ready-to-merge.test.ts @@ -44,6 +44,9 @@ vi.mock('../../../src/router/acknowledgments.js', () => ({ vi.mock('../../../src/router/reactions.js', () => ({ sendAcknowledgeReaction: vi.fn(), })); +vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ + lookupWorkItemForPR: vi.fn(), +})); // Register PM integrations in the registry import '../../../src/pm/index.js'; @@ -51,6 +54,7 @@ import '../../../src/pm/index.js'; import { PRReadyToMergeTrigger } from '../../../src/triggers/github/pr-ready-to-merge.js'; import type { TriggerContext } from '../../../src/triggers/types.js'; +import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; import { githubClient } from '../../../src/github/client.js'; describe('PRReadyToMergeTrigger', () => { @@ -76,6 +80,7 @@ describe('PRReadyToMergeTrigger', () => { beforeEach(() => { vi.clearAllMocks(); + vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); }); describe('resolveAgentType', () => { diff --git a/tests/unit/triggers/pr-review-submitted.test.ts b/tests/unit/triggers/pr-review-submitted.test.ts index 97a816fa..c0291091 100644 --- a/tests/unit/triggers/pr-review-submitted.test.ts +++ b/tests/unit/triggers/pr-review-submitted.test.ts @@ -1,10 +1,20 @@ -import { describe, expect, it } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { PRReviewSubmittedTrigger } from '../../../src/triggers/github/pr-review-submitted.js'; import type { TriggerContext } from '../../../src/triggers/types.js'; +vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ + lookupWorkItemForPR: vi.fn(), +})); +import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; + describe('PRReviewSubmittedTrigger', () => { const trigger = new PRReviewSubmittedTrigger(); + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); + }); + const mockProject = { id: 'test', name: 'Test', @@ -219,7 +229,7 @@ describe('PRReviewSubmittedTrigger', () => { expect(result).toBeNull(); }); - it('returns null when PR has no Trello URL', async () => { + it('fires without work item when PR has no work item reference', async () => { const ctx: TriggerContext = { project: mockProject, source: 'github', @@ -238,7 +248,8 @@ describe('PRReviewSubmittedTrigger', () => { const result = await trigger.handle(ctx); - expect(result).toBeNull(); + expect(result).not.toBeNull(); + expect(result?.workItemId).toBeUndefined(); }); it('uses review state as fallback when review body is null', async () => { diff --git a/tests/unit/triggers/review-requested.test.ts b/tests/unit/triggers/review-requested.test.ts index ba96b259..b6156bba 100644 --- a/tests/unit/triggers/review-requested.test.ts +++ b/tests/unit/triggers/review-requested.test.ts @@ -1,7 +1,12 @@ -import { describe, expect, it } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ReviewRequestedTrigger } from '../../../src/triggers/github/review-requested.js'; import type { TriggerContext } from '../../../src/triggers/types.js'; +vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ + lookupWorkItemForPR: vi.fn(), +})); +import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; + describe('ReviewRequestedTrigger', () => { const trigger = new ReviewRequestedTrigger(); @@ -36,6 +41,11 @@ describe('ReviewRequestedTrigger', () => { reviewer: 'cascade-reviewer', }; + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); + }); + const makeReviewRequestedPayload = (reviewerLogin = 'cascade-reviewer') => ({ action: 'review_requested', number: 42, @@ -158,7 +168,7 @@ describe('ReviewRequestedTrigger', () => { expect(result).toBeNull(); }); - it('returns null when PR body has no Trello card URL', async () => { + it('fires without work item when PR has no work item reference', async () => { const ctx: TriggerContext = { project: mockProjectWithReviewRequested, source: 'github', @@ -172,7 +182,8 @@ describe('ReviewRequestedTrigger', () => { personaIdentities: mockPersonaIdentities, }; const result = await trigger.handle(ctx); - expect(result).toBeNull(); + expect(result).not.toBeNull(); + expect(result?.workItemId).toBeUndefined(); }); it('triggers review agent when reviewer persona is requested', async () => {