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 }