fix(gitlab): guard against non-array API responses in MR/issue listing (#12911)

* fix(gitlab): guard against non-array API responses in MR/issue listing

fetchIssuesAsWorkItems and listMergeRequests parsed glab's JSON output
and called .map straight on it. When the GitLab API returns a JSON
object instead of an array (error body, unexpected shape) on a
successful exit, this crashed with a bare TypeError that got
misclassified as "Failed to load issues: JSON.parse(...).map is not
a function" instead of a useful message.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(gitlab): cover listIssues and keep payloads out of error classification

The guard missed listIssues in issues.ts — the RPC-backed issue list that
produces the reported "Failed to load issues: JSON.parse(...).map is not a
function". Hoist the guard into glab-api-response.ts so both files share it.

The thrown message is fed to classifyGlabError, which substring-matches it.
A response payload is content, not a diagnostic: an MR titled "fix network
timeout" classified as network_error and the canned copy replaced the payload
the user needed. Report a GitLab error envelope by its own message, and mark
an opaque body so classification is skipped.

* test(gitlab): make the list-guard tests fail on the regressions they name

Two assertions were vacuous under mutation. The envelope test used a "403
Forbidden" message whose keyword matches earlier in the classifier chain than
its sibling payload, so leaking the payload into classification still passed;
it now uses a 404 envelope beside a "403 forbidden" sibling. No call-site test
carried a classifier keyword, so deleting the marker-error branch entirely
failed only one unit test; the MR API path now uses a keyword-bearing body.

Also give the non-list branch the same "Failed to load issues" prefix as every
other list error, cover the `{ error }` envelope field, and pin the thrown type.

* test(gitlab): pin the reported-payload bound

Removing the 300-char slice survived the whole suite, and the banner's
break-words now depends on it. Name the limit and assert both branches
truncate, plus the envelope falling through a blank message to `error`.

* test(gitlab): pin message-over-error envelope precedence

Swapping the lookup order passed the whole suite. Anchor the bound regex too
so it cannot match an incidental ": " near the end of a message.

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>

src/main/gitlab/client-mr.test.ts

@@ -911,6 +911,57 @@ describe('gitlab client — MR operations', () => {
       expect(result.error?.type).toBe('permission_denied')
       expect(result.items).toEqual([])
     })
+
+    // Why: the title carries a classifier keyword, so this also pins that a wrapped payload stays
+    // out of the substring matcher — classifying it would swap the body for "check your connection".
+    it('reports the body instead of ".map is not a function" when the API returns a non-array', async () => {
+      glabApiWithHeadersMock.mockResolvedValueOnce({
+        body: JSON.stringify({ data: [{ iid: 7, title: 'fix network timeout' }] }),
+        headers: {}
+      })
+      const result = await listMergeRequests('/repo', 'opened')
+      expect(result.items).toEqual([])
+      expect(result.error?.type).toBe('unknown')
+      expect(result.error?.message).toContain('fix network timeout')
+      expect(result.error?.message).not.toContain('is not a function')
+    })
+
+    it('reports the body instead of ".map is not a function" when the cwd fallback returns a non-array', async () => {
+      resolveIssueSourceMock.mockResolvedValueOnce({ source: null, fellBack: false })
+      glabExecFileAsyncMock.mockResolvedValueOnce({
+        stdout: JSON.stringify({ data: [], total: 0 })
+      })
+      const result = await listMergeRequests('/repo', 'opened')
+      expect(result.items).toEqual([])
+      expect(result.error?.message).toContain('{"data":[],"total":0}')
+      expect(result.error?.message).not.toContain('is not a function')
+    })
+
+    // Why: the whole point of surfacing the body — a GitLab error envelope now
+    // classifies like any other glab failure instead of collapsing to 'unknown'.
+    it('classifies a GitLab error envelope returned on exit 0', async () => {
+      glabApiWithHeadersMock.mockResolvedValueOnce({
+        body: JSON.stringify({ message: '403 Forbidden' }),
+        headers: {}
+      })
+      const result = await listMergeRequests('/repo', 'opened')
+      expect(result.items).toEqual([])
+      expect(result.error?.type).toBe('permission_denied')
+    })
+
+    // Why: the sibling title matches an earlier classifier branch than the envelope does, so this
+    // fails if the payload leaks into classification instead of only the envelope's own message.
+    it('classifies an error envelope by its message, not its sibling payload', async () => {
+      glabApiWithHeadersMock.mockResolvedValueOnce({
+        body: JSON.stringify({
+          message: '404 Project Not Found',
+          data: [{ iid: 7, title: '403 forbidden in CI' }]
+        }),
+        headers: {}
+      })
+      const result = await listMergeRequests('/repo', 'opened')
+      expect(result.error?.type).toBe('not_found')
+    })
   })
 
   describe('updateMR', () => {

src/main/gitlab/client-work-items.test.ts

@@ -191,4 +191,16 @@ describe('gitlab client — combined listWorkItems', () => {
     expect(result.items[0].title).toBe('live issue')
     expect(result.error).toBeDefined()
   })
+
+  it('reports the body instead of ".map is not a function" when the issues fetch returns a non-array', async () => {
+    glabApiWithHeadersMock.mockResolvedValueOnce({ body: '[]', headers: {} })
+    glabExecFileAsyncMock.mockResolvedValueOnce({
+      stdout: JSON.stringify({ data: [], total: 0 })
+    })
+
+    const result = await listWorkItems('/repo', 'opened', 1, 20)
+    expect(result.items).toEqual([])
+    expect(result.error?.message).toContain('{"data":[],"total":0}')
+    expect(result.error?.message).not.toContain('is not a function')
+  })
 })

src/main/gitlab/client.ts

@@ -26,7 +26,7 @@ import {
   acquire,
   classifyGlabError,
   classifyJobLogError,
-  classifyListIssuesError,
+  classifyListFetchError,
   isMissingJobLogError,
   getGlabKnownHosts,
   getProjectRef,
@@ -36,6 +36,7 @@ import {
   glabApiWithHeaders,
   glabExecFileAsync,
   parseGlabAuthStatusHosts,
+  parseGlabJsonList,
   release,
   resolveIssueSource,
   type LocalGitExecOptions,
@@ -468,7 +469,7 @@ export async function listMergeRequests(
         ],
         glabRepoExecOptions(repoPath, connectionId, localGitOptions)
       )
-      const data = JSON.parse(stdout) as Parameters<typeof mapMRToWorkItem>[0][]
+      const data = parseGlabJsonList<Parameters<typeof mapMRToWorkItem>[0]>(stdout)
       return {
         items: data.map((d) => mapMRToWorkItem(d, 'unknown')),
         page,
@@ -478,14 +479,13 @@ export async function listMergeRequests(
         totalPages: data.length < perPage ? page : page + 1
       }
     } catch (err) {
-      const stderr = err instanceof Error ? err.message : String(err)
       return {
         items: [],
         page,
         perPage,
         totalCount: 0,
         totalPages: 0,
-        error: classifyListIssuesError(stderr)
+        error: classifyListFetchError(err)
       }
     } finally {
       release()
@@ -505,7 +505,7 @@ export async function listMergeRequests(
       [...glabHostnameArgs(projectRef, connectionId), path],
       glabRepoExecOptions(repoPath, connectionId, localGitOptions)
     )
-    const data = JSON.parse(body) as Parameters<typeof mapMRToWorkItem>[0][]
+    const data = parseGlabJsonList<Parameters<typeof mapMRToWorkItem>[0]>(body)
     return {
       items: data.map((d) => mapMRToWorkItem(d, repoId, projectRef)),
       page,
@@ -517,14 +517,13 @@ export async function listMergeRequests(
         Math.max(1, Math.ceil(parseHeaderInt(headers['x-total'], 0) / perPage))
     }
   } catch (err) {
-    const stderr = err instanceof Error ? err.message : String(err)
     return {
       items: [],
       page,
       perPage,
       totalCount: 0,
       totalPages: 0,
-      error: classifyListIssuesError(stderr)
+      error: classifyListFetchError(err)
     }
   } finally {
     release()
@@ -688,7 +687,7 @@ export async function fetchIssuesAsWorkItems(
       ],
       glabRepoExecOptions(repoPath, connectionId, localGitOptions)
     )
-    const data = JSON.parse(stdout) as Parameters<typeof mapIssueToWorkItem>[0][]
+    const data = parseGlabJsonList<Parameters<typeof mapIssueToWorkItem>[0]>(stdout)
     return {
       items: data.map((d) => mapIssueToWorkItem(d, projectRef.path, projectRef)),
       error: undefined
@@ -696,7 +695,7 @@ export async function fetchIssuesAsWorkItems(
   } catch (err) {
     return {
       items: [],
-      error: classifyListIssuesError(err instanceof Error ? err.message : String(err))
+      error: classifyListFetchError(err)
     }
   } finally {
     release()

src/main/gitlab/gl-utils.test.ts

@@ -17,8 +17,10 @@ import {
   _resetProjectRefCache,
   classifyGlabError,
   classifyJobLogError,
+  classifyListFetchError,
   classifyListIssuesError,
   getIssueProjectRef,
+  parseGlabJsonList,
   isMissingJobLogError,
   getGlabKnownHosts,
   getProjectRef,
@@ -27,6 +29,7 @@ import {
   parseGlabAuthStatusHosts,
   resolveIssueSource
 } from './gl-utils'
+import { GlabNonListResponseError } from './glab-api-response'
 import { rememberGlabKnownHost, rememberGlabKnownHosts } from './gitlab-known-host-probe'
 import { registerSshGitProvider, unregisterSshGitProvider } from '../providers/ssh-git-dispatch'
 import { REMOTE_URL_PROBE_TIMEOUT_MS } from '../git/remote-url-probe'
@@ -501,6 +504,69 @@ gitlab.example.com:8080:
   })
 })
 
+describe('parseGlabJsonList', () => {
+  it('returns the parsed list unchanged', () => {
+    expect(parseGlabJsonList<{ iid: number }>('[{"iid":1}]')).toEqual([{ iid: 1 }])
+  })
+
+  it.each([
+    ['null', 'null'],
+    ['a number', '0'],
+    ['a string', '"nope"'],
+    ['an object', '{"data":[]}']
+  ])('reports the raw payload for %s as an unclassifiable body', (_label, payload) => {
+    expect(() => parseGlabJsonList(payload)).toThrow(GlabNonListResponseError)
+    expect(() => parseGlabJsonList(payload)).toThrow(payload)
+  })
+
+  // Why: glab allows a 10MB body, and the renderer's error banner has no length guard of its own.
+  it.each([
+    ['an opaque body', `{"data":"${'x'.repeat(50_000)}"}`],
+    ['an error envelope', `{"message":"${'x'.repeat(50_000)}"}`]
+  ])('bounds the reported payload for %s', (_label, payload) => {
+    expect(() => parseGlabJsonList(payload)).toThrow(
+      /^GitLab returned (?:a non-list response|an error): .{300}$/
+    )
+  })
+
+  it.each([
+    ['message', '{"message":"403 Forbidden"}', '403 Forbidden'],
+    ['error', '{"error":"insufficient_scope"}', 'insufficient_scope'],
+    ['error when message is blank', '{"message":"  ","error":"real_error"}', 'real_error'],
+    // Why: GitLab sends both on some endpoints; `message` is the human-facing one.
+    [
+      'message when both are set',
+      '{"message":"404 Project Not Found","error":"insufficient_scope"}',
+      '404 Project Not Found'
+    ]
+  ])('reports a GitLab error envelope by its %s field', (_label, payload, reported) => {
+    // Why: an envelope is GitLab's own diagnostic, so it stays classifiable — unlike a raw body.
+    expect(() => parseGlabJsonList(payload)).toThrow(`GitLab returned an error: ${reported}`)
+    expect(() => parseGlabJsonList(payload)).not.toThrow(GlabNonListResponseError)
+  })
+})
+
+describe('classifyListFetchError', () => {
+  it('keeps opaque payload text away from the classifier', () => {
+    // Why: the title would otherwise substring-match as a network failure and replace the payload.
+    const payload = '{"data":[{"title":"fix network timeout"}]}'
+    let thrown: unknown
+    try {
+      parseGlabJsonList(payload)
+    } catch (err) {
+      thrown = err
+    }
+    expect(thrown).toBeInstanceOf(GlabNonListResponseError)
+    const classified = classifyListFetchError(thrown)
+    expect(classified.type).toBe('unknown')
+    expect(classified.message).toContain('fix network timeout')
+  })
+
+  it('still classifies ordinary glab failures by their stderr', () => {
+    expect(classifyListFetchError(new Error('HTTP 403 Forbidden')).type).toBe('permission_denied')
+  })
+})
+
 describe('parseGlabApiResponse', () => {
   it('splits headers and body at the first blank line (LF)', () => {
     const stdout = 'HTTP/2.0 200 OK\nX-Total: 42\nX-Total-Pages: 3\n\n[{"iid":1}]'

src/main/gitlab/gl-utils.ts

@@ -5,6 +5,7 @@ export { glabExecFileAsync, gitExecFileAsync }
 export {
   classifyGlabError,
   classifyJobLogError,
+  classifyListFetchError,
   classifyListIssuesError,
   isMissingJobLogError
 } from './glab-error-classification'
@@ -28,7 +29,7 @@ export type {
   ProjectRef,
   ResolvedIssueSource
 } from './gitlab-project-ref-resolution'
-export { parseGlabApiResponse, type GlabApiResponse } from './glab-api-response'
+export { parseGlabApiResponse, parseGlabJsonList, type GlabApiResponse } from './glab-api-response'
 
 const MAX_CONCURRENT = 4
 let running = 0

src/main/gitlab/glab-api-response.ts

@@ -25,6 +25,48 @@ export function parseGlabApiResponse(stdout: string): GlabApiResponse {
   return { body, headers }
 }
 
+/** A non-list body carrying no GitLab error text — opaque data, so there is nothing to classify. */
+export class GlabNonListResponseError extends Error {}
+
+// Why: glab allows a 10MB body; this is what keeps a proxy's whole response out of the error banner.
+const REPORTED_PAYLOAD_LIMIT = 300
+
+/**
+ * Parse a glab list response, failing readably when GitLab answers with a JSON object.
+ *
+ * Why: glab exits 0 on error envelopes and proxy wrappers, so `JSON.parse(...).map` threw an
+ * opaque `.map is not a function` that the caller's classifier could only report as "unknown".
+ */
+export function parseGlabJsonList<T>(payload: string): T[] {
+  const parsed: unknown = JSON.parse(payload)
+  if (Array.isArray(parsed)) {
+    return parsed as T[]
+  }
+  const reported = gitlabErrorText(parsed)
+  if (reported) {
+    throw new Error(`GitLab returned an error: ${reported}`)
+  }
+  // Why: slice the raw payload rather than re-serializing `parsed` — same text, without
+  // stringifying a multi-megabyte body just to keep the preview.
+  throw new GlabNonListResponseError(
+    `GitLab returned a non-list response: ${payload.trim().slice(0, REPORTED_PAYLOAD_LIMIT)}`
+  )
+}
+
+/** GitLab reports API failures as `{ message }` or `{ error }`; anything else is opaque data. */
+function gitlabErrorText(parsed: unknown): string | null {
+  if (typeof parsed !== 'object' || parsed === null) {
+    return null
+  }
+  const { message, error } = parsed as { message?: unknown; error?: unknown }
+  for (const value of [message, error]) {
+    if (typeof value === 'string' && value.trim()) {
+      return value.trim().slice(0, REPORTED_PAYLOAD_LIMIT)
+    }
+  }
+  return null
+}
+
 function findHeaderBodySeparator(stdout: string): { index: number; bodyStart: number } | null {
   let lineStart = 0
   for (let index = 0; index < stdout.length; index++) {

src/main/gitlab/glab-error-classification.ts

@@ -1,4 +1,5 @@
 import type { ClassifiedError } from '../../shared/types'
+import { GlabNonListResponseError } from './glab-api-response'
 
 // Why: glab CLI surfaces API errors as unstructured stderr. Map known
 // patterns to typed errors so callers can show user-friendly messages.
@@ -33,6 +34,8 @@ export function classifyGlabError(stderr: string): ClassifiedError {
   return { type: 'unknown', message: `Failed to update issue: ${stderr.trim()}` }
 }
 
+const LIST_READ_FAILURE = 'Failed to load issues'
+
 // Why: classifyGlabError's copy is phrased for edit/update operations; list
 // issues is a read op, so rewrite messages for read-context banners.
 export function classifyListIssuesError(stderr: string): ClassifiedError {
@@ -46,11 +49,20 @@ export function classifyListIssuesError(stderr: string): ClassifiedError {
     validation_error: `Invalid request — ${trimmed}`,
     rate_limited: 'GitLab rate limit hit. Try again in a few minutes.',
     network_error: 'Network error — check your connection.',
-    unknown: `Failed to load issues: ${trimmed}`
+    unknown: `${LIST_READ_FAILURE}: ${trimmed}`
   }
   return { type: c.type, message: readMessages[c.type] }
 }
 
+// Why: an opaque response body is content, not a diagnostic — substring-matching it would render
+// an MR titled "fix network timeout" as "check your connection" and discard the body.
+export function classifyListFetchError(err: unknown): ClassifiedError {
+  if (err instanceof GlabNonListResponseError) {
+    return { type: 'unknown', message: `${LIST_READ_FAILURE}: ${err.message}` }
+  }
+  return classifyListIssuesError(err instanceof Error ? err.message : String(err))
+}
+
 // Why: a job trace is a read on a pipeline job, so classifyGlabError's issue-edit
 // copy ("permission to edit this issue") would land verbatim on a Checks row.
 export function classifyJobLogError(stderr: string): ClassifiedError {

src/main/gitlab/issues.test.ts

@@ -186,6 +186,32 @@ describe('gitlab issue operations', () => {
     expect(result.error?.type).toBe('permission_denied')
   })
 
+  it('reports the body instead of ".map is not a function" when the API returns a non-array', async () => {
+    getIssueProjectRefMock.mockResolvedValueOnce({ host: 'gitlab.com', path: 'stablyai/orca' })
+    glabExecFileAsyncMock.mockResolvedValueOnce({
+      stdout: JSON.stringify({ data: [], total: 0 })
+    })
+
+    const result = await listIssues('/repo-root', 5)
+
+    expect(result.items).toEqual([])
+    expect(result.error?.type).toBe('unknown')
+    expect(result.error?.message).toContain('{"data":[],"total":0}')
+    expect(result.error?.message).not.toContain('is not a function')
+  })
+
+  it('reports a GitLab error envelope by its own message', async () => {
+    getIssueProjectRefMock.mockResolvedValueOnce({ host: 'gitlab.com', path: 'stablyai/orca' })
+    glabExecFileAsyncMock.mockResolvedValueOnce({
+      stdout: JSON.stringify({ message: '403 Forbidden' })
+    })
+
+    const result = await listIssues('/repo-root', 5)
+
+    expect(result.items).toEqual([])
+    expect(result.error?.type).toBe('permission_denied')
+  })
+
   it('returns an isolated not_found error (never a cwd-inferred glab call) when the project is unresolved', async () => {
     // Why: a cwd-inferred `glab issue list` would hit `git: exit status 128`
     // on an SSH connection and, in an "All projects" aggregate, sink the

src/main/gitlab/issues.ts

@@ -13,7 +13,7 @@ import type {
 } from '../../shared/types'
 import { mapGitLabIssueInfo } from './mappers'
 // prettier-ignore
-import { glabExecFileAsync, acquire, release, getIssueProjectRef, resolveIssueSource, classifyGlabError, classifyListIssuesError, getGlabKnownHosts, glabRepoExecOptions, glabHostnameArgs, type LocalGitExecOptions, type ProjectRef } from './gl-utils'
+import { glabExecFileAsync, acquire, release, getIssueProjectRef, resolveIssueSource, classifyGlabError, classifyListFetchError, getGlabKnownHosts, glabRepoExecOptions, glabHostnameArgs, parseGlabJsonList, type LocalGitExecOptions, type ProjectRef } from './gl-utils'
 
 // Why: parallel to GitHub's IssueListResult — distinguishes a successful-
 // empty listing from a failed fetch.
@@ -128,7 +128,7 @@ export async function listIssues(
       ],
       glabRepoExecOptions(repoPath, connectionId, localGitOptions)
     )
-    const data = JSON.parse(stdout) as Record<string, unknown>[]
+    const data = parseGlabJsonList<Record<string, unknown>>(stdout)
     // Why: GitLab's project issues endpoint returns true issues only
     // (MRs are a separate endpoint), so no equivalent of GitHub's
     // pull_request filter is needed here.
@@ -136,10 +136,9 @@ export async function listIssues(
       items: data.map((d) => mapGitLabIssueInfo(d as Parameters<typeof mapGitLabIssueInfo>[0]))
     }
   } catch (err) {
-    const stderr = err instanceof Error ? err.message : String(err)
     return {
       items: [],
-      error: classifyListIssuesError(stderr)
+      error: classifyListFetchError(err)
     }
   } finally {
     release()