feat(reminder): add notification, alarm, and dialog device adapters - #269
Merged
MeteorsLiu merged 3 commits intoAug 20, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced Aug 17, 2026
LUPENGHAN
force-pushed
the
feature/reminder-adapters-notifications
branch
from
August 17, 2026 05:22
642377e to
bc52a99
Compare
LUPENGHAN
force-pushed
the
feature/reminder-adapters-notifications
branch
from
August 17, 2026 07:00
bc52a99 to
b3d06f5
Compare
LUPENGHAN
force-pushed
the
feature/reminder-adapters-notifications
branch
from
August 17, 2026 07:26
b3d06f5 to
06c5061
Compare
LUPENGHAN
force-pushed
the
feature/reminder-adapters-notifications
branch
from
August 17, 2026 07:52
06c5061 to
eb4bf6a
Compare
LUPENGHAN
added a commit
to LUPENGHAN/timeflow
that referenced
this pull request
Aug 17, 2026
Code review (PR 1024XEngineer#269): two bugs. ExpoSystemNotification.ensureAndroidChannel() cached setNotificationChannelAsync()'s promise in a module-level variable without resetting it on rejection. Once it failed once (e.g. called before the notifications module finished initializing after app launch), every future show() call re-awaited the same rejected promise and threw -- every 'low' strength reminder for the rest of the app session would fail to deliver, not just skip channel setup. Reset channelReady to null in the catch so the next call retries, same fix shape as ExpoAudioPlayback.ensureAudioMode() already uses. TimeflowAlarmBridge.nativeGetAlarmPermissionStatus() and nativeOpenAlarmPermissionSettings() were the only two functions in the file that didn't wrap their native call in try/catch, unlike every sibling (nativeScheduleAlarm, nativeCancelAlarm, nativeCancelAllAlarms, nativeStopAlarmRinging, nativeConsumeAlarmDispositions, nativeRequestNotificationPermission). A native-side rejection propagated uncaught through NativeDeviceCapability.getStatus() into useReminderPermissionsOnLaunch's try/finally, where it was only caught by the outer runPrompt().catch() -- silently stalling the whole permission-request flow. Added matching try/catch to both.
LUPENGHAN
added a commit
to LUPENGHAN/timeflow
that referenced
this pull request
Aug 18, 2026
Two changes on top of the rebase onto upstream/main's native alarm module (1024XEngineer#264/1024XEngineer#265): - AlarmSchedulerPort.consumeNativeDispositions (single-phase, clears the native buffer on read) is now peekNativeDispositions + ackNativeDispositions (two-phase), matching the split 1024XEngineer#265 already made on the native side. hydrateNativeDispositions() now acks only after the whole batch persists successfully, so a mid-batch failure leaves the un-acked rows in the native buffer for retry instead of losing them. - This PR's own interface changes (DeviceCapabilityPort.onAppActive, LocationMonitorPort.rebuild's new LocationRebuildTarget param, dropping MockTimeListener from shared/time) were never propagated to their implementers, so `npm run check` failed standalone -- expected for a mid-stack PR per its own description, but not acceptable for merging. Added the minimal implementations needed to close the gap: onAppActive on Native/MockDeviceCapability, MockLocationMonitor.rebuild's signature, and a new infrastructure/time/MockTimeListener placeholder. 1024XEngineer#269 replaces all of these with real adapters; this just keeps 1024XEngineer#266 green on its own. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Wintercom
pushed a commit
that referenced
this pull request
Aug 19, 2026
* feat(reminder): add reminder engine core (domain + application) LocalReminderApplication and its ports (AlarmSchedulerPort, LocationMonitorPort, DeviceCapabilityPort, NotificationChannels, ReminderDeliveryPort, ReminderApplicationPort), plus the domain layer (reminder.ts, strengthDelivery.ts) that drives arm/fire/confirm state transitions and recurring-schedule advancement. Pure logic layer: no device-specific adapters yet, nothing wired into the app. shared/time gains format.ts (used by the delivery strength calc); MockClock/MockTimeListener are removed since nothing in this stack still needs a fake clock once the real engine lands. * fix(reminder): close native/JS double-fire race in acknowledgeNativeFire Code review (PR #266): acknowledgeNativeFire() only checked the persisted disposition_state (confirmed/pending) before proceeding, not the in-memory activeDeliveries/deliverLocks sets. Those sets are exactly what canDeliver() checks on the JS-driven handleTime path to skip a schedule the native alarm already owns -- but the guard only worked in that one direction. If handleTime was already mid-flight for a schedule (added to activeDeliveries, not yet persisted any disposition change) at the moment the native alarm for the same schedule fired, acknowledgeNativeFire would fall through and the two channels could both run -- the exact double-fire the surrounding comment says this mechanism eliminates. Added the activeDeliveries check as an additional early-return, matching canDeliver()'s guard. * fix(reminder): peek/ack native dispositions, close CI gap after rebase Two changes on top of the rebase onto upstream/main's native alarm module (#264/#265): - AlarmSchedulerPort.consumeNativeDispositions (single-phase, clears the native buffer on read) is now peekNativeDispositions + ackNativeDispositions (two-phase), matching the split #265 already made on the native side. hydrateNativeDispositions() now acks only after the whole batch persists successfully, so a mid-batch failure leaves the un-acked rows in the native buffer for retry instead of losing them. - This PR's own interface changes (DeviceCapabilityPort.onAppActive, LocationMonitorPort.rebuild's new LocationRebuildTarget param, dropping MockTimeListener from shared/time) were never propagated to their implementers, so `npm run check` failed standalone -- expected for a mid-stack PR per its own description, but not acceptable for merging. Added the minimal implementations needed to close the gap: onAppActive on Native/MockDeviceCapability, MockLocationMonitor.rebuild's signature, and a new infrastructure/time/MockTimeListener placeholder. #269 replaces all of these with real adapters; this just keeps #266 green on its own. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(reminder): break cold-start deadlock in hydrateNativeDispositions, add adapter tests hydrateNativeDispositions() runs inside startInternal(), which is itself the task currently occupying opChain. It was calling the public confirm()/snooze() wrappers, which re-enqueue onto that same opChain -- but that chain can't advance until startInternal() (the caller) returns, and startInternal() won't return until those re-enqueued calls resolve. Any cold start with a confirmed/snoozed row sitting in the native disposition buffer deadlocked permanently. Fixed by calling confirmInternal()/snoozeInternal() directly, since hydrateNativeDispositions() is already running as the sole active opChain task and doesn't need to re-enqueue. Also closes the Codecov patch-coverage gap on the adapter changes from the previous commit: onAppActive on Native/MockDeviceCapability, and the MockLocationMonitor/MockTimeListener mocks. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(reminder): add application-layer coverage for LocalReminderApplication Covers what review flagged as missing: cold-start peek/ack (batch-level ack only after every row persists, none on partial failure), hydration for each native disposition state (confirmed/snoozed/pending), the native/JS dual-channel race guard, alarm rescheduling after confirm/snooze, stop/ restart cleanup, and recurring-reminder re-arming across two occurrences (demonstrating the next_trigger_at signal / state-layer-computes-the-next- occurrence split with actual test evidence, not just an explanation). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(reminder): cover location triggering and strength delivery plans Codecov flagged patch coverage at 66% after the application-layer test commit -- LocalReminderApplication.ts's location-schedule path (arm on leave, deliver on re-entry, no re-fire while still armed) and strengthDelivery.ts's low/high branches had zero coverage. Both were only exercised indirectly (or not at all) by the existing tests, which focused on time-triggered schedules. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(reminder): cover delivery channels by strength and native alarm event routing Codecov target for this PR's patch is 91.16%, was at 76.78%. Adds: - low strength delivery: system notification only, no popup/vibration/audio - high strength delivery: popup + vibration + tts, falls back to local audio when tts fails - native "snoozed"/"dismissed" events routed through alarms.subscribe() to snooze()/confirm(), including unsubscribe on stop() --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
LUPENGHAN
added a commit
to LUPENGHAN/timeflow
that referenced
this pull request
Aug 19, 2026
Code review (PR 1024XEngineer#269): two bugs. ExpoSystemNotification.ensureAndroidChannel() cached setNotificationChannelAsync()'s promise in a module-level variable without resetting it on rejection. Once it failed once (e.g. called before the notifications module finished initializing after app launch), every future show() call re-awaited the same rejected promise and threw -- every 'low' strength reminder for the rest of the app session would fail to deliver, not just skip channel setup. Reset channelReady to null in the catch so the next call retries, same fix shape as ExpoAudioPlayback.ensureAudioMode() already uses. TimeflowAlarmBridge.nativeGetAlarmPermissionStatus() and nativeOpenAlarmPermissionSettings() were the only two functions in the file that didn't wrap their native call in try/catch, unlike every sibling (nativeScheduleAlarm, nativeCancelAlarm, nativeCancelAllAlarms, nativeStopAlarmRinging, nativeConsumeAlarmDispositions, nativeRequestNotificationPermission). A native-side rejection propagated uncaught through NativeDeviceCapability.getStatus() into useReminderPermissionsOnLaunch's try/finally, where it was only caught by the outer runPrompt().catch() -- silently stalling the whole permission-request flow. Added matching try/catch to both.
LUPENGHAN
force-pushed
the
feature/reminder-adapters-notifications
branch
from
August 19, 2026 06:45
b51d7f4 to
cc7c7ff
Compare
LUPENGHAN
marked this pull request as ready for review
August 19, 2026 06:51
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
审阅了本 PR 的通知、原生闹钟、权限、事件桥接及替换后的适配器导出。发现两个会影响真实提醒状态一致性的回归：冷启动处置记录没有接入应用层约定的 peek/ack 接口，且取消失败会被报告为成功。git diff --check 通过；本地 npm run typecheck 未能执行，因为工作区的 tsc 启动文件没有执行权限。
Additional findings
frontend/src/infrastructure/notifications/NativeAlarmScheduler.ts:?: [P1] Preserve cancellation failure in the receipt:nativeCancelAlarm()now returnsvoidand swallows both native rejection and a false native result, while this method unconditionally returns{ cancelled: true }. Callers such asLocalReminderApplication.cancelScheduledAlarm()clear the registration after awaiting this result; if the native alarm service is unavailable or cancellation fails, the persisted/runtime registration can be removed even though the OS alarm remains scheduled and may still fire. Return the native cancellation outcome (and false for failure) so callers do not claim a cancellation that did not happen.
Part of 1024XEngineer#263. ExpoSystemNotification, NativeAlarmScheduler (+ TimeflowAlarmBridge -- the JS<->native event bridge to the timeflow-alarm module), NativeDeviceCapability (real expo-location permission reads/requests + onAppActive for the settings-page-return flow), ReactNativeAlertDialog, ReactNativeVibration. Only depends on application interfaces already on main -- independent of the audio/location/data-layer PRs in this stack. Removes MockAlarmScheduler, MockDeviceCapability, MockNotificationChannels, MockReminderDelivery, MockReminderRecovery. Also removes two dangling test files (nativeAlarmScheduler.test.ts, nativeDeviceCapability.test.ts) written against the old mock-backed versions -- known coverage gap, see Issue 1024XEngineer#263 Out of Scope.
Code review (PR 1024XEngineer#269): two bugs. ExpoSystemNotification.ensureAndroidChannel() cached setNotificationChannelAsync()'s promise in a module-level variable without resetting it on rejection. Once it failed once (e.g. called before the notifications module finished initializing after app launch), every future show() call re-awaited the same rejected promise and threw -- every 'low' strength reminder for the rest of the app session would fail to deliver, not just skip channel setup. Reset channelReady to null in the catch so the next call retries, same fix shape as ExpoAudioPlayback.ensureAudioMode() already uses. TimeflowAlarmBridge.nativeGetAlarmPermissionStatus() and nativeOpenAlarmPermissionSettings() were the only two functions in the file that didn't wrap their native call in try/catch, unlike every sibling (nativeScheduleAlarm, nativeCancelAlarm, nativeCancelAllAlarms, nativeStopAlarmRinging, nativeConsumeAlarmDispositions, nativeRequestNotificationPermission). A native-side rejection propagated uncaught through NativeDeviceCapability.getStatus() into useReminderPermissionsOnLaunch's try/finally, where it was only caught by the outer runPrompt().catch() -- silently stalling the whole permission-request flow. Added matching try/catch to both.
LUPENGHAN
force-pushed
the
feature/reminder-adapters-notifications
branch
2 times, most recently
from
August 19, 2026 07:58
5988a3f to
817d0b7
Compare
…pters
Codecov flagged patch coverage at 1.6% (183 lines missing) across
NativeAlarmScheduler.ts, NativeDeviceCapability.ts, TimeflowAlarmBridge.ts,
ExpoSystemNotification.ts, ReactNativeVibration.ts, and
ReactNativeAlertDialog.ts -- none of them had tests.
NativeAlarmScheduler/TimeflowAlarmBridge, ReactNativeVibration, and
ReactNativeAlertDialog were straightforward to test directly (mocked
NativeModules.TimeflowAlarm, a fake NativeEventEmitter installed via
Object.defineProperty since react-native/index.js exports it through a
getter, fake timers for the vibration pattern).
NativeDeviceCapability and ExpoSystemNotification were not: both call
`await import('expo-location')` / `await import('expo-notifications')`
directly, which throws in this Jest environment without
--experimental-vm-modules, so jest.mock(...) can never be reached -- the
try/catch around the import swallows that TypeError the same way it would
swallow a real "native module unavailable" failure. Added a constructor
seam to each (loadLocationModule / loadNotificationsModule, defaulting to
the real dynamic import) so tests can inject a fake module and exercise
the real logic instead of only ever hitting the fallback branch. Both
production call sites (createAppServices.ts) still construct with no
arguments, unchanged.
ExpoSystemNotification also caches "channel already created" in a
module-level variable, so its tests use jest.isolateModules() + require()
to get a fresh module instance per case instead of a poisoned shared cache.
Patch coverage on the six files is now 83-100%.
LUPENGHAN
force-pushed
the
feature/reminder-adapters-notifications
branch
from
August 19, 2026 08:06
817d0b7 to
eb9e666
Compare
8 tasks
This was referenced Aug 19, 2026
MeteorsLiu
approved these changes
Aug 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
关联 Issue
Part of #263
依赖 #266（先合）、#267、#268（先合）。跟 #270 改动内容互相独立，可以并行审；但四个适配器
PR 都改
createAppServices.ts，需按 #267→#268→#269→#270 顺序依次合并，合并前会 rebase 到上一个已合并 PR 的 main。
改动
ExpoSystemNotification（实现SystemNotificationPort）：基于 expo-notifications 的轻度提醒通知，懒建 Android 通知渠道
NativeAlarmScheduler+TimeflowAlarmBridge（JS↔原生timeflow-alarm模块事件桥）：实现
AlarmSchedulerPort，调度/取消/重建原生闹钟，订阅原生 fired/dismissed/snoozed 事件NativeDeviceCapability（实现DeviceCapabilityPort）：真实读取/申请 expo-location 前台/后台定位权限 +
onAppActive，配合设置页回退流程读取通知/精确闹钟/悬浮窗/全屏通知/电池优化等原生权限状态
ReactNativeAlertDialog、ReactNativeVibration：分别实现弹窗确认和震动两个端口createAppServices.ts（alarms端口在 feat(reminder): add alarm scheduling, RN bridge, and ring activity/service #265 已经接了NativeAlarmScheduler；本 PR 新增的通知/设备能力/弹窗/震动几个端口的接线留给 feat(reminder): wire real engine into the app, drop remaining mocks #271）MockAlarmScheduler、MockDeviceCapability、MockNotificationChannels、MockReminderDelivery、MockReminderRecovery修复（review 中发现）
nativeCancelAlarm()原来是Boolean(await NativeAlarm.cancel(alarmId))之外还额外做了一层封装，导致原生返回“取消失败”时上层仍可能读到取消成功的假象；
NativeAlarmScheduler.cancel()因此可能在原生闹钟其实还留着的情况下告诉调用方“已取消”。改为直接透传原生
cancel()返回的布尔值，不再吞掉/改写失败结果consumeNativeDispositions()：读原生 disposition 缓冲区的同时清空它，JS 侧读到后如果落盘失败，这条状态就永久丢失、原生也
不会再重放。改成两阶段协议：
peekNativeDispositions()（只读、不清缓冲区）+ackNativeDispositions(scheduleIds)（JS 侧落盘成功后才确认，原生才清缓冲区）；NativeAlarmScheduler相应实现这两个新方法，ackNativeDispositions()失败时不清缓冲区，下次冷启动重新 peek 到、重放同样的幂等状态转换
ExpoSystemNotification.ensureAndroidChannel()把setNotificationChannelAsync()的 promise缓存进模块级变量，但从不在失败时清掉；只要失败过一次（比如 App 刚启动、通知模块还没初始化好
时调用），后续所有
show()都会重新 await 这个已经 reject 的 promise 并跟着抛错——不只是这一次跳过渠道设置，而是整个 App 生命周期内所有
low强度提醒都发不出去了。修复：catch里把channelReady重置为null，下一次调用会重试渠道设置，跟ExpoAudioPlayback.ensureAudioMode()已有的修复是同一种形状（
d51d7f3）TimeflowAlarmBridge.nativeGetAlarmPermissionStatus()和nativeOpenAlarmPermissionSettings()是文件里僅有的两个没包 try/catch 的函数（
nativeScheduleAlarm/nativeCancelAlarm/nativeCancelAllAlarms/nativeStopAlarmRinging/nativeAckAlarmDispositions/nativeRequestNotificationPermission都有）。原生侧一旦抛错就会未捕获地穿透NativeDeviceCapability.getStatus()，掉进useReminderPermissionsOnLaunch的 try/finally里只被最外层
runPrompt().catch()接住——整条权限申请续弹流程会静默卡死。补了跟其它函数一致的 try/catch（
d51d7f3）验证
npx tsc --noEmit、npx eslint .、npx prettier --check .全绿npm run test：CI 全绿NativeAlarmScheduler/TimeflowAlarmBridge、ReactNativeVibration、ReactNativeAlertDialog（直接 mockNativeModules.TimeflowAlarm，通过Object.defineProperty装一个假NativeEventEmitter，因为react-native/index.js是用getter 导出它的，直接赋值会被忽略；震动模式测试用 fake timers）
NativeDeviceCapability/ExpoSystemNotification都在构造函数上加了注入口子（
loadLocationModule/loadNotificationsModule，默认走真实的动态import()），因为它们直接
await import('expo-location')/await import('expo-notifications')，这个项目的 Jest配置没开
--experimental-vm-modules，动态 import 在测试环境必抛错、被源码自己的 try/catch当成"原生模块不可用"吞掉，
jest.mock(...)完全够不到——测试注入假模块绕开这一层，测真正的逻辑而不是永远只走 fallback 分支；生产代码里
createAppServices.ts仍然是无参构造，不受影响。ExpoSystemNotification另外把"渠道已建好"缓存在模块级变量里，测试用jest.isolateModules()+require()换取每个用例一份全新模块实例，避免前一条用例的缓存污染后一条
TimeflowAlarmBridge.ts/NativeDeviceCapability.ts/NativeAlarmScheduler.ts/ExpoSystemNotification.ts/ReactNativeVibration.ts/ReactNativeAlertDialog.ts）patchcoverage 从 Codecov 最初报的 1.6%（183 行未覆盖）提到 83-100%
本轮不含（见 #263 Out of Scope）
createAppServices.ts里通知/设备能力/弹窗/震动几个端口换成真实实现）留给 feat(reminder): wire real engine into the app, drop remaining mocks #271 统一处理