mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
fix(mobile): record a route key only once a frame carried it (OTA phase C, C7.7 round 2)
CodeRabbit on `MobileWebShellScreen.tsx:271`. The effect recorded the route's key and then published, so a publish the hook refused for having no host was remembered as though it had gone out. `publishRoute` is now keyed on everything the host is built from rather than on the session alone, so the render that brings the host re-runs the effect, and the key is written only after a frame has left. Reported honestly: this does not repair a lost tap, and the case beside it says so. The host is built from the route the render holds, so a route that moved before it existed rides the first `init` either way and `publishRoute` then answers "did not move". What the change removes is a key recorded for a frame nobody sent -- the same contract finding 2 fixed on the `ready` path. The case pins the delivery count across the gap: nothing reported while there is no host, nothing reported once there is one and it has sent nothing, and exactly one report when the `init` answering the page's ask carries the route. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
This commit is contained in:
@@ -434,6 +434,58 @@ describe('the hybrid shell screen', () => {
|
||||
expect(dependencies.storageRefreshes).toBe(2)
|
||||
})
|
||||
|
||||
/**
|
||||
* A route that moved before the host existed (CodeRabbit on `:271`).
|
||||
*
|
||||
* The effect recorded the route's key and then published, so a publish the hook refused for
|
||||
* having no host counted as delivered anyway. This pins the delivery contract across that gap:
|
||||
* the caller is told once, and only when a frame carrying the route actually reached the page.
|
||||
*
|
||||
* It does not reproduce a lost tap, and the fix beside it is hygiene rather than a repair: the
|
||||
* host is built from the route the render holds, so a route that moved before it existed is in
|
||||
* the first `init` regardless, and `publishRoute` then answers "did not move". What the fix
|
||||
* removes is a key recorded for a frame nobody sent.
|
||||
*/
|
||||
it('reports a route that moved before the host existed once, when the page receives it', async () => {
|
||||
dependencies.client = null
|
||||
const delivered: { pathname: string; params?: Record<string, string> }[] = []
|
||||
const screen = (params: Record<string, string>) =>
|
||||
createElement(MobileWebShellScreen, {
|
||||
hostId: 'host-1',
|
||||
route: { pathname: '/h/host-1', params },
|
||||
fallback: createElement(NativeFallback),
|
||||
onRouteDelivered: (route) => delivered.push(route)
|
||||
})
|
||||
dependencies.state = readyState('session-one')
|
||||
const rendered: { tree: ReactTestRenderer | null } = { tree: null }
|
||||
await act(async () => {
|
||||
rendered.tree = create(screen({ paneKey: '' }))
|
||||
})
|
||||
const tree = rendered.tree
|
||||
if (tree === null) {
|
||||
throw new Error('screen did not render')
|
||||
}
|
||||
mounted.push(tree)
|
||||
// The tap, with no host to take it: nothing reached the page, so nothing is reported.
|
||||
await act(async () => {
|
||||
tree.update(screen({ paneKey: 'pane-1' }))
|
||||
})
|
||||
expect(delivered).toEqual([])
|
||||
dependencies.client = createFakeRpcClient()
|
||||
await act(async () => {
|
||||
tree.update(screen({ paneKey: 'pane-1' }))
|
||||
})
|
||||
// Still nothing: the host now holds that route and has not sent anything yet.
|
||||
expect(delivered).toEqual([])
|
||||
await act(async () => {
|
||||
byName(tree, 'ShellViewProbe')[0].props.onBridgeMessage({
|
||||
nativeEvent: { json: clientFrame({ type: 'ready' }) }
|
||||
})
|
||||
})
|
||||
// The `init` that answered the ask carried it, so the caller may spend the param — once.
|
||||
expect(delivered).toEqual([{ pathname: '/h/host-1', params: { paneKey: 'pane-1' } }])
|
||||
})
|
||||
|
||||
it('ends that wait on the page asking for a session', async () => {
|
||||
dependencies.client = createFakeRpcClient()
|
||||
const tree = await render(readyState('session-one'))
|
||||
|
||||
@@ -273,10 +273,15 @@ export function MobileWebShellScreen({
|
||||
if (key === publishedRouteKey.current) {
|
||||
return
|
||||
}
|
||||
publishedRouteKey.current = key
|
||||
if (publishRoute(route)) {
|
||||
onRouteDelivered?.(route)
|
||||
// Recorded only once a frame has gone out. A publish the hook refuses — no host yet, which is
|
||||
// the gap between a ready session and its mounted bridge — leaves the key unrecorded, so the
|
||||
// render that brings the host publishes the route the page never received. `publishRoute`'s
|
||||
// identity changes with the inputs the host is built from, which is what re-runs this.
|
||||
if (!publishRoute(route)) {
|
||||
return
|
||||
}
|
||||
publishedRouteKey.current = key
|
||||
onRouteDelivered?.(route)
|
||||
}, [onRouteDelivered, publishRoute, route])
|
||||
|
||||
// A profile read that rejected never becomes a host, so the session would otherwise sit in
|
||||
|
||||
@@ -255,6 +255,11 @@ export function useMobileWebShellBridge(args: {
|
||||
),
|
||||
// Fenced on the session the same way inbound frames are: a host left over from a session this
|
||||
// render has moved past must not be handed this one's route.
|
||||
//
|
||||
// Keyed on everything the host is built from, not on the session alone: a caller that holds a
|
||||
// route the host was not there to take retries when this identity changes, and the host's own
|
||||
// effect is a layout effect, so by the time a passive effect sees the new identity the host
|
||||
// behind it exists.
|
||||
publishRoute: useCallback(
|
||||
(route: BridgeInitRoute) => {
|
||||
const mounted = hostRef.current
|
||||
@@ -262,7 +267,7 @@ export function useMobileWebShellBridge(args: {
|
||||
mounted !== null && mounted.sessionId === sessionId && mounted.host.publishRoute(route)
|
||||
)
|
||||
},
|
||||
[sessionId]
|
||||
[buildId, client, sessionId, snapshot]
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user