From 7bcaee270d53e111bb0dbd8caf216520568a41d3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=BDan=20Mlakar?=
 <148876659+zanmlakar@users.noreply.github.com>
Date: Wed, 28 Oct 2026 01:42:24 +0100
Subject: [PATCH] fix(integrations): stop treating any "already" failure as a
 successful issue/MR close or reopen (#28777)

GitHub issue close/reopen swallowed every gh failure mentioning "already"
(e.g. "API rate limit already exceeded"), and GitLab MR close/reopen did the
same for glab. gh exits 0 when the issue is already in the target state, so
every gh failure is now reported. For glab, only its fixed no-op refusals
("this merge request has been closed" / "this merge request is already open")
count as success; "has already been merged" and all other errors surface.

Fixes #28766

Co-authored-by: Neil <neil@stably.ai>
---
 src/main/github/issue-update.ts                |  6 ++----
 src/main/github/issues.test.ts                 | 12 ++++++++++++
 .../gitlab/client-mr-review-actions.test.ts    | 18 ++++++++++++++++++
 .../gitlab/merge-request-state-mutations.ts    | 10 +++++++---
 4 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/src/main/github/issue-update.ts b/src/main/github/issue-update.ts
index 0f059da5c12..718398c57af 100644
--- a/src/main/github/issue-update.ts
+++ b/src/main/github/issue-update.ts
@@ -56,11 +56,9 @@ export async function updateIssue(
         await ghExecFileAsync(['issue', 'reopen', String(issueNumber), '--repo', repo], ghOptions)
       }
     } catch (err) {
+      // Why: gh exits 0 when the issue is already in the target state, so any failure is real.
       const stderr = err instanceof Error ? err.message : String(err)
-      // Repeated state transitions are idempotent.
-      if (!stderr.toLowerCase().includes('already')) {
-        errors.push(classifyGhError(stderr).message)
-      }
+      errors.push(classifyGhError(stderr).message)
     } finally {
       release()
     }
diff --git a/src/main/github/issues.test.ts b/src/main/github/issues.test.ts
index 29419bb9807..f9cb9fca6d1 100644
--- a/src/main/github/issues.test.ts
+++ b/src/main/github/issues.test.ts
@@ -537,4 +537,16 @@ describe('issue source operations', () => {
       { cwd: '/repo-root', host: 'github.com' }
     )
   })
+
+  it('reports a failed close whose gh error mentions "already"', async () => {
+    getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
+    ghExecFileAsyncMock.mockRejectedValueOnce(
+      new Error('GraphQL: API rate limit already exceeded for user ID 1.')
+    )
+
+    await expect(updateIssue('/repo-root', 924, { state: 'closed' })).resolves.toEqual({
+      ok: false,
+      error: 'GitHub rate limit hit. Try again in a few minutes.'
+    })
+  })
 })
diff --git a/src/main/gitlab/client-mr-review-actions.test.ts b/src/main/gitlab/client-mr-review-actions.test.ts
index 29b4210aba2..311306b114c 100644
--- a/src/main/gitlab/client-mr-review-actions.test.ts
+++ b/src/main/gitlab/client-mr-review-actions.test.ts
@@ -515,4 +515,22 @@ describe('gitlab client — MR operations', () => {
       expect(options.env).toBeUndefined()
     })
   })
+
+  describe('glab state-change refusals', () => {
+    const projectRef = { host: 'gitlab.com', path: 'g/p' }
+    const mutations = { close: closeMR, reopen: reopenMR }
+
+    it.each([
+      ['close', 'this merge request has been closed', true],
+      ['reopen', 'this merge request is already open.', true],
+      ['close', 'this merge request has already been merged', false],
+      ['reopen', 'this merge request has already been merged', false]
+    ] as const)('%s refused with "%s" reports ok=%s', async (action, message, ok) => {
+      glabExecFileAsyncMock.mockRejectedValueOnce(new Error(message))
+
+      await expect(mutations[action]('/repo', 6, undefined, null, projectRef)).resolves.toEqual(
+        ok ? { ok: true } : { ok: false, error: message }
+      )
+    })
+  })
 })
diff --git a/src/main/gitlab/merge-request-state-mutations.ts b/src/main/gitlab/merge-request-state-mutations.ts
index 2260874bbfe..6d02f2e0755 100644
--- a/src/main/gitlab/merge-request-state-mutations.ts
+++ b/src/main/gitlab/merge-request-state-mutations.ts
@@ -10,6 +10,11 @@ import {
 } from './gl-utils'
 import { withProjectRef } from './merge-request-project-resolution'
 
+// Why: glab's MRCheckErrors refuses a no-op close/reopen with these fixed texts; every other
+// failure, including "has already been merged", must surface.
+const GLAB_MR_ALREADY_CLOSED = 'this merge request has been closed'
+const GLAB_MR_ALREADY_OPEN = 'this merge request is already open'
+
 export async function closeMR(
   repoPath: string,
   iid: number,
@@ -33,8 +38,7 @@ export async function closeMR(
         return { ok: true }
       } catch (err) {
         const msg = err instanceof Error ? err.message : String(err)
-        // glab signals already-closed with nonzero exit; treat the repeated transition as success.
-        if (msg.toLowerCase().includes('already')) {
+        if (msg.toLowerCase().includes(GLAB_MR_ALREADY_CLOSED)) {
           return { ok: true }
         }
         return { ok: false, error: msg }
@@ -70,7 +74,7 @@ export async function reopenMR(
         return { ok: true }
       } catch (err) {
         const msg = err instanceof Error ? err.message : String(err)
-        if (msg.toLowerCase().includes('already')) {
+        if (msg.toLowerCase().includes(GLAB_MR_ALREADY_OPEN)) {
           return { ok: true }
         }
         return { ok: false, error: msg }