Repository navigation
feat(#10436): serve WebUI settings through the domain app config - #10438
Open
ironAiken2 wants to merge 36 commits into
Open
ironAiken2 wants to merge 36 commits into
ironAiken2 wants to merge 36 commits into
Conversation
github-actions
Bot
added
area:ux
Contributor
Author
|
Dev server on box |
Contributor
Contributor
Author
|
📍 Walkthrough · 4 stops · 45f05ba — Open the walkthrough
|
ironAiken2
force-pushed
the
feat/gh-10436-webui-settings-app-config
branch
from
October 27, 2026 07:55
c3f127a to
d400d89
Compare
ironAiken2
marked this pull request as ready for review
October 29, 2026 07:35
This was referenced Oct 29, 2026
ironAiken2
added a commit
that referenced
this pull request
Oct 30, 2026
…stem Settings page One sequential spec exercises #10438 end to end against the live cluster: the page (groups, the 56-row inventory, file-value defaults, changed marker and reset per row, toolbar reset, Apply with its confirmation and reload, the public-scope write, the leave guard, the domain-admin view), precedence (a domain document overrides every domain key after sign-in, wrong-typed and absent keys fall back to the file, the other scope's keys are ignored, the public document overrides the login page before sign-in), the visible effects (hideAgents, menuBlocklist, the CPU cap in the launcher, the Downloads menu, Models on the Data page, loginAttemptLimit, plugin paths) and the cleanup contract. `webui-settings-util.ts` mirrors WEBUI_SETTING_DEFINITIONS, drives the `settings` sub-key of both scopes through scopedUpsertAppConfigFragments with snapshot/restore, and wraps the page. test-util gains fetchConfigToml (extracted from modifyConfigToml) and getSideNavItem (moved from page-access-control.spec.ts). The domain admin's Apply is not covered: on the dev cluster that account lacks app_config_fragment permissions at DOMAIN scope, so the server refuses the write.
ironAiken2
added a commit
that referenced
this pull request
Oct 30, 2026
#10438 moved the page to /admin/system-settings, dropped loginAttemptLimit, loginBlockTime and eduAppNamePrefix from the definition table (53 rows: 10 public, 43 domain), moved storageWarningThreshold into the Diagnostics group and showNonInstalledImages into the Session launcher group, and made the toolbar Reset save and reload as soon as it is confirmed. - The row table mirrors the new definitions; the domain-admin test derives the hidden groups from the rows (Sign-in and Plugins, no longer Diagnostics, which now holds a domain key). - The toolbar Reset test waits for the reload. - The loginAttemptLimit effect test is removed: the key is config.toml only. - The SFTP step closes its error dialog by its footer button so the file browser step starts from the explorer.
agatha197
requested changes
Oct 30, 2026
agatha197
left a comment
agatha197
left a comment
Contributor
There was a problem hiding this comment.
Verified the code-review findings against the head; these five block merge.
ironAiken2
force-pushed
the
feat/gh-10436-webui-settings-app-config
branch
from
October 30, 2026 10:10
c3d8ba8 to
f069db9
Compare
Contributor
PR Analysis Report📖 Docs PreviewView the docs preview for this PR GitHub Pages may take up to a minute to hydrate after the deploy. English (
日本語 (
한국어 (
ภาษาไทย (
|
…ice sign-in The switch only affects the /interactive-login card an external service opens to sign in with a Backend.AI account, not SSO into Backend.AI itself; the title now says so in every locale.
The SAML / OpenID choice is a MultiSelector instead of a checkbox list, matching how other multi-value settings are picked.
The row is titled Allow SSO sign-in, and its description says the buttons only work when the manager's SSO plugin is installed, since the WebUI cannot tell on its own.
…eckbox rows SettingItem rendered a custom row's description bare, so it inherited the 16px body size while checkbox and select rows wrapped theirs in Text at 14px. The custom branch now wraps it the same way; the user settings page's custom rows follow.
… with auto sign-out The realm name row is titled after what it does, the SSO OpenID button label, in every locale; autoLogout moves to the top of the Sign-in group.
SettingList's Reset already asks for confirmation, so confirming it now writes both scopes with an empty document and reloads instead of only clearing the draft. A per-row reset still edits the draft and waits for Apply.
Both plugin rows show the loader's fixed root in a disabled box with the relative path beside it, instead of folder and file rows that hid where the files are served from. The login row takes one path and rejects spaces and commas; the page row takes a comma-separated list. An invalid entry shows the error state and is not committed. PluginPathListInput and its three strings go away.
…eparator between the boxes The disabled box reads /dist/plugins and a / sits between it and the path box.
The route path and menu key follow the page's name; the component and test ids keep their file names.
…oml parse pure - loadConfigFromWebServer resolved the public-config target from the merged config, whose apiEndpoint is this page's own (the webserver's was stripped), so a fresh Electron install never read the public settings and a server switch read the old server's slice. It now targets apiEndpoint. - refreshConfigFromToml no longer writes globalThis.backendaiwebui.debug; rendering System Settings undid the public debug override until reload. The reload path in useUpdateRawConfig syncs it explicitly instead.
…a typed word - menuBlocklist / menuInactivelist ignore system-settings, so an admin cannot hide the page that edits both lists. - The toolbar Reset deletes the saved settings documents, so it opens BAIDeleteConfirmModal with requireConfirmInput (the localized Reset word) instead of SettingList's generic reset prompt; new strings webuiSettings.ResetAllSettings / ResetAllSettingsDesc in every locale.
Where the WebUI settings live (the flat `settings` key per scope), which keys are public, how documents merge over config.toml, which keys stay in the file, and who may edit them.
…pp config Auto logout had two homes: the user's own toggle in user settings (localStorage `backendaiwebui.settings.user.auto_logout`) and the deployment default, which this PR already serves through the public app config `settings` with config.toml as the fallback. The user toggle overrode the admin's value, so an admin change never reached anyone who had once touched the toggle. The user-settings item, its `auto_logout` setting type, resolveAutoLogout and the two `userSettings.*AutoLogout` i18n keys are removed; autoLogoutState now takes `autoLogout` straight from the merged login config. A value left in localStorage is no longer read. The manual's "Auto logout" section under user settings is removed in en/ko/ja/th, and the search-palette tests that used the setting as their sample item now use Automatic update check / My keypair information.
`debug` already lives on loginConfigState (config.toml, overridden by the public app config `settings`), but its readers still took the Lit-era copy at `globalThis.backendaiwebui.debug`, so syncDebugFlag rewrote that copy on every path that settled the config. The five readers now read the atom: BAIErrorBoundary, AppLauncherModal, usePainKiller and useBackendAIAppLauncher through useLoginConfig(), and the i18n key-marker postProcessor, which runs outside React, through jotaiStore.get(loginConfigState). The global and syncDebugFlag are removed. With nothing left to throw on a missing global, `relieve` no longer needs the try/catch guards ImportImageModal and useScanImage carried for FR-3953; the regression test keeps asserting that the import button is not stranded. The uncalled useUpdateRawConfig is removed.
The new maxATOMPLUSDevicesPerContainer / maxHyperaccelLPUDevicesPerContainer parsing defaulted both to 8, so they always reached backendaiclient._config and useResourceLimitAndRemaining no longer fell back to maxCUDADevicesPerContainer for those devices: a deployment with a CUDA cap of 16 and no ATOM+/LPU key was silently capped at 8. Both are now unset when the key is absent, a placeholder or not a number, and _config only receives them when set, which is main's behaviour. The keys stay in the defaults (as undefined) so the settings table still maps onto them.
The design diagram and the Decision still described syncDebugFlag and the globalThis.backendaiwebui.debug copy that 51cfd67 removed; the merged value now lives only in loginConfigState. The Consequences line also claimed tsc rejects a definition that does not match LoginConfigState, but def() casts the key, so the guard is webuiSettings.test.ts.
Apply writes the domain document, then the public one, with no rollback. A failure only showed the error toast, so after a domain write succeeded and the public one failed, the admin could not tell that part of the change was already live. The failure now opens an error dialog that lists the settings that were not saved (the changed domain and public rows when the first write fails, only the public rows when the second does), says the rest was saved and applies after a reload, and asks to apply again. Four webuiSettings keys added in every locale.
loadConfigFromWebServer merged the server's config.toml onto rawConfigState, which already held the previous endpoint's server config, so signing in to a second endpoint kept every key the first server set and the second did not. It now merges onto the bundled config.toml (fetchAppConfigOnce), and the public settings slice is applied to that result as before. It also stops setting loginPluginState from the joined server. The Electron login screen is drawn before any endpoint is known (first launch, and right after a logout, which opens the panel before this merge), so a server's login plugin could only ever apply on some launches; the bundled plugin.login stays in charge, as on main.
The plugin diagnostic validated plugin.page with a single-segment name pattern, so a folder path the loader accepts (vendor/dashboard) was reported invalid, and it read the file even when the public settings document overrode the value. It also expected plugin.login to be a name.js filename, which the loader (resolvePluginScriptUrl, and main's sanitiser before it) refuses, since it appends .js itself. checkPluginConfiguration now takes the effective pluginPages and loginPlugin from loginConfigState and flags whatever isValidPluginPath rejects, page entries and the login plugin alike, under the existing InvalidPluginNames result. The plugin.sidebar check goes: nothing reads that key. The three InvalidPluginFilename keys are removed from every locale.
…lugin error The free-text and list rows, and both PluginPathInput boxes, rendered UncontrolledInput without a label, so every one took the catalog fallback name "Select"; PluginPathInput's aria-label never reached the input, since UncontrolledInput takes its accessible name from label only. Each now gets its row title as a hidden label and a webui-setting-<key>-input test id. PluginPathInput kept its error state until the next commit, so a row reset (or any other change to the value) left the red border on a valid value. The error is now tied to the value it was raised against and clears when that value changes.
connectViaGQL read domainConfig.settings with a hand-written query string through client.query, after loginSessionAuthMyUserQuery had returned: relay-compiler never checked it against data/schema.graphql, so a schema change would only show as settings silently falling back, and every sign-in paid one more sequential round trip. myAppConfigs(configNames: ["domainConfig"]) now rides in loginSessionAuthMyUserQuery with @catch(to: NULL), so a failed read still leaves the base config in place instead of failing the sign-in. fetchDomainWebUISettings becomes readDomainWebUISettings, which picks the settings sub-key out of the returned rows.
Domain admins are out of scope for this work. The menu item now shows only to superadmins (like Branding and Information), the route requires `access: 'superadmin'`, and the page renders one list that reads and writes both the public and the domain document. WebUISettingList loses its canEditPublic switch and the domain-admin variant that hid the public rows. ADR 0017's "Who sees what" / "Who may write" lines follow.
Restores the four user_settings.md files to the branch point: removing the Auto logout section belongs to the automated docs update, not to this PR.
ironAiken2
force-pushed
the
feat/gh-10436-webui-settings-app-config
branch
from
November 3, 2026 06:40
867821e to
d8ec017
Compare
4 tasks
ironAiken2
added a commit
that referenced
this pull request
Nov 3, 2026
…stem Settings page One sequential spec exercises #10438 end to end against the live cluster: the page (groups, the 56-row inventory, file-value defaults, changed marker and reset per row, toolbar reset, Apply with its confirmation and reload, the public-scope write, the leave guard, the domain-admin view), precedence (a domain document overrides every domain key after sign-in, wrong-typed and absent keys fall back to the file, the other scope's keys are ignored, the public document overrides the login page before sign-in), the visible effects (hideAgents, menuBlocklist, the CPU cap in the launcher, the Downloads menu, Models on the Data page, loginAttemptLimit, plugin paths) and the cleanup contract. `webui-settings-util.ts` mirrors WEBUI_SETTING_DEFINITIONS, drives the `settings` sub-key of both scopes through scopedUpsertAppConfigFragments with snapshot/restore, and wraps the page. test-util gains fetchConfigToml (extracted from modifyConfigToml) and getSideNavItem (moved from page-access-control.spec.ts). The domain admin's Apply is not covered: on the dev cluster that account lacks app_config_fragment permissions at DOMAIN scope, so the server refuses the write.
ironAiken2
added a commit
that referenced
this pull request
Nov 3, 2026
loginBlockTime and eduAppNamePrefix from the definition table (53 rows: 10 public, 43 domain), moved storageWarningThreshold into the Diagnostics group and showNonInstalledImages into the Session launcher group, and made the toolbar Reset save and reload as soon as it is confirmed. - The row table mirrors the new definitions; the domain-admin test derives the hidden groups from the rows (Sign-in and Plugins, no longer Diagnostics, which now holds a domain key). - The toolbar Reset test waits for the reload. - The loginAttemptLimit effect test is removed: the key is config.toml only. - The SFTP step closes its error dialog by its footer button so the file browser step starts from the explorer.
ironAiken2
added a commit
that referenced
this pull request
Nov 3, 2026
#10438 now routes System Settings for superadmins only, keeps it out of menuBlocklist / menuInactivelist, confirms the toolbar Reset with a typed word in a delete-confirm modal, reads `debug` from loginConfigState only (no more `globalThis.backendaiwebui.debug`) and names each row's input with a `webui-setting-<key>-input` test id. main meanwhile scoped a team project's Data page to that project's folders. - The domain-admin test asserts the Unauthorized Access screen and no menu item instead of a reduced page. - The toolbar Reset test saves values first, types the confirmation word, and checks both documents are deleted. - `debug` is asserted through the i18n key markers it turns on. - The blocklist and inactivelist tests include `system-settings` and check it stays visible and enabled. - The row inventory ignores the `-input` test ids. - The folder explorer test creates a folder in the current project.
Contributor
Author
|
Heads-up on ADR numbering: #10813 (consolidate |
ironAiken2
added a commit
that referenced
this pull request
Nov 3, 2026
…-settings #10438 now serves System Settings at /admin/system-settings with the menu key system-settings, so the Configurations redirect, its help anchor, the search-index tab override and the e2e expectations point there.
This branch has not been deployed
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 #10436
Summary
The layout is recorded as ADR 0018 (
docs/adr/0018-webui-settings-in-the-app-config-settings-key.md).Settings that today come only from the static
config.tomlcan now be changed per domain at runtime through the server-side app config, with the file as the fallback. The layout and precedence follow the decision recorded on the issue:settingsdocument per scope.domainConfig.settingsoverrides the file for 43 keys after login;publicConfigByDomain.<domain>.settingsdoes so for the ten keys read before sign-in: the six login-page keys,autoLogout,pluginLogin/pluginPage(plugin.login/plugin.page, edited as folder / file rows; the plugin scripts are imported at bootstrap, soPluginLoadernow reads the effective login config instead of the raw file), anddebug, which the diagnostics surfaces (i18n key markers, error details) read at call time, so a superadmin can turn them on per domain without a webserver restart. Both now have one home each: the per-user Auto logout toggle in user settings is removed (with its i18n keys; the manual is left to the automated docs PR), and the Lit-eraglobalThis.backendaiwebui.debugcopy is gone — its five readers takedebugfromloginConfigState. Key names are theconfig.tomlnames (menu.blocklist→menuBlocklist,menu.inactivelist→menuInactivelist), comma lists become arrays.config.tomlis the fallback per key, astheme.jsonis forappearance: a key no document carries keeps the file value, and nothing is seeded. An untouched deployment behaves exactly as before.publicConfigByDomainread is one shared, cached request per page load (react/src/helper/publicAppConfig.ts) that serves the appearance document and the settings slice; when the app joins another webserver it looks the slice up under that server'sapiEndpoint. After loginconnectViaGQLreadsdomainConfigonce through the plain client, merges it, and stores the effective config beforeapplyConfigToClient, so the ~40_configconsumers need no change./admin/system-settings,access: superadmin; domain admins are out of scope) edits the document with the sharedSettingList: each row's default is the file value, the "changed" dot and the reset icon work against it, only rows that differ are saved, and Apply confirms, writes both scopes and reloads the page, as Branding does. The toolbar's Reset deletes the saved values, so it opens aBAIDeleteConfirmModalthat asks for the Reset word to be typed, then applies at once; a per-row reset only edits the draft.menuBlocklist/menuInactivelistignoresystem-settings, so the page cannot be hidden by the lists it edits.Dropped while reviewing the page row by row:
eduAppNamePrefix(the edu app launcher no longer strips a prefix; the key leaves the parser andconfig.toml.sample, the webserver template follows in BA-8735) and theloginAttemptLimit/loginBlockTimesettings (the webserver limiter covers them; removing the browser-side limiter itself is on #10664). The SSO vendors row is a SAML / OpenID checkbox list, number rows show their unit as an input suffix, and the groups read Sign-in, Session, App launcher, Images, Data, Menu, Downloads, Account, Diagnostics, Plugins.Carried along because the settings table exposed them: the ATOM+ and Hyperaccel LPU caps were never parsed (the webserver renders ATOM+ as
maxATOMPlUS…, both spellings are read now);storageWarningThresholdflows throughLoginConfigStateso the diagnostics page sees an override.Not in this PR, left as the issue lists:
userConfig.settings(personal overrides, the sibling issue under #10207),connectionMode(a webserver forces SESSION wheneverapiEndpointis set, and the Electron app reads the user's stored choice first, so a document value would change nothing), dropping the dead keys fromconfig.toml.sample(#10664) and from the webserver template (BA-8735).Config: general.apiDomainName = "default"
(The webserver sets
apiDomainNameitself; without it the pre-login slice is not fetched and the login-page keys keep the file values.)Verification
VERIFY_TESTS=1 bash scripts/verify.sh→=== ALL PASS ===react/src/helper/webuiSettings.test.ts(definition table, scope/type-guarded merge, differs-from-file document),react/src/helper/publicAppConfig.test.ts(target resolution, one request per page load, failure paths).customThemeConfig.test.tsandloginSessionAuth.test.tsupdated for the shared fetch and the domain read.Review checklist
Walkthrough
Not shown in the walkthrough:
publicConfigByDomain.<domain>.settingsis applied to the login config at bootstrap anddomainConfig.settingsafter login (useWebUIConfig.ts,loginSessionAuth.ts,webuiSettings.ts); only its effect is visible, through the values the page and the app show after Apply.ssoRealmNamesaved in the Login page group shows as the SSO button label before sign-in; this server has no SSO vendor configured, so the label has nothing to attach to.eduAppNamePrefixnow reachesbackendaiclient._config;maxATOMPLUSDevicesPerContainer/maxHyperaccelLPUDevicesPerContainerare parsed (both ATOM+ spellings);storageWarningThresholdflows throughLoginConfigState.webuiSettings.test.ts,publicAppConfig.test.ts, and the updatedcustomThemeConfig.test.ts/loginSessionAuth.test.ts.