diff --git a/mobile/src/components/use-new-workspace-setup-script.ts b/mobile/src/components/use-new-workspace-setup-script.ts index db81043210f..233ff9ec080 100644 --- a/mobile/src/components/use-new-workspace-setup-script.ts +++ b/mobile/src/components/use-new-workspace-setup-script.ts @@ -35,9 +35,9 @@ export function useNewWorkspaceSetupScript(args: { } let stale = false void operations - .readRepoHooks(selectedRepo.id) + .readRepoHooksIfAvailable(selectedRepo.id) .then((result) => { - if (stale) { + if (stale || !result) { return } const command = result.hooks?.scripts?.setup?.trim() || null diff --git a/mobile/src/session/host-session-tab-operations.ts b/mobile/src/session/host-session-tab-operations.ts index 8b8418ff80a..554df8f6718 100644 --- a/mobile/src/session/host-session-tab-operations.ts +++ b/mobile/src/session/host-session-tab-operations.ts @@ -31,7 +31,7 @@ export type HostSessionTabOperations = { workspaceId: string, commandId: string ): Promise - createBrowser(workspaceId: string, url: string): Promise<{ browserPageId: string }> + createBrowser(workspaceId: string, url: string): Promise<{ browserPageId?: string }> activate(workspaceId: string, tabId: string, leafId?: string): Promise close(workspaceId: string, tabId: string): Promise } diff --git a/mobile/src/session/host-session-terminal-operations.ts b/mobile/src/session/host-session-terminal-operations.ts index 6457df719cd..10e3799e646 100644 --- a/mobile/src/session/host-session-terminal-operations.ts +++ b/mobile/src/session/host-session-terminal-operations.ts @@ -64,6 +64,8 @@ export type HostSessionTerminalOperations = { enter: boolean, clientId: string | null ): Promise + /** Answers only for a terminal this same instance subscribed: the reply is dropped otherwise. + * A caller that routes query replies must route `subscribe` through the same instance too. */ sendQueryReply( terminalId: string, bytes: string, diff --git a/mobile/src/session/mobile-session-route-parity.test.ts b/mobile/src/session/mobile-session-route-parity.test.ts index bccdf408253..dbf204dcd96 100644 --- a/mobile/src/session/mobile-session-route-parity.test.ts +++ b/mobile/src/session/mobile-session-route-parity.test.ts @@ -70,7 +70,7 @@ const HEAD_CALLBACK_BODY_SHA256 = 'e66e6436cdb9a66e870c06fdfc140106502fbeddd8db4 const HEAD_EFFECT_SHA256 = 'd9ebfaabc1e79773cdada7ab370b20459ed972f1f8edce1652199f4d0391cd13' const HEAD_CONTENT_HOOK_SHA256 = '9c3b612fef3f370d66873aefdbe1d701f20cb64ded31fef5cc45fde6f8189581' const HEAD_NESTED_FUNCTION_SHA256 = - '5042c3622cbf32166e5a2af5a534ff0305ee9446e8fd607a4baf7ec3e485b7ba' + 'd0aada4091de4551fcb5edabad4aa84249799d2fd3244c0f7493eadb4fe3ba58' const HEAD_NATIVE_REGISTRATION_SHA256 = 'cab85e4e4a3f43289ba93ddea9ccce57aea83e0bf14fd1620a965aad0c1cb49e' const HEAD_NATIVE_REMOVAL_SHA256 = @@ -79,7 +79,7 @@ const HEAD_TIMER_CREATION_SHA256 = '1a31b625e2174c3db77272249843196d2b6b06ab1e654a96d8f7858e3082e66b' const HEAD_TIMER_CLEANUP_SHA256 = 'c73f1d1c2cc89642f3d727d6f3b6b81860a9d6f34234541a2065ec3d1a8cd116' const HEAD_RUNTIME_STRING_SHA256 = - 'd5cf66745d014a3848798f7fe388d2d3370e657451dd8353e19e972b07eec324' + 'f7e7d7a99d506589d1b620b86f39bbce97223ab7ad4143b1f06a6a8d8ab81f5b' const HEAD_HOST_JSX_SHA256 = '390405926b1695fa3a33686f0bc192b432f5468d8576499d7cafbb4922defbb5' const HEAD_LEAF_JSX_SHA256 = 'd5f1ef0db57c63eb3e4ee7c98e8483bc21882a151ce0ca24e42c7d1234e1dace' const HEAD_STYLE_REFERENCE_SHA256 = @@ -517,7 +517,7 @@ describe('mobile session route extraction parity', () => { it('preserves runtime strings, styles, and the expanded JSX tree', () => { const strings = readRuntimeStrings() - expect(strings).toHaveLength(523) + expect(strings).toHaveLength(522) expect(hash(strings)).toBe(HEAD_RUNTIME_STRING_SHA256) const jsx = readJsxFacts(readDefinitions()) expect(jsx.host).toHaveLength(124) diff --git a/mobile/src/session/native-host-session-quick-command-operations.ts b/mobile/src/session/native-host-session-quick-command-operations.ts index ad5e4f85c34..36a7ce9906d 100644 --- a/mobile/src/session/native-host-session-quick-command-operations.ts +++ b/mobile/src/session/native-host-session-quick-command-operations.ts @@ -1,7 +1,4 @@ -import { - parseNormalizedTerminalQuickCommands, - type TerminalQuickCommandMutation -} from '../terminal/quick-commands' +import { parseNormalizedTerminalQuickCommands } from '../terminal/quick-commands' import type { RpcClient } from '../transport/rpc-client' import { isLogicalClientCutoverError } from '../transport/stable-logical-rpc-client' import type { @@ -19,36 +16,31 @@ export function nativeHostSessionQuickCommandOperations( return { async snapshot(workspaceId, signal) { return quickCommandSnapshot( - await quickCommandRequest(client, 'settings.getTerminalQuickCommands', undefined, signal), + await loadWithCutoverRetry(client, signal), workspaceId, 'Failed to load quick commands' ) }, async mutate(workspaceId, mutation) { - return quickCommandSnapshot( - await quickCommandRequest(client, 'settings.updateTerminalQuickCommands', { - mutation - }), - workspaceId, - 'Failed to save quick command' - ) + // Why no cutover retry here: a quick-command mutation is not idempotent, so a replay + // after a logical cutover could apply the same edit twice. + const response = await client.sendRequest('settings.updateTerminalQuickCommands', { + mutation + }) + if (!response.ok) { + throw new Error(response.error.message || 'Failed to save quick command') + } + return quickCommandSnapshot(response.result, workspaceId, 'Failed to save quick command') } } } -async function quickCommandRequest( - client: RpcClient, - method: 'settings.getTerminalQuickCommands' | 'settings.updateTerminalQuickCommands', - params?: { mutation: TerminalQuickCommandMutation }, - signal?: AbortSignal -) { +async function loadWithCutoverRetry(client: RpcClient, signal?: AbortSignal) { for (let retry = 0; ; retry += 1) { try { - const response = params - ? await client.sendRequest(method, params) - : await client.sendRequest(method) + const response = await client.sendRequest('settings.getTerminalQuickCommands') if (!response.ok) { - throw new Error(response.error.message || 'quick_commands_failed') + throw new Error(response.error.message || 'Failed to load quick commands') } return response.result } catch (error) { diff --git a/mobile/src/session/native-host-session-tab-operations.test.ts b/mobile/src/session/native-host-session-tab-operations.test.ts index d064b683853..59e55b0a57d 100644 --- a/mobile/src/session/native-host-session-tab-operations.test.ts +++ b/mobile/src/session/native-host-session-tab-operations.test.ts @@ -74,7 +74,10 @@ describe('native host session tab operations', () => { worktree: 'id:workspace-1', url: 'https://example.com', activate: true - } + }, + // The caller carried this budget before the seam existed; a browser create that parks on + // reconnect leaves the composer spinning with no error. + { timeoutMs: 30_000 } ], [ 'session.tabs.activate', diff --git a/mobile/src/session/native-host-session-tab-operations.ts b/mobile/src/session/native-host-session-tab-operations.ts index c35f8b1ca31..084bdc9aed9 100644 --- a/mobile/src/session/native-host-session-tab-operations.ts +++ b/mobile/src/session/native-host-session-tab-operations.ts @@ -83,19 +83,18 @@ export function nativeHostSessionTabOperations(client: RpcClient): HostSessionTa ) }, async createBrowser(workspaceId, url) { - const response = await client.sendRequest('browser.tabCreate', { - worktree: `id:${workspaceId}`, - url, - activate: true - }) + const response = await client.sendRequest( + 'browser.tabCreate', + { worktree: `id:${workspaceId}`, url, activate: true }, + { timeoutMs: 30_000 } + ) if (!response.ok) { - throw new Error('browser_create_failed') + throw new Error(response.error.message) } + // A host that answers without a page id still created the tab; the caller only loses the + // focus hint, so this is not a create failure. const result = (response as RpcSuccess).result as { browserPageId?: unknown } - if (typeof result.browserPageId !== 'string') { - throw new Error('browser_create_failed') - } - return { browserPageId: result.browserPageId } + return typeof result.browserPageId === 'string' ? { browserPageId: result.browserPageId } : {} }, async activate(workspaceId, tabId, leafId) { // Why: a relay-to-direct cutover rejects the in-flight request; activation is idempotent, diff --git a/mobile/src/session/use-mobile-native-chat-stop.ts b/mobile/src/session/use-mobile-native-chat-stop.ts index 2b66a860d94..8dd8aa4f654 100644 --- a/mobile/src/session/use-mobile-native-chat-stop.ts +++ b/mobile/src/session/use-mobile-native-chat-stop.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useRef, type MutableRefObject } from 'react' +import { useCallback, useEffect, useLayoutEffect, useRef, type MutableRefObject } from 'react' import type { HostSessionNativeChatOperations } from './host-session-native-chat-operations' import { mobileNativeChatOperationTarget } from './mobile-native-chat-operation-target' import { openMobileNativeChatSendBudget } from './mobile-native-chat-send' @@ -30,7 +30,11 @@ export function useMobileNativeChatStop(args: { * never happen. */ const dropSecondEscapeRef = useRef<(() => void) | null>(null) const activeRouteRef = useRef({ operations, enabled, streamIdentity }) - activeRouteRef.current = { operations, enabled, streamIdentity } + // Layout, not render or a passive Effect: the paced Escape fires from a timer, and only a + // synchronous post-commit write guarantees it never reads a route the render discarded. + useLayoutEffect(() => { + activeRouteRef.current = { operations, enabled, streamIdentity } + }, [operations, enabled, streamIdentity]) const cancelSecondEscape = useCallback(() => { if (timerRef.current) { clearTimeout(timerRef.current) diff --git a/mobile/src/session/use-mobile-session-terminal-input.ts b/mobile/src/session/use-mobile-session-terminal-input.ts index 79b0352b1b4..db31020ca3b 100644 --- a/mobile/src/session/use-mobile-session-terminal-input.ts +++ b/mobile/src/session/use-mobile-session-terminal-input.ts @@ -224,9 +224,9 @@ export function useMobileSessionTerminalInput(scope: MobileSessionFileActionsMod } getTerminalRef(target.handle)?.clear() try { - if (!(await sessionOperations.terminal.clear(target.handle))) { - throw new Error('terminal_clear_failed') - } + // Why the result is ignored: a host that answers `ok: false` still cleared the local + // buffer above, and only a transport failure told the user the clear did not happen. + await sessionOperations.terminal.clear(target.handle) showToast('Terminal cleared') } catch { showToast("Couldn't clear terminal", 1500) diff --git a/mobile/src/tasks/github-project-host-routing-source.test.ts b/mobile/src/tasks/github-project-host-routing-source.test.ts index be992ed4558..92e80628b6a 100644 --- a/mobile/src/tasks/github-project-host-routing-source.test.ts +++ b/mobile/src/tasks/github-project-host-routing-source.test.ts @@ -45,13 +45,19 @@ describe('mobile GitHub Project host routing boundary', () => { it('pins Project-row PR actions to the row repository identity', () => { // Every Project-row action resolves its target from the row plus the active Project host. - const targets = [...compositionSource.matchAll(/projectRowMutationTarget\(([^)]*)\)/g)] + const targets = [ + ...compositionSource.matchAll(/projectRow(?:Mutation|PullRequest)Target\(([^)]*)\)/g) + ] expect(targets.length).toBeGreaterThan(10) for (const target of targets) { expect(target[1].replace(/\s+/g, ' ').trim()).toBe('row, activeGitHubProjectHost') } - // The target type carries the host that the PR mutations forward as prRepo. - expect(projectMutationAdapter).toContain('prRepo: slugPayload(target)') + // The target type carries the host that the PR mutations forward as prRepo, and a row with + // no slug forwards null rather than being refused. + expect(projectMutationAdapter).toContain('prRepo: prRepoPayload(target)') + expect(projectMutationAdapter).toMatch( + /function prRepoPayload\(target: HostTaskProjectItemTarget\) \{\s*return target\.owner && target\.repo/ + ) for (const method of [ 'fetchResolveReviewThread', 'fetchAddPRReviewCommentReply', diff --git a/mobile/src/tasks/mobile-tasks-mutation-targets.ts b/mobile/src/tasks/mobile-tasks-mutation-targets.ts index cf82d2d5112..851b0ec7e28 100644 --- a/mobile/src/tasks/mobile-tasks-mutation-targets.ts +++ b/mobile/src/tasks/mobile-tasks-mutation-targets.ts @@ -9,6 +9,7 @@ import type { TaskItem } from './mobile-tasks-project-workspace-types' export { projectRowIdentityTarget, projectRowMutationTarget, + projectRowPullRequestTarget, projectRowSlugTarget } from './mobile-tasks-project-row-targets' diff --git a/mobile/src/tasks/mobile-tasks-project-row-targets.ts b/mobile/src/tasks/mobile-tasks-project-row-targets.ts index e24f73b22e3..25b2442711e 100644 --- a/mobile/src/tasks/mobile-tasks-project-row-targets.ts +++ b/mobile/src/tasks/mobile-tasks-project-row-targets.ts @@ -24,6 +24,28 @@ export function projectRowMutationTarget( : null } +/** + * For the pull-request mutations the host addresses by number, carrying the slug only as + * optional `prRepo` decoration: reviewers, check reruns, merges and file-viewed state. The slug + * is not required, because a row without one still names a pull request the host can act on. + */ +export function projectRowPullRequestTarget( + row: GitHubProjectRow, + host: string +): HostTaskProjectItemTarget | null { + const slug = splitRepositorySlug(row.content.repository) + const type = projectRowType(row) + return type && row.content.number + ? { + owner: slug?.owner ?? '', + repo: slug?.repo ?? '', + host, + number: row.content.number, + type + } + : null +} + /** * For mutations the host addresses by repository slug and comment id — the `*BySlug` comment * edits. They read no issue number and no issue/PR kind, so requiring those refused rows diff --git a/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts b/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts index 9f5844bbb5c..63eb851d792 100644 --- a/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts +++ b/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts @@ -25,11 +25,11 @@ const hash = (parts: string[] | string): string => * plus the row-target locals the adapters take in place of inline slug/number checks. Diff hooks, * declarations and styles are untouched. */ -const SCREEN_HOOKS = 'e7c47c8624d0c51100d1704cd7865a022d1ea372ad659c1f9d2581afa9e0de6a' +const SCREEN_HOOKS = 'b5d85ab1cd2996a6986604caf497c8a577dc118b095c49d555c576d98f48148f' const DIFF_HOOKS = '93c7189b32bed8456cc51814fffa8ce80cf62011ef968a9d53ddec2b9686f58f' -const STATEMENTS = '268d9cebe985c14b35052128ce985e81306d4507aae49a6301c165dc65d93337' +const STATEMENTS = '1d059c8359400f2aedb16c94a7dba0c8a21b5a06edb45e2b0b7ff0317250d564' const DECLARATIONS = 'cff54172af17a877789be1479c2eb6ca97d83c3e31dd831cd59395962f2b4c4a' -const SEMANTICS = '69cfc4f59322d1050b47f7028171ef5c9cbda6c3e51878676ecf9be68122affa' +const SEMANTICS = 'f767906884b93537f2c6369d6d0bd2d4cb39b4314c31cca9d8f9e5e9b78a75ee' const STYLES = '1db6af69c791d9963928541ad5310942fcbda6d984b422c90b6eb92b6816579a' const RENDER_TREE = 'a959c6712c70024127a429a47ae7689c629393752fa99a1ac6ba311d0bf13ca6' @@ -58,7 +58,7 @@ describe('Mobile Tasks refactor parity', () => { it('preserves RPC calls, runtime strings, and JSX host signatures', () => { const semantics = readMobileTasksSemanticSource() - expect(semantics.split('\n')).toHaveLength(3_276) + expect(semantics.split('\n')).toHaveLength(3_278) expect(hash(semantics)).toBe(SEMANTICS) }) diff --git a/mobile/src/tasks/native-host-task-item-mutation-operations.ts b/mobile/src/tasks/native-host-task-item-mutation-operations.ts index 59f385bee77..7f30a1efa8c 100644 --- a/mobile/src/tasks/native-host-task-item-mutation-operations.ts +++ b/mobile/src/tasks/native-host-task-item-mutation-operations.ts @@ -14,14 +14,25 @@ export function nativeHostTaskItemMutationOperations( target.provider === 'github' ? await setGitHubClosed(client, target, closed) : await setGitLabClosed(client, target, closed) - assertMutation(response, 'Failed to update task status') + // Provider-keyed, because each caller reported its own provider's wording before the seam. + assertMutation( + response, + target.provider === 'github' + ? 'Failed to update GitHub status' + : 'Failed to update GitLab item' + ) }, async updateMetadata(target, updates) { const response = target.provider === 'github' ? await updateGitHubMetadata(client, target, updates) : await updateGitLabMetadata(client, target, updates) - assertMutation(response, 'Failed to update task') + assertMutation( + response, + target.provider === 'github' + ? 'Failed to update GitHub issue' + : 'Failed to update GitLab item' + ) } } } diff --git a/mobile/src/tasks/native-host-task-linear-operations.ts b/mobile/src/tasks/native-host-task-linear-operations.ts index 4f2fd3c2679..1f763b58393 100644 --- a/mobile/src/tasks/native-host-task-linear-operations.ts +++ b/mobile/src/tasks/native-host-task-linear-operations.ts @@ -1,5 +1,10 @@ import type { HostTaskLinearOperations } from './host-task-linear-operations' import type { RpcRequestSender } from '../transport/rpc-client' +import type { SendRequestOptions } from '../transport/rpc-client' + +/** The interactive Linear writes carry their own budget; the reads that only feed a picker do + * not, so a slow host degrades the picker instead of the whole screen. */ +const INTERACTIVE: SendRequestOptions = { timeoutMs: 30_000 } export function nativeHostTaskLinearOperations(client: RpcRequestSender): HostTaskLinearOperations { return { @@ -16,26 +21,23 @@ export function nativeHostTaskLinearOperations(client: RpcRequestSender): HostTa workspaceId: target.workspaceId }), async selectWorkspace(workspaceId) { - assertMutation( - await request(client, 'linear.selectWorkspace', { workspaceId }), - 'Failed to select workspace' - ) + // Why no result check: the picker already switched, and it reloads the Linear context next. + // A rejected switch surfaces through that reload, not as a second error on the same tap. + await client.sendRequest('linear.selectWorkspace', { workspaceId }) }, async updateState(target, stateId) { - assertMutation( - await request(client, 'linear.updateIssue', { - id: target.issueId, - workspaceId: target.workspaceId, - updates: { stateId } - }), - 'Failed to update Linear issue' - ) + await request(client, 'linear.updateIssue', { + id: target.issueId, + workspaceId: target.workspaceId, + updates: { stateId } + }) }, async addComment(target, body) { const result = await request<{ ok?: boolean; id?: string; error?: string }>( client, 'linear.addIssueComment', - { issueId: target.issueId, workspaceId: target.workspaceId, body } + { issueId: target.issueId, workspaceId: target.workspaceId, body }, + INTERACTIVE ) assertMutation(result, 'Failed to add comment') return result.id @@ -43,21 +45,32 @@ export function nativeHostTaskLinearOperations(client: RpcRequestSender): HostTa async loadIssue(target) { const issue = await request - > | null>(client, 'linear.getIssue', { id: target.issueId, workspaceId: target.workspaceId }) + > | null>( + client, + 'linear.getIssue', + { id: target.issueId, workspaceId: target.workspaceId }, + INTERACTIVE + ) if (!issue) { - throw new Error('Linear issue not found') + throw new Error('Sub-issue not found') } return issue }, async createSubIssue(target, title) { return createdIssue( - await request(client, 'linear.createIssue', { - teamId: target.teamId, - title, - workspaceId: target.workspaceId, - parentIssueId: target.issueId, - projectId: target.projectId ?? null - }) + await request( + client, + 'linear.createIssue', + { + teamId: target.teamId, + title, + workspaceId: target.workspaceId, + parentIssueId: target.issueId, + projectId: target.projectId ?? null + }, + INTERACTIVE + ), + 'Failed to create sub-issue' ) }, async createIssue(payload) { @@ -67,7 +80,8 @@ export function nativeHostTaskLinearOperations(client: RpcRequestSender): HostTa title: payload.title, description: payload.description, workspaceId: payload.team.workspaceId - }) + }), + 'Failed to create Linear issue' ) } } @@ -76,9 +90,12 @@ export function nativeHostTaskLinearOperations(client: RpcRequestSender): HostTa async function request( client: RpcRequestSender, method: string, - payload?: object + payload?: object, + options?: SendRequestOptions ): Promise { - const response = await client.sendRequest(method, payload, { timeoutMs: 30_000 }) + const response = options + ? await client.sendRequest(method, payload, options) + : await client.sendRequest(method, payload) if (!response.ok) { throw new Error(response.error?.message ?? 'Task request failed') } @@ -94,7 +111,7 @@ function assertMutation( } } -function createdIssue(result: unknown) { +function createdIssue(result: unknown, fallback: string) { const issue = result as { ok?: boolean id?: string @@ -104,7 +121,7 @@ function createdIssue(result: unknown) { error?: string } if (issue.ok === false || !issue.id || !issue.identifier) { - throw new Error(issue.error ?? 'Failed to create Linear issue') + throw new Error(issue.error ?? fallback) } return { id: issue.id, diff --git a/mobile/src/tasks/native-host-task-project-file-operations.ts b/mobile/src/tasks/native-host-task-project-file-operations.ts index 126bbf0eae3..978fb925027 100644 --- a/mobile/src/tasks/native-host-task-project-file-operations.ts +++ b/mobile/src/tasks/native-host-task-project-file-operations.ts @@ -55,7 +55,11 @@ export function nativeHostTaskProjectFileOperations( function repoPayload(target: HostTaskProjectItemTarget, repoId: string) { return { repo: `id:${repoId}`, - prRepo: { owner: target.owner, repo: target.repo, host: target.host } + // Null when the row carries no slug: that is what the screens sent before this seam. + prRepo: + target.owner && target.repo + ? { owner: target.owner, repo: target.repo, host: target.host } + : null } } diff --git a/mobile/src/tasks/native-host-task-project-mutation-operations.ts b/mobile/src/tasks/native-host-task-project-mutation-operations.ts index 3f2f5dd339c..a7e304da850 100644 --- a/mobile/src/tasks/native-host-task-project-mutation-operations.ts +++ b/mobile/src/tasks/native-host-task-project-mutation-operations.ts @@ -86,10 +86,11 @@ export function nativeHostTaskProjectMutationOperations( await fetchResolveReviewThread(client, repoId, { threadId, resolve, - // Why: `prRepo` is fork/GHES decoration the host treats as optional, and a draft row + // Why: a draft row // has no slug — send it only when one resolved rather than an empty pair. - prRepo: target.owner && target.repo ? slugPayload(target) : null - }) + prRepo: prRepoPayload(target) + }), + 'Failed to resolve thread' ) }, async replyReviewComment(target, repoId, payload) { @@ -97,8 +98,9 @@ export function nativeHostTaskProjectMutationOperations( await fetchAddPRReviewCommentReply(client, repoId, { prNumber: target.number, ...payload, - prRepo: slugPayload(target) - }) + prRepo: prRepoPayload(target) + }), + 'Failed to reply' ) }, async addConversationComment(target, repoId, body) { @@ -106,9 +108,10 @@ export function nativeHostTaskProjectMutationOperations( await fetchAddIssueComment(client, repoId, { prNumber: target.number, body, - prRepo: slugPayload(target), + prRepo: prRepoPayload(target), type: target.type - }) + }), + 'Failed to reply' ) }, async requestReviewers(target, repoId, reviewers) { @@ -116,8 +119,9 @@ export function nativeHostTaskProjectMutationOperations( await fetchRequestPRReviewers(client, repoId, { prNumber: target.number, reviewers, - prRepo: slugPayload(target) - }) + prRepo: prRepoPayload(target) + }), + 'Failed to request reviewers' ) }, async rerunChecks(target, repoId, payload) { @@ -125,10 +129,11 @@ export function nativeHostTaskProjectMutationOperations( await fetchRerunPRChecks( client, repoId, - { prNumber: target.number, ...payload, prRepo: slugPayload(target) }, + { 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' ) }, async merge(target, repoId, method) { @@ -136,14 +141,23 @@ export function nativeHostTaskProjectMutationOperations( await fetchMergePR( client, repoId, - { prNumber: target.number, method, prRepo: slugPayload(target) }, + { prNumber: target.number, method, prRepo: prRepoPayload(target) }, { timeoutMs: PROJECT_PR_MUTATION_TIMEOUT_MS } - ) + ), + 'Failed to merge pull request' ) } } } +/** Fork/GHES decoration the host treats as optional. A row with no repository slug sent `null` + * before this seam existed, so refusing the whole call here would lose a working path. */ +function prRepoPayload(target: HostTaskProjectItemTarget) { + return target.owner && target.repo + ? { owner: target.owner, repo: target.repo, host: target.host } + : null +} + function slugPayload(target: HostTaskProjectItemTarget) { return { owner: target.owner, @@ -153,35 +167,52 @@ function slugPayload(target: HostTaskProjectItemTarget) { } } +/** The wording each caller reported for a refused mutation before these calls moved behind the + * seam. A host that refuses without a message must still name the action that failed. */ +const PROJECT_MUTATION_FALLBACKS: Record = { + 'github.project.updateIssueBySlug': 'Failed to update GitHub item', + 'github.project.updatePullRequestBySlug': 'Failed to update GitHub item', + 'github.project.addIssueCommentBySlug': 'Failed to add comment', + 'github.project.updateIssueCommentBySlug': 'Failed to edit comment', + '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' +} + async function projectMutation( client: RpcRequestSender, method: string, payload: object ): Promise { + 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 } } if (!response.ok) { - throw new Error(response.error?.message ?? 'GitHub Project request failed') + throw new Error(response.error?.message ?? fallback) } if (response.result?.ok === false) { const error = response.result.error - throw new Error( - typeof error === 'string' ? error : (error?.message ?? 'GitHub Project request failed') - ) + throw new Error(typeof error === 'string' ? error : (error?.message ?? fallback)) } return (response.result ?? {}) as T } -function requirePrMutation(result: GitHubPrMutationOutcome): void { +function requirePrMutation(result: GitHubPrMutationOutcome, fallback: string): void { if (!result.ok) { - throw new Error(result.error) + // The wrapper mints `Request failed: ` when the host says nothing; the caller's own + // wording is what the user read before these calls moved behind the seam. + throw new Error(result.error.startsWith('Request failed: ') ? fallback : result.error) } } -function prMutationComment(result: GitHubPrMutationOutcome): DetailComment | undefined { - requirePrMutation(result) +function prMutationComment( + result: GitHubPrMutationOutcome, + fallback: string +): DetailComment | undefined { + requirePrMutation(result, fallback) return (result as { comment?: DetailComment }).comment } diff --git a/mobile/src/tasks/use-mobile-tasks-project-file-merge-actions.tsx b/mobile/src/tasks/use-mobile-tasks-project-file-merge-actions.tsx index a8cfff17a07..4c5125dc3d7 100644 --- a/mobile/src/tasks/use-mobile-tasks-project-file-merge-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-project-file-merge-actions.tsx @@ -7,7 +7,10 @@ import type { HostedReviewMergeMethod, TaskItem } from './mobile-tasks-legacy-foundation' -import { projectRowMutationTarget, taskItemMutationTarget } from './mobile-tasks-mutation-targets' +import { + projectRowPullRequestTarget, + taskItemMutationTarget +} from './mobile-tasks-mutation-targets' export function useMobileTasksProjectFileMergeActions(model: ProjectReviewCheckActionsModel) { const { @@ -45,7 +48,7 @@ export function useMobileTasksProjectFileMergeActions(model: ProjectReviewCheckA return } const repo = findProjectRowRepo(row) - const target = projectRowMutationTarget(row, activeGitHubProjectHost) + const target = projectRowPullRequestTarget(row, activeGitHubProjectHost) if ( !taskOperations || row.itemType !== 'PULL_REQUEST' || @@ -90,7 +93,7 @@ export function useMobileTasksProjectFileMergeActions(model: ProjectReviewCheckA const addProjectGitHubFileReviewComment = useCallback( async (row: GitHubProjectRow, file: GitHubDetailFile, line: number): Promise => { const repo = findProjectRowRepo(row) - const target = projectRowMutationTarget(row, activeGitHubProjectHost) + const target = projectRowPullRequestTarget(row, activeGitHubProjectHost) if ( !taskOperations || projectMutating || @@ -157,7 +160,7 @@ export function useMobileTasksProjectFileMergeActions(model: ProjectReviewCheckA const mergeProjectGitHubPullRequest = useCallback( async (row: GitHubProjectRow, method: HostedReviewMergeMethod): Promise => { const repo = findProjectRowRepo(row) - const target = projectRowMutationTarget(row, activeGitHubProjectHost) + const target = projectRowPullRequestTarget(row, activeGitHubProjectHost) if ( !taskOperations || projectMutating || diff --git a/mobile/src/tasks/use-mobile-tasks-project-review-check-actions.tsx b/mobile/src/tasks/use-mobile-tasks-project-review-check-actions.tsx index 6f8621db38d..e5b739dc0f6 100644 --- a/mobile/src/tasks/use-mobile-tasks-project-review-check-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-project-review-check-actions.tsx @@ -6,7 +6,7 @@ import { type GitHubProjectRow, splitReviewerList } from './mobile-tasks-legacy-foundation' -import { projectRowMutationTarget } from './mobile-tasks-mutation-targets' +import { projectRowPullRequestTarget } from './mobile-tasks-mutation-targets' export function useMobileTasksProjectReviewCheckActions(model: ProjectMetadataActionsModel) { const { @@ -25,7 +25,7 @@ export function useMobileTasksProjectReviewCheckActions(model: ProjectMetadataAc const requestProjectGitHubReviewers = useCallback( async (row: GitHubProjectRow, logins?: string[]): Promise => { const repo = findProjectRowRepo(row) - const target = projectRowMutationTarget(row, activeGitHubProjectHost) + const target = projectRowPullRequestTarget(row, activeGitHubProjectHost) if ( !taskOperations || projectMutating || @@ -92,7 +92,7 @@ export function useMobileTasksProjectReviewCheckActions(model: ProjectMetadataAc const refreshProjectGitHubChecks = useCallback( async (row: GitHubProjectRow): Promise => { const repo = findProjectRowRepo(row) - const target = projectRowMutationTarget(row, activeGitHubProjectHost) + const target = projectRowPullRequestTarget(row, activeGitHubProjectHost) if ( !taskOperations || projectMutating || @@ -125,7 +125,7 @@ export function useMobileTasksProjectReviewCheckActions(model: ProjectMetadataAc const rerunProjectGitHubChecks = useCallback( async (row: GitHubProjectRow, failedOnly: boolean): Promise => { const repo = findProjectRowRepo(row) - const target = projectRowMutationTarget(row, activeGitHubProjectHost) + const target = projectRowPullRequestTarget(row, activeGitHubProjectHost) if ( !taskOperations || projectMutating || @@ -157,7 +157,7 @@ export function useMobileTasksProjectReviewCheckActions(model: ProjectMetadataAc const toggleProjectGitHubFileViewed = useCallback( async (row: GitHubProjectRow, file: GitHubDetailFile): Promise => { const repo = findProjectRowRepo(row) - const target = projectRowMutationTarget(row, activeGitHubProjectHost) + const target = projectRowPullRequestTarget(row, activeGitHubProjectHost) if ( !taskOperations || projectMutating || diff --git a/mobile/src/worktree/host-workspace-catalog-operations.ts b/mobile/src/worktree/host-workspace-catalog-operations.ts index d63bfbd6f3d..ac43f47f6ad 100644 --- a/mobile/src/worktree/host-workspace-catalog-operations.ts +++ b/mobile/src/worktree/host-workspace-catalog-operations.ts @@ -33,7 +33,7 @@ export type HostWorkspaceCatalogOperations = { sleepWorkspace(workspaceId: string): Promise notifyForeground(): void subscribeChanges(listener: (event: HostWorkspaceChange) => void): () => void - // Why: a hosted page reads `connected` from the shell's relayed snapshot, so it can issue its - // first catalog request a beat before that socket serves one. A direct socket omits this. + // Why: a provider that relays connection state can report `connected` a beat before its + // socket serves a catalog request. A direct socket omits this, so the flag is optional. readonly connectionStateIsRelayed?: boolean } diff --git a/mobile/src/worktree/host-workspace-creation-operations.ts b/mobile/src/worktree/host-workspace-creation-operations.ts index 5fee7251d74..4ca39de323e 100644 --- a/mobile/src/worktree/host-workspace-creation-operations.ts +++ b/mobile/src/worktree/host-workspace-creation-operations.ts @@ -89,6 +89,9 @@ export type HostWorkspaceCreationOperations = { connectSsh(targetId: string): Promise detectAgents(connectionId: string | null): Promise readRepoHooks(repoId: string): Promise + /** Null when the host refuses the read. The create sheet keeps whatever it already showed + * rather than replacing it with a "no setup script" answer it cannot stand behind. */ + readRepoHooksIfAvailable(repoId: string): Promise readRuntimeCapabilities(): Promise listSparsePresets(repoId: string): Promise saveSparsePreset( diff --git a/mobile/src/worktree/native-host-workspace-creation-read-operations.ts b/mobile/src/worktree/native-host-workspace-creation-read-operations.ts index cb13a1caae0..ab180034909 100644 --- a/mobile/src/worktree/native-host-workspace-creation-read-operations.ts +++ b/mobile/src/worktree/native-host-workspace-creation-read-operations.ts @@ -23,6 +23,7 @@ type ReadOperations = Pick< | 'connectSsh' | 'detectAgents' | 'readRepoHooks' + | 'readRepoHooksIfAvailable' | 'readRuntimeCapabilities' > @@ -37,10 +38,12 @@ export function nativeHostWorkspaceCreationReadOperations( return result.repos }, async readRetiredWorktreeNames(repoId) { - const result = await successfulResult( - client.sendRequest('worktree.listRetiredNames', { repo: `id:${repoId}` }) - ) - return readRetiredNameRegistryForRepo(result, repoId) + // Why no ok check: a refused read must read as an empty registry, not as a failure. The + // caller holds its previous answer on a failure, which would keep offering a spent name. + const response = await client.sendRequest('worktree.listRetiredNames', { + repo: `id:${repoId}` + }) + return readRetiredNameRegistryForRepo((response as { result?: unknown }).result, repoId) }, async readRuntimeSettings() { const result = await successfulResult<{ settings: NewWorkspaceRuntimeSettings }>( @@ -90,6 +93,10 @@ export function nativeHostWorkspaceCreationReadOperations( client.sendRequest('repo.hooks', { repo: `id:${repoId}` }) ) }, + async readRepoHooksIfAvailable(repoId) { + const response = await client.sendRequest('repo.hooks', { repo: `id:${repoId}` }) + return response.ok ? ((response as RpcSuccess).result as NewWorkspaceRepoHooks) : null + }, readRuntimeCapabilities() { return readNewWorktreeRuntimeCapabilities(client) } diff --git a/mobile/src/worktree/rpc-workspace-creation-operations.ts b/mobile/src/worktree/rpc-workspace-creation-operations.ts index 0a411d65fc4..051deca5ba1 100644 --- a/mobile/src/worktree/rpc-workspace-creation-operations.ts +++ b/mobile/src/worktree/rpc-workspace-creation-operations.ts @@ -9,9 +9,9 @@ export type RpcWorkspaceCreationOperations = Omit< 'createBlankWorkspace' | 'createWorkspaceFromSource' > -/** Every workspace-creation call that is a plain desktop request. The native app passes its socket - * and the hosted page passes a bridge-backed sender, so neither side owns a second copy. Creation - * itself is excluded: it needs connection state for its retry, which a sender cannot report. */ +/** Every workspace-creation call that is a plain desktop request, written against a bare request + * sender so any non-socket provider can supply one without a second copy. Creation itself is + * excluded: it needs connection state for its retry, which a sender cannot report. */ export function rpcWorkspaceCreationOperations( client: RpcRequestSender ): RpcWorkspaceCreationOperations {