Skip to content

fix: propagate validator errors from empty bool sources - #2452

Open
sergioperezcheco wants to merge 2 commits into
urfave:mainfrom
sergioperezcheco:fix/empty-bool-source-validator-20261003
Open

sergioperezcheco wants to merge 2 commits into
urfave:mainfrom
sergioperezcheco:fix/empty-bool-source-validator-20261003

Conversation

@sergioperezcheco

Copy link
Copy Markdown

What type of PR is this?

  • bug

What this PR does / why we need it:

An empty environment variable or file is already interpreted as false for a BoolFlag, but PostParse discards the error returned by Set in that branch. A custom validator can reject the value while Command.Run still succeeds and executes the flag and command actions. Explicit false from the same source correctly returns an error.

Normalize the empty bool value to false before the existing error-handling path. This preserves empty-source and fallback semantics while making validator failures consistent. The public Command.Run regression covers environment and file sources, explicit values, command-line overrides, and action execution.

Which issue(s) this PR fixes:

No existing issue found. This is distinct from #2451, which skips empty sources for non-string/non-bool flags; it does not change the existing empty-bool-to-false behavior.

Testing

On Go 1.26.4 / macOS arm64, the new regression fails on the original production source for both empty-source cases and passes with the fix. go test -race -count=1 ./..., its urfave_cli_no_template variant, make test with both tag configurations, make vet, golangci-lint 2.13.1, formatting, make generate, make v3diff, and make check-binary-size passed. Generated API documentation is unchanged. Other OS/Go versions and documentation examples were not run locally.

Implemented and tested with Hermes Agent using GPT-6.1-Sol.

Release Notes

Return BoolFlag validator errors for empty environment-variable and file sources instead of continuing command execution.

Co-Authored-By: OpenAI GPT-6.1-Sol <noreply@openai.com>
Signed-off-by: sergioperezcheco <checo520@outlook.com>
@sergioperezcheco
sergioperezcheco requested a review from a team as a code owner October 3, 2026 04:54
@greptile-apps

greptile-apps Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Changes how boolean flags handle empty values from sources.

The PR appears safe to merge.

Summary

The PR normalizes empty boolean environment-variable and file values to false before the existing error-handling path, so validator errors stop command execution. It adds regression coverage for empty and explicit source values, command-line overrides, and action execution.

Reviews (2) · Last reviewed commit: "fix: propagate validator errors from emp..."

@coilysiren

Copy link
Copy Markdown
Member

Please forgive the WIP review bot 🙏🏼

@coilysiren coilysiren 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.

Can you write a version of these tests that had a block of input variants at the top? As opposed to composing said variants while you walk down the for loop chain

Co-Authored-By: OpenAI GPT-6.1-Sol <noreply@openai.com>
Signed-off-by: sergioperezcheco <checo520@outlook.com>
@sergioperezcheco

Copy link
Copy Markdown
Author

Updated the regression to a single table of 12 input variants at the top, with explicit expected errors, instead of constructing cases in nested loops. The environment/file, empty/false/true, and command-line override cases are preserved. Both full race-test configurations, vet, and lint pass locally.

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

None yet

Development

Successfully merging this pull request may close these issues.

2 participants