Repository navigation
fix: deduplicate reminder targets before delete and complete - #98
Conversation
delete 1 1 resolved the same reminder twice, deleted it, then failed the second lookup. Keep the first id so the prompt and the delete see one reminder. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
🦞👀 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 October 7, 2026, 7:50 AM ET / 11:50 UTC (Revision 15). ClawSweeper reviewWhat this changesThe PR makes delete and complete process each resolved reminder once, preserves input order and validation, and adds regression tests and documentation. Merge readiness✅ Ready for maintainer review The fix remains necessary: current main and v0.3.8 still resolve repeated targets multiple times. No blocking correctness finding or additional contributor action remains. Priority: P2 Review scores
Verification
How this fits togetherremindctl translates command-line indexes and ID prefixes into Apple Reminders targets. Delete and complete use those targets for previews and EventKit mutations. flowchart TD
A[Indexes and ID prefixes] --> B[Fetch reminders]
B --> C[Resolve and validate targets]
C --> D[Keep first occurrence]
D --> E[Preview and confirmation]
D --> F[EventKit mutation]
F --> G[Command result]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Use one ordered target per reminder identity throughout preview, confirmation, and mutation. Do we have a high-confidence way to reproduce the issue? Yes: main resolves a repeated ID twice, and deletion commits removal for each occurrence, causing the second lookup to fail. This review established the mechanism from source without executing target code. Is this the best way to solve the issue? Yes: deduplicating resolved identities in the shared resolver aligns previews and mutations while preserving order and validating every input. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning medium; reviewed against cdd7a06b8527. LabelsLabel changes: No label changes. Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (14 earlier review cycles; latest 8 shown)
|
Cover repeated IDs and indexes, mixed prefixes, filtered numeric views, first-input order, and validation after duplicates. Document unique delete and complete targets. Co-authored-by: Sebastien Tardif <sebtardif@ncf.ca>
Passing the same reminder more than once to
deletepreviously counted it twice and then failed after the first removal. Resolve each reminder ID once, preserving the first input order, so delete and complete share consistent previews, counts, and mutation targets. Numeric indexes, full IDs, and prefixes that identify the same reminder are deduplicated after resolution; every input still must be valid and unambiguous.Sebastien Tardif (@SebTardif) contributed the resolver fix and verified deletion of a disposable reminder through EventKit. This update retains that implementation and commit, extends regression coverage, and documents the behavior.
Validation:
nice -n 19 swift test --jobs 2 --filter IDResolverTestspasses all 10 tests with the fix, covering repeated indexes/full IDs, mixed prefixes and case, filtered numeric views, first-input order, and invalid subsequent inputs.git diff --checkpass.