fix(list): skip reminders with a missing calendar - #72
Conversation
An orphaned EventKit reminder with a nil calendar trapped list. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
🦞👀 Pull request received. I will update this pull request when review starts. |
Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
Codex review: needs real behavior proof before merge. Reviewed August 27, 2026, 8:17 AM ET / 12:17 UTC. ClawSweeper reviewWhat this changesThe PR makes reminder listing skip EventKit reminders with no calendar identity, adding a small mapping helper and coverage for missing and present calendar values. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 3 items remain This PR remains necessary: current main still force-dereferences Priority: P2 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherThe flowchart LR
A[EventKit reminder store] --> B[Reminder fetch]
B --> C[Calendar identity mapping]
C -->|calendar present| D[Reminder snapshot]
C -->|calendar missing| E[Skip orphaned row]
D --> F[List command]
F --> G[Terminal output]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the optional-calendar guard at the EventKit snapshot boundary and support it with a redacted macOS run showing Do we have a high-confidence way to reproduce the issue? No high-confidence live reproduction is provided. Current main directly dereferences the calendar during fetched-reminder conversion, while the PR’s evidence exercises only a synthetic mapping helper. Is this the best way to solve the issue? Yes; handling the optional calendar at the EventKit snapshot boundary is the narrowest maintainable repair and leaves command rendering unchanged. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning high; reviewed against df4b872c1646. LabelsLabel 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
HistoryReview history (25 earlier review cycles; latest 8 shown)
|
What Problem This Solves
remindctl listforce-unwrapsreminder.calendar. An orphaned EventKit reminder with a missing calendar traps the process.Why This Change Was Made
EventKit exposes calendar as an implicitly unwrapped optional. Nil is a real store state, not a programmer error.
User Impact
remindctl listskips the orphaned row and prints the rest of the list.Evidence
terminal output from the calendar mapping helper:
Real behavior proof
Behavior addressed: Reminders with a nil calendar are skipped instead of trapping list.
Real environment tested: macOS, Swift toolchain, clone at /tmp/pr-remindctl on the patched branch.
Exact steps or command run after this patch: swift test --filter ReminderCalendarMappingTests
Evidence after fix: terminal output copied below.
Observed result after fix: A nil calendar id maps to skip. A present id still produces a list identity.
What was not tested: A live EventKit store that already contains an orphaned reminder.