Skip to content

fix(cli): preserve compact-like option values - #5974

Open
13pgpg wants to merge 1 commit into
volcengine:mainfrom
13pgpg:fix/compact-preprocessor-values
Open

13pgpg wants to merge 1 commit into
volcengine:mainfrom
13pgpg:fix/compact-preprocessor-values

Conversation

@13pgpg

@13pgpg 13pgpg commented Oct 13, 2026

Copy link
Copy Markdown

Description

preprocess_cli_args runs before Clap. Its compact preprocessor used to rewrite every -c or --compact token, including a token that belongs to another option's value or appears after --. This changed the argv that Clap actually received and produced a misleading compact-option error.

The preprocessor now uses the existing Clap-derived cli_value_options() metadata to pass value-bearing options and their next argument through unchanged. It also stops at --. Commands without a compact token return immediately, so they do not build the option metadata. Actual bare -c / --compact and explicit -c false / --compact false retain their existing behavior.

Human Involvement

  • A human participated in the implementation or review loop
  • This PR was generated entirely by AI agents without human participation in the loop

Related Issue

Fixes #5665. This change is limited to the compact argv preprocessor. Whether Clap accepts hyphen-prefixed values for particular options remains a separate parser concern; #5571 addresses --content.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • Preserve -c / --compact when they are values of another option.
  • Preserve all arguments after -- and retain compact boolean forms.
  • Add a regression test for --account, --content, the -- separator, and --compact false.

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Rust 1.91.1 on macOS x86_64:

  • cargo test -p ov_cli --bin ov compact --locked — 20 passed.
  • cargo test -p ov_cli --bin ov --locked -- --skip curated_help_lists_timeout_for_waiting_commands — 508 passed, 1 filtered.
  • Unfiltered CLI unit tests: 508 passed, 1 failed. The unrelated help_ui::tests::curated_help_lists_timeout_for_waiting_commands expects a timeout option in ov add-skill help; the same test also fails on an unmodified main checkout.
  • The config_override integration binary did not complete in this local environment: one test failed and another waited over 60 seconds for its configured service. It was stopped; CI should verify it in the intended environment.
  • git diff --check passed. Repository-wide rustfmt check reports existing formatting differences outside this patch; the changed regions match rustfmt output.

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

Not applicable.

Additional Notes

No CLI option name or documented invocation changes. The regression verifies the argv handed to Clap; actual acceptance of leading-hyphen values depends on each Clap option's parser settings.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

[Cleanup]: CLI compact preprocessor rewrites argv without context

1 participant