From e736a9f29bc4a8fabb8a4e3aaf31a6cee50ef58f Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Sun, 20 Sep 2026 09:49:59 -0400 Subject: [PATCH] feat(mobile): take the session screen's inputs, links, clipboard and routers through the platform seams (OTA phase C, C7.2) (#21790) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(mobile): put the session screen's nine text inputs on the web font seam (OTA phase C, C7.2) The landed text-input census, run over `app/h/[hostId]/session/[worktreeId].tsx`, reports nine sizes that do not come from `TEXT_INPUT_FONT_SIZE`. Six declare the app's body size and move in place, which is the same number natively. Three do not — a 22px key-capture field and the chat's two 15px fields — so each gets a `.web.ts` sibling of the address bar's shape, with a shared base so the two halves can differ in nothing but the size. The capture field is the one the move shrinks rather than raises: 22 already clears the focus-zoom floor, and the census reads the seam as a binding rather than as a number, so there is no expression that keeps 22 and still says where the size came from. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): drop the theme import the composer's style split left behind `oxlint` over the whole tree, which CI runs, reads it as an error. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): open the session screen's three external URLs through the platform seam (OTA phase C, C7.2) The landed external-link census, run over the session route's closure, reports three modules reaching react-native's `Linking`: a terminal link tap whose open mode is the phone's browser, and the two WebView-backed readers, each of which sends a tapped link to the system browser rather than navigating the artifact away. Inside the shell `Linking.openURL` calls `window.open`, which both shells refuse and which resolves either way, so all three reported success into a tap that did nothing. The seam also stops swallowing the failure: each site caught and discarded, and `openExternalLink` names it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): take the session screen's seven clipboard sites through the platform seam (OTA phase C, C7.2) `expo-clipboard` resolves to `navigator.clipboard` on the web, which needs a secure context — iOS serves the page from a custom scheme and Android from `https`, so that path works on one platform and silently not on the other. The landed census now reports the module out of the route's closure entirely. The seam grows its reader half, on the landed `native.clipboard.read` verb: text, a PNG, and a presence probe. Two degradations are recorded rather than implied. No shell serves an image, so the page answers null and the terminal's paste takes the branch an empty clipboard already took; and the shell serves no presence verb, so `contents` answers what this side knows rather than reading to find out, which would raise iOS's paste-consent prompt on every foreground. The copy-path sheet gains the failure toast its two neighbours already had: it showed "Path copied" before the write, and the seam rejects rather than returning false. The route parity pin moves with it: five clipboard hooks join the expanded route and one runtime string joins the sheet. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): take the session domain's three routers through the handoff seam (OTA phase C, C7.2) Inside the page a screen is one document standing in for one screen, and `useRouteHandoff` is the only thing that knows which targets the page keeps and which it hands back to the app. The three holders here are the workspace-missing bounce, the file-tap preview push, and the pane-tap param consume. The domain's census is narrower than the two landed ones because it has to be: eight of its hooks take `useFocusEffect` and two take `useLocalSearchParams`, neither of which can navigate, so the rule is a closed list of names rather than a ban on any value import — which also catches expo-router's module-singleton `router`, a spelling a `useRouter` rule would have read as clean. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): list the two style siblings the session screen's inputs added (OTA phase C, C7.2) The overrides census fails on an unlisted `.web.*`. One raises the chat's two 15px fields past the focus-zoom floor; the other lowers a 22px capture field onto the seam, and its entry says why a reduction is the right answer there. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): let the text-input census read a literal already clear of the floor (OTA phase C, C7.2, ruling 12) The floor is the rule and the seam is the mechanism. A binding rule alone made the custom-key capture field an offender at 22, where nothing can zoom, and the only way to satisfy it was to lower a one-character field to 16 — the tail wagging the dog. The seam's web half now exports the floor it already computed `Math.max` against, and the census reads that number out of that file rather than carrying a second copy of 16. The rule becomes "the seam's binding, or a literal at or above the floor", with no per-site exemption: a literal under the floor is still reported, which is the case the seam exists for. A tree whose seam declares no floor is refused rather than judged against a number the census invented. So the capture field goes back to 22 on both platforms and its split, its override entry and its parity test go with it. The chat's two fields stay split, because 15 is under the floor however it is spelled. Red-first: with the rule removed, a planted literal 16 and a literal 22 are both reported and the refusal case does not throw; a literal 15 is reported either way. All three route closures that run this census — session, source-control, review — report 0 offenders and 0 unresolved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): answer clipboard reads on the read grant and catch refused writes `contents()` reported no text on a route that granted `native.clipboard.read` without the write, because it read `verbs.granted`, which is write AND read. The verbs hook now exposes the two grants separately and the web seam answers on the read one; `granted` keeps its meaning for the callers that need both. The Markdown copy action was the one write of eight in the session domain with nowhere for a rejection to go: the seam rejects when the pasteboard refused the text, the callback had no failure branch, and its caller drops the promise, so a refused write raised an unhandled rejection and still left "Copied" on screen. It now takes the error haptic and the "Couldn't copy" toast the other copy paths show. A census over `src/session` fails if any `writeText` call site lacks a failure branch, so the ninth site cannot arrive without one. The route parity pin moves with it: one callback body, one runtime string. Its refresh note claimed six clipboard hook sites for a delta of five; the walk from `SessionScreen` reaches five, and the terminal's paste is not among them. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): probe both clipboard kinds at once and judge sizes against the floor Moving the two clipboard probes into an object literal serialised them: the migrated `contents()` awaited `hasStringAsync` before `hasImageAsync` was called, where both callers had used `Promise.all`. That path runs on mount, on every AppState foreground and on every select-mode toggle. Restored, with an ordering probe that deadlocks unless both probes start before either answers. The floor case could not fail for the reason it named: its fixture declared 16, so a census carrying its own copy of 16 passed it. It now plants a seam declaring 20 and a literal 18, the size that is clean under one floor and an offence under the other. Two stale wordings from the reverted split: one closure case still said "both split style modules" over a one-element list. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): buzz the quick-command row when a copy is refused The last of the seven migrated writes without the error haptic. The row already said "Couldn't copy" on its own control, in red, for the 1500 ms the toast the other six show would have lasted, so it never claimed a refused write had landed; what it had no way to say was anything the thumb still on the button could feel. Its first test, on the harness its list already uses: the seam rejects when the pasteboard refuses, and the two cases are the difference between the row that shows a green check over nothing copied and the row that does not. The list's own test gains the haptics mock the row's new import needs. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): make the clipboard census require the await its rule depends on `hasFailureBranch` accepted any enclosing `try` with a `catch`, so the one shape the census exists to stop passed it: `void clipboard.writeText(...)` inside a try/catch is an unhandled rejection with a handler three lines above it that can never run, because the block returns before the promise settles. It now requires the call to be awaited inside the try's own block, or to carry a `.catch` along its own chain. The boundary walk stopped only at function and method declarations, so a `catch` outside an arrow answered for the call left running inside it; every function-like node ends the search now. Five cases over snippets read through the same reader, because a `void` write would have to be committed to be tested against the real tree. Control on a real site: making the Markdown write un-awaited inside its own try reports it. Two provenance fixes. The runtime-string delta across C7.2 is two literals, not one: "Couldn't copy path" took the count from 532 to 533 and "Couldn't copy" took it to 534. And main's C7.4 made `BRIDGE_CLIPBOARD_MIMES` `['text']`, so an image mime is a value the schema does not admit rather than a refusal the verb spells out, with `native.media.pick { source: 'clipboard' }` waiting on C7.6; the web seam and its test said otherwise. Behaviour unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): let an unmounted quick-command row emit nothing on a refusal The haptic I added ran before the mounted guard, so a copy pressed on a row that then scrolled out of the list, or a sheet closed over it, still buzzed when the rejection arrived. A buzz with no row to explain it is feedback for nothing, and the guard was already there for the feedback state one line below. Red-first: press, unmount, then reject. The success path already guarded first. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): make the clipboard census require a catch with something in it Any `.catch` property access counted as a failure branch, so two shapes that handle nothing passed: `clipboard.writeText(text).catch` reads the handler's name and registers nothing, and `.catch()` swallows the rejection while the caller goes on to say the write landed. The rule now requires `.catch` to be the callee of a call carrying at least one argument. Red-first with both shapes in the snippet reader, the accepting cases unchanged. Control on the real tree: emptying the notes sheet's handler reports `MobileSessionSheets.tsx:174`, and restoring it greens. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- ...le-web-app-session-external-links.test.mjs | 81 ++++++ ...obile-web-app-session-text-inputs.test.mjs | 76 ++++++ ...eb-app-source-control-text-inputs.test.mjs | 84 +++++- ...bile-web-app-text-input-font-size-seam.mjs | 62 ++++- .../src/components/CustomKeyModal.styles.ts | 32 ++- mobile/src/components/MobileHtmlPreview.tsx | 5 +- .../components/MobileRichMarkdownEditor.tsx | 5 +- mobile/src/components/TextInputModal.tsx | 3 +- .../bridge/use-native-verbs.ts | 17 +- .../router-seam-census.test-support.ts | 44 ++++ mobile/src/platform/clipboard.test.tsx | 86 +++++- mobile/src/platform/clipboard.ts | 54 ++++ mobile/src/platform/clipboard.web.test.tsx | 102 ++++++- mobile/src/platform/clipboard.web.ts | 37 ++- .../src/platform/text-input-font-size.test.ts | 42 ++- .../src/platform/text-input-font-size.web.ts | 13 +- mobile/src/session/MobileNativeChatAsk.tsx | 3 +- .../src/session/MobileNativeChatComposer.tsx | 17 +- .../src/session/MobileNativeChatQuestion.tsx | 15 +- mobile/src/session/MobileSessionSheets.tsx | 18 +- mobile/src/session/QuickCommandEditorForm.tsx | 3 +- mobile/src/session/QuickCommandRow.test.ts | 148 +++++++++++ mobile/src/session/QuickCommandRow.tsx | 11 +- mobile/src/session/QuickCommandsList.test.ts | 8 + mobile/src/session/QuickCommandsList.tsx | 3 +- ...pboard-write-failure-branch-census.test.ts | 249 ++++++++++++++++++ .../mobile-native-chat-input-base-styles.ts | 24 ++ .../mobile-native-chat-input-styles.ts | 17 ++ ...obile-native-chat-input-styles.web.test.ts | 93 +++++++ .../mobile-native-chat-input-styles.web.ts | 17 ++ .../mobile-session-command-input-styles.ts | 3 +- .../mobile-session-route-parity.test.ts | 28 +- .../session-router-seam-census.test.ts | 76 ++++++ .../session/use-mobile-file-tap-handlers.ts | 4 +- .../use-mobile-session-accessory-selection.ts | 17 +- .../session/use-mobile-session-attachments.ts | 12 +- .../use-mobile-session-diff-comments.ts | 7 +- .../use-mobile-session-file-actions.ts | 4 +- .../session/use-mobile-session-foundation.ts | 5 +- .../use-mobile-session-markdown-actions.ts | 16 +- .../session/use-mobile-terminal-paste.test.ts | 8 +- .../src/session/use-mobile-terminal-paste.ts | 8 +- .../use-notification-pane-navigation.ts | 5 +- mobile/web-entry/web-overrides.json | 4 + 44 files changed, 1443 insertions(+), 123 deletions(-) create mode 100644 config/scripts/mobile-web-app-session-external-links.test.mjs create mode 100644 config/scripts/mobile-web-app-session-text-inputs.test.mjs create mode 100644 mobile/src/session/QuickCommandRow.test.ts create mode 100644 mobile/src/session/clipboard-write-failure-branch-census.test.ts create mode 100644 mobile/src/session/mobile-native-chat-input-base-styles.ts create mode 100644 mobile/src/session/mobile-native-chat-input-styles.ts create mode 100644 mobile/src/session/mobile-native-chat-input-styles.web.test.ts create mode 100644 mobile/src/session/mobile-native-chat-input-styles.web.ts create mode 100644 mobile/src/session/session-router-seam-census.test.ts diff --git a/config/scripts/mobile-web-app-session-external-links.test.mjs b/config/scripts/mobile-web-app-session-external-links.test.mjs new file mode 100644 index 00000000000..2a1a742c67c --- /dev/null +++ b/config/scripts/mobile-web-app-session-external-links.test.mjs @@ -0,0 +1,81 @@ +/** + * What the session screen may reach for a URL, and whose clipboard it writes. + * + * Inside the shell's WebView react-native-web's `Linking.openURL` calls + * `window.open(url, '_blank', 'noopener')`, which both shells refuse — iOS returns nil from + * `createWebViewWith`, Android false from `onCreateWindow` — and resolves whether or not anything + * opened. A call site left on that path reports success into a tap that did nothing, which is the + * one failure the `externalLink` grant exists to remove. + * + * This screen's openers are a terminal link tap whose open mode is the phone's browser, and the two + * WebView-backed readers it reaches through the file and Markdown panels, each of which sends a + * tapped link to the system browser rather than navigating the artifact away. + * + * The rule, not the three call sites it happens to have today: a module entering this closure later + * is held to it without anyone remembering to add it here. + */ +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' +import { mobileWebAppRouteClosure } from './build-mobile-web-app-bundle.mjs' +import { mobileWebAppDependenciesPresent } from './mobile-web-app-bundle-dependencies.mjs' +import { + EXTERNAL_LINK_SEAM as SEAM, + externalLinkOffenders +} from './mobile-web-app-external-link-seam.mjs' + +const mobileDir = fileURLToPath(new URL('../../mobile/', import.meta.url)) +const describeClosure = mobileWebAppDependenciesPresent() ? describe : describe.skip + +const SESSION = 'app/h/[hostId]/session/[worktreeId].tsx' + +/** The clipboard seam, as the web build resolves it. */ +const CLIPBOARD_SEAM = 'src/platform/clipboard.web.ts' + +describeClosure( + 'the session screen closure', + () => { + it('opens every external URL through the platform seam', async () => { + const closure = await mobileWebAppRouteClosure(SESSION) + expect(externalLinkOffenders(mobileDir, closure)).toEqual([]) + }) + + it('contains the seam, so the rule above is not vacuous', async () => { + // Without this an empty offender list would also be what a closure that reaches no link code + // at all produces, and the census would pass against a page that opens nothing. + const closure = await mobileWebAppRouteClosure(SESSION) + expect(closure.local).toContain(SEAM) + expect(closure.local.length).toBeGreaterThan(900) + }) + }, + 240_000 +) + +/** + * The screen does not touch the clipboard through the browser's own. + * + * `expo-clipboard` resolves to `ExpoClipboard.web.js`, which is `navigator.clipboard`: it needs a + * secure context, and the iOS shell serves the page from a custom scheme while Android serves + * `https`, so that path works on one platform and silently not on the other. This screen is the + * heaviest clipboard user in the app — a quick command's body, a diff note, a Markdown document, a + * terminal selection, a structured send prompt, and the terminal's own paste — so all of it goes + * through the seam and none of it through the browser. + * + * Asserted as the module's absence from the closure rather than as a count of importers: a new + * import anywhere in the tree puts the file back, whoever writes it and whatever they name it. + */ +describeClosure( + 'the clipboard the session screen reaches', + () => { + it("does not carry expo-clipboard's web module at all", async () => { + const closure = await mobileWebAppRouteClosure(SESSION) + expect(closure.modules.filter((file) => file.endsWith('ExpoClipboard.web.js'))).toEqual([]) + }) + + it('carries the seam that replaced it, so the absence above is not vacuous', async () => { + // An empty list is also what a closure reaching no clipboard code at all would produce. + const closure = await mobileWebAppRouteClosure(SESSION) + expect(closure.local).toContain(CLIPBOARD_SEAM) + }) + }, + 240_000 +) diff --git a/config/scripts/mobile-web-app-session-text-inputs.test.mjs b/config/scripts/mobile-web-app-session-text-inputs.test.mjs new file mode 100644 index 00000000000..c8b1f76bb1c --- /dev/null +++ b/config/scripts/mobile-web-app-session-text-inputs.test.mjs @@ -0,0 +1,76 @@ +/** + * Every text input the session screen reaches, and the size it declares. + * + * iOS zooms the page on focus of any input under 16px and does not zoom back out, so the document + * spends the rest of that typing session at a scale other than 1 — which `keyboard-occlusion.web.ts` + * reads as "not a keyboard" on purpose, because geometry cannot separate a zoom from a keyboard. + * + * This closure is the one that cannot afford it. The terminal's own input is a hidden field the + * keyboard seam's geometry is the whole basis of, and this screen reaches nine inputs that declare + * a size — a custom-key capture field, a prompt modal, the chat's ask/composer/question fields, the + * quick-command editor and its search, and the terminal command bar. One of them left off the seam + * leaves every later focus on this screen measuring a zoomed document. + * + * The rule is the closure rather than the nine sites it happens to have today: a module entering it + * later is held to it without anyone remembering to add it here. + */ +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' +import { mobileWebAppRouteClosure } from './build-mobile-web-app-bundle.mjs' +import { mobileWebAppDependenciesPresent } from './mobile-web-app-bundle-dependencies.mjs' +import { + TEXT_INPUT_FONT_SIZE_SEAM, + textInputFontSizeOffenders, + unresolvedTextInputStyles +} from './mobile-web-app-text-input-font-size-seam.mjs' + +const mobileDir = fileURLToPath(new URL('../../mobile/', import.meta.url)) +const describeClosure = mobileWebAppDependenciesPresent() ? describe : describe.skip + +const SESSION = 'app/h/[hostId]/session/[worktreeId].tsx' + +/** + * The style module this screen splits, as the page bundle resolves it. + * + * Named rather than left to the offender list because a split is the one fix that can be undone + * without reopening the offence: delete the `.web.ts` and the native sibling's size is what the + * page runs, which is a 15px chat composer and a zoomed document, and the offender list would say + * so — but only the next time someone reads it. Listed here, the closure says which file the page + * loads. + */ +const SPLIT_WEB_STYLES = ['src/session/mobile-native-chat-input-styles.web.ts'] + +describeClosure( + 'the text inputs the session screen reaches', + () => { + it('takes every input size through the seam', async () => { + const closure = await mobileWebAppRouteClosure(SESSION) + expect(textInputFontSizeOffenders(mobileDir, closure)).toEqual([]) + }) + + it('reads every input it found, so the list above is complete', async () => { + // The completeness half: an empty offender list is evidence only if every input was read. + const closure = await mobileWebAppRouteClosure(SESSION) + expect(unresolvedTextInputStyles(mobileDir, closure)).toEqual([]) + }) + + it('carries the seam, so the rule is not vacuous', async () => { + // Without this an empty offender list would also be what a closure reaching no text input at + // all produces, and the census would pass against a page that has nothing to raise. + const closure = await mobileWebAppRouteClosure(SESSION) + expect(closure.local).toContain(TEXT_INPUT_FONT_SIZE_SEAM) + expect(closure.local.length).toBeGreaterThan(900) + }) + + it('loads the web half of the split style module, not the native one', async () => { + const closure = await mobileWebAppRouteClosure(SESSION) + expect(closure.local).toEqual(expect.arrayContaining(SPLIT_WEB_STYLES)) + expect( + closure.local.filter((file) => + SPLIT_WEB_STYLES.some((web) => file === web.replace('.web.ts', '.ts')) + ) + ).toEqual([]) + }) + }, + 240_000 +) diff --git a/config/scripts/mobile-web-app-source-control-text-inputs.test.mjs b/config/scripts/mobile-web-app-source-control-text-inputs.test.mjs index 332b78b98f7..b4da8e7ee67 100644 --- a/config/scripts/mobile-web-app-source-control-text-inputs.test.mjs +++ b/config/scripts/mobile-web-app-source-control-text-inputs.test.mjs @@ -20,6 +20,7 @@ import { mobileWebAppRouteClosure } from './build-mobile-web-app-bundle.mjs' import { mobileWebAppDependenciesPresent } from './mobile-web-app-bundle-dependencies.mjs' import { TEXT_INPUT_FONT_SIZE_SEAM, + textInputFontSizeFloor, textInputFontSizeOffenders, unresolvedTextInputStyles } from './mobile-web-app-text-input-font-size-seam.mjs' @@ -34,12 +35,26 @@ const REVIEW = 'app/h/[hostId]/review/[worktreeId].tsx' const SEAM_SOURCE = { 'src/platform/text-input-font-size.ts': 'export const TEXT_INPUT_FONT_SIZE = 14' } + +/** + * The seam's web half, seeded into every scratch tree below. + * + * Not a fixture detail: the floor is declared here and the census reads it here, so a tree without + * this file is one the rule refuses to judge at all. Seeding it makes every case a tree with a + * seam, which is what a real one is; the case that checks the refusal writes its own over the top. + */ +const FLOOR_SOURCE = { + 'src/platform/text-input-font-size.web.ts': [ + 'export const TEXT_INPUT_FONT_SIZE_FLOOR = 16', + 'export const TEXT_INPUT_FONT_SIZE = 16' + ].join('\n') +} const SEAM_IMPORT = "import { TEXT_INPUT_FONT_SIZE } from '../platform/text-input-font-size'" /** A scratch module tree, so a planted offender never lands in the tree other censuses walk. */ function plant(files) { const root = mkdtempSync(join(tmpdir(), 'orca-text-input-census-')) - for (const [path, source] of Object.entries(files)) { + for (const [path, source] of Object.entries({ ...FLOOR_SOURCE, ...files })) { mkdirSync(join(root, path.slice(0, path.lastIndexOf('/'))), { recursive: true }) writeFileSync(join(root, path), source) } @@ -191,6 +206,73 @@ describe('the size a text input declares, as the census reads it', () => { } }) + /** + * The floor is the rule and the seam is the mechanism, so a literal already clear of the floor + * satisfies it without binding to anything. + * + * Written as three sizes rather than one: a rule that only proved 22 passes would also be + * satisfied by a census that stopped reading literals at all, and the 15 is the case the whole + * seam exists for. The boundary is included because "at or above" is where an off-by-one lives. + */ + it.each([ + ['under the floor, which is the offence the seam exists for', 15, ['src/ui/Sized.tsx:1']], + ['exactly the floor', 16, []], + ['well above the floor, which no binding could keep', 22, []] + ])('reads a literal %s', (_label, size, expected) => { + const root = plant({ + 'src/ui/Sized.tsx': `export const Sized = () => `, + ...SEAM_SOURCE + }) + try { + const closure = { local: ['src/ui/Sized.tsx'] } + expect(textInputFontSizeOffenders(root, closure)).toEqual(expected) + expect(unresolvedTextInputStyles(root, closure)).toEqual([]) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + /** + * The floor really is the seam's, checked against a seam that does not say 16. + * + * The first version of this case planted a floor of 16 and asserted the census read 16, which a + * census carrying its own copy of the number passes just as happily. A tree whose seam says 20 + * is the only fixture that can tell the two apart, and the 18 below is the size that is clean + * under one floor and an offence under the other. + */ + it('judges against the floor the seam declares, not against a number of its own', () => { + const root = plant({ + 'src/ui/Sized.tsx': 'export const Sized = () => ', + 'src/platform/text-input-font-size.web.ts': [ + 'export const TEXT_INPUT_FONT_SIZE_FLOOR = 20', + 'export const TEXT_INPUT_FONT_SIZE = 20' + ].join('\n') + }) + try { + expect(textInputFontSizeFloor(root)).toBe(20) + expect(textInputFontSizeOffenders(root, { local: ['src/ui/Sized.tsx'] })).toEqual([ + 'src/ui/Sized.tsx:1' + ]) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + it('refuses to judge a tree whose seam declares no floor, rather than assuming one', () => { + const root = plant({ + 'src/ui/Sized.tsx': 'export const Sized = () => ', + 'src/platform/text-input-font-size.ts': 'export const TEXT_INPUT_FONT_SIZE = 14', + 'src/platform/text-input-font-size.web.ts': 'export const TEXT_INPUT_FONT_SIZE = 16' + }) + try { + expect(() => textInputFontSizeOffenders(root, { local: ['src/ui/Sized.tsx'] })).toThrow( + /declares no numeric/ + ) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + it('names a style shape it cannot follow rather than dropping it', () => { const root = plant({ 'src/ui/Called.tsx': 'export const Called = () => ' diff --git a/config/scripts/mobile-web-app-text-input-font-size-seam.mjs b/config/scripts/mobile-web-app-text-input-font-size-seam.mjs index 5447f016081..758252e0886 100644 --- a/config/scripts/mobile-web-app-text-input-font-size-seam.mjs +++ b/config/scripts/mobile-web-app-text-input-font-size-seam.mjs @@ -18,8 +18,46 @@ export const TEXT_INPUT_FONT_SIZE_SEAM = 'src/platform/text-input-font-size.web. const SEAM_EXPORT = 'TEXT_INPUT_FONT_SIZE' const SEAM_MODULE = 'src/platform/text-input-font-size.ts' +/** The floor's name in the seam's web half, which is the only place the number is written. */ +const FLOOR_EXPORT = 'TEXT_INPUT_FONT_SIZE_FLOOR' + const parse = (file, source) => ts.createSourceFile(file, source, ts.ScriptTarget.Latest, true) +/** One read per tree: the file does not change under a run, and every style asks for it. */ +const floorByRoot = new Map() + +/** + * The size at or above which an input cannot make iOS zoom the page, read from the seam itself. + * + * Read rather than restated, and that is the whole reason this rule can exist: the seam's web half + * already computes `Math.max(bodySize, floor)`, so a census that wrote `16` beside it would be a + * second copy of the one number the seam is for, and the two would drift in the direction nobody + * reads again. + * + * Absent is a throw rather than a default. A census that silently fell back to a number of its own + * would go on passing while the thing it measures against had moved or gone. + */ +export function textInputFontSizeFloor(mobileDir) { + const cached = floorByRoot.get(mobileDir) + if (cached !== undefined) { + return cached + } + const source = readOrNull(join(mobileDir, TEXT_INPUT_FONT_SIZE_SEAM)) + if (source === null) { + throw new Error(`[text-input-font-size-seam] no seam at ${TEXT_INPUT_FONT_SIZE_SEAM}`) + } + const parsed = parse(TEXT_INPUT_FONT_SIZE_SEAM, source) + const declared = declarationOf(parsed, FLOOR_EXPORT) + if (declared === null || !ts.isNumericLiteral(declared)) { + throw new Error( + `[text-input-font-size-seam] ${TEXT_INPUT_FONT_SIZE_SEAM} declares no numeric ${FLOOR_EXPORT}` + ) + } + const floor = Number(declared.text) + floorByRoot.set(mobileDir, floor) + return floor +} + function readOrNull(path) { try { return readFileSync(path, 'utf8') @@ -371,7 +409,25 @@ function resolveStyleKey(mobileDir, file, exportName, key, seen = new Set()) { return null } -/** The `fontSize` a style object literal declares, with whether it came through the seam. */ +/** + * Whether a size is a literal that already clears the floor. + * + * The floor is the rule and the seam is the mechanism, so a style that declares a number at or + * above it satisfies the rule without binding to anything: 22 on a capture field cannot zoom a + * page, and making it read the seam would have lowered it to 16 to satisfy a census. A literal + * under the floor is still an offence, which is the case the rule was written for. + * + * Literals only. `typography.bodySize + 1` is 15 today and whatever the theme says tomorrow, and a + * census that evaluated expressions would be a second renderer. + */ +function isLiteralAtOrAboveFloor(mobileDir, initializer) { + return ( + ts.isNumericLiteral(initializer) && + Number(initializer.text) >= textInputFontSizeFloor(mobileDir) + ) +} + +/** The `fontSize` a style object literal declares, with whether the rule is satisfied. */ function fontSizeIn(mobileDir, parsed, file, object) { for (const entry of object.properties) { if (ts.isPropertyAssignment(entry) && entry.name.getText() === 'fontSize') { @@ -379,7 +435,9 @@ function fontSizeIn(mobileDir, parsed, file, object) { file, text: entry.initializer.getText(), line: parsed.getLineAndCharacterOfPosition(entry.getStart(parsed)).line + 1, - onSeam: isSeamBinding(mobileDir, parsed, file, entry.initializer) + onSeam: + isSeamBinding(mobileDir, parsed, file, entry.initializer) || + isLiteralAtOrAboveFloor(mobileDir, entry.initializer) } } } diff --git a/mobile/src/components/CustomKeyModal.styles.ts b/mobile/src/components/CustomKeyModal.styles.ts index 991e557441f..ad710d68735 100644 --- a/mobile/src/components/CustomKeyModal.styles.ts +++ b/mobile/src/components/CustomKeyModal.styles.ts @@ -1,5 +1,6 @@ import { StyleSheet } from 'react-native' import { colors, spacing, radii, typography } from '../theme/mobile-theme' +import { TEXT_INPUT_FONT_SIZE } from '../platform/text-input-font-size' export const customKeyModalStyles = StyleSheet.create({ header: { @@ -7,6 +8,22 @@ export const customKeyModalStyles = StyleSheet.create({ alignItems: 'center', paddingBottom: spacing.sm }, + keyInput: { + width: '100%', + height: 56, + borderRadius: 10, + backgroundColor: colors.bgPanel, + borderWidth: 1, + borderColor: colors.borderSubtle, + color: colors.textPrimary, + fontFamily: typography.monoFamily, + // 22 on both platforms, and no binding to the seam: the floor is the rule and this clears it, + // so reading the seam here would have lowered a one-character capture field to 16 to satisfy + // a census. C7 ruling 12. + fontSize: 22, + fontWeight: '600', + textAlign: 'center' + }, backButton: { width: 30, height: 30, @@ -153,19 +170,6 @@ export const customKeyModalStyles = StyleSheet.create({ chipGlyphSelected: { color: 'rgba(10,10,10,0.5)' }, - keyInput: { - width: '100%', - height: 56, - borderRadius: 10, - backgroundColor: colors.bgPanel, - borderWidth: 1, - borderColor: colors.borderSubtle, - color: colors.textPrimary, - fontFamily: typography.monoFamily, - fontSize: 22, - fontWeight: '600', - textAlign: 'center' - }, moreLink: { paddingVertical: spacing.sm, alignItems: 'center' @@ -240,7 +244,7 @@ export const customKeyModalStyles = StyleSheet.create({ borderRadius: radii.input, paddingHorizontal: spacing.md, paddingVertical: spacing.sm, - fontSize: 14, + fontSize: TEXT_INPUT_FONT_SIZE, fontFamily: typography.monoFamily, borderWidth: 1, borderColor: colors.borderSubtle diff --git a/mobile/src/components/MobileHtmlPreview.tsx b/mobile/src/components/MobileHtmlPreview.tsx index 335d2600ff9..dfee78b49c6 100644 --- a/mobile/src/components/MobileHtmlPreview.tsx +++ b/mobile/src/components/MobileHtmlPreview.tsx @@ -1,7 +1,8 @@ import { useState } from 'react' -import { Linking, Pressable, StyleSheet, Text, View } from 'react-native' +import { Pressable, StyleSheet, Text, View } from 'react-native' import { WebView } from 'react-native-webview' import { Code, Eye } from 'lucide-react-native' +import { openExternalLink } from '../platform/external-link' import { colors, spacing, typography } from '../theme/mobile-theme' type Props = { @@ -50,7 +51,7 @@ export function MobileHtmlPreview({ html, renderSource }: Props) { if (request.url === 'about:blank' || request.url.startsWith('data:')) { return true } - void Linking.openURL(request.url).catch(() => {}) + openExternalLink(request.url) return false }} /> diff --git a/mobile/src/components/MobileRichMarkdownEditor.tsx b/mobile/src/components/MobileRichMarkdownEditor.tsx index 2ad299170ea..b19a1bd0b56 100644 --- a/mobile/src/components/MobileRichMarkdownEditor.tsx +++ b/mobile/src/components/MobileRichMarkdownEditor.tsx @@ -8,7 +8,8 @@ import { type ComponentType, type ForwardedRef } from 'react' -import { Keyboard, Linking, Pressable, ScrollView, StyleSheet, View } from 'react-native' +import { Keyboard, Pressable, ScrollView, StyleSheet, View } from 'react-native' +import { openExternalLink } from '../platform/external-link' import { Bold, Code2, @@ -109,7 +110,7 @@ function MobileRichMarkdownEditorInner( onOpenLink(url) return } - void Linking.openURL(url).catch(() => {}) + openExternalLink(url) }, [onOpenLink] ) diff --git a/mobile/src/components/TextInputModal.tsx b/mobile/src/components/TextInputModal.tsx index 717b14d8e67..3fd007d6a5c 100644 --- a/mobile/src/components/TextInputModal.tsx +++ b/mobile/src/components/TextInputModal.tsx @@ -9,6 +9,7 @@ import { type KeyboardTypeOptions } from 'react-native' import { colors, spacing, radii, typography } from '../theme/mobile-theme' +import { TEXT_INPUT_FONT_SIZE } from '../platform/text-input-font-size' import { BottomDrawer } from './BottomDrawer' type Props = { @@ -133,7 +134,7 @@ const styles = StyleSheet.create({ borderRadius: radii.input, paddingHorizontal: spacing.md, paddingVertical: Platform.OS === 'ios' ? spacing.sm + 2 : spacing.sm, - fontSize: typography.bodySize, + fontSize: TEXT_INPUT_FONT_SIZE, borderWidth: 1, borderColor: colors.borderSubtle }, diff --git a/mobile/src/mobile-web-shell/bridge/use-native-verbs.ts b/mobile/src/mobile-web-shell/bridge/use-native-verbs.ts index 4b592b9de2e..a568926fd0d 100644 --- a/mobile/src/mobile-web-shell/bridge/use-native-verbs.ts +++ b/mobile/src/mobile-web-shell/bridge/use-native-verbs.ts @@ -24,8 +24,17 @@ import { * a screen that read a field which is not there. */ export type NativeVerbs = { - /** Whether this shell serves the verbs at all; false leaves a caller its own fallback. */ + /** Whether this shell serves the clipboard verbs at all; false leaves a caller its own fallback. */ granted: boolean + /** + * Per verb, because the grants are per verb and a caller usually wants one of them. + * + * `granted` is both, which is the right question for a screen that copies and pastes and the + * wrong one for anything else: a route granted only `native.clipboard.read` reads `granted` + * false and would report an empty clipboard rather than one it is allowed to read. + */ + canWriteClipboardText: boolean + canReadClipboardText: boolean writeClipboardText: (value: string) => Promise readClipboardText: () => Promise } @@ -145,8 +154,12 @@ export function useNativeVerbs(): NativeVerbs { } const mime: BridgeClipboardMime = 'text' + const canWriteClipboardText = has('native.clipboard.write') + const canReadClipboardText = has('native.clipboard.read') return { - granted: has('native.clipboard.write') && has('native.clipboard.read'), + granted: canWriteClipboardText && canReadClipboardText, + canWriteClipboardText, + canReadClipboardText, writeClipboardText: async (value) => (await call('native.clipboard.write', { mime, value }, clipboardWriteResultSchema)).written, readClipboardText: async () => diff --git a/mobile/src/navigation/router-seam-census.test-support.ts b/mobile/src/navigation/router-seam-census.test-support.ts index 41f974620eb..28d634ad0a1 100644 --- a/mobile/src/navigation/router-seam-census.test-support.ts +++ b/mobile/src/navigation/router-seam-census.test-support.ts @@ -59,3 +59,47 @@ export function callsRouteHandoff(source: ts.SourceFile): boolean { ts.forEachChild(source, visit) return found } + +/** + * Every value name a module imports from expo-router, so a domain can say which ones it allows. + * + * The two landed censuses answer "no value import at all", which is the right rule for a domain + * whose only reach into expo-router is a router. The session domain's is not: eight of its hooks + * take `useFocusEffect` and two take `useLocalSearchParams`, neither of which can navigate, and a + * blanket rule there would have to be turned off rather than narrowed. + * + * Names rather than a boolean for `useRouter`, because the hazard is the category and not the one + * spelling of it: `import { router }` is expo-router's module singleton and navigates from anywhere, + * and a rule written against `useRouter` alone would have read it as clean. + * + * The imported name, not the local one: `import { useRouter as useAppRouter }` is the same import. + */ +export function expoRouterValueImports(source: ts.SourceFile): string[] { + const names = new Set() + for (const statement of source.statements) { + if (!ts.isImportDeclaration(statement) || statement.importClause?.isTypeOnly === true) { + continue + } + const specifier = statement.moduleSpecifier + if (!ts.isStringLiteral(specifier) || specifier.text !== 'expo-router') { + continue + } + const bindings = statement.importClause?.namedBindings + if (bindings !== undefined && ts.isNamedImports(bindings)) { + for (const element of bindings.elements) { + if (element.isTypeOnly) { + continue + } + names.add((element.propertyName ?? element.name).text) + } + } + // A default or namespace import hands the whole module over under one name, router included. + if (statement.importClause?.name !== undefined) { + names.add('default') + } + if (bindings !== undefined && ts.isNamespaceImport(bindings)) { + names.add('*') + } + } + return [...names].sort() +} diff --git a/mobile/src/platform/clipboard.test.tsx b/mobile/src/platform/clipboard.test.tsx index 4a930e10489..b0635916004 100644 --- a/mobile/src/platform/clipboard.test.tsx +++ b/mobile/src/platform/clipboard.test.tsx @@ -1,13 +1,19 @@ /** The native form of the clipboard seam: the app's own `expo-clipboard`, and what it answers. */ import { act, create } from 'react-test-renderer' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import type { ClipboardWriter } from './clipboard' +import type { ClipboardReader, ClipboardWriter } from './clipboard' -const clipboard = vi.hoisted(() => ({ setStringAsync: vi.fn(() => Promise.resolve(true)) })) +const clipboard = vi.hoisted(() => ({ + setStringAsync: vi.fn(() => Promise.resolve(true)), + getStringAsync: vi.fn(() => Promise.resolve('')), + getImageAsync: vi.fn(() => Promise.resolve(null)), + hasStringAsync: vi.fn(() => Promise.resolve(false)), + hasImageAsync: vi.fn(() => Promise.resolve(false)) +})) vi.mock('expo-clipboard', () => clipboard) -import { useClipboardWriter } from './clipboard' +import { useClipboardReader, useClipboardWriter } from './clipboard' /** The hook as a screen holds it; `react-test-renderer` is what every other seam test here uses. */ function mountWriter(): ClipboardWriter { @@ -26,9 +32,34 @@ function mountWriter(): ClipboardWriter { return writer } +/** The reader as a screen holds it, mounted the same way. */ +function mountReader(): ClipboardReader { + const held: { reader: ClipboardReader | null } = { reader: null } + function Screen(): null { + held.reader = useClipboardReader() + return null + } + act(() => { + create() + }) + const reader = held.reader + if (reader === null) { + throw new Error('nothing mounted') + } + return reader +} + beforeEach(() => { clipboard.setStringAsync.mockReset() clipboard.setStringAsync.mockImplementation(() => Promise.resolve(true)) + clipboard.getStringAsync.mockReset() + clipboard.getStringAsync.mockImplementation(() => Promise.resolve('')) + clipboard.getImageAsync.mockReset() + clipboard.getImageAsync.mockImplementation(() => Promise.resolve(null)) + clipboard.hasStringAsync.mockReset() + clipboard.hasStringAsync.mockImplementation(() => Promise.resolve(false)) + clipboard.hasImageAsync.mockReset() + clipboard.hasImageAsync.mockImplementation(() => Promise.resolve(false)) }) afterEach(() => { @@ -50,3 +81,52 @@ describe('writing the clipboard on a phone', () => { await expect(writer.writeText('copied')).rejects.toThrow(/did not accept/) }) }) + +describe('reading the clipboard on a phone', () => { + it('hands back the text the app has', async () => { + clipboard.getStringAsync.mockImplementation(() => Promise.resolve('pasted')) + await expect(mountReader().readText()).resolves.toBe('pasted') + }) + + it('asks for a PNG, which is the format the upload path re-encodes to', async () => { + await expect(mountReader().readImage()).resolves.toBeNull() + expect(clipboard.getImageAsync.mock.calls).toEqual([[{ format: 'png' }]]) + }) + + it('starts both probes before either has answered', async () => { + // Awaiting them in turn puts an IPC round trip on the critical path of every mount, every + // foreground and every select-mode toggle, which is where these callers run. The order is the + // subject, so neither probe resolves until both have been called. + const started: string[] = [] + let releaseString = (): void => {} + clipboard.hasStringAsync.mockImplementation( + () => + new Promise((resolve) => { + started.push('string') + releaseString = () => resolve(true) + }) + ) + clipboard.hasImageAsync.mockImplementation(() => { + started.push('image') + // The image probe answers first: with a sequential await this line is never reached, because + // nothing would have called it before the text probe settled. + releaseString() + return Promise.resolve(false) + }) + await expect(mountReader().contents()).resolves.toEqual({ text: true, image: false }) + expect(started).toEqual(['string', 'image']) + }) + + it('probes both kinds without reading either', async () => { + clipboard.hasImageAsync.mockImplementation(() => Promise.resolve(true)) + await expect(mountReader().contents()).resolves.toEqual({ text: false, image: true }) + // The probe is the whole point on iOS: reading to find out raises the paste-consent prompt. + expect(clipboard.getStringAsync).not.toHaveBeenCalled() + expect(clipboard.getImageAsync).not.toHaveBeenCalled() + }) + + it('reads a probe that threw as absent, rather than disabling paste on a rejection', async () => { + clipboard.hasStringAsync.mockImplementation(() => Promise.reject(new Error('no pasteboard'))) + await expect(mountReader().contents()).resolves.toEqual({ text: false, image: false }) + }) +}) diff --git a/mobile/src/platform/clipboard.ts b/mobile/src/platform/clipboard.ts index 36aa93c08fe..c63dc86ff5d 100644 --- a/mobile/src/platform/clipboard.ts +++ b/mobile/src/platform/clipboard.ts @@ -26,3 +26,57 @@ export function useClipboardWriter(): ClipboardWriter { [] ) } + +/** + * What is on the clipboard, without reading it. + * + * Two flags rather than one because the callers act on them differently: text pastes into the + * terminal as keystrokes and an image is uploaded first, and the accessory row enables its paste + * button for either. + */ +export type ClipboardContents = { text: boolean; image: boolean } + +/** + * A PNG off the clipboard, in the shape the image upload path already takes. + * + * Structural rather than re-exported from `expo-clipboard`, so the web sibling can answer without + * the package: a type imported from a module the page never loads is a runtime import the bundler + * cannot drop. + */ +export type ClipboardImage = { data: string; size: { width: number; height: number } } + +/** + * Reading the device clipboard, which is three calls on a phone and one verb on the web. + * + * `readImage` answers null for "nothing there", which is what `getImageAsync` answers and what the + * terminal's paste already branches on — so the page's lack of an image verb degrades into the + * path that was always there rather than into a new error. + */ +export type ClipboardReader = { + readText: () => Promise + readImage: () => Promise + contents: () => Promise +} + +export function useClipboardReader(): ClipboardReader { + return useMemo( + () => ({ + readText: async () => await Clipboard.getStringAsync(), + readImage: async () => await Clipboard.getImageAsync({ format: 'png' }), + // Swallowed here rather than at each caller, which is where it already was: a probe that + // threw would disable the paste button, and every platform reason for it to throw is one + // the read itself reports better. + // Both probes started before either is awaited, which is what the call sites did before this + // seam existed. Awaiting them in turn puts an IPC round trip on the critical path of every + // mount, every foreground and every select-mode toggle. + contents: async () => { + const [text, image] = await Promise.all([ + Clipboard.hasStringAsync().catch(() => false), + Clipboard.hasImageAsync().catch(() => false) + ]) + return { text, image } + } + }), + [] + ) +} diff --git a/mobile/src/platform/clipboard.web.test.tsx b/mobile/src/platform/clipboard.web.test.tsx index 9a76bd31a06..01edf0d66f1 100644 --- a/mobile/src/platform/clipboard.web.test.tsx +++ b/mobile/src/platform/clipboard.web.test.tsx @@ -24,13 +24,17 @@ import { createFakeBridgePortPair, type BridgePortPair } from '../mobile-web-shell/bridge/bridge-port-pair-test-harness' -import { useClipboardWriter } from './clipboard.web' -import type { ClipboardWriter } from './clipboard' +import { useClipboardReader, useClipboardWriter } from './clipboard.web' +import type { ClipboardReader, ClipboardWriter } from './clipboard' -const held: { writer: ClipboardWriter | null } = { writer: null } +const held: { writer: ClipboardWriter | null; reader: ClipboardReader | null } = { + writer: null, + reader: null +} function Screen(): null { held.writer = useClipboardWriter() + held.reader = useClipboardReader() return null } @@ -54,8 +58,18 @@ async function mount(pair: BridgePortPair): Promise { return writer } +async function mountReader(pair: BridgePortPair): Promise { + await mount(pair) + const reader = held.reader + if (reader === null) { + throw new Error('nothing mounted') + } + return reader +} + beforeEach(() => { held.writer = null + held.reader = null }) describe('writing the clipboard from inside the shell', () => { @@ -91,3 +105,85 @@ describe('writing the clipboard from inside the shell', () => { expect(pair.toShell).toHaveLength(before) }) }) + +describe('reading the clipboard from inside the shell', () => { + it('asks the shell for the text and never the desktop', async () => { + const pair = createFakeBridgePortPair() + const reader = await mountReader(pair) + const read = reader.readText() + await pair.flush() + await expect(read).resolves.toBe('pasteboard') + expect(pair.rpc.requests).toEqual([]) + }) + + it('rejects the read on a route that was not granted it, without sending a frame', async () => { + const pair = createFakeBridgePortPair({ routeGrants: ['navigate', 'storage'] }) + const reader = await mountReader(pair) + const before = pair.toShell.length + const read = reader.readText().catch((error: unknown) => error) + await pair.flush() + expect(String(await read)).toMatch(/did not grant/) + expect(pair.toShell).toHaveLength(before) + }) + + /** + * The degradation, recorded rather than implied. + * + * No shell reads an image for the page yet — `native.clipboard.read` admits only `text`, so an + * image mime is `invalid-params` rather than a refusal of its own, and a 24 MiB base64 image + * cannot cross an 8 MiB reply cap. The pasteboard's image is `native.media.pick + * { source: 'clipboard' }`, landed in C7.4 and unwired until C7.6. So the page answers what an + * empty clipboard answers and the terminal's paste takes the branch it already had. + */ + it('answers no image, without asking the shell for one', async () => { + const pair = createFakeBridgePortPair() + const reader = await mountReader(pair) + const before = pair.toShell.length + await expect(reader.readImage()).resolves.toBeNull() + await pair.flush() + expect(pair.toShell).toHaveLength(before) + }) + + /** + * `contents` is not a probe here and cannot be one: the shell serves no "is there text" verb, and + * reading to find out would raise iOS's paste-consent prompt on every mount and every foreground, + * which is the whole reason the phone has `hasStringAsync`. So it answers what this side knows. + */ + it('reports text as possible when the read verb is granted, and never an image', async () => { + const pair = createFakeBridgePortPair() + const reader = await mountReader(pair) + const before = pair.toShell.length + await expect(reader.contents()).resolves.toEqual({ text: true, image: false }) + await pair.flush() + expect(pair.toShell).toHaveLength(before) + }) + + it('reports no text at all on a route the read verb was withheld from', async () => { + const pair = createFakeBridgePortPair({ routeGrants: ['navigate', 'storage'] }) + const reader = await mountReader(pair) + await expect(reader.contents()).resolves.toEqual({ text: false, image: false }) + }) + + /** + * The read grant on its own is enough to paste, so it is the grant this answers on. + * + * Asking whether both clipboard verbs are granted is the right question for a screen that copies + * and pastes and the wrong one here: a route granted only the read would have been told its + * clipboard was empty, and its paste button would never enable. + */ + it('reports text as possible on a route granted the read but not the write', async () => { + const pair = createFakeBridgePortPair({ + routeGrants: ['navigate', 'storage', 'native.clipboard.read'] + }) + const reader = await mountReader(pair) + await expect(reader.contents()).resolves.toEqual({ text: true, image: false }) + // And the write still refuses, so the pair really is asymmetric rather than both granted. + const writer = held.writer + if (writer === null) { + throw new Error('nothing mounted') + } + const written = writer.writeText('x').catch((error: unknown) => error) + await pair.flush() + expect(String(await written)).toMatch(/did not grant/) + }) +}) diff --git a/mobile/src/platform/clipboard.web.ts b/mobile/src/platform/clipboard.web.ts index cad68fba085..52839c6ff51 100644 --- a/mobile/src/platform/clipboard.web.ts +++ b/mobile/src/platform/clipboard.web.ts @@ -1,6 +1,6 @@ import { useMemo } from 'react' import { useNativeVerbs } from '../mobile-web-shell/bridge/use-native-verbs' -import type { ClipboardWriter } from './clipboard' +import type { ClipboardReader, ClipboardWriter } from './clipboard' /** * Web sibling: the page has no clipboard of its own worth using, so the shell writes for it. @@ -27,3 +27,38 @@ export function useClipboardWriter(): ClipboardWriter { [verbs] ) } + +/** + * Web sibling: the shell reads text for the page, and no shell reads an image for it yet. + * + * `native.clipboard.read` is text and only text: `BRIDGE_CLIPBOARD_MIMES` is `['text']`, so an + * image mime is not a refusal the verb spells out but a value its schema does not admit, answered + * `invalid-params`. Widening it cannot work — `CLIPBOARD_IMAGE_MAX_BASE64_CHARS` is 24 MiB against + * a reply cap of 8 MiB — so an image on the pasteboard is `native.media.pick { source: 'clipboard' }`, + * which C7.4 landed and C7.6 will wire. `readImage` therefore answers null, which is the answer an + * empty clipboard already gives, and the terminal's paste takes the branch it has always taken for + * one. A recorded degradation, not a silent one: on the page an image on the clipboard pastes + * nothing until that wiring lands. + * + * `contents` cannot be a probe. The shell serves no "is there text" verb and reading to find out + * would raise iOS's paste-consent prompt on every mount and every foreground, which is the whole + * reason `hasStringAsync` exists. So it answers what this side actually knows: a shell that granted + * the read verb may have text, and no shell has an image. The paste button is enabled on a maybe + * and the read is what settles it, which is the same order a phone runs when the probe throws. + * + * The read grant specifically, not both: a route granted only `native.clipboard.read` can paste, + * and answering on the pair would tell it its clipboard is empty. + */ +export function useClipboardReader(): ClipboardReader { + const verbs = useNativeVerbs() + + return useMemo( + () => ({ + readText: async () => await verbs.readClipboardText(), + readImage: async () => await Promise.resolve(null), + contents: async () => + await Promise.resolve({ text: verbs.canReadClipboardText, image: false }) + }), + [verbs] + ) +} diff --git a/mobile/src/platform/text-input-font-size.test.ts b/mobile/src/platform/text-input-font-size.test.ts index 09f1f60967b..1b281eda6eb 100644 --- a/mobile/src/platform/text-input-font-size.test.ts +++ b/mobile/src/platform/text-input-font-size.test.ts @@ -15,12 +15,17 @@ import { browserAddressFieldStyles } from '../browser/browser-address-field-styl import { mobileBrowserPaneStyles } from '../browser/mobile-browser-pane-styles' import { listStyles } from '../source-control/mobile-source-control-list-styles' import { mobileDiffReviewControlStyles } from '../components/mobile-diff-review-control-styles' +import { customKeyModalStyles } from '../components/CustomKeyModal.styles' +import { mobileSessionCommandInputStyles } from '../session/mobile-session-command-input-styles' import { typography } from '../theme/mobile-theme' import { TEXT_INPUT_FONT_SIZE } from './text-input-font-size' -import { TEXT_INPUT_FONT_SIZE as WEB_TEXT_INPUT_FONT_SIZE } from './text-input-font-size.web' +import { + TEXT_INPUT_FONT_SIZE_FLOOR, + TEXT_INPUT_FONT_SIZE as WEB_TEXT_INPUT_FONT_SIZE +} from './text-input-font-size.web' /** - * The size the two page-served text inputs carry, on each platform. + * The size every page-served text input carries, on each platform. * * Both halves are asserted from here because a node test resolves the native sibling, so the web * value cannot be read off the style object: the bundler is what swaps the module, and that swap @@ -31,7 +36,15 @@ const MOBILE_ROOT = join(import.meta.dirname, '..', '..') const STYLE_MODULES = [ 'src/source-control/mobile-source-control-list-styles.ts', 'src/components/mobile-diff-review-control-styles.ts', - 'src/browser/mobile-browser-pane-styles.ts' + 'src/browser/mobile-browser-pane-styles.ts', + // The session screen's six, which declare the app's body size and so need no sibling: the + // native seam is that size, so the move is the same number and the swap is the whole change. + 'src/components/CustomKeyModal.styles.ts', + 'src/components/TextInputModal.tsx', + 'src/session/MobileNativeChatAsk.tsx', + 'src/session/QuickCommandEditorForm.tsx', + 'src/session/QuickCommandsList.tsx', + 'src/session/mobile-session-command-input-styles.ts' ] /** @@ -41,16 +54,24 @@ const STYLE_MODULES = [ * seam's value on both platforms the way the four above do. Its web half is where the raise lives, * and that is the file that has to carry the binding. */ -const SPLIT_STYLE_MODULES = ['src/browser/browser-address-field-styles.web.ts'] +const SPLIT_STYLE_MODULES = [ + 'src/browser/browser-address-field-styles.web.ts', + // The session screen's one: the chat's two fields sit at 15, under the floor and not the body + // size, so they keep a native sibling. The custom-key capture field needed no split — 22 clears + // the floor, and the census reads a literal that does as satisfying the rule (C7 ruling 12). + 'src/session/mobile-native-chat-input-styles.web.ts' +] /** The seam's export, so the source check below looks for a binding rather than for a mention. */ const SEAM_EXPORT_NAME = 'TEXT_INPUT_FONT_SIZE' describe('the font size the page-served text inputs carry', () => { it('clears the size iOS zooms the page for, on the web', () => { - // 16 is the floor; below it a focus zooms the document and the keyboard seam, which reads a - // scale other than 1 as no keyboard, stops lifting for the rest of the session. - expect(WEB_TEXT_INPUT_FONT_SIZE).toBeGreaterThanOrEqual(16) + // Below the floor a focus zooms the document, and the keyboard seam reads a scale other than + // 1 as no keyboard and stops lifting for the rest of the session. The number is the seam's + // own, read rather than restated, because the census over every page route reads it too. + expect(WEB_TEXT_INPUT_FONT_SIZE).toBeGreaterThanOrEqual(TEXT_INPUT_FONT_SIZE_FLOOR) + expect(TEXT_INPUT_FONT_SIZE_FLOOR).toBe(16) }) it('leaves a phone rendering exactly what it rendered before', () => { @@ -58,6 +79,13 @@ describe('the font size the page-served text inputs carry', () => { expect(listStyles.commitInput.fontSize).toBe(typography.bodySize) expect(mobileDiffReviewControlStyles.composerInput.fontSize).toBe(typography.bodySize) expect(mobileBrowserPaneStyles.keyboardInput.fontSize).toBe(typography.bodySize) + expect(customKeyModalStyles.fieldInput.fontSize).toBe(typography.bodySize) + // The capture field beside it, which is the one input on this screen no seam touches. + expect(customKeyModalStyles.keyInput.fontSize).toBe(22) + expect(customKeyModalStyles.keyInput.fontSize).toBeGreaterThanOrEqual( + TEXT_INPUT_FONT_SIZE_FLOOR + ) + expect(mobileSessionCommandInputStyles.textInput.fontSize).toBe(typography.bodySize) // The pane's address bar is the one that is split: it keeps the compact size natively, so the // seam reaches it through the `.web.ts` sibling rather than through this constant. expect(browserAddressFieldStyles.input.fontSize).toBe(typography.metaSize) diff --git a/mobile/src/platform/text-input-font-size.web.ts b/mobile/src/platform/text-input-font-size.web.ts index dbb4d6d3ef9..c8c06ee1c8f 100644 --- a/mobile/src/platform/text-input-font-size.web.ts +++ b/mobile/src/platform/text-input-font-size.web.ts @@ -1,7 +1,14 @@ import { typography } from '../theme/mobile-theme' -/** Below this, iOS Safari and every iOS WebView zoom the page when an input takes focus. */ -const IOS_FOCUS_ZOOM_FLOOR = 16 +/** + * Below this, iOS Safari and every iOS WebView zoom the page when an input takes focus. + * + * Exported because the floor is the rule and this constant is the only statement of it. The + * census over a page route's closure reads this number out of this file rather than restating it, + * so an input that declares a literal already at or above it is on the floor by construction and + * needs no binding — and a floor that moved would move both halves together. + */ +export const TEXT_INPUT_FONT_SIZE_FLOOR = 16 /** * Web sibling: the app's body size, raised to the size that stops the page being zoomed. @@ -18,4 +25,4 @@ const IOS_FOCUS_ZOOM_FLOOR = 16 * * `Math.max` rather than the constant, so a theme that raises the body size past 16 keeps it. */ -export const TEXT_INPUT_FONT_SIZE = Math.max(typography.bodySize, IOS_FOCUS_ZOOM_FLOOR) +export const TEXT_INPUT_FONT_SIZE = Math.max(typography.bodySize, TEXT_INPUT_FONT_SIZE_FLOOR) diff --git a/mobile/src/session/MobileNativeChatAsk.tsx b/mobile/src/session/MobileNativeChatAsk.tsx index a5ba0ef7f72..33c9311715e 100644 --- a/mobile/src/session/MobileNativeChatAsk.tsx +++ b/mobile/src/session/MobileNativeChatAsk.tsx @@ -3,6 +3,7 @@ import { Pressable, ScrollView, StyleSheet, Text, TextInput, View } from 'react- import { Check } from 'lucide-react-native' import type { AskAnswerSelection, AskPrompt } from '../../../src/shared/native-chat-ask' import { colors, radii, spacing, typography } from '../theme/mobile-theme' +import { TEXT_INPUT_FONT_SIZE } from '../platform/text-input-font-size' type Props = { prompt: AskPrompt @@ -331,7 +332,7 @@ const styles = StyleSheet.create({ borderColor: colors.borderSubtle, borderRadius: radii.card, color: colors.textPrimary, - fontSize: typography.bodySize, + fontSize: TEXT_INPUT_FONT_SIZE, padding: spacing.sm, minHeight: 44, marginBottom: spacing.xs diff --git a/mobile/src/session/MobileNativeChatComposer.tsx b/mobile/src/session/MobileNativeChatComposer.tsx index 29efb22f547..99764923c40 100644 --- a/mobile/src/session/MobileNativeChatComposer.tsx +++ b/mobile/src/session/MobileNativeChatComposer.tsx @@ -10,7 +10,7 @@ import { View } from 'react-native' import { ArrowUp, ImagePlus, Mic, Square, X } from 'lucide-react-native' -import { colors, radii, spacing, typography } from '../theme/mobile-theme' +import { colors, radii, spacing } from '../theme/mobile-theme' import { getVerifiedNativeChatCommands } from '../../../src/shared/native-chat-agent-profiles' import { structuredSlashCommands } from '../../../src/shared/structured-agent-session-composer' import type { AgentSessionConversationCommand } from '../../../src/shared/agent-session-conversation-command' @@ -30,6 +30,7 @@ import { type MobileNativeChatSessionOptionPickersProps } from './MobileNativeChatSessionOptionPickers' import type { PendingNativeChatImage } from './mobile-native-chat-image-attachment' +import { mobileNativeChatInputStyles } from './mobile-native-chat-input-styles' const NO_FILE_PATHS: string[] = [] const NO_ATTACHMENTS: PendingNativeChatImage[] = [] @@ -248,7 +249,7 @@ export function MobileNativeChatComposer({ { triggerSuccess() showToast('Notes copied') @@ -228,8 +230,13 @@ export function MobileSessionSheets({ controller }: { controller: MobileSessionC const target = markdownActionTarget setMarkdownActionTarget(null) if (target) { - void Clipboard.setStringAsync(target.relativePath || target.filePath) - showToast('Path copied') + void clipboard + .writeText(target.relativePath || target.filePath) + .then(() => showToast('Path copied')) + .catch(() => { + triggerError() + showToast("Couldn't copy path", 1500) + }) } } }, @@ -284,7 +291,8 @@ export function MobileSessionSheets({ controller }: { controller: MobileSessionC const combined = drafts .map((draft) => `# ${draft.title}\n\n${draft.content}`) .join('\n\n---\n\n') - void Clipboard.setStringAsync(combined) + void clipboard + .writeText(combined) .then(() => { setLeaveDrafts(null) leaveSession() diff --git a/mobile/src/session/QuickCommandEditorForm.tsx b/mobile/src/session/QuickCommandEditorForm.tsx index 171bf8ef2ce..90deabe9236 100644 --- a/mobile/src/session/QuickCommandEditorForm.tsx +++ b/mobile/src/session/QuickCommandEditorForm.tsx @@ -2,6 +2,7 @@ import { useState } from 'react' import { View, Text, Pressable, TextInput, StyleSheet, Switch } from 'react-native' import { ChevronDown, ChevronRight } from 'lucide-react-native' import { colors, spacing, radii, typography } from '../theme/mobile-theme' +import { TEXT_INPUT_FONT_SIZE } from '../platform/text-input-font-size' import { MobileAgentIcon } from '../components/MobileAgentIcon' import { getQuickCommandAgentLabel, @@ -250,7 +251,7 @@ const styles = StyleSheet.create({ borderRadius: radii.input, paddingHorizontal: spacing.md, paddingVertical: spacing.sm + 2, - fontSize: 14, + fontSize: TEXT_INPUT_FONT_SIZE, borderWidth: 1, borderColor: colors.borderSubtle }, diff --git a/mobile/src/session/QuickCommandRow.test.ts b/mobile/src/session/QuickCommandRow.test.ts new file mode 100644 index 00000000000..b47e2160bf3 --- /dev/null +++ b/mobile/src/session/QuickCommandRow.test.ts @@ -0,0 +1,148 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { TerminalQuickCommand } from '../../../src/shared/terminal-quick-command-types' +import { QuickCommandRow } from './QuickCommandRow' + +const clipboard = vi.hoisted(() => ({ setStringAsync: vi.fn(() => Promise.resolve(true)) })) +const haptics = vi.hoisted(() => ({ notificationAsync: vi.fn(() => Promise.resolve()) })) + +vi.mock('react-native', () => ({ + Platform: { OS: 'ios' }, + Pressable: 'Pressable', + StyleSheet: { create: (styles: T) => styles, hairlineWidth: 1 }, + Text: 'Text', + View: 'View' +})) + +vi.mock('lucide-react-native', () => ({ + Check: 'Check', + Copy: 'Copy', + Pencil: 'Pencil', + Play: 'Play', + Trash2: 'Trash2' +})) + +vi.mock('expo-clipboard', () => clipboard) + +vi.mock('expo-haptics', () => ({ + ...haptics, + performAndroidHapticsAsync: vi.fn(() => Promise.resolve()), + AndroidHaptics: { Reject: 'reject' }, + NotificationFeedbackType: { Error: 'error', Success: 'success' } +})) + +vi.mock('../components/MobileAgentIcon', () => ({ MobileAgentIcon: 'MobileAgentIcon' })) + +const COMMAND: TerminalQuickCommand = { + id: 'qc-1', + label: 'Run tests', + command: 'run the tests', + appendEnter: true +} + +/** + * The seventh migrated write, and the one that does not answer a refusal with a toast. + * + * A row inside a scrolling list says so on its own control: the copy button's label becomes + * "Couldn't copy" and its icon turns red for the same 1500 ms the toast would have lasted. What it + * had no way to say was anything a thumb could feel, and the button sits under the thumb that just + * pressed it. The seam rejects on a refusal rather than resolving false, so these two cases are the + * difference between the row that shows a green check over nothing copied and the row that does not. + */ +describe('the quick-command row when the pasteboard refuses the text', () => { + let renderer: ReactTestRenderer | null = null + + beforeEach(() => { + clipboard.setStringAsync.mockReset() + haptics.notificationAsync.mockReset() + haptics.notificationAsync.mockImplementation(() => Promise.resolve()) + }) + + afterEach(() => { + act(() => renderer?.unmount()) + renderer = null + }) + + /** + * The copy control, found by its label rather than its position among the row's four buttons. + * + * All four labels the button can carry, because the label is what the copy state changes: a + * finder keyed to one of them stops finding the button in the state it is meant to read. + */ + const COPY_LABELS = new Set([ + `Copy ${COMMAND.label}`, + 'Copied', + "Couldn't copy", + 'Nothing to copy' + ]) + + function copyButton() { + const button = renderer!.root + .findAll((node) => node.props.accessibilityRole === 'button') + .find((node) => COPY_LABELS.has(String(node.props.accessibilityLabel))) + if (button === undefined) { + throw new Error('the row has no copy button') + } + return button + } + + function rowProps() { + return { + command: COMMAND, + first: true, + onLaunch: vi.fn(), + onEdit: vi.fn(), + onDelete: vi.fn(), + disabled: false + } + } + + async function mountAndCopy(): Promise { + await act(async () => { + renderer = create(createElement(QuickCommandRow, rowProps())) + }) + await act(async () => { + copyButton().props.onPress() + }) + } + + it('says it could not copy and buzzes the error', async () => { + clipboard.setStringAsync.mockResolvedValue(false) + await mountAndCopy() + expect(copyButton().props.accessibilityLabel).toBe("Couldn't copy") + expect(haptics.notificationAsync).toHaveBeenCalledWith('error') + }) + + it('emits nothing at all when the row is gone before the refusal arrives', async () => { + // A copy pressed on a row that then scrolls out of the list, or a sheet closed over it. The + // rejection still arrives, and a buzz with no row to explain it is feedback for nothing. + let refuse: ((error: Error) => void) | undefined + clipboard.setStringAsync.mockImplementation( + () => + new Promise((_resolve, reject) => { + refuse = reject + }) + ) + await act(async () => { + renderer = create(createElement(QuickCommandRow, rowProps())) + }) + await act(async () => { + copyButton().props.onPress() + }) + act(() => renderer?.unmount()) + renderer = null + await act(async () => { + refuse?.(new Error('pasteboard refused')) + }) + expect(haptics.notificationAsync).not.toHaveBeenCalled() + }) + + it('shows the copied label and no error buzz when the write lands', async () => { + // The control: a failure assertion is only evidence if the success path reads differently. + clipboard.setStringAsync.mockResolvedValue(true) + await mountAndCopy() + expect(copyButton().props.accessibilityLabel).toBe('Copied') + expect(haptics.notificationAsync).not.toHaveBeenCalled() + }) +}) diff --git a/mobile/src/session/QuickCommandRow.tsx b/mobile/src/session/QuickCommandRow.tsx index 8815d4da5f0..abf2df50d58 100644 --- a/mobile/src/session/QuickCommandRow.tsx +++ b/mobile/src/session/QuickCommandRow.tsx @@ -1,6 +1,7 @@ import { useEffect, useRef, useState } from 'react' import { View, Text, Pressable, StyleSheet } from 'react-native' -import * as Clipboard from 'expo-clipboard' +import { useClipboardWriter } from '../platform/clipboard' +import { triggerError } from '../platform/haptics' import { Check, Copy, Pencil, Play, Trash2 } from 'lucide-react-native' import { colors, spacing, typography } from '../theme/mobile-theme' import { MobileAgentIcon } from '../components/MobileAgentIcon' @@ -33,6 +34,7 @@ export function QuickCommandRow({ onDelete, disabled }: QuickCommandRowProps) { + const clipboard = useClipboardWriter() const isAgent = isAgentQuickCommand(command) const body = getTerminalQuickCommandBody(command) const canCopy = body.trim().length > 0 @@ -67,15 +69,20 @@ export function QuickCommandRow({ return } try { - await Clipboard.setStringAsync(body) + await clipboard.writeText(body) if (!mountedRef.current) { return } setFeedback({ body, status: 'copied' }) } catch { + // The guard first: a row unmounted before the refusal arrives has nothing to explain a buzz + // with, and the feedback it would set is read by a component that is gone. if (!mountedRef.current) { return } + // The row says so on its own control rather than in a toast; the buzz is the part a thumb + // resting on the button it just pressed can notice without looking. + triggerError() setFeedback({ body, status: 'failed' }) } if (copyResetTimerRef.current) { diff --git a/mobile/src/session/QuickCommandsList.test.ts b/mobile/src/session/QuickCommandsList.test.ts index 37e41990a08..aae0bf7bbe3 100644 --- a/mobile/src/session/QuickCommandsList.test.ts +++ b/mobile/src/session/QuickCommandsList.test.ts @@ -27,6 +27,14 @@ vi.mock('lucide-react-native', () => ({ vi.mock('expo-clipboard', () => ({ setStringAsync: vi.fn() })) +// The row buzzes when a copy is refused, and the real module reads `__DEV__` at import. +vi.mock('expo-haptics', () => ({ + notificationAsync: vi.fn(), + performAndroidHapticsAsync: vi.fn(), + AndroidHaptics: { Reject: 'reject' }, + NotificationFeedbackType: { Error: 'error', Success: 'success' } +})) + vi.mock('../components/MobileAgentIcon', () => ({ MobileAgentIcon: 'MobileAgentIcon' })) describe('QuickCommandsList search', () => { diff --git a/mobile/src/session/QuickCommandsList.tsx b/mobile/src/session/QuickCommandsList.tsx index 02a6809bb60..2dae27e3a06 100644 --- a/mobile/src/session/QuickCommandsList.tsx +++ b/mobile/src/session/QuickCommandsList.tsx @@ -1,6 +1,7 @@ import { View, Text, Pressable, TextInput, StyleSheet, ActivityIndicator } from 'react-native' import { Check, Plus, Search } from 'lucide-react-native' import { colors, spacing } from '../theme/mobile-theme' +import { TEXT_INPUT_FONT_SIZE } from '../platform/text-input-font-size' import { MobileAgentIcon } from '../components/MobileAgentIcon' import { MOBILE_AGENT_CATALOG } from '../tasks/mobile-agent-catalog' import type { TerminalQuickCommand } from '../../../src/shared/terminal-quick-command-types' @@ -207,7 +208,7 @@ const styles = StyleSheet.create({ paddingHorizontal: spacing.md, paddingVertical: spacing.sm }, - searchInput: { flex: 1, color: colors.textPrimary, fontSize: 14, padding: 0 }, + searchInput: { flex: 1, color: colors.textPrimary, fontSize: TEXT_INPUT_FONT_SIZE, padding: 0 }, error: { color: colors.statusRed, fontSize: 13, paddingHorizontal: spacing.xs }, loading: { paddingVertical: spacing.lg }, empty: { diff --git a/mobile/src/session/clipboard-write-failure-branch-census.test.ts b/mobile/src/session/clipboard-write-failure-branch-census.test.ts new file mode 100644 index 00000000000..633a4fff4aa --- /dev/null +++ b/mobile/src/session/clipboard-write-failure-branch-census.test.ts @@ -0,0 +1,249 @@ +import { readFileSync } from 'node:fs' +import ts from 'typescript-api' +import { describe, expect, it } from 'vitest' +import { parse, productFiles } from '../navigation/router-seam-census.test-support' + +const SESSION_ROOT = import.meta.dirname + +/** + * Every clipboard write on this screen has somewhere for its failure to go. + * + * The seam rejects when the pasteboard refused the text, which is the whole reason it exists: a + * caller that showed "Copied" over a write that did not land was the failure it replaced. But a + * rejection needs a reader. Seven of these sites are fire-and-forget — a sheet row's `onPress`, a + * `void copy(...)` in a render tree — so a site without a failure branch does not merely stay + * quiet, it raises an unhandled rejection and still leaves "Copied" on screen. + * + * A census rather than seven tests, because the eighth site is the one that will be written by + * somebody who never read this file. Structural rather than behavioural on purpose: what each site + * does about a failure is its own business, and what this holds is that it does something. + */ + +/** The seam's own name, so a local helper called `writeText` is not mistaken for it. */ +const SEAM_METHOD = 'writeText' +const CATCH_METHOD = 'catch' +const SEAM_HOOK = 'useClipboardWriter' + +/** + * Whether a call is answered for, by either shape this tree uses. + * + * `await` inside the block of a `try` that catches, or a `.catch(...)` on the promise itself. + * Walked upward from the call rather than matched on text: both shapes put the handler somewhere + * other than the line the write is on, and a regex over the file would pass a `catch` that belongs + * to a different statement entirely. + * + * The `await` is not a formality. A promise nobody waits for settles after the block that started + * it has returned, so a `void clipboard.writeText(...)` inside a `try` is the unhandled rejection + * this rule exists to stop, with a `catch` above it that can never run. `return` is not enough + * either, for the same reason: only `return await` keeps the call inside the block. + */ +function hasFailureBranch(call: ts.CallExpression): boolean { + let node: ts.Node = call + let awaited = false + while (node.parent !== undefined) { + const parent: ts.Node = node.parent + if (ts.isAwaitExpression(parent)) { + awaited = true + } + // `clipboard.writeText(x).then(...).catch(...)`: the handler is further along this same chain, + // and `parent.expression === node` is what keeps it to this chain rather than any nearby catch. + // Called, with something to call: `.catch` read as a property registers nothing, and `.catch()` + // with no argument swallows the rejection while the caller goes on to say the write landed. + if ( + ts.isPropertyAccessExpression(parent) && + parent.expression === node && + parent.name.text === CATCH_METHOD && + parent.parent !== undefined && + ts.isCallExpression(parent.parent) && + parent.parent.expression === parent && + parent.parent.arguments.length > 0 + ) { + return true + } + // `tryBlock === node` because a write inside the catch or finally clause is not answered for by + // the try it is written in: that handler has already run. + if (ts.isTryStatement(parent) && parent.catchClause !== undefined && parent.tryBlock === node) { + return awaited + } + // Every function-like node ends the search, arrows and function expressions included: a `catch` + // outside one answers for whoever called it, not for the call left running inside. + if (ts.isFunctionLike(parent)) { + return false + } + node = parent + } + return false +} + +/** Every `x.writeText(...)` in a parsed module, as `file:line`, with whether its failure is handled. */ +function clipboardWritesIn( + source: ts.SourceFile, + name: string +): { at: string; handled: boolean }[] { + const found: { at: string; handled: boolean }[] = [] + const visit = (node: ts.Node): void => { + if ( + ts.isCallExpression(node) && + ts.isPropertyAccessExpression(node.expression) && + node.expression.name.text === SEAM_METHOD + ) { + const line = source.getLineAndCharacterOfPosition(node.getStart(source)).line + 1 + found.push({ at: `${name}:${line}`, handled: hasFailureBranch(node) }) + } + ts.forEachChild(node, visit) + } + ts.forEachChild(source, visit) + return found +} + +function clipboardWrites(root: string, name: string): { at: string; handled: boolean }[] { + return clipboardWritesIn(parse(root, name), name) +} + +/** A snippet read the way the census reads a module, for shapes no product file holds today. */ +function writesInSnippet(source: string): { at: string; handled: boolean }[] { + return clipboardWritesIn( + ts.createSourceFile('snippet.ts', source, ts.ScriptTarget.Latest, true), + 'snippet.ts' + ) +} + +/** Whether the census would report the single write in a snippet. */ +function reports(source: string): boolean { + const writes = writesInSnippet(source) + if (writes.length !== 1) { + throw new Error(`the snippet holds ${writes.length} writes, not one`) + } + return !writes[0].handled +} + +describe('every clipboard write in the session domain answers for its failure', () => { + const files = productFiles(SESSION_ROOT) + const writes = files.flatMap((name) => clipboardWrites(SESSION_ROOT, name)) + + it('finds the writes it is written against, so an empty list is not a pass', () => { + // The completeness half: a rule over nothing is a rule that cannot fail. The count is a floor + // rather than an equality, because a new copy button is not this census's business to approve. + expect(writes.length).toBeGreaterThanOrEqual(8) + expect( + files.filter((name) => readFileSync(`${SESSION_ROOT}/${name}`, 'utf8').includes(SEAM_HOOK)) + .length + ).toBeGreaterThan(0) + }) + + it('leaves none of them without one', () => { + expect(writes.filter((write) => !write.handled).map((write) => write.at)).toEqual([]) + }) +}) + +/** + * What the rule counts as somewhere for a failure to go, in the shapes a product file does not hold. + * + * The census walks real modules, so the shapes it must refuse cannot be planted in one: a `void` + * write is exactly the mistake it exists to catch, and it would have to be committed to be tested. + * Snippets instead, read through the same reader, so a rule that stops matching is a red here + * rather than a site that quietly passes for years. + */ +describe('the failure branch the census will accept', () => { + it('refuses a write nobody waits for, however well the block around it is guarded', () => { + // The whole point of the rule. `void` detaches the promise from the block: the `try` has + // returned long before the rejection settles, and the process gets an unhandled rejection with + // a `catch` sitting three lines above it that never runs. + expect( + reports(` + async function copy(clipboard: Clipboard, text: string) { + try { + void clipboard.writeText(text) + } catch { + showToast("Couldn't copy") + } + } + `) + ).toBe(true) + }) + + it('refuses a write whose only catch is outside the function it sits in', () => { + // An arrow ends the search as surely as a declaration does. The `catch` here answers for + // `forEach`, which returns before the write it started has settled. + expect( + reports(` + async function copyAll(clipboard: Clipboard, rows: string[]) { + try { + rows.forEach((row) => { + clipboard.writeText(row) + }) + } catch { + showToast("Couldn't copy") + } + } + `) + ).toBe(true) + }) + + it('refuses a write in the catch block, which its own try does not answer for', () => { + expect( + reports(` + async function copy(clipboard: Clipboard, text: string) { + try { + await save(text) + } catch { + await clipboard.writeText(text) + } + } + `) + ).toBe(true) + }) + + it('refuses a `.catch` that is read rather than called', () => { + // `clipboard.writeText(text).catch` is the handler's name, not a handler. It registers nothing, + // and the rejection goes exactly where it would have gone with no `catch` written at all. + expect( + reports(` + function copy(clipboard: Clipboard, text: string) { + const retry = clipboard.writeText(text).catch + return retry + } + `) + ).toBe(true) + }) + + it('refuses a `.catch()` with nothing to handle the rejection', () => { + // Called, so the promise is handled in the sense that nothing is reported — and the user is + // told a write landed when it did not, which is the failure this rule is written against. + expect( + reports(` + function copy(clipboard: Clipboard, text: string) { + clipboard.writeText(text).catch() + showToast('Copied') + } + `) + ).toBe(true) + }) + + it('accepts an awaited write inside a try that catches', () => { + expect( + reports(` + async function copy(clipboard: Clipboard, text: string) { + try { + await clipboard.writeText(text) + } catch { + showToast("Couldn't copy") + } + } + `) + ).toBe(false) + }) + + it('accepts a write that carries its own catch along the chain', () => { + expect( + reports(` + function copy(clipboard: Clipboard, text: string) { + clipboard + .writeText(text) + .then(() => showToast('Copied')) + .catch(() => showToast("Couldn't copy")) + } + `) + ).toBe(false) + }) +}) diff --git a/mobile/src/session/mobile-native-chat-input-base-styles.ts b/mobile/src/session/mobile-native-chat-input-base-styles.ts new file mode 100644 index 00000000000..6cc2453bed6 --- /dev/null +++ b/mobile/src/session/mobile-native-chat-input-base-styles.ts @@ -0,0 +1,24 @@ +import { colors, radii, spacing } from '../theme/mobile-theme' + +/** + * The chat's two free-text fields, minus the one thing that is a platform answer. + * + * The composer spans its row and grows to 140; the question's field shares the row with a send + * button and stops at 120. Everything else about them is the same and is here, because a `.web.ts` + * cannot import a value from the file it shadows and two copies of a style object drift apart on + * everything except the difference that was meant to be between them. + */ +const chatInputSurface = { + minHeight: 40, + color: colors.textPrimary, + backgroundColor: colors.bgRaised, + borderRadius: radii.input, + paddingHorizontal: spacing.md, + paddingTop: spacing.sm, + paddingBottom: spacing.sm +} as const + +export const mobileNativeChatInputBase = { + input: { ...chatInputSurface, width: '100%', maxHeight: 140 }, + freeInput: { ...chatInputSurface, flex: 1, maxHeight: 120 } +} as const diff --git a/mobile/src/session/mobile-native-chat-input-styles.ts b/mobile/src/session/mobile-native-chat-input-styles.ts new file mode 100644 index 00000000000..2d1591db9e7 --- /dev/null +++ b/mobile/src/session/mobile-native-chat-input-styles.ts @@ -0,0 +1,17 @@ +import { StyleSheet } from 'react-native' +import { typography } from '../theme/mobile-theme' +import { mobileNativeChatInputBase } from './mobile-native-chat-input-base-styles' + +/** + * Native: both chat fields sit one point above the body size, which is what they have rendered at. + * + * The `.web.ts` sibling raises them to the text-input seam, because 15 is under the size below + * which iOS zooms the page on focus — and this screen is the one that cannot afford that zoom, the + * terminal's keyboard lift being pure geometry on a document it assumes is at scale 1. + */ +const CHAT_INPUT_FONT_SIZE = typography.bodySize + 1 + +export const mobileNativeChatInputStyles = StyleSheet.create({ + input: { ...mobileNativeChatInputBase.input, fontSize: CHAT_INPUT_FONT_SIZE }, + freeInput: { ...mobileNativeChatInputBase.freeInput, fontSize: CHAT_INPUT_FONT_SIZE } +}) diff --git a/mobile/src/session/mobile-native-chat-input-styles.web.test.ts b/mobile/src/session/mobile-native-chat-input-styles.web.test.ts new file mode 100644 index 00000000000..de905bfb7b7 --- /dev/null +++ b/mobile/src/session/mobile-native-chat-input-styles.web.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, it, vi } from 'vitest' + +// StyleSheet.create is identity in React Native and on RN Web alike, and every other export of the +// module reaches the native runtime this test does not have. +vi.mock('react-native', () => ({ + StyleSheet: { create: (styles: unknown) => styles } +})) + +// The seam as the page bundle resolves it. Without this the `.web.ts` styles below would read the +// native seam and the test would pass on a size that no browser ever renders. +vi.mock( + '../platform/text-input-font-size', + async () => await import('../platform/text-input-font-size.web') +) + +import { TEXT_INPUT_FONT_SIZE } from '../platform/text-input-font-size' +import { TEXT_INPUT_FONT_SIZE_FLOOR } from '../platform/text-input-font-size.web' +import { colors, radii, spacing, typography } from '../theme/mobile-theme' +import { mobileNativeChatInputBase } from './mobile-native-chat-input-base-styles' +import { mobileNativeChatInputStyles } from './mobile-native-chat-input-styles' +import { mobileNativeChatInputStyles as onWeb } from './mobile-native-chat-input-styles.web' + +/** Every property the two fields carried before the split, read off the commit that split them. */ +const BEFORE_THE_SPLIT = { + input: { + width: '100%', + maxHeight: 140, + minHeight: 40, + color: colors.textPrimary, + fontSize: typography.bodySize + 1, + backgroundColor: colors.bgRaised, + borderRadius: radii.input, + paddingHorizontal: spacing.md, + paddingTop: spacing.sm, + paddingBottom: spacing.sm + }, + freeInput: { + flex: 1, + minHeight: 40, + maxHeight: 120, + color: colors.textPrimary, + fontSize: typography.bodySize + 1, + backgroundColor: colors.bgRaised, + borderRadius: radii.input, + paddingHorizontal: spacing.md, + paddingTop: spacing.sm, + paddingBottom: spacing.sm + } +} as const + +const KEYS = ['input', 'freeInput'] as const + +describe('the chat composer and question fields natively', () => { + it.each(KEYS)('renders exactly what it rendered before the split: %s', (key) => { + expect(mobileNativeChatInputStyles[key]).toEqual(BEFORE_THE_SPLIT[key]) + // Key for key as well as value for value: `toEqual` would pass over an extra undefined. + expect(Object.keys(mobileNativeChatInputStyles[key]).sort()).toEqual( + Object.keys(BEFORE_THE_SPLIT[key]).sort() + ) + }) + + it('sits one point under the floor, which is why the split exists', () => { + // The premise, not a restatement: if the body size ever rose to 15 this whole pair collapses + // into an in-place move and someone should be told rather than left maintaining three files. + expect(BEFORE_THE_SPLIT.input.fontSize).toBeLessThan(TEXT_INPUT_FONT_SIZE_FLOOR) + }) +}) + +describe('the chat composer and question fields on the web', () => { + it.each(KEYS)('takes its size from the seam, clear of the focus-zoom floor: %s', (key) => { + expect(onWeb[key].fontSize).toBe(TEXT_INPUT_FONT_SIZE) + expect(onWeb[key].fontSize).toBeGreaterThanOrEqual(TEXT_INPUT_FONT_SIZE_FLOOR) + expect(onWeb[key].fontSize).toBeGreaterThan(BEFORE_THE_SPLIT[key].fontSize) + }) + + // The split is one value, not a second style: everything the siblings do not differ on comes from + // the same object, so a padding or a colour cannot drift between the platforms. + it.each(KEYS)('differs from the native style in nothing but the size: %s', (key) => { + expect(mobileNativeChatInputBase[key]).not.toHaveProperty('fontSize') + expect(mobileNativeChatInputStyles[key]).toMatchObject(mobileNativeChatInputBase[key]) + expect(onWeb[key]).toMatchObject(mobileNativeChatInputBase[key]) + expect(Object.keys(onWeb[key]).sort()).toEqual( + Object.keys(mobileNativeChatInputStyles[key]).sort() + ) + }) + + it('keeps the two fields apart where they were always apart', () => { + // A shared base is how two styles drift into one. The composer spans its row; the question's + // field shares the row with a send button, and neither shape is the other's. + expect(onWeb.input).toMatchObject({ width: '100%', maxHeight: 140 }) + expect(onWeb.freeInput).toMatchObject({ flex: 1, maxHeight: 120 }) + }) +}) diff --git a/mobile/src/session/mobile-native-chat-input-styles.web.ts b/mobile/src/session/mobile-native-chat-input-styles.web.ts new file mode 100644 index 00000000000..252a79052df --- /dev/null +++ b/mobile/src/session/mobile-native-chat-input-styles.web.ts @@ -0,0 +1,17 @@ +import { StyleSheet } from 'react-native' +import { TEXT_INPUT_FONT_SIZE } from '../platform/text-input-font-size' +import { mobileNativeChatInputBase } from './mobile-native-chat-input-base-styles' + +/** + * Web sibling: both chat fields go on the text-input seam, one point up from the 15 they carry + * natively and clear of the floor below which iOS zooms the page on focus. + * + * The zoom is not cosmetic on this screen. `keyboard-occlusion.web.ts` reads a visual viewport + * scale other than 1 as "not a keyboard" and answers 0, so one focus of the composer would leave + * the terminal's own keyboard lift at 0 for the rest of the session — and the terminal's input is + * a hidden field whose only feedback that it is focused is the lift. + */ +export const mobileNativeChatInputStyles = StyleSheet.create({ + input: { ...mobileNativeChatInputBase.input, fontSize: TEXT_INPUT_FONT_SIZE }, + freeInput: { ...mobileNativeChatInputBase.freeInput, fontSize: TEXT_INPUT_FONT_SIZE } +}) diff --git a/mobile/src/session/mobile-session-command-input-styles.ts b/mobile/src/session/mobile-session-command-input-styles.ts index b19c13a502e..31ddffca08a 100644 --- a/mobile/src/session/mobile-session-command-input-styles.ts +++ b/mobile/src/session/mobile-session-command-input-styles.ts @@ -1,6 +1,7 @@ import { StyleSheet } from 'react-native' import { colors, spacing, radii, typography } from '../theme/mobile-theme' +import { TEXT_INPUT_FONT_SIZE } from '../platform/text-input-font-size' export const mobileSessionCommandInputStyles = StyleSheet.create({ createWarningBanner: { @@ -157,7 +158,7 @@ export const mobileSessionCommandInputStyles = StyleSheet.create({ borderRadius: radii.input, paddingHorizontal: spacing.md, paddingVertical: 0, - fontSize: 14, + fontSize: TEXT_INPUT_FONT_SIZE, fontFamily: typography.monoFamily, marginRight: spacing.sm }, diff --git a/mobile/src/session/mobile-session-route-parity.test.ts b/mobile/src/session/mobile-session-route-parity.test.ts index 616582a8d7e..82ceec90a1f 100644 --- a/mobile/src/session/mobile-session-route-parity.test.ts +++ b/mobile/src/session/mobile-session-route-parity.test.ts @@ -62,10 +62,15 @@ const HOST_COMPONENT_NAMES = new Set([ 'View' ]) -const HEAD_MAIN_HOOK_SHA256 = '1b436d21f48e4d7b316178ba9eb7d8f0d3801ffd4e42b6b8987adb1cfcbac570' -const HEAD_HOOK_BINDING_SHA256 = '5b324d661574950c24c47ad9675afc40f34bf3d6dc0ea7b81a469cf708803dc8' +// Refreshed by C7.2: five clipboard hooks joined the expanded route, which is the whole of the +5 — +// a writer in the diff-note, Markdown and selection actions, a reader in the selection actions and +// the attachment probe. The screen's other four clipboard sites (the terminal's paste, the sheets, +// the quick-command row, the diff-review send) sit outside the walk from `SessionScreen` and so do +// not move this pin. The copy-path sheet also gained the failure toast the other two copies had. +const HEAD_MAIN_HOOK_SHA256 = '6d309ebdf13ecf21e4b42fb29de9db586a3c9835ead43015a5261b67bf18b8f6' +const HEAD_HOOK_BINDING_SHA256 = '9041e8a74efdacc6099933bac11fb624aff46c99648746cf5504bf320ec431c5' const HEAD_CALLBACK_IDENTITY_SHA256 = - '2a9e4825df007f6ef53b81aa5004991d6318eee7507b44d625c07e630be432eb' + 'ed45268b61372abcfeb29e9ce91822f1fb5214542b78356c7cef869824a09d37' // Pins that no callback body in the route changed unnoticed. Body text, not behaviour: the sends // and repo reads inside them now name their `RpcOperation` instead of the raw `sendRequest` port. // Refreshed in step 6 for the gesture flush, whose `terminal.send` became `terminalInputSend` and @@ -78,14 +83,14 @@ const HEAD_CALLBACK_IDENTITY_SHA256 = // byteLength }` cast: the preview reader checks the content and salvages the flag, so `readMarkdownTab` // reads `fallback.value` directly. The dictation-mode refresh is main's own body again — it forwards // whatever mode the reply carried, so an absent one leaves the mic as inert as main left it. -const HEAD_CALLBACK_BODY_SHA256 = 'ceba525103ccac47df766063d58593ba083d59785f86257d849e355669ed47ae' +const HEAD_CALLBACK_BODY_SHA256 = 'a5cad68712a53a2d5fb5514ecd391adb5bc7621d3542ac895ec65a383ac1810f' // Refreshed for the startup effect: both `worktree.activate` sends became `worktreeActivate`, and // the sleeping-agent check reads that operation's verdict instead of the reply envelope. Refreshed // again when the reporter took the reply and interpreted it itself, retiring the hand-built // refusal the timer site passed when it had no reply at all. Refreshed once more for the // last-visited-worktree effect, whose bare store write became the one writer of that key, so the // hybrid shell's page mirror sees it as it is written rather than one `init` later. -const HEAD_EFFECT_SHA256 = '224184b2559a09067001ac2bfc8779122637c5f40727ba4bcbb789264fc91b4e' +const HEAD_EFFECT_SHA256 = 'dfce9d5cb921c734bd44801283aa579ee61ab69acbbf69ac1e769de24fd829ce' const HEAD_CONTENT_HOOK_SHA256 = '9c3b612fef3f370d66873aefdbe1d701f20cb64ded31fef5cc45fde6f8189581' // Same pin for the 12 bodies that sit in nested functions rather than callbacks, moved by the same // rewrite of those send and read expressions. Count unchanged. Refreshed again in step 6 for @@ -106,11 +111,14 @@ const HEAD_TIMER_CLEANUP_SHA256 = 'c73f1d1c2cc89642f3d727d6f3b6b81860a9d6f342345 // Six method literals fewer than before step 6: `terminal.send` and `terminal.clearBuffer` went // first, then `worktree.activate` twice, `session.tabs.createTerminal` and // `terminal.setDisplayMode`. Each is now fixed at its operation's definition instead of being -// spelled at the call site. +// spelled at the call site. Two literals more across C7.2, both of them the toast a refused write +// now shows: "Couldn't copy path" when the sheets moved onto the clipboard seam, taking the count +// from 532 to 533, and "Couldn't copy" when the Markdown copy action gained the failure branch the +// other copy paths already had, taking it to 534. const HEAD_RUNTIME_STRING_SHA256 = - 'fcb1e8d5926d52055279eec6e3805bf2d51403e2b9414c30d9c034d4a87c32b9' + 'ce4c68956cec3b49aaf785e99bc2d7efd3eafeb4fdac6ce116cd854546c045f4' const HEAD_HOST_JSX_SHA256 = '390405926b1695fa3a33686f0bc192b432f5468d8576499d7cafbb4922defbb5' -const HEAD_LEAF_JSX_SHA256 = '21dba981875e173f692590bf910d60964660c5f4cbb79f3a377c7e54f6a1f016' +const HEAD_LEAF_JSX_SHA256 = 'c7e1a4b90197697f1eaa640c38da63281b4f7b84fb036ae2152f00c2f7d7cb77' const HEAD_STYLE_REFERENCE_SHA256 = '295a3501c2c6d7bea7c8bbf38b3f3534f01344cd7e1b91bb8e07c040821d596a' const HEAD_IDENTITY_FIELD_SHA256 = @@ -501,7 +509,7 @@ describe('mobile session route extraction parity', () => { const contentBindings = CONTENT_COMPONENT_NAMES.flatMap( (name) => readHookFacts(name, definitions).bindings ) - expect(main.hooks).toHaveLength(270) + expect(main.hooks).toHaveLength(275) expect(hash(main.hooks)).toBe(HEAD_MAIN_HOOK_SHA256) expect(hash(main.bindings)).toBe(HEAD_HOOK_BINDING_SHA256) expect(main.callbacks).toHaveLength(77) @@ -546,7 +554,7 @@ describe('mobile session route extraction parity', () => { it('preserves runtime strings, styles, and the expanded JSX tree', () => { const strings = readRuntimeStrings() - expect(strings).toHaveLength(532) + expect(strings).toHaveLength(534) expect(hash(strings)).toBe(HEAD_RUNTIME_STRING_SHA256) const jsx = readJsxFacts(readDefinitions()) expect(jsx.host).toHaveLength(124) diff --git a/mobile/src/session/session-router-seam-census.test.ts b/mobile/src/session/session-router-seam-census.test.ts new file mode 100644 index 00000000000..e2699252e9b --- /dev/null +++ b/mobile/src/session/session-router-seam-census.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it } from 'vitest' +import { + callsRouteHandoff, + expoRouterValueImports, + parse, + productFiles +} from '../navigation/router-seam-census.test-support' + +const SESSION_ROOT = import.meta.dirname + +/** + * Which modules here hold a router, so the census cannot pass by seeing nothing. + * + * Three, and each for a different target. The foundation hook bounces a deleted workspace back to + * its host; the file-tap handlers push a preview route from a terminal link or a chat path; the + * notification hook consumes a pane tap by rewriting this route's own params. Only the first two + * can leave the page, which is the whole reason the third is in the list anyway — a `setParams` on + * expo-router's router and a `setParams` on the handoff's are the same call, and listing it here is + * what stops someone later giving it back its own `useRouter` because "it never navigates". + */ +const ROUTER_HOLDERS = [ + 'use-mobile-file-tap-handlers.ts', + 'use-mobile-session-foundation.ts', + 'use-notification-pane-navigation.ts' +] + +/** + * The expo-router names this domain may still import, and why each one is not a router. + * + * `useFocusEffect` reads whether this screen is the focused one in the document's own stack and + * `useLocalSearchParams` reads the params of the route already mounted. Neither takes a target, so + * neither can put a screen in front of the page; both are the page's own router answering about the + * page's own route, which is exactly what it is for. + * + * A closed list rather than a ban on `useRouter`: the hazard is anything that navigates, and + * expo-router exports a module-singleton `router` that does it from a plain function. A rule written + * against the one spelling would have read that as clean. + */ +const NON_NAVIGATING_ROUTER_NAMES = ['useFocusEffect', 'useLocalSearchParams'] + +describe('the session domain reaches the router through the handoff seam', () => { + const files = productFiles(SESSION_ROOT) + + it('walks the modules it is written against', () => { + expect(files).toEqual(expect.arrayContaining(ROUTER_HOLDERS)) + expect(files.length).toBeGreaterThan(200) + }) + + it('imports nothing from expo-router that can navigate', () => { + const offenders = files + .map((name) => ({ + name, + imported: expoRouterValueImports(parse(SESSION_ROOT, name)).filter( + (imported) => !NON_NAVIGATING_ROUTER_NAMES.includes(imported) + ) + })) + .filter((entry) => entry.imported.length > 0) + .map((entry) => `${entry.name} (${entry.imported.join(', ')})`) + expect(offenders).toEqual([]) + }) + + it('takes the router from useRouteHandoff at every screen that holds one', () => { + expect(files.filter((name) => callsRouteHandoff(parse(SESSION_ROOT, name))).sort()).toEqual( + [...ROUTER_HOLDERS].sort() + ) + }) + + it('still reaches expo-router for the two names that answer about its own route', () => { + // The completeness half: the rule above also passes over a domain that imports nothing at all, + // which is what it would read as if someone moved these hooks and left the list behind. + const imported = new Set( + files.flatMap((name) => expoRouterValueImports(parse(SESSION_ROOT, name))) + ) + expect([...imported].sort()).toEqual([...NON_NAVIGATING_ROUTER_NAMES].sort()) + }) +}) diff --git a/mobile/src/session/use-mobile-file-tap-handlers.ts b/mobile/src/session/use-mobile-file-tap-handlers.ts index 9c3019f3073..ad0386aa422 100644 --- a/mobile/src/session/use-mobile-file-tap-handlers.ts +++ b/mobile/src/session/use-mobile-file-tap-handlers.ts @@ -1,5 +1,5 @@ import { useCallback, useLayoutEffect, useRef, type MutableRefObject } from 'react' -import { useRouter } from 'expo-router' +import { useRouteHandoff } from '../navigation/route-handoff' import { triggerSelection } from '../platform/haptics' import { openMobileFileTap, type FileTapSessionTab } from './mobile-file-tap-open' import { openMobileNativeChatFileTap } from './mobile-native-chat-open-file' @@ -58,7 +58,7 @@ export function useMobileFileTapHandlers( worktreeId, worktreeName } = options - const router = useRouter() + const router = useRouteHandoff() const routerRef = useRef(router) const optionsRef = useRef(options) const activationSeqRef = useRef(0) diff --git a/mobile/src/session/use-mobile-session-accessory-selection.ts b/mobile/src/session/use-mobile-session-accessory-selection.ts index fdcd7885f79..7e3cfcfa76c 100644 --- a/mobile/src/session/use-mobile-session-accessory-selection.ts +++ b/mobile/src/session/use-mobile-session-accessory-selection.ts @@ -1,6 +1,6 @@ import { useRef, useCallback } from 'react' import { Keyboard, Platform, type View } from 'react-native' -import * as Clipboard from 'expo-clipboard' +import { useClipboardReader, useClipboardWriter } from '../platform/clipboard' import { newTabRepoListRead, type MobileRuntimeRepoSummary } from './mobile-session-read-operations' import { triggerSelection, @@ -43,6 +43,8 @@ export function useMobileSessionAccessorySelection(scope: MobileSessionTerminalI handleAccessoryKey, clearSessionTabActionSheetKeyboardListener } = scope + const clipboard = useClipboardWriter() + const clipboardContents = useClipboardReader().contents const trimsGutterRef = useTerminalCopyTrimsGutter(client, connState) // Why: hold-to-repeat matches iOS cadence (400ms then 45ms); non-repeatable keys fire once (holding is destructive). const repeatTimeoutRef = useRef | null>(null) @@ -118,7 +120,7 @@ export function useMobileSessionAccessorySelection(scope: MobileSessionTerminalI return } try { - await Clipboard.setStringAsync( + await clipboard.writeText( trimsGutterRef.current ? stripTerminalSelectionGutter(text) : text ) triggerSuccess() @@ -138,7 +140,7 @@ export function useMobileSessionAccessorySelection(scope: MobileSessionTerminalI showToast("Couldn't copy", 1500) } }, - [showToast] + [clipboard, showToast] ) const handleSelectionEvicted = useCallback( @@ -203,13 +205,10 @@ export function useMobileSessionAccessorySelection(scope: MobileSessionTerminalI }, [client, isFloatingWorkspaceRoute, worktreeId]) const refreshCanPaste = useCallback(() => { - void Promise.all([ - Clipboard.hasStringAsync().catch(() => false), - Clipboard.hasImageAsync().catch(() => false) - ]).then(([hasString, hasImage]) => { - setCanPaste(hasString || hasImage) + void clipboardContents().then(({ text, image }) => { + setCanPaste(text || image) }) - }, []) + }, [clipboardContents, setCanPaste]) return { repeatTimeoutRef, repeatIntervalRef, diff --git a/mobile/src/session/use-mobile-session-attachments.ts b/mobile/src/session/use-mobile-session-attachments.ts index 21352916f65..86d351c1c5d 100644 --- a/mobile/src/session/use-mobile-session-attachments.ts +++ b/mobile/src/session/use-mobile-session-attachments.ts @@ -1,6 +1,6 @@ import { useEffect } from 'react' import { AppState, type AppStateStatus } from 'react-native' -import * as Clipboard from 'expo-clipboard' +import { useClipboardReader } from '../platform/clipboard' import { triggerSelection, triggerError } from '../platform/haptics' import { loadMobileNewTabAgentOptions } from './mobile-new-tab-agent-loader' import { useMobileSessionImageAttachments } from './use-mobile-session-image-attachments' @@ -39,6 +39,7 @@ export function useMobileSessionAttachments(scope: MobileSessionAccessorySelecti refreshCanPaste, activeSessionTab } = scope + const clipboardContents = useClipboardReader().contents const agent = activeSessionTab && 'agentStatus' in activeSessionTab ? (activeSessionTab.agentStatus?.agentType ?? nativeChatController.nativeChatAgent) @@ -99,12 +100,9 @@ export function useMobileSessionAttachments(scope: MobileSessionAccessorySelecti useEffect(() => { let mounted = true const refresh = () => { - void Promise.all([ - Clipboard.hasStringAsync().catch(() => false), - Clipboard.hasImageAsync().catch(() => false) - ]).then(([hasString, hasImage]) => { + void clipboardContents().then(({ text, image }) => { if (mounted) { - setCanPaste(hasString || hasImage) + setCanPaste(text || image) } }) } @@ -120,7 +118,7 @@ export function useMobileSessionAttachments(scope: MobileSessionAccessorySelecti mounted = false sub.remove() } - }, [selectModeActive]) + }, [clipboardContents, selectModeActive, setCanPaste]) useEffect(() => { const shouldLoadAgentOptions = showCreateTabDrawer || pendingDiffNotesDelivery !== null diff --git a/mobile/src/session/use-mobile-session-diff-comments.ts b/mobile/src/session/use-mobile-session-diff-comments.ts index a7955767fa0..07691cb2177 100644 --- a/mobile/src/session/use-mobile-session-diff-comments.ts +++ b/mobile/src/session/use-mobile-session-diff-comments.ts @@ -1,5 +1,5 @@ import { useEffect, useCallback } from 'react' -import * as Clipboard from 'expo-clipboard' +import { useClipboardWriter } from '../platform/clipboard' import { interpretOrThrowRefusalMessage } from '../transport/rpc-refusal-message' import { sessionWorktreeRecordRead } from './mobile-session-read-operations' import { sessionWorktreeNotesWrite } from './mobile-session-write-operations' @@ -27,6 +27,7 @@ export function useMobileSessionDiffComments(scope: MobileSessionDocumentReaders setPendingDiffNotesDelivery, showToast } = scope + const clipboard = useClipboardWriter() const loadDiffComments = useCallback(async (): Promise => { if (!client || connState !== 'connected' || !worktreeId || isFloatingWorkspaceRoute) { setDiffComments([]) @@ -131,14 +132,14 @@ export function useMobileSessionDiffComments(scope: MobileSessionDocumentReaders return } try { - await Clipboard.setStringAsync(formatDiffComments(comments)) + await clipboard.writeText(formatDiffComments(comments)) triggerSuccess() showToast('Notes copied') } catch { triggerError() showToast("Couldn't copy notes", 1600) } - }, [showToast]) + }, [clipboard, showToast]) const sendDiffCommentsToAgent = useCallback((): void => { const comments = diffCommentsRef.current.filter((comment) => !comment.sentAt) diff --git a/mobile/src/session/use-mobile-session-file-actions.ts b/mobile/src/session/use-mobile-session-file-actions.ts index 56aa2b049d8..f34d8721e57 100644 --- a/mobile/src/session/use-mobile-session-file-actions.ts +++ b/mobile/src/session/use-mobile-session-file-actions.ts @@ -1,5 +1,5 @@ import { useRef, useCallback } from 'react' -import { Linking } from 'react-native' +import { openExternalLink } from '../platform/external-link' import { useMobileFileTapHandlers } from './use-mobile-file-tap-handlers' import { resolveMobileNativeChatFileSessionId } from './mobile-native-chat-eligibility' import { activateOpenedSourceControlDiffTab } from './opened-mobile-session-tab' @@ -92,7 +92,7 @@ export function useMobileSessionFileActions(scope: MobileSessionTerminalSendActi // Why: browser.tabCreate resolves a real worktree, which the floating // sentinel doesn't have — open taps in the phone browser instead. if (terminalLinkOpenMode === 'phone-browser' || isFloatingWorkspaceRoute) { - void Linking.openURL(url).catch(() => {}) + openExternalLink(url) return } void handleCreateBrowserRef.current?.(url) diff --git a/mobile/src/session/use-mobile-session-foundation.ts b/mobile/src/session/use-mobile-session-foundation.ts index 80a67a35702..0ca3d3b93a2 100644 --- a/mobile/src/session/use-mobile-session-foundation.ts +++ b/mobile/src/session/use-mobile-session-foundation.ts @@ -1,6 +1,7 @@ import { useState, useEffect } from 'react' import { useSafeAreaInsets } from 'react-native-safe-area-context' -import { useLocalSearchParams, useRouter } from 'expo-router' +import { useLocalSearchParams } from 'expo-router' +import { useRouteHandoff } from '../navigation/route-handoff' import { HOST_DOCK_MIN_WIDTH } from '../storage/preferences' import { useHostClient, useForceReconnect } from '../transport/client-context' import { @@ -33,7 +34,7 @@ export function useMobileSessionFoundation() { const isFolderWorkspaceRoute = worktreeId.startsWith('folder:') // Synthetic ids have no repo scope. // Why: the floating sentinel has no repo/worktree, so repo-backed surfaces hide. const isFloatingWorkspaceRoute = isFloatingWorkspaceWorktreeId(worktreeId) - const router = useRouter() + const router = useRouteHandoff() const insets = useSafeAreaInsets() // Why: shared client per host owned by RpcClientProvider (docs/mobile-shared-client-per-host.md). const { client, clientId, state: connState } = useHostClient(hostId) diff --git a/mobile/src/session/use-mobile-session-markdown-actions.ts b/mobile/src/session/use-mobile-session-markdown-actions.ts index b250f05f9f4..84c5089a389 100644 --- a/mobile/src/session/use-mobile-session-markdown-actions.ts +++ b/mobile/src/session/use-mobile-session-markdown-actions.ts @@ -1,6 +1,6 @@ import { useEffect, useCallback } from 'react' import { BackHandler, Keyboard } from 'react-native' -import * as Clipboard from 'expo-clipboard' +import { useClipboardWriter } from '../platform/clipboard' import { markdownTabSave } from './mobile-session-write-operations' import { triggerSuccess, triggerError } from '../platform/haptics' import type { DirtyMarkdownDraft, MobileSessionTab } from './mobile-session-route-types' @@ -23,6 +23,7 @@ export function useMobileSessionMarkdownActions(scope: MobileSessionDiffComments showToast, readMarkdownTab } = scope + const clipboard = useClipboardWriter() const updateMarkdownLocalContent = useCallback((tabId: string, content: string) => { setMarkdownDocs((prev) => { const current = prev.get(tabId) @@ -46,11 +47,20 @@ export function useMobileSessionMarkdownActions(scope: MobileSessionDiffComments if (current?.status !== 'ready') { return } - await Clipboard.setStringAsync(current.localContent) + // Caught here because the only caller is `void copyMarkdownLocalContent(...)`: the seam + // rejects when the pasteboard refused the text, and an uncaught rejection would leave + // "Copied" as the last word on a copy that did not happen. + try { + await clipboard.writeText(current.localContent) + } catch { + triggerError() + showToast("Couldn't copy", 1500) + return + } triggerSuccess() showToast('Copied') }, - [markdownDocs, showToast] + [clipboard, markdownDocs, showToast] ) const getDirtyMarkdownDrafts = useCallback(() => { diff --git a/mobile/src/session/use-mobile-terminal-paste.test.ts b/mobile/src/session/use-mobile-terminal-paste.test.ts index 5a59c4be792..0749e4fb4d3 100644 --- a/mobile/src/session/use-mobile-terminal-paste.test.ts +++ b/mobile/src/session/use-mobile-terminal-paste.test.ts @@ -2,7 +2,13 @@ import { describe, expect, it, vi } from 'vitest' import type { RpcClient } from '../transport/rpc-client' import { useMobileTerminalPaste } from './use-mobile-terminal-paste' -vi.mock('react', () => ({ useCallback: (callback: unknown) => callback })) +// `useMemo` joins `useCallback` because the paste now reads the clipboard through the platform +// seam, which is a hook. Called rather than cached: this test mounts nothing, so there is no +// render to hold a value across. +vi.mock('react', () => ({ + useCallback: (callback: unknown) => callback, + useMemo: (factory: () => unknown) => factory() +})) vi.mock('expo-clipboard', () => ({ getStringAsync: async () => '', getImageAsync: async () => ({ data: 'png' }) diff --git a/mobile/src/session/use-mobile-terminal-paste.ts b/mobile/src/session/use-mobile-terminal-paste.ts index 46edd9076f8..b6260630a32 100644 --- a/mobile/src/session/use-mobile-terminal-paste.ts +++ b/mobile/src/session/use-mobile-terminal-paste.ts @@ -2,7 +2,7 @@ import { separateImagePasteFromFollowingText } from '../../../src/shared/image-p import { reportWorkerTerminalUserInput } from '../terminal/worker-terminal-takeover-report' import { useCallback, type RefObject } from 'react' import { terminalInputSend } from '../terminal/mobile-terminal-operations' -import * as Clipboard from 'expo-clipboard' +import { useClipboardReader } from '../platform/clipboard' import { File as FsFile, Paths } from 'expo-file-system' import { ImageManipulator, SaveFormat } from 'expo-image-manipulator' import type { TerminalModes } from '../terminal/terminal-webview-contract' @@ -111,13 +111,14 @@ export function useMobileTerminalPaste({ refreshCanPaste, showToast }: UseMobileTerminalPasteOptions): () => Promise { + const clipboard = useClipboardReader() return useCallback(async () => { if (!client || !activeHandle || !canSend) { return } const targetHandle = activeHandle try { - const text = await Clipboard.getStringAsync() + const text = await clipboard.readText() let payload: string | null = null if (text.length > 0) { payload = buildMobileTerminalClipboardTextPayload( @@ -125,7 +126,7 @@ export function useMobileTerminalPaste({ ptyModesRef.current.get(targetHandle) ) } else { - const image = await Clipboard.getImageAsync({ format: 'png' }) + const image = await clipboard.readImage() if (!image) { refreshCanPaste() return @@ -193,6 +194,7 @@ export function useMobileTerminalPaste({ }, [ activeHandle, agent, + clipboard, activeHandleRef, activeSessionTabTypeRef, canSend, diff --git a/mobile/src/session/use-notification-pane-navigation.ts b/mobile/src/session/use-notification-pane-navigation.ts index f6d3bb15cd4..55af7089c2f 100644 --- a/mobile/src/session/use-notification-pane-navigation.ts +++ b/mobile/src/session/use-notification-pane-navigation.ts @@ -1,5 +1,6 @@ import { useEffect } from 'react' -import { useLocalSearchParams, useRouter } from 'expo-router' +import { useLocalSearchParams } from 'expo-router' +import { useRouteHandoff } from '../navigation/route-handoff' import { parsePaneKey } from '../../../src/shared/stable-pane-id' import type { MobileSessionTab } from './mobile-session-route-types' @@ -25,7 +26,7 @@ export function useNotificationPaneNavigation({ switchSessionTab: (tab: MobileSessionTab) => void }) { const { paneKey } = useLocalSearchParams<{ paneKey?: string }>() - const router = useRouter() + const router = useRouteHandoff() useEffect(() => { if (!terminalsLoaded || typeof paneKey !== 'string' || !paneKey) { return diff --git a/mobile/web-entry/web-overrides.json b/mobile/web-entry/web-overrides.json index cda64e312bf..d48b1914ef1 100644 --- a/mobile/web-entry/web-overrides.json +++ b/mobile/web-entry/web-overrides.json @@ -85,6 +85,10 @@ "file": "src/browser/browser-screencast-request.web.ts", "reason": "Not an RN Web API gap but a transport one that exists only in the page: a screencast frame crosses the bridge as one message under BRIDGE_MAX_MESSAGE_BYTES, and a phone's mobile view at the native device scale factor produces a worst-case JPEG larger than that. This file budgets the mobile view's area against the cap, the envelope it measures rather than names, and one worst-case bytes-per-pixel constant. Web view mode is untouched, because a letterboxed desktop viewport is not an area the page can predict." }, + { + "file": "src/session/mobile-native-chat-input-styles.web.ts", + "reason": "The chat's composer and its question field render one point above the app's body size, which is 15 and under the floor below which iOS zooms the page on focus. keyboard-occlusion.web.ts reads a scale other than 1 as 'no keyboard' and answers 0, and on this screen that lift is the terminal's only feedback that its hidden input has focus, so one focus of the composer would cost the rest of the session. This file puts both fields on TEXT_INPUT_FONT_SIZE; the native sibling keeps 15." + }, { "file": "src/browser/browser-address-field-styles.web.ts", "reason": "The address bar renders at the theme's 12px meta size, and in a browser an input under 16px makes iOS zoom the page on focus and never zoom back. keyboard-occlusion.web.ts reads that scale as 'no keyboard' and answers 0, so one focus would stop the pane lifting for the rest of the typing session. This file puts the input and its overlaid label on TEXT_INPUT_FONT_SIZE with a line box to match; the native sibling keeps 12px, which is what a phone has always rendered and where no page can zoom."