From 33d9abb5301671cb615fb21b131e6621489296cf Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Fri, 4 Sep 2026 16:58:38 -0400 Subject: [PATCH] fix(mobile): restore eight native-build behaviours lost in the hybrid refactor P1-1 notification taps resolve against loadHostCatalog again, so a host whose keychain read failed still routes (to credential recovery) instead of dying on an unknown-host gate. The resolver seam now *requires* credentialStatus, making the token-only loader a compile error. P1-2 thread nameIsAutoManaged into the tasks workspace-create call; a typed name was published as displayNameKind:'generated'. P1-3 chat Stop sends a bare Escape (enter:false) on both the native and hybrid paths; enter:true submitted whatever the agent had parked on its input line. P1-4 the repeat controller owns the press-time accessory send; the dock's extra handleAccessoryKey call made every arrow/Backspace/Tab tap emit twice. P1-5 capture the send origin through normalizeReconcileText, and compare pending text the same way, so a multi-line prompt can match its transcript echo. P1-6 restore mobileNativeChatStreamPreview so tool stdout/stderr stops rendering as the streaming assistant bubble. P1-7 restore the cross-workspace preview gate and address files.open at the workspace the host resolved. P1-8 derive browser Back/Forward from the live tab props and treat the new screencast navigation event as an additive refinement, so an older host that never emits it cannot freeze the arrows. P3: Array.isArray guard on both refreshChecks seams; empty GitLab todo list reads as []; project merge/rerun get the 60s budget their item-level twins kept; toggleGitHubProjectFieldVisibility uses a functional updater; file explorer and preview Retry reconnect without the operations object that is null exactly when disconnected; syntax/inline token keys include the index; the legacy files.list fallback is scoped per workspace. Two parity digests re-frozen with a note (tasks composition, session host JSX). Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- mobile/app/_layout.tsx | 4 +- mobile/src/browser/MobileBrowserPane.tsx | 6 + ...ile-browser-navigation-enablement.test.tsx | 156 ++++++++++++++++++ .../src/browser/use-mobile-browser-stream.ts | 12 +- .../src/components/MobileSyntaxSegments.tsx | 5 +- .../components/pr-sidebar/CommentMarkdown.tsx | 5 +- mobile/src/files/MobileFileExplorerPanel.tsx | 14 +- mobile/src/files/MobileFilePreviewScreen.tsx | 14 +- ...ile-web-native-chat-terminal-operations.ts | 3 +- ...eb-task-project-mutation-roundtrip.test.ts | 17 +- .../notification-route-coordination.test.ts | 11 ++ .../notification-routing.test.ts | 26 ++- .../src/notifications/notification-routing.ts | 11 +- .../src/session/MobileSessionCommandDock.tsx | 6 +- ...accessory-key-press-send-ownership.test.ts | 32 ++++ mobile/src/session/github-pr-mutations.ts | 20 ++- .../src/session/mobile-file-tap-open.test.ts | 81 +++++++++ mobile/src/session/mobile-file-tap-open.ts | 10 +- .../session/mobile-native-chat-eligibility.ts | 2 + .../mobile-native-chat-streaming-gate.test.ts | 35 ++++ .../mobile-native-chat-streaming-gate.ts | 16 ++ .../mobile-session-route-parity.test.ts | 5 +- ...ost-session-native-chat-operations.test.ts | 66 ++++++++ ...ive-host-session-native-chat-operations.ts | 18 +- .../use-mobile-native-chat-controller.ts | 7 +- .../use-mobile-native-chat-drafts.test.ts | 33 ++++ .../session/use-mobile-native-chat-drafts.ts | 5 +- ...e-mobile-native-chat-pending-deliveries.ts | 5 +- .../mobile-tasks-refactor-parity.test.ts | 14 +- .../native-host-task-item-file-operations.ts | 8 +- .../tasks/native-host-task-list-operations.ts | 6 +- ...tive-host-task-operation-contracts.test.ts | 57 +++++++ ...ative-host-task-project-file-operations.ts | 6 +- ...e-host-task-project-mutation-operations.ts | 25 +-- ...sk-workspace-create-name-authority.test.ts | 23 +++ ...e-mobile-tasks-client-settings-actions.tsx | 36 ++-- ...-mobile-tasks-workspace-create-actions.tsx | 11 +- .../terminal-accessory-repeat.test.ts | 31 ++++ 38 files changed, 750 insertions(+), 92 deletions(-) create mode 100644 mobile/src/browser/mobile-browser-navigation-enablement.test.tsx create mode 100644 mobile/src/session/accessory-key-press-send-ownership.test.ts create mode 100644 mobile/src/session/native-host-session-native-chat-operations.test.ts create mode 100644 mobile/src/tasks/native-host-task-operation-contracts.test.ts create mode 100644 mobile/src/tasks/task-workspace-create-name-authority.test.ts diff --git a/mobile/app/_layout.tsx b/mobile/app/_layout.tsx index 33ecf727b22..acbc6d1a6ae 100644 --- a/mobile/app/_layout.tsx +++ b/mobile/app/_layout.tsx @@ -30,7 +30,7 @@ import { MOBILE_NATIVE_BASELINE_MODE } from '../src/mobile-web/mobile-native-baseline-mode' import { mobileHostWorkspaceEntry } from '../src/mobile-web/mobile-web-home-navigation' -import { loadHosts } from '../src/transport/host-store' +import { loadHostCatalog, loadHosts } from '../src/transport/host-store' import { extractPairingCodeFromUrl } from '../src/transport/pairing' import { recoverMobileRelayPairing } from '../src/transport/mobile-relay-pairing-recovery' @@ -171,7 +171,7 @@ export default function RootLayout() { } async function getNavigation(data: unknown) { - return notificationNavigationResolverRef.current!.resolve(data, loadHosts) + return notificationNavigationResolverRef.current!.resolve(data, loadHostCatalog) } async function handleNotificationResponse(response: Notifications.NotificationResponse) { diff --git a/mobile/src/browser/MobileBrowserPane.tsx b/mobile/src/browser/MobileBrowserPane.tsx index 841c35f3921..0771c2a3643 100644 --- a/mobile/src/browser/MobileBrowserPane.tsx +++ b/mobile/src/browser/MobileBrowserPane.tsx @@ -177,6 +177,12 @@ export function MobileBrowserPane({ focused: addressFocused, url: tab.url }) + // The screencast `navigation` event is a newer-host refinement; the tab props stay + // the baseline so an older host that never emits it does not freeze Back/Forward. + useEffect(() => { + setNavigationState({ canGoBack: tab.canGoBack, canGoForward: tab.canGoForward }) + }, [tab.canGoBack, tab.canGoForward]) + useEffect(() => { if (addressSync.nextState === addressSyncState) { return diff --git a/mobile/src/browser/mobile-browser-navigation-enablement.test.tsx b/mobile/src/browser/mobile-browser-navigation-enablement.test.tsx new file mode 100644 index 00000000000..67c312fb378 --- /dev/null +++ b/mobile/src/browser/mobile-browser-navigation-enablement.test.tsx @@ -0,0 +1,156 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { describe, expect, it, vi } from 'vitest' +import type { BrowserScreencastFrame } from '../transport/browser-screencast-protocol' +import { MobileBrowserPane, type MobileBrowserTab } from './MobileBrowserPane' + +vi.mock('react-native', () => ({ + ActivityIndicator: 'ActivityIndicator', + AppState: { currentState: 'active', addEventListener: () => ({ remove: () => {} }) }, + Image: 'Image', + PanResponder: { create: () => ({ panHandlers: {} }) }, + PixelRatio: { get: () => 2 }, + Platform: { OS: 'android' }, + Pressable: 'Pressable', + StyleSheet: { + absoluteFillObject: { position: 'absolute', top: 0, left: 0, right: 0, bottom: 0 }, + create: (styles: unknown) => styles + }, + Text: 'Text', + TextInput: 'TextInput', + View: 'View' +})) + +vi.mock('lucide-react-native', () => ({ + ArrowUp: 'ArrowUp', + ChevronLeft: 'ChevronLeft', + ChevronRight: 'ChevronRight', + Monitor: 'Monitor', + RefreshCw: 'RefreshCw', + Smartphone: 'Smartphone' +})) + +let pageCounter = 0 + +function browserTab(navigation: { canGoBack: boolean; canGoForward: boolean }): MobileBrowserTab { + return { + type: 'browser', + id: `tab-${pageCounter}`, + title: 'Dashboard', + browserWorkspaceId: 'bw-1', + browserPageId: `page-${pageCounter}`, + url: 'https://dashboard.example', + loading: false, + isActive: true, + ...navigation + } +} + +function backButtonDisabled(renderer: ReactTestRenderer): boolean { + const back = renderer.root + .findAllByType('Pressable') + .find((node) => node.props.accessibilityLabel === 'Back') + if (!back) { + throw new Error('Back control not found') + } + return back.props.disabled === true +} + +async function renderPane(navigation: { canGoBack: boolean; canGoForward: boolean }): Promise<{ + renderer: ReactTestRenderer + emit: (payload: unknown) => void + update: (next: { canGoBack: boolean; canGoForward: boolean }) => Promise +}> { + pageCounter += 1 + let listener: ((payload: unknown) => void) | null = null + const operations = { + subscribe: ( + _target: unknown, + _request: unknown, + handlers: { + onEvent: (payload: unknown) => void + onFrame?: (frame: BrowserScreencastFrame) => void + } + ) => { + listener = handlers.onEvent + return () => {} + }, + request: vi.fn() + } + const worktreeId = `wt-${pageCounter}` + const props = (tab: MobileBrowserTab) => ({ + operations, + worktreeId, + tab, + screencastSupported: true, + keyboardLift: 0, + bottomInset: 0, + onToast: () => {} + }) + + let renderer: ReactTestRenderer + await act(async () => { + renderer = create(createElement(MobileBrowserPane, props(browserTab(navigation))), { + createNodeMock: () => ({ setNativeProps: () => {} }) + }) + await Promise.resolve() + }) + const mounted: ReactTestRenderer = renderer! + const viewport = mounted.root + .findAllByType('View') + .find((node) => typeof node.props.onLayout === 'function') + if (!viewport) { + throw new Error('Viewport with onLayout not found') + } + act(() => { + viewport.props.onLayout({ nativeEvent: { layout: { width: 360, height: 640 } } }) + }) + if (!listener) { + throw new Error('browser.screencast subscription not created') + } + return { + renderer: mounted, + emit: (payload) => act(() => listener?.(payload)), + update: async (next) => { + await act(async () => { + mounted.update(createElement(MobileBrowserPane, props(browserTab(next)))) + await Promise.resolve() + }) + } + } +} + +describe('MobileBrowserPane navigation enablement', () => { + it('keeps Back enabled from the tab props against a host that never reports navigation', async () => { + const { renderer, emit } = await renderPane({ canGoBack: true, canGoForward: false }) + + expect(backButtonDisabled(renderer)).toBe(false) + + // An older host emits `ready` without the navigation flags and no `navigation` event. + emit({ type: 'ready', tab: { url: 'https://dashboard.example' } }) + + expect(backButtonDisabled(renderer)).toBe(false) + }) + + it('accepts a newer host navigation event as a refinement between tab updates', async () => { + const { renderer, emit } = await renderPane({ canGoBack: true, canGoForward: false }) + + emit({ + type: 'navigation', + tab: { url: 'https://dashboard.example/start', canGoBack: false, canGoForward: false } + }) + + expect(backButtonDisabled(renderer)).toBe(true) + }) + + it('follows the tab props when an older host republishes navigability', async () => { + const { renderer, emit, update } = await renderPane({ canGoBack: false, canGoForward: false }) + + expect(backButtonDisabled(renderer)).toBe(true) + emit({ type: 'ready', tab: { url: 'https://dashboard.example' } }) + + // Without a `navigation` event, the republished tab is the only signal there is. + await update({ canGoBack: true, canGoForward: false }) + expect(backButtonDisabled(renderer)).toBe(false) + }) +}) diff --git a/mobile/src/browser/use-mobile-browser-stream.ts b/mobile/src/browser/use-mobile-browser-stream.ts index edf211f56c4..e44d061969f 100644 --- a/mobile/src/browser/use-mobile-browser-stream.ts +++ b/mobile/src/browser/use-mobile-browser-stream.ts @@ -240,10 +240,16 @@ export function useMobileBrowserStream(args: MobileBrowserStreamArgs) { return } const event = payload as ScreencastEvent - if ((event.type === 'ready' || event.type === 'navigation') && event.tab) { + // Only a payload that actually carries the flags may override the tab props; + // an older host's `ready` omits them and would pin both arrows off. + if ( + (event.type === 'ready' || event.type === 'navigation') && + typeof event.tab?.canGoBack === 'boolean' && + typeof event.tab.canGoForward === 'boolean' + ) { setNavigationState?.({ - canGoBack: event.tab.canGoBack === true, - canGoForward: event.tab.canGoForward === true + canGoBack: event.tab.canGoBack, + canGoForward: event.tab.canGoForward }) } handleBrowserScreencastEvent({ diff --git a/mobile/src/components/MobileSyntaxSegments.tsx b/mobile/src/components/MobileSyntaxSegments.tsx index 0da9fe1d438..8f63e4bdade 100644 --- a/mobile/src/components/MobileSyntaxSegments.tsx +++ b/mobile/src/components/MobileSyntaxSegments.tsx @@ -6,8 +6,9 @@ export function MobileSyntaxSegments({ segments }: { segments: MobileSyntaxSegme let sourceOffset = 0 return ( <> - {segments.map((segment) => { - const key = `${sourceOffset}:${segment.kind}` + {segments.map((segment, index) => { + // The index keeps two adjacent zero-length segments of the same kind distinct. + const key = `${index}:${sourceOffset}:${segment.kind}` sourceOffset += segment.text.length return ( diff --git a/mobile/src/components/pr-sidebar/CommentMarkdown.tsx b/mobile/src/components/pr-sidebar/CommentMarkdown.tsx index 10615cead68..ad2170c80b7 100644 --- a/mobile/src/components/pr-sidebar/CommentMarkdown.tsx +++ b/mobile/src/components/pr-sidebar/CommentMarkdown.tsx @@ -254,8 +254,9 @@ function parseInlineSafely(text: string): InlineToken[] { function keyedInlineTokens(tokens: InlineToken[]): { key: string; token: InlineToken }[] { let sourceOffset = 0 - return tokens.map((token) => { - const key = `${sourceOffset}:${token.kind}` + // The index keeps two adjacent zero-length tokens of the same kind distinct. + return tokens.map((token, index) => { + const key = `${index}:${sourceOffset}:${token.kind}` sourceOffset += token.text.length return { key, token } }) diff --git a/mobile/src/files/MobileFileExplorerPanel.tsx b/mobile/src/files/MobileFileExplorerPanel.tsx index cdd9c389dae..932797b1115 100644 --- a/mobile/src/files/MobileFileExplorerPanel.tsx +++ b/mobile/src/files/MobileFileExplorerPanel.tsx @@ -65,6 +65,12 @@ export function MobileFileExplorerPanel(props: { : null), [forceReconnect, hostId, nativeHost.client, operationsProp] ) + // Why: `operations` is null in exactly the disconnected state Retry exists for + // (no client), so the revive path cannot hang off it (#5049). + const reconnect = useCallback( + () => (operations ? operations.reconnect() : forceReconnect(hostId)), + [forceReconnect, hostId, operations] + ) const connState = connectionState ?? nativeHost.state const scopeRef = useRef('') const scope = `${hostId}:${worktreeId}` @@ -233,12 +239,12 @@ export function MobileFileExplorerPanel(props: { (relativePath: string) => { if (connState !== 'connected' && hostId) { pendingDirectoryRetriesRef.current.add(relativePath) - void operations?.reconnect() + void reconnect() return } void loadDirectory(relativePath) }, - [connState, hostId, loadDirectory, operations] + [connState, hostId, loadDirectory, reconnect] ) const previewFile = useCallback( @@ -316,9 +322,7 @@ export function MobileFileExplorerPanel(props: { - connState !== 'connected' && hostId - ? void operations?.reconnect() - : void loadDirectory('') + connState !== 'connected' && hostId ? void reconnect() : void loadDirectory('') } > Retry diff --git a/mobile/src/files/MobileFilePreviewScreen.tsx b/mobile/src/files/MobileFilePreviewScreen.tsx index bec3f3d8bfc..0d0ea508bb3 100644 --- a/mobile/src/files/MobileFilePreviewScreen.tsx +++ b/mobile/src/files/MobileFilePreviewScreen.tsx @@ -54,6 +54,16 @@ export function MobileFilePreviewScreen({ : null), [forceReconnect, nativeHost.client, operationsProp, previewHostId] ) + // Why: `operations` is null in exactly the disconnected state Retry exists for. + const reconnect = useCallback( + () => + operations + ? operations.reconnect() + : previewHostId + ? forceReconnect(previewHostId) + : Promise.resolve(), + [forceReconnect, operations, previewHostId] + ) const connState = connectionState ?? nativeHost.state const handleOpenExternalUrl = useCallback( (url: string) => { @@ -221,11 +231,11 @@ export function MobileFilePreviewScreen({ (preview.status === 'error' && preview.reconnect) || connState !== 'connected' ) { - await operations?.reconnect() + await reconnect() return } void loadPreview() - }, [connState, loadPreview, operations, preview, previewParams]) + }, [connState, loadPreview, preview, previewParams, reconnect]) const displayPath = previewParams?.source === 'terminalArtifact' diff --git a/mobile/src/mobile-web/mobile-web-native-chat-terminal-operations.ts b/mobile/src/mobile-web/mobile-web-native-chat-terminal-operations.ts index 255899217a7..795f95cbd71 100644 --- a/mobile/src/mobile-web/mobile-web-native-chat-terminal-operations.ts +++ b/mobile/src/mobile-web/mobile-web-native-chat-terminal-operations.ts @@ -113,7 +113,8 @@ export async function executeMobileWebNativeChatTerminalOperation(args: { args.client, binding.hostTerminalId!, String.fromCharCode(27), - true, + // Escape must not carry Return: the extra newline submits the agent's input line. + false, args.terminalClientId, payload.deadline, false, diff --git a/mobile/src/mobile-web/mobile-web-task-project-mutation-roundtrip.test.ts b/mobile/src/mobile-web/mobile-web-task-project-mutation-roundtrip.test.ts index d946fb5fc3a..4b20f8f10ef 100644 --- a/mobile/src/mobile-web/mobile-web-task-project-mutation-roundtrip.test.ts +++ b/mobile/src/mobile-web/mobile-web-task-project-mutation-roundtrip.test.ts @@ -167,12 +167,17 @@ it('revalidates opaque GitHub Project mutation targets before every write', asyn expect( sendRequest.mock.calls.filter(([method]) => method === 'github.project.viewTable') ).toHaveLength(18) - expect(sendRequest).toHaveBeenCalledWith('github.mergePR', { - repo: `id:host-repo-private`, - prNumber: 8, - method: 'squash', - prRepo: { owner: 'stablyai', repo: 'orca', host: 'github.com' } - }) + expect(sendRequest).toHaveBeenCalledWith( + 'github.mergePR', + { + repo: `id:host-repo-private`, + prNumber: 8, + method: 'squash', + prRepo: { owner: 'stablyai', repo: 'orca', host: 'github.com' } + }, + // A merge routinely outruns the 30s default. + { timeoutMs: 60_000 } + ) expect(sendRequest).toHaveBeenCalledWith( 'github.prChecks', { diff --git a/mobile/src/notifications/notification-route-coordination.test.ts b/mobile/src/notifications/notification-route-coordination.test.ts index bc4a2296896..ed3961c7beb 100644 --- a/mobile/src/notifications/notification-route-coordination.test.ts +++ b/mobile/src/notifications/notification-route-coordination.test.ts @@ -196,4 +196,15 @@ describe('notification route coordination', () => { // A bare push into the nested host route lands on a blank host screen (#12001). expect(notificationEffect).not.toContain('mobileHomeDestination(') }) + + it('validates the tap against the full host catalog, not just token-backed hosts', () => { + const start = rootLayoutSource.indexOf('// ─── Notification tap routing ───') + const end = rootLayoutSource.indexOf('// ─── End notification tap routing ───', start) + const notificationEffect = rootLayoutSource.slice(start, end) + + // loadHosts() omits any host whose keychain read failed, which both kills the tap + // (unknown host id) and hides the credential-recovery status. + expect(notificationEffect).toContain('resolve(data, loadHostCatalog)') + expect(notificationEffect).not.toContain('resolve(data, loadHosts)') + }) }) diff --git a/mobile/src/notifications/notification-routing.test.ts b/mobile/src/notifications/notification-routing.test.ts index 3a213b2d7a6..03504062f27 100644 --- a/mobile/src/notifications/notification-routing.test.ts +++ b/mobile/src/notifications/notification-routing.test.ts @@ -4,7 +4,8 @@ import { getNotificationNavigationPath, getNotificationNavigationTarget, LatestNotificationNavigationResolver, - resolveNotificationNavigation + resolveNotificationNavigation, + type NotificationKnownHost } from './notification-routing' describe('notification routing', () => { @@ -113,7 +114,7 @@ describe('notification routing', () => { await expect( resolveNotificationNavigation( { hostId: 'host-1', worktreeId: 'repo::/tmp/worktree' }, - async () => [{ id: 'host-1' }] + async () => [{ id: 'host-1', credentialStatus: 'ready' as const }] ) ).resolves.toEqual({ target: { @@ -125,9 +126,22 @@ describe('notification routing', () => { }) }) + it('routes a host with unreadable credentials to recovery instead of dropping the tap', async () => { + await expect( + resolveNotificationNavigation({ hostId: 'host-1', worktreeId: 'workspace-one' }, async () => [ + { id: 'host-1', credentialStatus: 'temporarily-unavailable' as const } + ]) + ).resolves.toMatchObject({ target: { credentialRecovery: 'retry' } }) + await expect( + resolveNotificationNavigation({ hostId: 'host-1' }, async () => [ + { id: 'host-1', credentialStatus: 'missing' as const } + ]) + ).resolves.toMatchObject({ target: { credentialRecovery: 're-pair' } }) + }) + it('suppresses an older tap whose paired-host read finishes after a newer tap', async () => { - const firstHosts = deferred() - const secondHosts = deferred() + const firstHosts = deferred() + const secondHosts = deferred() const resolver = new LatestNotificationNavigationResolver() const first = resolver.resolve( { hostId: 'host-1', worktreeId: 'workspace-one' }, @@ -138,11 +152,11 @@ describe('notification routing', () => { () => secondHosts.promise ) - secondHosts.resolve([{ id: 'host-1' }]) + secondHosts.resolve([{ id: 'host-1', credentialStatus: 'ready' }]) await expect(second).resolves.toMatchObject({ target: { kind: 'session', hostWorkspaceId: 'workspace-two' } }) - firstHosts.resolve([{ id: 'host-1' }]) + firstHosts.resolve([{ id: 'host-1', credentialStatus: 'ready' }]) await expect(first).resolves.toBeNull() }) }) diff --git a/mobile/src/notifications/notification-routing.ts b/mobile/src/notifications/notification-routing.ts index b76ac35f32d..cafcdb236df 100644 --- a/mobile/src/notifications/notification-routing.ts +++ b/mobile/src/notifications/notification-routing.ts @@ -71,11 +71,16 @@ export function getNotificationNavigationPath( return getNotificationNavigationTargetPath(target) } +// Why: the credential status must be required, not optional — a loader that only knows +// device-token-backed hosts (loadHosts) silently drops unreadable ones from knownHostIds +// and kills the tap instead of routing it to credential recovery. +export type NotificationKnownHost = { id: string; credentialStatus: HostCredentialStatus } + export async function resolveNotificationNavigation( data: unknown, - loadKnownHosts: () => Promise + loadKnownHosts: () => Promise ): Promise { - let hosts: readonly { id: string; credentialStatus?: HostCredentialStatus }[] + let hosts: readonly NotificationKnownHost[] try { hosts = await loadKnownHosts() } catch { @@ -98,7 +103,7 @@ export class LatestNotificationNavigationResolver { async resolve( data: unknown, - loadKnownHosts: () => Promise + loadKnownHosts: () => Promise ): Promise { const sequence = ++this.latestSequence const navigation = await resolveNotificationNavigation(data, loadKnownHosts) diff --git a/mobile/src/session/MobileSessionCommandDock.tsx b/mobile/src/session/MobileSessionCommandDock.tsx index d009d1a0624..f9dd9e6dc5e 100644 --- a/mobile/src/session/MobileSessionCommandDock.tsx +++ b/mobile/src/session/MobileSessionCommandDock.tsx @@ -196,9 +196,9 @@ export function MobileSessionCommandDock({ controller }: { controller: MobileSes if (!key.repeatable) { return } - const input = createTerminalLiveAccessoryInput(key) - void handleAccessoryKey(input) - startAccessoryRepeat(input) + // startAccessoryRepeat owns the press-time send; sending here too + // emits the key twice per tap. + startAccessoryRepeat(createTerminalLiveAccessoryInput(key)) }} onPressOut={() => { if (key.repeatable) { diff --git a/mobile/src/session/accessory-key-press-send-ownership.test.ts b/mobile/src/session/accessory-key-press-send-ownership.test.ts new file mode 100644 index 00000000000..25676f0b90c --- /dev/null +++ b/mobile/src/session/accessory-key-press-send-ownership.test.ts @@ -0,0 +1,32 @@ +import { readFileSync } from 'node:fs' +import { describe, expect, it } from 'vitest' + +const commandDockSource = readFileSync( + new URL('./MobileSessionCommandDock.tsx', import.meta.url), + 'utf8' +) + +function repeatablePressInBlock(): string { + const start = commandDockSource.indexOf('onPressIn={() => {') + expect(start).toBeGreaterThanOrEqual(0) + const end = commandDockSource.indexOf('onPressOut={', start) + expect(end).toBeGreaterThan(start) + return commandDockSource.slice(start, end) +} + +describe('accessory key press send ownership', () => { + // startAccessoryRepeat's controller sends at press time and then repeats; a second + // press-time send in the dock emitted every arrow/Backspace/Tab tap twice. + it('gives the repeat controller sole ownership of the press-time send', () => { + const block = repeatablePressInBlock() + + expect(block).toContain('startAccessoryRepeat(createTerminalLiveAccessoryInput(key))') + expect(block).not.toContain('handleAccessoryKey(') + }) + + it('still sends non-repeatable keys once on release', () => { + expect(commandDockSource).toContain( + 'void handleAccessoryKey(createTerminalLiveAccessoryInput(key))' + ) + }) +}) diff --git a/mobile/src/session/github-pr-mutations.ts b/mobile/src/session/github-pr-mutations.ts index bd1d9c423b9..aa0852fb503 100644 --- a/mobile/src/session/github-pr-mutations.ts +++ b/mobile/src/session/github-pr-mutations.ts @@ -52,10 +52,14 @@ function extractMutationError(error: unknown, method: string): string { async function sendGithubPrMutation( client: Pick, method: string, - params: Record + params: Record, + options?: { timeoutMs?: number } ): Promise { try { - const response = await client.sendRequest(method, params) + // Keep the two-argument call shape when no timeout is requested. + const response = options + ? await client.sendRequest(method, params, options) + : await client.sendRequest(method, params) if (!response.ok) { return { ok: false, error: response.error?.message || `Request failed: ${method}` } } @@ -79,7 +83,8 @@ async function sendGithubPrMutation( export async function fetchMergePR( client: Pick, worktreeId: string, - args: { prNumber: number; method?: GitHubPRMergeMethod; prRepo?: GitHubPrRepoSlug | null } + args: { prNumber: number; method?: GitHubPRMergeMethod; prRepo?: GitHubPrRepoSlug | null }, + options?: { timeoutMs?: number } ): Promise { const params: Record = { prNumber: args.prNumber } if (args.method) { @@ -88,7 +93,8 @@ export async function fetchMergePR( return sendGithubPrMutation( client, 'github.mergePR', - buildGithubPrParams('github.mergePR', worktreeId, params, { prRepo: args.prRepo }) + buildGithubPrParams('github.mergePR', worktreeId, params, { prRepo: args.prRepo }), + options ) } @@ -312,7 +318,8 @@ export async function fetchRerunPRChecks( headSha?: string | null failedOnly?: boolean prRepo?: GitHubPrRepoSlug | null - } + }, + options?: { timeoutMs?: number } ): Promise { const params: Record = { prNumber: args.prNumber } if (args.failedOnly !== undefined) { @@ -324,6 +331,7 @@ export async function fetchRerunPRChecks( return sendGithubPrMutation( client, 'github.rerunPRChecks', - buildGithubPrParams('github.rerunPRChecks', worktreeId, params, { prRepo: args.prRepo }) + buildGithubPrParams('github.rerunPRChecks', worktreeId, params, { prRepo: args.prRepo }), + options ) } diff --git a/mobile/src/session/mobile-file-tap-open.test.ts b/mobile/src/session/mobile-file-tap-open.test.ts index a25e7ba28cd..cc7afeef094 100644 --- a/mobile/src/session/mobile-file-tap-open.test.ts +++ b/mobile/src/session/mobile-file-tap-open.test.ts @@ -113,6 +113,87 @@ describe('openMobileFileTap', () => { expect(switchSessionTab).toHaveBeenCalledWith(openedTab) }) + it('opens a sibling-workspace path through the resolved owning workspace', async () => { + const operations = createOperations([ + { + kind: 'worktree-file' as const, + relativePath: 'docs/readme.md', + localAbsolutePath: '/repo-b/docs/readme.md', + workspaceId: 'wt-2' + } + ]) + const pushPreviewRoute = vi.fn() + + openMobileTerminalFileTap({ + operations, + hostId: 'host-1', + worktreeId: 'wt-1', + worktreeName: 'workspace one', + pathText: '/repo-b/docs/readme.md', + terminalHandle: 'terminal-1', + line: null, + column: null, + pushPreviewRoute, + openBrowser: vi.fn(), + triggerOpenFeedback: vi.fn(), + fetchSessionTabs: vi.fn(), + getSessionTabs: () => [], + getActiveSessionTabId: () => null, + getActivationState: activeTerminalState, + switchSessionTab: vi.fn(), + scheduleDelayedAction: vi.fn() + }) + await Promise.resolve() + await Promise.resolve() + + // Opening it in this session's workspace would hit a same-named file or nothing. + expect(pushPreviewRoute).toHaveBeenCalledWith({ + pathname: '/h/[hostId]/files/preview/[worktreeId]', + params: expect.objectContaining({ + hostId: 'host-1', + worktreeId: 'wt-2', + source: 'worktree', + relativePath: 'docs/readme.md' + }) + }) + expect(pushPreviewRoute.mock.calls[0]?.[0].params).not.toHaveProperty('worktreeName') + expect(operations.openWorktreeFile).not.toHaveBeenCalled() + }) + + it('addresses files.open at the workspace the host resolved', async () => { + const operations = createOperations([ + { + kind: 'worktree-file' as const, + relativePath: 'src/index.ts', + localAbsolutePath: '/repo/src/index.ts', + workspaceId: 'wt-1' + } + ]) + + openMobileTerminalFileTap({ + operations, + hostId: 'host-1', + worktreeId: 'wt-1', + pathText: 'src/index.ts', + terminalHandle: 'terminal-1', + line: null, + column: null, + pushPreviewRoute: vi.fn(), + openBrowser: vi.fn(), + triggerOpenFeedback: vi.fn(), + fetchSessionTabs: vi.fn(), + getSessionTabs: () => [], + getActiveSessionTabId: () => 'terminal-tab', + getActivationState: activeTerminalState, + switchSessionTab: vi.fn(), + scheduleDelayedAction: vi.fn() + }) + await Promise.resolve() + await Promise.resolve() + + expect(operations.openWorktreeFile).toHaveBeenCalledWith('wt-1', 'src/index.ts') + }) + it('opens the file when optional haptic feedback is unavailable', async () => { const operations = createOperations([worktreeTarget('README.md', '/repo/README.md')]) diff --git a/mobile/src/session/mobile-file-tap-open.ts b/mobile/src/session/mobile-file-tap-open.ts index 2ea2d5b75a4..71560dfeb91 100644 --- a/mobile/src/session/mobile-file-tap-open.ts +++ b/mobile/src/session/mobile-file-tap-open.ts @@ -132,7 +132,13 @@ async function openMobileFileTapAsync( const openedPath = resolved.relativePath triggerMobileTerminalOpenFeedback(options.triggerOpenFeedback) - if (options.line !== null || options.column !== null) { + // A sibling-workspace hit has no tab in this session to open into, so it must go + // to the preview route addressed at the workspace that actually holds the file. + if ( + resolvedWorktreeId !== options.worktreeId || + options.line !== null || + options.column !== null + ) { options.pushPreviewRoute( createMobileFilePreviewHref({ hostId: options.hostId, @@ -151,7 +157,7 @@ async function openMobileFileTapAsync( options.openBrowser(filesystemPathToFileUri(resolved.localAbsolutePath)) return } - await options.operations.openWorktreeFile(options.worktreeId, openedPath) + await options.operations.openWorktreeFile(resolvedWorktreeId, openedPath) scheduleOpenedWorktreeTabActivation(options, openedPath) } diff --git a/mobile/src/session/mobile-native-chat-eligibility.ts b/mobile/src/session/mobile-native-chat-eligibility.ts index 04b1fc2733e..064a33cfbda 100644 --- a/mobile/src/session/mobile-native-chat-eligibility.ts +++ b/mobile/src/session/mobile-native-chat-eligibility.ts @@ -35,6 +35,8 @@ export type MobileNativeChatTab = { export type MobileNativeChatAgentStatusWithProvider = MobileWebNativeChatAgentStatus & { model?: string + /** Host flag marking `lastAssistantMessage` as tool output rather than a reply. */ + lastAssistantMessageIsToolOutput?: boolean providerSession?: { id: string transcriptPath?: string diff --git a/mobile/src/session/mobile-native-chat-streaming-gate.test.ts b/mobile/src/session/mobile-native-chat-streaming-gate.test.ts index 312be591ca9..401aaee14ca 100644 --- a/mobile/src/session/mobile-native-chat-streaming-gate.test.ts +++ b/mobile/src/session/mobile-native-chat-streaming-gate.test.ts @@ -3,6 +3,7 @@ import type { NativeChatMessage } from '../../../src/shared/native-chat-types' import { createMobileNativeChatStreamingGate, deriveMobileNativeChatStreaming, + mobileNativeChatStreamPreview, type MobileNativeChatStreamingGate } from './mobile-native-chat-streaming-gate' @@ -33,6 +34,40 @@ function run(ticks: { folded: NativeChatMessage[]; text?: string; live?: boolean return { gate, results } } +describe('mobileNativeChatStreamPreview', () => { + it('drops a preview the provider flagged as tool output', () => { + // Regression: a Bash result was published as `lastAssistantMessage` for the status + // card, then rendered here as an un-collapsed assistant bubble that no catch-up rule + // could retire, so it sat in the chat for the rest of the turn. + expect( + mobileNativeChatStreamPreview( + { + lastAssistantMessage: 'Exit code 1\nimport { Foo }', + lastAssistantMessageIsToolOutput: true + }, + true + ) + ).toBeUndefined() + }) + + it('passes assistant prose through while working', () => { + expect(mobileNativeChatStreamPreview({ lastAssistantMessage: 'Working on it' }, true)).toBe( + 'Working on it' + ) + }) + + it('drops any preview once the turn is not working', () => { + expect( + mobileNativeChatStreamPreview({ lastAssistantMessage: 'Working on it' }, false) + ).toBeUndefined() + }) + + it('tolerates a missing status', () => { + expect(mobileNativeChatStreamPreview(null, true)).toBeUndefined() + expect(mobileNativeChatStreamPreview(undefined, true)).toBeUndefined() + }) +}) + describe('deriveMobileNativeChatStreaming', () => { it('shows a genuine reply that repeats the previous turn as a prefix', () => { const prior = [assistant('a1', 'The tests pass.')] diff --git a/mobile/src/session/mobile-native-chat-streaming-gate.ts b/mobile/src/session/mobile-native-chat-streaming-gate.ts index a857803214c..2fcaa9dd552 100644 --- a/mobile/src/session/mobile-native-chat-streaming-gate.ts +++ b/mobile/src/session/mobile-native-chat-streaming-gate.ts @@ -18,6 +18,22 @@ export type MobileNativeChatStreamingGate = { baselineTailId: string | null } +/** The live status line doubles as a tool-output mirror: when the host marks the + * last assistant message as tool stdout/stderr there is no reply to preview, and + * rendering it paints raw command output as a streaming chat bubble. */ +export function mobileNativeChatStreamPreview( + status: + | { lastAssistantMessage?: string; lastAssistantMessageIsToolOutput?: boolean } + | null + | undefined, + working: boolean +): string | undefined { + if (!working || status?.lastAssistantMessageIsToolOutput === true) { + return undefined + } + return status?.lastAssistantMessage +} + export function createMobileNativeChatStreamingGate( scopeKey: string | null = null ): MobileNativeChatStreamingGate { diff --git a/mobile/src/session/mobile-session-route-parity.test.ts b/mobile/src/session/mobile-session-route-parity.test.ts index c9a2dd6191e..d269aee1896 100644 --- a/mobile/src/session/mobile-session-route-parity.test.ts +++ b/mobile/src/session/mobile-session-route-parity.test.ts @@ -84,7 +84,10 @@ const HEAD_TIMER_CREATION_SHA256 = const HEAD_TIMER_CLEANUP_SHA256 = '8a45ae3c8a01a639a40ffaf3c0fc89a2e0b610623306818c86bad4ef9195b824' const HEAD_RUNTIME_STRING_SHA256 = '77fce1bf3cd5c150255a191a107569b11d334da95d3cb47459189965f57f901b' -const HEAD_HOST_JSX_SHA256 = '2911efcb57dbc9f6de1f2a7b3ed6ca4fa8a9735d48649fe062cb400df756e1ce' +// Re-frozen when the repeatable accessory key's press handler dropped its duplicate +// handleAccessoryKey call: startAccessoryRepeat already sends at press time, so every +// tap emitted the key twice. Same element count, one attribute body changed. +const HEAD_HOST_JSX_SHA256 = '5b6acbcb34eaa59aa0020f7d6337ccbeb40b3195ff0d798911d9c035d49042fa' const HEAD_LEAF_JSX_SHA256 = '7551bacf163f59c150cc8a9150c443df9804a882365f459053d3ab73ac557f42' const HEAD_STYLE_REFERENCE_SHA256 = '3e4f57e5c8691d443187ffe306eae28506d5505276ea3de7a4f2f1df1cfa3885' diff --git a/mobile/src/session/native-host-session-native-chat-operations.test.ts b/mobile/src/session/native-host-session-native-chat-operations.test.ts new file mode 100644 index 00000000000..d9ee2dd0d4e --- /dev/null +++ b/mobile/src/session/native-host-session-native-chat-operations.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it, vi } from 'vitest' +import type { RpcClient } from '../transport/rpc-client' +import type { HostSessionNativeChatTarget } from './host-session-native-chat-operations' +import { nativeHostSessionNativeChatOperations } from './native-host-session-native-chat-operations' + +function target(overrides: Partial = {}): HostSessionNativeChatTarget { + return { + workspaceId: 'wt-1', + agent: 'claude', + sessionId: 'session-1', + transcriptPath: null, + terminalId: 'terminal-1', + clientId: 'device-1', + ...overrides + } +} + +function client(sendRequest: RpcClient['sendRequest']): RpcClient { + return { sendRequest } as unknown as RpcClient +} + +describe('native host session native chat operations', () => { + it('stops the agent with a bare Escape that cannot submit the input line', async () => { + const sendRequest = vi.fn().mockResolvedValue({ + ok: true, + result: { delivered: true } + }) + const operations = nativeHostSessionNativeChatOperations(client(sendRequest)) + + await operations.stop(target(), Date.now() + 15_000) + + expect(sendRequest).toHaveBeenCalledWith( + 'terminal.send', + expect.objectContaining({ text: String.fromCharCode(27), enter: false }), + expect.anything() + ) + }) + + it('keeps the legacy file inventory scoped to the workspace that produced it', async () => { + const sendRequest = vi.fn(async (method, params) => { + if (method === 'files.searchPaths') { + return { ok: false, error: { code: 'method_not_found', message: 'unsupported' } } + } + const worktree = (params as { worktree: string }).worktree + return { + ok: true, + result: { + files: + worktree === 'id:wt-1' + ? [{ relativePath: 'alpha/one.ts' }] + : [{ relativePath: 'beta/two.ts' }] + } + } + }) + const operations = nativeHostSessionNativeChatOperations(client(sendRequest)) + + await expect(operations.searchFiles(target(), 'o')).resolves.toEqual(['alpha/one.ts']) + // A second workspace must re-read; the first workspace's inventory is not its own. + await expect(operations.searchFiles(target({ workspaceId: 'wt-2' }), 'o')).resolves.toEqual([ + 'beta/two.ts' + ]) + // The first workspace still answers from its cached inventory. + await expect(operations.searchFiles(target(), 'o')).resolves.toEqual(['alpha/one.ts']) + expect(sendRequest.mock.calls.filter(([method]) => method === 'files.list')).toHaveLength(2) + }) +}) diff --git a/mobile/src/session/native-host-session-native-chat-operations.ts b/mobile/src/session/native-host-session-native-chat-operations.ts index 28c75705991..89c36a176b0 100644 --- a/mobile/src/session/native-host-session-native-chat-operations.ts +++ b/mobile/src/session/native-host-session-native-chat-operations.ts @@ -22,8 +22,10 @@ export function nativeHostSessionNativeChatOperations( client: RpcClient ): HostSessionNativeChatOperations { let searchSupported: boolean | null = null - let legacyPaths: string[] | null = null - let legacyLoad: Promise | null = null + // Why: the legacy full-inventory fallback is per workspace — one shared list makes + // `@` autocomplete in a second workspace suggest the first workspace's files. + const legacyPathsByWorkspace = new Map() + const legacyLoadByWorkspace = new Map>() return { async readability(workspaceId) { if (isFloatingWorkspaceWorktreeId(workspaceId)) { @@ -92,7 +94,9 @@ export function nativeHostSessionNativeChatOperations( return sendNative(target, text, enter, client, deadline) }, stop(target, deadline) { - return sendNative(target, escape(), true, client, deadline) + // Escape must not carry Return: the extra newline submits whatever the agent + // had parked on its input line. + return sendNative(target, escape(), false, client, deadline) }, async searchFiles(target, query) { if (searchSupported !== false) { @@ -110,22 +114,28 @@ export function nativeHostSessionNativeChatOperations( } searchSupported = false } + let legacyPaths = legacyPathsByWorkspace.get(target.workspaceId) if (!legacyPaths) { + let legacyLoad = legacyLoadByWorkspace.get(target.workspaceId) if (!legacyLoad) { + // Older hosts expose only the full inventory RPC; overlapping queries must + // share one slow local/SSH read. legacyLoad = client .sendRequest('files.list', { worktree: `id:${target.workspaceId}` }) .then((response) => (response.ok ? extractPaths(response.result) : null)) .finally(() => { - legacyLoad = null + legacyLoadByWorkspace.delete(target.workspaceId) }) + legacyLoadByWorkspace.set(target.workspaceId, legacyLoad) } const paths = await legacyLoad if (!paths) { return [] } legacyPaths = paths + legacyPathsByWorkspace.set(target.workspaceId, paths) } return rankSuggestions(legacyPaths, query, FILE_RESULT_LIMIT) }, diff --git a/mobile/src/session/use-mobile-native-chat-controller.ts b/mobile/src/session/use-mobile-native-chat-controller.ts index fbbf2a69896..c249a8a1a13 100644 --- a/mobile/src/session/use-mobile-native-chat-controller.ts +++ b/mobile/src/session/use-mobile-native-chat-controller.ts @@ -16,6 +16,7 @@ import { useMobileNativeChatTarget } from './use-mobile-native-chat-target' import { useNativeChatAcceptedAction } from './use-native-chat-action-outcomes' import { useThrottledLatestValue } from './use-throttled-latest-value' import { isMobileNativeChatAgentWorking } from './mobile-native-chat-working-state' +import { mobileNativeChatStreamPreview } from './mobile-native-chat-streaming-gate' import type { HostSessionChatDraftOperations } from './host-session-chat-draft-operations' import type { HostSessionChatPendingDeliveryOperations } from './host-session-chat-pending-delivery-operations' import type { MobileNativeChatController } from './mobile-native-chat-controller-contract' @@ -159,9 +160,9 @@ export function useMobileNativeChatController(args: { // Throttle the streaming bubble: OpenCode emits a status frame per streamed // part, and each one re-renders and re-parses the whole accumulated markdown. const nativeChatStreamingText = useThrottledLatestValue( - nativeChatAgentWorking && !activeChatStructured - ? nativeChatStatus?.lastAssistantMessage - : undefined, + activeChatStructured + ? undefined + : mobileNativeChatStreamPreview(nativeChatStatus, nativeChatAgentWorking), NATIVE_CHAT_STREAM_THROTTLE_MS ) const { diff --git a/mobile/src/session/use-mobile-native-chat-drafts.test.ts b/mobile/src/session/use-mobile-native-chat-drafts.test.ts index 38fdfcd4efd..74a7ccfd03d 100644 --- a/mobile/src/session/use-mobile-native-chat-drafts.test.ts +++ b/mobile/src/session/use-mobile-native-chat-drafts.test.ts @@ -340,6 +340,39 @@ describe('useMobileNativeChatDrafts', () => { ]) }) + it('normalizes a multi-line prompt so its transcript echo can retire it', async () => { + const prompt = 'first line\nsecond line' + // The transcript records the prompt with whitespace runs collapsed. + const echo = userTextMessage('m1', 'first line second line') + await mount('a') + // An identical earlier turn must be counted, or the pending retires against it. + await act(async () => + renderer?.update(createElement(Harness, { tabId: 'a', messages: [echo] })) + ) + expect(state?.captureSendOrigin(prompt)).toMatchObject({ + normalizedText: 'first line second line', + baselineOccurrences: 1 + }) + + const origin = state?.captureSendOrigin(prompt) + act(() => { + if (origin) { + state?.acceptSend(origin, prompt) + } + }) + expect(state?.pending.map((pending) => pending.text)).toEqual([prompt]) + + await act(async () => + renderer?.update( + createElement(Harness, { + tabId: 'a', + messages: [echo, userTextMessage('m2', 'first line second line')] + }) + ) + ) + expect(state?.pending).toEqual([]) + }) + it('clears one pending per landed message so duplicate sends are not all dropped', async () => { await mount('a') const origin = state?.captureSendOrigin('ping') diff --git a/mobile/src/session/use-mobile-native-chat-drafts.ts b/mobile/src/session/use-mobile-native-chat-drafts.ts index 2af4265cfbe..6a0477bebe5 100644 --- a/mobile/src/session/use-mobile-native-chat-drafts.ts +++ b/mobile/src/session/use-mobile-native-chat-drafts.ts @@ -4,6 +4,7 @@ import type { HostSessionChatDraftOperations } from './host-session-chat-draft-o import type { HostSessionChatPendingDeliveryOperations } from './host-session-chat-pending-delivery-operations' import { findLandedUnconfirmedSends, + normalizeReconcileText, type UnconfirmedSend } from './mobile-native-chat-draft-reconcile' import { mobileNativeChatScopeKey } from './mobile-native-chat-scope-key' @@ -149,7 +150,9 @@ export function useMobileNativeChatDrafts(args: { ? { draftKey, draftEditGeneration: draftEditGenerationsRef.current.readDraft(draftKey), - ...capturePendingOrigin(text.trim()) + // Transcript rows are compared fully normalized (ANSI stripped, whitespace + // runs collapsed); a bare trim leaves every multi-line prompt unmatchable. + ...capturePendingOrigin(normalizeReconcileText(text)) } : null, [capturePendingOrigin, draftKey] diff --git a/mobile/src/session/use-mobile-native-chat-pending-deliveries.ts b/mobile/src/session/use-mobile-native-chat-pending-deliveries.ts index 4e966f6bb15..80a83e40a72 100644 --- a/mobile/src/session/use-mobile-native-chat-pending-deliveries.ts +++ b/mobile/src/session/use-mobile-native-chat-pending-deliveries.ts @@ -6,7 +6,8 @@ import { countUserTextOccurrences, findLandedImagePreviewEchoes, mergeLandedImagePreviewEchoes, - migrateImagePreviewMessageIds + migrateImagePreviewMessageIds, + normalizeReconcileText } from './mobile-native-chat-draft-reconcile' import { rebaseMobileNativeChatPendingBaselines } from './mobile-native-chat-pending-baseline' import { retireLandedMobileNativeChatPending } from './mobile-native-chat-pending-retirement' @@ -192,7 +193,7 @@ export function useMobileNativeChatPendingDeliveries(args: { const current = pendingBySessionRef.current[storageKey] ?? NO_PENDING_MESSAGES const earlierOutstanding = current.filter( (pending) => - pending.text.trim() === origin.normalizedText && + normalizeReconcileText(pending.text) === origin.normalizedText && pending.expectedOccurrence > origin.baselineOccurrences ).length const expectedImageEchoOrdinal = diff --git a/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts b/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts index b5a8f643baf..59fbee99d31 100644 --- a/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts +++ b/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts @@ -36,12 +36,18 @@ const hash = (parts: string[] | string): string => * * Declarations dropped from 196 to 195 when projectRowGitHubRepository was * deleted: it carried over from the monolith with no caller in either shape. + * + * Re-frozen when the monolith's `nameIsAutoManaged` computation was restored to the + * workspace-create action (it had been dropped, so a typed workspace name was + * discarded) and `toggleGitHubProjectFieldVisibility` went back to the monolith's + * functional state updater. Both move the composition toward the baseline, not away: + * one added statement, one changed callback body, no hook or declaration count change. */ -const SCREEN_HOOKS = '8ba3974a4c4c26bd04aa22cb0e59757c98ebda4a91ab6b7081215c5e8710d0b3' +const SCREEN_HOOKS = 'c5026e3f5633d36eae56f0492c5b239896f0b6d363079be1568de6460d7eae30' const DIFF_HOOKS = '93c7189b32bed8456cc51814fffa8ce80cf62011ef968a9d53ddec2b9686f58f' -const STATEMENTS = '02ef31a6b6a30748c41e485dbac8fb4e4433e887ecdbd2db5442a81c31bdf244' +const STATEMENTS = '300258f651ed5aec07966cd4a382597ee21b381764aaa6222d18ce35b2600dd3' const DECLARATIONS = '11ddb68df1bdde8ee3299bb391de14765e6860b8406d0c17f837223e3667bd13' -const SEMANTICS = '541a22531e77209f9c9fea32b49cb0539a8e1bd2e6b74ca7e5e1de0642f028c9' +const SEMANTICS = '990736b2b2b450230fca06bb27ed97903d9827c59ad7e4c7be442a0d854f5750' const STYLES = '1db6af69c791d9963928541ad5310942fcbda6d984b422c90b6eb92b6816579a' const RENDER_TREE = '92596eb283232607d8c2df3f09ba970232c7df496555c6f59e0c7160a00501af' @@ -70,7 +76,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_236) + expect(semantics.split('\n')).toHaveLength(3_237) expect(hash(semantics)).toBe(SEMANTICS) }) diff --git a/mobile/src/tasks/native-host-task-item-file-operations.ts b/mobile/src/tasks/native-host-task-item-file-operations.ts index 53ed28cff2c..0a69cff9e7a 100644 --- a/mobile/src/tasks/native-host-task-item-file-operations.ts +++ b/mobile/src/tasks/native-host-task-item-file-operations.ts @@ -3,13 +3,17 @@ import type { RpcClient } from '../transport/rpc-client' export function nativeHostTaskItemFileOperations(client: RpcClient): HostTaskItemFileOperations { return { - refreshChecks(target, headSha) { - return request(client, 'github.prChecks', { + async refreshChecks(target, headSha) { + const checks = await request(client, 'github.prChecks', { ...repoPayload(target), prNumber: target.number, headSha, noCache: true }) + if (!Array.isArray(checks)) { + throw new Error('Invalid checks response') + } + return checks }, async rerunChecks(target, headSha, failedOnly) { const result = await request<{ ok?: boolean; error?: string }>( diff --git a/mobile/src/tasks/native-host-task-list-operations.ts b/mobile/src/tasks/native-host-task-list-operations.ts index 45b9c06f1eb..ad3dbc51c22 100644 --- a/mobile/src/tasks/native-host-task-list-operations.ts +++ b/mobile/src/tasks/native-host-task-list-operations.ts @@ -41,7 +41,11 @@ export function nativeHostTaskListOperations(client: RpcClient): HostTaskListOpe ) }, async listGitLabTodos(repoId) { - return successfulResult(client.sendRequest('gitlab.todos', { repo: `id:${repoId}` })) + // An empty todo list comes back as null from older hosts; the seam promises an array. + const todos = await successfulResult( + client.sendRequest('gitlab.todos', { repo: `id:${repoId}` }) + ) + return Array.isArray(todos) ? todos : [] }, async listLinear(payload) { const response = payload.query diff --git a/mobile/src/tasks/native-host-task-operation-contracts.test.ts b/mobile/src/tasks/native-host-task-operation-contracts.test.ts new file mode 100644 index 00000000000..99d2bcd2d85 --- /dev/null +++ b/mobile/src/tasks/native-host-task-operation-contracts.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it, vi } from 'vitest' +import type { RpcClient } from '../transport/rpc-client' +import { nativeHostTaskItemFileOperations } from './native-host-task-item-file-operations' +import { nativeHostTaskListOperations } from './native-host-task-list-operations' +import { nativeHostTaskProjectFileOperations } from './native-host-task-project-file-operations' +import { nativeHostTaskProjectMutationOperations } from './native-host-task-project-mutation-operations' + +function client(sendRequest: RpcClient['sendRequest']): RpcClient { + return { sendRequest } as unknown as RpcClient +} + +const itemTarget = { repoId: 'repo-1', number: 7 } +const projectTarget = { number: 7, slug: { owner: 'orca', repo: 'orca' }, type: 'pr' as const } + +describe('native host task operation contracts', () => { + it('rejects a non-array checks payload instead of crashing the checks list', async () => { + const sendRequest = vi + .fn() + .mockResolvedValue({ ok: true, result: { error: 'rate limited' } }) + + await expect( + nativeHostTaskItemFileOperations(client(sendRequest)).refreshChecks(itemTarget, 'sha') + ).rejects.toThrow('Invalid checks response') + await expect( + nativeHostTaskProjectFileOperations(client(sendRequest)).refreshChecks( + projectTarget, + 'repo-1', + 'sha' + ) + ).rejects.toThrow('Invalid checks response') + }) + + it('reads an absent GitLab todo list as empty rather than an error banner', async () => { + const sendRequest = vi + .fn() + .mockResolvedValue({ ok: true, result: null }) + + await expect( + nativeHostTaskListOperations(client(sendRequest)).listGitLabTodos('repo-1') + ).resolves.toEqual([]) + }) + + it('gives project merge and rerun the long timeout their item-level twins use', async () => { + const sendRequest = vi.fn().mockResolvedValue({ + ok: true, + result: { ok: true } + }) + const operations = nativeHostTaskProjectMutationOperations(client(sendRequest)) + + await operations.rerunChecks(projectTarget, 'repo-1', { failedOnly: true }) + await operations.merge(projectTarget, 'repo-1', 'squash') + + for (const call of sendRequest.mock.calls) { + expect(call[2]).toEqual({ timeoutMs: 60_000 }) + } + }) +}) 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 d5b47da30e8..0f76b6aae8d 100644 --- a/mobile/src/tasks/native-host-task-project-file-operations.ts +++ b/mobile/src/tasks/native-host-task-project-file-operations.ts @@ -7,12 +7,16 @@ export function nativeHostTaskProjectFileOperations( ): HostTaskProjectFileOperations { return { async refreshChecks(target, repoId, headSha) { - return request(client, 'github.prChecks', { + const checks = await request(client, 'github.prChecks', { ...repoPayload(target, repoId), prNumber: target.number, headSha, noCache: true }) + if (!Array.isArray(checks)) { + throw new Error('Invalid checks response') + } + return checks }, async setFileViewed(target, repoId, payload) { const result = await request(client, 'github.setPRFileViewed', { 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 cb40f10c6ef..8c713385522 100644 --- a/mobile/src/tasks/native-host-task-project-mutation-operations.ts +++ b/mobile/src/tasks/native-host-task-project-mutation-operations.ts @@ -13,6 +13,8 @@ import { fetchResolveReviewThread } from '../session/github-pr-mutations' +const PROJECT_PR_MUTATION_TIMEOUT_MS = 60_000 + export function nativeHostTaskProjectMutationOperations( client: RpcClient ): HostTaskProjectMutationOperations { @@ -116,20 +118,23 @@ export function nativeHostTaskProjectMutationOperations( }, async rerunChecks(target, repoId, payload) { requirePrMutation( - await fetchRerunPRChecks(client, repoId, { - prNumber: target.number, - ...payload, - prRepo: slugPayload(target) - }) + await fetchRerunPRChecks( + client, + repoId, + { prNumber: target.number, ...payload, prRepo: slugPayload(target) }, + // A CI rerun and a merge both routinely outrun the 30s default. + { timeoutMs: PROJECT_PR_MUTATION_TIMEOUT_MS } + ) ) }, async merge(target, repoId, method) { requirePrMutation( - await fetchMergePR(client, repoId, { - prNumber: target.number, - method, - prRepo: slugPayload(target) - }) + await fetchMergePR( + client, + repoId, + { prNumber: target.number, method, prRepo: slugPayload(target) }, + { timeoutMs: PROJECT_PR_MUTATION_TIMEOUT_MS } + ) ) } } diff --git a/mobile/src/tasks/task-workspace-create-name-authority.test.ts b/mobile/src/tasks/task-workspace-create-name-authority.test.ts new file mode 100644 index 00000000000..e6f25258e41 --- /dev/null +++ b/mobile/src/tasks/task-workspace-create-name-authority.test.ts @@ -0,0 +1,23 @@ +import { readFileSync } from 'node:fs' +import { describe, expect, it } from 'vitest' + +const createActionsSource = readFileSync( + new URL('./use-mobile-tasks-workspace-create-actions.tsx', import.meta.url), + 'utf8' +) + +describe('task workspace create name authority', () => { + // Without this flag the host defaults nameIsAutoManaged to true and drops the typed + // name, publishing the generated one with displayNameKind 'generated'. + it('threads the typed-name decision into the create call', () => { + expect(createActionsSource).toContain( + 'const nameIsAutoManaged =\n !trimmedWorkspaceName || trimmedWorkspaceName === workspaceLastAutoName' + ) + expect(createActionsSource).toContain('nameIsAutoManaged,') + }) + + it('keeps the last auto-generated name in scope and in the callback deps', () => { + const deps = createActionsSource.slice(createActionsSource.lastIndexOf(' [')) + expect(deps).toContain('workspaceLastAutoName') + }) +}) diff --git a/mobile/src/tasks/use-mobile-tasks-client-settings-actions.tsx b/mobile/src/tasks/use-mobile-tasks-client-settings-actions.tsx index ef9a685ff35..09bf37f6577 100644 --- a/mobile/src/tasks/use-mobile-tasks-client-settings-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-client-settings-actions.tsx @@ -12,7 +12,6 @@ export function useMobileTasksClientSettingsActions(model: ProjectRepositoryReso const { defaultRepoSelectionRef, githubProjectFieldVisibilityScope, - githubProjectHiddenFieldIdsByView, repoSelectionHydratedRef, setDefaultGitHubPreset, setGithubCurrentPage, @@ -104,24 +103,25 @@ export function useMobileTasksClientSettingsActions(model: ProjectRepositoryReso if (!githubProjectFieldVisibilityScope) { return } - const hidden = new Set( - githubProjectHiddenFieldIdsByView[githubProjectFieldVisibilityScope] ?? [] - ) - if (hidden.has(fieldId)) { - hidden.delete(fieldId) - } else { - hidden.add(fieldId) - } - const next = { ...githubProjectHiddenFieldIdsByView } - if (hidden.size === 0) { - delete next[githubProjectFieldVisibilityScope] - } else { - next[githubProjectFieldVisibilityScope] = [...hidden] - } - setGithubProjectHiddenFieldIdsByView(next) - persistTaskResumeState({ githubProjectHiddenFieldIdsByView: next }) + // Why: two toggles in one batch both read render scope and the first is lost. + setGithubProjectHiddenFieldIdsByView((current) => { + const hidden = new Set(current[githubProjectFieldVisibilityScope] ?? []) + if (hidden.has(fieldId)) { + hidden.delete(fieldId) + } else { + hidden.add(fieldId) + } + const next = { ...current } + if (hidden.size === 0) { + delete next[githubProjectFieldVisibilityScope] + } else { + next[githubProjectFieldVisibilityScope] = [...hidden] + } + persistTaskResumeState({ githubProjectHiddenFieldIdsByView: next }) + return next + }) }, - [githubProjectFieldVisibilityScope, githubProjectHiddenFieldIdsByView, persistTaskResumeState] + [githubProjectFieldVisibilityScope, persistTaskResumeState] ) const persistTaskSource = useCallback( (nextProvider: TaskProvider) => { diff --git a/mobile/src/tasks/use-mobile-tasks-workspace-create-actions.tsx b/mobile/src/tasks/use-mobile-tasks-workspace-create-actions.tsx index b704103b238..1b34c3ee280 100644 --- a/mobile/src/tasks/use-mobile-tasks-workspace-create-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-workspace-create-actions.tsx @@ -33,7 +33,8 @@ export function useMobileTasksWorkspaceCreateActions(model: WorkspaceSshStateMod taskWorkspaceCreationOperations, tasksSupported, trustedOrcaHooks, - workspaceDetectedAgentIds + workspaceDetectedAgentIds, + workspaceLastAutoName } = model const createWorkspace = useCallback( async ( @@ -139,6 +140,10 @@ export function useMobileTasksWorkspaceCreateActions(model: WorkspaceSshStateMod }) return } + const trimmedWorkspaceName = workspaceNameOverride?.trim() ?? '' + // A typed name that still matches the generated one stays auto-managed. + const nameIsAutoManaged = + !trimmedWorkspaceName || trimmedWorkspaceName === workspaceLastAutoName let selection: MobileComposerCreateSelection if (item.provider === 'github') { const source = item.source @@ -232,6 +237,7 @@ export function useMobileTasksWorkspaceCreateActions(model: WorkspaceSshStateMod workspaceName: workspaceNameOverride, note: comment, sparseCheckout: sparseCheckoutOverride, + nameIsAutoManaged, worktreeCreateIdempotency: taskWorkspaceCreationOperations .readRuntimeCapabilities() .then((capabilities) => capabilities.worktreeCreateIdempotency) @@ -267,7 +273,8 @@ export function useMobileTasksWorkspaceCreateActions(model: WorkspaceSshStateMod taskWorkspaceCreationOperations, tasksSupported, trustedOrcaHooks, - workspaceDetectedAgentIds + workspaceDetectedAgentIds, + workspaceLastAutoName ] ) return Object.assign(model, { diff --git a/mobile/src/terminal/terminal-accessory-repeat.test.ts b/mobile/src/terminal/terminal-accessory-repeat.test.ts index eab103eac15..41937ac3fcc 100644 --- a/mobile/src/terminal/terminal-accessory-repeat.test.ts +++ b/mobile/src/terminal/terminal-accessory-repeat.test.ts @@ -46,6 +46,37 @@ describe('terminal accessory repeat', () => { expect(sent).toEqual(['down', 'down', 'down']) }) + it('emits one keystroke per tap and holds at the iOS 400ms/45ms cadence', async () => { + vi.useFakeTimers() + const sent: number[] = [] + const start = Date.now() + const send = vi.fn(async () => { + sent.push(Date.now() - start) + return true + }) + const repeat = createTerminalAccessoryRepeatController() + + // A tap: press and release before the repeat delay elapses. + repeat.start('down', send) + await vi.advanceTimersByTimeAsync(0) + repeat.stop() + await vi.runAllTimersAsync() + expect(sent).toEqual([0]) + + // A hold: first send at press time, then 400ms, then every 45ms. + sent.length = 0 + const holdStart = Date.now() + repeat.start('down', send) + await vi.advanceTimersByTimeAsync(TERMINAL_ACCESSORY_REPEAT_DELAY_MS) + await vi.advanceTimersByTimeAsync(TERMINAL_ACCESSORY_REPEAT_INTERVAL_MS) + repeat.stop() + expect(sent.map((at) => at - (holdStart - start))).toEqual([ + 0, + TERMINAL_ACCESSORY_REPEAT_DELAY_MS, + TERMINAL_ACCESSORY_REPEAT_DELAY_MS + TERMINAL_ACCESSORY_REPEAT_INTERVAL_MS + ]) + }) + it('does not schedule another repeat after release while a send is pending', async () => { vi.useFakeTimers() const pending = deferred()