fix: show reminder URLs in Reminders notes#60
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 30, 2026, 7:36 PM ET / 23:36 UTC. Summary Reproducibility: not applicable. for this PR as a patch review. The linked issue has user screenshots, and the PR body reports live macOS/EventKit proof for the proposed opt-in behavior. Review metrics: 2 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the opt-in managed-notes path only if maintainers accept it as the supported answer for #59; otherwise redirect that issue to a different app-visible URL approach. Do we have a high-confidence way to reproduce the issue? Not applicable for this PR as a patch review. The linked issue has user screenshots, and the PR body reports live macOS/EventKit proof for the proposed opt-in behavior. Is this the best way to solve the issue? Mostly yes: the latest opt-in shape preserves existing default URL behavior and isolates managed notes writes. The remaining question is whether OpenClaw should own that notes-mirroring contract. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning high; reviewed against 4e8643ff0ed9. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
@openclaw-mantis visual task: verify on macOS that remindctl add/edit --url mirrors one URL line into Reminders.app notes and --clear-url removes it. |
|
Added live Reminders.app proof to the PR body for add, edit, and clear-url behavior. @clawsweeper re-review |
|
Addressed the authored-note compatibility finding by switching the generated mirror to a tool-owned |
|
Fixed the notes-only preservation finding: when no URL mirror is being added, replaced, or cleared, authored notes now pass through unchanged, including spacing and empty strings. make check passes on the updated head. @clawsweeper re-review |
|
Updated head 65171b1 fixes the notes-only preservation finding and make check passes. @clawsweeper re-review |
|
Updated head 523a84c preserves non-managed note text exactly around URL add/replace/clear, including whitespace, blank lines, empty strings, and newline style. make check passes. @clawsweeper re-review |
|
Updated head 2cd2771 preserves authored empty notes on URL clear and make check passes with the focused regression. The remaining notes-mirror default is a maintainer product-contract decision. @clawsweeper re-review |
|
Current head 2cd2771 fixes the empty-string URL-clear edge and make check passes. The remaining notes-mirror default/prefix ownership question is a maintainer contract decision rather than a contributor code repair. @clawsweeper re-review |
|
Updated head fa22f14 uses an explicit |
|
Updated current head b6ca899 to preserve the existing default |
b6ca899 to
cd7e8f2
Compare
|
Landed as 5ac23a9. Thanks @TurboTheTurtle for the contribution and iterative proof work. Verification:
No release, version, tag, or registry action performed. |
Closes #59.
Keeps
--urlin the public EventKit URL field and also mirrors it to one tool-ownedremindctl URL (managed): ...notes line so the link is visible in Reminders.app.edit --urlreplaces that managed line, notes-only edits retain it, and--clear-urlremoves the EventKit value plus only the managed line while preserving authored notes.This rewrites the contributor baseline to make the existing
--urlcommand satisfy the reported behavior directly; no additional opt-in flag is required. AuthoredURL: ...and plainremindctl URL: ...notes remain untouched.Proof
swift test --filter ReminderURLNoteMirrorTests: 11 focused URL-mapping tests passed.make check: strict formatting/SwiftLint passed; 65 tests passed; RemindCore line coverage 91.1% (613/673, 90% required).make build: signed release CLI built successfully; builtadd --help/edit --helpshow the EventKit + Reminders notes contract.make docs-site: documentation site built successfully.inforeturned the EventKit URL plus managed notes link; notes-only edit retained it; URL edit replaced it;--clear-urlremoved the URL and only the managed line; reminder and list were deleted; cleanup verified no test list remained.No release, version, tag, or registry action included.