fix: ensure last-invoked option wins for shared dest names#3261
Closed
Dhi13man wants to merge 1 commit intopallets:mainfrom
Closed
fix: ensure last-invoked option wins for shared dest names#3261Dhi13man wants to merge 1 commit intopallets:mainfrom
Dhi13man wants to merge 1 commit intopallets:mainfrom
Conversation
When multiple options target the same parameter name (dest), the parser stores only the last-written value. Previously, processing order caused earlier-invoked options to read the wrong value from opts, and the guard in handle_parse_result prevented the correct (last-invoked) option from overwriting ctx.params. Filter param_order to keep only the last invocation per dest name so overshadowed options process as uninvoked, falling through to defaults. The existing guard then allows the winning option's value through. Closes #2786 Co-Authored-By: Dhiman's Agentic Suite <dhiman.seal@hotmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--custom bar --fetchincorrectly returned the sentinel value ($_fetch) instead of the callback result (foo) because the parser's last-written value was read by the wrong parameter during processingparam_orderto keep only the last invocation per dest name, so overshadowed options process as uninvoked and the existing guard inhandle_parse_resultlets the winning value throughCloses #2786
Test plan
test_dual_option_callback_last_winscovering all 5 cases:--fetchalone → callback result--custom baralone → explicit value--custom bar --fetch→ last option wins (callback result)--fetch --custom bar→ last option wins (explicit value)Nonedefault