From 561c232eb6a9621dca80642acd0b9a1bc8c3b513 Mon Sep 17 00:00:00 2001 From: Zbigniew Sobiecki Date: Sun, 22 Feb 2026 12:32:43 +0100 Subject: [PATCH 1/6] feat(jira): add description support for JIRA subtasks created via AddChecklist (#472) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The planning agent writes detailed step descriptions (files to modify, specific changes, testing strategy) in the card body, but this context was lost when creating bare JIRA subtasks with only a summary. This threads an optional description through the AddChecklist gadget → PMProvider.addChecklistItem() → JIRA adapter, where it is converted to ADF and set on the subtask. Trello checklist items don't support descriptions, so the Trello adapter ignores the field. The schema change is backward-compatible: items accept string | {name, description?}. Co-authored-by: Claude Opus 4.6 --- src/agents/prompts/templates/planning.eta | 5 +- src/gadgets/pm/AddChecklist.ts | 37 +++++++-- src/gadgets/pm/core/addChecklist.ts | 8 +- src/pm/jira/adapter.ts | 8 +- src/pm/trello/adapter.ts | 7 +- src/pm/types.ts | 7 +- .../unit/gadgets/pm/core/addChecklist.test.ts | 80 ++++++++++++++++++- tests/unit/pm/jira/adapter.test.ts | 34 ++++++++ 8 files changed, 170 insertions(+), 16 deletions(-) diff --git a/src/agents/prompts/templates/planning.eta b/src/agents/prompts/templates/planning.eta index 2931e8a9..40cfcf73 100644 --- a/src/agents/prompts/templates/planning.eta +++ b/src/agents/prompts/templates/planning.eta @@ -101,7 +101,8 @@ Update the <%= it.workItemNoun || 'card' %> description with **emoji section hea **IMPORTANT:** - After updating the <%= it.workItemNoun || 'card' %>, ALWAYS call `AddChecklist` to create an interactive "📋 Implementation Steps" checklist with each step as an item. -- When referencing other <%= it.workItemNounPlural || 'cards' %> (related stories, dependencies), ALWAYS use markdown links: `[<%= it.workItemNounCap || 'Card' %> Title](URL)` +<% if (it.pmType === 'jira') { %>- When calling `AddChecklist`, pass items as objects with `name` and `description`. The description should include the files to modify, specific changes, and testing notes from the corresponding Implementation Step section. This becomes the JIRA subtask description. +<% } %>- When referencing other <%= it.workItemNounPlural || 'cards' %> (related stories, dependencies), ALWAYS use markdown links: `[<%= it.workItemNounCap || 'Card' %> Title](URL)` ## Comment Format @@ -124,7 +125,7 @@ Review the updated description and move to TODO when ready to implement! **<%= it.pmName || 'Trello' %> (for outputting your plan):** - `ReadWorkItem` - Read <%= it.workItemNoun || 'card' %> details (title, description, comments, labels) - `UpdateWorkItem` - Update <%= it.workItemNoun || 'card' %> title/description -- `AddChecklist` - Add an interactive checklist to a <%= it.workItemNoun || 'card' %> (use for implementation steps) +- `AddChecklist` - Add an interactive checklist to a <%= it.workItemNoun || 'card' %> (use for implementation steps)<% if (it.pmType === 'jira') { %>. Each item can include a `description` with files-to-modify, specific changes, and testing notes — these become subtask descriptions.<% } %> - `PostComment` - Post a comment on a <%= it.workItemNoun || 'card' %> **Codebase exploration (READ-ONLY):** diff --git a/src/gadgets/pm/AddChecklist.ts b/src/gadgets/pm/AddChecklist.ts index b1eb94c7..25cf3d6f 100644 --- a/src/gadgets/pm/AddChecklist.ts +++ b/src/gadgets/pm/AddChecklist.ts @@ -11,20 +11,45 @@ export class AddChecklist extends Gadget({ checklistName: z .string() .describe('Name of the checklist (e.g., "Acceptance Criteria" or "Implementation Steps")'), - items: z.array(z.string()).min(1).describe('List of checklist items to add'), + items: z + .array( + z.union([ + z.string(), + z.object({ + name: z.string().describe('Checklist item name / subtask title'), + description: z + .string() + .optional() + .describe( + 'Detailed description (used as JIRA subtask description, ignored for Trello)', + ), + }), + ]), + ) + .min(1) + .describe( + 'List of checklist items to add. Use objects with name+description for richer subtasks.', + ), }), examples: [ { params: { - workItemId: 'abc123', + workItemId: 'PROJ-42', checklistName: 'Implementation Steps', items: [ - 'Add reset password endpoint to API', - 'Create email template for reset link', - 'Add password validation logic', + { + name: 'Add reset password endpoint to API', + description: + '**Files:** `src/api/auth.ts`\n- Add POST /auth/reset-password route\n- Validate email format and lookup user\n- Generate time-limited reset token', + }, + { + name: 'Create email template for reset link', + description: + '**Files:** `src/templates/reset-password.html`\n- Create responsive HTML email template\n- Include reset link with token parameter', + }, ], }, - comment: 'Add implementation steps checklist to a work item', + comment: 'Add implementation steps with descriptions to a JIRA issue', }, ], }) { diff --git a/src/gadgets/pm/core/addChecklist.ts b/src/gadgets/pm/core/addChecklist.ts index bf4148be..eb47f781 100644 --- a/src/gadgets/pm/core/addChecklist.ts +++ b/src/gadgets/pm/core/addChecklist.ts @@ -1,9 +1,11 @@ import { getPMProvider } from '../../../pm/index.js'; +export type ChecklistItemInput = string | { name: string; description?: string }; + export interface AddChecklistParams { workItemId: string; checklistName: string; - items: string[]; + items: ChecklistItemInput[]; } export async function addChecklist(params: AddChecklistParams): Promise { @@ -11,7 +13,9 @@ export async function addChecklist(params: AddChecklistParams): Promise const checklist = await provider.createChecklist(params.workItemId, params.checklistName); for (const item of params.items) { - await provider.addChecklistItem(checklist.id, item); + const name = typeof item === 'string' ? item : item.name; + const description = typeof item === 'string' ? undefined : item.description; + await provider.addChecklistItem(checklist.id, name, false, description); } return `Checklist "${params.checklistName}" created with ${params.items.length} items on work item ${params.workItemId}`; diff --git a/src/pm/jira/adapter.ts b/src/pm/jira/adapter.ts index 70151ba5..1673da5b 100644 --- a/src/pm/jira/adapter.ts +++ b/src/pm/jira/adapter.ts @@ -242,7 +242,12 @@ export class JiraPMProvider implements PMProvider { }; } - async addChecklistItem(_checklistId: string, name: string, _checked = false): Promise { + async addChecklistItem( + _checklistId: string, + name: string, + _checked = false, + description?: string, + ): Promise { // Extract parent issue key from checklistId format: "checklist-PROJ-123-timestamp" // or "subtasks-PROJ-123" // Use \d{10,} to only strip timestamps (10+ digits), not issue numbers like PROJ-5 @@ -258,6 +263,7 @@ export class JiraPMProvider implements PMProvider { parent: { key: parentKey }, summary: name, issuetype: { name: issueType }, + ...(description ? { description: markdownToAdf(description) } : {}), }); } diff --git a/src/pm/trello/adapter.ts b/src/pm/trello/adapter.ts index c558f90f..d2f392cf 100644 --- a/src/pm/trello/adapter.ts +++ b/src/pm/trello/adapter.ts @@ -146,7 +146,12 @@ export class TrelloPMProvider implements PMProvider { }; } - async addChecklistItem(checklistId: string, name: string, checked = false): Promise { + async addChecklistItem( + checklistId: string, + name: string, + checked = false, + _description?: string, + ): Promise { await trelloClient.addChecklistItem(checklistId, name, checked); } diff --git a/src/pm/types.ts b/src/pm/types.ts index a5d91e92..6654a784 100644 --- a/src/pm/types.ts +++ b/src/pm/types.ts @@ -80,7 +80,12 @@ export interface PMProvider { // Checklists getChecklists(workItemId: string): Promise; createChecklist(workItemId: string, name: string): Promise; - addChecklistItem(checklistId: string, name: string, checked?: boolean): Promise; + addChecklistItem( + checklistId: string, + name: string, + checked?: boolean, + description?: string, + ): Promise; updateChecklistItem(workItemId: string, checkItemId: string, complete: boolean): Promise; // Attachments & custom fields diff --git a/tests/unit/gadgets/pm/core/addChecklist.test.ts b/tests/unit/gadgets/pm/core/addChecklist.test.ts index 1f5132c4..8e1131d5 100644 --- a/tests/unit/gadgets/pm/core/addChecklist.test.ts +++ b/tests/unit/gadgets/pm/core/addChecklist.test.ts @@ -15,7 +15,7 @@ beforeEach(() => { }); describe('addChecklist', () => { - it('creates checklist and adds items', async () => { + it('creates checklist and adds string items', async () => { mockProvider.createChecklist.mockResolvedValue({ id: 'cl1', name: 'My Tasks', @@ -32,11 +32,85 @@ describe('addChecklist', () => { expect(mockProvider.createChecklist).toHaveBeenCalledWith('item1', 'My Tasks'); expect(mockProvider.addChecklistItem).toHaveBeenCalledTimes(2); - expect(mockProvider.addChecklistItem).toHaveBeenCalledWith('cl1', 'Task A'); - expect(mockProvider.addChecklistItem).toHaveBeenCalledWith('cl1', 'Task B'); + expect(mockProvider.addChecklistItem).toHaveBeenCalledWith('cl1', 'Task A', false, undefined); + expect(mockProvider.addChecklistItem).toHaveBeenCalledWith('cl1', 'Task B', false, undefined); expect(result).toBe('Checklist "My Tasks" created with 2 items on work item item1'); }); + it('creates checklist and adds object items with descriptions', async () => { + mockProvider.createChecklist.mockResolvedValue({ + id: 'cl1', + name: 'Steps', + workItemId: 'PROJ-42', + items: [], + }); + mockProvider.addChecklistItem.mockResolvedValue(undefined); + + const result = await addChecklist({ + workItemId: 'PROJ-42', + checklistName: 'Steps', + items: [ + { name: 'Add endpoint', description: '**Files:** `src/api.ts`\n- Add POST route' }, + { name: 'Write tests' }, + ], + }); + + expect(mockProvider.addChecklistItem).toHaveBeenCalledTimes(2); + expect(mockProvider.addChecklistItem).toHaveBeenCalledWith( + 'cl1', + 'Add endpoint', + false, + '**Files:** `src/api.ts`\n- Add POST route', + ); + expect(mockProvider.addChecklistItem).toHaveBeenCalledWith( + 'cl1', + 'Write tests', + false, + undefined, + ); + expect(result).toBe('Checklist "Steps" created with 2 items on work item PROJ-42'); + }); + + it('handles mixed string and object items', async () => { + mockProvider.createChecklist.mockResolvedValue({ + id: 'cl1', + name: 'Mixed', + workItemId: 'item1', + items: [], + }); + mockProvider.addChecklistItem.mockResolvedValue(undefined); + + await addChecklist({ + workItemId: 'item1', + checklistName: 'Mixed', + items: [ + 'Simple string item', + { name: 'Object item', description: 'Detailed description' }, + 'Another string', + ], + }); + + expect(mockProvider.addChecklistItem).toHaveBeenCalledTimes(3); + expect(mockProvider.addChecklistItem).toHaveBeenCalledWith( + 'cl1', + 'Simple string item', + false, + undefined, + ); + expect(mockProvider.addChecklistItem).toHaveBeenCalledWith( + 'cl1', + 'Object item', + false, + 'Detailed description', + ); + expect(mockProvider.addChecklistItem).toHaveBeenCalledWith( + 'cl1', + 'Another string', + false, + undefined, + ); + }); + it('creates checklist with no items', async () => { mockProvider.createChecklist.mockResolvedValue({ id: 'cl1', diff --git a/tests/unit/pm/jira/adapter.test.ts b/tests/unit/pm/jira/adapter.test.ts index 82cfd111..e1d6339e 100644 --- a/tests/unit/pm/jira/adapter.test.ts +++ b/tests/unit/pm/jira/adapter.test.ts @@ -495,6 +495,40 @@ describe('JiraPMProvider', () => { expect(mockJiraClient.getIssueTypesForProject).toHaveBeenCalledOnce(); }); + it('passes description as ADF to createIssue when provided', async () => { + const adfDoc = { type: 'doc', version: 1, content: [{ type: 'paragraph' }] }; + mockMarkdownToAdf.mockReturnValue(adfDoc); + mockJiraClient.createIssue.mockResolvedValue({ key: 'PROJ-105' }); + + await provider.addChecklistItem( + 'checklist-PROJ-1-1234567890', + 'Subtask with description', + false, + '**Files:** `src/api.ts`\n- Add POST route', + ); + + expect(mockMarkdownToAdf).toHaveBeenCalledWith('**Files:** `src/api.ts`\n- Add POST route'); + expect(mockJiraClient.createIssue).toHaveBeenCalledWith( + expect.objectContaining({ + project: { key: 'PROJ' }, + parent: { key: 'PROJ-1' }, + summary: 'Subtask with description', + issuetype: { name: 'Sub-task' }, + description: adfDoc, + }), + ); + }); + + it('omits description from createIssue when not provided', async () => { + mockJiraClient.createIssue.mockResolvedValue({ key: 'PROJ-106' }); + + await provider.addChecklistItem('checklist-PROJ-1-1234567890', 'No description subtask'); + + expect(mockJiraClient.createIssue).toHaveBeenCalledWith( + expect.not.objectContaining({ description: expect.anything() }), + ); + }); + it('falls back to "Subtask" when no subtask type found', async () => { const providerNoConfig = new JiraPMProvider({ ...mockConfig, From 9961e745c02c9239a16cf0cf5b3269a65afc3fa0 Mon Sep 17 00:00:00 2001 From: Zbigniew Sobiecki Date: Sun, 22 Feb 2026 12:02:53 +0000 Subject: [PATCH 2/6] feat(pm): add DeleteChecklistItem gadget for removing descoped plan steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the respond-to-planning-comment agent needs to remove descoped items from a plan, it previously had no way to delete checklist items and resorted to marking them as "Done" — which is semantically wrong. This adds a DeleteChecklistItem gadget that actually removes items. Changes wired through the full stack: - JIRA client: deleteIssue(issueKey) - Trello client: deleteChecklistItem(checklistId, checkItemId) - PMProvider interface: deleteChecklistItem(workItemId, checkItemId) - JIRA adapter: deletes the subtask issue - Trello adapter: scans checklists to find the item, then deletes it - Core function, gadget class, index export, agent registration - Prompt updated for respond-to-planning-comment agent Co-Authored-By: Claude Opus 4.6 --- .../templates/respond-to-planning-comment.eta | 1 + src/agents/shared/gadgets.ts | 3 +- src/gadgets/pm/DeleteChecklistItem.ts | 28 +++++++++ src/gadgets/pm/core/deleteChecklistItem.ts | 14 +++++ src/gadgets/pm/index.ts | 1 + src/jira/client.ts | 5 ++ src/pm/jira/adapter.ts | 5 ++ src/pm/trello/adapter.ts | 12 ++++ src/pm/types.ts | 1 + src/trello/client.ts | 8 +++ tests/helpers/mockPMProvider.ts | 1 + tests/unit/agents/shared/gadgets.test.ts | 7 ++- tests/unit/backends/agent-profiles.test.ts | 1 + .../pm/core/deleteChecklistItem.test.ts | 42 +++++++++++++ tests/unit/jira/client.test.ts | 18 ++++++ tests/unit/pm/jira/adapter.test.ts | 19 ++++++ tests/unit/pm/trello/adapter.test.ts | 60 +++++++++++++++++++ tests/unit/trello/client.test.ts | 20 +++++++ 18 files changed, 243 insertions(+), 3 deletions(-) create mode 100644 src/gadgets/pm/DeleteChecklistItem.ts create mode 100644 src/gadgets/pm/core/deleteChecklistItem.ts create mode 100644 tests/unit/gadgets/pm/core/deleteChecklistItem.test.ts diff --git a/src/agents/prompts/templates/respond-to-planning-comment.eta b/src/agents/prompts/templates/respond-to-planning-comment.eta index 8b0f1fae..50202c89 100644 --- a/src/agents/prompts/templates/respond-to-planning-comment.eta +++ b/src/agents/prompts/templates/respond-to-planning-comment.eta @@ -78,6 +78,7 @@ Based on your comment, I've made the following changes: - `UpdateWorkItem` - Update <%= it.workItemNoun || 'card' %> title/description - `AddChecklist` - Add an interactive checklist to a <%= it.workItemNoun || 'card' %> - `UpdateChecklistItem` - Update checklist item state (complete/incomplete) or name +- `DeleteChecklistItem` - Delete a checklist item / subtask (use to remove descoped steps — do NOT mark removed items as complete) - `PostComment` - Post a comment on a <%= it.workItemNoun || 'card' %> **Codebase exploration (READ-ONLY):** diff --git a/src/agents/shared/gadgets.ts b/src/agents/shared/gadgets.ts index ea601668..12eebe4d 100644 --- a/src/agents/shared/gadgets.ts +++ b/src/agents/shared/gadgets.ts @@ -23,6 +23,7 @@ import { AddChecklist, CreateWorkItem, ListWorkItems, + PMDeleteChecklistItem, PMUpdateChecklistItem, PostComment, ReadWorkItem, @@ -73,7 +74,7 @@ export function buildWorkItemGadgets(caps: AgentCapabilities): CreateBuilderOpti new AddChecklist(), // UpdateChecklistItem gated by capability — prevents planning from marking items complete // prematurely, while respond-to-planning-comment CAN update them - ...(caps.canUpdateChecklists ? [new PMUpdateChecklistItem()] : []), + ...(caps.canUpdateChecklists ? [new PMUpdateChecklistItem(), new PMDeleteChecklistItem()] : []), // Session control new Finish(), ]; diff --git a/src/gadgets/pm/DeleteChecklistItem.ts b/src/gadgets/pm/DeleteChecklistItem.ts new file mode 100644 index 00000000..230cf719 --- /dev/null +++ b/src/gadgets/pm/DeleteChecklistItem.ts @@ -0,0 +1,28 @@ +import { Gadget, z } from 'llmist'; +import { deleteChecklistItem } from './core/deleteChecklistItem.js'; + +export class PMDeleteChecklistItem extends Gadget({ + name: 'DeleteChecklistItem', + description: + 'Delete a checklist item from a work item. For JIRA this deletes the subtask issue. For Trello this removes the checklist item. Use this to remove descoped or invalid plan steps — do NOT mark items as "complete" if they were never done.', + timeoutMs: 15000, + schema: z.object({ + workItemId: z.string().describe('The work item ID (Trello card ID or JIRA issue key)'), + checkItemId: z + .string() + .describe('The checklist item ID to delete (JIRA subtask key or Trello check item ID)'), + }), + examples: [ + { + params: { + workItemId: 'PROJ-42', + checkItemId: 'PROJ-48', + }, + comment: 'Delete a descoped subtask from a JIRA issue', + }, + ], +}) { + override async execute(params: this['params']): Promise { + return deleteChecklistItem(params.workItemId, params.checkItemId); + } +} diff --git a/src/gadgets/pm/core/deleteChecklistItem.ts b/src/gadgets/pm/core/deleteChecklistItem.ts new file mode 100644 index 00000000..cb57db42 --- /dev/null +++ b/src/gadgets/pm/core/deleteChecklistItem.ts @@ -0,0 +1,14 @@ +import { getPMProvider } from '../../../pm/index.js'; + +export async function deleteChecklistItem( + workItemId: string, + checkItemId: string, +): Promise { + try { + await getPMProvider().deleteChecklistItem(workItemId, checkItemId); + return `Checklist item ${checkItemId} deleted from work item ${workItemId}`; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + return `Error deleting checklist item: ${message}`; + } +} diff --git a/src/gadgets/pm/index.ts b/src/gadgets/pm/index.ts index 470fb296..452918a1 100644 --- a/src/gadgets/pm/index.ts +++ b/src/gadgets/pm/index.ts @@ -5,3 +5,4 @@ export { CreateWorkItem } from './CreateWorkItem.js'; export { ListWorkItems } from './ListWorkItems.js'; export { AddChecklist } from './AddChecklist.js'; export { PMUpdateChecklistItem } from './UpdateChecklistItem.js'; +export { PMDeleteChecklistItem } from './DeleteChecklistItem.js'; diff --git a/src/jira/client.ts b/src/jira/client.ts index 582330a5..7a20fce7 100644 --- a/src/jira/client.ts +++ b/src/jira/client.ts @@ -238,6 +238,11 @@ export const jiraClient = { } }, + async deleteIssue(issueKey: string) { + logger.debug('Deleting JIRA issue', { issueKey }); + await getClient().issues.deleteIssue({ issueIdOrKey: issueKey }); + }, + async addAttachmentFile(issueKey: string, buffer: Buffer, filename: string) { logger.debug('Adding JIRA attachment', { issueKey, filename }); await getClient().issueAttachments.addAttachment({ diff --git a/src/pm/jira/adapter.ts b/src/pm/jira/adapter.ts index 1673da5b..6b1751b6 100644 --- a/src/pm/jira/adapter.ts +++ b/src/pm/jira/adapter.ts @@ -277,6 +277,11 @@ export class JiraPMProvider implements PMProvider { await this.moveWorkItem(checkItemId, targetStatus); } + async deleteChecklistItem(_workItemId: string, checkItemId: string): Promise { + // checkItemId is a JIRA issue key (subtask) + await jiraClient.deleteIssue(checkItemId); + } + async getAttachments(workItemId: string): Promise { const issue = await jiraClient.getIssue(workItemId); const attachments = diff --git a/src/pm/trello/adapter.ts b/src/pm/trello/adapter.ts index d2f392cf..00c44f5b 100644 --- a/src/pm/trello/adapter.ts +++ b/src/pm/trello/adapter.ts @@ -167,6 +167,18 @@ export class TrelloPMProvider implements PMProvider { ); } + async deleteChecklistItem(workItemId: string, checkItemId: string): Promise { + const checklists = await trelloClient.getCardChecklists(workItemId); + for (const cl of checklists) { + const item = cl.checkItems.find((i) => i.id === checkItemId); + if (item) { + await trelloClient.deleteChecklistItem(cl.id, checkItemId); + return; + } + } + throw new Error(`Checklist item ${checkItemId} not found on card ${workItemId}`); + } + async getAttachments(workItemId: string): Promise { const attachments = await trelloClient.getCardAttachments(workItemId); return attachments.map((a) => ({ diff --git a/src/pm/types.ts b/src/pm/types.ts index 6654a784..5c90a3d6 100644 --- a/src/pm/types.ts +++ b/src/pm/types.ts @@ -87,6 +87,7 @@ export interface PMProvider { description?: string, ): Promise; updateChecklistItem(workItemId: string, checkItemId: string, complete: boolean): Promise; + deleteChecklistItem(workItemId: string, checkItemId: string): Promise; // Attachments & custom fields getAttachments(workItemId: string): Promise; diff --git a/src/trello/client.ts b/src/trello/client.ts index 99d8a43f..decb279f 100644 --- a/src/trello/client.ts +++ b/src/trello/client.ts @@ -405,6 +405,14 @@ export const trelloClient = { }); }, + async deleteChecklistItem(checklistId: string, checkItemId: string): Promise { + logger.debug('Deleting checklist item', { checklistId, checkItemId }); + await getClient().checklists.deleteChecklistCheckItem({ + id: checklistId, + idCheckItem: checkItemId, + }); + }, + async addActionReaction( actionId: string, emoji: { shortName: string; native: string; unified: string }, diff --git a/tests/helpers/mockPMProvider.ts b/tests/helpers/mockPMProvider.ts index 33d47aea..573ba462 100644 --- a/tests/helpers/mockPMProvider.ts +++ b/tests/helpers/mockPMProvider.ts @@ -30,6 +30,7 @@ export function createMockPMProvider() { createChecklist: vi.fn(), addChecklistItem: vi.fn(), updateChecklistItem: vi.fn(), + deleteChecklistItem: vi.fn(), addAttachment: vi.fn(), addAttachmentFile: vi.fn(), getCustomFieldNumber: vi.fn(), diff --git a/tests/unit/agents/shared/gadgets.test.ts b/tests/unit/agents/shared/gadgets.test.ts index 1f00d901..3f5eaf8b 100644 --- a/tests/unit/agents/shared/gadgets.test.ts +++ b/tests/unit/agents/shared/gadgets.test.ts @@ -39,6 +39,7 @@ vi.mock('../../../../src/gadgets/pm/index.js', () => ({ AddChecklist: mockClass('AddChecklist'), CreateWorkItem: mockClass('CreateWorkItem'), ListWorkItems: mockClass('ListWorkItems'), + PMDeleteChecklistItem: mockClass('PMDeleteChecklistItem'), PMUpdateChecklistItem: mockClass('PMUpdateChecklistItem'), PostComment: mockClass('PostComment'), ReadWorkItem: mockClass('ReadWorkItem'), @@ -124,14 +125,16 @@ describe('buildWorkItemGadgets', () => { expect(gadgets).not.toContain('CreatePR'); }); - it('includes PMUpdateChecklistItem when canUpdateChecklists is true', () => { + it('includes PMUpdateChecklistItem and PMDeleteChecklistItem when canUpdateChecklists is true', () => { const gadgets = names(buildWorkItemGadgets(FULL_CAPS)); expect(gadgets).toContain('PMUpdateChecklistItem'); + expect(gadgets).toContain('PMDeleteChecklistItem'); }); - it('excludes PMUpdateChecklistItem when canUpdateChecklists is false', () => { + it('excludes PMUpdateChecklistItem and PMDeleteChecklistItem when canUpdateChecklists is false', () => { const gadgets = names(buildWorkItemGadgets(READ_ONLY_CAPS)); expect(gadgets).not.toContain('PMUpdateChecklistItem'); + expect(gadgets).not.toContain('PMDeleteChecklistItem'); }); }); diff --git a/tests/unit/backends/agent-profiles.test.ts b/tests/unit/backends/agent-profiles.test.ts index 5622b15f..5bd4d464 100644 --- a/tests/unit/backends/agent-profiles.test.ts +++ b/tests/unit/backends/agent-profiles.test.ts @@ -79,6 +79,7 @@ vi.mock('../../../src/gadgets/pm/index.js', () => ({ AddChecklist: mockClass('AddChecklist'), CreateWorkItem: mockClass('CreateWorkItem'), ListWorkItems: mockClass('ListWorkItems'), + PMDeleteChecklistItem: mockClass('PMDeleteChecklistItem'), PMUpdateChecklistItem: mockClass('PMUpdateChecklistItem'), PostComment: mockClass('PostComment'), ReadWorkItem: mockClass('ReadWorkItem'), diff --git a/tests/unit/gadgets/pm/core/deleteChecklistItem.test.ts b/tests/unit/gadgets/pm/core/deleteChecklistItem.test.ts new file mode 100644 index 00000000..976ca898 --- /dev/null +++ b/tests/unit/gadgets/pm/core/deleteChecklistItem.test.ts @@ -0,0 +1,42 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { createMockPMProvider } from '../../../../helpers/mockPMProvider.js'; + +const mockProvider = createMockPMProvider(); + +vi.mock('../../../../../src/pm/index.js', () => ({ + getPMProvider: vi.fn(() => mockProvider), +})); + +import { deleteChecklistItem } from '../../../../../src/gadgets/pm/core/deleteChecklistItem.js'; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('deleteChecklistItem', () => { + it('deletes a checklist item and returns success message', async () => { + mockProvider.deleteChecklistItem.mockResolvedValue(undefined); + + const result = await deleteChecklistItem('item1', 'checkItem1'); + + expect(mockProvider.deleteChecklistItem).toHaveBeenCalledWith('item1', 'checkItem1'); + expect(result).toBe('Checklist item checkItem1 deleted from work item item1'); + }); + + it('returns error message on failure', async () => { + mockProvider.deleteChecklistItem.mockRejectedValue(new Error('API error')); + + const result = await deleteChecklistItem('item1', 'checkItem1'); + + expect(result).toBe('Error deleting checklist item: API error'); + }); + + it('handles non-Error thrown value', async () => { + mockProvider.deleteChecklistItem.mockRejectedValue('string error'); + + const result = await deleteChecklistItem('item1', 'ci1'); + + expect(result).toBe('Error deleting checklist item: string error'); + }); +}); diff --git a/tests/unit/jira/client.test.ts b/tests/unit/jira/client.test.ts index 6d44f958..23413dcb 100644 --- a/tests/unit/jira/client.test.ts +++ b/tests/unit/jira/client.test.ts @@ -22,6 +22,7 @@ const { getIssue: vi.fn(), editIssue: vi.fn(), createIssue: vi.fn(), + deleteIssue: vi.fn(), doTransition: vi.fn(), getTransitions: vi.fn(), }, @@ -75,6 +76,7 @@ describe('jiraClient', () => { mockIssues.getIssue.mockReset(); mockIssues.editIssue.mockReset(); mockIssues.createIssue.mockReset(); + mockIssues.deleteIssue.mockReset(); mockIssues.doTransition.mockReset(); mockIssues.getTransitions.mockReset(); mockIssueComments.getComments.mockReset(); @@ -435,6 +437,22 @@ describe('jiraClient', () => { }); }); + describe('deleteIssue', () => { + it('calls deleteIssue with the issue key', async () => { + mockIssues.deleteIssue.mockResolvedValue(undefined); + + await withJiraCredentials(creds, () => jiraClient.deleteIssue('TEST-5')); + + expect(mockIssues.deleteIssue).toHaveBeenCalledWith({ issueIdOrKey: 'TEST-5' }); + }); + + it('throws when called outside scope', async () => { + await expect(jiraClient.deleteIssue('TEST-5')).rejects.toThrow( + 'No JIRA credentials in scope', + ); + }); + }); + describe('transitionIssue', () => { it('calls doTransition with issue key and transition id', async () => { mockIssues.doTransition.mockResolvedValue(undefined); diff --git a/tests/unit/pm/jira/adapter.test.ts b/tests/unit/pm/jira/adapter.test.ts index e1d6339e..4f4ad511 100644 --- a/tests/unit/pm/jira/adapter.test.ts +++ b/tests/unit/pm/jira/adapter.test.ts @@ -9,6 +9,7 @@ const { mockJiraClient, mockAdfToPlainText, mockMarkdownToAdf } = vi.hoisted(() addComment: vi.fn(), updateComment: vi.fn(), createIssue: vi.fn(), + deleteIssue: vi.fn(), getIssueTypesForProject: vi.fn(), searchIssues: vi.fn(), getTransitions: vi.fn(), @@ -563,6 +564,24 @@ describe('JiraPMProvider', () => { }); }); + describe('deleteChecklistItem', () => { + it('delegates to jiraClient.deleteIssue with the subtask key', async () => { + mockJiraClient.deleteIssue.mockResolvedValue(undefined); + + await provider.deleteChecklistItem('PROJ-1', 'PROJ-5'); + + expect(mockJiraClient.deleteIssue).toHaveBeenCalledWith('PROJ-5'); + }); + + it('ignores workItemId (not needed for JIRA subtask deletion)', async () => { + mockJiraClient.deleteIssue.mockResolvedValue(undefined); + + await provider.deleteChecklistItem('PROJ-99', 'PROJ-5'); + + expect(mockJiraClient.deleteIssue).toHaveBeenCalledWith('PROJ-5'); + }); + }); + describe('getAttachments', () => { it('maps JIRA attachment fields to Attachment type', async () => { mockJiraClient.getIssue.mockResolvedValue({ diff --git a/tests/unit/pm/trello/adapter.test.ts b/tests/unit/pm/trello/adapter.test.ts index 211fd5d8..5072d3c7 100644 --- a/tests/unit/pm/trello/adapter.test.ts +++ b/tests/unit/pm/trello/adapter.test.ts @@ -17,6 +17,7 @@ const { mockTrelloClient } = vi.hoisted(() => ({ createChecklist: vi.fn(), addChecklistItem: vi.fn(), updateChecklistItem: vi.fn(), + deleteChecklistItem: vi.fn(), getCardAttachments: vi.fn(), addAttachment: vi.fn(), addAttachmentFile: vi.fn(), @@ -301,6 +302,65 @@ describe('TrelloPMProvider', () => { }); }); + describe('deleteChecklistItem', () => { + it('finds the item in checklists and deletes it', async () => { + mockTrelloClient.getCardChecklists.mockResolvedValue([ + { + id: 'cl-1', + name: 'Steps', + idCard: 'card-1', + checkItems: [ + { id: 'item-1', name: 'Step 1', state: 'incomplete' }, + { id: 'item-2', name: 'Step 2', state: 'incomplete' }, + ], + }, + ]); + mockTrelloClient.deleteChecklistItem.mockResolvedValue(undefined); + + await provider.deleteChecklistItem('card-1', 'item-2'); + + expect(mockTrelloClient.getCardChecklists).toHaveBeenCalledWith('card-1'); + expect(mockTrelloClient.deleteChecklistItem).toHaveBeenCalledWith('cl-1', 'item-2'); + }); + + it('searches across multiple checklists', async () => { + mockTrelloClient.getCardChecklists.mockResolvedValue([ + { + id: 'cl-1', + name: 'First', + idCard: 'card-1', + checkItems: [{ id: 'item-1', name: 'Step 1', state: 'incomplete' }], + }, + { + id: 'cl-2', + name: 'Second', + idCard: 'card-1', + checkItems: [{ id: 'item-3', name: 'Step 3', state: 'complete' }], + }, + ]); + mockTrelloClient.deleteChecklistItem.mockResolvedValue(undefined); + + await provider.deleteChecklistItem('card-1', 'item-3'); + + expect(mockTrelloClient.deleteChecklistItem).toHaveBeenCalledWith('cl-2', 'item-3'); + }); + + it('throws when item is not found on any checklist', async () => { + mockTrelloClient.getCardChecklists.mockResolvedValue([ + { + id: 'cl-1', + name: 'Steps', + idCard: 'card-1', + checkItems: [{ id: 'item-1', name: 'Step 1', state: 'incomplete' }], + }, + ]); + + await expect(provider.deleteChecklistItem('card-1', 'nonexistent')).rejects.toThrow( + 'Checklist item nonexistent not found on card card-1', + ); + }); + }); + describe('getAttachments', () => { it('maps attachment fields correctly', async () => { mockTrelloClient.getCardAttachments.mockResolvedValue([ diff --git a/tests/unit/trello/client.test.ts b/tests/unit/trello/client.test.ts index cf67cdcd..cda58ace 100644 --- a/tests/unit/trello/client.test.ts +++ b/tests/unit/trello/client.test.ts @@ -25,6 +25,7 @@ const { mockCards, mockChecklists, mockLists } = vi.hoisted(() => ({ }, mockChecklists: { createChecklistCheckItems: vi.fn(), + deleteChecklistCheckItem: vi.fn(), }, mockLists: { getListCards: vi.fn(), @@ -318,6 +319,25 @@ describe('trelloClient', () => { }); }); + describe('deleteChecklistItem', () => { + it('calls deleteChecklistCheckItem with correct params', async () => { + mockChecklists.deleteChecklistCheckItem.mockResolvedValue(undefined); + + await withTrelloCredentials(creds, () => trelloClient.deleteChecklistItem('cl-1', 'item-5')); + + expect(mockChecklists.deleteChecklistCheckItem).toHaveBeenCalledWith({ + id: 'cl-1', + idCheckItem: 'item-5', + }); + }); + + it('throws when called outside scope', async () => { + await expect(trelloClient.deleteChecklistItem('cl-1', 'item-5')).rejects.toThrow( + 'No Trello credentials in scope', + ); + }); + }); + describe('getCardAttachments', () => { it('returns attachments via fetch', async () => { const attachments = [ From f69ad7a6486aa7e3ff89113a4c24eb968e514e02 Mon Sep 17 00:00:00 2001 From: Zbigniew Sobiecki Date: Sun, 22 Feb 2026 12:44:24 +0000 Subject: [PATCH 3/6] fix: pass preSeededCommentId in llmist backend + strengthen checklist prompt The llmist backend path in executeAgent() was missing the preSeededCommentId parameter when creating ProgressMonitor, causing the ack comment posted by the router to be orphaned while ProgressMonitor created a new comment. The claude-code backend path already passed this correctly. Also strengthens the respond-to-planning-comment prompt with explicit instructions for checklist management (add/rename/delete) so agents use DeleteChecklistItem when narrowing scope instead of creating duplicate checklists. Additionally carries ackCommentId through the in-worker webhook queue so queued webhooks don't lose their ack comment reference. Co-Authored-By: Claude Opus 4.6 --- src/agents/base.ts | 1 + .../templates/respond-to-planning-comment.eta | 11 +++ src/triggers/github/webhook-handler.ts | 11 ++- src/triggers/jira/webhook-handler.ts | 8 +- src/triggers/shared/webhook-queue.ts | 8 +- src/triggers/trello/webhook-handler.ts | 12 ++- src/utils/webhookQueue.ts | 8 +- tests/unit/triggers/webhook-queue.test.ts | 84 +++++++++++++++++++ tests/unit/utils/webhookQueue.test.ts | 35 ++++++++ 9 files changed, 166 insertions(+), 12 deletions(-) create mode 100644 tests/unit/triggers/webhook-queue.test.ts diff --git a/src/agents/base.ts b/src/agents/base.ts index 0c38e123..f705f7c5 100644 --- a/src/agents/base.ts +++ b/src/agents/base.ts @@ -434,6 +434,7 @@ export async function executeAgent( customModels: CUSTOM_MODELS as ModelSpec[], repoDir, trello: cardId ? { cardId } : undefined, + preSeededCommentId: input.ackCommentId as string | undefined, }), interactive, diff --git a/src/agents/prompts/templates/respond-to-planning-comment.eta b/src/agents/prompts/templates/respond-to-planning-comment.eta index 50202c89..e5473053 100644 --- a/src/agents/prompts/templates/respond-to-planning-comment.eta +++ b/src/agents/prompts/templates/respond-to-planning-comment.eta @@ -53,6 +53,17 @@ You are running in a cloned copy of the project repository. Before updating the 4. **Make surgical updates** to the <%= it.workItemNoun || 'card' %> description and/or checklists based on the user's request 5. **Post a reply comment** via PostComment explaining what you changed and why +## Updating the Plan and Checklists + +When modifying the plan, **update the existing checklists in place** — do NOT create duplicate checklists. + +- **Adding steps**: Use `AddChecklist` only when there is no existing checklist to add items to. Otherwise use `UpdateChecklistItem` to rename existing items or add to an existing checklist. +- **Renaming/rewriting steps**: Use `UpdateChecklistItem` to change the text of existing checklist items. +- **Removing steps**: Use `DeleteChecklistItem` to permanently remove checklist items / subtasks that are no longer needed. Do NOT mark removed items as "complete" — they were never done, so deleting is the correct action. +- **Reordering**: Delete and re-add items as needed to achieve the desired order. + +When the user asks to narrow scope, focus on a subset, or drop items from the plan, **always delete** the out-of-scope items rather than leaving them in the checklist. + ## Response Format When updating the <%= it.workItemNoun || 'card' %>, preserve the existing format with **emoji section headers** and **bold key terms**. Only modify the sections that need to change. diff --git a/src/triggers/github/webhook-handler.ts b/src/triggers/github/webhook-handler.ts index c540d5f8..7b28e6ce 100644 --- a/src/triggers/github/webhook-handler.ts +++ b/src/triggers/github/webhook-handler.ts @@ -156,8 +156,13 @@ async function runGitHubAgentJob( function processNextQueuedGitHubWebhook(registry: TriggerRegistry): void { processNextQueuedWebhook( - (payload, eventType) => - processGitHubWebhook(payload, eventType ?? 'pull_request_review_comment', registry), + (payload, eventType, ackCommentId) => + processGitHubWebhook( + payload, + eventType ?? 'pull_request_review_comment', + registry, + ackCommentId as number | undefined, + ), 'GitHub', (entry) => entry.eventType ?? 'pull_request_review_comment', ); @@ -181,7 +186,7 @@ export async function processGitHubWebhook( } if (isCurrentlyProcessing()) { - const queued = enqueueWebhook(payload, eventType); + const queued = enqueueWebhook(payload, eventType, ackCommentId); if (queued) { logger.info('Currently processing, GitHub webhook queued', { queueLength: getQueueLength(), diff --git a/src/triggers/jira/webhook-handler.ts b/src/triggers/jira/webhook-handler.ts index 6609baea..44dc8b9a 100644 --- a/src/triggers/jira/webhook-handler.ts +++ b/src/triggers/jira/webhook-handler.ts @@ -155,7 +155,11 @@ async function cleanupOrphanJiraAck( } function processNextQueuedJiraWebhook(registry: TriggerRegistry): void { - processNextQueuedWebhook((payload) => processJiraWebhook(payload, registry), 'JIRA'); + processNextQueuedWebhook( + (payload, _eventType, ackCommentId) => + processJiraWebhook(payload, registry, ackCommentId as string | undefined), + 'JIRA', + ); } export async function processJiraWebhook( @@ -173,7 +177,7 @@ export async function processJiraWebhook( } if (isCurrentlyProcessing()) { - const queued = enqueueWebhook(payload); + const queued = enqueueWebhook(payload, undefined, ackCommentId); if (queued) { logger.info('Currently processing, JIRA webhook queued', { queueLength: getQueueLength() }); } else { diff --git a/src/triggers/shared/webhook-queue.ts b/src/triggers/shared/webhook-queue.ts index 5b90f36f..48892b9f 100644 --- a/src/triggers/shared/webhook-queue.ts +++ b/src/triggers/shared/webhook-queue.ts @@ -9,7 +9,11 @@ import { logger } from '../../utils/logging.js'; * @param getEventType - Optional function to extract event type from the queued entry. */ export function processNextQueuedWebhook( - processWebhook: (payload: unknown, eventType?: string) => Promise, + processWebhook: ( + payload: unknown, + eventType?: string, + ackCommentId?: string | number, + ) => Promise, label: string, getEventType?: (entry: { payload: unknown; eventType?: string }) => string | undefined, ): void { @@ -20,7 +24,7 @@ export function processNextQueuedWebhook( if (eventType) logContext.eventType = eventType; logger.info(`Processing queued ${label} webhook`, logContext); setImmediate(() => { - processWebhook(next.payload, eventType).catch((err) => { + processWebhook(next.payload, eventType, next.ackCommentId).catch((err) => { logger.error(`Failed to process queued ${label} webhook`, { error: String(err) }); }); }); diff --git a/src/triggers/trello/webhook-handler.ts b/src/triggers/trello/webhook-handler.ts index 56fafded..25f6f8bc 100644 --- a/src/triggers/trello/webhook-handler.ts +++ b/src/triggers/trello/webhook-handler.ts @@ -61,13 +61,17 @@ async function executeAgent( // ============================================================================ function processNextQueued(registry: TriggerRegistry): void { - processNextQueuedWebhook((payload) => processTrelloWebhook(payload, registry), 'Trello'); + processNextQueuedWebhook( + (payload, _eventType, ackCommentId) => + processTrelloWebhook(payload, registry, ackCommentId as string | undefined), + 'Trello', + ); } -function tryQueueWebhook(payload: TrelloWebhookPayload): boolean { +function tryQueueWebhook(payload: TrelloWebhookPayload, ackCommentId?: string): boolean { if (!isCurrentlyProcessing()) return false; - const queued = enqueueWebhook(payload); + const queued = enqueueWebhook(payload, undefined, ackCommentId); if (queued) { logger.info('Currently processing, webhook queued', { queueLength: getQueueLength() }); } else { @@ -161,7 +165,7 @@ export async function processTrelloWebhook( return; } - if (tryQueueWebhook(payload)) { + if (tryQueueWebhook(payload, ackCommentId)) { return; } diff --git a/src/utils/webhookQueue.ts b/src/utils/webhookQueue.ts index 9f2b546a..3edda042 100644 --- a/src/utils/webhookQueue.ts +++ b/src/utils/webhookQueue.ts @@ -5,12 +5,17 @@ const MAX_QUEUE_SIZE = 10; interface QueuedWebhook { payload: unknown; eventType?: string; // Optional for backward compatibility (Trello doesn't need it) + ackCommentId?: string | number; receivedAt: Date; } const queue: QueuedWebhook[] = []; -export function enqueueWebhook(payload: unknown, eventType?: string): boolean { +export function enqueueWebhook( + payload: unknown, + eventType?: string, + ackCommentId?: string | number, +): boolean { if (queue.length >= MAX_QUEUE_SIZE) { logger.warn('Webhook queue full, rejecting', { queueLength: queue.length, @@ -22,6 +27,7 @@ export function enqueueWebhook(payload: unknown, eventType?: string): boolean { queue.push({ payload, eventType, + ackCommentId, receivedAt: new Date(), }); diff --git a/tests/unit/triggers/webhook-queue.test.ts b/tests/unit/triggers/webhook-queue.test.ts new file mode 100644 index 00000000..534e9c19 --- /dev/null +++ b/tests/unit/triggers/webhook-queue.test.ts @@ -0,0 +1,84 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { processNextQueuedWebhook } from '../../../src/triggers/shared/webhook-queue.js'; +import { clearQueue, enqueueWebhook } from '../../../src/utils/webhookQueue.js'; + +describe('processNextQueuedWebhook', () => { + afterEach(() => { + clearQueue(); + }); + + it('does nothing when queue is empty', () => { + const processWebhook = vi.fn().mockResolvedValue(undefined); + + processNextQueuedWebhook(processWebhook, 'Test'); + + expect(processWebhook).not.toHaveBeenCalled(); + }); + + it('forwards payload and eventType to processWebhook', async () => { + const processWebhook = vi.fn().mockResolvedValue(undefined); + enqueueWebhook({ action: 'test' }, 'issue_comment'); + + processNextQueuedWebhook(processWebhook, 'Test'); + + // processWebhook is called via setImmediate — wait for it + await new Promise((resolve) => setImmediate(resolve)); + + expect(processWebhook).toHaveBeenCalledWith( + { action: 'test' }, + undefined, // eventType comes from getEventType, not the queued entry + undefined, // no ackCommentId + ); + }); + + it('uses getEventType to extract event type from queued entry', async () => { + const processWebhook = vi.fn().mockResolvedValue(undefined); + enqueueWebhook({ action: 'test' }, 'pull_request'); + + processNextQueuedWebhook(processWebhook, 'Test', (entry) => entry.eventType); + + await new Promise((resolve) => setImmediate(resolve)); + + expect(processWebhook).toHaveBeenCalledWith({ action: 'test' }, 'pull_request', undefined); + }); + + it('forwards ackCommentId through the queue', async () => { + const processWebhook = vi.fn().mockResolvedValue(undefined); + enqueueWebhook({ action: 'test' }, 'issue_comment', 'ack-123'); + + processNextQueuedWebhook(processWebhook, 'Test', (entry) => entry.eventType); + + await new Promise((resolve) => setImmediate(resolve)); + + expect(processWebhook).toHaveBeenCalledWith({ action: 'test' }, 'issue_comment', 'ack-123'); + }); + + it('forwards numeric ackCommentId through the queue', async () => { + const processWebhook = vi.fn().mockResolvedValue(undefined); + enqueueWebhook({ action: 'test' }, undefined, 10646); + + processNextQueuedWebhook(processWebhook, 'Test'); + + await new Promise((resolve) => setImmediate(resolve)); + + expect(processWebhook).toHaveBeenCalledWith({ action: 'test' }, undefined, 10646); + }); + + it('processes items in FIFO order preserving ackCommentId', async () => { + const processWebhook = vi.fn().mockResolvedValue(undefined); + enqueueWebhook({ order: 1 }, undefined, 'first-ack'); + enqueueWebhook({ order: 2 }, undefined, 'second-ack'); + + // Process first item + processNextQueuedWebhook(processWebhook, 'Test'); + await new Promise((resolve) => setImmediate(resolve)); + + expect(processWebhook).toHaveBeenCalledWith({ order: 1 }, undefined, 'first-ack'); + + // Process second item + processNextQueuedWebhook(processWebhook, 'Test'); + await new Promise((resolve) => setImmediate(resolve)); + + expect(processWebhook).toHaveBeenCalledWith({ order: 2 }, undefined, 'second-ack'); + }); +}); diff --git a/tests/unit/utils/webhookQueue.test.ts b/tests/unit/utils/webhookQueue.test.ts index a95072bb..0a830a97 100644 --- a/tests/unit/utils/webhookQueue.test.ts +++ b/tests/unit/utils/webhookQueue.test.ts @@ -128,6 +128,41 @@ describe('webhookQueue', () => { }); }); + describe('ackCommentId', () => { + it('preserves string ackCommentId through enqueue/dequeue', () => { + enqueueWebhook({ action: 'test' }, undefined, 'comment-42'); + + const item = dequeueWebhook(); + + expect(item?.ackCommentId).toBe('comment-42'); + }); + + it('preserves numeric ackCommentId through enqueue/dequeue', () => { + enqueueWebhook({ action: 'test' }, undefined, 10646); + + const item = dequeueWebhook(); + + expect(item?.ackCommentId).toBe(10646); + }); + + it('defaults ackCommentId to undefined when not provided', () => { + enqueueWebhook({ action: 'test' }); + + const item = dequeueWebhook(); + + expect(item?.ackCommentId).toBeUndefined(); + }); + + it('preserves ackCommentId alongside eventType', () => { + enqueueWebhook({ action: 'test' }, 'issue_comment', 'ack-99'); + + const item = dequeueWebhook(); + + expect(item?.eventType).toBe('issue_comment'); + expect(item?.ackCommentId).toBe('ack-99'); + }); + }); + describe('getMaxQueueSize', () => { it('returns the maximum queue size', () => { const maxSize = getMaxQueueSize(); From c8de317bdab417901e10f56aff8362f81cad531b Mon Sep 17 00:00:00 2001 From: Zbigniew Sobiecki Date: Sun, 22 Feb 2026 13:18:41 +0000 Subject: [PATCH 4/6] fix(jira): fall back to transition when subtask delete returns 403 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When `deleteIssue()` fails with 403 (missing "Delete Issues" permission), fall back to transitioning the subtask to a terminal status (Cancelled, Won't Do, Rejected, Closed, or Done — in priority order). Also refactor webhook handlers in router/index.ts to extract helper functions, resolving all 3 pre-existing cognitive complexity warnings. Co-Authored-By: Claude Opus 4.6 --- src/pm/jira/adapter.ts | 31 ++- src/router/index.ts | 321 ++++++++++++++++------------- tests/unit/pm/jira/adapter.test.ts | 48 +++++ 3 files changed, 251 insertions(+), 149 deletions(-) diff --git a/src/pm/jira/adapter.ts b/src/pm/jira/adapter.ts index 6b1751b6..3bb983a4 100644 --- a/src/pm/jira/adapter.ts +++ b/src/pm/jira/adapter.ts @@ -279,7 +279,36 @@ export class JiraPMProvider implements PMProvider { async deleteChecklistItem(_workItemId: string, checkItemId: string): Promise { // checkItemId is a JIRA issue key (subtask) - await jiraClient.deleteIssue(checkItemId); + try { + await jiraClient.deleteIssue(checkItemId); + } catch (error) { + const is403 = + error instanceof Error && + (error.message.includes('403') || error.message.includes('Forbidden')); + if (!is403) throw error; + + // Deletion not permitted — transition to a terminal status instead + logger.info('Delete not permitted, transitioning subtask to terminal status', { + issueKey: checkItemId, + }); + const transitions = await jiraClient.getTransitions(checkItemId); + const terminalNames = ['cancelled', "won't do", 'rejected', 'closed', 'done']; + let match: JiraTransition | undefined; + for (const name of terminalNames) { + match = transitions.find((t: JiraTransition) => { + const toName = (t.to?.name ?? '').toLowerCase(); + const tName = (t.name ?? '').toLowerCase(); + return toName === name || tName === name; + }); + if (match) break; + } + if (!match?.id) { + throw new Error( + `Cannot delete subtask ${checkItemId}: deletion returned 403 and no terminal transition found (available: ${transitions.map((t: JiraTransition) => t.name).join(', ')})`, + ); + } + await jiraClient.transitionIssue(checkItemId, match.id); + } } async getAttachments(workItemId: string): Promise { diff --git a/src/router/index.ts b/src/router/index.ts index 26a2a3ee..e9f8d0fc 100644 --- a/src/router/index.ts +++ b/src/router/index.ts @@ -183,6 +183,66 @@ async function tryPostTrelloAck( return commentId ?? undefined; } +async function isSelfAuthoredTrelloComment(payload: unknown, projectId: string): Promise { + const action = (payload as Record).action as Record | undefined; + const commentAuthorId = action?.idMemberCreator as string | undefined; + if (!commentAuthorId) return false; + try { + const botId = await resolveTrelloBotMemberId(projectId); + return !!botId && commentAuthorId === botId; + } catch { + return false; // Identity resolution failed — proceed normally + } +} + +async function processTrelloWebhookEvent( + project: RouterProjectConfig, + cardId: string, + actionType: string, + payload: unknown, +): Promise { + if (actionType === 'commentCard' && (await isSelfAuthoredTrelloComment(payload, project.id))) { + console.log('[Router] Ignoring self-authored Trello comment'); + return; + } + + console.log('[Router] Queueing Trello job:', { actionType, cardId, projectId: project.id }); + + // Fire-and-forget acknowledgment reaction — only for comment actions + if (actionType === 'commentCard') { + void sendAcknowledgeReaction('trello', project.id, payload).catch((err) => + console.error('[Router] Trello reaction error:', err), + ); + } + + // Try to post an ack comment via trigger matching (non-blocking best-effort) + let ackCommentId: string | undefined; + try { + ackCommentId = await tryPostTrelloAck(project.id, cardId, payload); + } catch (err) { + console.warn('[Router] Trello ack comment failed (non-fatal):', String(err)); + } + + const job: CascadeJob = { + type: 'trello', + source: 'trello', + payload, + projectId: project.id, + cardId, + actionType: actionType || 'unknown', + receivedAt: new Date().toISOString(), + ackCommentId, + }; + + try { + const jobId = await addJob(job); + console.log('[Router] Trello job queued:', { jobId, actionType, ackCommentId }); + } catch (err) { + console.error('[Router] Failed to queue Trello job:', err); + // Still return to caller — Trello gets 200 to avoid retries + } +} + /** * Try to match a trigger and post an ack comment for a GitHub webhook. * Returns the ack comment ID if posted, undefined otherwise. @@ -226,6 +286,94 @@ async function tryPostGitHubAck( return commentId ?? undefined; } +async function isSelfAuthoredGitHubComment( + payload: unknown, + repoFullName: string, +): Promise { + const p = payload as Record; + const commentUser = (p.comment as Record | undefined)?.user as + | Record + | undefined; + const login = commentUser?.login as string | undefined; + if (!login) return false; + try { + const project = await findProjectByRepo(repoFullName); + if (!project) return false; + const personas = await resolvePersonaIdentities(project.id); + return isCascadeBot(login, personas); + } catch { + return false; // Persona resolution failed — proceed normally + } +} + +function fireGitHubAckReaction(repoFullName: string, payload: unknown): void { + void (async () => { + try { + const project = await findProjectByRepo(repoFullName); + if (!project) { + console.warn('[Router] No project found for repo, skipping GitHub reaction', { + repoFullName, + }); + return; + } + const personaIdentities = await resolvePersonaIdentities(project.id); + await sendAcknowledgeReaction('github', repoFullName, payload, personaIdentities, project); + } catch (err) { + console.warn('[Router] GitHub reaction error:', String(err)); + } + })(); +} + +async function processGitHubWebhookEvent( + eventType: string, + repoFullName: string, + payload: unknown, +): Promise { + const isCommentEvent = + eventType === 'issue_comment' || eventType === 'pull_request_review_comment'; + + if (isCommentEvent && (await isSelfAuthoredGitHubComment(payload, repoFullName))) { + console.log('[Router] Ignoring self-authored GitHub comment'); + return; + } + + console.log('[Router] Queueing GitHub job:', { eventType, repoFullName }); + + // Fire-and-forget acknowledgment reaction — only for comment events that @mention the bot + if (isCommentEvent) { + fireGitHubAckReaction(repoFullName, payload); + } + + // Try to post an ack comment via trigger matching (non-blocking best-effort) + let ackCommentId: number | undefined; + try { + ackCommentId = await tryPostGitHubAck(eventType, repoFullName, payload); + } catch (err) { + console.warn('[Router] GitHub ack comment failed (non-fatal):', String(err)); + } + + const job: CascadeJob = { + type: 'github', + source: 'github', + payload, + eventType, + repoFullName, + receivedAt: new Date().toISOString(), + ackCommentId, + }; + + // Fire pre-actions (non-blocking) before queueing + const p = payload as Record; + firePreActions(job as GitHubJob, p); + + try { + const jobId = await addJob(job); + console.log('[Router] GitHub job queued:', { jobId, eventType, ackCommentId }); + } catch (err) { + console.error('[Router] Failed to queue GitHub job:', err); + } +} + /** * Try to match a trigger and post an ack comment for a JIRA webhook. * Returns the ack comment ID if posted, undefined otherwise. @@ -250,6 +398,25 @@ async function tryPostJiraAck( return commentId ?? undefined; } +async function isSelfAuthoredJiraComment( + webhookEvent: string, + payload: unknown, + projectId: string, +): Promise { + if (!webhookEvent.startsWith('comment_')) return false; + const p = payload as Record; + const comment = p.comment as Record | undefined; + const author = comment?.author as Record | undefined; + const commentAuthorId = author?.accountId as string | undefined; + if (!commentAuthorId) return false; + try { + const botId = await resolveJiraBotAccountId(projectId); + return !!botId && commentAuthorId === botId; + } catch { + return false; // Identity resolution failed — proceed normally + } +} + /** * Fire non-blocking pre-actions for a GitHub job before it is queued. * Currently adds a 👀 reaction for first-time check_suite success events. @@ -364,60 +531,7 @@ app.post('/trello/webhook', async (c) => { }); if (shouldProcess && project && cardId) { - // Skip self-authored Trello comments to prevent infinite loops - if (actionType === 'commentCard') { - const action = (payload as Record).action as - | Record - | undefined; - const commentAuthorId = action?.idMemberCreator as string | undefined; - if (commentAuthorId) { - try { - const botId = await resolveTrelloBotMemberId(project.id); - if (botId && commentAuthorId === botId) { - console.log('[Router] Ignoring self-authored Trello comment'); - return c.text('OK', 200); - } - } catch { - // Identity resolution failed — proceed normally - } - } - } - - console.log('[Router] Queueing Trello job:', { actionType, cardId, projectId: project.id }); - - // Fire-and-forget acknowledgment reaction — only for comment actions - if (actionType === 'commentCard') { - void sendAcknowledgeReaction('trello', project.id, payload).catch((err) => - console.error('[Router] Trello reaction error:', err), - ); - } - - // Try to post an ack comment via trigger matching (non-blocking best-effort) - let ackCommentId: string | undefined; - try { - ackCommentId = await tryPostTrelloAck(project.id, cardId, payload); - } catch (err) { - console.warn('[Router] Trello ack comment failed (non-fatal):', String(err)); - } - - const job: CascadeJob = { - type: 'trello', - source: 'trello', - payload, - projectId: project.id, - cardId, - actionType: actionType || 'unknown', - receivedAt: new Date().toISOString(), - ackCommentId, - }; - - try { - const jobId = await addJob(job); - console.log('[Router] Trello job queued:', { jobId, actionType, ackCommentId }); - } catch (err) { - console.error('[Router] Failed to queue Trello job:', err); - // Still return 200 to Trello to avoid retries - } + await processTrelloWebhookEvent(project, cardId, actionType || 'unknown', payload); } else { console.log(`[Router] Ignoring Trello: ${actionType || 'unknown'}`); } @@ -507,82 +621,7 @@ app.post('/github/webhook', async (c) => { }); if (shouldProcess) { - // Skip self-authored GitHub comments to prevent infinite loops - if (eventType === 'issue_comment' || eventType === 'pull_request_review_comment') { - const commentLogin = (p.comment as Record | undefined)?.user as - | Record - | undefined; - const login = commentLogin?.login as string | undefined; - if (login) { - try { - const project = await findProjectByRepo(repoFullName); - if (project) { - const personas = await resolvePersonaIdentities(project.id); - if (isCascadeBot(login, personas)) { - console.log('[Router] Ignoring self-authored GitHub comment'); - return c.text('OK', 200); - } - } - } catch { - // Persona resolution failed — proceed normally - } - } - } - - console.log('[Router] Queueing GitHub job:', { eventType, repoFullName }); - - // Fire-and-forget acknowledgment reaction — only for comment events that @mention the bot - if (eventType === 'issue_comment' || eventType === 'pull_request_review_comment') { - void (async () => { - try { - const project = await findProjectByRepo(repoFullName); - if (!project) { - console.warn('[Router] No project found for repo, skipping GitHub reaction', { - repoFullName, - }); - return; - } - const personaIdentities = await resolvePersonaIdentities(project.id); - await sendAcknowledgeReaction( - 'github', - repoFullName, - payload, - personaIdentities, - project, - ); - } catch (err) { - console.warn('[Router] GitHub reaction error:', String(err)); - } - })(); - } - - // Try to post an ack comment via trigger matching (non-blocking best-effort) - let ackCommentId: number | undefined; - try { - ackCommentId = await tryPostGitHubAck(eventType, repoFullName, payload); - } catch (err) { - console.warn('[Router] GitHub ack comment failed (non-fatal):', String(err)); - } - - const job: CascadeJob = { - type: 'github', - source: 'github', - payload, - eventType, - repoFullName, - receivedAt: new Date().toISOString(), - ackCommentId, - }; - - // Fire pre-actions (non-blocking) before queueing - firePreActions(job as GitHubJob, p); - - try { - const jobId = await addJob(job); - console.log('[Router] GitHub job queued:', { jobId, eventType, ackCommentId }); - } catch (err) { - console.error('[Router] Failed to queue GitHub job:', err); - } + await processGitHubWebhookEvent(eventType, repoFullName, payload); } else { console.log('[Router] Ignoring GitHub event:', eventType); } @@ -651,25 +690,11 @@ app.post('/jira/webhook', async (c) => { }); if (shouldProcess && project) { - // Skip self-authored JIRA comments to prevent infinite loops - if (webhookEvent.startsWith('comment_')) { - const comment = p.comment as Record | undefined; - const author = comment?.author as Record | undefined; - const commentAuthorId = author?.accountId as string | undefined; - if (commentAuthorId) { - try { - const botId = await resolveJiraBotAccountId(project.id); - if (botId && commentAuthorId === botId) { - console.log('[Router] Ignoring self-authored JIRA comment'); - return c.text('OK', 200); - } - } catch { - // Identity resolution failed — proceed normally - } - } + if (await isSelfAuthoredJiraComment(webhookEvent, payload, project.id)) { + console.log('[Router] Ignoring self-authored JIRA comment'); + } else { + await queueJiraJob(project, issueKey, webhookEvent, payload, config.fullProjects); } - - await queueJiraJob(project, issueKey, webhookEvent, payload, config.fullProjects); } else { console.log(`[Router] Ignoring JIRA: ${webhookEvent}`); } diff --git a/tests/unit/pm/jira/adapter.test.ts b/tests/unit/pm/jira/adapter.test.ts index 4f4ad511..d6a93135 100644 --- a/tests/unit/pm/jira/adapter.test.ts +++ b/tests/unit/pm/jira/adapter.test.ts @@ -580,6 +580,54 @@ describe('JiraPMProvider', () => { expect(mockJiraClient.deleteIssue).toHaveBeenCalledWith('PROJ-5'); }); + + it('falls back to transition when deleteIssue returns 403', async () => { + mockJiraClient.deleteIssue.mockRejectedValue(new Error('Request failed with status 403')); + mockJiraClient.getTransitions.mockResolvedValue([ + { id: 't-1', name: 'In Progress', to: { name: 'In Progress' } }, + { id: 't-2', name: 'Cancelled', to: { name: 'Cancelled' } }, + ]); + mockJiraClient.transitionIssue.mockResolvedValue(undefined); + + await provider.deleteChecklistItem('PROJ-1', 'PROJ-5'); + + expect(mockJiraClient.transitionIssue).toHaveBeenCalledWith('PROJ-5', 't-2'); + }); + + it('tries terminal statuses in priority order (cancelled preferred over done)', async () => { + mockJiraClient.deleteIssue.mockRejectedValue(new Error('403 Forbidden')); + mockJiraClient.getTransitions.mockResolvedValue([ + { id: 't-done', name: 'Done', to: { name: 'Done' } }, + { id: 't-cancel', name: 'Cancel', to: { name: 'Cancelled' } }, + ]); + mockJiraClient.transitionIssue.mockResolvedValue(undefined); + + await provider.deleteChecklistItem('PROJ-1', 'PROJ-5'); + + // Should pick "Cancelled" (higher priority) over "Done" + expect(mockJiraClient.transitionIssue).toHaveBeenCalledWith('PROJ-5', 't-cancel'); + }); + + it('throws when no terminal transition available after 403', async () => { + mockJiraClient.deleteIssue.mockRejectedValue(new Error('403 Forbidden')); + mockJiraClient.getTransitions.mockResolvedValue([ + { id: 't-1', name: 'In Progress', to: { name: 'In Progress' } }, + { id: 't-2', name: 'In Review', to: { name: 'In Review' } }, + ]); + + await expect(provider.deleteChecklistItem('PROJ-1', 'PROJ-5')).rejects.toThrow( + 'Cannot delete subtask PROJ-5: deletion returned 403 and no terminal transition found', + ); + }); + + it('re-throws non-403 errors without fallback', async () => { + mockJiraClient.deleteIssue.mockRejectedValue(new Error('500 Internal Server Error')); + + await expect(provider.deleteChecklistItem('PROJ-1', 'PROJ-5')).rejects.toThrow( + '500 Internal Server Error', + ); + expect(mockJiraClient.getTransitions).not.toHaveBeenCalled(); + }); }); describe('getAttachments', () => { From 620c9434bbaca4586fe4a57a87734a68a5ae95dc Mon Sep 17 00:00:00 2001 From: Zbigniew Sobiecki Date: Sun, 22 Feb 2026 14:43:22 +0000 Subject: [PATCH 5/6] refactor(pm): clean up pm integration layer remove dead code, improve type safety, and eliminate duplication across the pm integration subsystem. changes: - remove unused getTriggerHandlers() from interface - remove unused withCredentials() from registry - change createProvider to accept ProjectConfig (removes unsafe PMProviderConfig casts) - change TriggerResult.agentType to string | null - delete factory.ts (inline into index.ts) - delegate ack/reaction methods in trello and jira integrations to existing router functions - consolidate resolveLifecycleConfig into registry - replace redundant createPMProvider calls with getPMProvider() in webhook-handler - migrate TriggerResult.cardId to workItemId Co-Authored-By: Claude Opus 4.6 --- src/agents/shared/promptContext.ts | 7 +- src/api/routers/webhooks.ts | 19 +- src/backends/secretBuilder.ts | 12 +- src/gadgets/index.ts | 12 - src/gadgets/trello/AddChecklistToCard.ts | 52 --- src/gadgets/trello/CreateTrelloCard.ts | 52 --- src/gadgets/trello/GetMyRecentActivity.ts | 82 ----- src/gadgets/trello/ListTrelloCards.ts | 22 -- src/gadgets/trello/PostTrelloComment.ts | 26 -- src/gadgets/trello/ReadTrelloCard.ts | 34 -- src/gadgets/trello/UpdateChecklistItem.ts | 36 -- src/gadgets/trello/UpdateTrelloCard.ts | 52 --- src/gadgets/trello/core/addChecklist.ts | 22 -- src/gadgets/trello/core/createCard.ts | 21 -- src/gadgets/trello/core/listCards.ts | 27 -- src/gadgets/trello/core/postComment.ts | 11 - src/gadgets/trello/core/readCard.ts | 83 ----- src/gadgets/trello/core/updateCard.ts | 39 -- .../trello/core/updateChecklistItem.ts | 17 - src/gadgets/trello/index.ts | 8 - src/pm/config.ts | 75 ++++ src/pm/factory.ts | 25 -- src/pm/index.ts | 19 +- src/pm/integration.ts | 72 ++++ src/pm/jira/integration.ts | 136 +++++++ src/pm/lifecycle.ts | 37 +- src/pm/registry.ts | 53 +++ src/pm/trello/adapter.ts | 1 + src/pm/trello/integration.ts | 119 +++++++ src/pm/webhook-handler.ts | 211 +++++++++++ src/router/acknowledgments.ts | 7 +- src/router/config.ts | 41 ++- src/router/reactions.ts | 3 +- src/triggers/github/check-suite-failure.ts | 2 +- src/triggers/github/check-suite-success.ts | 2 +- src/triggers/github/pr-comment-mention.ts | 2 +- src/triggers/github/pr-merged.ts | 58 +-- src/triggers/github/pr-opened.ts | 2 +- src/triggers/github/pr-ready-to-merge.ts | 60 ++-- src/triggers/github/pr-review-submitted.ts | 2 +- src/triggers/github/review-requested.ts | 1 - src/triggers/github/webhook-handler.ts | 40 +-- src/triggers/jira/comment-mention.ts | 4 +- src/triggers/jira/issue-transitioned.ts | 8 +- src/triggers/jira/label-added.ts | 10 +- src/triggers/jira/webhook-handler.ts | 214 +---------- src/triggers/shared/agent-execution.ts | 24 +- src/triggers/shared/agent-result-handler.ts | 12 +- src/triggers/shared/budget.ts | 11 +- src/triggers/trello/card-moved.ts | 8 +- src/triggers/trello/comment-mention.ts | 7 +- src/triggers/trello/label-added.ts | 12 +- src/triggers/trello/webhook-handler.ts | 204 +---------- src/types/index.ts | 5 +- tests/unit/gadgets/trello.test.ts | 136 ------- tests/unit/gadgets/trello/core.test.ts | 333 ------------------ tests/unit/pm/factory.test.ts | 26 +- tests/unit/pm/lifecycle.test.ts | 32 +- tests/unit/triggers/agent-execution.test.ts | 6 +- tests/unit/triggers/card-moved.test.ts | 37 +- .../unit/triggers/check-suite-failure.test.ts | 2 +- .../unit/triggers/check-suite-success.test.ts | 2 +- .../github-pr-comment-mention.test.ts | 2 +- .../triggers/jira-comment-mention.test.ts | 1 - .../triggers/jira-issue-transitioned.test.ts | 1 - tests/unit/triggers/jira-label-added.test.ts | 34 +- tests/unit/triggers/label-added.test.ts | 48 ++- tests/unit/triggers/pr-merged.test.ts | 91 +++-- tests/unit/triggers/pr-opened.test.ts | 2 +- tests/unit/triggers/pr-ready-to-merge.test.ts | 103 ++++-- .../unit/triggers/pr-review-submitted.test.ts | 2 +- tests/unit/triggers/review-requested.test.ts | 2 +- .../triggers/trello-comment-mention.test.ts | 2 +- 73 files changed, 1174 insertions(+), 1809 deletions(-) delete mode 100644 src/gadgets/trello/AddChecklistToCard.ts delete mode 100644 src/gadgets/trello/CreateTrelloCard.ts delete mode 100644 src/gadgets/trello/GetMyRecentActivity.ts delete mode 100644 src/gadgets/trello/ListTrelloCards.ts delete mode 100644 src/gadgets/trello/PostTrelloComment.ts delete mode 100644 src/gadgets/trello/ReadTrelloCard.ts delete mode 100644 src/gadgets/trello/UpdateChecklistItem.ts delete mode 100644 src/gadgets/trello/UpdateTrelloCard.ts delete mode 100644 src/gadgets/trello/core/addChecklist.ts delete mode 100644 src/gadgets/trello/core/createCard.ts delete mode 100644 src/gadgets/trello/core/listCards.ts delete mode 100644 src/gadgets/trello/core/postComment.ts delete mode 100644 src/gadgets/trello/core/readCard.ts delete mode 100644 src/gadgets/trello/core/updateCard.ts delete mode 100644 src/gadgets/trello/core/updateChecklistItem.ts delete mode 100644 src/gadgets/trello/index.ts create mode 100644 src/pm/config.ts delete mode 100644 src/pm/factory.ts create mode 100644 src/pm/integration.ts create mode 100644 src/pm/jira/integration.ts create mode 100644 src/pm/registry.ts create mode 100644 src/pm/trello/integration.ts create mode 100644 src/pm/webhook-handler.ts delete mode 100644 tests/unit/gadgets/trello.test.ts delete mode 100644 tests/unit/gadgets/trello/core.test.ts diff --git a/src/agents/shared/promptContext.ts b/src/agents/shared/promptContext.ts index 2d6beabc..c75252a6 100644 --- a/src/agents/shared/promptContext.ts +++ b/src/agents/shared/promptContext.ts @@ -1,3 +1,4 @@ +import { getTrelloConfig } from '../../pm/config.js'; import { getPMProvider } from '../../pm/index.js'; import type { ProjectConfig } from '../../types/index.js'; import type { PromptContext } from '../prompts/index.js'; @@ -29,8 +30,8 @@ export function buildPromptContext( cardUrl: cardId ? pmProvider.getWorkItemUrl(cardId) : undefined, projectId: project.id, baseBranch: project.baseBranch, - storiesListId: project.trello?.lists?.stories, - processedLabelId: project.trello?.labels?.processed, + storiesListId: getTrelloConfig(project)?.lists?.stories, + processedLabelId: getTrelloConfig(project)?.labels?.processed, pmType: pmProvider.type, workItemNoun: isJira ? 'issue' : 'card', workItemNounPlural: isJira ? 'issues' : 'cards', @@ -50,7 +51,7 @@ export function buildPromptContext( originalCardName: debugContext.originalCardName, originalCardUrl: debugContext.originalCardUrl, detectedAgentType: debugContext.detectedAgentType, - debugListId: project.trello?.lists?.debug, + debugListId: getTrelloConfig(project)?.lists?.debug, }), }; } diff --git a/src/api/routers/webhooks.ts b/src/api/routers/webhooks.ts index 2c0da047..df218c1c 100644 --- a/src/api/routers/webhooks.ts +++ b/src/api/routers/webhooks.ts @@ -6,6 +6,7 @@ import { getAllProjectCredentials } from '../../config/provider.js'; import { getDb } from '../../db/client.js'; import { findProjectByIdFromDb } from '../../db/repositories/configRepository.js'; import { projects } from '../../db/schema/index.js'; +import { getJiraConfig, getTrelloConfig } from '../../pm/config.js'; import { parseRepoFullName } from '../../utils/repo.js'; import { protectedProcedure, router } from '../trpc.js'; @@ -78,12 +79,14 @@ async function resolveProjectContext( const creds = await getAllProjectCredentials(projectId); // Resolve JIRA label names from config (with defaults) - const jiraLabels = project.jira + const jiraConfig = getJiraConfig(project); + const trelloConfig = getTrelloConfig(project); + const jiraLabels = jiraConfig ? [ - project.jira.labels?.processing ?? 'cascade-processing', - project.jira.labels?.processed ?? 'cascade-processed', - project.jira.labels?.error ?? 'cascade-error', - project.jira.labels?.readyToProcess ?? 'cascade-ready', + jiraConfig.labels?.processing ?? 'cascade-processing', + jiraConfig.labels?.processed ?? 'cascade-processed', + jiraConfig.labels?.error ?? 'cascade-error', + jiraConfig.labels?.readyToProcess ?? 'cascade-ready', ] : undefined; @@ -92,9 +95,9 @@ async function resolveProjectContext( orgId: project.orgId, repo: project.repo, pmType: project.pm?.type ?? 'trello', - boardId: project.trello?.boardId, - jiraBaseUrl: project.jira?.baseUrl, - jiraProjectKey: project.jira?.projectKey, + boardId: trelloConfig?.boardId, + jiraBaseUrl: jiraConfig?.baseUrl, + jiraProjectKey: jiraConfig?.projectKey, jiraLabels, trelloApiKey: creds.TRELLO_API_KEY ?? '', trelloToken: creds.TRELLO_TOKEN ?? '', diff --git a/src/backends/secretBuilder.ts b/src/backends/secretBuilder.ts index 58666d62..682136ac 100644 --- a/src/backends/secretBuilder.ts +++ b/src/backends/secretBuilder.ts @@ -1,5 +1,6 @@ import { getAllProjectCredentials } from '../config/provider.js'; import { getPersonaToken } from '../github/personas.js'; +import { getJiraConfig } from '../pm/config.js'; import type { AgentInput, ProjectConfig } from '../types/index.js'; import { parseRepoFullName } from '../utils/repo.js'; import type { AgentProfile } from './agent-profiles.js'; @@ -41,11 +42,12 @@ export async function augmentProjectSecrets( } // Inject JIRA integration config so cascade-tools can construct JiraPMProvider - if (project.jira) { - projectSecrets.CASCADE_JIRA_PROJECT_KEY = project.jira.projectKey; - projectSecrets.CASCADE_JIRA_BASE_URL = project.jira.baseUrl; - if (project.jira.statuses) { - projectSecrets.CASCADE_JIRA_STATUSES = JSON.stringify(project.jira.statuses); + const jiraConfig = getJiraConfig(project); + if (jiraConfig) { + projectSecrets.CASCADE_JIRA_PROJECT_KEY = jiraConfig.projectKey; + projectSecrets.CASCADE_JIRA_BASE_URL = jiraConfig.baseUrl; + if (jiraConfig.statuses) { + projectSecrets.CASCADE_JIRA_STATUSES = JSON.stringify(jiraConfig.statuses); } } diff --git a/src/gadgets/index.ts b/src/gadgets/index.ts index 3e2bb328..bc147e29 100644 --- a/src/gadgets/index.ts +++ b/src/gadgets/index.ts @@ -9,17 +9,5 @@ export { VerifyChanges } from './VerifyChanges.js'; // Search gadgets export { RipGrep } from './RipGrep.js'; export { AstGrep } from './AstGrep.js'; - -// Trello gadgets -export { - ReadTrelloCard, - PostTrelloComment, - UpdateTrelloCard, - CreateTrelloCard, - ListTrelloCards, - GetMyRecentActivity, - AddChecklistToCard, -} from './trello/index.js'; - // GitHub gadgets export { GetPRDetails, GetPRComments, ReplyToReviewComment } from './github/index.js'; diff --git a/src/gadgets/trello/AddChecklistToCard.ts b/src/gadgets/trello/AddChecklistToCard.ts deleted file mode 100644 index 75d6c240..00000000 --- a/src/gadgets/trello/AddChecklistToCard.ts +++ /dev/null @@ -1,52 +0,0 @@ -import { Gadget, z } from 'llmist'; -import { addChecklist } from './core/addChecklist.js'; - -export class AddChecklistToCard extends Gadget({ - name: 'AddChecklistToCard', - description: - 'Add a checklist with items to a Trello card. Use this to create interactive checklists for acceptance criteria or implementation steps.', - timeoutMs: 30000, - schema: z.object({ - cardId: z.string().describe('The Trello card ID'), - checklistName: z - .string() - .describe( - 'Name of the checklist (e.g., "✅ Acceptance Criteria" or "📋 Implementation Steps")', - ), - items: z.array(z.string()).min(1).describe('List of checklist items to add'), - }), - examples: [ - { - params: { - cardId: 'abc123', - checklistName: '✅ Acceptance Criteria', - items: [ - 'User can request password reset via email', - 'Reset link expires after 24 hours', - 'User must set a new password meeting security requirements', - ], - }, - comment: 'Add acceptance criteria checklist to a story card', - }, - { - params: { - cardId: 'abc123', - checklistName: '📋 Implementation Steps', - items: [ - 'Add reset password endpoint to API', - 'Create email template for reset link', - 'Add password validation logic', - ], - }, - comment: 'Add implementation steps checklist to a story card', - }, - ], -}) { - override async execute(params: this['params']): Promise { - return addChecklist({ - cardId: params.cardId, - checklistName: params.checklistName, - items: params.items, - }); - } -} diff --git a/src/gadgets/trello/CreateTrelloCard.ts b/src/gadgets/trello/CreateTrelloCard.ts deleted file mode 100644 index 482b4e6d..00000000 --- a/src/gadgets/trello/CreateTrelloCard.ts +++ /dev/null @@ -1,52 +0,0 @@ -import { Gadget, z } from 'llmist'; -import { createCard } from './core/createCard.js'; - -export class CreateTrelloCard extends Gadget({ - name: 'CreateTrelloCard', - description: - 'Create a new Trello card in a specific list. Use this to create user story cards or break down work into smaller tasks.', - timeoutMs: 30000, - schema: z.object({ - listId: z.string().describe('The Trello list ID where the card should be created'), - title: z - .string() - .max(200) - .describe( - 'Card title. For user stories, use format: "As a [role], I want [action] so that [benefit]"', - ), - description: z - .string() - .optional() - .describe( - 'Card description (markdown supported). Include acceptance criteria and technical notes.', - ), - }), - examples: [ - { - params: { - listId: 'abc123', - title: 'As a user, I want to reset my password so that I can recover my account', - description: - '## Acceptance Criteria\n\n- [ ] User can request password reset via email\n- [ ] Reset link expires after 24 hours\n- [ ] User must set a new password meeting security requirements\n\n## Technical Notes\n\n- Use existing email service\n- Store reset tokens in database with expiry', - }, - comment: 'Create an INVEST-compatible user story card', - }, - { - params: { - listId: 'abc123', - title: 'Add email validation to signup form', - description: - '## Acceptance Criteria\n\n- [ ] Email format is validated on blur\n- [ ] Error message is shown for invalid emails', - }, - comment: 'Create a simple task card', - }, - ], -}) { - override async execute(params: this['params']): Promise { - return createCard({ - listId: params.listId, - title: params.title, - description: params.description, - }); - } -} diff --git a/src/gadgets/trello/GetMyRecentActivity.ts b/src/gadgets/trello/GetMyRecentActivity.ts deleted file mode 100644 index 7a0f25d2..00000000 --- a/src/gadgets/trello/GetMyRecentActivity.ts +++ /dev/null @@ -1,82 +0,0 @@ -import { Gadget, z } from 'llmist'; -import { trelloClient } from '../../trello/client.js'; -import { formatGadgetError } from '../utils.js'; - -export class GetMyRecentActivity extends Gadget({ - name: 'GetMyRecentActivity', - description: - 'Get your recent Trello activity (cards created, updated, comments posted). Use this to find cards you recently worked on or to understand context when the user refers to previous work.', - timeoutMs: 30000, - schema: z.object({ - limit: z - .number() - .int() - .min(1) - .max(50) - .optional() - .default(20) - .describe('Number of recent actions to retrieve (default 20, max 50)'), - }), - examples: [ - { - params: { limit: 10 }, - comment: 'Get my last 10 actions to find cards I recently created', - }, - { - params: { limit: 20 }, - comment: 'Get my recent activity with default limit of 20', - }, - ], -}) { - override async execute(params: this['params']): Promise { - try { - const actions = await trelloClient.getMyActions(params.limit); - - if (actions.length === 0) { - return 'No recent activity found.'; - } - - let result = `# My Recent Activity (${actions.length} actions)\n\n`; - - for (const action of actions) { - const date = new Date(action.date).toISOString(); - const actionDesc = this.formatAction(action); - result += `- **${action.type}** (${date}): ${actionDesc}\n`; - } - - return result; - } catch (error) { - return formatGadgetError('getting recent activity', error); - } - } - - private formatAction(action: { - type: string; - data: { - card?: { id: string; name: string; shortLink?: string }; - list?: { name: string }; - text?: string; - }; - }): string { - const card = action.data.card; - const list = action.data.list; - - switch (action.type) { - case 'createCard': - return `Created card "${card?.name}" (ID: ${card?.id})${list ? ` in list "${list.name}"` : ''}`; - case 'updateCard': - return `Updated card "${card?.name}" (ID: ${card?.id})`; - case 'commentCard': - return `Commented on "${card?.name}" (ID: ${card?.id}): "${action.data.text?.slice(0, 50)}${(action.data.text?.length || 0) > 50 ? '...' : ''}"`; - case 'addLabelToCard': - return `Added label to "${card?.name}" (ID: ${card?.id})`; - case 'removeLabelFromCard': - return `Removed label from "${card?.name}" (ID: ${card?.id})`; - case 'moveCardToBoard': - case 'moveCardFromBoard': - return `Moved card "${card?.name}" (ID: ${card?.id})`; - default: - return card ? `"${card.name}" (ID: ${card.id})` : 'Unknown action'; - } - } -} diff --git a/src/gadgets/trello/ListTrelloCards.ts b/src/gadgets/trello/ListTrelloCards.ts deleted file mode 100644 index 5b1b28d4..00000000 --- a/src/gadgets/trello/ListTrelloCards.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { Gadget, z } from 'llmist'; -import { listCards } from './core/listCards.js'; - -export class ListTrelloCards extends Gadget({ - name: 'ListTrelloCards', - description: - 'List all cards on a Trello list. Use this to see cards you created or to find cards to update.', - timeoutMs: 30000, - schema: z.object({ - listId: z.string().describe('The Trello list ID'), - }), - examples: [ - { - params: { listId: 'abc123' }, - comment: 'List all cards in the STORIES list to find ones to update', - }, - ], -}) { - override async execute(params: this['params']): Promise { - return listCards(params.listId); - } -} diff --git a/src/gadgets/trello/PostTrelloComment.ts b/src/gadgets/trello/PostTrelloComment.ts deleted file mode 100644 index 95ffc2f2..00000000 --- a/src/gadgets/trello/PostTrelloComment.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { Gadget, z } from 'llmist'; -import { postComment } from './core/postComment.js'; - -export class PostTrelloComment extends Gadget({ - name: 'PostTrelloComment', - description: - 'Post a comment to a Trello card. Use this to communicate with the user, ask questions, or provide status updates.', - timeoutMs: 30000, - schema: z.object({ - cardId: z.string().describe('The Trello card ID'), - text: z.string().describe('The comment text to post (supports markdown)'), - }), - examples: [ - { - params: { - cardId: 'abc123', - text: '📋 **Brief Ready for Review**\n\nI have analyzed the codebase and updated the card description.', - }, - comment: 'Post a status update to the card', - }, - ], -}) { - override async execute(params: this['params']): Promise { - return postComment(params.cardId, params.text); - } -} diff --git a/src/gadgets/trello/ReadTrelloCard.ts b/src/gadgets/trello/ReadTrelloCard.ts deleted file mode 100644 index cf4527b3..00000000 --- a/src/gadgets/trello/ReadTrelloCard.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { Gadget, z } from 'llmist'; -import { readCard } from './core/readCard.js'; - -export class ReadTrelloCard extends Gadget({ - name: 'ReadTrelloCard', - description: - 'Read a Trello card to retrieve its title, description, comments, checklists, and attachments. Use this to understand the current state of the card before making changes.', - timeoutMs: 30000, - schema: z.object({ - cardId: z.string().describe('The Trello card ID'), - includeComments: z - .boolean() - .optional() - .default(true) - .describe('Whether to include comments in the response'), - }), - examples: [ - { - params: { cardId: 'abc123', includeComments: true }, - comment: 'Read the card with its comments to understand context', - }, - { - params: { cardId: 'abc123', includeComments: false }, - comment: 'Read just the card title and description', - }, - ], -}) { - override async execute(params: this['params']): Promise { - return readCard(params.cardId, params.includeComments); - } -} - -/** @deprecated Use readCard from './core/readCard.js' instead */ -export { readCard as formatCardData } from './core/readCard.js'; diff --git a/src/gadgets/trello/UpdateChecklistItem.ts b/src/gadgets/trello/UpdateChecklistItem.ts deleted file mode 100644 index 5d75faa5..00000000 --- a/src/gadgets/trello/UpdateChecklistItem.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { Gadget, z } from 'llmist'; -import { updateChecklistItem } from './core/updateChecklistItem.js'; - -export class UpdateChecklistItem extends Gadget({ - name: 'UpdateChecklistItem', - description: - 'Update a checklist item state on a Trello card. Use this to mark acceptance criteria as complete or incomplete.', - timeoutMs: 15000, - schema: z.object({ - cardId: z.string().describe('The Trello card ID'), - checkItemId: z.string().describe('The checklist item ID to update'), - state: z.enum(['complete', 'incomplete']).describe('The new state for the checklist item'), - }), - examples: [ - { - params: { - cardId: 'abc123', - checkItemId: 'item456', - state: 'complete', - }, - comment: 'Mark an acceptance criterion as complete', - }, - { - params: { - cardId: 'abc123', - checkItemId: 'item789', - state: 'incomplete', - }, - comment: 'Mark an acceptance criterion as incomplete', - }, - ], -}) { - override async execute(params: this['params']): Promise { - return updateChecklistItem(params.cardId, params.checkItemId, params.state); - } -} diff --git a/src/gadgets/trello/UpdateTrelloCard.ts b/src/gadgets/trello/UpdateTrelloCard.ts deleted file mode 100644 index 30549f96..00000000 --- a/src/gadgets/trello/UpdateTrelloCard.ts +++ /dev/null @@ -1,52 +0,0 @@ -import { Gadget, z } from 'llmist'; -import { updateCard } from './core/updateCard.js'; - -export class UpdateTrelloCard extends Gadget({ - name: 'UpdateTrelloCard', - description: - 'Update a Trello card title and/or description. Use this to save your analysis, brief, or plan to the card.', - timeoutMs: 30000, - schema: z.object({ - cardId: z.string().describe('The Trello card ID'), - title: z - .string() - .max(80) - .optional() - .describe('New card title (max 80 chars). Should be action-oriented.'), - description: z - .string() - .optional() - .describe( - 'New card description (markdown supported). Use this to save the full brief or plan.', - ), - addLabelIds: z - .array(z.string()) - .optional() - .describe('Label IDs to add to the card (e.g., for marking as processed)'), - }), - examples: [ - { - params: { - cardId: 'abc123', - description: '## Context\n\nBackground info...\n\n## Requirements\n\n- Item 1\n- Item 2', - }, - comment: 'Update the card description with a structured brief', - }, - { - params: { - cardId: 'abc123', - title: 'Add user authentication flow', - }, - comment: 'Update just the card title', - }, - ], -}) { - override async execute(params: this['params']): Promise { - return updateCard({ - cardId: params.cardId, - title: params.title, - description: params.description, - addLabelIds: params.addLabelIds, - }); - } -} diff --git a/src/gadgets/trello/core/addChecklist.ts b/src/gadgets/trello/core/addChecklist.ts deleted file mode 100644 index c3f36006..00000000 --- a/src/gadgets/trello/core/addChecklist.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { trelloClient } from '../../../trello/client.js'; - -export interface AddChecklistParams { - cardId: string; - checklistName: string; - items: string[]; -} - -export async function addChecklist(params: AddChecklistParams): Promise { - try { - const checklist = await trelloClient.createChecklist(params.cardId, params.checklistName); - - for (const item of params.items) { - await trelloClient.addChecklistItem(checklist.id, item); - } - - return `Checklist "${params.checklistName}" created with ${params.items.length} items on card ${params.cardId}`; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - return `Error adding checklist: ${message}`; - } -} diff --git a/src/gadgets/trello/core/createCard.ts b/src/gadgets/trello/core/createCard.ts deleted file mode 100644 index 90c05ad1..00000000 --- a/src/gadgets/trello/core/createCard.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { trelloClient } from '../../../trello/client.js'; - -export interface CreateCardParams { - listId: string; - title: string; - description?: string; -} - -export async function createCard(params: CreateCardParams): Promise { - try { - const card = await trelloClient.createCard(params.listId, { - name: params.title, - desc: params.description, - }); - - return `Card created successfully: "${card.name}" - ${card.shortUrl}`; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - return `Error creating card: ${message}`; - } -} diff --git a/src/gadgets/trello/core/listCards.ts b/src/gadgets/trello/core/listCards.ts deleted file mode 100644 index 1da8a3f9..00000000 --- a/src/gadgets/trello/core/listCards.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { trelloClient } from '../../../trello/client.js'; - -export async function listCards(listId: string): Promise { - try { - const cards = await trelloClient.getListCards(listId); - - if (cards.length === 0) { - return 'No cards found in this list.'; - } - - let result = `# Cards (${cards.length})\n\n`; - for (const card of cards) { - result += `## ${card.name}\n`; - result += `- **ID:** ${card.id}\n`; - result += `- **URL:** ${card.shortUrl}\n`; - if (card.desc) { - result += `- **Description:** ${card.desc.slice(0, 100)}${card.desc.length > 100 ? '...' : ''}\n`; - } - result += '\n'; - } - - return result; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - return `Error listing cards: ${message}`; - } -} diff --git a/src/gadgets/trello/core/postComment.ts b/src/gadgets/trello/core/postComment.ts deleted file mode 100644 index d660fbac..00000000 --- a/src/gadgets/trello/core/postComment.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { trelloClient } from '../../../trello/client.js'; - -export async function postComment(cardId: string, text: string): Promise { - try { - await trelloClient.addComment(cardId, text); - return 'Comment posted successfully'; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - return `Error posting comment: ${message}`; - } -} diff --git a/src/gadgets/trello/core/readCard.ts b/src/gadgets/trello/core/readCard.ts deleted file mode 100644 index eba42bbb..00000000 --- a/src/gadgets/trello/core/readCard.ts +++ /dev/null @@ -1,83 +0,0 @@ -import { trelloClient } from '../../../trello/client.js'; - -type CardData = Awaited>; -type ChecklistData = Awaited>; -type AttachmentData = Awaited>; -type CommentData = Awaited>; - -export async function readCard(cardId: string, includeComments = true): Promise { - try { - const [card, checklists, attachments] = await Promise.all([ - trelloClient.getCard(cardId), - trelloClient.getCardChecklists(cardId), - trelloClient.getCardAttachments(cardId), - ]); - - let result = formatHeader(card); - result += formatLabels(card.labels); - result += formatChecklists(checklists); - result += formatAttachments(attachments); - - if (includeComments) { - const comments = await trelloClient.getCardComments(cardId); - result += formatComments(comments); - } - - return result; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - return `Error reading card: ${message}`; - } -} - -function formatHeader(card: CardData): string { - return `# ${card.name}\n\n**URL:** ${card.url}\n\n## Description\n\n${card.desc || '(No description)'}\n\n`; -} - -function formatLabels(labels: CardData['labels']): string { - if (labels.length === 0) return ''; - return `## Labels\n\n${labels.map((l) => `- ${l.name} (${l.color})`).join('\n')}\n\n`; -} - -function formatChecklists(checklists: ChecklistData): string { - if (checklists.length === 0) return ''; - - let result = '## Checklists\n\n'; - for (const checklist of checklists) { - result += `### ${checklist.name} [checklistId: ${checklist.id}]\n\n`; - for (const item of checklist.checkItems) { - const checkbox = item.state === 'complete' ? '[x]' : '[ ]'; - result += `- ${checkbox} ${item.name} [checkItemId: ${item.id}]\n`; - } - result += '\n'; - } - return result; -} - -function formatAttachments(attachments: AttachmentData): string { - if (attachments.length === 0) return ''; - - let result = '## Attachments\n\n'; - for (const att of attachments) { - result += `- [${att.name}](${att.url})`; - if (att.date) { - result += ` (${new Date(att.date).toISOString()})`; - } - result += '\n'; - } - return `${result}\n`; -} - -function formatComments(comments: CommentData): string { - if (comments.length === 0) { - return '## Comments\n\n(No comments)\n\n'; - } - - let result = `## Comments (${comments.length})\n\n`; - for (const comment of comments.slice().reverse()) { - const date = new Date(comment.date).toISOString(); - result += `### ${comment.memberCreator.fullName} (${date})\n\n`; - result += `${comment.data.text}\n\n`; - } - return result; -} diff --git a/src/gadgets/trello/core/updateCard.ts b/src/gadgets/trello/core/updateCard.ts deleted file mode 100644 index cefe2044..00000000 --- a/src/gadgets/trello/core/updateCard.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { trelloClient } from '../../../trello/client.js'; - -export interface UpdateCardParams { - cardId: string; - title?: string; - description?: string; - addLabelIds?: string[]; -} - -export async function updateCard(params: UpdateCardParams): Promise { - if (!params.title && !params.description && !params.addLabelIds?.length) { - return 'Nothing to update - provide title, description, or labels'; - } - - try { - if (params.title || params.description) { - await trelloClient.updateCard(params.cardId, { - name: params.title, - desc: params.description, - }); - } - - if (params.addLabelIds?.length) { - for (const labelId of params.addLabelIds) { - await trelloClient.addLabelToCard(params.cardId, labelId); - } - } - - const updated: string[] = []; - if (params.title) updated.push('title'); - if (params.description) updated.push('description'); - if (params.addLabelIds?.length) updated.push(`${params.addLabelIds.length} label(s)`); - - return `Card updated: ${updated.join(', ')}`; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - return `Error updating card: ${message}`; - } -} diff --git a/src/gadgets/trello/core/updateChecklistItem.ts b/src/gadgets/trello/core/updateChecklistItem.ts deleted file mode 100644 index e4aebbd8..00000000 --- a/src/gadgets/trello/core/updateChecklistItem.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { trelloClient } from '../../../trello/client.js'; - -export async function updateChecklistItem( - cardId: string, - checkItemId: string, - state: 'complete' | 'incomplete', -): Promise { - try { - await trelloClient.updateChecklistItem(cardId, checkItemId, state); - - const action = state === 'complete' ? 'marked complete' : 'marked incomplete'; - return `Checklist item ${checkItemId} ${action} on card ${cardId}`; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - return `Error updating checklist item: ${message}`; - } -} diff --git a/src/gadgets/trello/index.ts b/src/gadgets/trello/index.ts deleted file mode 100644 index 8aa2877f..00000000 --- a/src/gadgets/trello/index.ts +++ /dev/null @@ -1,8 +0,0 @@ -export { ReadTrelloCard, formatCardData } from './ReadTrelloCard.js'; -export { PostTrelloComment } from './PostTrelloComment.js'; -export { UpdateTrelloCard } from './UpdateTrelloCard.js'; -export { CreateTrelloCard } from './CreateTrelloCard.js'; -export { ListTrelloCards } from './ListTrelloCards.js'; -export { GetMyRecentActivity } from './GetMyRecentActivity.js'; -export { AddChecklistToCard } from './AddChecklistToCard.js'; -export { UpdateChecklistItem } from './UpdateChecklistItem.js'; diff --git a/src/pm/config.ts b/src/pm/config.ts new file mode 100644 index 00000000..24207fae --- /dev/null +++ b/src/pm/config.ts @@ -0,0 +1,75 @@ +/** + * Type-safe accessor functions for provider-specific PM config. + * + * Instead of accessing `project.trello?.xxx` or `project.jira?.xxx` directly, + * consumers use these accessors which extract the config from either the + * unified `project.pm.config` or the legacy top-level fields. + */ + +import type { ProjectConfig } from '../types/index.js'; + +/** Trello-specific configuration (from project_integrations JSONB) */ +export interface TrelloConfig { + boardId: string; + lists: Record; + labels: Record; + customFields?: { cost?: string }; + triggers?: Record; +} + +/** JIRA-specific configuration (from project_integrations JSONB) */ +export interface JiraConfig { + projectKey: string; + baseUrl: string; + statuses: Record; + issueTypes?: Record; + customFields?: { cost?: string }; + labels?: { + processing?: string; + processed?: string; + error?: string; + readyToProcess?: string; + }; + triggers?: Record; +} + +/** + * Get the Trello config for a project. + * Returns the config or undefined if this is not a Trello project. + */ +export function getTrelloConfig(project: ProjectConfig): TrelloConfig | undefined { + if (project.pm?.type !== 'trello' && project.pm?.type !== undefined) return undefined; + return project.trello as TrelloConfig | undefined; +} + +/** + * Get the JIRA config for a project. + * Returns the config or undefined if this is not a JIRA project. + * + * Falls back to checking `project.jira` directly when `pm.type` is unset + * (legacy projects / test fixtures that don't set `pm.type`). + */ +export function getJiraConfig(project: ProjectConfig): JiraConfig | undefined { + if (project.pm?.type !== undefined && project.pm?.type !== 'jira') return undefined; + return project.jira as JiraConfig | undefined; +} + +/** + * Get the cost custom field ID for a project, regardless of PM type. + */ +export function getCostFieldId(project: ProjectConfig): string | undefined { + if (project.pm?.type === 'jira') { + return getJiraConfig(project)?.customFields?.cost; + } + return getTrelloConfig(project)?.customFields?.cost; +} + +/** + * Get PM-specific trigger config for a project. + */ +export function getPMTriggerConfig(project: ProjectConfig): Record | undefined { + if (project.pm?.type === 'jira') { + return getJiraConfig(project)?.triggers; + } + return getTrelloConfig(project)?.triggers; +} diff --git a/src/pm/factory.ts b/src/pm/factory.ts deleted file mode 100644 index 6a2cc45a..00000000 --- a/src/pm/factory.ts +++ /dev/null @@ -1,25 +0,0 @@ -/** - * Factory for creating PM providers based on project configuration. - */ - -import type { ProjectConfig } from '../types/index.js'; -import { JiraPMProvider } from './jira/adapter.js'; -import { TrelloPMProvider } from './trello/adapter.js'; -import type { PMProvider } from './types.js'; - -export function createPMProvider(project: ProjectConfig): PMProvider { - const pmType = project.pm?.type ?? 'trello'; - - switch (pmType) { - case 'trello': - return new TrelloPMProvider(); - case 'jira': { - if (!project.jira) { - throw new Error(`Project '${project.id}' has pm.type=jira but no jira config`); - } - return new JiraPMProvider(project.jira); - } - default: - throw new Error(`Unknown PM type: ${pmType}`); - } -} diff --git a/src/pm/index.ts b/src/pm/index.ts index ba83dd6c..1cc44b72 100644 --- a/src/pm/index.ts +++ b/src/pm/index.ts @@ -11,8 +11,25 @@ export type { } from './types.js'; export { withPMProvider, getPMProvider, getPMProviderOrNull } from './context.js'; -export { createPMProvider } from './factory.js'; export { TrelloPMProvider } from './trello/adapter.js'; export { JiraPMProvider } from './jira/adapter.js'; export { PMLifecycleManager, resolveProjectPMConfig } from './lifecycle.js'; export type { ProjectPMConfig } from './lifecycle.js'; + +// PMIntegration interface + registry +export type { PMIntegration, PMWebhookEvent } from './integration.js'; +export { pmRegistry } from './registry.js'; +export { processPMWebhook } from './webhook-handler.js'; + +import type { ProjectConfig } from '../types/index.js'; +import { JiraIntegration } from './jira/integration.js'; +import { pmRegistry } from './registry.js'; +// Register built-in integrations at import time +import { TrelloIntegration } from './trello/integration.js'; +import type { PMProvider } from './types.js'; +pmRegistry.register(new TrelloIntegration()); +pmRegistry.register(new JiraIntegration()); + +export function createPMProvider(project: ProjectConfig): PMProvider { + return pmRegistry.createProvider(project); +} diff --git a/src/pm/integration.ts b/src/pm/integration.ts new file mode 100644 index 00000000..b315623c --- /dev/null +++ b/src/pm/integration.ts @@ -0,0 +1,72 @@ +/** + * PMIntegration — the higher-level contract that encapsulates everything a PM + * provider needs: data operations, credential scoping, webhook parsing, + * router-side operations, config resolution, and trigger registration. + * + * Each PM provider (Trello, JIRA, future ClickUp/Linear) implements this + * interface as a single self-contained class. Generic infrastructure (router, + * webhook handler, lifecycle manager) consumes the interface without + * provider-specific branching. + */ + +import type { CascadeConfig, ProjectConfig } from '../types/index.js'; +import type { ProjectPMConfig } from './lifecycle.js'; +import type { PMProvider } from './types.js'; + +/** + * Normalized webhook event — what the generic webhook handler operates on. + */ +export interface PMWebhookEvent { + /** Provider-specific event type (e.g. 'updateCard', 'jira:issue_updated') */ + eventType: string; + /** Provider-specific identifier for matching a project (boardId, projectKey) */ + projectIdentifier: string; + /** Work item ID when available (cardId, issueKey) */ + workItemId?: string; + /** Original payload, passed to trigger dispatch */ + raw: unknown; +} + +export interface PMIntegration { + /** Provider identifier — matches the string stored in project_integrations.provider */ + readonly type: string; + + // --- Data operations --- + /** Create a PMProvider instance from the project config */ + createProvider(project: ProjectConfig): PMProvider; + + // --- Credential lifecycle --- + /** Resolve credentials from DB and run `fn` within the credential scope */ + withCredentials(projectId: string, fn: () => Promise): Promise; + + // --- Config --- + /** Extract normalized lifecycle config (labels, statuses) from provider-specific config */ + resolveLifecycleConfig(project: ProjectConfig): ProjectPMConfig; + + // --- Webhook processing --- + /** Parse a raw webhook body into a normalized event, or null if irrelevant */ + parseWebhookPayload(raw: unknown): PMWebhookEvent | null; + + /** Check if a webhook event was authored by the integration's own bot account */ + isSelfAuthored(event: PMWebhookEvent, projectId: string): Promise; + + // --- Router-side operations (lightweight, no SDK) --- + /** Post an acknowledgment comment; returns comment ID or null on failure */ + postAckComment(projectId: string, workItemId: string, message: string): Promise; + + /** Delete an acknowledgment comment (cleanup on no-match) */ + deleteAckComment(projectId: string, workItemId: string, commentId: string): Promise; + + /** Send an acknowledgment reaction (e.g. 👀 emoji) on the source event */ + sendReaction(projectId: string, event: PMWebhookEvent): Promise; + + // --- Project lookup --- + /** Find the project config + cascade config from a webhook identifier */ + lookupProject( + identifier: string, + ): Promise<{ project: ProjectConfig; config: CascadeConfig } | null>; + + // --- Work item ID extraction --- + /** Extract a work item ID from text (e.g. PR body). Returns null if not found. */ + extractWorkItemId(text: string): string | null; +} diff --git a/src/pm/jira/integration.ts b/src/pm/jira/integration.ts new file mode 100644 index 00000000..36662ada --- /dev/null +++ b/src/pm/jira/integration.ts @@ -0,0 +1,136 @@ +/** + * JiraIntegration — implements PMIntegration for JIRA. + * + * Encapsulates all JIRA-specific concerns: credential resolution, + * webhook parsing, ack comments, reactions, project lookup, and triggers. + * + * Router-side operations (ack comments, reactions, bot identity) delegate + * to the single-source-of-truth functions in router/acknowledgments.ts + * and router/reactions.ts. + */ + +import { + findProjectById, + getIntegrationCredential, + loadProjectConfigByJiraProjectKey, +} from '../../config/provider.js'; +import { withJiraCredentials } from '../../jira/client.js'; +import { + deleteJiraAck, + postJiraAck, + resolveJiraBotAccountId, +} from '../../router/acknowledgments.js'; +import { sendAcknowledgeReaction } from '../../router/reactions.js'; +import type { CascadeConfig, ProjectConfig } from '../../types/index.js'; +import { getJiraConfig } from '../config.js'; +import type { PMIntegration, PMWebhookEvent } from '../integration.js'; +import type { ProjectPMConfig } from '../lifecycle.js'; +import type { PMProvider } from '../types.js'; +import { JiraPMProvider } from './adapter.js'; + +// JIRA issue key pattern +const JIRA_ISSUE_KEY_REGEX = /\b([A-Z][A-Z0-9]+-\d+)\b/; + +export class JiraIntegration implements PMIntegration { + readonly type = 'jira'; + + createProvider(project: ProjectConfig): PMProvider { + const jiraConfig = getJiraConfig(project); + if (!jiraConfig?.projectKey) { + throw new Error('JIRA integration requires projectKey in config'); + } + return new JiraPMProvider(jiraConfig); + } + + async withCredentials(projectId: string, fn: () => Promise): Promise { + const email = await getIntegrationCredential(projectId, 'pm', 'email'); + const apiToken = await getIntegrationCredential(projectId, 'pm', 'api_token'); + const project = await findProjectById(projectId); + const baseUrl = (project ? getJiraConfig(project)?.baseUrl : undefined) ?? ''; + return withJiraCredentials({ email, apiToken, baseUrl }, fn); + } + + resolveLifecycleConfig(project: ProjectConfig): ProjectPMConfig { + const jiraConfig = getJiraConfig(project); + const jiraLabels = jiraConfig?.labels; + return { + labels: { + processing: jiraLabels?.processing ?? 'cascade-processing', + processed: jiraLabels?.processed ?? 'cascade-processed', + error: jiraLabels?.error ?? 'cascade-error', + readyToProcess: jiraLabels?.readyToProcess ?? 'cascade-ready', + }, + statuses: { + inProgress: jiraConfig?.statuses?.inProgress, + inReview: jiraConfig?.statuses?.inReview, + done: jiraConfig?.statuses?.done, + merged: jiraConfig?.statuses?.merged, + }, + }; + } + + parseWebhookPayload(raw: unknown): PMWebhookEvent | null { + if (!raw || typeof raw !== 'object') return null; + const p = raw as Record; + const webhookEvent = p.webhookEvent as string | undefined; + if (typeof webhookEvent !== 'string') return null; + + const issue = p.issue as Record | undefined; + const issueKey = issue?.key as string | undefined; + const fields = issue?.fields as Record | undefined; + const projectField = fields?.project as Record | undefined; + const projectKey = projectField?.key as string | undefined; + + if (!projectKey) return null; + + return { + eventType: webhookEvent, + projectIdentifier: projectKey, + workItemId: issueKey, + raw, + }; + } + + async isSelfAuthored(event: PMWebhookEvent, projectId: string): Promise { + if (!event.eventType.startsWith('comment_')) return false; + const p = event.raw as Record; + const comment = p.comment as Record | undefined; + const author = comment?.author as Record | undefined; + const commentAuthorId = author?.accountId as string | undefined; + if (!commentAuthorId) return false; + + try { + const botId = await resolveJiraBotAccountId(projectId); + return !!botId && commentAuthorId === botId; + } catch { + return false; + } + } + + async postAckComment( + projectId: string, + workItemId: string, + message: string, + ): Promise { + return postJiraAck(projectId, workItemId, message); + } + + async deleteAckComment(projectId: string, workItemId: string, commentId: string): Promise { + return deleteJiraAck(projectId, workItemId, commentId); + } + + async sendReaction(projectId: string, event: PMWebhookEvent): Promise { + return sendAcknowledgeReaction('jira', projectId, event.raw); + } + + async lookupProject( + identifier: string, + ): Promise<{ project: ProjectConfig; config: CascadeConfig } | null> { + return (await loadProjectConfigByJiraProjectKey(identifier)) ?? null; + } + + extractWorkItemId(text: string): string | null { + const match = text.match(JIRA_ISSUE_KEY_REGEX); + return match ? match[1] : null; + } +} diff --git a/src/pm/lifecycle.ts b/src/pm/lifecycle.ts index 17f638f3..cbd3c6c5 100644 --- a/src/pm/lifecycle.ts +++ b/src/pm/lifecycle.ts @@ -8,6 +8,7 @@ import type { ProjectConfig } from '../types/index.js'; import { safeOperation, silentOperation } from '../utils/safeOperation.js'; +import { pmRegistry } from './registry.js'; import type { PMProvider } from './types.js'; /** @@ -30,42 +31,10 @@ export interface ProjectPMConfig { /** * Resolve PM-specific config (labels, statuses) from project configuration. + * Delegates to the registered integration's resolveLifecycleConfig(). */ export function resolveProjectPMConfig(project: ProjectConfig): ProjectPMConfig { - if (project.pm?.type === 'jira' && project.jira) { - // JIRA uses label strings (not IDs) and status names from config - const jiraLabels = project.jira.labels; - return { - labels: { - processing: jiraLabels?.processing ?? 'cascade-processing', - processed: jiraLabels?.processed ?? 'cascade-processed', - error: jiraLabels?.error ?? 'cascade-error', - readyToProcess: jiraLabels?.readyToProcess ?? 'cascade-ready', - }, - statuses: { - inProgress: project.jira.statuses.inProgress, - inReview: project.jira.statuses.inReview, - done: project.jira.statuses.done, - merged: project.jira.statuses.merged, - }, - }; - } - - // Trello — labels are IDs from project config - return { - labels: { - processing: project.trello?.labels?.processing, - processed: project.trello?.labels?.processed, - error: project.trello?.labels?.error, - readyToProcess: project.trello?.labels?.readyToProcess, - }, - statuses: { - inProgress: project.trello?.lists?.inProgress, - inReview: project.trello?.lists?.inReview, - done: project.trello?.lists?.done, - merged: project.trello?.lists?.merged, - }, - }; + return pmRegistry.resolveLifecycleConfig(project); } export class PMLifecycleManager { diff --git a/src/pm/registry.ts b/src/pm/registry.ts new file mode 100644 index 00000000..9737498a --- /dev/null +++ b/src/pm/registry.ts @@ -0,0 +1,53 @@ +/** + * PMIntegrationRegistry — singleton that holds all registered PM integrations. + * + * Populated at import time by each integration module. The router, worker, + * and shared infrastructure use `pmRegistry.get(type)` to obtain the + * integration instance without provider-specific branching. + */ + +import type { ProjectConfig } from '../types/index.js'; +import type { PMIntegration } from './integration.js'; +import type { ProjectPMConfig } from './lifecycle.js'; +import type { PMProvider } from './types.js'; + +class PMIntegrationRegistry { + private integrations = new Map(); + + register(integration: PMIntegration): void { + this.integrations.set(integration.type, integration); + } + + get(type: string): PMIntegration { + const integration = this.integrations.get(type); + if (!integration) { + throw new Error( + `Unknown PM integration type: '${type}'. Registered: ${[...this.integrations.keys()].join(', ')}`, + ); + } + return integration; + } + + getOrNull(type: string): PMIntegration | null { + return this.integrations.get(type) ?? null; + } + + all(): PMIntegration[] { + return [...this.integrations.values()]; + } + + /** Convenience: get the integration for a project and create its PMProvider */ + createProvider(project: ProjectConfig): PMProvider { + const type = project.pm?.type ?? 'trello'; + return this.get(type).createProvider(project); + } + + /** Convenience: resolve lifecycle config from project */ + resolveLifecycleConfig(project: ProjectConfig): ProjectPMConfig { + const type = project.pm?.type ?? 'trello'; + return this.get(type).resolveLifecycleConfig(project); + } +} + +/** Singleton registry, populated at import time */ +export const pmRegistry = new PMIntegrationRegistry(); diff --git a/src/pm/trello/adapter.ts b/src/pm/trello/adapter.ts index 00c44f5b..ff42461f 100644 --- a/src/pm/trello/adapter.ts +++ b/src/pm/trello/adapter.ts @@ -28,6 +28,7 @@ export class TrelloPMProvider implements PMProvider { title: card.name, description: card.desc, url: card.url, + status: card.idList, labels: card.labels.map( (l): WorkItemLabel => ({ id: l.id, diff --git a/src/pm/trello/integration.ts b/src/pm/trello/integration.ts new file mode 100644 index 00000000..170e5f20 --- /dev/null +++ b/src/pm/trello/integration.ts @@ -0,0 +1,119 @@ +/** + * TrelloIntegration — implements PMIntegration for Trello. + * + * Encapsulates all Trello-specific concerns: credential resolution, + * webhook parsing, ack comments, reactions, project lookup, and triggers. + * + * Router-side operations (ack comments, reactions, bot identity) delegate + * to the single-source-of-truth functions in router/acknowledgments.ts + * and router/reactions.ts. + */ + +import { getIntegrationCredential, loadProjectConfigByBoardId } from '../../config/provider.js'; +import { + deleteTrelloAck, + postTrelloAck, + resolveTrelloBotMemberId, +} from '../../router/acknowledgments.js'; +import { sendAcknowledgeReaction } from '../../router/reactions.js'; +import { withTrelloCredentials } from '../../trello/client.js'; +import type { CascadeConfig, ProjectConfig } from '../../types/index.js'; +import { getTrelloConfig } from '../config.js'; +import type { PMIntegration, PMWebhookEvent } from '../integration.js'; +import type { ProjectPMConfig } from '../lifecycle.js'; +import type { PMProvider } from '../types.js'; +import { TrelloPMProvider } from './adapter.js'; + +export class TrelloIntegration implements PMIntegration { + readonly type = 'trello'; + + createProvider(_project: ProjectConfig): PMProvider { + return new TrelloPMProvider(); + } + + async withCredentials(projectId: string, fn: () => Promise): Promise { + const apiKey = await getIntegrationCredential(projectId, 'pm', 'api_key'); + const token = await getIntegrationCredential(projectId, 'pm', 'token'); + return withTrelloCredentials({ apiKey, token }, fn); + } + + resolveLifecycleConfig(project: ProjectConfig): ProjectPMConfig { + const trelloConfig = getTrelloConfig(project); + return { + labels: { + processing: trelloConfig?.labels?.processing, + processed: trelloConfig?.labels?.processed, + error: trelloConfig?.labels?.error, + readyToProcess: trelloConfig?.labels?.readyToProcess, + }, + statuses: { + inProgress: trelloConfig?.lists?.inProgress, + inReview: trelloConfig?.lists?.inReview, + done: trelloConfig?.lists?.done, + merged: trelloConfig?.lists?.merged, + }, + }; + } + + parseWebhookPayload(raw: unknown): PMWebhookEvent | null { + if (!raw || typeof raw !== 'object') return null; + const p = raw as Record; + const action = p.action as Record | undefined; + const model = p.model as Record | undefined; + if (!action || !model) return null; + + const boardId = model.id as string; + const actionType = action.type as string; + const data = action.data as Record | undefined; + const card = data?.card as Record | undefined; + const cardId = card?.id as string | undefined; + + return { + eventType: actionType, + projectIdentifier: boardId, + workItemId: cardId, + raw, + }; + } + + async isSelfAuthored(event: PMWebhookEvent, projectId: string): Promise { + const p = event.raw as Record; + const action = p.action as Record | undefined; + const commentAuthorId = action?.idMemberCreator as string | undefined; + if (!commentAuthorId) return false; + + try { + const botId = await resolveTrelloBotMemberId(projectId); + return !!botId && commentAuthorId === botId; + } catch { + return false; + } + } + + async postAckComment( + projectId: string, + workItemId: string, + message: string, + ): Promise { + return postTrelloAck(projectId, workItemId, message); + } + + async deleteAckComment(projectId: string, workItemId: string, commentId: string): Promise { + return deleteTrelloAck(projectId, workItemId, commentId); + } + + async sendReaction(projectId: string, event: PMWebhookEvent): Promise { + return sendAcknowledgeReaction('trello', projectId, event.raw); + } + + async lookupProject( + identifier: string, + ): Promise<{ project: ProjectConfig; config: CascadeConfig } | null> { + return (await loadProjectConfigByBoardId(identifier)) ?? null; + } + + extractWorkItemId(text: string): string | null { + const match = text.match(/https:\/\/trello\.com\/c\/([a-zA-Z0-9]+)/); + return match ? match[1] : null; + } +} diff --git a/src/pm/webhook-handler.ts b/src/pm/webhook-handler.ts new file mode 100644 index 00000000..ce29950b --- /dev/null +++ b/src/pm/webhook-handler.ts @@ -0,0 +1,211 @@ +/** + * Generic PM webhook processor. + * + * Extracts the common webhook processing flow from the Trello and JIRA + * webhook handlers into a single PM-agnostic function. Provider-specific + * behavior (credential resolution, payload parsing, project lookup, + * ack comment management) is delegated to the PMIntegration interface. + */ + +import { withGitHubToken } from '../github/client.js'; +import { getPersonaToken } from '../github/personas.js'; +import type { TriggerRegistry } from '../triggers/registry.js'; +import { runAgentExecutionPipeline } from '../triggers/shared/agent-execution.js'; +import { processNextQueuedWebhook } from '../triggers/shared/webhook-queue.js'; +import type { TriggerResult } from '../triggers/types.js'; +import type { + CascadeConfig, + ProjectConfig, + TriggerContext, + TriggerSource, +} from '../types/index.js'; +import { + clearCardActive, + enqueueWebhook, + getQueueLength, + isCardActive, + isCurrentlyProcessing, + logger, + setCardActive, + setProcessing, + startWatchdog, +} from '../utils/index.js'; +import { injectLlmApiKeys } from '../utils/llmEnv.js'; +import { getPMProvider, withPMProvider } from './context.js'; +import type { PMIntegration } from './integration.js'; +import { PMLifecycleManager, resolveProjectPMConfig } from './lifecycle.js'; +import { pmRegistry } from './registry.js'; + +// ============================================================================ +// Agent Execution +// ============================================================================ + +async function executeAgent( + integration: PMIntegration, + result: TriggerResult, + project: ProjectConfig, + config: CascadeConfig, +): Promise { + if (!result.agentType) return; + const githubToken = await getPersonaToken(project.id, result.agentType); + const restoreLlmEnv = await injectLlmApiKeys(project.id); + + try { + await integration.withCredentials(project.id, () => + withGitHubToken(githubToken, () => + runAgentExecutionPipeline(result, project, config, { + logLabel: `${integration.type} agent`, + }), + ), + ); + } finally { + restoreLlmEnv(); + } +} + +// ============================================================================ +// Webhook Processing +// ============================================================================ + +function processNextQueued(integration: PMIntegration, registry: TriggerRegistry): void { + processNextQueuedWebhook( + (payload, _eventType, ackCommentId) => + processPMWebhook(integration, payload, registry, ackCommentId as string | undefined), + integration.type.charAt(0).toUpperCase() + integration.type.slice(1), + ); +} + +async function cleanupOrphanAck( + integration: PMIntegration, + projectId: string, + payload: unknown, + ackCommentId: string, +): Promise { + const event = integration.parseWebhookPayload(payload); + if (event?.workItemId) { + logger.info('Cleaning up orphan ack comment', { ackCommentId }); + await integration.deleteAckComment(projectId, event.workItemId, ackCommentId).catch(() => {}); + } +} + +async function handleMatchedTrigger( + integration: PMIntegration, + registry: TriggerRegistry, + payload: unknown, + project: ProjectConfig, + config: CascadeConfig, + ackCommentId?: string, +): Promise { + const ctx: TriggerContext = { project, source: integration.type as TriggerSource, payload }; + const result = await registry.dispatch(ctx); + if (!result) { + logger.info(`No trigger matched for ${integration.type} webhook`); + if (ackCommentId) { + await cleanupOrphanAck(integration, project.id, payload, ackCommentId); + } + return; + } + + // Pass ack comment ID into agent input for ProgressMonitor pre-seeding + if (ackCommentId) { + result.agentInput.ackCommentId = ackCommentId; + } + + const workItemId = result.workItemId; + if (workItemId && isCardActive(workItemId)) { + logger.info('Work item already being processed, skipping', { workItemId }); + return; + } + + logger.info(`${integration.type} trigger matched`, { + agentType: result.agentType, + workItemId, + }); + + setProcessing(true); + startWatchdog(config.defaults.watchdogTimeoutMs); + + const pmConfig = resolveProjectPMConfig(project); + const lifecycle = new PMLifecycleManager(getPMProvider(), pmConfig); + + try { + if (workItemId) { + setCardActive(workItemId); + } + await executeAgent(integration, result, project, config); + } catch (err) { + logger.error(`Failed to process ${integration.type} webhook`, { error: String(err) }); + if (workItemId) { + await lifecycle.handleError(workItemId, String(err)); + } + } finally { + if (workItemId) { + clearCardActive(workItemId); + } + setProcessing(false); + processNextQueued(integration, registry); + } +} + +/** + * Generic PM webhook processor. + * + * Validates the payload via the integration's `parseWebhookPayload()`, + * looks up the project, establishes credential + PM provider scope, + * dispatches to the trigger registry, and runs the matched agent. + * + * Used by both Trello and JIRA webhook handlers. + */ +export async function processPMWebhook( + integration: PMIntegration, + payload: unknown, + registry: TriggerRegistry, + ackCommentId?: string, +): Promise { + logger.info(`Processing ${integration.type} webhook`); + + const event = integration.parseWebhookPayload(payload); + if (!event) { + logger.warn(`Invalid ${integration.type} webhook payload`, { + payload: JSON.stringify(payload).slice(0, 200), + }); + return; + } + + if (isCurrentlyProcessing()) { + const queued = enqueueWebhook(payload, undefined, ackCommentId); + if (queued) { + logger.info(`Currently processing, ${integration.type} webhook queued`, { + queueLength: getQueueLength(), + }); + } else { + logger.warn(`Queue full, ${integration.type} webhook rejected`, { + queueLength: getQueueLength(), + }); + } + return; + } + + logger.info(`${integration.type} webhook details`, { + projectIdentifier: event.projectIdentifier, + workItemId: event.workItemId, + eventType: event.eventType, + }); + + const projectConfig = await integration.lookupProject(event.projectIdentifier); + if (!projectConfig) { + logger.warn(`No project configured for ${integration.type} identifier`, { + identifier: event.projectIdentifier, + }); + return; + } + const { project, config } = projectConfig; + + // Establish credential + PM provider scope for trigger dispatch + const pmProvider = pmRegistry.createProvider(project); + await integration.withCredentials(project.id, () => + withPMProvider(pmProvider, () => + handleMatchedTrigger(integration, registry, payload, project, config, ackCommentId), + ), + ); +} diff --git a/src/router/acknowledgments.ts b/src/router/acknowledgments.ts index 173b7151..fd1f059f 100644 --- a/src/router/acknowledgments.ts +++ b/src/router/acknowledgments.ts @@ -16,6 +16,7 @@ import { findProjectByRepo, getIntegrationCredential, } from '../config/provider.js'; +import { getJiraConfig } from '../pm/config.js'; import { markdownToAdf } from '../pm/jira/adf.js'; import type { ProjectConfig } from '../types/index.js'; @@ -147,7 +148,7 @@ export async function postJiraAck( jiraEmail = await getIntegrationCredential(projectId, 'pm', 'email'); jiraApiToken = await getIntegrationCredential(projectId, 'pm', 'api_token'); const project = await findProjectById(projectId); - jiraBaseUrl = project?.jira?.baseUrl ?? ''; + jiraBaseUrl = (project ? getJiraConfig(project)?.baseUrl : undefined) ?? ''; if (!jiraBaseUrl) throw new Error('Missing JIRA base URL'); } catch { console.warn('[Ack] Missing JIRA credentials, skipping ack comment'); @@ -188,7 +189,7 @@ export async function deleteJiraAck( jiraEmail = await getIntegrationCredential(projectId, 'pm', 'email'); jiraApiToken = await getIntegrationCredential(projectId, 'pm', 'api_token'); const project = await findProjectById(projectId); - jiraBaseUrl = project?.jira?.baseUrl ?? ''; + jiraBaseUrl = (project ? getJiraConfig(project)?.baseUrl : undefined) ?? ''; if (!jiraBaseUrl) throw new Error('Missing JIRA base URL'); } catch { return; @@ -233,7 +234,7 @@ export async function resolveJiraBotAccountId(projectId: string): Promise { const config: CascadeConfig = await loadConfig(); return { - projects: config.projects.map((p) => ({ - id: p.id, - repo: p.repo, - pmType: p.pm?.type ?? 'trello', - ...(p.trello && { - trello: { - boardId: p.trello.boardId, - lists: p.trello.lists, - labels: p.trello.labels, - }, - }), - ...(p.jira && { - jira: { - projectKey: p.jira.projectKey, - baseUrl: p.jira.baseUrl, - }, - }), - })), + projects: config.projects.map((p) => { + const trelloConfig = getTrelloConfig(p); + const jiraConfig = getJiraConfig(p); + return { + id: p.id, + repo: p.repo, + pmType: p.pm?.type ?? 'trello', + ...(trelloConfig && { + trello: { + boardId: trelloConfig.boardId, + lists: trelloConfig.lists, + labels: trelloConfig.labels, + }, + }), + ...(jiraConfig && { + jira: { + projectKey: jiraConfig.projectKey, + baseUrl: jiraConfig.baseUrl, + }, + }), + }; + }), fullProjects: config.projects, }; } diff --git a/src/router/reactions.ts b/src/router/reactions.ts index d0855f1d..976f1757 100644 --- a/src/router/reactions.ts +++ b/src/router/reactions.ts @@ -11,6 +11,7 @@ import { getProjectGitHubToken } from '../config/projects.js'; import { findProjectById, getIntegrationCredential } from '../config/provider.js'; import { type PersonaIdentities, isCascadeBot } from '../github/personas.js'; +import { getJiraConfig } from '../pm/config.js'; import { trelloClient, withTrelloCredentials } from '../trello/client.js'; import type { ProjectConfig } from '../types/index.js'; import { parseRepoFullName } from '../utils/repo.js'; @@ -210,7 +211,7 @@ async function sendJiraReaction(projectId: string, payload: unknown): Promise { } } +/** + * Establish PM credential scope for the project. + * Uses the integration's withCredentials() for the correct PM type. + * Falls through to running fn() directly if no PM type is configured. + */ +async function withPMCredentials(project: ProjectConfig, fn: () => Promise): Promise { + const pmType = project.pm?.type; + if (!pmType) return fn(); + const integration = pmRegistry.getOrNull(pmType); + if (!integration) return fn(); + return integration.withCredentials(project.id, fn); +} + async function executeGitHubAgent( result: TriggerResult, project: ProjectConfig, config: CascadeConfig, ): Promise { - const trelloApiKey = await getIntegrationCredentialOrNull(project.id, 'pm', 'api_key').then( - (v) => v ?? '', - ); - const trelloToken = await getIntegrationCredentialOrNull(project.id, 'pm', 'token').then( - (v) => v ?? '', - ); + if (!result.agentType) return; const githubToken = await getPersonaToken(project.id, result.agentType); - const restoreLlmEnv = await injectLlmApiKeys(project.id); const executionConfig: AgentExecutionConfig = { @@ -104,7 +110,7 @@ async function executeGitHubAgent( try { const pmProvider = createPMProvider(project); - await withTrelloCredentials({ apiKey: trelloApiKey, token: trelloToken }, () => + await withPMCredentials(project, () => withPMProvider(pmProvider, () => withGitHubToken(githubToken, () => runAgentExecutionPipeline(result, project, config, executionConfig), @@ -124,6 +130,7 @@ async function runGitHubAgentJob( registry: TriggerRegistry, routerAckCommentId?: number, ): Promise { + if (!result.agentType) return; // Use the persona token for the agent that will do the work (for ack comments) let prCommentToken: string; try { @@ -205,21 +212,14 @@ export async function processGitHubWebhook( } const { project, config } = projectConfig; - // Resolve credentials early — trigger handlers may call GitHub/Trello APIs - const trelloApiKey = await getIntegrationCredentialOrNull(project.id, 'pm', 'api_key').then( - (v) => v ?? '', - ); - const trelloToken = await getIntegrationCredentialOrNull(project.id, 'pm', 'token').then( - (v) => v ?? '', - ); - // Resolve persona identities and use implementer token for webhook processing const personaIdentities = await resolvePersonaIdentities(project.id); const githubToken = await getPersonaToken(project.id, 'implementation'); const pmProvider = createPMProvider(project); + // Establish PM credential + provider scope for trigger dispatch const ctx: TriggerContext = { project, source: 'github', payload, personaIdentities }; - const result = await withTrelloCredentials({ apiKey: trelloApiKey, token: trelloToken }, () => + const result = await withPMCredentials(project, () => withPMProvider(pmProvider, () => withGitHubToken(githubToken, () => registry.dispatch(ctx))), ); diff --git a/src/triggers/jira/comment-mention.ts b/src/triggers/jira/comment-mention.ts index e84e26e5..a4982984 100644 --- a/src/triggers/jira/comment-mention.ts +++ b/src/triggers/jira/comment-mention.ts @@ -7,6 +7,7 @@ import { resolveJiraTriggerEnabled } from '../../config/triggerConfig.js'; import { jiraClient } from '../../jira/client.js'; +import { getJiraConfig } from '../../pm/config.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; @@ -111,7 +112,7 @@ export class JiraCommentMentionTrigger implements TriggerHandler { if (ctx.source !== 'jira') return false; // Check trigger config — default enabled for backward compatibility - if (!resolveJiraTriggerEnabled(ctx.project.jira?.triggers, 'commentMention')) { + if (!resolveJiraTriggerEnabled(getJiraConfig(ctx.project)?.triggers, 'commentMention')) { return false; } @@ -190,7 +191,6 @@ export class JiraCommentMentionTrigger implements TriggerHandler { triggerCommentAuthor: authorName, }, workItemId: issueKey, - cardId: issueKey, }; } } diff --git a/src/triggers/jira/issue-transitioned.ts b/src/triggers/jira/issue-transitioned.ts index 1f9f6017..1c3d647c 100644 --- a/src/triggers/jira/issue-transitioned.ts +++ b/src/triggers/jira/issue-transitioned.ts @@ -6,6 +6,7 @@ */ import { resolveJiraTriggerEnabled } from '../../config/triggerConfig.js'; +import { getJiraConfig } from '../../pm/config.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; @@ -51,7 +52,7 @@ export class JiraIssueTransitionedTrigger implements TriggerHandler { if (ctx.source !== 'jira') return false; // Check trigger config — default enabled for backward compatibility - if (!resolveJiraTriggerEnabled(ctx.project.jira?.triggers, 'issueTransitioned')) { + if (!resolveJiraTriggerEnabled(getJiraConfig(ctx.project)?.triggers, 'issueTransitioned')) { return false; } @@ -69,7 +70,7 @@ export class JiraIssueTransitionedTrigger implements TriggerHandler { const newStatus = statusChange?.toString; if (!newStatus) return null; - const jiraConfig = ctx.project.jira; + const jiraConfig = getJiraConfig(ctx.project); if (!jiraConfig?.statuses) return null; for (const [cascadeStatus, jiraStatus] of Object.entries(jiraConfig.statuses)) { @@ -94,7 +95,7 @@ export class JiraIssueTransitionedTrigger implements TriggerHandler { return null; } - const jiraConfig = ctx.project.jira; + const jiraConfig = getJiraConfig(ctx.project); if (!jiraConfig?.statuses) { logger.debug('No JIRA status configuration, skipping issue transition trigger', { projectId: ctx.project.id, @@ -131,7 +132,6 @@ export class JiraIssueTransitionedTrigger implements TriggerHandler { agentType, agentInput: { cardId: issueKey }, workItemId: issueKey, - cardId: issueKey, }; } } diff --git a/src/triggers/jira/label-added.ts b/src/triggers/jira/label-added.ts index 56b4ab59..672ac763 100644 --- a/src/triggers/jira/label-added.ts +++ b/src/triggers/jira/label-added.ts @@ -14,6 +14,7 @@ import { resolveJiraTriggerEnabled, resolveReadyToProcessEnabled, } from '../../config/triggerConfig.js'; +import { getJiraConfig } from '../../pm/config.js'; import { resolveProjectPMConfig } from '../../pm/lifecycle.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; @@ -69,7 +70,7 @@ export class JiraReadyToProcessLabelTrigger implements TriggerHandler { if (ctx.source !== 'jira') return false; // Check trigger config — default enabled for backward compatibility - if (!resolveJiraTriggerEnabled(ctx.project.jira?.triggers, 'readyToProcessLabel')) { + if (!resolveJiraTriggerEnabled(getJiraConfig(ctx.project)?.triggers, 'readyToProcessLabel')) { return false; } @@ -101,7 +102,7 @@ export class JiraReadyToProcessLabelTrigger implements TriggerHandler { const currentStatus = payload.issue?.fields?.status?.name; if (!currentStatus) return null; - const jiraConfig = ctx.project.jira; + const jiraConfig = getJiraConfig(ctx.project); if (!jiraConfig?.statuses) return null; for (const [cascadeStatus, jiraStatus] of Object.entries(jiraConfig.statuses)) { @@ -126,7 +127,7 @@ export class JiraReadyToProcessLabelTrigger implements TriggerHandler { return null; } - const jiraConfig = ctx.project.jira; + const jiraConfig = getJiraConfig(ctx.project); if (!jiraConfig?.statuses) { logger.debug('No JIRA status configuration, skipping label trigger', { projectId: ctx.project.id, @@ -153,7 +154,7 @@ export class JiraReadyToProcessLabelTrigger implements TriggerHandler { } // Check per-agent ready-to-process toggle - if (!resolveReadyToProcessEnabled(ctx.project.jira?.triggers, agentType)) { + if (!resolveReadyToProcessEnabled(getJiraConfig(ctx.project)?.triggers, agentType)) { logger.info('JIRA ready-to-process disabled for agent type, skipping', { issueKey, agentType, @@ -171,7 +172,6 @@ export class JiraReadyToProcessLabelTrigger implements TriggerHandler { agentType, agentInput: { cardId: issueKey }, workItemId: issueKey, - cardId: issueKey, }; } } diff --git a/src/triggers/jira/webhook-handler.ts b/src/triggers/jira/webhook-handler.ts index 44dc8b9a..9f034b88 100644 --- a/src/triggers/jira/webhook-handler.ts +++ b/src/triggers/jira/webhook-handler.ts @@ -1,221 +1,19 @@ /** * JIRA webhook handler. * - * Processes JIRA webhooks: validates payload, extracts project key, - * finds project via findProjectByJiraProjectKey(), resolves creds, - * and dispatches to the trigger registry. + * Thin wrapper around the generic PM webhook processor. + * Resolves the JIRA integration from the registry and delegates. */ -import { - getIntegrationCredential, - loadProjectConfigByJiraProjectKey, -} from '../../config/provider.js'; -import { withGitHubToken } from '../../github/client.js'; -import { getPersonaToken } from '../../github/personas.js'; -import { withJiraCredentials } from '../../jira/client.js'; -import { - PMLifecycleManager, - createPMProvider, - resolveProjectPMConfig, - withPMProvider, -} from '../../pm/index.js'; -import type { CascadeConfig, ProjectConfig, TriggerContext } from '../../types/index.js'; -import { - enqueueWebhook, - getQueueLength, - isCurrentlyProcessing, - logger, - setProcessing, - startWatchdog, -} from '../../utils/index.js'; -import { injectLlmApiKeys } from '../../utils/llmEnv.js'; +import { pmRegistry } from '../../pm/index.js'; +import { processPMWebhook } from '../../pm/webhook-handler.js'; import type { TriggerRegistry } from '../registry.js'; -import { runAgentExecutionPipeline } from '../shared/agent-execution.js'; -import { processNextQueuedWebhook } from '../shared/webhook-queue.js'; -import type { TriggerResult } from '../types.js'; - -interface JiraWebhookPayload { - webhookEvent: string; - issue?: { - id?: string; - key: string; - fields?: { - project?: { key?: string }; - status?: { name?: string }; - summary?: string; - comment?: { comments?: unknown[] }; - }; - }; - comment?: { - id?: string; - body?: unknown; - author?: { displayName?: string; accountId?: string }; - }; - changelog?: { - items?: Array<{ - field?: string; - fromString?: string; - toString?: string; - }>; - }; -} - -function isJiraWebhookPayload(payload: unknown): payload is JiraWebhookPayload { - const p = payload as Record; - return typeof p?.webhookEvent === 'string'; -} - -function extractProjectKey(payload: JiraWebhookPayload): string | undefined { - return payload.issue?.fields?.project?.key; -} - -async function executeJiraAgent( - result: TriggerResult, - project: ProjectConfig, - config: CascadeConfig, -): Promise { - const jiraEmail = await getIntegrationCredential(project.id, 'pm', 'email'); - const jiraApiToken = await getIntegrationCredential(project.id, 'pm', 'api_token'); - const jiraBaseUrl = project.jira?.baseUrl ?? ''; - const githubToken = await getPersonaToken(project.id, result.agentType); - - const restoreLlmEnv = await injectLlmApiKeys(project.id); - - try { - const pmProvider = createPMProvider(project); - await withJiraCredentials( - { email: jiraEmail, apiToken: jiraApiToken, baseUrl: jiraBaseUrl }, - () => - withPMProvider(pmProvider, () => - withGitHubToken(githubToken, () => - runAgentExecutionPipeline(result, project, config, { logLabel: 'JIRA agent' }), - ), - ), - ); - } finally { - restoreLlmEnv(); - } -} - -async function handleMatchedJiraTrigger( - registry: TriggerRegistry, - payload: JiraWebhookPayload, - project: ProjectConfig, - config: CascadeConfig, - pmProvider: ReturnType, - ackCommentId?: string, -): Promise { - const ctx: TriggerContext = { project, source: 'jira', payload }; - const result = await registry.dispatch(ctx); - if (!result) { - logger.info('No trigger matched for JIRA webhook', { event: payload.webhookEvent }); - if (ackCommentId && payload.issue?.key) { - await cleanupOrphanJiraAck(project.id, payload.issue.key, ackCommentId); - } - return; - } - - // Pass ack comment ID into agent input for ProgressMonitor pre-seeding - if (ackCommentId) { - result.agentInput.ackCommentId = ackCommentId; - } - - logger.info('JIRA trigger matched', { - agentType: result.agentType, - workItemId: result.workItemId, - }); - - setProcessing(true); - startWatchdog(config.defaults.watchdogTimeoutMs); - - const pmConfig = resolveProjectPMConfig(project); - const lifecycle = new PMLifecycleManager(pmProvider, pmConfig); - - try { - await executeJiraAgent(result, project, config); - } catch (err) { - logger.error('Failed to process JIRA webhook', { error: String(err) }); - if (result.workItemId) { - await lifecycle.handleError(result.workItemId, String(err)); - } - } finally { - setProcessing(false); - processNextQueuedJiraWebhook(registry); - } -} - -async function cleanupOrphanJiraAck( - projectId: string, - issueKey: string, - ackCommentId: string, -): Promise { - logger.info('Cleaning up orphan ack comment', { ackCommentId, issueKey }); - const { deleteJiraAck } = await import('../../router/acknowledgments.js'); - await deleteJiraAck(projectId, issueKey, ackCommentId).catch(() => {}); -} - -function processNextQueuedJiraWebhook(registry: TriggerRegistry): void { - processNextQueuedWebhook( - (payload, _eventType, ackCommentId) => - processJiraWebhook(payload, registry, ackCommentId as string | undefined), - 'JIRA', - ); -} export async function processJiraWebhook( payload: unknown, registry: TriggerRegistry, ackCommentId?: string, ): Promise { - logger.info('Processing JIRA webhook'); - - if (!isJiraWebhookPayload(payload)) { - logger.warn('Invalid JIRA webhook payload', { - payload: JSON.stringify(payload).slice(0, 200), - }); - return; - } - - if (isCurrentlyProcessing()) { - const queued = enqueueWebhook(payload, undefined, ackCommentId); - if (queued) { - logger.info('Currently processing, JIRA webhook queued', { queueLength: getQueueLength() }); - } else { - logger.warn('Queue full, JIRA webhook rejected', { queueLength: getQueueLength() }); - } - return; - } - - const projectKey = extractProjectKey(payload); - if (!projectKey) { - logger.warn('JIRA webhook missing project key'); - return; - } - - logger.info('JIRA webhook details', { - event: payload.webhookEvent, - issueKey: payload.issue?.key, - projectKey, - }); - - const projectConfig = await loadProjectConfigByJiraProjectKey(projectKey); - if (!projectConfig) { - logger.warn('No project configured for JIRA project key', { projectKey }); - return; - } - const { project, config } = projectConfig; - - // Establish JIRA credential + PM provider scope - const jiraEmail = await getIntegrationCredential(project.id, 'pm', 'email'); - const jiraApiToken = await getIntegrationCredential(project.id, 'pm', 'api_token'); - const jiraBaseUrl = project.jira?.baseUrl ?? ''; - const pmProvider = createPMProvider(project); - - await withJiraCredentials( - { email: jiraEmail, apiToken: jiraApiToken, baseUrl: jiraBaseUrl }, - () => - withPMProvider(pmProvider, () => - handleMatchedJiraTrigger(registry, payload, project, config, pmProvider, ackCommentId), - ), - ); + const integration = pmRegistry.get('jira'); + await processPMWebhook(integration, payload, registry, ackCommentId); } diff --git a/src/triggers/shared/agent-execution.ts b/src/triggers/shared/agent-execution.ts index 81503620..1ea6adab 100644 --- a/src/triggers/shared/agent-execution.ts +++ b/src/triggers/shared/agent-execution.ts @@ -73,7 +73,7 @@ async function checkPreRunBudget( */ async function runPostAgentLifecycle( workItemId: string, - result: TriggerResult, + agentType: string, agentResult: AgentResult, project: ProjectConfig, config: CascadeConfig, @@ -86,7 +86,7 @@ async function runPostAgentLifecycle( handleSuccessOnlyForAgentType, } = executionConfig; - await handleAgentResultArtifacts(workItemId, result.agentType, agentResult, project); + await handleAgentResultArtifacts(workItemId, agentType, agentResult, project); const postBudgetCheck = await checkBudgetExceeded(workItemId, project, config); if (postBudgetCheck?.exceeded) { @@ -103,12 +103,12 @@ async function runPostAgentLifecycle( const shouldCallHandleSuccess = agentResult.success && - (!handleSuccessOnlyForAgentType || result.agentType === handleSuccessOnlyForAgentType); + (!handleSuccessOnlyForAgentType || agentType === handleSuccessOnlyForAgentType); if (shouldCallHandleSuccess) { await lifecycle.handleSuccess( workItemId, - result.agentType, + agentType, agentResult.prUrl, agentResult.progressCommentId, ); @@ -143,9 +143,15 @@ export async function runAgentExecutionPipeline( config: CascadeConfig, executionConfig: AgentExecutionConfig = {}, ): Promise { + if (!result.agentType) { + logger.warn('No agent type in trigger result, skipping execution pipeline'); + return; + } + const agentType = result.agentType; + const { skipPrepareForAgent = false, onFailure, logLabel = 'Agent' } = executionConfig; - const workItemId = result.cardId ?? result.workItemId; + const workItemId = result.workItemId; const pmProvider = createPMProvider(project); const pmConfig = resolveProjectPMConfig(project); const lifecycle = new PMLifecycleManager(pmProvider, pmConfig); @@ -158,10 +164,10 @@ export async function runAgentExecutionPipeline( } if (workItemId && !skipPrepareForAgent) { - await lifecycle.prepareForAgent(workItemId, result.agentType); + await lifecycle.prepareForAgent(workItemId, agentType); } - const agentResult = await runAgent(result.agentType, { + const agentResult = await runAgent(agentType, { ...result.agentInput, remainingBudgetUsd, project, @@ -171,7 +177,7 @@ export async function runAgentExecutionPipeline( if (workItemId) { await runPostAgentLifecycle( workItemId, - result, + agentType, agentResult, project, config, @@ -181,7 +187,7 @@ export async function runAgentExecutionPipeline( } logger.info(`${logLabel} completed`, { - agentType: result.agentType, + agentType, success: agentResult.success, runId: agentResult.runId, }); diff --git a/src/triggers/shared/agent-result-handler.ts b/src/triggers/shared/agent-result-handler.ts index 777bca14..a526ba05 100644 --- a/src/triggers/shared/agent-result-handler.ts +++ b/src/triggers/shared/agent-result-handler.ts @@ -1,19 +1,9 @@ +import { getCostFieldId } from '../../pm/config.js'; import { getPMProvider } from '../../pm/index.js'; import type { AgentResult, ProjectConfig } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; import { safeOperation } from '../../utils/safeOperation.js'; -/** - * Resolve the cost custom field ID from the project config. - * Supports both Trello and JIRA projects. - */ -function getCostFieldId(project: ProjectConfig): string | undefined { - if (project.pm?.type === 'jira') { - return project.jira?.customFields?.cost; - } - return project.trello?.customFields?.cost; -} - /** * Update cost custom field on the work item (card/issue). * Shared between GitHub, Trello, and JIRA webhook handlers. diff --git a/src/triggers/shared/budget.ts b/src/triggers/shared/budget.ts index 2aad72c3..9ba5e638 100644 --- a/src/triggers/shared/budget.ts +++ b/src/triggers/shared/budget.ts @@ -1,3 +1,4 @@ +import { getCostFieldId } from '../../pm/config.js'; import { getPMProvider } from '../../pm/index.js'; import type { CascadeConfig, ProjectConfig } from '../../types/index.js'; @@ -8,16 +9,6 @@ export interface BudgetCheckResult { remaining: number; } -/** - * Resolve the cost custom field ID from the project config. - */ -function getCostFieldId(project: ProjectConfig): string | undefined { - if (project.pm?.type === 'jira') { - return project.jira?.customFields?.cost; - } - return project.trello?.customFields?.cost; -} - /** * Resolve the card budget for a project. * Returns `null` if no cost custom field is configured (budget enforcement not applicable). diff --git a/src/triggers/trello/card-moved.ts b/src/triggers/trello/card-moved.ts index 47c20a8e..3f90e79e 100644 --- a/src/triggers/trello/card-moved.ts +++ b/src/triggers/trello/card-moved.ts @@ -1,4 +1,5 @@ import { resolveTrelloTriggerEnabled } from '../../config/triggerConfig.js'; +import { getTrelloConfig } from '../../pm/config.js'; import type { TrelloWebhookPayload, TriggerContext, @@ -29,12 +30,13 @@ function createCardMovedTrigger(config: CardMovedConfig): TriggerHandler { if (!isTrelloWebhookPayload(ctx.payload)) return false; // Check trigger config — default enabled for backward compatibility - if (!resolveTrelloTriggerEnabled(ctx.project.trello?.triggers, config.triggerConfigKey)) { + const trelloConfig = getTrelloConfig(ctx.project); + if (!resolveTrelloTriggerEnabled(trelloConfig?.triggers, config.triggerConfigKey)) { return false; } const payload = ctx.payload; - const targetListId = ctx.project.trello?.lists[config.listKey]; + const targetListId = trelloConfig?.lists[config.listKey]; // Card moved into the target list const isMove = @@ -64,7 +66,7 @@ function createCardMovedTrigger(config: CardMovedConfig): TriggerHandler { return { agentType: config.agentType, agentInput: { cardId }, - cardId, + workItemId: cardId, }; }, }; diff --git a/src/triggers/trello/comment-mention.ts b/src/triggers/trello/comment-mention.ts index 63e15010..5ecfc741 100644 --- a/src/triggers/trello/comment-mention.ts +++ b/src/triggers/trello/comment-mention.ts @@ -1,4 +1,5 @@ import { resolveTrelloTriggerEnabled } from '../../config/triggerConfig.js'; +import { getTrelloConfig } from '../../pm/config.js'; import { trelloClient } from '../../trello/client.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; @@ -37,7 +38,7 @@ export class TrelloCommentMentionTrigger implements TriggerHandler { if (!isTrelloWebhookPayload(ctx.payload)) return false; // Check trigger config — default enabled for backward compatibility - if (!resolveTrelloTriggerEnabled(ctx.project.trello?.triggers, 'commentMention')) { + if (!resolveTrelloTriggerEnabled(getTrelloConfig(ctx.project)?.triggers, 'commentMention')) { return false; } @@ -76,7 +77,7 @@ export class TrelloCommentMentionTrigger implements TriggerHandler { } // Fetch card to verify it's in the PLANNING list - const planningListId = ctx.project.trello?.lists.planning; + const planningListId = getTrelloConfig(ctx.project)?.lists.planning; if (!planningListId) { logger.debug('Planning list not configured, skipping comment mention trigger', { projectId: ctx.project.id, @@ -110,7 +111,7 @@ export class TrelloCommentMentionTrigger implements TriggerHandler { triggerCommentText: commentText, triggerCommentAuthor: commentAuthor, }, - cardId, + workItemId: cardId, }; } } diff --git a/src/triggers/trello/label-added.ts b/src/triggers/trello/label-added.ts index 9fa90a9d..27e4afbf 100644 --- a/src/triggers/trello/label-added.ts +++ b/src/triggers/trello/label-added.ts @@ -2,6 +2,7 @@ import { resolveReadyToProcessEnabled, resolveTrelloTriggerEnabled, } from '../../config/triggerConfig.js'; +import { getTrelloConfig } from '../../pm/config.js'; import { trelloClient } from '../../trello/client.js'; import { logger } from '../../utils/logging.js'; import type { @@ -22,12 +23,13 @@ export class ReadyToProcessLabelTrigger implements TriggerHandler { // Check trigger config — default enabled for backward compatibility // (checks if any agent has readyToProcessLabel enabled) - if (!resolveTrelloTriggerEnabled(ctx.project.trello?.triggers, 'readyToProcessLabel')) { + const trelloConfig = getTrelloConfig(ctx.project); + if (!resolveTrelloTriggerEnabled(trelloConfig?.triggers, 'readyToProcessLabel')) { return false; } const payload = ctx.payload; - const readyLabelId = ctx.project.trello?.labels.readyToProcess; + const readyLabelId = trelloConfig?.labels.readyToProcess; return ( payload.action.type === 'addLabelToCard' && payload.action.data.label?.id === readyLabelId @@ -54,7 +56,7 @@ export class ReadyToProcessLabelTrigger implements TriggerHandler { logger.info('Determining agent type from list', { cardId, currentListId }); // Determine agent type based on current list - const lists = ctx.project.trello?.lists ?? {}; + const lists = getTrelloConfig(ctx.project)?.lists ?? {}; let agentType: string; if (currentListId === lists.briefing) { @@ -72,7 +74,7 @@ export class ReadyToProcessLabelTrigger implements TriggerHandler { logger.info('Agent type determined', { agentType, cardId, listId: currentListId }); // Check per-agent ready-to-process toggle - if (!resolveReadyToProcessEnabled(ctx.project.trello?.triggers, agentType)) { + if (!resolveReadyToProcessEnabled(getTrelloConfig(ctx.project)?.triggers, agentType)) { logger.info('Ready-to-process disabled for agent type, skipping', { agentType, cardId }); return null; } @@ -80,7 +82,7 @@ export class ReadyToProcessLabelTrigger implements TriggerHandler { return { agentType, agentInput: { cardId }, - cardId, + workItemId: cardId, }; } } diff --git a/src/triggers/trello/webhook-handler.ts b/src/triggers/trello/webhook-handler.ts index 25f6f8bc..9b3f2b36 100644 --- a/src/triggers/trello/webhook-handler.ts +++ b/src/triggers/trello/webhook-handler.ts @@ -1,201 +1,19 @@ -import { getIntegrationCredential, loadProjectConfigByBoardId } from '../../config/provider.js'; -import { withGitHubToken } from '../../github/client.js'; -import { getPersonaToken } from '../../github/personas.js'; -import { - PMLifecycleManager, - createPMProvider, - resolveProjectPMConfig, - withPMProvider, -} from '../../pm/index.js'; -import { withTrelloCredentials } from '../../trello/client.js'; -import type { CascadeConfig, ProjectConfig, TriggerContext } from '../../types/index.js'; -import { - clearCardActive, - enqueueWebhook, - getQueueLength, - isCardActive, - isCurrentlyProcessing, - logger, - setCardActive, - setProcessing, - startWatchdog, -} from '../../utils/index.js'; -import { injectLlmApiKeys } from '../../utils/llmEnv.js'; +/** + * Trello webhook handler. + * + * Thin wrapper around the generic PM webhook processor. + * Resolves the Trello integration from the registry and delegates. + */ + +import { pmRegistry } from '../../pm/index.js'; +import { processPMWebhook } from '../../pm/webhook-handler.js'; import type { TriggerRegistry } from '../registry.js'; -import { runAgentExecutionPipeline } from '../shared/agent-execution.js'; -import { processNextQueuedWebhook } from '../shared/webhook-queue.js'; -import type { TrelloWebhookPayload, TriggerResult } from '../types.js'; -import { isTrelloWebhookPayload } from '../types.js'; - -// ============================================================================ -// Agent Execution -// ============================================================================ - -async function executeAgent( - result: TriggerResult, - project: ProjectConfig, - config: CascadeConfig, -): Promise { - const trelloApiKey = await getIntegrationCredential(project.id, 'pm', 'api_key'); - const trelloToken = await getIntegrationCredential(project.id, 'pm', 'token'); - const githubToken = await getPersonaToken(project.id, result.agentType); - - const restoreLlmEnv = await injectLlmApiKeys(project.id); - - try { - const pmProvider = createPMProvider(project); - await withTrelloCredentials({ apiKey: trelloApiKey, token: trelloToken }, () => - withPMProvider(pmProvider, () => - withGitHubToken(githubToken, () => - runAgentExecutionPipeline(result, project, config, { logLabel: 'Agent' }), - ), - ), - ); - } finally { - restoreLlmEnv(); - } -} - -// ============================================================================ -// Webhook Processing -// ============================================================================ - -function processNextQueued(registry: TriggerRegistry): void { - processNextQueuedWebhook( - (payload, _eventType, ackCommentId) => - processTrelloWebhook(payload, registry, ackCommentId as string | undefined), - 'Trello', - ); -} - -function tryQueueWebhook(payload: TrelloWebhookPayload, ackCommentId?: string): boolean { - if (!isCurrentlyProcessing()) return false; - - const queued = enqueueWebhook(payload, undefined, ackCommentId); - if (queued) { - logger.info('Currently processing, webhook queued', { queueLength: getQueueLength() }); - } else { - logger.warn('Queue full, webhook rejected', { queueLength: getQueueLength() }); - } - return true; -} - -async function cleanupOrphanTrelloAck( - projectId: string, - payload: TrelloWebhookPayload, - ackCommentId: string, -): Promise { - const cardId = (payload.action?.data?.card as Record | undefined)?.id as - | string - | undefined; - if (cardId) { - logger.info('Cleaning up orphan ack comment', { ackCommentId, cardId }); - const { deleteTrelloAck } = await import('../../router/acknowledgments.js'); - await deleteTrelloAck(projectId, cardId, ackCommentId).catch(() => {}); - } -} - -async function handleMatchedTrigger( - registry: TriggerRegistry, - payload: TrelloWebhookPayload, - actionType: string | undefined, - project: ProjectConfig, - config: CascadeConfig, - pmProvider: ReturnType, - ackCommentId?: string, -): Promise { - const ctx: TriggerContext = { project, source: 'trello', payload }; - const result = await registry.dispatch(ctx); - if (!result) { - logger.info('No trigger matched for webhook', { actionType }); - if (ackCommentId) { - await cleanupOrphanTrelloAck(project.id, payload, ackCommentId); - } - return; - } - - // Pass ack comment ID into agent input for ProgressMonitor pre-seeding - if (ackCommentId) { - result.agentInput.ackCommentId = ackCommentId; - } - - const cardId = result.cardId ?? result.workItemId; - if (cardId && isCardActive(cardId)) { - logger.info('Card already being processed, skipping', { cardId }); - return; - } - - logger.info('Trigger matched', { agentType: result.agentType, cardId }); - setProcessing(true); - startWatchdog(config.defaults.watchdogTimeoutMs); - - const pmConfig = resolveProjectPMConfig(project); - const lifecycle = new PMLifecycleManager(pmProvider, pmConfig); - - try { - if (cardId) { - setCardActive(cardId); - } - await executeAgent(result, project, config); - } catch (err) { - logger.error('Failed to process webhook', { error: String(err) }); - if (cardId) { - await lifecycle.handleError(cardId, String(err)); - } - } finally { - if (cardId) { - clearCardActive(cardId); - } - setProcessing(false); - processNextQueued(registry); - } -} export async function processTrelloWebhook( payload: unknown, registry: TriggerRegistry, ackCommentId?: string, ): Promise { - logger.info('Processing Trello webhook'); - - if (!isTrelloWebhookPayload(payload)) { - logger.warn('Invalid Trello webhook payload', { - payload: JSON.stringify(payload).slice(0, 200), - }); - return; - } - - if (tryQueueWebhook(payload, ackCommentId)) { - return; - } - - const boardId = payload.model.id; - const actionType = payload.action?.type; - logger.info('Webhook details', { boardId, actionType }); - - const projectConfig = await loadProjectConfigByBoardId(boardId); - if (!projectConfig) { - logger.warn('No project configured for board', { boardId }); - return; - } - const { project, config } = projectConfig; - - // Establish Trello credential + PM provider scope for all downstream operations - const trelloApiKey = await getIntegrationCredential(project.id, 'pm', 'api_key'); - const trelloToken = await getIntegrationCredential(project.id, 'pm', 'token'); - const pmProvider = createPMProvider(project); - - await withTrelloCredentials({ apiKey: trelloApiKey, token: trelloToken }, () => - withPMProvider(pmProvider, () => - handleMatchedTrigger( - registry, - payload, - actionType, - project, - config, - pmProvider, - ackCommentId, - ), - ), - ); + const integration = pmRegistry.get('trello'); + await processPMWebhook(integration, payload, registry, ackCommentId); } diff --git a/src/types/index.ts b/src/types/index.ts index 0a5b8db0..ee72b897 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -68,11 +68,8 @@ export interface TriggerContext { } export interface TriggerResult { - agentType: string; + agentType: string | null; agentInput: AgentInput; - /** @deprecated Use workItemId instead */ - cardId?: string; - /** Alias for cardId — preferred name for PM-agnostic code */ workItemId?: string; prNumber?: number; } diff --git a/tests/unit/gadgets/trello.test.ts b/tests/unit/gadgets/trello.test.ts deleted file mode 100644 index a04eca5b..00000000 --- a/tests/unit/gadgets/trello.test.ts +++ /dev/null @@ -1,136 +0,0 @@ -import { describe, expect, it } from 'vitest'; -import { - AddChecklistToCard, - CreateTrelloCard, - GetMyRecentActivity, - ListTrelloCards, - PostTrelloComment, - ReadTrelloCard, - UpdateTrelloCard, -} from '../../../src/gadgets/trello/index.js'; - -describe('Trello Gadgets', () => { - describe('ReadTrelloCard', () => { - it('is a valid llmist Gadget class', () => { - const gadget = new ReadTrelloCard(); - expect(gadget).toBeDefined(); - expect(typeof gadget.execute).toBe('function'); - }); - - it('has correct metadata', () => { - const gadget = new ReadTrelloCard(); - expect(gadget.name).toBe('ReadTrelloCard'); - expect(gadget.description).toContain('Trello card'); - }); - }); - - describe('PostTrelloComment', () => { - it('is a valid llmist Gadget class', () => { - const gadget = new PostTrelloComment(); - expect(gadget).toBeDefined(); - expect(typeof gadget.execute).toBe('function'); - }); - - it('has correct metadata', () => { - const gadget = new PostTrelloComment(); - expect(gadget.name).toBe('PostTrelloComment'); - expect(gadget.description).toContain('comment'); - }); - }); - - describe('UpdateTrelloCard', () => { - it('is a valid llmist Gadget class', () => { - const gadget = new UpdateTrelloCard(); - expect(gadget).toBeDefined(); - expect(typeof gadget.execute).toBe('function'); - }); - - it('has correct metadata', () => { - const gadget = new UpdateTrelloCard(); - expect(gadget.name).toBe('UpdateTrelloCard'); - expect(gadget.description).toContain('Update'); - }); - }); - - describe('CreateTrelloCard', () => { - it('is a valid llmist Gadget class', () => { - const gadget = new CreateTrelloCard(); - expect(gadget).toBeDefined(); - expect(typeof gadget.execute).toBe('function'); - }); - - it('has correct metadata', () => { - const gadget = new CreateTrelloCard(); - expect(gadget.name).toBe('CreateTrelloCard'); - expect(gadget.description).toContain('Create'); - }); - - it('has user story format in description', () => { - const gadget = new CreateTrelloCard(); - expect(gadget.description).toContain('user story'); - }); - }); - - describe('ListTrelloCards', () => { - it('is a valid llmist Gadget class', () => { - const gadget = new ListTrelloCards(); - expect(gadget).toBeDefined(); - expect(typeof gadget.execute).toBe('function'); - }); - - it('has correct metadata', () => { - const gadget = new ListTrelloCards(); - expect(gadget.name).toBe('ListTrelloCards'); - expect(gadget.description).toContain('List'); - }); - - it('mentions finding cards in description', () => { - const gadget = new ListTrelloCards(); - expect(gadget.description).toContain('find cards'); - }); - }); - - describe('GetMyRecentActivity', () => { - it('is a valid llmist Gadget class', () => { - const gadget = new GetMyRecentActivity(); - expect(gadget).toBeDefined(); - expect(typeof gadget.execute).toBe('function'); - }); - - it('has correct metadata', () => { - const gadget = new GetMyRecentActivity(); - expect(gadget.name).toBe('GetMyRecentActivity'); - expect(gadget.description).toContain('recent'); - }); - - it('mentions activity types in description', () => { - const gadget = new GetMyRecentActivity(); - expect(gadget.description).toContain('created'); - expect(gadget.description).toContain('updated'); - }); - }); - - describe('AddChecklistToCard', () => { - it('is a valid llmist Gadget class', () => { - const gadget = new AddChecklistToCard(); - expect(gadget).toBeDefined(); - expect(typeof gadget.execute).toBe('function'); - }); - - it('has correct metadata', () => { - const gadget = new AddChecklistToCard(); - expect(gadget.name).toBe('AddChecklistToCard'); - expect(gadget.description).toContain('checklist'); - }); - - it('mentions acceptance criteria in description', () => { - const gadget = new AddChecklistToCard(); - expect(gadget.description).toContain('acceptance criteria'); - }); - - it('mentions implementation steps in description', () => { - const gadget = new AddChecklistToCard(); - expect(gadget.description).toContain('implementation steps'); - }); - }); -}); diff --git a/tests/unit/gadgets/trello/core.test.ts b/tests/unit/gadgets/trello/core.test.ts deleted file mode 100644 index c05dec79..00000000 --- a/tests/unit/gadgets/trello/core.test.ts +++ /dev/null @@ -1,333 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; - -vi.mock('../../../../src/trello/client.js', () => ({ - trelloClient: { - getCard: vi.fn(), - getCardChecklists: vi.fn(), - getCardAttachments: vi.fn(), - getCardComments: vi.fn(), - addComment: vi.fn(), - updateCard: vi.fn(), - addLabelToCard: vi.fn(), - createCard: vi.fn(), - getListCards: vi.fn(), - createChecklist: vi.fn(), - addChecklistItem: vi.fn(), - updateChecklistItem: vi.fn(), - }, -})); - -import { addChecklist } from '../../../../src/gadgets/trello/core/addChecklist.js'; -import { createCard } from '../../../../src/gadgets/trello/core/createCard.js'; -import { listCards } from '../../../../src/gadgets/trello/core/listCards.js'; -import { postComment } from '../../../../src/gadgets/trello/core/postComment.js'; -import { readCard } from '../../../../src/gadgets/trello/core/readCard.js'; -import { updateCard } from '../../../../src/gadgets/trello/core/updateCard.js'; -import { updateChecklistItem } from '../../../../src/gadgets/trello/core/updateChecklistItem.js'; -import { trelloClient } from '../../../../src/trello/client.js'; - -const mockTrello = vi.mocked(trelloClient); - -beforeEach(() => { - vi.clearAllMocks(); -}); - -describe('readCard', () => { - const mockCard = { - name: 'Test Card', - url: 'https://trello.com/c/abc123', - desc: 'A description', - labels: [{ name: 'Bug', color: 'red' }], - }; - - it('formats card with title, description, labels, checklists, attachments', async () => { - mockTrello.getCard.mockResolvedValue(mockCard as ReturnType); - mockTrello.getCardChecklists.mockResolvedValue([ - { - id: 'cl1', - name: 'Tasks', - checkItems: [{ id: 'ci1', name: 'Item 1', state: 'incomplete' }], - }, - ] as Awaited>); - mockTrello.getCardAttachments.mockResolvedValue([ - { name: 'file.txt', url: 'https://example.com/file.txt', date: '2024-01-01T00:00:00Z' }, - ] as Awaited>); - mockTrello.getCardComments.mockResolvedValue([]); - - const result = await readCard('abc123', false); - - expect(result).toContain('# Test Card'); - expect(result).toContain('A description'); - expect(result).toContain('Bug (red)'); - expect(result).toContain('Tasks'); - expect(result).toContain('[ ] Item 1'); - expect(result).toContain('file.txt'); - }); - - it('shows "(No description)" when desc empty', async () => { - mockTrello.getCard.mockResolvedValue({ - ...mockCard, - desc: '', - } as ReturnType); - mockTrello.getCardChecklists.mockResolvedValue([]); - mockTrello.getCardAttachments.mockResolvedValue([]); - - const result = await readCard('abc123', false); - - expect(result).toContain('(No description)'); - }); - - it('fetches comments when includeComments=true', async () => { - mockTrello.getCard.mockResolvedValue(mockCard as ReturnType); - mockTrello.getCardChecklists.mockResolvedValue([]); - mockTrello.getCardAttachments.mockResolvedValue([]); - mockTrello.getCardComments.mockResolvedValue([ - { - date: '2024-01-01T00:00:00Z', - memberCreator: { fullName: 'John' }, - data: { text: 'Hello world' }, - }, - ] as Awaited>); - - const result = await readCard('abc123', true); - - expect(result).toContain('John'); - expect(result).toContain('Hello world'); - expect(mockTrello.getCardComments).toHaveBeenCalledWith('abc123'); - }); - - it('skips comments when includeComments=false', async () => { - mockTrello.getCard.mockResolvedValue(mockCard as ReturnType); - mockTrello.getCardChecklists.mockResolvedValue([]); - mockTrello.getCardAttachments.mockResolvedValue([]); - - await readCard('abc123', false); - - expect(mockTrello.getCardComments).not.toHaveBeenCalled(); - }); - - it('returns error message on API failure', async () => { - mockTrello.getCard.mockRejectedValue(new Error('API down')); - - const result = await readCard('abc123'); - - expect(result).toBe('Error reading card: API down'); - }); -}); - -describe('postComment', () => { - it('returns success message', async () => { - mockTrello.addComment.mockResolvedValue(undefined as never); - - const result = await postComment('card1', 'Hello'); - - expect(result).toBe('Comment posted successfully'); - expect(mockTrello.addComment).toHaveBeenCalledWith('card1', 'Hello'); - }); - - it('returns error message on failure', async () => { - mockTrello.addComment.mockRejectedValue(new Error('Network error')); - - const result = await postComment('card1', 'Hello'); - - expect(result).toBe('Error posting comment: Network error'); - }); -}); - -describe('updateCard', () => { - it('returns early when nothing to update', async () => { - const result = await updateCard({ cardId: 'card1' }); - - expect(result).toBe('Nothing to update - provide title, description, or labels'); - expect(mockTrello.updateCard).not.toHaveBeenCalled(); - }); - - it('updates title and description', async () => { - mockTrello.updateCard.mockResolvedValue(undefined as never); - - const result = await updateCard({ - cardId: 'card1', - title: 'New Title', - description: 'New Desc', - }); - - expect(mockTrello.updateCard).toHaveBeenCalledWith('card1', { - name: 'New Title', - desc: 'New Desc', - }); - expect(result).toContain('title'); - expect(result).toContain('description'); - }); - - it('adds labels', async () => { - mockTrello.addLabelToCard.mockResolvedValue(undefined as never); - - const result = await updateCard({ - cardId: 'card1', - addLabelIds: ['label1', 'label2'], - }); - - expect(mockTrello.addLabelToCard).toHaveBeenCalledTimes(2); - expect(result).toContain('2 label(s)'); - }); - - it('returns summary of what was updated', async () => { - mockTrello.updateCard.mockResolvedValue(undefined as never); - mockTrello.addLabelToCard.mockResolvedValue(undefined as never); - - const result = await updateCard({ - cardId: 'card1', - title: 'Title', - addLabelIds: ['l1'], - }); - - expect(result).toBe('Card updated: title, 1 label(s)'); - }); - - it('returns error message on failure', async () => { - mockTrello.updateCard.mockRejectedValue(new Error('Forbidden')); - - const result = await updateCard({ cardId: 'card1', title: 'New' }); - - expect(result).toBe('Error updating card: Forbidden'); - }); -}); - -describe('createCard', () => { - it('returns success with card name and URL', async () => { - mockTrello.createCard.mockResolvedValue({ - name: 'My Card', - shortUrl: 'https://trello.com/c/xyz', - } as Awaited>); - - const result = await createCard({ listId: 'list1', title: 'My Card' }); - - expect(result).toBe('Card created successfully: "My Card" - https://trello.com/c/xyz'); - }); - - it('returns error message on failure', async () => { - mockTrello.createCard.mockRejectedValue(new Error('List not found')); - - const result = await createCard({ listId: 'bad', title: 'Test' }); - - expect(result).toBe('Error creating card: List not found'); - }); -}); - -describe('listCards', () => { - it('formats cards with name, ID, URL', async () => { - mockTrello.getListCards.mockResolvedValue([ - { id: 'c1', name: 'Card 1', shortUrl: 'https://trello.com/c/1', desc: '' }, - { id: 'c2', name: 'Card 2', shortUrl: 'https://trello.com/c/2', desc: 'Short desc' }, - ] as Awaited>); - - const result = await listCards('list1'); - - expect(result).toContain('Cards (2)'); - expect(result).toContain('Card 1'); - expect(result).toContain('c1'); - expect(result).toContain('Card 2'); - }); - - it('returns empty message when no cards', async () => { - mockTrello.getListCards.mockResolvedValue([]); - - const result = await listCards('list1'); - - expect(result).toBe('No cards found in this list.'); - }); - - it('truncates long descriptions at 100 chars', async () => { - const longDesc = 'a'.repeat(150); - mockTrello.getListCards.mockResolvedValue([ - { id: 'c1', name: 'Card', shortUrl: 'https://trello.com/c/1', desc: longDesc }, - ] as Awaited>); - - const result = await listCards('list1'); - - expect(result).toContain(`${'a'.repeat(100)}...`); - expect(result).not.toContain('a'.repeat(101)); - }); - - it('returns error message on failure', async () => { - mockTrello.getListCards.mockRejectedValue(new Error('Board not found')); - - const result = await listCards('list1'); - - expect(result).toBe('Error listing cards: Board not found'); - }); -}); - -describe('addChecklist', () => { - it('creates checklist and adds items', async () => { - mockTrello.createChecklist.mockResolvedValue({ id: 'cl1' } as Awaited< - ReturnType - >); - mockTrello.addChecklistItem.mockResolvedValue(undefined as never); - - const result = await addChecklist({ - cardId: 'card1', - checklistName: 'Tasks', - items: ['Item 1', 'Item 2'], - }); - - expect(mockTrello.createChecklist).toHaveBeenCalledWith('card1', 'Tasks'); - expect(mockTrello.addChecklistItem).toHaveBeenCalledTimes(2); - expect(mockTrello.addChecklistItem).toHaveBeenCalledWith('cl1', 'Item 1'); - expect(mockTrello.addChecklistItem).toHaveBeenCalledWith('cl1', 'Item 2'); - }); - - it('returns success with item count', async () => { - mockTrello.createChecklist.mockResolvedValue({ id: 'cl1' } as Awaited< - ReturnType - >); - mockTrello.addChecklistItem.mockResolvedValue(undefined as never); - - const result = await addChecklist({ - cardId: 'card1', - checklistName: 'Checklist', - items: ['A', 'B', 'C'], - }); - - expect(result).toContain('3 items'); - }); - - it('returns error message on failure', async () => { - mockTrello.createChecklist.mockRejectedValue(new Error('Card not found')); - - const result = await addChecklist({ - cardId: 'bad', - checklistName: 'Tasks', - items: ['Item'], - }); - - expect(result).toBe('Error adding checklist: Card not found'); - }); -}); - -describe('updateChecklistItem', () => { - it('returns success for "complete" state', async () => { - mockTrello.updateChecklistItem.mockResolvedValue(undefined as never); - - const result = await updateChecklistItem('card1', 'ci1', 'complete'); - - expect(result).toContain('marked complete'); - expect(mockTrello.updateChecklistItem).toHaveBeenCalledWith('card1', 'ci1', 'complete'); - }); - - it('returns success for "incomplete" state', async () => { - mockTrello.updateChecklistItem.mockResolvedValue(undefined as never); - - const result = await updateChecklistItem('card1', 'ci1', 'incomplete'); - - expect(result).toContain('marked incomplete'); - }); - - it('returns error message on failure', async () => { - mockTrello.updateChecklistItem.mockRejectedValue(new Error('Not found')); - - const result = await updateChecklistItem('card1', 'ci1', 'complete'); - - expect(result).toBe('Error updating checklist item: Not found'); - }); -}); diff --git a/tests/unit/pm/factory.test.ts b/tests/unit/pm/factory.test.ts index 09e625d1..f2d4654c 100644 --- a/tests/unit/pm/factory.test.ts +++ b/tests/unit/pm/factory.test.ts @@ -1,5 +1,4 @@ import { describe, expect, it, vi } from 'vitest'; -import { createPMProvider } from '../../../src/pm/factory.js'; import type { ProjectConfig } from '../../../src/types/index.js'; // Mock the adapters @@ -20,6 +19,27 @@ vi.mock('../../../src/pm/jira/adapter.js', () => ({ })), })); +// Mock provider.ts to avoid DB calls from integration constructors +vi.mock('../../../src/config/provider.js', () => ({ + getIntegrationCredential: vi.fn().mockResolvedValue('mock-cred'), + loadProjectConfigByBoardId: vi.fn().mockResolvedValue(null), + loadProjectConfigByJiraProjectKey: vi.fn().mockResolvedValue(null), + findProjectById: vi.fn().mockResolvedValue(null), +})); + +vi.mock('../../../src/trello/client.js', () => ({ + withTrelloCredentials: vi.fn((_creds, fn) => fn()), + trelloClient: {}, +})); + +vi.mock('../../../src/jira/client.js', () => ({ + withJiraCredentials: vi.fn((_creds, fn) => fn()), + jiraClient: {}, +})); + +// Import after mocks so the integrations register with mocked adapters +// factory.ts was removed; createPMProvider is now an inline function in index.ts +import { createPMProvider } from '../../../src/pm/index.js'; import { JiraPMProvider } from '../../../src/pm/jira/adapter.js'; import { TrelloPMProvider } from '../../../src/pm/trello/adapter.js'; @@ -107,7 +127,7 @@ describe('pm/factory', () => { }; expect(() => createPMProvider(project)).toThrow( - "Project 'proj1' has pm.type=jira but no jira config", + 'JIRA integration requires projectKey in config', ); }); @@ -122,7 +142,7 @@ describe('pm/factory', () => { pm: { type: 'unknown' }, } as ProjectConfig; - expect(() => createPMProvider(project)).toThrow('Unknown PM type: unknown'); + expect(() => createPMProvider(project)).toThrow("Unknown PM integration type: 'unknown'"); }); }); }); diff --git a/tests/unit/pm/lifecycle.test.ts b/tests/unit/pm/lifecycle.test.ts index 61a9b180..f45c7374 100644 --- a/tests/unit/pm/lifecycle.test.ts +++ b/tests/unit/pm/lifecycle.test.ts @@ -1,4 +1,30 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; + +// Mock dependencies before imports +vi.mock('../../../src/config/provider.js', () => ({ + getIntegrationCredential: vi.fn().mockResolvedValue('mock-cred'), + loadProjectConfigByBoardId: vi.fn().mockResolvedValue(null), + loadProjectConfigByJiraProjectKey: vi.fn().mockResolvedValue(null), + findProjectById: vi.fn().mockResolvedValue(null), +})); + +vi.mock('../../../src/trello/client.js', () => ({ + withTrelloCredentials: vi.fn((_creds, fn) => fn()), + trelloClient: {}, +})); + +vi.mock('../../../src/jira/client.js', () => ({ + withJiraCredentials: vi.fn((_creds, fn) => fn()), + jiraClient: {}, +})); + +vi.mock('../../../src/utils/safeOperation.js', () => ({ + safeOperation: vi.fn((fn) => fn()), + silentOperation: vi.fn((fn) => fn()), +})); + +// Import after mocks — side-effect import registers integrations with pmRegistry +import '../../../src/pm/index.js'; import { PMLifecycleManager, type ProjectPMConfig, @@ -7,12 +33,6 @@ import { import type { PMProvider } from '../../../src/pm/types.js'; import type { ProjectConfig } from '../../../src/types/index.js'; -// Mock safeOperation utilities -vi.mock('../../../src/utils/safeOperation.js', () => ({ - safeOperation: vi.fn((fn) => fn()), - silentOperation: vi.fn((fn) => fn()), -})); - describe('pm/lifecycle', () => { describe('resolveProjectPMConfig', () => { it('returns JIRA config when project type is jira', () => { diff --git a/tests/unit/triggers/agent-execution.test.ts b/tests/unit/triggers/agent-execution.test.ts index 0e36d8da..5d5305b9 100644 --- a/tests/unit/triggers/agent-execution.test.ts +++ b/tests/unit/triggers/agent-execution.test.ts @@ -81,7 +81,7 @@ const mockConfig: CascadeConfig = { const mockTriggerResult: TriggerResult = { agentType: 'implementation', - cardId: 'card-123', + workItemId: 'card-123', agentInput: { someInput: 'value' }, }; @@ -304,7 +304,7 @@ describe('runAgentExecutionPipeline', () => { it('uses cardId when present', async () => { const result: TriggerResult = { agentType: 'implementation', - cardId: 'card-456', + workItemId: 'card-456', agentInput: {}, }; @@ -313,7 +313,7 @@ describe('runAgentExecutionPipeline', () => { expect(mockLifecycle.prepareForAgent).toHaveBeenCalledWith('card-456', 'implementation'); }); - it('falls back to workItemId when cardId is absent', async () => { + it('uses workItemId when present', async () => { const result: TriggerResult = { agentType: 'implementation', workItemId: 'issue-789', diff --git a/tests/unit/triggers/card-moved.test.ts b/tests/unit/triggers/card-moved.test.ts index aa29529c..8af247b4 100644 --- a/tests/unit/triggers/card-moved.test.ts +++ b/tests/unit/triggers/card-moved.test.ts @@ -1,4 +1,35 @@ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; + +// Mocks required for PM integration registration (pm/index.js side-effect) +vi.mock('../../../src/config/provider.js', () => ({ + getIntegrationCredential: vi.fn(), + loadProjectConfigByBoardId: vi.fn(), + loadProjectConfigByJiraProjectKey: vi.fn(), + findProjectById: vi.fn(), +})); +vi.mock('../../../src/trello/client.js', () => ({ + withTrelloCredentials: vi.fn(), + trelloClient: { getCard: vi.fn() }, +})); +vi.mock('../../../src/jira/client.js', () => ({ + withJiraCredentials: vi.fn(), + jiraClient: {}, +})); +vi.mock('../../../src/router/acknowledgments.js', () => ({ + postTrelloAck: vi.fn(), + deleteTrelloAck: vi.fn(), + resolveTrelloBotMemberId: vi.fn(), + postJiraAck: vi.fn(), + deleteJiraAck: vi.fn(), + resolveJiraBotAccountId: vi.fn(), +})); +vi.mock('../../../src/router/reactions.js', () => ({ + sendAcknowledgeReaction: vi.fn(), +})); + +// Register PM integrations in the registry +import '../../../src/pm/index.js'; + import { CardMovedToBriefingTrigger, CardMovedToPlanningTrigger, @@ -159,7 +190,7 @@ describe('CardMovedToBriefingTrigger', () => { const result = await trigger.handle(ctx); expect(result.agentType).toBe('briefing'); - expect(result.cardId).toBe('card123'); + expect(result.workItemId).toBe('card123'); expect(result.agentInput.cardId).toBe('card123'); }); }); @@ -240,6 +271,6 @@ describe('CardMovedToTodoTrigger', () => { const result = await trigger.handle(ctx); expect(result.agentType).toBe('implementation'); - expect(result.cardId).toBe('card456'); + expect(result.workItemId).toBe('card456'); }); }); diff --git a/tests/unit/triggers/check-suite-failure.test.ts b/tests/unit/triggers/check-suite-failure.test.ts index a4dff112..c0cd0ba5 100644 --- a/tests/unit/triggers/check-suite-failure.test.ts +++ b/tests/unit/triggers/check-suite-failure.test.ts @@ -179,7 +179,7 @@ describe('CheckSuiteFailureTrigger', () => { cardId: 'abc123', }, prNumber: 42, - cardId: 'abc123', + workItemId: 'abc123', }); }); diff --git a/tests/unit/triggers/check-suite-success.test.ts b/tests/unit/triggers/check-suite-success.test.ts index d9433245..bd8388ac 100644 --- a/tests/unit/triggers/check-suite-success.test.ts +++ b/tests/unit/triggers/check-suite-success.test.ts @@ -187,7 +187,7 @@ describe('CheckSuiteSuccessTrigger', () => { cardId: 'abc123', }, prNumber: 42, - cardId: 'abc123', + workItemId: 'abc123', }); }); diff --git a/tests/unit/triggers/github-pr-comment-mention.test.ts b/tests/unit/triggers/github-pr-comment-mention.test.ts index 373f8ed6..bdaedb79 100644 --- a/tests/unit/triggers/github-pr-comment-mention.test.ts +++ b/tests/unit/triggers/github-pr-comment-mention.test.ts @@ -211,7 +211,7 @@ describe('PRCommentMentionTrigger', () => { expect(result).not.toBeNull(); expect(result?.agentType).toBe('respond-to-pr-comment'); - expect(result?.cardId).toBe(CARD_SHORT_ID); + expect(result?.workItemId).toBe(CARD_SHORT_ID); expect(result?.agentInput.prNumber).toBe(42); expect(result?.agentInput.repoFullName).toBe('owner/repo'); expect(result?.agentInput.triggerCommentBody).toContain(`@${IMPLEMENTER_USERNAME}`); diff --git a/tests/unit/triggers/jira-comment-mention.test.ts b/tests/unit/triggers/jira-comment-mention.test.ts index 977c8ee3..7af32e37 100644 --- a/tests/unit/triggers/jira-comment-mention.test.ts +++ b/tests/unit/triggers/jira-comment-mention.test.ts @@ -219,7 +219,6 @@ describe('JiraCommentMentionTrigger', () => { expect(result).not.toBeNull(); expect(result?.agentType).toBe('respond-to-planning-comment'); expect(result?.workItemId).toBe('DAM-13'); - expect(result?.cardId).toBe('DAM-13'); expect(result?.agentInput.triggerCommentAuthor).toBe('Human User'); }); diff --git a/tests/unit/triggers/jira-issue-transitioned.test.ts b/tests/unit/triggers/jira-issue-transitioned.test.ts index 21b2bd80..d9a2efe0 100644 --- a/tests/unit/triggers/jira-issue-transitioned.test.ts +++ b/tests/unit/triggers/jira-issue-transitioned.test.ts @@ -158,7 +158,6 @@ describe('JiraIssueTransitionedTrigger', () => { expect(result).not.toBeNull(); expect(result?.agentType).toBe('implementation'); expect(result?.workItemId).toBe('PROJ-42'); - expect(result?.cardId).toBe('PROJ-42'); expect(result?.agentInput).toEqual({ cardId: 'PROJ-42' }); }); diff --git a/tests/unit/triggers/jira-label-added.test.ts b/tests/unit/triggers/jira-label-added.test.ts index 52383d09..31691c8e 100644 --- a/tests/unit/triggers/jira-label-added.test.ts +++ b/tests/unit/triggers/jira-label-added.test.ts @@ -1,4 +1,35 @@ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; + +// Mocks required for PM integration registration (pm/index.js side-effect) +vi.mock('../../../src/config/provider.js', () => ({ + getIntegrationCredential: vi.fn(), + loadProjectConfigByBoardId: vi.fn(), + loadProjectConfigByJiraProjectKey: vi.fn(), + findProjectById: vi.fn(), +})); +vi.mock('../../../src/trello/client.js', () => ({ + withTrelloCredentials: vi.fn(), + trelloClient: { getCard: vi.fn() }, +})); +vi.mock('../../../src/jira/client.js', () => ({ + withJiraCredentials: vi.fn(), + jiraClient: {}, +})); +vi.mock('../../../src/router/acknowledgments.js', () => ({ + postTrelloAck: vi.fn(), + deleteTrelloAck: vi.fn(), + resolveTrelloBotMemberId: vi.fn(), + postJiraAck: vi.fn(), + deleteJiraAck: vi.fn(), + resolveJiraBotAccountId: vi.fn(), +})); +vi.mock('../../../src/router/reactions.js', () => ({ + sendAcknowledgeReaction: vi.fn(), +})); + +// Register PM integrations in the registry +import '../../../src/pm/index.js'; + import { JiraReadyToProcessLabelTrigger } from '../../../src/triggers/jira/label-added.js'; import type { TriggerContext } from '../../../src/types/index.js'; @@ -212,7 +243,6 @@ describe('JiraReadyToProcessLabelTrigger', () => { expect(result).not.toBeNull(); expect(result?.agentType).toBe('briefing'); expect(result?.workItemId).toBe('TEST-42'); - expect(result?.cardId).toBe('TEST-42'); expect(result?.agentInput.cardId).toBe('TEST-42'); }); diff --git a/tests/unit/triggers/label-added.test.ts b/tests/unit/triggers/label-added.test.ts index b1515683..3a1e1578 100644 --- a/tests/unit/triggers/label-added.test.ts +++ b/tests/unit/triggers/label-added.test.ts @@ -1,13 +1,42 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { TriggerContext } from '../../../src/triggers/types.js'; -// Import the module first, then mock it -import * as trelloClientModule from '../../../src/trello/client.js'; +// Mocks required for PM integration registration (pm/index.js side-effect) +vi.mock('../../../src/config/provider.js', () => ({ + getIntegrationCredential: vi.fn(), + loadProjectConfigByBoardId: vi.fn(), + loadProjectConfigByJiraProjectKey: vi.fn(), + findProjectById: vi.fn(), +})); +vi.mock('../../../src/trello/client.js', () => ({ + withTrelloCredentials: vi.fn(), + trelloClient: { getCard: vi.fn() }, +})); +vi.mock('../../../src/jira/client.js', () => ({ + withJiraCredentials: vi.fn(), + jiraClient: {}, +})); +vi.mock('../../../src/router/acknowledgments.js', () => ({ + postTrelloAck: vi.fn(), + deleteTrelloAck: vi.fn(), + resolveTrelloBotMemberId: vi.fn(), + postJiraAck: vi.fn(), + deleteJiraAck: vi.fn(), + resolveJiraBotAccountId: vi.fn(), +})); +vi.mock('../../../src/router/reactions.js', () => ({ + sendAcknowledgeReaction: vi.fn(), +})); + +// Register PM integrations in the registry +import '../../../src/pm/index.js'; + +import { trelloClient } from '../../../src/trello/client.js'; import { ReadyToProcessLabelTrigger } from '../../../src/triggers/trello/label-added.js'; describe('ReadyToProcessLabelTrigger', () => { const trigger = new ReadyToProcessLabelTrigger(); - let mockGetCard: ReturnType; + const mockGetCard = vi.mocked(trelloClient.getCard); const mockProject = { id: 'test', @@ -29,12 +58,7 @@ describe('ReadyToProcessLabelTrigger', () => { }; beforeEach(() => { - mockGetCard = vi.fn(); - vi.spyOn(trelloClientModule.trelloClient, 'getCard').mockImplementation(mockGetCard); - }); - - afterEach(() => { - vi.restoreAllMocks(); + vi.clearAllMocks(); }); describe('matches', () => { @@ -171,7 +195,7 @@ describe('ReadyToProcessLabelTrigger', () => { const result = await trigger.handle(ctx); expect(result.agentType).toBe('briefing'); - expect(result.cardId).toBe('card123'); + expect(result.workItemId).toBe('card123'); expect(mockGetCard).toHaveBeenCalledWith('card123'); }); @@ -207,7 +231,7 @@ describe('ReadyToProcessLabelTrigger', () => { const result = await trigger.handle(ctx); expect(result.agentType).toBe('planning'); - expect(result.cardId).toBe('card456'); + expect(result.workItemId).toBe('card456'); }); it('returns implementation agent when card is in todo list', async () => { @@ -242,7 +266,7 @@ describe('ReadyToProcessLabelTrigger', () => { const result = await trigger.handle(ctx); expect(result.agentType).toBe('implementation'); - expect(result.cardId).toBe('card789'); + expect(result.workItemId).toBe('card789'); }); it('defaults to briefing agent when card is in unknown list', async () => { diff --git a/tests/unit/triggers/pr-merged.test.ts b/tests/unit/triggers/pr-merged.test.ts index f86841b5..9fb198c5 100644 --- a/tests/unit/triggers/pr-merged.test.ts +++ b/tests/unit/triggers/pr-merged.test.ts @@ -1,6 +1,4 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { PRMergedTrigger } from '../../../src/triggers/github/pr-merged.js'; -import type { TriggerContext } from '../../../src/triggers/types.js'; // Mock the GitHub client vi.mock('../../../src/github/client.js', () => ({ @@ -9,17 +7,50 @@ vi.mock('../../../src/github/client.js', () => ({ }, })); -// Mock the Trello client +// Mock the PM provider context +const mockProvider = { + getWorkItem: vi.fn(), + moveWorkItem: vi.fn(), + addComment: vi.fn(), +}; +vi.mock('../../../src/pm/context.js', () => ({ + getPMProvider: () => mockProvider, +})); + +// Mocks required for PM integration registration (pm/index.js side-effect) +vi.mock('../../../src/config/provider.js', () => ({ + getIntegrationCredential: vi.fn(), + loadProjectConfigByBoardId: vi.fn(), + loadProjectConfigByJiraProjectKey: vi.fn(), + findProjectById: vi.fn(), +})); vi.mock('../../../src/trello/client.js', () => ({ - trelloClient: { - getCard: vi.fn(), - moveCardToList: vi.fn(), - addComment: vi.fn(), - }, + withTrelloCredentials: vi.fn(), + trelloClient: { getCard: vi.fn() }, +})); +vi.mock('../../../src/jira/client.js', () => ({ + withJiraCredentials: vi.fn(), + jiraClient: {}, })); +vi.mock('../../../src/router/acknowledgments.js', () => ({ + postTrelloAck: vi.fn(), + deleteTrelloAck: vi.fn(), + resolveTrelloBotMemberId: vi.fn(), + postJiraAck: vi.fn(), + deleteJiraAck: vi.fn(), + resolveJiraBotAccountId: vi.fn(), +})); +vi.mock('../../../src/router/reactions.js', () => ({ + sendAcknowledgeReaction: vi.fn(), +})); + +// Register PM integrations in the registry +import '../../../src/pm/index.js'; + +import { PRMergedTrigger } from '../../../src/triggers/github/pr-merged.js'; +import type { TriggerContext } from '../../../src/triggers/types.js'; import { githubClient } from '../../../src/github/client.js'; -import { trelloClient } from '../../../src/trello/client.js'; describe('PRMergedTrigger', () => { const trigger = new PRMergedTrigger(); @@ -121,13 +152,12 @@ describe('PRMergedTrigger', () => { baseRef: 'main', merged: true, }); - vi.mocked(trelloClient.getCard).mockResolvedValue({ + mockProvider.getWorkItem.mockResolvedValue({ id: 'abc123', - name: 'Card', - desc: '', + title: 'Card', + description: '', url: '', - shortUrl: '', - idList: 'todo-list-id', + status: 'todo-list-id', labels: [], }); @@ -150,15 +180,15 @@ describe('PRMergedTrigger', () => { const result = await trigger.handle(ctx); expect(githubClient.getPR).toHaveBeenCalledWith('owner', 'repo', 123); - expect(trelloClient.moveCardToList).toHaveBeenCalledWith('abc123', 'merged-list-id'); - expect(trelloClient.addComment).toHaveBeenCalledWith( + expect(mockProvider.moveWorkItem).toHaveBeenCalledWith('abc123', 'merged-list-id'); + expect(mockProvider.addComment).toHaveBeenCalledWith( 'abc123', 'PR #123 has been merged to main', ); expect(result).toEqual({ - agentType: '', + agentType: null, agentInput: {}, - cardId: 'abc123', + workItemId: 'abc123', prNumber: 123, }); }); @@ -194,7 +224,7 @@ describe('PRMergedTrigger', () => { const result = await trigger.handle(ctx); expect(result).toBeNull(); - expect(trelloClient.moveCardToList).not.toHaveBeenCalled(); + expect(mockProvider.moveWorkItem).not.toHaveBeenCalled(); }); it('returns null when PR has no Trello card URL', async () => { @@ -228,7 +258,7 @@ describe('PRMergedTrigger', () => { const result = await trigger.handle(ctx); expect(result).toBeNull(); - expect(trelloClient.moveCardToList).not.toHaveBeenCalled(); + expect(mockProvider.moveWorkItem).not.toHaveBeenCalled(); }); it('skips move and comment when card is already in MERGED list', async () => { @@ -242,13 +272,12 @@ describe('PRMergedTrigger', () => { baseRef: 'main', merged: true, }); - vi.mocked(trelloClient.getCard).mockResolvedValue({ + mockProvider.getWorkItem.mockResolvedValue({ id: 'abc123', - name: 'Card', - desc: '', + title: 'Card', + description: '', url: '', - shortUrl: '', - idList: 'merged-list-id', + status: 'merged-list-id', labels: [], }); @@ -270,13 +299,13 @@ describe('PRMergedTrigger', () => { const result = await trigger.handle(ctx); - expect(trelloClient.getCard).toHaveBeenCalledWith('abc123'); - expect(trelloClient.moveCardToList).not.toHaveBeenCalled(); - expect(trelloClient.addComment).not.toHaveBeenCalled(); + expect(mockProvider.getWorkItem).toHaveBeenCalledWith('abc123'); + expect(mockProvider.moveWorkItem).not.toHaveBeenCalled(); + expect(mockProvider.addComment).not.toHaveBeenCalled(); expect(result).toEqual({ - agentType: '', + agentType: null, agentInput: {}, - cardId: 'abc123', + workItemId: 'abc123', prNumber: 123, }); }); @@ -325,7 +354,7 @@ describe('PRMergedTrigger', () => { const result = await trigger.handle(ctx); expect(result).toBeNull(); - expect(trelloClient.moveCardToList).not.toHaveBeenCalled(); + expect(mockProvider.moveWorkItem).not.toHaveBeenCalled(); }); }); }); diff --git a/tests/unit/triggers/pr-opened.test.ts b/tests/unit/triggers/pr-opened.test.ts index ebb3c75c..c2b8027c 100644 --- a/tests/unit/triggers/pr-opened.test.ts +++ b/tests/unit/triggers/pr-opened.test.ts @@ -212,7 +212,7 @@ describe('PROpenedTrigger', () => { triggerCommentUrl: 'https://github.com/owner/repo/pull/42', }, prNumber: 42, - cardId: 'abc123', + workItemId: 'abc123', }); }); diff --git a/tests/unit/triggers/pr-ready-to-merge.test.ts b/tests/unit/triggers/pr-ready-to-merge.test.ts index fc14c5f9..63c34ac5 100644 --- a/tests/unit/triggers/pr-ready-to-merge.test.ts +++ b/tests/unit/triggers/pr-ready-to-merge.test.ts @@ -1,6 +1,4 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { PRReadyToMergeTrigger } from '../../../src/triggers/github/pr-ready-to-merge.js'; -import type { TriggerContext } from '../../../src/triggers/types.js'; vi.mock('../../../src/github/client.js', () => ({ githubClient: { @@ -10,16 +8,50 @@ vi.mock('../../../src/github/client.js', () => ({ }, })); +// Mock the PM provider context +const mockProvider = { + getWorkItem: vi.fn(), + moveWorkItem: vi.fn(), + addComment: vi.fn(), +}; +vi.mock('../../../src/pm/context.js', () => ({ + getPMProvider: () => mockProvider, +})); + +// Mocks required for PM integration registration (pm/index.js side-effect) +vi.mock('../../../src/config/provider.js', () => ({ + getIntegrationCredential: vi.fn(), + loadProjectConfigByBoardId: vi.fn(), + loadProjectConfigByJiraProjectKey: vi.fn(), + findProjectById: vi.fn(), +})); vi.mock('../../../src/trello/client.js', () => ({ - trelloClient: { - getCard: vi.fn(), - moveCardToList: vi.fn(), - addComment: vi.fn(), - }, + withTrelloCredentials: vi.fn(), + trelloClient: { getCard: vi.fn() }, +})); +vi.mock('../../../src/jira/client.js', () => ({ + withJiraCredentials: vi.fn(), + jiraClient: {}, })); +vi.mock('../../../src/router/acknowledgments.js', () => ({ + postTrelloAck: vi.fn(), + deleteTrelloAck: vi.fn(), + resolveTrelloBotMemberId: vi.fn(), + postJiraAck: vi.fn(), + deleteJiraAck: vi.fn(), + resolveJiraBotAccountId: vi.fn(), +})); +vi.mock('../../../src/router/reactions.js', () => ({ + sendAcknowledgeReaction: vi.fn(), +})); + +// Register PM integrations in the registry +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 { githubClient } from '../../../src/github/client.js'; -import { trelloClient } from '../../../src/trello/client.js'; describe('PRReadyToMergeTrigger', () => { const trigger = new PRReadyToMergeTrigger(); @@ -246,13 +278,12 @@ describe('PRReadyToMergeTrigger', () => { commitId: 'sha123', }, ]); - vi.mocked(trelloClient.getCard).mockResolvedValue({ + mockProvider.getWorkItem.mockResolvedValue({ id: 'abc123', - name: 'Card', - desc: '', + title: 'Card', + description: '', url: '', - shortUrl: '', - idList: 'todo-list-id', + status: 'todo-list-id', labels: [], }); @@ -275,15 +306,15 @@ describe('PRReadyToMergeTrigger', () => { const result = await trigger.handle(ctx); - expect(trelloClient.moveCardToList).toHaveBeenCalledWith('abc123', 'done-list-id'); - expect(trelloClient.addComment).toHaveBeenCalledWith( + expect(mockProvider.moveWorkItem).toHaveBeenCalledWith('abc123', 'done-list-id'); + expect(mockProvider.addComment).toHaveBeenCalledWith( 'abc123', 'PR #42 approved and all checks passing - moved to DONE', ); expect(result).toEqual({ - agentType: '', + agentType: null, agentInput: {}, - cardId: 'abc123', + workItemId: 'abc123', prNumber: 42, }); }); @@ -307,13 +338,12 @@ describe('PRReadyToMergeTrigger', () => { commitId: 'sha123', }, ]); - vi.mocked(trelloClient.getCard).mockResolvedValue({ + mockProvider.getWorkItem.mockResolvedValue({ id: 'abc123', - name: 'Card', - desc: '', + title: 'Card', + description: '', url: '', - shortUrl: '', - idList: 'todo-list-id', + status: 'todo-list-id', labels: [], }); @@ -344,8 +374,8 @@ describe('PRReadyToMergeTrigger', () => { const result = await trigger.handle(ctx); - expect(trelloClient.moveCardToList).toHaveBeenCalledWith('abc123', 'done-list-id'); - expect(result?.cardId).toBe('abc123'); + expect(mockProvider.moveWorkItem).toHaveBeenCalledWith('abc123', 'done-list-id'); + expect(result?.workItemId).toBe('abc123'); }); it('returns null when PR has no Trello URL (check_suite path)', async () => { @@ -380,7 +410,7 @@ describe('PRReadyToMergeTrigger', () => { const result = await trigger.handle(ctx); expect(result).toBeNull(); - expect(trelloClient.moveCardToList).not.toHaveBeenCalled(); + expect(mockProvider.moveWorkItem).not.toHaveBeenCalled(); }); it('returns null when checks are not all passing', async () => { @@ -423,7 +453,7 @@ describe('PRReadyToMergeTrigger', () => { const result = await trigger.handle(ctx); expect(result).toBeNull(); - expect(trelloClient.moveCardToList).not.toHaveBeenCalled(); + expect(mockProvider.moveWorkItem).not.toHaveBeenCalled(); }); it('returns null when no approval exists', async () => { @@ -561,13 +591,12 @@ describe('PRReadyToMergeTrigger', () => { commitId: 'sha123', }, ]); - vi.mocked(trelloClient.getCard).mockResolvedValue({ + mockProvider.getWorkItem.mockResolvedValue({ id: 'abc123', - name: 'Card', - desc: '', + title: 'Card', + description: '', url: '', - shortUrl: '', - idList: 'done-list-id', + status: 'done-list-id', labels: [], }); @@ -590,13 +619,13 @@ describe('PRReadyToMergeTrigger', () => { const result = await trigger.handle(ctx); - expect(trelloClient.getCard).toHaveBeenCalledWith('abc123'); - expect(trelloClient.moveCardToList).not.toHaveBeenCalled(); - expect(trelloClient.addComment).not.toHaveBeenCalled(); + expect(mockProvider.getWorkItem).toHaveBeenCalledWith('abc123'); + expect(mockProvider.moveWorkItem).not.toHaveBeenCalled(); + expect(mockProvider.addComment).not.toHaveBeenCalled(); expect(result).toEqual({ - agentType: '', + agentType: null, agentInput: {}, - cardId: 'abc123', + workItemId: 'abc123', prNumber: 42, }); }); @@ -661,7 +690,7 @@ describe('PRReadyToMergeTrigger', () => { const result = await trigger.handle(ctx); expect(result).toBeNull(); - expect(trelloClient.moveCardToList).not.toHaveBeenCalled(); + expect(mockProvider.moveWorkItem).not.toHaveBeenCalled(); }); }); }); diff --git a/tests/unit/triggers/pr-review-submitted.test.ts b/tests/unit/triggers/pr-review-submitted.test.ts index 042cab41..97a816fa 100644 --- a/tests/unit/triggers/pr-review-submitted.test.ts +++ b/tests/unit/triggers/pr-review-submitted.test.ts @@ -161,7 +161,7 @@ describe('PRReviewSubmittedTrigger', () => { triggerCommentUrl: 'https://github.com/owner/repo/pull/42#pullrequestreview-100', }, prNumber: 42, - cardId: 'abc123', + workItemId: 'abc123', }); }); diff --git a/tests/unit/triggers/review-requested.test.ts b/tests/unit/triggers/review-requested.test.ts index 098d4922..ba96b259 100644 --- a/tests/unit/triggers/review-requested.test.ts +++ b/tests/unit/triggers/review-requested.test.ts @@ -186,7 +186,7 @@ describe('ReviewRequestedTrigger', () => { expect(result).not.toBeNull(); expect(result?.agentType).toBe('review'); expect(result?.prNumber).toBe(42); - expect(result?.cardId).toBe('abc123'); + expect(result?.workItemId).toBe('abc123'); expect(result?.agentInput).toMatchObject({ prNumber: 42, repoFullName: 'owner/repo', diff --git a/tests/unit/triggers/trello-comment-mention.test.ts b/tests/unit/triggers/trello-comment-mention.test.ts index 7b9f0101..b8a8fb66 100644 --- a/tests/unit/triggers/trello-comment-mention.test.ts +++ b/tests/unit/triggers/trello-comment-mention.test.ts @@ -145,7 +145,7 @@ describe('TrelloCommentMentionTrigger', () => { expect(result).not.toBeNull(); expect(result?.agentType).toBe('respond-to-planning-comment'); - expect(result?.cardId).toBe('card-1'); + expect(result?.workItemId).toBe('card-1'); expect(result?.agentInput.cardId).toBe('card-1'); expect(result?.agentInput.triggerCommentText).toContain(`@${BOT_USERNAME}`); }); From 0712d75f8a2b8eadfc9bbe01e1235f30c4e27e7a Mon Sep 17 00:00:00 2001 From: Zbigniew Sobiecki Date: Sun, 22 Feb 2026 15:12:53 +0000 Subject: [PATCH 6/6] refactor(triggers): decouple GitHub triggers from PR body work item parsing Replace Trello URL checks in trigger gates with implementer persona authorship checks and DB-backed work item resolution. This fixes JIRA projects where PRs were silently skipped because triggers only matched Trello card URLs. Key changes: - Add pr_work_items table to store PR-to-work-item links at PR creation - Gate check-suite-success/failure on implementer persona (not URL presence) - Add resolveWorkItemId() utility: DB lookup with PR body extraction fallback - All triggers now fire even without a linked work item (graceful degradation) - Thread projectId/cardId through session state to CreatePR gadget for persistence - Add user.login to PRDetails for persona matching Co-Authored-By: Claude Opus 4.6 --- src/agents/base.ts | 6 + src/agents/shared/builderFactory.ts | 6 +- src/agents/shared/githubAgent.ts | 2 + src/db/migrations/0014_pr_work_items.sql | 10 ++ src/db/migrations/meta/_journal.json | 7 + src/db/repositories/prWorkItemsRepository.ts | 40 ++++++ src/db/schema/index.ts | 1 + src/db/schema/prWorkItems.ts | 20 +++ src/gadgets/github/CreatePR.ts | 20 ++- src/gadgets/github/core/createPR.ts | 9 +- src/gadgets/sessionState.ts | 19 ++- src/github/client.ts | 2 + src/triggers/github/check-suite-failure.ts | 31 +++-- src/triggers/github/check-suite-success.ts | 39 ++++-- src/triggers/github/pr-comment-mention.ts | 16 +-- src/triggers/github/pr-merged.ts | 14 +- src/triggers/github/pr-opened.ts | 22 +-- src/triggers/github/pr-ready-to-merge.ts | 14 +- src/triggers/github/pr-review-submitted.ts | 14 +- src/triggers/github/review-requested.ts | 12 +- src/triggers/github/utils.ts | 25 ++++ .../prWorkItemsRepository.test.ts | 125 ++++++++++++++++++ tests/unit/gadgets/github.test.ts | 12 ++ .../unit/gadgets/github/core/createPR.test.ts | 2 + tests/unit/github/client.test.ts | 6 +- .../unit/triggers/check-suite-failure.test.ts | 80 ++++++++++- .../unit/triggers/check-suite-success.test.ts | 116 +++++++++++++++- .../github-pr-comment-mention.test.ts | 12 +- tests/unit/triggers/github-utils.test.ts | 72 +++++++++- tests/unit/triggers/pr-merged.test.ts | 5 + tests/unit/triggers/pr-opened.test.ts | 16 ++- tests/unit/triggers/pr-ready-to-merge.test.ts | 5 + .../unit/triggers/pr-review-submitted.test.ts | 17 ++- tests/unit/triggers/review-requested.test.ts | 17 ++- 34 files changed, 712 insertions(+), 102 deletions(-) create mode 100644 src/db/migrations/0014_pr_work_items.sql create mode 100644 src/db/repositories/prWorkItemsRepository.ts create mode 100644 src/db/schema/prWorkItems.ts create mode 100644 tests/unit/db/repositories/prWorkItemsRepository.test.ts 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 () => {