Skip to content

Fix negative numeric long-flag parsing#18

Closed
clawsweeper[bot] wants to merge 1 commit into
mainfrom
clawsweeper/issue-openclaw-goplaces-17
Closed

Fix negative numeric long-flag parsing#18
clawsweeper[bot] wants to merge 1 commit into
mainfrom
clawsweeper/issue-openclaw-goplaces-17

Conversation

@clawsweeper

@clawsweeper clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Normalize known numeric long flags when a negative numeric value is provided as the following space-separated token before Kong parses args.
  • Add mock-server CLI regressions for search, nearby, autocomplete, and directions using negative longitude/coordinate examples.
  • Preserve existing --flag=value behavior and normal invalid numeric errors.

Fixes #17

Validation

  • go test ./internal/cli -run 'TestNormalizeNegativeNumericFlagArgs|TestRunSearchAcceptsSpaceSeparatedNegativeLongitude|TestRunNearbyAcceptsSpaceSeparatedNegativeLongitude|TestRunAutocompleteAcceptsSpaceSeparatedNegativeLongitude|TestRunDirectionsAcceptsSpaceSeparatedNegativeCoordinates' -count=1
  • go test ./...

Credit

Reported by @technicalpickles in #17.

@clawsweeper clawsweeper Bot added clawsweeper Tracked by ClawSweeper automation clawsweeper:autogenerated PR created automatically by ClawSweeper clawsweeper:autofix Maintainer opted this PR into bounded ClawSweeper-reviewed autofix without merge labels Jul 3, 2026
@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Codex review: passed. Reviewed July 3, 2026, 5:51 PM ET / 21:51 UTC.

Summary
The PR adds a CLI argument pre-normalizer for known numeric long flags so space-separated negative values are converted to equals form before Kong parsing, plus regression tests for search, nearby, autocomplete, and directions.

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.

  • CLI files touched: 4 files changed, 270 added, 1 removed. The patch is scoped to CLI argument normalization and regression tests, which keeps the fix boundary narrow.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #17
Summary: This PR is the generated fix candidate for the linked negative numeric flag parsing issue.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Next step before merge

  • [P2] No repair lane is needed because the PR has no concrete automated-fix blocker from this review.

Security
Cleared: The diff only changes local CLI argument normalization and Go tests, with no dependency, workflow, credential, or package-resolution changes.

Review details

Best 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 changes

Label changes:

  • add P2: The PR fixes a documented CLI parsing bug with limited blast radius and an existing equals-form workaround.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This is a ClawSweeper bot autofix PR, so the external-contributor real behavior proof gate does not apply.

Label justifications:

  • P2: The PR fixes a documented CLI parsing bug with limited blast radius and an existing equals-form workaround.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This is a ClawSweeper bot autofix PR, so the external-contributor real behavior proof gate does not apply.
Evidence reviewed

What I checked:

  • Current parser path: Current main constructs the Kong parser and passes the original args directly into parseWithExit, so main has not already implemented this normalization. (internal/cli/run.go:54, b3fba0220115)
  • Documented flag contract: README documents that long flags accept either space-separated or equals form, which makes the linked negative longitude report a docs-backed CLI contract bug rather than a new feature. (README.md:93, b3fba0220115)
  • Patch scope: The PR adds internal/cli/args.go, internal/cli/args_test.go, updates internal/cli/run.go to normalize args before parsing, and adds mock-server regressions in internal/cli/cli_test.go. (internal/cli/run.go:51, 3ffec0431ead)
  • GitHub PR state: GitHub reports the PR as mergeable/CLEAN with successful test and audit check runs, and it closes the linked issue. (3ffec0431ead)
  • Target policy check: No AGENTS.md was found inside the target goplaces repository. (b3fba0220115)

Likely related people:

  • Peter Steinberger: Current-main blame ties the CLI parser setup, numeric coordinate flag definitions, and latest release-tagged CLI behavior to the same commit. (role: introduced behavior and recent area contributor; confidence: high; commits: cb837848f1bb; files: internal/cli/run.go, internal/cli/root.go)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal priority bug or improvement with limited blast radius. and removed clawsweeper:autofix Maintainer opted this PR into bounded ClawSweeper-reviewed autofix without merge labels Jul 3, 2026
@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

🦞✅
ClawSweeper autofix is complete for this exact head.

Source: clawsweeper[bot]
Feedback: structured ClawSweeper verdict: pass (sha=3ffec0431eadd8e4c8c8c35e6187053437163322)
Head: 3ffec0431ead

No actionable findings remain. I removed the repair-loop label and left the PR open for maintainer review and merge.

Automerge progress:

  • 2026-07-03 21:51:59 UTC review passed 3ffec0431ead (structured ClawSweeper verdict: pass (sha=3ffec0431eadd8e4c8c8c35e6187053437163...)

@technicalpickles

Copy link
Copy Markdown

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):

  • Reproduced the original bug on main: a search call with a space-separated negative longitude fails with --lng: expected float value but got "<value>" (short flag).
  • Same command on this branch gets past parsing and reaches the Places API (400 API_KEY_INVALID, as expected with a fake key) — confirmed for search (including the exact repro shape from --lng (and any negative numeric flag) misparsed as a new flag in space-separated form #17) and for directions with four negative coordinates (negative lat/lng on both endpoints).
  • Equals-form (--lng=<value>) still works, no regression.
  • go test ./... passes on this branch.

LGTM from a user-testing perspective — thanks for the fast turnaround.

@steipete

steipete commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

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.

@steipete steipete closed this Jul 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clawsweeper:autogenerated PR created automatically by ClawSweeper clawsweeper Tracked by ClawSweeper automation P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--lng (and any negative numeric flag) misparsed as a new flag in space-separated form

2 participants