fix(store): bound EventKit fetchReminders with a 30s timeout - #66
fix(store): bound EventKit fetchReminders with a 30s timeout#66SebTardif wants to merge 1 commit into
Conversation
fetchReminders used an unbounded continuation. If EventKit never calls back, remindctl list hangs forever. Resume once with a 30s timeout and map the failure to operationFailed. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
Codex review: needs real behavior proof before merge. Reviewed August 2, 2026, 8:37 AM ET / 12:37 UTC. ClawSweeper reviewWhat this changesThe PR changes EventKit reminder retrieval to throw after 30 seconds rather than letting reminder-loading commands wait indefinitely for an EventKit callback. Merge readiness⛔ Blocked until stronger real behavior proof is added - 4 items remain This PR remains necessary: current main still waits on an EventKit continuation with no timeout, so a missing callback can block reminder-loading commands indefinitely. The timeout direction is focused and source-consistent, but the PR needs after-fix runtime evidence before merge because its body documents code review rather than an observed EventKit result. Priority: P2 Review scores
Verification
How this fits together
flowchart LR
A[CLI reminder commands] --> B[Reminders store]
B --> C[Calendar selection]
C --> D[EventKit reminder fetch]
D --> E[Callback or timeout]
E --> F[Reminder records or error]
F --> G[CLI output or command action]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land a bounded, single-resume fetch only after proving that normal EventKit retrieval still succeeds and that a deliberately stalled or controllably delayed retrieval reports the intended timeout without a double-resume failure. Do we have a high-confidence way to reproduce the issue? Yes, from current source: Is this the best way to solve the issue? Unclear: the timeout plus one-shot continuation is a narrow maintainable remedy, but its selected 30-second policy and post-fix behavior still need runtime proof before it is confirmed as the best final implementation. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning high; reviewed against f6ec31cf074a. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
What Problem This Solves
fetchRemindersusedwithCheckedContinuationwith no timeout. If EventKit never invokes the callback,remindctl list(and any path that loads reminders) hangs forever.Evidence
After
Task.sleep30s resumesoperationFailed("Timed out waiting for EventKit reminders after 30s")Real behavior proof
/tmp/oc-batch-20260802/remindctl.fetchRemindersisasync throwswith 30s bound; publicreminders(matching:)usestry await.RemindCoreError.operationFailed.Summary
30s timeout around EventKit
fetchReminderswith safe one-shot resume.