apache / apache/rocketmq-dashboard
[Studio][Bug] Saving any general setting rolls back AI/LLM configuration saved from the AI tab
- Dominant language
- Java
- Stars
- 1.4k
- Forks
- 683
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 58
Description
## Problem
`web/src/pages/settings/GeneralSettingsTab.tsx` fetches the general settings once on mount and keeps that snapshot in state. Every save path on the tab builds the request payload from that snapshot (`buildPayload`, lines 44-59) and posts it to `POST /api/settings/general/save`, including the `llmProvider`, `model` and `baseUrl` fields the tab never renders or edits.
The AI assistant tab (`AiAssistantTab`) saves the same persisted record through `POST /llm/config` (`LlmConfigService.saveConfig` writes `llmProvider`/`model`/`baseUrl` into the same `GeneralSettingsVO` row). Because the antd Tabs keep the visited panes mounted (`web/src/pages/settings/index.tsx` has no `destroyOnHidden`), the general tab never refetches after that save.
## Evidence
1. Open `/settings` (general tab mounts, snapshot: `llmProvider=openai`).
2. Switch to the AI tab, change provider/model/base URL, save — `POST /llm/config` persists the new values.
3. Switch back to the general tab (still mounted, stale snapshot), change the session timeout, save — `POST /api/settings/general/save` carries the stale `llmProvider`/`model`/`baseUrl`.
4. `SettingsService.saveGeneralSettings` replaces the row; its empty-value backfill covers `llmEngine`/`deploymentName`/`apiVersion`/`awsRegion`/`maxTokens`/`temperature`/`apiKey`/`dingtalkSigningSecret` but **not** `llmProvider`/`model`/`baseUrl` (`SettingsService.java:128-166`, `GeneralSettingsUpdateDTO.java:49-81` maps them), so the stale values overwrite the row.
The same stale payload is also sent by the theme/compact switches and by every "send test notification" button (`sendTest` calls `mergeAndSave` first).
## Impact
Any general-settings save silently reverts the AI provider/model/base URL to the values seen at page load. The AI chat then calls the old provider (with a key that may not match) or the wrong billing model, with no error shown. Session timeout, requireLogin and notification fields are equally exposed to the same rollback if changed elsewhere.
A regression test demonstrating the stale payload on the unfixed code lives in the linked PR.
## Expected behavior
A general-settings save must send the currently persisted values for the fields it does not manage, so saving general settings never reverts an AI-tab save.
## Related work
- #2444 (closed stale, no fix) reported the same full-snapshot overwrite family for concurrent general-settings forms.
- #2863 (closed, unmerged) proposed the server-side backfill direction; the equivalent empty-value backfill is present in the current code via merged #2963 (`SettingsService.saveGeneralSettings` backfills `llmEngine`/`deploymentName`/`apiVersion`/`awsRegion`/`maxTokens`/`temperature`/`apiKey`/`dingtalkSigningSecret`, but not `llmProvider`/`model`/`baseUrl`). A stale client snapshot sends non-null old values, which that backfill cannot address.
- #813 (closed not_planned) covered the reduced-payload facet of the same save path (omitted `llmEngine`/`maxTokens`/`temperature` and the stale in-memory LLM override); this defect sends explicit stale non-null values, which that mechanism does not reach.
## PR
Fix: #4255.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with web/src/pages/settings/GeneralSettingsTab.tsx, especially buildPayload and mergeAndSave, then trace POST /api/settings/general/save into SettingsService.java and GeneralSettingsUpdateDTO.java. Review the regression test in PR #4255 and verify that saving general settings after an AI-tab save preserves the current llmProvider, model, and baseUrl values.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java, typescript
- Domain
- full-stack
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 25/100