fix: validate reminder indexes and honor parsing time zones - #87
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. Reviewed September 13, 2026, 3:09 AM ET / 07:09 UTC. ClawSweeper reviewWhat this changesThe PR prevents crashes from invalid reminder indexes and makes local date parsing respect the caller’s time zone, with regression tests and documentation. Merge readiness✅ Ready for maintainer review The fixes remain necessary on current main and v0.3.6. The patch is focused, preserves existing caller contracts, and has no identified blocking defect. Priority: P2 Review scores
Verification
How this fits togetherRemindCore translates reminder identifiers and date strings for the macOS Reminders CLI. Its results determine which reminders commands select and how due dates and date filters are interpreted. flowchart LR
A[CLI input] --> B[RemindCore parsing]
C[Reminder listing] --> D[Validate numeric index]
B --> D
B --> E[Interpret date and time zone]
D --> F[Selected reminder or error]
E --> G[Due date or date filter]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Retain the focused bounds check and shared local-date formatter while preserving default-view selection and explicit ISO-offset precedence. Do we have a high-confidence way to reproduce the issue? Yes: current-main source subtracts one from Int.min before validation and ignores a supplied calendar’s time zone for local strings. The PR reports matching before/after execution; this review did not run code. Is this the best way to solve the issue? Yes: validating before arithmetic and passing the existing calendar time zone into local formatters directly repair the defects without adding configuration, changing public signatures, or rewriting stored data. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning medium; reviewed against d27218000139. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
The minimum integer reminder index crashed because the resolver subtracted one before checking bounds. Validate the one-based range first, preserving the default-view/numericFrom contract. Local date parsing also ignored the caller's Calendar time zone; all local date formats now share a formatter that takes it explicitly, while ISO offsets retain precedence.
Regression tests cover the integer extremes, zero/negative/out-of-range indexes, opposing GMT+14/GMT-10 zones, local ISO and formatted dates, all-day metadata, and explicit UTC offsets. Existing date spellings and public signatures remain supported.
Proof: a standalone executable compiled from the actual RemindCore sources trapped with exit -5 for Int.min and failed a GMT+14 date assertion before the change; the rebuilt probe passes both index and dual-time-zone checks. The built, locally signed CLI now returns exit 1 and
Invalid identifier: "-9223372036854775808".forinfo -- -9223372036854775808, instead of crashing.make checkpasses strict lint, the full Swift suite and coverage-gate tests, with 94.1% RemindCore coverage (950/1010);make buildandmake docs-sitepass. Isolated Codex autoreview is scoped-clean through P2.