A tapped notification opens the session it is about - #89
Merged
Conversation
A sealed push carried projectId as its only routing id, and projectId is sha256(realpath(folder)) with no machine input — two machines holding the same repo at the same path mint the identical id. A phone addresses a project as <machineUuid>.<projectId>, so a tap had no machine to open. composePush now returns the routing ids alongside the strings, because it is the only per-message-type switch with Zod-narrowed access to escalationId and sessionId. The dispatcher seals machineUuid from a new required ProjectCoreRemoteDeps.machineDeviceId(), and terminalId when the message names a session — omitted rather than emitted empty, since the phone treats a present key as a session it can resolve. machineDeviceId is required, not optional: optional would let the wizard-promotion supplier ship unroutable pushes and still compile. Its two suppliers differ in how well they can answer — host-server reads the live machine socket, relay-promotion can only report the uuid the enabling agent:enableRelay carried. The title is capped alongside the body: the relay rejects an oversized box outright and answers no push:result, so an uncapped title loses the whole notification rather than truncating it.
…at it The three fan-in notification providers merge every warm project's stream into one, then threw away the only thing that said whose it was. HandlerEscalation and TerminalNotificationMessage name no project themselves, and a consumer that falls back to the FOCUSED project is wrong for exactly the case this fan-out exists to serve — a background project's agent. Each stream now carries ProjectScoped<T> = (entryId, message), the entryId being the registry key already in its correct local-or-remote shape. The surfacers take it through to the toast layer. Dedup keys are unchanged and deliberately NOT the scoped record: keying on the escalationId alone is what keeps the provider's per-rebuild re-seed idempotent, and what lets one escalation arriving both live and via push surface once.
A notification named a session and then left the user to find it by hand. The routing information was already there — the fan-in streams carry the entryId, an escalation carries its terminalId — and it was discarded at the last step. NotificationRoute is the wire shape; resolveNotificationRoute turns it into a place, and refuses rather than guesses. There is deliberately no projectId-only fallback: computeProjectId hashes the folder path with no machine input, so two machines holding the same repo at the same path mint the identical id and that match can name the wrong one. An old bridge that sealed no machineUuid is unroutable, not resolvable. applyNotificationRoute is the one idempotent applier, taking a ProviderContainer because the row that raised the toast is disposed by the switch it triggers. It never calls revealHandlerTab or switchToAgentPage after a focus write — the per-session UI restore re-applies the target's own saved tab a frame later and would silently undo them — and hands over through the pending providers instead. It never writes pendingActiveSessionId for the already-focused project, where nothing would drain it and its presence stops reconcileActiveSession falling back. Its dedup store is separate from the notification surfacer's and namespaced: sharing that set would make every tappable notification's id already-present and the tap a permanent no-op. A claim is released on every non-success exit, a throw included, so a tap that could not land stays retryable. A notification tap means show me what happened, not resume this, so it suppresses the pending id's auto-start. The suppressor holds the id it names rather than a flag: five other sites queue a pending id without knowing it exists, and a bare flag surviving a bootstrap's early return would eat an unrelated Recent-list tap's resume. Both drains hold a stamp back while a queued session id is unresolved. The applier stamps after activating the project, while the bootstrap is still fetching the new project's list, so spending it then would reveal a transcript that is not the one asked for — and the restore that resolving it arms would hide the panel again a frame later with the request already gone.
The four OS tap entry points — a warm fln tap, an iOS push tap, and each of their cold-start reads — become thin adapters over the applier W3 already built. Each decodes a route and hands it to one funnel in main; none of them knows anything about navigation. The path that mattered most was the one with no adapter at all: pushBackgroundHandler renders EVERY Android background push, because the FCM message is data-only, and it was showing them with no payload. A payload is the only channel a route rides on, so without it no Android notification was tappable-to-route — the headline case of the whole change. A route is sealed into the OS notification whenever the producer named one, which is deliberately a weaker gate than the in-app toast's. The toast chip lives 8s, so resolving at show time is as good as resolving at press time; a notification sits in the shade indefinitely and the applier re-resolves against freshly awaited state, so gating it here would bake a cold-cache miss into a notification that would have opened fine an hour later. The cold-start read now waits for endOfFrame first. runApp only SCHEDULES the first frame, and the applier's cross-project path pushes on the root navigator; reading before that navigator attaches spends the route on nothing with no retry behind it. Two platform gates are extracted as pure predicates so they are no longer deletable with every gate green: the push tap is iOS-only (on Android PushPlugin fires it for any launch intent whose extras decode, and fln's own select-notification intent carries a payload extra, so it would deliver every Android tap twice), and launch details are read only where fln implements them (Linux throws, Windows replays a tap it already delivered). Not covered by any test, and stated rather than implied: the four registrations themselves, the _nextId isolate seed, and show(payload:) reaching the platform channel all need a device.
… spent
A max-effort review of the four-wave notification-tap work found eight real defects. The through-line is a route that survives further than it can be honoured.
The pending-id guard held two surface drains back but nothing watched the provider that clears them, so the only retry notified nobody: reconcileActiveSession selects the queued id off the persisted cache, _bootstrapSessions re-sets the same value, and Riverpod skips on ==. build() now watches it.
_applyAcrossProjects restored the prior pending ids unconditionally on failure, writing back a value another project's bootstrap had already consumed; each restore is now conditional on our own write still standing. nav_controller's project-switch branch clears the start suppressor alongside the id it qualifies, so it cannot re-pair with a later Recent-list tap and eat the start that tap is.
decodeNotificationRoute returned a non-null route for {}, spending the applier's unconditional side effects (exitDemoMode above all) on a tap that then resolves nowhere; a route naming nothing is now no route. namedOrNull returns the trimmed value rather than testing trim() and returning the padding, and is shared with the push decoder so the two cannot accept different ids -- the sealed-push title and body go through it too, since the casts they were are the only thing in that try that could throw on payload content, and a throw there costs the whole alert.
The focused-project path never sent session:focus, so the bridge still believed the client was on the previously selected session and the session now on screen kept its unread dot. ensureMachineRelay returned the inbound auth's deviceUuid as agentDeviceId where every sibling delegates to the live socket, so a re-enable over a running socket sealed pushes naming a machine the relay does not have this host on.
Cleanup: the toast chip's resolve moved inside its own branch (it was computed and discarded on the backgrounded path, which is the common one), an unreachable mobile guard in _revealAgentPanel deleted, and push-dispatcher's sourceMessageId re-pinned to msg.id rather than any string -- that equality is what the app dedups on. bridge/CLAUDE.md records machineDeviceId, added to the remote-deps contract by W1.
Left alone, as design decisions rather than repairs: the pending stamp surviving an interactive cross-project bootstrap, cross-isolate push dedup, the focused-project start suppressor, service-PTY notification ids, and the absent retry/failure surface for a route that cannot be applied.
Gates: flutter analyze clean, flutter test 3356 pass, bridge 3335 tests 0 fail, check:font-tokens OK.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A notification told you something happened somewhere else and then left you to
find it yourself. This makes every one of them a way to get there — the OS
notification, the in-app toast, and a cold start from a terminated app.
Four waves, one per commit, each gated and committed on its own.
What each wave does
a6748d36— the push learns which machine it came from. A push carried aprojectIdand nothing else, andcomputeProjectIdissha256(realpath)withno machine input — so the same repo checked out at the same path on two machines
mints one id. The phone could not tell them apart, and there is deliberately no
projectId-only fallback anywhere downstream because of it. The dispatcher now
seals
machineUuidandterminalIdalongside;machineDeviceIdis a requiredmember of
ProjectCoreRemoteDepsso a new supplier cannot forget it.83cedb2b— a notification says which project it is about. The threefan-out providers carry the
entryIdattributing each notification to itsproject, which is what lets a toast point at something.
afc87fde— the route itself. A pureNotificationRoute, a resolver thatturns one into a
NavLocationagainst the known sessions, and an idempotentapplier. Plus the Tier-1 toast action: a chip that opens what the toast is
about.
ea99bc30— the four OS tap entry points, as thin adapters over the above:warm tap, cold-start launch details, the push tap callback, and the in-app path.
0f2d44a0— max-effort review fixes. Eight real defects, below.Two things worth knowing before reading the diff
On Android,
pushBackgroundHandlerrenders every user-visible backgroundpush. The FCM message is data-only (
relay/src/push/fcm.ts), so that handleris not one path among several — it is the path. It was forwarding no payload,
which meant not one Android notification was tappable-to-route. Nothing in the
suite would have noticed; there is now a round-trip test that would.
routeOfTapResponseinspects only the payload, deliberately. On Windows aplain body tap arrives as
selectedNotificationAction, because the pluginclassifies on whether the toast's
launchargument is non-null and our payloadis that argument; the same path fills
actionIdwith the payload verbatim.Filtering on the response type would drop every Windows tap, and treating a
non-null
actionIdas a button press would misroute every one of them.A related claim in the planning material is false, and should not be carried
forward: a payload is not what enables the iOS tap callback. The Dart side
sends
'payload': payload ?? ''andbuildUserDictstores it unconditionally,so
isAFlutterLocalNotificationalways sees at least@"". Null payloads arestill not interchangeable with empty ones — but the reason is the Windows
classification above, not iOS.
What the review found
build()watchedneither provider that clears them — and there is a real path with no
incidental rebuild behind it, so the retry notified nobody and the handler tab
a user tapped for never appeared.
_applyAcrossProjectsrestored the prior pending ids unconditionally onfailure, able to write back a value another project's bootstrap had already
consumed — permanently unspendable afterwards.
decodeNotificationRoutereturned a non-null route for{}, spending theapplier's unconditional side effects (
exitDemoModeabove all) on a tap thatthen resolved to nothing.
namedOrNulltestedtrim()and returned the padding, so" <uuid>"passedas naming something and was then used verbatim.
ensureMachineRelayused the inbound auth'sdeviceUuidwhere every siblingmember delegates to the live socket, so a re-enable over a running socket
sealed pushes naming a machine the relay does not have this host on.
session:focus, so the session now onscreen kept the unread dot the user was reading.
path, and
push-dispatcher'ssourceMessageIdwas pinned toany Stringrather than
msg.id— that exact equality is what the app dedups on.Deliberately not done
Each is a design decision rather than a repair, and none is a regression:
bootstrap; deciding when a
_selectViewcall counts as user intent needs arule the per-session UI restore also has to satisfy.
(in-band and via FCM); deduping needs cross-isolate state, and today tapping
the leftover twin foregrounds the app and does nothing visible.
notification for a finished session in the focused project can relaunch the
agent.
route lands on an empty workspace.
surface — where
openRecentSessionshows a message for the same class ofrefusal.
Testing
flutter analyzeclean ·flutter test3356 pass ·bun run --filter antgrid-bridge test3335 tests, 0 fail ·npm run check:font-tokensOK.Needs a physical device — no automated coverage exists or can: the four tap
registrations themselves, the
_nextIdisolate seed, andshow(payload:)reaching the platform channel. The two platform gates were the cases that could
be deleted while everything stayed green, so they are now pure predicates with
unit tests.