Fix Android notification deeplinks to target agent chat#1450
Conversation
Keep agent notification deeplinks alive while the host connection is still coming online, and clear handled Expo notification responses so stale Android launches do not replay an older chat target.
|
| Filename | Overview |
|---|---|
| packages/app/src/app/_layout.tsx | Switches cold-start notification handling from async to synchronous getLastNotificationResponse() and adds clearLastNotificationResponse() after consuming a response; change is clean and well-scoped. |
| packages/app/src/app/h/[serverId]/agent/[agentId].tsx | Adds connection-grace-period logic with a timer effect and a state variable; contains a redundant reset effect that adds noise without contributing behavior. |
| packages/app/src/app/h/[serverId]/agent/agent-ready-route-state.ts | New module extracting the fallback-decision predicate into a testable pure function; logic is correct and the extraction is justified by the focused unit tests. |
| packages/app/src/app/h/[serverId]/agent/agent-ready-route-state.test.ts | Four targeted tests cover the main branches; missing a test for the case where agentCwd is known, workspaces have hydrated, the connection has timed out, and fallback should trigger. |
Sequence Diagram
sequenceDiagram
participant OS as Android OS
participant Layout as PushNotificationRouter
participant Route as HostAgentReadyRoute
participant Store as SessionStore
participant Runtime as HostRuntime
OS->>Layout: Cold-start (notification tap)
Layout->>Layout: getLastNotificationResponse() [sync]
Layout->>Layout: openFromResponse(response)
Layout->>Layout: clearLastNotificationResponse()
Layout->>Route: navigate /h/:serverId/agent/:agentId
Route->>Runtime: useHostRuntimeConnectionStatus(serverId)
Runtime-->>Route: connecting (not yet online)
Route->>Route: startTimer(5000ms)
alt Connection comes online within 5s
Runtime-->>Route: "status = online"
Route->>Route: clearTimeout, setConnectionFallbackReady(false)
Route->>Runtime: fetchAgent(agentId)
Runtime-->>Route: agent.cwd
Store-->>Route: resolvedWorkspaceId
Route->>Route: navigateToPreparedWorkspaceTab
else Timer fires AND agentCwd known AND workspaces pending
Route->>Route: shouldFallbackHostAgentReadyRoute → false
Store-->>Route: "hasHydratedWorkspaces = true"
Route->>Route: shouldFallbackHostAgentReadyRoute → true
Route->>Route: router.replace(hostRootRoute)
else Timer fires AND no agentCwd
Route->>Route: shouldFallbackHostAgentReadyRoute → true
Route->>Route: router.replace(hostRootRoute)
end
Reviews (1): Last reviewed commit: "Fix Android notification deeplink routin..." | Re-trigger Greptile
| useEffect(() => { | ||
| setConnectionFallbackReady(false); | ||
| }, [agentId, serverId]); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
Redundant reset effect — Effect 2 (lines 52-68) unconditionally calls
setConnectionFallbackReady(false) as its very first operation before starting the timer, so this separate effect adds no behavior. The only gap where it could matter is when Effect 2 exits early via !agentId || !serverId || redirectedRef.current, but those conditions imply the component is either parameterless or already navigated, making the fallback state irrelevant in either case.
| useEffect(() => { | |
| setConnectionFallbackReady(false); | |
| }, [agentId, serverId]); | |
| useEffect(() => { | |
| useEffect(() => { |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| }); | ||
|
|
||
| it("does not fall back while a known agent cwd waits for workspace hydration", () => { | ||
| expect( | ||
| shouldFallbackHostAgentReadyRoute({ | ||
| agentCwd: "/repo/project", | ||
| hasHydratedWorkspaces: false, | ||
| hasClient: false, | ||
| isConnected: false, | ||
| connectionFallbackReady: true, |
There was a problem hiding this comment.
Missing coverage for the "fallback triggers after workspace hydrates" path
Test 3 verifies that a known agentCwd with hasHydratedWorkspaces: false suppresses fallback. The symmetric case — agentCwd set, hasHydratedWorkspaces: true, hasClient: false, isConnected: false, connectionFallbackReady: true — is untested. This is exactly the scenario the PR calls out: workspace data has arrived but the host connection timed out. Without a test here, accidentally widening the first guard (e.g. removing the !input.hasHydratedWorkspaces check) would silently flip the behavior of this path.
Summary
Fix Android notification taps so they reliably land in the intended agent chat instead of falling back to the host start page or replaying an older chat target.
/h/:serverId/agent/:agentIddeeplinks alive while the host connection is still coming onlineRoot Cause Analysis
There are two related failure modes in the native notification launch path.
1. Cold-start notification taps could be abandoned before the host socket came online
PushNotificationRouterreads the notification response and callsnavigateToAgent({ serverId, agentId, ... }). On a cold Android start, session/workspace state often is not hydrated yet, sonavigateToAgentfalls back to the intermediate route/h/:serverId/agent/:agentId.That route previously treated
!client || !isConnectedas a terminal failure. On its first effect pass it immediately didrouter.replace(buildHostRootRoute(serverId))and setredirectedRef.current = true. If Android launched from a notification before the host runtime reachedonline, the original agent-chat intent was discarded and never retried. The visible result was a notification tap landing on the host start page instead of the target chat.2. Handled notification responses could be replayed on later native mounts
The native notification effect read
Notifications.getLastNotificationResponseAsync()on mount and routed from it, but never cleared the response after handling it. Expo documentsclearLastNotificationResponse()for the exact case where an app selects a route from a notification response and should not continue selecting that route after it has already been handled:https://docs.expo.dev/versions/v54.0.0/sdk/notifications/#notificationsclearlastnotificationresponse
On Android, this can present as a later launch replaying an older notification response, sending the user to the wrong previous chat instead of their current/expected position.
Fix Details
HostAgentReadyRoutenow tracks a short connection grace period before falling back to host root.shouldFallbackHostAgentReadyRoute()and covered with focused tests.agentCwdbut workspace hydration is still pending, it continues to wait instead of abandoning the deeplink.fetchAgent()resolution path can run and route to the prepared workspace tab for the target agent.getLastNotificationResponse()API and callsclearLastNotificationResponse()immediately after consuming a response.Testing
npm cinpm run build:app-depsnpm run test --workspace=@getpaseo/app -- agent-ready-route-state.test.tsnpm run typecheck --workspace=@getpaseo/appnpm run lint -- "packages/app/src/app/_layout.tsx" "packages/app/src/app/h/[serverId]/agent/[agentId].tsx" "packages/app/src/app/h/[serverId]/agent/agent-ready-route-state.ts" "packages/app/src/app/h/[serverId]/agent/agent-ready-route-state.test.ts"npm run format:check:files -- "packages/app/src/app/_layout.tsx" "packages/app/src/app/h/[serverId]/agent/[agentId].tsx" "packages/app/src/app/h/[serverId]/agent/agent-ready-route-state.ts" "packages/app/src/app/h/[serverId]/agent/agent-ready-route-state.test.ts"Manual Android Repro To Validate