mirror of
https://github.com/stablyai/orca.git
synced 2026-09-28 16:02:45 +00:00
Fix Project mutation acceptance and Linear detail ordering tests
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
This commit is contained in:
@@ -108,14 +108,14 @@ describe('mobile GitHub Project host routing boundary', () => {
|
||||
)
|
||||
// Per method, not once per file: dropping prRepo from a single mutation must fail here.
|
||||
for (const method of [
|
||||
'fetchResolveReviewThread',
|
||||
'fetchAddPRReviewCommentReply',
|
||||
'fetchAddIssueComment',
|
||||
'fetchRequestPRReviewers',
|
||||
'fetchRerunPRChecks',
|
||||
'fetchMergePR'
|
||||
'github.resolveReviewThread',
|
||||
'github.addPRReviewCommentReply',
|
||||
'github.addIssueComment',
|
||||
'github.requestPRReviewers',
|
||||
'github.rerunPRChecks',
|
||||
'github.mergePR'
|
||||
]) {
|
||||
const offset = projectMutationAdapter.indexOf(`${method}(`)
|
||||
const offset = projectMutationAdapter.indexOf(`'${method}'`)
|
||||
expect(offset, `${method} must remain wired`).toBeGreaterThan(-1)
|
||||
// Bounded to this adapter method, so a neighbour's prRepo cannot satisfy it.
|
||||
expect(
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
import { expect, it, vi } from 'vitest'
|
||||
import type { RpcRequestSender } from '../transport/rpc-client'
|
||||
import { nativeHostTaskDetailOperations } from './native-host-task-detail-operations'
|
||||
|
||||
it('waits for raw Linear requests so a comments transport rejection wins over an issue refusal', async () => {
|
||||
let rejectComments!: (error: Error) => void
|
||||
const comments = new Promise<never>((_resolve, reject) => {
|
||||
rejectComments = reject
|
||||
})
|
||||
const sendRequest = vi.fn<RpcRequestSender['sendRequest']>((method) =>
|
||||
method === 'linear.getIssue'
|
||||
? Promise.resolve({
|
||||
id: 'test',
|
||||
_meta: { runtimeId: 'host' },
|
||||
ok: false,
|
||||
error: { code: 'refused', message: 'issue refused' }
|
||||
})
|
||||
: comments
|
||||
)
|
||||
const outcome = nativeHostTaskDetailOperations({ sendRequest })
|
||||
.loadLinear({ issueId: 'issue-1', workspaceId: 'workspace-1' })
|
||||
.catch((error: unknown) => error)
|
||||
await new Promise<void>((resolve) => setTimeout(resolve, 0))
|
||||
const commentsError = new Error('comments transport failed')
|
||||
rejectComments(commentsError)
|
||||
await expect(outcome).resolves.toBe(commentsError)
|
||||
})
|
||||
@@ -81,8 +81,13 @@ export function nativeHostTaskDetailOperations(client: RpcRequestSender): HostTa
|
||||
{ timeoutMs: 30_000 }
|
||||
)
|
||||
])
|
||||
const issue = await successfulResult<LinearIssue | null>(issueResponse)
|
||||
const comments = await optionalComments(commentsResponse)
|
||||
if (!issueResponse.ok) {
|
||||
throw new Error(issueResponse.error.message)
|
||||
}
|
||||
const issue = issueResponse.result as LinearIssue | null
|
||||
const comments = commentsResponse.ok
|
||||
? ((commentsResponse.result as DetailComment[]) ?? [])
|
||||
: []
|
||||
if (!issue) {
|
||||
throw new Error('Details not found')
|
||||
}
|
||||
@@ -91,13 +96,6 @@ export function nativeHostTaskDetailOperations(client: RpcRequestSender): HostTa
|
||||
}
|
||||
}
|
||||
|
||||
/** Tolerates a refusal envelope only. A transport rejection still fails the detail load, so a
|
||||
* timed-out comment read cannot render as an issue that simply has no comments. */
|
||||
async function optionalComments(request: Promise<unknown>): Promise<DetailComment[]> {
|
||||
const response = (await request) as { ok: boolean; result?: unknown }
|
||||
return response.ok ? ((response.result as DetailComment[]) ?? []) : []
|
||||
}
|
||||
|
||||
async function successfulResult<T>(request: Promise<unknown>): Promise<T> {
|
||||
const response = (await request) as {
|
||||
ok: boolean
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import type { RpcRequestSender } from '../transport/rpc-client'
|
||||
import type { HostTaskProjectMutationOperations } from './host-task-project-mutation-operations'
|
||||
import { nativeHostTaskProjectMutationOperations } from './native-host-task-project-mutation-operations'
|
||||
|
||||
const target = { owner: 'orca', repo: 'orca', host: 'github.com', number: 7, type: 'pr' as const }
|
||||
const mutations: Array<
|
||||
[string, (ops: HostTaskProjectMutationOperations) => Promise<unknown>, string]
|
||||
> = [
|
||||
[
|
||||
'updateItem',
|
||||
(ops) => ops.updateItem(target, { title: 'title' }),
|
||||
'Failed to update GitHub item'
|
||||
],
|
||||
[
|
||||
'updateIssue',
|
||||
(ops) => ops.updateItem({ ...target, type: 'issue' }, {}),
|
||||
'Failed to update GitHub item'
|
||||
],
|
||||
['updateComment', (ops) => ops.updateComment(target, 1, 'body'), 'Failed to edit comment'],
|
||||
['deleteComment', (ops) => ops.deleteComment(target, 1), 'Failed to delete comment'],
|
||||
['updateMetadata', (ops) => ops.updateMetadata(target, {}), 'Failed to update GitHub item'],
|
||||
[
|
||||
'updateField',
|
||||
(ops) =>
|
||||
ops.updateField({ ...target, projectId: 'p', itemId: 'i' }, 'f', {
|
||||
kind: 'text',
|
||||
text: 'value'
|
||||
}),
|
||||
'Failed to update project field'
|
||||
],
|
||||
[
|
||||
'clearField',
|
||||
(ops) => ops.updateField({ ...target, projectId: 'p', itemId: 'i' }, 'f', null),
|
||||
'Failed to update project field'
|
||||
],
|
||||
['updateIssueType', (ops) => ops.updateIssueType(target, null), 'Failed to update issue type'],
|
||||
[
|
||||
'replyReviewComment',
|
||||
(ops) => ops.replyReviewComment(target, 'repo-1', { commentId: 1, body: 'body' }),
|
||||
'Failed to reply'
|
||||
],
|
||||
[
|
||||
'addConversationComment',
|
||||
(ops) => ops.addConversationComment(target, 'repo-1', 'body'),
|
||||
'Failed to reply'
|
||||
],
|
||||
[
|
||||
'requestReviewers',
|
||||
(ops) => ops.requestReviewers(target, 'repo-1', ['reviewer']),
|
||||
'Failed to request reviewers'
|
||||
],
|
||||
[
|
||||
'rerunChecks',
|
||||
(ops) => ops.rerunChecks(target, 'repo-1', { failedOnly: true }),
|
||||
'Failed to rerun checks'
|
||||
],
|
||||
['merge', (ops) => ops.merge(target, 'repo-1', 'squash'), 'Failed to merge pull request']
|
||||
]
|
||||
|
||||
function operations(result: unknown) {
|
||||
const sendRequest = vi
|
||||
.fn<RpcRequestSender['sendRequest']>()
|
||||
.mockResolvedValue({ id: 'test', _meta: { runtimeId: 'host' }, ok: true, result })
|
||||
return nativeHostTaskProjectMutationOperations({ sendRequest })
|
||||
}
|
||||
|
||||
describe('Project mutation result acceptance', () => {
|
||||
it.each(mutations)(
|
||||
'%s rejects absent results, accepts missing ok, and reports refusal',
|
||||
async (_name, mutate, fallback) => {
|
||||
for (const result of [null, undefined]) {
|
||||
await expect(mutate(operations(result))).rejects.toBeInstanceOf(TypeError)
|
||||
}
|
||||
for (const result of [{}, { ok: undefined }, { ok: true }]) {
|
||||
await expect(mutate(operations(result))).resolves.toBeUndefined()
|
||||
}
|
||||
await expect(mutate(operations({ ok: false }))).rejects.toThrow(fallback)
|
||||
}
|
||||
)
|
||||
|
||||
it('addComment requires truthy ok and preserves the host message', async () => {
|
||||
for (const result of [null, undefined]) {
|
||||
await expect(operations(result).addComment(target, 'body')).rejects.toBeInstanceOf(TypeError)
|
||||
}
|
||||
for (const result of [{}, { ok: undefined }, { ok: false }]) {
|
||||
await expect(operations(result).addComment(target, 'body')).rejects.toThrow(
|
||||
'Failed to add comment'
|
||||
)
|
||||
}
|
||||
await expect(
|
||||
operations({ ok: false, error: { message: 'host refusal' } }).addComment(target, 'body')
|
||||
).rejects.toThrow('host refusal')
|
||||
await expect(operations({ ok: true }).addComment(target, 'body')).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
it('merge preserves its string refusal', async () => {
|
||||
await expect(
|
||||
operations({ ok: false, error: 'merge refusal' }).merge(target, 'repo-1', 'merge')
|
||||
).rejects.toThrow('merge refusal')
|
||||
})
|
||||
|
||||
it('resolveReviewThread requires the literal true result', async () => {
|
||||
for (const result of [null, undefined, {}, { ok: true }, false]) {
|
||||
await expect(
|
||||
operations(result).resolveReviewThread(target, 'repo-1', 'thread', true)
|
||||
).rejects.toThrow('Failed to resolve thread')
|
||||
}
|
||||
await expect(
|
||||
operations(true).resolveReviewThread(target, 'repo-1', 'thread', true)
|
||||
).resolves.toBeUndefined()
|
||||
})
|
||||
})
|
||||
@@ -4,15 +4,6 @@ import type {
|
||||
HostTaskProjectMutationOperations
|
||||
} from './host-task-project-mutation-operations'
|
||||
import type { RpcRequestSender } from '../transport/rpc-client'
|
||||
import {
|
||||
fetchAddIssueComment,
|
||||
fetchAddPRReviewCommentReply,
|
||||
fetchMergePR,
|
||||
fetchRequestPRReviewers,
|
||||
fetchRerunPRChecks,
|
||||
fetchResolveReviewThread,
|
||||
type GitHubPrMutationOutcome
|
||||
} from '../session/github-pr-mutations'
|
||||
|
||||
const PROJECT_PR_MUTATION_TIMEOUT_MS = 60_000
|
||||
/** Every project mutation carried a connect deadline before this seam existed. Without one the
|
||||
@@ -87,89 +78,84 @@ export function nativeHostTaskProjectMutationOperations(
|
||||
})
|
||||
},
|
||||
async resolveReviewThread(target, repoId, threadId, resolve) {
|
||||
requirePrMutation(
|
||||
await fetchResolveReviewThread(
|
||||
client,
|
||||
repoId,
|
||||
{
|
||||
threadId,
|
||||
resolve,
|
||||
// Why: a draft row
|
||||
// has no slug — send it only when one resolved rather than an empty pair.
|
||||
prRepo: prRepoPayload(target)
|
||||
},
|
||||
{ timeoutMs: PROJECT_MUTATION_TIMEOUT_MS }
|
||||
),
|
||||
resolve ? 'Failed to resolve thread' : 'Failed to reopen thread'
|
||||
const response = await client.sendRequest(
|
||||
'github.resolveReviewThread',
|
||||
{
|
||||
repo: `id:${repoId}`,
|
||||
prRepo: prRepoPayload(target),
|
||||
threadId,
|
||||
resolve
|
||||
},
|
||||
{ timeoutMs: PROJECT_MUTATION_TIMEOUT_MS }
|
||||
)
|
||||
if (!response.ok) {
|
||||
throw new Error(response.error.message)
|
||||
}
|
||||
if (response.result !== true) {
|
||||
throw new Error(resolve ? 'Failed to resolve thread' : 'Failed to reopen thread')
|
||||
}
|
||||
},
|
||||
async replyReviewComment(target, repoId, payload) {
|
||||
return prMutationComment(
|
||||
await fetchAddPRReviewCommentReply(
|
||||
client,
|
||||
repoId,
|
||||
{
|
||||
prNumber: target.number,
|
||||
...payload,
|
||||
prRepo: prRepoPayload(target)
|
||||
},
|
||||
{ timeoutMs: PROJECT_MUTATION_TIMEOUT_MS }
|
||||
),
|
||||
'Failed to reply'
|
||||
const result = await projectMutation<{ comment?: DetailComment }>(
|
||||
client,
|
||||
'github.addPRReviewCommentReply',
|
||||
{
|
||||
repo: `id:${repoId}`,
|
||||
prNumber: target.number,
|
||||
prRepo: prRepoPayload(target),
|
||||
...payload
|
||||
}
|
||||
)
|
||||
return result.comment
|
||||
},
|
||||
async addConversationComment(target, repoId, body) {
|
||||
return prMutationComment(
|
||||
await fetchAddIssueComment(
|
||||
client,
|
||||
repoId,
|
||||
{
|
||||
prNumber: target.number,
|
||||
body,
|
||||
prRepo: prRepoPayload(target),
|
||||
type: target.type
|
||||
},
|
||||
{ timeoutMs: PROJECT_MUTATION_TIMEOUT_MS }
|
||||
),
|
||||
'Failed to reply'
|
||||
const result = await projectMutation<{ comment?: DetailComment }>(
|
||||
client,
|
||||
'github.addIssueComment',
|
||||
{
|
||||
repo: `id:${repoId}`,
|
||||
number: target.number,
|
||||
prRepo: prRepoPayload(target),
|
||||
body,
|
||||
type: target.type
|
||||
}
|
||||
)
|
||||
return result.comment
|
||||
},
|
||||
async requestReviewers(target, repoId, reviewers) {
|
||||
requirePrMutation(
|
||||
await fetchRequestPRReviewers(
|
||||
client,
|
||||
repoId,
|
||||
{
|
||||
prNumber: target.number,
|
||||
reviewers,
|
||||
prRepo: prRepoPayload(target)
|
||||
},
|
||||
{ timeoutMs: PROJECT_MUTATION_TIMEOUT_MS }
|
||||
),
|
||||
'Failed to request reviewers'
|
||||
)
|
||||
await projectMutation(client, 'github.requestPRReviewers', {
|
||||
repo: `id:${repoId}`,
|
||||
prNumber: target.number,
|
||||
prRepo: prRepoPayload(target),
|
||||
reviewers
|
||||
})
|
||||
},
|
||||
async rerunChecks(target, repoId, payload) {
|
||||
requirePrMutation(
|
||||
await fetchRerunPRChecks(
|
||||
client,
|
||||
repoId,
|
||||
{ prNumber: target.number, ...payload, prRepo: prRepoPayload(target) },
|
||||
// A CI rerun and a merge both routinely outrun the 30s default.
|
||||
{ timeoutMs: PROJECT_PR_MUTATION_TIMEOUT_MS }
|
||||
),
|
||||
'Failed to rerun checks'
|
||||
await projectMutation(
|
||||
client,
|
||||
'github.rerunPRChecks',
|
||||
{
|
||||
repo: `id:${repoId}`,
|
||||
prNumber: target.number,
|
||||
prRepo: prRepoPayload(target),
|
||||
...payload
|
||||
},
|
||||
false,
|
||||
PROJECT_PR_MUTATION_TIMEOUT_MS
|
||||
)
|
||||
},
|
||||
async merge(target, repoId, method) {
|
||||
requirePrMutation(
|
||||
await fetchMergePR(
|
||||
client,
|
||||
repoId,
|
||||
{ prNumber: target.number, method, prRepo: prRepoPayload(target) },
|
||||
{ timeoutMs: PROJECT_PR_MUTATION_TIMEOUT_MS }
|
||||
),
|
||||
'Failed to merge pull request'
|
||||
await projectMutation(
|
||||
client,
|
||||
'github.mergePR',
|
||||
{
|
||||
repo: `id:${repoId}`,
|
||||
prNumber: target.number,
|
||||
prRepo: prRepoPayload(target),
|
||||
method
|
||||
},
|
||||
false,
|
||||
PROJECT_PR_MUTATION_TIMEOUT_MS
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -202,49 +188,40 @@ const PROJECT_MUTATION_FALLBACKS: Record<string, string> = {
|
||||
'github.project.deleteIssueCommentBySlug': 'Failed to delete comment',
|
||||
'github.project.updateItemField': 'Failed to update project field',
|
||||
'github.project.clearItemField': 'Failed to update project field',
|
||||
'github.project.updateIssueTypeBySlug': 'Failed to update issue type'
|
||||
'github.project.updateIssueTypeBySlug': 'Failed to update issue type',
|
||||
'github.addPRReviewCommentReply': 'Failed to reply',
|
||||
'github.addIssueComment': 'Failed to reply',
|
||||
'github.requestPRReviewers': 'Failed to request reviewers',
|
||||
'github.rerunPRChecks': 'Failed to rerun checks',
|
||||
'github.mergePR': 'Failed to merge pull request'
|
||||
}
|
||||
|
||||
async function projectMutation<T extends object = object>(
|
||||
client: RpcRequestSender,
|
||||
method: string,
|
||||
payload: object,
|
||||
requireOk = false
|
||||
requireOk = false,
|
||||
timeoutMs = PROJECT_MUTATION_TIMEOUT_MS
|
||||
): Promise<T> {
|
||||
const fallback = PROJECT_MUTATION_FALLBACKS[method] ?? 'GitHub Project request failed'
|
||||
const response = (await client.sendRequest(method, payload, { timeoutMs: 30_000 })) as {
|
||||
ok: boolean
|
||||
result?: { ok?: boolean; error?: string | { message?: string } }
|
||||
error?: { message?: string }
|
||||
}
|
||||
const response = await client.sendRequest(method, payload, { timeoutMs })
|
||||
if (!response.ok) {
|
||||
throw new Error(response.error?.message ?? fallback)
|
||||
throw new Error(response.error.message)
|
||||
}
|
||||
if (requireOk && !response.result?.ok) {
|
||||
throw new Error(fallback)
|
||||
const result = response.result as { ok?: boolean; error?: string | { message?: string } }
|
||||
if (requireOk ? !result.ok : result.ok === false) {
|
||||
const error = result.error
|
||||
if (!method.startsWith('github.project.')) {
|
||||
throw new Error((error as string | undefined) ?? fallback)
|
||||
}
|
||||
const acceptsString =
|
||||
method === 'github.project.updateIssueCommentBySlug' ||
|
||||
method === 'github.project.deleteIssueCommentBySlug'
|
||||
throw new Error(
|
||||
acceptsString && typeof error === 'string'
|
||||
? error
|
||||
: ((typeof error === 'object' ? error?.message : undefined) ?? fallback)
|
||||
)
|
||||
}
|
||||
if (response.result?.ok === false) {
|
||||
const error = response.result.error
|
||||
throw new Error(typeof error === 'string' ? error : (error?.message ?? fallback))
|
||||
}
|
||||
return (response.result ?? {}) as T
|
||||
}
|
||||
|
||||
/** The wrapper substitutes its own copy for two cases the caller used to word itself: a host
|
||||
* that says nothing (`Request failed: <method>`) and a review thread it could not update. */
|
||||
const WRAPPER_SUBSTITUTED_COPY = ['Request failed: ', 'Failed to update review thread.']
|
||||
|
||||
function requirePrMutation(result: GitHubPrMutationOutcome, fallback: string): void {
|
||||
if (!result.ok) {
|
||||
const substituted = WRAPPER_SUBSTITUTED_COPY.some((copy) => result.error.startsWith(copy))
|
||||
throw new Error(substituted ? fallback : result.error)
|
||||
}
|
||||
}
|
||||
|
||||
function prMutationComment(
|
||||
result: GitHubPrMutationOutcome,
|
||||
fallback: string
|
||||
): DetailComment | undefined {
|
||||
requirePrMutation(result, fallback)
|
||||
return (result as { comment?: DetailComment }).comment
|
||||
return result as T
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user