refactor: simplify command registration and decoding - #85
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, 2:34 AM ET / 06:34 UTC (Revision 2). ClawSweeper reviewWhat this changesThe PR shares command registration with shell completion, checks handler concurrency safety, removes unused decoding helpers, and simplifies equivalent location and app-opening branches. Merge readiness✅ Ready for maintainer review This remains a useful, bounded cleanup absent from the reviewed main branch. No actionable correctness or security defect was found; collaborator-authored work is also protected from automatic closure. Priority: P3 Review scores
Verification
How this fits togetherremindctl translates terminal arguments into Apple Reminders operations through RemindCore and EventKit. Its command registry also supplies help and shell completion. flowchart TD
A[Terminal arguments] --> B[Command router]
C[Shared command registry] --> B
C --> D[Help and shell completion]
B --> E[Parsed values and runtime options]
E --> F[Command handlers]
F --> G[Apple Reminders and terminal output]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Use one immutable command registry and compiler-checked handlers while preserving existing CLI behavior. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR removes duplication and unchecked concurrency declarations rather than reporting a runtime failure; no introduced defect was identified. Is this the best way to solve the issue? Yes. Sharing the existing registry and checking its handler types addresses the duplication narrowly, and the simplified branches preserve their existing decisions. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning medium; reviewed against 1b71fb003087. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
Shell completion duplicated the router's command list, command specifications bypassed Sendable checking, and internal option-decoding helpers had no callers. Share the command registry, require
@Sendablehandlers, remove the unused helpers, and collapse equivalent app-opening and location branches.Commander 0.2.4 already makes command signatures and parsed values Sendable. Public command flags, aliases, output, the RemindCore API, and the macOS/Swift floors are preserved.
Validation:
make checkpasses strict Swift/shell/workflow lint, 92 tests and 93.8% RemindCore line coverage (898/957); built CLI passes 40 before/after byte comparisons against a disposable synthetic EventKit list, covering show/search/info/export/list/status/link in every output mode plus help, version and bash/zsh completion. All synthetic data was removed. macOS's/bin/bashwas selected for local scripts after diagnosing a Homebrew Bash heredoc stall.Exact independent head:
918243fb0c47e66415e8dbd6d2119b91fa178a06. CI. The identical reviewed patch was rebased onto main without the other cleanup commits (stable patch ID unchanged), then local gates and live binary comparisons were repeated. Isolated Codex autoreview is scoped-clean through P2.