From 32d117c68b0a68dba35b3269db7e39f4502ab1ca Mon Sep 17 00:00:00 2001 From: Wynne668 Date: Mon, 29 Jun 2026 10:15:27 +0800 Subject: [PATCH] fix(control-ui): persist Set Default agent through config save --- ui/src/ui/app-render.ts | 4 +- ui/src/ui/controllers/agents.test.ts | 77 ++++++++++++- ui/src/ui/controllers/agents.ts | 17 ++- ...agents-set-default-persistence.e2e.test.ts | 106 ++++++++++++++++++ 4 files changed, 200 insertions(+), 4 deletions(-) create mode 100644 ui/src/ui/e2e/agents-set-default-persistence.e2e.test.ts diff --git a/ui/src/ui/app-render.ts b/ui/src/ui/app-render.ts index 1ef7ece49993..7f3c7fc1b0d6 100644 --- a/ui/src/ui/app-render.ts +++ b/ui/src/ui/app-render.ts @@ -53,6 +53,7 @@ import { resetToolsEffectiveState, refreshVisibleToolsEffectiveForCurrentSession, saveAgentsConfig, + setDefaultAgent, } from "./controllers/agents.ts"; import { setAssistantAvatarOverride } from "./controllers/assistant-identity.ts"; import { loadChannels } from "./controllers/channels.ts"; @@ -66,7 +67,6 @@ import { resetConfigPendingChanges, runUpdate, saveConfig, - stageDefaultAgentConfigEntry, stageConfigPreset, updateConfigRawValue, updateConfigFormValue, @@ -3480,7 +3480,7 @@ export function renderApp(state: AppViewState) { updateConfigFormValue(state, basePathResult, { primary, fallbacks: normalized }); }, onSetDefault: (agentId) => { - stageDefaultAgentConfigEntry(state, agentId); + void setDefaultAgent(state, agentId); }, }), ) diff --git a/ui/src/ui/controllers/agents.test.ts b/ui/src/ui/controllers/agents.test.ts index 18d3551df87e..8a103056c800 100644 --- a/ui/src/ui/controllers/agents.test.ts +++ b/ui/src/ui/controllers/agents.test.ts @@ -1,6 +1,12 @@ // Control UI tests cover agents behavior. import { describe, expect, it, vi } from "vitest"; -import { loadAgents, loadToolsCatalog, loadToolsEffective, saveAgentsConfig } from "./agents.ts"; +import { + loadAgents, + loadToolsCatalog, + loadToolsEffective, + saveAgentsConfig, + setDefaultAgent, +} from "./agents.ts"; import type { AgentsConfigSaveState, AgentsState } from "./agents.ts"; type TestRequest = (method: string, payload?: unknown) => Promise; @@ -430,3 +436,72 @@ describe("saveAgentsConfig", () => { expect(state.agentsSelectedId).toBe("main"); }); }); + +describe("setDefaultAgent", () => { + it("stages the canonical default flag and persists it through config.set", async () => { + const { state, request } = createSaveState(); + state.configForm = { agents: { list: [{ id: "main" }, { id: "kimi" }] } }; + state.configFormOriginal = { agents: { list: [{ id: "main" }, { id: "kimi" }] } }; + state.configFormDirty = false; + request + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce({ + hash: "hash-2", + raw: '{"agents":{"list":[{"id":"main"},{"id":"kimi","default":true}]}}', + config: { agents: { list: [{ id: "main" }, { id: "kimi", default: true }] } }, + valid: true, + issues: [], + }) + .mockResolvedValueOnce({ + defaultId: "kimi", + mainKey: "main", + scope: "per-sender", + agents: [ + { id: "main", name: "main" }, + { id: "kimi", name: "kimi" }, + ], + }); + + await setDefaultAgent(state, "kimi"); + + const [method, params] = requireFirstRequestCall(request); + const requestParams = requireRecord(params); + expect(method).toBe("config.set"); + expect(JSON.parse(String(requestParams.raw))).toEqual({ + agents: { list: [{ id: "main" }, { id: "kimi", default: true }] }, + }); + }); + + it("does not persist when the agent is absent from the config list", async () => { + const { state, request } = createSaveState(); + state.configForm = { agents: { list: [{ id: "main" }] } }; + + await setDefaultAgent(state, "ghost"); + + expect(request).not.toHaveBeenCalled(); + }); + + it("does not persist unrelated dirty agent config drafts", async () => { + const { state, request } = createSaveState(); + state.configFormDirty = true; + state.configFormOriginal = { agents: { list: [{ id: "main" }, { id: "kimi" }] } }; + state.configForm = { + agents: { + list: [{ id: "main", model: "gpt-5.5" }, { id: "kimi" }], + }, + }; + + await setDefaultAgent(state, "kimi"); + + expect(request).not.toHaveBeenCalled(); + expect(state.configForm).toEqual({ + agents: { + list: [ + { id: "main", model: "gpt-5.5" }, + { id: "kimi", default: true }, + ], + }, + }); + expect(state.configFormDirty).toBe(true); + }); +}); diff --git a/ui/src/ui/controllers/agents.ts b/ui/src/ui/controllers/agents.ts index 41f4bbc2bb01..698e714bbbd2 100644 --- a/ui/src/ui/controllers/agents.ts +++ b/ui/src/ui/controllers/agents.ts @@ -13,7 +13,7 @@ import type { ToolsCatalogResult, ToolsEffectiveResult, } from "../types.ts"; -import { saveConfig } from "./config.ts"; +import { saveConfig, stageDefaultAgentConfigEntry } from "./config.ts"; import type { ConfigState } from "./config.ts"; import { formatMissingOperatorReadScopeMessage, @@ -246,3 +246,18 @@ export async function saveAgentsConfig(state: AgentsConfigSaveState) { state.agentsSelectedId = selectedBefore; } } + +export async function setDefaultAgent( + state: AgentsConfigSaveState, + agentId: string, +): Promise { + const hadPendingConfigDraft = state.configFormDirty; + // Set Default is a one-click action on a clean draft, but saveConfig serializes the + // whole form. If other edits were already dirty, keep them staged for the explicit + // Save button instead of committing unrelated pending config changes. + if (stageDefaultAgentConfigEntry(state, agentId)) { + if (!hadPendingConfigDraft && state.configFormDirty) { + await saveAgentsConfig(state); + } + } +} diff --git a/ui/src/ui/e2e/agents-set-default-persistence.e2e.test.ts b/ui/src/ui/e2e/agents-set-default-persistence.e2e.test.ts new file mode 100644 index 000000000000..76ab46340402 --- /dev/null +++ b/ui/src/ui/e2e/agents-set-default-persistence.e2e.test.ts @@ -0,0 +1,106 @@ +// Control UI tests cover Agents page Set Default persistence behavior. +import { chromium, type Browser } from "playwright"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { + canRunPlaywrightChromium, + installMockGateway, + resolvePlaywrightChromiumExecutablePath, + startControlUiE2eServer, + type ControlUiE2eServer, + type MockGatewayRequest, +} from "../../test-helpers/control-ui-e2e.ts"; + +const chromiumExecutablePath = resolvePlaywrightChromiumExecutablePath(chromium.executablePath()); +const chromiumAvailable = canRunPlaywrightChromium(chromiumExecutablePath); +const allowMissingChromium = process.env.OPENCLAW_UI_E2E_ALLOW_MISSING_CHROMIUM === "1"; +const describeControlUiE2e = chromiumAvailable || !allowMissingChromium ? describe : describe.skip; + +let browser: Browser; +let server: ControlUiE2eServer; + +function requireRecord(value: unknown): Record { + if (!value || typeof value !== "object" || Array.isArray(value)) { + throw new Error("Expected object value"); + } + return value as Record; +} + +function requestParams(request: MockGatewayRequest): Record { + return requireRecord(request.params); +} + +describeControlUiE2e("Control UI agents Set Default mocked Gateway E2E", () => { + beforeAll(async () => { + if (!chromiumAvailable) { + throw new Error( + `Playwright Chromium is not installed or cannot start at ${chromiumExecutablePath}. Run \`pnpm --dir ui exec playwright install --with-deps chromium\`, or set OPENCLAW_UI_E2E_ALLOW_MISSING_CHROMIUM=1 only when intentionally skipping this lane.`, + ); + } + server = await startControlUiE2eServer(); + browser = await chromium.launch({ executablePath: chromiumExecutablePath }); + }); + + afterAll(async () => { + await browser?.close(); + await server?.close(); + }); + + it("persists Set Default through config.set instead of only staging the form draft", async () => { + const context = await browser.newContext({ + locale: "en-US", + serviceWorkers: "block", + viewport: { height: 900, width: 1280 }, + }); + const page = await context.newPage(); + const gateway = await installMockGateway(page, { + assistantName: "Main agent", + defaultAgentId: "main", + methodResponses: { + "agents.list": { + agents: [ + { id: "main", name: "Main agent" }, + { id: "kimi", name: "Kimi agent" }, + ], + defaultId: "main", + mainKey: "main", + scope: "agent", + }, + "config.get": { + config: { agents: { list: [{ id: "main" }, { id: "kimi" }] } }, + hash: "hash-1", + issues: [], + raw: '{"agents":{"list":[{"id":"main"},{"id":"kimi"}]}}', + valid: true, + }, + "config.set": { + config: { agents: { list: [{ id: "main" }, { id: "kimi", default: true }] } }, + hash: "hash-2", + issues: [], + raw: '{"agents":{"list":[{"id":"main"},{"id":"kimi","default":true}]}}', + valid: true, + }, + }, + }); + + try { + const response = await page.goto(`${server.baseUrl}agents`); + expect(response?.status()).toBe(200); + + // selectOption / click auto-wait for the element to be actionable (enabled), so + // these implicitly assert the dropdown loaded and Set Default is clickable for a + // non-default agent. + await page.locator("select.agents-select").selectOption("kimi"); + await page.getByRole("button", { name: "Set Default", exact: true }).click(); + + // The fix routes Set Default through the canonical save path; without it the click + // only stages a form draft and never emits config.set, so this request never arrives. + const setRequest = await gateway.waitForRequest("config.set"); + const raw = requestParams(setRequest).raw; + expect(JSON.parse(String(raw))).toEqual({ + agents: { list: [{ id: "main" }, { id: "kimi", default: true }] }, + }); + } finally { + await context.close(); + } + }); +});