fix(github): recover oversized issue creation (#8217)
* fix(github): recover oversized issue creation (#7704) * fix(github): preserve partial issue creation --------- Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com>
This commit is contained in:
parent
66cad529a2
commit
b677b2a209
|
|
@ -239,6 +239,163 @@ describe('issue source operations', () => {
|
|||
)
|
||||
})
|
||||
|
||||
it('recovers issue 7704 oversized inline-image creation', async () => {
|
||||
const imagePrefix = 'data:image/png;base64,'
|
||||
const body = imagePrefix + 'x'.repeat(133596 - imagePrefix.length)
|
||||
expect(body).toContain('data:image')
|
||||
expect(body).toHaveLength(133596)
|
||||
|
||||
getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
|
||||
ghExecFileAsyncMock
|
||||
.mockRejectedValueOnce(new Error('HTTP 422: body is too long (maximum is 65536 characters)'))
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify({
|
||||
number: 926,
|
||||
html_url: 'https://github.com/stablyai/orca/issues/926'
|
||||
})
|
||||
})
|
||||
.mockResolvedValueOnce({ stdout: '' })
|
||||
|
||||
await expect(
|
||||
createIssue('/repo-root', 'Image issue', body, undefined, undefined, {
|
||||
labels: ['bug'],
|
||||
assignees: ['octo']
|
||||
})
|
||||
).resolves.toEqual({
|
||||
ok: true,
|
||||
number: 926,
|
||||
url: 'https://github.com/stablyai/orca/issues/926'
|
||||
})
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
expect.arrayContaining([
|
||||
`body=${body}`,
|
||||
'title=Image issue',
|
||||
'labels[]=bug',
|
||||
'assignees[]=octo'
|
||||
]),
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.arrayContaining(['body=', 'title=Image issue', 'labels[]=bug', 'assignees[]=octo']),
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
3,
|
||||
['api', '-X', 'PATCH', 'repos/stablyai/orca/issues/926', '--raw-field', `body=${body}`],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
})
|
||||
|
||||
it('recognizes the oversized-body response from structured gh stderr', async () => {
|
||||
getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
|
||||
ghExecFileAsyncMock
|
||||
.mockRejectedValueOnce(
|
||||
Object.assign(new Error('Command failed: gh'), {
|
||||
stderr: 'gh: body is too long (maximum is 65536 characters) (HTTP 422)'
|
||||
})
|
||||
)
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify({
|
||||
number: 929,
|
||||
html_url: 'https://github.com/stablyai/orca/issues/929'
|
||||
})
|
||||
})
|
||||
.mockResolvedValueOnce({ stdout: '' })
|
||||
|
||||
await expect(createIssue('/repo-root', 'Image issue', 'data:image')).resolves.toEqual({
|
||||
ok: true,
|
||||
number: 929,
|
||||
url: 'https://github.com/stablyai/orca/issues/929'
|
||||
})
|
||||
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(3)
|
||||
})
|
||||
|
||||
it('does not retry unrelated create failures', async () => {
|
||||
getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
|
||||
ghExecFileAsyncMock.mockRejectedValueOnce(new Error('HTTP 422: assignees is invalid'))
|
||||
|
||||
await expect(createIssue('/repo-root', 'Invalid issue', 'Body')).resolves.toEqual({
|
||||
ok: false,
|
||||
error: 'HTTP 422: assignees is invalid'
|
||||
})
|
||||
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('stops when placeholder create fails during oversized-body recovery', async () => {
|
||||
const body = `data:image/png;base64,${'x'.repeat(100)}`
|
||||
getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
|
||||
ghExecFileAsyncMock
|
||||
.mockRejectedValueOnce(new Error('body is too long (maximum is 65536 characters)'))
|
||||
.mockRejectedValueOnce(new Error('HTTP 500: create failed'))
|
||||
|
||||
await expect(createIssue('/repo-root', 'Placeholder failure', body)).resolves.toEqual({
|
||||
ok: false,
|
||||
error: 'HTTP 500: create failed'
|
||||
})
|
||||
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('preserves fields during oversized-body recovery', async () => {
|
||||
const localGitOptions = { wslDistro: 'Ubuntu' }
|
||||
const body = `data:image/png;base64,${'x'.repeat(100)}`
|
||||
getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
|
||||
ghExecFileAsyncMock
|
||||
.mockRejectedValueOnce(new Error('body is too long (maximum is 65536 characters)'))
|
||||
.mockResolvedValueOnce({ stdout: JSON.stringify({ number: 927, url: 'issue-url' }) })
|
||||
.mockResolvedValueOnce({ stdout: '' })
|
||||
|
||||
await expect(
|
||||
createIssue(
|
||||
'/repo-root',
|
||||
'Fields issue',
|
||||
body,
|
||||
undefined,
|
||||
null,
|
||||
{
|
||||
labels: ['bug'],
|
||||
assignees: ['octo']
|
||||
},
|
||||
localGitOptions
|
||||
)
|
||||
).resolves.toEqual({ ok: true, number: 927, url: 'issue-url' })
|
||||
const firstCreateArgs = [...ghExecFileAsyncMock.mock.calls[0][0]]
|
||||
const fallbackCreateArgs = [...ghExecFileAsyncMock.mock.calls[1][0]]
|
||||
firstCreateArgs[firstCreateArgs.indexOf(`body=${body}`)] = 'body='
|
||||
expect(fallbackCreateArgs).toEqual(firstCreateArgs)
|
||||
expect(ghExecFileAsyncMock.mock.calls.every((call) => call[1]?.wslDistro === 'Ubuntu')).toBe(
|
||||
true
|
||||
)
|
||||
expect(resolveIssueSourceMock).toHaveBeenCalledWith(
|
||||
'/repo-root',
|
||||
undefined,
|
||||
null,
|
||||
localGitOptions
|
||||
)
|
||||
})
|
||||
|
||||
it('reports partial success when oversized-body patch fails', async () => {
|
||||
getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
|
||||
ghExecFileAsyncMock
|
||||
.mockRejectedValueOnce(new Error('body is too long (maximum is 65536 characters)'))
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify({
|
||||
number: 928,
|
||||
html_url: 'https://github.com/stablyai/orca/issues/928'
|
||||
})
|
||||
})
|
||||
.mockRejectedValueOnce(new Error('HTTP 500: update failed'))
|
||||
|
||||
await expect(createIssue('/repo-root', 'Partial issue', 'data:image')).resolves.toEqual({
|
||||
ok: true,
|
||||
number: 928,
|
||||
url: 'https://github.com/stablyai/orca/issues/928',
|
||||
bodySaveWarning:
|
||||
'Issue https://github.com/stablyai/orca/issues/928 was created, but saving its body failed: HTTP 500: update failed'
|
||||
})
|
||||
})
|
||||
|
||||
it('updates issue body through the REST issue endpoint', async () => {
|
||||
getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
|
||||
ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' })
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import type {
|
|||
ClassifiedError,
|
||||
GitHubAssignableUser,
|
||||
GitHubCreateIssueFields,
|
||||
GitHubCreateIssueResult,
|
||||
GitHubCommentResult,
|
||||
GitHubIssueUpdate,
|
||||
IssueInfo,
|
||||
|
|
@ -15,7 +16,7 @@ import type {
|
|||
import { mapIssueInfo } from './mappers'
|
||||
import type { LocalGitExecOptions, OwnerRepo } from './gh-utils'
|
||||
// prettier-ignore
|
||||
import { ghExecFileAsync, acquire, release, getIssueOwnerRepo, resolveIssueSource, classifyGhError, classifyListIssuesError, ghRepoExecOptions, githubRepoContext } from './gh-utils'
|
||||
import { ghExecFileAsync, acquire, release, getIssueOwnerRepo, resolveIssueSource, classifyGhError, classifyListIssuesError, ghRepoExecOptions, githubRepoContext, extractExecError } from './gh-utils'
|
||||
|
||||
// Why: distinguishes a successful-empty listing from a failed fetch. The
|
||||
// previous `catch { return [] }` conflated a 403 on a private upstream with an
|
||||
|
|
@ -31,6 +32,11 @@ export type IssueListResult = {
|
|||
error?: ClassifiedError
|
||||
}
|
||||
|
||||
function githubIssueErrorMessage(error: unknown): string {
|
||||
const { stderr, stdout } = extractExecError(error)
|
||||
return stderr.trim() || stdout.trim()
|
||||
}
|
||||
|
||||
/**
|
||||
* Get a single issue by number.
|
||||
* Uses gh api --cache so 304 Not Modified responses don't count against the rate limit.
|
||||
|
|
@ -162,7 +168,7 @@ export async function createIssue(
|
|||
connectionId?: string | null,
|
||||
fields?: GitHubCreateIssueFields,
|
||||
localGitOptions: LocalGitExecOptions = {}
|
||||
): Promise<{ ok: true; number: number; url: string } | { ok: false; error: string }> {
|
||||
): Promise<GitHubCreateIssueResult> {
|
||||
const trimmedTitle = title.trim()
|
||||
if (!trimmedTitle) {
|
||||
return { ok: false, error: 'Title is required' }
|
||||
|
|
@ -180,25 +186,71 @@ export async function createIssue(
|
|||
}
|
||||
await acquire()
|
||||
try {
|
||||
const args = [
|
||||
'api',
|
||||
'-X',
|
||||
'POST',
|
||||
`repos/${ownerRepo.owner}/${ownerRepo.repo}/issues`,
|
||||
'--raw-field',
|
||||
`title=${trimmedTitle}`,
|
||||
'--raw-field',
|
||||
`body=${body}`
|
||||
]
|
||||
for (const label of fields?.labels ?? []) {
|
||||
args.push('--raw-field', `labels[]=${label}`)
|
||||
}
|
||||
for (const assignee of fields?.assignees ?? []) {
|
||||
args.push('--raw-field', `assignees[]=${assignee}`)
|
||||
const createArgs = (issueBody: string) => {
|
||||
const args = [
|
||||
'api',
|
||||
'-X',
|
||||
'POST',
|
||||
`repos/${ownerRepo.owner}/${ownerRepo.repo}/issues`,
|
||||
'--raw-field',
|
||||
`title=${trimmedTitle}`,
|
||||
'--raw-field',
|
||||
`body=${issueBody}`
|
||||
]
|
||||
for (const label of fields?.labels ?? []) {
|
||||
args.push('--raw-field', `labels[]=${label}`)
|
||||
}
|
||||
for (const assignee of fields?.assignees ?? []) {
|
||||
args.push('--raw-field', `assignees[]=${assignee}`)
|
||||
}
|
||||
return args
|
||||
}
|
||||
|
||||
const parseIssue = (stdout: string) =>
|
||||
JSON.parse(stdout) as { number?: number; html_url?: string; url?: string }
|
||||
|
||||
let data: { number?: number; html_url?: string; url?: string }
|
||||
try {
|
||||
const { stdout } = await ghExecFileAsync(createArgs(body), ghOptions)
|
||||
data = parseIssue(stdout)
|
||||
} catch (err) {
|
||||
const message = githubIssueErrorMessage(err)
|
||||
if (!/body is too long \(maximum is \d+ characters\)/i.test(message)) {
|
||||
return { ok: false, error: message }
|
||||
}
|
||||
|
||||
// Why: GitHub rejects oversized bodies on create but accepts the same body
|
||||
// on update, so establish the issue before attaching its body.
|
||||
const { stdout } = await ghExecFileAsync(createArgs(''), ghOptions)
|
||||
data = parseIssue(stdout)
|
||||
if (typeof data.number !== 'number') {
|
||||
return { ok: false, error: 'Unexpected response from GitHub' }
|
||||
}
|
||||
|
||||
try {
|
||||
await ghExecFileAsync(
|
||||
[
|
||||
'api',
|
||||
'-X',
|
||||
'PATCH',
|
||||
`repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${data.number}`,
|
||||
'--raw-field',
|
||||
`body=${body}`
|
||||
],
|
||||
ghOptions
|
||||
)
|
||||
} catch (patchErr) {
|
||||
const patchMessage = githubIssueErrorMessage(patchErr)
|
||||
const identity = data.html_url ?? data.url ?? `#${data.number}`
|
||||
return {
|
||||
ok: true,
|
||||
number: data.number,
|
||||
url: String(data.html_url ?? data.url ?? ''),
|
||||
bodySaveWarning: `Issue ${identity} was created, but saving its body failed: ${patchMessage}`
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const { stdout } = await ghExecFileAsync(args, ghOptions)
|
||||
const data = JSON.parse(stdout) as { number?: number; html_url?: string; url?: string }
|
||||
if (typeof data.number !== 'number') {
|
||||
return { ok: false, error: 'Unexpected response from GitHub' }
|
||||
}
|
||||
|
|
@ -208,8 +260,7 @@ export async function createIssue(
|
|||
url: String(data.html_url ?? data.url ?? '')
|
||||
}
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err)
|
||||
return { ok: false, error: message }
|
||||
return { ok: false, error: githubIssueErrorMessage(err) }
|
||||
} finally {
|
||||
release()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -89,6 +89,7 @@ import type {
|
|||
GitStatusResult,
|
||||
GitUpstreamStatus,
|
||||
GitHubAssignableUser,
|
||||
GitHubCreateIssueResult,
|
||||
GitHubPRFile,
|
||||
GitHubPRFileContents,
|
||||
GitHubPrStartPoint,
|
||||
|
|
@ -1488,7 +1489,7 @@ export type PreloadApi = {
|
|||
body: string
|
||||
labels?: string[]
|
||||
assignees?: string[]
|
||||
}) => Promise<{ ok: true; number: number; url: string } | { ok: false; error: string }>
|
||||
}) => Promise<GitHubCreateIssueResult>
|
||||
countWorkItems: (args: { repoPath: string; repoId?: string; query?: string }) => Promise<number>
|
||||
listWorkItems: (args: {
|
||||
repoPath: string
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ import type {
|
|||
GitHubPRRefreshReason,
|
||||
GitHubAssignableUser,
|
||||
GitHubCommentResult,
|
||||
GitHubCreateIssueResult,
|
||||
GitHubWorkItem,
|
||||
JiraProjectStatusOrder,
|
||||
GitPushTarget,
|
||||
|
|
@ -1262,8 +1263,7 @@ const api = {
|
|||
body: string
|
||||
labels?: string[]
|
||||
assignees?: string[]
|
||||
}): Promise<{ ok: true; number: number; url: string } | { ok: false; error: string }> =>
|
||||
ipcRenderer.invoke('gh:createIssue', args),
|
||||
}): Promise<GitHubCreateIssueResult> => ipcRenderer.invoke('gh:createIssue', args),
|
||||
|
||||
countWorkItems: (args: {
|
||||
repoPath: string
|
||||
|
|
|
|||
|
|
@ -6865,7 +6865,9 @@ export default function TaskPage(): React.JSX.Element {
|
|||
labels: newIssueLabels,
|
||||
assignees: newIssueAssignees.map((assignee) => assignee.login)
|
||||
},
|
||||
{ timeoutMs: 30_000 }
|
||||
// Why: oversized-body recovery can require two 30-second writes
|
||||
// after GitHub rejects the initial create request.
|
||||
{ timeoutMs: 65_000 }
|
||||
)
|
||||
: await window.api.gh.createIssue({
|
||||
repoPath: newIssueTargetRepo.path,
|
||||
|
|
@ -6883,28 +6885,42 @@ export default function TaskPage(): React.JSX.Element {
|
|||
)
|
||||
return
|
||||
}
|
||||
toast.success(
|
||||
translate('auto.components.TaskPage.3f9604efc7', 'Opened issue #{{value0}}', {
|
||||
value0: result.number
|
||||
}),
|
||||
{
|
||||
action: result.url
|
||||
? {
|
||||
label: translate('auto.components.TaskPage.9c57663908', 'View'),
|
||||
onClick: () => window.open(result.url, '_blank')
|
||||
}
|
||||
: undefined
|
||||
}
|
||||
const createdIssueToast = translate(
|
||||
'auto.components.TaskPage.3f9604efc7',
|
||||
'Opened issue #{{value0}}',
|
||||
{ value0: result.number }
|
||||
)
|
||||
const createdIssueToastOptions = {
|
||||
action: result.url
|
||||
? {
|
||||
label: translate('auto.components.TaskPage.9c57663908', 'View'),
|
||||
onClick: () => window.open(result.url, '_blank')
|
||||
}
|
||||
: undefined
|
||||
}
|
||||
if (result.bodySaveWarning) {
|
||||
toast.warning(createdIssueToast, {
|
||||
...createdIssueToastOptions,
|
||||
description: result.bodySaveWarning
|
||||
})
|
||||
} else {
|
||||
toast.success(createdIssueToast, createdIssueToastOptions)
|
||||
}
|
||||
setNewIssueOpen(false)
|
||||
setNewIssueTitle('')
|
||||
setNewIssueBody('')
|
||||
setNewIssueLabels([])
|
||||
setNewIssueAssignees([])
|
||||
// Why: a successful submit is the only path that discards the recovery
|
||||
// draft. Closing `newIssueOpen` in the same commit keeps the write-through
|
||||
// effect (gated on it) from re-persisting the emptied fields.
|
||||
clearNewIssueDraft()
|
||||
if (result.bodySaveWarning) {
|
||||
// Why: retain the unsaved body for recovery, but clear the title so
|
||||
// reopening the composer cannot repeat the create with one click.
|
||||
setNewIssueTitle('')
|
||||
setNewIssueDraft({ title: '' })
|
||||
} else {
|
||||
setNewIssueTitle('')
|
||||
setNewIssueBody('')
|
||||
setNewIssueLabels([])
|
||||
setNewIssueAssignees([])
|
||||
// Why: only a complete success discards the recovery draft; a partial
|
||||
// body save keeps the original text available without another create.
|
||||
clearNewIssueDraft()
|
||||
}
|
||||
// Why: bump the nonce so the list refetches and shows the new issue.
|
||||
setTaskRefreshNonce((current) => current + 1)
|
||||
|
||||
|
|
@ -6973,7 +6989,8 @@ export default function TaskPage(): React.JSX.Element {
|
|||
newIssueTitle,
|
||||
openGitHubDetailPage,
|
||||
setDialogWorkItem,
|
||||
clearNewIssueDraft
|
||||
clearNewIssueDraft,
|
||||
setNewIssueDraft
|
||||
])
|
||||
|
||||
const handleCreateNewLinearProject = useCallback(async (): Promise<void> => {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,33 @@
|
|||
import { readFileSync } from 'node:fs'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
const taskPageSource = readFileSync(new URL('./TaskPage.tsx', import.meta.url), 'utf8')
|
||||
|
||||
function issueCreationSection(): string {
|
||||
const start = taskPageSource.indexOf('const handleCreateNewIssue')
|
||||
const end = taskPageSource.indexOf('const handleCreateNewLinearProject', start)
|
||||
return taskPageSource.slice(start, end)
|
||||
}
|
||||
|
||||
describe('TaskPage GitHub issue creation', () => {
|
||||
it('covers the complete remote oversized-body recovery timeout envelope', () => {
|
||||
const section = issueCreationSection()
|
||||
|
||||
expect(section).toContain("'github.createIssue'")
|
||||
expect(section).toContain('{ timeoutMs: 65_000 }')
|
||||
})
|
||||
|
||||
it('treats a body-save warning as created while preserving the recovery draft', () => {
|
||||
const section = issueCreationSection()
|
||||
const warningBranch = section.slice(
|
||||
section.indexOf('if (result.bodySaveWarning)'),
|
||||
section.indexOf('// Why: bump the nonce')
|
||||
)
|
||||
|
||||
expect(warningBranch).toContain('toast.warning')
|
||||
expect(warningBranch).toContain('description: result.bodySaveWarning')
|
||||
expect(warningBranch).toContain("setNewIssueDraft({ title: '' })")
|
||||
expect(warningBranch).toContain('} else {')
|
||||
expect(warningBranch).toContain('clearNewIssueDraft()')
|
||||
})
|
||||
})
|
||||
|
|
@ -1743,6 +1743,10 @@ export type GitHubCreateIssueFields = {
|
|||
assignees?: string[]
|
||||
}
|
||||
|
||||
export type GitHubCreateIssueResult =
|
||||
| { ok: true; number: number; url: string; bodySaveWarning?: string }
|
||||
| { ok: false; error: string }
|
||||
|
||||
export type GitHubIssueCloseReason = 'completed' | 'not_planned' | 'duplicate'
|
||||
|
||||
export type GitHubIssueUpdate = {
|
||||
|
|
|
|||
Loading…
Reference in New Issue