diff --git a/app/lib/main.dart b/app/lib/main.dart index c8980ffc..959f6d56 100644 --- a/app/lib/main.dart +++ b/app/lib/main.dart @@ -2,6 +2,8 @@ import 'dart:async'; import 'package:app_links/app_links.dart'; import 'package:flutter/foundation.dart'; +import 'package:flutter_local_notifications/flutter_local_notifications.dart' + show FlutterLocalNotificationsPlugin; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; @@ -35,6 +37,7 @@ import 'providers/drawer_order.dart'; import 'providers/first_run.dart'; import 'providers/host_status.dart'; import 'providers/local_host_warmup.dart'; +import 'providers/notification_route_apply.dart'; import 'providers/post_signin_provisioning.dart'; import 'providers/projects.dart'; import 'providers/provider_retry.dart'; @@ -43,6 +46,7 @@ import 'providers/recent_agents.dart'; import 'navigation/nav_console.dart'; import 'navigation/nav_controller.dart'; import 'navigation/nav_serialization.dart'; +import 'navigation/notification_route.dart'; import 'navigation/platform_route_guard.dart'; import 'navigation/root_navigator.dart'; import 'screens/app_shell.dart'; @@ -51,7 +55,10 @@ import 'screens/device_cap_dialog.dart'; import 'screens/sign_in_screen.dart'; import 'services/devices_api.dart' show DeviceCapInfo; import 'services/app_settings_service.dart'; +import 'services/local_notification_service.dart'; +import 'services/notification_tap.dart'; import 'services/push_background_handler.dart'; +import 'services/push_identity.dart'; import 'providers/update_available.dart'; import 'storage/cached_sessions_store.dart'; import 'storage/drawer_collapsed_store.dart'; @@ -62,6 +69,7 @@ import 'storage/recent_agents_store.dart'; import 'storage/update_handoff_store.dart'; import 'update/update_gate.dart'; import 'util/ab_log.dart'; +import 'util/detached.dart'; import 'widgets/auth_splash.dart'; import 'widgets/demo_frame.dart'; import 'window/window_chrome.dart'; @@ -310,6 +318,42 @@ Future main() async { // Subscribe for in-flight links while the app is running. appLinks.uriLinkStream.listen((uri) => unawaited(handleLink(uri))); + // The one place a tapped notification is turned into a navigation, whichever + // surface delivered it. No context: an OS-level tap has no widget behind it, + // so the applier falls back to the root navigator. + Future handleNotificationRoute(NotificationRoute? route) async { + if (route == null) return; + await applyNotificationRoute(null, container, route); + } + + // Unconditional and platform-independent — a desktop tap is worth as much as + // a phone one. Registering it is not evidence a tap can ever arrive: fln + // installs its method-call handler only inside `initialize`, which the demo's + // shell mount deliberately skips, so an install that has only ever shown the + // sample project has no warm-tap channel at all. Nothing is logged here. + LocalNotificationService().onTap = (response) => detached( + 'Main', + 'notification tap failed', + () => handleNotificationRoute(routeOfTapResponse(response)), + ); + + // iOS ONLY — see [pushTapRegistrationSupported] for why the narrowing is the + // substance. Even on iOS this callback is not ours alone: push forwards fln's + // local-notification userInfo down it with no source filter, which is why a + // map that decodes to nothing returns quietly rather than logging. + if (pushTapRegistrationSupported(defaultTargetPlatform)) { + Push.instance.addOnNotificationTap( + (data) => detached('Main', 'push notification tap failed', () async { + final decoded = await decodePush( + pushDataMap(data), + pushIdentity: PushIdentity.secure(), + ); + if (decoded == null) return; + await handleNotificationRoute(routeOfPush(decoded)); + }), + ); + } + analytics.track(AnalyticsEvents.appActive); WidgetsBinding.instance.addObserver( _TelemetryLifecycleObserver(onPause: analytics.flush), @@ -331,6 +375,53 @@ Future main() async { // half-open connection. It's best-effort and invalidates currentUserProvider // when it completes, so the UI updates as soon as the session lands. unawaited(appLinks.getInitialLink().then(handleLink)); + + // The notification that launched a terminated app. Each source is read + // EXACTLY once, because none of them is drained by the read: push's + // terminated-tap value is a static it never clears, fln's iOS + // launch-response dict is never emptied, and Android's `onNewIntent` re-arms + // the launch intent — so a later re-read is a guaranteed duplicate rather + // than a retry. Both iOS sources are read, because either can be the one that + // fired (push's skip guard keys on a launch option an fln-launched start + // never sets) and the applier's value-dedup absorbs the overlap. + detached('Main', 'cold-start notification tap failed', () async { + // `runApp` only SCHEDULES the first frame, so the root navigator is not + // attached yet. The applier's cross-project path hands its route to a + // pending slot and then pushes on that navigator; reading before it exists + // spends the route on nothing, with no retry behind it. + await WidgetsBinding.instance.endOfFrame; + // Platform-gated (see [launchDetailsSupported]) AND wrapped: the gate keeps + // the known-broken platforms out, the try keeps an unknown one from taking + // the whole cold-start block — the push read below it included — with it. + if (launchDetailsSupported(defaultTargetPlatform)) { + try { + final details = await FlutterLocalNotificationsPlugin() + .getNotificationAppLaunchDetails(); + // The response is the gate, not `didNotificationLaunchApp`: a route + // needs a payload, and only the response carries one. + final response = details?.notificationResponse; + if (response != null) { + await handleNotificationRoute(routeOfTapResponse(response)); + } + } catch (e) { + AbLog.error( + 'Main', + 'notification launch details failed', + fields: {'error': '$e'}, + ); + } + } + if (defaultTargetPlatform != TargetPlatform.iOS) return; + final tapped = + await Push.instance.notificationTapWhichLaunchedAppFromTerminated; + if (tapped == null) return; + final decoded = await decodePush( + pushDataMap(tapped), + pushIdentity: PushIdentity.secure(), + ); + if (decoded == null) return; + await handleNotificationRoute(routeOfPush(decoded)); + }); } class _TelemetryLifecycleObserver extends WidgetsBindingObserver { diff --git a/app/lib/navigation/nav_controller.dart b/app/lib/navigation/nav_controller.dart index 8f7d4981..c5590927 100644 --- a/app/lib/navigation/nav_controller.dart +++ b/app/lib/navigation/nav_controller.dart @@ -142,6 +142,11 @@ class NavController extends Notifier { // clears any stale pending from an earlier switch, so the new project's // bootstrap can't consume a session id meant for a different project. ref.read(pendingActiveSessionIdProvider.notifier).set(loc.sessionId); + // Cleared with the id it qualifies, never left behind: back()/forward() + // and a deep link all mean "resume this", and a suppressor outliving the + // queue it was written for would silently re-pair with a later + // Recent-list tap on the same session and eat the start that tap IS. + ref.read(pendingSessionStartSuppressedIdProvider.notifier).set(null); } else if (loc.target == currentTarget && loc.sessionId != null) { // Same project: bootstrap won't re-run, so select directly. ref.read(activeSessionIdProvider.notifier).set(loc.sessionId); @@ -183,6 +188,12 @@ class NavController extends Notifier { .set( loc.file == null ? null : (target: pendingTarget, value: loc.file!), ); + // A [NavLocation] can never ASK for the agent transcript — there is no + // WorkspaceView for it — so this is only ever the null half. It belongs + // here all the same: the drains run in one post-frame callback with this + // one last, so a stamp an earlier location left would otherwise override + // the tab a back()/forward()/deep link just named. + ref.read(pendingAgentPageProvider.notifier).set(null); } } diff --git a/app/lib/navigation/notification_route.dart b/app/lib/navigation/notification_route.dart new file mode 100644 index 00000000..803768b9 --- /dev/null +++ b/app/lib/navigation/notification_route.dart @@ -0,0 +1,233 @@ +// app/lib/navigation/notification_route.dart +import 'dart:convert'; + +import 'package:collection/collection.dart'; +import 'package:flutter/foundation.dart'; + +import '../models/recent_session_row.dart'; +import '../models/session_target.dart'; +import '../models/workspace_view.dart'; +import '../providers/ui_attention_providers.dart' show WorkbenchSurface; +import '../util/device_id.dart'; +import 'nav_location.dart'; + +/// What a tapped notification names — the wire shape only. Turning one into a +/// place the app can go is [resolveNotificationRoute], which refuses rather +/// than guesses. +/// +/// Every field is optional because every producer names a different subset: a +/// live stream already holds the drawer entryId, a sealed push carries +/// machine + project + session, and a hook notification may name nothing but a +/// title (`sessionId` is optional on `notification:push`). +@immutable +class NotificationRoute { + /// Pre-resolved drawer entry id, in its local-or-remote shape. Set by the + /// in-app paths, which already know which entry the notification arrived on. + final String? registrationId; + + /// Sealed-push path only, and null from a bridge that predates the widened + /// payload — which is why an absent one is unroutable, never guessed at. + final String? machineUuid; + final String? projectId; + final String? terminalId; + final String? sourceMessageId; + + /// `agent` or `handler`. Anything else is treated as `agent`: an unknown kind + /// from a newer bridge must land the user on the session, not nowhere. + final String? kind; + + const NotificationRoute({ + this.registrationId, + this.machineUuid, + this.projectId, + this.terminalId, + this.sourceMessageId, + this.kind, + }); + + /// Value equality is load-bearing, not a convenience: the applier dedups on + /// it, because [sourceMessageId] is nullable by design and a route without + /// one would otherwise have no dedup key at all. + @override + bool operator ==(Object other) => + identical(this, other) || + other is NotificationRoute && + other.registrationId == registrationId && + other.machineUuid == machineUuid && + other.projectId == projectId && + other.terminalId == terminalId && + other.sourceMessageId == sourceMessageId && + other.kind == kind; + + @override + int get hashCode => Object.hash( + registrationId, + machineUuid, + projectId, + terminalId, + sourceMessageId, + kind, + ); + + @override + String toString() => + 'NotificationRoute(registrationId: $registrationId, ' + 'machineUuid: $machineUuid, projectId: $projectId, ' + 'terminalId: $terminalId, sourceMessageId: $sourceMessageId, ' + 'kind: $kind)'; +} + +/// Serializes [r] for a payload slot that only carries a string — an OS +/// notification's action payload. +/// +/// A null field is OMITTED, never emitted as `""`: the two mean different +/// things downstream (an empty id names no project, but would still satisfy a +/// `!= null` test), so the encoding must not be able to manufacture one. +String encodeNotificationRoute(NotificationRoute r) => jsonEncode({ + if (r.registrationId != null) 'registrationId': r.registrationId, + if (r.machineUuid != null) 'machineUuid': r.machineUuid, + if (r.projectId != null) 'projectId': r.projectId, + if (r.terminalId != null) 'terminalId': r.terminalId, + if (r.sourceMessageId != null) 'sourceMessageId': r.sourceMessageId, + if (r.kind != null) 'kind': r.kind, +}); + +/// Parses a payload back into a route, or null when there is nothing to parse. +/// +/// The payload arrives from the OS notification the user tapped and is applied +/// fire-and-forget, so a throw here would be an unhandled async error: anything +/// that is not a JSON object degrades to null, and a value of the wrong type +/// degrades to an absent field rather than failing the whole route. +NotificationRoute? decodeNotificationRoute(String? payload) { + if (payload == null) return null; + final Object? decoded; + try { + decoded = jsonDecode(payload); + } catch (_) { + return null; + } + if (decoded is! Map) return null; + final route = NotificationRoute( + registrationId: namedOrNull(decoded['registrationId']), + machineUuid: namedOrNull(decoded['machineUuid']), + projectId: namedOrNull(decoded['projectId']), + terminalId: namedOrNull(decoded['terminalId']), + sourceMessageId: namedOrNull(decoded['sourceMessageId']), + kind: namedOrNull(decoded['kind']), + ); + // A payload naming NOTHING is the same answer as no payload, and must reach + // the applier as one: `{}` is a valid JSON object, so without this it would + // arrive as a non-null route and spend the applier's unconditional + // side effects — leaving the demo, above all — on a tap that then resolves + // to no destination at all. + return route == const NotificationRoute() ? null : route; +} + +/// The value of a field that actually names something, or null. +/// +/// Blank and absent collapse deliberately: `LocalProject('')` is a target the +/// app would try to focus, so an empty id must not survive far enough to be +/// tested for null. +/// +/// Public because the sealed-push decoder (`services/push_background_handler. +/// dart`) has to answer identically: an id only one of them accepts is an id +/// that survives decoding to address nothing. One definition is what makes that +/// structural rather than a comment. +String? namedOrNull(Object? value) { + if (value is! String) return null; + final trimmed = value.trim(); + // The TRIMMED value, not the original: testing `trim()` and returning the + // padding would let " " pass as naming something and then be used + // verbatim as a machine id, matching no drawer entry and no cached origin. + return trimmed.isEmpty ? null : trimmed; +} + +/// Resolves [route] to a place, or null when nothing can be addressed without +/// guessing. +/// +/// [known] is the cached-session universe (`recentSessionsProvider`); +/// [localDeviceUuid] is this install's machine uuid, null on a platform that +/// hosts no projects — which is why locality is decided per row against that +/// uuid and never by a platform test. +NavLocation? resolveNotificationRoute( + NotificationRoute route, { + required List known, + required String? localDeviceUuid, +}) { + final target = _resolveTarget( + route, + known: known, + localDeviceUuid: localDeviceUuid, + ); + if (target == null) return null; + return NavLocation( + target: target, + surface: WorkbenchSurface.workspace, + // Carried whenever the route names a session; whether that session is still + // open is the applier's question, not this one's. + sessionId: namedOrNull(route.terminalId), + view: namedOrNull(route.kind) == 'handler' ? WorkspaceView.handler : null, + ); +} + +SessionTarget? _resolveTarget( + NotificationRoute route, { + required List known, + required String? localDeviceUuid, +}) { + final registrationId = namedOrNull(route.registrationId); + if (registrationId != null) { + // A cached row is preferred over splitting the id ourselves because the row + // was classified against the real project list: a bare id belonging to no + // local project is still local, and only the row knows that. + final row = known.firstWhereOrNull( + (r) => r.origin.registrationId == registrationId, + ); + if (row != null) return _targetOf(row.origin); + return _splitRegistrationId(registrationId); + } + + final machineUuid = namedOrNull(route.machineUuid); + final projectId = namedOrNull(route.projectId); + if (machineUuid != null && projectId != null) { + if (machineUuid == localDeviceUuid) return LocalProject(projectId); + return RemoteProject(machineUuid: machineUuid, projectId: projectId); + } + + final terminalId = namedOrNull(route.terminalId); + if (terminalId != null) { + // Session ids are uuids minted per session, so a hit names exactly one + // project — but only if there IS one hit. Distinct registration ids, not + // rows: the same session listed twice under one project is not ambiguous. + final origins = {}; + for (final row in known) { + if (row.session.id != terminalId) continue; + origins[row.origin.registrationId] = row.origin; + } + if (origins.length == 1) return _targetOf(origins.values.first); + return null; + } + + // Deliberately no projectId-only fallback. `computeProjectId` hashes the + // folder path with no machine input, so the same repo checked out at the same + // path on two machines mints the identical id — a "unique" match there is a + // confident wrong machine. Unroutable is the honest answer. + return null; +} + +SessionTarget _targetOf(RecentOrigin origin) { + final machineUuid = origin.machineUuid; + if (origin.isLocal || machineUuid == null) { + return LocalProject(origin.projectId); + } + return RemoteProject(machineUuid: machineUuid, projectId: origin.projectId); +} + +SessionTarget _splitRegistrationId(String registrationId) { + final machineUuid = baseDeviceUuid(registrationId); + if (machineUuid == registrationId) return LocalProject(registrationId); + return RemoteProject( + machineUuid: machineUuid, + projectId: baseProjectId(registrationId), + ); +} diff --git a/app/lib/providers/notification_route_apply.dart b/app/lib/providers/notification_route_apply.dart new file mode 100644 index 00000000..d087c1a0 --- /dev/null +++ b/app/lib/providers/notification_route_apply.dart @@ -0,0 +1,359 @@ +// app/lib/providers/notification_route_apply.dart +import 'dart:collection'; + +import 'package:flutter/widgets.dart' show BuildContext; +import 'package:flutter_riverpod/flutter_riverpod.dart'; + +import '../models/session_target.dart'; +import '../navigation/nav_controller.dart'; +import '../navigation/nav_location.dart'; +import '../navigation/notification_route.dart'; +import '../navigation/root_navigator.dart'; +import '../widgets/drawer_entry_row.dart' show activateDrawerEntryById; +import 'agent_transport.dart'; +import 'demo_mode.dart'; +import 'device_provisioning.dart'; +import 'providers.dart' show focusedServiceOrNull; +import 'recent_sessions.dart'; +import 'sessions.dart'; +import 'ui_attention_providers.dart'; +import 'visible_surface.dart'; + +/// Upper bound on remembered routes. Insertion-ordered eviction, the same shape +/// as WorkspaceShell's notification dedup — this is a duplicate suppressor, not +/// a ledger. +const int _kMaxAppliedRoutes = 128; + +/// Routes this container has already applied. +/// +/// Deliberately NOT WorkspaceShell's `_notifiedIds`: that set has already +/// consumed every id it surfaced by the time the user can tap the toast, so +/// sharing it would make every tappable notification's id already-present and +/// the tap a permanent no-op. Keys are namespaced for the same reason — the two +/// stores must stay unable to answer for each other even if they ever meet. +class _AppliedRoutes { + final LinkedHashSet _keys = LinkedHashSet(); + + /// True while an apply is between its first read and its last write. + /// + /// The dedup only closes IDENTICAL routes, and two different ones overlapping + /// is what breaks: `activateDrawerEntryById` saves and restores a prior target + /// around a ~30s cold open, so the second apply's restore hands back a target + /// the first one had already left — including null, which focuses no project + /// at all. An 8s toast and a stacked second one make that window ordinary. + bool inFlight = false; + + /// Records [route] as applied, or returns false when it already was. + /// + /// Two keys, because neither alone covers the producers: a sealed push and a + /// live stream both carry an id worth trusting, but `sourceMessageId` is + /// nullable by design, and the two iOS cold-start entries can deliver ONE tap + /// twice as two equal-valued routes. + bool claim(NotificationRoute route) { + final keys = _keysOf(route); + if (keys.any(_keys.contains)) return false; + for (final key in keys) { + _keys.add(key); + if (_keys.length > _kMaxAppliedRoutes) _keys.remove(_keys.first); + } + return true; + } + + /// Forgets a claim that never became a navigation. + /// + /// The claim is taken before anything can fail, because it doubles as the + /// in-flight guard — so every path that ends without moving the user has to + /// give it back, a THROW included. A burnt claim is permanent: the toast + /// stays on screen and offering a retry that can only ever return false is + /// worse than no chip. + /// + /// What the retry window actually is: the toast's own duration, and nothing + /// longer. A failure slower than that — the shape the asleep machine makes, + /// where `activateDrawerEntryById`'s cold open can outlive the chip — is + /// refused by [inFlight] rather than retried, because the press that would + /// retry it arrives while the first attempt is still running. The two + /// durations are coupled: lengthening the cold-open bound past the toast, or + /// shortening the toast, moves failures out of the retryable window. + void release(NotificationRoute route) => _keys.removeAll(_keysOf(route)); + + /// The canonical encoding, never `hashCode`: `Object.hash` over six nullable + /// strings collides, and a collision here is the failure this whole store + /// exists to remove — the colliding route's chip becomes a permanently dead + /// no-op, and [release] hands back the other route's claim. + List _keysOf(NotificationRoute route) { + final sourceMessageId = route.sourceMessageId; + return [ + if (sourceMessageId != null) 'route:id:$sourceMessageId', + 'route:value:${encodeNotificationRoute(route)}', + ]; + } +} + +final _appliedRoutesProvider = Provider<_AppliedRoutes>( + (ref) => _AppliedRoutes(), +); + +/// Takes the user to what a tapped notification named. Returns whether the +/// route was applied — false covers both "already applied" and "names nothing +/// this install can address". +/// +/// Takes the [ProviderContainer], never a caller's `WidgetRef`: a cross-project +/// route runs `activateDrawerEntryById`, which can spend ~30s opening a cold +/// remote project and tears down the toast that started it on the way. The +/// [context] is optional for the same reason — an OS-level tap has no widget at +/// all — and falls back to the app's root navigator. +Future applyNotificationRoute( + BuildContext? context, + ProviderContainer ref, + NotificationRoute route, +) async { + final applied = ref.read(_appliedRoutesProvider); + // Before anything starts, not after it resolves: `activateDrawerEntryById` + // saves and restores a prior target around the cold open, so two applies + // overlapping there restore a stale one. The claim is also the in-flight + // guard for two DIFFERENT routes, which the dedup cannot see. + if (applied.inFlight) return false; + if (!applied.claim(route)) return false; + applied.inFlight = true; + try { + return await _apply(context, ref, route); + } catch (_) { + // A throw is a non-success exit like any other, and this one is reachable: + // `localDeviceUuidProvider` is built to REJECT rather than stall on a + // keychain read error. The toast runs this call detached, so the throw is + // only logged — leaving a claim burnt here would make every later press, + // of this route and of every route equal to it, a silent no-op for the + // life of the container. + applied.release(route); + rethrow; + } finally { + applied.inFlight = false; + } +} + +Future _apply( + BuildContext? context, + ProviderContainer ref, + NotificationRoute route, +) async { + // The demo's candidate universes are the sample project alone, so a real + // route resolves to nothing until it is left. Leaving also clears the focused + // target and the nav history, which is why every read below happens after. + // + // Unconditional and deliberate, ahead of every branch that can still return + // false, so a route that turns out to be unroutable has still ended the demo. + // That is the acceptable half of the trade: a notification the user tapped is + // real traffic from a real machine, and the demo is a scope entered and left + // rather than state worth protecting. Deferring it until a branch is known to + // navigate is only blocked for the terminalId-only rule, which resolves out of + // `recentSessionsProvider` and would answer for the sample project — every + // producer today names a registrationId, which resolves without it. + if (ref.read(demoModeProvider)) exitDemoMode(ref); + + // `.value` is null on mobile by design — there is no local host there — so + // the future is what tells locality apart from "not loaded yet". + final localDeviceUuid = await ref.read(localDeviceUuidProvider.future); + + final loc = resolveNotificationRoute( + route, + known: ref.read(recentSessionsProvider), + localDeviceUuid: localDeviceUuid, + ); + // Nothing was addressed, so nothing was spent: the Recent rows a terminalId + // is matched against hydrate asynchronously, and the toast outlives that. + if (loc == null) { + ref.read(_appliedRoutesProvider).release(route); + return false; + } + final target = loc.target!; + + if (target != ref.read(selectedTargetProvider)) { + // The tapped widget is transient — a toast entry, and one this very route + // tears down — so a dead one falls through to the app's single Navigator, + // which outlives every route. + BuildContext? navContext; + if (context != null && context.mounted) navContext = context; + navContext ??= ref.read(rootNavigatorKeyProvider).currentContext; + if (navContext == null) { + // Nothing can dial the project from here, so the queued state below would + // be seeded and taken back in the same breath — over whatever another + // site had already queued. + ref.read(_appliedRoutesProvider).release(route); + return false; + } + // Checked on the line above, or freshly read off the root navigator key; + // `activateDrawerEntryById` re-guards each of its own context uses. The lint + // cannot follow either through the nullable. + // ignore: use_build_context_synchronously + return _applyAcrossProjects(navContext, ref, route, loc, target); + } + return _applyInFocusedProject(ref, loc); +} + +/// The project the route names is not the focused one. +/// +/// The session is queued rather than written: the new project's session list +/// has not landed, and `_bootstrapSessions` is what resolves the id against it. +/// The suppression id rides with it — a tap means "show me what happened", +/// never "restart this agent". +Future _applyAcrossProjects( + BuildContext navContext, + ProviderContainer ref, + NotificationRoute route, + NavLocation loc, + SessionTarget target, +) async { + final sessionId = loc.sessionId; + // Saved, not assumed absent: another site's queued pick is state this route + // is borrowing, and an activation that fails owes it back untouched. + final priorPendingId = ref.read(pendingActiveSessionIdProvider); + final priorSuppressedId = ref.read(pendingSessionStartSuppressedIdProvider); + ref.read(pendingActiveSessionIdProvider.notifier).set(sessionId); + // Names the id it suppresses, so a run that never consumes it leaves a value + // that answers for nobody rather than a flag that answers for everyone. + ref.read(pendingSessionStartSuppressedIdProvider.notifier).set(sessionId); + + // A cold remote project needs its machine dialled and promoted before it can + // be focused, and this is the one path that does that and reports its typed + // failures. + bool ok; + try { + ok = await activateDrawerEntryById(navContext, ref, target.registrationId); + } catch (_) { + // The activation speaks for itself; here it is a route that did not land, + // and the queued state below must not outlive it. + ok = false; + } + if (!ok) { + // Only OUR write is taken back. A bootstrap that landed for another + // project while this activation ran has already consumed the value we + // snapshotted, and writing it back would re-arm an id nothing will ever + // resolve — which holds `reconcileActiveSession` off its fallback and, via + // the same guard, leaves both surface drains permanently unspendable. + if (ref.read(pendingActiveSessionIdProvider) == sessionId) { + ref.read(pendingActiveSessionIdProvider.notifier).set(priorPendingId); + } + if (ref.read(pendingSessionStartSuppressedIdProvider) == sessionId) { + ref + .read(pendingSessionStartSuppressedIdProvider.notifier) + .set(priorSuppressedId); + } + // The machine was asleep or the open was refused; the toast is still up and + // its retry has to be able to reach here again. + ref.read(_appliedRoutesProvider).release(route); + return false; + } + + ref.read(workbenchSurfaceProvider.notifier).set(WorkbenchSurface.workspace); + _handOverSurface(ref, loc); + ref.read(navControllerProvider.notifier).commit(loc); + return true; +} + +/// The project the route names is already focused. +/// +/// The session id is written straight through and never queued: nothing would +/// drain a pending id here (the bootstrap's listener returns on an unchanged +/// project), and while one is set `reconcileActiveSession` selects null instead +/// of falling back once the current session leaves the list. +bool _applyInFocusedProject(ProviderContainer ref, NavLocation loc) { + final sessionId = loc.sessionId; + // Only ever true for a route that NAMED a session and did not get it. A route + // that names none is a project destination and keeps its surface. + var sessionRefused = false; + if (sessionId != null) { + ref.read(activeSessionIdProvider.notifier).set(sessionId); + // Read back: the write is silently refused for a session the bridge is + // deleting, and revealing that session's surface afterwards would aim the + // workspace at a transcript nobody is going to be shown. + // + // Only that refusal, never a presence test of our own: an id this app does + // not recognise is written through by design (see [ActiveSessionId]), + // because the list for a project lands in stages and a guard demanding + // presence drops every selection made before it does — which is most of + // them. A session deleted while the toast was up is corrected by + // [reconcileActiveSession] on the next list change. + sessionRefused = ref.read(activeSessionIdProvider) != sessionId; + if (!sessionRefused) { + // Announcing the pick is what CLEARS the unread dot, and this path is the + // only route to a session that would otherwise skip it: the cross-project + // path gets it from `_bootstrapSessions`, and every manual tap from + // `session_row`'s own focused-project branch. Without it the bridge still + // believes this client is on the previously selected session — so the + // session now on screen keeps its dot, and the one the user left is + // exempted from earning another. + // + // `focusedServiceOrNull`, not the façade: this runs from a tap handler + // past an await, where the focused project's `ProjectSession` may be + // unresolved and reading the provider directly THROWS. + focusedServiceOrNull(ref, (s) => s.sessionsService)?.focus(sessionId); + } + } + // Even a route that lost its session still moves the user to the workspace: + // the project is a real destination, and reporting success from the settings + // surface without leaving it is the one outcome a tap cannot explain. + ref.read(workbenchSurfaceProvider.notifier).set(WorkbenchSurface.workspace); + if (sessionRefused) { + // No session-scoped reveal, but the peers still get their null write — this + // destination must not inherit a surface an earlier navigation left + // pending. + _clearSurfaceHandovers(ref); + // Only the SESSION-scoped half of this route was dropped; the project and + // the workspace surface are still where the user now is, and history has + // to say so or `back()` re-applies the entry before this one and silently + // discards the move. Carries the session actually in focus rather than + // none: that is the true destination, it dedupes to a no-op when the user + // was already here, and a session-less entry would instead be FILLED by + // [NavController]'s activeSessionId listener with whatever is selected + // next. + ref + .read(navControllerProvider.notifier) + .commit( + NavLocation( + target: loc.target, + surface: WorkbenchSurface.workspace, + sessionId: ref.read(activeSessionIdProvider), + ), + ); + return true; + } + _handOverSurface(ref, loc); + ref.read(navControllerProvider.notifier).commit(loc); + return true; +} + +/// Hands the destination surface to WorkspaceShell as pending state. +/// +/// Never `revealHandlerTabProvider` / `switchToAgentProvider`: those act on the +/// frame they are called, and the per-session UI restore that a focus change +/// arms re-applies the target session's own saved tab a frame later, silently +/// undoing them. Called AFTER the focus write and stamped with the focus it +/// leaves behind, so a drain running much later can tell it no longer applies. +/// +/// Both are written when null too, dropping a value an earlier navigation left +/// pending that this destination must not inherit. +/// +/// A route that names no session hands over neither: it asks for a project, and +/// a project opens in the layout its own last session was left in. The drains +/// hold a stamp back until the queued session id resolves, and a session-less +/// route queues none — so a stamp here would be spent on the frame it lands, +/// before the bootstrap's own default pick arms the restore that undoes it. +void _handOverSurface(ProviderContainer ref, NavLocation loc) { + final target = ref.read(selectedTargetProvider); + final view = loc.view; + final agentPage = view == null && loc.sessionId != null; + ref + .read(pendingWorkspaceViewProvider.notifier) + .set(view == null ? null : (target: target, value: view)); + ref + .read(pendingAgentPageProvider.notifier) + .set(agentPage ? (target: target, value: true) : null); +} + +/// Drops both handovers without naming a destination — the route reached the +/// project but not the session, so it may neither reveal that session's surface +/// nor leave an earlier navigation's pending one to be drained in its place. +void _clearSurfaceHandovers(ProviderContainer ref) { + ref.read(pendingWorkspaceViewProvider.notifier).set(null); + ref.read(pendingAgentPageProvider.notifier).set(null); +} diff --git a/app/lib/providers/providers.dart b/app/lib/providers/providers.dart index ef40496b..3f02d23d 100644 --- a/app/lib/providers/providers.dart +++ b/app/lib/providers/providers.dart @@ -140,28 +140,42 @@ final terminalStateProvider = StreamProvider((ref) { return seededStream(() => service.currentState, service.stateStream); }); +/// A message paired with the drawer entryId of the project it came from. +/// +/// The entryId is the only project attribution these fan-in streams can carry: +/// `HandlerEscalation` and `TerminalNotificationMessage` name no project at all, +/// 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. The id +/// is the registry key, i.e. already in its correct local-or-remote shape. +typedef ProjectScoped = ({String entryId, T message}); + /// Agent desktop-notification signals (OSC 9 / OSC 777) merged across /// ALL warm projects — not just the focused one — so a background project's /// agent can still raise a toast / OS notification. Rebuilds (and re-subscribes) /// when the warm set changes or a session resolves; the warm-set is small /// (kWarmCap), so the per-rebuild resubscribe is cheap. final terminalNotificationsProvider = - StreamProvider((ref) { + StreamProvider>((ref) { final openProjects = ref.watch(projectSessionRegistryProvider); - final controller = StreamController(); + final controller = + StreamController>(); final subs = >[]; for (final id in openProjects) { final session = ref.watch(projectSessionProvider(id)).value; if (session == null) continue; for (final bundle in session.checkoutServiceBundles) { subs.add( - bundle.terminalService.notificationStream.listen(controller.add), + bundle.terminalService.notificationStream.listen( + (m) => controller.add((entryId: id, message: m)), + ), ); } subs.add( session.checkoutServiceBundleStream.listen((bundle) { subs.add( - bundle.terminalService.notificationStream.listen(controller.add), + bundle.terminalService.notificationStream.listen( + (m) => controller.add((entryId: id, message: m)), + ), ); }), ); @@ -178,36 +192,40 @@ final terminalNotificationsProvider = /// Plugin/hook-sourced agent notifications (notification:push) merged across all /// warm projects — same fan-out as terminalNotificationsProvider but for the /// intent-aware plugin path. Rebuilds when the warm set changes. -final agentPushNotificationsProvider = StreamProvider(( - ref, -) { - final openProjects = ref.watch(projectSessionRegistryProvider); - final controller = StreamController(); - final subs = >[]; - for (final id in openProjects) { - final session = ref.watch(projectSessionProvider(id)).value; - if (session == null) continue; - for (final bundle in session.checkoutServiceBundles) { - subs.add( - bundle.terminalService.pushNotificationStream.listen(controller.add), - ); - } - subs.add( - session.checkoutServiceBundleStream.listen((bundle) { +final agentPushNotificationsProvider = + StreamProvider>((ref) { + final openProjects = ref.watch(projectSessionRegistryProvider); + final controller = + StreamController>(); + final subs = >[]; + for (final id in openProjects) { + final session = ref.watch(projectSessionProvider(id)).value; + if (session == null) continue; + for (final bundle in session.checkoutServiceBundles) { + subs.add( + bundle.terminalService.pushNotificationStream.listen( + (m) => controller.add((entryId: id, message: m)), + ), + ); + } subs.add( - bundle.terminalService.pushNotificationStream.listen(controller.add), + session.checkoutServiceBundleStream.listen((bundle) { + subs.add( + bundle.terminalService.pushNotificationStream.listen( + (m) => controller.add((entryId: id, message: m)), + ), + ); + }), ); - }), - ); - } - ref.onDispose(() { - for (final s in subs) { - s.cancel(); - } - controller.close(); - }); - return controller.stream; -}); + } + ref.onDispose(() { + for (final s in subs) { + s.cancel(); + } + controller.close(); + }); + return controller.stream; + }); /// Handler "needs you" escalations (handler:escalation) merged across all warm /// projects — same fan-out as [agentPushNotificationsProvider]. Drives the @@ -220,27 +238,32 @@ final agentPushNotificationsProvider = StreamProvider(( /// Each subscribe therefore also seeds the project's currently-pending /// escalations; the consumer de-dupes by escalationId, so re-seeding the same id /// across rebuilds is harmless. -final handlerEscalationsProvider = StreamProvider((ref) { - final openProjects = ref.watch(projectSessionRegistryProvider); - final controller = StreamController(); - final subs = >[]; - for (final id in openProjects) { - final session = ref.watch(projectSessionProvider(id)).value; - if (session == null) continue; - final handler = session.handlerService; - subs.add(handler.escalationStream.listen(controller.add)); - for (final esc in handler.currentState.escalations) { - controller.add(esc); - } - } - ref.onDispose(() { - for (final s in subs) { - s.cancel(); - } - controller.close(); - }); - return controller.stream; -}); +final handlerEscalationsProvider = + StreamProvider>((ref) { + final openProjects = ref.watch(projectSessionRegistryProvider); + final controller = StreamController>(); + final subs = >[]; + for (final id in openProjects) { + final session = ref.watch(projectSessionProvider(id)).value; + if (session == null) continue; + final handler = session.handlerService; + subs.add( + handler.escalationStream.listen( + (esc) => controller.add((entryId: id, message: esc)), + ), + ); + for (final esc in handler.currentState.escalations) { + controller.add((entryId: id, message: esc)); + } + } + ref.onDispose(() { + for (final s in subs) { + s.cancel(); + } + controller.close(); + }); + return controller.stream; + }); /// The agent terminal the workspace is showing: the focused session's own tab. /// diff --git a/app/lib/providers/sessions.dart b/app/lib/providers/sessions.dart index b22ce1ba..5166568a 100644 --- a/app/lib/providers/sessions.dart +++ b/app/lib/providers/sessions.dart @@ -150,6 +150,34 @@ final pendingActiveSessionIdProvider = () => ValueController(null), ); +/// The queued session id whose auto-start [pendingActiveSessionIdProvider]'s +/// drain must skip, or null. +/// +/// The drain starts a stopped session and speaks its refusal +/// (`_bootstrapSessions` in workspace_shell.dart) because a Recent-list tap +/// means "resume this". A notification tap means "show me what happened": +/// restarting an agent the user let finish spends tokens nobody asked for. +/// +/// Holds the ID rather than a bare flag, and the drain only honours it when it +/// EQUALS the id being resolved. Five other sites queue a pending id without +/// knowing this provider exists, and the bootstrap has early returns past the +/// point one is set — a flag surviving any of those would silently suppress an +/// unrelated later Recent-list tap's resume, which is the one thing that tap +/// means. +/// +/// What that buys, precisely: a leftover value can only ever answer for a +/// PAIR that survived together — this provider and the pending id both left +/// set by a run that returned early, or both left by the three sites that null +/// the pending id on a failed activation without seeing this one. A pair like +/// that eats one auto-start: the Recent-list tap on that same session, or the +/// default pick the bootstrap falls through to when that session is gone from +/// the list. One tap, self-clearing on the next drain — the price of keeping +/// those five sites ignorant of this provider, which is what makes them safe. +final pendingSessionStartSuppressedIdProvider = + NotifierProvider, String?>( + () => ValueController(null), + ); + /// The currently focused session entry, or null if none. final activeSessionProvider = Provider((ref) { final id = ref.watch(activeSessionIdProvider); diff --git a/app/lib/providers/visible_surface.dart b/app/lib/providers/visible_surface.dart index 3a55f352..c91af60d 100644 --- a/app/lib/providers/visible_surface.dart +++ b/app/lib/providers/visible_surface.dart @@ -51,6 +51,21 @@ final pendingWorkspaceViewProvider = PendingNav? >(() => ValueController(null)); +/// The agent transcript a navigation named, waiting for WorkspaceShell to show +/// it. +/// +/// [WorkspaceView] has no agent member — the transcript is not a workspace tab +/// — so a route that wants it cannot go through [pendingWorkspaceViewProvider], +/// and `switchToAgentProvider` is null whenever the shell is between mounts. +/// Same handover, same [PendingNav] stamp, drained beside the view. +/// +/// The value is always true: what is carried is the REQUEST, and the stamp is +/// what makes it self-invalidating, exactly as for the pending view. +final pendingAgentPageProvider = + NotifierProvider?>, PendingNav?>( + () => ValueController(null), + ); + /// A file a navigation named, waiting for the file explorer to open it. /// /// Grouped with [pendingWorkspaceViewProvider] because a file is only reachable diff --git a/app/lib/screens/workspace_shell.dart b/app/lib/screens/workspace_shell.dart index e2983f96..6b33524d 100644 --- a/app/lib/screens/workspace_shell.dart +++ b/app/lib/screens/workspace_shell.dart @@ -32,9 +32,12 @@ import '../models/session_entry.dart'; import '../project/project_session_registry.dart'; import '../providers/agent_transport.dart'; import '../providers/demo_mode.dart'; +import '../providers/device_provisioning.dart' show localDeviceUuidProvider; import '../providers/new_session_picker.dart' show newSessionStartInFlightProvider; +import '../providers/notification_route_apply.dart'; import '../providers/providers.dart'; +import '../providers/recent_sessions.dart' show recentSessionsProvider; import '../providers/relay_error_banner.dart'; import '../providers/session_search.dart'; import '../providers/session_workspace_state.dart'; @@ -46,7 +49,7 @@ import '../providers/visible_surface.dart'; import '../services/app_settings_service.dart'; import '../services/local_notification_service.dart'; import '../services/push_background_handler.dart' - show decodePush, pushDataOf, pushDedupKey; + show decodePush, pushDataOf, pushDedupKey, routeOfPush; import '../services/push_identity.dart'; import '../services/sessions_service.dart' show SessionOperationException, SessionsService; @@ -69,6 +72,7 @@ import '../widgets/workspace_panel.dart'; import '../navigation/back_intent.dart'; import '../navigation/nav_controller.dart'; import '../navigation/nav_location.dart'; +import '../navigation/notification_route.dart'; import 'app_settings_screen.dart'; /// Mobile page order. The drawer is NOT a page — it stays a `Scaffold.drawer` @@ -294,7 +298,18 @@ class WorkspaceShellState extends ConsumerState if (key != null && !_markNotified(key)) { return; // already surfaced (this surface or the live stream) } - _onAgentNotification(title: decoded.title, body: decoded.body); + // The only caller whose route is built from the wire rather than + // from a drawer entry: a push arrives from a machine this install has + // to name for itself, so [routeOfPush] addresses it by machine + + // project (or by session id) and answers null for a payload sealed by + // a bridge that carried neither — a projectId alone is unroutable by + // design, since `computeProjectId` hashes the folder path and can + // name the wrong machine with confidence. + _onAgentNotification( + title: decoded.title, + body: decoded.body, + route: routeOfPush(decoded), + ); } catch (e) { // Async listener: an uncaught throw here is an unhandled rejection. AbLog.error( @@ -423,7 +438,7 @@ class WorkspaceShellState extends ConsumerState // ── Terminal notifications ─────────────────────────────────────────── - void _onNotification(TerminalNotificationMessage msg) { + void _onNotification(TerminalNotificationMessage msg, String entryId) { // A session terminal's id IS the session id (service PTYs use their own, // which never matches an active session). if (_isViewingSession(msg.terminalId)) return; @@ -433,7 +448,16 @@ class WorkspaceShellState extends ConsumerState final title = (msg.title != null && msg.title!.isNotEmpty) ? msg.title! : 'Agent'; - _onAgentNotification(title: title, body: body); + _onAgentNotification( + title: title, + body: body, + route: NotificationRoute( + registrationId: entryId, + terminalId: msg.terminalId, + kind: 'agent', + sourceMessageId: msg.id, + ), + ); } /// Reads the live focus state into [isViewingSession] — every surfacer below @@ -449,7 +473,7 @@ class WorkspaceShellState extends ConsumerState lifecycle: _lifecycle, ); - void _onAgentNotificationPush(NotificationPushMessage msg) { + void _onAgentNotificationPush(NotificationPushMessage msg, String entryId) { if (_isViewingSession(msg.sessionId)) return; const labels = { 'permission_request': 'Permission needed', @@ -468,10 +492,19 @@ class WorkspaceShellState extends ConsumerState final body = (msg.message != null && msg.message!.isNotEmpty) ? msg.message! : label; - _onAgentNotification(title: title, body: body); + _onAgentNotification( + title: title, + body: body, + route: NotificationRoute( + registrationId: entryId, + terminalId: msg.sessionId, + kind: 'agent', + sourceMessageId: msg.id, + ), + ); } - void _onHandlerEscalation(HandlerEscalation esc) { + void _onHandlerEscalation(HandlerEscalation esc, String entryId) { // Handler escalations name their session in `terminalId`. if (_isViewingSession(esc.terminalId)) return; // Route through the shared surfacer (foreground toast / background OS @@ -481,21 +514,103 @@ class WorkspaceShellState extends ConsumerState ? 'Handler — urgent' : 'Handler needs you'; final body = esc.question.isNotEmpty ? esc.question : 'Agent needs you'; - _onAgentNotification(title: title, body: body); + _onAgentNotification( + title: title, + body: body, + route: NotificationRoute( + registrationId: entryId, + terminalId: esc.terminalId, + kind: 'handler', + sourceMessageId: esc.escalationId, + ), + ); } - void _onAgentNotification({required String title, required String body}) { + /// [route] is what tapping this notification should open, or null when the + /// producer could name nothing addressable — which is the foreground-push + /// path against a bridge that sealed no machine, since a projectId alone is + /// not a machine (`computeProjectId` hashes the folder path) and must never + /// be used to pick an entry. + void _onAgentNotification({ + required String title, + required String body, + NotificationRoute? route, + }) { if (shouldShowInAppToast(_lifecycle)) { + // Gated on the route actually RESOLVING, not on one having been built: an + // offer that opens nothing is worse than none, and the two conditions part + // company on any id that names no place (a blank one among them). Resolved + // synchronously against what is already loaded, which is exact for the + // in-app producers — they carry a registrationId, and neither that rule + // nor the terminalId one consults the device uuid. + // + // Inside this branch, because it gates the CHIP and nothing else: the OS + // notification below carries `route` verbatim and never reads this + // answer, so resolving it there would scan the whole cached-session + // universe — on the backgrounded path, which is the common one — to + // discard the result. A chip lives 8s, so "resolves now" is as good as + // "resolves when pressed", while an OS notification sits in the shade + // indefinitely and the applier re-resolves at tap time against freshly + // awaited state; gating the payload here would bake a cold-cache miss + // into a notification that would have resolved fine an hour later. + final destination = route == null + ? null + : resolveNotificationRoute( + route, + known: ref.read(recentSessionsProvider), + localDeviceUuid: ref.read(localDeviceUuidProvider).value, + ); + if (destination == null) { + showAbToastOverlay( + context, + toast: AbToast(icon: AbIcons.bell, title: title, description: body), + ); + return; + } + // Both captured before the tap: applying the route switches projects, + // which unmounts this shell out from under the overlay entry still + // holding the callback. `context` read through the State getter at tap + // time would throw on the defunct element; the captured element answers + // `mounted` false instead, which is what the applier tests. + final container = ref.container; + final toastContext = context; showAbToastOverlay( context, - toast: AbToast(icon: AbIcons.bell, title: title, description: body), + toast: AbToast( + icon: AbIcons.bell, + title: title, + description: body, + actionLabel: 'Open', + onAction: () => detached( + 'WorkspaceShell', + 'notification route failed', + () => applyNotificationRoute(toastContext, container, route!), + ), + ), + // The action cannot dismiss its own toast (`showAbToastOn`'s remove is + // local to that call), so it stays pressable for its whole life — long + // enough to be worth reaching for, and the applier absorbs the second + // press. + duration: const Duration(seconds: 8), ); return; } // App is not focused (occluded or minimized): only the OS notification can // surface above the foreground app — an in-app toast would be painted // behind it. Fire-and-forget; `show` logs delivery failures internally. - _osNotifications.show(title: title, body: body); + // + // The payload is the whole tap: `main` decodes it back into this same route + // and applies it. Carried whenever the producer named one — `route`, not the + // chip's `tappable` — because resolution is redone at tap time and this + // notification outlives the state it would have been judged against here. + // Null rather than an empty route when there is nothing to name at all: on + // Windows the payload is what makes a body tap arrive as + // `selectedNotificationAction`, and an empty one lands nowhere. + _osNotifications.show( + title: title, + body: body, + payload: route == null ? null : encodeNotificationRoute(route), + ); } // ── Preferences ────────────────────────────────────────────────────── @@ -615,7 +730,15 @@ class WorkspaceShellState extends ConsumerState try { await ref.read(projectSessionProvider(triggeredFor).future); } catch (_) { - // Surfaced elsewhere; nothing actionable here. + // Surfaced elsewhere; nothing actionable here — but the queued pick was + // this run's to resolve, and no later run will: left set it holds + // `reconcileActiveSession` off a fallback and leaves every surface + // handover unspendable, since the drains wait on it. Same clear, same + // guard as the requestList failure below. + if (mounted && ref.read(selectedRegistrationIdProvider) == triggeredFor) { + ref.read(pendingActiveSessionIdProvider.notifier).set(null); + ref.read(pendingSessionStartSuppressedIdProvider.notifier).set(null); + } return; } if (!mounted || ref.read(selectedRegistrationIdProvider) != triggeredFor) { @@ -638,6 +761,7 @@ class WorkspaceShellState extends ConsumerState // to a default and holds the explorer's checkout unsettled, so the // banner's "switch and back" would be the only way out. ref.read(pendingActiveSessionIdProvider.notifier).set(null); + ref.read(pendingSessionStartSuppressedIdProvider.notifier).set(null); ref .read(relayErrorBannerProvider.notifier) .set( @@ -655,8 +779,15 @@ class WorkspaceShellState extends ConsumerState // 1. Pending session-id (from a cross-project session-row click). final pendingId = ref.read(pendingActiveSessionIdProvider); + // Honoured only for the id it names: every other site that queues a pending + // id leaves this one alone, so a value left over from a run that returned + // early must not answer for theirs. + final startSuppressed = + pendingId != null && + ref.read(pendingSessionStartSuppressedIdProvider) == pendingId; if (pendingId != null) { ref.read(pendingActiveSessionIdProvider.notifier).set(null); + ref.read(pendingSessionStartSuppressedIdProvider.notifier).set(null); // `!s.deleting` on both filters below: a cross-project tap or a cold open // must not land on a session the bridge is already removing. final desired = list @@ -666,7 +797,11 @@ class WorkspaceShellState extends ConsumerState ref.read(activeSessionIdProvider.notifier).set(desired.id); // A start already queued behind an isolated checkout's setup run is // the create flow's own, prompt and all — see [sessionStartQueued]. - if (!desired.running && !sessionStartQueued(desired.setup)) { + // A notification tap asked to SEE this session, not to resume it — + // see [pendingSessionStartSuppressedIdProvider]. + if (!startSuppressed && + !desired.running && + !sessionStartQueued(desired.setup)) { // The cross-project half of a session-row / Recent-list tap, so a // refused start has to speak here too — otherwise the same tap reports // its failure only when the project happened to be focused already. @@ -734,7 +869,15 @@ class WorkspaceShellState extends ConsumerState orElse: () => active.first, ); ref.read(activeSessionIdProvider.notifier).set(session.id); - if (!session.running && !sessionStartQueued(session.setup)) { + // The suppressor is honoured here too, not only in the branch above: a + // route naming a session this project no longer has (deleted since the + // cache write, or a stale terminalId) falls THROUGH to this pick, and + // starting `active.first` would spend tokens on an agent the tap never + // named — the exact cost the suppressor exists to prevent, arrived at by + // the one path where the user's intent is furthest from a resume. + if (!startSuppressed && + !session.running && + !sessionStartQueued(session.setup)) { await _startBestEffort(svc, session.id); if (!mounted) return; if (ref.read(selectedRegistrationIdProvider) != triggeredFor) return; @@ -922,45 +1065,49 @@ class WorkspaceShellState extends ConsumerState // OS notification while backgrounded (gated in _onNotification). The // provider reloads on project switch and replays a carried-over value; // guard against re-handling the same emission. - ref.listen>( + ref.listen>>( terminalNotificationsProvider, (prev, next) { - final msg = next.value; - if (msg == null) return; - if (prev?.value == msg) return; // carried-over value on reload - _onNotification(msg); + final scoped = next.value; + if (scoped == null) return; + if (prev?.value == scoped) return; // carried-over value on reload + _onNotification(scoped.message, scoped.entryId); }, ); - ref.listen>( + ref.listen>>( agentPushNotificationsProvider, (prev, next) { - final msg = next.value; - if (msg == null) return; - if (prev?.value == msg) return; // carried-over value on reload + final scoped = next.value; + if (scoped == null) return; + if (prev?.value == scoped) return; // carried-over value on reload + final msg = scoped.message; // Shared dedup key with the FCM path: the bridge seals // `sourceMessageId === msg.id` for agent notifications, so this id // guards both surfaces — a connected-but-backgrounded phone that gets // the same event live AND via push surfaces it only once (matches the // handler-escalation path below). if (!_markNotified(msg.id)) return; // once/id - _onAgentNotificationPush(msg); + _onAgentNotificationPush(msg, scoped.entryId); }, ); - ref.listen>(handlerEscalationsProvider, ( - prev, - next, - ) { - final esc = next.value; - if (esc == null) return; - // Shared dedup key with the FCM path: the bridge seals - // `sourceMessageId === escalationId` for handler pushes, so this same id - // guards both surfaces and a single escalation is surfaced only once - // whether it arrives live or via push. - if (!_markNotified(esc.escalationId)) return; // once/id - _onHandlerEscalation(esc); - }); + ref.listen>>( + handlerEscalationsProvider, + (prev, next) { + final scoped = next.value; + if (scoped == null) return; + final esc = scoped.message; + // Shared dedup key with the FCM path: the bridge seals + // `sourceMessageId === escalationId` for handler pushes, so this same + // id guards both surfaces and a single escalation is surfaced only once + // whether it arrives live or via push. Keying on the escalationId alone + // — not the project-scoped record — is also what keeps the provider's + // per-rebuild re-seed idempotent. + if (!_markNotified(esc.escalationId)) return; // once/id + _onHandlerEscalation(esc, scoped.entryId); + }, + ); // Surface launcher/transport errors inline (local-mode spawn // failures, a relay stream transport that errors) instead of leaving the @@ -1042,10 +1189,24 @@ class WorkspaceShellState extends ConsumerState // route has prefs or a PageView, and the build that finally lands one is the // frame the drain has to run on. Deferred a frame so the drain's provider // writes never land during build. - if (ref.watch(pendingWorkspaceViewProvider) != null) { + // Both read unconditionally, never as `a != null || b != null`: a + // short-circuited watch registers no dependency, so the second provider's + // own write would never rebuild this route. + final pendingView = ref.watch(pendingWorkspaceViewProvider); + final pendingAgentPage = ref.watch(pendingAgentPageProvider); + // The third input to both drains, watched for the same reason even though + // nothing here reads it: they hold a stamp back while a queued session id + // is unresolved, so the write that CLEARS it is the retry. Without this + // dependency the retry rides on an incidental rebuild, and there is a real + // case with none — `reconcileActiveSession` selects the queued id off the + // persisted cache, so `_bootstrapSessions` later re-sets the same value and + // notifies nobody, stranding the stamp and the tab it named. + ref.watch(pendingActiveSessionIdProvider); + if (pendingView != null || pendingAgentPage != null) { WidgetsBinding.instance.addPostFrameCallback((_) { if (!mounted) return; _drainPendingWorkspaceView(); + _drainPendingAgentPage(); }); } @@ -1499,6 +1660,11 @@ class WorkspaceShellState extends ConsumerState notifier.set(null); return; } + // Not spent while a queued session id is still unresolved — see + // [_drainPendingAgentPage], which carries the same guard for the same + // reason. This is the drain a handler route uses, so the cross-project + // escalation tap is the flow that needs it most. + if (ref.read(pendingActiveSessionIdProvider) != null) return; final mobile = _isMobileLayout; // Mobile needs the PageView, which does not exist until a build past the // boot gate — and a tab switched behind the page the user is looking at @@ -1518,6 +1684,82 @@ class WorkspaceShellState extends ConsumerState _revealWorkspaceView(pending.value); } + /// Show the agent transcript a navigation left in [pendingAgentPageProvider]. + /// + /// Same three halves as [_drainPendingWorkspaceView] and for the same + /// reasons: a stamp for another project is spent unshown, and a mobile route + /// with no PageView yet leaves the request for the build that has one. The + /// desktop half is [_revealAgentPanel] — the mirror of the view drain's + /// [_openContextPanel], because the agent panel is only the default zone in + /// the layouts that still mount it. + void _drainPendingAgentPage() { + final pending = ref.read(pendingAgentPageProvider); + if (pending == null) return; + final notifier = ref.read(pendingAgentPageProvider.notifier); + if (pending.target != ref.read(selectedTargetProvider)) { + notifier.set(null); + return; + } + // Not spent while a queued session id is still unresolved. The + // cross-project applier seeds that id, activates the project, and stamps + // this request all before `_bootstrapSessions` has the new project's list, + // so the transcript this asks for is not the one on screen yet — and the + // per-session UI restore that resolving it arms would hide the panel again + // a frame after the reveal, with the request already spent. The rebuild + // that restore triggers is what retries it, exactly as the `hasClients` + // guard below leaves the request for the build that has a PageView. It is + // also what keeps [_revealAgentPanel]'s [_updateSessionUi] off a key + // pairing the NEW project's entryId with the session id the old one left + // in [activeSessionIdProvider]. + // + // The retry therefore depends on that id eventually clearing. Every path + // that queues one clears it on the run that resolves it, and a run that + // returns early leaves the next project-open to do so — but a stamp whose + // id nothing ever resolves is never spent, which is the same shape as the + // stuck id already holding `reconcileActiveSession` off a fallback. + if (ref.read(pendingActiveSessionIdProvider) != null) return; + final mobile = _isMobileLayout; + if (mobile && !_pageController.hasClients) return; + notifier.set(null); + if (mobile) { + switchToAgentPage(); + return; + } + _revealAgentPanel(); + } + + /// Give the agent panel room on a desktop-shaped layout, the inverse of + /// [_openContextPanel]. + /// + /// Two shipped layouts hide it outright: [_PanelMode.contextExpanded] drops + /// [_agentPanel] from [_buildPanels] entirely, and the touch tablet keeps it + /// mounted at zero readable width under a context pane that is both open AND + /// expanded. The panel mode is per-session and restored on every focus + /// change, so a route into a session the user last left expanded would + /// otherwise reveal nothing and spend its request doing it. Never widens a + /// merely narrow split — only the modes where the transcript is not on + /// screen at all. + void _revealAgentPanel() { + if (isMobilePlatform) { + // Expanded is the whole of it: a normally-open pane takes a quarter of + // the width and leaves the transcript the other three, which is why + // `agentPanelVisible` reads that state as the agent being on screen. + // Un-expanding rather than closing is also the exact mirror of the mouse + // desktop's contextExpanded → normal below, and it keeps the file or + // diff the user deliberately opened. + if (_tabletEndDrawerOpen && _tabletContextPanelExpanded) { + _setTabletContextExpanded(false); + } + return; + } + if (_effectivePanelMode == _PanelMode.contextExpanded) { + setState(() { + _panelMode = _PanelMode.normal; + _updateSessionUi((s) => s.copyWith(panelMode: _panelMode!.name)); + }); + } + } + bool get _isMobileLayout => MediaQuery.sizeOf(context).width < kCompactBreakpoint; diff --git a/app/lib/services/local_notification_service.dart b/app/lib/services/local_notification_service.dart index 93ae72be..a437a841 100644 --- a/app/lib/services/local_notification_service.dart +++ b/app/lib/services/local_notification_service.dart @@ -1,3 +1,5 @@ +import 'dart:math' show Random; + import 'package:flutter_local_notifications/flutter_local_notifications.dart'; import '../util/ab_log.dart'; @@ -6,14 +8,15 @@ import '../util/ab_log.dart'; /// OS notifications when the app is backgrounded; the caller decides when to /// invoke based on AppLifecycleState. Degrades silently if unavailable. /// -/// One instance per isolate, because [_ready] has to outlive the widget that -/// initialized it. `WorkspaceShell` constructs this in `initState` and a surface -/// swap or project switch remounts the whole shell — and the demo's mount -/// deliberately SKIPS [init] (its `DarwinInitializationSettings` would raise the -/// iOS alert-permission prompt on behalf of a sample project). Per-instance -/// readiness would therefore make [show] a no-op for the demo's whole lifetime, -/// silently dropping the handler escalations that still fan out from the user's -/// other warm projects. +/// One instance per isolate, because [_ready] and [onTap] have to outlive the +/// widget that installed them. `WorkspaceShell` constructs this in `initState`, +/// and the demo's mount deliberately SKIPS [init] (its +/// `DarwinInitializationSettings` would raise the iOS alert-permission prompt +/// on behalf of a sample project). Per-instance readiness would therefore make +/// [show] a no-op for the demo's whole lifetime, silently dropping the handler +/// escalations that still fan out from the user's other warm projects. Sharing +/// the instance is also what lets `main` install [onTap] once, before any shell +/// exists, and have every later mount deliver into it. class LocalNotificationService { LocalNotificationService._(); @@ -26,12 +29,32 @@ class LocalNotificationService { FlutterLocalNotificationsPlugin(); bool _ready = false; - /// Monotonic, wrapping notification id. A wall-clock id (seconds) collides - /// for notifications fired within the same second — on Android/iOS a - /// duplicate id replaces the prior notification, silently dropping it. A - /// per-instance counter (masked to a positive 31-bit int for the Android - /// `int` id) gives each show() a distinct id. - int _nextId = 0; + /// Where a tapped notification is delivered. Assigned once from `main`, + /// before any shell mounts — a warm tap can arrive before the first frame. + /// + /// Reached through the stable [_dispatchTap] trampoline rather than handed to + /// the plugin directly, so assigning it after [init] still takes effect. + void Function(NotificationResponse)? onTap; + + /// Monotonic, wrapping notification id, seeded per isolate rather than from + /// zero. A wall-clock id (seconds) collides for notifications fired within + /// the same second, so this is a counter (masked to a positive 31-bit int for + /// the Android `int` id) — but the headless push isolate and the main engine + /// each run their own, and from a shared origin they mint the same ids. Now + /// that a payload rides along, such a collision is no longer a duplicate + /// harmlessly replacing its twin (`FLAG_UPDATE_CURRENT`) but a tap carrying + /// the WRONG route; a random base makes the two ranges all but certain to + /// differ. + int _nextId = Random().nextInt(1 << 20) << 10; + + /// Stable target for `onDidReceiveNotificationResponse`, so the callback the + /// plugin holds is never the one that has to be replaced. + /// + /// Required rather than tidy: `FlutterLocalNotificationsWindows.initialize` + /// early-returns when it is already ready — BEFORE assigning its user + /// callback — and [init] runs on every shell mount, so a closure captured at + /// any later call would never be installed. + void _dispatchTap(NotificationResponse response) => onTap?.call(response); Future init() async { try { @@ -53,7 +76,14 @@ class LocalNotificationService { // on web, or under test bindings), so a `null` stays OPTIMISTIC: leave // `_ready` true and let `show` attempt delivery, matching the prior // always-ready behavior. Only a definitive `false` disables us. - final ok = await _plugin.initialize(settings: settings); + // + // No `onDidReceiveBackgroundNotificationResponse`: that entry point is + // for action buttons answered without resuming the app, and there are + // none here — every tap means "show me this", which needs the app. + final ok = await _plugin.initialize( + settings: settings, + onDidReceiveNotificationResponse: _dispatchTap, + ); _ready = ok ?? true; if (!_ready) { AbLog.warn( @@ -75,14 +105,24 @@ class LocalNotificationService { /// (`_ready == false`); logs and swallows any platform error so a delivery /// failure never breaks the caller. Callers fire-and-forget. /// - /// Carries no `payload:`/`actions:` and [init] registers no response - /// callback, because a tapped action here has nothing to deliver and nowhere - /// to deliver it: `bridge/src/push/push-dispatcher.ts` seals only - /// `{title, body, kind, projectId, sourceMessageId}`, so an escalation's - /// quick choices never reach this layer; and an answer must be sealed on a - /// live E2E session (`HandlerService.reply` → `ProjectSession.send`), which - /// a background/headless isolate does not have and no offline queue holds. - /// On iOS the escalation notification is not ours at all — the NSE in + /// [payload] is what a tap hands back through [onTap] — the encoded route the + /// caller wants opened (`navigation/notification_route.dart`), and the only + /// channel one rides on: nothing else about the notification survives to the + /// tap. Pass null, not an empty route, when there is nothing to open — on + /// Windows the payload is what makes a BODY tap arrive as + /// `selectedNotificationAction`, so an empty one turns a plain launch into a + /// tap that resolves to nothing. + /// + /// Never the sealed push blob itself: on Windows the payload becomes the + /// toast XML's `launch` attribute, and an oversized document makes `show` + /// throw into the swallowed log below — after which Windows notifications + /// silently stop appearing. + /// + /// Still no `actions:`, because an escalation's quick choices have nowhere to + /// go: they are not in the sealed payload, and an answer must be sealed on a + /// live E2E session (`HandlerService.reply` → `ProjectSession.send`), which a + /// background/headless isolate does not have and no offline queue holds. On + /// iOS the escalation notification is not ours at all — the NSE in /// `ios/NotificationService` renders the APNs alert, and the forked `push` /// plugin never forwards `response.actionIdentifier`. /// @@ -91,7 +131,11 @@ class LocalNotificationService { /// a pending-answer store flushed once `handler:status` replays the /// still-unanswered escalation; then `showsUserInterface: true` actions here /// so the tap resumes the app instead of a headless isolate. - Future show({required String title, required String body}) async { + Future show({ + required String title, + required String body, + String? payload, + }) async { if (!_ready) return; final id = _nextId; _nextId = (_nextId + 1) & 0x7fffffff; @@ -100,6 +144,7 @@ class LocalNotificationService { id: id, title: title, body: body, + payload: payload, notificationDetails: const NotificationDetails( android: AndroidNotificationDetails( 'agent_notifications', diff --git a/app/lib/services/notification_tap.dart b/app/lib/services/notification_tap.dart new file mode 100644 index 00000000..b89bd396 --- /dev/null +++ b/app/lib/services/notification_tap.dart @@ -0,0 +1,45 @@ +import 'package:flutter/foundation.dart' show TargetPlatform; +import 'package:flutter_local_notifications/flutter_local_notifications.dart'; + +import '../navigation/notification_route.dart'; + +/// What a tapped OS notification named, or null when it carried no route. +/// +/// The payload is the ONLY input, deliberately: +/// +/// - The response type says nothing useful. On Windows a plain body tap arrives +/// as [NotificationResponseType.selectedNotificationAction], because the +/// plugin classifies on whether the toast's `launch` argument is non-null +/// (`src/plugin.cpp:50-53`) and our payload IS that argument — so filtering on +/// `selectedNotification` would drop every Windows tap. +/// - `actionId` says nothing either, and worse. The same Windows path fills it +/// with the payload verbatim (`lib/src/plugin/ffi.dart:127-133`), so treating +/// a non-null `actionId` as a button press misroutes every Windows tap. We add +/// no action buttons anywhere, so there is no button case to tell apart. +/// - `id` is never keyed on: it is null on Windows and, on the platforms that do +/// carry one, it is a per-isolate counter that the headless push isolate and +/// the main engine mint independently. +NotificationRoute? routeOfTapResponse(NotificationResponse r) => + decodeNotificationRoute(r.payload); + +/// Whether `Push.addOnNotificationTap` may be registered on [platform]. +/// +/// iOS alone, and the narrowing is the substance rather than a transport check: +/// on Android `PushPlugin` rebuilds a RemoteMessage out of ANY launch intent's +/// extras and fires that callback whenever the resulting data map is non-empty +/// — and fln's own select-notification intent carries a `payload` String extra, +/// so registering there would deliver every Android tap a second time in a +/// different shape. +bool pushTapRegistrationSupported(TargetPlatform platform) => + platform == TargetPlatform.iOS; + +/// Whether `getNotificationAppLaunchDetails` may be called on [platform]. +/// +/// fln has no Linux branch and falls through to a platform-interface base that +/// THROWS UnimplementedError; Windows both throws StateError before `initialize` +/// and hard-codes `didNotificationLaunchApp` true for a warm tap it has already +/// delivered in-process, which would replay that tap at every launch. +bool launchDetailsSupported(TargetPlatform platform) => + platform == TargetPlatform.iOS || + platform == TargetPlatform.android || + platform == TargetPlatform.macOS; diff --git a/app/lib/services/push_background_handler.dart b/app/lib/services/push_background_handler.dart index db6f7e39..fb799098 100644 --- a/app/lib/services/push_background_handler.dart +++ b/app/lib/services/push_background_handler.dart @@ -4,22 +4,40 @@ import 'package:antgrid_relay_client/antgrid_relay_client.dart' show openPushBlob; import 'package:push/push.dart'; +import '../navigation/notification_route.dart'; import '../util/ab_log.dart'; import 'local_notification_service.dart'; import 'push_identity.dart'; /// Decoded push payload. `kind` distinguishes handler-escalation urgency from -/// ordinary agent notifications; `projectId` attributes the push to a project. -/// Both are sealed in by the bridge (`bridge/src/push/compose.ts`). +/// ordinary agent notifications; the routing ids say what the push is about. +/// Split across two bridge files: `compose.ts` narrows the message union and so +/// is the only place `terminalId`, `sourceMessageId`, `kind` and the strings can +/// be read; `push-dispatcher.ts` stamps `projectId` and `machineUuid`, which the +/// message never carries. +/// +/// `projectId` and `machineUuid` are nullable together: a bridge older than the +/// widened payload seals neither, and the pair is what [routeOfPush] addresses +/// a project by. Neither is guessable — `computeProjectId` hashes the folder +/// path with no machine input — so absence is unroutable, never inferred. +/// +/// `terminalId` is nullable because `notification:push` carries an optional +/// `sessionId`: the hook producer often names no session, and such a push is +/// about the project alone. /// /// `sourceMessageId` is nullable: the bridge does not always stamp one, and two /// distinct pushes that both lack it MUST NOT collapse to the same dedup key /// (see [pushDedupKey]). +/// +/// Every one of them is absent-as-null, never `''`: an empty id still satisfies +/// a `!= null` test and would address a project nobody has. typedef DecodedPush = ({ String title, String body, String? kind, String? projectId, + String? machineUuid, + String? terminalId, String? sourceMessageId, }); @@ -37,6 +55,34 @@ String? pushDedupKey(DecodedPush decoded) { return null; } +/// What tapping this push should open, or null when it names nothing this app +/// could address. +/// +/// Null rather than a route with only a title: the payload rides in an OS +/// notification's launch slot, and a route that cannot resolve buys a chip the +/// user can only ever press to no effect. The structural test mirrors +/// [resolveNotificationRoute]'s own preconditions — a machine AND a project, or +/// a session id to look up — and deliberately not its data, which does not +/// exist yet in the isolate that seals the payload. +/// +/// No `registrationId`: that is the in-app paths' pre-resolved id, and a push +/// arrives from a machine this install has to name for itself. +NotificationRoute? routeOfPush(DecodedPush decoded) { + final machineUuid = decoded.machineUuid; + final projectId = decoded.projectId; + final terminalId = decoded.terminalId; + final addressable = + (machineUuid != null && projectId != null) || terminalId != null; + if (!addressable) return null; + return NotificationRoute( + machineUuid: machineUuid, + projectId: projectId, + terminalId: terminalId, + sourceMessageId: decoded.sourceMessageId, + kind: decoded.kind, + ); +} + /// Pure decrypt+parse of a push data payload. Testable without the plugin. Future decodePush( Map data, { @@ -54,29 +100,52 @@ Future decodePush( if (json == null) return null; try { final m = jsonDecode(json) as Map; - final src = m['sourceMessageId'] as String?; + // [namedOrNull], not a cast: a throw anywhere in this `try` drops the WHOLE + // notification, so a newer bridge sending one id in an unexpected shape + // must cost that id, never the alert. Shared with the route decoder rather + // than restated, because the route this feeds is re-tested by that same + // predicate — an id only one of them accepts is an id that survives + // decoding to address nothing. return ( - title: (m['title'] as String?) ?? 'Agent', - body: (m['body'] as String?) ?? '', - kind: m['kind'] as String?, - projectId: m['projectId'] as String?, - // Preserve absence as null, not '' — an empty id must not dedup-collide. - sourceMessageId: (src != null && src.isNotEmpty) ? src : null, + // The strings go through it too, and that is the point of the rule above + // rather than an extension of it: the cast these two used to be is the + // only thing in this `try` that can throw on the payload's own content, + // and a throw here costs the WHOLE alert instead of one field. Blank + // collapses into the fallback for the same reason an id does — + // `composePush` never sends an empty title, so one could only come from a + // bridge that meant nothing by it, and a blank heading is not an + // improvement on 'Agent'. + title: namedOrNull(m['title']) ?? 'Agent', + body: namedOrNull(m['body']) ?? '', + kind: namedOrNull(m['kind']), + projectId: namedOrNull(m['projectId']), + machineUuid: namedOrNull(m['machineUuid']), + terminalId: namedOrNull(m['terminalId']), + sourceMessageId: namedOrNull(m['sourceMessageId']), ); } catch (_) { return null; } } -/// Narrow `push`'s pigeon-typed data payload to the plain map [decodePush] -/// takes. `push` types it `Map?` because that is pigeon's -/// lowest common denominator; the sealed-blob fields are always non-null -/// strings. Narrowing here keeps [decodePush] testable without the plugin. -Map pushDataOf(RemoteMessage message) => { - for (final e in (message.data ?? const {}).entries) +/// Narrow a pigeon-typed data payload to the plain map [decodePush] takes. +/// `push` types it `Map?` because that is pigeon's lowest +/// common denominator; the sealed-blob fields are always non-null strings. +/// Narrowing here keeps [decodePush] testable without the plugin. +/// +/// Takes the bare map, not a [RemoteMessage]: the notification-tap APIs hand +/// one directly and never build a message. On iOS the map they hand is the FULL +/// APNs userInfo, with `epk`/`box` at top level beside a nested `aps` — which +/// this drops as a non-String value, harmlessly. +Map pushDataMap(Map? raw) => { + for (final e in (raw ?? const {}).entries) if (e.key != null && e.value is String) e.key!: e.value! as String, }; +/// [pushDataMap] over the data of a delivered message. +Map pushDataOf(RemoteMessage message) => + pushDataMap(message.data); + /// Background message handler, registered via [Push.addOnBackgroundMessage] /// from both `main` and `pushBackgroundMain`. /// @@ -95,7 +164,18 @@ Future pushBackgroundHandler(RemoteMessage message) async { if (decoded == null) return; final notifications = LocalNotificationService(); await notifications.init(); - await notifications.show(title: decoded.title, body: decoded.body); + // The FCM message is data-only (`relay/src/push/fcm.ts`), so on Android this + // is the ONLY thing that renders a background push — every one of them is + // tappable-to-route or none is. Null, never an encoded empty route: on + // Windows a payload is what classifies a body tap as + // `selectedNotificationAction`, so an empty one buys a tap that resolves to + // nothing in place of the plain launch. + final route = routeOfPush(decoded); + await notifications.show( + title: decoded.title, + body: decoded.body, + payload: route == null ? null : encodeNotificationRoute(route), + ); } catch (e) { AbLog.error( 'PushBackgroundHandler', diff --git a/app/lib/widgets/agent_panel.dart b/app/lib/widgets/agent_panel.dart index d5176541..697e7f05 100644 --- a/app/lib/widgets/agent_panel.dart +++ b/app/lib/widgets/agent_panel.dart @@ -579,6 +579,10 @@ class HandlerHeaderControl extends ConsumerWidget { // is handed over as pending state instead — the same handover a deep link // naming a view uses, drained by the shell after the restore. void openHandler() { + // Every branch below lands on the handler tab, by handover or by call, so + // a pending agent-page stamp from an earlier navigation must not survive + // any of them — its drain runs last and would override the tab. + ref.read(pendingAgentPageProvider.notifier).set(null); final waiting = session?.runState == HandlerRunState.needsYou ? null : state.escalations diff --git a/app/test/navigation/nav_controller_test.dart b/app/test/navigation/nav_controller_test.dart index 301b6994..054013c1 100644 --- a/app/test/navigation/nav_controller_test.dart +++ b/app/test/navigation/nav_controller_test.dart @@ -278,6 +278,25 @@ void main() { expect(c.read(pendingFilePathProvider), isNull); }); + // A [NavLocation] can never ask for the agent transcript, so this write is + // only ever the null half — and it is the half that matters: the drains run + // in one post-frame callback with the agent's LAST, so a stamp a notification + // route left pending would override the tab this location just named. + test('apply drops an agent-page stamp the location did not name', () { + final nav = c.read(navControllerProvider.notifier); + nav.commit(_loc('a', view: WorkspaceView.git)); + nav.commit(_loc('b')); + c.read(pendingAgentPageProvider.notifier).set(( + target: const LocalProject('a'), + value: true, + )); + + nav.back(); + + expect(c.read(pendingAgentPageProvider), isNull); + expect(c.read(pendingWorkspaceViewProvider)?.value, WorkspaceView.git); + }); + test('back restores the file recorded with the entry', () { final nav = c.read(navControllerProvider.notifier); nav.commit( diff --git a/app/test/navigation/notification_route_test.dart b/app/test/navigation/notification_route_test.dart new file mode 100644 index 00000000..fe29af36 --- /dev/null +++ b/app/test/navigation/notification_route_test.dart @@ -0,0 +1,263 @@ +// app/test/navigation/notification_route_test.dart +import 'dart:convert'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:antgrid/models/ab_project.dart'; +import 'package:antgrid/models/recent_session_row.dart'; +import 'package:antgrid/models/session_entry.dart'; +import 'package:antgrid/models/session_target.dart'; +import 'package:antgrid/models/workspace_view.dart'; +import 'package:antgrid/navigation/notification_route.dart'; +import 'package:antgrid/providers/ui_attention_providers.dart'; + +const _localUuid = 'uuid-this-machine'; + +SessionEntry _session(String id, {int lastUsedAt = 0}) => SessionEntry( + id: id, + name: id.toUpperCase(), + createdAt: 0, + lastUsedAt: lastUsedAt, + archived: false, + running: false, +); + +AbProject _localProject(String projectId) => AbProject( + projectId: projectId, + folder: '/repos/$projectId', + displayName: projectId, + hostDeviceUuid: _localUuid, + hostMachineName: 'This Mac', + lastOpenedAt: DateTime.fromMillisecondsSinceEpoch(0), +); + +/// One local project and one remote project, each holding a distinct session: +/// the shape that makes a terminalId lookup meaningful across two machines. +List _known() => buildRecentSessions( + cached: { + 'projLocal': [_session('sess-local', lastUsedAt: 100)], + 'uuidA.projRemote': [_session('sess-remote', lastUsedAt: 200)], + }, + locals: [_localProject('projLocal')], + remotes: const [], + inventory: const [], + localDeviceLabel: 'This Mac', +); + +void main() { + group('resolveNotificationRoute', () { + test('a bare pre-resolved registrationId is a local project', () { + final loc = resolveNotificationRoute( + const NotificationRoute(registrationId: 'projLocal'), + known: _known(), + localDeviceUuid: _localUuid, + ); + expect(loc!.target, const LocalProject('projLocal')); + expect(loc.surface, WorkbenchSurface.workspace); + }); + + test('a compound pre-resolved registrationId is a remote project', () { + final loc = resolveNotificationRoute( + const NotificationRoute(registrationId: 'uuidA.projRemote'), + known: _known(), + localDeviceUuid: _localUuid, + ); + expect( + loc!.target, + const RemoteProject(machineUuid: 'uuidA', projectId: 'projRemote'), + ); + }); + + test('a registrationId no cached row names is still split', () { + final loc = resolveNotificationRoute( + const NotificationRoute(registrationId: 'uuidZ.projUnknown'), + known: _known(), + localDeviceUuid: _localUuid, + ); + expect( + loc!.target, + const RemoteProject(machineUuid: 'uuidZ', projectId: 'projUnknown'), + ); + }); + + test('machineUuid equal to this device resolves local', () { + final loc = resolveNotificationRoute( + const NotificationRoute( + machineUuid: _localUuid, + projectId: 'projLocal', + ), + known: _known(), + localDeviceUuid: _localUuid, + ); + expect(loc!.target, const LocalProject('projLocal')); + expect(loc.target!.registrationId, 'projLocal'); + }); + + test('a foreign machineUuid resolves remote', () { + final loc = resolveNotificationRoute( + const NotificationRoute(machineUuid: 'uuidB', projectId: 'projB'), + known: _known(), + localDeviceUuid: _localUuid, + ); + expect( + loc!.target, + const RemoteProject(machineUuid: 'uuidB', projectId: 'projB'), + ); + expect(loc.target!.registrationId, 'uuidB.projB'); + }); + + test('a null localDeviceUuid never makes a route local', () { + final loc = resolveNotificationRoute( + const NotificationRoute(machineUuid: 'uuidA', projectId: 'projRemote'), + known: _known(), + localDeviceUuid: null, + ); + expect( + loc!.target, + const RemoteProject(machineUuid: 'uuidA', projectId: 'projRemote'), + ); + }); + + test('a terminalId matching exactly one cached row names its project', () { + final loc = resolveNotificationRoute( + const NotificationRoute(terminalId: 'sess-remote'), + known: _known(), + localDeviceUuid: _localUuid, + ); + expect( + loc!.target, + const RemoteProject(machineUuid: 'uuidA', projectId: 'projRemote'), + ); + expect(loc.sessionId, 'sess-remote'); + }); + + test('a terminalId matching no cached row is unroutable', () { + expect( + resolveNotificationRoute( + const NotificationRoute(terminalId: 'sess-gone'), + known: _known(), + localDeviceUuid: _localUuid, + ), + isNull, + ); + }); + + test('a terminalId matching two machines is unroutable', () { + final known = buildRecentSessions( + cached: { + 'uuidA.proj': [_session('sess-dup')], + 'uuidB.proj': [_session('sess-dup')], + }, + locals: const [], + remotes: const [], + inventory: const [], + localDeviceLabel: 'This Mac', + ); + expect( + resolveNotificationRoute( + const NotificationRoute(terminalId: 'sess-dup'), + known: known, + localDeviceUuid: _localUuid, + ), + isNull, + ); + }); + + test('a projectId with no machineUuid and no terminalId is unroutable', () { + expect( + resolveNotificationRoute( + const NotificationRoute(projectId: 'projLocal'), + known: _known(), + localDeviceUuid: _localUuid, + ), + isNull, + ); + }); + + test('a route naming nothing is unroutable', () { + expect( + resolveNotificationRoute( + const NotificationRoute(sourceMessageId: 'm1', kind: 'handler'), + known: _known(), + localDeviceUuid: _localUuid, + ), + isNull, + ); + }); + + test('kind handler asks for the handler tab, agent asks for none', () { + WorkspaceView? viewFor(String? kind) => resolveNotificationRoute( + NotificationRoute(registrationId: 'projLocal', kind: kind), + known: _known(), + localDeviceUuid: _localUuid, + )!.view; + expect(viewFor('handler'), WorkspaceView.handler); + expect(viewFor('agent'), isNull); + expect(viewFor(null), isNull); + expect(viewFor('something-new'), isNull); + }); + + test('a blank id names nothing rather than a blank project', () { + expect( + resolveNotificationRoute( + const NotificationRoute(registrationId: ' '), + known: _known(), + localDeviceUuid: _localUuid, + ), + isNull, + ); + }); + }); + + group('encode/decode', () { + const full = NotificationRoute( + registrationId: 'uuidA.projRemote', + machineUuid: 'uuidA', + projectId: 'projRemote', + terminalId: 'sess-remote', + sourceMessageId: 'msg-1', + kind: 'handler', + ); + + test('round-trips every field', () { + expect(decodeNotificationRoute(encodeNotificationRoute(full)), full); + }); + + test('a null field is omitted, never encoded as an empty string', () { + const partial = NotificationRoute(registrationId: 'projLocal'); + final json = + jsonDecode(encodeNotificationRoute(partial)) as Map; + expect(json.keys, ['registrationId']); + expect( + decodeNotificationRoute(encodeNotificationRoute(partial)), + partial, + ); + }); + + test('an empty field decodes as absent', () { + expect( + decodeNotificationRoute('{"registrationId":"","kind":"agent"}'), + const NotificationRoute(kind: 'agent'), + ); + }); + + test('a non-string field decodes as absent', () { + expect( + decodeNotificationRoute('{"registrationId":7,"kind":"agent"}'), + const NotificationRoute(kind: 'agent'), + ); + }); + + test('refuses null, non-JSON and a non-object', () { + expect(decodeNotificationRoute(null), isNull); + expect(decodeNotificationRoute('not json'), isNull); + expect(decodeNotificationRoute('["projLocal"]'), isNull); + expect(decodeNotificationRoute(''), isNull); + }); + + test('equal routes share a hashCode, so a value dedup works', () { + final again = decodeNotificationRoute(encodeNotificationRoute(full))!; + expect(again.hashCode, full.hashCode); + expect(full, isNot(const NotificationRoute(registrationId: 'projLocal'))); + }); + }); +} diff --git a/app/test/navigation/pending_workspace_view_test.dart b/app/test/navigation/pending_workspace_view_test.dart index 113c847d..072e635f 100644 --- a/app/test/navigation/pending_workspace_view_test.dart +++ b/app/test/navigation/pending_workspace_view_test.dart @@ -7,6 +7,7 @@ import 'package:antgrid/models/pending_nav.dart'; import 'package:antgrid/models/session_target.dart'; import 'package:antgrid/models/workspace_view.dart'; +import 'package:antgrid/providers/sessions.dart'; import 'package:antgrid/providers/ui_attention_providers.dart'; import 'package:antgrid/providers/value_controller.dart'; import 'package:antgrid/providers/visible_surface.dart'; @@ -83,12 +84,30 @@ Future _withMobileShell( body: body, ); +/// A touch tablet: mobile PLATFORM at desktop WIDTH, which is its own layout +/// (`_buildTabletTouch`) and the one where "the agent panel is the default +/// zone" needs the most care — the context pane is a dock there, not an +/// overlay. +Future _withTabletShell( + WidgetTester tester, + Future Function(ProviderContainer container) body, +) => _withShell( + tester, + platform: TargetPlatform.android, + size: const Size(1400, 900), + body: body, +); + /// The harness leaves the selected target at its default, so a value the shell /// should honour carries that same stamp; anything else names a project this /// route is not. PendingNav _pending(WorkspaceView view) => (target: null, value: view); +/// The agent page carries no value of its own — the request IS the value — so +/// the stamp is the whole of it. +const PendingNav _pendingAgentPage = (target: null, value: true); + void main() { testWidgets('a view pending on desktop docks it beside the agent', ( tester, @@ -256,4 +275,210 @@ void main() { expect(container.read(pendingWorkspaceViewProvider), isNull); }); }); + + // The transcript is not a workspace tab, so a route that wants it hands over + // this provider instead — and on mobile that means moving the PageView back, + // which is the whole request when the user is looking at the workspace page. + testWidgets('an agent page pending on mobile moves the PageView back', ( + tester, + ) async { + await _withMobileShell(tester, (container) async { + container + .read(pendingWorkspaceViewProvider.notifier) + .set(_pending(WorkspaceView.git)); + await _settle(tester); + expect(container.read(agentSurfaceVisibleProvider), isFalse); + + container.read(pendingAgentPageProvider.notifier).set(_pendingAgentPage); + await _settle(tester); + + expect(container.read(agentSurfaceVisibleProvider), isTrue); + expect(find.byType(AgentPanel), findsOneWidget); + expect(container.read(pendingAgentPageProvider), isNull); + }); + }); + + // Same self-invalidation as the pending view: nothing rewrites the provider + // when the user leaves through the drawer instead. + testWidgets('an agent page pending for another project is spent unshown', ( + tester, + ) async { + await _withMobileShell(tester, (container) async { + container + .read(pendingWorkspaceViewProvider.notifier) + .set(_pending(WorkspaceView.git)); + await _settle(tester); + expect(container.read(agentSurfaceVisibleProvider), isFalse); + + container + .read(pendingAgentPageProvider.notifier) + .set((target: const LocalProject('somewhere-else'), value: true)); + await _settle(tester); + + expect(container.read(agentSurfaceVisibleProvider), isFalse); + expect(container.read(pendingAgentPageProvider), isNull); + }); + }); + + // Desktop in its ordinary split already has the agent panel on screen, so + // the request is honoured by spending it and moving nothing. + testWidgets('an agent page pending on desktop is spent, changing nothing', ( + tester, + ) async { + await _withDesktopShell(tester, (container) async { + container.read(pendingAgentPageProvider.notifier).set(_pendingAgentPage); + await _settle(tester); + + expect(find.byType(AgentPanel), findsOneWidget); + expect(container.read(visibleWorkspaceViewProvider), WorkspaceView.files); + expect(container.read(pendingAgentPageProvider), isNull); + }); + }); + + // "The agent panel is the default desktop zone" is false in the mode that + // drops it from the layout entirely — and the mode is per-session and + // restored on focus, so a route into a session the user left expanded would + // otherwise reveal nothing and spend the request doing it. + testWidgets('an agent page pending restores an expanded context panel', ( + tester, + ) async { + await _withDesktopShell(tester, (container) async { + // The panel's own tab-bar control, called rather than hunted for: which + // icon carries it is not what this test is about. + tester + .widget(find.byType(WorkspacePanel)) + .onToggleExpand!(); + await _settle(tester); + expect(find.byType(AgentPanel), findsNothing); + + container.read(pendingAgentPageProvider.notifier).set(_pendingAgentPage); + await _settle(tester); + + expect(find.byType(AgentPanel), findsOneWidget); + expect(container.read(pendingAgentPageProvider), isNull); + }); + }); + + // The touch tablet's context pane is a DOCK, not an overlay: open at a + // quarter of the width it leaves the transcript the other three, which is + // why `agentSurfaceVisibleProvider` reads that state as the agent being on + // screen. So the request has nothing to do, and closing the pane would take + // away the file or diff the user deliberately opened. + testWidgets('an agent page pending leaves an open tablet pane alone', ( + tester, + ) async { + await _withTabletShell(tester, (container) async { + container + .read(pendingWorkspaceViewProvider.notifier) + .set(_pending(WorkspaceView.git)); + await _settle(tester); + expect(container.read(visibleWorkspaceViewProvider), WorkspaceView.git); + expect(container.read(agentSurfaceVisibleProvider), isTrue); + + container.read(pendingAgentPageProvider.notifier).set(_pendingAgentPage); + await _settle(tester); + + expect(container.read(visibleWorkspaceViewProvider), WorkspaceView.git); + expect(container.read(agentSurfaceVisibleProvider), isTrue); + expect(container.read(pendingAgentPageProvider), isNull); + }); + }); + + // The one tablet state that does hide the transcript, and the mirror of the + // mouse desktop's contextExpanded → normal: un-expanded, never closed. + testWidgets('an agent page pending un-expands a tablet pane', (tester) async { + await _withTabletShell(tester, (container) async { + container + .read(pendingWorkspaceViewProvider.notifier) + .set(_pending(WorkspaceView.git)); + await _settle(tester); + tester + .widget(find.byType(WorkspacePanel)) + .onToggleExpand!(); + await _settle(tester); + // Squeezing the agent pane to nothing leaves `AgentBar`'s row narrower + // than its own content, which the framework reports as an overflow. That + // is a property of the expanded tablet state itself — it is why + // `agentSurfaceVisibleProvider` calls the transcript off screen there — + // and not of the reveal this test is about. + expect(tester.takeException(), isA()); + expect(container.read(agentSurfaceVisibleProvider), isFalse); + + container.read(pendingAgentPageProvider.notifier).set(_pendingAgentPage); + await _settle(tester); + + expect(container.read(agentSurfaceVisibleProvider), isTrue); + expect( + tester.widget(find.byType(WorkspacePanel)).isExpanded, + isFalse, + ); + // Still open on the view the user picked — only the width gave way. + expect(container.read(visibleWorkspaceViewProvider), WorkspaceView.git); + expect(container.read(pendingAgentPageProvider), isNull); + }); + }); + + // The cross-project applier seeds the queued session id, activates, and + // stamps this request all before the new project's list has landed. Spending + // it there spends it on the OLD session's layout: the per-session restore the + // resolution arms re-applies the target's own saved panel mode a frame later + // and hides the panel again, with the request already gone. + testWidgets('an agent page pending waits for a queued session id', ( + tester, + ) async { + await _withDesktopShell(tester, (container) async { + tester + .widget(find.byType(WorkspacePanel)) + .onToggleExpand!(); + await _settle(tester); + expect(find.byType(AgentPanel), findsNothing); + + container + .read(pendingActiveSessionIdProvider.notifier) + .set('not-resolved-yet'); + container.read(pendingAgentPageProvider.notifier).set(_pendingAgentPage); + await _settle(tester); + + expect(find.byType(AgentPanel), findsNothing); + expect( + container.read(pendingAgentPageProvider), + isNotNull, + reason: 'unspent, so a later rebuild can still honour it', + ); + + // The list lands and the bootstrap consumes the queued id. The rebuild + // that follows is what retries the drain — here a pending view stands in + // for the per-session restore's own setState. + container.read(pendingActiveSessionIdProvider.notifier).set(null); + container + .read(pendingWorkspaceViewProvider.notifier) + .set(_pending(WorkspaceView.files)); + await _settle(tester); + + expect(find.byType(AgentPanel), findsOneWidget); + expect(container.read(pendingAgentPageProvider), isNull); + }); + }); + + // Both halves of the handover written in ONE synchronous block, which is the + // state a drain sees when a navigation stamps a tab while an earlier agent + // request is still pending: neither may eat the other. + testWidgets('a view and an agent page pending together are both drained', ( + tester, + ) async { + await _withMobileShell(tester, (container) async { + container + .read(pendingWorkspaceViewProvider.notifier) + .set(_pending(WorkspaceView.git)); + container.read(pendingAgentPageProvider.notifier).set(_pendingAgentPage); + await _settle(tester); + + expect(container.read(pendingWorkspaceViewProvider), isNull); + expect(container.read(pendingAgentPageProvider), isNull); + // The agent drain runs last, so the page it asked for is where the user + // lands — and mobile publishes no workspace view from the agent page. + expect(container.read(agentSurfaceVisibleProvider), isTrue); + expect(container.read(visibleWorkspaceViewProvider), isNull); + }); + }); } diff --git a/app/test/providers/notification_attribution_test.dart b/app/test/providers/notification_attribution_test.dart new file mode 100644 index 00000000..5b5b38e9 --- /dev/null +++ b/app/test/providers/notification_attribution_test.dart @@ -0,0 +1,180 @@ +import 'package:antgrid/project/project_session.dart'; +import 'package:antgrid/project/project_session_registry.dart'; +import 'package:antgrid/providers/providers.dart'; +import 'package:antgrid/storage/cached_sessions_store.dart'; +import 'package:antgrid/test_helpers/fake_agent_transport.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import '../helpers/prefs_test_mock.dart'; + +Map _escalationJson(String escalationId, {int at = 1}) => { + 'escalationId': escalationId, + 'question': 'q', + 'reasoning': 'r', + 'draftReply': 'd', + 'urgency': 'normal', + 'at': at, +}; + +Map _statusJson( + String projectId, + List> escalations, +) => { + 'projectId': projectId, + 'sessions': [ + { + 'terminalId': 't1', + 'state': 'needs_you', + 'pendingEscalations': escalations.length, + 'armedAt': 0, + 'goal': 'summary', + 'backlog': const >[], + 'escalations': escalations, + }, + ], +}; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + setUp(useInMemoryPrefs); + + test('a fanned-in escalation names the project it came from', () async { + final transportA = FakeAgentTransport(); + final transportB = FakeAgentTransport(); + final cache = await CachedSessionsStore.open(); + addTearDown(cache.close); + final sessions = { + 'A': ProjectSession( + projectId: 'A', + transport: transportA, + mode: ProjectSessionMode.local, + cachedSessionsStore: cache, + onClose: () async => await transportA.dispose(), + ), + 'B': ProjectSession( + projectId: 'B', + transport: transportB, + mode: ProjectSessionMode.local, + cachedSessionsStore: cache, + onClose: () async => await transportB.dispose(), + ), + }; + for (final s in sessions.values) { + addTearDown(s.close); + } + + // Both projects already hold a pending escalation, so the provider's + // per-build seed has something to attribute as well as the live stream. + transportA.emit('handler:status', _statusJson('A', [ + _escalationJson('from-a'), + ])); + transportB.emit('handler:status', _statusJson('B', [ + _escalationJson('from-b'), + ])); + await Future.delayed(Duration.zero); + + final container = ProviderContainer( + overrides: [ + projectSessionProvider.overrideWith((ref, id) async => sessions[id]!), + ], + ); + addTearDown(container.dispose); + await container.read(projectSessionProvider('A').future); + await container.read(projectSessionProvider('B').future); + final registry = container.read(projectSessionRegistryProvider.notifier); + registry.touch('A', isLocal: true); + registry.touch('B', isLocal: true); + + final seen = <(String, String)>[]; + final sub = container.listen(handlerEscalationsProvider, (_, next) { + final scoped = next.value; + if (scoped == null) return; + seen.add((scoped.entryId, scoped.message.escalationId)); + }); + addTearDown(sub.close); + await Future.delayed(Duration.zero); + + expect(seen, containsAll([('A', 'from-a'), ('B', 'from-b')])); + + transportB.emit('handler:escalation', { + 'projectId': 'B', + 'escalationId': 'live-b', + 'terminalId': 't1', + 'question': 'q', + 'reasoning': 'r', + 'draftReply': 'd', + 'urgency': 'high', + }); + await Future.delayed(Duration.zero); + + expect(seen.last, ('B', 'live-b')); + }); + + test('a checkout that appears after the fan-out subscribed still names its ' + 'project', () async { + // The late-subscribe closure is nested one level deeper than the others, so + // it is the one place the loop's `id` can be captured wrong — hoist the + // handler out of the loop and every late checkout's notifications get + // attributed to whichever project the loop happened to end on. + final transportA = FakeAgentTransport(); + final transportB = FakeAgentTransport(); + final cache = await CachedSessionsStore.open(); + addTearDown(cache.close); + final sessions = { + 'A': ProjectSession( + projectId: 'A', + transport: transportA, + mode: ProjectSessionMode.local, + cachedSessionsStore: cache, + onClose: () async => await transportA.dispose(), + ), + 'B': ProjectSession( + projectId: 'B', + transport: transportB, + mode: ProjectSessionMode.local, + cachedSessionsStore: cache, + onClose: () async => await transportB.dispose(), + ), + }; + for (final s in sessions.values) { + addTearDown(s.close); + } + + final container = ProviderContainer( + overrides: [ + projectSessionProvider.overrideWith((ref, id) async => sessions[id]!), + ], + ); + addTearDown(container.dispose); + await container.read(projectSessionProvider('A').future); + await container.read(projectSessionProvider('B').future); + final registry = container.read(projectSessionRegistryProvider.notifier); + registry.touch('A', isLocal: true); + registry.touch('B', isLocal: true); + + final seen = <(String, String?)>[]; + final sub = container.listen(terminalNotificationsProvider, (_, next) { + final scoped = next.value; + if (scoped == null) return; + seen.add((scoped.entryId, scoped.message.title)); + }); + addTearDown(sub.close); + await Future.delayed(Duration.zero); + + // Created only now, so the bundle reaches the provider over + // checkoutServiceBundleStream rather than the initial listing. + sessions['B']!.servicesForCheckout('late-checkout'); + await Future.delayed(Duration.zero); + + transportB.emit('terminal:notification', { + 'checkoutId': 'late-checkout', + 'terminalId': 't1', + 'kind': 'osc9', + 'title': 'from-late-b', + }); + await Future.delayed(Duration.zero); + + expect(seen, [('B', 'from-late-b')]); + }); +} diff --git a/app/test/providers/notification_route_apply_test.dart b/app/test/providers/notification_route_apply_test.dart new file mode 100644 index 00000000..b4874928 --- /dev/null +++ b/app/test/providers/notification_route_apply_test.dart @@ -0,0 +1,677 @@ +// Applying a tapped notification's route is the one place several separately +// documented rules meet, and each of them fails silently: a pending session +// id written for the ALREADY-focused project is never drained and suppresses +// `reconcileActiveSession`'s fallback; a surface revealed by call rather than +// by handover is undone a frame later by the per-session UI restore; and a +// dedup store shared with the toast's own would make every tap a no-op. +import 'package:antgrid/models/ab_project.dart'; +import 'package:antgrid/models/drawer_entry.dart'; +import 'package:antgrid/models/recent_session_row.dart'; +import 'package:antgrid/models/session_entry.dart'; +import 'package:antgrid/models/session_target.dart'; +import 'package:antgrid/models/workspace_view.dart'; +import 'package:antgrid/navigation/nav_controller.dart'; +import 'package:antgrid/navigation/notification_route.dart'; +import 'package:antgrid/navigation/root_navigator.dart'; +import 'package:antgrid/providers/agent_transport.dart'; +import 'package:antgrid/providers/device_provisioning.dart'; +import 'package:antgrid/providers/drawer_entries.dart'; +import 'package:antgrid/providers/notification_route_apply.dart'; +import 'package:antgrid/providers/recent_sessions.dart'; +import 'package:antgrid/providers/sessions.dart'; +import 'package:antgrid/providers/ui_attention_providers.dart'; +import 'package:antgrid/providers/visible_surface.dart'; +import 'package:antgrid/services/sessions_service.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +// Riverpod 3 keeps `Override` out of the main barrel. +import 'package:flutter_riverpod/misc.dart' show Override; +import 'package:flutter_test/flutter_test.dart'; + +import '../helpers/prefs_test_mock.dart'; +import '../helpers/test_store_overrides.dart'; + +const _localUuid = 'this-machine'; +const _projectId = 'proj-1'; +const _sessionId = 'session-1'; + +SessionEntry _entry(String id, {bool deleting = false}) => SessionEntry( + id: id, + name: id, + createdAt: 0, + lastUsedAt: 0, + archived: false, + running: false, + deleting: deleting, +); + +RecentSessionRow _row(SessionEntry session, {String projectId = _projectId}) => + RecentSessionRow( + session: session, + origin: RecentOrigin( + isLocal: true, + registrationId: projectId, + projectId: projectId, + machineUuid: null, + projectName: projectId, + deviceName: 'this machine', + ), + ); + +/// [sessions] seeds both universes — the project's live list and the Recent +/// rows. [noLiveList] is the pre-bootstrap state, where no list has landed for +/// this project at all. +ProviderContainer _container({ + required List sessions, + SessionTarget? focused, + bool noLiveList = false, + List extraOverrides = const [], +}) { + final container = ProviderContainer( + overrides: [ + localDeviceUuidProvider.overrideWith((_) async => _localUuid), + recentSessionsProvider.overrideWithValue([ + for (final s in sessions) _row(s), + ]), + // The real one resolves a per-project session graph; the write guard on + // `activeSessionIdProvider` is the only thing that reads it here. + freshSessionsStateProvider.overrideWithValue( + noLiveList + ? null + : SessionsState(projectId: _projectId, sessions: sessions), + ), + ...extraOverrides, + ], + ); + addTearDown(container.dispose); + if (focused != null) { + container.read(selectedTargetProvider.notifier).set(focused); + } + return container; +} + +/// A context for the paths that reach `activateDrawerEntryById`, which takes a +/// widget context for its own snackbars and drawer pop. Nothing here renders +/// the app — the applier is driven against the container directly. +Future _someContext(WidgetTester tester) async { + await tester.pumpWidget(const SizedBox.shrink()); + return tester.element(find.byType(SizedBox)); +} + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + // The default desktop case, and the one that used to strand the app: a + // pending id here is never drained (the bootstrap's listener returns on an + // unchanged project) and makes `reconcileActiveSession` select null instead + // of falling back once the session leaves the list. + test('a focused-project route selects the session directly', () async { + final container = _container( + sessions: [_entry(_sessionId)], + focused: const LocalProject(_projectId), + ); + + final applied = await applyNotificationRoute( + null, + container, + const NotificationRoute( + registrationId: _projectId, + terminalId: _sessionId, + kind: 'agent', + ), + ); + + expect(applied, isTrue); + expect(container.read(activeSessionIdProvider), _sessionId); + expect(container.read(pendingActiveSessionIdProvider), isNull); + expect(container.read(pendingSessionStartSuppressedIdProvider), isNull); + expect( + container.read(workbenchSurfaceProvider), + WorkbenchSurface.workspace, + ); + }); + + // The transcript is not a workspace tab, so an agent-kind route hands over + // the agent page; a handler-kind one hands over the tab instead. Never both. + test('agent hands over the page, handler hands over the tab', () async { + final agent = _container( + sessions: [_entry(_sessionId)], + focused: const LocalProject(_projectId), + ); + await applyNotificationRoute( + null, + agent, + const NotificationRoute( + registrationId: _projectId, + terminalId: _sessionId, + kind: 'agent', + ), + ); + expect(agent.read(pendingAgentPageProvider), ( + target: const LocalProject(_projectId), + value: true, + )); + expect(agent.read(pendingWorkspaceViewProvider), isNull); + + final handler = _container( + sessions: [_entry(_sessionId)], + focused: const LocalProject(_projectId), + ); + await applyNotificationRoute( + null, + handler, + const NotificationRoute( + registrationId: _projectId, + terminalId: _sessionId, + kind: 'handler', + ), + ); + expect(handler.read(pendingWorkspaceViewProvider), ( + target: const LocalProject(_projectId), + value: WorkspaceView.handler, + )); + expect(handler.read(pendingAgentPageProvider), isNull); + }); + + // The write is silently refused for a session the bridge is deleting, so the + // applier reads it back — revealing that session's surface afterwards would + // aim the workspace at a transcript nobody is going to be shown. The project + // is still a destination, so the surface still moves: reporting success from + // the settings screen without leaving it explains nothing to the user. + test('a deleting session reaches the project but stamps no surface', () async { + final container = _container( + sessions: [_entry('other'), _entry(_sessionId, deleting: true)], + focused: const LocalProject(_projectId), + ); + container.read(activeSessionIdProvider.notifier).set('other'); + container + .read(workbenchSurfaceProvider.notifier) + .set(WorkbenchSurface.appSettings); + // A tab an earlier navigation left pending: this destination inherits + // nothing, so it is dropped rather than drained in the session's place. + container.read(pendingWorkspaceViewProvider.notifier).set(( + target: const LocalProject(_projectId), + value: WorkspaceView.git, + )); + + final applied = await applyNotificationRoute( + null, + container, + const NotificationRoute( + registrationId: _projectId, + terminalId: _sessionId, + kind: 'handler', + ), + ); + + expect(applied, isTrue); + expect(container.read(activeSessionIdProvider), 'other'); + expect( + container.read(workbenchSurfaceProvider), + WorkbenchSurface.workspace, + ); + expect(container.read(pendingWorkspaceViewProvider), isNull); + expect(container.read(pendingAgentPageProvider), isNull); + // Only the session-scoped half was skipped. History still has to name the + // place the user was just moved to, carrying the session actually in + // focus — otherwise `back()` re-applies the entry before this one and + // silently discards the move. + final nav = container.read(navControllerProvider); + expect(nav.current?.target, const LocalProject(_projectId)); + expect(nav.current?.surface, WorkbenchSurface.workspace); + expect(nav.current?.sessionId, 'other'); + }); + + // Only a session it can SEE as deleting is refused. An id the app does not + // recognise is written through by design ([ActiveSessionId]) — the list lands + // in stages, and a guard demanding presence would drop every selection made + // before it does, which is the ordinary case and the whole point of the wave. + test('a session named before any list landed is still selected', () async { + final container = _container( + sessions: [_entry(_sessionId)], + focused: const LocalProject(_projectId), + noLiveList: true, + ); + + final applied = await applyNotificationRoute( + null, + container, + const NotificationRoute( + registrationId: _projectId, + terminalId: _sessionId, + kind: 'agent', + ), + ); + + expect(applied, isTrue); + expect(container.read(activeSessionIdProvider), _sessionId); + }); + + testWidgets('a cross-project route opens the project and queues the session', ( + tester, + ) async { + useInMemoryPrefs(); + final stores = await buildTestStoreOverrides(); + addTearDown(stores.close); + final project = AbProject( + projectId: _projectId, + folder: '/repos/$_projectId', + displayName: _projectId, + hostDeviceUuid: _localUuid, + hostMachineName: 'This machine', + lastOpenedAt: DateTime.fromMillisecondsSinceEpoch(0), + ); + final container = _container( + sessions: [_entry(_sessionId)], + focused: const LocalProject('somewhere-else'), + extraOverrides: [ + ...stores.overrides, + // A local entry activates synchronously through `selectProject`, so the + // success path needs no navigator and no live transport. + drawerEntriesProvider.overrideWithValue([LocalProjectEntry(project)]), + ], + ); + // Both halves of the queued state, so the assertion is about the MECHANISM: + // nothing else in the suite proves the applier ever SETS the suppressor, + // and deleting that write would otherwise leave the suite green. + final queued = []; + final suppressed = []; + final subs = [ + container.listen(pendingActiveSessionIdProvider, (_, n) => queued.add(n)), + container.listen( + pendingSessionStartSuppressedIdProvider, + (_, n) => suppressed.add(n), + ), + ]; + addTearDown(() { + for (final s in subs) { + s.close(); + } + }); + + final applied = await applyNotificationRoute( + await _someContext(tester), + container, + const NotificationRoute( + registrationId: _projectId, + terminalId: _sessionId, + kind: 'handler', + ), + ); + + expect(applied, isTrue); + expect( + container.read(selectedTargetProvider), + const LocalProject(_projectId), + ); + expect(queued, [_sessionId]); + expect( + suppressed, + [_sessionId], + reason: 'a tap says show me, never resume — and it says it for THIS id', + ); + expect( + container.read(workbenchSurfaceProvider), + WorkbenchSurface.workspace, + ); + // Stamped with the target it just moved to, not the one it left. + expect(container.read(pendingWorkspaceViewProvider), ( + target: const LocalProject(_projectId), + value: WorkspaceView.handler, + )); + // `recordProjectFocus` stands down while a session activation is queued, so + // the applier's own commit is the only entry one tap records. + final nav = container.read(navControllerProvider); + expect(nav.current?.target, const LocalProject(_projectId)); + expect(nav.current?.sessionId, _sessionId); + expect(nav.past, isEmpty); + }); + + // A cross-project route queues the session for `_bootstrapSessions` instead + // of writing it, and the suppressor rides with that id — so an activation + // that never lands must put back exactly what it found. + testWidgets('a failed cross-project activation restores what it borrowed', ( + tester, + ) async { + final container = _container( + sessions: [_entry(_sessionId)], + focused: const LocalProject('somewhere-else'), + extraOverrides: [drawerEntriesProvider.overrideWithValue(const [])], + ); + // Another site's queued pick. The applier is borrowing this state, so a + // failure owes it back rather than nulling it. + container.read(pendingActiveSessionIdProvider.notifier).set('someone-elses'); + + // No drawer entry and a bare (non-compound) id, so neither the warm-refocus + // nor the cold-open path can do anything — the shape of a machine asleep. + final applied = await applyNotificationRoute( + await _someContext(tester), + container, + const NotificationRoute( + registrationId: _projectId, + terminalId: _sessionId, + kind: 'agent', + ), + ); + + expect(applied, isFalse); + expect(container.read(pendingActiveSessionIdProvider), 'someone-elses'); + expect(container.read(pendingSessionStartSuppressedIdProvider), isNull); + expect(container.read(activeSessionIdProvider), isNull); + }); + + // The toast is still on screen when this returns, so its retry has to be able + // to reach the applier again — a claim taken and burnt makes every later + // press a silent no-op. + testWidgets('a failed activation can be retried', (tester) async { + final container = _container( + sessions: [_entry(_sessionId)], + focused: const LocalProject('somewhere-else'), + extraOverrides: [drawerEntriesProvider.overrideWithValue(const [])], + ); + const route = NotificationRoute( + registrationId: _projectId, + terminalId: _sessionId, + sourceMessageId: 'msg-1', + ); + final context = await _someContext(tester); + + expect(await applyNotificationRoute(context, container, route), isFalse); + // The retry reaches the resolver again rather than the dedup. + final queued = []; + final sub = container.listen( + pendingActiveSessionIdProvider, + (_, next) => queued.add(next), + ); + addTearDown(sub.close); + expect(await applyNotificationRoute(context, container, route), isFalse); + expect(queued, isNotEmpty, reason: 'the second press got as far as queueing'); + }); + + // Nothing can dial a project without a context, so seeding the queued state + // there would clobber another site's pick to attempt nothing at all. + testWidgets('a context-less cross-project route queues nothing and stays ' + 'retryable', (tester) async { + final container = _container( + sessions: [_entry(_sessionId)], + focused: const LocalProject('somewhere-else'), + extraOverrides: [drawerEntriesProvider.overrideWithValue(const [])], + ); + final queued = []; + final sub = container.listen( + pendingActiveSessionIdProvider, + (_, next) => queued.add(next), + ); + addTearDown(sub.close); + const route = NotificationRoute( + registrationId: _projectId, + terminalId: _sessionId, + ); + + expect(await applyNotificationRoute(null, container, route), isFalse); + expect(queued, isEmpty); + + // The claim this path took was given back, so the same route reaches the + // activation once a live context arrives. A burnt one returns false at the + // dedup with nothing queued — indistinguishable from the answer above, and + // the reason that assertion alone does not pin the release. + expect( + await applyNotificationRoute( + await _someContext(tester), + container, + route, + ), + isFalse, + ); + expect(queued, isNotEmpty, reason: 'the retry got as far as queueing'); + }); + + // The claim doubles as the in-flight guard, so a THROW owes it back like + // every other exit: `localDeviceUuidProvider` is built to REJECT rather than + // stall on a keychain read error, and the toast runs the applier detached, so + // the throw is only logged. An unreleased claim there is a chip that can + // never work again for the life of the container. + test('a throwing apply leaves the route retryable', () async { + var failing = true; + final container = ProviderContainer( + overrides: [ + // An `Error`, not an `Exception`: Riverpod 3 does not retry Errors, so + // `.future` rejects here the way `noProviderRetry` makes production's + // every failure reject. + localDeviceUuidProvider.overrideWith((_) async { + if (failing) throw StateError('keychain unreadable'); + return _localUuid; + }), + recentSessionsProvider.overrideWithValue([_row(_entry(_sessionId))]), + freshSessionsStateProvider.overrideWithValue(null), + ], + ); + addTearDown(container.dispose); + container + .read(selectedTargetProvider.notifier) + .set(const LocalProject(_projectId)); + const route = NotificationRoute( + registrationId: _projectId, + terminalId: _sessionId, + sourceMessageId: 'msg-1', + ); + + await expectLater( + applyNotificationRoute(null, container, route), + throwsStateError, + ); + + failing = false; + container.invalidate(localDeviceUuidProvider); + + expect(await applyNotificationRoute(null, container, route), isTrue); + expect(container.read(activeSessionIdProvider), _sessionId); + }); + + // Applying a route is what unmounts the shell the toast's callback captured, + // so a dead element is the ordinary state by the time the applier reads it — + // and the app's one Navigator, which outlives every route, is what it falls + // back to. + testWidgets('an unmounted context falls back to the root navigator', ( + tester, + ) async { + useInMemoryPrefs(); + final stores = await buildTestStoreOverrides(); + addTearDown(stores.close); + final project = AbProject( + projectId: _projectId, + folder: '/repos/$_projectId', + displayName: _projectId, + hostDeviceUuid: _localUuid, + hostMachineName: 'This machine', + lastOpenedAt: DateTime.fromMillisecondsSinceEpoch(0), + ); + final container = _container( + sessions: [_entry(_sessionId)], + focused: const LocalProject('somewhere-else'), + extraOverrides: [ + ...stores.overrides, + drawerEntriesProvider.overrideWithValue([LocalProjectEntry(project)]), + ], + ); + Widget app(List children) => UncontrolledProviderScope( + container: container, + child: MaterialApp( + navigatorKey: container.read(rootNavigatorKeyProvider), + home: Column(children: children), + ), + ); + + await tester.pumpWidget(app(const [Text('toast anchor')])); + final toastContext = tester.element(find.text('toast anchor')); + // The Navigator carries the key across this rebuild; only the anchor goes. + await tester.pumpWidget(app(const [])); + expect(toastContext.mounted, isFalse); + + final applied = await applyNotificationRoute( + toastContext, + container, + const NotificationRoute( + registrationId: _projectId, + terminalId: _sessionId, + ), + ); + + expect(applied, isTrue); + expect( + container.read(selectedTargetProvider), + const LocalProject(_projectId), + ); + }); + + test('the same sourceMessageId applies once', () async { + final container = _container( + sessions: [_entry(_sessionId), _entry('later')], + focused: const LocalProject(_projectId), + ); + const first = NotificationRoute( + registrationId: _projectId, + terminalId: _sessionId, + sourceMessageId: 'msg-1', + ); + // Same id, different session: the second delivery of one notification, not + // a second notification. + const second = NotificationRoute( + registrationId: _projectId, + terminalId: 'later', + sourceMessageId: 'msg-1', + ); + + expect(await applyNotificationRoute(null, container, first), isTrue); + expect(await applyNotificationRoute(null, container, second), isFalse); + expect(container.read(activeSessionIdProvider), _sessionId); + }); + + // `sourceMessageId` is nullable by design, and the two iOS cold-start entries + // can both fire for one tap — so the value has to be a key of its own. + test('an id-less route applied twice applies once', () async { + final container = _container( + sessions: [_entry(_sessionId)], + focused: const LocalProject(_projectId), + ); + const route = NotificationRoute( + registrationId: _projectId, + terminalId: _sessionId, + ); + + expect(await applyNotificationRoute(null, container, route), isTrue); + expect(await applyNotificationRoute(null, container, route), isFalse); + }); + + // Dedup is per container, not per route shape: a genuinely different + // notification about the same session still lands. + test('a different route is not swallowed by the dedup', () async { + final container = _container( + sessions: [_entry(_sessionId), _entry('later')], + focused: const LocalProject(_projectId), + ); + + expect( + await applyNotificationRoute( + null, + container, + const NotificationRoute( + registrationId: _projectId, + terminalId: _sessionId, + ), + ), + isTrue, + ); + expect( + await applyNotificationRoute( + null, + container, + const NotificationRoute( + registrationId: _projectId, + terminalId: 'later', + ), + ), + isTrue, + ); + expect(container.read(activeSessionIdProvider), 'later'); + }); + + // The dedup only closes IDENTICAL routes. Two DIFFERENT ones overlapping is + // the case that corrupts focus: `activateDrawerEntryById` restores a prior + // target after its await, so the loser hands back one the winner had left. + test('a second route while one is in flight is refused', () async { + final container = _container( + sessions: [_entry(_sessionId), _entry('later')], + focused: const LocalProject(_projectId), + ); + + final first = applyNotificationRoute( + null, + container, + const NotificationRoute( + registrationId: _projectId, + terminalId: _sessionId, + ), + ); + // Started before the first has resumed past its `localDeviceUuid` await. + final second = applyNotificationRoute( + null, + container, + const NotificationRoute(registrationId: _projectId, terminalId: 'later'), + ); + + expect(await first, isTrue); + expect(await second, isFalse); + expect(container.read(activeSessionIdProvider), _sessionId); + }); + + // Unroutable is a real answer: a pre-W1 bridge sealed no machineUuid, and a + // projectId alone names no machine (`computeProjectId` hashes the path). + test('a route that resolves to nothing changes no state', () async { + final container = _container( + sessions: [_entry(_sessionId)], + focused: const LocalProject(_projectId), + ); + + final applied = await applyNotificationRoute( + null, + container, + const NotificationRoute(projectId: _projectId, kind: 'handler'), + ); + + expect(applied, isFalse); + expect(container.read(activeSessionIdProvider), isNull); + expect(container.read(pendingWorkspaceViewProvider), isNull); + expect(container.read(pendingAgentPageProvider), isNull); + }); + + // The Recent rows a terminalId is matched against hydrate asynchronously and + // the toast outlives that, so a tap arriving early must leave the route + // spendable — a claim burnt here makes every later press a silent no-op. + test('a route unroutable before the rows land can be applied again', () async { + var rows = []; + final container = ProviderContainer( + overrides: [ + localDeviceUuidProvider.overrideWith((_) async => _localUuid), + recentSessionsProvider.overrideWith((ref) => rows), + freshSessionsStateProvider.overrideWithValue(null), + ], + ); + addTearDown(container.dispose); + container + .read(selectedTargetProvider.notifier) + .set(const LocalProject(_projectId)); + const route = NotificationRoute( + terminalId: _sessionId, + sourceMessageId: 'msg-1', + ); + + expect(await applyNotificationRoute(null, container, route), isFalse); + + rows = [_row(_entry(_sessionId))]; + container.invalidate(recentSessionsProvider); + + expect(await applyNotificationRoute(null, container, route), isTrue); + expect(container.read(activeSessionIdProvider), _sessionId); + }); +} diff --git a/app/test/screens/notification_route_start_suppression_test.dart b/app/test/screens/notification_route_start_suppression_test.dart new file mode 100644 index 00000000..7ca3a8d1 --- /dev/null +++ b/app/test/screens/notification_route_start_suppression_test.dart @@ -0,0 +1,202 @@ +// Reusing the pending-session-id handover for a notification tap is not a pure +// focus move: its drain auto-starts a stopped session, because a Recent-list +// tap means "resume this". A notification tap means "show me what happened", and +// restarting an agent the user let finish spends tokens nobody asked for — so +// the suppressor NAMES the queued id it speaks for. Five other sites queue an id +// without knowing it exists, and the bootstrap can return early past the point +// one is set, so a bare flag would eventually answer for one of theirs. +import 'package:antgrid/providers/relay_error_banner.dart'; +import 'package:antgrid/providers/sessions.dart'; +import 'package:antgrid/providers/value_controller.dart'; +import 'package:antgrid/test_helpers/fake_agent_transport.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import '../helpers/workspace_shell_harness.dart'; + +Map _stopped(String id) => { + 'id': id, + 'name': id, + 'createdAt': 0, + 'lastUsedAt': 0, + 'archived': false, + 'running': false, + 'mode': 'terminal', +}; + +/// Answers the bootstrap's OWN `session:list` — the transport's hydrator sends +/// one too, with no pending reply behind it. +void _answerList(FakeAgentTransport transport, String sessionId) { + final list = transport.sent.lastWhere((m) => m['type'] == 'session:list'); + transport.emit('session:list:result', { + 'requestId': list['requestId'], + 'sessions': [_stopped(sessionId)], + }); +} + +void main() { + testWidgets('a suppressed pending id focuses without starting', ( + tester, + ) async { + final transport = FakeAgentTransport(); + final container = await pumpWorkspaceShell( + tester, + transport: (_) => transport, + extraOverrides: [ + pendingActiveSessionIdProvider.overrideWith( + () => ValueController('session-1'), + ), + pendingSessionStartSuppressedIdProvider.overrideWith( + () => ValueController('session-1'), + ), + ], + ); + await tester.pump(); + await tester.pump(); + + _answerList(transport, 'session-1'); + await tester.pump(); + + expect(container.read(activeSessionIdProvider), 'session-1'); + expect(transport.sent.where((m) => m['type'] == 'session:start'), isEmpty); + // Cleared with the id it rode in on, so the next queued pick starts from + // the Recent-list default rather than inheriting this one. + expect(container.read(pendingSessionStartSuppressedIdProvider), isNull); + expect(container.read(pendingActiveSessionIdProvider), isNull); + + // The list result schedules the session cache's debounced flush; leaving it + // pending fails the test on the binding's timer check. + await tester.pump(const Duration(seconds: 1)); + }); + + testWidgets('an unsuppressed pending id still starts the session', ( + tester, + ) async { + final transport = FakeAgentTransport(); + final container = await pumpWorkspaceShell( + tester, + transport: (_) => transport, + extraOverrides: [ + pendingActiveSessionIdProvider.overrideWith( + () => ValueController('session-1'), + ), + ], + ); + await tester.pump(); + await tester.pump(); + + _answerList(transport, 'session-1'); + await tester.pump(); + + expect(container.read(activeSessionIdProvider), 'session-1'); + expect( + transport.sent.where((m) => m['type'] == 'session:start'), + isNotEmpty, + reason: 'a Recent-list tap on a stopped session means resume it', + ); + + // The start is never answered; past the service's 15s bound it fails, and + // the detached bootstrap must survive that (see + // workspace_shell_bootstrap_timeout_test.dart). + await tester.pump(const Duration(seconds: 20)); + }); + + // The named session is gone from the list, so the pending branch falls + // THROUGH to the default pick — and that pick auto-starts. A tap asking to + // see a session the bridge has since deleted must not resume an unrelated + // agent instead: it is the same token spend, arrived at by the path where the + // user's intent is furthest from a resume. + testWidgets('a suppressed pending id that no longer exists starts nothing', ( + tester, + ) async { + final transport = FakeAgentTransport(); + final container = await pumpWorkspaceShell( + tester, + transport: (_) => transport, + extraOverrides: [ + pendingActiveSessionIdProvider.overrideWith( + () => ValueController('session-gone'), + ), + pendingSessionStartSuppressedIdProvider.overrideWith( + () => ValueController('session-gone'), + ), + ], + ); + await tester.pump(); + await tester.pump(); + + _answerList(transport, 'session-1'); + await tester.pump(); + + expect(container.read(activeSessionIdProvider), 'session-1'); + expect(transport.sent.where((m) => m['type'] == 'session:start'), isEmpty); + + await tester.pump(const Duration(seconds: 1)); + }); + + // The one early return that drops a queued id without a drain having + // consumed it. Left set, the suppressor outlives the run that wrote it and + // eats the next Recent-list tap on that same session. + testWidgets('a failed session list clears the suppressor with the id', ( + tester, + ) async { + final transport = FakeAgentTransport(); + final container = await pumpWorkspaceShell( + tester, + transport: (_) => transport, + extraOverrides: [ + pendingActiveSessionIdProvider.overrideWith( + () => ValueController('session-1'), + ), + pendingSessionStartSuppressedIdProvider.overrideWith( + () => ValueController('session-1'), + ), + ], + ); + await tester.pump(); + await tester.pump(); + + // Never answered: past the service's 15s pending-reply bound the request + // fails, which is the branch that raises the SESSIONS banner. + await tester.pump(const Duration(seconds: 20)); + + expect(container.read(relayErrorBannerProvider)?.code, 'SESSIONS'); + expect(container.read(pendingActiveSessionIdProvider), isNull); + expect(container.read(pendingSessionStartSuppressedIdProvider), isNull); + }); + + // The one a bare flag could not survive: a suppressor left behind by a run + // that returned early, meeting the NEXT tap's queued id. It names a session + // nobody is resolving, so it must not speak for this one. + testWidgets('a suppressor naming another session does not suppress', ( + tester, + ) async { + final transport = FakeAgentTransport(); + final container = await pumpWorkspaceShell( + tester, + transport: (_) => transport, + extraOverrides: [ + pendingActiveSessionIdProvider.overrideWith( + () => ValueController('session-1'), + ), + pendingSessionStartSuppressedIdProvider.overrideWith( + () => ValueController('a-session-nobody-is-resolving'), + ), + ], + ); + await tester.pump(); + await tester.pump(); + + _answerList(transport, 'session-1'); + await tester.pump(); + + expect(container.read(activeSessionIdProvider), 'session-1'); + expect( + transport.sent.where((m) => m['type'] == 'session:start'), + isNotEmpty, + reason: 'a stale suppressor must not eat a Recent-list tap resume', + ); + expect(container.read(pendingSessionStartSuppressedIdProvider), isNull); + + await tester.pump(const Duration(seconds: 20)); + }); +} diff --git a/app/test/screens/notification_toast_action_test.dart b/app/test/screens/notification_toast_action_test.dart new file mode 100644 index 00000000..248eae29 --- /dev/null +++ b/app/test/screens/notification_toast_action_test.dart @@ -0,0 +1,210 @@ +// A toast about another session is only useful if it is a way to get to it. +// The half that breaks silently is the surface: revealing the handler tab by +// CALL after a session change is undone a frame later by the shell's +// per-session UI restore, so the route has to hand the tab over as pending +// state and let the drain apply it. +import 'dart:async'; + +import 'package:antgrid/design/widgets/ab_toast.dart'; +import 'package:antgrid/models/handler_state.dart' show HandlerEscalation; +import 'package:antgrid/models/pending_nav.dart'; +import 'package:antgrid/models/session_target.dart'; +import 'package:antgrid/models/workspace_view.dart'; +import 'package:antgrid/providers/agent_transport.dart'; +import 'package:antgrid/providers/providers.dart'; +import 'package:antgrid/providers/sessions.dart'; +import 'package:antgrid/providers/visible_surface.dart'; +import 'package:flutter/foundation.dart'; +import 'package:flutter/widgets.dart' show Size; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import '../helpers/workspace_shell_harness.dart'; + +/// The compound entry the harness focuses, split the way the drawer keys it. +const _machineUuid = 'agent-123'; +const _projectId = 'test-project'; +const _entryId = '$_machineUuid.$_projectId'; +const _target = RemoteProject(machineUuid: _machineUuid, projectId: _projectId); + +const _escalation = HandlerEscalation( + escalationId: 'esc-1', + terminalId: 'session-9', + question: 'Should I force-push?', + reasoning: 'r', + draftReply: 'd', + urgency: 'high', + at: 1, +); + +/// Bounded pumps: the shell always has something animating, so it never +/// settles. +Future _settle(WidgetTester tester) async { + for (var i = 0; i < 6; i++) { + await tester.pump(const Duration(milliseconds: 200)); + } +} + +/// The platform override must be cleared inside the test body — the binding +/// asserts every foundation debug variable is unset before tearDown runs. +Future _withShell( + WidgetTester tester, + Stream<({String entryId, HandlerEscalation message})> escalations, + Future Function(ProviderContainer container) body, +) async { + debugDefaultTargetPlatformOverride = TargetPlatform.windows; + tester.view.physicalSize = const Size(1400, 900); + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.reset); + try { + final container = await pumpWorkspaceShell( + tester, + extraOverrides: [ + handlerEscalationsProvider.overrideWith((ref) => escalations), + ], + ); + // The applier compares the resolved target against this; the harness + // leaves it unset while overriding the registration id directly. + container.read(selectedTargetProvider.notifier).set(_target); + await _settle(tester); + await body(container); + } finally { + debugDefaultTargetPlatformOverride = null; + } +} + +void main() { + testWidgets('an escalation toast opens the session it came from', ( + tester, + ) async { + await _withShell(tester, Stream.value(( + entryId: _entryId, + message: _escalation, + )), (container) async { + // Every write of the pending view, so the assertion is about the + // MECHANISM and not only its outcome — the drain clears it again. + final handedOver = >[]; + final sub = container.listen(pendingWorkspaceViewProvider, (_, next) { + if (next != null) handedOver.add(next); + }); + addTearDown(sub.close); + + final toast = tester.widget(find.byType(AbToast)); + expect(toast.actionLabel, 'Open'); + + await tester.tap(find.text('Open')); + await _settle(tester); + + expect(container.read(activeSessionIdProvider), _escalation.terminalId); + expect(handedOver, [(target: _target, value: WorkspaceView.handler)]); + // Honoured by the drain, which is what a direct reveal would have lost. + expect( + container.read(visibleWorkspaceViewProvider), + WorkspaceView.handler, + ); + // A same-project route never queues the session id: nothing would drain + // one here, and while set it makes `reconcileActiveSession` select null. + expect(container.read(pendingActiveSessionIdProvider), isNull); + + // Outlive the toast's own timer so nothing fires past the test. + await tester.pump(const Duration(seconds: 12)); + }); + }); + + testWidgets('a second tap on the same toast applies once', (tester) async { + await _withShell(tester, Stream.value(( + entryId: _entryId, + message: _escalation, + )), (container) async { + await tester.tap(find.text('Open')); + await _settle(tester); + container.read(activeSessionIdProvider.notifier).set(null); + + // Past `showAbToastOverlay`'s 4s default and well inside the 8s this + // toast asked for: the second tap only reaches a chip that is still on + // screen, so the two halves — the longer duration and the dedup that has + // to absorb what it makes possible — are pinned together. + // ~2.4s has already elapsed in the settles above, so this lands near 7s: + // past a 6s toast, inside the 8s one, which is the window that pins the + // duration from both sides rather than only against the 4s default. + await tester.pump(const Duration(milliseconds: 4500)); + expect(find.text('Open'), findsOneWidget); + + // The action cannot dismiss its own toast, so it stays pressable for the + // whole 8s — the applier's dedup is what absorbs the second press. + await tester.tap(find.text('Open')); + await _settle(tester); + + expect(container.read(activeSessionIdProvider), isNull); + + await tester.pump(const Duration(seconds: 12)); + }); + }); + + // The complement of the case above, and what keeps that one honest: without a + // sourceMessageId every route about one session is value-identical, so the + // dedup that swallows a re-tap would swallow the NEXT escalation too — the + // second and every later Open on that session, permanently. + testWidgets('a later escalation on the same session opens it again', ( + tester, + ) async { + const second = HandlerEscalation( + escalationId: 'esc-2', + terminalId: 'session-9', + question: 'And now?', + reasoning: 'r', + draftReply: 'd', + urgency: 'high', + at: 2, + ); + final controller = + StreamController<({String entryId, HandlerEscalation message})>.broadcast(); + addTearDown(controller.close); + + await _withShell(tester, controller.stream, (container) async { + controller.add((entryId: _entryId, message: _escalation)); + await _settle(tester); + await tester.tap(find.text('Open')); + await _settle(tester); + expect(container.read(activeSessionIdProvider), 'session-9'); + container.read(activeSessionIdProvider.notifier).set(null); + await tester.pump(const Duration(seconds: 12)); + + controller.add((entryId: _entryId, message: second)); + await _settle(tester); + await tester.tap(find.text('Open')); + await _settle(tester); + + expect(container.read(activeSessionIdProvider), 'session-9'); + await tester.pump(const Duration(seconds: 12)); + }); + }); + + // The action is offered only when the route RESOLVES, not merely when an + // entryId is present: a chip that opens nothing is worse than no chip. + testWidgets('an unroutable notification keeps the plain toast', ( + tester, + ) async { + const blank = HandlerEscalation( + escalationId: 'esc-blank', + terminalId: 'session-9', + question: 'Whose project is this?', + reasoning: 'r', + draftReply: 'd', + urgency: 'high', + at: 1, + ); + await _withShell(tester, Stream.value((entryId: ' ', message: blank)), ( + container, + ) async { + final toast = tester.widget(find.byType(AbToast)); + expect(toast.actionLabel, isNull); + expect(find.text('Open'), findsNothing); + + // The plain toast keeps `showAbToastOverlay`'s 4s default; only the + // actionable one is held open long enough to be reached for. + await tester.pump(const Duration(seconds: 5)); + expect(find.byType(AbToast), findsNothing); + }); + }); +} diff --git a/app/test/services/notification_tap_test.dart b/app/test/services/notification_tap_test.dart new file mode 100644 index 00000000..fa785cb6 --- /dev/null +++ b/app/test/services/notification_tap_test.dart @@ -0,0 +1,31 @@ +// The two platform gates around the OS tap entry points. Neither is reachable +// from a widget test — one registers a plugin callback, the other runs before +// the first frame — so without this they are deletable with every gate green, +// and each deletion is a silent field bug: a double-delivered Android tap, or a +// launch-details call that throws on Linux and replays itself on Windows. +import 'package:antgrid/services/notification_tap.dart'; +import 'package:flutter/foundation.dart' show TargetPlatform; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + test('the push tap is registered on iOS alone', () { + for (final p in TargetPlatform.values) { + expect( + pushTapRegistrationSupported(p), + p == TargetPlatform.iOS, + reason: '$p', + ); + } + }); + + test('launch details are read everywhere fln implements them', () { + expect(launchDetailsSupported(TargetPlatform.iOS), isTrue); + expect(launchDetailsSupported(TargetPlatform.android), isTrue); + expect(launchDetailsSupported(TargetPlatform.macOS), isTrue); + // The two that misbehave rather than merely lack a branch: Linux throws + // UnimplementedError, Windows replays a tap it already delivered. + expect(launchDetailsSupported(TargetPlatform.linux), isFalse); + expect(launchDetailsSupported(TargetPlatform.windows), isFalse); + expect(launchDetailsSupported(TargetPlatform.fuchsia), isFalse); + }); +} diff --git a/app/test/services/push_background_handler_test.dart b/app/test/services/push_background_handler_test.dart index e3e080d6..9bc94d3c 100644 --- a/app/test/services/push_background_handler_test.dart +++ b/app/test/services/push_background_handler_test.dart @@ -1,6 +1,7 @@ import 'dart:convert'; import 'package:cryptography/cryptography.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:antgrid/navigation/notification_route.dart'; import 'package:antgrid/services/push_identity.dart'; import 'package:antgrid/services/push_background_handler.dart'; import 'package:push/push.dart'; @@ -77,6 +78,175 @@ void main() { expect(decoded.sourceMessageId, 'm9'); }); + test('decodePush carries machineUuid and terminalId through', () async { + final identity = PushIdentity.inMemory(); + final kp = await identity.ensureKeypair(); + final box = await _sealTo(kp.pubkeyB64, { + 'title': 'Task complete', + 'body': 'done', + 'projectId': 'proj-42', + 'machineUuid': 'machine-1', + 'terminalId': 'sess-7', + }); + final decoded = await decodePush(box, pushIdentity: identity); + expect(decoded!.machineUuid, 'machine-1'); + expect(decoded.terminalId, 'sess-7'); + }); + + test('decodePush maps absent routing ids to null', () async { + final identity = PushIdentity.inMemory(); + final kp = await identity.ensureKeypair(); + final box = await _sealTo(kp.pubkeyB64, {'title': 'x', 'body': 'y'}); + final decoded = await decodePush(box, pushIdentity: identity); + expect(decoded!.machineUuid, isNull); + expect(decoded.terminalId, isNull); + expect(decoded.projectId, isNull); + }); + + test('decodePush maps blank routing ids to null', () async { + final identity = PushIdentity.inMemory(); + final kp = await identity.ensureKeypair(); + final box = await _sealTo(kp.pubkeyB64, { + 'title': 'x', + 'body': 'y', + 'projectId': '', + 'machineUuid': '', + 'terminalId': '', + }); + final decoded = await decodePush(box, pushIdentity: identity); + // '' would satisfy a `!= null` test and address a project nobody has. + expect(decoded!.projectId, isNull); + expect(decoded.machineUuid, isNull); + expect(decoded.terminalId, isNull); + }); + + test('decodePush survives a routing id of the wrong type', () async { + final identity = PushIdentity.inMemory(); + final kp = await identity.ensureKeypair(); + final box = await _sealTo(kp.pubkeyB64, { + 'title': 'Task complete', + 'body': 'done', + 'machineUuid': 7, + 'projectId': 'proj-42', + }); + final decoded = await decodePush(box, pushIdentity: identity); + // A cast would throw into decodePush's catch and drop the whole alert. + expect(decoded, isNotNull); + expect(decoded!.title, 'Task complete'); + expect(decoded.machineUuid, isNull); + expect(decoded.projectId, 'proj-42'); + }); + + group('routeOfPush', () { + test('addresses a project by machine + project', () { + const decoded = ( + title: 't', + body: 'b', + kind: 'handler', + projectId: 'proj-42', + machineUuid: 'machine-1', + terminalId: 'sess-7', + sourceMessageId: 'm9', + ); + expect( + routeOfPush(decoded), + const NotificationRoute( + machineUuid: 'machine-1', + projectId: 'proj-42', + terminalId: 'sess-7', + sourceMessageId: 'm9', + kind: 'handler', + ), + ); + }); + + test('a session alone is still addressable', () { + const decoded = ( + title: 't', + body: 'b', + kind: null, + projectId: null, + machineUuid: null, + terminalId: 'sess-7', + sourceMessageId: null, + ); + expect(routeOfPush(decoded)?.terminalId, 'sess-7'); + }); + + test('a project without its machine names nothing', () { + const decoded = ( + title: 't', + body: 'b', + kind: null, + projectId: 'proj-42', + machineUuid: null, + terminalId: null, + sourceMessageId: 'm9', + ); + // The same repo at the same path on two machines mints one projectId. + expect(routeOfPush(decoded), isNull); + }); + + test('a pre-W1 bridge payload names nothing', () { + const decoded = ( + title: 't', + body: 'b', + kind: 'agent', + projectId: null, + machineUuid: null, + terminalId: null, + sourceMessageId: 'm9', + ); + expect(routeOfPush(decoded), isNull); + }); + + // The step `pushBackgroundHandler` performs before handing the payload to + // the OS. FCM is data-only, so that handler renders EVERY Android + // background push — if what it seals does not survive the round trip, no + // Android notification is tappable-to-route at all, and nothing else in + // this suite would notice. + test('the sealed payload survives the round trip a tap makes', () async { + final identity = PushIdentity.inMemory(); + final kp = await identity.ensureKeypair(); + final decoded = await decodePush( + await _sealTo(kp.pubkeyB64, { + 'title': 'Handler needs you', + 'body': 'Deploy?', + 'kind': 'handler', + 'projectId': 'proj-42', + 'machineUuid': 'machine-1', + 'terminalId': 'sess-7', + 'sourceMessageId': 'e1', + }), + pushIdentity: identity, + ); + final route = routeOfPush(decoded!); + expect( + decodeNotificationRoute(encodeNotificationRoute(route!)), + route, + ); + }); + }); + + // Blank and whitespace collapse to absent the way `namedOrNull` does on the route + // side: an id only one of the two predicates accepts survives decoding and + // then addresses nothing. + test('decodePush drops a whitespace-only routing id', () async { + final identity = PushIdentity.inMemory(); + final kp = await identity.ensureKeypair(); + final decoded = await decodePush( + await _sealTo(kp.pubkeyB64, { + 'title': 't', + 'body': 'b', + 'machineUuid': ' ', + 'projectId': 'proj-42', + }), + pushIdentity: identity, + ); + expect(decoded!.machineUuid, isNull); + expect(routeOfPush(decoded), isNull); + }); + test('decodePush maps a missing/empty sourceMessageId to null', () async { final identity = PushIdentity.inMemory(); final kp = await identity.ensureKeypair(); @@ -91,6 +261,8 @@ void main() { body: 'b', kind: null, projectId: null, + machineUuid: null, + terminalId: null, sourceMessageId: 'src1', ); expect(pushDedupKey(withSrc), 'src1'); @@ -100,6 +272,8 @@ void main() { body: 'b', kind: null, projectId: null, + machineUuid: null, + terminalId: null, sourceMessageId: null, ); // No id → null so the caller shows the push rather than deduping it away. diff --git a/app/test/widgets/handler/handler_header_pill_test.dart b/app/test/widgets/handler/handler_header_pill_test.dart index 363c4fe1..4cdc949c 100644 --- a/app/test/widgets/handler/handler_header_pill_test.dart +++ b/app/test/widgets/handler/handler_header_pill_test.dart @@ -242,6 +242,40 @@ void main() { expect(revealed, isFalse); }); + // The control lands on the handler tab by handover or by call, and the + // agent-page drain runs LAST — so a stamp a notification route left + // pending would override the tab on the very frame this opens it. + testWidgets('drops an agent-page stamp an earlier route left pending', ( + tester, + ) async { + final container = await pumpWithContainer( + tester, + { + 't1': _session('t1', runState: HandlerRunState.watching), + 't2': _session( + 't2', + runState: HandlerRunState.needsYou, + pendingEscalations: 1, + ), + }, + focused: 't1', + onReveal: () {}, + ); + container.read(pendingAgentPageProvider.notifier).set(( + target: null, + value: true, + )); + + await tester.tap(find.text('NEEDS YOU 1')); + await tester.pump(); + + expect(container.read(pendingAgentPageProvider), isNull); + expect( + container.read(pendingWorkspaceViewProvider)?.value, + WorkspaceView.handler, + ); + }); + testWidgets('leaves focus alone when the focused session is the one ' 'waiting', (tester) async { var revealed = false; diff --git a/bridge/CLAUDE.md b/bridge/CLAUDE.md index da74da3b..33f69341 100644 --- a/bridge/CLAUDE.md +++ b/bridge/CLAUDE.md @@ -167,7 +167,7 @@ already has them. - `relay-client.ts` — the ONE machine WebSocket (a machine holds exactly one `RelayClient`, owned by `HostServer`). v3 auth: single signed `hello` (epoch from `relay-epoch.ts`, persisted at `/relay-epoch`, minted once per process); `welcome` = authenticated — backoff resets ONLY there, equal jitter applied to the scheduled delay only. `getLicenseToken` runs before each (re)connect. Terminal-vs-retryable is the v3 error contract (last error frame's `retryable:false` → stop reconnecting; stream-scoped `ref` errors never count): `onAuthRevoked` fires only on the identity-dead verdicts `LICENSE_INVALID|LICENSE_REVOKED` (`LICENSE_AUTH_DEAD`; index.ts emits `{"event":"auth_revoked"}` to stderr). `LICENSE_EXPIRED` is deliberately NOT one — it is recoverable by time, so it takes the plain terminal path (stop reconnecting) while token maintenance keeps re-minting, and `onMinted` → `redialWithFreshToken()` brings the socket back on the first fresh mint with no process restart. `SUPERSEDED` = another socket now holds our deviceId — either a newer instance of ourselves, or (equal epoch, same key) our own redial evicting the half-open socket we just abandoned, which is the ordinary watchdog path and lands on the dead socket; log + stop reconnecting on THIS socket, never auth_revoked. Clock-skew self-heal: `error{AUTH_FAILED, serverTime}` → offset applied to the next hello's `ts`, once per offset value. E2E session is reactive, acked, make-before-break: kind-byte dispatch (0x01 = handshake plaintext, 0x00 = sealed), sealed `established`/`ping`/`pong` liveness, rekey keeps ≤2 receive contexts and zeroizes old keys only after the new confirm verifies. - `relay-slot.ts` — re-export of `antgrid-wire`'s slot helpers (one TS copy, shared with the relay). The phone reaches us on a per-machine SLOT (`#`; see `packages/antgrid_relay_client/CLAUDE.md`). The slot is the ROUTE address — `_peerId`, `pending.peerId`, the `phoneEd25519ByDeviceId` key and the single-active-phone takeover check all stay on it. Everything keyed by the ACCOUNT device uses `baseSlotDeviceId`: both transcripts in `handleClientHello` (the agent one is the HKDF salt — a slot there derives keys the app can't open), the `trustedPeers`/`pairedPhones` lookups in `resolvePhoneEd25519PubB64`/`backfillPeerPubkey`, and the `pairedPhones.upsert` row. Stripping never widens admission — every candidate is still gated by `verifyTranscriptSig`. Presence is filtered by `isForeignSlot`: the relay fans it to every same-account peer, so a sibling slot would otherwise repoint our reply address (`peer-online`) or suppress our heavy stream because another machine's socket closed (`peer-offline`). - `stream-mux.ts` — multiplexes project cores over the machine socket as sealed `{s, m}` envelopes (`s` absent/`"0"` = machine control plane; the ENVELOPE JSON is what gets fragmented, so `s` survives reassembly). `attachStream(bus, opts)` → `StreamHandle{streamId, detach, sendTunnel}`; admission = `stream-open` → `stream-opened` vs `error{ref: streamId}` (a rejection leaves the socket and other streams live); `opts.mayDeliver` is the OUTBOUND authorization hook, re-read on every bus frame and every `sendTunnel` (tunnel bypasses the bus) — absent means always-deliver, so a caller that answers to a switch must fail closed in its own provider; on each `welcome` the mux re-opens every attached stream (the relay dropped its `openStreams` on the disconnect). Current relays admit every stream a healthy machine opens — no per-account quota survives (`SESSION_LIMIT_EXCEEDED` is retired, kept only for relays predating the worker-limit change; `ErrorCode` in `packages/antgrid-wire/src/relay-protocol.ts` reserves the name for exactly that reason, and the relay side is the Streams bullet in `relay/CLAUDE.md`). The one rejection a current relay can still send is `STREAM_LIMIT_EXCEEDED`, the relay's structural per-connection ceiling: orders of magnitude above real use, so treat it as our bug (a leak of undetached streams), never as backpressure to retry. An inbound frame for an unknown streamId is dropped AND answered with a control-plane `stream-invalid {streamId}` (rate-limited per dead id): a host restart re-attaches every project under fresh ids, and without that notice the phone replays onto the dead id forever with nothing to trigger a renegotiation. -- `host-server.ts` + `paired-phones.ts` — machine-level device trust. `HostServer.startRemoteControlPlane()` owns the single machine RelayClient (bare `deviceUuid` — the only registration shape; compound `deviceUuid.projectId` is gone); project cores attach as streams via `remoteDepsFor(projectId)` (`ProjectCoreRemoteDeps = {attachStream, currentPeerPubkey, sendPushDeliver}` — `wireRelaySlot` is deleted). Stream admission publishes `stream-ready {projectId, streamId}`, and `buildProjectsAdvertisement` (`agent:projects`) carries per-project `streamId` so a reconnecting phone binds without a fresh `project:start`; stopped projects start on demand (`handleControlPlaneVerb` → `project:start`). `startCore` re-advertises unconditionally: an open no phone asked for (restart re-open, desktop-side open) lands AFTER the handshake advert, and nothing else announces it. A rejected verb returns `control:result {ok:false,error}` to the phone (never silently dropped). Authorization for a remote device is `loadRemoteAccessPolicy(abDir)` (`agents/mobile-access-policy.json` — the filename and the `mobile-access:*` verbs keep the old spelling on purpose: both cross a version boundary the rename cannot reach) — ONE machine-wide boolean, the only gate, read live at every check via `remoteAccessEnabled()` so `mobile-access:set` takes effect without restarting a core. It gates the stream in BOTH directions and both halves are load-bearing: inbound at `currentPhoneAllowed()` (agent-core's bus handler + `handleTunnelMessage`), outbound at the stream's `mayDeliver` (`attachRelayStream`). Inbound alone is not enough — a project the phone cold-started opens as a `mode:"remote"` core with no `PromotionHandle`, so `demoteAllPromoted()` (which turning the switch off also runs, for every PROMOTED slot) never touches it and it would keep streaming terminal/tree/git at the phone. Gating at the send, not at detach, is deliberate: the core and its stream stay alive, so flipping the switch back on resumes the same `streamId` with no re-attach and no destroyed work. Which projectId that phone may name is bounded solely by `isSafeProjectId` + the `seenProjects` catalog — every remote verb (`project:start`, both sessions RPCs) must do that lookup, there is no second gate behind it. `loadPairedPhones(abDir)` (`agents/paired-phones.json`) is NOT authorization: it is the identity/push-token/`lastSeenAt` row, kept for push targeting and freshness (hence `watch()` + `touchLastSeen` survive). +- `host-server.ts` + `paired-phones.ts` — machine-level device trust. `HostServer.startRemoteControlPlane()` owns the single machine RelayClient (bare `deviceUuid` — the only registration shape; compound `deviceUuid.projectId` is gone); project cores attach as streams via `remoteDepsFor(projectId)` (`ProjectCoreRemoteDeps = {attachStream, currentPeerPubkey, sendPushDeliver, machineDeviceId}` — `wireRelaySlot` is deleted). Stream admission publishes `stream-ready {projectId, streamId}`, and `buildProjectsAdvertisement` (`agent:projects`) carries per-project `streamId` so a reconnecting phone binds without a fresh `project:start`; stopped projects start on demand (`handleControlPlaneVerb` → `project:start`). `startCore` re-advertises unconditionally: an open no phone asked for (restart re-open, desktop-side open) lands AFTER the handshake advert, and nothing else announces it. A rejected verb returns `control:result {ok:false,error}` to the phone (never silently dropped). Authorization for a remote device is `loadRemoteAccessPolicy(abDir)` (`agents/mobile-access-policy.json` — the filename and the `mobile-access:*` verbs keep the old spelling on purpose: both cross a version boundary the rename cannot reach) — ONE machine-wide boolean, the only gate, read live at every check via `remoteAccessEnabled()` so `mobile-access:set` takes effect without restarting a core. It gates the stream in BOTH directions and both halves are load-bearing: inbound at `currentPhoneAllowed()` (agent-core's bus handler + `handleTunnelMessage`), outbound at the stream's `mayDeliver` (`attachRelayStream`). Inbound alone is not enough — a project the phone cold-started opens as a `mode:"remote"` core with no `PromotionHandle`, so `demoteAllPromoted()` (which turning the switch off also runs, for every PROMOTED slot) never touches it and it would keep streaming terminal/tree/git at the phone. Gating at the send, not at detach, is deliberate: the core and its stream stay alive, so flipping the switch back on resumes the same `streamId` with no re-attach and no destroyed work. Which projectId that phone may name is bounded solely by `isSafeProjectId` + the `seenProjects` catalog — every remote verb (`project:start`, both sessions RPCs) must do that lookup, there is no second gate behind it. `loadPairedPhones(abDir)` (`agents/paired-phones.json`) is NOT authorization: it is the identity/push-token/`lastSeenAt` row, kept for push targeting and freshness (hence `watch()` + `touchLastSeen` survive). - `auth/` — in-memory OAuth (no on-disk store). `credentials.ts` parses one JSON line from stdin into a `BootstrapPayload` (`local | remote`, 10s idle timeout) written by the app on spawn. `oauth-client.ts` mints tokens via `POST /api/auth/oauth2/token` (`grant_type=client_credentials`, `resource=/api/auth`); `startTokenMaintenance` re-mints at 80% of TTL (30s retry). On `invalid_client` → emit `auth_revoked` to stderr, exit 4 — that verdict is keyed on the ERROR CODE, not the status: Better-Auth answers a revoked device with 401 but a deleted client row with **400** ("missing client"), and a sign-out rotates the device and drops its row, so both mean the cached pair is dead. Credentials reach the host only once, via the stdin bootstrap, so a host left running on a rotated-away pair can never recover on its own — the app respawns it when the account device changes (`local_host_warmup.dart`). **The boot-time control-plane mint is exempt from the exit** (`fatalRevokeArmed`, disarmed across `start()`'s `startRemoteControlPlane()`): host.json and the ready marker are already out by then, so exiting would have the app's supervisor respawn straight back into the same dead pair — a permanent crash loop that also takes down the loopback plane local work depends on. Boot logs and serves loopback-only; a verdict from token maintenance afterwards is still fatal. - `crash-reporting.ts` — Sentry (`@sentry/bun`) for the HOST process only, into the same self-hosted errex project as the app. Three things gate it and all three must hold: the user's consent, which arrives on the stdin bootstrap as `telemetryEnabled` (the app reads the SAME setting that decides its own Sentry init, so one install cannot report from one half and not the other) and whose ABSENCE means off — a CLI or test host has nobody who consented; a `SENTRY_DSN` baked in at build time by `--define`, exactly like `LICENSE_API_URL`, so an un-`--define`d dev build is inert unless the env var is deliberately set — and it must carry a NUMERIC project id, because the JS SDKs reject any other and `Sentry.init` swallows the refusal (no throw, no status; later captures and even `flush` then succeed while sending nothing), which is why init verifies `getClient()?.getDsn()` and logs at error rather than trusting itself. errex issues SLUGS (`antgrid-app`), so the DSN that works for the app does NOT work here; sentry-dart takes the last path segment as an opaque String, which is why only this side is affected; and `scrubCrashEvent`, which strips paths, source lines, locals and the hostname before transmit — kept in lockstep with `app/lib/analytics/crash_reporting.dart`, since a path that survives one scrubber and not the other is one leak wearing two faces. Consent is fixed for the host's lifetime (the bootstrap is read once); that is the same restart-scoped gate the app applies to itself, not an oversight. Every integration that reads request bodies or source off disk is excluded (reasons are per-name in the file). **`OnUncaughtException`/`OnUnhandledRejection` are deliberately KEPT** and own the capture on both top-level paths, because they are what stamps a fatal `handled: false` (`auto.node.onuncaughtexception`) — a hand-rolled `captureException` reports the same crash as `generic`/`handled: true`, which is wrong in exactly the dimension this instrumentation answers. They are re-added with options pinned rather than inherited, and the contract has a second half that lives in `index.ts`: the SDK re-counts the OTHER `uncaughtException` listeners AT CRASH TIME, so it defers to our teardown only while one of ours is registered, and as the sole listener it exits on its own and skips the PTY sweep. `index.ts` therefore owns the EXIT and registers its handlers as early as `shutdown` can be closed over; do not widen that window from either side. **The `hook` subcommand is deliberately uninstrumented** — see the comment on its action for why an SDK there would be both unconsented and unable to catch the failure it looks like it would catch. diff --git a/bridge/src/host-server.ts b/bridge/src/host-server.ts index 037c376f..643cd2b5 100644 --- a/bridge/src/host-server.ts +++ b/bridge/src/host-server.ts @@ -512,7 +512,13 @@ export class HostServer { attachStream: (bus, opts) => client.attachStream(bus, opts), currentPeerPubkey: () => client.currentPeerPubkey(), sendPushDeliver: (m) => client.sendPushDeliver(m), - agentDeviceId: auth.deviceUuid, + // The LIVE socket's id, like every member beside it — not the inbound + // auth's. The credential swap above is gated on nothing being live, so a + // re-enable over an already-running socket leaves `client` registered + // under the previous deviceUuid; `auth.deviceUuid` would then name a + // machine the relay does not have this host on, and every push a + // wizard-promoted core seals would be unopenable on the phone. + agentDeviceId: client.deviceId, }; } @@ -1812,6 +1818,9 @@ export class HostServer { }, currentPeerPubkey: () => client.currentPeerPubkey(), currentPeerSupportsCheckoutRouting: () => client.peerSupportsCheckoutRouting, + // client.deviceId, NOT the one from identityFor(): a local core is handed a fresh + // randomUUID(), which addresses no machine the phone knows. + machineDeviceId: () => client.deviceId, sendPushDeliver: (m) => client.sendPushDeliver(m), }; } diff --git a/bridge/src/project-core.ts b/bridge/src/project-core.ts index f4e3c901..f38a55a2 100644 --- a/bridge/src/project-core.ts +++ b/bridge/src/project-core.ts @@ -25,6 +25,12 @@ export interface ProjectCoreRemoteDeps { currentPeerPubkey(): string | null; /** E2E app capability, established only after authenticated app:ready. */ currentPeerSupportsCheckoutRouting?(): boolean; + /** The bare machine deviceUuid this host registers under. The phone addresses + * a project as `.`, so a push sealed without it is a + * push the phone cannot open. Required, unlike currentPeerSupportsCheckoutRouting: + * optional would let the wizard-promotion supplier ship unroutable pushes and + * still compile. */ + machineDeviceId(): string; /** Blind FCM push forward over the machine socket (fallback delivery). */ sendPushDeliver(msg: { pushToken: string; provider: "fcm" | "apns"; blob: { epk: string; box: string } }): void; } @@ -515,6 +521,7 @@ export class ProjectCore { // the live path handles the online case and the dispatcher no-ops then. const dispatcher = createPushDispatcher({ projectId: core.projectId, + machineUuid: () => remote.machineDeviceId(), // Fire when the phone can't receive in-band: no live peer OR backgrounded // (`client:focus-state`). NOT connState.suppressed — that's the heavy-stream // gate, whose `peerOnline` defaults true, so it reads "can receive in-band" diff --git a/bridge/src/push/compose.ts b/bridge/src/push/compose.ts index 1b98e316..f310f860 100644 --- a/bridge/src/push/compose.ts +++ b/bridge/src/push/compose.ts @@ -8,19 +8,32 @@ const AGENT_LABELS: Record = { error: "Agent error", }; -export function composePush(msg: AbMessage): { title: string; body: string; kind: "agent" | "handler" } | null { +/** The routing ids ride along with the strings because this is the only + * per-message-type switch with Zod-narrowed access to them: a second switch in + * the dispatcher would have to re-narrow the union to reach `escalationId` and + * `sessionId`. `terminalId` is omitted rather than emitted empty — the phone + * treats a present key as a session it can resolve. */ +export interface ComposedPush { + title: string; + body: string; + kind: "agent" | "handler"; + sourceMessageId: string; + terminalId?: string; +} + +export function composePush(msg: AbMessage): ComposedPush | null { if (msg.type === "notification:push") { const label = AGENT_LABELS[msg.notificationType] ?? "Agent"; // body deliberately does NOT fall back to sessionTitle: that would make // title === body, which is the whole point of carrying two fields. const title = msg.sessionTitle && msg.sessionTitle.length > 0 ? msg.sessionTitle : label; const body = msg.message && msg.message.length > 0 ? msg.message : label; - return { title, body, kind: "agent" }; + return { title, body, kind: "agent", sourceMessageId: msg.id, ...(msg.sessionId ? { terminalId: msg.sessionId } : {}) }; } if (msg.type === "handler:escalation") { const title = msg.urgency === "high" ? "Handler — urgent" : "Handler needs you"; const body = msg.question && msg.question.length > 0 ? msg.question : "Agent needs you"; - return { title, body, kind: "handler" }; + return { title, body, kind: "handler", sourceMessageId: msg.escalationId, terminalId: msg.terminalId }; } return null; } diff --git a/bridge/src/push/push-dispatcher.ts b/bridge/src/push/push-dispatcher.ts index 418706b8..0da78da1 100644 --- a/bridge/src/push/push-dispatcher.ts +++ b/bridge/src/push/push-dispatcher.ts @@ -4,6 +4,11 @@ const log = logger.child({ component: "push-dispatcher" }); import { composePush } from "./compose"; const MAX_BODY_LEN = 480; // keep the sealed payload well under FCM's ~4 KB data cap +// The title is a session name (protocol.ts SessionEntry), which nothing upstream +// bounds. Oversizing it is silent data loss, not a truncated toast: the relay Zod- +// rejects a `box` over 8192 base64 chars before forwarding, and a rejected deliver +// produces no push:result, so the notification simply never exists. +const MAX_TITLE_LEN = 120; export interface PushTarget { pushToken: string; @@ -21,6 +26,11 @@ export interface PushDispatcherDeps { * send. Plural because with no live peer the agent can't know which allowed * device the user holds — see resolveTargets in project-core.ts. */ resolveTargets: () => PushTarget[]; + /** The bare machine deviceUuid this host registers under. A getter because the + * two suppliers differ in how well they can answer: host-server reads the live + * machine socket's identity, while the wizard-promotion path can only report + * the uuid the enabling `agent:enableRelay` carried (see relay-promotion.ts). */ + machineUuid: () => string; seal: (json: string, recipientPushPubkeyB64: string) => { epk: string; box: string }; deliver: (token: string, provider: "fcm" | "apns", blob: { epk: string; box: string }) => void; } @@ -56,13 +66,16 @@ export function createPushDispatcher(deps: PushDispatcherDeps) { log.warn("push: %s DROPPED — no push target for project %s", composed.kind, deps.projectId); return; } - const sourceMessageId = msg.type === "handler:escalation" ? msg.escalationId : msg.id; const payload = JSON.stringify({ - title: composed.title, + title: composed.title.slice(0, MAX_TITLE_LEN), body: composed.body.slice(0, MAX_BODY_LEN), kind: composed.kind, projectId: deps.projectId, - sourceMessageId, + // projectId is sha256(realpath(folder)) with no machine input, so two + // machines holding the same repo at the same path mint the identical id. + machineUuid: deps.machineUuid(), + ...(composed.terminalId ? { terminalId: composed.terminalId } : {}), + sourceMessageId: composed.sourceMessageId, }); // Seal per target: each phone has its own push key, so the ciphertext can't // be shared even though the plaintext is identical. diff --git a/bridge/src/relay-promotion.ts b/bridge/src/relay-promotion.ts index aa99fdfa..8af4028a 100644 --- a/bridge/src/relay-promotion.ts +++ b/bridge/src/relay-promotion.ts @@ -124,6 +124,7 @@ export function createRelayPromotion(deps: RelayPromotionDeps): RelayPromotionCo const remote: ProjectCoreRemoteDeps = { attachStream: (b, opts) => ensured.attachStream(b, opts), currentPeerPubkey: () => ensured.currentPeerPubkey(), + machineDeviceId: () => ensured.agentDeviceId, sendPushDeliver: (m) => ensured.sendPushDeliver(m), }; attachment = deps.attach(remote); diff --git a/bridge/tests/project-core.test.ts b/bridge/tests/project-core.test.ts index c244ea60..a07c4136 100644 --- a/bridge/tests/project-core.test.ts +++ b/bridge/tests/project-core.test.ts @@ -28,6 +28,7 @@ function fakeRemoteDeps(): { deps: ProjectCoreRemoteDeps; calls: Array<{ bus: Me return handle; }, currentPeerPubkey: () => null, + machineDeviceId: () => "machine-uuid", sendPushDeliver: () => {}, }; return { deps, calls }; diff --git a/bridge/tests/push/push-dispatcher.test.ts b/bridge/tests/push/push-dispatcher.test.ts index d26282c5..8dca7ac9 100644 --- a/bridge/tests/push/push-dispatcher.test.ts +++ b/bridge/tests/push/push-dispatcher.test.ts @@ -13,6 +13,7 @@ function harness(overrides: Partial[0]> projectId: "p1", shouldFallback: () => true, resolveTargets: () => [target], + machineUuid: () => "machine-uuid-1", seal: (json, pubkey) => { sealed.push(json); sealKeys.push(pubkey); return { epk: "E", box: "B" }; }, deliver: (t, prov, blob) => delivered.push({ t, prov, blob }), ...overrides, @@ -21,29 +22,34 @@ function harness(overrides: Partial[0]> } test("composePush mirrors the app strings", () => { - expect(composePush(createMessage("notification:push", { notificationType: "task_complete", message: "built", projectId: "p1" }))) - .toEqual({ title: "Task complete", body: "built", kind: "agent" }); + // The agent path's sourceMessageId is pinned to msg.id, not just to "a + // string": the app dedups the live toast against the FCM one on that exact + // equality (`_markNotified(msg.id)` vs `pushDedupKey`), so any other id here + // surfaces one notification twice and leaves the push tap undeduped. + const agent = createMessage("notification:push", { notificationType: "task_complete", message: "built", projectId: "p1" }); + expect(composePush(agent)) + .toEqual({ title: "Task complete", body: "built", kind: "agent", sourceMessageId: agent.id }); expect(composePush(createMessage("handler:escalation", { projectId: "p1", escalationId: "e1", terminalId: "t", question: "Deploy?", reasoning: "", draftReply: "", urgency: "high", at: 1, - }))).toEqual({ title: "Handler — urgent", body: "Deploy?", kind: "handler" }); + }))).toEqual({ title: "Handler — urgent", body: "Deploy?", kind: "handler", sourceMessageId: "e1", terminalId: "t" }); }); test("composePush: sessionTitle becomes the title, message the body", () => { expect(composePush(createMessage("notification:push", { notificationType: "task_complete", message: "Added a regression test", sessionTitle: "Fix auth bug", projectId: "p1", - }))).toEqual({ title: "Fix auth bug", body: "Added a regression test", kind: "agent" }); + }))).toEqual({ title: "Fix auth bug", body: "Added a regression test", kind: "agent", sourceMessageId: expect.any(String) }); }); test("composePush: sessionTitle without a message keeps the label as the body", () => { expect(composePush(createMessage("notification:push", { notificationType: "task_complete", sessionTitle: "Fix auth bug", projectId: "p1", - }))).toEqual({ title: "Fix auth bug", body: "Task complete", kind: "agent" }); + }))).toEqual({ title: "Fix auth bug", body: "Task complete", kind: "agent", sourceMessageId: expect.any(String) }); }); test("composePush: neither field degrades to today's exact strings", () => { expect(composePush(createMessage("notification:push", { notificationType: "task_complete", projectId: "p1", - }))).toEqual({ title: "Task complete", body: "Task complete", kind: "agent" }); + }))).toEqual({ title: "Task complete", body: "Task complete", kind: "agent", sourceMessageId: expect.any(String) }); }); test("composePush: body never falls back to sessionTitle", () => { @@ -57,13 +63,13 @@ test("composePush: body never falls back to sessionTitle", () => { test("composePush: empty strings are treated as absent", () => { expect(composePush(createMessage("notification:push", { notificationType: "error", message: "", sessionTitle: "", projectId: "p1", - }))).toEqual({ title: "Agent error", body: "Agent error", kind: "agent" }); + }))).toEqual({ title: "Agent error", body: "Agent error", kind: "agent", sourceMessageId: expect.any(String) }); }); test("composePush: sessionTitle titles a permission request too", () => { expect(composePush(createMessage("notification:push", { notificationType: "permission_request", message: "Run rm -rf build?", sessionTitle: "Fix auth bug", projectId: "p1", - }))).toEqual({ title: "Fix auth bug", body: "Run rm -rf build?", kind: "agent" }); + }))).toEqual({ title: "Fix auth bug", body: "Run rm -rf build?", kind: "agent", sourceMessageId: expect.any(String) }); }); test("suppressed peer → seals payload and delivers", () => { @@ -74,7 +80,39 @@ test("suppressed peer → seals payload and delivers", () => { expect(delivered).toHaveLength(1); expect(delivered[0].t).toBe("tok"); const payload = JSON.parse(sealed[0]); - expect(payload).toEqual({ title: "Handler needs you", body: "Deploy?", kind: "handler", projectId: "p1", sourceMessageId: "e1" }); + expect(payload).toEqual({ + title: "Handler needs you", body: "Deploy?", kind: "handler", + projectId: "p1", machineUuid: "machine-uuid-1", terminalId: "t", sourceMessageId: "e1", + }); +}); + +test("a notification that names no session seals no terminalId key at all", () => { + // An empty string would read to the phone as a session it should resolve and + // fail to find; the hook path's sessionId is legitimately optional. + const { d, sealed } = harness(); + d.onOutbound(createMessage("notification:push", { notificationType: "idle", projectId: "p1" })); + expect(Object.keys(JSON.parse(sealed[0]))).not.toContain("terminalId"); +}); + +test("a notification that names a session seals it as the terminalId", () => { + // The hook path is the primary producer, and the phone resolves this id back + // to a cached session uuid to pick the terminal to open — a neighbouring + // field (msg.id, the checkoutId) type-checks here and lands on nothing. + const { d, sealed } = harness(); + d.onOutbound(createMessage("notification:push", { + notificationType: "task_complete", sessionId: "sess-1", projectId: "p1", + })); + expect(JSON.parse(sealed[0]).terminalId).toBe("sess-1"); +}); + +test("an unbounded session title is capped before sealing", () => { + // The relay rejects an oversized box outright and answers no push:result, so + // an uncapped title loses the whole notification rather than truncating it. + const { d, sealed } = harness(); + d.onOutbound(createMessage("notification:push", { + notificationType: "task_complete", sessionTitle: "x".repeat(300), projectId: "p1", + })); + expect(JSON.parse(sealed[0]).title.length).toBe(120); }); test("not suppressed (in-band available) → no delivery", () => { diff --git a/bridge/tests/push/push-restart-targeting.test.ts b/bridge/tests/push/push-restart-targeting.test.ts index 69988c4d..bbbd417f 100644 --- a/bridge/tests/push/push-restart-targeting.test.ts +++ b/bridge/tests/push/push-restart-targeting.test.ts @@ -2,11 +2,11 @@ import { test, expect, afterEach, beforeEach } from "bun:test"; import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { randomUUID } from "node:crypto"; +import { createDecipheriv, hkdfSync, randomUUID } from "node:crypto"; import { ProjectCore } from "../../src/project-core"; import { computeProjectId } from "../../src/project-id"; import { loadPairedPhones, type PairedPhonesStore } from "../../src/paired-phones"; -import { generateEphemeralKeypair } from "../../src/key-exchange"; +import { generateEphemeralKeypair, deriveSharedSecret } from "../../src/key-exchange"; import { createMessage } from "../../src/protocol"; import type { MessageBus } from "../../src/message-bus"; @@ -30,6 +30,20 @@ afterEach(() => { interface Delivered { pushToken: string; provider: string; + blob: { epk: string; box: string }; +} + +/** Opens a sealed push with the recipient's push private key, mirroring the + * app's decode. Deliberately independent of `sealPush` rather than sharing a + * helper with it, so a change to the sealing format fails here instead of + * being masked by both sides moving together. */ +function openPush(blob: { epk: string; box: string }, privateKey: Buffer): any { + const epk = Buffer.from(blob.epk, "base64"); + const key = Buffer.from(hkdfSync("sha256", deriveSharedSecret(privateKey, epk), epk, "antgrid-push-v1", 32)); + const raw = Buffer.from(blob.box, "base64"); + const decipher = createDecipheriv("aes-256-gcm", key, raw.subarray(0, 12)); + decipher.setAuthTag(raw.subarray(raw.length - 16)); + return JSON.parse(Buffer.concat([decipher.update(raw.subarray(12, raw.length - 16)), decipher.final()]).toString("utf8")); } /** A relay slot whose phone has NEVER connected during this agent lifetime — @@ -71,6 +85,7 @@ async function startRestartedAgent(opts: { mobileAccess: boolean }) { return { streamId: "s1", detach: () => {}, sendTunnel: () => {} }; }, currentPeerPubkey: () => null, + machineDeviceId: () => "machine-uuid", sendPushDeliver: (p) => delivered.push(p), }, }); @@ -82,7 +97,7 @@ async function startRestartedAgent(opts: { mobileAccess: boolean }) { createMessage("notification:push", { notificationType: "task_complete", message: "built", projectId }), "control", ); - return { notify, delivered, projectId }; + return { notify, delivered, projectId, phonePush }; } test("push targets the persisted phone when no peer has connected this agent lifetime", async () => { @@ -100,6 +115,20 @@ test("push targets the persisted phone when no peer has connected this agent lif expect(delivered[0].provider).toBe("fcm"); }); +test("the sealed payload names the machine the phone must dial", async () => { + // This is the only suite that drives a real ProjectCore through to deliver(), + // so it is the only place the project-core -> dispatcher hop is proved rather + // than injected. projectId alone is sha256(realpath(folder)) and names no + // machine, so a payload missing machineUuid is one the phone cannot open. + const { notify, delivered, projectId, phonePush } = await startRestartedAgent({ mobileAccess: true }); + + notify(); + + const opened = openPush(delivered[0].blob, phonePush.privateKey); + expect(opened.machineUuid).toBe("machine-uuid"); + expect(opened.projectId).toBe(projectId); +}); + test("persisted-store fallback still refuses to push from a machine with mobile access off", async () => { // The trust boundary is unchanged by the fallback: push carries project // activity OFF this machine, so a registered phone with a valid token must diff --git a/bridge/tests/relay-promotion.test.ts b/bridge/tests/relay-promotion.test.ts index 23d03159..b4e78aad 100644 --- a/bridge/tests/relay-promotion.test.ts +++ b/bridge/tests/relay-promotion.test.ts @@ -11,6 +11,7 @@ import { expect, test } from "bun:test"; import { MessageBus } from "../src/message-bus"; import { createMessage, type AbMessage } from "../src/protocol"; import { createRelayPromotion, type MachineRelaySession, type LocalStreamAttachment } from "../src/relay-promotion"; +import type { ProjectCoreRemoteDeps } from "../src/project-core"; import type { StreamHandle } from "../src/stream-mux"; const ENABLE = createMessage("agent:enableRelay", { @@ -38,6 +39,7 @@ function makeMachineSession(overrides: Partial = {}): Machi * bring-up, exactly one attach per promotion) did not. */ function makeDeps(session: MachineRelaySession) { const calls = { ensureMachineRelay: 0, attach: 0, detach: 0 }; + let attached: ProjectCoreRemoteDeps | null = null; const handle: StreamHandle = { streamId: "s1", detach: () => {}, sendTunnel: () => {} }; return { calls, @@ -45,10 +47,12 @@ function makeDeps(session: MachineRelaySession) { calls.ensureMachineRelay++; return session; }, - attach: (): LocalStreamAttachment => { + attach: (remote: ProjectCoreRemoteDeps): LocalStreamAttachment => { calls.attach++; + attached = remote; return { handle, detach: () => { calls.detach++; } }; }, + get attached() { return attached; }, }; } @@ -70,6 +74,9 @@ test("enableRelay attaches the core as a stream and emits relayReady from the ma expect(ready).toBeDefined(); // @ts-expect-error narrowed at runtime expect(ready.agentDeviceId).toBe(session.agentDeviceId); + // The promoted core seals pushes addressed to the machine the wizard just + // brought up; tsc alone would accept any string here. + expect(deps.attached?.machineDeviceId()).toBe(session.agentDeviceId); ctrl.stop(); expect(deps.calls.detach).toBe(1);