Repository navigation
Conversation
`osb config set` stores anything that parses as an int or a float as a TOML number. For string keys that changed the value: an API key `007123` was written as `7123`, `1e10` as `10000000000.0` and `NaN` as `nan`, and the resolver then used the changed text, so authentication silently used the wrong key. Use a number only when it writes back as the same text, and store the value as a string otherwise. `45`, `1.5` and `true` keep their native types as before.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
github-actions
Bot
added
the
size/S
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e5b243d25
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| number = parse(raw) | ||
| except ValueError: | ||
| continue | ||
| return number if str(number) == raw else raw |
There was a problem hiding this comment.
When this generic helper is used for a typed setting, numeric spellings that Python normalizes are now quoted. For example, osb config set connection.request_timeout 1e2 used to write a float that resolve_config coerced to 100, but now writes "1e2"; _int_or_none cannot parse that string and silently falls back to 30. Likewise, connection.use_server_proxy 1.00 now resolves to False instead of True, despite numeric scalar coercion being explicitly covered in cli/tests/test_config.py:162-173. Preserve raw text only for string-valued keys, or continue accepting these quoted numeric forms in the typed coercers.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed, thanks. Fixed in 4f1b78a by going the first way: _parse_toml_value is back to its original inference, and config set stores the value verbatim only for the keys resolve_config reads as text. Those keys are listed once, as STRING_KEYS next to resolve_config: api_key, domain, protocol, defaults.image and defaults.timeout. request_timeout 1e2 resolves to 100 again and use_server_proxy 1.00 to True, and both are covered by test_set_still_infers_numbers_for_typed_keys.
…rest The round-trip rule also quoted noncanonical numbers for typed keys: `config set connection.request_timeout 1e2` was written as "1e2", which the resolver cannot parse, so it fell back to 30, and `connection.use_server_proxy 1.00` resolved to False. Store values verbatim only for the keys resolve_config reads as text (api_key, domain, protocol, defaults.image, defaults.timeout), and keep the original type inference for every other key.
acodercat
changed the title
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
acodercat
changed the title
osb config setinfers the TOML type of every value:true/falsebecome booleans, and anything thatint()orfloat()accepts becomes a number. The resolver turns numbers back into strings for string keys such asconnection.api_key, as its docstring describes for all-digit keys. But by then the text has already changed:osb config set connection.api_key 007123writesapi_key = 7123, and the CLI authenticates with7123.1e10is written as10000000000.0,NaNasnan,1_000as1000, and1.50as1.5.So a key or other string value that happens to look like a number is silently replaced with a different one.
Summary
opensandbox_cli.configlists the keysresolve_configreads as text inSTRING_KEYS:connection.api_key,connection.domain,connection.protocol,defaults.imageanddefaults.timeout.config setstores values for those keys verbatim, as strings.request_timeout,use_server_proxyandcolorare still written as numbers or booleans, so spellings like1e2or1.00still resolve as before.Testing
Not run (explain why)
Unit tests
Integration tests
e2e / manual verification
New
test_set_keeps_text_a_number_would_changefor007123,1e10,NaN,1_000and1.50: the stored and the resolvedapi_keymust equal the typed text. Every case fails on current main.New
test_set_still_infers_numbers_for_typed_keys:request_timeout 1e2must resolve to 100 anduse_server_proxy 1.00toTrue. Storing every value as text fails it.The existing
test_set_infers_scalar_typesstill passes. The full CLI suite passes (337 passed), andruffandpyrightare clean.Breaking Changes
Checklist