Repository navigation
fix: reject surplus command arguments - #95
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 22, 2026, 8:10 AM ET / 12:10 UTC. ClawSweeper reviewWhat this changesUpdates Commander to 0.3.0 to reject surplus arguments, preserves bulk commands and renamed executables, and adds regression tests and documentation. Merge readiness✅ Ready for maintainer review This fix remains necessary: current main and v0.3.7 still accept surplus positional arguments. No blocking correctness defect was found, and the collaborator-authored PR is not eligible for automatic cleanup. Priority: P2 Review scores
Verification
How this fits togetherremindctl routes terminal arguments through Commander before executing commands against Apple Reminders. Argument validation determines whether command handlers run or the CLI returns an error. flowchart TD
A[Terminal arguments] --> B[Aliases and global controls]
B --> C[Command routing]
C --> D[Validate argument counts]
D --> E[Reject malformed invocation]
D --> F[Execute reminder command]
F --> G[Apple Reminders and terminal output]
Before mergeNone. Agent review detailsSecurityNone. Review metricsNone. Technical reviewBest possible solution: Keep argument validation in Commander, explicitly preserve supported bulk inputs, and reject malformed requests before command execution. Do we have a high-confidence way to reproduce the issue? Yes, from source: current main's parser accepts both positionals in Is this the best way to solve the issue? Yes. Upgrading the existing parser and declaring the three supported variadic arguments avoids duplicating validation in individual handlers. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning medium; reviewed against 2a1da5fab273. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
Commander 0.2.4 silently discarded surplus positional arguments: for example,
remindctl completion bash unexpectedreturned success. Update to Commander 0.3.0 so malformed invocations fail before command execution, and use its explicit root-argument API to preserve renamed executables without the old argv rewrite helper.Declare the existing multi-value arguments on
list,complete, anddeleteexplicitly so strict validation retains bulk operations. Update the list-action test helper, document the behavior, and record the fix in Unreleased.Validation: the new surplus-argument regression failed before the update; all 9 focused routing/list-action tests pass afterward, including bulk targets, renamed binaries, and literal controls after
--. Swift format and strict SwiftLint pass. The docs build passed on AWS Crabbox. The release harness requires macOScsreqand is covered by the PR’s macOS CI, along with the full Swift coverage and universal-binary gates. Independent Codex autoreview found no actionable P0–P2 findings.