fix: validate CLI mutation intent and literal arguments - #88
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:14 AM ET / 07:14 UTC. ClawSweeper reviewWhat this changesThe PR validates list mutation requests before Reminders access, preserves literal arguments after Merge readiness✅ Ready for maintainer review The fixes remain necessary on current main. Review found no introduced blocking defect, and this collaborator-authored PR is not eligible for cleanup closure. Priority: P2 Review scores
Verification
How this fits togetherremindctl translates terminal commands into Apple Reminders reads and writes through EventKit. Its command router and input validation determine which operation reaches the store and which arguments become reminder data. flowchart TD
A[Terminal arguments] --> B[Command router]
B --> C[Help or version output]
B --> D[Parse command inputs]
D --> E{Inputs valid?}
E -->|No| F[Error before store access]
E -->|Yes| G[EventKit reads and writes]
G --> H[Reminder or list output]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Keep command-intent validation at the CLI boundary while preserving valid list operations, literal positional data, and existing EventKit authorization. Do we have a high-confidence way to reproduce the issue? Yes, from source: current main selects deletion for a named list with both delete and rename flags, intercepts controls after Is this the best way to solve the issue? Yes. The patch corrects the existing command boundary using the pinned parser's semantics and retains established valid operations without introducing configuration or storage changes. 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
|
list --delete --rename Newsilently chose deletion, while mutation flags without a target fell through to listing. Parse mutation flags into one validated action before touching EventKit; reject conflicts and missing targets while keeping ordinary reads and single mutations intact.The router also treated help/version text after
--as a global control, and used the executable filename as the Commander command name. Honor the tokenizer's terminator boundary and resolve the canonical descriptor so literal titles and renamed executables work. Require finite positive geofence radii:inf,+inf, and1e309previously passed validation and reached geocoding.Regression coverage exercises actual router entry points, parsed list actions, empty targets, conflicting mutation combinations, valid actions, and finite-radius boundaries. Documentation and Unreleased notes describe the corrected behavior.
Live proof: the original CLI deleted a disposable list for the conflicting delete/rename request, rejected a renamed executable, and intercepted help/version after
--. The rebuilt signed CLI rejects every conflict and missing target, preserves the same list ID and contents, accepts literal--help/--versionreminder titles, runs under a different executable filename, rejects nonfinite radii before geocoding, and still adds/renames/reads normally. All synthetic data was removed. Radius regression tests first failed forinf,+inf, and1e309, then passed with the finite-value check.Validation: strict Swift/shell/workflow lint, the full Swift suite and coverage-gate tests pass; RemindCore coverage remains 94.0% (958/1019).
make buildandmake docs-sitepass. Final isolated Codex autoreview is scoped-clean through P2.