diff --git a/config/scripts/mobile-app-navigation-targets.mjs b/config/scripts/mobile-app-navigation-targets.mjs new file mode 100644 index 00000000000..9cb270726dc --- /dev/null +++ b/config/scripts/mobile-app-navigation-targets.mjs @@ -0,0 +1,284 @@ +import { readdirSync, readFileSync } from 'node:fs' +import { join } from 'node:path' +import { fileURLToPath } from 'node:url' +import ts from 'typescript-api' + +/** + * The host-route patterns the mobile app navigates to, read from its navigation call sites. + * + * Call sites, not every `/h/...` string the sources contain: a route also appears in the screen + * that mounts it, in a `pathname === ...` comparison and in a template type, and harvesting those + * makes every declared route "reachable" through its own mount. A census built on that answers + * "is this route declared", which it already knows, instead of "does anything hop to it". + * + * Resolved one step past the literal, because the two hops that matter are not written as one: + * the files explorer is pushed as `{ pathname: descriptor.pathname }` and the preview as + * `push(createMobileFilePreviewHref(...))`. So a local binding or a call is followed to the + * function that returns the pathname. A target it still cannot read is reported rather than + * dropped. + */ + +const MOBILE_ROOT = join(fileURLToPath(new URL('../..', import.meta.url)), 'mobile') + +/** `router`/`navigation` methods, plus the host-list action that wraps one. */ +const ROUTER_METHODS = new Set(['push', 'replace', 'navigate']) +const ROUTER_RECEIVERS = new Set(['router', 'navigation']) +const ACTION_NAVIGATORS = new Set(['navigateFromHostList']) + +function sourceFiles(dir) { + const found = [] + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const path = join(dir, entry.name) + if (entry.isDirectory()) { + found.push(...sourceFiles(path)) + } else if (/\.tsx?$/.test(entry.name) && !entry.name.includes('.test.')) { + found.push(path) + } + } + return found +} + +function each(node, visit) { + visit(node) + ts.forEachChild(node, (child) => each(child, visit)) +} + +/** Past the annotations a href picks up on its way to the call. */ +function unwrap(node) { + let current = node + while ( + current !== undefined && + (ts.isAsExpression(current) || + ts.isParenthesizedExpression(current) || + ts.isNonNullExpression(current) || + ts.isSatisfiesExpression(current)) + ) { + current = current.expression + } + return current +} + +/** A string or template as a pattern, each interpolation standing for one segment. */ +function literalPattern(node) { + const value = unwrap(node) + if (value === undefined) { + return null + } + if (ts.isStringLiteralLike(value)) { + return value.text + } + if (ts.isTemplateExpression(value)) { + return value.head.text + value.templateSpans.map((span) => `[p]${span.literal.text}`).join('') + } + return null +} + +/** A host route with its query dropped and adjacent interpolations collapsed to one segment. */ +function normalize(raw) { + if (raw === null || !raw.startsWith('/h/')) { + return null + } + return raw + .split('?')[0] + .replaceAll(/(?:\[p\])+/g, '[p]') + .replace(/\/$/, '') +} + +function nameOf(expression) { + if (ts.isIdentifier(expression)) { + return expression.text + } + return ts.isPropertyAccessExpression(expression) ? expression.name.text : null +} + +function isNavigator(expression) { + const name = nameOf(expression) + if (name === null) { + return false + } + if (ACTION_NAVIGATORS.has(name)) { + return true + } + if (!ROUTER_METHODS.has(name) || !ts.isPropertyAccessExpression(expression)) { + return false + } + return ROUTER_RECEIVERS.has(nameOf(expression.expression) ?? '') +} + +/** A href written out: the string itself, or the `pathname` of an object literal. */ +function writtenPathnames(node) { + const found = new Set() + const direct = normalize(literalPattern(node)) + if (direct !== null) { + found.add(direct) + } + const value = unwrap(node) + if (value === undefined || !ts.isObjectLiteralExpression(value)) { + return found + } + for (const property of value.properties) { + if (!ts.isPropertyAssignment(property) || property.name.getText() !== 'pathname') { + continue + } + const pathname = normalize(literalPattern(property.initializer)) + if (pathname !== null) { + found.add(pathname) + } + } + return found +} + +function parseSources() { + return [...sourceFiles(join(MOBILE_ROOT, 'src')), ...sourceFiles(join(MOBILE_ROOT, 'app'))].map( + (file) => ({ + file, + source: ts.createSourceFile( + file, + readFileSync(file, 'utf8'), + ts.ScriptTarget.Latest, + true, + file.endsWith('.tsx') ? ts.ScriptKind.TSX : ts.ScriptKind.TS + ) + }) + ) +} + +/** Functions that return a host route, by name, so a `push(builder(...))` resolves. */ +function routeBuilders(parsed) { + const builders = new Map() + for (const { source } of parsed) { + each(source, (node) => { + const name = declaredFunctionName(node) + if (name === undefined) { + return + } + const found = new Set() + each(node, (inner) => { + if (!ts.isReturnStatement(inner) || inner.expression === undefined) { + return + } + for (const pathname of writtenPathnames(inner.expression)) { + found.add(pathname) + } + }) + if (found.size > 0) { + builders.set(name, [...found]) + } + }) + } + return builders +} + +function declaredFunctionName(node) { + if (ts.isFunctionDeclaration(node)) { + return node.name?.text + } + if (!ts.isVariableDeclaration(node) || !ts.isIdentifier(node.name)) { + return undefined + } + const initializer = node.initializer + if (initializer === undefined) { + return undefined + } + return ts.isArrowFunction(initializer) || ts.isFunctionExpression(initializer) + ? node.name.text + : undefined +} + +/** Local consts holding a host route, written out or built by one of the builders above. */ +function localRouteBindings(source, builders) { + const bound = new Map() + each(source, (node) => { + if (!ts.isVariableDeclaration(node) || !ts.isIdentifier(node.name)) { + return + } + const initializer = unwrap(node.initializer) + if (initializer === undefined) { + return + } + const direct = normalize(literalPattern(initializer)) + if (direct !== null) { + bound.set(node.name.text, [direct]) + return + } + if (!ts.isCallExpression(initializer)) { + return + } + const built = builders.get(nameOf(initializer.expression) ?? '') + if (built !== undefined) { + bound.set(node.name.text, built) + } + }) + return bound +} + +function resolveTarget(expression, builders, bound) { + const found = writtenPathnames(expression) + if (found.size > 0) { + return found + } + const value = unwrap(expression) + if (value === undefined) { + return found + } + const indirect = ts.isCallExpression(value) + ? builders.get(nameOf(value.expression) ?? '') + : ts.isPropertyAccessExpression(value) && + value.name.text === 'pathname' && + ts.isIdentifier(value.expression) + ? bound.get(value.expression.text) + : ts.isIdentifier(value) + ? bound.get(value.text) + : undefined + for (const pathname of indirect ?? []) { + found.add(pathname) + } + if (found.size > 0 || !ts.isObjectLiteralExpression(value)) { + return found + } + for (const property of value.properties) { + if (!ts.isPropertyAssignment(property) || property.name.getText() !== 'pathname') { + continue + } + for (const pathname of resolveTarget(property.initializer, builders, bound)) { + found.add(pathname) + } + } + return found +} + +/** + * Every host route the app navigates to, and the call sites whose target could not be read. + * + * `unresolved` is mostly navigation away from `/h` altogether (pairing, settings) and the shell's + * own forwarding of a href it was handed; it is returned rather than swallowed so a new indirection + * is visible instead of quietly shrinking the census. + */ +export function mobileAppNavigationTargets() { + const parsed = parseSources() + const builders = routeBuilders(parsed) + const targets = new Set() + const unresolved = [] + for (const { file, source } of parsed) { + const bound = localRouteBindings(source, builders) + each(source, (node) => { + if (!ts.isCallExpression(node) || !isNavigator(node.expression)) { + return + } + const argument = node.arguments[0] + if (argument === undefined) { + return + } + const found = resolveTarget(argument, builders, bound) + if (found.size === 0) { + const line = source.getLineAndCharacterOfPosition(node.getStart()).line + 1 + unresolved.push(`${file.slice(MOBILE_ROOT.length + 1)}:${line}`) + return + } + for (const pathname of found) { + targets.add(pathname) + } + }) + } + return { targets: [...targets].sort(), unresolved } +} diff --git a/config/scripts/mobile-web-app-files-external-links.test.mjs b/config/scripts/mobile-web-app-files-external-links.test.mjs index 29bcf1d7b3b..d4f8a40fd94 100644 --- a/config/scripts/mobile-web-app-files-external-links.test.mjs +++ b/config/scripts/mobile-web-app-files-external-links.test.mjs @@ -15,7 +15,6 @@ import { fileURLToPath } from 'node:url' import { describe, expect, it } from 'vitest' import { mobileWebAppRouteClosure } from './build-mobile-web-app-bundle.mjs' -import { MOBILE_WEB_PAGE_ROUTES } from './mobile-web-page-routes.mjs' import { mobileWebAppDependenciesPresent } from './mobile-web-app-bundle-dependencies.mjs' import { EXTERNAL_LINK_SEAM as SEAM, @@ -60,34 +59,6 @@ describeClosure( 240_000 ) -/** - * The explorer declares at least what the preview does, because it can become the preview. - * - * Grants are resolved once, from the route the shell opened: `grantsForRoute` reads - * `session.routePathname` and `init.grants.native` carries the answer for the life of that - * session. The explorer's rows push to the preview, and because the preview is a page route the - * handoff keeps that push inside the same document — no second `init`, no re-resolved grants. So a - * preview reached that way runs under the explorer's grants, and anything the preview is granted - * and the explorer is not is refused at the call site with nothing on screen to say so. - * - * Pinned as a superset rather than as equality: the explorer may legitimately need a grant the - * preview does not. - */ -describe('the grants an in-page hop carries', () => { - const grantsOf = (pathname) => - MOBILE_WEB_PAGE_ROUTES.find((route) => route.pathname === pathname)?.grants - - it('gives the explorer every grant the preview declares', () => { - const explorer = grantsOf('/h/[hostId]/files/[worktreeId]') - const preview = grantsOf('/h/[hostId]/files/preview/[worktreeId]') - // Both declared, so a renamed route cannot turn this into a comparison of two undefineds. - expect(explorer, 'the explorer is declared').toBeDefined() - expect(preview, 'the preview is declared').toBeDefined() - expect(preview.length, 'the preview declares something to inherit').toBeGreaterThan(0) - expect(preview.filter((grant) => !explorer.includes(grant))).toEqual([]) - }) -}) - /** * Neither files page writes a clipboard, which is why neither is granted `native.clipboard.write`. * diff --git a/config/scripts/mobile-web-app-handoff-grants-render.test.mjs b/config/scripts/mobile-web-app-handoff-grants-render.test.mjs new file mode 100644 index 00000000000..f5271fa355d --- /dev/null +++ b/config/scripts/mobile-web-app-handoff-grants-render.test.mjs @@ -0,0 +1,290 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { chromium } from 'playwright-core' +import { buildMobileWebAppBundle } from './build-mobile-web-app-bundle.mjs' +import { mobileWebAppDependenciesPresent } from './mobile-web-app-bundle-dependencies.mjs' +import { + createBundleServer, + installShellDouble, + readBridgeFaultGrant, + readBridgeProtocolVersion, + readShellCsp +} from './mobile-web-app-render-harness.mjs' + +/** + * The in-page hop the sidebar makes, in a browser, under the grants the session actually has. + * + * On a wide layout `app/h/_layout.tsx` renders the worktree list beside every `/h` route, and its + * header pushes `/h//tasks` through `useRouteHandoff`. Keeping that local runs the tasks page + * under the opener's grants, so its copy actions refuse with nothing on screen. The unit tests pin + * the decision; only a browser proves the control exists, is reachable at that viewport, and that + * the document does not move when the hop is handed over. + */ + +const HOST_ROUTE = '/h/render-check-host' +const FILES_ROUTE = '/h/render-check-host/files/wt-1' +const HOST_PATTERN = '/h/[hostId]' +const TASKS_PATTERN = '/h/[hostId]/tasks' +const FILES_PATTERN = '/h/[hostId]/files/[worktreeId]' +const SHELL_SESSION_ID = 'render-check-session' +const SHELL_BUILD_ID = 'render-check-build' +const SHELL_HOST = { + id: 'render-check-host', + name: 'Render Check Host', + endpoint: 'ws://render-check', + lastConnected: 1 +} +/** The manifest's own pairs, as the shell would send them. */ +const PAGE_ROUTE_GRANTS = [ + { pathname: HOST_PATTERN, grants: ['navigate', 'storage', 'haptics'] }, + { pathname: FILES_PATTERN, grants: ['navigate', 'storage', 'externalLink', 'haptics'] }, + { + pathname: TASKS_PATTERN, + grants: ['navigate', 'storage', 'externalLink', 'haptics', 'native.clipboard.write'] + } +] +/** Wide enough for `app/h/_layout.tsx` to render the sidebar beside the route. */ +const WIDE = { width: 1180, height: 820 } +const NARROW = { width: 390, height: 844 } + +const bundles = mobileWebAppDependenciesPresent() +const describeRender = bundles ? describe : describe.skip + +let scratch +let server +let browser +let origin +let cspHeader = null +let bridgeVersion = null +let faultGrant = null + +beforeAll(async () => { + if (!bundles) { + return + } + cspHeader = await readShellCsp() + bridgeVersion = await readBridgeProtocolVersion() + faultGrant = await readBridgeFaultGrant() + scratch = await mkdtemp(join(tmpdir(), 'orca-mobile-web-app-handoff-')) + const built = await buildMobileWebAppBundle({ outDir: join(scratch, 'bundle') }) + const served = await createBundleServer({ outDir: built.outDir, cspHeader }) + server = served.server + origin = served.origin + const executablePath = process.env.ORCA_MOBILE_WEB_RENDER_BROWSER + browser = await chromium.launch({ headless: true, ...(executablePath ? { executablePath } : {}) }) +}, 180_000) + +afterAll(async () => { + await browser?.close() + server?.close() + if (scratch) { + await rm(scratch, { recursive: true, force: true }) + } +}) + +/** Opens the worktree list at a viewport, under a named set of session grants. */ +async function openHostRoute({ + viewport, + grants, + pageRouteGrants = PAGE_ROUTE_GRANTS, + route = HOST_ROUTE, + awaitText = SHELL_HOST.name +}) { + const page = await browser.newPage({ viewport }) + await page.addInitScript(installShellDouble, { + version: bridgeVersion, + sessionId: SHELL_SESSION_ID, + buildId: SHELL_BUILD_ID, + route: { pathname: route }, + host: SHELL_HOST, + storage: {}, + faultGrant, + grants, + pageRoutes: [HOST_PATTERN, FILES_PATTERN, TASKS_PATTERN], + pageRouteGrants + }) + const errors = [] + const scripts = [] + // Every script answer, not only the ones that arrived: a chunk the navigation waits on can fail + // with a status the 200-only list cannot show. + const jsResponses = [] + page.on('pageerror', (error) => errors.push(`${error.name}: ${error.message}`)) + page.on('console', (message) => { + if (message.type() === 'error') { + errors.push(`console.error: ${message.text()}`) + } + }) + page.on('response', (response) => { + const path = new URL(response.url()).pathname + if (!path.endsWith('.js')) { + return + } + jsResponses.push({ status: response.status(), path }) + if (response.status() === 200) { + scripts.push(path) + } + }) + await page.goto(`${origin}/`, { waitUntil: 'load' }) + await page.waitForFunction(() => document.documentElement.dataset.orcaWebEntry === 'mounted', { + timeout: 30_000, + polling: 250 + }) + await page.waitForFunction((needle) => document.body.innerText.includes(needle), awaitText, { + timeout: 30_000, + polling: 250 + }) + return { page, errors, scripts, jsResponses } +} + +/** Every `navigate` notify the page posted, in order. */ +function navigates(page) { + return page.evaluate(() => + (globalThis.__orcaRenderCheckNotifies ?? []).filter((frame) => frame.name === 'navigate') + ) +} + +/** + * The wait for the hop to land, and the page's own account of why it did not. + * + * A navigation that never commits reads as a bare 30 s timeout. The CI failure this file first hit + * was a `TypeError` thrown inside React Navigation that blanked the document, and it was invisible + * because the error assertions run after a wait that never returns. + */ +async function waitForTasksRoute(page, opened, clickedAt) { + try { + await page.waitForFunction(() => location.pathname.endsWith('/tasks'), { + timeout: 30_000, + polling: 250 + }) + } catch (cause) { + const seen = await page.evaluate(() => ({ + pathname: location.pathname, + text: document.body.innerText.slice(0, 300) + })) + throw new Error( + [ + `the document never reached /tasks; pathname is ${seen.pathname}`, + `page errors: ${JSON.stringify(opened.errors)}`, + `navigate notifies: ${JSON.stringify(await navigates(page))}`, + `body text: ${JSON.stringify(seen.text)}`, + `js responses since the click: ${JSON.stringify(opened.jsResponses.slice(clickedAt))}` + ].join('\n'), + { cause } + ) + } +} + +describeRender('the sidebar hop to tasks, under the session it was opened with', () => { + it('hands the hop to the shell when the session cannot cover tasks', async () => { + const opened = await openHostRoute({ + viewport: WIDE, + grants: [faultGrant, 'navigate', 'storage', 'haptics'] + }) + const { page, errors, scripts } = opened + const loadedBefore = [...scripts] + // The header's own control, by the name a user reads; it is the sidebar's on a wide layout. + await page.getByLabel('Tasks').first().click() + await page.waitForTimeout(1_500) + expect(await navigates(page)).toEqual([ + { v: bridgeVersion, type: 'notify', name: 'navigate', href: `${HOST_ROUTE}/tasks` } + ]) + // Handed over, not taken: the document stayed on the worktree list, and the tasks chunk was + // never fetched — which is what says the page did not quietly render it under these grants. + expect(await page.evaluate(() => location.pathname)).toBe(HOST_ROUTE) + expect(scripts.filter((path) => !loadedBefore.includes(path))).toEqual([]) + expect(errors).toEqual([]) + await page.close() + }, 60_000) + + it('keeps the hop in the document when the session covers tasks', async () => { + // The same tap, the same viewport, one more grant. Without this the case above would pass on a + // page that simply never navigates. + const opened = await openHostRoute({ + viewport: WIDE, + grants: [ + faultGrant, + 'navigate', + 'storage', + 'externalLink', + 'haptics', + 'native.clipboard.write' + ] + }) + const { page, errors } = opened + const clickedAt = opened.jsResponses.length + await page.getByLabel('Tasks').first().click() + await waitForTasksRoute(page, opened, clickedAt) + expect(await navigates(page)).toEqual([]) + expect(errors).toEqual([]) + await page.close() + }, 60_000) + + it('hands the hop over from the narrow header too, whose control C2.10 named', async () => { + // At this viewport `app/h/_layout.tsx` renders no sidebar, so the route's own header is the + // narrow toolbar. C2.10 gave its Tasks control the wide sibling's role and label, so the hop + // can be aimed at here rather than asserted absent, and the rule must hold on this branch as + // well: the control the phone actually presses is this one. + const opened = await openHostRoute({ + viewport: NARROW, + grants: [faultGrant, 'navigate', 'storage', 'haptics'] + }) + const { page, errors, scripts } = opened + const loadedBefore = [...scripts] + const tasks = page.getByLabel('Tasks') + // Exactly one: the narrow layout renders one toolbar, so this is the control, not a pick + // among siblings that could have hidden a wide header rendering here. + expect(await tasks.count()).toBe(1) + await tasks.click() + await page.waitForTimeout(1_500) + expect(await navigates(page)).toEqual([ + { v: bridgeVersion, type: 'notify', name: 'navigate', href: `${HOST_ROUTE}/tasks` } + ]) + expect(await page.evaluate(() => location.pathname)).toBe(HOST_ROUTE) + expect(scripts.filter((path) => !loadedBefore.includes(path))).toEqual([]) + expect(errors).toEqual([]) + await page.close() + }, 60_000) + + it('keeps the old behaviour when the shell sent no pairs at all', async () => { + // An older shell: the page cannot tell covered from uncovered, and must not start handing + // every hop over on the strength of a field nobody sent. + const opened = await openHostRoute({ + viewport: WIDE, + grants: [faultGrant, 'navigate', 'storage', 'haptics'], + pageRouteGrants: null + }) + const { page, errors } = opened + const clickedAt = opened.jsResponses.length + await page.getByLabel('Tasks').first().click() + await waitForTasksRoute(page, opened, clickedAt) + expect(await navigates(page)).toEqual([]) + expect(errors).toEqual([]) + await page.close() + }, 60_000) + + it('hands the sidebar hop over from a files route too, which is the general shape', async () => { + // The defect is not "the worktree list pushes tasks": on a wide layout the sidebar renders + // beside EVERY `/h` route, so the same hop exists from files, whose session carries + // `externalLink` but not `native.clipboard.write`. One opener proving it would leave the + // general case to inference. + const opened = await openHostRoute({ + viewport: WIDE, + grants: [faultGrant, 'navigate', 'storage', 'externalLink', 'haptics'], + route: FILES_ROUTE, + awaitText: SHELL_HOST.name + }) + const { page, errors, scripts } = opened + const loadedBefore = [...scripts] + await page.getByLabel('Tasks').first().click() + await page.waitForTimeout(1_500) + expect(await navigates(page)).toEqual([ + { v: bridgeVersion, type: 'notify', name: 'navigate', href: `${HOST_ROUTE}/tasks` } + ]) + expect(await page.evaluate(() => location.pathname)).toBe(FILES_ROUTE) + expect(scripts.filter((path) => !loadedBefore.includes(path))).toEqual([]) + expect(errors).toEqual([]) + await page.close() + }, 60_000) +}) diff --git a/config/scripts/mobile-web-app-render-harness.mjs b/config/scripts/mobile-web-app-render-harness.mjs index 5d5b13b879b..1809985f6c9 100644 --- a/config/scripts/mobile-web-app-render-harness.mjs +++ b/config/scripts/mobile-web-app-render-harness.mjs @@ -170,6 +170,7 @@ export function installShellDouble({ faultGrant, grants, pageRoutes = null, + pageRouteGrants = null, replies, streams = [], windowCaps = null @@ -224,6 +225,9 @@ export function installShellDouble({ native: grants ?? [faultGrant] }, ...(pageRoutes === null ? {} : { pageRoutes }), + // Omitted when the caller names none, which is the older-shell case the page falls back + // on: an absent field is not an empty one, and the page reads the difference. + ...(pageRouteGrants === null ? {} : { pageRouteGrants }), // Omitted for a shell too old to name one, which is the case the page has a panel for. ...(route === null ? {} : { route }), ...(host === null ? {} : { host }), 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 26eaeac1ebf..7e3e689a049 100644 --- a/config/scripts/mobile-web-app-session-terminal-closure.test.mjs +++ b/config/scripts/mobile-web-app-session-terminal-closure.test.mjs @@ -108,11 +108,16 @@ const MERMAID_PACKAGE = 'node_modules/mermaid/' /** * The module list with mermaid on the page, recorded at the base in the docstring above, plus - * one: `src/mobile-web-shell/bridge/bridge-haptics-notify.ts`, which `haptics.web.ts` reaches + * three. + * + * One is `src/mobile-web-shell/bridge/bridge-haptics-notify.ts`, which `haptics.web.ts` reaches * since C7.10 E landed beside this pin (#21864 and #21871 were each green against a main without - * the other). + * the other). The other two are C2.9's: `src/mobile-web-shell/bridge/bridge-page-route-grants.ts` + * and the `mobile-web-bundle/manifest-contract.ts` whose grant grammar it imports rather than + * restates. Both reach every page closure through `bridge-envelope.ts`, which the page reads to + * parse `init`, so this count moves for any route the page serves and not for the session alone. */ -const MODULES_WITH_MERMAID = 4324 +const MODULES_WITH_MERMAID = 4326 const artifactModules = (inputs) => inputs.filter((input) => input.includes(MERMAID_PAGE_ENGINE)) const packageModules = (inputs) => inputs.filter((input) => input.includes(MERMAID_PACKAGE)) diff --git a/config/scripts/mobile-web-page-route-hop-coverage.test.mjs b/config/scripts/mobile-web-page-route-hop-coverage.test.mjs new file mode 100644 index 00000000000..259df53b745 --- /dev/null +++ b/config/scripts/mobile-web-page-route-hop-coverage.test.mjs @@ -0,0 +1,108 @@ +import { describe, expect, it } from 'vitest' +import { mobileAppNavigationTargets } from './mobile-app-navigation-targets.mjs' +import { MOBILE_WEB_PAGE_ROUTES } from './mobile-web-page-routes.mjs' + +/** + * Every in-page hop between page routes, and whether the opener's grants cover the target. + * + * Grants are resolved once, from the route the shell opened, so a push kept inside the document + * runs the target under the opener's list. C2.9 made the handoff refuse to keep a hop it cannot + * cover, which is the fix; this is the census that says which hops those are, so adding a grant to + * a route — or a new push between two — shows up as a change here rather than as a verb that + * silently refuses on a device. + * + * Openers are every page route, not the one that pushes: on a wide layout `app/h/_layout.tsx` + * renders the worktree-list sidebar beside every `/h` route, and its header pushes tasks. That is + * what makes a pairwise pin the wrong shape — the sidebar reaches everything. + * + * Targets are the routes the app navigates to, read from its call sites rather than from every + * `/h/...` template in the sources: a route's own mount declares its pathname, so harvesting those + * made every declared route reachable and the filter inert. + */ + +/** Whether a concrete pattern from the source names the same route as a manifest pattern. */ +function sameRoute(pushed, declared) { + const a = pushed.split('/') + const b = declared.split('/') + if (a.length !== b.length) { + return false + } + return a.every((segment, index) => { + const other = b[index] + const dynamic = (value) => value?.startsWith('[') === true + return dynamic(segment) || dynamic(other) ? true : segment === other + }) +} + +/** + * Which hops the rule hands to the shell, pinned by name. + * + * Empty would mean every page route covers every other, which is not a property this codebase has + * and not one to assume: the point of the pin is that a new entry appears when a route's grants + * grow, and that the entry is read before it ships rather than found on a device. + * + * What is NOT here is the point of the census. `files/[worktreeId] -> files/preview/[worktreeId]` + * is absent because the preview declares no more than the explorer, so that hop stays in the + * document — which is C3.1's pairwise pin, now a consequence of the rule rather than a rule of its + * own. The two `-> tasks` entries and the four `-> files/*` entries are the hops the sidebar and + * the rows make into a route that asks for more than their opener holds. + */ +const HANDED_OFF = [ + '/h/[hostId] -> /h/[hostId]/files/[worktreeId]', + '/h/[hostId] -> /h/[hostId]/files/preview/[worktreeId]', + '/h/[hostId] -> /h/[hostId]/tasks', + '/h/[hostId]/agent-history/[worktreeId] -> /h/[hostId]/files/[worktreeId]', + '/h/[hostId]/agent-history/[worktreeId] -> /h/[hostId]/files/preview/[worktreeId]', + '/h/[hostId]/agent-history/[worktreeId] -> /h/[hostId]/tasks', + '/h/[hostId]/files/[worktreeId] -> /h/[hostId]/tasks', + '/h/[hostId]/files/preview/[worktreeId] -> /h/[hostId]/tasks' +] + +describe('in-page hops between page routes', () => { + it('finds the hops the app actually builds, so the census is not empty', () => { + const { targets } = mobileAppNavigationTargets() + // The sidebar's tasks push is the hop this lane exists for; if the census stops seeing it the + // pin below would go quietly green. Deleting the header's two pushes reds this case, which is + // what the derivation bought: the tasks screen still declares its own pathname. + expect(targets.some((pattern) => sameRoute(pattern, '/h/[hostId]/tasks'))).toBe(true) + }) + + it('pins every hop the handoff must take away from the page', () => { + const pushed = mobileAppNavigationTargets().targets + const handedOff = [] + for (const opener of MOBILE_WEB_PAGE_ROUTES) { + for (const target of MOBILE_WEB_PAGE_ROUTES) { + if (target.pathname === opener.pathname) { + continue + } + const reachable = pushed.some((pattern) => sameRoute(pattern, target.pathname)) + if (!reachable) { + continue + } + const covered = target.grants.every((grant) => opener.grants.includes(grant)) + if (!covered) { + handedOff.push(`${opener.pathname} -> ${target.pathname}`) + } + } + } + expect(handedOff.sort()).toEqual([...HANDED_OFF].sort()) + }) + + it('covers a hop whose target asks for no more than its opener, rather than handing it off', () => { + // The other half of the rule, asserted on the manifest rather than assumed: a target declaring + // a subset stays in the document, which is what keeps an ordinary hop cheap. + // The explorer to its own preview, which is the hop C3.1 pinned pairwise: the preview asks for + // no more than the explorer, so the rule keeps it local and the pairwise pin is redundant. + const explorer = MOBILE_WEB_PAGE_ROUTES.find( + (route) => route.pathname === '/h/[hostId]/files/[worktreeId]' + ) + const preview = MOBILE_WEB_PAGE_ROUTES.find( + (route) => route.pathname === '/h/[hostId]/files/preview/[worktreeId]' + ) + if (!explorer || !preview) { + throw new Error('the manifest lost a route this census is written against') + } + expect(preview.grants.length, 'the preview declares something to inherit').toBeGreaterThan(0) + expect(preview.grants.filter((grant) => !explorer.grants.includes(grant))).toEqual([]) + }) +}) diff --git a/config/scripts/mobile-web-page-routes.mjs b/config/scripts/mobile-web-page-routes.mjs index ffcbd5fc3e2..018c80dbeca 100644 --- a/config/scripts/mobile-web-page-routes.mjs +++ b/config/scripts/mobile-web-page-routes.mjs @@ -41,12 +41,12 @@ export const MOBILE_WEB_PAGE_ROUTES = [ // components the host layout renders above it. // // `externalLink` is transitive, not its own: a row opens the preview, and because that is a page - // route the handoff keeps the push inside this document. Grants are resolved once, from the route - // the shell opened (`grantsForRoute` on `session.routePathname`), so a preview reached that way - // runs under *this* route's grants for the life of the session — and a Markdown link in it would - // be refused by `notifyExternalLink` and do nothing at all. So a route must declare a superset of - // the grants of every page route its screens push to locally, which for this one means the - // preview's list. The census beside it pins that pair. + // route and this list covers what it declares, the handoff keeps that push inside this document. + // Grants are resolved once, from the route the shell opened (`grantsForRoute` on + // `session.routePathname`), so a preview reached that way runs under *this* route's grants for + // the life of the session. Covering the preview is therefore what buys the cheap in-document hop, + // not what makes it correct: a target this list did not cover would be handed to the shell and + // reopened under its own grants instead. The census beside it reads that relation off this list. // // Nothing the explorer itself renders opens a URL. The two openers in its own closure are the // shared layout's — the protocol wall, and the New Workspace source field the sidebar renders on @@ -54,12 +54,10 @@ export const MOBILE_WEB_PAGE_ROUTES = [ // `externalLink`. That tablet tap stays dead on all of them: a pre-existing gap this route // neither widens nor fixes. // - // One hop is still open and is not this series' to close: the sidebar `HostScreen` the layout - // renders on a wide layout pushes to `/h//tasks` through the handoff, which is local, so - // from any page route on a tablet the tasks page runs without `native.clipboard.write` and its - // copy actions refuse silently. Pre-existing on main for the worktree list and agent history - // since C2.1; the fix is a handoff rule — hand off to the shell when the target's grants exceed - // the session's — in its own PR. + // The sidebar `HostScreen` the layout renders on a wide layout pushes to `/h//tasks` from + // every page route, and no other route declares the `native.clipboard.write` that one asks for. + // The handoff gives that hop to the shell rather than keeping it here, which is why this list + // does not grow a grant it has no screen for. { pathname: '/h/[hostId]/files/[worktreeId]', grants: ['navigate', 'storage', 'externalLink', 'haptics'] diff --git a/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx b/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx index d81f5d1ec04..4475bfa4d92 100644 --- a/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx +++ b/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx @@ -152,6 +152,7 @@ export function MobileWebShellScreen({ const { state, pageRoutes, + pageRouteGrants, routeGrants, retry, reportShellFailure, @@ -167,6 +168,7 @@ export function MobileWebShellScreen({ hostId, route, pageRoutes, + pageRouteGrants, routeGrants, session: state, snapshot, diff --git a/mobile/src/mobile-web-shell/bridge-host-contract.ts b/mobile/src/mobile-web-shell/bridge-host-contract.ts index 670dc334001..2e623323828 100644 --- a/mobile/src/mobile-web-shell/bridge-host-contract.ts +++ b/mobile/src/mobile-web-shell/bridge-host-contract.ts @@ -84,6 +84,14 @@ export type BridgeHostOptions = { route: BridgeInitRoute /** Every route pattern the shell would render from the page, so the page knows what to keep. */ pageRoutes: readonly string[] + /** + * What each of those patterns declared, from the manifest this shell already holds. + * + * The page decides an in-page hop with it: a push is kept local only when the target's grants are + * covered by this session's. Optional, because a shell with no manifest entry for a pattern has + * nothing to say about it and the page then keeps its old rule. + */ + pageRouteGrants?: readonly { pathname: string; grants: readonly string[] }[] /** * What the route this session was opened for declared, narrowed to what this shell implements. * diff --git a/mobile/src/mobile-web-shell/bridge-host-init.test.ts b/mobile/src/mobile-web-shell/bridge-host-init.test.ts index 32c1ee2dfd3..dbb116b60d1 100644 --- a/mobile/src/mobile-web-shell/bridge-host-init.test.ts +++ b/mobile/src/mobile-web-shell/bridge-host-init.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' import { clientFrame, createFakeRpcClient } from './bridge-host-test-fakes' -import { harness, HOST, PAGE_ROUTES, ROUTE } from './bridge-host-test-harness' +import { harness, HOST, PAGE_ROUTE_GRANTS, PAGE_ROUTES, ROUTE } from './bridge-host-test-harness' import { BRIDGE_MAX_PENDING_REQUESTS, BRIDGE_MAX_ROUTE_PATHNAME_CHARS, @@ -53,11 +53,62 @@ describe('init and state', () => { }, route: ROUTE, pageRoutes: PAGE_ROUTES, + pageRouteGrants: PAGE_ROUTE_GRANTS, host: HOST, storage: {} }) }) + /** + * The page cannot decide an in-page hop without knowing what the target needs. + * + * `pageRoutes` says which patterns this shell would render; it does not say what each one + * declared. A page that keeps a push local on the strength of the pattern alone runs the target + * under the opener's grants, which is how the tasks page reached the sidebar without + * `native.clipboard.write`. So `init` carries the manifest's own pairs. + */ + it('carries what every page route declared, not only which patterns exist', () => { + const bridge = harness() + bridge.host.receive(clientFrame({ type: 'ready' })) + const init = bridge.last() + expect(init.type).toBe('init') + if (init.type !== 'init') { + throw new Error('expected an init frame') + } + // Every pattern the page is told it may keep has an entry saying what keeping it costs. + expect((init.pageRouteGrants ?? []).map((entry) => entry.pathname)).toEqual([...PAGE_ROUTES]) + expect(init.pageRouteGrants).toEqual(PAGE_ROUTE_GRANTS) + }) + + it('refuses a grant name the manifest grammar refuses, naming the field it came from', () => { + // The host reads the manifest through the same grammar the desktop wrote it under, so a name + // the bundle could not have declared cannot reach the page through this field either. + const bridge = harness({ + pageRouteGrants: [{ pathname: '/h/[hostId]', grants: ['native.clipboard'] }] + }) + bridge.host.receive(clientFrame({ type: 'ready' })) + expect(bridge.posted.length).toBe(0) + expect(bridge.routeRefusals).toHaveLength(1) + const [reason] = bridge.routeRefusals + // The prefix is the whole point: this route is well formed, so a reason that does not name the + // field sends whoever reads the refusal to look at a pathname that was never the problem. + expect(reason.startsWith('pageRouteGrants: ')).toBe(true) + expect(reason.slice('pageRouteGrants: '.length)).not.toBe('') + // The callback and the diagnostic are two readers of one verdict; they must not disagree. + expect(bridge.diagnostics).toEqual([{ kind: 'route-refused', issue: reason }]) + }) + + it('blames the route, not the pairs, when the route is the malformed one', () => { + // The control for the case above. Both refusals arrive through one string, so without an + // opener that fails for the other reason the prefix assertion holds on any reason at all. + const bridge = harness({ route: { pathname: '/h/a?b' } }) + bridge.host.receive(clientFrame({ type: 'ready' })) + expect(bridge.routeRefusals).toHaveLength(1) + const [reason] = bridge.routeRefusals + expect(reason.startsWith('pageRouteGrants: ')).toBe(false) + expect(reason).not.toBe('') + }) + it('names the screen the page is standing in for, which its own `/` cannot tell it', () => { const route = { pathname: '/h/host-a/session/wt-1', params: { name: 'a branch' } } const bridge = harness({ route }) diff --git a/mobile/src/mobile-web-shell/bridge-host-test-harness.ts b/mobile/src/mobile-web-shell/bridge-host-test-harness.ts index 43e35d41cb5..232af559e6a 100644 --- a/mobile/src/mobile-web-shell/bridge-host-test-harness.ts +++ b/mobile/src/mobile-web-shell/bridge-host-test-harness.ts @@ -52,6 +52,11 @@ export type Harness = { export const ROUTE = { pathname: '/h/host-a' } export const PAGE_ROUTES = ['/h/[hostId]'] +/** What those patterns declared, as the manifest would carry it, `haptics` included: it is on every + * real entry, so a pair without it is a shape the host never receives. */ +export const PAGE_ROUTE_GRANTS = [ + { pathname: '/h/[hostId]', grants: ['navigate', 'storage', 'haptics'] } +] export const HOST = { id: 'host-a', name: 'Host A', endpoint: 'ws://host-a', lastConnected: 5 } export function harness( @@ -72,6 +77,8 @@ export function harness( */ /** What the mounted route declared; everything this shell implements unless a case narrows it. */ routeGrants?: readonly string[] + /** The manifest pairs this shell would send; a case may hand it a malformed one. */ + pageRouteGrants?: readonly { pathname: string; grants: readonly string[] }[] /** Stands for a host rebuilt under a page whose session already handshook. */ sessionEstablished?: boolean ready?: boolean @@ -106,6 +113,7 @@ export function harness( sessionId: 'session-a', route: options.route ?? ROUTE, pageRoutes: PAGE_ROUTES, + pageRouteGrants: options.pageRouteGrants ?? PAGE_ROUTE_GRANTS, routeGrants: options.routeGrants ?? MOBILE_WEB_SHELL_GRANTS, sessionEstablished: options.sessionEstablished ?? false, host: HOST, diff --git a/mobile/src/mobile-web-shell/bridge-host.ts b/mobile/src/mobile-web-shell/bridge-host.ts index 07654822e27..c5eb18cbebf 100644 --- a/mobile/src/mobile-web-shell/bridge-host.ts +++ b/mobile/src/mobile-web-shell/bridge-host.ts @@ -16,6 +16,7 @@ import { type BridgeConnectionSnapshot, type BridgeHostMessage } from './bridge/bridge-envelope' +import { BridgePageRouteGrantsSchema } from './bridge/bridge-page-route-grants' import { captureBridgeError } from './bridge/bridge-error-capture' import { createBridgeInitFrame } from './bridge/bridge-init-frame' import { BRIDGE_HAPTICS_NOTIFY } from './bridge/bridge-haptics-notify' @@ -51,7 +52,17 @@ export function createBridgeHost(options: BridgeHostOptions): BridgeHost { // the wire as a route no page will accept; without this the page refuses the whole `init`, asks // again on its backoff forever, and the shell un-hides a WebView that will never paint. const parsedRoute = BridgeInitRouteSchema.safeParse(options.route) - const route = parsedRoute.success ? parsedRoute.data : null + // Checked here for the reason the route is: a pair the page's reader would refuse takes the whole + // `init` with it, and a session that never gets one is worse than one that never starts. + const parsedRouteGrants = + options.pageRouteGrants === undefined + ? null + : BridgePageRouteGrantsSchema.safeParse(options.pageRouteGrants) + const routeGrantsIssue = + parsedRouteGrants !== null && !parsedRouteGrants.success + ? (parsedRouteGrants.error.issues[0]?.message ?? 'unknown') + : null + const route = parsedRoute.success && routeGrantsIssue === null ? parsedRoute.data : null let closed = false // One document's turn at the bridge. `close` ends it and the next `ready` begins the next one; // between the two the view belongs to no document, so nothing is served and nothing is posted. @@ -154,6 +165,7 @@ export function createBridgeHost(options: BridgeHostOptions): BridgeHost { connection: snapshot(), route, pageRoutes, + ...(parsedRouteGrants?.success === true ? { pageRouteGrants: parsedRouteGrants.data } : {}), granted, host, storage: options.readStorage() @@ -356,9 +368,11 @@ export function createBridgeHost(options: BridgeHostOptions): BridgeHost { if (route === null) { // At construction rather than on the first `ready`: the verdict does not depend on the page // behaving, and a shell that waited for a frame would hold a blank view until one arrived. - const issue = parsedRoute.success - ? 'unknown' - : (parsedRoute.error.issues[0]?.message ?? 'unknown') + const issue = routeGrantsIssue + ? `pageRouteGrants: ${routeGrantsIssue}` + : parsedRoute.success + ? 'unknown' + : (parsedRoute.error.issues[0]?.message ?? 'unknown') options.onDiagnostic?.({ kind: 'route-refused', issue }) options.onRouteRefused(issue) } diff --git a/mobile/src/mobile-web-shell/bridge/bridge-client-session.ts b/mobile/src/mobile-web-shell/bridge/bridge-client-session.ts index faf39dc00c3..8872d00acdc 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-client-session.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-client-session.ts @@ -15,6 +15,14 @@ export type BridgeShellSession = { /** The route patterns this page may keep for itself. Empty for a shell that names none, which * hands every navigation back and is what a shell with no `navigate` grant can honour. */ pageRoutes: readonly string[] + /** + * What each of those patterns declared, when the shell said. + * + * `null` for a shell that sent none, which is the only thing that separates "this route needs + * nothing" from "nobody told me". The handoff keeps its older rule on `null` and cannot invent a + * coverage verdict out of an absent field. + */ + pageRouteGrants: readonly { pathname: string; grants: readonly string[] }[] | null /** Null for a shell too old to name it; the page's own `loadHosts()` then answers with nothing. */ host: BridgeInitHost | null /** The allowlisted keys as the app held them when this page opened. */ @@ -36,6 +44,7 @@ export function readShellSession( grants: message.grants, route: message.route ?? null, pageRoutes: message.pageRoutes ?? [], + pageRouteGrants: message.pageRouteGrants ?? null, host: message.host ?? null, storage: message.storage ?? {} } diff --git a/mobile/src/mobile-web-shell/bridge/bridge-envelope.ts b/mobile/src/mobile-web-shell/bridge/bridge-envelope.ts index 3168a59f833..5128446fc42 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-envelope.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-envelope.ts @@ -3,6 +3,7 @@ import { isRpcResponse } from '../../transport/rpc-response-shape' import type { RpcResponse } from '../../transport/types' import { BridgeErrorCaptureSchema } from './bridge-error-capture' import { BRIDGE_HAPTICS_NOTIFY_FIELDS } from './bridge-haptics-notify' +import { BridgePageRouteGrantsSchema } from './bridge-page-route-grants' import { isPageStorageKey, PAGE_STORAGE_MAX_ENTRIES, @@ -396,7 +397,20 @@ const BridgeHostMessageSchema = z.union([ pageRoutes: z .array(z.string().min(1).max(BRIDGE_MAX_ROUTE_PATHNAME_CHARS)) .max(BRIDGE_MAX_PAGE_ROUTES) - .optional() + .optional(), + /** + * What each of those patterns declared, so the page can tell a hop it may keep from one it must + * hand back. + * + * `pageRoutes` says which routes this shell would render; it does not say what each costs. A + * page keeping a push local on the pattern alone runs the target under the opener's grants, + * which is how the tasks page was reached from the sidebar without `native.clipboard.write`. + * + * Optional in both directions: an older shell omits it and the page falls back to today's + * behaviour, an older page ignores it. The grant grammar is the manifest's own, so a name the + * bundle could not have declared cannot arrive here either. + */ + pageRouteGrants: BridgePageRouteGrantsSchema.optional() }) ]) diff --git a/mobile/src/mobile-web-shell/bridge/bridge-init-frame.ts b/mobile/src/mobile-web-shell/bridge/bridge-init-frame.ts index 59adbc06265..889f8d35de2 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-init-frame.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-init-frame.ts @@ -34,6 +34,9 @@ export function createBridgeInitFrame(args: { route: BridgeInitRoute /** The route patterns the page keeps for itself; everything else comes back as `navigate`. */ pageRoutes: readonly string[] + /** What each of those patterns declared, so the page can tell a hop it may keep from one it + * must hand back. Omitted by a shell that has none, which leaves the page on its old rule. */ + pageRouteGrants?: readonly { pathname: string; grants: readonly string[] }[] /** What this session may do: the protocol's own grant plus what its route declared. */ granted: readonly string[] /** The host the page is showing, minus the credential the bridge already carries for it. */ @@ -58,6 +61,16 @@ export function createBridgeInitFrame(args: { }, route: args.route, pageRoutes: [...args.pageRoutes], + // Copied entry by entry for the reason the grants are: nothing the shell keeps may be + // reachable through a frame it hands out. + ...(args.pageRouteGrants === undefined + ? {} + : { + pageRouteGrants: args.pageRouteGrants.map((entry) => ({ + pathname: entry.pathname, + grants: [...entry.grants] + })) + }), host: args.host, // Copied for the same reason the grants are: the frame is serialized straight after, and what // the shell holds must not be reachable through what it hands out. diff --git a/mobile/src/mobile-web-shell/bridge/bridge-page-route-grants.ts b/mobile/src/mobile-web-shell/bridge/bridge-page-route-grants.ts new file mode 100644 index 00000000000..12885458179 --- /dev/null +++ b/mobile/src/mobile-web-shell/bridge/bridge-page-route-grants.ts @@ -0,0 +1,33 @@ +import { z } from 'zod' +import { + MOBILE_WEB_BUNDLE_MAX_ROUTE_GRANTS, + MobileWebBundleGrantNameSchema +} from '../../../../src/shared/mobile-web-bundle/manifest-contract' +import { BRIDGE_MAX_PAGE_ROUTES, BRIDGE_MAX_ROUTE_PATHNAME_CHARS } from './bridge-caps' + +/** + * What each page route declared, as it crosses in `init`. + * + * `pageRoutes` says which patterns this shell would render; it does not say what each costs. A page + * that keeps a push local on the strength of the pattern alone runs the target under the opener's + * grants, which is how the tasks page was reached from the sidebar without `native.clipboard.write`. + * + * Its own module rather than a block inside the envelope: the envelope is at its line ceiling, and + * this is a self-contained shape the host validates before it builds a frame — so it is read in two + * places and belongs in one. + * + * The grant grammar is the manifest's own, imported rather than restated, so a name the bundle + * could not have declared cannot arrive here either. + */ +export const BridgePageRouteGrantsSchema = z + .array( + z + .object({ + pathname: z.string().min(1).max(BRIDGE_MAX_ROUTE_PATHNAME_CHARS), + grants: z.array(MobileWebBundleGrantNameSchema).max(MOBILE_WEB_BUNDLE_MAX_ROUTE_GRANTS) + }) + .strict() + ) + .max(BRIDGE_MAX_PAGE_ROUTES) + +export type BridgePageRouteGrants = z.infer diff --git a/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.test.ts b/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.test.ts index dad86cba6c7..0913fca311a 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.test.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.test.ts @@ -37,6 +37,7 @@ describe('the bridge port pair', () => { // pair names both so the session it hands back is the shape a page on a route actually holds. route: expect.objectContaining({ pathname: expect.any(String) }), pageRoutes: expect.any(Array), + pageRouteGrants: null, host: expect.objectContaining({ id: expect.any(String) }), storage: expect.any(Object) }) diff --git a/mobile/src/mobile-web-shell/bridge/bridge-rpc-client-frames.test.ts b/mobile/src/mobile-web-shell/bridge/bridge-rpc-client-frames.test.ts index af267840b37..cc7e87906b2 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-rpc-client-frames.test.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-rpc-client-frames.test.ts @@ -127,6 +127,7 @@ describe('bridge client handshake', () => { route: null, // And one that names no page routes, so the page hands every navigation back. pageRoutes: [], + pageRouteGrants: null, // And no host and no stored keys, which is what `host-store.web.ts` then answers with. host: null, storage: {} diff --git a/mobile/src/mobile-web-shell/mobile-web-shell-session-contract.ts b/mobile/src/mobile-web-shell/mobile-web-shell-session-contract.ts index a12d1d107f4..674c88046eb 100644 --- a/mobile/src/mobile-web-shell/mobile-web-shell-session-contract.ts +++ b/mobile/src/mobile-web-shell/mobile-web-shell-session-contract.ts @@ -186,6 +186,9 @@ export type MobileWebShellSession = { /** Every route pattern this shell would render from the page, as the bundle in hand declares * them. The page is told, so it keeps a navigation into one of them instead of handing it back. */ readonly pageRoutes: readonly string[] + /** The same routes with what each declared, which is what lets the page tell a hop it may keep + * from one that would run under the wrong grants. */ + readonly pageRouteGrants: readonly { pathname: string; grants: readonly string[] }[] /** What the route this mount stands for declared, narrowed to what this shell implements. It is * what `init` grants, so a route that asked for less is served less. */ readonly routeGrants: readonly string[] diff --git a/mobile/src/mobile-web-shell/mobile-web-shell-session-grants.test.ts b/mobile/src/mobile-web-shell/mobile-web-shell-session-grants.test.ts index 5bb55257d57..e71d99f6928 100644 --- a/mobile/src/mobile-web-shell/mobile-web-shell-session-grants.test.ts +++ b/mobile/src/mobile-web-shell/mobile-web-shell-session-grants.test.ts @@ -58,3 +58,65 @@ describe('falling back to the cached generation after a failed download', () => expect([...step.session.routeGrants]).toEqual(['navigate', 'native.clipboard.read']) }) }) + +/** + * The route/grant pairs the page is told about must survive the download path, not only the two + * paths that open a generation already on disk. + * + * `init.pageRouteGrants` is how the page decides an in-page hop is covered. A session that reaches + * `ready` without them carries the default (or the previous generation's), the page reads every + * target as listed-with-no-entry, and hands every hop to the shell. That is the first install and + * every update after it. + */ +describe('the route grants a downloaded generation is activated under', () => { + const TWO_ROUTES = [ + { pathname: '/h/[hostId]', grants: ['navigate', 'storage'] }, + { pathname: '/h/[hostId]/tasks', grants: ['navigate', 'native.clipboard.write'] } + ] + const DOWNLOADED: MobileWebShellManifestFacts = { + ...MANIFEST, + buildId: 'c'.repeat(64), + routes: TWO_ROUTES + } + + function activate(session: Parameters[0], manifest: MobileWebShellManifestFacts) { + return run( + session, + { type: 'manifest-read', manifest }, + { type: 'download-staged' }, + { + type: 'activated', + generationDirectory: '/cache/gen', + sessionId: 'session-downloaded', + buildId: manifest.buildId, + totalBytes: manifest.totalBytes, + elapsedMs: 9 + } + ) + } + + it('is the manifest it downloaded, on a cold cache', () => { + const step = activate(afterCacheRead(null).session, DOWNLOADED) + expect(step.session.state.kind).toBe('ready') + expect(step.session.pageRouteGrants).toEqual(TWO_ROUTES) + }) + + it('is the manifest it matched, on a cached hit', () => { + const step = run(afterCacheRead(CACHED).session, { type: 'manifest-read', manifest: MANIFEST }) + expect(step.session.state.kind).toBe('activating') + expect(step.session.pageRouteGrants).toEqual(PAGE_ROUTES) + }) + + it("replaces the previous generation's entries when the generation changes", () => { + // The session already holds the older bundle's pairs, so the assertion fails on a stale field + // rather than only on the empty default. + const first = run(afterCacheRead(CACHED).session, { type: 'manifest-read', manifest: MANIFEST }) + expect(first.session.pageRouteGrants).toEqual(PAGE_ROUTES) + const step = activate( + run(first.session, { type: 'retry-pressed' }, { type: 'cache-read', generation: CACHED }) + .session, + DOWNLOADED + ) + expect(step.session.pageRouteGrants).toEqual(TWO_ROUTES) + }) +}) diff --git a/mobile/src/mobile-web-shell/mobile-web-shell-session.ts b/mobile/src/mobile-web-shell/mobile-web-shell-session.ts index 06f6c931fb1..b8bffcd42fd 100644 --- a/mobile/src/mobile-web-shell/mobile-web-shell-session.ts +++ b/mobile/src/mobile-web-shell/mobile-web-shell-session.ts @@ -15,7 +15,7 @@ import type { MobileWebShellStep } from './mobile-web-shell-session-contract' import { awaitsGates, gateKey, gateVerdict } from './mobile-web-shell-gates' -import { grantsForRoute, implementedPageRoutes, matchesRoutePattern } from './page-route-policy' +import { matchesRoutePattern, routeViewOf } from './page-route-policy' /** * The host's connection state as the three answers a step here needs. @@ -40,7 +40,6 @@ export function readMobileWebShellReachability( const CHECKING: MobileWebShellSessionState = { kind: 'checking' } const NATIVE_ROUTE: MobileWebShellSessionState = { kind: 'native-route' } -/** Whether the page renders this route: listed by the bundle, and needing nothing this shell lacks. */ function rendersRoute(pageRoutes: readonly string[], pathname: string): boolean { return pageRoutes.some((pattern) => matchesRoutePattern(pathname, pattern)) } @@ -49,6 +48,7 @@ export function createMobileWebShellSession(routePathname: string): MobileWebShe return { routePathname, pageRoutes: [], + pageRouteGrants: [], routeGrants: [], state: CHECKING, retriedOnce: false, @@ -148,11 +148,10 @@ function onCacheRead( return step(session, { cached: null, state: { kind: 'offline' } }) } // The cached bundle's own list, which is the only one an unreachable host can be judged by. - const pageRoutes = implementedPageRoutes(generation.routes) - const routeGrants = grantsForRoute(generation.routes, session.routePathname) - return rendersRoute(pageRoutes, session.routePathname) - ? openCached(session, generation, { cached: generation, pageRoutes, routeGrants }) - : step(session, { cached: generation, pageRoutes, routeGrants, state: NATIVE_ROUTE }) + const view = routeViewOf(generation.routes, session.routePathname) + return rendersRoute(view.pageRoutes, session.routePathname) + ? openCached(session, generation, { cached: generation, ...view }) + : step(session, { cached: generation, ...view, state: NATIVE_ROUTE }) } return step(session, { cached: generation, state: CHECKING }, [{ kind: 'read-manifest' }]) } @@ -167,10 +166,12 @@ function onManifestRead( } // Before the compat verdict, because a route that stays native has nothing to wall about: a // bundle this shell could not open is not a reason to refuse a screen it was never going to open. - const pageRoutes = implementedPageRoutes(manifest.routes) - const routeGrants = grantsForRoute(manifest.routes, session.routePathname) + const { pageRoutes, pageRouteGrants, routeGrants } = routeViewOf( + manifest.routes, + session.routePathname + ) if (!rendersRoute(pageRoutes, session.routePathname)) { - return step(session, { pageRoutes, routeGrants, state: NATIVE_ROUTE }) + return step(session, { pageRoutes, pageRouteGrants, routeGrants, state: NATIVE_ROUTE }) } const verdict = evaluateMobileWebBundleCompat({ hostCapabilities: gates.hostCapabilities, @@ -182,12 +183,13 @@ function onManifestRead( } const cached = session.cached if (cached !== null && cached.buildId === manifest.buildId) { - return openCached(session, cached, { pageRoutes, routeGrants }) + return openCached(session, cached, { pageRoutes, pageRouteGrants, routeGrants }) } return step( session, { pageRoutes, + pageRouteGrants, routeGrants, state: { kind: 'fetching', @@ -258,12 +260,14 @@ function onDownloadFailed( // download was attempted, so its `pageRoutes` and `routeGrants` are already on the session: // opening the cached page under them would grant it what a bundle it is not running declared, // and would mount it for a route only the newer bundle claims. - const pageRoutes = implementedPageRoutes(cached.routes) - const routeGrants = grantsForRoute(cached.routes, session.routePathname) + const { pageRoutes, pageRouteGrants, routeGrants } = routeViewOf( + cached.routes, + session.routePathname + ) if (!rendersRoute(pageRoutes, session.routePathname)) { - return step(session, { pageRoutes, routeGrants, state: NATIVE_ROUTE }) + return step(session, { pageRoutes, pageRouteGrants, routeGrants, state: NATIVE_ROUTE }) } - return openCached(session, cached, { pageRoutes, routeGrants }) + return openCached(session, cached, { pageRoutes, pageRouteGrants, routeGrants }) } return step(session, { state: { kind: 'failed', reason: 'download-failed', retriedOnce: session.retriedOnce } diff --git a/mobile/src/mobile-web-shell/page-route-policy.test.ts b/mobile/src/mobile-web-shell/page-route-policy.test.ts index 2d137c8fe1e..00dfbb35899 100644 --- a/mobile/src/mobile-web-shell/page-route-policy.test.ts +++ b/mobile/src/mobile-web-shell/page-route-policy.test.ts @@ -1,11 +1,11 @@ import { describe, expect, it } from 'vitest' import { MobileWebBundleRouteSchema } from '../../../src/shared/mobile-web-bundle/manifest-contract' import { - implementedPageRoutes, matchesRoutePattern, pageRendersRoute, MOBILE_WEB_SHELL_GRANTS, - grantsForRoute + grantsForRoute, + routeViewOf } from './page-route-policy' import { BRIDGE_HAPTICS_GRANT } from './bridge/bridge-haptics-notify' import { @@ -14,6 +14,16 @@ import { BRIDGE_NATIVE_VERBS } from './bridge/bridge-native-verbs' +/** + * The patterns a session would be told it may keep, read through the view the reducer builds. + * + * The pathname is arbitrary here: `pageRoutes` is the filtered list and does not depend on which + * route the session was opened for, which the grant cases below read separately. + */ +function pageRoutesOf(routes: Parameters[0]): string[] { + return routeViewOf(routes, '/h/host-1').pageRoutes +} + describe('matching a concrete route against a pattern', () => { it('matches a dynamic segment against one segment and never against a path', () => { expect(matchesRoutePattern('/h/host-1', '/h/[hostId]')).toBe(true) @@ -41,19 +51,17 @@ describe('matching a concrete route against a pattern', () => { describe('the routes this shell will render from the page', () => { it('keeps a route whose grants it implements', () => { - expect(implementedPageRoutes([{ pathname: '/h/[hostId]', grants: ['navigate'] }])).toEqual([ - '/h/[hostId]' - ]) - expect(implementedPageRoutes([{ pathname: '/h/[hostId]', grants: [] }])).toEqual([ + expect(pageRoutesOf([{ pathname: '/h/[hostId]', grants: ['navigate'] }])).toEqual([ '/h/[hostId]' ]) + expect(pageRoutesOf([{ pathname: '/h/[hostId]', grants: [] }])).toEqual(['/h/[hostId]']) }) it('drops a route needing a grant this app has never heard of', () => { // The whole point of the negotiation: a newer desktop shipping a screen that needs more than // this app can do leaves that one route native rather than handing it a dead tap. expect( - implementedPageRoutes([ + pageRoutesOf([ { pathname: '/h/[hostId]', grants: ['navigate', 'teleport'] }, { pathname: '/h/[hostId]/tasks', grants: ['navigate'] } ]) @@ -61,7 +69,7 @@ describe('the routes this shell will render from the page', () => { }) it('answers nothing for a desktop older than the field', () => { - expect(implementedPageRoutes(undefined)).toEqual([]) + expect(pageRoutesOf(undefined)).toEqual([]) expect(pageRendersRoute(undefined, '/h/host-1')).toBe(false) }) @@ -123,7 +131,7 @@ describe('the grants this app implements', () => { * contract's rule and is pinned there, beside the pattern that decides it. */ it('serves a route that needs the screencast lane', () => { expect( - implementedPageRoutes([ + pageRoutesOf([ { pathname: '/h/[hostId]/session/[worktreeId]', grants: ['navigate', 'screencastBinary'] } ]) ).toEqual(['/h/[hostId]/session/[worktreeId]']) @@ -142,7 +150,7 @@ describe('the grants this app implements', () => { { pathname: '/h/[hostId]/session/[worktreeId]', grants: ['navigate', 'aGrantFromTheFuture'] } ] expect(grantsForRoute(routes, '/h/host-1/session/wt-1')).toEqual(['navigate']) - expect(implementedPageRoutes(routes)).toEqual([]) + expect(pageRoutesOf(routes)).toEqual([]) }) it('resolves the screencast lane for a route that declares it', () => { @@ -176,12 +184,12 @@ describe('the native verbs this app serves', () => { it('names them so a route can declare one, which is what keeps that route native without it', () => { // A bundle listing a route that needs the clipboard, against a shell too old to serve it. expect( - implementedPageRoutes([ + pageRoutesOf([ { pathname: '/h/[hostId]/tasks', grants: ['navigate', 'native.clipboard.write'] } ]) ).toEqual(['/h/[hostId]/tasks']) expect( - implementedPageRoutes([ + pageRoutesOf([ { pathname: '/h/[hostId]/tasks', grants: ['navigate', 'native.dictation.start'] } ]) ).toEqual([]) @@ -199,7 +207,7 @@ describe('the native verbs this app serves', () => { describe('a grant name this build has never heard of', () => { it('leaves that route native rather than refusing the bundle', () => { expect( - implementedPageRoutes([ + pageRoutesOf([ { pathname: '/h/[hostId]', grants: ['navigate'] }, { pathname: '/h/[hostId]/tasks', grants: ['navigate', 'native.dictation.start'] } ]) @@ -228,7 +236,7 @@ describe('a grant name this build has never heard of', () => { /** * What a token on every page route costs against a shell that does not carry it. * - * `implementedPageRoutes` filters on `grants.every(implementsGrant)`, so one grant this build lacks + * The route filter behind this view is `grants.every(implementsGrant)`, so one grant this build lacks * takes the whole route native rather than degrading the feature that needed it. `haptics` is * declared by all five page routes, which makes the whole set conditional on a shell carrying the * token; the route list itself is pinned in `config/scripts/mobile-web-app-haptics-seam.test.mjs`, @@ -241,7 +249,7 @@ describe('a page route that needs the haptics token', () => { } it('is served by this shell, which implements the token', () => { - expect(implementedPageRoutes([route])).toEqual(['/h/[hostId]']) + expect(pageRoutesOf([route])).toEqual(['/h/[hostId]']) }) it('renders natively against a shell whose grant list does not carry it', () => { @@ -253,10 +261,10 @@ describe('a page route that needs the haptics token', () => { grant === BRIDGE_HAPTICS_GRANT ? 'hapticsUnderAnotherName' : grant ) } - expect(implementedPageRoutes([older])).toEqual([]) + expect(pageRoutesOf([older])).toEqual([]) // The control, so the empty list above is the token and not the other two grants. expect( - implementedPageRoutes([ + pageRoutesOf([ { ...route, grants: route.grants.filter((grant) => grant !== BRIDGE_HAPTICS_GRANT) } ]) ).toEqual(['/h/[hostId]']) diff --git a/mobile/src/mobile-web-shell/page-route-policy.ts b/mobile/src/mobile-web-shell/page-route-policy.ts index 5e3a2c87dbd..8d9c7100fcd 100644 --- a/mobile/src/mobile-web-shell/page-route-policy.ts +++ b/mobile/src/mobile-web-shell/page-route-policy.ts @@ -59,16 +59,21 @@ export function matchesRoutePattern(pathname: string, pattern: string): boolean } /** - * The patterns this shell will render from the page: listed, and needing nothing it lacks. + * The routes this shell will render from the page: listed, and needing nothing it lacks. * * `every` and not `some`: one grant this build lacks takes the whole route native, so a token every * page route declares couples the whole set to a shell that carries it — `haptics` is the first, * and against a shell without it no page route is served at all. */ -export function implementedPageRoutes(routes: readonly MobileWebPageRoute[] | undefined): string[] { - return (routes ?? []) - .filter((route) => route.grants.every(implementsGrant)) - .map((route) => route.pathname) +function implementedPageRouteEntries( + routes: readonly MobileWebPageRoute[] | undefined +): MobileWebPageRoute[] { + return (routes ?? []).filter((route) => route.grants.every(implementsGrant)) +} + +/** The patterns alone, for the readers in this module that only name routes. */ +function implementedPageRoutes(routes: readonly MobileWebPageRoute[] | undefined): string[] { + return implementedPageRouteEntries(routes).map((route) => route.pathname) } /** @@ -104,3 +109,19 @@ export function grantsForRoute( const declared = (routes ?? []).find((route) => matchesRoutePattern(pathname, route.pathname)) return declared === undefined ? [] : declared.grants.filter(implementsGrant) } + +/** + * What one bundle's route list says about one session, in the three shapes the reducer needs. + * + * Derived together because they are one reading of one list: the patterns the page may keep, what + * each of them declared, and what this route itself was granted. Three sites used to spell this + * out; a fourth spelling is how they drift. + */ +export function routeViewOf(routes: readonly MobileWebPageRoute[] | undefined, pathname: string) { + const entries = implementedPageRouteEntries(routes) + return { + pageRoutes: entries.map((route) => route.pathname), + pageRouteGrants: entries, + routeGrants: grantsForRoute(routes, pathname) + } +} diff --git a/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.test.ts b/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.test.ts index d793bee94fe..68fd49f10aa 100644 --- a/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.test.ts +++ b/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.test.ts @@ -128,6 +128,7 @@ function Harness(props: { // Built inline on every render, as a caller writes it: the host is not rebuilt for it. route: { pathname: '/h/host-1' }, pageRoutes: ['/h/[hostId]'], + pageRouteGrants: [{ pathname: '/h/[hostId]', grants: ['navigate', 'storage'] }], routeGrants: [ 'navigate', 'storage', diff --git a/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.ts b/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.ts index 840c34e8606..fde2ee6f455 100644 --- a/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.ts +++ b/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.ts @@ -62,6 +62,7 @@ export function useMobileWebShellBridge(args: { route: BridgeInitRoute /** The route patterns the page keeps for itself; everything else comes back as `navigate`. */ pageRoutes: readonly string[] + pageRouteGrants: readonly { pathname: string; grants: readonly string[] }[] /** What this route declared, which is what `init` grants and what every grant check reads. */ routeGrants: readonly string[] /** Opens a screen the page does not render, over the still-mounted view. */ @@ -104,6 +105,7 @@ export function useMobileWebShellBridge(args: { // object in the deps would rebuild the host on every render and settle its pendings each time. const routeRef = useRef(args.route) const pageRoutesRef = useRef(args.pageRoutes) + const pageRouteGrantsRef = useRef(args.pageRouteGrants) const routeGrantsRef = useRef(args.routeGrants) /** The session that has completed a handshake, so a host rebuilt for it inherits that. */ const establishedSessionRef = useRef(null) @@ -125,6 +127,7 @@ export function useMobileWebShellBridge(args: { useLayoutEffect(() => { routeRef.current = args.route pageRoutesRef.current = args.pageRoutes + pageRouteGrantsRef.current = args.pageRouteGrants routeGrantsRef.current = args.routeGrants navigateRef.current = args.onNavigate externalLinkRef.current = args.onExternalLink @@ -150,6 +153,7 @@ export function useMobileWebShellBridge(args: { args.onStorageWrite, args.readStorage, args.pageRoutes, + args.pageRouteGrants, args.routeGrants, args.route ]) @@ -167,6 +171,7 @@ export function useMobileWebShellBridge(args: { sessionId, route: routeRef.current, pageRoutes: pageRoutesRef.current, + pageRouteGrants: pageRouteGrantsRef.current, routeGrants: routeGrantsRef.current, sessionEstablished: establishedSessionRef.current === sessionId, onPageFault: (error) => { diff --git a/mobile/src/mobile-web-shell/use-mobile-web-shell-session.ts b/mobile/src/mobile-web-shell/use-mobile-web-shell-session.ts index 8c9e2296dec..119918141cf 100644 --- a/mobile/src/mobile-web-shell/use-mobile-web-shell-session.ts +++ b/mobile/src/mobile-web-shell/use-mobile-web-shell-session.ts @@ -25,6 +25,7 @@ export type MobileWebShellSessionView = { readonly state: MobileWebShellSessionState /** The route patterns this shell would render from the page, for the page to be told about. */ readonly pageRoutes: readonly string[] + readonly pageRouteGrants: readonly { pathname: string; grants: readonly string[] }[] readonly routeGrants: readonly string[] readonly retry: () => void /** B3's failure reasons, forwarded verbatim; the reducer owns what each one means. */ @@ -226,6 +227,7 @@ export function useMobileWebShellSession(args: { return { state, pageRoutes: sessionRef.current.pageRoutes, + pageRouteGrants: sessionRef.current.pageRouteGrants, routeGrants: sessionRef.current.routeGrants, retry, reportShellFailure, diff --git a/mobile/src/navigation/route-handoff.web.test.tsx b/mobile/src/navigation/route-handoff.web.test.tsx index 9a6d493a4f4..8471a858617 100644 --- a/mobile/src/navigation/route-handoff.web.test.tsx +++ b/mobile/src/navigation/route-handoff.web.test.tsx @@ -475,3 +475,107 @@ describe('navigation options', () => { ]) }) }) + +/** + * An in-page hop only to a route this session's grants already cover. + * + * Grants are resolved once, from the route the shell opened, so a push kept local runs the target + * under the opener's list. On a wide layout the sidebar reaches the tasks page from every `/h` + * route, so keeping that hop local runs tasks without `native.clipboard.write` and its copy actions + * refuse with nothing on screen to say why. Handing it to the shell opens it as its own session, + * with its own grants. + */ +describe('an in-page hop the session cannot cover', () => { + const TASKS = '/h/host-a/tasks' + const PAIRS = [ + { pathname: '/h/[hostId]', grants: ['navigate', 'storage', 'haptics'] }, + { + pathname: '/h/[hostId]/tasks', + grants: ['navigate', 'storage', 'externalLink', 'haptics', 'native.clipboard.write'] + } + ] + const withPairs = (native: string[]) => ({ + ...INIT, + grants: { ...INIT.grants, native }, + pageRoutes: ['/h/[hostId]', '/h/[hostId]/tasks'], + pageRouteGrants: PAIRS + }) + + it('goes to the shell when the target needs a grant this session lacks', () => { + const { posted, handoff } = mount(withPairs(['navigate', 'storage', 'haptics'])) + handoff.push(TASKS) + expect(navigations(posted)).toEqual([ + { v: BRIDGE_PROTOCOL_VERSION, type: 'notify', name: 'navigate', href: TASKS } + ]) + expect(router.push).not.toHaveBeenCalled() + }) + + it('stays in this document when the session already covers the target', () => { + const { posted, handoff } = mount( + withPairs(['navigate', 'storage', 'externalLink', 'haptics', 'native.clipboard.write']) + ) + handoff.push(TASKS) + expect(navigations(posted)).toEqual([]) + expect(router.push).toHaveBeenCalledWith(TASKS, undefined) + }) + + it("hands off when the session lacks any one of the target's grants, not the clipboard alone", () => { + // The tasks route declares five grants and this session holds four. Without a case that + // withholds `externalLink` alone, a rule reading only the verb grants would pass every case. + const { posted, handoff } = mount( + withPairs(['navigate', 'storage', 'haptics', 'native.clipboard.write']) + ) + handoff.push(TASKS) + expect(navigations(posted)).toEqual([ + { v: BRIDGE_PROTOCOL_VERSION, type: 'notify', name: 'navigate', href: TASKS } + ]) + expect(router.push).not.toHaveBeenCalled() + }) + + it('keeps a hop whose target declares a subset, which is C3.1 without its pairwise pin', () => { + // explorer ⊇ preview: the opener was granted more than the target asks for. + const { posted, handoff } = mount({ + ...INIT, + grants: { ...INIT.grants, native: ['navigate', 'storage', 'externalLink', 'haptics'] }, + pageRoutes: ['/h/[hostId]/files/[worktreeId]', '/h/[hostId]/files/preview/[worktreeId]'], + pageRouteGrants: [ + { pathname: '/h/[hostId]/files/[worktreeId]', grants: ['navigate', 'storage', 'haptics'] }, + { pathname: '/h/[hostId]/files/preview/[worktreeId]', grants: ['navigate', 'haptics'] } + ] + }) + handoff.push('/h/host-a/files/preview/wt-1') + expect(navigations(posted)).toEqual([]) + expect(router.push).toHaveBeenCalledWith('/h/host-a/files/preview/wt-1', undefined) + }) + + it('leaves a non-page route exactly as it was', () => { + const { posted, handoff } = mount(withPairs(['navigate', 'storage', 'haptics'])) + handoff.push('/h/host-a/session/wt-1') + expect(navigations(posted)).toHaveLength(1) + expect(router.push).not.toHaveBeenCalled() + }) + + it('keeps the old rule when the shell named no grants, so an older shell is unchanged', () => { + // Absent, not empty: a shell that says nothing cannot be read as "this route needs nothing". + const { posted, handoff } = mount({ + ...INIT, + grants: { ...INIT.grants, native: ['navigate', 'storage', 'haptics'] }, + pageRoutes: ['/h/[hostId]', '/h/[hostId]/tasks'] + }) + handoff.push(TASKS) + expect(navigations(posted)).toEqual([]) + expect(router.push).toHaveBeenCalledWith(TASKS, undefined) + }) + + it('hands off a target the shell lists with no entry of its own', () => { + // Listed as renderable but absent from the pairs: the page cannot show it is covered, and a + // hop it cannot justify goes to the shell rather than running on the opener's grants. + const { posted, handoff } = mount({ + ...withPairs(['navigate', 'storage', 'haptics']), + pageRouteGrants: [{ pathname: '/h/[hostId]', grants: ['navigate', 'storage', 'haptics'] }] + }) + handoff.push(TASKS) + expect(navigations(posted)).toHaveLength(1) + expect(router.push).not.toHaveBeenCalled() + }) +}) diff --git a/mobile/src/navigation/route-handoff.web.ts b/mobile/src/navigation/route-handoff.web.ts index c7ddcd40ebd..7106a96f8eb 100644 --- a/mobile/src/navigation/route-handoff.web.ts +++ b/mobile/src/navigation/route-handoff.web.ts @@ -136,11 +136,39 @@ export function useRouteHandoff(): RouteHandoff { return useMemo(() => { const report = createRefusalReporter() - /** Whether this document is the one that renders the target, which is the shell's answer. */ + /** + * Whether this document both renders the target and may: pattern listed, grants covered. + * + * Covered matters because grants are resolved once, from the route the shell opened, and a push + * kept local runs the target under the opener's list. On a wide layout the sidebar reaches the + * tasks page from every `/h` route, so keeping that hop local runs tasks without + * `native.clipboard.write` and its copy actions refuse with nothing on screen to say why. + * Handing it over instead opens it as its own session, with its own grants. + * + * A shell that sent no pairs gets the old answer: `null` is "nobody told me", which is not the + * same as "this route needs nothing", and an older shell must keep working. A target the shell + * lists but names no entry for is not covered — the page cannot justify the hop, so it hands it + * over rather than guessing. + */ const servedHere = (target: string): boolean => { const pathname = pathnameOf(target) - const pageRoutes = client.getShellSession()?.pageRoutes ?? [] - return pageRoutes.some((pattern) => matchesRoutePattern(pathname, pattern)) + const session = client.getShellSession() + const pattern = (session?.pageRoutes ?? []).find((candidate) => + matchesRoutePattern(pathname, candidate) + ) + if (pattern === undefined) { + return false + } + const pairs = session?.pageRouteGrants ?? null + if (pairs === null) { + return true + } + const declared = pairs.find((entry) => entry.pathname === pattern) + if (declared === undefined) { + return false + } + const held = session?.grants.native ?? [] + return declared.grants.every((grant) => held.includes(grant)) } const handOff = (href: RouterHref): RouteHandoffOutcome => { // Resolved, not stringified: the object form is `[object Object]` under `String`, and the diff --git a/src/shared/mobile-web-bundle/manifest-contract.ts b/src/shared/mobile-web-bundle/manifest-contract.ts index b0a554b8dd8..23a48d1922e 100644 --- a/src/shared/mobile-web-bundle/manifest-contract.ts +++ b/src/shared/mobile-web-bundle/manifest-contract.ts @@ -38,6 +38,19 @@ const ROUTE_PATHNAME_PATTERN = /^\/(?![/\\])[^?#\s]*$/ */ const GRANT_NAME_PATTERN = /^(?:[a-zA-Z][a-zA-Z0-9]*|native(?:\.[a-z][a-z0-9]*){2,})$/ +/** + * One grant name, as both the manifest and the bridge read it. + * + * Exported so the `init` frame's route-grant pairs are checked by the same grammar the desktop + * wrote the manifest under. Two spellings of one rule drift, and the half that matters is the half + * the page believes. + */ +export const MobileWebBundleGrantNameSchema = z + .string() + .min(1) + .max(MAX_GRANT_NAME_LENGTH) + .regex(GRANT_NAME_PATTERN) + /** Every segment must be a name the bundle root can hold on all three desktop platforms: no * traversal, and none of the Windows shapes that cannot be created or that resolve to a device. * The regex already bans absolute paths, backslashes, spaces, and empty segments. */ @@ -110,9 +123,7 @@ export function computeMobileWebBundleId(assets: readonly MobileWebBundleAsset[] export const MobileWebBundleRouteSchema = z .object({ pathname: z.string().min(1).max(MAX_ROUTE_PATHNAME_LENGTH).regex(ROUTE_PATHNAME_PATTERN), - grants: z - .array(z.string().min(1).max(MAX_GRANT_NAME_LENGTH).regex(GRANT_NAME_PATTERN)) - .max(MOBILE_WEB_BUNDLE_MAX_ROUTE_GRANTS) + grants: z.array(MobileWebBundleGrantNameSchema).max(MOBILE_WEB_BUNDLE_MAX_ROUTE_GRANTS) }) .strict()