From b19983779fde34aec754ae5f5505f14c86ff973b Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 21 Sep 2026 09:36:52 -0400 Subject: [PATCH] fix(mobile): catch the custom-key save the page store refuses (OTA phase C, C7.7 round 2) Round 2 addendum. `addKey` awaited `saveCustomKeys` with no catch and both of its callers are `void addKey(...)`, so the rejection had nowhere to go. `orca:custom-accessory-keys` is in the session route's page allowlist and a page write over `PAGE_STORAGE_MAX_VALUE_CHARS` rejects rather than drops (the size contract of 33.4, extended by 33.6 to a key `init` could not carry), so past ~16 KB of accessory keys this surfaced as an unhandled rejection in the page -- which the fault boundary reports and which drops the generation. Every other allowlisted writer in this closure already catches: the two write chains in `TerminalShortcutSettings`, the live-input save and the session-view preference. Caught at the boundary and logged, and the drawer neither announces the key nor closes: a row on the accessory bar that no store holds, gone at the next load, is the failure the allowlist exists to avoid. Red first -- the case saw the refusal escape with the page's own message -- and the control reds again when the catch rethrows. Belongs in `8b4c559e90` by the brief; it is its own commit because that one was already made and amending is forbidden. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- mobile/src/components/CustomKeyModal.tsx | 15 +- .../custom-key-modal-save-refusal.test.tsx | 163 ++++++++++++++++++ 2 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 mobile/src/components/custom-key-modal-save-refusal.test.tsx diff --git a/mobile/src/components/CustomKeyModal.tsx b/mobile/src/components/CustomKeyModal.tsx index d783a487b50..cded289e692 100644 --- a/mobile/src/components/CustomKeyModal.tsx +++ b/mobile/src/components/CustomKeyModal.tsx @@ -111,7 +111,20 @@ export function CustomKeyModal({ visible, onClose, onKeysChanged, onManageShortc const existing = await loadCustomKeys() const newKey: CustomKey = { ...key, id: `custom-${Date.now()}` } const updated = [...existing, newKey] - await saveCustomKeys(updated) + // Caught here because both callers are `void addKey(...)`, which leaves a rejection nowhere + // to go. On the page this key is allowlisted and its write rejects for size — the contract + // `page-async-storage` states, and the one ruling 33.6 extends to a key `init` could not + // carry at all — so an uncaught save here reaches the document's unhandled-rejection + // handler, which reports a page fault and drops the generation for a key nobody could add. + // Every other allowlisted writer in this closure already catches its own save. + try { + await saveCustomKeys(updated) + } catch (error) { + // Neither reported nor closed: a drawer that dismissed itself and announced the key would + // put a row on the accessory bar that no store holds and the next load would not have. + console.warn('[custom-keys] the store would not take this key', error) + return + } onKeysChanged(updated) onClose() }, diff --git a/mobile/src/components/custom-key-modal-save-refusal.test.tsx b/mobile/src/components/custom-key-modal-save-refusal.test.tsx new file mode 100644 index 00000000000..a66d38f95e2 --- /dev/null +++ b/mobile/src/components/custom-key-modal-save-refusal.test.tsx @@ -0,0 +1,163 @@ +import { createElement, type ReactNode } from 'react' +import { act, create } from 'react-test-renderer' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const store = vi.hoisted(() => { + // Annotated rather than asserted: the literal alone narrows `raw` to `null`. + const held: { raw: string | null; refuse: boolean } = { raw: null, refuse: false } + return held +}) + +vi.mock('@react-native-async-storage/async-storage', () => ({ + default: { + getItem: async () => store.raw, + setItem: async () => { + if (store.refuse) { + throw new Error( + 'Orca could not save orca:custom-accessory-keys: a stored value may be too large.' + ) + } + } + } +})) +/** + * Function components rather than host strings, so a case can match a node by identity: the + * renderer types `node.type` as an `ElementType`, which a string literal is not, and the tests + * ratchet checks this file. + */ +const hosts = vi.hoisted(() => { + const make = (name: string) => { + const Host = (props: { children?: ReactNode }): ReactNode => props.children ?? null + Host.displayName = name + return Host + } + return { + View: make('View'), + Text: make('Text'), + Pressable: make('Pressable'), + TextInput: make('TextInput'), + Switch: make('Switch') + } +}) + +vi.mock('react-native', () => ({ + View: hosts.View, + Text: hosts.Text, + Pressable: hosts.Pressable, + TextInput: hosts.TextInput, + Switch: hosts.Switch, + StyleSheet: { create: (styles: T) => styles, absoluteFillObject: {} }, + Platform: { OS: 'ios', select: (options: Record) => options.ios } +})) +vi.mock('lucide-react-native', () => ({ ChevronLeft: hosts.View })) +vi.mock('./BottomDrawer', () => ({ BottomDrawer: hosts.View })) + +import { CustomKeyModal } from './CustomKeyModal' + +/** The one label a node renders, flattened, without walking a fiber into a cycle. */ +function labelOf(node: { props: { children?: unknown } }): string { + const seen: string[] = [] + const walk = (value: unknown): void => { + if (typeof value === 'string') { + seen.push(value) + return + } + if (Array.isArray(value)) { + for (const child of value) { + walk(child) + } + } + } + walk(node.props.children) + return seen.join(' ') +} + +/** Through the drawer as a user reaches it: pick the shortcut type, then press Add. */ +function addAShortcut(renderer: ReturnType): void { + const pressables = () => renderer.root.findAll((node) => node.type === hosts.Pressable) + const press = (match: (label: string) => boolean, what: string): void => { + const target = pressables().find((node) => + match( + node + .findAll((child) => child.type === hosts.Text) + .map((child) => labelOf(child)) + .join(' ') + ) + ) + if (target === undefined) { + throw new Error(`the modal rendered no ${what} control`) + } + const onPress = target.props.onPress + if (typeof onPress !== 'function') { + throw new Error(`the ${what} control has no press handler`) + } + act(() => { + onPress() + }) + } + press((label) => label.includes('Shortcut Combo'), 'shortcut-type') + press((label) => label.trim() === 'Add', 'save') +} + +beforeEach(() => { + store.raw = null + store.refuse = false +}) + +/** + * The page refuses a write the app would have taken, and the modal is one of its callers. + * + * `orca:custom-accessory-keys` is in the session route's page allowlist, and on the page a write + * over `PAGE_STORAGE_MAX_VALUE_CHARS` rejects rather than dropping — that is the size contract of + * ruling 33.4, and ruling 33.6 adds the key `init` could not carry at all. Every other allowlisted + * writer in this closure catches its save; this one awaited it inside a `void` call, so the + * rejection had nowhere to go but the page's unhandled-rejection handler, which reports a page + * fault and drops the generation. + */ +describe('adding a custom key when the store refuses the write', () => { + it('does not let the refusal escape as an unhandled rejection', async () => { + store.refuse = true + const unhandled = vi.fn() + process.on('unhandledRejection', unhandled) + const onKeysChanged = vi.fn() + const onClose = vi.fn() + let renderer: ReturnType | null = null + act(() => { + renderer = create(createElement(CustomKeyModal, { visible: true, onClose, onKeysChanged })) + }) + if (renderer === null) { + throw new Error('the modal did not render') + } + addAShortcut(renderer) + // Two turns: the load settles, then the save rejects into whatever catches it. + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + process.off('unhandledRejection', unhandled) + expect(unhandled).not.toHaveBeenCalled() + // And the modal does not report a key it failed to store: a row that looks added and is not + // is the failure the allowlist exists to avoid. + expect(onKeysChanged).not.toHaveBeenCalled() + expect(onClose).not.toHaveBeenCalled() + }) + + it('reports the key and closes when the store takes it, so the case above is the refusal', async () => { + const onKeysChanged = vi.fn() + const onClose = vi.fn() + let renderer: ReturnType | null = null + act(() => { + renderer = create(createElement(CustomKeyModal, { visible: true, onClose, onKeysChanged })) + }) + if (renderer === null) { + throw new Error('the modal did not render') + } + addAShortcut(renderer) + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + expect(onKeysChanged).toHaveBeenCalledTimes(1) + expect(onClose).toHaveBeenCalledTimes(1) + }) +})