Uh oh!
There was an error while loading. Please reload this page.
fix(config): keep '=' inside the value of config set - #411
Conversation
keyValue.split("=") destructures only the first two parts, so a value
containing "=" was silently truncated at the first one. base64 padding
and URLs with a query string are the common cases:
config set apiKey=YWJjZGVm== stored YWJjZGVm
config set rpcUrl=https://x/v1?key=a&m=fast stored https://x/v1?key
Splits on the first separator instead. An empty value and a missing
separator behave as before.Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Problem
config setdestructures the split, so it only ever keeps the first two parts:A value containing
=is silently truncated at the first one. Two everyday cases:There is no error and no warning. The command reports
Configuration successfully updated, and the value only turns out to be wrong later, when whatever reads it fails for a reason that does not point back here.Change
Splits on the first separator instead:
The two existing behaviours are unchanged:
config set invalidFormat) still fails withInvalid format. Use 'key=value'.config set defaultNetwork=) still stores""Tests
Three cases added to
tests/actions/getSetReset.test.ts: base64 padding, a query string, and the empty value. The first two fail onv0.40-devand pass with the change; the empty-value and invalid-format tests pass either way, which is what pins the unchanged behaviour.Verification
Run against
v0.40-devate822a9e:And with the fix reverted, to confirm the new tests actually catch it:
Branch
Targeting
v0.40-devas the active integration branch perdocs/BRANCHING.md. Happy to retarget at a stable branch if you would rather ship it directly.One thing I left alone
reset()has adelete config[key]that operates on a local copy and is never written back — the removal actually happens becausewriteConfig(key, undefined)makesJSON.stringifydrop the key. It works, so I did not touch it here, butremoveConfig(key)already exists and says it directly. Happy to send that separately if it is worth having.