From 60ecda47aff041c1aaf3a1acb5df1ff8955f8e8a Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sun, 20 Sep 2026 08:54:45 -0400 Subject: [PATCH] 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 --- mobile/src/platform/clipboard.web.test.tsx | 9 +- mobile/src/platform/clipboard.web.ts | 14 +- ...pboard-write-failure-branch-census.test.ts | 162 ++++++++++++++++-- .../mobile-session-route-parity.test.ts | 6 +- 4 files changed, 160 insertions(+), 31 deletions(-) diff --git a/mobile/src/platform/clipboard.web.test.tsx b/mobile/src/platform/clipboard.web.test.tsx index 67fa20e42d9..01edf0d66f1 100644 --- a/mobile/src/platform/clipboard.web.test.tsx +++ b/mobile/src/platform/clipboard.web.test.tsx @@ -129,10 +129,11 @@ describe('reading the clipboard from inside the shell', () => { /** * The degradation, recorded rather than implied. * - * No shell serves an image yet — `native.clipboard.read` refuses `{ mime: 'image' }` by name and - * a 24 MiB base64 image cannot cross an 8 MiB reply cap — so the page answers what an empty - * clipboard answers and the terminal's paste takes the branch it already had. On the page an - * image on the clipboard pastes nothing until the media verbs land. + * 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() diff --git a/mobile/src/platform/clipboard.web.ts b/mobile/src/platform/clipboard.web.ts index 7b2f20fb192..52839c6ff51 100644 --- a/mobile/src/platform/clipboard.web.ts +++ b/mobile/src/platform/clipboard.web.ts @@ -31,12 +31,14 @@ export function useClipboardWriter(): ClipboardWriter { /** * Web sibling: the shell reads text for the page, and no shell reads an image for it yet. * - * `native.clipboard.read` serves text and refuses `{ mime: 'image' }` by name. Widening it cannot - * work — `CLIPBOARD_IMAGE_MAX_BASE64_CHARS` is 24 MiB against a reply cap of 8 MiB — so an image - * is the media verbs' job and is not served here. `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 the media verbs land. + * `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 diff --git a/mobile/src/session/clipboard-write-failure-branch-census.test.ts b/mobile/src/session/clipboard-write-failure-branch-census.test.ts index f151c8ae761..cfe62b04453 100644 --- a/mobile/src/session/clipboard-write-failure-branch-census.test.ts +++ b/mobile/src/session/clipboard-write-failure-branch-census.test.ts @@ -21,42 +21,59 @@ const SESSION_ROOT = import.meta.dirname /** 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 a `try` with a `catch`, 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. + * `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.Node): boolean { - let node: ts.Node | undefined = call - while (node !== undefined) { - if (ts.isTryStatement(node) && node.catchClause !== undefined) { - return true +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 catch is further out in the same chain. + // `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. if ( - ts.isCallExpression(node) && - ts.isPropertyAccessExpression(node.expression) && - node.expression.name.text === 'catch' + ts.isPropertyAccessExpression(parent) && + parent.expression === node && + parent.name.text === CATCH_METHOD ) { return true } - // A function boundary ends the search: a `catch` outside it belongs to a different call. - if (ts.isFunctionDeclaration(node) || ts.isMethodDeclaration(node)) { + // `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 = node.parent + node = parent } return false } -/** Every `x.writeText(...)` in a module, as `file:line`, with whether its failure is handled. */ -function clipboardWrites(root: string, name: string): { at: string; handled: boolean }[] { - const source = parse(root, name) +/** 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 ( @@ -73,6 +90,27 @@ function clipboardWrites(root: string, name: string): { at: string; handled: boo 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)) @@ -91,3 +129,89 @@ describe('every clipboard write in the session domain answers for its failure', 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('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-session-route-parity.test.ts b/mobile/src/session/mobile-session-route-parity.test.ts index 6afd381218a..82ceec90a1f 100644 --- a/mobile/src/session/mobile-session-route-parity.test.ts +++ b/mobile/src/session/mobile-session-route-parity.test.ts @@ -111,8 +111,10 @@ 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. One literal more in C7.2: the Markdown copy action's "Couldn't copy", -// the toast the other copy paths already showed when a write was refused. +// 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 = 'ce4c68956cec3b49aaf785e99bc2d7efd3eafeb4fdac6ce116cd854546c045f4' const HEAD_HOST_JSX_SHA256 = '390405926b1695fa3a33686f0bc192b432f5468d8576499d7cafbb4922defbb5'