This is an automated email from the ASF dual-hosted git repository.
lizhimins pushed a commit to branch rocketmq-studio
in repository https://gitbox.apache.org/repos/asf/rocketmq-dashboard.git
The following commit(s) were added to refs/heads/rocketmq-studio by this push:
new 975a82e55 fix(settings): keep scoped writes and fail closed on prompts
(#6279)
975a82e55 is described below
commit 975a82e557aada209d52fa0bb0774232cb336079
Author: pilichoumao <[email protected]>
AuthorDate: Mon Oct 12 16:16:26 2026 +0800
fix(settings): keep scoped writes and fail closed on prompts (#6279)
Two fixes where a component kept acting on state it should not have
trusted: a settings save that wrote over fields belonging to the other settings
surface, and a CLI risk gate that continued after its own prompt failed to
render.
- #6279 saving either half of the settings clobbered the other. In one
direction the console form posts `llmProvider`, `model` and `baseUrl` from its
mount-time snapshot (`GeneralSettingsTab.tsx:44-59`) and
`SettingsController.java:47-48` passed them straight to `saveGeneralSettings`,
which inherits a stored value only when the incoming field is **blank**
(`SettingsService.java:144-163`), so a stale non-blank value was written back
over a newer AI save; in the other direction `LlmConf [...]
- #6323 `rmqctl/cmd/catalog.go:372-373` discarded the results of the two
`fmt.Fprintf(out, …)` calls that print the risk WARNING and the target prompt
inside `defaultConfirm`, then went on to start the stdin reader goroutine — so
when stderr could not be written, an L3 change such as `group reset-offset`
still issued its preview and apply requests and exited 0 on a `yes` already
sitting in the TTY buffer, without the operator ever being shown what was about
to happen → both writes are [...]
Tests: `SettingsWriteScopeTest` (three cases) constructs the real
`SettingsService` and `LlmConfigService` over a mocked repository and asserts
an actual `Thread.State.BLOCKED` for the in-flight general write,
`SettingsWritePersistenceTest` (three cases) proves on H2 that the flag is
never stored and that a legacy full update still replaces the AI fields,
`SettingsControllerTest` pins the routing with `saveGeneralSettings(any(),
eq(true))`, `generalSettings.test.ts` pins the payload a [...]
Folded: #6323 was closed as folded into this commit (same author).
---
docs/ai-agent-architecture.md | 3 +
docs/api-spec.md | 3 +
rmqctl/cmd/catalog.go | 8 +-
rmqctl/cmd/catalog_confirm_output_test.go | 128 +++++++++++++++++++
.../rocketmq/studio/ops/ai/LlmConfigService.java | 71 +++++------
.../studio/settings/GeneralSettingsUpdateDTO.java | 2 +
.../studio/settings/SettingsController.java | 6 +-
.../rocketmq/studio/settings/SettingsService.java | 21 ++++
.../studio/ops/ai/LlmConfigServiceTest.java | 6 +
.../studio/settings/SettingsControllerTest.java | 12 ++
.../settings/SettingsWritePersistenceTest.java | 87 +++++++++++++
.../studio/settings/SettingsWriteScopeTest.java | 136 +++++++++++++++++++++
web/src/api/generalSettings.test.ts | 9 ++
web/src/api/settings.ts | 1 +
web/src/pages/settings/GeneralSettingsTab.tsx | 1 +
.../settings/__tests__/GeneralSettingsTab.test.tsx | 1 +
16 files changed, 453 insertions(+), 42 deletions(-)
diff --git a/docs/ai-agent-architecture.md b/docs/ai-agent-architecture.md
index 25a54861e..c8b8766ed 100644
--- a/docs/ai-agent-architecture.md
+++ b/docs/ai-agent-architecture.md
@@ -182,6 +182,9 @@ drain 写 `SHUTDOWN` 而不是把一次重新部署误标成用户取消。停
`NODE_EXTRA_CA_CERTS`/代理变量都已在白名单里。不加 `RMQCTL_CONFIG`——我们显式传 `--config`。
-
内置工具（`Bash`/`Read`/`Write`/`Edit`/`Glob`/`Grep`/`Task`/`WebFetch`/`WebSearch`/`TodoWrite`/
`NotebookEdit`）**继续全部禁用**。放行的只有 MCP 工具。Agent 不该在服务端容器里跑 shell 或读文件系统。
+- 独立 CLI 的 L2/L3 交互确认必须成功写出警告和目标提示，才能读取确认答案；
+ stderr 写入失败会在发送任何预览或执行请求之前以非零状态退出。显式 `--yes`
+ 和 `--dry-run` 保持原有的免提示行为。
- L2/L3 变更走工具协议自身的两步：dry-run 返回 `confirm_token`，apply 消费它。服务端由
`ToolMutationFilter` + `ToolTokenService` + `studio.ai.allow-l3-tools`（默认
false）强制。
**因此托管 Agent 默认无法执行 L3 工具**，除非部署方显式打开开关；系统提示词已交代它必须先预览、
diff --git a/docs/api-spec.md b/docs/api-spec.md
index b7a8842e8..bbbd2261b 100644
--- a/docs/api-spec.md
+++ b/docs/api-spec.md
@@ -2275,6 +2275,9 @@ POST /api/settings/general/save
| `clearApiKey` | `boolean` | 否 | 传 `true` 时显式清除现有密钥，优先级高于 `apiKey` |
| `model` | `string` | 是 | 模型名称 |
| `baseUrl` | `string` | 是 | Base URL |
+| `preserveLlmConfig` | `boolean` | 否 | 默认 `false`；通用设置表单传 `true`，保留服务端当前 AI
配置 |
+
+`preserveLlmConfig=true` 时，请求中的 AI 字段（包括 API Key
及其清空标记）不会覆盖当前值；原有必填字段校验不变。省略该标记或传 `false` 时维持原完整更新契约。控制台通用表单使用该标记，AI 表单只修改 AI
配置，两条路径的读、合并和保存在单个服务进程内串行执行。这不提供跨进程的版本冲突检测；旧调用方的完整更新仍可能覆盖其他写入。标记不存入设置
JSON，也不出现在读取响应中。
**Response `data`:** `null`
diff --git a/rmqctl/cmd/catalog.go b/rmqctl/cmd/catalog.go
index 7f1dddb11..735d2dce8 100644
--- a/rmqctl/cmd/catalog.go
+++ b/rmqctl/cmd/catalog.go
@@ -369,8 +369,12 @@ func defaultConfirm(ctx context.Context, in io.Reader, out
io.Writer, commandPat
if (stat.Mode() & os.ModeCharDevice) == 0 {
return riskConfirmationRequired(commandPath, riskLevel)
}
- fmt.Fprintf(out, "WARNING: %q is a %s operation.\n", commandPath,
riskLevel)
- fmt.Fprintf(out, "Arguments will be sent to %s. Type \"yes\" to
continue: ", server)
+ if _, err := fmt.Fprintf(out, "WARNING: %q is a %s operation.\n",
commandPath, riskLevel); err != nil {
+ return fmt.Errorf("write confirmation warning: %w", err)
+ }
+ if _, err := fmt.Fprintf(out, "Arguments will be sent to %s. Type
\"yes\" to continue: ", server); err != nil {
+ return fmt.Errorf("write confirmation prompt: %w", err)
+ }
// The stdin read itself cannot be interrupted, so run it alongside the
context: a canceled
// context always wins over an answer that may already have been typed,
and the buffered
diff --git a/rmqctl/cmd/catalog_confirm_output_test.go
b/rmqctl/cmd/catalog_confirm_output_test.go
new file mode 100644
index 000000000..07763db3d
--- /dev/null
+++ b/rmqctl/cmd/catalog_confirm_output_test.go
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package cmd
+
+import (
+ "bytes"
+ "context"
+ "encoding/json"
+ "errors"
+ "io"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "sync/atomic"
+ "testing"
+)
+
+var errConfirmationOutput = errors.New("synthetic confirmation output failure")
+
+type confirmationFailWriter struct{ failAt, writes int }
+
+func (w *confirmationFailWriter) Write(p []byte) (int, error) {
+ w.writes++
+ if w.writes == w.failAt {
+ return 0, errConfirmationOutput
+ }
+ return len(p), nil
+}
+
+type confirmationCountingReader struct {
+ io.Reader
+ reads atomic.Int32
+}
+
+func (r *confirmationCountingReader) Read(p []byte) (int, error) {
+ r.reads.Add(1)
+ return r.Reader.Read(p)
+}
+
+func TestDefaultConfirmOutputFailureDoesNotReadAnswer(t *testing.T) {
+ for _, failAt := range []int{1, 2} {
+ reader := &confirmationCountingReader{Reader:
strings.NewReader("yes\n")}
+ err := defaultConfirm(context.Background(), charDeviceReader{r:
reader}, &confirmationFailWriter{failAt: failAt}, "group reset-offset", "L3",
"http://studio";)
+ if !errors.Is(err, errConfirmationOutput) {
+ t.Errorf("write %d: lost error: %v", failAt, err)
+ }
+ if reader.reads.Load() != 0 {
+ t.Errorf("write %d: consumed answer after output
failure", failAt)
+ }
+ }
+}
+
+func TestConfirmationOutputFailureStopsToolExecution(t *testing.T) {
+ cases := []struct {
+ name, answer string
+ failAt int
+ flags []string
+ wantCode, wantCalls int
+ wantRead bool
+ }{
+ {name: "first prompt fails", answer: "yes\n", failAt: 1,
wantCode: 1},
+ {name: "second prompt fails", answer: "yes\n", failAt: 2,
wantCode: 1},
+ {name: "visible prompt accepted", answer: "yes\n", wantCalls:
2, wantRead: true},
+ {name: "visible prompt rejected", answer: "no\n", wantCode: 1,
wantRead: true},
+ {name: "yes bypasses prompt", failAt: 1, flags:
[]string{"--yes"}, wantCalls: 2},
+ {name: "dry run bypasses prompt", failAt: 1, flags:
[]string{"--dry-run"}, wantCalls: 1},
+ }
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ var calls atomic.Int32
+ server := httptest.NewServer(http.HandlerFunc(func(w
http.ResponseWriter, r *http.Request) {
+ calls.Add(1)
+ var request struct {
+ Name string `json:"name"`
+ Arguments map[string]any
`json:"arguments"`
+ }
+ if err :=
json.NewDecoder(r.Body).Decode(&request); err != nil {
+ t.Error(err)
+ return
+ }
+ if request.Name != "rmq.group.reset_offset" {
+ t.Errorf("unexpected tool %s",
request.Name)
+ }
+ status := "EXECUTED"
+ if request.Arguments["dry_run"] == true {
+ status = "PLANNED"
+ } else if request.Arguments["confirm_token"] !=
"synthetic-token" {
+ t.Error("apply without preview token")
+ }
+ writeStudioSuccess(t, w,
map[string]any{"status": status, "instanceId": "test-instance",
"confirm_token": "synthetic-token", "plan": map[string]any{}, "result":
map[string]any{}})
+ }))
+ defer server.Close()
+ reader := &confirmationCountingReader{Reader:
strings.NewReader(tc.answer)}
+ writer := &confirmationFailWriter{failAt: tc.failAt}
+ app := NewApp(&bytes.Buffer{}, writer)
+ app.In = charDeviceReader{r: reader}
+ app.HTTP = server.Client()
+ app.Store.Getenv = testEnv
+ path := newTestConfigPath(t)
+ if err := app.Store.Save(path,
newTestConfig(server.URL)); err != nil {
+ t.Fatal(err)
+ }
+ args := []string{"--config", path, "--instance-id",
"test-instance", "group", "reset-offset", "--group-name", "orders",
"--topic-name", "orders", "--timestamp", "1700000000000"}
+ code := app.Execute(append(args, tc.flags...))
+ t.Logf("exit=%d HTTP requests=%d answer reads=%d",
code, calls.Load(), reader.reads.Load())
+ if code != tc.wantCode || int(calls.Load()) !=
tc.wantCalls {
+ t.Errorf("exit/calls=%d/%d, want %d/%d", code,
calls.Load(), tc.wantCode, tc.wantCalls)
+ }
+ if (reader.reads.Load() > 0) != tc.wantRead {
+ t.Errorf("answer read=%v, want %v",
reader.reads.Load() > 0, tc.wantRead)
+ }
+ })
+ }
+}
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/ops/ai/LlmConfigService.java
b/server/src/main/java/org/apache/rocketmq/studio/ops/ai/LlmConfigService.java
index 138087787..feabb0dcd 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/ops/ai/LlmConfigService.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/ops/ai/LlmConfigService.java
@@ -107,44 +107,33 @@ public class LlmConfigService {
}
public synchronized void saveConfig(LlmConfigVO config) {
- LlmConfigVO normalized = normalizeWithStoredApiKey(config);
- LlmOperationResultVO validation = validate(normalized);
- if (validation.getStatus() != 0) {
- throw new LlmGatewayException(400, validation.getCode(),
validation.getErrMsg(), validation.getHint());
- }
- GeneralSettingsVO current = settingsService.getGeneralSettings();
- // The env-injected token is authoritative at runtime but must never
be persisted.
- String persistedApiKey = !StringUtils.hasText(envToken())
- ? normalized.getApiKey()
- : defaultString(current.getApiKey(), "");
- GeneralSettingsVO updated = GeneralSettingsVO.builder()
- .theme(current.getTheme())
- .compact(current.isCompact())
- .desktopNotify(current.isDesktopNotify())
- .notifySound(current.isNotifySound())
- .sessionTimeout(current.getSessionTimeout())
- .requireLogin(current.isRequireLogin())
- // The LLM form never edits notification channels; keep the
stored values
- // instead of persisting nulls over them.
- .dingtalkWebhook(current.getDingtalkWebhook())
- .smsWebhook(current.getSmsWebhook())
- .emailRecipients(current.getEmailRecipients())
- .llmProvider(normalized.getProvider())
- .llmEngine(normalized.getEngine())
- .apiKey(persistedApiKey)
- // Carry the clear intent into the persisted settings: without
it a blank key plus
- // clearApiKey=false means "keep the stored key" to
SettingsService's merge, so the
- // secret the operator just removed would be written straight
back.
- .clearApiKey(normalized.isClearApiKey())
- .model(normalized.getModel())
- .baseUrl(normalized.getApiBase())
- .deploymentName(normalized.getDeploymentName())
- .apiVersion(normalized.getApiVersion())
- .awsRegion(normalized.getAwsRegion())
- .maxTokens(normalized.getMaxTokens())
- .temperature(normalized.getTemperature())
- .build();
- settingsService.saveGeneralSettings(updated);
+ settingsService.updateGeneralSettings(current -> {
+ LlmConfigVO normalized = normalizeWithStoredApiKey(config,
current);
+ LlmOperationResultVO validation = validate(normalized);
+ if (validation.getStatus() != 0) {
+ throw new LlmGatewayException(400, validation.getCode(),
validation.getErrMsg(), validation.getHint());
+ }
+ // The env-injected token is authoritative at runtime but must
never be persisted.
+ String persistedApiKey = !StringUtils.hasText(envToken())
+ ? normalized.getApiKey()
+ : defaultString(current.getApiKey(), "");
+ return current.toBuilder()
+ .llmProvider(normalized.getProvider())
+ .llmEngine(normalized.getEngine())
+ .apiKey(persistedApiKey)
+ // Carry the clear intent into the persisted settings:
without it a blank key plus
+ // clearApiKey=false means "keep the stored key" to
SettingsService's merge, so the
+ // secret the operator just removed would be written
straight back.
+ .clearApiKey(normalized.isClearApiKey())
+ .model(normalized.getModel())
+ .baseUrl(normalized.getApiBase())
+ .deploymentName(normalized.getDeploymentName())
+ .apiVersion(normalized.getApiVersion())
+ .awsRegion(normalized.getAwsRegion())
+ .maxTokens(normalized.getMaxTokens())
+ .temperature(normalized.getTemperature())
+ .build();
+ });
}
public LlmOperationResultVO testConfig(LlmConfigVO config) {
@@ -307,6 +296,10 @@ public class LlmConfigService {
}
private LlmConfigVO normalizeWithStoredApiKey(LlmConfigVO config) {
+ return normalizeWithStoredApiKey(config, null);
+ }
+
+ private LlmConfigVO normalizeWithStoredApiKey(LlmConfigVO config,
GeneralSettingsVO current) {
LlmConfigVO normalized = normalize(config);
if (normalized.isClearApiKey()) {
normalized.setApiKey("");
@@ -321,7 +314,7 @@ public class LlmConfigService {
// otherwise fall back to the key stored in settings.
String effectiveKey = envToken();
if (!StringUtils.hasText(effectiveKey)) {
- effectiveKey =
settingsService.getGeneralSettings().getApiKey();
+ effectiveKey = current == null ?
settingsService.getGeneralSettings().getApiKey() : current.getApiKey();
}
normalized.setApiKey(defaultString(effectiveKey, ""));
}
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/settings/GeneralSettingsUpdateDTO.java
b/server/src/main/java/org/apache/rocketmq/studio/settings/GeneralSettingsUpdateDTO.java
index a467fccb9..65bba18b2 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/settings/GeneralSettingsUpdateDTO.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/settings/GeneralSettingsUpdateDTO.java
@@ -58,6 +58,8 @@ public class GeneralSettingsUpdateDTO {
private String deploymentName;
private String apiVersion;
private String awsRegion;
+ /** Preserve all AI fields when saving the console's non-AI settings form.
*/
+ private boolean preserveLlmConfig;
private String dingtalkWebhook;
@ToString.Exclude
private String dingtalkSigningSecret;
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsController.java
b/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsController.java
index f8f01c9b3..0034a40a3 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsController.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsController.java
@@ -45,7 +45,11 @@ public class SettingsController {
@PostMapping("/general/save")
public Result<Void> saveGeneralSettings(@Valid @RequestBody
GeneralSettingsUpdateDTO request) {
- settingsService.saveGeneralSettings(request.toSettings());
+ if (request.isPreserveLlmConfig()) {
+ settingsService.saveGeneralSettings(request.toSettings(), true);
+ } else {
+ settingsService.saveGeneralSettings(request.toSettings());
+ }
return Result.ok();
}
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsService.java
b/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsService.java
index 1d616969e..2cbe056cd 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsService.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/settings/SettingsService.java
@@ -51,6 +51,7 @@ import java.util.Arrays;
import java.util.List;
import java.util.Locale;
import java.util.Set;
+import java.util.function.UnaryOperator;
@Slf4j
@Service
@@ -127,6 +128,26 @@ public class SettingsService {
}
+ /** Keep read, construction, secret inheritance and persistence under the
same write monitor. */
+ public synchronized void
updateGeneralSettings(UnaryOperator<GeneralSettingsVO> update) {
+
saveGeneralSettings(update.apply(settingsRepository.loadGeneralSettings()));
+ }
+
+ /** Explicitly scoped console writes leave AI fields owned by the AI
endpoint unchanged. */
+ public synchronized void saveGeneralSettings(GeneralSettingsVO settings,
boolean preserveLlmConfig) {
+ if (preserveLlmConfig) {
+ GeneralSettingsVO current =
settingsRepository.loadGeneralSettings();
+ settings = settings.toBuilder()
+
.llmProvider(current.getLlmProvider()).llmEngine(current.getLlmEngine())
+ .apiKey(current.getApiKey()).clearApiKey(false)
+ .model(current.getModel()).baseUrl(current.getBaseUrl())
+
.deploymentName(current.getDeploymentName()).apiVersion(current.getApiVersion())
+
.awsRegion(current.getAwsRegion()).maxTokens(current.getMaxTokens())
+ .temperature(current.getTemperature()).build();
+ }
+ saveGeneralSettings(settings);
+ }
+
public synchronized void saveGeneralSettings(GeneralSettingsVO settings) {
log.info("Saving general settings");
validateLlmBaseUrl(settings.getBaseUrl());
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/ops/ai/LlmConfigServiceTest.java
b/server/src/test/java/org/apache/rocketmq/studio/ops/ai/LlmConfigServiceTest.java
index 4c5f7e5ef..3b5eb3a9c 100644
---
a/server/src/test/java/org/apache/rocketmq/studio/ops/ai/LlmConfigServiceTest.java
+++
b/server/src/test/java/org/apache/rocketmq/studio/ops/ai/LlmConfigServiceTest.java
@@ -30,6 +30,7 @@ import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.UnaryOperator;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -64,6 +65,11 @@ class LlmConfigServiceTest {
.model("gpt-4o")
.baseUrl("https://api.openai.com/v1";)
.build());
+ doAnswer(invocation -> {
+ UnaryOperator<GeneralSettingsVO> update =
invocation.getArgument(0);
+
settingsService.saveGeneralSettings(update.apply(settingsService.getGeneralSettings()));
+ return null;
+ }).when(settingsService).updateGeneralSettings(any());
llmClient = mock(OpenAiCompatibleLlmClient.class);
agentProviders = mock(AgentProviderRegistry.class);
llmConfigService = new LlmConfigService(
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsControllerTest.java
b/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsControllerTest.java
index 74258f82b..60bfffe4b 100644
---
a/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsControllerTest.java
+++
b/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsControllerTest.java
@@ -41,6 +41,7 @@ import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.verify;
@@ -67,6 +68,17 @@ class SettingsControllerTest extends WebMvcAuthTestSupport {
@MockitoBean
private NotificationOutboxService notificationOutboxService;
+ @Test
+ void scopedGeneralSaveShouldPassPreservationIntentTest() throws Exception {
+
mockMvc.perform(post("/api/settings/general/save").contentType(MediaType.APPLICATION_JSON).content("""
+
{"theme":"dark","compact":false,"desktopNotify":false,"notifySound":false,
+
"sessionTimeout":60,"requireLogin":false,"llmProvider":"openai","model":"old-model",
+
"baseUrl":"https://api.openai.com/v1","preserveLlmConfig":true}
+ """))
+ .andExpect(status().isOk());
+
verify(settingsService).saveGeneralSettings(any(GeneralSettingsVO.class),
eq(true));
+ }
+
@Test
void getGeneralSettingsShouldReturnSettingsTest() throws Exception {
GeneralSettingsVO settings = GeneralSettingsVO.builder()
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsWritePersistenceTest.java
b/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsWritePersistenceTest.java
new file mode 100644
index 000000000..621e2ef16
--- /dev/null
+++
b/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsWritePersistenceTest.java
@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.rocketmq.studio.settings;
+
+import org.apache.rocketmq.studio.ops.ai.LlmConfigService;
+import org.apache.rocketmq.studio.ops.ai.LlmConfigVO;
+import org.apache.rocketmq.studio.persistence.mapper.RmqSettingsMapper;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.test.context.ActiveProfiles;
+import static org.assertj.core.api.Assertions.assertThat;
+
+@SpringBootTest(properties = {
+
"spring.datasource.url=jdbc:h2:mem:settings-write-scope;MODE=MySQL;DB_CLOSE_DELAY=-1;DATABASE_TO_LOWER=TRUE",
+ "studio.llm.token="
+})
+@ActiveProfiles("dev")
+class SettingsWritePersistenceTest {
+ @Autowired
+ private SettingsService settings;
+ @Autowired
+ private LlmConfigService llm;
+ @Autowired
+ private SettingsRepository repository;
+ @Autowired
+ private RmqSettingsMapper mapper;
+
+ @BeforeEach
+ void initializeTest() {
+
settings.saveGeneralSettings(GeneralSettingsVO.builder().theme("light").sessionTimeout(1440)
+
.llmProvider("openai").model("old-model").apiKey("synthetic-key")
+
.baseUrl("https://api.openai.com/v1";).emailRecipients("[email protected]").build());
+ }
+
+ @Test
+ void scopedSaveShouldPersistBothFormsWithoutPersistingTheIntentFlagTest() {
+ GeneralSettingsVO pending =
repository.loadGeneralSettings().toBuilder()
+ .theme("dark").emailRecipients("[email protected]").build();
+
llm.saveConfig(LlmConfigVO.builder().provider("openai").model("new-model")
+
.apiBase("https://api.openai.com/v1";).maxTokens(8192).temperature(0.3).build());
+ settings.saveGeneralSettings(pending, true);
+ GeneralSettingsVO result = repository.loadGeneralSettings();
+ assertThat(result.getTheme()).isEqualTo("dark");
+ assertThat(result.getEmailRecipients()).isEqualTo("[email protected]");
+ assertThat(result.getModel()).isEqualTo("new-model");
+ assertThat(result.getApiKey()).isEqualTo("synthetic-key");
+ assertThat(result.getMaxTokens()).isEqualTo(8192);
+ assertThat(result.getTemperature()).isEqualTo(0.3);
+ assertThat(mapper.selectList(null)).hasSize(1);
+
assertThat(mapper.selectList(null).getFirst().getJson()).doesNotContain("preserveLlmConfig");
+ }
+
+ @Test
+ void legacyFullSaveShouldStillUpdateAiFieldsTest() {
+
settings.saveGeneralSettings(repository.loadGeneralSettings().toBuilder().model("legacy-model")
+ .apiKey("legacy-key").build());
+ GeneralSettingsVO result = repository.loadGeneralSettings();
+ assertThat(result.getModel()).isEqualTo("legacy-model");
+ assertThat(result.getApiKey()).isEqualTo("legacy-key");
+ }
+
+ @Test
+ void aiKeyClearShouldRetainNotificationFieldsTest() {
+
llm.saveConfig(LlmConfigVO.builder().provider("ollama").model("local-model")
+
.apiBase("http://localhost:11434/v1";).clearApiKey(true).build());
+ GeneralSettingsVO result = repository.loadGeneralSettings();
+ assertThat(result.getApiKey()).isEmpty();
+ assertThat(result.getEmailRecipients()).isEqualTo("[email protected]");
+ assertThat(result.getTheme()).isEqualTo("light");
+ }
+}
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsWriteScopeTest.java
b/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsWriteScopeTest.java
new file mode 100644
index 000000000..0a36b6ec7
--- /dev/null
+++
b/server/src/test/java/org/apache/rocketmq/studio/settings/SettingsWriteScopeTest.java
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.rocketmq.studio.settings;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import org.apache.rocketmq.studio.audit.OperationAuditService;
+import org.apache.rocketmq.studio.ops.ai.LlmConfigService;
+import org.apache.rocketmq.studio.ops.ai.LlmConfigVO;
+import org.apache.rocketmq.studio.ops.ai.LlmProperties;
+import org.apache.rocketmq.studio.ops.ai.OpenAiCompatibleLlmClient;
+import org.apache.rocketmq.studio.ops.ai.AgentProviderRegistry;
+import org.apache.rocketmq.studio.ops.alert.NotificationOutboxService;
+import org.junit.jupiter.api.Test;
+import org.springframework.web.client.RestClient;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.ArgumentMatchers.any;
+
+class SettingsWriteScopeTest {
+ private final AtomicReference<GeneralSettingsVO> stored = new
AtomicReference<>(GeneralSettingsVO.builder()
+
.theme("light").sessionTimeout(1440).llmProvider("openai").apiKey("synthetic-key")
+ .model("old-model").baseUrl("https://api.openai.com/v1";).build());
+ private final SettingsRepository repository =
mock(SettingsRepository.class);
+
+ private SettingsService settings() {
+ when(repository.loadGeneralSettings()).thenAnswer(inv ->
stored.get().toBuilder().build());
+ doAnswer(inv -> {
+ stored.set(((GeneralSettingsVO)
inv.getArgument(0)).toBuilder().build());
+ return null;
+ })
+ .when(repository).saveGeneralSettings(any());
+ return new SettingsService(repository, RestClient.builder(), new
ObjectMapper(), mock(OperationAuditService.class));
+ }
+
+ private LlmConfigService llm(SettingsService settings) {
+ return new LlmConfigService(settings,
mock(OpenAiCompatibleLlmClient.class),
+ mock(AgentProviderRegistry.class), new LlmProperties());
+ }
+
+ private LlmConfigVO config() {
+ return
LlmConfigVO.builder().provider("openai").apiKey("synthetic-key").model("new-model")
+
.apiBase("https://api.openai.com/v1";).maxTokens(4096).temperature(0.7).build();
+ }
+
+ @Test
+ void scopedWriteShouldIgnoreStaleAiFieldsAndSecretClearIntentTest() {
+ SettingsService service = settings();
+ GeneralSettingsVO current =
stored.get().toBuilder().llmEngine("http").apiVersion("version")
+
.deploymentName("deployment").awsRegion("region").maxTokens(8192).temperature(0.3).build();
+ stored.set(current);
+
service.saveGeneralSettings(current.toBuilder().theme("dark").apiKey("stale-key").clearApiKey(true)
+
.llmProvider("stale-provider").llmEngine("stale-engine").model("stale-model")
+
.baseUrl("stale-invalid-url").deploymentName("stale-deployment").apiVersion("stale-version")
+
.awsRegion("stale-region").maxTokens(1).temperature(1.9).build(), true);
+
assertThat(stored.get()).usingRecursiveComparison().ignoringFields("theme").isEqualTo(current);
+ assertThat(stored.get().getTheme()).isEqualTo("dark");
+ }
+
+ @Test
+ void scopedGeneralRequestShouldPreserveAnInterveningAiSaveTest() throws
Exception {
+ SettingsService service = settings();
+ ObjectMapper mapper = new
ObjectMapper().disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
+ GeneralSettingsUpdateDTO request = mapper.readValue("""
+
{"theme":"dark","compact":false,"desktopNotify":false,"notifySound":false,
+
"sessionTimeout":1440,"requireLogin":false,"llmProvider":"openai",
+
"model":"old-model","baseUrl":"https://api.openai.com/v1","preserveLlmConfig":true}
+ """, GeneralSettingsUpdateDTO.class);
+ llm(service).saveConfig(config());
+ new SettingsController(service,
mock(NotificationOutboxService.class)).saveGeneralSettings(request);
+ assertThat(stored.get().getTheme()).isEqualTo("dark");
+ assertThat(stored.get().getModel()).isEqualTo("new-model");
+ }
+
+ @Test
+ void aiSaveShouldReadAfterAnInFlightGeneralWriteTest() throws Exception {
+ SettingsService service = settings();
+ CountDownLatch persisting = new CountDownLatch(1);
+ CountDownLatch release = new CountDownLatch(1);
+ doAnswer(inv -> {
+ if (Thread.currentThread().getName().equals("general-writer")) {
+ persisting.countDown();
+ assertThat(release.await(5, TimeUnit.SECONDS)).isTrue();
+ }
+ stored.set(((GeneralSettingsVO)
inv.getArgument(0)).toBuilder().build());
+ return null;
+ }).when(repository).saveGeneralSettings(any());
+ AtomicReference<Thread> aiThread = new AtomicReference<>();
+ try (ExecutorService general = Executors.newSingleThreadExecutor(r ->
new Thread(r, "general-writer"));
+ ExecutorService ai = Executors.newSingleThreadExecutor(r -> {
+ Thread thread = new Thread(r, "ai-writer");
+ aiThread.set(thread);
+ return thread;
+ })) {
+ Future<?> generalWrite = general.submit(() ->
service.saveGeneralSettings(stored.get().toBuilder()
+
.theme("dark").emailRecipients("[email protected]").build()));
+ try {
+ assertThat(persisting.await(5, TimeUnit.SECONDS)).isTrue();
+ Future<?> aiWrite = ai.submit(() ->
llm(service).saveConfig(config()));
+ long deadline = System.nanoTime() +
TimeUnit.SECONDS.toNanos(5);
+ while (aiThread.get().getState() != Thread.State.BLOCKED &&
System.nanoTime() < deadline) {
+ Thread.yield();
+ }
+
assertThat(aiThread.get().getState()).isEqualTo(Thread.State.BLOCKED);
+ release.countDown();
+ generalWrite.get(5, TimeUnit.SECONDS);
+ aiWrite.get(5, TimeUnit.SECONDS);
+ assertThat(stored.get().getTheme()).isEqualTo("dark");
+
assertThat(stored.get().getEmailRecipients()).isEqualTo("[email protected]");
+ assertThat(stored.get().getModel()).isEqualTo("new-model");
+ } finally { release.countDown(); }
+ }
+ }
+}
diff --git a/web/src/api/generalSettings.test.ts
b/web/src/api/generalSettings.test.ts
index e59711e2a..1ff96ae54 100644
--- a/web/src/api/generalSettings.test.ts
+++ b/web/src/api/generalSettings.test.ts
@@ -95,6 +95,15 @@ describe('general settings API', () => {
await expect(saveGeneralSettings(update)).resolves.toBeUndefined();
});
+ it('forwards the explicit non-AI scope without changing legacy requests',
async () => {
+ const update: GeneralSettingsUpdate = { ...editableSettings,
preserveLlmConfig: true };
+ mock.onPost('/settings/general/save').reply((config) => {
+ expect(JSON.parse(config.data)).toEqual(update);
+ return [200, { code: 200, data: null }];
+ });
+ await expect(saveGeneralSettings(update)).resolves.toBeUndefined();
+ });
+
it('sends the explicit API key clear flag', async () => {
const update: GeneralSettingsUpdate = { ...editableSettings, clearApiKey:
true };
mock.onPost('/settings/general/save').reply((config) => {
diff --git a/web/src/api/settings.ts b/web/src/api/settings.ts
index 45344ecd3..63561c4fe 100644
--- a/web/src/api/settings.ts
+++ b/web/src/api/settings.ts
@@ -48,6 +48,7 @@ export type GeneralSettingsUpdate = Omit<
> & {
apiKey?: string;
clearApiKey?: boolean;
+ preserveLlmConfig?: boolean;
clearDingtalkSigningSecret?: boolean;
};
diff --git a/web/src/pages/settings/GeneralSettingsTab.tsx
b/web/src/pages/settings/GeneralSettingsTab.tsx
index 7680bcd4e..e63814579 100644
--- a/web/src/pages/settings/GeneralSettingsTab.tsx
+++ b/web/src/pages/settings/GeneralSettingsTab.tsx
@@ -43,6 +43,7 @@ const toThemeMode = (value?: string): ThemeMode =>
value === 'light' || value === 'dark' ? value : 'system';
const buildPayload = (settings: GeneralSettings): GeneralSettingsUpdate => ({
+ preserveLlmConfig: true,
theme: settings.theme,
compact: settings.compact,
desktopNotify: settings.desktopNotify,
diff --git a/web/src/pages/settings/__tests__/GeneralSettingsTab.test.tsx
b/web/src/pages/settings/__tests__/GeneralSettingsTab.test.tsx
index 7eb329042..1ebb42c44 100644
--- a/web/src/pages/settings/__tests__/GeneralSettingsTab.test.tsx
+++ b/web/src/pages/settings/__tests__/GeneralSettingsTab.test.tsx
@@ -117,6 +117,7 @@ describe('GeneralSettingsTab', () => {
await waitFor(() =>
expect(saveGeneralSettings).toHaveBeenCalledWith(
expect.objectContaining({
+ preserveLlmConfig: true,
theme: 'system',
compact: false,
desktopNotify: false,
