From 6f845d182f1a0ef5efd5366cfdf973dfd68623a4 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 14 Sep 2026 13:50:34 -0400 Subject: [PATCH] refactor(mobile): send the github.* PR surface and the review loaders through RpcOperation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The step-4 first half for `src/session/`: eight files, 38 references to the raw request port, all replaced with declared operations. No behaviour change — the 50 goldens recorded in the previous commit do not move, which is the claim. - 21 operations over 21 methods. The seven PR reads keep their defensive parsers as readers; the ten status-envelope mutations share one reader because the `{ok, error}` convention is one host convention, not ten; the two bare-boolean mutations read the payload unchecked because `=== true` is the caller's confirmation rule. - Four second readers, each justified in place: git.status and git.branchCompare for the PR branch context (a refusal costs a fallback, not the screen), git.branchCompare and git.branchDiff for review (the projection is not a superset of the verbatim payload), and worktree.show for the review notes the summary reader drops. - Every failure text is preserved, including the two main kept apart: a refusal with no message falls back to the screen's copy, a transport drop with no message surfaces its empty message verbatim. `sendRaw`'s callers replaced theirs a second time, so those fall back on both paths. - No retry, and no operation reads a dropped reply as a failed mutation: the rejection reaches each wrapper's catch as the original object. - `github-pr-mutations.ts` split along the action/comment seam it already had in its consumers, so no file needs a max-lines bump. Inventory: src/session/ 47 files / 114 references -> 39 / 76. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- .../session/github-pr-comment-mutations.ts | 137 +++++++ .../session/github-pr-mutation-operations.ts | 130 +++++++ .../src/session/github-pr-mutation-outcome.ts | 94 +++++ mobile/src/session/github-pr-mutations.ts | 354 +++++------------- mobile/src/session/github-pr-parsers.ts | 27 ++ .../src/session/github-pr-read-operations.ts | 143 +++++++ mobile/src/session/github-pr-repo-slug.ts | 84 +++++ mobile/src/session/github-pr-rpc.ts | 281 ++++++-------- .../src/session/mobile-diff-review-loaders.ts | 119 ++++-- .../session/mobile-diff-review-operations.ts | 137 +++++++ .../mobile-review-terminal-operations.ts | 53 +++ .../src/session/mobile-session-rpc-sender.ts | 10 + mobile/src/session/pr-ai-triage-launch.ts | 38 +- mobile/src/session/use-mobile-pr-actions.ts | 6 +- .../session/use-mobile-pr-branch-context.ts | 12 +- .../session/use-mobile-pr-comment-actions.ts | 6 +- .../src/session/use-mobile-pr-title-action.ts | 6 +- .../unvalidated-rpc-request-port-inventory.ts | 8 - 18 files changed, 1145 insertions(+), 500 deletions(-) create mode 100644 mobile/src/session/github-pr-comment-mutations.ts create mode 100644 mobile/src/session/github-pr-mutation-operations.ts create mode 100644 mobile/src/session/github-pr-mutation-outcome.ts create mode 100644 mobile/src/session/github-pr-read-operations.ts create mode 100644 mobile/src/session/github-pr-repo-slug.ts create mode 100644 mobile/src/session/mobile-diff-review-operations.ts create mode 100644 mobile/src/session/mobile-review-terminal-operations.ts create mode 100644 mobile/src/session/mobile-session-rpc-sender.ts diff --git a/mobile/src/session/github-pr-comment-mutations.ts b/mobile/src/session/github-pr-comment-mutations.ts new file mode 100644 index 00000000000..91c00c95210 --- /dev/null +++ b/mobile/src/session/github-pr-comment-mutations.ts @@ -0,0 +1,137 @@ +import { + githubPrIssueCommentAdd, + githubPrIssueCommentDelete, + githubPrIssueCommentEdit, + githubPrReviewCommentReplyAdd, + githubPrReviewThreadResolve +} from './github-pr-mutation-operations' +import { + settleGithubPrConfirmation, + settleGithubPrMutation, + type GitHubPrMutationOutcome +} from './github-pr-mutation-outcome' +import { + githubPrRepoSlugParam, + githubPrRequestParams, + type GitHubPrRepoSlug +} from './github-pr-repo-slug' +import type { MobileSessionRpcSender } from './mobile-session-rpc-sender' + +// The conversation half of the github.* PR mutation surface: review-thread replies, root comments, +// thread resolution and the slug-addressed comment edit/delete. Split from the PR action mutations +// because the two are driven by different hooks and this file was over the max-lines budget. + +// Reply within a review thread. Host returns GitHubCommentResult +// (`{ ok, comment } | { ok:false, error }`), which the status reader admits. +// We refetch afterward, so the returned comment is unused. +export function fetchAddPRReviewCommentReply( + client: MobileSessionRpcSender, + worktreeId: string, + args: { + prNumber: number + commentId: number + body: string + threadId?: string + path?: string + line?: number + prRepo?: GitHubPrRepoSlug | null + } +): Promise { + const params: Record = { + prNumber: args.prNumber, + commentId: args.commentId, + body: args.body + } + if (args.threadId) { + params.threadId = args.threadId + } + if (args.path) { + params.path = args.path + } + if (typeof args.line === 'number') { + params.line = args.line + } + return settleGithubPrMutation(githubPrReviewCommentReplyAdd, () => + githubPrReviewCommentReplyAdd.request( + client, + githubPrRequestParams(githubPrReviewCommentReplyAdd.operation.method, worktreeId, params, { + prRepo: args.prRepo + }) + ) + ) +} + +// Add a root conversation comment to the PR. Host returns GitHubCommentResult. +export function fetchAddIssueComment( + client: MobileSessionRpcSender, + worktreeId: string, + args: { prNumber: number; body: string; prRepo?: GitHubPrRepoSlug | null } +): Promise { + const params: Record = { + number: args.prNumber, + body: args.body, + type: 'pr' + } + return settleGithubPrMutation(githubPrIssueCommentAdd, () => + githubPrIssueCommentAdd.request( + client, + githubPrRequestParams(githubPrIssueCommentAdd.operation.method, worktreeId, params, { + prRepo: args.prRepo + }) + ) + ) +} + +// Resolve/unresolve a review thread. `resolve` picks the direction (the host runs +// the matching GraphQL mutation). Unlike the comment mutations, the host returns a +// bare boolean, so a falsy result is a failure rather than the "no status" success. +export function fetchResolveReviewThread( + client: MobileSessionRpcSender, + worktreeId: string, + args: { threadId: string; resolve: boolean; prRepo?: GitHubPrRepoSlug | null } +): Promise { + return settleGithubPrConfirmation( + githubPrReviewThreadResolve, + () => + githubPrReviewThreadResolve.request( + client, + githubPrRequestParams( + githubPrReviewThreadResolve.operation.method, + worktreeId, + { threadId: args.threadId, resolve: args.resolve }, + { prRepo: args.prRepo } + ) + ), + 'Failed to update review thread.' + ) +} + +// Edit a root conversation (issue) comment. The host RPC is slug-addressed +// (owner/repo/commentId), not worktree-addressed, so the params are passed +// directly rather than via the PR-scoped builder. Host returns the +// GitHubProjectMutationResult `{ ok }` envelope the status reader admits. +export function fetchUpdateIssueComment( + client: MobileSessionRpcSender, + args: { owner: string; repo: string; host?: string; commentId: number; body: string } +): Promise { + return settleGithubPrMutation(githubPrIssueCommentEdit, () => + githubPrIssueCommentEdit.request(client, { + ...githubPrRepoSlugParam(args), + commentId: args.commentId, + body: args.body + }) + ) +} + +// Delete a root conversation (issue) comment. Slug-addressed like the edit wrapper. +export function fetchDeleteIssueComment( + client: MobileSessionRpcSender, + args: { owner: string; repo: string; host?: string; commentId: number } +): Promise { + return settleGithubPrMutation(githubPrIssueCommentDelete, () => + githubPrIssueCommentDelete.request(client, { + ...githubPrRepoSlugParam(args), + commentId: args.commentId + }) + ) +} diff --git a/mobile/src/session/github-pr-mutation-operations.ts b/mobile/src/session/github-pr-mutation-operations.ts new file mode 100644 index 00000000000..0a035a8b65d --- /dev/null +++ b/mobile/src/session/github-pr-mutation-operations.ts @@ -0,0 +1,130 @@ +import { bindDeferredRpcOperation, defineRpcOperation } from '../transport/rpc-operation' +import type { RpcMethodName } from '../transport/rpc-params-contract' +import type { RpcCompatibleReader } from '../transport/rpc-operation-contract' +import { rpcPayloadMember, rpcReadUnchecked } from '../transport/rpc-reader-payload' + +// Host-state changes on the `github.*` PR surface. A lost reply here is *unknown*, never failed: +// none of these operations interprets a transport rejection, so the rejection object — and the +// delivery-unknown mark the WeakSet holds on it — reaches the wrapper's own catch intact. The +// wrappers still collapse it into their `{ ok: false }` outcome, exactly as main did; nothing here +// retries, and no operation below treats a dropped reply as evidence the mutation did not happen. + +/** + * What a PR mutation reported in-band. `structured: false` is the host returning void or a bare + * value with no `ok` member, which every caller has always read as success. + */ +export type GitHubPrMutationStatus = + | { readonly structured: false } + | { readonly structured: true; readonly ok: unknown; readonly error: unknown } + +/** + * One reader for ten methods, not ten readers. + * + * The `ok in result` test and the `error` read are a single host convention — GitHubProjectMutation + * -Result and GitHubCommentResult share it — so there is no input on which two of these methods + * would want different answers. Which failure text a caller shows is the caller's, not the + * reader's: `extractMutationError` still names the method in its fallback. + */ +const mutationStatusReader: RpcCompatibleReader< + unknown, + 'pr-mutation-status', + GitHubPrMutationStatus +> = (raw) => + raw && typeof raw === 'object' && 'ok' in raw + ? rpcReadUnchecked('pr-mutation-status', { + structured: true, + ok: raw.ok, + error: rpcPayloadMember(raw, 'error') + }) + : rpcReadUnchecked('pr-mutation-status', { structured: false }) + +// Ten operations, one definition site: they share a method-independent acceptance, barrier and +// reader, and writing the same five lines ten times would hide that rather than show it. Name and +// method stay per operation, which is what a call site picks. +function mutationStatusOperation(name: string, method: Method) { + return bindDeferredRpcOperation( + defineRpcOperation({ + name, + method, + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: mutationStatusReader + }) + ) +} + +export const githubPrMergeRun = mutationStatusOperation('github.merge-pr', 'github.mergePR') + +export const githubPrAutoMergeSet = mutationStatusOperation( + 'github.set-pr-auto-merge', + 'github.setPRAutoMerge' +) + +export const githubPrStateSet = mutationStatusOperation( + 'github.update-pr-state', + 'github.updatePRState' +) + +export const githubPrReviewersRequest = mutationStatusOperation( + 'github.request-pr-reviewers', + 'github.requestPRReviewers' +) + +export const githubPrReviewersRemove = mutationStatusOperation( + 'github.remove-pr-reviewers', + 'github.removePRReviewers' +) + +export const githubPrChecksRerun = mutationStatusOperation( + 'github.rerun-pr-checks', + 'github.rerunPRChecks' +) + +export const githubPrReviewCommentReplyAdd = mutationStatusOperation( + 'github.add-pr-review-comment-reply', + 'github.addPRReviewCommentReply' +) + +export const githubPrIssueCommentAdd = mutationStatusOperation( + 'github.add-issue-comment', + 'github.addIssueComment' +) + +export const githubPrIssueCommentEdit = mutationStatusOperation( + 'github.update-issue-comment-by-slug', + 'github.project.updateIssueCommentBySlug' +) + +export const githubPrIssueCommentDelete = mutationStatusOperation( + 'github.delete-issue-comment-by-slug', + 'github.project.deleteIssueCommentBySlug' +) + +// The two mutations whose host result is a bare boolean rather than a status envelope. Their +// payload is unread here on purpose: `=== true` is the caller's confirmation rule, and reading it +// as a status would turn a `false` into the "no structured status" success the envelope methods get. +const mutationConfirmationReader: RpcCompatibleReader< + unknown, + 'pr-mutation-confirmation', + unknown +> = (raw) => rpcReadUnchecked('pr-mutation-confirmation', raw) + +export const githubPrTitleSet = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'github.update-pr-title', + method: 'github.updatePRTitle', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: mutationConfirmationReader + }) +) + +export const githubPrReviewThreadResolve = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'github.resolve-review-thread', + method: 'github.resolveReviewThread', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: mutationConfirmationReader + }) +) diff --git a/mobile/src/session/github-pr-mutation-outcome.ts b/mobile/src/session/github-pr-mutation-outcome.ts new file mode 100644 index 00000000000..13c2d211074 --- /dev/null +++ b/mobile/src/session/github-pr-mutation-outcome.ts @@ -0,0 +1,94 @@ +import type { RpcMethodName } from '../transport/rpc-params-contract' +import { refusedRpcMessageOrFallback } from '../transport/rpc-refusal-message' +import type { RpcResponse } from '../transport/types' +import type { GitHubPrMutationStatus } from './github-pr-mutation-operations' + +// How a `github.*` PR mutation's reply becomes the one outcome the action engine routes on. The +// two settle shapes below are the two reply contracts the host uses, and they differ in one place +// that matters: what an empty failure message becomes. + +export type GitHubPrMutationOutcome = { ok: true } | { ok: false; error: string } + +// Host failure `error` is either a bare string (github.* PR mutations) or an +// object `{ message }` (github.project.* slug mutations). Read whichever is present +// so the slug edit/delete failures surface a real message, not a generic fallback. +export function extractMutationError(error: unknown, method: string): string { + if (typeof error === 'string') { + return error + } + if (error && typeof error === 'object' && 'message' in error) { + const message = error.message + if (typeof message === 'string' && message.length > 0) { + return message + } + } + return `Request failed: ${method}` +} + +/** As much of a bound mutation operation as the settle shapes below need. */ +export type GitHubPrMutationOperation = { + readonly operation: { readonly method: RpcMethodName } + readonly interpret: (reply: RpcResponse) => Value +} + +/** + * The status-envelope mutations. Two catches, because main had two paths: a transport drop + * surfaces its own message verbatim, empty included, while a refusal with no message falls back to + * the method's copy. The transport rejection reaches this catch as the original object, so the + * delivery-unknown mark it carries is intact for anything that later asks — nothing here retries, + * and a dropped reply is never read as evidence the mutation failed to reach the host. + */ +export async function settleGithubPrMutation( + mutation: GitHubPrMutationOperation, + send: () => Promise +): Promise { + const method = mutation.operation.method + const fallback = `Request failed: ${method}` + let reply: RpcResponse + try { + reply = await send() + } catch (error) { + return { ok: false, error: error instanceof Error ? error.message : fallback } + } + let status: GitHubPrMutationStatus + try { + status = mutation.interpret(reply) + } catch (error) { + return { ok: false, error: refusedRpcMessageOrFallback(error, fallback) } + } + // No structured status (host returned void/undefined) — treat as success. + if (!status.structured || status.ok === true) { + return { ok: true } + } + return { ok: false, error: extractMutationError(status.error, method) } +} + +/** + * The two mutations whose host result is a bare boolean. + * + * Both catches fall back here, unlike the status-envelope shape above: main sent these through + * `sendRaw`, whose empty message was then replaced by the wrapper's own `|| 'Request failed: …'`, + * so an empty transport message never reached the caller on this path. + */ +export async function settleGithubPrConfirmation( + mutation: GitHubPrMutationOperation, + send: () => Promise, + unconfirmed: string +): Promise { + const fallback = `Request failed: ${mutation.operation.method}` + let reply: RpcResponse + try { + reply = await send() + } catch (error) { + return { ok: false, error: refusedRpcMessageOrFallback(error, fallback) } + } + let confirmation: unknown + try { + confirmation = mutation.interpret(reply) + } catch (error) { + return { ok: false, error: refusedRpcMessageOrFallback(error, fallback) } + } + // Why: the host returns a bare `true` on success; a missing/undefined result is + // not a confirmed success, so require an explicit `=== true` rather than `!== false`. + return confirmation === true ? { ok: true } : { ok: false, error: unconfirmed } +} diff --git a/mobile/src/session/github-pr-mutations.ts b/mobile/src/session/github-pr-mutations.ts index bd1d9c423b9..4d4598fd58f 100644 --- a/mobile/src/session/github-pr-mutations.ts +++ b/mobile/src/session/github-pr-mutations.ts @@ -1,83 +1,36 @@ import type { GitHubPRMergeMethod } from '../../../src/shared/github/pull-request-types' -import type { RpcClient } from '../transport/rpc-client' -import { buildGithubPrParams, githubPrRepoSlugParam, type GitHubPrRepoSlug } from './github-pr-rpc' +import { + githubPrAutoMergeSet, + githubPrChecksRerun, + githubPrMergeRun, + githubPrReviewersRemove, + githubPrReviewersRequest, + githubPrStateSet, + githubPrTitleSet +} from './github-pr-mutation-operations' +import { + settleGithubPrConfirmation, + settleGithubPrMutation, + type GitHubPrMutationOutcome +} from './github-pr-mutation-outcome' +import { githubPrRequestParams, type GitHubPrRepoSlug } from './github-pr-repo-slug' +import type { MobileSessionRpcSender } from './mobile-session-rpc-sender' -// Mutation wrappers for the github.* PR surface, split out so github-pr-rpc.ts -// stays under the max-lines budget. They mirror the read wrappers' shape but -// return a host-status outcome (the host mutations all return -// `{ ok: true } | { ok: false; error: string }`). +// The PR action half of the github.* mutation surface: merge, auto-merge, open/close, reviewers, +// check reruns and the inline title edit. The conversation mutations live next door; both are +// re-exported here so consumers keep one entry point for the surface. -export type GitHubPrMutationOutcome = { ok: true } | { ok: false; error: string } +export type { GitHubPrMutationOutcome } from './github-pr-mutation-outcome' +export { + fetchAddIssueComment, + fetchAddPRReviewCommentReply, + fetchDeleteIssueComment, + fetchResolveReviewThread, + fetchUpdateIssueComment +} from './github-pr-comment-mutations' -// Sends a request whose host result is a bare boolean (not the `{ ok }` envelope), -// normalizing a transport throw into a failure so the raw-boolean callers below -// never see an unhandled rejection. -type RawResult = { ok: true; result: unknown } | { ok: false; error: string } - -async function sendRaw( - client: Pick, - method: string, - params: Record -): Promise { - try { - const response = await client.sendRequest(method, params) - if (!response.ok) { - return { ok: false, error: response.error?.message || `Request failed: ${method}` } - } - return { ok: true, result: response.result } - } catch (err) { - return { ok: false, error: err instanceof Error ? err.message : `Request failed: ${method}` } - } -} - -// Host failure `error` is either a bare string (github.* PR mutations) or an -// object `{ message }` (github.project.* slug mutations). Read whichever is present -// so the slug edit/delete failures surface a real message, not a generic fallback. -function extractMutationError(error: unknown, method: string): string { - if (typeof error === 'string') { - return error - } - if (error && typeof error === 'object' && 'message' in error) { - const message = (error as { message?: unknown }).message - if (typeof message === 'string' && message.length > 0) { - return message - } - } - return `Request failed: ${method}` -} - -// The host returns the success/failure shape inside `result`; a transport-level -// `response.ok === false` (timeout/connection) is also a failure. Both collapse -// into one outcome the action hook classifies via classifyPrSidebarFailure. -async function sendGithubPrMutation( - client: Pick, - method: string, - params: Record -): Promise { - try { - const response = await client.sendRequest(method, params) - if (!response.ok) { - return { ok: false, error: response.error?.message || `Request failed: ${method}` } - } - const result = response.result - if (result && typeof result === 'object' && 'ok' in result) { - const r = result as { ok: boolean; error?: unknown } - if (r.ok === true) { - return { ok: true } - } - return { ok: false, error: extractMutationError(r.error, method) } - } - // No structured status (host returned void/undefined) — treat as success. - return { ok: true } - } catch (err) { - // Why: a transport drop must not escape as an unhandled rejection — normalize - // to the `{ ok:false, error }` outcome the action engine routes on. - return { ok: false, error: err instanceof Error ? err.message : `Request failed: ${method}` } - } -} - -export async function fetchMergePR( - client: Pick, +export function fetchMergePR( + client: MobileSessionRpcSender, worktreeId: string, args: { prNumber: number; method?: GitHubPRMergeMethod; prRepo?: GitHubPrRepoSlug | null } ): Promise { @@ -85,40 +38,39 @@ export async function fetchMergePR( if (args.method) { params.method = args.method } - return sendGithubPrMutation( - client, - 'github.mergePR', - buildGithubPrParams('github.mergePR', worktreeId, params, { prRepo: args.prRepo }) + return settleGithubPrMutation(githubPrMergeRun, () => + githubPrMergeRun.request( + client, + githubPrRequestParams(githubPrMergeRun.operation.method, worktreeId, params, { + prRepo: args.prRepo + }) + ) ) } // Edit the hosted-review title. The host returns a bare boolean (true on success), -// which sendGithubPrMutation reads via its "no structured status" success branch -// only when not boolean — so handle the boolean explicitly like resolveReviewThread. -export async function fetchUpdatePRTitle( - client: Pick, +// so it takes the confirmation shape rather than the status envelope. +export function fetchUpdatePRTitle( + client: MobileSessionRpcSender, worktreeId: string, args: { prNumber: number; title: string; prRepo?: GitHubPrRepoSlug | null } ): Promise { const params: Record = { prNumber: args.prNumber, title: args.title } - const response = await sendRaw( - client, - 'github.updatePRTitle', - buildGithubPrParams('github.updatePRTitle', worktreeId, params, { prRepo: args.prRepo }) + return settleGithubPrConfirmation( + githubPrTitleSet, + () => + githubPrTitleSet.request( + client, + githubPrRequestParams(githubPrTitleSet.operation.method, worktreeId, params, { + prRepo: args.prRepo + }) + ), + 'Failed to update title.' ) - if (!response.ok) { - return { ok: false, error: response.error || 'Request failed: github.updatePRTitle' } - } - // Why: the host returns a bare `true` on success; a missing/undefined result is - // not a confirmed success, so require an explicit `=== true` rather than `!== false`. - if (response.result !== true) { - return { ok: false, error: 'Failed to update title.' } - } - return { ok: true } } -export async function fetchSetPRAutoMerge( - client: Pick, +export function fetchSetPRAutoMerge( + client: MobileSessionRpcSender, worktreeId: string, args: { prNumber: number @@ -131,181 +83,72 @@ export async function fetchSetPRAutoMerge( if (args.method) { params.method = args.method } - return sendGithubPrMutation( - client, - 'github.setPRAutoMerge', - buildGithubPrParams('github.setPRAutoMerge', worktreeId, params, { prRepo: args.prRepo }) + return settleGithubPrMutation(githubPrAutoMergeSet, () => + githubPrAutoMergeSet.request( + client, + githubPrRequestParams(githubPrAutoMergeSet.operation.method, worktreeId, params, { + prRepo: args.prRepo + }) + ) ) } -export async function fetchUpdatePRState( - client: Pick, +export function fetchUpdatePRState( + client: MobileSessionRpcSender, worktreeId: string, args: { prNumber: number; state: 'open' | 'closed'; prRepo?: GitHubPrRepoSlug | null } ): Promise { - return sendGithubPrMutation( - client, - 'github.updatePRState', - buildGithubPrParams( - 'github.updatePRState', - worktreeId, - { prNumber: args.prNumber, updates: { state: args.state } }, - { prRepo: args.prRepo } + return settleGithubPrMutation(githubPrStateSet, () => + githubPrStateSet.request( + client, + githubPrRequestParams( + githubPrStateSet.operation.method, + worktreeId, + { prNumber: args.prNumber, updates: { state: args.state } }, + { prRepo: args.prRepo } + ) ) ) } -export async function fetchRequestPRReviewers( - client: Pick, +export function fetchRequestPRReviewers( + client: MobileSessionRpcSender, worktreeId: string, args: { prNumber: number; reviewers: string[]; prRepo?: GitHubPrRepoSlug | null } ): Promise { - return sendGithubPrMutation( - client, - 'github.requestPRReviewers', - buildGithubPrParams( - 'github.requestPRReviewers', - worktreeId, - { prNumber: args.prNumber, reviewers: args.reviewers }, - { prRepo: args.prRepo } + return settleGithubPrMutation(githubPrReviewersRequest, () => + githubPrReviewersRequest.request( + client, + githubPrRequestParams( + githubPrReviewersRequest.operation.method, + worktreeId, + { prNumber: args.prNumber, reviewers: args.reviewers }, + { prRepo: args.prRepo } + ) ) ) } -export async function fetchRemovePRReviewers( - client: Pick, +export function fetchRemovePRReviewers( + client: MobileSessionRpcSender, worktreeId: string, args: { prNumber: number; reviewers: string[]; prRepo?: GitHubPrRepoSlug | null } ): Promise { - return sendGithubPrMutation( - client, - 'github.removePRReviewers', - buildGithubPrParams( - 'github.removePRReviewers', - worktreeId, - { prNumber: args.prNumber, reviewers: args.reviewers }, - { prRepo: args.prRepo } + return settleGithubPrMutation(githubPrReviewersRemove, () => + githubPrReviewersRemove.request( + client, + githubPrRequestParams( + githubPrReviewersRemove.operation.method, + worktreeId, + { prNumber: args.prNumber, reviewers: args.reviewers }, + { prRepo: args.prRepo } + ) ) ) } -// Reply within a review thread. Host returns GitHubCommentResult -// (`{ ok, comment } | { ok:false, error }`), which sendGithubPrMutation reads via -// its `ok in result` branch. We refetch afterward, so the returned comment is unused. -export async function fetchAddPRReviewCommentReply( - client: Pick, - worktreeId: string, - args: { - prNumber: number - commentId: number - body: string - threadId?: string - path?: string - line?: number - prRepo?: GitHubPrRepoSlug | null - } -): Promise { - const params: Record = { - prNumber: args.prNumber, - commentId: args.commentId, - body: args.body - } - if (args.threadId) { - params.threadId = args.threadId - } - if (args.path) { - params.path = args.path - } - if (typeof args.line === 'number') { - params.line = args.line - } - return sendGithubPrMutation( - client, - 'github.addPRReviewCommentReply', - buildGithubPrParams('github.addPRReviewCommentReply', worktreeId, params, { - prRepo: args.prRepo - }) - ) -} - -// Add a root conversation comment to the PR. Host returns GitHubCommentResult. -export async function fetchAddIssueComment( - client: Pick, - worktreeId: string, - args: { prNumber: number; body: string; prRepo?: GitHubPrRepoSlug | null } -): Promise { - const params: Record = { - number: args.prNumber, - body: args.body, - type: 'pr' - } - return sendGithubPrMutation( - client, - 'github.addIssueComment', - buildGithubPrParams('github.addIssueComment', worktreeId, params, { prRepo: args.prRepo }) - ) -} - -// Resolve/unresolve a review thread. `resolve` picks the direction (the host runs -// the matching GraphQL mutation). Unlike the comment mutations, the host returns a -// bare boolean, so a falsy result is a failure rather than the "no status" success. -export async function fetchResolveReviewThread( - client: Pick, - worktreeId: string, - args: { threadId: string; resolve: boolean; prRepo?: GitHubPrRepoSlug | null } -): Promise { - const response = await sendRaw( - client, - 'github.resolveReviewThread', - buildGithubPrParams( - 'github.resolveReviewThread', - worktreeId, - { threadId: args.threadId, resolve: args.resolve }, - { prRepo: args.prRepo } - ) - ) - if (!response.ok) { - return { - ok: false, - error: response.error || 'Request failed: github.resolveReviewThread' - } - } - // Why: the host returns a bare `true` on success; a missing/undefined result is - // not a confirmed success, so require an explicit `=== true` rather than `!== false`. - if (response.result !== true) { - return { ok: false, error: 'Failed to update review thread.' } - } - return { ok: true } -} - -// Edit a root conversation (issue) comment. The host RPC is slug-addressed -// (owner/repo/commentId), not worktree-addressed, so the params are passed -// directly rather than via buildGithubPrParams. Host returns the -// GitHubProjectMutationResult `{ ok }` envelope sendGithubPrMutation reads. -export async function fetchUpdateIssueComment( - client: Pick, - args: { owner: string; repo: string; host?: string; commentId: number; body: string } -): Promise { - return sendGithubPrMutation(client, 'github.project.updateIssueCommentBySlug', { - ...githubPrRepoSlugParam(args), - commentId: args.commentId, - body: args.body - }) -} - -// Delete a root conversation (issue) comment. Slug-addressed like the edit wrapper. -export async function fetchDeleteIssueComment( - client: Pick, - args: { owner: string; repo: string; host?: string; commentId: number } -): Promise { - return sendGithubPrMutation(client, 'github.project.deleteIssueCommentBySlug', { - ...githubPrRepoSlugParam(args), - commentId: args.commentId - }) -} - -export async function fetchRerunPRChecks( - client: Pick, +export function fetchRerunPRChecks( + client: MobileSessionRpcSender, worktreeId: string, args: { prNumber: number @@ -321,9 +164,12 @@ export async function fetchRerunPRChecks( if (args.headSha) { params.headSha = args.headSha } - return sendGithubPrMutation( - client, - 'github.rerunPRChecks', - buildGithubPrParams('github.rerunPRChecks', worktreeId, params, { prRepo: args.prRepo }) + return settleGithubPrMutation(githubPrChecksRerun, () => + githubPrChecksRerun.request( + client, + githubPrRequestParams(githubPrChecksRerun.operation.method, worktreeId, params, { + prRepo: args.prRepo + }) + ) ) } diff --git a/mobile/src/session/github-pr-parsers.ts b/mobile/src/session/github-pr-parsers.ts index 05a59200d1c..b1f8e4a7a0c 100644 --- a/mobile/src/session/github-pr-parsers.ts +++ b/mobile/src/session/github-pr-parsers.ts @@ -14,6 +14,10 @@ import type { GitHubWorkItem, GitHubWorkItemDetails } from '../../../src/shared/github/work-item-types' +import { + normalizeGitHubPRForBranchOutcome, + type GitHubPRForBranchResponse +} from '../../../src/shared/github/pull-request-for-branch-outcome' import { readPRComments } from './github-pr-comment-parsers' import type { HostedReviewInfo } from '../../../src/shared/hosted-review' import { @@ -109,6 +113,29 @@ export function readPRForBranch(value: unknown): PRInfo | null { } } +/** + * The branch lookup's whole answer, outcome classification included. + * + * Throws rather than degrading, twice: a host that could not reach GitHub answers in-band with + * `kind: 'upstream-error'` and the sidebar has always surfaced that message, and a reply whose PR + * body will not parse would otherwise render as "no pull request". + */ +export function readPRForBranchOutcome(value: unknown): PRInfo | null { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the normalizer discriminates on `kind` before reading anything else and treats every other shape as a legacy PRInfo, which readPRForBranch then validates. + const outcome = normalizeGitHubPRForBranchOutcome(value as GitHubPRForBranchResponse) + if (outcome.kind === 'upstream-error') { + throw new Error(outcome.message) + } + if (outcome.kind === 'no-pr') { + return null + } + const pr = readPRForBranch(outcome.pr) + if (!pr) { + throw new Error('GitHub returned an invalid pull request response.') + } + return pr +} + function readWorkItem(value: unknown): Omit | null { if (!isRecord(value)) { return null diff --git a/mobile/src/session/github-pr-read-operations.ts b/mobile/src/session/github-pr-read-operations.ts new file mode 100644 index 00000000000..c345d0fec4d --- /dev/null +++ b/mobile/src/session/github-pr-read-operations.ts @@ -0,0 +1,143 @@ +import type { PRCheckDetail, PRCheckRunDetails } from '../../../src/shared/github/check-types' +import type { GitHubAssignableUser, PRInfo } from '../../../src/shared/github/pull-request-types' +import type { GitHubWorkItemDetails } from '../../../src/shared/github/work-item-types' +import type { HostedReviewInfo } from '../../../src/shared/hosted-review' +import { bindDeferredRpcOperation, defineRpcOperation } from '../transport/rpc-operation' +import type { RpcCompatibleReader } from '../transport/rpc-operation-contract' +import { rpcPayloadMember, rpcReadUnchecked } from '../transport/rpc-reader-payload' +import type { GitHubPrRepoSlug } from './github-pr-repo-slug' +import { + readAssignableUsers, + readForBranch, + readPRCheckDetails, + readPRChecks, + readPRForBranchOutcome, + readWorkItemDetails +} from './github-pr-parsers' + +// The PR sidebar's reads. Every one of these replies was re-typed and hand-parsed at the wrapper; +// the readers below are now the only place that says what each payload is. They keep the defensive +// parsers unchanged, so a payload that used to degrade to null still degrades to null. +// +// All seven share one acceptance: a refused read is an error the sidebar shows, never a skip. The +// wrapper turns the throw back into its `{ ok: false, error }` outcome, which is the contract the +// sidebar's loaders route on. + +const repoSlugReader: RpcCompatibleReader = ( + raw +) => { + if (!raw || typeof raw !== 'object') { + return rpcReadUnchecked('pr-repo-slug', null) + } + const owner = rpcPayloadMember(raw, 'owner') + const repo = rpcPayloadMember(raw, 'repo') + const host = rpcPayloadMember(raw, 'host') + return rpcReadUnchecked( + 'pr-repo-slug', + typeof owner === 'string' && typeof repo === 'string' + ? { owner, repo, ...(typeof host === 'string' && host ? { host } : {}) } + : null + ) +} + +/** Whether the worktree's repo has a GitHub remote, which gates the dedicated PR-view icon. */ +export const githubPrRepoSlugRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'github.pr-repo-slug', + method: 'github.repoSlug', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: repoSlugReader + }) +) + +const hostedReviewInfoReader: RpcCompatibleReader< + unknown, + 'hosted-review-for-branch', + HostedReviewInfo | null +> = (raw) => rpcReadUnchecked('hosted-review-for-branch', readForBranch(raw)) + +export const hostedReviewBranchLookupRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'hostedReview.for-branch', + method: 'hostedReview.forBranch', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: hostedReviewInfoReader + }) +) + +/** The one reader here that throws rather than degrading, because main's parse did. */ +const prForBranchReader: RpcCompatibleReader = (raw) => + rpcReadUnchecked('pr-for-branch', readPRForBranchOutcome(raw)) + +export const githubPrForBranchRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'github.pr-for-branch', + method: 'github.prForBranch', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: prForBranchReader + }) +) + +const workItemDetailsReader: RpcCompatibleReader< + unknown, + 'pr-work-item-details', + GitHubWorkItemDetails | null +> = (raw) => rpcReadUnchecked('pr-work-item-details', readWorkItemDetails(raw)) + +export const githubPrWorkItemDetailsRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'github.pr-work-item-details', + method: 'github.workItemDetails', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: workItemDetailsReader + }) +) + +const prChecksReader: RpcCompatibleReader = (raw) => + rpcReadUnchecked('pr-checks', readPRChecks(raw)) + +export const githubPrChecksRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'github.pr-checks', + method: 'github.prChecks', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: prChecksReader + }) +) + +const prCheckDetailsReader: RpcCompatibleReader< + unknown, + 'pr-check-run-details', + PRCheckRunDetails | null +> = (raw) => rpcReadUnchecked('pr-check-run-details', readPRCheckDetails(raw)) + +export const githubPrCheckDetailsRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'github.pr-check-details', + method: 'github.prCheckDetails', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: prCheckDetailsReader + }) +) + +const assignableUsersReader: RpcCompatibleReader< + unknown, + 'pr-assignable-users', + GitHubAssignableUser[] +> = (raw) => rpcReadUnchecked('pr-assignable-users', readAssignableUsers(raw)) + +export const githubPrAssignableUsersRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'github.pr-assignable-users', + method: 'github.listAssignableUsers', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: assignableUsersReader + }) +) diff --git a/mobile/src/session/github-pr-repo-slug.ts b/mobile/src/session/github-pr-repo-slug.ts new file mode 100644 index 00000000000..b1d5eb6d876 --- /dev/null +++ b/mobile/src/session/github-pr-repo-slug.ts @@ -0,0 +1,84 @@ +import type { RpcMethodName, RpcSendParams } from '../transport/rpc-params-contract' +import { mobileRepoSelectorFromWorktreeId } from '../source-control/mobile-pr-create' + +// Why: a fork PR's head lives in a different owner/repo; the host's SlugRepo +// (`{ owner, repo }`) identifies it. Only a subset of github.* methods accept it. +// Why: `host` must survive the RPC boundary or GHES actions on the host fall +// back to a same-named github.com repo (src/shared/types.ts identity contract). +export type GitHubPrRepoSlug = { owner: string; repo: string; host?: string } + +export function githubPrRepoSlugParam(slug: GitHubPrRepoSlug): { + owner: string + repo: string + host?: string +} { + return { owner: slug.owner, repo: slug.repo, ...(slug.host ? { host: slug.host } : {}) } +} + +// Why: `prRepo` remains method-asymmetric. Keep the RPC schema allow-list here +// so fork/GHES identity reaches every PR-scoped read or mutation that accepts it. +const METHODS_ACCEPTING_PR_REPO = new Set([ + 'github.prChecks', + 'github.prCheckDetails', + 'github.rerunPRChecks', + 'github.resolveReviewThread', + 'github.setPRFileViewed', + 'github.updatePRState', + 'github.requestPRReviewers', + 'github.removePRReviewers', + 'github.mergePR', + 'github.setPRAutoMerge', + 'github.updatePRTitle', + 'github.prComments', + 'github.prFileContents', + 'github.addPRReviewComment', + 'github.addIssueComment', + 'github.addPRReviewCommentReply' +]) + +// Why: only github.prChecks declares a `headSha` param (PullRequestCheckDetails +// does not), so headSha is forwarded just to that read. Check runs are commit-keyed. +const METHODS_ACCEPTING_HEAD_SHA = new Set(['github.prChecks']) + +export type GitHubPrParamOptions = { + prRepo?: GitHubPrRepoSlug | null + headSha?: string | null +} + +export function buildGithubPrParams( + method: string, + worktreeId: string, + params: Record, + options?: GitHubPrParamOptions +): Record { + const built: Record = { + repo: mobileRepoSelectorFromWorktreeId(worktreeId), + ...params + } + if (options?.prRepo && METHODS_ACCEPTING_PR_REPO.has(method) && !('prRepo' in built)) { + built.prRepo = githubPrRepoSlugParam(options.prRepo) + } + if (options?.headSha && METHODS_ACCEPTING_HEAD_SHA.has(method) && !('headSha' in built)) { + built.headSha = options.headSha + } + return built +} + +/** + * The same record, presented as one method's send params — the single seam where the PR surface's + * record-shaped builder meets the typed operations. + * + * The builder cannot be typed per method: `repo` is prepended and `prRepo`/`headSha` appended from + * an allow-list, so the key order is a property of the builder rather than of any call site, and + * the wire payload is recorded as JSON with that order intact. Writing each call site's literal + * instead would reorder the bytes. One assertion here rather than one per wrapper. + */ +export function githubPrRequestParams( + method: Method, + worktreeId: string, + params: Record, + options?: GitHubPrParamOptions +): RpcSendParams { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the caller supplies the method's own declared fields; this adds only `repo`, and `prRepo`/`headSha` for the methods whose schema declares them. The sender recordings pin the bytes. + return buildGithubPrParams(method, worktreeId, params, options) as RpcSendParams +} diff --git a/mobile/src/session/github-pr-rpc.ts b/mobile/src/session/github-pr-rpc.ts index 485184993ec..a13a30ec5b7 100644 --- a/mobile/src/session/github-pr-rpc.ts +++ b/mobile/src/session/github-pr-rpc.ts @@ -2,24 +2,24 @@ import type { PRCheckDetail, PRCheckRunDetails } from '../../../src/shared/githu import type { GitHubAssignableUser, PRInfo } from '../../../src/shared/github/pull-request-types' import type { GitHubWorkItemDetails } from '../../../src/shared/github/work-item-types' import type { HostedReviewInfo } from '../../../src/shared/hosted-review' -import { - normalizeGitHubPRForBranchOutcome, - type GitHubPRForBranchResponse -} from '../../../src/shared/github/pull-request-for-branch-outcome' -import type { RpcClient } from '../transport/rpc-client' -import type { RpcSuccess } from '../transport/types' +import type { RpcMethodName } from '../transport/rpc-params-contract' +import { refusedRpcMessageOrFallback } from '../transport/rpc-refusal-message' +import type { RpcResponse } from '../transport/types' import { mobileRepoSelectorFromWorktreeId } from '../source-control/mobile-pr-create' import { - readAssignableUsers, - readForBranch, - readPRCheckDetails, - readPRChecks, - readPRForBranch, - readWorkItemDetails -} from './github-pr-parsers' + githubPrAssignableUsersRead, + githubPrCheckDetailsRead, + githubPrChecksRead, + githubPrForBranchRead, + githubPrRepoSlugRead, + githubPrWorkItemDetailsRead, + hostedReviewBranchLookupRead +} from './github-pr-read-operations' +import { githubPrRequestParams, type GitHubPrRepoSlug } from './github-pr-repo-slug' +import type { MobileSessionRpcSender } from './mobile-session-rpc-sender' -// Re-export the defensive parsers so consumers (and tests) have a single entry -// point for the github.* PR RPC surface. +// Re-export the defensive parsers and the PR-scoped param builder so consumers (and tests) have a +// single entry point for the github.* PR RPC surface. export { readAssignableUsers, readForBranch, @@ -28,193 +28,138 @@ export { readPRForBranch, readWorkItemDetails } from './github-pr-parsers' - -// Why: a fork PR's head lives in a different owner/repo; the host's SlugRepo -// (`{ owner, repo }`) identifies it. Only a subset of github.* methods accept it. -// Why: `host` must survive the RPC boundary or GHES actions on the host fall -// back to a same-named github.com repo (src/shared/types.ts identity contract). -export type GitHubPrRepoSlug = { owner: string; repo: string; host?: string } - -export function githubPrRepoSlugParam(slug: GitHubPrRepoSlug): Record { - return { owner: slug.owner, repo: slug.repo, ...(slug.host ? { host: slug.host } : {}) } -} +export { + buildGithubPrParams, + githubPrRepoSlugParam, + type GitHubPrRepoSlug +} from './github-pr-repo-slug' export type GitHubPrReadOutcome = { ok: true; result: T } | { ok: false; error: string } -// Why: `prRepo` remains method-asymmetric. Keep the RPC schema allow-list here -// so fork/GHES identity reaches every PR-scoped read or mutation that accepts it. -const METHODS_ACCEPTING_PR_REPO = new Set([ - 'github.prChecks', - 'github.prCheckDetails', - 'github.rerunPRChecks', - 'github.resolveReviewThread', - 'github.setPRFileViewed', - 'github.updatePRState', - 'github.requestPRReviewers', - 'github.removePRReviewers', - 'github.mergePR', - 'github.setPRAutoMerge', - 'github.updatePRTitle', - 'github.prComments', - 'github.prFileContents', - 'github.addPRReviewComment', - 'github.addIssueComment', - 'github.addPRReviewCommentReply' -]) - -// Why: only github.prChecks declares a `headSha` param (PullRequestCheckDetails -// does not), so headSha is forwarded just to that read. Check runs are commit-keyed. -const METHODS_ACCEPTING_HEAD_SHA = new Set(['github.prChecks']) - -export function buildGithubPrParams( - method: string, - worktreeId: string, - params: Record, - options?: { prRepo?: GitHubPrRepoSlug | null; headSha?: string | null } -): Record { - const built: Record = { - repo: mobileRepoSelectorFromWorktreeId(worktreeId), - ...params - } - if (options?.prRepo && METHODS_ACCEPTING_PR_REPO.has(method) && !('prRepo' in built)) { - built.prRepo = githubPrRepoSlugParam(options.prRepo) - } - if (options?.headSha && METHODS_ACCEPTING_HEAD_SHA.has(method) && !('headSha' in built)) { - built.headSha = options.headSha - } - return built +/** As much of a bound read operation as the settle shape below needs. */ +type GitHubPrReadOperation = { + readonly operation: { readonly method: RpcMethodName } + readonly interpret: (reply: RpcResponse) => Value } -async function sendGithubPrRead( - client: Pick, - method: string, - params: Record, - parse: (value: unknown) => T -): Promise> { +/** + * Two failure texts main kept apart, and one it shared. + * + * A refusal with no message falls back to the method's own copy, because that is what + * `response.error?.message || ...` did. A reader that threw — the host reporting an upstream error + * in-band, or a PR body that would not parse — surfaces its own text verbatim, because that threw + * into the same catch a transport drop did. + */ +function githubPrFailureText(reply: RpcResponse, error: unknown, fallback: string): string { + if (!reply.ok) { + return refusedRpcMessageOrFallback(error, fallback) + } + return error instanceof Error ? error.message : fallback +} + +async function settleGithubPrRead( + read: GitHubPrReadOperation, + send: () => Promise +): Promise> { + const fallback = `Request failed: ${read.operation.method}` + let reply: RpcResponse try { - const response = await client.sendRequest(method, params) - if (!response.ok) { - return { ok: false, error: response.error?.message || `Request failed: ${method}` } - } - return { ok: true, result: parse((response as RpcSuccess).result) } - } catch (err) { - // Why: a transport drop or a parser throw must not escape as an unhandled - // rejection — normalize to the `{ ok:false, error }` contract callers expect. - return { ok: false, error: err instanceof Error ? err.message : `Request failed: ${method}` } + reply = await send() + } catch (error) { + // A transport drop surfaces its own message verbatim, empty included. + return { ok: false, error: error instanceof Error ? error.message : fallback } + } + try { + return { ok: true, result: read.interpret(reply) } + } catch (error) { + return { ok: false, error: githubPrFailureText(reply, error, fallback) } } } // Probes whether the worktree's repo has a GitHub remote (a non-null slug). Used // to decide whether the dedicated PR-view icon is available — independent of // whether the branch has an open PR. -export async function fetchGithubRepoSlug( - client: Pick, +export function fetchGithubRepoSlug( + client: MobileSessionRpcSender, worktreeId: string ): Promise> { - return sendGithubPrRead( - client, - 'github.repoSlug', - buildGithubPrParams('github.repoSlug', worktreeId, {}), - (value) => { - if (!value || typeof value !== 'object') { - return null - } - const record = value as Record - const owner = record.owner - const repo = record.repo - const host = record.host - return typeof owner === 'string' && typeof repo === 'string' - ? { owner, repo, ...(typeof host === 'string' && host ? { host } : {}) } - : null - } + return settleGithubPrRead(githubPrRepoSlugRead, () => + githubPrRepoSlugRead.request( + client, + githubPrRequestParams(githubPrRepoSlugRead.operation.method, worktreeId, {}) + ) ) } -export async function fetchHostedReviewForBranch( - client: Pick, +export function fetchHostedReviewForBranch( + client: MobileSessionRpcSender, worktreeId: string, args: { branch: string; linkedGitHubPR?: number | null } ): Promise> { - return sendGithubPrRead( - client, - 'hostedReview.forBranch', - { + return settleGithubPrRead(hostedReviewBranchLookupRead, () => + hostedReviewBranchLookupRead.request(client, { repo: mobileRepoSelectorFromWorktreeId(worktreeId), branch: args.branch, linkedGitHubPR: args.linkedGitHubPR ?? null, // Why: the mobile PR sidebar is only ever open on the selected worktree, // so it belongs in the host's fast re-check tier (#11532). active: true - }, - readForBranch + }) ) } -export async function fetchPRForBranch( - client: Pick, +export function fetchPRForBranch( + client: MobileSessionRpcSender, worktreeId: string, args: { branch: string; linkedPRNumber?: number | null } ): Promise> { - return sendGithubPrRead( - client, - 'github.prForBranch', - buildGithubPrParams('github.prForBranch', worktreeId, { - branch: args.branch, - linkedPRNumber: args.linkedPRNumber ?? null - }), - (value) => { - const outcome = normalizeGitHubPRForBranchOutcome(value as GitHubPRForBranchResponse) - if (outcome.kind === 'upstream-error') { - throw new Error(outcome.message) - } - if (outcome.kind === 'no-pr') { - return null - } - const pr = readPRForBranch(outcome.pr) - if (!pr) { - throw new Error('GitHub returned an invalid pull request response.') - } - return pr - } + return settleGithubPrRead(githubPrForBranchRead, () => + githubPrForBranchRead.request( + client, + githubPrRequestParams(githubPrForBranchRead.operation.method, worktreeId, { + branch: args.branch, + linkedPRNumber: args.linkedPRNumber ?? null + }) + ) ) } -export async function fetchWorkItemDetails( - client: Pick, +export function fetchWorkItemDetails( + client: MobileSessionRpcSender, worktreeId: string, args: { prNumber: number } ): Promise> { - return sendGithubPrRead( - client, - 'github.workItemDetails', - buildGithubPrParams('github.workItemDetails', worktreeId, { - number: args.prNumber, - type: 'pr' - }), - readWorkItemDetails + return settleGithubPrRead(githubPrWorkItemDetailsRead, () => + githubPrWorkItemDetailsRead.request( + client, + githubPrRequestParams(githubPrWorkItemDetailsRead.operation.method, worktreeId, { + number: args.prNumber, + type: 'pr' + }) + ) ) } -export async function fetchPRChecks( - client: Pick, +export function fetchPRChecks( + client: MobileSessionRpcSender, worktreeId: string, args: { prNumber: number; headSha?: string | null; prRepo?: GitHubPrRepoSlug | null } ): Promise> { - return sendGithubPrRead( - client, - 'github.prChecks', - buildGithubPrParams( - 'github.prChecks', - worktreeId, - { prNumber: args.prNumber }, - { prRepo: args.prRepo, headSha: args.headSha } - ), - readPRChecks + return settleGithubPrRead(githubPrChecksRead, () => + githubPrChecksRead.request( + client, + githubPrRequestParams( + githubPrChecksRead.operation.method, + worktreeId, + { prNumber: args.prNumber }, + { prRepo: args.prRepo, headSha: args.headSha } + ) + ) ) } -export async function fetchPRCheckDetails( - client: Pick, +export function fetchPRCheckDetails( + client: MobileSessionRpcSender, worktreeId: string, args: { checkRunId?: number @@ -237,22 +182,24 @@ export async function fetchPRCheckDetails( if (args.url !== undefined) { params.url = args.url } - return sendGithubPrRead( - client, - 'github.prCheckDetails', - buildGithubPrParams('github.prCheckDetails', worktreeId, params, { prRepo: args.prRepo }), - readPRCheckDetails + return settleGithubPrRead(githubPrCheckDetailsRead, () => + githubPrCheckDetailsRead.request( + client, + githubPrRequestParams(githubPrCheckDetailsRead.operation.method, worktreeId, params, { + prRepo: args.prRepo + }) + ) ) } -export async function fetchAssignableUsers( - client: Pick, +export function fetchAssignableUsers( + client: MobileSessionRpcSender, worktreeId: string ): Promise> { - return sendGithubPrRead( - client, - 'github.listAssignableUsers', - buildGithubPrParams('github.listAssignableUsers', worktreeId, {}), - readAssignableUsers + return settleGithubPrRead(githubPrAssignableUsersRead, () => + githubPrAssignableUsersRead.request( + client, + githubPrRequestParams(githubPrAssignableUsersRead.operation.method, worktreeId, {}) + ) ) } diff --git a/mobile/src/session/mobile-diff-review-loaders.ts b/mobile/src/session/mobile-diff-review-loaders.ts index 0b1a14d156b..636275af68a 100644 --- a/mobile/src/session/mobile-diff-review-loaders.ts +++ b/mobile/src/session/mobile-diff-review-loaders.ts @@ -8,18 +8,22 @@ import { normalizeMobileDiffComments } from './mobile-diff-comments' import { buildMobileDiffHunks } from './mobile-diff-hunks' import { highlightMobileDiffLines, resolveMobileSyntaxLanguage } from './mobile-file-syntax' import { - readMobileBranchCompareResult, - readMobileGitStatusResult, - readMobileReviewGitDiffResult, - readMobileReviewWorktreeMetadata -} from './mobile-diff-review-rpc' + reviewBranchCompareRead, + reviewBranchFileDiffRead, + reviewFileDiffRead, + reviewWorktreeMetadataRead +} from './mobile-diff-review-operations' +import type { MobileReviewGitDiffResult } from './mobile-diff-review-rpc' import { canOpenMobileBranchCompareDiff, type MobileGitBranchCompareResult } from '../source-control/mobile-branch-compare' import { resolveMobileBranchCompareBaseRef } from '../source-control/mobile-branch-base-ref' +import { gitStatusProjectionRead } from '../source-control/mobile-git-read-operations' import { isMobileGitUnavailable } from '../source-control/mobile-git-status' +import { refusedRpcMessageOrFallback } from '../transport/rpc-refusal-message' import type { RpcClient } from '../transport/rpc-client' +import type { RpcResponse } from '../transport/types' import type { MobileDiffReviewQueueItem } from './mobile-diff-review-queue' import type { ReviewDiffState, ReviewScreenState } from './mobile-diff-review-screen-model' import { reviewDescriptorFromItem } from './mobile-diff-review-screen-model' @@ -36,6 +40,12 @@ type DiffLoadInput = { branchCompare: MobileGitBranchCompareResult | null } +/** One settled file diff and the operation that reads it; the two methods share a reader. */ +type PendingFileDiff = { + reply: RpcResponse + interpret: (reply: RpcResponse) => MobileReviewGitDiffResult | null +} + export async function loadMobileDiffReviewBranchCompare( client: RpcClient, worktreeId: string @@ -45,21 +55,29 @@ export async function loadMobileDiffReviewBranchCompare( if (!baseRef) { return { result: null } } - const response = await client.sendRequest('git.branchCompare', { + const reply = await reviewBranchCompareRead.request(client, { worktree: `id:${worktreeId}`, baseRef }) - if (!response.ok) { - if (isMobileGitUnavailable(response.error?.code, response.error?.message)) { - return { result: null } + // Why the raw refusal: a host that does not offer git to mobile is a capability gap this + // screen degrades on, and no acceptance policy carries the code and message through. + if (!reply.ok && isMobileGitUnavailable(reply.error?.code, reply.error?.message)) { + return { result: null } + } + let parsed: MobileGitBranchCompareResult | null + try { + parsed = reviewBranchCompareRead.interpret(reply) + } catch (error) { + return { + result: null, + error: refusedRpcMessageOrFallback(error, 'Committed changes unavailable') } - return { result: null, error: response.error?.message || 'Committed changes unavailable' } } - const parsed = readMobileBranchCompareResult(response.result) return parsed ? { result: parsed } : { result: null, error: 'Committed changes response was invalid' } } catch (err) { + // A transport drop surfaces its own message verbatim; only a refusal falls back above. return { result: null, error: err instanceof Error ? err.message : 'Committed changes failed' } } } @@ -68,27 +86,38 @@ export async function loadMobileDiffReviewSnapshot( client: RpcClient, worktreeId: string ): Promise { - const statusResponse = await client.sendRequest('git.status', { worktree: `id:${worktreeId}` }) - if (!statusResponse.ok) { - if (isMobileGitUnavailable(statusResponse.error?.code, statusResponse.error?.message)) { - return { kind: 'unavailable', message: 'Update Orca desktop to review changes on mobile.' } - } - throw new Error(statusResponse.error?.message || 'Unable to load changes') + const statusReply = await gitStatusProjectionRead.request(client, { + worktree: `id:${worktreeId}` + }) + if ( + !statusReply.ok && + isMobileGitUnavailable(statusReply.error?.code, statusReply.error?.message) + ) { + return { kind: 'unavailable', message: 'Update Orca desktop to review changes on mobile.' } + } + let status + try { + status = gitStatusProjectionRead.interpret(statusReply) + } catch (error) { + throw new Error(refusedRpcMessageOrFallback(error, 'Unable to load changes')) } - const status = readMobileGitStatusResult(statusResponse.result) if (!status) { throw new Error('Source control response was invalid') } - const [branch, worktreeResponse] = await Promise.all([ + // Both legs are interpreted after the barrier, not as each lands: a refused worktree.show must + // not decide the error before the compare leg has had its own chance to fail. + const [branch, worktreeReply] = await Promise.all([ loadMobileDiffReviewBranchCompare(client, worktreeId), - client.sendRequest('worktree.show', { worktree: `id:${worktreeId}` }) + reviewWorktreeMetadataRead.request(client, { worktree: `id:${worktreeId}` }) ]) - if (!worktreeResponse.ok) { - throw new Error(worktreeResponse.error?.message || 'Unable to load review notes') + let metadata + try { + metadata = reviewWorktreeMetadataRead.interpret(worktreeReply) + } catch (error) { + throw new Error(refusedRpcMessageOrFallback(error, 'Unable to load review notes')) } - const metadata = readMobileReviewWorktreeMetadata(worktreeResponse.result) const comments = normalizeMobileDiffComments(metadata.diffComments, worktreeId) const normalizedReviewState = normalizeMobileDiffReviewState(metadata.mobileDiffReview) const branchEntries = @@ -121,24 +150,26 @@ export async function loadMobileDiffReviewSnapshot( export async function loadMobileDiffReviewDiff(input: DiffLoadInput): Promise { const { client, worktreeId, item, branchCompare } = input - const response = + const pending = item.scope === 'branch' - ? await loadBranchFileDiff(client, worktreeId, item, branchCompare) - : await client.sendRequest('git.diff', { - worktree: `id:${worktreeId}`, - filePath: item.filePath, - staged: item.scope === 'staged' - }) - if (!response.ok) { - if (response.error?.code === 'diff_too_large') { + ? await requestBranchFileDiff(client, worktreeId, item, branchCompare) + : await requestWorktreeFileDiff(client, worktreeId, item) + if (!pending.reply.ok) { + // Why the raw refusal: `diff_too_large` is a render mode rather than a failure, and no + // acceptance policy carries the code through. + if (pending.reply.error?.code === 'diff_too_large') { return { kind: 'too-large', itemKey: item.key } } if (item.status === 'deleted') { return { kind: 'deleted', itemKey: item.key } } - throw new Error(response.error?.message || 'Unable to load diff') } - const result = readMobileReviewGitDiffResult(response.result) + let result: MobileReviewGitDiffResult | null + try { + result = pending.interpret(pending.reply) + } catch (error) { + throw new Error(refusedRpcMessageOrFallback(error, 'Unable to load diff')) + } if (!result) { throw new Error('Diff response was invalid') } @@ -159,17 +190,30 @@ export async function loadMobileDiffReviewDiff(input: DiffLoadInput): Promise { + const reply = await reviewFileDiffRead.request(client, { + worktree: `id:${worktreeId}`, + filePath: item.filePath, + staged: item.scope === 'staged' + }) + return { reply, interpret: (settled) => reviewFileDiffRead.interpret(settled) } +} + +async function requestBranchFileDiff( client: RpcClient, worktreeId: string, item: MobileDiffReviewQueueItem, branchCompare: MobileGitBranchCompareResult | null -) { +): Promise { const summary = branchCompare?.summary if (!summary || !summary.headOid || !summary.mergeBase) { throw new Error('Committed diff is unavailable') } - return client.sendRequest('git.branchDiff', { + const reply = await reviewBranchFileDiffRead.request(client, { worktree: `id:${worktreeId}`, filePath: item.filePath, ...(item.oldPath ? { oldPath: item.oldPath } : {}), @@ -180,4 +224,5 @@ async function loadBranchFileDiff( mergeBase: summary.mergeBase } }) + return { reply, interpret: (settled) => reviewBranchFileDiffRead.interpret(settled) } } diff --git a/mobile/src/session/mobile-diff-review-operations.ts b/mobile/src/session/mobile-diff-review-operations.ts new file mode 100644 index 00000000000..1427865acd6 --- /dev/null +++ b/mobile/src/session/mobile-diff-review-operations.ts @@ -0,0 +1,137 @@ +import { bindDeferredRpcOperation, defineRpcOperation } from '../transport/rpc-operation' +import type { RpcCompatibleReader } from '../transport/rpc-operation-contract' +import { rpcReadUnchecked } from '../transport/rpc-reader-payload' +import type { MobileGitBranchCompareResult } from '../source-control/mobile-branch-compare' +import type { MobileGitStatusResult } from '../source-control/mobile-git-status' +import { + readMobileBranchCompareResult, + readMobileGitStatusResult, + readMobileReviewGitDiffResult, + readMobileReviewWorktreeMetadata, + type MobileReviewGitDiffResult, + type MobileReviewWorktreeMetadata +} from './mobile-diff-review-rpc' + +// What the review screen and the PR branch-context loader read. Both work from the same three +// projections — normalized status, normalized branch compare, the review notes on the worktree — +// and neither reads a raw host payload. + +const statusProjectionReader: RpcCompatibleReader< + unknown, + 'normalized-status', + MobileGitStatusResult | null +> = (raw) => rpcReadUnchecked('normalized-status', readMobileGitStatusResult(raw)) + +/** + * git.status read for the PR branch context. The third policy on this method, and the only one that + * skips: the standalone PR entry point derives branch and head SHA from status and falls back to + * branchCompare's headOid, so a refused status leaves it with no branch rather than an error to + * show. The review screen's read (`gitStatusProjectionRead`) must surface the message instead, + * because the screen has nothing to render without it. One reader serves both — the projection is + * the same, only what a refusal means differs. + */ +export const branchContextStatusRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'git.branch-context-status-or-skip', + method: 'git.status', + acceptance: 'success-result-or-skip', + barrier: 'after-caller-barrier', + read: statusProjectionReader + }) +) + +const branchCompareProjectionReader: RpcCompatibleReader< + unknown, + 'normalized-branch-compare', + MobileGitBranchCompareResult | null +> = (raw) => rpcReadUnchecked('normalized-branch-compare', readMobileBranchCompareResult(raw)) + +/** + * git.branchCompare, second reader on the method. The Changes screen publishes the host payload + * verbatim through `gitBranchCompareRead`; this one normalizes. The projection is not a superset — + * it answers null when `summary` or `entries` is not the expected shape, or when `baseRef`, + * `compareRef` or `changedFiles` is missing — and review and PR context both depend on that null to + * report "committed changes response was invalid" rather than rendering a partial compare. Sharing + * the verbatim reader would hand them a payload they would then have to re-parse. + */ +export const reviewBranchCompareRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'git.review-branch-compare', + method: 'git.branchCompare', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: branchCompareProjectionReader + }) +) + +/** The same projection, read where a refused compare only costs the head-SHA fallback. */ +export const branchContextCompareRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'git.branch-context-compare-or-skip', + method: 'git.branchCompare', + acceptance: 'success-result-or-skip', + barrier: 'after-caller-barrier', + read: branchCompareProjectionReader + }) +) + +const reviewMetadataReader: RpcCompatibleReader< + unknown, + 'review-worktree-metadata', + MobileReviewWorktreeMetadata +> = (raw) => rpcReadUnchecked('review-worktree-metadata', readMobileReviewWorktreeMetadata(raw)) + +/** + * worktree.show, second reader on the method. `worktreeSummaryRead` projects `{ baseRef, linkedPR }` + * and drops everything else, so it would answer the review screen with no notes at all for every + * reply. The two are read side by side in one snapshot — branch-base resolution asks for the + * summary while the screen asks for the notes — which is why neither can be widened into the other + * without changing what the other sees. + */ +export const reviewWorktreeMetadataRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'worktree.review-metadata', + method: 'worktree.show', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: reviewMetadataReader + }) +) + +const reviewDiffReader: RpcCompatibleReader< + unknown, + 'review-file-diff', + MobileReviewGitDiffResult | null +> = (raw) => rpcReadUnchecked('review-file-diff', readMobileReviewGitDiffResult(raw)) + +/** + * The worktree file diff. Its refusal carries meaning the acceptance policy cannot: `diff_too_large` + * is a render mode, not a failure, so the caller reads that code off the raw reply before it + * interprets — the same raw-refusal read `use-mobile-source-control-loaders.ts` makes for the + * mobile-git capability gap. + */ +export const reviewFileDiffRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'git.review-file-diff', + method: 'git.diff', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: reviewDiffReader + }) +) + +/** + * The committed-range equivalent, second reader on git.branchDiff. `gitBranchDiffRead` hands the + * Changes screen's branch preview the host payload verbatim; review needs the + * text/binary/too-large discrimination, and a reply that matches none of the three has to read as + * null so the screen says the diff was invalid instead of rendering an empty file. + */ +export const reviewBranchFileDiffRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'git.review-branch-file-diff', + method: 'git.branchDiff', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: reviewDiffReader + }) +) diff --git a/mobile/src/session/mobile-review-terminal-operations.ts b/mobile/src/session/mobile-review-terminal-operations.ts new file mode 100644 index 00000000000..e3a9bdf425c --- /dev/null +++ b/mobile/src/session/mobile-review-terminal-operations.ts @@ -0,0 +1,53 @@ +import { bindDeferredRpcOperation, defineRpcOperation } from '../transport/rpc-operation' +import type { RpcCompatibleReader } from '../transport/rpc-operation-contract' +import { rpcReadUnchecked } from '../transport/rpc-reader-payload' +import { + readMobileReviewCreatedTerminal, + readMobileReviewTerminalSendAccepted, + type MobileReviewTerminalTab +} from './mobile-diff-review-rpc' + +// Dropping a prompt into a fresh agent terminal: create the tab, then send the text. There is no +// higher-level agent-composer RPC on mobile, so this pair is the launch mechanism — the PR triage +// actions and the review-notes send sheet both drive it. + +const createdTerminalReader: RpcCompatibleReader< + unknown, + 'created-terminal-tab', + MobileReviewTerminalTab | null +> = (raw) => rpcReadUnchecked('created-terminal-tab', readMobileReviewCreatedTerminal(raw)) + +/** + * A refused create is an error the caller surfaces: there is nowhere to put the prompt. The reply + * is read for the terminal handle the send below is addressed to, so an unreadable tab is a failure + * even though the envelope was accepted. + */ +export const reviewTerminalCreateRun = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'session.create-review-terminal', + method: 'session.tabs.createTerminal', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: createdTerminalReader + }) +) + +/** + * An accepted send can still report in-band that the terminal is locked, which is a different + * failure from a refused send and the caller says so. The reader answers that one question. + */ +const terminalSendAcceptedReader: RpcCompatibleReader< + unknown, + 'terminal-send-accepted', + boolean +> = (raw) => rpcReadUnchecked('terminal-send-accepted', readMobileReviewTerminalSendAccepted(raw)) + +export const reviewTerminalSendRun = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'terminal.send-review-prompt', + method: 'terminal.send', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: terminalSendAcceptedReader + }) +) diff --git a/mobile/src/session/mobile-session-rpc-sender.ts b/mobile/src/session/mobile-session-rpc-sender.ts new file mode 100644 index 00000000000..1c5795b5b7a --- /dev/null +++ b/mobile/src/session/mobile-session-rpc-sender.ts @@ -0,0 +1,10 @@ +import { githubPrRepoSlugRead } from './github-pr-read-operations' + +/** + * What a session-screen operation needs to send with. + * + * Derived from an operation rather than restated, so accepting a client does not require a module + * to name the raw request port. It stays exactly as narrow as the `Pick` + * it replaces — widening it to `RpcClient` would make every unit test build a whole client. + */ +export type MobileSessionRpcSender = Parameters[0] diff --git a/mobile/src/session/pr-ai-triage-launch.ts b/mobile/src/session/pr-ai-triage-launch.ts index 6d6c0bbc8b2..6a5cf9ac155 100644 --- a/mobile/src/session/pr-ai-triage-launch.ts +++ b/mobile/src/session/pr-ai-triage-launch.ts @@ -1,42 +1,46 @@ -import type { RpcClient } from '../transport/rpc-client' -import { - readMobileReviewCreatedTerminal, - readMobileReviewTerminalSendAccepted -} from './mobile-diff-review-rpc' +import { refusedRpcMessageOrFallback } from '../transport/rpc-refusal-message' +import { reviewTerminalCreateRun, reviewTerminalSendRun } from './mobile-review-terminal-operations' +import type { MobileSessionRpcSender } from './mobile-session-rpc-sender' // Pure launch path for the PR triage actions ("Fix checks with AI" / "Resolve // conflicts with AI"). Reuses the same two RPCs the diff-review send flow uses — // session.tabs.createTerminal then terminal.send — so the prompt is dropped into a -// fresh agent terminal in the worktree. There is no higher-level agent-composer RPC -// on mobile, so this createTerminal+send pair is the launch mechanism. Kept free of -// react-native imports so it stays unit-testable in the node test environment. +// fresh agent terminal in the worktree. Kept free of react-native imports so it +// stays unit-testable in the node test environment. export async function createTerminalAndSendPrompt( - client: Pick, + client: MobileSessionRpcSender, worktreeId: string, prompt: string ): Promise { - const created = await client.sendRequest('session.tabs.createTerminal', { + // Each request is awaited outside its catch so a transport drop propagates as the original + // error object; only a refusal is rewritten into the step's own copy. + const createdReply = await reviewTerminalCreateRun.request(client, { worktree: `id:${worktreeId}`, activate: false, select: true, navigation: 'caller' }) - if (!created.ok) { - throw new Error(created.error?.message || 'Failed to create terminal') + let terminalTab + try { + terminalTab = reviewTerminalCreateRun.interpret(createdReply) + } catch (error) { + throw new Error(refusedRpcMessageOrFallback(error, 'Failed to create terminal')) } - const terminalTab = readMobileReviewCreatedTerminal(created.result) if (!terminalTab) { throw new Error('Created terminal response was invalid') } - const sent = await client.sendRequest('terminal.send', { + const sentReply = await reviewTerminalSendRun.request(client, { terminal: terminalTab.terminal, text: prompt, enter: true }) - if (!sent.ok) { - throw new Error(sent.error?.message || 'Failed to send prompt') + let accepted + try { + accepted = reviewTerminalSendRun.interpret(sentReply) + } catch (error) { + throw new Error(refusedRpcMessageOrFallback(error, 'Failed to send prompt')) } - if (!readMobileReviewTerminalSendAccepted(sent.result)) { + if (!accepted) { throw new Error('Terminal input is locked') } } diff --git a/mobile/src/session/use-mobile-pr-actions.ts b/mobile/src/session/use-mobile-pr-actions.ts index 51c06e529c0..710360c8554 100644 --- a/mobile/src/session/use-mobile-pr-actions.ts +++ b/mobile/src/session/use-mobile-pr-actions.ts @@ -10,6 +10,7 @@ import { fetchUpdatePRState } from './github-pr-mutations' import type { GitHubPrRepoSlug } from './github-pr-rpc' +import type { MobileSessionRpcSender } from './mobile-session-rpc-sender' import { PrActionsEngine, type PrActionMutations, type PrActionBusyKey } from './pr-actions-engine' export type { PrActionBusyKey, PrActionMutations } from './pr-actions-engine' @@ -26,10 +27,7 @@ export type PrActionsInput = { mutations?: PrActionMutations } -function realMutations( - client: Pick, - worktreeId: string -): PrActionMutations { +function realMutations(client: MobileSessionRpcSender, worktreeId: string): PrActionMutations { return { mergePR: (args) => fetchMergePR(client, worktreeId, args), setPRAutoMerge: (args) => fetchSetPRAutoMerge(client, worktreeId, args), diff --git a/mobile/src/session/use-mobile-pr-branch-context.ts b/mobile/src/session/use-mobile-pr-branch-context.ts index d0816b02f71..7c67ded6ecf 100644 --- a/mobile/src/session/use-mobile-pr-branch-context.ts +++ b/mobile/src/session/use-mobile-pr-branch-context.ts @@ -5,7 +5,7 @@ import type { MobileGitBranchCompareResult } from '../source-control/mobile-bran import type { MobileGitStatusResult } from '../source-control/mobile-git-status' import { resolveMobileBranchCompareBaseRef } from '../source-control/mobile-branch-base-ref' import { fetchGithubRepoSlug } from './github-pr-rpc' -import { readMobileBranchCompareResult, readMobileGitStatusResult } from './mobile-diff-review-rpc' +import { branchContextCompareRead, branchContextStatusRead } from './mobile-diff-review-operations' export type MobilePrBranchContext = { branch: string | null @@ -173,8 +173,9 @@ async function readGitStatus( client: RpcClient, worktreeId: string ): Promise { - const response = await client.sendRequest('git.status', { worktree: `id:${worktreeId}` }) - return response.ok ? readMobileGitStatusResult(response.result) : null + const reply = await branchContextStatusRead.request(client, { worktree: `id:${worktreeId}` }) + const status = branchContextStatusRead.interpret(reply) + return status.accepted ? status.value : null } async function readBranchCompare( @@ -187,9 +188,10 @@ async function readBranchCompare( if (!baseRef) { return null } - const response = await client.sendRequest('git.branchCompare', { + const reply = await branchContextCompareRead.request(client, { worktree: `id:${worktreeId}`, baseRef }) - return response.ok ? readMobileBranchCompareResult(response.result) : null + const compared = branchContextCompareRead.interpret(reply) + return compared.accepted ? compared.value : null } diff --git a/mobile/src/session/use-mobile-pr-comment-actions.ts b/mobile/src/session/use-mobile-pr-comment-actions.ts index 187dc32dbdb..e5be0c7d8f5 100644 --- a/mobile/src/session/use-mobile-pr-comment-actions.ts +++ b/mobile/src/session/use-mobile-pr-comment-actions.ts @@ -3,6 +3,7 @@ import type { PRComment } from '../../../src/shared/github/comment-types' import type { ConnectionState } from '../transport/types' import type { RpcClient } from '../transport/rpc-client' import type { GitHubPrRepoSlug } from './github-pr-rpc' +import type { MobileSessionRpcSender } from './mobile-session-rpc-sender' import { fetchAddIssueComment, fetchAddPRReviewCommentReply, @@ -69,10 +70,7 @@ export type PrCommentActionsInput = { mutations?: PrCommentMutations } -function realMutations( - client: Pick, - worktreeId: string -): PrCommentMutations { +function realMutations(client: MobileSessionRpcSender, worktreeId: string): PrCommentMutations { return { reply: (args) => fetchAddPRReviewCommentReply(client, worktreeId, args), resolveThread: (args) => fetchResolveReviewThread(client, worktreeId, args), diff --git a/mobile/src/session/use-mobile-pr-title-action.ts b/mobile/src/session/use-mobile-pr-title-action.ts index bcf8d8ea96e..b33c87cecbf 100644 --- a/mobile/src/session/use-mobile-pr-title-action.ts +++ b/mobile/src/session/use-mobile-pr-title-action.ts @@ -2,6 +2,7 @@ import { useCallback, useMemo, useRef, useState } from 'react' import type { ConnectionState } from '../transport/types' import type { RpcClient } from '../transport/rpc-client' import type { GitHubPrRepoSlug } from './github-pr-rpc' +import type { MobileSessionRpcSender } from './mobile-session-rpc-sender' import { fetchUpdatePRTitle, type GitHubPrMutationOutcome } from './github-pr-mutations' import { triggerError, triggerSuccess } from '../platform/haptics' import { buildUpdatePRTitleParams } from './pr-title-edit' @@ -27,10 +28,7 @@ export type PrTitleActionInput = { mutations?: PrTitleMutations } -function realMutations( - client: Pick, - worktreeId: string -): PrTitleMutations { +function realMutations(client: MobileSessionRpcSender, worktreeId: string): PrTitleMutations { return { updateTitle: (args) => fetchUpdatePRTitle(client, worktreeId, args) } diff --git a/mobile/src/transport/unvalidated-rpc-request-port-inventory.ts b/mobile/src/transport/unvalidated-rpc-request-port-inventory.ts index 3e09655c5a1..85f580e3796 100644 --- a/mobile/src/transport/unvalidated-rpc-request-port-inventory.ts +++ b/mobile/src/transport/unvalidated-rpc-request-port-inventory.ts @@ -100,10 +100,7 @@ export const UNVALIDATED_RPC_REQUEST_PORT_PENDING: readonly UnvalidatedRpcReques // src/session/ — session screen: chat, diff review, PR actions, tabs { file: 'src/session/ai-vault-resume-launch.ts', references: 3 }, { file: 'src/session/ai-vault-resume-preparation.ts', references: 2 }, - { file: 'src/session/github-pr-mutations.ts', references: 16 }, - { file: 'src/session/github-pr-rpc.ts', references: 9 }, { file: 'src/session/mobile-clipboard-image.ts', references: 7 }, - { file: 'src/session/mobile-diff-review-loaders.ts', references: 5 }, { file: 'src/session/mobile-file-tap-open.ts', references: 3 }, { file: 'src/session/mobile-image-attachment.ts', references: 2 }, { file: 'src/session/mobile-native-chat-image-attachment.ts', references: 1 }, @@ -116,7 +113,6 @@ export const UNVALIDATED_RPC_REQUEST_PORT_PENDING: readonly UnvalidatedRpcReques { file: 'src/session/mobile-session-tabs-stream-health.ts', references: 1 }, { file: 'src/session/mobile-structured-agent-session-launch.ts', references: 3 }, { file: 'src/session/mobile-structured-agent-session-rpc.ts', references: 1 }, - { file: 'src/session/pr-ai-triage-launch.ts', references: 3 }, { file: 'src/session/use-live-worktree-name.ts', references: 1 }, { file: 'src/session/use-mobile-diff-review-comment-actions.ts', references: 1 }, { file: 'src/session/use-mobile-diff-review-git-actions.ts', references: 2 }, @@ -127,10 +123,6 @@ export const UNVALIDATED_RPC_REQUEST_PORT_PENDING: readonly UnvalidatedRpcReques { file: 'src/session/use-mobile-native-chat-readability.ts', references: 1 }, { file: 'src/session/use-mobile-native-chat-session.ts', references: 1 }, { file: 'src/session/use-mobile-native-chat-stop.ts', references: 1 }, - { file: 'src/session/use-mobile-pr-actions.ts', references: 1 }, - { file: 'src/session/use-mobile-pr-branch-context.ts', references: 2 }, - { file: 'src/session/use-mobile-pr-comment-actions.ts', references: 1 }, - { file: 'src/session/use-mobile-pr-title-action.ts', references: 1 }, { file: 'src/session/use-mobile-session-accessory-selection.ts', references: 1 }, { file: 'src/session/use-mobile-session-close-actions.ts', references: 3 }, { file: 'src/session/use-mobile-session-content-create-actions.ts', references: 4 },