Skip to content

fix(cli): reject zero, negative and reversed numeric arguments - #654

Merged
Ziinc merged 5 commits into
mainfrom
ccr-b84e8000-3ab84a-numeric-args
Oct 3, 2026
Merged

Ziinc merged 5 commits into
mainfrom
ccr-b84e8000-3ab84a-numeric-args

Conversation

@Ziinc

@Ziinc Ziinc commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator
  • clap read -1 as an unknown flag. Value-taking args now accept negative numbers as values, so validation rejects them as non-positive.
  • --limit 0, --start-line 0 / --end-line 0, file read with start > end, and --workspace <= 0 are now invalid_arguments (matching agent-review).
  • file read --start-line N without --end-line reads N..N+299; it used to read N..300, which returned nothing and looked like EOF.
    🤖 Generated with Claude Code
    https://claude.ai/code/session_01MxqTt1WtVRKkjNXQpgkJS9
    Generated by Claude Code
    Generated by Claude Code

clap treated `-1` as an unknown flag, so negative values failed with a
confusing parse error. Value-taking args now accept negative numbers as
values, and validation rejects them with a clear message: --limit,
--start-line and --end-line must be positive, `file read` rejects
start-line > end-line (matching agent-review), --workspace must be a
positive id, and agent-review line numbers must be positive.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MxqTt1WtVRKkjNXQpgkJS9
…umeric-args

# Conflicts:
#	src-tauri/src/cli/tests.rs

@Ziinc Ziinc left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at ea5e8a47: Reviewed negative-number parsing, positive ID/limit/line validation, reversed line ranges, and the parser regression tests. No actionable correctness or code-quality issues found in this diff.

Validation: static review of the diff, surrounding implementation and tests; PR-head CI reports success. Rust/integration tests were not rerun locally because this environment has no Cargo toolchain.

…line

`treq file read --start-line 301` without `--end-line` used the absolute
default end of line 300, read 301..300 and returned no lines with exit 0,
so an agent paging a long file stopped as if it had reached EOF. The
default end is now start-line + 299.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E1poGWXMSFcPsJ8V5gujWG
Keep both test modules in cli/tests.rs and this PR's --end-line description.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MxqTt1WtVRKkjNXQpgkJS9
…mmit test

CreateCommit returns its error as a JSON envelope on stdout, so a losing
process now fails with "nothing to commit" there rather than on stderr.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MxqTt1WtVRKkjNXQpgkJS9

Ziinc commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator Author

CI: test-rust / test / ubuntu-22.04 failed on ac0af69 after merging main. cli_parallel_commit_test (#678) has been failing on main since #659 merged, because the losing processes now report "nothing to commit" on stdout. I ported the fix from #705 into this PR (the test now passes locally); it becomes a no-op once #705 lands.


Generated by Claude Code

@Ziinc
Ziinc merged commit 915f961 into main Oct 3, 2026
23 checks passed
@Ziinc
Ziinc deleted the ccr-b84e8000-3ab84a-numeric-args branch October 3, 2026 11:12

This branch was successfully deployed

1 active deployment
preview — 1f644ec4 Deployed Oct 3, 2026 by Ziinc via build #1671
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants