From 13f6d086c612bd753e3fbc4d067363d12fccb249 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 3 Aug 2026 17:41:55 -0700 Subject: [PATCH] fix(mobile): keep session option sends scoped --- mobile/pnpm-lock.yaml | 72 ++--------- .../session/MobileNativeChatComposer.test.ts | 55 ++++++++- .../src/session/MobileNativeChatComposer.tsx | 9 +- .../MobileNativeChatComposerSuggestions.tsx | 1 + ...bileNativeChatSessionOptionPickers.test.ts | 53 +++++++- .../MobileNativeChatSessionOptionPickers.tsx | 10 +- .../use-mobile-native-chat-controller.ts | 11 +- ...se-mobile-native-chat-message-send.test.ts | 62 +++++++++- .../use-mobile-native-chat-message-send.ts | 28 +++-- ...mobile-native-chat-session-options.test.ts | 116 ++++++++++++++++-- .../use-mobile-native-chat-session-options.ts | 83 +++++++++---- 11 files changed, 368 insertions(+), 132 deletions(-) diff --git a/mobile/pnpm-lock.yaml b/mobile/pnpm-lock.yaml index 6510f9685b5..9788413f925 100644 --- a/mobile/pnpm-lock.yaml +++ b/mobile/pnpm-lock.yaml @@ -1951,56 +1951,48 @@ packages: engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] - libc: [glibc] '@oxfmt/binding-linux-arm64-musl@0.52.0': resolution: {integrity: sha512-wZg6bLjDvh2KibyI3QFUYo8GTXneIFsd0JvehtvJiUmQ8WRPERgxd/VM4ctWb86U5FT1FkqgS8/wZKVB+AZScg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] - libc: [musl] '@oxfmt/binding-linux-ppc64-gnu@0.52.0': resolution: {integrity: sha512-IngE8uxhNvxcMrLjZNDo9xNLY7rEK33AKnaMd2B46he1e/mz2CfcW6If/U1wUjdRZddm1QzQaciqZkuMkdh1FA==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [ppc64] os: [linux] - libc: [glibc] '@oxfmt/binding-linux-riscv64-gnu@0.52.0': resolution: {integrity: sha512-H3+DdFMv/efN3Efmhsv18jDrpiWWqKG7wsfAlQBqAt6z/E2Bx+TwEj2Nowe51CPOWB8/mFBC2dAMSgVFLvvowA==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [riscv64] os: [linux] - libc: [glibc] '@oxfmt/binding-linux-riscv64-musl@0.52.0': resolution: {integrity: sha512-zji+1kb7lJKohSDjzC1IsS+K/cKRs1hdVf0ZH0VbdbiakmtLvN9twBoXo/k8VdjFax7kfo+DyPxS7vv52br1aw==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [riscv64] os: [linux] - libc: [musl] '@oxfmt/binding-linux-s390x-gnu@0.52.0': resolution: {integrity: sha512-hcLBYedpCy7ToUvvBidWk7+11Yhg1oAZ4+6hKPic/mQI6NaqXJSXMps5nFlwUuX2ewhtLZZDPg63TI042qGKBg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [s390x] os: [linux] - libc: [glibc] '@oxfmt/binding-linux-x64-gnu@0.52.0': resolution: {integrity: sha512-IDO2loXK2OtTOhSPchU9MW25mWL2QCDGdJbjN8MXKZVS80qXe5gMTwQWu/gMJ3juoBHbkuUZNB2N1LHzNT7DoA==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] - libc: [glibc] '@oxfmt/binding-linux-x64-musl@0.52.0': resolution: {integrity: sha512-mAV2Hjn0SatJ+KoAzKUC3eJhdJ8wv+3m1KyuS0dTsbF0c5weq+QrCt/DRZZM+uj/XiKzCDEUKYsBF30e2qkcyw==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] - libc: [musl] '@oxfmt/binding-openharmony-arm64@0.52.0': resolution: {integrity: sha512-vd4npaUIwChxp7XzkqmepBWTT9YMcSe/NBApVGPC30/lLyOVaV3dvma1SKo03t8O73BPRAG7EyJzGlN5cJM5hQ==} @@ -2073,56 +2065,48 @@ packages: engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] - libc: [glibc] '@oxlint/binding-linux-arm64-musl@1.71.0': resolution: {integrity: sha512-fJZrs5sDZtTaPIOiemRQQmo82Ezy+vOGXemPc4Ok7iVVsYsFa7SlW6Z5XN819VfsqBHRm3NJ3rTdnR8+bJYJdQ==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] - libc: [musl] '@oxlint/binding-linux-ppc64-gnu@1.71.0': resolution: {integrity: sha512-cwl7VKGERIy9p+G+AvZdfy/06q0aHXaTt/mMRReC751iuNYJgqKjB7NydXSS30nBT9vtr2tunciOtrR4fD6FUA==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [ppc64] os: [linux] - libc: [glibc] '@oxlint/binding-linux-riscv64-gnu@1.71.0': resolution: {integrity: sha512-eZ8ieVXvzGi8jr7+ybQGPK2STw3mldfxZlgA2738iflfB/rzA69sE6m5rDRpQaxC7dpm745Enlh1Tod0QAk9Gg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [riscv64] os: [linux] - libc: [glibc] '@oxlint/binding-linux-riscv64-musl@1.71.0': resolution: {integrity: sha512-puMDbQYe6+NXwfMusojoA7CXGn2b3utukmd23PQqc1E3XhVCwyZ+FueSMzDYeNgDV2dUfIVXAAKZBcFDeCL6sA==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [riscv64] os: [linux] - libc: [musl] '@oxlint/binding-linux-s390x-gnu@1.71.0': resolution: {integrity: sha512-4NJLxBs1ujISCt3L/1FcywLs73PWtJuw+piD6feK2V6h6OS6P7xu9/sWt1DTRLibe6QCzmfZzmM/2HPORoV/Lg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [s390x] os: [linux] - libc: [glibc] '@oxlint/binding-linux-x64-gnu@1.71.0': resolution: {integrity: sha512-cFDaiR8L3430qp88tfZnvFlt3KotFhR/DlbIL0nHOMMYiG/9Wy4l+6f7t8G8pTa9bd8Lt8+M0y/qjRQ/xcB74g==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] - libc: [glibc] '@oxlint/binding-linux-x64-musl@1.71.0': resolution: {integrity: sha512-orfixdt76KlpNly9z0PkWBBNfwjKz+JFVLP/7wnVchlKNU9Dpt9InU/ZggeSej6fC7qwHmHNOGlhLnQXcYoGuA==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] - libc: [musl] '@oxlint/binding-openharmony-arm64@1.71.0': resolution: {integrity: sha512-9emQu2lAp6yhPB3XuI+++vR+l/o6JR1X+EpxwcumPdQXBWXEPAsquPGL7l158EqU8SebQMXTUa/S5zN98juyHw==} @@ -2584,42 +2568,36 @@ packages: engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] - libc: [glibc] '@rolldown/binding-linux-arm64-musl@1.1.3': resolution: {integrity: sha512-BO9+oPL8K9poZJBfYPsXNtYjPE5uM3qeehT3aFcW4LITOl+iSqhp0abzjR2nWBUNjIZeKXjAEWBZ64WjNoHd6w==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] - libc: [musl] '@rolldown/binding-linux-ppc64-gnu@1.1.3': resolution: {integrity: sha512-f3VpLB1vQ0Eo6ecr/6cekLnvYMFF4YBFoVGkfkvPLq1bAkbAwHYQPZKoAmG6OJyTcxxoC+AvezGx/S1obNC0Mw==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [ppc64] os: [linux] - libc: [glibc] '@rolldown/binding-linux-s390x-gnu@1.1.3': resolution: {integrity: sha512-AmurZ26Pqx/RI9N1gzEOCklkKXl927yjfXWUUS0O7Puh8ARM/Ob8qfrD3qnWksScdw6cSrW5PSHE9DyLu7+PtA==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [s390x] os: [linux] - libc: [glibc] '@rolldown/binding-linux-x64-gnu@1.1.3': resolution: {integrity: sha512-JJpqs8bRGITDOdbkNKnlojzBabbOHrqjSvDr0IVsZObE1lBcPjxItUEY9eWIDbxaJ3cGrXPWGfGkIxFijg/URg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] - libc: [glibc] '@rolldown/binding-linux-x64-musl@1.1.3': resolution: {integrity: sha512-rSJcdjPxzA/by/6/rYs+v+bXU7UjvnbUWz8MJb6kh6+knqB1dCrtHg0uu7C/4haqJvqdkYHQ5IGn+tCH9GLW/g==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] - libc: [musl] '@rolldown/binding-openharmony-arm64@1.1.3': resolution: {integrity: sha512-hQ3/PYkDJICgevvyNcVrihVeqq7k1Pp3VZ9lY+dauAYUJKO+auqApvANhvR1An9BhmqYKvW2Mu1F9u4DXSMLxQ==} @@ -4934,28 +4912,24 @@ packages: engines: {node: '>= 12.0.0'} cpu: [arm64] os: [linux] - libc: [glibc] lightningcss-linux-arm64-musl@1.32.0: resolution: {integrity: sha512-UpQkoenr4UJEzgVIYpI80lDFvRmPVg6oqboNHfoH4CQIfNA+HOrZ7Mo7KZP02dC6LjghPQJeBsvXhJod/wnIBg==} engines: {node: '>= 12.0.0'} cpu: [arm64] os: [linux] - libc: [musl] lightningcss-linux-x64-gnu@1.32.0: resolution: {integrity: sha512-V7Qr52IhZmdKPVr+Vtw8o+WLsQJYCTd8loIfpDaMRWGUZfBOYEJeyJIkqGIDMZPwPx24pUMfwSxxI8phr/MbOA==} engines: {node: '>= 12.0.0'} cpu: [x64] os: [linux] - libc: [glibc] lightningcss-linux-x64-musl@1.32.0: resolution: {integrity: sha512-bYcLp+Vb0awsiXg/80uCRezCYHNg1/l3mt0gzHnWV9XP1W5sKa5/TCdGWaR/zBM2PeF/HbsQv/j2URNOiVuxWg==} engines: {node: '>= 12.0.0'} cpu: [x64] os: [linux] - libc: [musl] lightningcss-win32-arm64-msvc@1.32.0: resolution: {integrity: sha512-8SbC8BR40pS6baCM8sbtYDSwEVQd4JlFTOlaD3gWGHfThTcABnNDBda6eTZeqbofalIJhFx0qKzgHJmcPTnGdw==} @@ -9717,7 +9691,7 @@ snapshots: dependencies: '@types/yargs-parser': 21.0.3 - '@typescript-eslint/eslint-plugin@8.59.2(@typescript-eslint/parser@8.59.2(eslint@9.39.4)(typescript@6.0.3))(eslint@9.39.4)(typescript@5.9.3)': + '@typescript-eslint/eslint-plugin@8.59.2(@typescript-eslint/parser@8.59.2(eslint@9.39.4)(typescript@5.9.3))(eslint@9.39.4)(typescript@5.9.3)': dependencies: '@eslint-community/regexpp': 4.12.2 '@typescript-eslint/parser': 8.59.2(eslint@9.39.4)(typescript@6.0.3) @@ -9737,7 +9711,7 @@ snapshots: dependencies: '@typescript-eslint/scope-manager': 8.59.2 '@typescript-eslint/types': 8.59.2 - '@typescript-eslint/typescript-estree': 8.59.2(typescript@6.0.3) + '@typescript-eslint/typescript-estree': 8.59.2(typescript@5.9.3) '@typescript-eslint/visitor-keys': 8.59.2 debug: 4.4.3 eslint: 9.39.4 @@ -9754,15 +9728,6 @@ snapshots: transitivePeerDependencies: - supports-color - '@typescript-eslint/project-service@8.59.2(typescript@6.0.3)': - dependencies: - '@typescript-eslint/tsconfig-utils': 8.59.2(typescript@6.0.3) - '@typescript-eslint/types': 8.59.2 - debug: 4.4.3 - typescript: 6.0.3 - transitivePeerDependencies: - - supports-color - '@typescript-eslint/scope-manager@8.59.2': dependencies: '@typescript-eslint/types': 8.59.2 @@ -9772,10 +9737,6 @@ snapshots: dependencies: typescript: 5.9.3 - '@typescript-eslint/tsconfig-utils@8.59.2(typescript@6.0.3)': - dependencies: - typescript: 6.0.3 - '@typescript-eslint/type-utils@8.59.2(eslint@9.39.4)(typescript@5.9.3)': dependencies: '@typescript-eslint/types': 8.59.2 @@ -9805,21 +9766,6 @@ snapshots: transitivePeerDependencies: - supports-color - '@typescript-eslint/typescript-estree@8.59.2(typescript@6.0.3)': - dependencies: - '@typescript-eslint/project-service': 8.59.2(typescript@6.0.3) - '@typescript-eslint/tsconfig-utils': 8.59.2(typescript@6.0.3) - '@typescript-eslint/types': 8.59.2 - '@typescript-eslint/visitor-keys': 8.59.2 - debug: 4.4.3 - minimatch: 10.2.5 - semver: 7.7.4 - tinyglobby: 0.2.17 - ts-api-utils: 2.5.0(typescript@6.0.3) - typescript: 6.0.3 - transitivePeerDependencies: - - supports-color - '@typescript-eslint/utils@8.59.2(eslint@9.39.4)(typescript@5.9.3)': dependencies: '@eslint-community/eslint-utils': 4.9.1(eslint@9.39.4) @@ -10888,11 +10834,11 @@ snapshots: eslint-config-universe@15.0.4(eslint@9.39.4)(prettier@2.8.8)(typescript@5.9.3): dependencies: - '@typescript-eslint/eslint-plugin': 8.59.2(@typescript-eslint/parser@8.59.2(eslint@9.39.4)(typescript@6.0.3))(eslint@9.39.4)(typescript@5.9.3) + '@typescript-eslint/eslint-plugin': 8.59.2(@typescript-eslint/parser@8.59.2(eslint@9.39.4)(typescript@5.9.3))(eslint@9.39.4)(typescript@5.9.3) '@typescript-eslint/parser': 8.59.2(eslint@9.39.4)(typescript@6.0.3) eslint: 9.39.4 eslint-config-prettier: 9.1.2(eslint@9.39.4) - eslint-plugin-import: 2.32.0(@typescript-eslint/parser@8.59.2(eslint@9.39.4)(typescript@6.0.3))(eslint@9.39.4) + eslint-plugin-import: 2.32.0(@typescript-eslint/parser@8.59.2(eslint@9.39.4)(typescript@5.9.3))(eslint@9.39.4) eslint-plugin-n: 17.24.0(eslint@9.39.4)(typescript@5.9.3) eslint-plugin-node: 11.1.0(eslint@9.39.4) eslint-plugin-prettier: 5.5.5(eslint-config-prettier@9.1.2(eslint@9.39.4))(eslint@9.39.4)(prettier@2.8.8) @@ -10916,7 +10862,7 @@ snapshots: transitivePeerDependencies: - supports-color - eslint-module-utils@2.12.1(@typescript-eslint/parser@8.59.2(eslint@9.39.4)(typescript@6.0.3))(eslint-import-resolver-node@0.3.10)(eslint@9.39.4): + eslint-module-utils@2.12.1(@typescript-eslint/parser@8.59.2(eslint@9.39.4)(typescript@5.9.3))(eslint-import-resolver-node@0.3.10)(eslint@9.39.4): dependencies: debug: 3.2.7 optionalDependencies: @@ -10939,7 +10885,7 @@ snapshots: eslint-utils: 2.1.0 regexpp: 3.2.0 - eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.59.2(eslint@9.39.4)(typescript@6.0.3))(eslint@9.39.4): + eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.59.2(eslint@9.39.4)(typescript@5.9.3))(eslint@9.39.4): dependencies: '@rtsao/scc': 1.1.0 array-includes: 3.1.9 @@ -10950,7 +10896,7 @@ snapshots: doctrine: 2.1.0 eslint: 9.39.4 eslint-import-resolver-node: 0.3.10 - eslint-module-utils: 2.12.1(@typescript-eslint/parser@8.59.2(eslint@9.39.4)(typescript@6.0.3))(eslint-import-resolver-node@0.3.10)(eslint@9.39.4) + eslint-module-utils: 2.12.1(@typescript-eslint/parser@8.59.2(eslint@9.39.4)(typescript@5.9.3))(eslint-import-resolver-node@0.3.10)(eslint@9.39.4) hasown: 2.0.3 is-core-module: 2.16.2 is-glob: 4.0.3 @@ -14133,10 +14079,6 @@ snapshots: dependencies: typescript: 5.9.3 - ts-api-utils@2.5.0(typescript@6.0.3): - dependencies: - typescript: 6.0.3 - ts-declaration-location@1.0.7(typescript@5.9.3): dependencies: picomatch: 4.0.4 diff --git a/mobile/src/session/MobileNativeChatComposer.test.ts b/mobile/src/session/MobileNativeChatComposer.test.ts index d19d5c52757..897d4dc945d 100644 --- a/mobile/src/session/MobileNativeChatComposer.test.ts +++ b/mobile/src/session/MobileNativeChatComposer.test.ts @@ -92,10 +92,61 @@ describe('MobileNativeChatComposer', () => { await act(async () => sendButton().props.onPress()) - expect(onSend).toHaveBeenCalledWith('hello') + expect(onSend).toHaveBeenCalledWith(' hello') expect(onChangeText).not.toHaveBeenCalled() }) + it('preserves leading whitespace so prose is not turned into a slash command', async () => { + const onSend = vi.fn().mockResolvedValue(true) + const restore = suppressRendererWarning() + try { + await act(async () => { + renderer = create( + createElement(MobileNativeChatComposer, { + value: ' /clear is prose ', + onChangeText: vi.fn(), + onSend + }) + ) + }) + } finally { + restore() + } + await act(async () => sendButton().props.onPress()) + expect(onSend).toHaveBeenCalledWith(' /clear is prose') + }) + + it('blocks composer submission while a session-option command is pending', async () => { + const onSend = vi.fn().mockResolvedValue(true) + const restore = suppressRendererWarning() + try { + await act(async () => { + renderer = create( + createElement(MobileNativeChatComposer, { + value: 'hello', + onChangeText: vi.fn(), + onSend, + sessionOptions: { + isWorking: false, + controller: { + snapshot: [], + pendingId: 'model', + setOption: vi.fn(), + invokeAction: vi.fn(), + recordCommand: vi.fn() + } + } + }) + ) + }) + } finally { + restore() + } + expect(sendButton().props).toMatchObject({ disabled: true }) + await act(async () => sendButton().props.onPress()) + expect(onSend).not.toHaveBeenCalled() + }) + it('keeps the draft when the send is rejected', async () => { const onChangeText = vi.fn() const onSend = vi.fn().mockResolvedValue(false) @@ -103,7 +154,7 @@ describe('MobileNativeChatComposer', () => { await act(async () => sendButton().props.onPress()) - expect(onSend).toHaveBeenCalledWith('hello') + expect(onSend).toHaveBeenCalledWith(' hello') expect(onChangeText).not.toHaveBeenCalled() }) diff --git a/mobile/src/session/MobileNativeChatComposer.tsx b/mobile/src/session/MobileNativeChatComposer.tsx index d44a4855fa0..8097ab45c70 100644 --- a/mobile/src/session/MobileNativeChatComposer.tsx +++ b/mobile/src/session/MobileNativeChatComposer.tsx @@ -89,10 +89,15 @@ export function MobileNativeChatComposer({ const sendingRef = useRef(false) const [sending, setSending] = useState(false) const trimmed = value.trim() + const sessionOptionDispatching = sessionOptions?.controller.pendingId != null // An attached image alone is a valid send (desktop parity), so the image rides // along even when the user sends no accompanying text. const canSend = - (trimmed.length > 0 || attachments.length > 0) && !disabled && !sending && !isAttaching + (trimmed.length > 0 || attachments.length > 0) && + !disabled && + !sending && + !isAttaching && + !sessionOptionDispatching const trigger = useMemo(() => detectAutocompleteTrigger(value, cursor), [value, cursor]) const suggestions = useMemo(() => { @@ -143,7 +148,7 @@ export function MobileNativeChatComposer({ sendingRef.current = true setSending(true) try { - const accepted = await onSend(trimmed) + const accepted = await onSend(value.trimEnd()) if (accepted) { setCursor(0) } diff --git a/mobile/src/session/MobileNativeChatComposerSuggestions.tsx b/mobile/src/session/MobileNativeChatComposerSuggestions.tsx index 52391158295..a9ac71f33ea 100644 --- a/mobile/src/session/MobileNativeChatComposerSuggestions.tsx +++ b/mobile/src/session/MobileNativeChatComposerSuggestions.tsx @@ -32,6 +32,7 @@ export function MobileNativeChatComposerSuggestions({ {suggestions.map((suggestion) => ( [styles.suggestion, pressed && styles.suggestionPressed]} onPress={() => onPick(suggestion)} > diff --git a/mobile/src/session/MobileNativeChatSessionOptionPickers.test.ts b/mobile/src/session/MobileNativeChatSessionOptionPickers.test.ts index e6ebe44d1e7..75909c35938 100644 --- a/mobile/src/session/MobileNativeChatSessionOptionPickers.test.ts +++ b/mobile/src/session/MobileNativeChatSessionOptionPickers.test.ts @@ -71,15 +71,39 @@ describe('MobileNativeChatSessionOptionPickers', () => { }) } - const pill = (name: string): { props: { onPress: () => void; disabled?: boolean } } => + const pill = ( + name: string + ): { + props: { + onPress: () => void + disabled?: boolean + accessibilityRole?: string + accessibilityState?: { disabled?: boolean } + } + } => renderer!.root.find( (node) => node.type === 'Pressable' && typeof node.props.accessibilityLabel === 'string' && node.props.accessibilityLabel.startsWith(name) - ) as { props: { onPress: () => void; disabled?: boolean } } + ) as { + props: { + onPress: () => void + disabled?: boolean + accessibilityRole?: string + accessibilityState?: { disabled?: boolean } + } + } - const rowByText = (text: string): { props: { onPress: () => void } } => { + const rowByText = ( + text: string + ): { + props: { + onPress: () => void + accessibilityRole?: string + accessibilityState?: { checked?: boolean; disabled?: boolean } + } + } => { const label = renderer!.root .findAll((node) => node.type === 'Text') .find((node) => (node.props as { children?: unknown }).children === text) @@ -93,7 +117,13 @@ describe('MobileNativeChatSessionOptionPickers', () => { if (!parent) { throw new Error(`No pressable row for ${text}`) } - return parent as unknown as { props: { onPress: () => void } } + return parent as unknown as { + props: { + onPress: () => void + accessibilityRole?: string + accessibilityState?: { checked?: boolean; disabled?: boolean } + } + } } beforeEach(() => { @@ -132,6 +162,20 @@ describe('MobileNativeChatSessionOptionPickers', () => { expect(setOption).toHaveBeenCalledWith('model', 'opus') }) + it('announces choice selection and disabled state', async () => { + mount([MODEL_DESCRIPTOR, EFFORT_DESCRIPTOR]) + expect(pill('Model').props).toMatchObject({ + accessibilityRole: 'button', + accessibilityState: { disabled: false } + }) + await act(async () => pill('Model').props.onPress()) + expect(rowByText('Sonnet 5').props).toMatchObject({ + accessibilityRole: 'radio', + accessibilityState: { checked: true, disabled: false } + }) + expect(rowByText('Opus 4.8').props.accessibilityState?.checked).toBe(false) + }) + it('closes without dispatch when re-picking the tracked value', async () => { mount([MODEL_DESCRIPTOR, EFFORT_DESCRIPTOR]) await act(async () => pill('Model').props.onPress()) @@ -149,6 +193,7 @@ describe('MobileNativeChatSessionOptionPickers', () => { } ]) await act(async () => pill('Model').props.onPress()) + expect(rowByText('Choose in agent picker…').props.accessibilityRole).toBe('button') await act(async () => rowByText('Choose in agent picker…').props.onPress()) expect(invokeAction).toHaveBeenCalledWith('model') }) diff --git a/mobile/src/session/MobileNativeChatSessionOptionPickers.tsx b/mobile/src/session/MobileNativeChatSessionOptionPickers.tsx index c5a725d7aad..1e79c158da2 100644 --- a/mobile/src/session/MobileNativeChatSessionOptionPickers.tsx +++ b/mobile/src/session/MobileNativeChatSessionOptionPickers.tsx @@ -36,6 +36,8 @@ function Pill({ return ( [styles.pill, pressed && !disabled && styles.pressed]} onPress={onPress} disabled={disabled} @@ -68,6 +70,8 @@ function ChoiceRow({ }): React.JSX.Element { return ( } - // Why: absolute On/Off only when we have tracked truth. Unknown composed - // booleans leave the group unselected so empty radios are not a selection. + // Unknown booleans leave both radios unselected instead of inventing truth. if (descriptor.kind.type === 'boolean') { const current = descriptor.kind.currentValue return ( @@ -235,6 +240,7 @@ export function MobileNativeChatSessionOptionPickers({ ) : null} setOpen(null)} hitSlop={8} diff --git a/mobile/src/session/use-mobile-native-chat-controller.ts b/mobile/src/session/use-mobile-native-chat-controller.ts index 70b69ac586c..d7e6af535c7 100644 --- a/mobile/src/session/use-mobile-native-chat-controller.ts +++ b/mobile/src/session/use-mobile-native-chat-controller.ts @@ -1,6 +1,5 @@ import { useCallback, - useEffect, useRef, type Dispatch, type MutableRefObject, @@ -250,10 +249,6 @@ export function useMobileNativeChatController(args: { // tracking, but the options hook needs the seam's dispatcher — a ref breaks // the cycle without re-creating the send callbacks per snapshot. const recordSessionOptionCommandRef = useRef<(command: string) => void>(() => {}) - const handleCommandSend = useCallback( - (command: string) => recordSessionOptionCommandRef.current(command), - [] - ) const { send: handleNativeChatSend, @@ -266,7 +261,7 @@ export function useMobileNativeChatController(args: { handleRef: activeHandleRef, deviceTokenRef, agentRef: activeChatAgentRef, - onCommandSend: handleCommandSend, + commandSendRef: recordSessionOptionCommandRef, captureSendOrigin, readSeededLaunchDraftSeed, clearDraftForSend, @@ -291,9 +286,7 @@ export function useMobileNativeChatController(args: { dispatchCommand: handleNativeChatDispatchCommand, onAgentPicker: handleAgentPicker }) - useEffect(() => { - recordSessionOptionCommandRef.current = sessionOptions.recordCommand - }, [sessionOptions.recordCommand]) + recordSessionOptionCommandRef.current = sessionOptions.recordCommand // Card actions retire the route's held failure banner too, not just sends. const answerAsk = useNativeChatAcceptedAction(handleNativeChatAnswerAsk, onSendResolved) const cancelAsk = useNativeChatAcceptedAction(handleNativeChatCancelAsk, onSendResolved) diff --git a/mobile/src/session/use-mobile-native-chat-message-send.test.ts b/mobile/src/session/use-mobile-native-chat-message-send.test.ts index 0046043450d..71f2a98d99a 100644 --- a/mobile/src/session/use-mobile-native-chat-message-send.test.ts +++ b/mobile/src/session/use-mobile-native-chat-message-send.test.ts @@ -31,19 +31,22 @@ describe('useMobileNativeChatMessageSend', () => { const acceptSend = vi.fn() const holdUnconfirmedSend = vi.fn() const onCommandSend = vi.fn() + const commandSendRef = { current: onCommandSend } + const agentRef = { current: null as string | null } const mount = ( readSeededLaunchDraftSeed: () => { text: string; createdAt: number | null } | null, agent: string | null = 'claude' ): void => { + agentRef.current = agent function Probe(): null { api = useMobileNativeChatMessageSend({ client: { sendRequest: vi.fn() } as never, enabled: true, handleRef: { current: 'term' }, deviceTokenRef: { current: 'device' }, - agentRef: { current: agent }, - onCommandSend, + agentRef, + commandSendRef, captureSendOrigin: () => ({ draftKey: 'k', pendingKey: 'p' }) as never, readSeededLaunchDraftSeed, clearDraftForSend: () => {}, @@ -79,6 +82,7 @@ describe('useMobileNativeChatMessageSend', () => { acceptSend.mockReset() holdUnconfirmedSend.mockReset() onCommandSend.mockReset() + commandSendRef.current = onCommandSend }) afterEach(() => { act(() => { @@ -205,14 +209,14 @@ describe('useMobileNativeChatMessageSend', () => { }) it('never creates an optimistic echo for an unknown slash token', async () => { - // `/model` is not in Claude's verified catalog, but it still dispatches to - // the TUI — same no-echo rule, without claiming a verified command ran. + // `/model` is not in Claude's autocomplete catalog, but the session-option + // recorder still recognizes it without claiming a generic command ran. mount(() => null) await act(async () => { await api!.send('/model sonnet') }) expect(acceptSend).not.toHaveBeenCalled() - expect(onCommandSend).not.toHaveBeenCalled() + expect(onCommandSend).toHaveBeenCalledWith('/model sonnet') }) it('classifies per agent: /model is a catalog command for Codex', async () => { @@ -238,13 +242,59 @@ describe('useMobileNativeChatMessageSend', () => { }) it('dispatchCommand surfaces the outcome without echo or composer sync', async () => { - mount(() => null) + mount(() => ({ text: DRAFT, createdAt: 1 })) let outcome: string | undefined await act(async () => { outcome = await api!.dispatchCommand('/model sonnet') }) expect(outcome).toBe('accepted') expect(acceptSend).not.toHaveBeenCalled() + expect(onCommandSend).not.toHaveBeenCalled() expect(sentArgs().resolvedLaunchDraft).toBeUndefined() }) + + it('binds classification to the agent that started the send', async () => { + let resolveSend!: (outcome: MobileNativeChatSendOutcome) => void + sendWithOutcome.mockReturnValue( + new Promise((resolve) => { + resolveSend = resolve + }) + ) + mount(() => null, 'claude') + let sending!: Promise + act(() => { + sending = api!.send('$skill') + }) + agentRef.current = 'codex' + await act(async () => { + resolveSend('accepted') + await sending + }) + expect(acceptSend).toHaveBeenCalledTimes(1) + }) + + it('records a command against the tab that started the send', async () => { + let resolveSend!: (outcome: MobileNativeChatSendOutcome) => void + sendWithOutcome.mockReturnValue( + new Promise((resolve) => { + resolveSend = resolve + }) + ) + const originalRecorder = vi.fn() + const nextRecorder = vi.fn() + commandSendRef.current = originalRecorder + mount(() => null) + commandSendRef.current = originalRecorder + let sending!: Promise + act(() => { + sending = api!.send('/clear') + }) + commandSendRef.current = nextRecorder + await act(async () => { + resolveSend('accepted') + await sending + }) + expect(originalRecorder).toHaveBeenCalledWith('/clear') + expect(nextRecorder).not.toHaveBeenCalled() + }) }) diff --git a/mobile/src/session/use-mobile-native-chat-message-send.ts b/mobile/src/session/use-mobile-native-chat-message-send.ts index d07f4b6daa3..bde08d9b60e 100644 --- a/mobile/src/session/use-mobile-native-chat-message-send.ts +++ b/mobile/src/session/use-mobile-native-chat-message-send.ts @@ -40,9 +40,9 @@ export function useMobileNativeChatMessageSend(args: { deviceTokenRef: MutableRefObject /** Active tab's agent — classification is per-agent (command catalogs differ). */ agentRef: MutableRefObject - /** Fires after a catalog command send is accepted, so session-option state can - * track typed commands like `/model sonnet` (desktop recordOutgoingCommand parity). */ - onCommandSend?: (command: string) => void + /** Captured when a control send starts so a later tab switch cannot record its + * session-option effects against the newly active tab. */ + commandSendRef: MutableRefObject<(command: string) => void> captureSendOrigin: (text: string) => MobileNativeChatSendOrigin | null /** Launch-context text Orca parked on the agent's TUI input line, or null. Read * at send time so the pre-clear can be sized to every line it occupies. */ @@ -63,7 +63,7 @@ export function useMobileNativeChatMessageSend(args: { handleRef, deviceTokenRef, agentRef, - onCommandSend, + commandSendRef, captureSendOrigin, readSeededLaunchDraftSeed, clearDraftForSend, @@ -78,10 +78,13 @@ export function useMobileNativeChatMessageSend(args: { text: string, images: string[] | undefined, syncComposer: boolean, + recordControlSend: boolean, sharedDeadline?: number ): Promise => { const handle = handleRef.current const origin = captureSendOrigin(text) + const agent = agentRef.current + const recordCommand = commandSendRef.current // Why: the lease collapses one render after `connState`, so a question-card // answer (which reaches this send directly) would otherwise burn the whole // 15s heal+send budget waiting on a socket that is already gone. @@ -176,7 +179,7 @@ export function useMobileNativeChatMessageSend(args: { // TUI, not the conversation — the transcript never echoes it as a user // turn, so an optimistic bubble would sit at "Queued" forever and the // unconfirmed hold could never observe a landing. - const classification = classifyMobileNativeChatSend(agentRef.current, text) + const classification = classifyMobileNativeChatSend(agent, text) if (outcome === 'unknown') { if (classification === 'chat') { // Why: an ack-lost send usually WAS delivered (issue seen on cellular @@ -198,8 +201,10 @@ export function useMobileNativeChatMessageSend(args: { // `images` are local preview URIs for the optimistic echo only — the actual // image bytes already rode along as a bracketed paste before this text send. acceptSend(origin, text, images) - } else if (classification === 'command') { - onCommandSend?.(text.trim()) + } else if (recordControlSend) { + // The session-option catalog can recognize controls omitted from the + // autocomplete catalog (for example Claude `/model` and `/fast`). + recordCommand(text.trim()) } return 'accepted' }, @@ -209,11 +214,11 @@ export function useMobileNativeChatMessageSend(args: { captureSendOrigin, clearDraftForSend, client, + commandSendRef, deviceTokenRef, enabled, handleRef, holdUnconfirmedSend, - onCommandSend, onSendError, readSeededLaunchDraftSeed, restoreRejectedDraft @@ -222,7 +227,7 @@ export function useMobileNativeChatMessageSend(args: { const sendWithOutcome = useCallback( (text: string, images?: string[], deadline?: number) => - sendMessage(text, images, true, deadline), + sendMessage(text, images, true, true, deadline), [sendMessage] ) @@ -237,12 +242,13 @@ export function useMobileNativeChatMessageSend(args: { // A question answer is not composer text, so it never syncs the draft. const answerQuestion = useCallback( async (text: string): Promise => - (await sendMessage(text, undefined, false)) !== 'rejected', + (await sendMessage(text, undefined, false, true)) !== 'rejected', [sendMessage] ) const dispatchCommand = useCallback( - (text: string): Promise => sendMessage(text, undefined, false), + (text: string): Promise => + sendMessage(text, undefined, false, false), [sendMessage] ) diff --git a/mobile/src/session/use-mobile-native-chat-session-options.test.ts b/mobile/src/session/use-mobile-native-chat-session-options.test.ts index f03ac6ab6d4..d77f91e9a70 100644 --- a/mobile/src/session/use-mobile-native-chat-session-options.test.ts +++ b/mobile/src/session/use-mobile-native-chat-session-options.test.ts @@ -13,26 +13,36 @@ type HookArgs = Parameters[0] describe('useMobileNativeChatSessionOptions', () => { let renderer: ReactTestRenderer | null = null let api: MobileNativeChatSessionOptionsController | null = null + let hookArgs: HookArgs const dispatchCommand = vi.fn<(command: string) => Promise>() const onAgentPicker = vi.fn() + function Probe(): null { + api = useMobileNativeChatSessionOptions(hookArgs) + return null + } + const mount = (overrides: Partial = {}): void => { - function Probe(): null { - api = useMobileNativeChatSessionOptions({ - agent: 'claude', - scopeKey: 'host\0worktree\0tab', - reportedModel: null, - dispatchCommand, - onAgentPicker, - ...overrides - }) - return null + hookArgs = { + agent: 'claude', + scopeKey: 'host\0worktree\0tab', + reportedModel: null, + dispatchCommand, + onAgentPicker, + ...overrides } act(() => { renderer = create(createElement(Probe)) }) } + const update = (overrides: Partial): void => { + hookArgs = { ...hookArgs, ...overrides } + act(() => { + renderer!.update(createElement(Probe)) + }) + } + beforeEach(() => { globalThis.IS_REACT_ACT_ENVIRONMENT = true clearMobileSessionOptionRecordsForTests() @@ -59,6 +69,11 @@ describe('useMobileNativeChatSessionOptions', () => { expect(api!.snapshot).toEqual([]) }) + it('does not expose catalog-backed agents outside the Claude and Codex scope', () => { + mount({ agent: 'gemini' }) + expect(api!.snapshot).toEqual([]) + }) + it('applies a model pick through the catalog modelApply command', async () => { mount() let applied: boolean | undefined @@ -119,4 +134,85 @@ describe('useMobileNativeChatSessionOptions', () => { expect(effort).toMatchObject({ valueSource: 'dispatched' }) expect(effort!.kind).toMatchObject({ currentValue: 'low' }) }) + + it('keeps the latest queued operation pending until it settles', async () => { + const resolvers: Array<(outcome: MobileNativeChatSendOutcome) => void> = [] + dispatchCommand.mockImplementation( + () => + new Promise((resolve) => { + resolvers.push(resolve) + }) + ) + mount({ reportedModel: 'claude-sonnet-5' }) + let first!: Promise + let second!: Promise + await act(async () => { + first = api!.setOption('effort', 'low') + second = api!.setOption('model', 'opus') + await Promise.resolve() + }) + expect(api!.pendingId).toBe('model') + await act(async () => { + resolvers[0]!('accepted') + await Promise.resolve() + }) + expect(dispatchCommand).toHaveBeenCalledTimes(2) + expect(api!.pendingId).toBe('model') + await act(async () => { + resolvers[1]!('accepted') + await Promise.all([first, second]) + }) + expect(api!.pendingId).toBeNull() + }) + + it('does not dispatch a queued option into a newly active tab', async () => { + let resolveFirst!: (outcome: MobileNativeChatSendOutcome) => void + dispatchCommand.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirst = resolve + }) + ) + mount({ reportedModel: 'claude-sonnet-5' }) + let first!: Promise + let queued!: Promise + await act(async () => { + first = api!.setOption('effort', 'low') + queued = api!.setOption('model', 'opus') + await Promise.resolve() + }) + update({ scopeKey: 'host\0worktree\0other-tab' }) + await act(async () => { + resolveFirst('accepted') + await first + }) + await expect(queued).resolves.toBe(false) + expect(dispatchCommand).toHaveBeenCalledTimes(1) + }) + + it('does not dispatch queued work after unmount', async () => { + let resolveFirst!: (outcome: MobileNativeChatSendOutcome) => void + dispatchCommand.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirst = resolve + }) + ) + mount({ reportedModel: 'claude-sonnet-5' }) + let first!: Promise + let queued!: Promise + await act(async () => { + first = api!.setOption('effort', 'low') + queued = api!.setOption('model', 'opus') + await Promise.resolve() + }) + act(() => { + renderer!.unmount() + }) + renderer = null + resolveFirst('accepted') + await expect(first).resolves.toBe(true) + await expect(queued).resolves.toBe(false) + expect(dispatchCommand).toHaveBeenCalledTimes(1) + }) }) diff --git a/mobile/src/session/use-mobile-native-chat-session-options.ts b/mobile/src/session/use-mobile-native-chat-session-options.ts index df10fb3a7da..b173d2c2704 100644 --- a/mobile/src/session/use-mobile-native-chat-session-options.ts +++ b/mobile/src/session/use-mobile-native-chat-session-options.ts @@ -32,6 +32,8 @@ export type MobileNativeChatSessionOptionsController = { recordCommand: (command: string) => void } +type PendingOperation = { id: string; token: number } + // Why: per-tab records survive chat↔terminal flips and remounts, like desktop's // scope cache. Bounded so long sessions across many tabs can't grow unbounded. const MOBILE_SESSION_OPTION_RECORD_CAP = 32 @@ -71,13 +73,28 @@ export function useMobileNativeChatSessionOptions(args: { onAgentPicker?: () => void }): MobileNativeChatSessionOptionsController { const { agent, scopeKey, reportedModel, dispatchCommand, onAgentPicker } = args - const catalog = useMemo(() => (agent ? getAgentSessionOptionCatalog(agent) : null), [agent]) + const catalog = useMemo( + () => (agent === 'claude' || agent === 'codex' ? getAgentSessionOptionCatalog(agent) : null), + [agent] + ) + const identity = agent && scopeKey ? `${scopeKey}\0${agent}` : null const [version, setVersion] = useState(0) - const [pendingId, setPendingId] = useState(null) + const [pendingByIdentity, setPendingByIdentity] = useState< + Record + >({}) + const pendingId = identity ? (pendingByIdentity[identity]?.id ?? null) : null const bump = useCallback(() => setVersion((current) => current + 1), []) - // Why: ordered applies — a later absolute target must observe the result of - // an earlier dispatch instead of computing against a stale baseline. - const applyQueueRef = useRef>(Promise.resolve()) + const activeIdentityRef = useRef(identity) + activeIdentityRef.current = identity + const applyQueuesRef = useRef(new Map>()) + const operationTokenRef = useRef(0) + + useEffect( + () => () => { + activeIdentityRef.current = null + }, + [] + ) // Seed the current model from live agent status; hook reports are authority // over locally dispatched guesses (desktop 'reported' source parity). @@ -104,23 +121,47 @@ export function useMobileNativeChatSessionOptions(args: { return buildMobileSessionOptionSnapshot({ catalog, record: getScopedRecord(scopeKey, agent) }) }, [agent, catalog, scopeKey, version]) - const runSerialized = useCallback((id: string, run: () => Promise): Promise => { - const chained = applyQueueRef.current.then(run, run) - applyQueueRef.current = chained.then( - () => undefined, - () => undefined - ) - setPendingId(id) - void chained.finally(() => setPendingId(null)) - return chained - }, []) + const runSerialized = useCallback( + (operationIdentity: string, id: string, run: () => Promise): Promise => { + const previous = applyQueuesRef.current.get(operationIdentity) ?? Promise.resolve() + const runIfCurrent = (): Promise => + activeIdentityRef.current === operationIdentity ? run() : Promise.resolve(false) + const chained = previous.then(runIfCurrent, runIfCurrent) + const tail = chained.then( + () => undefined, + () => undefined + ) + applyQueuesRef.current.set(operationIdentity, tail) + operationTokenRef.current += 1 + const token = operationTokenRef.current + setPendingByIdentity((current) => ({ + ...current, + [operationIdentity]: { id, token } + })) + void tail.then(() => { + if (applyQueuesRef.current.get(operationIdentity) === tail) { + applyQueuesRef.current.delete(operationIdentity) + } + setPendingByIdentity((current) => { + if (current[operationIdentity]?.token !== token) { + return current + } + const next = { ...current } + delete next[operationIdentity] + return next + }) + }) + return chained + }, + [] + ) const setOption = useCallback( (id: string, value: SessionOptionValue): Promise => { - if (!catalog || !scopeKey || !agent) { + if (!catalog || !scopeKey || !agent || !identity) { return Promise.resolve(false) } - return runSerialized(id, async () => { + return runSerialized(identity, id, async () => { const record = getScopedRecord(scopeKey, agent) const previousModelId = typeof record.model?.value === 'string' ? record.model.value : null const apply = @@ -176,15 +217,15 @@ export function useMobileNativeChatSessionOptions(args: { return true }) }, - [agent, bump, catalog, dispatchCommand, runSerialized, scopeKey] + [agent, bump, catalog, dispatchCommand, identity, runSerialized, scopeKey] ) const invokeAction = useCallback( (id: string): Promise => { - if (!catalog || !scopeKey || !agent) { + if (!catalog || !scopeKey || !agent || !identity) { return Promise.resolve(false) } - return runSerialized(id, async () => { + return runSerialized(identity, id, async () => { const record = getScopedRecord(scopeKey, agent) const modelId = typeof record.model?.value === 'string' ? record.model.value : null const apply = @@ -211,7 +252,7 @@ export function useMobileNativeChatSessionOptions(args: { return false }) }, - [agent, bump, catalog, dispatchCommand, onAgentPicker, runSerialized, scopeKey] + [agent, bump, catalog, dispatchCommand, identity, onAgentPicker, runSerialized, scopeKey] ) const recordCommand = useCallback(