#35831 argsman, cli: Allow options after non-option arguments (GNU-style)
| Reviewability | Reviews | Agreement | Size | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| #35831 argsman, cli: Allow options after non-option arguments (GNU-style) | Author silent 80d | 1 | Positive | 103 + 182 tests | |||||||||
|
|
|
|
| |||||||||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||||||||
Goal
This PR updates ArgsManager to parse options that appear after non-option arguments (subcommands and positional parameters) using GNU-style conventions. Unrecognized options following a command now produce errors instead of being silently swallowed into the command's positional arguments, and the '--' separator is supported to terminate option parsing. Additionally, bitcoin-cli and bitcoin-tx are updated to consume the pre-filtered arguments from ArgsManager.
Problem: In existing code, any argument following the first non-option argument was treated verbatim as a positional command argument. Consequently, trailing options such as -rpcwallet were silently ignored, potentially sending commands to the wrong wallet or triggering unexpected RPC errors when misspelled options were absorbed as positional values.
Reviewability
Stale · Author silent 80d
No author activity for 80 days; a review may wait a long time for a response.
Waiting on (from the thread read):
- reviewer: vicjuma to check the fix in bitcoin-tx 2026-08-13
Agreement
Positive
support without stated reasons, no open objection (w0xlt, w0xlt, w0xlt)
Thread notes: Re-checked claims from schema 1: updated clears_with from answer to change for all 5 objections since each identified a defect requiring a code fix. All defects were fixed in the latest push and confirmed by w0xlt's ACK. / Updated claim clears_with attributes to change for concrete defects. Marked w0xlt's objections as accepted following their ACK of the fix commit.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| vicjuma (contributor) | objection, interface | trailing options are not supported across all tools such as bitcoin-tx, causing errors | fixed · fix pushed · fixed in push by updating CommandLineRawTx in bitcoin-tx to use GetCommand() | changed | yes | 2026-08-11: 'I am a bit skeptical though cause probably it will have to be replicated across all the other targets too as opposed to just `bitcoin-cli`. This will mean it should be possible to use this in `bitcoin' Status from: 2026-08-13: pablomartin4btc: 'Fixed: `bitcoin-tx` `CommandLineRawTx` now uses `gArgs.GetCommand()` instead of re-scanning raw `argv`' |
| w0xlt (contributor) | objection, correctness | using '--' before a command fails with an invalid parameter error instead of ending option processing | fixed · fix pushed · fixed in push and confirmed by reviewer ACK | changed | yes | 2026-08-12: '`--` is only recognized after the command, so using it before the first positional argument (for example, `bitcoin-wallet -help -- create`) incorrectly fails with `Invalid parameter --`.' Status from: 2026-08-13: w0xlt: 'ACK 67a8e5bea2e30c66d7f4eb33e8b124943c5d705f' |
| w0xlt (contributor) | objection, correctness | on Windows, post-command slash options with path values containing slashes are mistaken for positional arguments | fixed · fix pushed · fixed in push and confirmed by reviewer ACK | changed | yes | 2026-08-13: 'on Windows, a post-command option such as `/datadir=C:/bitcoin` is mistaken for a positional argument because its value contains `/`.' Status from: 2026-08-13: w0xlt: 'ACK 67a8e5bea2e30c66d7f4eb33e8b124943c5d705f' |
| w0xlt (contributor) | objection, correctness | a lone hyphen after a command is rejected as an invalid option instead of being treated as a positional argument | fixed · fix pushed · fixed in push and confirmed by reviewer ACK | changed | yes | 2026-08-13: 'A lone - after a command, such as in `bitcoin-cli echo -`, is incorrectly treated as an option and rejected with `Invalid parameter -`.' Status from: 2026-08-13: w0xlt: 'ACK 67a8e5bea2e30c66d7f4eb33e8b124943c5d705f' |
| w0xlt (contributor) | objection, correctness | bitcoin-tx rereads raw arguments and misinterprets trailing options like -json as transaction commands | fixed · fix pushed · fixed in push and confirmed by reviewer ACK | changed | yes | 2026-08-13: 'the parser correctly recognizes a trailing option like `-json`, but `bitcoin-tx` later rereads the original arguments and mistakenly treats `-json` as a transaction-editing command, causing error: `un' Status from: 2026-08-13: w0xlt: 'ACK 67a8e5bea2e30c66d7f4eb33e8b124943c5d705f' |
Support:
- w0xlt (contributor) (Concept ACK, 2026-07-29): Concept ACK. [not substantive]
- w0xlt (contributor) (Approach ACK, 2026-08-13): Approach ACK [not substantive]
- w0xlt (contributor) (ACK, 2026-08-13): ACK 67a8e5bea2e30c66d7f4eb33e8b124943c5d705f [not substantive]
Participants:
- w0xlt (contributor, objection, 7 statements 2026-07-29 to 2026-08-13): identified multiple parsing edge cases that were resolved, gave Concept ACK, Approach ACK, and full ACK
- vicjuma (contributor, objection, 2 statements 2026-08-11 to 2026-08-11): noted inconsistency and failure in bitcoin-tx when using trailing options
Files
191 lines under test/bench/ci.
- src/test/argsman_tests.cpp +167/-6
- src/common/args.cpp +68/-10
- src/bitcoin-tx.cpp +17/-12
- doc/release-notes-35831.md +14/-0
- test/functional/interface_bitcoin_cli.py +10/-3
- src/bitcoin-cli.cpp +4/-7
- test/functional/data/util/bitcoin-util-test.json +5/-0
Card
The short text the ranking pass compares across PRs.
This PR implements GNU-style command-line option parsing in ArgsManager, allowing options to be specified after subcommands and positional arguments. It prevents misplaced or misspelled options from being silently treated as positional parameters, fixes wallet routing errors with trailing -rpcwallet arguments, and adds '--' delimiter support. The changes directly benefit bitcoin-cli, bitcoin-wallet, and bitcoin-tx. All review feedback on edge cases has been addressed and the PR is ACKed by w0xlt.
Data
thread read through 2026-08-13 (14 statements), head 67a8e5bea2
labels: none
Categories
RPC / REST / ZMQ
P3 · user request
The change fixes unexpected CLI behavior where trailing flags such as -rpcwallet were silently absorbed into positional arguments instead of being parsed as options. This provides standard GNU-style argument handling for bitcoin-cli users without altering RPC wire protocols.
Membership: Modifies src/bitcoin-cli.cpp and interface_bitcoin_cli.py tests.
Factors: security/stability 1, bug 1, performance 0, user value 2, leverage 0
Tools and scripts
P3 · cleanup (assessed alone as P4; Raised from P4: silently swallowing options after a command (e.g. trailing -rpcwallet) is a real user-facing CLI correctness problem, not incidental cleanup.)
Updating bitcoin-tx to consume pre-filtered arguments from ArgsManager is an incidental update following the core argument parsing change.
Membership: Modifies src/bitcoin-tx.cpp to adapt to the new argument parsing conventions.
Factors: security/stability 1, bug 1, performance 0, user value 2, leverage 0
Utilities (logging, arguments, libraries)
P2 · bug fix (ranking pass: Moved up: silently swallowed trailing options are a user-facing parsing bug of the same class as the other ArgsManager P2s.)
Options following positional commands were previously swallowed silently, creating traps where options like -rpcwallet were ignored and unintended commands or wallets were targeted. Updating ArgsManager to support GNU-style conventions and the '--' terminator eliminates a subtle footgun in shared option parsing.
Membership: Modifies ArgsManager option handling in src/common/args.cpp.
Factors: security/stability 1, bug 1, performance 0, user value 2, leverage 0