Summary
Failed-parse telemetry can drop valid flag names when a positional argument appears before them.
This was observed during the 0.4.0 RC cloud smoke test:
chctl cloud org usage ORG_ID --from-date yesterday --to-date today
The command correctly exits 2 because the date values are invalid. Its telemetry event is:
{
"command": "cloud org usage",
"flags": [],
"exit_code": 2,
"outcome": "invalid_value"
}
The expected flags are:
No flag values or positional values should be collected.
Why it matters
outcome = "invalid_value" without the relevant flag names loses much of the useful signal. Positional-first command shapes are common, so parse-error analytics will systematically undercount flags appearing after a positional.
Successful parses do not have this problem: capture() receives ArgMatches, skips positional values, and still records all command-line flag names.
Cause
This is a follow-on to #320.
capture_lossy() in crates/clickhousectl/src/telemetry.rs walks raw argv against clap definitions. It handles defined flags and subcommands, but does not consume defined positional arguments. At a leaf command, ORG_ID is not a subcommand, so it reaches the final else { break; }. The walker never examines --from-date or --to-date.
This behavior preserves privacy, but treats a known positional slot the same as an unknown token.
Possible fix
Extend the definition-anchored walker to understand the current command's positional slots:
- Track eligible positional definitions and their arity/index at the current command.
- When a token can satisfy a defined positional, consume it without retaining or recording the value.
- Continue walking so later canonical flag names can be captured.
- Keep the existing conservative behavior when no positional can accept the token: stop at the unknown token.
- Keep stopping at
--, and keep skipping values consumed by named flags.
This avoids transmitting any user-provided string: the event still contains only names cloned from clap definitions.
If reproducing enough of clap's positional semantics becomes fragile (variadic positionals, last, trailing_var_arg, hyphenated values), another option is to obtain partial matches from a cloned clap command and feed those through the existing definition-only capture() path. That should only be used if tests prove it cannot weaken the current privacy guarantees.
Acceptance criteria
- The reproduction records
command = "cloud org usage", flags = ["from-date", "to-date"], exit 2, and outcome = "invalid_value".
- No positional value, invalid flag value, or unmatched token appears in the payload.
- Unknown tokens still stop capture conservatively when no defined positional can consume them.
-- still prevents anything after it from being considered.
- Existing typo, suggestion, alias, short-flag, and hostile-argv privacy tests remain green.
- Unit tests cover required, optional, and variadic positional shapes, including flag values or positional values that resemble defined flags/subcommands.
- Add an end-to-end telemetry test for a failed parse with a positional before the invalid flag.
Summary
Failed-parse telemetry can drop valid flag names when a positional argument appears before them.
This was observed during the 0.4.0 RC cloud smoke test:
chctl cloud org usage ORG_ID --from-date yesterday --to-date todayThe command correctly exits 2 because the date values are invalid. Its telemetry event is:
{ "command": "cloud org usage", "flags": [], "exit_code": 2, "outcome": "invalid_value" }The expected flags are:
No flag values or positional values should be collected.
Why it matters
outcome = "invalid_value"without the relevant flag names loses much of the useful signal. Positional-first command shapes are common, so parse-error analytics will systematically undercount flags appearing after a positional.Successful parses do not have this problem:
capture()receivesArgMatches, skips positional values, and still records all command-line flag names.Cause
This is a follow-on to #320.
capture_lossy()incrates/clickhousectl/src/telemetry.rswalks raw argv against clap definitions. It handles defined flags and subcommands, but does not consume defined positional arguments. At a leaf command,ORG_IDis not a subcommand, so it reaches the finalelse { break; }. The walker never examines--from-dateor--to-date.This behavior preserves privacy, but treats a known positional slot the same as an unknown token.
Possible fix
Extend the definition-anchored walker to understand the current command's positional slots:
--, and keep skipping values consumed by named flags.This avoids transmitting any user-provided string: the event still contains only names cloned from clap definitions.
If reproducing enough of clap's positional semantics becomes fragile (variadic positionals,
last,trailing_var_arg, hyphenated values), another option is to obtain partial matches from a cloned clap command and feed those through the existing definition-onlycapture()path. That should only be used if tests prove it cannot weaken the current privacy guarantees.Acceptance criteria
command = "cloud org usage",flags = ["from-date", "to-date"], exit 2, andoutcome = "invalid_value".--still prevents anything after it from being considered.