Skip to content

Commit 1f10f29

Browse files
committed
polish(notifications): assert deterministic notif id, cap pending map, align foreground semantics
1 parent dee4e8a commit 1f10f29

3 files changed

Lines changed: 55 additions & 4 deletions

File tree

app/lib/notifications/notification_observer.dart

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,13 @@ class NotificationController with WidgetsBindingObserver {
4141

4242
@override
4343
void didChangeAppLifecycleState(AppLifecycleState state) {
44-
_foreground = state == AppLifecycleState.resumed;
44+
// `inactive` (a transient system overlay: control center, app switcher,
45+
// call banner) still counts as foreground — the user is effectively in-app,
46+
// so a status notification would be redundant. This mirrors
47+
// `SrvRequestHandler`'s foreground semantics.
48+
_foreground =
49+
state == AppLifecycleState.resumed ||
50+
state == AppLifecycleState.inactive;
4551
}
4652

4753
void _onSessions(SessionsState sessions) {

app/lib/ui/widgets/srv_request_handler.dart

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,11 @@ class _SrvRequestHandlerState extends ConsumerState<SrvRequestHandler>
3636

3737
/// Backgrounded requests for which we fired a notification. Kept so that if
3838
/// the user resumes the app without acting on the notification, we can still
39-
/// present the dialog (the `srvRequests` stream has no replay).
39+
/// present the dialog (the `srvRequests` stream has no replay). Soft-capped
40+
/// so a long backgrounded session can't grow it without bound (mirrors the
41+
/// SPEC-07 status-notification queue); oldest entries are evicted first.
4042
final Map<String, Envelope> _pendingBackground = {};
43+
static const _kMaxPendingBackground = 50;
4144

4245
/// Salted so request-notification ids can't collide with the status
4346
/// notifications keyed on `sessionId.hashCode`.
@@ -124,6 +127,9 @@ class _SrvRequestHandlerState extends ConsumerState<SrvRequestHandler>
124127
// Not shown (no permission / dismissed / platform throw): fall through
125128
// to present the dialog now, so the request stays answerable.
126129
if (shown) {
130+
if (_pendingBackground.length >= _kMaxPendingBackground) {
131+
_pendingBackground.remove(_pendingBackground.keys.first);
132+
}
127133
_pendingBackground[env.id] = env;
128134
return;
129135
}

app/test/srv_request_handler_notify_test.dart

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class _RecordingNotificationService extends NotificationService {
2222
/// Value returned from [show] — set to `false` to simulate a notification
2323
/// that could not be displayed (no permission / dismissed / platform throw).
2424
final bool result;
25-
final List<({String? category, String? payload})> shown = [];
25+
final List<({int id, String? category, String? payload})> shown = [];
2626

2727
@override
2828
Future<bool> show({
@@ -32,7 +32,7 @@ class _RecordingNotificationService extends NotificationService {
3232
String? payload,
3333
String? category,
3434
}) async {
35-
shown.add((category: category, payload: payload));
35+
shown.add((id: id, category: category, payload: payload));
3636
return result;
3737
}
3838
}
@@ -362,4 +362,43 @@ void main() {
362362
expect(find.text('Approve'), findsNothing);
363363
},
364364
);
365+
366+
testWidgets(
367+
'notification id is deterministic per requestId and differs across requests',
368+
(tester) async {
369+
final (transport, notifications, _) = await pumpHandler(tester);
370+
tester.binding.handleAppLifecycleStateChanged(AppLifecycleState.paused);
371+
await tester.pump();
372+
373+
Envelope confirm(String id) => Envelope(
374+
t: MsgType.srvRequest,
375+
id: id,
376+
body: {
377+
'kind': 'confirmAction',
378+
'action': 'rm -rf build/',
379+
'sessionId': 's1',
380+
},
381+
);
382+
383+
// Same env.id dispatched twice → same notification id (OS dedup).
384+
transport.emit(confirm('req-same'));
385+
await tester.pump();
386+
await tester.pump();
387+
transport.emit(confirm('req-same'));
388+
await tester.pump();
389+
await tester.pump();
390+
391+
// Different env.id → different notification id.
392+
transport.emit(confirm('req-other'));
393+
await tester.pump();
394+
await tester.pump();
395+
396+
expect(notifications.shown, hasLength(3));
397+
final firstId = notifications.shown[0].id;
398+
final secondId = notifications.shown[1].id;
399+
final otherId = notifications.shown[2].id;
400+
expect(secondId, firstId);
401+
expect(otherId, isNot(firstId));
402+
},
403+
);
365404
}

0 commit comments

Comments
 (0)