Skip to content

feat(backend): wire time-window trigger to WS reminder dispatch pipeline - #109

Closed
LUPENGHAN wants to merge 1 commit into
1024XEngineer:MVPfrom
LUPENGHAN:feature/reminder-dispatch-pipeline
Closed

LUPENGHAN wants to merge 1 commit into
1024XEngineer:MVPfrom
LUPENGHAN:feature/reminder-dispatch-pipeline

Conversation

@LUPENGHAN

Copy link
Copy Markdown
Contributor

Summary

  • Closes Proposal:时间到达提醒完整闭环(时间监听 → 系统日历引用检查 → 提醒下发 + 日历清理) #102. Background dispatcher polls schedules entering their reminder window, checks system calendar existence over WS before reminding when a calendar ref is bound, and cancels the schedule when the calendar was deleted by the user.
  • Adds per-device asyncio.Lock in ConnectionManager to fix a real concurrency gap: the new background worker and the existing WS endpoint's own receive loop can now both write to the same connection, and Starlette's WebSocket.send() has no internal locking.
  • Scope for this round is calendar-only per the boundary constraints doc and prior discussion — alarm-related logic (system_alarm_ref_id) is left unused this round.

Test plan

Implements Issue #102: background dispatcher polls schedules entering
their reminder window, checks system calendar existence over WS before
reminding when a calendar ref is bound, and cancels when the calendar
was deleted. Adds per-device connection locking to fix a real
concurrency gap between the new worker and the existing WS endpoint.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review found three high-confidence issues in the new dispatch path. git diff --check and python3 -m compileall -q backend/src backend/tests pass; the test suite could not be run because pytest is not installed in the review environment.

if schedule.next_step is NextStep.NEEDS_REFS_CHECK:
ref_id = schedule.system_schedule_ref_id
assert ref_id is not None
await self._connections.send(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Route by the key used by ConnectionManager — run_websocket_session registers connections under the URL device_id, but this dispatcher passes the schedule's user_id. Those are distinct fields in the model, so production sends will normally return False and drop both the refs-check and reminder messages. Carry the target device ID (or add an explicit user-to-device mapping) before dispatching.

snapshots = TimeWindowTriggerService(query_port).find_snapshots_entering_window(now)
for snapshot in snapshots:
try:
command_port.mark_triggered(snapshot.schedule_id, now)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not mark the trigger consumed before delivery succeeds. mark_triggered is committed here, while ConnectionManager.send can return False for an offline/unmapped device and the dispatcher ignores that result. The schedule then no longer matches the query and its reminder is lost permanently. Only commit the consumed state after a successful dispatch, or persist a retryable outbox/state.

if (result.ok and result.system_schedule_exists is not None)
else True
)
await dispatcher.handle_refs_check_reply(result.schedule_id, calendar_exists)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Validate the reply origin before mutating the pending schedule. device_id is ignored and the pending table is keyed only by schedule_id, so any connected WebSocket can submit a forged system.refs.check.result for another user's schedule and force either cancellation or reminder dispatch. Bind the pending check to the originating device/user and require that identity here.

@LUPENGHAN

Copy link
Copy Markdown
Contributor Author

设计被替代:系统日历/系统闹钟同步机制整体拿掉,时间维度改用客户端本地 AlarmManager,地理围栏命中后不再经过系统引用检查。这个 PR 对应的旧设计不再合并,后续会有新 issue 覆盖。

@LUPENGHAN LUPENGHAN closed this Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant