Fix GitLab label, member and assignee edits through glab (#28776)

src/main/gitlab/issue-update.ts

@@ -62,11 +62,9 @@ export async function updateIssue(
         ...glabHostEnvOptions(projectRef, connectionId)
       })
     } catch (err) {
+      // Why: glab exits 0 for an issue already in the target state, so every failure is real.
       const stderr = err instanceof Error ? err.message : String(err)
-      // Treat "already closed/reopened" as a no-op (matches gh path).
-      if (!stderr.toLowerCase().includes('already')) {
-        errors.push(classifyGlabError(stderr).message)
-      }
+      errors.push(classifyGlabError(stderr).message)
     } finally {
       release()
     }
@@ -111,12 +109,14 @@ export async function updateIssue(
     editArgs.push('--unlabel', label)
     hasEditArgs = true
   }
+  // Why: an unprefixed --assignee replaces every assignee and --unassign clears them all; glab
+  // reads a `+`/`-` prefix as add/remove (since at least v1.22).
   for (const assignee of updates.addAssignees ?? []) {
-    editArgs.push('--assignee', assignee)
+    editArgs.push(`--assignee=+${assignee}`)
     hasEditArgs = true
   }
   for (const assignee of updates.removeAssignees ?? []) {
-    editArgs.push('--unassignee', assignee)
+    editArgs.push(`--assignee=-${assignee}`)
     hasEditArgs = true
   }
 

src/main/gitlab/issues.test.ts

@@ -33,10 +33,52 @@ vi.mock('./gl-utils', async () => {
   }
 })
 
+import { parseGlabApiResponse } from './gl-utils'
 import { addIssueComment, createIssue, getIssue, listIssues } from './issues'
 import { updateIssue } from './issue-update'
 import { listAssignableUsers, listLabels } from './project-label-and-member-lookup'
 
+/** Raw `glab api -i` stdout: status line, CRLF header lines, blank line, then the JSON body. */
+function glabIncludeOutput(headers: Record<string, string>, body: unknown): string {
+  const headerLines = Object.entries(headers)
+    .map(([name, value]) => `${name}: ${value}\r\n`)
+    .join('')
+  return `HTTP/2.0 200 OK\r\nContent-Type: application/json\r\n${headerLines}\r\n${JSON.stringify(body)}\n`
+}
+
+/** Answers each `glab api -i` call by endpoint through the real header/body parser. */
+function serveGlabApiPages(stdoutByEndpoint: Map<string, string>): void {
+  glabApiWithHeadersMock.mockImplementation(async (args: string[]) => {
+    const stdout = stdoutByEndpoint.get(args.at(-1) ?? '')
+    if (stdout === undefined) {
+      throw new Error(`unexpected glab api call: ${args.join(' ')}`)
+    }
+    return parseGlabApiResponse(stdout)
+  })
+}
+
+/** Mirrors glab's cmdutils.ParseAssignees: `+` adds, `-` or `!` removes, anything else replaces all. */
+function glabAssigneeEdits(args: string[]): { add: string[]; remove: string[]; replace: string[] } {
+  const add: string[] = []
+  const remove: string[] = []
+  const replace: string[] = []
+  for (const arg of args) {
+    if (!arg.startsWith('--assignee=')) {
+      continue
+    }
+    for (const value of arg.slice('--assignee='.length).split(',')) {
+      if (value.startsWith('+')) {
+        add.push(value.slice(1))
+      } else if (value.startsWith('-') || value.startsWith('!')) {
+        remove.push(value.slice(1))
+      } else {
+        replace.push(value)
+      }
+    }
+  }
+  return { add, remove, replace }
+}
+
 describe('gitlab issue operations', () => {
   beforeEach(() => {
     glabExecFileAsyncMock.mockReset()
@@ -105,9 +147,13 @@ describe('gitlab issue operations', () => {
           created_at: '2026-06-16T00:00:00.000Z'
         })
       })
-      .mockResolvedValueOnce({ stdout: 'bug\nfrontend\n' })
-      .mockResolvedValueOnce({ stdout: '{"id":1,"username":"octo","avatar_url":""}\n' })
-    glabApiWithHeadersMock.mockResolvedValueOnce({ body: '[]', headers: {} })
+    glabApiWithHeadersMock
+      .mockResolvedValueOnce({ body: '[]', headers: {} })
+      .mockResolvedValueOnce({ body: '[{"id":1,"name":"bug"}]', headers: { 'x-next-page': '' } })
+      .mockResolvedValueOnce({
+        body: '[{"id":1,"username":"octo","avatar_url":""}]',
+        headers: { 'x-next-page': '' }
+      })
 
     await getIssue('/repo-root', 923, null, localGitOptions)
     await listIssues('/repo-root', 5, undefined, 'opened', undefined, null, localGitOptions)
@@ -122,8 +168,12 @@ describe('gitlab issue operations', () => {
       localGitOptions
     )
     await addIssueComment('/repo-root', 923, 'Comment', undefined, null, null, localGitOptions)
-    await listLabels('/repo-root', undefined, null, localGitOptions)
-    await listAssignableUsers('/repo-root', undefined, null, localGitOptions)
+    await expect(listLabels('/repo-root', undefined, null, localGitOptions)).resolves.toEqual([
+      'bug'
+    ])
+    await expect(
+      listAssignableUsers('/repo-root', undefined, null, localGitOptions)
+    ).resolves.toMatchObject([{ username: 'octo' }])
 
     expect(getIssueProjectRefMock).toHaveBeenCalledWith(
       '/repo-root',
@@ -141,6 +191,10 @@ describe('gitlab issue operations', () => {
     expect(glabExecFileAsyncMock.mock.calls.every((call) => call[1]?.wslDistro === 'Ubuntu')).toBe(
       true
     )
+    expect(glabApiWithHeadersMock).toHaveBeenCalledTimes(3)
+    expect(glabApiWithHeadersMock.mock.calls.every((call) => call[1]?.wslDistro === 'Ubuntu')).toBe(
+      true
+    )
     expect(glabApiWithHeadersMock).toHaveBeenCalledWith(
       [
         'projects/stablyai%2Forca/issues?page=1&per_page=5&order_by=updated_at&sort=desc&state=opened'
@@ -371,11 +425,14 @@ describe('gitlab issue operations', () => {
     expect(options.env?.GITLAB_HOST).toBe('gitlab.example.internal')
   })
 
-  it("updateIssue treats 'already closed' as a no-op", async () => {
+  // Why: `glab issue close` exits 0 for a closed issue, so a failure mentioning "already" is real.
+  it('updateIssue reports a failed close even when its text says "already"', async () => {
     getIssueProjectRefMock.mockResolvedValueOnce({ host: 'gitlab.com', path: 'stablyai/orca' })
     glabExecFileAsyncMock.mockRejectedValueOnce(new Error('Issue is already closed'))
 
-    await expect(updateIssue('/repo-root', 5, { state: 'closed' })).resolves.toEqual({ ok: true })
+    await expect(updateIssue('/repo-root', 5, { state: 'closed' })).resolves.toMatchObject({
+      ok: false
+    })
   })
 
   it('updateIssue applies field edits via `glab issue update`', async () => {
@@ -405,15 +462,32 @@ describe('gitlab issue operations', () => {
         'bug',
         '--unlabel',
         'stale',
-        '--assignee',
-        'alice',
-        '--unassignee',
-        'bob'
+        '--assignee=+alice',
+        '--assignee=-bob'
       ],
       { cwd: '/repo-root' }
     )
   })
 
+  it('updateIssue sends assignee edits glab applies as add/remove, never as a replace', async () => {
+    getIssueProjectRefMock.mockResolvedValueOnce({ host: 'gitlab.com', path: 'stablyai/orca' })
+    glabExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' })
+
+    await updateIssue('/repo-root', 5, {
+      addAssignees: ['alice', 'carol'],
+      removeAssignees: ['bob']
+    })
+
+    const [args] = glabExecFileAsyncMock.mock.calls[0]
+    expect(glabAssigneeEdits(args)).toEqual({
+      add: ['alice', 'carol'],
+      remove: ['bob'],
+      replace: []
+    })
+    // Why: glab has no --unassignee, and its --unassign clears every assignee.
+    expect(args.filter((arg: string) => arg.startsWith('--unassign'))).toEqual([])
+  })
+
   it('updateIssue applies body edits via the issue API', async () => {
     getIssueProjectRefMock.mockResolvedValueOnce({ host: 'gitlab.com', path: 'stablyai/orca' })
     glabExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' })
@@ -428,16 +502,40 @@ describe('gitlab issue operations', () => {
     )
   })
 
-  it('routes issue metadata reads through the selected SSH GitLab host', async () => {
+  it('walks label and member pages of glab api -i output on the selected SSH GitLab host', async () => {
     getIssueProjectRefMock
       .mockResolvedValueOnce({ host: 'git.internal', path: 'stablyai/orca' })
       .mockResolvedValueOnce({ host: 'git.internal', path: 'stablyai/orca' })
-    glabExecFileAsyncMock
-      .mockResolvedValueOnce({ stdout: 'bug\nfeature\n' })
-      .mockResolvedValueOnce({
-        stdout:
-          '{"id":12,"username":"alice","name":"Alice","avatar_url":"https://example.com/a.png","state":"active"}\n'
-      })
+    serveGlabApiPages(
+      new Map([
+        [
+          'projects/stablyai%2Forca/labels?per_page=100&page=1',
+          glabIncludeOutput({ 'X-Next-Page': '2', 'X-Page': '1', 'X-Total-Pages': '2' }, [
+            { id: 1, name: 'bug', color: '#d9534f', is_project_label: true }
+          ])
+        ],
+        [
+          'projects/stablyai%2Forca/labels?per_page=100&page=2',
+          glabIncludeOutput({ 'X-Next-Page': '', 'X-Page': '2', 'X-Total-Pages': '2' }, [
+            { id: 4, name: 'feature', color: '#f0ad4e', is_project_label: false }
+          ])
+        ],
+        [
+          'projects/stablyai%2Forca/members/all?per_page=100&page=1',
+          glabIncludeOutput({ 'X-Next-Page': '', 'X-Page': '1' }, [
+            {
+              id: 12,
+              username: 'alice',
+              name: 'Alice',
+              state: 'active',
+              avatar_url: 'https://example.com/a.png',
+              web_url: 'https://git.internal/alice',
+              access_level: 30
+            }
+          ])
+        ]
+      ])
+    )
 
     await expect(listLabels('/repo-root', 'upstream', 'conn-1')).resolves.toEqual([
       'bug',
@@ -453,24 +551,54 @@ describe('gitlab issue operations', () => {
       }
     ])
 
-    expect(glabExecFileAsyncMock.mock.calls[0][0]).toEqual([
-      'api',
-      '--hostname',
-      'git.internal',
-      '--paginate',
-      'projects/stablyai%2Forca/labels',
-      '--jq',
-      '.[].name'
-    ])
-    expect(glabExecFileAsyncMock.mock.calls[1][0]).toEqual([
-      'api',
-      '--hostname',
-      'git.internal',
-      '--paginate',
-      'projects/stablyai%2Forca/members/all?per_page=100',
-      '--jq',
-      '.[] | {id, username, name, avatar_url, state}'
+    expect(glabApiWithHeadersMock.mock.calls.map(([args]) => args)).toEqual([
+      ['--hostname', 'git.internal', 'projects/stablyai%2Forca/labels?per_page=100&page=1'],
+      ['--hostname', 'git.internal', 'projects/stablyai%2Forca/labels?per_page=100&page=2'],
+      ['--hostname', 'git.internal', 'projects/stablyai%2Forca/members/all?per_page=100&page=1']
     ])
+    expect(glabExecFileAsyncMock).not.toHaveBeenCalled()
+  })
+
+  it('probes past a full label page when a proxy strips the pagination headers', async () => {
+    getIssueProjectRefMock.mockResolvedValueOnce({ host: 'gitlab.com', path: 'stablyai/orca' })
+    const fullPage = Array.from({ length: 100 }, (_, index) => ({ id: index, name: `l${index}` }))
+    serveGlabApiPages(
+      new Map([
+        ['projects/stablyai%2Forca/labels?per_page=100&page=1', glabIncludeOutput({}, fullPage)],
+        [
+          'projects/stablyai%2Forca/labels?per_page=100&page=2',
+          glabIncludeOutput({}, [{ id: 100, name: 'last' }])
+        ]
+      ])
+    )
+
+    const labels = await listLabels('/repo-root')
+
+    expect(labels).toHaveLength(101)
+    expect(labels.at(-1)).toBe('last')
+    expect(glabApiWithHeadersMock).toHaveBeenCalledTimes(2)
+  })
+
+  it('stops paging when x-next-page does not advance and returns no members on an error body', async () => {
+    getIssueProjectRefMock
+      .mockResolvedValueOnce({ host: 'gitlab.com', path: 'stablyai/orca' })
+      .mockResolvedValueOnce({ host: 'gitlab.com', path: 'stablyai/orca' })
+    serveGlabApiPages(
+      new Map([
+        [
+          'projects/stablyai%2Forca/labels?per_page=100&page=1',
+          glabIncludeOutput({ 'X-Next-Page': '1' }, [{ id: 1, name: 'bug' }])
+        ],
+        [
+          'projects/stablyai%2Forca/members/all?per_page=100&page=1',
+          glabIncludeOutput({}, { message: '403 Forbidden' })
+        ]
+      ])
+    )
+
+    await expect(listLabels('/repo-root')).resolves.toEqual(['bug'])
+    await expect(listAssignableUsers('/repo-root')).resolves.toEqual([])
+    expect(glabApiWithHeadersMock).toHaveBeenCalledTimes(2)
   })
 
   it('addIssueComment posts to /notes and maps the response', async () => {

src/main/gitlab/project-label-and-member-lookup.ts

@@ -1,17 +1,28 @@
 import type { GitLabAssignableUser } from '../../shared/gitlab-types'
 import type { IssueSourcePreference } from '../../shared/repo-types'
+import { mapGitLabUser, type GitLabRawUser } from './gitlab-assignable-user-mapping'
 import {
   acquire,
   getGlabKnownHosts,
-  glabExecFileAsync,
+  glabApiWithHeaders,
   glabHostnameArgs,
   glabRepoExecOptions,
+  parseGlabJsonList,
+  parseGlabPaginationHeader,
   release,
   resolveIssueSource,
-  type LocalGitExecOptions
+  type LocalGitExecOptions,
+  type ProjectRef
 } from './gl-utils'
 import { encodedProject } from './project-path-encoding'
 
+// GitLab's per_page maximum.
+const PAGE_SIZE = 100
+// Why: bounds the walk (5,000 entries) when a proxy strips x-next-page or it never advances.
+const MAX_PAGES = 50
+
+type GitLabRawLabel = { name?: string | null } | null
+
 export async function listLabels(
   repoPath: string,
   preference?: IssueSourcePreference,
@@ -31,21 +42,14 @@ export async function listLabels(
   }
   await acquire()
   try {
-    const { stdout } = await glabExecFileAsync(
-      [
-        'api',
-        ...glabHostnameArgs(projectRef, connectionId),
-        '--paginate',
-        `projects/${encodedProject(projectRef.path)}/labels`,
-        '--jq',
-        '.[].name'
-      ],
-      glabRepoExecOptions(repoPath, connectionId, localGitOptions)
+    const labels = await fetchAllPages<GitLabRawLabel>(
+      repoPath,
+      projectRef,
+      `projects/${encodedProject(projectRef.path)}/labels`,
+      connectionId,
+      localGitOptions
     )
-    return stdout
-      .trim()
-      .split('\n')
-      .filter((l) => l.length > 0)
+    return labels.flatMap((label) => (label?.name ? [label.name] : []))
   } catch {
     return []
   } finally {
@@ -75,50 +79,56 @@ export async function listAssignableUsers(
     // Why: `members/all` returns project members including those inherited
     // from parent groups — important for projects under a top-level group
     // where assignable users typically come from the group, not the project.
-    // --paginate walks every page; --jq emits NDJSON.
-    const { stdout } = await glabExecFileAsync(
-      [
-        'api',
-        ...glabHostnameArgs(projectRef, connectionId),
-        '--paginate',
-        `projects/${encodedProject(projectRef.path)}/members/all?per_page=100`,
-        '--jq',
-        '.[] | {id, username, name, avatar_url, state}'
-      ],
-      glabRepoExecOptions(repoPath, connectionId, localGitOptions)
+    const members = await fetchAllPages<GitLabRawUser | null>(
+      repoPath,
+      projectRef,
+      `projects/${encodedProject(projectRef.path)}/members/all`,
+      connectionId,
+      localGitOptions
     )
-    type RESTMember = {
-      id?: number
-      username?: string
-      name?: string | null
-      avatar_url?: string | null
-      state?: string | null
-    }
-    const users: GitLabAssignableUser[] = []
-    for (const line of stdout.split('\n')) {
-      const trimmed = line.trim()
-      if (!trimmed) {
-        continue
-      }
-      try {
-        const user = JSON.parse(trimmed) as RESTMember
-        if (user.username) {
-          users.push({
-            ...(typeof user.id === 'number' ? { id: user.id } : {}),
-            username: user.username,
-            name: user.name ?? null,
-            avatarUrl: user.avatar_url ?? '',
-            ...(user.state !== undefined ? { state: user.state } : {})
-          })
-        }
-      } catch {
-        // Skip malformed NDJSON lines defensively.
-      }
-    }
-    return users
+    return members.map(mapGitLabUser).filter((user): user is GitLabAssignableUser => user !== null)
   } catch {
     return []
   } finally {
     release()
   }
 }
+
+// Why: `glab api` has no --jq and older glab lacks `--output ndjson`, so walk the pages here
+// and parse each one as a JSON list.
+async function fetchAllPages<T>(
+  repoPath: string,
+  projectRef: ProjectRef,
+  endpoint: string,
+  connectionId: string | null | undefined,
+  localGitOptions: LocalGitExecOptions
+): Promise<T[]> {
+  const items: T[] = []
+  let page: number | undefined = 1
+  for (let fetched = 0; page !== undefined && fetched < MAX_PAGES; fetched += 1) {
+    const { body, headers } = await glabApiWithHeaders(
+      [
+        ...glabHostnameArgs(projectRef, connectionId),
+        `${endpoint}?per_page=${PAGE_SIZE}&page=${page}`
+      ],
+      glabRepoExecOptions(repoPath, connectionId, localGitOptions)
+    )
+    const pageItems = parseGlabJsonList<T>(body)
+    items.push(...pageItems)
+    page = nextPageNumber(headers['x-next-page'], page, pageItems.length)
+  }
+  return items
+}
+
+function nextPageNumber(
+  nextPageHeader: string | undefined,
+  page: number,
+  pageLength: number
+): number | undefined {
+  // Why: GitLab sends an empty x-next-page on the last page, so an absent one means a proxy
+  // stripped it; a full page then probes one more.
+  if (nextPageHeader === undefined) {
+    return pageLength < PAGE_SIZE ? undefined : page + 1
+  }
+  return parseGlabPaginationHeader(nextPageHeader, page + 1)
+}