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 = [