diff --git a/crates/ironclaw_webui/frontend/src/pages/settings/components/tools-tab.test.ts b/crates/ironclaw_webui/frontend/src/pages/settings/components/tools-tab.test.ts index a7be8eadae9..6ec0e156be5 100644 --- a/crates/ironclaw_webui/frontend/src/pages/settings/components/tools-tab.test.ts +++ b/crates/ironclaw_webui/frontend/src/pages/settings/components/tools-tab.test.ts @@ -75,7 +75,12 @@ function componentProps(node, component) { return props; } -function renderToolsModule({ tools = [], translations = {}, toolError = null } = {}) { +function renderToolsModule({ + tools = [], + translations = {}, + toolError = null, + pendingPermissions = {}, +} = {}) { const saved = []; const translate = (key, params = {}) => { let value = translations[key] || key; @@ -98,6 +103,7 @@ function renderToolsModule({ tools = [], translations = {}, toolError = null } = query: { isLoading: false, error: null }, setPermission: () => {}, savedTools: {}, + pendingPermissions, error: toolError, }), }; @@ -184,6 +190,50 @@ test("Tool permission select follows global unless a per-tool override exists", assert.equal(componentProps(overrideSelect, "SelectMenu").value, "ask_each_time"); }); +test("Tool permission select retains a pending selection while saving", () => { + const { exports } = renderToolsModule(); + const rendered = exports.ToolRow({ + tool: { + name: "builtin.echo", + description: "Echo", + state: "ask_each_time", + default_state: "ask_each_time", + effective_source: "override", + locked: false, + }, + pendingPermission: "disabled", + onPermissionChange: () => {}, + isSaved: false, + }); + + const select = findComponentNode(rendered, "SelectMenu"); + assert.equal(componentProps(select, "SelectMenu").value, "disabled"); +}); + +test("ToolsTab wires pending permissions to the matching tool row", () => { + const tool = { + name: "builtin.echo", + description: "Echo", + state: "ask_each_time", + default_state: "ask_each_time", + effective_source: "override", + locked: false, + }; + const { exports } = renderToolsModule({ + tools: [tool], + pendingPermissions: { + "builtin.echo": { requestId: 7, state: "disabled" }, + }, + }); + + const rendered = exports.ToolsTab({}); + const row = findComponentNode(rendered, exports.ToolRow); + assert.ok(row, "expected ToolsTab to render the tool row"); + const props = componentProps(row, exports.ToolRow); + assert.equal(props.tool, tool); + assert.equal(props.pendingPermission, "disabled"); +}); + test("Tool rows localize built-in descriptions by capability id", () => { const { exports } = renderToolsModule({ translations: { diff --git a/crates/ironclaw_webui/frontend/src/pages/settings/components/tools-tab.tsx b/crates/ironclaw_webui/frontend/src/pages/settings/components/tools-tab.tsx index 7c61d8f53b8..81233aa8314 100644 --- a/crates/ironclaw_webui/frontend/src/pages/settings/components/tools-tab.tsx +++ b/crates/ironclaw_webui/frontend/src/pages/settings/components/tools-tab.tsx @@ -83,7 +83,7 @@ function AutoApproveCard({ settings, onSave, savedKeys, isLoading }) { ); } -function ToolRow({ tool, onPermissionChange, isSaved }) { +function ToolRow({ tool, pendingPermission, onPermissionChange, isSaved }) { const t = useT(); const description = translatedToolDescription(t, tool); const permissionStates = [ @@ -102,7 +102,8 @@ function ToolRow({ tool, onPermissionChange, isSaved }) { const current = permissionStates.find((p) => p.value === tool.state) || permissionStates[1]; const effectiveSource = tool.effective_source || "default"; - const selectedState = effectiveSource === "override" ? tool.state : "default"; + const selectedState = + pendingPermission ?? (effectiveSource === "override" ? tool.state : "default"); const isDefault = effectiveSource === "default" && tool.state === tool.default_state; return ( @@ -156,6 +157,7 @@ function ToolRow({ tool, onPermissionChange, isSaved }) { options={permissionStates} onChange={(value) => onPermissionChange(tool.name, value)} ariaLabel={t("tools.permissionFor", { name: tool.name })} + aria-busy={pendingPermission != null} className="w-36 sm:w-44" data-testid="settings-tool-permission-select" /> @@ -179,7 +181,14 @@ export function ToolsTab({ searchQuery = "", }) { const t = useT(); - const { tools, query, setPermission, savedTools, error: permissionError } = useTools(); + const { + tools, + query, + setPermission, + savedTools, + pendingPermissions, + error: permissionError, + } = useTools(); if (query.isLoading) { return ( @@ -283,6 +292,7 @@ export function ToolsTab({ diff --git a/crates/ironclaw_webui/frontend/src/pages/settings/hooks/useTools.test.ts b/crates/ironclaw_webui/frontend/src/pages/settings/hooks/useTools.test.ts index 2ff5e39d40b..3b0ab950068 100644 --- a/crates/ironclaw_webui/frontend/src/pages/settings/hooks/useTools.test.ts +++ b/crates/ironclaw_webui/frontend/src/pages/settings/hooks/useTools.test.ts @@ -5,9 +5,24 @@ import { runVmModuleForTest } from "../../../test-support/vm-module-harness"; function loadUseTools({ mutationError = null } = {}) { const calls = []; + const callbackDependencies = []; + const clearedTimeouts = []; + const activeTimeouts = new Map(); + let mutationOptions; + let effectCleanup; + let nextTimeoutId = 0; + const mutate = (payload) => calls.push({ type: "mutate", payload }); + const reset = () => calls.push({ type: "reset" }); const context = { React: { - useCallback: (fn) => fn, + useCallback: (fn, dependencies) => { + callbackDependencies.push(dependencies); + return fn; + }, + useEffect: (effect) => { + effectCleanup = effect(); + }, + useRef: (initial) => ({ current: initial }), useState: (initial) => [ initial, (updater) => calls.push({ type: "setState", updater }), @@ -17,22 +32,41 @@ function loadUseTools({ mutationError = null } = {}) { globalThis: {}, throwIfApiFailed: (value) => value, updateToolPermission: () => {}, - useMutation: (options) => ({ - error: mutationError, - mutate: (payload) => calls.push({ type: "mutate", payload }), - reset: () => calls.push({ type: "reset" }), - options, - }), + useMutation: (options) => { + mutationOptions = options; + return { + error: mutationError, + mutate, + reset, + options, + }; + }, useQuery: () => ({ data: { tools: [] }, isLoading: false, error: null }), useQueryClient: () => ({ + invalidateQueries: (...args) => calls.push({ type: "invalidateQueries", args }), setQueryData: (...args) => calls.push({ type: "setQueryData", args }), }), + clearTimeout: (timeoutId) => { + clearedTimeouts.push(timeoutId); + activeTimeouts.delete(timeoutId); + }, + setTimeout: (callback, delay) => { + nextTimeoutId += 1; + activeTimeouts.set(nextTimeoutId, { callback, delay }); + return nextTimeoutId; + }, }; const exports = runVmModuleForTest("./useTools.ts", ["useTools"], context, import.meta.url); return { calls, + activeTimeouts, + callbackDependencies, + clearedTimeouts, + getEffectCleanup: () => effectCleanup, + getMutationOptions: () => mutationOptions, + mutationFunctions: { mutate, reset }, useTools: exports.useTools, }; } @@ -44,7 +78,142 @@ test("setPermission clears the previous mutation error before retrying save", () tools.setPermission("builtin.echo", "disabled"); assert.equal(calls[0].type, "reset"); - assert.equal(calls[1].type, "mutate"); - assert.equal(calls[1].payload.name, "builtin.echo"); - assert.equal(calls[1].payload.state, "disabled"); + const mutation = calls.find((call) => call.type === "mutate"); + assert.equal(mutation.payload.name, "builtin.echo"); + assert.equal(mutation.payload.state, "disabled"); +}); + +test("setPermission immediately retains the selected permission while saving", () => { + const { calls, useTools } = loadUseTools(); + const tools = useTools(); + + tools.setPermission("builtin.echo", "disabled"); + + const pendingUpdate = calls.find((call) => call.type === "setState"); + assert.ok(pendingUpdate, "expected the pending permission to update immediately"); + assert.deepEqual(JSON.parse(JSON.stringify(pendingUpdate.updater({}))), { + "builtin.echo": { requestId: 1, state: "disabled" }, + }); + assert.equal(calls.at(-1).type, "mutate"); + assert.equal(calls.at(-1).payload.requestId, 1); +}); + +test("failed permission saves clear the pending selection so the server value is restored", () => { + const { calls, getMutationOptions, useTools } = loadUseTools(); + const tools = useTools(); + + tools.setPermission("builtin.echo", "disabled"); + const pendingPermission = calls.find((call) => call.type === "setState").updater({}); + const variables = calls.find((call) => call.type === "mutate").payload; + + getMutationOptions().onError(new Error("permission denied"), variables); + + const rollbackUpdate = calls.filter((call) => call.type === "setState").at(-1); + assert.deepEqual(JSON.parse(JSON.stringify(rollbackUpdate.updater(pendingPermission))), {}); + const invalidation = calls.find((call) => call.type === "invalidateQueries"); + assert.deepEqual(JSON.parse(JSON.stringify(invalidation.args[0])), { + queryKey: ["settings-tools"], + }); +}); + +test("a newer failed save refetches after an older successful response is ignored", () => { + const { calls, getMutationOptions, useTools } = loadUseTools(); + const tools = useTools(); + + tools.setPermission("builtin.echo", "always_allow"); + tools.setPermission("builtin.echo", "disabled"); + const mutations = calls.filter((call) => call.type === "mutate"); + const pendingUpdates = calls.filter((call) => call.type === "setState"); + const pendingPermissions = pendingUpdates.reduce( + (state, update) => update.updater(state), + {} + ); + + getMutationOptions().onSuccess( + { tool: { name: "builtin.echo", state: "always_allow" } }, + mutations[0].payload + ); + assert.equal(calls.some((call) => call.type === "setQueryData"), false); + + getMutationOptions().onError(new Error("permission denied"), mutations[1].payload); + + const rollbackUpdate = calls.filter((call) => call.type === "setState").at(-1); + assert.deepEqual( + JSON.parse(JSON.stringify(rollbackUpdate.updater(pendingPermissions))), + {} + ); + assert.equal(calls.filter((call) => call.type === "invalidateQueries").length, 2); +}); + +test("an older successful response cannot overwrite a newer successful save", () => { + const { calls, getMutationOptions, useTools } = loadUseTools(); + const tools = useTools(); + + tools.setPermission("builtin.echo", "always_allow"); + tools.setPermission("builtin.echo", "disabled"); + const mutations = calls.filter((call) => call.type === "mutate"); + + getMutationOptions().onSuccess( + { tool: { name: "builtin.echo", state: "disabled" } }, + mutations[1].payload + ); + + const cacheUpdates = calls.filter((call) => call.type === "setQueryData"); + assert.equal(cacheUpdates.length, 1); + const latest = cacheUpdates[0].args[1]({ + tools: [{ name: "builtin.echo", state: "ask_each_time" }], + }); + assert.equal(latest.tools[0].state, "disabled"); + + getMutationOptions().onSuccess( + { tool: { name: "builtin.echo", state: "always_allow" } }, + mutations[0].payload + ); + + assert.equal(calls.filter((call) => call.type === "setQueryData").length, 1); + assert.equal(calls.filter((call) => call.type === "invalidateQueries").length, 1); +}); + +test("saved indicator timeouts restart per tool and are cleared on unmount", () => { + const { + activeTimeouts, + calls, + clearedTimeouts, + getEffectCleanup, + getMutationOptions, + useTools, + } = loadUseTools(); + const tools = useTools(); + + tools.setPermission("builtin.echo", "always_allow"); + let mutation = calls.filter((call) => call.type === "mutate").at(-1); + getMutationOptions().onSuccess( + { tool: { name: "builtin.echo", state: "always_allow" } }, + mutation.payload + ); + assert.deepEqual([...activeTimeouts.keys()], [1]); + + tools.setPermission("builtin.echo", "disabled"); + mutation = calls.filter((call) => call.type === "mutate").at(-1); + getMutationOptions().onSuccess( + { tool: { name: "builtin.echo", state: "disabled" } }, + mutation.payload + ); + assert.deepEqual(clearedTimeouts, [1]); + assert.deepEqual([...activeTimeouts.keys()], [2]); + assert.equal(activeTimeouts.get(2).delay, 2000); + + getEffectCleanup()(); + assert.deepEqual(clearedTimeouts, [1, 2]); + assert.equal(activeTimeouts.size, 0); +}); + +test("setPermission depends on stable mutation functions", () => { + const { callbackDependencies, mutationFunctions, useTools } = loadUseTools(); + + useTools(); + + const dependencies = callbackDependencies.at(-1); + assert.equal(dependencies[0], mutationFunctions.mutate); + assert.equal(dependencies[1], mutationFunctions.reset); }); diff --git a/crates/ironclaw_webui/frontend/src/pages/settings/hooks/useTools.ts b/crates/ironclaw_webui/frontend/src/pages/settings/hooks/useTools.ts index a740607b7a3..a83646f803a 100644 --- a/crates/ironclaw_webui/frontend/src/pages/settings/hooks/useTools.ts +++ b/crates/ironclaw_webui/frontend/src/pages/settings/hooks/useTools.ts @@ -14,13 +14,41 @@ export function useTools() { const tools = query.data?.tools || []; const [savedTools, setSavedTools] = React.useState({}); + const [pendingPermissions, setPendingPermissions] = React.useState({}); + const nextRequestId = React.useRef(0); + const pendingRequestIds = React.useRef({}); + const savedTimeouts = React.useRef({}); + + React.useEffect(() => { + return () => { + for (const timeoutId of Object.values(savedTimeouts.current)) { + clearTimeout(timeoutId); + } + savedTimeouts.current = {}; + }; + }, []); + + const clearPendingPermission = React.useCallback((name, requestId) => { + if (pendingRequestIds.current[name] !== requestId) return; + delete pendingRequestIds.current[name]; + setPendingPermissions((prev) => { + if (prev[name]?.requestId !== requestId) return prev; + const next = { ...prev }; + delete next[name]; + return next; + }); + }, []); const mutation = useMutation({ // Treat `success: false` as a failed save so the UI never shows a fake // "Saved" indicator for a permission change that didn't persist. mutationFn: async ({ name, state }) => throwIfApiFailed(await updateToolPermission(name, state), "Save failed"), - onSuccess: (data, { name, state }) => { + onSuccess: (data, { name, state, requestId }) => { + if (pendingRequestIds.current[name] !== requestId) { + queryClient.invalidateQueries({ queryKey: ["settings-tools"] }); + return; + } queryClient.setQueryData(["settings-tools"], (old) => { if (!old) return old; const updatedTool = data?.tool; @@ -31,18 +59,43 @@ export function useTools() { ), }; }); + clearPendingPermission(name, requestId); setSavedTools((prev) => ({ ...prev, [name]: true })); - setTimeout(() => setSavedTools((prev) => ({ ...prev, [name]: false })), 2000); + if (savedTimeouts.current[name]) { + clearTimeout(savedTimeouts.current[name]); + } + savedTimeouts.current[name] = setTimeout(() => { + setSavedTools((prev) => ({ ...prev, [name]: false })); + delete savedTimeouts.current[name]; + }, 2000); + }, + onError: (_error, { name, requestId }) => { + clearPendingPermission(name, requestId); + queryClient.invalidateQueries({ queryKey: ["settings-tools"] }); }, }); const setPermission = React.useCallback( (name, state) => { + const requestId = nextRequestId.current + 1; + nextRequestId.current = requestId; + pendingRequestIds.current[name] = requestId; mutation.reset(); - mutation.mutate({ name, state }); + setPendingPermissions((prev) => ({ + ...prev, + [name]: { requestId, state }, + })); + mutation.mutate({ name, state, requestId }); }, - [mutation] + [mutation.mutate, mutation.reset] ); - return { tools, query, setPermission, savedTools, error: mutation.error }; + return { + tools, + query, + setPermission, + savedTools, + pendingPermissions, + error: mutation.error, + }; } diff --git a/crates/ironclaw_webui/frontend/src/pages/settings/lib/settings-api.test.ts b/crates/ironclaw_webui/frontend/src/pages/settings/lib/settings-api.test.ts index ae97acd77ed..589fda5bed7 100644 --- a/crates/ironclaw_webui/frontend/src/pages/settings/lib/settings-api.test.ts +++ b/crates/ironclaw_webui/frontend/src/pages/settings/lib/settings-api.test.ts @@ -1,11 +1,35 @@ import assert from "node:assert/strict"; -import { test } from "vitest"; +import { test, vi } from "vitest"; import { settingsFromOperatorConfig, toolFromConfigEntry, + updateToolPermission, } from "./settings-api"; +function toolPermissionResponse({ + name = "builtin.echo", + state = "disabled", + defaultState = "ask_each_time", + source = "override", + mutable = true, +} = {}) { + return { + entry: { + key: `tool.${name}`, + value: { + name, + state, + default_state: defaultState, + locked: !mutable, + effective_source: source, + }, + mutable, + source, + }, + }; +} + test("settingsFromOperatorConfig maps the global auto-approve key", () => { const settings = settingsFromOperatorConfig({ entries: [ @@ -64,3 +88,127 @@ test("toolFromConfigEntry normalizes legacy and malformed permission values", () } ); }); + +test("updateToolPermission aborts a lost request after the bounded save timeout", async () => { + vi.useFakeTimers(); + let requestOptions; + vi.stubGlobal("sessionStorage", { + getItem: () => "", + removeItem: () => {}, + setItem: () => {}, + }); + vi.stubGlobal( + "fetch", + (_path, options) => + new Promise((_resolve, reject) => { + requestOptions = options; + options.signal.addEventListener( + "abort", + () => reject(new Error("permission save aborted")), + { once: true } + ); + }) + ); + + try { + const update = updateToolPermission("builtin.echo", "disabled"); + const rejected = assert.rejects(update, /permission save aborted/); + + await vi.advanceTimersByTimeAsync(30_000); + await rejected; + + assert.equal(requestOptions.signal.aborted, true); + assert.equal(JSON.parse(requestOptions.body).state, "disabled"); + } finally { + vi.useRealTimers(); + vi.unstubAllGlobals(); + } +}); + +test("updateToolPermission rejects missing or malformed persisted entries", async () => { + const responses = [{}, toolPermissionResponse({ state: "unexpected" })]; + vi.stubGlobal("sessionStorage", { + getItem: () => "", + removeItem: () => {}, + setItem: () => {}, + }); + vi.stubGlobal( + "fetch", + async () => + new Response(JSON.stringify(responses.shift()), { + status: 200, + headers: { "Content-Type": "application/json" }, + }) + ); + + try { + await assert.rejects( + updateToolPermission("builtin.echo", "disabled"), + /missing a valid persisted tool entry/ + ); + await assert.rejects( + updateToolPermission("builtin.echo", "disabled"), + /missing a valid persisted tool entry/ + ); + } finally { + vi.unstubAllGlobals(); + } +}); + +test("updateToolPermission requires the persisted entry to confirm the requested state", async () => { + vi.stubGlobal("sessionStorage", { + getItem: () => "", + removeItem: () => {}, + setItem: () => {}, + }); + vi.stubGlobal( + "fetch", + async () => + new Response(JSON.stringify(toolPermissionResponse({ state: "ask_each_time" })), { + status: 200, + headers: { "Content-Type": "application/json" }, + }) + ); + + try { + await assert.rejects( + updateToolPermission("builtin.echo", "disabled"), + /did not confirm the requested tool state/ + ); + } finally { + vi.unstubAllGlobals(); + } +}); + +test("updateToolPermission accepts canonical override and default responses", async () => { + const responses = [ + toolPermissionResponse(), + toolPermissionResponse({ state: "ask_each_time", source: "global" }), + ]; + vi.stubGlobal("sessionStorage", { + getItem: () => "", + removeItem: () => {}, + setItem: () => {}, + }); + vi.stubGlobal( + "fetch", + async () => + new Response(JSON.stringify(responses.shift()), { + status: 200, + headers: { "Content-Type": "application/json" }, + }) + ); + + try { + const override = await updateToolPermission("builtin.echo", "disabled"); + assert.equal(override.success, true); + assert.equal(override.tool.state, "disabled"); + + const inherited = await updateToolPermission("builtin.echo", "default"); + assert.equal(inherited.success, true); + assert.equal(inherited.tool.state, "ask_each_time"); + assert.equal(inherited.tool.effective_source, "global"); + } finally { + vi.unstubAllGlobals(); + } +}); diff --git a/crates/ironclaw_webui/frontend/src/pages/settings/lib/settings-api.ts b/crates/ironclaw_webui/frontend/src/pages/settings/lib/settings-api.ts index 39d20d456d1..415d622812e 100644 --- a/crates/ironclaw_webui/frontend/src/pages/settings/lib/settings-api.ts +++ b/crates/ironclaw_webui/frontend/src/pages/settings/lib/settings-api.ts @@ -4,6 +4,7 @@ const OPERATOR_CONFIG_BASE = "/api/webchat/v2/operator/config"; const SETTINGS_TOOLS_BASE = "/api/webchat/v2/settings/tools"; const AUTO_APPROVE_KEY = "agent.auto_approve_tools"; const TOOL_PREFIX = "tool."; +const TOOL_PERMISSION_UPDATE_TIMEOUT_MS = 30_000; const TOOL_PERMISSION_STATES = new Set(["always_allow", "ask_each_time", "disabled"]); const TOOL_PERMISSION_UPDATE_STATES = new Set([ "default", @@ -26,6 +27,35 @@ function normalizeEffectiveSource(source) { return ["default", "global", "override"].includes(source) ? source : "default"; } +function persistedToolFromConfigEntry(entry, expectedName, requestedState) { + const value = entry?.value; + const hasPersistedShape = + entry?.key === `${TOOL_PREFIX}${expectedName}` && + value != null && + typeof value === "object" && + !Array.isArray(value) && + value.name === expectedName && + TOOL_PERMISSION_STATES.has(value.state) && + TOOL_PERMISSION_STATES.has(value.default_state) && + typeof value.locked === "boolean" && + ["default", "global", "override", "locked"].includes(value.effective_source) && + entry.source === value.effective_source && + typeof entry.mutable === "boolean"; + if (!hasPersistedShape) { + throw new Error("Permission save response is missing a valid persisted tool entry"); + } + + const tool = toolFromConfigEntry(entry); + const confirmsRequestedState = + requestedState === "default" + ? value.effective_source !== "override" + : tool?.state === requestedState; + if (!tool || !confirmsRequestedState) { + throw new Error("Permission save response did not confirm the requested tool state"); + } + return tool; +} + export function toolFromConfigEntry(entry) { if (!entry?.key?.startsWith(TOOL_PREFIX)) return null; const value = entry.value || {}; @@ -198,11 +228,19 @@ export async function fetchTools() { } export async function updateToolPermission(name, state) { const normalized = normalizeToolUpdateState(state); - const data = await apiFetch(`${SETTINGS_TOOLS_BASE}/${encodeURIComponent(name)}`, { - method: "POST", - body: JSON.stringify({ state: normalized }), - }); - return { success: true, tool: toolFromConfigEntry(data.entry), entry: data.entry }; + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), TOOL_PERMISSION_UPDATE_TIMEOUT_MS); + try { + const data = await apiFetch(`${SETTINGS_TOOLS_BASE}/${encodeURIComponent(name)}`, { + method: "POST", + body: JSON.stringify({ state: normalized }), + signal: controller.signal, + }); + const tool = persistedToolFromConfigEntry(data?.entry, name, normalized); + return { success: true, tool, entry: data.entry }; + } finally { + clearTimeout(timeoutId); + } } export function fetchExtensions() { return apiFetch("/api/webchat/v2/extensions"); diff --git a/tests/e2e/helpers.py b/tests/e2e/helpers.py index 13193c04fa6..9cfaeaf1e15 100644 --- a/tests/e2e/helpers.py +++ b/tests/e2e/helpers.py @@ -372,6 +372,9 @@ "settings_tool_row_for": ( "[data-testid='settings-tool-row'][data-tool-name='{name}']" ), + "settings_tool_permission_select": ( + "[data-testid='settings-tool-permission-select']" + ), "settings_tool_permission": ( "[data-testid='settings-tool-permission-select'] button[aria-haspopup='listbox']" ), diff --git a/tests/e2e/reborn_coverage_tests.txt b/tests/e2e/reborn_coverage_tests.txt index 15a16e111cd..1d51423ef9f 100644 --- a/tests/e2e/reborn_coverage_tests.txt +++ b/tests/e2e/reborn_coverage_tests.txt @@ -16,6 +16,7 @@ tests/e2e/scenarios/test_reborn_webui_v2_smoke.py tests/e2e/scenarios/test_reborn_webui_v2_legacy_pending_messages.py::test_reborn_legacy_send_failure_renders_inline_error_not_toast tests/e2e/scenarios/test_reborn_webui_v2_legacy_tool_permissions.py::test_reborn_legacy_tool_permission_menu_persists_after_reload +tests/e2e/scenarios/test_reborn_webui_v2_legacy_tool_permissions.py::test_reborn_legacy_tool_permission_retains_selection_while_saving tests/e2e/scenarios/test_reborn_webui_v2_legacy_extensions.py::test_reborn_v2_extension_registry_renders_while_installed_list_is_pending tests/e2e/scenarios/test_reborn_webui_v2_notifications.py::test_reborn_v2_error_toast_pauses_dismisses_and_stays_above_notifications tests/e2e/scenarios/test_reborn_v2_file_download.py diff --git a/tests/e2e/scenarios/test_reborn_webui_v2_legacy_tool_permissions.py b/tests/e2e/scenarios/test_reborn_webui_v2_legacy_tool_permissions.py index 6a2e57ac89a..5b4e99e2525 100644 --- a/tests/e2e/scenarios/test_reborn_webui_v2_legacy_tool_permissions.py +++ b/tests/e2e/scenarios/test_reborn_webui_v2_legacy_tool_permissions.py @@ -47,7 +47,11 @@ def _tool_entry( async def _open_mocked_tools_page( - reborn_v2_server, reborn_v2_browser, *, fail_permission_saves: bool = False + reborn_v2_server, + reborn_v2_browser, + *, + fail_permission_saves: bool = False, + delay_permission_saves: bool = False, ): context = await reborn_v2_browser.new_context(viewport={"width": 1280, "height": 720}) page = await context.new_page() @@ -70,6 +74,8 @@ async def _open_mocked_tools_page( } auto_approve_requests: list[dict] = [] permission_requests: list[dict] = [] + permission_save_started = asyncio.Event() + permission_save_release = asyncio.Event() def entries(): return [ @@ -132,6 +138,9 @@ async def handle_settings_tools(route): name = unquote(path.removeprefix("/api/webchat/v2/settings/tools/")) body = json.loads(request.post_data or "{}") permission_requests.append({"name": name, "body": body}) + permission_save_started.set() + if delay_permission_saves: + await permission_save_release.wait() if fail_permission_saves: await fulfill_json( route, @@ -186,6 +195,8 @@ async def handle_settings_tools(route): "page": page, "auto_approve_requests": auto_approve_requests, "permission_requests": permission_requests, + "permission_save_started": permission_save_started, + "permission_save_release": permission_save_release, } @@ -328,6 +339,41 @@ async def test_reborn_legacy_tool_permission_menu_persists_after_reload( await harness["context"].close() +async def test_reborn_legacy_tool_permission_retains_selection_while_saving( + reborn_v2_server, reborn_v2_browser +): + harness = await _open_mocked_tools_page( + reborn_v2_server, + reborn_v2_browser, + delay_permission_saves=True, + ) + try: + page = harness["page"] + button = _permission_button(page, "echo") + await expect(button).to_contain_text("Always allow", timeout=5000) + + await _choose_permission(page, "echo", "Ask each time") + await asyncio.wait_for(harness["permission_save_started"].wait(), timeout=5) + + select = _tool_row(page, "echo").locator( + SEL_V2["settings_tool_permission_select"] + ) + await expect(select).to_have_attribute("aria-busy", "true") + await expect(button).to_contain_text("Ask each time") + assert harness["permission_requests"][-1] == { + "name": "echo", + "body": {"state": "ask_each_time"}, + } + + harness["permission_save_release"].set() + await expect(_tool_row(page, "echo").get_by_text("saved")).to_be_visible( + timeout=5000 + ) + finally: + harness["permission_save_release"].set() + await harness["context"].close() + + async def test_reborn_legacy_tool_permission_save_failure_shows_error( reborn_v2_server, reborn_v2_browser ):