From ac4463db5746d0029e30d04e4a8b8bc3e78534ac Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Thu, 24 Sep 2026 20:18:27 +0300 Subject: [PATCH] fix(workitems): filter work items by state in the GitLab query browse_work_items list filtered by state after GitLab had paginated the unfiltered list, so closed items newer than an open one pushed it off the page: a page came back empty or short while hasMore was true, and hasMore described the unfiltered list. The namespace, project and group listing queries now pass state (IssuableState: opened/closed, no filter for both) and the client-side filter is gone; an empty state list matches nothing without a request. Closes #610 --- .../src/entities/workitems/registry.ts | 36 ++++++++++---- packages/gitlab-mcp/src/graphql/workItems.ts | 25 ++++++++-- .../tests/integration/workitems.test.ts | 47 +++++++++++++++++++ .../unit/entities/workitems/registry.test.ts | 17 +++---- .../workitems/schema-fallbacks.test.ts | 45 ++++++++++++++++++ 5 files changed, 147 insertions(+), 23 deletions(-) diff --git a/packages/gitlab-mcp/src/entities/workitems/registry.ts b/packages/gitlab-mcp/src/entities/workitems/registry.ts index db6442100..f65de5059 100644 --- a/packages/gitlab-mcp/src/entities/workitems/registry.ts +++ b/packages/gitlab-mcp/src/entities/workitems/registry.ts @@ -90,7 +90,13 @@ function assertUpdatableWidgets(input: object, verb: 'set' | 'update'): void { */ async function listWorkItems( client: GraphQLClient, - vars: { namespacePath: string; types?: string[]; first: number; after?: string }, + vars: { + namespacePath: string; + types?: string[]; + state?: 'opened' | 'closed'; + first: number; + after?: string; + }, ) { if (graphqlSupports('Namespace', 'workItems')) { return (await client.request(GET_NAMESPACE_WORK_ITEMS, vars)).namespace?.workItems ?? null; @@ -400,23 +406,33 @@ export const workitemsToolRegistry: ToolRegistry = new Map { - return state.includes(item.state); - }); // Apply simplification if requested and clean GIDs - const finalResults = filteredItems.map((item: GraphQLWorkItem) => { + const finalResults = (workItemsData?.nodes ?? []).map((item: GraphQLWorkItem) => { const cleanedItem = cleanWorkItemResponse(item as unknown as GitLabWorkItem); return simplifyWorkItem(cleanedItem as GraphQLWorkItem, simple); }); diff --git a/packages/gitlab-mcp/src/graphql/workItems.ts b/packages/gitlab-mcp/src/graphql/workItems.ts index a325c7a13..1d0314cb2 100644 --- a/packages/gitlab-mcp/src/graphql/workItems.ts +++ b/packages/gitlab-mcp/src/graphql/workItems.ts @@ -553,7 +553,14 @@ interface WorkItemListConnection { }; } -type WorkItemListVars = { namespacePath: string; types?: string[]; first?: number; after?: string }; +type WorkItemListVars = { + namespacePath: string; + types?: string[]; + /** IssuableState; omitted for every state. */ + state?: 'opened' | 'closed'; + first?: number; + after?: string; +}; // Listing selection shared by the namespace query and its project/group fallbacks. const WORK_ITEM_LIST_CONNECTION = ` @@ -659,13 +666,14 @@ export const GET_NAMESPACE_WORK_ITEMS: TypedDocumentNode< query GetNamespaceWorkItems( $namespacePath: ID! $types: [IssueType!] + $state: IssuableState $first: Int $after: String ) { namespace(fullPath: $namespacePath) { __typename fullPath - workItems(types: $types, first: $first, after: $after) { + workItems(types: $types, state: $state, first: $first, after: $after) { ${WORK_ITEM_LIST_CONNECTION} } } @@ -680,11 +688,12 @@ export const LIST_PROJECT_WORK_ITEMS: TypedDocumentNode< query ListProjectWorkItems( $namespacePath: ID! $types: [IssueType!] + $state: IssuableState $first: Int $after: String ) { project(fullPath: $namespacePath) { - workItems(types: $types, first: $first, after: $after) { + workItems(types: $types, state: $state, first: $first, after: $after) { ${WORK_ITEM_LIST_CONNECTION} } } @@ -695,9 +704,15 @@ export const LIST_GROUP_WORK_ITEMS: TypedDocumentNode< { group: { workItems: WorkItemListConnection | null } | null }, WorkItemListVars > = gql` - query ListGroupWorkItems($namespacePath: ID!, $types: [IssueType!], $first: Int, $after: String) { + query ListGroupWorkItems( + $namespacePath: ID! + $types: [IssueType!] + $state: IssuableState + $first: Int + $after: String + ) { group(fullPath: $namespacePath) { - workItems(types: $types, first: $first, after: $after) { + workItems(types: $types, state: $state, first: $first, after: $after) { ${WORK_ITEM_LIST_CONNECTION} } } diff --git a/packages/gitlab-mcp/tests/integration/workitems.test.ts b/packages/gitlab-mcp/tests/integration/workitems.test.ts index ef992d3e7..f68270d1e 100644 --- a/packages/gitlab-mcp/tests/integration/workitems.test.ts +++ b/packages/gitlab-mcp/tests/integration/workitems.test.ts @@ -182,6 +182,53 @@ describe('Work Items Integration - Using Handler Functions', () => { }, 30000); }); + describe('Work item state filter', () => { + // The state filter must reach GitLab: filtered after pagination, closed items + // newer than an open one push it off the page and leave the page empty. + it('returns the open item on the first page even when newer items are closed', async () => { + const namespace = requireTestData().project.path_with_namespace; + const created: string[] = []; + const create = async (title: string) => { + const item = (await helper.createWorkItem({ + namespace, + title, + workItemType: 'ISSUE', + })) as { id: string }; + created.push(item.id); + return item.id; + }; + + try { + const suffix = Date.now(); + const openId = await create(`State filter open ${suffix}`); + for (const title of [ + `State filter closed B ${suffix}`, + `State filter closed C ${suffix}`, + ]) { + await helper.updateWorkItem({ id: await create(title), state: 'CLOSE' }); + } + + const open = (await helper.listWorkItems({ namespace, state: ['OPEN'], first: 2 })) as { + items: Array<{ id: string; state: string }>; + }; + expect(open.items[0]?.id).toBe(openId); + expect(open.items.every((item) => item.state === 'OPEN')).toBe(true); + + const closed = (await helper.listWorkItems({ namespace, state: ['CLOSED'], first: 2 })) as { + items: Array<{ id: string; state: string }>; + }; + expect(closed.items).toHaveLength(2); + expect(closed.items.map((item) => item.id)).toEqual(created.slice(1).reverse()); + } finally { + for (const id of created) { + await helper.deleteWorkItem({ id }).catch((error: unknown) => { + console.warn(`Could not delete state filter fixture ${id}:`, error); + }); + } + } + }, 60000); + }); + describe('Work Items Widget Validation through Handlers', () => { it('should validate core widget types through list_work_items handler', async () => { const testData = requireTestData(); diff --git a/packages/gitlab-mcp/tests/unit/entities/workitems/registry.test.ts b/packages/gitlab-mcp/tests/unit/entities/workitems/registry.test.ts index 9d9666d3c..7850ec8ae 100644 --- a/packages/gitlab-mcp/tests/unit/entities/workitems/registry.test.ts +++ b/packages/gitlab-mcp/tests/unit/entities/workitems/registry.test.ts @@ -306,9 +306,11 @@ describe('Workitems Registry - CQRS Tools', () => { const tool = workitemsToolRegistry.get('browse_work_items'); const result = await tool?.handler({ action: 'list', namespace: 'test-group' }); + // The default state (OPEN) is filtered by GitLab. expect(mockClient.request).toHaveBeenCalledWith(expect.any(Object), { namespacePath: 'test-group', types: undefined, + state: 'opened', first: 20, after: undefined, }); @@ -379,6 +381,7 @@ describe('Workitems Registry - CQRS Tools', () => { expect(mockClient.request).toHaveBeenCalledWith(expect.any(Object), { namespacePath: 'test-group', types: undefined, + state: 'opened', first: 50, after: 'cursor-123', }); @@ -426,6 +429,7 @@ describe('Workitems Registry - CQRS Tools', () => { expect(mockClient.request).toHaveBeenCalledWith(expect.any(Object), { namespacePath: 'test-group', types: ['EPIC', 'ISSUE'], + state: 'opened', first: 20, after: undefined, }); @@ -916,17 +920,14 @@ describe('Workitems Registry - CQRS Tools', () => { expect(widgets.some((widget) => widget.type === 'TEST_REPORTS')).toBe(false); }); - it('should filter by state parameter', async () => { - const mockWorkItems = [ - createMockWorkItem({ id: 'gid://gitlab/WorkItem/1', state: 'OPEN' }), - createMockWorkItem({ id: 'gid://gitlab/WorkItem/2', state: 'CLOSED' }), - ]; - + it('should filter by state parameter in the GitLab query', async () => { + // GitLab applies the state filter before paginating, so the page it + // returns is passed through as is. mockClient.request.mockResolvedValueOnce({ namespace: { __typename: 'Group', workItems: { - nodes: mockWorkItems, + nodes: [createMockWorkItem({ id: 'gid://gitlab/WorkItem/1', state: 'OPEN' })], pageInfo: { hasNextPage: false, endCursor: null }, }, }, @@ -939,7 +940,7 @@ describe('Workitems Registry - CQRS Tools', () => { state: ['OPEN'], })) as { items: unknown[] }; - // Client-side filtering should only return OPEN items + expect(mockClient.request.mock.calls[0][1].state).toBe('opened'); expect(result.items.length).toBe(1); }); }); diff --git a/packages/gitlab-mcp/tests/unit/entities/workitems/schema-fallbacks.test.ts b/packages/gitlab-mcp/tests/unit/entities/workitems/schema-fallbacks.test.ts index f32bf35f5..81470ddc9 100644 --- a/packages/gitlab-mcp/tests/unit/entities/workitems/schema-fallbacks.test.ts +++ b/packages/gitlab-mcp/tests/unit/entities/workitems/schema-fallbacks.test.ts @@ -115,6 +115,51 @@ describe('browse_work_items list', () => { }); }); +describe('browse_work_items list state filter', () => { + // Filtering after GitLab paginated would leave pages empty while hasMore is + // true; the filter has to be part of the query. + it.each([ + [['OPEN'], 'opened'], + [['CLOSED'], 'closed'], + [['OPEN', 'CLOSED'], undefined], + ])('sends state %j to GitLab as %s', async (state, expected) => { + mockRequest.mockResolvedValueOnce({ namespace: { workItems: connection('1') } }); + await browse().handler({ action: 'list', namespace: 'grp/proj', state }); + expect(mockRequest.mock.calls[0][1].state).toBe(expected); + }); + + it('passes the state to the project fallback too', async () => { + missing.add('Namespace.workItems'); + mockRequest.mockResolvedValueOnce({ project: { workItems: connection('7') } }); + await browse().handler({ action: 'list', namespace: 'grp/proj', state: ['CLOSED'] }); + expect(mockRequest.mock.calls[0][1].state).toBe('closed'); + }); + + it('returns the page as GitLab filtered it, with its own hasMore', async () => { + mockRequest.mockResolvedValueOnce({ + namespace: { + workItems: { + nodes: [item('3')], + pageInfo: { hasNextPage: false, endCursor: 'c' }, + }, + }, + }); + const result = (await browse().handler({ + action: 'list', + namespace: 'grp/proj', + state: ['OPEN'], + })) as { items: Array<{ iid: string }>; hasMore: boolean }; + expect(result.items.map((i) => i.iid)).toEqual(['3']); + expect(result.hasMore).toBe(false); + }); + + it('matches nothing for an empty state list without calling GitLab', async () => { + const result = await browse().handler({ action: 'list', namespace: 'grp/proj', state: [] }); + expect(result).toEqual({ items: [], hasMore: false, endCursor: null }); + expect(mockRequest).not.toHaveBeenCalled(); + }); +}); + describe('browse_work_items get by IID', () => { it('falls back to the project listing filtered by IID', async () => { missing.add('Namespace.workItem');