Skip to content

cli/parser: don't restrict global options to subcommands - #22408

Merged
MikeMcQuaid merged 1 commit into
Homebrew:mainfrom
KEY60228:fix/global-option-validation
May 24, 2026
Merged

cli/parser: don't restrict global options to subcommands#22408
MikeMcQuaid merged 1 commit into
Homebrew:mainfrom
KEY60228:fix/global-option-validation

Conversation

@KEY60228

@KEY60228 KEY60228 commented May 24, 2026

Copy link
Copy Markdown
Contributor

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them? Performance claims (e.g. "this is faster") must include Hyperfine benchmarks.
  • Have you written new tests (excluding integration tests) for your changes? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) with your changes locally?

  • AI was used to generate or assist with generating this PR.

This PR was prepared with assistance from Claude Code (model: claude-opus-4-7). I reviewed all generated code and tests manually, reproduced the bug locally, and ran brew lgtm successfully before pushing. This is my only AI-assisted PR currently open.


Problem

Since 5.1.12, brew bundle cleanup -v is rejected:

$ brew bundle cleanup ---global --verbose --force
Error: Invalid usage: The `cleanup` subcommand does not accept the `-v` switch.

even though brew bundle cleanup -h still lists -v, --verbose among its options — help output and runtime disagree.

The same rejection happens on every bundle subcommand that doesn't re-declare --verbose itself: cleanup, dump, list, add, remove, edit, exec, sh, env. Only install and check still accept -v, because they declare it explicitly.

Root cause

Library/Homebrew/cli/parser.rb auto-registers -d/-q/-v/-h as global options for every command. #22280 moved switch -v, --verbose declarations into the install and check subcommand blocks so they could carry subcommand-specific descriptions.

record_option_metadata treats any option declared inside a subcommand block as constrained to that subcommand. That's correct for genuine subcommand-only options (e.g. --jobs on install), but re-declaring a global option inside a subcommand block silently constrains the global option too. So --verbose got pinned to ["install", "check"], while help generation kept showing it as a root option everywhere — producing the mismatch above.

Same family of regression as #22325 (--zap constraint scope), but at the global-option layer rather than per-option.

Fix

Skip subcommand-constraint recording for global options in record_option_metadata:

# Global options are accepted everywhere, so a subcommand block
# re-declaring one (e.g. for a custom description) must not constrain it.
next if global_option?(name)

global_option? checks against self.class.global_options. Subcommand-specific options keep their existing constraint behaviour — brew bundle exec --jobs=1 true still correctly errors. The install/check re-declarations of --verbose keep providing their custom descriptions; only the unintended constraint goes away.

Alternative considered

Adding switch -v, --verbose to each of the ~10 affected bundle subcommands would also fix the immediate symptom, but:

  • duplicates the declaration across many files;
  • doesn't future-proof --debug/--quiet against the same mistake;
  • doesn't help other commands that use the subcommand parser.

A single guard in the parser keeps the fix minimal and addresses the root cause.

Scope

  • --verbose is the only currently-broken global option; --debug and --quiet aren't re-declared anywhere today, but the fix protects all four.
  • Other commands using the subcommand parser (services, analytics, developer, completions, prof) don't re-declare global options inside subcommand blocks, so no behaviour change there.
  • brew bundle upgrade is unaffected here because Make bundle upgrade an alias #22348 already turned it into an alias of install --upgrade.

Tests

  • test/cli/parser_spec.rb: parser-level regression — a global option re-declared in one subcommand block is still accepted on other subcommands.
  • test/cmd/bundle_spec.rb: bundle-level regression — cleanup, dump, list accept --verbose/-v after parsing.

Both verified with a red/green cycle (failing on main, passing with the fix).

Manual verification

$ brew bundle cleanup -v --file=Brewfile    # works (was rejected before this PR)
$ brew bundle cleanup -h                    # still lists -v, --verbose
$ brew bundle exec --jobs=1 /usr/bin/true   # still correctly rejected

Copilot AI review requested due to automatic review settings May 24, 2026 15:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes a CLI parsing regression where global options (e.g., -v/--verbose) become incorrectly restricted to only the subcommands that re-declare them, causing valid invocations like brew bundle cleanup -v to error despite help output listing the option.

Changes:

  • Update CLI::Parser#record_option_metadata to avoid recording subcommand constraints for globally-registered options when they are re-declared inside a subcommand block.
  • Add a parser-level regression test covering global option re-declaration inside a subcommand.
  • Add a brew bundle regression test verifying --verbose/-v is accepted on subcommands that do not re-declare it.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
Library/Homebrew/cli/parser.rb Prevents global options re-declared in a subcommand from being constrained to that subcommand.
Library/Homebrew/test/cli/parser_spec.rb Adds coverage to ensure global options remain accepted across all subcommands after re-declaration.
Library/Homebrew/test/cmd/bundle_spec.rb Verifies brew bundle subcommands like cleanup/dump/list accept --verbose/-v.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MikeMcQuaid MikeMcQuaid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Makes sense, thanks!

@MikeMcQuaid
MikeMcQuaid enabled auto-merge May 24, 2026 15:54
@MikeMcQuaid
MikeMcQuaid added this pull request to the merge queue May 24, 2026
Merged via the queue into Homebrew:main with commit 651a932 May 24, 2026
42 of 43 checks passed
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.

3 participants