ReviewOS

stacks/bench-review

mobile: benchreview:// is registered but does not route the WebView

#94
Closed glennmichael123 opened this 9 days ago ·
9 days ago

Found by the new iOS e2e suite (#81).

What

config/mobile.ts declares:

urlSchemes: ['benchreview'],

iOS registers the scheme and hands the URL to the app — but the native shell does nothing with it. benchreview://judges leaves the WebView on whatever page it was already showing.

Verified on an iPhone 17 Pro simulator against a bundled (MOBILE_E2E=1) build: the app stayed on the coming-soon page through the full 30s wait. The screenshot is in the run output under storage/framework/runtime/e2e/ios/tests/.

Why it matters

It is the same shape as #80 — an entitlement declared with nothing on the other side of it — and it fails the same way, silently. Nothing logs, nothing errors; the link simply does not go anywhere.

It also blocked the offline flow, which originally reached its second page via the scheme. That flow navigates through the menu now, so the offline test fails only for offline reasons.

Reproduce

bun run test:e2e:ios -- --flow=deep-link --include-pending

The flow is kept as .maestro/flows/pending-03-deep-link.yaml rather than deleted — it is the evidence. The runner skips pending-* by default so a red suite always means a regression rather than a known bug.

Likely shape of the fix

The shell needs to map an incoming URL to a WebView navigation: strip the scheme and load the remaining path against the bundled site or the configured server. Worth checking whether this is bench's to fix or belongs in the Stacks mobile shell — the generated Sources are framework-owned, same as the PRODUCT_NAME gap #81 ran into.

8 days ago

Fixed by the native conversion (#79), verified 84ab374b

Classified before touching anything: framework (craft-native), not bench — and already resolved in the version bench builds today.

Why it looked broken

The repro was run against a web build. There, DeepLinkManager had one destination: dispatch the URL into the page. The shell never mapped a path to a screen because there were no screens.

Under renderer: 'native' the shell takes a different road, and it is all present in the generated sources:

DeepLinkManager.handleURL(url)
  └─ CraftHybrid.handleDeepLink(url)
       ├─ CraftHybridRoutes.appPath(of: url)   // benchreview://judges/7 → /judges/7
       ├─ routes.match(path)                   // against nativeScreens
       └─ controller.showNative(path:, fromLink: true)

appPath(of:) is the piece that makes a custom scheme work at all: for a non-http scheme it promotes the host into the first path segment, so benchreview://judges is /judges rather than an empty path. And when no controller exists yet — a link that launches the app — an all-native app appends to untilShell and replays once the shell is up, which is what makes the cold case work.

Verified on a simulator

Dedicated device, against the native build. All four shapes:

CaseLinkResult
Cold (app not running)benchreview://judges/7launches and lands on the judge screen
Cold, never-visited judgebenchreview://judges/12lands on that judge — rules out iOS state restoration
Two segmentsbenchreview://judges/1/educationeducation screen, not swallowed by /judges/:id
Another tab's stackbenchreview://court-houses/1courthouse screen
Negative controlbenchreview://judges-nope/7no detail screen — the assertions are not vacuous

Two things worth knowing

iOS asks before handing over a scheme link from the home screen ("Open in Bench Review?"), and does not ask when the app is already frontmost. The flow handles it conditionally; it is not a bug.

A scheme link to a path no native screen claims silently lands on the start screen. It does not open the web: isSite() requires http/https, so the browse fallback cannot fire for a custom scheme. A universal link to the same path would open it in the browser instead. This matters for #80, whose association file claims /article/* — a path with no native screen — so the two link types diverge there.

What replaced the pending flow

.maestro/flows/pending-03-deep-link.yaml → .maestro/flows/03-deep-link.yaml, unskipped. Assertions are about shape (a detail screen is on top, a two-segment path reaches its own screen) rather than about which judge, so the flow survives a data change. Each case starts from stopApp, because a pushed screen leaves the one below it in the accessibility tree and assertNotVisible would otherwise be measuring the previous step.

Universal links remain unverifiable locally — signed non-personal team plus the domain serving the association file (#80, blocked on #85).

Sign in to comment on this issue.