fix(FR-2355): handle duplicate TOML keys in config.toml parsing#6107
Open
ironAiken2 wants to merge 1 commit intomainfrom
Open
fix(FR-2355): handle duplicate TOML keys in config.toml parsing#6107ironAiken2 wants to merge 1 commit intomainfrom
ironAiken2 wants to merge 1 commit intomainfrom
Conversation
Contributor
Author
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has required the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Contributor
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
|---|---|---|---|
| 🔴 | Statements | 8.26% (+0.01% 🔼) |
1406/17031 |
| 🔴 | Branches | 7.43% (-0.01% 🔻) |
883/11884 |
| 🔴 | Functions | 5.41% (-0% 🔻) |
258/4767 |
| 🔴 | Lines | 7.95% (+0.01% 🔼) |
1319/16589 |
Show files with reduced coverage 🔻
St.❔ |
File | Statements | Branches | Functions | Lines |
|---|---|---|---|---|---|
| 🔴 | ... / useWebUIConfig.ts |
21.93% (+0.98% 🔼) |
12.73% (-0.48% 🔻) |
11.76% (-0.74% 🔻) |
22.12% (+0.97% 🔼) |
Test suite run success
775 tests passing in 36 suites.
Report generated by 🧪jest coverage report action from 26193ea
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens config.toml loading in the React WebUI so duplicated keys generated by the webserver don’t cause TOML parsing to fail silently and revert all config options to defaults.
Changes:
- Add a TOML text preprocessor intended to remove duplicate keys within a section before parsing.
- Change
fetchAndParseConfig()to return{ config, error }so callers can distinguish success vs failure paths. - Replace
console.warnwithuseBAILoggerlogging on config load failures and update call sites/tests accordingly.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| react/src/hooks/useWebUIConfig.ts | Adds deduplication + new fetch/parse result shape, and switches failure logging to useBAILogger. |
| react/src/hooks/useWebUIConfig.test.ts | Updates tests for new return type and adds a duplicate-key handling test. |
| react/src/helper/loginSessionAuth.ts | Updates call site to new { config } return shape. |
| react/src/components/EduAppLauncher.tsx | Updates call site to new { config } return shape. |
ironAiken2
force-pushed
the
03-23-fix_fr-2355_handle_duplicate_toml_keys_in_config_parsing
branch
from
02f0efd to
1ce5c97
Compare
yomybaby
requested changes
Mar 23, 2026
ironAiken2
force-pushed
the
03-23-fix_fr-2355_handle_duplicate_toml_keys_in_config_parsing
branch
from
1ce5c97 to
21872b9
Compare
ironAiken2
force-pushed
the
03-23-fix_fr-2355_handle_duplicate_toml_keys_in_config_parsing
branch
from
21872b9 to
dbb22ff
Compare
The Backend.AI webserver generates config.toml with duplicate keys
(e.g. two `debug = true` lines), which causes smol-toml to throw.
The error was silently swallowed, falling back to defaults and
making all config options appear disabled.
- Add deduplicateTomlKeys() preprocessing to remove duplicate keys
before TOML parsing (keeps last occurrence)
- Change fetchAndParseConfig return type to { config, error } to
distinguish fetch failure from parse failure
- Add useBAILogger-based error logging for parse failures
- Update all call sites for new return type
- Add test for duplicate key handling
ironAiken2
force-pushed
the
03-23-fix_fr-2355_handle_duplicate_toml_keys_in_config_parsing
branch
from
dbb22ff to
26193ea
Compare
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.
Resolves #6091 (FR-2355)
Summary
smol-tomlthrows on duplicate keys in TOML, but the previous code silently swallowed the error, causing all config options to fall back to defaults without any indication to the userfetchAndParseConfigreturn type fromRawTomlConfig | nullto{ config, error }to distinguish fetch failures (network error, non-200, SPA fallback) from parse failures (invalid TOML)configParseErrorStateJotai atom to store parse errors so other hooks can access them<pre>code block, remediation instructions, and early return (skip remaining config checks sincerawConfigisnull)useBAILogger(replacesconsole.warn)loginSessionAuth.ts,EduAppLauncher.tsx) for the new return typeConfigParseError,ConfigParseErrorDesc,ConfigParseErrorFix) across all 21 languagesTest plan
config.toml(e.g.,debug = falseanddebug = trueunder[general]), open the diagnostics page, and verify a critical alert is shown with the parse error message in a code blockconfig.tomlis validconfig.tomlis missing (404)pnpm run test— all 35 useWebUIConfig tests pass🤖 Generated with Claude Code