Skip to content

Telemetry: lossy parse capture drops flags after positional arguments #342

Description

@sdairs

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:

["from-date", "to-date"]

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:

  1. Track eligible positional definitions and their arity/index at the current command.
  2. When a token can satisfy a defined positional, consume it without retaining or recording the value.
  3. Continue walking so later canonical flag names can be captured.
  4. Keep the existing conservative behavior when no positional can accept the token: stop at the unknown token.
  5. 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.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions