diff --git a/config/scripts/build-mobile-web-app-bundle.mjs b/config/scripts/build-mobile-web-app-bundle.mjs index b7f7072804b..21215524deb 100644 --- a/config/scripts/build-mobile-web-app-bundle.mjs +++ b/config/scripts/build-mobile-web-app-bundle.mjs @@ -560,6 +560,10 @@ export async function buildMobileWebAppBundle({ const html = '\n\n\n\n' + '\n' + + // Undeclared, a browser asks the origin for /favicon.ico itself and the shell's asset server + // answers 403, the path being in no manifest. Empty rather than an asset: a WebView document + // has no tab for an icon, and the bundle's images are route assets named by their own bytes. + '\n' + `Orca\n${MOBILE_WEB_APP_ROOT_RESET}\n\n\n
\n` + `\n\n\n` const indexBytes = Buffer.from(html, 'utf8') diff --git a/config/scripts/build-mobile-web-app-bundle.test.mjs b/config/scripts/build-mobile-web-app-bundle.test.mjs index 17e8f9ca006..1e43486221c 100644 --- a/config/scripts/build-mobile-web-app-bundle.test.mjs +++ b/config/scripts/build-mobile-web-app-bundle.test.mjs @@ -368,6 +368,20 @@ describeBundling('the app bundle', () => { }) }, 120_000) + it('declares an icon, so no browser asks the shell for one', async () => { + await withScratch(async (scratch) => { + const outDir = join(scratch, 'icon') + const { manifest } = await buildMobileWebAppBundle({ outDir }) + const html = await readFile(join(outDir, 'index.html'), 'utf8') + // Undeclared, a browser asks the origin for /favicon.ico on its own, and the shell's asset + // server answers 403 because the path is in no manifest — repeatedly, on the emulator run. + expect(html).toContain('') + // And the empty URI rather than an asset: the bundle carries no icon, so a declaration + // naming one would point at a route image whose name changes with its bytes. + expect(manifest.assets.map((asset) => asset.path)).not.toContain('favicon.ico') + }) + }, 120_000) + it('carries the root reset, so the mounted tree has a height to be 1 of', async () => { await withScratch(async (scratch) => { const outDir = join(scratch, 'root-reset') diff --git a/config/scripts/mobile-web-app-expo-notifications-closure.test.mjs b/config/scripts/mobile-web-app-expo-notifications-closure.test.mjs new file mode 100644 index 00000000000..7e9110e0605 --- /dev/null +++ b/config/scripts/mobile-web-app-expo-notifications-closure.test.mjs @@ -0,0 +1,56 @@ +import { join } from 'node:path' +import { fileURLToPath } from 'node:url' +import * as esbuild from 'esbuild' +import { describe, expect, it } from 'vitest' +import { mobileWebAppBuildOptions } from './build-mobile-web-app-bundle.mjs' +import { mobileWebAppDependenciesPresent } from './mobile-web-app-bundle-dependencies.mjs' +import { collectMobileWebAppRoutes } from './mobile-web-app-route-manifest.mjs' + +/** + * `expo-notifications`, and why no page route may import it. + * + * It is not a module a browser can merely import. `DevicePushTokenAutoRegistration.fx` runs at + * import: it adds a push-token listener, which React Native Web answers with a warning and an inert + * subscription, and it reads the persisted server registration out of `window.localStorage`. That + * read is guarded by `typeof localStorage === 'undefined'`, and the Android shell's WebView has DOM + * storage off, where `window.localStorage` is `null` rather than undefined — so the guard passes + * and the read raises "Cannot read properties of null (reading 'getItem')". The emulator run saw + * both lines on every page load, the second at error level, from a subsystem the page cannot use: + * push registration needs a device token the shell owns and a gateway the page has no client for. + * + * Two modules imported it — `push-token.ts` and `desktop-notification-channel.ts`, both reached + * through `push-registration.ts`, which `app/h/_layout.tsx` pulls in via the host screen's remove + * action. Both now have `.web` siblings. This is the fence, because nothing else stops a third + * importer: every call in those two files was already inert on web, so a page that imports one + * behaves correctly and still loads the package. + */ + +const projectDir = fileURLToPath(new URL('../..', import.meta.url)) +const appDir = join(projectDir, 'mobile', 'app') + +const describeClosure = mobileWebAppDependenciesPresent() ? describe : describe.skip + +describeClosure( + 'expo-notifications against the page', + () => { + it('is in no module the shipped bundle contains', async () => { + // The bundle the shell serves, not a closure read per route: the entry's manifest is what + // reaches every route, deferred chunks included, so this is the whole of what a document + // can load. Read per route, a module would only have to move one route over to hide. + const routes = await collectMobileWebAppRoutes(appDir) + expect(routes.length).toBeGreaterThan(5) + const { metafile } = await esbuild.build({ + ...mobileWebAppBuildOptions(routes), + metafile: true, + write: false + }) + const modules = Object.keys(metafile.inputs) + expect(modules.filter((input) => input.includes('expo-notifications'))).toEqual([]) + // The precondition: a walk that resolved nothing would also contain nothing. The two modules + // that imported it are still here, as their siblings. + expect(modules).toContain('src/notifications/push-token.web.ts') + expect(modules).toContain('src/notifications/desktop-notification-channel.web.ts') + }, 300_000) + }, + 600_000 +) diff --git a/config/scripts/mobile-web-app-render-harness.mjs b/config/scripts/mobile-web-app-render-harness.mjs index 2b8536ce5f5..f056223bec1 100644 --- a/config/scripts/mobile-web-app-render-harness.mjs +++ b/config/scripts/mobile-web-app-render-harness.mjs @@ -453,8 +453,13 @@ export async function createBundleServer({ transformChunk, handleRequest }) { + const requestedPaths = [] const server = createServer((request, response) => { const path = new URL(request.url, 'http://localhost').pathname + // Every path this origin was asked for, the browser's own fetches included. A favicon request + // is made by the browser process rather than the page, and Playwright's `page.on('request')` + // never reports one, so the server is the only place a check can see it. + requestedPaths.push(path) // An endpoint of the check's own, answered before anything is looked for on disk: a policy's // `report-uri` has to name a real server, and naming this one keeps it on the page's origin. if (handleRequest?.(request, response, path)) { @@ -462,7 +467,10 @@ export async function createBundleServer({ } // A browser asks for this on its own and the shell's WebView never does. The bundle carries // no icon, so a 404 would put a console error in every check that runs against a full Chrome - // -- which is what CI resolves -- and none against the bundled headless shell. + // -- which is what CI resolves -- and none against the bundled headless shell. Kept for the + // probe documents the checks compose themselves, which declare no icon; the page's own + // document does declare one, and answering 204 hides nothing from a check that reads the + // request rather than the response (`mobile-web-app-session-render.test.mjs`). if (path === '/favicon.ico') { response.writeHead(204) response.end() @@ -502,7 +510,7 @@ export async function createBundleServer({ ) }) await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) - return { server, origin: `http://127.0.0.1:${String(server.address().port)}` } + return { server, origin: `http://127.0.0.1:${String(server.address().port)}`, requestedPaths } } /** diff --git a/config/scripts/mobile-web-app-session-render.test.mjs b/config/scripts/mobile-web-app-session-render.test.mjs index 014797eaaf7..57ba6457869 100644 --- a/config/scripts/mobile-web-app-session-render.test.mjs +++ b/config/scripts/mobile-web-app-session-render.test.mjs @@ -89,6 +89,7 @@ let server let browser let origin let routeChunks = {} +let servedPaths = [] let cspHeader = null let bridgeVersion = null let faultGrant = null @@ -106,6 +107,7 @@ beforeAll(async () => { const served = await createBundleServer({ outDir: built.outDir, cspHeader }) server = served.server origin = served.origin + servedPaths = served.requestedPaths const executablePath = process.env.ORCA_MOBILE_WEB_RENDER_BROWSER browser = await chromium.launch({ headless: true, ...(executablePath ? { executablePath } : {}) }) }, 240_000) @@ -119,8 +121,16 @@ afterAll(async () => { }) /** A page carrying every signal these cases read: uncaught errors, console errors, request paths. */ -async function openPage(route) { +async function openPage(route, replies = {}, { domStorageOff = false } = {}) { const page = await browser.newPage({ viewport: { width: 390, height: 844 } }) + if (domStorageOff) { + // What the Android shell serves: DOM storage is off on its WebView, and a WebView with it off + // answers `window.localStorage` with `null` rather than leaving it undefined. Read off the + // device rather than assumed — the emulator run's own error names `null` (reading 'getItem'). + await page.addInitScript(() => { + Object.defineProperty(window, 'localStorage', { configurable: true, get: () => null }) + }) + } // At document start, where the native shell installs the real channel: the entry reads it while // its own script runs, so a channel added after `load` would already be too late. await page.addInitScript(installShellDouble, { @@ -133,9 +143,10 @@ async function openPage(route) { faultGrant, grants: [faultGrant, ...sessionGrants()], pageRoutes: PAGE_ROUTE_PATTERNS, - replies: {} + replies }) const errors = [] + const warnings = [] const scripts = [] const requestedHosts = [] page.on('pageerror', (error) => errors.push(`${error.name}: ${error.message}`)) @@ -143,6 +154,11 @@ async function openPage(route) { if (message.type() === 'error') { errors.push(`console.error: ${message.text()}`) } + // Kept apart from `errors`: the bridge reports a refused storage write at warning level, so a + // page writing a key it was never handed is invisible to every assertion above. + if (message.type() === 'warning') { + warnings.push(message.text()) + } }) // Every request, not only the ones that answered: a CSP refusal fails the request, and a check // reading responses alone would read a blocked fetch as one that never happened. @@ -153,7 +169,7 @@ async function openPage(route) { scripts.push(path) } }) - return { page, errors, scripts, requestedHosts } + return { page, errors, warnings, scripts, requestedHosts } } /** @@ -186,8 +202,8 @@ async function waitForRoute({ page, errors }, route, awaitText) { } } -async function openRoute(route, awaitText) { - const opened = await openPage(route) +async function openRoute(route, awaitText, replies = {}, options = {}) { + const opened = await openPage(route, replies, options) await opened.page.goto(`${origin}/`, { waitUntil: 'load' }) await waitForRoute(opened, route, awaitText) return opened @@ -265,6 +281,62 @@ describeRender( await opened.page.close() }, 120_000) + it('asks the origin for no icon, which the shell has none to answer with', async () => { + // The document declares ``. Without it a browser asks the + // origin for /favicon.ico on its own, and the shell's asset server answers 403 because the + // path is in no manifest — which the emulator run saw, repeatedly. + // + // Only a full Chrome asks; the bundled headless shell never does, so against the default + // browser this case is a precondition rather than a measurement. + // `ORCA_MOBILE_WEB_RENDER_BROWSER` is what CI resolves, and that is where this bites. + // Read off the server's own log, not the page's: a favicon fetch is made by the browser + // process rather than the page, and Playwright's `page.on('request')` never reports one. + // The whole file's log, because no case here may produce this request. + const opened = await openRoute(SESSION_ROUTE, 'Terminal') + // Settled rather than read at the paint: a browser asks for the icon after `load`, later + // than the text the route waited on, and reading there passes on a request still to come. + await opened.page.waitForLoadState('networkidle') + expect(servedPaths.filter((path) => path === '/favicon.ico')).toEqual([]) + // The precondition, so a run that recorded no request at all cannot pass this. + expect(servedPaths).toContain('/') + await opened.page.close() + }, 120_000) + + it('paints with DOM storage off, which is how the Android shell serves it', async () => { + // Every other case here runs against a real `localStorage`, which the page never has. The + // one module that needed it was `expo-notifications`: `push-registration.ts` reached it and + // its `DevicePushTokenAutoRegistration.fx` reads the persisted registration at import behind + // a `typeof localStorage === 'undefined'` guard, which `null` walks straight through. That + // put "Cannot read properties of null (reading 'getItem')" at error level on every page load + // on the device. The page has no push registration; the shell owns it. + const opened = await openRoute(SESSION_ROUTE, 'Terminal', {}, { domStorageOff: true }) + // Exact and not a filter, like the case above it: a module reaching browser storage the page + // does not have is a defect wherever it comes from. + expect(opened.errors).toEqual([]) + await opened.page.close() + }, 120_000) + + it('writes no storage key it was never handed, on a mount that read the host status', async () => { + // `status.get` is what arms it: `host-status-gates.ts` runs on every mount above the route, + // and on a readable status the native `host-app-version-store.ts` writes + // `orca:host-app-version:v1:` — a key no page route reads and `page-storage-keys.ts` + // does not admit, so the bridge refused it and logged one `storage-write-dropped` per mount + // on the device. Answered here because the other cases' double answers no RPC at all, which + // is exactly why this went unseen: the write needs a reply, not a control. + const opened = await openRoute(SESSION_ROUTE, 'Terminal', { + 'status.get': { + protocolVersion: 9, + minCompatibleMobileVersion: 1, + appVersion: '1.4.191', + capabilities: [] + } + }) + // The whole refusal and not this one key: any page-closure writer of an unlisted key lands + // on the same line, and naming the key here would let the next one through. + expect(opened.warnings.filter((text) => text.includes('storage-write-dropped'))).toEqual([]) + await opened.page.close() + }, 120_000) + it('asks the desktop for the session it was opened on, so the page above is live', async () => { // The precondition every assertion above needs: a screen that mounted and asked for nothing // would paint the same chrome. The three reads are the header's live title, the tab snapshot @@ -303,7 +375,8 @@ describeRender( * what this route owes it is the `screencastBinary` grant, which * `mobile-web-app-screencast-lane-grant.test.mjs` derives from this closure. * - * **The storage refusals.** A page write needs a control to make it. The refusal's own chain is + * **The storage refusals a control makes.** The case above covers the writes a mount makes on its + * own; a refusal a user's own write earns still needs the control. That chain is * `mobile/src/session/mobile-structured-send-page-storage-refusal.test.ts` end to end over the * real `page-async-storage`. */ diff --git a/config/scripts/mobile-web-app-session-terminal-closure.test.mjs b/config/scripts/mobile-web-app-session-terminal-closure.test.mjs index 2dd1095869e..b7b8ef4b70d 100644 --- a/config/scripts/mobile-web-app-session-terminal-closure.test.mjs +++ b/config/scripts/mobile-web-app-session-terminal-closure.test.mjs @@ -347,8 +347,15 @@ const MERMAID_PACKAGE = 'node_modules/mermaid/' * * modules 4270 -> 4271 (+1) * local modules 1022 -> 1023 (+1) + * + * Cutting `expo-notifications` out of the page takes 62 vendored modules with it: 55 of its own, + * and behind it expo-application 3, abort-controller 2, badgin 1, event-target-shim 1. The three + * `.web` siblings replace their native files, so the local +1 is `host-app-version.ts` alone. + * + * modules 4271 -> 4210 (-61) + * local modules 1023 -> 1024 (+1) */ -const SESSION_ROUTE_MODULES = 4271 +const SESSION_ROUTE_MODULES = 4210 /** What the page enters this route through once the route is a switch with a `.web.tsx` sibling. */ const ROUTE_ENTRY = [ diff --git a/mobile/src/diagnostics/connection-diagnostics-report.ts b/mobile/src/diagnostics/connection-diagnostics-report.ts index c49eb0b01f7..41c22014471 100644 --- a/mobile/src/diagnostics/connection-diagnostics-report.ts +++ b/mobile/src/diagnostics/connection-diagnostics-report.ts @@ -5,7 +5,7 @@ import type { ConnectionState, MobileConnectionDiagnosticPath } from '../transport/types' -import { normalizeHostAppVersion } from '../transport/host-app-version-store' +import { normalizeHostAppVersion } from '../transport/host-app-version' import { formatEndpoint } from './host-reachability' import { diagnoseConnection } from './connection-diagnostics-analysis' import { redactConnectionLogEntry, redactConnectionLogText } from './connection-log-redaction' diff --git a/mobile/src/notifications/desktop-notification-channel.web.ts b/mobile/src/notifications/desktop-notification-channel.web.ts new file mode 100644 index 00000000000..3f022bc46fb --- /dev/null +++ b/mobile/src/notifications/desktop-notification-channel.web.ts @@ -0,0 +1,10 @@ +/** + * Web sibling: the page creates no Android notification channel and cannot register push. The + * native file already returns early off Android, so what this changes is the import: it reaches + * `expo-notifications`, which at import reads the persisted registration behind a + * `typeof localStorage === 'undefined'` guard that the shell's DOM-storage-off WebView walks + * through with `null`, raising on `.getItem`. See `push-token.web.ts` beside it. + */ +export const DESKTOP_NOTIFICATION_CHANNEL_ID = 'orca-desktop' + +export const ensureDesktopNotificationChannel = (): Promise => Promise.resolve() diff --git a/mobile/src/notifications/push-token.web.ts b/mobile/src/notifications/push-token.web.ts new file mode 100644 index 00000000000..8b6b5acf1ae --- /dev/null +++ b/mobile/src/notifications/push-token.web.ts @@ -0,0 +1,14 @@ +import type { MobilePushToken } from './push-token' + +/** + * Web sibling: the page holds no device push token and cannot register one — the token is the + * shell's and the gateway has no page client. The import is the defect, not the calls, which are + * already inert here: `expo-notifications` runs `DevicePushTokenAutoRegistration.fx` at import, + * which reads the persisted registration behind a `typeof localStorage === 'undefined'` guard that + * Android's DOM-storage-off WebView walks through with `null`, raising on `.getItem`. + */ +export const getDevicePushToken = (): Promise => Promise.resolve(null) + +export function addPushTokenListener(_listener: (token: MobilePushToken) => void): () => void { + return () => {} +} diff --git a/mobile/src/transport/host-app-version-store.test.ts b/mobile/src/transport/host-app-version-store.test.ts index a9f9f92d105..9f7d916c3a7 100644 --- a/mobile/src/transport/host-app-version-store.test.ts +++ b/mobile/src/transport/host-app-version-store.test.ts @@ -9,11 +9,8 @@ vi.mock('@react-native-async-storage/async-storage', () => ({ default: asyncStorageMock })) -import { - loadHostAppVersion, - normalizeHostAppVersion, - recordHostAppVersion -} from './host-app-version-store' +import { normalizeHostAppVersion } from './host-app-version' +import { loadHostAppVersion, recordHostAppVersion } from './host-app-version-store' describe('host app version store', () => { beforeEach(() => { diff --git a/mobile/src/transport/host-app-version-store.ts b/mobile/src/transport/host-app-version-store.ts index 9c649cd2d0c..60c7d54e46c 100644 --- a/mobile/src/transport/host-app-version-store.ts +++ b/mobile/src/transport/host-app-version-store.ts @@ -1,23 +1,7 @@ import AsyncStorage from '@react-native-async-storage/async-storage' +import { normalizeHostAppVersion } from './host-app-version' const STORAGE_KEY_PREFIX = 'orca:host-app-version:v1:' -const MAX_VERSION_LENGTH = 64 - -export function normalizeHostAppVersion(value: unknown): string | null { - if (typeof value !== 'string') { - return null - } - const normalized = value.trim() - if ( - normalized.length === 0 || - normalized.length > MAX_VERSION_LENGTH || - normalized.includes('\n') || - normalized.includes('\r') - ) { - return null - } - return normalized -} export async function loadHostAppVersion(hostId: string): Promise { try { diff --git a/mobile/src/transport/host-app-version-store.web.ts b/mobile/src/transport/host-app-version-store.web.ts new file mode 100644 index 00000000000..0f7cbb91a29 --- /dev/null +++ b/mobile/src/transport/host-app-version-store.web.ts @@ -0,0 +1,10 @@ +/** + * Web sibling: the page keeps no record of the host's app version, because nothing in it reads one + * — the reader is the native troubleshoot screen's, outside this bundle. Not admitted through the + * storage seam for that reason: a key the page only writes is not page state, so `status.get` had + * every mount post `orca:host-app-version:v1:` for the bridge to refuse and log. + */ +export const loadHostAppVersion = (_hostId: string): Promise => Promise.resolve(null) + +export const recordHostAppVersion = (_hostId: string, _value: unknown): Promise => + Promise.resolve() diff --git a/mobile/src/transport/host-app-version.ts b/mobile/src/transport/host-app-version.ts new file mode 100644 index 00000000000..e8d0c8a6a04 --- /dev/null +++ b/mobile/src/transport/host-app-version.ts @@ -0,0 +1,25 @@ +const MAX_VERSION_LENGTH = 64 + +/** + * A host's self-reported app version as anything may hold it, or null. + * + * Beside the store rather than inside it because the two have different hosts: every build reads a + * version off `status.get`, and only a build with a device store keeps one. The bounds are the + * reasons a reported string is unusable at all — a newline splices a line into a diagnostics + * report, and an unbounded one is a host deciding how much of this device's storage to spend. + */ +export function normalizeHostAppVersion(value: unknown): string | null { + if (typeof value !== 'string') { + return null + } + const normalized = value.trim() + if ( + normalized.length === 0 || + normalized.length > MAX_VERSION_LENGTH || + normalized.includes('\n') || + normalized.includes('\r') + ) { + return null + } + return normalized +} diff --git a/mobile/src/transport/host-status-capability-ignorability.test.ts b/mobile/src/transport/host-status-capability-ignorability.test.ts index 4d4e9101d3c..b0714d09b33 100644 --- a/mobile/src/transport/host-status-capability-ignorability.test.ts +++ b/mobile/src/transport/host-status-capability-ignorability.test.ts @@ -21,7 +21,6 @@ import type { RpcResponse } from './types' const recordHostAppVersionMock = vi.hoisted(() => vi.fn().mockResolvedValue(undefined)) vi.mock('./host-app-version-store', () => ({ - normalizeHostAppVersion: (value: unknown) => (typeof value === 'string' ? value : null), recordHostAppVersion: (...args: unknown[]) => recordHostAppVersionMock(...args) })) diff --git a/mobile/src/transport/host-status-gates.test.ts b/mobile/src/transport/host-status-gates.test.ts index 7820eebc0d7..983f8146391 100644 --- a/mobile/src/transport/host-status-gates.test.ts +++ b/mobile/src/transport/host-status-gates.test.ts @@ -7,7 +7,6 @@ import { useHostStatusGates, type HostStatusGates } from './host-status-gates' const recordHostAppVersionMock = vi.hoisted(() => vi.fn().mockResolvedValue(undefined)) vi.mock('./host-app-version-store', () => ({ - normalizeHostAppVersion: (value: unknown) => (typeof value === 'string' ? value : null), recordHostAppVersion: (...args: unknown[]) => recordHostAppVersionMock(...args) })) diff --git a/mobile/src/transport/host-status-gates.ts b/mobile/src/transport/host-status-gates.ts index c9f92cb2305..f8e4817ec67 100644 --- a/mobile/src/transport/host-status-gates.ts +++ b/mobile/src/transport/host-status-gates.ts @@ -4,7 +4,8 @@ import type { ConnectionState } from './types' import { hostStatusProbe, readHostStatusGates } from './host-status-probe-operations' import { evaluateCompat, type CompatVerdict } from './protocol-compat' import type { HostStatusReply } from './host-status-reply-schema' -import { normalizeHostAppVersion, recordHostAppVersion } from './host-app-version-store' +import { normalizeHostAppVersion } from './host-app-version' +import { recordHostAppVersion } from './host-app-version-store' export type HostStatusGates = { hostCapabilities: string[] diff --git a/mobile/web-entry/web-overrides.json b/mobile/web-entry/web-overrides.json index cf2cbd812dd..f931d6084e0 100644 --- a/mobile/web-entry/web-overrides.json +++ b/mobile/web-entry/web-overrides.json @@ -156,6 +156,18 @@ { "file": "src/terminal/terminal-live-input-text-write.web.ts", "reason": "setNativeProps does not exist on React Native Web, where a TextInput ref is the DOM node, so the native write threw out of the session route's mount effect and faulted the page instead of clearing the field; this one sets value on the or