Fix negative numeric long-flag parsing#18
Conversation
|
Codex review: passed. Reviewed July 3, 2026, 5:51 PM ET / 21:51 UTC. Summary Reproducibility: yes. The linked issue gives a concrete failing command, current main passes raw args into Kong, and docs advertise the affected space-separated long-flag form; I did not run the CLI in this read-only review. Review metrics: 1 noteworthy metric.
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:
Next step before merge
Security Review detailsBest possible solution: Land the narrow parser compatibility shim with its focused CLI regression coverage after normal PR gates complete. Do we have a high-confidence way to reproduce the issue? Yes. The linked issue gives a concrete failing command, current main passes raw args into Kong, and docs advertise the affected space-separated long-flag form; I did not run the CLI in this read-only review. Is this the best way to solve the issue? Yes. Normalizing only known numeric long flags before Kong parsing is a narrow compatibility shim that preserves the documented flag contract without broad parser replacement. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning high; reviewed against b3fba0220115. 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
|
|
🦞✅ Source: No actionable findings remain. I removed the repair-loop label and left the PR open for maintainer review and merge. Automerge progress:
|
|
Built this branch locally and manually verified the fix against the real API (using an invalid key, so it fails on auth rather than parsing — confirms the argv normalization works end-to-end, not just in the mock-server unit tests):
LGTM from a user-testing perspective — thanks for the fast turnaround. |
|
Superseded by the maintainer rewrite in #19, landed as 1d0fd5c. The original branch could not be updated because maintainer edits were disabled. The landed commit preserves the bot contribution as a co-author and uses Kong's supported parser option instead of a hand-maintained numeric-flag allowlist. Thanks for the fix and regression coverage. For future external branches, enabling “Allow edits by maintainers” lets us improve the original PR in place. |
Summary
Fixes #17
Validation
Credit
Reported by @technicalpickles in #17.