mirror of
https://github.com/stablyai/orca.git
synced 2026-11-02 18:17:36 +00:00
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>
This commit is contained in:
@@ -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()
|
||||
}
|
||||
|
||||
@@ -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.'
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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 }
|
||||
)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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 }
|
||||
|
||||
Reference in New Issue
Block a user