From e1c8df41e533b1c49adaca55e5d6c374e90ebe8a Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Mon, 21 Sep 2026 20:20:44 -0400 Subject: [PATCH] fix(mobile): declare externalLink on the two page routes that reach the protocol wall (OTA phase C follow-up) (#22113) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(mobile): hold every page route to the externalLink call site it reaches The grant call-site census carried an exact allowance for the two routes that reach the shared protocol wall's `openExternalLink` without declaring `externalLink`. Removing it makes the census enforce the declaration instead of recording the gap; it now names both routes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): declare externalLink on the worktree list and agent history Both routes render the shared layout's `HostProtocolGate`, whose `ProtocolBlockScreen` opens its Update Orca link through `openExternalLink`, and neither declared the grant: the tap posted a notify the shell refuses, with nothing on screen. The census measures one call site in each closure, `src/components/ProtocolBlockScreen.tsx`. Repinned by measurement, with the manifest change named: the route-list pin, and the handed-off hop census, which goes 23 rows to 19. The four rows that leave are these two routes into the explorer and its preview — all four now declare the same four grants, so a tapped file stays in the document instead of costing a native frame and a second bridge session. A new case asserts that coverage, so the four absences are load-bearing. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- ...ile-web-app-handoff-grants-render.test.mjs | 2 +- ...ile-web-app-page-grant-call-sites.test.mjs | 23 +--------- ...obile-web-page-route-hop-coverage.test.mjs | 45 +++++++++++++++---- config/scripts/mobile-web-page-routes.mjs | 29 ++++++------ .../scripts/mobile-web-page-routes.test.mjs | 4 +- 5 files changed, 56 insertions(+), 47 deletions(-) diff --git a/config/scripts/mobile-web-app-handoff-grants-render.test.mjs b/config/scripts/mobile-web-app-handoff-grants-render.test.mjs index bf0ae266210..abc3bd013a4 100644 --- a/config/scripts/mobile-web-app-handoff-grants-render.test.mjs +++ b/config/scripts/mobile-web-app-handoff-grants-render.test.mjs @@ -40,7 +40,7 @@ const SHELL_HOST = { } /** The manifest's own pairs, as the shell would send them. */ const PAGE_ROUTE_GRANTS = [ - { pathname: HOST_PATTERN, grants: ['navigate', 'storage', 'haptics'] }, + { pathname: HOST_PATTERN, grants: ['navigate', 'storage', 'externalLink', 'haptics'] }, { pathname: FILES_PATTERN, grants: ['navigate', 'storage', 'externalLink', 'haptics'] }, { pathname: TASKS_PATTERN, diff --git a/config/scripts/mobile-web-app-page-grant-call-sites.test.mjs b/config/scripts/mobile-web-app-page-grant-call-sites.test.mjs index 5b440aab93c..2c1eec4dde0 100644 --- a/config/scripts/mobile-web-app-page-grant-call-sites.test.mjs +++ b/config/scripts/mobile-web-app-page-grant-call-sites.test.mjs @@ -41,25 +41,6 @@ function closureOf(pathname) { return held } -/** - * The one place a page route reaches a seam it does not declare, recorded rather than exempted. - * - * `app/h/_layout.tsx` wraps every `/h` route in `HostProtocolGate`, whose `ProtocolBlockScreen` - * offers an Update Orca link through `openExternalLink`. Six routes declare `externalLink` and two - * do not, so on those two the wall's link posts a notify the shell refuses — a dead tap with - * nothing on screen. Pre-existing on main and not C7.7's to change: widening two other routes' - * grants is a capability decision, and this lane reports rather than fixes it. - * - * Exact, so it reds in both directions: adding the grant to either route empties an entry here and - * a new gap anywhere adds one. - */ -const KNOWN_UNDECLARED = new Map([ - [ - 'externalLink', - ['/h/[hostId] needs externalLink', '/h/[hostId]/agent-history/[worktreeId] needs externalLink'] - ] -]) - describe('the call-site reader', () => { const navigate = PAGE_GRANT_CALL_SITES[0] const storage = PAGE_GRANT_CALL_SITES[1] @@ -150,10 +131,10 @@ describeClosure( */ it.each(PAGE_GRANT_CALL_SITES.map((row) => [row.grants.join(' + '), row]))( 'declares %s on every registered route whose own call sites reach it', - async (name, row) => { + async (_name, row) => { expect( await grantsMissingForRow(mobileDir, MOBILE_WEB_PAGE_ROUTES, closureOf, row) - ).toEqual(KNOWN_UNDECLARED.get(name) ?? []) + ).toEqual([]) } ) diff --git a/config/scripts/mobile-web-page-route-hop-coverage.test.mjs b/config/scripts/mobile-web-page-route-hop-coverage.test.mjs index fc2395cadca..3991e64913e 100644 --- a/config/scripts/mobile-web-page-route-hop-coverage.test.mjs +++ b/config/scripts/mobile-web-page-route-hop-coverage.test.mjs @@ -44,8 +44,14 @@ function sameRoute(pushed, declared) { * 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. + * own. Absent for the same reason, and measured rather than reasoned: the four hops the worktree + * list and the history screen make into the explorer and its preview, which left this list when + * those two declared the `externalLink` their own protocol wall reaches and took it from 23 rows + * to 19. The four now declare the same four grants, so a tapped file costs no native frame and no + * second bridge session. + * + * What remains beside the session rows is twelve: four openers holding no `native.clipboard.write` + * into the three routes that ask for it — tasks, the hub and review. * * Absent for the same reason, and the reason C4 registered its two routes in one PR: * `source-control ⇄ review` in both directions. The hub's rows push review and review replaces @@ -53,21 +59,17 @@ function sameRoute(pushed, declared) { * landing alone would have put a handoff — a new native screen and a new bridge session — between * a changed-file row and its diff. * - * The seven C7 rows are the same rule with the arrows all one way: 16 -> 23, every new entry - * `X -> session`, one from each other page route. The session screen's fourteen grants are a strict + * The seven C7 rows are the same rule with the arrows all one way: every one `X -> session`, one + * from each other page route. The session screen's fourteen grants are a strict * superset of every other route's, so nothing can reach it under the grants it was opened with — * and nothing it pushes to leaves, because its own seven targets each declare a subset. A row in * the other direction would mean a route had grown a grant the session lacks. */ const HANDED_OFF = [ - '/h/[hostId] -> /h/[hostId]/files/[worktreeId]', - '/h/[hostId] -> /h/[hostId]/files/preview/[worktreeId]', '/h/[hostId] -> /h/[hostId]/review/[worktreeId]', '/h/[hostId] -> /h/[hostId]/session/[worktreeId]', '/h/[hostId] -> /h/[hostId]/source-control/[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]/review/[worktreeId]', '/h/[hostId]/agent-history/[worktreeId] -> /h/[hostId]/session/[worktreeId]', '/h/[hostId]/agent-history/[worktreeId] -> /h/[hostId]/source-control/[worktreeId]', @@ -133,6 +135,33 @@ describe('in-page hops between page routes', () => { expect(preview.grants.filter((grant) => !explorer.grants.includes(grant))).toEqual([]) }) + it('keeps the file hops local from the two routes whose rows open them', () => { + // The other half of the four rows that left the list above. Asserted as coverage rather than as + // their absence: an unregistered route is absent too, and a worktree row opening a file is the + // hop a phone actually makes. + const grantsOf = (pathname) => { + const route = MOBILE_WEB_PAGE_ROUTES.find((entry) => entry.pathname === pathname) + if (!route) { + throw new Error(`${pathname} is not registered`) + } + return route.grants + } + const explorer = grantsOf('/h/[hostId]/files/[worktreeId]') + const preview = grantsOf('/h/[hostId]/files/preview/[worktreeId]') + expect(explorer.length, 'the explorer declares something to cover').toBeGreaterThan(0) + for (const opener of ['/h/[hostId]', '/h/[hostId]/agent-history/[worktreeId]']) { + const held = grantsOf(opener) + expect( + explorer.filter((grant) => !held.includes(grant)), + opener + ).toEqual([]) + expect( + preview.filter((grant) => !held.includes(grant)), + opener + ).toEqual([]) + } + }) + it('keeps every hop out of the session local, which is the other half of its seven rows', () => { // Asserted as grant coverage rather than as the absence of seven rows: absent is also what an // unregistered route looks like, and a `session -> tasks` handoff would read the same either diff --git a/config/scripts/mobile-web-page-routes.mjs b/config/scripts/mobile-web-page-routes.mjs index b81ac8d36fe..37437a5454f 100644 --- a/config/scripts/mobile-web-page-routes.mjs +++ b/config/scripts/mobile-web-page-routes.mjs @@ -21,13 +21,17 @@ export const MOBILE_WEB_PAGE_ROUTES = [ // The worktree list. `navigate` because every row opens a session screen that is still native. // `storage` because its pins and its last-visited repo are the app's, not the document's. - { pathname: '/h/[hostId]', grants: ['navigate', 'storage', 'haptics'] }, + // `externalLink` for the one opener the census finds in this closure: `app/h/_layout.tsx` wraps + // every `/h` route in `HostProtocolGate`, so without the grant the wall's Update Orca tap posts a + // notify the shell refuses, with nothing on screen to say why. + { pathname: '/h/[hostId]', grants: ['navigate', 'storage', 'externalLink', 'haptics'] }, // Agent session history. `navigate` because a resumed session opens the session screen, which is // native, and because the list above now reaches this one without leaving the page. `storage` // because the host layout above every page route reads the app's own sidebar width. + // `externalLink` for that same layout's wall, which is this route's only opener too. { pathname: '/h/[hostId]/agent-history/[worktreeId]', - grants: ['navigate', 'storage', 'haptics'] + grants: ['navigate', 'storage', 'externalLink', 'haptics'] }, // Tasks. `navigate` for the session screens its rows open and for the Back that pops the native // stack; `storage` for the shared components it renders; `externalLink` for the provider links @@ -40,20 +44,16 @@ export const MOBILE_WEB_PAGE_ROUTES = [ // The file explorer. `navigate` because its Back pops the native stack. `storage` for the shared // 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 and this list covers what it declares, the handoff keeps that push inside this document. + // `externalLink` is the layout wall's here, the single opener the census finds in this closure: + // nothing the explorer itself renders opens a URL. It is also inherited, and that is what makes + // the hop below cheap: a row opens the preview, and because that is a page 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 - // a wide layout — and every `/h` route reaches both, `/h/[hostId]` included, which declares no - // `externalLink`. That tablet tap stays dead on all of them: a pre-existing gap this route - // neither widens nor fixes. - // // 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 @@ -62,11 +62,10 @@ export const MOBILE_WEB_PAGE_ROUTES = [ pathname: '/h/[hostId]/files/[worktreeId]', grants: ['navigate', 'storage', 'externalLink', 'haptics'] }, - // The file preview. Same three. `externalLink` is this route's own rather than inherited: a - // Markdown preview renders links and `MobileMarkdown` opens them through the platform seam, which - // is a consumer inside the domain rather than the shared wall. The explorer declares the same - // list only because it can become this route in-page, so the two happen to be equal today and - // the reasons are not. + // The file preview. Same three. `externalLink` has a consumer inside the domain as well as the + // shared wall every `/h` route carries: a Markdown preview renders links and `MobileMarkdown` + // opens them through the platform seam. That second site is what the census finds here and not in + // the explorer, which declares the same list for the wall and for the hop into this route. { pathname: '/h/[hostId]/files/preview/[worktreeId]', grants: ['navigate', 'storage', 'externalLink', 'haptics'] diff --git a/config/scripts/mobile-web-page-routes.test.mjs b/config/scripts/mobile-web-page-routes.test.mjs index 0cdfe79509c..9d8df651a7e 100644 --- a/config/scripts/mobile-web-page-routes.test.mjs +++ b/config/scripts/mobile-web-page-routes.test.mjs @@ -46,10 +46,10 @@ async function withScratch(run) { * the route tree resolves to, and what the built manifest actually carries. */ const EXPECTED_PAGE_ROUTES = [ - { pathname: '/h/[hostId]', grants: ['navigate', 'storage', 'haptics'] }, + { pathname: '/h/[hostId]', grants: ['navigate', 'storage', 'externalLink', 'haptics'] }, { pathname: '/h/[hostId]/agent-history/[worktreeId]', - grants: ['navigate', 'storage', 'haptics'] + grants: ['navigate', 'storage', 'externalLink', 'haptics'] }, { pathname: '/h/[hostId]/tasks',